Quoting Rob Herring (2025-01-08 06:11:28)
> On Tue, Jan 7, 2025 at 7:28 PM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote:
> >
> > Extract the simple bus into a self contained driver so that devices are
> > still populated when a node has two (or more) compatibles with the least
> > specific one being the generic "simple-bus". Allow the driver to be a
> > module so that in a fully modular build a driver module for the more
> > specific compatible will be loaded first before trying to match this
> > driver.
> >
> > Cc: Rob Herring <robh@xxxxxxxxxx>
> > Cc: Saravana Kannan <saravanak@xxxxxxxxxx>
> > Cc: <devicetree@xxxxxxxxxxxxxxx>
> > Cc: Arnd Bergmann <arnd@xxxxxxxx>
> > Cc: "Uwe Kleine-König" <u.kleine-koenig@xxxxxxxxxxxx>
> > Cc: Bjorn Andersson <andersson@xxxxxxxxxx>
> > Cc: Konrad Dybcio <konradybcio@xxxxxxxxxx>
> > Cc: <linux-arm-msm@xxxxxxxxxxxxxxx>
> > Signed-off-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>
> > ---
> > drivers/bus/Kconfig | 23 +++++++++++
> > drivers/bus/Makefile | 3 ++
> > drivers/bus/simple-bus.c | 79 +++++++++++++++++++++++++++++++++++++
> > drivers/bus/simple-pm-bus.c | 2 +
> > drivers/of/platform.c | 50 +++++++++++++++++++++++
> > 5 files changed, 157 insertions(+)
> > create mode 100644 drivers/bus/simple-bus.c
> >
> > diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
> > index ff669a8ccad9..7c2aa1350578 100644
> > --- a/drivers/bus/Kconfig
> > +++ b/drivers/bus/Kconfig
> > @@ -261,6 +261,29 @@ config DA8XX_MSTPRI
> > configuration. Allows to adjust the priorities of all master
> > peripherals.
> >
> > +config ALLOW_SIMPLE_BUS_OVERRIDE
> > + bool "Allow simple-bus compatible OF nodes to match other drivers"
> > + depends on OF
> > + help
> > + Allow nodes with the "simple-bus" compatible to use a more specific
> > + driver which populates child devices itself.
>
> A kconfig option for this is not going to work. What does a distro kernel pick?
I was thinking a distro kernel would set it to =Y
>
> > +
> > +config OF_SIMPLE_BUS
> > + tristate "OF Simple Bus Driver"
> > + depends on ALLOW_SIMPLE_BUS_OVERRIDE || COMPILE_TEST
> > + default ALLOW_SIMPLE_BUS_OVERRIDE
> > + help
> > + Driver for the "simple-bus" compatible nodes in DeviceTree. Child
> > + nodes are usually automatically populated on the platform bus when a
> > + node is compatible with "simple-bus". This driver maintains that
> > + feature but it fails probe to allow other drivers to try to probe
> > + with a more specific compatible if possible.
> > +
> > + Those other drivers depend on this kconfig symbol so that they match
> > + the builtin or modular status of this driver. Don't disable this
> > + symbol if ALLOW_SIMPLE_BUS_OVERRIDE is set and there isn't another
> > + driver for the simple-bus compatible.
>
> Giving users some rope to hang themselves?
Heh.
>
> > +
> > source "drivers/bus/fsl-mc/Kconfig"
> > source "drivers/bus/mhi/Kconfig"
> >
> > diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
> > index cddd4984d6af..f3968221d704 100644
> > --- a/drivers/bus/Makefile
> > +++ b/drivers/bus/Makefile
> > @@ -40,5 +40,8 @@ obj-$(CONFIG_VEXPRESS_CONFIG) += vexpress-config.o
> >
> > obj-$(CONFIG_DA8XX_MSTPRI) += da8xx-mstpri.o
> >
> > +# Must be last for driver registration ordering
> > +obj-$(CONFIG_OF_SIMPLE_BUS) += simple-bus.o
> > +
> > # MHI
> > obj-y += mhi/
> > diff --git a/drivers/bus/simple-bus.c b/drivers/bus/simple-bus.c
> > new file mode 100644
> > index 000000000000..3e39b9818566
> > --- /dev/null
> > +++ b/drivers/bus/simple-bus.c
> > @@ -0,0 +1,79 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Simple Bus Driver
> > + */
> > +
> > +#include <linux/mod_devicetable.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/platform_device.h>
> > +
> > +static struct platform_driver simple_bus_driver;
> > +
> > +static int has_specific_simple_bus_drv(struct device_driver *drv, void *dev)
> > +{
> > + /* Skip if it's this simple bus driver */
> > + if (drv == &simple_bus_driver.driver)
> > + return 0;
> > +
> > + if (of_driver_match_device(dev, drv)) {
> > + dev_dbg(dev, "Allowing '%s' to probe more specifically\n", drv->name);
> > + return 1;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int simple_bus_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + const struct of_dev_auxdata *lookup = dev_get_platdata(dev);
> > + struct device_node *np = dev->of_node;
> > +
> > + /*
> > + * If any other driver wants the device, leave the device to the other
> > + * driver. Only check drivers that come after this driver so that if an
> > + * earlier driver failed to probe we don't populate any devices, and
> > + * only check if there's a more specific compatible.
> > + */
> > + if (of_property_match_string(np, "compatible", "simple-bus") != 0 &&
> > + bus_for_each_drv(&platform_bus_type, &simple_bus_driver.driver, dev,
> > + has_specific_simple_bus_drv))
> > + return -ENODEV;
>
> If this driver is built-in and the specific driver is a module, this
> doesn't work, right?
Yes. The thinking is that if the specific driver is a module we would
make the simple bus driver be a module as well.
>
> I think we're going to have to have a list of specific compatibles
> that have or don't have a specific driver. Today, the list with
> specific drivers is short, but that could easily change if this
> becomes the way to handle things.
Are you talking about simple_pm_bus_probe()? I don't know how to make a
list of specific compatibles work because we can't know if the more
specific driver has been compiled or shipped as a module. We could get
stuck waiting forever for the specific driver to be registered, leading
to what looks like a non-working system because the simple-bus driver
refused to probe. I decided to punt on that problem by relying on
userspace to load the specific driver module before the simple-bus one.
Or if the two drivers are builtin then the specific driver would be
registered before the simple-bus driver via link ordering and it will
work.
> We should consider if new platforms
> should just avoid 'simple-bus' and use something new.
Yes, I think new platforms should entirely avoid "simple-bus" for their
SoC node.
One idea I had was to populate the devices for simple-bus and then
remove them if a more specific driver registers to allow the specific
driver to bind and do what it wants. That's not great because we would
almost always cycle through deleting devices and repopulating them,
unless we play tricks with initcall ordering. The benefit is that we
don't have to do gymnastics to avoid the probe of the simple-bus driver.
Maybe the best approach is to simply avoid all of this and drop the
"simple-bus" compatible from the soc node? It introduces an annoying
hurdle where you have to enable the new driver that does exactly the
same thing as "simple-bus" does so you continue to have a working
system, but it avoids the headaches of trying to make the fallback to
"simple-bus" work and it would match how new DTs would be written. We
could make the driver 'default ARCH_<SOC_VENDOR>' so that it gets built
for olddefconfig users too.
