Re: [PATCH] gpio: mpfs: Make the irqchip immutable

[Conor Dooley <conor@xxxxxxxxxx>]

Hey Geert,

On Tue, Nov 29, 2022 at 05:23:22PM +0100, Geert Uytterhoeven wrote:
> Commit 6c846d026d49 ("gpio: Don't fiddle with irqchips marked as
> immutable") added a warning to indicate if the gpiolib is altering the
> internals of irqchips.  Following this change the following warning is
> now observed for the gpio-mpfs driver:
> 
>     gpio gpiochip0: (20122000.gpio): not an immutable chip, please consider fixing it!
> 
> Fix this by making the irqchip in the gpio-mpfs driver immutable.
> 
> While at it, drop of the unneeded masking of the hwirq number, as it is
> always smaller than the number of GPIOs/interrupts handled by the
> controller.

Huh, I didn't think this was upstream yet?
Lewis should be working on an updated version of it at the moment, last
I heard he was looking into using regmap stuff as pointed out in a
review.

> Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> ---
> Not tested with interrupts, as there are no inputs described in the
> Icicle DTS yet.
> ---
>  drivers/gpio/gpio-mpfs.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-mpfs.c b/drivers/gpio/gpio-mpfs.c
> index 168be0b90cf8c656..fd294b581ae77369 100644
> --- a/drivers/gpio/gpio-mpfs.c
> +++ b/drivers/gpio/gpio-mpfs.c
> @@ -168,8 +168,9 @@ static void mpfs_gpio_irq_unmask(struct irq_data *data)
>  {
>  	struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
>  	struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
> -	int gpio_index = irqd_to_hwirq(data) % MAX_NUM_GPIO;
> +	int gpio_index = irqd_to_hwirq(data);
>  
> +	gpiochip_enable_irq(gc, gpio_index);
>  	mpfs_gpio_direction_input(gc, gpio_index);
>  	mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_IRQ_REG, gpio_index, 1);
>  	mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_GPIO_CTRL(gpio_index),
> @@ -180,11 +181,12 @@ static void mpfs_gpio_irq_mask(struct irq_data *data)
>  {
>  	struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
>  	struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
> -	int gpio_index = irqd_to_hwirq(data) % MAX_NUM_GPIO;
> +	int gpio_index = irqd_to_hwirq(data);
>  
>  	mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_IRQ_REG, gpio_index, 1);
>  	mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_GPIO_CTRL(gpio_index),
>  			     MPFS_GPIO_EN_INT, 0);
> +	gpiochip_disable_irq(gc, gpio_index);
>  }
>  
>  static const struct irq_chip mpfs_gpio_irqchip = {
> @@ -192,7 +194,8 @@ static const struct irq_chip mpfs_gpio_irqchip = {
>  	.irq_set_type = mpfs_gpio_irq_set_type,
>  	.irq_mask	= mpfs_gpio_irq_mask,
>  	.irq_unmask	= mpfs_gpio_irq_unmask,
> -	.flags = IRQCHIP_MASK_ON_SUSPEND,
> +	.flags = IRQCHIP_IMMUTABLE | IRQCHIP_MASK_ON_SUSPEND,
> +	GPIOCHIP_IRQ_RESOURCE_HELPERS,
>  };
>  
>  static void mpfs_gpio_irq_handler(struct irq_desc *desc)
> -- 
> 2.25.1
>