| Message ID | 20240828203721.2751904-17-quic_nkela@quicinc.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | arm64: qcom: Introduce SA8255p Ride platform | expand |
On Wed, 28 Aug 2024 13:37:15 -0700, Nikunj Kela wrote: > Add "qcom,sa8255p-geni-se-qup" compatible for representing QUP on > SA8255p. > > Clocks are being managed by the firmware VM and not required on > SA8255p Linux VM hence removing it from required list. > > CC: Praveen Talari <quic_ptalari@quicinc.com> > Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com> > --- > .../bindings/soc/qcom/qcom,geni-se.yaml | 47 +++++++++++++++++-- > 1 file changed, 43 insertions(+), 4 deletions(-) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: i2c@984000:compatible:0: 'qcom,sa8255p-geni-i2c' is not one of ['qcom,geni-i2c', 'qcom,geni-i2c-master-hub'] from schema $id: http://devicetree.org/schemas/soc/qcom/qcom,geni-se.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: i2c@984000: 'clocks' is a required property from schema $id: http://devicetree.org/schemas/soc/qcom/qcom,geni-se.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: i2c@984000: 'clock-names' is a required property from schema $id: http://devicetree.org/schemas/soc/qcom/qcom,geni-se.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: i2c@984000: Unevaluated properties are not allowed ('compatible' was unexpected) from schema $id: http://devicetree.org/schemas/soc/qcom/qcom,geni-se.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: serial@990000:compatible:0: 'qcom,sa8255p-geni-uart' is not one of ['qcom,geni-uart', 'qcom,geni-debug-uart'] from schema $id: http://devicetree.org/schemas/soc/qcom/qcom,geni-se.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: serial@990000:power-domains: [[4294967295, 4], [4294967295, 4]] is too long from schema $id: http://devicetree.org/schemas/soc/qcom/qcom,geni-se.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: serial@990000: 'clocks' is a required property from schema $id: http://devicetree.org/schemas/soc/qcom/qcom,geni-se.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: serial@990000: 'clock-names' is a required property from schema $id: http://devicetree.org/schemas/soc/qcom/qcom,geni-se.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: serial@990000: Unevaluated properties are not allowed ('compatible', 'power-domain-names', 'power-domains' were unexpected) from schema $id: http://devicetree.org/schemas/soc/qcom/qcom,geni-se.yaml# Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: /example-1/soc/geniqup@9c0000/i2c@984000: failed to match any schema with compatible: ['qcom,sa8255p-geni-i2c'] Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: /example-1/soc/geniqup@9c0000/serial@990000: failed to match any schema with compatible: ['qcom,sa8255p-geni-uart'] doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240828203721.2751904-17-quic_nkela@quicinc.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On Wed, Aug 28, 2024 at 01:37:15PM -0700, Nikunj Kela wrote: > Add "qcom,sa8255p-geni-se-qup" compatible for representing QUP on > SA8255p. > > Clocks are being managed by the firmware VM and not required on > SA8255p Linux VM hence removing it from required list. > > CC: Praveen Talari <quic_ptalari@quicinc.com> > Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com> > --- > .../bindings/soc/qcom/qcom,geni-se.yaml | 47 +++++++++++++++++-- > 1 file changed, 43 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml > index 7b031ef09669..40e3a3e045da 100644 > --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml > @@ -22,17 +22,16 @@ properties: > enum: > - qcom,geni-se-qup > - qcom,geni-se-i2c-master-hub > + - qcom,sa8255p-geni-se-qup Same problems. If you decide to use generic compatibles, it means it covers all devices. Otherwise it does not make any sense. > > reg: > description: QUP wrapper common register address and length. > maxItems: 1 > > clock-names: > - minItems: 1 Huh? Best regards, Krzysztof
On 8/29/2024 12:42 AM, Krzysztof Kozlowski wrote: > On Wed, Aug 28, 2024 at 01:37:15PM -0700, Nikunj Kela wrote: >> Add "qcom,sa8255p-geni-se-qup" compatible for representing QUP on >> SA8255p. >> >> Clocks are being managed by the firmware VM and not required on >> SA8255p Linux VM hence removing it from required list. >> >> CC: Praveen Talari <quic_ptalari@quicinc.com> >> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com> >> --- >> .../bindings/soc/qcom/qcom,geni-se.yaml | 47 +++++++++++++++++-- >> 1 file changed, 43 insertions(+), 4 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml >> index 7b031ef09669..40e3a3e045da 100644 >> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml >> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml >> @@ -22,17 +22,16 @@ properties: >> enum: >> - qcom,geni-se-qup >> - qcom,geni-se-i2c-master-hub >> + - qcom,sa8255p-geni-se-qup > Same problems. If you decide to use generic compatibles, it means it > covers all devices. Otherwise it does not make any sense. Hi Krzysztof, SA8255p platform is not compatible with generic ones. At the time generic compatibles were added, no one thought of such platform will appear in future. Please advise what should we do in this case? Thanks, -Nikunj >> >> reg: >> description: QUP wrapper common register address and length. >> maxItems: 1 >> >> clock-names: >> - minItems: 1 > Huh? > > Best regards, > Krzysztof >
On 29/08/2024 16:23, Nikunj Kela wrote: > > On 8/29/2024 12:42 AM, Krzysztof Kozlowski wrote: >> On Wed, Aug 28, 2024 at 01:37:15PM -0700, Nikunj Kela wrote: >>> Add "qcom,sa8255p-geni-se-qup" compatible for representing QUP on >>> SA8255p. >>> >>> Clocks are being managed by the firmware VM and not required on >>> SA8255p Linux VM hence removing it from required list. >>> >>> CC: Praveen Talari <quic_ptalari@quicinc.com> >>> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com> >>> --- >>> .../bindings/soc/qcom/qcom,geni-se.yaml | 47 +++++++++++++++++-- >>> 1 file changed, 43 insertions(+), 4 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml >>> index 7b031ef09669..40e3a3e045da 100644 >>> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml >>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml >>> @@ -22,17 +22,16 @@ properties: >>> enum: >>> - qcom,geni-se-qup >>> - qcom,geni-se-i2c-master-hub >>> + - qcom,sa8255p-geni-se-qup >> Same problems. If you decide to use generic compatibles, it means it >> covers all devices. Otherwise it does not make any sense. > > Hi Krzysztof, > > SA8255p platform is not compatible with generic ones. At the time > generic compatibles were added, no one thought of such platform will That's kind of obvious and expected yet these were added... > appear in future. Please advise what should we do in this case? I don't know. We keep telling - do not use generic compatibles, because you will have something like this, but people use generic compatibles - so what can I say? I told you so? Can we get agreement that using generic compatibles is a wrong idea? Or sort of promise - we won't use them? Or policy? I don't know, we can move on assuming this was a mistake 8 years ago, approaches evolve, reviews change, but I am just afraid I will be repeating the same to several future contributions and every time come with long arguments exhausting my energy - don't add generic compatibles. If devices are not compatible, I suggest different bindings. Best regards, Krzysztof
On 8/30/2024 2:58 AM, Krzysztof Kozlowski wrote: > On 29/08/2024 16:23, Nikunj Kela wrote: >> On 8/29/2024 12:42 AM, Krzysztof Kozlowski wrote: >>> On Wed, Aug 28, 2024 at 01:37:15PM -0700, Nikunj Kela wrote: >>>> Add "qcom,sa8255p-geni-se-qup" compatible for representing QUP on >>>> SA8255p. >>>> >>>> Clocks are being managed by the firmware VM and not required on >>>> SA8255p Linux VM hence removing it from required list. >>>> >>>> CC: Praveen Talari <quic_ptalari@quicinc.com> >>>> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com> >>>> --- >>>> .../bindings/soc/qcom/qcom,geni-se.yaml | 47 +++++++++++++++++-- >>>> 1 file changed, 43 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml >>>> index 7b031ef09669..40e3a3e045da 100644 >>>> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml >>>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml >>>> @@ -22,17 +22,16 @@ properties: >>>> enum: >>>> - qcom,geni-se-qup >>>> - qcom,geni-se-i2c-master-hub >>>> + - qcom,sa8255p-geni-se-qup >>> Same problems. If you decide to use generic compatibles, it means it >>> covers all devices. Otherwise it does not make any sense. >> Hi Krzysztof, >> >> SA8255p platform is not compatible with generic ones. At the time >> generic compatibles were added, no one thought of such platform will > That's kind of obvious and expected yet these were added... > >> appear in future. Please advise what should we do in this case? > I don't know. We keep telling - do not use generic compatibles, because > you will have something like this, but people use generic compatibles - > so what can I say? I told you so? > > Can we get agreement that using generic compatibles is a wrong idea? Or > sort of promise - we won't use them? Or policy? I don't know, we can > move on assuming this was a mistake 8 years ago, approaches evolve, > reviews change, but I am just afraid I will be repeating the same to > several future contributions and every time come with long arguments > exhausting my energy - don't add generic compatibles. > > If devices are not compatible, I suggest different bindings. > > Best regards, > Krzysztof Hi Krzysztof, I will bring your concerns (raised above) to Qualcomm leads' attention. Thank you for your feedback and support. Thanks, -Nikunj >
diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml index 7b031ef09669..40e3a3e045da 100644 --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml @@ -22,17 +22,16 @@ properties: enum: - qcom,geni-se-qup - qcom,geni-se-i2c-master-hub + - qcom,sa8255p-geni-se-qup reg: description: QUP wrapper common register address and length. maxItems: 1 clock-names: - minItems: 1 maxItems: 2 clocks: - minItems: 1 maxItems: 2 "#address-cells": @@ -57,8 +56,6 @@ properties: required: - compatible - reg - - clock-names - - clocks - "#address-cells" - "#size-cells" - ranges @@ -83,6 +80,17 @@ patternProperties: $ref: /schemas/serial/qcom,serial-geni-qcom.yaml# allOf: + - if: + not: + properties: + compatible: + contains: + const: qcom,sa8255p-geni-se-qup + then: + required: + - clocks + - clock-names + - if: properties: compatible: @@ -162,4 +170,35 @@ examples: }; }; + - | + + soc { + #address-cells = <2>; + #size-cells = <2>; + + geniqup@9c0000 { + compatible = "qcom,sa8255p-geni-se-qup"; + reg = <0 0x9c0000 0 0x6000>; + #address-cells = <2>; + #size-cells = <2>; + ranges; + + i2c1: i2c@984000 { + compatible = "qcom,sa8255p-geni-i2c"; + reg = <0 0x984000 0 0x4000>; + interrupts = <GIC_SPI 551 IRQ_TYPE_LEVEL_HIGH>; + #address-cells = <1>; + #size-cells = <0>; + power-domains = <&scmi9_pd 1>; + }; + + uart4: serial@990000 { + compatible = "qcom,sa8255p-geni-uart"; + reg = <0 0x990000 0 0x4000>; + interrupts = <GIC_SPI 531 IRQ_TYPE_LEVEL_HIGH>; + power-domains = <&scmi11_pd 4>, <&scmi11_dvfs 4>; + power-domain-names = "power", "perf"; + }; + }; + }; ...
Add "qcom,sa8255p-geni-se-qup" compatible for representing QUP on SA8255p. Clocks are being managed by the firmware VM and not required on SA8255p Linux VM hence removing it from required list. CC: Praveen Talari <quic_ptalari@quicinc.com> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com> --- .../bindings/soc/qcom/qcom,geni-se.yaml | 47 +++++++++++++++++-- 1 file changed, 43 insertions(+), 4 deletions(-)