From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?utf-8?Q?Beno=C3=AEt_Th=C3=A9baudeau?= Date: Tue, 26 Feb 2013 19:47:21 +0100 (CET) Subject: [U-Boot] [PATCH v2 2/2] mtd: nand: mxc_nand: Fix is_16bit_nand() In-Reply-To: <1361903720-26935-2-git-send-email-fabio.estevam@freescale.com> References: <1361903720-26935-1-git-send-email-fabio.estevam@freescale.com> <1361903720-26935-2-git-send-email-fabio.estevam@freescale.com> Message-ID: <947333580.132976.1361904441015.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 Fabio, On Tuesday, February 26, 2013 7:35:20 PM, Fabio Estevam wrote: > Currently is_16bit_nand() is a per SoC function and it decides the bus nand > width by reading some boot related registers. > > This method works when NAND is the boot medium, but does not work if another > boot medium is used. For example: booting from a SD card and then using NAND > to store the environment variables, would lead to the following error: > > NAND bus width 16 instead 8 bit > No NAND device found!!! > 0 MiB > > Use CONFIG_SYS_NAND_BUSWIDTH_16BIT symbol to decide the bus width. > > If it is defined in the board file, then consider 16-bit NAND bus-width, > otherwise assume 8-bit NAND is used. > > This also aligns with Documentation/devicetree/bindings/mtd/nand.txt, which > states: > > nand-bus-width : 8 or 16 bus width if not present 82 > > Signed-off-by: Fabio Estevam > --- > Changes since v1: > - Add an entry into README > README | 3 ++- > drivers/mtd/nand/mxc_nand.c | 37 +++---------------------------------- > 2 files changed, 5 insertions(+), 35 deletions(-) > > diff --git a/README b/README > index bdd2c81..1a6a7a5 100644 > --- a/README > +++ b/README > @@ -3717,8 +3717,9 @@ Low Level (hardware related) configuration options: > Defined to tell the NAND controller that the NAND chip is using > a 16 bit bus. > Not all NAND drivers use this symbol. > - Example of driver that uses it: > + Example of drivers that use it: > - drivers/mtd/nand/ndfc.c > + - drivers/mtd/nand/mxc_nand.c > > - CONFIG_SYS_NDFC_EBC0_CFG > Sets the EBC0_CFG register for the NDFC. If not defined > diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c > index d0ded48..bb475f2 100644 > --- a/drivers/mtd/nand/mxc_nand.c > +++ b/drivers/mtd/nand/mxc_nand.c > @@ -98,45 +98,14 @@ static struct nand_ecclayout nand_hw_eccoob2k = { > #endif > #endif > > -#ifdef CONFIG_MX27 > static int is_16bit_nand(void) > { > - struct system_control_regs *sc_regs = > - (struct system_control_regs *)IMX_SYSTEM_CTL_BASE; > - > - if (readl(&sc_regs->fmcr) & NF_16BIT_SEL) > - return 1; > - else > - return 0; > -} > -#elif defined(CONFIG_MX31) > -static int is_16bit_nand(void) > -{ > - struct clock_control_regs *sc_regs = > - (struct clock_control_regs *)CCM_BASE; > - > - if (readl(&sc_regs->rcsr) & CCM_RCSR_NF16B) > - return 1; > - else > - return 0; > -} > -#elif defined(CONFIG_MX25) || defined(CONFIG_MX35) > -static int is_16bit_nand(void) > -{ > - struct ccm_regs *ccm = (struct ccm_regs *)IMX_CCM_BASE; > - > - if (readl(&ccm->rcsr) & CCM_RCSR_NF_16BIT_SEL) > - return 1; > - else > - return 0; > -} > +#if defined(CONFIG_SYS_NAND_BUSWIDTH_16BIT) > + return 1; > #else > -#warning "8/16 bit NAND autodetection not supported" > -static int is_16bit_nand(void) > -{ > return 0; > -} > #endif > +} > > static uint32_t *mxc_nand_memcpy32(uint32_t *dest, uint32_t *source, size_t > size) > { That's correct. is_16bit_nand() is only used to conditionally set NAND_BUSWIDTH_16 in this->options, then this flag is tested everywhere else, so we could also completely drop is_16bit_nand() and just do: #ifdef CONFIG_SYS_NAND_BUSWIDTH_16BIT this->options |= NAND_BUSWIDTH_16; #endif What do you think is better? I'm fine with your implementation. Should I rebase my series on that, keeping both series separate, or should I integrate these 2 patches as is at the beginning of my series? Best regards, Beno?t