* [PATCH v1] backlight: apple_bl: Convert to a platform driver
@ 2026-03-14 11:50 Rafael J. Wysocki
2026-03-16 1:59 ` Claude review: " Claude Code Review Bot
2026-03-16 1:59 ` Claude Code Review Bot
0 siblings, 2 replies; 3+ messages in thread
From: Rafael J. Wysocki @ 2026-03-14 11:50 UTC (permalink / raw)
To: Lee Jones
Cc: LKML, Linux ACPI, Daniel Thompson, Jingoo Han, Helge Deller,
dri-devel, linux-fbdev
From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
In all cases in which a struct acpi_driver is used for binding a driver
to an ACPI device object, a corresponding platform device is created by
the ACPI core and that device is regarded as a proper representation of
underlying hardware. Accordingly, a struct platform_driver should be
used by driver code to bind to that device. There are multiple reasons
why drivers should not bind directly to ACPI device objects [1].
Overall, it is better to bind drivers to platform devices than to their
ACPI companions, so convert the Apple Backlight ACPI driver to a
platform one.
While this is not expected to alter functionality, it changes sysfs
layout and so it will be visible to user space.
Link: https://lore.kernel.org/all/2396510.ElGaqSPkdT@rafael.j.wysocki/ [1]
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/video/backlight/apple_bl.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/video/backlight/apple_bl.c b/drivers/video/backlight/apple_bl.c
index aaa824437a2a..423513d68b5b 100644
--- a/drivers/video/backlight/apple_bl.c
+++ b/drivers/video/backlight/apple_bl.c
@@ -24,6 +24,7 @@
#include <linux/pci.h>
#include <linux/acpi.h>
#include <linux/atomic.h>
+#include <linux/platform_device.h>
#include <acpi/video.h>
static struct backlight_device *apple_backlight_device;
@@ -134,7 +135,7 @@ static const struct hw_data nvidia_chipset_data = {
.set_brightness = nvidia_chipset_set_brightness,
};
-static int apple_bl_add(struct acpi_device *dev)
+static int apple_bl_probe(struct platform_device *pdev)
{
struct backlight_properties props;
struct pci_dev *host;
@@ -193,7 +194,7 @@ static int apple_bl_add(struct acpi_device *dev)
return 0;
}
-static void apple_bl_remove(struct acpi_device *dev)
+static void apple_bl_remove(struct platform_device *pdev)
{
backlight_device_unregister(apple_backlight_device);
@@ -206,12 +207,12 @@ static const struct acpi_device_id apple_bl_ids[] = {
{"", 0},
};
-static struct acpi_driver apple_bl_driver = {
- .name = "Apple backlight",
- .ids = apple_bl_ids,
- .ops = {
- .add = apple_bl_add,
- .remove = apple_bl_remove,
+static struct platform_driver apple_bl_driver = {
+ .probe = apple_bl_probe,
+ .remove = apple_bl_remove,
+ .driver = {
+ .name = "Apple backlight",
+ .acpi_match_table = apple_bl_ids,
},
};
@@ -224,12 +225,12 @@ static int __init apple_bl_init(void)
if (acpi_video_get_backlight_type() != acpi_backlight_vendor)
return -ENODEV;
- return acpi_bus_register_driver(&apple_bl_driver);
+ return platform_driver_register(&apple_bl_driver);
}
static void __exit apple_bl_exit(void)
{
- acpi_bus_unregister_driver(&apple_bl_driver);
+ platform_driver_unregister(&apple_bl_driver);
}
module_init(apple_bl_init);
--
2.51.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Claude review: backlight: apple_bl: Convert to a platform driver
2026-03-14 11:50 [PATCH v1] backlight: apple_bl: Convert to a platform driver Rafael J. Wysocki
@ 2026-03-16 1:59 ` Claude Code Review Bot
2026-03-16 1:59 ` Claude Code Review Bot
1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-03-16 1:59 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: backlight: apple_bl: Convert to a platform driver
Author: "Rafael J. Wysocki" <rafael@kernel.org>
Patches: 1
Reviewed: 2026-03-16T11:59:43.952358
---
This is a single patch that converts the Apple backlight ACPI driver (`apple_bl`) from using `struct acpi_driver` to `struct platform_driver`. This is part of a broader effort by Rafael to move drivers away from binding directly to ACPI device objects and instead binding to the platform devices that the ACPI core already creates.
The conversion is clean and correct. The patch is small, well-scoped, and the commit message clearly explains the rationale and references the broader discussion about why drivers should not bind directly to ACPI device objects.
**Verdict: Looks good.**
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 3+ messages in thread
* Claude review: backlight: apple_bl: Convert to a platform driver
2026-03-14 11:50 [PATCH v1] backlight: apple_bl: Convert to a platform driver Rafael J. Wysocki
2026-03-16 1:59 ` Claude review: " Claude Code Review Bot
@ 2026-03-16 1:59 ` Claude Code Review Bot
1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-03-16 1:59 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Overall**: This is a correct and straightforward ACPI-to-platform driver conversion. No functional issues found.
**Positive observations:**
1. The `#include <linux/platform_device.h>` addition is correctly placed in alphabetical order.
2. The function signatures are updated correctly:
- `apple_bl_add(struct acpi_device *dev)` → `apple_bl_probe(struct platform_device *pdev)` — the rename from `add` to `probe` follows platform driver convention.
- `apple_bl_remove(struct acpi_device *dev)` → `apple_bl_remove(struct platform_device *pdev)` — the `void` return type matches `platform_driver.remove` (line 241 of `platform_device.h`).
3. The driver struct conversion is correct:
```c
static struct platform_driver apple_bl_driver = {
.probe = apple_bl_probe,
.remove = apple_bl_remove,
.driver = {
.name = "Apple backlight",
.acpi_match_table = apple_bl_ids,
},
};
```
The `acpi_match_table` correctly replaces the old `.ids` field, keeping the same `apple_bl_ids` array.
4. The init/exit functions correctly switch from `acpi_bus_register_driver`/`acpi_bus_unregister_driver` to `platform_driver_register`/`platform_driver_unregister`.
**Minor observation (not a bug):**
- Neither the old `acpi_device *dev` parameter nor the new `platform_device *pdev` parameter is actually used inside `apple_bl_probe()` or `apple_bl_remove()`. The driver relies entirely on global state (`hw_data`, `apple_backlight_device`). This is a pre-existing pattern and not something introduced by this patch, so it's fine to leave it as-is. However, a follow-up could pass `&pdev->dev` to `backlight_device_register()` instead of `NULL` (line 182-183), which would create a proper device hierarchy in sysfs. This is purely optional.
- The `MODULE_DEVICE_TABLE(acpi, apple_bl_ids)` at line 242 is correctly retained, ensuring module autoloading still works.
**Reviewed-by worthy**: Yes, this patch is correct and ready.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-03-16 1:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-14 11:50 [PATCH v1] backlight: apple_bl: Convert to a platform driver Rafael J. Wysocki
2026-03-16 1:59 ` Claude review: " Claude Code Review Bot
2026-03-16 1:59 ` Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox