Patchwork [U-Boot,1/3] imx: Add GPIO_TO_PORT macro

login
register
mail settings
Submitter Vikram Narayanan
Date April 4, 2012, 4:05 p.m.
Message ID <4F7C7153.3040601@gmail.com>
Download mbox | patch
Permalink /patch/150769/
State Rejected
Delegated to: Stefano Babic
Headers show

Comments

Vikram Narayanan - April 4, 2012, 4:05 p.m.
imx: Add GPIO_TO_PORT macro

Signed-off-by: Vikram Narayanan <vikram186@gmail.com>
---
 arch/arm/include/asm/arch-mx5/gpio.h |    2 ++
 arch/arm/include/asm/arch-mx6/gpio.h |    2 ++
 2 files changed, 4 insertions(+), 0 deletions(-)
  50.0% arch/arm/include/asm/arch-mx5/
  50.0% arch/arm/include/asm/arch-mx6/
Stefano Babic - April 6, 2012, 8:56 a.m.
On 04/04/2012 18:05, Vikram Narayanan wrote:
> imx: Add GPIO_TO_PORT macro
> 
> Signed-off-by: Vikram Narayanan <vikram186@gmail.com>

Hi,

> ---
>  arch/arm/include/asm/arch-mx5/gpio.h |    2 ++
>  arch/arm/include/asm/arch-mx6/gpio.h |    2 ++
>  2 files changed, 4 insertions(+), 0 deletions(-)
>   50.0% arch/arm/include/asm/arch-mx5/
>   50.0% arch/arm/include/asm/arch-mx6/
> 
> diff --git a/arch/arm/include/asm/arch-mx5/gpio.h b/arch/arm/include/asm/arch-mx5/gpio.h
> index 1dc34e9..bcb5edb 100644
> --- a/arch/arm/include/asm/arch-mx5/gpio.h
> +++ b/arch/arm/include/asm/arch-mx5/gpio.h
> @@ -25,6 +25,8 @@
>  #ifndef __ASM_ARCH_MX5_GPIO_H
>  #define __ASM_ARCH_MX5_GPIO_H
>  
> +#define GPIO_TO_PORT(number)		(number/32)
> +
>  /* GPIO registers */
>  struct gpio_regs {
>  	u32	gpio_dr;
> diff --git a/arch/arm/include/asm/arch-mx6/gpio.h b/arch/arm/include/asm/arch-mx6/gpio.h
> index 20c4e57..385d12d 100644
> --- a/arch/arm/include/asm/arch-mx6/gpio.h
> +++ b/arch/arm/include/asm/arch-mx6/gpio.h
> @@ -25,6 +25,8 @@
>  #ifndef __ASM_ARCH_MX6_GPIO_H
>  #define __ASM_ARCH_MX6_GPIO_H
>  
> +#define GPIO_TO_PORT(number)		(number/32)
> +
>  /* GPIO registers */
>  struct gpio_regs {
>  	u32	gpio_dr;

NAK. We have already (and probably too many) GPIO_TO_PORT:

arch/arm/include/asm/arch-mx6/imx-regs.h:#define GPIO_TO_PORT(number)	
(((number)/32)+1)
arch/arm/include/asm/arch-mx5/mx5x_pins.h:#define GPIO_TO_PORT(n)
  (n / GPIO_NUM_PIN)

Are they not enough ?

Best regards,
Stefano Babic
Dirk Behme - April 9, 2012, 5:33 a.m.
Dear Vikram ,

On 06.04.2012 10:56, Stefano Babic wrote:
> On 04/04/2012 18:05, Vikram Narayanan wrote:
>> imx: Add GPIO_TO_PORT macro
>>
>> Signed-off-by: Vikram Narayanan<vikram186@gmail.com>
>
> Hi,
>
>> ---
>>   arch/arm/include/asm/arch-mx5/gpio.h |    2 ++
>>   arch/arm/include/asm/arch-mx6/gpio.h |    2 ++
>>   2 files changed, 4 insertions(+), 0 deletions(-)
>>    50.0% arch/arm/include/asm/arch-mx5/
>>    50.0% arch/arm/include/asm/arch-mx6/
>>
>> diff --git a/arch/arm/include/asm/arch-mx5/gpio.h b/arch/arm/include/asm/arch-mx5/gpio.h
>> index 1dc34e9..bcb5edb 100644
>> --- a/arch/arm/include/asm/arch-mx5/gpio.h
>> +++ b/arch/arm/include/asm/arch-mx5/gpio.h
>> @@ -25,6 +25,8 @@
>>   #ifndef __ASM_ARCH_MX5_GPIO_H
>>   #define __ASM_ARCH_MX5_GPIO_H
>>
>> +#define GPIO_TO_PORT(number)		(number/32)
>> +
>>   /* GPIO registers */
>>   struct gpio_regs {
>>   	u32	gpio_dr;
>> diff --git a/arch/arm/include/asm/arch-mx6/gpio.h b/arch/arm/include/asm/arch-mx6/gpio.h
>> index 20c4e57..385d12d 100644
>> --- a/arch/arm/include/asm/arch-mx6/gpio.h
>> +++ b/arch/arm/include/asm/arch-mx6/gpio.h
>> @@ -25,6 +25,8 @@
>>   #ifndef __ASM_ARCH_MX6_GPIO_H
>>   #define __ASM_ARCH_MX6_GPIO_H
>>
>> +#define GPIO_TO_PORT(number)		(number/32)
>> +
>>   /* GPIO registers */
>>   struct gpio_regs {
>>   	u32	gpio_dr;
>
> NAK. We have already (and probably too many) GPIO_TO_PORT:
>
> arch/arm/include/asm/arch-mx6/imx-regs.h:#define GPIO_TO_PORT(number)	
> (((number)/32)+1)
> arch/arm/include/asm/arch-mx5/mx5x_pins.h:#define GPIO_TO_PORT(n)
>    (n / GPIO_NUM_PIN)
>
> Are they not enough ?

Vikram, it seems to me you sent the already NAKed patch to me

https://github.com/dirkbehme/u-boot-imx6/pull/2

again?

Please send U-Boot patches only to this U-Boot mailing list.

Dirk
Vikram Narayanan - April 9, 2012, 2:30 p.m.
Hello Dirk,

On 4/9/2012 11:03 AM, Dirk Behme wrote:
> Dear Vikram ,
>
> On 06.04.2012 10:56, Stefano Babic wrote:
>> On 04/04/2012 18:05, Vikram Narayanan wrote:
>>> imx: Add GPIO_TO_PORT macro
>>>
>>> Signed-off-by: Vikram Narayanan<vikram186@gmail.com>
>>
>> Hi,
>>
>>> ---
>>> arch/arm/include/asm/arch-mx5/gpio.h | 2 ++
>>> arch/arm/include/asm/arch-mx6/gpio.h | 2 ++
>>> 2 files changed, 4 insertions(+), 0 deletions(-)
>>> 50.0% arch/arm/include/asm/arch-mx5/
>>> 50.0% arch/arm/include/asm/arch-mx6/
>>>
>>> diff --git a/arch/arm/include/asm/arch-mx5/gpio.h
>>> b/arch/arm/include/asm/arch-mx5/gpio.h
>>> index 1dc34e9..bcb5edb 100644
>>> --- a/arch/arm/include/asm/arch-mx5/gpio.h
>>> +++ b/arch/arm/include/asm/arch-mx5/gpio.h
>>> @@ -25,6 +25,8 @@
>>> #ifndef __ASM_ARCH_MX5_GPIO_H
>>> #define __ASM_ARCH_MX5_GPIO_H
>>>
>>> +#define GPIO_TO_PORT(number) (number/32)
>>> +
>>> /* GPIO registers */
>>> struct gpio_regs {
>>> u32 gpio_dr;
>>> diff --git a/arch/arm/include/asm/arch-mx6/gpio.h
>>> b/arch/arm/include/asm/arch-mx6/gpio.h
>>> index 20c4e57..385d12d 100644
>>> --- a/arch/arm/include/asm/arch-mx6/gpio.h
>>> +++ b/arch/arm/include/asm/arch-mx6/gpio.h
>>> @@ -25,6 +25,8 @@
>>> #ifndef __ASM_ARCH_MX6_GPIO_H
>>> #define __ASM_ARCH_MX6_GPIO_H
>>>
>>> +#define GPIO_TO_PORT(number) (number/32)
>>> +
>>> /* GPIO registers */
>>> struct gpio_regs {
>>> u32 gpio_dr;
>>
>> NAK. We have already (and probably too many) GPIO_TO_PORT:
>>
>> arch/arm/include/asm/arch-mx6/imx-regs.h:#define GPIO_TO_PORT(number)
>> (((number)/32)+1)
>> arch/arm/include/asm/arch-mx5/mx5x_pins.h:#define GPIO_TO_PORT(n)
>> (n / GPIO_NUM_PIN)
>>
>> Are they not enough ?

@Stefano: Yeah. Just didn't notice it. Will resend it.


> Vikram, it seems to me you sent the already NAKed patch to me
>
> https://github.com/dirkbehme/u-boot-imx6/pull/2
>
> again?
>

Sorry, something went wrong with my filter settings in my mailbox. I 
wasn't aware that it got NACked. Sorry for sending a pull req anyways.

> Please send U-Boot patches only to this U-Boot mailing list.

Sure. From now on!

> Dirk

~Vikram
Vikram Narayanan - April 9, 2012, 4:09 p.m.
Hi,

On 4/6/2012 2:26 PM, Stefano Babic wrote:
> On 04/04/2012 18:05, Vikram Narayanan wrote:
>> imx: Add GPIO_TO_PORT macro
>>
>> Signed-off-by: Vikram Narayanan<vikram186@gmail.com>
>
> Hi,
>
>> ---
>>   arch/arm/include/asm/arch-mx5/gpio.h |    2 ++
>>   arch/arm/include/asm/arch-mx6/gpio.h |    2 ++
>>   2 files changed, 4 insertions(+), 0 deletions(-)
>>    50.0% arch/arm/include/asm/arch-mx5/
>>    50.0% arch/arm/include/asm/arch-mx6/
>>
>> diff --git a/arch/arm/include/asm/arch-mx5/gpio.h b/arch/arm/include/asm/arch-mx5/gpio.h
>> index 1dc34e9..bcb5edb 100644
>> --- a/arch/arm/include/asm/arch-mx5/gpio.h
>> +++ b/arch/arm/include/asm/arch-mx5/gpio.h
>> @@ -25,6 +25,8 @@
>>   #ifndef __ASM_ARCH_MX5_GPIO_H
>>   #define __ASM_ARCH_MX5_GPIO_H
>>
>> +#define GPIO_TO_PORT(number)		(number/32)
>> +
>>   /* GPIO registers */
>>   struct gpio_regs {
>>   	u32	gpio_dr;
>> diff --git a/arch/arm/include/asm/arch-mx6/gpio.h b/arch/arm/include/asm/arch-mx6/gpio.h
>> index 20c4e57..385d12d 100644
>> --- a/arch/arm/include/asm/arch-mx6/gpio.h
>> +++ b/arch/arm/include/asm/arch-mx6/gpio.h
>> @@ -25,6 +25,8 @@
>>   #ifndef __ASM_ARCH_MX6_GPIO_H
>>   #define __ASM_ARCH_MX6_GPIO_H
>>
>> +#define GPIO_TO_PORT(number)		(number/32)
>> +
>>   /* GPIO registers */
>>   struct gpio_regs {
>>   	u32	gpio_dr;
>
> NAK. We have already (and probably too many) GPIO_TO_PORT:

Yes. You are right.
So, instead of defining all the headers this way,

#if defined(CONFIG_MX53) || defined(CONFIG_MX51)
#include <asm/arch/mx5x_pins.h>
#elif defined(CONFIG_MX6)
#include <asm/arch/imx-regs.h>
#endif
..... etc

Why not define the GPIO_TO_PORT macro in the driver? Anyways for all its 
the same 32 pins. Any suggestions/flames?

Thanks,
Vikram
Stefano Babic - April 9, 2012, 10:31 p.m.
Am 09/04/2012 18:09, schrieb Vikram Narayanan:
> Hi,
> 

Hi,

> 
> Yes. You are right.
> So, instead of defining all the headers this way,
> 
> #if defined(CONFIG_MX53) || defined(CONFIG_MX51)
> #include <asm/arch/mx5x_pins.h>
> #elif defined(CONFIG_MX6)
> #include <asm/arch/imx-regs.h>
> #endif
> ..... etc


We have not this code - I cannot find in u-boot, and wedo not want to
introduce it. As you say, it is nasty. Where have you find it ?

There is no driver including mx*_pins.h. At the moment, only board
specific code includes the SOC specific pin header.

> 
> Why not define the GPIO_TO_PORT macro in the driver?

Maybe there was some use of the macro outside the driver in the past. I
think before i.MX code was adapted to use common gpio_ functions, boards
are used to write directly into the registers of the GPIO controller.

I do not see any track of the macro in the current tree. So yes, we can
move GPIO_ macros inside the driver.

> Anyways for all its
> the same 32 pins. Any suggestions/flames?

It seems to me also that the defined GPIO_PORT for MX6 is wrong.

arch/arm/include/asm/arch-mx6/imx-regs.h:

#define GPIO_TO_PORT(number)		(((number)/32)+1)

Why is the port starting from 1 ? It is wrong, but really GPIO_TO_PORT()
is not used anymore.

Best regards,
Stefano Babic
Vikram Narayanan - April 10, 2012, 2:47 a.m.
Hi Stefano,

On 4/10/2012 4:01 AM, stefano babic wrote:
> Am 09/04/2012 18:09, schrieb Vikram Narayanan:
>> Hi,
>>
>
> Hi,
>
>>
>> Yes. You are right.
>> So, instead of defining all the headers this way,
>>
>> #if defined(CONFIG_MX53) || defined(CONFIG_MX51)
>> #include<asm/arch/mx5x_pins.h>
>> #elif defined(CONFIG_MX6)
>> #include<asm/arch/imx-regs.h>
>> #endif
>> ..... etc
>
>
> We have not this code - I cannot find in u-boot, and wedo not want to
> introduce it. As you say, it is nasty. Where have you find it ?

I don't find it anyway. If I want to use the existing macro, it would 
result in this.

> There is no driver including mx*_pins.h. At the moment, only board
> specific code includes the SOC specific pin header.
>
>>
>> Why not define the GPIO_TO_PORT macro in the driver?
>
> Maybe there was some use of the macro outside the driver in the past. I
> think before i.MX code was adapted to use common gpio_ functions, boards
> are used to write directly into the registers of the GPIO controller.
>
> I do not see any track of the macro in the current tree. So yes, we can
> move GPIO_ macros inside the driver.

Sure. I'll just do that.

>> Anyways for all its
>> the same 32 pins. Any suggestions/flames?
>
> It seems to me also that the defined GPIO_PORT for MX6 is wrong.
>
> arch/arm/include/asm/arch-mx6/imx-regs.h:
>
> #define GPIO_TO_PORT(number)		(((number)/32)+1)
>
> Why is the port starting from 1 ? It is wrong, but really GPIO_TO_PORT()
> is not used anymore.

Yes. You are right. I'll send the v2 for this.

Thanks for your response,
Vikram

> Best regards,
> Stefano Babic
>

Patch

diff --git a/arch/arm/include/asm/arch-mx5/gpio.h b/arch/arm/include/asm/arch-mx5/gpio.h
index 1dc34e9..bcb5edb 100644
--- a/arch/arm/include/asm/arch-mx5/gpio.h
+++ b/arch/arm/include/asm/arch-mx5/gpio.h
@@ -25,6 +25,8 @@ 
 #ifndef __ASM_ARCH_MX5_GPIO_H
 #define __ASM_ARCH_MX5_GPIO_H
 
+#define GPIO_TO_PORT(number)		(number/32)
+
 /* GPIO registers */
 struct gpio_regs {
 	u32	gpio_dr;
diff --git a/arch/arm/include/asm/arch-mx6/gpio.h b/arch/arm/include/asm/arch-mx6/gpio.h
index 20c4e57..385d12d 100644
--- a/arch/arm/include/asm/arch-mx6/gpio.h
+++ b/arch/arm/include/asm/arch-mx6/gpio.h
@@ -25,6 +25,8 @@ 
 #ifndef __ASM_ARCH_MX6_GPIO_H
 #define __ASM_ARCH_MX6_GPIO_H
 
+#define GPIO_TO_PORT(number)		(number/32)
+
 /* GPIO registers */
 struct gpio_regs {
 	u32	gpio_dr;