Re: [PATCH] HID: Support Microsoft Surface Duo SPI-based touch controller driver as a module.
flat view
From: Felipe Balbi <balbi@kernel.org>
Date: 2021-08-12 05:08:51
Hi Dmitry, Dmitry Antipov [off-list ref] writes:
Surface Duo uses a touch digitizer that communicates to the main SoC via SPI and presents itself as a HID device. This patch contains the changes needed for the driver to work as a module: HID Core affordances for SPI devices, addition of the new Device IDs, and a new quirk in hid-microsoft. The driver itself is being prepared for a submission in the near future.
commit log should be broken down at 72 characters
Signed-off-by: Dmitry Antipov dmanti@microsoft.com
this is not the correct way of adding your Signed-off-by line, I'm afraid. It should look like this: Signed-off-by: Dmitry Antipov <redacted>
quoted hunk ↗ jump to hunk
--- drivers/hid/hid-core.c | 3 +++ drivers/hid/hid-ids.h | 2 ++ drivers/hid/hid-microsoft.c | 15 +++++++++++++-- drivers/hid/hid-quirks.c | 2 ++ include/linux/hid.h | 2 ++ 5 files changed, 22 insertions(+), 2 deletions(-)diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 7db332139f7d..123a0e3a6b1a 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c@@ -2005,6 +2005,9 @@ int hid_connect(struct hid_device *hdev, unsigned int connect_mask) case BUS_I2C: bus = "I2C"; break; + case BUS_SPI: + bus = "SPI"; + break; case BUS_VIRTUAL: bus = "VIRTUAL"; break;
this should come as its own patch since it's not directly related to $subject
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 8f1893e68112..5c181d23a7ae 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h@@ -885,6 +885,8 @@ #define USB_DEVICE_ID_MS_XBOX_ONE_S_CONTROLLER 0x02fd #define USB_DEVICE_ID_MS_PIXART_MOUSE 0x00cb #define USB_DEVICE_ID_8BITDO_SN30_PRO_PLUS 0x02e0 +#define SPI_DEVICE_ID_MS_SURFACE_G6_0 0x0c1d +#define SPI_DEVICE_ID_MS_SURFACE_G6_1 0x0c42 #define USB_VENDOR_ID_MOJO 0x8282 #define USB_DEVICE_ID_RETRO_ADAPTER 0x3201diff --git a/drivers/hid/hid-microsoft.c b/drivers/hid/hid-microsoft.c index 071fd093a5f4..50ea1f68c285 100644 --- a/drivers/hid/hid-microsoft.c +++ b/drivers/hid/hid-microsoft.c@@ -27,6 +27,7 @@ #define MS_DUPLICATE_USAGES BIT(5) #define MS_SURFACE_DIAL BIT(6) #define MS_QUIRK_FF BIT(7) +#define MS_NOHIDINPUT BIT(8) struct ms_data { unsigned long quirks;@@ -367,6 +368,7 @@ static int ms_probe(struct hid_device *hdev, const struct hid_device_id *id) unsigned long quirks = id->driver_data; struct ms_data *ms; int ret; + unsigned int connect_mask; ms = devm_kzalloc(&hdev->dev, sizeof(*ms), GFP_KERNEL); if (ms == NULL)@@ -376,20 +378,25 @@ static int ms_probe(struct hid_device *hdev, const struct hid_device_id *id) hid_set_drvdata(hdev, ms); + connect_mask = HID_CONNECT_DEFAULT | ((quirks & MS_HIDINPUT) ? + HID_CONNECT_HIDINPUT_FORCE : 0); + if (quirks & MS_NOGET) hdev->quirks |= HID_QUIRK_NOGET; if (quirks & MS_SURFACE_DIAL) hdev->quirks |= HID_QUIRK_INPUT_PER_APP; + if (quirks & MS_NOHIDINPUT) + connect_mask &= ~HID_CONNECT_HIDINPUT; + ret = hid_parse(hdev); if (ret) { hid_err(hdev, "parse failed\n"); goto err_free; } - ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT | ((quirks & MS_HIDINPUT) ? - HID_CONNECT_HIDINPUT_FORCE : 0)); + ret = hid_hw_start(hdev, connect_mask); if (ret) { hid_err(hdev, "hw start failed\n"); goto err_free;
it looks like adding connect_mask could also be done as a separate patch where the first patch just converts the existing code to use connect_mask and addition for both G6 IDs is done separately
quoted hunk ↗ jump to hunk
@@ -450,6 +457,10 @@ static const struct hid_device_id ms_devices[] = { .driver_data = MS_QUIRK_FF }, { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_8BITDO_SN30_PRO_PLUS), .driver_data = MS_QUIRK_FF }, + { HID_SPI_DEVICE(USB_VENDOR_ID_MICROSOFT, SPI_DEVICE_ID_MS_SURFACE_G6_0), + .driver_data = MS_NOHIDINPUT }, + { HID_SPI_DEVICE(USB_VENDOR_ID_MICROSOFT, SPI_DEVICE_ID_MS_SURFACE_G6_1), + .driver_data = MS_NOHIDINPUT }, { } }; MODULE_DEVICE_TABLE(hid, ms_devices);diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c index 51b39bda9a9d..01609e5425b9 100644 --- a/drivers/hid/hid-quirks.c +++ b/drivers/hid/hid-quirks.c@@ -506,6 +506,8 @@ static const struct hid_device_id hid_have_special_driver[] = { { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_DIGITAL_MEDIA_3KV1) }, { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_POWER_COVER) }, { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_PRESENTER_8K_BT) }, + { HID_SPI_DEVICE(USB_VENDOR_ID_MICROSOFT, SPI_DEVICE_ID_MS_SURFACE_G6_0) }, + { HID_SPI_DEVICE(USB_VENDOR_ID_MICROSOFT, SPI_DEVICE_ID_MS_SURFACE_G6_1) }, #endif #if IS_ENABLED(CONFIG_HID_MONTEREY) { HID_USB_DEVICE(USB_VENDOR_ID_MONTEREY, USB_DEVICE_ID_GENIUS_KB29E) },diff --git a/include/linux/hid.h b/include/linux/hid.h index 9e067f937dbc..32823c6b65f6 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h@@ -684,6 +684,8 @@ struct hid_descriptor { .bus = BUS_BLUETOOTH, .vendor = (ven), .product = (prod) #define HID_I2C_DEVICE(ven, prod) \ .bus = BUS_I2C, .vendor = (ven), .product = (prod) +#define HID_SPI_DEVICE(ven, prod) \ + .bus = BUS_SPI, .vendor = (ven), .product = (prod)
Adding this helper should be done as a seperate too. -- balbi