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