On Fri, 24 May 2019 at 00:28, Russell King - ARM Linux admin
<li...@armlinux.org.uk> wrote:
>
> On Thu, May 23, 2019 at 01:20:40AM +0000, Ioana Ciornei wrote:
> > @@ -111,7 +114,16 @@ static const char *phylink_an_mode_str(unsigned int
> > mode)
> > static int phylink_validate(struct phylink *pl, unsigned long *supported,
> > struct phylink_link_state *state)
> > {
> > - pl->ops->validate(pl->netdev, supported, state);
> > + struct phylink_notifier_info info = {
> > + .supported = supported,
> > + .state = state,
> > + };
> > +
> > + if (pl->ops)
> > + pl->ops->validate(pl->netdev, supported, state);
> > + else
> > + blocking_notifier_call_chain(&pl->notifier_chain,
> > + PHYLINK_VALIDATE, &info);
>
> I don't like this use of notifiers for several reasons:
>
> 1. It becomes harder to grep for users of this.
> 2. We lose documentation about what is passed for each method.
> 3. We lose const-ness for parameters, which then allows users to
> modify phylink-internal data structures inappropriately from
> these notifier calls.
>
> Please find another way.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up
Hi Russell,
Items 2 and 3 can be addressed by creating an union of structures in
struct phylink_notifier_info, just like switchdev does.
For 1 (grep), you mean that the notifiers are upper-case and the
regular callbacks are lower-case?
-Vladimir
