[U-Boot] [PATCH 1/3] imx-common: fix iomux settings

[Peng Fan]

Hi Stefano, Benoit,

On Mon, Sep 21, 2015 at 10:13:42PM +0200, Stefano Babic wrote:
>Hi Benoit, Peng,
>
>On 21/09/2015 20:41, Benoît Thébaudeau wrote:
>> Hi Peng,
>> 
>> On Mon, Sep 21, 2015 at 3:05 AM, Peng Fan <b51431 at freescale.com> wrote:
>>> On Sun, Sep 20, 2015 at 05:02:58PM +0200, Benoît Thébaudeau wrote:
>>>> On Sun, Sep 20, 2015 at 3:02 PM, Peng Fan <b51431 at freescale.com> wrote:
>>>>> On Sun, Sep 20, 2015 at 01:33:20PM +0200, Benoît Thébaudeau wrote:
>>>>>>>> Also If still checking mux_ctrl_ofs, we have no chance to set iomux
>>>>>>>> for i.MX7D IOMUXC_LPSR_SW_MUX_CTL_PAD_GPIO1_IO00, because the mux_ctrl_ofs
>>>>>>>> for this register is 0.
>>>>>>
>>>>>> The need is clear, but then the test mechanism should be changed, not
>>>>>> removed. You could find a free bit in mux_ctrl_ofs or in mux_mode or
>>>>>> elsewhere in IOMUX_PAD (e.g. bit 63, which is currently reserved),
>>>>>> something like NO_PAD_CTRL, or create a reserved value other than
>>>>>> __NA_ for mux_ctrl_ofs/mux_mode.
>>>>>
>>>>> Stefano,
>>>>>
>>>>> There is '#define NO_PAD_CTRL             (1 << 17)' now,
>>>>> we can add'NO_MUX_CTRL' and 'NO_SEL_CTRL(select input)', but need to check
>>>>> whether the __NA__ pads are used or not now.
>>>>> also need a big change for the layout and related macro definition:
>>>>> 39  * MUX_CTRL_OFS:            0..11 (12)
>>>>> 40  * PAD_CTRL_OFS:           12..23 (12)
>>>>> 41  * SEL_INPUT_OFS:          24..35 (12)
>>>>> 42  * MUX_MODE + SION:        36..40  (5)
>>>>> 43  * PAD_CTRL + NO_PAD_CTRL: 41..58 (18)
>>>>> 44  * SEL_INP:                59..62  (4)
>>>>> 45  * reserved:                 63    (1)
>>>>>
>>>>> Can we just use the following way, since only i.mx7 has the requirement of
>>>>> mux_ctrl_ofs maybe at 0.
>>>>> if (is_soc_type(MX7)) {
>>>>>         __raw_writel(mux_mode, base + mux_ctrl_ofs);
>>>>> } else {
>>>>>         if (mux_ctrl_ofs)
>>>>>         __raw_writel(mux_mode, base + mux_ctrl_ofs);
>>>>> }
>>>>> I prefer this simple way for now, since we are at RC2 now. Later we can
>>>>> refactor the code using the way to provide macros NO_MUX_CTRL or NO_SEL_CTRL.
>>>>> What do you think?
>>>>
>>>> Maybe, but instead of NO_MUX_CTRL and the like we could also just
>>>> define __NA_ to (-1) instead of 0 and mask the passed values
>>>> appropriately in IOMUX_PAD(). This should be done for all types of
>>>> offsets, and __NA_ should be used everywhere instead of raw 0x000
>>>> values. -1 is guaranteed not to be needed by any SoC because of the
>>>> word alignment requirement for valid offsets. That would keep the
>>>> changes small.
>>>
>>> We can not just simple use __NA_ with value -1.
>>> see
>>> 70 #define IOMUX_PAD(pad_ctrl_ofs, mux_ctrl_ofs, mux_mode, sel_input_ofs,  \
>>> 71                 sel_input, pad_ctrl)                                    \
>>> 72         (((iomux_v3_cfg_t)(mux_ctrl_ofs) << MUX_CTRL_OFS_SHIFT)     |   \
>>> 73         ((iomux_v3_cfg_t)(mux_mode)      << MUX_MODE_SHIFT)         |   \
>>> 74         ((iomux_v3_cfg_t)(pad_ctrl_ofs)  << MUX_PAD_CTRL_OFS_SHIFT) |   \
>>> 75         ((iomux_v3_cfg_t)(pad_ctrl)      << MUX_PAD_CTRL_SHIFT)     |   \
>>> 76         ((iomux_v3_cfg_t)(sel_input_ofs) << MUX_SEL_INPUT_OFS_SHIFT)|   \
>>> 77         ((iomux_v3_cfg_t)(sel_input)     << MUX_SEL_INPUT_SHIFT))
>>>
>>> iomux_v3_cfg_t(mux_ctrl_ofs) << MUX_CTRL_OFS_SHIFT should be changed to
>>> `iomux_v3_cfg_t((mux_ctrl_ofs) << MUX_CTRL_OFS_SHIFT) & MUX_CTRL_OFS_MASK`,
>> 
>> Yes. That's why I said "mask the passed values appropriately in IOMUX_PAD()".
>> 
>>> in iomux-v3.c, need to test if (!(mux_ctrl_ofs & 1)) {xxxxx}.
>> 
>> Yes, of course.
>> 
>>> I am not sure whether this will incur unexpected things or not,
>> 
>> There's no reason.
>> 
>>> also
>>> the IOMUX_PAD with 0, but not __NA_ need to change to use __NA_.
>> 
>> And also, as I said, do the same for pad_ctrl_ofs and sel_input_ofs in
>> order to be consistent, and replace definitions like:
>>          MX25_PAD_CTL_GRP_DVS_MISC               = IOMUX_PAD(0x418,
>> 0x000, 0, 0, 0, NO_PAD_CTRL),
>> with:
>>          MX25_PAD_CTL_GRP_DVS_MISC               = IOMUX_PAD(0x418,
>> __NA_, 0, __NA_, 0, NO_PAD_CTRL),
>> 
>>> So I prefer to use is_soc_type(MXC_CPU_MX7) for now.

I have sent out one patch: https://patchwork.ozlabs.org/patch/520204/

>> 
>> Yes, that's OK for now. I was suggesting that as a longterm approach.
>
>Agree.
>
>> This change would be simple, but many definitions would have to be
>> updated.
>
>Yes - so let it after next release.

I'll prepare the patch for next release.

Regards,
Peng.
>
>Best regards,
>Stefano
>
>-- 
>=====================================================================
>DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
>HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
>=====================================================================

--