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

[Jiri Kosina]

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?

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