mbox series

[RESEND,v2,0/4] Touch Bar driver for Apple Macs with T2 Security Chip

Message ID 5AEC08E1-0AEF-49BF-94F6-AA1AD71545D0@live.com
Headers show
Series Touch Bar driver for Apple Macs with T2 Security Chip | expand

Message

Aditya Garg Feb. 3, 2025, 4:58 p.m. UTC
From: Aditya Garg <gargaditya08@live.com>

This patch series adds support for Touch Bar on found on MacBook Pro
models with T2 Security Chip. These drivers support the mode found on
Windows Bootcamp drivers.

V2: Added checks to the keyboard driver to ensure backlight driver is
initialised first.

Aditya Garg (2):
  HID: hid-appletb-kbd: add support for fn toggle between media and
    function mode
  HID: hid-appletb-kbd: add support for automatic brightness control
    while using the touchbar

Kerem Karabay (2):
  HID: hid-appletb-bl: add driver for the backlight of Apple Touch Bars
  HID: hid-appletb-kbd: add driver for the keyboard mode of Apple Touch
    Bars

 .../ABI/testing/sysfs-driver-hid-appletb-kbd  |  13 +
 drivers/hid/Kconfig                           |  25 +
 drivers/hid/Makefile                          |   2 +
 drivers/hid/hid-appletb-bl.c                  | 207 +++++++
 drivers/hid/hid-appletb-kbd.c                 | 506 ++++++++++++++++++
 drivers/hid/hid-quirks.c                      |   8 +-
 6 files changed, 759 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-hid-appletb-kbd
 create mode 100644 drivers/hid/hid-appletb-bl.c
 create mode 100644 drivers/hid/hid-appletb-kbd.c

Comments

Aditya Garg Feb. 4, 2025, 2:11 a.m. UTC | #1
Hi Jiri

> On 4 Feb 2025, at 3:25 AM, Jiri Kosina <jikos@kernel.org> wrote:
> 
> On Mon, 3 Feb 2025, Aditya Garg wrote:
> 
>> From: Kerem Karabay <kekrby@gmail.com>
>> 
>> 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@innovation.ch>.
>> 
>> Signed-off-by: Kerem Karabay <kekrby@gmail.com>
>> Co-developed-by: Aditya Garg <gargaditya08@live.com>
>> Signed-off-by: Aditya Garg <gargaditya08@live.com>
>> ---
>> 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
>
Aditya Garg Feb. 4, 2025, 2:54 a.m. UTC | #2
> On 4 Feb 2025, at 7:47 AM, Jiri Kosina <jikos@kernel.org> wrote:
> 
> On Tue, 4 Feb 2025, Aditya Garg wrote:
> 
>> 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.
> 
> Sorry for being dense, but does that mean that it's either hid-appletb-bl
> or hid-apple, but never both to make a good user experience on those
> devices?
> 
> If so, can you please point out what exactly is the reason?
> 
> Either those have different VID/PID combination, and then it can be easily
> made conditional both in code and in runtime.
> Are we talking about conflicting VID/PID combinations, some of them
> needing current hid-apple, and some of them needing (in a mutually
> exclusive way) hid-appletb-bl?
> 

The driver uses the 0th interface for managing the touchbar and the 1st interface for backlight, but there is no touchbar on MacBook Air 2020, so the 0th interface is used for backlight on that.

I remember the author trying various combinations, but having a separate driver was the most feasible option, both for backlight, and the drivers for windows and macOS mode.

> Thanks,
> 
> --
> Jiri Kosina
> SUSE Labs
>