From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?utf-8?Q?Beno=C3=AEt_Th=C3=A9baudeau?= Date: Wed, 29 May 2013 16:54:31 +0200 (CEST) Subject: [U-Boot] [PATCH v4 1/7] arm: vf610: Add IOMUX support for Vybrid VF610 In-Reply-To: <51A59E4C.3030306@denx.de> References: <1369731347-9994-1-git-send-email-b18965@freescale.com> <1369731347-9994-2-git-send-email-b18965@freescale.com> <1512799881.1269794.1369767420172.JavaMail.root@advansee.com> <81BA6E5E0BC2344391CABCEE22D1B6D833FFDE@039-SN1MPN1-003.039d.mgd.msft.net> <51A59E4C.3030306@denx.de> Message-ID: <2078601675.1324075.1369839271762.JavaMail.root@advansee.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 I agree: Reviewed-by: Beno?t Th?baudeau Best regards, Beno?t