From: Rafał Miłecki rafal@milecki.pl
It isn't true that CPU port is always the last one. Switches BCM5301x have 9 ports (port 6 being inactive) and they use port 5 as CPU by default (depending on design some other may be CPU ports too).
A more reliable way of determining number of ports is to check for the last set bit in the "enabled_ports" bitfield.
This fixes b53 internal state, it will allow providing accurate info to the DSA and is required to fix BCM5301x support.
Fixes: 967dd82ffc52 ("net: dsa: b53: Add support for Broadcom RoboSwitch") Cc: stable@vger.kernel.org Signed-off-by: Rafał Miłecki rafal@milecki.pl --- drivers/net/dsa/b53/b53_common.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c index bd1417a66cbf..dcf9d7e5ae14 100644 --- a/drivers/net/dsa/b53/b53_common.c +++ b/drivers/net/dsa/b53/b53_common.c @@ -2612,9 +2612,8 @@ static int b53_switch_init(struct b53_device *dev) dev->cpu_port = 5; }
- /* cpu port is always last */ - dev->num_ports = dev->cpu_port + 1; dev->enabled_ports |= BIT(dev->cpu_port); + dev->num_ports = fls(dev->enabled_ports);
/* Include non standard CPU port built-in PHYs to be probed */ if (is539x(dev) || is531x5(dev)) {
From: Rafał Miłecki rafal@milecki.pl
Setting DSA_MAX_PORTS caused DSA to call b53 callbacks (e.g. b53_disable_port() during dsa_register_switch()) for invalid (non-existent) ports. That made b53 modify unrelated registers and is one of reasons for a broken BCM5301x support.
This problem exists for years but DSA_MAX_PORTS usage has changed few times so it's hard to specify a single commit this change fixes.
Cc: stable@vger.kernel.org Signed-off-by: Rafał Miłecki rafal@milecki.pl --- drivers/net/dsa/b53/b53_common.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c index dcf9d7e5ae14..5646eb8afe38 100644 --- a/drivers/net/dsa/b53/b53_common.c +++ b/drivers/net/dsa/b53/b53_common.c @@ -2615,6 +2615,8 @@ static int b53_switch_init(struct b53_device *dev) dev->enabled_ports |= BIT(dev->cpu_port); dev->num_ports = fls(dev->enabled_ports);
+ dev->ds->num_ports = min_t(unsigned int, dev->num_ports, DSA_MAX_PORTS); + /* Include non standard CPU port built-in PHYs to be probed */ if (is539x(dev) || is531x5(dev)) { for (i = 0; i < dev->num_ports; i++) { @@ -2659,7 +2661,6 @@ struct b53_device *b53_switch_alloc(struct device *base, return NULL;
ds->dev = base; - ds->num_ports = DSA_MAX_PORTS;
dev = devm_kzalloc(base, sizeof(*dev), GFP_KERNEL); if (!dev)
On 9/1/2021 2:21 AM, Rafał Miłecki wrote:
From: Rafał Miłecki rafal@milecki.pl
Setting DSA_MAX_PORTS caused DSA to call b53 callbacks (e.g. b53_disable_port() during dsa_register_switch()) for invalid (non-existent) ports. That made b53 modify unrelated registers and is one of reasons for a broken BCM5301x support.
This problem exists for years but DSA_MAX_PORTS usage has changed few times so it's hard to specify a single commit this change fixes.
You should still try to identify the relevant tags that this is fixing such that this gets back ported to the appropriate trees. We could use Fixes: 7e99e3470172 ("net: dsa: remove dsa_switch_alloc helper"), to minimize the amount of work doing the back port.
On Wed, 1 Sep 2021 10:21:55 -0700 Florian Fainelli wrote:
On 9/1/2021 2:21 AM, Rafał Miłecki wrote:
From: Rafał Miłecki rafal@milecki.pl
Setting DSA_MAX_PORTS caused DSA to call b53 callbacks (e.g. b53_disable_port() during dsa_register_switch()) for invalid (non-existent) ports. That made b53 modify unrelated registers and is one of reasons for a broken BCM5301x support.
This problem exists for years but DSA_MAX_PORTS usage has changed few times so it's hard to specify a single commit this change fixes.
You should still try to identify the relevant tags that this is fixing such that this gets back ported to the appropriate trees. We could use Fixes: 7e99e3470172 ("net: dsa: remove dsa_switch_alloc helper"), to minimize the amount of work doing the back port.
To be clear are you okay with the fixes tag you provided or should we wait for Rafał to double check?
On 9/1/2021 4:36 PM, Jakub Kicinski wrote:
On Wed, 1 Sep 2021 10:21:55 -0700 Florian Fainelli wrote:
On 9/1/2021 2:21 AM, Rafał Miłecki wrote:
From: Rafał Miłecki rafal@milecki.pl
Setting DSA_MAX_PORTS caused DSA to call b53 callbacks (e.g. b53_disable_port() during dsa_register_switch()) for invalid (non-existent) ports. That made b53 modify unrelated registers and is one of reasons for a broken BCM5301x support.
This problem exists for years but DSA_MAX_PORTS usage has changed few times so it's hard to specify a single commit this change fixes.
You should still try to identify the relevant tags that this is fixing such that this gets back ported to the appropriate trees. We could use Fixes: 7e99e3470172 ("net: dsa: remove dsa_switch_alloc helper"), to minimize the amount of work doing the back port.
To be clear are you okay with the fixes tag you provided or should we wait for Rafał to double check?
That Fixes tag is correct and won't cause conflicts AFAICT with backports all the way down to that commit.
On 9/1/2021 2:21 AM, Rafał Miłecki wrote:
From: Rafał Miłecki rafal@milecki.pl
It isn't true that CPU port is always the last one. Switches BCM5301x have 9 ports (port 6 being inactive) and they use port 5 as CPU by default (depending on design some other may be CPU ports too).
A more reliable way of determining number of ports is to check for the last set bit in the "enabled_ports" bitfield.
This fixes b53 internal state, it will allow providing accurate info to the DSA and is required to fix BCM5301x support.
Fixes: 967dd82ffc52 ("net: dsa: b53: Add support for Broadcom RoboSwitch") Cc: stable@vger.kernel.org Signed-off-by: Rafał Miłecki rafal@milecki.pl
For a bug fix, this looks appropriate to me, and for net-next, we need to remove the dev->num_ports and b53_for_each_port() entirely as there is no need to duplicate what DSA already maintains for us. Thanks!
Reviewed-by: Florian Fainelli f.fainelli@gmail.com
linux-stable-mirror@lists.linaro.org
-
Florian Fainelli -
Jakub Kicinski -
Rafał Miłecki