| Message ID | 1436791779-21798-1-git-send-email-dirk.behme@de.bosch.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jul 13, 2015 at 02:49:37PM +0200, Dirk Behme wrote: > From: Oleksij Rempel <external.Oleksij.Rempel@de.bosch.com> > > Remove the IRQ GPIO polling and request. Existing DTS should not be affected > since the IRQ registration was and is based on "interrupts" descriptor of DTS. But this means that consecutive touchscreen readings will be delayed by the time it takes to schedule the thread. What is the motivation for this change? Thanks. > > Signed-off-by: Oleksij Rempel <external.Oleksij.Rempel@de.bosch.com> > Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com> > --- > > Note: All 3 patches in this series are against zforce in input next > https://git.kernel.org/cgit/linux/kernel/git/dtor/input.git/log/?h=next > to fit on top of the previous zforce gpiod change. > > drivers/input/touchscreen/zforce_ts.c | 135 ++++++++++++++++------------------ > 1 file changed, 62 insertions(+), 73 deletions(-) > > diff --git a/drivers/input/touchscreen/zforce_ts.c b/drivers/input/touchscreen/zforce_ts.c > index 32749db..19dc297 100644 > --- a/drivers/input/touchscreen/zforce_ts.c > +++ b/drivers/input/touchscreen/zforce_ts.c > @@ -510,73 +510,71 @@ static irqreturn_t zforce_irq_thread(int irq, void *dev_id) > if (!ts->suspending && device_may_wakeup(&client->dev)) > pm_stay_awake(&client->dev); > > - while (!gpiod_get_value_cansleep(ts->gpio_int)) { > - ret = zforce_read_packet(ts, payload_buffer); > - if (ret < 0) { > - dev_err(&client->dev, > - "could not read packet, ret: %d\n", ret); > - break; > - } > + ret = zforce_read_packet(ts, payload_buffer); > + if (ret < 0) { > + dev_err(&client->dev, > + "could not read packet, ret: %d\n", ret); > + return IRQ_HANDLED; > + } > + > + payload = &payload_buffer[PAYLOAD_BODY]; > > - payload = &payload_buffer[PAYLOAD_BODY]; > - > - switch (payload[RESPONSE_ID]) { > - case NOTIFICATION_TOUCH: > - /* > - * Always report touch-events received while > - * suspending, when being a wakeup source > - */ > - if (ts->suspending && device_may_wakeup(&client->dev)) > - pm_wakeup_event(&client->dev, 500); > - zforce_touch_event(ts, &payload[RESPONSE_DATA]); > - break; > - > - case NOTIFICATION_BOOTCOMPLETE: > - ts->boot_complete = payload[RESPONSE_DATA]; > - zforce_complete(ts, payload[RESPONSE_ID], 0); > - break; > - > - case RESPONSE_INITIALIZE: > - case RESPONSE_DEACTIVATE: > - case RESPONSE_SETCONFIG: > - case RESPONSE_RESOLUTION: > - case RESPONSE_SCANFREQ: > - zforce_complete(ts, payload[RESPONSE_ID], > - payload[RESPONSE_DATA]); > - break; > - > - case RESPONSE_STATUS: > - /* > - * Version Payload Results > - * [2:major] [2:minor] [2:build] [2:rev] > - */ > - ts->version_major = (payload[RESPONSE_DATA + 1] << 8) | > - payload[RESPONSE_DATA]; > - ts->version_minor = (payload[RESPONSE_DATA + 3] << 8) | > - payload[RESPONSE_DATA + 2]; > - ts->version_build = (payload[RESPONSE_DATA + 5] << 8) | > - payload[RESPONSE_DATA + 4]; > - ts->version_rev = (payload[RESPONSE_DATA + 7] << 8) | > - payload[RESPONSE_DATA + 6]; > - dev_dbg(&ts->client->dev, > - "Firmware Version %04x:%04x %04x:%04x\n", > - ts->version_major, ts->version_minor, > - ts->version_build, ts->version_rev); > - > - zforce_complete(ts, payload[RESPONSE_ID], 0); > - break; > - > - case NOTIFICATION_INVALID_COMMAND: > - dev_err(&ts->client->dev, "invalid command: 0x%x\n", > + switch (payload[RESPONSE_ID]) { > + case NOTIFICATION_TOUCH: > + /* > + * Always report touch-events received while > + * suspending, when being a wakeup source > + */ > + if (ts->suspending && device_may_wakeup(&client->dev)) > + pm_wakeup_event(&client->dev, 500); > + zforce_touch_event(ts, &payload[RESPONSE_DATA]); > + break; > + > + case NOTIFICATION_BOOTCOMPLETE: > + ts->boot_complete = payload[RESPONSE_DATA]; > + zforce_complete(ts, payload[RESPONSE_ID], 0); > + break; > + > + case RESPONSE_INITIALIZE: > + case RESPONSE_DEACTIVATE: > + case RESPONSE_SETCONFIG: > + case RESPONSE_RESOLUTION: > + case RESPONSE_SCANFREQ: > + zforce_complete(ts, payload[RESPONSE_ID], > payload[RESPONSE_DATA]); > - break; > + break; > > - default: > - dev_err(&ts->client->dev, > - "unrecognized response id: 0x%x\n", > - payload[RESPONSE_ID]); > - break; > - } > + case RESPONSE_STATUS: > + /* > + * Version Payload Results > + * [2:major] [2:minor] [2:build] [2:rev] > + */ > + ts->version_major = (payload[RESPONSE_DATA + 1] << 8) | > + payload[RESPONSE_DATA]; > + ts->version_minor = (payload[RESPONSE_DATA + 3] << 8) | > + payload[RESPONSE_DATA + 2]; > + ts->version_build = (payload[RESPONSE_DATA + 5] << 8) | > + payload[RESPONSE_DATA + 4]; > + ts->version_rev = (payload[RESPONSE_DATA + 7] << 8) | > + payload[RESPONSE_DATA + 6]; > + dev_dbg(&ts->client->dev, > + "Firmware Version %04x:%04x %04x:%04x\n", > + ts->version_major, ts->version_minor, > + ts->version_build, ts->version_rev); > + > + zforce_complete(ts, payload[RESPONSE_ID], 0); > + break; > + > + case NOTIFICATION_INVALID_COMMAND: > + dev_err(&ts->client->dev, "invalid command: 0x%x\n", > + payload[RESPONSE_DATA]); > + break; > + > + default: > + dev_err(&ts->client->dev, > + "unrecognized response id: 0x%x\n", > + payload[RESPONSE_ID]); > + break; > } > > if (!ts->suspending && device_may_wakeup(&client->dev)) > @@ -754,15 +752,6 @@ static int zforce_probe(struct i2c_client *client, > if (!ts) > return -ENOMEM; > > - /* INT GPIO */ > - ts->gpio_int = devm_gpiod_get_index(&client->dev, NULL, 0, GPIOD_IN); > - if (IS_ERR(ts->gpio_int)) { > - ret = PTR_ERR(ts->gpio_int); > - dev_err(&client->dev, > - "failed to request interrupt GPIO: %d\n", ret); > - return ret; > - } > - > /* RST GPIO */ > ts->gpio_rst = devm_gpiod_get_index(&client->dev, NULL, 1, > GPIOD_OUT_HIGH); > -- > 2.3.4 >
Hi Dmitry, On 13.07.2015 19:04, Dmitry Torokhov wrote: > On Mon, Jul 13, 2015 at 02:49:37PM +0200, Dirk Behme wrote: >> From: Oleksij Rempel <external.Oleksij.Rempel@de.bosch.com> >> >> Remove the IRQ GPIO polling and request. Existing DTS should not be affected >> since the IRQ registration was and is based on "interrupts" descriptor of DTS. > > But this means that consecutive touchscreen readings will be delayed by > the time it takes to schedule the thread. > > What is the motivation for this change? This is the generic part we've done for a special hardware configuration: There is some hardware which uses an I2C Serializer / Deserializer (SerDes) to communicate with the zFroce touch driver. In this case the SerDes will be configured as an interrupt controller and the zForce driver will have no access to poll the GPIO line. Best regards Di >> >> Signed-off-by: Oleksij Rempel <external.Oleksij.Rempel@de.bosch.com> >> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com> >> --- >> >> Note: All 3 patches in this series are against zforce in input next >> https://git.kernel.org/cgit/linux/kernel/git/dtor/input.git/log/?h=next >> to fit on top of the previous zforce gpiod change. >> >> drivers/input/touchscreen/zforce_ts.c | 135 ++++++++++++++++------------------ >> 1 file changed, 62 insertions(+), 73 deletions(-) >> >> diff --git a/drivers/input/touchscreen/zforce_ts.c b/drivers/input/touchscreen/zforce_ts.c >> index 32749db..19dc297 100644 >> --- a/drivers/input/touchscreen/zforce_ts.c >> +++ b/drivers/input/touchscreen/zforce_ts.c >> @@ -510,73 +510,71 @@ static irqreturn_t zforce_irq_thread(int irq, void *dev_id) >> if (!ts->suspending && device_may_wakeup(&client->dev)) >> pm_stay_awake(&client->dev); >> >> - while (!gpiod_get_value_cansleep(ts->gpio_int)) { >> - ret = zforce_read_packet(ts, payload_buffer); >> - if (ret < 0) { >> - dev_err(&client->dev, >> - "could not read packet, ret: %d\n", ret); >> - break; >> - } >> + ret = zforce_read_packet(ts, payload_buffer); >> + if (ret < 0) { >> + dev_err(&client->dev, >> + "could not read packet, ret: %d\n", ret); >> + return IRQ_HANDLED; >> + } >> + >> + payload = &payload_buffer[PAYLOAD_BODY]; >> >> - payload = &payload_buffer[PAYLOAD_BODY]; >> - >> - switch (payload[RESPONSE_ID]) { >> - case NOTIFICATION_TOUCH: >> - /* >> - * Always report touch-events received while >> - * suspending, when being a wakeup source >> - */ >> - if (ts->suspending && device_may_wakeup(&client->dev)) >> - pm_wakeup_event(&client->dev, 500); >> - zforce_touch_event(ts, &payload[RESPONSE_DATA]); >> - break; >> - >> - case NOTIFICATION_BOOTCOMPLETE: >> - ts->boot_complete = payload[RESPONSE_DATA]; >> - zforce_complete(ts, payload[RESPONSE_ID], 0); >> - break; >> - >> - case RESPONSE_INITIALIZE: >> - case RESPONSE_DEACTIVATE: >> - case RESPONSE_SETCONFIG: >> - case RESPONSE_RESOLUTION: >> - case RESPONSE_SCANFREQ: >> - zforce_complete(ts, payload[RESPONSE_ID], >> - payload[RESPONSE_DATA]); >> - break; >> - >> - case RESPONSE_STATUS: >> - /* >> - * Version Payload Results >> - * [2:major] [2:minor] [2:build] [2:rev] >> - */ >> - ts->version_major = (payload[RESPONSE_DATA + 1] << 8) | >> - payload[RESPONSE_DATA]; >> - ts->version_minor = (payload[RESPONSE_DATA + 3] << 8) | >> - payload[RESPONSE_DATA + 2]; >> - ts->version_build = (payload[RESPONSE_DATA + 5] << 8) | >> - payload[RESPONSE_DATA + 4]; >> - ts->version_rev = (payload[RESPONSE_DATA + 7] << 8) | >> - payload[RESPONSE_DATA + 6]; >> - dev_dbg(&ts->client->dev, >> - "Firmware Version %04x:%04x %04x:%04x\n", >> - ts->version_major, ts->version_minor, >> - ts->version_build, ts->version_rev); >> - >> - zforce_complete(ts, payload[RESPONSE_ID], 0); >> - break; >> - >> - case NOTIFICATION_INVALID_COMMAND: >> - dev_err(&ts->client->dev, "invalid command: 0x%x\n", >> + switch (payload[RESPONSE_ID]) { >> + case NOTIFICATION_TOUCH: >> + /* >> + * Always report touch-events received while >> + * suspending, when being a wakeup source >> + */ >> + if (ts->suspending && device_may_wakeup(&client->dev)) >> + pm_wakeup_event(&client->dev, 500); >> + zforce_touch_event(ts, &payload[RESPONSE_DATA]); >> + break; >> + >> + case NOTIFICATION_BOOTCOMPLETE: >> + ts->boot_complete = payload[RESPONSE_DATA]; >> + zforce_complete(ts, payload[RESPONSE_ID], 0); >> + break; >> + >> + case RESPONSE_INITIALIZE: >> + case RESPONSE_DEACTIVATE: >> + case RESPONSE_SETCONFIG: >> + case RESPONSE_RESOLUTION: >> + case RESPONSE_SCANFREQ: >> + zforce_complete(ts, payload[RESPONSE_ID], >> payload[RESPONSE_DATA]); >> - break; >> + break; >> >> - default: >> - dev_err(&ts->client->dev, >> - "unrecognized response id: 0x%x\n", >> - payload[RESPONSE_ID]); >> - break; >> - } >> + case RESPONSE_STATUS: >> + /* >> + * Version Payload Results >> + * [2:major] [2:minor] [2:build] [2:rev] >> + */ >> + ts->version_major = (payload[RESPONSE_DATA + 1] << 8) | >> + payload[RESPONSE_DATA]; >> + ts->version_minor = (payload[RESPONSE_DATA + 3] << 8) | >> + payload[RESPONSE_DATA + 2]; >> + ts->version_build = (payload[RESPONSE_DATA + 5] << 8) | >> + payload[RESPONSE_DATA + 4]; >> + ts->version_rev = (payload[RESPONSE_DATA + 7] << 8) | >> + payload[RESPONSE_DATA + 6]; >> + dev_dbg(&ts->client->dev, >> + "Firmware Version %04x:%04x %04x:%04x\n", >> + ts->version_major, ts->version_minor, >> + ts->version_build, ts->version_rev); >> + >> + zforce_complete(ts, payload[RESPONSE_ID], 0); >> + break; >> + >> + case NOTIFICATION_INVALID_COMMAND: >> + dev_err(&ts->client->dev, "invalid command: 0x%x\n", >> + payload[RESPONSE_DATA]); >> + break; >> + >> + default: >> + dev_err(&ts->client->dev, >> + "unrecognized response id: 0x%x\n", >> + payload[RESPONSE_ID]); >> + break; >> } >> >> if (!ts->suspending && device_may_wakeup(&client->dev)) >> @@ -754,15 +752,6 @@ static int zforce_probe(struct i2c_client *client, >> if (!ts) >> return -ENOMEM; >> >> - /* INT GPIO */ >> - ts->gpio_int = devm_gpiod_get_index(&client->dev, NULL, 0, GPIOD_IN); >> - if (IS_ERR(ts->gpio_int)) { >> - ret = PTR_ERR(ts->gpio_int); >> - dev_err(&client->dev, >> - "failed to request interrupt GPIO: %d\n", ret); >> - return ret; >> - } >> - >> /* RST GPIO */ >> ts->gpio_rst = devm_gpiod_get_index(&client->dev, NULL, 1, >> GPIOD_OUT_HIGH); >> -- >> 2.3.4 >> -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jul 14, 2015 at 09:42:00AM +0200, Dirk Behme wrote: > Hi Dmitry, > > On 13.07.2015 19:04, Dmitry Torokhov wrote: > >On Mon, Jul 13, 2015 at 02:49:37PM +0200, Dirk Behme wrote: > >>From: Oleksij Rempel <external.Oleksij.Rempel@de.bosch.com> > >> > >>Remove the IRQ GPIO polling and request. Existing DTS should not be affected > >>since the IRQ registration was and is based on "interrupts" descriptor of DTS. > > > >But this means that consecutive touchscreen readings will be delayed by > >the time it takes to schedule the thread. > > > >What is the motivation for this change? > > > This is the generic part we've done for a special hardware > configuration: There is some hardware which uses an I2C Serializer / > Deserializer (SerDes) to communicate with the zFroce touch driver. > In this case the SerDes will be configured as an interrupt > controller and the zForce driver will have no access to poll the > GPIO line. In this case can we make gpio optional and use it if it is specified falling back on one read per ISR invocation in its absence? Thanks.
diff --git a/drivers/input/touchscreen/zforce_ts.c b/drivers/input/touchscreen/zforce_ts.c index 32749db..19dc297 100644 --- a/drivers/input/touchscreen/zforce_ts.c +++ b/drivers/input/touchscreen/zforce_ts.c @@ -510,73 +510,71 @@ static irqreturn_t zforce_irq_thread(int irq, void *dev_id) if (!ts->suspending && device_may_wakeup(&client->dev)) pm_stay_awake(&client->dev); - while (!gpiod_get_value_cansleep(ts->gpio_int)) { - ret = zforce_read_packet(ts, payload_buffer); - if (ret < 0) { - dev_err(&client->dev, - "could not read packet, ret: %d\n", ret); - break; - } + ret = zforce_read_packet(ts, payload_buffer); + if (ret < 0) { + dev_err(&client->dev, + "could not read packet, ret: %d\n", ret); + return IRQ_HANDLED; + } + + payload = &payload_buffer[PAYLOAD_BODY]; - payload = &payload_buffer[PAYLOAD_BODY]; - - switch (payload[RESPONSE_ID]) { - case NOTIFICATION_TOUCH: - /* - * Always report touch-events received while - * suspending, when being a wakeup source - */ - if (ts->suspending && device_may_wakeup(&client->dev)) - pm_wakeup_event(&client->dev, 500); - zforce_touch_event(ts, &payload[RESPONSE_DATA]); - break; - - case NOTIFICATION_BOOTCOMPLETE: - ts->boot_complete = payload[RESPONSE_DATA]; - zforce_complete(ts, payload[RESPONSE_ID], 0); - break; - - case RESPONSE_INITIALIZE: - case RESPONSE_DEACTIVATE: - case RESPONSE_SETCONFIG: - case RESPONSE_RESOLUTION: - case RESPONSE_SCANFREQ: - zforce_complete(ts, payload[RESPONSE_ID], - payload[RESPONSE_DATA]); - break; - - case RESPONSE_STATUS: - /* - * Version Payload Results - * [2:major] [2:minor] [2:build] [2:rev] - */ - ts->version_major = (payload[RESPONSE_DATA + 1] << 8) | - payload[RESPONSE_DATA]; - ts->version_minor = (payload[RESPONSE_DATA + 3] << 8) | - payload[RESPONSE_DATA + 2]; - ts->version_build = (payload[RESPONSE_DATA + 5] << 8) | - payload[RESPONSE_DATA + 4]; - ts->version_rev = (payload[RESPONSE_DATA + 7] << 8) | - payload[RESPONSE_DATA + 6]; - dev_dbg(&ts->client->dev, - "Firmware Version %04x:%04x %04x:%04x\n", - ts->version_major, ts->version_minor, - ts->version_build, ts->version_rev); - - zforce_complete(ts, payload[RESPONSE_ID], 0); - break; - - case NOTIFICATION_INVALID_COMMAND: - dev_err(&ts->client->dev, "invalid command: 0x%x\n", + switch (payload[RESPONSE_ID]) { + case NOTIFICATION_TOUCH: + /* + * Always report touch-events received while + * suspending, when being a wakeup source + */ + if (ts->suspending && device_may_wakeup(&client->dev)) + pm_wakeup_event(&client->dev, 500); + zforce_touch_event(ts, &payload[RESPONSE_DATA]); + break; + + case NOTIFICATION_BOOTCOMPLETE: + ts->boot_complete = payload[RESPONSE_DATA]; + zforce_complete(ts, payload[RESPONSE_ID], 0); + break; + + case RESPONSE_INITIALIZE: + case RESPONSE_DEACTIVATE: + case RESPONSE_SETCONFIG: + case RESPONSE_RESOLUTION: + case RESPONSE_SCANFREQ: + zforce_complete(ts, payload[RESPONSE_ID], payload[RESPONSE_DATA]); - break; + break; - default: - dev_err(&ts->client->dev, - "unrecognized response id: 0x%x\n", - payload[RESPONSE_ID]); - break; - } + case RESPONSE_STATUS: + /* + * Version Payload Results + * [2:major] [2:minor] [2:build] [2:rev] + */ + ts->version_major = (payload[RESPONSE_DATA + 1] << 8) | + payload[RESPONSE_DATA]; + ts->version_minor = (payload[RESPONSE_DATA + 3] << 8) | + payload[RESPONSE_DATA + 2]; + ts->version_build = (payload[RESPONSE_DATA + 5] << 8) | + payload[RESPONSE_DATA + 4]; + ts->version_rev = (payload[RESPONSE_DATA + 7] << 8) | + payload[RESPONSE_DATA + 6]; + dev_dbg(&ts->client->dev, + "Firmware Version %04x:%04x %04x:%04x\n", + ts->version_major, ts->version_minor, + ts->version_build, ts->version_rev); + + zforce_complete(ts, payload[RESPONSE_ID], 0); + break; + + case NOTIFICATION_INVALID_COMMAND: + dev_err(&ts->client->dev, "invalid command: 0x%x\n", + payload[RESPONSE_DATA]); + break; + + default: + dev_err(&ts->client->dev, + "unrecognized response id: 0x%x\n", + payload[RESPONSE_ID]); + break; } if (!ts->suspending && device_may_wakeup(&client->dev)) @@ -754,15 +752,6 @@ static int zforce_probe(struct i2c_client *client, if (!ts) return -ENOMEM; - /* INT GPIO */ - ts->gpio_int = devm_gpiod_get_index(&client->dev, NULL, 0, GPIOD_IN); - if (IS_ERR(ts->gpio_int)) { - ret = PTR_ERR(ts->gpio_int); - dev_err(&client->dev, - "failed to request interrupt GPIO: %d\n", ret); - return ret; - } - /* RST GPIO */ ts->gpio_rst = devm_gpiod_get_index(&client->dev, NULL, 1, GPIOD_OUT_HIGH);