linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Saravana Kannan <saravanak@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Vladimir Oltean <olteanv@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>, Len Brown <lenb@kernel.org>,
	Alvin Sipraga <ALSI@bang-olufsen.dk>,
	kernel-team@android.com, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org, linux-acpi@vger.kernel.org
Subject: Re: [PATCH v1 1/2] driver core: fw_devlink: Add support for FWNODE_FLAG_BROKEN_PARENT
Date: Thu, 9 Sep 2021 17:00:08 +0200	[thread overview]
Message-ID: <YToheBmLrce1CuZt@lunn.ch> (raw)
In-Reply-To: <CAGETcx_U--ayNCo2GH1-EuzuD9usywjQm+B57X_YwFOjA3e+3Q@mail.gmail.com>

> Sigh... looks like some drivers register their mdio bus in their
> dsa_switch_ops->setup while others do it in their actual probe
> function (which actually makes more sense to me).
 
Drivers are free to do whatever they want. The driver model allows it.

> I'm okay with this big hammer for now while we figure out something
> better.


> 
> >
> > diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> > index 53f034fc2ef7..7ecd910f7fb8 100644
> > --- a/drivers/net/phy/mdio_bus.c
> > +++ b/drivers/net/phy/mdio_bus.c
> > @@ -525,6 +525,9 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
> >             NULL == bus->read || NULL == bus->write)
> >                 return -EINVAL;
> >
> > +       if (bus->parent && bus->parent->of_node)
> > +               bus->parent->of_node->fwnode.flags |= FWNODE_FLAG_BROKEN_PARENT;
> > +
> >         BUG_ON(bus->state != MDIOBUS_ALLOCATED &&
> >                bus->state != MDIOBUS_UNREGISTERED);
> >
> > So basically saying all MDIO busses potentially have a problem.
> >
> > I also don't like the name FWNODE_FLAG_BROKEN_PARENT. The parents are
> > not broken, they work fine, if fw_devlink gets out of the way and
> > allows them to do their job.
> 
> The parent assuming the child will be probed as soon as it's added is
> a broken expectation/assumption. fw_devlink is just catching them
> immediately.

Why is it broken? As i said in the history, DSA has worked since
2008. This behaviour is not that old, but it has been used and worked
for a number of years.

I actual think your model of the driver model is too simple and needs
to accept that a driver probe is not atomic. Resources a driver
registers with other parts of the kernel can be used before the probe
completes. And we have some corners of the kernel that depend on that.

This is particularly true for network drivers. As soon as you register
a network interface to the stack, it will start using it, before the
probe function has completed. It does not wait around for the driver
core to say it has completed probing. And i doubt this is unique to
networking. Maybe when a frame buffer driver registers a frame buffer
with the core, the core starts to draw the splash screen, before the
probe finishes? Maybe when a block driver registers a block device,
the core starts reading the partition table, before the probe
finishes? These are all examples of using a resource before the probe
completes.

> Having said that, this is not the hill either of us should choose to
> die on. So, how about something like:
> FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD
> 
> If that works, I can clean up the series with this and the MDIO fix
> you mentioned.

That is O.K. for me as a fix. I can test patches next week.

     Andrew

  parent reply	other threads:[~2021-09-09 15:00 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-26  7:45 [PATCH v1 0/2] Fix rtl8366rb issues with fw_devlink=on Saravana Kannan
2021-08-26  7:45 ` [PATCH v1 1/2] driver core: fw_devlink: Add support for FWNODE_FLAG_BROKEN_PARENT Saravana Kannan
2021-08-26 13:13   ` Andrew Lunn
2021-08-26 19:56     ` Saravana Kannan
2021-08-26 20:53       ` Andrew Lunn
2021-08-26 23:44         ` Saravana Kannan
2021-08-27  1:23           ` Andrew Lunn
2021-08-27  4:02             ` Saravana Kannan
2021-08-27 13:44               ` Andrew Lunn
2021-08-27 18:06                 ` Saravana Kannan
2021-08-27 20:11                   ` Andrew Lunn
2021-08-27 21:33                     ` Saravana Kannan
2021-08-28 17:01                       ` Andrew Lunn
2021-08-30 19:03                         ` Saravana Kannan
2021-08-31 13:16                           ` Andrew Lunn
2021-08-31 19:26                             ` Saravana Kannan
2021-08-31 22:05                               ` Andrew Lunn
2021-08-31 22:18                                 ` Saravana Kannan
2021-08-31 23:02                                   ` Andrew Lunn
2021-08-31 23:18                                     ` Vladimir Oltean
2021-08-31 23:27                                       ` Andrew Lunn
2021-09-01  1:28                                       ` Vladimir Oltean
2021-09-01  1:38                                         ` Vladimir Oltean
2021-09-01  2:19                                           ` Saravana Kannan
2021-09-01  9:02                                             ` Vladimir Oltean
2021-09-01 22:57                                               ` Saravana Kannan
2021-09-01  2:00                                       ` Saravana Kannan
2021-09-01  8:46                                         ` Vladimir Oltean
2021-09-01 22:53                                           ` Saravana Kannan
2021-09-02 17:41                                           ` Andrew Lunn
2021-09-02 17:58                                             ` Saravana Kannan
2021-09-30  5:33                                       ` Saravana Kannan
2021-09-30 13:15                                         ` Andrew Lunn
2021-09-30 13:43                                         ` Vladimir Oltean
2021-09-30 14:00                                           ` Andrew Lunn
2021-09-30 17:31                                             ` Saravana Kannan
2021-09-30 19:38                                               ` Andrew Lunn
2021-09-30 19:48                                                 ` Saravana Kannan
2021-09-30 20:06                                                   ` Florian Fainelli
2021-09-30 20:14                                                     ` Saravana Kannan
2021-09-30 21:16                                                       ` Florian Fainelli
2021-09-30 20:22                                                     ` Andrew Lunn
2021-09-01  1:46                                     ` Saravana Kannan
2021-08-31 23:18                                   ` Andrew Lunn
2021-09-08 18:35                             ` Saravana Kannan
2021-09-09  1:39                           ` Andrew Lunn
2021-09-09  3:21                             ` Saravana Kannan
2021-09-09 10:38                               ` Vladimir Oltean
2021-09-09 15:00                               ` Andrew Lunn [this message]
2021-09-09 16:56                             ` Florian Fainelli
2021-08-26  7:45 ` [PATCH v1 2/2] net: dsa: rtl8366rb: Quick fix to work with fw_devlink=on Saravana Kannan
2021-08-26  7:55   ` Greg Kroah-Hartman
2021-08-26 11:25   ` Florian Fainelli
2021-08-26 17:29     ` Saravana Kannan
2021-08-26 11:29   ` Alvin Šipraga
2021-08-26 17:26     ` Saravana Kannan
2021-08-26 18:04       ` Alvin Šipraga

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YToheBmLrce1CuZt@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=ALSI@bang-olufsen.dk \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kernel-team@android.com \
    --cc=kuba@kernel.org \
    --cc=lenb@kernel.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=rafael@kernel.org \
    --cc=saravanak@google.com \
    --cc=vivien.didelot@gmail.com \
    --subject='Re: [PATCH v1 1/2] driver core: fw_devlink: Add support for FWNODE_FLAG_BROKEN_PARENT' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).