Re: [PATCH v6 05/52] dt-bindings: memory: tegra20: mc: Document new interconnect property

[Krzysztof Kozlowski]

On Tue, Oct 27, 2020 at 10:17:48PM +0300, Dmitry Osipenko wrote:
> 27.10.2020 11:55, Krzysztof Kozlowski пишет:
> > On Mon, Oct 26, 2020 at 01:16:48AM +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@xxxxxxxxxx>
> >> Signed-off-by: Dmitry Osipenko <digetx@xxxxxxxxx>
> >> ---
> >>  .../bindings/memory-controllers/nvidia,tegra20-mc.txt          | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.txt b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.txt
> >> index e55328237df4..739b7c6f2e26 100644
> >> --- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.txt
> >> +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.txt
> >> @@ -16,6 +16,8 @@ Required properties:
> >>    IOMMU specifier needed to encode an address. GART supports only a single
> >>    address space that is shared by all devices, therefore no additional
> >>    information needed for the address encoding.
> >> +- #interconnect-cells : Should be 1. This cell represents memory client.
> >> +  The assignments may be found in header file <dt-bindings/memory/tegra20-mc.h>.
> > 
> > This is a list of required properties so you break the ABI. All existing
> > DTBs will be affected.
> 
> This is optional property for the older DTBs, but for newer DTs it's
> mandatory.

We do not consider here "older" or "newer" DTBs, but existing ones in
the world using this binding.

If it was optional so far, it cannot be made mandatory without changing
the ABI. Which is an ABI break.

> IIUC, it should be defined as a required property in the
> binding.
> 
> Please see tegra_mc_interconnect_setup() in "memory: tegra-mc: Add
> interconnect framework", which check presence of the ICC DT property.

The implementation indeed does not enforce it (except adding error msg,
about which I commented). Therefore it should be an optional property.

Best regards,
Krzysztof