Re: [PATCH 15/15] OMAP: GPIO: cleanup show revision, remove cpu_is checks, display only once
Kevin Hilman writes:
> "Varadarajan, Charulatha" writes:
>
>> Kevin,
>>
>> On Sat, Apr 23, 2011 at 04:32, Kevin Hilman wrote:
>>> Remove cpu_is_* checks from gpio_show_revision() by passing in the
>>> revision address offset from platform data. SoCs with no revision
>>> register (15xx, 7xx, and all MPUIOs) use -1 to signify no register.
>>>
>>> While here, all GPIO banks are assumed to be the same revision, so fix
>>> show_revision() to only show the revision for the first bank it finds.
>>> This removes duplicate GPIO revision prints during boot.
>>>
>>> Signed-off-by: Kevin Hilman
>>> ---
>>> arch/arm/mach-omap1/gpio15xx.c | 2 ++
>>> arch/arm/mach-omap1/gpio16xx.c | 2 ++
>>> arch/arm/mach-omap1/gpio7xx.c | 2 ++
>>> arch/arm/mach-omap2/gpio.c | 2 ++
>>> arch/arm/plat-omap/gpio.c | 14 ++
>>> arch/arm/plat-omap/include/plat/gpio.h | 1 +
>>> 6 files changed, 15 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap1/gpio15xx.c b/arch/arm/mach-omap1/gpio15xx.c
>>> index 9175624..6f77c36 100644
>>> --- a/arch/arm/mach-omap1/gpio15xx.c
>>> +++ b/arch/arm/mach-omap1/gpio15xx.c
>>> @@ -35,6 +35,7 @@ static struct __initdata resource
>>> omap15xx_mpu_gpio_resources[] = {
>>> };
>>>
>>> static struct omap_gpio_reg_offs omap15xx_mpuio_regs = {
>>> + .revision = -1,
>>
>> Assigning -1 to u16 type. Instead you may want to use 0x?
>>
>
> The compiler will do the right thing, so personally, I prefer using -1.
> It's safer if/when the type is changed, but the mask not updated.
Actually, you're right here.
While the compiler does the "right thing" for the assignment, it was not
doing the right thing for the comparision in the driver for the revision
check, and thus trying to read from offset 0x on OMAP1 for MPUIO
banks (that's probably the reason for a boot hang for you on 17xx.)
At least for me, with this change it's booting on OMAP1 (omap5912/OSK
for me.)
I'll change the usage of -1 here to USHRT_MAX.
Thanks,
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: cleanup show revision, remove cpu_is checks, display only once
"Varadarajan, Charulatha" writes:
> Kevin,
>
> On Sat, Apr 23, 2011 at 04:32, Kevin Hilman wrote:
>> Remove cpu_is_* checks from gpio_show_revision() by passing in the
>> revision address offset from platform data. SoCs with no revision
>> register (15xx, 7xx, and all MPUIOs) use -1 to signify no register.
>>
>> While here, all GPIO banks are assumed to be the same revision, so fix
>> show_revision() to only show the revision for the first bank it finds.
>> This removes duplicate GPIO revision prints during boot.
>>
>> Signed-off-by: Kevin Hilman
>> ---
>> arch/arm/mach-omap1/gpio15xx.c | 2 ++
>> arch/arm/mach-omap1/gpio16xx.c | 2 ++
>> arch/arm/mach-omap1/gpio7xx.c | 2 ++
>> arch/arm/mach-omap2/gpio.c | 2 ++
>> arch/arm/plat-omap/gpio.c | 14 ++
>> arch/arm/plat-omap/include/plat/gpio.h | 1 +
>> 6 files changed, 15 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap1/gpio15xx.c b/arch/arm/mach-omap1/gpio15xx.c
>> index 9175624..6f77c36 100644
>> --- a/arch/arm/mach-omap1/gpio15xx.c
>> +++ b/arch/arm/mach-omap1/gpio15xx.c
>> @@ -35,6 +35,7 @@ static struct __initdata resource
>> omap15xx_mpu_gpio_resources[] = {
>> };
>>
>> static struct omap_gpio_reg_offs omap15xx_mpuio_regs = {
>> + .revision = -1,
>
> Assigning -1 to u16 type. Instead you may want to use 0x?
>
The compiler will do the right thing, so personally, I prefer using -1.
It's safer if/when the type is changed, but the mask not updated.
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: cleanup show revision, remove cpu_is checks, display only once
Kevin,
On Sat, Apr 23, 2011 at 04:32, Kevin Hilman wrote:
> Remove cpu_is_* checks from gpio_show_revision() by passing in the
> revision address offset from platform data. SoCs with no revision
> register (15xx, 7xx, and all MPUIOs) use -1 to signify no register.
>
> While here, all GPIO banks are assumed to be the same revision, so fix
> show_revision() to only show the revision for the first bank it finds.
> This removes duplicate GPIO revision prints during boot.
>
> Signed-off-by: Kevin Hilman
> ---
> arch/arm/mach-omap1/gpio15xx.c | 2 ++
> arch/arm/mach-omap1/gpio16xx.c | 2 ++
> arch/arm/mach-omap1/gpio7xx.c | 2 ++
> arch/arm/mach-omap2/gpio.c | 2 ++
> arch/arm/plat-omap/gpio.c | 14 ++
> arch/arm/plat-omap/include/plat/gpio.h | 1 +
> 6 files changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm/mach-omap1/gpio15xx.c b/arch/arm/mach-omap1/gpio15xx.c
> index 9175624..6f77c36 100644
> --- a/arch/arm/mach-omap1/gpio15xx.c
> +++ b/arch/arm/mach-omap1/gpio15xx.c
> @@ -35,6 +35,7 @@ static struct __initdata resource
> omap15xx_mpu_gpio_resources[] = {
> };
>
> static struct omap_gpio_reg_offs omap15xx_mpuio_regs = {
> + .revision = -1,
Assigning -1 to u16 type. Instead you may want to use 0x?
> .direction = OMAP_MPUIO_IO_CNTL,
> .datain = OMAP_MPUIO_INPUT_LATCH,
> .dataout = OMAP_MPUIO_OUTPUT,
> @@ -75,6 +76,7 @@ static struct __initdata resource omap15xx_gpio_resources[]
> = {
> };
<>
> diff --git a/arch/arm/plat-omap/include/plat/gpio.h
> b/arch/arm/plat-omap/include/plat/gpio.h
> index 7a3f067..91e8de3 100644
> --- a/arch/arm/plat-omap/include/plat/gpio.h
> +++ b/arch/arm/plat-omap/include/plat/gpio.h
> @@ -175,6 +175,7 @@ struct omap_gpio_dev_attr {
> };
>
> struct omap_gpio_reg_offs {
> + u16 revision;
> u16 direction;
> u16 datain;
> u16 dataout;
-V Charulatha
--
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
[PATCH 15/15] OMAP: GPIO: cleanup show revision, remove cpu_is checks, display only once
Remove cpu_is_* checks from gpio_show_revision() by passing in the
revision address offset from platform data. SoCs with no revision
register (15xx, 7xx, and all MPUIOs) use -1 to signify no register.
While here, all GPIO banks are assumed to be the same revision, so fix
show_revision() to only show the revision for the first bank it finds.
This removes duplicate GPIO revision prints during boot.
Signed-off-by: Kevin Hilman
---
arch/arm/mach-omap1/gpio15xx.c |2 ++
arch/arm/mach-omap1/gpio16xx.c |2 ++
arch/arm/mach-omap1/gpio7xx.c |2 ++
arch/arm/mach-omap2/gpio.c |2 ++
arch/arm/plat-omap/gpio.c | 14 ++
arch/arm/plat-omap/include/plat/gpio.h |1 +
6 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-omap1/gpio15xx.c b/arch/arm/mach-omap1/gpio15xx.c
index 9175624..6f77c36 100644
--- a/arch/arm/mach-omap1/gpio15xx.c
+++ b/arch/arm/mach-omap1/gpio15xx.c
@@ -35,6 +35,7 @@ static struct __initdata resource
omap15xx_mpu_gpio_resources[] = {
};
static struct omap_gpio_reg_offs omap15xx_mpuio_regs = {
+ .revision = -1,
.direction = OMAP_MPUIO_IO_CNTL,
.datain = OMAP_MPUIO_INPUT_LATCH,
.dataout= OMAP_MPUIO_OUTPUT,
@@ -75,6 +76,7 @@ static struct __initdata resource omap15xx_gpio_resources[] =
{
};
static struct omap_gpio_reg_offs omap15xx_gpio_regs = {
+ .revision = -1,
.direction = OMAP1510_GPIO_DIR_CONTROL,
.datain = OMAP1510_GPIO_DATA_INPUT,
.dataout= OMAP1510_GPIO_DATA_OUTPUT,
diff --git a/arch/arm/mach-omap1/gpio16xx.c b/arch/arm/mach-omap1/gpio16xx.c
index a6d4a71..6cbfc70 100644
--- a/arch/arm/mach-omap1/gpio16xx.c
+++ b/arch/arm/mach-omap1/gpio16xx.c
@@ -38,6 +38,7 @@ static struct __initdata resource
omap16xx_mpu_gpio_resources[] = {
};
static struct omap_gpio_reg_offs omap16xx_mpuio_regs = {
+ .revision = -1,
.direction = OMAP_MPUIO_IO_CNTL,
.datain = OMAP_MPUIO_INPUT_LATCH,
.dataout= OMAP_MPUIO_OUTPUT,
@@ -78,6 +79,7 @@ static struct __initdata resource omap16xx_gpio1_resources[]
= {
};
static struct omap_gpio_reg_offs omap16xx_gpio_regs = {
+ .revision = OMAP1610_GPIO_REVISION,
.direction = OMAP1610_GPIO_DIRECTION,
.set_dataout= OMAP1610_GPIO_SET_DATAOUT,
.clr_dataout= OMAP1610_GPIO_CLEAR_DATAOUT,
diff --git a/arch/arm/mach-omap1/gpio7xx.c b/arch/arm/mach-omap1/gpio7xx.c
index 880f3cb..293a22e 100644
--- a/arch/arm/mach-omap1/gpio7xx.c
+++ b/arch/arm/mach-omap1/gpio7xx.c
@@ -40,6 +40,7 @@ static struct __initdata resource
omap7xx_mpu_gpio_resources[] = {
};
static struct omap_gpio_reg_offs omap7xx_mpuio_regs = {
+ .revision = -1,
.direction = OMAP_MPUIO_IO_CNTL / 2,
.datain = OMAP_MPUIO_INPUT_LATCH / 2,
.dataout= OMAP_MPUIO_OUTPUT / 2,
@@ -80,6 +81,7 @@ static struct __initdata resource omap7xx_gpio1_resources[] =
{
};
static struct omap_gpio_reg_offs omap7xx_gpio_regs = {
+ .revision = -1,
.direction = OMAP7XX_GPIO_DIR_CONTROL,
.datain = OMAP7XX_GPIO_DATA_INPUT,
.dataout= OMAP7XX_GPIO_DATA_OUTPUT,
diff --git a/arch/arm/mach-omap2/gpio.c b/arch/arm/mach-omap2/gpio.c
index 316d930..9a46d77 100644
--- a/arch/arm/mach-omap2/gpio.c
+++ b/arch/arm/mach-omap2/gpio.c
@@ -71,6 +71,7 @@ static int omap2_gpio_dev_init(struct omap_hwmod *oh, void
*unused)
case 0:
case 1:
pdata->bank_type = METHOD_GPIO_24XX;
+ pdata->regs->revision = OMAP24XX_GPIO_REVISION;
pdata->regs->direction = OMAP24XX_GPIO_OE;
pdata->regs->datain = OMAP24XX_GPIO_DATAIN;
pdata->regs->dataout = OMAP24XX_GPIO_DATAOUT;
@@ -86,6 +87,7 @@ static int omap2_gpio_dev_init(struct omap_hwmod *oh, void
*unused)
break;
case 2:
pdata->bank_type = METHOD_GPIO_44XX;
+ pdata->regs->revision = OMAP4_GPIO_REVISION;
pdata->regs->direction = OMAP4_GPIO_OE;
pdata->regs->datain = OMAP4_GPIO_DATAIN;
pdata->regs->dataout = OMAP4_GPIO_DATAOUT;
diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c
index 17f29cc..f88616e 100644
--- a/arch/arm/plat-omap/gpio.c
+++ b/arch/arm/plat-omap/gpio.c
@@ -985,19 +985,17 @@ static int gpio_2irq(struct gpio_chip *chip, unsigned
offset)
static void __init omap_gpio_show_rev(struct gpio_bank *bank)
{
+ static bool called;
u32 rev;
- if (cpu_is_omap16xx() && !(bank->method != METHOD_MPUIO))
- rev = __raw_readw(bank->base + OMAP1610_GPIO_REVISION);
- else if (cpu_is_omap24xx() || cpu_is_omap34xx())
- rev = __raw_readl(bank->base + OMAP24XX_GPIO_REVISION);
- else if (cpu_is_omap44xx())
-
