Re: [PATCH RESEND v2 1/4] HID: hid-appletb-bl: add driver for the backlight of Apple Touch Bars

[Aditya Garg]

Hi Jiri

> On 4 Feb 2025, at 3:25 AM, Jiri Kosina <jikos@xxxxxxxxxx> wrote:
> 
> On Mon, 3 Feb 2025, Aditya Garg wrote:
> 
>> From: Kerem Karabay <kekrby@xxxxxxxxx>
>> 
>> This commit adds a driver for the backlight of Apple Touch Bars on x86
>> Macs. Note that currently only T2 Macs are supported.
>> 
>> This driver is based on previous work done by Ronald Tschalär
>> <ronald@xxxxxxxxxxxxx>.
>> 
>> Signed-off-by: Kerem Karabay <kekrby@xxxxxxxxx>
>> Co-developed-by: Aditya Garg <gargaditya08@xxxxxxxx>
>> Signed-off-by: Aditya Garg <gargaditya08@xxxxxxxx>
>> ---
>> drivers/hid/Kconfig          |  10 ++
>> drivers/hid/Makefile         |   1 +
>> drivers/hid/hid-appletb-bl.c | 207 +++++++++++++++++++++++++++++++++++
>> drivers/hid/hid-quirks.c     |   4 +-
>> 4 files changed, 221 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/hid/hid-appletb-bl.c
>> 
>> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
>> index 4d2a89d65..f6678db27 100644
>> --- a/drivers/hid/Kconfig
>> +++ b/drivers/hid/Kconfig
>> @@ -148,6 +148,16 @@ config HID_APPLEIR
>> 
>>    Say Y here if you want support for Apple infrared remote control.
>> 
>> +config HID_APPLETB_BL
>> +    tristate "Apple Touch Bar Backlight"
>> +    depends on BACKLIGHT_CLASS_DEVICE
>> +    help
>> +      Say Y here if you want support for the backlight of Touch Bars on x86
>> +      MacBook Pros.
>> +
>> +      To compile this driver as a module, choose M here: the
>> +      module will be called hid-appletb-bl.
>> +
>> config HID_ASUS
>>    tristate "Asus"
>>    depends on USB_HID
>> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
>> index 24de45f36..444d24cec 100644
>> --- a/drivers/hid/Makefile
>> +++ b/drivers/hid/Makefile
>> @@ -29,6 +29,7 @@ obj-$(CONFIG_HID_ALPS)        += hid-alps.o
>> obj-$(CONFIG_HID_ACRUX)        += hid-axff.o
>> obj-$(CONFIG_HID_APPLE)        += hid-apple.o
>> obj-$(CONFIG_HID_APPLEIR)    += hid-appleir.o
>> +obj-$(CONFIG_HID_APPLETB_BL)    += hid-appletb-bl.o
> 
> Is there a reason not to build hid-appletb-bl into hid-apple(.ko) driver
> proper?
> 

You might have noticed that the hid-apple driver has code for the magic backlight. Now we have a case of MacBook Air 2020, which does not have a touchbar but still shows presence of the touchbar backlight device, just because it needs it for the keyboard backlight. In case we merge both the backlight and touchbar code in a driver, on that model, the backlight breaks. If kept separate, the touchbar driver simply doesn't load and backlight works.

In the future, we also shall send another driver, which shall support the macOS mode of the touchbar, that is we can use the touchbar as a second display. It shall be a drm driver and both current and the drm driver need the touchbar backlight driver in common.
> Otherwise the code looks good to me, but I'd prefer to have this built-in
> as we generally try to keep things contained in one single per-vendor HID
> driver.
> 
> Thanks,
> 
> --
> Jiri Kosina
> SUSE Labs
>