Explicitely zero-ize the local mdio_regmap_config data, and explicitely
set the .autoscan parameter, as we only have a PCS on this bus.

Fixes: 5d1f3fe7d2d5 ("net: stmmac: dwmac-sogfpga: use the lynx pcs driver")
Suggested-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V3->V4 : Move pcs_regmap_cfg into a more local block, and zeroize mrc
V2->V3 : New patch

 .../net/ethernet/stmicro/stmmac/dwmac-socfpga.c   | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index 1fb808be843b..6267bcb60206 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -389,7 +389,6 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 	struct net_device	*ndev;
 	struct stmmac_priv	*stpriv;
 	const struct socfpga_dwmac_ops *ops;
-	struct regmap_config pcs_regmap_cfg;
 
 	ops = device_get_match_data(&pdev->dev);
 	if (!ops) {
@@ -447,19 +446,22 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_dvr_remove;
 
-	memset(&pcs_regmap_cfg, 0, sizeof(pcs_regmap_cfg));
-	pcs_regmap_cfg.reg_bits = 16;
-	pcs_regmap_cfg.val_bits = 16;
-	pcs_regmap_cfg.reg_shift = REGMAP_UPSHIFT(1);
-
 	/* Create a regmap for the PCS so that it can be used by the PCS driver,
 	 * if we have such a PCS
 	 */
 	if (dwmac->tse_pcs_base) {
+		struct regmap_config pcs_regmap_cfg;
 		struct mdio_regmap_config mrc;
 		struct regmap *pcs_regmap;
 		struct mii_bus *pcs_bus;
 
+		memset(&pcs_regmap_cfg, 0, sizeof(pcs_regmap_cfg));
+		memset(&mrc, 0, sizeof(mrc));
+
+		pcs_regmap_cfg.reg_bits = 16;
+		pcs_regmap_cfg.val_bits = 16;
+		pcs_regmap_cfg.reg_shift = REGMAP_UPSHIFT(1);
+
 		pcs_regmap = devm_regmap_init_mmio(&pdev->dev, dwmac->tse_pcs_base,
 						   &pcs_regmap_cfg);
 		if (IS_ERR(pcs_regmap)) {
@@ -470,6 +472,7 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 		mrc.regmap = pcs_regmap;
 		mrc.parent = &pdev->dev;
 		mrc.valid_addr = 0x0;
+		mrc.autoscan = false;
 
 		snprintf(mrc.name, MII_BUS_ID_SIZE, "%s-pcs-mii", ndev->name);
 		pcs_bus = devm_mdio_regmap_register(&pdev->dev, &mrc);
-- 
2.40.1

On Wed, Jun 07, 2023 at 03:59:41PM +0200, Maxime Chevallier wrote:
> Explicitely zero-ize the local mdio_regmap_config data, and explicitely

nit: Explicitely -> explicitly

On Wed, Jun 07, 2023 at 03:59:41PM +0200, Maxime Chevallier wrote:
> @@ -447,19 +446,22 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
>  		struct mdio_regmap_config mrc;
>  		struct regmap *pcs_regmap;
>  		struct mii_bus *pcs_bus;
>  
...
> +		memset(&mrc, 0, sizeof(mrc));
...
>  		mrc.parent = &pdev->dev;
>  		mrc.valid_addr = 0x0;
> +		mrc.autoscan = false;

Isn't this covered by the memset() ?

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

On Wed, 7 Jun 2023 13:28:03 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Wed, Jun 07, 2023 at 03:59:41PM +0200, Maxime Chevallier wrote:
> > @@ -447,19 +446,22 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
> >  		struct mdio_regmap_config mrc;
> >  		struct regmap *pcs_regmap;
> >  		struct mii_bus *pcs_bus;
> >    
> ...
> > +		memset(&mrc, 0, sizeof(mrc));  
> ...
> >  		mrc.parent = &pdev->dev;
> >  		mrc.valid_addr = 0x0;
> > +		mrc.autoscan = false;  
> 
> Isn't this covered by the memset() ?

I have the same answer as for the above. It's redundant, but I don't
think there's any harm having it set explicitely ?

Maxime

On Wed, Jun 07, 2023 at 04:54:09PM +0200, Maxime Chevallier wrote:
> On Wed, 7 Jun 2023 13:28:03 +0100
> "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> 
> > On Wed, Jun 07, 2023 at 03:59:41PM +0200, Maxime Chevallier wrote:
> > > @@ -447,19 +446,22 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
> > >  		struct mdio_regmap_config mrc;
> > >  		struct regmap *pcs_regmap;
> > >  		struct mii_bus *pcs_bus;
> > >    
> > ...
> > > +		memset(&mrc, 0, sizeof(mrc));  
> > ...
> > >  		mrc.parent = &pdev->dev;
> > >  		mrc.valid_addr = 0x0;
> > > +		mrc.autoscan = false;  
> > 
> > Isn't this covered by the memset() ?
> 
> I have the same answer as for the above. It's redundant, but I don't
> think there's any harm having it set explicitely ?

No harm, just redundant. I don't think this is a good enough reason not
to merge it.

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