Patchwork [U-Boot,1/3] imx25: Move MXC_GPIO_PORT_TO_NUM to imx-regs.h

login
register
mail settings
Submitter Vikram Narayanan
Date June 10, 2012, 1:02 p.m.
Message ID <4FD49AF4.2090506@gmail.com>
Download mbox | patch
Permalink /patch/163994/
State Changes Requested
Headers show

Comments

Vikram Narayanan - June 10, 2012, 1:02 p.m.
Move the macro to imx-regs.h so that the other mx25 boards can
make use of it.

Signed-off-by: Vikram Narayanan <vikram186@gmail.com>
---
 arch/arm/include/asm/arch-mx25/gpio.h     |    4 ----
 arch/arm/include/asm/arch-mx25/imx-regs.h |    6 ++++++
 2 files changed, 6 insertions(+), 4 deletions(-)
Fabio Estevam - June 10, 2012, 2:33 p.m.
Hi Vikram,

On Sun, Jun 10, 2012 at 10:02 AM, Vikram Narayanan <vikram186@gmail.com> wrote:

> +
> +/* Converts a GPIO port number and the internal bit position
> + * to the GPIO number
> + */
> +#define MXC_GPIO_PORT_TO_NUM(port, bit) (((port - 1) << 5) + (bit & 0x1f))

Just a minor comment:

MXC_GPIO_PORT_TO_NUM looks like a very looong string.

Couldn't we use the same macro as in the Linux kernel (IMX_GPIO_NR) instead?

It is more concise and it would nice to have the same macro name for
kernel and U-boot.

What do you think?

Thanks,

Fabio Estevam
Fabio Estevam - June 10, 2012, 3:03 p.m.
On Sun, Jun 10, 2012 at 10:02 AM, Vikram Narayanan <vikram186@gmail.com> wrote:
> Move the macro to imx-regs.h so that the other mx25 boards can
> make use of it.
>
> Signed-off-by: Vikram Narayanan <vikram186@gmail.com>
> ---
>  arch/arm/include/asm/arch-mx25/gpio.h     |    4 ----
>  arch/arm/include/asm/arch-mx25/imx-regs.h |    6 ++++++
>  2 files changed, 6 insertions(+), 4 deletions(-)

Also, this macro is useful not only for mx25, and it would be nice to
let it avaible for the other i.MX SoCs as well.
Vikram Narayanan - June 11, 2012, 3 p.m.
On 6/10/2012 8:03 PM, Fabio Estevam wrote:
> Hi Vikram,
>
> On Sun, Jun 10, 2012 at 10:02 AM, Vikram Narayanan<vikram186@gmail.com>  wrote:
>
>> +
>> +/* Converts a GPIO port number and the internal bit position
>> + * to the GPIO number
>> + */
>> +#define MXC_GPIO_PORT_TO_NUM(port, bit) (((port - 1)<<  5) + (bit&  0x1f))
>
> Just a minor comment:
>
> MXC_GPIO_PORT_TO_NUM looks like a very looong string.
>
> Couldn't we use the same macro as in the Linux kernel (IMX_GPIO_NR) instead?
>
> It is more concise and it would nice to have the same macro name for
> kernel and U-boot.
>
> What do you think?

Yes. That should be a better option. I'll go with it.

~Vikram

> Thanks,
>
> Fabio Estevam
Vikram Narayanan - June 11, 2012, 3:02 p.m.
Hello Fabio,

On 6/10/2012 8:33 PM, Fabio Estevam wrote:
> On Sun, Jun 10, 2012 at 10:02 AM, Vikram Narayanan<vikram186@gmail.com>  wrote:
>> Move the macro to imx-regs.h so that the other mx25 boards can
>> make use of it.
>>
>> Signed-off-by: Vikram Narayanan<vikram186@gmail.com>
>> ---
>>   arch/arm/include/asm/arch-mx25/gpio.h     |    4 ----
>>   arch/arm/include/asm/arch-mx25/imx-regs.h |    6 ++++++
>>   2 files changed, 6 insertions(+), 4 deletions(-)
>
> Also, this macro is useful not only for mx25, and it would be nice to
> let it avaible for the other i.MX SoCs as well.

Agree. But I guess u-boot doesn't have a common folder where this can be 
put up something like plat-mxc. So, I should think where to place this 
macro to avoid duplicates in many header files. Any pointers?

Thanks,
Vikram
Troy Kisky - June 11, 2012, 7:56 p.m.
On 6/11/2012 8:00 AM, Vikram Narayanan wrote:
> On 6/10/2012 8:03 PM, Fabio Estevam wrote:
>> Hi Vikram,
>>
>> On Sun, Jun 10, 2012 at 10:02 AM, Vikram 
>> Narayanan<vikram186@gmail.com>  wrote:
>>
>>> +
>>> +/* Converts a GPIO port number and the internal bit position
>>> + * to the GPIO number
>>> + */
>>> +#define MXC_GPIO_PORT_TO_NUM(port, bit) (((port - 1)<<  5) + (bit&  
>>> 0x1f))
>>
>> Just a minor comment:
>>
>> MXC_GPIO_PORT_TO_NUM looks like a very looong string.
>>
>> Couldn't we use the same macro as in the Linux kernel (IMX_GPIO_NR) 
>> instead?
>>
>> It is more concise and it would nice to have the same macro name for
>> kernel and U-boot.
>>
>> What do you think?
>
> Yes. That should be a better option. I'll go with it.

We have this line already

arch/arm/include/asm/arch-mx6/imx-regs.h:#define GPIO_NUMBER(port, 
index)               ((((port)-1)*32)+((index)&31))

>
> ~Vikram
>
>> Thanks,
>>
>> Fabio Estevam
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
> .
>
Fabio Estevam - June 13, 2012, 11:37 a.m.
On Mon, Jun 11, 2012 at 12:02 PM, Vikram Narayanan <vikram186@gmail.com> wrote:

>> Also, this macro is useful not only for mx25, and it would be nice to
>> let it avaible for the other i.MX SoCs as well.
>
>
> Agree. But I guess u-boot doesn't have a common folder where this can be put
> up something like plat-mxc. So, I should think where to place this macro to
> avoid duplicates in many header files. Any pointers?

Yes, a common folder would help indeed.

Copying Stefano, so that he could provide us some suggestion on this.

Thanks,

Fabio Estevam
Stefano Babic - June 15, 2012, 2:55 p.m.
On 13/06/2012 13:37, Fabio Estevam wrote:
> On Mon, Jun 11, 2012 at 12:02 PM, Vikram Narayanan <vikram186@gmail.com> wrote:
> 
>>> Also, this macro is useful not only for mx25, and it would be nice to
>>> let it avaible for the other i.MX SoCs as well.
>>
>>
>> Agree. But I guess u-boot doesn't have a common folder where this can be put
>> up something like plat-mxc. So, I should think where to place this macro to
>> avoid duplicates in many header files. Any pointers?
> 
> Yes, a common folder would help indeed.
> 
> Copying Stefano, so that he could provide us some suggestion on this.

A general common folder helps - we have already an imx-common, whose
name was copied from omap-common we already head. At the moment, it
contains files common to i.MX5 and i.MX6. However, it is located under
armv7, and it is not accessible for other i.MX.

Really I am for a solution like in kernel, with a plat-mxc into
include/asm/, where we can put common things for all i.MX.

Best regards,
Stefano Babic
Vikram Narayanan - June 15, 2012, 6:10 p.m.
Hi Stefano,

On 6/15/2012 8:25 PM, Stefano Babic wrote:
> On 13/06/2012 13:37, Fabio Estevam wrote:
>> On Mon, Jun 11, 2012 at 12:02 PM, Vikram Narayanan<vikram186@gmail.com>  wrote:
>>
>>>> Also, this macro is useful not only for mx25, and it would be nice to
>>>> let it avaible for the other i.MX SoCs as well.
>>>
>>>
>>> Agree. But I guess u-boot doesn't have a common folder where this can be put
>>> up something like plat-mxc. So, I should think where to place this macro to
>>> avoid duplicates in many header files. Any pointers?
>>
>> Yes, a common folder would help indeed.
>>
>> Copying Stefano, so that he could provide us some suggestion on this.
>
> A general common folder helps - we have already an imx-common, whose
> name was copied from omap-common we already head. At the moment, it
> contains files common to i.MX5 and i.MX6. However, it is located under
> armv7, and it is not accessible for other i.MX.
>
> Really I am for a solution like in kernel, with a plat-mxc into
> include/asm/, where we can put common things for all i.MX.

I'm with the same idea too. In this way we can avoid duplicate 
datastructures spread across many arch directories.

So, are there any opposers for this?

Regards,
Vikram
Fabio Estevam - June 15, 2012, 6:13 p.m.
On Fri, Jun 15, 2012 at 3:10 PM, Vikram Narayanan <vikram186@gmail.com> wrote:

>> Really I am for a solution like in kernel, with a plat-mxc into
>> include/asm/, where we can put common things for all i.MX.
>
>
> I'm with the same idea too. In this way we can avoid duplicate
> datastructures spread across many arch directories.

Agree!

Patch

diff --git a/arch/arm/include/asm/arch-mx25/gpio.h b/arch/arm/include/asm/arch-mx25/gpio.h
index dc6edc7..cc32d50 100644
--- a/arch/arm/include/asm/arch-mx25/gpio.h
+++ b/arch/arm/include/asm/arch-mx25/gpio.h
@@ -25,10 +25,6 @@ 
 #ifndef __ASM_ARCH_MX25_GPIO_H
 #define __ASM_ARCH_MX25_GPIO_H
 
-/* Converts a GPIO port number and the internal bit position
- * to the GPIO number
- */
-#define MXC_GPIO_PORT_TO_NUM(port, bit) (((port - 1) << 5) + (bit & 0x1f))
 
 /* GPIO registers */
 struct gpio_regs {
diff --git a/arch/arm/include/asm/arch-mx25/imx-regs.h b/arch/arm/include/asm/arch-mx25/imx-regs.h
index cf925d7..1f67abb 100644
--- a/arch/arm/include/asm/arch-mx25/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx25/imx-regs.h
@@ -356,4 +356,10 @@  struct aips_regs {
 #define CHIP_REV_1_0		0x10
 #define CHIP_REV_1_1		0x11
 
+
+/* Converts a GPIO port number and the internal bit position
+ * to the GPIO number
+ */
+#define MXC_GPIO_PORT_TO_NUM(port, bit) (((port - 1) << 5) + (bit & 0x1f))
+
 #endif				/* _IMX_REGS_H */