[PATCH net-next v3 14/14] net: dsa: b53: ensure BCM5325 PHYs are enabled

Álvaro Fernández Rojas posted 14 patches 4 months ago
There is a newer version of this series
[PATCH net-next v3 14/14] net: dsa: b53: ensure BCM5325 PHYs are enabled
Posted by Álvaro Fernández Rojas 4 months ago
According to the datasheet, BCM5325 uses B53_PD_MODE_CTRL_25 register to
disable clocking to individual PHYs.
Only ports 1-4 can be enabled or disabled and the datasheet is explicit
about not toggling BIT(0) since it disables the PLL power and the switch.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 12 ++++++++++++
 drivers/net/dsa/b53/b53_regs.h   |  2 ++
 2 files changed, 14 insertions(+)

 v3: add changes requested by Florian:
  - Use in_range() helper.

 v2: add changes requested by Florian:
  - Move B53_PD_MODE_CTRL_25 to b53_setup_port().

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 3503f363e2419..eac40e95c8c53 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -660,6 +660,18 @@ int b53_setup_port(struct dsa_switch *ds, int port)
 	if (dsa_is_user_port(ds, port))
 		b53_set_eap_mode(dev, port, EAP_MODE_SIMPLIFIED);
 
+	if (is5325(dev) &&
+	    in_range(port, B53_PD_MODE_PORT_MIN, B53_PD_MODE_PORT_MAX)) {
+		u8 reg;
+
+		b53_read8(dev, B53_CTRL_PAGE, B53_PD_MODE_CTRL_25, &reg);
+		if (dsa_is_unused_port(ds, port))
+			reg |= BIT(port);
+		else
+			reg &= ~BIT(port);
+		b53_write8(dev, B53_CTRL_PAGE, B53_PD_MODE_CTRL_25, reg);
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL(b53_setup_port);
diff --git a/drivers/net/dsa/b53/b53_regs.h b/drivers/net/dsa/b53/b53_regs.h
index d6849cf6b0a3a..880c67130a9fc 100644
--- a/drivers/net/dsa/b53/b53_regs.h
+++ b/drivers/net/dsa/b53/b53_regs.h
@@ -105,6 +105,8 @@
 
 /* Power-down mode control */
 #define B53_PD_MODE_CTRL_25		0x0f
+#define  B53_PD_MODE_PORT_MIN		1
+#define  B53_PD_MODE_PORT_MAX		4
 
 /* IP Multicast control (8 bit) */
 #define B53_IP_MULTICAST_CTRL		0x21
-- 
2.39.5

Re: [PATCH net-next v3 14/14] net: dsa: b53: ensure BCM5325 PHYs are enabled
Posted by Jonas Gorski 4 months ago
On Thu, Jun 12, 2025 at 10:38 AM Álvaro Fernández Rojas
<noltari@gmail.com> wrote:
>
> According to the datasheet, BCM5325 uses B53_PD_MODE_CTRL_25 register to
> disable clocking to individual PHYs.
> Only ports 1-4 can be enabled or disabled and the datasheet is explicit
> about not toggling BIT(0) since it disables the PLL power and the switch.
>
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> ---
>  drivers/net/dsa/b53/b53_common.c | 12 ++++++++++++
>  drivers/net/dsa/b53/b53_regs.h   |  2 ++
>  2 files changed, 14 insertions(+)
>
>  v3: add changes requested by Florian:
>   - Use in_range() helper.
>
>  v2: add changes requested by Florian:
>   - Move B53_PD_MODE_CTRL_25 to b53_setup_port().
>
> diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> index 3503f363e2419..eac40e95c8c53 100644
> --- a/drivers/net/dsa/b53/b53_common.c
> +++ b/drivers/net/dsa/b53/b53_common.c
> @@ -660,6 +660,18 @@ int b53_setup_port(struct dsa_switch *ds, int port)
>         if (dsa_is_user_port(ds, port))
>                 b53_set_eap_mode(dev, port, EAP_MODE_SIMPLIFIED);
>
> +       if (is5325(dev) &&
> +           in_range(port, B53_PD_MODE_PORT_MIN, B53_PD_MODE_PORT_MAX)) {

This happen to match, but the third argument of in_range() isn't the
maximum, but the range (max - start), so semantically this looks
wrong.

> +               u8 reg;
> +
> +               b53_read8(dev, B53_CTRL_PAGE, B53_PD_MODE_CTRL_25, &reg);
> +               if (dsa_is_unused_port(ds, port))
> +                       reg |= BIT(port);
> +               else
> +                       reg &= ~BIT(port);
> +               b53_write8(dev, B53_CTRL_PAGE, B53_PD_MODE_CTRL_25, reg);
> +       }
> +
>         return 0;
>  }
>  EXPORT_SYMBOL(b53_setup_port);
> diff --git a/drivers/net/dsa/b53/b53_regs.h b/drivers/net/dsa/b53/b53_regs.h
> index d6849cf6b0a3a..880c67130a9fc 100644
> --- a/drivers/net/dsa/b53/b53_regs.h
> +++ b/drivers/net/dsa/b53/b53_regs.h
> @@ -105,6 +105,8 @@
>
>  /* Power-down mode control */
>  #define B53_PD_MODE_CTRL_25            0x0f
> +#define  B53_PD_MODE_PORT_MIN          1
> +#define  B53_PD_MODE_PORT_MAX          4
>
>  /* IP Multicast control (8 bit) */
>  #define B53_IP_MULTICAST_CTRL          0x21
> --
> 2.39.5
>

Regards,
Jonas
Re: [PATCH net-next v3 14/14] net: dsa: b53: ensure BCM5325 PHYs are enabled
Posted by Álvaro Fernández Rojas 4 months ago
Hi Jonas,

El jue, 12 jun 2025 a las 11:19, Jonas Gorski
(<jonas.gorski@gmail.com>) escribió:
>
> On Thu, Jun 12, 2025 at 10:38 AM Álvaro Fernández Rojas
> <noltari@gmail.com> wrote:
> >
> > According to the datasheet, BCM5325 uses B53_PD_MODE_CTRL_25 register to
> > disable clocking to individual PHYs.
> > Only ports 1-4 can be enabled or disabled and the datasheet is explicit
> > about not toggling BIT(0) since it disables the PLL power and the switch.
> >
> > Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> > ---
> >  drivers/net/dsa/b53/b53_common.c | 12 ++++++++++++
> >  drivers/net/dsa/b53/b53_regs.h   |  2 ++
> >  2 files changed, 14 insertions(+)
> >
> >  v3: add changes requested by Florian:
> >   - Use in_range() helper.
> >
> >  v2: add changes requested by Florian:
> >   - Move B53_PD_MODE_CTRL_25 to b53_setup_port().
> >
> > diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> > index 3503f363e2419..eac40e95c8c53 100644
> > --- a/drivers/net/dsa/b53/b53_common.c
> > +++ b/drivers/net/dsa/b53/b53_common.c
> > @@ -660,6 +660,18 @@ int b53_setup_port(struct dsa_switch *ds, int port)
> >         if (dsa_is_user_port(ds, port))
> >                 b53_set_eap_mode(dev, port, EAP_MODE_SIMPLIFIED);
> >
> > +       if (is5325(dev) &&
> > +           in_range(port, B53_PD_MODE_PORT_MIN, B53_PD_MODE_PORT_MAX)) {
>
> This happen to match, but the third argument of in_range() isn't the
> maximum, but the range (max - start), so semantically this looks
> wrong.

I can change it in order to avoid confusion.
Which one do you prefer?

1. Change defines:
#define B53_PD_MODE_PORT_START 1
#define B53_PD_MODE_PORT_LEN 4
in_range(port, B53_PD_MODE_PORT_START, B53_PD_MODE_PORT_LEN)

2. Just use magic numbers and avoid adding defines:
in_range(port, 1, 4)

>
> > +               u8 reg;
> > +
> > +               b53_read8(dev, B53_CTRL_PAGE, B53_PD_MODE_CTRL_25, &reg);
> > +               if (dsa_is_unused_port(ds, port))
> > +                       reg |= BIT(port);
> > +               else
> > +                       reg &= ~BIT(port);
> > +               b53_write8(dev, B53_CTRL_PAGE, B53_PD_MODE_CTRL_25, reg);
> > +       }
> > +
> >         return 0;
> >  }
> >  EXPORT_SYMBOL(b53_setup_port);
> > diff --git a/drivers/net/dsa/b53/b53_regs.h b/drivers/net/dsa/b53/b53_regs.h
> > index d6849cf6b0a3a..880c67130a9fc 100644
> > --- a/drivers/net/dsa/b53/b53_regs.h
> > +++ b/drivers/net/dsa/b53/b53_regs.h
> > @@ -105,6 +105,8 @@
> >
> >  /* Power-down mode control */
> >  #define B53_PD_MODE_CTRL_25            0x0f
> > +#define  B53_PD_MODE_PORT_MIN          1
> > +#define  B53_PD_MODE_PORT_MAX          4
> >
> >  /* IP Multicast control (8 bit) */
> >  #define B53_IP_MULTICAST_CTRL          0x21
> > --
> > 2.39.5
> >
>
> Regards,
> Jonas