| Message ID | 20220219012457.2889385-1-jwerner@chromium.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v2] dt-bindings: memory: lpddr2: Adjust revision ID property to match lpddr3 | expand |
| Context | Check | Description |
|---|---|---|
| robh/patch-applied | success | |
| robh/checkpatch | warning | total: 0 errors, 1 warnings, 32 lines checked |
| robh/dtbs-check | success | |
| robh/dt-meta-schema | success |
19.02.2022 04:24, Julius Werner пишет: > Commit 3539a2 (dt-bindings: memory: lpddr2: Add revision-id properties) > added the properties `revision-id1` and `revision-id2` to the > "jedec,lpddr2" binding. The "jedec,lpddr3" binding already had a single > array property `revision-id` for the same purpose. For consistency > between related memory types, this patch deprecates the LPDDR2 > properties and instead adds a property in the same style as for LPDDR3 > to that binding. > > Signed-off-by: Julius Werner <jwerner@chromium.org> > --- Every revised version of the patch must contain changelog. > .../memory-controllers/ddr/jedec,lpddr2.yaml | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr2.yaml b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr2.yaml > index 25ed0266f6dd3d..37229738f47271 100644 > --- a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr2.yaml > +++ b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr2.yaml > @@ -30,12 +30,23 @@ properties: > maximum: 255 > description: | > Revision 1 value of SDRAM chip. Obtained from device datasheet. > + Property is deprecated, use revision-id instead. > + deprecated: true > > revision-id2: > $ref: /schemas/types.yaml#/definitions/uint32 > maximum: 255 > description: | > Revision 2 value of SDRAM chip. Obtained from device datasheet. > + Property is deprecated, use revision-id instead. > + deprecated: true > + > + revision-id: > + $ref: /schemas/types.yaml#/definitions/uint32-array > + minItems: 2 > + maxItems: 2 > + description: | > + Revision IDs read from Mode Register 6 and 7. One byte per uint32 cell (i.e. <MR6 MR7>). > > density: > $ref: /schemas/types.yaml#/definitions/uint32 > @@ -164,8 +175,7 @@ examples: > compatible = "elpida,ECB240ABACN", "jedec,lpddr2-s4"; > density = <2048>; > io-width = <32>; > - revision-id1 = <1>; > - revision-id2 = <0>; > + revision-id = <123 234>; > > tRPab-min-tck = <3>; > tRCD-min-tck = <3>; It's not enough to change only the binding. You should also update the device-trees and drivers/memory.
On 19/02/2022 02:24, Julius Werner wrote: > Commit 3539a2 (dt-bindings: memory: lpddr2: Add revision-id properties) > added the properties `revision-id1` and `revision-id2` to the > "jedec,lpddr2" binding. The "jedec,lpddr3" binding already had a single > array property `revision-id` for the same purpose. For consistency > between related memory types, this patch deprecates the LPDDR2 > properties and instead adds a property in the same style as for LPDDR3 > to that binding. > > Signed-off-by: Julius Werner <jwerner@chromium.org> > --- > .../memory-controllers/ddr/jedec,lpddr2.yaml | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr2.yaml b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr2.yaml > index 25ed0266f6dd3d..37229738f47271 100644 > --- a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr2.yaml > +++ b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr2.yaml > @@ -30,12 +30,23 @@ properties: > maximum: 255 > description: | > Revision 1 value of SDRAM chip. Obtained from device datasheet. > + Property is deprecated, use revision-id instead. > + deprecated: true > > revision-id2: > $ref: /schemas/types.yaml#/definitions/uint32 > maximum: 255 > description: | > Revision 2 value of SDRAM chip. Obtained from device datasheet. > + Property is deprecated, use revision-id instead. > + deprecated: true > + > + revision-id: > + $ref: /schemas/types.yaml#/definitions/uint32-array > + minItems: 2 > + maxItems: 2 You need maximum value under items. See: Documentation/devicetree/bindings/arm/l2c2x0.yaml > + description: | > + Revision IDs read from Mode Register 6 and 7. One byte per uint32 cell (i.e. <MR6 MR7>). > > density: > $ref: /schemas/types.yaml#/definitions/uint32 > @@ -164,8 +175,7 @@ examples: > compatible = "elpida,ECB240ABACN", "jedec,lpddr2-s4"; > density = <2048>; > io-width = <32>; > - revision-id1 = <1>; > - revision-id2 = <0>; > + revision-id = <123 234>; Don't change the original values. Plus what Dmitry pointed out. > > tRPab-min-tck = <3>; > tRCD-min-tck = <3>; Best regards, Krzysztof
> > + revision-id: > > + $ref: /schemas/types.yaml#/definitions/uint32-array > > + minItems: 2 > > + maxItems: 2 > > You need maximum value under items. See: > Documentation/devicetree/bindings/arm/l2c2x0.yaml Sorry, can you clarify how this is supposed to be? Do you want revision-id: minItems: 2 maxItems: 2 items: minItems: 2 maxItems: 2 or just revision-id: items: minItems: 2 maxItems: 2 I see examples of both in the file you linked (and also examples of what my original patch did). Is there any authoritative documentation somewhere I can read that specifies which of those is correct? (I tried looking at https://json-schema.org/understanding-json-schema/reference/array.html#length but I'm not sure if that's relevant here.) For updating existing DTSes, do you want that in the same patch or a separate patch in a series?
On 23/02/2022 03:44, Julius Werner wrote: >>> + revision-id: >>> + $ref: /schemas/types.yaml#/definitions/uint32-array >>> + minItems: 2 >>> + maxItems: 2 >> >> You need maximum value under items. See: >> Documentation/devicetree/bindings/arm/l2c2x0.yaml > > Sorry, can you clarify how this is supposed to be? Do you want > > revision-id: > minItems: 2 > maxItems: 2 > items: > minItems: 2 > maxItems: 2 > > or just > > revision-id: > items: > minItems: 2 > maxItems: 2 > > I see examples of both in the file you linked (and also examples of > what my original patch did). There is no example of the first case in linked file. I am not sure if it is correct even... I did not ask about maximum number of items, but "maximum value", so like this: https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/arm/l2c2x0.yaml#L73 > Is there any authoritative documentation > somewhere I can read that specifies which of those is correct? (I > tried looking at > https://json-schema.org/understanding-json-schema/reference/array.html#length > but I'm not sure if that's relevant here.) example-schema.yaml is the best, but it might not cover that part. We need more docs, I know... > For updating existing DTSes, do you want that in the same patch or a > separate patch in a series? Separate patch in this series, please. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr2.yaml b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr2.yaml index 25ed0266f6dd3d..37229738f47271 100644 --- a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr2.yaml +++ b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr2.yaml @@ -30,12 +30,23 @@ properties: maximum: 255 description: | Revision 1 value of SDRAM chip. Obtained from device datasheet. + Property is deprecated, use revision-id instead. + deprecated: true revision-id2: $ref: /schemas/types.yaml#/definitions/uint32 maximum: 255 description: | Revision 2 value of SDRAM chip. Obtained from device datasheet. + Property is deprecated, use revision-id instead. + deprecated: true + + revision-id: + $ref: /schemas/types.yaml#/definitions/uint32-array + minItems: 2 + maxItems: 2 + description: | + Revision IDs read from Mode Register 6 and 7. One byte per uint32 cell (i.e. <MR6 MR7>). density: $ref: /schemas/types.yaml#/definitions/uint32 @@ -164,8 +175,7 @@ examples: compatible = "elpida,ECB240ABACN", "jedec,lpddr2-s4"; density = <2048>; io-width = <32>; - revision-id1 = <1>; - revision-id2 = <0>; + revision-id = <123 234>; tRPab-min-tck = <3>; tRCD-min-tck = <3>;
Commit 3539a2 (dt-bindings: memory: lpddr2: Add revision-id properties) added the properties `revision-id1` and `revision-id2` to the "jedec,lpddr2" binding. The "jedec,lpddr3" binding already had a single array property `revision-id` for the same purpose. For consistency between related memory types, this patch deprecates the LPDDR2 properties and instead adds a property in the same style as for LPDDR3 to that binding. Signed-off-by: Julius Werner <jwerner@chromium.org> --- .../memory-controllers/ddr/jedec,lpddr2.yaml | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)