RFC: OMAP GPMC DT bindings
Subsystems (8)
v3 cover RFC: OMAP GPMC DT bindings
This is a series of patches to support GPMC peripherals on OMAP boards.
Depends on Linus' master +
omap-next (branch omap-for-v3.8/cleanup-headers-gpmc)
The only supported peripheral for now is NAND, but other types would be easy to add.
Version 2 addresses details pointed out by Jon Hunter, Afzal Mohammed and Rob Herring:
- add "reg" and "ti,hwmod" properties to Documentation
- use generic of_mtd functions and the property names defined by them, namely "nand-bus-width" and "nand-ecc-mode"
- reduce the default register space size in the Documentation to 8K, as found in the hwmod code
- switch to a DT layout based on ranges and address translation. Although this property is not currently looked at as long as the handling code still uses the runtime calculation methods, we now have these values in the bindings, eventually allowing us to switch the implementation with less pain.
Version 3 includes fixes pointed out by Jon Hunter:
- better documentation of the 'ranges' property to describe the fact that it's representing the CS lines
- GPMC_CS_CONFIGx -> GPMC_CONFIGx in comments
- drop interrupt-parent from example bindings
- add of_node_put() at the end of the child iteration
Daniel Mack (4):
mtd: omap-nand: pass device_node in platform data
ARM: OMAP: gpmc: enable hwecc for AM33xx SoCs
ARM: OMAP: gpmc: don't create devices from initcall on DT
ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND
Documentation/devicetree/bindings/bus/ti-gpmc.txt | 76 +++++++++++ .../devicetree/bindings/mtd/gpmc-nand.txt | 60 +++++++++ arch/arm/mach-omap2/gpmc-nand.c | 2 +- arch/arm/mach-omap2/gpmc.c | 148 +++++++++++++++++++++ drivers/mtd/nand/omap2.c | 4 +- include/linux/platform_data/mtd-nand-omap2.h | 2 + 6 files changed, 290 insertions(+), 2 deletions(-) create mode 100644 Documentation/devicetree/bindings/bus/ti-gpmc.txt create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt
Signature · 1 line
1.7.11.7
PATCH v3 1/4 mtd: omap-nand: pass device_node in platform data
Pass an optional device_node pointer in the platform data, which in turn will be put into a mtd_part_parser_data. This way, code that sets up the platform devices can pass along the node from DT so that the partitions can be parsed.
For non-DT boards, this change has no effect.
--- drivers/mtd/nand/omap2.c | 4 +++- include/linux/platform_data/mtd-nand-omap2.h | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-)
drivers/mtd/nand/omap2.c +3 -1
@@ -1331,6 +1331,7 @@ static int __devinit omap_nand_probe(struct platform_device *pdev)dma_cap_mask_t mask;unsigned sig;struct resource *res; +struct mtd_part_parser_data ppdata = {}; pdata = pdev->dev.platform_data;if (pdata ==NULL ) {
@@ -1556,7 +1557,8 @@ static int __devinit omap_nand_probe(struct platform_device *pdev) goto out_release_mem_region; } -mtd_device_parse_register (&info->mtd,NULL ,NULL , pdata->parts, + ppdata.of_node = pdata->of_node; +mtd_device_parse_register (&info->mtd,NULL , &ppdata, pdata->parts, pdata->nr_parts);platform_set_drvdata (pdev, &info->mtd);
include/linux/platform_data/mtd-nand-omap2.h +2 -0
@@ -60,6 +60,8 @@ struct omap_nand_platform_data {int devsize;enum omap_ecc ecc_opt;struct gpmc_nand_regs reg; +/* for passing the partitions */ + struct device_node *of_node; };#endif
Signature · 1 line
1.7.11.7
PATCH v3 2/4 ARM: OMAP: gpmc: enable hwecc for AM33xx SoCs
In reply to Daniel Mack--- arch/arm/mach-omap2/gpmc-nand.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/mach-omap2/gpmc-nand.c +1 -1
@@ -92,7 +92,7 @@ static int omap2_nand_gpmc_retime(static bool __init gpmc_hwecc_bch_capable (enum omap_ecc ecc_opt) {/* support only OMAP3 class */ -if (!cpu_is_omap34xx ()) { +if (!cpu_is_omap34xx () && !soc_is_am33xx ()) {pr_err ("BCH ecc is not supported on this CPU\n" );return 0 ; }
Signature · 1 line
1.7.11.7
PATCH v3 2/4 ARM: OMAP: gpmc: enable hwecc for AM33xx SoCs
Quoted 10 lines · Daniel Mack
Signed-off-by Daniel Mack <[email protected]>--- arch/arm/mach-omap2/gpmc-nand.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/arm/mach-omap2/gpmc-nand.c +0
-0
arch/arm/mach-omap2/gpmc-nand.c static int omap2_nand_gpmc_retime( · line 98
Quoted 8 lines
arch/arm/mach-omap2/gpmc-nand.c +1
-1@@ -92,7 +92,7 @@ static int omap2_nand_gpmc_retime(static bool __init gpmc_hwecc_bch_capable (enum omap_ecc ecc_opt) {/* support only OMAP3 class */ -if (!cpu_is_omap34xx ()) { +if (!cpu_is_omap34xx () && !soc_is_am33xx ()) {pr_err ("BCH ecc is not supported on this CPU\n" );return 0 ; }
Please add '!soc_is_am33xx()' check for BCH4 failure case also. BCH4 support tested in AM335x.
https://lkml.org/lkml/2012/10/31/82
Thanks
Avinash
Quoted 9 lines
Signature · 7 lines
1.7.11.7 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH v3 3/4 ARM: OMAP: gpmc: don't create devices from initcall on DT
In reply to Daniel MackOn DT driven boards, the gpmc node will match the driver. Hence, there's no need to do that unconditionally from the initcall.
--- arch/arm/mach-omap2/gpmc.c | 7 +++++++ 1 file changed, 7 insertions(+)
arch/arm/mach-omap2/gpmc.c +7 -0
@@ -844,6 +844,13 @@ static int __init omap_gpmc_init(void)struct platform_device *pdev;char *oh_name ="gpmc" ; +/* +* if the board boots up with a populated DT, do not +* manually add the device from this initcall +*/ +if (of_have_populated_dt ()) +return -ENODEV ; + oh =omap_hwmod_lookup (oh_name);if (!oh) {pr_err ("Could not look up %s\n" , oh_name);
Signature · 1 line
1.7.11.7
PATCH v3 4/4 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND
In reply to Daniel MackThis patch adds basic DT bindings for OMAP GPMC.
The actual peripherals are instanciated from child nodes within the GPMC node, and the only type of device that is currently supported is NAND.
Code was added to parse the generic GPMC timing parameters and some documentation with examples on how to use them.
Successfully tested on an AM33xx board.
--- Documentation/devicetree/bindings/bus/ti-gpmc.txt | 76 +++++++++++ .../devicetree/bindings/mtd/gpmc-nand.txt | 60 +++++++++ arch/arm/mach-omap2/gpmc.c | 141 +++++++++++++++++++++ 3 files changed, 277 insertions(+) create mode 100644 Documentation/devicetree/bindings/bus/ti-gpmc.txt create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt
Documentation/devicetree/bindings/bus/ti-gpmc.txt new file +76 -0
@@ -0,0 +1,76 @@ +Device tree bindings for OMAP general purpose memory controllers (GPMC) + +The actual devices are instantiated from the child nodes of a GPMC node. + +Required properties: + + - compatible: Should be set to "ti,gpmc" + - reg: A resource specifier for the register space + (see the example below) + - ti,hwmods: Should be set to "ti,gpmc" until the DT transition is + completed. + - #address-cells: Must be set to 2 to allow memory address translation + - #size-cells: Must be set to 1 to allow CS address passing + - ranges: Must be set up to reflect the memory layout with four + integer values for each chip-select line in use: + + <cs-number> 0 <physical address of mapping> <size> + + Note that this property is not currently parsed. + Calculated values derived from the contents of the + per-CS register GPMC_CONFIG7 (as set up by the + bootloader) are used. That will change in the future, + so be sure to fill the correct values here. + +Timing properties for child nodes. All are optional and default to 0. + + - gpmc,sync-clk: Minimum clock period for synchronous mode, in picoseconds + + Chip-select signal timings corresponding to GPMC_CONFIG2: + - gpmc,cs-on: Assertion time + - gpmc,cs-rd-off: Read deassertion time + - gpmc,cs-wr-off: Write deassertion time + + ADV signal timings corresponding to GPMC_CONFIG3: + - gpmc,adv-on: Assertion time + - gpmc,adv-rd-off: Read deassertion time + - gpmc,adv-wr-off: Write deassertion time + + WE signals timings corresponding to GPMC_CONFIG4: + - gpmc,we-on: Assertion time + - gpmc,we-off: Deassertion time + + OE signals timings corresponding to GPMC_CONFIG4: + - gpmc,oe-on: Assertion time + - gpmc,oe-off: Deassertion time + + Access time and cycle time timings corresponding to GPMC_CONFIG5: + - gpmc,page-burst-access: Multiple access word delay + - gpmc,access: Start-cycle to first data valid delay + - gpmc,rd-cycle: Total read cycle time + - gpmc,wr-cycle: Total write cycle time + +The following are only on OMAP3430: + - gpmc,wr-access + - gpmc,wr-data-mux-bus + + +Example for an AM33xx board: + + gpmc: gpmc at 50000000 { + compatible = "ti,gpmc"; + ti,hwmods = "gpmc"; + reg = <0x50000000 0x2000>; + interrupts = <100>; + + #address-cells = <2>; + #size-cells = <1>; + ranges = <0 0 0x08000000 0x10000000>; /* CS0 @addr 0x8000000, size 0x10000000 */ + + /* child nodes go here */ + }; + + + + +
Documentation/devicetree/bindings/mtd/gpmc-nand.txt new file +60 -0
60 lines not on this page · raw message
arch/arm/mach-omap2/gpmc.c +141 -0
171 lines not on this page · raw message
Signature · 1 line
1.7.11.7
PATCH v3 4/4 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND
Quoted 11 lines · Daniel Mack
This patch adds basic DT bindings for OMAP GPMC.
The actual peripherals are instanciated from child nodes within the GPMC node, and the only type of device that is currently supported is NAND.
Code was added to parse the generic GPMC timing parameters and some documentation with examples on how to use them.
Successfully tested on an AM33xx board.
Signed-off-by Daniel Mack <[email protected]>
[...]
Quoted 14 lines
Documentation/devicetree/bindings/mtd/gpmc-nand.txt new file +14
-0+ + nand at 0,0 { + reg = <0 0 0>; /* CS0, offset 0 */ + nand-bus-width = <16>; + nand-ecc-mode = "none"; + + gpmc,sync-clk = <0>; + gpmc,cs-on = <0>; + gpmc,cs-rd-off = <36>; + gpmc,cs-wr-off = <36>; + gpmc,adv-on = <6>; + gpmc,adv-rd-off = <24>; + gpmc,adv-wr-off = <36>; + gpmc,we-off = <30>;
Documentation/devicetree/bindings/mtd/gpmc-nand.txt line 56
Quoted 10 lines
Documentation/devicetree/bindings/mtd/gpmc-nand.txt new file +10
-0+ gpmc,oe-off = <48>; + gpmc,access = <54>; + gpmc,rd-cycle = <72>; + gpmc,wr-cycle = <72>; + gpmc,wr-access = <30>; + gpmc,wr-data-mux-bus = <0>; + + #address-cells = <1>; + #size-cells = <1>; +
Can you take the timings (for example) from arago tree. The timings is tested in am335x-evm So the timings can be directly used to populate GPMC timings. Timings can found at
http://arago-project.org/git/projects/?p=linux-am33x.git;a=commitdiff;
h=66bfbd2c5b35dc81edce0c24843c476161ab5978;hp=370630359cb8db711cf0941cd2a242e28ccfb61e
[...]
Quoted 1 line
+static int
gpmc_probe_dt(struct platform_device *pdev)
Can you take care of the following section mismatch.
WARNING: vmlinux.o(.text+0x1e2d0): Section mismatch in reference from the function gpmc_probe_dt() to the function .init.text:gpmc_nand_init().
[...]
arch/arm/mach-omap2/gpmc.c static int __devinit gpmc_mem_init(void) · line 865
Quoted 4 lines
arch/arm/mach-omap2/gpmc.c +4
-0+ + val =of_get_nand_ecc_mode (child); +if (val >=0 ) + gpmc_nand_data->ecc_opt = val;
This will fail for BCH. Index of "soft_bch" is 5 & also don't have selection option between for BCH4 & BCH8 also.
Can you use the of_property_read_u32 (as done early) to pass the ecc selection from dt file. This will help selection of BCH4 & BCH8 ecc options.
Thanks
Avinash
PATCH v3 4/4 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND
Quoted 44 lines · Avinash
Quoted 11 lines · Daniel Mack
This patch adds basic DT bindings for OMAP GPMC.
The actual peripherals are instanciated from child nodes within the GPMC node, and the only type of device that is currently supported is NAND.
Code was added to parse the generic GPMC timing parameters and some documentation with examples on how to use them.
Successfully tested on an AM33xx board.
Signed-off-by Daniel Mack <[email protected]>[...]
Quoted 14 lines
Documentation/devicetree/bindings/mtd/gpmc-nand.txt new file +14
-0+ + nand at 0,0 { + reg = <0 0 0>; /* CS0, offset 0 */ + nand-bus-width = <16>; + nand-ecc-mode = "none"; + + gpmc,sync-clk = <0>; + gpmc,cs-on = <0>; + gpmc,cs-rd-off = <36>; + gpmc,cs-wr-off = <36>; + gpmc,adv-on = <6>; + gpmc,adv-rd-off = <24>; + gpmc,adv-wr-off = <36>; + gpmc,we-off = <30>;Documentation/devicetree/bindings/mtd/gpmc-nand.txt line 56
Quoted 10 lines
Documentation/devicetree/bindings/mtd/gpmc-nand.txt new file +10
-0+ gpmc,oe-off = <48>; + gpmc,access = <54>; + gpmc,rd-cycle = <72>; + gpmc,wr-cycle = <72>; + gpmc,wr-access = <30>; + gpmc,wr-data-mux-bus = <0>; + + #address-cells = <1>; + #size-cells = <1>; +Can you take the timings (for example) from arago tree. The timings is tested in am335x-evm So the timings can be directly used to populate GPMC timings. Timings can found at
http://arago-project.org/git/projects/?p=linux-am33x.git;a=commitdiff;
h=66bfbd2c5b35dc81edce0c24843c476161ab5978;hp=370630359cb8db711cf0941cd2a242e28ccfb61e
Quoted 6 lines
[...]
Quoted 1 line
+static int
gpmc_probe_dt(struct platform_device *pdev)Can you take care of the following section mismatch.
WARNING:vmlinux.o(.text+0x1e2d0): Section mismatch in reference from the functiongpmc_probe_dt()to the function .init.text:gpmc_nand_init().
Sore, both fixed for v4.
Quoted 10 lines
[...]
arch/arm/mach-omap2/gpmc.c static int __devinit gpmc_mem_init(void) · line 865
Quoted 4 lines
arch/arm/mach-omap2/gpmc.c +4
-0+ + val =of_get_nand_ecc_mode (child); +if (val >=0 ) + gpmc_nand_data->ecc_opt = val;This will fail for BCH. Index of "
soft_bch" is 5 & also don't have selection option between for BCH4 & BCH8 also.
Can you use theof_property_read_u32(as done early) to pass the ecc selection from dt file. This will help selection of BCH4 & BCH8 ecc options.
Hmm. Shouldn't we rather teach of_get_nand_ecc_mode() that two modes and bring the enum in sync?
Daniel
PATCH v3 4/4 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND
Quoted 65 lines · Daniel Mack
Quoted 44 lines · Avinash
Quoted 11 lines · Daniel Mack
This patch adds basic DT bindings for OMAP GPMC.
The actual peripherals are instanciated from child nodes within the GPMC node, and the only type of device that is currently supported is NAND.
Code was added to parse the generic GPMC timing parameters and some documentation with examples on how to use them.
Successfully tested on an AM33xx board.
Signed-off-by Daniel Mack <[email protected]>[...]
Quoted 14 lines
Documentation/devicetree/bindings/mtd/gpmc-nand.txt new file +14
-0+ + nand at 0,0 { + reg = <0 0 0>; /* CS0, offset 0 */ + nand-bus-width = <16>; + nand-ecc-mode = "none"; + + gpmc,sync-clk = <0>; + gpmc,cs-on = <0>; + gpmc,cs-rd-off = <36>; + gpmc,cs-wr-off = <36>; + gpmc,adv-on = <6>; + gpmc,adv-rd-off = <24>; + gpmc,adv-wr-off = <36>; + gpmc,we-off = <30>;Documentation/devicetree/bindings/mtd/gpmc-nand.txt line 56
Quoted 10 lines
Documentation/devicetree/bindings/mtd/gpmc-nand.txt new file +10
-0+ gpmc,oe-off = <48>; + gpmc,access = <54>; + gpmc,rd-cycle = <72>; + gpmc,wr-cycle = <72>; + gpmc,wr-access = <30>; + gpmc,wr-data-mux-bus = <0>; + + #address-cells = <1>; + #size-cells = <1>; +Can you take the timings (for example) from arago tree. The timings is tested in am335x-evm So the timings can be directly used to populate GPMC timings. Timings can found at
http://arago-project.org/git/projects/?p=linux-am33x.git;a=commitdiff;
h=66bfbd2c5b35dc81edce0c24843c476161ab5978;hp=370630359cb8db711cf0941cd2a242e28ccfb61eQuoted 6 lines
[...]
Quoted 1 line
+static int
gpmc_probe_dt(struct platform_device *pdev)Can you take care of the following section mismatch.
WARNING:vmlinux.o(.text+0x1e2d0): Section mismatch in reference from the functiongpmc_probe_dt()to the function .init.text:gpmc_nand_init().Sore, both fixed for v4.
Quoted 10 lines
[...]
arch/arm/mach-omap2/gpmc.c static int __devinit gpmc_mem_init(void) · line 865
Quoted 4 lines
arch/arm/mach-omap2/gpmc.c +4
-0+ + val =of_get_nand_ecc_mode (child); +if (val >=0 ) + gpmc_nand_data->ecc_opt = val;This will fail for BCH. Index of "
soft_bch" is 5 & also don't have selection option between for BCH4 & BCH8 also.
Can you use theof_property_read_u32(as done early) to pass the ecc selection from dt file. This will help selection of BCH4 & BCH8 ecc options.
Quoted 2 lines
Hmm. Shouldn't we rather teach
of_get_nand_ecc_mode()that two modes and bring the enum in sync?
ecc_opt is for selecting different ecc layout and not for selecting ecc mode.
In omap2 driver NAND_ECC_HW ecc mode supports 3 ecc layout
OMAP_ECC_HAMMING_CODE_HW_ROMCODE OMAP_ECC_BCH4_CODE_HW OMAP_ECC_BCH8_CODE_HW
So selection of ecc layout data should come from DT not ecc mode.
Thanks
Avinash
Quoted 6 lines
Daniel
PATCH v3 4/4 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND
Quoted 81 lines · Avinash
Quoted 60 lines · Daniel Mack
Quoted 40 lines · Avinash
Quoted 11 lines · Daniel Mack
This patch adds basic DT bindings for OMAP GPMC.
The actual peripherals are instanciated from child nodes within the GPMC node, and the only type of device that is currently supported is NAND.
Code was added to parse the generic GPMC timing parameters and some documentation with examples on how to use them.
Successfully tested on an AM33xx board.
Signed-off-by Daniel Mack <[email protected]>[...]
Quoted 14 lines
Documentation/devicetree/bindings/mtd/gpmc-nand.txt new file +14
-0+ + nand at 0,0 { + reg = <0 0 0>; /* CS0, offset 0 */ + nand-bus-width = <16>; + nand-ecc-mode = "none"; + + gpmc,sync-clk = <0>; + gpmc,cs-on = <0>; + gpmc,cs-rd-off = <36>; + gpmc,cs-wr-off = <36>; + gpmc,adv-on = <6>; + gpmc,adv-rd-off = <24>; + gpmc,adv-wr-off = <36>; + gpmc,we-off = <30>;Documentation/devicetree/bindings/mtd/gpmc-nand.txt line 56
Quoted 10 lines
Documentation/devicetree/bindings/mtd/gpmc-nand.txt new file +10
-0+ gpmc,oe-off = <48>; + gpmc,access = <54>; + gpmc,rd-cycle = <72>; + gpmc,wr-cycle = <72>; + gpmc,wr-access = <30>; + gpmc,wr-data-mux-bus = <0>; + + #address-cells = <1>; + #size-cells = <1>; +Can you take the timings (for example) from arago tree. The timings is tested in am335x-evm So the timings can be directly used to populate GPMC timings. Timings can found at
Quoted 8 lines
http://arago-project.org/git/projects/?p=linux-am33x.git;a=commitdiff;
h=66bfbd2c5b35dc81edce0c24843c476161ab5978;hp=370630359cb8db711cf0941cd2a242e28ccfb61e[...]
Quoted 1 line
+static int
gpmc_probe_dt(struct platform_device *pdev)Can you take care of the following section mismatch.
WARNING:vmlinux.o(.text+0x1e2d0): Section mismatch in reference from the functiongpmc_probe_dt()to the function .init.text:gpmc_nand_init().Sore, both fixed for v4.
Quoted 9 lines
[...]
arch/arm/mach-omap2/gpmc.c static int __devinit gpmc_mem_init(void) · line 865
Quoted 4 lines
arch/arm/mach-omap2/gpmc.c +4
-0+ + val =of_get_nand_ecc_mode (child); +if (val >=0 ) + gpmc_nand_data->ecc_opt = val;This will fail for BCH. Index of "
soft_bch" is 5 & also don't have selection option between for BCH4 & BCH8 also.
Can you use theof_property_read_u32(as done early) to pass the ecc selection from dt file. This will help selection of BCH4 & BCH8 ecc options.Quoted 2 lines
Hmm. Shouldn't we rather teach
of_get_nand_ecc_mode()that two modes and bring the enum in sync?
ecc_optis for selecting different ecc layout and not for selecting ecc mode.In omap2 driver
NAND_ECC_HWecc mode supports 3 ecc layout
OMAP_ECC_HAMMING_CODE_HW_ROMCODEOMAP_ECC_BCH4_CODE_HWOMAP_ECC_BCH8_CODE_HWSo selection of ecc layout data should come from DT not ecc mode.
Thanks
AvinashQuoted 5 lines
Daniel
Quoted 5 lines
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi,
I am working on an AEMIF driver for DaVinci that is similar to this. Is this a memory or mfd device?
Murali
PATCH v3 4/4 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND
In reply to Philip, AvinashQuoted 69 lines · Avinash
Quoted 65 lines · Daniel Mack
Quoted 44 lines · Avinash
Quoted 11 lines · Daniel Mack
This patch adds basic DT bindings for OMAP GPMC.
The actual peripherals are instanciated from child nodes within the GPMC node, and the only type of device that is currently supported is NAND.
Code was added to parse the generic GPMC timing parameters and some documentation with examples on how to use them.
Successfully tested on an AM33xx board.
Signed-off-by Daniel Mack <[email protected]>[...]
Quoted 14 lines
Documentation/devicetree/bindings/mtd/gpmc-nand.txt new file +14
-0+ + nand at 0,0 { + reg = <0 0 0>; /* CS0, offset 0 */ + nand-bus-width = <16>; + nand-ecc-mode = "none"; + + gpmc,sync-clk = <0>; + gpmc,cs-on = <0>; + gpmc,cs-rd-off = <36>; + gpmc,cs-wr-off = <36>; + gpmc,adv-on = <6>; + gpmc,adv-rd-off = <24>; + gpmc,adv-wr-off = <36>; + gpmc,we-off = <30>;Documentation/devicetree/bindings/mtd/gpmc-nand.txt line 56
Quoted 10 lines
Documentation/devicetree/bindings/mtd/gpmc-nand.txt new file +10
-0+ gpmc,oe-off = <48>; + gpmc,access = <54>; + gpmc,rd-cycle = <72>; + gpmc,wr-cycle = <72>; + gpmc,wr-access = <30>; + gpmc,wr-data-mux-bus = <0>; + + #address-cells = <1>; + #size-cells = <1>; +Can you take the timings (for example) from arago tree. The timings is tested in am335x-evm So the timings can be directly used to populate GPMC timings. Timings can found at
http://arago-project.org/git/projects/?p=linux-am33x.git;a=commitdiff;
h=66bfbd2c5b35dc81edce0c24843c476161ab5978;hp=370630359cb8db711cf0941cd2a242e28ccfb61eQuoted 6 lines
[...]
Quoted 1 line
+static int
gpmc_probe_dt(struct platform_device *pdev)Can you take care of the following section mismatch.
WARNING:vmlinux.o(.text+0x1e2d0): Section mismatch in reference from the functiongpmc_probe_dt()to the function .init.text:gpmc_nand_init().Sore, both fixed for v4.
Quoted 10 lines
[...]
arch/arm/mach-omap2/gpmc.c static int __devinit gpmc_mem_init(void) · line 865
Quoted 4 lines
arch/arm/mach-omap2/gpmc.c +4
-0+ + val =of_get_nand_ecc_mode (child); +if (val >=0 ) + gpmc_nand_data->ecc_opt = val;This will fail for BCH. Index of "
soft_bch" is 5 & also don't have selection option between for BCH4 & BCH8 also.
Can you use theof_property_read_u32(as done early) to pass the ecc selection from dt file. This will help selection of BCH4 & BCH8 ecc options.Quoted 2 lines
Hmm. Shouldn't we rather teach
of_get_nand_ecc_mode()that two modes and bring the enum in sync?
Quoted 8 lines
ecc_optis for selecting different ecc layout and not for selecting ecc mode.In omap2 driver
NAND_ECC_HWecc mode supports 3 ecc layout
OMAP_ECC_HAMMING_CODE_HW_ROMCODEOMAP_ECC_BCH4_CODE_HWOMAP_ECC_BCH8_CODE_HWSo selection of ecc layout data should come from DT not ecc mode.
Ok, I see. I would still like to set them by string rather than magic numbers that map to enum entries. Valid values would be "none", "hw", "hw-romcode", "bch4" and "bch8". Are you ok with that?
Thanks,
Daniel
PATCH v3 4/4 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND
Quoted 77 lines · Daniel Mack
Quoted 76 lines · Avinash
Quoted 65 lines · Daniel Mack
Quoted 44 lines · Avinash
Quoted 11 lines · Daniel Mack
This patch adds basic DT bindings for OMAP GPMC.
The actual peripherals are instanciated from child nodes within the GPMC node, and the only type of device that is currently supported is NAND.
Code was added to parse the generic GPMC timing parameters and some documentation with examples on how to use them.
Successfully tested on an AM33xx board.
Signed-off-by Daniel Mack <[email protected]>[...]
Quoted 14 lines
Documentation/devicetree/bindings/mtd/gpmc-nand.txt new file +14
-0+ + nand at 0,0 { + reg = <0 0 0>; /* CS0, offset 0 */ + nand-bus-width = <16>; + nand-ecc-mode = "none"; + + gpmc,sync-clk = <0>; + gpmc,cs-on = <0>; + gpmc,cs-rd-off = <36>; + gpmc,cs-wr-off = <36>; + gpmc,adv-on = <6>; + gpmc,adv-rd-off = <24>; + gpmc,adv-wr-off = <36>; + gpmc,we-off = <30>;Documentation/devicetree/bindings/mtd/gpmc-nand.txt line 56
Quoted 10 lines
Documentation/devicetree/bindings/mtd/gpmc-nand.txt new file +10
-0+ gpmc,oe-off = <48>; + gpmc,access = <54>; + gpmc,rd-cycle = <72>; + gpmc,wr-cycle = <72>; + gpmc,wr-access = <30>; + gpmc,wr-data-mux-bus = <0>; + + #address-cells = <1>; + #size-cells = <1>; +Can you take the timings (for example) from arago tree. The timings is tested in am335x-evm So the timings can be directly used to populate GPMC timings. Timings can found at
http://arago-project.org/git/projects/?p=linux-am33x.git;a=commitdiff;
h=66bfbd2c5b35dc81edce0c24843c476161ab5978;hp=370630359cb8db711cf0941cd2a242e28ccfb61eQuoted 6 lines
[...]
Quoted 1 line
+static int
gpmc_probe_dt(struct platform_device *pdev)Can you take care of the following section mismatch.
WARNING:vmlinux.o(.text+0x1e2d0): Section mismatch in reference from the functiongpmc_probe_dt()to the function .init.text:gpmc_nand_init().Sore, both fixed for v4.
Quoted 10 lines
[...]
arch/arm/mach-omap2/gpmc.c static int __devinit gpmc_mem_init(void) · line 865
Quoted 4 lines
arch/arm/mach-omap2/gpmc.c +4
-0+ + val =of_get_nand_ecc_mode (child); +if (val >=0 ) + gpmc_nand_data->ecc_opt = val;This will fail for BCH. Index of "
soft_bch" is 5 & also don't have selection option between for BCH4 & BCH8 also.
Can you use theof_property_read_u32(as done early) to pass the ecc selection from dt file. This will help selection of BCH4 & BCH8 ecc options.Quoted 2 lines
Hmm. Shouldn't we rather teach
of_get_nand_ecc_mode()that two modes and bring the enum in sync?
ecc_optis for selecting different ecc layout and not for selecting ecc mode.In omap2 driver
NAND_ECC_HWecc mode supports 3 ecc layout
OMAP_ECC_HAMMING_CODE_HW_ROMCODEOMAP_ECC_BCH4_CODE_HWOMAP_ECC_BCH8_CODE_HW
Quoted 5 lines
Quoted 1 line
So selection of ecc layout data should come from DT not ecc mode.
Ok, I see. I would still like to set them by string rather than magic numbers that map to enum entries. Valid values would be "none", "hw", "hw-romcode", "bch4" and "bch8". Are you ok with that?
Ok, that's nice. Better use ecc_opt instead of ecc_mode.
Thanks
Avinash
Quoted 6 lines
Thanks,
Daniel
PATCH v3 4/4 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND
Quoted 78 lines · Avinash
Quoted 77 lines · Daniel Mack
Quoted 76 lines · Avinash
Quoted 65 lines · Daniel Mack
Quoted 44 lines · Avinash
Quoted 11 lines · Daniel Mack
This patch adds basic DT bindings for OMAP GPMC.
The actual peripherals are instanciated from child nodes within the GPMC node, and the only type of device that is currently supported is NAND.
Code was added to parse the generic GPMC timing parameters and some documentation with examples on how to use them.
Successfully tested on an AM33xx board.
Signed-off-by Daniel Mack <[email protected]>[...]
Quoted 14 lines
Documentation/devicetree/bindings/mtd/gpmc-nand.txt new file +14
-0+ + nand at 0,0 { + reg = <0 0 0>; /* CS0, offset 0 */ + nand-bus-width = <16>; + nand-ecc-mode = "none"; + + gpmc,sync-clk = <0>; + gpmc,cs-on = <0>; + gpmc,cs-rd-off = <36>; + gpmc,cs-wr-off = <36>; + gpmc,adv-on = <6>; + gpmc,adv-rd-off = <24>; + gpmc,adv-wr-off = <36>; + gpmc,we-off = <30>;Documentation/devicetree/bindings/mtd/gpmc-nand.txt line 56
Quoted 10 lines
Documentation/devicetree/bindings/mtd/gpmc-nand.txt new file +10
-0+ gpmc,oe-off = <48>; + gpmc,access = <54>; + gpmc,rd-cycle = <72>; + gpmc,wr-cycle = <72>; + gpmc,wr-access = <30>; + gpmc,wr-data-mux-bus = <0>; + + #address-cells = <1>; + #size-cells = <1>; +Can you take the timings (for example) from arago tree. The timings is tested in am335x-evm So the timings can be directly used to populate GPMC timings. Timings can found at
http://arago-project.org/git/projects/?p=linux-am33x.git;a=commitdiff;
h=66bfbd2c5b35dc81edce0c24843c476161ab5978;hp=370630359cb8db711cf0941cd2a242e28ccfb61eQuoted 6 lines
[...]
Quoted 1 line
+static int
gpmc_probe_dt(struct platform_device *pdev)Can you take care of the following section mismatch.
WARNING:vmlinux.o(.text+0x1e2d0): Section mismatch in reference from the functiongpmc_probe_dt()to the function .init.text:gpmc_nand_init().Sore, both fixed for v4.
Quoted 10 lines
[...]
arch/arm/mach-omap2/gpmc.c static int __devinit gpmc_mem_init(void) · line 865
Quoted 4 lines
arch/arm/mach-omap2/gpmc.c +4
-0+ + val =of_get_nand_ecc_mode (child); +if (val >=0 ) + gpmc_nand_data->ecc_opt = val;This will fail for BCH. Index of "
soft_bch" is 5 & also don't have selection option between for BCH4 & BCH8 also.
Can you use theof_property_read_u32(as done early) to pass the ecc selection from dt file. This will help selection of BCH4 & BCH8 ecc options.Quoted 2 lines
Hmm. Shouldn't we rather teach
of_get_nand_ecc_mode()that two modes and bring the enum in sync?
ecc_optis for selecting different ecc layout and not for selecting ecc mode.In omap2 driver
NAND_ECC_HWecc mode supports 3 ecc layout
OMAP_ECC_HAMMING_CODE_HW_ROMCODEOMAP_ECC_BCH4_CODE_HWOMAP_ECC_BCH8_CODE_HW
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
So selection of ecc layout data should come from DT not ecc mode.
Ok, I see. I would still like to set them by string rather than magic numbers that map to enum entries. Valid values would be "none", "hw", "hw-romcode", "bch4" and "bch8". Are you ok with that?
Ok, that's nice. Better use
ecc_optinstead ofecc_mode.
I did some more extensive tests that include reading the same nand pages from both U-Boot and the kernel with BCH8 ECC, and it turns out that ->is_elm_used needs to be set in the pdata in order to make this work.
So the question is whether we actually want to have a DT property for that or just always enable that bit in case a hardware supported ecc mode is selected. Any opinion on this?
That's the last topic before I'm clear to send off v4.
Thanks,
Daniel
PATCH v3 4/4 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND
Quoted 91 lines · Daniel Mack
Quoted 78 lines · Avinash
Quoted 77 lines · Daniel Mack
Quoted 76 lines · Avinash
Quoted 65 lines · Daniel Mack
Quoted 44 lines · Avinash
Quoted 11 lines · Daniel Mack
This patch adds basic DT bindings for OMAP GPMC.
The actual peripherals are instanciated from child nodes within the GPMC node, and the only type of device that is currently supported is NAND.
Code was added to parse the generic GPMC timing parameters and some documentation with examples on how to use them.
Successfully tested on an AM33xx board.
Signed-off-by Daniel Mack <[email protected]>[...]
Quoted 14 lines
(quoted diff) +14
-0+ + nand at 0,0 { + reg = <0 0 0>; /* CS0, offset 0 */ + nand-bus-width = <16>; + nand-ecc-mode = "none"; + + gpmc,sync-clk = <0>; + gpmc,cs-on = <0>; + gpmc,cs-rd-off = <36>; + gpmc,cs-wr-off = <36>; + gpmc,adv-on = <6>; + gpmc,adv-rd-off = <24>; + gpmc,adv-wr-off = <36>; + gpmc,we-off = <30>;Review
Quoted 10 lines
(quoted diff) +10
-0+ gpmc,oe-off = <48>; + gpmc,access = <54>; + gpmc,rd-cycle = <72>; + gpmc,wr-cycle = <72>; + gpmc,wr-access = <30>; + gpmc,wr-data-mux-bus = <0>; + + #address-cells = <1>; + #size-cells = <1>; +Can you take the timings (for example) from arago tree. The timings is tested in am335x-evm So the timings can be directly used to populate GPMC timings. Timings can found at
http://arago-project.org/git/projects/?p=linux-am33x.git;a=commitdiff;
h=66bfbd2c5b35dc81edce0c24843c476161ab5978;hp=370630359cb8db711cf0941cd2a242e28ccfb61eQuoted 6 lines
[...]
Quoted 1 line
+static int
gpmc_probe_dt(struct platform_device *pdev)Can you take care of the following section mismatch.
WARNING:vmlinux.o(.text+0x1e2d0): Section mismatch in reference from the functiongpmc_probe_dt()to the function .init.text:gpmc_nand_init().Sore, both fixed for v4.
Quoted 10 lines
[...]
Review
Quoted 4 lines
(quoted diff) +4
-0+ + val = of_get_nand_ecc_mode(child); + if (val >= 0) + gpmc_nand_data->ecc_opt = val;This will fail for BCH. Index of "
soft_bch" is 5 & also don't have selection option between for BCH4 & BCH8 also.
Can you use theof_property_read_u32(as done early) to pass the ecc selection from dt file. This will help selection of BCH4 & BCH8 ecc options.Quoted 2 lines
Hmm. Shouldn't we rather teach
of_get_nand_ecc_mode()that two modes and bring the enum in sync?
ecc_optis for selecting different ecc layout and not for selecting ecc mode.In omap2 driver
NAND_ECC_HWecc mode supports 3 ecc layout
OMAP_ECC_HAMMING_CODE_HW_ROMCODEOMAP_ECC_BCH4_CODE_HWOMAP_ECC_BCH8_CODE_HWQuoted 7 lines
Quoted 5 lines
Quoted 1 line
So selection of ecc layout data should come from DT not ecc mode.
Ok, I see. I would still like to set them by string rather than magic numbers that map to enum entries. Valid values would be "none", "hw", "hw-romcode", "bch4" and "bch8". Are you ok with that?
Ok, that's nice. Better use
ecc_optinstead ofecc_mode.I did some more extensive tests that include reading the same nand pages from both U-Boot and the kernel with BCH8 ECC, and it turns out that ->is_elm_used needs to be set in the pdata in order to make this work.
Quoted 5 lines
So the question is whether we actually want to have a DT property for that or just always enable that bit in case a hardware supported ecc mode is selected. Any opinion on this?
That's the last topic before I'm clear to send off v4.
Any opionion on this, anyone?
Thanks,
Daniel
PATCH v3 4/4 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND
In reply to Daniel MackQuoted 87 lines · Daniel Mack
Quoted 78 lines · Avinash
Quoted 77 lines · Daniel Mack
Quoted 76 lines · Avinash
Quoted 65 lines · Daniel Mack
Quoted 44 lines · Avinash
Quoted 11 lines · Daniel Mack
This patch adds basic DT bindings for OMAP GPMC.
The actual peripherals are instanciated from child nodes within the GPMC node, and the only type of device that is currently supported is NAND.
Code was added to parse the generic GPMC timing parameters and some documentation with examples on how to use them.
Successfully tested on an AM33xx board.
Signed-off-by Daniel Mack <[email protected]>[...]
Quoted 14 lines
(quoted diff) +14
-0+ + nand at 0,0 { + reg = <0 0 0>; /* CS0, offset 0 */ + nand-bus-width = <16>; + nand-ecc-mode = "none"; + + gpmc,sync-clk = <0>; + gpmc,cs-on = <0>; + gpmc,cs-rd-off = <36>; + gpmc,cs-wr-off = <36>; + gpmc,adv-on = <6>; + gpmc,adv-rd-off = <24>; + gpmc,adv-wr-off = <36>; + gpmc,we-off = <30>;Review
Quoted 10 lines
(quoted diff) +10
-0+ gpmc,oe-off = <48>; + gpmc,access = <54>; + gpmc,rd-cycle = <72>; + gpmc,wr-cycle = <72>; + gpmc,wr-access = <30>; + gpmc,wr-data-mux-bus = <0>; + + #address-cells = <1>; + #size-cells = <1>; +Can you take the timings (for example) from arago tree. The timings is tested in am335x-evm So the timings can be directly used to populate GPMC timings. Timings can found at
http://arago-project.org/git/projects/?p=linux-am33x.git;a=commitdiff;
h=66bfbd2c5b35dc81edce0c24843c476161ab5978;hp=370630359cb8db711cf0941cd2a242e28ccfb61eQuoted 6 lines
[...]
Quoted 1 line
+static int
gpmc_probe_dt(struct platform_device *pdev)Can you take care of the following section mismatch.
WARNING:vmlinux.o(.text+0x1e2d0): Section mismatch in reference from the functiongpmc_probe_dt()to the function .init.text:gpmc_nand_init().Sore, both fixed for v4.
Quoted 10 lines
[...]
Review
Quoted 4 lines
(quoted diff) +4
-0+ + val = of_get_nand_ecc_mode(child); + if (val >= 0) + gpmc_nand_data->ecc_opt = val;This will fail for BCH. Index of "
soft_bch" is 5 & also don't have selection option between for BCH4 & BCH8 also.
Can you use theof_property_read_u32(as done early) to pass the ecc selection from dt file. This will help selection of BCH4 & BCH8 ecc options.Quoted 2 lines
Hmm. Shouldn't we rather teach
of_get_nand_ecc_mode()that two modes and bring the enum in sync?
ecc_optis for selecting different ecc layout and not for selecting ecc mode.In omap2 driver
NAND_ECC_HWecc mode supports 3 ecc layout
OMAP_ECC_HAMMING_CODE_HW_ROMCODEOMAP_ECC_BCH4_CODE_HWOMAP_ECC_BCH8_CODE_HWQuoted 7 lines
Quoted 5 lines
Quoted 1 line
So selection of ecc layout data should come from DT not ecc mode.
Ok, I see. I would still like to set them by string rather than magic numbers that map to enum entries. Valid values would be "none", "hw", "hw-romcode", "bch4" and "bch8". Are you ok with that?
Ok, that's nice. Better use
ecc_optinstead ofecc_mode.
Quoted 7 lines
I did some more extensive tests that include reading the same nand pages from both U-Boot and the kernel with BCH8 ECC, and it turns out that ->is_elm_used needs to be set in the pdata in order to make this work.
So the question is whether we actually want to have a DT property for that or just always enable that bit in case a hardware supported ecc mode is selected. Any opinion on this?
Yes, is_elm_used data should come from DT. There may be hardware which uses BCH8 ecc scheme even without ELM hardware support with software error correction support. So is_elm_used data should come from DT property.
Thanks
Avinash
Quoted 8 lines
That's the last topic before I'm clear to send off v4.
Thanks,
Daniel
PATCH v3 4/4 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND
In reply to Daniel MackQuoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Daniel" == Daniel Mack <[email protected]> writes:
Hi,
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
In omap2 driver
NAND_ECC_HWecc mode supports 3 ecc layoutOMAP_ECC_HAMMING_CODE_HW_ROMCODEOMAP_ECC_BCH4_CODE_HWOMAP_ECC_BCH8_CODE_HW
Quoted 7 lines
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
So selection of ecc layout data should come from DT not ecc mode.
Ok, I see. I would still like to set them by string rather than magic numbers that map to enum entries. Valid values would be "none", "hw", "hw-romcode", "bch4" and "bch8". Are you ok with that?
Ok, that's nice. Better use
ecc_optinstead ofecc_mode.
Daniel> I did some more extensive tests that include reading the same Daniel> nand pages from both U-Boot and the kernel with BCH8 ECC, and Daniel> it turns out that -> is_elm_used needs to be set in the pdata Daniel> in order to make this work.
So what you're saying is that the choice of ELM or not is not just an optimization, it really changes the ECC layout? Perhaps it should be treated as a seperate layout (E.G. bch8-elm) then?
Signature · 1 line
Bye, Peter Korsgaard
PATCH v3 4/4 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND
Quoted 17 lines · Peter Korsgaard
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Daniel" == Daniel Mack <[email protected]> writes:
Hi,
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
In omap2 driver
NAND_ECC_HWecc mode supports 3 ecc layoutOMAP_ECC_HAMMING_CODE_HW_ROMCODEOMAP_ECC_BCH4_CODE_HWOMAP_ECC_BCH8_CODE_HWQuoted 7 lines
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
So selection of ecc layout data should come from DT not ecc mode.
Ok, I see. I would still like to set them by string rather than magic numbers that map to enum entries. Valid values would be "none", "hw", "hw-romcode", "bch4" and "bch8". Are you ok with that?
Ok, that's nice. Better use
ecc_optinstead ofecc_mode.
Quoted 8 lines
Daniel> I did some more extensive tests that include reading the same Daniel> nand pages from both U-Boot and the kernel with BCH8 ECC, and Daniel> it turns out that ->
is_elm_usedneeds to be set in the pdata Daniel> in order to make this work.So what you're saying is that the choice of ELM or not is not just an optimization, it really changes the ECC layout? Perhaps it should be treated as a seperate layout (E.G. bch8-elm) then?
That is what I experienced, yes. The kernel was unable to parse NAND pages that were written from U-Boot with bch8 hardware mode when the elm module was not active. Maybe someone from TI can explain that? Giving it a dedicated name would also solve the problem with the extra DT property.
I'll wait until this is decided before I resend a new set that also fixes the issue with the errornousely forgotten Documentation file.
Thanks,
Daniel
PATCH v3 4/4 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND
Quoted 18 lines · Daniel Mack
Quoted 17 lines · Peter Korsgaard
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Daniel" == Daniel Mack <[email protected]> writes:
Hi,
Quoted 12 lines
Quoted 12 lines
Quoted 10 lines
Quoted 6 lines
In omap2 driver
NAND_ECC_HWecc mode supports 3 ecc layoutOMAP_ECC_HAMMING_CODE_HW_ROMCODEOMAP_ECC_BCH4_CODE_HWOMAP_ECC_BCH8_CODE_HWSo selection of ecc layout data should come from DT not ecc mode.
Ok, I see. I would still like to set them by string rather than magic numbers that map to enum entries. Valid values would be "none", "hw", "hw-romcode", "bch4" and "bch8". Are you ok with that?
Ok, that's nice. Better use
ecc_optinstead ofecc_mode.
Quoted 8 lines
Quoted 8 lines
Daniel> I did some more extensive tests that include reading the same Daniel> nand pages from both U-Boot and the kernel with BCH8 ECC, and Daniel> it turns out that ->
is_elm_usedneeds to be set in the pdata Daniel> in order to make this work.So what you're saying is that the choice of ELM or not is not just an optimization, it really changes the ECC layout? Perhaps it should be treated as a seperate layout (E.G. bch8-elm) then?
Peter,
In patch [1], mtd: nand: omap2: Support for hardware BCH
ecc_layout made compatible with Rom Boot Loader ECC layout for am335x.
This action is based on is_elm_used flag.
Requirement of this flag is to identify the whether the platform ELM module & based on this configure ELM module if present.
But ideally BCH8 ecc lay out didn't have a dependency on ELM, it can work with software BCH ecc. RBL compatibility is missing in software BCH because of addition of constant polynomial to ecc vector. If we remove the dependency on erased page handling by ecc vector with constant polynomial, software BCH can do the job of RBL compatibility.
Ivan,
Do you have any suggestions?
Discussion for RBL compatibility found at [2].
It is good that software BCH also support RBL compatibility by suppressing constant polynomial modification. Then ecc layout can be selected from DT entry and error correction can be chosen between software/hardware depending on the availability of ELM hardware.
Currently RBL BCH8 compatibility depends on the availability of ELM hardware. Later once software BCH start supporting RBL compatibility, we can remove the check.
Quoted 5 lines
That is what I experienced, yes. The kernel was unable to parse NAND pages that were written from U-Boot with bch8 hardware mode when the elm module was not active. Maybe someone from TI can explain that? Giving it a dedicated name would also solve the problem with the extra DT property.
Daniel,
Currently BCH8 is supported with software ecc error correction in mainline. The layout for BCH8 ECC layout is
0-1 -> BAD block markers
2-11-> oob free area
12-63-> BCH8 ECC.
RBL ECC layout is
0-1 -> BAD block markers
2-57-> BCH8 ecc layout
58-63-> OOB free area
As u-boot also maintain RBL ecc layout, write from U-boot and read from Linux requires compatibility with RBL ecc layout. The same is achieved in the patch [1], with by setting is_elm_used to true.
1. https://lkml.org/lkml/2012/10/31/87
2. https://lkml.org/lkml/2012/10/11/20
Thanks
Avinash
Quoted 9 lines
I'll wait until this is decided before I resend a new set that also fixes the issue with the errornousely forgotten Documentation file.
Thanks,
Daniel
PATCH v3 4/4 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND
Hi Avinash,
Hi Peter,
Quoted 53 lines · Avinash
Quoted 2 lines · Daniel Mack
Quoted 1 line · Peter Korsgaard
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Daniel" == Daniel Mack <[email protected]> writes:
Peter,
In patch [1], mtd: nand: omap2: Support for hardware BCH
ecc_layoutmade compatible with Rom Boot Loader ECC layout for am335x.This action is based on
is_elm_usedflag.Requirement of this flag is to identify the whether the platform ELM module & based on this configure ELM module if present.
But ideally BCH8 ecc lay out didn't have a dependency on ELM, it can work with software BCH ecc. RBL compatibility is missing in software BCH because of addition of constant polynomial to ecc vector. If we remove the dependency on erased page handling by ecc vector with constant polynomial, software BCH can do the job of RBL compatibility.
Ivan,
Do you have any suggestions?
Discussion for RBL compatibility found at [2].It is good that software BCH also support RBL compatibility by suppressing constant polynomial modification. Then ecc layout can be selected from DT entry and error correction can be chosen between software/hardware depending on the availability of ELM hardware.
Currently RBL BCH8 compatibility depends on the availability of ELM hardware. Later once software BCH start supporting RBL compatibility, we can remove the check.Quoted 5 lines
That is what I experienced, yes. The kernel was unable to parse NAND pages that were written from U-Boot with bch8 hardware mode when the elm module was not active. Maybe someone from TI can explain that? Giving it a dedicated name would also solve the problem with the extra DT property.
Daniel,
Currently BCH8 is supported with software ecc error correction in mainline. The layout for BCH8 ECC layout is
0-1 -> BAD block markers
2-11-> oob free area
12-63-> BCH8 ECC.RBL ECC layout is
0-1 -> BAD block markers
2-57-> BCH8 ecc layout
58-63-> OOB free area
Quoted 7 lines
As u-boot also maintain RBL ecc layout, write from U-boot and read from Linux requires compatibility with RBL ecc layout. The same is achieved in the patch [1], with by setting
is_elm_usedto true.1. https://lkml.org/lkml/2012/10/31/87
2. https://lkml.org/lkml/2012/10/11/20
So, after reading this, I'm still uncertain what's your preferred way of handling the bindings here. Are you saying we should stick with the is_elm_used flag, and subsequently care for BCH8 software mode compatibility?
I would like to submit a new version soon, so it can be queued up for the next merge window, and that decision is the last blocker currently for sending out a new series.
Many thanks,
Daniel
PATCH v3 4/4 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND
Quoted 62 lines · Daniel Mack
Hi Avinash,
Hi Peter,Quoted 58 lines · Avinash
Quoted 2 lines · Daniel Mack
Quoted 1 line · Peter Korsgaard
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Daniel" == Daniel Mack <[email protected]> writes:
Peter,
In patch [1], mtd: nand: omap2: Support for hardware BCH
ecc_layoutmade compatible with Rom Boot Loader ECC layout for am335x.This action is based on
is_elm_usedflag.Requirement of this flag is to identify the whether the platform ELM module & based on this configure ELM module if present.
But ideally BCH8 ecc lay out didn't have a dependency on ELM, it can work with software BCH ecc. RBL compatibility is missing in software BCH because of addition of constant polynomial to ecc vector. If we remove the dependency on erased page handling by ecc vector with constant polynomial, software BCH can do the job of RBL compatibility.
Ivan,
Do you have any suggestions?
Discussion for RBL compatibility found at [2].It is good that software BCH also support RBL compatibility by suppressing constant polynomial modification. Then ecc layout can be selected from DT entry and error correction can be chosen between software/hardware depending on the availability of ELM hardware.
Currently RBL BCH8 compatibility depends on the availability of ELM hardware. Later once software BCH start supporting RBL compatibility, we can remove the check.Quoted 5 lines
That is what I experienced, yes. The kernel was unable to parse NAND pages that were written from U-Boot with bch8 hardware mode when the elm module was not active. Maybe someone from TI can explain that? Giving it a dedicated name would also solve the problem with the extra DT property.
Daniel,
Currently BCH8 is supported with software ecc error correction in mainline. The layout for BCH8 ECC layout is
0-1 -> BAD block markers
2-11-> oob free area
12-63-> BCH8 ECC.RBL ECC layout is
0-1 -> BAD block markers
2-57-> BCH8 ecc layout
58-63-> OOB free areaAs u-boot also maintain RBL ecc layout, write from U-boot and read from Linux requires compatibility with RBL ecc layout. The same is achieved in the patch [1], with by setting
is_elm_usedto true.
Quoted 7 lines
Quoted 2 lines
1. https://lkml.org/lkml/2012/10/31/87
2. https://lkml.org/lkml/2012/10/11/20So, after reading this, I'm still uncertain what's your preferred way of handling the bindings here. Are you saying we should stick with the
is_elm_usedflag, and subsequently care for BCH8 software mode compatibility?
Ideally RBL compatibility didn't depend on the availability of ELM module. So from am335x perspective, RBL compatibility is achieved with ECC error correction with ELM module. I would submit one more revision of BCH8 ELM error correction support which would check for bch8-am335xrbl-compatible.
Note: RBL ecc layout can vary from SOC to SOC
Daniel,
So can you start supporting "bch8-am335xrbl-compatible" and remove usage of is_elm_used in dt. This should come in ecc_opt field.
In omap2 NAND driver, AM335x RBL compatibility is achieved depending on ecc_layout and runtime detection of elm module. So options related to can be
1. bch8-am335xrbl-compatible is enabled and run time detection of
Of elm module passed, RBL compatibility achieved.
2. bch8-am335xrbl-compatible is enabled and run time detection of
of elm module failed, RBL compatibility sacrificed but continue with software BCH8 error correction. Sacrificing RBL compatibility because of constant polynomial addition and usage of 14 byte instead of 13 byte.
Ivan,
Do you have any plan of removing addition of constant polynomial to ecc data and go for extra byte checking to find erased pages?
This way we can start support BCH8 with RBL compatible and kernel Didn't put any restriction of the ecc layout.
Thanks
Avinash
Quoted 10 lines
I would like to submit a new version soon, so it can be queued up for the next merge window, and that decision is the last blocker currently for sending out a new series.
Many thanks,
Daniel
PATCH v3 4/4 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND
On Wed, Nov 28, 2012 at 05:01:30AM +0000, Philip, Avinash wrote:
(...)
Quoted 15 lines
Daniel,
So can you start supporting "bch8-am335xrbl-compatible" and remove usage of
is_elm_usedin dt. This should come inecc_optfield.In omap2 NAND driver, AM335x RBL compatibility is achieved depending on
ecc_layoutand runtime detection of elm module. So options related to can be
1. bch8-am335xrbl-compatible is enabled and run time detection of
Of elm module passed, RBL compatibility achieved.
2. bch8-am335xrbl-compatible is enabled and run time detection of
of elm module failed, RBL compatibility sacrificed but continue with software BCH8 error correction. Sacrificing RBL compatibility because of constant polynomial addition and usage of 14 byte instead of 13 byte.
Quoted 5 lines
Ivan,
Do you have any plan of removing addition of constant polynomial to ecc data and go for extra byte checking to find erased pages?
This way we can start support BCH8 with RBL compatible and kernel Didn't put any restriction of the ecc layout.
Hello Avinash,
Sorry about the response delay.
First a short reminder of pros and cons of the "constant polynomial addition" (let's just call it "CPA") feature:
pros:
- it elegantly solves all problems related to reading an erased page (no clumsy hack needed)
- better performance: when a bitflip appears on an erased page (often this is a "sticky" bitflip), there is no need to perform a full scan+cleanup of the page each time it is read
cons:
- the ecc vector is not compatible with RBL
RBL compatibility is not necessary in my case, because I'm using the OMAP MLC ROM boot mode. Rather than completely removing the CPA feature, I'd like to keep it as an option; it could even be used with the ELM module.
I'm OK to submit a patch in this direction, but first I'd like to wait for the dust to settle on arch/arm/mach-omap2 and mtd/nand/omap2.c with Afzal patches and everything; things have become a bit complicated to follow recently :-)
Also, I think it would be nice to move BCH code out of omap2.c into a separate file. What do you think ?
BR,
--
Ivan
PATCH v3 4/4 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND
Quoted 25 lines · Ivan Djelic
On Wed, Nov 28, 2012 at 05:01:30AM +0000, Philip, Avinash wrote:
(...)Quoted 15 lines
Daniel,
So can you start supporting "bch8-am335xrbl-compatible" and remove usage of
is_elm_usedin dt. This should come inecc_optfield.In omap2 NAND driver, AM335x RBL compatibility is achieved depending on
ecc_layoutand runtime detection of elm module. So options related to can be
1. bch8-am335xrbl-compatible is enabled and run time detection of
Of elm module passed, RBL compatibility achieved.
2. bch8-am335xrbl-compatible is enabled and run time detection of
of elm module failed, RBL compatibility sacrificed but continue with software BCH8 error correction. Sacrificing RBL compatibility because of constant polynomial addition and usage of 14 byte instead of 13 byte.Quoted 5 lines
Ivan,
Do you have any plan of removing addition of constant polynomial to ecc data and go for extra byte checking to find erased pages?
This way we can start support BCH8 with RBL compatible and kernel Didn't put any restriction of the ecc layout.Hello Avinash,
Quoted 8 lines
Sorry about the response delay.
First a short reminder of pros and cons of the "constant polynomial addition" (let's just call it "CPA") feature:pros:
- it elegantly solves all problems related to reading an erased page (no clumsy hack needed)
- better performance: when a bitflip appears on an erased page (often this is a "sticky" bitflip), there is no need to perform a full scan+cleanup of the page each time it is read
No need for scan + cleanup on each read, as the chances of encountering bit flips in erased page is less. Also to find bit flips in erased page, compare ecc vector for read erased page against a standard ecc vector. Presence of bit flips can find by checking the compare results. In case of mismatch, should go for correction of bit flips in erased page with full scan.
So with this approach, we can nullify the effect of CPA for erased page bit flip handling.
Quoted 7 lines
cons:
- the ecc vector is not compatible with RBLRBL compatibility is not necessary in my case, because I'm using the OMAP MLC ROM boot mode. Rather than completely removing the CPA feature, I'd like to keep it as an option; it could even be used with the ELM module.
I agree RBL compatibility depends on the user. But with RBL compatibility, there is no sacrifice of any existing feature. Is it right?
Also nand driver get simplified with removal of CPA, so that both HW & SW error correction can go for same ecc calculation.
Quoted 4 lines
I'm OK to submit a patch in this direction, but first I'd like to wait for the dust to settle on
arch/arm/mach-omap2andmtd/nand/omap2.cwith Afzal patches and everything; things have become a bit complicated to follow recently :-)
Afzal's changes are in & are settled now.
Quoted 2 lines
Also, I think it would be nice to move BCH code out of
omap2.cinto a separate file. What do you think ?
With BCH code you mean, omap3_correct_data_bch()?
I think this can be part of omap2 driver it self as it is ecc correctable method for nand driver.
Thanks
Avinash
Quoted 5 lines
BR,
--
Ivan
PATCH v3 4/4 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND
On Wed, Dec 05, 2012 at 05:15:52AM +0000, Philip, Avinash wrote:
(...)
Quoted 8 lines
Quoted 7 lines
First a short reminder of pros and cons of the "constant polynomial addition" (let's just call it "CPA") feature:
pros:
- it elegantly solves all problems related to reading an erased page (no clumsy hack needed)
- better performance: when a bitflip appears on an erased page (often this is a "sticky" bitflip), there is no need to perform a full scan+cleanup of the page each time it is read
Quoted 4 lines
No need for scan + cleanup on each read, as the chances of encountering bit flips in erased page is less. Also to find bit flips in erased page, compare ecc vector for read erased page against a standard ecc vector. Presence of bit flips can find by checking the compare results. In case of mismatch, should go for correction of bit flips in erased page with full scan.
Hi Avinash,
I explicitly mentioned the condition "when a bitflip appears on an erased page", in which case you _do_ have to do a full scan upon each read, until you erase the block; and then, the bitflip may still be there (this is what I called a "sticky" bitflip).
My experience with recent devices (4-bit/8-bit) is that erased pages with a single bitflip are not uncommon. For those pages, there is undeniably a performance penalty compared to CPA.
Benchmarks would be needed to quantify the overall performance impact, but I suspect it is small.
Quoted 2 lines
So with this approach, we can nullify the effect of CPA for erased page bit flip handling.
Well, not completely. I would happily drop CPA is that were the case.
Quoted 8 lines
Quoted 7 lines
cons:
- the ecc vector is not compatible with RBLRBL compatibility is not necessary in my case, because I'm using the OMAP MLC ROM boot mode. Rather than completely removing the CPA feature, I'd like to keep it as an option; it could even be used with the ELM module.
Quoted 5 lines
I agree RBL compatibility depends on the user. But with RBL compatibility, there is no sacrifice of any existing feature. Is it right?
Also nand driver get simplified with removal of CPA, so that both HW & SW error correction can go for same ecc calculation.
Indeed that would be a simplification.
Quoted 6 lines
Quoted 4 lines
I'm OK to submit a patch in this direction, but first I'd like to wait for the dust to settle on
arch/arm/mach-omap2andmtd/nand/omap2.cwith Afzal patches and everything; things have become a bit complicated to follow recently :-)Afzal's changes are in & are settled now.
What about this set: http://lists.infradead.org/pipermail/linux-mtd/2012-October/044337.html
I cannot see it in l2-mtd-2.6 ? Or did I miss something ?
Thanks,
--
Ivan
PATCH v3 4/4 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND
Quoted 15 lines · Ivan Djelic
On Wed, Dec 05, 2012 at 05:15:52AM +0000, Philip, Avinash wrote:
(...)Quoted 8 lines
Quoted 7 lines
First a short reminder of pros and cons of the "constant polynomial addition" (let's just call it "CPA") feature:
pros:
- it elegantly solves all problems related to reading an erased page (no clumsy hack needed)
- better performance: when a bitflip appears on an erased page (often this is a "sticky" bitflip), there is no need to perform a full scan+cleanup of the page each time it is readQuoted 4 lines
No need for scan + cleanup on each read, as the chances of encountering bit flips in erased page is less. Also to find bit flips in erased page, compare ecc vector for read erased page against a standard ecc vector. Presence of bit flips can find by checking the compare results. In case of mismatch, should go for correction of bit flips in erased page with full scan.
Quoted 5 lines
Hi Avinash,
I explicitly mentioned the condition "when a bitflip appears on an erased page", in which case you _do_ have to do a full scan upon each read, until you erase the block; and then, the bitflip may still be there (this is what I called a "sticky" bitflip).
Bit flips in erased page require full scan.
Quoted 2 lines
My experience with recent devices (4-bit/8-bit) is that erased pages with a single bitflip are not uncommon.
What about erased page without bit flips? If we take the probability of bit flips and non-bit flips in erased page, erased pages with bit flips will be very less.
So probability of scanning erased page is less.
Quoted 1 line
For those pages, there is undeniably a performance penalty compared to CPA.
Pages with bit flips would have better performance with CPA as no explicit scan present. Pages without bit flips, would have better performance if we compare against standard ecc vector than with present implementation of software BCH.
The same can be achieved with software BCH also.
Quoted 1 line
Benchmarks would be needed to quantify the overall performance impact, but I suspect it is small.
So overall performance would be better with checking with standard ecc vector for finding erased page and handling bit flip in erased page with full scan due to probability that bit flips in erased page is very less than pages without bit flips.
But with performance improvement for bit flipped erased page with CPA, we have to sacrifice RBL compatibility. So a common layout across all components would be missing.
Quoted 5 lines
Quoted 3 lines
So with this approach, we can nullify the effect of CPA for erased page bit flip handling.
Well, not completely. I would happily drop CPA is that were the case.
So the choices can be
1. Performance drop in case of bit flip in erased page with full scan of page if bit flip in erased page. But can achieve RBL compatibility, simplification of ecc calculation.
2. Sacrifice RBL compatibility with CPA, but still performance can be improved for erased pages without bit flips.
But initial discussion in this direction told RBL compatibility would be a better option
https://lkml.org/lkml/2012/10/11/240
Quoted 18 lines
Quoted 9 lines
Quoted 8 lines
cons:
- the ecc vector is not compatible with RBLRBL compatibility is not necessary in my case, because I'm using the OMAP MLC ROM boot mode. Rather than completely removing the CPA feature, I'd like to keep it as an option; it could even be used with the ELM module.
Quoted 5 lines
I agree RBL compatibility depends on the user. But with RBL compatibility, there is no sacrifice of any existing feature. Is it right?
Also nand driver get simplified with removal of CPA, so that both HW & SW error correction can go for same ecc calculation.
Indeed that would be a simplification.
Quoted 1 line
Quoted 1 line
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
I'm OK to submit a patch in this direction, but first I'd like to wait for the dust to settle on
arch/arm/mach-omap2andmtd/nand/omap2.cwith Afzal patches and everything; things have become a bit complicated to follow recently :-)Afzal's changes are in & are settled now.
What about this set: http://lists.infradead.org/pipermail/linux-mtd/2012-October/044337.html
I cannot see it in l2-mtd-2.6 ? Or did I miss something ?
Yes, Afzal's changes are present in linux-next. I think it will get in next merge window.
These changes not present in l2-mtd-2.6
Thanks
Avinash
Quoted 5 lines
Thanks,
--
Ivan
v3 cover RFC: OMAP GPMC DT bindings
In reply to Daniel MackQuoted 22 lines · Daniel Mack
This is a series of patches to support GPMC peripherals on OMAP boards.
Depends on Linus' master +
omap-next (branch omap-for-v3.8/cleanup-headers-gpmc)The only supported peripheral for now is NAND, but other types would be easy to add.
Version 2 addresses details pointed out by Jon Hunter, Afzal Mohammed and Rob Herring:
- add "reg" and "ti,hwmod" properties to Documentation
- use genericof_mtdfunctions and the property names defined by them, namely "nand-bus-width" and "nand-ecc-mode"
- reduce the default register space size in the Documentation to 8K, as found in the hwmod code
- switch to a DT layout based on ranges and address translation. Although this property is not currently looked at as long as the handling code still uses the runtime calculation methods, we now have these values in the bindings, eventually allowing us to switch the implementation with less pain.
Quoted 7 lines
Version 3 includes fixes pointed out by Jon Hunter:
- better documentation of the 'ranges' property to describe the fact that it's representing the CS lines
-GPMC_CS_CONFIGx->GPMC_CONFIGxin comments
- drop interrupt-parent from example bindings
- addof_node_put()at the end of the child iteration
Thanks. But I am still not completely happy with this. You may wish to wait until we have resolved all the current comments before sending out another version.
Cheers
Jon