linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [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).