Re: [PATCH v1] driver core/ACPI: Introduce companion_bus_register()

[Rafael J. Wysocki (Intel)]

On Sun, Sep 6, 2026 at 6:03 PM Rafael J. Wysocki <rafael@xxxxxxxxxx> wrote:
>
> From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>
> 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 <rafael.j.wysocki@xxxxxxxxx>
> ---
>  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);