linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH] gpio/omap: Fix IRQ handling for SPARSE_IRQ
@ 2012-02-23 22:46 Cousson, Benoit
  2012-02-23 23:08 ` Tony Lindgren
  2012-02-24 14:14 ` Rob Herring
  0 siblings, 2 replies; 12+ messages in thread
From: Cousson, Benoit @ 2012-02-23 22:46 UTC (permalink / raw)
  To: linux-arm-kernel

The GPIO driver is still relying on internal OMAP IRQ defines that
are not relevant anymore if OMAP is built with SPARSE_IRQ.

Replace the defines with the proper IRQ base number.
Clean some comment style issue.
Remove some hidden and ugly cpu_class_is_omap1() inside the
gpio header.

XXX: That fix might be broken for OMAP1 MPUIO case.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
---

Hi Tony,

Please note that this patch is still RFC, because I do not know how to fix properly the ugly cpu_class_is_omap1 and the dependency with IH_MPUIO_BASE to detect a MPUIO.

I'm still sending it, because it is needed to have SPARSE_IRQ working on OMAP4 with the previous series I've just sent.

Regards,
Benoit

 arch/arm/plat-omap/include/plat/gpio.h |   22 ++------------------
 drivers/gpio/gpio-omap.c               |   33 ++++++++++++++++---------------
 2 files changed, 20 insertions(+), 35 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/gpio.h b/arch/arm/plat-omap/include/plat/gpio.h
index cb75b65..b8a96c6 100644
--- a/arch/arm/plat-omap/include/plat/gpio.h
+++ b/arch/arm/plat-omap/include/plat/gpio.h
@@ -218,30 +218,14 @@ extern void omap_set_gpio_debounce(int gpio, int enable);
 extern void omap_set_gpio_debounce_time(int gpio, int enable);
 /*-------------------------------------------------------------------------*/
 
-/* Wrappers for "new style" GPIO calls, using the new infrastructure
+/*
+ * Wrappers for "new style" GPIO calls, using the new infrastructure
  * which lets us plug in FPGA, I2C, and other implementations.
- * *
+ *
  * The original OMAP-specific calls should eventually be removed.
  */
 
 #include <linux/errno.h>
 #include <asm-generic/gpio.h>
 
-static inline int irq_to_gpio(unsigned irq)
-{
-	int tmp;
-
-	/* omap1 SOC mpuio */
-	if (cpu_class_is_omap1() && (irq < (IH_MPUIO_BASE + 16)))
-		return (irq - IH_MPUIO_BASE) + OMAP_MAX_GPIO_LINES;
-
-	/* SOC gpio */
-	tmp = irq - IH_GPIO_BASE;
-	if (tmp < OMAP_MAX_GPIO_LINES)
-		return tmp;
-
-	/* we don't supply reverse mappings for non-SOC gpios */
-	return -EIO;
-}
-
 #endif
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index bc2bd69..afef0f7 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -93,6 +93,11 @@ struct gpio_bank {
 #define GPIO_BIT(bank, gpio) (1 << GPIO_INDEX(bank, gpio))
 #define GPIO_MOD_CTRL_BIT	BIT(0)
 
+static int irq_to_gpio(struct gpio_bank *bank, unsigned int gpio_irq)
+{
+	return gpio_irq - bank->irq_base + bank->chip.base;
+}
+
 static void _set_gpio_direction(struct gpio_bank *bank, int gpio, int is_input)
 {
 	void __iomem *reg = bank->base;
@@ -369,7 +374,7 @@ static int _set_gpio_triggering(struct gpio_bank *bank, int gpio, int trigger)
 
 static int gpio_irq_type(struct irq_data *d, unsigned type)
 {
-	struct gpio_bank *bank;
+	struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
 	unsigned gpio;
 	int retval;
 	unsigned long flags;
@@ -377,13 +382,11 @@ static int gpio_irq_type(struct irq_data *d, unsigned type)
 	if (!cpu_class_is_omap2() && d->irq > IH_MPUIO_BASE)
 		gpio = OMAP_MPUIO(d->irq - IH_MPUIO_BASE);
 	else
-		gpio = d->irq - IH_GPIO_BASE;
+		gpio = irq_to_gpio(bank, d->irq);
 
 	if (type & ~IRQ_TYPE_SENSE_MASK)
 		return -EINVAL;
 
-	bank = irq_data_get_irq_chip_data(d);
-
 	if (!bank->regs->leveldetect0 &&
 		(type & (IRQ_TYPE_LEVEL_LOW|IRQ_TYPE_LEVEL_HIGH)))
 		return -EINVAL;
@@ -524,14 +527,10 @@ static void _reset_gpio(struct gpio_bank *bank, int gpio)
 /* Use disable_irq_wake() and enable_irq_wake() functions from drivers */
 static int gpio_wake_enable(struct irq_data *d, unsigned int enable)
 {
-	unsigned int gpio = d->irq - IH_GPIO_BASE;
-	struct gpio_bank *bank;
-	int retval;
-
-	bank = irq_data_get_irq_chip_data(d);
-	retval = _set_gpio_wakeup(bank, gpio, enable);
+	struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
+	unsigned int gpio = irq_to_gpio(bank, d->irq);
 
-	return retval;
+	return _set_gpio_wakeup(bank, gpio, enable);
 }
 
 static int omap_gpio_request(struct gpio_chip *chip, unsigned offset)
@@ -675,11 +674,13 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
 
 		gpio_irq = bank->irq_base;
 		for (; isr != 0; isr >>= 1, gpio_irq++) {
-			gpio_index = GPIO_INDEX(bank, irq_to_gpio(gpio_irq));
+			int gpio = irq_to_gpio(bank, gpio_irq);
 
 			if (!(isr & 1))
 				continue;
 
+			gpio_index = GPIO_INDEX(bank, gpio);
+
 			/*
 			 * Some chips can't respond to both rising and falling
 			 * at the same time.  If this irq was requested with
@@ -705,8 +706,8 @@ exit:
 
 static void gpio_irq_shutdown(struct irq_data *d)
 {
-	unsigned int gpio = d->irq - IH_GPIO_BASE;
 	struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
+	unsigned int gpio = irq_to_gpio(bank, d->irq);
 	unsigned long flags;
 
 	spin_lock_irqsave(&bank->lock, flags);
@@ -716,16 +717,16 @@ static void gpio_irq_shutdown(struct irq_data *d)
 
 static void gpio_ack_irq(struct irq_data *d)
 {
-	unsigned int gpio = d->irq - IH_GPIO_BASE;
 	struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
+	unsigned int gpio = irq_to_gpio(bank, d->irq);
 
 	_clear_gpio_irqstatus(bank, gpio);
 }
 
 static void gpio_mask_irq(struct irq_data *d)
 {
-	unsigned int gpio = d->irq - IH_GPIO_BASE;
 	struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
+	unsigned int gpio = irq_to_gpio(bank, d->irq);
 	unsigned long flags;
 
 	spin_lock_irqsave(&bank->lock, flags);
@@ -736,8 +737,8 @@ static void gpio_mask_irq(struct irq_data *d)
 
 static void gpio_unmask_irq(struct irq_data *d)
 {
-	unsigned int gpio = d->irq - IH_GPIO_BASE;
 	struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
+	unsigned int gpio = irq_to_gpio(bank, d->irq);
 	unsigned int irq_mask = GPIO_BIT(bank, gpio);
 	u32 trigger = irqd_get_trigger_type(d);
 	unsigned long flags;
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC PATCH] gpio/omap: Fix IRQ handling for SPARSE_IRQ
  2012-02-23 22:46 [RFC PATCH] gpio/omap: Fix IRQ handling for SPARSE_IRQ Cousson, Benoit
@ 2012-02-23 23:08 ` Tony Lindgren
  2012-02-24 10:11   ` Cousson, Benoit
  2012-02-24 14:14 ` Rob Herring
  1 sibling, 1 reply; 12+ messages in thread
From: Tony Lindgren @ 2012-02-23 23:08 UTC (permalink / raw)
  To: linux-arm-kernel

* Cousson, Benoit <b-cousson@ti.com> [120223 14:14]:
> The GPIO driver is still relying on internal OMAP IRQ defines that
> are not relevant anymore if OMAP is built with SPARSE_IRQ.

Great :)
 
> Please note that this patch is still RFC, because I do not know
> how to fix properly the ugly cpu_class_is_omap1 and the dependency
> with IH_MPUIO_BASE to detect a MPUIO.

Sounds like gpio_to_irq() needs to be set in the
arch/arm/*omap*/gpio*.c then.

Tony

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC PATCH] gpio/omap: Fix IRQ handling for SPARSE_IRQ
  2012-02-23 23:08 ` Tony Lindgren
@ 2012-02-24 10:11   ` Cousson, Benoit
  2012-02-24 10:37     ` DebBarma, Tarun Kanti
  0 siblings, 1 reply; 12+ messages in thread
From: Cousson, Benoit @ 2012-02-24 10:11 UTC (permalink / raw)
  To: linux-arm-kernel

+ Tarun

On 2/24/2012 12:08 AM, Tony Lindgren wrote:
> * Cousson, Benoit<b-cousson@ti.com>  [120223 14:14]:
>> The GPIO driver is still relying on internal OMAP IRQ defines that
>> are not relevant anymore if OMAP is built with SPARSE_IRQ.
>
> Great :)
>
>> Please note that this patch is still RFC, because I do not know
>> how to fix properly the ugly cpu_class_is_omap1 and the dependency
>> with IH_MPUIO_BASE to detect a MPUIO.
>
> Sounds like gpio_to_irq() needs to be set in the
> arch/arm/*omap*/gpio*.c then.

In fact, after a second thought, that might even work for OMAP1 because 
I'm using the proper base (IRQ and GPIO) to convert the IRQ number.

static int irq_to_gpio(struct gpio_bank *bank, unsigned int gpio_irq)
{
	return gpio_irq - bank->irq_base + bank->chip.base;
}

But it might be good to test it on OMAP1 platform.


Tarun,

Do you have an OMAP1 board to test that.

Regards,
Benoit

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC PATCH] gpio/omap: Fix IRQ handling for SPARSE_IRQ
  2012-02-24 10:11   ` Cousson, Benoit
@ 2012-02-24 10:37     ` DebBarma, Tarun Kanti
  2012-02-24 13:24       ` DebBarma, Tarun Kanti
  0 siblings, 1 reply; 12+ messages in thread
From: DebBarma, Tarun Kanti @ 2012-02-24 10:37 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Feb 24, 2012 at 3:41 PM, Cousson, Benoit <b-cousson@ti.com> wrote:
> + Tarun
>
>
> On 2/24/2012 12:08 AM, Tony Lindgren wrote:
>>
>> * Cousson, Benoit<b-cousson@ti.com> ?[120223 14:14]:
>>>
>>> The GPIO driver is still relying on internal OMAP IRQ defines that
>>> are not relevant anymore if OMAP is built with SPARSE_IRQ.
>>
>>
>> Great :)
>>
>>> Please note that this patch is still RFC, because I do not know
>>> how to fix properly the ugly cpu_class_is_omap1 and the dependency
>>> with IH_MPUIO_BASE to detect a MPUIO.
>>
>>
>> Sounds like gpio_to_irq() needs to be set in the
>> arch/arm/*omap*/gpio*.c then.
>
>
> In fact, after a second thought, that might even work for OMAP1 because I'm
> using the proper base (IRQ and GPIO) to convert the IRQ number.
>
>
> static int irq_to_gpio(struct gpio_bank *bank, unsigned int gpio_irq)
> {
>
> ? ? ? ?return gpio_irq - bank->irq_base + bank->chip.base;
> }
>
> But it might be good to test it on OMAP1 platform.
>
>
> Tarun,
>
> Do you have an OMAP1 board to test that.
Yes, I will test on OMAP1 board.
--
Tarun
>
> Regards,
> Benoit

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC PATCH] gpio/omap: Fix IRQ handling for SPARSE_IRQ
  2012-02-24 10:37     ` DebBarma, Tarun Kanti
@ 2012-02-24 13:24       ` DebBarma, Tarun Kanti
  2012-02-24 13:32         ` Cousson, Benoit
  0 siblings, 1 reply; 12+ messages in thread
From: DebBarma, Tarun Kanti @ 2012-02-24 13:24 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Benoit,

On Fri, Feb 24, 2012 at 4:07 PM, DebBarma, Tarun Kanti
<tarun.kanti@ti.com> wrote:
> On Fri, Feb 24, 2012 at 3:41 PM, Cousson, Benoit <b-cousson@ti.com> wrote:
>> + Tarun
>>
>>
>> On 2/24/2012 12:08 AM, Tony Lindgren wrote:
>>>
>>> * Cousson, Benoit<b-cousson@ti.com> ?[120223 14:14]:
>>>>
>>>> The GPIO driver is still relying on internal OMAP IRQ defines that
>>>> are not relevant anymore if OMAP is built with SPARSE_IRQ.
>>>
>>>
>>> Great :)
>>>
>>>> Please note that this patch is still RFC, because I do not know
>>>> how to fix properly the ugly cpu_class_is_omap1 and the dependency
>>>> with IH_MPUIO_BASE to detect a MPUIO.
>>>
>>>
>>> Sounds like gpio_to_irq() needs to be set in the
>>> arch/arm/*omap*/gpio*.c then.
>>
>>
>> In fact, after a second thought, that might even work for OMAP1 because I'm
>> using the proper base (IRQ and GPIO) to convert the IRQ number.
>>
>>
>> static int irq_to_gpio(struct gpio_bank *bank, unsigned int gpio_irq)
>> {
>>
>> ? ? ? ?return gpio_irq - bank->irq_base + bank->chip.base;
>> }
>>
>> But it might be good to test it on OMAP1 platform.
>>
>>
>> Tarun,
>>
>> Do you have an OMAP1 board to test that.
> Yes, I will test on OMAP1 board.
I have booted the image on OMAP1 with following change.
I guess bank->irq_base was a typo?

static int irq_to_gpio(struct gpio_bank *bank, unsigned int gpio_irq)
{
        //return gpio_irq - bank->irq_base + bank->chip.base;
        return gpio_irq - bank->virtual_irq_start + bank->chip.base;
}
--
Tarun

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC PATCH] gpio/omap: Fix IRQ handling for SPARSE_IRQ
  2012-02-24 13:24       ` DebBarma, Tarun Kanti
@ 2012-02-24 13:32         ` Cousson, Benoit
  2012-02-24 13:53           ` DebBarma, Tarun Kanti
  0 siblings, 1 reply; 12+ messages in thread
From: Cousson, Benoit @ 2012-02-24 13:32 UTC (permalink / raw)
  To: linux-arm-kernel

On 2/24/2012 2:24 PM, DebBarma, Tarun Kanti wrote:
> Hi Benoit,
>
> On Fri, Feb 24, 2012 at 4:07 PM, DebBarma, Tarun Kanti
> <tarun.kanti@ti.com>  wrote:
>> On Fri, Feb 24, 2012 at 3:41 PM, Cousson, Benoit<b-cousson@ti.com>  wrote:
>>> + Tarun
>>>
>>>
>>> On 2/24/2012 12:08 AM, Tony Lindgren wrote:
>>>>
>>>> * Cousson, Benoit<b-cousson@ti.com>    [120223 14:14]:
>>>>>
>>>>> The GPIO driver is still relying on internal OMAP IRQ defines that
>>>>> are not relevant anymore if OMAP is built with SPARSE_IRQ.
>>>>
>>>>
>>>> Great :)
>>>>
>>>>> Please note that this patch is still RFC, because I do not know
>>>>> how to fix properly the ugly cpu_class_is_omap1 and the dependency
>>>>> with IH_MPUIO_BASE to detect a MPUIO.
>>>>
>>>>
>>>> Sounds like gpio_to_irq() needs to be set in the
>>>> arch/arm/*omap*/gpio*.c then.
>>>
>>>
>>> In fact, after a second thought, that might even work for OMAP1 because I'm
>>> using the proper base (IRQ and GPIO) to convert the IRQ number.
>>>
>>>
>>> static int irq_to_gpio(struct gpio_bank *bank, unsigned int gpio_irq)
>>> {
>>>
>>>         return gpio_irq - bank->irq_base + bank->chip.base;
>>> }
>>>
>>> But it might be good to test it on OMAP1 platform.
>>>
>>>
>>> Tarun,
>>>
>>> Do you have an OMAP1 board to test that.
>> Yes, I will test on OMAP1 board.
> I have booted the image on OMAP1 with following change.
> I guess bank->irq_base was a typo?

Not at all :-), it was done on purpose to get rid if the static IRQ 
definition.
I was expecting that kind of issue because OMAP1 is populating 
virtual_irq_start with the IH_MPUIO_BASE, but was expecting the dynamic 
alloc_decs to still provide the good value. The easy fix is to still use 
that for OMAP1 GPIO since nobody will want to spend time fixing OMAP1 
boards to do that properly :-(

I'll update the patch accordingly.

> static int irq_to_gpio(struct gpio_bank *bank, unsigned int gpio_irq)
> {
>          //return gpio_irq - bank->irq_base + bank->chip.base;
>          return gpio_irq - bank->virtual_irq_start + bank->chip.base;
> }
> --

Thanks for the help,
Benoit

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC PATCH] gpio/omap: Fix IRQ handling for SPARSE_IRQ
  2012-02-24 13:32         ` Cousson, Benoit
@ 2012-02-24 13:53           ` DebBarma, Tarun Kanti
  2012-02-24 13:56             ` Cousson, Benoit
  0 siblings, 1 reply; 12+ messages in thread
From: DebBarma, Tarun Kanti @ 2012-02-24 13:53 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Feb 24, 2012 at 7:02 PM, Cousson, Benoit <b-cousson@ti.com> wrote:
> On 2/24/2012 2:24 PM, DebBarma, Tarun Kanti wrote:
>>
>> Hi Benoit,
>>
>> On Fri, Feb 24, 2012 at 4:07 PM, DebBarma, Tarun Kanti
>> <tarun.kanti@ti.com> ?wrote:
>>>
>>> On Fri, Feb 24, 2012 at 3:41 PM, Cousson, Benoit<b-cousson@ti.com>
>>> ?wrote:
>>>>
>>>> + Tarun
>>>>
>>>>
>>>> On 2/24/2012 12:08 AM, Tony Lindgren wrote:
>>>>>
>>>>>
>>>>> * Cousson, Benoit<b-cousson@ti.com> ? ?[120223 14:14]:
>>>>>>
>>>>>>
>>>>>> The GPIO driver is still relying on internal OMAP IRQ defines that
>>>>>> are not relevant anymore if OMAP is built with SPARSE_IRQ.
>>>>>
>>>>>
>>>>>
>>>>> Great :)
>>>>>
>>>>>> Please note that this patch is still RFC, because I do not know
>>>>>> how to fix properly the ugly cpu_class_is_omap1 and the dependency
>>>>>> with IH_MPUIO_BASE to detect a MPUIO.
>>>>>
>>>>>
>>>>>
>>>>> Sounds like gpio_to_irq() needs to be set in the
>>>>> arch/arm/*omap*/gpio*.c then.
>>>>
>>>>
>>>>
>>>> In fact, after a second thought, that might even work for OMAP1 because
>>>> I'm
>>>> using the proper base (IRQ and GPIO) to convert the IRQ number.
>>>>
>>>>
>>>> static int irq_to_gpio(struct gpio_bank *bank, unsigned int gpio_irq)
>>>> {
>>>>
>>>> ? ? ? ?return gpio_irq - bank->irq_base + bank->chip.base;
>>>> }
>>>>
>>>> But it might be good to test it on OMAP1 platform.
>>>>
>>>>
>>>> Tarun,
>>>>
>>>> Do you have an OMAP1 board to test that.
>>>
>>> Yes, I will test on OMAP1 board.
>>
>> I have booted the image on OMAP1 with following change.
>> I guess bank->irq_base was a typo?
>
>
> Not at all :-), it was done on purpose to get rid if the static IRQ
> definition.
BTW, it was giving me following compilation error...
drivers/gpio/gpio-omap.c: In function 'irq_to_gpio':
drivers/gpio/gpio-omap.c:90: error: 'struct gpio_bank' has no member
named 'irq_base'
make[2]: *** [drivers/gpio/gpio-omap.o] Error 1
--
Tarun

> I was expecting that kind of issue because OMAP1 is populating
> virtual_irq_start with the IH_MPUIO_BASE, but was expecting the dynamic
> alloc_decs to still provide the good value. The easy fix is to still use
> that for OMAP1 GPIO since nobody will want to spend time fixing OMAP1 boards
> to do that properly :-(
>
> I'll update the patch accordingly.
>
>
>> static int irq_to_gpio(struct gpio_bank *bank, unsigned int gpio_irq)
>> {
>> ? ? ? ? //return gpio_irq - bank->irq_base + bank->chip.base;
>> ? ? ? ? return gpio_irq - bank->virtual_irq_start + bank->chip.base;
>> }
>> --
>
>
> Thanks for the help,
> Benoit
>
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC PATCH] gpio/omap: Fix IRQ handling for SPARSE_IRQ
  2012-02-24 13:53           ` DebBarma, Tarun Kanti
@ 2012-02-24 13:56             ` Cousson, Benoit
  2012-02-24 15:09               ` DebBarma, Tarun Kanti
  0 siblings, 1 reply; 12+ messages in thread
From: Cousson, Benoit @ 2012-02-24 13:56 UTC (permalink / raw)
  To: linux-arm-kernel

On 2/24/2012 2:53 PM, DebBarma, Tarun Kanti wrote:
> On Fri, Feb 24, 2012 at 7:02 PM, Cousson, Benoit<b-cousson@ti.com>  wrote:
>> On 2/24/2012 2:24 PM, DebBarma, Tarun Kanti wrote:
>>>
>>> Hi Benoit,
>>>
>>> On Fri, Feb 24, 2012 at 4:07 PM, DebBarma, Tarun Kanti
>>> <tarun.kanti@ti.com>    wrote:
>>>>
>>>> On Fri, Feb 24, 2012 at 3:41 PM, Cousson, Benoit<b-cousson@ti.com>
>>>>   wrote:
>>>>>
>>>>> + Tarun
>>>>>
>>>>>
>>>>> On 2/24/2012 12:08 AM, Tony Lindgren wrote:
>>>>>>
>>>>>>
>>>>>> * Cousson, Benoit<b-cousson@ti.com>      [120223 14:14]:
>>>>>>>
>>>>>>>
>>>>>>> The GPIO driver is still relying on internal OMAP IRQ defines that
>>>>>>> are not relevant anymore if OMAP is built with SPARSE_IRQ.
>>>>>>
>>>>>>
>>>>>>
>>>>>> Great :)
>>>>>>
>>>>>>> Please note that this patch is still RFC, because I do not know
>>>>>>> how to fix properly the ugly cpu_class_is_omap1 and the dependency
>>>>>>> with IH_MPUIO_BASE to detect a MPUIO.
>>>>>>
>>>>>>
>>>>>>
>>>>>> Sounds like gpio_to_irq() needs to be set in the
>>>>>> arch/arm/*omap*/gpio*.c then.
>>>>>
>>>>>
>>>>>
>>>>> In fact, after a second thought, that might even work for OMAP1 because
>>>>> I'm
>>>>> using the proper base (IRQ and GPIO) to convert the IRQ number.
>>>>>
>>>>>
>>>>> static int irq_to_gpio(struct gpio_bank *bank, unsigned int gpio_irq)
>>>>> {
>>>>>
>>>>>         return gpio_irq - bank->irq_base + bank->chip.base;
>>>>> }
>>>>>
>>>>> But it might be good to test it on OMAP1 platform.
>>>>>
>>>>>
>>>>> Tarun,
>>>>>
>>>>> Do you have an OMAP1 board to test that.
>>>>
>>>> Yes, I will test on OMAP1 board.
>>>
>>> I have booted the image on OMAP1 with following change.
>>> I guess bank->irq_base was a typo?
>>
>>
>> Not at all :-), it was done on purpose to get rid if the static IRQ
>> definition.
> BTW, it was giving me following compilation error...
> drivers/gpio/gpio-omap.c: In function 'irq_to_gpio':
> drivers/gpio/gpio-omap.c:90: error: 'struct gpio_bank' has no member
> named 'irq_base'
> make[2]: *** [drivers/gpio/gpio-omap.o] Error 1

Oh, that's different... you are missing the cleanup patches from the DT 
series I did before this one:

eaabbb0 gpio/omap: Fix IRQ handling for SPARSE_IRQ
fa560a7 arm/dts: OMAP3: Add gpio nodes
bfeb298 arm/dts: OMAP4: Add gpio nodes
3062158 gpio/omap: Add DT support to GPIO driver
a140d12 gpio/omap: Use devm_ API and add request_mem_region
0416689 gpio/omap: Remove bank->id information and misc cleanup
249f60c Merge branch 'for_3.4/gpio_cleanup_fixes_v9' of 
git://gitorious.org/~tarunkanti/omap-sw-deve

I'll push a temp branch for you.

Benoit

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC PATCH] gpio/omap: Fix IRQ handling for SPARSE_IRQ
  2012-02-23 22:46 [RFC PATCH] gpio/omap: Fix IRQ handling for SPARSE_IRQ Cousson, Benoit
  2012-02-23 23:08 ` Tony Lindgren
@ 2012-02-24 14:14 ` Rob Herring
  2012-02-24 14:54   ` Cousson, Benoit
  1 sibling, 1 reply; 12+ messages in thread
From: Rob Herring @ 2012-02-24 14:14 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/23/2012 04:46 PM, Cousson, Benoit wrote:
> The GPIO driver is still relying on internal OMAP IRQ defines that
> are not relevant anymore if OMAP is built with SPARSE_IRQ.
> 
> Replace the defines with the proper IRQ base number.
> Clean some comment style issue.
> Remove some hidden and ugly cpu_class_is_omap1() inside the
> gpio header.
> 
> XXX: That fix might be broken for OMAP1 MPUIO case.
> 
> Signed-off-by: Benoit Cousson <b-cousson@ti.com>
> ---
> 
> Hi Tony,
> 
> Please note that this patch is still RFC, because I do not know how to fix properly the ugly cpu_class_is_omap1 and the dependency with IH_MPUIO_BASE to detect a MPUIO.
> 
> I'm still sending it, because it is needed to have SPARSE_IRQ working on OMAP4 with the previous series I've just sent.
> 
> Regards,
> Benoit
> 
>  arch/arm/plat-omap/include/plat/gpio.h |   22 ++------------------
>  drivers/gpio/gpio-omap.c               |   33 ++++++++++++++++---------------
>  2 files changed, 20 insertions(+), 35 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/include/plat/gpio.h b/arch/arm/plat-omap/include/plat/gpio.h
> index cb75b65..b8a96c6 100644
> --- a/arch/arm/plat-omap/include/plat/gpio.h
> +++ b/arch/arm/plat-omap/include/plat/gpio.h
> @@ -218,30 +218,14 @@ extern void omap_set_gpio_debounce(int gpio, int enable);
>  extern void omap_set_gpio_debounce_time(int gpio, int enable);
>  /*-------------------------------------------------------------------------*/
>  
> -/* Wrappers for "new style" GPIO calls, using the new infrastructure
> +/*
> + * Wrappers for "new style" GPIO calls, using the new infrastructure
>   * which lets us plug in FPGA, I2C, and other implementations.
> - * *
> + *
>   * The original OMAP-specific calls should eventually be removed.
>   */
>  
>  #include <linux/errno.h>
>  #include <asm-generic/gpio.h>
>  
> -static inline int irq_to_gpio(unsigned irq)
> -{
> -	int tmp;
> -
> -	/* omap1 SOC mpuio */
> -	if (cpu_class_is_omap1() && (irq < (IH_MPUIO_BASE + 16)))
> -		return (irq - IH_MPUIO_BASE) + OMAP_MAX_GPIO_LINES;
> -
> -	/* SOC gpio */
> -	tmp = irq - IH_GPIO_BASE;
> -	if (tmp < OMAP_MAX_GPIO_LINES)
> -		return tmp;
> -
> -	/* we don't supply reverse mappings for non-SOC gpios */
> -	return -EIO;
> -}
> -
>  #endif
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index bc2bd69..afef0f7 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -93,6 +93,11 @@ struct gpio_bank {
>  #define GPIO_BIT(bank, gpio) (1 << GPIO_INDEX(bank, gpio))
>  #define GPIO_MOD_CTRL_BIT	BIT(0)
>  
> +static int irq_to_gpio(struct gpio_bank *bank, unsigned int gpio_irq)
> +{
> +	return gpio_irq - bank->irq_base + bank->chip.base;

Ideally, you could do something like this when you have a domain setup:

irq_get_irq_data(gpio_irq)->hw_irq + bank->chip.base

Also, with sparse irq you need to have a call to irq_alloc_desc. You can
avoid that by setting NR_IRQS or machine .nr_irqs, but that needs to go
away.

Otherwise, it certainly is a step in the right direction.

Rob

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC PATCH] gpio/omap: Fix IRQ handling for SPARSE_IRQ
  2012-02-24 14:14 ` Rob Herring
@ 2012-02-24 14:54   ` Cousson, Benoit
  0 siblings, 0 replies; 12+ messages in thread
From: Cousson, Benoit @ 2012-02-24 14:54 UTC (permalink / raw)
  To: linux-arm-kernel

On 2/24/2012 3:14 PM, Rob Herring wrote:
> On 02/23/2012 04:46 PM, Cousson, Benoit wrote:
...
>> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
>> index bc2bd69..afef0f7 100644
>> --- a/drivers/gpio/gpio-omap.c
>> +++ b/drivers/gpio/gpio-omap.c
>> @@ -93,6 +93,11 @@ struct gpio_bank {
>>   #define GPIO_BIT(bank, gpio) (1<<  GPIO_INDEX(bank, gpio))
>>   #define GPIO_MOD_CTRL_BIT	BIT(0)
>>
>> +static int irq_to_gpio(struct gpio_bank *bank, unsigned int gpio_irq)
>> +{
>> +	return gpio_irq - bank->irq_base + bank->chip.base;
>
> Ideally, you could do something like this when you have a domain setup:
>
> irq_get_irq_data(gpio_irq)->hw_irq + bank->chip.base

OK, good to know. I still plan to move the current irq_domain basic 
support to the irqchip stuff you have done. I'll do that after 3.4.

> Also, with sparse irq you need to have a call to irq_alloc_desc.

irq_alloc_desc was already added as part of the DT migration patch.
It was not explicit in the changelog, but that patch is based on GPIO 
cleanup + DT migration series.

> You can avoid that by setting NR_IRQS or machine .nr_irqs, but that needs to go
> away.

Based on a recommendation you did in the commit to fix GIC I did that in 
the SPARSE_IRQ series I've just sent before that RFC patch:

+#ifdef CONFIG_SPARSE_IRQ
+#define NR_IRQS			NR_IRQS_LEGACY
+#else
  #define NR_IRQS			OMAP_GPMC_IRQ_END
+#endif

> Otherwise, it certainly is a step in the right direction.

Yeah, there is still a long way to clean all the old nasty IRQ stuff in 
OMAP :-)

Thanks,
Benoit

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC PATCH] gpio/omap: Fix IRQ handling for SPARSE_IRQ
  2012-02-24 13:56             ` Cousson, Benoit
@ 2012-02-24 15:09               ` DebBarma, Tarun Kanti
  2012-02-24 15:12                 ` Cousson, Benoit
  0 siblings, 1 reply; 12+ messages in thread
From: DebBarma, Tarun Kanti @ 2012-02-24 15:09 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Benoit,

On Fri, Feb 24, 2012 at 7:26 PM, Cousson, Benoit <b-cousson@ti.com> wrote:
> On 2/24/2012 2:53 PM, DebBarma, Tarun Kanti wrote:
>>
>> On Fri, Feb 24, 2012 at 7:02 PM, Cousson, Benoit<b-cousson@ti.com> ?wrote:
>>>
>>> On 2/24/2012 2:24 PM, DebBarma, Tarun Kanti wrote:
>>>>
>>>>
>>>> Hi Benoit,
>>>>
>>>> On Fri, Feb 24, 2012 at 4:07 PM, DebBarma, Tarun Kanti
>>>> <tarun.kanti@ti.com> ? ?wrote:
>>>>>
>>>>>
>>>>> On Fri, Feb 24, 2012 at 3:41 PM, Cousson, Benoit<b-cousson@ti.com>
>>>>> ?wrote:
>>>>>>
>>>>>>
>>>>>> + Tarun
>>>>>>
>>>>>>
>>>>>> On 2/24/2012 12:08 AM, Tony Lindgren wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> * Cousson, Benoit<b-cousson@ti.com> ? ? ?[120223 14:14]:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> The GPIO driver is still relying on internal OMAP IRQ defines that
>>>>>>>> are not relevant anymore if OMAP is built with SPARSE_IRQ.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Great :)
>>>>>>>
>>>>>>>> Please note that this patch is still RFC, because I do not know
>>>>>>>> how to fix properly the ugly cpu_class_is_omap1 and the dependency
>>>>>>>> with IH_MPUIO_BASE to detect a MPUIO.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Sounds like gpio_to_irq() needs to be set in the
>>>>>>> arch/arm/*omap*/gpio*.c then.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> In fact, after a second thought, that might even work for OMAP1
>>>>>> because
>>>>>> I'm
>>>>>> using the proper base (IRQ and GPIO) to convert the IRQ number.
>>>>>>
>>>>>>
>>>>>> static int irq_to_gpio(struct gpio_bank *bank, unsigned int gpio_irq)
>>>>>> {
>>>>>>
>>>>>> ? ? ? ?return gpio_irq - bank->irq_base + bank->chip.base;
>>>>>> }
>>>>>>
>>>>>> But it might be good to test it on OMAP1 platform.
>>>>>>
>>>>>>
>>>>>> Tarun,
>>>>>>
>>>>>> Do you have an OMAP1 board to test that.
>>>>>
>>>>>
>>>>> Yes, I will test on OMAP1 board.
>>>>
>>>>
>>>> I have booted the image on OMAP1 with following change.
>>>> I guess bank->irq_base was a typo?
>>>
>>>
>>>
>>> Not at all :-), it was done on purpose to get rid if the static IRQ
>>> definition.
>>
>> BTW, it was giving me following compilation error...
>> drivers/gpio/gpio-omap.c: In function 'irq_to_gpio':
>> drivers/gpio/gpio-omap.c:90: error: 'struct gpio_bank' has no member
>> named 'irq_base'
>> make[2]: *** [drivers/gpio/gpio-omap.o] Error 1
>
>
> Oh, that's different... you are missing the cleanup patches from the DT
> series I did before this one:
>
> eaabbb0 gpio/omap: Fix IRQ handling for SPARSE_IRQ
> fa560a7 arm/dts: OMAP3: Add gpio nodes
> bfeb298 arm/dts: OMAP4: Add gpio nodes
> 3062158 gpio/omap: Add DT support to GPIO driver
> a140d12 gpio/omap: Use devm_ API and add request_mem_region
> 0416689 gpio/omap: Remove bank->id information and misc cleanup
> 249f60c Merge branch 'for_3.4/gpio_cleanup_fixes_v9' of
> git://gitorious.org/~tarunkanti/omap-sw-deve
>
> I'll push a temp branch for you.

I have pulled from git://gitorious.org/omap-pm/linux.git for_3.4/dt_gpio_dt.
It is working on OMAP1710.
--
Tarun

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC PATCH] gpio/omap: Fix IRQ handling for SPARSE_IRQ
  2012-02-24 15:09               ` DebBarma, Tarun Kanti
@ 2012-02-24 15:12                 ` Cousson, Benoit
  0 siblings, 0 replies; 12+ messages in thread
From: Cousson, Benoit @ 2012-02-24 15:12 UTC (permalink / raw)
  To: linux-arm-kernel

On 2/24/2012 4:09 PM, DebBarma, Tarun Kanti wrote:
> Hi Benoit,
>
> On Fri, Feb 24, 2012 at 7:26 PM, Cousson, Benoit<b-cousson@ti.com>  wrote:
>> On 2/24/2012 2:53 PM, DebBarma, Tarun Kanti wrote:
>>>
>>> On Fri, Feb 24, 2012 at 7:02 PM, Cousson, Benoit<b-cousson@ti.com>    wrote:
>>>>
>>>> On 2/24/2012 2:24 PM, DebBarma, Tarun Kanti wrote:
>>>>>
>>>>>
>>>>> Hi Benoit,
>>>>>
>>>>> On Fri, Feb 24, 2012 at 4:07 PM, DebBarma, Tarun Kanti
>>>>> <tarun.kanti@ti.com>      wrote:
>>>>>>
>>>>>>
>>>>>> On Fri, Feb 24, 2012 at 3:41 PM, Cousson, Benoit<b-cousson@ti.com>
>>>>>>   wrote:
>>>>>>>
>>>>>>>
>>>>>>> + Tarun
>>>>>>>
>>>>>>>
>>>>>>> On 2/24/2012 12:08 AM, Tony Lindgren wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> * Cousson, Benoit<b-cousson@ti.com>        [120223 14:14]:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> The GPIO driver is still relying on internal OMAP IRQ defines that
>>>>>>>>> are not relevant anymore if OMAP is built with SPARSE_IRQ.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Great :)
>>>>>>>>
>>>>>>>>> Please note that this patch is still RFC, because I do not know
>>>>>>>>> how to fix properly the ugly cpu_class_is_omap1 and the dependency
>>>>>>>>> with IH_MPUIO_BASE to detect a MPUIO.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Sounds like gpio_to_irq() needs to be set in the
>>>>>>>> arch/arm/*omap*/gpio*.c then.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> In fact, after a second thought, that might even work for OMAP1
>>>>>>> because
>>>>>>> I'm
>>>>>>> using the proper base (IRQ and GPIO) to convert the IRQ number.
>>>>>>>
>>>>>>>
>>>>>>> static int irq_to_gpio(struct gpio_bank *bank, unsigned int gpio_irq)
>>>>>>> {
>>>>>>>
>>>>>>>         return gpio_irq - bank->irq_base + bank->chip.base;
>>>>>>> }
>>>>>>>
>>>>>>> But it might be good to test it on OMAP1 platform.
>>>>>>>
>>>>>>>
>>>>>>> Tarun,
>>>>>>>
>>>>>>> Do you have an OMAP1 board to test that.
>>>>>>
>>>>>>
>>>>>> Yes, I will test on OMAP1 board.
>>>>>
>>>>>
>>>>> I have booted the image on OMAP1 with following change.
>>>>> I guess bank->irq_base was a typo?
>>>>
>>>>
>>>>
>>>> Not at all :-), it was done on purpose to get rid if the static IRQ
>>>> definition.
>>>
>>> BTW, it was giving me following compilation error...
>>> drivers/gpio/gpio-omap.c: In function 'irq_to_gpio':
>>> drivers/gpio/gpio-omap.c:90: error: 'struct gpio_bank' has no member
>>> named 'irq_base'
>>> make[2]: *** [drivers/gpio/gpio-omap.o] Error 1
>>
>>
>> Oh, that's different... you are missing the cleanup patches from the DT
>> series I did before this one:
>>
>> eaabbb0 gpio/omap: Fix IRQ handling for SPARSE_IRQ
>> fa560a7 arm/dts: OMAP3: Add gpio nodes
>> bfeb298 arm/dts: OMAP4: Add gpio nodes
>> 3062158 gpio/omap: Add DT support to GPIO driver
>> a140d12 gpio/omap: Use devm_ API and add request_mem_region
>> 0416689 gpio/omap: Remove bank->id information and misc cleanup
>> 249f60c Merge branch 'for_3.4/gpio_cleanup_fixes_v9' of
>> git://gitorious.org/~tarunkanti/omap-sw-deve
>>
>> I'll push a temp branch for you.
>
> I have pulled from git://gitorious.org/omap-pm/linux.git for_3.4/dt_gpio_dt.
> It is working on OMAP1710.

Hehe, that's cool.

I'll repost the gpio DT series and add that one in it.

Thanks a lot.
Benoit

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2012-02-24 15:12 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-23 22:46 [RFC PATCH] gpio/omap: Fix IRQ handling for SPARSE_IRQ Cousson, Benoit
2012-02-23 23:08 ` Tony Lindgren
2012-02-24 10:11   ` Cousson, Benoit
2012-02-24 10:37     ` DebBarma, Tarun Kanti
2012-02-24 13:24       ` DebBarma, Tarun Kanti
2012-02-24 13:32         ` Cousson, Benoit
2012-02-24 13:53           ` DebBarma, Tarun Kanti
2012-02-24 13:56             ` Cousson, Benoit
2012-02-24 15:09               ` DebBarma, Tarun Kanti
2012-02-24 15:12                 ` Cousson, Benoit
2012-02-24 14:14 ` Rob Herring
2012-02-24 14:54   ` Cousson, Benoit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).