[PATCH v5 02/10] dt-bindings: introduce RPMH RSC bindings for Qualcomm SoCs

[PATCH v5 02/10] dt-bindings: introduce RPMH RSC bindings for Qualcomm SoCs

[Lina Iyer]

Add device binding documentation for Qualcomm Technology Inc's RPMH RSC
driver. The driver is used for communicating resource state requests for
shared resources.

Cc: devicetree@vger.kernel.org
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---

Changes in v3:
	- Move to soc/qcom
	- Amend text per Stephen's suggestions

Changes in v2:
	- Amend text to describe the registers in reg property
	- Add reg-names for the registers
	- Update examples to use GIC_SPI in interrupts instead of 0
	- Rephrase incorrect description

Changes in v3:
	- Fix unwanted capitalization
	- Remove clients from the examples, this doc does not describe
	  them
	- Rephrase introductory paragraph
	- Remove hardware specifics from DT bindings
---
 .../devicetree/bindings/soc/qcom/rpmh-rsc.txt      | 127 +++++++++++++++++++++
 1 file changed, 127 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt

diff --git a/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
new file mode 100644
index 000000000000..dcf71a5b302f
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
@@ -0,0 +1,127 @@
+RPMH RSC:
+------------
+
+Resource Power Manager Hardened (RPMH) is the mechanism for communicating with
+the hardened resource accelerators on Qualcomm SoCs. Requests to the resources
+can be written to the Trigger Command Set (TCS)  registers and using a (addr,
+val) pair and triggered. Messages in the TCS are then sent in sequence over an
+internal bus.
+
+The hardware block (Direct Resource Voter or DRV) is a part of the h/w entity
+(Resource State Coordinator a.k.a RSC) that can handle a multiple sleep and
+active/wake resource requests. Multiple such DRVs can exist in a SoC and can
+be written to from Linux. The structure of each DRV follows the same template
+with a few variations that are captured by the properties here.
+
+A TCS may be triggered from Linux or triggered by the F/W after all the CPUs
+have powered off to facilitate idle power saving. TCS could be classified as -
+
+	SLEEP,  /* Triggered by F/W */
+	WAKE,   /* Triggered by F/W */
+	ACTIVE, /* Triggered by Linux */
+	CONTROL /* Triggered by F/W */
+
+The order in which they are described in the DT, should match the hardware
+configuration.
+
+Requests can be made for the state of a resource, when the subsystem is active
+or idle. When all subsystems like Modem, GPU, CPU are idle, the resource state
+will be an aggregate of the sleep votes from each of those subsystems. Clients
+may request a sleep value for their shared resources in addition to the active
+mode requests.
+
+Properties:
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: Should be "qcom,rpmh-rsc".
+
+- reg:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: The first register specifies the base address of the DRV.
+	            The second register specifies the start address of the
+	            TCS.
+
+- reg-names:
+	Usage: required
+	Value type: <string>
+	Definition: Maps the register specified in the reg property. Must be
+	            "drv" and "tcs".
+
+- interrupts:
+	Usage: required
+	Value type: <prop-encoded-interrupt>
+	Definition: The interrupt that trips when a message complete/response
+	           is received for this DRV from the accelerators.
+
+- qcom,drv-id:
+	Usage: required
+	Value type: <u32>
+	Definition: the id of the DRV in the RSC block.
+
+- qcom,tcs-config:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: the tuple defining the configuration of TCS.
+	            Must have 2 cells which describe each TCS type.
+	            <type number_of_tcs>.
+	            The order of the TCS must match the hardware
+	            configuration.
+	- Cell #1 (TCS Type): TCS types to be specified -
+	            SLEEP_TCS
+	            WAKE_TCS
+	            ACTIVE_TCS
+	            CONTROL_TCS
+	- Cell #2 (Number of TCS): <u32>
+
+- label:
+	Usage: optional
+	Value type: <string>
+	Definition: Name for the RSC. The name would be used in trace logs.
+
+Drivers that want to use the RSC to communicate with RPMH must specify their
+bindings as child of the RSC controllers they wish to communicate with.
+
+Example 1:
+
+For a TCS whose RSC base address is is 0x179C0000 and is at a DRV id of 2, the
+register offsets for DRV2 start at 0D00, the register calculations are like
+this -
+First tuple: 0x179C0000 + 0x10000 * 2 = 0x179E0000
+Second tuple: 0x179E0000 + 0xD00 = 0x179E0D00
+
+	apps_rsc: rsc@179e000 {
+		label = "apps_rsc";
+		compatible = "qcom,rpmh-rsc";
+		reg = <0x179e0000 0x10000>, <0x179e0d00 0x3000>;
+		reg-names = "drv", "tcs";
+		interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>;
+		qcom,drv-id = <2>;
+		qcom,tcs-config = <SLEEP_TCS   3>,
+				  <WAKE_TCS    3>,
+				  <ACTIVE_TCS  2>,
+				  <CONTROL_TCS 1>;
+	};
+
+Example 2:
+
+For a TCS whose RSC base address is 0xAF20000 and is at DRV id of 0, the
+register offsets for DRV0 start at 01C00, the register calculations are like
+this -
+First tuple: 0xAF20000
+Second tuple: 0xAF20000 + 0x1C00 = 0xAF21C00
+
+	disp_rsc: rsc@af20000 {
+		label = "disp_rsc";
+		compatible = "qcom,rpmh-rsc";
+		reg = <0xaf20000 0x10000>, <0xaf21c00 0x3000>;
+		reg-names = "drv", "tcs";
+		interrupts = <GIC_SPI 129 IRQ_TYPE_LEVEL_HIGH>;
+		qcom,drv-id = <0>;
+		qcom,tcs-config = <SLEEP_TCS   1>,
+				  <WAKE_TCS    1>,
+				  <ACTIVE_TCS  0>,
+				  <CONTROL_TCS 0>;
+	};
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v5 02/10] dt-bindings: introduce RPMH RSC bindings for Qualcomm SoCs

[Stephen Boyd]

Quoting Lina Iyer (2018-04-05 09:18:26)
> diff --git a/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
> new file mode 100644
> index 000000000000..dcf71a5b302f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
> @@ -0,0 +1,127 @@
> +RPMH RSC:
> +------------
> +
> +Resource Power Manager Hardened (RPMH) is the mechanism for communicating with
> +the hardened resource accelerators on Qualcomm SoCs. Requests to the resources
> +can be written to the Trigger Command Set (TCS)  registers and using a (addr,
> +val) pair and triggered. Messages in the TCS are then sent in sequence over an
> +internal bus.
> +
> +The hardware block (Direct Resource Voter or DRV) is a part of the h/w entity
> +(Resource State Coordinator a.k.a RSC) that can handle a multiple sleep and

s/ a / /

> +active/wake resource requests. Multiple such DRVs can exist in a SoC and can
> +be written to from Linux. The structure of each DRV follows the same template
> +with a few variations that are captured by the properties here.
> +
> +A TCS may be triggered from Linux or triggered by the F/W after all the CPUs
> +have powered off to facilitate idle power saving. TCS could be classified as -

s/ -/:/

> +
> +       SLEEP,  /* Triggered by F/W */
> +       WAKE,   /* Triggered by F/W */
> +       ACTIVE, /* Triggered by Linux */
> +       CONTROL /* Triggered by F/W */

Drop the commas?

> +
> +The order in which they are described in the DT, should match the hardware
> +configuration.
> +
> +Requests can be made for the state of a resource, when the subsystem is active
> +or idle. When all subsystems like Modem, GPU, CPU are idle, the resource state
> +will be an aggregate of the sleep votes from each of those subsystems. Clients
> +may request a sleep value for their shared resources in addition to the active
> +mode requests.
> +
> +Properties:
> +
> +- compatible:
> +       Usage: required
> +       Value type: <string>
> +       Definition: Should be "qcom,rpmh-rsc".
> +
> +- reg:
> +       Usage: required
> +       Value type: <prop-encoded-array>
> +       Definition: The first register specifies the base address of the DRV.
> +                   The second register specifies the start address of the
> +                   TCS.
> +
> +- reg-names:
> +       Usage: required
> +       Value type: <string>
> +       Definition: Maps the register specified in the reg property. Must be
> +                   "drv" and "tcs".
> +
> +- interrupts:
> +       Usage: required
> +       Value type: <prop-encoded-interrupt>
> +       Definition: The interrupt that trips when a message complete/response
> +                  is received for this DRV from the accelerators.
> +
> +- qcom,drv-id:
> +       Usage: required
> +       Value type: <u32>
> +       Definition: the id of the DRV in the RSC block.
> +
> +- qcom,tcs-config:
> +       Usage: required
> +       Value type: <prop-encoded-array>
> +       Definition: the tuple defining the configuration of TCS.
> +                   Must have 2 cells which describe each TCS type.
> +                   <type number_of_tcs>.
> +                   The order of the TCS must match the hardware
> +                   configuration.
> +       - Cell #1 (TCS Type): TCS types to be specified -
> +                   SLEEP_TCS
> +                   WAKE_TCS
> +                   ACTIVE_TCS
> +                   CONTROL_TCS
> +       - Cell #2 (Number of TCS): <u32>
> +
> +- label:
> +       Usage: optional
> +       Value type: <string>
> +       Definition: Name for the RSC. The name would be used in trace logs.
> +
> +Drivers that want to use the RSC to communicate with RPMH must specify their
> +bindings as child of the RSC controllers they wish to communicate with.

s/child/child nodes/


> +
> +Example 1:
> +
> +For a TCS whose RSC base address is is 0x179C0000 and is at a DRV id of 2, the
> +register offsets for DRV2 start at 0D00, the register calculations are like
> +this -
> +First tuple: 0x179C0000 + 0x10000 * 2 = 0x179E0000
> +Second tuple: 0x179E0000 + 0xD00 = 0x179E0D00
> +
> +       apps_rsc: rsc@179e000 {
> +               label = "apps_rsc";
> +               compatible = "qcom,rpmh-rsc";
> +               reg = <0x179e0000 0x10000>, <0x179e0d00 0x3000>;

The first reg property overlaps the second one. Does this second one
ever move around? I would hardcode it in the driver to be 0xd00 away
from the drv base instead of specifying it in DT if it's the same all
the time.

Also, the example shows 0x179c0000 which I guess is the actual beginning
of the RSC block. So the binding seems to be for one DRV inside of an
RSC. Can we get the full description of the RSC in the binding instead?
I imagine that means there's a DRV0,1,2 and those probably have an
interrupt per each DRV and then a different TCS config per each one too?
If the binding can describe all of the RSC then we can use different
DRVs by changing the qcom,drv-id property.

	rsc@179c0000 {
		compatible = "qcom,rpmh-rsc";
		reg = <0x179c0000 0x10000>,
		      <0x179d0000 0x10000>,
		      <0x179e0000 0x10000>;
		qcom,tcs-offset = <0xd00>;
		qcom,drv-id = <0/1/2>;
		interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
			     <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
			     <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>;
	}

This is sort of what I imagine it would look like. I have no idea how
the tcs config would work unless each DRV has the same TCS config
though. Otherwise, if each node is for a drv, then I would expect the
node would be called 'drv' and we wouldn't need the drv-id property and
the compatible string would say drv instead of rsc?

BTW, what are the other DRVs used for in the apps RSC?

> +               reg-names = "drv", "tcs";
> +               interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>;
> +               qcom,drv-id = <2>;
> +               qcom,tcs-config = <SLEEP_TCS   3>,
> +                                 <WAKE_TCS    3>,
> +                                 <ACTIVE_TCS  2>,
> +                                 <CONTROL_TCS 1>;
> +       };
> +
> +Example 2:
> +
> +For a TCS whose RSC base address is 0xAF20000 and is at DRV id of 0, the
> +register offsets for DRV0 start at 01C00, the register calculations are like
> +this -
> +First tuple: 0xAF20000
> +Second tuple: 0xAF20000 + 0x1C00 = 0xAF21C00
> +
> +       disp_rsc: rsc@af20000 {
> +               label = "disp_rsc";
> +               compatible = "qcom,rpmh-rsc";
> +               reg = <0xaf20000 0x10000>, <0xaf21c00 0x3000>;

Ok. The TCS offset seems totally random now.

> +               reg-names = "drv", "tcs";
> +               interrupts = <GIC_SPI 129 IRQ_TYPE_LEVEL_HIGH>;
> +               qcom,drv-id = <0>;
> +               qcom,tcs-config = <SLEEP_TCS   1>,
> +                                 <WAKE_TCS    1>,
> +                                 <ACTIVE_TCS  0>,
> +                                 <CONTROL_TCS 0>;
> +       };

Re: [PATCH v5 02/10] dt-bindings: introduce RPMH RSC bindings for Qualcomm SoCs

[Lina Iyer]

On Fri, Apr 06 2018 at 19:14 -0600, Stephen Boyd wrote:
>Quoting Lina Iyer (2018-04-05 09:18:26)
>> diff --git a/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
>> new file mode 100644
>> index 000000000000..dcf71a5b302f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
>> @@ -0,0 +1,127 @@
>> +RPMH RSC:
>> +------------
>> +
>> +Resource Power Manager Hardened (RPMH) is the mechanism for communicating with
>> +the hardened resource accelerators on Qualcomm SoCs. Requests to the resources
>> +can be written to the Trigger Command Set (TCS)  registers and using a (addr,
>> +val) pair and triggered. Messages in the TCS are then sent in sequence over an
>> +internal bus.
>> +
>> +The hardware block (Direct Resource Voter or DRV) is a part of the h/w entity
>> +(Resource State Coordinator a.k.a RSC) that can handle a multiple sleep and
>
>s/ a / /
>
>> +active/wake resource requests. Multiple such DRVs can exist in a SoC and can
>> +be written to from Linux. The structure of each DRV follows the same template
>> +with a few variations that are captured by the properties here.
>> +
>> +A TCS may be triggered from Linux or triggered by the F/W after all the CPUs
>> +have powered off to facilitate idle power saving. TCS could be classified as -
>
>s/ -/:/
>
>> +
>> +       SLEEP,  /* Triggered by F/W */
>> +       WAKE,   /* Triggered by F/W */
>> +       ACTIVE, /* Triggered by Linux */
>> +       CONTROL /* Triggered by F/W */
>
>Drop the commas?
>
>> +
>> +The order in which they are described in the DT, should match the hardware
>> +configuration.
>> +
>> +Requests can be made for the state of a resource, when the subsystem is active
>> +or idle. When all subsystems like Modem, GPU, CPU are idle, the resource state
>> +will be an aggregate of the sleep votes from each of those subsystems. Clients
>> +may request a sleep value for their shared resources in addition to the active
>> +mode requests.
>> +
>> +Properties:
>> +
>> +- compatible:
>> +       Usage: required
>> +       Value type: <string>
>> +       Definition: Should be "qcom,rpmh-rsc".
>> +
>> +- reg:
>> +       Usage: required
>> +       Value type: <prop-encoded-array>
>> +       Definition: The first register specifies the base address of the DRV.
>> +                   The second register specifies the start address of the
>> +                   TCS.
>> +
>> +- reg-names:
>> +       Usage: required
>> +       Value type: <string>
>> +       Definition: Maps the register specified in the reg property. Must be
>> +                   "drv" and "tcs".
>> +
>> +- interrupts:
>> +       Usage: required
>> +       Value type: <prop-encoded-interrupt>
>> +       Definition: The interrupt that trips when a message complete/response
>> +                  is received for this DRV from the accelerators.
>> +
>> +- qcom,drv-id:
>> +       Usage: required
>> +       Value type: <u32>
>> +       Definition: the id of the DRV in the RSC block.
>> +
>> +- qcom,tcs-config:
>> +       Usage: required
>> +       Value type: <prop-encoded-array>
>> +       Definition: the tuple defining the configuration of TCS.
>> +                   Must have 2 cells which describe each TCS type.
>> +                   <type number_of_tcs>.
>> +                   The order of the TCS must match the hardware
>> +                   configuration.
>> +       - Cell #1 (TCS Type): TCS types to be specified -
>> +                   SLEEP_TCS
>> +                   WAKE_TCS
>> +                   ACTIVE_TCS
>> +                   CONTROL_TCS
>> +       - Cell #2 (Number of TCS): <u32>
>> +
>> +- label:
>> +       Usage: optional
>> +       Value type: <string>
>> +       Definition: Name for the RSC. The name would be used in trace logs.
>> +
>> +Drivers that want to use the RSC to communicate with RPMH must specify their
>> +bindings as child of the RSC controllers they wish to communicate with.
>
>s/child/child nodes/
>
>
Ok to these as well as the above.

>> +
>> +Example 1:
>> +
>> +For a TCS whose RSC base address is is 0x179C0000 and is at a DRV id of 2, the
>> +register offsets for DRV2 start at 0D00, the register calculations are like
>> +this -
>> +First tuple: 0x179C0000 + 0x10000 * 2 = 0x179E0000
>> +Second tuple: 0x179E0000 + 0xD00 = 0x179E0D00
>> +
>> +       apps_rsc: rsc@179e000 {
>> +               label = "apps_rsc";
>> +               compatible = "qcom,rpmh-rsc";
>> +               reg = <0x179e0000 0x10000>, <0x179e0d00 0x3000>;
>
>The first reg property overlaps the second one. Does this second one
>ever move around? I would hardcode it in the driver to be 0xd00 away
>from the drv base instead of specifying it in DT if it's the same all
>the time.
>
>Also, the example shows 0x179c0000 which I guess is the actual beginning
>of the RSC block. So the binding seems to be for one DRV inside of an
>RSC. Can we get the full description of the RSC in the binding instead?
>I imagine that means there's a DRV0,1,2 and those probably have an
>interrupt per each DRV and then a different TCS config per each one too?
>If the binding can describe all of the RSC then we can use different
>DRVs by changing the qcom,drv-id property.
>
>	rsc@179c0000 {
>		compatible = "qcom,rpmh-rsc";
>		reg = <0x179c0000 0x10000>,
>		      <0x179d0000 0x10000>,
>		      <0x179e0000 0x10000>;
>		qcom,tcs-offset = <0xd00>;
>		qcom,drv-id = <0/1/2>;
>		interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
>			     <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
>			     <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>;
>	}
>
>This is sort of what I imagine it would look like. I have no idea how
>the tcs config would work unless each DRV has the same TCS config
>though. Otherwise, if each node is for a drv, then I would expect the
>node would be called 'drv' and we wouldn't need the drv-id property and
>the compatible string would say drv instead of rsc?
>
>BTW, what are the other DRVs used for in the apps RSC?
>
The DRV is the voter for an execution environment (Linux, Hypervisor,
ATF) in the RSC. The RSC has a lot of other registers that Linux is not
privy to. They are access restricted. The memory organization of the RSC
mandates that we know the DRV id to access registers specific to the
DRV. Unfortunately, not all RSC have identical DRV configuration and the
register space is also variable depending on the capability of the RSC.
There are functionalities supported by other RSCs in the SoC that are
not supported by the RSC associated with the application processor,
while not many RSCs' support multiple DRVs. Therefore it doesn't benefit
describing the whole RSC as it is not usable from Linux (because of
access restrictions).

>> +               reg-names = "drv", "tcs";
>> +               interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>;
>> +               qcom,drv-id = <2>;
>> +               qcom,tcs-config = <SLEEP_TCS   3>,
>> +                                 <WAKE_TCS    3>,
>> +                                 <ACTIVE_TCS  2>,
>> +                                 <CONTROL_TCS 1>;
>> +       };
>> +
>> +Example 2:
>> +
>> +For a TCS whose RSC base address is 0xAF20000 and is at DRV id of 0, the
>> +register offsets for DRV0 start at 01C00, the register calculations are like
>> +this -
>> +First tuple: 0xAF20000
>> +Second tuple: 0xAF20000 + 0x1C00 = 0xAF21C00
>> +
>> +       disp_rsc: rsc@af20000 {
>> +               label = "disp_rsc";
>> +               compatible = "qcom,rpmh-rsc";
>> +               reg = <0xaf20000 0x10000>, <0xaf21c00 0x3000>;
>
>Ok. The TCS offset seems totally random now.
>
Yes it would appear so. Because the register space is optimized based on
the functionality supported by the RSC, the TCS for a DRV is at a
different offset in the RSC. Hence the explicit description of the
address in the binding.

Thanks,
Lina

>> +               reg-names = "drv", "tcs";
>> +               interrupts = <GIC_SPI 129 IRQ_TYPE_LEVEL_HIGH>;
>> +               qcom,drv-id = <0>;
>> +               qcom,tcs-config = <SLEEP_TCS   1>,
>> +                                 <WAKE_TCS    1>,
>> +                                 <ACTIVE_TCS  0>,
>> +                                 <CONTROL_TCS 0>;
>> +       };

Re: [PATCH v5 02/10] dt-bindings: introduce RPMH RSC bindings for Qualcomm SoCs

[Bjorn Andersson]

On Mon 09 Apr 09:08 PDT 2018, Lina Iyer wrote:

> On Fri, Apr 06 2018 at 19:14 -0600, Stephen Boyd wrote:
> > Quoting Lina Iyer (2018-04-05 09:18:26)
> > > diff --git a/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
[..]
> > > +Example 1:
> > > +
> > > +For a TCS whose RSC base address is is 0x179C0000 and is at a DRV id of 2, the
> > > +register offsets for DRV2 start at 0D00, the register calculations are like
> > > +this -
> > > +First tuple: 0x179C0000 + 0x10000 * 2 = 0x179E0000
> > > +Second tuple: 0x179E0000 + 0xD00 = 0x179E0D00
> > > +
> > > +       apps_rsc: rsc@179e000 {
> > > +               label = "apps_rsc";
> > > +               compatible = "qcom,rpmh-rsc";
> > > +               reg = <0x179e0000 0x10000>, <0x179e0d00 0x3000>;
> > 
> > The first reg property overlaps the second one. Does this second one
> > ever move around? I would hardcode it in the driver to be 0xd00 away
> > from the drv base instead of specifying it in DT if it's the same all
> > the time.
[..]
> > 
> The DRV is the voter for an execution environment (Linux, Hypervisor,
> ATF) in the RSC. The RSC has a lot of other registers that Linux is not
> privy to. They are access restricted. The memory organization of the RSC
> mandates that we know the DRV id to access registers specific to the
> DRV. Unfortunately, not all RSC have identical DRV configuration and the
> register space is also variable depending on the capability of the RSC.
> There are functionalities supported by other RSCs in the SoC that are
> not supported by the RSC associated with the application processor,
> while not many RSCs' support multiple DRVs. Therefore it doesn't benefit
> describing the whole RSC as it is not usable from Linux (because of
> access restrictions).
> 

I generally prefer that we describe the hardware blocks as accurate as
possible, instead of applying current restrictions on Linux onto the
description. This ensures that we can reuse the binding and drivers in
configurations not considered today. However, afaict we still have the
problem that we need a way to express where in the RSC our TCS sits.

Regardless of what's right or not, the given example causes the driver
to fail probing, so something needs to be changed. (Making the drv size
0xd00 is functional but doesn't really relate to any bondary in the
register space).

Regards,
Bjorn

Re: [PATCH v5 02/10] dt-bindings: introduce RPMH RSC bindings for Qualcomm SoCs

[Lina Iyer]

On Tue, Apr 10 2018 at 13:36 -0600, Bjorn Andersson wrote:
>On Mon 09 Apr 09:08 PDT 2018, Lina Iyer wrote:
>
>> On Fri, Apr 06 2018 at 19:14 -0600, Stephen Boyd wrote:
>> > Quoting Lina Iyer (2018-04-05 09:18:26)
>> > > diff --git a/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
>[..]
>> > > +Example 1:
>> > > +
>> > > +For a TCS whose RSC base address is is 0x179C0000 and is at a DRV id of 2, the
>> > > +register offsets for DRV2 start at 0D00, the register calculations are like
>> > > +this -
>> > > +First tuple: 0x179C0000 + 0x10000 * 2 = 0x179E0000
>> > > +Second tuple: 0x179E0000 + 0xD00 = 0x179E0D00
>> > > +
>> > > +       apps_rsc: rsc@179e000 {
>> > > +               label = "apps_rsc";
>> > > +               compatible = "qcom,rpmh-rsc";
>> > > +               reg = <0x179e0000 0x10000>, <0x179e0d00 0x3000>;
>> >
>> > The first reg property overlaps the second one. Does this second one
>> > ever move around? I would hardcode it in the driver to be 0xd00 away
>> > from the drv base instead of specifying it in DT if it's the same all
>> > the time.
>[..]
>> >
>> The DRV is the voter for an execution environment (Linux, Hypervisor,
>> ATF) in the RSC. The RSC has a lot of other registers that Linux is not
>> privy to. They are access restricted. The memory organization of the RSC
>> mandates that we know the DRV id to access registers specific to the
>> DRV. Unfortunately, not all RSC have identical DRV configuration and the
>> register space is also variable depending on the capability of the RSC.
>> There are functionalities supported by other RSCs in the SoC that are
>> not supported by the RSC associated with the application processor,
>> while not many RSCs' support multiple DRVs. Therefore it doesn't benefit
>> describing the whole RSC as it is not usable from Linux (because of
>> access restrictions).
>>
>
>I generally prefer that we describe the hardware blocks as accurate as
>possible, instead of applying current restrictions on Linux onto the
>description. This ensures that we can reuse the binding and drivers in
>configurations not considered today. However, afaict we still have the
>problem that we need a way to express where in the RSC our TCS sits.
>
>Regardless of what's right or not, the given example causes the driver
>to fail probing, so something needs to be changed.
I have been using this in DT and I haven't seen failures. Could you send
me the logs?

Thanks,
Lina

>(Making the drv size
>0xd00 is functional but doesn't really relate to any bondary in the
>register space).
>
>Regards,
>Bjorn

Re: [PATCH v5 02/10] dt-bindings: introduce RPMH RSC bindings for Qualcomm SoCs

[Stephen Boyd]

Quoting Lina Iyer (2018-04-09 09:08:00)
> On Fri, Apr 06 2018 at 19:14 -0600, Stephen Boyd wrote:
> >Quoting Lina Iyer (2018-04-05 09:18:26)
> >> diff --git a/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
> >> new file mode 100644
> >> index 000000000000..dcf71a5b302f
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
> >> @@ -0,0 +1,127 @@
> >> +
> >> +Example 1:
> >> +
> >> +For a TCS whose RSC base address is is 0x179C0000 and is at a DRV id of 2, the
> >> +register offsets for DRV2 start at 0D00, the register calculations are like
> >> +this -
> >> +First tuple: 0x179C0000 + 0x10000 * 2 = 0x179E0000
> >> +Second tuple: 0x179E0000 + 0xD00 = 0x179E0D00
> >> +
> >> +       apps_rsc: rsc@179e000 {
> >> +               label = "apps_rsc";
> >> +               compatible = "qcom,rpmh-rsc";
> >> +               reg = <0x179e0000 0x10000>, <0x179e0d00 0x3000>;
> >
> >The first reg property overlaps the second one. Does this second one
> >ever move around? I would hardcode it in the driver to be 0xd00 away
> >from the drv base instead of specifying it in DT if it's the same all
> >the time.
> >
> >Also, the example shows 0x179c0000 which I guess is the actual beginning
> >of the RSC block. So the binding seems to be for one DRV inside of an
> >RSC. Can we get the full description of the RSC in the binding instead?
> >I imagine that means there's a DRV0,1,2 and those probably have an
> >interrupt per each DRV and then a different TCS config per each one too?
> >If the binding can describe all of the RSC then we can use different
> >DRVs by changing the qcom,drv-id property.
> >
> >       rsc@179c0000 {
> >               compatible = "qcom,rpmh-rsc";
> >               reg = <0x179c0000 0x10000>,
> >                     <0x179d0000 0x10000>,
> >                     <0x179e0000 0x10000>;
> >               qcom,tcs-offset = <0xd00>;
> >               qcom,drv-id = <0/1/2>;
> >               interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
> >                            <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
> >                            <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>;
> >       }
> >
> >This is sort of what I imagine it would look like. I have no idea how
> >the tcs config would work unless each DRV has the same TCS config
> >though. Otherwise, if each node is for a drv, then I would expect the
> >node would be called 'drv' and we wouldn't need the drv-id property and
> >the compatible string would say drv instead of rsc?
> >
> >BTW, what are the other DRVs used for in the apps RSC?
> >
> The DRV is the voter for an execution environment (Linux, Hypervisor,
> ATF) in the RSC. The RSC has a lot of other registers that Linux is not
> privy to. They are access restricted.

Alright. Well sometimes access restrictions aren't there, so this isn't
a good assumption to make.

> The memory organization of the RSC
> mandates that we know the DRV id to access registers specific to the
> DRV.

I think qcom,drv-id covers that, no?

> Unfortunately, not all RSC have identical DRV configuration and the
> register space is also variable depending on the capability of the RSC.
> There are functionalities supported by other RSCs in the SoC that are
> not supported by the RSC associated with the application processor,
> while not many RSCs' support multiple DRVs. Therefore it doesn't benefit
> describing the whole RSC as it is not usable from Linux (because of
> access restrictions).

If we're not describing the whole RSC in the RSC binding then we're not
going to get very far. From what I can tell, this binding describes one
DRV inside of an RSC instead of the whole RSC. Yes we'll probably never
use the ATF part of the RSC in Linux, but we may use the hypervisor part
if we use KVM/Xen so the binding should be describing as much as it can
about this device in case some software needs to use it.

Put another way, even if the "apps" RSC is complicated, we should be
describing it to the best of our abilities in the binding so that when
it is used by non-linux OSes things still work by simply tweaking the
drv-id that we use to pick the right things out of the node.

Or we're describing the RSC but it's really a container node that
doesn't do much besides hold DRVs? So this is described at the wrong
level?

Re: [PATCH v5 02/10] dt-bindings: introduce RPMH RSC bindings for Qualcomm SoCs

[Lina Iyer]

On Wed, Apr 11 2018 at 09:29 -0600, Stephen Boyd wrote:
>Quoting Lina Iyer (2018-04-09 09:08:00)
>> On Fri, Apr 06 2018 at 19:14 -0600, Stephen Boyd wrote:
>> >Quoting Lina Iyer (2018-04-05 09:18:26)
>> >> diff --git a/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
>> >> new file mode 100644
>> >> index 000000000000..dcf71a5b302f
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
>> >> @@ -0,0 +1,127 @@
>> >> +
>> >> +Example 1:
>> >> +
>> >> +For a TCS whose RSC base address is is 0x179C0000 and is at a DRV id of 2, the
>> >> +register offsets for DRV2 start at 0D00, the register calculations are like
>> >> +this -
>> >> +First tuple: 0x179C0000 + 0x10000 * 2 = 0x179E0000
>> >> +Second tuple: 0x179E0000 + 0xD00 = 0x179E0D00
>> >> +
>> >> +       apps_rsc: rsc@179e000 {
>> >> +               label = "apps_rsc";
>> >> +               compatible = "qcom,rpmh-rsc";
>> >> +               reg = <0x179e0000 0x10000>, <0x179e0d00 0x3000>;
>> >
>> >The first reg property overlaps the second one. Does this second one
>> >ever move around? I would hardcode it in the driver to be 0xd00 away
>> >from the drv base instead of specifying it in DT if it's the same all
>> >the time.
>> >
>> >Also, the example shows 0x179c0000 which I guess is the actual beginning
>> >of the RSC block. So the binding seems to be for one DRV inside of an
>> >RSC. Can we get the full description of the RSC in the binding instead?
>> >I imagine that means there's a DRV0,1,2 and those probably have an
>> >interrupt per each DRV and then a different TCS config per each one too?
>> >If the binding can describe all of the RSC then we can use different
>> >DRVs by changing the qcom,drv-id property.
>> >
>> >       rsc@179c0000 {
>> >               compatible = "qcom,rpmh-rsc";
>> >               reg = <0x179c0000 0x10000>,
>> >                     <0x179d0000 0x10000>,
>> >                     <0x179e0000 0x10000>;
>> >               qcom,tcs-offset = <0xd00>;
>> >               qcom,drv-id = <0/1/2>;
>> >               interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
>> >                            <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
>> >                            <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>;
>> >       }
>> >
>> >This is sort of what I imagine it would look like. I have no idea how
>> >the tcs config would work unless each DRV has the same TCS config
>> >though. Otherwise, if each node is for a drv, then I would expect the
>> >node would be called 'drv' and we wouldn't need the drv-id property and
>> >the compatible string would say drv instead of rsc?
>> >
>> >BTW, what are the other DRVs used for in the apps RSC?
>> >
>> The DRV is the voter for an execution environment (Linux, Hypervisor,
>> ATF) in the RSC. The RSC has a lot of other registers that Linux is not
>> privy to. They are access restricted.
>
>Alright. Well sometimes access restrictions aren't there, so this isn't
>a good assumption to make.
>
>> The memory organization of the RSC
>> mandates that we know the DRV id to access registers specific to the
>> DRV.
>
>I think qcom,drv-id covers that, no?
>
>> Unfortunately, not all RSC have identical DRV configuration and the
>> register space is also variable depending on the capability of the RSC.
>> There are functionalities supported by other RSCs in the SoC that are
>> not supported by the RSC associated with the application processor,
>> while not many RSCs' support multiple DRVs. Therefore it doesn't benefit
>> describing the whole RSC as it is not usable from Linux (because of
>> access restrictions).
>
>If we're not describing the whole RSC in the RSC binding then we're not
>going to get very far. From what I can tell, this binding describes one
>DRV inside of an RSC instead of the whole RSC. Yes we'll probably never
>use the ATF part of the RSC in Linux, but we may use the hypervisor part
>if we use KVM/Xen so the binding should be describing as much as it can
>about this device in case some software needs to use it.
>
The RSC is pretty much this. A set of registers that are RSC specific at
the address pointed to by the "rsc" reg and the TCS regsiters pointed to
by the "tcs" reg. You do not want to clobber multiple DRVs into the same
device node. It will be a lot confusing for the drivers to determine
which DRV to vote.
>Put another way, even if the "apps" RSC is complicated, we should be
>describing it to the best of our abilities in the binding so that when
>it is used by non-linux OSes things still work by simply tweaking the
>drv-id that we use to pick the right things out of the node.
>
>Or we're describing the RSC but it's really a container node that
>doesn't do much besides hold DRVs? So this is described at the wrong
>level?
What we are describing is a DRV, but a standalone DRV alone is useless
without the necessary RSC registers. So its a unique RSC+DRV combination
that is represented here.

Hope that helps.

-- Lina

Re: [PATCH v5 02/10] dt-bindings: introduce RPMH RSC bindings for Qualcomm SoCs

[Stephen Boyd]

Quoting Lina Iyer (2018-04-11 14:24:31)
> On Wed, Apr 11 2018 at 09:29 -0600, Stephen Boyd wrote:
> >Quoting Lina Iyer (2018-04-09 09:08:00)
> >> On Fri, Apr 06 2018 at 19:14 -0600, Stephen Boyd wrote:
> >> >Quoting Lina Iyer (2018-04-05 09:18:26)
> >> >> diff --git a/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
> >> >> new file mode 100644
> >> >> index 000000000000..dcf71a5b302f
> >> >> --- /dev/null
> >> >> +++ b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
> >> >> @@ -0,0 +1,127 @@
> >> >> +
> >> >> +Example 1:
> >> >> +
> >> >> +For a TCS whose RSC base address is is 0x179C0000 and is at a DRV id of 2, the
> >> >> +register offsets for DRV2 start at 0D00, the register calculations are like
> >> >> +this -
> >> >> +First tuple: 0x179C0000 + 0x10000 * 2 = 0x179E0000
> >> >> +Second tuple: 0x179E0000 + 0xD00 = 0x179E0D00
> >> >> +
> >> >> +       apps_rsc: rsc@179e000 {
> >> >> +               label = "apps_rsc";
> >> >> +               compatible = "qcom,rpmh-rsc";
> >> >> +               reg = <0x179e0000 0x10000>, <0x179e0d00 0x3000>;
> >> >
> >> >The first reg property overlaps the second one. Does this second one
> >> >ever move around? I would hardcode it in the driver to be 0xd00 away
> >> >from the drv base instead of specifying it in DT if it's the same all
> >> >the time.
> >> >
> >> >Also, the example shows 0x179c0000 which I guess is the actual beginning
> >> >of the RSC block. So the binding seems to be for one DRV inside of an
> >> >RSC. Can we get the full description of the RSC in the binding instead?
> >> >I imagine that means there's a DRV0,1,2 and those probably have an
> >> >interrupt per each DRV and then a different TCS config per each one too?
> >> >If the binding can describe all of the RSC then we can use different
> >> >DRVs by changing the qcom,drv-id property.
> >> >
> >> >       rsc@179c0000 {
> >> >               compatible = "qcom,rpmh-rsc";
> >> >               reg = <0x179c0000 0x10000>,
> >> >                     <0x179d0000 0x10000>,
> >> >                     <0x179e0000 0x10000>;
> >> >               qcom,tcs-offset = <0xd00>;
> >> >               qcom,drv-id = <0/1/2>;
> >> >               interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
> >> >                            <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
> >> >                            <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>;
> >> >       }
> >> >
> >> >This is sort of what I imagine it would look like. I have no idea how
> >> >the tcs config would work unless each DRV has the same TCS config
> >> >though. Otherwise, if each node is for a drv, then I would expect the
> >> >node would be called 'drv' and we wouldn't need the drv-id property and
> >> >the compatible string would say drv instead of rsc?
> >> >
> >> >BTW, what are the other DRVs used for in the apps RSC?
> >> >
> >> The DRV is the voter for an execution environment (Linux, Hypervisor,
> >> ATF) in the RSC. The RSC has a lot of other registers that Linux is not
> >> privy to. They are access restricted.
> >
> >Alright. Well sometimes access restrictions aren't there, so this isn't
> >a good assumption to make.
> >
> >> The memory organization of the RSC
> >> mandates that we know the DRV id to access registers specific to the
> >> DRV.
> >
> >I think qcom,drv-id covers that, no?
> >
> >> Unfortunately, not all RSC have identical DRV configuration and the
> >> register space is also variable depending on the capability of the RSC.
> >> There are functionalities supported by other RSCs in the SoC that are
> >> not supported by the RSC associated with the application processor,
> >> while not many RSCs' support multiple DRVs. Therefore it doesn't benefit
> >> describing the whole RSC as it is not usable from Linux (because of
> >> access restrictions).
> >
> >If we're not describing the whole RSC in the RSC binding then we're not
> >going to get very far. From what I can tell, this binding describes one
> >DRV inside of an RSC instead of the whole RSC. Yes we'll probably never
> >use the ATF part of the RSC in Linux, but we may use the hypervisor part
> >if we use KVM/Xen so the binding should be describing as much as it can
> >about this device in case some software needs to use it.
> >
> The RSC is pretty much this. A set of registers that are RSC specific at
> the address pointed to by the "rsc" reg and the TCS regsiters pointed to
> by the "tcs" reg. You do not want to clobber multiple DRVs into the same
> device node. It will be a lot confusing for the drivers to determine
> which DRV to vote.

Well it seems like an RSC contains many DRVs and those DRVs contain many
TCSes. This is what I get after talking with Bjorn on IRC.

	+--------------------------------------------------+ (0x00000)
	|                                                  |
	|                  DRV #0                          |
	|                                                  |
	|----------                          --------------| (tcs-offset (0xd00))
	|                  DRV0_TCS0                       | 
	|                common space                      | 
	|                cmd sequencer                     | 0xd00 + 0x14
	|                                                  |
	|                  DRV0_TCS1                       | 
	|                common space                      | 0xd00 + 0x2a0
	|                cmd sequencer                     | 0xd00 + 0x2a0 + 0x14
	|                                                  |
	|                  DRV0_TCS2                       | 
	|                                                  |
	|                                                  |
	+--------------------------------------------------+ (0x10000)
	|                                                  |
	|                  DRV #1                          |
	|                                                  |
	|----------                          --------------| (tcs-offset)
	|                  DRV1_TCS0                       | 
	|                  DRV1_TCS1                       | 
	|                  DRV1_TCS2                       | 
	+--------------------------------------------------+ (0x20000)
	|                                                  |
	|                  DRV #2                          |
	|                                                  |
	|----------                          --------------|
	|                  DRV2_TCS0                       | 
	|                  DRV2_TCS1                       | 
	|                  DRV2_TCS2                       | 
	|                  DRV2_TCS3                       | 
	|                  DRV2_TCS4                       | 
	|                  DRV2_TCS5                       | 
	+--------------------------------------------------+

I think I understand it now. There aren't any "RSC common" registers
that are common to the entire RSC. Instead, everything goes into a DRV,
or into a common TCS space, or into a TCS "queue".

> >Put another way, even if the "apps" RSC is complicated, we should be
> >describing it to the best of our abilities in the binding so that when
> >it is used by non-linux OSes things still work by simply tweaking the
> >drv-id that we use to pick the right things out of the node.
> >
> >Or we're describing the RSC but it's really a container node that
> >doesn't do much besides hold DRVs? So this is described at the wrong
> >level?
> What we are describing is a DRV, but a standalone DRV alone is useless
> without the necessary RSC registers. So its a unique RSC+DRV combination
> that is represented here.
> 

If my understanding is correct up there then the binding could either
describe a single RSC DRV, or it could describe all the RSC DRV
instances and interrupts going into the RSC "block" and then we can use
drv-id to pick the offset we jump to.

I imagine we don't have any practical use-case for the entire RSC space
because there aren't any common RSC registers to deal with. So we've
boiled this all down to describing one DRV and then I wonder why we care
about having drv-id at all? It looks to be used to check for a max
number of TCS, but that's already described by DT so it doesn't seem
very useful to double check what the hardware can tells us.

Long story short, we can remove drv-id and just describe drvs by
themselves?

Re: [PATCH v5 02/10] dt-bindings: introduce RPMH RSC bindings for Qualcomm SoCs

[Lina Iyer]

On Fri, Apr 13 2018 at 16:40 -0600, Stephen Boyd wrote:
>Quoting Lina Iyer (2018-04-11 14:24:31)
>> On Wed, Apr 11 2018 at 09:29 -0600, Stephen Boyd wrote:
>> >Quoting Lina Iyer (2018-04-09 09:08:00)
>> >> On Fri, Apr 06 2018 at 19:14 -0600, Stephen Boyd wrote:
>> >> >Quoting Lina Iyer (2018-04-05 09:18:26)
>> >> >> diff --git a/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
>> >> >> new file mode 100644
>> >> >> index 000000000000..dcf71a5b302f
>> >> >> --- /dev/null
>> >> >> +++ b/Documentation/devicetree/bindings/soc/qcom/rpmh-rsc.txt
>> >> >> @@ -0,0 +1,127 @@
>> >> >> +
>> >> >> +Example 1:
>> >> >> +
>> >> >> +For a TCS whose RSC base address is is 0x179C0000 and is at a DRV id of 2, the
>> >> >> +register offsets for DRV2 start at 0D00, the register calculations are like
>> >> >> +this -
>> >> >> +First tuple: 0x179C0000 + 0x10000 * 2 = 0x179E0000
>> >> >> +Second tuple: 0x179E0000 + 0xD00 = 0x179E0D00
>> >> >> +
>> >> >> +       apps_rsc: rsc@179e000 {
>> >> >> +               label = "apps_rsc";
>> >> >> +               compatible = "qcom,rpmh-rsc";
>> >> >> +               reg = <0x179e0000 0x10000>, <0x179e0d00 0x3000>;
>> >> >
>> >> >The first reg property overlaps the second one. Does this second one
>> >> >ever move around? I would hardcode it in the driver to be 0xd00 away
>> >> >from the drv base instead of specifying it in DT if it's the same all
>> >> >the time.
>> >> >
>> >> >Also, the example shows 0x179c0000 which I guess is the actual beginning
>> >> >of the RSC block. So the binding seems to be for one DRV inside of an
>> >> >RSC. Can we get the full description of the RSC in the binding instead?
>> >> >I imagine that means there's a DRV0,1,2 and those probably have an
>> >> >interrupt per each DRV and then a different TCS config per each one too?
>> >> >If the binding can describe all of the RSC then we can use different
>> >> >DRVs by changing the qcom,drv-id property.
>> >> >
>> >> >       rsc@179c0000 {
>> >> >               compatible = "qcom,rpmh-rsc";
>> >> >               reg = <0x179c0000 0x10000>,
>> >> >                     <0x179d0000 0x10000>,
>> >> >                     <0x179e0000 0x10000>;
>> >> >               qcom,tcs-offset = <0xd00>;
>> >> >               qcom,drv-id = <0/1/2>;
>> >> >               interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>,
>> >> >                            <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>,
>> >> >                            <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>;
>> >> >       }
>> >> >
>> >> >This is sort of what I imagine it would look like. I have no idea how
>> >> >the tcs config would work unless each DRV has the same TCS config
>> >> >though. Otherwise, if each node is for a drv, then I would expect the
>> >> >node would be called 'drv' and we wouldn't need the drv-id property and
>> >> >the compatible string would say drv instead of rsc?
>> >> >
>> >> >BTW, what are the other DRVs used for in the apps RSC?
>> >> >
>> >> The DRV is the voter for an execution environment (Linux, Hypervisor,
>> >> ATF) in the RSC. The RSC has a lot of other registers that Linux is not
>> >> privy to. They are access restricted.
>> >
>> >Alright. Well sometimes access restrictions aren't there, so this isn't
>> >a good assumption to make.
>> >
>> >> The memory organization of the RSC
>> >> mandates that we know the DRV id to access registers specific to the
>> >> DRV.
>> >
>> >I think qcom,drv-id covers that, no?
>> >
>> >> Unfortunately, not all RSC have identical DRV configuration and the
>> >> register space is also variable depending on the capability of the RSC.
>> >> There are functionalities supported by other RSCs in the SoC that are
>> >> not supported by the RSC associated with the application processor,
>> >> while not many RSCs' support multiple DRVs. Therefore it doesn't benefit
>> >> describing the whole RSC as it is not usable from Linux (because of
>> >> access restrictions).
>> >
>> >If we're not describing the whole RSC in the RSC binding then we're not
>> >going to get very far. From what I can tell, this binding describes one
>> >DRV inside of an RSC instead of the whole RSC. Yes we'll probably never
>> >use the ATF part of the RSC in Linux, but we may use the hypervisor part
>> >if we use KVM/Xen so the binding should be describing as much as it can
>> >about this device in case some software needs to use it.
>> >
>> The RSC is pretty much this. A set of registers that are RSC specific at
>> the address pointed to by the "rsc" reg and the TCS regsiters pointed to
>> by the "tcs" reg. You do not want to clobber multiple DRVs into the same
>> device node. It will be a lot confusing for the drivers to determine
>> which DRV to vote.
>
>Well it seems like an RSC contains many DRVs and those DRVs contain many
>TCSes. This is what I get after talking with Bjorn on IRC.
>
>	+--------------------------------------------------+ (0x00000)
>	|                                                  |
>	|                  DRV #0                          |
>	|                                                  |
>	|----------                          --------------| (tcs-offset (0xd00))
>	|                  DRV0_TCS0                       |
>	|                common space                      |
>	|                cmd sequencer                     | 0xd00 + 0x14
>	|                                                  |
>	|                  DRV0_TCS1                       |
>	|                common space                      | 0xd00 + 0x2a0
>	|                cmd sequencer                     | 0xd00 + 0x2a0 + 0x14
>	|                                                  |
>	|                  DRV0_TCS2                       |
>	|                                                  |
>	|                                                  |
>	+--------------------------------------------------+ (0x10000)
>	|                                                  |
>	|                  DRV #1                          |
>	|                                                  |
>	|----------                          --------------| (tcs-offset)
>	|                  DRV1_TCS0                       |
>	|                  DRV1_TCS1                       |
>	|                  DRV1_TCS2                       |
>	+--------------------------------------------------+ (0x20000)
>	|                                                  |
>	|                  DRV #2                          |
>	|                                                  |
>	|----------                          --------------|
>	|                  DRV2_TCS0                       |
>	|                  DRV2_TCS1                       |
>	|                  DRV2_TCS2                       |
>	|                  DRV2_TCS3                       |
>	|                  DRV2_TCS4                       |
>	|                  DRV2_TCS5                       |
>	+--------------------------------------------------+
>
>I think I understand it now. There aren't any "RSC common" registers
>that are common to the entire RSC. Instead, everything goes into a DRV,
>or into a common TCS space, or into a TCS "queue".
>
>> >Put another way, even if the "apps" RSC is complicated, we should be
>> >describing it to the best of our abilities in the binding so that when
>> >it is used by non-linux OSes things still work by simply tweaking the
>> >drv-id that we use to pick the right things out of the node.
>> >
>> >Or we're describing the RSC but it's really a container node that
>> >doesn't do much besides hold DRVs? So this is described at the wrong
>> >level?
>> What we are describing is a DRV, but a standalone DRV alone is useless
>> without the necessary RSC registers. So its a unique RSC+DRV combination
>> that is represented here.
>>
>
>If my understanding is correct up there then the binding could either
>describe a single RSC DRV, or it could describe all the RSC DRV
>instances and interrupts going into the RSC "block" and then we can use
>drv-id to pick the offset we jump to.
>
Your understanding is correct.

>I imagine we don't have any practical use-case for the entire RSC space
>because there aren't any common RSC registers to deal with.
Not true.

>So we've
>boiled this all down to describing one DRV and then I wonder why we care
>about having drv-id at all? It looks to be used to check for a max
>number of TCS, but that's already described by DT so it doesn't seem
>very useful to double check what the hardware can tells us.
>
There is also a number of commands per TCS (NCPT), that may way vary
between different RSCs. The RSC of the application processor has 16
commands in each TCS, but that is variable. I am not saying it cannot be
described in DT, but it is something I read from the common RSC
registers, currently.
Also, I will using common/DRV0 registers to write wakeup time value,
when the processor subsystem goes into power down. This is not DRV2
register, but is a DRV0 register that we will have special access to.
The patches for those I intend to publish, when we have support for
sleep/suspend with this new architecture. So the address of the start of
the RSC (=DRV0) is necessary.

>Long story short, we can remove drv-id and just describe drvs by
>themselves?
Yes, we may. As long as I have a way to describe the register addresss
of the start of the DRV (0x20000 for DRV#2)  and the tcs-offset (0xd00),
we can work with the RSC-DRV in the driver.

Thanks,
Lina

Re: [PATCH v5 02/10] dt-bindings: introduce RPMH RSC bindings for Qualcomm SoCs

[Stephen Boyd]

Quoting Lina Iyer (2018-04-16 09:08:18)
> On Fri, Apr 13 2018 at 16:40 -0600, Stephen Boyd wrote:
> >Well it seems like an RSC contains many DRVs and those DRVs contain many
> >TCSes. This is what I get after talking with Bjorn on IRC.
> >
> >       +--------------------------------------------------+ (0x00000)
> >       |                                                  |
> >       |                  DRV #0                          |
> >       |                                                  |
> >       |----------                          --------------| (tcs-offset (0xd00))
> >       |                  DRV0_TCS0                       |
> >       |                common space                      |
> >       |                cmd sequencer                     | 0xd00 + 0x14
> >       |                                                  |
> >       |                  DRV0_TCS1                       |
> >       |                common space                      | 0xd00 + 0x2a0
> >       |                cmd sequencer                     | 0xd00 + 0x2a0 + 0x14
> >       |                                                  |
> >       |                  DRV0_TCS2                       |
> >       |                                                  |
> >       |                                                  |
> >       +--------------------------------------------------+ (0x10000)
> >       |                                                  |
> >       |                  DRV #1                          |
> >       |                                                  |
> >       |----------                          --------------| (tcs-offset)
> >       |                  DRV1_TCS0                       |
> >       |                  DRV1_TCS1                       |
> >       |                  DRV1_TCS2                       |
> >       +--------------------------------------------------+ (0x20000)
> >       |                                                  |
> >       |                  DRV #2                          |
> >       |                                                  |
> >       |----------                          --------------|
> >       |                  DRV2_TCS0                       |
> >       |                  DRV2_TCS1                       |
> >       |                  DRV2_TCS2                       |
> >       |                  DRV2_TCS3                       |
> >       |                  DRV2_TCS4                       |
> >       |                  DRV2_TCS5                       |
> >       +--------------------------------------------------+
> >
> >I think I understand it now. There aren't any "RSC common" registers
> >that are common to the entire RSC. Instead, everything goes into a DRV,
> >or into a common TCS space, or into a TCS "queue".
> >
> >> >Put another way, even if the "apps" RSC is complicated, we should be
> >> >describing it to the best of our abilities in the binding so that when
> >> >it is used by non-linux OSes things still work by simply tweaking the
> >> >drv-id that we use to pick the right things out of the node.
> >> >
> >> >Or we're describing the RSC but it's really a container node that
> >> >doesn't do much besides hold DRVs? So this is described at the wrong
> >> >level?
> >> What we are describing is a DRV, but a standalone DRV alone is useless
> >> without the necessary RSC registers. So its a unique RSC+DRV combination
> >> that is represented here.
> >>
> >
> >If my understanding is correct up there then the binding could either
> >describe a single RSC DRV, or it could describe all the RSC DRV
> >instances and interrupts going into the RSC "block" and then we can use
> >drv-id to pick the offset we jump to.
> >
> Your understanding is correct.
> 
> >I imagine we don't have any practical use-case for the entire RSC space
> >because there aren't any common RSC registers to deal with.
> Not true.

So then my understanding is not correct! :/

> 
> >So we've
> >boiled this all down to describing one DRV and then I wonder why we care
> >about having drv-id at all? It looks to be used to check for a max
> >number of TCS, but that's already described by DT so it doesn't seem
> >very useful to double check what the hardware can tells us.
> >
> There is also a number of commands per TCS (NCPT), that may way vary
> between different RSCs. The RSC of the application processor has 16
> commands in each TCS, but that is variable. I am not saying it cannot be
> described in DT, but it is something I read from the common RSC
> registers, currently.
> Also, I will using common/DRV0 registers to write wakeup time value,
> when the processor subsystem goes into power down. This is not DRV2
> register, but is a DRV0 register that we will have special access to.
> The patches for those I intend to publish, when we have support for
> sleep/suspend with this new architecture. So the address of the start of
> the RSC (=DRV0) is necessary.
> 
> >Long story short, we can remove drv-id and just describe drvs by
> >themselves?
> Yes, we may. As long as I have a way to describe the register addresss
> of the start of the DRV (0x20000 for DRV#2)  and the tcs-offset (0xd00),
> we can work with the RSC-DRV in the driver.
> 

From this new information it seems like we need to know about all the
DRVs in the RSC then. So let's go back to my previous binding proposal
describing all of them and having the qcom,drv-id property describe
which one to use most of the time and hardcode the assumption to use
DRV0 in the driver when we need to do things for sleep/suspend? Then
we're back to describing the whole RSC and configuring the picker to
pick the right DRV depending on firmware configuration.

Re: [PATCH v5 02/10] dt-bindings: introduce RPMH RSC bindings for Qualcomm SoCs

[Lina Iyer]

On Mon, Apr 16 2018 at 00:01 -0600, Stephen Boyd wrote:
>Quoting Lina Iyer (2018-04-16 09:08:18)
>> On Fri, Apr 13 2018 at 16:40 -0600, Stephen Boyd wrote:
>> >Well it seems like an RSC contains many DRVs and those DRVs contain many
>> >TCSes. This is what I get after talking with Bjorn on IRC.
>> >
>> >       +--------------------------------------------------+ (0x00000)
>> >       |                                                  |
>> >       |                  DRV #0                          |
>> >       |                                                  |
>> >       |----------                          --------------| (tcs-offset (0xd00))
>> >       |                  DRV0_TCS0                       |
>> >       |                common space                      |
>> >       |                cmd sequencer                     | 0xd00 + 0x14
>> >       |                                                  |
>> >       |                  DRV0_TCS1                       |
>> >       |                common space                      | 0xd00 + 0x2a0
>> >       |                cmd sequencer                     | 0xd00 + 0x2a0 + 0x14
>> >       |                                                  |
>> >       |                  DRV0_TCS2                       |
>> >       |                                                  |
>> >       |                                                  |
>> >       +--------------------------------------------------+ (0x10000)
>> >       |                                                  |
>> >       |                  DRV #1                          |
>> >       |                                                  |
>> >       |----------                          --------------| (tcs-offset)
>> >       |                  DRV1_TCS0                       |
>> >       |                  DRV1_TCS1                       |
>> >       |                  DRV1_TCS2                       |
>> >       +--------------------------------------------------+ (0x20000)
>> >       |                                                  |
>> >       |                  DRV #2                          |
>> >       |                                                  |
>> >       |----------                          --------------|
>> >       |                  DRV2_TCS0                       |
>> >       |                  DRV2_TCS1                       |
>> >       |                  DRV2_TCS2                       |
>> >       |                  DRV2_TCS3                       |
>> >       |                  DRV2_TCS4                       |
>> >       |                  DRV2_TCS5                       |
>> >       +--------------------------------------------------+
>> >
>> >I think I understand it now. There aren't any "RSC common" registers
>> >that are common to the entire RSC. Instead, everything goes into a DRV,
>> >or into a common TCS space, or into a TCS "queue".
>> >
>> >> >Put another way, even if the "apps" RSC is complicated, we should be
>> >> >describing it to the best of our abilities in the binding so that when
>> >> >it is used by non-linux OSes things still work by simply tweaking the
>> >> >drv-id that we use to pick the right things out of the node.
>> >> >
>> >> >Or we're describing the RSC but it's really a container node that
>> >> >doesn't do much besides hold DRVs? So this is described at the wrong
>> >> >level?
>> >> What we are describing is a DRV, but a standalone DRV alone is useless
>> >> without the necessary RSC registers. So its a unique RSC+DRV combination
>> >> that is represented here.
>> >>
>> >
>> >If my understanding is correct up there then the binding could either
>> >describe a single RSC DRV, or it could describe all the RSC DRV
>> >instances and interrupts going into the RSC "block" and then we can use
>> >drv-id to pick the offset we jump to.
>> >
>> Your understanding is correct.
>>
>> >I imagine we don't have any practical use-case for the entire RSC space
>> >because there aren't any common RSC registers to deal with.
>> Not true.
>
>So then my understanding is not correct! :/
>
>>
>> >So we've
>> >boiled this all down to describing one DRV and then I wonder why we care
>> >about having drv-id at all? It looks to be used to check for a max
>> >number of TCS, but that's already described by DT so it doesn't seem
>> >very useful to double check what the hardware can tells us.
>> >
>> There is also a number of commands per TCS (NCPT), that may way vary
>> between different RSCs. The RSC of the application processor has 16
>> commands in each TCS, but that is variable. I am not saying it cannot be
>> described in DT, but it is something I read from the common RSC
>> registers, currently.
>> Also, I will using common/DRV0 registers to write wakeup time value,
>> when the processor subsystem goes into power down. This is not DRV2
>> register, but is a DRV0 register that we will have special access to.
>> The patches for those I intend to publish, when we have support for
>> sleep/suspend with this new architecture. So the address of the start of
>> the RSC (=DRV0) is necessary.
>>
>> >Long story short, we can remove drv-id and just describe drvs by
>> >themselves?
>> Yes, we may. As long as I have a way to describe the register addresss
>> of the start of the DRV (0x20000 for DRV#2)  and the tcs-offset (0xd00),
>> we can work with the RSC-DRV in the driver.
>>
>
>From this new information it seems like we need to know about all the
>DRVs in the RSC then. So let's go back to my previous binding proposal
>describing all of them and having the qcom,drv-id property describe
>which one to use most of the time and hardcode the assumption to use
>DRV0 in the driver when we need to do things for sleep/suspend? Then
>we're back to describing the whole RSC and configuring the picker to
>pick the right DRV depending on firmware configuration.
>
Hmm.. I am okay with that, even though it might be bit confusing to
define all that and not use them.

-- Lina