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

[Daniel Machon <daniel.machon@microchip.com>]

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@sartura.hr>
> ---
>  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.

/Daniel