| Message ID | 20201025221735.3062-10-digetx@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce memory interconnect for NVIDIA Tegra SoCs | expand |
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
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.
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 --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>;