Skip to contentSkip to conversation

OMAP GPMC DT bindings

PATCH Superseded Daniel Mack 3 participants 22 messages Nov 29, 2012 – Dec 5, 2012

Subsystems (8)

v6 of 8 Superseded Latest is v8 →

lore
Share & tools
lore
#0 lore raw

v6 cover 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

Version 5 with regards to Avinash, Philip and Peter Korsgaard:

- Re-add accidentially forgotten Documentation/devicetree/bindings/bus/ti-gpmc.txt
- Rename "software" ecc mode to "sw"
- Initialize gpmc_nand_data->is_elm_used to 'true' rather than 1
- Drop ti,nand-ecc-use-elm binding in favor of a new ecc mode named "bch8-am335xrbl-compatible"
- Add two more patches for section mismatch fixups

Version 6:

- Dropped "bch8-am335xrbl-compatible" mode again. As discussed with Avinash, the ELM issue will be solved subsequently in s separate series.
- re-added a patch to bail out of automatic GPMC instanciation in case of DT boot.
- re-added the "of_node" addition in mtd_nand_omap2.h in 2/5

Again, many thanks to everybody, in particular Avinash, for the long disussion about how to the these details right in the first place.

Daniel

Daniel Mack (5):
ARM: OMAP: gpmc: don't create devices from initcall on DT
mtd: omap-nand: pass device_node in platform data
ARM: OMAP: gpmc-nand: drop __init annotation
ARM: OMAP: gpmc: enable hwecc for AM33xx SoCs
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          |  76 +++++++++
 arch/arm/mach-omap2/gpmc-nand.c                    |  15 +-
 arch/arm/mach-omap2/gpmc.c                         | 178 ++++++++++++++++++++-
 drivers/mtd/nand/omap2.c                           |   4 +-
 include/linux/platform_data/mtd-nand-omap2.h       |   4 +-
 6 files changed, 343 insertions(+), 10 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
#1 lore raw

PATCH v6 1/5 ARM: OMAP: gpmc: don't create devices from initcall on DT

On 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
index 60f1cce..1dcb30c 100644
@@ -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
#2 lore raw

PATCH v6 1/5 ARM: OMAP: gpmc: don't create devices from initcall on DT

Quoted 16 lines · Daniel Mack

On 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 +0 -0
index 60f1cce..1dcb30c 100644
@@ -844,6 +844,13 @@ static int __init omap_gpmc_init(void)
 	struct platform_device *pdev;
 	char *oh_name = "gpmc";
 

arch/arm/mach-omap2/gpmc.c static int __init omap_gpmc_init(void) · line 856

Quoted 10 lines
arch/arm/mach-omap2/gpmc.c +7 -0
+	/*
+	 * 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);

Thanks
Jon

#3 lore raw

PATCH v6 2/5 mtd: omap-nand: pass device_node in platform data

In reply to Daniel Mack

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 | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)
drivers/mtd/nand/omap2.c +3 -1
index 3282b15..a733f15 100644
@@ -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 +3 -1
index 24d32ca..6bf9ef4 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
Signature · 1 line
1.7.11.7
#4 lore raw

PATCH v6 3/5 ARM: OMAP: gpmc-nand: drop __init annotation

In reply to Daniel Mack

gpmc_nand_init() will be called from another driver's probe() function, so the easiest way to prevent section mismatches is to drop the annotation here.

---
 arch/arm/mach-omap2/gpmc-nand.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
arch/arm/mach-omap2/gpmc-nand.c +3 -3
index 8607735..f9f23a2 100644
@@ -89,7 +89,7 @@ static int omap2_nand_gpmc_retime(
 	return 0;
 }
 
-static bool __init gpmc_hwecc_bch_capable(enum omap_ecc ecc_opt)
+static bool gpmc_hwecc_bch_capable(enum omap_ecc ecc_opt)
 {
 	/* support only OMAP3 class */
 	if (!cpu_is_omap34xx()) {
@@ -110,8 +110,8 @@ static bool __init gpmc_hwecc_bch_capable(enum omap_ecc ecc_opt)
 	return 1;
 }
 
-int __init gpmc_nand_init(struct omap_nand_platform_data *gpmc_nand_data,
-			  struct gpmc_timings *gpmc_t)
+int gpmc_nand_init(struct omap_nand_platform_data *gpmc_nand_data,
+		   struct gpmc_timings *gpmc_t)
 {
 	int err	= 0;
 	struct device *dev = &gpmc_nand_device.dev;
Signature · 1 line
1.7.11.7
#5 lore raw

PATCH v6 4/5 ARM: OMAP: gpmc: enable hwecc for AM33xx SoCs

In reply to Daniel Mack

The am33xx is capable of handling bch error correction modes, so enable that feature in the driver.

---
 arch/arm/mach-omap2/gpmc-nand.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
arch/arm/mach-omap2/gpmc-nand.c +5 -4
index f9f23a2..c8a72ba 100644
@@ -92,17 +92,18 @@ static int omap2_nand_gpmc_retime(
 static bool 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;
 	}
Signature · 1 line
1.7.11.7
#6 lore raw

PATCH v6 4/5 ARM: OMAP: gpmc: enable hwecc for AM33xx SoCs

Quoted 25 lines · Daniel Mack

The am33xx is capable of handling bch error correction modes, so enable that feature in the driver.

---
 arch/arm/mach-omap2/gpmc-nand.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
arch/arm/mach-omap2/gpmc-nand.c +1 -3
index f9f23a2..c8a72ba 100644
@@ -92,17 +92,18 @@ static int omap2_nand_gpmc_retime(
 static bool 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.

arch/arm/mach-omap2/gpmc-nand.c static int omap2_nand_gpmc_retime( · line 109

Quoted 10 lines
arch/arm/mach-omap2/gpmc-nand.c +4 -1
+	 * 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;
 	}

Sorry I should have seen this earlier. Ideally, this type of thing should be reflected by the device-tree/platform-data and we should get away from these cpu_is_xxxx macros for hardware features (where we can). Furthermore, we need to avoid including plat-omap/gpmc.h in drivers for the single zImage work (I see the omap nand driver is including gpmc.h).

Tony, should this be addressed now or can we live this for the minute and fix-up later?

Cheers
Jon

#7 lore raw

PATCH v6 4/5 ARM: OMAP: gpmc: enable hwecc for AM33xx SoCs

Quoted 44 lines · Jon Hunter
Quoted 25 lines · Daniel Mack

The am33xx is capable of handling bch error correction modes, so enable that feature in the driver.

---
 arch/arm/mach-omap2/gpmc-nand.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
arch/arm/mach-omap2/gpmc-nand.c +1 -3
index f9f23a2..c8a72ba 100644
@@ -92,17 +92,18 @@ static int omap2_nand_gpmc_retime(
 static bool 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.

arch/arm/mach-omap2/gpmc-nand.c static int omap2_nand_gpmc_retime( · line 109

Quoted 10 lines
arch/arm/mach-omap2/gpmc-nand.c +4 -1
+	 * 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;
 	}

Sorry I should have seen this earlier. Ideally, this type of thing should be reflected by the device-tree/platform-data and we should get away from these cpu_is_xxxx macros for hardware features (where we can). Furthermore, we need to avoid including plat-omap/gpmc.h in drivers for the single zImage work (I see the omap nand driver is including gpmc.h).

Quoted 2 lines

Tony, should this be addressed now or can we live this for the minute and fix-up later?

Actually, I see that you do read the ecc mode from DT, so is this really needed? It would be good to eliminate this.

Cheers
Jon

#8 lore raw

PATCH v6 4/5 ARM: OMAP: gpmc: enable hwecc for AM33xx SoCs

Quoted 46 lines · Jon Hunter
Quoted 44 lines · Jon Hunter
Quoted 25 lines · Daniel Mack

The am33xx is capable of handling bch error correction modes, so enable that feature in the driver.

---
 arch/arm/mach-omap2/gpmc-nand.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
arch/arm/mach-omap2/gpmc-nand.c +1 -3
index f9f23a2..c8a72ba 100644
@@ -92,17 +92,18 @@ static int omap2_nand_gpmc_retime(
 static bool 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.

arch/arm/mach-omap2/gpmc-nand.c static int omap2_nand_gpmc_retime( · line 109

Quoted 10 lines
arch/arm/mach-omap2/gpmc-nand.c +4 -1
+	 * 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;
 	}

Sorry I should have seen this earlier. Ideally, this type of thing should be reflected by the device-tree/platform-data and we should get away from these cpu_is_xxxx macros for hardware features (where we can). Furthermore, we need to avoid including plat-omap/gpmc.h in drivers for the single zImage work (I see the omap nand driver is including gpmc.h).

Quoted 5 lines
Quoted 2 lines

Tony, should this be addressed now or can we live this for the minute and fix-up later?

Actually, I see that you do read the ecc mode from DT, so is this really needed? It would be good to eliminate this.

The ecc mode is read from DT, right. But the gpmc bindings can be used for many OMAP derivates in the future, and this check is there to make sure the configured settings are supported by the hardware after all, just like if the ecc mode would have been passed as static platform data. So what is it exactly that you want to get rid of?

I agree though that we could solve this with via the of_device_id's data pointer of the matching driver. But as you said yourself, this can well be done later, and the problem here is that we still need the cpu_is_xxx() macros for older platforms, right?

Thanks,
Daniel

#9 lore raw

PATCH v6 4/5 ARM: OMAP: gpmc: enable hwecc for AM33xx SoCs

Quoted 50 lines · Daniel Mack
Quoted 49 lines · Jon Hunter
Quoted 46 lines · Jon Hunter
Quoted 25 lines · Daniel Mack

The am33xx is capable of handling bch error correction modes, so enable that feature in the driver.

---
 arch/arm/mach-omap2/gpmc-nand.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
arch/arm/mach-omap2/gpmc-nand.c +1 -3
index f9f23a2..c8a72ba 100644
@@ -92,17 +92,18 @@ static int omap2_nand_gpmc_retime(
 static bool 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.

arch/arm/mach-omap2/gpmc-nand.c static int omap2_nand_gpmc_retime( · line 109

Quoted 10 lines
arch/arm/mach-omap2/gpmc-nand.c +4 -1
+	 * 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;
 	}

Sorry I should have seen this earlier. Ideally, this type of thing should be reflected by the device-tree/platform-data and we should get away from these cpu_is_xxxx macros for hardware features (where we can). Furthermore, we need to avoid including plat-omap/gpmc.h in drivers for the single zImage work (I see the omap nand driver is including gpmc.h).

Tony, should this be addressed now or can we live this for the minute and fix-up later?

Quoted 8 lines
Quoted 2 lines

Actually, I see that you do read the ecc mode from DT, so is this really needed? It would be good to eliminate this.

The ecc mode is read from DT, right. But the gpmc bindings can be used for many OMAP derivates in the future, and this check is there to make sure the configured settings are supported by the hardware after all, just like if the ecc mode would have been passed as static platform data. So what is it exactly that you want to get rid of?

The above function.

If there is a hardware bug that prevents us from using the hw-ecc mode that is supported (which is the case for omap3630 es1.0), then we should have a GPMC_ERRATA_xxx flag somewhere to indicate this and these errata flags should be populated at probe.

Quoted 4 lines

I agree though that we could solve this with via the of_device_id's data pointer of the matching driver. But as you said yourself, this can well be done later, and the problem here is that we still need the cpu_is_xxx() macros for older platforms, right?

Yes, but we cannot/shouldn't use these cpu_is_xxx macros from within driver code. Ok, here we can calling a platform function that is calling the macro, but for single zImage we cannot do that either. We cannot include platform headers in drivers for single zImage. We have to get away from that. Therefore, such information needs to be passed by platform data, device tree, etc.

I will let Tony decide on how he wants us to handle this.

Cheers
Jon

#10 lore raw

PATCH v6 4/5 ARM: OMAP: gpmc: enable hwecc for AM33xx SoCs

Quoted 72 lines · Jon Hunter
Quoted 50 lines · Daniel Mack
Quoted 49 lines · Jon Hunter
Quoted 46 lines · Jon Hunter
Quoted 25 lines · Daniel Mack

The am33xx is capable of handling bch error correction modes, so enable that feature in the driver.

---
 arch/arm/mach-omap2/gpmc-nand.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
arch/arm/mach-omap2/gpmc-nand.c +1 -3
index f9f23a2..c8a72ba 100644
@@ -92,17 +92,18 @@ static int omap2_nand_gpmc_retime(
 static bool 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.

arch/arm/mach-omap2/gpmc-nand.c static int omap2_nand_gpmc_retime( · line 109

Quoted 10 lines
arch/arm/mach-omap2/gpmc-nand.c +4 -1
+	 * 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;
 	}

Sorry I should have seen this earlier. Ideally, this type of thing should be reflected by the device-tree/platform-data and we should get away from these cpu_is_xxxx macros for hardware features (where we can). Furthermore, we need to avoid including plat-omap/gpmc.h in drivers for the single zImage work (I see the omap nand driver is including gpmc.h).

Tony, should this be addressed now or can we live this for the minute and fix-up later?

Quoted 8 lines
Quoted 2 lines

Actually, I see that you do read the ecc mode from DT, so is this really needed? It would be good to eliminate this.

The ecc mode is read from DT, right. But the gpmc bindings can be used for many OMAP derivates in the future, and this check is there to make sure the configured settings are supported by the hardware after all, just like if the ecc mode would have been passed as static platform data. So what is it exactly that you want to get rid of?

The above function.

If there is a hardware bug that prevents us from using the hw-ecc mode that is supported (which is the case for omap3630 es1.0), then we should have a GPMC_ERRATA_xxx flag somewhere to indicate this and these errata flags should be populated at probe.

Quoted 4 lines

I agree though that we could solve this with via the of_device_id's data pointer of the matching driver. But as you said yourself, this can well be done later, and the problem here is that we still need the cpu_is_xxx() macros for older platforms, right?

Quoted 8 lines

Yes, but we cannot/shouldn't use these cpu_is_xxx macros from within driver code. Ok, here we can calling a platform function that is calling the macro, but for single zImage we cannot do that either. We cannot include platform headers in drivers for single zImage. We have to get away from that. Therefore, such information needs to be passed by platform data, device tree, etc.

I will let Tony decide on how he wants us to handle this.

Any idea yet about this detail? The other small Documentation changes you brought up are fixed in my tree and ready for resubmission.

Daniel

maintainer
#11 lore raw

PATCH v6 4/5 ARM: OMAP: gpmc: enable hwecc for AM33xx SoCs

Quoted 80 lines · Daniel Mack
Quoted 79 lines · Jon Hunter
Quoted 50 lines · Daniel Mack
Quoted 49 lines · Jon Hunter
Quoted 46 lines · Jon Hunter
Quoted 25 lines · Daniel Mack

The am33xx is capable of handling bch error correction modes, so enable that feature in the driver.

---
 arch/arm/mach-omap2/gpmc-nand.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
arch/arm/mach-omap2/gpmc-nand.c +1 -3
index f9f23a2..c8a72ba 100644
@@ -92,17 +92,18 @@ static int omap2_nand_gpmc_retime(
 static bool 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.

arch/arm/mach-omap2/gpmc-nand.c static int omap2_nand_gpmc_retime( · line 109

Quoted 10 lines
arch/arm/mach-omap2/gpmc-nand.c +4 -1
+	 * 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;
 	}

Sorry I should have seen this earlier. Ideally, this type of thing should be reflected by the device-tree/platform-data and we should get away from these cpu_is_xxxx macros for hardware features (where we can). Furthermore, we need to avoid including plat-omap/gpmc.h in drivers for the single zImage work (I see the omap nand driver is including gpmc.h).

Tony, should this be addressed now or can we live this for the minute and fix-up later?

Quoted 8 lines
Quoted 2 lines

Actually, I see that you do read the ecc mode from DT, so is this really needed? It would be good to eliminate this.

The ecc mode is read from DT, right. But the gpmc bindings can be used for many OMAP derivates in the future, and this check is there to make sure the configured settings are supported by the hardware after all, just like if the ecc mode would have been passed as static platform data. So what is it exactly that you want to get rid of?

The above function.

If there is a hardware bug that prevents us from using the hw-ecc mode that is supported (which is the case for omap3630 es1.0), then we should have a GPMC_ERRATA_xxx flag somewhere to indicate this and these errata flags should be populated at probe.

Quoted 4 lines

I agree though that we could solve this with via the of_device_id's data pointer of the matching driver. But as you said yourself, this can well be done later, and the problem here is that we still need the cpu_is_xxx() macros for older platforms, right?

Yes, but we cannot/shouldn't use these cpu_is_xxx macros from within driver code. Ok, here we can calling a platform function that is calling the macro, but for single zImage we cannot do that either. We cannot include platform headers in drivers for single zImage. We have to get away from that. Therefore, such information needs to be passed by platform data, device tree, etc.

Quoted 4 lines
Quoted 1 line

I will let Tony decide on how he wants us to handle this.

Any idea yet about this detail? The other small Documentation changes you brought up are fixed in my tree and ready for resubmission.

The plat/cpu.h file will disappear after the merge window, which means omap2+ related drivers cannot use cpu_is_omap macros.

For legacy booting systems, this flag should be just passed in the platform_data from the platform init code. Then device tree can deal with it based on the compatible flag.

Regards,

Tony

#12 lore raw

PATCH v6 4/5 ARM: OMAP: gpmc: enable hwecc for AM33xx SoCs

Hi Tony,

Quoted 86 lines · Tony Lindgren
Quoted 80 lines · Daniel Mack
Quoted 79 lines · Jon Hunter
Quoted 50 lines · Daniel Mack
Quoted 49 lines · Jon Hunter
Quoted 46 lines · Jon Hunter
Quoted 25 lines · Daniel Mack

The am33xx is capable of handling bch error correction modes, so enable that feature in the driver.

---
 arch/arm/mach-omap2/gpmc-nand.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
arch/arm/mach-omap2/gpmc-nand.c +1 -3
index f9f23a2..c8a72ba 100644
@@ -92,17 +92,18 @@ static int omap2_nand_gpmc_retime(
 static bool 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.

arch/arm/mach-omap2/gpmc-nand.c static int omap2_nand_gpmc_retime( · line 109

Quoted 10 lines
arch/arm/mach-omap2/gpmc-nand.c +4 -1
+	 * 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;
 	}

Sorry I should have seen this earlier. Ideally, this type of thing should be reflected by the device-tree/platform-data and we should get away from these cpu_is_xxxx macros for hardware features (where we can). Furthermore, we need to avoid including plat-omap/gpmc.h in drivers for the single zImage work (I see the omap nand driver is including gpmc.h).

Tony, should this be addressed now or can we live this for the minute and fix-up later?

Quoted 8 lines
Quoted 2 lines

Actually, I see that you do read the ecc mode from DT, so is this really needed? It would be good to eliminate this.

The ecc mode is read from DT, right. But the gpmc bindings can be used for many OMAP derivates in the future, and this check is there to make sure the configured settings are supported by the hardware after all, just like if the ecc mode would have been passed as static platform data. So what is it exactly that you want to get rid of?

The above function.

If there is a hardware bug that prevents us from using the hw-ecc mode that is supported (which is the case for omap3630 es1.0), then we should have a GPMC_ERRATA_xxx flag somewhere to indicate this and these errata flags should be populated at probe.

Quoted 4 lines

I agree though that we could solve this with via the of_device_id's data pointer of the matching driver. But as you said yourself, this can well be done later, and the problem here is that we still need the cpu_is_xxx() macros for older platforms, right?

Yes, but we cannot/shouldn't use these cpu_is_xxx macros from within driver code. Ok, here we can calling a platform function that is calling the macro, but for single zImage we cannot do that either. We cannot include platform headers in drivers for single zImage. We have to get away from that. Therefore, such information needs to be passed by platform data, device tree, etc.

Quoted 4 lines
Quoted 1 line

I will let Tony decide on how he wants us to handle this.

Any idea yet about this detail? The other small Documentation changes you brought up are fixed in my tree and ready for resubmission.

Quoted 7 lines

The plat/cpu.h file will disappear after the merge window, which means omap2+ related drivers cannot use cpu_is_omap macros.

For legacy booting systems, this flag should be just passed in the platform_data from the platform init code. Then device tree can deal with it based on the compatible flag.

Ok, thanks for explaining.

Does that mean this patch series should be postponed until after the merge window and then build upon that change or should we merge the patch in question here as is and then care for the cleanups after the window?

I can also rebase this set on top of the removal patches if they exist already somewhere.

Daniel

maintainer
#13 lore raw

PATCH v6 4/5 ARM: OMAP: gpmc: enable hwecc for AM33xx SoCs

Quoted 10 lines · Daniel Mack
Quoted 8 lines · Tony Lindgren

The plat/cpu.h file will disappear after the merge window, which means omap2+ related drivers cannot use cpu_is_omap macros.

For legacy booting systems, this flag should be just passed in the platform_data from the platform init code. Then device tree can deal with it based on the compatible flag.

Quoted 6 lines

Ok, thanks for explaining.

Does that mean this patch series should be postponed until after the merge window and then build upon that change or should we merge the patch in question here as is and then care for the cleanups after the window?

Well to me it seems that you only have cpu_is_omap usage in arch/arm/mach-omap2/gpmc-nand.c, which will be OK. Only the code under drivers/* needs to be fixed for that. So your patches may be OK, but..

Quoted 2 lines

I can also rebase this set on top of the removal patches if they exist already somewhere.

..please check that your patches work with current linux next.

It's too late to queue thing for v3.8 merge window at this point, as we want the patches sitting in linux next for a week at least before they get pulled.

And we still need the acks for the device tree binding as well.

So let's plan on queueing these after -rc1.

Regards,

Tony

#14 lore raw

PATCH v6 4/5 ARM: OMAP: gpmc: enable hwecc for AM33xx SoCs

Quoted 18 lines · Tony Lindgren
Quoted 10 lines · Daniel Mack
Quoted 8 lines · Tony Lindgren

The plat/cpu.h file will disappear after the merge window, which means omap2+ related drivers cannot use cpu_is_omap macros.

For legacy booting systems, this flag should be just passed in the platform_data from the platform init code. Then device tree can deal with it based on the compatible flag.

Quoted 6 lines

Ok, thanks for explaining.

Does that mean this patch series should be postponed until after the merge window and then build upon that change or should we merge the patch in question here as is and then care for the cleanups after the window?

Quoted 4 lines

Well to me it seems that you only have cpu_is_omap usage in arch/arm/mach-omap2/gpmc-nand.c, which will be OK. Only the code under drivers/* needs to be fixed for that. So your patches may be OK, but..

The real problem here is that drivers/mtd/nand/omap2.c is including plat/gpmc.h and calling a function in arch/arm/mach-omap/gpmc-nand.c. So although Daniel's patches are not introducing any new problems for single zImage, they do highlight a problem that we have with the omap2 nand driver that needs to be addressed for single zImage.

So either we fix this now or after merging Daniel's changes. Either is fine with me.

Cheers
Jon

maintainer
#15 lore raw

PATCH v6 4/5 ARM: OMAP: gpmc: enable hwecc for AM33xx SoCs

Quoted 25 lines · Jon Hunter
Quoted 18 lines · Tony Lindgren
Quoted 10 lines · Daniel Mack
Quoted 8 lines · Tony Lindgren

The plat/cpu.h file will disappear after the merge window, which means omap2+ related drivers cannot use cpu_is_omap macros.

For legacy booting systems, this flag should be just passed in the platform_data from the platform init code. Then device tree can deal with it based on the compatible flag.

Quoted 6 lines

Ok, thanks for explaining.

Does that mean this patch series should be postponed until after the merge window and then build upon that change or should we merge the patch in question here as is and then care for the cleanups after the window?

Quoted 4 lines

Well to me it seems that you only have cpu_is_omap usage in arch/arm/mach-omap2/gpmc-nand.c, which will be OK. Only the code under drivers/* needs to be fixed for that. So your patches may be OK, but..

Quoted 8 lines

The real problem here is that drivers/mtd/nand/omap2.c is including plat/gpmc.h and calling a function in arch/arm/mach-omap/gpmc-nand.c. So although Daniel's patches are not introducing any new problems for single zImage, they do highlight a problem that we have with the omap2 nand driver that needs to be addressed for single zImage.

So either we fix this now or after merging Daniel's changes. Either is fine with me.

Ah I see. Yes it would be good to fix that issue first to avoid adding any new blockers for multiplatform build.

Regards,

Tony

#16 lore raw

PATCH v6 4/5 ARM: OMAP: gpmc: enable hwecc for AM33xx SoCs

Quoted 32 lines · Tony Lindgren
Quoted 31 lines · Jon Hunter
Quoted 18 lines · Tony Lindgren
Quoted 10 lines · Daniel Mack
Quoted 8 lines · Tony Lindgren

The plat/cpu.h file will disappear after the merge window, which means omap2+ related drivers cannot use cpu_is_omap macros.

For legacy booting systems, this flag should be just passed in the platform_data from the platform init code. Then device tree can deal with it based on the compatible flag.

Quoted 6 lines

Ok, thanks for explaining.

Does that mean this patch series should be postponed until after the merge window and then build upon that change or should we merge the patch in question here as is and then care for the cleanups after the window?

Quoted 4 lines

Well to me it seems that you only have cpu_is_omap usage in arch/arm/mach-omap2/gpmc-nand.c, which will be OK. Only the code under drivers/* needs to be fixed for that. So your patches may be OK, but..

The real problem here is that drivers/mtd/nand/omap2.c is including plat/gpmc.h and calling a function in arch/arm/mach-omap/gpmc-nand.c. So although Daniel's patches are not introducing any new problems for single zImage, they do highlight a problem that we have with the omap2 nand driver that needs to be addressed for single zImage.

Quoted 5 lines
Quoted 2 lines

So either we fix this now or after merging Daniel's changes. Either is fine with me.

Ah I see. Yes it would be good to fix that issue first to avoid adding any new blockers for multiplatform build.

Already fixed by Afzal here:

https://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap.git;a=commitdiff;h=2ef9f3dd

... which is also in linux-next.

Daniel

#17 lore raw

PATCH v6 4/5 ARM: OMAP: gpmc: enable hwecc for AM33xx SoCs

Quoted 36 lines · Daniel Mack
Quoted 35 lines · Tony Lindgren
Quoted 33 lines · Jon Hunter
Quoted 18 lines · Tony Lindgren
Quoted 10 lines · Daniel Mack
Quoted 8 lines · Tony Lindgren

The plat/cpu.h file will disappear after the merge window, which means omap2+ related drivers cannot use cpu_is_omap macros.

For legacy booting systems, this flag should be just passed in the platform_data from the platform init code. Then device tree can deal with it based on the compatible flag.

Quoted 6 lines

Ok, thanks for explaining.

Does that mean this patch series should be postponed until after the merge window and then build upon that change or should we merge the patch in question here as is and then care for the cleanups after the window?

Quoted 4 lines

Well to me it seems that you only have cpu_is_omap usage in arch/arm/mach-omap2/gpmc-nand.c, which will be OK. Only the code under drivers/* needs to be fixed for that. So your patches may be OK, but..

The real problem here is that drivers/mtd/nand/omap2.c is including plat/gpmc.h and calling a function in arch/arm/mach-omap/gpmc-nand.c. So although Daniel's patches are not introducing any new problems for single zImage, they do highlight a problem that we have with the omap2 nand driver that needs to be addressed for single zImage.

So either we fix this now or after merging Daniel's changes. Either is fine with me.

Quoted 8 lines
Quoted 2 lines

Ah I see. Yes it would be good to fix that issue first to avoid adding any new blockers for multiplatform build.

Already fixed by Afzal here:

https://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap.git;a=commitdiff;h=2ef9f3dd

... which is also in linux-next.

Great! So we can drop this change.

Cheers
Jon

#18 lore raw

PATCH v6 4/5 ARM: OMAP: gpmc: enable hwecc for AM33xx SoCs

In reply to Tony Lindgren
Quoted 26 lines · Tony Lindgren
Quoted 10 lines · Daniel Mack
Quoted 8 lines · Tony Lindgren

The plat/cpu.h file will disappear after the merge window, which means omap2+ related drivers cannot use cpu_is_omap macros.

For legacy booting systems, this flag should be just passed in the platform_data from the platform init code. Then device tree can deal with it based on the compatible flag.

Quoted 6 lines

Ok, thanks for explaining.

Does that mean this patch series should be postponed until after the merge window and then build upon that change or should we merge the patch in question here as is and then care for the cleanups after the window?

Well to me it seems that you only have cpu_is_omap usage in arch/arm/mach-omap2/gpmc-nand.c, which will be OK. Only the code under drivers/* needs to be fixed for that. So your patches may be OK, but..

Quoted 2 lines

I can also rebase this set on top of the removal patches if they exist already somewhere.

Quoted 7 lines

..please check that your patches work with current linux next.

It's too late to queue thing for v3.8 merge window at this point, as we want the patches sitting in linux next for a week at least before they get pulled.

And we still need the acks for the device tree binding as well.

Ok, I'll resubmit a v7 with the nits fixed that Jon pointed out, and then hope for the maintainer's Acks.

Quoted 1 line

So let's plan on queueing these after -rc1.

No problem really. The only thing is that I'll be on vacation when the merge window closes, hence I'd appreciate if you could already queue them up on your side, so they'll be taken care for :)

Daniel

#19 lore raw

PATCH v6 5/5 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND

In reply to Daniel Mack

This patch adds basic DT bindings for OMAP GPMC.

The actual peripherals are instantiated 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          |  76 +++++++++
 arch/arm/mach-omap2/gpmc.c                         | 171 ++++++++++++++++++++-
 3 files changed, 322 insertions(+), 1 deletion(-)
 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
new file mode 100644
index 0000000..ba3d6a5
@@ -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
+ - num-cs:		The maximum number of chip-select lines that controller
+			can support.
+ - num-waitpins:	The maximum number of wait pins that controller can
+			support.
+ - 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>;
+
+		num-cs = <8>;
+		#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 +76 -0

76 lines not on this page · raw message

arch/arm/mach-omap2/gpmc.c +170 -1

201 lines not on this page · raw message

Signature · 1 line
1.7.11.7
#20 lore raw

PATCH v6 5/5 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND

Quoted 74 lines · Daniel Mack

This patch adds basic DT bindings for OMAP GPMC.

The actual peripherals are instantiated 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          |  76 +++++++++
 arch/arm/mach-omap2/gpmc.c                         | 171 ++++++++++++++++++++-
 3 files changed, 322 insertions(+), 1 deletion(-)
 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 +49 -0
new file mode 100644
index 0000000..ba3d6a5
@@ -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
+ - num-cs:		The maximum number of chip-select lines that controller
+			can support.
+ - num-waitpins:	The maximum number of wait pins that controller can
+			support.
+ - 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

Documentation/devicetree/bindings/bus/ti-gpmc.txt line 59

Quoted 10 lines
Documentation/devicetree/bindings/bus/ti-gpmc.txt new file +10 -0
+
+ 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

Actually, these are applicable to AM335x as well. I believe that the comment should be "applicable to OMAP3+ and AM335x". The gpmc driver was updated to set the flags for these features based upon gpmc version ID.

Quoted 4 lines
Documentation/devicetree/bindings/bus/ti-gpmc.txt new file +4 -0
+
+
+Example for an AM33xx board:
+

Documentation/devicetree/bindings/bus/ti-gpmc.txt line 73

Quoted 10 lines
Documentation/devicetree/bindings/bus/ti-gpmc.txt new file +10 -0
+	gpmc: gpmc at 50000000 {
+		compatible = "ti,gpmc";
+		ti,hwmods = "gpmc";
+		reg = <0x50000000 0x2000>;
+		interrupts = <100>;
+
+		num-cs = <8>;
+		#address-cells = <2>;
+		#size-cells = <1>;
+		ranges = <0 0 0x08000000 0x10000000>; /* CS0 @addr 0x8000000, size 0x10000000 */

Nit, you have num-wait pins as a required property but not shown here in the example.

Quoted 41 lines
Documentation/devicetree/bindings/bus/ti-gpmc.txt new file +3 -0
+
+		/* child nodes go here */
+	};
Documentation/devicetree/bindings/mtd/gpmc-nand.txt new file +32 -0
new file mode 100644
index 0000000..58c876f
@@ -0,0 +1,76 @@
+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:
+
+		"sw"		Software method (default)
+		"hw"		Hardware method
+		"hw-romcode"	gpmc hamming mode method & romcode layout
+		"bch4"		4-bit BCH ecc code
+		"bch8"		8-bit BCH ecc code
+
+For inline partiton table parsing (optional):

Documentation/devicetree/bindings/mtd/gpmc-nand.txt line 42

Quoted 10 lines
Documentation/devicetree/bindings/mtd/gpmc-nand.txt new file +10 -0
+
+ - #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>;

Nit, good to make reg size consistent with the above example.

Documentation/devicetree/bindings/mtd/gpmc-nand.txt line 45

Quoted 3 lines
Documentation/devicetree/bindings/mtd/gpmc-nand.txt new file +3 -0
+		interrupts = <100>;
+		num-cs = <8>;
+		num-waitpins = <8>;

Nit, AM335x only has 2 wait-pins. Number of wait-pins is typically less than number of CS pins.

Other than these minor comments, looks good to me. Thanks for updating.

Cheers
Jon

#21 lore raw

PATCH v6 5/5 ARM: OMAP: gpmc: add DT bindings for GPMC timings and NAND

Quoted 163 lines · Jon Hunter
Quoted 74 lines · Daniel Mack

This patch adds basic DT bindings for OMAP GPMC.

The actual peripherals are instantiated 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          |  76 +++++++++
 arch/arm/mach-omap2/gpmc.c                         | 171 ++++++++++++++++++++-
 3 files changed, 322 insertions(+), 1 deletion(-)
 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 +49 -0
new file mode 100644
index 0000000..ba3d6a5
@@ -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
+ - num-cs:		The maximum number of chip-select lines that controller
+			can support.
+ - num-waitpins:	The maximum number of wait pins that controller can
+			support.
+ - 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

Documentation/devicetree/bindings/bus/ti-gpmc.txt line 59

Quoted 10 lines
Documentation/devicetree/bindings/bus/ti-gpmc.txt new file +10 -0
+
+ 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

Actually, these are applicable to AM335x as well. I believe that the comment should be "applicable to OMAP3+ and AM335x". The gpmc driver was updated to set the flags for these features based upon gpmc version ID.

Quoted 4 lines
Documentation/devicetree/bindings/bus/ti-gpmc.txt new file +4 -0
+
+
+Example for an AM33xx board:
+

Documentation/devicetree/bindings/bus/ti-gpmc.txt line 73

Quoted 10 lines
Documentation/devicetree/bindings/bus/ti-gpmc.txt new file +10 -0
+	gpmc: gpmc at 50000000 {
+		compatible = "ti,gpmc";
+		ti,hwmods = "gpmc";
+		reg = <0x50000000 0x2000>;
+		interrupts = <100>;
+
+		num-cs = <8>;
+		#address-cells = <2>;
+		#size-cells = <1>;
+		ranges = <0 0 0x08000000 0x10000000>; /* CS0 @addr 0x8000000, size 0x10000000 */

Nit, you have num-wait pins as a required property but not shown here in the example.

Quoted 41 lines
Documentation/devicetree/bindings/bus/ti-gpmc.txt new file +3 -0
+
+		/* child nodes go here */
+	};
Documentation/devicetree/bindings/mtd/gpmc-nand.txt new file +32 -0
new file mode 100644
index 0000000..58c876f
@@ -0,0 +1,76 @@
+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:
+
+		"sw"		Software method (default)
+		"hw"		Hardware method
+		"hw-romcode"	gpmc hamming mode method & romcode layout
+		"bch4"		4-bit BCH ecc code
+		"bch8"		8-bit BCH ecc code
+
+For inline partiton table parsing (optional):

Documentation/devicetree/bindings/mtd/gpmc-nand.txt line 42

Quoted 10 lines
Documentation/devicetree/bindings/mtd/gpmc-nand.txt new file +10 -0
+
+ - #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>;

Nit, good to make reg size consistent with the above example.

Quoted 8 lines

Documentation/devicetree/bindings/mtd/gpmc-nand.txt line 45

Quoted 3 lines
Documentation/devicetree/bindings/mtd/gpmc-nand.txt new file +3 -0
+		interrupts = <100>;
+		num-cs = <8>;
+		num-waitpins = <8>;

Nit, AM335x only has 2 wait-pins. Number of wait-pins is typically less than number of CS pins.

Other than these minor comments, looks good to me. Thanks for updating.

Thanks for these remarks! All fixed locally. I can send out a v7 tomorrow.

Best,
Daniel