Search Linux Wireless

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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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@xxxxxxxxxxxxxxx> 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.





[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Wireless Regulations]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux