From: "Russell King (Oracle)" <[email protected]>
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) <[email protected]>
Reviewed-by: Maxime Chevallier <[email protected]>
Signed-off-by: Romain Gantois <[email protected]>
---
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;
+ }
+
/* 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
Hi Romain, Russell
On Mon, Apr 15, 2024 at 11:18:42AM +0200, Romain Gantois wrote:
> From: "Russell King (Oracle)" <[email protected]>
>
> 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) <[email protected]>
> Reviewed-by: Maxime Chevallier <[email protected]>
> Signed-off-by: Romain Gantois <[email protected]>
> ---
> 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/[email protected]/
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/[email protected]/
https://lore.kernel.org/netdev/[email protected]/
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
>
>
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/[email protected]/
> 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,
--
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
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/[email protected]/
> > 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