netdev - Re: [PATCH 2/3] net: stmmac: Add NXP S32 SoC family support

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]
Date: Mon, 18 Mar 2024 00:26:15 +0100
From: Wadim Mueller <wafgo01@...il.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc: Wadim Mueller <wafgo01@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Rob Herring <robh+dt@...nel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
	Conor Dooley <conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Pengutronix Kernel Team <kernel@...gutronix.de>,
	Fabio Estevam <festevam@...il.com>,
	NXP Linux Team <linux-imx@....com>,
	Chester Lin <chester62515@...il.com>,
	Andreas Färber <afaerber@...e.de>,
	Matthias Brugger <mbrugger@...e.com>,
	NXP S32 Linux Team <s32@....com>,
	Alexandre Torgue <alexandre.torgue@...s.st.com>,
	Jose Abreu <joabreu@...opsys.com>,
	Maxime Coquelin <mcoquelin.stm32@...il.com>,
	Michael Turquette <mturquette@...libre.com>,
	Stephen Boyd <sboyd@...nel.org>,
	Richard Cochran <richardcochran@...il.com>,
	Andrew Halaney <ahalaney@...hat.com>,
	Simon Horman <horms@...nel.org>,
	Bartosz Golaszewski <bartosz.golaszewski@...aro.org>,
	Johannes Zink <j.zink@...gutronix.de>,
	Shenwei Wang <shenwei.wang@....com>,
	"Russell King (Oracle)" <rmk+kernel@...linux.org.uk>,
	Swee Leong Ching <leong.ching.swee@...el.com>,
	Giuseppe Cavallaro <peppe.cavallaro@...com>, netdev@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	linux-stm32@...md-mailman.stormreply.com, linux-clk@...r.kernel.org
Subject: Re: [PATCH 2/3] net: stmmac: Add NXP S32 SoC family support

On Sun, Mar 17, 2024 at 03:53:19PM +0100, Krzysztof Kozlowski wrote:
> On 15/03/2024 23:27, Wadim Mueller wrote:
> > Add support for NXP S32 SoC family's GMAC to the stmmac network driver. This driver implementation is based on the patchset originally contributed by Chester Lin [1], which itself draws heavily from NXP's downstream implementation [2]. The patchset was never merged.
> > 
> > The S32G2/3 SoCs feature multiple Ethernet interfaces (PFE0, PFE1, PFE2, and GMAC) which can be routed through a SerDes Subsystem, supporting various interfaces such as SGMII and RGMII. However, the current Glue Code lacks support for SerDes routing and pinctrl handling, relying solely on correct settings in U-Boot. Clock settings for this SoC are managed by the ATF Firmware.
> 
> 
> Please run scripts/checkpatch.pl and fix reported warnings. Some
> warnings can be ignored, but the code here looks like it needs a fix.
> Feel free to get in touch if the warning is not clear.
> 
> Read how commit msg should be wrapped.
> 
> Please wrap commit message according to Linux coding style / submission
> process (neither too early nor over the limit):
> https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597
> 
> > 
> > Changes made compared to [1]:
> > 
> >     Rebased onto Linux 6.8-rc7
> >     Consolidated into a single commit
> >     Minor adjustments in naming and usage of dev_err()/dev_info()
> > 
> > Test Environment:
> > The driver has been successfully tested on the official S32G-VNP-RDB3 Reference Design Board from NXP, utilizing an S32G3 SoC. The firmware and U-Boot used were from the BSP39 Release. The official BSP39 Ubuntu 22.04 Release was successfully booted. A network stress test using iperf [3] was also executed without issues.
> > 
> > [1] https://patchwork.kernel.org/project/netdevbpf/patch/20221031101052.14956-6-clin@suse.com/#25068228
> > [2] https://github.com/nxp-auto-linux/linux/blob/release/bsp39.0-5.15.129-rt/drivers/net/ethernet/stmicro/stmmac/dwmac-s32cc.c
> > [3] https://linux.die.net/man/1/iperf
> > [4] https://github.com/nxp-auto-linux/u-boot
> > [5] https://github.com/nxp-auto-linux/arm-trusted-firmware
> > 
> > Signed-off-by: Wadim Mueller <wafgo01@...il.com>
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/Kconfig   |  12 +
> >  drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
> 
> That's totally unrelated to DTS. Do not mix independent work in one
> patchset. This targets net-next, not SoC, so please send it as separate
> patchset when net-next reopens, so after merge window.
>

Let me try to explain why I was thinking that both should be part of the
same patchset. 

The DTS file patch [1/3] is refering to a NIC for which there is no
upstream driver (or lets call it better glue code for the real driver) available. 

This patch here is supposed to add support for this driver. So without this part the DT
node named "gmac0" of [1/3] is not of much use. Thats why I was thinking they
should be part of one patchset.

But your statement also totally makes sense to me.

Thanks for the feedback!

> >  drivers/net/ethernet/stmicro/stmmac/common.h  |   3 +
> >  .../net/ethernet/stmicro/stmmac/dwmac-s32.c   | 313 ++++++++++++++++++
> >  .../net/ethernet/stmicro/stmmac/dwmac4_dma.c  |   9 +
> >  .../net/ethernet/stmicro/stmmac/dwmac4_dma.h  |   3 +
> >  drivers/net/ethernet/stmicro/stmmac/hwif.h    |   5 +
> >  .../net/ethernet/stmicro/stmmac/stmmac_main.c |   7 +
> >  include/linux/stmmac.h                        |   9 +
> >  9 files changed, 362 insertions(+)
> >  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-s32.c
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > index 85dcda51df05..1cdf2da0251c 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > @@ -142,6 +142,18 @@ config DWMAC_ROCKCHIP
> >  	  This selects the Rockchip RK3288 SoC glue layer support for
> >  	  the stmmac device drive
> 
> 
> ...
> 
> > +
> > +	plat_dat->safety_feat_cfg->tsoee = 1;
> > +	plat_dat->safety_feat_cfg->mrxpee = 1;
> > +	plat_dat->safety_feat_cfg->mestee = 1;
> > +	plat_dat->safety_feat_cfg->mrxee = 1;
> > +	plat_dat->safety_feat_cfg->mtxee = 1;
> > +	plat_dat->safety_feat_cfg->epsi = 1;
> > +	plat_dat->safety_feat_cfg->edpp = 1;
> > +	plat_dat->safety_feat_cfg->prtyen = 1;
> > +	plat_dat->safety_feat_cfg->tmouten = 1;
> > +
> > +	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
> > +	if (ret)
> > +		goto err_gmac_exit;
> > +
> > +	return 0;
> > +
> > +err_gmac_exit:
> > +	s32_gmac_exit(pdev, plat_dat->bsp_priv);
> > +	return ret;
> > +}
> > +
> > +static const struct of_device_id s32_dwmac_match[] = {
> > +	{ .compatible = "nxp,s32-dwmac" },
> 
> 
> Missing bindings.
> 
> Please run scripts/checkpatch.pl and fix reported warnings. Some
> warnings can be ignored, but the code here looks like it needs a fix.
> Feel free to get in touch if the warning is not clear.
> 
> > +	{}
> > +};
> 
> 
> 
> Best regards,
> Krzysztof
>