mbox series

[00/19] Add initial support for QCS8300

Message ID 20240904-qcs8300_initial_dtsi-v1-0-d0ea9afdc007@quicinc.com (mailing list archive)
Headers show
Series Add initial support for QCS8300 | expand

Message

Jingyi Wang Sept. 4, 2024, 8:33 a.m. UTC
Add initial support for QCS8300 SoC and QCS8300 RIDE board.

This revision brings support for:
- CPUs with cpu idle
- interrupt-controller with PDC wakeup support
- gcc
- TLMM
- interconnect
- qup with uart
- smmu
- pmic
- ufs
- ipcc
- sram
- remoteprocs including ADSP,CDSP and GPDSP

Signed-off-by: Jingyi Wang <quic_jingyw@quicinc.com>
---
patch series organized as:
- 1-2: remoteproc binding and driver
- 3-5: ufs binding and driver
- 6-7: rpmhpd binding and driver
- 8-15: bindings for other components found on the SoC
- 16-19: changes to support the device tree

dependencies:
tlmm: https://lore.kernel.org/linux-arm-msm/20240819064933.1778204-1-quic_jingyw@quicinc.com/
gcc: https://lore.kernel.org/all/20240820-qcs8300-gcc-v1-0-d81720517a82@quicinc.com/
interconnect: https://lore.kernel.org/linux-arm-msm/20240827151622.305-1-quic_rlaggysh@quicinc.com/

dtb check got following err:
/local/mnt/workspace/jingyi/aim500/linux/arch/arm64/boot/dts/qcom/qcs8300-ride.dtb: interconnect@1680000: Unevaluated properties are not allowed ('reg' was unexpected)
which is cause by "reg" compatible missing in dt binding, will be fixed in interconnect patch series.

---
Jingyi Wang (11):
      dt-bindings: remoteproc: qcom,sa8775p-pas: Document QCS8300 remoteproc
      remoteproc: qcom: pas: Add QCS8300 remoteproc support
      dt-bindings: qcom,pdc: document QCS8300 Power Domain Controller
      dt-bindings: soc: qcom: add qcom,qcs8300-imem compatible
      dt-bindings: mailbox: qcom-ipcc: Document QCS8300 IPCC
      dt-bindings: mfd: qcom,tcsr: Add compatible for QCS8300
      dt-bindings: nvmem: qfprom: Add compatible for QCS8300
      dt-bindings: arm: qcom: document QCS8275/QCS8300 SoC and reference board
      arm64: defconfig: enable clock controller, interconnect and pinctrl for QCS8300
      arm64: dts: qcom: add initial support for QCS8300 DTSI
      arm64: dts: qcom: add base QCS8300 RIDE dts

Kyle Deng (1):
      dt-bindings: soc: qcom,aoss-qmp: Document the QCS8300 AOSS channel

Shazad Hussain (1):
      dt-bindings: power: rpmpd: Add QCS8300 power domains

Tingguo Cheng (1):
      pmdomain: qcom: rpmhpd: Add QCS8300 power domains

Xin Liu (3):
      dt-bindings: phy: Add QMP UFS PHY comptible for QCS8300
      dt-bindings: ufs: qcom: Document the QCS8300 UFS Controller
      phy: qcom-qmp-ufs: Add support for QCS8300

Zhenhua Huang (2):
      dt-bindings: arm-smmu: Add compatible for QCS8300 SoC
      dt-bindings: firmware: qcom,scm: document SCM on QCS8300 SoCs

 Documentation/devicetree/bindings/arm/qcom.yaml    |    8 +
 .../devicetree/bindings/firmware/qcom,scm.yaml     |    1 +
 .../bindings/interrupt-controller/qcom,pdc.yaml    |    1 +
 .../devicetree/bindings/iommu/arm,smmu.yaml        |    2 +
 .../devicetree/bindings/mailbox/qcom-ipcc.yaml     |    1 +
 .../devicetree/bindings/mfd/qcom,tcsr.yaml         |    1 +
 .../devicetree/bindings/nvmem/qcom,qfprom.yaml     |    1 +
 .../bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml    |    2 +
 .../devicetree/bindings/power/qcom,rpmpd.yaml      |    1 +
 .../bindings/remoteproc/qcom,sa8775p-pas.yaml      |    6 +
 .../bindings/soc/qcom/qcom,aoss-qmp.yaml           |    1 +
 .../devicetree/bindings/sram/qcom,imem.yaml        |    1 +
 .../devicetree/bindings/ufs/qcom,ufs.yaml          |    2 +
 arch/arm64/boot/dts/qcom/Makefile                  |    1 +
 arch/arm64/boot/dts/qcom/qcs8300-ride.dts          |  246 ++++
 arch/arm64/boot/dts/qcom/qcs8300.dtsi              | 1282 ++++++++++++++++++++
 arch/arm64/configs/defconfig                       |    3 +
 drivers/phy/qualcomm/phy-qcom-qmp-ufs.c            |    3 +
 drivers/pmdomain/qcom/rpmhpd.c                     |   24 +
 drivers/remoteproc/qcom_q6v5_pas.c                 |    3 +
 include/dt-bindings/power/qcom-rpmpd.h             |   19 +
 21 files changed, 1609 insertions(+)
---
base-commit: eb8c5ca373cbb018a84eb4db25c863302c9b6314
change-id: 20240829-qcs8300_initial_dtsi-1a386eb317d3

Best regards,

Comments

Krzysztof Kozlowski Sept. 4, 2024, 9:34 a.m. UTC | #1
On 04/09/2024 10:33, Jingyi Wang wrote:
> Add initial support for QCS8300 SoC and QCS8300 RIDE board.
> 
> This revision brings support for:
> - CPUs with cpu idle
> - interrupt-controller with PDC wakeup support
> - gcc
> - TLMM
> - interconnect
> - qup with uart
> - smmu
> - pmic
> - ufs
> - ipcc
> - sram
> - remoteprocs including ADSP,CDSP and GPDSP
> 
> Signed-off-by: Jingyi Wang <quic_jingyw@quicinc.com>
> ---
> patch series organized as:
> - 1-2: remoteproc binding and driver
> - 3-5: ufs binding and driver
> - 6-7: rpmhpd binding and driver
> - 8-15: bindings for other components found on the SoC

Limit your CC list. I found like 8 unnecessary addresses for already
huge Cc list. Or organize your patches per subsystem, as we usually expect.

> - 16-19: changes to support the device tree
> 
> dependencies:
> tlmm: https://lore.kernel.org/linux-arm-msm/20240819064933.1778204-1-quic_jingyw@quicinc.com/
> gcc: https://lore.kernel.org/all/20240820-qcs8300-gcc-v1-0-d81720517a82@quicinc.com/
> interconnect: https://lore.kernel.org/linux-arm-msm/20240827151622.305-1-quic_rlaggysh@quicinc.com/

Why? UFS cannot depend on pinctrl for example.

This blocks testing and merging.

Please organize properly (so decouple) your patches, so that there is no
fake dependency.

Best regards,
Krzysztof
Krzysztof Kozlowski Sept. 4, 2024, 10:19 a.m. UTC | #2
On 04/09/2024 11:34, Krzysztof Kozlowski wrote:
> On 04/09/2024 10:33, Jingyi Wang wrote:
>> Add initial support for QCS8300 SoC and QCS8300 RIDE board.
>>
>> This revision brings support for:
>> - CPUs with cpu idle
>> - interrupt-controller with PDC wakeup support
>> - gcc
>> - TLMM
>> - interconnect
>> - qup with uart
>> - smmu
>> - pmic
>> - ufs
>> - ipcc
>> - sram
>> - remoteprocs including ADSP,CDSP and GPDSP
>>
>> Signed-off-by: Jingyi Wang <quic_jingyw@quicinc.com>
>> ---
>> patch series organized as:
>> - 1-2: remoteproc binding and driver
>> - 3-5: ufs binding and driver
>> - 6-7: rpmhpd binding and driver
>> - 8-15: bindings for other components found on the SoC
> 
> Limit your CC list. I found like 8 unnecessary addresses for already
> huge Cc list. Or organize your patches per subsystem, as we usually expect.
> 
>> - 16-19: changes to support the device tree
>>
>> dependencies:
>> tlmm: https://lore.kernel.org/linux-arm-msm/20240819064933.1778204-1-quic_jingyw@quicinc.com/
>> gcc: https://lore.kernel.org/all/20240820-qcs8300-gcc-v1-0-d81720517a82@quicinc.com/
>> interconnect: https://lore.kernel.org/linux-arm-msm/20240827151622.305-1-quic_rlaggysh@quicinc.com/
> 
> Why? UFS cannot depend on pinctrl for example.
> 
> This blocks testing and merging.
> 
> Please organize properly (so decouple) your patches, so that there is no
> fake dependency.

Let me also add here one more thought. That's like fourth or fifth
QCS/SA patchset last two weeks from Qualcomm and they repeat the same
mistakes. Not correctly organized, huge cc list, same problems with
bindings or drivers.

I am giving much more comments to fix than review/ack tags.

I am not going to review this. I will also slow down with reviewing
other Qualcomm patches. Why? Because you post simultaneously, apparently
you do not learn from other review, so I have to keep repeating the same.

I am overwhelmed with this, so please expect two week review time from me.

Best regards,
Krzysztof
Rob Herring Sept. 4, 2024, 1:36 p.m. UTC | #3
On Wed, 04 Sep 2024 16:33:41 +0800, Jingyi Wang wrote:
> Add initial support for QCS8300 SoC and QCS8300 RIDE board.
> 
> This revision brings support for:
> - CPUs with cpu idle
> - interrupt-controller with PDC wakeup support
> - gcc
> - TLMM
> - interconnect
> - qup with uart
> - smmu
> - pmic
> - ufs
> - ipcc
> - sram
> - remoteprocs including ADSP,CDSP and GPDSP
> 
> Signed-off-by: Jingyi Wang <quic_jingyw@quicinc.com>
> ---
> patch series organized as:
> - 1-2: remoteproc binding and driver
> - 3-5: ufs binding and driver
> - 6-7: rpmhpd binding and driver
> - 8-15: bindings for other components found on the SoC
> - 16-19: changes to support the device tree
> 
> dependencies:
> tlmm: https://lore.kernel.org/linux-arm-msm/20240819064933.1778204-1-quic_jingyw@quicinc.com/
> gcc: https://lore.kernel.org/all/20240820-qcs8300-gcc-v1-0-d81720517a82@quicinc.com/
> interconnect: https://lore.kernel.org/linux-arm-msm/20240827151622.305-1-quic_rlaggysh@quicinc.com/
> 
> dtb check got following err:
> /local/mnt/workspace/jingyi/aim500/linux/arch/arm64/boot/dts/qcom/qcs8300-ride.dtb: interconnect@1680000: Unevaluated properties are not allowed ('reg' was unexpected)
> which is cause by "reg" compatible missing in dt binding, will be fixed in interconnect patch series.
> 
> ---
> Jingyi Wang (11):
>       dt-bindings: remoteproc: qcom,sa8775p-pas: Document QCS8300 remoteproc
>       remoteproc: qcom: pas: Add QCS8300 remoteproc support
>       dt-bindings: qcom,pdc: document QCS8300 Power Domain Controller
>       dt-bindings: soc: qcom: add qcom,qcs8300-imem compatible
>       dt-bindings: mailbox: qcom-ipcc: Document QCS8300 IPCC
>       dt-bindings: mfd: qcom,tcsr: Add compatible for QCS8300
>       dt-bindings: nvmem: qfprom: Add compatible for QCS8300
>       dt-bindings: arm: qcom: document QCS8275/QCS8300 SoC and reference board
>       arm64: defconfig: enable clock controller, interconnect and pinctrl for QCS8300
>       arm64: dts: qcom: add initial support for QCS8300 DTSI
>       arm64: dts: qcom: add base QCS8300 RIDE dts
> 
> Kyle Deng (1):
>       dt-bindings: soc: qcom,aoss-qmp: Document the QCS8300 AOSS channel
> 
> Shazad Hussain (1):
>       dt-bindings: power: rpmpd: Add QCS8300 power domains
> 
> Tingguo Cheng (1):
>       pmdomain: qcom: rpmhpd: Add QCS8300 power domains
> 
> Xin Liu (3):
>       dt-bindings: phy: Add QMP UFS PHY comptible for QCS8300
>       dt-bindings: ufs: qcom: Document the QCS8300 UFS Controller
>       phy: qcom-qmp-ufs: Add support for QCS8300
> 
> Zhenhua Huang (2):
>       dt-bindings: arm-smmu: Add compatible for QCS8300 SoC
>       dt-bindings: firmware: qcom,scm: document SCM on QCS8300 SoCs
> 
>  Documentation/devicetree/bindings/arm/qcom.yaml    |    8 +
>  .../devicetree/bindings/firmware/qcom,scm.yaml     |    1 +
>  .../bindings/interrupt-controller/qcom,pdc.yaml    |    1 +
>  .../devicetree/bindings/iommu/arm,smmu.yaml        |    2 +
>  .../devicetree/bindings/mailbox/qcom-ipcc.yaml     |    1 +
>  .../devicetree/bindings/mfd/qcom,tcsr.yaml         |    1 +
>  .../devicetree/bindings/nvmem/qcom,qfprom.yaml     |    1 +
>  .../bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml    |    2 +
>  .../devicetree/bindings/power/qcom,rpmpd.yaml      |    1 +
>  .../bindings/remoteproc/qcom,sa8775p-pas.yaml      |    6 +
>  .../bindings/soc/qcom/qcom,aoss-qmp.yaml           |    1 +
>  .../devicetree/bindings/sram/qcom,imem.yaml        |    1 +
>  .../devicetree/bindings/ufs/qcom,ufs.yaml          |    2 +
>  arch/arm64/boot/dts/qcom/Makefile                  |    1 +
>  arch/arm64/boot/dts/qcom/qcs8300-ride.dts          |  246 ++++
>  arch/arm64/boot/dts/qcom/qcs8300.dtsi              | 1282 ++++++++++++++++++++
>  arch/arm64/configs/defconfig                       |    3 +
>  drivers/phy/qualcomm/phy-qcom-qmp-ufs.c            |    3 +
>  drivers/pmdomain/qcom/rpmhpd.c                     |   24 +
>  drivers/remoteproc/qcom_q6v5_pas.c                 |    3 +
>  include/dt-bindings/power/qcom-rpmpd.h             |   19 +
>  21 files changed, 1609 insertions(+)
> ---
> base-commit: eb8c5ca373cbb018a84eb4db25c863302c9b6314
> change-id: 20240829-qcs8300_initial_dtsi-1a386eb317d3
> 
> Best regards,
> --
> Jingyi Wang <quic_jingyw@quicinc.com>
> 
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


New warnings running 'make CHECK_DTBS=y qcom/qcs8300-ride.dtb' for 20240904-qcs8300_initial_dtsi-v1-0-d0ea9afdc007@quicinc.com:

In file included from arch/arm64/boot/dts/qcom/qcs8300-ride.dts:11:
arch/arm64/boot/dts/qcom/qcs8300.dtsi:6:10: fatal error: dt-bindings/clock/qcom,qcs8300-gcc.h: No such file or directory
    6 | #include <dt-bindings/clock/qcom,qcs8300-gcc.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[3]: *** [scripts/Makefile.lib:434: arch/arm64/boot/dts/qcom/qcs8300-ride.dtb] Error 1
make[2]: *** [scripts/Makefile.build:490: arch/arm64/boot/dts/qcom] Error 2
make[2]: Target 'arch/arm64/boot/dts/qcom/qcs8300-ride.dtb' not remade because of errors.
make[1]: *** [/home/rob/proj/linux-dt-testing/Makefile:1390: qcom/qcs8300-ride.dtb] Error 2
make: *** [Makefile:224: __sub-make] Error 2
make: Target 'qcom/qcs8300-ride.dtb' not remade because of errors.
Jingyi Wang Sept. 5, 2024, 2:45 a.m. UTC | #4
Hi Rob,

On 9/4/2024 9:36 PM, Rob Herring (Arm) wrote:
> 
> On Wed, 04 Sep 2024 16:33:41 +0800, Jingyi Wang wrote:
>> Add initial support for QCS8300 SoC and QCS8300 RIDE board.
>>
>> This revision brings support for:
>> - CPUs with cpu idle
>> - interrupt-controller with PDC wakeup support
>> - gcc
>> - TLMM
>> - interconnect
>> - qup with uart
>> - smmu
>> - pmic
>> - ufs
>> - ipcc
>> - sram
>> - remoteprocs including ADSP,CDSP and GPDSP
>>
>> Signed-off-by: Jingyi Wang <quic_jingyw@quicinc.com>
>> ---
>> patch series organized as:
>> - 1-2: remoteproc binding and driver
>> - 3-5: ufs binding and driver
>> - 6-7: rpmhpd binding and driver
>> - 8-15: bindings for other components found on the SoC
>> - 16-19: changes to support the device tree
>>
>> dependencies:
>> tlmm: https://lore.kernel.org/linux-arm-msm/20240819064933.1778204-1-quic_jingyw@quicinc.com/
>> gcc: https://lore.kernel.org/all/20240820-qcs8300-gcc-v1-0-d81720517a82@quicinc.com/
>> interconnect: https://lore.kernel.org/linux-arm-msm/20240827151622.305-1-quic_rlaggysh@quicinc.com/
>>
>> dtb check got following err:
>> /local/mnt/workspace/jingyi/aim500/linux/arch/arm64/boot/dts/qcom/qcs8300-ride.dtb: interconnect@1680000: Unevaluated properties are not allowed ('reg' was unexpected)
>> which is cause by "reg" compatible missing in dt binding, will be fixed in interconnect patch series.
>>
>> ---
>> Jingyi Wang (11):
>>       dt-bindings: remoteproc: qcom,sa8775p-pas: Document QCS8300 remoteproc
>>       remoteproc: qcom: pas: Add QCS8300 remoteproc support
>>       dt-bindings: qcom,pdc: document QCS8300 Power Domain Controller
>>       dt-bindings: soc: qcom: add qcom,qcs8300-imem compatible
>>       dt-bindings: mailbox: qcom-ipcc: Document QCS8300 IPCC
>>       dt-bindings: mfd: qcom,tcsr: Add compatible for QCS8300
>>       dt-bindings: nvmem: qfprom: Add compatible for QCS8300
>>       dt-bindings: arm: qcom: document QCS8275/QCS8300 SoC and reference board
>>       arm64: defconfig: enable clock controller, interconnect and pinctrl for QCS8300
>>       arm64: dts: qcom: add initial support for QCS8300 DTSI
>>       arm64: dts: qcom: add base QCS8300 RIDE dts
>>
>> Kyle Deng (1):
>>       dt-bindings: soc: qcom,aoss-qmp: Document the QCS8300 AOSS channel
>>
>> Shazad Hussain (1):
>>       dt-bindings: power: rpmpd: Add QCS8300 power domains
>>
>> Tingguo Cheng (1):
>>       pmdomain: qcom: rpmhpd: Add QCS8300 power domains
>>
>> Xin Liu (3):
>>       dt-bindings: phy: Add QMP UFS PHY comptible for QCS8300
>>       dt-bindings: ufs: qcom: Document the QCS8300 UFS Controller
>>       phy: qcom-qmp-ufs: Add support for QCS8300
>>
>> Zhenhua Huang (2):
>>       dt-bindings: arm-smmu: Add compatible for QCS8300 SoC
>>       dt-bindings: firmware: qcom,scm: document SCM on QCS8300 SoCs
>>
>>  Documentation/devicetree/bindings/arm/qcom.yaml    |    8 +
>>  .../devicetree/bindings/firmware/qcom,scm.yaml     |    1 +
>>  .../bindings/interrupt-controller/qcom,pdc.yaml    |    1 +
>>  .../devicetree/bindings/iommu/arm,smmu.yaml        |    2 +
>>  .../devicetree/bindings/mailbox/qcom-ipcc.yaml     |    1 +
>>  .../devicetree/bindings/mfd/qcom,tcsr.yaml         |    1 +
>>  .../devicetree/bindings/nvmem/qcom,qfprom.yaml     |    1 +
>>  .../bindings/phy/qcom,sc8280xp-qmp-ufs-phy.yaml    |    2 +
>>  .../devicetree/bindings/power/qcom,rpmpd.yaml      |    1 +
>>  .../bindings/remoteproc/qcom,sa8775p-pas.yaml      |    6 +
>>  .../bindings/soc/qcom/qcom,aoss-qmp.yaml           |    1 +
>>  .../devicetree/bindings/sram/qcom,imem.yaml        |    1 +
>>  .../devicetree/bindings/ufs/qcom,ufs.yaml          |    2 +
>>  arch/arm64/boot/dts/qcom/Makefile                  |    1 +
>>  arch/arm64/boot/dts/qcom/qcs8300-ride.dts          |  246 ++++
>>  arch/arm64/boot/dts/qcom/qcs8300.dtsi              | 1282 ++++++++++++++++++++
>>  arch/arm64/configs/defconfig                       |    3 +
>>  drivers/phy/qualcomm/phy-qcom-qmp-ufs.c            |    3 +
>>  drivers/pmdomain/qcom/rpmhpd.c                     |   24 +
>>  drivers/remoteproc/qcom_q6v5_pas.c                 |    3 +
>>  include/dt-bindings/power/qcom-rpmpd.h             |   19 +
>>  21 files changed, 1609 insertions(+)
>> ---
>> base-commit: eb8c5ca373cbb018a84eb4db25c863302c9b6314
>> change-id: 20240829-qcs8300_initial_dtsi-1a386eb317d3
>>
>> Best regards,
>> --
>> Jingyi Wang <quic_jingyw@quicinc.com>
>>
>>
>>
> 
> 
> My bot found new DTB warnings on the .dts files added or changed in this
> series.
> 
> Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
> are fixed by another series. Ultimately, it is up to the platform
> maintainer whether these warnings are acceptable or not. No need to reply
> unless the platform maintainer has comments.
> 
> If you already ran DT checks and didn't see these error(s), then
> make sure dt-schema is up to date:
> 
>   pip3 install dtschema --upgrade
> 
> 
> New warnings running 'make CHECK_DTBS=y qcom/qcs8300-ride.dtb' for 20240904-qcs8300_initial_dtsi-v1-0-d0ea9afdc007@quicinc.com:
> 
> In file included from arch/arm64/boot/dts/qcom/qcs8300-ride.dts:11:
> arch/arm64/boot/dts/qcom/qcs8300.dtsi:6:10: fatal error: dt-bindings/clock/qcom,qcs8300-gcc.h: No such file or directory
>     6 | #include <dt-bindings/clock/qcom,qcs8300-gcc.h>
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> compilation terminated.
> make[3]: *** [scripts/Makefile.lib:434: arch/arm64/boot/dts/qcom/qcs8300-ride.dtb] Error 1
> make[2]: *** [scripts/Makefile.build:490: arch/arm64/boot/dts/qcom] Error 2
> make[2]: Target 'arch/arm64/boot/dts/qcom/qcs8300-ride.dtb' not remade because of errors.
> make[1]: *** [/home/rob/proj/linux-dt-testing/Makefile:1390: qcom/qcs8300-ride.dtb] Error 2
> make: *** [Makefile:224: __sub-make] Error 2
> make: Target 'qcom/qcs8300-ride.dtb' not remade because of errors.
> 
> 
> 
> 
> 
As mentioned in the cover letter, the dtsi is depend on the gcc patch series:
https://lore.kernel.org/all/20240820-qcs8300-gcc-v1-0-d81720517a82@quicinc.com/
which includes the qcom,qcs8300-gcc.h file.

Thanks,
Jingyi
Jingyi Wang Sept. 5, 2024, 5:08 a.m. UTC | #5
Hi Krzysztof,

On 9/4/2024 6:19 PM, Krzysztof Kozlowski wrote:
> On 04/09/2024 11:34, Krzysztof Kozlowski wrote:
>> On 04/09/2024 10:33, Jingyi Wang wrote:
>>> Add initial support for QCS8300 SoC and QCS8300 RIDE board.
>>>
>>> This revision brings support for:
>>> - CPUs with cpu idle
>>> - interrupt-controller with PDC wakeup support
>>> - gcc
>>> - TLMM
>>> - interconnect
>>> - qup with uart
>>> - smmu
>>> - pmic
>>> - ufs
>>> - ipcc
>>> - sram
>>> - remoteprocs including ADSP,CDSP and GPDSP
>>>
>>> Signed-off-by: Jingyi Wang <quic_jingyw@quicinc.com>
>>> ---
>>> patch series organized as:
>>> - 1-2: remoteproc binding and driver
>>> - 3-5: ufs binding and driver
>>> - 6-7: rpmhpd binding and driver
>>> - 8-15: bindings for other components found on the SoC
>>
>> Limit your CC list. I found like 8 unnecessary addresses for already
>> huge Cc list. Or organize your patches per subsystem, as we usually expect.
>>
>>> - 16-19: changes to support the device tree
>>>
>>> dependencies:
>>> tlmm: https://lore.kernel.org/linux-arm-msm/20240819064933.1778204-1-quic_jingyw@quicinc.com/
>>> gcc: https://lore.kernel.org/all/20240820-qcs8300-gcc-v1-0-d81720517a82@quicinc.com/
>>> interconnect: https://lore.kernel.org/linux-arm-msm/20240827151622.305-1-quic_rlaggysh@quicinc.com/
>>
>> Why? UFS cannot depend on pinctrl for example.
>>
>> This blocks testing and merging.
>>
>> Please organize properly (so decouple) your patches, so that there is no
>> fake dependency.
> 
> Let me also add here one more thought. That's like fourth or fifth
> QCS/SA patchset last two weeks from Qualcomm and they repeat the same
> mistakes. Not correctly organized, huge cc list, same problems with
> bindings or drivers.
> 
> I am giving much more comments to fix than review/ack tags.
> 
> I am not going to review this. I will also slow down with reviewing
> other Qualcomm patches. Why? Because you post simultaneously, apparently
> you do not learn from other review, so I have to keep repeating the same.
> 
> I am overwhelmed with this, so please expect two week review time from me.
> 
> Best regards,
> Krzysztof
> 
The CC list is generated from B4 tool, however, thanks for your advice and we
will decouple the changes to avoid this. And could you please help us to confirm
the better way to handle binding changes which just add one compatible, should
it be submitted as a single patch or submmitted together with dts patch series?

Thanks,
Jingyi
Konrad Dybcio Sept. 5, 2024, 11:33 a.m. UTC | #6
On 5.09.2024 7:08 AM, Jingyi Wang wrote:
> Hi Krzysztof,
> 
> On 9/4/2024 6:19 PM, Krzysztof Kozlowski wrote:
>> On 04/09/2024 11:34, Krzysztof Kozlowski wrote:
>>> On 04/09/2024 10:33, Jingyi Wang wrote:
>>>> Add initial support for QCS8300 SoC and QCS8300 RIDE board.
>>>>
>>>> This revision brings support for:
>>>> - CPUs with cpu idle
>>>> - interrupt-controller with PDC wakeup support
>>>> - gcc
>>>> - TLMM
>>>> - interconnect
>>>> - qup with uart
>>>> - smmu
>>>> - pmic
>>>> - ufs
>>>> - ipcc
>>>> - sram
>>>> - remoteprocs including ADSP,CDSP and GPDSP
>>>>
>>>> Signed-off-by: Jingyi Wang <quic_jingyw@quicinc.com>
>>>> ---
>>>> patch series organized as:
>>>> - 1-2: remoteproc binding and driver
>>>> - 3-5: ufs binding and driver
>>>> - 6-7: rpmhpd binding and driver
>>>> - 8-15: bindings for other components found on the SoC
>>>
>>> Limit your CC list. I found like 8 unnecessary addresses for already
>>> huge Cc list. Or organize your patches per subsystem, as we usually expect.
>>>
>>>> - 16-19: changes to support the device tree
>>>>
>>>> dependencies:
>>>> tlmm: https://lore.kernel.org/linux-arm-msm/20240819064933.1778204-1-quic_jingyw@quicinc.com/
>>>> gcc: https://lore.kernel.org/all/20240820-qcs8300-gcc-v1-0-d81720517a82@quicinc.com/
>>>> interconnect: https://lore.kernel.org/linux-arm-msm/20240827151622.305-1-quic_rlaggysh@quicinc.com/
>>>
>>> Why? UFS cannot depend on pinctrl for example.
>>>
>>> This blocks testing and merging.
>>>
>>> Please organize properly (so decouple) your patches, so that there is no
>>> fake dependency.
>>
>> Let me also add here one more thought. That's like fourth or fifth
>> QCS/SA patchset last two weeks from Qualcomm and they repeat the same
>> mistakes. Not correctly organized, huge cc list, same problems with
>> bindings or drivers.
>>
>> I am giving much more comments to fix than review/ack tags.
>>
>> I am not going to review this. I will also slow down with reviewing
>> other Qualcomm patches. Why? Because you post simultaneously, apparently
>> you do not learn from other review, so I have to keep repeating the same.
>>
>> I am overwhelmed with this, so please expect two week review time from me.
>>
>> Best regards,
>> Krzysztof
>>
> The CC list is generated from B4 tool, however, thanks for your advice and we
> will decouple the changes to avoid this. And could you please help us to confirm
> the better way to handle binding changes which just add one compatible, should
> it be submitted as a single patch or submmitted together with dts patch series?

The tool did its job here, it's just that this series is very long and a ton
of people ended up being involved due to bindings oneliners

Konrad