[PATCH v7,1/3] dt-bindings: binding for jz4780-{nand,bch}

[PATCH v7,1/3] dt-bindings: binding for jz4780-{nand,bch}

[Harvey Hunt]

From: Alex Smith <alex.smith-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>

Add DT bindings for NAND devices connected to the NEMC on JZ4780 SoCs,
as well as the hardware BCH controller, used by the jz4780_{nand,bch}
drivers.

Signed-off-by: Alex Smith <alex.smith-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
Cc: Zubair Lutfullah Kakakhel <Zubair.Kakakhel-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
Cc: David Woodhouse <dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
Cc: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Alex Smith <alex-oucj9GSTHrKwpo/f3jThPQ@public.gmane.org>
Signed-off-by: Harvey Hunt <harvey.hunt-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
---
v6 -> v7:
 - Add nand-ecc-mode to DT bindings.
 - Add nand-on-flash-bbt to DT bindings.

v5 -> v6:
 - No change.

v4 -> v5:
 - Rename ingenic,bch-device to ingenic,bch-controller to fit with
   existing convention.

v3 -> v4:
 - No change

v2 -> v3:
 - Rebase to 4.0-rc6
 - Changed ingenic,ecc-size to common nand-ecc-step-size
 - Changed ingenic,ecc-strength to common nand-ecc-strength
 - Changed ingenic,busy-gpio to common rb-gpios
 - Changed ingenic,wp-gpio to common wp-gpios

v1 -> v2:
 - Rebase to 4.0-rc3

 .../bindings/mtd/ingenic,jz4780-nand.txt           | 61 ++++++++++++++++++++++
 1 file changed, 61 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt

diff --git a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
new file mode 100644
index 0000000..44a0468
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
@@ -0,0 +1,61 @@
+* Ingenic JZ4780 NAND/BCH
+
+This file documents the device tree bindings for NAND flash devices on the
+JZ4780. NAND devices are connected to the NEMC controller (described in
+memory-controllers/ingenic,jz4780-nemc.txt), and thus NAND device nodes must
+be children of the NEMC node.
+
+Required NAND device properties:
+- compatible: Should be set to "ingenic,jz4780-nand".
+- reg: For each bank with a NAND chip attached, should specify a bank number,
+  an offset of 0 and a size of 0x1000000 (i.e. the whole NEMC bank).
+
+Optional NAND device properties:
+- ingenic,bch-controller: To make use of the hardware BCH controller, this
+  property must contain a phandle for the BCH controller node. The required
+  properties for this node are described below. If this is not specified,
+  software BCH will be used instead.
+- nand-ecc-step-size: ECC block size in bytes.
+- nand-ecc-strength: ECC strength (max number of correctable bits).
+- nand-ecc-mode: String, operation mode of the NAND ecc mode. "hw" by default
+- nand-on-flash-bbt: boolean to enable on flash bbt option if not present false
+- rb-gpios: GPIO specifier for the busy pin.
+- wp-gpios: GPIO specifier for the write protect pin.
+
+Example:
+
+nemc: nemc@13410000 {
+	...
+
+	nand: nand@1 {
+		compatible = "ingenic,jz4780-nand";
+		reg = <1 0 0x1000000>;	/* Bank 1 */
+
+		ingenic,bch-controller = <&bch>;
+		nand-ecc-step-size = <1024>;
+		nand-ecc-strength = <24>;
+		nand-ecc-mode = "hw";
+		nand-on-flash-bbt;
+
+		rb-gpios = <&gpa 20 GPIO_ACTIVE_LOW>;
+		wp-gpios = <&gpf 22 GPIO_ACTIVE_LOW>;
+	};
+};
+
+The BCH controller is a separate SoC component used for error correction on
+NAND devices. The following is a description of the device properties for a
+BCH controller.
+
+Required BCH properties:
+- compatible: Should be set to "ingenic,jz4780-bch".
+- reg: Should specify the BCH controller registers location and length.
+- clocks: Clock for the BCH controller.
+
+Example:
+
+bch: bch@134d0000 {
+	compatible = "ingenic,jz4780-bch";
+	reg = <0x134d0000 0x10000>;
+
+	clocks = <&cgu JZ4780_CLK_BCH>;
+};
-- 
2.6.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v7,3/3] MIPS: dts: jz4780/ci20: Add NEMC, BCH and NAND device tree nodes

[Harvey Hunt]

From: Alex Smith <alex.smith@imgtec.com>

Add device tree nodes for the NEMC and BCH to the JZ4780 device tree,
and make use of them in the Ci20 device tree to add a node for the
board's NAND.

Note that since the pinctrl driver is not yet upstream, this includes
neither pin configuration nor busy/write-protect GPIO pins for the
NAND. Use of the NAND relies on the boot loader to have left the pins
configured in a usable state, which should be the case when booted
from the NAND.

Signed-off-by: Alex Smith <alex.smith@imgtec.com>
Cc: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Brian Norris <computersforpeace@gmail.com>
Cc: Paul Burton <paul.burton@imgtec.com>
Cc: linux-mtd@lists.infradead.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-mips@linux-mips.org
Cc: Alex Smith <alex@alex-smith.me.uk>
Signed-off-by: Harvey Hunt <harvey.hunt@imgtec.com>
---
v6 -> v7:
 - Add nand-ecc-mode to DT.
 - Add nand-on-flash-bbt to DT.

v4 -> v5:
 - New patch adding DT nodes for the NAND so that the driver can be
   tested.

 arch/mips/boot/dts/ingenic/ci20.dts    | 54 ++++++++++++++++++++++++++++++++++
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 26 ++++++++++++++++
 2 files changed, 80 insertions(+)

diff --git a/arch/mips/boot/dts/ingenic/ci20.dts b/arch/mips/boot/dts/ingenic/ci20.dts
index 9fcb9e7..453f1d3 100644
--- a/arch/mips/boot/dts/ingenic/ci20.dts
+++ b/arch/mips/boot/dts/ingenic/ci20.dts
@@ -42,3 +42,57 @@
 &uart4 {
 	status = "okay";
 };
+
+&nemc {
+	status = "okay";
+
+	nand: nand@1 {
+		compatible = "ingenic,jz4780-nand";
+		reg = <1 0 0x1000000>;
+
+		ingenic,nemc-tAS = <10>;
+		ingenic,nemc-tAH = <5>;
+		ingenic,nemc-tBP = <10>;
+		ingenic,nemc-tAW = <15>;
+		ingenic,nemc-tSTRV = <100>;
+
+		ingenic,bch-controller = <&bch>;
+		ingenic,ecc-size = <1024>;
+		ingenic,ecc-strength = <24>;
+
+		nand-ecc-mode = "hw";
+		nand-on-flash-bbt;
+
+		#address-cells = <2>;
+		#size-cells = <2>;
+
+		partition@0 {
+			label = "u-boot-spl";
+			reg = <0x0 0x0 0x0 0x800000>;
+		};
+
+		partition@0x800000 {
+			label = "u-boot";
+			reg = <0x0 0x800000 0x0 0x200000>;
+		};
+
+		partition@0xa00000 {
+			label = "u-boot-env";
+			reg = <0x0 0xa00000 0x0 0x200000>;
+		};
+
+		partition@0xc00000 {
+			label = "boot";
+			reg = <0x0 0xc00000 0x0 0x4000000>;
+		};
+
+		partition@0x8c00000 {
+			label = "system";
+			reg = <0x0 0x4c00000 0x1 0xfb400000>;
+		};
+	};
+};
+
+&bch {
+	status = "okay";
+};
diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi b/arch/mips/boot/dts/ingenic/jz4780.dtsi
index 65389f6..b868b42 100644
--- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
+++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
@@ -108,4 +108,30 @@
 
 		status = "disabled";
 	};
+
+	nemc: nemc@13410000 {
+		compatible = "ingenic,jz4780-nemc";
+		reg = <0x13410000 0x10000>;
+		#address-cells = <2>;
+		#size-cells = <1>;
+		ranges = <1 0 0x1b000000 0x1000000
+			  2 0 0x1a000000 0x1000000
+			  3 0 0x19000000 0x1000000
+			  4 0 0x18000000 0x1000000
+			  5 0 0x17000000 0x1000000
+			  6 0 0x16000000 0x1000000>;
+
+		clocks = <&cgu JZ4780_CLK_NEMC>;
+
+		status = "disabled";
+	};
+
+	bch: bch@134d0000 {
+		compatible = "ingenic,jz4780-bch";
+		reg = <0x134d0000 0x10000>;
+
+		clocks = <&cgu JZ4780_CLK_BCH>;
+
+		status = "disabled";
+	};
 };
-- 
2.6.0

Re: [PATCH v7,3/3] MIPS: dts: jz4780/ci20: Add NEMC, BCH and NAND device tree nodes

[Ezequiel Garcia]

On 6 October 2015 at 13:27, Harvey Hunt <harvey.hunt@imgtec.com> wrote:
> From: Alex Smith <alex.smith@imgtec.com>
>
> Add device tree nodes for the NEMC and BCH to the JZ4780 device tree,
> and make use of them in the Ci20 device tree to add a node for the
> board's NAND.
>
> Note that since the pinctrl driver is not yet upstream, this includes
> neither pin configuration nor busy/write-protect GPIO pins for the
> NAND. Use of the NAND relies on the boot loader to have left the pins
> configured in a usable state, which should be the case when booted
> from the NAND.
>
> Signed-off-by: Alex Smith <alex.smith@imgtec.com>
> Cc: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Brian Norris <computersforpeace@gmail.com>
> Cc: Paul Burton <paul.burton@imgtec.com>
> Cc: linux-mtd@lists.infradead.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-mips@linux-mips.org
> Cc: Alex Smith <alex@alex-smith.me.uk>
> Signed-off-by: Harvey Hunt <harvey.hunt@imgtec.com>
> ---
> v6 -> v7:
>  - Add nand-ecc-mode to DT.
>  - Add nand-on-flash-bbt to DT.
>
> v4 -> v5:
>  - New patch adding DT nodes for the NAND so that the driver can be
>    tested.
>
>  arch/mips/boot/dts/ingenic/ci20.dts    | 54 ++++++++++++++++++++++++++++++++++
>  arch/mips/boot/dts/ingenic/jz4780.dtsi | 26 ++++++++++++++++
>  2 files changed, 80 insertions(+)
>
> diff --git a/arch/mips/boot/dts/ingenic/ci20.dts b/arch/mips/boot/dts/ingenic/ci20.dts
> index 9fcb9e7..453f1d3 100644
> --- a/arch/mips/boot/dts/ingenic/ci20.dts
> +++ b/arch/mips/boot/dts/ingenic/ci20.dts
> @@ -42,3 +42,57 @@
>  &uart4 {
>         status = "okay";
>  };
> +
> +&nemc {
> +       status = "okay";
> +
> +       nand: nand@1 {
> +               compatible = "ingenic,jz4780-nand";
> +               reg = <1 0 0x1000000>;
> +

Why is this in the ci20.dts instead of the SoC dtsi?

Seems at least compatible and reg is not board-specific.

Thanks,
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar

RE: [PATCH v7,3/3] MIPS: dts: jz4780/ci20: Add NEMC, BCH and NAND device tree nodes

[Harvey Hunt]

On 8 October 2015 at 22:23, Ezequiel Garcia < ezequiel@vanguardiasur.com.ar> wrote:
>On 6 October 2015 at 13:27, Harvey Hunt <harvey.hunt@imgtec.com> wrote:
>> From: Alex Smith <alex.smith@imgtec.com>
>>
>> Add device tree nodes for the NEMC and BCH to the JZ4780 device tree,
>> and make use of them in the Ci20 device tree to add a node for the
>> board's NAND.
>>
>> Note that since the pinctrl driver is not yet upstream, this includes
>> neither pin configuration nor busy/write-protect GPIO pins for the
>> NAND. Use of the NAND relies on the boot loader to have left the pins
>> configured in a usable state, which should be the case when booted
>> from the NAND.
>>
>> Signed-off-by: Alex Smith <alex.smith@imgtec.com>
>> Cc: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
>> Cc: David Woodhouse <dwmw2@infradead.org>
>> Cc: Brian Norris <computersforpeace@gmail.com>
>> Cc: Paul Burton <paul.burton@imgtec.com>
>> Cc: linux-mtd@lists.infradead.org
>> Cc: devicetree@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: linux-mips@linux-mips.org
>> Cc: Alex Smith <alex@alex-smith.me.uk>
>> Signed-off-by: Harvey Hunt <harvey.hunt@imgtec.com>
>> ---
>> v6 -> v7:
>>  - Add nand-ecc-mode to DT.
>>  - Add nand-on-flash-bbt to DT.
>>
>> v4 -> v5:
>>  - New patch adding DT nodes for the NAND so that the driver can be
>>    tested.
>>
>>  arch/mips/boot/dts/ingenic/ci20.dts    | 54 ++++++++++++++++++++++++++++++++++
>>  arch/mips/boot/dts/ingenic/jz4780.dtsi | 26 ++++++++++++++++
>>  2 files changed, 80 insertions(+)
>>
>> diff --git a/arch/mips/boot/dts/ingenic/ci20.dts b/arch/mips/boot/dts/ingenic/ci20.dts
>> index 9fcb9e7..453f1d3 100644
>> --- a/arch/mips/boot/dts/ingenic/ci20.dts
>> +++ b/arch/mips/boot/dts/ingenic/ci20.dts
>> @@ -42,3 +42,57 @@
>>  &uart4 {
>>         status = "okay";
>>  };
>> +
>> +&nemc {
>> +       status = "okay";
>> +
>> +       nand: nand@1 {
>> +               compatible = "ingenic,jz4780-nand";
>> +               reg = <1 0 0x1000000>;
>> +
>
>Why is this in the ci20.dts instead of the SoC dtsi?
>
>Seems at least compatible and reg is not board-specific.
>
>Thanks,
>-- 
>Ezequiel García, VanguardiaSur
>www.vanguardiasur.com.ar

Hi Ezequiel,

The number of NAND nodes under the NEMC node is board specific - some devices
could have 2 NAND banks and others could have none. Including the compatible
property in jz4780.dtsi would imply that all JZ4780 boards have at least one NAND bank.

The size in the reg property would be the same for all NAND devices (as it refers to the
NAND registers), however the bank number would be different, so that can also be seen
as board specific.

Thanks,

Harvey

Re: [PATCH v7,3/3] MIPS: dts: jz4780/ci20: Add NEMC, BCH and NAND device tree nodes

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 4330 bytes --]

Hi Harvey,

On Tue, Oct 06, 2015 at 05:27:17PM +0100, Harvey Hunt wrote:
> From: Alex Smith <alex.smith-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
> 
> Add device tree nodes for the NEMC and BCH to the JZ4780 device tree,
> and make use of them in the Ci20 device tree to add a node for the
> board's NAND.
> 
> Note that since the pinctrl driver is not yet upstream, this includes
> neither pin configuration nor busy/write-protect GPIO pins for the
> NAND. Use of the NAND relies on the boot loader to have left the pins
> configured in a usable state, which should be the case when booted
> from the NAND.
> 
> Signed-off-by: Alex Smith <alex.smith-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
> Cc: Zubair Lutfullah Kakakhel <Zubair.Kakakhel-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
> Cc: David Woodhouse <dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
> Cc: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Paul Burton <paul.burton-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
> Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-mips-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org
> Cc: Alex Smith <alex-oucj9GSTHrKwpo/f3jThPQ@public.gmane.org>
> Signed-off-by: Harvey Hunt <harvey.hunt-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
> ---
> v6 -> v7:
>  - Add nand-ecc-mode to DT.
>  - Add nand-on-flash-bbt to DT.
> 
> v4 -> v5:
>  - New patch adding DT nodes for the NAND so that the driver can be
>    tested.
> 
>  arch/mips/boot/dts/ingenic/ci20.dts    | 54 ++++++++++++++++++++++++++++++++++
>  arch/mips/boot/dts/ingenic/jz4780.dtsi | 26 ++++++++++++++++
>  2 files changed, 80 insertions(+)
> 
> diff --git a/arch/mips/boot/dts/ingenic/ci20.dts b/arch/mips/boot/dts/ingenic/ci20.dts
> index 9fcb9e7..453f1d3 100644
> --- a/arch/mips/boot/dts/ingenic/ci20.dts
> +++ b/arch/mips/boot/dts/ingenic/ci20.dts
> @@ -42,3 +42,57 @@
>  &uart4 {
>  	status = "okay";
>  };
> +
> +&nemc {
> +	status = "okay";
> +
> +	nand: nand@1 {
> +		compatible = "ingenic,jz4780-nand";

Isn't the NAND a micron part? This doesn't seem right. Is the device
driver and binding already accepted upstream with that compatible
string?

Cheers
James

> +		reg = <1 0 0x1000000>;
> +
> +		ingenic,nemc-tAS = <10>;
> +		ingenic,nemc-tAH = <5>;
> +		ingenic,nemc-tBP = <10>;
> +		ingenic,nemc-tAW = <15>;
> +		ingenic,nemc-tSTRV = <100>;
> +
> +		ingenic,bch-controller = <&bch>;
> +		ingenic,ecc-size = <1024>;
> +		ingenic,ecc-strength = <24>;
> +
> +		nand-ecc-mode = "hw";
> +		nand-on-flash-bbt;
> +
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +
> +		partition@0 {
> +			label = "u-boot-spl";
> +			reg = <0x0 0x0 0x0 0x800000>;
> +		};
> +
> +		partition@0x800000 {
> +			label = "u-boot";
> +			reg = <0x0 0x800000 0x0 0x200000>;
> +		};
> +
> +		partition@0xa00000 {
> +			label = "u-boot-env";
> +			reg = <0x0 0xa00000 0x0 0x200000>;
> +		};
> +
> +		partition@0xc00000 {
> +			label = "boot";
> +			reg = <0x0 0xc00000 0x0 0x4000000>;
> +		};
> +
> +		partition@0x8c00000 {
> +			label = "system";
> +			reg = <0x0 0x4c00000 0x1 0xfb400000>;
> +		};
> +	};
> +};
> +
> +&bch {
> +	status = "okay";
> +};
> diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi b/arch/mips/boot/dts/ingenic/jz4780.dtsi
> index 65389f6..b868b42 100644
> --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
> +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
> @@ -108,4 +108,30 @@
>  
>  		status = "disabled";
>  	};
> +
> +	nemc: nemc@13410000 {
> +		compatible = "ingenic,jz4780-nemc";
> +		reg = <0x13410000 0x10000>;
> +		#address-cells = <2>;
> +		#size-cells = <1>;
> +		ranges = <1 0 0x1b000000 0x1000000
> +			  2 0 0x1a000000 0x1000000
> +			  3 0 0x19000000 0x1000000
> +			  4 0 0x18000000 0x1000000
> +			  5 0 0x17000000 0x1000000
> +			  6 0 0x16000000 0x1000000>;
> +
> +		clocks = <&cgu JZ4780_CLK_NEMC>;
> +
> +		status = "disabled";
> +	};
> +
> +	bch: bch@134d0000 {
> +		compatible = "ingenic,jz4780-bch";
> +		reg = <0x134d0000 0x10000>;
> +
> +		clocks = <&cgu JZ4780_CLK_BCH>;
> +
> +		status = "disabled";
> +	};
>  };
> -- 
> 2.6.0
> 
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v7,3/3] MIPS: dts: jz4780/ci20: Add NEMC, BCH and NAND device tree nodes

[Alex Smith]

Hi James,

On 15 October 2015 at 09:47, James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org> wrote:
>> diff --git a/arch/mips/boot/dts/ingenic/ci20.dts b/arch/mips/boot/dts/ingenic/ci20.dts
>> index 9fcb9e7..453f1d3 100644
>> --- a/arch/mips/boot/dts/ingenic/ci20.dts
>> +++ b/arch/mips/boot/dts/ingenic/ci20.dts
>> @@ -42,3 +42,57 @@
>>  &uart4 {
>>       status = "okay";
>>  };
>> +
>> +&nemc {
>> +     status = "okay";
>> +
>> +     nand: nand@1 {
>> +             compatible = "ingenic,jz4780-nand";
>
> Isn't the NAND a micron part? This doesn't seem right. Is the device
> driver and binding already accepted upstream with that compatible
> string?

This is the compatible string for the JZ4780 NAND driver, this patch
is part of the series adding that. Detection of the NAND part is
handled by the MTD subsystem.

Thanks,
Alex
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7,3/3] MIPS: dts: jz4780/ci20: Add NEMC, BCH and NAND device tree nodes

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 1323 bytes --]

Hi Alex,

On Fri, Oct 16, 2015 at 11:11:29AM +0100, Alex Smith wrote:
> Hi James,
> 
> On 15 October 2015 at 09:47, James Hogan <james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org> wrote:
> >> diff --git a/arch/mips/boot/dts/ingenic/ci20.dts b/arch/mips/boot/dts/ingenic/ci20.dts
> >> index 9fcb9e7..453f1d3 100644
> >> --- a/arch/mips/boot/dts/ingenic/ci20.dts
> >> +++ b/arch/mips/boot/dts/ingenic/ci20.dts
> >> @@ -42,3 +42,57 @@
> >>  &uart4 {
> >>       status = "okay";
> >>  };
> >> +
> >> +&nemc {
> >> +     status = "okay";
> >> +
> >> +     nand: nand@1 {
> >> +             compatible = "ingenic,jz4780-nand";
> >
> > Isn't the NAND a micron part? This doesn't seem right. Is the device
> > driver and binding already accepted upstream with that compatible
> > string?
> 
> This is the compatible string for the JZ4780 NAND driver, this patch
> is part of the series adding that. Detection of the NAND part is
> handled by the MTD subsystem.

Right (didn't spot that it was part of a series).

The node appears to describe the NAND interface itself, i.e. a part of
the SoC, so should be in the SoC dtsi file, with overrides in the board
file if necessary for it to work with a particular NAND part
(potentially utilising status="disabled"). Would you agree?

Cheers
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v7,3/3] MIPS: dts: jz4780/ci20: Add NEMC, BCH and NAND device tree nodes

[Paul Burton]

On Fri, Oct 16, 2015 at 11:31:12AM +0100, James Hogan wrote:
> > >> +
> > >> +&nemc {
> > >> +     status = "okay";
> > >> +
> > >> +     nand: nand@1 {
> > >> +             compatible = "ingenic,jz4780-nand";
> > >
> > > Isn't the NAND a micron part? This doesn't seem right. Is the device
> > > driver and binding already accepted upstream with that compatible
> > > string?
> > 
> > This is the compatible string for the JZ4780 NAND driver, this patch
> > is part of the series adding that. Detection of the NAND part is
> > handled by the MTD subsystem.
> 
> Right (didn't spot that it was part of a series).
> 
> The node appears to describe the NAND interface itself, i.e. a part of
> the SoC, so should be in the SoC dtsi file, with overrides in the board
> file if necessary for it to work with a particular NAND part
> (potentially utilising status="disabled"). Would you agree?

Hi James,

The "nemc" node there is for the Nand & External Memory Controller which
is a hardware block inside the SoC. It has 6 banks (ie. 6 chip select
pins, each associated with a different address range, that connect to
different devices). NAND flash is one such possible device, but a board
could connect it to any of the 6 chip selects, or banks. To represent
that in the SoC dtsi you'd want to have 6 NAND nodes, each disabled by
default, which doesn't make a whole lot of sense to me. Other, non-NAND
devices can connect to the NEMC too - for example the ethernet
controller on the CI20 is connected to one bank.

The NAND device nodes are sort of a mix of describing the NAND flash
(ie. Micron part as you point out) and its connections & properties, the
way the NEMC should be used to interact with it alongside the BCH block,
and the configuration for the NEMC such as timing parameters.

I imagine the most semantically correct means of describing it would
probably be for the compatible string to reflect the Micron NAND part,
and the NEMC driver to pick up on the relevant properties of its child
nodes for configuring timings, whether the device is NAND etc. However
the handling of registering NAND devices with MTD would probably then
have to be part of the NEMC driver, which feels a bit off too.

Thanks,
    Paul

Re: [PATCH v7,3/3] MIPS: dts: jz4780/ci20: Add NEMC, BCH and NAND device tree nodes

[Boris Brezillon]

Paul, Harvey,

On Fri, 16 Oct 2015 11:48:48 +0100
Paul Burton <paul.burton-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org> wrote:

> On Fri, Oct 16, 2015 at 11:31:12AM +0100, James Hogan wrote:
> > > >> +
> > > >> +&nemc {
> > > >> +     status = "okay";
> > > >> +
> > > >> +     nand: nand@1 {
> > > >> +             compatible = "ingenic,jz4780-nand";
> > > >
> > > > Isn't the NAND a micron part? This doesn't seem right. Is the device
> > > > driver and binding already accepted upstream with that compatible
> > > > string?
> > > 
> > > This is the compatible string for the JZ4780 NAND driver, this patch
> > > is part of the series adding that. Detection of the NAND part is
> > > handled by the MTD subsystem.
> > 
> > Right (didn't spot that it was part of a series).
> > 
> > The node appears to describe the NAND interface itself, i.e. a part of
> > the SoC, so should be in the SoC dtsi file, with overrides in the board
> > file if necessary for it to work with a particular NAND part
> > (potentially utilising status="disabled"). Would you agree?
> 
> Hi James,
> 
> The "nemc" node there is for the Nand & External Memory Controller which
> is a hardware block inside the SoC. It has 6 banks (ie. 6 chip select
> pins, each associated with a different address range, that connect to
> different devices). NAND flash is one such possible device, but a board
> could connect it to any of the 6 chip selects, or banks. To represent
> that in the SoC dtsi you'd want to have 6 NAND nodes, each disabled by
> default, which doesn't make a whole lot of sense to me. Other, non-NAND
> devices can connect to the NEMC too - for example the ethernet
> controller on the CI20 is connected to one bank.
> 
> The NAND device nodes are sort of a mix of describing the NAND flash
> (ie. Micron part as you point out) and its connections & properties, the
> way the NEMC should be used to interact with it alongside the BCH block,
> and the configuration for the NEMC such as timing parameters.
> 
> I imagine the most semantically correct means of describing it would
> probably be for the compatible string to reflect the Micron NAND part,
> and the NEMC driver to pick up on the relevant properties of its child
> nodes for configuring timings, whether the device is NAND etc. However
> the handling of registering NAND devices with MTD would probably then
> have to be part of the NEMC driver, which feels a bit off too.

Another solution would be to describe both the NAND controller and the
NAND chip in the DT (with the NAND chip being a chip of the NAND
controller).
Actually this is already what other binding are doing [1][2]. I know
those bindings are representing NAND controllers which can interface
with more than one NAND chip, but I think that even in the 1:1 case it
would make it clearer to represent both the NAND chip and the NAND
controller.

In your case this would give the following representation

+&nemc {
+	status = "okay";
+
+	nandc: nand-controller@1 {
+		compatible = "ingenic,jz4780-nand";
+		reg = <1 0 0x1000000>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ingenic,bch-controller = <&bch>;
+
+		nand@0 {
+			nand-ecc-mode = "hw";
+			nand-on-flash-bbt;
+			nand-ecc-size = <1024>;
+			nand-ecc-strength = <24>;
+
+			#address-cells = <2>;
+			#size-cells = <2>;
+
+			partition@0 {
+				label = "u-boot-spl";
+				reg = <0x0 0x0 0x0 0x800000>;
+			};
+			/* ... */
+
+		};
+	};
+};

Best Regards,

Boris

[1]http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt#L119
[2]http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/mtd/sunxi-nand.txt#L28

> 
> Thanks,
>     Paul
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7,1/3] dt-bindings: binding for jz4780-{nand,bch}

[Boris Brezillon]

Hi Harvey,

On Tue, 6 Oct 2015 17:27:15 +0100
Harvey Hunt <harvey.hunt@imgtec.com> wrote:

> From: Alex Smith <alex.smith@imgtec.com>
> 
> Add DT bindings for NAND devices connected to the NEMC on JZ4780 SoCs,
> as well as the hardware BCH controller, used by the jz4780_{nand,bch}
> drivers.

Patch 3 does not seem to follow this binding: it still uses the
ingenic,ecc-xxx properties and defines NAND timings.

Also, as answered to patch 3, I think it would be clearer to separate
the nand controller and nand chip representation.

Best Regards,

Boris

> 
> Signed-off-by: Alex Smith <alex.smith@imgtec.com>
> Cc: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Brian Norris <computersforpeace@gmail.com>
> Cc: linux-mtd@lists.infradead.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: Alex Smith <alex@alex-smith.me.uk>
> Signed-off-by: Harvey Hunt <harvey.hunt@imgtec.com>
> ---
> v6 -> v7:
>  - Add nand-ecc-mode to DT bindings.
>  - Add nand-on-flash-bbt to DT bindings.
> 
> v5 -> v6:
>  - No change.
> 
> v4 -> v5:
>  - Rename ingenic,bch-device to ingenic,bch-controller to fit with
>    existing convention.
> 
> v3 -> v4:
>  - No change
> 
> v2 -> v3:
>  - Rebase to 4.0-rc6
>  - Changed ingenic,ecc-size to common nand-ecc-step-size
>  - Changed ingenic,ecc-strength to common nand-ecc-strength
>  - Changed ingenic,busy-gpio to common rb-gpios
>  - Changed ingenic,wp-gpio to common wp-gpios
> 
> v1 -> v2:
>  - Rebase to 4.0-rc3
> 
>  .../bindings/mtd/ingenic,jz4780-nand.txt           | 61 ++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
> 
> diff --git a/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
> new file mode 100644
> index 0000000..44a0468
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt
> @@ -0,0 +1,61 @@
> +* Ingenic JZ4780 NAND/BCH
> +
> +This file documents the device tree bindings for NAND flash devices on the
> +JZ4780. NAND devices are connected to the NEMC controller (described in
> +memory-controllers/ingenic,jz4780-nemc.txt), and thus NAND device nodes must
> +be children of the NEMC node.
> +
> +Required NAND device properties:
> +- compatible: Should be set to "ingenic,jz4780-nand".
> +- reg: For each bank with a NAND chip attached, should specify a bank number,
> +  an offset of 0 and a size of 0x1000000 (i.e. the whole NEMC bank).
> +
> +Optional NAND device properties:
> +- ingenic,bch-controller: To make use of the hardware BCH controller, this
> +  property must contain a phandle for the BCH controller node. The required
> +  properties for this node are described below. If this is not specified,
> +  software BCH will be used instead.
> +- nand-ecc-step-size: ECC block size in bytes.
> +- nand-ecc-strength: ECC strength (max number of correctable bits).
> +- nand-ecc-mode: String, operation mode of the NAND ecc mode. "hw" by default
> +- nand-on-flash-bbt: boolean to enable on flash bbt option if not present false
> +- rb-gpios: GPIO specifier for the busy pin.
> +- wp-gpios: GPIO specifier for the write protect pin.
> +
> +Example:
> +
> +nemc: nemc@13410000 {
> +	...
> +
> +	nand: nand@1 {
> +		compatible = "ingenic,jz4780-nand";
> +		reg = <1 0 0x1000000>;	/* Bank 1 */
> +
> +		ingenic,bch-controller = <&bch>;
> +		nand-ecc-step-size = <1024>;
> +		nand-ecc-strength = <24>;
> +		nand-ecc-mode = "hw";
> +		nand-on-flash-bbt;
> +
> +		rb-gpios = <&gpa 20 GPIO_ACTIVE_LOW>;
> +		wp-gpios = <&gpf 22 GPIO_ACTIVE_LOW>;
> +	};
> +};
> +
> +The BCH controller is a separate SoC component used for error correction on
> +NAND devices. The following is a description of the device properties for a
> +BCH controller.
> +
> +Required BCH properties:
> +- compatible: Should be set to "ingenic,jz4780-bch".
> +- reg: Should specify the BCH controller registers location and length.
> +- clocks: Clock for the BCH controller.
> +
> +Example:
> +
> +bch: bch@134d0000 {
> +	compatible = "ingenic,jz4780-bch";
> +	reg = <0x134d0000 0x10000>;
> +
> +	clocks = <&cgu JZ4780_CLK_BCH>;
> +};



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH v7,1/3] dt-bindings: binding for jz4780-{nand,bch}

[Harvey Hunt]

Hi Boris,

On 04/11/15 07:57, Boris Brezillon wrote:
> Hi Harvey,
>
> On Tue, 6 Oct 2015 17:27:15 +0100
> Harvey Hunt <harvey.hunt@imgtec.com> wrote:
>
>> From: Alex Smith <alex.smith@imgtec.com>
>>
>> Add DT bindings for NAND devices connected to the NEMC on JZ4780 SoCs,
>> as well as the hardware BCH controller, used by the jz4780_{nand,bch}
>> drivers.
>
> Patch 3 does not seem to follow this binding: it still uses the
> ingenic,ecc-xxx properties and defines NAND timings.

I'll fix this for v8.

> Also, as answered to patch 3, I think it would be clearer to separate
> the nand controller and nand chip representation.

I agree - I'll implement this in the next version.

> Best Regards,
>
> Boris

Thanks,

Harvey Hunt

Re: [PATCH v7,1/3] dt-bindings: binding for jz4780-{nand,bch}

[Harvey Hunt]

Hi Boris,

On 04/11/15 07:57, Boris Brezillon wrote:
> Hi Harvey,
>
> On Tue, 6 Oct 2015 17:27:15 +0100
> Harvey Hunt <harvey.hunt@imgtec.com> wrote:
>
>> From: Alex Smith <alex.smith@imgtec.com>
>>
>> Add DT bindings for NAND devices connected to the NEMC on JZ4780 SoCs,
>> as well as the hardware BCH controller, used by the jz4780_{nand,bch}
>> drivers.
>
> Patch 3 does not seem to follow this binding: it still uses the
> ingenic,ecc-xxx properties and defines NAND timings.

Thanks, I'll fix that for v8.

> Also, as answered to patch 3, I think it would be clearer to separate
> the nand controller and nand chip representation.

That approach makes sense - I'll change it for the next series.

> Best Regards,
>
> Boris

Best regards,

Harvey Hunt

Re: [PATCH v7,3/3] MIPS: dts: jz4780/ci20: Add NEMC, BCH and NAND device tree nodes

[Harvey Hunt]

Hi Boris,

On 04/11/15 07:51, Boris Brezillon wrote:
> Paul, Harvey,
>
> On Fri, 16 Oct 2015 11:48:48 +0100
> Paul Burton <paul.burton@imgtec.com> wrote:
>
>> On Fri, Oct 16, 2015 at 11:31:12AM +0100, James Hogan wrote:
>>>>>> +
>>>>>> +&nemc {
>>>>>> +     status = "okay";
>>>>>> +
>>>>>> +     nand: nand@1 {
>>>>>> +             compatible = "ingenic,jz4780-nand";
>>>>>
>>>>> Isn't the NAND a micron part? This doesn't seem right. Is the device
>>>>> driver and binding already accepted upstream with that compatible
>>>>> string?
>>>>
>>>> This is the compatible string for the JZ4780 NAND driver, this patch
>>>> is part of the series adding that. Detection of the NAND part is
>>>> handled by the MTD subsystem.
>>>
>>> Right (didn't spot that it was part of a series).
>>>
>>> The node appears to describe the NAND interface itself, i.e. a part of
>>> the SoC, so should be in the SoC dtsi file, with overrides in the board
>>> file if necessary for it to work with a particular NAND part
>>> (potentially utilising status="disabled"). Would you agree?
>>
>> Hi James,
>>
>> The "nemc" node there is for the Nand & External Memory Controller which
>> is a hardware block inside the SoC. It has 6 banks (ie. 6 chip select
>> pins, each associated with a different address range, that connect to
>> different devices). NAND flash is one such possible device, but a board
>> could connect it to any of the 6 chip selects, or banks. To represent
>> that in the SoC dtsi you'd want to have 6 NAND nodes, each disabled by
>> default, which doesn't make a whole lot of sense to me. Other, non-NAND
>> devices can connect to the NEMC too - for example the ethernet
>> controller on the CI20 is connected to one bank.
>>
>> The NAND device nodes are sort of a mix of describing the NAND flash
>> (ie. Micron part as you point out) and its connections & properties, the
>> way the NEMC should be used to interact with it alongside the BCH block,
>> and the configuration for the NEMC such as timing parameters.
>>
>> I imagine the most semantically correct means of describing it would
>> probably be for the compatible string to reflect the Micron NAND part,
>> and the NEMC driver to pick up on the relevant properties of its child
>> nodes for configuring timings, whether the device is NAND etc. However
>> the handling of registering NAND devices with MTD would probably then
>> have to be part of the NEMC driver, which feels a bit off too.
>
> Another solution would be to describe both the NAND controller and the
> NAND chip in the DT (with the NAND chip being a chip of the NAND
> controller).
> Actually this is already what other binding are doing [1][2]. I know
> those bindings are representing NAND controllers which can interface
> with more than one NAND chip, but I think that even in the 1:1 case it
> would make it clearer to represent both the NAND chip and the NAND
> controller.
>
> In your case this would give the following representation
>
> +&nemc {
> +	status = "okay";
> +
> +	nandc: nand-controller@1 {
> +		compatible = "ingenic,jz4780-nand";
> +		reg = <1 0 0x1000000>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		ingenic,bch-controller = <&bch>;
> +
> +		nand@0 {
> +			nand-ecc-mode = "hw";
> +			nand-on-flash-bbt;
> +			nand-ecc-size = <1024>;
> +			nand-ecc-strength = <24>;
> +
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +
> +			partition@0 {
> +				label = "u-boot-spl";
> +				reg = <0x0 0x0 0x0 0x800000>;
> +			};
> +			/* ... */
> +
> +		};
> +	};
> +};

I'll implement this in v8 - thanks for the example DT. :-)

> Best Regards,
>
> Boris
>
> [1]http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt#L119
> [2]http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/mtd/sunxi-nand.txt#L28
>
>>
>> Thanks,
>>      Paul
>>
>> ______________________________________________________
>> Linux MTD discussion mailing list
>> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>
>
>

Best regards,

Harvey Hunt