Re: [PATCH net-next v11 00/12] net: pcs: Introduce support for fwnode PCS

[Andrew Lunn <andrew@lunn.ch>]

> I think the most annoying one will be stmmac with the 3 different PCS
> implementation but we already handled that in a upcoming ipq50xx driver
> currently present in OpenWrt mainline.

stmmac is always annoying :-(

If this can make it more uniform, make internal and external PCS look
the same, that would be great.


> @@ -6808,7 +6792,10 @@ static void mvpp2_acpi_start(struct mvpp2_port *port)
>         };
>         struct phylink_pcs *pcs;
> 
> -       pcs = mvpp2_select_pcs(&port->phylink_config, port->phy_interface);
> +       if (mvpp2_is_xlg(port->phy_interface))
> +               pcs = &port->pcs_xlg;
> +       else
> +               pcs = &port->pcs_gmac;

I thought the point of new API was that phylink picked the PCS. Why
this if? Why not register both and let phylink pick?

> +static int mvpp2_port_fill_pcs(struct phylink_config *config,
> +                              struct phylink_pcs **available_pcs,
> +                              unsigned int num_possible_pcs)
> +{
> +       struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> +
> +       available_pcs[0] = &port->pcs_gmac;
> +
> +       if (mvpp2_port_supports_xlg(port)) {
> +               if (num_possible_pcs < 2)
> +                       return -EINVAL;

This seems like defensive programming. If we said there was two, we
should assume available_pcs has room for two. I would not even pass
int num_possible_pcs as a parameter.

    Andrew