Re: [PATCH RFC net-next 3/6] net: dsa: sja1105: use .mac_select_pcs() interface

On Fri, Feb 25, 2022 at 10:57:40AM +0000, Russell King (Oracle) wrote:

quoted

quoted

-	.phylink_mac_config	= sja1105_mac_config,

Deleting sja1105_mac_config() here is safe not because
phylink_mac_config() stops calling pl->mac_ops->mac_config(), but
because dsa_port_phylink_mac_config() first checks whether
ds->ops->phylink_mac_config is implemented, and that is purely an
artefact of providing a phylib-style ds->ops->adjust_link, right?

Yes and no.

We already have a several DSA drivers that have NULL phylink_mac_config
and that don't provide an adjust_link function. Even if adjust_link was
eventually killed off, the test in dsa_port_phylink_mac_config() would
still be necessary unless all these DSA drivers are updated with a stub
function for it.

Consequently, I view phylink_mac_config in DSA as entirely optional and
that optionality is already very much a part of the DSA interface, even
though that is not the case with the corresponding phylink_mac_ops
.mac_config method.

Moreover, this optionality is a common theme in DSA switch operations
methods.

quoted

Maybe it's worth mentioning.

Given that .phylink_mac_config is already established as being optional
in DSA, does the addition of one more instance need to be explicitly
mentioned?

I am aware of that, I am just pointing out that this is an unintended
side effect of the existence of adjust_link, and non-DSA phylink drivers
still need the mac_config stub. When going back and forth between a DSA
and a non-DSA driver, this is not obvious until you take into account
the dsa_port_phylink_mac_config() trampoline. Anyway, don't mention it
if you don't think it is necessary.