[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <beaa89d2-c8b1-4c3e-8e04-22caf753570a@oss.qualcomm.com>
Date: Thu, 30 Oct 2025 15:44:30 +0800
From: Jingyi Wang <jingyi.wang@....qualcomm.com>
To: Rob Herring <robh@...nel.org>
Cc: Bjorn Andersson <andersson@...nel.org>,
Mathieu Poirier <mathieu.poirier@...aro.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley
<conor+dt@...nel.org>,
Manivannan Sadhasivam <mani@...nel.org>, aiqun.yu@....qualcomm.com,
tingwei.zhang@....qualcomm.com, trilok.soni@....qualcomm.com,
yijie.yang@....qualcomm.com, linux-arm-msm@...r.kernel.org,
linux-remoteproc@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org,
Sibi Sankar <sibi.sankar@....qualcomm.com>
Subject: Re: [PATCH v2 3/7] dt-bindings: remoteproc: qcom,pas: Document pas
for SoCCP on Kaanapali and Glymur platforms
On 10/29/2025 9:22 PM, Rob Herring wrote:
> On Wed, Oct 29, 2025 at 01:05:41AM -0700, Jingyi Wang wrote:
>> Document the component used to boot SoCCP on Kaanapali SoC and add
>> compatible for Glymur SoCCP which could fallback to Kaanapali. Extend
>> the "qcom,smem-states" and "qcom,smem-state-names" properties and
>> add conditions for the "interrupts" and "interrupt-names" properties
>> in the pas-common.
>>
>> Co-developed-by: Sibi Sankar <sibi.sankar@....qualcomm.com>
>> Signed-off-by: Sibi Sankar <sibi.sankar@....qualcomm.com>
>> Signed-off-by: Jingyi Wang <jingyi.wang@....qualcomm.com>
>> ---
>> .../remoteproc/qcom,kaanapali-soccp-pas.yaml | 134 +++++++++++++++++++++
>> .../bindings/remoteproc/qcom,pas-common.yaml | 83 +++++++++----
>> 2 files changed, 194 insertions(+), 23 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,kaanapali-soccp-pas.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,kaanapali-soccp-pas.yaml
>> new file mode 100644
>> index 000000000000..6b53121eede1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,kaanapali-soccp-pas.yaml
>> @@ -0,0 +1,134 @@
>> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/remoteproc/qcom,kaanapali-soccp-pas.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm Kaanapali SoCCP Peripheral Authentication Service
>> +
>> +maintainers:
>> + - Jingyi Wang <jingyi.wang@....qualcomm.com>
>> +
>> +description:
>> + The SoC Control Processor (SoCCP) is small RISC-V MCU that controls USB
>> + Type-C, battery charging and various other functions on Qualcomm SoCs, somewhat
>> + analogous to traditional PC Embedded Controllers. This document describes
>> + the Peripheral Authentication Service loads and boots firmware for SoCCP.
>> +
>> +properties:
>> + compatible:
>> + oneOf:
>> + - items:
>> + - enum:
>> + - qcom,glymur-soccp-pas
>> + - const: qcom,kaanapali-soccp-pas
>> + - enum:
>> + - qcom,kaanapali-soccp-pas
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + clocks:
>> + items:
>> + - description: XO clock
>> +
>> + clock-names:
>> + items:
>> + - const: xo
>> +
>> + power-domains:
>> + items:
>> + - description: CX power domain
>> + - description: MX power domain
>> +
>> + power-domain-names:
>> + items:
>> + - const: cx
>> + - const: mx
>> +
>> + firmware-name:
>> + $ref: /schemas/types.yaml#/definitions/string-array
>
> Already has a type. Drop.
>
well noted
>> + items:
>> + - description: Firmware name of the Hexagon core
>> + - description: Firmware name of the Hexagon Devicetree
>> +
>> + memory-region:
>> + items:
>> + - description: Memory region for main Firmware authentication
>> + - description: Memory region for Devicetree Firmware authentication
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - memory-region
>> + - power-domains
>> + - power-domain-names
>> +
>> +allOf:
>> + - $ref: /schemas/remoteproc/qcom,pas-common.yaml#
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/clock/qcom,rpmh.h>
>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>> + #include <dt-bindings/interrupt-controller/irq.h>
>> + #include <dt-bindings/mailbox/qcom-ipcc.h>
>> + #include <dt-bindings/power/qcom-rpmpd.h>
>> +
>> + remoteproc@...000 {
>> + compatible = "qcom,kaanapali-soccp-pas";
>> + reg = <0x00d00000 0x200000>;
>> +
>> + clocks = <&rpmhcc RPMH_CXO_CLK>;
>> + clock-names = "xo";
>> +
>> + interrupts-extended = <&intc GIC_SPI 167 IRQ_TYPE_EDGE_RISING>,
>> + <&soccp_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,
>> + <&soccp_smp2p_in 1 IRQ_TYPE_EDGE_RISING>,
>> + <&soccp_smp2p_in 2 IRQ_TYPE_EDGE_RISING>,
>> + <&soccp_smp2p_in 3 IRQ_TYPE_EDGE_RISING>,
>> + <&soccp_smp2p_in 9 IRQ_TYPE_EDGE_RISING>,
>> + <&soccp_smp2p_in 10 IRQ_TYPE_EDGE_RISING>;
>> + interrupt-names = "wdog",
>> + "fatal",
>> + "ready",
>> + "handover",
>> + "stop-ack",
>> + "pong",
>> + "wake-ack";
>> +
>> + memory-region = <&soccp_mem>,
>> + <&soccp_dtb_mem_mem>;
>> +
>> + firmware-name = "qcom/kaanapali/soccp.mbn",
>> + "qcom/kaanapali/soccp_dtb.mbn";
>> +
>> + power-domains = <&rpmhpd RPMHPD_CX>,
>> + <&rpmhpd RPMHPD_MX>;
>> + power-domain-names = "cx",
>> + "mx";
>> +
>> + qcom,smem-states = <&soccp_smp2p_out 0>,
>> + <&soccp_smp2p_out 10>,
>> + <&soccp_smp2p_out 9>,
>> + <&soccp_smp2p_out 8>;
>> + qcom,smem-state-names = "stop",
>> + "wakeup",
>> + "sleep",
>> + "ping";
>> +
>> + glink-edge {
>> + interrupts-extended = <&ipcc IPCC_MPROC_SOCCP
>> + IPCC_MPROC_SIGNAL_GLINK_QMP
>> + IRQ_TYPE_EDGE_RISING>;
>> + mboxes = <&ipcc IPCC_MPROC_SOCCP
>> + IPCC_MPROC_SIGNAL_GLINK_QMP>;
>> +
>> + label = "soccp";
>> + qcom,remote-pid = <19>;
>> +
>> + /* ... */
>> + };
>> + };
>> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
>> index 63a82e7a8bf8..f81d088c2bad 100644
>> --- a/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
>> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,pas-common.yaml
>> @@ -24,26 +24,6 @@ properties:
>> interconnects:
>> maxItems: 1
>>
>> - interrupts:
>> - minItems: 5
>> - items:
>> - - description: Watchdog interrupt
>> - - description: Fatal interrupt
>> - - description: Ready interrupt
>> - - description: Handover interrupt
>> - - description: Stop acknowledge interrupt
>> - - description: Shutdown acknowledge interrupt
>> -
>> - interrupt-names:
>> - minItems: 5
>> - items:
>> - - const: wdog
>> - - const: fatal
>> - - const: ready
>> - - const: handover
>> - - const: stop-ack
>> - - const: shutdown-ack
>> -
>> power-domains:
>> minItems: 1
>> maxItems: 3
>> @@ -55,13 +35,21 @@ properties:
>> qcom,smem-states:
>> $ref: /schemas/types.yaml#/definitions/phandle-array
>> description: States used by the AP to signal the Hexagon core
>> + minItems: 1
>> items:
>> - - description: Stop the modem
>> + - description: Stop the remoteproc
>> + - description: Wake up the remoteproc
>> + - description: Make the remoteproc sleep
>> + - description: Ping the remoteproc
>>
>> qcom,smem-state-names:
>> description: The names of the state bits used for SMP2P output
>> + minItems: 1
>> items:
>> - const: stop
>> + - const: wakeup
>> + - const: sleep
>> + - const: ping
>>
>> smd-edge:
>> $ref: /schemas/remoteproc/qcom,smd-edge.yaml#
>> @@ -80,9 +68,58 @@ properties:
>> required:
>> - clocks
>> - clock-names
>> - - interrupts
>> - - interrupt-names
>> - qcom,smem-states
>> - qcom,smem-state-names
>>
>> +allOf:
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + enum:
>> + - qcom,kaanapali-soccp-pas
>
> The point of common schemas is to not have if/then/else schemas. If
> interrupts is now variable, then it is no longer common and should be
> moved out of the common schema. Or just have the widest constraints that
> covers all cases ({minItems: 5, maxItems: 7}).
>
>
Thanks for clarify, if we use the comman schema, we need to change the
constraint from "const" to "enum" like:
interrupts:
minItems: 5
items:
- description: Watchdog interrupt
- description: Fatal interrupt
- description: Ready interrupt
- description: Handover interrupt
- description: Stop acknowledge interrupt
- description: Shutdown acknowledge interrupt
- description: Pong interrupt
- description: Wake acknowledge interrupt
(define "maxItems: 7" will cause err:
hint: "maxItems" is not needed with an "items" list)
interrupt-names:
minItems: 5
maxItems: 7
items:
enum:
- wdog
- fatal
- ready
- handover
- stop-ack
- shutdown-ack
- pong
- wake-ack
is this okay?
Thanks,
Jingyi
>> + then:
>> + properties:
>> + interrupts:
>> + items:
>> + - description: Watchdog interrupt
>> + - description: Fatal interrupt
>> + - description: Ready interrupt
>> + - description: Handover interrupt
>> + - description: Stop acknowledge interrupt
>> + - description: Pong interrupt
>> + - description: Wake acknowledge interrupt
>> +
>> + interrupt-names:
>> + items:
>> + - const: wdog
>> + - const: fatal
>> + - const: ready
>> + - const: handover
>> + - const: stop-ack
>> + - const: pong
>> + - const: wake-ack
>> +
>> + else:
>> + properties:
>> + interrupts:
>> + minItems: 5
>> + items:
>> + - description: Watchdog interrupt
>> + - description: Fatal interrupt
>> + - description: Ready interrupt
>> + - description: Handover interrupt
>> + - description: Stop acknowledge interrupt
>> + - description: Shutdown acknowledge interrupt
>> +
>> + interrupt-names:
>> + minItems: 5
>> + items:
>> + - const: wdog
>> + - const: fatal
>> + - const: ready
>> + - const: handover
>> + - const: stop-ack
>> + - const: shutdown-ack
>> +
>> additionalProperties: true
>>
>> --
>> 2.25.1
>>
Powered by blists - more mailing lists