plat-orion multi purpose pins problem for mv78200

plat-orion multi purpose pins problem for mv78200

[Joey Oravec]

Nicola, Lennert -

There's a problem in 3.0-rc5 when calling mv78xx0_mpp_conf() on the 
MV78200 processor. This processor has 49 multi purpose pins but only 31 
GPIOs. MPP[31:0] map directly to GPIO[31:0] but the next 17 are a little 
more complex:

MPP[39:32] = GPIO[7:0]
MPP[46:40] = GPIO[23:17]
MPP[47] = GPIO[16]
MPP[48] = GPIO[8]
MPP[49] = GPIO[9]

See arch/arm/plat-orion/mpp.c. Inside orion_mpp_conf() the array gets 
processed by calling MPP_NUM() then calling orion_gpio_set_valid(). As 
demonstrated above, the MPP number and GPIO number are not 
interchangeable and on the MV78200 that's a problem when trying to setup 
any MPP > 31.

So we need to map each MPP to a given GPIO -- I don't know how the 
mapping will differ across processors. We also need to keep-clear which 
functions are called with an MPP number and which functions are called 
with a GPIO number.

Followup question -- Marvell has several SoC families, but Linux uess 
plat-orion for everything. For example MV78200 isn't an Orion it's a 
Discovery Innovation series. Would it make more sense to have separate 
plat-* code for the separate families?

-joey

plat-orion multi purpose pins problem for mv78200

[Joey Oravec]

It looks like there's a second problem here. Imagine you're calling 
mv78xx0_mpp_conf() and the array has these entries:

MPP16_GPIO
MPP47_UNUSED

These are separate MPP pins that can each bring out the same GPIO 
resource. Notice that as the code stands today, the second call is going 
to clobber the values valid_input and valid_output for GPIO[16]. The 
logic for orion_gpio_is_valid() needs to check if a given GPIO is 
brought-out on any MPP pin.

In the short term, it might be worth disabling the check in 
orion_gpio_is_valid() to avoid the bug.

-joey

On 7/1/2011 3:14 PM, Joey Oravec wrote:
> Nicola, Lennert -
>
> There's a problem in 3.0-rc5 when calling mv78xx0_mpp_conf() on the 
> MV78200 processor. This processor has 49 multi purpose pins but only 
> 31 GPIOs. MPP[31:0] map directly to GPIO[31:0] but the next 17 are a 
> little more complex:
>
> MPP[39:32] = GPIO[7:0]
> MPP[46:40] = GPIO[23:17]
> MPP[47] = GPIO[16]
> MPP[48] = GPIO[8]
> MPP[49] = GPIO[9]
>
> See arch/arm/plat-orion/mpp.c. Inside orion_mpp_conf() the array gets 
> processed by calling MPP_NUM() then calling orion_gpio_set_valid(). As 
> demonstrated above, the MPP number and GPIO number are not 
> interchangeable and on the MV78200 that's a problem when trying to 
> setup any MPP > 31.
>
> So we need to map each MPP to a given GPIO -- I don't know how the 
> mapping will differ across processors. We also need to keep-clear 
> which functions are called with an MPP number and which functions are 
> called with a GPIO number.
>
> Followup question -- Marvell has several SoC families, but Linux uess 
> plat-orion for everything. For example MV78200 isn't an Orion it's a 
> Discovery Innovation series. Would it make more sense to have separate 
> plat-* code for the separate families?
>
> -joey
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

plat-orion multi purpose pins problem for mv78200

[Simon Guinot]

Hi Joey,

On Fri, Jul 01, 2011 at 03:14:48PM -0400, Joey Oravec wrote:
> Nicola, Lennert -
> 
> There's a problem in 3.0-rc5 when calling mv78xx0_mpp_conf() on the
> MV78200 processor. This processor has 49 multi purpose pins but only
> 31 GPIOs. MPP[31:0] map directly to GPIO[31:0] but the next 17 are a
> little more complex:
> 
> MPP[39:32] = GPIO[7:0]
> MPP[46:40] = GPIO[23:17]
> MPP[47] = GPIO[16]
> MPP[48] = GPIO[8]
> MPP[49] = GPIO[9]

It doesn't seems the code was behaving differently before 3.0-rc. I mean
before the MPP and GPIO consolidation work. It was ?

> 
> See arch/arm/plat-orion/mpp.c. Inside orion_mpp_conf() the array
> gets processed by calling MPP_NUM() then calling
> orion_gpio_set_valid(). As demonstrated above, the MPP number and
> GPIO number are not interchangeable and on the MV78200 that's a
> problem when trying to setup any MPP > 31.
> 
> So we need to map each MPP to a given GPIO -- I don't know how the
> mapping will differ across processors. We also need to keep-clear
> which functions are called with an MPP number and which functions
> are called with a GPIO number.

As I don't have access to either a MV78200 device or his specification
document, let me ask you a few questions.

Does hardware allows a configuration like MPP0_GE0_COL and MPP32_GPIO
simultaneously ?

If not (and only if not), the problem is simply mapping a MPP number on
the correct GPIO number. Isn't it ?

In that case, what about using the MPPxx_GPIO definitions (found in
mach-mv78xx0/mpp.h) to map the right GPIO number ?

For example:

#define MPP32_GPIO        MPP(32, 0x1, 1, 1, 1)

could be replaced by:

#define MPP32_GPIO        MPP(0, 0x1, 1, 1, 1)

> 
> Followup question -- Marvell has several SoC families, but Linux
> uess plat-orion for everything. For example MV78200 isn't an Orion
> it's a Discovery Innovation series. Would it make more sense to have
> separate plat-* code for the separate families?

As far as possible, we have to share code between the SoCs families.

Regards,

Simon
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110702/c00680b4/attachment.sig>

plat-orion multi purpose pins problem for mv78200

[saeed bishara]

On Sat, Jul 2, 2011 at 3:23 PM, Simon Guinot <simon@sequanux.org> wrote:
> Hi Joey,
>
> On Fri, Jul 01, 2011 at 03:14:48PM -0400, Joey Oravec wrote:
>> Nicola, Lennert -
>>
>> There's a problem in 3.0-rc5 when calling mv78xx0_mpp_conf() on the
>> MV78200 processor. This processor has 49 multi purpose pins but only
>> 31 GPIOs. MPP[31:0] map directly to GPIO[31:0] but the next 17 are a
>> little more complex:
>>
>> MPP[39:32] = GPIO[7:0]
>> MPP[46:40] = GPIO[23:17]
>> MPP[47] = GPIO[16]
>> MPP[48] = GPIO[8]
>> MPP[49] = GPIO[9]
>
> It doesn't seems the code was behaving differently before 3.0-rc. I mean
> before the MPP and GPIO consolidation work. It was ?
that's right, this issue was before the consolidation work.
>
>>
>> See arch/arm/plat-orion/mpp.c. Inside orion_mpp_conf() the array
>> gets processed by calling MPP_NUM() then calling
>> orion_gpio_set_valid(). As demonstrated above, the MPP number and
>> GPIO number are not interchangeable and on the MV78200 that's a
>> problem when trying to setup any MPP > 31.
>>
>> So we need to map each MPP to a given GPIO -- I don't know how the
>> mapping will differ across processors. We also need to keep-clear
>> which functions are called with an MPP number and which functions
>> are called with a GPIO number.
>
> As I don't have access to either a MV78200 device or his specification
> document, let me ask you a few questions.
>
> Does hardware allows a configuration like MPP0_GE0_COL and MPP32_GPIO
> simultaneously ?
yes, this is possible.
>
> If not (and only if not), the problem is simply mapping a MPP number on
> the correct GPIO number. Isn't it ?
>
> In that case, what about using the MPPxx_GPIO definitions (found in
> mach-mv78xx0/mpp.h) to map the right GPIO number ?
>
> For example:
>
> #define MPP32_GPIO ? ? ? ?MPP(32, 0x1, 1, 1, 1)
>
> could be replaced by:
>
> #define MPP32_GPIO ? ? ? ?MPP(0, 0x1, 1, 1, 1)
that won't work because the num (first number) is used as the mpp
index. see orion_mpp_conf() implementation.
I think the solution is to extend the MPP define to include the
explicit gpio number, so it will look like:
#define MPP(_num, _gpio _sel, _in, _out, _78100_A0)

saeed

[PATCH v2] genirq: replace irq_gc_ack() with {set,clr}_bit variants

[Simon Guinot]

From: Simon Guinot <sguinot@lacie.com>

Depending on the device, interrupts acknowledgement is done by setting
or by clearing a dedicated register. Replace irq_gc_ack() with some
{set,clr}_bit variants allows to handle both cases.

Note that this patch affects the following SoCs: Davinci, Samsung and
Orion. Except for this last, the change is minor: irq_gc_ack() is just
renamed into irq_gc_ack_set_bit().

For the Orion SoCs, the edge GPIO interrupts support is currently
broken. irq_gc_ack() try to acknowledge a such interrupt by setting
the corresponding cause register bit. The Orion GPIO device expect the
opposite. To fix this issue, the irq_gc_ack_clr_bit() variant is used.

Tested on Network Space v2.

Reported-by: Joey Oravec <joravec@drewtech.com>
Signed-off-by: Simon Guinot <sguinot@lacie.com>
---
Changes for v2: update patch description (mention the affected SoCs).

 arch/arm/mach-davinci/irq.c      |    2 +-
 arch/arm/plat-orion/gpio.c       |    2 +-
 arch/arm/plat-s5p/irq-gpioint.c  |    2 +-
 arch/arm/plat-samsung/irq-uart.c |    2 +-
 include/linux/irq.h              |    3 ++-
 kernel/irq/generic-chip.c        |   18 ++++++++++++++++--
 6 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-davinci/irq.c b/arch/arm/mach-davinci/irq.c
index bfe68ec..d8c1af0 100644
--- a/arch/arm/mach-davinci/irq.c
+++ b/arch/arm/mach-davinci/irq.c
@@ -53,7 +53,7 @@ davinci_alloc_gc(void __iomem *base, unsigned int irq_start, unsigned int num)
 
 	gc = irq_alloc_generic_chip("AINTC", 1, irq_start, base, handle_edge_irq);
 	ct = gc->chip_types;
-	ct->chip.irq_ack = irq_gc_ack;
+	ct->chip.irq_ack = irq_gc_ack_set_bit;
 	ct->chip.irq_mask = irq_gc_mask_clr_bit;
 	ct->chip.irq_unmask = irq_gc_mask_set_bit;
 
diff --git a/arch/arm/plat-orion/gpio.c b/arch/arm/plat-orion/gpio.c
index 5b4fffa..41ab97e 100644
--- a/arch/arm/plat-orion/gpio.c
+++ b/arch/arm/plat-orion/gpio.c
@@ -432,7 +432,7 @@ void __init orion_gpio_init(int gpio_base, int ngpio,
 	ct->regs.mask = ochip->mask_offset + GPIO_EDGE_MASK_OFF;
 	ct->regs.ack = GPIO_EDGE_CAUSE_OFF;
 	ct->type = IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING;
-	ct->chip.irq_ack = irq_gc_ack;
+	ct->chip.irq_ack = irq_gc_ack_clr_bit;
 	ct->chip.irq_mask = irq_gc_mask_clr_bit;
 	ct->chip.irq_unmask = irq_gc_mask_set_bit;
 	ct->chip.irq_set_type = gpio_irq_set_type;
diff --git a/arch/arm/plat-s5p/irq-gpioint.c b/arch/arm/plat-s5p/irq-gpioint.c
index 135abda..327ab9f 100644
--- a/arch/arm/plat-s5p/irq-gpioint.c
+++ b/arch/arm/plat-s5p/irq-gpioint.c
@@ -152,7 +152,7 @@ static __init int s5p_gpioint_add(struct s3c_gpio_chip *chip)
 	if (!gc)
 		return -ENOMEM;
 	ct = gc->chip_types;
-	ct->chip.irq_ack = irq_gc_ack;
+	ct->chip.irq_ack = irq_gc_ack_set_bit;
 	ct->chip.irq_mask = irq_gc_mask_set_bit;
 	ct->chip.irq_unmask = irq_gc_mask_clr_bit;
 	ct->chip.irq_set_type = s5p_gpioint_set_type,
diff --git a/arch/arm/plat-samsung/irq-uart.c b/arch/arm/plat-samsung/irq-uart.c
index 32582c0..0e46588 100644
--- a/arch/arm/plat-samsung/irq-uart.c
+++ b/arch/arm/plat-samsung/irq-uart.c
@@ -55,7 +55,7 @@ static void __init s3c_init_uart_irq(struct s3c_uart_irq *uirq)
 	gc = irq_alloc_generic_chip("s3c-uart", 1, uirq->base_irq, reg_base,
 				    handle_level_irq);
 	ct = gc->chip_types;
-	ct->chip.irq_ack = irq_gc_ack;
+	ct->chip.irq_ack = irq_gc_ack_set_bit;
 	ct->chip.irq_mask = irq_gc_mask_set_bit;
 	ct->chip.irq_unmask = irq_gc_mask_clr_bit;
 	ct->regs.ack = S3C64XX_UINTP;
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 8b45384..baa397e 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -676,7 +676,8 @@ void irq_gc_mask_disable_reg(struct irq_data *d);
 void irq_gc_mask_set_bit(struct irq_data *d);
 void irq_gc_mask_clr_bit(struct irq_data *d);
 void irq_gc_unmask_enable_reg(struct irq_data *d);
-void irq_gc_ack(struct irq_data *d);
+void irq_gc_ack_set_bit(struct irq_data *d);
+void irq_gc_ack_clr_bit(struct irq_data *d);
 void irq_gc_mask_disable_reg_and_ack(struct irq_data *d);
 void irq_gc_eoi(struct irq_data *d);
 int irq_gc_set_wake(struct irq_data *d, unsigned int on);
diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index 31a9db7..3a2cab4 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -101,10 +101,10 @@ void irq_gc_unmask_enable_reg(struct irq_data *d)
 }
 
 /**
- * irq_gc_ack - Ack pending interrupt
+ * irq_gc_ack_set_bit - Ack pending interrupt via setting bit
  * @d: irq_data
  */
-void irq_gc_ack(struct irq_data *d)
+void irq_gc_ack_set_bit(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
 	u32 mask = 1 << (d->irq - gc->irq_base);
@@ -115,6 +115,20 @@ void irq_gc_ack(struct irq_data *d)
 }
 
 /**
+ * irq_gc_ack_clr_bit - Ack pending interrupt via clearing bit
+ * @d: irq_data
+ */
+void irq_gc_ack_clr_bit(struct irq_data *d)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	u32 mask = ~(1 << (d->irq - gc->irq_base));
+
+	irq_gc_lock(gc);
+	irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack);
+	irq_gc_unlock(gc);
+}
+
+/**
  * irq_gc_mask_disable_reg_and_ack- Mask and ack pending interrupt
  * @d: irq_data
  */
-- 
1.7.5.1

[PATCH v2] genirq: replace irq_gc_ack() with {set, clr}_bit variants

[Nicolas Pitre]

On Mon, 4 Jul 2011, Simon Guinot wrote:

> From: Simon Guinot <sguinot@lacie.com>
> 
> Depending on the device, interrupts acknowledgement is done by setting
> or by clearing a dedicated register. Replace irq_gc_ack() with some
> {set,clr}_bit variants allows to handle both cases.
> 
> Note that this patch affects the following SoCs: Davinci, Samsung and
> Orion. Except for this last, the change is minor: irq_gc_ack() is just
> renamed into irq_gc_ack_set_bit().
> 
> For the Orion SoCs, the edge GPIO interrupts support is currently
> broken. irq_gc_ack() try to acknowledge a such interrupt by setting
> the corresponding cause register bit. The Orion GPIO device expect the
> opposite. To fix this issue, the irq_gc_ack_clr_bit() variant is used.
> 
> Tested on Network Space v2.
> 
> Reported-by: Joey Oravec <joravec@drewtech.com>
> Signed-off-by: Simon Guinot <sguinot@lacie.com>

Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>




> ---
> Changes for v2: update patch description (mention the affected SoCs).
> 
>  arch/arm/mach-davinci/irq.c      |    2 +-
>  arch/arm/plat-orion/gpio.c       |    2 +-
>  arch/arm/plat-s5p/irq-gpioint.c  |    2 +-
>  arch/arm/plat-samsung/irq-uart.c |    2 +-
>  include/linux/irq.h              |    3 ++-
>  kernel/irq/generic-chip.c        |   18 ++++++++++++++++--
>  6 files changed, 22 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/mach-davinci/irq.c b/arch/arm/mach-davinci/irq.c
> index bfe68ec..d8c1af0 100644
> --- a/arch/arm/mach-davinci/irq.c
> +++ b/arch/arm/mach-davinci/irq.c
> @@ -53,7 +53,7 @@ davinci_alloc_gc(void __iomem *base, unsigned int irq_start, unsigned int num)
>  
>  	gc = irq_alloc_generic_chip("AINTC", 1, irq_start, base, handle_edge_irq);
>  	ct = gc->chip_types;
> -	ct->chip.irq_ack = irq_gc_ack;
> +	ct->chip.irq_ack = irq_gc_ack_set_bit;
>  	ct->chip.irq_mask = irq_gc_mask_clr_bit;
>  	ct->chip.irq_unmask = irq_gc_mask_set_bit;
>  
> diff --git a/arch/arm/plat-orion/gpio.c b/arch/arm/plat-orion/gpio.c
> index 5b4fffa..41ab97e 100644
> --- a/arch/arm/plat-orion/gpio.c
> +++ b/arch/arm/plat-orion/gpio.c
> @@ -432,7 +432,7 @@ void __init orion_gpio_init(int gpio_base, int ngpio,
>  	ct->regs.mask = ochip->mask_offset + GPIO_EDGE_MASK_OFF;
>  	ct->regs.ack = GPIO_EDGE_CAUSE_OFF;
>  	ct->type = IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING;
> -	ct->chip.irq_ack = irq_gc_ack;
> +	ct->chip.irq_ack = irq_gc_ack_clr_bit;
>  	ct->chip.irq_mask = irq_gc_mask_clr_bit;
>  	ct->chip.irq_unmask = irq_gc_mask_set_bit;
>  	ct->chip.irq_set_type = gpio_irq_set_type;
> diff --git a/arch/arm/plat-s5p/irq-gpioint.c b/arch/arm/plat-s5p/irq-gpioint.c
> index 135abda..327ab9f 100644
> --- a/arch/arm/plat-s5p/irq-gpioint.c
> +++ b/arch/arm/plat-s5p/irq-gpioint.c
> @@ -152,7 +152,7 @@ static __init int s5p_gpioint_add(struct s3c_gpio_chip *chip)
>  	if (!gc)
>  		return -ENOMEM;
>  	ct = gc->chip_types;
> -	ct->chip.irq_ack = irq_gc_ack;
> +	ct->chip.irq_ack = irq_gc_ack_set_bit;
>  	ct->chip.irq_mask = irq_gc_mask_set_bit;
>  	ct->chip.irq_unmask = irq_gc_mask_clr_bit;
>  	ct->chip.irq_set_type = s5p_gpioint_set_type,
> diff --git a/arch/arm/plat-samsung/irq-uart.c b/arch/arm/plat-samsung/irq-uart.c
> index 32582c0..0e46588 100644
> --- a/arch/arm/plat-samsung/irq-uart.c
> +++ b/arch/arm/plat-samsung/irq-uart.c
> @@ -55,7 +55,7 @@ static void __init s3c_init_uart_irq(struct s3c_uart_irq *uirq)
>  	gc = irq_alloc_generic_chip("s3c-uart", 1, uirq->base_irq, reg_base,
>  				    handle_level_irq);
>  	ct = gc->chip_types;
> -	ct->chip.irq_ack = irq_gc_ack;
> +	ct->chip.irq_ack = irq_gc_ack_set_bit;
>  	ct->chip.irq_mask = irq_gc_mask_set_bit;
>  	ct->chip.irq_unmask = irq_gc_mask_clr_bit;
>  	ct->regs.ack = S3C64XX_UINTP;
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index 8b45384..baa397e 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -676,7 +676,8 @@ void irq_gc_mask_disable_reg(struct irq_data *d);
>  void irq_gc_mask_set_bit(struct irq_data *d);
>  void irq_gc_mask_clr_bit(struct irq_data *d);
>  void irq_gc_unmask_enable_reg(struct irq_data *d);
> -void irq_gc_ack(struct irq_data *d);
> +void irq_gc_ack_set_bit(struct irq_data *d);
> +void irq_gc_ack_clr_bit(struct irq_data *d);
>  void irq_gc_mask_disable_reg_and_ack(struct irq_data *d);
>  void irq_gc_eoi(struct irq_data *d);
>  int irq_gc_set_wake(struct irq_data *d, unsigned int on);
> diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
> index 31a9db7..3a2cab4 100644
> --- a/kernel/irq/generic-chip.c
> +++ b/kernel/irq/generic-chip.c
> @@ -101,10 +101,10 @@ void irq_gc_unmask_enable_reg(struct irq_data *d)
>  }
>  
>  /**
> - * irq_gc_ack - Ack pending interrupt
> + * irq_gc_ack_set_bit - Ack pending interrupt via setting bit
>   * @d: irq_data
>   */
> -void irq_gc_ack(struct irq_data *d)
> +void irq_gc_ack_set_bit(struct irq_data *d)
>  {
>  	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
>  	u32 mask = 1 << (d->irq - gc->irq_base);
> @@ -115,6 +115,20 @@ void irq_gc_ack(struct irq_data *d)
>  }
>  
>  /**
> + * irq_gc_ack_clr_bit - Ack pending interrupt via clearing bit
> + * @d: irq_data
> + */
> +void irq_gc_ack_clr_bit(struct irq_data *d)
> +{
> +	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> +	u32 mask = ~(1 << (d->irq - gc->irq_base));
> +
> +	irq_gc_lock(gc);
> +	irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack);
> +	irq_gc_unlock(gc);
> +}
> +
> +/**
>   * irq_gc_mask_disable_reg_and_ack- Mask and ack pending interrupt
>   * @d: irq_data
>   */
> -- 
> 1.7.5.1
> 

plat-orion multi purpose pins problem for mv78200

[Joey Oravec]

On 7/3/2011 8:46 AM, saeed bishara wrote:
>> It doesn't seems the code was behaving differently before 3.0-rc. I mean
>> before the MPP and GPIO consolidation work. It was ?
> that's right, this issue was before the consolidation work.
Agreed. I only meant to say that the MPP code is broken for mv78200 in 
3.0-rc5, so the maintainers should consider a short-term fix of 
disabling the checks in orion_gpio_is_valid().

>> Does hardware allows a configuration like MPP0_GE0_COL and MPP32_GPIO
>> simultaneously ?
> yes, this is possible.
Agreed. MPP is the pin that can be mux'ed to 7 resources. GPIO is just 
one of those resources. Yes the hardware supports your example. Most 
resources can be routed to several different pins to allow flexibility 
in board design.

Since MPP is just a pin-mux, it's possible for Marvell to build an SoC 
in the future that can route a few different GPIOs onto a given MPP pin. 
It seems like any solution should account for this.

>> If not (and only if not), the problem is simply mapping a MPP number on
>> the correct GPIO number. Isn't it ?
>>
>> In that case, what about using the MPPxx_GPIO definitions (found in
>> mach-mv78xx0/mpp.h) to map the right GPIO number ?
>>
>> For example:
>>
>> #define MPP32_GPIO        MPP(32, 0x1, 1, 1, 1)
>>
>> could be replaced by:
>>
>> #define MPP32_GPIO        MPP(0, 0x1, 1, 1, 1)
> that won't work because the num (first number) is used as the mpp
> index. see orion_mpp_conf() implementation.
> I think the solution is to extend the MPP define to include the
> explicit gpio number, so it will look like:
> #define MPP(_num, _gpio _sel, _in, _out, _78100_A0)
That could work as long as a value is reserved to mean "not gpio" like:

#define MPP0_GPIO        MPP(0, 0, 0x0, 1, 1, 1)
#define MPP0_GE0_COL        MPP(0, -1, 0x1, 1, 0, 1)

Note that orion_gpio_set_valid() and orion_gpio_is_valid() would both 
need rework. The functions need to handle that a GPIO can be mux'ed onto 
any MPP pin. I described this problem in another email on the thread.

There has been some discussion of a pinmux API. Does this API offer any 
simpler solution?

-joey

[PATCH v2] genirq: replace irq_gc_ack() with {set,clr}_bit variants

[Simon Guinot]

Hi Thomas,

On Mon, Jul 04, 2011 at 04:33:19PM +0200, Simon Guinot wrote:
> From: Simon Guinot <sguinot@lacie.com>
> 
> Depending on the device, interrupts acknowledgement is done by setting
> or by clearing a dedicated register. Replace irq_gc_ack() with some
> {set,clr}_bit variants allows to handle both cases.
> 
> Note that this patch affects the following SoCs: Davinci, Samsung and
> Orion. Except for this last, the change is minor: irq_gc_ack() is just
> renamed into irq_gc_ack_set_bit().
> 
> For the Orion SoCs, the edge GPIO interrupts support is currently
> broken. irq_gc_ack() try to acknowledge a such interrupt by setting
> the corresponding cause register bit. The Orion GPIO device expect the
> opposite. To fix this issue, the irq_gc_ack_clr_bit() variant is used.
> 
> Tested on Network Space v2.
> 
> Reported-by: Joey Oravec <joravec@drewtech.com>
> Signed-off-by: Simon Guinot <sguinot@lacie.com>
> ---
> Changes for v2: update patch description (mention the affected SoCs).
> 
>  arch/arm/mach-davinci/irq.c      |    2 +-
>  arch/arm/plat-orion/gpio.c       |    2 +-
>  arch/arm/plat-s5p/irq-gpioint.c  |    2 +-
>  arch/arm/plat-samsung/irq-uart.c |    2 +-
>  include/linux/irq.h              |    3 ++-
>  kernel/irq/generic-chip.c        |   18 ++++++++++++++++--
>  6 files changed, 22 insertions(+), 7 deletions(-)

Please, apply this patch.

Regards,

Simon
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110706/dd8a0d68/attachment.sig>

plat-orion multi purpose pins problem for mv78200

[Simon Guinot]

On Tue, Jul 05, 2011 at 11:37:57AM -0400, Joey Oravec wrote:
> On 7/3/2011 8:46 AM, saeed bishara wrote:
> >>It doesn't seems the code was behaving differently before 3.0-rc. I mean
> >>before the MPP and GPIO consolidation work. It was ?
> >that's right, this issue was before the consolidation work.
> Agreed. I only meant to say that the MPP code is broken for mv78200
> in 3.0-rc5, so the maintainers should consider a short-term fix of
> disabling the checks in orion_gpio_is_valid().

And break code relying on the orion_gpio_is_valid() correctness ?
IMHO, this kind of workaround is just not acceptable.

> 
> >>Does hardware allows a configuration like MPP0_GE0_COL and MPP32_GPIO
> >>simultaneously ?
> >yes, this is possible.
> Agreed. MPP is the pin that can be mux'ed to 7 resources. GPIO is
> just one of those resources. Yes the hardware supports your example.
> Most resources can be routed to several different pins to allow
> flexibility in board design.
> 
> Since MPP is just a pin-mux, it's possible for Marvell to build an
> SoC in the future that can route a few different GPIOs onto a given
> MPP pin. It seems like any solution should account for this.
> 
> >>If not (and only if not), the problem is simply mapping a MPP number on
> >>the correct GPIO number. Isn't it ?
> >>
> >>In that case, what about using the MPPxx_GPIO definitions (found in
> >>mach-mv78xx0/mpp.h) to map the right GPIO number ?
> >>
> >>For example:
> >>
> >>#define MPP32_GPIO        MPP(32, 0x1, 1, 1, 1)
> >>
> >>could be replaced by:
> >>
> >>#define MPP32_GPIO        MPP(0, 0x1, 1, 1, 1)
> >that won't work because the num (first number) is used as the mpp
> >index. see orion_mpp_conf() implementation.
> >I think the solution is to extend the MPP define to include the
> >explicit gpio number, so it will look like:
> >#define MPP(_num, _gpio _sel, _in, _out, _78100_A0)
> That could work as long as a value is reserved to mean "not gpio" like:
> 
> #define MPP0_GPIO        MPP(0, 0, 0x0, 1, 1, 1)
> #define MPP0_GE0_COL        MPP(0, -1, 0x1, 1, 0, 1)

Maybe we could add a function pointer to the parameter list for
orion_mpp_conf(). Something like: int (*mpp_to_gpio) (unsigned int num).

If given to orion_mpp_conf(), the function would be used to perform the
MPP to GPIO conversion. And if NULL, a 1:1 GPIO to MPP mapping would be
used (current code).

A such solution would let almost unchanged the code found in
mach-{kirkwood,orion5x,dove}.

> 
> Note that orion_gpio_set_valid() and orion_gpio_is_valid() would
> both need rework. The functions need to handle that a GPIO can be
> mux'ed onto any MPP pin. I described this problem in another email
> on the thread.

I don't understand what's wrong with the GPIO array.

Simon
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110706/7c8a4a63/attachment.sig>

plat-orion multi purpose pins problem for mv78200

[saeed bishara]

>> >>It doesn't seems the code was behaving differently before 3.0-rc. I mean
>> >>before the MPP and GPIO consolidation work. It was ?
>> >that's right, this issue was before the consolidation work.
>> Agreed. I only meant to say that the MPP code is broken for mv78200
>> in 3.0-rc5, so the maintainers should consider a short-term fix of
>> disabling the checks in orion_gpio_is_valid().
>
> And break code relying on the orion_gpio_is_valid() correctness ?
> IMHO, this kind of workaround is just not acceptable.
agree
>
>> >I think the solution is to extend the MPP define to include the
>> >explicit gpio number, so it will look like:
>> >#define MPP(_num, _gpio _sel, _in, _out, _78100_A0)
>> That could work as long as a value is reserved to mean "not gpio" like:
>>
>> #define MPP0_GPIO ? ? ? ?MPP(0, 0, 0x0, 1, 1, 1)
>> #define MPP0_GE0_COL ? ? ? ?MPP(0, -1, 0x1, 1, 0, 1)
>
> Maybe we could add a function pointer to the parameter list for
> orion_mpp_conf(). Something like: int (*mpp_to_gpio) (unsigned int num).
>
> If given to orion_mpp_conf(), the function would be used to perform the
> MPP to GPIO conversion. And if NULL, a 1:1 GPIO to MPP mapping would be
> used (current code).
this is not enough, because for each mpp element of the mpp_list, we
have to know whether the mpp is selected for gpio mode or not.
saeed

plat-orion multi purpose pins problem for mv78200

[Joey Oravec]

On 7/6/2011 12:18 PM, Simon Guinot wrote:
>> Note that orion_gpio_set_valid() and orion_gpio_is_valid() would
>> both need rework. The functions need to handle that a GPIO can be
>> mux'ed onto any MPP pin. I described this problem in another email
>> on the thread.
> I don't understand what's wrong with the GPIO array.

I tried to describe the case in my reply: 
http://lists.arm.linux.org.uk/lurker/message/20110701.215657.7efe0a42.en.html

Assume that we've solved the mpp_to_gpio mapping. Then imagine you pass 
a large array to mv78xx0_mpp_conf() that includes:

MPP16_GPIO (this mpp corresponds to GPIO16)
MPP47_UNUSED (this mpp corresponds to GPIO16)

The code today processes the array in-order. When it processes 
MPP16_GPIO it will mark the GPIO16 valid. When it processes MPP47_UNUSED 
it would currently mark GPIO16 invalid. This is a problem because it 
still assumes a 1:1 relationship.

-joey

plat-orion multi purpose pins problem for mv78200

[saeed bishara]

On Wed, Jul 6, 2011 at 9:49 PM, Joey Oravec <joravec@drewtech.com> wrote:
> On 7/6/2011 12:18 PM, Simon Guinot wrote:
>>>
>>> Note that orion_gpio_set_valid() and orion_gpio_is_valid() would
>>> both need rework. The functions need to handle that a GPIO can be
>>> mux'ed onto any MPP pin. I described this problem in another email
>>> on the thread.
>>
>> I don't understand what's wrong with the GPIO array.
>
> I tried to describe the case in my reply:
> http://lists.arm.linux.org.uk/lurker/message/20110701.215657.7efe0a42.en.html
>
> Assume that we've solved the mpp_to_gpio mapping. Then imagine you pass a
> large array to mv78xx0_mpp_conf() that includes:
>
> MPP16_GPIO (this mpp corresponds to GPIO16)
> MPP47_UNUSED (this mpp corresponds to GPIO16)
>
> The code today processes the array in-order. When it processes MPP16_GPIO it
> will mark the GPIO16 valid. When it processes MPP47_UNUSED it would
> currently mark GPIO16 invalid. This is a problem because it still assumes a
> 1:1 relationship.
I agree, this is why we need some method to make the orion_mpp_conf()
know which mpps are gpios, when that done, then the gpio of MPP47 in
your case will not be set as invalid.
one option to do that is to assume that mpp with _in == out_ == 0 is
not a gpio. so the orion_mpp_conf() will look like this:
--- a/arch/arm/plat-orion/mpp.c
+++ b/arch/arm/plat-orion/mpp.c
@@ -41,6 +41,7 @@ void __init orion_mpp_conf(unsigned int *mpp_list,
unsigned int variant_mask,
        for ( ; *mpp_list; mpp_list++) {
                unsigned int num = MPP_NUM(*mpp_list);
                unsigned int sel = MPP_SEL(*mpp_list);
+               unsigned int gpio_num = MPP_GPIO(*mpp_list);
                int shift, gpio_mode;

                if (num > mpp_max) {
@@ -64,9 +65,8 @@ void __init orion_mpp_conf(unsigned int *mpp_list,
unsigned int variant_mask,
                        gpio_mode |= GPIO_INPUT_OK;
                if (*mpp_list & MPP_OUTPUT_MASK)
                        gpio_mode |= GPIO_OUTPUT_OK;
-               if (sel != 0)
-                       gpio_mode = 0;
-               orion_gpio_set_valid(num, gpio_mode);
+               if (gpio_mode != 0)
+                       orion_gpio_set_valid(gpio_num, gpio_mode);
        }


and of course this will require that any non gpio mpp will have to
have the _in and _out set to 0.
saeed

plat-orion multi purpose pins problem for mv78200

[Joey Oravec]

On 7/7/2011 2:40 AM, saeed bishara wrote:
>>>> Note that orion_gpio_set_valid() and orion_gpio_is_valid() would
>>>> both need rework. The functions need to handle that a GPIO can be
>>>> mux'ed onto any MPP pin. I described this problem in another email
>>>> on the thread.
>>> I don't understand what's wrong with the GPIO array.
>> I tried to describe the case in my reply:
>> http://lists.arm.linux.org.uk/lurker/message/20110701.215657.7efe0a42.en.html
>>
>> Assume that we've solved the mpp_to_gpio mapping. Then imagine you pass a
>> large array to mv78xx0_mpp_conf() that includes:
>>
>> MPP16_GPIO (this mpp corresponds to GPIO16)
>> MPP47_UNUSED (this mpp corresponds to GPIO16)
>>
>> The code today processes the array in-order. When it processes MPP16_GPIO it
>> will mark the GPIO16 valid. When it processes MPP47_UNUSED it would
>> currently mark GPIO16 invalid. This is a problem because it still assumes a
>> 1:1 relationship.
> I agree, this is why we need some method to make the orion_mpp_conf()
> know which mpps are gpios, when that done, then the gpio of MPP47 in
> your case will not be set as invalid.
> one option to do that is to assume that mpp with _in == out_ == 0 is
> not a gpio. so the orion_mpp_conf() will look like this:
> --- a/arch/arm/plat-orion/mpp.c
> +++ b/arch/arm/plat-orion/mpp.c
> @@ -41,6 +41,7 @@ void __init orion_mpp_conf(unsigned int *mpp_list,
> unsigned int variant_mask,
>          for ( ; *mpp_list; mpp_list++) {
>                  unsigned int num = MPP_NUM(*mpp_list);
>                  unsigned int sel = MPP_SEL(*mpp_list);
> +               unsigned int gpio_num = MPP_GPIO(*mpp_list);
>                  int shift, gpio_mode;
>
>                  if (num>  mpp_max) {
> @@ -64,9 +65,8 @@ void __init orion_mpp_conf(unsigned int *mpp_list,
> unsigned int variant_mask,
>                          gpio_mode |= GPIO_INPUT_OK;
>                  if (*mpp_list&  MPP_OUTPUT_MASK)
>                          gpio_mode |= GPIO_OUTPUT_OK;
> -               if (sel != 0)
> -                       gpio_mode = 0;
> -               orion_gpio_set_valid(num, gpio_mode);
> +               if (gpio_mode != 0)
> +                       orion_gpio_set_valid(gpio_num, gpio_mode);
>          }
>
>
> and of course this will require that any non gpio mpp will have to
> have the _in and _out set to 0.

Yes this proposal is better for mv78xx0 than the current code, but it 
may fail if you call orion_mpp_conf() multiple times. Imagine you setup 
the MPP16_GPIO as described, then a subsequent call wants to configure 
differently and sets:

MPP16_UNUSED (this mpp corresponds to GPIO16)

Note orion_gpio_set_valid() contains code to mark GPIOs as valid or 
invalid, but proposed change will only make a call to mark a GPIO as 
valid. This example would reconfigure the MPP but leave the GPIO marked 
valid.

Eventually, we would need to handle this example to implement the 
proposed pinmux API. Right?

-joey

plat-orion multi purpose pins problem for mv78200

[saeed bishara]

On Thu, Jul 7, 2011 at 4:49 PM, Joey Oravec <joravec@drewtech.com> wrote:
> On 7/7/2011 2:40 AM, saeed bishara wrote:
>>>>>
>>>>> Note that orion_gpio_set_valid() and orion_gpio_is_valid() would
>>>>> both need rework. The functions need to handle that a GPIO can be
>>>>> mux'ed onto any MPP pin. I described this problem in another email
>>>>> on the thread.
>>>>
>>>> I don't understand what's wrong with the GPIO array.
>>>
>>> I tried to describe the case in my reply:
>>>
>>> http://lists.arm.linux.org.uk/lurker/message/20110701.215657.7efe0a42.en.html
>>>
>>> Assume that we've solved the mpp_to_gpio mapping. Then imagine you pass a
>>> large array to mv78xx0_mpp_conf() that includes:
>>>
>>> MPP16_GPIO (this mpp corresponds to GPIO16)
>>> MPP47_UNUSED (this mpp corresponds to GPIO16)
>>>
>>> The code today processes the array in-order. When it processes MPP16_GPIO
>>> it
>>> will mark the GPIO16 valid. When it processes MPP47_UNUSED it would
>>> currently mark GPIO16 invalid. This is a problem because it still assumes
>>> a
>>> 1:1 relationship.
>>
>> I agree, this is why we need some method to make the orion_mpp_conf()
>> know which mpps are gpios, when that done, then the gpio of MPP47 in
>> your case will not be set as invalid.
>> one option to do that is to assume that mpp with _in == out_ == 0 is
>> not a gpio. so the orion_mpp_conf() will look like this:
>> --- a/arch/arm/plat-orion/mpp.c
>> +++ b/arch/arm/plat-orion/mpp.c
>> @@ -41,6 +41,7 @@ void __init orion_mpp_conf(unsigned int *mpp_list,
>> unsigned int variant_mask,
>> ? ? ? ? for ( ; *mpp_list; mpp_list++) {
>> ? ? ? ? ? ? ? ? unsigned int num = MPP_NUM(*mpp_list);
>> ? ? ? ? ? ? ? ? unsigned int sel = MPP_SEL(*mpp_list);
>> + ? ? ? ? ? ? ? unsigned int gpio_num = MPP_GPIO(*mpp_list);
>> ? ? ? ? ? ? ? ? int shift, gpio_mode;
>>
>> ? ? ? ? ? ? ? ? if (num> ?mpp_max) {
>> @@ -64,9 +65,8 @@ void __init orion_mpp_conf(unsigned int *mpp_list,
>> unsigned int variant_mask,
>> ? ? ? ? ? ? ? ? ? ? ? ? gpio_mode |= GPIO_INPUT_OK;
>> ? ? ? ? ? ? ? ? if (*mpp_list& ?MPP_OUTPUT_MASK)
>> ? ? ? ? ? ? ? ? ? ? ? ? gpio_mode |= GPIO_OUTPUT_OK;
>> - ? ? ? ? ? ? ? if (sel != 0)
>> - ? ? ? ? ? ? ? ? ? ? ? gpio_mode = 0;
>> - ? ? ? ? ? ? ? orion_gpio_set_valid(num, gpio_mode);
>> + ? ? ? ? ? ? ? if (gpio_mode != 0)
>> + ? ? ? ? ? ? ? ? ? ? ? orion_gpio_set_valid(gpio_num, gpio_mode);
>> ? ? ? ? }
>>
>>
>> and of course this will require that any non gpio mpp will have to
>> have the _in and _out set to 0.
>
> Yes this proposal is better for mv78xx0 than the current code, but it may
> fail if you call orion_mpp_conf() multiple times. Imagine you setup the
> MPP16_GPIO as described, then a subsequent call wants to configure
> differently and sets:
the issue that you're describing is theoretical and doesn't happen at
least in the boards that supported by the kernel.
I don't think it deserve to complicate our code.
>
> MPP16_UNUSED (this mpp corresponds to GPIO16)
>
> Note orion_gpio_set_valid() contains code to mark GPIOs as valid or invalid,
> but proposed change will only make a call to mark a GPIO as valid. This
> example would reconfigure the MPP but leave the GPIO marked valid.
>
> Eventually, we would need to handle this example to implement the proposed
> pinmux API. Right?
it makes sense. have you tried it?
>
> -joey
>

plat-orion multi purpose pins problem for mv78200

[Joey Oravec]

On 7/10/2011 9:21 AM, saeed bishara wrote:
>>> I agree, this is why we need some method to make the orion_mpp_conf()
>>> know which mpps are gpios, when that done, then the gpio of MPP47 in
>>> your case will not be set as invalid.
>>> one option to do that is to assume that mpp with _in == out_ == 0 is
>>> not a gpio. so the orion_mpp_conf() will look like this:
>>> --- a/arch/arm/plat-orion/mpp.c
>>> +++ b/arch/arm/plat-orion/mpp.c
>>> @@ -41,6 +41,7 @@ void __init orion_mpp_conf(unsigned int *mpp_list,
>>> unsigned int variant_mask,
>>>          for ( ; *mpp_list; mpp_list++) {
>>>                  unsigned int num = MPP_NUM(*mpp_list);
>>>                  unsigned int sel = MPP_SEL(*mpp_list);
>>> +               unsigned int gpio_num = MPP_GPIO(*mpp_list);
>>>                  int shift, gpio_mode;
>>>
>>>                  if (num>    mpp_max) {
>>> @@ -64,9 +65,8 @@ void __init orion_mpp_conf(unsigned int *mpp_list,
>>> unsigned int variant_mask,
>>>                          gpio_mode |= GPIO_INPUT_OK;
>>>                  if (*mpp_list&    MPP_OUTPUT_MASK)
>>>                          gpio_mode |= GPIO_OUTPUT_OK;
>>> -               if (sel != 0)
>>> -                       gpio_mode = 0;
>>> -               orion_gpio_set_valid(num, gpio_mode);
>>> +               if (gpio_mode != 0)
>>> +                       orion_gpio_set_valid(gpio_num, gpio_mode);
>>>          }
>>>
>>>
>>> and of course this will require that any non gpio mpp will have to
>>> have the _in and _out set to 0.
>> Yes this proposal is better for mv78xx0 than the current code, but it may
>> fail if you call orion_mpp_conf() multiple times. Imagine you setup the
>> MPP16_GPIO as described, then a subsequent call wants to configure
>> differently and sets:
> the issue that you're describing is theoretical and doesn't happen at
> least in the boards that supported by the kernel.
> I don't think it deserve to complicate our code.

I agree no boards are currently doing this, especially since the 
functions are marked __init and the code is not capable of it today. But 
it's reasonable that a driver would want to switch MPPs between a 
peripheral and a high-impedance state, for example during suspend/resume.

>> MPP16_UNUSED (this mpp corresponds to GPIO16)
>>
>> Note orion_gpio_set_valid() contains code to mark GPIOs as valid or invalid,
>> but proposed change will only make a call to mark a GPIO as valid. This
>> example would reconfigure the MPP but leave the GPIO marked valid.
>>
>> Eventually, we would need to handle this example to implement the proposed
>> pinmux API. Right?
> it makes sense. have you tried it?

The latest proposal I could find is from June: 
http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/052955.html

I have not tried to implement that API yet. The proposal allows changing 
mux settings at any time, so it seems like the mv78xx0 should be 
prepared to do that at some point in the future.

-joey

[PATCH v2] genirq: replace irq_gc_ack() with {set, clr}_bit variants

[Kukjin Kim]

Nicolas Pitre wrote:
> 
> On Mon, 4 Jul 2011, Simon Guinot wrote:
> 
> > From: Simon Guinot <sguinot@lacie.com>
> >
> > Depending on the device, interrupts acknowledgement is done by setting
> > or by clearing a dedicated register. Replace irq_gc_ack() with some
> > {set,clr}_bit variants allows to handle both cases.
> >
> > Note that this patch affects the following SoCs: Davinci, Samsung and
> > Orion. Except for this last, the change is minor: irq_gc_ack() is just
> > renamed into irq_gc_ack_set_bit().
> >
> > For the Orion SoCs, the edge GPIO interrupts support is currently
> > broken. irq_gc_ack() try to acknowledge a such interrupt by setting
> > the corresponding cause register bit. The Orion GPIO device expect the
> > opposite. To fix this issue, the irq_gc_ack_clr_bit() variant is used.
> >
> > Tested on Network Space v2.
> >
> > Reported-by: Joey Oravec <joravec@drewtech.com>
> > Signed-off-by: Simon Guinot <sguinot@lacie.com>
> 
> Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>
> 
Sorry for late response...

If required,
Acked-by: Kukjin Kim <kgene.kim@samsung.com>

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

> 
> > ---
> > Changes for v2: update patch description (mention the affected SoCs).
> >
> >  arch/arm/mach-davinci/irq.c      |    2 +-
> >  arch/arm/plat-orion/gpio.c       |    2 +-
> >  arch/arm/plat-s5p/irq-gpioint.c  |    2 +-
> >  arch/arm/plat-samsung/irq-uart.c |    2 +-
> >  include/linux/irq.h              |    3 ++-
> >  kernel/irq/generic-chip.c        |   18 ++++++++++++++++--
> >  6 files changed, 22 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/arm/mach-davinci/irq.c b/arch/arm/mach-davinci/irq.c
> > index bfe68ec..d8c1af0 100644
> > --- a/arch/arm/mach-davinci/irq.c
> > +++ b/arch/arm/mach-davinci/irq.c
> > @@ -53,7 +53,7 @@ davinci_alloc_gc(void __iomem *base, unsigned int
> irq_start, unsigned int num)
> >
> >  	gc = irq_alloc_generic_chip("AINTC", 1, irq_start, base,
handle_edge_irq);
> >  	ct = gc->chip_types;
> > -	ct->chip.irq_ack = irq_gc_ack;
> > +	ct->chip.irq_ack = irq_gc_ack_set_bit;
> >  	ct->chip.irq_mask = irq_gc_mask_clr_bit;
> >  	ct->chip.irq_unmask = irq_gc_mask_set_bit;
> >
> > diff --git a/arch/arm/plat-orion/gpio.c b/arch/arm/plat-orion/gpio.c
> > index 5b4fffa..41ab97e 100644
> > --- a/arch/arm/plat-orion/gpio.c
> > +++ b/arch/arm/plat-orion/gpio.c
> > @@ -432,7 +432,7 @@ void __init orion_gpio_init(int gpio_base, int
ngpio,
> >  	ct->regs.mask = ochip->mask_offset + GPIO_EDGE_MASK_OFF;
> >  	ct->regs.ack = GPIO_EDGE_CAUSE_OFF;
> >  	ct->type = IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING;
> > -	ct->chip.irq_ack = irq_gc_ack;
> > +	ct->chip.irq_ack = irq_gc_ack_clr_bit;
> >  	ct->chip.irq_mask = irq_gc_mask_clr_bit;
> >  	ct->chip.irq_unmask = irq_gc_mask_set_bit;
> >  	ct->chip.irq_set_type = gpio_irq_set_type;
> > diff --git a/arch/arm/plat-s5p/irq-gpioint.c
b/arch/arm/plat-s5p/irq-gpioint.c
> > index 135abda..327ab9f 100644
> > --- a/arch/arm/plat-s5p/irq-gpioint.c
> > +++ b/arch/arm/plat-s5p/irq-gpioint.c
> > @@ -152,7 +152,7 @@ static __init int s5p_gpioint_add(struct
s3c_gpio_chip
> *chip)
> >  	if (!gc)
> >  		return -ENOMEM;
> >  	ct = gc->chip_types;
> > -	ct->chip.irq_ack = irq_gc_ack;
> > +	ct->chip.irq_ack = irq_gc_ack_set_bit;
> >  	ct->chip.irq_mask = irq_gc_mask_set_bit;
> >  	ct->chip.irq_unmask = irq_gc_mask_clr_bit;
> >  	ct->chip.irq_set_type = s5p_gpioint_set_type,
> > diff --git a/arch/arm/plat-samsung/irq-uart.c
b/arch/arm/plat-samsung/irq-uart.c
> > index 32582c0..0e46588 100644
> > --- a/arch/arm/plat-samsung/irq-uart.c
> > +++ b/arch/arm/plat-samsung/irq-uart.c
> > @@ -55,7 +55,7 @@ static void __init s3c_init_uart_irq(struct
s3c_uart_irq *uirq)
> >  	gc = irq_alloc_generic_chip("s3c-uart", 1, uirq->base_irq, reg_base,
> >  				    handle_level_irq);
> >  	ct = gc->chip_types;
> > -	ct->chip.irq_ack = irq_gc_ack;
> > +	ct->chip.irq_ack = irq_gc_ack_set_bit;
> >  	ct->chip.irq_mask = irq_gc_mask_set_bit;
> >  	ct->chip.irq_unmask = irq_gc_mask_clr_bit;
> >  	ct->regs.ack = S3C64XX_UINTP;
> > diff --git a/include/linux/irq.h b/include/linux/irq.h
> > index 8b45384..baa397e 100644
> > --- a/include/linux/irq.h
> > +++ b/include/linux/irq.h
> > @@ -676,7 +676,8 @@ void irq_gc_mask_disable_reg(struct irq_data *d);
> >  void irq_gc_mask_set_bit(struct irq_data *d);
> >  void irq_gc_mask_clr_bit(struct irq_data *d);
> >  void irq_gc_unmask_enable_reg(struct irq_data *d);
> > -void irq_gc_ack(struct irq_data *d);
> > +void irq_gc_ack_set_bit(struct irq_data *d);
> > +void irq_gc_ack_clr_bit(struct irq_data *d);
> >  void irq_gc_mask_disable_reg_and_ack(struct irq_data *d);
> >  void irq_gc_eoi(struct irq_data *d);
> >  int irq_gc_set_wake(struct irq_data *d, unsigned int on);
> > diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
> > index 31a9db7..3a2cab4 100644
> > --- a/kernel/irq/generic-chip.c
> > +++ b/kernel/irq/generic-chip.c
> > @@ -101,10 +101,10 @@ void irq_gc_unmask_enable_reg(struct irq_data *d)
> >  }
> >
> >  /**
> > - * irq_gc_ack - Ack pending interrupt
> > + * irq_gc_ack_set_bit - Ack pending interrupt via setting bit
> >   * @d: irq_data
> >   */
> > -void irq_gc_ack(struct irq_data *d)
> > +void irq_gc_ack_set_bit(struct irq_data *d)
> >  {
> >  	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> >  	u32 mask = 1 << (d->irq - gc->irq_base);
> > @@ -115,6 +115,20 @@ void irq_gc_ack(struct irq_data *d)
> >  }
> >
> >  /**
> > + * irq_gc_ack_clr_bit - Ack pending interrupt via clearing bit
> > + * @d: irq_data
> > + */
> > +void irq_gc_ack_clr_bit(struct irq_data *d)
> > +{
> > +	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> > +	u32 mask = ~(1 << (d->irq - gc->irq_base));
> > +
> > +	irq_gc_lock(gc);
> > +	irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack);
> > +	irq_gc_unlock(gc);
> > +}
> > +
> > +/**
> >   * irq_gc_mask_disable_reg_and_ack- Mask and ack pending interrupt
> >   * @d: irq_data
> >   */
> > --
> > 1.7.5.1
> >