Message ID | 20240828203721.2751904-1-quic_nkela@quicinc.com |
---|---|
Headers | show |
Series | arm64: qcom: Introduce SA8255p Ride platform | expand |
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
On Tue, 03 Sep 2024 15:02:32 -0700, Nikunj Kela wrote: > Add compatible for TLMM block representing support on SA8255p. > > SA8255p uses the same TLMM block as SA8775p however the ownership > of pins are split between Firmware VM and Linux VM on SA8255p. For > example, pins used by UART are owned and configured by Firmware VM > while pins used by ethernet are owned and configured by Linux VM. > Therefore, adding a sa8255p specific compatible to mark the difference. > > Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com> > --- > .../devicetree/bindings/pinctrl/qcom,sa8775p-tlmm.yaml | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: ./Documentation/devicetree/bindings/pinctrl/qcom,sa8775p-tlmm.yaml:22:9: [warning] wrong indentation: expected 10 but found 8 (indentation) ./Documentation/devicetree/bindings/pinctrl/qcom,sa8775p-tlmm.yaml:23:11: [warning] wrong indentation: expected 12 but found 10 (indentation) ./Documentation/devicetree/bindings/pinctrl/qcom,sa8775p-tlmm.yaml:26:9: [warning] wrong indentation: expected 10 but found 8 (indentation) dtschema/dtc warnings/errors: doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240903220240.2594102-14-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 Tue, Sep 03, 2024 at 03:02:33PM -0700, Nikunj Kela wrote: > Add compatible for the cpufreq engine representing support on SA8255p. > > Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com> > --- > .../bindings/cpufreq/cpufreq-qcom-hw.yaml | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml > index 1e9797f96410..84865e553c8b 100644 > --- a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml > +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml > @@ -34,6 +34,7 @@ properties: > items: > - enum: > - qcom,qdu1000-cpufreq-epss > + - qcom,sa8255p-cpufreq-epss > - qcom,sa8775p-cpufreq-epss > - qcom,sc7280-cpufreq-epss > - qcom,sc8280xp-cpufreq-epss > @@ -206,6 +207,21 @@ allOf: > interrupt-names: > minItems: 2 > > + - if: > + properties: > + compatible: > + contains: > + enum: > + - qcom,sa8255p-cpufreq-epss > + then: > + properties: > + reg: > + minItems: 2 > + maxItems: 2 > + > + reg-names: > + minItems: 2 > + maxItems: 2 What about interrupts? You need to constrain each of such lists. Best regards, Krzysztof
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 Tue, Sep 03, 2024 at 03:02:38PM -0700, Nikunj Kela wrote: > This change extends scmi node name so as to allow multiple virtual > SCMI instances. > > Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com> > --- > Documentation/devicetree/bindings/firmware/arm,scmi.yaml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 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
> Just to clarify to I2C maintainers: > This is incomplete. Missing driver changes. Thanks, Krzysztof!
On 9/3/2024 11:26 PM, Krzysztof Kozlowski wrote: > On Tue, Sep 03, 2024 at 03:02:33PM -0700, Nikunj Kela wrote: >> Add compatible for the cpufreq engine representing support on SA8255p. >> >> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com> >> --- >> .../bindings/cpufreq/cpufreq-qcom-hw.yaml | 16 ++++++++++++++++ >> 1 file changed, 16 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml >> index 1e9797f96410..84865e553c8b 100644 >> --- a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml >> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml >> @@ -34,6 +34,7 @@ properties: >> items: >> - enum: >> - qcom,qdu1000-cpufreq-epss >> + - qcom,sa8255p-cpufreq-epss >> - qcom,sa8775p-cpufreq-epss >> - qcom,sc7280-cpufreq-epss >> - qcom,sc8280xp-cpufreq-epss >> @@ -206,6 +207,21 @@ allOf: >> interrupt-names: >> minItems: 2 >> >> + - if: >> + properties: >> + compatible: >> + contains: >> + enum: >> + - qcom,sa8255p-cpufreq-epss >> + then: >> + properties: >> + reg: >> + minItems: 2 >> + maxItems: 2 >> + >> + reg-names: >> + minItems: 2 >> + maxItems: 2 > What about interrupts? You need to constrain each of such lists. > > Best regards, > Krzysztof Interrupts are not required, I still need to put constraints for interrupts? BTW, there is no if block for SA8775p binding in this file. Thanks, -Nikunj
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. [1]: https://git.codelinaro.org/clo/linux-kernel/kernel-qcom/-/tree/nkela/sa8255p_v6_11_rc2?ref_type=heads
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" > 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 I will put more details in the description of the patch, though, I had put the description in the cover letter for this entire series. >
On 9/4/2024 12:47 AM, Krzysztof Kozlowski wrote: > 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 The driver changes will soon be posted. They are being reviewed internally. For a quick look on what is coming next, you can refer to CodeLinaro git repo[1] [1]: https://git.codelinaro.org/clo/linux-kernel/kernel-qcom/-/tree/nkela/sa8255p_v6_11_rc2?ref_type=heads
On 04/09/2024 14:56, Nikunj Kela wrote: > > On 9/4/2024 12:47 AM, Krzysztof Kozlowski wrote: >> 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 > > The driver changes will soon be posted. They are being reviewed > internally. For a quick look on what is coming next, you can refer to > CodeLinaro git repo[1] Upstream does not work like that. This patch is just wrong and pointless without driver change. Never send such stuff separately from the driver. Or fix the binding, if the intention was there is no driver. Best regards, Krzysztof
On 04/09/2024 14:41, Nikunj Kela wrote: > > 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. It is not obvious to me. I doubt it is obvious to others. Commit msg does not say they are compatible and usually difference in clocks/interconnects is not reason of incompatibility. So why suddenly here we would understand it differently? > > >> 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? I proposed separate binding file. But your commit msg suggested these are compatible. Lack of driver change is also proof of that. I don't want to keep discussing this because it does not lead to anywhere. We keep repeating the same. > > >> >> 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. You just affected all the DTS everywhere. It's your task to check all DTS everywhere. Not ours. Best regards, Krzysztof
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, 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
On 04/09/2024 16:19, Nikunj Kela wrote: > > On 9/4/2024 6:17 AM, Krzysztof Kozlowski wrote: >> On 04/09/2024 14:27, Nikunj Kela wrote: >>> On 9/3/2024 11:26 PM, Krzysztof Kozlowski wrote: >>>> On Tue, Sep 03, 2024 at 03:02:33PM -0700, Nikunj Kela wrote: >>>>> Add compatible for the cpufreq engine representing support on SA8255p. >>>>> >>>>> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com> >>>>> --- >>>>> .../bindings/cpufreq/cpufreq-qcom-hw.yaml | 16 ++++++++++++++++ >>>>> 1 file changed, 16 insertions(+) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml >>>>> index 1e9797f96410..84865e553c8b 100644 >>>>> --- a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml >>>>> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.yaml >>>>> @@ -34,6 +34,7 @@ properties: >>>>> items: >>>>> - enum: >>>>> - qcom,qdu1000-cpufreq-epss >>>>> + - qcom,sa8255p-cpufreq-epss >>>>> - qcom,sa8775p-cpufreq-epss >>>>> - qcom,sc7280-cpufreq-epss >>>>> - qcom,sc8280xp-cpufreq-epss >>>>> @@ -206,6 +207,21 @@ allOf: >>>>> interrupt-names: >>>>> minItems: 2 >>>>> >>>>> + - if: >>>>> + properties: >>>>> + compatible: >>>>> + contains: >>>>> + enum: >>>>> + - qcom,sa8255p-cpufreq-epss >>>>> + then: >>>>> + properties: >>>>> + reg: >>>>> + minItems: 2 >>>>> + maxItems: 2 >>>>> + >>>>> + reg-names: >>>>> + minItems: 2 >>>>> + maxItems: 2 >>>> What about interrupts? You need to constrain each of such lists. >>>> >>>> Best regards, >>>> Krzysztof >>> Interrupts are not required, I still need to put constraints for >> It's irrelevant whether they are required or not. Each property should >> be narrowed. > > So evenif we don't use interrupts property in our DT(patch#21), we need > to mention interrupts here? You suggest we put interrupts with maxItems: 0? I don't understand. You use three quite separate statements. "Not required", "don't use" and here "maxItems: 0" which means not allowed. All of these mean something else and I keep guessing and responding according to what you write. Probably half of my advises are just trash, because it turns out it is something entirely else than what I read. Make a decision how the hardware looks like. > > I wonder why SA8775p compatible is not in constraint list.. > >>> interrupts? BTW, there is no if block for SA8775p binding in this file. >> >> >> Best regards, >> Krzysztof >> Best regards, Krzysztof
> The driver changes will soon be posted. They are being reviewed > internally. And what do you do when internal reviewers tell you that everything is wrong and you need to change the binding? You just wasted a lot of peoples time. Please don't post patches until you know they are correct, complete, build W=1, and pass all the standard static analysers. I suggest you try to find an experience Mainline developer who can mentor you. Andrew
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
> 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
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
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 >
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.