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

[Thangaraj.S]

Hi Andrew,
Thanks for the comments.

> -----Original Message-----
> From: Andrew Lunn <andrew@xxxxxxx>
> Sent: Tuesday, May 26, 2026 10:10 PM
> To: Thangaraj Samynathan - I53494 <Thangaraj.S@xxxxxxxxxxxxx>
> Cc: netdev@xxxxxxxxxxxxxxx; andrew+netdev@xxxxxxx;
> davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx;
> pabeni@xxxxxxxxxx; Bryan Whitehead - C21958
> <Bryan.Whitehead@xxxxxxxxxxxxx>; UNGLinuxDriver
> <UNGLinuxDriver@xxxxxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH net-next v2 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
> 
> > +     if (!adapter->is_sgmii_en && strap & STRAP_READ_USE_RMII_EN_) {
> > +             if (strap & STRAP_READ_RMII_EN_)
> > +                     adapter->is_rmii_en = true;
> > +     }
> 
> What does it mean when both SGMII and RMII is strapped? What if neither
> is strapped?
[Thangaraj Samynathan] When both are enabled, Interface will SGMII. When
Both are disabled interface will be RGMII.
> 
> > +
> >       netif_dbg(adapter, drv, adapter->netdev,
> >                 "SGMII I/F %sable\n", adapter->is_sgmii_en ? "En" :
> > "Dis");
> 
> Since there is a netif_dbg() for sgmii, maybe add one for rmii, so the code is
> somewhat symmetrical?
[Thangaraj Samynathan] Sure. Will add it in the next revision. 
> 
> >       struct lan743x_rx       rx[LAN743X_USED_RX_CHANNELS];
> >       bool                    is_pci11x1x;
> >       bool                    is_sgmii_en;
> > +     bool                    is_rmii_en;
> 
> Are there other strapping bits? Are we going to get is_gmii_en, and
> is_rgmii_en?
> 
> Does a collection of bools make sense, when the interface modes are
> typically mutually exclusive at the hardware level? Would an enum be
> better? Or why not phy_interface_t ?
[Thangaraj Samynathan] No, we have only three strap bits for SGMII and RMII interfaces. 
> 
>         Andrew