Re: [PATCH v2 08/14] gpio: dwapb: Convert to use IRQ core provided macros

From: Serge Semin <fancer.lancer@gmail.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	linux-gpio@vger.kernel.org
Subject: Re: [PATCH v2 08/14] gpio: dwapb: Convert to use IRQ core provided macros
Date: Wed, 15 Apr 2020 20:53:09 +0300	[thread overview]
Message-ID: <20200415175309.6l6skv7qsdjqsryi@mobilestation> (raw)
In-Reply-To: <20200415141534.31240-9-andriy.shevchenko@linux.intel.com>

On Wed, Apr 15, 2020 at 05:15:28PM +0300, Andy Shevchenko wrote:
> IRQ core provides macros such as IRQ_RETVAL().
> Convert code to use them.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Tested-by: Serge Semin <fancer.lancer@gmail.com>
> Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
> ---
>  drivers/gpio/gpio-dwapb.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
> index 8b30ded9322a..4edac592c253 100644
> --- a/drivers/gpio/gpio-dwapb.c
> +++ b/drivers/gpio/gpio-dwapb.c
> @@ -258,8 +258,7 @@ static int dwapb_irq_set_type(struct irq_data *d, u32 type)
>  	irq_hw_number_t bit = irqd_to_hwirq(d);
>  	unsigned long level, polarity, flags;
>  
> -	if (type & ~(IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING |
> -		     IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW))
> +	if (type & ~IRQ_TYPE_SENSE_MASK)
>  		return -EINVAL;
>  
>  	spin_lock_irqsave(&gc->bgpio_lock, flags);
> @@ -351,12 +350,7 @@ static int dwapb_gpio_set_config(struct gpio_chip *gc, unsigned offset,
>  
>  static irqreturn_t dwapb_irq_handler_mfd(int irq, void *dev_id)
>  {
> -	u32 worked;
> -	struct dwapb_gpio *gpio = dev_id;
> -
> -	worked = dwapb_do_irq(gpio);
> -
> -	return worked ? IRQ_HANDLED : IRQ_NONE;
> +	return IRQ_RETVAL(dwapb_do_irq(dev_id));
>  }

BTW Forgot to mention. Irrelevantly to this patch just so you know seeing
you are from Intel and this part is being utilized by the Intel Quark SoC.
dwapb_irq_handler_mfd() handler will cause a problem in RT-patched kernel
(I've seen such issue in another GPIO-driver). So if PREEMP_RT_FULL patch
is applied and the FULL-RT scheduler is enabled all interrupt handlers
specified by request_irq()-based methods will be handled by a kernel thread,
while generic_handle_irq() is supposed to be called from the atomic context
only (with interrupts disabled). As a result an ugly stack dump will be printed
to the kernel log by the next code:
https://elixir.bootlin.com/linux/latest/source/kernel/irq/handle.c#L152

A way to fix this is described in Documentation/driver-api/gpio/driver.rst

Regards,
-Sergey

>  
>  static void dwapb_configure_irqs(struct dwapb_gpio *gpio,
> -- 
> 2.25.1
> 