[PATCH 1/1] net: stmmac: add support for platform specific reset

[PATCH 1/1] net: stmmac: add support for platform specific reset

[Shenwei Wang]

This patch adds support for platform-specific reset logic in the
stmmac driver. Some SoCs require a different reset mechanism than
the standard dwmac IP reset. To support these platforms, a new function
pointer 'fix_soc_reset' is added to the plat_stmmacenet_data structure.
The stmmac_reset macro in hwif.h is modified to call the 'fix_soc_reset'
function if it exists. This enables the driver to use the platform-specific
reset logic when necessary.

Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
 drivers/net/ethernet/stmicro/stmmac/hwif.h | 10 +++++++++-
 include/linux/stmmac.h                     |  1 +
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index 16a7421715cb..e24ce870690e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -215,7 +215,15 @@ struct stmmac_dma_ops {
 };
 
 #define stmmac_reset(__priv, __args...) \
-	stmmac_do_callback(__priv, dma, reset, __args)
+({ \
+	int __result = -EINVAL; \
+	if ((__priv) && (__priv)->plat && (__priv)->plat->fix_soc_reset) { \
+		__result = (__priv)->plat->fix_soc_reset((__priv)->plat, ##__args); \
+	} else { \
+		__result = stmmac_do_callback(__priv, dma, reset, __args); \
+	} \
+	__result; \
+})
 #define stmmac_dma_init(__priv, __args...) \
 	stmmac_do_void_callback(__priv, dma, init, __args)
 #define stmmac_init_chan(__priv, __args...) \
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index a152678b82b7..9044477fad61 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -223,6 +223,7 @@ struct plat_stmmacenet_data {
 	struct stmmac_rxq_cfg rx_queues_cfg[MTL_MAX_RX_QUEUES];
 	struct stmmac_txq_cfg tx_queues_cfg[MTL_MAX_TX_QUEUES];
 	void (*fix_mac_speed)(void *priv, unsigned int speed);
+	int (*fix_soc_reset)(void *priv, void __iomem *ioaddr);
 	int (*serdes_powerup)(struct net_device *ndev, void *priv);
 	void (*serdes_powerdown)(struct net_device *ndev, void *priv);
 	void (*speed_mode_2500)(struct net_device *ndev, void *priv);
-- 
2.34.1

[PATCH 1/1] net: stmmac: add support for platform specific reset

[Shenwei Wang]

This patch adds support for platform-specific reset logic in the
stmmac driver. Some SoCs require a different reset mechanism than
the standard dwmac IP reset. To support these platforms, a new function
pointer 'fix_soc_reset' is added to the plat_stmmacenet_data structure.
The stmmac_reset macro in hwif.h is modified to call the 'fix_soc_reset'
function if it exists. This enables the driver to use the platform-specific
reset logic when necessary.

Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
 drivers/net/ethernet/stmicro/stmmac/hwif.h | 10 +++++++++-
 include/linux/stmmac.h                     |  1 +
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index 16a7421715cb..e24ce870690e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -215,7 +215,15 @@ struct stmmac_dma_ops {
 };
 
 #define stmmac_reset(__priv, __args...) \
-	stmmac_do_callback(__priv, dma, reset, __args)
+({ \
+	int __result = -EINVAL; \
+	if ((__priv) && (__priv)->plat && (__priv)->plat->fix_soc_reset) { \
+		__result = (__priv)->plat->fix_soc_reset((__priv)->plat, ##__args); \
+	} else { \
+		__result = stmmac_do_callback(__priv, dma, reset, __args); \
+	} \
+	__result; \
+})
 #define stmmac_dma_init(__priv, __args...) \
 	stmmac_do_void_callback(__priv, dma, init, __args)
 #define stmmac_init_chan(__priv, __args...) \
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index a152678b82b7..9044477fad61 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -223,6 +223,7 @@ struct plat_stmmacenet_data {
 	struct stmmac_rxq_cfg rx_queues_cfg[MTL_MAX_RX_QUEUES];
 	struct stmmac_txq_cfg tx_queues_cfg[MTL_MAX_TX_QUEUES];
 	void (*fix_mac_speed)(void *priv, unsigned int speed);
+	int (*fix_soc_reset)(void *priv, void __iomem *ioaddr);
 	int (*serdes_powerup)(struct net_device *ndev, void *priv);
 	void (*serdes_powerdown)(struct net_device *ndev, void *priv);
 	void (*speed_mode_2500)(struct net_device *ndev, void *priv);
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] net: stmmac: dwmac-imx: use platform specific reset for imx93 SoCs

[Shenwei Wang]

The patch addresses an issue with the reset logic on the i.MX93 SoC, which
requires configuration of the correct interface speed under RMII mode to
complete the reset. The patch implements a fix_soc_reset function and uses
it specifically for the i.MX93 SoCs.

Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-imx.c   | 27 ++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
index ac8580f501e2..3329150010ce 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
@@ -19,9 +19,9 @@
 #include <linux/pm_wakeirq.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
-#include <linux/stmmac.h>
 
 #include "stmmac_platform.h"
+#include "common.h"
 
 #define GPR_ENET_QOS_INTF_MODE_MASK	GENMASK(21, 16)
 #define GPR_ENET_QOS_INTF_SEL_MII	(0x0 << 16)
@@ -37,6 +37,10 @@
 #define MX93_GPR_ENET_QOS_INTF_SEL_RGMII	(0x1 << 1)
 #define MX93_GPR_ENET_QOS_CLK_GEN_EN		(0x1 << 0)
 
+#define DMA_BUS_MODE			0x00001000
+#define DMA_BUS_MODE_SFT_RESET		(0x1 << 0)
+#define RMII_RESET_SPEED		(0x3 << 14)
+
 struct imx_dwmac_ops {
 	u32 addr_width;
 	bool mac_rgmii_txclk_auto_adj;
@@ -207,6 +211,24 @@ static void imx_dwmac_fix_speed(void *priv, unsigned int speed)
 		dev_err(dwmac->dev, "failed to set tx rate %lu\n", rate);
 }
 
+static int imx_dwmac_mx93_reset(void *priv, void __iomem *ioaddr)
+{
+	struct plat_stmmacenet_data *plat_dat = priv;
+	u32 value = readl(ioaddr + DMA_BUS_MODE);
+
+	/* DMA SW reset */
+	value |= DMA_BUS_MODE_SFT_RESET;
+	writel(value, ioaddr + DMA_BUS_MODE);
+
+	usleep_range(100, 200);
+	if (plat_dat->interface == PHY_INTERFACE_MODE_RMII)
+		writel(RMII_RESET_SPEED, ioaddr + MAC_CTRL_REG);
+
+	return readl_poll_timeout(ioaddr + DMA_BUS_MODE, value,
+				 !(value & DMA_BUS_MODE_SFT_RESET),
+				 10000, 1000000);
+}
+
 static int
 imx_dwmac_parse_dt(struct imx_priv_data *dwmac, struct device *dev)
 {
@@ -305,6 +327,9 @@ static int imx_dwmac_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_dwmac_init;
 
+	if (of_machine_is_compatible("fsl,imx93"))
+		dwmac->plat_dat->fix_soc_reset = imx_dwmac_mx93_reset;
+
 	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
 	if (ret)
 		goto err_drv_probe;
-- 
2.34.1

[PATCH 2/2] net: stmmac: dwmac-imx: use platform specific reset for imx93 SoCs

[Shenwei Wang]

The patch addresses an issue with the reset logic on the i.MX93 SoC, which
requires configuration of the correct interface speed under RMII mode to
complete the reset. The patch implements a fix_soc_reset function and uses
it specifically for the i.MX93 SoCs.

Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-imx.c   | 27 ++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
index ac8580f501e2..3329150010ce 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
@@ -19,9 +19,9 @@
 #include <linux/pm_wakeirq.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
-#include <linux/stmmac.h>
 
 #include "stmmac_platform.h"
+#include "common.h"
 
 #define GPR_ENET_QOS_INTF_MODE_MASK	GENMASK(21, 16)
 #define GPR_ENET_QOS_INTF_SEL_MII	(0x0 << 16)
@@ -37,6 +37,10 @@
 #define MX93_GPR_ENET_QOS_INTF_SEL_RGMII	(0x1 << 1)
 #define MX93_GPR_ENET_QOS_CLK_GEN_EN		(0x1 << 0)
 
+#define DMA_BUS_MODE			0x00001000
+#define DMA_BUS_MODE_SFT_RESET		(0x1 << 0)
+#define RMII_RESET_SPEED		(0x3 << 14)
+
 struct imx_dwmac_ops {
 	u32 addr_width;
 	bool mac_rgmii_txclk_auto_adj;
@@ -207,6 +211,24 @@ static void imx_dwmac_fix_speed(void *priv, unsigned int speed)
 		dev_err(dwmac->dev, "failed to set tx rate %lu\n", rate);
 }
 
+static int imx_dwmac_mx93_reset(void *priv, void __iomem *ioaddr)
+{
+	struct plat_stmmacenet_data *plat_dat = priv;
+	u32 value = readl(ioaddr + DMA_BUS_MODE);
+
+	/* DMA SW reset */
+	value |= DMA_BUS_MODE_SFT_RESET;
+	writel(value, ioaddr + DMA_BUS_MODE);
+
+	usleep_range(100, 200);
+	if (plat_dat->interface == PHY_INTERFACE_MODE_RMII)
+		writel(RMII_RESET_SPEED, ioaddr + MAC_CTRL_REG);
+
+	return readl_poll_timeout(ioaddr + DMA_BUS_MODE, value,
+				 !(value & DMA_BUS_MODE_SFT_RESET),
+				 10000, 1000000);
+}
+
 static int
 imx_dwmac_parse_dt(struct imx_priv_data *dwmac, struct device *dev)
 {
@@ -305,6 +327,9 @@ static int imx_dwmac_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_dwmac_init;
 
+	if (of_machine_is_compatible("fsl,imx93"))
+		dwmac->plat_dat->fix_soc_reset = imx_dwmac_mx93_reset;
+
 	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
 	if (ret)
 		goto err_drv_probe;
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/1] net: stmmac: add support for platform specific reset

[Simon Horman]

On Tue, Mar 21, 2023 at 02:09:20PM -0500, Shenwei Wang wrote:
> This patch adds support for platform-specific reset logic in the
> stmmac driver. Some SoCs require a different reset mechanism than
> the standard dwmac IP reset. To support these platforms, a new function
> pointer 'fix_soc_reset' is added to the plat_stmmacenet_data structure.
> The stmmac_reset macro in hwif.h is modified to call the 'fix_soc_reset'
> function if it exists. This enables the driver to use the platform-specific
> reset logic when necessary.
> 
> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/hwif.h | 10 +++++++++-
>  include/linux/stmmac.h                     |  1 +
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> index 16a7421715cb..e24ce870690e 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> @@ -215,7 +215,15 @@ struct stmmac_dma_ops {
>  };
>  
>  #define stmmac_reset(__priv, __args...) \
> -	stmmac_do_callback(__priv, dma, reset, __args)
> +({ \
> +	int __result = -EINVAL; \
> +	if ((__priv) && (__priv)->plat && (__priv)->plat->fix_soc_reset) { \
> +		__result = (__priv)->plat->fix_soc_reset((__priv)->plat, ##__args); \
> +	} else { \
> +		__result = stmmac_do_callback(__priv, dma, reset, __args); \
> +	} \
> +	__result; \
> +})

Hi Shenwei Wang,

I am wondering if any consideration was given to an approach
that has a bit better type safety.

Something like this (*compile tested only!*):

static inline int stmmac_reset(struct stmmac_priv *priv, void __iomem *ioaddr)
{
       struct plat_stmmacenet_data *plat = priv ? priv->plat : NULL;

       if (plat && plat->fix_soc_reset)
	       return plat->fix_soc_reset(plat, ioaddr);

       return stmmac_do_callback(priv, dma, reset, ioaddr);
}

In which case the first parameter of the fix_soc_reset field
of struct plat_stmmacenet_data can become struct plat_stmmacenet_data *.

diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 9044477fad61..b26ade7e4be8 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -223,6 +223,8 @@ struct plat_stmmacenet_data {
 	struct stmmac_rxq_cfg rx_queues_cfg[MTL_MAX_RX_QUEUES];
 	struct stmmac_txq_cfg tx_queues_cfg[MTL_MAX_TX_QUEUES];
 	void (*fix_mac_speed)(void *priv, unsigned int speed);
+	int (*fix_soc_reset)(struct plat_stmmacenet_data *,
+			     void __iomem *ioaddr);
 	int (*serdes_powerup)(struct net_device *ndev, void *priv);
 	void (*serdes_powerdown)(struct net_device *ndev, void *priv);
 	void (*speed_mode_2500)(struct net_device *ndev, void *priv);

I do see that the approach you have is
in keeping with the existing structure of stmmac_do_callback().
But I guess my question there is: why is that model used?
And could there be a plan to move away from that model?

Re: [PATCH 1/1] net: stmmac: add support for platform specific reset

[Simon Horman]

On Tue, Mar 21, 2023 at 02:09:20PM -0500, Shenwei Wang wrote:
> This patch adds support for platform-specific reset logic in the
> stmmac driver. Some SoCs require a different reset mechanism than
> the standard dwmac IP reset. To support these platforms, a new function
> pointer 'fix_soc_reset' is added to the plat_stmmacenet_data structure.
> The stmmac_reset macro in hwif.h is modified to call the 'fix_soc_reset'
> function if it exists. This enables the driver to use the platform-specific
> reset logic when necessary.
> 
> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/hwif.h | 10 +++++++++-
>  include/linux/stmmac.h                     |  1 +
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> index 16a7421715cb..e24ce870690e 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> @@ -215,7 +215,15 @@ struct stmmac_dma_ops {
>  };
>  
>  #define stmmac_reset(__priv, __args...) \
> -	stmmac_do_callback(__priv, dma, reset, __args)
> +({ \
> +	int __result = -EINVAL; \
> +	if ((__priv) && (__priv)->plat && (__priv)->plat->fix_soc_reset) { \
> +		__result = (__priv)->plat->fix_soc_reset((__priv)->plat, ##__args); \
> +	} else { \
> +		__result = stmmac_do_callback(__priv, dma, reset, __args); \
> +	} \
> +	__result; \
> +})

Hi Shenwei Wang,

I am wondering if any consideration was given to an approach
that has a bit better type safety.

Something like this (*compile tested only!*):

static inline int stmmac_reset(struct stmmac_priv *priv, void __iomem *ioaddr)
{
       struct plat_stmmacenet_data *plat = priv ? priv->plat : NULL;

       if (plat && plat->fix_soc_reset)
	       return plat->fix_soc_reset(plat, ioaddr);

       return stmmac_do_callback(priv, dma, reset, ioaddr);
}

In which case the first parameter of the fix_soc_reset field
of struct plat_stmmacenet_data can become struct plat_stmmacenet_data *.

diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 9044477fad61..b26ade7e4be8 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -223,6 +223,8 @@ struct plat_stmmacenet_data {
 	struct stmmac_rxq_cfg rx_queues_cfg[MTL_MAX_RX_QUEUES];
 	struct stmmac_txq_cfg tx_queues_cfg[MTL_MAX_TX_QUEUES];
 	void (*fix_mac_speed)(void *priv, unsigned int speed);
+	int (*fix_soc_reset)(struct plat_stmmacenet_data *,
+			     void __iomem *ioaddr);
 	int (*serdes_powerup)(struct net_device *ndev, void *priv);
 	void (*serdes_powerdown)(struct net_device *ndev, void *priv);
 	void (*speed_mode_2500)(struct net_device *ndev, void *priv);

I do see that the approach you have is
in keeping with the existing structure of stmmac_do_callback().
But I guess my question there is: why is that model used?
And could there be a plan to move away from that model?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [EXT] Re: [PATCH 1/1] net: stmmac: add support for platform specific reset

[Shenwei Wang]

> -----Original Message-----
> From: Simon Horman <simon.horman@corigine.com>
> Sent: Wednesday, March 22, 2023 11:33 AM
> To: Shenwei Wang <shenwei.wang@nxp.com>
> Cc: David S. Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Maxime Coquelin <mcoquelin.stm32@gmail.com>;
> Shawn Guo <shawnguo@kernel.org>; dl-linux-imx <linux-imx@nxp.com>;
> Giuseppe Cavallaro <peppe.cavallaro@st.com>; Alexandre Torgue
> <alexandre.torgue@foss.st.com>; Jose Abreu <joabreu@synopsys.com>; Sascha
> Hauer <s.hauer@pengutronix.de>; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Wong Vee
> Khee <veekhee@apple.com>; Jon Hunter <jonathanh@nvidia.com>;
> Mohammad Athari Bin Ismail <mohammad.athari.ismail@intel.com>; Andrey
> Konovalov <andrey.konovalov@linaro.org>; Revanth Kumar Uppala
> <ruppala@nvidia.com>; Tan Tee Min <tee.min.tan@linux.intel.com>;
> netdev@vger.kernel.org; linux-stm32@st-md-mailman.stormreply.com; linux-
> arm-kernel@lists.infradead.org; imx@lists.linux.dev
> Subject: [EXT] Re: [PATCH 1/1] net: stmmac: add support for platform specific
> reset
> 
> Caution: EXT Email
> 
> On Tue, Mar 21, 2023 at 02:09:20PM -0500, Shenwei Wang wrote:
> > This patch adds support for platform-specific reset logic in the
> > stmmac driver. Some SoCs require a different reset mechanism than the
> > standard dwmac IP reset. To support these platforms, a new function
> > pointer 'fix_soc_reset' is added to the plat_stmmacenet_data structure.
> > The stmmac_reset macro in hwif.h is modified to call the 'fix_soc_reset'
> > function if it exists. This enables the driver to use the
> > platform-specific reset logic when necessary.
> >
> > Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/hwif.h | 10 +++++++++-
> >  include/linux/stmmac.h                     |  1 +
> >  2 files changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h
> > b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> > index 16a7421715cb..e24ce870690e 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
> > +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> > @@ -215,7 +215,15 @@ struct stmmac_dma_ops {  };
> >
> >  #define stmmac_reset(__priv, __args...) \
> > -     stmmac_do_callback(__priv, dma, reset, __args)
> > +({ \
> > +     int __result = -EINVAL; \
> > +     if ((__priv) && (__priv)->plat && (__priv)->plat->fix_soc_reset) { \
> > +             __result = (__priv)->plat->fix_soc_reset((__priv)->plat, ##__args); \
> > +     } else { \
> > +             __result = stmmac_do_callback(__priv, dma, reset, __args); \
> > +     } \
> > +     __result; \
> > +})
> 
> Hi Shenwei Wang,
> 
> I am wondering if any consideration was given to an approach that has a bit
> better type safety.
> 

The patch aims to add the missing feature to the original source file in a consistent coding style. 
I am also okay to use the function prototype if everyone agrees.

Thanks,
Shenwei

> Something like this (*compile tested only!*):
> 
> static inline int stmmac_reset(struct stmmac_priv *priv, void __iomem *ioaddr) {
>        struct plat_stmmacenet_data *plat = priv ? priv->plat : NULL;
> 
>        if (plat && plat->fix_soc_reset)
>                return plat->fix_soc_reset(plat, ioaddr);
> 
>        return stmmac_do_callback(priv, dma, reset, ioaddr); }
> 
> In which case the first parameter of the fix_soc_reset field of struct
> plat_stmmacenet_data can become struct plat_stmmacenet_data *.
> 
> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h index
> 9044477fad61..b26ade7e4be8 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -223,6 +223,8 @@ struct plat_stmmacenet_data {
>         struct stmmac_rxq_cfg rx_queues_cfg[MTL_MAX_RX_QUEUES];
>         struct stmmac_txq_cfg tx_queues_cfg[MTL_MAX_TX_QUEUES];
>         void (*fix_mac_speed)(void *priv, unsigned int speed);
> +       int (*fix_soc_reset)(struct plat_stmmacenet_data *,
> +                            void __iomem *ioaddr);
>         int (*serdes_powerup)(struct net_device *ndev, void *priv);
>         void (*serdes_powerdown)(struct net_device *ndev, void *priv);
>         void (*speed_mode_2500)(struct net_device *ndev, void *priv);
> 
> I do see that the approach you have is
> in keeping with the existing structure of stmmac_do_callback().
> But I guess my question there is: why is that model used?
> And could there be a plan to move away from that model?

RE: [EXT] Re: [PATCH 1/1] net: stmmac: add support for platform specific reset

[Shenwei Wang]

> -----Original Message-----
> From: Simon Horman <simon.horman@corigine.com>
> Sent: Wednesday, March 22, 2023 11:33 AM
> To: Shenwei Wang <shenwei.wang@nxp.com>
> Cc: David S. Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Maxime Coquelin <mcoquelin.stm32@gmail.com>;
> Shawn Guo <shawnguo@kernel.org>; dl-linux-imx <linux-imx@nxp.com>;
> Giuseppe Cavallaro <peppe.cavallaro@st.com>; Alexandre Torgue
> <alexandre.torgue@foss.st.com>; Jose Abreu <joabreu@synopsys.com>; Sascha
> Hauer <s.hauer@pengutronix.de>; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Wong Vee
> Khee <veekhee@apple.com>; Jon Hunter <jonathanh@nvidia.com>;
> Mohammad Athari Bin Ismail <mohammad.athari.ismail@intel.com>; Andrey
> Konovalov <andrey.konovalov@linaro.org>; Revanth Kumar Uppala
> <ruppala@nvidia.com>; Tan Tee Min <tee.min.tan@linux.intel.com>;
> netdev@vger.kernel.org; linux-stm32@st-md-mailman.stormreply.com; linux-
> arm-kernel@lists.infradead.org; imx@lists.linux.dev
> Subject: [EXT] Re: [PATCH 1/1] net: stmmac: add support for platform specific
> reset
> 
> Caution: EXT Email
> 
> On Tue, Mar 21, 2023 at 02:09:20PM -0500, Shenwei Wang wrote:
> > This patch adds support for platform-specific reset logic in the
> > stmmac driver. Some SoCs require a different reset mechanism than the
> > standard dwmac IP reset. To support these platforms, a new function
> > pointer 'fix_soc_reset' is added to the plat_stmmacenet_data structure.
> > The stmmac_reset macro in hwif.h is modified to call the 'fix_soc_reset'
> > function if it exists. This enables the driver to use the
> > platform-specific reset logic when necessary.
> >
> > Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/hwif.h | 10 +++++++++-
> >  include/linux/stmmac.h                     |  1 +
> >  2 files changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h
> > b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> > index 16a7421715cb..e24ce870690e 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
> > +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> > @@ -215,7 +215,15 @@ struct stmmac_dma_ops {  };
> >
> >  #define stmmac_reset(__priv, __args...) \
> > -     stmmac_do_callback(__priv, dma, reset, __args)
> > +({ \
> > +     int __result = -EINVAL; \
> > +     if ((__priv) && (__priv)->plat && (__priv)->plat->fix_soc_reset) { \
> > +             __result = (__priv)->plat->fix_soc_reset((__priv)->plat, ##__args); \
> > +     } else { \
> > +             __result = stmmac_do_callback(__priv, dma, reset, __args); \
> > +     } \
> > +     __result; \
> > +})
> 
> Hi Shenwei Wang,
> 
> I am wondering if any consideration was given to an approach that has a bit
> better type safety.
> 

The patch aims to add the missing feature to the original source file in a consistent coding style. 
I am also okay to use the function prototype if everyone agrees.

Thanks,
Shenwei

> Something like this (*compile tested only!*):
> 
> static inline int stmmac_reset(struct stmmac_priv *priv, void __iomem *ioaddr) {
>        struct plat_stmmacenet_data *plat = priv ? priv->plat : NULL;
> 
>        if (plat && plat->fix_soc_reset)
>                return plat->fix_soc_reset(plat, ioaddr);
> 
>        return stmmac_do_callback(priv, dma, reset, ioaddr); }
> 
> In which case the first parameter of the fix_soc_reset field of struct
> plat_stmmacenet_data can become struct plat_stmmacenet_data *.
> 
> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h index
> 9044477fad61..b26ade7e4be8 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -223,6 +223,8 @@ struct plat_stmmacenet_data {
>         struct stmmac_rxq_cfg rx_queues_cfg[MTL_MAX_RX_QUEUES];
>         struct stmmac_txq_cfg tx_queues_cfg[MTL_MAX_TX_QUEUES];
>         void (*fix_mac_speed)(void *priv, unsigned int speed);
> +       int (*fix_soc_reset)(struct plat_stmmacenet_data *,
> +                            void __iomem *ioaddr);
>         int (*serdes_powerup)(struct net_device *ndev, void *priv);
>         void (*serdes_powerdown)(struct net_device *ndev, void *priv);
>         void (*speed_mode_2500)(struct net_device *ndev, void *priv);
> 
> I do see that the approach you have is
> in keeping with the existing structure of stmmac_do_callback().
> But I guess my question there is: why is that model used?
> And could there be a plan to move away from that model?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH 1/1] net: stmmac: add support for platform specific reset

[Shenwei Wang]

> -----Original Message-----
> From: Shenwei Wang <shenwei.wang@nxp.com>
> Sent: Tuesday, March 21, 2023 2:09 PM
> To: David S. Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Maxime Coquelin <mcoquelin.stm32@gmail.com>;
> Shawn Guo <shawnguo@kernel.org>; dl-linux-imx <linux-imx@nxp.com>
> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>; Alexandre Torgue
> <alexandre.torgue@foss.st.com>; Jose Abreu <joabreu@synopsys.com>; Sascha
> Hauer <s.hauer@pengutronix.de>; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Wong Vee
> Khee <veekhee@apple.com>; Jon Hunter <jonathanh@nvidia.com>;
> Mohammad Athari Bin Ismail <mohammad.athari.ismail@intel.com>; Andrey
> Konovalov <andrey.konovalov@linaro.org>; Revanth Kumar Uppala
> <ruppala@nvidia.com>; Tan Tee Min <tee.min.tan@linux.intel.com>; Shenwei
> Wang <shenwei.wang@nxp.com>; netdev@vger.kernel.org; linux-stm32@st-
> md-mailman.stormreply.com; linux-arm-kernel@lists.infradead.org;
> imx@lists.linux.dev
> Subject: [PATCH 1/1] net: stmmac: add support for platform specific reset
> 
> This patch adds support for platform-specific reset logic in the stmmac driver.
> Some SoCs require a different reset mechanism than the standard dwmac IP
> reset. To support these platforms, a new function pointer 'fix_soc_reset' is
> added to the plat_stmmacenet_data structure.
> The stmmac_reset macro in hwif.h is modified to call the 'fix_soc_reset'
> function if it exists. This enables the driver to use the platform-specific reset
> logic when necessary.
> 

A soft ping. 😊

Thanks,
Shenwei

> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/hwif.h | 10 +++++++++-
>  include/linux/stmmac.h                     |  1 +
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h
> b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> index 16a7421715cb..e24ce870690e 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> @@ -215,7 +215,15 @@ struct stmmac_dma_ops {  };
> 
>  #define stmmac_reset(__priv, __args...) \
> -	stmmac_do_callback(__priv, dma, reset, __args)
> +({ \
> +	int __result = -EINVAL; \
> +	if ((__priv) && (__priv)->plat && (__priv)->plat->fix_soc_reset) { \
> +		__result = (__priv)->plat->fix_soc_reset((__priv)->plat,
> ##__args); \
> +	} else { \
> +		__result = stmmac_do_callback(__priv, dma, reset, __args); \
> +	} \
> +	__result; \
> +})
>  #define stmmac_dma_init(__priv, __args...) \
>  	stmmac_do_void_callback(__priv, dma, init, __args)  #define
> stmmac_init_chan(__priv, __args...) \ diff --git a/include/linux/stmmac.h
> b/include/linux/stmmac.h index a152678b82b7..9044477fad61 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -223,6 +223,7 @@ struct plat_stmmacenet_data {
>  	struct stmmac_rxq_cfg rx_queues_cfg[MTL_MAX_RX_QUEUES];
>  	struct stmmac_txq_cfg tx_queues_cfg[MTL_MAX_TX_QUEUES];
>  	void (*fix_mac_speed)(void *priv, unsigned int speed);
> +	int (*fix_soc_reset)(void *priv, void __iomem *ioaddr);
>  	int (*serdes_powerup)(struct net_device *ndev, void *priv);
>  	void (*serdes_powerdown)(struct net_device *ndev, void *priv);
>  	void (*speed_mode_2500)(struct net_device *ndev, void *priv);
> --
> 2.34.1

RE: [PATCH 1/1] net: stmmac: add support for platform specific reset

[Shenwei Wang]

> -----Original Message-----
> From: Shenwei Wang <shenwei.wang@nxp.com>
> Sent: Tuesday, March 21, 2023 2:09 PM
> To: David S. Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Maxime Coquelin <mcoquelin.stm32@gmail.com>;
> Shawn Guo <shawnguo@kernel.org>; dl-linux-imx <linux-imx@nxp.com>
> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>; Alexandre Torgue
> <alexandre.torgue@foss.st.com>; Jose Abreu <joabreu@synopsys.com>; Sascha
> Hauer <s.hauer@pengutronix.de>; Pengutronix Kernel Team
> <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Wong Vee
> Khee <veekhee@apple.com>; Jon Hunter <jonathanh@nvidia.com>;
> Mohammad Athari Bin Ismail <mohammad.athari.ismail@intel.com>; Andrey
> Konovalov <andrey.konovalov@linaro.org>; Revanth Kumar Uppala
> <ruppala@nvidia.com>; Tan Tee Min <tee.min.tan@linux.intel.com>; Shenwei
> Wang <shenwei.wang@nxp.com>; netdev@vger.kernel.org; linux-stm32@st-
> md-mailman.stormreply.com; linux-arm-kernel@lists.infradead.org;
> imx@lists.linux.dev
> Subject: [PATCH 1/1] net: stmmac: add support for platform specific reset
> 
> This patch adds support for platform-specific reset logic in the stmmac driver.
> Some SoCs require a different reset mechanism than the standard dwmac IP
> reset. To support these platforms, a new function pointer 'fix_soc_reset' is
> added to the plat_stmmacenet_data structure.
> The stmmac_reset macro in hwif.h is modified to call the 'fix_soc_reset'
> function if it exists. This enables the driver to use the platform-specific reset
> logic when necessary.
> 

A soft ping. 😊

Thanks,
Shenwei

> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/hwif.h | 10 +++++++++-
>  include/linux/stmmac.h                     |  1 +
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h
> b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> index 16a7421715cb..e24ce870690e 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> @@ -215,7 +215,15 @@ struct stmmac_dma_ops {  };
> 
>  #define stmmac_reset(__priv, __args...) \
> -	stmmac_do_callback(__priv, dma, reset, __args)
> +({ \
> +	int __result = -EINVAL; \
> +	if ((__priv) && (__priv)->plat && (__priv)->plat->fix_soc_reset) { \
> +		__result = (__priv)->plat->fix_soc_reset((__priv)->plat,
> ##__args); \
> +	} else { \
> +		__result = stmmac_do_callback(__priv, dma, reset, __args); \
> +	} \
> +	__result; \
> +})
>  #define stmmac_dma_init(__priv, __args...) \
>  	stmmac_do_void_callback(__priv, dma, init, __args)  #define
> stmmac_init_chan(__priv, __args...) \ diff --git a/include/linux/stmmac.h
> b/include/linux/stmmac.h index a152678b82b7..9044477fad61 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -223,6 +223,7 @@ struct plat_stmmacenet_data {
>  	struct stmmac_rxq_cfg rx_queues_cfg[MTL_MAX_RX_QUEUES];
>  	struct stmmac_txq_cfg tx_queues_cfg[MTL_MAX_TX_QUEUES];
>  	void (*fix_mac_speed)(void *priv, unsigned int speed);
> +	int (*fix_soc_reset)(void *priv, void __iomem *ioaddr);
>  	int (*serdes_powerup)(struct net_device *ndev, void *priv);
>  	void (*serdes_powerdown)(struct net_device *ndev, void *priv);
>  	void (*speed_mode_2500)(struct net_device *ndev, void *priv);
> --
> 2.34.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/1] net: stmmac: add support for platform specific reset

[Jakub Kicinski]

On Thu, 30 Mar 2023 14:55:02 +0000 Shenwei Wang wrote:
> A soft ping. 😊

Simon's proposal is much better.
Just do that instead of pinging the list :|

Re: [PATCH 1/1] net: stmmac: add support for platform specific reset

[Jakub Kicinski]

On Thu, 30 Mar 2023 14:55:02 +0000 Shenwei Wang wrote:
> A soft ping. 😊

Simon's proposal is much better.
Just do that instead of pinging the list :|

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel