| Message ID | 20240903220240.2594102-17-quic_nkela@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,01/21] dt-bindings: arm: qcom: add the SoC ID for SA8255P | expand |
On Tue, Sep 03, 2024 at 03:02:35PM -0700, Nikunj Kela wrote: > Add compatible representing spi support on SA8255p. > > Clocks and interconnects are being configured in firmware VM > on SA8255p platform, therefore making them optional. > Please use standard email subjects, so with the PATCH keyword in the title. helps here to create proper versioned patches. Another useful tool is b4. Skipping the PATCH keyword makes filtering of emails more difficult thus making the review process less convenient. > CC: Praveen Talari <quic_ptalari@quicinc.com> > Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com> > --- > .../bindings/spi/qcom,spi-geni-qcom.yaml | 60 +++++++++++++++++-- > 1 file changed, 56 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml b/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml > index 2e20ca313ec1..75b52c0a7440 100644 > --- a/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml > +++ b/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml > @@ -25,10 +25,45 @@ description: > > allOf: > - $ref: /schemas/spi/spi-controller.yaml# > + - if: > + properties: > + compatible: > + contains: > + const: qcom,sa8255p-geni-spi Not much improved. All my previous (v1) and other patch (i2c) comments apply. > + then: > + required: > + - power-domains > + - power-domain-names > + > + properties: > + power-domains: > + minItems: 2 > + > + else: > + required: > + - clocks > + - clock-names > + > + properties: > + power-domains: > + maxItems: 1 > + > + interconnects: > + minItems: 2 > + maxItems: 3 > + > + interconnect-names: > + minItems: 2 > + items: > + - const: qup-core > + - const: qup-config > + - const: qup-memory > > properties: > compatible: > - const: qcom,geni-spi > + enum: > + - qcom,geni-spi > + - qcom,sa8255p-geni-spi You have entire commit msg to explain why this device's programming model is not compatible with existing generic compatible which must cover all variants (because it is crazy generic). Best regards, Krzysztof
On 04/09/2024 00:02, Nikunj Kela wrote: > Add compatible representing spi support on SA8255p. > > Clocks and interconnects are being configured in firmware VM > on SA8255p platform, therefore making them optional. > > CC: Praveen Talari <quic_ptalari@quicinc.com> > Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com> Also this is incomplete - adding compatible without driver change is not expected. It cannot even work. Best regards, Krzysztof
On 9/4/2024 12:48 AM, Krzysztof Kozlowski wrote: > On 04/09/2024 00:02, Nikunj Kela wrote: >> Add compatible representing spi support on SA8255p. >> >> Clocks and interconnects are being configured in firmware VM >> on SA8255p platform, therefore making them optional. >> >> CC: Praveen Talari <quic_ptalari@quicinc.com> >> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com> > Also this is incomplete - adding compatible without driver change is not > expected. It cannot even work. > > Best regards, > Krzysztof Link for CLO branch is provided in I2C patch series. The driver changes will soon follow.
On Wed, Sep 04, 2024 at 05:49:40AM GMT, Nikunj Kela wrote: > > On 9/4/2024 12:48 AM, Krzysztof Kozlowski wrote: > > On 04/09/2024 00:02, Nikunj Kela wrote: > >> Add compatible representing spi support on SA8255p. > >> > >> Clocks and interconnects are being configured in firmware VM > >> on SA8255p platform, therefore making them optional. > >> > >> CC: Praveen Talari <quic_ptalari@quicinc.com> > >> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com> > > Also this is incomplete - adding compatible without driver change is not > > expected. It cannot even work. > > > > Best regards, > > Krzysztof > > Link for CLO branch is provided in I2C patch series. The driver changes > will soon follow. So, what's the point of posting the dt-bindings without corresponding driver changes?
diff --git a/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml b/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml index 2e20ca313ec1..75b52c0a7440 100644 --- a/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml +++ b/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml @@ -25,10 +25,45 @@ description: allOf: - $ref: /schemas/spi/spi-controller.yaml# + - if: + properties: + compatible: + contains: + const: qcom,sa8255p-geni-spi + then: + required: + - power-domains + - power-domain-names + + properties: + power-domains: + minItems: 2 + + else: + required: + - clocks + - clock-names + + properties: + power-domains: + maxItems: 1 + + interconnects: + minItems: 2 + maxItems: 3 + + interconnect-names: + minItems: 2 + items: + - const: qup-core + - const: qup-config + - const: qup-memory properties: compatible: - const: qcom,geni-spi + enum: + - qcom,geni-spi + - qcom,sa8255p-geni-spi clocks: maxItems: 1 @@ -61,15 +96,19 @@ properties: operating-points-v2: true power-domains: - maxItems: 1 + minItems: 1 + maxItems: 2 + + power-domain-names: + items: + - const: power + - const: perf reg: maxItems: 1 required: - compatible - - clocks - - clock-names - interrupts - reg @@ -116,3 +155,16 @@ examples: #address-cells = <1>; #size-cells = <0>; }; + + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + + spi@888000 { + compatible = "qcom,sa8255p-geni-spi"; + reg = <0x888000 0x4000>; + interrupts = <GIC_SPI 584 IRQ_TYPE_LEVEL_HIGH>; + #address-cells = <1>; + #size-cells = <0>; + power-domains = <&scmi10_pd 16>, <&scmi10_dvfs 16>; + power-domain-names = "power", "perf"; + };
Add compatible representing spi support on SA8255p. Clocks and interconnects are being configured in firmware VM on SA8255p platform, therefore making them optional. CC: Praveen Talari <quic_ptalari@quicinc.com> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com> --- .../bindings/spi/qcom,spi-geni-qcom.yaml | 60 +++++++++++++++++-- 1 file changed, 56 insertions(+), 4 deletions(-)