* [patch 1/1] iMX51: introduce MX51_GPIO_NR
@ 2010-11-22 22:45 Arnaud Patard (Rtp)
2010-11-24 5:21 ` Amit Kucheria
2010-11-24 16:49 ` Fabio Estevam
0 siblings, 2 replies; 9+ messages in thread
From: Arnaud Patard (Rtp) @ 2010-11-22 22:45 UTC (permalink / raw)
To: linux-arm-kernel
An embedded and charset-unspecified text was scrubbed...
Name: add_gpionr.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20101122/7ec3eabe/attachment.ksh>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [patch 1/1] iMX51: introduce MX51_GPIO_NR
2010-11-22 22:45 [patch 1/1] iMX51: introduce MX51_GPIO_NR Arnaud Patard (Rtp)
@ 2010-11-24 5:21 ` Amit Kucheria
2010-11-24 7:46 ` Lothar Waßmann
2010-11-24 16:28 ` Arnaud Patard (Rtp)
2010-11-24 16:49 ` Fabio Estevam
1 sibling, 2 replies; 9+ messages in thread
From: Amit Kucheria @ 2010-11-24 5:21 UTC (permalink / raw)
To: linux-arm-kernel
On 10 Nov 22, Arnaud Patard wrote:
> Currently, to define a GPIO number, we're using something like :
>
> #define EFIKAMX_PCBID0 (2*32 + 16)
>
> to define GPIO 3 16.
>
> This is not really readable and it's error prone imho (note the 3 vs 2).
> So, I'm introducing a new macro to define this in a better way. Now, the
> code sample become :
>
> #define EFIKAMX_PCBID0 MX51_GPIO_NR(3, 16)
>
I have been unhappy over the readability of the existing gpios definitions.
So this patch is welcome. A couple of commments below.
>
> Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
> Cc: Amit Kucheria <amit.kucheria@linaro.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Eric B?nard <eric@eukrea.com>
>
> Index: gpionr/arch/arm/mach-mx5/board-cpuimx51sd.c
> ===================================================================
> --- gpionr.orig/arch/arm/mach-mx5/board-cpuimx51sd.c 2010-11-22 23:04:23.000000000 +0100
> +++ gpionr/arch/arm/mach-mx5/board-cpuimx51sd.c 2010-11-22 23:04:25.000000000 +0100
> @@ -43,19 +43,19 @@
> #include "devices-imx51.h"
> #include "devices.h"
>
> -#define USBH1_RST (1*32 + 28)
> -#define ETH_RST (1*32 + 31)
> -#define TSC2007_IRQGPIO (2*32 + 12)
> -#define CAN_IRQGPIO (0*32 + 1)
> -#define CAN_RST (3*32 + 15)
> -#define CAN_NCS (3*32 + 24)
> -#define CAN_RXOBF (0*32 + 4)
> -#define CAN_RX1BF (0*32 + 6)
> -#define CAN_TXORTS (0*32 + 7)
> -#define CAN_TX1RTS (0*32 + 8)
> -#define CAN_TX2RTS (0*32 + 9)
> -#define I2C_SCL (3*32 + 16)
> -#define I2C_SDA (3*32 + 17)
> +#define USBH1_RST MX51_GPIO_NR(2, 28)
> +#define ETH_RST MX51_GPIO_NR(2, 31)
> +#define TSC2007_IRQGPIO MX51_GPIO_NR(3, 12)
> +#define CAN_IRQGPIO MX51_GPIO_NR(1, 1)
> +#define CAN_RST MX51_GPIO_NR(4, 15)
> +#define CAN_NCS MX51_GPIO_NR(4, 24)
> +#define CAN_RXOBF MX51_GPIO_NR(1, 4)
> +#define CAN_RX1BF MX51_GPIO_NR(1, 6)
> +#define CAN_TXORTS MX51_GPIO_NR(1, 7)
> +#define CAN_TX1RTS MX51_GPIO_NR(1, 8)
> +#define CAN_TX2RTS MX51_GPIO_NR(1, 9)
> +#define I2C_SCL MX51_GPIO_NR(4, 16)
> +#define I2C_SDA MX51_GPIO_NR(4, 17)
>
> /* USB_CTRL_1 */
> #define MX51_USB_CTRL_1_OFFSET 0x10
> @@ -243,7 +243,7 @@
> .mode = SPI_MODE_0,
> .chip_select = 0,
> .platform_data = &mcp251x_info,
> - .irq = gpio_to_irq(0 * 32 + 1)
> + .irq = gpio_to_irq(CAN_IRQGPIO)
> },
> };
>
> Index: gpionr/arch/arm/plat-mxc/include/mach/iomux-mx51.h
> ===================================================================
> --- gpionr.orig/arch/arm/plat-mxc/include/mach/iomux-mx51.h 2010-11-22 23:04:23.000000000 +0100
> +++ gpionr/arch/arm/plat-mxc/include/mach/iomux-mx51.h 2010-11-22 23:04:25.000000000 +0100
> @@ -15,6 +15,8 @@
>
> #include <mach/iomux-v3.h>
>
Please add a comment here that bank numbers start from 1 to make it explicit
> +#define MX51_GPIO_NR(bank, nr) ((bank-1)*32+nr)
> +
^^^^^^^^^ whitespace fixes
Also, why is this in iomux-mx51.h? It might be better to introduce a gpio.h
> /*
> * various IOMUX alternate output functions (1-7)
> */
> Index: gpionr/arch/arm/mach-mx5/eukrea_mbimx51-baseboard.c
> ===================================================================
> --- gpionr.orig/arch/arm/mach-mx5/eukrea_mbimx51-baseboard.c 2010-11-22 23:04:23.000000000 +0100
> +++ gpionr/arch/arm/mach-mx5/eukrea_mbimx51-baseboard.c 2010-11-22 23:04:25.000000000 +0100
> @@ -33,12 +33,12 @@
> #include "devices-imx51.h"
> #include "devices.h"
>
> -#define MBIMX51_TSC2007_GPIO (2*32 + 30)
> +#define MBIMX51_TSC2007_GPIO MX51_GPIO_NR(3, 30)
> #define MBIMX51_TSC2007_IRQ (MXC_INTERNAL_IRQS + MBIMX51_TSC2007_GPIO)
> -#define MBIMX51_LED0 (2*32 + 5)
> -#define MBIMX51_LED1 (2*32 + 6)
> -#define MBIMX51_LED2 (2*32 + 7)
> -#define MBIMX51_LED3 (2*32 + 8)
> +#define MBIMX51_LED0 MX51_GPIO_NR(3, 5)
> +#define MBIMX51_LED1 MX51_GPIO_NR(3, 6)
> +#define MBIMX51_LED2 MX51_GPIO_NR(3, 7)
> +#define MBIMX51_LED3 MX51_GPIO_NR(3, 8)
>
> static struct gpio_led mbimx51_leds[] = {
> {
> Index: gpionr/arch/arm/mach-mx5/eukrea_mbimxsd-baseboard.c
> ===================================================================
> --- gpionr.orig/arch/arm/mach-mx5/eukrea_mbimxsd-baseboard.c 2010-11-22 23:04:23.000000000 +0100
> +++ gpionr/arch/arm/mach-mx5/eukrea_mbimxsd-baseboard.c 2010-11-22 23:04:25.000000000 +0100
> @@ -70,8 +70,8 @@
> MX51_PAD_SD1_DATA3__SD1_DATA3,
> };
>
> -#define GPIO_LED1 (2 * 32 + 30)
> -#define GPIO_SWITCH1 (2 * 32 + 31)
> +#define GPIO_LED1 MX51_GPIO_NR(3, 30)
> +#define GPIO_SWITCH1 MX51_GPIO_NR(3, 31)
>
> static struct gpio_led eukrea_mbimxsd_leds[] = {
> {
> Index: gpionr/arch/arm/mach-mx5/board-cpuimx51.c
> ===================================================================
> --- gpionr.orig/arch/arm/mach-mx5/board-cpuimx51.c 2010-11-22 23:04:23.000000000 +0100
> +++ gpionr/arch/arm/mach-mx5/board-cpuimx51.c 2010-11-22 23:04:25.000000000 +0100
> @@ -40,11 +40,11 @@
> #include "devices-imx51.h"
> #include "devices.h"
>
> -#define CPUIMX51_USBH1_STP (0*32 + 27)
> -#define CPUIMX51_QUARTA_GPIO (2*32 + 28)
> -#define CPUIMX51_QUARTB_GPIO (2*32 + 25)
> -#define CPUIMX51_QUARTC_GPIO (2*32 + 26)
> -#define CPUIMX51_QUARTD_GPIO (2*32 + 27)
> +#define CPUIMX51_USBH1_STP MX51_GPIO_NR(1, 27)
> +#define CPUIMX51_QUARTA_GPIO MX51_GPIO_NR(3, 28)
> +#define CPUIMX51_QUARTB_GPIO MX51_GPIO_NR(3, 25)
> +#define CPUIMX51_QUARTC_GPIO MX51_GPIO_NR(3, 26)
> +#define CPUIMX51_QUARTD_GPIO MX51_GPIO_NR(3, 27)
> #define CPUIMX51_QUARTA_IRQ (MXC_INTERNAL_IRQS + CPUIMX51_QUARTA_GPIO)
> #define CPUIMX51_QUARTB_IRQ (MXC_INTERNAL_IRQS + CPUIMX51_QUARTB_GPIO)
> #define CPUIMX51_QUARTC_IRQ (MXC_INTERNAL_IRQS + CPUIMX51_QUARTC_GPIO)
> Index: gpionr/arch/arm/mach-mx5/board-mx51_efikamx.c
> ===================================================================
> --- gpionr.orig/arch/arm/mach-mx5/board-mx51_efikamx.c 2010-11-22 23:04:23.000000000 +0100
> +++ gpionr/arch/arm/mach-mx5/board-mx51_efikamx.c 2010-11-22 23:04:25.000000000 +0100
> @@ -43,22 +43,22 @@
>
> #define MX51_USB_PLL_DIV_24_MHZ 0x01
>
> -#define EFIKAMX_PCBID0 (2*32 + 16)
> -#define EFIKAMX_PCBID1 (2*32 + 17)
> -#define EFIKAMX_PCBID2 (2*32 + 11)
> -
> -#define EFIKAMX_BLUE_LED (2*32 + 13)
> -#define EFIKAMX_GREEN_LED (2*32 + 14)
> -#define EFIKAMX_RED_LED (2*32 + 15)
> +#define EFIKAMX_PCBID0 MX51_GPIO_NR(3, 16)
> +#define EFIKAMX_PCBID1 MX51_GPIO_NR(3, 17)
> +#define EFIKAMX_PCBID2 MX51_GPIO_NR(3, 11)
> +
> +#define EFIKAMX_BLUE_LED MX51_GPIO_NR(3, 13)
> +#define EFIKAMX_GREEN_LED MX51_GPIO_NR(3, 14)
> +#define EFIKAMX_RED_LED MX51_GPIO_NR(3, 15)
>
> -#define EFIKAMX_POWER_KEY (1*32 + 31)
> +#define EFIKAMX_POWER_KEY MX51_GPIO_NR(2, 31)
>
> -#define EFIKAMX_SPI_CS0 (3*32 + 24)
> -#define EFIKAMX_SPI_CS1 (3*32 + 25)
> +#define EFIKAMX_SPI_CS0 MX51_GPIO_NR(4, 24)
> +#define EFIKAMX_SPI_CS1 MX51_GPIO_NR(4, 25)
>
> /* board 1.1 doesn't have same reset gpio */
> -#define EFIKAMX_RESET1_1 (2*32 + 2)
> -#define EFIKAMX_RESET (0*32 + 4)
> +#define EFIKAMX_RESET1_1 MX51_GPIO_NR(3, 2)
> +#define EFIKAMX_RESET MX51_GPIO_NR(1, 4)
>
> /* the pci ids pin have pull up. they're driven low according to board id */
> #define MX51_PAD_PCBID0 IOMUX_PAD(0x518, 0x130, 3, 0x0, 0, PAD_CTL_PUS_100K_UP)
> Index: gpionr/arch/arm/mach-mx5/board-mx51_babbage.c
> ===================================================================
> --- gpionr.orig/arch/arm/mach-mx5/board-mx51_babbage.c 2010-11-22 23:04:23.000000000 +0100
> +++ gpionr/arch/arm/mach-mx5/board-mx51_babbage.c 2010-11-22 23:04:25.000000000 +0100
> @@ -38,13 +38,13 @@
> #include "devices.h"
> #include "cpu_op-mx51.h"
>
> -#define BABBAGE_USB_HUB_RESET (0*32 + 7) /* GPIO_1_7 */
> -#define BABBAGE_USBH1_STP (0*32 + 27) /* GPIO_1_27 */
> -#define BABBAGE_PHY_RESET (1*32 + 5) /* GPIO_2_5 */
> -#define BABBAGE_FEC_PHY_RESET (1*32 + 14) /* GPIO_2_14 */
> -#define BABBAGE_POWER_KEY (1*32 + 21) /* GPIO_2_21 */
> -#define BABBAGE_ECSPI1_CS0 (3*32 + 24) /* GPIO_4_24 */
> -#define BABBAGE_ECSPI1_CS1 (3*32 + 25) /* GPIO_4_25 */
> +#define BABBAGE_USB_HUB_RESET MX51_GPIO_NR(1, 7)
> +#define BABBAGE_USBH1_STP MX51_GPIO_NR(1, 27)
> +#define BABBAGE_PHY_RESET MX51_GPIO_NR(2, 5)
> +#define BABBAGE_FEC_PHY_RESET MX51_GPIO_NR(2, 14)
> +#define BABBAGE_POWER_KEY MX51_GPIO_NR(2, 21)
> +#define BABBAGE_ECSPI1_CS0 MX51_GPIO_NR(4, 24)
> +#define BABBAGE_ECSPI1_CS1 MX51_GPIO_NR(4, 25)
>
> /* USB_CTRL_1 */
> #define MX51_USB_CTRL_1_OFFSET 0x10
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [patch 1/1] iMX51: introduce MX51_GPIO_NR
2010-11-24 5:21 ` Amit Kucheria
@ 2010-11-24 7:46 ` Lothar Waßmann
2010-11-24 16:28 ` Arnaud Patard (Rtp)
1 sibling, 0 replies; 9+ messages in thread
From: Lothar Waßmann @ 2010-11-24 7:46 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Amit Kucheria writes:
> On 10 Nov 22, Arnaud Patard wrote:
> > Currently, to define a GPIO number, we're using something like :
> >
> > #define EFIKAMX_PCBID0 (2*32 + 16)
> >
> > to define GPIO 3 16.
> >
> > This is not really readable and it's error prone imho (note the 3 vs 2).
> > So, I'm introducing a new macro to define this in a better way. Now, the
> > code sample become :
> >
> > #define EFIKAMX_PCBID0 MX51_GPIO_NR(3, 16)
> >
>
> I have been unhappy over the readability of the existing gpios definitions.
> So this patch is welcome. A couple of commments below.
>
> >
> > Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
> > Cc: Amit Kucheria <amit.kucheria@linaro.org>
> > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > Cc: Eric B?nard <eric@eukrea.com>
> >
> > Index: gpionr/arch/arm/mach-mx5/board-cpuimx51sd.c
> > ===================================================================
> > --- gpionr.orig/arch/arm/mach-mx5/board-cpuimx51sd.c 2010-11-22 23:04:23.000000000 +0100
> > +++ gpionr/arch/arm/mach-mx5/board-cpuimx51sd.c 2010-11-22 23:04:25.000000000 +0100
> > @@ -43,19 +43,19 @@
> > #include "devices-imx51.h"
> > #include "devices.h"
> >
> > -#define USBH1_RST (1*32 + 28)
> > -#define ETH_RST (1*32 + 31)
> > -#define TSC2007_IRQGPIO (2*32 + 12)
> > -#define CAN_IRQGPIO (0*32 + 1)
> > -#define CAN_RST (3*32 + 15)
> > -#define CAN_NCS (3*32 + 24)
> > -#define CAN_RXOBF (0*32 + 4)
> > -#define CAN_RX1BF (0*32 + 6)
> > -#define CAN_TXORTS (0*32 + 7)
> > -#define CAN_TX1RTS (0*32 + 8)
> > -#define CAN_TX2RTS (0*32 + 9)
> > -#define I2C_SCL (3*32 + 16)
> > -#define I2C_SDA (3*32 + 17)
> > +#define USBH1_RST MX51_GPIO_NR(2, 28)
> > +#define ETH_RST MX51_GPIO_NR(2, 31)
> > +#define TSC2007_IRQGPIO MX51_GPIO_NR(3, 12)
> > +#define CAN_IRQGPIO MX51_GPIO_NR(1, 1)
> > +#define CAN_RST MX51_GPIO_NR(4, 15)
> > +#define CAN_NCS MX51_GPIO_NR(4, 24)
> > +#define CAN_RXOBF MX51_GPIO_NR(1, 4)
> > +#define CAN_RX1BF MX51_GPIO_NR(1, 6)
> > +#define CAN_TXORTS MX51_GPIO_NR(1, 7)
> > +#define CAN_TX1RTS MX51_GPIO_NR(1, 8)
> > +#define CAN_TX2RTS MX51_GPIO_NR(1, 9)
> > +#define I2C_SCL MX51_GPIO_NR(4, 16)
> > +#define I2C_SDA MX51_GPIO_NR(4, 17)
> >
> > /* USB_CTRL_1 */
> > #define MX51_USB_CTRL_1_OFFSET 0x10
> > @@ -243,7 +243,7 @@
> > .mode = SPI_MODE_0,
> > .chip_select = 0,
> > .platform_data = &mcp251x_info,
> > - .irq = gpio_to_irq(0 * 32 + 1)
> > + .irq = gpio_to_irq(CAN_IRQGPIO)
> > },
> > };
> >
> > Index: gpionr/arch/arm/plat-mxc/include/mach/iomux-mx51.h
> > ===================================================================
> > --- gpionr.orig/arch/arm/plat-mxc/include/mach/iomux-mx51.h 2010-11-22 23:04:23.000000000 +0100
> > +++ gpionr/arch/arm/plat-mxc/include/mach/iomux-mx51.h 2010-11-22 23:04:25.000000000 +0100
> > @@ -15,6 +15,8 @@
> >
> > #include <mach/iomux-v3.h>
> >
>
> Please add a comment here that bank numbers start from 1 to make it explicit
>
> > +#define MX51_GPIO_NR(bank, nr) ((bank-1)*32+nr)
> > +
> ^^^^^^^^^ whitespace fixes
>
there should be parens around the macro parameters: (((bank) - 1) * 32 + (nr))
Lothar Wa?mann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________
^ permalink raw reply [flat|nested] 9+ messages in thread
* [patch 1/1] iMX51: introduce MX51_GPIO_NR
2010-11-24 5:21 ` Amit Kucheria
2010-11-24 7:46 ` Lothar Waßmann
@ 2010-11-24 16:28 ` Arnaud Patard (Rtp)
1 sibling, 0 replies; 9+ messages in thread
From: Arnaud Patard (Rtp) @ 2010-11-24 16:28 UTC (permalink / raw)
To: linux-arm-kernel
Amit Kucheria <amit.kucheria@linaro.org> writes:
[...]
>> Index: gpionr/arch/arm/plat-mxc/include/mach/iomux-mx51.h
>> ===================================================================
>> --- gpionr.orig/arch/arm/plat-mxc/include/mach/iomux-mx51.h 2010-11-22 23:04:23.000000000 +0100
>> +++ gpionr/arch/arm/plat-mxc/include/mach/iomux-mx51.h 2010-11-22 23:04:25.000000000 +0100
>> @@ -15,6 +15,8 @@
>>
>> #include <mach/iomux-v3.h>
>>
>
> Please add a comment here that bank numbers start from 1 to make it explicit
>
ok.
>> +#define MX51_GPIO_NR(bank, nr) ((bank-1)*32+nr)
>> +
> ^^^^^^^^^ whitespace fixes
>
> Also, why is this in iomux-mx51.h?
Because I thought it was a good place to put it. fwiw, I've no strong opinion
on where to put it.
> It might be better to introduce a gpio.h
Introduce a gpio.h ? There's already one: arch/arm/plat-mxc/include/mach/gpio.h
So, you prefer to see this macro in it ?
Arnaud
^ permalink raw reply [flat|nested] 9+ messages in thread
* [patch 1/1] iMX51: introduce MX51_GPIO_NR
2010-11-22 22:45 [patch 1/1] iMX51: introduce MX51_GPIO_NR Arnaud Patard (Rtp)
2010-11-24 5:21 ` Amit Kucheria
@ 2010-11-24 16:49 ` Fabio Estevam
2010-11-24 17:04 ` Arnaud Patard (Rtp)
1 sibling, 1 reply; 9+ messages in thread
From: Fabio Estevam @ 2010-11-24 16:49 UTC (permalink / raw)
To: linux-arm-kernel
Hi Arnaud,
2010/11/22 Arnaud Patard <arnaud.patard@rtp-net.org>:
> Currently, to define a GPIO number, we're using something like :
>
> #define EFIKAMX_PCBID0 ? ? ? ? (2*32 + 16)
>
> to define GPIO 3 16.
>
> This is not really readable and it's error prone imho (note the 3 vs 2).
> So, I'm introducing a new macro to define this in a better way. Now, the
> code sample become :
>
> #define EFIKAMX_PCBID0 ? ? ? ? MX51_GPIO_NR(3, 16)
Can you rename the macro to MX5x_GPIO_NR instead of MX51_GPIO_NR?
This way we can also use this macro for MX53 and MX508 when they show
up in mainline.
Regards,
Fabio Estevam
^ permalink raw reply [flat|nested] 9+ messages in thread
* [patch 1/1] iMX51: introduce MX51_GPIO_NR
2010-11-24 16:49 ` Fabio Estevam
@ 2010-11-24 17:04 ` Arnaud Patard (Rtp)
2010-11-24 23:06 ` Richard Zhao
2010-11-25 8:24 ` Sascha Hauer
0 siblings, 2 replies; 9+ messages in thread
From: Arnaud Patard (Rtp) @ 2010-11-24 17:04 UTC (permalink / raw)
To: linux-arm-kernel
Fabio Estevam <festevam@gmail.com> writes:
Hi,
> Hi Arnaud,
>
> 2010/11/22 Arnaud Patard <arnaud.patard@rtp-net.org>:
>> Currently, to define a GPIO number, we're using something like :
>>
>> #define EFIKAMX_PCBID0 ? ? ? ? (2*32 + 16)
>>
>> to define GPIO 3 16.
>>
>> This is not really readable and it's error prone imho (note the 3 vs 2).
>> So, I'm introducing a new macro to define this in a better way. Now, the
>> code sample become :
>>
>> #define EFIKAMX_PCBID0 ? ? ? ? MX51_GPIO_NR(3, 16)
>
> Can you rename the macro to MX5x_GPIO_NR instead of MX51_GPIO_NR?
>
> This way we can also use this macro for MX53 and MX508 when they show
> up in mainline.
I've been wondering about to use MX5X instead of MX51 but I kept MX51
because I didn't know how the GPIO will work on MX53. If they're
compatible, you're right, the name should be MX5X_GPIO_NR and not
MX51_GPIO_NR. Assuming you mail means that, I'm going to switch to
MX5X_GPIO_NR.
Arnaud
^ permalink raw reply [flat|nested] 9+ messages in thread
* [patch 1/1] iMX51: introduce MX51_GPIO_NR
2010-11-24 17:04 ` Arnaud Patard (Rtp)
@ 2010-11-24 23:06 ` Richard Zhao
2010-11-25 8:24 ` Sascha Hauer
1 sibling, 0 replies; 9+ messages in thread
From: Richard Zhao @ 2010-11-24 23:06 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Nov 25, 2010 at 1:04 AM, Arnaud Patard
<arnaud.patard@rtp-net.org> wrote:
> Fabio Estevam <festevam@gmail.com> writes:
>
> Hi,
>
>> Hi Arnaud,
>>
>> 2010/11/22 Arnaud Patard <arnaud.patard@rtp-net.org>:
>>> Currently, to define a GPIO number, we're using something like :
>>>
>>> #define EFIKAMX_PCBID0 ? ? ? ? (2*32 + 16)
>>>
>>> to define GPIO 3 16.
>>>
>>> This is not really readable and it's error prone imho (note the 3 vs 2).
>>> So, I'm introducing a new macro to define this in a better way. Now, the
>>> code sample become :
>>>
>>> #define EFIKAMX_PCBID0 ? ? ? ? MX51_GPIO_NR(3, 16)
>>
>> Can you rename the macro to MX5x_GPIO_NR instead of MX51_GPIO_NR?
>>
>> This way we can also use this macro for MX53 and MX508 when they show
>> up in mainline.
>
> I've been wondering about to use MX5X instead of MX51 but I kept MX51
> because I didn't know how the GPIO will work on MX53. If they're
> compatible, you're right, the name should be MX5X_GPIO_NR and not
> MX51_GPIO_NR. Assuming you mail means that, I'm going to switch to
> MX5X_GPIO_NR.
For MX5x at lease for now (MX51/53/508), One bank is 32 gpios. But
bank number is different.
Thanks
Richard
^ permalink raw reply [flat|nested] 9+ messages in thread
* [patch 1/1] iMX51: introduce MX51_GPIO_NR
2010-11-24 17:04 ` Arnaud Patard (Rtp)
2010-11-24 23:06 ` Richard Zhao
@ 2010-11-25 8:24 ` Sascha Hauer
2010-11-25 8:52 ` Sascha Hauer
1 sibling, 1 reply; 9+ messages in thread
From: Sascha Hauer @ 2010-11-25 8:24 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Nov 24, 2010 at 06:04:09PM +0100, Arnaud Patard wrote:
> Fabio Estevam <festevam@gmail.com> writes:
>
> Hi,
>
> > Hi Arnaud,
> >
> > 2010/11/22 Arnaud Patard <arnaud.patard@rtp-net.org>:
> >> Currently, to define a GPIO number, we're using something like :
> >>
> >> #define EFIKAMX_PCBID0 ? ? ? ? (2*32 + 16)
> >>
> >> to define GPIO 3 16.
> >>
> >> This is not really readable and it's error prone imho (note the 3 vs 2).
> >> So, I'm introducing a new macro to define this in a better way. Now, the
> >> code sample become :
> >>
> >> #define EFIKAMX_PCBID0 ? ? ? ? MX51_GPIO_NR(3, 16)
> >
> > Can you rename the macro to MX5x_GPIO_NR instead of MX51_GPIO_NR?
> >
> > This way we can also use this macro for MX53 and MX508 when they show
> > up in mainline.
>
> I've been wondering about to use MX5X instead of MX51 but I kept MX51
> because I didn't know how the GPIO will work on MX53. If they're
> compatible, you're right, the name should be MX5X_GPIO_NR and not
> MX51_GPIO_NR. Assuming you mail means that, I'm going to switch to
> MX5X_GPIO_NR.
So far all i.MX use the same gpio numbering and I see no reason why this
should be changed in future SoCs, so I would go for a IMX_GPIO_NR
instead of SoC specific variants.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 9+ messages in thread
* [patch 1/1] iMX51: introduce MX51_GPIO_NR
2010-11-25 8:24 ` Sascha Hauer
@ 2010-11-25 8:52 ` Sascha Hauer
0 siblings, 0 replies; 9+ messages in thread
From: Sascha Hauer @ 2010-11-25 8:52 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Nov 25, 2010 at 09:24:56AM +0100, Sascha Hauer wrote:
> On Wed, Nov 24, 2010 at 06:04:09PM +0100, Arnaud Patard wrote:
> > Fabio Estevam <festevam@gmail.com> writes:
> >
> > Hi,
> >
> > > Hi Arnaud,
> > >
> > > 2010/11/22 Arnaud Patard <arnaud.patard@rtp-net.org>:
> > >> Currently, to define a GPIO number, we're using something like :
> > >>
> > >> #define EFIKAMX_PCBID0 ? ? ? ? (2*32 + 16)
> > >>
> > >> to define GPIO 3 16.
> > >>
> > >> This is not really readable and it's error prone imho (note the 3 vs 2).
> > >> So, I'm introducing a new macro to define this in a better way. Now, the
> > >> code sample become :
> > >>
> > >> #define EFIKAMX_PCBID0 ? ? ? ? MX51_GPIO_NR(3, 16)
> > >
> > > Can you rename the macro to MX5x_GPIO_NR instead of MX51_GPIO_NR?
> > >
> > > This way we can also use this macro for MX53 and MX508 when they show
> > > up in mainline.
> >
> > I've been wondering about to use MX5X instead of MX51 but I kept MX51
> > because I didn't know how the GPIO will work on MX53. If they're
> > compatible, you're right, the name should be MX5X_GPIO_NR and not
> > MX51_GPIO_NR. Assuming you mail means that, I'm going to switch to
> > MX5X_GPIO_NR.
>
> So far all i.MX use the same gpio numbering and I see no reason why this
> should be changed in future SoCs, so I would go for a IMX_GPIO_NR
> instead of SoC specific variants.
Ok, we are talking about Freescale here, but in this very special case I'm
confident they won't change it. I hope none of the hardware engineers
reads this, I'm sure they are always looking for good ideas to make
the life of the software developers harder.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-11-25 8:52 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-22 22:45 [patch 1/1] iMX51: introduce MX51_GPIO_NR Arnaud Patard (Rtp)
2010-11-24 5:21 ` Amit Kucheria
2010-11-24 7:46 ` Lothar Waßmann
2010-11-24 16:28 ` Arnaud Patard (Rtp)
2010-11-24 16:49 ` Fabio Estevam
2010-11-24 17:04 ` Arnaud Patard (Rtp)
2010-11-24 23:06 ` Richard Zhao
2010-11-25 8:24 ` Sascha Hauer
2010-11-25 8:52 ` Sascha Hauer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).