All of lore.kernel.org
 help / color / mirror / Atom feed
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: <Tudor.Ambarus@microchip.com>
Cc: <robh+dt@kernel.org>, <devicetree@vger.kernel.org>,
	<monstr@monstr.eu>, <thomas.petazzoni@bootlin.com>,
	<broonie@kernel.org>, <linux-spi@vger.kernel.org>,
	<richard@nod.at>, <vigneshr@ti.com>, <p.yadav@ti.com>,
	<michael@walle.cc>, <linux-mtd@lists.infradead.org>
Subject: Re: [PATCH v5 2/3] spi: dt-bindings: Describe stacked/parallel memories modes
Date: Wed, 22 Dec 2021 09:05:42 +0100	[thread overview]
Message-ID: <20211222090542.43dfe12e@xps13> (raw)
In-Reply-To: <a11a0650-4624-0a9f-d0a5-c45393fead7c@microchip.com>

Hello Tudor,

Tudor.Ambarus@microchip.com wrote on Wed, 22 Dec 2021 07:52:44 +0000:

> On 12/21/21 7:00 PM, Miquel Raynal wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > Describe two new memories modes:
> > - A stacked mode when the bus is common but the address space extended
> >   with an additinals wires.
> > - A parallel mode with parallel busses accessing parallel flashes where
> >   the data is spread.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> > 
> > Hello Rob,
> > 
> > I know the below does not pass the tests (at least the example patch 3
> > does not pass) but I believe the issue is probably on the tooling side
> > because the exact same thing with uing32-array instead is accepted. The
> > problem comes from the minItems/maxItems lines. Without them, this is
> > okay. The maxItems btw matches the "good enough value for now" idea.
> > 
> > The errors I get are:
> > 
> > $ make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/spi-controller.yaml
> >   LINT    Documentation/devicetree/bindings
> >   CHKDT   Documentation/devicetree/bindings/processed-schema-examples.json
> >   SCHEMA  Documentation/devicetree/bindings/processed-schema-examples.json
> >   DTEX    Documentation/devicetree/bindings/spi/spi-controller.example.dts
> >   DTC     Documentation/devicetree/bindings/spi/spi-controller.example.dt.yaml
> >   CHECK   Documentation/devicetree/bindings/spi/spi-controller.example.dt.yaml
> > /src/Documentation/devicetree/bindings/spi/spi-controller.example.dt.yaml: spi@80010000: flash@2:stacked-memories: [[268435456, 268435456]] is too short
> >         From schema: /src/Documentation/devicetree/bindings/spi/spi-controller.yaml
> > /src/Documentation/devicetree/bindings/spi/spi-controller.example.dt.yaml: spi@80010000: flash@2:stacked-memories: [[268435456, 268435456]] is too short
> >         From schema: /src/Documentation/devicetree/bindings/spi/mxs-spi.yaml
> > /src/Documentation/devicetree/bindings/spi/spi-controller.example.dt.yaml: spi@80010000: Unevaluated properties are not allowed ('#address-cells', '#size-cells', 'display@0', 'sensor@1', 'flash@2' were unexpected)
> >         From schema: /src/Documentation/devicetree/bindings/spi/mxs-spi.yaml
> > /src/Documentation/devicetree/bindings/spi/spi-controller.example.dt.yaml: flash@2: stacked-memories: [[268435456, 268435456]] is too short
> >         From schema: /src/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > 
> > 
> >  .../bindings/spi/spi-peripheral-props.yaml    | 25 +++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> > index 5dd209206e88..fedb7ae98ff6 100644
> > --- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> > +++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> > @@ -82,6 +82,31 @@ properties:
> >      description:
> >        Delay, in microseconds, after a write transfer.
> > 
> > +  stacked-memories:
> > +    description: Several SPI memories can be wired in stacked mode.
> > +      This basically means that either a device features several chip
> > +      selects, or that different devices must be seen as a single
> > +      bigger chip. This basically doubles (or more) the total address
> > +      space with only a single additional wire, while still needing
> > +      to repeat the commands when crossing a chip boundary. The size of
> > +      each chip should be provided as members of the array.
> > +    $ref: /schemas/types.yaml#/definitions/uint64-array
> > +    minItems: 2
> > +    maxItems: 4  
> 
> Why do we define maxItems? Can't we remove this restriction?

Rob usually prefers to bound properties, that's why we often see "good
enough values for now" in the bindings. If it's no longer the case it's
fine to drop the maxItems property.

> > +
> > +  parallel-memories:
> > +    description: Several SPI memories can be wired in parallel mode.
> > +      The devices are physically on a different buses but will always
> > +      act synchronously as each data word is spread across the
> > +      different memories (eg. even bits are stored in one memory, odd
> > +      bits in the other). This basically doubles the address space and
> > +      the throughput while greatly complexifying the wiring because as
> > +      many busses as devices must be wired. The size of each chip should
> > +      be provided as members of the array.
> > +    $ref: /schemas/types.yaml#/definitions/uint64-array
> > +    minItems: 2
> > +    maxItems: 4
> > +
> >  # The controller specific properties go here.
> >  allOf:
> >    - $ref: cdns,qspi-nor-peripheral-props.yaml#
> > --
> > 2.27.0
> >   
> 


Thanks,
Miquèl

WARNING: multiple messages have this Message-ID (diff)
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: <Tudor.Ambarus@microchip.com>
Cc: <robh+dt@kernel.org>, <devicetree@vger.kernel.org>,
	<monstr@monstr.eu>, <thomas.petazzoni@bootlin.com>,
	<broonie@kernel.org>, <linux-spi@vger.kernel.org>,
	<richard@nod.at>, <vigneshr@ti.com>, <p.yadav@ti.com>,
	<michael@walle.cc>, <linux-mtd@lists.infradead.org>
Subject: Re: [PATCH v5 2/3] spi: dt-bindings: Describe stacked/parallel memories modes
Date: Wed, 22 Dec 2021 09:05:42 +0100	[thread overview]
Message-ID: <20211222090542.43dfe12e@xps13> (raw)
In-Reply-To: <a11a0650-4624-0a9f-d0a5-c45393fead7c@microchip.com>

Hello Tudor,

Tudor.Ambarus@microchip.com wrote on Wed, 22 Dec 2021 07:52:44 +0000:

> On 12/21/21 7:00 PM, Miquel Raynal wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > Describe two new memories modes:
> > - A stacked mode when the bus is common but the address space extended
> >   with an additinals wires.
> > - A parallel mode with parallel busses accessing parallel flashes where
> >   the data is spread.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> > 
> > Hello Rob,
> > 
> > I know the below does not pass the tests (at least the example patch 3
> > does not pass) but I believe the issue is probably on the tooling side
> > because the exact same thing with uing32-array instead is accepted. The
> > problem comes from the minItems/maxItems lines. Without them, this is
> > okay. The maxItems btw matches the "good enough value for now" idea.
> > 
> > The errors I get are:
> > 
> > $ make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/spi-controller.yaml
> >   LINT    Documentation/devicetree/bindings
> >   CHKDT   Documentation/devicetree/bindings/processed-schema-examples.json
> >   SCHEMA  Documentation/devicetree/bindings/processed-schema-examples.json
> >   DTEX    Documentation/devicetree/bindings/spi/spi-controller.example.dts
> >   DTC     Documentation/devicetree/bindings/spi/spi-controller.example.dt.yaml
> >   CHECK   Documentation/devicetree/bindings/spi/spi-controller.example.dt.yaml
> > /src/Documentation/devicetree/bindings/spi/spi-controller.example.dt.yaml: spi@80010000: flash@2:stacked-memories: [[268435456, 268435456]] is too short
> >         From schema: /src/Documentation/devicetree/bindings/spi/spi-controller.yaml
> > /src/Documentation/devicetree/bindings/spi/spi-controller.example.dt.yaml: spi@80010000: flash@2:stacked-memories: [[268435456, 268435456]] is too short
> >         From schema: /src/Documentation/devicetree/bindings/spi/mxs-spi.yaml
> > /src/Documentation/devicetree/bindings/spi/spi-controller.example.dt.yaml: spi@80010000: Unevaluated properties are not allowed ('#address-cells', '#size-cells', 'display@0', 'sensor@1', 'flash@2' were unexpected)
> >         From schema: /src/Documentation/devicetree/bindings/spi/mxs-spi.yaml
> > /src/Documentation/devicetree/bindings/spi/spi-controller.example.dt.yaml: flash@2: stacked-memories: [[268435456, 268435456]] is too short
> >         From schema: /src/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > 
> > 
> >  .../bindings/spi/spi-peripheral-props.yaml    | 25 +++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> > index 5dd209206e88..fedb7ae98ff6 100644
> > --- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> > +++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> > @@ -82,6 +82,31 @@ properties:
> >      description:
> >        Delay, in microseconds, after a write transfer.
> > 
> > +  stacked-memories:
> > +    description: Several SPI memories can be wired in stacked mode.
> > +      This basically means that either a device features several chip
> > +      selects, or that different devices must be seen as a single
> > +      bigger chip. This basically doubles (or more) the total address
> > +      space with only a single additional wire, while still needing
> > +      to repeat the commands when crossing a chip boundary. The size of
> > +      each chip should be provided as members of the array.
> > +    $ref: /schemas/types.yaml#/definitions/uint64-array
> > +    minItems: 2
> > +    maxItems: 4  
> 
> Why do we define maxItems? Can't we remove this restriction?

Rob usually prefers to bound properties, that's why we often see "good
enough values for now" in the bindings. If it's no longer the case it's
fine to drop the maxItems property.

> > +
> > +  parallel-memories:
> > +    description: Several SPI memories can be wired in parallel mode.
> > +      The devices are physically on a different buses but will always
> > +      act synchronously as each data word is spread across the
> > +      different memories (eg. even bits are stored in one memory, odd
> > +      bits in the other). This basically doubles the address space and
> > +      the throughput while greatly complexifying the wiring because as
> > +      many busses as devices must be wired. The size of each chip should
> > +      be provided as members of the array.
> > +    $ref: /schemas/types.yaml#/definitions/uint64-array
> > +    minItems: 2
> > +    maxItems: 4
> > +
> >  # The controller specific properties go here.
> >  allOf:
> >    - $ref: cdns,qspi-nor-peripheral-props.yaml#
> > --
> > 2.27.0
> >   
> 


Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

  reply	other threads:[~2021-12-22  8:05 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-21 17:00 [PATCH v5 0/3] Stacked/parallel memories bindings Miquel Raynal
2021-12-21 17:00 ` Miquel Raynal
2021-12-21 17:00 ` [PATCH v5 1/3] dt-bindings: mtd: spi-nor: Allow two CS per device Miquel Raynal
2021-12-21 17:00   ` Miquel Raynal
2021-12-21 18:47   ` Pratyush Yadav
2021-12-21 18:47     ` Pratyush Yadav
2021-12-22  8:23     ` Miquel Raynal
2021-12-22  8:23       ` Miquel Raynal
2021-12-22  8:33       ` Pratyush Yadav
2021-12-22  8:33         ` Pratyush Yadav
2021-12-22  8:41       ` Miquel Raynal
2021-12-22  8:41         ` Miquel Raynal
2021-12-21 17:00 ` [PATCH v5 2/3] spi: dt-bindings: Describe stacked/parallel memories modes Miquel Raynal
2021-12-21 17:00   ` Miquel Raynal
2021-12-21 18:45   ` Pratyush Yadav
2021-12-21 18:45     ` Pratyush Yadav
2021-12-22  7:52   ` Tudor.Ambarus
2021-12-22  7:52     ` Tudor.Ambarus
2021-12-22  8:05     ` Miquel Raynal [this message]
2021-12-22  8:05       ` Miquel Raynal
2021-12-22  8:22       ` Tudor.Ambarus
2021-12-22  8:22         ` Tudor.Ambarus
2021-12-22  8:35         ` Miquel Raynal
2021-12-22  8:35           ` Miquel Raynal
2021-12-22  8:44           ` Tudor.Ambarus
2021-12-22  8:44             ` Tudor.Ambarus
2021-12-22  8:53             ` Miquel Raynal
2021-12-22  8:53               ` Miquel Raynal
2021-12-22 19:30           ` Rob Herring
2021-12-22 19:30             ` Rob Herring
2021-12-22 19:08   ` Rob Herring
2021-12-22 19:08     ` Rob Herring
2021-12-22 19:28     ` Rob Herring
2021-12-22 19:28       ` Rob Herring
2021-12-21 17:00 ` [PATCH v5 3/3] spi: dt-bindings: Add an example with two stacked flashes Miquel Raynal
2021-12-21 17:00   ` Miquel Raynal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211222090542.43dfe12e@xps13 \
    --to=miquel.raynal@bootlin.com \
    --cc=Tudor.Ambarus@microchip.com \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=michael@walle.cc \
    --cc=monstr@monstr.eu \
    --cc=p.yadav@ti.com \
    --cc=richard@nod.at \
    --cc=robh+dt@kernel.org \
    --cc=thomas.petazzoni@bootlin.com \
    --cc=vigneshr@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.