netdev - Re: [PATCH net-next 2/3] net: stmmac: qcom-ethqos: add rgmii set/clear functions

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: <aR7jZ4KkKE9nTsMh@shell.armlinux.org.uk>
Date: Thu, 20 Nov 2025 09:46:15 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
Cc: Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>,
	Alexandre Torgue <alexandre.torgue@...s.st.com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>,
	linux-arm-kernel@...ts.infradead.org, linux-arm-msm@...r.kernel.org,
	linux-stm32@...md-mailman.stormreply.com,
	Maxime Coquelin <mcoquelin.stm32@...il.com>, netdev@...r.kernel.org,
	Paolo Abeni <pabeni@...hat.com>, Vinod Koul <vkoul@...nel.org>
Subject: Re: [PATCH net-next 2/3] net: stmmac: qcom-ethqos: add rgmii
 set/clear functions

On Thu, Nov 20, 2025 at 10:42:04AM +0100, Konrad Dybcio wrote:
> On 11/19/25 12:34 PM, Russell King (Oracle) wrote:
> > The driver has a lot of bit manipulation of the RGMII registers. Add
> > a pair of helpers to set bits and clear bits, converting the various
> > calls to rgmii_updatel() as appropriate.
> > 
> > Most of the change was done via this sed script:
> > 
> > /rgmii_updatel/ {
> > 	N
> > 	/,$/N
> > 	/mask, / ! {
> > 		s|rgmii_updatel\(([^,]*,\s+([^,]*),\s+)\2,\s+|rgmii_setmask(\1|
> > 		s|rgmii_updatel\(([^,]*,\s+([^,]*),\s+)0,\s+|rgmii_clrmask(\1|
> > 		s|^\s+$||
> > 	}
> > }
> > 
> > and then formatting tweaked where necessary.
> > 
> > Signed-off-by: Russell King (Oracle) <rmk+kernel@...linux.org.uk>
> > ---
> >  .../stmicro/stmmac/dwmac-qcom-ethqos.c        | 187 +++++++++---------
> >  1 file changed, 89 insertions(+), 98 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> > index ae3cf163005b..cdaf02471d3a 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> > @@ -137,6 +137,18 @@ static void rgmii_updatel(struct qcom_ethqos *ethqos, u32 mask, u32 val,
> >  	rgmii_writel(ethqos, temp, offset);
> >  }
> >  
> > +static void rgmii_setmask(struct qcom_ethqos *ethqos, u32 mask,
> > +			  unsigned int offset)
> > +{
> > +	rgmii_updatel(ethqos, mask, mask, offset);
> > +}
> 
> It's almost unbelieveable there's no set/clr/rmw generics for
> readl and friends

Consider what that would mean - such operations can not be atomic, but
users would likely not realise, which means we get a load of new
potential bugs. Not having these means that driver authors get to
code this up, and because they realise they have to do separate read
and write operations, it's more obvious that there may be races.

The phy_* accessors are different - these take the bus lock while they
operate, and thus are atomic.

> 
> [...]
> >  	/* Set DLL_EN */
> > -	rgmii_updatel(ethqos, SDCC_DLL_CONFIG_DLL_EN,
> > -		      SDCC_DLL_CONFIG_DLL_EN, SDCC_HC_REG_DLL_CONFIG);
> > +	rgmii_setmask(ethqos, SDCC_DLL_CONFIG_DLL_EN,  SDCC_HC_REG_DLL_CONFIG);
> 
> double space
> 
> [...]
> 
> >  	/* Select RGMII, write 0 to interface select */
> > -	rgmii_updatel(ethqos, RGMII_CONFIG_INTF_SEL,
> > -		      0, RGMII_IO_MACRO_CONFIG);
> > +	rgmii_clrmask(ethqos, RGMII_CONFIG_INTF_SEL,  RGMII_IO_MACRO_CONFIG);
> 
> and here
> 
> Everything else looks in tact

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!