mbox series

[00/22] arm64: qcom: Introduce SA8255p Ride platform

Message ID 20240828203721.2751904-1-quic_nkela@quicinc.com
Headers show
Series arm64: qcom: Introduce SA8255p Ride platform | expand

Message

Nikunj Kela Aug. 28, 2024, 8:36 p.m. UTC
This series enables the support for SA8255p Qualcomm SoC and Ride
platform. This platform uses SCMI power, reset, performance, sensor
protocols for resources(e.g. clocks, regulator, interconnect, phy etc.)
management. SA8255p is a virtual platforms that uses Qualcomm smc/hvc
transport driver.

Multiple virtual SCMI instances are being used to achieve the parallelism.
SCMI platform stack runs in SMP enabled VM hence allows platform to service
multiple resource requests in parallel. Each device is assigned its own
dedicated SCMI channel and Tx/Rx doorbells.

Resource operations are grouped together to achieve better abstraction
and to reduce the number of requests being sent to SCMI platform(server)
thus improving boot time KPIs. This design approach was presented during
LinaroConnect 2024 conference[1].

Architecture:
------------
                                                          +--------------------+
                                                          |   Shared Memory    |
                                                          |                    |
                                                          | +----------------+ |                +----------------------------------+
     +----------------------------+                     +-+->  ufs-shmem     <-+---+            |            Linux VM              |
     |        Firmware VM         |                     | | +----------------+ |   |            |   +----------+   +----------+    |
     |                            |                     | |                    |   |            |   |   UFS    |   |   PCIe   |    |
     | +---------+ f +----------+ |                     | |                    |   |            |   |  Driver  |   |  Driver  |    |
     | |Drivers  <---+  SCMI    | |        e            | |         |          |   |            |   +--+----^--+   +----------+    |
     | | (clks,  | g | Server   +-+---------------------+ |                    |   |            |      |    |                      |
     | |  vreg,  +--->          | |        h              |         |          |  b|k           |     a|   l|                      |
     | |  gpio,  |   +--^-----+-+ |                       |                    |   |            |      |    |                      |
     | |  phy,   |      |     |   |                       |         |          |   |            |  +---v----+----+  +----------+   |
     | |  etc.)  |      |     |   |                       |                    |   +------------+--+  UFS SCMI   |  | PCIe SCMI|   |
     | +---------+      |     |   |                       |                    |                |  |  INSTANCE   |  | INSTANCE |   |
     |                  |     |   |                       |  +---------------+ |                |  +-^-----+-----+  +----------+   |
     |                  |     |   |                       |  |  pcie-shmem   | |                |    |     |                       |
     +------------------+-----+---+                       |  +---------------+ |                +----+-----+-----------------------+
                        |     |                           |                    |                     |     |
                        |     |                           +--------------------+                     |     |
                       d|IRQ i|HVC                                                                  j|IRQ c|HVC
                        |     |                                                                      |     |
                        |     |                                                                      |     |
+-----------------------+-----v----------------------------------------------------------------------+-----v------------------------------+
|                                                                                                                                         |
|                                                                                                                                         |
|                                                                                                                                         |
|                                                               HYPERVISOR                                                                |
|                                                                                                                                         |
|                                                                                                                                         |
+-----------------------------------------------------------------------------------------------------------------------------------------+

        +--------+   +--------+                                                                         +----------+  +-----------+
        | CLOCK  |   |  PHY   |                                                                         |   UFS    |  |   PCIe    |
        +--------+   +--------+                                                                         +----------+  +-----------+


This series is based on next-20240828.

[1]: https://resources.linaro.org/en/resource/wfnfEwBhRjLV1PEAJoDDte

Nikunj Kela (22):
  dt-bindings: arm: qcom: add the SoC ID for SA8255P
  soc: qcom: socinfo: add support for SA8255P
  dt-bindings: arm: qcom: add SA8255p Ride board
  dt-bindings: firmware: qcom,scm: document support for SA8255p
  dt-bindings: mailbox: qcom-ipcc: document the support for SA8255p
  dt-bindings: watchdog: qcom-wdt: document support on SA8255p
  dt-bindings: crypto: qcom,prng: document support for SA8255p
  dt-bindings: interrupt-controller: qcom-pdc: document support for
    SA8255p
  dt-bindings: soc: qcom: aoss-qmp: document support for SA8255p
  dt-bindings: pinctrl: document support for SA8255p
  pinctrl: qcom: sa8775p: Add support for SA8255p SoC
  dt-bindings: cpufreq: qcom-hw: document support for SA8255p
  dt-bindings: thermal: tsens: document support on SA8255p
  dt-bindings: arm-smmu: document the support on SA8255p
  dt-bindings: mfd: qcom,tcsr: document support for SA8255p
  dt-bindings: qcom: geni-se: document support for SA8255P
  dt-bindings: serial: document support for SA8255p
  dt-bindings: spi: document support for SA8255p
  dt-bindings: i2c: document support for SA8255p
  dt-bindings: firmware: arm,scmi: allow multiple virtual instances
  ARM: dt: GIC: add extended SPI specifier
  arm64: dts: qcom: Add reduced functional DT for SA8255p Ride platform

 .../devicetree/bindings/arm/qcom.yaml         |    6 +
 .../bindings/cpufreq/cpufreq-qcom-hw.yaml     |    1 +
 .../devicetree/bindings/crypto/qcom,prng.yaml |    1 +
 .../bindings/firmware/arm,scmi.yaml           |    2 +-
 .../bindings/firmware/qcom,scm.yaml           |    2 +
 .../bindings/i2c/qcom,i2c-geni-qcom.yaml      |   56 +-
 .../interrupt-controller/qcom,pdc.yaml        |    1 +
 .../devicetree/bindings/iommu/arm,smmu.yaml   |    3 +
 .../bindings/mailbox/qcom-ipcc.yaml           |    1 +
 .../devicetree/bindings/mfd/qcom,tcsr.yaml    |    1 +
 .../bindings/pinctrl/qcom,sa8775p-tlmm.yaml   |    4 +-
 .../serial/qcom,serial-geni-qcom.yaml         |   58 +-
 .../bindings/soc/qcom/qcom,aoss-qmp.yaml      |    1 +
 .../bindings/soc/qcom/qcom,geni-se.yaml       |   47 +-
 .../bindings/spi/qcom,spi-geni-qcom.yaml      |   64 +-
 .../bindings/thermal/qcom-tsens.yaml          |    1 +
 .../bindings/watchdog/qcom-wdt.yaml           |    1 +
 arch/arm64/boot/dts/qcom/Makefile             |    1 +
 arch/arm64/boot/dts/qcom/sa8255p-pmics.dtsi   |   80 +
 arch/arm64/boot/dts/qcom/sa8255p-ride.dts     |  149 +
 arch/arm64/boot/dts/qcom/sa8255p-scmi.dtsi    | 2312 ++++++++++++++++
 arch/arm64/boot/dts/qcom/sa8255p.dtsi         | 2405 +++++++++++++++++
 drivers/pinctrl/qcom/pinctrl-sa8775p.c        |    1 +
 drivers/soc/qcom/socinfo.c                    |    1 +
 include/dt-bindings/arm/qcom,ids.h            |    1 +
 .../interrupt-controller/arm-gic.h            |    1 +
 26 files changed, 5157 insertions(+), 44 deletions(-)
 create mode 100644 arch/arm64/boot/dts/qcom/sa8255p-pmics.dtsi
 create mode 100644 arch/arm64/boot/dts/qcom/sa8255p-ride.dts
 create mode 100644 arch/arm64/boot/dts/qcom/sa8255p-scmi.dtsi
 create mode 100644 arch/arm64/boot/dts/qcom/sa8255p.dtsi


base-commit: 195a402a75791e6e0d96d9da27ca77671bc656a8

Comments

Krzysztof Kozlowski Aug. 29, 2024, 7:57 a.m. UTC | #1
On 28/08/2024 22:36, Nikunj Kela wrote:
> This series enables the support for SA8255p Qualcomm SoC and Ride
> platform. This platform uses SCMI power, reset, performance, sensor
> protocols for resources(e.g. clocks, regulator, interconnect, phy etc.)
> management. SA8255p is a virtual platforms that uses Qualcomm smc/hvc
> transport driver.
> 

Who is supposed to merge it? The Cc-list is quite enormous and I got now
20 bounces:

"    Too many recipients to the message"

at least drop some non-maintainer related, I counted 5-7 Qualcomm ones
which should not be needed.

Best regards,
Krzysztof
Krzysztof Kozlowski Sept. 4, 2024, 5:54 a.m. UTC | #2
On Tue, Sep 03, 2024 at 03:02:19PM -0700, Nikunj Kela wrote:
> This series enables the support for SA8255p Qualcomm SoC and Ride
> platform. This platform uses SCMI power, reset, performance, sensor
> protocols for resources(e.g. clocks, regulator, interconnect, phy etc.)
> management. SA8255p is a virtual platforms that uses Qualcomm smc/hvc
> transport driver.
> 
> Multiple virtual SCMI instances are being used to achieve the parallelism.
> SCMI platform stack runs in SMP enabled VM hence allows platform to service
> multiple resource requests in parallel. Each device is assigned its own
> dedicated SCMI channel and Tx/Rx doorbells.
> 

Do not attach (thread) your patchsets to some other threads (unrelated
or older versions). This buries them deep in the mailbox and might
interfere with applying entire sets.

It does not look like you tested the bindings, at least after quick
look. Please run  (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).
Maybe you need to update your dtschema and yamllint.

Best regards,
Krzysztof
Krzysztof Kozlowski Sept. 4, 2024, 6:31 a.m. UTC | #3
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@quicinc.com>
> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
> ---
>  .../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
Krzysztof Kozlowski Sept. 4, 2024, 6:36 a.m. UTC | #4
On Tue, Sep 03, 2024 at 03:02:36PM -0700, Nikunj Kela wrote:
> Add compatibles representing UART support on SA8255p.
> 
> Clocks and interconnects are being configured in the 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>
> ---
>  .../serial/qcom,serial-geni-qcom.yaml         | 53 ++++++++++++++++---
>  1 file changed, 47 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/serial/qcom,serial-geni-qcom.yaml b/Documentation/devicetree/bindings/serial/qcom,serial-geni-qcom.yaml
> index dd33794b3534..b63c984684f3 100644
> --- a/Documentation/devicetree/bindings/serial/qcom,serial-geni-qcom.yaml
> +++ b/Documentation/devicetree/bindings/serial/qcom,serial-geni-qcom.yaml
> @@ -10,14 +10,13 @@ maintainers:
>    - Andy Gross <agross@kernel.org>
>    - Bjorn Andersson <bjorn.andersson@linaro.org>
>  
> -allOf:
> -  - $ref: /schemas/serial/serial.yaml#
> -
>  properties:
>    compatible:
>      enum:
>        - qcom,geni-uart
>        - qcom,geni-debug-uart
> +      - qcom,sa8255p-geni-uart
> +      - qcom,sa8255p-geni-debug-uart

Why devices are not compatible? What changed in programming model?

>  
>    clocks:
>      maxItems: 1
> @@ -51,18 +50,49 @@ properties:
>        - const: sleep
>  
>    power-domains:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2
> +
> +  power-domain-names:

This does not match power-domains anymore.

> +    items:
> +      - const: power
> +      - const: perf
>  
>    reg:
>      maxItems: 1
>  
>  required:
>    - compatible
> -  - clocks
> -  - clock-names
>    - interrupts
>    - reg
>  
> +allOf:
> +  - $ref: /schemas/serial/serial.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,sa8255p-geni-uart
> +              - qcom,sa8255p-geni-debug-uart
> +    then:
> +      required:
> +        - power-domains
> +        - power-domain-names
> +
> +      properties:
> +        power-domains:
> +          minItems: 2
> +
> +    else:
> +      required:
> +        - clocks
> +        - clock-names
> +
> +      properties:
> +        power-domains:
> +          maxItems: 1
> +
>  unevaluatedProperties: false
>  
>  examples:
> @@ -83,4 +113,15 @@ examples:
>                          <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_QUP_0 0>;
>          interconnect-names = "qup-core", "qup-config";
>      };
> +
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    serial@990000 {
> +        compatible = "qcom,sa8255p-geni-uart";
> +        reg = <0x990000 0x4000>;
> +        interrupts = <GIC_SPI 531 IRQ_TYPE_LEVEL_HIGH>;
> +        power-domains = <&scmi11_pd 4>, <&scmi11_dvfs 4>;
> +        power-domain-names = "power", "perf";
> +    };
>  ...
> -- 
> 2.34.1
>
Krzysztof Kozlowski Sept. 4, 2024, 7:47 a.m. UTC | #5
On 04/09/2024 00:02, Nikunj Kela wrote:
> Add compatibles representing UART support on SA8255p.
> 
> Clocks and interconnects are being configured in the 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>
> ---
>  .../serial/qcom,serial-geni-qcom.yaml         | 53 ++++++++++++++++---
>  1 file changed, 47 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/serial/qcom,serial-geni-qcom.yaml b/Documentation/devicetree/bindings/serial/qcom,serial-geni-qcom.yaml
> index dd33794b3534..b63c984684f3 100644
> --- a/Documentation/devicetree/bindings/serial/qcom,serial-geni-qcom.yaml
> +++ b/Documentation/devicetree/bindings/serial/qcom,serial-geni-qcom.yaml
> @@ -10,14 +10,13 @@ maintainers:
>    - Andy Gross <agross@kernel.org>
>    - Bjorn Andersson <bjorn.andersson@linaro.org>
>  
> -allOf:
> -  - $ref: /schemas/serial/serial.yaml#
> -
>  properties:
>    compatible:
>      enum:
>        - qcom,geni-uart
>        - qcom,geni-debug-uart
> +      - qcom,sa8255p-geni-uart
> +      - qcom,sa8255p-geni-debug-uart


Anyway, the entire patchset is organized wrong. Or you sent only subset.

Where is the driver change? This cannot work. To remind bindings go with
the driver (nothing new here).

Best regards,
Krzysztof
Krzysztof Kozlowski Sept. 4, 2024, 7:49 a.m. UTC | #6
On 04/09/2024 00:02, 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@quicinc.com>
> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
> ---
>  .../bindings/i2c/qcom,i2c-geni-qcom.yaml      | 33 +++++++++++++++++--
>  1 file changed, 31 insertions(+), 2 deletions(-)
> 

Just to clarify to I2C maintainers:
This is incomplete. Missing driver changes.

Best regards,
Krzysztof
Wolfram Sang Sept. 4, 2024, 7:55 a.m. UTC | #7
> Just to clarify to I2C maintainers:
> This is incomplete. Missing driver changes.

Thanks, Krzysztof!
Nikunj Kela Sept. 4, 2024, 12:41 p.m. UTC | #8
On 9/3/2024 11:31 PM, Krzysztof Kozlowski wrote:
> 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@quicinc.com>
>> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
>> ---
>>  .../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.

I mentioned in the description that clocks and interconnects on this
platform are configured in Firmware VM(over SCMI using power and perf
domains) therefore this is not compatible with existing generic 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...

Sorry, I missed that. You want me to use 'oneOf' expression with this
compatible?


>
> You have entire commit msg to explain what and why.

Will put more details in description.


>> 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?

We are removing clocks from generic required list and enforcing rules
for all compatibles other than sa8255p.


>> +
>>        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?

I guess I need to put here the same description as in the cover letter
to make it more clear. We are not using clocks and interconnects in this
platform in Linux. Instead, sending request to Firmware VM over
SCMI(using power and perf protocols)


>
>>      else:
>> +      required:
>> +        - clocks
>> +        - clock-names
> And clocks are required again?
Explained above.
>> +
>>        properties:
>>          clocks:
>>            maxItems: 1
> Eeee? So now all other variants have max 1 clock?

I will make if block for sa8255p up so else is not applied to rest of
the platforms.


>
> Nope, this wasn't ever tested on real DTS.

This is tested on SA8255p DTS and I ran DT schema check on SA8775p DT as
well.


>
> Best regards,
> Krzysztof
>
Nikunj Kela Sept. 4, 2024, 12:49 p.m. UTC | #9
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.
Nikunj Kela Sept. 4, 2024, 12:54 p.m. UTC | #10
On 9/3/2024 11:36 PM, Krzysztof Kozlowski wrote:
> On Tue, Sep 03, 2024 at 03:02:36PM -0700, Nikunj Kela wrote:
>> Add compatibles representing UART support on SA8255p.
>>
>> Clocks and interconnects are being configured in the 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>
>> ---
>>  .../serial/qcom,serial-geni-qcom.yaml         | 53 ++++++++++++++++---
>>  1 file changed, 47 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/serial/qcom,serial-geni-qcom.yaml b/Documentation/devicetree/bindings/serial/qcom,serial-geni-qcom.yaml
>> index dd33794b3534..b63c984684f3 100644
>> --- a/Documentation/devicetree/bindings/serial/qcom,serial-geni-qcom.yaml
>> +++ b/Documentation/devicetree/bindings/serial/qcom,serial-geni-qcom.yaml
>> @@ -10,14 +10,13 @@ maintainers:
>>    - Andy Gross <agross@kernel.org>
>>    - Bjorn Andersson <bjorn.andersson@linaro.org>
>>  
>> -allOf:
>> -  - $ref: /schemas/serial/serial.yaml#
>> -
>>  properties:
>>    compatible:
>>      enum:
>>        - qcom,geni-uart
>>        - qcom,geni-debug-uart
>> +      - qcom,sa8255p-geni-uart
>> +      - qcom,sa8255p-geni-debug-uart
> Why devices are not compatible? What changed in programming model?

The cover-letter explains what is changed for devices in this platform.
I will add the description in this patch too.


>
>>  
>>    clocks:
>>      maxItems: 1
>> @@ -51,18 +50,49 @@ properties:
>>        - const: sleep
>>  
>>    power-domains:
>> -    maxItems: 1
>> +    minItems: 1
>> +    maxItems: 2
>> +
>> +  power-domain-names:
> This does not match power-domains anymore.

Single power domain doesn't need to use power-domain-names binding as it
is not needed however for multiple(in this case 2), you need to provide
names. I will add this property to if block and only keep maxItems here.


>
>> +    items:
>> +      - const: power
>> +      - const: perf
>>  
>>    reg:
>>      maxItems: 1
>>  
>>  required:
>>    - compatible
>> -  - clocks
>> -  - clock-names
>>    - interrupts
>>    - reg
>>  
>> +allOf:
>> +  - $ref: /schemas/serial/serial.yaml#
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - qcom,sa8255p-geni-uart
>> +              - qcom,sa8255p-geni-debug-uart
>> +    then:
>> +      required:
>> +        - power-domains
>> +        - power-domain-names
>> +
>> +      properties:
>> +        power-domains:
>> +          minItems: 2
>> +
>> +    else:
>> +      required:
>> +        - clocks
>> +        - clock-names
>> +
>> +      properties:
>> +        power-domains:
>> +          maxItems: 1
>> +
>>  unevaluatedProperties: false
>>  
>>  examples:
>> @@ -83,4 +113,15 @@ examples:
>>                          <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_QUP_0 0>;
>>          interconnect-names = "qup-core", "qup-config";
>>      };
>> +
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +
>> +    serial@990000 {
>> +        compatible = "qcom,sa8255p-geni-uart";
>> +        reg = <0x990000 0x4000>;
>> +        interrupts = <GIC_SPI 531 IRQ_TYPE_LEVEL_HIGH>;
>> +        power-domains = <&scmi11_pd 4>, <&scmi11_dvfs 4>;
>> +        power-domain-names = "power", "perf";
>> +    };
>>  ...
>> -- 
>> 2.34.1
>>
Nikunj Kela Sept. 4, 2024, 12:58 p.m. UTC | #11
On 9/3/2024 10:54 PM, Krzysztof Kozlowski wrote:
> On Tue, Sep 03, 2024 at 03:02:19PM -0700, Nikunj Kela wrote:
>> This series enables the support for SA8255p Qualcomm SoC and Ride
>> platform. This platform uses SCMI power, reset, performance, sensor
>> protocols for resources(e.g. clocks, regulator, interconnect, phy etc.)
>> management. SA8255p is a virtual platforms that uses Qualcomm smc/hvc
>> transport driver.
>>
>> Multiple virtual SCMI instances are being used to achieve the parallelism.
>> SCMI platform stack runs in SMP enabled VM hence allows platform to service
>> multiple resource requests in parallel. Each device is assigned its own
>> dedicated SCMI channel and Tx/Rx doorbells.
>>
> Do not attach (thread) your patchsets to some other threads (unrelated
> or older versions). This buries them deep in the mailbox and might
> interfere with applying entire sets.
>
> It does not look like you tested the bindings, at least after quick
> look. Please run  (see
> Documentation/devicetree/bindings/writing-schema.rst for instructions).
> Maybe you need to update your dtschema and yamllint.
>
> Best regards,
> Krzysztof

Will fix spaces and send v3 in separate thread. Thanks
Krzysztof Kozlowski Sept. 4, 2024, 1:20 p.m. UTC | #12
On 04/09/2024 14:45, Nikunj Kela wrote:
> 
> On 9/4/2024 12:55 AM, Wolfram Sang wrote:
>>> Just to clarify to I2C maintainers:
>>> This is incomplete. Missing driver changes.
>> Thanks, Krzysztof!
> 
> Driver changes are going through internal review and will soon be
> posted. For your reference, we have pushed driver changes in CodeLinaro
> git branch(nkela/sa8255p_v6_11_rc2)  in kernel-qcom repo [1]. You can
> take a look at the changes that are in pipeline and will follow soon.
> 

Sorry, we are not reviewing other repos. Post patches ONLY when they are
ready. Sending one piece without driver is not correct and it does not
make any, absolutely any sense.

Best regards,
Krzysztof
Krzysztof Kozlowski Sept. 4, 2024, 1:24 p.m. UTC | #13
On 04/09/2024 14:54, Nikunj Kela wrote:
> 
> On 9/3/2024 11:36 PM, Krzysztof Kozlowski wrote:
>> On Tue, Sep 03, 2024 at 03:02:36PM -0700, Nikunj Kela wrote:
>>> Add compatibles representing UART support on SA8255p.
>>>
>>> Clocks and interconnects are being configured in the 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>
>>> ---
>>>  .../serial/qcom,serial-geni-qcom.yaml         | 53 ++++++++++++++++---
>>>  1 file changed, 47 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/serial/qcom,serial-geni-qcom.yaml b/Documentation/devicetree/bindings/serial/qcom,serial-geni-qcom.yaml
>>> index dd33794b3534..b63c984684f3 100644
>>> --- a/Documentation/devicetree/bindings/serial/qcom,serial-geni-qcom.yaml
>>> +++ b/Documentation/devicetree/bindings/serial/qcom,serial-geni-qcom.yaml
>>> @@ -10,14 +10,13 @@ maintainers:
>>>    - Andy Gross <agross@kernel.org>
>>>    - Bjorn Andersson <bjorn.andersson@linaro.org>
>>>  
>>> -allOf:
>>> -  - $ref: /schemas/serial/serial.yaml#
>>> -
>>>  properties:
>>>    compatible:
>>>      enum:
>>>        - qcom,geni-uart
>>>        - qcom,geni-debug-uart
>>> +      - qcom,sa8255p-geni-uart
>>> +      - qcom,sa8255p-geni-debug-uart
>> Why devices are not compatible? What changed in programming model?
> 
> The cover-letter explains what is changed for devices in this platform.
> I will add the description in this patch too.

Many of us do not read cover letters. They don't really matter,
especially that serial tree will not include it. Each commit must stand
on its own.

> 
> 
>>
>>>  
>>>    clocks:
>>>      maxItems: 1
>>> @@ -51,18 +50,49 @@ properties:
>>>        - const: sleep
>>>  
>>>    power-domains:
>>> -    maxItems: 1
>>> +    minItems: 1
>>> +    maxItems: 2
>>> +
>>> +  power-domain-names:
>> This does not match power-domains anymore.
> 
> Single power domain doesn't need to use power-domain-names binding as it
> is not needed however for multiple(in this case 2), you need to provide
> names. I will add this property to if block and only keep maxItems here.

The xxx and xxx-names properties always go in sync. Otherwise we do not
really know what is the power domain for other variants.

You are allowed to be unspecific about power domain (so maxItems: 1) if
it is obvious. You now made it non-obvious, so above flexibility does
not apply anymore.

Best regards,
Krzysztof
Andrew Lunn Sept. 4, 2024, 4:58 p.m. UTC | #14
> Sorry, didn't realize SPI uses different subject format than other
> subsystems. Will fix in v3. Thanks

Each subsystem is free to use its own form. e.g for netdev you will
want the prefix [PATCH net-next v42] net: stmmac: dwmac-qcom-ethqos:

This is another reason why you should be splitting these patches per
subsystem, and submitting both the DT bindings and the code changes as
a two patch patchset. You can then learn how each subsystem names its
patches.

Please pick one victim subsystem and work on the patches for just that
subsystem. Once you have them correct, you can use everything you
learned to fixup all your other patches, one by one.

	Andrew
Nikunj Kela Sept. 4, 2024, 9:06 p.m. UTC | #15
On 9/4/2024 9:58 AM, Andrew Lunn wrote:
>> Sorry, didn't realize SPI uses different subject format than other
>> subsystems. Will fix in v3. Thanks
> Each subsystem is free to use its own form. e.g for netdev you will
> want the prefix [PATCH net-next v42] net: stmmac: dwmac-qcom-ethqos:
of course they are! No one is disputing that.
>
> This is another reason why you should be splitting these patches per
> subsystem, and submitting both the DT bindings and the code changes as
> a two patch patchset. You can then learn how each subsystem names its
> patches.

Qualcomm QUPs chips have serial engines that can be configured as
UART/I2C/SPI so QUPs changes require to be pushed in one series for all
3 subsystems as they all are dependent.


>
> Please pick one victim subsystem and work on the patches for just that
> subsystem. Once you have them correct, you can use everything you
> learned to fixup all your other patches, one by one.
>
> 	Andrew
Andrew Lunn Sept. 4, 2024, 9:49 p.m. UTC | #16
> Qualcomm QUPs chips have serial engines that can be configured as
> UART/I2C/SPI so QUPs changes require to be pushed in one series for all
> 3 subsystems as they all are dependent.

So leave that until later. And when you do, explicit mention why you
are cross posting to three subsystems, because the hardware is
designed like that. And suggest a way it could be merged, which
subsystem should take the lead, and the others just need to provide
Acked-by. The Maintainers might disagree, want to do it differently,
but i find it always helps to state this from the beginning, otherwise
sometimes no Maintainer take the lead role.

But this patchset appears to be much more than QUPs. You should be
able the break the rest up into smaller patchsets, one per subsystem.

	Andrew
Nikunj Kela Sept. 4, 2024, 11:50 p.m. UTC | #17
Hi All,

I have decided to split this series into multiple smaller ones as follows:

- Patches 1/21 - 11/21, 13/21 - 14/21, 19/21: will split them to each
subsystem specific patch sets.

- Patches 15/21 - 18/21: will come in separate series along with QUPs
driver changes.

- Patches 20/21 - 21/21: will come in separate series after above two
sets are accepted.

Thanks,

-Nikunj


On 9/3/2024 3:02 PM, Nikunj Kela wrote:
> This series enables the support for SA8255p Qualcomm SoC and Ride
> platform. This platform uses SCMI power, reset, performance, sensor
> protocols for resources(e.g. clocks, regulator, interconnect, phy etc.)
> management. SA8255p is a virtual platforms that uses Qualcomm smc/hvc
> transport driver.
>
> Multiple virtual SCMI instances are being used to achieve the parallelism.
> SCMI platform stack runs in SMP enabled VM hence allows platform to service
> multiple resource requests in parallel. Each device is assigned its own
> dedicated SCMI channel and Tx/Rx doorbells.
>
> Resource operations are grouped together to achieve better abstraction
> and to reduce the number of requests being sent to SCMI platform(server)
> thus improving boot time KPIs. This design approach was presented during
> LinaroConnect 2024 conference[1].
>
> Architecture:
> ------------
>                                                           +--------------------+
>                                                           |   Shared Memory    |
>                                                           |                    |
>                                                           | +----------------+ |                +----------------------------------+
>      +----------------------------+                     +-+->  ufs-shmem     <-+---+            |            Linux VM              |
>      |        Firmware VM         |                     | | +----------------+ |   |            |   +----------+   +----------+    |
>      |                            |                     | |                    |   |            |   |   UFS    |   |   PCIe   |    |
>      | +---------+ f +----------+ |                     | |                    |   |            |   |  Driver  |   |  Driver  |    |
>      | |Drivers  <---+  SCMI    | |        e            | |         |          |   |            |   +--+----^--+   +----------+    |
>      | | (clks,  | g | Server   +-+---------------------+ |                    |   |            |      |    |                      |
>      | |  vreg,  +--->          | |        h              |         |          |  b|k           |     a|   l|                      |
>      | |  gpio,  |   +--^-----+-+ |                       |                    |   |            |      |    |                      |
>      | |  phy,   |      |     |   |                       |         |          |   |            |  +---v----+----+  +----------+   |
>      | |  etc.)  |      |     |   |                       |                    |   +------------+--+  UFS SCMI   |  | PCIe SCMI|   |
>      | +---------+      |     |   |                       |                    |                |  |  INSTANCE   |  | INSTANCE |   |
>      |                  |     |   |                       |  +---------------+ |                |  +-^-----+-----+  +----------+   |
>      |                  |     |   |                       |  |  pcie-shmem   | |                |    |     |                       |
>      +------------------+-----+---+                       |  +---------------+ |                +----+-----+-----------------------+
>                         |     |                           |                    |                     |     |
>                         |     |                           +--------------------+                     |     |
>                        d|IRQ i|HVC                                                                  j|IRQ c|HVC
>                         |     |                                                                      |     |
>                         |     |                                                                      |     |
> +-----------------------+-----v----------------------------------------------------------------------+-----v------------------------------+
> |                                                                                                                                         |
> |                                                                                                                                         |
> |                                                                                                                                         |
> |                                                               HYPERVISOR                                                                |
> |                                                                                                                                         |
> |                                                                                                                                         |
> +-----------------------------------------------------------------------------------------------------------------------------------------+
>
>         +--------+   +--------+                                                                         +----------+  +-----------+
>         | CLOCK  |   |  PHY   |                                                                         |   UFS    |  |   PCIe    |
>         +--------+   +--------+                                                                         +----------+  +-----------+
>
>
> This series is based on next-20240903.
>
> [1]: https://resources.linaro.org/en/resource/wfnfEwBhRjLV1PEAJoDDte
>
> ---
> Changes in v2:
>   - Patch 1/21 - 11/21
>     - Added Reviewed-by tag
>
>   - Patch 12/21
>     - Already applied in the maintainers tree
>
>   - Patch 13/21
>     - Modified subject line
>     - Fixed schema to include fallback
>
>   - Patch 14/21
>     - Added constraints
>
>   - Patch 15/21
>     - Modified schema to remove useless text
>    
>   - Patch 16/21
>     - Modified schema formatting
>     - Amended schema definition as advised
>
>   - Patch 17/21
>     - Moved allOf block after required
>     - Fixed formatting
>     - Modified schema to remove useless text
>
>   - Patch 18/21
>     - Fixed clock property changes
>
>   - Patch 19/21
>     - Fixed scmi nodename pattern
>
>   - Patch 20/21
>     - Modified subject line and description
>     - Added EPPI macro
>
>   - Patch 21/21
>     - Removed scmichannels label and alias
>     - Modified scmi node name to conform to schema
>     - Moved status property to be the last one in scmi instances
>     - Changed to lower case for cpu labels
>     - Added fallback compatible for tlmm node
>
> Nikunj Kela (21):
>   dt-bindings: arm: qcom: add the SoC ID for SA8255P
>   soc: qcom: socinfo: add support for SA8255P
>   dt-bindings: arm: qcom: add SA8255p Ride board
>   dt-bindings: firmware: qcom,scm: document support for SA8255p
>   dt-bindings: mailbox: qcom-ipcc: document the support for SA8255p
>   dt-bindings: watchdog: qcom-wdt: document support on SA8255p
>   dt-bindings: crypto: qcom,prng: document support for SA8255p
>   dt-bindings: interrupt-controller: qcom-pdc: document support for
>     SA8255p
>   dt-bindings: soc: qcom: aoss-qmp: document support for SA8255p
>   dt-bindings: arm-smmu: document the support on SA8255p
>   dt-bindings: mfd: qcom,tcsr: document support for SA8255p
>   dt-bindings: thermal: tsens: document support on SA8255p
>   dt-bindings: pinctrl: Add SA8255p TLMM
>   dt-bindings: cpufreq: qcom-hw: document support for SA8255p
>   dt-bindings: i2c: document support for SA8255p
>   dt-bindings: spi: document support for SA8255p
>   dt-bindings: serial: document support for SA8255p
>   dt-bindings: qcom: geni-se: document support for SA8255P
>   dt-bindings: firmware: arm,scmi: allow multiple virtual instances
>   dt-bindings: arm: GIC: add ESPI and EPPI specifiers
>   arm64: dts: qcom: Add reduced functional DT for SA8255p Ride platform
>
>  .../devicetree/bindings/arm/qcom.yaml         |    6 +
>  .../bindings/cpufreq/cpufreq-qcom-hw.yaml     |   16 +
>  .../devicetree/bindings/crypto/qcom,prng.yaml |    1 +
>  .../bindings/firmware/arm,scmi.yaml           |    2 +-
>  .../bindings/firmware/qcom,scm.yaml           |    2 +
>  .../bindings/i2c/qcom,i2c-geni-qcom.yaml      |   33 +-
>  .../interrupt-controller/qcom,pdc.yaml        |    1 +
>  .../devicetree/bindings/iommu/arm,smmu.yaml   |    3 +
>  .../bindings/mailbox/qcom-ipcc.yaml           |    1 +
>  .../devicetree/bindings/mfd/qcom,tcsr.yaml    |    1 +
>  .../bindings/pinctrl/qcom,sa8775p-tlmm.yaml   |    8 +-
>  .../serial/qcom,serial-geni-qcom.yaml         |   53 +-
>  .../bindings/soc/qcom/qcom,aoss-qmp.yaml      |    1 +
>  .../bindings/soc/qcom/qcom,geni-se.yaml       |   45 +-
>  .../bindings/spi/qcom,spi-geni-qcom.yaml      |   60 +-
>  .../bindings/thermal/qcom-tsens.yaml          |    1 +
>  .../bindings/watchdog/qcom-wdt.yaml           |    1 +
>  arch/arm64/boot/dts/qcom/Makefile             |    1 +
>  arch/arm64/boot/dts/qcom/sa8255p-pmics.dtsi   |   80 +
>  arch/arm64/boot/dts/qcom/sa8255p-ride.dts     |  148 +
>  arch/arm64/boot/dts/qcom/sa8255p-scmi.dtsi    | 2312 ++++++++++++++++
>  arch/arm64/boot/dts/qcom/sa8255p.dtsi         | 2405 +++++++++++++++++
>  drivers/soc/qcom/socinfo.c                    |    1 +
>  include/dt-bindings/arm/qcom,ids.h            |    1 +
>  .../interrupt-controller/arm-gic.h            |    2 +
>  25 files changed, 5169 insertions(+), 16 deletions(-)
>  create mode 100644 arch/arm64/boot/dts/qcom/sa8255p-pmics.dtsi
>  create mode 100644 arch/arm64/boot/dts/qcom/sa8255p-ride.dts
>  create mode 100644 arch/arm64/boot/dts/qcom/sa8255p-scmi.dtsi
>  create mode 100644 arch/arm64/boot/dts/qcom/sa8255p.dtsi
>
>
> base-commit: 6804f0edbe7747774e6ae60f20cec4ee3ad7c187
Nikunj Kela Sept. 9, 2024, 8:29 p.m. UTC | #18
On 9/4/2024 6:21 AM, Krzysztof Kozlowski wrote:
> On 04/09/2024 14:48, Nikunj Kela wrote:
>> On 9/3/2024 11:34 PM, Krzysztof Kozlowski wrote:
>>> 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.
>> Where did I miss PATCH keyword in the subject here? It says "[PATCH v2
>> 16/21] dt-bindings: spi: document support for SA8255p"
> Oh, wrong template. It was about spi prefix, 

These are the latest 4 commits in linux-next for spi:

12736adc43b7 dt-bindings: spi: nxp-fspi: add imx8ulp support
b0cdf9cc0895 spi: dt-bindings: Add rockchip,rk3576-spi compatible
d6d0af1b9eff dt-bindings: spi: add PIC64GX SPI/QSPI compatibility to
MPFS SPI/QSPI bindings
1c4d834e4e81 spi: dt-bindings: convert spi-sc18is602.txt to yaml format

Now I am confused which prefix format shall I use? first spi or first
dt-bindings?


> should be this one:
>
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching. For bindings, the preferred subjects are
> explained here:
> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
>
> Best regards,
> Krzysztof
>
Mark Brown Sept. 9, 2024, 10 p.m. UTC | #19
On Mon, Sep 09, 2024 at 01:29:37PM -0700, Nikunj Kela wrote:

> Now I am confused which prefix format shall I use? first spi or first
> dt-bindings?

spi: first.