Re: [PATCH v1 05/13] gpio: dwapb: Convert to use irqd_to_hwirq()

[Serge Semin <fancer.lancer@xxxxxxxxx>]

On Thu, Apr 09, 2020 at 05:12:20PM +0300, Andy Shevchenko wrote:
> Convert to use irqd_to_hwirq() instead of direct access to the hwirq member.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
>  drivers/gpio/gpio-dwapb.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
> index c0c267cddd80..3a1f3fae923f 100644
> --- a/drivers/gpio/gpio-dwapb.c
> +++ b/drivers/gpio/gpio-dwapb.c
> @@ -230,7 +230,7 @@ static void dwapb_irq_enable(struct irq_data *d)
>  
>  	spin_lock_irqsave(&gc->bgpio_lock, flags);
>  	val = dwapb_read(gpio, GPIO_INTEN);
> -	val |= BIT(d->hwirq);
> +	val |= BIT(irqd_to_hwirq(d));
>  	dwapb_write(gpio, GPIO_INTEN, val);
>  	spin_unlock_irqrestore(&gc->bgpio_lock, flags);
>  }
> @@ -245,7 +245,7 @@ static void dwapb_irq_disable(struct irq_data *d)
>  
>  	spin_lock_irqsave(&gc->bgpio_lock, flags);
>  	val = dwapb_read(gpio, GPIO_INTEN);
> -	val &= ~BIT(d->hwirq);
> +	val &= ~BIT(irqd_to_hwirq(d));
>  	dwapb_write(gpio, GPIO_INTEN, val);
>  	spin_unlock_irqrestore(&gc->bgpio_lock, flags);
>  }
> @@ -255,8 +255,8 @@ static int dwapb_irq_set_type(struct irq_data *d, u32 type)
>  	struct irq_chip_generic *igc = irq_data_get_irq_chip_data(d);
>  	struct dwapb_gpio *gpio = igc->private;
>  	struct gpio_chip *gc = &gpio->ports[0].gc;
> -	int bit = d->hwirq;
>  	unsigned long level, polarity, flags;
> +	u32 bit = irqd_to_hwirq(d);

I'm not saying that the rest of the driver code is highly coherent with
ideal design and style. But here I don't really see a point in converting
the type to u32. As I see it int-like type is more appropriate since
we don't need to signify the data type width in this context.

-Sergey

>  
>  	if (type & ~(IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING |
>  		     IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW))
> @@ -305,11 +305,12 @@ static int dwapb_irq_set_wake(struct irq_data *d, unsigned int enable)
>  	struct irq_chip_generic *igc = irq_data_get_irq_chip_data(d);
>  	struct dwapb_gpio *gpio = igc->private;
>  	struct dwapb_context *ctx = gpio->ports[0].ctx;
> +	u32 bit = irqd_to_hwirq(d);
>  
>  	if (enable)
> -		ctx->wake_en |= BIT(d->hwirq);
> +		ctx->wake_en |= BIT(bit);
>  	else
> -		ctx->wake_en &= ~BIT(d->hwirq);
> +		ctx->wake_en &= ~BIT(bit);
>  
>  	return 0;
>  }
> @@ -365,8 +366,9 @@ static void dwapb_configure_irqs(struct dwapb_gpio *gpio,
>  	struct gpio_chip *gc = &port->gc;
>  	struct fwnode_handle  *fwnode = pp->fwnode;
>  	struct irq_chip_generic	*irq_gc = NULL;
> -	unsigned int hwirq, ngpio = gc->ngpio;
> +	unsigned int ngpio = gc->ngpio;
>  	struct irq_chip_type *ct;
> +	irq_hw_number_t hwirq;
>  	int err, i;
>  
>  	gpio->domain = irq_domain_create_linear(fwnode, ngpio,
> -- 
> 2.25.1
>