| Message ID | 20220316092525.4554-4-amhetre@nvidia.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | memory: tegra: Add MC channels and error logging | expand |
16.03.2022 12:25, Ashish Mhetre пишет: > From tegra186 onwards, memory controller support multiple channels. > Reg items are updated with address and size of these channels. > Tegra186 has overall 5 memory controller channels. Tegra194 and tegra234 > have overall 17 memory controller channels each. > There is 1 reg item for memory controller stream-id registers. > So update the reg maxItems to 18 in tegra186 devicetree documentation. > > Signed-off-by: Ashish Mhetre <amhetre@nvidia.com> > --- > .../nvidia,tegra186-mc.yaml | 20 +++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml > index 13c4c82fd0d3..3c4e231dc1de 100644 > --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml > +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml > @@ -34,8 +34,8 @@ properties: > - nvidia,tegra234-mc > > reg: > - minItems: 1 > - maxItems: 3 > + minItems: 6 > + maxItems: 18 > > interrupts: > items: > @@ -142,7 +142,8 @@ allOf: > then: > properties: > reg: > - maxItems: 1 > + maxItems: 6 > + description: 5 memory controller channels and 1 for stream-id registers > > - if: > properties: > @@ -151,7 +152,8 @@ allOf: > then: > properties: > reg: > - minItems: 3 > + minItems: 18 > + description: 17 memory controller channels and 1 for stream-id registers > > - if: > properties: > @@ -160,7 +162,8 @@ allOf: > then: > properties: > reg: > - minItems: 3 > + minItems: 18 > + description: 17 memory controller channels and 1 for stream-id registers > > additionalProperties: false > > @@ -198,7 +201,12 @@ examples: > > external-memory-controller@2c60000 { > compatible = "nvidia,tegra186-emc"; > - reg = <0x0 0x02c60000 0x0 0x50000>; > + reg = <0x0 0x02c00000 0x0 0x10000>, /* MC-SID */ > + <0x0 0x02c10000 0x0 0x10000>, /* Broadcast channel */ > + <0x0 0x02c20000 0x0 0x10000>, /* MC0 */ > + <0x0 0x02c30000 0x0 0x10000>, /* MC1 */ > + <0x0 0x02c40000 0x0 0x10000>, /* MC2 */ > + <0x0 0x02c50000 0x0 0x10000>; /* MC3 */ > interrupts = <GIC_SPI 224 IRQ_TYPE_LEVEL_HIGH>; > clocks = <&bpmp TEGRA186_CLK_EMC>; > clock-names = "emc"; This is the EMC node, not MC.
On Wed, 16 Mar 2022 14:55:24 +0530, Ashish Mhetre wrote:
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.example.dt.yaml: memory-controller@2c00000: reg: [[0, 46137344, 0, 720896]] is too short
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.example.dt.yaml: memory-controller@2c00000: external-memory-controller@2c60000:reg: [[0, 46137344, 0, 65536], [0, 46202880, 0, 65536], [0, 46268416, 0, 65536], [0, 46333952, 0, 65536], [0, 46399488, 0, 65536], [0, 46465024, 0, 65536]] is too long
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.example.dt.yaml: memory-controller@2c00000: external-memory-controller@2c60000:reg: [[0, 46137344, 0, 65536], [0, 46202880, 0, 65536], [0, 46268416, 0, 65536], [0, 46333952, 0, 65536], [0, 46399488, 0, 65536], [0, 46465024, 0, 65536]] is too long
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.example.dt.yaml: memory-controller@2c00000: reg: [[0, 46137344, 0, 720896]] is too short
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/1606062
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
On 16/03/2022 10:25, Ashish Mhetre wrote: > From tegra186 onwards, memory controller support multiple channels. > Reg items are updated with address and size of these channels. > Tegra186 has overall 5 memory controller channels. Tegra194 and tegra234 > have overall 17 memory controller channels each. > There is 1 reg item for memory controller stream-id registers. > So update the reg maxItems to 18 in tegra186 devicetree documentation. > > Signed-off-by: Ashish Mhetre <amhetre@nvidia.com> > --- > .../nvidia,tegra186-mc.yaml | 20 +++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml > index 13c4c82fd0d3..3c4e231dc1de 100644 > --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml > +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml > @@ -34,8 +34,8 @@ properties: > - nvidia,tegra234-mc > > reg: > - minItems: 1 > - maxItems: 3 > + minItems: 6 > + maxItems: 18 Still ABI break and now the in-kernel DTS will report dt check errors. I think you ignored the comments you got about breaking ABI. Best regards, Krzysztof
On 3/20/2022 6:12 PM, Krzysztof Kozlowski wrote: > External email: Use caution opening links or attachments > > > On 16/03/2022 10:25, Ashish Mhetre wrote: >> From tegra186 onwards, memory controller support multiple channels. >> Reg items are updated with address and size of these channels. >> Tegra186 has overall 5 memory controller channels. Tegra194 and tegra234 >> have overall 17 memory controller channels each. >> There is 1 reg item for memory controller stream-id registers. >> So update the reg maxItems to 18 in tegra186 devicetree documentation. >> >> Signed-off-by: Ashish Mhetre <amhetre@nvidia.com> >> --- >> .../nvidia,tegra186-mc.yaml | 20 +++++++++++++------ >> 1 file changed, 14 insertions(+), 6 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml >> index 13c4c82fd0d3..3c4e231dc1de 100644 >> --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml >> +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml >> @@ -34,8 +34,8 @@ properties: >> - nvidia,tegra234-mc >> >> reg: >> - minItems: 1 >> - maxItems: 3 >> + minItems: 6 >> + maxItems: 18 > > Still ABI break and now the in-kernel DTS will report dt check errors. > The dt check error is because I mistakenly updated example in EMC node instead of MC. I'll fix it in next version. > I think you ignored the comments you got about breaking ABI. > No, I took care of the ABI break in v5. I have updated details about how we took care of it in first patch. > Best regards, > Krzysztof
On 22/03/2022 19:12, Ashish Mhetre wrote: > > > On 3/20/2022 6:12 PM, Krzysztof Kozlowski wrote: >> External email: Use caution opening links or attachments >> >> >> On 16/03/2022 10:25, Ashish Mhetre wrote: >>> From tegra186 onwards, memory controller support multiple channels. >>> Reg items are updated with address and size of these channels. >>> Tegra186 has overall 5 memory controller channels. Tegra194 and tegra234 >>> have overall 17 memory controller channels each. >>> There is 1 reg item for memory controller stream-id registers. >>> So update the reg maxItems to 18 in tegra186 devicetree documentation. >>> >>> Signed-off-by: Ashish Mhetre <amhetre@nvidia.com> >>> --- >>> .../nvidia,tegra186-mc.yaml | 20 +++++++++++++------ >>> 1 file changed, 14 insertions(+), 6 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml >>> index 13c4c82fd0d3..3c4e231dc1de 100644 >>> --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml >>> +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml >>> @@ -34,8 +34,8 @@ properties: >>> - nvidia,tegra234-mc >>> >>> reg: >>> - minItems: 1 >>> - maxItems: 3 >>> + minItems: 6 >>> + maxItems: 18 >> >> Still ABI break and now the in-kernel DTS will report dt check errors. >> > The dt check error is because I mistakenly updated example in EMC node > instead of MC. I'll fix it in next version. The existing DTS will start failing with: nvidia,tegra186-mc.example.dtb: memory-controller@2c00000: reg: [[0, 46137344, 0, 720896]] is too short because you require now length of 6 minimum. > >> I think you ignored the comments you got about breaking ABI. >> > No, I took care of the ABI break in v5. I have updated details about > how we took care of it in first patch. Right, driver part looks ok. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml index 13c4c82fd0d3..3c4e231dc1de 100644 --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra186-mc.yaml @@ -34,8 +34,8 @@ properties: - nvidia,tegra234-mc reg: - minItems: 1 - maxItems: 3 + minItems: 6 + maxItems: 18 interrupts: items: @@ -142,7 +142,8 @@ allOf: then: properties: reg: - maxItems: 1 + maxItems: 6 + description: 5 memory controller channels and 1 for stream-id registers - if: properties: @@ -151,7 +152,8 @@ allOf: then: properties: reg: - minItems: 3 + minItems: 18 + description: 17 memory controller channels and 1 for stream-id registers - if: properties: @@ -160,7 +162,8 @@ allOf: then: properties: reg: - minItems: 3 + minItems: 18 + description: 17 memory controller channels and 1 for stream-id registers additionalProperties: false @@ -198,7 +201,12 @@ examples: external-memory-controller@2c60000 { compatible = "nvidia,tegra186-emc"; - reg = <0x0 0x02c60000 0x0 0x50000>; + reg = <0x0 0x02c00000 0x0 0x10000>, /* MC-SID */ + <0x0 0x02c10000 0x0 0x10000>, /* Broadcast channel */ + <0x0 0x02c20000 0x0 0x10000>, /* MC0 */ + <0x0 0x02c30000 0x0 0x10000>, /* MC1 */ + <0x0 0x02c40000 0x0 0x10000>, /* MC2 */ + <0x0 0x02c50000 0x0 0x10000>; /* MC3 */ interrupts = <GIC_SPI 224 IRQ_TYPE_LEVEL_HIGH>; clocks = <&bpmp TEGRA186_CLK_EMC>; clock-names = "emc";
From tegra186 onwards, memory controller support multiple channels. Reg items are updated with address and size of these channels. Tegra186 has overall 5 memory controller channels. Tegra194 and tegra234 have overall 17 memory controller channels each. There is 1 reg item for memory controller stream-id registers. So update the reg maxItems to 18 in tegra186 devicetree documentation. Signed-off-by: Ashish Mhetre <amhetre@nvidia.com> --- .../nvidia,tegra186-mc.yaml | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)