[PATCH RFC 04/14] of: irq: document properties for wakeup interrupt parent

[PATCH RFC 04/14] of: irq: document properties for wakeup interrupt parent

[Lina Iyer]

Some interrupt controllers in a SoC, are always powered on and have a
select interrupts routed to them, so that they can wakeup the SoC from
suspend. Add wakeup-parent DT property to refer to these interrupt
controllers.

Cc: devicetree@vger.kernel.org
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
---
 .../bindings/interrupt-controller/interrupts.txt    | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt b/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
index 8a3c40829899..c10e31050dd2 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
@@ -108,3 +108,16 @@ commonly used:
 			sensitivity = <7>;
 		};
 	};
+
+3) Interrupt wakeup parent
+--------------------------
+
+Some interrupt controllers in a SoC, are always powered on and have a select
+interrupts routed to them, so that they can wakeup the SoC from suspend. These
+interrupt controllers do not fall into the category of a parent interrupt
+controller and can be specified by the "wakeup-parent" property and contain a
+single phandle referring to the wakeup capable interrupt controller.
+
+   Example:
+	wakeup-parent = <&pdc_intc>;
+
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH RFC 04/14] of: irq: document properties for wakeup interrupt parent

[Rob Herring]

On Thu, 29 Aug 2019 12:11:53 -0600, Lina Iyer wrote:
> Some interrupt controllers in a SoC, are always powered on and have a
> select interrupts routed to them, so that they can wakeup the SoC from
> suspend. Add wakeup-parent DT property to refer to these interrupt
> controllers.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> ---
>  .../bindings/interrupt-controller/interrupts.txt    | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH RFC 05/14] dt-bindings/interrupt-controller: pdc: add SPI config register

[Lina Iyer]

In addition to configuring the PDC, additional registers that interface
the GIC have to be configured to match the GPIO type. The registers on
some QCOM SoCs are access restricted, while on other SoCs are not. They
SoCs with access restriction to these SPI registers need to be written
from the firmware using the SCM interface. Add a flag to indicate if the
register is to be written using SCM interface.

Cc: devicetree@vger.kernel.org
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
---
 .../bindings/interrupt-controller/qcom,pdc.txt           | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
index 8e0797cb1487..852fcba98ea6 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
@@ -50,15 +50,22 @@ Properties:
 		    The second element is the GIC hwirq number for the PDC port.
 		    The third element is the number of interrupts in sequence.
 
+- qcom,scm-spi-cfg:
+	Usage: optional
+	Value type: <bool>
+	Definition: Specifies if the SPI configuration registers have to be
+		    written from the firmware.
+
 Example:
 
 	pdc: interrupt-controller@b220000 {
 		compatible = "qcom,sdm845-pdc";
-		reg = <0xb220000 0x30000>;
+		reg = <0xb220000 0x30000>, <0x179900f0 0x60>;
 		qcom,pdc-ranges = <0 512 94>, <94 641 15>, <115 662 7>;
 		#interrupt-cells = <2>;
 		interrupt-parent = <&intc>;
 		interrupt-controller;
+		qcom,scm-spi-cfg;
 	};
 
 DT binding of a device that wants to use the GIC SPI 514 as a wakeup
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH RFC 05/14] dt-bindings/interrupt-controller: pdc: add SPI config register

[Rob Herring]

On Thu, Aug 29, 2019 at 12:11:54PM -0600, Lina Iyer wrote:
> In addition to configuring the PDC, additional registers that interface
> the GIC have to be configured to match the GPIO type. The registers on
> some QCOM SoCs are access restricted, while on other SoCs are not. They
> SoCs with access restriction to these SPI registers need to be written

Took me a minute to figure out this is GIC SPI interrupts, not SPI bus.

> from the firmware using the SCM interface. Add a flag to indicate if the
> register is to be written using SCM interface.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> ---
>  .../bindings/interrupt-controller/qcom,pdc.txt           | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
> index 8e0797cb1487..852fcba98ea6 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
> +++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
> @@ -50,15 +50,22 @@ Properties:
>  		    The second element is the GIC hwirq number for the PDC port.
>  		    The third element is the number of interrupts in sequence.
>  
> +- qcom,scm-spi-cfg:
> +	Usage: optional
> +	Value type: <bool>
> +	Definition: Specifies if the SPI configuration registers have to be
> +		    written from the firmware.
> +
>  Example:
>  
>  	pdc: interrupt-controller@b220000 {
>  		compatible = "qcom,sdm845-pdc";
> -		reg = <0xb220000 0x30000>;
> +		reg = <0xb220000 0x30000>, <0x179900f0 0x60>;

There needs to be a description for reg updated. These aren't GIC 
registers are they? Because those go in the GIC node.

>  		qcom,pdc-ranges = <0 512 94>, <94 641 15>, <115 662 7>;
>  		#interrupt-cells = <2>;
>  		interrupt-parent = <&intc>;
>  		interrupt-controller;
> +		qcom,scm-spi-cfg;
>  	};
>  
>  DT binding of a device that wants to use the GIC SPI 514 as a wakeup
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

Re: [PATCH RFC 05/14] dt-bindings/interrupt-controller: pdc: add SPI config register

[Marc Zyngier]

On 02/09/2019 14:38, Rob Herring wrote:
> On Thu, Aug 29, 2019 at 12:11:54PM -0600, Lina Iyer wrote:
>> In addition to configuring the PDC, additional registers that interface
>> the GIC have to be configured to match the GPIO type. The registers on
>> some QCOM SoCs are access restricted, while on other SoCs are not. They
>> SoCs with access restriction to these SPI registers need to be written
> 
> Took me a minute to figure out this is GIC SPI interrupts, not SPI bus.
> 
>> from the firmware using the SCM interface. Add a flag to indicate if the
>> register is to be written using SCM interface.
>>
>> Cc: devicetree@vger.kernel.org
>> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
>> ---
>>  .../bindings/interrupt-controller/qcom,pdc.txt           | 9 ++++++++-
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
>> index 8e0797cb1487..852fcba98ea6 100644
>> --- a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
>> +++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
>> @@ -50,15 +50,22 @@ Properties:
>>  		    The second element is the GIC hwirq number for the PDC port.
>>  		    The third element is the number of interrupts in sequence.
>>  
>> +- qcom,scm-spi-cfg:
>> +	Usage: optional
>> +	Value type: <bool>
>> +	Definition: Specifies if the SPI configuration registers have to be
>> +		    written from the firmware.
>> +
>>  Example:
>>  
>>  	pdc: interrupt-controller@b220000 {
>>  		compatible = "qcom,sdm845-pdc";
>> -		reg = <0xb220000 0x30000>;
>> +		reg = <0xb220000 0x30000>, <0x179900f0 0x60>;
> 
> There needs to be a description for reg updated. These aren't GIC 
> registers are they? Because those go in the GIC node.

This is completely insane. Why are the GIC registers configured as
secure the first place, if they are expected to be in control of the
non-secure?

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH RFC 05/14] dt-bindings/interrupt-controller: pdc: add SPI config register

[Lina Iyer]

On Mon, Sep 02 2019 at 07:58 -0600, Marc Zyngier wrote:
>On 02/09/2019 14:38, Rob Herring wrote:
>> On Thu, Aug 29, 2019 at 12:11:54PM -0600, Lina Iyer wrote:
>>> In addition to configuring the PDC, additional registers that interface
>>> the GIC have to be configured to match the GPIO type. The registers on
>>> some QCOM SoCs are access restricted, while on other SoCs are not. They
>>> SoCs with access restriction to these SPI registers need to be written
>>
>> Took me a minute to figure out this is GIC SPI interrupts, not SPI bus.
>>
>>> from the firmware using the SCM interface. Add a flag to indicate if the
>>> register is to be written using SCM interface.
>>>
>>> Cc: devicetree@vger.kernel.org
>>> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
>>> ---
>>>  .../bindings/interrupt-controller/qcom,pdc.txt           | 9 ++++++++-
>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
>>> index 8e0797cb1487..852fcba98ea6 100644
>>> --- a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
>>> +++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
>>> @@ -50,15 +50,22 @@ Properties:
>>>  		    The second element is the GIC hwirq number for the PDC port.
>>>  		    The third element is the number of interrupts in sequence.
>>>
>>> +- qcom,scm-spi-cfg:
>>> +	Usage: optional
>>> +	Value type: <bool>
>>> +	Definition: Specifies if the SPI configuration registers have to be
>>> +		    written from the firmware.
>>> +
>>>  Example:
>>>
>>>  	pdc: interrupt-controller@b220000 {
>>>  		compatible = "qcom,sdm845-pdc";
>>> -		reg = <0xb220000 0x30000>;
>>> +		reg = <0xb220000 0x30000>, <0x179900f0 0x60>;
>>
>> There needs to be a description for reg updated. These aren't GIC
>> registers are they? Because those go in the GIC node.
>
They are not GIC registers. I will update this documentation.

>This is completely insane. Why are the GIC registers configured as
>secure the first place, if they are expected to be in control of the
>non-secure?
These are not GIC registers but located on the PDC interface to the GIC.
They may or may not be secure access controlled, depending on the SoC.

Thanks,
Lina

Re: [PATCH RFC 05/14] dt-bindings/interrupt-controller: pdc: add SPI config register

[Stephen Boyd]

Quoting Lina Iyer (2019-09-03 10:07:22)
> On Mon, Sep 02 2019 at 07:58 -0600, Marc Zyngier wrote:
> >On 02/09/2019 14:38, Rob Herring wrote:
> >> On Thu, Aug 29, 2019 at 12:11:54PM -0600, Lina Iyer wrote:
> >>> In addition to configuring the PDC, additional registers that interface
> >>> the GIC have to be configured to match the GPIO type. The registers on
> >>> some QCOM SoCs are access restricted, while on other SoCs are not. They
> >>> SoCs with access restriction to these SPI registers need to be written
> >>
> >> Took me a minute to figure out this is GIC SPI interrupts, not SPI bus.
> >>
> >>> from the firmware using the SCM interface. Add a flag to indicate if the
> >>> register is to be written using SCM interface.
> >>>
> >>> Cc: devicetree@vger.kernel.org
> >>> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> >>> ---
> >>>  .../bindings/interrupt-controller/qcom,pdc.txt           | 9 ++++++++-
> >>>  1 file changed, 8 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
> >>> index 8e0797cb1487..852fcba98ea6 100644
> >>> --- a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
> >>> +++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
> >>> @@ -50,15 +50,22 @@ Properties:
> >>>                 The second element is the GIC hwirq number for the PDC port.
> >>>                 The third element is the number of interrupts in sequence.
> >>>
> >>> +- qcom,scm-spi-cfg:
> >>> +   Usage: optional
> >>> +   Value type: <bool>
> >>> +   Definition: Specifies if the SPI configuration registers have to be
> >>> +               written from the firmware.
> >>> +
> >>>  Example:
> >>>
> >>>     pdc: interrupt-controller@b220000 {
> >>>             compatible = "qcom,sdm845-pdc";
> >>> -           reg = <0xb220000 0x30000>;
> >>> +           reg = <0xb220000 0x30000>, <0x179900f0 0x60>;
> >>
> >> There needs to be a description for reg updated. These aren't GIC
> >> registers are they? Because those go in the GIC node.
> >
> They are not GIC registers. I will update this documentation.
> 
> >This is completely insane. Why are the GIC registers configured as
> >secure the first place, if they are expected to be in control of the
> >non-secure?
> These are not GIC registers but located on the PDC interface to the GIC.
> They may or may not be secure access controlled, depending on the SoC.
> 

It looks like it falls under this "mailbox" device which is really the
catch all bucket for bits with no home besides they're related to the
apps CPUs/subsystem.

	apss_shared: mailbox@17990000 {
		compatible = "qcom,sdm845-apss-shared";
		reg = <0 0x17990000 0 0x1000>;
		#mbox-cells = <1>;
	};

Can you point to this node with a phandle and then parse the reg
property out of it to use in the scm readl/writel APIs? Maybe it can be
a two cell property with <&apps_shared 0xf0> to indicate the offset to
the registers to read/write? In non-secure mode presumably we need to
also write these registers? Good news is that there's a regmap for this
driver already, so maybe that can be acquired from the pdc driver.

Re: [PATCH RFC 05/14] dt-bindings/interrupt-controller: pdc: add SPI config register

[Lina Iyer]

Sorry, I couldn't get to this earlier.

On Thu, Sep 05 2019 at 18:03 -0600, Stephen Boyd wrote:
>Quoting Lina Iyer (2019-09-03 10:07:22)
>> On Mon, Sep 02 2019 at 07:58 -0600, Marc Zyngier wrote:
>> >On 02/09/2019 14:38, Rob Herring wrote:
>> >> On Thu, Aug 29, 2019 at 12:11:54PM -0600, Lina Iyer wrote:
>> These are not GIC registers but located on the PDC interface to the GIC.
>> They may or may not be secure access controlled, depending on the SoC.
>>
>
>It looks like it falls under this "mailbox" device which is really the
>catch all bucket for bits with no home besides they're related to the
>apps CPUs/subsystem.
>
Thanks for pointing to this.
>	apss_shared: mailbox@17990000 {
>		compatible = "qcom,sdm845-apss-shared";
>		reg = <0 0x17990000 0 0x1000>;
But this doesn't seem correct. The registers in this page are all not
mailbox door bell registers. We should restrict the space allocated to
the mbox to 0xC or something, definitely, not the whole page. They all
cannot be treated as a mailbox registers.
>		#mbox-cells = <1>;
>	};
>
>Can you point to this node with a phandle and then parse the reg
>property out of it to use in the scm readl/writel APIs? Maybe it can be
>a two cell property with <&apps_shared 0xf0> to indicate the offset to
>the registers to read/write? In non-secure mode presumably we need to
>also write these registers? Good news is that there's a regmap for this
>driver already, so maybe that can be acquired from the pdc driver.
>
The register space collection seems to be mix of different types of
application processor registers that should probably not be grouped up
under one subsystem. A single regmap doesn't seem correct either.

-- Lina

Re: [PATCH RFC 05/14] dt-bindings/interrupt-controller: pdc: add SPI config register

[Lina Iyer]

Adding Sibi

On Fri, Sep 13 2019 at 13:53 -0600, Lina Iyer wrote:
>Sorry, I couldn't get to this earlier.
>
>On Thu, Sep 05 2019 at 18:03 -0600, Stephen Boyd wrote:
>>Quoting Lina Iyer (2019-09-03 10:07:22)
>>>On Mon, Sep 02 2019 at 07:58 -0600, Marc Zyngier wrote:
>>>>On 02/09/2019 14:38, Rob Herring wrote:
>>>>> On Thu, Aug 29, 2019 at 12:11:54PM -0600, Lina Iyer wrote:
>>>These are not GIC registers but located on the PDC interface to the GIC.
>>>They may or may not be secure access controlled, depending on the SoC.
>>>
>>
>>It looks like it falls under this "mailbox" device which is really the
>>catch all bucket for bits with no home besides they're related to the
>>apps CPUs/subsystem.
>>
>Thanks for pointing to this.
>>	apss_shared: mailbox@17990000 {
>>		compatible = "qcom,sdm845-apss-shared";
>>		reg = <0 0x17990000 0 0x1000>;
>But this doesn't seem correct. The registers in this page are all not
>mailbox door bell registers. We should restrict the space allocated to
>the mbox to 0xC or something, definitely, not the whole page. They all
>cannot be treated as a mailbox registers.
>>		#mbox-cells = <1>;
>>	};
>>
>>Can you point to this node with a phandle and then parse the reg
>>property out of it to use in the scm readl/writel APIs? Maybe it can be
>>a two cell property with <&apps_shared 0xf0> to indicate the offset to
>>the registers to read/write? In non-secure mode presumably we need to
>>also write these registers? Good news is that there's a regmap for this
>>driver already, so maybe that can be acquired from the pdc driver.
>>
>The register space collection seems to be mix of different types of
>application processor registers that should probably not be grouped up
>under one subsystem. A single regmap doesn't seem correct either.
>
>-- Lina

Re: [PATCH RFC 05/14] dt-bindings/interrupt-controller: pdc: add SPI config register

[Stephen Boyd]

Quoting Lina Iyer (2019-09-17 14:50:20)
> On Fri, Sep 13 2019 at 13:53 -0600, Lina Iyer wrote:
> >On Thu, Sep 05 2019 at 18:03 -0600, Stephen Boyd wrote:
> >>Quoting Lina Iyer (2019-09-03 10:07:22)
> >>>On Mon, Sep 02 2019 at 07:58 -0600, Marc Zyngier wrote:
> >>>>On 02/09/2019 14:38, Rob Herring wrote:
> >>>>> On Thu, Aug 29, 2019 at 12:11:54PM -0600, Lina Iyer wrote:
> >>>These are not GIC registers but located on the PDC interface to the GIC.
> >>>They may or may not be secure access controlled, depending on the SoC.
> >>>
> >>
> >>It looks like it falls under this "mailbox" device which is really the
> >>catch all bucket for bits with no home besides they're related to the
> >>apps CPUs/subsystem.
> >>
> >Thanks for pointing to this.
> >>      apss_shared: mailbox@17990000 {
> >>              compatible = "qcom,sdm845-apss-shared";
> >>              reg = <0 0x17990000 0 0x1000>;
> >But this doesn't seem correct. The registers in this page are all not
> >mailbox door bell registers. We should restrict the space allocated to
> >the mbox to 0xC or something, definitely, not the whole page. They all
> >cannot be treated as a mailbox registers.

Well the binding is already done and this is the compatible string for
this node and register region. Sounds like this node is a mailbox plus
some more stuff in the same page.

> >>              #mbox-cells = <1>;
> >>      };
> >>
> >>Can you point to this node with a phandle and then parse the reg
> >>property out of it to use in the scm readl/writel APIs? Maybe it can be
> >>a two cell property with <&apps_shared 0xf0> to indicate the offset to
> >>the registers to read/write? In non-secure mode presumably we need to
> >>also write these registers? Good news is that there's a regmap for this
> >>driver already, so maybe that can be acquired from the pdc driver.
> >>
> >The register space collection seems to be mix of different types of
> >application processor registers that should probably not be grouped up
> >under one subsystem. A single regmap doesn't seem correct either.

Why isn't a single regmap correct? The PDC driver should be able to use
it to read/write into this register space. The lock on the regmap will
need to be changed to a raw lock though for RT. Otherwise it looks OK to
me.

Re: [PATCH RFC 05/14] dt-bindings/interrupt-controller: pdc: add SPI config register

[Sibi Sankar]

On 2019-09-21 03:50, Stephen Boyd wrote:
> Quoting Lina Iyer (2019-09-17 14:50:20)
>> On Fri, Sep 13 2019 at 13:53 -0600, Lina Iyer wrote:
>> >On Thu, Sep 05 2019 at 18:03 -0600, Stephen Boyd wrote:
>> >>Quoting Lina Iyer (2019-09-03 10:07:22)
>> >>>On Mon, Sep 02 2019 at 07:58 -0600, Marc Zyngier wrote:
>> >>>>On 02/09/2019 14:38, Rob Herring wrote:
>> >>>>> On Thu, Aug 29, 2019 at 12:11:54PM -0600, Lina Iyer wrote:
>> >>>These are not GIC registers but located on the PDC interface to the GIC.
>> >>>They may or may not be secure access controlled, depending on the SoC.
>> >>>
>> >>
>> >>It looks like it falls under this "mailbox" device which is really the
>> >>catch all bucket for bits with no home besides they're related to the
>> >>apps CPUs/subsystem.
>> >>
>> >Thanks for pointing to this.
>> >>      apss_shared: mailbox@17990000 {
>> >>              compatible = "qcom,sdm845-apss-shared";
>> >>              reg = <0 0x17990000 0 0x1000>;
>> >But this doesn't seem correct. The registers in this page are all not
>> >mailbox door bell registers. We should restrict the space allocated to
>> >the mbox to 0xC or something, definitely, not the whole page. They all
>> >cannot be treated as a mailbox registers.
> 
> Well the binding is already done and this is the compatible string for
> this node and register region. Sounds like this node is a mailbox plus
> some more stuff in the same page.
> 

Bjorn already noticed ^^ during the
original review. Hence the compatible
was correctly named "apss-shared"
instead of following the older bindings.

>> >>              #mbox-cells = <1>;
>> >>      };
>> >>
>> >>Can you point to this node with a phandle and then parse the reg
>> >>property out of it to use in the scm readl/writel APIs? Maybe it can be
>> >>a two cell property with <&apps_shared 0xf0> to indicate the offset to
>> >>the registers to read/write? In non-secure mode presumably we need to
>> >>also write these registers? Good news is that there's a regmap for this
>> >>driver already, so maybe that can be acquired from the pdc driver.
>> >>
>> >The register space collection seems to be mix of different types of
>> >application processor registers that should probably not be grouped up
>> >under one subsystem. A single regmap doesn't seem correct either.
> 
> Why isn't a single regmap correct? The PDC driver should be able to use
> it to read/write into this register space. The lock on the regmap will
> need to be changed to a raw lock though for RT. Otherwise it looks OK 
> to
> me.

-- 
-- Sibi Sankar --
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.

Re: [PATCH RFC 05/14] dt-bindings/interrupt-controller: pdc: add SPI config register

[Linus Walleij]

On Thu, Aug 29, 2019 at 8:47 PM Lina Iyer <ilina@codeaurora.org> wrote:

> +- qcom,scm-spi-cfg:
> +       Usage: optional
> +       Value type: <bool>
> +       Definition: Specifies if the SPI configuration registers have to be
> +                   written from the firmware.
> +
>  Example:
>
>         pdc: interrupt-controller@b220000 {
>                 compatible = "qcom,sdm845-pdc";
> -               reg = <0xb220000 0x30000>;
> +               reg = <0xb220000 0x30000>, <0x179900f0 0x60>;
>                 qcom,pdc-ranges = <0 512 94>, <94 641 15>, <115 662 7>;
>                 #interrupt-cells = <2>;
>                 interrupt-parent = <&intc>;
>                 interrupt-controller;
> +               qcom,scm-spi-cfg;

You can probably drop this bool if you just give names to the registers.

Like
reg = <0xb220000 0x30000>, <0x179900f0 0x60>;
reg-names = "gic", "pdc";

Then jus check explicitly for a "pdc" register and in that case
initialize the PDC.

Yours,
Linus Walleij

Re: [PATCH RFC 05/14] dt-bindings/interrupt-controller: pdc: add SPI config register

[Lina Iyer]

On Wed, Sep 11 2019 at 04:05 -0600, Linus Walleij wrote:
>On Thu, Aug 29, 2019 at 8:47 PM Lina Iyer <ilina@codeaurora.org> wrote:
>
>> +- qcom,scm-spi-cfg:
>> +       Usage: optional
>> +       Value type: <bool>
>> +       Definition: Specifies if the SPI configuration registers have to be
>> +                   written from the firmware.
>> +
>>  Example:
>>
>>         pdc: interrupt-controller@b220000 {
>>                 compatible = "qcom,sdm845-pdc";
>> -               reg = <0xb220000 0x30000>;
>> +               reg = <0xb220000 0x30000>, <0x179900f0 0x60>;
>>                 qcom,pdc-ranges = <0 512 94>, <94 641 15>, <115 662 7>;
>>                 #interrupt-cells = <2>;
>>                 interrupt-parent = <&intc>;
>>                 interrupt-controller;
>> +               qcom,scm-spi-cfg;
>
>You can probably drop this bool if you just give names to the registers.
>
>Like
>reg = <0xb220000 0x30000>, <0x179900f0 0x60>;
>reg-names = "gic", "pdc";
>
>Then jus check explicitly for a "pdc" register and in that case
>initialize the PDC.
>
Well the address remains the same. The bool defines how to access that
register address - from linux or from the firmware using SCM calls. But
I get your point, I could have different register namess - pdc-linux or
pdc-scm and request by name. I can then use that to determine the mode
for accessing the register.

Thanks,
Lina