Re: [PATCH net-next v5 3/3] net: stmmac: Add glue layer for Sophgo SG2044 SoC

From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Inochi Amaoto <inochiama@gmail.com>
Cc: "Andrew Lunn" <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	"Eric Dumazet" <edumazet@google.com>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Chen Wang" <unicorn_wang@outlook.com>,
	"Maxime Coquelin" <mcoquelin.stm32@gmail.com>,
	"Alexandre Torgue" <alexandre.torgue@foss.st.com>,
	"Richard Cochran" <richardcochran@gmail.com>,
	"Paul Walmsley" <paul.walmsley@sifive.com>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Albert Ou" <aou@eecs.berkeley.edu>,
	"Jan Petrous (OSS)" <jan.petrous@oss.nxp.com>,
	"Hariprasad Kelam" <hkelam@marvell.com>,
	"Clément Léger" <clement.leger@bootlin.com>,
	"Jisheng Zhang" <jszhang@kernel.org>,
	"Emil Renner Berthing" <emil.renner.berthing@canonical.com>,
	"Drew Fustini" <dfustini@tenstorrent.com>,
	"Furong Xu" <0x1207@gmail.com>,
	"Bartosz Golaszewski" <bartosz.golaszewski@linaro.org>,
	"Joe Hattori" <joe@pf.is.s.u-tokyo.ac.jp>,
	"Serge Semin" <fancer.lancer@gmail.com>,
	"Lothar Rubusch" <l.rubusch@gmail.com>,
	"Giuseppe Cavallaro" <peppe.cavallaro@st.com>,
	"Jose Abreu" <joabreu@synopsys.com>,
	netdev@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, sophgo@lists.linux.dev,
	linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-kernel@lists.infradead.org,
	linux-riscv@lists.infradead.org, "Yixun Lan" <dlan@gentoo.org>,
	"Longbin Li" <looong.bin@gmail.com>
Subject: Re: [PATCH net-next v5 3/3] net: stmmac: Add glue layer for Sophgo SG2044 SoC
Date: Sun, 16 Feb 2025 15:47:18 +0000	[thread overview]
Message-ID: <Z7IIht2Q-iXEFw7x@shell.armlinux.org.uk> (raw)
In-Reply-To: <20250216123953.1252523-4-inochiama@gmail.com>

On Sun, Feb 16, 2025 at 08:39:51PM +0800, Inochi Amaoto wrote:
> +static void sophgo_dwmac_fix_mac_speed(void *priv, unsigned int speed, unsigned int mode)
> +{
> +	struct sophgo_dwmac *dwmac = priv;
> +	long rate;
> +	int ret;
> +
> +	rate = rgmii_clock(speed);
> +	if (rate < 0) {
> +		dev_err(dwmac->dev, "invalid speed %u\n", speed);
> +		return;
> +	}
> +
> +	ret = clk_set_rate(dwmac->clk_tx, rate);
> +	if (ret)
> +		dev_err(dwmac->dev, "failed to set tx rate %ld: %pe\n",
> +			rate, ERR_PTR(ret));
> +}

There are a bunch of other platform support in stmmac that are doing
the same:

dwmac-s32.c is virtually identical
dwmac-imx.c does the same, although has some pre-conditions
dwmac-dwc-qos-eth.c is virually identical but the two steps are split
  across a bunch of register writes
dwmac-starfive.c looks the same
dwmac-rk.c also
dwmac-intel-plat.c also

So, I wonder whether either this should be a helper, or whether core
code should be doing this. Maybe something to look at as a part of
this patch submission?

We really need to stop writing the same code in different ways and
think more about how to reuse the code that's already present!

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