[PATCH v3 0/4] ARM: K2G: Add support for TI-SCI Generic PM Domains

[PATCH v3 0/4] ARM: K2G: Add support for TI-SCI Generic PM Domains

[Dave Gerlach]

Hi,
This is v3 of the series to add support for TI-SCI Generic PM Domains.
Previous versions can be found here:

v2: https://www.spinics.net/lists/kernel/msg2364612.html
v1: http://www.spinics.net/lists/arm-kernel/msg525204.html

This version is rebased on v4.10-rc2 but is the same as v2 with the
exception of patch 2 in which the devicetree binding documentation
needed to be updated to show the k2g_pds node should be a child of
the pmmc node. Apart from that, the acks provided by Ulf were added
to patches 1 and 3.

Now that the TI-SCI series has been merged [1] this series will be ready
to go in with an ack on the DT binding. Rob had raised some questions on
the necessity ti,sci-id property but I believe these were properly
addressed during the discussion of v2 so hopefully an ack is in order now.

Regards,
Dave

[1] http://www.spinics.net/lists/arm-kernel/msg536851.html

Dave Gerlach (4):
  PM / Domains: Add generic data pointer to genpd data struct
  dt-bindings: Add TI SCI PM Domains
  soc: ti: Add ti_sci_pm_domains driver
  ARM: keystone: Drop PM domain support for k2g

 .../devicetree/bindings/soc/ti/sci-pm-domain.txt   |  59 ++++++
 MAINTAINERS                                        |   3 +
 arch/arm/mach-keystone/Kconfig                     |   1 +
 arch/arm/mach-keystone/pm_domain.c                 |   4 +-
 drivers/soc/ti/Kconfig                             |  12 ++
 drivers/soc/ti/Makefile                            |   1 +
 drivers/soc/ti/ti_sci_pm_domains.c                 | 198 +++++++++++++++++++++
 include/dt-bindings/genpd/k2g.h                    |  90 ++++++++++
 include/linux/pm_domain.h                          |   1 +
 9 files changed, 368 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
 create mode 100644 drivers/soc/ti/ti_sci_pm_domains.c
 create mode 100644 include/dt-bindings/genpd/k2g.h

-- 
2.11.0

[PATCH v3 1/4] PM / Domains: Add generic data pointer to genpd data struct

[Dave Gerlach]

Add a void *data pointer to struct generic_pm_domain_data. Because this
exists for each device associated with a genpd it will allow us to
assign per-device data if needed on a platform for control of that
specific device.

Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
 include/linux/pm_domain.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 81ece61075df..73c9dba5cfcc 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -117,6 +117,7 @@ struct generic_pm_domain_data {
 	struct pm_domain_data base;
 	struct gpd_timing_data td;
 	struct notifier_block nb;
+	void *data;
 };
 
 #ifdef CONFIG_PM_GENERIC_DOMAINS
-- 
2.11.0

[PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains

[Dave Gerlach]

Add a generic power domain implementation, TI SCI PM Domains, that
will hook into the genpd framework and allow the TI SCI protocol to
control device power states.

Also, provide macros representing each device index as understood
by TI SCI to be used in the device node power-domain references.
These are identifiers for the K2G devices managed by the PMMC.

Signed-off-by: Nishanth Menon <nm@ti.com>
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
v2->v3:
	Update k2g_pds node docs to show it should be a child of pmmc node.
	In early versions a phandle was used to point to pmmc and docs still
	incorrectly showed this.

 .../devicetree/bindings/soc/ti/sci-pm-domain.txt   | 59 ++++++++++++++
 MAINTAINERS                                        |  2 +
 include/dt-bindings/genpd/k2g.h                    | 90 ++++++++++++++++++++++
 3 files changed, 151 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
 create mode 100644 include/dt-bindings/genpd/k2g.h

diff --git a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
new file mode 100644
index 000000000000..4c9064e512cb
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
@@ -0,0 +1,59 @@
+Texas Instruments TI-SCI Generic Power Domain
+---------------------------------------------
+
+Some TI SoCs contain a system controller (like the PMMC, etc...) that is
+responsible for controlling the state of the IPs that are present.
+Communication between the host processor running an OS and the system
+controller happens through a protocol known as TI-SCI [1]. This pm domain
+implementation plugs into the generic pm domain framework and makes use of
+the TI SCI protocol power on and off each device when needed.
+
+[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
+
+PM Domain Node
+==============
+The PM domain node represents the global PM domain managed by the PMMC,
+which in this case is the single implementation as documented by the generic
+PM domain bindings in Documentation/devicetree/bindings/power/power_domain.txt.
+Because this relies on the TI SCI protocol to communicate with the PMMC it
+must be a child of the pmmc node.
+
+Required Properties:
+--------------------
+- compatible: should be "ti,sci-pm-domain"
+- #power-domain-cells: Must be 0.
+
+Example (K2G):
+-------------
+	pmmc: pmmc {
+		compatible = "ti,k2g-sci";
+		...
+
+		k2g_pds: k2g_pds {
+			compatible = "ti,sci-pm-domain";
+			#power-domain-cells = <0>;
+		};
+	};
+
+PM Domain Consumers
+===================
+Hardware blocks that require SCI control over their state must provide
+a reference to the sci-pm-domain they are part of and a unique device
+specific ID that identifies the device.
+
+Required Properties:
+--------------------
+- power-domains: phandle pointing to the corresponding PM domain node.
+- ti,sci-id: index representing the device id to be passed oevr SCI to
+	     be used for device control.
+
+See dt-bindings/genpd/k2g.h for the list of valid identifiers for k2g.
+
+Example (K2G):
+--------------------
+	uart0: serial@02530c00 {
+		compatible = "ns16550a";
+		...
+		power-domains = <&k2g_pds>;
+		ti,sci-id = <K2G_DEV_UART0>;
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index cfff2c9e3d94..7e68af170dae 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12144,6 +12144,8 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
 F:	drivers/firmware/ti_sci*
 F:	include/linux/soc/ti/ti_sci_protocol.h
+F:	Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
+F:	include/dt-bindings/genpd/k2g.h
 
 THANKO'S RAREMONO AM/FM/SW RADIO RECEIVER USB DRIVER
 M:	Hans Verkuil <hverkuil@xs4all.nl>
diff --git a/include/dt-bindings/genpd/k2g.h b/include/dt-bindings/genpd/k2g.h
new file mode 100644
index 000000000000..91ad827e0ca1
--- /dev/null
+++ b/include/dt-bindings/genpd/k2g.h
@@ -0,0 +1,90 @@
+/*
+ * TI K2G SoC Device definitions
+ *
+ * Copyright (C) 2015-2016 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _DT_BINDINGS_GENPD_K2G_H
+#define _DT_BINDINGS_GENPD_K2G_H
+
+/* Documented in http://processors.wiki.ti.com/index.php/TISCI */
+
+#define K2G_DEV_PMMC0			0x0000
+#define K2G_DEV_MLB0			0x0001
+#define K2G_DEV_DSS0			0x0002
+#define K2G_DEV_MCBSP0			0x0003
+#define K2G_DEV_MCASP0			0x0004
+#define K2G_DEV_MCASP1			0x0005
+#define K2G_DEV_MCASP2			0x0006
+#define K2G_DEV_DCAN0			0x0008
+#define K2G_DEV_DCAN1			0x0009
+#define K2G_DEV_EMIF0			0x000a
+#define K2G_DEV_MMCHS0			0x000b
+#define K2G_DEV_MMCHS1			0x000c
+#define K2G_DEV_GPMC0			0x000d
+#define K2G_DEV_ELM0			0x000e
+#define K2G_DEV_SPI0			0x0010
+#define K2G_DEV_SPI1			0x0011
+#define K2G_DEV_SPI2			0x0012
+#define K2G_DEV_SPI3			0x0013
+#define K2G_DEV_ICSS0			0x0014
+#define K2G_DEV_ICSS1			0x0015
+#define K2G_DEV_USB0			0x0016
+#define K2G_DEV_USB1			0x0017
+#define K2G_DEV_NSS0			0x0018
+#define K2G_DEV_PCIE0			0x0019
+#define K2G_DEV_GPIO0			0x001b
+#define K2G_DEV_GPIO1			0x001c
+#define K2G_DEV_TIMER64_0		0x001d
+#define K2G_DEV_TIMER64_1		0x001e
+#define K2G_DEV_TIMER64_2		0x001f
+#define K2G_DEV_TIMER64_3		0x0020
+#define K2G_DEV_TIMER64_4		0x0021
+#define K2G_DEV_TIMER64_5		0x0022
+#define K2G_DEV_TIMER64_6		0x0023
+#define K2G_DEV_MSGMGR0			0x0025
+#define K2G_DEV_BOOTCFG0		0x0026
+#define K2G_DEV_ARM_BOOTROM0		0x0027
+#define K2G_DEV_DSP_BOOTROM0		0x0029
+#define K2G_DEV_DEBUGSS0		0x002b
+#define K2G_DEV_UART0			0x002c
+#define K2G_DEV_UART1			0x002d
+#define K2G_DEV_UART2			0x002e
+#define K2G_DEV_EHRPWM0			0x002f
+#define K2G_DEV_EHRPWM1			0x0030
+#define K2G_DEV_EHRPWM2			0x0031
+#define K2G_DEV_EHRPWM3			0x0032
+#define K2G_DEV_EHRPWM4			0x0033
+#define K2G_DEV_EHRPWM5			0x0034
+#define K2G_DEV_EQEP0			0x0035
+#define K2G_DEV_EQEP1			0x0036
+#define K2G_DEV_EQEP2			0x0037
+#define K2G_DEV_ECAP0			0x0038
+#define K2G_DEV_ECAP1			0x0039
+#define K2G_DEV_I2C0			0x003a
+#define K2G_DEV_I2C1			0x003b
+#define K2G_DEV_I2C2			0x003c
+#define K2G_DEV_EDMA0			0x003f
+#define K2G_DEV_SEMAPHORE0		0x0040
+#define K2G_DEV_INTC0			0x0041
+#define K2G_DEV_GIC0			0x0042
+#define K2G_DEV_QSPI0			0x0043
+#define K2G_DEV_ARM_64B_COUNTER0	0x0044
+#define K2G_DEV_TETRIS0			0x0045
+#define K2G_DEV_CGEM0			0x0046
+#define K2G_DEV_MSMC0			0x0047
+#define K2G_DEV_CBASS0			0x0049
+#define K2G_DEV_BOARD0			0x004c
+#define K2G_DEV_EDMA1			0x004f
+
+#endif
-- 
2.11.0

Re: [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains

[Rob Herring]

On Wed, Jan 04, 2017 at 02:55:34PM -0600, Dave Gerlach wrote:
> Add a generic power domain implementation, TI SCI PM Domains, that
> will hook into the genpd framework and allow the TI SCI protocol to
> control device power states.
> 
> Also, provide macros representing each device index as understood
> by TI SCI to be used in the device node power-domain references.
> These are identifiers for the K2G devices managed by the PMMC.
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> ---
> v2->v3:
> 	Update k2g_pds node docs to show it should be a child of pmmc node.
> 	In early versions a phandle was used to point to pmmc and docs still
> 	incorrectly showed this.
> 
>  .../devicetree/bindings/soc/ti/sci-pm-domain.txt   | 59 ++++++++++++++
>  MAINTAINERS                                        |  2 +
>  include/dt-bindings/genpd/k2g.h                    | 90 ++++++++++++++++++++++
>  3 files changed, 151 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>  create mode 100644 include/dt-bindings/genpd/k2g.h
> 
> diff --git a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
> new file mode 100644
> index 000000000000..4c9064e512cb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
> @@ -0,0 +1,59 @@
> +Texas Instruments TI-SCI Generic Power Domain
> +---------------------------------------------
> +
> +Some TI SoCs contain a system controller (like the PMMC, etc...) that is
> +responsible for controlling the state of the IPs that are present.
> +Communication between the host processor running an OS and the system
> +controller happens through a protocol known as TI-SCI [1]. This pm domain
> +implementation plugs into the generic pm domain framework and makes use of
> +the TI SCI protocol power on and off each device when needed.
> +
> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
> +
> +PM Domain Node
> +==============
> +The PM domain node represents the global PM domain managed by the PMMC,
> +which in this case is the single implementation as documented by the generic
> +PM domain bindings in Documentation/devicetree/bindings/power/power_domain.txt.
> +Because this relies on the TI SCI protocol to communicate with the PMMC it
> +must be a child of the pmmc node.
> +
> +Required Properties:
> +--------------------
> +- compatible: should be "ti,sci-pm-domain"
> +- #power-domain-cells: Must be 0.
> +
> +Example (K2G):
> +-------------
> +	pmmc: pmmc {
> +		compatible = "ti,k2g-sci";
> +		...
> +
> +		k2g_pds: k2g_pds {
> +			compatible = "ti,sci-pm-domain";
> +			#power-domain-cells = <0>;
> +		};
> +	};
> +
> +PM Domain Consumers
> +===================
> +Hardware blocks that require SCI control over their state must provide
> +a reference to the sci-pm-domain they are part of and a unique device
> +specific ID that identifies the device.
> +
> +Required Properties:
> +--------------------
> +- power-domains: phandle pointing to the corresponding PM domain node.
> +- ti,sci-id: index representing the device id to be passed oevr SCI to
> +	     be used for device control.

As I've already stated before, this goes in power-domain cells. When you 
have a single thing (i.e. node) that controls multiple things, then you 
you need to specify the ID for each of them in phandle args. This is how 
irqs, gpio, clocks, *everything* in DT works.

Rob

Re: [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains

[Dave Gerlach]

Rob,
On 01/09/2017 11:50 AM, Rob Herring wrote:
> On Wed, Jan 04, 2017 at 02:55:34PM -0600, Dave Gerlach wrote:
>> Add a generic power domain implementation, TI SCI PM Domains, that
>> will hook into the genpd framework and allow the TI SCI protocol to
>> control device power states.
>>
>> Also, provide macros representing each device index as understood
>> by TI SCI to be used in the device node power-domain references.
>> These are identifiers for the K2G devices managed by the PMMC.
>>
>> Signed-off-by: Nishanth Menon <nm@ti.com>
>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>> ---
>> v2->v3:
>> 	Update k2g_pds node docs to show it should be a child of pmmc node.
>> 	In early versions a phandle was used to point to pmmc and docs still
>> 	incorrectly showed this.
>>
>>  .../devicetree/bindings/soc/ti/sci-pm-domain.txt   | 59 ++++++++++++++
>>  MAINTAINERS                                        |  2 +
>>  include/dt-bindings/genpd/k2g.h                    | 90 ++++++++++++++++++++++
>>  3 files changed, 151 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>  create mode 100644 include/dt-bindings/genpd/k2g.h
>>
>> diff --git a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>> new file mode 100644
>> index 000000000000..4c9064e512cb
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>> @@ -0,0 +1,59 @@
>> +Texas Instruments TI-SCI Generic Power Domain
>> +---------------------------------------------
>> +
>> +Some TI SoCs contain a system controller (like the PMMC, etc...) that is
>> +responsible for controlling the state of the IPs that are present.
>> +Communication between the host processor running an OS and the system
>> +controller happens through a protocol known as TI-SCI [1]. This pm domain
>> +implementation plugs into the generic pm domain framework and makes use of
>> +the TI SCI protocol power on and off each device when needed.
>> +
>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>> +
>> +PM Domain Node
>> +==============
>> +The PM domain node represents the global PM domain managed by the PMMC,
>> +which in this case is the single implementation as documented by the generic
>> +PM domain bindings in Documentation/devicetree/bindings/power/power_domain.txt.
>> +Because this relies on the TI SCI protocol to communicate with the PMMC it
>> +must be a child of the pmmc node.
>> +
>> +Required Properties:
>> +--------------------
>> +- compatible: should be "ti,sci-pm-domain"
>> +- #power-domain-cells: Must be 0.
>> +
>> +Example (K2G):
>> +-------------
>> +	pmmc: pmmc {
>> +		compatible = "ti,k2g-sci";
>> +		...
>> +
>> +		k2g_pds: k2g_pds {
>> +			compatible = "ti,sci-pm-domain";
>> +			#power-domain-cells = <0>;
>> +		};
>> +	};
>> +
>> +PM Domain Consumers
>> +===================
>> +Hardware blocks that require SCI control over their state must provide
>> +a reference to the sci-pm-domain they are part of and a unique device
>> +specific ID that identifies the device.
>> +
>> +Required Properties:
>> +--------------------
>> +- power-domains: phandle pointing to the corresponding PM domain node.
>> +- ti,sci-id: index representing the device id to be passed oevr SCI to
>> +	     be used for device control.
>
> As I've already stated before, this goes in power-domain cells. When you
> have a single thing (i.e. node) that controls multiple things, then you
> you need to specify the ID for each of them in phandle args. This is how
> irqs, gpio, clocks, *everything* in DT works.

You think the reasoning for doing it this way provided by both Ulf and 
myself on v2 [1] is not valid then?

 From Ulf:

To me, the TI SCI ID, is similar to a "conid" for any another "device
resource" (like clock, pinctrl, regulator etc) which we can describe
in DT and assign to a device node. The only difference here, is that
we don't have common API to fetch the resource (like clk_get(),
regulator_get()), but instead we fetches the device's resource from
SoC specific code, via genpd's device ->attach() callback.

 From me:

Yes, you've pretty much hit it on the head. It is not an index into a 
list of genpds but rather identifies the device *within* a single genpd. 
It is a property specific to each device that resides in a ti-sci-genpd, 
not a mapping describing which genpd the device belongs to. The generic 
power domain binding is concerned with mapping the device to a specific 
genpd, which is does fine for us, but we have a sub mapping for devices 
that exist inside a genpd which, we must describe as well, hence the 
ti,sci-id.


So to summarize, the genpd framework does interpret the phandle arg as 
an index into multiple genpds, just as you've said other frameworks do, 
but this is not what I am trying to do, we have multiple devices within 
this *single* genpd, hence the need for the ti,sci-id property.

Regards,
Dave

[1] https://patchwork.kernel.org/patch/9385371/

>
> Rob
>

Re: [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains

[Rob Herring]

On Mon, Jan 9, 2017 at 11:57 AM, Dave Gerlach <d-gerlach@ti.com> wrote:
> Rob,
>
> On 01/09/2017 11:50 AM, Rob Herring wrote:
>>
>> On Wed, Jan 04, 2017 at 02:55:34PM -0600, Dave Gerlach wrote:
>>>
>>> Add a generic power domain implementation, TI SCI PM Domains, that
>>> will hook into the genpd framework and allow the TI SCI protocol to
>>> control device power states.
>>>
>>> Also, provide macros representing each device index as understood
>>> by TI SCI to be used in the device node power-domain references.
>>> These are identifiers for the K2G devices managed by the PMMC.
>>>
>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>> ---
>>> v2->v3:
>>>         Update k2g_pds node docs to show it should be a child of pmmc
>>> node.
>>>         In early versions a phandle was used to point to pmmc and docs
>>> still
>>>         incorrectly showed this.
>>>
>>>  .../devicetree/bindings/soc/ti/sci-pm-domain.txt   | 59 ++++++++++++++
>>>  MAINTAINERS                                        |  2 +
>>>  include/dt-bindings/genpd/k2g.h                    | 90
>>> ++++++++++++++++++++++
>>>  3 files changed, 151 insertions(+)
>>>  create mode 100644
>>> Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>  create mode 100644 include/dt-bindings/genpd/k2g.h
>>>
>>> diff --git a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>> new file mode 100644
>>> index 000000000000..4c9064e512cb
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>> @@ -0,0 +1,59 @@
>>> +Texas Instruments TI-SCI Generic Power Domain
>>> +---------------------------------------------
>>> +
>>> +Some TI SoCs contain a system controller (like the PMMC, etc...) that is
>>> +responsible for controlling the state of the IPs that are present.
>>> +Communication between the host processor running an OS and the system
>>> +controller happens through a protocol known as TI-SCI [1]. This pm
>>> domain
>>> +implementation plugs into the generic pm domain framework and makes use
>>> of
>>> +the TI SCI protocol power on and off each device when needed.
>>> +
>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>>> +
>>> +PM Domain Node
>>> +==============
>>> +The PM domain node represents the global PM domain managed by the PMMC,
>>> +which in this case is the single implementation as documented by the
>>> generic
>>> +PM domain bindings in
>>> Documentation/devicetree/bindings/power/power_domain.txt.
>>> +Because this relies on the TI SCI protocol to communicate with the PMMC
>>> it
>>> +must be a child of the pmmc node.
>>> +
>>> +Required Properties:
>>> +--------------------
>>> +- compatible: should be "ti,sci-pm-domain"
>>> +- #power-domain-cells: Must be 0.
>>> +
>>> +Example (K2G):
>>> +-------------
>>> +       pmmc: pmmc {
>>> +               compatible = "ti,k2g-sci";
>>> +               ...
>>> +
>>> +               k2g_pds: k2g_pds {
>>> +                       compatible = "ti,sci-pm-domain";
>>> +                       #power-domain-cells = <0>;
>>> +               };
>>> +       };
>>> +
>>> +PM Domain Consumers
>>> +===================
>>> +Hardware blocks that require SCI control over their state must provide
>>> +a reference to the sci-pm-domain they are part of and a unique device
>>> +specific ID that identifies the device.
>>> +
>>> +Required Properties:
>>> +--------------------
>>> +- power-domains: phandle pointing to the corresponding PM domain node.
>>> +- ti,sci-id: index representing the device id to be passed oevr SCI to
>>> +            be used for device control.
>>
>>
>> As I've already stated before, this goes in power-domain cells. When you
>> have a single thing (i.e. node) that controls multiple things, then you
>> you need to specify the ID for each of them in phandle args. This is how
>> irqs, gpio, clocks, *everything* in DT works.
>
>
> You think the reasoning for doing it this way provided by both Ulf and
> myself on v2 [1] is not valid then?
>
> From Ulf:
>
> To me, the TI SCI ID, is similar to a "conid" for any another "device
> resource" (like clock, pinctrl, regulator etc) which we can describe
> in DT and assign to a device node. The only difference here, is that
> we don't have common API to fetch the resource (like clk_get(),
> regulator_get()), but instead we fetches the device's resource from
> SoC specific code, via genpd's device ->attach() callback.

Sorry, but that sounds like a kernel problem to me and has nothing to
do with DT bindings.

> From me:
>
> Yes, you've pretty much hit it on the head. It is not an index into a list
> of genpds but rather identifies the device *within* a single genpd. It is a
> property specific to each device that resides in a ti-sci-genpd, not a
> mapping describing which genpd the device belongs to. The generic power
> domain binding is concerned with mapping the device to a specific genpd,
> which is does fine for us, but we have a sub mapping for devices that exist
> inside a genpd which, we must describe as well, hence the ti,sci-id.
>
>
> So to summarize, the genpd framework does interpret the phandle arg as an
> index into multiple genpds, just as you've said other frameworks do, but
> this is not what I am trying to do, we have multiple devices within this
> *single* genpd, hence the need for the ti,sci-id property.

Fix the genpd framework rather than work around it in DT.

Rob

Re: [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains

[Dave Gerlach]

Rob,
On 01/11/2017 03:34 PM, Rob Herring wrote:
> On Mon, Jan 9, 2017 at 11:57 AM, Dave Gerlach <d-gerlach@ti.com> wrote:
>> Rob,
>>
>> On 01/09/2017 11:50 AM, Rob Herring wrote:
>>>
>>> On Wed, Jan 04, 2017 at 02:55:34PM -0600, Dave Gerlach wrote:
>>>>
>>>> Add a generic power domain implementation, TI SCI PM Domains, that
>>>> will hook into the genpd framework and allow the TI SCI protocol to
>>>> control device power states.
>>>>
>>>> Also, provide macros representing each device index as understood
>>>> by TI SCI to be used in the device node power-domain references.
>>>> These are identifiers for the K2G devices managed by the PMMC.
>>>>
>>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>>> ---
>>>> v2->v3:
>>>>         Update k2g_pds node docs to show it should be a child of pmmc
>>>> node.
>>>>         In early versions a phandle was used to point to pmmc and docs
>>>> still
>>>>         incorrectly showed this.
>>>>
>>>>  .../devicetree/bindings/soc/ti/sci-pm-domain.txt   | 59 ++++++++++++++
>>>>  MAINTAINERS                                        |  2 +
>>>>  include/dt-bindings/genpd/k2g.h                    | 90
>>>> ++++++++++++++++++++++
>>>>  3 files changed, 151 insertions(+)
>>>>  create mode 100644
>>>> Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>  create mode 100644 include/dt-bindings/genpd/k2g.h
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>> new file mode 100644
>>>> index 000000000000..4c9064e512cb
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>> @@ -0,0 +1,59 @@
>>>> +Texas Instruments TI-SCI Generic Power Domain
>>>> +---------------------------------------------
>>>> +
>>>> +Some TI SoCs contain a system controller (like the PMMC, etc...) that is
>>>> +responsible for controlling the state of the IPs that are present.
>>>> +Communication between the host processor running an OS and the system
>>>> +controller happens through a protocol known as TI-SCI [1]. This pm
>>>> domain
>>>> +implementation plugs into the generic pm domain framework and makes use
>>>> of
>>>> +the TI SCI protocol power on and off each device when needed.
>>>> +
>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>>>> +
>>>> +PM Domain Node
>>>> +==============
>>>> +The PM domain node represents the global PM domain managed by the PMMC,
>>>> +which in this case is the single implementation as documented by the
>>>> generic
>>>> +PM domain bindings in
>>>> Documentation/devicetree/bindings/power/power_domain.txt.
>>>> +Because this relies on the TI SCI protocol to communicate with the PMMC
>>>> it
>>>> +must be a child of the pmmc node.
>>>> +
>>>> +Required Properties:
>>>> +--------------------
>>>> +- compatible: should be "ti,sci-pm-domain"
>>>> +- #power-domain-cells: Must be 0.
>>>> +
>>>> +Example (K2G):
>>>> +-------------
>>>> +       pmmc: pmmc {
>>>> +               compatible = "ti,k2g-sci";
>>>> +               ...
>>>> +
>>>> +               k2g_pds: k2g_pds {
>>>> +                       compatible = "ti,sci-pm-domain";
>>>> +                       #power-domain-cells = <0>;
>>>> +               };
>>>> +       };
>>>> +
>>>> +PM Domain Consumers
>>>> +===================
>>>> +Hardware blocks that require SCI control over their state must provide
>>>> +a reference to the sci-pm-domain they are part of and a unique device
>>>> +specific ID that identifies the device.
>>>> +
>>>> +Required Properties:
>>>> +--------------------
>>>> +- power-domains: phandle pointing to the corresponding PM domain node.
>>>> +- ti,sci-id: index representing the device id to be passed oevr SCI to
>>>> +            be used for device control.
>>>
>>>
>>> As I've already stated before, this goes in power-domain cells. When you
>>> have a single thing (i.e. node) that controls multiple things, then you
>>> you need to specify the ID for each of them in phandle args. This is how
>>> irqs, gpio, clocks, *everything* in DT works.
>>
>>
>> You think the reasoning for doing it this way provided by both Ulf and
>> myself on v2 [1] is not valid then?
>>
>> From Ulf:
>>
>> To me, the TI SCI ID, is similar to a "conid" for any another "device
>> resource" (like clock, pinctrl, regulator etc) which we can describe
>> in DT and assign to a device node. The only difference here, is that
>> we don't have common API to fetch the resource (like clk_get(),
>> regulator_get()), but instead we fetches the device's resource from
>> SoC specific code, via genpd's device ->attach() callback.
>
> Sorry, but that sounds like a kernel problem to me and has nothing to
> do with DT bindings.
>
>> From me:
>>
>> Yes, you've pretty much hit it on the head. It is not an index into a list
>> of genpds but rather identifies the device *within* a single genpd. It is a
>> property specific to each device that resides in a ti-sci-genpd, not a
>> mapping describing which genpd the device belongs to. The generic power
>> domain binding is concerned with mapping the device to a specific genpd,
>> which is does fine for us, but we have a sub mapping for devices that exist
>> inside a genpd which, we must describe as well, hence the ti,sci-id.
>>
>>
>> So to summarize, the genpd framework does interpret the phandle arg as an
>> index into multiple genpds, just as you've said other frameworks do, but
>> this is not what I am trying to do, we have multiple devices within this
>> *single* genpd, hence the need for the ti,sci-id property.
>
> Fix the genpd framework rather than work around it in DT.

I still disagree that this has nothing to do with DT bindings, as the 
current DT binding represents something different already. I am trying 
to extend it to give me additional information needed for our platforms. 
Are you saying that we should break what the current DT binding already 
represents to mean something else?

Regards,
Dave

>
> Rob
>

Re: [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains

[Rob Herring]

On Thu, Jan 12, 2017 at 9:27 AM, Dave Gerlach <d-gerlach@ti.com> wrote:
> Rob,
>
> On 01/11/2017 03:34 PM, Rob Herring wrote:
>>
>> On Mon, Jan 9, 2017 at 11:57 AM, Dave Gerlach <d-gerlach@ti.com> wrote:
>>>
>>> Rob,
>>>
>>> On 01/09/2017 11:50 AM, Rob Herring wrote:
>>>>
>>>>
>>>> On Wed, Jan 04, 2017 at 02:55:34PM -0600, Dave Gerlach wrote:
>>>>>
>>>>>
>>>>> Add a generic power domain implementation, TI SCI PM Domains, that
>>>>> will hook into the genpd framework and allow the TI SCI protocol to
>>>>> control device power states.
>>>>>
>>>>> Also, provide macros representing each device index as understood
>>>>> by TI SCI to be used in the device node power-domain references.
>>>>> These are identifiers for the K2G devices managed by the PMMC.
>>>>>
>>>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>>>> ---
>>>>> v2->v3:
>>>>>         Update k2g_pds node docs to show it should be a child of pmmc
>>>>> node.
>>>>>         In early versions a phandle was used to point to pmmc and docs
>>>>> still
>>>>>         incorrectly showed this.
>>>>>
>>>>>  .../devicetree/bindings/soc/ti/sci-pm-domain.txt   | 59 ++++++++++++++
>>>>>  MAINTAINERS                                        |  2 +
>>>>>  include/dt-bindings/genpd/k2g.h                    | 90
>>>>> ++++++++++++++++++++++
>>>>>  3 files changed, 151 insertions(+)
>>>>>  create mode 100644
>>>>> Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>  create mode 100644 include/dt-bindings/genpd/k2g.h
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>> new file mode 100644
>>>>> index 000000000000..4c9064e512cb
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>> @@ -0,0 +1,59 @@
>>>>> +Texas Instruments TI-SCI Generic Power Domain
>>>>> +---------------------------------------------
>>>>> +
>>>>> +Some TI SoCs contain a system controller (like the PMMC, etc...) that
>>>>> is
>>>>> +responsible for controlling the state of the IPs that are present.
>>>>> +Communication between the host processor running an OS and the system
>>>>> +controller happens through a protocol known as TI-SCI [1]. This pm
>>>>> domain
>>>>> +implementation plugs into the generic pm domain framework and makes
>>>>> use
>>>>> of
>>>>> +the TI SCI protocol power on and off each device when needed.
>>>>> +
>>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>>>>> +
>>>>> +PM Domain Node
>>>>> +==============
>>>>> +The PM domain node represents the global PM domain managed by the
>>>>> PMMC,
>>>>> +which in this case is the single implementation as documented by the
>>>>> generic
>>>>> +PM domain bindings in
>>>>> Documentation/devicetree/bindings/power/power_domain.txt.
>>>>> +Because this relies on the TI SCI protocol to communicate with the
>>>>> PMMC
>>>>> it
>>>>> +must be a child of the pmmc node.
>>>>> +
>>>>> +Required Properties:
>>>>> +--------------------
>>>>> +- compatible: should be "ti,sci-pm-domain"
>>>>> +- #power-domain-cells: Must be 0.
>>>>> +
>>>>> +Example (K2G):
>>>>> +-------------
>>>>> +       pmmc: pmmc {
>>>>> +               compatible = "ti,k2g-sci";
>>>>> +               ...
>>>>> +
>>>>> +               k2g_pds: k2g_pds {
>>>>> +                       compatible = "ti,sci-pm-domain";
>>>>> +                       #power-domain-cells = <0>;
>>>>> +               };
>>>>> +       };
>>>>> +
>>>>> +PM Domain Consumers
>>>>> +===================
>>>>> +Hardware blocks that require SCI control over their state must provide
>>>>> +a reference to the sci-pm-domain they are part of and a unique device
>>>>> +specific ID that identifies the device.
>>>>> +
>>>>> +Required Properties:
>>>>> +--------------------
>>>>> +- power-domains: phandle pointing to the corresponding PM domain node.
>>>>> +- ti,sci-id: index representing the device id to be passed oevr SCI to
>>>>> +            be used for device control.
>>>>
>>>>
>>>>
>>>> As I've already stated before, this goes in power-domain cells. When you
>>>> have a single thing (i.e. node) that controls multiple things, then you
>>>> you need to specify the ID for each of them in phandle args. This is how
>>>> irqs, gpio, clocks, *everything* in DT works.
>>>
>>>
>>>
>>> You think the reasoning for doing it this way provided by both Ulf and
>>> myself on v2 [1] is not valid then?
>>>
>>> From Ulf:
>>>
>>> To me, the TI SCI ID, is similar to a "conid" for any another "device
>>> resource" (like clock, pinctrl, regulator etc) which we can describe
>>> in DT and assign to a device node. The only difference here, is that
>>> we don't have common API to fetch the resource (like clk_get(),
>>> regulator_get()), but instead we fetches the device's resource from
>>> SoC specific code, via genpd's device ->attach() callback.
>>
>>
>> Sorry, but that sounds like a kernel problem to me and has nothing to
>> do with DT bindings.
>>
>>> From me:
>>>
>>> Yes, you've pretty much hit it on the head. It is not an index into a
>>> list
>>> of genpds but rather identifies the device *within* a single genpd. It is
>>> a
>>> property specific to each device that resides in a ti-sci-genpd, not a
>>> mapping describing which genpd the device belongs to. The generic power
>>> domain binding is concerned with mapping the device to a specific genpd,
>>> which is does fine for us, but we have a sub mapping for devices that
>>> exist
>>> inside a genpd which, we must describe as well, hence the ti,sci-id.
>>>
>>>
>>> So to summarize, the genpd framework does interpret the phandle arg as an
>>> index into multiple genpds, just as you've said other frameworks do, but
>>> this is not what I am trying to do, we have multiple devices within this
>>> *single* genpd, hence the need for the ti,sci-id property.
>>
>>
>> Fix the genpd framework rather than work around it in DT.
>
>
> I still disagree that this has nothing to do with DT bindings, as the
> current DT binding represents something different already. I am trying to
> extend it to give me additional information needed for our platforms. Are
> you saying that we should break what the current DT binding already
> represents to mean something else?

No idea because what's the current binding? From the patch, looks like
a new binding to me.

Rob

Re: [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains

[Dave Gerlach]

On 01/13/2017 01:25 PM, Rob Herring wrote:
> On Thu, Jan 12, 2017 at 9:27 AM, Dave Gerlach <d-gerlach@ti.com> wrote:
>> Rob,
>>
>> On 01/11/2017 03:34 PM, Rob Herring wrote:
>>>
>>> On Mon, Jan 9, 2017 at 11:57 AM, Dave Gerlach <d-gerlach@ti.com> wrote:
>>>>
>>>> Rob,
>>>>
>>>> On 01/09/2017 11:50 AM, Rob Herring wrote:
>>>>>
>>>>>
>>>>> On Wed, Jan 04, 2017 at 02:55:34PM -0600, Dave Gerlach wrote:
>>>>>>
>>>>>>
>>>>>> Add a generic power domain implementation, TI SCI PM Domains, that
>>>>>> will hook into the genpd framework and allow the TI SCI protocol to
>>>>>> control device power states.
>>>>>>
>>>>>> Also, provide macros representing each device index as understood
>>>>>> by TI SCI to be used in the device node power-domain references.
>>>>>> These are identifiers for the K2G devices managed by the PMMC.
>>>>>>
>>>>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>>>>> ---
>>>>>> v2->v3:
>>>>>>         Update k2g_pds node docs to show it should be a child of pmmc
>>>>>> node.
>>>>>>         In early versions a phandle was used to point to pmmc and docs
>>>>>> still
>>>>>>         incorrectly showed this.
>>>>>>
>>>>>>  .../devicetree/bindings/soc/ti/sci-pm-domain.txt   | 59 ++++++++++++++
>>>>>>  MAINTAINERS                                        |  2 +
>>>>>>  include/dt-bindings/genpd/k2g.h                    | 90
>>>>>> ++++++++++++++++++++++
>>>>>>  3 files changed, 151 insertions(+)
>>>>>>  create mode 100644
>>>>>> Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>  create mode 100644 include/dt-bindings/genpd/k2g.h
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>> new file mode 100644
>>>>>> index 000000000000..4c9064e512cb
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>> @@ -0,0 +1,59 @@
>>>>>> +Texas Instruments TI-SCI Generic Power Domain
>>>>>> +---------------------------------------------
>>>>>> +
>>>>>> +Some TI SoCs contain a system controller (like the PMMC, etc...) that
>>>>>> is
>>>>>> +responsible for controlling the state of the IPs that are present.
>>>>>> +Communication between the host processor running an OS and the system
>>>>>> +controller happens through a protocol known as TI-SCI [1]. This pm
>>>>>> domain
>>>>>> +implementation plugs into the generic pm domain framework and makes
>>>>>> use
>>>>>> of
>>>>>> +the TI SCI protocol power on and off each device when needed.
>>>>>> +
>>>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>>>>>> +
>>>>>> +PM Domain Node
>>>>>> +==============
>>>>>> +The PM domain node represents the global PM domain managed by the
>>>>>> PMMC,
>>>>>> +which in this case is the single implementation as documented by the
>>>>>> generic
>>>>>> +PM domain bindings in
>>>>>> Documentation/devicetree/bindings/power/power_domain.txt.
>>>>>> +Because this relies on the TI SCI protocol to communicate with the
>>>>>> PMMC
>>>>>> it
>>>>>> +must be a child of the pmmc node.
>>>>>> +
>>>>>> +Required Properties:
>>>>>> +--------------------
>>>>>> +- compatible: should be "ti,sci-pm-domain"
>>>>>> +- #power-domain-cells: Must be 0.
>>>>>> +
>>>>>> +Example (K2G):
>>>>>> +-------------
>>>>>> +       pmmc: pmmc {
>>>>>> +               compatible = "ti,k2g-sci";
>>>>>> +               ...
>>>>>> +
>>>>>> +               k2g_pds: k2g_pds {
>>>>>> +                       compatible = "ti,sci-pm-domain";
>>>>>> +                       #power-domain-cells = <0>;
>>>>>> +               };
>>>>>> +       };
>>>>>> +
>>>>>> +PM Domain Consumers
>>>>>> +===================
>>>>>> +Hardware blocks that require SCI control over their state must provide
>>>>>> +a reference to the sci-pm-domain they are part of and a unique device
>>>>>> +specific ID that identifies the device.
>>>>>> +
>>>>>> +Required Properties:
>>>>>> +--------------------
>>>>>> +- power-domains: phandle pointing to the corresponding PM domain node.
>>>>>> +- ti,sci-id: index representing the device id to be passed oevr SCI to
>>>>>> +            be used for device control.
>>>>>
>>>>>
>>>>>
>>>>> As I've already stated before, this goes in power-domain cells. When you
>>>>> have a single thing (i.e. node) that controls multiple things, then you
>>>>> you need to specify the ID for each of them in phandle args. This is how
>>>>> irqs, gpio, clocks, *everything* in DT works.
>>>>
>>>>
>>>>
>>>> You think the reasoning for doing it this way provided by both Ulf and
>>>> myself on v2 [1] is not valid then?
>>>>
>>>> From Ulf:
>>>>
>>>> To me, the TI SCI ID, is similar to a "conid" for any another "device
>>>> resource" (like clock, pinctrl, regulator etc) which we can describe
>>>> in DT and assign to a device node. The only difference here, is that
>>>> we don't have common API to fetch the resource (like clk_get(),
>>>> regulator_get()), but instead we fetches the device's resource from
>>>> SoC specific code, via genpd's device ->attach() callback.
>>>
>>>
>>> Sorry, but that sounds like a kernel problem to me and has nothing to
>>> do with DT bindings.
>>>
>>>> From me:
>>>>
>>>> Yes, you've pretty much hit it on the head. It is not an index into a
>>>> list
>>>> of genpds but rather identifies the device *within* a single genpd. It is
>>>> a
>>>> property specific to each device that resides in a ti-sci-genpd, not a
>>>> mapping describing which genpd the device belongs to. The generic power
>>>> domain binding is concerned with mapping the device to a specific genpd,
>>>> which is does fine for us, but we have a sub mapping for devices that
>>>> exist
>>>> inside a genpd which, we must describe as well, hence the ti,sci-id.
>>>>
>>>>
>>>> So to summarize, the genpd framework does interpret the phandle arg as an
>>>> index into multiple genpds, just as you've said other frameworks do, but
>>>> this is not what I am trying to do, we have multiple devices within this
>>>> *single* genpd, hence the need for the ti,sci-id property.
>>>
>>>
>>> Fix the genpd framework rather than work around it in DT.
>>
>>
>> I still disagree that this has nothing to do with DT bindings, as the
>> current DT binding represents something different already. I am trying to
>> extend it to give me additional information needed for our platforms. Are
>> you saying that we should break what the current DT binding already
>> represents to mean something else?
>
> No idea because what's the current binding? From the patch, looks like
> a new binding to me.

Yes, ti,sci-id is a new binding. I am referring to the current meaning 
of the "power-domains" binding, which is where you are asking this 
property to be added, in "power-domains" cells. This is documented here 
[1] in the kernel, although looking at it I must admit it is not very clear.

The power-domains cell represents an offset into an array of power 
domains, if you choose to use it. That's what the genpd framework is 
hard coded to interpret it as. This is correct, as it is an index into a 
static list of power domains, used to identify which power domain a 
device belongs to, which is exactly what the genpd framework itself is 
concerned with. This is already how it is used in the kernel today.

My ti,sci-id is not an index into a list of power domains, so it should 
not go in the power-domains cells and go against what the power-domains 
binding says that the cell expects. We have one single power domain, and 
the new ti,sci-id binding is not something the genpd framework itself is 
concerned with as it's our property to identify a device inside a power 
domain, not to identify which power domain it is associated with.

Regards,
Dave

[1] Documentation/devicetree/bindings/power/power_domain.txt

>
> Rob
>

Re: [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains

[Rob Herring]

On Fri, Jan 13, 2017 at 2:28 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
> On 01/13/2017 01:25 PM, Rob Herring wrote:
>>
>> On Thu, Jan 12, 2017 at 9:27 AM, Dave Gerlach <d-gerlach@ti.com> wrote:
>>>
>>> Rob,
>>>
>>> On 01/11/2017 03:34 PM, Rob Herring wrote:
>>>>
>>>>
>>>> On Mon, Jan 9, 2017 at 11:57 AM, Dave Gerlach <d-gerlach@ti.com> wrote:
>>>>>
>>>>>
>>>>> Rob,
>>>>>
>>>>> On 01/09/2017 11:50 AM, Rob Herring wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Wed, Jan 04, 2017 at 02:55:34PM -0600, Dave Gerlach wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Add a generic power domain implementation, TI SCI PM Domains, that
>>>>>>> will hook into the genpd framework and allow the TI SCI protocol to
>>>>>>> control device power states.
>>>>>>>
>>>>>>> Also, provide macros representing each device index as understood
>>>>>>> by TI SCI to be used in the device node power-domain references.
>>>>>>> These are identifiers for the K2G devices managed by the PMMC.
>>>>>>>
>>>>>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>>>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>>>>>> ---
>>>>>>> v2->v3:
>>>>>>>         Update k2g_pds node docs to show it should be a child of pmmc
>>>>>>> node.
>>>>>>>         In early versions a phandle was used to point to pmmc and
>>>>>>> docs
>>>>>>> still
>>>>>>>         incorrectly showed this.
>>>>>>>
>>>>>>>  .../devicetree/bindings/soc/ti/sci-pm-domain.txt   | 59
>>>>>>> ++++++++++++++
>>>>>>>  MAINTAINERS                                        |  2 +
>>>>>>>  include/dt-bindings/genpd/k2g.h                    | 90
>>>>>>> ++++++++++++++++++++++
>>>>>>>  3 files changed, 151 insertions(+)
>>>>>>>  create mode 100644
>>>>>>> Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>>  create mode 100644 include/dt-bindings/genpd/k2g.h
>>>>>>>
>>>>>>> diff --git
>>>>>>> a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>> new file mode 100644
>>>>>>> index 000000000000..4c9064e512cb
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>> @@ -0,0 +1,59 @@
>>>>>>> +Texas Instruments TI-SCI Generic Power Domain
>>>>>>> +---------------------------------------------
>>>>>>> +
>>>>>>> +Some TI SoCs contain a system controller (like the PMMC, etc...)
>>>>>>> that
>>>>>>> is
>>>>>>> +responsible for controlling the state of the IPs that are present.
>>>>>>> +Communication between the host processor running an OS and the
>>>>>>> system
>>>>>>> +controller happens through a protocol known as TI-SCI [1]. This pm
>>>>>>> domain
>>>>>>> +implementation plugs into the generic pm domain framework and makes
>>>>>>> use
>>>>>>> of
>>>>>>> +the TI SCI protocol power on and off each device when needed.
>>>>>>> +
>>>>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>>>>>>> +
>>>>>>> +PM Domain Node
>>>>>>> +==============
>>>>>>> +The PM domain node represents the global PM domain managed by the
>>>>>>> PMMC,
>>>>>>> +which in this case is the single implementation as documented by the
>>>>>>> generic
>>>>>>> +PM domain bindings in
>>>>>>> Documentation/devicetree/bindings/power/power_domain.txt.
>>>>>>> +Because this relies on the TI SCI protocol to communicate with the
>>>>>>> PMMC
>>>>>>> it
>>>>>>> +must be a child of the pmmc node.
>>>>>>> +
>>>>>>> +Required Properties:
>>>>>>> +--------------------
>>>>>>> +- compatible: should be "ti,sci-pm-domain"
>>>>>>> +- #power-domain-cells: Must be 0.
>>>>>>> +
>>>>>>> +Example (K2G):
>>>>>>> +-------------
>>>>>>> +       pmmc: pmmc {
>>>>>>> +               compatible = "ti,k2g-sci";
>>>>>>> +               ...
>>>>>>> +
>>>>>>> +               k2g_pds: k2g_pds {
>>>>>>> +                       compatible = "ti,sci-pm-domain";
>>>>>>> +                       #power-domain-cells = <0>;
>>>>>>> +               };
>>>>>>> +       };
>>>>>>> +
>>>>>>> +PM Domain Consumers
>>>>>>> +===================
>>>>>>> +Hardware blocks that require SCI control over their state must
>>>>>>> provide
>>>>>>> +a reference to the sci-pm-domain they are part of and a unique
>>>>>>> device
>>>>>>> +specific ID that identifies the device.
>>>>>>> +
>>>>>>> +Required Properties:
>>>>>>> +--------------------
>>>>>>> +- power-domains: phandle pointing to the corresponding PM domain
>>>>>>> node.
>>>>>>> +- ti,sci-id: index representing the device id to be passed oevr SCI
>>>>>>> to
>>>>>>> +            be used for device control.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> As I've already stated before, this goes in power-domain cells. When
>>>>>> you
>>>>>> have a single thing (i.e. node) that controls multiple things, then
>>>>>> you
>>>>>> you need to specify the ID for each of them in phandle args. This is
>>>>>> how
>>>>>> irqs, gpio, clocks, *everything* in DT works.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> You think the reasoning for doing it this way provided by both Ulf and
>>>>> myself on v2 [1] is not valid then?
>>>>>
>>>>> From Ulf:
>>>>>
>>>>> To me, the TI SCI ID, is similar to a "conid" for any another "device
>>>>> resource" (like clock, pinctrl, regulator etc) which we can describe
>>>>> in DT and assign to a device node. The only difference here, is that
>>>>> we don't have common API to fetch the resource (like clk_get(),
>>>>> regulator_get()), but instead we fetches the device's resource from
>>>>> SoC specific code, via genpd's device ->attach() callback.
>>>>
>>>>
>>>>
>>>> Sorry, but that sounds like a kernel problem to me and has nothing to
>>>> do with DT bindings.
>>>>
>>>>> From me:
>>>>>
>>>>> Yes, you've pretty much hit it on the head. It is not an index into a
>>>>> list
>>>>> of genpds but rather identifies the device *within* a single genpd. It
>>>>> is
>>>>> a
>>>>> property specific to each device that resides in a ti-sci-genpd, not a
>>>>> mapping describing which genpd the device belongs to. The generic power
>>>>> domain binding is concerned with mapping the device to a specific
>>>>> genpd,
>>>>> which is does fine for us, but we have a sub mapping for devices that
>>>>> exist
>>>>> inside a genpd which, we must describe as well, hence the ti,sci-id.
>>>>>
>>>>>
>>>>> So to summarize, the genpd framework does interpret the phandle arg as
>>>>> an
>>>>> index into multiple genpds, just as you've said other frameworks do,
>>>>> but
>>>>> this is not what I am trying to do, we have multiple devices within
>>>>> this
>>>>> *single* genpd, hence the need for the ti,sci-id property.
>>>>
>>>>
>>>>
>>>> Fix the genpd framework rather than work around it in DT.
>>>
>>>
>>>
>>> I still disagree that this has nothing to do with DT bindings, as the
>>> current DT binding represents something different already. I am trying to
>>> extend it to give me additional information needed for our platforms. Are
>>> you saying that we should break what the current DT binding already
>>> represents to mean something else?
>>
>>
>> No idea because what's the current binding? From the patch, looks like
>> a new binding to me.
>
>
> Yes, ti,sci-id is a new binding. I am referring to the current meaning of
> the "power-domains" binding, which is where you are asking this property to
> be added, in "power-domains" cells. This is documented here [1] in the
> kernel, although looking at it I must admit it is not very clear.
>
> The power-domains cell represents an offset into an array of power domains,
> if you choose to use it. That's what the genpd framework is hard coded to
> interpret it as. This is correct, as it is an index into a static list of
> power domains, used to identify which power domain a device belongs to,
> which is exactly what the genpd framework itself is concerned with. This is
> already how it is used in the kernel today.

Strictly speaking, the cells are purely for the interpretation of the
phandle they are associated with. If some controller wants to have 20
cells, then it could assuming a good reason. The reality is we tend to
align the meaning of the cells. If genpd is interpreting the cells and
not letting the driver for the power domain controller interpret them,
then still, genpd needs to be fixed.

IIRC, initially it was said genpd required 0 cells, hence my confusion.

> My ti,sci-id is not an index into a list of power domains, so it should not
> go in the power-domains cells and go against what the power-domains binding
> says that the cell expects. We have one single power domain, and the new
> ti,sci-id binding is not something the genpd framework itself is concerned
> with as it's our property to identify a device inside a power domain, not to
> identify which power domain it is associated with.

What is the id used for? I can understand why you need to know what
power domain a device is in (as power-domains identifies), but not
what devices are in a power domain.

Rob

Re: [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains

[Dave Gerlach]

On 01/13/2017 08:40 PM, Rob Herring wrote:
> On Fri, Jan 13, 2017 at 2:28 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
>> On 01/13/2017 01:25 PM, Rob Herring wrote:
>>>
>>> On Thu, Jan 12, 2017 at 9:27 AM, Dave Gerlach <d-gerlach@ti.com> wrote:
>>>>
>>>> Rob,
>>>>
>>>> On 01/11/2017 03:34 PM, Rob Herring wrote:
>>>>>
>>>>>
>>>>> On Mon, Jan 9, 2017 at 11:57 AM, Dave Gerlach <d-gerlach@ti.com> wrote:
>>>>>>
>>>>>>
>>>>>> Rob,
>>>>>>
>>>>>> On 01/09/2017 11:50 AM, Rob Herring wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Wed, Jan 04, 2017 at 02:55:34PM -0600, Dave Gerlach wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Add a generic power domain implementation, TI SCI PM Domains, that
>>>>>>>> will hook into the genpd framework and allow the TI SCI protocol to
>>>>>>>> control device power states.
>>>>>>>>
>>>>>>>> Also, provide macros representing each device index as understood
>>>>>>>> by TI SCI to be used in the device node power-domain references.
>>>>>>>> These are identifiers for the K2G devices managed by the PMMC.
>>>>>>>>
>>>>>>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>>>>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>>>>>>> ---
>>>>>>>> v2->v3:
>>>>>>>>         Update k2g_pds node docs to show it should be a child of pmmc
>>>>>>>> node.
>>>>>>>>         In early versions a phandle was used to point to pmmc and
>>>>>>>> docs
>>>>>>>> still
>>>>>>>>         incorrectly showed this.
>>>>>>>>
>>>>>>>>  .../devicetree/bindings/soc/ti/sci-pm-domain.txt   | 59
>>>>>>>> ++++++++++++++
>>>>>>>>  MAINTAINERS                                        |  2 +
>>>>>>>>  include/dt-bindings/genpd/k2g.h                    | 90
>>>>>>>> ++++++++++++++++++++++
>>>>>>>>  3 files changed, 151 insertions(+)
>>>>>>>>  create mode 100644
>>>>>>>> Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>>>  create mode 100644 include/dt-bindings/genpd/k2g.h
>>>>>>>>
>>>>>>>> diff --git
>>>>>>>> a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>>> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>>> new file mode 100644
>>>>>>>> index 000000000000..4c9064e512cb
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>>> @@ -0,0 +1,59 @@
>>>>>>>> +Texas Instruments TI-SCI Generic Power Domain
>>>>>>>> +---------------------------------------------
>>>>>>>> +
>>>>>>>> +Some TI SoCs contain a system controller (like the PMMC, etc...)
>>>>>>>> that
>>>>>>>> is
>>>>>>>> +responsible for controlling the state of the IPs that are present.
>>>>>>>> +Communication between the host processor running an OS and the
>>>>>>>> system
>>>>>>>> +controller happens through a protocol known as TI-SCI [1]. This pm
>>>>>>>> domain
>>>>>>>> +implementation plugs into the generic pm domain framework and makes
>>>>>>>> use
>>>>>>>> of
>>>>>>>> +the TI SCI protocol power on and off each device when needed.
>>>>>>>> +
>>>>>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>>>>>>>> +
>>>>>>>> +PM Domain Node
>>>>>>>> +==============
>>>>>>>> +The PM domain node represents the global PM domain managed by the
>>>>>>>> PMMC,
>>>>>>>> +which in this case is the single implementation as documented by the
>>>>>>>> generic
>>>>>>>> +PM domain bindings in
>>>>>>>> Documentation/devicetree/bindings/power/power_domain.txt.
>>>>>>>> +Because this relies on the TI SCI protocol to communicate with the
>>>>>>>> PMMC
>>>>>>>> it
>>>>>>>> +must be a child of the pmmc node.
>>>>>>>> +
>>>>>>>> +Required Properties:
>>>>>>>> +--------------------
>>>>>>>> +- compatible: should be "ti,sci-pm-domain"
>>>>>>>> +- #power-domain-cells: Must be 0.
>>>>>>>> +
>>>>>>>> +Example (K2G):
>>>>>>>> +-------------
>>>>>>>> +       pmmc: pmmc {
>>>>>>>> +               compatible = "ti,k2g-sci";
>>>>>>>> +               ...
>>>>>>>> +
>>>>>>>> +               k2g_pds: k2g_pds {
>>>>>>>> +                       compatible = "ti,sci-pm-domain";
>>>>>>>> +                       #power-domain-cells = <0>;
>>>>>>>> +               };
>>>>>>>> +       };
>>>>>>>> +
>>>>>>>> +PM Domain Consumers
>>>>>>>> +===================
>>>>>>>> +Hardware blocks that require SCI control over their state must
>>>>>>>> provide
>>>>>>>> +a reference to the sci-pm-domain they are part of and a unique
>>>>>>>> device
>>>>>>>> +specific ID that identifies the device.
>>>>>>>> +
>>>>>>>> +Required Properties:
>>>>>>>> +--------------------
>>>>>>>> +- power-domains: phandle pointing to the corresponding PM domain
>>>>>>>> node.
>>>>>>>> +- ti,sci-id: index representing the device id to be passed oevr SCI
>>>>>>>> to
>>>>>>>> +            be used for device control.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> As I've already stated before, this goes in power-domain cells. When
>>>>>>> you
>>>>>>> have a single thing (i.e. node) that controls multiple things, then
>>>>>>> you
>>>>>>> you need to specify the ID for each of them in phandle args. This is
>>>>>>> how
>>>>>>> irqs, gpio, clocks, *everything* in DT works.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> You think the reasoning for doing it this way provided by both Ulf and
>>>>>> myself on v2 [1] is not valid then?
>>>>>>
>>>>>> From Ulf:
>>>>>>
>>>>>> To me, the TI SCI ID, is similar to a "conid" for any another "device
>>>>>> resource" (like clock, pinctrl, regulator etc) which we can describe
>>>>>> in DT and assign to a device node. The only difference here, is that
>>>>>> we don't have common API to fetch the resource (like clk_get(),
>>>>>> regulator_get()), but instead we fetches the device's resource from
>>>>>> SoC specific code, via genpd's device ->attach() callback.
>>>>>
>>>>>
>>>>>
>>>>> Sorry, but that sounds like a kernel problem to me and has nothing to
>>>>> do with DT bindings.
>>>>>
>>>>>> From me:
>>>>>>
>>>>>> Yes, you've pretty much hit it on the head. It is not an index into a
>>>>>> list
>>>>>> of genpds but rather identifies the device *within* a single genpd. It
>>>>>> is
>>>>>> a
>>>>>> property specific to each device that resides in a ti-sci-genpd, not a
>>>>>> mapping describing which genpd the device belongs to. The generic power
>>>>>> domain binding is concerned with mapping the device to a specific
>>>>>> genpd,
>>>>>> which is does fine for us, but we have a sub mapping for devices that
>>>>>> exist
>>>>>> inside a genpd which, we must describe as well, hence the ti,sci-id.
>>>>>>
>>>>>>
>>>>>> So to summarize, the genpd framework does interpret the phandle arg as
>>>>>> an
>>>>>> index into multiple genpds, just as you've said other frameworks do,
>>>>>> but
>>>>>> this is not what I am trying to do, we have multiple devices within
>>>>>> this
>>>>>> *single* genpd, hence the need for the ti,sci-id property.
>>>>>
>>>>>
>>>>>
>>>>> Fix the genpd framework rather than work around it in DT.
>>>>
>>>>
>>>>
>>>> I still disagree that this has nothing to do with DT bindings, as the
>>>> current DT binding represents something different already. I am trying to
>>>> extend it to give me additional information needed for our platforms. Are
>>>> you saying that we should break what the current DT binding already
>>>> represents to mean something else?
>>>
>>>
>>> No idea because what's the current binding? From the patch, looks like
>>> a new binding to me.
>>
>>
>> Yes, ti,sci-id is a new binding. I am referring to the current meaning of
>> the "power-domains" binding, which is where you are asking this property to
>> be added, in "power-domains" cells. This is documented here [1] in the
>> kernel, although looking at it I must admit it is not very clear.
>>
>> The power-domains cell represents an offset into an array of power domains,
>> if you choose to use it. That's what the genpd framework is hard coded to
>> interpret it as. This is correct, as it is an index into a static list of
>> power domains, used to identify which power domain a device belongs to,
>> which is exactly what the genpd framework itself is concerned with. This is
>> already how it is used in the kernel today.
>
> Strictly speaking, the cells are purely for the interpretation of the
> phandle they are associated with. If some controller wants to have 20
> cells, then it could assuming a good reason. The reality is we tend to
> align the meaning of the cells. If genpd is interpreting the cells and
> not letting the driver for the power domain controller interpret them,
> then still, genpd needs to be fixed.

Ok, perhaps the genpd folks on the thread can jump in here with any 
thoughts that they have.

>
> IIRC, initially it was said genpd required 0 cells, hence my confusion.
>
>> My ti,sci-id is not an index into a list of power domains, so it should not
>> go in the power-domains cells and go against what the power-domains binding
>> says that the cell expects. We have one single power domain, and the new
>> ti,sci-id binding is not something the genpd framework itself is concerned
>> with as it's our property to identify a device inside a power domain, not to
>> identify which power domain it is associated with.
>
> What is the id used for? I can understand why you need to know what
> power domain a device is in (as power-domains identifies), but not
> what devices are in a power domain.

We have a system control processor that provides power management 
services to the OS and it responsible for handling the power state of 
each device. This control happens over a communication interface we have 
called TI SCI (implemented at drivers/firmware/ti-sci.c). The 
communication protocol uses these ids to identify each device within the 
power domain so that the control processor can do what is necessary to 
enable that device.

Regards,
Dave

>
> Rob
>

Re: [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains

[Tero Kristo]

On 17/01/17 00:12, Dave Gerlach wrote:
> On 01/13/2017 08:40 PM, Rob Herring wrote:
>> On Fri, Jan 13, 2017 at 2:28 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
>>> On 01/13/2017 01:25 PM, Rob Herring wrote:
>>>>
>>>> On Thu, Jan 12, 2017 at 9:27 AM, Dave Gerlach <d-gerlach@ti.com> wrote:
>>>>>
>>>>> Rob,
>>>>>
>>>>> On 01/11/2017 03:34 PM, Rob Herring wrote:
>>>>>>
>>>>>>
>>>>>> On Mon, Jan 9, 2017 at 11:57 AM, Dave Gerlach <d-gerlach@ti.com>
>>>>>> wrote:
>>>>>>>
>>>>>>>
>>>>>>> Rob,
>>>>>>>
>>>>>>> On 01/09/2017 11:50 AM, Rob Herring wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Wed, Jan 04, 2017 at 02:55:34PM -0600, Dave Gerlach wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Add a generic power domain implementation, TI SCI PM Domains, that
>>>>>>>>> will hook into the genpd framework and allow the TI SCI
>>>>>>>>> protocol to
>>>>>>>>> control device power states.
>>>>>>>>>
>>>>>>>>> Also, provide macros representing each device index as understood
>>>>>>>>> by TI SCI to be used in the device node power-domain references.
>>>>>>>>> These are identifiers for the K2G devices managed by the PMMC.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>>>>>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>>>>>>>> ---
>>>>>>>>> v2->v3:
>>>>>>>>>         Update k2g_pds node docs to show it should be a child
>>>>>>>>> of pmmc
>>>>>>>>> node.
>>>>>>>>>         In early versions a phandle was used to point to pmmc and
>>>>>>>>> docs
>>>>>>>>> still
>>>>>>>>>         incorrectly showed this.
>>>>>>>>>
>>>>>>>>>  .../devicetree/bindings/soc/ti/sci-pm-domain.txt   | 59
>>>>>>>>> ++++++++++++++
>>>>>>>>>  MAINTAINERS                                        |  2 +
>>>>>>>>>  include/dt-bindings/genpd/k2g.h                    | 90
>>>>>>>>> ++++++++++++++++++++++
>>>>>>>>>  3 files changed, 151 insertions(+)
>>>>>>>>>  create mode 100644
>>>>>>>>> Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>>>>  create mode 100644 include/dt-bindings/genpd/k2g.h
>>>>>>>>>
>>>>>>>>> diff --git
>>>>>>>>> a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>>>> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>>>> new file mode 100644
>>>>>>>>> index 000000000000..4c9064e512cb
>>>>>>>>> --- /dev/null
>>>>>>>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>>>> @@ -0,0 +1,59 @@
>>>>>>>>> +Texas Instruments TI-SCI Generic Power Domain
>>>>>>>>> +---------------------------------------------
>>>>>>>>> +
>>>>>>>>> +Some TI SoCs contain a system controller (like the PMMC, etc...)
>>>>>>>>> that
>>>>>>>>> is
>>>>>>>>> +responsible for controlling the state of the IPs that are
>>>>>>>>> present.
>>>>>>>>> +Communication between the host processor running an OS and the
>>>>>>>>> system
>>>>>>>>> +controller happens through a protocol known as TI-SCI [1].
>>>>>>>>> This pm
>>>>>>>>> domain
>>>>>>>>> +implementation plugs into the generic pm domain framework and
>>>>>>>>> makes
>>>>>>>>> use
>>>>>>>>> of
>>>>>>>>> +the TI SCI protocol power on and off each device when needed.
>>>>>>>>> +
>>>>>>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>>>>>>>>> +
>>>>>>>>> +PM Domain Node
>>>>>>>>> +==============
>>>>>>>>> +The PM domain node represents the global PM domain managed by the
>>>>>>>>> PMMC,
>>>>>>>>> +which in this case is the single implementation as documented
>>>>>>>>> by the
>>>>>>>>> generic
>>>>>>>>> +PM domain bindings in
>>>>>>>>> Documentation/devicetree/bindings/power/power_domain.txt.
>>>>>>>>> +Because this relies on the TI SCI protocol to communicate with
>>>>>>>>> the
>>>>>>>>> PMMC
>>>>>>>>> it
>>>>>>>>> +must be a child of the pmmc node.
>>>>>>>>> +
>>>>>>>>> +Required Properties:
>>>>>>>>> +--------------------
>>>>>>>>> +- compatible: should be "ti,sci-pm-domain"
>>>>>>>>> +- #power-domain-cells: Must be 0.
>>>>>>>>> +
>>>>>>>>> +Example (K2G):
>>>>>>>>> +-------------
>>>>>>>>> +       pmmc: pmmc {
>>>>>>>>> +               compatible = "ti,k2g-sci";
>>>>>>>>> +               ...
>>>>>>>>> +
>>>>>>>>> +               k2g_pds: k2g_pds {
>>>>>>>>> +                       compatible = "ti,sci-pm-domain";
>>>>>>>>> +                       #power-domain-cells = <0>;
>>>>>>>>> +               };
>>>>>>>>> +       };
>>>>>>>>> +
>>>>>>>>> +PM Domain Consumers
>>>>>>>>> +===================
>>>>>>>>> +Hardware blocks that require SCI control over their state must
>>>>>>>>> provide
>>>>>>>>> +a reference to the sci-pm-domain they are part of and a unique
>>>>>>>>> device
>>>>>>>>> +specific ID that identifies the device.
>>>>>>>>> +
>>>>>>>>> +Required Properties:
>>>>>>>>> +--------------------
>>>>>>>>> +- power-domains: phandle pointing to the corresponding PM domain
>>>>>>>>> node.
>>>>>>>>> +- ti,sci-id: index representing the device id to be passed
>>>>>>>>> oevr SCI
>>>>>>>>> to
>>>>>>>>> +            be used for device control.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> As I've already stated before, this goes in power-domain cells.
>>>>>>>> When
>>>>>>>> you
>>>>>>>> have a single thing (i.e. node) that controls multiple things, then
>>>>>>>> you
>>>>>>>> you need to specify the ID for each of them in phandle args.
>>>>>>>> This is
>>>>>>>> how
>>>>>>>> irqs, gpio, clocks, *everything* in DT works.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> You think the reasoning for doing it this way provided by both
>>>>>>> Ulf and
>>>>>>> myself on v2 [1] is not valid then?
>>>>>>>
>>>>>>> From Ulf:
>>>>>>>
>>>>>>> To me, the TI SCI ID, is similar to a "conid" for any another
>>>>>>> "device
>>>>>>> resource" (like clock, pinctrl, regulator etc) which we can describe
>>>>>>> in DT and assign to a device node. The only difference here, is that
>>>>>>> we don't have common API to fetch the resource (like clk_get(),
>>>>>>> regulator_get()), but instead we fetches the device's resource from
>>>>>>> SoC specific code, via genpd's device ->attach() callback.
>>>>>>
>>>>>>
>>>>>>
>>>>>> Sorry, but that sounds like a kernel problem to me and has nothing to
>>>>>> do with DT bindings.
>>>>>>
>>>>>>> From me:
>>>>>>>
>>>>>>> Yes, you've pretty much hit it on the head. It is not an index
>>>>>>> into a
>>>>>>> list
>>>>>>> of genpds but rather identifies the device *within* a single
>>>>>>> genpd. It
>>>>>>> is
>>>>>>> a
>>>>>>> property specific to each device that resides in a ti-sci-genpd,
>>>>>>> not a
>>>>>>> mapping describing which genpd the device belongs to. The generic
>>>>>>> power
>>>>>>> domain binding is concerned with mapping the device to a specific
>>>>>>> genpd,
>>>>>>> which is does fine for us, but we have a sub mapping for devices
>>>>>>> that
>>>>>>> exist
>>>>>>> inside a genpd which, we must describe as well, hence the ti,sci-id.
>>>>>>>
>>>>>>>
>>>>>>> So to summarize, the genpd framework does interpret the phandle
>>>>>>> arg as
>>>>>>> an
>>>>>>> index into multiple genpds, just as you've said other frameworks do,
>>>>>>> but
>>>>>>> this is not what I am trying to do, we have multiple devices within
>>>>>>> this
>>>>>>> *single* genpd, hence the need for the ti,sci-id property.
>>>>>>
>>>>>>
>>>>>>
>>>>>> Fix the genpd framework rather than work around it in DT.
>>>>>
>>>>>
>>>>>
>>>>> I still disagree that this has nothing to do with DT bindings, as the
>>>>> current DT binding represents something different already. I am
>>>>> trying to
>>>>> extend it to give me additional information needed for our
>>>>> platforms. Are
>>>>> you saying that we should break what the current DT binding already
>>>>> represents to mean something else?
>>>>
>>>>
>>>> No idea because what's the current binding? From the patch, looks like
>>>> a new binding to me.
>>>
>>>
>>> Yes, ti,sci-id is a new binding. I am referring to the current
>>> meaning of
>>> the "power-domains" binding, which is where you are asking this
>>> property to
>>> be added, in "power-domains" cells. This is documented here [1] in the
>>> kernel, although looking at it I must admit it is not very clear.
>>>
>>> The power-domains cell represents an offset into an array of power
>>> domains,
>>> if you choose to use it. That's what the genpd framework is hard
>>> coded to
>>> interpret it as. This is correct, as it is an index into a static
>>> list of
>>> power domains, used to identify which power domain a device belongs to,
>>> which is exactly what the genpd framework itself is concerned with.
>>> This is
>>> already how it is used in the kernel today.
>>
>> Strictly speaking, the cells are purely for the interpretation of the
>> phandle they are associated with. If some controller wants to have 20
>> cells, then it could assuming a good reason. The reality is we tend to
>> align the meaning of the cells. If genpd is interpreting the cells and
>> not letting the driver for the power domain controller interpret them,
>> then still, genpd needs to be fixed.
>
> Ok, perhaps the genpd folks on the thread can jump in here with any
> thoughts that they have.
>
>>
>> IIRC, initially it was said genpd required 0 cells, hence my confusion.
>>
>>> My ti,sci-id is not an index into a list of power domains, so it
>>> should not
>>> go in the power-domains cells and go against what the power-domains
>>> binding
>>> says that the cell expects. We have one single power domain, and the new
>>> ti,sci-id binding is not something the genpd framework itself is
>>> concerned
>>> with as it's our property to identify a device inside a power domain,
>>> not to
>>> identify which power domain it is associated with.
>>
>> What is the id used for? I can understand why you need to know what
>> power domain a device is in (as power-domains identifies), but not
>> what devices are in a power domain.
>
> We have a system control processor that provides power management
> services to the OS and it responsible for handling the power state of
> each device. This control happens over a communication interface we have
> called TI SCI (implemented at drivers/firmware/ti-sci.c). The
> communication protocol uses these ids to identify each device within the
> power domain so that the control processor can do what is necessary to
> enable that device.

I think a minor detail here that Rob might be missing right now is, that 
the ti,sci-id is only controlling the PM runtime handling, and providing 
the ID per-device for this purpose only. AFAIK, it is not really 
connected to the power domain anymore as such, as we don't have 
power-domains / per device anymore as was the case in some earlier 
revision of this work.

One could argue though that the whole usage of power-domains is now 
moot, as we basically only have implemented one genpd in the whole SoC, 
which doesn't really reflect the reality. I wonder if better approach 
would be to have this replaced with proper power domains at some point 
(if needed), and just have a runtime-pm implementation in place for the 
devices that require it.

So, as an example in DT, we would only have:

uart0: serial@02530c00 {
   compatible = "xyz";
   ...
   ti,sci-id = <K2G_DEV_UART0>;
};

This is somewhat analogous to what OMAP family of SoCs have in place 
now, under "ti,hwmods" property. I also wonder if the "ti,sci-id" should 
be replaced with something like "ti,sci-dev-id" to make its purpose clearer.

-Tero

>
> Regards,
> Dave
>
>>
>> Rob
>>
>

Re: [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains

[Rob Herring]

On Tue, Jan 17, 2017 at 1:48 AM, Tero Kristo <t-kristo@ti.com> wrote:
> On 17/01/17 00:12, Dave Gerlach wrote:
>>
>> On 01/13/2017 08:40 PM, Rob Herring wrote:
>>>
>>> On Fri, Jan 13, 2017 at 2:28 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
>>>>
>>>> On 01/13/2017 01:25 PM, Rob Herring wrote:
>>>>>
>>>>>
>>>>> On Thu, Jan 12, 2017 at 9:27 AM, Dave Gerlach <d-gerlach@ti.com> wrote:
>>>>>>
>>>>>>
>>>>>> Rob,
>>>>>>
>>>>>> On 01/11/2017 03:34 PM, Rob Herring wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Mon, Jan 9, 2017 at 11:57 AM, Dave Gerlach <d-gerlach@ti.com>
>>>>>>> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Rob,
>>>>>>>>
>>>>>>>> On 01/09/2017 11:50 AM, Rob Herring wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Wed, Jan 04, 2017 at 02:55:34PM -0600, Dave Gerlach wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Add a generic power domain implementation, TI SCI PM Domains, that
>>>>>>>>>> will hook into the genpd framework and allow the TI SCI
>>>>>>>>>> protocol to
>>>>>>>>>> control device power states.
>>>>>>>>>>
>>>>>>>>>> Also, provide macros representing each device index as understood
>>>>>>>>>> by TI SCI to be used in the device node power-domain references.
>>>>>>>>>> These are identifiers for the K2G devices managed by the PMMC.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>>>>>>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>>>>>>>>> ---
>>>>>>>>>> v2->v3:
>>>>>>>>>>         Update k2g_pds node docs to show it should be a child
>>>>>>>>>> of pmmc
>>>>>>>>>> node.
>>>>>>>>>>         In early versions a phandle was used to point to pmmc and
>>>>>>>>>> docs
>>>>>>>>>> still
>>>>>>>>>>         incorrectly showed this.
>>>>>>>>>>
>>>>>>>>>>  .../devicetree/bindings/soc/ti/sci-pm-domain.txt   | 59
>>>>>>>>>> ++++++++++++++
>>>>>>>>>>  MAINTAINERS                                        |  2 +
>>>>>>>>>>  include/dt-bindings/genpd/k2g.h                    | 90
>>>>>>>>>> ++++++++++++++++++++++
>>>>>>>>>>  3 files changed, 151 insertions(+)
>>>>>>>>>>  create mode 100644
>>>>>>>>>> Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>>>>>  create mode 100644 include/dt-bindings/genpd/k2g.h
>>>>>>>>>>
>>>>>>>>>> diff --git
>>>>>>>>>> a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>>>>> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>>>>> new file mode 100644
>>>>>>>>>> index 000000000000..4c9064e512cb
>>>>>>>>>> --- /dev/null
>>>>>>>>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>>>>> @@ -0,0 +1,59 @@
>>>>>>>>>> +Texas Instruments TI-SCI Generic Power Domain
>>>>>>>>>> +---------------------------------------------
>>>>>>>>>> +
>>>>>>>>>> +Some TI SoCs contain a system controller (like the PMMC, etc...)
>>>>>>>>>> that
>>>>>>>>>> is
>>>>>>>>>> +responsible for controlling the state of the IPs that are
>>>>>>>>>> present.
>>>>>>>>>> +Communication between the host processor running an OS and the
>>>>>>>>>> system
>>>>>>>>>> +controller happens through a protocol known as TI-SCI [1].
>>>>>>>>>> This pm
>>>>>>>>>> domain
>>>>>>>>>> +implementation plugs into the generic pm domain framework and
>>>>>>>>>> makes
>>>>>>>>>> use
>>>>>>>>>> of
>>>>>>>>>> +the TI SCI protocol power on and off each device when needed.
>>>>>>>>>> +
>>>>>>>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>>>>>>>>>> +
>>>>>>>>>> +PM Domain Node
>>>>>>>>>> +==============
>>>>>>>>>> +The PM domain node represents the global PM domain managed by the
>>>>>>>>>> PMMC,
>>>>>>>>>> +which in this case is the single implementation as documented
>>>>>>>>>> by the
>>>>>>>>>> generic
>>>>>>>>>> +PM domain bindings in
>>>>>>>>>> Documentation/devicetree/bindings/power/power_domain.txt.
>>>>>>>>>> +Because this relies on the TI SCI protocol to communicate with
>>>>>>>>>> the
>>>>>>>>>> PMMC
>>>>>>>>>> it
>>>>>>>>>> +must be a child of the pmmc node.
>>>>>>>>>> +
>>>>>>>>>> +Required Properties:
>>>>>>>>>> +--------------------
>>>>>>>>>> +- compatible: should be "ti,sci-pm-domain"
>>>>>>>>>> +- #power-domain-cells: Must be 0.
>>>>>>>>>> +
>>>>>>>>>> +Example (K2G):
>>>>>>>>>> +-------------
>>>>>>>>>> +       pmmc: pmmc {
>>>>>>>>>> +               compatible = "ti,k2g-sci";
>>>>>>>>>> +               ...
>>>>>>>>>> +
>>>>>>>>>> +               k2g_pds: k2g_pds {
>>>>>>>>>> +                       compatible = "ti,sci-pm-domain";
>>>>>>>>>> +                       #power-domain-cells = <0>;
>>>>>>>>>> +               };
>>>>>>>>>> +       };
>>>>>>>>>> +
>>>>>>>>>> +PM Domain Consumers
>>>>>>>>>> +===================
>>>>>>>>>> +Hardware blocks that require SCI control over their state must
>>>>>>>>>> provide
>>>>>>>>>> +a reference to the sci-pm-domain they are part of and a unique
>>>>>>>>>> device
>>>>>>>>>> +specific ID that identifies the device.
>>>>>>>>>> +
>>>>>>>>>> +Required Properties:
>>>>>>>>>> +--------------------
>>>>>>>>>> +- power-domains: phandle pointing to the corresponding PM domain
>>>>>>>>>> node.
>>>>>>>>>> +- ti,sci-id: index representing the device id to be passed
>>>>>>>>>> oevr SCI
>>>>>>>>>> to
>>>>>>>>>> +            be used for device control.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> As I've already stated before, this goes in power-domain cells.
>>>>>>>>> When
>>>>>>>>> you
>>>>>>>>> have a single thing (i.e. node) that controls multiple things, then
>>>>>>>>> you
>>>>>>>>> you need to specify the ID for each of them in phandle args.
>>>>>>>>> This is
>>>>>>>>> how
>>>>>>>>> irqs, gpio, clocks, *everything* in DT works.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> You think the reasoning for doing it this way provided by both
>>>>>>>> Ulf and
>>>>>>>> myself on v2 [1] is not valid then?
>>>>>>>>
>>>>>>>> From Ulf:
>>>>>>>>
>>>>>>>> To me, the TI SCI ID, is similar to a "conid" for any another
>>>>>>>> "device
>>>>>>>> resource" (like clock, pinctrl, regulator etc) which we can describe
>>>>>>>> in DT and assign to a device node. The only difference here, is that
>>>>>>>> we don't have common API to fetch the resource (like clk_get(),
>>>>>>>> regulator_get()), but instead we fetches the device's resource from
>>>>>>>> SoC specific code, via genpd's device ->attach() callback.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Sorry, but that sounds like a kernel problem to me and has nothing to
>>>>>>> do with DT bindings.
>>>>>>>
>>>>>>>> From me:
>>>>>>>>
>>>>>>>> Yes, you've pretty much hit it on the head. It is not an index
>>>>>>>> into a
>>>>>>>> list
>>>>>>>> of genpds but rather identifies the device *within* a single
>>>>>>>> genpd. It
>>>>>>>> is
>>>>>>>> a
>>>>>>>> property specific to each device that resides in a ti-sci-genpd,
>>>>>>>> not a
>>>>>>>> mapping describing which genpd the device belongs to. The generic
>>>>>>>> power
>>>>>>>> domain binding is concerned with mapping the device to a specific
>>>>>>>> genpd,
>>>>>>>> which is does fine for us, but we have a sub mapping for devices
>>>>>>>> that
>>>>>>>> exist
>>>>>>>> inside a genpd which, we must describe as well, hence the ti,sci-id.
>>>>>>>>
>>>>>>>>
>>>>>>>> So to summarize, the genpd framework does interpret the phandle
>>>>>>>> arg as
>>>>>>>> an
>>>>>>>> index into multiple genpds, just as you've said other frameworks do,
>>>>>>>> but
>>>>>>>> this is not what I am trying to do, we have multiple devices within
>>>>>>>> this
>>>>>>>> *single* genpd, hence the need for the ti,sci-id property.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Fix the genpd framework rather than work around it in DT.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> I still disagree that this has nothing to do with DT bindings, as the
>>>>>> current DT binding represents something different already. I am
>>>>>> trying to
>>>>>> extend it to give me additional information needed for our
>>>>>> platforms. Are
>>>>>> you saying that we should break what the current DT binding already
>>>>>> represents to mean something else?
>>>>>
>>>>>
>>>>>
>>>>> No idea because what's the current binding? From the patch, looks like
>>>>> a new binding to me.
>>>>
>>>>
>>>>
>>>> Yes, ti,sci-id is a new binding. I am referring to the current
>>>> meaning of
>>>> the "power-domains" binding, which is where you are asking this
>>>> property to
>>>> be added, in "power-domains" cells. This is documented here [1] in the
>>>> kernel, although looking at it I must admit it is not very clear.
>>>>
>>>> The power-domains cell represents an offset into an array of power
>>>> domains,
>>>> if you choose to use it. That's what the genpd framework is hard
>>>> coded to
>>>> interpret it as. This is correct, as it is an index into a static
>>>> list of
>>>> power domains, used to identify which power domain a device belongs to,
>>>> which is exactly what the genpd framework itself is concerned with.
>>>> This is
>>>> already how it is used in the kernel today.
>>>
>>>
>>> Strictly speaking, the cells are purely for the interpretation of the
>>> phandle they are associated with. If some controller wants to have 20
>>> cells, then it could assuming a good reason. The reality is we tend to
>>> align the meaning of the cells. If genpd is interpreting the cells and
>>> not letting the driver for the power domain controller interpret them,
>>> then still, genpd needs to be fixed.
>>
>>
>> Ok, perhaps the genpd folks on the thread can jump in here with any
>> thoughts that they have.
>>
>>>
>>> IIRC, initially it was said genpd required 0 cells, hence my confusion.
>>>
>>>> My ti,sci-id is not an index into a list of power domains, so it
>>>> should not
>>>> go in the power-domains cells and go against what the power-domains
>>>> binding
>>>> says that the cell expects. We have one single power domain, and the new
>>>> ti,sci-id binding is not something the genpd framework itself is
>>>> concerned
>>>> with as it's our property to identify a device inside a power domain,
>>>> not to
>>>> identify which power domain it is associated with.
>>>
>>>
>>> What is the id used for? I can understand why you need to know what
>>> power domain a device is in (as power-domains identifies), but not
>>> what devices are in a power domain.
>>
>>
>> We have a system control processor that provides power management
>> services to the OS and it responsible for handling the power state of
>> each device. This control happens over a communication interface we have
>> called TI SCI (implemented at drivers/firmware/ti-sci.c). The
>> communication protocol uses these ids to identify each device within the
>> power domain so that the control processor can do what is necessary to
>> enable that device.
>
>
> I think a minor detail here that Rob might be missing right now is, that the
> ti,sci-id is only controlling the PM runtime handling, and providing the ID
> per-device for this purpose only. AFAIK, it is not really connected to the
> power domain anymore as such, as we don't have power-domains / per device
> anymore as was the case in some earlier revision of this work.

So you used to have multiple power domains and now you don't? Did the
h/w change?

> One could argue though that the whole usage of power-domains is now moot, as
> we basically only have implemented one genpd in the whole SoC, which doesn't
> really reflect the reality. I wonder if better approach would be to have
> this replaced with proper power domains at some point (if needed), and just
> have a runtime-pm implementation in place for the devices that require it.

We're talking about bindings here. Any explanation in terms of
runtime-pm and genpd a) goes over my head and b) isn't relevant to
describing the hardware.

I'm still confused with how many power domains (as defined by the h/w
design) there are controlled by the SCI? Is it 1 or multiple? If 1,
then why do you need the sci-id? For other things that are not the
power domain?

Or perhaps the SCI abstracts things such that you don't really know
what the relationship between devices and power domains is? IOW, you
can't tell from the SCI interface how many power domains there are.

>
> So, as an example in DT, we would only have:
>
> uart0: serial@02530c00 {
>   compatible = "xyz";
>   ...
>   ti,sci-id = <K2G_DEV_UART0>;
> };
>
> This is somewhat analogous to what OMAP family of SoCs have in place now,
> under "ti,hwmods" property. I also wonder if the "ti,sci-id" should be
> replaced with something like "ti,sci-dev-id" to make its purpose clearer.

Describing in terms of hwmods doesn't help me either. Never understood
that either. Sorry.

Rob

Re: [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains

[Kevin Hilman]

Tero Kristo <t-kristo@ti.com> writes:

> On 17/01/17 00:12, Dave Gerlach wrote:
>> On 01/13/2017 08:40 PM, Rob Herring wrote:
>>> On Fri, Jan 13, 2017 at 2:28 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
>>>> On 01/13/2017 01:25 PM, Rob Herring wrote:
>>>>>
>>>>> On Thu, Jan 12, 2017 at 9:27 AM, Dave Gerlach <d-gerlach@ti.com> wrote:
>>>>>>
>>>>>> Rob,
>>>>>>
>>>>>> On 01/11/2017 03:34 PM, Rob Herring wrote:
>>>>>>>
>>>>>>>
>>>>>>> On Mon, Jan 9, 2017 at 11:57 AM, Dave Gerlach <d-gerlach@ti.com>
>>>>>>> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> Rob,
>>>>>>>>
>>>>>>>> On 01/09/2017 11:50 AM, Rob Herring wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Wed, Jan 04, 2017 at 02:55:34PM -0600, Dave Gerlach wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Add a generic power domain implementation, TI SCI PM Domains, that
>>>>>>>>>> will hook into the genpd framework and allow the TI SCI
>>>>>>>>>> protocol to
>>>>>>>>>> control device power states.
>>>>>>>>>>
>>>>>>>>>> Also, provide macros representing each device index as understood
>>>>>>>>>> by TI SCI to be used in the device node power-domain references.
>>>>>>>>>> These are identifiers for the K2G devices managed by the PMMC.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>>>>>>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>>>>>>>>> ---
>>>>>>>>>> v2->v3:
>>>>>>>>>>         Update k2g_pds node docs to show it should be a child
>>>>>>>>>> of pmmc
>>>>>>>>>> node.
>>>>>>>>>>         In early versions a phandle was used to point to pmmc and
>>>>>>>>>> docs
>>>>>>>>>> still
>>>>>>>>>>         incorrectly showed this.
>>>>>>>>>>
>>>>>>>>>>  .../devicetree/bindings/soc/ti/sci-pm-domain.txt   | 59
>>>>>>>>>> ++++++++++++++
>>>>>>>>>>  MAINTAINERS                                        |  2 +
>>>>>>>>>>  include/dt-bindings/genpd/k2g.h                    | 90
>>>>>>>>>> ++++++++++++++++++++++
>>>>>>>>>>  3 files changed, 151 insertions(+)
>>>>>>>>>>  create mode 100644
>>>>>>>>>> Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>>>>>  create mode 100644 include/dt-bindings/genpd/k2g.h
>>>>>>>>>>
>>>>>>>>>> diff --git
>>>>>>>>>> a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>>>>> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>>>>> new file mode 100644
>>>>>>>>>> index 000000000000..4c9064e512cb
>>>>>>>>>> --- /dev/null
>>>>>>>>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>>>>> @@ -0,0 +1,59 @@
>>>>>>>>>> +Texas Instruments TI-SCI Generic Power Domain
>>>>>>>>>> +---------------------------------------------
>>>>>>>>>> +
>>>>>>>>>> +Some TI SoCs contain a system controller (like the PMMC, etc...)
>>>>>>>>>> that
>>>>>>>>>> is
>>>>>>>>>> +responsible for controlling the state of the IPs that are
>>>>>>>>>> present.
>>>>>>>>>> +Communication between the host processor running an OS and the
>>>>>>>>>> system
>>>>>>>>>> +controller happens through a protocol known as TI-SCI [1].
>>>>>>>>>> This pm
>>>>>>>>>> domain
>>>>>>>>>> +implementation plugs into the generic pm domain framework and
>>>>>>>>>> makes
>>>>>>>>>> use
>>>>>>>>>> of
>>>>>>>>>> +the TI SCI protocol power on and off each device when needed.
>>>>>>>>>> +
>>>>>>>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>>>>>>>>>> +
>>>>>>>>>> +PM Domain Node
>>>>>>>>>> +==============
>>>>>>>>>> +The PM domain node represents the global PM domain managed by the
>>>>>>>>>> PMMC,
>>>>>>>>>> +which in this case is the single implementation as documented
>>>>>>>>>> by the
>>>>>>>>>> generic
>>>>>>>>>> +PM domain bindings in
>>>>>>>>>> Documentation/devicetree/bindings/power/power_domain.txt.
>>>>>>>>>> +Because this relies on the TI SCI protocol to communicate with
>>>>>>>>>> the
>>>>>>>>>> PMMC
>>>>>>>>>> it
>>>>>>>>>> +must be a child of the pmmc node.
>>>>>>>>>> +
>>>>>>>>>> +Required Properties:
>>>>>>>>>> +--------------------
>>>>>>>>>> +- compatible: should be "ti,sci-pm-domain"
>>>>>>>>>> +- #power-domain-cells: Must be 0.
>>>>>>>>>> +
>>>>>>>>>> +Example (K2G):
>>>>>>>>>> +-------------
>>>>>>>>>> +       pmmc: pmmc {
>>>>>>>>>> +               compatible = "ti,k2g-sci";
>>>>>>>>>> +               ...
>>>>>>>>>> +
>>>>>>>>>> +               k2g_pds: k2g_pds {
>>>>>>>>>> +                       compatible = "ti,sci-pm-domain";
>>>>>>>>>> +                       #power-domain-cells = <0>;
>>>>>>>>>> +               };
>>>>>>>>>> +       };
>>>>>>>>>> +
>>>>>>>>>> +PM Domain Consumers
>>>>>>>>>> +===================
>>>>>>>>>> +Hardware blocks that require SCI control over their state must
>>>>>>>>>> provide
>>>>>>>>>> +a reference to the sci-pm-domain they are part of and a unique
>>>>>>>>>> device
>>>>>>>>>> +specific ID that identifies the device.
>>>>>>>>>> +
>>>>>>>>>> +Required Properties:
>>>>>>>>>> +--------------------
>>>>>>>>>> +- power-domains: phandle pointing to the corresponding PM domain
>>>>>>>>>> node.
>>>>>>>>>> +- ti,sci-id: index representing the device id to be passed
>>>>>>>>>> oevr SCI
>>>>>>>>>> to
>>>>>>>>>> +            be used for device control.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> As I've already stated before, this goes in power-domain cells.
>>>>>>>>> When
>>>>>>>>> you
>>>>>>>>> have a single thing (i.e. node) that controls multiple things, then
>>>>>>>>> you
>>>>>>>>> you need to specify the ID for each of them in phandle args.
>>>>>>>>> This is
>>>>>>>>> how
>>>>>>>>> irqs, gpio, clocks, *everything* in DT works.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> You think the reasoning for doing it this way provided by both
>>>>>>>> Ulf and
>>>>>>>> myself on v2 [1] is not valid then?
>>>>>>>>
>>>>>>>> From Ulf:
>>>>>>>>
>>>>>>>> To me, the TI SCI ID, is similar to a "conid" for any another
>>>>>>>> "device
>>>>>>>> resource" (like clock, pinctrl, regulator etc) which we can describe
>>>>>>>> in DT and assign to a device node. The only difference here, is that
>>>>>>>> we don't have common API to fetch the resource (like clk_get(),
>>>>>>>> regulator_get()), but instead we fetches the device's resource from
>>>>>>>> SoC specific code, via genpd's device ->attach() callback.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Sorry, but that sounds like a kernel problem to me and has nothing to
>>>>>>> do with DT bindings.
>>>>>>>
>>>>>>>> From me:
>>>>>>>>
>>>>>>>> Yes, you've pretty much hit it on the head. It is not an index
>>>>>>>> into a
>>>>>>>> list
>>>>>>>> of genpds but rather identifies the device *within* a single
>>>>>>>> genpd. It
>>>>>>>> is
>>>>>>>> a
>>>>>>>> property specific to each device that resides in a ti-sci-genpd,
>>>>>>>> not a
>>>>>>>> mapping describing which genpd the device belongs to. The generic
>>>>>>>> power
>>>>>>>> domain binding is concerned with mapping the device to a specific
>>>>>>>> genpd,
>>>>>>>> which is does fine for us, but we have a sub mapping for devices
>>>>>>>> that
>>>>>>>> exist
>>>>>>>> inside a genpd which, we must describe as well, hence the ti,sci-id.
>>>>>>>>
>>>>>>>>
>>>>>>>> So to summarize, the genpd framework does interpret the phandle
>>>>>>>> arg as
>>>>>>>> an
>>>>>>>> index into multiple genpds, just as you've said other frameworks do,
>>>>>>>> but
>>>>>>>> this is not what I am trying to do, we have multiple devices within
>>>>>>>> this
>>>>>>>> *single* genpd, hence the need for the ti,sci-id property.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Fix the genpd framework rather than work around it in DT.
>>>>>>
>>>>>>
>>>>>>
>>>>>> I still disagree that this has nothing to do with DT bindings, as the
>>>>>> current DT binding represents something different already. I am
>>>>>> trying to
>>>>>> extend it to give me additional information needed for our
>>>>>> platforms. Are
>>>>>> you saying that we should break what the current DT binding already
>>>>>> represents to mean something else?
>>>>>
>>>>>
>>>>> No idea because what's the current binding? From the patch, looks like
>>>>> a new binding to me.
>>>>
>>>>
>>>> Yes, ti,sci-id is a new binding. I am referring to the current
>>>> meaning of
>>>> the "power-domains" binding, which is where you are asking this
>>>> property to
>>>> be added, in "power-domains" cells. This is documented here [1] in the
>>>> kernel, although looking at it I must admit it is not very clear.
>>>>
>>>> The power-domains cell represents an offset into an array of power
>>>> domains,
>>>> if you choose to use it. That's what the genpd framework is hard
>>>> coded to
>>>> interpret it as. This is correct, as it is an index into a static
>>>> list of
>>>> power domains, used to identify which power domain a device belongs to,
>>>> which is exactly what the genpd framework itself is concerned with.
>>>> This is
>>>> already how it is used in the kernel today.
>>>
>>> Strictly speaking, the cells are purely for the interpretation of the
>>> phandle they are associated with. If some controller wants to have 20
>>> cells, then it could assuming a good reason. The reality is we tend to
>>> align the meaning of the cells. If genpd is interpreting the cells and
>>> not letting the driver for the power domain controller interpret them,
>>> then still, genpd needs to be fixed.
>>
>> Ok, perhaps the genpd folks on the thread can jump in here with any
>> thoughts that they have.
>>
>>>
>>> IIRC, initially it was said genpd required 0 cells, hence my confusion.
>>>
>>>> My ti,sci-id is not an index into a list of power domains, so it
>>>> should not
>>>> go in the power-domains cells and go against what the power-domains
>>>> binding
>>>> says that the cell expects. We have one single power domain, and the new
>>>> ti,sci-id binding is not something the genpd framework itself is
>>>> concerned
>>>> with as it's our property to identify a device inside a power domain,
>>>> not to
>>>> identify which power domain it is associated with.
>>>
>>> What is the id used for? I can understand why you need to know what
>>> power domain a device is in (as power-domains identifies), but not
>>> what devices are in a power domain.
>>
>> We have a system control processor that provides power management
>> services to the OS and it responsible for handling the power state of
>> each device. This control happens over a communication interface we have
>> called TI SCI (implemented at drivers/firmware/ti-sci.c). The
>> communication protocol uses these ids to identify each device within the
>> power domain so that the control processor can do what is necessary to
>> enable that device.
>
> I think a minor detail here that Rob might be missing right now is,
> that the ti,sci-id is only controlling the PM runtime handling, and
> providing the ID per-device for this purpose only. AFAIK, it is not
> really connected to the power domain anymore as such, as we don't have
> power-domains / per device anymore as was the case in some earlier
> revision of this work.

I think this gets to the heart of things.  IMO The confusion arises
because we're throwing around the term "power domain" when there isn't
an actual hardware power domain here.

Unfortunately, the genpd bindings have used the terminology power-domain
when in fact genpd is more generic than that and can be used not just
for hardware power domains, but for arbitrary grouping of devices that
have common PM properties.  That's why genpd actually stands for generic
PM domain, not power domain.  Unfortunately, the bindings have grown
primarily out of the usage for hardware power domains.

> One could argue though that the whole usage of power-domains is now
> moot, as we basically only have implemented one genpd in the whole
> SoC, which doesn't really reflect the reality. I wonder if better
> approach would be to have this replaced with proper power domains at
> some point (if needed), and just have a runtime-pm implementation in
> place for the devices that require it.
>
> So, as an example in DT, we would only have:
>
> uart0: serial@02530c00 {
>   compatible = "xyz";
>   ...
>   ti,sci-id = <K2G_DEV_UART0>;
> };
>
> This is somewhat analogous to what OMAP family of SoCs have in place
> now, under "ti,hwmods" property. I also wonder if the "ti,sci-id"
> should be replaced with something like "ti,sci-dev-id" to make its
> purpose clearer.

Unless I'm missing something, that still begs the question of who reads
that property and takes care of the call into TI-SCI though.

Kevin

Re: [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains

[Rob Herring]

On Tue, Jan 17, 2017 at 6:07 PM, Kevin Hilman <khilman@baylibre.com> wrote:
> Tero Kristo <t-kristo@ti.com> writes:
>> On 17/01/17 00:12, Dave Gerlach wrote:
>>> On 01/13/2017 08:40 PM, Rob Herring wrote:
>>>> On Fri, Jan 13, 2017 at 2:28 PM, Dave Gerlach <d-gerlach@ti.com> wrote:

[...]

>>>>> My ti,sci-id is not an index into a list of power domains, so it
>>>>> should not
>>>>> go in the power-domains cells and go against what the power-domains
>>>>> binding
>>>>> says that the cell expects. We have one single power domain, and the new
>>>>> ti,sci-id binding is not something the genpd framework itself is
>>>>> concerned
>>>>> with as it's our property to identify a device inside a power domain,
>>>>> not to
>>>>> identify which power domain it is associated with.
>>>>
>>>> What is the id used for? I can understand why you need to know what
>>>> power domain a device is in (as power-domains identifies), but not
>>>> what devices are in a power domain.
>>>
>>> We have a system control processor that provides power management
>>> services to the OS and it responsible for handling the power state of
>>> each device. This control happens over a communication interface we have
>>> called TI SCI (implemented at drivers/firmware/ti-sci.c). The
>>> communication protocol uses these ids to identify each device within the
>>> power domain so that the control processor can do what is necessary to
>>> enable that device.
>>
>> I think a minor detail here that Rob might be missing right now is,
>> that the ti,sci-id is only controlling the PM runtime handling, and
>> providing the ID per-device for this purpose only. AFAIK, it is not
>> really connected to the power domain anymore as such, as we don't have
>> power-domains / per device anymore as was the case in some earlier
>> revision of this work.
>
> I think this gets to the heart of things.  IMO The confusion arises
> because we're throwing around the term "power domain" when there isn't
> an actual hardware power domain here.

I thought there was 1.

> Unfortunately, the genpd bindings have used the terminology power-domain
> when in fact genpd is more generic than that and can be used not just
> for hardware power domains, but for arbitrary grouping of devices that
> have common PM properties.  That's why genpd actually stands for generic
> PM domain, not power domain.  Unfortunately, the bindings have grown
> primarily out of the usage for hardware power domains.

Now it makes some sense.

So the question is does this PM domain grouping need to be described
in DT or not, and if so what does that look like?

We could continue to use the power domain binding (maybe we already
are and that ship has sailed). I'm not totally against the idea even
if there is no power domain, but I'm not sold on it either. If we do
go this route, then I still say the id should be a cell in the
power-domain phandle.

Another option is create something new either common or TI SCI
specific. It could be just a table of ids and phandles in the SCI
node. I'm much more comfortable with an isolated property in one node
than something scattered throughout the DT.

>> One could argue though that the whole usage of power-domains is now
>> moot, as we basically only have implemented one genpd in the whole
>> SoC, which doesn't really reflect the reality. I wonder if better
>> approach would be to have this replaced with proper power domains at
>> some point (if needed), and just have a runtime-pm implementation in
>> place for the devices that require it.
>>
>> So, as an example in DT, we would only have:
>>
>> uart0: serial@02530c00 {
>>   compatible = "xyz";
>>   ...
>>   ti,sci-id = <K2G_DEV_UART0>;
>> };
>>
>> This is somewhat analogous to what OMAP family of SoCs have in place
>> now, under "ti,hwmods" property. I also wonder if the "ti,sci-id"
>> should be replaced with something like "ti,sci-dev-id" to make its
>> purpose clearer.
>
> Unless I'm missing something, that still begs the question of who reads
> that property and takes care of the call into TI-SCI though.
>
> Kevin

Re: [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains

[Ulf Hansson]

+ Sudeep

On 19 January 2017 at 00:03, Rob Herring <robh@kernel.org> wrote:
> On Tue, Jan 17, 2017 at 6:07 PM, Kevin Hilman <khilman@baylibre.com> wrote:
>> Tero Kristo <t-kristo@ti.com> writes:
>>> On 17/01/17 00:12, Dave Gerlach wrote:
>>>> On 01/13/2017 08:40 PM, Rob Herring wrote:
>>>>> On Fri, Jan 13, 2017 at 2:28 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
>
> [...]
>
>>>>>> My ti,sci-id is not an index into a list of power domains, so it
>>>>>> should not
>>>>>> go in the power-domains cells and go against what the power-domains
>>>>>> binding
>>>>>> says that the cell expects. We have one single power domain, and the new
>>>>>> ti,sci-id binding is not something the genpd framework itself is
>>>>>> concerned
>>>>>> with as it's our property to identify a device inside a power domain,
>>>>>> not to
>>>>>> identify which power domain it is associated with.
>>>>>
>>>>> What is the id used for? I can understand why you need to know what
>>>>> power domain a device is in (as power-domains identifies), but not
>>>>> what devices are in a power domain.
>>>>
>>>> We have a system control processor that provides power management
>>>> services to the OS and it responsible for handling the power state of
>>>> each device. This control happens over a communication interface we have
>>>> called TI SCI (implemented at drivers/firmware/ti-sci.c). The
>>>> communication protocol uses these ids to identify each device within the
>>>> power domain so that the control processor can do what is necessary to
>>>> enable that device.
>>>
>>> I think a minor detail here that Rob might be missing right now is,
>>> that the ti,sci-id is only controlling the PM runtime handling, and
>>> providing the ID per-device for this purpose only. AFAIK, it is not
>>> really connected to the power domain anymore as such, as we don't have
>>> power-domains / per device anymore as was the case in some earlier
>>> revision of this work.
>>
>> I think this gets to the heart of things.  IMO The confusion arises
>> because we're throwing around the term "power domain" when there isn't
>> an actual hardware power domain here.
>
> I thought there was 1.
>
>> Unfortunately, the genpd bindings have used the terminology power-domain
>> when in fact genpd is more generic than that and can be used not just
>> for hardware power domains, but for arbitrary grouping of devices that
>> have common PM properties.  That's why genpd actually stands for generic
>> PM domain, not power domain.  Unfortunately, the bindings have grown
>> primarily out of the usage for hardware power domains.
>
> Now it makes some sense.
>
> So the question is does this PM domain grouping need to be described
> in DT or not, and if so what does that look like?

Yes, it's needed and already being done. For example, we have clock
domains for several Renesas platforms.

>
> We could continue to use the power domain binding (maybe we already
> are and that ship has sailed). I'm not totally against the idea even
> if there is no power domain, but I'm not sold on it either. If we do
> go this route, then I still say the id should be a cell in the
> power-domain phandle.
>
> Another option is create something new either common or TI SCI
> specific. It could be just a table of ids and phandles in the SCI
> node. I'm much more comfortable with an isolated property in one node
> than something scattered throughout the DT.

To me, this seems like the best possible solution.

However, perhaps we should also consider the SCPI Generic power domain
(drivers/firmware/scpi_pm_domain.c), because I believe it's closely
related.
To change the power state of a device, this PM domain calls
scpi_device_set|get_power_state() (drivers/firmware/arm_scpi.c), which
also needs a device id as a parameter. Very similar to our case with
the TI SCI domain.

Currently these SCPI device ids lacks corresponding DT bindings, so
the scpi_pm_domain tries to work around it by assigning ids
dynamically at genpd creation time.

That makes me wonder, whether we should think of something common/generic?

[...]

Kind regards
Uffe

Re: [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains

[Sudeep Holla]

On 20/01/17 14:00, Ulf Hansson wrote:
> + Sudeep
> 
> On 19 January 2017 at 00:03, Rob Herring <robh@kernel.org> wrote:

>>
>> We could continue to use the power domain binding (maybe we already
>> are and that ship has sailed). I'm not totally against the idea even
>> if there is no power domain, but I'm not sold on it either. If we do
>> go this route, then I still say the id should be a cell in the
>> power-domain phandle.
>>
>> Another option is create something new either common or TI SCI
>> specific. It could be just a table of ids and phandles in the SCI
>> node. I'm much more comfortable with an isolated property in one node
>> than something scattered throughout the DT.
> 
> To me, this seems like the best possible solution.
> 
> However, perhaps we should also consider the SCPI Generic power domain
> (drivers/firmware/scpi_pm_domain.c), because I believe it's closely
> related.
> To change the power state of a device, this PM domain calls
> scpi_device_set|get_power_state() (drivers/firmware/arm_scpi.c), which
> also needs a device id as a parameter. Very similar to our case with
> the TI SCI domain.
> 
> Currently these SCPI device ids lacks corresponding DT bindings, so
> the scpi_pm_domain tries to work around it by assigning ids
> dynamically at genpd creation time.
> 

IIUC do you mean the binding for the power domain provider to have a
list of domain ids ? If so yes, we don't have one.

But the idea was to have the range to be continuous and create genpd for
the complete range. Though the SCPI specification lacked a command to
get the max. no. of domains supported. That's the reason we had to
introduce the num-domains(*) which may be optional if we have firmware
interface to obtain that information.

-- 
Regards,
Sudeep

(*) P.S: but it has been considered for SCMI(which is an improvement and
more flexible/extensible replacement/upgrade to SCPI) which will be
released soon.

Re: [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains

[Dave Gerlach]

On 01/20/2017 08:00 AM, Ulf Hansson wrote:
> + Sudeep
> 
> On 19 January 2017 at 00:03, Rob Herring <robh@kernel.org> wrote:
>> On Tue, Jan 17, 2017 at 6:07 PM, Kevin Hilman <khilman@baylibre.com> wrote:
>>> Tero Kristo <t-kristo@ti.com> writes:
>>>> On 17/01/17 00:12, Dave Gerlach wrote:
>>>>> On 01/13/2017 08:40 PM, Rob Herring wrote:
>>>>>> On Fri, Jan 13, 2017 at 2:28 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
>>
>> [...]
>>
>>>>>>> My ti,sci-id is not an index into a list of power domains, so it
>>>>>>> should not
>>>>>>> go in the power-domains cells and go against what the power-domains
>>>>>>> binding
>>>>>>> says that the cell expects. We have one single power domain, and the new
>>>>>>> ti,sci-id binding is not something the genpd framework itself is
>>>>>>> concerned
>>>>>>> with as it's our property to identify a device inside a power domain,
>>>>>>> not to
>>>>>>> identify which power domain it is associated with.
>>>>>>
>>>>>> What is the id used for? I can understand why you need to know what
>>>>>> power domain a device is in (as power-domains identifies), but not
>>>>>> what devices are in a power domain.
>>>>>
>>>>> We have a system control processor that provides power management
>>>>> services to the OS and it responsible for handling the power state of
>>>>> each device. This control happens over a communication interface we have
>>>>> called TI SCI (implemented at drivers/firmware/ti-sci.c). The
>>>>> communication protocol uses these ids to identify each device within the
>>>>> power domain so that the control processor can do what is necessary to
>>>>> enable that device.
>>>>
>>>> I think a minor detail here that Rob might be missing right now is,
>>>> that the ti,sci-id is only controlling the PM runtime handling, and
>>>> providing the ID per-device for this purpose only. AFAIK, it is not
>>>> really connected to the power domain anymore as such, as we don't have
>>>> power-domains / per device anymore as was the case in some earlier
>>>> revision of this work.
>>>
>>> I think this gets to the heart of things.  IMO The confusion arises
>>> because we're throwing around the term "power domain" when there isn't
>>> an actual hardware power domain here.
>>
>> I thought there was 1.
>>
>>> Unfortunately, the genpd bindings have used the terminology power-domain
>>> when in fact genpd is more generic than that and can be used not just
>>> for hardware power domains, but for arbitrary grouping of devices that
>>> have common PM properties.  That's why genpd actually stands for generic
>>> PM domain, not power domain.  Unfortunately, the bindings have grown
>>> primarily out of the usage for hardware power domains.
>>
>> Now it makes some sense.
>>
>> So the question is does this PM domain grouping need to be described
>> in DT or not, and if so what does that look like?
> 
> Yes, it's needed and already being done. For example, we have clock
> domains for several Renesas platforms.
> 
>>
>> We could continue to use the power domain binding (maybe we already
>> are and that ship has sailed). I'm not totally against the idea even
>> if there is no power domain, but I'm not sold on it either. If we do
>> go this route, then I still say the id should be a cell in the
>> power-domain phandle.
>>
>> Another option is create something new either common or TI SCI
>> specific. It could be just a table of ids and phandles in the SCI
>> node. I'm much more comfortable with an isolated property in one node
>> than something scattered throughout the DT.
> 
> To me, this seems like the best possible solution.
> 
> However, perhaps we should also consider the SCPI Generic power domain
> (drivers/firmware/scpi_pm_domain.c), because I believe it's closely
> related.
> To change the power state of a device, this PM domain calls
> scpi_device_set|get_power_state() (drivers/firmware/arm_scpi.c), which
> also needs a device id as a parameter. Very similar to our case with
> the TI SCI domain.
> 
> Currently these SCPI device ids lacks corresponding DT bindings, so
> the scpi_pm_domain tries to work around it by assigning ids
> dynamically at genpd creation time.
> 
> That makes me wonder, whether we should think of something common/generic?

When you say something common/generic, do you mean a better binding for genpd,
or something bigger than that like a new driver? Because I do think a phandle
cell left open for the genpd provider to interpret solves both the scpi and
ti-sci problem we are facing here in the best way. Using generic PM domains lets
us do exactly what we want apart from interpreting the phandle cell with our
driver, and I feel like anything else we try at this point is just going to be
to work around that. Is bringing back genpd xlate something we can discuss?

Regards,
Dave

> 
> [...]
> 
> Kind regards
> Uffe
> 

Re: [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains

[Ulf Hansson]

[...]

>>> Another option is create something new either common or TI SCI
>>> specific. It could be just a table of ids and phandles in the SCI
>>> node. I'm much more comfortable with an isolated property in one node
>>> than something scattered throughout the DT.
>>
>> To me, this seems like the best possible solution.
>>
>> However, perhaps we should also consider the SCPI Generic power domain
>> (drivers/firmware/scpi_pm_domain.c), because I believe it's closely
>> related.
>> To change the power state of a device, this PM domain calls
>> scpi_device_set|get_power_state() (drivers/firmware/arm_scpi.c), which
>> also needs a device id as a parameter. Very similar to our case with
>> the TI SCI domain.
>>
>> Currently these SCPI device ids lacks corresponding DT bindings, so
>> the scpi_pm_domain tries to work around it by assigning ids
>> dynamically at genpd creation time.
>>
>> That makes me wonder, whether we should think of something common/generic?
>
> When you say something common/generic, do you mean a better binding for genpd,
> or something bigger than that like a new driver? Because I do think a phandle
> cell left open for the genpd provider to interpret solves both the scpi and
> ti-sci problem we are facing here in the best way. Using generic PM domains lets
> us do exactly what we want apart from interpreting the phandle cell with our
> driver, and I feel like anything else we try at this point is just going to be
> to work around that. Is bringing back genpd xlate something we can discuss?

Bringing back xlate, how would that help? Wouldn't that just mean that
you will get one genpd per device? That's not an option, I think we
are all in agreement to that.

Or am I missing something here?

Regarding something "common/generic", I was merely thinking of some
common bindings describing the list of phandle to device ids mapping.
However, let's not make this more complicated than it is already. So
please just ignore my suggestion so we can make this work for your
case first.

However, maybe I didn't fully understood Rob's suggestion. Perhaps Rob
can clarify once more.

Anyway, this is how interpreted Rob's suggestion:

TISCI_PM_DOMAIN_DEVLIST: tisci-pm-domain-devlist {
     devs = <&serial0>, <&mmc0>;
     devids = <5 10>;
}

With this, you should be to extract the devid which corresponds to the
device that has been attached to the TI SCI PM domain. Don't you
think?

Kind regards
Uffe

Re: [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains

[Dave Gerlach]

On 01/20/2017 10:52 AM, Ulf Hansson wrote:
> [...]
>
>>>> Another option is create something new either common or TI SCI
>>>> specific. It could be just a table of ids and phandles in the SCI
>>>> node. I'm much more comfortable with an isolated property in one node
>>>> than something scattered throughout the DT.
>>>
>>> To me, this seems like the best possible solution.
>>>
>>> However, perhaps we should also consider the SCPI Generic power domain
>>> (drivers/firmware/scpi_pm_domain.c), because I believe it's closely
>>> related.
>>> To change the power state of a device, this PM domain calls
>>> scpi_device_set|get_power_state() (drivers/firmware/arm_scpi.c), which
>>> also needs a device id as a parameter. Very similar to our case with
>>> the TI SCI domain.
>>>
>>> Currently these SCPI device ids lacks corresponding DT bindings, so
>>> the scpi_pm_domain tries to work around it by assigning ids
>>> dynamically at genpd creation time.
>>>
>>> That makes me wonder, whether we should think of something common/generic?
>>
>> When you say something common/generic, do you mean a better binding for genpd,
>> or something bigger than that like a new driver? Because I do think a phandle
>> cell left open for the genpd provider to interpret solves both the scpi and
>> ti-sci problem we are facing here in the best way. Using generic PM domains lets
>> us do exactly what we want apart from interpreting the phandle cell with our
>> driver, and I feel like anything else we try at this point is just going to be
>> to work around that. Is bringing back genpd xlate something we can discuss?
>
> Bringing back xlate, how would that help? Wouldn't that just mean that
> you will get one genpd per device? That's not an option, I think we
> are all in agreement to that.

Sure, perhaps the custom xlate wouldn't be the right way to do it, as we 
wouldn't be able to associate a device directly to a phandle, at least 
with how it was implemented before, but I think we can skip that 
entirely. Does opening up the interpretation of the cells of the 
'power-domains' phandle not solve all of these issues? Is that out of 
the question?

genpd_xlate_simple currently just makes sure the args_count of the 
'power-domains' phandle was zero and bails if it was not. Why couldn't 
we remove this check and let the driver interpret it while still using 
of_genpd_add_provider_simple to register the provider? It's still a 
'simple' provider from the perspective of the genpd framework and the 
actual pm domain mapping will not change, but now the driver can parse 
the cells and do whatever it needs to, such as reading a device id.

I think that's a bit more flexible and will avoid breaking anything that 
is there today.

Regards,
Dave

>
> Or am I missing something here?
>
> Regarding something "common/generic", I was merely thinking of some
> common bindings describing the list of phandle to device ids mapping.
> However, let's not make this more complicated than it is already. So
> please just ignore my suggestion so we can make this work for your
> case first.
>
> However, maybe I didn't fully understood Rob's suggestion. Perhaps Rob
> can clarify once more.
>
> Anyway, this is how interpreted Rob's suggestion:
>
> TISCI_PM_DOMAIN_DEVLIST: tisci-pm-domain-devlist {
>      devs = <&serial0>, <&mmc0>;
>      devids = <5 10>;
> }
>
> With this, you should be to extract the devid which corresponds to the
> device that has been attached to the TI SCI PM domain. Don't you
> think?
>
> Kind regards
> Uffe
>

Re: [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains

[Ulf Hansson]

On 23 January 2017 at 21:11, Dave Gerlach <d-gerlach@ti.com> wrote:
> On 01/20/2017 10:52 AM, Ulf Hansson wrote:
>>
>> [...]
>>
>>>>> Another option is create something new either common or TI SCI
>>>>> specific. It could be just a table of ids and phandles in the SCI
>>>>> node. I'm much more comfortable with an isolated property in one node
>>>>> than something scattered throughout the DT.
>>>>
>>>>
>>>> To me, this seems like the best possible solution.
>>>>
>>>> However, perhaps we should also consider the SCPI Generic power domain
>>>> (drivers/firmware/scpi_pm_domain.c), because I believe it's closely
>>>> related.
>>>> To change the power state of a device, this PM domain calls
>>>> scpi_device_set|get_power_state() (drivers/firmware/arm_scpi.c), which
>>>> also needs a device id as a parameter. Very similar to our case with
>>>> the TI SCI domain.
>>>>
>>>> Currently these SCPI device ids lacks corresponding DT bindings, so
>>>> the scpi_pm_domain tries to work around it by assigning ids
>>>> dynamically at genpd creation time.
>>>>
>>>> That makes me wonder, whether we should think of something
>>>> common/generic?
>>>
>>>
>>> When you say something common/generic, do you mean a better binding for
>>> genpd,
>>> or something bigger than that like a new driver? Because I do think a
>>> phandle
>>> cell left open for the genpd provider to interpret solves both the scpi
>>> and
>>> ti-sci problem we are facing here in the best way. Using generic PM
>>> domains lets
>>> us do exactly what we want apart from interpreting the phandle cell with
>>> our
>>> driver, and I feel like anything else we try at this point is just going
>>> to be
>>> to work around that. Is bringing back genpd xlate something we can
>>> discuss?
>>
>>
>> Bringing back xlate, how would that help? Wouldn't that just mean that
>> you will get one genpd per device? That's not an option, I think we
>> are all in agreement to that.
>
>
> Sure, perhaps the custom xlate wouldn't be the right way to do it, as we
> wouldn't be able to associate a device directly to a phandle, at least with
> how it was implemented before, but I think we can skip that entirely. Does
> opening up the interpretation of the cells of the 'power-domains' phandle
> not solve all of these issues? Is that out of the question?
>
> genpd_xlate_simple currently just makes sure the args_count of the
> 'power-domains' phandle was zero and bails if it was not. Why couldn't we
> remove this check and let the driver interpret it while still using
> of_genpd_add_provider_simple to register the provider? It's still a 'simple'
> provider from the perspective of the genpd framework and the actual pm
> domain mapping will not change, but now the driver can parse the cells and
> do whatever it needs to, such as reading a device id.
>
> I think that's a bit more flexible and will avoid breaking anything that is
> there today.

Would you mind providing an example? Perhaps also some code snippets
dealing with the parsing?

Kind regards
Uffe

Re: [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains

[Dave Gerlach]

On 01/24/2017 04:03 AM, Ulf Hansson wrote:
> On 23 January 2017 at 21:11, Dave Gerlach <d-gerlach@ti.com> wrote:
>> On 01/20/2017 10:52 AM, Ulf Hansson wrote:
>>>
>>> [...]
>>>
>>>>>> Another option is create something new either common or TI SCI
>>>>>> specific. It could be just a table of ids and phandles in the SCI
>>>>>> node. I'm much more comfortable with an isolated property in one node
>>>>>> than something scattered throughout the DT.
>>>>>
>>>>>
>>>>> To me, this seems like the best possible solution.
>>>>>
>>>>> However, perhaps we should also consider the SCPI Generic power domain
>>>>> (drivers/firmware/scpi_pm_domain.c), because I believe it's closely
>>>>> related.
>>>>> To change the power state of a device, this PM domain calls
>>>>> scpi_device_set|get_power_state() (drivers/firmware/arm_scpi.c), which
>>>>> also needs a device id as a parameter. Very similar to our case with
>>>>> the TI SCI domain.
>>>>>
>>>>> Currently these SCPI device ids lacks corresponding DT bindings, so
>>>>> the scpi_pm_domain tries to work around it by assigning ids
>>>>> dynamically at genpd creation time.
>>>>>
>>>>> That makes me wonder, whether we should think of something
>>>>> common/generic?
>>>>
>>>>
>>>> When you say something common/generic, do you mean a better binding for
>>>> genpd,
>>>> or something bigger than that like a new driver? Because I do think a
>>>> phandle
>>>> cell left open for the genpd provider to interpret solves both the scpi
>>>> and
>>>> ti-sci problem we are facing here in the best way. Using generic PM
>>>> domains lets
>>>> us do exactly what we want apart from interpreting the phandle cell with
>>>> our
>>>> driver, and I feel like anything else we try at this point is just going
>>>> to be
>>>> to work around that. Is bringing back genpd xlate something we can
>>>> discuss?
>>>
>>>
>>> Bringing back xlate, how would that help? Wouldn't that just mean that
>>> you will get one genpd per device? That's not an option, I think we
>>> are all in agreement to that.
>>
>>
>> Sure, perhaps the custom xlate wouldn't be the right way to do it, as we
>> wouldn't be able to associate a device directly to a phandle, at least with
>> how it was implemented before, but I think we can skip that entirely. Does
>> opening up the interpretation of the cells of the 'power-domains' phandle
>> not solve all of these issues? Is that out of the question?
>>
>> genpd_xlate_simple currently just makes sure the args_count of the
>> 'power-domains' phandle was zero and bails if it was not. Why couldn't we
>> remove this check and let the driver interpret it while still using
>> of_genpd_add_provider_simple to register the provider? It's still a 'simple'
>> provider from the perspective of the genpd framework and the actual pm
>> domain mapping will not change, but now the driver can parse the cells and
>> do whatever it needs to, such as reading a device id.
>>
>> I think that's a bit more flexible and will avoid breaking anything that is
>> there today.
>
> Would you mind providing an example? Perhaps also some code snippets
> dealing with the parsing?

So again the goal of this is to move the ti,sci-id value back to 
power-domains phandle instead of having a separate property, so that 
would be step one in the DT. Then in the power-domains node change 
#power-domain-cells to one. And then from there, the only change to the 
genpd framework is this:

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index a5e1262b964b..b82e61f0bcfa 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1603,8 +1603,6 @@ static struct generic_pm_domain genpd_xlate_simple
                                       struct of_phandle_args *genpdspec,
                                       void *data)
  {
-       if (genpdspec->args_count != 0)
-               return ERR_PTR(-EINVAL);
         return data;
  }


because genpd_xlate_simple only checks that the phandle is zero so that 
it can fail if it is not, but there's no functional reason it needs to 
do this. The genpd framework works as it did before no matter what the 
cells are set to if using of_genpd_add_provider_simple. Then in the 
attach_dev callback inside the ti_sci_pm_domains driver instead of doing

	ret = of_property_read_u32(np, "ti,sci-id", &idx);

to read the ti,sc-id for a device into idx we can now do:

        ret = of_parse_phandle_with_args(np, "power-domains",
                                    "#power-domain-cells", 0, &pd_args);
        idx = pd_args.args[0];

or even simpler from within our driver

	ret = of_property_read_u32_index(np, "power-domains", 1, &idx);

To read the value into idx.

This requires minimal changes to the genpd framework and gives the 
option for the driver to interpret the cell manually when using a simple 
provider. The genpd framework still uses the phandle just to get the 
power-domain device and the cells are left entirely up to the driver to 
interpret, but if desired you could still use the genpd onecell driver 
for a specific use of the phandle cell, or use it with zero cells.

I can send out an updated series if there are no major objections, or 
just to start discussion there.

Regards,
Dave

>
> Kind regards
> Uffe
>

Re: [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains

[Ulf Hansson]

On 25 January 2017 at 17:59, Dave Gerlach <d-gerlach@ti.com> wrote:
> On 01/24/2017 04:03 AM, Ulf Hansson wrote:
>>
>> On 23 January 2017 at 21:11, Dave Gerlach <d-gerlach@ti.com> wrote:
>>>
>>> On 01/20/2017 10:52 AM, Ulf Hansson wrote:
>>>>
>>>>
>>>> [...]
>>>>
>>>>>>> Another option is create something new either common or TI SCI
>>>>>>> specific. It could be just a table of ids and phandles in the SCI
>>>>>>> node. I'm much more comfortable with an isolated property in one node
>>>>>>> than something scattered throughout the DT.
>>>>>>
>>>>>>
>>>>>>
>>>>>> To me, this seems like the best possible solution.
>>>>>>
>>>>>> However, perhaps we should also consider the SCPI Generic power domain
>>>>>> (drivers/firmware/scpi_pm_domain.c), because I believe it's closely
>>>>>> related.
>>>>>> To change the power state of a device, this PM domain calls
>>>>>> scpi_device_set|get_power_state() (drivers/firmware/arm_scpi.c), which
>>>>>> also needs a device id as a parameter. Very similar to our case with
>>>>>> the TI SCI domain.
>>>>>>
>>>>>> Currently these SCPI device ids lacks corresponding DT bindings, so
>>>>>> the scpi_pm_domain tries to work around it by assigning ids
>>>>>> dynamically at genpd creation time.
>>>>>>
>>>>>> That makes me wonder, whether we should think of something
>>>>>> common/generic?
>>>>>
>>>>>
>>>>>
>>>>> When you say something common/generic, do you mean a better binding for
>>>>> genpd,
>>>>> or something bigger than that like a new driver? Because I do think a
>>>>> phandle
>>>>> cell left open for the genpd provider to interpret solves both the scpi
>>>>> and
>>>>> ti-sci problem we are facing here in the best way. Using generic PM
>>>>> domains lets
>>>>> us do exactly what we want apart from interpreting the phandle cell
>>>>> with
>>>>> our
>>>>> driver, and I feel like anything else we try at this point is just
>>>>> going
>>>>> to be
>>>>> to work around that. Is bringing back genpd xlate something we can
>>>>> discuss?
>>>>
>>>>
>>>>
>>>> Bringing back xlate, how would that help? Wouldn't that just mean that
>>>> you will get one genpd per device? That's not an option, I think we
>>>> are all in agreement to that.
>>>
>>>
>>>
>>> Sure, perhaps the custom xlate wouldn't be the right way to do it, as we
>>> wouldn't be able to associate a device directly to a phandle, at least
>>> with
>>> how it was implemented before, but I think we can skip that entirely.
>>> Does
>>> opening up the interpretation of the cells of the 'power-domains' phandle
>>> not solve all of these issues? Is that out of the question?
>>>
>>> genpd_xlate_simple currently just makes sure the args_count of the
>>> 'power-domains' phandle was zero and bails if it was not. Why couldn't we
>>> remove this check and let the driver interpret it while still using
>>> of_genpd_add_provider_simple to register the provider? It's still a
>>> 'simple'
>>> provider from the perspective of the genpd framework and the actual pm
>>> domain mapping will not change, but now the driver can parse the cells
>>> and
>>> do whatever it needs to, such as reading a device id.
>>>
>>> I think that's a bit more flexible and will avoid breaking anything that
>>> is
>>> there today.
>>
>>
>> Would you mind providing an example? Perhaps also some code snippets
>> dealing with the parsing?
>
>
> So again the goal of this is to move the ti,sci-id value back to
> power-domains phandle instead of having a separate property, so that would
> be step one in the DT. Then in the power-domains node change
> #power-domain-cells to one. And then from there, the only change to the
> genpd framework is this:
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index a5e1262b964b..b82e61f0bcfa 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1603,8 +1603,6 @@ static struct generic_pm_domain genpd_xlate_simple
>                                       struct of_phandle_args *genpdspec,
>                                       void *data)
>  {
> -       if (genpdspec->args_count != 0)
> -               return ERR_PTR(-EINVAL);
>         return data;
>  }
>
>
> because genpd_xlate_simple only checks that the phandle is zero so that it
> can fail if it is not, but there's no functional reason it needs to do this.
> The genpd framework works as it did before no matter what the cells are set
> to if using of_genpd_add_provider_simple. Then in the attach_dev callback
> inside the ti_sci_pm_domains driver instead of doing
>
>         ret = of_property_read_u32(np, "ti,sci-id", &idx);
>
> to read the ti,sc-id for a device into idx we can now do:
>
>        ret = of_parse_phandle_with_args(np, "power-domains",
>                                    "#power-domain-cells", 0, &pd_args);
>        idx = pd_args.args[0];
>
> or even simpler from within our driver
>
>         ret = of_property_read_u32_index(np, "power-domains", 1, &idx);
>
> To read the value into idx.
>
> This requires minimal changes to the genpd framework and gives the option
> for the driver to interpret the cell manually when using a simple provider.
> The genpd framework still uses the phandle just to get the power-domain
> device and the cells are left entirely up to the driver to interpret, but if
> desired you could still use the genpd onecell driver for a specific use of
> the phandle cell, or use it with zero cells.
>
> I can send out an updated series if there are no major objections, or just
> to start discussion there.

Ok, this seems to work! I am ready to review! :-)

Of course, don't forget to update the existing DT doc for the power
domain bindings.

Kind regards
Uffe

Re: [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains

[Rob Herring]

On Wed, Jan 25, 2017 at 10:59 AM, Dave Gerlach <d-gerlach@ti.com> wrote:
> On 01/24/2017 04:03 AM, Ulf Hansson wrote:
>>
>> On 23 January 2017 at 21:11, Dave Gerlach <d-gerlach@ti.com> wrote:
>>>
>>> On 01/20/2017 10:52 AM, Ulf Hansson wrote:
>>>>
>>>>
>>>> [...]
>>>>
>>>>>>> Another option is create something new either common or TI SCI
>>>>>>> specific. It could be just a table of ids and phandles in the SCI
>>>>>>> node. I'm much more comfortable with an isolated property in one node
>>>>>>> than something scattered throughout the DT.
>>>>>>
>>>>>>
>>>>>>
>>>>>> To me, this seems like the best possible solution.
>>>>>>
>>>>>> However, perhaps we should also consider the SCPI Generic power domain
>>>>>> (drivers/firmware/scpi_pm_domain.c), because I believe it's closely
>>>>>> related.
>>>>>> To change the power state of a device, this PM domain calls
>>>>>> scpi_device_set|get_power_state() (drivers/firmware/arm_scpi.c), which
>>>>>> also needs a device id as a parameter. Very similar to our case with
>>>>>> the TI SCI domain.
>>>>>>
>>>>>> Currently these SCPI device ids lacks corresponding DT bindings, so
>>>>>> the scpi_pm_domain tries to work around it by assigning ids
>>>>>> dynamically at genpd creation time.
>>>>>>
>>>>>> That makes me wonder, whether we should think of something
>>>>>> common/generic?
>>>>>
>>>>>
>>>>>
>>>>> When you say something common/generic, do you mean a better binding for
>>>>> genpd,
>>>>> or something bigger than that like a new driver? Because I do think a
>>>>> phandle
>>>>> cell left open for the genpd provider to interpret solves both the scpi
>>>>> and
>>>>> ti-sci problem we are facing here in the best way. Using generic PM
>>>>> domains lets
>>>>> us do exactly what we want apart from interpreting the phandle cell
>>>>> with
>>>>> our
>>>>> driver, and I feel like anything else we try at this point is just
>>>>> going
>>>>> to be
>>>>> to work around that. Is bringing back genpd xlate something we can
>>>>> discuss?
>>>>
>>>>
>>>>
>>>> Bringing back xlate, how would that help? Wouldn't that just mean that
>>>> you will get one genpd per device? That's not an option, I think we
>>>> are all in agreement to that.
>>>
>>>
>>>
>>> Sure, perhaps the custom xlate wouldn't be the right way to do it, as we
>>> wouldn't be able to associate a device directly to a phandle, at least
>>> with
>>> how it was implemented before, but I think we can skip that entirely.
>>> Does
>>> opening up the interpretation of the cells of the 'power-domains' phandle
>>> not solve all of these issues? Is that out of the question?
>>>
>>> genpd_xlate_simple currently just makes sure the args_count of the
>>> 'power-domains' phandle was zero and bails if it was not. Why couldn't we
>>> remove this check and let the driver interpret it while still using
>>> of_genpd_add_provider_simple to register the provider? It's still a
>>> 'simple'
>>> provider from the perspective of the genpd framework and the actual pm
>>> domain mapping will not change, but now the driver can parse the cells
>>> and
>>> do whatever it needs to, such as reading a device id.
>>>
>>> I think that's a bit more flexible and will avoid breaking anything that
>>> is
>>> there today.
>>
>>
>> Would you mind providing an example? Perhaps also some code snippets
>> dealing with the parsing?
>
>
> So again the goal of this is to move the ti,sci-id value back to
> power-domains phandle instead of having a separate property, so that would
> be step one in the DT. Then in the power-domains node change
> #power-domain-cells to one. And then from there, the only change to the
> genpd framework is this:

I'd still like to understand how the ID is used in order to understand
if as a power-domain cell is appropriate. I think the test is this: is
the ID meaningful to (or defined by) the device or the power domain
controller? The former should be a property in the device node. The
latter should be phandle args.

> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index a5e1262b964b..b82e61f0bcfa 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1603,8 +1603,6 @@ static struct generic_pm_domain genpd_xlate_simple
>                                       struct of_phandle_args *genpdspec,
>                                       void *data)
>  {
> -       if (genpdspec->args_count != 0)
> -               return ERR_PTR(-EINVAL);
>         return data;
>  }
>
>
> because genpd_xlate_simple only checks that the phandle is zero so that it
> can fail if it is not, but there's no functional reason it needs to do this.
> The genpd framework works as it did before no matter what the cells are set
> to if using of_genpd_add_provider_simple. Then in the attach_dev callback
> inside the ti_sci_pm_domains driver instead of doing
>
>         ret = of_property_read_u32(np, "ti,sci-id", &idx);
>
> to read the ti,sc-id for a device into idx we can now do:
>
>        ret = of_parse_phandle_with_args(np, "power-domains",
>                                    "#power-domain-cells", 0, &pd_args);
>        idx = pd_args.args[0];
>
> or even simpler from within our driver
>
>         ret = of_property_read_u32_index(np, "power-domains", 1, &idx);

This you should not be doing. The client driver shouldn't care how
many cells or what their values are.

Rob

Re: [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains

[Dave Gerlach]

On 01/25/2017 04:32 PM, Rob Herring wrote:
> On Wed, Jan 25, 2017 at 10:59 AM, Dave Gerlach <d-gerlach@ti.com> wrote:
>> On 01/24/2017 04:03 AM, Ulf Hansson wrote:
>>>
>>> On 23 January 2017 at 21:11, Dave Gerlach <d-gerlach@ti.com> wrote:
>>>>
>>>> On 01/20/2017 10:52 AM, Ulf Hansson wrote:
>>>>>
>>>>>
>>>>> [...]
>>>>>
>>>>>>>> Another option is create something new either common or TI SCI
>>>>>>>> specific. It could be just a table of ids and phandles in the SCI
>>>>>>>> node. I'm much more comfortable with an isolated property in one node
>>>>>>>> than something scattered throughout the DT.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> To me, this seems like the best possible solution.
>>>>>>>
>>>>>>> However, perhaps we should also consider the SCPI Generic power domain
>>>>>>> (drivers/firmware/scpi_pm_domain.c), because I believe it's closely
>>>>>>> related.
>>>>>>> To change the power state of a device, this PM domain calls
>>>>>>> scpi_device_set|get_power_state() (drivers/firmware/arm_scpi.c), which
>>>>>>> also needs a device id as a parameter. Very similar to our case with
>>>>>>> the TI SCI domain.
>>>>>>>
>>>>>>> Currently these SCPI device ids lacks corresponding DT bindings, so
>>>>>>> the scpi_pm_domain tries to work around it by assigning ids
>>>>>>> dynamically at genpd creation time.
>>>>>>>
>>>>>>> That makes me wonder, whether we should think of something
>>>>>>> common/generic?
>>>>>>
>>>>>>
>>>>>>
>>>>>> When you say something common/generic, do you mean a better binding for
>>>>>> genpd,
>>>>>> or something bigger than that like a new driver? Because I do think a
>>>>>> phandle
>>>>>> cell left open for the genpd provider to interpret solves both the scpi
>>>>>> and
>>>>>> ti-sci problem we are facing here in the best way. Using generic PM
>>>>>> domains lets
>>>>>> us do exactly what we want apart from interpreting the phandle cell
>>>>>> with
>>>>>> our
>>>>>> driver, and I feel like anything else we try at this point is just
>>>>>> going
>>>>>> to be
>>>>>> to work around that. Is bringing back genpd xlate something we can
>>>>>> discuss?
>>>>>
>>>>>
>>>>>
>>>>> Bringing back xlate, how would that help? Wouldn't that just mean that
>>>>> you will get one genpd per device? That's not an option, I think we
>>>>> are all in agreement to that.
>>>>
>>>>
>>>>
>>>> Sure, perhaps the custom xlate wouldn't be the right way to do it, as we
>>>> wouldn't be able to associate a device directly to a phandle, at least
>>>> with
>>>> how it was implemented before, but I think we can skip that entirely.
>>>> Does
>>>> opening up the interpretation of the cells of the 'power-domains' phandle
>>>> not solve all of these issues? Is that out of the question?
>>>>
>>>> genpd_xlate_simple currently just makes sure the args_count of the
>>>> 'power-domains' phandle was zero and bails if it was not. Why couldn't we
>>>> remove this check and let the driver interpret it while still using
>>>> of_genpd_add_provider_simple to register the provider? It's still a
>>>> 'simple'
>>>> provider from the perspective of the genpd framework and the actual pm
>>>> domain mapping will not change, but now the driver can parse the cells
>>>> and
>>>> do whatever it needs to, such as reading a device id.
>>>>
>>>> I think that's a bit more flexible and will avoid breaking anything that
>>>> is
>>>> there today.
>>>
>>>
>>> Would you mind providing an example? Perhaps also some code snippets
>>> dealing with the parsing?
>>
>>
>> So again the goal of this is to move the ti,sci-id value back to
>> power-domains phandle instead of having a separate property, so that would
>> be step one in the DT. Then in the power-domains node change
>> #power-domain-cells to one. And then from there, the only change to the
>> genpd framework is this:
>
> I'd still like to understand how the ID is used in order to understand
> if as a power-domain cell is appropriate. I think the test is this: is
> the ID meaningful to (or defined by) the device or the power domain
> controller? The former should be a property in the device node. The
> latter should be phandle args.

The device itself doesn't care about the ID or ever need to know what it is. The 
power domain controller is the only user of the ID. The power domain controller 
needs the ID for a specific device to be able to turn it on, so during probe of 
each device the power-domain controller makes this association which is why I 
included it in each device node, so I think it fits as a phandle arg based on 
your test.

>
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index a5e1262b964b..b82e61f0bcfa 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -1603,8 +1603,6 @@ static struct generic_pm_domain genpd_xlate_simple
>>                                       struct of_phandle_args *genpdspec,
>>                                       void *data)
>>  {
>> -       if (genpdspec->args_count != 0)
>> -               return ERR_PTR(-EINVAL);
>>         return data;
>>  }
>>
>>
>> because genpd_xlate_simple only checks that the phandle is zero so that it
>> can fail if it is not, but there's no functional reason it needs to do this.
>> The genpd framework works as it did before no matter what the cells are set
>> to if using of_genpd_add_provider_simple. Then in the attach_dev callback
>> inside the ti_sci_pm_domains driver instead of doing
>>
>>         ret = of_property_read_u32(np, "ti,sci-id", &idx);
>>
>> to read the ti,sc-id for a device into idx we can now do:
>>
>>        ret = of_parse_phandle_with_args(np, "power-domains",
>>                                    "#power-domain-cells", 0, &pd_args);
>>        idx = pd_args.args[0];
>>
>> or even simpler from within our driver
>>
>>         ret = of_property_read_u32_index(np, "power-domains", 1, &idx);
>
> This you should not be doing. The client driver shouldn't care how
> many cells or what their values are.

Client drivers in other places use xlate functions, like in the drivers/reset 
framework, to interpret the cells. Is doing it this way really that different?

Regards,
Dave

>
> Rob
>

Re: [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains

[Rob Herring]

On Thu, Jan 26, 2017 at 9:09 AM, Dave Gerlach <d-gerlach@ti.com> wrote:
> On 01/25/2017 04:32 PM, Rob Herring wrote:
>>
>> On Wed, Jan 25, 2017 at 10:59 AM, Dave Gerlach <d-gerlach@ti.com> wrote:

[...]

>>> because genpd_xlate_simple only checks that the phandle is zero so that
>>> it
>>> can fail if it is not, but there's no functional reason it needs to do
>>> this.
>>> The genpd framework works as it did before no matter what the cells are
>>> set
>>> to if using of_genpd_add_provider_simple. Then in the attach_dev callback
>>> inside the ti_sci_pm_domains driver instead of doing
>>>
>>>         ret = of_property_read_u32(np, "ti,sci-id", &idx);
>>>
>>> to read the ti,sc-id for a device into idx we can now do:
>>>
>>>        ret = of_parse_phandle_with_args(np, "power-domains",
>>>                                    "#power-domain-cells", 0, &pd_args);
>>>        idx = pd_args.args[0];
>>>
>>> or even simpler from within our driver
>>>
>>>         ret = of_property_read_u32_index(np, "power-domains", 1, &idx);
>>
>>
>> This you should not be doing. The client driver shouldn't care how
>> many cells or what their values are.
>
>
> Client drivers in other places use xlate functions, like in the
> drivers/reset framework, to interpret the cells. Is doing it this way really
> that different?

Oh, I was thinking the client driver, not the power domain controller
driver. NM.

Rob

[PATCH v3 3/4] soc: ti: Add ti_sci_pm_domains driver

[Dave Gerlach]

Introduce a ti_sci_pm_domains driver to act as a generic pm domain provider
to allow each device to attach and associate it's ti-sci-id so that it can
be controlled through the TI SCI protocol.

This driver implements a simple genpd where each device node has
a phandle to the power domain node and also must provide an index which
represents the ID to be passed with TI SCI representing the device using a
ti,sci-id property. Through this interface the genpd dev_ops start and
stop hooks will use TI SCI to turn on and off each device as determined
by pm_runtime usage.

Signed-off-by: Keerthy <j-keerthy@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
 MAINTAINERS                        |   1 +
 arch/arm/mach-keystone/Kconfig     |   1 +
 drivers/soc/ti/Kconfig             |  12 +++
 drivers/soc/ti/Makefile            |   1 +
 drivers/soc/ti/ti_sci_pm_domains.c | 198 +++++++++++++++++++++++++++++++++++++
 5 files changed, 213 insertions(+)
 create mode 100644 drivers/soc/ti/ti_sci_pm_domains.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 7e68af170dae..39d05f92bfcb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12146,6 +12146,7 @@ F:	drivers/firmware/ti_sci*
 F:	include/linux/soc/ti/ti_sci_protocol.h
 F:	Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
 F:	include/dt-bindings/genpd/k2g.h
+F:	drivers/soc/ti/ti_sci_pm_domains.c
 
 THANKO'S RAREMONO AM/FM/SW RADIO RECEIVER USB DRIVER
 M:	Hans Verkuil <hverkuil@xs4all.nl>
diff --git a/arch/arm/mach-keystone/Kconfig b/arch/arm/mach-keystone/Kconfig
index 24bd64dabdfc..18d49465cafb 100644
--- a/arch/arm/mach-keystone/Kconfig
+++ b/arch/arm/mach-keystone/Kconfig
@@ -9,6 +9,7 @@ config ARCH_KEYSTONE
 	select ARCH_SUPPORTS_BIG_ENDIAN
 	select ZONE_DMA if ARM_LPAE
 	select PINCTRL
+	select PM_GENERIC_DOMAINS if PM
 	help
 	  Support for boards based on the Texas Instruments Keystone family of
 	  SoCs.
diff --git a/drivers/soc/ti/Kconfig b/drivers/soc/ti/Kconfig
index 3557c5e32a93..39e152abe6b9 100644
--- a/drivers/soc/ti/Kconfig
+++ b/drivers/soc/ti/Kconfig
@@ -38,4 +38,16 @@ config WKUP_M3_IPC
 	  to communicate and use the Wakeup M3 for PM features like suspend
 	  resume and boots it using wkup_m3_rproc driver.
 
+config TI_SCI_PM_DOMAINS
+	tristate "TI SCI PM Domains Driver"
+	depends on TI_SCI_PROTOCOL
+	depends on PM_GENERIC_DOMAINS
+	help
+	  Generic power domain implementation for TI device implementing
+	  the TI SCI protocol.
+
+	  To compile this as a module, choose M here. The module will be
+	  called ti_sci_pm_domains. Note this is needed early in boot before
+	  rootfs may be available.
+
 endif # SOC_TI
diff --git a/drivers/soc/ti/Makefile b/drivers/soc/ti/Makefile
index 48ff3a79634f..7d572736c86e 100644
--- a/drivers/soc/ti/Makefile
+++ b/drivers/soc/ti/Makefile
@@ -5,3 +5,4 @@ obj-$(CONFIG_KEYSTONE_NAVIGATOR_QMSS)	+= knav_qmss.o
 knav_qmss-y := knav_qmss_queue.o knav_qmss_acc.o
 obj-$(CONFIG_KEYSTONE_NAVIGATOR_DMA)	+= knav_dma.o
 obj-$(CONFIG_WKUP_M3_IPC)		+= wkup_m3_ipc.o
+obj-$(CONFIG_TI_SCI_PM_DOMAINS)		+= ti_sci_pm_domains.o
diff --git a/drivers/soc/ti/ti_sci_pm_domains.c b/drivers/soc/ti/ti_sci_pm_domains.c
new file mode 100644
index 000000000000..ec76215d64c7
--- /dev/null
+++ b/drivers/soc/ti/ti_sci_pm_domains.c
@@ -0,0 +1,198 @@
+/*
+ * TI SCI Generic Power Domain Driver
+ *
+ * Copyright (C) 2015-2016 Texas Instruments Incorporated - http://www.ti.com/
+ *	J Keerthy <j-keerthy@ti.com>
+ *	Dave Gerlach <d-gerlach@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/slab.h>
+#include <linux/soc/ti/ti_sci_protocol.h>
+
+/**
+ * struct ti_sci_genpd_dev_data: holds data needed for every device attached
+ *				 to this genpd
+ * @idx: index of the device that identifies it with the system
+ *	 control processor.
+ */
+struct ti_sci_genpd_dev_data {
+	int idx;
+};
+
+/**
+ * struct ti_sci_pm_domain: TI specific data needed for power domain
+ * @ti_sci: handle to TI SCI protocol driver that provides ops to
+ *	    communicate with system control processor.
+ * @dev: pointer to dev for the driver for devm allocs
+ * @pd: generic_pm_domain for use with the genpd framework
+ */
+struct ti_sci_pm_domain {
+	const struct ti_sci_handle *ti_sci;
+	struct device *dev;
+	struct generic_pm_domain pd;
+};
+
+#define genpd_to_ti_sci_pd(gpd) container_of(gpd, struct ti_sci_pm_domain, pd)
+
+/**
+ * ti_sci_dev_id(): get prepopulated ti_sci id from struct dev
+ * @dev: pointer to device associated with this genpd
+ *
+ * Returns device_id stored from ti,sci_id property
+ */
+static int ti_sci_dev_id(struct device *dev)
+{
+	struct generic_pm_domain_data *genpd_data = dev_gpd_data(dev);
+	struct ti_sci_genpd_dev_data *sci_dev_data = genpd_data->data;
+
+	return sci_dev_data->idx;
+}
+
+/**
+ * ti_sci_dev_to_sci_handle(): get pointer to ti_sci_handle
+ * @dev: pointer to device associated with this genpd
+ *
+ * Returns ti_sci_handle to be used to communicate with system
+ *	   control processor.
+ */
+static const struct ti_sci_handle *ti_sci_dev_to_sci_handle(struct device *dev)
+{
+	struct generic_pm_domain *pd = pd_to_genpd(dev->pm_domain);
+	struct ti_sci_pm_domain *ti_sci_genpd = genpd_to_ti_sci_pd(pd);
+
+	return ti_sci_genpd->ti_sci;
+}
+
+/**
+ * ti_sci_dev_start(): genpd device start hook called to turn device on
+ * @dev: pointer to device associated with this genpd to be powered on
+ */
+static int ti_sci_dev_start(struct device *dev)
+{
+	const struct ti_sci_handle *ti_sci = ti_sci_dev_to_sci_handle(dev);
+	int idx = ti_sci_dev_id(dev);
+
+	return ti_sci->ops.dev_ops.get_device(ti_sci, idx);
+}
+
+/**
+ * ti_sci_dev_stop(): genpd device stop hook called to turn device off
+ * @dev: pointer to device associated with this genpd to be powered off
+ */
+static int ti_sci_dev_stop(struct device *dev)
+{
+	const struct ti_sci_handle *ti_sci = ti_sci_dev_to_sci_handle(dev);
+	int idx = ti_sci_dev_id(dev);
+
+	return ti_sci->ops.dev_ops.put_device(ti_sci, idx);
+}
+
+static int ti_sci_pd_attach_dev(struct generic_pm_domain *domain,
+				struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct ti_sci_pm_domain *ti_sci_genpd = genpd_to_ti_sci_pd(domain);
+	const struct ti_sci_handle *ti_sci = ti_sci_genpd->ti_sci;
+	struct ti_sci_genpd_dev_data *sci_dev_data;
+	struct generic_pm_domain_data *genpd_data;
+	int idx, ret = 0;
+
+	ret = of_property_read_u32(np, "ti,sci-id", &idx);
+	if (ret) {
+		dev_err(ti_sci_genpd->dev, "Cannot find ti,sci-id for %s\n",
+			dev_name(dev));
+		return -ENODEV;
+	}
+
+	/*
+	 * Check the validity of the requested idx, if the index is not valid
+	 * the PMMC will return a NAK here and we will not allocate it.
+	 */
+	ret = ti_sci->ops.dev_ops.is_valid(ti_sci, idx);
+	if (ret)
+		return -EINVAL;
+
+	sci_dev_data = kzalloc(sizeof(*sci_dev_data), GFP_KERNEL);
+	if (!sci_dev_data)
+		return -ENOMEM;
+
+	sci_dev_data->idx = idx;
+
+	genpd_data = dev_gpd_data(dev);
+	genpd_data->data = sci_dev_data;
+
+	return 0;
+}
+
+static void ti_sci_pd_detach_dev(struct generic_pm_domain *domain,
+				 struct device *dev)
+{
+	struct generic_pm_domain_data *genpd_data = dev_gpd_data(dev);
+	struct ti_sci_genpd_dev_data *sci_dev_data = genpd_data->data;
+
+	kfree(sci_dev_data);
+	genpd_data->data = NULL;
+}
+
+static const struct of_device_id ti_sci_pm_domain_matches[] = {
+	{ .compatible = "ti,sci-pm-domain", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, ti_sci_pm_domain_matches);
+
+static int ti_sci_pm_domain_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct ti_sci_pm_domain *ti_sci_pd;
+	int ret;
+
+	ti_sci_pd = devm_kzalloc(dev, sizeof(*ti_sci_pd), GFP_KERNEL);
+	if (!ti_sci_pd)
+		return -ENOMEM;
+
+	ti_sci_pd->ti_sci = devm_ti_sci_get_handle(dev);
+	if (IS_ERR(ti_sci_pd->ti_sci))
+		return PTR_ERR(ti_sci_pd->ti_sci);
+
+	ti_sci_pd->dev = dev;
+
+	ti_sci_pd->pd.attach_dev = ti_sci_pd_attach_dev;
+	ti_sci_pd->pd.detach_dev = ti_sci_pd_detach_dev;
+
+	ti_sci_pd->pd.dev_ops.start = ti_sci_dev_start;
+	ti_sci_pd->pd.dev_ops.stop = ti_sci_dev_stop;
+
+	pm_genpd_init(&ti_sci_pd->pd, NULL, true);
+
+	ret = of_genpd_add_provider_simple(np, &ti_sci_pd->pd);
+
+	return ret;
+}
+
+static struct platform_driver ti_sci_pm_domains_driver = {
+	.probe = ti_sci_pm_domain_probe,
+	.driver = {
+		.name = "ti_sci_pm_domains",
+		.of_match_table = ti_sci_pm_domain_matches,
+	},
+};
+module_platform_driver(ti_sci_pm_domains_driver);
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("TI System Control Interface (SCI) Power Domain driver");
+MODULE_AUTHOR("Dave Gerlach");
-- 
2.11.0

[PATCH v3 4/4] ARM: keystone: Drop PM domain support for k2g

[Dave Gerlach]

K2G will use a different power domain driver than the rest of the
keystone family in order to make use of the TI SCI protocol so prevent
the standard keystone pm_domain code from registering itself in
preparation for a new driver.

Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
 arch/arm/mach-keystone/pm_domain.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-keystone/pm_domain.c b/arch/arm/mach-keystone/pm_domain.c
index 8cbb35765a19..fe57e2692629 100644
--- a/arch/arm/mach-keystone/pm_domain.c
+++ b/arch/arm/mach-keystone/pm_domain.c
@@ -32,7 +32,9 @@ static struct pm_clk_notifier_block platform_domain_notifier = {
 };
 
 static const struct of_device_id of_keystone_table[] = {
-	{.compatible = "ti,keystone"},
+	{.compatible = "ti,k2hk"},
+	{.compatible = "ti,k2e"},
+	{.compatible = "ti,k2l"},
 	{ /* end of list */ },
 };
 
-- 
2.11.0

Re: [PATCH v3 0/4] ARM: K2G: Add support for TI-SCI Generic PM Domains

[Santosh Shilimkar]

On 1/4/2017 12:55 PM, Dave Gerlach wrote:
> Hi,
> This is v3 of the series to add support for TI-SCI Generic PM Domains.
> Previous versions can be found here:
>
> v2: https://www.spinics.net/lists/kernel/msg2364612.html
> v1: http://www.spinics.net/lists/arm-kernel/msg525204.html
>
> This version is rebased on v4.10-rc2 but is the same as v2 with the
> exception of patch 2 in which the devicetree binding documentation
> needed to be updated to show the k2g_pds node should be a child of
> the pmmc node. Apart from that, the acks provided by Ulf were added
> to patches 1 and 3.
>
> Now that the TI-SCI series has been merged [1] this series will be ready
> to go in with an ack on the DT binding. Rob had raised some questions on
> the necessity ti,sci-id property but I believe these were properly
> addressed during the discussion of v2 so hopefully an ack is in order now.
>
How do you plan to merge this series with below patch ?

>   PM / Domains: Add generic data pointer to genpd data struct
I think this one goes via Rafael's tree. If you want me to merge this
along with other patches then will need his ack.

Other way is to get that merged first via Rafael's tree and then
the remainder series.

Regards,
Santosh

Re: [PATCH v3 0/4] ARM: K2G: Add support for TI-SCI Generic PM Domains

[Dave Gerlach]

Santosh,
On 01/04/2017 03:54 PM, Santosh Shilimkar wrote:
> On 1/4/2017 12:55 PM, Dave Gerlach wrote:
>> Hi,
>> This is v3 of the series to add support for TI-SCI Generic PM Domains.
>> Previous versions can be found here:
>>
>> v2: https://www.spinics.net/lists/kernel/msg2364612.html
>> v1: http://www.spinics.net/lists/arm-kernel/msg525204.html
>>
>> This version is rebased on v4.10-rc2 but is the same as v2 with the
>> exception of patch 2 in which the devicetree binding documentation
>> needed to be updated to show the k2g_pds node should be a child of
>> the pmmc node. Apart from that, the acks provided by Ulf were added
>> to patches 1 and 3.
>>
>> Now that the TI-SCI series has been merged [1] this series will be ready
>> to go in with an ack on the DT binding. Rob had raised some questions on
>> the necessity ti,sci-id property but I believe these were properly
>> addressed during the discussion of v2 so hopefully an ack is in order
>> now.
>>
> How do you plan to merge this series with below patch ?
>
>>   PM / Domains: Add generic data pointer to genpd data struct
> I think this one goes via Rafael's tree. If you want me to merge this
> along with other patches then will need his ack.
>
> Other way is to get that merged first via Rafael's tree and then
> the remainder series.

I'd be happy with it going in through your tree with an ack to avoid any 
delay but I'd say it's Rafael's call as it is a patch to the genpd core, 
even though at this point I am the only user.

Regards,
Dave

>
> Regards,
> Santosh

Re: [PATCH v3 0/4] ARM: K2G: Add support for TI-SCI Generic PM Domains

[santosh.shilimkar]

On 1/4/17 2:06 PM, Dave Gerlach wrote:
> Santosh,
> On 01/04/2017 03:54 PM, Santosh Shilimkar wrote:
>> On 1/4/2017 12:55 PM, Dave Gerlach wrote:
>>> Hi,
>>> This is v3 of the series to add support for TI-SCI Generic PM Domains.
>>> Previous versions can be found here:
>>>
>>> v2: https://www.spinics.net/lists/kernel/msg2364612.html
>>> v1: http://www.spinics.net/lists/arm-kernel/msg525204.html
>>>
>>> This version is rebased on v4.10-rc2 but is the same as v2 with the
>>> exception of patch 2 in which the devicetree binding documentation
>>> needed to be updated to show the k2g_pds node should be a child of
>>> the pmmc node. Apart from that, the acks provided by Ulf were added
>>> to patches 1 and 3.
>>>
>>> Now that the TI-SCI series has been merged [1] this series will be ready
>>> to go in with an ack on the DT binding. Rob had raised some questions on
>>> the necessity ti,sci-id property but I believe these were properly
>>> addressed during the discussion of v2 so hopefully an ack is in order
>>> now.
>>>
>> How do you plan to merge this series with below patch ?
>>
>>>   PM / Domains: Add generic data pointer to genpd data struct
>> I think this one goes via Rafael's tree. If you want me to merge this
>> along with other patches then will need his ack.
>>
>> Other way is to get that merged first via Rafael's tree and then
>> the remainder series.
>
> I'd be happy with it going in through your tree with an ack to avoid any
> delay but I'd say it's Rafael's call as it is a patch to the genpd core,
> even though at this point I am the only user.
>
Am going to send pull request over weekend, so if you would like
me take the series via arm-soc tree, please get Rafaels, ack and
let me know.

Regards,
Santosh

Re: [PATCH v3 0/4] ARM: K2G: Add support for TI-SCI Generic PM Domains

[Dave Gerlach]

Santosh,
On 01/19/2017 11:51 AM, santosh.shilimkar@oracle.com wrote:
> On 1/4/17 2:06 PM, Dave Gerlach wrote:
>> Santosh,
>> On 01/04/2017 03:54 PM, Santosh Shilimkar wrote:
>>> On 1/4/2017 12:55 PM, Dave Gerlach wrote:
>>>> Hi,
>>>> This is v3 of the series to add support for TI-SCI Generic PM Domains.
>>>> Previous versions can be found here:
>>>>
>>>> v2: https://www.spinics.net/lists/kernel/msg2364612.html
>>>> v1: http://www.spinics.net/lists/arm-kernel/msg525204.html
>>>>
>>>> This version is rebased on v4.10-rc2 but is the same as v2 with the
>>>> exception of patch 2 in which the devicetree binding documentation
>>>> needed to be updated to show the k2g_pds node should be a child of
>>>> the pmmc node. Apart from that, the acks provided by Ulf were added
>>>> to patches 1 and 3.
>>>>
>>>> Now that the TI-SCI series has been merged [1] this series will be
>>>> ready
>>>> to go in with an ack on the DT binding. Rob had raised some
>>>> questions on
>>>> the necessity ti,sci-id property but I believe these were properly
>>>> addressed during the discussion of v2 so hopefully an ack is in order
>>>> now.
>>>>
>>> How do you plan to merge this series with below patch ?
>>>
>>>>   PM / Domains: Add generic data pointer to genpd data struct
>>> I think this one goes via Rafael's tree. If you want me to merge this
>>> along with other patches then will need his ack.
>>>
>>> Other way is to get that merged first via Rafael's tree and then
>>> the remainder series.
>>
>> I'd be happy with it going in through your tree with an ack to avoid any
>> delay but I'd say it's Rafael's call as it is a patch to the genpd core,
>> even though at this point I am the only user.
>>
> Am going to send pull request over weekend, so if you would like
> me take the series via arm-soc tree, please get Rafaels, ack and
> let me know.

Thanks for letting me know, unfortunately we have not yet reached 
alignment on the binding so I do not believe we will be ready for merge 
in time.

Regards,
Dave

>
> Regards,
> Santosh
>
>

Re: [PATCH v3 0/4] ARM: K2G: Add support for TI-SCI Generic PM Domains

[Santosh Shilimkar]

On 1/19/2017 10:59 AM, Dave Gerlach wrote:
> Santosh,
> On 01/19/2017 11:51 AM, santosh.shilimkar@oracle.com wrote:
>> On 1/4/17 2:06 PM, Dave Gerlach wrote:

[...]

>> Am going to send pull request over weekend, so if you would like
>> me take the series via arm-soc tree, please get Rafaels, ack and
>> let me know.
>
> Thanks for letting me know, unfortunately we have not yet reached
> alignment on the binding so I do not believe we will be ready for merge
> in time.
>
Thanks for clarification.

Regards,
Santosh