Hi Peng, On 20/09/2015 15:02, Peng Fan wrote: > 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 <sba...@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:
Right > 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? Yes, it is ok for now, green light on my side. Best regards, Stefano Babic -- ===================================================================== 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: sba...@denx.de ===================================================================== _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot