Re: [PATCH v4 01/12] wifi: ath12k: add multiple radio support in a single MAC HW un/register

[Kalle Valo <kvalo@kernel.org>]

Rameshkumar Sundaram <quic_ramess@quicinc.com> writes:

> On 3/14/2024 1:26 AM, Jonas Gorski wrote:
>> On Wed, 13 Mar 2024 at 20:18, Jeff Johnson <quic_jjohnson@quicinc.com> wrote:
>>>
>>> On 3/13/2024 9:58 AM, Kalle Valo wrote:
>>>> Kalle Valo <kvalo@kernel.org> writes:
>>>>
>>>>> Jeff Johnson <quic_jjohnson@quicinc.com> writes:
>>>>>
>>>>>> On 3/13/2024 5:57 AM, Rameshkumar Sundaram wrote:
>>>>>>> On 3/13/2024 3:23 AM, Jeff Johnson wrote:
>>>>>>>> and guess we have to figure out how to suppress the ath12k-check issues with
>>>>>>>> this macro
>>>>>>> ath12k-check complains about the reuse of ah and index arguments which
>>>>>>> may get evaluated multiple times if its an arithmetic expression, But
>>>>>>> areas where we use the macro in our code aren't doing so.
>>>>>>> Do you have any suggestions here ? or shall we go back and use this
>>>>>>> for-loop inline.
>>>>>>
>>>>>> The macro makes sense -- we'll need to update the overrides in ath12k-check.
>>>>>
>>>>> IIRC it is possible to avoid variable reuse in macros with typeof()
>>>>> operator (or something like that). I can't remember the details right
>>>>> now but I think there are examples in the kernel code.
>>>>
>>>> Here's the GCC documentation with an example:
>>>>
>>>> https://gcc.gnu.org/onlinedocs/gcc/Typeof.html
>>>>
> Thanks Kalle for the references, as Jeff mentioned below, we need to
> reuse the arguments since we write to ar and index arguments on each
> iteration.
>
> Defining local vars using typeof() without limiting their scope (since
> we are defining a for_each iterator{}) leads other issues like
> redefinition of variables in functions where we use this macro more
> than once :(
>
> Also even if we somehow manage to convince check-patch, we'll still
> end up evaluating index and ar arguments in every iteration of loop.
> This just gives an impression to check-patch that the macro is unsafe
> (although logically its not).
> Experts, what is the standard we should follow here. Please suggest.

Yeah, typeof() won't help here as we can't create a local variable. Or
at least I can't come up way to do that safely, ideas very welcome.

I think it's just best to ignore the checkpatch warning for now, unless
better proposals come up. ath12k-check has functionality to ignore
specific warnings (see checkpatch_filter array), I can add this warning
to the array.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches