RE: [PATCH net-next v3 1/2] net: lan743x: add RMII strap status detection for PCI11x1x

[Thangaraj.S]

Hi Simon,
Thanks for the comments.

> -----Original Message-----
> From: Simon Horman <horms@xxxxxxxxxx>
> Sent: Wednesday, June 10, 2026 9:55 PM
> To: Thangaraj Samynathan - I53494 <Thangaraj.S@xxxxxxxxxxxxx>
> Cc: 'Simon Horman' <horms@xxxxxxxxxx>; netdev@xxxxxxxxxxxxxxx; Bryan
> Whitehead - C21958 <Bryan.Whitehead@xxxxxxxxxxxxx>; UNGLinuxDriver
> <UNGLinuxDriver@xxxxxxxxxxxxx>; andrew+netdev@xxxxxxx;
> davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx;
> pabeni@xxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH net-next v3 1/2] net: lan743x: add RMII strap status
> detection for PCI11x1x
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> From: 'Simon Horman' <horms@xxxxxxxxxx>
> 
> This is an AI-generated review of your patch. The human sending this email
> has considered the AI review valid, or at least plausible.
> Full review at: https://netdev-ai.bots.linux.dev/sashiko/
> ---
> Subject: net: lan743x: add RMII strap status detection for PCI11x1x
> 
> This patch extends pci11x1x_strap_get_status() to read RMII strap bits from
> STRAP_READ and initialise a new is_rmii_en flag on the adapter, updating the
> netif_dbg() to print the selected interface as SGMII, RMII or RGMII.
> 
> > diff --git a/drivers/net/ethernet/microchip/lan743x_main.c
> > b/drivers/net/ethernet/microchip/lan743x_main.c
> > index 1cdce35e1423..0798f3f1f435 100644
> > --- a/drivers/net/ethernet/microchip/lan743x_main.c
> > +++ b/drivers/net/ethernet/microchip/lan743x_main.c
> > @@ -73,8 +74,15 @@ static void pci11x1x_strap_get_status(struct
> lan743x_adapter *adapter)
> >                       adapter->is_sgmii_en = false;
> >               }
> >       }
> > -     netif_dbg(adapter, drv, adapter->netdev,
> > -               "SGMII I/F %sable\n", adapter->is_sgmii_en ? "En" : "Dis");
> > +
> > +     if (!adapter->is_sgmii_en && strap & STRAP_READ_USE_RMII_EN_) {
> > +             if (strap & STRAP_READ_RMII_EN_)
> > +                     adapter->is_rmii_en = true;
> > +     }
> > +
> > +     netif_dbg(adapter, drv, adapter->netdev, "Selected I/F: %s\n",
> > +               adapter->is_sgmii_en ? "SGMII" :
> > +               adapter->is_rmii_en  ? "RMII"  : "RGMII");
> >  }
> 
> [Low]
> This isn't a bug, but does the new "Selected I/F: %s" debug print stay
> consistent with the rest of the driver when is_rmii_en is true?
> 
> In lan743x_mdiobus_init() the non-SGMII branch still prints unconditionally:
> 
>         } else {
>                 dev_dbg(&adapter->pdev->dev, "RGMII operation\n");
>                 // Only C22 support when RGMII I/F
>                 adapter->mdiobus->read = lan743x_mdiobus_read_c22;
>                 adapter->mdiobus->write = lan743x_mdiobus_write_c22;
>                 adapter->mdiobus->name = "lan743x-mdiobus";
> 
> So an RMII-strapped board will report "Selected I/F: RMII" from
> pci11x1x_strap_get_status() and then "RGMII operation" from
> lan743x_mdiobus_init() in the same boot. The C22-only setup is correct for
> both RGMII and RMII, so the only effect is the contradictory dmesg line.
> Should this dev_dbg() also be updated to reflect the RMII case, either in this
> patch or in the follow-up "net: lan743x: add support for RMII interface"?
[Thangaraj Samynathan]  Agreed, I will fix this in next revision.

Thanks,
Thangaraj Samynathan