From 1475af255e18f35dc46f8a7acc18354c73d45149 Mon Sep 17 00:00:00 2001 From: Kai-Heng Feng Date: Mon, 7 Jan 2019 15:24:29 +0800 Subject: HID: i2c-hid: Ignore input report if there's no data present on Elan touchpanels While using Elan touchpads, the message floods: [ 136.138487] i2c_hid i2c-DELL08D6:00: i2c_hid_get_input: incomplete report (14/65535) Though the message flood is annoying, the device it self works without any issue. I suspect that the device in question takes too much time to pull the IRQ back to high after I2C host has done reading its data. Since the host receives all useful data, let's ignore the input report when there's no data. Signed-off-by: Kai-Heng Feng Signed-off-by: Benjamin Tissoires --- drivers/hid/i2c-hid/i2c-hid-core.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'drivers/hid') diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c index 8555ce7e737b..2f940c1de616 100644 --- a/drivers/hid/i2c-hid/i2c-hid-core.c +++ b/drivers/hid/i2c-hid/i2c-hid-core.c @@ -50,6 +50,7 @@ #define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET BIT(1) #define I2C_HID_QUIRK_NO_RUNTIME_PM BIT(2) #define I2C_HID_QUIRK_DELAY_AFTER_SLEEP BIT(3) +#define I2C_HID_QUIRK_BOGUS_IRQ BIT(4) /* flags */ #define I2C_HID_STARTED 0 @@ -179,6 +180,8 @@ static const struct i2c_hid_quirks { I2C_HID_QUIRK_DELAY_AFTER_SLEEP }, { USB_VENDOR_ID_LG, I2C_DEVICE_ID_LG_8001, I2C_HID_QUIRK_NO_RUNTIME_PM }, + { USB_VENDOR_ID_ELAN, HID_ANY_ID, + I2C_HID_QUIRK_BOGUS_IRQ }, { 0, 0 } }; @@ -503,6 +506,12 @@ static void i2c_hid_get_input(struct i2c_hid *ihid) return; } + if (ihid->quirks & I2C_HID_QUIRK_BOGUS_IRQ && ret_size == 0xffff) { + dev_warn_once(&ihid->client->dev, "%s: IRQ triggered but " + "there's no data\n", __func__); + return; + } + if ((ret_size > size) || (ret_size < 2)) { dev_err(&ihid->client->dev, "%s: incomplete report (%d/%d)\n", __func__, size, ret_size); -- cgit From 0d28f49412405d87d3aae83da255070a46e67627 Mon Sep 17 00:00:00 2001 From: Hong Liu Date: Tue, 12 Feb 2019 20:05:20 +0800 Subject: HID: intel-ish-hid: avoid binding wrong ishtp_cl_device When performing a warm reset in ishtp bus driver, the ishtp_cl_device will not be removed, its fw_client still points to the already freed ishtp_device.fw_clients array. Later after driver finishing ishtp client enumeration, this dangling pointer may cause driver to bind the wrong ishtp_cl_device to the new client, causing wrong callback to be called for messages intended for the new client. This helps in development of firmware where frequent switching of firmwares is required without Linux reboot. Signed-off-by: Hong Liu Tested-by: Hongyan Song Acked-by: Srinivas Pandruvada Signed-off-by: Jiri Kosina --- drivers/hid/intel-ish-hid/ishtp/bus.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/hid') diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c index 728dc6d4561a..a271d6d169b1 100644 --- a/drivers/hid/intel-ish-hid/ishtp/bus.c +++ b/drivers/hid/intel-ish-hid/ishtp/bus.c @@ -675,7 +675,8 @@ int ishtp_cl_device_bind(struct ishtp_cl *cl) spin_lock_irqsave(&cl->dev->device_list_lock, flags); list_for_each_entry(cl_device, &cl->dev->device_list, device_link) { - if (cl_device->fw_client->client_id == cl->fw_client_id) { + if (cl_device->fw_client && + cl_device->fw_client->client_id == cl->fw_client_id) { cl->device = cl_device; rv = 0; break; @@ -735,6 +736,7 @@ void ishtp_bus_remove_all_clients(struct ishtp_device *ishtp_dev, spin_lock_irqsave(&ishtp_dev->device_list_lock, flags); list_for_each_entry_safe(cl_device, n, &ishtp_dev->device_list, device_link) { + cl_device->fw_client = NULL; if (warm_reset && cl_device->reference_count) continue; -- cgit From b22f805bbe090d42e2eed86aa075687e47f924b7 Mon Sep 17 00:00:00 2001 From: Hong Liu Date: Tue, 12 Feb 2019 20:05:21 +0800 Subject: HID: intel-ish-hid: Optimize writing ipc message from queue Currently we are using one additional static variable and a spinlock to prevent contention of writing IPC messages to ISH hardware, which is not necessary. Once ISH is ready to accept new data, we can push new data to hardware. This pushing of new data is already protected by wr_processing_spinlock for contention, which is enough. So use this spinlock to check both readiness for accepting new data and once ready allow writing of ipc message from queue to ISH hardware. While here, cleaned up some space after return. Signed-off-by: Hong Liu Tested-by: Hongyan Song Acked-by: Srinivas Pandruvada Signed-off-by: Jiri Kosina --- drivers/hid/intel-ish-hid/ipc/ipc.c | 19 +++---------------- drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h | 2 -- 2 files changed, 3 insertions(+), 18 deletions(-) (limited to 'drivers/hid') diff --git a/drivers/hid/intel-ish-hid/ipc/ipc.c b/drivers/hid/intel-ish-hid/ipc/ipc.c index 742191bb24c6..ff8eca11ff73 100644 --- a/drivers/hid/intel-ish-hid/ipc/ipc.c +++ b/drivers/hid/intel-ish-hid/ipc/ipc.c @@ -256,33 +256,22 @@ static int write_ipc_from_queue(struct ishtp_device *dev) int i; void (*ipc_send_compl)(void *); void *ipc_send_compl_prm; - static int out_ipc_locked; - unsigned long out_ipc_flags; if (dev->dev_state == ISHTP_DEV_DISABLED) - return -EINVAL; + return -EINVAL; - spin_lock_irqsave(&dev->out_ipc_spinlock, out_ipc_flags); - if (out_ipc_locked) { - spin_unlock_irqrestore(&dev->out_ipc_spinlock, out_ipc_flags); - return -EBUSY; - } - out_ipc_locked = 1; + spin_lock_irqsave(&dev->wr_processing_spinlock, flags); if (!ish_is_input_ready(dev)) { - out_ipc_locked = 0; - spin_unlock_irqrestore(&dev->out_ipc_spinlock, out_ipc_flags); + spin_unlock_irqrestore(&dev->wr_processing_spinlock, flags); return -EBUSY; } - spin_unlock_irqrestore(&dev->out_ipc_spinlock, out_ipc_flags); - spin_lock_irqsave(&dev->wr_processing_spinlock, flags); /* * if tx send list is empty - return 0; * may happen, as RX_COMPLETE handler doesn't check list emptiness. */ if (list_empty(&dev->wr_processing_list)) { spin_unlock_irqrestore(&dev->wr_processing_spinlock, flags); - out_ipc_locked = 0; return 0; } @@ -333,7 +322,6 @@ static int write_ipc_from_queue(struct ishtp_device *dev) dev->ipc_tx_bytes_cnt += IPC_HEADER_GET_LENGTH(doorbell_val); ish_reg_write(dev, IPC_REG_HOST2ISH_DRBL, doorbell_val); - out_ipc_locked = 0; ipc_send_compl = ipc_link->ipc_send_compl; ipc_send_compl_prm = ipc_link->ipc_send_compl_prm; @@ -914,7 +902,6 @@ struct ishtp_device *ish_dev_init(struct pci_dev *pdev) init_waitqueue_head(&dev->wait_hw_ready); spin_lock_init(&dev->wr_processing_spinlock); - spin_lock_init(&dev->out_ipc_spinlock); /* Init IPC processing and free lists */ INIT_LIST_HEAD(&dev->wr_processing_list); diff --git a/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h b/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h index e7c6bfefaf9e..e54ce1ef27dd 100644 --- a/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h +++ b/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h @@ -211,8 +211,6 @@ struct ishtp_device { /* For both processing list and free list */ spinlock_t wr_processing_spinlock; - spinlock_t out_ipc_spinlock; - struct ishtp_fw_client *fw_clients; /*Note:memory has to be allocated*/ DECLARE_BITMAP(fw_clients_map, ISHTP_CLIENTS_MAX); DECLARE_BITMAP(host_clients_map, ISHTP_CLIENTS_MAX); -- cgit From 7e06e0d5493b3e88016b247fe92f67a071d2c655 Mon Sep 17 00:00:00 2001 From: Hong Liu Date: Tue, 12 Feb 2019 20:05:22 +0800 Subject: HID: intel-ish-hid: move doorbell writing before flush Reading of IPC_REG_ISH_HOST_FWSTS will flush both message register and doorbell. So move the doorbell write before reading of IPC_REG_ISH_HOST_FWSTS. Signed-off-by: Hong Liu Tested-by: Hongyan Song Acked-by: Srinivas Pandruvada Signed-off-by: Jiri Kosina --- drivers/hid/intel-ish-hid/ipc/ipc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/hid') diff --git a/drivers/hid/intel-ish-hid/ipc/ipc.c b/drivers/hid/intel-ish-hid/ipc/ipc.c index ff8eca11ff73..30d8a639a5bb 100644 --- a/drivers/hid/intel-ish-hid/ipc/ipc.c +++ b/drivers/hid/intel-ish-hid/ipc/ipc.c @@ -314,6 +314,8 @@ static int write_ipc_from_queue(struct ishtp_device *dev) memcpy(®, &r_buf[length >> 2], rem); ish_reg_write(dev, reg_addr, reg); } + ish_reg_write(dev, IPC_REG_HOST2ISH_DRBL, doorbell_val); + /* Flush writes to msg registers and doorbell */ ish_reg_read(dev, IPC_REG_ISH_HOST_FWSTS); @@ -321,8 +323,6 @@ static int write_ipc_from_queue(struct ishtp_device *dev) ++dev->ipc_tx_cnt; dev->ipc_tx_bytes_cnt += IPC_HEADER_GET_LENGTH(doorbell_val); - ish_reg_write(dev, IPC_REG_HOST2ISH_DRBL, doorbell_val); - ipc_send_compl = ipc_link->ipc_send_compl; ipc_send_compl_prm = ipc_link->ipc_send_compl_prm; list_del_init(&ipc_link->link); -- cgit From 09cc8b361887787a3577aa0b6510af4b11b51b9e Mon Sep 17 00:00:00 2001 From: Hong Liu Date: Tue, 12 Feb 2019 20:05:23 +0800 Subject: HID: intel-ish-hid: remove data[128] usage on stack when sending HBM request Instead of using an 128-byte on-stack array to store the request, we can instantiate the request on stack directly. This can save the stack usage of these functions, since most of the requests are much smaller than 128 bytes. Signed-off-by: Hong Liu Tested-by: Hongyan Song Acked-by: Srinivas Pandruvada Signed-off-by: Jiri Kosina --- drivers/hid/intel-ish-hid/ishtp/bus.c | 2 +- drivers/hid/intel-ish-hid/ishtp/bus.h | 2 +- drivers/hid/intel-ish-hid/ishtp/hbm.c | 97 +++++++++++++---------------------- 3 files changed, 38 insertions(+), 63 deletions(-) (limited to 'drivers/hid') diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c index a271d6d169b1..f358a02325da 100644 --- a/drivers/hid/intel-ish-hid/ishtp/bus.c +++ b/drivers/hid/intel-ish-hid/ishtp/bus.c @@ -119,7 +119,7 @@ int ishtp_send_msg(struct ishtp_device *dev, struct ishtp_msg_hdr *hdr, * Return: This returns IPC send message status. */ int ishtp_write_message(struct ishtp_device *dev, struct ishtp_msg_hdr *hdr, - unsigned char *buf) + void *buf) { return ishtp_send_msg(dev, hdr, buf, NULL, NULL); } diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.h b/drivers/hid/intel-ish-hid/ishtp/bus.h index b8a5bcc82536..5c4763d73f8b 100644 --- a/drivers/hid/intel-ish-hid/ishtp/bus.h +++ b/drivers/hid/intel-ish-hid/ishtp/bus.h @@ -85,7 +85,7 @@ int ishtp_send_msg(struct ishtp_device *dev, /* Write a single-fragment message */ int ishtp_write_message(struct ishtp_device *dev, struct ishtp_msg_hdr *hdr, - unsigned char *buf); + void *buf); /* Use DMA to send/receive messages */ int ishtp_use_dma_transfer(void); diff --git a/drivers/hid/intel-ish-hid/ishtp/hbm.c b/drivers/hid/intel-ish-hid/ishtp/hbm.c index 8b5dd580ceec..d0b847c86935 100644 --- a/drivers/hid/intel-ish-hid/ishtp/hbm.c +++ b/drivers/hid/intel-ish-hid/ishtp/hbm.c @@ -136,19 +136,14 @@ int ishtp_hbm_start_wait(struct ishtp_device *dev) int ishtp_hbm_start_req(struct ishtp_device *dev) { struct ishtp_msg_hdr hdr; - unsigned char data[128]; - struct ishtp_msg_hdr *ishtp_hdr = &hdr; - struct hbm_host_version_request *start_req; - const size_t len = sizeof(struct hbm_host_version_request); + struct hbm_host_version_request start_req = { 0 }; - ishtp_hbm_hdr(ishtp_hdr, len); + ishtp_hbm_hdr(&hdr, sizeof(start_req)); /* host start message */ - start_req = (struct hbm_host_version_request *)data; - memset(start_req, 0, len); - start_req->hbm_cmd = HOST_START_REQ_CMD; - start_req->host_version.major_version = HBM_MAJOR_VERSION; - start_req->host_version.minor_version = HBM_MINOR_VERSION; + start_req.hbm_cmd = HOST_START_REQ_CMD; + start_req.host_version.major_version = HBM_MAJOR_VERSION; + start_req.host_version.minor_version = HBM_MINOR_VERSION; /* * (!) Response to HBM start may be so quick that this thread would get @@ -156,7 +151,7 @@ int ishtp_hbm_start_req(struct ishtp_device *dev) * So set it at first, change back to ISHTP_HBM_IDLE upon failure */ dev->hbm_state = ISHTP_HBM_START; - if (ishtp_write_message(dev, ishtp_hdr, data)) { + if (ishtp_write_message(dev, &hdr, &start_req)) { dev_err(dev->devc, "version message send failed\n"); dev->dev_state = ISHTP_DEV_RESETTING; dev->hbm_state = ISHTP_HBM_IDLE; @@ -178,19 +173,13 @@ int ishtp_hbm_start_req(struct ishtp_device *dev) void ishtp_hbm_enum_clients_req(struct ishtp_device *dev) { struct ishtp_msg_hdr hdr; - unsigned char data[128]; - struct ishtp_msg_hdr *ishtp_hdr = &hdr; - struct hbm_host_enum_request *enum_req; - const size_t len = sizeof(struct hbm_host_enum_request); + struct hbm_host_enum_request enum_req = { 0 }; /* enumerate clients */ - ishtp_hbm_hdr(ishtp_hdr, len); + ishtp_hbm_hdr(&hdr, sizeof(enum_req)); + enum_req.hbm_cmd = HOST_ENUM_REQ_CMD; - enum_req = (struct hbm_host_enum_request *)data; - memset(enum_req, 0, len); - enum_req->hbm_cmd = HOST_ENUM_REQ_CMD; - - if (ishtp_write_message(dev, ishtp_hdr, data)) { + if (ishtp_write_message(dev, &hdr, &enum_req)) { dev->dev_state = ISHTP_DEV_RESETTING; dev_err(dev->devc, "enumeration request send failed\n"); ish_hw_reset(dev); @@ -208,12 +197,8 @@ void ishtp_hbm_enum_clients_req(struct ishtp_device *dev) */ static int ishtp_hbm_prop_req(struct ishtp_device *dev) { - struct ishtp_msg_hdr hdr; - unsigned char data[128]; - struct ishtp_msg_hdr *ishtp_hdr = &hdr; - struct hbm_props_request *prop_req; - const size_t len = sizeof(struct hbm_props_request); + struct hbm_props_request prop_req = { 0 }; unsigned long next_client_index; uint8_t client_num; @@ -237,15 +222,12 @@ static int ishtp_hbm_prop_req(struct ishtp_device *dev) dev->fw_clients[client_num].client_id = next_client_index; - ishtp_hbm_hdr(ishtp_hdr, len); - prop_req = (struct hbm_props_request *)data; + ishtp_hbm_hdr(&hdr, sizeof(prop_req)); - memset(prop_req, 0, sizeof(struct hbm_props_request)); + prop_req.hbm_cmd = HOST_CLIENT_PROPERTIES_REQ_CMD; + prop_req.address = next_client_index; - prop_req->hbm_cmd = HOST_CLIENT_PROPERTIES_REQ_CMD; - prop_req->address = next_client_index; - - if (ishtp_write_message(dev, ishtp_hdr, data)) { + if (ishtp_write_message(dev, &hdr, &prop_req)) { dev->dev_state = ISHTP_DEV_RESETTING; dev_err(dev->devc, "properties request send failed\n"); ish_hw_reset(dev); @@ -266,19 +248,14 @@ static int ishtp_hbm_prop_req(struct ishtp_device *dev) static void ishtp_hbm_stop_req(struct ishtp_device *dev) { struct ishtp_msg_hdr hdr; - unsigned char data[128]; - struct ishtp_msg_hdr *ishtp_hdr = &hdr; - struct hbm_host_stop_request *req; - const size_t len = sizeof(struct hbm_host_stop_request); + struct hbm_host_stop_request stop_req = { 0 } ; - ishtp_hbm_hdr(ishtp_hdr, len); - req = (struct hbm_host_stop_request *)data; + ishtp_hbm_hdr(&hdr, sizeof(stop_req)); - memset(req, 0, sizeof(struct hbm_host_stop_request)); - req->hbm_cmd = HOST_STOP_REQ_CMD; - req->reason = DRIVER_STOP_REQUEST; + stop_req.hbm_cmd = HOST_STOP_REQ_CMD; + stop_req.reason = DRIVER_STOP_REQUEST; - ishtp_write_message(dev, ishtp_hdr, data); + ishtp_write_message(dev, &hdr, &stop_req); } /** @@ -294,15 +271,15 @@ int ishtp_hbm_cl_flow_control_req(struct ishtp_device *dev, struct ishtp_cl *cl) { struct ishtp_msg_hdr hdr; - unsigned char data[128]; - struct ishtp_msg_hdr *ishtp_hdr = &hdr; - const size_t len = sizeof(struct hbm_flow_control); + struct hbm_flow_control flow_ctrl; + const size_t len = sizeof(flow_ctrl); int rv; unsigned long flags; spin_lock_irqsave(&cl->fc_spinlock, flags); - ishtp_hbm_hdr(ishtp_hdr, len); - ishtp_hbm_cl_hdr(cl, ISHTP_FLOW_CONTROL_CMD, data, len); + + ishtp_hbm_hdr(&hdr, len); + ishtp_hbm_cl_hdr(cl, ISHTP_FLOW_CONTROL_CMD, &flow_ctrl, len); /* * Sync possible race when RB recycle and packet receive paths @@ -315,7 +292,7 @@ int ishtp_hbm_cl_flow_control_req(struct ishtp_device *dev, cl->recv_msg_num_frags = 0; - rv = ishtp_write_message(dev, ishtp_hdr, data); + rv = ishtp_write_message(dev, &hdr, &flow_ctrl); if (!rv) { ++cl->out_flow_ctrl_creds; ++cl->out_flow_ctrl_cnt; @@ -345,14 +322,13 @@ int ishtp_hbm_cl_flow_control_req(struct ishtp_device *dev, int ishtp_hbm_cl_disconnect_req(struct ishtp_device *dev, struct ishtp_cl *cl) { struct ishtp_msg_hdr hdr; - unsigned char data[128]; - struct ishtp_msg_hdr *ishtp_hdr = &hdr; - const size_t len = sizeof(struct hbm_client_connect_request); + struct hbm_client_connect_request disconn_req; + const size_t len = sizeof(disconn_req); - ishtp_hbm_hdr(ishtp_hdr, len); - ishtp_hbm_cl_hdr(cl, CLIENT_DISCONNECT_REQ_CMD, data, len); + ishtp_hbm_hdr(&hdr, len); + ishtp_hbm_cl_hdr(cl, CLIENT_DISCONNECT_REQ_CMD, &disconn_req, len); - return ishtp_write_message(dev, ishtp_hdr, data); + return ishtp_write_message(dev, &hdr, &disconn_req); } /** @@ -391,14 +367,13 @@ static void ishtp_hbm_cl_disconnect_res(struct ishtp_device *dev, int ishtp_hbm_cl_connect_req(struct ishtp_device *dev, struct ishtp_cl *cl) { struct ishtp_msg_hdr hdr; - unsigned char data[128]; - struct ishtp_msg_hdr *ishtp_hdr = &hdr; - const size_t len = sizeof(struct hbm_client_connect_request); + struct hbm_client_connect_request conn_req; + const size_t len = sizeof(conn_req); - ishtp_hbm_hdr(ishtp_hdr, len); - ishtp_hbm_cl_hdr(cl, CLIENT_CONNECT_REQ_CMD, data, len); + ishtp_hbm_hdr(&hdr, len); + ishtp_hbm_cl_hdr(cl, CLIENT_CONNECT_REQ_CMD, &conn_req, len); - return ishtp_write_message(dev, ishtp_hdr, data); + return ishtp_write_message(dev, &hdr, &conn_req); } /** -- cgit