Message ID | 20220930220606.303395-1-jwerner@chromium.org |
---|---|
State | Not Applicable, archived |
Headers | show |
Series | [1/4,v5] dt-bindings: memory: Factor out common properties of LPDDR bindings | expand |
Context | Check | Description |
---|---|---|
robh/checkpatch | success | |
robh/patch-applied | success | |
robh/dtbs-check | warning | build log |
robh/dt-meta-schema | success |
On Fri, 30 Sep 2022 15:06:03 -0700, Julius Werner wrote: > The bindings for different LPDDR versions mostly use the same kinds of > properties, so in order to reduce duplication when we're adding support > for more versions, this patch creates a new lpddr-props subschema that > can be referenced by the others to define these common parts. (This will > consider a few smaller I/O width and density numbers "legal" for LPDDR3 > that are usually not used there, but this should be harmless.) > > [...] Applied, thanks! [1/4] dt-bindings: memory: Factor out common properties of LPDDR bindings https://git.kernel.org/krzk/linux-mem-ctrl/c/087cf0c5a19c638dd3b26fe7034274b38bc8db6b [2/4] dt-bindings: memory: Add numeric LPDDR compatible string variant https://git.kernel.org/krzk/linux-mem-ctrl/c/f4deb90635ec8a7dd5d5e4e931ab539edc9a9c90 [3/4] dt-bindings: memory: Add jedec,lpddr4 and jedec,lpddr5 bindings https://git.kernel.org/krzk/linux-mem-ctrl/c/f4f2f33f148b159a7a6ad74d77e715ed1328904b [4/4] dt-bindings: memory: Add jedec,lpddrX-channel binding https://git.kernel.org/krzk/linux-mem-ctrl/c/9067db882716ed5650f9342da5406795955e6f39 Best regards,
On 18/10/2022 11:10, Krzysztof Kozlowski wrote: > On Fri, 30 Sep 2022 15:06:03 -0700, Julius Werner wrote: >> The bindings for different LPDDR versions mostly use the same kinds of >> properties, so in order to reduce duplication when we're adding support >> for more versions, this patch creates a new lpddr-props subschema that >> can be referenced by the others to define these common parts. (This will >> consider a few smaller I/O width and density numbers "legal" for LPDDR3 >> that are usually not used there, but this should be harmless.) >> >> [...] > > Applied, thanks! > > [1/4] dt-bindings: memory: Factor out common properties of LPDDR bindings > https://git.kernel.org/krzk/linux-mem-ctrl/c/087cf0c5a19c638dd3b26fe7034274b38bc8db6b > [2/4] dt-bindings: memory: Add numeric LPDDR compatible string variant > https://git.kernel.org/krzk/linux-mem-ctrl/c/f4deb90635ec8a7dd5d5e4e931ab539edc9a9c90 Run checkpatch before sending patches to the mailing list... This was a v5 so I expected it ti be clean. Best regards, Krzysztof
> > [1/4] dt-bindings: memory: Factor out common properties of LPDDR bindings > > https://git.kernel.org/krzk/linux-mem-ctrl/c/087cf0c5a19c638dd3b26fe7034274b38bc8db6b > > [2/4] dt-bindings: memory: Add numeric LPDDR compatible string variant > > https://git.kernel.org/krzk/linux-mem-ctrl/c/f4deb90635ec8a7dd5d5e4e931ab539edc9a9c90 > > Run checkpatch before sending patches to the mailing list... This was a > v5 so I expected it ti be clean. Apologies, I ran checkpatch originally but forgot to run it again after the incremental updates. Looks like there's a typo in the commit message, but I see you fixed it in the version you picked up, thanks for taking care of that. So I assume you don't need me to send a v6 update, right?
On 18/10/2022 17:36, Julius Werner wrote: >>> [1/4] dt-bindings: memory: Factor out common properties of LPDDR bindings >>> https://git.kernel.org/krzk/linux-mem-ctrl/c/087cf0c5a19c638dd3b26fe7034274b38bc8db6b >>> [2/4] dt-bindings: memory: Add numeric LPDDR compatible string variant >>> https://git.kernel.org/krzk/linux-mem-ctrl/c/f4deb90635ec8a7dd5d5e4e931ab539edc9a9c90 >> >> Run checkpatch before sending patches to the mailing list... This was a >> v5 so I expected it ti be clean. > > Apologies, I ran checkpatch originally but forgot to run it again > after the incremental updates. Looks like there's a typo in the commit > message, but I see you fixed it in the version you picked up, thanks > for taking care of that. So I assume you don't need me to send a v6 > update, right? No need for v6. Best regards, Krzysztof
On 30/09/2022 18:06, Julius Werner wrote: > The bindings for different LPDDR versions mostly use the same kinds of > properties, so in order to reduce duplication when we're adding support > for more versions, this patch creates a new lpddr-props subschema that > can be referenced by the others to define these common parts. (This will > consider a few smaller I/O width and density numbers "legal" for LPDDR3 > that are usually not used there, but this should be harmless.) > > Signed-off-by: Julius Werner <jwerner@chromium.org> > Acked-by: Rob Herring <robh@kernel.org> Julius, For the future, write cover letter which describes why you are doing this. You explained the "why" some time ago in responses, but all such information should be in cover letter (plus the applicable part in the individual patches). Grepping through past emails to find "why" is unnecessary burden. Best regards, Krzysztof
> For the future, write cover letter which describes why you are doing > this. You explained the "why" some time ago in responses, but all such > information should be in cover letter (plus the applicable part in the > individual patches). Sorry, I did write a cover letter here: https://lore.kernel.org/lkml/20220831013359.1807905-1-jwerner@chromium.org/ Are you saying I should have kept resending the cover letter on every new iteration of the series? I thought since we were already discussing detail questions and there seemed to be no general concerns on the series as a whole that wouldn't be necessary, but I can keep resending it next time if you prefer.
On 26/10/2022 19:04, Julius Werner wrote: >> For the future, write cover letter which describes why you are doing >> this. You explained the "why" some time ago in responses, but all such >> information should be in cover letter (plus the applicable part in the >> individual patches). > > Sorry, I did write a cover letter here: > https://lore.kernel.org/lkml/20220831013359.1807905-1-jwerner@chromium.org/ > > Are you saying I should have kept resending the cover letter on every > new iteration of the series? I thought since we were already > discussing detail questions and there seemed to be no general concerns > on the series as a whole that wouldn't be necessary, but I can keep > resending it next time if you prefer. Yes, please sending it. Other reviewers might not read v1 and they will have the same questions... Git helps with that - git branch --edit-description (and coverFromDescription = subject in the config) Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-props.yaml b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-props.yaml new file mode 100644 index 00000000000000..02700ac3c387ec --- /dev/null +++ b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-props.yaml @@ -0,0 +1,52 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/memory-controllers/ddr/jedec,lpddr-props.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Common properties for LPDDR types + +description: + Different LPDDR types generally use the same properties and only differ in the + range of legal values for each. This file defines the common parts that can be + reused for each type. + +maintainers: + - Krzysztof Kozlowski <krzk@kernel.org> + +properties: + revision-id: + $ref: /schemas/types.yaml#/definitions/uint32-array + description: + Revision IDs read from Mode Register 6 and 7. One byte per uint32 cell (i.e. <MR6 MR7>). + maxItems: 2 + items: + minimum: 0 + maximum: 255 + + density: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Density in megabits of SDRAM chip. Decoded from Mode Register 8. + enum: + - 64 + - 128 + - 256 + - 512 + - 1024 + - 2048 + - 4096 + - 8192 + - 16384 + - 32768 + + io-width: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + IO bus width in bits of SDRAM chip. Decoded from Mode Register 8. + enum: + - 8 + - 16 + - 32 + +additionalProperties: true diff --git a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr2.yaml b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr2.yaml index 9d78f140609b6c..e5e15d288d89b2 100644 --- a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr2.yaml +++ b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr2.yaml @@ -9,6 +9,9 @@ title: LPDDR2 SDRAM compliant to JEDEC JESD209-2 maintainers: - Krzysztof Kozlowski <krzk@kernel.org> +allOf: + - $ref: jedec,lpddr-props.yaml# + properties: compatible: oneOf: @@ -41,41 +44,6 @@ properties: Property is deprecated, use revision-id instead. deprecated: true - revision-id: - $ref: /schemas/types.yaml#/definitions/uint32-array - description: | - Revision IDs read from Mode Register 6 and 7. One byte per uint32 cell (i.e. <MR6 MR7>). - minItems: 2 - maxItems: 2 - items: - minimum: 0 - maximum: 255 - - density: - $ref: /schemas/types.yaml#/definitions/uint32 - description: | - Density in megabits of SDRAM chip. Obtained from device datasheet. - enum: - - 64 - - 128 - - 256 - - 512 - - 1024 - - 2048 - - 4096 - - 8192 - - 16384 - - 32768 - - io-width: - $ref: /schemas/types.yaml#/definitions/uint32 - description: | - IO bus width in bits of SDRAM chip. Obtained from device datasheet. - enum: - - 32 - - 16 - - 8 - tRRD-min-tck: $ref: /schemas/types.yaml#/definitions/uint32 maximum: 16 @@ -168,7 +136,7 @@ required: - density - io-width -additionalProperties: false +unevaluatedProperties: false examples: - | diff --git a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr3.yaml b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr3.yaml index 48908a19473c3f..0f7ab51842ae09 100644 --- a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr3.yaml +++ b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr3.yaml @@ -9,6 +9,9 @@ title: LPDDR3 SDRAM compliant to JEDEC JESD209-3 maintainers: - Krzysztof Kozlowski <krzk@kernel.org> +allOf: + - $ref: jedec,lpddr-props.yaml# + properties: compatible: items: @@ -20,24 +23,6 @@ properties: const: 1 deprecated: true - density: - $ref: /schemas/types.yaml#/definitions/uint32 - description: | - Density in megabits of SDRAM chip. - enum: - - 4096 - - 8192 - - 16384 - - 32768 - - io-width: - $ref: /schemas/types.yaml#/definitions/uint32 - description: | - IO bus width in bits of SDRAM chip. - enum: - - 32 - - 16 - manufacturer-id: $ref: /schemas/types.yaml#/definitions/uint32 description: | @@ -45,15 +30,6 @@ properties: deprecated, manufacturer should be derived from the compatible. deprecated: true - revision-id: - $ref: /schemas/types.yaml#/definitions/uint32-array - minItems: 2 - maxItems: 2 - items: - maximum: 255 - description: | - Revision value of SDRAM chip read from Mode Registers 6 and 7. - '#size-cells': const: 0 deprecated: true @@ -206,7 +182,7 @@ required: - density - io-width -additionalProperties: false +unevaluatedProperties: false examples: - |