Re: [PATCH v2] ACPI: video: Switch over to auxiliary bus type
From: Rafael J. Wysocki
Date: Tue Mar 10 2026 - 05:53:44 EST
On Mon, Mar 9, 2026 at 10:59 PM Mario Limonciello (AMD) (kernel.org)
<superm1@xxxxxxxxxx> wrote:
>
>
>
> On 3/9/2026 3:54 PM, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> >
> > Commit 02c057ddefef ("ACPI: video: Convert the driver to a platform one")
>
> Does checkpatch like the capitalized C in Commit?
Honestly, I don't care. In proper English, the first letter of a
sentence is a capital one.
> If not; fix that while committing.
>
> > switched over the ACPI video bus driver from an ACPI driver to a platform
> > driver, but that change introduced an unwanted and unexpected side effect.
> > Namely, on some systems, the ACPI device object of the ACPI video bus
> > device is an ACPI companion of multiple platform devices and, after
> > adding video_device_ids[] as an acpi_match_table to the acpi_video_bus
> > platform driver, all of those devices started to match that driver and
> > its probe callback is invoked for all of them (it fails, but it leaves
> > a confusing message in the log). Moreover, the MODULE_DEVICE_TABLE()
> > of the ACPI video driver module matches all of the devices sharing the
> > ACPI companion with the ACPI video bus device.
> >
> > To address this, make the core ACPI device enumeration code create an
> > auxiliary device for the ACPI video bus device object instead of a
> > platform device and switch over the ACPI video bus driver (once more)
> > to an auxiliary driver.
> >
> > Auxiliary driver generally is a better match for ACPI video bus than
> > platform driver, among other things because the ACPI video bus device
> > does not require any resources to be allocated for it during
> > enumeration. It also allows the ACPI video bus driver to stop abusing
> > device matching based on ACPI device IDs and it allows a special case
> > to be dropped from acpi_create_platform_device() because that function
> > need not worry about the ACPI video bus device any more.
> >
> > Fixes: 02c057ddefef ("ACPI: video: Convert the driver to a platform one")
> > Reported-by: Pratap Nirujogi <pratap.nirujogi@xxxxxxx>
> > Link: https://lore.kernel.org/linux-acpi/007e3390-6b2b-457e-83c7-c794c5952018@xxxxxxx/
> > Tested-by: Pratap Nirujogi <pratap.nirujogi@xxxxxxx>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>
> Reviewed-by: Mario Limonciello (AMD) <superm1@xxxxxxxxxx>
Thanks!
> > ---
> >
> > v1 -> v2:
> > * Added tags
> > * Updated changelog (slightly)
> >
> > ---
> > drivers/acpi/acpi_platform.c | 2 -
> > drivers/acpi/acpi_video.c | 45 ++++++++++++++++++++---------------------
> > drivers/acpi/scan.c | 47 +++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 70 insertions(+), 24 deletions(-)
> >
> > --- a/drivers/acpi/acpi_platform.c
> > +++ b/drivers/acpi/acpi_platform.c
> > @@ -135,7 +135,7 @@ struct platform_device *acpi_create_plat
> > }
> > }
> >
> > - if (adev->device_type == ACPI_BUS_TYPE_DEVICE && !adev->pnp.type.backlight) {
> > + if (adev->device_type == ACPI_BUS_TYPE_DEVICE) {
> > LIST_HEAD(resource_list);
> >
> > count = acpi_dev_get_resources(adev, &resource_list, NULL, NULL);
> > --- a/drivers/acpi/acpi_video.c
> > +++ b/drivers/acpi/acpi_video.c
> > @@ -9,6 +9,7 @@
> >
> > #define pr_fmt(fmt) "ACPI: video: " fmt
> >
> > +#include <linux/auxiliary_bus.h>
> > #include <linux/kernel.h>
> > #include <linux/module.h>
> > #include <linux/init.h>
> > @@ -21,7 +22,6 @@
> > #include <linux/sort.h>
> > #include <linux/pci.h>
> > #include <linux/pci_ids.h>
> > -#include <linux/platform_device.h>
> > #include <linux/slab.h>
> > #include <linux/dmi.h>
> > #include <linux/suspend.h>
> > @@ -77,8 +77,9 @@ static int register_count;
> > static DEFINE_MUTEX(register_count_mutex);
> > static DEFINE_MUTEX(video_list_lock);
> > static LIST_HEAD(video_bus_head);
> > -static int acpi_video_bus_probe(struct platform_device *pdev);
> > -static void acpi_video_bus_remove(struct platform_device *pdev);
> > +static int acpi_video_bus_probe(struct auxiliary_device *aux_dev,
> > + const struct auxiliary_device_id *id);
> > +static void acpi_video_bus_remove(struct auxiliary_device *aux);
> > static void acpi_video_bus_notify(acpi_handle handle, u32 event, void *data);
> >
> > /*
> > @@ -93,19 +94,16 @@ enum acpi_video_level_idx {
> > ACPI_VIDEO_FIRST_LEVEL, /* actual supported levels begin here */
> > };
> >
> > -static const struct acpi_device_id video_device_ids[] = {
> > - {ACPI_VIDEO_HID, 0},
> > - {"", 0},
> > +static const struct auxiliary_device_id video_bus_auxiliary_id_table[] = {
> > + { .name = "acpi.video_bus" },
> > + {},
> > };
> > -MODULE_DEVICE_TABLE(acpi, video_device_ids);
> > +MODULE_DEVICE_TABLE(auxiliary, video_bus_auxiliary_id_table);
> >
> > -static struct platform_driver acpi_video_bus = {
> > +static struct auxiliary_driver acpi_video_bus = {
> > .probe = acpi_video_bus_probe,
> > .remove = acpi_video_bus_remove,
> > - .driver = {
> > - .name = "acpi-video",
> > - .acpi_match_table = video_device_ids,
> > - },
> > + .id_table = video_bus_auxiliary_id_table,
> > };
> >
> > struct acpi_video_bus_flags {
> > @@ -1885,7 +1883,7 @@ static void acpi_video_dev_add_notify_ha
> > }
> >
> > static int acpi_video_bus_add_notify_handler(struct acpi_video_bus *video,
> > - struct platform_device *pdev)
> > + struct device *parent)
> > {
> > struct input_dev *input;
> > struct acpi_video_device *dev;
> > @@ -1908,7 +1906,7 @@ static int acpi_video_bus_add_notify_han
> > input->phys = video->phys;
> > input->id.bustype = BUS_HOST;
> > input->id.product = 0x06;
> > - input->dev.parent = &pdev->dev;
> > + input->dev.parent = parent;
> > input->evbit[0] = BIT(EV_KEY);
> > set_bit(KEY_SWITCHVIDEOMODE, input->keybit);
> > set_bit(KEY_VIDEO_NEXT, input->keybit);
> > @@ -1980,9 +1978,10 @@ static int acpi_video_bus_put_devices(st
> >
> > static int instance;
> >
> > -static int acpi_video_bus_probe(struct platform_device *pdev)
> > +static int acpi_video_bus_probe(struct auxiliary_device *aux_dev,
> > + const struct auxiliary_device_id *id_unused)
> > {
> > - struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
> > + struct acpi_device *device = ACPI_COMPANION(&aux_dev->dev);
> > struct acpi_video_bus *video;
> > bool auto_detect;
> > int error;
> > @@ -2019,7 +2018,7 @@ static int acpi_video_bus_probe(struct p
> > instance++;
> > }
> >
> > - platform_set_drvdata(pdev, video);
> > + auxiliary_set_drvdata(aux_dev, video);
> >
> > video->device = device;
> > strscpy(acpi_device_name(device), ACPI_VIDEO_BUS_NAME);
> > @@ -2068,7 +2067,7 @@ static int acpi_video_bus_probe(struct p
> > !auto_detect)
> > acpi_video_bus_register_backlight(video);
> >
> > - error = acpi_video_bus_add_notify_handler(video, pdev);
> > + error = acpi_video_bus_add_notify_handler(video, &aux_dev->dev);
> > if (error)
> > goto err_del;
> >
> > @@ -2096,10 +2095,10 @@ err_free_video:
> > return error;
> > }
> >
> > -static void acpi_video_bus_remove(struct platform_device *pdev)
> > +static void acpi_video_bus_remove(struct auxiliary_device *aux_dev)
> > {
> > - struct acpi_video_bus *video = platform_get_drvdata(pdev);
> > - struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
> > + struct acpi_video_bus *video = auxiliary_get_drvdata(aux_dev);
> > + struct acpi_device *device = ACPI_COMPANION(&aux_dev->dev);
> >
> > acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > acpi_video_bus_notify);
> > @@ -2163,7 +2162,7 @@ int acpi_video_register(void)
> >
> > dmi_check_system(video_dmi_table);
> >
> > - ret = platform_driver_register(&acpi_video_bus);
> > + ret = auxiliary_driver_register(&acpi_video_bus);
> > if (ret)
> > goto leave;
> >
> > @@ -2183,7 +2182,7 @@ void acpi_video_unregister(void)
> > {
> > mutex_lock(®ister_count_mutex);
> > if (register_count) {
> > - platform_driver_unregister(&acpi_video_bus);
> > + auxiliary_driver_unregister(&acpi_video_bus);
> > register_count = 0;
> > may_report_brightness_keys = false;
> > }
> > --- a/drivers/acpi/scan.c
> > +++ b/drivers/acpi/scan.c
> > @@ -6,6 +6,7 @@
> > #define pr_fmt(fmt) "ACPI: " fmt
> >
> > #include <linux/async.h>
> > +#include <linux/auxiliary_bus.h>
> > #include <linux/module.h>
> > #include <linux/init.h>
> > #include <linux/slab.h>
> > @@ -2192,6 +2193,46 @@ static acpi_status acpi_bus_check_add_2(
> > return acpi_bus_check_add(handle, false, (struct acpi_device **)ret_p);
> > }
> >
> > +static void acpi_video_bus_device_release(struct device *dev)
> > +{
> > + struct auxiliary_device *aux_dev = to_auxiliary_dev(dev);
> > +
> > + kfree(aux_dev);
> > +}
> > +
> > +static void acpi_create_video_bus_device(struct acpi_device *adev,
> > + struct acpi_device *parent)
> > +{
> > + struct auxiliary_device *aux_dev;
> > + static unsigned int aux_dev_id;
> > +
> > + aux_dev = kzalloc_obj(*aux_dev);
> > + if (!aux_dev)
> > + return;
> > +
> > + aux_dev->id = aux_dev_id++;
> > + aux_dev->name = "video_bus";
> > + aux_dev->dev.parent = acpi_get_first_physical_node(parent);
> > + if (!aux_dev->dev.parent)
> > + goto err;
> > +
> > + aux_dev->dev.release = acpi_video_bus_device_release;
> > +
> > + if (auxiliary_device_init(aux_dev))
> > + goto err;
> > +
> > + ACPI_COMPANION_SET(&aux_dev->dev, adev);
> > + if (__auxiliary_device_add(aux_dev, "acpi")) {
> > + auxiliary_device_uninit(aux_dev);
> > + goto err;
> > + }
> > +
> > + return;
> > +
> > +err:
> > + kfree(aux_dev);
> > +}
> > +
> > struct acpi_scan_system_dev {
> > struct list_head node;
> > struct acpi_device *adev;
> > @@ -2229,6 +2270,12 @@ static void acpi_default_enumeration(str
> > sd->adev = device;
> > list_add_tail(&sd->node, &acpi_scan_system_dev_list);
> > }
> > + } else if (device->pnp.type.backlight) {
> > + struct acpi_device *parent;
> > +
> > + parent = acpi_dev_parent(device);
> > + if (parent)
> > + acpi_create_video_bus_device(device, parent);
> > } else {
> > /* For a regular device object, create a platform device. */
> > acpi_create_platform_device(device, NULL);
> >
> >
> >
>
>