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

Version 4 fixes compilation for !CONFIG_MTD_NAND and includes more details from Jon Hunter and Avinash, Philip:

- Add "num-cs" and "num-waitpins" properties, which will eventually be used to get rid of GPMC_CS_NUM
- Better description of generic nand DT properties
- Dropped patch 3/4 as an equivalent fix was already merged
- Added ti,nand-ecc-use-elm property

Daniel Mack (3):
mtd: omap-nand: pass device_node in platform data
ARM: OMAP: gpmc: enable hwecc for AM33xx SoCs
ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND

 .../devicetree/bindings/mtd/gpmc-nand.txt          |  84 ++++++++++
 arch/arm/mach-omap2/gpmc-nand.c                    |   9 +-
 arch/arm/mach-omap2/gpmc.c                         | 170 +++++++++++++++++++++
 drivers/mtd/nand/omap2.c                           |   4 +-
 include/linux/platform_data/mtd-nand-omap2.h       |   2 +
 5 files changed, 264 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt

1.7.11.7

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(-)

index 5c8978e..1e1d6ef 100644
@@ -1333,6 +1333,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) {

@@ -1558,7 +1559,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);

index 24d32ca..5217d6e 100644
@@ -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

1.7.11.7

---
 arch/arm/mach-omap2/gpmc-nand.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

index 8607735..e23d561 100644
@@ -92,17 +92,18 @@ 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;
 	}
 
 	/*
-	 * For now, assume 4-bit mode is only supported on OMAP3630 ES1.x, x>=1.
-	 * Other chips may be added if confirmed to work.
+	 * For now, assume 4-bit mode is only supported on OMAP3630 ES1.x, x>=1
+	 * and AM33xx derivates. Other chips may be added if confirmed to work.
 	 */
 	if ((ecc_opt == OMAP_ECC_BCH4_CODE_HW) &&
-	    (!cpu_is_omap3630() || (GET_OMAP_REVISION() == 0))) {
+	    (!cpu_is_omap3630() || (GET_OMAP_REVISION() == 0)) &&
+	    (!soc_is_am33xx())) {
 		pr_err("BCH 4-bit mode is not supported on this CPU\n");
 		return 0;
 	}

1.7.11.7

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.

---
 .../devicetree/bindings/mtd/gpmc-nand.txt          |  84 ++++++++++
 arch/arm/mach-omap2/gpmc.c                         | 170 +++++++++++++++++++++
 2 files changed, 254 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt

new file mode 100644
index 0000000..20aa5b9
@@ -0,0 +1,84 @@
+Device tree bindings for GPMC connected NANDs
+
+GPMC connected NAND (found on OMAP boards) are represented as child nodes of
+the GPMC controller with a name of "nand".
+
+All timing relevant properties as well as generic gpmc child properties are
+explained in a separate documents - please refer to
+Documentation/devicetree/bindings/bus/ti-gpmc.txt
+
+For NAND specific properties such as ECC modes or bus width, please refer to
+Documentation/devicetree/bindings/mtd/nand.txt
+
+
+Required properties:
+
+ - reg:		The CS line the peripheral is connected to
+
+Optional properties:
+
+ - nand-bus-width: 		Set this numeric value to 16 if the hardware
+				is wired that way. If not specified, a bus
+				width of 8 is assumed.
+ - ti,nand-ecc-opt:		A string setting the ECC layout to use. One of:
+
+	Layouts for 1-bit ecc: stored at end of spare area:
+
+		"software"	Software method (default)
+		"hw"		Hardware method (let gpmc do the error detection)
+
+	Layouts for 1-bit ecc: stored at beginning of spare area as romcode:
+
+		"hw-romcode"	gpmc method & romcode layout
+		"bch4"		4-bit BCH ecc code
+		"bch8"		8-bit BCH ecc code
+
+ - ti,nand-ecc-use-elm:		Property without value to specify that the
+				hardware error correction mode should be used.
+
+For inline partiton table parsing (optional):
+
+ - #address-cells: should be set to 1
+ - #size-cells: should be set to 1
+
+Example for an AM33xx board:
+
+	gpmc: gpmc at 50000000 {
+		compatible = "ti,gpmc";
+		ti,hwmods = "gpmc";
+		reg = <0x50000000 0x1000000>;
+		interrupts = <100>;
+		num-cs = <8>;
+		num-waitpins = <8>;
+		#address-cells = <2>;
+		#size-cells = <1>;
+		ranges = <0 0 0x08000000 0x10000000>;	/* CS0: NAND */
+
+		nand at 0,0 {
+			reg = <0 0 0>; /* CS0, offset 0 */
+			nand-bus-width = <16>;
+			ti,nand-ecc-opt = "none";
+			ti,nand-ecc-use-elm;
+
+			gpmc,sync-clk = <0>;
+			gpmc,cs-on = <0>;
+			gpmc,cs-rd-off = <44>;
+			gpmc,cs-wr-off = <44>;
+			gpmc,adv-on = <6>;
+			gpmc,adv-rd-off = <34>;
+			gpmc,adv-wr-off = <44>;
+			gpmc,we-off = <40>;
+			gpmc,oe-off = <54>;
+			gpmc,access = <64>;
+			gpmc,rd-cycle = <82>;
+			gpmc,wr-cycle = <82>;
+			gpmc,wr-access = <40>;
+			gpmc,wr-data-mux-bus = <0>;
+
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			/* partitions go here */
+		};
+	};
+

1.7.11.7

Hi,

D> This patch adds basic DT bindings for OMAP GPMC.
D> The actual peripherals are instanciated from child nodes within the GPMC

s/instanciated/instantiated/

D> node, and the only type of device that is currently supported is NAND.

D> Code was added to parse the generic GPMC timing parameters and some D> documentation with examples on how to use them.

D> Successfully tested on an AM33xx board.

D> Signed-off-by: Daniel Mack <[email protected]>
D> ---

 D>  .../devicetree/bindings/mtd/gpmc-nand.txt          |  84 ++++++++++
 D>  arch/arm/mach-omap2/gpmc.c                         | 170 +++++++++++++++++++++

D> 2 files changed, 254 insertions(+)
D> create mode 100644 Documentation/devicetree/bindings/mtd/gpmc-nand.txt

D> diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt D> new file mode 100644
D> index 0000000..20aa5b9
D> --- /dev/null
D> +++ b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
D> @@ -0,0 +1,84 @@
D> +Device tree bindings for GPMC connected NANDs
D> +
D> +GPMC connected NAND (found on OMAP boards) are represented as child nodes of
D> +the GPMC controller with a name of "nand".
D> +
D> +All timing relevant properties as well as generic gpmc child properties are
D> +explained in a separate documents - please refer to
D> +Documentation/devicetree/bindings/bus/ti-gpmc.txt

Which this patch seems to be missing?

D> +
D> +For NAND specific properties such as ECC modes or bus width, please refer to D> +Documentation/devicetree/bindings/mtd/nand.txt
D> +
D> +
D> +Required properties:
D> +
D> + - reg: The CS line the peripheral is connected to
D> +
D> +Optional properties:
D> +
D> + - nand-bus-width: Set this numeric value to 16 if the hardware
D> + is wired that way. If not specified, a bus
D> + width of 8 is assumed.
D> + - ti,nand-ecc-opt: A string setting the ECC layout to use. One of:
D> +
D> + Layouts for 1-bit ecc: stored at end of spare area:
D> +
D> + "software" Software method (default)
D> + "hw" Hardware method (let gpmc do the error detection)

Nit: why are you spelling out 'software' when everything else is shortened?

D> +
D> + Layouts for 1-bit ecc: stored at beginning of spare area as romcode:
D> +
D> + "hw-romcode" gpmc method & romcode layout
D> + "bch4" 4-bit BCH ecc code
D> + "bch8" 8-bit BCH ecc code
D> +

These are not 1-bit - Well, romcode might be depending on SoC. Looking at omap2.c it seems to be an alias for hw, so that isn't really helpful on E.G. am33xx where you should select bch8 to have something compatible with the romcode.

D> + - ti,nand-ecc-use-elm: Property without value to specify that the
D> + hardware error correction mode should be used.

It's called the error location module. Like you pointed out yourself, this property isn't really nice. I haven't looked at the elm bindings in detail, but couldn't the nand driver automatically use the elm if the device tree provides a node for it? Perhaps the elm should be a subnode of the nand one?

Bye, Peter Korsgaard

Hi Peter,

Thanks, fixed.

True - sorry for that. Will be back at v5.

Right. Fixed.

Not sure whether I follow you here. drivers/mtd/nand/omap2.c handles cases for pdata->ecc_opt == OMAP_ECC_BCH8_CODE_HW, so the DT bindings need to offer a way to set it.

Or are you purely referring to the comments only?

That is to be discussed for the elm driver then. I'm not sure though whether the elm should always be used when its DT node is present.

For now, I would leave it the way it is and have an optional "ti,nand-ecc-use-elm". I just added some more lines of documenation to describe the fact that the elm driver is needed in order to make it work.

Daniel

Hi,

D> + Layouts for 1-bit ecc: stored at beginning of spare area as romcode:
D> +
D> + "hw-romcode" gpmc method & romcode layout
D> + "bch4" 4-bit BCH ecc code
D> + "bch8" 8-bit BCH ecc code
D> +

Daniel> Not sure whether I follow you here. drivers/mtd/nand/omap2.c handles Daniel> cases for pdata->ecc_opt == OMAP_ECC_BCH8_CODE_HW, so the DT bindings Daniel> need to offer a way to set it.

Daniel> Or are you purely referring to the comments only?

Yes, but the document states (the first line I'm quotinge) 'Layouts for 1-bit ecc'. The other comment was about hw-romcode not being a very good name, as it apparently means the 1bit hamming code and ECC layout used on the older omap3, and not E.G. the bch8/elm layout used by E.G. am335x.

D> + - ti,nand-ecc-use-elm: Property without value to specify that the
D> + hardware error correction mode should be used.

Daniel> That is to be discussed for the elm driver then. I'm not sure though Daniel> whether the elm should always be used when its DT node is present.

Daniel> For now, I would leave it the way it is and have an optional Daniel> "ti,nand-ecc-use-elm". I just added some more lines of documenation to Daniel> describe the fact that the elm driver is needed in order to make it work.

As discussed elsewhere, the elm also changes the ECC layout, so why not just have bchN (software bchN layout) / bchN-elm selections instead of the seperate ti,nand-ecc-use-elm?

Bye, Peter Korsgaard

Ok, I guess I'll just remove these comments then.

So which name would you pick? "bch1"?

Avinash - what do you think?

Thanks for the feedback - I'd also like to get these bindings right in the first place.

Daniel

Hi,

Daniel> So which name would you pick? "bch1"?

No, it's hamming code, isn't it? To me it seems similar to the elm discussion, E.G. we have a mix of ECC algorithm + ECC layout + use of hw acceleration or not.

The choice of hardware acceleration or not should preferably be transparent to this binding (E.G. it is purely an optimization, ECC strength/format shouldn't change), so the driver should be free to use it if it is available.

Daniel> Thanks for the feedback - I'd also like to get these bindings Daniel> right in the first place.

Me too!

Bye, Peter Korsgaard

...

Thanks for sending out the update. Am I missing something or did you drop the gpmc.txt binding documentation?

Cheers
Jon