diff mbox

irq-renesas-irqc: simplify irq_set_type() method

Message ID 201312140309.31407.sergei.shtylyov@cogentembedded.com (mailing list archive)
State Accepted
Commit ce70af18801fd8dac6e9f85308294948916e84d6
Headers show

Commit Message

Sergei Shtylyov Dec. 14, 2013, 12:09 a.m. UTC
Value 0 of the sense  selection field of CONFIG_n register means "disable event
detection" and serves in irqc_sense[] for marking the invalid values of the IRQ
type (by just omitting initializers). There is no need for INTC_IRQ_SENSE_VALID
and hence INTC_IRQ_SENSE() as all field values matching to the  valid IRQ types
are non-zero anyway.

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
The patch is against the 'irq/core' branch of the 'tip.git' repo.

 drivers/irqchip/irq-renesas-irqc.c |   17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Simon Horman Dec. 19, 2013, 8:24 a.m. UTC | #1
On Sat, Dec 14, 2013 at 03:09:31AM +0300, Sergei Shtylyov wrote:
> Value 0 of the sense  selection field of CONFIG_n register means "disable event
> detection" and serves in irqc_sense[] for marking the invalid values of the IRQ
> type (by just omitting initializers). There is no need for INTC_IRQ_SENSE_VALID
> and hence INTC_IRQ_SENSE() as all field values matching to the  valid IRQ types
> are non-zero anyway.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Magnus, could you review this?

> 
> ---
> The patch is against the 'irq/core' branch of the 'tip.git' repo.
> 
>  drivers/irqchip/irq-renesas-irqc.c |   17 +++++++----------
>  1 file changed, 7 insertions(+), 10 deletions(-)
> 
> Index: renesas/drivers/irqchip/irq-renesas-irqc.c
> ===================================================================
> --- renesas.orig/drivers/irqchip/irq-renesas-irqc.c
> +++ renesas/drivers/irqchip/irq-renesas-irqc.c
> @@ -81,15 +81,12 @@ static void irqc_irq_disable(struct irq_
>  	iowrite32(BIT(hw_irq), p->cpu_int_base + IRQC_EN_STS);
>  }
>  
> -#define INTC_IRQ_SENSE_VALID 0x10
> -#define INTC_IRQ_SENSE(x) (x + INTC_IRQ_SENSE_VALID)
> -
>  static unsigned char irqc_sense[IRQ_TYPE_SENSE_MASK + 1] = {
> -	[IRQ_TYPE_LEVEL_LOW] = INTC_IRQ_SENSE(0x01),
> -	[IRQ_TYPE_LEVEL_HIGH] = INTC_IRQ_SENSE(0x02),
> -	[IRQ_TYPE_EDGE_FALLING] = INTC_IRQ_SENSE(0x04), /* Synchronous */
> -	[IRQ_TYPE_EDGE_RISING] = INTC_IRQ_SENSE(0x08), /* Synchronous */
> -	[IRQ_TYPE_EDGE_BOTH] = INTC_IRQ_SENSE(0x0c),  /* Synchronous */
> +	[IRQ_TYPE_LEVEL_LOW]	= 0x01,
> +	[IRQ_TYPE_LEVEL_HIGH]	= 0x02,
> +	[IRQ_TYPE_EDGE_FALLING]	= 0x04,	/* Synchronous */
> +	[IRQ_TYPE_EDGE_RISING]	= 0x08,	/* Synchronous */
> +	[IRQ_TYPE_EDGE_BOTH]	= 0x0c,	/* Synchronous */
>  };
>  
>  static int irqc_irq_set_type(struct irq_data *d, unsigned int type)
> @@ -101,12 +98,12 @@ static int irqc_irq_set_type(struct irq_
>  
>  	irqc_dbg(&p->irq[hw_irq], "sense");
>  
> -	if (!(value & INTC_IRQ_SENSE_VALID))
> +	if (!value)
>  		return -EINVAL;
>  
>  	tmp = ioread32(p->iomem + IRQC_CONFIG(hw_irq));
>  	tmp &= ~0x3f;
> -	tmp |= value ^ INTC_IRQ_SENSE_VALID;
> +	tmp |= value;
>  	iowrite32(tmp, p->iomem + IRQC_CONFIG(hw_irq));
>  	return 0;
>  }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Magnus Damm Dec. 19, 2013, 9:53 a.m. UTC | #2
On Thu, Dec 19, 2013 at 5:24 PM, Simon Horman <horms@verge.net.au> wrote:
> On Sat, Dec 14, 2013 at 03:09:31AM +0300, Sergei Shtylyov wrote:
>> Value 0 of the sense  selection field of CONFIG_n register means "disable event
>> detection" and serves in irqc_sense[] for marking the invalid values of the IRQ
>> type (by just omitting initializers). There is no need for INTC_IRQ_SENSE_VALID
>> and hence INTC_IRQ_SENSE() as all field values matching to the  valid IRQ types
>> are non-zero anyway.
>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> Magnus, could you review this?

The patch is IMO only cosmetic so I can't say I care that much. But
since you ask. =)

Acked-by: Magnus Damm <damm@opensource.se>

Cheers,

/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Simon Horman Dec. 23, 2013, 12:27 a.m. UTC | #3
On Thu, Dec 19, 2013 at 06:53:32PM +0900, Magnus Damm wrote:
> On Thu, Dec 19, 2013 at 5:24 PM, Simon Horman <horms@verge.net.au> wrote:
> > On Sat, Dec 14, 2013 at 03:09:31AM +0300, Sergei Shtylyov wrote:
> >> Value 0 of the sense  selection field of CONFIG_n register means "disable event
> >> detection" and serves in irqc_sense[] for marking the invalid values of the IRQ
> >> type (by just omitting initializers). There is no need for INTC_IRQ_SENSE_VALID
> >> and hence INTC_IRQ_SENSE() as all field values matching to the  valid IRQ types
> >> are non-zero anyway.
> >>
> >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> >
> > Magnus, could you review this?
> 
> The patch is IMO only cosmetic so I can't say I care that much. But
> since you ask. =)
> 
> Acked-by: Magnus Damm <damm@opensource.se>

Thanks, I have queued this up.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

Index: renesas/drivers/irqchip/irq-renesas-irqc.c
===================================================================
--- renesas.orig/drivers/irqchip/irq-renesas-irqc.c
+++ renesas/drivers/irqchip/irq-renesas-irqc.c
@@ -81,15 +81,12 @@  static void irqc_irq_disable(struct irq_
 	iowrite32(BIT(hw_irq), p->cpu_int_base + IRQC_EN_STS);
 }
 
-#define INTC_IRQ_SENSE_VALID 0x10
-#define INTC_IRQ_SENSE(x) (x + INTC_IRQ_SENSE_VALID)
-
 static unsigned char irqc_sense[IRQ_TYPE_SENSE_MASK + 1] = {
-	[IRQ_TYPE_LEVEL_LOW] = INTC_IRQ_SENSE(0x01),
-	[IRQ_TYPE_LEVEL_HIGH] = INTC_IRQ_SENSE(0x02),
-	[IRQ_TYPE_EDGE_FALLING] = INTC_IRQ_SENSE(0x04), /* Synchronous */
-	[IRQ_TYPE_EDGE_RISING] = INTC_IRQ_SENSE(0x08), /* Synchronous */
-	[IRQ_TYPE_EDGE_BOTH] = INTC_IRQ_SENSE(0x0c),  /* Synchronous */
+	[IRQ_TYPE_LEVEL_LOW]	= 0x01,
+	[IRQ_TYPE_LEVEL_HIGH]	= 0x02,
+	[IRQ_TYPE_EDGE_FALLING]	= 0x04,	/* Synchronous */
+	[IRQ_TYPE_EDGE_RISING]	= 0x08,	/* Synchronous */
+	[IRQ_TYPE_EDGE_BOTH]	= 0x0c,	/* Synchronous */
 };
 
 static int irqc_irq_set_type(struct irq_data *d, unsigned int type)
@@ -101,12 +98,12 @@  static int irqc_irq_set_type(struct irq_
 
 	irqc_dbg(&p->irq[hw_irq], "sense");
 
-	if (!(value & INTC_IRQ_SENSE_VALID))
+	if (!value)
 		return -EINVAL;
 
 	tmp = ioread32(p->iomem + IRQC_CONFIG(hw_irq));
 	tmp &= ~0x3f;
-	tmp |= value ^ INTC_IRQ_SENSE_VALID;
+	tmp |= value;
 	iowrite32(tmp, p->iomem + IRQC_CONFIG(hw_irq));
 	return 0;
 }