| Message ID | 1369731347-9994-2-git-send-email-b18965@freescale.com |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Stefano Babic |
| Headers | show |
Hi Alison, On Tuesday, May 28, 2013 10:55:41 AM, Alison Wang wrote: > This patch adds the IOMUX support for Vybrid VF610 platform. > > There is a little difference for IOMUXC module between VF610 and i.MX > platform, the muxmode and pad configuration share one 32bit register on > VF610, but they are two independent registers on I.MX platform. A > CONFIG_IOMUX_SHARE_CONFIG_REG was introduced to fit this difference. > > Signed-off-by: Alison Wang <b18965@freescale.com> [...] > 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. Why not use "#ifdef CONFIG_VF610" since this is a platform-dependent code, and not a board-specific config option? > + 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. Best regards, Benoît
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
On 29/05/2013 07:29, Wang Huan-B18965 wrote: >> 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. Agree. This is a property of the processor and should be automatically set. It should not flow into board config file, and having a family of processor we cannot use ifdef CONFIG_VF610. IMHO the patch is ok. Acked-by: Stefano Babic <sbabic@denx.de> Best regards, Stefano Babic
Hi Stefano, On Wednesday, May 29, 2013 8:21:00 AM, Stefano Babic wrote: > On 29/05/2013 07:29, Wang Huan-B18965 wrote: > > >> 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. > > Agree. This is a property of the processor and should be automatically > set. It should not flow into board config file, and having a family of > processor we cannot use ifdef CONFIG_VF610. > > IMHO the patch is ok. > > Acked-by: Stefano Babic <sbabic@denx.de> I agree: Reviewed-by: Benoît Thébaudeau <benoit.thebaudeau@advansee.com> Best regards, Benoît
On 28/05/2013 10:55, Alison Wang wrote: > This patch adds the IOMUX support for Vybrid VF610 platform. > > There is a little difference for IOMUXC module between VF610 and i.MX > platform, the muxmode and pad configuration share one 32bit register on > VF610, but they are two independent registers on I.MX platform. A > CONFIG_IOMUX_SHARE_CONFIG_REG was introduced to fit this difference. > > Signed-off-by: Alison Wang <b18965@freescale.com> > --- Applied to u-boot-imx, thanks. Best regards, Stefano Babic
diff --git a/arch/arm/imx-common/Makefile b/arch/arm/imx-common/Makefile index 8bba8a5..9492326 100644 --- a/arch/arm/imx-common/Makefile +++ b/arch/arm/imx-common/Makefile @@ -27,7 +27,7 @@ include $(TOPDIR)/config.mk LIB = $(obj)libimx-common.o -ifeq ($(SOC),$(filter $(SOC),mx25 mx35 mx5 mx6)) +ifeq ($(SOC),$(filter $(SOC),mx25 mx35 mx5 mx6 vf610)) COBJS-y = iomux-v3.o endif ifeq ($(SOC),$(filter $(SOC),mx5 mx6)) 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 + 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, diff --git a/arch/arm/include/asm/imx-common/iomux-v3.h b/arch/arm/include/asm/imx-common/iomux-v3.h index 0b4e763..ebf54cf 100644 --- a/arch/arm/include/asm/imx-common/iomux-v3.h +++ b/arch/arm/include/asm/imx-common/iomux-v3.h @@ -121,6 +121,24 @@ typedef u64 iomux_v3_cfg_t; #define PAD_CTL_DSE_40ohm (6 << 3) #define PAD_CTL_DSE_34ohm (7 << 3) +#elif defined(CONFIG_VF610) + +#define PAD_MUX_MODE_SHIFT 20 + +#define PAD_CTL_SPEED_MED (1 << 12) +#define PAD_CTL_SPEED_HIGH (3 << 12) + +#define PAD_CTL_DSE_50ohm (3 << 6) +#define PAD_CTL_DSE_25ohm (6 << 6) +#define PAD_CTL_DSE_20ohm (7 << 6) + +#define PAD_CTL_PUS_47K_UP (1 << 4 | PAD_CTL_PUE) +#define PAD_CTL_PUS_100K_UP (2 << 4 | PAD_CTL_PUE) +#define PAD_CTL_PKE (1 << 3) +#define PAD_CTL_PUE (1 << 2 | PAD_CTL_PKE) + +#define PAD_CTL_OBE_IBE_ENABLE (3 << 0) + #else #define PAD_CTL_DVS (1 << 13)
This patch adds the IOMUX support for Vybrid VF610 platform. There is a little difference for IOMUXC module between VF610 and i.MX platform, the muxmode and pad configuration share one 32bit register on VF610, but they are two independent registers on I.MX platform. A CONFIG_IOMUX_SHARE_CONFIG_REG was introduced to fit this difference. Signed-off-by: Alison Wang <b18965@freescale.com> --- Changes in v4: - Rename "MVF600" to "VF610" - Define PAD_CTL_PUS_47K_UP and PAD_CTL_PUS_100K_UP with PAD_CTL_PUE enabled - Reorganize the definitions - Correct the spaces and tabs Changes in v3: - Define PAD_CTL_PUE with PKE enabled Changes in v2: - Use common iomux-v3 code arch/arm/imx-common/Makefile | 2 +- arch/arm/imx-common/iomux-v3.c | 6 ++++++ arch/arm/include/asm/imx-common/iomux-v3.h | 18 ++++++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-)