linux-kernel - Re: [PATCH v2] MTD: Onenand: Add device tree support for samsung onenand

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130923140823.GC16069@e106331-lin.cambridge.arm.com>
Date:	Mon, 23 Sep 2013 15:08:23 +0100
From:	Mark Rutland <mark.rutland@....com>
To:	Mateusz Krawczuk <m.krawczuk@...tner.samsung.com>
Cc:	"kyungmin.park@...sung.com" <kyungmin.park@...sung.com>,
	"dwmw2@...radead.org" <dwmw2@...radead.org>,
	"grant.likely@...aro.org" <grant.likely@...aro.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
	"rob.herring@...xeda.com" <rob.herring@...xeda.com>,
	Pawel Moll <Pawel.Moll@....com>,
	"swarren@...dotorg.org" <swarren@...dotorg.org>,
	"ijc+devicetree@...lion.org.uk" <ijc+devicetree@...lion.org.uk>,
	"rob@...dley.net" <rob@...dley.net>,
	"linux-samsung-soc@...r.kernel.org" 
	<linux-samsung-soc@...r.kernel.org>,
	"t.figa@...sung.com" <t.figa@...sung.com>,
	"m.szyprowski@...sung.com" <m.szyprowski@...sung.com>,
	"b.zolnierkie@...sung.com" <b.zolnierkie@...sung.com>
Subject: Re: [PATCH v2] MTD: Onenand: Add device tree support for samsung
 onenand

On Mon, Sep 23, 2013 at 02:06:48PM +0100, Mateusz Krawczuk wrote:
> This patch add clk and device tree nodes for samsung onenand driver.
> 
> since v1:
> Updated Documentation according to Mark Rutland notes.
> 
> Signed-off-by: Mateusz Krawczuk <m.krawczuk@...tner.samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@...sung.com>
> ---
>  .../devicetree/bindings/mtd/samsung-onenand.txt    | 44 ++++++++++++++++++++++
>  drivers/mtd/onenand/samsung.c                      | 37 +++++++++++++++++-
>  2 files changed, 80 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/mtd/samsung-onenand.txt
> 
> diff --git a/Documentation/devicetree/bindings/mtd/samsung-onenand.txt b/Documentation/devicetree/bindings/mtd/samsung-onenand.txt
> new file mode 100644
> index 0000000..5bf931c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/samsung-onenand.txt
> @@ -0,0 +1,44 @@
> +Device tree bindings for Samsung Onenand
> +
> +Required properties:
> +  - compatible: value should be either of the following.
> +      (a) "samsung,s3c6400-onenand",
> +		for onenand controller compatible with s3c6400.
> +      (b) "samsung,s3c6410-onenand",
> +		for onenand controller compatible with s3c6410.
> +      (c) "samsung,s5pc100-onenand",
> +		for onenand controller compatible with s5pc100.
> +      (d) "samsung,s5pc110-onenand",
> +      for s5pc100-like onenand controller used on s5pc110 which supports DMA.
> +

As I asked on the last posting, what are the differences between these
implementations?

> +Required properties for s5pc110:
> +
> + - reg: the offset and length of the control registers. First region describes
> +	OneNAND interface, second control registers.

Also, the complete reg description is a bit confusing. How about
something like:

 - reg: two register specifiers:
        [0] - The OneNAND interface
	[1] - The control registers

Do we expect future OneNAND devices which may require more reg entries
to describe?

> + - interrupt-parent, interrupts		Onenand memory interrupts

As it's a common property, I don't think interrupt-parent needs to
be described.

Is there more than one interrupt? What's it called on the manual?

> +
> +Required properties for others:
> +
> + - reg: the offset and length of the control registers. First region describes
> +	control registers, second OneNAND interface.

Why does the s5pc110 OneNAND binding take its registers in the opposite
order to the rest of these? Can we not fix up the driver first to make
it consistent? Then we only need to describe this once and it's going to
be far less of a headache to support.

> +
> +Clocks:
> + - gate - clock which output is supplied to external OneNAND flash memory.

How about the following (without the Clocks header):

- clocks: clock-specifiers for the clocks named in clock-names, per the
          common clock bindings.

- clock-names: should contain "gate" for the clock to the OneNAND flash
               memory.

Is this the only clock that might be necessary on all platforms?

> +
> +
> +For partiton table parsing (optional) please refer to:
> + [1] Documentation/devicetree/bindings/mtd/partition.txt
> +
> +Example for an s5pv210 board:
> +
> +		onenand@...00000 {
> +			compatible = "samsung,s5pc110-onenand";
> +			reg = <0xb0000000 0x20000>, <0xb0600000 0x2000>;
> +			interrupt-parent = <&vic1>;
> +			interrupts = <31>;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			clocks = <&clocks NANDXL>;
> +			clock-names = "gate";
> +			status = "okay";
> +		};

Cheers,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/