On Tue, Sep 03, 2024 at 03:02:34PM -0700, Nikunj Kela wrote: > Add compatible representing i2c support on SA8255p. > > Clocks and interconnects are being configured in Firmware VM > on SA8255p, therefore making them optional. > > CC: Praveen Talari <quic_ptalari@xxxxxxxxxxx> > Signed-off-by: Nikunj Kela <quic_nkela@xxxxxxxxxxx> > --- > .../bindings/i2c/qcom,i2c-geni-qcom.yaml | 33 +++++++++++++++++-- > 1 file changed, 31 insertions(+), 2 deletions(-) > I don't know what to do with this patch. Using specific compatibles next to generic compatible is just wrong, although mistake was probably allowing generic compatible. The patch does not explain the differences in interface which would explain why devices are not compatible. In the same time my advice of separate binding was not followed, because maybe these devices are compatible? But then it should be expressed... You have entire commit msg to explain what and why. > diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml b/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml > index 9f66a3bb1f80..b477fae734b6 100644 > --- a/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml > +++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml > @@ -15,6 +15,7 @@ properties: > enum: > - qcom,geni-i2c > - qcom,geni-i2c-master-hub > + - qcom,sa8255p-geni-i2c > > clocks: > minItems: 1 > @@ -69,8 +70,6 @@ properties: > required: > - compatible > - interrupts > - - clocks > - - clock-names > - reg > > allOf: > @@ -81,6 +80,10 @@ allOf: > contains: > const: qcom,geni-i2c-master-hub > then: > + required: > + - clocks > + - clock-names So it is required here? > + > properties: > clocks: > minItems: 2 > @@ -100,7 +103,21 @@ allOf: > items: > - const: qup-core > - const: qup-config > + > + - if: > + properties: > + compatible: > + contains: > + const: qcom,sa8255p-geni-i2c > + then: > + required: > + - power-domains > + And possible here? I assume with the same clocks? The same for interconnects - same values are valid? > else: > + required: > + - clocks > + - clock-names And clocks are required again? > + > properties: > clocks: > maxItems: 1 Eeee? So now all other variants have max 1 clock? Nope, this wasn't ever tested on real DTS. Best regards, Krzysztof
