[U-Boot] [PATCH v4 1/7] arm: vf610: Add IOMUX support for Vybrid VF610

[Wang Huan-B18965]

Hi, Benoit,

> > diff --git a/arch/arm/imx-common/iomux-v3.c
> > b/arch/arm/imx-common/iomux-v3.c index 7fe5ce7..35880c7 100644
> > --- a/arch/arm/imx-common/iomux-v3.c
> > +++ b/arch/arm/imx-common/iomux-v3.c
> > @@ -48,8 +48,14 @@ void imx_iomux_v3_setup_pad(iomux_v3_cfg_t pad)
> >  	if (sel_input_ofs)
> >  		__raw_writel(sel_input, base + sel_input_ofs);
> >
> > +#ifdef CONFIG_IOMUX_SHARE_CONF_REG
> 
> Where is this one defined? I don't see it in include/configs/vf610twr.h.
> 
[Alison Wang] CONFIG_IOMUX_SHARE_CONF_REG is defined in arch/arm/include/asm/arch-vf610/imx-regs.h. Because this is not a board configuration, it is related to the SOC.

Please refer to Stefano's comments below which also could be found in the email on May 15th.

Stefano wrote:
> +
> +/* MUX mode and PAD ctrl are in one register */
> +#define CONFIG_IOMUX_SHARE_CONF_REG

NAK. This is not a board configuration, it is related to the SOC. This
setup should flow into the related imx-regs.h for this SOC. When you set
CONFIG_MVF600, this value should be set automatically.

> Why not use "#ifdef CONFIG_VF610" since this is a platform-dependent
> code, and not a board-specific config option?
[Alison Wang] I use this CONFIG_IOMUX_SHARE_CONF_REG option, because this part of codes
not only could be used on VF610 platform, but also could be used on VF620 or other platforms.
When it is used on VF620 or others, you could just enable CONFIG_IOMUX_SHARE_CONF_REG
in the related imx-regs.h.
Otherwise, if "ifdef CONFIG_VF610" is used, you need to add "#if defined(CONFIG_VF610) || defined(CONFIG_VF620)"
When this part of codes is also used on VF620. Then when this part of codes is used on VF630 too, this line 
will be very very long.
 
> 
> > +	if (!(pad_ctrl & NO_PAD_CTRL))
> > +		__raw_writel((mux_mode << PAD_MUX_MODE_SHIFT) | pad_ctrl,
> > +			base + pad_ctrl_ofs);
> > +#else
> >  	if (!(pad_ctrl & NO_PAD_CTRL) && pad_ctrl_ofs)
> >  		__raw_writel(pad_ctrl, base + pad_ctrl_ofs);
> > +#endif
> >  }
> >
> >  void imx_iomux_v3_setup_multiple_pads(iomux_v3_cfg_t const *pad_list,
> 
> [...]
> 
> Apart from that, this patch is OK.
> 
Thanks.

Best Regards,
Alison Wang