Re: [PATCH v1] driver core/ACPI: Introduce companion_bus_register()
"Rafael J. Wysocki (Intel)" <[email protected]>
| Newsgroups | org.kernel.vger.linux-acpi,dev.linux.lists.driver-core,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CAJZ5v0iVB9RM=-shOPKqPXkyN3mkB2wyA-0b5aMpmXXwuBEqkw@mail.gmail.com> |
On Sun, Sep 6, 2026 at 6:03 PM Rafael J. Wysocki <[email protected]> wrote: > > From: Rafael J. Wysocki <[email protected]> > > The ACPI bus type does not allow drivers to be registered, so the sysfs > attributes related to drivers created for it and its devices are > useless, and its drivers/ directory is always empty. All of that is > confusing and wasteful. > > To allow skipping the creation of those sysfs attributes, introduce > the concept of a "companion" bus (as a bus without drivers whose > devices can be bound to other devices and used by their drivers) and > add a special registration function for registering "companion" bus > types, companion_bus_register(). > > Use that function for registering the ACPI bus type. > > Signed-off-by: Rafael J. Wysocki <[email protected]> > --- > drivers/acpi/bus.c | 8 ----- > drivers/base/bus.c | 66 ++++++++++++++++++++++++++++++++------------- > include/linux/device/bus.h | 1 > 3 files changed, 49 insertions(+), 26 deletions(-) > > --- a/drivers/acpi/bus.c > +++ b/drivers/acpi/bus.c > @@ -1120,11 +1120,6 @@ EXPORT_SYMBOL_GPL(acpi_driver_match_devi > ACPI Bus operations > -------------------------------------------------------------------------- */ > > -static int acpi_bus_match(struct device *dev, const struct device_driver *drv) > -{ > - return 0; > -} > - > static int acpi_device_uevent(const struct device *dev, struct kobj_uevent_env *env) > { > return __acpi_device_uevent_modalias(to_acpi_device(dev), env); > @@ -1132,7 +1127,6 @@ static int acpi_device_uevent(const stru > > const struct bus_type acpi_bus_type = { > .name = "acpi", > - .match = acpi_bus_match, > .uevent = acpi_device_uevent, > }; > > @@ -1451,7 +1445,7 @@ static int __init acpi_bus_init(void) > */ > acpi_root_dir = proc_mkdir(ACPI_BUS_FILE_ROOT, NULL); > > - result = bus_register(&acpi_bus_type); > + result = companion_bus_register(&acpi_bus_type); > if (!result) > return 0; > > --- a/drivers/base/bus.c > +++ b/drivers/base/bus.c > @@ -735,7 +735,7 @@ int bus_add_driver(struct device_driver > struct driver_private *priv; > int error = 0; > > - if (!sp) > + if (!sp || !sp->drivers_kset) This will leak the reference to sp if sp itself is not NULL, but sp->drivers_kset is (as dutifully reported by Sashiko). > return -EINVAL; > > /* > @@ -930,15 +930,7 @@ static ssize_t bus_uevent_store(const st > static struct bus_attribute bus_attr_uevent = __ATTR(uevent, 0200, NULL, > bus_uevent_store); > > -/** > - * bus_register - register a driver-core subsystem > - * @bus: bus to register > - * > - * Once we have that, we register the bus with the kobject > - * infrastructure, then register the children subsystems it has: > - * the devices and drivers that belong to the subsystem. > - */ > -int bus_register(const struct bus_type *bus) > +static int bus_register_internal(const struct bus_type *bus, bool use_drivers) > { > int retval; > struct subsys_private *priv; > @@ -960,7 +952,7 @@ int bus_register(const struct bus_type * > > bus_kobj->kset = bus_kset; > bus_kobj->ktype = &bus_ktype; > - priv->drivers_autoprobe = 1; > + priv->drivers_autoprobe = use_drivers; > > retval = kset_register(&priv->subsys); > if (retval) > @@ -976,10 +968,12 @@ int bus_register(const struct bus_type * > goto bus_devices_fail; > } > > - priv->drivers_kset = kset_create_and_add("drivers", NULL, bus_kobj); > - if (!priv->drivers_kset) { > - retval = -ENOMEM; > - goto bus_drivers_fail; > + if (use_drivers) { > + priv->drivers_kset = kset_create_and_add("drivers", NULL, bus_kobj); > + if (!priv->drivers_kset) { > + retval = -ENOMEM; > + goto bus_drivers_fail; > + } > } > > INIT_LIST_HEAD(&priv->interfaces); > @@ -989,9 +983,11 @@ int bus_register(const struct bus_type * > klist_init(&priv->klist_devices, klist_devices_get, klist_devices_put); > klist_init(&priv->klist_drivers, NULL, NULL); > > - retval = add_probe_files(bus); > - if (retval) > - goto bus_probe_files_fail; > + if (use_drivers) { > + retval = add_probe_files(bus); > + if (retval) > + goto bus_probe_files_fail; > + } > > retval = sysfs_create_groups(bus_kobj, bus->bus_groups); > if (retval) > @@ -1016,9 +1012,41 @@ out: > kfree(priv); > return retval; > } > + > +/** > + * bus_register - register a driver-core subsystem > + * @bus: bus to register > + * > + * Once we have that, we register the bus with the kobject > + * infrastructure, then register the children subsystems it has: > + * the devices and drivers that belong to the subsystem. > + */ > +int bus_register(const struct bus_type *bus) > +{ > + return bus_register_internal(bus, true); > +} > EXPORT_SYMBOL_GPL(bus_register); > > /** > + * companion_bus_register - register a companion bus type > + * @bus: companion bus to register > + * > + * A companion bus is a bus without drivers. Devices that belong to it can be > + * bound to other devices as their "companions" and represent interfaces that > + * can be used by the drivers of those other devices. They may also be used for > + * the enumeration of those other devices. > + * > + * The ACPI bus is a specific example of a companion bus. > + * > + * Registering a companion bus is like registering a regular bus except that it > + * skips the creation of sysfs interfaces related to drivers for @bus. > + */ > +int companion_bus_register(const struct bus_type *bus) > +{ > + return bus_register_internal(bus, false); > +} > + > +/** > * bus_unregister - remove a bus from the system > * @bus: bus. > * > @@ -1412,7 +1440,7 @@ struct device_driver *driver_find(const > struct kobject *k; > struct driver_private *priv; > > - if (!sp) > + if (!sp || !sp->drivers_kset) And same here, so I will send a v2. Nevertheless, if you have any concerns or comments on this other than the above, please let me know. > return NULL; > > k = kset_find_obj(sp->drivers_kset, name); > --- a/include/linux/device/bus.h > +++ b/include/linux/device/bus.h > @@ -113,6 +113,7 @@ struct bus_type { > bool need_parent_lock; > }; > > +int __must_check companion_bus_register(const struct bus_type *bus); > int __must_check bus_register(const struct bus_type *bus); > > void bus_unregister(const struct bus_type *bus);