Re: [PATCH net-next] net: lan969x: correct RGMII port mapping index

[Daniel Machon]

Hi Robert,

> On Mon, Sep 28, 2026 at 11:18 AM Daniel Machon
> <daniel.machon@xxxxxxxxxxxxx> wrote:
> >
> > Hi Robert,
> >
> > > Currently, the lan969x_port_dev_mapping does not check for RGMII ports
> > > and just returns the physical port index.
> > >
> > > However, this does not work for RGMII ports as they have dedicated DEVRGMII
> > > register space with an dedicated instance per RGMII port.
> > >
> > > So, check if requested port index is an RGMII port and return the correct
> > > DEVRGMII index.
> > >
> > > Signed-off-by: Robert Marko <robert.marko@xxxxxxxxxx>
> > > ---
> > >  drivers/net/ethernet/microchip/sparx5/lan969x/lan969x.c | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > >
> > > diff --git a/drivers/net/ethernet/microchip/sparx5/lan969x/lan969x.c b/drivers/net/ethernet/microchip/sparx5/lan969x/lan969x.c
> > > index f3a9c71bea36a..bf5320c7a6ca8 100644
> > > --- a/drivers/net/ethernet/microchip/sparx5/lan969x/lan969x.c
> > > +++ b/drivers/net/ethernet/microchip/sparx5/lan969x/lan969x.c
> > > @@ -142,6 +142,15 @@ static u32 lan969x_get_dev_mode_bit(struct sparx5 *sparx5, int port)
> > >
> > >  static u32 lan969x_port_dev_mapping(struct sparx5 *sparx5, int port)
> > >  {
> > > +       if (lan969x_port_is_rgmii(port)) {
> > > +               switch (port) {
> > > +               case 28:
> > > +                       return 0;
> > > +               case 29:
> > > +                       return 1;
> > > +               }
> > > +       }
> > > +
> > >         if (lan969x_port_is_5g(port)) {
> > >                 switch (port) {
> > >                 case 9:
> > > --
> > > 2.55.0
> > >
> >
> > The mapping itself is right, but as far as I can see nothing upstream
> > uses the returned index to access DEVRGMII. Every caller of
> > sparx5_port_dev_index() pairs it with sparx5_to_high_dev() or
> > sparx5_to_pcs_dev(), which never return TARGET_DEVRGMII. Those paths
> > are also never taken for RGMII ports. The code that does access
> > DEVRGMII, in lan969x_rgmii.c, computes its own index with
> > RGMII_PORT_IDX(). Sashiko, correctly points this out.
> >
> > So I don't think anything is broken today. Did you see a problem on
> > hardware that this fixes?
> >
> > FYI, we carry the same change downstream. But there it was added together with
> > RGMII MTU support, which reads and writes DEVRGMII_MAC_MAXLEN_CFG() using this
> > index.
> 
> Hi Andrew,
> As pointed out nothing yet is broken, hence no fixes tag.
> 
> I am using it downstream as well, for the MTU change support where as
> you pointed out RGMII code
> uses it to get the correct index.
> 
> So, I thought that it would be good idea to send it upstream before
> eventually getting around to sending
> the MTU change support as well.

It wasn't clear to me that this was preparation work for a future feature. As a
standlone patch it does nothing. IDK, to me it would make more sense to just
send it together with the patchset that would actually use it.

/Daniel

> 
> Regards,
> Robert
> 
> >
> > /Daniel
> 
> 
> 
> --
> Robert Marko
> Staff Embedded Linux Engineer
> Sartura d.d.
> Lendavska ulica 16a
> 10000 Zagreb, Croatia
> Email: robert.marko@xxxxxxxxxx
> Web: www.sartura.hr