Re: [PATCH 15/15] OMAP: GPIO: clean omap_gpio_mod_init function

2011-06-03 Thread Kevin Hilman
"Varadarajan, Charulatha"  writes:

> Kevin,
>
> On Thu, May 26, 2011 at 05:18, Kevin Hilman  wrote:
>> Tarun Kanti DebBarma  writes:
>>
>>> From: Charulatha V 
>>>
>>> With register offsets now defined for respective OMAP versions
>>> we can get rid of cpu_class_* checks. In addition, organized
>>> common initialization for the different OMAP silicon versions.
>>>
>>> Signed-off-by: Charulatha V 
>>> Signed-off-by: Tarun Kanti DebBarma 
>>
>> The sysconfig stuff in this patch should be removed.  In fact, now that
>> hwmod is used to manage all the GPIO IP blocks, the driver should not be
>> touching sysconfig at all.
>
> The sysconfig stuff in this patch is for OMAP16XX.
>

Ah, OK.  Sorry I missed that.

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


Re: [PATCH 15/15] OMAP: GPIO: clean omap_gpio_mod_init function

2011-06-03 Thread Varadarajan, Charulatha
Kevin,

On Thu, May 26, 2011 at 05:18, Kevin Hilman  wrote:
> Tarun Kanti DebBarma  writes:
>
>> From: Charulatha V 
>>
>> With register offsets now defined for respective OMAP versions
>> we can get rid of cpu_class_* checks. In addition, organized
>> common initialization for the different OMAP silicon versions.
>>
>> Signed-off-by: Charulatha V 
>> Signed-off-by: Tarun Kanti DebBarma 
>
> The sysconfig stuff in this patch should be removed.  In fact, now that
> hwmod is used to manage all the GPIO IP blocks, the driver should not be
> touching sysconfig at all.

The sysconfig stuff in this patch is for OMAP16XX.

>
> The hwmod defaults should be enough, and for enabling wake-ups, the
> device-specific code should be calling omap_hwmod_enable_wakeup() (which
> will also enable smart-idle if the IP supports it.)
>
>
>> ---
>>  arch/arm/mach-omap1/gpio16xx.c         |    1 +
>>  arch/arm/plat-omap/include/plat/gpio.h |    1 +
>>  drivers/gpio/gpio_omap.c               |   74 
>> +---
>>  3 files changed, 32 insertions(+), 44 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap1/gpio16xx.c b/arch/arm/mach-omap1/gpio16xx.c
>> index 24f6cfa..e9f8abd 100644
>> --- a/arch/arm/mach-omap1/gpio16xx.c
>> +++ b/arch/arm/mach-omap1/gpio16xx.c
>> @@ -227,6 +227,7 @@ static int __init omap16xx_gpio_init(void)
>>       omap16xx_gpio_regs.wkupset = OMAP1610_GPIO_SET_WAKEUPENA;
>>       omap16xx_gpio_regs.edgectrl1 = OMAP1610_GPIO_EDGE_CTRL1;
>>       omap16xx_gpio_regs.edgectrl2 = OMAP1610_GPIO_EDGE_CTRL2;
>> +     omap16xx_gpio_regs.sysconfig = OMAP1610_GPIO_SYSCONFIG;
>>
>>       for (i = 0; i < ARRAY_SIZE(omap16xx_gpio_dev); i++)
>>               platform_device_register(omap16xx_gpio_dev[i]);
>> diff --git a/arch/arm/plat-omap/include/plat/gpio.h 
>> b/arch/arm/plat-omap/include/plat/gpio.h
>> index f82881c..ac45191 100644
>> --- a/arch/arm/plat-omap/include/plat/gpio.h
>> +++ b/arch/arm/plat-omap/include/plat/gpio.h
>> @@ -176,6 +176,7 @@ struct omap_gpio_dev_attr {
>>
>>  struct omap_gpio_reg_offs {
>>       u16 revision;
>> +     u16 sysconfig;
>>       u16 direction;
>>       u16 datain;
>>       u16 dataout;
>> diff --git a/drivers/gpio/gpio_omap.c b/drivers/gpio/gpio_omap.c
>> index ebeb16e..3649c74 100644
>> --- a/drivers/gpio/gpio_omap.c
>> +++ b/drivers/gpio/gpio_omap.c
>> @@ -885,65 +885,51 @@ static void __init omap_gpio_show_rev(struct gpio_bank 
>> *bank)
>>       called = true;
>>  }
>>
>> -/* This lock class tells lockdep that GPIO irqs are in a different
>> +/*
>> + * This lock class tells lockdep that GPIO irqs are in a different
>>   * category than their parents, so it won't report false recursion.
>>   */
>>  static struct lock_class_key gpio_lock_class;
>>
>> -/* TODO: Cleanup cpu_is_* checks */
>>  static void omap_gpio_mod_init(struct gpio_bank *bank)
>>  {
>> -     if (cpu_class_is_omap2()) {
>> -             if (cpu_is_omap44xx()) {
>> -                     __raw_writel(0x, bank->base +
>> -                                     OMAP4_GPIO_IRQSTATUSCLR0);
>> -                     __raw_writel(0x, bank->base +
>> -                                      OMAP4_GPIO_DEBOUNCENABLE);
>> -                     /* Initialize interface clk ungated, module enabled */
>> -                     __raw_writel(0, bank->base + OMAP4_GPIO_CTRL);
>> -             } else if (cpu_is_omap34xx()) {
>> -                     __raw_writel(0x, bank->base +
>> -                                     OMAP24XX_GPIO_IRQENABLE1);
>> -                     __raw_writel(0x, bank->base +
>> -                                     OMAP24XX_GPIO_IRQSTATUS1);
>> -                     __raw_writel(0x, bank->base +
>> -                                     OMAP24XX_GPIO_DEBOUNCE_EN);
>> +     if (bank->width == 32) {
>> +             u32 l = 0;
>> +
>> +             if (bank->regs->irqenable_inv)
>> +                     l = ~l;
>>
>> +             __raw_writel(l, bank->base + bank->regs->irqstatus);
>
> The ->irqenable_inv flag doesn't affect ->irqstatus.
>
>> +             __raw_writel(l, bank->base + bank->regs->irqenable);
>> +
>> +             if (bank->regs->debounce_en != USHRT_MAX)
>> +                     __raw_writel(l, bank->base + bank->regs->debounce_en);
>
> If ->irqenable_inv = true, debounce was just enabled for all GPIOs in
> the bank.
>
>> +             if (bank->regs->ctrl != USHRT_MAX)
>>                       /* Initialize interface clk ungated, module enabled */
>> -                     __raw_writel(0, bank->base + OMAP24XX_GPIO_CTRL);
>> -             }
>> -     } else if (cpu_class_is_omap1()) {
>> -             if (bank_is_mpuio(bank)) {
>> -                     __raw_writew(0x, bank->base +
>> -                             OMAP_MPUIO_GPIO_MASKIT / bank->stride);
>> +                      __raw_writel(l, bank->base + bank->regs->ctrl);
>
> If ->irqenable_env = true, all the clocks were just gated.

Okay. Will do the needful.

>
> Similar problems b

Re: [PATCH 15/15] OMAP: GPIO: clean omap_gpio_mod_init function

2011-05-25 Thread Kevin Hilman
Tarun Kanti DebBarma  writes:

> From: Charulatha V 
>
> With register offsets now defined for respective OMAP versions
> we can get rid of cpu_class_* checks. In addition, organized
> common initialization for the different OMAP silicon versions.
>
> Signed-off-by: Charulatha V 
> Signed-off-by: Tarun Kanti DebBarma 

The sysconfig stuff in this patch should be removed.  In fact, now that
hwmod is used to manage all the GPIO IP blocks, the driver should not be
touching sysconfig at all.

The hwmod defaults should be enough, and for enabling wake-ups, the
device-specific code should be calling omap_hwmod_enable_wakeup() (which
will also enable smart-idle if the IP supports it.)


> ---
>  arch/arm/mach-omap1/gpio16xx.c |1 +
>  arch/arm/plat-omap/include/plat/gpio.h |1 +
>  drivers/gpio/gpio_omap.c   |   74 
> +---
>  3 files changed, 32 insertions(+), 44 deletions(-)
>
> diff --git a/arch/arm/mach-omap1/gpio16xx.c b/arch/arm/mach-omap1/gpio16xx.c
> index 24f6cfa..e9f8abd 100644
> --- a/arch/arm/mach-omap1/gpio16xx.c
> +++ b/arch/arm/mach-omap1/gpio16xx.c
> @@ -227,6 +227,7 @@ static int __init omap16xx_gpio_init(void)
>   omap16xx_gpio_regs.wkupset = OMAP1610_GPIO_SET_WAKEUPENA;
>   omap16xx_gpio_regs.edgectrl1 = OMAP1610_GPIO_EDGE_CTRL1;
>   omap16xx_gpio_regs.edgectrl2 = OMAP1610_GPIO_EDGE_CTRL2;
> + omap16xx_gpio_regs.sysconfig = OMAP1610_GPIO_SYSCONFIG;
>  
>   for (i = 0; i < ARRAY_SIZE(omap16xx_gpio_dev); i++)
>   platform_device_register(omap16xx_gpio_dev[i]);
> diff --git a/arch/arm/plat-omap/include/plat/gpio.h 
> b/arch/arm/plat-omap/include/plat/gpio.h
> index f82881c..ac45191 100644
> --- a/arch/arm/plat-omap/include/plat/gpio.h
> +++ b/arch/arm/plat-omap/include/plat/gpio.h
> @@ -176,6 +176,7 @@ struct omap_gpio_dev_attr {
>  
>  struct omap_gpio_reg_offs {
>   u16 revision;
> + u16 sysconfig;
>   u16 direction;
>   u16 datain;
>   u16 dataout;
> diff --git a/drivers/gpio/gpio_omap.c b/drivers/gpio/gpio_omap.c
> index ebeb16e..3649c74 100644
> --- a/drivers/gpio/gpio_omap.c
> +++ b/drivers/gpio/gpio_omap.c
> @@ -885,65 +885,51 @@ static void __init omap_gpio_show_rev(struct gpio_bank 
> *bank)
>   called = true;
>  }
>  
> -/* This lock class tells lockdep that GPIO irqs are in a different
> +/*
> + * This lock class tells lockdep that GPIO irqs are in a different
>   * category than their parents, so it won't report false recursion.
>   */
>  static struct lock_class_key gpio_lock_class;
>  
> -/* TODO: Cleanup cpu_is_* checks */
>  static void omap_gpio_mod_init(struct gpio_bank *bank)
>  {
> - if (cpu_class_is_omap2()) {
> - if (cpu_is_omap44xx()) {
> - __raw_writel(0x, bank->base +
> - OMAP4_GPIO_IRQSTATUSCLR0);
> - __raw_writel(0x, bank->base +
> -  OMAP4_GPIO_DEBOUNCENABLE);
> - /* Initialize interface clk ungated, module enabled */
> - __raw_writel(0, bank->base + OMAP4_GPIO_CTRL);
> - } else if (cpu_is_omap34xx()) {
> - __raw_writel(0x, bank->base +
> - OMAP24XX_GPIO_IRQENABLE1);
> - __raw_writel(0x, bank->base +
> - OMAP24XX_GPIO_IRQSTATUS1);
> - __raw_writel(0x, bank->base +
> - OMAP24XX_GPIO_DEBOUNCE_EN);
> + if (bank->width == 32) {
> + u32 l = 0;
> +
> + if (bank->regs->irqenable_inv)
> + l = ~l;
>  
> + __raw_writel(l, bank->base + bank->regs->irqstatus);

The ->irqenable_inv flag doesn't affect ->irqstatus.

> + __raw_writel(l, bank->base + bank->regs->irqenable);
> +
> + if (bank->regs->debounce_en != USHRT_MAX)
> + __raw_writel(l, bank->base + bank->regs->debounce_en);

If ->irqenable_inv = true, debounce was just enabled for all GPIOs in
the bank.

> + if (bank->regs->ctrl != USHRT_MAX)
>   /* Initialize interface clk ungated, module enabled */
> - __raw_writel(0, bank->base + OMAP24XX_GPIO_CTRL);
> - }
> - } else if (cpu_class_is_omap1()) {
> - if (bank_is_mpuio(bank)) {
> - __raw_writew(0x, bank->base +
> - OMAP_MPUIO_GPIO_MASKIT / bank->stride);
> +  __raw_writel(l, bank->base + bank->regs->ctrl);

If ->irqenable_env = true, all the clocks were just gated.

Similar problems below.

Kevin
> +
> + } else if (bank->width == 16) {
> + u16 l = 0;
> +
> + if (bank_is_mpuio(bank))
>   mpuio_init(bank);
> - }
> - if (cpu_is_omap15xx() && bank->method == METHOD_GPIO_151

[PATCH 15/15] OMAP: GPIO: clean omap_gpio_mod_init function

2011-05-24 Thread Tarun Kanti DebBarma
From: Charulatha V 

With register offsets now defined for respective OMAP versions
we can get rid of cpu_class_* checks. In addition, organized
common initialization for the different OMAP silicon versions.

Signed-off-by: Charulatha V 
Signed-off-by: Tarun Kanti DebBarma 
---
 arch/arm/mach-omap1/gpio16xx.c |1 +
 arch/arm/plat-omap/include/plat/gpio.h |1 +
 drivers/gpio/gpio_omap.c   |   74 +---
 3 files changed, 32 insertions(+), 44 deletions(-)

diff --git a/arch/arm/mach-omap1/gpio16xx.c b/arch/arm/mach-omap1/gpio16xx.c
index 24f6cfa..e9f8abd 100644
--- a/arch/arm/mach-omap1/gpio16xx.c
+++ b/arch/arm/mach-omap1/gpio16xx.c
@@ -227,6 +227,7 @@ static int __init omap16xx_gpio_init(void)
omap16xx_gpio_regs.wkupset = OMAP1610_GPIO_SET_WAKEUPENA;
omap16xx_gpio_regs.edgectrl1 = OMAP1610_GPIO_EDGE_CTRL1;
omap16xx_gpio_regs.edgectrl2 = OMAP1610_GPIO_EDGE_CTRL2;
+   omap16xx_gpio_regs.sysconfig = OMAP1610_GPIO_SYSCONFIG;
 
for (i = 0; i < ARRAY_SIZE(omap16xx_gpio_dev); i++)
platform_device_register(omap16xx_gpio_dev[i]);
diff --git a/arch/arm/plat-omap/include/plat/gpio.h 
b/arch/arm/plat-omap/include/plat/gpio.h
index f82881c..ac45191 100644
--- a/arch/arm/plat-omap/include/plat/gpio.h
+++ b/arch/arm/plat-omap/include/plat/gpio.h
@@ -176,6 +176,7 @@ struct omap_gpio_dev_attr {
 
 struct omap_gpio_reg_offs {
u16 revision;
+   u16 sysconfig;
u16 direction;
u16 datain;
u16 dataout;
diff --git a/drivers/gpio/gpio_omap.c b/drivers/gpio/gpio_omap.c
index ebeb16e..3649c74 100644
--- a/drivers/gpio/gpio_omap.c
+++ b/drivers/gpio/gpio_omap.c
@@ -885,65 +885,51 @@ static void __init omap_gpio_show_rev(struct gpio_bank 
*bank)
called = true;
 }
 
-/* This lock class tells lockdep that GPIO irqs are in a different
+/*
+ * This lock class tells lockdep that GPIO irqs are in a different
  * category than their parents, so it won't report false recursion.
  */
 static struct lock_class_key gpio_lock_class;
 
-/* TODO: Cleanup cpu_is_* checks */
 static void omap_gpio_mod_init(struct gpio_bank *bank)
 {
-   if (cpu_class_is_omap2()) {
-   if (cpu_is_omap44xx()) {
-   __raw_writel(0x, bank->base +
-   OMAP4_GPIO_IRQSTATUSCLR0);
-   __raw_writel(0x, bank->base +
-OMAP4_GPIO_DEBOUNCENABLE);
-   /* Initialize interface clk ungated, module enabled */
-   __raw_writel(0, bank->base + OMAP4_GPIO_CTRL);
-   } else if (cpu_is_omap34xx()) {
-   __raw_writel(0x, bank->base +
-   OMAP24XX_GPIO_IRQENABLE1);
-   __raw_writel(0x, bank->base +
-   OMAP24XX_GPIO_IRQSTATUS1);
-   __raw_writel(0x, bank->base +
-   OMAP24XX_GPIO_DEBOUNCE_EN);
+   if (bank->width == 32) {
+   u32 l = 0;
+
+   if (bank->regs->irqenable_inv)
+   l = ~l;
 
+   __raw_writel(l, bank->base + bank->regs->irqstatus);
+   __raw_writel(l, bank->base + bank->regs->irqenable);
+
+   if (bank->regs->debounce_en != USHRT_MAX)
+   __raw_writel(l, bank->base + bank->regs->debounce_en);
+
+   if (bank->regs->ctrl != USHRT_MAX)
/* Initialize interface clk ungated, module enabled */
-   __raw_writel(0, bank->base + OMAP24XX_GPIO_CTRL);
-   }
-   } else if (cpu_class_is_omap1()) {
-   if (bank_is_mpuio(bank)) {
-   __raw_writew(0x, bank->base +
-   OMAP_MPUIO_GPIO_MASKIT / bank->stride);
+__raw_writel(l, bank->base + bank->regs->ctrl);
+
+   } else if (bank->width == 16) {
+   u16 l = 0;
+
+   if (bank_is_mpuio(bank))
mpuio_init(bank);
-   }
-   if (cpu_is_omap15xx() && bank->method == METHOD_GPIO_1510) {
-   __raw_writew(0x, bank->base
-   + OMAP1510_GPIO_INT_MASK);
-   __raw_writew(0x, bank->base
-   + OMAP1510_GPIO_INT_STATUS);
-   }
-   if (cpu_is_omap16xx() && bank->method == METHOD_GPIO_1610) {
-   __raw_writew(0x, bank->base
-   + OMAP1610_GPIO_IRQENABLE1);
-   __raw_writew(0x, bank->base
-   + OMAP1610_GPIO_IRQSTATUS1);
-   __raw_writew(0x0014, bank->base
-