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

[Peng Fan]

On Sun, Sep 20, 2015 at 01:33:20PM +0200, Benoît Thébaudeau wrote:
>Hi Stefano, Peng, Fabio, all,
>
>Sorry for seeing this only now, but...
>
>On Sun, Sep 20, 2015 at 9:43 AM, Stefano Babic <sbabic at denx.de> wrote:
>>
>>
>> On 14/09/2015 07:34, Peng Fan wrote:
>>> When setting iomux for a pin mux, there is no need to check mux_ctrl_ofs.
>
>This assumption is wrong. This check was there for a reason. Some i.MX
>SoCs have some registers controlling pads but not muxes, either for a
>single pin or for groups of pins:
>http://git.denx.de/?p=u-boot/u-boot-imx.git;a=blob;f=arch/arm/include/asm/arch-mx25/iomux-mx25.h;h=220cf4ef2e94aa69482557852ed0cc0690a79cec;hb=HEAD
>http://git.denx.de/?p=u-boot/u-boot-imx.git;a=blob;f=arch/arm/include/asm/arch-mx35/iomux-mx35.h;h=5898b46f4720088b18882e21d0d2424fff987ab5;hb=HEAD
>http://git.denx.de/?p=u-boot/u-boot-imx.git;a=blob;f=arch/arm/include/asm/arch-mx5/iomux-mx51.h;h=b7b169505f91c4a213be59efca47e8a5aed770e7;hb=HEAD
>
>I have not checked whether these cases are currently used in-tree by
>U-Boot, but they have to be possible anyway in order to support these
>SoCs.

Benoît,

Thanks for pointing this out.
You mean piece of code like this, right?
509         MX25_PAD_CTL_GRP_DVS_MISC               = IOMUX_PAD(0x418, 0x000, 0, 0, 0, NO_PAD_CTRL),
510         MX25_PAD_CTL_GRP_DSE_FEC                = IOMUX_PAD(0x41c, 0x000, 0, 0, 0, NO_PAD_CTRL),
511         MX25_PAD_CTL_GRP_DVS_JTAG               = IOMUX_PAD(0x420, 0x000, 0, 0, 0, NO_PAD_CTRL),
512         MX25_PAD_CTL_GRP_DSE_NFC                = IOMUX_PAD(0x424, 0x000, 0, 0, 0, NO_PAD_CTRL),
513         MX25_PAD_CTL_GRP_DSE_CSI                = IOMUX_PAD(0x428, 0x000, 0, 0, 0, NO_PAD_CTRL),
514         MX25_PAD_CTL_GRP_DSE_WEIM               = IOMUX_PAD(0x42c, 0x000, 0, 0, 0, NO_PAD_CTRL),
515         MX25_PAD_CTL_GRP_DSE_DDR                = IOMUX_PAD(0x430, 0x000, 0, 0, 0, NO_PAD_CTRL),
516         MX25_PAD_CTL_GRP_DVS_CRM                = IOMUX_PAD(0x434, 0x000, 0, 0, 0, NO_PAD_CTRL),
517         MX25_PAD_CTL_GRP_DSE_KPP                = IOMUX_PAD(0x438, 0x000, 0, 0, 0, NO_PAD_CTRL),
518         MX25_PAD_CTL_GRP_DSE_SDHC1              = IOMUX_PAD(0x43c, 0x000, 0, 0, 0, NO_PAD_CTRL),
519         MX25_PAD_CTL_GRP_DSE_LCD                = IOMUX_PAD(0x440, 0x000, 0, 0, 0, NO_PAD_CTRL),
520         MX25_PAD_CTL_GRP_DSE_UART               = IOMUX_PAD(0x444, 0x000, 0, 0, 0, NO_PAD_CTRL),
521         MX25_PAD_CTL_GRP_DVS_NFC                = IOMUX_PAD(0x448, 0x000, 0, 0, 0, NO_PAD_CTRL),
522         MX25_PAD_CTL_GRP_DVS_CSI                = IOMUX_PAD(0x44c, 0x000, 0, 0, 0, NO_PAD_CTRL),
523         MX25_PAD_CTL_GRP_DSE_CSPI1              = IOMUX_PAD(0x450, 0x000, 0, 0, 0, NO_PAD_CTRL),
524         MX25_PAD_CTL_GRP_DDRTYPE                = IOMUX_PAD(0x454, 0x000, 0, 0, 0, NO_PAD_CTRL),
525         MX25_PAD_CTL_GRP_DVS_SDHC1              = IOMUX_PAD(0x458, 0x000, 0, 0, 0, NO_PAD_CTRL),
526         MX25_PAD_CTL_GRP_DVS_LCD                = IOMUX_PAD(0x45c, 0x000, 0, 0, 0, NO_PAD_CTRL)

My bad. I only took i.mx6/7 into consideration when working this patch.
>
>>> 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?

Regards,
Peng.

>
>>> Signed-off-by: Peng Fan <Peng.Fan at freescale.com>
>>> Cc: Stefano Babic <sbabic at denx.de>
>>> Cc: Fabio Estevam <fabio.estevam at freescale.com>
>>> ---
>>>  arch/arm/imx-common/iomux-v3.c | 3 +--
>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/imx-common/iomux-v3.c b/arch/arm/imx-common/iomux-v3.c
>>> index b4f481f..9b9cf58 100644
>>> --- a/arch/arm/imx-common/iomux-v3.c
>>> +++ b/arch/arm/imx-common/iomux-v3.c
>>> @@ -53,8 +53,7 @@ void imx_iomux_v3_setup_pad(iomux_v3_cfg_t pad)
>>>       }
>>>  #endif
>>>
>>> -     if (mux_ctrl_ofs)
>>> -             __raw_writel(mux_mode, base + mux_ctrl_ofs);
>>> +     __raw_writel(mux_mode, base + mux_ctrl_ofs);
>>>
>>>       if (sel_input_ofs)
>>>               __raw_writel(sel_input, base + sel_input_ofs);
>>>
>>
>> Applied (whole series) to u-boot-imx, thanks !
>
>Please fix.
>
>Best regards,
>Benoît

--