[PATCH] davinci: tnetv107x: fix register indexing for GPIOs numbers > 31

[PATCH] davinci: tnetv107x: fix register indexing for GPIOs numbers > 31

[Hirosh Dabui]

Changelog:

This patch fix a bug in the register indexing for GPIOs numbers >  31
to get the relevant hardware registers of tnetv107x to control the GPIOs.

In the structure tnetv107x_gpio_regs:

struct tnetv107x_gpio_regs {
            u32     idver;
            u32     data_in[3];
            u32     data_out[3];
            u32     direction[3];
            u32     enable[3];
};

The GPIO hardware register addresses of tnetv107x are stored.
The chip implements 3 registers of each entity to serve 96 GPIOs,
each register provides a subset of 32 GPIOs.
The driver provides these macros: gpio_reg_set_bit, gpio_reg_get_bit
and gpio_reg_clear_bit.

The bug implied the use of macros to access the relevant hardware
register e.g. the driver code used the macro like this:
'gpio_reg_clear_bit(&reg->data_out, gpio)'

But it has to be used like this:
'gpio_reg_clear_bit(reg->data_out, gpio)'.

The different results are shown here:
- &reg->data_out + 1 (it will add the full array size of data_out i.e. 12 bytes)
- reg->data_out + 1 (it will increment only the size of data_out i.e. only 4 bytes)

Signed-off-by: Hirosh Dabui <hirosh.dabui@snom.com>
---
 arch/arm/mach-davinci/gpio-tnetv107x.c |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-davinci/gpio-tnetv107x.c b/arch/arm/mach-davinci/gpio-tnetv107x.c
index d102986..3fa3e28 100644
--- a/arch/arm/mach-davinci/gpio-tnetv107x.c
+++ b/arch/arm/mach-davinci/gpio-tnetv107x.c
@@ -58,7 +58,7 @@ static int tnetv107x_gpio_request(struct gpio_chip *chip, unsigned offset)
 
 	spin_lock_irqsave(&ctlr->lock, flags);
 
-	gpio_reg_set_bit(&regs->enable, gpio);
+	gpio_reg_set_bit(regs->enable, gpio);
 
 	spin_unlock_irqrestore(&ctlr->lock, flags);
 
@@ -74,7 +74,7 @@ static void tnetv107x_gpio_free(struct gpio_chip *chip, unsigned offset)
 
 	spin_lock_irqsave(&ctlr->lock, flags);
 
-	gpio_reg_clear_bit(&regs->enable, gpio);
+	gpio_reg_clear_bit(regs->enable, gpio);
 
 	spin_unlock_irqrestore(&ctlr->lock, flags);
 }
@@ -88,7 +88,7 @@ static int tnetv107x_gpio_dir_in(struct gpio_chip *chip, unsigned offset)
 
 	spin_lock_irqsave(&ctlr->lock, flags);
 
-	gpio_reg_set_bit(&regs->direction, gpio);
+	gpio_reg_set_bit(regs->direction, gpio);
 
 	spin_unlock_irqrestore(&ctlr->lock, flags);
 
@@ -106,11 +106,11 @@ static int tnetv107x_gpio_dir_out(struct gpio_chip *chip,
 	spin_lock_irqsave(&ctlr->lock, flags);
 
 	if (value)
-		gpio_reg_set_bit(&regs->data_out, gpio);
+		gpio_reg_set_bit(regs->data_out, gpio);
 	else
-		gpio_reg_clear_bit(&regs->data_out, gpio);
+		gpio_reg_clear_bit(regs->data_out, gpio);
 
-	gpio_reg_clear_bit(&regs->direction, gpio);
+	gpio_reg_clear_bit(regs->direction, gpio);
 
 	spin_unlock_irqrestore(&ctlr->lock, flags);
 
@@ -124,7 +124,7 @@ static int tnetv107x_gpio_get(struct gpio_chip *chip, unsigned offset)
 	unsigned gpio = chip->base + offset;
 	int ret;
 
-	ret = gpio_reg_get_bit(&regs->data_in, gpio);
+	ret = gpio_reg_get_bit(regs->data_in, gpio);
 
 	return ret ? 1 : 0;
 }
@@ -140,9 +140,9 @@ static void tnetv107x_gpio_set(struct gpio_chip *chip,
 	spin_lock_irqsave(&ctlr->lock, flags);
 
 	if (value)
-		gpio_reg_set_bit(&regs->data_out, gpio);
+		gpio_reg_set_bit(regs->data_out, gpio);
 	else
-		gpio_reg_clear_bit(&regs->data_out, gpio);
+		gpio_reg_clear_bit(regs->data_out, gpio);
 
 	spin_unlock_irqrestore(&ctlr->lock, flags);
 }
-- 
1.7.1

[PATCH] davinci: tnetv107x: fix register indexing for GPIOs numbers > 31

[Nori, Sekhar]

On Fri, Jan 14, 2011 at 20:48:13, Hirosh Dabui wrote:
> Changelog:
> 
> This patch fix a bug in the register indexing for GPIOs numbers >  31
> to get the relevant hardware registers of tnetv107x to control the GPIOs.
> 
> In the structure tnetv107x_gpio_regs:
> 
> struct tnetv107x_gpio_regs {
>             u32     idver;
>             u32     data_in[3];
>             u32     data_out[3];
>             u32     direction[3];
>             u32     enable[3];
> };
> 
> The GPIO hardware register addresses of tnetv107x are stored.
> The chip implements 3 registers of each entity to serve 96 GPIOs,
> each register provides a subset of 32 GPIOs.
> The driver provides these macros: gpio_reg_set_bit, gpio_reg_get_bit
> and gpio_reg_clear_bit.
> 
> The bug implied the use of macros to access the relevant hardware
> register e.g. the driver code used the macro like this:
> 'gpio_reg_clear_bit(&reg->data_out, gpio)'
> 
> But it has to be used like this:
> 'gpio_reg_clear_bit(reg->data_out, gpio)'.
> 
> The different results are shown here:
> - &reg->data_out + 1 (it will add the full array size of data_out i.e. 12 bytes)
> - reg->data_out + 1 (it will increment only the size of data_out i.e. only 4 bytes)
> 
> Signed-off-by: Hirosh Dabui <hirosh.dabui@snom.com>

Cyril's ack was missed:

Acked-by: Cyril Chemparathy <cyril@ti.com>

Thanks,
Sekhar

> ---
>  arch/arm/mach-davinci/gpio-tnetv107x.c |   18 +++++++++---------
>  1 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/mach-davinci/gpio-tnetv107x.c b/arch/arm/mach-davinci/gpio-tnetv107x.c
> index d102986..3fa3e28 100644
> --- a/arch/arm/mach-davinci/gpio-tnetv107x.c
> +++ b/arch/arm/mach-davinci/gpio-tnetv107x.c
> @@ -58,7 +58,7 @@ static int tnetv107x_gpio_request(struct gpio_chip *chip, unsigned offset)
>  
>  	spin_lock_irqsave(&ctlr->lock, flags);
>  
> -	gpio_reg_set_bit(&regs->enable, gpio);
> +	gpio_reg_set_bit(regs->enable, gpio);
>  
>  	spin_unlock_irqrestore(&ctlr->lock, flags);
>  
> @@ -74,7 +74,7 @@ static void tnetv107x_gpio_free(struct gpio_chip *chip, unsigned offset)
>  
>  	spin_lock_irqsave(&ctlr->lock, flags);
>  
> -	gpio_reg_clear_bit(&regs->enable, gpio);
> +	gpio_reg_clear_bit(regs->enable, gpio);
>  
>  	spin_unlock_irqrestore(&ctlr->lock, flags);
>  }
> @@ -88,7 +88,7 @@ static int tnetv107x_gpio_dir_in(struct gpio_chip *chip, unsigned offset)
>  
>  	spin_lock_irqsave(&ctlr->lock, flags);
>  
> -	gpio_reg_set_bit(&regs->direction, gpio);
> +	gpio_reg_set_bit(regs->direction, gpio);
>  
>  	spin_unlock_irqrestore(&ctlr->lock, flags);
>  
> @@ -106,11 +106,11 @@ static int tnetv107x_gpio_dir_out(struct gpio_chip *chip,
>  	spin_lock_irqsave(&ctlr->lock, flags);
>  
>  	if (value)
> -		gpio_reg_set_bit(&regs->data_out, gpio);
> +		gpio_reg_set_bit(regs->data_out, gpio);
>  	else
> -		gpio_reg_clear_bit(&regs->data_out, gpio);
> +		gpio_reg_clear_bit(regs->data_out, gpio);
>  
> -	gpio_reg_clear_bit(&regs->direction, gpio);
> +	gpio_reg_clear_bit(regs->direction, gpio);
>  
>  	spin_unlock_irqrestore(&ctlr->lock, flags);
>  
> @@ -124,7 +124,7 @@ static int tnetv107x_gpio_get(struct gpio_chip *chip, unsigned offset)
>  	unsigned gpio = chip->base + offset;
>  	int ret;
>  
> -	ret = gpio_reg_get_bit(&regs->data_in, gpio);
> +	ret = gpio_reg_get_bit(regs->data_in, gpio);
>  
>  	return ret ? 1 : 0;
>  }
> @@ -140,9 +140,9 @@ static void tnetv107x_gpio_set(struct gpio_chip *chip,
>  	spin_lock_irqsave(&ctlr->lock, flags);
>  
>  	if (value)
> -		gpio_reg_set_bit(&regs->data_out, gpio);
> +		gpio_reg_set_bit(regs->data_out, gpio);
>  	else
> -		gpio_reg_clear_bit(&regs->data_out, gpio);
> +		gpio_reg_clear_bit(regs->data_out, gpio);
>  
>  	spin_unlock_irqrestore(&ctlr->lock, flags);
>  }
> -- 
> 1.7.1
> _______________________________________________
> Davinci-linux-open-source mailing list
> Davinci-linux-open-source at linux.davincidsp.com
> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
> 

[PATCH] davinci: tnetv107x: fix register indexing for GPIOs numbers > 31

[Kevin Hilman]

Hirosh Dabui <hirosh.dabui@snom.com> writes:

> Changelog:

This isn't needed.

Please repost one more time without this and include Cyril's ack please.

Thanks,

Kevin

> This patch fix a bug in the register indexing for GPIOs numbers >  31
> to get the relevant hardware registers of tnetv107x to control the GPIOs.
>
> In the structure tnetv107x_gpio_regs:
>
> struct tnetv107x_gpio_regs {
>             u32     idver;
>             u32     data_in[3];
>             u32     data_out[3];
>             u32     direction[3];
>             u32     enable[3];
> };
>
> The GPIO hardware register addresses of tnetv107x are stored.
> The chip implements 3 registers of each entity to serve 96 GPIOs,
> each register provides a subset of 32 GPIOs.
> The driver provides these macros: gpio_reg_set_bit, gpio_reg_get_bit
> and gpio_reg_clear_bit.
>
> The bug implied the use of macros to access the relevant hardware
> register e.g. the driver code used the macro like this:
> 'gpio_reg_clear_bit(&reg->data_out, gpio)'
>
> But it has to be used like this:
> 'gpio_reg_clear_bit(reg->data_out, gpio)'.
>
> The different results are shown here:
> - &reg->data_out + 1 (it will add the full array size of data_out i.e. 12 bytes)
> - reg->data_out + 1 (it will increment only the size of data_out i.e. only 4 bytes)
>
> Signed-off-by: Hirosh Dabui <hirosh.dabui@snom.com>
> ---
>  arch/arm/mach-davinci/gpio-tnetv107x.c |   18 +++++++++---------
>  1 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/gpio-tnetv107x.c b/arch/arm/mach-davinci/gpio-tnetv107x.c
> index d102986..3fa3e28 100644
> --- a/arch/arm/mach-davinci/gpio-tnetv107x.c
> +++ b/arch/arm/mach-davinci/gpio-tnetv107x.c
> @@ -58,7 +58,7 @@ static int tnetv107x_gpio_request(struct gpio_chip *chip, unsigned offset)
>  
>  	spin_lock_irqsave(&ctlr->lock, flags);
>  
> -	gpio_reg_set_bit(&regs->enable, gpio);
> +	gpio_reg_set_bit(regs->enable, gpio);
>  
>  	spin_unlock_irqrestore(&ctlr->lock, flags);
>  
> @@ -74,7 +74,7 @@ static void tnetv107x_gpio_free(struct gpio_chip *chip, unsigned offset)
>  
>  	spin_lock_irqsave(&ctlr->lock, flags);
>  
> -	gpio_reg_clear_bit(&regs->enable, gpio);
> +	gpio_reg_clear_bit(regs->enable, gpio);
>  
>  	spin_unlock_irqrestore(&ctlr->lock, flags);
>  }
> @@ -88,7 +88,7 @@ static int tnetv107x_gpio_dir_in(struct gpio_chip *chip, unsigned offset)
>  
>  	spin_lock_irqsave(&ctlr->lock, flags);
>  
> -	gpio_reg_set_bit(&regs->direction, gpio);
> +	gpio_reg_set_bit(regs->direction, gpio);
>  
>  	spin_unlock_irqrestore(&ctlr->lock, flags);
>  
> @@ -106,11 +106,11 @@ static int tnetv107x_gpio_dir_out(struct gpio_chip *chip,
>  	spin_lock_irqsave(&ctlr->lock, flags);
>  
>  	if (value)
> -		gpio_reg_set_bit(&regs->data_out, gpio);
> +		gpio_reg_set_bit(regs->data_out, gpio);
>  	else
> -		gpio_reg_clear_bit(&regs->data_out, gpio);
> +		gpio_reg_clear_bit(regs->data_out, gpio);
>  
> -	gpio_reg_clear_bit(&regs->direction, gpio);
> +	gpio_reg_clear_bit(regs->direction, gpio);
>  
>  	spin_unlock_irqrestore(&ctlr->lock, flags);
>  
> @@ -124,7 +124,7 @@ static int tnetv107x_gpio_get(struct gpio_chip *chip, unsigned offset)
>  	unsigned gpio = chip->base + offset;
>  	int ret;
>  
> -	ret = gpio_reg_get_bit(&regs->data_in, gpio);
> +	ret = gpio_reg_get_bit(regs->data_in, gpio);
>  
>  	return ret ? 1 : 0;
>  }
> @@ -140,9 +140,9 @@ static void tnetv107x_gpio_set(struct gpio_chip *chip,
>  	spin_lock_irqsave(&ctlr->lock, flags);
>  
>  	if (value)
> -		gpio_reg_set_bit(&regs->data_out, gpio);
> +		gpio_reg_set_bit(regs->data_out, gpio);
>  	else
> -		gpio_reg_clear_bit(&regs->data_out, gpio);
> +		gpio_reg_clear_bit(regs->data_out, gpio);
>  
>  	spin_unlock_irqrestore(&ctlr->lock, flags);
>  }

[PATCH] davinci: tnetv107x: fix register indexing for GPIOs numbers > 31

[Hirosh Dabui]

ok-super!
On 01/28/2011 10:47 PM, Kevin Hilman wrote:
> Hirosh Dabui<hirosh.dabui@snom.com>  writes:
>
>    
>> This patch fix a bug in the register indexing for GPIOs numbers>   31
>> to get the relevant hardware registers of tnetv107x to control the GPIOs.
>>
>> In the structure tnetv107x_gpio_regs:
>>
>> struct tnetv107x_gpio_regs {
>>              u32     idver;
>>              u32     data_in[3];
>>              u32     data_out[3];
>>              u32     direction[3];
>>              u32     enable[3];
>> };
>>
>> The GPIO hardware register addresses of tnetv107x are stored.
>> The chip implements 3 registers of each entity to serve 96 GPIOs,
>> each register provides a subset of 32 GPIOs.
>> The driver provides these macros: gpio_reg_set_bit, gpio_reg_get_bit
>> and gpio_reg_clear_bit.
>>
>> The bug implied the use of macros to access the relevant hardware
>> register e.g. the driver code used the macro like this:
>> 'gpio_reg_clear_bit(&reg->data_out, gpio)'
>>
>> But it has to be used like this:
>> 'gpio_reg_clear_bit(reg->data_out, gpio)'.
>>
>> The different results are shown here:
>> -&reg->data_out + 1 (it will add the full array size of data_out i.e. 12 bytes)
>> - reg->data_out + 1 (it will increment only the size of data_out i.e. only 4 bytes)
>>
>> Acked-by: Cyril Chemparathy<cyril@ti.com>
>> Signed-off-by: Hirosh Dabui<hirosh.dabui@snom.com>
>>      
> Thanks, applied and queuing for 2.6.39.
>
> Kevin
>    

[PATCH] davinci: tnetv107x: fix register indexing for GPIOs numbers > 31

[Kevin Hilman]

Hirosh Dabui <hirosh.dabui@snom.com> writes:

> This patch fix a bug in the register indexing for GPIOs numbers >  31
> to get the relevant hardware registers of tnetv107x to control the GPIOs.
>
> In the structure tnetv107x_gpio_regs:
>
> struct tnetv107x_gpio_regs {
>             u32     idver;
>             u32     data_in[3];
>             u32     data_out[3];
>             u32     direction[3];
>             u32     enable[3];
> };
>
> The GPIO hardware register addresses of tnetv107x are stored.
> The chip implements 3 registers of each entity to serve 96 GPIOs,
> each register provides a subset of 32 GPIOs.
> The driver provides these macros: gpio_reg_set_bit, gpio_reg_get_bit
> and gpio_reg_clear_bit.
>
> The bug implied the use of macros to access the relevant hardware
> register e.g. the driver code used the macro like this:
> 'gpio_reg_clear_bit(&reg->data_out, gpio)'
>
> But it has to be used like this:
> 'gpio_reg_clear_bit(reg->data_out, gpio)'.
>
> The different results are shown here:
> - &reg->data_out + 1 (it will add the full array size of data_out i.e. 12 bytes)
> - reg->data_out + 1 (it will increment only the size of data_out i.e. only 4 bytes)
>
> Acked-by: Cyril Chemparathy <cyril@ti.com>
> Signed-off-by: Hirosh Dabui <hirosh.dabui@snom.com>

Thanks, applied and queuing for 2.6.39.

Kevin

[PATCH] davinci: tnetv107x: fix register indexing for GPIOs numbers >31

[Jon Povey]

davinci-linux-open-source-bounces at linux.davincidsp.com wrote:

> The bug implied the use of macros to access the relevant hardware
> register e.g. the driver code used the macro like this:
> 'gpio_reg_clear_bit(&reg->data_out, gpio)'
>
> But it has to be used like this:
> 'gpio_reg_clear_bit(reg->data_out, gpio)'.

Could the macro be made into an inline function so it can do type-checking
to avoid/catch this in future?

--
Jon Povey
jon.povey at racelogic.co.uk

Racelogic is a limited company registered in England. Registered number 2743719 .
Registered Office Unit 10, Swan Business Centre, Osier Way, Buckingham, Bucks, MK18 1TB .

The information contained in this electronic mail transmission is intended by Racelogic Ltd for the use of the named individual or entity to which it is directed and may contain information that is confidential or privileged. If you have received this electronic mail transmission in error, please delete it from your system without copying or forwarding it, and notify the sender of the error by reply email so that the sender's address records can be corrected. The views expressed by the sender of this communication do not necessarily represent those of Racelogic Ltd. Please note that Racelogic reserves the right to monitor e-mail communications passing through its network

[PATCH] davinci: tnetv107x: fix register indexing for GPIOs numbers > 31

[Hirosh Dabui]

This patch fix a bug in the register indexing for GPIOs numbers >  31
to get the relevant hardware registers of tnetv107x to control the GPIOs.

In the structure tnetv107x_gpio_regs:

struct tnetv107x_gpio_regs {
            u32     idver;
            u32     data_in[3];
            u32     data_out[3];
            u32     direction[3];
            u32     enable[3];
};

The GPIO hardware register addresses of tnetv107x are stored.
The chip implements 3 registers of each entity to serve 96 GPIOs,
each register provides a subset of 32 GPIOs.
The driver provides these macros: gpio_reg_set_bit, gpio_reg_get_bit
and gpio_reg_clear_bit.

The bug implied the use of macros to access the relevant hardware
register e.g. the driver code used the macro like this:
'gpio_reg_clear_bit(&reg->data_out, gpio)'

But it has to be used like this:
'gpio_reg_clear_bit(reg->data_out, gpio)'.

The different results are shown here:
- &reg->data_out + 1 (it will add the full array size of data_out i.e. 12 bytes)
- reg->data_out + 1 (it will increment only the size of data_out i.e. only 4 bytes)

Acked-by: Cyril Chemparathy <cyril@ti.com>
Signed-off-by: Hirosh Dabui <hirosh.dabui@snom.com>
---
 arch/arm/mach-davinci/gpio-tnetv107x.c |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-davinci/gpio-tnetv107x.c b/arch/arm/mach-davinci/gpio-tnetv107x.c
index d102986..3fa3e28 100644
--- a/arch/arm/mach-davinci/gpio-tnetv107x.c
+++ b/arch/arm/mach-davinci/gpio-tnetv107x.c
@@ -58,7 +58,7 @@ static int tnetv107x_gpio_request(struct gpio_chip *chip, unsigned offset)
 
 	spin_lock_irqsave(&ctlr->lock, flags);
 
-	gpio_reg_set_bit(&regs->enable, gpio);
+	gpio_reg_set_bit(regs->enable, gpio);
 
 	spin_unlock_irqrestore(&ctlr->lock, flags);
 
@@ -74,7 +74,7 @@ static void tnetv107x_gpio_free(struct gpio_chip *chip, unsigned offset)
 
 	spin_lock_irqsave(&ctlr->lock, flags);
 
-	gpio_reg_clear_bit(&regs->enable, gpio);
+	gpio_reg_clear_bit(regs->enable, gpio);
 
 	spin_unlock_irqrestore(&ctlr->lock, flags);
 }
@@ -88,7 +88,7 @@ static int tnetv107x_gpio_dir_in(struct gpio_chip *chip, unsigned offset)
 
 	spin_lock_irqsave(&ctlr->lock, flags);
 
-	gpio_reg_set_bit(&regs->direction, gpio);
+	gpio_reg_set_bit(regs->direction, gpio);
 
 	spin_unlock_irqrestore(&ctlr->lock, flags);
 
@@ -106,11 +106,11 @@ static int tnetv107x_gpio_dir_out(struct gpio_chip *chip,
 	spin_lock_irqsave(&ctlr->lock, flags);
 
 	if (value)
-		gpio_reg_set_bit(&regs->data_out, gpio);
+		gpio_reg_set_bit(regs->data_out, gpio);
 	else
-		gpio_reg_clear_bit(&regs->data_out, gpio);
+		gpio_reg_clear_bit(regs->data_out, gpio);
 
-	gpio_reg_clear_bit(&regs->direction, gpio);
+	gpio_reg_clear_bit(regs->direction, gpio);
 
 	spin_unlock_irqrestore(&ctlr->lock, flags);
 
@@ -124,7 +124,7 @@ static int tnetv107x_gpio_get(struct gpio_chip *chip, unsigned offset)
 	unsigned gpio = chip->base + offset;
 	int ret;
 
-	ret = gpio_reg_get_bit(&regs->data_in, gpio);
+	ret = gpio_reg_get_bit(regs->data_in, gpio);
 
 	return ret ? 1 : 0;
 }
@@ -140,9 +140,9 @@ static void tnetv107x_gpio_set(struct gpio_chip *chip,
 	spin_lock_irqsave(&ctlr->lock, flags);
 
 	if (value)
-		gpio_reg_set_bit(&regs->data_out, gpio);
+		gpio_reg_set_bit(regs->data_out, gpio);
 	else
-		gpio_reg_clear_bit(&regs->data_out, gpio);
+		gpio_reg_clear_bit(regs->data_out, gpio);
 
 	spin_unlock_irqrestore(&ctlr->lock, flags);
 }
-- 
1.7.1