| 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
| ||
|
Message-ID: <Z0OmQI+4sEbA0lFs@lsv051416.swis.nl-cdc01.nxp.com>
Date: Sun, 24 Nov 2024 23:18:40 +0100
From: Jan Petrous <jan.petrous@....nxp.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Maxime Coquelin <mcoquelin.stm32@...il.com>,
Alexandre Torgue <alexandre.torgue@...s.st.com>,
Jose Abreu <joabreu@...opsys.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Vinod Koul <vkoul@...nel.org>,
Richard Cochran <richardcochran@...il.com>,
Andrew Lunn <andrew@...n.ch>,
Heiner Kallweit <hkallweit1@...il.com>,
Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>,
Emil Renner Berthing <kernel@...il.dk>,
Minda Chen <minda.chen@...rfivetech.com>,
Nicolas Ferre <nicolas.ferre@...rochip.com>,
Claudiu Beznea <claudiu.beznea@...on.dev>,
Iyappan Subramanian <iyappan@...amperecomputing.com>,
Keyur Chudgar <keyur@...amperecomputing.com>,
Quan Nguyen <quan@...amperecomputing.com>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Giuseppe Cavallaro <peppe.cavallaro@...com>,
linux-stm32@...md-mailman.stormreply.com,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
netdev@...r.kernel.org, linux-arm-msm@...r.kernel.org,
imx@...ts.linux.dev, devicetree@...r.kernel.org,
NXP S32 Linux Team <s32@....com>
Subject: Re: [PATCH v5 14/16] net: stmmac: dwmac-s32: add basic NXP S32G/S32R
glue driver
On Tue, Nov 19, 2024 at 04:58:35PM +0000, Russell King (Oracle) wrote:
> On Tue, Nov 19, 2024 at 04:00:20PM +0100, Jan Petrous via B4 Relay wrote:
> > From: "Jan Petrous (OSS)" <jan.petrous@....nxp.com>
> >
> > NXP S32G2xx/S32G3xx and S32R45 are automotive grade SoCs
> > that integrate one or two Synopsys DWMAC 5.10/5.20 IPs.
> >
> > The basic driver supports only RGMII interface.
> >
> > Signed-off-by: Jan Petrous (OSS) <jan.petrous@....nxp.com>
>
> One thing that stands out to me in this is the duplication of the PHY
> interface mode. I would much prefer if we didn't end up with multiple
> copies, but instead made use of the one already in plat_stmmacenet_data
> maybe by storing a its pointer in struct s32_priv_data?
I missed the struct plat_stmmacenet_data's phy_interface member. I
switch the duplicate member intf_mode in struct s32_priv_data to
pointer.
>
> > ---
> > drivers/net/ethernet/stmicro/stmmac/Kconfig | 12 ++
> > drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
> > drivers/net/ethernet/stmicro/stmmac/dwmac-s32.c | 204 ++++++++++++++++++++++++
> > 3 files changed, 217 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > index 05cc07b8f48c..a6579377bedb 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > @@ -154,6 +154,18 @@ config DWMAC_RZN1
> > the stmmac device driver. This support can make use of a custom MII
> > converter PCS device.
> >
> > +config DWMAC_S32
> > + tristate "NXP S32G/S32R GMAC support"
> > + default ARCH_S32
> > + depends on OF && (ARCH_S32 || COMPILE_TEST)
> > + help
> > + Support for ethernet controller on NXP S32CC SOCs.
> > +
> > + This selects NXP SoC glue layer support for the stmmac
> > + device driver. This driver is used for the S32CC series
> > + SOCs GMAC ethernet controller, ie. S32G2xx, S32G3xx and
> > + S32R45.
> > +
> > config DWMAC_SOCFPGA
> > tristate "SOCFPGA dwmac support"
> > default ARCH_INTEL_SOCFPGA
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
> > index c2f0e91f6bf8..1e87e2652c82 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
> > +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
> > @@ -22,6 +22,7 @@ obj-$(CONFIG_DWMAC_MESON) += dwmac-meson.o dwmac-meson8b.o
> > obj-$(CONFIG_DWMAC_QCOM_ETHQOS) += dwmac-qcom-ethqos.o
> > obj-$(CONFIG_DWMAC_ROCKCHIP) += dwmac-rk.o
> > obj-$(CONFIG_DWMAC_RZN1) += dwmac-rzn1.o
> > +obj-$(CONFIG_DWMAC_S32) += dwmac-s32.o
> > obj-$(CONFIG_DWMAC_SOCFPGA) += dwmac-altr-socfpga.o
> > obj-$(CONFIG_DWMAC_STARFIVE) += dwmac-starfive.o
> > obj-$(CONFIG_DWMAC_STI) += dwmac-sti.o
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-s32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-s32.c
> > new file mode 100644
> > index 000000000000..9af7cd093100
> > --- /dev/null
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-s32.c
> > @@ -0,0 +1,204 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * NXP S32G/R GMAC glue layer
> > + *
> > + * Copyright 2019-2024 NXP
> > + *
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/clk-provider.h>
> > +#include <linux/device.h>
> > +#include <linux/ethtool.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/of_mdio.h>
> > +#include <linux/of_address.h>
> > +#include <linux/phy.h>
> > +#include <linux/phylink.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/stmmac.h>
> > +
> > +#include "stmmac_platform.h"
> > +
> > +#define GMAC_TX_RATE_125M 125000000 /* 125MHz */
> > +
> > +/* SoC PHY interface control register */
> > +#define PHY_INTF_SEL_MII 0x00
> > +#define PHY_INTF_SEL_SGMII 0x01
> > +#define PHY_INTF_SEL_RGMII 0x02
> > +#define PHY_INTF_SEL_RMII 0x08
> > +
> > +struct s32_priv_data {
> > + void __iomem *ioaddr;
> > + void __iomem *ctrl_sts;
> > + struct device *dev;
> > + phy_interface_t intf_mode;
> > + struct clk *tx_clk;
> > + struct clk *rx_clk;
> > +};
> > +
> > +static int s32_gmac_write_phy_intf_select(struct s32_priv_data *gmac)
> > +{
> > + u32 intf_sel;
> > +
> > + switch (gmac->intf_mode) {
> > + case PHY_INTERFACE_MODE_RGMII:
> > + case PHY_INTERFACE_MODE_RGMII_ID:
> > + case PHY_INTERFACE_MODE_RGMII_TXID:
> > + case PHY_INTERFACE_MODE_RGMII_RXID:
> > + intf_sel = PHY_INTF_SEL_RGMII;
> > + break;
> > + default:
> > + dev_err(gmac->dev, "Unsupported PHY interface: %s\n",
> > + phy_modes(gmac->intf_mode));
> > + return -EINVAL;
> > + }
>
> This can be simplfied to:
>
> if (!phy_interface_mode_is_rgmii(...)) {
> dev_err(gmac->dev, "Unsupported PHY interface: %s\n",
> phy_modes(...));
> return -EINVAL;
> }
>
> Also, would it not be better to validate this in s32_dwmac_probe()?
>
Thanks, I move the interface mode to s32_dwmac_probe() in v6.
BR.
/Jan
Powered by blists - more mailing lists