mbox series

[net-next,v2,0/3] Followup fixes for the dwmac and altera lynx conversion

Message ID 20230606142144.308675-1-maxime.chevallier@bootlin.com (mailing list archive)
Headers show
Series Followup fixes for the dwmac and altera lynx conversion | expand

Message

Maxime Chevallier June 6, 2023, 2:21 p.m. UTC
Following the TSE PCS removal and port of altera_tse and dwmac_socfpga,
this series fixes some issues that slipped through the cracks.

Patch 1 fixes an unitialized struct in altera_tse

Patch 2 uses the correct Kconfig option for altera_tse

Patch 3 makes the Lynx PCS specific to dwmac_socfpga. This patch was
originally written by Russell, my modifications just moves the
#include<linux/pcs-lynx.h> around, to use it only in dwmac_socfpga.

Maxime Chevallier (3):
  net: altera-tse: Initialize the regmap_config struct before using it
  net: altera_tse: Use the correct Kconfig option for the PCS_LYNX
    depenency
  net: stmmac: make the pcs_lynx cleanup sequence specific to
    dwmac_socfpga

 drivers/net/ethernet/altera/Kconfig               |  2 +-
 drivers/net/ethernet/altera/altera_tse_main.c     |  1 +
 drivers/net/ethernet/stmicro/stmmac/common.h      |  1 -
 .../net/ethernet/stmicro/stmmac/dwmac-socfpga.c   | 15 ++++++++++++++-
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c |  3 ---
 5 files changed, 16 insertions(+), 6 deletions(-)

Comments

Russell King (Oracle) June 6, 2023, 2:32 p.m. UTC | #1
On Tue, Jun 06, 2023 at 04:21:41PM +0200, Maxime Chevallier wrote:
> Following the TSE PCS removal and port of altera_tse and dwmac_socfpga,
> this series fixes some issues that slipped through the cracks.
> 
> Patch 1 fixes an unitialized struct in altera_tse
> 
> Patch 2 uses the correct Kconfig option for altera_tse
> 
> Patch 3 makes the Lynx PCS specific to dwmac_socfpga. This patch was
> originally written by Russell, my modifications just moves the
> #include<linux/pcs-lynx.h> around, to use it only in dwmac_socfpga.

Hi Maxime,

I'm sorry, but I think you need an extra patch added to this series.
Looking at include/linux/mdio/mdio-regmap.h, that defines:

struct mdio_regmap_config {
        struct device *parent;
        struct regmap *regmap;
        char name[MII_BUS_ID_SIZE];
        u8 valid_addr;
        bool autoscan;
};

In dwmac-socfpga.c, you have:

                struct mdio_regmap_config mrc;

                mrc.regmap = pcs_regmap;
                mrc.parent = &pdev->dev;
                mrc.valid_addr = 0x0;

                snprintf(mrc.name, MII_BUS_ID_SIZE, "%s-pcs-mii", ndev->name);

So that's a tick for parent, tick for regmap, tick for name, tick
for valid_addr, but... autoscan is left uninitialised.
devm_mdio_regmap_register() reads this, and uses it to decide
how to set mii->phy_mask, which will be randomly ~0 or ~BIT(0)
depending on the value of mrc.autoscan.

Other than that, the series looks good. Thanks.
Maxime Chevallier June 6, 2023, 2:52 p.m. UTC | #2
Hello Russell,

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

> On Tue, Jun 06, 2023 at 04:21:41PM +0200, Maxime Chevallier wrote:
> > Following the TSE PCS removal and port of altera_tse and dwmac_socfpga,
> > this series fixes some issues that slipped through the cracks.
> > 
> > Patch 1 fixes an unitialized struct in altera_tse
> > 
> > Patch 2 uses the correct Kconfig option for altera_tse
> > 
> > Patch 3 makes the Lynx PCS specific to dwmac_socfpga. This patch was
> > originally written by Russell, my modifications just moves the
> > #include<linux/pcs-lynx.h> around, to use it only in dwmac_socfpga.  
> 
> Hi Maxime,
> 
> I'm sorry, but I think you need an extra patch added to this series.

Gosh you're right... The same this also goes for altera_tse... 

> Other than that, the series looks good. Thanks.

I'll followup shortly then. Nice catch !

Thanks,

Maxime