| Message ID | 20240828203721.2751904-1-quic_nkela@quicinc.com |
|---|---|
| Headers | show |
| Series | arm64: qcom: Introduce SA8255p Ride platform | expand |
On Wed, Aug 28, 2024 at 01:37:01PM -0700, Nikunj Kela wrote: > Add SocInfo support for SA8255P. > > Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com> > --- > drivers/soc/qcom/socinfo.c | 1 + > 1 file changed, 1 insertion(+) Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
On Wed, Aug 28, 2024 at 01:37:05PM -0700, Nikunj Kela wrote: > Add a compatible for the SA8255p platform's KPSS watchdog. > > Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com> > --- > Documentation/devicetree/bindings/watchdog/qcom-wdt.yaml | 1 + > 1 file changed, 1 insertion(+) Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
On Wed, Aug 28, 2024 at 01:37:06PM -0700, Nikunj Kela wrote: > Document SA8255p compatible for the True Random Number Generator. > > Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com> > --- > Documentation/devicetree/bindings/crypto/qcom,prng.yaml | 1 + > 1 file changed, 1 insertion(+) Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
On Wed, Aug 28, 2024 at 01:37:13PM -0700, Nikunj Kela wrote: > Add compatible for smmu representing support on SA8255p. > > Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com> > --- > Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 3 +++ > 1 file changed, 3 insertions(+) > Your subjects contain quite redundant/excessive information. In the same time they lack information about device. 1. s/document the support on/add/ 2. s/SA8255p/SA8255p SMMU-or-whatever-device-it-is/ Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
On Wed, Aug 28, 2024 at 01:37:18PM -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 | 56 ++++++++++++------- > 1 file changed, 36 insertions(+), 20 deletions(-) > > diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml b/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml > index 9f66a3bb1f80..88f513fc5b08 100644 > --- a/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml > +++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml > @@ -15,14 +15,13 @@ properties: > enum: > - qcom,geni-i2c > - qcom,geni-i2c-master-hub > + - qcom,sa8255p-geni-i2c Same as in other patches, this does not make sense. What is the point of generic compatibles? > > clocks: > - minItems: 1 > - maxItems: 2 Nope. > + description: phandles for the clock providers Useless description. This cannot be anything else than phandles for the clock providers. Best regards, Krzysztof
On 28/08/2024 22:37, Nikunj Kela wrote: > Add interrupt specifier for extended SPI interrupts. > > Qualcomm SA8255p platform uses extended SPI for SCMI 'a2p' doorbells. > > Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com> > --- This is still bindings patch. Use proper subject prefix. Best regards, Krzysztof
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
On 8/28/2024 8:06 PM, Rob Herring wrote: > On Wed, Aug 28, 2024 at 01:37:17PM -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. >> >> CC: Praveen Talari <quic_ptalari@quicinc.com> >> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com> >> --- >> .../bindings/spi/qcom,spi-geni-qcom.yaml | 64 +++++++++++++++---- >> 1 file changed, 53 insertions(+), 11 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..74ea7c4f2451 100644 >> --- a/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml >> +++ b/Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml >> @@ -25,10 +25,41 @@ description: >> >> allOf: >> - $ref: /schemas/spi/spi-controller.yaml# >> + - if: >> + properties: >> + compatible: >> + contains: >> + const: qcom,sa8255p-geni-spi >> + then: >> + required: >> + - power-domains >> + - power-domain-names > blank line > >> + properties: >> + power-domains: >> + minItems: 2 >> + maxItems: 2 > Drop maxItems as 2 is already the max (with my change below). > > Add blank line here. > >> + else: >> + required: >> + - clocks >> + - clock-names > blank line > >> + properties: >> + power-domains: >> + maxItems: 1 > blank line > >> + interconnects: >> + minItems: 2 >> + maxItems: 3 > blank line > >> + interconnect-names: >> + minItems: 2 >> + items: >> + - const: qup-core >> + - const: qup-config >> + - const: qup-memory >> >> properties: >> compatible: >> - const: qcom,geni-spi >> + enum: >> + - qcom,geni-spi >> + - qcom,sa8255p-geni-spi >> >> clocks: >> maxItems: 1 >> @@ -45,15 +76,10 @@ properties: >> - const: rx >> >> interconnects: >> - minItems: 2 >> - maxItems: 3 >> + description: phandles of interconnect bw provider >> >> interconnect-names: >> - minItems: 2 >> - items: >> - - const: qup-core >> - - const: qup-config >> - - const: qup-memory >> + description: names of interconnects > No, keep all properties defined at the top-level and then add > constraints in if/then schemas. > >> >> interrupts: >> maxItems: 1 >> @@ -61,15 +87,18 @@ properties: >> operating-points-v2: true >> >> power-domains: >> - maxItems: 1 >> + $ref: "/schemas/power/power-domain.yaml#/properties/power-domains" > Do you see an example of this anywhere else? No. You need: > > minItems: 1 > maxItems: 2 Thanks Rob for reviewing the patch. Will take care of your comments in next version. >> + >> + power-domain-names: >> + items: >> + - const: power >> + - const: perf >> >> reg: >> maxItems: 1 >> >> required: >> - compatible >> - - clocks >> - - clock-names >> - interrupts >> - reg >> >> @@ -116,3 +145,16 @@ examples: >> #address-cells = <1>; >> #size-cells = <0>; >> }; >> + >> + - | >> + #include <dt-bindings/interrupt-controller/arm-gic.h> >> + >> + spi@888000 { >> + compatible = "qcom,sa8255p-geni-spi"; >> + reg = <0x888000 0x4000>; >> + interrupts = <GIC_SPI 584 IRQ_TYPE_LEVEL_HIGH>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + power-domains = <&scmi10_pd 16>, <&scmi10_dvfs 16>; >> + power-domain-names = "power", "perf"; >> + }; >> -- >> 2.34.1 >>
On 8/29/2024 12:57 AM, Krzysztof Kozlowski wrote: > 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 Hi Krzysztof, I ran maintainers script to get all the emails. I kept maintainers, reviewers and "in file" ones in addition to some Qualcomm leads. I will drop "in file" and Qualcomm leads in next version. Thanks, -Nikunj
On 8/29/2024 12:36 AM, Krzysztof Kozlowski wrote: > On Wed, Aug 28, 2024 at 01:37:13PM -0700, Nikunj Kela wrote: >> Add compatible for smmu representing support on SA8255p. >> >> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com> >> --- >> Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 3 +++ >> 1 file changed, 3 insertions(+) >> > Your subjects contain quite redundant/excessive information. In the same > time they lack information about device. > > 1. s/document the support on/add/ > 2. s/SA8255p/SA8255p SMMU-or-whatever-device-it-is/ > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > Best regards, > Krzysztof Okay. I thought arm-smmu tag already indicate which device this patch is for but would put SMMU explicitly in the subject. Thanks, -Nikunj
On 8/29/2024 11:52 AM, Rob Herring wrote: > On Wed, Aug 28, 2024 at 01:37:20PM -0700, Nikunj Kela wrote: >> Add interrupt specifier for extended SPI interrupts. > What's an "extended SPI"? Is this a GIC spec thing? If so, what version? Extended SPI is an extended range of SPI interrupts supported by GIC. Excerpt below from Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml "The 1st cell is the interrupt type; 0 for SPI interrupts, 1 for PPI interrupts, 2 for interrupts in the Extended SPI range, 3 for the Extended PPI range. Other values are reserved for future use." "The 2nd cell contains the interrupt number for the interrupt type. SPI interrupts are in the range [0-987]. PPI interrupts are in the range [0-15]. Extented SPI interrupts are in the range [0-1023]. Extended PPI interrupts are in the range [0-127]." >> Qualcomm SA8255p platform uses extended SPI for SCMI 'a2p' doorbells. >> >> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com> >> --- >> include/dt-bindings/interrupt-controller/arm-gic.h | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/include/dt-bindings/interrupt-controller/arm-gic.h b/include/dt-bindings/interrupt-controller/arm-gic.h >> index 35b6f69b7db6..9c06248446b7 100644 >> --- a/include/dt-bindings/interrupt-controller/arm-gic.h >> +++ b/include/dt-bindings/interrupt-controller/arm-gic.h >> @@ -12,6 +12,7 @@ >> >> #define GIC_SPI 0 >> #define GIC_PPI 1 >> +#define GIC_ESPI 2 >> >> /* >> * Interrupt specifier cell 2. >> -- >> 2.34.1 >>
On 29/08/2024 17:39, Nikunj Kela wrote: > > On 8/29/2024 12:36 AM, Krzysztof Kozlowski wrote: >> On Wed, Aug 28, 2024 at 01:37:13PM -0700, Nikunj Kela wrote: >>> Add compatible for smmu representing support on SA8255p. >>> >>> Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com> >>> --- >>> Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >> Your subjects contain quite redundant/excessive information. In the same >> time they lack information about device. >> >> 1. s/document the support on/add/ >> 2. s/SA8255p/SA8255p SMMU-or-whatever-device-it-is/ >> >> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> >> Best regards, >> Krzysztof > > Okay. I thought arm-smmu tag already indicate which device this patch is > for but would put SMMU explicitly in the subject. arm,smmu indicates the binding file which might be or might not exactly be the same as actual device. Sometimes they have difference names. I am not saying that it would be beneficial here, but some other patches could benefit probably. Best regards, Krzysztof
On Thu, Aug 29, 2024 at 2:02 PM Nikunj Kela <quic_nkela@quicinc.com> wrote: > > > On 8/29/2024 11:52 AM, Rob Herring wrote: > > On Wed, Aug 28, 2024 at 01:37:20PM -0700, Nikunj Kela wrote: > >> Add interrupt specifier for extended SPI interrupts. > > What's an "extended SPI"? Is this a GIC spec thing? If so, what version? > > Extended SPI is an extended range of SPI interrupts supported by GIC. > > Excerpt below from > Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml > > "The 1st cell is the interrupt type; 0 for SPI interrupts, 1 for PPI > interrupts, 2 for interrupts in the Extended SPI range, 3 for the > Extended PPI range. Other values are reserved for future use." > > "The 2nd cell contains the interrupt number for the interrupt type. SPI > interrupts are in the range [0-987]. PPI interrupts are in the range > [0-15]. Extented SPI interrupts are in the range [0-1023]. Extended PPI > interrupts are in the range [0-127]." Looks like you should add EPPI define too while you're here. Rob
On 8/30/2024 7:44 AM, Rob Herring wrote: > On Thu, Aug 29, 2024 at 2:02 PM Nikunj Kela <quic_nkela@quicinc.com> wrote: >> >> On 8/29/2024 11:52 AM, Rob Herring wrote: >>> On Wed, Aug 28, 2024 at 01:37:20PM -0700, Nikunj Kela wrote: >>>> Add interrupt specifier for extended SPI interrupts. >>> What's an "extended SPI"? Is this a GIC spec thing? If so, what version? >> Extended SPI is an extended range of SPI interrupts supported by GIC. >> >> Excerpt below from >> Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml >> >> "The 1st cell is the interrupt type; 0 for SPI interrupts, 1 for PPI >> interrupts, 2 for interrupts in the Extended SPI range, 3 for the >> Extended PPI range. Other values are reserved for future use." >> >> "The 2nd cell contains the interrupt number for the interrupt type. SPI >> interrupts are in the range [0-987]. PPI interrupts are in the range >> [0-15]. Extented SPI interrupts are in the range [0-1023]. Extended PPI >> interrupts are in the range [0-127]." > Looks like you should add EPPI define too while you're here. > > Rob Sure Rob. I can add that. Generally, there is an ask for a usecase before we push anything that is used in DT. I won't have any usecase to show for EPPI.
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, 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
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 >
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
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
> Just to clarify to I2C maintainers: > This is incomplete. Missing driver changes. Thanks, Krzysztof!
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/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 >>
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 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
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: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
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 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
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, should be this one: Sorry, didn't realize SPI uses different subject format than other subsystems. Will fix in v3. Thanks > 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 >
> 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
> 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
On 9/4/2024 10:05 AM, Andrew Lunn wrote: >> 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. Let me clarify here, the patches have already been through multiple rounds of review and since this is new architecture that we are using, we want to make sure this gets reviewed internally as much as possible. While, we will be posting them soon, they are available on public git repo for anyone to take a feel of the amount of changes. Let's not be judgemental here. > 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. No one is born with experience. You learn as you go. Please note that this series has gone through internal review before I posted it in upstream. > 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
> No one is born with experience. You learn as you go. Please note that > this series has gone through internal review before I posted it in > upstream. Then i'm surprise you were not told to submit lots of smaller patchsets, one per subsystem, which are complete. I get nobody is born with experience, but for a company the size of Qualcomm, they can easily hire a few experienced mainline developers who can mentor you, rather than having overloaded Maintainers teach you the basics, and getting frustrated in the process. 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 04/09/2024 23:06, Nikunj Kela wrote: > > 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. No, they are not dependent. They have never been. Look how all other upstreaming process worked in the past. Best regards, Krzysztof
On Tue, Sep 03, 2024 at 03:02:19PM GMT, 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]. Please don't send new revisions as a reply to the previous patchset. Always start new thread for new submission. This is documented in your internal 'upstreaming' documents. If it is not, please update them.
On 9/5/2024 1:04 AM, Krzysztof Kozlowski wrote: > On 04/09/2024 23:06, Nikunj Kela wrote: >> 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. > No, they are not dependent. They have never been. Look how all other > upstreaming process worked in the past. Top level QUP node(patch#18) includes i2c,spi,uart nodes. soc/qcom/qcom,geni-se.yaml validate those subnodes against respective yaml. The example that is added in YAML file for QUP node will not find sa8255p compatibles if all 4 yaml(qup, i2c, spi, serial nodes) are not included in the same series. > > Best regards, > Krzysztof >
On 9/5/2024 7:09 AM, Krzysztof Kozlowski wrote: > On 05/09/2024 16:03, Nikunj Kela wrote: >> On 9/5/2024 1:04 AM, Krzysztof Kozlowski wrote: >>> On 04/09/2024 23:06, Nikunj Kela wrote: >>>> 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. >>> No, they are not dependent. They have never been. Look how all other >>> upstreaming process worked in the past. >> Top level QUP node(patch#18) includes i2c,spi,uart nodes. >> soc/qcom/qcom,geni-se.yaml validate those subnodes against respective >> yaml. The example that is added in YAML file for QUP node will not find >> sa8255p compatibles if all 4 yaml(qup, i2c, spi, serial nodes) are not >> included in the same series. >> > So where is the dependency? I don't see it. Ok, what is your suggestion on dt-schema check failure in that case as I mentioned above? Shall we remove examples from yaml that we added? > Anyway, if you insist, > provide reasons why this should be the only one patchset - from all > SoCs, all companies, all developers - getting an exception from standard > merging practice and from explicit rule about driver change. See > submitting bindings. > > This was re-iterated over and over, but you keep claiming you need some > sort of special treatment. If so, please provide arguments WHY this > requires special treatment and *all* other contributions are fine with it. > > Best regards, > Krzysztof >
On 05/09/2024 16:15, Nikunj Kela wrote: > > On 9/5/2024 7:09 AM, Krzysztof Kozlowski wrote: >> On 05/09/2024 16:03, Nikunj Kela wrote: >>> On 9/5/2024 1:04 AM, Krzysztof Kozlowski wrote: >>>> On 04/09/2024 23:06, Nikunj Kela wrote: >>>>> 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. >>>> No, they are not dependent. They have never been. Look how all other >>>> upstreaming process worked in the past. >>> Top level QUP node(patch#18) includes i2c,spi,uart nodes. >>> soc/qcom/qcom,geni-se.yaml validate those subnodes against respective >>> yaml. The example that is added in YAML file for QUP node will not find >>> sa8255p compatibles if all 4 yaml(qup, i2c, spi, serial nodes) are not >>> included in the same series. >>> >> So where is the dependency? I don't see it. > > Ok, what is your suggestion on dt-schema check failure in that case as I > mentioned above? Shall we remove examples from yaml that we added? I don't understand what sort of failure you want to fix and why examples have any problem here. I said it multiple times already but I think you never confirmed. Do you understand how patches are merged? That they go via different trees but everything must be 100% bisectable? Best regards, Krzysztof
> Ok, what is your suggestion on dt-schema check failure in that case as I > mentioned above? Shall we remove examples from yaml that we added? As Krzysztof keeps saying, Commit message. You have an unlimited amount of space to document why this SoC is special, how it is special, maybe include some ASCII art showing how it is special. Justify it being special. Once it is clear it is special, has dependencies which are real, we are likely to accept the patches. We know SoC vendors do weird things, and sometimes mainline processes just don't work. But you need to clear, upfront, and state, the process does not work because... in your commit message. Maybe put it below the ---. Something i often say to Mainline newbies. The code is easy, it is the processes which are hard. The commit message is part of the process. You want to try to anticipate all the questions Reviewers are going to ask and answer them in the commit message, before they ask them. It is process that you split patches by subsystem. It is process that binding changes and driver changes go together in the patchset. Your 'code review' should include all this, not just the lines of actual code. And to begin with, process is probably a lot more important than the actual code. So please concentrate on processes, get them right. Andrew
On 9/5/2024 7:49 AM, Krzysztof Kozlowski wrote: > On 05/09/2024 16:15, Nikunj Kela wrote: >> On 9/5/2024 7:09 AM, Krzysztof Kozlowski wrote: >>> On 05/09/2024 16:03, Nikunj Kela wrote: >>>> On 9/5/2024 1:04 AM, Krzysztof Kozlowski wrote: >>>>> On 04/09/2024 23:06, Nikunj Kela wrote: >>>>>> 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. >>>>> No, they are not dependent. They have never been. Look how all other >>>>> upstreaming process worked in the past. >>>> Top level QUP node(patch#18) includes i2c,spi,uart nodes. >>>> soc/qcom/qcom,geni-se.yaml validate those subnodes against respective >>>> yaml. The example that is added in YAML file for QUP node will not find >>>> sa8255p compatibles if all 4 yaml(qup, i2c, spi, serial nodes) are not >>>> included in the same series. >>>> >>> So where is the dependency? I don't see it. >> Ok, what is your suggestion on dt-schema check failure in that case as I >> mentioned above? Shall we remove examples from yaml that we added? >> >> >>> Anyway, if you insist, >>> provide reasons why this should be the only one patchset - from all >>> SoCs, all companies, all developers - getting an exception from standard >>> merging practice and from explicit rule about driver change. See >>> submitting bindings. >>> >>> This was re-iterated over and over, but you keep claiming you need some >>> sort of special treatment. If so, please provide arguments WHY this >>> requires special treatment and *all* other contributions are fine with it. > You did not respond to above about explaining why this patchset needs > special treatment, so I assume there is no exception here to be granted > so any new version will follow standard process (see submitting bindings > / writing bindings). > > Best regards, > Krzysztof Things will be clear after you see the driver changes. Without looking at the code, this discussion won't lead to anything constructive. So I deferred the QUP related discussion until driver patches are posted. Thanks, -Nikunj >
On 9/5/2024 7:39 AM, Krzysztof Kozlowski wrote: > On 05/09/2024 16:15, Nikunj Kela wrote: >> On 9/5/2024 7:09 AM, Krzysztof Kozlowski wrote: >>> On 05/09/2024 16:03, Nikunj Kela wrote: >>>> On 9/5/2024 1:04 AM, Krzysztof Kozlowski wrote: >>>>> On 04/09/2024 23:06, Nikunj Kela wrote: >>>>>> 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. >>>>> No, they are not dependent. They have never been. Look how all other >>>>> upstreaming process worked in the past. >>>> Top level QUP node(patch#18) includes i2c,spi,uart nodes. >>>> soc/qcom/qcom,geni-se.yaml validate those subnodes against respective >>>> yaml. The example that is added in YAML file for QUP node will not find >>>> sa8255p compatibles if all 4 yaml(qup, i2c, spi, serial nodes) are not >>>> included in the same series. >>>> >>> So where is the dependency? I don't see it. >> Ok, what is your suggestion on dt-schema check failure in that case as I >> mentioned above? Shall we remove examples from yaml that we added? > I don't understand what sort of failure you want to fix and why examples > have any problem here. If the QUPs yaml changes are not included in the same series with i2c,serial yaml changes, you see these errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: serial@990000:compatible:0: 'qcom,sa8255p-geni-uart' is not one of ['qcom,geni-uart', 'qcom,geni-debug-uart'] /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: i2c@984000:compatible:0: 'qcom,sa8255p-geni-i2c' is not one of ['qcom,geni-i2c', 'qcom,geni-i2c-master-hub'] > I said it multiple times already but I think you > never confirmed. Do you understand how patches are merged? That they go > via different trees but everything must be 100% bisectable? > > Best regards, > Krzysztof >
> If the QUPs yaml changes are not included in the same series with > i2c,serial yaml changes, you see these errors: > > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: serial@990000:compatible:0: 'qcom,sa8255p-geni-uart' is not one of ['qcom,geni-uart', 'qcom,geni-debug-uart'] > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: i2c@984000:compatible:0: 'qcom,sa8255p-geni-i2c' is not one of ['qcom,geni-i2c', 'qcom,geni-i2c-master-hub'] So you have a couple of options: 1) It sounds like you should get the QUP changes merged first. Then submit the i2c,serial changes. Is there a reason you cannot do this? Is there a mutual dependency between these two series, or just a one way dependency? 2) Explain in the commit message that following errors are expected because ... And explain in detail why the dependency cannot be broken to avoid the errors. Andrew
On 9/5/2024 9:23 AM, Andrew Lunn wrote: >> If the QUPs yaml changes are not included in the same series with >> i2c,serial yaml changes, you see these errors: >> >> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: serial@990000:compatible:0: 'qcom,sa8255p-geni-uart' is not one of ['qcom,geni-uart', 'qcom,geni-debug-uart'] >> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: i2c@984000:compatible:0: 'qcom,sa8255p-geni-i2c' is not one of ['qcom,geni-i2c', 'qcom,geni-i2c-master-hub'] > So you have a couple of options: > > 1) It sounds like you should get the QUP changes merged first. Then > submit the i2c,serial changes. Is there a reason you cannot do > this? Is there a mutual dependency between these two series, or > just a one way dependency? The ask in this thread is to create new yaml files since existing one is using generic compatibles. With new yaml, we would need to provide example and can't avoid it. If we have to provide example of QUP node, IMO, we should provide a few subnodes as well since just QUP node without subnodes(i2c/serial/spi) will not be very useful. We can possibly skip all 3 subnode and only keep one subsystem(e.g. serial) so QUP and UART yaml can go together(still need two subsystems) while SPI and I2C can go independently after QUP series is accepted. Not sure if that is acceptable to maintainers though. QUP node in actual DT will have all 3 types of subnodes(i2c,spi, serial) so example in this case won't be complete. > > 2) Explain in the commit message that following errors are expected > because ... And explain in detail why the dependency cannot be > broken to avoid the errors. > > Andrew
On 05/09/2024 18:56, Krzysztof Kozlowski wrote: > On 05/09/2024 18:08, Nikunj Kela wrote: >> >> On 9/5/2024 7:39 AM, Krzysztof Kozlowski wrote: >>> On 05/09/2024 16:15, Nikunj Kela wrote: >>>> On 9/5/2024 7:09 AM, Krzysztof Kozlowski wrote: >>>>> On 05/09/2024 16:03, Nikunj Kela wrote: >>>>>> On 9/5/2024 1:04 AM, Krzysztof Kozlowski wrote: >>>>>>> On 04/09/2024 23:06, Nikunj Kela wrote: >>>>>>>> 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. >>>>>>> No, they are not dependent. They have never been. Look how all other >>>>>>> upstreaming process worked in the past. >>>>>> Top level QUP node(patch#18) includes i2c,spi,uart nodes. >>>>>> soc/qcom/qcom,geni-se.yaml validate those subnodes against respective >>>>>> yaml. The example that is added in YAML file for QUP node will not find >>>>>> sa8255p compatibles if all 4 yaml(qup, i2c, spi, serial nodes) are not >>>>>> included in the same series. >>>>>> >>>>> So where is the dependency? I don't see it. >>>> Ok, what is your suggestion on dt-schema check failure in that case as I >>>> mentioned above? Shall we remove examples from yaml that we added? >>> I don't understand what sort of failure you want to fix and why examples >>> have any problem here. >> >> If the QUPs yaml changes are not included in the same series with > > They cannot be included in the same series. You just think that > including here solves the problem so go ahead, simulate the merging: > 1. Bjorn applies soc/qcom/qcom,geni-se.yaml patch and tests. His tree > MUST build, so it also must pass dt_binding_check. > Does it pass? No. > > 2. SPI maintainer... ah, no point even going there. > >> i2c,serial yaml changes, you see these errors: >> >> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: serial@990000:compatible:0: 'qcom,sa8255p-geni-uart' is not one of ['qcom,geni-uart', 'qcom,geni-debug-uart'] >> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.example.dtb: geniqup@9c0000: i2c@984000:compatible:0: 'qcom,sa8255p-geni-i2c' is not one of ['qcom,geni-i2c', 'qcom,geni-i2c-master-hub'] > > Don't grow examples if not needed. Or create dependencies and ask > maintainers to cross-merge. Or soc/geni-se binding could be also converted to just list compatibles instead of referencing other schema, just like MDSS. Best regards, Krzysztof
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.
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