Re: [PATCH 1/2] dt-bindings: mtd: Altera ASMI Parallel II IP Core

[matthew . gerlach]

Hi Rob,

Thanks for the feedback.  Please see my comments in line.

Matthew Gerlach

On Thu, 10 Aug 2017, Rob Herring wrote:

> On Sun, Aug 06, 2017 at 11:24:02AM -0700, matthew.gerlach@xxxxxxxxxxxxxxx wrote:
> > From: Matthew Gerlach <matthew.gerlach@xxxxxxxxxxxxxxx>
> >
> > Device Tree bindinds for Altera ASMI Parallel II IP Core.
>
> s/bindinds/bindings/
>
> > Signed-off-by: Matthew Gerlach <matthew.gerlach@xxxxxxxxxxxxxxx>
> > ---
> >  .../devicetree/bindings/mtd/altera-asmip2.txt      | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mtd/altera-asmip2.txt
> >
> > diff --git a/Documentation/devicetree/bindings/mtd/altera-asmip2.txt b/Documentation/devicetree/bindings/mtd/altera-asmip2.txt
> > new file mode 100644
> > index 0000000..3380366
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/altera-asmip2.txt
> > @@ -0,0 +1,22 @@
> > +* Altera ASMI Parallel II IP Core
> > +
> > +Required properties:
> > +- compatible : Should be "altr,asmi_parallel2".
>
> s/_/-/
>
> > +- reg : A tuple consisting of a physical address and length.
> > +
> > +Optional subnodes:
> > +Subnodes of the Altera ASMI Paralllel II IP Core are spi slave nodes.
>
> But this is not a general purpose SPI controller?


The component is a general pupose SPI controller, but this driver
expects a spi-nor device to be connected to it.  As such I wonder if the
compatible string should be "altr,asmi-parallel2-spi-nor".


> > +- reg : chip select value
> > +- label : optional user friendly label
> > +
> > +Example:
> > +
> > +qspi: spi@a0001000 {
> > +	compatible = "altr,asmi_parallel2";
> > +	reg = <0xa0001000 0x10>;
> > +
> > +	flash@0 {
>
> This should have a compatible.
>
> > +		reg = <0>;
> > +		label = "FPGA Image";
> > +	};

Since I have no way to test multiple flash chips, it might
be simpler to not specify any subnodes.

> > +};
> > --
> > 2.7.4