netdev - Re: [PATCH net-next v4 02/13] net: wwan: t7xx: Add control DMA interface

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHNKnsT9y0ssM3zVriEdEzoRMuJyianKrOx4BAcmT80PCJBigg@mail.gmail.com>
Date:   Fri, 11 Feb 2022 03:25:31 +0300
From:   Sergey Ryazanov <ryazanov.s.a@...il.com>
To:     "Martinez, Ricardo" <ricardo.martinez@...ux.intel.com>
Cc:     Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
        Netdev <netdev@...r.kernel.org>, linux-wireless@...r.kernel.org,
        Jakub Kicinski <kuba@...nel.org>,
        David Miller <davem@...emloft.net>,
        Johannes Berg <johannes@...solutions.net>,
        Loic Poulain <loic.poulain@...aro.org>,
        M Chetan Kumar <m.chetan.kumar@...el.com>,
        chandrashekar.devegowda@...el.com,
        Intel Corporation <linuxwwan@...el.com>,
        chiranjeevi.rapolu@...ux.intel.com,
        Haijun Liu (刘海军) 
        <haijun.liu@...iatek.com>, amir.hanania@...el.com,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        dinesh.sharma@...el.com, eliot.lee@...el.com,
        moises.veleta@...el.com, pierre-louis.bossart@...el.com,
        muralidharan.sethuraman@...el.com, Soumya.Prakash.Mishra@...el.com,
        sreehari.kancharla@...el.com
Subject: Re: [PATCH net-next v4 02/13] net: wwan: t7xx: Add control DMA interface

Hello Ricardo,

On Wed, Jan 19, 2022 at 1:22 AM Martinez, Ricardo
<ricardo.martinez@...ux.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.

-- 
Sergey