diff mbox series

[net-next,v3,2/5] net: stmmac: introduce pcs_init/pcs_exit stmmac operations

Message ID 20240415-rzn1-gmac1-v3-2-ab12f2c4401d@bootlin.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: stmmac: Add support for RZN1 GMAC devices | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 937 this patch: 937
netdev/build_tools success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 3 maintainers not CCed: horms@kernel.org ahalaney@redhat.com bartosz.golaszewski@linaro.org
netdev/build_clang success Errors and warnings before: 937 this patch: 937
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 949 this patch: 949
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 46 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 38 this patch: 38
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-04-15--15-00 (tests: 961)

Commit Message

Romain Gantois April 15, 2024, 9:18 a.m. UTC 
From: "Russell King (Oracle)" <rmk+kernel@armlinux.org.uk>

Introduce a mechanism whereby platforms can create their PCS instances
prior to the network device being published to userspace, but after
some of the core stmmac initialisation has been completed. This means
that the data structures that platforms need will be available.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 14 ++++++++++++++
 include/linux/stmmac.h                            |  2 ++
 2 files changed, 16 insertions(+)

Comments

Serge Semin April 16, 2024, 1:41 p.m. UTC | #1 
Hi Romain, Russell

On Mon, Apr 15, 2024 at 11:18:42AM +0200, Romain Gantois wrote:
> From: "Russell King (Oracle)" <rmk+kernel@armlinux.org.uk>
> 
> Introduce a mechanism whereby platforms can create their PCS instances
> prior to the network device being published to userspace, but after
> some of the core stmmac initialisation has been completed. This means
> that the data structures that platforms need will be available.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 14 ++++++++++++++
>  include/linux/stmmac.h                            |  2 ++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index fe3498e86de9d..25fa33ae7017b 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -7208,6 +7208,12 @@ static int stmmac_hw_init(struct stmmac_priv *priv)
>  	if (ret)
>  		return ret;
>  
> +	if (priv->plat->pcs_init) {
> +		ret = priv->plat->pcs_init(priv, priv->hw);
> +		if (ret)
> +			return ret;
> +	}
> +

I am currently working on my Memory-mapped DW XPCS patchset cooking:
https://lore.kernel.org/netdev/20231205103559.9605-1-fancer.lancer@gmail.com/
The changes in this series seems to intersect to what is/will be
introduced in my patchset. In particular as before I am going to
use the "pcs-handle" property for getting the XPCS node. If so what
about collecting PCS-related things in a single place. Like this:

int stmmac_xpcs_setup(struct net_device *ndev)
{
	...

	if (priv->plat->pcs_init) {
		return priv->plat->pcs_init(priv); /* Romain' part */
	} else if (fwnode_property_present(priv->plat->port_node, "pcs-handle")) {
		/* My DW XPCS part */
	} else if (priv->plat->mdio_bus_data && priv->plat->mdio_bus_data->has_xpcs) {
		/* Currently implemented procedure */
	}

	...
}

void stmmac_xpcs_clean(struct net_device *ndev)
{
	...

	if (priv->plat->pcs_exit) {
		priv->plat->pcs_exit(priv);
		return;

	}

	xpcs_destroy(priv->hw->xpcs);
	priv->hw->xpcs = NULL;
}

Please see the last two patches in my series:
https://lore.kernel.org/netdev/20231205103559.9605-16-fancer.lancer@gmail.com/
https://lore.kernel.org/netdev/20231205103559.9605-17-fancer.lancer@gmail.com/
as a reference of how the changes could be provided.

-Serge(y)

>  	/* Get the HW capability (new GMAC newer than 3.50a) */
>  	priv->hw_cap_support = stmmac_get_hw_features(priv);
>  	if (priv->hw_cap_support) {
> @@ -7290,6 +7296,12 @@ static int stmmac_hw_init(struct stmmac_priv *priv)
>  	return 0;
>  }
>  
> +static void stmmac_hw_exit(struct stmmac_priv *priv)
> +{
> +	if (priv->plat->pcs_exit)
> +		priv->plat->pcs_exit(priv, priv->hw);
> +}
> +
>  static void stmmac_napi_add(struct net_device *dev)
>  {
>  	struct stmmac_priv *priv = netdev_priv(dev);
> @@ -7804,6 +7816,7 @@ int stmmac_dvr_probe(struct device *device,
>  	    priv->hw->pcs != STMMAC_PCS_RTBI)
>  		stmmac_mdio_unregister(ndev);
>  error_mdio_register:
> +	stmmac_hw_exit(priv);
>  	stmmac_napi_del(ndev);
>  error_hw_init:
>  	destroy_workqueue(priv->wq);
> @@ -7844,6 +7857,7 @@ void stmmac_dvr_remove(struct device *dev)
>  	if (priv->hw->pcs != STMMAC_PCS_TBI &&
>  	    priv->hw->pcs != STMMAC_PCS_RTBI)
>  		stmmac_mdio_unregister(ndev);
> +	stmmac_hw_exit(priv);
>  	destroy_workqueue(priv->wq);
>  	mutex_destroy(&priv->lock);
>  	bitmap_free(priv->af_xdp_zc_qps);
> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> index dfa1828cd756a..941fde506e514 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -285,6 +285,8 @@ struct plat_stmmacenet_data {
>  	int (*crosststamp)(ktime_t *device, struct system_counterval_t *system,
>  			   void *ctx);
>  	void (*dump_debug_regs)(void *priv);
> +	int (*pcs_init)(struct stmmac_priv *priv, struct mac_device_info *hw);
> +	void (*pcs_exit)(struct stmmac_priv *priv, struct mac_device_info *hw);
>  	void *bsp_priv;
>  	struct clk *stmmac_clk;
>  	struct clk *pclk;
> 
> -- 
> 2.44.0
> 
>
Romain Gantois April 17, 2024, 9:30 a.m. UTC | #2 
Hi Serge,

On Tue, 16 Apr 2024, Serge Semin wrote:

> I am currently working on my Memory-mapped DW XPCS patchset cooking:
> https://lore.kernel.org/netdev/20231205103559.9605-1-fancer.lancer@gmail.com/
> The changes in this series seems to intersect to what is/will be
> introduced in my patchset. In particular as before I am going to
> use the "pcs-handle" property for getting the XPCS node. If so what
> about collecting PCS-related things in a single place. Like this:
> 
> int stmmac_xpcs_setup(struct net_device *ndev)
> {
> 	...
> 
> 	if (priv->plat->pcs_init) {
> 		return priv->plat->pcs_init(priv); /* Romain' part */
>	} else if (fwnode_property_present(priv->plat->port_node, "pcs-handle")) {
> 		/* My DW XPCS part */
> 	} else if (priv->plat->mdio_bus_data && priv->plat->mdio_bus_data->has_xpcs) {
> 		/* Currently implemented procedure */
> 	}
> 
> 	...
> }

That seems like a good idea to me, although those setup functions would have to 
be renamed to stmmac_pcs_setup/exit.

Thanks,
Serge Semin April 17, 2024, 1:11 p.m. UTC | #3 
On Wed, Apr 17, 2024 at 11:30:09AM +0200, Romain Gantois wrote:
> Hi Serge,
> 
> On Tue, 16 Apr 2024, Serge Semin wrote:
> 
> > I am currently working on my Memory-mapped DW XPCS patchset cooking:
> > https://lore.kernel.org/netdev/20231205103559.9605-1-fancer.lancer@gmail.com/
> > The changes in this series seems to intersect to what is/will be
> > introduced in my patchset. In particular as before I am going to
> > use the "pcs-handle" property for getting the XPCS node. If so what
> > about collecting PCS-related things in a single place. Like this:
> > 
> > int stmmac_xpcs_setup(struct net_device *ndev)
> > {
> > 	...
> > 
> > 	if (priv->plat->pcs_init) {
> > 		return priv->plat->pcs_init(priv); /* Romain' part */
> >	} else if (fwnode_property_present(priv->plat->port_node, "pcs-handle")) {
> > 		/* My DW XPCS part */
> > 	} else if (priv->plat->mdio_bus_data && priv->plat->mdio_bus_data->has_xpcs) {
> > 		/* Currently implemented procedure */
> > 	}
> > 
> > 	...
> > }
> 
> That seems like a good idea to me, although those setup functions would have to 
> be renamed to stmmac_pcs_setup/exit.

Why not, seeing they will be responsible for any PCS attached to the
MAC.

-Serge(y)

> 
> Thanks,
> 
> -- 
> Romain Gantois, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
diff mbox series

Patch

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index fe3498e86de9d..25fa33ae7017b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -7208,6 +7208,12 @@  static int stmmac_hw_init(struct stmmac_priv *priv)
 	if (ret)
 		return ret;
 
+	if (priv->plat->pcs_init) {
+		ret = priv->plat->pcs_init(priv, priv->hw);
+		if (ret)
+			return ret;
+	}
+
 	/* Get the HW capability (new GMAC newer than 3.50a) */
 	priv->hw_cap_support = stmmac_get_hw_features(priv);
 	if (priv->hw_cap_support) {
@@ -7290,6 +7296,12 @@  static int stmmac_hw_init(struct stmmac_priv *priv)
 	return 0;
 }
 
+static void stmmac_hw_exit(struct stmmac_priv *priv)
+{
+	if (priv->plat->pcs_exit)
+		priv->plat->pcs_exit(priv, priv->hw);
+}
+
 static void stmmac_napi_add(struct net_device *dev)
 {
 	struct stmmac_priv *priv = netdev_priv(dev);
@@ -7804,6 +7816,7 @@  int stmmac_dvr_probe(struct device *device,
 	    priv->hw->pcs != STMMAC_PCS_RTBI)
 		stmmac_mdio_unregister(ndev);
 error_mdio_register:
+	stmmac_hw_exit(priv);
 	stmmac_napi_del(ndev);
 error_hw_init:
 	destroy_workqueue(priv->wq);
@@ -7844,6 +7857,7 @@  void stmmac_dvr_remove(struct device *dev)
 	if (priv->hw->pcs != STMMAC_PCS_TBI &&
 	    priv->hw->pcs != STMMAC_PCS_RTBI)
 		stmmac_mdio_unregister(ndev);
+	stmmac_hw_exit(priv);
 	destroy_workqueue(priv->wq);
 	mutex_destroy(&priv->lock);
 	bitmap_free(priv->af_xdp_zc_qps);
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index dfa1828cd756a..941fde506e514 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -285,6 +285,8 @@  struct plat_stmmacenet_data {
 	int (*crosststamp)(ktime_t *device, struct system_counterval_t *system,
 			   void *ctx);
 	void (*dump_debug_regs)(void *priv);
+	int (*pcs_init)(struct stmmac_priv *priv, struct mac_device_info *hw);
+	void (*pcs_exit)(struct stmmac_priv *priv, struct mac_device_info *hw);
 	void *bsp_priv;
 	struct clk *stmmac_clk;
 	struct clk *pclk;