netdev - Re: [PATCH net-next 3/3] net: ethernet: ti: cpsw

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: <88515afd-6c5a-4245-bcff-d3340e94aa26@kernel.org>
Date: Thu, 12 Sep 2024 16:29:29 +0300
From: Roger Quadros <rogerq@...nel.org>
To: Simon Horman <horms@...nel.org>
Cc: "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Siddharth Vadapalli <s-vadapalli@...com>,
 Nathan Chancellor <nathan@...nel.org>,
 Nick Desaulniers <ndesaulniers@...gle.com>, Bill Wendling
 <morbo@...gle.com>, Justin Stitt <justinstitt@...gle.com>,
 netdev@...r.kernel.org, linux-omap@...r.kernel.org, llvm@...ts.linux.dev
Subject: Re: [PATCH net-next 3/3] net: ethernet: ti: cpsw_ale: Remove unused
 accessor functions



On 12/09/2024 14:27, Simon Horman wrote:
> On Thu, Sep 12, 2024 at 01:54:45PM +0300, Roger Quadros wrote:
>>
>>
>> On 12/09/2024 11:59, Simon Horman wrote:
>>> On Thu, Sep 12, 2024 at 10:07:27AM +0300, Roger Quadros wrote:
>>>> Hi Simon,
>>>>
>>>> On 10/09/2024 10:17, Simon Horman wrote:
> 
> ...
> 
>>>>>  	ALE_ENT_VID_MEMBER_LIST = 0,
>>>>>  	ALE_ENT_VID_UNREG_MCAST_MSK,
>>>>> @@ -217,14 +229,14 @@ static const struct ale_entry_fld vlan_entry_k3_cpswxg[] = {
>>>>>  
>>>>>  DEFINE_ALE_FIELD(entry_type,		60,	2)
>>>>>  DEFINE_ALE_FIELD(vlan_id,		48,	12)
>>>>> -DEFINE_ALE_FIELD(mcast_state,		62,	2)
>>>>> +DEFINE_ALE_FIELD_SET(mcast_state,	62,	2)
>>>>
>>>> I don't understand why we need separate macros for GET and SET.
>>>> The original intent was to use one macro for both.
>>>>
>>>> Otherwise we will have to add DEFINE_ALE_FIELD/1_SET to all the fields.
>>>
>>> Hi Roger,
>>>
>>> Sorry for not being clearer.
>>>
>>> My intent was to avoid declaring functions that are never used.
>>> Perhaps it is best explained by some examples.
>>>
>>> In the case of mcast_state, the compiler flags that the get accessor is
>>> never used. The intent is of this patch addresses that by declaring the set
>>> accessor for mcast_state. Likewise for other similar cases.
>>>
>>> OTOH, in the case of, f.e. vlan_id, the set and get accessor functions are
>>> both used, and DEFINE_ALE_FIELD continues to be used to define them both.
>>> DEFINE_ALE_FIELD is implemented as the combination of _SET and _GET.
>>>
>>
>> Thanks for the explanation Simon. I understand now.
>>
>> Would using __maybe_unused__ be preferable to get rid of the warnings?
>> That way we don't need to care if both set/get helpers are used or not
>> and don't have to touch the below code ever again except to add new fields.
> 
> Thanks Roger,
> 
> IMHO, it is nicer to not declare them at all.  But I do get your point and
> I'm happy to try that approach if you prefer it.
> 
> ...

Simon,

I don't have any preference. I'll leave it to you to decide on your next spin. Thanks.

-- 
cheers,
-roger