| Message ID | 1444398416-3073-4-git-send-email-ezequiel@vanguardiasur.com.ar (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Fri, Oct 09, 2015 at 10:46:56AM -0300, Ezequiel Garcia wrote: > As per the recent devicetree binding changes, this commit adds the > support for the new 'steps-per-period' property. > > Legacy properties to specify the rotary behavior, are now deprecated > and instead the new 'steps-per-period' is supported. The default behavior > is retained. > > This allows to support rotary-encoder devices with detents wich are capable > of producing a stable event on each step. > > Signed-off-by: Guido Martínez <guido@vanguardiasur.com.ar> > Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > --- > drivers/input/misc/rotary_encoder.c | 87 +++++++++++++++++++++++++++++++++++-- > include/linux/rotary_encoder.h | 2 +- > 2 files changed, 84 insertions(+), 5 deletions(-) > > diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c > index 15a4cc670a3f..e021615dd3c0 100644 > --- a/drivers/input/misc/rotary_encoder.c > +++ b/drivers/input/misc/rotary_encoder.c > @@ -142,6 +142,55 @@ static irqreturn_t rotary_encoder_half_period_irq(int irq, void *dev_id) > return IRQ_HANDLED; > } > > +static irqreturn_t rotary_encoder_quarter_period_irq(int irq, void *dev_id) > +{ > + struct rotary_encoder *encoder = dev_id; > + unsigned char sum; > + int state; > + > + state = rotary_encoder_get_state(encoder->pdata); > + > + /* > + * We encode the previous and the current state using a byte. > + * The previous state in the MSB nibble, the current state in the LSB > + * nibble. Then use a table to decide the direction of the turn. > + */ > + sum = (encoder->last_stable << 4) + state; > + switch (sum) { > + case 0x31: > + case 0x10: > + case 0x02: > + case 0x23: > + encoder->dir = 0; /* clockwise */ > + break; > + > + case 0x13: > + case 0x01: > + case 0x20: > + case 0x32: > + encoder->dir = 1; /* counter-clockwise */ > + break; > + > + default: > + /* > + * Ignore all other values. This covers the case when the > + * state didn't change (a spurious interrupt) and the > + * cases where the state changed by two steps, making it > + * impossible to tell the direction. > + * > + * In either case, don't report any event and save the > + * state for later. > + */ > + goto out; > + } > + > + rotary_encoder_report_event(encoder); > + > +out: > + encoder->last_stable = state; > + return IRQ_HANDLED; > +} > + > #ifdef CONFIG_OF > static const struct of_device_id rotary_encoder_of_match[] = { > { .compatible = "rotary-encoder", }, > @@ -176,7 +225,26 @@ static struct rotary_encoder_platform_data *rotary_encoder_parse_dt(struct devic > > pdata->relative_axis = of_property_read_bool(np, "rotary-encoder,relative-axis"); > pdata->rollover = of_property_read_bool(np, "rotary-encoder,rollover"); > - pdata->half_period = of_property_read_bool(np, "rotary-encoder,half-period"); > + > + if (!of_get_property(np, "rotary-encoder,steps-per-period", NULL)) { > + /* > + * Fallback to a one step per period behavior if the > + * 'steps-per-period' is not set. > + */ > + pdata->steps_per_period = 1; > + } else { > + of_property_read_u32(np, "rotary-encoder,steps-per-period", > + &pdata->steps_per_period); > + } > + > + /* > + * The 'half-period' property has been deprecated, you must use > + * 'steps-per-period' and set an appropriate value. > + */ > + if (of_get_property(np, "rotary-encoder,half-period", NULL)) { > + pr_warn(FW_WARN "\"rotary-encoder,half-period\" is deprecated\n"); > + pdata->steps_per_period = 2; > + } Hmm, I do not think we need to warn about old DTSes if we consider them ABI. How about if we parse like this: error = of_property_read_u32(np, "rotary-encoder,steps-per-period", &pdata->steps_per_period); if (error) { /* * The 'half-period' property has been deprecated, you must use * 'steps-per-period' and set an appropriate value, but we still * need to parse it to maintain compatibility. */ if (of_property_read_bool(np, "rotary-encoder,half-period")) { pdata->steps_per_period = 2; } else { /* Fallback to a one step per period behavior */ pdata->steps_per_period = 1; } } (no need to resubmit if you are OK with this). Thanks.
On 14 October 2015 at 03:55, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > On Fri, Oct 09, 2015 at 10:46:56AM -0300, Ezequiel Garcia wrote: >> As per the recent devicetree binding changes, this commit adds the >> support for the new 'steps-per-period' property. >> >> Legacy properties to specify the rotary behavior, are now deprecated >> and instead the new 'steps-per-period' is supported. The default behavior >> is retained. >> >> This allows to support rotary-encoder devices with detents wich are capable >> of producing a stable event on each step. >> >> Signed-off-by: Guido Martínez <guido@vanguardiasur.com.ar> >> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> >> --- >> drivers/input/misc/rotary_encoder.c | 87 +++++++++++++++++++++++++++++++++++-- >> include/linux/rotary_encoder.h | 2 +- >> 2 files changed, 84 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c >> index 15a4cc670a3f..e021615dd3c0 100644 >> --- a/drivers/input/misc/rotary_encoder.c >> +++ b/drivers/input/misc/rotary_encoder.c >> @@ -142,6 +142,55 @@ static irqreturn_t rotary_encoder_half_period_irq(int irq, void *dev_id) >> return IRQ_HANDLED; >> } >> >> +static irqreturn_t rotary_encoder_quarter_period_irq(int irq, void *dev_id) >> +{ >> + struct rotary_encoder *encoder = dev_id; >> + unsigned char sum; >> + int state; >> + >> + state = rotary_encoder_get_state(encoder->pdata); >> + >> + /* >> + * We encode the previous and the current state using a byte. >> + * The previous state in the MSB nibble, the current state in the LSB >> + * nibble. Then use a table to decide the direction of the turn. >> + */ >> + sum = (encoder->last_stable << 4) + state; >> + switch (sum) { >> + case 0x31: >> + case 0x10: >> + case 0x02: >> + case 0x23: >> + encoder->dir = 0; /* clockwise */ >> + break; >> + >> + case 0x13: >> + case 0x01: >> + case 0x20: >> + case 0x32: >> + encoder->dir = 1; /* counter-clockwise */ >> + break; >> + >> + default: >> + /* >> + * Ignore all other values. This covers the case when the >> + * state didn't change (a spurious interrupt) and the >> + * cases where the state changed by two steps, making it >> + * impossible to tell the direction. >> + * >> + * In either case, don't report any event and save the >> + * state for later. >> + */ >> + goto out; >> + } >> + >> + rotary_encoder_report_event(encoder); >> + >> +out: >> + encoder->last_stable = state; >> + return IRQ_HANDLED; >> +} >> + >> #ifdef CONFIG_OF >> static const struct of_device_id rotary_encoder_of_match[] = { >> { .compatible = "rotary-encoder", }, >> @@ -176,7 +225,26 @@ static struct rotary_encoder_platform_data *rotary_encoder_parse_dt(struct devic >> >> pdata->relative_axis = of_property_read_bool(np, "rotary-encoder,relative-axis"); >> pdata->rollover = of_property_read_bool(np, "rotary-encoder,rollover"); >> - pdata->half_period = of_property_read_bool(np, "rotary-encoder,half-period"); >> + >> + if (!of_get_property(np, "rotary-encoder,steps-per-period", NULL)) { >> + /* >> + * Fallback to a one step per period behavior if the >> + * 'steps-per-period' is not set. >> + */ >> + pdata->steps_per_period = 1; >> + } else { >> + of_property_read_u32(np, "rotary-encoder,steps-per-period", >> + &pdata->steps_per_period); >> + } >> + >> + /* >> + * The 'half-period' property has been deprecated, you must use >> + * 'steps-per-period' and set an appropriate value. >> + */ >> + if (of_get_property(np, "rotary-encoder,half-period", NULL)) { >> + pr_warn(FW_WARN "\"rotary-encoder,half-period\" is deprecated\n"); >> + pdata->steps_per_period = 2; >> + } > > Hmm, I do not think we need to warn about old DTSes if we consider them > ABI. How about if we parse like this: > > error = of_property_read_u32(np, "rotary-encoder,steps-per-period", > &pdata->steps_per_period); > if (error) { > /* > * The 'half-period' property has been deprecated, you must use > * 'steps-per-period' and set an appropriate value, but we still > * need to parse it to maintain compatibility. > */ > if (of_property_read_bool(np, "rotary-encoder,half-period")) { > pdata->steps_per_period = 2; > } else { > /* Fallback to a one step per period behavior */ > pdata->steps_per_period = 1; > } > } > > > (no need to resubmit if you are OK with this). > Looks good to me. Feel free to pick the patches and amend them as above. Thanks a lot!
diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c index 15a4cc670a3f..e021615dd3c0 100644 --- a/drivers/input/misc/rotary_encoder.c +++ b/drivers/input/misc/rotary_encoder.c @@ -142,6 +142,55 @@ static irqreturn_t rotary_encoder_half_period_irq(int irq, void *dev_id) return IRQ_HANDLED; } +static irqreturn_t rotary_encoder_quarter_period_irq(int irq, void *dev_id) +{ + struct rotary_encoder *encoder = dev_id; + unsigned char sum; + int state; + + state = rotary_encoder_get_state(encoder->pdata); + + /* + * We encode the previous and the current state using a byte. + * The previous state in the MSB nibble, the current state in the LSB + * nibble. Then use a table to decide the direction of the turn. + */ + sum = (encoder->last_stable << 4) + state; + switch (sum) { + case 0x31: + case 0x10: + case 0x02: + case 0x23: + encoder->dir = 0; /* clockwise */ + break; + + case 0x13: + case 0x01: + case 0x20: + case 0x32: + encoder->dir = 1; /* counter-clockwise */ + break; + + default: + /* + * Ignore all other values. This covers the case when the + * state didn't change (a spurious interrupt) and the + * cases where the state changed by two steps, making it + * impossible to tell the direction. + * + * In either case, don't report any event and save the + * state for later. + */ + goto out; + } + + rotary_encoder_report_event(encoder); + +out: + encoder->last_stable = state; + return IRQ_HANDLED; +} + #ifdef CONFIG_OF static const struct of_device_id rotary_encoder_of_match[] = { { .compatible = "rotary-encoder", }, @@ -176,7 +225,26 @@ static struct rotary_encoder_platform_data *rotary_encoder_parse_dt(struct devic pdata->relative_axis = of_property_read_bool(np, "rotary-encoder,relative-axis"); pdata->rollover = of_property_read_bool(np, "rotary-encoder,rollover"); - pdata->half_period = of_property_read_bool(np, "rotary-encoder,half-period"); + + if (!of_get_property(np, "rotary-encoder,steps-per-period", NULL)) { + /* + * Fallback to a one step per period behavior if the + * 'steps-per-period' is not set. + */ + pdata->steps_per_period = 1; + } else { + of_property_read_u32(np, "rotary-encoder,steps-per-period", + &pdata->steps_per_period); + } + + /* + * The 'half-period' property has been deprecated, you must use + * 'steps-per-period' and set an appropriate value. + */ + if (of_get_property(np, "rotary-encoder,half-period", NULL)) { + pr_warn(FW_WARN "\"rotary-encoder,half-period\" is deprecated\n"); + pdata->steps_per_period = 2; + } return pdata; } @@ -247,12 +315,23 @@ static int rotary_encoder_probe(struct platform_device *pdev) encoder->irq_a = gpio_to_irq(pdata->gpio_a); encoder->irq_b = gpio_to_irq(pdata->gpio_b); - /* request the IRQs */ - if (pdata->half_period) { + switch (pdata->steps_per_period) { + case 4: + handler = &rotary_encoder_quarter_period_irq; + encoder->last_stable = rotary_encoder_get_state(pdata); + break; + case 2: handler = &rotary_encoder_half_period_irq; encoder->last_stable = rotary_encoder_get_state(pdata); - } else { + break; + case 1: handler = &rotary_encoder_irq; + break; + default: + dev_err(dev, "'%d' is not a valid steps-per-period value\n", + pdata->steps_per_period); + err = -EINVAL; + goto exit_free_gpio_b; } err = request_irq(encoder->irq_a, handler, diff --git a/include/linux/rotary_encoder.h b/include/linux/rotary_encoder.h index 3f594dce5716..0bebba31908c 100644 --- a/include/linux/rotary_encoder.h +++ b/include/linux/rotary_encoder.h @@ -10,7 +10,7 @@ struct rotary_encoder_platform_data { unsigned int inverted_b; bool relative_axis; bool rollover; - bool half_period; + unsigned int steps_per_period; }; #endif /* __ROTARY_ENCODER_H__ */