[PATCH v2 14/15] ARM: mxs: Add initial mx28evk support

[Lothar Waßmann]

Shawn Guo writes:
> 2010/12/9 Uwe Kleine-König <u.kleine-koenig at pengutronix.de>:
> > Hello Shawn
> > On Thu, Dec 09, 2010 at 05:03:54PM +0800, Shawn Guo wrote:
> >> 2010/12/9 Uwe Kleine-König <u.kleine-koenig at pengutronix.de>:
> >> > Hello Shwan,
> > ups, sorry for mistyping your name.
> >
> >> > On Thu, Dec 09, 2010 at 03:04:37PM +0800, Shawn Guo wrote:
> >> >> 2010/12/9 Uwe Kleine-König <u.kleine-koenig at pengutronix.de>:
> >> >> > On Wed, Dec 08, 2010 at 12:32:02AM +0800, Shawn Guo wrote:
> >> >> >> +static iomux_cfg_t mx28evk_pads[] = {
> >> >> > This can be const and __initconst, ditto for mx23evk
> >> >> >
> >> >> With u64 iomux_cfg_t changes, I'm afraid the suggestion becomes invalid.
> >> > Really? Why?
> >> >
> >> I'm confused by the compiling error below when adding __initconst for
> >> mx28evk_pads[], and mistakenly blaming u64 iomux_cfg_t changes.
> >>
> >> arch/arm/mach-mxs/mach-mx28evk.c: In function ?mx28evk_init?:
> >> arch/arm/mach-mxs/mach-mx28evk.c:34: error: mx28_fec_pdata causes a
> >> section type conflict
> >> make[1]: *** [arch/arm/mach-mxs/mach-mx28evk.o] Error 1
> >> make: *** [arch/arm/mach-mxs] Error 2
> >>
> >> Actually it can be fixed by the following change.
> >>
> >> -static const struct fec_platform_data mx28_fec_pdata __initconst = {
> >> +static struct fec_platform_data mx28_fec_pdata __initconst = {
> >>          .phy = PHY_INTERFACE_MODE_RMII,
> >>  };
> > this change is wrong.  You need to assert that all data being marked
> > with __initconst is const, too.
> >
> After adding const for mx28evk_pads[], I got the following error.
> 
>   CC      arch/arm/mach-mxs/mach-mx28evk.o
> arch/arm/mach-mxs/mach-mx28evk.c: In function ?mx28evk_init?:
> arch/arm/mach-mxs/mach-mx28evk.c:102: warning: passing argument 1 of
> ?mxs_iomux_setup_multiple_pads? discards qualifiers from pointer
> target type
> arch/arm/mach-mxs/include/mach/iomux.h:115: note: expected
> ?iomux_cfg_t *? but argument is of type ?const iomux_cfg_t *?
> 
The argument of mxs_iomux_setup_multiple_pads() should get the const
attribute:
-int mxs_iomux_setup_multiple_pads(iomux_cfg_t *pad_list, unsigned count)
+int mxs_iomux_setup_multiple_pads(const iomux_cfg_t *pad_list, unsigned count)

> This takes me back to my first judgment.  Is it proper to add
> __initconst for mx28evk_pads[]? We are changing iomux_cfg_t to u64 for
> making pad definition modifiable.
> 
Modifiable in the sense that you can add platform specific PAD
settings by simply ORing them to the original pad definition so that
there is no need for runtime modifications like:
|	iomux_v3_cfg_t power_key = MX51_PAD_EIM_A27__GPIO_2_21;
...
|	power_key.pad_ctrl = MX51_GPIO_PAD_CTRL_2;
|	mxc_iomux_v3_setup_pad(&power_key);
but you can do:
|	iomux_v3_cfg_t power_key = (MX51_PAD_EIM_A27__GPIO_2_21 &
|				~MUX_PAD_CTRL_MASK) | MX51_GPIO_PAD_CTRL_2;
...
|	mxc_iomux_v3_setup_pad(power_key);
instead. You can also augment the pad desc in a static table:
|static iomux_cfg_t pad_desc[] = {
|	(MX51_PAD_EIM_A27__GPIO_2_21 & ~MUX_PAD_CTRL_MASK) | MX51_GPIO_PAD_CTRL_2,


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
___________________________________________________________