netdev - Re: [PATCH net v2 5/5] net: dsa: b53: do not touch DLL

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAOiHx=n6Mc+nM2QOa8okQbFcj9UHgfMbKKcNXG6D-VJjELHrsw@mail.gmail.com>
Date: Tue, 3 Jun 2025 08:15:38 +0200
From: Jonas Gorski <jonas.gorski@...il.com>
To: Florian Fainelli <florian.fainelli@...adcom.com>
Cc: Andrew Lunn <andrew@...n.ch>, Vladimir Oltean <olteanv@...il.com>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	Vivien Didelot <vivien.didelot@...il.com>, Álvaro Fernández Rojas <noltari@...il.com>, 
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net v2 5/5] net: dsa: b53: do not touch DLL_IQQD on bcm53115

On Mon, Jun 2, 2025 at 11:40 PM Florian Fainelli
<florian.fainelli@...adcom.com> wrote:
>
> On 6/2/25 12:39, Jonas Gorski wrote:
> > According to OpenMDK, bit 2 of the RGMII register has a different
> > meaning for BCM53115 [1]:
> >
> > "DLL_IQQD         1: In the IDDQ mode, power is down0: Normal function
> >                    mode"
> >
> > Configuring RGMII delay works without setting this bit, so let's keep it
> > at the default. For other chips, we always set it, so not clearing it
> > is not an issue.
> >
> > One would assume BCM53118 works the same, but OpenMDK is not quite sure
> > what this bit actually means [2]:
> >
> > "BYPASS_IMP_2NS_DEL #1: In the IDDQ mode, power is down#0: Normal
> >                      function mode1: Bypass dll65_2ns_del IP0: Use
> >                      dll65_2ns_del IP"
> >
> > So lets keep setting it for now.
> >
> > [1] https://github.com/Broadcom-Network-Switching-Software/OpenMDK/blob/master/cdk/PKG/chip/bcm53115/bcm53115_a0_defs.h#L19871
> > [2] https://github.com/Broadcom-Network-Switching-Software/OpenMDK/blob/master/cdk/PKG/chip/bcm53118/bcm53118_a0_defs.h#L14392
> >
> > Fixes: 967dd82ffc52 ("net: dsa: b53: Add support for Broadcom RoboSwitch")
> > Signed-off-by: Jonas Gorski <jonas.gorski@...il.com>
> > ---
> > v1 -> v2:
> > * new patch
> >
> >   drivers/net/dsa/b53/b53_common.c | 8 +++++---
> >   1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> > index be4493b769f4..862bdccb7439 100644
> > --- a/drivers/net/dsa/b53/b53_common.c
> > +++ b/drivers/net/dsa/b53/b53_common.c
> > @@ -1354,8 +1354,7 @@ static void b53_adjust_531x5_rgmii(struct dsa_switch *ds, int port,
> >        * tx_clk aligned timing (restoring to reset defaults)
> >        */
> >       b53_read8(dev, B53_CTRL_PAGE, off, &rgmii_ctrl);
> > -     rgmii_ctrl &= ~(RGMII_CTRL_DLL_RXC | RGMII_CTRL_DLL_TXC |
> > -                     RGMII_CTRL_TIMING_SEL);
> > +     rgmii_ctrl &= ~(RGMII_CTRL_DLL_RXC | RGMII_CTRL_DLL_TXC);
>
> Are not we missing a:
>
> if (dev->chip_id != BCM53115_DEVICE_ID)
>         rgmii_ctrl &= ~RGMII_CTRL_TIMING_SEL;
>
> here to be strictly identical before/after?

We could add it for symmetry, but it would be purely decorational. We
unconditionally set this bit again later, so clearing it before has no
actual effect, which is why I didn't add it.

Regards,
Jonas