Hi Peng, On Mon, Sep 21, 2015 at 3:05 AM, Peng Fan <b51...@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 <b51...@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. Yes, that's OK for now. I was suggesting that as a longterm approach. This change would be simple, but many definitions would have to be updated. Best regards, Benoît _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot