linux-kernel - Re: [PATCH RESEND 5/5] HID: multitouch: add device ID for Apple Touch Bar

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <vx6hvspvlfsv3palzvjpvsrmkl6s7qut366bhxq3tcwvyf7z63@drzftehh2rew>
Date: Tue, 25 Mar 2025 14:13:00 +0100
From: Benjamin Tissoires <bentiss@...nel.org>
To: Aditya Garg <gargaditya08@...e.com>
Cc: Jiri Kosina <jikos@...nel.org>, Jiri Kosina <jkosina@...e.com>, 
	Benjamin Tissoires <benjamin.tissoires@...hat.com>, Kerem Karabay <kekrby@...il.com>, 
	Orlando Chamberlain <orlandoch.dev@...il.com>, Aun-Ali Zaidi <admin@...eit.net>, 
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, "linux-input@...r.kernel.org" <linux-input@...r.kernel.org>
Subject: Re: [PATCH RESEND 5/5] HID: multitouch: add device ID for Apple
 Touch Bar

On Mar 10 2025, Aditya Garg wrote:
> From: Kerem Karabay <kekrby@...il.com>
> 
> This patch adds the device ID of Apple Touch Bar found on x86 MacBook Pros
> to the hid-multitouch driver.
> 
> Note that this is device ID is for T2 Macs. Testing on T1 Macs would be
> appreciated.
> 
> Signed-off-by: Kerem Karabay <kekrby@...il.com>
> Co-developed-by: Aditya Garg <gargaditya08@...e.com>
> Signed-off-by: Aditya Garg <gargaditya08@...e.com>
> ---
>  drivers/hid/Kconfig          |  1 +
>  drivers/hid/hid-multitouch.c | 25 +++++++++++++++++++++----
>  2 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index dfc245867..727a2ed0d 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -743,6 +743,7 @@ config HID_MULTITOUCH
>  	  Say Y here if you have one of the following devices:
>  	  - 3M PCT touch screens
>  	  - ActionStar dual touch panels
> +	  - Apple Touch Bar on x86 MacBook Pros
>  	  - Atmel panels
>  	  - Cando dual touch panels
>  	  - Chunghwa panels
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 66e33a482..078ceef62 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -221,6 +221,7 @@ static void mt_post_parse(struct mt_device *td, struct mt_application *app);
>  #define MT_CLS_GOOGLE				0x0111
>  #define MT_CLS_RAZER_BLADE_STEALTH		0x0112
>  #define MT_CLS_SMART_TECH			0x0113
> +#define MT_CLS_APPLE_TOUCHBAR			0x0114
>  #define MT_CLS_SIS				0x0457
>  
>  #define MT_DEFAULT_MAXCONTACT	10
> @@ -406,6 +407,12 @@ static const struct mt_class mt_classes[] = {
>  			MT_QUIRK_CONTACT_CNT_ACCURATE |
>  			MT_QUIRK_SEPARATE_APP_REPORT,
>  	},
> +	{ .name = MT_CLS_APPLE_TOUCHBAR,
> +		.quirks = MT_QUIRK_HOVERING |
> +			MT_QUIRK_SLOT_IS_CONTACTID_MINUS_ONE |
> +			MT_QUIRK_APPLE_TOUCHBAR,
> +		.maxcontacts = 11,
> +	},
>  	{ .name = MT_CLS_SIS,
>  		.quirks = MT_QUIRK_NOT_SEEN_MEANS_UP |
>  			MT_QUIRK_ALWAYS_VALID |
> @@ -1807,6 +1814,15 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  		}
>  	}
>  
> +	ret = hid_parse(hdev);
> +	if (ret != 0)
> +		return ret;
> +
> +	if (mtclass->name == MT_CLS_APPLE_TOUCHBAR &&
> +	    !hid_find_field(hdev, HID_INPUT_REPORT,
> +			    HID_DG_TOUCHPAD, HID_DG_TRANSDUCER_INDEX))
> +		return -ENODEV;
> +

That hunk and the one below make me very nervous. Is there any reason
preventing you to keep hid_parse() at the same place?

The rest of the series looks fine as everything seems properly guarded
by MT_CLS_APPLE_TOUCHBAR.

Cheers,
Benjamin

>  	td = devm_kzalloc(&hdev->dev, sizeof(struct mt_device), GFP_KERNEL);
>  	if (!td) {
>  		dev_err(&hdev->dev, "cannot allocate multitouch data\n");
> @@ -1854,10 +1870,6 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  
>  	timer_setup(&td->release_timer, mt_expired_timeout, 0);
>  
> -	ret = hid_parse(hdev);
> -	if (ret != 0)
> -		return ret;
> -
>  	if (mtclass->quirks & MT_QUIRK_FIX_CONST_CONTACT_ID)
>  		mt_fix_const_fields(hdev, HID_DG_CONTACTID);
>  
> @@ -2339,6 +2351,11 @@ static const struct hid_device_id mt_devices[] = {
>  		MT_USB_DEVICE(USB_VENDOR_ID_XIROKU,
>  			USB_DEVICE_ID_XIROKU_CSR2) },
>  
> +	/* Apple Touch Bar */
> +	{ .driver_data = MT_CLS_APPLE_TOUCHBAR,
> +		HID_USB_DEVICE(USB_VENDOR_ID_APPLE,
> +			USB_DEVICE_ID_APPLE_TOUCHBAR_DISPLAY) },
> +
>  	/* Google MT devices */
>  	{ .driver_data = MT_CLS_GOOGLE,
>  		HID_DEVICE(HID_BUS_ANY, HID_GROUP_ANY, USB_VENDOR_ID_GOOGLE,
> -- 
> 2.43.0
>