diff mbox series

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

Message ID 90644A22-3136-4D4E-864E-7F7210D0C713@live.com
State New
Headers show
Series HID: multitouch: Add support for Touch Bars on x86 MacBook Pros | expand

Commit Message

Aditya Garg March 10, 2025, 9:14 a.m. UTC 
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(-)

Comments

Benjamin Tissoires March 25, 2025, 4:11 p.m. UTC | #1 
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
> >>
diff mbox series

Patch

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,