Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <c1040c2c-477e-9598-8a7b-bfea88a0375b@linux.intel.com>
Date:   Tue, 15 Feb 2022 18:24:46 -0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.6.0
Subject: Re: [PATCH net-next v4 02/13] net: wwan: t7xx: Add control DMA
 interface
Content-Language: en-US
To:     Sergey Ryazanov <ryazanov.s.a@gmail.com>
Cc:     =?UTF-8?Q?Ilpo_J=c3=a4rvinen?= <ilpo.jarvinen@linux.intel.com>,
        Netdev <netdev@vger.kernel.org>, linux-wireless@vger.kernel.org,
        Jakub Kicinski <kuba@kernel.org>,
        David Miller <davem@davemloft.net>,
        Johannes Berg <johannes@sipsolutions.net>,
        Loic Poulain <loic.poulain@linaro.org>,
        M Chetan Kumar <m.chetan.kumar@intel.com>,
        chandrashekar.devegowda@intel.com,
        Intel Corporation <linuxwwan@intel.com>,
        chiranjeevi.rapolu@linux.intel.com,
        =?UTF-8?B?SGFpanVuIExpdSAo5YiY5rW35YabKQ==?= 
        <haijun.liu@mediatek.com>, amir.hanania@intel.com,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        dinesh.sharma@intel.com, eliot.lee@intel.com,
        moises.veleta@intel.com, pierre-louis.bossart@intel.com,
        muralidharan.sethuraman@intel.com, Soumya.Prakash.Mishra@intel.com,
        sreehari.kancharla@intel.com
References: <20220114010627.21104-1-ricardo.martinez@linux.intel.com>
 <20220114010627.21104-3-ricardo.martinez@linux.intel.com>
 <d5854453-84b-1eba-7cc7-d94f41a185d@linux.intel.com>
 <4a4b2848-d665-c9ba-c66a-dd4408e94ea5@linux.intel.com>
 <CAHNKnsT9y0ssM3zVriEdEzoRMuJyianKrOx4BAcmT80PCJBigg@mail.gmail.com>
From:   "Martinez, Ricardo" <ricardo.martinez@linux.intel.com>
In-Reply-To: <CAHNKnsT9y0ssM3zVriEdEzoRMuJyianKrOx4BAcmT80PCJBigg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


On 2/10/2022 4:25 PM, Sergey Ryazanov wrote:
> Hello Ricardo,
>
> On Wed, Jan 19, 2022 at 1:22 AM Martinez, Ricardo
> <ricardo.martinez@linux.intel.com> wrote:
>> On 1/18/2022 6:13 AM, Ilpo Järvinen wrote:
>>> On Thu, 13 Jan 2022, Ricardo Martinez wrote:
>> ...
>>>> +#define CLDMA_NUM 2
>>> I tried to understand its purpose but it seems that only one of the
>>> indexes is used in the arrays where this define gives the size? Related to
>>> this, ID_CLDMA0 is not used anywhere?
>> The modem HW has 2 CLDMAs, idx 0 for the app processor (SAP) and idx 1
>> for the modem (MD).
>>
>> CLDMA_NUM is defined as 2 to reflect the HW capabilities but mainly to
>> have a cleaner upcoming patches, which will use ID_CLDMA0.
>>
>> If having array's of size 1 is not a problem then we can define
>> CLDMA_NUM as 1 and play with the CLDMA indexes.
> Please keep CLDMA_NUM defined as 2. Especially if you have a plan for
> further driver development. Saving a few bytes in the structure for a
> short term is not worth the jungling with indexes, possible errors and
> further rework. Just document them as suggested by Ilpo and mark idx 0
> as unused at the moment.
>
> BTW, did you consider to define the cldma_id enum something like this:
>
> /**
>   * ...
>   * @CLDMA_ID_AP: ... (unused ATM)
>   * @CLDMA_ID_MD: ...
>   */
> enum cldma_id {
>      CLDMA_ID_AP = 0,
>      CLDMA_ID_MD = 1,
>
>      CLDMA_NUM
> };
>
> This way elements will be self descriptive as well as CLDMA_NUM value
> will be less puzzled.

I agree.

Actually, we already did the enum name changes, we'll incorporate the 
rest of the feedback.

>