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

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Tue, Apr 09, 2024 at 07:52:30PM +0300, Vladimir Oltean wrote:
> 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.

Okay. However, I can't predict when I'll get around to doing these
changes as my time is very limited over this week and next week - so
may be right before the merge window is due.

Maybe we should've done that for the ops->adjust_link vs
ops->phylink_mac_link_down/ops->phylink_mac_link_up as well.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!