Re: [RFC v1 net-next 6/8] net: dsa: felix: populate mac_capabilities for all ports

On Mon, Sep 12, 2022 at 10:16:21AM +0000, Vladimir Oltean wrote:

On Mon, Sep 12, 2022 at 09:48:36AM +0100, Russell King (Oracle) wrote:

quoted

On Sun, Sep 11, 2022 at 01:02:42PM -0700, Colin Foster wrote:

quoted

phylink_generic_validate() requires that mac_capabilities is correctly
populated. While no existing drivers have used phylink_generic_validate(),
the ocelot_ext.c driver will. Populate this element so the use of existing
functions is possible.

Ocelot always fills in the .phylink_validate method in struct
dsa_switch_ops, mac_capabilities won't be used as
phylink_generic_validate() will not be called by
dsa_port_phylink_validate().

Correct, but felix_phylink_validate() _can_ still directly call
phylink_validate(), right? Colin does not have the full support for
ocelot_ext in this patch set, but this is what he intends to do.

As you mentioned, I do in fact call phylink_generic_validate() in 8/8.

quoted

Also "no existing drivers have used phylink_generic_validate()" I
wonder which drivers you are referring to there. If you are referring
to DSA drivers, then it is extensively used. The following is from
Linus' tree as of today:

By "existing drivers", it is meant felix_vsc9959.c and seville_vsc9953.c,
two drivers in their own right, which use the common felix.c to talk to
(a) DSA and (b) the ocelot switch lib in drivers/net/ethernet/mscc/.
It is true that these existing drivers do not use phylink_generic_validate().
Furthermore, Colin's new ocelot_ext.c is on the same level as
felix_vsc9959.c and seville_vsc9953.c, will use felix.c in the same way,
and will want to use phylink_generic_validate().

quoted

Secondly, I don't see a purpose for this patch in the following
patches, as Ocelot continues to always fill in .phylink_validate,
and as I mentioned above, as long as that member is filled in,
mac_capabilities won't be used unless you explicitly call
phylink_generic_validate() in your .phylink_validate() callback.

Yes, explicit calling is what Colin explained that he wants to do.

quoted

Therefore, I think you can drop this patch from your series and
you won't see any functional change.

This is true. I am also a bit surprised at Colin's choices to
(a) not ask the netdev maintainers to pull into net-next the immutable
    branch that Lee provided here:
    https://lore.kernel.org/lkml/YxrjyHcceLOFlT%2Fc@google.com/ (local)
    and instead send some patches for review which are difficult to
    apply directly to any tree

As mentioned in the cover letter, I don't expect this to necessarily be
ready by the next merge window. But seemingly I misjudged whether
merging the net-next and Lee's tree would be more tedious for the netdev
maintainers than looking at the RFC for reviewers. I'm trying to create
as little hassle for people as I can. Apologies.