diff mbox

[RFC,1/2] dt-bindings: mtd: Add Cavium SOCs NAND bindings

Message ID 20170327160524.29019-2-jglauber@cavium.com
State Changes Requested
Delegated to: Boris Brezillon
Headers show

Commit Message

Jan Glauber March 27, 2017, 4:05 p.m. UTC
Add device tree binding description for Cavium SOC nand flash controller.

CC: Rob Herring <robh+dt@kernel.org>
CC: Mark Rutland <mark.rutland@arm.com>
CC: devicetree@vger.kernel.org

Signed-off-by: Jan Glauber <jglauber@cavium.com>
---
 .../devicetree/bindings/mtd/cavium_nand.txt        | 32 ++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/cavium_nand.txt

Comments

Boris Brezillon March 28, 2017, 8:20 p.m. UTC | #1
Hi Jan,

On Mon, 27 Mar 2017 18:05:23 +0200
Jan Glauber <jglauber@cavium.com> wrote:

> Add device tree binding description for Cavium SOC nand flash controller.
> 
> CC: Rob Herring <robh+dt@kernel.org>
> CC: Mark Rutland <mark.rutland@arm.com>
> CC: devicetree@vger.kernel.org
> 
> Signed-off-by: Jan Glauber <jglauber@cavium.com>
> ---
>  .../devicetree/bindings/mtd/cavium_nand.txt        | 32 ++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/cavium_nand.txt
> 
> diff --git a/Documentation/devicetree/bindings/mtd/cavium_nand.txt b/Documentation/devicetree/bindings/mtd/cavium_nand.txt
> new file mode 100644
> index 0000000..4698d1f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/cavium_nand.txt
> @@ -0,0 +1,32 @@
> +* Cavium NAND controller
> +
> +Required properties:
> +
> +- compatible:		should be "cavium,cn8xxx-nand"
> +- reg:			PCI devfn
> +- clocks:		must contain system clock
> +- #address-cells:	<1>
> +- #size-cells:		<0>
> +
> +The nand flash controller may contain up to 8 subnodes representing
> +NAND flash chips. Their properties are as follows.
> +
> +Required properties:
> +- compatible:		should be "cavium,nandcs"

Why do you need a compatible here? All sub-nodes should be representing
NAND devices connected to the NAND controller. If you need an extra
subnode to represent something that is not a NAND device, then it should
not have a reg property, so testing if reg is present to detect if the
subnode is reprensenting a NAND device should be enough.

Am I missing something?

> +- reg:			a single integer representing the chip-select number
> +- nand-ecc-mode:	see nand.txt
> +
> +Example:
> +
> +nfc: nand@b,0 {

        ^ nand-controller@xxx

> +	compatible = "cavium,cn8xxx-nand";
> +	reg = <0x5800 0 0 0 0>;
> +	clocks = <&sclk>;
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	nand@1 {
> +		compatible = "cavium,nandcs";
> +		reg = <1>;
> +		nand-ecc-mode = "on-die";
> +};
Jan Glauber March 28, 2017, 9:30 p.m. UTC | #2
On Tue, Mar 28, 2017 at 10:20:35PM +0200, Boris Brezillon wrote:
> Hi Jan,
> 
> On Mon, 27 Mar 2017 18:05:23 +0200
> Jan Glauber <jglauber@cavium.com> wrote:
> 
> > Add device tree binding description for Cavium SOC nand flash controller.
> > 
> > CC: Rob Herring <robh+dt@kernel.org>
> > CC: Mark Rutland <mark.rutland@arm.com>
> > CC: devicetree@vger.kernel.org
> > 
> > Signed-off-by: Jan Glauber <jglauber@cavium.com>
> > ---
> >  .../devicetree/bindings/mtd/cavium_nand.txt        | 32 ++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mtd/cavium_nand.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/cavium_nand.txt b/Documentation/devicetree/bindings/mtd/cavium_nand.txt
> > new file mode 100644
> > index 0000000..4698d1f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/cavium_nand.txt
> > @@ -0,0 +1,32 @@
> > +* Cavium NAND controller
> > +
> > +Required properties:
> > +
> > +- compatible:		should be "cavium,cn8xxx-nand"
> > +- reg:			PCI devfn
> > +- clocks:		must contain system clock
> > +- #address-cells:	<1>
> > +- #size-cells:		<0>
> > +
> > +The nand flash controller may contain up to 8 subnodes representing
> > +NAND flash chips. Their properties are as follows.
> > +
> > +Required properties:
> > +- compatible:		should be "cavium,nandcs"
> 
> Why do you need a compatible here? All sub-nodes should be representing
> NAND devices connected to the NAND controller. If you need an extra
> subnode to represent something that is not a NAND device, then it should
> not have a reg property, so testing if reg is present to detect if the
> subnode is reprensenting a NAND device should be enough.
> 
> Am I missing something?

Hi Boris,

You're right. We don't need or check this compatible. The chip type
which would make more sense than what I used above is detected via ONFI,
so I can just remove the compatible string.

> > +- reg:			a single integer representing the chip-select number
> > +- nand-ecc-mode:	see nand.txt
> > +
> > +Example:
> > +
> > +nfc: nand@b,0 {
> 
>         ^ nand-controller@xxx

OK.

> > +	compatible = "cavium,cn8xxx-nand";
> > +	reg = <0x5800 0 0 0 0>;
> > +	clocks = <&sclk>;
> > +	#address-cells = <1>;
> > +	#size-cells = <0>;
> > +
> > +	nand@1 {
> > +		compatible = "cavium,nandcs";
> > +		reg = <1>;
> > +		nand-ecc-mode = "on-die";
> > +};
Rob Herring (Arm) April 3, 2017, 1:29 p.m. UTC | #3
On Mon, Mar 27, 2017 at 06:05:23PM +0200, Jan Glauber wrote:
> Add device tree binding description for Cavium SOC nand flash controller.
> 
> CC: Rob Herring <robh+dt@kernel.org>
> CC: Mark Rutland <mark.rutland@arm.com>
> CC: devicetree@vger.kernel.org
> 
> Signed-off-by: Jan Glauber <jglauber@cavium.com>
> ---
>  .../devicetree/bindings/mtd/cavium_nand.txt        | 32 ++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/cavium_nand.txt
> 
> diff --git a/Documentation/devicetree/bindings/mtd/cavium_nand.txt b/Documentation/devicetree/bindings/mtd/cavium_nand.txt
> new file mode 100644
> index 0000000..4698d1f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/cavium_nand.txt
> @@ -0,0 +1,32 @@
> +* Cavium NAND controller
> +
> +Required properties:
> +
> +- compatible:		should be "cavium,cn8xxx-nand"

Don't use wildcards in compatible strings. For PCI devices, this should 
be based on the PCI vendor and device IDs.

> +- reg:			PCI devfn
> +- clocks:		must contain system clock
> +- #address-cells:	<1>
> +- #size-cells:		<0>
> +
> +The nand flash controller may contain up to 8 subnodes representing
> +NAND flash chips. Their properties are as follows.
> +
> +Required properties:
> +- compatible:		should be "cavium,nandcs"
> +- reg:			a single integer representing the chip-select number
> +- nand-ecc-mode:	see nand.txt
> +
> +Example:
> +
> +nfc: nand@b,0 {
> +	compatible = "cavium,cn8xxx-nand";
> +	reg = <0x5800 0 0 0 0>;
> +	clocks = <&sclk>;
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	nand@1 {
> +		compatible = "cavium,nandcs";
> +		reg = <1>;
> +		nand-ecc-mode = "on-die";
> +};
> -- 
> 2.9.0.rc0.21.g7777322
>
Jan Glauber April 3, 2017, 2:38 p.m. UTC | #4
On Mon, Apr 03, 2017 at 08:29:37AM -0500, Rob Herring wrote:
> On Mon, Mar 27, 2017 at 06:05:23PM +0200, Jan Glauber wrote:
> > Add device tree binding description for Cavium SOC nand flash controller.
> > 
> > CC: Rob Herring <robh+dt@kernel.org>
> > CC: Mark Rutland <mark.rutland@arm.com>
> > CC: devicetree@vger.kernel.org
> > 
> > Signed-off-by: Jan Glauber <jglauber@cavium.com>
> > ---
> >  .../devicetree/bindings/mtd/cavium_nand.txt        | 32 ++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mtd/cavium_nand.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/cavium_nand.txt b/Documentation/devicetree/bindings/mtd/cavium_nand.txt
> > new file mode 100644
> > index 0000000..4698d1f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/cavium_nand.txt
> > @@ -0,0 +1,32 @@
> > +* Cavium NAND controller
> > +
> > +Required properties:
> > +
> > +- compatible:		should be "cavium,cn8xxx-nand"
> 
> Don't use wildcards in compatible strings. For PCI devices, this should 
> be based on the PCI vendor and device IDs.
>

Is there a syntax for compatible PCI devices? I'm afraid I've not seen
this yet, can you give an example?

Most of Cavium's devices are PCI devices, we just added the compatible
as convenience and usually it is not parsed.

thanks,
Jan

> > +- reg:			PCI devfn
> > +- clocks:		must contain system clock
> > +- #address-cells:	<1>
> > +- #size-cells:		<0>
> > +
> > +The nand flash controller may contain up to 8 subnodes representing
> > +NAND flash chips. Their properties are as follows.
> > +
> > +Required properties:
> > +- compatible:		should be "cavium,nandcs"
> > +- reg:			a single integer representing the chip-select number
> > +- nand-ecc-mode:	see nand.txt
> > +
> > +Example:
> > +
> > +nfc: nand@b,0 {
> > +	compatible = "cavium,cn8xxx-nand";
> > +	reg = <0x5800 0 0 0 0>;
> > +	clocks = <&sclk>;
> > +	#address-cells = <1>;
> > +	#size-cells = <0>;
> > +
> > +	nand@1 {
> > +		compatible = "cavium,nandcs";
> > +		reg = <1>;
> > +		nand-ecc-mode = "on-die";
> > +};
> > -- 
> > 2.9.0.rc0.21.g7777322
> >
Rob Herring (Arm) April 3, 2017, 2:47 p.m. UTC | #5
On Mon, Apr 3, 2017 at 9:38 AM, Jan Glauber
<jan.glauber@caviumnetworks.com> wrote:
> On Mon, Apr 03, 2017 at 08:29:37AM -0500, Rob Herring wrote:
>> On Mon, Mar 27, 2017 at 06:05:23PM +0200, Jan Glauber wrote:
>> > Add device tree binding description for Cavium SOC nand flash controller.
>> >
>> > CC: Rob Herring <robh+dt@kernel.org>
>> > CC: Mark Rutland <mark.rutland@arm.com>
>> > CC: devicetree@vger.kernel.org
>> >
>> > Signed-off-by: Jan Glauber <jglauber@cavium.com>
>> > ---
>> >  .../devicetree/bindings/mtd/cavium_nand.txt        | 32 ++++++++++++++++++++++
>> >  1 file changed, 32 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/mtd/cavium_nand.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/mtd/cavium_nand.txt b/Documentation/devicetree/bindings/mtd/cavium_nand.txt
>> > new file mode 100644
>> > index 0000000..4698d1f
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/mtd/cavium_nand.txt
>> > @@ -0,0 +1,32 @@
>> > +* Cavium NAND controller
>> > +
>> > +Required properties:
>> > +
>> > +- compatible:              should be "cavium,cn8xxx-nand"
>>
>> Don't use wildcards in compatible strings. For PCI devices, this should
>> be based on the PCI vendor and device IDs.
>>
>
> Is there a syntax for compatible PCI devices? I'm afraid I've not seen
> this yet, can you give an example?

www.o3one.org/hwdocs/openfirmware/pci_supplement_2_1.pdf

> Most of Cavium's devices are PCI devices, we just added the compatible
> as convenience and usually it is not parsed.

Linux doesn't parse it, but it's still required in the binding.

Rob
Jan Glauber April 3, 2017, 4:18 p.m. UTC | #6
On Mon, Apr 03, 2017 at 09:47:05AM -0500, Rob Herring wrote:
> On Mon, Apr 3, 2017 at 9:38 AM, Jan Glauber
> <jan.glauber@caviumnetworks.com> wrote:
> > On Mon, Apr 03, 2017 at 08:29:37AM -0500, Rob Herring wrote:
> >> On Mon, Mar 27, 2017 at 06:05:23PM +0200, Jan Glauber wrote:
> >> > Add device tree binding description for Cavium SOC nand flash controller.
> >> >
> >> > CC: Rob Herring <robh+dt@kernel.org>
> >> > CC: Mark Rutland <mark.rutland@arm.com>
> >> > CC: devicetree@vger.kernel.org
> >> >
> >> > Signed-off-by: Jan Glauber <jglauber@cavium.com>
> >> > ---
> >> >  .../devicetree/bindings/mtd/cavium_nand.txt        | 32 ++++++++++++++++++++++
> >> >  1 file changed, 32 insertions(+)
> >> >  create mode 100644 Documentation/devicetree/bindings/mtd/cavium_nand.txt
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/mtd/cavium_nand.txt b/Documentation/devicetree/bindings/mtd/cavium_nand.txt
> >> > new file mode 100644
> >> > index 0000000..4698d1f
> >> > --- /dev/null
> >> > +++ b/Documentation/devicetree/bindings/mtd/cavium_nand.txt
> >> > @@ -0,0 +1,32 @@
> >> > +* Cavium NAND controller
> >> > +
> >> > +Required properties:
> >> > +
> >> > +- compatible:              should be "cavium,cn8xxx-nand"
> >>
> >> Don't use wildcards in compatible strings. For PCI devices, this should
> >> be based on the PCI vendor and device IDs.
> >>
> >
> > Is there a syntax for compatible PCI devices? I'm afraid I've not seen
> > this yet, can you give an example?
> 
> www.o3one.org/hwdocs/openfirmware/pci_supplement_2_1.pdf

Thanks, I probably should have read this before...

So it will be something like:
compatible = "pci177d,a04f"

A bit unreadable, but it solves the wildcard issue.

> > Most of Cavium's devices are PCI devices, we just added the compatible
> > as convenience and usually it is not parsed.
> 
> Linux doesn't parse it, but it's still required in the binding.

OK.

--Jan

> Rob
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/mtd/cavium_nand.txt b/Documentation/devicetree/bindings/mtd/cavium_nand.txt
new file mode 100644
index 0000000..4698d1f
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/cavium_nand.txt
@@ -0,0 +1,32 @@ 
+* Cavium NAND controller
+
+Required properties:
+
+- compatible:		should be "cavium,cn8xxx-nand"
+- reg:			PCI devfn
+- clocks:		must contain system clock
+- #address-cells:	<1>
+- #size-cells:		<0>
+
+The nand flash controller may contain up to 8 subnodes representing
+NAND flash chips. Their properties are as follows.
+
+Required properties:
+- compatible:		should be "cavium,nandcs"
+- reg:			a single integer representing the chip-select number
+- nand-ecc-mode:	see nand.txt
+
+Example:
+
+nfc: nand@b,0 {
+	compatible = "cavium,cn8xxx-nand";
+	reg = <0x5800 0 0 0 0>;
+	clocks = <&sclk>;
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	nand@1 {
+		compatible = "cavium,nandcs";
+		reg = <1>;
+		nand-ecc-mode = "on-die";
+};