| Message ID | 1343151487-27841-2-git-send-email-zonque@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tuesday, July 24, 2012 10:38 AM, Daniel Mack wrote: > Don't call gpio_to_irq() on GPIOs before gpio_request() succeeded on > them. This avoids Ooopses with incorrect DT bindings. > > Signed-off-by: Daniel Mack <zonque@gmail.com> > --- > drivers/input/misc/rotary_encoder.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c > index 350cbd1..474b04f 100644 > --- a/drivers/input/misc/rotary_encoder.c > +++ b/drivers/input/misc/rotary_encoder.c > @@ -223,8 +223,6 @@ static int __devinit rotary_encoder_probe(struct platform_device *pdev) > > encoder->input = input; > encoder->pdata = pdata; > - encoder->irq_a = gpio_to_irq(pdata->gpio_a); > - encoder->irq_b = gpio_to_irq(pdata->gpio_b); > > /* create and register the input driver */ > input->name = pdev->name; > @@ -275,6 +273,9 @@ static int __devinit rotary_encoder_probe(struct platform_device *pdev) > goto exit_free_gpio_a; > } > > + 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) { > handler = &rotary_encoder_half_period_irq; Daniel, You might want to put this patch first before adding the DT binding stuff. You also might consider using gpio_request_one() or even the devm_* version, instead of doing the gpio_request() followed by the gpio_direction_input(). Actually, if you convert the probe to use the managed resource devm_* stuff it would really cleanup the error path and the remove. Regardless... Reviewed-by: H Hartley Sweeten <hsweeten@visionengravers.com> -- 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 24.07.2012 21:45, H Hartley Sweeten wrote: > On Tuesday, July 24, 2012 10:38 AM, Daniel Mack wrote: >> Don't call gpio_to_irq() on GPIOs before gpio_request() succeeded on >> them. This avoids Ooopses with incorrect DT bindings. >> >> Signed-off-by: Daniel Mack <zonque@gmail.com> >> --- >> drivers/input/misc/rotary_encoder.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c >> index 350cbd1..474b04f 100644 >> --- a/drivers/input/misc/rotary_encoder.c >> +++ b/drivers/input/misc/rotary_encoder.c >> @@ -223,8 +223,6 @@ static int __devinit rotary_encoder_probe(struct platform_device *pdev) >> >> encoder->input = input; >> encoder->pdata = pdata; >> - encoder->irq_a = gpio_to_irq(pdata->gpio_a); >> - encoder->irq_b = gpio_to_irq(pdata->gpio_b); >> >> /* create and register the input driver */ >> input->name = pdev->name; >> @@ -275,6 +273,9 @@ static int __devinit rotary_encoder_probe(struct platform_device *pdev) >> goto exit_free_gpio_a; >> } >> >> + 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) { >> handler = &rotary_encoder_half_period_irq; > > Daniel, > > You might want to put this patch first before adding the DT binding > stuff. > > You also might consider using gpio_request_one() or even the > devm_* version, instead of doing the gpio_request() followed > by the gpio_direction_input(). > > Actually, if you convert the probe to use the managed resource > devm_* stuff it would really cleanup the error path and the remove. Yes, you're right. Thanks for the hint. New version coming up. Does your Reviewed-by: still apply? Daniel -- 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 Wednesday, July 25, 2012 12:43 AM, Daniel Mack wrote: > On 24.07.2012 21:45, H Hartley Sweeten wrote: >> You might want to put this patch first before adding the DT binding >> stuff. >> >> You also might consider using gpio_request_one() or even the >> devm_* version, instead of doing the gpio_request() followed >> by the gpio_direction_input(). >> >> Actually, if you convert the probe to use the managed resource >> devm_* stuff it would really cleanup the error path and the remove. > > Yes, you're right. Thanks for the hint. New version coming up. Does your > Reviewed-by: still apply? As pointed out by Dmitry, the devm_* stuff will not work with this driver. Please update the patch to just use the gpio_request_one() and I'll review it again. Sorry about that. Hartley -- 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
diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c index 350cbd1..474b04f 100644 --- a/drivers/input/misc/rotary_encoder.c +++ b/drivers/input/misc/rotary_encoder.c @@ -223,8 +223,6 @@ static int __devinit rotary_encoder_probe(struct platform_device *pdev) encoder->input = input; encoder->pdata = pdata; - encoder->irq_a = gpio_to_irq(pdata->gpio_a); - encoder->irq_b = gpio_to_irq(pdata->gpio_b); /* create and register the input driver */ input->name = pdev->name; @@ -275,6 +273,9 @@ static int __devinit rotary_encoder_probe(struct platform_device *pdev) goto exit_free_gpio_a; } + 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) { handler = &rotary_encoder_half_period_irq;
Don't call gpio_to_irq() on GPIOs before gpio_request() succeeded on them. This avoids Ooopses with incorrect DT bindings. Signed-off-by: Daniel Mack <zonque@gmail.com> --- drivers/input/misc/rotary_encoder.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)