2021-03-17 05:32:54

by Maulik Shah

[permalink] [raw]
Subject: [PATCH 1/3] arm64: dts: qcom: sm8350: Remove second reg from pdc

PDC interrupt controller driver do not use second reg. Remove it.
This is in preparation to convert PDC bindings to yaml where
dtbs_check reports it as additional reg.

Cc: [email protected]
Signed-off-by: Maulik Shah <[email protected]>
---
arch/arm64/boot/dts/qcom/sm8350.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8350.dtsi b/arch/arm64/boot/dts/qcom/sm8350.dtsi
index ea28514..b48bc1b 100644
--- a/arch/arm64/boot/dts/qcom/sm8350.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8350.dtsi
@@ -612,7 +612,7 @@

pdc: [email protected] {
compatible = "qcom,sm8350-pdc", "qcom,pdc";
- reg = <0 0x0b220000 0 0x30000>, <0 0x17c000f0 0 0x60>;
+ reg = <0 0x0b220000 0 0x30000>;
qcom,pdc-ranges = <0 480 40>, <40 140 14>, <54 263 1>, <55 306 4>,
<59 312 3>, <62 374 2>, <64 434 2>, <66 438 3>,
<69 86 1>, <70 520 54>, <124 609 31>, <155 63 1>,
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


2021-03-17 05:32:54

by Maulik Shah

[permalink] [raw]
Subject: [PATCH 3/3] dt-bindings: interrupt-controller: Convert bindings to yaml for qcom,pdc

This change converts PDC interrupt controller bindings to yaml.

Cc: [email protected]
Signed-off-by: Maulik Shah <[email protected]>
---
This change depends on [1] which adds sc7280 compatible for PDC

[1] https://patchwork.kernel.org/project/linux-arm-msm/list/?series=440315
---
.../bindings/interrupt-controller/qcom,pdc.txt | 76 ------------------
.../bindings/interrupt-controller/qcom,pdc.yaml | 93 ++++++++++++++++++++++
2 files changed, 93 insertions(+), 76 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.yaml

diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
deleted file mode 100644
index 98d89e5..0000000
--- a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
+++ /dev/null
@@ -1,76 +0,0 @@
-PDC interrupt controller
-
-Qualcomm Technologies Inc. SoCs based on the RPM Hardened architecture have a
-Power Domain Controller (PDC) that is on always-on domain. In addition to
-providing power control for the power domains, the hardware also has an
-interrupt controller that can be used to help detect edge low interrupts as
-well detect interrupts when the GIC is non-operational.
-
-GIC is parent interrupt controller at the highest level. Platform interrupt
-controller PDC is next in hierarchy, followed by others. Drivers requiring
-wakeup capabilities of their device interrupts routed through the PDC, must
-specify PDC as their interrupt controller and request the PDC port associated
-with the GIC interrupt. See example below.
-
-Properties:
-
-- compatible:
- Usage: required
- Value type: <string>
- Definition: Should contain "qcom,<soc>-pdc" and "qcom,pdc"
- - "qcom,sc7180-pdc": For SC7180
- - "qcom,sc7280-pdc": For SC7280
- - "qcom,sdm845-pdc": For SDM845
- - "qcom,sdm8250-pdc": For SM8250
- - "qcom,sdm8350-pdc": For SM8350
-
-- reg:
- Usage: required
- Value type: <prop-encoded-array>
- Definition: Specifies the base physical address for PDC hardware.
-
-- interrupt-cells:
- Usage: required
- Value type: <u32>
- Definition: Specifies the number of cells needed to encode an interrupt
- source.
- Must be 2.
- The first element of the tuple is the PDC pin for the
- interrupt.
- The second element is the trigger type.
-
-- interrupt-controller:
- Usage: required
- Value type: <bool>
- Definition: Identifies the node as an interrupt controller.
-
-- qcom,pdc-ranges:
- Usage: required
- Value type: <u32 array>
- Definition: Specifies the PDC pin offset and the number of PDC ports.
- The tuples indicates the valid mapping of valid PDC ports
- and their hwirq mapping.
- The first element of the tuple is the starting PDC port.
- The second element is the GIC hwirq number for the PDC port.
- The third element is the number of interrupts in sequence.
-
-Example:
-
- pdc: [email protected] {
- compatible = "qcom,sdm845-pdc";
- reg = <0xb220000 0x30000>;
- qcom,pdc-ranges = <0 512 94>, <94 641 15>, <115 662 7>;
- #interrupt-cells = <2>;
- interrupt-parent = <&intc>;
- interrupt-controller;
- };
-
-DT binding of a device that wants to use the GIC SPI 514 as a wakeup
-interrupt, must do -
-
- wake-device {
- interrupts-extended = <&pdc 2 IRQ_TYPE_LEVEL_HIGH>;
- };
-
-In this case interrupt 514 would be mapped to port 2 on the PDC as defined by
-the qcom,pdc-ranges property.
diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.yaml b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.yaml
new file mode 100644
index 0000000..26ae77c
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.yaml
@@ -0,0 +1,93 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/qcom,pdc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Technologies, Inc. PDC interrupt controller
+
+maintainers:
+ - Maulik Shah <[email protected]>
+
+description: |
+ Qualcomm Technologies, Inc. SoCs based on the RPM Hardened architecture have a
+ Power Domain Controller (PDC) that is on always-on domain. In addition to
+ providing power control for the power domains, the hardware also has an
+ interrupt controller that can be used to help detect edge low interrupts as
+ well detect interrupts when the GIC is non-operational.
+
+ GIC is parent interrupt controller at the highest level. Platform interrupt
+ controller PDC is next in hierarchy, followed by others. Drivers requiring
+ wakeup capabilities of their device interrupts routed through the PDC, must
+ specify PDC as their interrupt controller and request the PDC port associated
+ with the GIC interrupt. See example below.
+
+properties:
+ compatible:
+ items:
+ - enum:
+ # Should contain "qcom,<soc>-pdc" and "qcom,pdc"
+ - qcom,sc7180-pdc #For SC7180
+ - qcom,sc7280-pdc #For SC7280
+ - qcom,sdm845-pdc #For SDM845
+ - qcom,sm8250-pdc #For SM8250
+ - qcom,sm8350-pdc #For SM8350
+ - const: qcom,pdc
+
+ reg:
+ minItems: 1
+ items:
+ - description: PDC register base address
+
+ '#interrupt-cells':
+ # Specifies the number of cells needed to encode an interrupt.
+ # The first element of the tuple is the PDC pin for the interrupt.
+ # The second element is the trigger type.
+ const: 2
+
+ interrupt-controller: true
+
+ qcom,pdc-ranges:
+ description: |
+ Specifies the PDC pin offset and the number of PDC ports.
+ The tuples indicates the valid mapping of valid PDC ports
+ and their hwirq mapping.
+ $ref: /schemas/types.yaml#/definitions/uint32-matrix
+ items:
+ items:
+ - description: |
+ "a" The first element of the tuple is the starting PDC port.
+ - description: |
+ "b" The second element is the GIC SPI number for the PDC port.
+ - description: |
+ "c" The third element is the number of interrupts in sequence.
+
+required:
+ - compatible
+ - reg
+ - '#interrupt-cells'
+ - interrupt-controller
+ - qcom,pdc-ranges
+
+additionalProperties: false
+
+examples:
+ - |
+ pdc: [email protected] {
+ compatible = "qcom,sdm845-pdc", "qcom,pdc";
+ reg = <0xb220000 0x30000>;
+ qcom,pdc-ranges = <0 512 94>, <94 641 15>, <115 662 7>;
+ #interrupt-cells = <2>;
+ interrupt-parent = <&intc>;
+ interrupt-controller;
+ };
+
+ # DT binding of a device that wants to use the GIC SPI 514 as a wakeup
+ # interrupt, must do -
+ # wake-device {
+ # interrupts-extended = <&pdc 2 IRQ_TYPE_LEVEL_HIGH>;
+ # };
+
+ # In this case interrupt 514 would be mapped to port 2 on the PDC as defined
+ # by the qcom,pdc-ranges property.
+...
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

2021-03-17 09:19:00

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH 1/3] arm64: dts: qcom: sm8350: Remove second reg from pdc

On Wed, 17 Mar 2021 05:29:54 +0000,
Maulik Shah <[email protected]> wrote:
>
> PDC interrupt controller driver do not use second reg. Remove it.

This is a DT file, not a driver. What the driver does is irrelevant.

The real question is: what does this range do?

Thanks,

M.

--
Without deviation from the norm, progress is not possible.

2021-03-17 09:50:37

by Maulik Shah

[permalink] [raw]
Subject: Re: [PATCH 1/3] arm64: dts: qcom: sm8350: Remove second reg from pdc

Hi Marc,

On 3/17/2021 2:47 PM, Marc Zyngier wrote:
> On Wed, 17 Mar 2021 05:29:54 +0000,
> Maulik Shah <[email protected]> wrote:
>> PDC interrupt controller driver do not use second reg. Remove it.
> This is a DT file, not a driver. What the driver does is irrelevant.
>
> The real question is: what does this range do?
>
> Thanks,
>
> M.

This is to set interrupt type in SPI config for which there was a change
[1] but has not gone in for upstream PDC driver.

The second reg is not used in upstream PDC driver, probably when posting
downstream DT changes for sm8350/sm8250 it was carried in device node as is.

As its not mentioned in bindigs as well, dtbs_check reports it as
additional reg when converted to yaml.

[1]
https://patchwork.kernel.org/project/linux-arm-msm/patch/[email protected]/

Thanks,
Maulik
>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

2021-03-17 17:14:46

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH 1/3] arm64: dts: qcom: sm8350: Remove second reg from pdc

On Wed, 17 Mar 2021 09:48:09 +0000,
Maulik Shah <[email protected]> wrote:
>
> Hi Marc,
>
> On 3/17/2021 2:47 PM, Marc Zyngier wrote:
> > On Wed, 17 Mar 2021 05:29:54 +0000,
> > Maulik Shah <[email protected]> wrote:
> >> PDC interrupt controller driver do not use second reg. Remove it.
> > This is a DT file, not a driver. What the driver does is irrelevant.
> >
> > The real question is: what does this range do?
> >
> > Thanks,
> >
> > M.
>
> This is to set interrupt type in SPI config for which there was a
> change [1] but has not gone in for upstream PDC driver.
>
> The second reg is not used in upstream PDC driver, probably when
> posting downstream DT changes for sm8350/sm8250 it was carried in
> device node as is.
>
> As its not mentioned in bindigs as well, dtbs_check reports it as
> additional reg when converted to yaml.

Then I'd rather you provide accurate documentation in the binding
rather than changing the DT files. Other operating systems may use it,
and it isn't unlikely that Linux could use the feature at some point.

Thanks,

M.

--
Without deviation from the norm, progress is not possible.

2021-03-17 18:11:46

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH 1/3] arm64: dts: qcom: sm8350: Remove second reg from pdc

On Wed 17 Mar 09:02 CDT 2021, Marc Zyngier wrote:

> On Wed, 17 Mar 2021 09:48:09 +0000,
> Maulik Shah <[email protected]> wrote:
> >
> > Hi Marc,
> >
> > On 3/17/2021 2:47 PM, Marc Zyngier wrote:
> > > On Wed, 17 Mar 2021 05:29:54 +0000,
> > > Maulik Shah <[email protected]> wrote:
> > >> PDC interrupt controller driver do not use second reg. Remove it.
> > > This is a DT file, not a driver. What the driver does is irrelevant.
> > >
> > > The real question is: what does this range do?
> > >
> > > Thanks,
> > >
> > > M.
> >
> > This is to set interrupt type in SPI config for which there was a
> > change [1] but has not gone in for upstream PDC driver.
> >
> > The second reg is not used in upstream PDC driver, probably when
> > posting downstream DT changes for sm8350/sm8250 it was carried in
> > device node as is.
> >
> > As its not mentioned in bindigs as well, dtbs_check reports it as
> > additional reg when converted to yaml.
>
> Then I'd rather you provide accurate documentation in the binding
> rather than changing the DT files. Other operating systems may use it,
> and it isn't unlikely that Linux could use the feature at some point.
>

I agree. Maulik, please update the DT binding to document this region as
well.


It also seems relevant to pursue getting [1] into the upstream Linux
kernel. Is this something that you use downstream Maulik?

Regards,
Bjorn

2021-03-22 10:30:42

by Maulik Shah

[permalink] [raw]
Subject: Re: [PATCH 1/3] arm64: dts: qcom: sm8350: Remove second reg from pdc

Hi,

On 3/17/2021 11:38 PM, Bjorn Andersson wrote:
> On Wed 17 Mar 09:02 CDT 2021, Marc Zyngier wrote:
>
>> On Wed, 17 Mar 2021 09:48:09 +0000,
>> Maulik Shah <[email protected]> wrote:
>>> Hi Marc,
>>>
>>> On 3/17/2021 2:47 PM, Marc Zyngier wrote:
>>>> On Wed, 17 Mar 2021 05:29:54 +0000,
>>>> Maulik Shah <[email protected]> wrote:
>>>>> PDC interrupt controller driver do not use second reg. Remove it.
>>>> This is a DT file, not a driver. What the driver does is irrelevant.
>>>>
>>>> The real question is: what does this range do?
>>>>
>>>> Thanks,
>>>>
>>>> M.
>>> This is to set interrupt type in SPI config for which there was a
>>> change [1] but has not gone in for upstream PDC driver.
>>>
>>> The second reg is not used in upstream PDC driver, probably when
>>> posting downstream DT changes for sm8350/sm8250 it was carried in
>>> device node as is.
>>>
>>> As its not mentioned in bindigs as well, dtbs_check reports it as
>>> additional reg when converted to yaml.
>> Then I'd rather you provide accurate documentation in the binding
>> rather than changing the DT files. Other operating systems may use it,
>> and it isn't unlikely that Linux could use the feature at some point.
>>
> I agree. Maulik, please update the DT binding to document this region as
> well.
sure. updated in v2.
>
>
> It also seems relevant to pursue getting [1] into the upstream Linux
> kernel. Is this something that you use downstream Maulik?

Yes its used in downstream. We can pursue to get [1] in.

Thanks,
Maulik

>
> Regards,
> Bjorn

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation