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

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

[Aditya Garg]

Yes I can move hid_find_field to the original location as well. But, I would not want to devm_kzalloc as well unnecessarily if the touchbar is in the basic mode instead of drm mode which will cause this -ENODEV to be executed right?

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

[Aditya Garg]

FWIW, using my alt gmail account, since outlook is not sending me all the mails for some reason.

> On 25 Mar 2025, at 10:32 PM, Aditya Garg <adityagarg1208@gmail.com> wrote:
> 
> Yes I can move hid_find_field to the original location as well. But, I would not want to devm_kzalloc as well unnecessarily if the touchbar is in the basic mode instead of drm mode which will cause this -ENODEV to be executed right?

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

[Benjamin Tissoires]

On Mar 25 2025, Aditya Garg wrote:
> Yes I can move hid_find_field to the original location as well. But, I would not want to devm_kzalloc as well unnecessarily if the touchbar is in the basic mode instead of drm mode which will cause this -ENODEV to be executed right?

It shouldn't matter. hid_core calls devres_open_group() before calling
.probe(), and calls devres_release_group() on failure. So yes, we'll
allocate a piece of memory and release it after, but it's not something
uncommon.

Cheers,
Benjamin

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

[Aditya Garg]

> On 25 Mar 2025, at 10:59 PM, Benjamin Tissoires <bentiss@kernel.org> wrote:
> 
> On Mar 25 2025, Aditya Garg wrote:
>> Yes I can move hid_find_field to the original location as well. But, I would not want to devm_kzalloc as well unnecessarily if the touchbar is in the basic mode instead of drm mode which will cause this -ENODEV to be executed right?
> 
> It shouldn't matter. hid_core calls devres_open_group() before calling
> .probe(), and calls devres_release_group() on failure. So yes, we'll
> allocate a piece of memory and release it after, but it's not something
> uncommon.

Fair. I'll send a v2

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

[Aditya Garg]

> On 25 Mar 2025, at 11:01 PM, Aditya Garg <adityagarg1208@gmail.com> wrote:
> 
> 
> 
>>> On 25 Mar 2025, at 10:59 PM, Benjamin Tissoires <bentiss@kernel.org> wrote:
>>> 
>>> On Mar 25 2025, Aditya Garg wrote:
>>> Yes I can move hid_find_field to the original location as well. But, I would not want to devm_kzalloc as well unnecessarily if the touchbar is in the basic mode instead of drm mode which will cause this -ENODEV to be executed right?
>> 
>> It shouldn't matter. hid_core calls devres_open_group() before calling
>> .probe(), and calls devres_release_group() on failure. So yes, we'll
>> allocate a piece of memory and release it after, but it's not something
>> uncommon.
> 
> Fair. I'll send a v2

I've sent a v2 from my gmail address. Outlook is being too fussy these days, so hopefully I don't have to sign off twice using gmail as well as outlook.

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

[Jiri Kosina]

On Tue, 25 Mar 2025, Aditya Garg wrote:

> >>> Yes I can move hid_find_field to the original location as well. But, 
> >>> I would not want to devm_kzalloc as well unnecessarily if the 
> >>> touchbar is in the basic mode instead of drm mode which will cause 
> >>> this -ENODEV to be executed right?
> >> 
> >> It shouldn't matter. hid_core calls devres_open_group() before calling
> >> .probe(), and calls devres_release_group() on failure. So yes, we'll
> >> allocate a piece of memory and release it after, but it's not something
> >> uncommon.
> > 
> > Fair. I'll send a v2
> 
> I've sent a v2 from my gmail address. Outlook is being too fussy these 
> days, so hopefully I don't have to sign off twice using gmail as well as 
> outlook.

Thanks. Please always make sure that either in the cover letter or in the 
individual patches you otline the differences between individual patch 
submissions.

-- 
Jiri Kosina
SUSE Labs

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

[Aditya Garg]

> On 26 Mar 2025, at 3:24 PM, Jiri Kosina <jikos@kernel.org> wrote:
> 
> On Tue, 25 Mar 2025, Aditya Garg wrote:
> 
>>>>> Yes I can move hid_find_field to the original location as well. But,
>>>>> I would not want to devm_kzalloc as well unnecessarily if the
>>>>> touchbar is in the basic mode instead of drm mode which will cause
>>>>> this -ENODEV to be executed right?
>>>> 
>>>> It shouldn't matter. hid_core calls devres_open_group() before calling
>>>> .probe(), and calls devres_release_group() on failure. So yes, we'll
>>>> allocate a piece of memory and release it after, but it's not something
>>>> uncommon.
>>> 
>>> Fair. I'll send a v2
>> 
>> I've sent a v2 from my gmail address. Outlook is being too fussy these
>> days, so hopefully I don't have to sign off twice using gmail as well as
>> outlook.
> 
> Thanks. Please always make sure that either in the cover letter or in the
> individual patches you otline the differences between individual patch
> submissions.
> 
Sorry about that. I recently switched to git send-email and it took time for me to get familiar with it. Such minor mistakes do happen in such cases. In case a v3 is needed, I'll share the Changelog. For v2 that I sent, changelog is:

v2: keep parse at original location in patch 5 as suggested by Benjamin

[PATCH RESEND 0/5] HID: multitouch: Add support for Touch Bars on x86 MacBook Pros

[Aditya Garg]

Hi all!

This patch series aims to improve the Touch Bar support for x86 Macs.

Recently, the hid-appletb-kbd and hid-appletb-bl drivers were upstreamed
into the Linux kernel [1]. They enabled the Touch Bar to display a
predefined set of media and function keys, exactly the same it does on
Windows Bootcamp.

Now we are about to get support added for the DRM mode of the Touch Bar
as well [2].

The DRM mode enables the Touch Bar to act as a second display,
just like macOS. So now you can add a widget, put a clock or anything
else on the Touch Bar as long as you can develop a daemon.

Now via these patches, in the DRM mode, we can use the Touch Bar as a
touch screen. The Touch Bar seems to be not compliant with the HID spec,
thus via these patches several tweaks have been done under the cover of
a single quirk, MT_QUIRK_APPLE_TOUCHBAR.

For the case of T2 Macs, apple-bce [3], the driver for the T2 Security
Chip is also needed for all the peripherals, including the Touch Bar
to work. It is still WIP, and will be subsequently sent later to the
appropriate tree. Till then, I'll suggest for get the driver from [3],
or more preferably, get Linux support from https://t2linux.org/.

Cheers
Aditya

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/log/?h=for-6.15/apple
[2]: https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/0670c2f56e45b3f4541985a9ebe06d04308e43b0
[3]: https://github.com/t2linux/apple-bce-drv

Kerem Karabay (5):
  HID: multitouch: Get the contact ID from HID_DG_TRANSDUCER_INDEX
    fields in case of Apple Touch Bar
  HID: multitouch: support getting the tip state from HID_DG_TOUCH
    fields in Apple Touch Bar
  HID: multitouch: take cls->maxcontacts into account for Apple Touch
    Bar even without a HID_DG_CONTACTMAX field
  HID: multitouch: specify that Apple Touch Bar is direct
  HID: multitouch: add device ID for Apple Touch Bar

 drivers/hid/Kconfig          |  1 +
 drivers/hid/hid-multitouch.c | 70 +++++++++++++++++++++++++++++++-----
 2 files changed, 62 insertions(+), 9 deletions(-)

-- 
2.43.0

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

[Aditya Garg]

From: Kerem Karabay <kekrby@gmail.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@gmail.com>
Co-developed-by: Aditya Garg <gargaditya08@live.com>
Signed-off-by: Aditya Garg <gargaditya08@live.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;
+
 	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

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

[Benjamin Tissoires]

On Mar 10 2025, Aditya Garg wrote:
> From: Kerem Karabay <kekrby@gmail.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@gmail.com>
> Co-developed-by: Aditya Garg <gargaditya08@live.com>
> Signed-off-by: Aditya Garg <gargaditya08@live.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
> 

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

[Aditya Garg]

> On 25 Mar 2025, at 6:43 PM, Benjamin Tissoires <bentiss@kernel.org> wrote:
> 
> On Mar 10 2025, Aditya Garg wrote:
>> From: Kerem Karabay <kekrby@gmail.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@gmail.com>
>> Co-developed-by: Aditya Garg <gargaditya08@live.com>
>> Signed-off-by: Aditya Garg <gargaditya08@live.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?
> 
Wouldn't we need to parse in order to do hid_find_field? Although I haven't tried putting it at the same place tbh.

> 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
>> 

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

[Benjamin Tissoires]

On Mar 25 2025, Aditya Garg wrote:
> 
> 
> > On 25 Mar 2025, at 6:43 PM, Benjamin Tissoires <bentiss@kernel.org> wrote:
> > 
> > On Mar 10 2025, Aditya Garg wrote:
> >> From: Kerem Karabay <kekrby@gmail.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@gmail.com>
> >> Co-developed-by: Aditya Garg <gargaditya08@live.com>
> >> Signed-off-by: Aditya Garg <gargaditya08@live.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?
> > 
> Wouldn't we need to parse in order to do hid_find_field? Although I haven't tried putting it at the same place tbh.

Yes, you need hid_parse() to be able to call hid_find_field(). But you
can put hid_find_field() after hid_parse() at the original location, no?

Cheers,
Benjamin

> 
> > 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
> >>