Re: [PATCH net-next 1/3] dt-bindings: net: qcom,ipa: make imem interconnect optional

On Mon, Jul 26, 2021 at 9:59 AM Alex Elder [off-list ref] wrote:

On 7/23/21 3:52 PM, Rob Herring wrote:

quoted

On Mon, Jul 19, 2021 at 04:24:54PM -0500, Alex Elder wrote:

quoted

On some newer SoCs, the interconnect between IPA and SoC internal
memory (imem) is not used.  Reflect this in the binding by moving
the definition of the "imem" interconnect to the end and defining
minItems to be 2 for both the interconnects and interconnect-names
properties.

Signed-off-by: Alex Elder <redacted>
---
  .../devicetree/bindings/net/qcom,ipa.yaml      | 18 ++++++++++--------
  1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/qcom,ipa.yaml b/Documentation/devicetree/bindings/net/qcom,ipa.yaml
index ed88ba4b94df5..4853ab7017bd9 100644
--- a/Documentation/devicetree/bindings/net/qcom,ipa.yaml
+++ b/Documentation/devicetree/bindings/net/qcom,ipa.yaml

@@ -87,16 +87,18 @@ properties:
        - const: ipa-setup-ready

    interconnects:
+    minItems: 2
      items:
-      - description: Interconnect path between IPA and main memory
-      - description: Interconnect path between IPA and internal memory
-      - description: Interconnect path between IPA and the AP subsystem
+      - description: Path leading to system memory
+      - description: Path between the AP and IPA config space
+      - description: Path leading to internal memory

    interconnect-names:
+    minItems: 2
      items:
        - const: memory
-      - const: imem
        - const: config
+      - const: imem

What about existing users? This will generate warnings. Doing this for
the 2nd item would avoid the need for .dts updates:

- enum: [ imem, config ]

If I understand correctly, the effect of this would be that
the second item can either be "imem" or "config", and the third
(if present) could only be "imem"?

Yes for the 2nd, but the 3rd item could only be 'config'.

And you're saying that otherwise, existing users (the only
one it applies to at the moment is "sdm845.dtsi") would
produce warnings, because the interconnects are listed
in an order different from what the binding specifies.

Is that correct?

Yes.

If so, what you propose suggests "imem" could be listed twice.
It doesn't make sense, and maybe it's precluded in other ways
so that's OK.

Good observation. There are generic checks that the strings are unique.

 But I'd be happy to update "sdm845.dtsi" to
address your concern.  (Maybe that's something you would rather
avoid?)

Better to not change DT if you don't have to. You're probably okay if
all clients (consumers of the dtb) used names and didn't care about
the order. And I have no idea if all users of SDM845 are okay with a
DTB change being required. That's up to QCom maintainers. I only care
that ABI breakages are documented as such.

Also, I need to make a separate update to "sm8350.dtsi" because
that was defined before I understood what I do now about the
interconnects.  It uses the wrong names, and should combine
its first two interconnects into just one.

If the interconnects was ignored in that case, then the change doesn't matter.

Rob