* [PATCH net-next 0/2] net: dsa: mv88e6xxx: LAG fixes @ 2021-01-15 10:58 Tobias Waldekranz 2021-01-15 10:58 ` [PATCH net-next 1/2] net: dsa: mv88e6xxx: Provide dummy implementations for trunk setters Tobias Waldekranz 2021-01-15 10:58 ` [PATCH net-next 2/2] net: dsa: mv88e6xxx: Only allow LAG offload on supported hardware Tobias Waldekranz 0 siblings, 2 replies; 13+ messages in thread From: Tobias Waldekranz @ 2021-01-15 10:58 UTC (permalink / raw) To: davem, kuba; +Cc: andrew, vivien.didelot, f.fainelli, olteanv, netdev The kernel test robot kindly pointed out that Global 2 support in mv88e6xxx is optional. This also made me realize that we should verify that the driver and hardware actually supports LAG offloading before trying to configure it. Tobias Waldekranz (2): net: dsa: mv88e6xxx: Provide dummy implementations for trunk setters net: dsa: mv88e6xxx: Only allow LAG offload on supported hardware drivers/net/dsa/mv88e6xxx/chip.c | 4 ++++ drivers/net/dsa/mv88e6xxx/chip.h | 9 +++++++++ drivers/net/dsa/mv88e6xxx/global2.h | 12 ++++++++++++ 3 files changed, 25 insertions(+) -- 2.17.1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next 1/2] net: dsa: mv88e6xxx: Provide dummy implementations for trunk setters 2021-01-15 10:58 [PATCH net-next 0/2] net: dsa: mv88e6xxx: LAG fixes Tobias Waldekranz @ 2021-01-15 10:58 ` Tobias Waldekranz 2021-01-15 11:10 ` Vladimir Oltean 2021-01-15 14:30 ` Andrew Lunn 2021-01-15 10:58 ` [PATCH net-next 2/2] net: dsa: mv88e6xxx: Only allow LAG offload on supported hardware Tobias Waldekranz 1 sibling, 2 replies; 13+ messages in thread From: Tobias Waldekranz @ 2021-01-15 10:58 UTC (permalink / raw) To: davem, kuba; +Cc: andrew, vivien.didelot, f.fainelli, olteanv, netdev Support for Global 2 registers is build-time optional. In the case where it was not enabled the build would fail as no "dummy" implementation of these functions was available. Fixes: 57e661aae6a8 ("net: dsa: mv88e6xxx: Link aggregation support") Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com> --- drivers/net/dsa/mv88e6xxx/global2.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/net/dsa/mv88e6xxx/global2.h b/drivers/net/dsa/mv88e6xxx/global2.h index 60febaf4da76..253a79582a1d 100644 --- a/drivers/net/dsa/mv88e6xxx/global2.h +++ b/drivers/net/dsa/mv88e6xxx/global2.h @@ -525,6 +525,18 @@ static inline int mv88e6xxx_g2_trunk_clear(struct mv88e6xxx_chip *chip) return -EOPNOTSUPP; } +static inline int mv88e6xxx_g2_trunk_mask_write(struct mv88e6xxx_chip *chip, + int num, bool hash, u16 mask) +{ + return -EOPNOTSUPP; +} + +static inline int mv88e6xxx_g2_trunk_mapping_write(struct mv88e6xxx_chip *chip, + int id, u16 map) +{ + return -EOPNOTSUPP; +} + static inline int mv88e6xxx_g2_device_mapping_write(struct mv88e6xxx_chip *chip, int target, int port) { -- 2.17.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 1/2] net: dsa: mv88e6xxx: Provide dummy implementations for trunk setters 2021-01-15 10:58 ` [PATCH net-next 1/2] net: dsa: mv88e6xxx: Provide dummy implementations for trunk setters Tobias Waldekranz @ 2021-01-15 11:10 ` Vladimir Oltean 2021-01-15 14:30 ` Andrew Lunn 1 sibling, 0 replies; 13+ messages in thread From: Vladimir Oltean @ 2021-01-15 11:10 UTC (permalink / raw) To: Tobias Waldekranz; +Cc: davem, kuba, andrew, vivien.didelot, f.fainelli, netdev On Fri, Jan 15, 2021 at 11:58:33AM +0100, Tobias Waldekranz wrote: > Support for Global 2 registers is build-time optional. In the case > where it was not enabled the build would fail as no "dummy" > implementation of these functions was available. > > Fixes: 57e661aae6a8 ("net: dsa: mv88e6xxx: Link aggregation support") > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com> > --- Reviewed-by: Vladimir Oltean <olteanv@gmail.com> Tested-by: Vladimir Oltean <olteanv@gmail.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 1/2] net: dsa: mv88e6xxx: Provide dummy implementations for trunk setters 2021-01-15 10:58 ` [PATCH net-next 1/2] net: dsa: mv88e6xxx: Provide dummy implementations for trunk setters Tobias Waldekranz 2021-01-15 11:10 ` Vladimir Oltean @ 2021-01-15 14:30 ` Andrew Lunn 2021-01-15 14:36 ` Vladimir Oltean 1 sibling, 1 reply; 13+ messages in thread From: Andrew Lunn @ 2021-01-15 14:30 UTC (permalink / raw) To: Tobias Waldekranz Cc: davem, kuba, vivien.didelot, f.fainelli, olteanv, netdev On Fri, Jan 15, 2021 at 11:58:33AM +0100, Tobias Waldekranz wrote: > Support for Global 2 registers is build-time optional. I was never particularly happy about that. Maybe we should revisit what features we loose when global 2 is dropped, and see if it still makes sense to have it as optional? However, until that happens: Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 1/2] net: dsa: mv88e6xxx: Provide dummy implementations for trunk setters 2021-01-15 14:30 ` Andrew Lunn @ 2021-01-15 14:36 ` Vladimir Oltean 2021-01-15 14:48 ` Andrew Lunn 0 siblings, 1 reply; 13+ messages in thread From: Vladimir Oltean @ 2021-01-15 14:36 UTC (permalink / raw) To: Andrew Lunn Cc: Tobias Waldekranz, davem, kuba, vivien.didelot, f.fainelli, netdev On Fri, Jan 15, 2021 at 03:30:30PM +0100, Andrew Lunn wrote: > On Fri, Jan 15, 2021 at 11:58:33AM +0100, Tobias Waldekranz wrote: > > Support for Global 2 registers is build-time optional. > > I was never particularly happy about that. Maybe we should revisit > what features we loose when global 2 is dropped, and see if it still > makes sense to have it as optional? Marvell switch newbie here, what do you mean "global 2 is dropped"? ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 1/2] net: dsa: mv88e6xxx: Provide dummy implementations for trunk setters 2021-01-15 14:36 ` Vladimir Oltean @ 2021-01-15 14:48 ` Andrew Lunn 2021-01-15 14:53 ` Vladimir Oltean 2021-01-15 20:12 ` Tobias Waldekranz 0 siblings, 2 replies; 13+ messages in thread From: Andrew Lunn @ 2021-01-15 14:48 UTC (permalink / raw) To: Vladimir Oltean Cc: Tobias Waldekranz, davem, kuba, vivien.didelot, f.fainelli, netdev On Fri, Jan 15, 2021 at 04:36:49PM +0200, Vladimir Oltean wrote: > On Fri, Jan 15, 2021 at 03:30:30PM +0100, Andrew Lunn wrote: > > On Fri, Jan 15, 2021 at 11:58:33AM +0100, Tobias Waldekranz wrote: > > > Support for Global 2 registers is build-time optional. > > > > I was never particularly happy about that. Maybe we should revisit > > what features we loose when global 2 is dropped, and see if it still > > makes sense to have it as optional? > > Marvell switch newbie here, what do you mean "global 2 is dropped"? I was not aware detect() actually enforced it when needed. It used to be, you could leave it out, and you would just get reduced functionality for devices which had global2, but the code was not compiled in. At the beginning of the life of this driver, i guess it was maybe 25%/75% without/with global2, so it might of made sense to reduce the binary size. But today the driver is much bigger with lots of other things which those early chips don't have, SERDES for example. And that ratio has dramatically reduced, there are very few devices without those registers. This is why i think we can make our lives easier and make global2 always compiled in. Andrew ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 1/2] net: dsa: mv88e6xxx: Provide dummy implementations for trunk setters 2021-01-15 14:48 ` Andrew Lunn @ 2021-01-15 14:53 ` Vladimir Oltean 2021-01-15 20:12 ` Tobias Waldekranz 1 sibling, 0 replies; 13+ messages in thread From: Vladimir Oltean @ 2021-01-15 14:53 UTC (permalink / raw) To: Andrew Lunn Cc: Tobias Waldekranz, davem, kuba, vivien.didelot, f.fainelli, netdev On Fri, Jan 15, 2021 at 03:48:36PM +0100, Andrew Lunn wrote: > On Fri, Jan 15, 2021 at 04:36:49PM +0200, Vladimir Oltean wrote: > > On Fri, Jan 15, 2021 at 03:30:30PM +0100, Andrew Lunn wrote: > > > On Fri, Jan 15, 2021 at 11:58:33AM +0100, Tobias Waldekranz wrote: > > > > Support for Global 2 registers is build-time optional. > > > > > > I was never particularly happy about that. Maybe we should revisit > > > what features we loose when global 2 is dropped, and see if it still > > > makes sense to have it as optional? > > > > Marvell switch newbie here, what do you mean "global 2 is dropped"? > > I was not aware detect() actually enforced it when needed. It used to > be, you could leave it out, and you would just get reduced > functionality for devices which had global2, but the code was not > compiled in. > > At the beginning of the life of this driver, i guess it was maybe > 25%/75% without/with global2, so it might of made sense to reduce the > binary size. But today the driver is much bigger with lots of other > things which those early chips don't have, SERDES for example. And > that ratio has dramatically reduced, there are very few devices > without those registers. This is why i think we can make our lives > easier and make global2 always compiled in. That makes sense, I thought you meant something else by "global 2 support is dropped", nevermind. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 1/2] net: dsa: mv88e6xxx: Provide dummy implementations for trunk setters 2021-01-15 14:48 ` Andrew Lunn 2021-01-15 14:53 ` Vladimir Oltean @ 2021-01-15 20:12 ` Tobias Waldekranz 2021-01-18 17:54 ` Andrew Lunn 1 sibling, 1 reply; 13+ messages in thread From: Tobias Waldekranz @ 2021-01-15 20:12 UTC (permalink / raw) To: Andrew Lunn, Vladimir Oltean Cc: davem, kuba, vivien.didelot, f.fainelli, netdev On Fri, Jan 15, 2021 at 15:48, Andrew Lunn <andrew@lunn.ch> wrote: > On Fri, Jan 15, 2021 at 04:36:49PM +0200, Vladimir Oltean wrote: >> On Fri, Jan 15, 2021 at 03:30:30PM +0100, Andrew Lunn wrote: >> > On Fri, Jan 15, 2021 at 11:58:33AM +0100, Tobias Waldekranz wrote: >> > > Support for Global 2 registers is build-time optional. >> > >> > I was never particularly happy about that. Maybe we should revisit >> > what features we loose when global 2 is dropped, and see if it still >> > makes sense to have it as optional? >> >> Marvell switch newbie here, what do you mean "global 2 is dropped"? > > I was not aware detect() actually enforced it when needed. It used to > be, you could leave it out, and you would just get reduced > functionality for devices which had global2, but the code was not > compiled in. > > At the beginning of the life of this driver, i guess it was maybe > 25%/75% without/with global2, so it might of made sense to reduce the > binary size. But today the driver is much bigger with lots of other > things which those early chips don't have, SERDES for example. And > that ratio has dramatically reduced, there are very few devices > without those registers. This is why i think we can make our lives > easier and make global2 always compiled in. Hear, hear! I took a quick look at the (stripped) object sizes (ppc32): # du -ab 6116 ./global1_vtu.o 5904 ./devlink.o 11500 ./port.o 9640 ./global2.o 3016 ./phy.o 5368 ./global1.o 51784 ./chip.o 9892 ./serdes.o 5140 ./global1_atu.o 1916 ./global2_avb.o 2248 ./global2_scratch.o 948 ./port_hidden.o 1828 ./smi.o 119396 . So, roughly, you save 10%/13k. That hardly justifies the complexity IMO. Andrew, do you want to do this? If not, I can look into it. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 1/2] net: dsa: mv88e6xxx: Provide dummy implementations for trunk setters 2021-01-15 20:12 ` Tobias Waldekranz @ 2021-01-18 17:54 ` Andrew Lunn 0 siblings, 0 replies; 13+ messages in thread From: Andrew Lunn @ 2021-01-18 17:54 UTC (permalink / raw) To: Tobias Waldekranz Cc: Vladimir Oltean, davem, kuba, vivien.didelot, f.fainelli, netdev > Hear, hear! > > I took a quick look at the (stripped) object sizes (ppc32): > > # du -ab > 6116 ./global1_vtu.o > 5904 ./devlink.o > 11500 ./port.o > 9640 ./global2.o > 3016 ./phy.o > 5368 ./global1.o > 51784 ./chip.o > 9892 ./serdes.o > 5140 ./global1_atu.o > 1916 ./global2_avb.o > 2248 ./global2_scratch.o > 948 ./port_hidden.o > 1828 ./smi.o > 119396 . size, part of binutils, is the better tool to use. $ size *.o text data bss dec hex filename 36055 2692 4 38751 975f chip.o 3821 116 4 3941 f65 devlink.o 3229 96 0 3325 cfd global1_atu.o 4388 0 0 4388 1124 global1.o 4449 24 0 4473 1179 global1_vtu.o 1548 0 0 1548 60c global2_avb.o 6350 0 0 6350 18ce global2.o 1412 0 0 1412 584 global2_scratch.o 1757 0 0 1757 6dd phy.o 284 0 0 284 11c port_hidden.o 7516 0 0 7516 1d5c port.o 6570 188 0 6758 1a66 serdes.o 764 0 0 764 2fc smi.o > Andrew, do you want to do this? If not, I can look into it. Yes, i will add it to my TODO list. Andrew ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next 2/2] net: dsa: mv88e6xxx: Only allow LAG offload on supported hardware 2021-01-15 10:58 [PATCH net-next 0/2] net: dsa: mv88e6xxx: LAG fixes Tobias Waldekranz 2021-01-15 10:58 ` [PATCH net-next 1/2] net: dsa: mv88e6xxx: Provide dummy implementations for trunk setters Tobias Waldekranz @ 2021-01-15 10:58 ` Tobias Waldekranz 2021-01-15 11:15 ` Vladimir Oltean 2021-01-15 14:39 ` Andrew Lunn 1 sibling, 2 replies; 13+ messages in thread From: Tobias Waldekranz @ 2021-01-15 10:58 UTC (permalink / raw) To: davem, kuba; +Cc: andrew, vivien.didelot, f.fainelli, olteanv, netdev There are chips that do have Global 2 registers, and therefore trunk mapping/mask tables are not available. Additionally Global 2 register support is build-time optional, so we have to make sure that it is compiled in. Fixes: 57e661aae6a8 ("net: dsa: mv88e6xxx: Link aggregation support") Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com> --- drivers/net/dsa/mv88e6xxx/chip.c | 4 ++++ drivers/net/dsa/mv88e6xxx/chip.h | 9 +++++++++ 2 files changed, 13 insertions(+) diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c index dcb1726b68cc..c48d166c2a70 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -5385,9 +5385,13 @@ static bool mv88e6xxx_lag_can_offload(struct dsa_switch *ds, struct net_device *lag, struct netdev_lag_upper_info *info) { + struct mv88e6xxx_chip *chip = ds->priv; struct dsa_port *dp; int id, members = 0; + if (!mv88e6xxx_has_lag(chip)) + return false; + id = dsa_lag_id(ds->dst, lag); if (id < 0 || id >= ds->num_lag_ids) return false; diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h index 3543055bcb51..333b4fab5aa2 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.h +++ b/drivers/net/dsa/mv88e6xxx/chip.h @@ -662,6 +662,15 @@ static inline bool mv88e6xxx_has_pvt(struct mv88e6xxx_chip *chip) return chip->info->pvt; } +static inline bool mv88e6xxx_has_lag(struct mv88e6xxx_chip *chip) +{ +#if (defined(CONFIG_NET_DSA_MV88E6XXX_GLOBAL2)) + return chip->info->global2_addr != 0; +#else + return false; +#endif +} + static inline unsigned int mv88e6xxx_num_databases(struct mv88e6xxx_chip *chip) { return chip->info->num_databases; -- 2.17.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: Only allow LAG offload on supported hardware 2021-01-15 10:58 ` [PATCH net-next 2/2] net: dsa: mv88e6xxx: Only allow LAG offload on supported hardware Tobias Waldekranz @ 2021-01-15 11:15 ` Vladimir Oltean 2021-01-15 11:29 ` Vladimir Oltean 2021-01-15 14:39 ` Andrew Lunn 1 sibling, 1 reply; 13+ messages in thread From: Vladimir Oltean @ 2021-01-15 11:15 UTC (permalink / raw) To: Tobias Waldekranz; +Cc: davem, kuba, andrew, vivien.didelot, f.fainelli, netdev On Fri, Jan 15, 2021 at 11:58:34AM +0100, Tobias Waldekranz wrote: > There are chips that do have Global 2 registers, and therefore trunk ~~ do not > mapping/mask tables are not available. Additionally Global 2 register > support is build-time optional, so we have to make sure that it is > compiled in. > > Fixes: 57e661aae6a8 ("net: dsa: mv88e6xxx: Link aggregation support") > Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com> > --- > drivers/net/dsa/mv88e6xxx/chip.c | 4 ++++ > drivers/net/dsa/mv88e6xxx/chip.h | 9 +++++++++ > 2 files changed, 13 insertions(+) > > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c > index dcb1726b68cc..c48d166c2a70 100644 > --- a/drivers/net/dsa/mv88e6xxx/chip.c > +++ b/drivers/net/dsa/mv88e6xxx/chip.c > @@ -5385,9 +5385,13 @@ static bool mv88e6xxx_lag_can_offload(struct dsa_switch *ds, > struct net_device *lag, > struct netdev_lag_upper_info *info) > { > + struct mv88e6xxx_chip *chip = ds->priv; > struct dsa_port *dp; > int id, members = 0; > > + if (!mv88e6xxx_has_lag(chip)) > + return false; > + > id = dsa_lag_id(ds->dst, lag); > if (id < 0 || id >= ds->num_lag_ids) > return false; > diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h > index 3543055bcb51..333b4fab5aa2 100644 > --- a/drivers/net/dsa/mv88e6xxx/chip.h > +++ b/drivers/net/dsa/mv88e6xxx/chip.h > @@ -662,6 +662,15 @@ static inline bool mv88e6xxx_has_pvt(struct mv88e6xxx_chip *chip) > return chip->info->pvt; > } > > +static inline bool mv88e6xxx_has_lag(struct mv88e6xxx_chip *chip) > +{ > +#if (defined(CONFIG_NET_DSA_MV88E6XXX_GLOBAL2)) > + return chip->info->global2_addr != 0; > +#else > + return false; > +#endif > +} > + Should we also report ds->num_lag_ids = 0 if !mv88e6xxx_has_lag()? > static inline unsigned int mv88e6xxx_num_databases(struct mv88e6xxx_chip *chip) > { > return chip->info->num_databases; > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: Only allow LAG offload on supported hardware 2021-01-15 11:15 ` Vladimir Oltean @ 2021-01-15 11:29 ` Vladimir Oltean 0 siblings, 0 replies; 13+ messages in thread From: Vladimir Oltean @ 2021-01-15 11:29 UTC (permalink / raw) To: Tobias Waldekranz; +Cc: davem, kuba, andrew, vivien.didelot, f.fainelli, netdev On Fri, Jan 15, 2021 at 01:15:23PM +0200, Vladimir Oltean wrote: > On Fri, Jan 15, 2021 at 11:58:34AM +0100, Tobias Waldekranz wrote: > > There are chips that do have Global 2 registers, and therefore trunk > ~~ > do not > > mapping/mask tables are not available. Additionally Global 2 register > > support is build-time optional, so we have to make sure that it is > > compiled in. > > > > Fixes: 57e661aae6a8 ("net: dsa: mv88e6xxx: Link aggregation support") > > Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com> > > --- > > drivers/net/dsa/mv88e6xxx/chip.c | 4 ++++ > > drivers/net/dsa/mv88e6xxx/chip.h | 9 +++++++++ > > 2 files changed, 13 insertions(+) > > > > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c > > index dcb1726b68cc..c48d166c2a70 100644 > > --- a/drivers/net/dsa/mv88e6xxx/chip.c > > +++ b/drivers/net/dsa/mv88e6xxx/chip.c > > @@ -5385,9 +5385,13 @@ static bool mv88e6xxx_lag_can_offload(struct dsa_switch *ds, > > struct net_device *lag, > > struct netdev_lag_upper_info *info) > > { > > + struct mv88e6xxx_chip *chip = ds->priv; > > struct dsa_port *dp; > > int id, members = 0; > > > > + if (!mv88e6xxx_has_lag(chip)) > > + return false; > > + > > id = dsa_lag_id(ds->dst, lag); > > if (id < 0 || id >= ds->num_lag_ids) > > return false; > > diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h > > index 3543055bcb51..333b4fab5aa2 100644 > > --- a/drivers/net/dsa/mv88e6xxx/chip.h > > +++ b/drivers/net/dsa/mv88e6xxx/chip.h > > @@ -662,6 +662,15 @@ static inline bool mv88e6xxx_has_pvt(struct mv88e6xxx_chip *chip) > > return chip->info->pvt; > > } > > > > +static inline bool mv88e6xxx_has_lag(struct mv88e6xxx_chip *chip) > > +{ > > +#if (defined(CONFIG_NET_DSA_MV88E6XXX_GLOBAL2)) > > + return chip->info->global2_addr != 0; > > +#else > > + return false; > > +#endif > > +} > > + > > Should we also report ds->num_lag_ids = 0 if !mv88e6xxx_has_lag()? > > > static inline unsigned int mv88e6xxx_num_databases(struct mv88e6xxx_chip *chip) > > { > > return chip->info->num_databases; > > -- > > 2.17.1 > > Actually in mv88e6xxx_detect there is this: err = mv88e6xxx_g2_require(chip); if (err) return err; #else /* !CONFIG_NET_DSA_MV88E6XXX_GLOBAL2 */ static inline int mv88e6xxx_g2_require(struct mv88e6xxx_chip *chip) { if (chip->info->global2_addr) { dev_err(chip->dev, "this chip requires CONFIG_NET_DSA_MV88E6XXX_GLOBAL2 enabled\n"); return -EOPNOTSUPP; } return 0; } #endif So CONFIG_NET_DSA_MV88E6XXX_GLOBAL2 is optional only if you use chips that don't support the global2 area. Otherwise it is mandatory. So I would update the commit message to not say "Additionally Global 2 register support is build-time optional", because it doesn't matter. So I would simplify it to: static inline bool mv88e6xxx_has_lag(struct mv88e6xxx_chip *chip) { return !!chip->info->global2_addr; } ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: Only allow LAG offload on supported hardware 2021-01-15 10:58 ` [PATCH net-next 2/2] net: dsa: mv88e6xxx: Only allow LAG offload on supported hardware Tobias Waldekranz 2021-01-15 11:15 ` Vladimir Oltean @ 2021-01-15 14:39 ` Andrew Lunn 1 sibling, 0 replies; 13+ messages in thread From: Andrew Lunn @ 2021-01-15 14:39 UTC (permalink / raw) To: Tobias Waldekranz Cc: davem, kuba, vivien.didelot, f.fainelli, olteanv, netdev > +static inline bool mv88e6xxx_has_lag(struct mv88e6xxx_chip *chip) > +{ > +#if (defined(CONFIG_NET_DSA_MV88E6XXX_GLOBAL2)) > + return chip->info->global2_addr != 0; > +#else > + return false; > +#endif Given Vladimirs comments, this is just FYI: You should not use #if like this. Use if (IS_ENABLED(CONFIG_NET_DSA_MV88E6XXX_GLOBAL2)) return chip->info->global2_addr != 0; return false; The advantage of this is it all gets compiled, so syntax errors in the mostly unused leg get found quickly. The generated code should still be optimal, since at build time it can evaluate the if and completely remove it. Andrew ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-01-18 17:57 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-15 10:58 [PATCH net-next 0/2] net: dsa: mv88e6xxx: LAG fixes Tobias Waldekranz 2021-01-15 10:58 ` [PATCH net-next 1/2] net: dsa: mv88e6xxx: Provide dummy implementations for trunk setters Tobias Waldekranz 2021-01-15 11:10 ` Vladimir Oltean 2021-01-15 14:30 ` Andrew Lunn 2021-01-15 14:36 ` Vladimir Oltean 2021-01-15 14:48 ` Andrew Lunn 2021-01-15 14:53 ` Vladimir Oltean 2021-01-15 20:12 ` Tobias Waldekranz 2021-01-18 17:54 ` Andrew Lunn 2021-01-15 10:58 ` [PATCH net-next 2/2] net: dsa: mv88e6xxx: Only allow LAG offload on supported hardware Tobias Waldekranz 2021-01-15 11:15 ` Vladimir Oltean 2021-01-15 11:29 ` Vladimir Oltean 2021-01-15 14:39 ` Andrew Lunn
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).