[PATCH 1/1] arm: imx: share imx5x_register_gpios for mach-mx5

[PATCH 1/1] arm: imx: share imx5x_register_gpios for mach-mx5

[Uwe Kleine-König]

Hello Richard,

On Thu, Nov 25, 2010 at 10:28:32PM +0800, Richard Zhao wrote:
> From: Richard Zhao <richard.zhao@freescale.com>
> 
> Add variable mx5x_gpio_port_cnt, and init in machine init function.
> It can be share by mx51, mx53 and coming mx50.
> 
> Signed-off-by: Richard Zhao <linuxzsc@gmail.com>
> 
> diff --git a/arch/arm/mach-mx5/board-cpuimx51.c b/arch/arm/mach-mx5/board-cpuimx51.c
> index 5ff5522..cd7bd4c 100644
> --- a/arch/arm/mach-mx5/board-cpuimx51.c
> +++ b/arch/arm/mach-mx5/board-cpuimx51.c
> @@ -242,6 +242,8 @@ __setup("otg_mode=", eukrea_cpuimx51_otg_mode);
>   */
>  static void __init eukrea_cpuimx51_init(void)
>  {
> +	mx5x_gpio_port_cnt = 4;
> +
IMO the machine files shouldn't need to know how many gpio banks are
available.  This is a per-SoC thing and so should be set in a SoC function.

How do you want to continue this change?  mx51 and mx53 have the same
number of ports?  Does mx50 have more or less?  The addresses are the
same?

Best regards
Uwe

>  	mxc_iomux_v3_setup_multiple_pads(eukrea_cpuimx51_pads,
>  					ARRAY_SIZE(eukrea_cpuimx51_pads));
>  
> diff --git a/arch/arm/mach-mx5/board-cpuimx51sd.c b/arch/arm/mach-mx5/board-cpuimx51sd.c
> index ff1f45e..e47345a 100644
> --- a/arch/arm/mach-mx5/board-cpuimx51sd.c
> +++ b/arch/arm/mach-mx5/board-cpuimx51sd.c
> @@ -262,6 +262,8 @@ static struct platform_device *platform_devices[] __initdata = {
>  
>  static void __init eukrea_cpuimx51sd_init(void)
>  {
> +	mx5x_gpio_port_cnt = 4;
> +
>  	mxc_iomux_v3_setup_multiple_pads(eukrea_cpuimx51sd_pads,
>  					ARRAY_SIZE(eukrea_cpuimx51sd_pads));
>  
> diff --git a/arch/arm/mach-mx5/board-mx51_3ds.c b/arch/arm/mach-mx5/board-mx51_3ds.c
> index 6cc2805..cb22fc3 100644
> --- a/arch/arm/mach-mx5/board-mx51_3ds.c
> +++ b/arch/arm/mach-mx5/board-mx51_3ds.c
> @@ -160,6 +160,8 @@ static struct spi_board_info mx51_3ds_spi_nor_device[] = {
>   */
>  static void __init mxc_board_init(void)
>  {
> +	mx5x_gpio_port_cnt = 4;
> +
>  	mxc_iomux_v3_setup_multiple_pads(mx51_3ds_pads,
>  					ARRAY_SIZE(mx51_3ds_pads));
>  	mxc_init_imx_uart();
> diff --git a/arch/arm/mach-mx5/board-mx51_babbage.c b/arch/arm/mach-mx5/board-mx51_babbage.c
> index ef32843..8054c09 100644
> --- a/arch/arm/mach-mx5/board-mx51_babbage.c
> +++ b/arch/arm/mach-mx5/board-mx51_babbage.c
> @@ -347,9 +347,12 @@ static const struct spi_imx_master mx51_babbage_spi_pdata __initconst = {
>   */
>  static void __init mxc_board_init(void)
>  {
> +	mx5x_gpio_port_cnt = 4;
> +
>  	iomux_v3_cfg_t usbh1stp = MX51_PAD_USBH1_STP__USBH1_STP;
>  	iomux_v3_cfg_t power_key = MX51_PAD_EIM_A27__GPIO_2_21;
>  
> +	mx5x_gpio_port_cnt = 4;
>  #if defined(CONFIG_CPU_FREQ_IMX)
>  	get_cpu_op = mx51_get_cpu_op;
>  #endif
> diff --git a/arch/arm/mach-mx5/board-mx51_efikamx.c b/arch/arm/mach-mx5/board-mx51_efikamx.c
> index 4b2718b..cf315cb 100644
> --- a/arch/arm/mach-mx5/board-mx51_efikamx.c
> +++ b/arch/arm/mach-mx5/board-mx51_efikamx.c
> @@ -314,6 +314,8 @@ void mx51_efikamx_reset(void)
>  
>  static void __init mxc_board_init(void)
>  {
> +	mx5x_gpio_port_cnt = 4;
> +
>  	mxc_iomux_v3_setup_multiple_pads(mx51efikamx_pads,
>  					ARRAY_SIZE(mx51efikamx_pads));
>  	mx51_efikamx_board_id();
> diff --git a/arch/arm/mach-mx5/devices.c b/arch/arm/mach-mx5/devices.c
> index d926203..b8371d2 100644
> --- a/arch/arm/mach-mx5/devices.c
> +++ b/arch/arm/mach-mx5/devices.c
> @@ -183,13 +183,11 @@ static struct mxc_gpio_port mxc_gpio_ports[] = {
>  	},
>  };
>  
> -int __init imx51_register_gpios(void)
> -{
> -	return mxc_gpio_init(mxc_gpio_ports, 4);
> -}
> +int mx5x_gpio_port_cnt;
>  
> -int __init imx53_register_gpios(void)
> +int __init imx5x_register_gpios(void)
>  {
> -	return mxc_gpio_init(mxc_gpio_ports, ARRAY_SIZE(mxc_gpio_ports));
> +	if (mx5x_gpio_port_cnt)
> +		return mxc_gpio_init(mxc_gpio_ports, mx5x_gpio_port_cnt);
> +	return -EINVAL;
>  }
> -
> diff --git a/arch/arm/mach-mx5/devices.h b/arch/arm/mach-mx5/devices.h
> index af1d07c..d3411dc 100644
> --- a/arch/arm/mach-mx5/devices.h
> +++ b/arch/arm/mach-mx5/devices.h
> @@ -4,3 +4,5 @@ extern struct platform_device mxc_usbdr_udc_device;
>  extern struct platform_device mxc_wdt;
>  extern struct platform_device mxc_hsi2c_device;
>  extern struct platform_device mxc_keypad_device;
> +
> +extern int mx5x_gpio_port_cnt;
> diff --git a/arch/arm/mach-mx5/mm.c b/arch/arm/mach-mx5/mm.c
> index e57f968..b0ea57b 100644
> --- a/arch/arm/mach-mx5/mm.c
> +++ b/arch/arm/mach-mx5/mm.c
> @@ -61,7 +61,7 @@ void __init mx53_map_io(void)
>  	iotable_init(mx53_io_desc, ARRAY_SIZE(mx53_io_desc));
>  }
>  
> -int imx51_register_gpios(void);
> +int imx5x_register_gpios(void);
>  
>  void __init mx51_init_irq(void)
>  {
> @@ -78,11 +78,9 @@ void __init mx51_init_irq(void)
>  		panic("unable to map TZIC interrupt controller\n");
>  
>  	tzic_init_irq(tzic_virt);
> -	imx51_register_gpios();
> +	imx5x_register_gpios();
>  }
>  
> -int imx53_register_gpios(void);
> -
>  void __init mx53_init_irq(void)
>  {
>  	unsigned long tzic_addr;
> @@ -95,5 +93,5 @@ void __init mx53_init_irq(void)
>  		panic("unable to map TZIC interrupt controller\n");
>  
>  	tzic_init_irq(tzic_virt);
> -	imx53_register_gpios();
> +	imx5x_register_gpios();
>  }
> -- 
> 1.7.1
> 
> 
> 

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH 1/1] arm: imx: share imx5x_register_gpios for mach-mx5

[Richard Zhao]

> IMO the machine files shouldn't need to know how many gpio banks are
> available. ?This is a per-SoC thing and so should be set in a SoC function.
But we don't have SoC level device file.
>
> How do you want to continue this change? ?mx51 and mx53 have the same
> number of ports? ?Does mx50 have more or less? ?The addresses are the
> same?
mx53 and mx50 have similar memory map. but mx51 is different.  Sorry,
I forgot upstream version IO_ADDR can not handler differnt SoC base
addr offset. It can not be shared with mx51, but it can be shared with
mx50/53, correct? May plan is mx53/50 share
arch/arm/plat-mxc/include/mach/mx5x.h file.
It seems mx53 gpio don't work here. It can not share mxc_gpio_ports with mx50.
>
> Best regards
> Uwe
>
>> ? ? ? mxc_iomux_v3_setup_multiple_pads(eukrea_cpuimx51_pads,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ARRAY_SIZE(eukrea_cpuimx51_pads));
>>
>> diff --git a/arch/arm/mach-mx5/board-cpuimx51sd.c b/arch/arm/mach-mx5/board-cpuimx51sd.c
>> index ff1f45e..e47345a 100644
>> --- a/arch/arm/mach-mx5/board-cpuimx51sd.c
>> +++ b/arch/arm/mach-mx5/board-cpuimx51sd.c
>> @@ -262,6 +262,8 @@ static struct platform_device *platform_devices[] __initdata = {
>>
>> ?static void __init eukrea_cpuimx51sd_init(void)
>> ?{
>> + ? ? mx5x_gpio_port_cnt = 4;
>> +
>> ? ? ? mxc_iomux_v3_setup_multiple_pads(eukrea_cpuimx51sd_pads,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ARRAY_SIZE(eukrea_cpuimx51sd_pads));
>>
>> diff --git a/arch/arm/mach-mx5/board-mx51_3ds.c b/arch/arm/mach-mx5/board-mx51_3ds.c
>> index 6cc2805..cb22fc3 100644
>> --- a/arch/arm/mach-mx5/board-mx51_3ds.c
>> +++ b/arch/arm/mach-mx5/board-mx51_3ds.c
>> @@ -160,6 +160,8 @@ static struct spi_board_info mx51_3ds_spi_nor_device[] = {
>> ? */
>> ?static void __init mxc_board_init(void)
>> ?{
>> + ? ? mx5x_gpio_port_cnt = 4;
>> +
>> ? ? ? mxc_iomux_v3_setup_multiple_pads(mx51_3ds_pads,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ARRAY_SIZE(mx51_3ds_pads));
>> ? ? ? mxc_init_imx_uart();
>> diff --git a/arch/arm/mach-mx5/board-mx51_babbage.c b/arch/arm/mach-mx5/board-mx51_babbage.c
>> index ef32843..8054c09 100644
>> --- a/arch/arm/mach-mx5/board-mx51_babbage.c
>> +++ b/arch/arm/mach-mx5/board-mx51_babbage.c
>> @@ -347,9 +347,12 @@ static const struct spi_imx_master mx51_babbage_spi_pdata __initconst = {
>> ? */
>> ?static void __init mxc_board_init(void)
>> ?{
>> + ? ? mx5x_gpio_port_cnt = 4;
>> +
>> ? ? ? iomux_v3_cfg_t usbh1stp = MX51_PAD_USBH1_STP__USBH1_STP;
>> ? ? ? iomux_v3_cfg_t power_key = MX51_PAD_EIM_A27__GPIO_2_21;
>>
>> + ? ? mx5x_gpio_port_cnt = 4;
>> ?#if defined(CONFIG_CPU_FREQ_IMX)
>> ? ? ? get_cpu_op = mx51_get_cpu_op;
>> ?#endif
>> diff --git a/arch/arm/mach-mx5/board-mx51_efikamx.c b/arch/arm/mach-mx5/board-mx51_efikamx.c
>> index 4b2718b..cf315cb 100644
>> --- a/arch/arm/mach-mx5/board-mx51_efikamx.c
>> +++ b/arch/arm/mach-mx5/board-mx51_efikamx.c
>> @@ -314,6 +314,8 @@ void mx51_efikamx_reset(void)
>>
>> ?static void __init mxc_board_init(void)
>> ?{
>> + ? ? mx5x_gpio_port_cnt = 4;
>> +
>> ? ? ? mxc_iomux_v3_setup_multiple_pads(mx51efikamx_pads,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ARRAY_SIZE(mx51efikamx_pads));
>> ? ? ? mx51_efikamx_board_id();
>> diff --git a/arch/arm/mach-mx5/devices.c b/arch/arm/mach-mx5/devices.c
>> index d926203..b8371d2 100644
>> --- a/arch/arm/mach-mx5/devices.c
>> +++ b/arch/arm/mach-mx5/devices.c
>> @@ -183,13 +183,11 @@ static struct mxc_gpio_port mxc_gpio_ports[] = {
>> ? ? ? },
>> ?};
>>
>> -int __init imx51_register_gpios(void)
>> -{
>> - ? ? return mxc_gpio_init(mxc_gpio_ports, 4);
>> -}
>> +int mx5x_gpio_port_cnt;
>>
>> -int __init imx53_register_gpios(void)
>> +int __init imx5x_register_gpios(void)
>> ?{
>> - ? ? return mxc_gpio_init(mxc_gpio_ports, ARRAY_SIZE(mxc_gpio_ports));
>> + ? ? if (mx5x_gpio_port_cnt)
>> + ? ? ? ? ? ? return mxc_gpio_init(mxc_gpio_ports, mx5x_gpio_port_cnt);
>> + ? ? return -EINVAL;
>> ?}
>> -
>> diff --git a/arch/arm/mach-mx5/devices.h b/arch/arm/mach-mx5/devices.h
>> index af1d07c..d3411dc 100644
>> --- a/arch/arm/mach-mx5/devices.h
>> +++ b/arch/arm/mach-mx5/devices.h
>> @@ -4,3 +4,5 @@ extern struct platform_device mxc_usbdr_udc_device;
>> ?extern struct platform_device mxc_wdt;
>> ?extern struct platform_device mxc_hsi2c_device;
>> ?extern struct platform_device mxc_keypad_device;
>> +
>> +extern int mx5x_gpio_port_cnt;
>> diff --git a/arch/arm/mach-mx5/mm.c b/arch/arm/mach-mx5/mm.c
>> index e57f968..b0ea57b 100644
>> --- a/arch/arm/mach-mx5/mm.c
>> +++ b/arch/arm/mach-mx5/mm.c
>> @@ -61,7 +61,7 @@ void __init mx53_map_io(void)
>> ? ? ? iotable_init(mx53_io_desc, ARRAY_SIZE(mx53_io_desc));
>> ?}
>>
>> -int imx51_register_gpios(void);
>> +int imx5x_register_gpios(void);
>>
>> ?void __init mx51_init_irq(void)
>> ?{
>> @@ -78,11 +78,9 @@ void __init mx51_init_irq(void)
>> ? ? ? ? ? ? ? panic("unable to map TZIC interrupt controller\n");
>>
>> ? ? ? tzic_init_irq(tzic_virt);
>> - ? ? imx51_register_gpios();
>> + ? ? imx5x_register_gpios();
>> ?}
>>
>> -int imx53_register_gpios(void);
>> -
>> ?void __init mx53_init_irq(void)
>> ?{
>> ? ? ? unsigned long tzic_addr;
>> @@ -95,5 +93,5 @@ void __init mx53_init_irq(void)
>> ? ? ? ? ? ? ? panic("unable to map TZIC interrupt controller\n");
>>
>> ? ? ? tzic_init_irq(tzic_virt);
>> - ? ? imx53_register_gpios();
>> + ? ? imx5x_register_gpios();
>> ?}
>> --
>> 1.7.1
>>
>>
>>
>
> --
> Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? | Uwe Kleine-K?nig ? ? ? ? ? ?|
> Industrial Linux Solutions ? ? ? ? ? ? ? ? | http://www.pengutronix.de/ ?|
>

[PATCH 1/1] arm: imx: share imx5x_register_gpios for mach-mx5

[Uwe Kleine-König]

Hey Richard,

On Thu, Nov 25, 2010 at 02:44:48PM +0800, Richard Zhao wrote:
> > IMO the machine files shouldn't need to know how many gpio banks are
> > available. ?This is a per-SoC thing and so should be set in a SoC function.
> But we don't have SoC level device file.
Check how I did it for imx{1,21,27,25} in arch/arm/plat-mxc/gpio.c.

> > How do you want to continue this change? ?mx51 and mx53 have the same
> > number of ports? ?Does mx50 have more or less? ?The addresses are the
> > same?
> mx53 and mx50 have similar memory map. but mx51 is different.  Sorry,
> I forgot upstream version IO_ADDR can not handler differnt SoC base
> addr offset.
It might even work in this case, still I prefer a more explicit way.

>              It can not be shared with mx51, but it can be shared with
> mx50/53, correct? May plan is mx53/50 share
> arch/arm/plat-mxc/include/mach/mx5x.h file.
mx5x.h is for mx50 and mx53 but not mx51?  No please.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH 1/1] arm: imx: share imx5x_register_gpios for mach-mx5

[Richard Zhao]

2010/11/25 Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
> Hey Richard,
>
> On Thu, Nov 25, 2010 at 02:44:48PM +0800, Richard Zhao wrote:
>> > IMO the machine files shouldn't need to know how many gpio banks are
>> > available. ?This is a per-SoC thing and so should be set in a SoC function.
>> But we don't have SoC level device file.
> Check how I did it for imx{1,21,27,25} in arch/arm/plat-mxc/gpio.c.
I considered put it there too. but we can not add new defconfig, and
even can not create new items in Kconfig. So we can not use ifdefs.
And per my understanding, mx5x is going to use one zImage, correct?
>
>> > How do you want to continue this change? ?mx51 and mx53 have the same
>> > number of ports? ?Does mx50 have more or less? ?The addresses are the
>> > same?
>> mx53 and mx50 have similar memory map. but mx51 is different. ?Sorry,
>> I forgot upstream version IO_ADDR can not handler differnt SoC base
>> addr offset.
> It might even work in this case, still I prefer a more explicit way.
>
>> ? ? ? ? ? ? ?It can not be shared with mx51, but it can be shared with
>> mx50/53, correct? May plan is mx53/50 share
>> arch/arm/plat-mxc/include/mach/mx5x.h file.
> mx5x.h is for mx50 and mx53 but not mx51? ?No please.
For memory map, mx51 is a special case. From mx53 on , IC tuned memory
map to support large memory.
If we use things like mx53-50.h, symbols is hard to define.
MX53_50_XXX_XXX is wired. Any possibly, we need to add new SoC with
similar memory map in the future.

Thanks
Richard
>
> Best regards
> Uwe
>
> --
> Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? | Uwe Kleine-K?nig ? ? ? ? ? ?|
> Industrial Linux Solutions ? ? ? ? ? ? ? ? | http://www.pengutronix.de/ ?|
>

[PATCH 1/1] arm: imx: share imx5x_register_gpios for mach-mx5

[Uwe Kleine-König]

On Thu, Nov 25, 2010 at 04:01:04PM +0800, Richard Zhao wrote:
> 2010/11/25 Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
> > Hey Richard,
> >
> > On Thu, Nov 25, 2010 at 02:44:48PM +0800, Richard Zhao wrote:
> >> > IMO the machine files shouldn't need to know how many gpio banks are
> >> > available. ?This is a per-SoC thing and so should be set in a SoC function.
> >> But we don't have SoC level device file.
> > Check how I did it for imx{1,21,27,25} in arch/arm/plat-mxc/gpio.c.
> I considered put it there too. but we can not add new defconfig, and
> even can not create new items in Kconfig. So we can not use ifdefs.
Why not add new Kconfig symbols?  Up to now I was unfettered by scruples
adding new ones.

> And per my understanding, mx5x is going to use one zImage, correct?
It should be possible to have a single image for more than one SoC, yes.

> >> > How do you want to continue this change? ?mx51 and mx53 have the same
> >> > number of ports? ?Does mx50 have more or less? ?The addresses are the
> >> > same?
> >> mx53 and mx50 have similar memory map. but mx51 is different. ?Sorry,
> >> I forgot upstream version IO_ADDR can not handler differnt SoC base
> >> addr offset.
> > It might even work in this case, still I prefer a more explicit way.
> >
> >> ? ? ? ? ? ? ?It can not be shared with mx51, but it can be shared with
> >> mx50/53, correct? May plan is mx53/50 share
> >> arch/arm/plat-mxc/include/mach/mx5x.h file.
> > mx5x.h is for mx50 and mx53 but not mx51? ?No please.
> For memory map, mx51 is a special case. From mx53 on , IC tuned memory
> map to support large memory.
> If we use things like mx53-50.h, symbols is hard to define.
> MX53_50_XXX_XXX is wired. Any possibly, we need to add new SoC with
> similar memory map in the future.
I still think defining symbols for a single SoC is fine most of the
time.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH 1/1] arm: imx: share imx5x_register_gpios for mach-mx5

[Sascha Hauer]

On Thu, Nov 25, 2010 at 09:14:06AM +0100, Uwe Kleine-K?nig wrote:
> On Thu, Nov 25, 2010 at 04:01:04PM +0800, Richard Zhao wrote:
> > 2010/11/25 Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
> > > Hey Richard,
> > >
> > > On Thu, Nov 25, 2010 at 02:44:48PM +0800, Richard Zhao wrote:
> > >> > IMO the machine files shouldn't need to know how many gpio banks are
> > >> > available. ?This is a per-SoC thing and so should be set in a SoC function.
> > >> But we don't have SoC level device file.
> > > Check how I did it for imx{1,21,27,25} in arch/arm/plat-mxc/gpio.c.
> > I considered put it there too. but we can not add new defconfig, and
> > even can not create new items in Kconfig. So we can not use ifdefs.
> Why not add new Kconfig symbols?  Up to now I was unfettered by scruples
> adding new ones.
> 
> > And per my understanding, mx5x is going to use one zImage, correct?
> It should be possible to have a single image for more than one SoC, yes.
> 
> > >> > How do you want to continue this change? ?mx51 and mx53 have the same
> > >> > number of ports? ?Does mx50 have more or less? ?The addresses are the
> > >> > same?
> > >> mx53 and mx50 have similar memory map. but mx51 is different. ?Sorry,
> > >> I forgot upstream version IO_ADDR can not handler differnt SoC base
> > >> addr offset.
> > > It might even work in this case, still I prefer a more explicit way.
> > >
> > >> ? ? ? ? ? ? ?It can not be shared with mx51, but it can be shared with
> > >> mx50/53, correct? May plan is mx53/50 share
> > >> arch/arm/plat-mxc/include/mach/mx5x.h file.
> > > mx5x.h is for mx50 and mx53 but not mx51? ?No please.
> > For memory map, mx51 is a special case. From mx53 on , IC tuned memory
> > map to support large memory.
> > If we use things like mx53-50.h, symbols is hard to define.
> > MX53_50_XXX_XXX is wired. Any possibly, we need to add new SoC with
> > similar memory map in the future.
> I still think defining symbols for a single SoC is fine most of the
> time.

+1

MX2X was confusing when the i.MX25 came out and we won't start *any*
MX5X things when we already know that it's wrong. Go with MX51_ MX53_
MX50_ or whatever the SoC of the day is. Putting things together to
share the code is done in the upper layers, *not* the register defines.

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 |

[PATCH 1/1] arm: imx: share imx5x_register_gpios for mach-mx5

[Richard Zhao]

2010/11/25 Sascha Hauer <s.hauer@pengutronix.de>:
> On Thu, Nov 25, 2010 at 09:14:06AM +0100, Uwe Kleine-K?nig wrote:
>> On Thu, Nov 25, 2010 at 04:01:04PM +0800, Richard Zhao wrote:
>> > 2010/11/25 Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
>> > > Hey Richard,
>> > >
>> > > On Thu, Nov 25, 2010 at 02:44:48PM +0800, Richard Zhao wrote:
>> > >> > IMO the machine files shouldn't need to know how many gpio banks are
>> > >> > available. ?This is a per-SoC thing and so should be set in a SoC function.
>> > >> But we don't have SoC level device file.
>> > > Check how I did it for imx{1,21,27,25} in arch/arm/plat-mxc/gpio.c.
>> > I considered put it there too. but we can not add new defconfig, and
>> > even can not create new items in Kconfig. So we can not use ifdefs.
>> Why not add new Kconfig symbols? ?Up to now I was unfettered by scruples
>> adding new ones.
>>
>> > And per my understanding, mx5x is going to use one zImage, correct?
>> It should be possible to have a single image for more than one SoC, yes.
Thanks. I go through the code again. Here is mx50 adding plan:
1. basic memory map definitions
Add it in individual arch/arm/plat-mxc/include/mach/mx50.h
2. clock.c crm_regs.h
clock tree is different. mx50 use arch/arm/mach-mx5/clock-mx50.c.
Possibly individual crm_regs.h too.
3. iomux
individual arch/arm/plat-mxc/include/mach/iomux-mx50.h
4. gpio
Add it in arch/arm/plat-mxc/gpio.c under macro CONFIG_SOC_IMX50.
5. irq
Add it in individual arch/arm/plat-mxc/include/mach/mx50.h
6. device and resource.
Dynamicly add device. SoC level data is in arch/arm/plat-mxc/devices
init section, plat data is in board level file.

Any comments?

Thanks
Richard
>>
>> > >> > How do you want to continue this change? ?mx51 and mx53 have the same
>> > >> > number of ports? ?Does mx50 have more or less? ?The addresses are the
>> > >> > same?
>> > >> mx53 and mx50 have similar memory map. but mx51 is different. ?Sorry,
>> > >> I forgot upstream version IO_ADDR can not handler differnt SoC base
>> > >> addr offset.
>> > > It might even work in this case, still I prefer a more explicit way.
>> > >
>> > >> ? ? ? ? ? ? ?It can not be shared with mx51, but it can be shared with
>> > >> mx50/53, correct? May plan is mx53/50 share
>> > >> arch/arm/plat-mxc/include/mach/mx5x.h file.
>> > > mx5x.h is for mx50 and mx53 but not mx51? ?No please.
>> > For memory map, mx51 is a special case. From mx53 on , IC tuned memory
>> > map to support large memory.
>> > If we use things like mx53-50.h, symbols is hard to define.
>> > MX53_50_XXX_XXX is wired. Any possibly, we need to add new SoC with
>> > similar memory map in the future.
>> I still think defining symbols for a single SoC is fine most of the
>> time.
>
> +1
>
> MX2X was confusing when the i.MX25 came out and we won't start *any*
> MX5X things when we already know that it's wrong. Go with MX51_ MX53_
> MX50_ or whatever the SoC of the day is. Putting things together to
> share the code is done in the upper layers, *not* the register defines.
>
> 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 |
>

[PATCH 1/1] arm: imx: share imx5x_register_gpios for mach-mx5

[Uwe Kleine-König]

Hi Richard,

On Thu, Nov 25, 2010 at 07:10:27PM +0800, Richard Zhao wrote:
> Thanks. I go through the code again. Here is mx50 adding plan:
> 1. basic memory map definitions
> Add it in individual arch/arm/plat-mxc/include/mach/mx50.h
I hope the memory map itself goes into arch/arm/mach-mx5/mm-mx50.c or
similar.  The constants go into include/mach/mx50.h though.

> 2. clock.c crm_regs.h
> clock tree is different. mx50 use arch/arm/mach-mx5/clock-mx50.c.
> Possibly individual crm_regs.h too.
Then please use proper prefixes (and maybe make the definitions local to
a .c file if possible).
> 3. iomux
> individual arch/arm/plat-mxc/include/mach/iomux-mx50.h
is mx50 using iomux-v3?

> 4. gpio
> Add it in arch/arm/plat-mxc/gpio.c under macro CONFIG_SOC_IMX50.
yes

> 5. irq
> Add it in individual arch/arm/plat-mxc/include/mach/mx50.h
yes, the constants go there

> 6. device and resource.
> Dynamicly add device. SoC level data is in arch/arm/plat-mxc/devices
> init section, plat data is in board level file.
most of the time yes.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH 1/1] arm: imx: share imx5x_register_gpios for mach-mx5

[Richard Zhao]

From: Richard Zhao <richard.zhao@freescale.com>

Add variable mx5x_gpio_port_cnt, and init in machine init function.
It can be share by mx51, mx53 and coming mx50.

Signed-off-by: Richard Zhao <linuxzsc@gmail.com>

diff --git a/arch/arm/mach-mx5/board-cpuimx51.c b/arch/arm/mach-mx5/board-cpuimx51.c
index 5ff5522..cd7bd4c 100644
--- a/arch/arm/mach-mx5/board-cpuimx51.c
+++ b/arch/arm/mach-mx5/board-cpuimx51.c
@@ -242,6 +242,8 @@ __setup("otg_mode=", eukrea_cpuimx51_otg_mode);
  */
 static void __init eukrea_cpuimx51_init(void)
 {
+	mx5x_gpio_port_cnt = 4;
+
 	mxc_iomux_v3_setup_multiple_pads(eukrea_cpuimx51_pads,
 					ARRAY_SIZE(eukrea_cpuimx51_pads));
 
diff --git a/arch/arm/mach-mx5/board-cpuimx51sd.c b/arch/arm/mach-mx5/board-cpuimx51sd.c
index ff1f45e..e47345a 100644
--- a/arch/arm/mach-mx5/board-cpuimx51sd.c
+++ b/arch/arm/mach-mx5/board-cpuimx51sd.c
@@ -262,6 +262,8 @@ static struct platform_device *platform_devices[] __initdata = {
 
 static void __init eukrea_cpuimx51sd_init(void)
 {
+	mx5x_gpio_port_cnt = 4;
+
 	mxc_iomux_v3_setup_multiple_pads(eukrea_cpuimx51sd_pads,
 					ARRAY_SIZE(eukrea_cpuimx51sd_pads));
 
diff --git a/arch/arm/mach-mx5/board-mx51_3ds.c b/arch/arm/mach-mx5/board-mx51_3ds.c
index 6cc2805..cb22fc3 100644
--- a/arch/arm/mach-mx5/board-mx51_3ds.c
+++ b/arch/arm/mach-mx5/board-mx51_3ds.c
@@ -160,6 +160,8 @@ static struct spi_board_info mx51_3ds_spi_nor_device[] = {
  */
 static void __init mxc_board_init(void)
 {
+	mx5x_gpio_port_cnt = 4;
+
 	mxc_iomux_v3_setup_multiple_pads(mx51_3ds_pads,
 					ARRAY_SIZE(mx51_3ds_pads));
 	mxc_init_imx_uart();
diff --git a/arch/arm/mach-mx5/board-mx51_babbage.c b/arch/arm/mach-mx5/board-mx51_babbage.c
index ef32843..8054c09 100644
--- a/arch/arm/mach-mx5/board-mx51_babbage.c
+++ b/arch/arm/mach-mx5/board-mx51_babbage.c
@@ -347,9 +347,12 @@ static const struct spi_imx_master mx51_babbage_spi_pdata __initconst = {
  */
 static void __init mxc_board_init(void)
 {
+	mx5x_gpio_port_cnt = 4;
+
 	iomux_v3_cfg_t usbh1stp = MX51_PAD_USBH1_STP__USBH1_STP;
 	iomux_v3_cfg_t power_key = MX51_PAD_EIM_A27__GPIO_2_21;
 
+	mx5x_gpio_port_cnt = 4;
 #if defined(CONFIG_CPU_FREQ_IMX)
 	get_cpu_op = mx51_get_cpu_op;
 #endif
diff --git a/arch/arm/mach-mx5/board-mx51_efikamx.c b/arch/arm/mach-mx5/board-mx51_efikamx.c
index 4b2718b..cf315cb 100644
--- a/arch/arm/mach-mx5/board-mx51_efikamx.c
+++ b/arch/arm/mach-mx5/board-mx51_efikamx.c
@@ -314,6 +314,8 @@ void mx51_efikamx_reset(void)
 
 static void __init mxc_board_init(void)
 {
+	mx5x_gpio_port_cnt = 4;
+
 	mxc_iomux_v3_setup_multiple_pads(mx51efikamx_pads,
 					ARRAY_SIZE(mx51efikamx_pads));
 	mx51_efikamx_board_id();
diff --git a/arch/arm/mach-mx5/devices.c b/arch/arm/mach-mx5/devices.c
index d926203..b8371d2 100644
--- a/arch/arm/mach-mx5/devices.c
+++ b/arch/arm/mach-mx5/devices.c
@@ -183,13 +183,11 @@ static struct mxc_gpio_port mxc_gpio_ports[] = {
 	},
 };
 
-int __init imx51_register_gpios(void)
-{
-	return mxc_gpio_init(mxc_gpio_ports, 4);
-}
+int mx5x_gpio_port_cnt;
 
-int __init imx53_register_gpios(void)
+int __init imx5x_register_gpios(void)
 {
-	return mxc_gpio_init(mxc_gpio_ports, ARRAY_SIZE(mxc_gpio_ports));
+	if (mx5x_gpio_port_cnt)
+		return mxc_gpio_init(mxc_gpio_ports, mx5x_gpio_port_cnt);
+	return -EINVAL;
 }
-
diff --git a/arch/arm/mach-mx5/devices.h b/arch/arm/mach-mx5/devices.h
index af1d07c..d3411dc 100644
--- a/arch/arm/mach-mx5/devices.h
+++ b/arch/arm/mach-mx5/devices.h
@@ -4,3 +4,5 @@ extern struct platform_device mxc_usbdr_udc_device;
 extern struct platform_device mxc_wdt;
 extern struct platform_device mxc_hsi2c_device;
 extern struct platform_device mxc_keypad_device;
+
+extern int mx5x_gpio_port_cnt;
diff --git a/arch/arm/mach-mx5/mm.c b/arch/arm/mach-mx5/mm.c
index e57f968..b0ea57b 100644
--- a/arch/arm/mach-mx5/mm.c
+++ b/arch/arm/mach-mx5/mm.c
@@ -61,7 +61,7 @@ void __init mx53_map_io(void)
 	iotable_init(mx53_io_desc, ARRAY_SIZE(mx53_io_desc));
 }
 
-int imx51_register_gpios(void);
+int imx5x_register_gpios(void);
 
 void __init mx51_init_irq(void)
 {
@@ -78,11 +78,9 @@ void __init mx51_init_irq(void)
 		panic("unable to map TZIC interrupt controller\n");
 
 	tzic_init_irq(tzic_virt);
-	imx51_register_gpios();
+	imx5x_register_gpios();
 }
 
-int imx53_register_gpios(void);
-
 void __init mx53_init_irq(void)
 {
 	unsigned long tzic_addr;
@@ -95,5 +93,5 @@ void __init mx53_init_irq(void)
 		panic("unable to map TZIC interrupt controller\n");
 
 	tzic_init_irq(tzic_virt);
-	imx53_register_gpios();
+	imx5x_register_gpios();
 }
-- 
1.7.1

[PATCH 1/1] arm: imx: share imx5x_register_gpios for mach-mx5

[Richard Zhao]

2010/11/25 Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
> Hi Richard,
>
> On Thu, Nov 25, 2010 at 07:10:27PM +0800, Richard Zhao wrote:
>> Thanks. I go through the code again. Here is mx50 adding plan:
>> 1. basic memory map definitions
>> Add it in individual arch/arm/plat-mxc/include/mach/mx50.h
> I hope the memory map itself goes into arch/arm/mach-mx5/mm-mx50.c or
> similar. ?The constants go into include/mach/mx50.h though.
ok
>
>> 2. clock.c crm_regs.h
>> clock tree is different. mx50 use arch/arm/mach-mx5/clock-mx50.c.
>> Possibly individual crm_regs.h too.
> Then please use proper prefixes (and maybe make the definitions local to
> a .c file if possible).
>> 3. iomux
>> individual arch/arm/plat-mxc/include/mach/iomux-mx50.h
> is mx50 using iomux-v3?
Yes.
>
>> 4. gpio
>> Add it in arch/arm/plat-mxc/gpio.c under macro CONFIG_SOC_IMX50.
> yes
>
>> 5. irq
>> Add it in individual arch/arm/plat-mxc/include/mach/mx50.h
> yes, the constants go there
>
>> 6. device and resource.
>> Dynamicly add device. SoC level data is in arch/arm/plat-mxc/devices
>> init section, plat data is in board level file.
> most of the time yes.
>
> Best regards
> Uwe
>
> --
> Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? | Uwe Kleine-K?nig ? ? ? ? ? ?|
> Industrial Linux Solutions ? ? ? ? ? ? ? ? | http://www.pengutronix.de/ ?|
>