Re: [PATCHv1 2/2] Input: EXC3000: Add support to query model and fw_version
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2019-11-13 00:23:45
Also in:
lkml
Hi Sebastian, On Thu, Nov 07, 2019 at 07:10:10PM +0100, Sebastian Reichel wrote:
quoted hunk ↗ jump to hunk
Expose model and fw_version via sysfs. Also query the model in probe to make sure, that the I2C communication with the device works before successfully probing the driver. Signed-off-by: Sebastian Reichel <redacted> --- drivers/input/touchscreen/exc3000.c | 136 ++++++++++++++++++++++++++++ 1 file changed, 136 insertions(+)diff --git a/drivers/input/touchscreen/exc3000.c b/drivers/input/touchscreen/exc3000.c index 7d695022082c..4c9f132629b9 100644 --- a/drivers/input/touchscreen/exc3000.c +++ b/drivers/input/touchscreen/exc3000.c@@ -41,6 +41,11 @@ struct exc3000_data { struct touchscreen_properties prop; struct timer_list timer; u8 buf[2 * EXC3000_LEN_FRAME]; + char model[11]; + char fw_version[6]; + struct completion wait_event; + struct mutex query_lock; + int query_result; }; static void exc3000_report_slots(struct input_dev *input,@@ -121,6 +126,35 @@ static int exc3000_read_data(struct i2c_client *client, return 0; } +static void exc3000_query_interrupt(struct exc3000_data *data) +{ + u8 *buf = data->buf; + int err; + + data->query_result = 0; + + err = i2c_master_recv(data->client, buf, EXC3000_LEN_FRAME); + if (err < 0) { + data->query_result = err; + goto out; + } + + if (buf[0] == 0x42 && buf[4] == 'E') { + memcpy(data->model, buf+5, 10); + data->model[10] = '\0'; + goto out; + } else if (buf[0] == 0x42 && buf[4] == 'D') { + memcpy(data->fw_version, buf+5, 5); + data->fw_version[5] = '\0'; + goto out; + } + + data->query_result = -EPROTO; + +out: + complete(&data->wait_event); +} + static irqreturn_t exc3000_interrupt(int irq, void *dev_id) { struct exc3000_data *data = dev_id;@@ -129,6 +163,11 @@ static irqreturn_t exc3000_interrupt(int irq, void *dev_id) int slots, total_slots; int error; + if (mutex_is_locked(&data->query_lock)) { + exc3000_query_interrupt(data); + goto out; + } + error = exc3000_read_data(data->client, buf, &total_slots); if (error) { /* Schedule a timer to release "stuck" contacts */@@ -156,12 +195,92 @@ static irqreturn_t exc3000_interrupt(int irq, void *dev_id) return IRQ_HANDLED; } +static int exc3000_get_fw_version(struct device *dev, struct device_attribute *attr, char *buf) +{ + struct exc3000_data *data = dev_get_drvdata(dev); + static const u8 request[68] = + {0x67, 0x00, 0x42, 0x00, 0x03, 0x01, 'D', 0x00}; + struct i2c_client *client = data->client; + int err; + + mutex_lock(&data->query_lock); + + data->query_result = -ETIMEDOUT; + reinit_completion(&data->wait_event); + + err = i2c_master_send(client, request, sizeof(request)); + if (err < 0) { + mutex_unlock(&data->query_lock); + return err; + } + + wait_for_completion_interruptible_timeout(&data->wait_event, 1*HZ); + mutex_unlock(&data->query_lock); + + if (data->query_result < 0) + return data->query_result; + + return sprintf(buf, "%s\n", data->fw_version); +} +static DEVICE_ATTR(fw_version, S_IRUGO, exc3000_get_fw_version, NULL); + +static ssize_t exc3000_get_model(struct exc3000_data *data) +{ + static const u8 request[68] = + {0x67, 0x00, 0x42, 0x00, 0x03, 0x01, 'E', 0x00}; + struct i2c_client *client = data->client; + int err; + + mutex_lock(&data->query_lock); + data->query_result = -ETIMEDOUT; + reinit_completion(&data->wait_event); + + err = i2c_master_send(client, request, sizeof(request)); + if (err < 0) { + mutex_unlock(&data->query_lock); + return err; + } + + wait_for_completion_interruptible_timeout(&data->wait_event, 1*HZ); + mutex_unlock(&data->query_lock); + + return data->query_result; +} + +static ssize_t exc3000_get_model_sysfs(struct device *dev, struct device_attribute *attr, char *buf) +{ + struct exc3000_data *data = dev_get_drvdata(dev); + int err = exc3000_get_model(data);
Do we really need to re-fetch model (and firmware ID) on each access? Can we query it as probe time and cache? This I think would simplify the driver, as you probably would not need to hook it into the ISR. Can you just post a read/write transaction to fetch it without waiting for interrupt? Or, if single transaction does not work and you need to wait for certain time for response - just add msleep() and maybe mark driver for async probe... Thanks. -- Dmitry