diff mbox series

[v3,3/6] mtd: rawnand: tegra: add devicetree binding

Message ID 20180531221637.6017-4-stefan@agner.ch
State Superseded
Headers show
Series mtd: rawnand: add NVIDIA Tegra NAND flash support | expand

Commit Message

Stefan Agner May 31, 2018, 10:16 p.m. UTC
This adds the devicetree binding for the Tegra 2 NAND flash
controller.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 .../bindings/mtd/nvidia-tegra20-nand.txt      | 64 +++++++++++++++++++
 1 file changed, 64 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt

Comments

Boris Brezillon June 1, 2018, 7:30 a.m. UTC | #1
On Fri,  1 Jun 2018 00:16:34 +0200
Stefan Agner <stefan@agner.ch> wrote:

> This adds the devicetree binding for the Tegra 2 NAND flash
> controller.
> 
> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  .../bindings/mtd/nvidia-tegra20-nand.txt      | 64 +++++++++++++++++++
>  1 file changed, 64 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> 
> diff --git a/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> new file mode 100644
> index 000000000000..5cd984ef046b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> @@ -0,0 +1,64 @@
> +NVIDIA Tegra NAND Flash controller
> +
> +Required properties:
> +- compatible: Must be one of:
> +  - "nvidia,tegra20-nand"

As discussed previously, I prefer "nvidia,tegra20-nand-controller" or
"nvidia,tegra20-nfc".

> +- reg: MMIO address range
> +- interrupts: interrupt output of the NFC controller
> +- clocks: Must contain an entry for each entry in clock-names.
> +  See ../clocks/clock-bindings.txt for details.
> +- clock-names: Must include the following entries:
> +  - nand
> +- resets: Must contain an entry for each entry in reset-names.
> +  See ../reset/reset.txt for details.
> +- reset-names: Must include the following entries:
> +  - nand
> +
> +Optional children nodes:
> +Individual NAND chips are children of the NAND controller node. Currently
> +only one NAND chip supported.
> +
> +Required children node properties:
> +- reg: An integer ranging from 1 to 6 representing the CS line to use.
> +
> +Optional children node properties:
> +- nand-ecc-mode: String, operation mode of the NAND ecc mode. Currently only
> +		 "hw" is supported.
> +- nand-ecc-algo: string, algorithm of NAND ECC.
> +		 Supported values with "hw" ECC mode are: "rs", "bch".
> +- nand-bus-width : See nand.txt
> +- nand-on-flash-bbt: See nand.txt
> +- nand-ecc-strength: integer representing the number of bits to correct
> +		     per ECC step (always 512). Supported strength using HW ECC
> +		     modes are:
> +		     - RS: 4, 6, 8
> +		     - BCH: 4, 8, 14, 16
> +- nand-ecc-maximize: See nand.txt
> +- nand-is-boot-medium: Makes sure only ECC strengths supported by the boot ROM
> +		       are choosen.
> +- wp-gpios: GPIO specifier for the write protect pin.
> +
> +Optional child node of NAND chip nodes:
> +Partitions: see partition.txt
> +
> +  Example:
> +	nand@70008000 {

	nand-controller@70008000 {

> +		compatible = "nvidia,tegra20-nand";

		compatible = "nvidia,tegra20-nand-controller";

or

		compatible = "nvidia,tegra20-nfc";

> +		reg = <0x70008000 0x100>;
> +		interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&tegra_car TEGRA20_CLK_NDFLASH>;
> +		clock-names = "nand";
> +		resets = <&tegra_car 13>;
> +		reset-names = "nand";
> +
> +		nand-chip@0 {

		nand@0 {

> +			reg = <0>;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			nand-bus-width = <8>;
> +			nand-on-flash-bbt;
> +			nand-ecc-algo = "bch";
> +			nand-ecc-strength = <8>;
> +			wp-gpios = <&gpio TEGRA_GPIO(S, 0) GPIO_ACTIVE_LOW>;
> +		};
> +	};

With this addressed,

Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring June 5, 2018, 8:13 p.m. UTC | #2
On Fri, Jun 01, 2018 at 12:16:34AM +0200, Stefan Agner wrote:
> This adds the devicetree binding for the Tegra 2 NAND flash
> controller.
> 
> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  .../bindings/mtd/nvidia-tegra20-nand.txt      | 64 +++++++++++++++++++
>  1 file changed, 64 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt

Reviewed-by: Rob Herring <robh@kernel.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dmitry Osipenko June 5, 2018, 8:19 p.m. UTC | #3
On 01.06.2018 10:30, Boris Brezillon wrote:
> On Fri,  1 Jun 2018 00:16:34 +0200
> Stefan Agner <stefan@agner.ch> wrote:
> 
>> This adds the devicetree binding for the Tegra 2 NAND flash
>> controller.
>>
>> Signed-off-by: Lucas Stach <dev@lynxeye.de>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>>  .../bindings/mtd/nvidia-tegra20-nand.txt      | 64 +++++++++++++++++++
>>  1 file changed, 64 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
>> new file mode 100644
>> index 000000000000..5cd984ef046b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
>> @@ -0,0 +1,64 @@
>> +NVIDIA Tegra NAND Flash controller
>> +
>> +Required properties:
>> +- compatible: Must be one of:
>> +  - "nvidia,tegra20-nand"
> 
> As discussed previously, I prefer "nvidia,tegra20-nand-controller" or
> "nvidia,tegra20-nfc".
> 
>> +- reg: MMIO address range
>> +- interrupts: interrupt output of the NFC controller
>> +- clocks: Must contain an entry for each entry in clock-names.
>> +  See ../clocks/clock-bindings.txt for details.
>> +- clock-names: Must include the following entries:
>> +  - nand
>> +- resets: Must contain an entry for each entry in reset-names.
>> +  See ../reset/reset.txt for details.
>> +- reset-names: Must include the following entries:
>> +  - nand
>> +
>> +Optional children nodes:
>> +Individual NAND chips are children of the NAND controller node. Currently
>> +only one NAND chip supported.
>> +
>> +Required children node properties:
>> +- reg: An integer ranging from 1 to 6 representing the CS line to use.
>> +
>> +Optional children node properties:
>> +- nand-ecc-mode: String, operation mode of the NAND ecc mode. Currently only
>> +		 "hw" is supported.
>> +- nand-ecc-algo: string, algorithm of NAND ECC.
>> +		 Supported values with "hw" ECC mode are: "rs", "bch".
>> +- nand-bus-width : See nand.txt
>> +- nand-on-flash-bbt: See nand.txt
>> +- nand-ecc-strength: integer representing the number of bits to correct
>> +		     per ECC step (always 512). Supported strength using HW ECC
>> +		     modes are:
>> +		     - RS: 4, 6, 8
>> +		     - BCH: 4, 8, 14, 16
>> +- nand-ecc-maximize: See nand.txt
>> +- nand-is-boot-medium: Makes sure only ECC strengths supported by the boot ROM
>> +		       are choosen.
>> +- wp-gpios: GPIO specifier for the write protect pin.
>> +
>> +Optional child node of NAND chip nodes:
>> +Partitions: see partition.txt
>> +
>> +  Example:
>> +	nand@70008000 {
> 
> 	nand-controller@70008000 {
> 
>> +		compatible = "nvidia,tegra20-nand";
> 
> 		compatible = "nvidia,tegra20-nand-controller";
> 
> or
> 
> 		compatible = "nvidia,tegra20-nfc";
> 

Maybe it's just me, but when I'm reading "nfc", my first association is the
"Near Field Communication". Probably an explicit
"nvidia,tegra20-nand-controller" variant is more preferable.

>> +		reg = <0x70008000 0x100>;
>> +		interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
>> +		clocks = <&tegra_car TEGRA20_CLK_NDFLASH>;
>> +		clock-names = "nand";
>> +		resets = <&tegra_car 13>;
>> +		reset-names = "nand";
>> +
>> +		nand-chip@0 {
> 
> 		nand@0 {
> 
>> +			reg = <0>;
>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
>> +			nand-bus-width = <8>;
>> +			nand-on-flash-bbt;
>> +			nand-ecc-algo = "bch";
>> +			nand-ecc-strength = <8>;
>> +			wp-gpios = <&gpio TEGRA_GPIO(S, 0) GPIO_ACTIVE_LOW>;
>> +		};
>> +	};
> 
> With this addressed,
> 
> Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thierry Reding June 6, 2018, 10:39 a.m. UTC | #4
On Tue, Jun 05, 2018 at 11:19:14PM +0300, Dmitry Osipenko wrote:
> On 01.06.2018 10:30, Boris Brezillon wrote:
> > On Fri,  1 Jun 2018 00:16:34 +0200
> > Stefan Agner <stefan@agner.ch> wrote:
> > 
> >> This adds the devicetree binding for the Tegra 2 NAND flash
> >> controller.
> >>
> >> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> >> Signed-off-by: Stefan Agner <stefan@agner.ch>
> >> ---
> >>  .../bindings/mtd/nvidia-tegra20-nand.txt      | 64 +++++++++++++++++++
> >>  1 file changed, 64 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> >> new file mode 100644
> >> index 000000000000..5cd984ef046b
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> >> @@ -0,0 +1,64 @@
> >> +NVIDIA Tegra NAND Flash controller
> >> +
> >> +Required properties:
> >> +- compatible: Must be one of:
> >> +  - "nvidia,tegra20-nand"
> > 
> > As discussed previously, I prefer "nvidia,tegra20-nand-controller" or
> > "nvidia,tegra20-nfc".
> > 
> >> +- reg: MMIO address range
> >> +- interrupts: interrupt output of the NFC controller
> >> +- clocks: Must contain an entry for each entry in clock-names.
> >> +  See ../clocks/clock-bindings.txt for details.
> >> +- clock-names: Must include the following entries:
> >> +  - nand
> >> +- resets: Must contain an entry for each entry in reset-names.
> >> +  See ../reset/reset.txt for details.
> >> +- reset-names: Must include the following entries:
> >> +  - nand
> >> +
> >> +Optional children nodes:
> >> +Individual NAND chips are children of the NAND controller node. Currently
> >> +only one NAND chip supported.
> >> +
> >> +Required children node properties:
> >> +- reg: An integer ranging from 1 to 6 representing the CS line to use.
> >> +
> >> +Optional children node properties:
> >> +- nand-ecc-mode: String, operation mode of the NAND ecc mode. Currently only
> >> +		 "hw" is supported.
> >> +- nand-ecc-algo: string, algorithm of NAND ECC.
> >> +		 Supported values with "hw" ECC mode are: "rs", "bch".
> >> +- nand-bus-width : See nand.txt
> >> +- nand-on-flash-bbt: See nand.txt
> >> +- nand-ecc-strength: integer representing the number of bits to correct
> >> +		     per ECC step (always 512). Supported strength using HW ECC
> >> +		     modes are:
> >> +		     - RS: 4, 6, 8
> >> +		     - BCH: 4, 8, 14, 16
> >> +- nand-ecc-maximize: See nand.txt
> >> +- nand-is-boot-medium: Makes sure only ECC strengths supported by the boot ROM
> >> +		       are choosen.
> >> +- wp-gpios: GPIO specifier for the write protect pin.
> >> +
> >> +Optional child node of NAND chip nodes:
> >> +Partitions: see partition.txt
> >> +
> >> +  Example:
> >> +	nand@70008000 {
> > 
> > 	nand-controller@70008000 {
> > 
> >> +		compatible = "nvidia,tegra20-nand";
> > 
> > 		compatible = "nvidia,tegra20-nand-controller";
> > 
> > or
> > 
> > 		compatible = "nvidia,tegra20-nfc";
> > 
> 
> Maybe it's just me, but when I'm reading "nfc", my first association is the
> "Near Field Communication". Probably an explicit
> "nvidia,tegra20-nand-controller" variant is more preferable.

We don't really use a -controller suffix for any of the other
controllers because it is kind of implied. "nfc" is also not something
that is ever referred to in the technical documentation.

"nvidia,tegra20-nand" would be most consistent with all the rest of
Tegra (c.f. "nvidia,tegra*-ahci", "nvidia,tegra*-pci",
"nvidia,tegra*-hda", "nvidia,tegra*-gmi", ...).

Thierry
Boris Brezillon June 6, 2018, 10:45 a.m. UTC | #5
Hi Thierry,

On Wed, 6 Jun 2018 12:39:03 +0200
Thierry Reding <thierry.reding@gmail.com> wrote:

> On Tue, Jun 05, 2018 at 11:19:14PM +0300, Dmitry Osipenko wrote:
> > On 01.06.2018 10:30, Boris Brezillon wrote:  
> > > On Fri,  1 Jun 2018 00:16:34 +0200
> > > Stefan Agner <stefan@agner.ch> wrote:
> > >   
> > >> This adds the devicetree binding for the Tegra 2 NAND flash
> > >> controller.
> > >>
> > >> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> > >> Signed-off-by: Stefan Agner <stefan@agner.ch>
> > >> ---
> > >>  .../bindings/mtd/nvidia-tegra20-nand.txt      | 64 +++++++++++++++++++
> > >>  1 file changed, 64 insertions(+)
> > >>  create mode 100644 Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> > >>
> > >> diff --git a/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> > >> new file mode 100644
> > >> index 000000000000..5cd984ef046b
> > >> --- /dev/null
> > >> +++ b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> > >> @@ -0,0 +1,64 @@
> > >> +NVIDIA Tegra NAND Flash controller
> > >> +
> > >> +Required properties:
> > >> +- compatible: Must be one of:
> > >> +  - "nvidia,tegra20-nand"  
> > > 
> > > As discussed previously, I prefer "nvidia,tegra20-nand-controller" or
> > > "nvidia,tegra20-nfc".
> > >   
> > >> +- reg: MMIO address range
> > >> +- interrupts: interrupt output of the NFC controller
> > >> +- clocks: Must contain an entry for each entry in clock-names.
> > >> +  See ../clocks/clock-bindings.txt for details.
> > >> +- clock-names: Must include the following entries:
> > >> +  - nand
> > >> +- resets: Must contain an entry for each entry in reset-names.
> > >> +  See ../reset/reset.txt for details.
> > >> +- reset-names: Must include the following entries:
> > >> +  - nand
> > >> +
> > >> +Optional children nodes:
> > >> +Individual NAND chips are children of the NAND controller node. Currently
> > >> +only one NAND chip supported.
> > >> +
> > >> +Required children node properties:
> > >> +- reg: An integer ranging from 1 to 6 representing the CS line to use.
> > >> +
> > >> +Optional children node properties:
> > >> +- nand-ecc-mode: String, operation mode of the NAND ecc mode. Currently only
> > >> +		 "hw" is supported.
> > >> +- nand-ecc-algo: string, algorithm of NAND ECC.
> > >> +		 Supported values with "hw" ECC mode are: "rs", "bch".
> > >> +- nand-bus-width : See nand.txt
> > >> +- nand-on-flash-bbt: See nand.txt
> > >> +- nand-ecc-strength: integer representing the number of bits to correct
> > >> +		     per ECC step (always 512). Supported strength using HW ECC
> > >> +		     modes are:
> > >> +		     - RS: 4, 6, 8
> > >> +		     - BCH: 4, 8, 14, 16
> > >> +- nand-ecc-maximize: See nand.txt
> > >> +- nand-is-boot-medium: Makes sure only ECC strengths supported by the boot ROM
> > >> +		       are choosen.
> > >> +- wp-gpios: GPIO specifier for the write protect pin.
> > >> +
> > >> +Optional child node of NAND chip nodes:
> > >> +Partitions: see partition.txt
> > >> +
> > >> +  Example:
> > >> +	nand@70008000 {  
> > > 
> > > 	nand-controller@70008000 {
> > >   
> > >> +		compatible = "nvidia,tegra20-nand";  
> > > 
> > > 		compatible = "nvidia,tegra20-nand-controller";
> > > 
> > > or
> > > 
> > > 		compatible = "nvidia,tegra20-nfc";
> > >   
> > 
> > Maybe it's just me, but when I'm reading "nfc", my first association is the
> > "Near Field Communication". Probably an explicit
> > "nvidia,tegra20-nand-controller" variant is more preferable.  

I also prefer nvidia,tegra20-nand-controller.

> 
> We don't really use a -controller suffix for any of the other
> controllers because it is kind of implied. "nfc" is also not something
> that is ever referred to in the technical documentation.
> 
> "nvidia,tegra20-nand" would be most consistent with all the rest of
> Tegra (c.f. "nvidia,tegra*-ahci", "nvidia,tegra*-pci",
> "nvidia,tegra*-hda", "nvidia,tegra*-gmi", ...).

People get confused about what this node represents when you just have
"nvidia,tegra20-nand", and then you start seeing NAND related props or
partition nodes being defined under the NAND controller node.
I really prefer to have the "-controller" prefix here to avoid such
confusions.

Regards,

Boris
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thierry Reding June 6, 2018, 11:07 a.m. UTC | #6
On Wed, Jun 06, 2018 at 12:45:40PM +0200, Boris Brezillon wrote:
> Hi Thierry,
> 
> On Wed, 6 Jun 2018 12:39:03 +0200
> Thierry Reding <thierry.reding@gmail.com> wrote:
> 
> > On Tue, Jun 05, 2018 at 11:19:14PM +0300, Dmitry Osipenko wrote:
> > > On 01.06.2018 10:30, Boris Brezillon wrote:  
> > > > On Fri,  1 Jun 2018 00:16:34 +0200
> > > > Stefan Agner <stefan@agner.ch> wrote:
> > > >   
> > > >> This adds the devicetree binding for the Tegra 2 NAND flash
> > > >> controller.
> > > >>
> > > >> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> > > >> Signed-off-by: Stefan Agner <stefan@agner.ch>
> > > >> ---
> > > >>  .../bindings/mtd/nvidia-tegra20-nand.txt      | 64 +++++++++++++++++++
> > > >>  1 file changed, 64 insertions(+)
> > > >>  create mode 100644 Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> > > >>
> > > >> diff --git a/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> > > >> new file mode 100644
> > > >> index 000000000000..5cd984ef046b
> > > >> --- /dev/null
> > > >> +++ b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> > > >> @@ -0,0 +1,64 @@
> > > >> +NVIDIA Tegra NAND Flash controller
> > > >> +
> > > >> +Required properties:
> > > >> +- compatible: Must be one of:
> > > >> +  - "nvidia,tegra20-nand"  
> > > > 
> > > > As discussed previously, I prefer "nvidia,tegra20-nand-controller" or
> > > > "nvidia,tegra20-nfc".
> > > >   
> > > >> +- reg: MMIO address range
> > > >> +- interrupts: interrupt output of the NFC controller
> > > >> +- clocks: Must contain an entry for each entry in clock-names.
> > > >> +  See ../clocks/clock-bindings.txt for details.
> > > >> +- clock-names: Must include the following entries:
> > > >> +  - nand
> > > >> +- resets: Must contain an entry for each entry in reset-names.
> > > >> +  See ../reset/reset.txt for details.
> > > >> +- reset-names: Must include the following entries:
> > > >> +  - nand
> > > >> +
> > > >> +Optional children nodes:
> > > >> +Individual NAND chips are children of the NAND controller node. Currently
> > > >> +only one NAND chip supported.
> > > >> +
> > > >> +Required children node properties:
> > > >> +- reg: An integer ranging from 1 to 6 representing the CS line to use.
> > > >> +
> > > >> +Optional children node properties:
> > > >> +- nand-ecc-mode: String, operation mode of the NAND ecc mode. Currently only
> > > >> +		 "hw" is supported.
> > > >> +- nand-ecc-algo: string, algorithm of NAND ECC.
> > > >> +		 Supported values with "hw" ECC mode are: "rs", "bch".
> > > >> +- nand-bus-width : See nand.txt
> > > >> +- nand-on-flash-bbt: See nand.txt
> > > >> +- nand-ecc-strength: integer representing the number of bits to correct
> > > >> +		     per ECC step (always 512). Supported strength using HW ECC
> > > >> +		     modes are:
> > > >> +		     - RS: 4, 6, 8
> > > >> +		     - BCH: 4, 8, 14, 16
> > > >> +- nand-ecc-maximize: See nand.txt
> > > >> +- nand-is-boot-medium: Makes sure only ECC strengths supported by the boot ROM
> > > >> +		       are choosen.
> > > >> +- wp-gpios: GPIO specifier for the write protect pin.
> > > >> +
> > > >> +Optional child node of NAND chip nodes:
> > > >> +Partitions: see partition.txt
> > > >> +
> > > >> +  Example:
> > > >> +	nand@70008000 {  
> > > > 
> > > > 	nand-controller@70008000 {
> > > >   
> > > >> +		compatible = "nvidia,tegra20-nand";  
> > > > 
> > > > 		compatible = "nvidia,tegra20-nand-controller";
> > > > 
> > > > or
> > > > 
> > > > 		compatible = "nvidia,tegra20-nfc";
> > > >   
> > > 
> > > Maybe it's just me, but when I'm reading "nfc", my first association is the
> > > "Near Field Communication". Probably an explicit
> > > "nvidia,tegra20-nand-controller" variant is more preferable.  
> 
> I also prefer nvidia,tegra20-nand-controller.
> 
> > 
> > We don't really use a -controller suffix for any of the other
> > controllers because it is kind of implied. "nfc" is also not something
> > that is ever referred to in the technical documentation.
> > 
> > "nvidia,tegra20-nand" would be most consistent with all the rest of
> > Tegra (c.f. "nvidia,tegra*-ahci", "nvidia,tegra*-pci",
> > "nvidia,tegra*-hda", "nvidia,tegra*-gmi", ...).
> 
> People get confused about what this node represents when you just have
> "nvidia,tegra20-nand", and then you start seeing NAND related props or
> partition nodes being defined under the NAND controller node.
> I really prefer to have the "-controller" prefix here to avoid such
> confusions.

Hmm... odd. I mean, the node is already called nand-controller@...,
which makes it pretty obvious to me that this represents a controller
rather than a NAND chip. Also, the placement of this in the DT hierarchy
should make it pretty obvious that it is a controller since you can't
just put a NAND chip directly on the CPU's address bus.

In addition I think the nvidia,tegra* part already pretty strongly
suggests that this is part of an SoC, so further implies "controller".

Thierry
Stefan Agner June 6, 2018, 12:14 p.m. UTC | #7
On 06.06.2018 13:07, Thierry Reding wrote:
> On Wed, Jun 06, 2018 at 12:45:40PM +0200, Boris Brezillon wrote:
>> Hi Thierry,
>>
>> On Wed, 6 Jun 2018 12:39:03 +0200
>> Thierry Reding <thierry.reding@gmail.com> wrote:
>>
>> > On Tue, Jun 05, 2018 at 11:19:14PM +0300, Dmitry Osipenko wrote:
>> > > On 01.06.2018 10:30, Boris Brezillon wrote:
>> > > > On Fri,  1 Jun 2018 00:16:34 +0200
>> > > > Stefan Agner <stefan@agner.ch> wrote:
>> > > >
>> > > >> This adds the devicetree binding for the Tegra 2 NAND flash
>> > > >> controller.
>> > > >>
>> > > >> Signed-off-by: Lucas Stach <dev@lynxeye.de>
>> > > >> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> > > >> ---
>> > > >>  .../bindings/mtd/nvidia-tegra20-nand.txt      | 64 +++++++++++++++++++
>> > > >>  1 file changed, 64 insertions(+)
>> > > >>  create mode 100644 Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
>> > > >>
>> > > >> diff --git a/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
>> > > >> new file mode 100644
>> > > >> index 000000000000..5cd984ef046b
>> > > >> --- /dev/null
>> > > >> +++ b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
>> > > >> @@ -0,0 +1,64 @@
>> > > >> +NVIDIA Tegra NAND Flash controller
>> > > >> +
>> > > >> +Required properties:
>> > > >> +- compatible: Must be one of:
>> > > >> +  - "nvidia,tegra20-nand"
>> > > >
>> > > > As discussed previously, I prefer "nvidia,tegra20-nand-controller" or
>> > > > "nvidia,tegra20-nfc".
>> > > >
>> > > >> +- reg: MMIO address range
>> > > >> +- interrupts: interrupt output of the NFC controller
>> > > >> +- clocks: Must contain an entry for each entry in clock-names.
>> > > >> +  See ../clocks/clock-bindings.txt for details.
>> > > >> +- clock-names: Must include the following entries:
>> > > >> +  - nand
>> > > >> +- resets: Must contain an entry for each entry in reset-names.
>> > > >> +  See ../reset/reset.txt for details.
>> > > >> +- reset-names: Must include the following entries:
>> > > >> +  - nand
>> > > >> +
>> > > >> +Optional children nodes:
>> > > >> +Individual NAND chips are children of the NAND controller node. Currently
>> > > >> +only one NAND chip supported.
>> > > >> +
>> > > >> +Required children node properties:
>> > > >> +- reg: An integer ranging from 1 to 6 representing the CS line to use.
>> > > >> +
>> > > >> +Optional children node properties:
>> > > >> +- nand-ecc-mode: String, operation mode of the NAND ecc mode. Currently only
>> > > >> +		 "hw" is supported.
>> > > >> +- nand-ecc-algo: string, algorithm of NAND ECC.
>> > > >> +		 Supported values with "hw" ECC mode are: "rs", "bch".
>> > > >> +- nand-bus-width : See nand.txt
>> > > >> +- nand-on-flash-bbt: See nand.txt
>> > > >> +- nand-ecc-strength: integer representing the number of bits to correct
>> > > >> +		     per ECC step (always 512). Supported strength using HW ECC
>> > > >> +		     modes are:
>> > > >> +		     - RS: 4, 6, 8
>> > > >> +		     - BCH: 4, 8, 14, 16
>> > > >> +- nand-ecc-maximize: See nand.txt
>> > > >> +- nand-is-boot-medium: Makes sure only ECC strengths supported by the boot ROM
>> > > >> +		       are choosen.
>> > > >> +- wp-gpios: GPIO specifier for the write protect pin.
>> > > >> +
>> > > >> +Optional child node of NAND chip nodes:
>> > > >> +Partitions: see partition.txt
>> > > >> +
>> > > >> +  Example:
>> > > >> +	nand@70008000 {
>> > > >
>> > > > 	nand-controller@70008000 {
>> > > >
>> > > >> +		compatible = "nvidia,tegra20-nand";
>> > > >
>> > > > 		compatible = "nvidia,tegra20-nand-controller";
>> > > >
>> > > > or
>> > > >
>> > > > 		compatible = "nvidia,tegra20-nfc";
>> > > >
>> > >
>> > > Maybe it's just me, but when I'm reading "nfc", my first association is the
>> > > "Near Field Communication". Probably an explicit
>> > > "nvidia,tegra20-nand-controller" variant is more preferable.
>>
>> I also prefer nvidia,tegra20-nand-controller.
>>
>> >
>> > We don't really use a -controller suffix for any of the other
>> > controllers because it is kind of implied. "nfc" is also not something
>> > that is ever referred to in the technical documentation.
>> >
>> > "nvidia,tegra20-nand" would be most consistent with all the rest of
>> > Tegra (c.f. "nvidia,tegra*-ahci", "nvidia,tegra*-pci",
>> > "nvidia,tegra*-hda", "nvidia,tegra*-gmi", ...).
>>
>> People get confused about what this node represents when you just have
>> "nvidia,tegra20-nand", and then you start seeing NAND related props or
>> partition nodes being defined under the NAND controller node.
>> I really prefer to have the "-controller" prefix here to avoid such
>> confusions.
> 
> Hmm... odd. I mean, the node is already called nand-controller@...,
> which makes it pretty obvious to me that this represents a controller
> rather than a NAND chip. Also, the placement of this in the DT hierarchy
> should make it pretty obvious that it is a controller since you can't
> just put a NAND chip directly on the CPU's address bus.
> 
> In addition I think the nvidia,tegra* part already pretty strongly
> suggests that this is part of an SoC, so further implies "controller".

The reference manual states:
"16.0 NAND FLASH CONTROLLER

The NAND flash controller allows Tegra 2 Processor to access NAND flash
memories for mass storage."

So I guess "nvidia,tegra20-nand-flash-controller" would be most
explicit. But the manual also has "GPIO controller" and we use
"nvidia,tegra20-gpio" as compatible string.

I dislike nfc since it is an increasingly less known abbreviation and
not used in the reference manual at all.

"nvidia,tegra20-nand" or "nvidia,tegra20-nand-flash-controller" is fine
for me...

--
Stefan
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Boris Brezillon June 6, 2018, 12:31 p.m. UTC | #8
On Wed, 06 Jun 2018 14:14:23 +0200
Stefan Agner <stefan@agner.ch> wrote:

> On 06.06.2018 13:07, Thierry Reding wrote:
> > On Wed, Jun 06, 2018 at 12:45:40PM +0200, Boris Brezillon wrote:  
> >> Hi Thierry,
> >>
> >> On Wed, 6 Jun 2018 12:39:03 +0200
> >> Thierry Reding <thierry.reding@gmail.com> wrote:
> >>  
> >> > On Tue, Jun 05, 2018 at 11:19:14PM +0300, Dmitry Osipenko wrote:  
> >> > > On 01.06.2018 10:30, Boris Brezillon wrote:  
> >> > > > On Fri,  1 Jun 2018 00:16:34 +0200
> >> > > > Stefan Agner <stefan@agner.ch> wrote:
> >> > > >  
> >> > > >> This adds the devicetree binding for the Tegra 2 NAND flash
> >> > > >> controller.
> >> > > >>
> >> > > >> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> >> > > >> Signed-off-by: Stefan Agner <stefan@agner.ch>
> >> > > >> ---
> >> > > >>  .../bindings/mtd/nvidia-tegra20-nand.txt      | 64 +++++++++++++++++++
> >> > > >>  1 file changed, 64 insertions(+)
> >> > > >>  create mode 100644 Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> >> > > >>
> >> > > >> diff --git a/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> >> > > >> new file mode 100644
> >> > > >> index 000000000000..5cd984ef046b
> >> > > >> --- /dev/null
> >> > > >> +++ b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
> >> > > >> @@ -0,0 +1,64 @@
> >> > > >> +NVIDIA Tegra NAND Flash controller
> >> > > >> +
> >> > > >> +Required properties:
> >> > > >> +- compatible: Must be one of:
> >> > > >> +  - "nvidia,tegra20-nand"  
> >> > > >
> >> > > > As discussed previously, I prefer "nvidia,tegra20-nand-controller" or
> >> > > > "nvidia,tegra20-nfc".
> >> > > >  
> >> > > >> +- reg: MMIO address range
> >> > > >> +- interrupts: interrupt output of the NFC controller
> >> > > >> +- clocks: Must contain an entry for each entry in clock-names.
> >> > > >> +  See ../clocks/clock-bindings.txt for details.
> >> > > >> +- clock-names: Must include the following entries:
> >> > > >> +  - nand
> >> > > >> +- resets: Must contain an entry for each entry in reset-names.
> >> > > >> +  See ../reset/reset.txt for details.
> >> > > >> +- reset-names: Must include the following entries:
> >> > > >> +  - nand
> >> > > >> +
> >> > > >> +Optional children nodes:
> >> > > >> +Individual NAND chips are children of the NAND controller node. Currently
> >> > > >> +only one NAND chip supported.
> >> > > >> +
> >> > > >> +Required children node properties:
> >> > > >> +- reg: An integer ranging from 1 to 6 representing the CS line to use.
> >> > > >> +
> >> > > >> +Optional children node properties:
> >> > > >> +- nand-ecc-mode: String, operation mode of the NAND ecc mode. Currently only
> >> > > >> +		 "hw" is supported.
> >> > > >> +- nand-ecc-algo: string, algorithm of NAND ECC.
> >> > > >> +		 Supported values with "hw" ECC mode are: "rs", "bch".
> >> > > >> +- nand-bus-width : See nand.txt
> >> > > >> +- nand-on-flash-bbt: See nand.txt
> >> > > >> +- nand-ecc-strength: integer representing the number of bits to correct
> >> > > >> +		     per ECC step (always 512). Supported strength using HW ECC
> >> > > >> +		     modes are:
> >> > > >> +		     - RS: 4, 6, 8
> >> > > >> +		     - BCH: 4, 8, 14, 16
> >> > > >> +- nand-ecc-maximize: See nand.txt
> >> > > >> +- nand-is-boot-medium: Makes sure only ECC strengths supported by the boot ROM
> >> > > >> +		       are choosen.
> >> > > >> +- wp-gpios: GPIO specifier for the write protect pin.
> >> > > >> +
> >> > > >> +Optional child node of NAND chip nodes:
> >> > > >> +Partitions: see partition.txt
> >> > > >> +
> >> > > >> +  Example:
> >> > > >> +	nand@70008000 {  
> >> > > >
> >> > > > 	nand-controller@70008000 {
> >> > > >  
> >> > > >> +		compatible = "nvidia,tegra20-nand";  
> >> > > >
> >> > > > 		compatible = "nvidia,tegra20-nand-controller";
> >> > > >
> >> > > > or
> >> > > >
> >> > > > 		compatible = "nvidia,tegra20-nfc";
> >> > > >  
> >> > >
> >> > > Maybe it's just me, but when I'm reading "nfc", my first association is the
> >> > > "Near Field Communication". Probably an explicit
> >> > > "nvidia,tegra20-nand-controller" variant is more preferable.  
> >>
> >> I also prefer nvidia,tegra20-nand-controller.
> >>  
> >> >
> >> > We don't really use a -controller suffix for any of the other
> >> > controllers because it is kind of implied. "nfc" is also not something
> >> > that is ever referred to in the technical documentation.
> >> >
> >> > "nvidia,tegra20-nand" would be most consistent with all the rest of
> >> > Tegra (c.f. "nvidia,tegra*-ahci", "nvidia,tegra*-pci",
> >> > "nvidia,tegra*-hda", "nvidia,tegra*-gmi", ...).  
> >>
> >> People get confused about what this node represents when you just have
> >> "nvidia,tegra20-nand", and then you start seeing NAND related props or
> >> partition nodes being defined under the NAND controller node.
> >> I really prefer to have the "-controller" prefix here to avoid such
> >> confusions.  
> > 
> > Hmm... odd. I mean, the node is already called nand-controller@...,
> > which makes it pretty obvious to me that this represents a controller
> > rather than a NAND chip. Also, the placement of this in the DT hierarchy
> > should make it pretty obvious that it is a controller since you can't
> > just put a NAND chip directly on the CPU's address bus.
> > 
> > In addition I think the nvidia,tegra* part already pretty strongly
> > suggests that this is part of an SoC, so further implies "controller".  
> 
> The reference manual states:
> "16.0 NAND FLASH CONTROLLER
> 
> The NAND flash controller allows Tegra 2 Processor to access NAND flash
> memories for mass storage."
> 
> So I guess "nvidia,tegra20-nand-flash-controller" would be most
> explicit. But the manual also has "GPIO controller" and we use
> "nvidia,tegra20-gpio" as compatible string.
> 
> I dislike nfc since it is an increasingly less known abbreviation and
> not used in the reference manual at all.
> 
> "nvidia,tegra20-nand" or "nvidia,tegra20-nand-flash-controller" is fine
> for me...

Enough bikeshedding, let's go for "nvidia,tegra20-nand" :-). As long as
the representation clearly differentiate the NAND controller and the
NAND chips I'm fine with it.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
new file mode 100644
index 000000000000..5cd984ef046b
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt
@@ -0,0 +1,64 @@ 
+NVIDIA Tegra NAND Flash controller
+
+Required properties:
+- compatible: Must be one of:
+  - "nvidia,tegra20-nand"
+- reg: MMIO address range
+- interrupts: interrupt output of the NFC controller
+- clocks: Must contain an entry for each entry in clock-names.
+  See ../clocks/clock-bindings.txt for details.
+- clock-names: Must include the following entries:
+  - nand
+- resets: Must contain an entry for each entry in reset-names.
+  See ../reset/reset.txt for details.
+- reset-names: Must include the following entries:
+  - nand
+
+Optional children nodes:
+Individual NAND chips are children of the NAND controller node. Currently
+only one NAND chip supported.
+
+Required children node properties:
+- reg: An integer ranging from 1 to 6 representing the CS line to use.
+
+Optional children node properties:
+- nand-ecc-mode: String, operation mode of the NAND ecc mode. Currently only
+		 "hw" is supported.
+- nand-ecc-algo: string, algorithm of NAND ECC.
+		 Supported values with "hw" ECC mode are: "rs", "bch".
+- nand-bus-width : See nand.txt
+- nand-on-flash-bbt: See nand.txt
+- nand-ecc-strength: integer representing the number of bits to correct
+		     per ECC step (always 512). Supported strength using HW ECC
+		     modes are:
+		     - RS: 4, 6, 8
+		     - BCH: 4, 8, 14, 16
+- nand-ecc-maximize: See nand.txt
+- nand-is-boot-medium: Makes sure only ECC strengths supported by the boot ROM
+		       are choosen.
+- wp-gpios: GPIO specifier for the write protect pin.
+
+Optional child node of NAND chip nodes:
+Partitions: see partition.txt
+
+  Example:
+	nand@70008000 {
+		compatible = "nvidia,tegra20-nand";
+		reg = <0x70008000 0x100>;
+		interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&tegra_car TEGRA20_CLK_NDFLASH>;
+		clock-names = "nand";
+		resets = <&tegra_car 13>;
+		reset-names = "nand";
+
+		nand-chip@0 {
+			reg = <0>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			nand-bus-width = <8>;
+			nand-on-flash-bbt;
+			nand-ecc-algo = "bch";
+			nand-ecc-strength = <8>;
+			wp-gpios = <&gpio TEGRA_GPIO(S, 0) GPIO_ACTIVE_LOW>;
+		};
+	};