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

[Robert Marko]

On Mon, Sep 28, 2026 at 1:04 PM Daniel Machon
<daniel.machon@xxxxxxxxxxxxx> wrote:
>
> 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.

That is fine by me.

Regards,
Robert

>
> /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



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