| Message ID | 20211221170058.18333-3-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Stacked/parallel memories bindings | expand |
On 21/12/21 06:00PM, Miquel Raynal wrote: > 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> Acked-by: Pratyush Yadav <p.yadav@ti.com>
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? > + > + 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 >
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
On 12/22/21 10:05 AM, Miquel Raynal wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Hello Tudor, Hi! > > 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 right, I saw it. > fine to drop the maxItems property. There's no such hardware limitation as far as I know, that's why I've asked. Maybe Rob can advise. Cheers, ta > >>> + >>> + 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 >
Hi Tudor, Tudor.Ambarus@microchip.com wrote on Wed, 22 Dec 2021 08:22:05 +0000: > On 12/22/21 10:05 AM, Miquel Raynal wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > Hello Tudor, > > Hi! > > > > > 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 > > right, I saw it. > > > fine to drop the maxItems property. > > There's no such hardware limitation as far as I know, that's why I've > asked. Maybe Rob can advise. Yes, I'll follow what Rob thinks its best: - keeping maxItems: 4 (as it is), which is a good enough value - dropping the maxItems here because in the end no bounding is necessary - using maxItems: 2 to match the SPI CS even though in theory these two numbers are not correlated (stacked-memories might very well be used by other non SPI memories as well). BTW if you're fine with the proposal your Ack is welcome ;) Thanks, Miquèl
On 12/22/21 10:35 AM, Miquel Raynal wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Hi Tudor, > > Tudor.Ambarus@microchip.com wrote on Wed, 22 Dec 2021 08:22:05 +0000: >> On 12/22/21 10:05 AM, Miquel Raynal wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>> >>> Hello Tudor, >> >> Hi! >> >>> >>> 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 >> >> right, I saw it. >> >>> fine to drop the maxItems property. >> >> There's no such hardware limitation as far as I know, that's why I've >> asked. Maybe Rob can advise. > > Yes, I'll follow what Rob thinks its best: > - keeping maxItems: 4 (as it is), which is a good enough value > - dropping the maxItems here because in the end no bounding is necessary Then I would drop maxItems for stacked-memories. For parallel-memories: does the maxItems depend on the number of I/O lines? > - using maxItems: 2 to match the SPI CS even though in theory these two > numbers are not correlated (stacked-memories might very well be > used by other non SPI memories as well). > > BTW if you're fine with the proposal your Ack is welcome ;) > > Thanks, > Miquèl >
Hi Tudor, Tudor.Ambarus@microchip.com wrote on Wed, 22 Dec 2021 08:44:16 +0000: > On 12/22/21 10:35 AM, Miquel Raynal wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > Hi Tudor, > > > > Tudor.Ambarus@microchip.com wrote on Wed, 22 Dec 2021 08:22:05 +0000: > >> On 12/22/21 10:05 AM, Miquel Raynal wrote: > >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > >>> > >>> Hello Tudor, > >> > >> Hi! > >> > >>> > >>> 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 > >> > >> right, I saw it. > >> > >>> fine to drop the maxItems property. > >> > >> There's no such hardware limitation as far as I know, that's why I've > >> asked. Maybe Rob can advise. > > > > Yes, I'll follow what Rob thinks its best: > > - keeping maxItems: 4 (as it is), which is a good enough value > > - dropping the maxItems here because in the end no bounding is necessary > Then I would drop maxItems for stacked-memories. For parallel-memories: > does the maxItems depend on the number of I/O lines? Well, if you look into controller constraints, I bet most of them will not support more than 8 data lines for now. For example, Xilinx QSPI controller accepts up to two devices with 4 data lines on each. But I believe it would be completely possible to use 4 devices with 2 data lines or up to 8 with one as well. This is pure theory, I haven't seen nor heard about such hosts so far, so let's wait for Rob advice on that number and see what he has in mind. > > - using maxItems: 2 to match the SPI CS even though in theory these two > > numbers are not correlated (stacked-memories might very well be > > used by other non SPI memories as well). > > > > BTW if you're fine with the proposal your Ack is welcome ;) > > > > Thanks, > > Miquèl > > > Thanks, Miquèl
On Tue, Dec 21, 2021 at 06:00:57PM +0100, Miquel Raynal wrote: > 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 I'm not seeing any of these. Are you up to date with dtschema? Rob
On Wed, Dec 22, 2021 at 03:08:59PM -0400, Rob Herring wrote: > On Tue, Dec 21, 2021 at 06:00:57PM +0100, Miquel Raynal wrote: > > 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 > > I'm not seeing any of these. Are you up to date with dtschema? NM, I was missing a patch and now see it. It is indeed a tool problem. I'll work on a fix. Rob
On Wed, Dec 22, 2021 at 09:35:58AM +0100, Miquel Raynal wrote: > Hi Tudor, > > Tudor.Ambarus@microchip.com wrote on Wed, 22 Dec 2021 08:22:05 +0000: > > On 12/22/21 10:05 AM, Miquel Raynal wrote: > > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > > > Hello Tudor, > > > > Hi! > > > > > > > > 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 > > > > right, I saw it. > > > > > fine to drop the maxItems property. > > > > There's no such hardware limitation as far as I know, that's why I've > > asked. Maybe Rob can advise. > > Yes, I'll follow what Rob thinks its best: > - keeping maxItems: 4 (as it is), which is a good enough value That's what I already suggested, though I would have expected a bigger value. For example, something more than the cost or electrical limit of what's practical. I don't know that is though. We don't want to be updating it frequently. > - dropping the maxItems here because in the end no bounding is necessary This will implicitly set the max to 2 based on minItems. That's because most of the time we want an exact number of entries. > - using maxItems: 2 to match the SPI CS even though in theory these two > numbers are not correlated (stacked-memories might very well be > used by other non SPI memories as well). > > BTW if you're fine with the proposal your Ack is welcome ;) > > Thanks, > Miquèl >
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 + + 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#
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(+)