On Mon, Jul 26, 2021 at 9:59 AM Alex Elder <elder@xxxxxxxx> wrote: > > On 7/23/21 3:52 PM, Rob Herring wrote: > > On Mon, Jul 19, 2021 at 04:24:54PM -0500, Alex Elder wrote: > >> 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 <elder@xxxxxxxxxx> > >> --- > >> .../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
