The gpiochip_lock_as_irq call can fail and return an error, while the
irq_startup is not expected to fail (returns an unsigned int which is not
checked by irq core code).
irq_request/release_resources functions have been created to address this
problem.
Move gpiochip_lock/unlock_as_irq calls into
irq_request/release_resources functions to prevent using a gpio as an irq
if the gpiochip_lock_as_irq call failed.
Signed-off-by: Boris Brezillon <[email protected]>
---
drivers/pinctrl/pinctrl-at91.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index f4cd0b9..a481406 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -1477,28 +1477,25 @@ static void gpio_irq_ack(struct irq_data *d)
/* the interrupt is already cleared before by reading ISR */
}
-static unsigned int gpio_irq_startup(struct irq_data *d)
+static int gpio_irq_request_res(struct irq_data *d)
{
struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
unsigned pin = d->hwirq;
int ret;
ret = gpiochip_lock_as_irq(&at91_gpio->chip, pin);
- if (ret) {
+ if (ret)
dev_err(at91_gpio->chip.dev, "unable to lock pind %lu IRQ\n",
d->hwirq);
- return ret;
- }
- gpio_irq_unmask(d);
- return 0;
+
+ return ret;
}
-static void gpio_irq_shutdown(struct irq_data *d)
+static void gpio_irq_release_res(struct irq_data *d)
{
struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
unsigned pin = d->hwirq;
- gpio_irq_mask(d);
gpiochip_unlock_as_irq(&at91_gpio->chip, pin);
}
@@ -1577,8 +1574,8 @@ void at91_pinctrl_gpio_resume(void)
static struct irq_chip gpio_irqchip = {
.name = "GPIO",
.irq_ack = gpio_irq_ack,
- .irq_startup = gpio_irq_startup,
- .irq_shutdown = gpio_irq_shutdown,
+ .irq_request_resources = gpio_irq_request_res,
+ .irq_release_resources = gpio_irq_release_res,
.irq_disable = gpio_irq_mask,
.irq_mask = gpio_irq_mask,
.irq_unmask = gpio_irq_unmask,
--
1.9.1
> On Feb 9, 2015, at 2:23 AM, Boris Brezillon <[email protected]> wrote:
>
> The gpiochip_lock_as_irq call can fail and return an error, while the
> irq_startup is not expected to fail (returns an unsigned int which is not
> checked by irq core code).
>
> irq_request/release_resources functions have been created to address this
> problem.
>
> Move gpiochip_lock/unlock_as_irq calls into
> irq_request/release_resources functions to prevent using a gpio as an irq
> if the gpiochip_lock_as_irq call failed.
>
> Signed-off-by: Boris Brezillon <[email protected]>
> —
I’m travelling will take a look end of the week
Best Regards,
J.
> drivers/pinctrl/pinctrl-at91.c | 17 +++++++----------
> 1 file changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index f4cd0b9..a481406 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -1477,28 +1477,25 @@ static void gpio_irq_ack(struct irq_data *d)
> /* the interrupt is already cleared before by reading ISR */
> }
>
> -static unsigned int gpio_irq_startup(struct irq_data *d)
> +static int gpio_irq_request_res(struct irq_data *d)
> {
> struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
> unsigned pin = d->hwirq;
> int ret;
>
> ret = gpiochip_lock_as_irq(&at91_gpio->chip, pin);
> - if (ret) {
> + if (ret)
> dev_err(at91_gpio->chip.dev, "unable to lock pind %lu IRQ\n",
> d->hwirq);
> - return ret;
> - }
> - gpio_irq_unmask(d);
> - return 0;
> +
> + return ret;
> }
>
> -static void gpio_irq_shutdown(struct irq_data *d)
> +static void gpio_irq_release_res(struct irq_data *d)
> {
> struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
> unsigned pin = d->hwirq;
>
> - gpio_irq_mask(d);
> gpiochip_unlock_as_irq(&at91_gpio->chip, pin);
> }
>
> @@ -1577,8 +1574,8 @@ void at91_pinctrl_gpio_resume(void)
> static struct irq_chip gpio_irqchip = {
> .name = "GPIO",
> .irq_ack = gpio_irq_ack,
> - .irq_startup = gpio_irq_startup,
> - .irq_shutdown = gpio_irq_shutdown,
> + .irq_request_resources = gpio_irq_request_res,
> + .irq_release_resources = gpio_irq_release_res,
> .irq_disable = gpio_irq_mask,
> .irq_mask = gpio_irq_mask,
> .irq_unmask = gpio_irq_unmask,
> --
> 1.9.1
>
On Sun, Feb 08, 2015 at 07:23:42PM +0100, Boris Brezillon wrote:
> The gpiochip_lock_as_irq call can fail and return an error, while the
> irq_startup is not expected to fail (returns an unsigned int which is not
> checked by irq core code).
>
> irq_request/release_resources functions have been created to address this
> problem.
>
> Move gpiochip_lock/unlock_as_irq calls into
> irq_request/release_resources functions to prevent using a gpio as an irq
> if the gpiochip_lock_as_irq call failed.
>
> Signed-off-by: Boris Brezillon <[email protected]>
It sounds good for me, you can add my
Acked-by: Ludovic Desroches <[email protected]>
> ---
> drivers/pinctrl/pinctrl-at91.c | 17 +++++++----------
> 1 file changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index f4cd0b9..a481406 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -1477,28 +1477,25 @@ static void gpio_irq_ack(struct irq_data *d)
> /* the interrupt is already cleared before by reading ISR */
> }
>
> -static unsigned int gpio_irq_startup(struct irq_data *d)
> +static int gpio_irq_request_res(struct irq_data *d)
> {
> struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
> unsigned pin = d->hwirq;
> int ret;
>
> ret = gpiochip_lock_as_irq(&at91_gpio->chip, pin);
> - if (ret) {
> + if (ret)
> dev_err(at91_gpio->chip.dev, "unable to lock pind %lu IRQ\n",
> d->hwirq);
> - return ret;
> - }
> - gpio_irq_unmask(d);
> - return 0;
> +
> + return ret;
> }
>
> -static void gpio_irq_shutdown(struct irq_data *d)
> +static void gpio_irq_release_res(struct irq_data *d)
> {
> struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
> unsigned pin = d->hwirq;
>
> - gpio_irq_mask(d);
> gpiochip_unlock_as_irq(&at91_gpio->chip, pin);
> }
>
> @@ -1577,8 +1574,8 @@ void at91_pinctrl_gpio_resume(void)
> static struct irq_chip gpio_irqchip = {
> .name = "GPIO",
> .irq_ack = gpio_irq_ack,
> - .irq_startup = gpio_irq_startup,
> - .irq_shutdown = gpio_irq_shutdown,
> + .irq_request_resources = gpio_irq_request_res,
> + .irq_release_resources = gpio_irq_release_res,
> .irq_disable = gpio_irq_mask,
> .irq_mask = gpio_irq_mask,
> .irq_unmask = gpio_irq_unmask,
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> On Feb 9, 2015, at 10:50 PM, Jean-Christophe PLAGNIOL-VILLARD <[email protected]> wrote:
>
>
>> On Feb 9, 2015, at 2:23 AM, Boris Brezillon <[email protected]> wrote:
>>
>> The gpiochip_lock_as_irq call can fail and return an error, while the
>> irq_startup is not expected to fail (returns an unsigned int which is not
>> checked by irq core code).
>>
>> irq_request/release_resources functions have been created to address this
>> problem.
>>
>> Move gpiochip_lock/unlock_as_irq calls into
>> irq_request/release_resources functions to prevent using a gpio as an irq
>> if the gpiochip_lock_as_irq call failed.
>>
>> Signed-off-by: Boris Brezillon <[email protected]>
>> —
>
> I’m travelling will take a look end of the week
just get back from travelling will take a look after Chinese new year
as all my hw are at the office
Best Regards,
J.
>
> Best Regards,
> J.
>> drivers/pinctrl/pinctrl-at91.c | 17 +++++++----------
>> 1 file changed, 7 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
>> index f4cd0b9..a481406 100644
>> --- a/drivers/pinctrl/pinctrl-at91.c
>> +++ b/drivers/pinctrl/pinctrl-at91.c
>> @@ -1477,28 +1477,25 @@ static void gpio_irq_ack(struct irq_data *d)
>> /* the interrupt is already cleared before by reading ISR */
>> }
>>
>> -static unsigned int gpio_irq_startup(struct irq_data *d)
>> +static int gpio_irq_request_res(struct irq_data *d)
>> {
>> struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
>> unsigned pin = d->hwirq;
>> int ret;
>>
>> ret = gpiochip_lock_as_irq(&at91_gpio->chip, pin);
>> - if (ret) {
>> + if (ret)
>> dev_err(at91_gpio->chip.dev, "unable to lock pind %lu IRQ\n",
>> d->hwirq);
>> - return ret;
>> - }
>> - gpio_irq_unmask(d);
>> - return 0;
>> +
>> + return ret;
>> }
>>
>> -static void gpio_irq_shutdown(struct irq_data *d)
>> +static void gpio_irq_release_res(struct irq_data *d)
>> {
>> struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
>> unsigned pin = d->hwirq;
>>
>> - gpio_irq_mask(d);
>> gpiochip_unlock_as_irq(&at91_gpio->chip, pin);
>> }
>>
>> @@ -1577,8 +1574,8 @@ void at91_pinctrl_gpio_resume(void)
>> static struct irq_chip gpio_irqchip = {
>> .name = "GPIO",
>> .irq_ack = gpio_irq_ack,
>> - .irq_startup = gpio_irq_startup,
>> - .irq_shutdown = gpio_irq_shutdown,
>> + .irq_request_resources = gpio_irq_request_res,
>> + .irq_release_resources = gpio_irq_release_res,
>> .irq_disable = gpio_irq_mask,
>> .irq_mask = gpio_irq_mask,
>> .irq_unmask = gpio_irq_unmask,
>> --
>> 1.9.1
>>
>
On Fri, 20 Feb 2015 15:50:36 +0800
Jean-Christophe PLAGNIOL-VILLARD <[email protected]> wrote:
>
> > On Feb 9, 2015, at 10:50 PM, Jean-Christophe PLAGNIOL-VILLARD <[email protected]> wrote:
> >
> >
> >> On Feb 9, 2015, at 2:23 AM, Boris Brezillon <[email protected]> wrote:
> >>
> >> The gpiochip_lock_as_irq call can fail and return an error, while the
> >> irq_startup is not expected to fail (returns an unsigned int which is not
> >> checked by irq core code).
> >>
> >> irq_request/release_resources functions have been created to address this
> >> problem.
> >>
> >> Move gpiochip_lock/unlock_as_irq calls into
> >> irq_request/release_resources functions to prevent using a gpio as an irq
> >> if the gpiochip_lock_as_irq call failed.
> >>
> >> Signed-off-by: Boris Brezillon <[email protected]>
> >> —
> >
> > I’m travelling will take a look end of the week
>
> just get back from travelling will take a look after Chinese new year
> as all my hw are at the office
Ping
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Le 08/02/2015 19:23, Boris Brezillon a ?crit :
> The gpiochip_lock_as_irq call can fail and return an error, while the
> irq_startup is not expected to fail (returns an unsigned int which is not
> checked by irq core code).
>
> irq_request/release_resources functions have been created to address this
> problem.
>
> Move gpiochip_lock/unlock_as_irq calls into
> irq_request/release_resources functions to prevent using a gpio as an irq
> if the gpiochip_lock_as_irq call failed.
>
> Signed-off-by: Boris Brezillon <[email protected]>
If it can speed up things:
Acked-by: Nicolas Ferre <[email protected]>
Linus,
can we schedule this fix during the 4.0-rc phase?
Bye,
> ---
> drivers/pinctrl/pinctrl-at91.c | 17 +++++++----------
> 1 file changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index f4cd0b9..a481406 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -1477,28 +1477,25 @@ static void gpio_irq_ack(struct irq_data *d)
> /* the interrupt is already cleared before by reading ISR */
> }
>
> -static unsigned int gpio_irq_startup(struct irq_data *d)
> +static int gpio_irq_request_res(struct irq_data *d)
> {
> struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
> unsigned pin = d->hwirq;
> int ret;
>
> ret = gpiochip_lock_as_irq(&at91_gpio->chip, pin);
> - if (ret) {
> + if (ret)
> dev_err(at91_gpio->chip.dev, "unable to lock pind %lu IRQ\n",
> d->hwirq);
> - return ret;
> - }
> - gpio_irq_unmask(d);
> - return 0;
> +
> + return ret;
> }
>
> -static void gpio_irq_shutdown(struct irq_data *d)
> +static void gpio_irq_release_res(struct irq_data *d)
> {
> struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
> unsigned pin = d->hwirq;
>
> - gpio_irq_mask(d);
> gpiochip_unlock_as_irq(&at91_gpio->chip, pin);
> }
>
> @@ -1577,8 +1574,8 @@ void at91_pinctrl_gpio_resume(void)
> static struct irq_chip gpio_irqchip = {
> .name = "GPIO",
> .irq_ack = gpio_irq_ack,
> - .irq_startup = gpio_irq_startup,
> - .irq_shutdown = gpio_irq_shutdown,
> + .irq_request_resources = gpio_irq_request_res,
> + .irq_release_resources = gpio_irq_release_res,
> .irq_disable = gpio_irq_mask,
> .irq_mask = gpio_irq_mask,
> .irq_unmask = gpio_irq_unmask,
>
--
Nicolas Ferre
> On Feb 26, 2015, at 9:32 PM, Nicolas Ferre <[email protected]> wrote:
>
> Le 08/02/2015 19:23, Boris Brezillon a ?crit :
>> The gpiochip_lock_as_irq call can fail and return an error, while the
>> irq_startup is not expected to fail (returns an unsigned int which is not
>> checked by irq core code).
>>
>> irq_request/release_resources functions have been created to address this
>> problem.
>>
>> Move gpiochip_lock/unlock_as_irq calls into
>> irq_request/release_resources functions to prevent using a gpio as an irq
>> if the gpiochip_lock_as_irq call failed.
>>
>> Signed-off-by: Boris Brezillon <[email protected]>
>
> If it can speed up things:
> Acked-by: Nicolas Ferre <[email protected]>
>
> Linus,
> can we schedule this fix during the 4.0-rc phase?
As I said I?ll test after CNY which finish this week for us
as we only re-open the office Monday
Best Regards,
J.
>
> Bye,
>
>> ---
>> drivers/pinctrl/pinctrl-at91.c | 17 +++++++----------
>> 1 file changed, 7 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
>> index f4cd0b9..a481406 100644
>> --- a/drivers/pinctrl/pinctrl-at91.c
>> +++ b/drivers/pinctrl/pinctrl-at91.c
>> @@ -1477,28 +1477,25 @@ static void gpio_irq_ack(struct irq_data *d)
>> /* the interrupt is already cleared before by reading ISR */
>> }
>>
>> -static unsigned int gpio_irq_startup(struct irq_data *d)
>> +static int gpio_irq_request_res(struct irq_data *d)
>> {
>> struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
>> unsigned pin = d->hwirq;
>> int ret;
>>
>> ret = gpiochip_lock_as_irq(&at91_gpio->chip, pin);
>> - if (ret) {
>> + if (ret)
>> dev_err(at91_gpio->chip.dev, "unable to lock pind %lu IRQ\n",
>> d->hwirq);
>> - return ret;
>> - }
>> - gpio_irq_unmask(d);
>> - return 0;
>> +
>> + return ret;
>> }
>>
>> -static void gpio_irq_shutdown(struct irq_data *d)
>> +static void gpio_irq_release_res(struct irq_data *d)
>> {
>> struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
>> unsigned pin = d->hwirq;
>>
>> - gpio_irq_mask(d);
>> gpiochip_unlock_as_irq(&at91_gpio->chip, pin);
>> }
>>
>> @@ -1577,8 +1574,8 @@ void at91_pinctrl_gpio_resume(void)
>> static struct irq_chip gpio_irqchip = {
>> .name = "GPIO",
>> .irq_ack = gpio_irq_ack,
>> - .irq_startup = gpio_irq_startup,
>> - .irq_shutdown = gpio_irq_shutdown,
>> + .irq_request_resources = gpio_irq_request_res,
>> + .irq_release_resources = gpio_irq_release_res,
>> .irq_disable = gpio_irq_mask,
>> .irq_mask = gpio_irq_mask,
>> .irq_unmask = gpio_irq_unmask,
>>
>
>
> --
> Nicolas Ferre