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

Andrew Lunn <[email protected]>
Newsgroups dev.linux.lists.llvm,org.infradead.lists.linux-arm-kernel,org.infradead.lists.linux-mediatek,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-doc,org.kernel.vger.linux-kernel,org.kernel.vger.netdev
Message-ID <[email protected]>
> 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.