From mboxrd@z Thu Jan 1 00:00:00 1970 From: LW@KARO-electronics.de (=?iso-8859-15?Q?Lothar_Wa=DFmann?=) Date: Thu, 9 Dec 2010 13:27:53 +0100 Subject: [PATCH v2 14/15] ARM: mxs: Add initial mx28evk support In-Reply-To: References: <1290754154-9428-1-git-send-email-shawn.guo@freescale.com> <1291739523-25077-11-git-send-email-shawn.guo@freescale.com> <20101208202822.GD17441@pengutronix.de> <20101209083237.GO17441@pengutronix.de> <20101209093746.GU17441@pengutronix.de> Message-ID: <19712.52041.958400.447834@ipc1.ka-ro> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Shawn Guo writes: > 2010/12/9 Uwe Kleine-K?nig : > > Hello Shawn > > On Thu, Dec 09, 2010 at 05:03:54PM +0800, Shawn Guo wrote: > >> 2010/12/9 Uwe Kleine-K?nig : > >> > 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 : > >> >> > 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 ___________________________________________________________