diff mbox

[v3,1/5] mtd: nand: tegra: add devicetree binding

Message ID 1431282602-7137-2-git-send-email-dev@lynxeye.de
State Needs Review / ACK, archived
Headers show

Checks

Context Check Description
robh/checkpatch warning total: 1 errors, 0 warnings, 0 lines checked
robh/patch-applied success

Commit Message

Lucas Stach May 10, 2015, 6:29 p.m. UTC
This adds the devicetree binding for the Tegra 2 NAND flash
controller.

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

Comments

Brian Norris July 21, 2015, 9:05 p.m. UTC | #1
On Sun, May 10, 2015 at 08:29:58PM +0200, Lucas Stach wrote:
> This adds the devicetree binding for the Tegra 2 NAND flash
> controller.
> 
> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> ---
>  .../bindings/mtd/nvidia,tegra20-nand.txt           | 29 ++++++++++++++++++++++
>  1 file changed, 29 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 0000000..522d442
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/nvidia,tegra20-nand.txt
> @@ -0,0 +1,29 @@
> +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 properties:
> +- nvidia,wp-gpios: GPIO used to disable write protection of the flash

I think write-protect is a pretty common function, so we might want to
just remove the 'nvidia,' prefix, so we can eventually move your code to
the core nand_base.c library (BTW, I noticed you grab the GPIO, but you
don't do anything with it; is that intentional?). In fact, I've seen
requests for that very feature on the mailing list.

> +
> +  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";
> +	};

Otherwise, looks good.

Reviewed-by: Brian Norris <computersforpeace@gmail.com>

I have a few comments on the NAND driver, too.

Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lucas Stach July 22, 2015, 8:15 p.m. UTC | #2
Am Dienstag, den 21.07.2015, 14:05 -0700 schrieb Brian Norris:
> On Sun, May 10, 2015 at 08:29:58PM +0200, Lucas Stach wrote:
> > This adds the devicetree binding for the Tegra 2 NAND flash
> > controller.
> > 
> > Signed-off-by: Lucas Stach <dev@lynxeye.de>
> > ---
> >  .../bindings/mtd/nvidia,tegra20-nand.txt           | 29 ++++++++++++++++++++++
> >  1 file changed, 29 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 0000000..522d442
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/nvidia,tegra20-nand.txt
> > @@ -0,0 +1,29 @@
> > +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 properties:
> > +- nvidia,wp-gpios: GPIO used to disable write protection of the flash
> 
> I think write-protect is a pretty common function, so we might want to
> just remove the 'nvidia,' prefix, so we can eventually move your code to
> the core nand_base.c library (BTW, I noticed you grab the GPIO, but you
> don't do anything with it; is that intentional?). In fact, I've seen
> requests for that very feature on the mailing list.
> 

Actually I request and activate the GPIO in the driver, so the chip is
permanently unprotected.

I agree that it would be nice to integrate this better into the NAND
core. If it's okay for you I'll drop the nvidia prefix and follow up
with patches to move this to the core after this driver is in. I don't
really want this driver get blocked on more dependencies.

> > +
> > +  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";
> > +	};
> 
> Otherwise, looks good.
> 
> Reviewed-by: Brian Norris <computersforpeace@gmail.com>
> 
> I have a few comments on the NAND driver, too.
> 
Thanks for the review.

Regards,
Lucas


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Brian Norris July 22, 2015, 10:32 p.m. UTC | #3
On Wed, Jul 22, 2015 at 10:15:29PM +0200, Lucas Stach wrote:
> Am Dienstag, den 21.07.2015, 14:05 -0700 schrieb Brian Norris:
> > > +Optional properties:
> > > +- nvidia,wp-gpios: GPIO used to disable write protection of the flash
> > 
> > I think write-protect is a pretty common function, so we might want to
> > just remove the 'nvidia,' prefix, so we can eventually move your code to
> > the core nand_base.c library (BTW, I noticed you grab the GPIO, but you
> > don't do anything with it; is that intentional?). In fact, I've seen
> > requests for that very feature on the mailing list.
> > 
> 
> Actually I request and activate the GPIO in the driver, so the chip is
> permanently unprotected.

Whoops, I missed that part. I'm not too familiar with the GPIO API.
Thanks for clearing that up.

> I agree that it would be nice to integrate this better into the NAND
> core. If it's okay for you I'll drop the nvidia prefix and follow up
> with patches to move this to the core after this driver is in. I don't
> really want this driver get blocked on more dependencies.

Perfect. Yes, sorry for the delays, and I think most things look really
good, actually.

And I guess I wasn't clear: I'd only ask you to change the binding name
now, since we can't change that later; the code can be moved at a later
time, once the DT ABI is set. FWIW, there are other features that might
be useful alongside common NAND write-protect support. For instance, a
user might want to gain a little extra safety by having MTD bring WP#
low only when erasing/writing, so that most of the time, the flash is
protected. 

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

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 0000000..522d442
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/nvidia,tegra20-nand.txt
@@ -0,0 +1,29 @@ 
+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 properties:
+- nvidia,wp-gpios: GPIO used to disable write protection of the flash
+
+  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";
+	};