| Message ID | 20240910-ti-warn-v1-3-afd1e404abbe@kernel.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | net: ethernet: ti: Address some warnings | expand |
Hi Simon, On 10/09/2024 10:17, Simon Horman wrote: > W=1 builds flag that some accessor functions for ALE fields are unused. > > Address this by splitting up the macros used to define these > accessors to allow only those that are used to be declared. > > The warnings are verbose, but for example, the mcast_state case is > flagged by clang-18 as: > > .../cpsw_ale.c:220:1: warning: unused function 'cpsw_ale_get_mcast_state' [-Wunused-function] > 220 | DEFINE_ALE_FIELD(mcast_state, 62, 2) > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > .../cpsw_ale.c:145:19: note: expanded from macro 'DEFINE_ALE_FIELD' > 145 | static inline int cpsw_ale_get_##name(u32 *ale_entry) \ > | ^~~~~~~~~~~~~~~~~~~ > <scratch space>:196:1: note: expanded from here > 196 | cpsw_ale_get_mcast_state > | ^~~~~~~~~~~~~~~~~~~~~~~~ > > Compile tested only. > No functional change intended. > > Signed-off-by: Simon Horman <horms@kernel.org> > --- > drivers/net/ethernet/ti/cpsw_ale.c | 30 +++++++++++++++++++++--------- > 1 file changed, 21 insertions(+), 9 deletions(-) > > diff --git a/drivers/net/ethernet/ti/cpsw_ale.c b/drivers/net/ethernet/ti/cpsw_ale.c > index 64bf22cd860c..d37b4ddd6787 100644 > --- a/drivers/net/ethernet/ti/cpsw_ale.c > +++ b/drivers/net/ethernet/ti/cpsw_ale.c > @@ -141,27 +141,39 @@ static inline void cpsw_ale_set_field(u32 *ale_entry, u32 start, u32 bits, > ale_entry[idx] |= (value << start); > } > > -#define DEFINE_ALE_FIELD(name, start, bits) \ > +#define DEFINE_ALE_FIELD_GET(name, start, bits) \ > static inline int cpsw_ale_get_##name(u32 *ale_entry) \ > { \ > return cpsw_ale_get_field(ale_entry, start, bits); \ > -} \ > +} > + > +#define DEFINE_ALE_FIELD_SET(name, start, bits) \ > static inline void cpsw_ale_set_##name(u32 *ale_entry, u32 value) \ > { \ > cpsw_ale_set_field(ale_entry, start, bits, value); \ > } > > -#define DEFINE_ALE_FIELD1(name, start) \ > +#define DEFINE_ALE_FIELD(name, start, bits) \ > +DEFINE_ALE_FIELD_GET(name, start, bits) \ > +DEFINE_ALE_FIELD_SET(name, start, bits) > + > +#define DEFINE_ALE_FIELD1_GET(name, start) \ > static inline int cpsw_ale_get_##name(u32 *ale_entry, u32 bits) \ > { \ > return cpsw_ale_get_field(ale_entry, start, bits); \ > -} \ > +} > + > +#define DEFINE_ALE_FIELD1_SET(name, start) \ > static inline void cpsw_ale_set_##name(u32 *ale_entry, u32 value, \ > u32 bits) \ > { \ > cpsw_ale_set_field(ale_entry, start, bits, value); \ > } > > +#define DEFINE_ALE_FIELD1(name, start) \ > +DEFINE_ALE_FIELD1_GET(name, start) \ > +DEFINE_ALE_FIELD1_SET(name, start) > + > enum { > 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. > DEFINE_ALE_FIELD1(port_mask, 66) > DEFINE_ALE_FIELD(super, 65, 1) > DEFINE_ALE_FIELD(ucast_type, 62, 2) > -DEFINE_ALE_FIELD1(port_num, 66) > -DEFINE_ALE_FIELD(blocked, 65, 1) > -DEFINE_ALE_FIELD(secure, 64, 1) > -DEFINE_ALE_FIELD(mcast, 40, 1) > +DEFINE_ALE_FIELD1_SET(port_num, 66) > +DEFINE_ALE_FIELD_SET(blocked, 65, 1) > +DEFINE_ALE_FIELD_SET(secure, 64, 1) > +DEFINE_ALE_FIELD_GET(mcast, 40, 1) > > #define NU_VLAN_UNREG_MCAST_IDX 1 > >
On Thu, Sep 12, 2024 at 10:07:27AM +0300, Roger Quadros wrote: > Hi Simon, > > On 10/09/2024 10:17, Simon Horman wrote: > > W=1 builds flag that some accessor functions for ALE fields are unused. > > > > Address this by splitting up the macros used to define these > > accessors to allow only those that are used to be declared. > > > > The warnings are verbose, but for example, the mcast_state case is > > flagged by clang-18 as: > > > > .../cpsw_ale.c:220:1: warning: unused function 'cpsw_ale_get_mcast_state' [-Wunused-function] > > 220 | DEFINE_ALE_FIELD(mcast_state, 62, 2) > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > .../cpsw_ale.c:145:19: note: expanded from macro 'DEFINE_ALE_FIELD' > > 145 | static inline int cpsw_ale_get_##name(u32 *ale_entry) \ > > | ^~~~~~~~~~~~~~~~~~~ > > <scratch space>:196:1: note: expanded from here > > 196 | cpsw_ale_get_mcast_state > > | ^~~~~~~~~~~~~~~~~~~~~~~~ > > > > Compile tested only. > > No functional change intended. > > > > Signed-off-by: Simon Horman <horms@kernel.org> > > --- > > drivers/net/ethernet/ti/cpsw_ale.c | 30 +++++++++++++++++++++--------- > > 1 file changed, 21 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/net/ethernet/ti/cpsw_ale.c b/drivers/net/ethernet/ti/cpsw_ale.c > > index 64bf22cd860c..d37b4ddd6787 100644 > > --- a/drivers/net/ethernet/ti/cpsw_ale.c > > +++ b/drivers/net/ethernet/ti/cpsw_ale.c > > @@ -141,27 +141,39 @@ static inline void cpsw_ale_set_field(u32 *ale_entry, u32 start, u32 bits, > > ale_entry[idx] |= (value << start); > > } > > > > -#define DEFINE_ALE_FIELD(name, start, bits) \ > > +#define DEFINE_ALE_FIELD_GET(name, start, bits) \ > > static inline int cpsw_ale_get_##name(u32 *ale_entry) \ > > { \ > > return cpsw_ale_get_field(ale_entry, start, bits); \ > > -} \ > > +} > > + > > +#define DEFINE_ALE_FIELD_SET(name, start, bits) \ > > static inline void cpsw_ale_set_##name(u32 *ale_entry, u32 value) \ > > { \ > > cpsw_ale_set_field(ale_entry, start, bits, value); \ > > } > > > > -#define DEFINE_ALE_FIELD1(name, start) \ > > +#define DEFINE_ALE_FIELD(name, start, bits) \ > > +DEFINE_ALE_FIELD_GET(name, start, bits) \ > > +DEFINE_ALE_FIELD_SET(name, start, bits) > > + > > +#define DEFINE_ALE_FIELD1_GET(name, start) \ > > static inline int cpsw_ale_get_##name(u32 *ale_entry, u32 bits) \ > > { \ > > return cpsw_ale_get_field(ale_entry, start, bits); \ > > -} \ > > +} > > + > > +#define DEFINE_ALE_FIELD1_SET(name, start) \ > > static inline void cpsw_ale_set_##name(u32 *ale_entry, u32 value, \ > > u32 bits) \ > > { \ > > cpsw_ale_set_field(ale_entry, start, bits, value); \ > > } > > > > +#define DEFINE_ALE_FIELD1(name, start) \ > > +DEFINE_ALE_FIELD1_GET(name, start) \ > > +DEFINE_ALE_FIELD1_SET(name, start) > > + > > enum { > > 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. > > > DEFINE_ALE_FIELD1(port_mask, 66) > > DEFINE_ALE_FIELD(super, 65, 1) > > DEFINE_ALE_FIELD(ucast_type, 62, 2) > > -DEFINE_ALE_FIELD1(port_num, 66) > > -DEFINE_ALE_FIELD(blocked, 65, 1) > > -DEFINE_ALE_FIELD(secure, 64, 1) > > -DEFINE_ALE_FIELD(mcast, 40, 1) > > +DEFINE_ALE_FIELD1_SET(port_num, 66) > > +DEFINE_ALE_FIELD_SET(blocked, 65, 1) > > +DEFINE_ALE_FIELD_SET(secure, 64, 1) > > +DEFINE_ALE_FIELD_GET(mcast, 40, 1) > > > > #define NU_VLAN_UNREG_MCAST_IDX 1 > > > > > > -- > cheers, > -roger >
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: >>> W=1 builds flag that some accessor functions for ALE fields are unused. >>> >>> Address this by splitting up the macros used to define these >>> accessors to allow only those that are used to be declared. >>> >>> The warnings are verbose, but for example, the mcast_state case is >>> flagged by clang-18 as: >>> >>> .../cpsw_ale.c:220:1: warning: unused function 'cpsw_ale_get_mcast_state' [-Wunused-function] >>> 220 | DEFINE_ALE_FIELD(mcast_state, 62, 2) >>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >>> .../cpsw_ale.c:145:19: note: expanded from macro 'DEFINE_ALE_FIELD' >>> 145 | static inline int cpsw_ale_get_##name(u32 *ale_entry) \ >>> | ^~~~~~~~~~~~~~~~~~~ >>> <scratch space>:196:1: note: expanded from here >>> 196 | cpsw_ale_get_mcast_state >>> | ^~~~~~~~~~~~~~~~~~~~~~~~ >>> >>> Compile tested only. >>> No functional change intended. >>> >>> Signed-off-by: Simon Horman <horms@kernel.org> >>> --- >>> drivers/net/ethernet/ti/cpsw_ale.c | 30 +++++++++++++++++++++--------- >>> 1 file changed, 21 insertions(+), 9 deletions(-) >>> >>> diff --git a/drivers/net/ethernet/ti/cpsw_ale.c b/drivers/net/ethernet/ti/cpsw_ale.c >>> index 64bf22cd860c..d37b4ddd6787 100644 >>> --- a/drivers/net/ethernet/ti/cpsw_ale.c >>> +++ b/drivers/net/ethernet/ti/cpsw_ale.c >>> @@ -141,27 +141,39 @@ static inline void cpsw_ale_set_field(u32 *ale_entry, u32 start, u32 bits, >>> ale_entry[idx] |= (value << start); >>> } >>> >>> -#define DEFINE_ALE_FIELD(name, start, bits) \ >>> +#define DEFINE_ALE_FIELD_GET(name, start, bits) \ >>> static inline int cpsw_ale_get_##name(u32 *ale_entry) \ >>> { \ >>> return cpsw_ale_get_field(ale_entry, start, bits); \ >>> -} \ >>> +} >>> + >>> +#define DEFINE_ALE_FIELD_SET(name, start, bits) \ >>> static inline void cpsw_ale_set_##name(u32 *ale_entry, u32 value) \ >>> { \ >>> cpsw_ale_set_field(ale_entry, start, bits, value); \ >>> } >>> >>> -#define DEFINE_ALE_FIELD1(name, start) \ >>> +#define DEFINE_ALE_FIELD(name, start, bits) \ >>> +DEFINE_ALE_FIELD_GET(name, start, bits) \ >>> +DEFINE_ALE_FIELD_SET(name, start, bits) >>> + >>> +#define DEFINE_ALE_FIELD1_GET(name, start) \ >>> static inline int cpsw_ale_get_##name(u32 *ale_entry, u32 bits) \ >>> { \ >>> return cpsw_ale_get_field(ale_entry, start, bits); \ >>> -} \ >>> +} >>> + >>> +#define DEFINE_ALE_FIELD1_SET(name, start) \ >>> static inline void cpsw_ale_set_##name(u32 *ale_entry, u32 value, \ >>> u32 bits) \ >>> { \ >>> cpsw_ale_set_field(ale_entry, start, bits, value); \ >>> } >>> >>> +#define DEFINE_ALE_FIELD1(name, start) \ >>> +DEFINE_ALE_FIELD1_GET(name, start) \ >>> +DEFINE_ALE_FIELD1_SET(name, start) >>> + >>> enum { >>> 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. >> >>> DEFINE_ALE_FIELD1(port_mask, 66) >>> DEFINE_ALE_FIELD(super, 65, 1) >>> DEFINE_ALE_FIELD(ucast_type, 62, 2) >>> -DEFINE_ALE_FIELD1(port_num, 66) >>> -DEFINE_ALE_FIELD(blocked, 65, 1) >>> -DEFINE_ALE_FIELD(secure, 64, 1) >>> -DEFINE_ALE_FIELD(mcast, 40, 1) >>> +DEFINE_ALE_FIELD1_SET(port_num, 66) >>> +DEFINE_ALE_FIELD_SET(blocked, 65, 1) >>> +DEFINE_ALE_FIELD_SET(secure, 64, 1) >>> +DEFINE_ALE_FIELD_GET(mcast, 40, 1) >>> >>> #define NU_VLAN_UNREG_MCAST_IDX 1 >>> >>> >>
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. ...
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.
On Thu, Sep 12, 2024 at 04:29:29PM +0300, Roger Quadros wrote: > > > 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. Thanks, let me think about it. I'll probably hold off on v2 until the next the development cycle as the patch queue seems busy enough this week.
diff --git a/drivers/net/ethernet/ti/cpsw_ale.c b/drivers/net/ethernet/ti/cpsw_ale.c index 64bf22cd860c..d37b4ddd6787 100644 --- a/drivers/net/ethernet/ti/cpsw_ale.c +++ b/drivers/net/ethernet/ti/cpsw_ale.c @@ -141,27 +141,39 @@ static inline void cpsw_ale_set_field(u32 *ale_entry, u32 start, u32 bits, ale_entry[idx] |= (value << start); } -#define DEFINE_ALE_FIELD(name, start, bits) \ +#define DEFINE_ALE_FIELD_GET(name, start, bits) \ static inline int cpsw_ale_get_##name(u32 *ale_entry) \ { \ return cpsw_ale_get_field(ale_entry, start, bits); \ -} \ +} + +#define DEFINE_ALE_FIELD_SET(name, start, bits) \ static inline void cpsw_ale_set_##name(u32 *ale_entry, u32 value) \ { \ cpsw_ale_set_field(ale_entry, start, bits, value); \ } -#define DEFINE_ALE_FIELD1(name, start) \ +#define DEFINE_ALE_FIELD(name, start, bits) \ +DEFINE_ALE_FIELD_GET(name, start, bits) \ +DEFINE_ALE_FIELD_SET(name, start, bits) + +#define DEFINE_ALE_FIELD1_GET(name, start) \ static inline int cpsw_ale_get_##name(u32 *ale_entry, u32 bits) \ { \ return cpsw_ale_get_field(ale_entry, start, bits); \ -} \ +} + +#define DEFINE_ALE_FIELD1_SET(name, start) \ static inline void cpsw_ale_set_##name(u32 *ale_entry, u32 value, \ u32 bits) \ { \ cpsw_ale_set_field(ale_entry, start, bits, value); \ } +#define DEFINE_ALE_FIELD1(name, start) \ +DEFINE_ALE_FIELD1_GET(name, start) \ +DEFINE_ALE_FIELD1_SET(name, start) + enum { 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) DEFINE_ALE_FIELD1(port_mask, 66) DEFINE_ALE_FIELD(super, 65, 1) DEFINE_ALE_FIELD(ucast_type, 62, 2) -DEFINE_ALE_FIELD1(port_num, 66) -DEFINE_ALE_FIELD(blocked, 65, 1) -DEFINE_ALE_FIELD(secure, 64, 1) -DEFINE_ALE_FIELD(mcast, 40, 1) +DEFINE_ALE_FIELD1_SET(port_num, 66) +DEFINE_ALE_FIELD_SET(blocked, 65, 1) +DEFINE_ALE_FIELD_SET(secure, 64, 1) +DEFINE_ALE_FIELD_GET(mcast, 40, 1) #define NU_VLAN_UNREG_MCAST_IDX 1
W=1 builds flag that some accessor functions for ALE fields are unused. Address this by splitting up the macros used to define these accessors to allow only those that are used to be declared. The warnings are verbose, but for example, the mcast_state case is flagged by clang-18 as: .../cpsw_ale.c:220:1: warning: unused function 'cpsw_ale_get_mcast_state' [-Wunused-function] 220 | DEFINE_ALE_FIELD(mcast_state, 62, 2) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ .../cpsw_ale.c:145:19: note: expanded from macro 'DEFINE_ALE_FIELD' 145 | static inline int cpsw_ale_get_##name(u32 *ale_entry) \ | ^~~~~~~~~~~~~~~~~~~ <scratch space>:196:1: note: expanded from here 196 | cpsw_ale_get_mcast_state | ^~~~~~~~~~~~~~~~~~~~~~~~ Compile tested only. No functional change intended. Signed-off-by: Simon Horman <horms@kernel.org> --- drivers/net/ethernet/ti/cpsw_ale.c | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-)