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

[Shawn Guo]

Hi Lothar,

On Thu, Dec 9, 2010 at 8:27 PM, Lothar Waßmann <LW at karo-electronics.de> wrote:
> 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,
>
>
Thanks for the explanation.  But I'm a little puzzled by the use of
"const" here. Per your suggestion, I have the following.

mach-28evk.c
static const iomux_cfg_t mx28evk_pads[] __initconst = {

iomux.h
int mxs_iomux_setup_multiple_pads(const iomux_cfg_t *pad_list, unsigned count);

iomux.c
int mxs_iomux_setup_multiple_pads(const iomux_cfg_t *pad_list, unsigned count)

Compiler complains as below.

  CC      arch/arm/mach-mxs/iomux.o
arch/arm/mach-mxs/iomux.c:89: error: conflicting types for
‘mxs_iomux_setup_multiple_pads’
arch/arm/mach-mxs/include/mach/iomux.h:115: note: previous declaration
of ‘mxs_iomux_setup_multiple_pads’ was here
make[1]: *** [arch/arm/mach-mxs/iomux.o] Error 1
make: *** [arch/arm/mach-mxs] Error 2

If I remove the "const" in iomux.h as below.

iomux.h
int mxs_iomux_setup_multiple_pads(iomux_cfg_t *pad_list, unsigned count);

Compiler gives the following warning.

  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 *’

What's wrong here?

-- 
Regards,
Shawn