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

Shawn Guo writes:

2010/12/9 Uwe Kleine-K?nig [off-list ref]:

quoted

Hello Shawn
On Thu, Dec 09, 2010 at 05:03:54PM +0800, Shawn Guo wrote:

quoted

2010/12/9 Uwe Kleine-K?nig [off-list ref]:

quoted

Hello Shwan,

ups, sorry for mistyping your name.

quoted

quoted

On Thu, Dec 09, 2010 at 03:04:37PM +0800, Shawn Guo wrote:

quoted

2010/12/9 Uwe Kleine-K?nig [off-list ref]:

quoted

On Wed, Dec 08, 2010 at 12:32:02AM +0800, Shawn Guo wrote:

quoted

+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
___________________________________________________________