[v1] net: dsa: b53: fix ageing time for BCM53101

[PATCH net] net: dsa: b53: fix ageing time for BCM53101

Posted by Jonas Gorski 1 month, 1 week ago

For some reason Broadcom decided that BCM53101 uses 0.5s increments for
the ageing time register, but kept the field width the same [1]. Due to
this, the actual ageing time was always half of what was configured.

Fix this by adapting the limits and value calculation for BCM53101.

[1] https://github.com/Broadcom-Network-Switching-Software/OpenMDK/blob/master/cdk/PKG/chip/bcm53101/bcm53101_a0_defs.h#L28966

Fixes: e39d14a760c0 ("net: dsa: b53: implement setting ageing time")
Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
Lacking matching hardware, this is only run-tested on non-matching
(BCM53115).

 drivers/net/dsa/b53/b53_common.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 829b1f087e9e..b85ca17e8fdd 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1273,9 +1273,16 @@ static int b53_setup(struct dsa_switch *ds)
 	 */
 	ds->untag_vlan_aware_bridge_pvid = true;
 
-	/* Ageing time is set in seconds */
-	ds->ageing_time_min = 1 * 1000;
-	ds->ageing_time_max = AGE_TIME_MAX * 1000;
+
+	if (dev->chip_id == BCM53101_DEVICE_ID) {
+		/* BCM53101 uses 0.5 second increments */
+		ds->ageing_time_min = 1 * 500;
+		ds->ageing_time_max = AGE_TIME_MAX * 500;
+	} else {
+		/* Everything else uses 1 second increments */
+		ds->ageing_time_min = 1 * 1000;
+		ds->ageing_time_max = AGE_TIME_MAX * 1000;
+	}
 
 	ret = b53_reset_switch(dev);
 	if (ret) {
@@ -2559,7 +2566,10 @@ int b53_set_ageing_time(struct dsa_switch *ds, unsigned int msecs)
 	else
 		reg = B53_AGING_TIME_CONTROL;
 
-	atc = DIV_ROUND_CLOSEST(msecs, 1000);
+	if (dev->chip_id == BCM53101_DEVICE_ID)
+		atc = DIV_ROUND_CLOSEST(msecs, 500);
+	else
+		atc = DIV_ROUND_CLOSEST(msecs, 1000);
 
 	if (!is5325(dev) && !is5365(dev))
 		atc |= AGE_CHANGE;

base-commit: ec79003c5f9d2c7f9576fc69b8dbda80305cbe3a
-- 
2.43.0

Re: [PATCH net] net: dsa: b53: fix ageing time for BCM53101

Posted by Andrew Lunn 1 month, 1 week ago

On Sat, Aug 23, 2025 at 11:06:16AM +0200, Jonas Gorski wrote:
> For some reason Broadcom decided that BCM53101 uses 0.5s increments for
> the ageing time register, but kept the field width the same [1]. Due to
> this, the actual ageing time was always half of what was configured.
> 
> Fix this by adapting the limits and value calculation for BCM53101.
> 
> [1] https://github.com/Broadcom-Network-Switching-Software/OpenMDK/blob/master/cdk/PKG/chip/bcm53101/bcm53101_a0_defs.h#L28966

Is line 28966 correct? In order to find a reference to age, i needed
to search further in the file.

Are these devices organised in families/generations. Are you sure this
does not apply to:

	BCM53101_DEVICE_ID = 0x53101,
	BCM53115_DEVICE_ID = 0x53115,
	BCM53125_DEVICE_ID = 0x53125,
	BCM53128_DEVICE_ID = 0x53128,

	Andrew

Re: [PATCH net] net: dsa: b53: fix ageing time for BCM53101

Posted by Jonas Gorski 1 month, 1 week ago

Hi,

On Sat, Aug 23, 2025 at 5:00 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Sat, Aug 23, 2025 at 11:06:16AM +0200, Jonas Gorski wrote:
> > For some reason Broadcom decided that BCM53101 uses 0.5s increments for
> > the ageing time register, but kept the field width the same [1]. Due to
> > this, the actual ageing time was always half of what was configured.
> >
> > Fix this by adapting the limits and value calculation for BCM53101.
> >
> > [1] https://github.com/Broadcom-Network-Switching-Software/OpenMDK/blob/master/cdk/PKG/chip/bcm53101/bcm53101_a0_defs.h#L28966
>
> Is line 28966 correct? In order to find a reference to age, i needed
> to search further in the file.

Hm, indeed, it's #30768. Not sure where that original line came from,
maybe I miss-clicked before copying the link in the address bar.

> Are these devices organised in families/generations. Are you sure this
> does not apply to:
>
>         BCM53101_DEVICE_ID = 0x53101,

This is the chip for which I am fixing/changing it :)

>         BCM53115_DEVICE_ID = 0x53115,
>         BCM53125_DEVICE_ID = 0x53125,
>         BCM53128_DEVICE_ID = 0x53128,

Yes, pretty sure:

$ grep -l -r "Specifies the aging time in 0.5 seconds" cdk/PKG/chip | sort
cdk/PKG/chip/bcm53101/bcm53101_a0_defs.h

$ grep -l -r "Specifies the aging time in seconds" cdk/PKG/chip | sort
cdk/PKG/chip/bcm53010/bcm53010_a0_defs.h
cdk/PKG/chip/bcm53020/bcm53020_a0_defs.h
cdk/PKG/chip/bcm53084/bcm53084_a0_defs.h
cdk/PKG/chip/bcm53115/bcm53115_a0_defs.h
cdk/PKG/chip/bcm53118/bcm53118_a0_defs.h
cdk/PKG/chip/bcm53125/bcm53125_a0_defs.h
cdk/PKG/chip/bcm53128/bcm53128_a0_defs.h
cdk/PKG/chip/bcm53134/bcm53134_a0_defs.h
cdk/PKG/chip/bcm53242/bcm53242_a0_defs.h
cdk/PKG/chip/bcm53262/bcm53262_a0_defs.h
cdk/PKG/chip/bcm53280/bcm53280_a0_defs.h
cdk/PKG/chip/bcm53280/bcm53280_b0_defs.h
cdk/PKG/chip/bcm53600/bcm53600_a0_defs.h
cdk/PKG/chip/bcm89500/bcm89500_a0_defs.h

This is also what the datasheets say (they sometimes disagree, but not
in this case).

There are a few chips supported by b53 for which I lack
datasheets/descriptions, so they may also have that issue. Mostly the
BCM7* and BCM58* families. Maybe Florian knows more there.
BCM538*/BCM539* are likely not affected, they are older than BCM53101.

Best regards,
Jonas

Re: [PATCH net] net: dsa: b53: fix ageing time for BCM53101

Posted by Andrew Lunn 1 month, 1 week ago

On Sat, Aug 23, 2025 at 05:27:02PM +0200, Jonas Gorski wrote:
> Hi,
> 
> On Sat, Aug 23, 2025 at 5:00 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > On Sat, Aug 23, 2025 at 11:06:16AM +0200, Jonas Gorski wrote:
> > > For some reason Broadcom decided that BCM53101 uses 0.5s increments for
> > > the ageing time register, but kept the field width the same [1]. Due to
> > > this, the actual ageing time was always half of what was configured.
> > >
> > > Fix this by adapting the limits and value calculation for BCM53101.
> > >
> > > [1] https://github.com/Broadcom-Network-Switching-Software/OpenMDK/blob/master/cdk/PKG/chip/bcm53101/bcm53101_a0_defs.h#L28966
> >
> > Is line 28966 correct? In order to find a reference to age, i needed
> > to search further in the file.
> 
> Hm, indeed, it's #30768. Not sure where that original line came from,
> maybe I miss-clicked before copying the link in the address bar.

Or a new version has been dumped there, changing all the line numbers?
I've not looked, is there a tag you can use instead of master?

> > Are these devices organised in families/generations. Are you sure this
> > does not apply to:
> >
> >         BCM53101_DEVICE_ID = 0x53101,
> 
> This is the chip for which I am fixing/changing it :)
> 
> >         BCM53115_DEVICE_ID = 0x53115,
> >         BCM53125_DEVICE_ID = 0x53125,
> >         BCM53128_DEVICE_ID = 0x53128,
> 
> Yes, pretty sure:
> 
> $ grep -l -r "Specifies the aging time in 0.5 seconds" cdk/PKG/chip | sort
> cdk/PKG/chip/bcm53101/bcm53101_a0_defs.h
> 
> $ grep -l -r "Specifies the aging time in seconds" cdk/PKG/chip | sort
> cdk/PKG/chip/bcm53010/bcm53010_a0_defs.h
> cdk/PKG/chip/bcm53020/bcm53020_a0_defs.h
> cdk/PKG/chip/bcm53084/bcm53084_a0_defs.h
> cdk/PKG/chip/bcm53115/bcm53115_a0_defs.h
> cdk/PKG/chip/bcm53118/bcm53118_a0_defs.h
> cdk/PKG/chip/bcm53125/bcm53125_a0_defs.h
> cdk/PKG/chip/bcm53128/bcm53128_a0_defs.h
> cdk/PKG/chip/bcm53134/bcm53134_a0_defs.h
> cdk/PKG/chip/bcm53242/bcm53242_a0_defs.h
> cdk/PKG/chip/bcm53262/bcm53262_a0_defs.h
> cdk/PKG/chip/bcm53280/bcm53280_a0_defs.h
> cdk/PKG/chip/bcm53280/bcm53280_b0_defs.h
> cdk/PKG/chip/bcm53600/bcm53600_a0_defs.h
> cdk/PKG/chip/bcm89500/bcm89500_a0_defs.h

Thanks. That is pretty convincing. Lets see if Florian has anything to
add.

	Andrew

Re: [PATCH net] net: dsa: b53: fix ageing time for BCM53101

Posted by Jonas Gorski 1 month, 1 week ago

On Sat, Aug 23, 2025 at 5:31 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Sat, Aug 23, 2025 at 05:27:02PM +0200, Jonas Gorski wrote:
> > Hi,
> >
> > On Sat, Aug 23, 2025 at 5:00 PM Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > On Sat, Aug 23, 2025 at 11:06:16AM +0200, Jonas Gorski wrote:
> > > > For some reason Broadcom decided that BCM53101 uses 0.5s increments for
> > > > the ageing time register, but kept the field width the same [1]. Due to
> > > > this, the actual ageing time was always half of what was configured.
> > > >
> > > > Fix this by adapting the limits and value calculation for BCM53101.
> > > >
> > > > [1] https://github.com/Broadcom-Network-Switching-Software/OpenMDK/blob/master/cdk/PKG/chip/bcm53101/bcm53101_a0_defs.h#L28966
> > >
> > > Is line 28966 correct? In order to find a reference to age, i needed
> > > to search further in the file.
> >
> > Hm, indeed, it's #30768. Not sure where that original line came from,
> > maybe I miss-clicked before copying the link in the address bar.
>
> Or a new version has been dumped there, changing all the line numbers?
> I've not looked, is there a tag you can use instead of master?

Uh, indeed the repository was updated. I didn't expect that, since its
was unchanged since its creation in 2020 with a single commit, so I
assumed Broadcom abandoned it like they did with a lot of other
repositories, and treated it as a static code dump.

Though they force pushed a new master. Well, "new". master is now
v2.10.9, and was previously v2.11.0. Don't ask me. But at least they
also added tags for the two versions.

So https://github.com/Broadcom/OpenMDK/blob/v2.11.0/cdk/PKG/chip/bcm53101/bcm53101_a0_defs.h#L28966
is what the link should now be (they also moved the repository).

That's not the first time I saw Broadcom force pushing to a public
(SDK) repository, so that link might also break eventually anyway.

Best regards,
Jonas

Re: [PATCH net] net: dsa: b53: fix ageing time for BCM53101

Posted by Jakub Kicinski 1 month, 1 week ago

On Sat, 23 Aug 2025 18:00:25 +0200 Jonas Gorski wrote:
> So https://github.com/Broadcom/OpenMDK/blob/v2.11.0/cdk/PKG/chip/bcm53101/bcm53101_a0_defs.h#L28966
> is what the link should now be (they also moved the repository).

Maybe using a hash would be even safer than the tag?