Re: [PATCH net-next 2/3] net: dsa: allow DSA switch drivers to provide their own phylink mac ops

[Vladimir Oltean <olteanv@gmail.com>]

On Tue, Apr 09, 2024 at 05:29:23PM +0100, Russell King (Oracle) wrote:
> This changes the logic - it allows driver authors to provide the
> MAC operations, omit the mac_link_down() op _and_ an
> ops->phylink_mac_link_down() function. This could lead to buggy
> drivers since this will only happen in this path and none of the
> others.
> 
> I want this to be an "either provide phylink_mac_ops, and thus
> none of the phylink_mac_* ops in dsa_switch_ops will be called" or
> "don't provide phylink_mac_ops and the phylink_mac_* ops in
> dsa_switch_ops will be called". It's then completely clear cut
> that it's one or the other, whereas the code above makes it
> unclear.

If you want for the API transition to be self-documenting and clear,
it would be good to do that validation separately and more comprehensively
rather than just a fall-through for one single operation here.

If phylink_mac_link_ops is provided, the following ds->ops methods are
obsoleted and can't be provided at the same time (fail probing otherwise):

- phylink_mac_select_pcs()
- phylink_mac_prepare()
- phylink_mac_config()
- phylink_mac_finish()
- phylink_mac_link_down()
- phylink_mac_link_up()

Hopefully it makes it more clear that the following are _not_ obsoleted
by the dedicated phylink mac_ops:

- phylink_get_caps()
- phylink_fixed_state()

Then (after this validation), the simplified
"if (ops && ops->mac_link_down) else (ds->ops->phylink_mac_link_down)"
would be equivalent, because we've errored out on the case which has a
mix of old and new API.