diff mbox series

[v6,09/52] dt-bindings: memory: tegra30: mc: Document new interconnect property

Message ID 20201025221735.3062-10-digetx@gmail.com
State Superseded
Headers show
Series [v6,01/52] clk: tegra: Export Tegra20 EMC kernel symbols | expand

Commit Message

Dmitry Osipenko Oct. 25, 2020, 10:16 p.m. UTC
Memory controller is interconnected with memory clients and with the
External Memory Controller. Document new interconnect property which
turns memory controller into interconnect provider.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 .../bindings/memory-controllers/nvidia,tegra30-mc.yaml       | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Krzysztof Kozlowski Oct. 27, 2020, 9:05 a.m. UTC | #1
On Mon, Oct 26, 2020 at 01:16:52AM +0300, Dmitry Osipenko wrote:
> Memory controller is interconnected with memory clients and with the
> External Memory Controller. Document new interconnect property which
> turns memory controller into interconnect provider.
> 
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  .../bindings/memory-controllers/nvidia,tegra30-mc.yaml       | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml
> index 84fd57bcf0dc..5436e6d420bc 100644
> --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml
> +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml
> @@ -57,6 +57,9 @@ properties:
>    "#iommu-cells":
>      const: 1
>  
> +  "#interconnect-cells":
> +    const: 1
> +
>  patternProperties:
>    "^emc-timings-[0-9]+$":
>      type: object
> @@ -120,6 +123,7 @@ required:
>    - clock-names
>    - "#reset-cells"
>    - "#iommu-cells"
> +  - "#interconnect-cells"

Rob,

You were fine with adding a new required property which breaks all
existing DTBs?

Were these bindings marked as unstable? The patchset does not even
say/scream that it breaks the ABI, so this might be quite a surprise for
someone...

Best regards,
Krzysztof
Dmitry Osipenko Oct. 27, 2020, 7:18 p.m. UTC | #2
27.10.2020 12:05, Krzysztof Kozlowski пишет:
> On Mon, Oct 26, 2020 at 01:16:52AM +0300, Dmitry Osipenko wrote:
>> Memory controller is interconnected with memory clients and with the
>> External Memory Controller. Document new interconnect property which
>> turns memory controller into interconnect provider.
>>
>> Acked-by: Rob Herring <robh@kernel.org>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  .../bindings/memory-controllers/nvidia,tegra30-mc.yaml       | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml
>> index 84fd57bcf0dc..5436e6d420bc 100644
>> --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml
>> +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml
>> @@ -57,6 +57,9 @@ properties:
>>    "#iommu-cells":
>>      const: 1
>>  
>> +  "#interconnect-cells":
>> +    const: 1
>> +
>>  patternProperties:
>>    "^emc-timings-[0-9]+$":
>>      type: object
>> @@ -120,6 +123,7 @@ required:
>>    - clock-names
>>    - "#reset-cells"
>>    - "#iommu-cells"
>> +  - "#interconnect-cells"
> 
> Rob,
> 
> You were fine with adding a new required property which breaks all
> existing DTBs?

This is a required property for the new bindings and optional for the
older. Does it really need to be made optional in the binding?

> Were these bindings marked as unstable? The patchset does not even
> say/scream that it breaks the ABI, so this might be quite a surprise for
> someone...

Please see tegra_mc_interconnect_setup() in "memory: tegra-mc: Add
interconnect framework" patch, which check presence of the new ICC DT
property.
Krzysztof Kozlowski Oct. 27, 2020, 7:39 p.m. UTC | #3
On Tue, Oct 27, 2020 at 10:18:35PM +0300, Dmitry Osipenko wrote:
> 27.10.2020 12:05, Krzysztof Kozlowski пишет:
> > On Mon, Oct 26, 2020 at 01:16:52AM +0300, Dmitry Osipenko wrote:
> >> Memory controller is interconnected with memory clients and with the
> >> External Memory Controller. Document new interconnect property which
> >> turns memory controller into interconnect provider.
> >>
> >> Acked-by: Rob Herring <robh@kernel.org>
> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> >> ---
> >>  .../bindings/memory-controllers/nvidia,tegra30-mc.yaml       | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml
> >> index 84fd57bcf0dc..5436e6d420bc 100644
> >> --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml
> >> +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml
> >> @@ -57,6 +57,9 @@ properties:
> >>    "#iommu-cells":
> >>      const: 1
> >>  
> >> +  "#interconnect-cells":
> >> +    const: 1
> >> +
> >>  patternProperties:
> >>    "^emc-timings-[0-9]+$":
> >>      type: object
> >> @@ -120,6 +123,7 @@ required:
> >>    - clock-names
> >>    - "#reset-cells"
> >>    - "#iommu-cells"
> >> +  - "#interconnect-cells"
> > 
> > Rob,
> > 
> > You were fine with adding a new required property which breaks all
> > existing DTBs?
> 
> This is a required property for the new bindings and optional for the
> older. Does it really need to be made optional in the binding?

Mhmm... that's an interesting point. I assumed that the bindings should
reflect current status of the ABI, but I could imagine that you update
the bindings while keeping the driver working with older DTBs.

How do you actually track then the ABI? If incompatible change can be
added to the bindings, later anyone anytime can also update the driver
to enforce the bindings. To require such property.

Best regards,
Krzysztof


> 
> > Were these bindings marked as unstable? The patchset does not even
> > say/scream that it breaks the ABI, so this might be quite a surprise for
> > someone...
> 
> Please see tegra_mc_interconnect_setup() in "memory: tegra-mc: Add
> interconnect framework" patch, which check presence of the new ICC DT
> property.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml
index 84fd57bcf0dc..5436e6d420bc 100644
--- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml
@@ -57,6 +57,9 @@  properties:
   "#iommu-cells":
     const: 1
 
+  "#interconnect-cells":
+    const: 1
+
 patternProperties:
   "^emc-timings-[0-9]+$":
     type: object
@@ -120,6 +123,7 @@  required:
   - clock-names
   - "#reset-cells"
   - "#iommu-cells"
+  - "#interconnect-cells"
 
 additionalProperties: false
 
@@ -135,6 +139,7 @@  examples:
 
         #iommu-cells = <1>;
         #reset-cells = <1>;
+        #interconnect-cells = <1>;
 
         emc-timings-1 {
             nvidia,ram-code = <1>;