| Message ID | 20230622130507.606713-2-ben.dooks@codethink.co.uk (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [1/3] net: macb: check constant to define and fix __be32 warnings | expand |
On Thu, Jun 22, 2023 at 02:05:05PM +0100, Ben Dooks wrote: > The checks on ipv4 addresses in the filtering code check against > a constant of 0xFFFFFFFF, so replace it with MACB_IPV4_MASK and > then make sure it is of __be32 type to avoid the following > sparse warnigns: > > drivers/net/ethernet/cadence/macb_main.c:3448:39: warning: restricted __be32 degrades to integer > drivers/net/ethernet/cadence/macb_main.c:3453:39: warning: restricted __be32 degrades to integer > drivers/net/ethernet/cadence/macb_main.c:3483:20: warning: restricted __be32 degrades to integer > drivers/net/ethernet/cadence/macb_main.c:3497:20: warning: restricted __be32 degrades to integer > > Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> > --- > drivers/net/ethernet/cadence/macb_main.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c > index f20ec0d5260b..538d4c7e023b 100644 > --- a/drivers/net/ethernet/cadence/macb_main.c > +++ b/drivers/net/ethernet/cadence/macb_main.c > @@ -3418,6 +3418,8 @@ static int macb_get_ts_info(struct net_device *netdev, > return ethtool_op_get_ts_info(netdev, info); > } > > +#define MACB_IPV4_MASK htonl(0xFFFFFFFF) > + Hi Ben, according to a recent thread, it seems that the preferred approach might be ~(__le32)0. https://lore.kernel.org/netdev/20230522153615.247577-1-minhuadotchen@gmail.com/
On 2023-06-22 16:49, Simon Horman wrote: > On Thu, Jun 22, 2023 at 02:05:05PM +0100, Ben Dooks wrote: >> The checks on ipv4 addresses in the filtering code check against >> a constant of 0xFFFFFFFF, so replace it with MACB_IPV4_MASK and >> then make sure it is of __be32 type to avoid the following >> sparse warnigns: >> >> drivers/net/ethernet/cadence/macb_main.c:3448:39: warning: restricted >> __be32 degrades to integer >> drivers/net/ethernet/cadence/macb_main.c:3453:39: warning: restricted >> __be32 degrades to integer >> drivers/net/ethernet/cadence/macb_main.c:3483:20: warning: restricted >> __be32 degrades to integer >> drivers/net/ethernet/cadence/macb_main.c:3497:20: warning: restricted >> __be32 degrades to integer >> >> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> >> --- >> drivers/net/ethernet/cadence/macb_main.c | 10 ++++++---- >> 1 file changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/net/ethernet/cadence/macb_main.c >> b/drivers/net/ethernet/cadence/macb_main.c >> index f20ec0d5260b..538d4c7e023b 100644 >> --- a/drivers/net/ethernet/cadence/macb_main.c >> +++ b/drivers/net/ethernet/cadence/macb_main.c >> @@ -3418,6 +3418,8 @@ static int macb_get_ts_info(struct net_device >> *netdev, >> return ethtool_op_get_ts_info(netdev, info); >> } >> >> +#define MACB_IPV4_MASK htonl(0xFFFFFFFF) >> + > > Hi Ben, > > according to a recent thread, it seems that the preferred approach > might be > ~(__le32)0. > > https://lore.kernel.org/netdev/20230522153615.247577-1-minhuadotchen@gmail.com/ Out of interest, should we keep the define then or simply go through changing all the places where change is needed?
On Fri, Jun 23, 2023 at 10:40:52AM +0100, Ben Dooks wrote: > > > On 2023-06-22 16:49, Simon Horman wrote: > > On Thu, Jun 22, 2023 at 02:05:05PM +0100, Ben Dooks wrote: > > > The checks on ipv4 addresses in the filtering code check against > > > a constant of 0xFFFFFFFF, so replace it with MACB_IPV4_MASK and > > > then make sure it is of __be32 type to avoid the following > > > sparse warnigns: > > > > > > drivers/net/ethernet/cadence/macb_main.c:3448:39: warning: > > > restricted __be32 degrades to integer > > > drivers/net/ethernet/cadence/macb_main.c:3453:39: warning: > > > restricted __be32 degrades to integer > > > drivers/net/ethernet/cadence/macb_main.c:3483:20: warning: > > > restricted __be32 degrades to integer > > > drivers/net/ethernet/cadence/macb_main.c:3497:20: warning: > > > restricted __be32 degrades to integer > > > > > > Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> > > > --- > > > drivers/net/ethernet/cadence/macb_main.c | 10 ++++++---- > > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/net/ethernet/cadence/macb_main.c > > > b/drivers/net/ethernet/cadence/macb_main.c > > > index f20ec0d5260b..538d4c7e023b 100644 > > > --- a/drivers/net/ethernet/cadence/macb_main.c > > > +++ b/drivers/net/ethernet/cadence/macb_main.c > > > @@ -3418,6 +3418,8 @@ static int macb_get_ts_info(struct net_device > > > *netdev, > > > return ethtool_op_get_ts_info(netdev, info); > > > } > > > > > > +#define MACB_IPV4_MASK htonl(0xFFFFFFFF) > > > + > > > > Hi Ben, > > > > according to a recent thread, it seems that the preferred approach might > > be > > ~(__le32)0. > > > > https://lore.kernel.org/netdev/20230522153615.247577-1-minhuadotchen@gmail.com/ > > Out of interest, should we keep the define then or simply go through > changing > all the places where change is needed? My personal opinion is that a #define is a nice way to go here.
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index f20ec0d5260b..538d4c7e023b 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -3418,6 +3418,8 @@ static int macb_get_ts_info(struct net_device *netdev, return ethtool_op_get_ts_info(netdev, info); } +#define MACB_IPV4_MASK htonl(0xFFFFFFFF) + static void gem_enable_flow_filters(struct macb *bp, bool enable) { struct net_device *netdev = bp->dev; @@ -3445,12 +3447,12 @@ static void gem_enable_flow_filters(struct macb *bp, bool enable) /* only enable fields with no masking */ tp4sp_m = &(fs->m_u.tcp_ip4_spec); - if (enable && (tp4sp_m->ip4src == 0xFFFFFFFF)) + if (enable && (tp4sp_m->ip4src == MACB_IPV4_MASK)) t2_scr = GEM_BFINS(CMPAEN, 1, t2_scr); else t2_scr = GEM_BFINS(CMPAEN, 0, t2_scr); - if (enable && (tp4sp_m->ip4dst == 0xFFFFFFFF)) + if (enable && (tp4sp_m->ip4dst == MACB_IPV4_MASK)) t2_scr = GEM_BFINS(CMPBEN, 1, t2_scr); else t2_scr = GEM_BFINS(CMPBEN, 0, t2_scr); @@ -3480,7 +3482,7 @@ static void gem_prog_cmp_regs(struct macb *bp, struct ethtool_rx_flow_spec *fs) tp4sp_m = &(fs->m_u.tcp_ip4_spec); /* ignore field if any masking set */ - if (tp4sp_m->ip4src == 0xFFFFFFFF) { + if (tp4sp_m->ip4src == MACB_IPV4_MASK) { /* 1st compare reg - IP source address */ w0 = 0; w1 = 0; @@ -3494,7 +3496,7 @@ static void gem_prog_cmp_regs(struct macb *bp, struct ethtool_rx_flow_spec *fs) } /* ignore field if any masking set */ - if (tp4sp_m->ip4dst == 0xFFFFFFFF) { + if (tp4sp_m->ip4dst == MACB_IPV4_MASK) { /* 2nd compare reg - IP destination address */ w0 = 0; w1 = 0;
The checks on ipv4 addresses in the filtering code check against a constant of 0xFFFFFFFF, so replace it with MACB_IPV4_MASK and then make sure it is of __be32 type to avoid the following sparse warnigns: drivers/net/ethernet/cadence/macb_main.c:3448:39: warning: restricted __be32 degrades to integer drivers/net/ethernet/cadence/macb_main.c:3453:39: warning: restricted __be32 degrades to integer drivers/net/ethernet/cadence/macb_main.c:3483:20: warning: restricted __be32 degrades to integer drivers/net/ethernet/cadence/macb_main.c:3497:20: warning: restricted __be32 degrades to integer Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> --- drivers/net/ethernet/cadence/macb_main.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)