* [PATCH net 1/2] net: dsa: b53: Fix calculating number of switch ports
@ 2021-09-01 9:21 Rafał Miłecki
2021-09-01 9:21 ` [PATCH net 2/2] net: dsa: b53: Set correct number of ports in the DSA struct Rafał Miłecki
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Rafał Miłecki @ 2021-09-01 9:21 UTC (permalink / raw)
To: Andrew Lunn, Vivien Didelot, Vladimir Oltean, David S . Miller,
Jakub Kicinski
Cc: Florian Fainelli, netdev, Rafał Miłecki, stable
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)) {
--
2.26.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH net 2/2] net: dsa: b53: Set correct number of ports in the DSA struct
2021-09-01 9:21 [PATCH net 1/2] net: dsa: b53: Fix calculating number of switch ports Rafał Miłecki
@ 2021-09-01 9:21 ` Rafał Miłecki
2021-09-01 17:21 ` Florian Fainelli
2021-09-01 17:16 ` [PATCH net 1/2] net: dsa: b53: Fix calculating number of switch ports Florian Fainelli
2021-09-02 8:30 ` [PATCH V2 " Rafał Miłecki
2 siblings, 1 reply; 9+ messages in thread
From: Rafał Miłecki @ 2021-09-01 9:21 UTC (permalink / raw)
To: Andrew Lunn, Vivien Didelot, Vladimir Oltean, David S . Miller,
Jakub Kicinski
Cc: Florian Fainelli, netdev, Rafał Miłecki, stable
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)
--
2.26.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH net 2/2] net: dsa: b53: Set correct number of ports in the DSA struct
2021-09-01 9:21 ` [PATCH net 2/2] net: dsa: b53: Set correct number of ports in the DSA struct Rafał Miłecki
@ 2021-09-01 17:21 ` Florian Fainelli
2021-09-01 23:36 ` Jakub Kicinski
0 siblings, 1 reply; 9+ messages in thread
From: Florian Fainelli @ 2021-09-01 17:21 UTC (permalink / raw)
To: Rafał Miłecki, Andrew Lunn, Vivien Didelot,
Vladimir Oltean, David S . Miller, Jakub Kicinski
Cc: netdev, Rafał Miłecki, stable
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.
--
Florian
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH net 2/2] net: dsa: b53: Set correct number of ports in the DSA struct
2021-09-01 17:21 ` Florian Fainelli
@ 2021-09-01 23:36 ` Jakub Kicinski
2021-09-02 1:38 ` Florian Fainelli
0 siblings, 1 reply; 9+ messages in thread
From: Jakub Kicinski @ 2021-09-01 23:36 UTC (permalink / raw)
To: Florian Fainelli
Cc: Rafał Miłecki, Andrew Lunn, Vivien Didelot,
Vladimir Oltean, David S . Miller, netdev,
Rafał Miłecki, stable
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?
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH net 2/2] net: dsa: b53: Set correct number of ports in the DSA struct
2021-09-01 23:36 ` Jakub Kicinski
@ 2021-09-02 1:38 ` Florian Fainelli
0 siblings, 0 replies; 9+ messages in thread
From: Florian Fainelli @ 2021-09-02 1:38 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Rafał Miłecki, Andrew Lunn, Vivien Didelot,
Vladimir Oltean, David S . Miller, netdev,
Rafał Miłecki, stable
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.
--
Florian
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net 1/2] net: dsa: b53: Fix calculating number of switch ports
2021-09-01 9:21 [PATCH net 1/2] net: dsa: b53: Fix calculating number of switch ports Rafał Miłecki
2021-09-01 9:21 ` [PATCH net 2/2] net: dsa: b53: Set correct number of ports in the DSA struct Rafał Miłecki
@ 2021-09-01 17:16 ` Florian Fainelli
2021-09-02 8:30 ` [PATCH V2 " Rafał Miłecki
2 siblings, 0 replies; 9+ messages in thread
From: Florian Fainelli @ 2021-09-01 17:16 UTC (permalink / raw)
To: Rafał Miłecki, Andrew Lunn, Vivien Didelot,
Vladimir Oltean, David S . Miller, Jakub Kicinski
Cc: netdev, Rafał Miłecki, stable
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>
--
Florian
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH V2 net 1/2] net: dsa: b53: Fix calculating number of switch ports
2021-09-01 9:21 [PATCH net 1/2] net: dsa: b53: Fix calculating number of switch ports Rafał Miłecki
2021-09-01 9:21 ` [PATCH net 2/2] net: dsa: b53: Set correct number of ports in the DSA struct Rafał Miłecki
2021-09-01 17:16 ` [PATCH net 1/2] net: dsa: b53: Fix calculating number of switch ports Florian Fainelli
@ 2021-09-02 8:30 ` Rafał Miłecki
2021-09-02 8:30 ` [PATCH V2 net 2/2] net: dsa: b53: Set correct number of ports in the DSA struct Rafał Miłecki
2021-09-02 12:40 ` [PATCH V2 net 1/2] net: dsa: b53: Fix calculating number of switch ports patchwork-bot+netdevbpf
2 siblings, 2 replies; 9+ messages in thread
From: Rafał Miłecki @ 2021-09-02 8:30 UTC (permalink / raw)
To: Andrew Lunn, Vivien Didelot, Vladimir Oltean, David S . Miller,
Jakub Kicinski
Cc: Florian Fainelli, netdev, Rafał Miłecki
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")
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
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)) {
--
2.26.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH V2 net 2/2] net: dsa: b53: Set correct number of ports in the DSA struct
2021-09-02 8:30 ` [PATCH V2 " Rafał Miłecki
@ 2021-09-02 8:30 ` Rafał Miłecki
2021-09-02 12:40 ` [PATCH V2 net 1/2] net: dsa: b53: Fix calculating number of switch ports patchwork-bot+netdevbpf
1 sibling, 0 replies; 9+ messages in thread
From: Rafał Miłecki @ 2021-09-02 8:30 UTC (permalink / raw)
To: Andrew Lunn, Vivien Didelot, Vladimir Oltean, David S . Miller,
Jakub Kicinski
Cc: Florian Fainelli, netdev, Rafał Miłecki
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. It seems the most accurate to reference commit dropping
dsa_switch_alloc() in the Fixes tag.
Fixes: 7e99e3470172 ("net: dsa: remove dsa_switch_alloc helper")
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V2: Update commit & add Fixes tag
---
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)
--
2.26.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH V2 net 1/2] net: dsa: b53: Fix calculating number of switch ports
2021-09-02 8:30 ` [PATCH V2 " Rafał Miłecki
2021-09-02 8:30 ` [PATCH V2 net 2/2] net: dsa: b53: Set correct number of ports in the DSA struct Rafał Miłecki
@ 2021-09-02 12:40 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 9+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-09-02 12:40 UTC (permalink / raw)
To: =?utf-8?b?UmFmYcWCIE1pxYJlY2tpIDx6YWplYzVAZ21haWwuY29tPg==?=
Cc: andrew, vivien.didelot, olteanv, davem, kuba, f.fainelli, netdev, rafal
Hello:
This series was applied to netdev/net.git (refs/heads/master):
On Thu, 2 Sep 2021 10:30:50 +0200 you 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.
>
> [...]
Here is the summary with links:
- [V2,net,1/2] net: dsa: b53: Fix calculating number of switch ports
https://git.kernel.org/netdev/net/c/cdb067d31c0f
- [V2,net,2/2] net: dsa: b53: Set correct number of ports in the DSA struct
https://git.kernel.org/netdev/net/c/d12e1c464988
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-09-02 12:40 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-01 9:21 [PATCH net 1/2] net: dsa: b53: Fix calculating number of switch ports Rafał Miłecki
2021-09-01 9:21 ` [PATCH net 2/2] net: dsa: b53: Set correct number of ports in the DSA struct Rafał Miłecki
2021-09-01 17:21 ` Florian Fainelli
2021-09-01 23:36 ` Jakub Kicinski
2021-09-02 1:38 ` Florian Fainelli
2021-09-01 17:16 ` [PATCH net 1/2] net: dsa: b53: Fix calculating number of switch ports Florian Fainelli
2021-09-02 8:30 ` [PATCH V2 " Rafał Miłecki
2021-09-02 8:30 ` [PATCH V2 net 2/2] net: dsa: b53: Set correct number of ports in the DSA struct Rafał Miłecki
2021-09-02 12:40 ` [PATCH V2 net 1/2] net: dsa: b53: Fix calculating number of switch ports patchwork-bot+netdevbpf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.