Older BCM5325M switches lack some registers that newer BCM5325E have, so
we need to be able to differentiate them in order to check whether the
registers are available or not.
Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
drivers/net/dsa/b53/b53_common.c | 34 ++++++++++++++++++++++++++------
drivers/net/dsa/b53/b53_priv.h | 16 +++++++++++++--
2 files changed, 42 insertions(+), 8 deletions(-)
v3: detect BCM5325 variants as requested by Florian.
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 222107223d109..2975dab6ee0bb 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -2490,8 +2490,18 @@ struct b53_chip_data {
static const struct b53_chip_data b53_switch_chips[] = {
{
- .chip_id = BCM5325_DEVICE_ID,
- .dev_name = "BCM5325",
+ .chip_id = BCM5325M_DEVICE_ID,
+ .dev_name = "BCM5325M",
+ .vlans = 16,
+ .enabled_ports = 0x3f,
+ .arl_bins = 2,
+ .arl_buckets = 1024,
+ .imp_port = 5,
+ .duplex_reg = B53_DUPLEX_STAT_FE,
+ },
+ {
+ .chip_id = BCM5325E_DEVICE_ID,
+ .dev_name = "BCM5325E",
.vlans = 16,
.enabled_ports = 0x3f,
.arl_bins = 2,
@@ -2938,10 +2948,22 @@ int b53_switch_detect(struct b53_device *dev)
b53_write16(dev, B53_VLAN_PAGE, B53_VLAN_TABLE_ACCESS_25, 0xf);
b53_read16(dev, B53_VLAN_PAGE, B53_VLAN_TABLE_ACCESS_25, &tmp);
- if (tmp == 0xf)
- dev->chip_id = BCM5325_DEVICE_ID;
- else
+ if (tmp == 0xf) {
+ u32 phy_id;
+ int val;
+
+ val = b53_phy_read16(dev->ds, 0, MII_PHYSID1);
+ phy_id = (val & 0xffff) << 16;
+ val = b53_phy_read16(dev->ds, 0, MII_PHYSID2);
+ phy_id |= (val & 0xffff);
+
+ if (phy_id == 0x0143bc30)
+ dev->chip_id = BCM5325E_DEVICE_ID;
+ else
+ dev->chip_id = BCM5325M_DEVICE_ID;
+ } else {
dev->chip_id = BCM5365_DEVICE_ID;
+ }
break;
case BCM5389_DEVICE_ID:
case BCM5395_DEVICE_ID:
@@ -2975,7 +2997,7 @@ int b53_switch_detect(struct b53_device *dev)
}
}
- if (dev->chip_id == BCM5325_DEVICE_ID)
+ if (is5325(dev))
return b53_read8(dev, B53_STAT_PAGE, B53_REV_ID_25,
&dev->core_rev);
else
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index a5ef7071ba07b..deea4d83f0e93 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -60,7 +60,8 @@ struct b53_io_ops {
enum {
BCM4908_DEVICE_ID = 0x4908,
- BCM5325_DEVICE_ID = 0x25,
+ BCM5325M_DEVICE_ID = 0x25,
+ BCM5325E_DEVICE_ID = 0x25e,
BCM5365_DEVICE_ID = 0x65,
BCM5389_DEVICE_ID = 0x89,
BCM5395_DEVICE_ID = 0x95,
@@ -162,7 +163,18 @@ struct b53_device {
static inline int is5325(struct b53_device *dev)
{
- return dev->chip_id == BCM5325_DEVICE_ID;
+ return dev->chip_id == BCM5325E_DEVICE_ID ||
+ dev->chip_id == BCM5325M_DEVICE_ID;
+}
+
+static inline int is5325e(struct b53_device *dev)
+{
+ return dev->chip_id == BCM5325E_DEVICE_ID;
+}
+
+static inline int is5325m(struct b53_device *dev)
+{
+ return dev->chip_id == BCM5325M_DEVICE_ID;
}
static inline int is5365(struct b53_device *dev)
--
2.39.5
On Thu, Jun 12, 2025 at 10:37 AM Álvaro Fernández Rojas
<noltari@gmail.com> wrote:
>
> Older BCM5325M switches lack some registers that newer BCM5325E have, so
> we need to be able to differentiate them in order to check whether the
> registers are available or not.
Did you test this with a BCM5325M?
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> ---
> drivers/net/dsa/b53/b53_common.c | 34 ++++++++++++++++++++++++++------
> drivers/net/dsa/b53/b53_priv.h | 16 +++++++++++++--
> 2 files changed, 42 insertions(+), 8 deletions(-)
>
> v3: detect BCM5325 variants as requested by Florian.
>
> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> index 222107223d109..2975dab6ee0bb 100644
> --- a/drivers/net/dsa/b53/b53_common.c
> +++ b/drivers/net/dsa/b53/b53_common.c
> @@ -2490,8 +2490,18 @@ struct b53_chip_data {
>
> static const struct b53_chip_data b53_switch_chips[] = {
> {
> - .chip_id = BCM5325_DEVICE_ID,
> - .dev_name = "BCM5325",
> + .chip_id = BCM5325M_DEVICE_ID,
> + .dev_name = "BCM5325M",
> + .vlans = 16,
Are you sure about BCM5325M supporting VLANs at all? All the
documentation I can find implies it does not. And if it does not, not
sure if it makes sense to support it.
> + .enabled_ports = 0x3f,
> + .arl_bins = 2,
> + .arl_buckets = 1024,
> + .imp_port = 5,
> + .duplex_reg = B53_DUPLEX_STAT_FE,
> + },
> + {
> + .chip_id = BCM5325E_DEVICE_ID,
> + .dev_name = "BCM5325E",
> .vlans = 16,
> .enabled_ports = 0x3f,
> .arl_bins = 2,
> @@ -2938,10 +2948,22 @@ int b53_switch_detect(struct b53_device *dev)
> b53_write16(dev, B53_VLAN_PAGE, B53_VLAN_TABLE_ACCESS_25, 0xf);
> b53_read16(dev, B53_VLAN_PAGE, B53_VLAN_TABLE_ACCESS_25, &tmp);
>
> - if (tmp == 0xf)
> - dev->chip_id = BCM5325_DEVICE_ID;
> - else
> + if (tmp == 0xf) {
> + u32 phy_id;
> + int val;
> +
> + val = b53_phy_read16(dev->ds, 0, MII_PHYSID1);
> + phy_id = (val & 0xffff) << 16;
> + val = b53_phy_read16(dev->ds, 0, MII_PHYSID2);
> + phy_id |= (val & 0xffff);
You should ignore the least significant nibble, as it encodes the chip revision.
> +
> + if (phy_id == 0x0143bc30)
> + dev->chip_id = BCM5325E_DEVICE_ID;
> + else
> + dev->chip_id = BCM5325M_DEVICE_ID;
> + } else {
> dev->chip_id = BCM5365_DEVICE_ID;
> + }
> break;
> case BCM5389_DEVICE_ID:
> case BCM5395_DEVICE_ID:
> @@ -2975,7 +2997,7 @@ int b53_switch_detect(struct b53_device *dev)
> }
> }
>
> - if (dev->chip_id == BCM5325_DEVICE_ID)
> + if (is5325(dev))
> return b53_read8(dev, B53_STAT_PAGE, B53_REV_ID_25,
> &dev->core_rev);
> else
> diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
> index a5ef7071ba07b..deea4d83f0e93 100644
> --- a/drivers/net/dsa/b53/b53_priv.h
> +++ b/drivers/net/dsa/b53/b53_priv.h
> @@ -60,7 +60,8 @@ struct b53_io_ops {
>
> enum {
> BCM4908_DEVICE_ID = 0x4908,
> - BCM5325_DEVICE_ID = 0x25,
> + BCM5325M_DEVICE_ID = 0x25,
> + BCM5325E_DEVICE_ID = 0x25e,
Maybe we should have a b53_priv::variant_id field or so. Other chips
also can have variants, so we might want to avoid polluting the chip
id space. We currently don't care about them, but might in the future
as they have different feature support (e.g. there are bcm531x5
variants with and without CFP support).
Regards,
Jonas
Hi Jonas,
El jue, 12 jun 2025 a las 11:17, Jonas Gorski
(<jonas.gorski@gmail.com>) escribió:
>
> On Thu, Jun 12, 2025 at 10:37 AM Álvaro Fernández Rojas
> <noltari@gmail.com> wrote:
> >
> > Older BCM5325M switches lack some registers that newer BCM5325E have, so
> > we need to be able to differentiate them in order to check whether the
> > registers are available or not.
>
> Did you test this with a BCM5325M?
Nope, I don't have any device with a BCM5325M.
>
> > Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> > ---
> > drivers/net/dsa/b53/b53_common.c | 34 ++++++++++++++++++++++++++------
> > drivers/net/dsa/b53/b53_priv.h | 16 +++++++++++++--
> > 2 files changed, 42 insertions(+), 8 deletions(-)
> >
> > v3: detect BCM5325 variants as requested by Florian.
> >
> > diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> > index 222107223d109..2975dab6ee0bb 100644
> > --- a/drivers/net/dsa/b53/b53_common.c
> > +++ b/drivers/net/dsa/b53/b53_common.c
> > @@ -2490,8 +2490,18 @@ struct b53_chip_data {
> >
> > static const struct b53_chip_data b53_switch_chips[] = {
> > {
> > - .chip_id = BCM5325_DEVICE_ID,
> > - .dev_name = "BCM5325",
> > + .chip_id = BCM5325M_DEVICE_ID,
> > + .dev_name = "BCM5325M",
> > + .vlans = 16,
>
> Are you sure about BCM5325M supporting VLANs at all? All the
> documentation I can find implies it does not. And if it does not, not
> sure if it makes sense to support it.
Since Florian suggested that we should be able to differentiate them I
assumed we were supporting it, but if it doesn't make sense to support
it at all we can drop this patch entirely and not check for 5325m in
B53_VLAN_ID_IDX access of the next patch (5/14).
>
> > + .enabled_ports = 0x3f,
> > + .arl_bins = 2,
> > + .arl_buckets = 1024,
> > + .imp_port = 5,
> > + .duplex_reg = B53_DUPLEX_STAT_FE,
> > + },
> > + {
> > + .chip_id = BCM5325E_DEVICE_ID,
> > + .dev_name = "BCM5325E",
> > .vlans = 16,
> > .enabled_ports = 0x3f,
> > .arl_bins = 2,
> > @@ -2938,10 +2948,22 @@ int b53_switch_detect(struct b53_device *dev)
> > b53_write16(dev, B53_VLAN_PAGE, B53_VLAN_TABLE_ACCESS_25, 0xf);
> > b53_read16(dev, B53_VLAN_PAGE, B53_VLAN_TABLE_ACCESS_25, &tmp);
> >
> > - if (tmp == 0xf)
> > - dev->chip_id = BCM5325_DEVICE_ID;
> > - else
> > + if (tmp == 0xf) {
> > + u32 phy_id;
> > + int val;
> > +
> > + val = b53_phy_read16(dev->ds, 0, MII_PHYSID1);
> > + phy_id = (val & 0xffff) << 16;
> > + val = b53_phy_read16(dev->ds, 0, MII_PHYSID2);
> > + phy_id |= (val & 0xffff);
>
> You should ignore the least significant nibble, as it encodes the chip revision.
So the correct would be:
val = b53_phy_read16(dev->ds, 0, MII_PHYSID2);
phy_id |= (val & 0xfff0);
Right?
>
> > +
> > + if (phy_id == 0x0143bc30)
> > + dev->chip_id = BCM5325E_DEVICE_ID;
> > + else
> > + dev->chip_id = BCM5325M_DEVICE_ID;
> > + } else {
> > dev->chip_id = BCM5365_DEVICE_ID;
> > + }
> > break;
> > case BCM5389_DEVICE_ID:
> > case BCM5395_DEVICE_ID:
> > @@ -2975,7 +2997,7 @@ int b53_switch_detect(struct b53_device *dev)
> > }
> > }
> >
> > - if (dev->chip_id == BCM5325_DEVICE_ID)
> > + if (is5325(dev))
> > return b53_read8(dev, B53_STAT_PAGE, B53_REV_ID_25,
> > &dev->core_rev);
> > else
> > diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
> > index a5ef7071ba07b..deea4d83f0e93 100644
> > --- a/drivers/net/dsa/b53/b53_priv.h
> > +++ b/drivers/net/dsa/b53/b53_priv.h
> > @@ -60,7 +60,8 @@ struct b53_io_ops {
> >
> > enum {
> > BCM4908_DEVICE_ID = 0x4908,
> > - BCM5325_DEVICE_ID = 0x25,
> > + BCM5325M_DEVICE_ID = 0x25,
> > + BCM5325E_DEVICE_ID = 0x25e,
>
> Maybe we should have a b53_priv::variant_id field or so. Other chips
> also can have variants, so we might want to avoid polluting the chip
> id space. We currently don't care about them, but might in the future
> as they have different feature support (e.g. there are bcm531x5
> variants with and without CFP support).
That makes sense.
I can rework this patch with a new variant field if differentiating
the BCM5325M is finally needed.
>
> Regards,
> Jonas
Best regards,
Álvaro.
© 2016 - 2025 Red Hat, Inc.