[PATCH net-next 0/2] Followup fixes for the dwmac and altera lynx conversion

[PATCH net-next 0/2] Followup fixes for the dwmac and altera lynx conversion

[Maxime Chevallier]

This series is a couple of fixes for errors that were detected following
the conversion of altera TSE and dwmac_socfpga to PCS Lynx.

These are targeted towards net-next as the patches they fix are also in
net-next.

Sorry about the bugs,

Maxime

Maxime Chevallier (2):
  net: stmmac: Add PCS_LYNX as a dependency for the whole driver
  net: altera-tse: Initialize the regmap_config struct before using it

 drivers/net/ethernet/altera/altera_tse_main.c | 1 +
 drivers/net/ethernet/stmicro/stmmac/Kconfig   | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

-- 
2.40.1

[PATCH net-next 1/2] net: stmmac: Add PCS_LYNX as a dependency for the whole driver

[Maxime Chevallier]

Although pcs_lynx is only used on dwmac_socfpga for now, the cleanup
path is in the generic driver, and triggers build issues for other
stmmac variants. Make sure we build pcs_lynx in all cases too, as for
XPCS.

Fixes: 5d1f3fe7d2d5 ("net: stmmac: dwmac-sogfpga: use the lynx pcs driver")
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/ethernet/stmicro/stmmac/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 5583f0b055ec..fa956f2081a5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -5,6 +5,7 @@ config STMMAC_ETH
 	depends on PTP_1588_CLOCK_OPTIONAL
 	select MII
 	select PCS_XPCS
+	select PCS_LYNX
 	select PAGE_POOL
 	select PHYLINK
 	select CRC32
@@ -160,7 +161,6 @@ config DWMAC_SOCFPGA
 	select MFD_SYSCON
 	select MDIO_REGMAP
 	select REGMAP_MMIO
-	select PCS_LYNX
 	help
 	  Support for ethernet controller on Altera SOCFPGA
 
-- 
2.40.1

[PATCH net-next 2/2] net: altera-tse: Initialize the regmap_config struct before using it

[Maxime Chevallier]

The regmap_config needs to be zeroed before using it. This will cause
spurious errors at probe time as config->pad_bits is containing random
uninitialized data.

Fixes: db48abbaa18e ("net: ethernet: altera-tse: Convert to mdio-regmap and use PCS Lynx")
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/ethernet/altera/altera_tse_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/altera/altera_tse_main.c b/drivers/net/ethernet/altera/altera_tse_main.c
index d866c0f1b503..df509abcd378 100644
--- a/drivers/net/ethernet/altera/altera_tse_main.c
+++ b/drivers/net/ethernet/altera/altera_tse_main.c
@@ -1255,6 +1255,7 @@ static int altera_tse_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_free_netdev;
 
+	memset(&pcs_regmap_cfg, 0, sizeof(pcs_regmap_cfg));
 	/* SGMII PCS address space. The location can vary depending on how the
 	 * IP is integrated. We can have a resource dedicated to it at a specific
 	 * address space, but if it's not the case, we fallback to the mdiophy0
-- 
2.40.1

Re: [PATCH net-next 1/2] net: stmmac: Add PCS_LYNX as a dependency for the whole driver

[Geert Uytterhoeven]

 	Hi Maxime,

On Tue, 6 Jun 2023, Maxime Chevallier wrote:
> Although pcs_lynx is only used on dwmac_socfpga for now, the cleanup
> path is in the generic driver, and triggers build issues for other
> stmmac variants. Make sure we build pcs_lynx in all cases too, as for
> XPCS.

That seems suboptimal to me, as it needlesly increases kernel size for
people who do not use dwmac_socfpga.  Hence I made an alternative patch:
https://lore.kernel/org/7b36ac43778b41831debd5c30b5b37d268512195.1686039915.git.geert+renesas@glider.be

> Fixes: 5d1f3fe7d2d5 ("net: stmmac: dwmac-sogfpga: use the lynx pcs driver")
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> index 5583f0b055ec..fa956f2081a5 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> @@ -5,6 +5,7 @@ config STMMAC_ETH
> 	depends on PTP_1588_CLOCK_OPTIONAL
> 	select MII
> 	select PCS_XPCS
> +	select PCS_LYNX
> 	select PAGE_POOL
> 	select PHYLINK
> 	select CRC32
> @@ -160,7 +161,6 @@ config DWMAC_SOCFPGA
> 	select MFD_SYSCON
> 	select MDIO_REGMAP
> 	select REGMAP_MMIO
> -	select PCS_LYNX
> 	help
> 	  Support for ethernet controller on Altera SOCFPGA

Gr{oetje,eeting}s,

 						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
 							    -- Linus Torvalds

Re: [PATCH net-next 1/2] net: stmmac: Add PCS_LYNX as a dependency for the whole driver

[Russell King (Oracle)]

On Tue, Jun 06, 2023 at 10:29:20AM +0200, Geert Uytterhoeven wrote:
> 	Hi Maxime,
> 
> On Tue, 6 Jun 2023, Maxime Chevallier wrote:
> > Although pcs_lynx is only used on dwmac_socfpga for now, the cleanup
> > path is in the generic driver, and triggers build issues for other
> > stmmac variants. Make sure we build pcs_lynx in all cases too, as for
> > XPCS.
> 
> That seems suboptimal to me, as it needlesly increases kernel size for
> people who do not use dwmac_socfpga.  Hence I made an alternative patch:
> https://lore.kernel/org/7b36ac43778b41831debd5c30b5b37d268512195.1686039915.git.geert+renesas@glider.be

A better solution would be to re-architect the removal code so that
whatever creates the PCS is also responsible for removing it.

Also, dwmac_socfpga nees to be reorganised anyway, because it calls
stmmac_dvr_probe() which then goes on to call register_netdev(),
publishing the network device, and then after stmmac_dvr_probe(),
further device setup is done. As the basic driver probe flow should
be setup and then publish, the existing code structure violates that.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next 1/2] net: stmmac: Add PCS_LYNX as a dependency for the whole driver

[Maxime Chevallier]

Hello Geert, Russell,

On Tue, 6 Jun 2023 10:30:32 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Tue, Jun 06, 2023 at 10:29:20AM +0200, Geert Uytterhoeven wrote:
> > 	Hi Maxime,
> > 
> > On Tue, 6 Jun 2023, Maxime Chevallier wrote:  
> > > Although pcs_lynx is only used on dwmac_socfpga for now, the cleanup
> > > path is in the generic driver, and triggers build issues for other
> > > stmmac variants. Make sure we build pcs_lynx in all cases too, as for
> > > XPCS.  
> > 
> > That seems suboptimal to me, as it needlesly increases kernel size for
> > people who do not use dwmac_socfpga.  Hence I made an alternative patch:
> > https://lore.kernel/org/7b36ac43778b41831debd5c30b5b37d268512195.1686039915.git.geert+renesas@glider.be  
> 
> A better solution would be to re-architect the removal code so that
> whatever creates the PCS is also responsible for removing it.
> 
> Also, dwmac_socfpga nees to be reorganised anyway, because it calls
> stmmac_dvr_probe() which then goes on to call register_netdev(),
> publishing the network device, and then after stmmac_dvr_probe(),
> further device setup is done. As the basic driver probe flow should
> be setup and then publish, the existing code structure violates that.
> 

I agree that this solution is definitely suboptimal, I wanted mostly to get it
fixed quickly as this breaks other stmmac variants.

Do we still go on with the current patch (as Geert's has issues) and then
consider reworking dwmac_socfpga ?

Maxime

Re: [PATCH net-next 1/2] net: stmmac: Add PCS_LYNX as a dependency for the whole driver

[Russell King (Oracle)]

On Tue, Jun 06, 2023 at 12:13:11PM +0200, Maxime Chevallier wrote:
> Hello Geert, Russell,
> 
> On Tue, 6 Jun 2023 10:30:32 +0100
> "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> 
> > On Tue, Jun 06, 2023 at 10:29:20AM +0200, Geert Uytterhoeven wrote:
> > > 	Hi Maxime,
> > > 
> > > On Tue, 6 Jun 2023, Maxime Chevallier wrote:  
> > > > Although pcs_lynx is only used on dwmac_socfpga for now, the cleanup
> > > > path is in the generic driver, and triggers build issues for other
> > > > stmmac variants. Make sure we build pcs_lynx in all cases too, as for
> > > > XPCS.  
> > > 
> > > That seems suboptimal to me, as it needlesly increases kernel size for
> > > people who do not use dwmac_socfpga.  Hence I made an alternative patch:
> > > https://lore.kernel/org/7b36ac43778b41831debd5c30b5b37d268512195.1686039915.git.geert+renesas@glider.be  
> > 
> > A better solution would be to re-architect the removal code so that
> > whatever creates the PCS is also responsible for removing it.
> > 
> > Also, dwmac_socfpga nees to be reorganised anyway, because it calls
> > stmmac_dvr_probe() which then goes on to call register_netdev(),
> > publishing the network device, and then after stmmac_dvr_probe(),
> > further device setup is done. As the basic driver probe flow should
> > be setup and then publish, the existing code structure violates that.
> > 
> 
> I agree that this solution is definitely suboptimal, I wanted mostly to get it
> fixed quickly as this breaks other stmmac variants.
> 
> Do we still go on with the current patch (as Geert's has issues) and then
> consider reworking dwmac_socfpga ?

As Geert himself mentioned, passed on from Arnd:
  As pointed out by Arnd, this doesn't work when PCS_LYNX is a loadable
  module and STMMAC is built-in:
  https://lore.kernel.org/r/11bd37e9-c62e-46ba-9456-8e3b353df28f@app.fastmail.com

So Geert's solution will just get rid of the build error, but leave the
Lynx PCS undestroyed. I take Geert's comment as a self-nack on his
proposed patch.

The changes are only in net-next at the moment, and we're at -rc5.
There's probably about 2.5 weeks to get this sorted before the merge
window opens.

So, we currently have your suggestion. Here's mine as an immediate
fix. This doesn't address all the points I've raised, but should
resolve the immediate issue.

Untested since I don't have the hardware... (the test build is
running):

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index e399fccbafe5..239c7e9ed41d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -494,6 +494,17 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 	return ret;
 }
 
+static void socfpga_dwmac_remove(struct platform_device *pdev)
+{
+	struct net_device *ndev = platform_get_drvdata(pdev);
+	struct stmmac_priv *priv = netdev_priv(ndev);
+	struct phylink_pcs *pcs = priv->hw->lynx_pcs;
+
+	stmmac_pltfr_remove(pdev);
+
+	lynx_pcs_destroy(pcs);
+}
+
 #ifdef CONFIG_PM_SLEEP
 static int socfpga_dwmac_resume(struct device *dev)
 {
@@ -565,7 +576,7 @@ MODULE_DEVICE_TABLE(of, socfpga_dwmac_match);
 
 static struct platform_driver socfpga_dwmac_driver = {
 	.probe  = socfpga_dwmac_probe,
-	.remove_new = stmmac_pltfr_remove,
+	.remove_new = socfpga_dwmac_remove,
 	.driver = {
 		.name           = "socfpga-dwmac",
 		.pm		= &socfpga_dwmac_pm_ops,
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index fa07b0d50b46..1801f8cc8413 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -940,9 +940,6 @@ static struct phylink_pcs *stmmac_mac_select_pcs(struct phylink_config *config,
 	if (priv->hw->xpcs)
 		return &priv->hw->xpcs->pcs;
 
-	if (priv->hw->lynx_pcs)
-		return priv->hw->lynx_pcs;
-
 	return NULL;
 }
 
-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next 1/2] net: stmmac: Add PCS_LYNX as a dependency for the whole driver

[Russell King (Oracle)]

On Tue, Jun 06, 2023 at 11:26:12AM +0100, Russell King (Oracle) wrote:
> On Tue, Jun 06, 2023 at 12:13:11PM +0200, Maxime Chevallier wrote:
> > Hello Geert, Russell,
> > 
> > On Tue, 6 Jun 2023 10:30:32 +0100
> > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > 
> > > On Tue, Jun 06, 2023 at 10:29:20AM +0200, Geert Uytterhoeven wrote:
> > > > 	Hi Maxime,
> > > > 
> > > > On Tue, 6 Jun 2023, Maxime Chevallier wrote:  
> > > > > Although pcs_lynx is only used on dwmac_socfpga for now, the cleanup
> > > > > path is in the generic driver, and triggers build issues for other
> > > > > stmmac variants. Make sure we build pcs_lynx in all cases too, as for
> > > > > XPCS.  
> > > > 
> > > > That seems suboptimal to me, as it needlesly increases kernel size for
> > > > people who do not use dwmac_socfpga.  Hence I made an alternative patch:
> > > > https://lore.kernel/org/7b36ac43778b41831debd5c30b5b37d268512195.1686039915.git.geert+renesas@glider.be  
> > > 
> > > A better solution would be to re-architect the removal code so that
> > > whatever creates the PCS is also responsible for removing it.
> > > 
> > > Also, dwmac_socfpga nees to be reorganised anyway, because it calls
> > > stmmac_dvr_probe() which then goes on to call register_netdev(),
> > > publishing the network device, and then after stmmac_dvr_probe(),
> > > further device setup is done. As the basic driver probe flow should
> > > be setup and then publish, the existing code structure violates that.
> > > 
> > 
> > I agree that this solution is definitely suboptimal, I wanted mostly to get it
> > fixed quickly as this breaks other stmmac variants.
> > 
> > Do we still go on with the current patch (as Geert's has issues) and then
> > consider reworking dwmac_socfpga ?
> 
> As Geert himself mentioned, passed on from Arnd:
>   As pointed out by Arnd, this doesn't work when PCS_LYNX is a loadable
>   module and STMMAC is built-in:
>   https://lore.kernel.org/r/11bd37e9-c62e-46ba-9456-8e3b353df28f@app.fastmail.com
> 
> So Geert's solution will just get rid of the build error, but leave the
> Lynx PCS undestroyed. I take Geert's comment as a self-nack on his
> proposed patch.
> 
> The changes are only in net-next at the moment, and we're at -rc5.
> There's probably about 2.5 weeks to get this sorted before the merge
> window opens.
> 
> So, we currently have your suggestion. Here's mine as an immediate
> fix. This doesn't address all the points I've raised, but should
> resolve the immediate issue.
> 
> Untested since I don't have the hardware... (the test build is
> running):
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> index e399fccbafe5..239c7e9ed41d 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> @@ -494,6 +494,17 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
>  	return ret;
>  }
>  
> +static void socfpga_dwmac_remove(struct platform_device *pdev)
> +{
> +	struct net_device *ndev = platform_get_drvdata(pdev);
> +	struct stmmac_priv *priv = netdev_priv(ndev);
> +	struct phylink_pcs *pcs = priv->hw->lynx_pcs;
> +
> +	stmmac_pltfr_remove(pdev);
> +
> +	lynx_pcs_destroy(pcs);
> +}
> +
>  #ifdef CONFIG_PM_SLEEP
>  static int socfpga_dwmac_resume(struct device *dev)
>  {
> @@ -565,7 +576,7 @@ MODULE_DEVICE_TABLE(of, socfpga_dwmac_match);
>  
>  static struct platform_driver socfpga_dwmac_driver = {
>  	.probe  = socfpga_dwmac_probe,
> -	.remove_new = stmmac_pltfr_remove,
> +	.remove_new = socfpga_dwmac_remove,
>  	.driver = {
>  		.name           = "socfpga-dwmac",
>  		.pm		= &socfpga_dwmac_pm_ops,
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index fa07b0d50b46..1801f8cc8413 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -940,9 +940,6 @@ static struct phylink_pcs *stmmac_mac_select_pcs(struct phylink_config *config,
>  	if (priv->hw->xpcs)
>  		return &priv->hw->xpcs->pcs;
>  
> -	if (priv->hw->lynx_pcs)
> -		return priv->hw->lynx_pcs;
> -

This hunk is completely wrong... but I guess you spotted that anyway.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next 1/2] net: stmmac: Add PCS_LYNX as a dependency for the whole driver

[Geert Uytterhoeven]

Hi Russell,

On Tue, Jun 6, 2023 at 12:26 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
> On Tue, Jun 06, 2023 at 12:13:11PM +0200, Maxime Chevallier wrote:
> > On Tue, 6 Jun 2023 10:30:32 +0100
> > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> > > On Tue, Jun 06, 2023 at 10:29:20AM +0200, Geert Uytterhoeven wrote:
> > > > On Tue, 6 Jun 2023, Maxime Chevallier wrote:
> > > > > Although pcs_lynx is only used on dwmac_socfpga for now, the cleanup
> > > > > path is in the generic driver, and triggers build issues for other
> > > > > stmmac variants. Make sure we build pcs_lynx in all cases too, as for
> > > > > XPCS.
> > > >
> > > > That seems suboptimal to me, as it needlesly increases kernel size for
> > > > people who do not use dwmac_socfpga.  Hence I made an alternative patch:
> > > > https://lore.kernel/org/7b36ac43778b41831debd5c30b5b37d268512195.1686039915.git.geert+renesas@glider.be
> > >
> > > A better solution would be to re-architect the removal code so that
> > > whatever creates the PCS is also responsible for removing it.
> > >
> > > Also, dwmac_socfpga nees to be reorganised anyway, because it calls
> > > stmmac_dvr_probe() which then goes on to call register_netdev(),
> > > publishing the network device, and then after stmmac_dvr_probe(),
> > > further device setup is done. As the basic driver probe flow should
> > > be setup and then publish, the existing code structure violates that.
> > >
> >
> > I agree that this solution is definitely suboptimal, I wanted mostly to get it
> > fixed quickly as this breaks other stmmac variants.
> >
> > Do we still go on with the current patch (as Geert's has issues) and then
> > consider reworking dwmac_socfpga ?
>
> As Geert himself mentioned, passed on from Arnd:
>   As pointed out by Arnd, this doesn't work when PCS_LYNX is a loadable
>   module and STMMAC is built-in:
>   https://lore.kernel.org/r/11bd37e9-c62e-46ba-9456-8e3b353df28f@app.fastmail.com
>
> So Geert's solution will just get rid of the build error, but leave the
> Lynx PCS undestroyed. I take Geert's comment as a self-nack on his
> proposed patch.

Exactly...

> The changes are only in net-next at the moment, and we're at -rc5.
> There's probably about 2.5 weeks to get this sorted before the merge
> window opens.
>
> So, we currently have your suggestion. Here's mine as an immediate
> fix. This doesn't address all the points I've raised, but should
> resolve the immediate issue.
>
> Untested since I don't have the hardware... (the test build is
> running):
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> index e399fccbafe5..239c7e9ed41d 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> @@ -494,6 +494,17 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
>         return ret;
>  }
>
> +static void socfpga_dwmac_remove(struct platform_device *pdev)
> +{
> +       struct net_device *ndev = platform_get_drvdata(pdev);
> +       struct stmmac_priv *priv = netdev_priv(ndev);
> +       struct phylink_pcs *pcs = priv->hw->lynx_pcs;
> +
> +       stmmac_pltfr_remove(pdev);
> +
> +       lynx_pcs_destroy(pcs);
> +}
> +
>  #ifdef CONFIG_PM_SLEEP
>  static int socfpga_dwmac_resume(struct device *dev)
>  {
> @@ -565,7 +576,7 @@ MODULE_DEVICE_TABLE(of, socfpga_dwmac_match);
>
>  static struct platform_driver socfpga_dwmac_driver = {
>         .probe  = socfpga_dwmac_probe,
> -       .remove_new = stmmac_pltfr_remove,
> +       .remove_new = socfpga_dwmac_remove,
>         .driver = {
>                 .name           = "socfpga-dwmac",
>                 .pm             = &socfpga_dwmac_pm_ops,
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index fa07b0d50b46..1801f8cc8413 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -940,9 +940,6 @@ static struct phylink_pcs *stmmac_mac_select_pcs(struct phylink_config *config,
>         if (priv->hw->xpcs)
>                 return &priv->hw->xpcs->pcs;
>
> -       if (priv->hw->lynx_pcs)
> -               return priv->hw->lynx_pcs;
> -
>         return NULL;
>  }

I think the above hunk is wrong, and should be replaced by a removal
of the call to lynx_pcs_destroy()?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH net-next 1/2] net: stmmac: Add PCS_LYNX as a dependency for the whole driver

[Russell King (Oracle)]

On Tue, Jun 06, 2023 at 11:34:44AM +0100, Russell King (Oracle) wrote:
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index fa07b0d50b46..1801f8cc8413 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -940,9 +940,6 @@ static struct phylink_pcs *stmmac_mac_select_pcs(struct phylink_config *config,
> >  	if (priv->hw->xpcs)
> >  		return &priv->hw->xpcs->pcs;
> >  
> > -	if (priv->hw->lynx_pcs)
> > -		return priv->hw->lynx_pcs;
> > -
> 
> This hunk is completely wrong... but I guess you spotted that anyway.

Replacement...

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index e399fccbafe5..239c7e9ed41d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -494,6 +494,17 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 	return ret;
 }
 
+static void socfpga_dwmac_remove(struct platform_device *pdev)
+{
+	struct net_device *ndev = platform_get_drvdata(pdev);
+	struct stmmac_priv *priv = netdev_priv(ndev);
+	struct phylink_pcs *pcs = priv->hw->lynx_pcs;
+
+	stmmac_pltfr_remove(pdev);
+
+	lynx_pcs_destroy(pcs);
+}
+
 #ifdef CONFIG_PM_SLEEP
 static int socfpga_dwmac_resume(struct device *dev)
 {
@@ -565,7 +576,7 @@ MODULE_DEVICE_TABLE(of, socfpga_dwmac_match);
 
 static struct platform_driver socfpga_dwmac_driver = {
 	.probe  = socfpga_dwmac_probe,
-	.remove_new = stmmac_pltfr_remove,
+	.remove_new = socfpga_dwmac_remove,
 	.driver = {
 		.name           = "socfpga-dwmac",
 		.pm		= &socfpga_dwmac_pm_ops,
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index c784a6731f08..3db1cb0fd160 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -665,9 +665,6 @@ int stmmac_mdio_unregister(struct net_device *ndev)
 	if (priv->hw->xpcs)
 		xpcs_destroy(priv->hw->xpcs);
 
-	if (priv->hw->lynx_pcs)
-		lynx_pcs_destroy(priv->hw->lynx_pcs);
-
 	mdiobus_unregister(priv->mii);
 	priv->mii->priv = NULL;
 	mdiobus_free(priv->mii);

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next 1/2] net: stmmac: Add PCS_LYNX as a dependency for the whole driver

[Russell King (Oracle)]

On Tue, Jun 06, 2023 at 12:35:23PM +0200, Geert Uytterhoeven wrote:
> Hi Russell,
> 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index fa07b0d50b46..1801f8cc8413 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -940,9 +940,6 @@ static struct phylink_pcs *stmmac_mac_select_pcs(struct phylink_config *config,
> >         if (priv->hw->xpcs)
> >                 return &priv->hw->xpcs->pcs;
> >
> > -       if (priv->hw->lynx_pcs)
> > -               return priv->hw->lynx_pcs;
> > -
> >         return NULL;
> >  }
> 
> I think the above hunk is wrong, and should be replaced by a removal
> of the call to lynx_pcs_destroy()?

Indeed, and wrong file too. Thanks for spotting, I think we spotted
the mistake at almost the same time. Replacement patch sent.

It'd be good to have the patch thoroughly reviewed to make sure I
haven't missed anything else, bearing in mind that I don't know this
driver inside out and don't have the hardware.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next 1/2] net: stmmac: Add PCS_LYNX as a dependency for the whole driver

[Maxime Chevallier]

On Tue, 6 Jun 2023 11:44:08 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Tue, Jun 06, 2023 at 12:35:23PM +0200, Geert Uytterhoeven wrote:
> > Hi Russell,
> >   
> > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > index fa07b0d50b46..1801f8cc8413 100644
> > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > @@ -940,9 +940,6 @@ static struct phylink_pcs *stmmac_mac_select_pcs(struct phylink_config *config,
> > >         if (priv->hw->xpcs)
> > >                 return &priv->hw->xpcs->pcs;
> > >
> > > -       if (priv->hw->lynx_pcs)
> > > -               return priv->hw->lynx_pcs;
> > > -
> > >         return NULL;
> > >  }  
> > 
> > I think the above hunk is wrong, and should be replaced by a removal
> > of the call to lynx_pcs_destroy()?  
> 
> Indeed, and wrong file too. Thanks for spotting, I think we spotted
> the mistake at almost the same time. Replacement patch sent.
> 
> It'd be good to have the patch thoroughly reviewed to make sure I
> haven't missed anything else, bearing in mind that I don't know this
> driver inside out and don't have the hardware.

I will give it a try righ-away,

Thanks,

Maxime

> Thanks.
>