Re: [U-Boot] [PATCH 1/1] am33xx: add CONFIG_SYS_NAND_DEVICE_WIDTH to determine NAND device bus-width
Hi, Please see last set of queries inline.. From: Scott Wood [mailto:scottw...@freescale.com] On Sat, 2013-09-28 at 06:24 +, Gupta, Pekon wrote: From: Scott Wood [mailto:scottw...@freescale.com] On Fri, 2013-09-27 at 04:18 +, Gupta, Pekon wrote: [snip] (1) drivers/mtd/nand/fsl_ifc_spl.c (2) drivers/mtd/nand/fsl_elbc_spl.c So CONFIG_SYS_NAND_DEVICE_WIDTH should help them also. right ? How would it help? I meant, instead of requiring the modification in board file, CONFIG_SPL_DEVICE_WIDTH can be put in include/configs/*.h So can this new CONFIG_xx be accepted ? Only for SPL usage. Ok. so should I rename to CONFIG_SPL_NAND_DEVICE_WIDTH ? I have recently sent another version of this patch (as PATCH 2/4) of series below. This series also adds checks for reading ONFI params. Request you to review that also, so that I can include all feedbacks in next patch version. http://lists.denx.de/pipermail/u-boot/2013-September/163878.html It looked like you were removing the code that does dynamic detection, which would also affect non-SPL. - /* If we are 16 bit dev, our gpmc config tells us that */ - if ((readl(gpmc_cfg-cs[cs].config1) 0x3000) == 0x1000) omap_gpmc.c never had dynamic detection support. Above gpmc_config bit which is used to tell whether device is x16 or x8, gets actually hard- coded in gpmc_init(). Thus it was actually a mechanism to pass hard-coded bus- width information to nand driver. Refer: arch/arm/cpu/armv7/am33xx/mem.c : gpmc_init() So, instead of hacking the gpmc_init() everytime for different devices, this patch introduces a generic CONFIG which can be used everywhere. Re-added dynamic detection for u-boot (second stage) in above series. CONFIG_SPL_NAND_DEVICE_WIDTH would be used only for SPL boot. http://lists.denx.de/pipermail/u-boot/2013-September/163879.html It looks like you do more NAND config in gpmc_init() than just setting this one bit, so I don't think you save anything here. BTW, do you not need to set this bit in the config register for the hardware to work in the SPL case? Yes, I'm not changing the default configs for GPMC in gpmc_init(), because they are ok for x8 device. I'm just overriding them again during board_nand_init() if CONFIG_SYS_NAND_DEVICE_WIDTH == x16 device. Is this due to wanting to do identification as x8? Yes, reading of on-chip ONFI params should happen only in x8 mode. [PATCH 1/4] of above series should explain this. http://lists.denx.de/pipermail/u-boot/2013-September/163882.html (dropping few emails from CC list as they are bouncing, and u-boot mailman rejects email saying too many recipients). with regards, pekon ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/1] am33xx: add CONFIG_SYS_NAND_DEVICE_WIDTH to determine NAND device bus-width
On Wed, 2013-10-02 at 14:40 +, Gupta, Pekon wrote: Hi, Please see last set of queries inline.. From: Scott Wood [mailto:scottw...@freescale.com] On Sat, 2013-09-28 at 06:24 +, Gupta, Pekon wrote: From: Scott Wood [mailto:scottw...@freescale.com] On Fri, 2013-09-27 at 04:18 +, Gupta, Pekon wrote: [snip] (1) drivers/mtd/nand/fsl_ifc_spl.c (2) drivers/mtd/nand/fsl_elbc_spl.c So CONFIG_SYS_NAND_DEVICE_WIDTH should help them also. right ? How would it help? I meant, instead of requiring the modification in board file, CONFIG_SPL_DEVICE_WIDTH can be put in include/configs/*.h Those registers are already specified in include/configs/*.h for eLBC/IFC. So can this new CONFIG_xx be accepted ? Only for SPL usage. Ok. so should I rename to CONFIG_SPL_NAND_DEVICE_WIDTH ? In theory yes, though the other similar SPL NAND parameters use CONFIG_SYS. -Scott ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/1] am33xx: add CONFIG_SYS_NAND_DEVICE_WIDTH to determine NAND device bus-width
On 25/09/13 06:23, Pekon Gupta wrote: NAND driver needs to know bus-width of the connected NAND device, in order to perform proper I/O and initialize itself. Currently there is no CONFIG option to provide this information to NAND driver. - SPL NAND driver does not have framework to parse ONFI parameter page. - NAND drivers which cannot self initialize !defined(CONFIG_SYS_NAND_SELF_INIT) do not have any information about device bus-width during board_nand_init(), So, any device-width specific configurations are not possible there. - There should be some mechanism to pass device bus-width information for non-ONFI compliant devices. This patch (1) adds CONFIG_SYS_NAND_DEVICE_WIDTH which can take following value 16: NAND device with x16 bus-width 8: NAND device with x8 bus-width (2) removes GPMC_NAND_ECC_LP_x16_LAYOUT, as NAND layout is determined based on ecc-scheme and oobsize during initialization in board_nand_init(). Thus this config is redundant. Signed-off-by: Pekon Gupta pe...@ti.com snip diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h index 978bca7..c92cb2f 100644 --- a/include/configs/am335x_evm.h +++ b/include/configs/am335x_evm.h @@ -216,6 +216,7 @@ #ifdef CONFIG_NAND #define CONFIG_SYS_NAND_5_ADDR_CYCLE +#define CONFIG_SYS_NAND_DEVICE_WIDTH 8 This new setting ... #define CONFIG_SYS_NAND_BLOCK_SIZE 131072 #define CONFIG_SYS_NAND_PAGE_SIZE2048 #define CONFIG_SYS_NAND_OOBSIZE 64 @@ -366,7 +367,6 @@ /* NAND support */ #ifdef CONFIG_NAND #define CONFIG_CMD_NAND -#define GPMC_NAND_ECC_LP_x16_LAYOUT 1 ... does *not* match with what you're taking out here !! #if !defined(CONFIG_SPI_BOOT) !defined(CONFIG_NOR_BOOT) #define MTDIDS_DEFAULT nand0=omap2-nand.0 #define MTDPARTS_DEFAULT mtdparts=omap2-nand.0:128k(SPL), \ diff --git a/include/configs/am3517_crane.h b/include/configs/am3517_crane.h index 1fd2508..0985221 100644 --- a/include/configs/am3517_crane.h +++ b/include/configs/am3517_crane.h @@ -269,7 +269,6 @@ #define CONFIG_SYS_MONITOR_BASE CONFIG_SYS_FLASH_BASE #define CONFIG_NAND_OMAP_GPMC -#define GPMC_NAND_ECC_LP_x16_LAYOUT 1 Same here ... #define CONFIG_ENV_IS_IN_NAND1 #define SMNAND_ENV_OFFSET0x26 /* environment starts here */ @@ -332,6 +331,7 @@ /* NAND boot config */ #define CONFIG_SYS_NAND_5_ADDR_CYCLE +#define CONFIG_SYS_NAND_DEVICE_WIDTH 8 ... !! #define CONFIG_SYS_NAND_PAGE_COUNT 64 #define CONFIG_SYS_NAND_PAGE_SIZE2048 #define CONFIG_SYS_NAND_OOBSIZE 64 diff --git a/include/configs/am3517_evm.h b/include/configs/am3517_evm.h index 6500878..8593d44 100644 --- a/include/configs/am3517_evm.h +++ b/include/configs/am3517_evm.h @@ -263,7 +263,6 @@ #define CONFIG_SYS_MONITOR_BASE CONFIG_SYS_FLASH_BASE #define CONFIG_NAND_OMAP_GPMC -#define GPMC_NAND_ECC_LP_x16_LAYOUT 1 Again ... #define CONFIG_ENV_IS_IN_NAND1 #define SMNAND_ENV_OFFSET0x26 /* environment starts here */ @@ -326,6 +325,7 @@ /* NAND boot config */ #define CONFIG_SYS_NAND_5_ADDR_CYCLE +#define CONFIG_SYS_NAND_DEVICE_WIDTH 8 ... !! #define CONFIG_SYS_NAND_PAGE_COUNT 64 #define CONFIG_SYS_NAND_PAGE_SIZE2048 #define CONFIG_SYS_NAND_OOBSIZE 64 diff --git a/include/configs/cm_t35.h b/include/configs/cm_t35.h index bc5b66c..1e3dd0d 100644 --- a/include/configs/cm_t35.h +++ b/include/configs/cm_t35.h @@ -164,8 +164,6 @@ #define CONFIG_SYS_NAND_BASE NAND_BASE /* physical address */ /* to access nand at */ /* CS0 */ -#define GPMC_NAND_ECC_LP_x8_LAYOUT - And here you don't specify the new setting at all. #define CONFIG_SYS_MAX_NAND_DEVICE 1 /* Max number of NAND */ /* devices */ /* Environment information */ snip There's several other instances of the same issue through the whole patch. Mark J. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/1] am33xx: add CONFIG_SYS_NAND_DEVICE_WIDTH to determine NAND device bus-width
Hi Mark, From: Mark Jackson [mailto:mpfj-l...@newflow.co.uk] To: Gupta, Pekon; scottw...@freescale.com; Rini, Tom snip diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h index 978bca7..c92cb2f 100644 --- a/include/configs/am335x_evm.h +++ b/include/configs/am335x_evm.h @@ -216,6 +216,7 @@ #ifdef CONFIG_NAND #define CONFIG_SYS_NAND_5_ADDR_CYCLE +#define CONFIG_SYS_NAND_DEVICE_WIDTH 8 This new setting ... #define CONFIG_SYS_NAND_BLOCK_SIZE 131072 #define CONFIG_SYS_NAND_PAGE_SIZE 2048 #define CONFIG_SYS_NAND_OOBSIZE64 @@ -366,7 +367,6 @@ /* NAND support */ #ifdef CONFIG_NAND #define CONFIG_CMD_NAND -#define GPMC_NAND_ECC_LP_x16_LAYOUT1 ... does *not* match with what you're taking out here !! This CONFIG was used to define the layout based on (1) large-page of small-page NAND, and (2) BADBLOCK_MARKER position based on x16 or x8 device. Now this define is no more used, as ecc.layout is configured based on ecc-scheme. Please refer following patch. http://lists.denx.de/pipermail/u-boot/2013-September/163867.html Yes, I can put these into independent patch sets. However, if you can please test these patches on your system, This would at-least confirm whether these changes work, then I can re-send this series, with other comments. *Patch Series to test* http://lists.denx.de/pipermail/u-boot/2013-September/163865.html http://lists.denx.de/pipermail/u-boot/2013-September/163878.html with regards, pekon #if !defined(CONFIG_SPI_BOOT) !defined(CONFIG_NOR_BOOT) #define MTDIDS_DEFAULT nand0=omap2-nand.0 #define MTDPARTS_DEFAULT mtdparts=omap2- nand.0:128k(SPL), \ diff --git a/include/configs/am3517_crane.h b/include/configs/am3517_crane.h index 1fd2508..0985221 100644 --- a/include/configs/am3517_crane.h +++ b/include/configs/am3517_crane.h @@ -269,7 +269,6 @@ #define CONFIG_SYS_MONITOR_BASE CONFIG_SYS_FLASH_BASE #define CONFIG_NAND_OMAP_GPMC -#define GPMC_NAND_ECC_LP_x16_LAYOUT1 Same here ... #define CONFIG_ENV_IS_IN_NAND 1 #define SMNAND_ENV_OFFSET 0x26 /* environment starts here */ @@ -332,6 +331,7 @@ /* NAND boot config */ #define CONFIG_SYS_NAND_5_ADDR_CYCLE +#define CONFIG_SYS_NAND_DEVICE_WIDTH 8 ... !! #define CONFIG_SYS_NAND_PAGE_COUNT 64 #define CONFIG_SYS_NAND_PAGE_SIZE 2048 #define CONFIG_SYS_NAND_OOBSIZE64 diff --git a/include/configs/am3517_evm.h b/include/configs/am3517_evm.h index 6500878..8593d44 100644 --- a/include/configs/am3517_evm.h +++ b/include/configs/am3517_evm.h @@ -263,7 +263,6 @@ #define CONFIG_SYS_MONITOR_BASE CONFIG_SYS_FLASH_BASE #define CONFIG_NAND_OMAP_GPMC -#define GPMC_NAND_ECC_LP_x16_LAYOUT1 Again ... #define CONFIG_ENV_IS_IN_NAND 1 #define SMNAND_ENV_OFFSET 0x26 /* environment starts here */ @@ -326,6 +325,7 @@ /* NAND boot config */ #define CONFIG_SYS_NAND_5_ADDR_CYCLE +#define CONFIG_SYS_NAND_DEVICE_WIDTH 8 ... !! #define CONFIG_SYS_NAND_PAGE_COUNT 64 #define CONFIG_SYS_NAND_PAGE_SIZE 2048 #define CONFIG_SYS_NAND_OOBSIZE64 diff --git a/include/configs/cm_t35.h b/include/configs/cm_t35.h index bc5b66c..1e3dd0d 100644 --- a/include/configs/cm_t35.h +++ b/include/configs/cm_t35.h @@ -164,8 +164,6 @@ #define CONFIG_SYS_NAND_BASE NAND_BASE /* physical address */ /* to access nand at */ /* CS0 */ -#define GPMC_NAND_ECC_LP_x8_LAYOUT - And here you don't specify the new setting at all. #define CONFIG_SYS_MAX_NAND_DEVICE 1 /* Max number of NAND */ /* devices */ /* Environment information */ snip There's several other instances of the same issue through the whole patch. Mark J. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/1] am33xx: add CONFIG_SYS_NAND_DEVICE_WIDTH to determine NAND device bus-width
On Sat, 2013-09-28 at 06:24 +, Gupta, Pekon wrote: From: Scott Wood [mailto:scottw...@freescale.com] On Fri, 2013-09-27 at 04:18 +, Gupta, Pekon wrote: Apart from SPL, CONFIG_SYS_NAND_DEVICE_WIDTH also be useful for (1) drivers which do not use CONFIG_SYS_NAND_ONFI_DETECTION, where code for reading on-chip ONFI parameters is not enabled in nand_base.c (2) non ONFI compatible NAND devices. Unlikely, given that they've all managed to work without this so far. E.g. eLBC and IFC hardcode this information on a per-chip basis in the #defines that hold values for config registers, and prior to this patch omap_gpmc had code to read a config register (regardless of where it originally got set). (1) drivers/mtd/nand/fsl_ifc_spl.c They are doing same way as OMAP used to. They are also using controller configurations to tell driver about the]NAND bus-width port_size = (cspr CSPR_PORT_SIZE_16) ? 16 : 8; Yes. Note that CSPR is set per-chip. (2) drivers/mtd/nand/fsl_elbc_spl.c They are doing incomplete check. Rather they are not caring for x16 device Right, I forgot that eLBC doesn't support 16-bit NAND (hardware limitation). So CONFIG_SYS_NAND_DEVICE_WIDTH should help them also. right ? How would it help? So can this new CONFIG_xx be accepted ? Only for SPL usage. It looked like you were removing the code that does dynamic detection, which would also affect non-SPL. -/* If we are 16 bit dev, our gpmc config tells us that */ -if ((readl(gpmc_cfg-cs[cs].config1) 0x3000) == 0x1000) omap_gpmc.c never had dynamic detection support. Above gpmc_config bit which is used to tell whether device is x16 or x8, gets actually hard-coded in gpmc_init(). Thus it was actually a mechanism to pass hard-coded bus- width information to nand driver. Refer: arch/arm/cpu/armv7/am33xx/mem.c : gpmc_init() So, instead of hacking the gpmc_init() everytime for different devices, this patch introduces a generic CONFIG which can be used everywhere. It looks like you do more NAND config in gpmc_init() than just setting this one bit, so I don't think you save anything here. BTW, do you not need to set this bit in the config register for the hardware to work in the SPL case? Yes, I'm not changing the default configs for GPMC in gpmc_init(), because they are ok for x8 device. I'm just overriding them again during board_nand_init() if CONFIG_SYS_NAND_DEVICE_WIDTH == x16 device. Is this due to wanting to do identification as x8? -Scott ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/1] am33xx: add CONFIG_SYS_NAND_DEVICE_WIDTH to determine NAND device bus-width
From: Scott Wood [mailto:scottw...@freescale.com] On Fri, 2013-09-27 at 04:18 +, Gupta, Pekon wrote: From: Scott Wood [mailto:scottw...@freescale.com] On Thu, 2013-09-26 at 13:14 +, Gupta, Pekon wrote: From: Gupta, Pekon pe...@ti.com [snip] The changelog that introduced am335x_spl_bch.c says that custom read_page implementation is required for proper syndrome generation. Other than that ECC mode, AFAICT you're already using nand_spl_simple.c. Oh I missed referring to nand_spl_simple.c. Thanks for pointing out. Yes, we already have a generic SPL driver. because SPL is mostly statically configured, and it just does plain NAND read (it doesn’t even support NAND write, etc). But I do not know the hardware configurations tweaks of other SoC, So at-least have common CONFIG_SYS_NAND_xx which all SPL drivers can use. Again, are you introducing this symbol for use only in SPL? Apart from SPL, CONFIG_SYS_NAND_DEVICE_WIDTH also be useful for (1) drivers which do not use CONFIG_SYS_NAND_ONFI_DETECTION, where code for reading on-chip ONFI parameters is not enabled in nand_base.c (2) non ONFI compatible NAND devices. Unlikely, given that they've all managed to work without this so far. E.g. eLBC and IFC hardcode this information on a per-chip basis in the #defines that hold values for config registers, and prior to this patch omap_gpmc had code to read a config register (regardless of where it originally got set). (1) drivers/mtd/nand/fsl_ifc_spl.c They are doing same way as OMAP used to. They are also using controller configurations to tell driver about the]NAND bus-width port_size = (cspr CSPR_PORT_SIZE_16) ? 16 : 8; (2) drivers/mtd/nand/fsl_elbc_spl.c They are doing incomplete check. Rather they are not caring for x16 device For a x16 device, badblock marker should be 16-bit (2 bytes) but here they are checking of only 1st byte. if (i++ 2 buf[page_offs + bad_marker] != 0xff) So CONFIG_SYS_NAND_DEVICE_WIDTH should help them also. right ? So can this new CONFIG_xx be accepted ? It looked like you were removing the code that does dynamic detection, which would also affect non-SPL. - /* If we are 16 bit dev, our gpmc config tells us that */ - if ((readl(gpmc_cfg-cs[cs].config1) 0x3000) == 0x1000) omap_gpmc.c never had dynamic detection support. Above gpmc_config bit which is used to tell whether device is x16 or x8, gets actually hard-coded in gpmc_init(). Thus it was actually a mechanism to pass hard-coded bus- width information to nand driver. Refer: arch/arm/cpu/armv7/am33xx/mem.c : gpmc_init() So, instead of hacking the gpmc_init() everytime for different devices, this patch introduces a generic CONFIG which can be used everywhere. It looks like you do more NAND config in gpmc_init() than just setting this one bit, so I don't think you save anything here. BTW, do you not need to set this bit in the config register for the hardware to work in the SPL case? Yes, I'm not changing the default configs for GPMC in gpmc_init(), because they are ok for x8 device. I'm just overriding them again during board_nand_init() if CONFIG_SYS_NAND_DEVICE_WIDTH == x16 device. + /* reconfigure GPMC.CONFIG1 register with correct device-width */ + gpmc_config = readl(gpmc_cfg-cs[cs].config1); + if (nand-options NAND_BUSWIDTH_16) + gpmc_config |= (0x1 12); + else + gpmc_config = ~(0x1 12); + writel(gpmc_config, gpmc_cfg-cs[cs].config1); + This way im not breaking any backward compatibility, just avoiding the need to hack the board file for x16 devices. with regards, pekon ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/1] am33xx: add CONFIG_SYS_NAND_DEVICE_WIDTH to determine NAND device bus-width
On Fri, 2013-09-27 at 04:18 +, Gupta, Pekon wrote: From: Scott Wood [mailto:scottw...@freescale.com] On Thu, 2013-09-26 at 13:14 +, Gupta, Pekon wrote: From: Gupta, Pekon pe...@ti.com NAND driver needs to know bus-width of the connected NAND device, in order to perform proper I/O and initialize itself. Currently there is no CONFIG option to provide this information to NAND driver. - SPL NAND driver does not have framework to parse ONFI parameter page. Is this about SPL? It looks like a more general change. Yes, actually I would have loved to see a generic SPL driver for all platforms, How could that possibly work? NAND SPL (as in drivers/mtd/nand/am335x_spl_bch.c) had following limitation (a) depends on CONFIG_SYS_NAND_xx (for NAND device parameters like erasesize, pagesize, oobsize, etc). (b) can only do NAND read access. (c) calls nand_chip-ecc.hwctl() for doing controller specific configurations, which is populated while doing board_nand_init(). Above (a), (b), and (c) do not have any SoC specific dependency. And can be put into a generic framework which can be used for all SPL drivers. Not all NAND controllers expose a low-level interface that matches hwctl() (e.g. fsl_ifc and fsl_elbc). For those that do, we do have a common nand_spl_simple.c that works for many drivers, without needing to hardcode the bus width (though perhaps a few bytes could be shaved off by hardcoding it). BTW, referring to driver in NAND context is ambiguous... I prefer to use it to refer to the controller drivers when not qualified as high level driver or similar. All SoC specific configurations are done in either: - board_nand_init(): initializations based on device. - nand_chip-ecc.hwctl(): configurations for ECC scheme. which is *shared* code between SPL u-boot driver, and is present in u-boot driver file (like drivers/mtd/nand/omap_gpmc.c), not in SPL driver file. The missing piece was device bus-width detection which I addresses in this patch by adding CONFIG_SYS_NAND_DEVICE_WIDTH. The changelog that introduced am335x_spl_bch.c says that custom read_page implementation is required for proper syndrome generation. Other than that ECC mode, AFAICT you're already using nand_spl_simple.c. because SPL is mostly statically configured, and it just does plain NAND read (it doesn’t even support NAND write, etc). But I do not know the hardware configurations tweaks of other SoC, So at-least have common CONFIG_SYS_NAND_xx which all SPL drivers can use. Again, are you introducing this symbol for use only in SPL? Apart from SPL, CONFIG_SYS_NAND_DEVICE_WIDTH also be useful for (1) drivers which do not use CONFIG_SYS_NAND_ONFI_DETECTION, where code for reading on-chip ONFI parameters is not enabled in nand_base.c (2) non ONFI compatible NAND devices. Unlikely, given that they've all managed to work without this so far. E.g. eLBC and IFC hardcode this information on a per-chip basis in the #defines that hold values for config registers, and prior to this patch omap_gpmc had code to read a config register (regardless of where it originally got set). It looked like you were removing the code that does dynamic detection, which would also affect non-SPL. -/* If we are 16 bit dev, our gpmc config tells us that */ -if ((readl(gpmc_cfg-cs[cs].config1) 0x3000) == 0x1000) omap_gpmc.c never had dynamic detection support. Above gpmc_config bit which is used to tell whether device is x16 or x8, gets actually hard-coded in gpmc_init(). Thus it was actually a mechanism to pass hard-coded bus-width information to nand driver. Refer: arch/arm/cpu/armv7/am33xx/mem.c : gpmc_init() So, instead of hacking the gpmc_init() everytime for different devices, this patch introduces a generic CONFIG which can be used everywhere. It looks like you do more NAND config in gpmc_init() than just setting this one bit, so I don't think you save anything here. BTW, do you not need to set this bit in the config register for the hardware to work in the SPL case? -Scott ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/1] am33xx: add CONFIG_SYS_NAND_DEVICE_WIDTH to determine NAND device bus-width
(re-sending by dropping cc-list, as u-boot maillist moderated earlier mail saying Too many recipients to the message :-) ) From: Scott Wood [mailto:scottw...@freescale.com] On Wed, 2013-09-25 at 22:08 +, Woodruff, Richard wrote: Short comment. Apologies for top posting: The first incarnations of SPL and loader mainly cared about the boot flash device. Is there an SPL that cares about something other than the boot device now? AFAIK about SPL driver in drivers/mtd/nand/am335x_spl_bch.c This driver depends on CONFIG_SYS_NAND_xx for deriving all device parameter - CONFIG_SYS_NAND_BLOCK_SIZE - CONFIG_SYS_NAND_PAGE_SIZE - CONFIG_SYS_NAND_ECCSIZE - CONFIG_SYS_NAND_ECCBYTES Thus, IMO SPL drivers are written for following two uses: (1) load secondary boot-loader, which can further do dynamic device probing And based on that enables some device specific features. (2) load linux kernel image directly (in falcon-mode SPL_OS_BOOT), which will reduce the boot-time as all configurations are static. So, in either usage SPL drivers need to be statically configured to keep their foot-print small and boot them fast. OMAPs require a resistor strap to specify the width of the boot device. The values is latched for SW to read. As such always a run time check of width was sufficient for boot device. On dev board we would have many devices and use DIP switch to select which one was in use. A run time not compile time was way to support this. If information is dynamically available it is better to use this if focus is boot device management. Agree, This was done on TI814x EVM boards which are development boards. but if you refer actual production board (custom boards of end-user), having even a single extra resistor means adding BOM cost. Also in production boards everything is known, (like NAND bus-width, page-size, etc), so dynamic detection becomes redundant because linux would anyways re-do the device probing when it boots. And if anyone really wants to do dynamic device probing in u-boot. Then go till u-boot second stage which has most of device probing capabilities same as linux kernel. (atleast in case of NAND driver). Other tricks writing patterns and reading results is also possible but simplicity of compile vs. that is debatable. Regards, Richard W. I agree. Outside of SPL (where hardcoding can be useful due to size constraints, and you only care about the boot device), it's best to let drivers determine the best way to learn about such configuration. From: Gupta, Pekon pe...@ti.com NAND driver needs to know bus-width of the connected NAND device, in order to perform proper I/O and initialize itself. Currently there is no CONFIG option to provide this information to NAND driver. - SPL NAND driver does not have framework to parse ONFI parameter page. Is this about SPL? It looks like a more general change. Yes, actually I would have loved to see a generic SPL driver for all platforms, because SPL is mostly statically configured, and it just does plain NAND read (it doesn’t even support NAND write, etc). But I do not know the hardware configurations tweaks of other SoC, So at-least have common CONFIG_SYS_NAND_xx which all SPL drivers can use. - NAND drivers which cannot self initialize !defined(CONFIG_SYS_NAND_SELF_INIT) do not have any information about device bus-width during board_nand_init(), So, any device-width specific configurations are not possible there. Convert to self-init. That's what it's for. I read it in doc/README.nand: CONFIG_SYS_NAND_SELF_INIT And this path is applicable only for second-stage u-boot loader. But today not many drivers are using this model. So just mentioned it here. With regards, pekon ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/1] am33xx: add CONFIG_SYS_NAND_DEVICE_WIDTH to determine NAND device bus-width
On Thu, 2013-09-26 at 13:14 +, Gupta, Pekon wrote: From: Gupta, Pekon pe...@ti.com NAND driver needs to know bus-width of the connected NAND device, in order to perform proper I/O and initialize itself. Currently there is no CONFIG option to provide this information to NAND driver. - SPL NAND driver does not have framework to parse ONFI parameter page. Is this about SPL? It looks like a more general change. Yes, actually I would have loved to see a generic SPL driver for all platforms, How could that possibly work? because SPL is mostly statically configured, and it just does plain NAND read (it doesn’t even support NAND write, etc). But I do not know the hardware configurations tweaks of other SoC, So at-least have common CONFIG_SYS_NAND_xx which all SPL drivers can use. Again, are you introducing this symbol for use only in SPL? It looked like you were removing the code that does dynamic detection, which would also affect non-SPL. -Scott ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/1] am33xx: add CONFIG_SYS_NAND_DEVICE_WIDTH to determine NAND device bus-width
From: Scott Wood [mailto:scottw...@freescale.com] On Wed, 2013-09-25 at 22:08 +, Woodruff, Richard wrote: Short comment. Apologies for top posting: The first incarnations of SPL and loader mainly cared about the boot flash device. Is there an SPL that cares about something other than the boot device now? AFAIK about SPL driver in drivers/mtd/nand/am335x_spl_bch.c This driver depends on CONFIG_SYS_NAND_xx for deriving all device parameter - CONFIG_SYS_NAND_BLOCK_SIZE - CONFIG_SYS_NAND_PAGE_SIZE - CONFIG_SYS_NAND_ECCSIZE - CONFIG_SYS_NAND_ECCBYTES Thus, IMO SPL drivers are written for following two uses: (1) load secondary boot-loader, which can further do dynamic device probing And based on that enables some device specific features. (2) load linux kernel image directly (in falcon-mode SPL_OS_BOOT), which will reduce the boot-time as all configurations are static. So, in either usage SPL drivers need to be statically configured to keep their foot-print small and boot them fast. OMAPs require a resistor strap to specify the width of the boot device. The values is latched for SW to read. As such always a run time check of width was sufficient for boot device. On dev board we would have many devices and use DIP switch to select which one was in use. A run time not compile time was way to support this. If information is dynamically available it is better to use this if focus is boot device management. Agree, This was done on TI814x EVM boards which are development boards. but if you refer actual production board (custom boards of end-user), having even a single extra resistor means adding BOM cost. Also in production boards everything is known, (like NAND bus-width, page-size, etc), so dynamic detection becomes redundant because linux would anyways re-do the device probing when it boots. And if anyone really wants to do dynamic device probing in u-boot. Then go till u-boot second stage which has most of device probing capabilities same as linux kernel. (atleast in case of NAND driver). Other tricks writing patterns and reading results is also possible but simplicity of compile vs. that is debatable. Regards, Richard W. I agree. Outside of SPL (where hardcoding can be useful due to size constraints, and you only care about the boot device), it's best to let drivers determine the best way to learn about such configuration. From: Gupta, Pekon pe...@ti.com NAND driver needs to know bus-width of the connected NAND device, in order to perform proper I/O and initialize itself. Currently there is no CONFIG option to provide this information to NAND driver. - SPL NAND driver does not have framework to parse ONFI parameter page. Is this about SPL? It looks like a more general change. Yes, actually I would have loved to see a generic SPL driver for all platforms, because SPL is mostly statically configured, and it just does plain NAND read (it doesn’t even support NAND write, etc). But I do not know the hardware configurations tweaks of other SoC, So at-least have common CONFIG_SYS_NAND_xx which all SPL drivers can use. - NAND drivers which cannot self initialize !defined(CONFIG_SYS_NAND_SELF_INIT) do not have any information about device bus-width during board_nand_init(), So, any device-width specific configurations are not possible there. Convert to self-init. That's what it's for. I read it in doc/README.nand: CONFIG_SYS_NAND_SELF_INIT And this path is applicable only for second-stage u-boot loader. But today not many drivers are using this model. So just mentioned it here. With regards, pekon ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/1] am33xx: add CONFIG_SYS_NAND_DEVICE_WIDTH to determine NAND device bus-width
From: Scott Wood [mailto:scottw...@freescale.com] On Thu, 2013-09-26 at 13:14 +, Gupta, Pekon wrote: From: Gupta, Pekon pe...@ti.com NAND driver needs to know bus-width of the connected NAND device, in order to perform proper I/O and initialize itself. Currently there is no CONFIG option to provide this information to NAND driver. - SPL NAND driver does not have framework to parse ONFI parameter page. Is this about SPL? It looks like a more general change. Yes, actually I would have loved to see a generic SPL driver for all platforms, How could that possibly work? NAND SPL (as in drivers/mtd/nand/am335x_spl_bch.c) had following limitation (a) depends on CONFIG_SYS_NAND_xx (for NAND device parameters like erasesize, pagesize, oobsize, etc). (b) can only do NAND read access. (c) calls nand_chip-ecc.hwctl() for doing controller specific configurations, which is populated while doing board_nand_init(). Above (a), (b), and (c) do not have any SoC specific dependency. And can be put into a generic framework which can be used for all SPL drivers. All SoC specific configurations are done in either: - board_nand_init(): initializations based on device. - nand_chip-ecc.hwctl(): configurations for ECC scheme. which is *shared* code between SPL u-boot driver, and is present in u-boot driver file (like drivers/mtd/nand/omap_gpmc.c), not in SPL driver file. The missing piece was device bus-width detection which I addresses in this patch by adding CONFIG_SYS_NAND_DEVICE_WIDTH. because SPL is mostly statically configured, and it just does plain NAND read (it doesn’t even support NAND write, etc). But I do not know the hardware configurations tweaks of other SoC, So at-least have common CONFIG_SYS_NAND_xx which all SPL drivers can use. Again, are you introducing this symbol for use only in SPL? Apart from SPL, CONFIG_SYS_NAND_DEVICE_WIDTH also be useful for (1) drivers which do not use CONFIG_SYS_NAND_ONFI_DETECTION, where code for reading on-chip ONFI parameters is not enabled in nand_base.c (2) non ONFI compatible NAND devices. It looked like you were removing the code that does dynamic detection, which would also affect non-SPL. - /* If we are 16 bit dev, our gpmc config tells us that */ - if ((readl(gpmc_cfg-cs[cs].config1) 0x3000) == 0x1000) omap_gpmc.c never had dynamic detection support. Above gpmc_config bit which is used to tell whether device is x16 or x8, gets actually hard-coded in gpmc_init(). Thus it was actually a mechanism to pass hard-coded bus-width information to nand driver. Refer: arch/arm/cpu/armv7/am33xx/mem.c : gpmc_init() So, instead of hacking the gpmc_init() everytime for different devices, this patch introduces a generic CONFIG which can be used everywhere. I have sent this patch separately to see the acceptance of new generic CONFIG_SYS_NAND_DEVICE_WIDTH. If this gets accepted, then I'll resend the patch-series which actually does dynamic detection.. http://lists.denx.de/pipermail/u-boot/2013-September/162295.html http://lists.denx.de/pipermail/u-boot/2013-September/162296.html (dropping others in CC list as u-boot maillist rejects the mail saying too many recipients. !! ) with regards, pekon ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/1] am33xx: add CONFIG_SYS_NAND_DEVICE_WIDTH to determine NAND device bus-width
Short comment. Apologies for top posting: The first incarnations of SPL and loader mainly cared about the boot flash device. OMAPs require a resistor strap to specify the width of the boot device. The values is latched for SW to read. As such always a run time check of width was sufficient for boot device. On dev board we would have many devices and use DIP switch to select which one was in use. A run time not compile time was way to support this. If information is dynamically available it is better to use this if focus is boot device management. Other tricks writing patterns and reading results is also possible but simplicity of compile vs. that is debatable. Regards, Richard W. -Original Message- From: Gupta, Pekon Sent: Wednesday, September 25, 2013 12:18 AM To: scottw...@freescale.com; Rini, Tom Cc: u-boot@lists.denx.de; Balbi, Felipe; Kipisz, Steven; sba...@denx.de; nota...@gmail.com; luca.ceres...@comelit.it; Woodruff, Richard; we...@corscience.de; peter.bar...@logicpd.com; frede...@kriewitz.eu; tom@windriver.com; Menon, Nishanth; srin...@mistralsolutions.com; Hiremath, Vaibhav; Gupta, Pekon Subject: [PATCH 1/1] am33xx: add CONFIG_SYS_NAND_DEVICE_WIDTH to determine NAND device bus-width NAND driver needs to know bus-width of the connected NAND device, in order to perform proper I/O and initialize itself. Currently there is no CONFIG option to provide this information to NAND driver. - SPL NAND driver does not have framework to parse ONFI parameter page. - NAND drivers which cannot self initialize !defined(CONFIG_SYS_NAND_SELF_INIT) do not have any information about device bus-width during board_nand_init(), So, any device-width specific configurations are not possible there. - There should be some mechanism to pass device bus-width information for non-ONFI compliant devices. @@ -772,10 +768,10 @@ int board_nand_init(struct nand_chip *nand) nand-priv = bch_priv; nand-cmd_ctrl = omap_nand_hwcontrol; nand-options |= NAND_NO_PADDING | NAND_CACHEPRG; - /* If we are 16 bit dev, our gpmc config tells us that */ - if ((readl(gpmc_cfg-cs[cs].config1) 0x3000) == 0x1000) ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/1] am33xx: add CONFIG_SYS_NAND_DEVICE_WIDTH to determine NAND device bus-width
On Wed, 2013-09-25 at 22:08 +, Woodruff, Richard wrote: Short comment. Apologies for top posting: The first incarnations of SPL and loader mainly cared about the boot flash device. Is there an SPL that cares about something other than the boot device now? OMAPs require a resistor strap to specify the width of the boot device. The values is latched for SW to read. As such always a run time check of width was sufficient for boot device. On dev board we would have many devices and use DIP switch to select which one was in use. A run time not compile time was way to support this. If information is dynamically available it is better to use this if focus is boot device management. Other tricks writing patterns and reading results is also possible but simplicity of compile vs. that is debatable. Regards, Richard W. I agree. Outside of SPL (where hardcoding can be useful due to size constraints, and you only care about the boot device), it's best to let drivers determine the best way to learn about such configuration. -Original Message- From: Gupta, Pekon Sent: Wednesday, September 25, 2013 12:18 AM To: scottw...@freescale.com; Rini, Tom Cc: u-boot@lists.denx.de; Balbi, Felipe; Kipisz, Steven; sba...@denx.de; nota...@gmail.com; luca.ceres...@comelit.it; Woodruff, Richard; we...@corscience.de; peter.bar...@logicpd.com; frede...@kriewitz.eu; tom@windriver.com; Menon, Nishanth; srin...@mistralsolutions.com; Hiremath, Vaibhav; Gupta, Pekon Subject: [PATCH 1/1] am33xx: add CONFIG_SYS_NAND_DEVICE_WIDTH to determine NAND device bus-width NAND driver needs to know bus-width of the connected NAND device, in order to perform proper I/O and initialize itself. Currently there is no CONFIG option to provide this information to NAND driver. - SPL NAND driver does not have framework to parse ONFI parameter page. Is this about SPL? It looks like a more general change. - NAND drivers which cannot self initialize !defined(CONFIG_SYS_NAND_SELF_INIT) do not have any information about device bus-width during board_nand_init(), So, any device-width specific configurations are not possible there. Convert to self-init. That's what it's for. -Scott ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
