Re: [PATCH net-next 3/3] net: ethernet: ti: cpsw_ale: Remove unused accessor functions

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

 



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.

...




[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux