| Message ID | 20230607135941.407054-6-maxime.chevallier@bootlin.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 06b9dede1e7d8b7a199f8014aca5a7d7137b41b0 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Followup fixes for the dwmac and altera lynx conversion | expand |
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() ?
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.
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 03:59:41PM +0200, Maxime Chevallier wrote:
> Explicitely zero-ize the local mdio_regmap_config data, and explicitely
nit: Explicitely -> explicitly
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);
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(-)