diff mbox series

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

Message ID 20200409141228.49561-6-andriy.shevchenko@linux.intel.com
State New
Headers show
Series gpio: dwapb: Clean up the driver and a fix | expand

Commit Message

Andy Shevchenko April 9, 2020, 2:12 p.m. UTC
Convert to use irqd_to_hwirq() instead of direct access to the hwirq member.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpio-dwapb.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

Comments

Serge Semin April 15, 2020, 12:42 p.m. UTC | #1
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@linux.intel.com>
> ---
>  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
>
Andy Shevchenko April 15, 2020, 1:16 p.m. UTC | #2
On Wed, Apr 15, 2020 at 03:42:42PM +0300, Serge Semin wrote:
> 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.

> > -	int bit = d->hwirq;

> > +	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.

> > +	u32 bit = irqd_to_hwirq(d);

The idea is that it corresponds the hardware register width. But, i think the
proper one is irq_hw_number_t as return type in all cases.
Let me check this and fix accordingly.

> > +	irq_hw_number_t hwirq;
diff mbox series

Patch

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);
 
 	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,