2018-11-09 05:04:22

by Naga Sureshkumar Relli

[permalink] [raw]
Subject: [LINUX PATCH v12 1/3] dt-bindings: mtd: arasan: Add device tree binding documentation

This patch adds the dts binding document for arasan nand flash controller

Signed-off-by: Naga Sureshkumar Relli <[email protected]>
---
Changes in v12:
- Removed interrupt-parent description as it is implied as suggested by
Rob Herring
- Added missing ';' as required
Changes in v11:
- Updated compatible description as suggested by Boris
- Removed arasan-has-dma property
Changes in v10:
- None
Changes in v9:
- None
Changes in v8:
- Updated compatible and clock-names as per Boris comments
Changes in v7:
- Corrected the acronyms those should be in caps
Changes in v6:
- Removed num-cs property
- Separated nandchip from nand controller
Changes in v5:
- None
Changes in v4:
- Added num-cs property
- Added clock support
Changes in v3:
- None
Changes in v2:
- None
---
.../devicetree/bindings/mtd/arasan_nand.txt | 32 ++++++++++++++++++++++
1 file changed, 32 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mtd/arasan_nand.txt

diff --git a/Documentation/devicetree/bindings/mtd/arasan_nand.txt b/Documentation/devicetree/bindings/mtd/arasan_nand.txt
new file mode 100644
index 0000000..b522daf
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/arasan_nand.txt
@@ -0,0 +1,32 @@
+Arasan NAND Flash Controller with ONFI 3.1 support
+
+Required properties:
+- compatible: Should be "xlnx,zynqmp-nand", "arasan,nfc-v3p10"
+- reg: Memory map for module access
+- interrupts: Should contain the interrupt for the device
+- clock-name: List of input clocks - "sys", "flash"
+ (See clock bindings for details)
+- clocks: Clock phandles (see clock bindings for details)
+
+Required properties for child node:
+- nand-ecc-mode: see nand.txt
+
+For NAND partition information please refer the below file
+Documentation/devicetree/bindings/mtd/partition.txt
+
+Example:
+ nfc: nand@ff100000 {
+ compatible = "xlnx,zynqmp-nand", "arasan,nfc-v3p10";
+ reg = <0x0 0xff100000 0x1000>;
+ clock-name = "sys", "flash";
+ clocks = <&misc_clk &misc_clk>;
+ interrupt-parent = <&gic>;
+ interrupts = <0 14 4>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ nand@0 {
+ reg = <0>;
+ nand-ecc-mode = "hw";
+ };
+ };
--
2.7.4



2018-11-09 06:30:05

by Boris Brezillon

[permalink] [raw]
Subject: Re: [LINUX PATCH v12 1/3] dt-bindings: mtd: arasan: Add device tree binding documentation

On Fri, 9 Nov 2018 10:30:39 +0530
Naga Sureshkumar Relli <[email protected]> wrote:

> This patch adds the dts binding document for arasan nand flash controller
>
> Signed-off-by: Naga Sureshkumar Relli <[email protected]>
> ---
> Changes in v12:
> - Removed interrupt-parent description as it is implied as suggested by
> Rob Herring
> - Added missing ';' as required
> Changes in v11:
> - Updated compatible description as suggested by Boris
> - Removed arasan-has-dma property
> Changes in v10:
> - None
> Changes in v9:
> - None
> Changes in v8:
> - Updated compatible and clock-names as per Boris comments
> Changes in v7:
> - Corrected the acronyms those should be in caps
> Changes in v6:
> - Removed num-cs property
> - Separated nandchip from nand controller
> Changes in v5:
> - None
> Changes in v4:
> - Added num-cs property
> - Added clock support
> Changes in v3:
> - None
> Changes in v2:
> - None
> ---
> .../devicetree/bindings/mtd/arasan_nand.txt | 32 ++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mtd/arasan_nand.txt
>
> diff --git a/Documentation/devicetree/bindings/mtd/arasan_nand.txt b/Documentation/devicetree/bindings/mtd/arasan_nand.txt
> new file mode 100644
> index 0000000..b522daf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/arasan_nand.txt
> @@ -0,0 +1,32 @@
> +Arasan NAND Flash Controller with ONFI 3.1 support
> +
> +Required properties:
> +- compatible: Should be "xlnx,zynqmp-nand", "arasan,nfc-v3p10"
> +- reg: Memory map for module access
> +- interrupts: Should contain the interrupt for the device
> +- clock-name: List of input clocks - "sys", "flash"
> + (See clock bindings for details)
> +- clocks: Clock phandles (see clock bindings for details)
> +
> +Required properties for child node:
> +- nand-ecc-mode: see nand.txt

Why is it required? Can't you fallback to HW when this prop is missing?
Oh, and reg is not listed in the required props.

> +
> +For NAND partition information please refer the below file
> +Documentation/devicetree/bindings/mtd/partition.txt
> +
> +Example:
> + nfc: nand@ff100000 {
> + compatible = "xlnx,zynqmp-nand", "arasan,nfc-v3p10";
> + reg = <0x0 0xff100000 0x1000>;
> + clock-name = "sys", "flash";
> + clocks = <&misc_clk &misc_clk>;
> + interrupt-parent = <&gic>;
> + interrupts = <0 14 4>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + nand@0 {
> + reg = <0>;
> + nand-ecc-mode = "hw";
> + };
> + };


2018-11-09 12:35:23

by Naga Sureshkumar Relli

[permalink] [raw]
Subject: RE: [LINUX PATCH v12 1/3] dt-bindings: mtd: arasan: Add device tree binding documentation

Hi Boris,

> -----Original Message-----
> From: Boris Brezillon [mailto:[email protected]]
> Sent: Friday, November 9, 2018 11:59 AM
> To: Naga Sureshkumar Relli <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Michal Simek
> <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [LINUX PATCH v12 1/3] dt-bindings: mtd: arasan: Add device tree binding
> documentation
>
> On Fri, 9 Nov 2018 10:30:39 +0530
> Naga Sureshkumar Relli <[email protected]> wrote:
>
> > This patch adds the dts binding document for arasan nand flash
> > controller
> >
> > Signed-off-by: Naga Sureshkumar Relli
> > <[email protected]>
> > ---
> > Changes in v12:
> > - Removed interrupt-parent description as it is implied as suggested by
> > Rob Herring
> > - Added missing ';' as required
> > Changes in v11:
> > - Updated compatible description as suggested by Boris
> > - Removed arasan-has-dma property
> > Changes in v10:
> > - None
> > Changes in v9:
> > - None
> > Changes in v8:
> > - Updated compatible and clock-names as per Boris comments Changes in
> > v7:
> > - Corrected the acronyms those should be in caps Changes in v6:
> > - Removed num-cs property
> > - Separated nandchip from nand controller Changes in v5:
> > - None
> > Changes in v4:
> > - Added num-cs property
> > - Added clock support
> > Changes in v3:
> > - None
> > Changes in v2:
> > - None
> > ---
> > .../devicetree/bindings/mtd/arasan_nand.txt | 32 ++++++++++++++++++++++
> > 1 file changed, 32 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/mtd/arasan_nand.txt
> >
> > diff --git a/Documentation/devicetree/bindings/mtd/arasan_nand.txt
> > b/Documentation/devicetree/bindings/mtd/arasan_nand.txt
> > new file mode 100644
> > index 0000000..b522daf
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/arasan_nand.txt
> > @@ -0,0 +1,32 @@
> > +Arasan NAND Flash Controller with ONFI 3.1 support
> > +
> > +Required properties:
> > +- compatible: Should be "xlnx,zynqmp-nand", "arasan,nfc-v3p10"
> > +- reg: Memory map for module access
> > +- interrupts: Should contain the interrupt for the device
> > +- clock-name: List of input clocks - "sys", "flash"
> > + (See clock bindings for details)
> > +- clocks: Clock phandles (see clock bindings for details)
> > +
> > +Required properties for child node:
> > +- nand-ecc-mode: see nand.txt
>
> Why is it required? Can't you fallback to HW when this prop is missing?
Yes, we can.
Do you want me to update that in driver now?
Looks like you have some comments in driver as well, so while addressing those I will update the code to fallback to HW ECC when
It is missing.

> Oh, and reg is not listed in the required props.
Which reg?
I already mention this " reg: Memory map for module access " in required properties.

Thanks,
Naga Sureshkumar Relli
>
> > +
> > +For NAND partition information please refer the below file
> > +Documentation/devicetree/bindings/mtd/partition.txt
> > +
> > +Example:
> > + nfc: nand@ff100000 {
> > + compatible = "xlnx,zynqmp-nand", "arasan,nfc-v3p10";
> > + reg = <0x0 0xff100000 0x1000>;
> > + clock-name = "sys", "flash";
> > + clocks = <&misc_clk &misc_clk>;
> > + interrupt-parent = <&gic>;
> > + interrupts = <0 14 4>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + nand@0 {
> > + reg = <0>;
> > + nand-ecc-mode = "hw";
> > + };
> > + };


2018-11-09 12:55:08

by Miquel Raynal

[permalink] [raw]
Subject: Re: [LINUX PATCH v12 1/3] dt-bindings: mtd: arasan: Add device tree binding documentation

Hi Naga,

[...]

> > > diff --git a/Documentation/devicetree/bindings/mtd/arasan_nand.txt
> > > b/Documentation/devicetree/bindings/mtd/arasan_nand.txt
> > > new file mode 100644
> > > index 0000000..b522daf
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mtd/arasan_nand.txt
> > > @@ -0,0 +1,32 @@
> > > +Arasan NAND Flash Controller with ONFI 3.1 support
> > > +
> > > +Required properties:
> > > +- compatible: Should be "xlnx,zynqmp-nand", "arasan,nfc-v3p10"
> > > +- reg: Memory map for module access
> > > +- interrupts: Should contain the interrupt for the device
> > > +- clock-name: List of input clocks - "sys", "flash"
> > > + (See clock bindings for details)
> > > +- clocks: Clock phandles (see clock bindings for details)
> > > +
> > > +Required properties for child node:
> > > +- nand-ecc-mode: see nand.txt
> >
> > Why is it required? Can't you fallback to HW when this prop is missing?
> Yes, we can.
> Do you want me to update that in driver now?
> Looks like you have some comments in driver as well, so while addressing those I will update the code to fallback to HW ECC when
> It is missing.

Yes, please.

>
> > Oh, and reg is not listed in the required props.
> Which reg?
> I already mention this " reg: Memory map for module access " in required properties.
>
> Thanks,
> Naga Sureshkumar Relli
> >
> > > +
> > > +For NAND partition information please refer the below file
> > > +Documentation/devicetree/bindings/mtd/partition.txt
> > > +
> > > +Example:
> > > + nfc: nand@ff100000 {
> > > + compatible = "xlnx,zynqmp-nand", "arasan,nfc-v3p10";
> > > + reg = <0x0 0xff100000 0x1000>;
> > > + clock-name = "sys", "flash";
> > > + clocks = <&misc_clk &misc_clk>;
> > > + interrupt-parent = <&gic>;
> > > + interrupts = <0 14 4>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> > > + nand@0 {
> > > + reg = <0>;

This one, for the CS line(s).

> > > + nand-ecc-mode = "hw";
> > > + };
> > > + };
>


Thanks,
Miquèl

2018-11-09 13:22:19

by Naga Sureshkumar Relli

[permalink] [raw]
Subject: RE: [LINUX PATCH v12 1/3] dt-bindings: mtd: arasan: Add device tree binding documentation

Hi Miquel,

> -----Original Message-----
> From: Miquel Raynal [mailto:[email protected]]
> Sent: Friday, November 9, 2018 6:24 PM
> To: Naga Sureshkumar Relli <[email protected]>
> Cc: Boris Brezillon <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]; Michal Simek
> <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [LINUX PATCH v12 1/3] dt-bindings: mtd: arasan: Add device tree binding
> documentation
>
> Hi Naga,
>
> [...]
>
> > > > diff --git a/Documentation/devicetree/bindings/mtd/arasan_nand.txt
> > > > b/Documentation/devicetree/bindings/mtd/arasan_nand.txt
> > > > new file mode 100644
> > > > index 0000000..b522daf
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/mtd/arasan_nand.txt
> > > > @@ -0,0 +1,32 @@
> > > > +Arasan NAND Flash Controller with ONFI 3.1 support
> > > > +
> > > > +Required properties:
> > > > +- compatible: Should be "xlnx,zynqmp-nand", "arasan,nfc-v3p10"
> > > > +- reg: Memory map for module access
> > > > +- interrupts: Should contain the interrupt for the device
> > > > +- clock-name: List of input clocks - "sys", "flash"
> > > > + (See clock bindings for details)
> > > > +- clocks: Clock phandles (see clock bindings for details)
> > > > +
> > > > +Required properties for child node:
> > > > +- nand-ecc-mode: see nand.txt
> > >
> > > Why is it required? Can't you fallback to HW when this prop is missing?
> > Yes, we can.
> > Do you want me to update that in driver now?
> > Looks like you have some comments in driver as well, so while
> > addressing those I will update the code to fallback to HW ECC when It is missing.
>
> Yes, please.
>
> >
> > > Oh, and reg is not listed in the required props.
> > Which reg?
> > I already mention this " reg: Memory map for module access " in required properties.
> >
> > Thanks,
> > Naga Sureshkumar Relli
> > >
> > > > +
> > > > +For NAND partition information please refer the below file
> > > > +Documentation/devicetree/bindings/mtd/partition.txt
> > > > +
> > > > +Example:
> > > > + nfc: nand@ff100000 {
> > > > + compatible = "xlnx,zynqmp-nand", "arasan,nfc-v3p10";
> > > > + reg = <0x0 0xff100000 0x1000>;
> > > > + clock-name = "sys", "flash";
> > > > + clocks = <&misc_clk &misc_clk>;
> > > > + interrupt-parent = <&gic>;
> > > > + interrupts = <0 14 4>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > +
> > > > + nand@0 {
> > > > + reg = <0>;
>
> This one, for the CS line(s).
Ok got it. I will update.

Thanks,
Naga Sureshkumar Relli
>
> > > > + nand-ecc-mode = "hw";
> > > > + };
> > > > + };
> >
>
>
> Thanks,
> Miquèl

2018-11-16 11:51:50

by Martin Lund

[permalink] [raw]
Subject: Re: [LINUX PATCH v12 1/3] dt-bindings: mtd: arasan: Add device tree binding documentation

Hi Naga,

I've been working on running up the latest kernel (v4.20-rc2) on our
custom Xilinx hw board so that I can test the v12 version of your
Arasan nand driver.

I've managed to get the driver successfully up and running and ready
for testing with a Micron MT29F64G08AFAAAWP device. However, setting
it up I've found a few inaccuracies in the documentation of the device
tree bindings.

This is the device configuration that ended up working for me with
linux v4.20-rc2:

nfc: nand@ff100000 {
compatible = "xlnx,zynqmp-nand", "arasan,nfc-v3p10";
reg = <0x0 0xff100000 0x0 0x1000>;
clock-names = "clk_sys", "clk_flash";
clocks = <&clk200>, <&clk100>;
interrupt-parent = <&gic>;
interrupts = <0 14 4>;
#address-cells = <1>;
#size-cells = <0>;

nand@0 {
reg = <0>;
nand-ecc-mode = "hw";
};
};

Compared with the example you will notice that "clock-name" should be
"clock-names". reg was missing a "0x0".

I think it is helpful to provide a real-world working example, so you
might also consider changing the example "clocks" configuration to
clk200/clk100 since there is no clk_misc among the clock sources of
any of the xilinx zynqmp board device tree configurations.

Br, Martin

On Fri, Nov 9, 2018 at 6:00 AM Naga Sureshkumar Relli
<[email protected]> wrote:
>
> This patch adds the dts binding document for arasan nand flash controller
>
> Signed-off-by: Naga Sureshkumar Relli <[email protected]>
> ---
> Changes in v12:
> - Removed interrupt-parent description as it is implied as suggested by
> Rob Herring
> - Added missing ';' as required
> Changes in v11:
> - Updated compatible description as suggested by Boris
> - Removed arasan-has-dma property
> Changes in v10:
> - None
> Changes in v9:
> - None
> Changes in v8:
> - Updated compatible and clock-names as per Boris comments
> Changes in v7:
> - Corrected the acronyms those should be in caps
> Changes in v6:
> - Removed num-cs property
> - Separated nandchip from nand controller
> Changes in v5:
> - None
> Changes in v4:
> - Added num-cs property
> - Added clock support
> Changes in v3:
> - None
> Changes in v2:
> - None
> ---
> .../devicetree/bindings/mtd/arasan_nand.txt | 32 ++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mtd/arasan_nand.txt
>
> diff --git a/Documentation/devicetree/bindings/mtd/arasan_nand.txt b/Documentation/devicetree/bindings/mtd/arasan_nand.txt
> new file mode 100644
> index 0000000..b522daf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/arasan_nand.txt
> @@ -0,0 +1,32 @@
> +Arasan NAND Flash Controller with ONFI 3.1 support
> +
> +Required properties:
> +- compatible: Should be "xlnx,zynqmp-nand", "arasan,nfc-v3p10"
> +- reg: Memory map for module access
> +- interrupts: Should contain the interrupt for the device
> +- clock-name: List of input clocks - "sys", "flash"
> + (See clock bindings for details)
> +- clocks: Clock phandles (see clock bindings for details)
> +
> +Required properties for child node:
> +- nand-ecc-mode: see nand.txt
> +
> +For NAND partition information please refer the below file
> +Documentation/devicetree/bindings/mtd/partition.txt
> +
> +Example:
> + nfc: nand@ff100000 {
> + compatible = "xlnx,zynqmp-nand", "arasan,nfc-v3p10";
> + reg = <0x0 0xff100000 0x1000>;
> + clock-name = "sys", "flash";
> + clocks = <&misc_clk &misc_clk>;
> + interrupt-parent = <&gic>;
> + interrupts = <0 14 4>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + nand@0 {
> + reg = <0>;
> + nand-ecc-mode = "hw";
> + };
> + };
> --
> 2.7.4
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

2018-11-16 12:11:46

by Martin Lund

[permalink] [raw]
Subject: Re: [LINUX PATCH v12 1/3] dt-bindings: mtd: arasan: Add device tree binding documentation

On Fri, Nov 16, 2018 at 12:50 PM Martin Lund
<[email protected]> wrote:
> Compared with the example you will notice that "clock-name" should be
> "clock-names". reg was missing a "0x0".

Also, "sys" should be "clk_sys" and "flash" should be "clk_flash".

2018-11-16 12:36:50

by Michal Simek

[permalink] [raw]
Subject: Re: [LINUX PATCH v12 1/3] dt-bindings: mtd: arasan: Add device tree binding documentation

On 16. 11. 18 12:50, Martin Lund wrote:
> Hi Naga,
>
> I've been working on running up the latest kernel (v4.20-rc2) on our
> custom Xilinx hw board so that I can test the v12 version of your
> Arasan nand driver.
>
> I've managed to get the driver successfully up and running and ready
> for testing with a Micron MT29F64G08AFAAAWP device. However, setting
> it up I've found a few inaccuracies in the documentation of the device
> tree bindings.
>
> This is the device configuration that ended up working for me with
> linux v4.20-rc2:
>
> nfc: nand@ff100000 {
> compatible = "xlnx,zynqmp-nand", "arasan,nfc-v3p10";
> reg = <0x0 0xff100000 0x0 0x1000>;
> clock-names = "clk_sys", "clk_flash";
> clocks = <&clk200>, <&clk100>;
> interrupt-parent = <&gic>;
> interrupts = <0 14 4>;
> #address-cells = <1>;
> #size-cells = <0>;
>
> nand@0 {
> reg = <0>;
> nand-ecc-mode = "hw";
> };
> };
>
> Compared with the example you will notice that "clock-name" should be
> "clock-names". reg was missing a "0x0".

clock-names and even that names - you are right it is not correct and
should be fixed.

Missing 0x0 in reg doesn't matter because it depends on address/size cells.

> I think it is helpful to provide a real-world working example, so you
> might also consider changing the example "clocks" configuration to
> clk200/clk100 since there is no clk_misc among the clock sources of
> any of the xilinx zynqmp board device tree configurations.

Real example is the best normally just c&p from existing dts is the way
to go.
But in connection to clocks it doesn't matter what exactly should be
there and I don't think there is any consistency in that. Hopefully this
will be removed by yaml conversion.

Thanks,
Michal


2018-11-16 13:51:43

by Naga Sureshkumar Relli

[permalink] [raw]
Subject: RE: [LINUX PATCH v12 1/3] dt-bindings: mtd: arasan: Add device tree binding documentation

Hi,

> -----Original Message-----
> From: Michal Simek [mailto:[email protected]]
> Sent: Friday, November 16, 2018 6:04 PM
> To: Martin Lund <[email protected]>; Naga Sureshkumar Relli
> <[email protected]>
> Cc: [email protected]; [email protected]; [email protected]; David
> Woodhouse <[email protected]>; Brian Norris <[email protected]>; Marek
> Vašut <[email protected]>; naga sureshkumar <[email protected]>; linux-
> [email protected]; [email protected]; [email protected]
> Subject: Re: [LINUX PATCH v12 1/3] dt-bindings: mtd: arasan: Add device tree binding
> documentation
>
> On 16. 11. 18 12:50, Martin Lund wrote:
> > Hi Naga,
> >
> > I've been working on running up the latest kernel (v4.20-rc2) on our
> > custom Xilinx hw board so that I can test the v12 version of your
> > Arasan nand driver.
> >
> > I've managed to get the driver successfully up and running and ready
> > for testing with a Micron MT29F64G08AFAAAWP device. However, setting
> > it up I've found a few inaccuracies in the documentation of the device
> > tree bindings.
> >
> > This is the device configuration that ended up working for me with
> > linux v4.20-rc2:
> >
> > nfc: nand@ff100000 {
> > compatible = "xlnx,zynqmp-nand", "arasan,nfc-v3p10";
> > reg = <0x0 0xff100000 0x0 0x1000>;
> > clock-names = "clk_sys", "clk_flash";
> > clocks = <&clk200>, <&clk100>;
> > interrupt-parent = <&gic>;
> > interrupts = <0 14 4>;
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > nand@0 {
> > reg = <0>;
> > nand-ecc-mode = "hw";
> > };
> > };
> >
> > Compared with the example you will notice that "clock-name" should be
> > "clock-names". reg was missing a "0x0".
>
> clock-names and even that names - you are right it is not correct and should be fixed.
Clock-names I will change from "clock-name" to "clock-names".
But I got some review comments previously, to use "sys" instead of "clk_sys" and "flash" instead of "clk_flash".
And I have to change this in driver.
I will update that.

Thanks,
Naga Sureshkumar Relli

>
> Missing 0x0 in reg doesn't matter because it depends on address/size cells.
>
> > I think it is helpful to provide a real-world working example, so you
> > might also consider changing the example "clocks" configuration to
> > clk200/clk100 since there is no clk_misc among the clock sources of
> > any of the xilinx zynqmp board device tree configurations.
>
> Real example is the best normally just c&p from existing dts is the way to go.
> But in connection to clocks it doesn't matter what exactly should be there and I don't think
> there is any consistency in that. Hopefully this will be removed by yaml conversion.
>
> Thanks,
> Michal

2018-11-16 14:23:44

by Martin Lund

[permalink] [raw]
Subject: Re: [LINUX PATCH v12 1/3] dt-bindings: mtd: arasan: Add device tree binding documentation

Yes, the clk_ prefix is kind of unnecessary. Though, all the
clock-names in zynqmp.dtsi are still using the prefix - hopefully that
will eventually be cleaned out.

Either way, we just need to make sure the device tree doc clock-names
are consistent with whatever is used by the driver.

Br, Martin
On Fri, Nov 16, 2018 at 2:50 PM Naga Sureshkumar Relli
<[email protected]> wrote:
>
> Hi,
>
> > -----Original Message-----
> > From: Michal Simek [mailto:[email protected]]
> > Sent: Friday, November 16, 2018 6:04 PM
> > To: Martin Lund <[email protected]>; Naga Sureshkumar Relli
> > <[email protected]>
> > Cc: [email protected]; [email protected]; [email protected]; David
> > Woodhouse <[email protected]>; Brian Norris <[email protected]>; Marek
> > Vašut <[email protected]>; naga sureshkumar <[email protected]>; linux-
> > [email protected]; [email protected]; [email protected]
> > Subject: Re: [LINUX PATCH v12 1/3] dt-bindings: mtd: arasan: Add device tree binding
> > documentation
> >
> > On 16. 11. 18 12:50, Martin Lund wrote:
> > > Hi Naga,
> > >
> > > I've been working on running up the latest kernel (v4.20-rc2) on our
> > > custom Xilinx hw board so that I can test the v12 version of your
> > > Arasan nand driver.
> > >
> > > I've managed to get the driver successfully up and running and ready
> > > for testing with a Micron MT29F64G08AFAAAWP device. However, setting
> > > it up I've found a few inaccuracies in the documentation of the device
> > > tree bindings.
> > >
> > > This is the device configuration that ended up working for me with
> > > linux v4.20-rc2:
> > >
> > > nfc: nand@ff100000 {
> > > compatible = "xlnx,zynqmp-nand", "arasan,nfc-v3p10";
> > > reg = <0x0 0xff100000 0x0 0x1000>;
> > > clock-names = "clk_sys", "clk_flash";
> > > clocks = <&clk200>, <&clk100>;
> > > interrupt-parent = <&gic>;
> > > interrupts = <0 14 4>;
> > > #address-cells = <1>;
> > > #size-cells = <0>;
> > >
> > > nand@0 {
> > > reg = <0>;
> > > nand-ecc-mode = "hw";
> > > };
> > > };
> > >
> > > Compared with the example you will notice that "clock-name" should be
> > > "clock-names". reg was missing a "0x0".
> >
> > clock-names and even that names - you are right it is not correct and should be fixed.
> Clock-names I will change from "clock-name" to "clock-names".
> But I got some review comments previously, to use "sys" instead of "clk_sys" and "flash" instead of "clk_flash".
> And I have to change this in driver.
> I will update that.
>
> Thanks,
> Naga Sureshkumar Relli
>
> >
> > Missing 0x0 in reg doesn't matter because it depends on address/size cells.
> >
> > > I think it is helpful to provide a real-world working example, so you
> > > might also consider changing the example "clocks" configuration to
> > > clk200/clk100 since there is no clk_misc among the clock sources of
> > > any of the xilinx zynqmp board device tree configurations.
> >
> > Real example is the best normally just c&p from existing dts is the way to go.
> > But in connection to clocks it doesn't matter what exactly should be there and I don't think
> > there is any consistency in that. Hopefully this will be removed by yaml conversion.
> >
> > Thanks,
> > Michal
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/