From 542f25a94471570e2594be5b422b9ca572cf88a1 Mon Sep 17 00:00:00 2001 From: Paulo Miguel Almeida Date: Mon, 17 Oct 2022 20:51:22 +1300 Subject: [PATCH 01/51] HID: hyperv: Replace one-element array with flexible-array member One-element arrays are deprecated, and we are replacing them with flexible array members instead. So, replace one-element array with flexible-array member in structs synthhid_msg, synthhid_input_report, pipe_prt_msg and refactor the rest of the code accordingly. This helps with the ongoing efforts to tighten the FORTIFY_SOURCE routines on memcpy() and help us make progress towards globally enabling -fstrict-flex-arrays=3 [1]. Link: https://github.com/KSPP/linux/issues/79 Link: https://github.com/KSPP/linux/issues/210 Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1] Signed-off-by: Paulo Miguel Almeida Reviewed-by: Benjamin Tissoires Reviewed-by: Michael Kelley Signed-off-by: Jiri Kosina --- drivers/hid/hid-hyperv.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c index e0bc731241960..208cf8d981a57 100644 --- a/drivers/hid/hid-hyperv.c +++ b/drivers/hid/hid-hyperv.c @@ -61,7 +61,7 @@ struct synthhid_msg_hdr { struct synthhid_msg { struct synthhid_msg_hdr header; - char data[1]; /* Enclosed message */ + char data[]; /* Enclosed message */ }; union synthhid_version { @@ -99,7 +99,7 @@ struct synthhid_device_info_ack { struct synthhid_input_report { struct synthhid_msg_hdr header; - char buffer[1]; + char buffer[]; }; #pragma pack(pop) @@ -118,7 +118,7 @@ enum pipe_prot_msg_type { struct pipe_prt_msg { enum pipe_prot_msg_type type; u32 size; - char data[1]; + char data[]; }; struct mousevsc_prt_msg { @@ -232,7 +232,7 @@ static void mousevsc_on_receive_device_info(struct mousevsc_dev *input_device, ret = vmbus_sendpacket(input_device->device->channel, &ack, - sizeof(struct pipe_prt_msg) - sizeof(unsigned char) + + sizeof(struct pipe_prt_msg) + sizeof(struct synthhid_device_info_ack), (unsigned long)&ack, VM_PKT_DATA_INBAND, @@ -271,16 +271,14 @@ static void mousevsc_on_receive(struct hv_device *device, * malicious/buggy hypervisor/host, add a check here to * ensure we don't corrupt memory. */ - if ((pipe_msg->size + sizeof(struct pipe_prt_msg) - - sizeof(unsigned char)) + if (struct_size(pipe_msg, data, pipe_msg->size) > sizeof(struct mousevsc_prt_msg)) { WARN_ON(1); break; } memcpy(&input_dev->protocol_resp, pipe_msg, - pipe_msg->size + sizeof(struct pipe_prt_msg) - - sizeof(unsigned char)); + struct_size(pipe_msg, data, pipe_msg->size)); complete(&input_dev->wait_event); break; @@ -359,8 +357,7 @@ static int mousevsc_connect_to_vsp(struct hv_device *device) request->request.version_requested.version = SYNTHHID_INPUT_VERSION; ret = vmbus_sendpacket(device->channel, request, - sizeof(struct pipe_prt_msg) - - sizeof(unsigned char) + + sizeof(struct pipe_prt_msg) + sizeof(struct synthhid_protocol_request), (unsigned long)request, VM_PKT_DATA_INBAND, From deb3b88bbb7a197152aee1eb0157283a6914d504 Mon Sep 17 00:00:00 2001 From: Matt Ranostay Date: Fri, 30 Sep 2022 17:52:06 -0700 Subject: [PATCH 02/51] HID: mcp2221: switch i2c registration to devm functions Switch from i2c_add_adapter() to resource managed devm_i2c_add_adapter() for matching rest of driver initialization, and more concise code. Signed-off-by: Matt Ranostay Reviewed-by: Jonathan Cameron Signed-off-by: Jiri Kosina --- drivers/hid/hid-mcp2221.c | 50 +++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c index de52e9f7bb8cb..4d10a24e3e130 100644 --- a/drivers/hid/hid-mcp2221.c +++ b/drivers/hid/hid-mcp2221.c @@ -824,6 +824,20 @@ static int mcp2221_raw_event(struct hid_device *hdev, return 1; } +/* Device resource managed function for HID unregistration */ +static void mcp2221_hid_unregister(void *ptr) +{ + struct hid_device *hdev = ptr; + + hid_hw_close(hdev); + hid_hw_stop(hdev); +} + +/* This is needed to be sure hid_hw_stop() isn't called twice by the subsystem */ +static void mcp2221_remove(struct hid_device *hdev) +{ +} + static int mcp2221_probe(struct hid_device *hdev, const struct hid_device_id *id) { @@ -849,7 +863,8 @@ static int mcp2221_probe(struct hid_device *hdev, ret = hid_hw_open(hdev); if (ret) { hid_err(hdev, "can't open device\n"); - goto err_hstop; + hid_hw_stop(hdev); + return ret; } mutex_init(&mcp->lock); @@ -857,6 +872,10 @@ static int mcp2221_probe(struct hid_device *hdev, hid_set_drvdata(hdev, mcp); mcp->hdev = hdev; + ret = devm_add_action_or_reset(&hdev->dev, mcp2221_hid_unregister, hdev); + if (ret) + return ret; + /* Set I2C bus clock diviser */ if (i2c_clk_freq > 400) i2c_clk_freq = 400; @@ -873,19 +892,17 @@ static int mcp2221_probe(struct hid_device *hdev, "MCP2221 usb-i2c bridge on hidraw%d", ((struct hidraw *)hdev->hidraw)->minor); - ret = i2c_add_adapter(&mcp->adapter); + ret = devm_i2c_add_adapter(&hdev->dev, &mcp->adapter); if (ret) { hid_err(hdev, "can't add usb-i2c adapter: %d\n", ret); - goto err_i2c; + return ret; } i2c_set_adapdata(&mcp->adapter, mcp); /* Setup GPIO chip */ mcp->gc = devm_kzalloc(&hdev->dev, sizeof(*mcp->gc), GFP_KERNEL); - if (!mcp->gc) { - ret = -ENOMEM; - goto err_gc; - } + if (!mcp->gc) + return -ENOMEM; mcp->gc->label = "mcp2221_gpio"; mcp->gc->direction_input = mcp_gpio_direction_input; @@ -900,26 +917,9 @@ static int mcp2221_probe(struct hid_device *hdev, ret = devm_gpiochip_add_data(&hdev->dev, mcp->gc, mcp); if (ret) - goto err_gc; + return ret; return 0; - -err_gc: - i2c_del_adapter(&mcp->adapter); -err_i2c: - hid_hw_close(mcp->hdev); -err_hstop: - hid_hw_stop(mcp->hdev); - return ret; -} - -static void mcp2221_remove(struct hid_device *hdev) -{ - struct mcp2221 *mcp = hid_get_drvdata(hdev); - - i2c_del_adapter(&mcp->adapter); - hid_hw_close(mcp->hdev); - hid_hw_stop(mcp->hdev); } static const struct hid_device_id mcp2221_devices[] = { From ea418b35103a98329cb019927cc3668c3759b9eb Mon Sep 17 00:00:00 2001 From: Matt Ranostay Date: Fri, 30 Sep 2022 17:52:07 -0700 Subject: [PATCH 03/51] HID: mcp2221: change 'select GPIOLIB' to imply To avoid recursive dependencies on GPIOLIB when 'imply IIO' is requested with other drivers we should switch GPIOLIB to an imply. This isn't the most ideal solution but avoids modifiying the Kconfig for other drivers, and only requires a singular IS_REACHABLE(CONFIG_GPIOLIB) check. Signed-off-by: Matt Ranostay Reviewed-by: Jonathan Cameron Signed-off-by: Jiri Kosina --- drivers/hid/Kconfig | 2 +- drivers/hid/hid-mcp2221.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig index 185a077d59cdd..745fc38794ad7 100644 --- a/drivers/hid/Kconfig +++ b/drivers/hid/Kconfig @@ -1252,7 +1252,7 @@ config HID_ALPS config HID_MCP2221 tristate "Microchip MCP2221 HID USB-to-I2C/SMbus host support" depends on USB_HID && I2C - depends on GPIOLIB + imply GPIOLIB help Provides I2C and SMBUS host adapter functionality over USB-HID through MCP2221 device. diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c index 4d10a24e3e130..fb54f1c6fd9c8 100644 --- a/drivers/hid/hid-mcp2221.c +++ b/drivers/hid/hid-mcp2221.c @@ -915,9 +915,11 @@ static int mcp2221_probe(struct hid_device *hdev, mcp->gc->can_sleep = 1; mcp->gc->parent = &hdev->dev; +#if IS_REACHABLE(CONFIG_GPIOLIB) ret = devm_gpiochip_add_data(&hdev->dev, mcp->gc, mcp); if (ret) return ret; +#endif return 0; } From 960f9df7c620ecb6030aff1d9a6c3d67598b8290 Mon Sep 17 00:00:00 2001 From: Matt Ranostay Date: Fri, 30 Sep 2022 17:52:08 -0700 Subject: [PATCH 04/51] HID: mcp2221: add ADC/DAC support via iio subsystem Add support for 3x 10-bit ADC and 1x DAC channels registered via the iio subsystem. To prevent breakage and unexpected dependencies this support only is only built if CONFIG_IIO is enabled, and is only weakly referenced by 'imply IIO' within the respective Kconfig. Additionally the iio device only gets registered if at least one channel is enabled in the power-on configuration read from SRAM. Signed-off-by: Matt Ranostay Reviewed-by: Jonathan Cameron Signed-off-by: Jiri Kosina --- drivers/hid/Kconfig | 1 + drivers/hid/hid-mcp2221.c | 258 +++++++++++++++++++++++++++++++++++++- 2 files changed, 258 insertions(+), 1 deletion(-) diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig index 745fc38794ad7..17cce4c50e8dc 100644 --- a/drivers/hid/Kconfig +++ b/drivers/hid/Kconfig @@ -1253,6 +1253,7 @@ config HID_MCP2221 tristate "Microchip MCP2221 HID USB-to-I2C/SMbus host support" depends on USB_HID && I2C imply GPIOLIB + imply IIO help Provides I2C and SMBUS host adapter functionality over USB-HID through MCP2221 device. diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c index fb54f1c6fd9c8..2b3c3a4833008 100644 --- a/drivers/hid/hid-mcp2221.c +++ b/drivers/hid/hid-mcp2221.c @@ -10,12 +10,14 @@ #include #include #include +#include #include #include #include #include #include #include +#include #include "hid-ids.h" /* Commands codes in a raw output report */ @@ -30,6 +32,9 @@ enum { MCP2221_I2C_CANCEL = 0x10, MCP2221_GPIO_SET = 0x50, MCP2221_GPIO_GET = 0x51, + MCP2221_SET_SRAM_SETTINGS = 0x60, + MCP2221_GET_SRAM_SETTINGS = 0x61, + MCP2221_READ_FLASH_DATA = 0xb0, }; /* Response codes in a raw input report */ @@ -89,6 +94,7 @@ struct mcp2221 { struct i2c_adapter adapter; struct mutex lock; struct completion wait_in_report; + struct delayed_work init_work; u8 *rxbuf; u8 txbuf[64]; int rxbuf_idx; @@ -97,6 +103,18 @@ struct mcp2221 { struct gpio_chip *gc; u8 gp_idx; u8 gpio_dir; + u8 mode[4]; +#if IS_REACHABLE(CONFIG_IIO) + struct iio_chan_spec iio_channels[3]; + u16 adc_values[3]; + u8 adc_scale; + u8 dac_value; + u16 dac_scale; +#endif +}; + +struct mcp2221_iio { + struct mcp2221 *mcp; }; /* @@ -713,7 +731,7 @@ static int mcp_get_i2c_eng_state(struct mcp2221 *mcp, static int mcp2221_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data, int size) { - u8 *buf; + u8 *buf, tmp; struct mcp2221 *mcp = hid_get_drvdata(hdev); switch (data[0]) { @@ -745,6 +763,9 @@ static int mcp2221_raw_event(struct hid_device *hdev, break; } mcp->status = mcp_get_i2c_eng_state(mcp, data, 8); +#if IS_REACHABLE(CONFIG_IIO) + memcpy(&mcp->adc_values, &data[50], sizeof(mcp->adc_values)); +#endif break; default: mcp->status = -EIO; @@ -816,6 +837,66 @@ static int mcp2221_raw_event(struct hid_device *hdev, complete(&mcp->wait_in_report); break; + case MCP2221_SET_SRAM_SETTINGS: + switch (data[1]) { + case MCP2221_SUCCESS: + mcp->status = 0; + break; + default: + mcp->status = -EAGAIN; + } + complete(&mcp->wait_in_report); + break; + + case MCP2221_GET_SRAM_SETTINGS: + switch (data[1]) { + case MCP2221_SUCCESS: + memcpy(&mcp->mode, &data[22], 4); +#if IS_REACHABLE(CONFIG_IIO) + mcp->dac_value = data[6] & GENMASK(4, 0); +#endif + mcp->status = 0; + break; + default: + mcp->status = -EAGAIN; + } + complete(&mcp->wait_in_report); + break; + + case MCP2221_READ_FLASH_DATA: + switch (data[1]) { + case MCP2221_SUCCESS: + mcp->status = 0; + + /* Only handles CHIP SETTINGS subpage currently */ + if (mcp->txbuf[1] != 0) { + mcp->status = -EIO; + break; + } + +#if IS_REACHABLE(CONFIG_IIO) + /* DAC scale value */ + tmp = FIELD_GET(GENMASK(7, 6), data[6]); + if ((data[6] & BIT(5)) && tmp) + mcp->dac_scale = tmp + 4; + else + mcp->dac_scale = 5; + + /* ADC scale value */ + tmp = FIELD_GET(GENMASK(4, 3), data[7]); + if ((data[7] & BIT(2)) && tmp) + mcp->adc_scale = tmp - 1; + else + mcp->adc_scale = 0; +#endif + + break; + default: + mcp->status = -EAGAIN; + } + complete(&mcp->wait_in_report); + break; + default: mcp->status = -EIO; complete(&mcp->wait_in_report); @@ -838,6 +919,176 @@ static void mcp2221_remove(struct hid_device *hdev) { } +#if IS_REACHABLE(CONFIG_IIO) +static int mcp2221_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *channel, int *val, + int *val2, long mask) +{ + struct mcp2221_iio *priv = iio_priv(indio_dev); + struct mcp2221 *mcp = priv->mcp; + int ret; + + if (mask == IIO_CHAN_INFO_SCALE) { + if (channel->output) + *val = 1 << mcp->dac_scale; + else + *val = 1 << mcp->adc_scale; + + return IIO_VAL_INT; + } + + mutex_lock(&mcp->lock); + + if (channel->output) { + *val = mcp->dac_value; + ret = IIO_VAL_INT; + } else { + /* Read ADC values */ + ret = mcp_chk_last_cmd_status(mcp); + + if (!ret) { + *val = le16_to_cpu(mcp->adc_values[channel->address]); + if (*val >= BIT(10)) + ret = -EINVAL; + else + ret = IIO_VAL_INT; + } + } + + mutex_unlock(&mcp->lock); + + return ret; +} + +static int mcp2221_write_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int val, int val2, long mask) +{ + struct mcp2221_iio *priv = iio_priv(indio_dev); + struct mcp2221 *mcp = priv->mcp; + int ret; + + if (val < 0 || val >= BIT(5)) + return -EINVAL; + + mutex_lock(&mcp->lock); + + memset(mcp->txbuf, 0, 12); + mcp->txbuf[0] = MCP2221_SET_SRAM_SETTINGS; + mcp->txbuf[4] = BIT(7) | val; + + ret = mcp_send_data_req_status(mcp, mcp->txbuf, 12); + if (!ret) + mcp->dac_value = val; + + mutex_unlock(&mcp->lock); + + return ret; +} + +static const struct iio_info mcp2221_info = { + .read_raw = &mcp2221_read_raw, + .write_raw = &mcp2221_write_raw, +}; + +static int mcp_iio_channels(struct mcp2221 *mcp) +{ + int idx, cnt = 0; + bool dac_created = false; + + /* GP0 doesn't have ADC/DAC alternative function */ + for (idx = 1; idx < MCP_NGPIO; idx++) { + struct iio_chan_spec *chan = &mcp->iio_channels[cnt]; + + switch (mcp->mode[idx]) { + case 2: + chan->address = idx - 1; + chan->channel = cnt++; + break; + case 3: + /* GP1 doesn't have DAC alternative function */ + if (idx == 1 || dac_created) + continue; + /* DAC1 and DAC2 outputs are connected to the same DAC */ + dac_created = true; + chan->output = 1; + cnt++; + break; + default: + continue; + }; + + chan->type = IIO_VOLTAGE; + chan->indexed = 1; + chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW); + chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE); + chan->scan_index = -1; + } + + return cnt; +} + +static void mcp_init_work(struct work_struct *work) +{ + struct iio_dev *indio_dev; + struct mcp2221 *mcp = container_of(work, struct mcp2221, init_work.work); + struct mcp2221_iio *data; + static int retries = 5; + int ret, num_channels; + + hid_hw_power(mcp->hdev, PM_HINT_FULLON); + mutex_lock(&mcp->lock); + + mcp->txbuf[0] = MCP2221_GET_SRAM_SETTINGS; + ret = mcp_send_data_req_status(mcp, mcp->txbuf, 1); + + if (ret == -EAGAIN) + goto reschedule_task; + + num_channels = mcp_iio_channels(mcp); + if (!num_channels) + goto unlock; + + mcp->txbuf[0] = MCP2221_READ_FLASH_DATA; + mcp->txbuf[1] = 0; + ret = mcp_send_data_req_status(mcp, mcp->txbuf, 2); + + if (ret == -EAGAIN) + goto reschedule_task; + + indio_dev = devm_iio_device_alloc(&mcp->hdev->dev, sizeof(*data)); + if (!indio_dev) + goto unlock; + + data = iio_priv(indio_dev); + data->mcp = mcp; + + indio_dev->name = "mcp2221"; + indio_dev->modes = INDIO_DIRECT_MODE; + indio_dev->info = &mcp2221_info; + indio_dev->channels = mcp->iio_channels; + indio_dev->num_channels = num_channels; + + devm_iio_device_register(&mcp->hdev->dev, indio_dev); + +unlock: + mutex_unlock(&mcp->lock); + hid_hw_power(mcp->hdev, PM_HINT_NORMAL); + + return; + +reschedule_task: + mutex_unlock(&mcp->lock); + hid_hw_power(mcp->hdev, PM_HINT_NORMAL); + + if (!retries--) + return; + + /* Device is not ready to read SRAM or FLASH data, try again */ + schedule_delayed_work(&mcp->init_work, msecs_to_jiffies(100)); +} +#endif + static int mcp2221_probe(struct hid_device *hdev, const struct hid_device_id *id) { @@ -921,6 +1172,11 @@ static int mcp2221_probe(struct hid_device *hdev, return ret; #endif +#if IS_REACHABLE(CONFIG_IIO) + INIT_DELAYED_WORK(&mcp->init_work, mcp_init_work); + schedule_delayed_work(&mcp->init_work, msecs_to_jiffies(100)); +#endif + return 0; } From baf34f3bbe6de7ac1efb4e31342403e9cca8888d Mon Sep 17 00:00:00 2001 From: Stephen Kitt Date: Wed, 12 Oct 2022 18:33:00 +0200 Subject: [PATCH 05/51] HID: i2c: use simple i2c probe All these drivers have an i2c probe function which doesn't use the "struct i2c_device_id *id" parameter, so they can trivially be converted to the "probe_new" style of probe with a single argument. This is part of an ongoing transition to single-argument i2c probe functions. Old-style probe functions involve a call to i2c_match_id: in drivers/i2c/i2c-core-base.c, /* * When there are no more users of probe(), * rename probe_new to probe. */ if (driver->probe_new) status = driver->probe_new(client); else if (driver->probe) status = driver->probe(client, i2c_match_id(driver->id_table, client)); else status = -EINVAL; Drivers which don't need the second parameter can be declared using probe_new instead, avoiding the call to i2c_match_id. Drivers which do can still be converted to probe_new-style, calling i2c_match_id themselves (as is done currently for of_match_id). This change was done using the following Coccinelle script, and fixed up for whitespace changes: @ rule1 @ identifier fn; identifier client, id; @@ - static int fn(struct i2c_client *client, const struct i2c_device_id *id) + static int fn(struct i2c_client *client) { ...when != id } @ rule2 depends on rule1 @ identifier rule1.fn; identifier driver; @@ struct i2c_driver driver = { - .probe + .probe_new = ( fn | - &fn + fn ) , }; Signed-off-by: Stephen Kitt Signed-off-by: Jiri Kosina --- drivers/hid/i2c-hid/i2c-hid-of-elan.c | 5 ++--- drivers/hid/i2c-hid/i2c-hid-of-goodix.c | 5 ++--- drivers/hid/i2c-hid/i2c-hid-of.c | 5 ++--- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/hid/i2c-hid/i2c-hid-of-elan.c b/drivers/hid/i2c-hid/i2c-hid-of-elan.c index 2d991325e734c..76ddc8be1cbbc 100644 --- a/drivers/hid/i2c-hid/i2c-hid-of-elan.c +++ b/drivers/hid/i2c-hid/i2c-hid-of-elan.c @@ -68,8 +68,7 @@ static void elan_i2c_hid_power_down(struct i2chid_ops *ops) regulator_disable(ihid_elan->vcc33); } -static int i2c_hid_of_elan_probe(struct i2c_client *client, - const struct i2c_device_id *id) +static int i2c_hid_of_elan_probe(struct i2c_client *client) { struct i2c_hid_of_elan *ihid_elan; @@ -119,7 +118,7 @@ static struct i2c_driver elan_i2c_hid_ts_driver = { .probe_type = PROBE_PREFER_ASYNCHRONOUS, .of_match_table = of_match_ptr(elan_i2c_hid_of_match), }, - .probe = i2c_hid_of_elan_probe, + .probe_new = i2c_hid_of_elan_probe, .remove = i2c_hid_core_remove, .shutdown = i2c_hid_core_shutdown, }; diff --git a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c index ec6c73f75ffe0..29c6cb174032a 100644 --- a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c +++ b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c @@ -87,8 +87,7 @@ static int ihid_goodix_vdd_notify(struct notifier_block *nb, return ret; } -static int i2c_hid_of_goodix_probe(struct i2c_client *client, - const struct i2c_device_id *id) +static int i2c_hid_of_goodix_probe(struct i2c_client *client) { struct i2c_hid_of_goodix *ihid_goodix; int ret; @@ -167,7 +166,7 @@ static struct i2c_driver goodix_i2c_hid_ts_driver = { .probe_type = PROBE_PREFER_ASYNCHRONOUS, .of_match_table = of_match_ptr(goodix_i2c_hid_of_match), }, - .probe = i2c_hid_of_goodix_probe, + .probe_new = i2c_hid_of_goodix_probe, .remove = i2c_hid_core_remove, .shutdown = i2c_hid_core_shutdown, }; diff --git a/drivers/hid/i2c-hid/i2c-hid-of.c b/drivers/hid/i2c-hid/i2c-hid-of.c index 97a27a803f58d..10176568133af 100644 --- a/drivers/hid/i2c-hid/i2c-hid-of.c +++ b/drivers/hid/i2c-hid/i2c-hid-of.c @@ -66,8 +66,7 @@ static void i2c_hid_of_power_down(struct i2chid_ops *ops) ihid_of->supplies); } -static int i2c_hid_of_probe(struct i2c_client *client, - const struct i2c_device_id *dev_id) +static int i2c_hid_of_probe(struct i2c_client *client) { struct device *dev = &client->dev; struct i2c_hid_of *ihid_of; @@ -138,7 +137,7 @@ static struct i2c_driver i2c_hid_of_driver = { .of_match_table = of_match_ptr(i2c_hid_of_match), }, - .probe = i2c_hid_of_probe, + .probe_new = i2c_hid_of_probe, .remove = i2c_hid_core_remove, .shutdown = i2c_hid_core_shutdown, .id_table = i2c_hid_of_id_table, From daf405c8b9b93388295dec07391798039d0f5c26 Mon Sep 17 00:00:00 2001 From: Jiri Kosina Date: Thu, 20 Oct 2022 13:42:38 +0200 Subject: [PATCH 06/51] HID: mcp2221: fix usage of tmp variable in mcp2221_raw_event() In mcp2221_raw_event(), 'tmp' is used only conditionally. Move the declaration into the conditional block in order to prevent unused variable warning. Reported-by: kernel test robot Fixes: 960f9df7c620 ("HID: mcp2221: add ADC/DAC support via iio subsystem") Signed-off-by: Jiri Kosina --- drivers/hid/hid-mcp2221.c | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c index 2b3c3a4833008..b3eaf170f0ec4 100644 --- a/drivers/hid/hid-mcp2221.c +++ b/drivers/hid/hid-mcp2221.c @@ -731,7 +731,7 @@ static int mcp_get_i2c_eng_state(struct mcp2221 *mcp, static int mcp2221_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data, int size) { - u8 *buf, tmp; + u8 *buf; struct mcp2221 *mcp = hid_get_drvdata(hdev); switch (data[0]) { @@ -875,19 +875,22 @@ static int mcp2221_raw_event(struct hid_device *hdev, } #if IS_REACHABLE(CONFIG_IIO) - /* DAC scale value */ - tmp = FIELD_GET(GENMASK(7, 6), data[6]); - if ((data[6] & BIT(5)) && tmp) - mcp->dac_scale = tmp + 4; - else - mcp->dac_scale = 5; - - /* ADC scale value */ - tmp = FIELD_GET(GENMASK(4, 3), data[7]); - if ((data[7] & BIT(2)) && tmp) - mcp->adc_scale = tmp - 1; - else - mcp->adc_scale = 0; + { + u8 tmp; + /* DAC scale value */ + tmp = FIELD_GET(GENMASK(7, 6), data[6]); + if ((data[6] & BIT(5)) && tmp) + mcp->dac_scale = tmp + 4; + else + mcp->dac_scale = 5; + + /* ADC scale value */ + tmp = FIELD_GET(GENMASK(4, 3), data[7]); + if ((data[7] & BIT(2)) && tmp) + mcp->adc_scale = tmp - 1; + else + mcp->adc_scale = 0; + } #endif break; From e91fc483552df10291f9efac5dd9241ca129f2a4 Mon Sep 17 00:00:00 2001 From: Matt Ranostay Date: Thu, 20 Oct 2022 23:29:59 +0800 Subject: [PATCH 07/51] HID: mcp2221: fix 'cast to restricted __le16' sparse warnings Use (__force __le16) cast for adc_values le16_to_cpu conversion to correct following sparse warnings: drivers/hid/hid-mcp2221.c:950:32: sparse: sparse: cast to restricted __le16 Reported-by: kernel test robot Fixes: 960f9df7c620 ("HID: mcp2221: add ADC/DAC support via iio subsystem") Signed-off-by: Matt Ranostay Signed-off-by: Jiri Kosina --- drivers/hid/hid-mcp2221.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c index b3eaf170f0ec4..3014932c8cef5 100644 --- a/drivers/hid/hid-mcp2221.c +++ b/drivers/hid/hid-mcp2221.c @@ -950,7 +950,7 @@ static int mcp2221_read_raw(struct iio_dev *indio_dev, ret = mcp_chk_last_cmd_status(mcp); if (!ret) { - *val = le16_to_cpu(mcp->adc_values[channel->address]); + *val = le16_to_cpu((__force __le16) mcp->adc_values[channel->address]); if (*val >= BIT(10)) ret = -EINVAL; else From 3d74c9eca1a2bda03e45f18d13154ac3e0dfba85 Mon Sep 17 00:00:00 2001 From: Matt Ranostay Date: Thu, 20 Oct 2022 23:30:00 +0800 Subject: [PATCH 08/51] HID: mcp2221: correct undefined references when CONFIG_GPIOLIB isn't defined Singular #ifdef IS_REACHABLE(CONFIG_GPIOLIB) weren't covering all the gpiolib functions that were being referenced. Update the code regions that are commented out when CONFIG_GPIOLIB isn't enabled to avoid errors. Fixes: 960f9df7c620 ("HID: mcp2221: add ADC/DAC support via iio subsystem") Signed-off-by: Matt Ranostay Signed-off-by: Jiri Kosina --- drivers/hid/hid-mcp2221.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c index 3014932c8cef5..5886543b17f34 100644 --- a/drivers/hid/hid-mcp2221.c +++ b/drivers/hid/hid-mcp2221.c @@ -585,6 +585,7 @@ static const struct i2c_algorithm mcp_i2c_algo = { .functionality = mcp_i2c_func, }; +#if IS_REACHABLE(CONFIG_GPIOLIB) static int mcp_gpio_get(struct gpio_chip *gc, unsigned int offset) { @@ -688,6 +689,7 @@ static int mcp_gpio_get_direction(struct gpio_chip *gc, return GPIO_LINE_DIRECTION_OUT; } +#endif /* Gives current state of i2c engine inside mcp2221 */ static int mcp_get_i2c_eng_state(struct mcp2221 *mcp, @@ -1153,6 +1155,7 @@ static int mcp2221_probe(struct hid_device *hdev, } i2c_set_adapdata(&mcp->adapter, mcp); +#if IS_REACHABLE(CONFIG_GPIOLIB) /* Setup GPIO chip */ mcp->gc = devm_kzalloc(&hdev->dev, sizeof(*mcp->gc), GFP_KERNEL); if (!mcp->gc) @@ -1169,7 +1172,6 @@ static int mcp2221_probe(struct hid_device *hdev, mcp->gc->can_sleep = 1; mcp->gc->parent = &hdev->dev; -#if IS_REACHABLE(CONFIG_GPIOLIB) ret = devm_gpiochip_add_data(&hdev->dev, mcp->gc, mcp); if (ret) return ret; From 6a4628997cfcc1eb1e34943f011d85bae36eadbc Mon Sep 17 00:00:00 2001 From: Paulo Miguel Almeida Date: Mon, 24 Oct 2022 13:57:42 +1300 Subject: [PATCH 09/51] HID: hyperv: remove unused struct synthhid_msg struct synthhid_msg was meant to be a generic representation of the possible protocol messages sent through VMBus. In practice, only the header is read and depending on the message type, a cast to the actual type is done. Also, SYNTHHID_MAX_INPUT_REPORT_SIZE constant isn't used which I suspect is a leftover from the refactoring made while this driver was at the staging folder. This patch removes struct synthhid_msg and refactor the code accordingly. Signed-off-by: Paulo Miguel Almeida Reviewed-by: Michael Kelley Signed-off-by: Jiri Kosina --- drivers/hid/hid-hyperv.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c index 208cf8d981a57..0be717bb09d4c 100644 --- a/drivers/hid/hid-hyperv.c +++ b/drivers/hid/hid-hyperv.c @@ -22,9 +22,6 @@ struct hv_input_dev_info { unsigned short reserved[11]; }; -/* The maximum size of a synthetic input message. */ -#define SYNTHHID_MAX_INPUT_REPORT_SIZE 16 - /* * Current version * @@ -59,11 +56,6 @@ struct synthhid_msg_hdr { u32 size; }; -struct synthhid_msg { - struct synthhid_msg_hdr header; - char data[]; /* Enclosed message */ -}; - union synthhid_version { struct { u16 minor_version; @@ -251,7 +243,7 @@ static void mousevsc_on_receive(struct hv_device *device, struct vmpacket_descriptor *packet) { struct pipe_prt_msg *pipe_msg; - struct synthhid_msg *hid_msg; + struct synthhid_msg_hdr *hid_msg_hdr; struct mousevsc_dev *input_dev = hv_get_drvdata(device); struct synthhid_input_report *input_report; size_t len; @@ -262,9 +254,9 @@ static void mousevsc_on_receive(struct hv_device *device, if (pipe_msg->type != PIPE_MESSAGE_DATA) return; - hid_msg = (struct synthhid_msg *)pipe_msg->data; + hid_msg_hdr = (struct synthhid_msg_hdr *)pipe_msg->data; - switch (hid_msg->header.type) { + switch (hid_msg_hdr->type) { case SYNTH_HID_PROTOCOL_RESPONSE: /* * While it will be impossible for us to protect against @@ -309,7 +301,7 @@ static void mousevsc_on_receive(struct hv_device *device, break; default: pr_err("unsupported hid msg type - type %d len %d\n", - hid_msg->header.type, hid_msg->header.size); + hid_msg_hdr->type, hid_msg_hdr->size); break; } From 5476fcf7f7b901db1cea92acb1abdd12609e30e1 Mon Sep 17 00:00:00 2001 From: Kerem Karabay Date: Sat, 24 Sep 2022 12:53:05 +0300 Subject: [PATCH 10/51] HID: apple: fix key translations where multiple quirks attempt to translate the same key The hid-apple driver does not support chaining translations or dependencies on other translations. This creates two problems: 1 - In Non-English keyboards of Macs, KEY_102ND and KEY_GRAVE are swapped and the APPLE_ISO_TILDE_QUIRK is used to work around this problem. The quirk is not set for the Macs where these bugs happen yet (see the 2nd patch for that), but this can be forced by setting the iso_layout parameter. Unfortunately, this only partially works. KEY_102ND gets translated to KEY_GRAVE, but KEY_GRAVE does not get translated to KEY_102ND, so both of them end up functioning as KEY_GRAVE. This is because the driver translates the keys as if Fn was pressed and the original is sent if it is not pressed, without any further translations happening on the key[#463]. KEY_GRAVE is present at macbookpro_no_esc_fn_keys[#195], so this is what happens: - KEY_GRAVE -> KEY_ESC (as if Fn is pressed) - KEY_GRAVE is returned (Fn isn't pressed, so translation is discarded) - KEY_GRAVE -> KEY_102ND (this part is not reached!) ... 2 - In case the touchbar does not work, the driver supports sending Escape when Fn+KEY_GRAVE is pressed. As mentioned previously, KEY_102ND is actually KEY_GRAVE and needs to be translated before this happens. Normally, these are the steps that should happen: - KEY_102ND -> KEY_GRAVE - KEY_GRAVE -> KEY_ESC (Fn is pressed) - KEY_ESC is returned Though this is what happens instead, as dependencies on other translations are not supported: - KEY_102ND -> KEY_ESC (Fn is pressed) - KEY_ESC is returned This patch fixes both bugs by ordering the translations correctly and by making the translations continue and not return immediately after translating a key so that chained translations work and translations can depend on other ones. This patch also simplifies the implementation of the swap_fn_leftctrl option a little bit, as it makes it simply use a normal translation instead adding extra code to translate a key to KEY_FN[#381]. This change wasn't put in another patch as the code that translates the Fn key needs to be changed because of the changes in the patch, and those changes would be discarded with the next patch anyway (the part that originally translates KEY_FN to KEY_LEFTCTRL needs to be made an else-if branch of the part that transltes KEY_LEFTCTRL to KEY_FN). Note: Line numbers (#XYZ) are for drivers/hid/hid-apple.c at commit 20afcc462579 ("HID: apple: Add "GANSS" to the non-Apple list"). Note: These bugs are only present on Macs with a keyboard with no dedicated escape key and a non-English layout. Signed-off-by: Kerem Karabay Signed-off-by: Jiri Kosina --- drivers/hid/hid-apple.c | 102 +++++++++++++++++----------------------- 1 file changed, 44 insertions(+), 58 deletions(-) diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c index 6970797cdc56d..e86bbf85b87e9 100644 --- a/drivers/hid/hid-apple.c +++ b/drivers/hid/hid-apple.c @@ -314,6 +314,7 @@ static const struct apple_key_translation swapped_option_cmd_keys[] = { static const struct apple_key_translation swapped_fn_leftctrl_keys[] = { { KEY_FN, KEY_LEFTCTRL }, + { KEY_LEFTCTRL, KEY_FN }, { } }; @@ -375,24 +376,40 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input, struct apple_sc *asc = hid_get_drvdata(hid); const struct apple_key_translation *trans, *table; bool do_translate; - u16 code = 0; + u16 code = usage->code; unsigned int real_fnmode; - u16 fn_keycode = (swap_fn_leftctrl) ? (KEY_LEFTCTRL) : (KEY_FN); - - if (usage->code == fn_keycode) { - asc->fn_on = !!value; - input_event_with_scancode(input, usage->type, KEY_FN, - usage->hid, value); - return 1; - } - if (fnmode == 3) { real_fnmode = (asc->quirks & APPLE_IS_NON_APPLE) ? 2 : 1; } else { real_fnmode = fnmode; } + if (swap_fn_leftctrl) { + trans = apple_find_translation(swapped_fn_leftctrl_keys, code); + + if (trans) + code = trans->to; + } + + if (iso_layout > 0 || (iso_layout < 0 && (asc->quirks & APPLE_ISO_TILDE_QUIRK) && + hid->country == HID_COUNTRY_INTERNATIONAL_ISO)) { + trans = apple_find_translation(apple_iso_keyboard, code); + + if (trans) + code = trans->to; + } + + if (swap_opt_cmd) { + trans = apple_find_translation(swapped_option_cmd_keys, code); + + if (trans) + code = trans->to; + } + + if (code == KEY_FN) + asc->fn_on = !!value; + if (real_fnmode) { if (hid->product == USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI || hid->product == USB_DEVICE_ID_APPLE_ALU_WIRELESS_ISO || @@ -430,15 +447,18 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input, else table = apple_fn_keys; - trans = apple_find_translation (table, usage->code); + trans = apple_find_translation(table, code); if (trans) { - if (test_bit(trans->from, input->key)) + bool from_is_set = test_bit(trans->from, input->key); + bool to_is_set = test_bit(trans->to, input->key); + + if (from_is_set) code = trans->from; - else if (test_bit(trans->to, input->key)) + else if (to_is_set) code = trans->to; - if (!code) { + if (!(from_is_set || to_is_set)) { if (trans->flags & APPLE_FLAG_FKEY) { switch (real_fnmode) { case 1: @@ -455,62 +475,31 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input, do_translate = asc->fn_on; } - code = do_translate ? trans->to : trans->from; + if (do_translate) + code = trans->to; } - - input_event_with_scancode(input, usage->type, code, - usage->hid, value); - return 1; } if (asc->quirks & APPLE_NUMLOCK_EMULATION && - (test_bit(usage->code, asc->pressed_numlock) || + (test_bit(code, asc->pressed_numlock) || test_bit(LED_NUML, input->led))) { - trans = apple_find_translation(powerbook_numlock_keys, - usage->code); + trans = apple_find_translation(powerbook_numlock_keys, code); if (trans) { if (value) - set_bit(usage->code, - asc->pressed_numlock); + set_bit(code, asc->pressed_numlock); else - clear_bit(usage->code, - asc->pressed_numlock); + clear_bit(code, asc->pressed_numlock); - input_event_with_scancode(input, usage->type, - trans->to, usage->hid, value); + code = trans->to; } - - return 1; } } - if (iso_layout > 0 || (iso_layout < 0 && (asc->quirks & APPLE_ISO_TILDE_QUIRK) && - hid->country == HID_COUNTRY_INTERNATIONAL_ISO)) { - trans = apple_find_translation(apple_iso_keyboard, usage->code); - if (trans) { - input_event_with_scancode(input, usage->type, - trans->to, usage->hid, value); - return 1; - } - } + if (usage->code != code) { + input_event_with_scancode(input, usage->type, code, usage->hid, value); - if (swap_opt_cmd) { - trans = apple_find_translation(swapped_option_cmd_keys, usage->code); - if (trans) { - input_event_with_scancode(input, usage->type, - trans->to, usage->hid, value); - return 1; - } - } - - if (swap_fn_leftctrl) { - trans = apple_find_translation(swapped_fn_leftctrl_keys, usage->code); - if (trans) { - input_event_with_scancode(input, usage->type, - trans->to, usage->hid, value); - return 1; - } + return 1; } return 0; @@ -640,9 +629,6 @@ static void apple_setup_input(struct input_dev *input) apple_setup_key_translation(input, apple2021_fn_keys); apple_setup_key_translation(input, macbookpro_no_esc_fn_keys); apple_setup_key_translation(input, macbookpro_dedicated_esc_fn_keys); - - if (swap_fn_leftctrl) - apple_setup_key_translation(input, swapped_fn_leftctrl_keys); } static int apple_input_mapping(struct hid_device *hdev, struct hid_input *hi, From 084bc074c231e716cbcb9e8f9db05b17fd3563cf Mon Sep 17 00:00:00 2001 From: Kerem Karabay Date: Sat, 24 Sep 2022 12:53:06 +0300 Subject: [PATCH 11/51] HID: apple: enable APPLE_ISO_TILDE_QUIRK for the keyboards of Macs with the T2 chip The iso_layout parameter must be manually set to get the driver to swap KEY_102ND and KEY_GRAVE. This patch eliminates the need to do that. This is safe to do, as Macs with keyboards that do not need the quirk will keep working the same way as the value of hid->country will be different than HID_COUNTRY_INTERNATIONAL_ISO. This was tested by one person with a Mac with the WELLSPRINGT2_J152F keyboard with a layout that does not require the quirk to be set. Signed-off-by: Kerem Karabay Signed-off-by: Jiri Kosina --- drivers/hid/hid-apple.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c index e86bbf85b87e9..c671ce94671ca 100644 --- a/drivers/hid/hid-apple.c +++ b/drivers/hid/hid-apple.c @@ -997,21 +997,21 @@ static const struct hid_device_id apple_devices[] = { { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING9_JIS), .driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRINGT2_J140K), - .driver_data = APPLE_HAS_FN | APPLE_BACKLIGHT_CTL }, + .driver_data = APPLE_HAS_FN | APPLE_BACKLIGHT_CTL | APPLE_ISO_TILDE_QUIRK }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRINGT2_J132), - .driver_data = APPLE_HAS_FN | APPLE_BACKLIGHT_CTL }, + .driver_data = APPLE_HAS_FN | APPLE_BACKLIGHT_CTL | APPLE_ISO_TILDE_QUIRK }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRINGT2_J680), - .driver_data = APPLE_HAS_FN | APPLE_BACKLIGHT_CTL }, + .driver_data = APPLE_HAS_FN | APPLE_BACKLIGHT_CTL | APPLE_ISO_TILDE_QUIRK }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRINGT2_J213), - .driver_data = APPLE_HAS_FN | APPLE_BACKLIGHT_CTL }, + .driver_data = APPLE_HAS_FN | APPLE_BACKLIGHT_CTL | APPLE_ISO_TILDE_QUIRK }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRINGT2_J214K), - .driver_data = APPLE_HAS_FN }, + .driver_data = APPLE_HAS_FN | APPLE_ISO_TILDE_QUIRK }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRINGT2_J223), - .driver_data = APPLE_HAS_FN }, + .driver_data = APPLE_HAS_FN | APPLE_ISO_TILDE_QUIRK }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRINGT2_J230K), - .driver_data = APPLE_HAS_FN }, + .driver_data = APPLE_HAS_FN | APPLE_ISO_TILDE_QUIRK }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRINGT2_J152F), - .driver_data = APPLE_HAS_FN }, + .driver_data = APPLE_HAS_FN | APPLE_ISO_TILDE_QUIRK }, { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_2009_ANSI), .driver_data = APPLE_NUMLOCK_EMULATION | APPLE_HAS_FN }, { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_2009_ISO), From 05086f3db530b3b73920b0c8c50f5d5bc8494375 Mon Sep 17 00:00:00 2001 From: Joshua Jun Date: Sun, 23 Oct 2022 00:24:08 +0200 Subject: [PATCH 12/51] HID: wiimote: Add support for the DJ Hero turntable This adds support for the turntable extension for Wiimote devices. jstest-gtk and html5 gamepad tester show everything correctly but when trying to map the controller in software like rpcs3 or dolphin it currently doesn't map correctly Co-authored-by: Bogdan Petru Signed-off-by: Bogdan Petru Signed-off-by: Joshua Jun Signed-off-by: Jiri Kosina --- drivers/hid/hid-wiimote-core.c | 7 + drivers/hid/hid-wiimote-modules.c | 225 ++++++++++++++++++++++++++++++ drivers/hid/hid-wiimote.h | 1 + 3 files changed, 233 insertions(+) diff --git a/drivers/hid/hid-wiimote-core.c b/drivers/hid/hid-wiimote-core.c index 4399d6c6afef2..09db8111dc262 100644 --- a/drivers/hid/hid-wiimote-core.c +++ b/drivers/hid/hid-wiimote-core.c @@ -458,6 +458,9 @@ static __u8 wiimote_cmd_read_ext(struct wiimote_data *wdata, __u8 *rmem) if (rmem[0] == 0x00 && rmem[1] == 0x00 && rmem[4] == 0x01 && rmem[5] == 0x03) return WIIMOTE_EXT_GUITAR; + if (rmem[0] == 0x03 && rmem[1] == 0x00 && + rmem[4] == 0x01 && rmem[5] == 0x03) + return WIIMOTE_EXT_TURNTABLE; return WIIMOTE_EXT_UNKNOWN; } @@ -495,6 +498,7 @@ static bool wiimote_cmd_map_mp(struct wiimote_data *wdata, __u8 exttype) case WIIMOTE_EXT_GUITAR: wmem = 0x07; break; + case WIIMOTE_EXT_TURNTABLE: case WIIMOTE_EXT_NUNCHUK: wmem = 0x05; break; @@ -1082,6 +1086,7 @@ static const char *wiimote_exttype_names[WIIMOTE_EXT_NUM] = { [WIIMOTE_EXT_PRO_CONTROLLER] = "Nintendo Wii U Pro Controller", [WIIMOTE_EXT_DRUMS] = "Nintendo Wii Drums", [WIIMOTE_EXT_GUITAR] = "Nintendo Wii Guitar", + [WIIMOTE_EXT_TURNTABLE] = "Nintendo Wii Turntable" }; /* @@ -1669,6 +1674,8 @@ static ssize_t wiimote_ext_show(struct device *dev, return sprintf(buf, "drums\n"); case WIIMOTE_EXT_GUITAR: return sprintf(buf, "guitar\n"); + case WIIMOTE_EXT_TURNTABLE: + return sprintf(buf, "turntable\n"); case WIIMOTE_EXT_UNKNOWN: default: return sprintf(buf, "unknown\n"); diff --git a/drivers/hid/hid-wiimote-modules.c b/drivers/hid/hid-wiimote-modules.c index 213c58bf24951..dbccdfa639167 100644 --- a/drivers/hid/hid-wiimote-modules.c +++ b/drivers/hid/hid-wiimote-modules.c @@ -2403,6 +2403,230 @@ static const struct wiimod_ops wiimod_guitar = { .in_ext = wiimod_guitar_in_ext, }; +/* + * Turntable + * DJ Hero came with a Turntable Controller that was plugged in + * as an extension. + * We create a separate device for turntables and report all information via this + * input device. +*/ + +enum wiimod_turntable_keys { + WIIMOD_TURNTABLE_KEY_G_RIGHT, + WIIMOD_TURNTABLE_KEY_R_RIGHT, + WIIMOD_TURNTABLE_KEY_B_RIGHT, + WIIMOD_TURNTABLE_KEY_G_LEFT, + WIIMOD_TURNTABLE_KEY_R_LEFT, + WIIMOD_TURNTABLE_KEY_B_LEFT, + WIIMOD_TURNTABLE_KEY_EUPHORIA, + WIIMOD_TURNTABLE_KEY_PLUS, + WIIMOD_TURNTABLE_KEY_MINUS, + WIIMOD_TURNTABLE_KEY_NUM +}; + +static const __u16 wiimod_turntable_map[] = { + BTN_1, /* WIIMOD_TURNTABLE_KEY_G_RIGHT */ + BTN_2, /* WIIMOD_TURNTABLE_KEY_R_RIGHT */ + BTN_3, /* WIIMOD_TURNTABLE_KEY_B_RIGHT */ + BTN_4, /* WIIMOD_TURNTABLE_KEY_G_LEFT */ + BTN_5, /* WIIMOD_TURNTABLE_KEY_R_LEFT */ + BTN_6, /* WIIMOD_TURNTABLE_KEY_B_LEFT */ + BTN_7, /* WIIMOD_TURNTABLE_KEY_EUPHORIA */ + BTN_START, /* WIIMOD_TURNTABLE_KEY_PLUS */ + BTN_SELECT, /* WIIMOD_TURNTABLE_KEY_MINUS */ +}; + +static void wiimod_turntable_in_ext(struct wiimote_data *wdata, const __u8 *ext) +{ + __u8 be, cs, sx, sy, ed, rtt, rbg, rbr, rbb, ltt, lbg, lbr, lbb, bp, bm; + /* + * Byte | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0 | + *------+------+-----+-----+-----+-----+------+------+--------+ + * 0 | RTT<4:3> | SX <5:0> | + * 1 | RTT<2:1> | SY <5:0> | + *------+------+-----+-----+-----+-----+------+------+--------+ + * 2 |RTT<0>| ED<4:3> | CS<3:0> | RTT<5> | + *------+------+-----+-----+-----+-----+------+------+--------+ + * 3 | ED<2:0> | LTT<4:0> | + *------+------+-----+-----+-----+-----+------+------+--------+ + * 4 | 0 | 0 | LBR | B- | 0 | B+ | RBR | LTT<5> | + *------+------+-----+-----+-----+-----+------+------+--------+ + * 5 | LBB | 0 | RBG | BE | LBG | RBB | 0 | 0 | + *------+------+-----+-----+-----+-----+------+------+--------+ + * All pressed buttons are 0 + * + * With Motion+ enabled, it will look like this: + * Byte | 8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | + *------+------+-----+-----+-----+-----+------+------+--------+ + * 1 | RTT<4:3> | SX <5:1> | 0 | + * 2 | RTT<2:1> | SY <5:1> | 0 | + *------+------+-----+-----+-----+-----+------+------+--------+ + * 3 |RTT<0>| ED<4:3> | CS<3:0> | RTT<5> | + *------+------+-----+-----+-----+-----+------+------+--------+ + * 4 | ED<2:0> | LTT<4:0> | + *------+------+-----+-----+-----+-----+------+------+--------+ + * 5 | 0 | 0 | LBR | B- | 0 | B+ | RBR | XXXX | + *------+------+-----+-----+-----+-----+------+------+--------+ + * 6 | LBB | 0 | RBG | BE | LBG | RBB | XXXX | XXXX | + *------+------+-----+-----+-----+-----+------+------+--------+ + */ + + be = !(ext[5] & 0x10); + cs = ((ext[2] & 0x1e)); + sx = ext[0] & 0x3f; + sy = ext[1] & 0x3f; + ed = (ext[3] & 0xe0) >> 5; + rtt = ((ext[2] & 0x01) << 5 | (ext[0] & 0xc0) >> 3 | (ext[1] & 0xc0) >> 5 | ( ext[2] & 0x80 ) >> 7); + ltt = ((ext[4] & 0x01) << 5 | (ext[3] & 0x1f)); + rbg = !(ext[5] & 0x20); + rbr = !(ext[4] & 0x02); + rbb = !(ext[5] & 0x04); + lbg = !(ext[5] & 0x08); + lbb = !(ext[5] & 0x80); + lbr = !(ext[4] & 0x20); + bm = !(ext[4] & 0x10); + bp = !(ext[4] & 0x04); + + if (wdata->state.flags & WIIPROTO_FLAG_MP_ACTIVE) { + ltt = (ext[4] & 0x01) << 5; + sx &= 0x3e; + sy &= 0x3e; + } + + input_report_abs(wdata->extension.input, ABS_X, sx); + input_report_abs(wdata->extension.input, ABS_Y, sy); + input_report_abs(wdata->extension.input, ABS_HAT0X, rtt); + input_report_abs(wdata->extension.input, ABS_HAT1X, ltt); + input_report_abs(wdata->extension.input, ABS_HAT2X, cs); + input_report_abs(wdata->extension.input, ABS_HAT3X, ed); + input_report_key(wdata->extension.input, + wiimod_turntable_map[WIIMOD_TURNTABLE_KEY_G_RIGHT], + rbg); + input_report_key(wdata->extension.input, + wiimod_turntable_map[WIIMOD_TURNTABLE_KEY_R_RIGHT], + rbr); + input_report_key(wdata->extension.input, + wiimod_turntable_map[WIIMOD_TURNTABLE_KEY_B_RIGHT], + rbb); + input_report_key(wdata->extension.input, + wiimod_turntable_map[WIIMOD_TURNTABLE_KEY_G_LEFT], + lbg); + input_report_key(wdata->extension.input, + wiimod_turntable_map[WIIMOD_TURNTABLE_KEY_R_LEFT], + lbr); + input_report_key(wdata->extension.input, + wiimod_turntable_map[WIIMOD_TURNTABLE_KEY_B_LEFT], + lbb); + input_report_key(wdata->extension.input, + wiimod_turntable_map[WIIMOD_TURNTABLE_KEY_EUPHORIA], + be); + input_report_key(wdata->extension.input, + wiimod_turntable_map[WIIMOD_TURNTABLE_KEY_PLUS], + bp); + input_report_key(wdata->extension.input, + wiimod_turntable_map[WIIMOD_TURNTABLE_KEY_MINUS], + bm); + + input_sync(wdata->extension.input); +} + +static int wiimod_turntable_open(struct input_dev *dev) +{ + struct wiimote_data *wdata = input_get_drvdata(dev); + unsigned long flags; + + spin_lock_irqsave(&wdata->state.lock, flags); + wdata->state.flags |= WIIPROTO_FLAG_EXT_USED; + wiiproto_req_drm(wdata, WIIPROTO_REQ_NULL); + spin_unlock_irqrestore(&wdata->state.lock, flags); + + return 0; +} + +static void wiimod_turntable_close(struct input_dev *dev) +{ + struct wiimote_data *wdata = input_get_drvdata(dev); + unsigned long flags; + + spin_lock_irqsave(&wdata->state.lock, flags); + wdata->state.flags &= ~WIIPROTO_FLAG_EXT_USED; + wiiproto_req_drm(wdata, WIIPROTO_REQ_NULL); + spin_unlock_irqrestore(&wdata->state.lock, flags); +} + +static int wiimod_turntable_probe(const struct wiimod_ops *ops, + struct wiimote_data *wdata) +{ + int ret, i; + + wdata->extension.input = input_allocate_device(); + if (!wdata->extension.input) + return -ENOMEM; + + input_set_drvdata(wdata->extension.input, wdata); + wdata->extension.input->open = wiimod_turntable_open; + wdata->extension.input->close = wiimod_turntable_close; + wdata->extension.input->dev.parent = &wdata->hdev->dev; + wdata->extension.input->id.bustype = wdata->hdev->bus; + wdata->extension.input->id.vendor = wdata->hdev->vendor; + wdata->extension.input->id.product = wdata->hdev->product; + wdata->extension.input->id.version = wdata->hdev->version; + wdata->extension.input->name = WIIMOTE_NAME " Turntable"; + + set_bit(EV_KEY, wdata->extension.input->evbit); + for (i = 0; i < WIIMOD_TURNTABLE_KEY_NUM; ++i) + set_bit(wiimod_turntable_map[i], + wdata->extension.input->keybit); + + set_bit(EV_ABS, wdata->extension.input->evbit); + set_bit(ABS_X, wdata->extension.input->absbit); + set_bit(ABS_Y, wdata->extension.input->absbit); + set_bit(ABS_HAT0X, wdata->extension.input->absbit); + set_bit(ABS_HAT1X, wdata->extension.input->absbit); + set_bit(ABS_HAT2X, wdata->extension.input->absbit); + set_bit(ABS_HAT3X, wdata->extension.input->absbit); + input_set_abs_params(wdata->extension.input, + ABS_X, 0, 63, 1, 0); + input_set_abs_params(wdata->extension.input, + ABS_Y, 63, 0, 1, 0); + input_set_abs_params(wdata->extension.input, + ABS_HAT0X, -8, 8, 0, 0); + input_set_abs_params(wdata->extension.input, + ABS_HAT1X, -8, 8, 0, 0); + input_set_abs_params(wdata->extension.input, + ABS_HAT2X, 0, 31, 1, 1); + input_set_abs_params(wdata->extension.input, + ABS_HAT3X, 0, 7, 0, 0); + ret = input_register_device(wdata->extension.input); + if (ret) + goto err_free; + + return 0; + +err_free: + input_free_device(wdata->extension.input); + wdata->extension.input = NULL; + return ret; +} + +static void wiimod_turntable_remove(const struct wiimod_ops *ops, + struct wiimote_data *wdata) +{ + if (!wdata->extension.input) + return; + + input_unregister_device(wdata->extension.input); + wdata->extension.input = NULL; +} + +static const struct wiimod_ops wiimod_turntable = { + .flags = 0, + .arg = 0, + .probe = wiimod_turntable_probe, + .remove = wiimod_turntable_remove, + .in_ext = wiimod_turntable_in_ext, +}; + /* * Builtin Motion Plus * This module simply sets the WIIPROTO_FLAG_BUILTIN_MP protocol flag which @@ -2657,4 +2881,5 @@ const struct wiimod_ops *wiimod_ext_table[WIIMOTE_EXT_NUM] = { [WIIMOTE_EXT_PRO_CONTROLLER] = &wiimod_pro, [WIIMOTE_EXT_DRUMS] = &wiimod_drums, [WIIMOTE_EXT_GUITAR] = &wiimod_guitar, + [WIIMOTE_EXT_TURNTABLE] = &wiimod_turntable, }; diff --git a/drivers/hid/hid-wiimote.h b/drivers/hid/hid-wiimote.h index ad4ff837f43ec..9c12f63f6dd2d 100644 --- a/drivers/hid/hid-wiimote.h +++ b/drivers/hid/hid-wiimote.h @@ -88,6 +88,7 @@ enum wiimote_exttype { WIIMOTE_EXT_PRO_CONTROLLER, WIIMOTE_EXT_DRUMS, WIIMOTE_EXT_GUITAR, + WIIMOTE_EXT_TURNTABLE, WIIMOTE_EXT_NUM, }; From 037c1aaeb96fe5f778026f4c1ef28b26cf600bfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Exp=C3=B3sito?= Date: Sat, 29 Oct 2022 18:12:39 +0200 Subject: [PATCH 13/51] HID: input: do not query XP-PEN Deco LW battery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The XP-PEN Deco LW drawing tablet can be connected by USB cable or using a USB Bluetooth dongle. When it is connected using the dongle, there might be a small delay until the tablet is paired with the dongle. Fetching the device battery during this delay results in random battery percentage values. Add a quirk to avoid actively querying the battery percentage and wait for the device to report it on its own. Reported-by: Mia Kanashi Tested-by: Mia Kanashi Signed-off-by: José Expósito Signed-off-by: Jiri Kosina --- drivers/hid/hid-input.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c index 859aeb07542e3..d728a94c642eb 100644 --- a/drivers/hid/hid-input.c +++ b/drivers/hid/hid-input.c @@ -340,6 +340,7 @@ static enum power_supply_property hidinput_battery_props[] = { #define HID_BATTERY_QUIRK_PERCENT (1 << 0) /* always reports percent */ #define HID_BATTERY_QUIRK_FEATURE (1 << 1) /* ask for feature report */ #define HID_BATTERY_QUIRK_IGNORE (1 << 2) /* completely ignore the battery */ +#define HID_BATTERY_QUIRK_AVOID_QUERY (1 << 3) /* do not query the battery */ static const struct hid_device_id hid_battery_quirks[] = { { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, @@ -373,6 +374,8 @@ static const struct hid_device_id hid_battery_quirks[] = { HID_BATTERY_QUIRK_IGNORE }, { HID_USB_DEVICE(USB_VENDOR_ID_ELAN, USB_DEVICE_ID_ASUS_UX550VE_TOUCHSCREEN), HID_BATTERY_QUIRK_IGNORE }, + { HID_USB_DEVICE(USB_VENDOR_ID_UGEE, USB_DEVICE_ID_UGEE_XPPEN_TABLET_DECO_L), + HID_BATTERY_QUIRK_AVOID_QUERY }, { HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, I2C_DEVICE_ID_HP_ENVY_X360_15), HID_BATTERY_QUIRK_IGNORE }, { HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, I2C_DEVICE_ID_HP_ENVY_X360_15T_DR100), @@ -554,6 +557,9 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type, dev->battery_avoid_query = report_type == HID_INPUT_REPORT && field->physical == HID_DG_STYLUS; + if (quirks & HID_BATTERY_QUIRK_AVOID_QUERY) + dev->battery_avoid_query = true; + dev->battery = power_supply_register(&dev->dev, psy_desc, &psy_cfg); if (IS_ERR(dev->battery)) { error = PTR_ERR(dev->battery); From f9ce4db0ec2b6301f4264ba8627863e2632c922b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Exp=C3=B3sito?= Date: Sat, 29 Oct 2022 18:12:40 +0200 Subject: [PATCH 14/51] HID: uclogic: Add support for XP-PEN Deco LW MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The XP-PEN Deco LW is a UGEE v2 device with a frame with 8 buttons. Its pen has 2 buttons, supports tilt and pressure. It can be connected by USB cable or using a USB Bluetooth dongle to use it in wireless mode. When it is connected using the dongle, the device battery is used to power it. Its vendor, product and version are identical to the Deco L. The only difference reported by its firmware is the product name. In order to add support for battery reporting, add a new HID descriptor and a quirk to detect the wireless version of the tablet. Link: https://github.com/DIGImend/digimend-kernel-drivers/issues/635 Tested-by: Mia Kanashi Tested-by: Andreas Grosse Tested-by: Mia Kanashi Signed-off-by: José Expósito Signed-off-by: Jiri Kosina --- drivers/hid/hid-uclogic-params.c | 73 ++++++++++++++++++++++++++++++++ drivers/hid/hid-uclogic-rdesc.c | 34 +++++++++++++++ drivers/hid/hid-uclogic-rdesc.h | 7 +++ 3 files changed, 114 insertions(+) diff --git a/drivers/hid/hid-uclogic-params.c b/drivers/hid/hid-uclogic-params.c index 34fa991e6267e..cd1233d7e2535 100644 --- a/drivers/hid/hid-uclogic-params.c +++ b/drivers/hid/hid-uclogic-params.c @@ -18,6 +18,7 @@ #include "usbhid/usbhid.h" #include "hid-ids.h" #include +#include #include /** @@ -1211,6 +1212,69 @@ static int uclogic_params_ugee_v2_init_frame_mouse(struct uclogic_params *p) return rc; } +/** + * uclogic_params_ugee_v2_has_battery() - check whether a UGEE v2 device has + * battery or not. + * @hdev: The HID device of the tablet interface. + * + * Returns: + * True if the device has battery, false otherwise. + */ +static bool uclogic_params_ugee_v2_has_battery(struct hid_device *hdev) +{ + /* The XP-PEN Deco LW vendor, product and version are identical to the + * Deco L. The only difference reported by their firmware is the product + * name. Add a quirk to support battery reporting on the wireless + * version. + */ + if (hdev->vendor == USB_VENDOR_ID_UGEE && + hdev->product == USB_DEVICE_ID_UGEE_XPPEN_TABLET_DECO_L) { + struct usb_device *udev = hid_to_usb_dev(hdev); + + if (strstarts(udev->product, "Deco LW")) + return true; + } + + return false; +} + +/** + * uclogic_params_ugee_v2_init_battery() - initialize UGEE v2 battery reporting. + * @hdev: The HID device of the tablet interface, cannot be NULL. + * @p: Parameters to fill in, cannot be NULL. + * + * Returns: + * Zero, if successful. A negative errno code on error. + */ +static int uclogic_params_ugee_v2_init_battery(struct hid_device *hdev, + struct uclogic_params *p) +{ + int rc = 0; + + if (!hdev || !p) + return -EINVAL; + + /* Some tablets contain invalid characters in hdev->uniq, throwing a + * "hwmon: '' is not a valid name attribute, please fix" error. + * Use the device vendor and product IDs instead. + */ + snprintf(hdev->uniq, sizeof(hdev->uniq), "%x-%x", hdev->vendor, + hdev->product); + + rc = uclogic_params_frame_init_with_desc(&p->frame_list[1], + uclogic_rdesc_ugee_v2_battery_template_arr, + uclogic_rdesc_ugee_v2_battery_template_size, + UCLOGIC_RDESC_UGEE_V2_BATTERY_ID); + if (rc) + return rc; + + p->frame_list[1].suffix = "Battery"; + p->pen.subreport_list[1].value = 0xf2; + p->pen.subreport_list[1].id = UCLOGIC_RDESC_UGEE_V2_BATTERY_ID; + + return rc; +} + /** * uclogic_params_ugee_v2_init() - initialize a UGEE graphics tablets by * discovering their parameters. @@ -1334,6 +1398,15 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params, if (rc) goto cleanup; + /* Initialize the battery interface*/ + if (uclogic_params_ugee_v2_has_battery(hdev)) { + rc = uclogic_params_ugee_v2_init_battery(hdev, &p); + if (rc) { + hid_err(hdev, "error initializing battery: %d\n", rc); + goto cleanup; + } + } + output: /* Output parameters */ memcpy(params, &p, sizeof(*params)); diff --git a/drivers/hid/hid-uclogic-rdesc.c b/drivers/hid/hid-uclogic-rdesc.c index 4bd54c4fb5b0b..6524b4b61b25c 100644 --- a/drivers/hid/hid-uclogic-rdesc.c +++ b/drivers/hid/hid-uclogic-rdesc.c @@ -1035,6 +1035,40 @@ const __u8 uclogic_rdesc_ugee_v2_frame_mouse_template_arr[] = { const size_t uclogic_rdesc_ugee_v2_frame_mouse_template_size = sizeof(uclogic_rdesc_ugee_v2_frame_mouse_template_arr); +/* Fixed report descriptor template for UGEE v2 battery reports */ +const __u8 uclogic_rdesc_ugee_v2_battery_template_arr[] = { + 0x05, 0x01, /* Usage Page (Desktop), */ + 0x09, 0x07, /* Usage (Keypad), */ + 0xA1, 0x01, /* Collection (Application), */ + 0x85, UCLOGIC_RDESC_UGEE_V2_BATTERY_ID, + /* Report ID, */ + 0x75, 0x08, /* Report Size (8), */ + 0x95, 0x02, /* Report Count (2), */ + 0x81, 0x01, /* Input (Constant), */ + 0x05, 0x84, /* Usage Page (Power Device), */ + 0x05, 0x85, /* Usage Page (Battery System), */ + 0x09, 0x65, /* Usage Page (AbsoluteStateOfCharge), */ + 0x75, 0x08, /* Report Size (8), */ + 0x95, 0x01, /* Report Count (1), */ + 0x15, 0x00, /* Logical Minimum (0), */ + 0x26, 0xff, 0x00, /* Logical Maximum (255), */ + 0x81, 0x02, /* Input (Variable), */ + 0x75, 0x01, /* Report Size (1), */ + 0x95, 0x01, /* Report Count (1), */ + 0x15, 0x00, /* Logical Minimum (0), */ + 0x25, 0x01, /* Logical Maximum (1), */ + 0x09, 0x44, /* Usage Page (Charging), */ + 0x81, 0x02, /* Input (Variable), */ + 0x95, 0x07, /* Report Count (7), */ + 0x81, 0x01, /* Input (Constant), */ + 0x75, 0x08, /* Report Size (8), */ + 0x95, 0x07, /* Report Count (7), */ + 0x81, 0x01, /* Input (Constant), */ + 0xC0 /* End Collection */ +}; +const size_t uclogic_rdesc_ugee_v2_battery_template_size = + sizeof(uclogic_rdesc_ugee_v2_battery_template_arr); + /* Fixed report descriptor for Ugee EX07 frame */ const __u8 uclogic_rdesc_ugee_ex07_frame_arr[] = { 0x05, 0x01, /* Usage Page (Desktop), */ diff --git a/drivers/hid/hid-uclogic-rdesc.h b/drivers/hid/hid-uclogic-rdesc.h index 0502a06564964..a1f78c07293ff 100644 --- a/drivers/hid/hid-uclogic-rdesc.h +++ b/drivers/hid/hid-uclogic-rdesc.h @@ -161,6 +161,9 @@ extern const size_t uclogic_rdesc_v2_frame_dial_size; /* Device ID byte offset in v2 frame dial reports */ #define UCLOGIC_RDESC_V2_FRAME_DIAL_DEV_ID_BYTE 0x4 +/* Report ID for tweaked UGEE v2 battery reports */ +#define UCLOGIC_RDESC_UGEE_V2_BATTERY_ID 0xba + /* Fixed report descriptor template for UGEE v2 pen reports */ extern const __u8 uclogic_rdesc_ugee_v2_pen_template_arr[]; extern const size_t uclogic_rdesc_ugee_v2_pen_template_size; @@ -177,6 +180,10 @@ extern const size_t uclogic_rdesc_ugee_v2_frame_dial_template_size; extern const __u8 uclogic_rdesc_ugee_v2_frame_mouse_template_arr[]; extern const size_t uclogic_rdesc_ugee_v2_frame_mouse_template_size; +/* Fixed report descriptor template for UGEE v2 battery reports */ +extern const __u8 uclogic_rdesc_ugee_v2_battery_template_arr[]; +extern const size_t uclogic_rdesc_ugee_v2_battery_template_size; + /* Fixed report descriptor for Ugee EX07 frame */ extern const __u8 uclogic_rdesc_ugee_ex07_frame_arr[]; extern const size_t uclogic_rdesc_ugee_ex07_frame_size; From f55b9b56cefb1c4a7422e2b1fcb3a84ae4cd74d1 Mon Sep 17 00:00:00 2001 From: Roderick Colenbrander Date: Sat, 29 Oct 2022 11:48:39 -0700 Subject: [PATCH 15/51] HID: playstation: initial DualShock4 USB support. Add basic support for DualShock4 USB controller with buttons and sticks. Signed-off-by: Roderick Colenbrander Signed-off-by: Jiri Kosina --- drivers/hid/hid-playstation.c | 174 +++++++++++++++++++++++++++++++++- 1 file changed, 172 insertions(+), 2 deletions(-) diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c index 0b58763bfd301..04d03d2e40dbc 100644 --- a/drivers/hid/hid-playstation.c +++ b/drivers/hid/hid-playstation.c @@ -2,7 +2,7 @@ /* * HID driver for Sony DualSense(TM) controller. * - * Copyright (c) 2020 Sony Interactive Entertainment + * Copyright (c) 2020-2022 Sony Interactive Entertainment */ #include @@ -283,6 +283,35 @@ struct dualsense_output_report { struct dualsense_output_report_common *common; }; +#define DS4_INPUT_REPORT_USB 0x01 +#define DS4_INPUT_REPORT_USB_SIZE 64 + +#define DS4_FEATURE_REPORT_PAIRING_INFO 0x12 +#define DS4_FEATURE_REPORT_PAIRING_INFO_SIZE 16 + +struct dualshock4 { + struct ps_device base; + struct input_dev *gamepad; +}; + +/* Main DualShock4 input report excluding any BT/USB specific headers. */ +struct dualshock4_input_report_common { + uint8_t x, y; + uint8_t rx, ry; + uint8_t buttons[3]; + uint8_t z, rz; + + uint8_t reserved[20]; +} __packed; +static_assert(sizeof(struct dualshock4_input_report_common) == 29); + +struct dualshock4_input_report_usb { + uint8_t report_id; /* 0x01 */ + struct dualshock4_input_report_common common; + uint8_t reserved[34]; +} __packed; +static_assert(sizeof(struct dualshock4_input_report_usb) == DS4_INPUT_REPORT_USB_SIZE); + /* * Common gamepad buttons across DualShock 3 / 4 and DualSense. * Note: for device with a touchpad, touchpad button is not included @@ -1465,6 +1494,135 @@ static struct ps_device *dualsense_create(struct hid_device *hdev) return ERR_PTR(ret); } +static int dualshock4_get_mac_address(struct dualshock4 *ds4) +{ + uint8_t *buf; + int ret = 0; + + buf = kzalloc(DS4_FEATURE_REPORT_PAIRING_INFO_SIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; + + ret = ps_get_report(ds4->base.hdev, DS4_FEATURE_REPORT_PAIRING_INFO, buf, + DS4_FEATURE_REPORT_PAIRING_INFO_SIZE); + if (ret) { + hid_err(ds4->base.hdev, "Failed to retrieve DualShock4 pairing info: %d\n", ret); + goto err_free; + } + + memcpy(ds4->base.mac_address, &buf[1], sizeof(ds4->base.mac_address)); + +err_free: + kfree(buf); + return ret; +} + +static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *report, + u8 *data, int size) +{ + struct hid_device *hdev = ps_dev->hdev; + struct dualshock4 *ds4 = container_of(ps_dev, struct dualshock4, base); + struct dualshock4_input_report_common *ds4_report; + uint8_t value; + + /* + * DualShock4 in USB uses the full HID report for reportID 1, but + * Bluetooth uses a minimal HID report for reportID 1 and reports + * the full report using reportID 17. + */ + if (hdev->bus == BUS_USB && report->id == DS4_INPUT_REPORT_USB && + size == DS4_INPUT_REPORT_USB_SIZE) { + struct dualshock4_input_report_usb *usb = (struct dualshock4_input_report_usb *)data; + + ds4_report = &usb->common; + } else { + hid_err(hdev, "Unhandled reportID=%d\n", report->id); + return -1; + } + + input_report_abs(ds4->gamepad, ABS_X, ds4_report->x); + input_report_abs(ds4->gamepad, ABS_Y, ds4_report->y); + input_report_abs(ds4->gamepad, ABS_RX, ds4_report->rx); + input_report_abs(ds4->gamepad, ABS_RY, ds4_report->ry); + input_report_abs(ds4->gamepad, ABS_Z, ds4_report->z); + input_report_abs(ds4->gamepad, ABS_RZ, ds4_report->rz); + + value = ds4_report->buttons[0] & DS_BUTTONS0_HAT_SWITCH; + if (value >= ARRAY_SIZE(ps_gamepad_hat_mapping)) + value = 8; /* center */ + input_report_abs(ds4->gamepad, ABS_HAT0X, ps_gamepad_hat_mapping[value].x); + input_report_abs(ds4->gamepad, ABS_HAT0Y, ps_gamepad_hat_mapping[value].y); + + input_report_key(ds4->gamepad, BTN_WEST, ds4_report->buttons[0] & DS_BUTTONS0_SQUARE); + input_report_key(ds4->gamepad, BTN_SOUTH, ds4_report->buttons[0] & DS_BUTTONS0_CROSS); + input_report_key(ds4->gamepad, BTN_EAST, ds4_report->buttons[0] & DS_BUTTONS0_CIRCLE); + input_report_key(ds4->gamepad, BTN_NORTH, ds4_report->buttons[0] & DS_BUTTONS0_TRIANGLE); + input_report_key(ds4->gamepad, BTN_TL, ds4_report->buttons[1] & DS_BUTTONS1_L1); + input_report_key(ds4->gamepad, BTN_TR, ds4_report->buttons[1] & DS_BUTTONS1_R1); + input_report_key(ds4->gamepad, BTN_TL2, ds4_report->buttons[1] & DS_BUTTONS1_L2); + input_report_key(ds4->gamepad, BTN_TR2, ds4_report->buttons[1] & DS_BUTTONS1_R2); + input_report_key(ds4->gamepad, BTN_SELECT, ds4_report->buttons[1] & DS_BUTTONS1_CREATE); + input_report_key(ds4->gamepad, BTN_START, ds4_report->buttons[1] & DS_BUTTONS1_OPTIONS); + input_report_key(ds4->gamepad, BTN_THUMBL, ds4_report->buttons[1] & DS_BUTTONS1_L3); + input_report_key(ds4->gamepad, BTN_THUMBR, ds4_report->buttons[1] & DS_BUTTONS1_R3); + input_report_key(ds4->gamepad, BTN_MODE, ds4_report->buttons[2] & DS_BUTTONS2_PS_HOME); + input_sync(ds4->gamepad); + + return 0; +} + +static struct ps_device *dualshock4_create(struct hid_device *hdev) +{ + struct dualshock4 *ds4; + struct ps_device *ps_dev; + int ret; + + ds4 = devm_kzalloc(&hdev->dev, sizeof(*ds4), GFP_KERNEL); + if (!ds4) + return ERR_PTR(-ENOMEM); + + /* + * Patch version to allow userspace to distinguish between + * hid-generic vs hid-playstation axis and button mapping. + */ + hdev->version |= HID_PLAYSTATION_VERSION_PATCH; + + ps_dev = &ds4->base; + ps_dev->hdev = hdev; + spin_lock_init(&ps_dev->lock); + ps_dev->parse_report = dualshock4_parse_report; + hid_set_drvdata(hdev, ds4); + + ret = dualshock4_get_mac_address(ds4); + if (ret) { + hid_err(hdev, "Failed to get MAC address from DualShock4\n"); + return ERR_PTR(ret); + } + snprintf(hdev->uniq, sizeof(hdev->uniq), "%pMR", ds4->base.mac_address); + + ret = ps_devices_list_add(ps_dev); + if (ret) + return ERR_PTR(ret); + + ds4->gamepad = ps_gamepad_create(hdev, NULL); + if (IS_ERR(ds4->gamepad)) { + ret = PTR_ERR(ds4->gamepad); + goto err; + } + + ret = ps_device_set_player_id(ps_dev); + if (ret) { + hid_err(hdev, "Failed to assign player id for DualShock4: %d\n", ret); + goto err; + } + + return &ds4->base; + +err: + ps_devices_list_remove(ps_dev); + return ERR_PTR(ret); +} + static int ps_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data, int size) { @@ -1499,7 +1657,15 @@ static int ps_probe(struct hid_device *hdev, const struct hid_device_id *id) goto err_stop; } - if (hdev->product == USB_DEVICE_ID_SONY_PS5_CONTROLLER || + if (hdev->product == USB_DEVICE_ID_SONY_PS4_CONTROLLER || + hdev->product == USB_DEVICE_ID_SONY_PS4_CONTROLLER_2) { + dev = dualshock4_create(hdev); + if (IS_ERR(dev)) { + hid_err(hdev, "Failed to create dualshock4.\n"); + ret = PTR_ERR(dev); + goto err_close; + } + } else if (hdev->product == USB_DEVICE_ID_SONY_PS5_CONTROLLER || hdev->product == USB_DEVICE_ID_SONY_PS5_CONTROLLER_2) { dev = dualsense_create(hdev); if (IS_ERR(dev)) { @@ -1533,6 +1699,10 @@ static void ps_remove(struct hid_device *hdev) } static const struct hid_device_id ps_devices[] = { + /* Sony DualShock 4 controllers for PS4 */ + { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER) }, + { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_2) }, + /* Sony DualSense controllers for PS5 */ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER) }, { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER) }, { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER_2) }, From 9a62280a1bdc71e89ea44395bd03829ad896c447 Mon Sep 17 00:00:00 2001 From: Roderick Colenbrander Date: Sat, 29 Oct 2022 11:48:40 -0700 Subject: [PATCH 16/51] HID: playstation: report DualShock4 hardware and firmware version. Report DualShock4 hardware and firmware version info through sysfs. It uses the same sysfs nodes as the DualSense did (and hid-sony). Signed-off-by: Roderick Colenbrander Signed-off-by: Jiri Kosina --- drivers/hid/hid-playstation.c | 38 +++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c index 04d03d2e40dbc..df4353a4f1e9c 100644 --- a/drivers/hid/hid-playstation.c +++ b/drivers/hid/hid-playstation.c @@ -286,6 +286,8 @@ struct dualsense_output_report { #define DS4_INPUT_REPORT_USB 0x01 #define DS4_INPUT_REPORT_USB_SIZE 64 +#define DS4_FEATURE_REPORT_FIRMWARE_INFO 0xa3 +#define DS4_FEATURE_REPORT_FIRMWARE_INFO_SIZE 49 #define DS4_FEATURE_REPORT_PAIRING_INFO 0x12 #define DS4_FEATURE_REPORT_PAIRING_INFO_SIZE 16 @@ -1494,6 +1496,30 @@ static struct ps_device *dualsense_create(struct hid_device *hdev) return ERR_PTR(ret); } +static int dualshock4_get_firmware_info(struct dualshock4 *ds4) +{ + uint8_t *buf; + int ret; + + buf = kzalloc(DS4_FEATURE_REPORT_FIRMWARE_INFO_SIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; + + ret = ps_get_report(ds4->base.hdev, DS4_FEATURE_REPORT_FIRMWARE_INFO, buf, + DS4_FEATURE_REPORT_FIRMWARE_INFO_SIZE); + if (ret) { + hid_err(ds4->base.hdev, "Failed to retrieve DualShock4 firmware info: %d\n", ret); + goto err_free; + } + + ds4->base.hw_version = get_unaligned_le16(&buf[35]); + ds4->base.fw_version = get_unaligned_le16(&buf[41]); + +err_free: + kfree(buf); + return ret; +} + static int dualshock4_get_mac_address(struct dualshock4 *ds4) { uint8_t *buf; @@ -1600,6 +1626,12 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev) } snprintf(hdev->uniq, sizeof(hdev->uniq), "%pMR", ds4->base.mac_address); + ret = dualshock4_get_firmware_info(ds4); + if (ret) { + hid_err(hdev, "Failed to get firmware info from DualShock4\n"); + return ERR_PTR(ret); + } + ret = ps_devices_list_add(ps_dev); if (ret) return ERR_PTR(ret); @@ -1616,6 +1648,12 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev) goto err; } + /* + * Reporting hardware and firmware is important as there are frequent updates, which + * can change behavior. + */ + hid_info(hdev, "Registered DualShock4 controller hw_version=0x%08x fw_version=0x%08x\n", + ds4->base.hw_version, ds4->base.fw_version); return &ds4->base; err: From 8871ed304e6938361c28b3c86f64a4190dad7dbb Mon Sep 17 00:00:00 2001 From: Roderick Colenbrander Date: Sat, 29 Oct 2022 11:48:41 -0700 Subject: [PATCH 17/51] HID: playstation: add DualShock4 battery support. Provide DualShock4 battery support through powersupply framework. Signed-off-by: Roderick Colenbrander Signed-off-by: Jiri Kosina --- drivers/hid/hid-playstation.c | 70 +++++++++++++++++++++++++++++++++-- 1 file changed, 66 insertions(+), 4 deletions(-) diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c index df4353a4f1e9c..20111aa20e86a 100644 --- a/drivers/hid/hid-playstation.c +++ b/drivers/hid/hid-playstation.c @@ -291,6 +291,12 @@ struct dualsense_output_report { #define DS4_FEATURE_REPORT_PAIRING_INFO 0x12 #define DS4_FEATURE_REPORT_PAIRING_INFO_SIZE 16 +/* Status field of DualShock4 input report. */ +#define DS4_STATUS0_BATTERY_CAPACITY GENMASK(3, 0) +#define DS4_STATUS0_CABLE_STATE BIT(4) +/* Battery status within batery_status field. */ +#define DS4_BATTERY_STATUS_FULL 11 + struct dualshock4 { struct ps_device base; struct input_dev *gamepad; @@ -302,15 +308,16 @@ struct dualshock4_input_report_common { uint8_t rx, ry; uint8_t buttons[3]; uint8_t z, rz; - uint8_t reserved[20]; + uint8_t status[2]; + uint8_t reserved2; } __packed; -static_assert(sizeof(struct dualshock4_input_report_common) == 29); +static_assert(sizeof(struct dualshock4_input_report_common) == 32); struct dualshock4_input_report_usb { uint8_t report_id; /* 0x01 */ struct dualshock4_input_report_common common; - uint8_t reserved[34]; + uint8_t reserved[31]; } __packed; static_assert(sizeof(struct dualshock4_input_report_usb) == DS4_INPUT_REPORT_USB_SIZE); @@ -1549,7 +1556,9 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report * struct hid_device *hdev = ps_dev->hdev; struct dualshock4 *ds4 = container_of(ps_dev, struct dualshock4, base); struct dualshock4_input_report_common *ds4_report; - uint8_t value; + uint8_t battery_capacity, value; + int battery_status; + unsigned long flags; /* * DualShock4 in USB uses the full HID report for reportID 1, but @@ -1594,6 +1603,53 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report * input_report_key(ds4->gamepad, BTN_MODE, ds4_report->buttons[2] & DS_BUTTONS2_PS_HOME); input_sync(ds4->gamepad); + /* + * Interpretation of the battery_capacity data depends on the cable state. + * When no cable is connected (bit4 is 0): + * - 0:10: percentage in units of 10%. + * When a cable is plugged in: + * - 0-10: percentage in units of 10%. + * - 11: battery is full + * - 14: not charging due to Voltage or temperature error + * - 15: charge error + */ + if (ds4_report->status[0] & DS4_STATUS0_CABLE_STATE) { + uint8_t battery_data = ds4_report->status[0] & DS4_STATUS0_BATTERY_CAPACITY; + + if (battery_data < 10) { + /* Take the mid-point for each battery capacity value, + * because on the hardware side 0 = 0-9%, 1=10-19%, etc. + * This matches official platform behavior, which does + * the same. + */ + battery_capacity = battery_data * 10 + 5; + battery_status = POWER_SUPPLY_STATUS_CHARGING; + } else if (battery_data == 10) { + battery_capacity = 100; + battery_status = POWER_SUPPLY_STATUS_CHARGING; + } else if (battery_data == DS4_BATTERY_STATUS_FULL) { + battery_capacity = 100; + battery_status = POWER_SUPPLY_STATUS_FULL; + } else { /* 14, 15 and undefined values */ + battery_capacity = 0; + battery_status = POWER_SUPPLY_STATUS_UNKNOWN; + } + } else { + uint8_t battery_data = ds4_report->status[0] & DS4_STATUS0_BATTERY_CAPACITY; + + if (battery_data < 10) + battery_capacity = battery_data * 10 + 5; + else /* 10 */ + battery_capacity = 100; + + battery_status = POWER_SUPPLY_STATUS_DISCHARGING; + } + + spin_lock_irqsave(&ps_dev->lock, flags); + ps_dev->battery_capacity = battery_capacity; + ps_dev->battery_status = battery_status; + spin_unlock_irqrestore(&ps_dev->lock, flags); + return 0; } @@ -1616,6 +1672,8 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev) ps_dev = &ds4->base; ps_dev->hdev = hdev; spin_lock_init(&ps_dev->lock); + ps_dev->battery_capacity = 100; /* initial value until parse_report. */ + ps_dev->battery_status = POWER_SUPPLY_STATUS_UNKNOWN; ps_dev->parse_report = dualshock4_parse_report; hid_set_drvdata(hdev, ds4); @@ -1642,6 +1700,10 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev) goto err; } + ret = ps_device_register_battery(ps_dev); + if (ret) + goto err; + ret = ps_device_set_player_id(ps_dev); if (ret) { hid_err(hdev, "Failed to assign player id for DualShock4: %d\n", ret); From 752038248808a7ff176bbdb668f19ae7d2a9816b Mon Sep 17 00:00:00 2001 From: Roderick Colenbrander Date: Sat, 29 Oct 2022 11:48:42 -0700 Subject: [PATCH 18/51] HID: playstation: add DualShock4 touchpad support. Support the DualShock4 touchpad as a separate input device. The code describes the touchpad input reports through structures similar a bit to the DualSense code. Signed-off-by: Roderick Colenbrander Signed-off-by: Jiri Kosina --- drivers/hid/hid-playstation.c | 66 +++++++++++++++++++++++++++++++++-- 1 file changed, 63 insertions(+), 3 deletions(-) diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c index 20111aa20e86a..63fc3a67ea747 100644 --- a/drivers/hid/hid-playstation.c +++ b/drivers/hid/hid-playstation.c @@ -291,17 +291,43 @@ struct dualsense_output_report { #define DS4_FEATURE_REPORT_PAIRING_INFO 0x12 #define DS4_FEATURE_REPORT_PAIRING_INFO_SIZE 16 +/* + * Status of a DualShock4 touch point contact. + * Contact IDs, with highest bit set are 'inactive' + * and any associated data is then invalid. + */ +#define DS4_TOUCH_POINT_INACTIVE BIT(7) + /* Status field of DualShock4 input report. */ #define DS4_STATUS0_BATTERY_CAPACITY GENMASK(3, 0) #define DS4_STATUS0_CABLE_STATE BIT(4) /* Battery status within batery_status field. */ #define DS4_BATTERY_STATUS_FULL 11 +/* DualShock4 hardware limits */ +#define DS4_TOUCHPAD_WIDTH 1920 +#define DS4_TOUCHPAD_HEIGHT 942 + struct dualshock4 { struct ps_device base; struct input_dev *gamepad; + struct input_dev *touchpad; }; +struct dualshock4_touch_point { + uint8_t contact; + uint8_t x_lo; + uint8_t x_hi:4, y_lo:4; + uint8_t y_hi; +} __packed; +static_assert(sizeof(struct dualshock4_touch_point) == 4); + +struct dualshock4_touch_report { + uint8_t timestamp; + struct dualshock4_touch_point points[2]; +} __packed; +static_assert(sizeof(struct dualshock4_touch_report) == 9); + /* Main DualShock4 input report excluding any BT/USB specific headers. */ struct dualshock4_input_report_common { uint8_t x, y; @@ -317,7 +343,9 @@ static_assert(sizeof(struct dualshock4_input_report_common) == 32); struct dualshock4_input_report_usb { uint8_t report_id; /* 0x01 */ struct dualshock4_input_report_common common; - uint8_t reserved[31]; + uint8_t num_touch_reports; + struct dualshock4_touch_report touch_reports[3]; + uint8_t reserved[3]; } __packed; static_assert(sizeof(struct dualshock4_input_report_usb) == DS4_INPUT_REPORT_USB_SIZE); @@ -1556,8 +1584,9 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report * struct hid_device *hdev = ps_dev->hdev; struct dualshock4 *ds4 = container_of(ps_dev, struct dualshock4, base); struct dualshock4_input_report_common *ds4_report; - uint8_t battery_capacity, value; - int battery_status; + struct dualshock4_touch_report *touch_reports; + uint8_t battery_capacity, num_touch_reports, value; + int battery_status, i, j; unsigned long flags; /* @@ -1570,6 +1599,8 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report * struct dualshock4_input_report_usb *usb = (struct dualshock4_input_report_usb *)data; ds4_report = &usb->common; + num_touch_reports = usb->num_touch_reports; + touch_reports = usb->touch_reports; } else { hid_err(hdev, "Unhandled reportID=%d\n", report->id); return -1; @@ -1603,6 +1634,29 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report * input_report_key(ds4->gamepad, BTN_MODE, ds4_report->buttons[2] & DS_BUTTONS2_PS_HOME); input_sync(ds4->gamepad); + for (i = 0; i < num_touch_reports; i++) { + struct dualshock4_touch_report *touch_report = &touch_reports[i]; + + for (j = 0; j < ARRAY_SIZE(touch_report->points); j++) { + struct dualshock4_touch_point *point = &touch_report->points[j]; + bool active = (point->contact & DS4_TOUCH_POINT_INACTIVE) ? false : true; + + input_mt_slot(ds4->touchpad, j); + input_mt_report_slot_state(ds4->touchpad, MT_TOOL_FINGER, active); + + if (active) { + int x = (point->x_hi << 8) | point->x_lo; + int y = (point->y_hi << 4) | point->y_lo; + + input_report_abs(ds4->touchpad, ABS_MT_POSITION_X, x); + input_report_abs(ds4->touchpad, ABS_MT_POSITION_Y, y); + } + } + input_mt_sync_frame(ds4->touchpad); + input_sync(ds4->touchpad); + } + input_report_key(ds4->touchpad, BTN_LEFT, ds4_report->buttons[2] & DS_BUTTONS2_TOUCHPAD); + /* * Interpretation of the battery_capacity data depends on the cable state. * When no cable is connected (bit4 is 0): @@ -1700,6 +1754,12 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev) goto err; } + ds4->touchpad = ps_touchpad_create(hdev, DS4_TOUCHPAD_WIDTH, DS4_TOUCHPAD_HEIGHT, 2); + if (IS_ERR(ds4->touchpad)) { + ret = PTR_ERR(ds4->touchpad); + goto err; + } + ret = ps_device_register_battery(ps_dev); if (ret) goto err; From 12882ed83c5833880e74b1fdc92ebf64d8f97651 Mon Sep 17 00:00:00 2001 From: Roderick Colenbrander Date: Sat, 29 Oct 2022 11:48:43 -0700 Subject: [PATCH 19/51] HID: playstation: add DualShock4 accelerometer and gyroscope support. Support accelerometer and gyroscope as separate input devices similar how DualSense and hid-sony do it. Signed-off-by: Roderick Colenbrander Signed-off-by: Jiri Kosina --- drivers/hid/hid-playstation.c | 170 +++++++++++++++++++++++++++++++++- 1 file changed, 168 insertions(+), 2 deletions(-) diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c index 63fc3a67ea747..03f33dea5d938 100644 --- a/drivers/hid/hid-playstation.c +++ b/drivers/hid/hid-playstation.c @@ -286,6 +286,8 @@ struct dualsense_output_report { #define DS4_INPUT_REPORT_USB 0x01 #define DS4_INPUT_REPORT_USB_SIZE 64 +#define DS4_FEATURE_REPORT_CALIBRATION 0x02 +#define DS4_FEATURE_REPORT_CALIBRATION_SIZE 37 #define DS4_FEATURE_REPORT_FIRMWARE_INFO 0xa3 #define DS4_FEATURE_REPORT_FIRMWARE_INFO_SIZE 49 #define DS4_FEATURE_REPORT_PAIRING_INFO 0x12 @@ -305,13 +307,27 @@ struct dualsense_output_report { #define DS4_BATTERY_STATUS_FULL 11 /* DualShock4 hardware limits */ +#define DS4_ACC_RES_PER_G 8192 +#define DS4_ACC_RANGE (4*DS_ACC_RES_PER_G) +#define DS4_GYRO_RES_PER_DEG_S 1024 +#define DS4_GYRO_RANGE (2048*DS_GYRO_RES_PER_DEG_S) #define DS4_TOUCHPAD_WIDTH 1920 #define DS4_TOUCHPAD_HEIGHT 942 struct dualshock4 { struct ps_device base; struct input_dev *gamepad; + struct input_dev *sensors; struct input_dev *touchpad; + + /* Calibration data for accelerometer and gyroscope. */ + struct ps_calibration_data accel_calib_data[3]; + struct ps_calibration_data gyro_calib_data[3]; + + /* Timestamp for sensor data */ + bool sensor_timestamp_initialized; + uint32_t prev_sensor_timestamp; + uint32_t sensor_timestamp_us; }; struct dualshock4_touch_point { @@ -334,9 +350,16 @@ struct dualshock4_input_report_common { uint8_t rx, ry; uint8_t buttons[3]; uint8_t z, rz; - uint8_t reserved[20]; + + /* Motion sensors */ + __le16 sensor_timestamp; + uint8_t sensor_temperature; + __le16 gyro[3]; /* x, y, z */ + __le16 accel[3]; /* x, y, z */ + uint8_t reserved2[5]; + uint8_t status[2]; - uint8_t reserved2; + uint8_t reserved3; } __packed; static_assert(sizeof(struct dualshock4_input_report_common) == 32); @@ -1531,6 +1554,97 @@ static struct ps_device *dualsense_create(struct hid_device *hdev) return ERR_PTR(ret); } +static int dualshock4_get_calibration_data(struct dualshock4 *ds4) +{ + struct hid_device *hdev = ds4->base.hdev; + short gyro_pitch_bias, gyro_pitch_plus, gyro_pitch_minus; + short gyro_yaw_bias, gyro_yaw_plus, gyro_yaw_minus; + short gyro_roll_bias, gyro_roll_plus, gyro_roll_minus; + short gyro_speed_plus, gyro_speed_minus; + short acc_x_plus, acc_x_minus; + short acc_y_plus, acc_y_minus; + short acc_z_plus, acc_z_minus; + int speed_2x; + int range_2g; + int ret = 0; + uint8_t *buf; + + buf = kzalloc(DS4_FEATURE_REPORT_CALIBRATION_SIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; + + ret = ps_get_report(hdev, DS4_FEATURE_REPORT_CALIBRATION, buf, + DS4_FEATURE_REPORT_CALIBRATION_SIZE); + if (ret) { + hid_err(hdev, "Failed to retrieve DualShock4 calibration info: %d\n", ret); + goto err_free; + } + + gyro_pitch_bias = get_unaligned_le16(&buf[1]); + gyro_yaw_bias = get_unaligned_le16(&buf[3]); + gyro_roll_bias = get_unaligned_le16(&buf[5]); + gyro_pitch_plus = get_unaligned_le16(&buf[7]); + gyro_pitch_minus = get_unaligned_le16(&buf[9]); + gyro_yaw_plus = get_unaligned_le16(&buf[11]); + gyro_yaw_minus = get_unaligned_le16(&buf[13]); + gyro_roll_plus = get_unaligned_le16(&buf[15]); + gyro_roll_minus = get_unaligned_le16(&buf[17]); + gyro_speed_plus = get_unaligned_le16(&buf[19]); + gyro_speed_minus = get_unaligned_le16(&buf[21]); + acc_x_plus = get_unaligned_le16(&buf[23]); + acc_x_minus = get_unaligned_le16(&buf[25]); + acc_y_plus = get_unaligned_le16(&buf[27]); + acc_y_minus = get_unaligned_le16(&buf[29]); + acc_z_plus = get_unaligned_le16(&buf[31]); + acc_z_minus = get_unaligned_le16(&buf[33]); + + /* + * Set gyroscope calibration and normalization parameters. + * Data values will be normalized to 1/DS4_GYRO_RES_PER_DEG_S degree/s. + */ + speed_2x = (gyro_speed_plus + gyro_speed_minus); + ds4->gyro_calib_data[0].abs_code = ABS_RX; + ds4->gyro_calib_data[0].bias = gyro_pitch_bias; + ds4->gyro_calib_data[0].sens_numer = speed_2x*DS4_GYRO_RES_PER_DEG_S; + ds4->gyro_calib_data[0].sens_denom = gyro_pitch_plus - gyro_pitch_minus; + + ds4->gyro_calib_data[1].abs_code = ABS_RY; + ds4->gyro_calib_data[1].bias = gyro_yaw_bias; + ds4->gyro_calib_data[1].sens_numer = speed_2x*DS4_GYRO_RES_PER_DEG_S; + ds4->gyro_calib_data[1].sens_denom = gyro_yaw_plus - gyro_yaw_minus; + + ds4->gyro_calib_data[2].abs_code = ABS_RZ; + ds4->gyro_calib_data[2].bias = gyro_roll_bias; + ds4->gyro_calib_data[2].sens_numer = speed_2x*DS4_GYRO_RES_PER_DEG_S; + ds4->gyro_calib_data[2].sens_denom = gyro_roll_plus - gyro_roll_minus; + + /* + * Set accelerometer calibration and normalization parameters. + * Data values will be normalized to 1/DS4_ACC_RES_PER_G g. + */ + range_2g = acc_x_plus - acc_x_minus; + ds4->accel_calib_data[0].abs_code = ABS_X; + ds4->accel_calib_data[0].bias = acc_x_plus - range_2g / 2; + ds4->accel_calib_data[0].sens_numer = 2*DS4_ACC_RES_PER_G; + ds4->accel_calib_data[0].sens_denom = range_2g; + + range_2g = acc_y_plus - acc_y_minus; + ds4->accel_calib_data[1].abs_code = ABS_Y; + ds4->accel_calib_data[1].bias = acc_y_plus - range_2g / 2; + ds4->accel_calib_data[1].sens_numer = 2*DS4_ACC_RES_PER_G; + ds4->accel_calib_data[1].sens_denom = range_2g; + + range_2g = acc_z_plus - acc_z_minus; + ds4->accel_calib_data[2].abs_code = ABS_Z; + ds4->accel_calib_data[2].bias = acc_z_plus - range_2g / 2; + ds4->accel_calib_data[2].sens_numer = 2*DS4_ACC_RES_PER_G; + ds4->accel_calib_data[2].sens_denom = range_2g; + +err_free: + kfree(buf); + return ret; +} + static int dualshock4_get_firmware_info(struct dualshock4 *ds4) { uint8_t *buf; @@ -1587,6 +1701,7 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report * struct dualshock4_touch_report *touch_reports; uint8_t battery_capacity, num_touch_reports, value; int battery_status, i, j; + uint16_t sensor_timestamp; unsigned long flags; /* @@ -1634,6 +1749,44 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report * input_report_key(ds4->gamepad, BTN_MODE, ds4_report->buttons[2] & DS_BUTTONS2_PS_HOME); input_sync(ds4->gamepad); + /* Parse and calibrate gyroscope data. */ + for (i = 0; i < ARRAY_SIZE(ds4_report->gyro); i++) { + int raw_data = (short)le16_to_cpu(ds4_report->gyro[i]); + int calib_data = mult_frac(ds4->gyro_calib_data[i].sens_numer, + raw_data - ds4->gyro_calib_data[i].bias, + ds4->gyro_calib_data[i].sens_denom); + + input_report_abs(ds4->sensors, ds4->gyro_calib_data[i].abs_code, calib_data); + } + + /* Parse and calibrate accelerometer data. */ + for (i = 0; i < ARRAY_SIZE(ds4_report->accel); i++) { + int raw_data = (short)le16_to_cpu(ds4_report->accel[i]); + int calib_data = mult_frac(ds4->accel_calib_data[i].sens_numer, + raw_data - ds4->accel_calib_data[i].bias, + ds4->accel_calib_data[i].sens_denom); + + input_report_abs(ds4->sensors, ds4->accel_calib_data[i].abs_code, calib_data); + } + + /* Convert timestamp (in 5.33us unit) to timestamp_us */ + sensor_timestamp = le16_to_cpu(ds4_report->sensor_timestamp); + if (!ds4->sensor_timestamp_initialized) { + ds4->sensor_timestamp_us = DIV_ROUND_CLOSEST(sensor_timestamp*16, 3); + ds4->sensor_timestamp_initialized = true; + } else { + uint16_t delta; + + if (ds4->prev_sensor_timestamp > sensor_timestamp) + delta = (U16_MAX - ds4->prev_sensor_timestamp + sensor_timestamp + 1); + else + delta = sensor_timestamp - ds4->prev_sensor_timestamp; + ds4->sensor_timestamp_us += DIV_ROUND_CLOSEST(delta*16, 3); + } + ds4->prev_sensor_timestamp = sensor_timestamp; + input_event(ds4->sensors, EV_MSC, MSC_TIMESTAMP, ds4->sensor_timestamp_us); + input_sync(ds4->sensors); + for (i = 0; i < num_touch_reports; i++) { struct dualshock4_touch_report *touch_report = &touch_reports[i]; @@ -1748,12 +1901,25 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev) if (ret) return ERR_PTR(ret); + ret = dualshock4_get_calibration_data(ds4); + if (ret) { + hid_err(hdev, "Failed to get calibration data from DualShock4\n"); + goto err; + } + ds4->gamepad = ps_gamepad_create(hdev, NULL); if (IS_ERR(ds4->gamepad)) { ret = PTR_ERR(ds4->gamepad); goto err; } + ds4->sensors = ps_sensors_create(hdev, DS4_ACC_RANGE, DS4_ACC_RES_PER_G, + DS4_GYRO_RANGE, DS4_GYRO_RES_PER_DEG_S); + if (IS_ERR(ds4->sensors)) { + ret = PTR_ERR(ds4->sensors); + goto err; + } + ds4->touchpad = ps_touchpad_create(hdev, DS4_TOUCHPAD_WIDTH, DS4_TOUCHPAD_HEIGHT, 2); if (IS_ERR(ds4->touchpad)) { ret = PTR_ERR(ds4->touchpad); From 4e463ec4ba839af9758366573b32e20df1ea2e47 Mon Sep 17 00:00:00 2001 From: Roderick Colenbrander Date: Sat, 29 Oct 2022 11:48:44 -0700 Subject: [PATCH 20/51] HID: playstation: Add DualShock4 rumble support. This patch implements DualShock4 rumble support in a similar manner as the DualSense implementation. It adds an output worker with granular control of different features of the main DualShock4 output report. Signed-off-by: Roderick Colenbrander Signed-off-by: Jiri Kosina --- drivers/hid/hid-playstation.c | 148 +++++++++++++++++++++++++++++++++- 1 file changed, 147 insertions(+), 1 deletion(-) diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c index 03f33dea5d938..319f400dd9462 100644 --- a/drivers/hid/hid-playstation.c +++ b/drivers/hid/hid-playstation.c @@ -285,6 +285,8 @@ struct dualsense_output_report { #define DS4_INPUT_REPORT_USB 0x01 #define DS4_INPUT_REPORT_USB_SIZE 64 +#define DS4_OUTPUT_REPORT_USB 0x05 +#define DS4_OUTPUT_REPORT_USB_SIZE 32 #define DS4_FEATURE_REPORT_CALIBRATION 0x02 #define DS4_FEATURE_REPORT_CALIBRATION_SIZE 37 @@ -306,6 +308,9 @@ struct dualsense_output_report { /* Battery status within batery_status field. */ #define DS4_BATTERY_STATUS_FULL 11 +/* Flags for DualShock4 output report. */ +#define DS4_OUTPUT_VALID_FLAG0_MOTOR 0x01 + /* DualShock4 hardware limits */ #define DS4_ACC_RES_PER_G 8192 #define DS4_ACC_RANGE (4*DS_ACC_RES_PER_G) @@ -328,6 +333,14 @@ struct dualshock4 { bool sensor_timestamp_initialized; uint32_t prev_sensor_timestamp; uint32_t sensor_timestamp_us; + + bool update_rumble; + uint8_t motor_left; + uint8_t motor_right; + + struct work_struct output_worker; + bool output_worker_initialized; + void *output_report_dmabuf; }; struct dualshock4_touch_point { @@ -372,6 +385,45 @@ struct dualshock4_input_report_usb { } __packed; static_assert(sizeof(struct dualshock4_input_report_usb) == DS4_INPUT_REPORT_USB_SIZE); +/* Common data between Bluetooth and USB DualShock4 output reports. */ +struct dualshock4_output_report_common { + uint8_t valid_flag0; + uint8_t valid_flag1; + + uint8_t reserved; + + uint8_t motor_right; + uint8_t motor_left; + + uint8_t lightbar_red; + uint8_t lightbar_green; + uint8_t lightbar_blue; + uint8_t lightbar_blink_on; + uint8_t lightbar_blink_off; +} __packed; + +struct dualshock4_output_report_usb { + uint8_t report_id; /* 0x5 */ + struct dualshock4_output_report_common common; + uint8_t reserved[21]; +} __packed; +static_assert(sizeof(struct dualshock4_output_report_usb) == DS4_OUTPUT_REPORT_USB_SIZE); + +/* + * The DualShock4 has a main output report used to control most features. It is + * largely the same between Bluetooth and USB except for different headers and CRC. + * This structure hide the differences between the two to simplify sending output reports. + */ +struct dualshock4_output_report { + uint8_t *data; /* Start of data */ + uint8_t len; /* Size of output report */ + + /* Points to USB data payload in case for a USB report else NULL. */ + struct dualshock4_output_report_usb *usb; + /* Points to common section of report, so past any headers. */ + struct dualshock4_output_report_common *common; +}; + /* * Common gamepad buttons across DualShock 3 / 4 and DualSense. * Note: for device with a touchpad, touchpad button is not included @@ -399,6 +451,7 @@ static const struct {int x; int y; } ps_gamepad_hat_mapping[] = { }; static inline void dualsense_schedule_work(struct dualsense *ds); +static inline void dualshock4_schedule_work(struct dualshock4 *ds4); static void dualsense_set_lightbar(struct dualsense *ds, uint8_t red, uint8_t green, uint8_t blue); /* @@ -1692,6 +1745,49 @@ static int dualshock4_get_mac_address(struct dualshock4 *ds4) return ret; } +static void dualshock4_init_output_report(struct dualshock4 *ds4, + struct dualshock4_output_report *rp, void *buf) +{ + struct hid_device *hdev = ds4->base.hdev; + + if (hdev->bus == BUS_USB) { + struct dualshock4_output_report_usb *usb = buf; + + memset(usb, 0, sizeof(*usb)); + usb->report_id = DS4_OUTPUT_REPORT_USB; + + rp->data = buf; + rp->len = sizeof(*usb); + rp->usb = usb; + rp->common = &usb->common; + } +} + +static void dualshock4_output_worker(struct work_struct *work) +{ + struct dualshock4 *ds4 = container_of(work, struct dualshock4, output_worker); + struct dualshock4_output_report report; + struct dualshock4_output_report_common *common; + unsigned long flags; + + dualshock4_init_output_report(ds4, &report, ds4->output_report_dmabuf); + common = report.common; + + spin_lock_irqsave(&ds4->base.lock, flags); + + if (ds4->update_rumble) { + /* Select classic rumble style haptics and enable it. */ + common->valid_flag0 |= DS4_OUTPUT_VALID_FLAG0_MOTOR; + common->motor_left = ds4->motor_left; + common->motor_right = ds4->motor_right; + ds4->update_rumble = false; + } + + spin_unlock_irqrestore(&ds4->base.lock, flags); + + hid_hw_output_report(ds4->base.hdev, report.data, report.len); +} + static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report *report, u8 *data, int size) { @@ -1860,10 +1956,52 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report * return 0; } +static int dualshock4_play_effect(struct input_dev *dev, void *data, struct ff_effect *effect) +{ + struct hid_device *hdev = input_get_drvdata(dev); + struct dualshock4 *ds4 = hid_get_drvdata(hdev); + unsigned long flags; + + if (effect->type != FF_RUMBLE) + return 0; + + spin_lock_irqsave(&ds4->base.lock, flags); + ds4->update_rumble = true; + ds4->motor_left = effect->u.rumble.strong_magnitude / 256; + ds4->motor_right = effect->u.rumble.weak_magnitude / 256; + spin_unlock_irqrestore(&ds4->base.lock, flags); + + dualshock4_schedule_work(ds4); + return 0; +} + +static void dualshock4_remove(struct ps_device *ps_dev) +{ + struct dualshock4 *ds4 = container_of(ps_dev, struct dualshock4, base); + unsigned long flags; + + spin_lock_irqsave(&ds4->base.lock, flags); + ds4->output_worker_initialized = false; + spin_unlock_irqrestore(&ds4->base.lock, flags); + + cancel_work_sync(&ds4->output_worker); +} + +static inline void dualshock4_schedule_work(struct dualshock4 *ds4) +{ + unsigned long flags; + + spin_lock_irqsave(&ds4->base.lock, flags); + if (ds4->output_worker_initialized) + schedule_work(&ds4->output_worker); + spin_unlock_irqrestore(&ds4->base.lock, flags); +} + static struct ps_device *dualshock4_create(struct hid_device *hdev) { struct dualshock4 *ds4; struct ps_device *ps_dev; + uint8_t max_output_report_size; int ret; ds4 = devm_kzalloc(&hdev->dev, sizeof(*ds4), GFP_KERNEL); @@ -1882,8 +2020,16 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev) ps_dev->battery_capacity = 100; /* initial value until parse_report. */ ps_dev->battery_status = POWER_SUPPLY_STATUS_UNKNOWN; ps_dev->parse_report = dualshock4_parse_report; + ps_dev->remove = dualshock4_remove; + INIT_WORK(&ds4->output_worker, dualshock4_output_worker); + ds4->output_worker_initialized = true; hid_set_drvdata(hdev, ds4); + max_output_report_size = sizeof(struct dualshock4_output_report_usb); + ds4->output_report_dmabuf = devm_kzalloc(&hdev->dev, max_output_report_size, GFP_KERNEL); + if (!ds4->output_report_dmabuf) + return ERR_PTR(-ENOMEM); + ret = dualshock4_get_mac_address(ds4); if (ret) { hid_err(hdev, "Failed to get MAC address from DualShock4\n"); @@ -1907,7 +2053,7 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev) goto err; } - ds4->gamepad = ps_gamepad_create(hdev, NULL); + ds4->gamepad = ps_gamepad_create(hdev, dualshock4_play_effect); if (IS_ERR(ds4->gamepad)) { ret = PTR_ERR(ds4->gamepad); goto err; From 316f57fb3f0c83d15b7c682a7b0e0a63f31cbea5 Mon Sep 17 00:00:00 2001 From: Roderick Colenbrander Date: Sat, 29 Oct 2022 11:48:45 -0700 Subject: [PATCH 21/51] HID: playstation: make LED brightness adjustable in ps_led_register. Make the max_brightness adjustable through ps_led_info struct. This paves the way for a next DualShock4 patch to allow larger brightness values. Signed-off-by: Roderick Colenbrander Signed-off-by: Jiri Kosina --- drivers/hid/hid-playstation.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c index 319f400dd9462..662c6f220571a 100644 --- a/drivers/hid/hid-playstation.c +++ b/drivers/hid/hid-playstation.c @@ -60,6 +60,7 @@ struct ps_calibration_data { struct ps_led_info { const char *name; const char *color; + int max_brightness; enum led_brightness (*brightness_get)(struct led_classdev *cdev); int (*brightness_set)(struct led_classdev *cdev, enum led_brightness); }; @@ -703,7 +704,7 @@ static int ps_led_register(struct ps_device *ps_dev, struct led_classdev *led, return -ENOMEM; led->brightness = 0; - led->max_brightness = 1; + led->max_brightness = led_info->max_brightness; led->flags = LED_CORE_SUSPENDRESUME; led->brightness_get = led_info->brightness_get; led->brightness_set_blocking = led_info->brightness_set; @@ -1459,15 +1460,15 @@ static struct ps_device *dualsense_create(struct hid_device *hdev) int i, ret; static const struct ps_led_info player_leds_info[] = { - { LED_FUNCTION_PLAYER1, "white", dualsense_player_led_get_brightness, + { LED_FUNCTION_PLAYER1, "white", 1, dualsense_player_led_get_brightness, dualsense_player_led_set_brightness }, - { LED_FUNCTION_PLAYER2, "white", dualsense_player_led_get_brightness, + { LED_FUNCTION_PLAYER2, "white", 1, dualsense_player_led_get_brightness, dualsense_player_led_set_brightness }, - { LED_FUNCTION_PLAYER3, "white", dualsense_player_led_get_brightness, + { LED_FUNCTION_PLAYER3, "white", 1, dualsense_player_led_get_brightness, dualsense_player_led_set_brightness }, - { LED_FUNCTION_PLAYER4, "white", dualsense_player_led_get_brightness, + { LED_FUNCTION_PLAYER4, "white", 1, dualsense_player_led_get_brightness, dualsense_player_led_set_brightness }, - { LED_FUNCTION_PLAYER5, "white", dualsense_player_led_get_brightness, + { LED_FUNCTION_PLAYER5, "white", 1, dualsense_player_led_get_brightness, dualsense_player_led_set_brightness } }; From 4521109a8f40621ced642d62f1e887bd6b2d1946 Mon Sep 17 00:00:00 2001 From: Roderick Colenbrander Date: Sat, 29 Oct 2022 11:48:46 -0700 Subject: [PATCH 22/51] HID: playstation: support DualShock4 lightbar. Expose the lightbar LEDs in the same manner as hid-sony through individual LEDs for backwards compatibility reasons. There is a slight change in LED naming to use the input device name as opposed to the MAC address like hid-sony did. This is expected to not cause any issues and should make the naming more compliant. In addition set a default lightbar color based on player ID. Signed-off-by: Roderick Colenbrander Signed-off-by: Jiri Kosina --- drivers/hid/hid-playstation.c | 141 +++++++++++++++++++++++++++++++++- 1 file changed, 138 insertions(+), 3 deletions(-) diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c index 662c6f220571a..d42fda13580a2 100644 --- a/drivers/hid/hid-playstation.c +++ b/drivers/hid/hid-playstation.c @@ -311,6 +311,8 @@ struct dualsense_output_report { /* Flags for DualShock4 output report. */ #define DS4_OUTPUT_VALID_FLAG0_MOTOR 0x01 +#define DS4_OUTPUT_VALID_FLAG0_LED 0x02 +#define DS4_OUTPUT_VALID_FLAG0_LED_BLINK 0x04 /* DualShock4 hardware limits */ #define DS4_ACC_RES_PER_G 8192 @@ -339,6 +341,14 @@ struct dualshock4 { uint8_t motor_left; uint8_t motor_right; + /* Lightbar leds */ + bool update_lightbar; + bool lightbar_enabled; /* For use by global LED control. */ + uint8_t lightbar_red; + uint8_t lightbar_green; + uint8_t lightbar_blue; + struct led_classdev lightbar_leds[4]; + struct work_struct output_worker; bool output_worker_initialized; void *output_report_dmabuf; @@ -697,8 +707,14 @@ static int ps_led_register(struct ps_device *ps_dev, struct led_classdev *led, { int ret; - led->name = devm_kasprintf(&ps_dev->hdev->dev, GFP_KERNEL, - "%s:%s:%s", ps_dev->input_dev_name, led_info->color, led_info->name); + if (led_info->name) { + led->name = devm_kasprintf(&ps_dev->hdev->dev, GFP_KERNEL, + "%s:%s:%s", ps_dev->input_dev_name, led_info->color, led_info->name); + } else { + /* Backwards compatible mode for hid-sony, but not compliant with LED class spec. */ + led->name = devm_kasprintf(&ps_dev->hdev->dev, GFP_KERNEL, + "%s:%s", ps_dev->input_dev_name, led_info->color); + } if (!led->name) return -ENOMEM; @@ -1746,6 +1762,60 @@ static int dualshock4_get_mac_address(struct dualshock4 *ds4) return ret; } +static enum led_brightness dualshock4_led_get_brightness(struct led_classdev *led) +{ + struct hid_device *hdev = to_hid_device(led->dev->parent); + struct dualshock4 *ds4 = hid_get_drvdata(hdev); + unsigned int led_index; + + led_index = led - ds4->lightbar_leds; + switch (led_index) { + case 0: + return ds4->lightbar_red; + case 1: + return ds4->lightbar_green; + case 2: + return ds4->lightbar_blue; + case 3: + return ds4->lightbar_enabled; + } + + return -1; +} + +static int dualshock4_led_set_brightness(struct led_classdev *led, enum led_brightness value) +{ + struct hid_device *hdev = to_hid_device(led->dev->parent); + struct dualshock4 *ds4 = hid_get_drvdata(hdev); + unsigned long flags; + unsigned int led_index; + + spin_lock_irqsave(&ds4->base.lock, flags); + + led_index = led - ds4->lightbar_leds; + switch (led_index) { + case 0: + ds4->lightbar_red = value; + break; + case 1: + ds4->lightbar_green = value; + break; + case 2: + ds4->lightbar_blue = value; + break; + case 3: + ds4->lightbar_enabled = !!value; + } + + ds4->update_lightbar = true; + + spin_unlock_irqrestore(&ds4->base.lock, flags); + + dualshock4_schedule_work(ds4); + + return 0; +} + static void dualshock4_init_output_report(struct dualshock4 *ds4, struct dualshock4_output_report *rp, void *buf) { @@ -1784,6 +1854,18 @@ static void dualshock4_output_worker(struct work_struct *work) ds4->update_rumble = false; } + if (ds4->update_lightbar) { + common->valid_flag0 |= DS4_OUTPUT_VALID_FLAG0_LED; + /* Comptabile behavior with hid-sony, which used a dummy global LED to + * allow enabling/disabling the lightbar. The global LED maps to + * lightbar_enabled. + */ + common->lightbar_red = ds4->lightbar_enabled ? ds4->lightbar_red : 0; + common->lightbar_green = ds4->lightbar_enabled ? ds4->lightbar_green : 0; + common->lightbar_blue = ds4->lightbar_enabled ? ds4->lightbar_blue : 0; + ds4->update_lightbar = false; + } + spin_unlock_irqrestore(&ds4->base.lock, flags); hid_hw_output_report(ds4->base.hdev, report.data, report.len); @@ -1998,12 +2080,52 @@ static inline void dualshock4_schedule_work(struct dualshock4 *ds4) spin_unlock_irqrestore(&ds4->base.lock, flags); } +/* Set default lightbar color based on player. */ +static void dualshock4_set_default_lightbar_colors(struct dualshock4 *ds4) +{ + /* Use same player colors as PlayStation 4. + * Array of colors is in RGB. + */ + static const int player_colors[4][3] = { + { 0x00, 0x00, 0x40 }, /* Blue */ + { 0x40, 0x00, 0x00 }, /* Red */ + { 0x00, 0x40, 0x00 }, /* Green */ + { 0x20, 0x00, 0x20 } /* Pink */ + }; + + uint8_t player_id = ds4->base.player_id % ARRAY_SIZE(player_colors); + + ds4->lightbar_enabled = true; + ds4->lightbar_red = player_colors[player_id][0]; + ds4->lightbar_green = player_colors[player_id][1]; + ds4->lightbar_blue = player_colors[player_id][2]; + + ds4->update_lightbar = true; + dualshock4_schedule_work(ds4); +} + static struct ps_device *dualshock4_create(struct hid_device *hdev) { struct dualshock4 *ds4; struct ps_device *ps_dev; uint8_t max_output_report_size; - int ret; + int i, ret; + + /* The DualShock4 has an RGB lightbar, which the original hid-sony driver + * exposed as a set of 4 LEDs for the 3 color channels and a global control. + * Ideally this should have used the multi-color LED class, which didn't exist + * yet. In addition the driver used a naming scheme not compliant with the LED + * naming spec by using ":", which contained many colons. + * We use a more compliant by using ":" name now. Ideally + * would have been "::indicator", but that would break + * existing applications (e.g. Android). Nothing matches against MAC address. + */ + static const struct ps_led_info lightbar_leds_info[] = { + { NULL, "red", 255, dualshock4_led_get_brightness, dualshock4_led_set_brightness }, + { NULL, "green", 255, dualshock4_led_get_brightness, dualshock4_led_set_brightness }, + { NULL, "blue", 255, dualshock4_led_get_brightness, dualshock4_led_set_brightness }, + { NULL, "global", 1, dualshock4_led_get_brightness, dualshock4_led_set_brightness }, + }; ds4 = devm_kzalloc(&hdev->dev, sizeof(*ds4), GFP_KERNEL); if (!ds4) @@ -2060,6 +2182,9 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev) goto err; } + /* Use gamepad input device name as primary device name for e.g. LEDs */ + ps_dev->input_dev_name = dev_name(&ds4->gamepad->dev); + ds4->sensors = ps_sensors_create(hdev, DS4_ACC_RANGE, DS4_ACC_RES_PER_G, DS4_GYRO_RANGE, DS4_GYRO_RES_PER_DEG_S); if (IS_ERR(ds4->sensors)) { @@ -2077,12 +2202,22 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev) if (ret) goto err; + for (i = 0; i < ARRAY_SIZE(lightbar_leds_info); i++) { + const struct ps_led_info *led_info = &lightbar_leds_info[i]; + + ret = ps_led_register(ps_dev, &ds4->lightbar_leds[i], led_info); + if (ret < 0) + goto err; + } + ret = ps_device_set_player_id(ps_dev); if (ret) { hid_err(hdev, "Failed to assign player id for DualShock4: %d\n", ret); goto err; } + dualshock4_set_default_lightbar_colors(ds4); + /* * Reporting hardware and firmware is important as there are frequent updates, which * can change behavior. From 82d93f64c3ce28e2157e399e9d4ac935188cae7d Mon Sep 17 00:00:00 2001 From: Roderick Colenbrander Date: Sat, 29 Oct 2022 11:48:47 -0700 Subject: [PATCH 23/51] HID: playstation: support DualShock4 lightbar blink. Support lightbar blink through LEDs framework. Signed-off-by: Roderick Colenbrander Signed-off-by: Jiri Kosina --- drivers/hid/hid-playstation.c | 47 ++++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c index d42fda13580a2..7ceb37f04d24c 100644 --- a/drivers/hid/hid-playstation.c +++ b/drivers/hid/hid-playstation.c @@ -63,6 +63,7 @@ struct ps_led_info { int max_brightness; enum led_brightness (*brightness_get)(struct led_classdev *cdev); int (*brightness_set)(struct led_classdev *cdev, enum led_brightness); + int (*blink_set)(struct led_classdev *led, unsigned long *on, unsigned long *off); }; /* Seed values for DualShock4 / DualSense CRC32 for different report types. */ @@ -319,6 +320,7 @@ struct dualsense_output_report { #define DS4_ACC_RANGE (4*DS_ACC_RES_PER_G) #define DS4_GYRO_RES_PER_DEG_S 1024 #define DS4_GYRO_RANGE (2048*DS_GYRO_RES_PER_DEG_S) +#define DS4_LIGHTBAR_MAX_BLINK 255 /* 255 centiseconds */ #define DS4_TOUCHPAD_WIDTH 1920 #define DS4_TOUCHPAD_HEIGHT 942 @@ -343,10 +345,13 @@ struct dualshock4 { /* Lightbar leds */ bool update_lightbar; + bool update_lightbar_blink; bool lightbar_enabled; /* For use by global LED control. */ uint8_t lightbar_red; uint8_t lightbar_green; uint8_t lightbar_blue; + uint8_t lightbar_blink_on; /* In increments of 10ms. */ + uint8_t lightbar_blink_off; /* In increments of 10ms. */ struct led_classdev lightbar_leds[4]; struct work_struct output_worker; @@ -724,6 +729,7 @@ static int ps_led_register(struct ps_device *ps_dev, struct led_classdev *led, led->flags = LED_CORE_SUSPENDRESUME; led->brightness_get = led_info->brightness_get; led->brightness_set_blocking = led_info->brightness_set; + led->blink_set = led_info->blink_set; ret = devm_led_classdev_register(&ps_dev->hdev->dev, led); if (ret) { @@ -1783,6 +1789,37 @@ static enum led_brightness dualshock4_led_get_brightness(struct led_classdev *le return -1; } +static int dualshock4_led_set_blink(struct led_classdev *led, unsigned long *delay_on, + unsigned long *delay_off) +{ + struct hid_device *hdev = to_hid_device(led->dev->parent); + struct dualshock4 *ds4 = hid_get_drvdata(hdev); + unsigned long flags; + + spin_lock_irqsave(&ds4->base.lock, flags); + + if (!*delay_on && !*delay_off) { + /* Default to 1 Hz (50 centiseconds on, 50 centiseconds off). */ + ds4->lightbar_blink_on = 50; + ds4->lightbar_blink_off = 50; + } else { + /* Blink delays in centiseconds. */ + ds4->lightbar_blink_on = min_t(unsigned long, *delay_on/10, DS4_LIGHTBAR_MAX_BLINK); + ds4->lightbar_blink_off = min_t(unsigned long, *delay_off/10, DS4_LIGHTBAR_MAX_BLINK); + } + + ds4->update_lightbar_blink = true; + + spin_unlock_irqrestore(&ds4->base.lock, flags); + + dualshock4_schedule_work(ds4); + + *delay_on = ds4->lightbar_blink_on; + *delay_off = ds4->lightbar_blink_off; + + return 0; +} + static int dualshock4_led_set_brightness(struct led_classdev *led, enum led_brightness value) { struct hid_device *hdev = to_hid_device(led->dev->parent); @@ -1866,6 +1903,13 @@ static void dualshock4_output_worker(struct work_struct *work) ds4->update_lightbar = false; } + if (ds4->update_lightbar_blink) { + common->valid_flag0 |= DS4_OUTPUT_VALID_FLAG0_LED_BLINK; + common->lightbar_blink_on = ds4->lightbar_blink_on; + common->lightbar_blink_off = ds4->lightbar_blink_off; + ds4->update_lightbar_blink = false; + } + spin_unlock_irqrestore(&ds4->base.lock, flags); hid_hw_output_report(ds4->base.hdev, report.data, report.len); @@ -2124,7 +2168,8 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev) { NULL, "red", 255, dualshock4_led_get_brightness, dualshock4_led_set_brightness }, { NULL, "green", 255, dualshock4_led_get_brightness, dualshock4_led_set_brightness }, { NULL, "blue", 255, dualshock4_led_get_brightness, dualshock4_led_set_brightness }, - { NULL, "global", 1, dualshock4_led_get_brightness, dualshock4_led_set_brightness }, + { NULL, "global", 1, dualshock4_led_get_brightness, dualshock4_led_set_brightness, + dualshock4_led_set_blink }, }; ds4 = devm_kzalloc(&hdev->dev, sizeof(*ds4), GFP_KERNEL); From a23b063b84d0d54f30822f9f2fa4c6c2f4ea1756 Mon Sep 17 00:00:00 2001 From: Roderick Colenbrander Date: Sat, 29 Oct 2022 11:48:48 -0700 Subject: [PATCH 24/51] HID: playstation: add option to ignore CRC in ps_get_report. This patch adds a parameter to ps_get_report to ignore CRC checks. This prepares for DualShock4, which has some HID reports, which lack CRC. Signed-off-by: Roderick Colenbrander Signed-off-by: Jiri Kosina --- drivers/hid/hid-playstation.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c index 7ceb37f04d24c..81a12aafed174 100644 --- a/drivers/hid/hid-playstation.c +++ b/drivers/hid/hid-playstation.c @@ -672,7 +672,8 @@ static struct input_dev *ps_gamepad_create(struct hid_device *hdev, return gamepad; } -static int ps_get_report(struct hid_device *hdev, uint8_t report_id, uint8_t *buf, size_t size) +static int ps_get_report(struct hid_device *hdev, uint8_t report_id, uint8_t *buf, size_t size, + bool check_crc) { int ret; @@ -693,7 +694,7 @@ static int ps_get_report(struct hid_device *hdev, uint8_t report_id, uint8_t *bu return -EINVAL; } - if (hdev->bus == BUS_BLUETOOTH) { + if (hdev->bus == BUS_BLUETOOTH && check_crc) { /* Last 4 bytes contains crc32. */ uint8_t crc_offset = size - 4; uint32_t report_crc = get_unaligned_le32(&buf[crc_offset]); @@ -894,7 +895,7 @@ static int dualsense_get_calibration_data(struct dualsense *ds) return -ENOMEM; ret = ps_get_report(ds->base.hdev, DS_FEATURE_REPORT_CALIBRATION, buf, - DS_FEATURE_REPORT_CALIBRATION_SIZE); + DS_FEATURE_REPORT_CALIBRATION_SIZE, true); if (ret) { hid_err(ds->base.hdev, "Failed to retrieve DualSense calibration info: %d\n", ret); goto err_free; @@ -976,7 +977,7 @@ static int dualsense_get_firmware_info(struct dualsense *ds) return -ENOMEM; ret = ps_get_report(ds->base.hdev, DS_FEATURE_REPORT_FIRMWARE_INFO, buf, - DS_FEATURE_REPORT_FIRMWARE_INFO_SIZE); + DS_FEATURE_REPORT_FIRMWARE_INFO_SIZE, true); if (ret) { hid_err(ds->base.hdev, "Failed to retrieve DualSense firmware info: %d\n", ret); goto err_free; @@ -1009,7 +1010,7 @@ static int dualsense_get_mac_address(struct dualsense *ds) return -ENOMEM; ret = ps_get_report(ds->base.hdev, DS_FEATURE_REPORT_PAIRING_INFO, buf, - DS_FEATURE_REPORT_PAIRING_INFO_SIZE); + DS_FEATURE_REPORT_PAIRING_INFO_SIZE, true); if (ret) { hid_err(ds->base.hdev, "Failed to retrieve DualSense pairing info: %d\n", ret); goto err_free; @@ -1650,7 +1651,7 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4) return -ENOMEM; ret = ps_get_report(hdev, DS4_FEATURE_REPORT_CALIBRATION, buf, - DS4_FEATURE_REPORT_CALIBRATION_SIZE); + DS4_FEATURE_REPORT_CALIBRATION_SIZE, true); if (ret) { hid_err(hdev, "Failed to retrieve DualShock4 calibration info: %d\n", ret); goto err_free; @@ -1731,7 +1732,7 @@ static int dualshock4_get_firmware_info(struct dualshock4 *ds4) return -ENOMEM; ret = ps_get_report(ds4->base.hdev, DS4_FEATURE_REPORT_FIRMWARE_INFO, buf, - DS4_FEATURE_REPORT_FIRMWARE_INFO_SIZE); + DS4_FEATURE_REPORT_FIRMWARE_INFO_SIZE, true); if (ret) { hid_err(ds4->base.hdev, "Failed to retrieve DualShock4 firmware info: %d\n", ret); goto err_free; @@ -1755,7 +1756,7 @@ static int dualshock4_get_mac_address(struct dualshock4 *ds4) return -ENOMEM; ret = ps_get_report(ds4->base.hdev, DS4_FEATURE_REPORT_PAIRING_INFO, buf, - DS4_FEATURE_REPORT_PAIRING_INFO_SIZE); + DS4_FEATURE_REPORT_PAIRING_INFO_SIZE, true); if (ret) { hid_err(ds4->base.hdev, "Failed to retrieve DualShock4 pairing info: %d\n", ret); goto err_free; From 2d77474a239294786feb4fe9864c845fe92db239 Mon Sep 17 00:00:00 2001 From: Roderick Colenbrander Date: Sat, 29 Oct 2022 11:48:49 -0700 Subject: [PATCH 25/51] HID: playstation: add DualShock4 bluetooth support. Add support for DualShock4 in Bluetooth mode. In Bluetooth, the device is a bit strange in that after 'calibration' it switches sending all its input data from a basic report (only containing buttons/sticks) to an extended report, which also contains touchpad, motion sensors and other data. The overall design of this code is similar to the DualSense code. Signed-off-by: Roderick Colenbrander Signed-off-by: Jiri Kosina --- drivers/hid/hid-playstation.c | 170 ++++++++++++++++++++++++++++------ 1 file changed, 144 insertions(+), 26 deletions(-) diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c index 81a12aafed174..a2f1bd1400a27 100644 --- a/drivers/hid/hid-playstation.c +++ b/drivers/hid/hid-playstation.c @@ -287,11 +287,17 @@ struct dualsense_output_report { #define DS4_INPUT_REPORT_USB 0x01 #define DS4_INPUT_REPORT_USB_SIZE 64 +#define DS4_INPUT_REPORT_BT 0x11 +#define DS4_INPUT_REPORT_BT_SIZE 78 #define DS4_OUTPUT_REPORT_USB 0x05 #define DS4_OUTPUT_REPORT_USB_SIZE 32 +#define DS4_OUTPUT_REPORT_BT 0x11 +#define DS4_OUTPUT_REPORT_BT_SIZE 78 #define DS4_FEATURE_REPORT_CALIBRATION 0x02 #define DS4_FEATURE_REPORT_CALIBRATION_SIZE 37 +#define DS4_FEATURE_REPORT_CALIBRATION_BT 0x05 +#define DS4_FEATURE_REPORT_CALIBRATION_BT_SIZE 41 #define DS4_FEATURE_REPORT_FIRMWARE_INFO 0xa3 #define DS4_FEATURE_REPORT_FIRMWARE_INFO_SIZE 49 #define DS4_FEATURE_REPORT_PAIRING_INFO 0x12 @@ -310,6 +316,9 @@ struct dualsense_output_report { /* Battery status within batery_status field. */ #define DS4_BATTERY_STATUS_FULL 11 +#define DS4_OUTPUT_HWCTL_CRC32 0x40 +#define DS4_OUTPUT_HWCTL_HID 0x80 + /* Flags for DualShock4 output report. */ #define DS4_OUTPUT_VALID_FLAG0_MOTOR 0x01 #define DS4_OUTPUT_VALID_FLAG0_LED 0x02 @@ -401,6 +410,17 @@ struct dualshock4_input_report_usb { } __packed; static_assert(sizeof(struct dualshock4_input_report_usb) == DS4_INPUT_REPORT_USB_SIZE); +struct dualshock4_input_report_bt { + uint8_t report_id; /* 0x11 */ + uint8_t reserved[2]; + struct dualshock4_input_report_common common; + uint8_t num_touch_reports; + struct dualshock4_touch_report touch_reports[4]; /* BT has 4 compared to 3 for USB */ + uint8_t reserved2[2]; + __le32 crc32; +} __packed; +static_assert(sizeof(struct dualshock4_input_report_bt) == DS4_INPUT_REPORT_BT_SIZE); + /* Common data between Bluetooth and USB DualShock4 output reports. */ struct dualshock4_output_report_common { uint8_t valid_flag0; @@ -425,6 +445,16 @@ struct dualshock4_output_report_usb { } __packed; static_assert(sizeof(struct dualshock4_output_report_usb) == DS4_OUTPUT_REPORT_USB_SIZE); +struct dualshock4_output_report_bt { + uint8_t report_id; /* 0x11 */ + uint8_t hw_control; + uint8_t audio_control; + struct dualshock4_output_report_common common; + uint8_t reserved[61]; + __le32 crc32; +} __packed; +static_assert(sizeof(struct dualshock4_output_report_bt) == DS4_OUTPUT_REPORT_BT_SIZE); + /* * The DualShock4 has a main output report used to control most features. It is * largely the same between Bluetooth and USB except for different headers and CRC. @@ -434,6 +464,8 @@ struct dualshock4_output_report { uint8_t *data; /* Start of data */ uint8_t len; /* Size of output report */ + /* Points to Bluetooth data payload in case for a Bluetooth report else NULL. */ + struct dualshock4_output_report_bt *bt; /* Points to USB data payload in case for a USB report else NULL. */ struct dualshock4_output_report_usb *usb; /* Points to common section of report, so past any headers. */ @@ -1646,26 +1678,49 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4) int ret = 0; uint8_t *buf; - buf = kzalloc(DS4_FEATURE_REPORT_CALIBRATION_SIZE, GFP_KERNEL); - if (!buf) - return -ENOMEM; + if (ds4->base.hdev->bus == BUS_USB) { + buf = kzalloc(DS4_FEATURE_REPORT_CALIBRATION_SIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; - ret = ps_get_report(hdev, DS4_FEATURE_REPORT_CALIBRATION, buf, - DS4_FEATURE_REPORT_CALIBRATION_SIZE, true); - if (ret) { - hid_err(hdev, "Failed to retrieve DualShock4 calibration info: %d\n", ret); - goto err_free; + ret = ps_get_report(hdev, DS4_FEATURE_REPORT_CALIBRATION, buf, + DS4_FEATURE_REPORT_CALIBRATION_SIZE, true); + if (ret) { + hid_err(hdev, "Failed to retrieve DualShock4 calibration info: %d\n", ret); + goto err_free; + } + } else { /* Bluetooth */ + buf = kzalloc(DS4_FEATURE_REPORT_CALIBRATION_BT_SIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; + + ret = ps_get_report(hdev, DS4_FEATURE_REPORT_CALIBRATION_BT, buf, + DS4_FEATURE_REPORT_CALIBRATION_BT_SIZE, true); + if (ret) { + hid_err(hdev, "Failed to retrieve DualShock4 calibration info: %d\n", ret); + goto err_free; + } } gyro_pitch_bias = get_unaligned_le16(&buf[1]); gyro_yaw_bias = get_unaligned_le16(&buf[3]); gyro_roll_bias = get_unaligned_le16(&buf[5]); - gyro_pitch_plus = get_unaligned_le16(&buf[7]); - gyro_pitch_minus = get_unaligned_le16(&buf[9]); - gyro_yaw_plus = get_unaligned_le16(&buf[11]); - gyro_yaw_minus = get_unaligned_le16(&buf[13]); - gyro_roll_plus = get_unaligned_le16(&buf[15]); - gyro_roll_minus = get_unaligned_le16(&buf[17]); + if (ds4->base.hdev->bus == BUS_USB) { + gyro_pitch_plus = get_unaligned_le16(&buf[7]); + gyro_pitch_minus = get_unaligned_le16(&buf[9]); + gyro_yaw_plus = get_unaligned_le16(&buf[11]); + gyro_yaw_minus = get_unaligned_le16(&buf[13]); + gyro_roll_plus = get_unaligned_le16(&buf[15]); + gyro_roll_minus = get_unaligned_le16(&buf[17]); + } else { + /* BT + Dongle */ + gyro_pitch_plus = get_unaligned_le16(&buf[7]); + gyro_yaw_plus = get_unaligned_le16(&buf[9]); + gyro_roll_plus = get_unaligned_le16(&buf[11]); + gyro_pitch_minus = get_unaligned_le16(&buf[13]); + gyro_yaw_minus = get_unaligned_le16(&buf[15]); + gyro_roll_minus = get_unaligned_le16(&buf[17]); + } gyro_speed_plus = get_unaligned_le16(&buf[19]); gyro_speed_minus = get_unaligned_le16(&buf[21]); acc_x_plus = get_unaligned_le16(&buf[23]); @@ -1731,8 +1786,11 @@ static int dualshock4_get_firmware_info(struct dualshock4 *ds4) if (!buf) return -ENOMEM; + /* Note USB and BT support the same feature report, but this report + * lacks CRC support, so must be disabled in ps_get_report. + */ ret = ps_get_report(ds4->base.hdev, DS4_FEATURE_REPORT_FIRMWARE_INFO, buf, - DS4_FEATURE_REPORT_FIRMWARE_INFO_SIZE, true); + DS4_FEATURE_REPORT_FIRMWARE_INFO_SIZE, false); if (ret) { hid_err(ds4->base.hdev, "Failed to retrieve DualShock4 firmware info: %d\n", ret); goto err_free; @@ -1748,21 +1806,38 @@ static int dualshock4_get_firmware_info(struct dualshock4 *ds4) static int dualshock4_get_mac_address(struct dualshock4 *ds4) { + struct hid_device *hdev = ds4->base.hdev; uint8_t *buf; int ret = 0; - buf = kzalloc(DS4_FEATURE_REPORT_PAIRING_INFO_SIZE, GFP_KERNEL); - if (!buf) - return -ENOMEM; + if (hdev->bus == BUS_USB) { + buf = kzalloc(DS4_FEATURE_REPORT_PAIRING_INFO_SIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; + + ret = ps_get_report(hdev, DS4_FEATURE_REPORT_PAIRING_INFO, buf, + DS4_FEATURE_REPORT_PAIRING_INFO_SIZE, false); + if (ret) { + hid_err(hdev, "Failed to retrieve DualShock4 pairing info: %d\n", ret); + goto err_free; + } - ret = ps_get_report(ds4->base.hdev, DS4_FEATURE_REPORT_PAIRING_INFO, buf, - DS4_FEATURE_REPORT_PAIRING_INFO_SIZE, true); - if (ret) { - hid_err(ds4->base.hdev, "Failed to retrieve DualShock4 pairing info: %d\n", ret); - goto err_free; - } + memcpy(ds4->base.mac_address, &buf[1], sizeof(ds4->base.mac_address)); + } else { + /* Rely on HIDP for Bluetooth */ + if (strlen(hdev->uniq) != 17) + return -EINVAL; + + ret = sscanf(hdev->uniq, "%02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx", + &ds4->base.mac_address[5], &ds4->base.mac_address[4], + &ds4->base.mac_address[3], &ds4->base.mac_address[2], + &ds4->base.mac_address[1], &ds4->base.mac_address[0]); - memcpy(ds4->base.mac_address, &buf[1], sizeof(ds4->base.mac_address)); + if (ret != sizeof(ds4->base.mac_address)) + return -EINVAL; + + ret = 0; + } err_free: kfree(buf); @@ -1859,7 +1934,18 @@ static void dualshock4_init_output_report(struct dualshock4 *ds4, { struct hid_device *hdev = ds4->base.hdev; - if (hdev->bus == BUS_USB) { + if (hdev->bus == BUS_BLUETOOTH) { + struct dualshock4_output_report_bt *bt = buf; + + memset(bt, 0, sizeof(*bt)); + bt->report_id = DS4_OUTPUT_REPORT_BT; + + rp->data = buf; + rp->len = sizeof(*bt); + rp->bt = bt; + rp->usb = NULL; + rp->common = &bt->common; + } else { /* USB */ struct dualshock4_output_report_usb *usb = buf; memset(usb, 0, sizeof(*usb)); @@ -1867,6 +1953,7 @@ static void dualshock4_init_output_report(struct dualshock4 *ds4, rp->data = buf; rp->len = sizeof(*usb); + rp->bt = NULL; rp->usb = usb; rp->common = &usb->common; } @@ -1913,6 +2000,22 @@ static void dualshock4_output_worker(struct work_struct *work) spin_unlock_irqrestore(&ds4->base.lock, flags); + /* Bluetooth packets need additional flags as well as a CRC in the last 4 bytes. */ + if (report.bt) { + uint32_t crc; + uint8_t seed = PS_OUTPUT_CRC32_SEED; + + /* Hardware control flags need to set to let the device know + * there is HID data as well as CRC. + */ + report.bt->hw_control = DS4_OUTPUT_HWCTL_HID | DS4_OUTPUT_HWCTL_CRC32; + + crc = crc32_le(0xFFFFFFFF, &seed, 1); + crc = ~crc32_le(crc, report.data, report.len - 4); + + report.bt->crc32 = cpu_to_le32(crc); + } + hid_hw_output_report(ds4->base.hdev, report.data, report.len); } @@ -1940,6 +2043,19 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report * ds4_report = &usb->common; num_touch_reports = usb->num_touch_reports; touch_reports = usb->touch_reports; + } else if (hdev->bus == BUS_BLUETOOTH && report->id == DS4_INPUT_REPORT_BT && + size == DS4_INPUT_REPORT_BT_SIZE) { + struct dualshock4_input_report_bt *bt = (struct dualshock4_input_report_bt *)data; + + /* Last 4 bytes of input report contains CRC. */ + if (!ps_check_crc32(PS_INPUT_CRC32_SEED, data, size - 4, bt->crc32)) { + hid_err(hdev, "DualShock4 input CRC's check failed\n"); + return -EILSEQ; + } + + ds4_report = &bt->common; + num_touch_reports = bt->num_touch_reports; + touch_reports = bt->touch_reports; } else { hid_err(hdev, "Unhandled reportID=%d\n", report->id); return -1; @@ -2354,7 +2470,9 @@ static void ps_remove(struct hid_device *hdev) static const struct hid_device_id ps_devices[] = { /* Sony DualShock 4 controllers for PS4 */ + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER) }, { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER) }, + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_2) }, { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_2) }, /* Sony DualSense controllers for PS5 */ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER) }, From 58feecb4172b49ad1459e57556667a59df5bdda2 Mon Sep 17 00:00:00 2001 From: Roderick Colenbrander Date: Sat, 29 Oct 2022 11:48:50 -0700 Subject: [PATCH 26/51] HID: playstation: set default DualShock4 BT poll interval to 4ms. The poll interval for DualShock4 in Bluetooth mode is adjustable through the main output report. Configure it to 4ms, which is similar to USB. Signed-off-by: Roderick Colenbrander Signed-off-by: Jiri Kosina --- drivers/hid/hid-playstation.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c index a2f1bd1400a27..05553d07cb1bb 100644 --- a/drivers/hid/hid-playstation.c +++ b/drivers/hid/hid-playstation.c @@ -316,6 +316,17 @@ struct dualsense_output_report { /* Battery status within batery_status field. */ #define DS4_BATTERY_STATUS_FULL 11 +/* The lower 6 bits of hw_control of the Bluetooth main output report + * control the interval at which Dualshock 4 reports data: + * 0x00 - 1ms + * 0x01 - 1ms + * 0x02 - 2ms + * 0x3E - 62ms + * 0x3F - disabled + */ +#define DS4_OUTPUT_HWCTL_BT_POLL_MASK 0x3F +/* Default to 4ms poll interval, which is same as USB (not adjustable). */ +#define DS4_BT_DEFAULT_POLL_INTERVAL_MS 4 #define DS4_OUTPUT_HWCTL_CRC32 0x40 #define DS4_OUTPUT_HWCTL_HID 0x80 @@ -348,6 +359,10 @@ struct dualshock4 { uint32_t prev_sensor_timestamp; uint32_t sensor_timestamp_us; + /* Bluetooth poll interval */ + bool update_bt_poll_interval; + uint8_t bt_poll_interval; + bool update_rumble; uint8_t motor_left; uint8_t motor_right; @@ -2010,6 +2025,11 @@ static void dualshock4_output_worker(struct work_struct *work) */ report.bt->hw_control = DS4_OUTPUT_HWCTL_HID | DS4_OUTPUT_HWCTL_CRC32; + if (ds4->update_bt_poll_interval) { + report.bt->hw_control |= ds4->bt_poll_interval; + ds4->update_bt_poll_interval = false; + } + crc = crc32_le(0xFFFFFFFF, &seed, 1); crc = ~crc32_le(crc, report.data, report.len - 4); @@ -2241,6 +2261,13 @@ static inline void dualshock4_schedule_work(struct dualshock4 *ds4) spin_unlock_irqrestore(&ds4->base.lock, flags); } +static void dualshock4_set_bt_poll_interval(struct dualshock4 *ds4, uint8_t interval) +{ + ds4->bt_poll_interval = interval; + ds4->update_bt_poll_interval = true; + dualshock4_schedule_work(ds4); +} + /* Set default lightbar color based on player. */ static void dualshock4_set_default_lightbar_colors(struct dualshock4 *ds4) { @@ -2372,6 +2399,8 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev) goto err; } + dualshock4_set_bt_poll_interval(ds4, DS4_BT_DEFAULT_POLL_INTERVAL_MS); + ret = ps_device_set_player_id(ps_dev); if (ret) { hid_err(hdev, "Failed to assign player id for DualShock4: %d\n", ret); From c64ed0cd9324f9e5f44deb6834ad9fb5bfa436bc Mon Sep 17 00:00:00 2001 From: Roderick Colenbrander Date: Sat, 29 Oct 2022 11:48:51 -0700 Subject: [PATCH 27/51] HID: playstation: add DualShock4 dongle support. This patch adds support for the DualShock4 dongle in a very similar way we contributed to hid-sony before. The dongle is a USB to Bluetooth bridge and uses the same HID reports as a USB device. It reports data through the DS4's main USB input report independent on whether a Bluetooth controller is connected. For this reason there is custom dongle report parsing code to detect controller hotplug and kick of calibration work until we are ready to process actual input reports. The logic also incorporates a workaround needed for Steam in which hid-playstation and Steam using hidraw can fight. Signed-off-by: Roderick Colenbrander Signed-off-by: Jiri Kosina --- drivers/hid/hid-playstation.c | 146 ++++++++++++++++++++++++++++++++-- 1 file changed, 140 insertions(+), 6 deletions(-) diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c index 05553d07cb1bb..bae3e712a5623 100644 --- a/drivers/hid/hid-playstation.c +++ b/drivers/hid/hid-playstation.c @@ -315,6 +315,11 @@ struct dualsense_output_report { #define DS4_STATUS0_CABLE_STATE BIT(4) /* Battery status within batery_status field. */ #define DS4_BATTERY_STATUS_FULL 11 +/* Status1 bit2 contains dongle connection state: + * 0 = connectd + * 1 = disconnected + */ +#define DS4_STATUS1_DONGLE_STATE BIT(2) /* The lower 6 bits of hw_control of the Bluetooth main output report * control the interval at which Dualshock 4 reports data: @@ -344,6 +349,13 @@ struct dualsense_output_report { #define DS4_TOUCHPAD_WIDTH 1920 #define DS4_TOUCHPAD_HEIGHT 942 +enum dualshock4_dongle_state { + DONGLE_DISCONNECTED, + DONGLE_CALIBRATING, + DONGLE_CONNECTED, + DONGLE_DISABLED +}; + struct dualshock4 { struct ps_device base; struct input_dev *gamepad; @@ -354,6 +366,11 @@ struct dualshock4 { struct ps_calibration_data accel_calib_data[3]; struct ps_calibration_data gyro_calib_data[3]; + /* Only used on dongle to track state transitions. */ + enum dualshock4_dongle_state dongle_state; + /* Used during calibration. */ + struct work_struct dongle_hotplug_worker; + /* Timestamp for sensor data */ bool sensor_timestamp_initialized; uint32_t prev_sensor_timestamp; @@ -513,9 +530,11 @@ static const struct {int x; int y; } ps_gamepad_hat_mapping[] = { {0, 0}, }; +static int dualshock4_get_calibration_data(struct dualshock4 *ds4); static inline void dualsense_schedule_work(struct dualsense *ds); static inline void dualshock4_schedule_work(struct dualshock4 *ds4); static void dualsense_set_lightbar(struct dualsense *ds, uint8_t red, uint8_t green, uint8_t blue); +static void dualshock4_set_default_lightbar_colors(struct dualshock4 *ds4); /* * Add a new ps_device to ps_devices if it doesn't exist. @@ -1678,6 +1697,33 @@ static struct ps_device *dualsense_create(struct hid_device *hdev) return ERR_PTR(ret); } +static void dualshock4_dongle_calibration_work(struct work_struct *work) +{ + struct dualshock4 *ds4 = container_of(work, struct dualshock4, dongle_hotplug_worker); + unsigned long flags; + enum dualshock4_dongle_state dongle_state; + int ret; + + ret = dualshock4_get_calibration_data(ds4); + if (ret < 0) { + /* This call is very unlikely to fail for the dongle. When it + * fails we are probably in a very bad state, so mark the + * dongle as disabled. We will re-enable the dongle if a new + * DS4 hotplug is detect from sony_raw_event as any issues + * are likely resolved then (the dongle is quite stupid). + */ + hid_err(ds4->base.hdev, "DualShock 4 USB dongle: calibration failed, disabling device\n"); + dongle_state = DONGLE_DISABLED; + } else { + hid_info(ds4->base.hdev, "DualShock 4 USB dongle: calibration completed\n"); + dongle_state = DONGLE_CONNECTED; + } + + spin_lock_irqsave(&ds4->base.lock, flags); + ds4->dongle_state = dongle_state; + spin_unlock_irqrestore(&ds4->base.lock, flags); +} + static int dualshock4_get_calibration_data(struct dualshock4 *ds4) { struct hid_device *hdev = ds4->base.hdev; @@ -1694,15 +1740,34 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4) uint8_t *buf; if (ds4->base.hdev->bus == BUS_USB) { + int retries; + buf = kzalloc(DS4_FEATURE_REPORT_CALIBRATION_SIZE, GFP_KERNEL); if (!buf) return -ENOMEM; - ret = ps_get_report(hdev, DS4_FEATURE_REPORT_CALIBRATION, buf, - DS4_FEATURE_REPORT_CALIBRATION_SIZE, true); - if (ret) { - hid_err(hdev, "Failed to retrieve DualShock4 calibration info: %d\n", ret); - goto err_free; + /* We should normally receive the feature report data we asked + * for, but hidraw applications such as Steam can issue feature + * reports as well. In particular for Dongle reconnects, Steam + * and this function are competing resulting in often receiving + * data for a different HID report, so retry a few times. + */ + for (retries = 0; retries < 3; retries++) { + ret = ps_get_report(hdev, DS4_FEATURE_REPORT_CALIBRATION, buf, + DS4_FEATURE_REPORT_CALIBRATION_SIZE, true); + if (ret) { + if (retries < 2) { + hid_warn(hdev, "Retrying DualShock 4 get calibration report (0x02) request\n"); + continue; + } else { + ret = -EILSEQ; + goto err_free; + } + hid_err(hdev, "Failed to retrieve DualShock4 calibration info: %d\n", ret); + goto err_free; + } else { + break; + } } } else { /* Bluetooth */ buf = kzalloc(DS4_FEATURE_REPORT_CALIBRATION_BT_SIZE, GFP_KERNEL); @@ -2220,6 +2285,62 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report * return 0; } +static int dualshock4_dongle_parse_report(struct ps_device *ps_dev, struct hid_report *report, + u8 *data, int size) +{ + struct dualshock4 *ds4 = container_of(ps_dev, struct dualshock4, base); + bool connected = false; + + /* The dongle reports data using the main USB report (0x1) no matter whether a controller + * is connected with mostly zeros. The report does contain dongle status, which we use to + * determine if a controller is connected and if so we forward to the regular DualShock4 + * parsing code. + */ + if (data[0] == DS4_INPUT_REPORT_USB && size == DS4_INPUT_REPORT_USB_SIZE) { + struct dualshock4_input_report_common *ds4_report = (struct dualshock4_input_report_common *)&data[1]; + unsigned long flags; + + connected = ds4_report->status[1] & DS4_STATUS1_DONGLE_STATE ? false : true; + + if (ds4->dongle_state == DONGLE_DISCONNECTED && connected) { + hid_info(ps_dev->hdev, "DualShock 4 USB dongle: controller connected\n"); + + dualshock4_set_default_lightbar_colors(ds4); + + spin_lock_irqsave(&ps_dev->lock, flags); + ds4->dongle_state = DONGLE_CALIBRATING; + spin_unlock_irqrestore(&ps_dev->lock, flags); + + schedule_work(&ds4->dongle_hotplug_worker); + + /* Don't process the report since we don't have + * calibration data, but let hidraw have it anyway. + */ + return 0; + } else if ((ds4->dongle_state == DONGLE_CONNECTED || + ds4->dongle_state == DONGLE_DISABLED) && !connected) { + hid_info(ps_dev->hdev, "DualShock 4 USB dongle: controller disconnected\n"); + + spin_lock_irqsave(&ps_dev->lock, flags); + ds4->dongle_state = DONGLE_DISCONNECTED; + spin_unlock_irqrestore(&ps_dev->lock, flags); + + /* Return 0, so hidraw can get the report. */ + return 0; + } else if (ds4->dongle_state == DONGLE_CALIBRATING || + ds4->dongle_state == DONGLE_DISABLED || + ds4->dongle_state == DONGLE_DISCONNECTED) { + /* Return 0, so hidraw can get the report. */ + return 0; + } + } + + if (connected) + return dualshock4_parse_report(ps_dev, report, data, size); + + return 0; +} + static int dualshock4_play_effect(struct input_dev *dev, void *data, struct ff_effect *effect) { struct hid_device *hdev = input_get_drvdata(dev); @@ -2249,6 +2370,9 @@ static void dualshock4_remove(struct ps_device *ps_dev) spin_unlock_irqrestore(&ds4->base.lock, flags); cancel_work_sync(&ds4->output_worker); + + if (ps_dev->hdev->product == USB_DEVICE_ID_SONY_PS4_CONTROLLER_DONGLE) + cancel_work_sync(&ds4->dongle_hotplug_worker); } static inline void dualshock4_schedule_work(struct dualshock4 *ds4) @@ -2342,6 +2466,14 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev) if (!ds4->output_report_dmabuf) return ERR_PTR(-ENOMEM); + if (hdev->product == USB_DEVICE_ID_SONY_PS4_CONTROLLER_DONGLE) { + ds4->dongle_state = DONGLE_DISCONNECTED; + INIT_WORK(&ds4->dongle_hotplug_worker, dualshock4_dongle_calibration_work); + + /* Override parse report for dongle specific hotplug handling. */ + ps_dev->parse_report = dualshock4_dongle_parse_report; + } + ret = dualshock4_get_mac_address(ds4); if (ret) { hid_err(hdev, "Failed to get MAC address from DualShock4\n"); @@ -2457,7 +2589,8 @@ static int ps_probe(struct hid_device *hdev, const struct hid_device_id *id) } if (hdev->product == USB_DEVICE_ID_SONY_PS4_CONTROLLER || - hdev->product == USB_DEVICE_ID_SONY_PS4_CONTROLLER_2) { + hdev->product == USB_DEVICE_ID_SONY_PS4_CONTROLLER_2 || + hdev->product == USB_DEVICE_ID_SONY_PS4_CONTROLLER_DONGLE) { dev = dualshock4_create(hdev); if (IS_ERR(dev)) { hid_err(hdev, "Failed to create dualshock4.\n"); @@ -2503,6 +2636,7 @@ static const struct hid_device_id ps_devices[] = { { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER) }, { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_2) }, { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_2) }, + { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_DONGLE) }, /* Sony DualSense controllers for PS5 */ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER) }, { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER) }, From f45d50ede6f9e6d76a218e6ed06cc352acad466f Mon Sep 17 00:00:00 2001 From: Michael Zaidman Date: Sat, 5 Nov 2022 23:11:39 +0200 Subject: [PATCH 28/51] HID: ft260: ft260_xfer_status routine cleanup After clarifying with FTDI's support, it turned out that the error condition (bit 1) in byte 1 of the i2c status HID report is a status bit reflecting all error conditions. When bits 2, 3, or 4 are raised to 1, bit 1 is set to 1 also. Since the ft260_xfer_status routine tests the error condition bit and exits in the case of an error, the program flow never reaches the conditional expressions for 2, 3, and 4 bits when any of them indicates an error state. Though these expressions are never evaluated to true, they are checked several times per IO, increasing the ft260_xfer_status polling cycle duration. The patch removes the conditional expressions for 2, 3, and 4 bits in byte 1 of the i2c status HID report. Signed-off-by: Michael Zaidman Tested-by: Guillaume Champagne Signed-off-by: Jiri Kosina --- drivers/hid/hid-ft260.c | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index 79505c64dbfe7..a35201d68b15e 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -313,27 +313,17 @@ static int ft260_xfer_status(struct ft260_device *dev) if (report.bus_status & FT260_I2C_STATUS_CTRL_BUSY) return -EAGAIN; - if (report.bus_status & FT260_I2C_STATUS_BUS_BUSY) - return -EBUSY; - - if (report.bus_status & FT260_I2C_STATUS_ERROR) + /* + * The error condition (bit 1) is a status bit reflecting any + * error conditions. When any of the bits 2, 3, or 4 are raised + * to 1, bit 1 is also set to 1. + */ + if (report.bus_status & FT260_I2C_STATUS_ERROR) { + hid_err(hdev, "i2c bus error: %#02x\n", report.bus_status); return -EIO; + } - ret = -EIO; - - if (report.bus_status & FT260_I2C_STATUS_ADDR_NO_ACK) - ft260_dbg("unacknowledged address\n"); - - if (report.bus_status & FT260_I2C_STATUS_DATA_NO_ACK) - ft260_dbg("unacknowledged data\n"); - - if (report.bus_status & FT260_I2C_STATUS_ARBITR_LOST) - ft260_dbg("arbitration loss\n"); - - if (report.bus_status & FT260_I2C_STATUS_CTRL_IDLE) - ret = 0; - - return ret; + return 0; } static int ft260_hid_output_report(struct hid_device *hdev, u8 *data, @@ -376,7 +366,7 @@ static int ft260_hid_output_report_check_status(struct ft260_device *dev, break; } while (--try); - if (ret == 0 || ret == -EBUSY) + if (ret == 0) return 0; ft260_i2c_reset(hdev); From 6fca5e3f5574ca1bd5bade5737848c816f924c6a Mon Sep 17 00:00:00 2001 From: Michael Zaidman Date: Sat, 5 Nov 2022 23:11:40 +0200 Subject: [PATCH 29/51] HID: ft260: improve i2c write performance The patch improves the I2C write performance by 20 - 30 percent by revising the sleep time in the ft260_hid_output_report_check_status() in the following ways: 1. Reduce the wait time and start to poll earlier. Sending a large amount of data at a low I2C clock rate saturates the internal FT260 buffer and causes hiccups in status readiness, as shown below in the log fragment. Aligning the status check wait time to the worst case significantly reduces the write performance. [Oct22 10:28] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 d[0] 0x0 [ +0.005296] ft260_xfer_status: bus_status 0x20, clock 100 [ +0.013460] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 d[0] 0x0 [ +0.003244] ft260_hid_output_report_check_status: wait 1920 usec, len 38 [ +0.000190] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.015324] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 d[0] 0x0 [ +0.003491] ft260_hid_output_report_check_status: wait 1920 usec, len 38 [ +0.000202] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.016047] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 d[0] 0x0 [ +0.002768] ft260_hid_output_report_check_status: wait 1920 usec, len 38 [ +0.000150] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.011389] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 d[0] 0x0 [ +0.003467] ft260_hid_output_report_check_status: wait 1920 usec, len 38 [ +0.000191] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000172] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000131] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000241] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000233] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000190] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000196] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.011314] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 d[0] 0x0 [ +0.003334] ft260_hid_output_report_check_status: wait 1920 usec, len 38 [ +0.000227] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000204] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000198] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000147] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.011060] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 d[0] 0x0 Before: $ sudo ./i2cperf -f 2 -o 2 -s 32 -r 0-0xff 13 0x51 -S Fill block with increment via i2ctransfer by chunks ------------------------------------------------------------------- data rate(bps) efficiency(%) data size(B) total IOs IO size(B) ------------------------------------------------------------------- 40510 80 256 8 32 After: $ sudo ./i2cperf -f 2 -o 2 -s 32 -r 0-0xff 13 0x51 -S Fill block with increment via i2ctransfer by chunks ------------------------------------------------------------------- data rate(bps) efficiency(%) data size(B) total IOs IO size(B) ------------------------------------------------------------------- 52584 80 256 8 32 2. Do not sleep if the estimated I2C transfer time is below 2 ms since the first xfer status query frequently takes around 1.5 ms, and the following status queries take about 200us on average. So we usually return from the routine after the first 1 - 3 status checks. [Oct22 11:14] ft260_i2c_write: rep 0xd4 addr 0x51 off 0 len 18 d[0] 0x0 [ +0.004270] ft260_xfer_status: bus_status 0x20, clock 100 [ +0.013889] ft260_i2c_write: rep 0xd4 addr 0x51 off 0 len 18 d[0] 0x0 [ +0.000856] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000138] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.013352] ft260_i2c_write: rep 0xd4 addr 0x51 off 0 len 18 d[0] 0x0 [ +0.001501] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000177] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.014477] ft260_i2c_write: rep 0xd4 addr 0x51 off 0 len 18 d[0] 0x0 [ +0.001377] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000233] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000191] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.013197] ft260_i2c_write: rep 0xd4 addr 0x51 off 0 len 18 d[0] 0x0 Before: $ sudo ./i2cperf -f 2 -o 2 -s 16 -r 0-0xff 13 0x51 -S Fill block with increment via i2ctransfer by chunks ------------------------------------------------------------------- data rate(bps) efficiency(%) data size(B) total IOs IO size(B) ------------------------------------------------------------------- 28826 73 256 16 16 After: $ sudo ./i2cperf -f 2 -o 2 -s 16 -r 0-0xff 13 0x51 -S Fill block with increment via i2ctransfer by chunks ------------------------------------------------------------------- data rate(bps) efficiency(%) data size(B) total IOs IO size(B) ------------------------------------------------------------------- 45138 73 256 16 16 Signed-off-by: Michael Zaidman Tested-by: Guillaume Champagne Signed-off-by: Jiri Kosina --- drivers/hid/hid-ft260.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index a35201d68b15e..44106cadd746d 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -345,7 +345,7 @@ static int ft260_hid_output_report(struct hid_device *hdev, u8 *data, static int ft260_hid_output_report_check_status(struct ft260_device *dev, u8 *data, int len) { - int ret, usec, try = 3; + int ret, usec, try = 100; struct hid_device *hdev = dev->hdev; ret = ft260_hid_output_report(hdev, data, len); @@ -356,10 +356,14 @@ static int ft260_hid_output_report_check_status(struct ft260_device *dev, return ret; } - /* transfer time = 1 / clock(KHz) * 10 bits * bytes */ - usec = 10000 / dev->clock * len; - usleep_range(usec, usec + 100); - ft260_dbg("wait %d usec, len %d\n", usec, len); + /* transfer time = 1 / clock(KHz) * 9 bits * bytes */ + usec = len * 9000 / dev->clock; + if (usec > 2000) { + usec -= 1500; + usleep_range(usec, usec + 100); + ft260_dbg("wait %d usec, len %d\n", usec, len); + } + do { ret = ft260_xfer_status(dev); if (ret != -EAGAIN) From 1edfae51d5763439b2c89c9419c377f9c0da354d Mon Sep 17 00:00:00 2001 From: Michael Zaidman Date: Sat, 5 Nov 2022 23:11:41 +0200 Subject: [PATCH 30/51] HID: ft260: support i2c writes larger than HID report size To support longer than one HID report size write, the driver splits a single i2c message data payload into multiple i2c messages of HID report size. However, it does not replicate the offset bytes within the EEPROM chip in every consequent HID report because it is not and should not be aware of the EEPROM type. It breaks the i2c write message integrity and causes the EEPROM device not to acknowledge the second HID report keeping the i2c bus busy until the ft260 controller reports failure. This patch preserves the i2c write message integrity by manipulating the i2c flag bits across multiple HID reports to be seen by the EEPROM device as a single i2c write transfer. Before: $ sudo ./i2cperf -f 2 -o 2 -s 64 -r 0-0xff 13 0x51 -S Error: Sending messages failed: Input/output error [ +3.667741] ft260_i2c_write: rep 0xde addr 0x51 off 0 len 60 d[0] 0x0 [ +0.007330] ft260_hid_output_report_check_status: wait 6400 usec, len 64 [ +0.000203] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000001] ft260_i2c_write: rep 0xd1 addr 0x51 off 60 len 6 d[0] 0x0 [ +0.002337] ft260_hid_output_report_check_status: wait 1000 usec, len 10 [ +0.000157] ft260_xfer_status: bus_status 0x2e, clock 100 [ +0.000241] ft260_i2c_reset: done [ +0.000003] ft260_i2c_write: failed to start transfer, ret -5 After: $ sudo ./i2cperf -f 2 -o 2 -s 128 -r 0-0xff 13 0x51 -S Fill block with increment via i2ctransfer by chunks ------------------------------------------------------------------- data rate(bps) efficiency(%) data size(B) total IOs IO size(B) ------------------------------------------------------------------- 71260 86 256 2 128 Signed-off-by: Michael Zaidman Tested-by: Guillaume Champagne Signed-off-by: Jiri Kosina --- drivers/hid/hid-ft260.c | 41 +++++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index 44106cadd746d..cec83f69ebdc4 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -378,41 +378,46 @@ static int ft260_hid_output_report_check_status(struct ft260_device *dev, } static int ft260_i2c_write(struct ft260_device *dev, u8 addr, u8 *data, - int data_len, u8 flag) + int len, u8 flag) { - int len, ret, idx = 0; + int ret, wr_len, idx = 0; struct hid_device *hdev = dev->hdev; struct ft260_i2c_write_request_report *rep = (struct ft260_i2c_write_request_report *)dev->write_buf; + rep->flag = FT260_FLAG_START; + do { - if (data_len <= FT260_WR_DATA_MAX) - len = data_len; - else - len = FT260_WR_DATA_MAX; + if (len <= FT260_WR_DATA_MAX) { + wr_len = len; + if (flag == FT260_FLAG_START_STOP) + rep->flag |= FT260_FLAG_STOP; + } else { + wr_len = FT260_WR_DATA_MAX; + } - rep->report = FT260_I2C_DATA_REPORT_ID(len); + rep->report = FT260_I2C_DATA_REPORT_ID(wr_len); rep->address = addr; - rep->length = len; - rep->flag = flag; + rep->length = wr_len; - memcpy(rep->data, &data[idx], len); + memcpy(rep->data, &data[idx], wr_len); - ft260_dbg("rep %#02x addr %#02x off %d len %d d[0] %#02x\n", - rep->report, addr, idx, len, data[0]); + ft260_dbg("rep %#02x addr %#02x off %d len %d wlen %d flag %#x d[0] %#02x\n", + rep->report, addr, idx, len, wr_len, + rep->flag, data[0]); ret = ft260_hid_output_report_check_status(dev, (u8 *)rep, - len + 4); + wr_len + 4); if (ret < 0) { - hid_err(hdev, "%s: failed to start transfer, ret %d\n", - __func__, ret); + hid_err(hdev, "%s: failed with %d\n", __func__, ret); return ret; } - data_len -= len; - idx += len; + len -= wr_len; + idx += wr_len; + rep->flag = 0; - } while (data_len > 0); + } while (len > 0); return 0; } From 0acb869f40ecc328ed87e389cc08122e3aeba1a8 Mon Sep 17 00:00:00 2001 From: Michael Zaidman Date: Sat, 5 Nov 2022 23:11:42 +0200 Subject: [PATCH 31/51] HID: ft260: support i2c reads greater than HID report size A random i2c read operation in EEPROM devices is implemented as a dummy write operation, followed by a current address read operation. The dummy write operation is used to load the target byte or word address (a.k.a offset) into the offset counter, from which the subsequent read operation then reads. To support longer than one HID report size random read, the ft260 driver issues multiple pairs of i2c write offset + read data transactions of HID report size so that the EEPROM device sees many i2c random read requests from different offsets. Two issues with the current implementation: - This approach suffers from extra overhead caused by writing offset requests. - Necessity to handle offset per HID report in big-endian representation as EEPROM devices expect. The current implementation does not do it and correctly handles the reads up to 60 bytes only. This patch addresses both issues by implementing more efficient approach. It issues a single i2c read request of up to the EEPROM page size and then waits for the data to arrive in multiple HID reports. For example, to read the 256 bytes from a 24LC512 chip, which has 128 bytes page size, the old method performs six ft260_i2c_write_read transactions while the new - two only. Before: $ sudo ./i2cperf -d 2 -o 2 -s 128 -r 0-0xff 13 0x51 -S Read block via i2ctransfer by chunks ------------------------------------------------------------------- data rate(bps) efficiency(%) data size(B) total IOs IO size(B) ------------------------------------------------------------------- 40803 85 256 2 128 Kernel log of a single 128 bytes read request: [ +2.376308] ft260_i2c_write_read: read_off 0x0 left_len 128 len 60 [ +0.000002] ft260_i2c_write: rep 0xd0 addr 0x51 off 0 len 2 wlen 2 flag 0x2 d[0] 0x0 [ +0.000707] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000173] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000001] ft260_i2c_read: rep 0xc2 addr 0x51 len 60 [ +0.008660] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.000156] ft260_xfer_status: bus_status 0x20, clock 100 [ +0.000001] ft260_i2c_write_read: read_off 0x3c left_len 68 len 60 [ +0.000001] ft260_i2c_write: rep 0xd0 addr 0x51 off 0 len 2 wlen 2 flag 0x2 d[0] 0x3c [ +0.001034] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000191] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000001] ft260_i2c_read: rep 0xc2 addr 0x51 len 60 [ +0.008614] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.000203] ft260_xfer_status: bus_status 0x20, clock 100 [ +0.000001] ft260_i2c_write_read: read_off 0x78 left_len 8 len 8 [ +0.000001] ft260_i2c_write: rep 0xd0 addr 0x51 off 0 len 2 wlen 2 flag 0x2 d[0] 0x78 [ +0.000987] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000192] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000001] ft260_i2c_read: rep 0xc2 addr 0x51 len 8 [ +0.002614] ft260_raw_event: i2c resp: rep 0xd1 len 8 [ +0.000200] ft260_xfer_status: bus_status 0x20, clock 100 After: $ sudo ./i2cperf -d 2 -o 2 -s 128 -r 0-0xff 13 0x51 -S Read block via i2ctransfer by chunks ------------------------------------------------------------------- data rate(bps) efficiency(%) data size(B) total IOs IO size(B) ------------------------------------------------------------------- 43990 85 256 2 128 Kernel log of a single 128 bytes read request: [ +1.464346] ft260_i2c_write_read: off 0x0 rlen 128 wlen 2 [ +0.000002] ft260_i2c_write: rep 0xd0 addr 0x51 off 0 len 2 wlen 2 flag 0x2 d[0] 0x0 [ +0.001653] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000188] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000002] ft260_i2c_read: rep 0xc2 addr 0x51 len 128 rlen 60 flag 0x3 [ +0.008609] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.000157] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000002] ft260_i2c_read: rep 0xc2 addr 0x51 len 68 rlen 60 flag 0x0 [ +0.008840] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.000203] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000002] ft260_i2c_read: rep 0xc2 addr 0x51 len 8 rlen 8 flag 0x4 [ +0.002794] ft260_raw_event: i2c resp: rep 0xd1 len 8 [ +0.000201] ft260_xfer_status: bus_status 0x20, clock 100 Signed-off-by: Michael Zaidman Tested-by: Guillaume Champagne Signed-off-by: Jiri Kosina --- drivers/hid/hid-ft260.c | 127 +++++++++++++++++++++------------------- 1 file changed, 66 insertions(+), 61 deletions(-) diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index cec83f69ebdc4..a354089bb747d 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -456,49 +456,62 @@ static int ft260_smbus_write(struct ft260_device *dev, u8 addr, u8 cmd, static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data, u16 len, u8 flag) { + u16 rd_len; + int timeout, ret; struct ft260_i2c_read_request_report rep; struct hid_device *hdev = dev->hdev; - int timeout; - int ret; - if (len > FT260_RD_DATA_MAX) { - hid_err(hdev, "%s: unsupported rd len: %d\n", __func__, len); - return -EINVAL; - } + if ((flag & FT260_FLAG_START_REPEATED) == FT260_FLAG_START_REPEATED) + flag = FT260_FLAG_START_REPEATED; + else + flag = FT260_FLAG_START; + do { + if (len <= FT260_RD_DATA_MAX) { + rd_len = len; + flag |= FT260_FLAG_STOP; + } else { + rd_len = FT260_RD_DATA_MAX; + } - dev->read_idx = 0; - dev->read_buf = data; - dev->read_len = len; + dev->read_idx = 0; + dev->read_buf = data; + dev->read_len = rd_len; - rep.report = FT260_I2C_READ_REQ; - rep.length = cpu_to_le16(len); - rep.address = addr; - rep.flag = flag; + rep.report = FT260_I2C_READ_REQ; + rep.length = cpu_to_le16(rd_len); + rep.address = addr; + rep.flag = flag; - ft260_dbg("rep %#02x addr %#02x len %d\n", rep.report, rep.address, - rep.length); + ft260_dbg("rep %#02x addr %#02x len %d rlen %d flag %#x\n", + rep.report, rep.address, len, rd_len, flag); - reinit_completion(&dev->wait); + reinit_completion(&dev->wait); - ret = ft260_hid_output_report(hdev, (u8 *)&rep, sizeof(rep)); - if (ret < 0) { - hid_err(hdev, "%s: failed to start transaction, ret %d\n", - __func__, ret); - return ret; - } + ret = ft260_hid_output_report(hdev, (u8 *)&rep, sizeof(rep)); + if (ret < 0) { + hid_err(hdev, "%s: failed with %d\n", __func__, ret); + return ret; + } - timeout = msecs_to_jiffies(5000); - if (!wait_for_completion_timeout(&dev->wait, timeout)) { - ft260_i2c_reset(hdev); - return -ETIMEDOUT; - } + timeout = msecs_to_jiffies(5000); + if (!wait_for_completion_timeout(&dev->wait, timeout)) { + ft260_i2c_reset(hdev); + return -ETIMEDOUT; + } - ret = ft260_xfer_status(dev); - if (ret == 0) - return 0; + ret = ft260_xfer_status(dev); + if (ret < 0) { + ft260_i2c_reset(hdev); + return -EIO; + } - ft260_i2c_reset(hdev); - return -EIO; + len -= rd_len; + data += rd_len; + flag = 0; + + } while (len > 0); + + return 0; } /* @@ -509,45 +522,37 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data, */ static int ft260_i2c_write_read(struct ft260_device *dev, struct i2c_msg *msgs) { - int len, ret; - u16 left_len = msgs[1].len; - u8 *read_buf = msgs[1].buf; + int ret; + int wr_len = msgs[0].len; + int rd_len = msgs[1].len; + struct hid_device *hdev = dev->hdev; u8 addr = msgs[0].addr; u16 read_off = 0; - struct hid_device *hdev = dev->hdev; - if (msgs[0].len > 2) { - hid_err(hdev, "%s: unsupported wr len: %d\n", __func__, - msgs[0].len); + if (wr_len > 2) { + hid_err(hdev, "%s: invalid wr_len: %d\n", __func__, wr_len); return -EOPNOTSUPP; } - memcpy(&read_off, msgs[0].buf, msgs[0].len); - - do { - if (left_len <= FT260_RD_DATA_MAX) - len = left_len; + if (ft260_debug) { + if (wr_len == 2) + read_off = be16_to_cpu(*(u16 *)msgs[0].buf); else - len = FT260_RD_DATA_MAX; + read_off = *msgs[0].buf; - ft260_dbg("read_off %#x left_len %d len %d\n", read_off, - left_len, len); - - ret = ft260_i2c_write(dev, addr, (u8 *)&read_off, msgs[0].len, - FT260_FLAG_START); - if (ret < 0) - return ret; - - ret = ft260_i2c_read(dev, addr, read_buf, len, - FT260_FLAG_START_STOP); - if (ret < 0) - return ret; + pr_info("%s: off %#x rlen %d wlen %d\n", __func__, + read_off, rd_len, wr_len); + } - left_len -= len; - read_buf += len; - read_off += len; + ret = ft260_i2c_write(dev, addr, msgs[0].buf, wr_len, + FT260_FLAG_START); + if (ret < 0) + return ret; - } while (left_len > 0); + ret = ft260_i2c_read(dev, addr, msgs[1].buf, rd_len, + FT260_FLAG_START_STOP_REPEATED); + if (ret < 0) + return ret; return 0; } From 54410c14800ad652c77e5c6fc5c17baad6e42cb6 Mon Sep 17 00:00:00 2001 From: Michael Zaidman Date: Sat, 5 Nov 2022 23:11:43 +0200 Subject: [PATCH 32/51] HID: ft260: improve i2c large reads performance The patch increases the read buffer size to 180 bytes. It reduces the number of ft260_i2c_read() calls by three, improving the big reads performance. $ sudo i2ctransfer -y -f 13 w2@0x51 0x0 0x0 r180 Before: [ +4.071878] ft260_i2c_write_read: off 0x0 rlen 180 wlen 2 [ +0.000005] ft260_i2c_write: rep 0xd0 addr 0x51 off 0 len 2 wlen 2 flag 0x2 d[0] 0x0 [ +0.001097] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000175] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000004] ft260_i2c_read: rep 0xc2 addr 0x51 len 180 rlen 60 flag 0x3 [ +0.008579] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.000208] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000001] ft260_i2c_read: rep 0xc2 addr 0x51 len 120 rlen 60 flag 0x0 [ +0.008794] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.000181] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000002] ft260_i2c_read: rep 0xc2 addr 0x51 len 60 rlen 60 flag 0x4 [ +0.008817] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.000223] ft260_xfer_status: bus_status 0x20, clock 100 After: [ +11.611642] ft260_i2c_write_read: off 0x0 rlen 180 wlen 2 [ +0.000005] ft260_i2c_write: rep 0xd0 addr 0x51 off 0 len 2 wlen 2 flag 0x2 d[0] 0x0 [ +0.008001] ft260_xfer_status: bus_status 0x20, clock 100 [ +0.000001] ft260_i2c_read: rep 0xc2 addr 0x51 len 180 rlen 180 flag 0x7 [ +0.008994] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.007987] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.007992] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.000206] ft260_xfer_status: bus_status 0x20, clock 100 Suggested-by: Enrik Berkhan Signed-off-by: Michael Zaidman Signed-off-by: Jiri Kosina --- drivers/hid/hid-ft260.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index a354089bb747d..91f9087e49dc5 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -30,12 +30,19 @@ MODULE_PARM_DESC(debug, "Toggle FT260 debugging messages"); #define FT260_REPORT_MAX_LENGTH (64) #define FT260_I2C_DATA_REPORT_ID(len) (FT260_I2C_REPORT_MIN + (len - 1) / 4) + /* - * The input report format assigns 62 bytes for the data payload, but ft260 - * returns 60 and 2 in two separate transactions. To minimize transfer time - * in reading chunks mode, set the maximum read payload length to 60 bytes. - */ -#define FT260_RD_DATA_MAX (60) + * The ft260 input report format defines 62 bytes for the data payload, but + * when requested 62 bytes, the controller returns 60 and 2 in separate input + * reports. To achieve better performance with the multi-report read data + * transfers, we set the maximum read payload length to a multiple of 60. + * With a 100 kHz I2C clock, one 240 bytes read takes about 1/27 second, + * which is excessive; On the other hand, some higher layer drivers like at24 + * or optoe limit the i2c reads to 128 bytes. To not block other drivers out + * of I2C for potentially troublesome amounts of time, we select the maximum + * read payload length to be 180 bytes. +*/ +#define FT260_RD_DATA_MAX (180) #define FT260_WR_DATA_MAX (60) /* From 76e76e7993f3d8da484424135a4a8acf7f3cac0b Mon Sep 17 00:00:00 2001 From: Michael Zaidman Date: Sat, 5 Nov 2022 23:11:44 +0200 Subject: [PATCH 33/51] HID: ft260: do not populate /dev/hidraw device Do not populate the /dev/hidraw on ft260 interfaces when the hid-ft260 driver is loaded. $ sudo insmod hid-ft260.ko $ ls /dev/hidraw* /dev/hidraw0 $ sudo rmmod hid-ft260.ko $ ls /dev/hidraw* /dev/hidraw0 /dev/hidraw1 /dev/hidraw2 Reported-by: Enrik Berkhan Signed-off-by: Michael Zaidman Signed-off-by: Jiri Kosina --- drivers/hid/hid-ft260.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index 91f9087e49dc5..8d6d2a19b9ed9 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -939,7 +939,7 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id) return ret; } - ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW); + ret = hid_hw_start(hdev, 0); if (ret) { hid_err(hdev, "failed to start HID HW\n"); return ret; @@ -966,6 +966,10 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id) if (ret <= 0) goto err_hid_close; + hid_info(hdev, "USB HID v%x.%02x Device [%s] on %s\n", + hdev->version >> 8, hdev->version & 0xff, hdev->name, + hdev->phys); + hid_set_drvdata(hdev, dev); dev->hdev = hdev; dev->adap.owner = THIS_MODULE; @@ -974,8 +978,7 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id) dev->adap.quirks = &ft260_i2c_quirks; dev->adap.dev.parent = &hdev->dev; snprintf(dev->adap.name, sizeof(dev->adap.name), - "FT260 usb-i2c bridge on hidraw%d", - ((struct hidraw *)hdev->hidraw)->minor); + "FT260 usb-i2c bridge"); mutex_init(&dev->lock); init_completion(&dev->wait); From b7121e3c04440cc2af9cabbabb24efd23741294a Mon Sep 17 00:00:00 2001 From: Michael Zaidman Date: Sat, 5 Nov 2022 23:11:45 +0200 Subject: [PATCH 34/51] HID: ft260: skip unexpected HID input reports The FT260 is not supposed to generate unexpected HID reports. However, in theory, the unsolicited HID Input reports can be issued by a specially crafted malicious USB device masquerading as FT260 when the attacker has physical access to the USB port. In this case, the read_buf pointer points to the final data portion of the previous I2C Read transfer, and the memcpy invoked in the ft260_raw_event() will try copying the content of the unexpected report into the wrong location. This commit sets the Read buffer pointer to NULL on the I2C Read transaction completion and checks it in the ft260_raw_event() to detect and skip the unsolicited Input report. Reported-by: Enrik Berkhan Signed-off-by: Michael Zaidman Signed-off-by: Jiri Kosina --- drivers/hid/hid-ft260.c | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index 8d6d2a19b9ed9..8b6ebc5228eb3 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -464,7 +464,7 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data, u16 len, u8 flag) { u16 rd_len; - int timeout, ret; + int timeout, ret = 0; struct ft260_i2c_read_request_report rep; struct hid_device *hdev = dev->hdev; @@ -480,10 +480,6 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data, rd_len = FT260_RD_DATA_MAX; } - dev->read_idx = 0; - dev->read_buf = data; - dev->read_len = rd_len; - rep.report = FT260_I2C_READ_REQ; rep.length = cpu_to_le16(rd_len); rep.address = addr; @@ -494,22 +490,30 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data, reinit_completion(&dev->wait); + dev->read_idx = 0; + dev->read_buf = data; + dev->read_len = rd_len; + ret = ft260_hid_output_report(hdev, (u8 *)&rep, sizeof(rep)); if (ret < 0) { hid_err(hdev, "%s: failed with %d\n", __func__, ret); - return ret; + goto ft260_i2c_read_exit; } timeout = msecs_to_jiffies(5000); if (!wait_for_completion_timeout(&dev->wait, timeout)) { + ret = -ETIMEDOUT; ft260_i2c_reset(hdev); - return -ETIMEDOUT; + goto ft260_i2c_read_exit; } + dev->read_buf = NULL; + ret = ft260_xfer_status(dev); if (ret < 0) { + ret = -EIO; ft260_i2c_reset(hdev); - return -EIO; + goto ft260_i2c_read_exit; } len -= rd_len; @@ -518,7 +522,9 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data, } while (len > 0); - return 0; +ft260_i2c_read_exit: + dev->read_buf = NULL; + return ret; } /* @@ -1036,6 +1042,13 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report, ft260_dbg("i2c resp: rep %#02x len %d\n", xfer->report, xfer->length); + if ((dev->read_buf == NULL) || + (xfer->length > dev->read_len - dev->read_idx)) { + hid_err(hdev, "unexpected report %#02x, length %d\n", + xfer->report, xfer->length); + return -1; + } + memcpy(&dev->read_buf[dev->read_idx], &xfer->data, xfer->length); dev->read_idx += xfer->length; @@ -1044,10 +1057,9 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report, complete(&dev->wait); } else { - hid_err(hdev, "unknown report: %#02x\n", xfer->report); - return 0; + hid_err(hdev, "unhandled report %#02x\n", xfer->report); } - return 1; + return 0; } static struct hid_driver ft260_driver = { From 3b56ff4820cf9d36a770d01769d5d9a6c3b14891 Mon Sep 17 00:00:00 2001 From: Michael Zaidman Date: Sat, 5 Nov 2022 23:11:46 +0200 Subject: [PATCH 35/51] HID: ft260: remove SMBus Quick command support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The i2cdetect uses the SMBus Quick command by default to scan devices on the I2C bus. The FT260 implements an I2C bus controller. The SMBus is derived from I2C, but there are several differences between the specifications of the two buses in the areas of timing, protocols, operation modes, and electrical characteristics. One of the differences is that the I2C devices allow the slave not to ACK its slave address, but SMBus requires it to always ACK it as a mechanism to detect a detachable device’s presence on the bus. Since FT260 is the I2C bus controller, it does not acknowledge the SMBus Quick write command, which sends a single bit to the device at the place of the RD/WR bit. The ft260 driver attempted to mimic the SMBus Quick Write functionality by writing a single byte as the SMBus Byte Write command does. Usually, one byte in the SMBus Quick Write will be fine. However, it may cause problems with devices with a control register at offset 0, like i2c muxes, for example, when scanned with the i2cdetect utility. The i2cdetect with the "-r" option uses the SMBus Read Byte command, which is a reasonable workaround. To prevent the I2C bus from locking at write-only devices (most notably clock chips at address 0x69), use the "-r" option in conjunction with scanning range parameters. This patch removes the SMBus Quick command support. $ sudo i2cdetect -y 13 Warning: Can't use SMBus Quick Write command, will skip some addresses 0 1 2 3 4 5 6 7 8 9 a b c d e f 00: 10: 20: 30: -- -- -- -- -- -- -- -- 40: 50: 50 51 -- -- -- -- -- -- -- -- -- -- -- -- -- -- 60: 70: $ sudo i2cdetect -y -r 13 0 1 2 3 4 5 6 7 8 9 a b c d e f 00: -- -- -- -- -- -- -- -- 10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 50: 50 51 -- -- -- -- -- -- -- -- -- -- -- -- -- -- 60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 70: -- -- -- -- -- -- -- -- Reported-by: Vince Asbridge Reported-by: Stephen Shirron Reported-by: Enrik Berkhan Signed-off-by: Michael Zaidman Signed-off-by: Jiri Kosina --- drivers/hid/hid-ft260.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index 8b6ebc5228eb3..d186aa5a8e736 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -630,14 +630,6 @@ static int ft260_smbus_xfer(struct i2c_adapter *adapter, u16 addr, u16 flags, } switch (size) { - case I2C_SMBUS_QUICK: - if (read_write == I2C_SMBUS_READ) - ret = ft260_i2c_read(dev, addr, &data->byte, 0, - FT260_FLAG_START_STOP); - else - ret = ft260_smbus_write(dev, addr, cmd, NULL, 0, - FT260_FLAG_START_STOP); - break; case I2C_SMBUS_BYTE: if (read_write == I2C_SMBUS_READ) ret = ft260_i2c_read(dev, addr, &data->byte, 1, @@ -720,7 +712,7 @@ static int ft260_smbus_xfer(struct i2c_adapter *adapter, u16 addr, u16 flags, static u32 ft260_functionality(struct i2c_adapter *adap) { - return I2C_FUNC_I2C | I2C_FUNC_SMBUS_BYTE | I2C_FUNC_SMBUS_QUICK | + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_BYTE | I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA | I2C_FUNC_SMBUS_BLOCK_DATA | I2C_FUNC_SMBUS_I2C_BLOCK; } From 728b117e7862b1eb05c3aad3b2f087b26453d56a Mon Sep 17 00:00:00 2001 From: Michael Zaidman Date: Sat, 5 Nov 2022 23:11:47 +0200 Subject: [PATCH 36/51] HID: ft260: missed NACK from big i2c read The FT260 controller does not return NACK when performing a big read (of multiple hid reports size) from a non-existing device or from the device responding with NACK when it is not ready to serve the request. However, it responds correctly with NACK to a read of up to a single hid report size. To overcome this issue, we split the muli-report read request into a read of a single HID report of 60 bytes size and a multi-report read. Big read of 256 bytes with first read of 60 bytes: $ sudo ./i2cperf -d 2 -o 2 -s 256 -r 0-0xff 1 0x50 -S [ +5.633280] ft260_i2c_write_read: off 0x0 rlen 255 wlen 2 [ +0.000006] ft260_i2c_write: rep 0xd0 addr 0x50 off 0 len 2 wlen 2 flag 0x2 d[0] 0x0 [ +0.013205] ft260_xfer_status: bus_status 0x20, clock 100 [ +0.000007] ft260_i2c_read: rep 0xc2 addr 0x50 len 255 rlen 60 flag 0x3 [ +0.010932] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.004733] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000006] ft260_i2c_read: rep 0xc2 addr 0x50 len 195 rlen 128 flag 0x0 [ +0.012572] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.005789] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.003189] ft260_raw_event: i2c resp: rep 0xd1 len 8 [ +0.004092] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000010] ft260_i2c_read: rep 0xc2 addr 0x50 len 67 rlen 67 flag 0x4 [ +0.011688] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.004700] ft260_raw_event: i2c resp: rep 0xd1 len 7 [ +0.004858] ft260_xfer_status: bus_status 0x20, clock 100 Read from non-existing device at address 8. The first 60 read responded with NACK. $ sudo ./i2cperf -d 2 -o 2 -s 256 -r 0-0xff 1 0x8 -S [Oct19 15:37] ft260_i2c_write_read: off 0x0 rlen 255 wlen 2 [ +0.000007] ft260_i2c_write: rep 0xd0 addr 0x8 off 0 len 2 wlen 2 flag 0x2 d[0] 0x0 [ +0.022820] ft260_xfer_status: bus_status 0x20, clock 100 [ +0.000007] ft260_i2c_read: rep 0xc2 addr 0x8 len 255 rlen 60 flag 0x3 [ +0.010658] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.005965] ft260_xfer_status: bus_status 0x46, clock 100 <-- NACK [ +0.000009] ft260 0003:0403:6030.0004: i2c bus error: 0x46 [ +0.007784] ft260_i2c_reset: done Co-developed-by: Enrik Berkhan Signed-off-by: Enrik Berkhan Signed-off-by: Michael Zaidman Signed-off-by: Jiri Kosina --- drivers/hid/hid-ft260.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index d186aa5a8e736..40fae81386e33 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -464,6 +464,7 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data, u16 len, u8 flag) { u16 rd_len; + u16 rd_data_max = 60; int timeout, ret = 0; struct ft260_i2c_read_request_report rep; struct hid_device *hdev = dev->hdev; @@ -473,12 +474,13 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data, else flag = FT260_FLAG_START; do { - if (len <= FT260_RD_DATA_MAX) { + if (len <= rd_data_max) { rd_len = len; flag |= FT260_FLAG_STOP; } else { - rd_len = FT260_RD_DATA_MAX; + rd_len = rd_data_max; } + rd_data_max = FT260_RD_DATA_MAX; rep.report = FT260_I2C_READ_REQ; rep.length = cpu_to_le16(rd_len); From 4b3da6853a619a952e8caf2e8393264dd42ffa27 Mon Sep 17 00:00:00 2001 From: Michael Zaidman Date: Sat, 5 Nov 2022 23:11:48 +0200 Subject: [PATCH 37/51] HID: ft260: wake up device from power saving mode The FT260 can enter a power saving mode after being idle for longer than 5 seconds. When being woken up from power saving mode by an I2C write request, a possible NACK is not correctly reported by the controller. As a workaround, the driver will issue an I2C status report two times in ft260_xfer_status() after the chip has been idle for more than 5s. Co-developed-by: Enrik Berkhan Signed-off-by: Enrik Berkhan Signed-off-by: Michael Zaidman Signed-off-by: Jiri Kosina --- drivers/hid/hid-ft260.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index 40fae81386e33..ac133980dfe95 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -31,6 +31,8 @@ MODULE_PARM_DESC(debug, "Toggle FT260 debugging messages"); #define FT260_REPORT_MAX_LENGTH (64) #define FT260_I2C_DATA_REPORT_ID(len) (FT260_I2C_REPORT_MIN + (len - 1) / 4) +#define FT260_WAKEUP_NEEDED_AFTER_MS (4800) /* 5s minus 200ms margin */ + /* * The ft260 input report format defines 62 bytes for the data payload, but * when requested 62 bytes, the controller returns 60 and 2 in separate input @@ -237,6 +239,7 @@ struct ft260_device { struct completion wait; struct mutex lock; u8 write_buf[FT260_REPORT_MAX_LENGTH]; + unsigned long need_wakeup_at; u8 *read_buf; u16 read_idx; u16 read_len; @@ -306,6 +309,20 @@ static int ft260_xfer_status(struct ft260_device *dev) struct ft260_get_i2c_status_report report; int ret; + if (time_is_before_jiffies(dev->need_wakeup_at)) { + ret = ft260_hid_feature_report_get(hdev, FT260_I2C_STATUS, + (u8 *)&report, sizeof(report)); + if (unlikely(ret < 0)) { + hid_err(hdev, "failed to retrieve status: %d, no wakeup\n", + ret); + } else { + dev->need_wakeup_at = jiffies + + msecs_to_jiffies(FT260_WAKEUP_NEEDED_AFTER_MS); + ft260_dbg("bus_status %#02x, wakeup\n", + report.bus_status); + } + } + ret = ft260_hid_feature_report_get(hdev, FT260_I2C_STATUS, (u8 *)&report, sizeof(report)); if (unlikely(ret < 0)) { From c2500bdffe5a95fbd0aecdd5be3d2f175ad22f92 Mon Sep 17 00:00:00 2001 From: Michael Zaidman Date: Sat, 5 Nov 2022 23:11:49 +0200 Subject: [PATCH 38/51] HID: ft260: fix a NULL pointer dereference in ft260_i2c_write The zero-length passed into the ft260_i2c_write() triggered the NULL pointer dereference in the debug message on data[0] access. Since the controller does not support a write of zero length, let's not allow it. Before: $ sudo i2ctransfer -y 13 w0@0x51 Killed After: $ sudo i2ctransfer -y 13 w0@0x51 Error: Sending messages failed: Invalid argument Reported-by: Enrik Berkhan Signed-off-by: Michael Zaidman Signed-off-by: Jiri Kosina --- drivers/hid/hid-ft260.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index ac133980dfe95..b4f180c8750a5 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -409,6 +409,9 @@ static int ft260_i2c_write(struct ft260_device *dev, u8 addr, u8 *data, struct ft260_i2c_write_request_report *rep = (struct ft260_i2c_write_request_report *)dev->write_buf; + if (len < 1) + return -EINVAL; + rep->flag = FT260_FLAG_START; do { From 5afac727defa0b2a3dffb2abd5fb5f594b98d217 Mon Sep 17 00:00:00 2001 From: Michael Zaidman Date: Sat, 5 Nov 2022 23:11:50 +0200 Subject: [PATCH 39/51] HID: ft260: missed NACK from busy device When writing into a slow device like an EEPROM chip, the controller may exit the busy state before the device releases the bus. In this case, the ft260_xfer_status returns success before the data transfer completion. The patch fixes it by returning from the ft260_xfer_status() with the "-EAGAIN" on both controller and bus busy status when appropriate. It does not apply to the i2c combined transactions when after the write IO, the controller keeps the bus busy until the read IO and then between reading IOs to ensure an atomic operation. Co-developed-by: Germain Hebert Signed-off-by: Germain Hebert Signed-off-by: Michael Zaidman Signed-off-by: Jiri Kosina --- drivers/hid/hid-ft260.c | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index b4f180c8750a5..d5b092b85d7fa 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -303,7 +303,7 @@ static int ft260_i2c_reset(struct hid_device *hdev) return ret; } -static int ft260_xfer_status(struct ft260_device *dev) +static int ft260_xfer_status(struct ft260_device *dev, u8 bus_busy) { struct hid_device *hdev = dev->hdev; struct ft260_get_i2c_status_report report; @@ -334,7 +334,7 @@ static int ft260_xfer_status(struct ft260_device *dev) ft260_dbg("bus_status %#02x, clock %u\n", report.bus_status, dev->clock); - if (report.bus_status & FT260_I2C_STATUS_CTRL_BUSY) + if (report.bus_status & (FT260_I2C_STATUS_CTRL_BUSY | bus_busy)) return -EAGAIN; /* @@ -369,8 +369,11 @@ static int ft260_hid_output_report(struct hid_device *hdev, u8 *data, static int ft260_hid_output_report_check_status(struct ft260_device *dev, u8 *data, int len) { + u8 bus_busy; int ret, usec, try = 100; struct hid_device *hdev = dev->hdev; + struct ft260_i2c_write_request_report *rep = + (struct ft260_i2c_write_request_report *)data; ret = ft260_hid_output_report(hdev, data, len); if (ret < 0) { @@ -388,8 +391,18 @@ static int ft260_hid_output_report_check_status(struct ft260_device *dev, ft260_dbg("wait %d usec, len %d\n", usec, len); } + /* + * Do not check the busy bit for combined transactions + * since the controller keeps the bus busy between writing + * and reading IOs to ensure an atomic operation. + */ + if (rep->flag == FT260_FLAG_START) + bus_busy = 0; + else + bus_busy = FT260_I2C_STATUS_BUS_BUSY; + do { - ret = ft260_xfer_status(dev); + ret = ft260_xfer_status(dev, bus_busy); if (ret != -EAGAIN) break; } while (--try); @@ -488,6 +501,7 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data, int timeout, ret = 0; struct ft260_i2c_read_request_report rep; struct hid_device *hdev = dev->hdev; + u8 bus_busy = 0; if ((flag & FT260_FLAG_START_REPEATED) == FT260_FLAG_START_REPEATED) flag = FT260_FLAG_START_REPEATED; @@ -531,7 +545,10 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data, dev->read_buf = NULL; - ret = ft260_xfer_status(dev); + if (flag & FT260_FLAG_STOP) + bus_busy = FT260_I2C_STATUS_BUS_BUSY; + + ret = ft260_xfer_status(dev, bus_busy); if (ret < 0) { ret = -EIO; ft260_i2c_reset(hdev); @@ -1003,7 +1020,7 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id) mutex_init(&dev->lock); init_completion(&dev->wait); - ret = ft260_xfer_status(dev); + ret = ft260_xfer_status(dev, FT260_I2C_STATUS_BUS_BUSY); if (ret) ft260_i2c_reset(hdev); From fb5d783b3c66a0110454a54d2f3bcb7dad4309a1 Mon Sep 17 00:00:00 2001 From: Michael Zaidman Date: Sat, 5 Nov 2022 23:11:51 +0200 Subject: [PATCH 40/51] HID: ft260: fix 'cast to restricted' kernel CI bot warnings Fix 'cast to restricted' sparse warnings reported by kernel test robot in https://lore.kernel.org/all/202211021607.ssjymlKi-lkp@intel.com/ Reported-by: kernel test robot Signed-off-by: Michael Zaidman Signed-off-by: Jiri Kosina --- drivers/hid/hid-ft260.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index d5b092b85d7fa..333341e80b0ec 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -588,7 +588,7 @@ static int ft260_i2c_write_read(struct ft260_device *dev, struct i2c_msg *msgs) if (ft260_debug) { if (wr_len == 2) - read_off = be16_to_cpu(*(u16 *)msgs[0].buf); + read_off = be16_to_cpu(*(__be16 *)msgs[0].buf); else read_off = *msgs[0].buf; @@ -830,7 +830,7 @@ static int ft260_byte_show(struct hid_device *hdev, int id, u8 *cfg, int len, } static int ft260_word_show(struct hid_device *hdev, int id, u8 *cfg, int len, - u16 *field, u8 *buf) + __le16 *field, u8 *buf) { int ret; @@ -859,9 +859,9 @@ static int ft260_word_show(struct hid_device *hdev, int id, u8 *cfg, int len, #define FT260_I2CST_ATTR_SHOW(name) \ FT260_ATTR_SHOW(name, ft260_get_i2c_status_report, \ - FT260_I2C_STATUS, u16, ft260_word_show) + FT260_I2C_STATUS, __le16, ft260_word_show) -#define FT260_ATTR_STORE(name, reptype, id, req, type, func) \ +#define FT260_ATTR_STORE(name, reptype, id, req, type, ctype, func) \ static ssize_t name##_store(struct device *kdev, \ struct device_attribute *attr, \ const char *buf, size_t count) \ @@ -871,7 +871,7 @@ static int ft260_word_show(struct hid_device *hdev, int id, u8 *cfg, int len, type name; \ int ret; \ \ - if (!func(buf, 10, &name)) { \ + if (!func(buf, 10, (ctype *)&name)) { \ rep.name = name; \ rep.report = id; \ rep.request = req; \ @@ -887,11 +887,11 @@ static int ft260_word_show(struct hid_device *hdev, int id, u8 *cfg, int len, #define FT260_BYTE_ATTR_STORE(name, reptype, req) \ FT260_ATTR_STORE(name, reptype, FT260_SYSTEM_SETTINGS, req, \ - u8, kstrtou8) + u8, u8, kstrtou8) #define FT260_WORD_ATTR_STORE(name, reptype, req) \ FT260_ATTR_STORE(name, reptype, FT260_SYSTEM_SETTINGS, req, \ - u16, kstrtou16) + __le16, u16, kstrtou16) FT260_SSTAT_ATTR_SHOW(chip_mode); static DEVICE_ATTR_RO(chip_mode); From 9861a25fc248508c781c18b198436c0a97cb3d99 Mon Sep 17 00:00:00 2001 From: Yauhen Kharuzhy Date: Sun, 6 Nov 2022 00:34:22 +0200 Subject: [PATCH 41/51] HID: hid-sensor-custom: Allow more than one hinge angle sensor Some devices has two sets of accelerometers and the sensor hub exports two hinge angle 'sensors' based on accelerometer values. To allow more than one sensor of the same type, use PLATFORM_DEVID_AUTO instead of PLATFORM_DEVID_NONE when registering platform device for it. Checked on the Lenovo Yoga Book YB1-X91L tablet. Signed-off-by: Yauhen Kharuzhy Reviewed-by: Jonathan Cameron Acked-by: Srinivas Pandruvada Signed-off-by: Jiri Kosina --- drivers/hid/hid-sensor-custom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c index 32c2306e240d6..a6fc89ee1287c 100644 --- a/drivers/hid/hid-sensor-custom.c +++ b/drivers/hid/hid-sensor-custom.c @@ -862,7 +862,7 @@ hid_sensor_register_platform_device(struct platform_device *pdev, return ERR_PTR(-ENOMEM); custom_pdev = platform_device_register_data(pdev->dev.parent, dev_name, - PLATFORM_DEVID_NONE, hsdev, + PLATFORM_DEVID_AUTO, hsdev, sizeof(*hsdev)); kfree(dev_name); return custom_pdev; From af89dfde2a5f8b9d2489813f0ec575d0457ad902 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Exp=C3=B3sito?= Date: Thu, 10 Nov 2022 18:49:55 +0100 Subject: [PATCH 42/51] HID: uclogic: Standardize test name prefix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 961bcdf956a4 ("drm/tests: Change "igt_" prefix to "drm_test_"") introduced a new naming convention for the KUnit tests present in the DRM subsystem: "drm_test__". This naming convention is very convenient because it allows to easily run all subsystem tests or all driver tests using kunit.py's wildcards. Follow the naming conventions used in the DRM subsystem adapted to the HID subsystem: "hid_test__". Signed-off-by: José Expósito Reviewed-by: Maíra Canal Signed-off-by: Jiri Kosina --- drivers/hid/hid-uclogic-params-test.c | 4 ++-- drivers/hid/hid-uclogic-rdesc-test.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/hid/hid-uclogic-params-test.c b/drivers/hid/hid-uclogic-params-test.c index 57ef5d3e4b74c..bfa7ccb7d1e8b 100644 --- a/drivers/hid/hid-uclogic-params-test.c +++ b/drivers/hid/hid-uclogic-params-test.c @@ -136,7 +136,7 @@ static void uclogic_parse_ugee_v2_desc_case_desc(struct uclogic_parse_ugee_v2_de KUNIT_ARRAY_PARAM(uclogic_parse_ugee_v2_desc, uclogic_parse_ugee_v2_desc_cases, uclogic_parse_ugee_v2_desc_case_desc); -static void uclogic_parse_ugee_v2_desc_test(struct kunit *test) +static void hid_test_uclogic_parse_ugee_v2_desc(struct kunit *test) { int res; s32 desc_params[UCLOGIC_RDESC_PH_ID_NUM]; @@ -175,7 +175,7 @@ static void uclogic_parse_ugee_v2_desc_test(struct kunit *test) } static struct kunit_case hid_uclogic_params_test_cases[] = { - KUNIT_CASE_PARAM(uclogic_parse_ugee_v2_desc_test, + KUNIT_CASE_PARAM(hid_test_uclogic_parse_ugee_v2_desc, uclogic_parse_ugee_v2_desc_gen_params), {} }; diff --git a/drivers/hid/hid-uclogic-rdesc-test.c b/drivers/hid/hid-uclogic-rdesc-test.c index 3971a0854c3e1..b429c541bf2f7 100644 --- a/drivers/hid/hid-uclogic-rdesc-test.c +++ b/drivers/hid/hid-uclogic-rdesc-test.c @@ -187,7 +187,7 @@ static void uclogic_template_case_desc(struct uclogic_template_case *t, KUNIT_ARRAY_PARAM(uclogic_template, uclogic_template_cases, uclogic_template_case_desc); -static void uclogic_template_test(struct kunit *test) +static void hid_test_uclogic_template(struct kunit *test) { __u8 *res; const struct uclogic_template_case *params = test->param_value; @@ -203,7 +203,7 @@ static void uclogic_template_test(struct kunit *test) } static struct kunit_case hid_uclogic_rdesc_test_cases[] = { - KUNIT_CASE_PARAM(uclogic_template_test, uclogic_template_gen_params), + KUNIT_CASE_PARAM(hid_test_uclogic_template, uclogic_template_gen_params), {} }; From 6df849caeb49e8975e815a6f906528b2da795fbd Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Thu, 20 Oct 2022 22:02:19 +0100 Subject: [PATCH 43/51] HID: intel-ish-hid: ishtp: remove variable rb_count The variable rb_count is being incremented but it is never referenced, it is redundant and can be removed. Signed-off-by: Colin Ian King Acked-by: Srinivas Pandruvada Signed-off-by: Jiri Kosina --- drivers/hid/intel-ish-hid/ishtp/client.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/hid/intel-ish-hid/ishtp/client.c b/drivers/hid/intel-ish-hid/ishtp/client.c index df0a825694f52..2d92fc129ce4d 100644 --- a/drivers/hid/intel-ish-hid/ishtp/client.c +++ b/drivers/hid/intel-ish-hid/ishtp/client.c @@ -841,7 +841,6 @@ void recv_ishtp_cl_msg(struct ishtp_device *dev, unsigned char *buffer = NULL; struct ishtp_cl_rb *complete_rb = NULL; unsigned long flags; - int rb_count; if (ishtp_hdr->reserved) { dev_err(dev->devc, "corrupted message header.\n"); @@ -855,9 +854,7 @@ void recv_ishtp_cl_msg(struct ishtp_device *dev, } spin_lock_irqsave(&dev->read_list_spinlock, flags); - rb_count = -1; list_for_each_entry(rb, &dev->read_list.list, list) { - ++rb_count; cl = rb->cl; if (!cl || !(cl->host_client_id == ishtp_hdr->host_addr && cl->fw_client_id == ishtp_hdr->fw_addr) || From fd7b68b763c4dfa65e3c145c624427d5fd11202f Mon Sep 17 00:00:00 2001 From: Aditya Garg Date: Fri, 4 Nov 2022 11:55:35 +0000 Subject: [PATCH 44/51] HID: apple: Swap Control and Command keys on Apple keyboards This patch allows users to swap the control and command keys. This can be useful for the Mac users who are used to using Command instead of Control in macOS for various commonly used shortcuts. Signed-off-by: Aditya Garg Signed-off-by: Jiri Kosina --- drivers/hid/hid-apple.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c index c671ce94671ca..1ccab8aa326cd 100644 --- a/drivers/hid/hid-apple.c +++ b/drivers/hid/hid-apple.c @@ -59,6 +59,12 @@ MODULE_PARM_DESC(swap_opt_cmd, "Swap the Option (\"Alt\") and Command (\"Flag\") "(For people who want to keep Windows PC keyboard muscle memory. " "[0] = as-is, Mac layout. 1 = swapped, Windows layout.)"); +static unsigned int swap_ctrl_cmd; +module_param(swap_ctrl_cmd, uint, 0644); +MODULE_PARM_DESC(swap_ctrl_cmd, "Swap the Control (\"Ctrl\") and Command (\"Flag\") keys. " + "(For people who are used to Mac shortcuts involving Command instead of Control. " + "[0] = No change. 1 = Swapped.)"); + static unsigned int swap_fn_leftctrl; module_param(swap_fn_leftctrl, uint, 0644); MODULE_PARM_DESC(swap_fn_leftctrl, "Swap the Fn and left Control keys. " @@ -308,7 +314,15 @@ static const struct apple_key_translation swapped_option_cmd_keys[] = { { KEY_LEFTALT, KEY_LEFTMETA }, { KEY_LEFTMETA, KEY_LEFTALT }, { KEY_RIGHTALT, KEY_RIGHTMETA }, - { KEY_RIGHTMETA,KEY_RIGHTALT }, + { KEY_RIGHTMETA, KEY_RIGHTALT }, + { } +}; + +static const struct apple_key_translation swapped_ctrl_cmd_keys[] = { + { KEY_LEFTCTRL, KEY_LEFTMETA }, + { KEY_LEFTMETA, KEY_LEFTCTRL }, + { KEY_RIGHTCTRL, KEY_RIGHTMETA }, + { KEY_RIGHTMETA, KEY_RIGHTCTRL }, { } }; @@ -407,6 +421,13 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input, code = trans->to; } + if (swap_ctrl_cmd) { + trans = apple_find_translation(swapped_ctrl_cmd_keys, code); + + if (trans) + code = trans->to; + } + if (code == KEY_FN) asc->fn_on = !!value; From 54980d30eff608545884416576416060b80d011e Mon Sep 17 00:00:00 2001 From: Roderick Colenbrander Date: Wed, 16 Nov 2022 08:00:21 -0800 Subject: [PATCH 45/51] HID: playstation: fix DualShock4 bluetooth memory corruption bug. The size of the output buffer used for output reports was not updated to the larger size needed for Bluetooth. This ultimately resulted in memory corruption of surrounding structures e.g. due to memsets. Fixes: 2d77474a2392 ("HID: playstation: add DualShock4 bluetooth support.") Reported-by: Benjamin Tissoires Signed-off-by: Roderick Colenbrander Signed-off-by: Jiri Kosina --- drivers/hid/hid-playstation.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c index bae3e712a5623..f5e0d06d3cd8b 100644 --- a/drivers/hid/hid-playstation.c +++ b/drivers/hid/hid-playstation.c @@ -2461,7 +2461,7 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev) ds4->output_worker_initialized = true; hid_set_drvdata(hdev, ds4); - max_output_report_size = sizeof(struct dualshock4_output_report_usb); + max_output_report_size = sizeof(struct dualshock4_output_report_bt); ds4->output_report_dmabuf = devm_kzalloc(&hdev->dev, max_output_report_size, GFP_KERNEL); if (!ds4->output_report_dmabuf) return ERR_PTR(-ENOMEM); From da03e502bb22ec859af4f7d1f7d4d5f237b6c3fe Mon Sep 17 00:00:00 2001 From: Roderick Colenbrander Date: Wed, 16 Nov 2022 08:00:22 -0800 Subject: [PATCH 46/51] HID: playstation: fix DualShock4 bluetooth CRC endian issue. The driver was by accident reading the CRC directly from a hardware structure instead of using get_unaligned_le32. Fixes: 2d77474a2392 ("HID: playstation: add DualShock4 bluetooth support.") Reported-by: kernel test robot Signed-off-by: Roderick Colenbrander Signed-off-by: Jiri Kosina --- drivers/hid/hid-playstation.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c index f5e0d06d3cd8b..7b5aef538044a 100644 --- a/drivers/hid/hid-playstation.c +++ b/drivers/hid/hid-playstation.c @@ -2131,9 +2131,10 @@ static int dualshock4_parse_report(struct ps_device *ps_dev, struct hid_report * } else if (hdev->bus == BUS_BLUETOOTH && report->id == DS4_INPUT_REPORT_BT && size == DS4_INPUT_REPORT_BT_SIZE) { struct dualshock4_input_report_bt *bt = (struct dualshock4_input_report_bt *)data; + uint32_t report_crc = get_unaligned_le32(&bt->crc32); /* Last 4 bytes of input report contains CRC. */ - if (!ps_check_crc32(PS_INPUT_CRC32_SEED, data, size - 4, bt->crc32)) { + if (!ps_check_crc32(PS_INPUT_CRC32_SEED, data, size - 4, report_crc)) { hid_err(hdev, "DualShock4 input CRC's check failed\n"); return -EILSEQ; } From 9984fbf55b9bd998b4ff66395cbb118020c1effa Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Fri, 18 Nov 2022 09:02:45 -0800 Subject: [PATCH 47/51] HID: i2c: let RMI devices decide what constitutes wakeup event HID-RMI is special in the sense that it does not carry HID events directly, but rather uses HID protocol as a wrapper/transport for RMI protocol. Therefore we should not assume that all data coming from the device via interrupt is associated with user activity and report wakeup event indiscriminately, but rather let HID-RMI do that when appropriate. HID-RMI devices tag responses to the commands issued by the host as RMI_READ_DATA_REPORT_ID whereas motion and other input events from the device are tagged as RMI_ATTN_REPORT_ID. Change hid-rmi to report wakeup events when receiving the latter packets. This allows ChromeOS to accurately identify wakeup source and make correct decision on the mode of the resume the system should take ("dark" where the display stays off vs normal one). Fixes: d951ae1ce803 ("HID: i2c-hid: Report wakeup events") Signed-off-by: Dmitry Torokhov Signed-off-by: Jiri Kosina --- drivers/hid/hid-rmi.c | 2 ++ drivers/hid/i2c-hid/i2c-hid-core.c | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c index bb1f423f4ace3..84e7ba5314d3f 100644 --- a/drivers/hid/hid-rmi.c +++ b/drivers/hid/hid-rmi.c @@ -326,6 +326,8 @@ static int rmi_input_event(struct hid_device *hdev, u8 *data, int size) if (!(test_bit(RMI_STARTED, &hdata->flags))) return 0; + pm_wakeup_event(hdev->dev.parent, 0); + local_irq_save(flags); rmi_set_attn_data(rmi_dev, data[1], &data[2], size - 2); diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c index 0667b6022c3b7..a9428b7f34a46 100644 --- a/drivers/hid/i2c-hid/i2c-hid-core.c +++ b/drivers/hid/i2c-hid/i2c-hid-core.c @@ -554,7 +554,8 @@ static void i2c_hid_get_input(struct i2c_hid *ihid) i2c_hid_dbg(ihid, "input: %*ph\n", ret_size, ihid->inbuf); if (test_bit(I2C_HID_STARTED, &ihid->flags)) { - pm_wakeup_event(&ihid->client->dev, 0); + if (ihid->hid->group != HID_GROUP_RMI) + pm_wakeup_event(&ihid->client->dev, 0); hid_input_report(ihid->hid, HID_INPUT_REPORT, ihid->inbuf + sizeof(__le16), From 9d013910df22de91333a0acc81d1dbb115bd76f6 Mon Sep 17 00:00:00 2001 From: Marcus Folkesson Date: Thu, 17 Nov 2022 13:13:26 +0100 Subject: [PATCH 48/51] HID: hid-sensor-custom: set fixed size for custom attributes This is no bugfix (so no Fixes: tag is necessary) as it is taken care of in hid_sensor_custom_add_attributes(). The motivation for this patch is that: hid_sensor_custom_field.attr_name and hid_sensor_custom_field.attrs has the size of HID_CUSTOM_TOTAL_ATTRS and used in same context. We compare against HID_CUSTOM_TOTAL_ATTRS when looping through hid_custom_attrs. We will silent the smatch error: hid_sensor_custom_add_attributes() error: buffer overflow 'hid_custom_attrs' 8 <= 10 Signed-off-by: Marcus Folkesson Acked-by: Jonathan Cameron Signed-off-by: Jiri Kosina --- drivers/hid/hid-sensor-custom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c index a6fc89ee1287c..f444e63e9f36e 100644 --- a/drivers/hid/hid-sensor-custom.c +++ b/drivers/hid/hid-sensor-custom.c @@ -62,7 +62,7 @@ struct hid_sensor_sample { u32 raw_len; } __packed; -static struct attribute hid_custom_attrs[] = { +static struct attribute hid_custom_attrs[HID_CUSTOM_TOTAL_ATTRS] = { {.name = "name", .mode = S_IRUGO}, {.name = "units", .mode = S_IRUGO}, {.name = "unit-expo", .mode = S_IRUGO}, From 989f7cc94f1174269282a4a46bb735c7f11527de Mon Sep 17 00:00:00 2001 From: Marcus Folkesson Date: Thu, 17 Nov 2022 13:13:22 +0100 Subject: [PATCH 49/51] HID: hid-alps: use default remove for hid device hid_device_remove() will call hid_hw_stop() as default .remove function if no function is specified. Signed-off-by: Marcus Folkesson Signed-off-by: Jiri Kosina --- drivers/hid/hid-alps.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/drivers/hid/hid-alps.c b/drivers/hid/hid-alps.c index db146d0f7937e..669d769ea1dcd 100644 --- a/drivers/hid/hid-alps.c +++ b/drivers/hid/hid-alps.c @@ -820,11 +820,6 @@ static int alps_probe(struct hid_device *hdev, const struct hid_device_id *id) return 0; } -static void alps_remove(struct hid_device *hdev) -{ - hid_hw_stop(hdev); -} - static const struct hid_device_id alps_id[] = { { HID_DEVICE(HID_BUS_ANY, HID_GROUP_ANY, USB_VENDOR_ID_ALPS_JP, HID_DEVICE_ID_ALPS_U1_DUAL) }, @@ -842,7 +837,6 @@ static struct hid_driver alps_driver = { .name = "hid-alps", .id_table = alps_id, .probe = alps_probe, - .remove = alps_remove, .raw_event = alps_raw_event, .input_mapping = alps_input_mapping, .input_configured = alps_input_configured, From 163a7fbff7a78c7c055e6c0ad26124ae551fe313 Mon Sep 17 00:00:00 2001 From: Marcus Folkesson Date: Thu, 17 Nov 2022 13:13:14 +0100 Subject: [PATCH 50/51] HID: hid-elan: use default remove for hid device hid_device_remove() will call hid_hw_stop() as default .remove function if no function is specified. Signed-off-by: Marcus Folkesson Signed-off-by: Jiri Kosina --- drivers/hid/hid-elan.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/drivers/hid/hid-elan.c b/drivers/hid/hid-elan.c index 8e4a5528e25df..76d93fc48f6a2 100644 --- a/drivers/hid/hid-elan.c +++ b/drivers/hid/hid-elan.c @@ -507,11 +507,6 @@ static int elan_probe(struct hid_device *hdev, const struct hid_device_id *id) return ret; } -static void elan_remove(struct hid_device *hdev) -{ - hid_hw_stop(hdev); -} - static const struct hid_device_id elan_devices[] = { { HID_USB_DEVICE(USB_VENDOR_ID_ELAN, USB_DEVICE_ID_HP_X2), .driver_data = ELAN_HAS_LED }, @@ -529,7 +524,6 @@ static struct hid_driver elan_driver = { .input_configured = elan_input_configured, .raw_event = elan_raw_event, .probe = elan_probe, - .remove = elan_remove, }; module_hid_driver(elan_driver); From 8b7e58409b1813c58eea542d9f3b8db35b4ac1f7 Mon Sep 17 00:00:00 2001 From: Andreas Bergmeier Date: Fri, 11 Nov 2022 20:45:26 +0100 Subject: [PATCH 51/51] HID: logitech HID++: Send SwID in GetProtocolVersion According to docs a SwID should be sent for GetProtocolVersion. > 0x10.DeviceIndex.0x00.0x1n where n is SwID. Signed-off-by: Andreas Bergmeier Reviewed-by: Bastien Nocera Signed-off-by: Jiri Kosina --- drivers/hid/hid-logitech-hidpp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c index 71a9c258a20bc..9afbc68bf0633 100644 --- a/drivers/hid/hid-logitech-hidpp.c +++ b/drivers/hid/hid-logitech-hidpp.c @@ -895,7 +895,7 @@ static int hidpp_root_get_protocol_version(struct hidpp_device *hidpp) ret = hidpp_send_rap_command_sync(hidpp, REPORT_ID_HIDPP_SHORT, HIDPP_PAGE_ROOT_IDX, - CMD_ROOT_GET_PROTOCOL_VERSION, + CMD_ROOT_GET_PROTOCOL_VERSION | LINUX_KERNEL_SW_ID, ping_data, sizeof(ping_data), &response); if (ret == HIDPP_ERROR_INVALID_SUBID) {