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

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

[Nishanth Menon]

Texas Instruments' Keystone generation System on Chips (SoC)
starting with 66AK2G02[1], now include a dedicated SoC System Control
entity called PMMC(Power Management Micro Controller) in line with
ARM architecture recommendations. The function of this module is
to integrate all system operations in a centralized location.
Communication with the SoC System Control entity from various
processing units like ARM/DSP occurs over Message Manager hardware
block.

This series adds the base support for device control using generic
power domain framework over the TI System Control Interface (TI-SCI)
protocol[2][3].

Overall architecture is very similar to SCPI[4] as follows:
+----------------+  +---------+   +------------+
| TI SCI GENPD(*)|  |TISCI Clk|   |TISCI reset |
+------+---------+  +--+------+   +------+-----+
       |               |                 |
       |          +----v--------------+  |
       +----------> TISCI Protocol    <--+
                  +----+--------------+
                       |
                   +---v-----------+
                   | MAILBOX  FWK  |
                   +---+-----------+
                       |
                   +---v-----------+
                   |  TI MSGMGR    |-> TISCI hardware block
                   +---------------+

(*) This series.

Baseline: v4.8-rc1 + [3] (the dependency is due to MAINTAINERS update)
Bootlog: http://pastebin.ubuntu.com/23071846/ (with the addition of couple of mach patches + dts)
Integrated series is available:
https://github.com/nmenon/linux-2.6-playground/commits/upstream/v4.9/tisci-genpd-v1

Dave Gerlach (3):
  Documentation: dt: Add TI-SCI PM Domains
  dt-bindings: genpd: Add K2G device definitions
  soc: ti: Add ti_sci_pm_domains driver

[1] http://www.ti.com/product/66ak2g02
[2] http://processors.wiki.ti.com/index.php/TISCI
[3] https://lkml.org/lkml/2016/8/19/768

 .../devicetree/bindings/soc/ti/sci-pm-domain.txt   |  58 ++++++
 MAINTAINERS                                        |   3 +
 arch/arm/mach-keystone/Kconfig                     |   1 +
 drivers/soc/ti/Kconfig                             |  12 ++
 drivers/soc/ti/Makefile                            |   1 +
 drivers/soc/ti/ti_sci_pm_domains.c                 | 222 +++++++++++++++++++++
 include/dt-bindings/genpd/k2g.h                    |  90 +++++++++
 7 files changed, 387 insertions(+)
 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.9.1.200.gb1ec08f

[PATCH 1/3] Documentation: dt: Add TI-SCI PM Domains

[Nishanth Menon]

From: Dave Gerlach <d-gerlach@ti.com>

Add a generic power domain implementation, TI SCI PM Domains, that
will hook into the genpd framework and allow each PD, which will be
created one per device, to be managed over the TI-SCI protocol.

Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
---
 .../devicetree/bindings/soc/ti/sci-pm-domain.txt   | 58 ++++++++++++++++++++++
 MAINTAINERS                                        |  1 +
 2 files changed, 59 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt

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..059a5d71d692
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
@@ -0,0 +1,58 @@
+Texas Instruments TI-SCI Generic Power Domain
+---------------------------------------------
+
+Some TI SoCs contain a system controller (like the PMMC, etc...) that are
+responsible for control 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 Domains Nodes
+================
+The PM domains node represents the global PM domain managed by the PMMC,
+which in this case is one cell implementation as documented by the generic
+PM domain bindings in
+Documentation/devicetree/bindings/power/power_domain.txt.
+
+Required Properties:
+--------------------
+- compatible: should be "ti,sci-pm-domains"
+- #power-domain-cells: Must be 1 so that an offset can be provided in each
+		       device node.
+- ti,sci: Phandle to the TI SCI device to use for managing the devices
+
+Example:
+--------------------
+/* From arch/arm/boot/dts/k2g.dtsi */
+k2g_pds: k2g_pds {
+        compatible = "ti,sci-pm-domains";
+        #power-domain-cells = <1>;
+        ti,sci = <&pmmc>;
+};
+
+PM Domain Consumers
+===================
+Hardware blocks belonging to a PM domain should contain a "power-domains"
+property that is a phandle pointing to the corresponding PM domain node
+along with an index representing the device id to be passed for the PMMC
+for device control.
+
+See dt-bindings/genpd/k2g.h for the list of valid identifiers for k2g.
+
+Example:
+--------------------
+/* From arch/arm/boot/dts/k2g.dtsi */
+uart0: serial@02530c00 {
+	compatible = "ns16550a";
+	current-speed = <115200>;
+	reg-shift = <2>;
+	reg-io-width = <4>;
+	reg = <0x02530c00 0x100>;
+	interrupts = <GIC_SPI 164 IRQ_TYPE_EDGE_RISING>;
+	clock-frequency = <200000000>;
+	status = "disabled";
+	power-domains = <&k2g_pds K2G_DEV_UART0>;
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 0c5e3d16f694..657fb6a10286 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11573,6 +11573,7 @@ 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
 
 THANKO'S RAREMONO AM/FM/SW RADIO RECEIVER USB DRIVER
 M:	Hans Verkuil <hverkuil@xs4all.nl>
-- 
2.9.1.200.gb1ec08f

[PATCH 2/3] dt-bindings: genpd: Add K2G device definitions

[Nishanth Menon]

From: Dave Gerlach <d-gerlach@ti.com>

Provide macros representing each devices 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. Includes
contributions from Peter Ujfalusi <peter.ujfalusi@ti.com>

Signed-off-by: Keerthy <j-keerthy@ti.com>
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
---
 MAINTAINERS                     |  1 +
 include/dt-bindings/genpd/k2g.h | 90 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+)
 create mode 100644 include/dt-bindings/genpd/k2g.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 657fb6a10286..ea14b08c30bb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11574,6 +11574,7 @@ 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.9.1.200.gb1ec08f

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

[Nishanth Menon]

From: Dave Gerlach <d-gerlach@ti.com>

Introduce a ti_sci_pm_domains to act as a generic pm domain provider to
allow each device to map into it's own genpd and be controlled through
the TI SCI protocol.

This driver implements a onecell genpd where each device node will have
a phandle to the power domain node and an index which represents the ID
to be passed with TI SCI representing the device. Through this interface
the power_on and power_off hooks within the driver will use TI SCI to
turn on and off hardware blocks as determined by pm_runtime usage.

This includes contributions by Andrew F. Davis <afd@ti.com>

Signed-off-by: Keerthy <j-keerthy@ti.com>
Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
Signed-off-by: Nishanth Menon <nm@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 | 222 +++++++++++++++++++++++++++++++++++++
 5 files changed, 237 insertions(+)
 create mode 100644 drivers/soc/ti/ti_sci_pm_domains.c

diff --git a/MAINTAINERS b/MAINTAINERS
index ea14b08c30bb..448f6801bd78 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11575,6 +11575,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 8ff61be1a29f..34ef4b60adc6 100644
--- a/arch/arm/mach-keystone/Kconfig
+++ b/arch/arm/mach-keystone/Kconfig
@@ -11,6 +11,7 @@ config ARCH_KEYSTONE
 	select MIGHT_HAVE_PCI
 	select PCI_DOMAINS if PCI
 	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..ab6b20201731
--- /dev/null
+++ b/drivers/soc/ti/ti_sci_pm_domains.c
@@ -0,0 +1,222 @@
+/*
+ * 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>
+
+#define TI_GENPD_NAME_LENGTH	16
+
+/**
+ * struct ti_sci_genpd_data: holds data needed for every 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_list_mutex: Mutex for protecting the global list of power domains
+ * @pd_list: list that hols all power domains as they are allocated
+ */
+struct ti_sci_genpd_data {
+	const struct ti_sci_handle *ti_sci;
+	struct device *dev;
+	struct mutex pd_list_mutex; /* Protect master list of domains */
+	struct list_head pd_list;
+};
+
+/**
+ * struct ti_sci_pm_domain: TI specific data needed for each power domain
+ * @idx: index of the power domain that identifies it with the system
+ *	 control processor.
+ * @pd: generic_pm_domain for use with the genpd framework
+ * @node: list_head for tracking all domains in a list
+ * @parent: pointer to the global data defined for all domains
+ */
+struct ti_sci_pm_domain {
+	int idx;
+	struct generic_pm_domain pd;
+	struct list_head node;
+	struct ti_sci_genpd_data *parent;
+};
+
+#define genpd_to_ti_sci_pd(gpd) container_of(gpd, struct ti_sci_pm_domain, pd)
+
+/**
+ * pd_power_off(): generic_pm_domain power_off hook
+ * @domain: generic_pm_domain struct provided by genpd framework of the
+ *	    pd to be shut off
+ *
+ * This hook uses the put_device dev_op provided by ti_sci to power off the
+ * device associated to the power domain provided.
+ */
+static int pd_power_off(struct generic_pm_domain *domain)
+{
+	struct ti_sci_pm_domain *ti_sci_pd = genpd_to_ti_sci_pd(domain);
+	const struct ti_sci_handle *ti_sci = ti_sci_pd->parent->ti_sci;
+
+	return ti_sci->ops.dev_ops.put_device(ti_sci,
+					      ti_sci_pd->idx);
+}
+
+/**
+ * pd_power_on(): generic_pm_domain power_on hook
+ * @domain: generic_pm_domain struct provided by genpd framework of the
+ *	    pd to be powered on
+ *
+ * This hook uses the get_device dev_op provided by ti_sci to power on the
+ * device associated to the power domain provided.
+ */
+static int pd_power_on(struct generic_pm_domain *domain)
+{
+	struct ti_sci_pm_domain *ti_sci_pd = genpd_to_ti_sci_pd(domain);
+	const struct ti_sci_handle *ti_sci = ti_sci_pd->parent->ti_sci;
+
+	return ti_sci->ops.dev_ops.get_device(ti_sci,
+					      ti_sci_pd->idx);
+}
+
+/**
+ * of_ti_sci_genpd_xlate_onecell() - Xlate function using a single index.
+ * @genpdspec: OF phandle args to define an index to be communicated over
+ *	       TI SCI to the system control processor to identify it
+ * @data: xlate function private data - pointer to the ti_sci_genpd_data
+ *	  struct containing global sci pm domain data
+ *
+ * This is xlate function takes a single cell as an index representing the
+ * id to be passed to the system control processor. As each device in the
+ * device tree probes a single pm domain will be created specifically for it
+ * based on the index passed in the pweor-domains property. If no pm domain
+ * yet exists for that index it is created, otherwise it is looked up and
+ * returned. Through this 1 to 1 association between power domains and
+ * devices the genpd framework will work to directly control devices
+ * through the pm_runtime framework.
+ */
+static struct generic_pm_domain *of_ti_sci_genpd_xlate_onecell(
+					struct of_phandle_args *genpdspec,
+					void *data)
+{
+	struct ti_sci_genpd_data *ti_sci_genpd = data;
+	const struct ti_sci_handle *ti_sci = ti_sci_genpd->ti_sci;
+	struct device *dev = ti_sci_genpd->dev;
+	unsigned int idx = genpdspec->args[0];
+	struct ti_sci_pm_domain *ti_sci_pd = NULL, *pd;
+	char *name;
+	int ret = 0;
+
+	if (genpdspec->args_count != 1)
+		return ERR_PTR(-EINVAL);
+
+	/*
+	 * 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 ERR_PTR(-EINVAL);
+
+	mutex_lock(&ti_sci_genpd->pd_list_mutex);
+	list_for_each_entry(pd, &ti_sci_genpd->pd_list, node) {
+		if (pd->idx == idx) {
+			ti_sci_pd = pd;
+			goto unlock_and_return;
+		}
+	}
+
+	ti_sci_pd = devm_kzalloc(dev,
+				 sizeof(*ti_sci_pd),
+				 GFP_KERNEL);
+	if (!ti_sci_pd) {
+		ret = -ENOMEM;
+		goto unlock_and_return;
+	}
+
+	ti_sci_pd->idx = idx;
+	ti_sci_pd->pd.power_off = pd_power_off;
+	ti_sci_pd->pd.power_on = pd_power_on;
+
+	name = devm_kzalloc(dev, TI_GENPD_NAME_LENGTH, GFP_KERNEL);
+	if (!name) {
+		devm_kfree(dev, ti_sci_pd);
+		ret = -ENOMEM;
+		goto unlock_and_return;
+	}
+
+	snprintf(name, TI_GENPD_NAME_LENGTH, "pd-%d", idx);
+	ti_sci_pd->pd.name = name;
+
+	ti_sci_pd->parent = ti_sci_genpd;
+	/*
+	 * Init each pd as is_off so we always call pd_power_on
+	 * to make sure reference counting is properly maintained
+	 * on the SCI side
+	 */
+	pm_genpd_init(&ti_sci_pd->pd, NULL, true);
+
+	list_add(&ti_sci_pd->node, &ti_sci_genpd->pd_list);
+
+unlock_and_return:
+	mutex_unlock(&ti_sci_genpd->pd_list_mutex);
+
+	if (ret)
+		return ERR_PTR(ret);
+	else
+		return &ti_sci_pd->pd;
+}
+
+static const struct of_device_id ti_sci_pm_domain_matches[] = {
+	{ .compatible = "ti,sci-pm-domains", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, ti_sci_pm_domain_matches);
+
+static int ti_sci_pm_domains_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct ti_sci_genpd_data *ti_sci_genpd;
+
+	ti_sci_genpd = devm_kzalloc(dev, sizeof(*ti_sci_genpd), GFP_KERNEL);
+	if (!ti_sci_genpd)
+		return -ENOMEM;
+
+	ti_sci_genpd->ti_sci = devm_ti_sci_get_handle(dev);
+	if (IS_ERR(ti_sci_genpd->ti_sci))
+		return PTR_ERR(ti_sci_genpd->ti_sci);
+
+	ti_sci_genpd->dev = dev;
+
+	INIT_LIST_HEAD(&ti_sci_genpd->pd_list);
+	mutex_init(&ti_sci_genpd->pd_list_mutex);
+
+	return __of_genpd_add_provider(np, of_ti_sci_genpd_xlate_onecell,
+				       ti_sci_genpd);
+}
+
+static struct platform_driver ti_sci_pm_domains_driver = {
+	.probe = ti_sci_pm_domains_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.9.1.200.gb1ec08f

Re: [PATCH 3/3] soc: ti: Add ti_sci_pm_domains driver

[Ulf Hansson]

+ Jon

[...]

> +
> +static int ti_sci_pm_domains_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct device_node *np = dev->of_node;
> +       struct ti_sci_genpd_data *ti_sci_genpd;
> +
> +       ti_sci_genpd = devm_kzalloc(dev, sizeof(*ti_sci_genpd), GFP_KERNEL);
> +       if (!ti_sci_genpd)
> +               return -ENOMEM;
> +
> +       ti_sci_genpd->ti_sci = devm_ti_sci_get_handle(dev);
> +       if (IS_ERR(ti_sci_genpd->ti_sci))
> +               return PTR_ERR(ti_sci_genpd->ti_sci);
> +
> +       ti_sci_genpd->dev = dev;
> +
> +       INIT_LIST_HEAD(&ti_sci_genpd->pd_list);
> +       mutex_init(&ti_sci_genpd->pd_list_mutex);
> +
> +       return __of_genpd_add_provider(np, of_ti_sci_genpd_xlate_onecell,
> +                                      ti_sci_genpd);

Jon Hunter are working on adding robust method to be able to remove
initialized genpds [1].

In that series we intend to remove the __of_genpd_add_provider() API,
and instead only have of_genpd_add_provider_onecell() and
of_genpd_add_provider_simple(). Could you please convert to use any of
these APIs instead?

Kind regards
Uffe

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

Re: [PATCH 2/3] dt-bindings: genpd: Add K2G device definitions

[Ulf Hansson]

On 20 August 2016 at 01:56, Nishanth Menon <nm@ti.com> wrote:
> From: Dave Gerlach <d-gerlach@ti.com>
>
> Provide macros representing each devices 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. Includes
> contributions from Peter Ujfalusi <peter.ujfalusi@ti.com>
>
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> Signed-off-by: Nishanth Menon <nm@ti.com>

I would squash this change into patch 1/3, as the DT documentation
patch refers to dt-bindings/genpd/k2g.h which is added in @subject
patch.

Otherwise feel free to add my tag below, which applied to patch 1/3 as well.

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe

> ---
>  MAINTAINERS                     |  1 +
>  include/dt-bindings/genpd/k2g.h | 90 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 91 insertions(+)
>  create mode 100644 include/dt-bindings/genpd/k2g.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 657fb6a10286..ea14b08c30bb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11574,6 +11574,7 @@ 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.9.1.200.gb1ec08f
>

Re: [PATCH 3/3] soc: ti: Add ti_sci_pm_domains driver

[Dave Gerlach]

Hi,
On 08/25/2016 02:27 AM, Ulf Hansson wrote:
> + Jon
>
> [...]
>
>> +
>> +static int ti_sci_pm_domains_probe(struct platform_device *pdev)
>> +{
>> +       struct device *dev = &pdev->dev;
>> +       struct device_node *np = dev->of_node;
>> +       struct ti_sci_genpd_data *ti_sci_genpd;
>> +
>> +       ti_sci_genpd = devm_kzalloc(dev, sizeof(*ti_sci_genpd), GFP_KERNEL);
>> +       if (!ti_sci_genpd)
>> +               return -ENOMEM;
>> +
>> +       ti_sci_genpd->ti_sci = devm_ti_sci_get_handle(dev);
>> +       if (IS_ERR(ti_sci_genpd->ti_sci))
>> +               return PTR_ERR(ti_sci_genpd->ti_sci);
>> +
>> +       ti_sci_genpd->dev = dev;
>> +
>> +       INIT_LIST_HEAD(&ti_sci_genpd->pd_list);
>> +       mutex_init(&ti_sci_genpd->pd_list_mutex);
>> +
>> +       return __of_genpd_add_provider(np, of_ti_sci_genpd_xlate_onecell,
>> +                                      ti_sci_genpd);
>
> Jon Hunter are working on adding robust method to be able to remove
> initialized genpds [1].
>
> In that series we intend to remove the __of_genpd_add_provider() API,
> and instead only have of_genpd_add_provider_onecell() and
> of_genpd_add_provider_simple(). Could you please convert to use any of
> these APIs instead?

Thanks for pointing this out. I took at look at the series you've linked and the
short answer is that I see no good way to directly convert what we've done to
use those APIs.

On this platform each device has it's power state controlled through the SCI
protocol described in the cover letter. The system makes a request for powering
on or off the device using a unique ID number for each device, as provided in
patches 1 and 2. These operations map to those of a genpd, so we decided to do a
1:1 device to genpd mapping, where each device has it's own genpd.

The split that we took from the provided simple and onecell xlate functions
arises from this mapping. The IDs are not necessarily linear and also they are
not necessarily defined in a fixed way for all SoCs, they are entirely data
driven based on the provided device ID. To make use of these IDs, I created a
new xlate function that takes a onecell value but instead dynamically allocates
a new genpd, at probe time, to give us a genpd that contains the necessary SoC
specific data for that device that probed and is mapped directly to the device.
This lets us only create the genpds we need without having to redefine a static
list of all possible genpds and duplicate the data.

So my question back would be, how critical is it to be able to drop the ability
to provide custom xlate functions?

Regards,
Dave

>
> Kind regards
> Uffe
>
> [1]
> http://www.spinics.net/lists/arm-kernel/msg524151.html
>

Re: [PATCH 3/3] soc: ti: Add ti_sci_pm_domains driver

[Dave Gerlach]

Jon, Ulf,
On 08/26/2016 06:37 PM, Dave Gerlach wrote:
> Hi,
> On 08/25/2016 02:27 AM, Ulf Hansson wrote:
>> + Jon
>>
>> [...]
>>
>>> +
>>> +static int ti_sci_pm_domains_probe(struct platform_device *pdev)
>>> +{
>>> +       struct device *dev = &pdev->dev;
>>> +       struct device_node *np = dev->of_node;
>>> +       struct ti_sci_genpd_data *ti_sci_genpd;
>>> +
>>> +       ti_sci_genpd = devm_kzalloc(dev, sizeof(*ti_sci_genpd), GFP_KERNEL);
>>> +       if (!ti_sci_genpd)
>>> +               return -ENOMEM;
>>> +
>>> +       ti_sci_genpd->ti_sci = devm_ti_sci_get_handle(dev);
>>> +       if (IS_ERR(ti_sci_genpd->ti_sci))
>>> +               return PTR_ERR(ti_sci_genpd->ti_sci);
>>> +
>>> +       ti_sci_genpd->dev = dev;
>>> +
>>> +       INIT_LIST_HEAD(&ti_sci_genpd->pd_list);
>>> +       mutex_init(&ti_sci_genpd->pd_list_mutex);
>>> +
>>> +       return __of_genpd_add_provider(np, of_ti_sci_genpd_xlate_onecell,
>>> +                                      ti_sci_genpd);
>>
>> Jon Hunter are working on adding robust method to be able to remove
>> initialized genpds [1].
>>
>> In that series we intend to remove the __of_genpd_add_provider() API,
>> and instead only have of_genpd_add_provider_onecell() and
>> of_genpd_add_provider_simple(). Could you please convert to use any of
>> these APIs instead?
>
> Thanks for pointing this out. I took at look at the series you've linked and the
> short answer is that I see no good way to directly convert what we've done to
> use those APIs.
>
> On this platform each device has it's power state controlled through the SCI
> protocol described in the cover letter. The system makes a request for powering
> on or off the device using a unique ID number for each device, as provided in
> patches 1 and 2. These operations map to those of a genpd, so we decided to do a
> 1:1 device to genpd mapping, where each device has it's own genpd.
>
> The split that we took from the provided simple and onecell xlate functions
> arises from this mapping. The IDs are not necessarily linear and also they are
> not necessarily defined in a fixed way for all SoCs, they are entirely data
> driven based on the provided device ID. To make use of these IDs, I created a
> new xlate function that takes a onecell value but instead dynamically allocates
> a new genpd, at probe time, to give us a genpd that contains the necessary SoC
> specific data for that device that probed and is mapped directly to the device.
> This lets us only create the genpds we need without having to redefine a static
> list of all possible genpds and duplicate the data.
>
> So my question back would be, how critical is it to be able to drop the ability
> to provide custom xlate functions?

After thinking about this a bit more, I believe I see a way we can use 
the of_genpd_add_provider_onecell, although not optimally. The device 
IDs, and therefore the genpd IDs (in the last email I mentioned we are 
doing a 1:1 device to genpd mapping) are fixed and defined by the 
hardware, and are not linear, there can be gaps and we won't necessarily 
always start at 0 even though we do on this SoC. However, we could build 
an array of genpds that map to our IDs similar to how several have done 
it, like in drivers/soc/bcm/raspberrypi-power.c, but because our IDs can 
have gaps, there will be unused struct generic_pm_domains that get 
allocated and are never touched because the index of the element doesn't 
correspond to a genpd ID.

Based on the ID set provided in patch 2 of this series I see 12 gaps, so 
we'd be wasting space the size of 12 genpds. The ID mapping will change 
on future SoCs, so this number could be larger. Do you think this is an 
acceptable solution? It allows us to play nice with the new genpd 
framework changes at the cost of wasting some space allocated to filler 
genpds.

Regards,
Dave

>
> Regards,
> Dave
>
>>
>> Kind regards
>> Uffe
>>
>> [1]
>> http://www.spinics.net/lists/arm-kernel/msg524151.html
>>
>

Re: [PATCH 3/3] soc: ti: Add ti_sci_pm_domains driver

[Ulf Hansson]

On 30 August 2016 at 21:43, Dave Gerlach <d-gerlach@ti.com> wrote:
> Jon, Ulf,
>
> On 08/26/2016 06:37 PM, Dave Gerlach wrote:
>>
>> Hi,
>> On 08/25/2016 02:27 AM, Ulf Hansson wrote:
>>>
>>> + Jon
>>>
>>> [...]
>>>
>>>> +
>>>> +static int ti_sci_pm_domains_probe(struct platform_device *pdev)
>>>> +{
>>>> +       struct device *dev = &pdev->dev;
>>>> +       struct device_node *np = dev->of_node;
>>>> +       struct ti_sci_genpd_data *ti_sci_genpd;
>>>> +
>>>> +       ti_sci_genpd = devm_kzalloc(dev, sizeof(*ti_sci_genpd),
>>>> GFP_KERNEL);
>>>> +       if (!ti_sci_genpd)
>>>> +               return -ENOMEM;
>>>> +
>>>> +       ti_sci_genpd->ti_sci = devm_ti_sci_get_handle(dev);
>>>> +       if (IS_ERR(ti_sci_genpd->ti_sci))
>>>> +               return PTR_ERR(ti_sci_genpd->ti_sci);
>>>> +
>>>> +       ti_sci_genpd->dev = dev;
>>>> +
>>>> +       INIT_LIST_HEAD(&ti_sci_genpd->pd_list);
>>>> +       mutex_init(&ti_sci_genpd->pd_list_mutex);
>>>> +
>>>> +       return __of_genpd_add_provider(np,
>>>> of_ti_sci_genpd_xlate_onecell,
>>>> +                                      ti_sci_genpd);
>>>
>>>
>>> Jon Hunter are working on adding robust method to be able to remove
>>> initialized genpds [1].
>>>
>>> In that series we intend to remove the __of_genpd_add_provider() API,
>>> and instead only have of_genpd_add_provider_onecell() and
>>> of_genpd_add_provider_simple(). Could you please convert to use any of
>>> these APIs instead?
>>
>>
>> Thanks for pointing this out. I took at look at the series you've linked
>> and the
>> short answer is that I see no good way to directly convert what we've done
>> to
>> use those APIs.
>>
>> On this platform each device has it's power state controlled through the
>> SCI
>> protocol described in the cover letter. The system makes a request for
>> powering
>> on or off the device using a unique ID number for each device, as provided
>> in
>> patches 1 and 2. These operations map to those of a genpd, so we decided
>> to do a
>> 1:1 device to genpd mapping, where each device has it's own genpd.
>>
>> The split that we took from the provided simple and onecell xlate
>> functions
>> arises from this mapping. The IDs are not necessarily linear and also they
>> are
>> not necessarily defined in a fixed way for all SoCs, they are entirely
>> data
>> driven based on the provided device ID. To make use of these IDs, I
>> created a
>> new xlate function that takes a onecell value but instead dynamically
>> allocates
>> a new genpd, at probe time, to give us a genpd that contains the necessary
>> SoC
>> specific data for that device that probed and is mapped directly to the
>> device.
>> This lets us only create the genpds we need without having to redefine a
>> static
>> list of all possible genpds and duplicate the data.
>>
>> So my question back would be, how critical is it to be able to drop the
>> ability
>> to provide custom xlate functions?
>
>
> After thinking about this a bit more, I believe I see a way we can use the
> of_genpd_add_provider_onecell, although not optimally. The device IDs, and
> therefore the genpd IDs (in the last email I mentioned we are doing a 1:1
> device to genpd mapping) are fixed and defined by the hardware, and are not
> linear, there can be gaps and we won't necessarily always start at 0 even
> though we do on this SoC. However, we could build an array of genpds that
> map to our IDs similar to how several have done it, like in
> drivers/soc/bcm/raspberrypi-power.c, but because our IDs can have gaps,
> there will be unused struct generic_pm_domains that get allocated and are
> never touched because the index of the element doesn't correspond to a genpd
> ID.

There are a couple of ways to solve your problem without having to
create one genpd instance for each of the platform devices. As a
matter of fact, I think that approach should be avoided if possible.
Genpd isn't designed for these kind purposes, even if it can be done
like that.

Here are some ideas which could be used to solve the problem differently.

*)
Assuming each platform device has a driver for it!?
Then why don't you implement the ->runtime_suspend|resume() callbacks
on the driver level and call the SCI interface to power on/off the
device from there? This ought to be the most straight forward
solution.


**)
You may also use genpd's (struct generic_pm_domain)
->attach|detach_dev() callbacks to create the device specific data
(the SCI device ID). The ->attach_dev() callback are invoked when a
device gets attached to its PM domain (genpd) at probe time.

Currently these callbacks are already being used by SoCs that uses the
PM clk API from a genpd client. That is needed to create the device
specific PM clock list. We would have to do something similar here for
the SCI device IDs.

Then, you must also assign/implement the ->start() and ->stop()
callbacks of the struct gpd_dev_ops, which is a part of the genpd
struct. These callbacks can then easily invoke SoC specific code/APIs
and perhaps that is the issue with my first suggested approach!?

>
> Based on the ID set provided in patch 2 of this series I see 12 gaps, so
> we'd be wasting space the size of 12 genpds. The ID mapping will change on
> future SoCs, so this number could be larger. Do you think this is an
> acceptable solution? It allows us to play nice with the new genpd framework
> changes at the cost of wasting some space allocated to filler genpds.

There are other issues as well, which mostly has do to with a
unnecessary long execution path, involving taking mutexes etc in
genpd.

All you actually need, is to be able to power on/off a device from a
driver's ->runtime_suspend|resume() callback. Don't you think?

Kind regards
Uffe

Re: [PATCH 1/3] Documentation: dt: Add TI-SCI PM Domains

[Rob Herring]

On Fri, Aug 19, 2016 at 06:56:51PM -0500, Nishanth Menon wrote:
> From: Dave Gerlach <d-gerlach@ti.com>
> 
> Add a generic power domain implementation, TI SCI PM Domains, that
> will hook into the genpd framework and allow each PD, which will be
> created one per device, to be managed over the TI-SCI protocol.
> 
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
>  .../devicetree/bindings/soc/ti/sci-pm-domain.txt   | 58 ++++++++++++++++++++++
>  MAINTAINERS                                        |  1 +
>  2 files changed, 59 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
> 
> 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..059a5d71d692
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
> @@ -0,0 +1,58 @@
> +Texas Instruments TI-SCI Generic Power Domain
> +---------------------------------------------
> +
> +Some TI SoCs contain a system controller (like the PMMC, etc...) that are
> +responsible for control 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 Domains Nodes
> +================
> +The PM domains node represents the global PM domain managed by the PMMC,
> +which in this case is one cell implementation as documented by the generic
> +PM domain bindings in
> +Documentation/devicetree/bindings/power/power_domain.txt.
> +
> +Required Properties:
> +--------------------
> +- compatible: should be "ti,sci-pm-domains"
> +- #power-domain-cells: Must be 1 so that an offset can be provided in each
> +		       device node.
> +- ti,sci: Phandle to the TI SCI device to use for managing the devices

How about just making this a child node of the SCI device?

Rob

Re: [PATCH 3/3] soc: ti: Add ti_sci_pm_domains driver

[Dave Gerlach]

On 08/30/2016 03:26 PM, Ulf Hansson wrote:
> On 30 August 2016 at 21:43, Dave Gerlach <d-gerlach@ti.com> wrote:
>> Jon, Ulf,
>>
>> On 08/26/2016 06:37 PM, Dave Gerlach wrote:
>>>
>>> Hi,
>>> On 08/25/2016 02:27 AM, Ulf Hansson wrote:
>>>>
>>>> + Jon
>>>>
>>>> [...]
>>>>
>>>>> +
>>>>> +static int ti_sci_pm_domains_probe(struct platform_device *pdev)
>>>>> +{
>>>>> +       struct device *dev = &pdev->dev;
>>>>> +       struct device_node *np = dev->of_node;
>>>>> +       struct ti_sci_genpd_data *ti_sci_genpd;
>>>>> +
>>>>> +       ti_sci_genpd = devm_kzalloc(dev, sizeof(*ti_sci_genpd),
>>>>> GFP_KERNEL);
>>>>> +       if (!ti_sci_genpd)
>>>>> +               return -ENOMEM;
>>>>> +
>>>>> +       ti_sci_genpd->ti_sci = devm_ti_sci_get_handle(dev);
>>>>> +       if (IS_ERR(ti_sci_genpd->ti_sci))
>>>>> +               return PTR_ERR(ti_sci_genpd->ti_sci);
>>>>> +
>>>>> +       ti_sci_genpd->dev = dev;
>>>>> +
>>>>> +       INIT_LIST_HEAD(&ti_sci_genpd->pd_list);
>>>>> +       mutex_init(&ti_sci_genpd->pd_list_mutex);
>>>>> +
>>>>> +       return __of_genpd_add_provider(np,
>>>>> of_ti_sci_genpd_xlate_onecell,
>>>>> +                                      ti_sci_genpd);
>>>>
>>>>
>>>> Jon Hunter are working on adding robust method to be able to remove
>>>> initialized genpds [1].
>>>>
>>>> In that series we intend to remove the __of_genpd_add_provider() API,
>>>> and instead only have of_genpd_add_provider_onecell() and
>>>> of_genpd_add_provider_simple(). Could you please convert to use any of
>>>> these APIs instead?
>>>
>>>
>>> Thanks for pointing this out. I took at look at the series you've linked
>>> and the
>>> short answer is that I see no good way to directly convert what we've done
>>> to
>>> use those APIs.
>>>
>>> On this platform each device has it's power state controlled through the
>>> SCI
>>> protocol described in the cover letter. The system makes a request for
>>> powering
>>> on or off the device using a unique ID number for each device, as provided
>>> in
>>> patches 1 and 2. These operations map to those of a genpd, so we decided
>>> to do a
>>> 1:1 device to genpd mapping, where each device has it's own genpd.
>>>
>>> The split that we took from the provided simple and onecell xlate
>>> functions
>>> arises from this mapping. The IDs are not necessarily linear and also they
>>> are
>>> not necessarily defined in a fixed way for all SoCs, they are entirely
>>> data
>>> driven based on the provided device ID. To make use of these IDs, I
>>> created a
>>> new xlate function that takes a onecell value but instead dynamically
>>> allocates
>>> a new genpd, at probe time, to give us a genpd that contains the necessary
>>> SoC
>>> specific data for that device that probed and is mapped directly to the
>>> device.
>>> This lets us only create the genpds we need without having to redefine a
>>> static
>>> list of all possible genpds and duplicate the data.
>>>
>>> So my question back would be, how critical is it to be able to drop the
>>> ability
>>> to provide custom xlate functions?
>>
>>
>> After thinking about this a bit more, I believe I see a way we can use the
>> of_genpd_add_provider_onecell, although not optimally. The device IDs, and
>> therefore the genpd IDs (in the last email I mentioned we are doing a 1:1
>> device to genpd mapping) are fixed and defined by the hardware, and are not
>> linear, there can be gaps and we won't necessarily always start at 0 even
>> though we do on this SoC. However, we could build an array of genpds that
>> map to our IDs similar to how several have done it, like in
>> drivers/soc/bcm/raspberrypi-power.c, but because our IDs can have gaps,
>> there will be unused struct generic_pm_domains that get allocated and are
>> never touched because the index of the element doesn't correspond to a genpd
>> ID.
>
> There are a couple of ways to solve your problem without having to
> create one genpd instance for each of the platform devices. As a
> matter of fact, I think that approach should be avoided if possible.
> Genpd isn't designed for these kind purposes, even if it can be done
> like that.
>

Yes, ok. For K2G and future devices, the kernel will just request the 
device over the TI_SCI protocol and the firmware will guarantee 
everything that it needs it on. So we thought the 1:1 device to genpd 
mapping made sense in that regard, but maybe it is a stretch of the 
framework in the wrong way.

> Here are some ideas which could be used to solve the problem differently.
>
> *)
> Assuming each platform device has a driver for it!?
> Then why don't you implement the ->runtime_suspend|resume() callbacks
> on the driver level and call the SCI interface to power on/off the
> device from there? This ought to be the most straight forward
> solution.

Straightforward yes but not a realistic option as we are using shared 
drivers from other platforms so sticking in platform specific code won't 
work.

>
>
> **)
> You may also use genpd's (struct generic_pm_domain)
> ->attach|detach_dev() callbacks to create the device specific data
> (the SCI device ID). The ->attach_dev() callback are invoked when a
> device gets attached to its PM domain (genpd) at probe time.
>
> Currently these callbacks are already being used by SoCs that uses the
> PM clk API from a genpd client. That is needed to create the device
> specific PM clock list. We would have to do something similar here for
> the SCI device IDs.
>
> Then, you must also assign/implement the ->start() and ->stop()
> callbacks of the struct gpd_dev_ops, which is a part of the genpd
> struct. These callbacks can then easily invoke SoC specific code/APIs
> and perhaps that is the issue with my first suggested approach!?

I've taken a look at what you have suggested here and I think it could 
work for us, thanks for the suggestion, I will give it a shot, I think 
that this will work just as well from a functional perspective.

>
>>
>> Based on the ID set provided in patch 2 of this series I see 12 gaps, so
>> we'd be wasting space the size of 12 genpds. The ID mapping will change on
>> future SoCs, so this number could be larger. Do you think this is an
>> acceptable solution? It allows us to play nice with the new genpd framework
>> changes at the cost of wasting some space allocated to filler genpds.
>
> There are other issues as well, which mostly has do to with a
> unnecessary long execution path, involving taking mutexes etc in
> genpd.
>
> All you actually need, is to be able to power on/off a device from a
> driver's ->runtime_suspend|resume() callback. Don't you think?

Yes, but I thought the point of these frameworks was that they let us 
avoid doing it manually with platform specific code inside the drivers. 
I'll look at the callbacks in the genpd framework instead, that seems 
like a good place to do it.

Regards,
Dave

>
> Kind regards
> Uffe
>

Re: [PATCH 3/3] soc: ti: Add ti_sci_pm_domains driver

[Kevin Hilman]

Dave Gerlach <d-gerlach@ti.com> writes:

> On 08/30/2016 03:26 PM, Ulf Hansson wrote:
>> On 30 August 2016 at 21:43, Dave Gerlach <d-gerlach@ti.com> wrote:
>>> Jon, Ulf,
>>>
>>> On 08/26/2016 06:37 PM, Dave Gerlach wrote:
>>>>
>>>> Hi,
>>>> On 08/25/2016 02:27 AM, Ulf Hansson wrote:
>>>>>
>>>>> + Jon
>>>>>
>>>>> [...]
>>>>>
>>>>>> +
>>>>>> +static int ti_sci_pm_domains_probe(struct platform_device *pdev)
>>>>>> +{
>>>>>> +       struct device *dev = &pdev->dev;
>>>>>> +       struct device_node *np = dev->of_node;
>>>>>> +       struct ti_sci_genpd_data *ti_sci_genpd;
>>>>>> +
>>>>>> +       ti_sci_genpd = devm_kzalloc(dev, sizeof(*ti_sci_genpd),
>>>>>> GFP_KERNEL);
>>>>>> +       if (!ti_sci_genpd)
>>>>>> +               return -ENOMEM;
>>>>>> +
>>>>>> +       ti_sci_genpd->ti_sci = devm_ti_sci_get_handle(dev);
>>>>>> +       if (IS_ERR(ti_sci_genpd->ti_sci))
>>>>>> +               return PTR_ERR(ti_sci_genpd->ti_sci);
>>>>>> +
>>>>>> +       ti_sci_genpd->dev = dev;
>>>>>> +
>>>>>> +       INIT_LIST_HEAD(&ti_sci_genpd->pd_list);
>>>>>> +       mutex_init(&ti_sci_genpd->pd_list_mutex);
>>>>>> +
>>>>>> +       return __of_genpd_add_provider(np,
>>>>>> of_ti_sci_genpd_xlate_onecell,
>>>>>> +                                      ti_sci_genpd);
>>>>>
>>>>>
>>>>> Jon Hunter are working on adding robust method to be able to remove
>>>>> initialized genpds [1].
>>>>>
>>>>> In that series we intend to remove the __of_genpd_add_provider() API,
>>>>> and instead only have of_genpd_add_provider_onecell() and
>>>>> of_genpd_add_provider_simple(). Could you please convert to use any of
>>>>> these APIs instead?
>>>>
>>>>
>>>> Thanks for pointing this out. I took at look at the series you've linked
>>>> and the
>>>> short answer is that I see no good way to directly convert what we've done
>>>> to
>>>> use those APIs.
>>>>
>>>> On this platform each device has it's power state controlled through the
>>>> SCI
>>>> protocol described in the cover letter. The system makes a request for
>>>> powering
>>>> on or off the device using a unique ID number for each device, as provided
>>>> in
>>>> patches 1 and 2. These operations map to those of a genpd, so we decided
>>>> to do a
>>>> 1:1 device to genpd mapping, where each device has it's own genpd.
>>>>
>>>> The split that we took from the provided simple and onecell xlate
>>>> functions
>>>> arises from this mapping. The IDs are not necessarily linear and also they
>>>> are
>>>> not necessarily defined in a fixed way for all SoCs, they are entirely
>>>> data
>>>> driven based on the provided device ID. To make use of these IDs, I
>>>> created a
>>>> new xlate function that takes a onecell value but instead dynamically
>>>> allocates
>>>> a new genpd, at probe time, to give us a genpd that contains the necessary
>>>> SoC
>>>> specific data for that device that probed and is mapped directly to the
>>>> device.
>>>> This lets us only create the genpds we need without having to redefine a
>>>> static
>>>> list of all possible genpds and duplicate the data.
>>>>
>>>> So my question back would be, how critical is it to be able to drop the
>>>> ability
>>>> to provide custom xlate functions?
>>>
>>>
>>> After thinking about this a bit more, I believe I see a way we can use the
>>> of_genpd_add_provider_onecell, although not optimally. The device IDs, and
>>> therefore the genpd IDs (in the last email I mentioned we are doing a 1:1
>>> device to genpd mapping) are fixed and defined by the hardware, and are not
>>> linear, there can be gaps and we won't necessarily always start at 0 even
>>> though we do on this SoC. However, we could build an array of genpds that
>>> map to our IDs similar to how several have done it, like in
>>> drivers/soc/bcm/raspberrypi-power.c, but because our IDs can have gaps,
>>> there will be unused struct generic_pm_domains that get allocated and are
>>> never touched because the index of the element doesn't correspond to a genpd
>>> ID.
>>
>> There are a couple of ways to solve your problem without having to
>> create one genpd instance for each of the platform devices. As a
>> matter of fact, I think that approach should be avoided if possible.
>> Genpd isn't designed for these kind purposes, even if it can be done
>> like that.
>>
>
> Yes, ok. For K2G and future devices, the kernel will just request the
> device over the TI_SCI protocol and the firmware will guarantee
> everything that it needs it on. So we thought the 1:1 device to genpd
> mapping made sense in that regard, but maybe it is a stretch of the
> framework in the wrong way.
>
>> Here are some ideas which could be used to solve the problem differently.
>>
>> *)
>> Assuming each platform device has a driver for it!?
>> Then why don't you implement the ->runtime_suspend|resume() callbacks
>> on the driver level and call the SCI interface to power on/off the
>> device from there? This ought to be the most straight forward
>> solution.
>
> Straightforward yes but not a realistic option as we are using shared
> drivers from other platforms so sticking in platform specific code
> won't work.
>
>>
>>
>> **)
>> You may also use genpd's (struct generic_pm_domain)
>> ->attach|detach_dev() callbacks to create the device specific data
>> (the SCI device ID). The ->attach_dev() callback are invoked when a
>> device gets attached to its PM domain (genpd) at probe time.
>>
>> Currently these callbacks are already being used by SoCs that uses the
>> PM clk API from a genpd client. That is needed to create the device
>> specific PM clock list. We would have to do something similar here for
>> the SCI device IDs.
>>
>> Then, you must also assign/implement the ->start() and ->stop()
>> callbacks of the struct gpd_dev_ops, which is a part of the genpd
>> struct. These callbacks can then easily invoke SoC specific code/APIs
>> and perhaps that is the issue with my first suggested approach!?
>
> I've taken a look at what you have suggested here and I think it could
> work for us, thanks for the suggestion, I will give it a shot, I think
> that this will work just as well from a functional perspective.
>
>>
>>>
>>> Based on the ID set provided in patch 2 of this series I see 12 gaps, so
>>> we'd be wasting space the size of 12 genpds. The ID mapping will change on
>>> future SoCs, so this number could be larger. Do you think this is an
>>> acceptable solution? It allows us to play nice with the new genpd framework
>>> changes at the cost of wasting some space allocated to filler genpds.
>>
>> There are other issues as well, which mostly has do to with a
>> unnecessary long execution path, involving taking mutexes etc in
>> genpd.
>>
>> All you actually need, is to be able to power on/off a device from a
>> driver's ->runtime_suspend|resume() callback. Don't you think?
>
> Yes, but I thought the point of these frameworks was that they let us
> avoid doing it manually with platform specific code inside the
> drivers. I'll look at the callbacks in the genpd framework instead,
> that seems like a good place to do it.

One more idea...

Since you don't really have a domain (a group of devices), what you
really have is each device having an independent power switch, so as Ulf
suggested, what you really need is for all the devices to share the same
set of runtime PM callbacks that call SCI.  The only difference is the
unique ID.

Rather than using all of genpd, you could also just use a pm_domain
which is what genpd is built on top of (and also omap_device, which
you're probably familiar with also.)

That would allow you to keep the drivers completely generic, yet share
all the SCI specific "domain" code inside a pm_domain.

Kevin

Re: [PATCH 3/3] soc: ti: Add ti_sci_pm_domains driver

[Ulf Hansson]

>
>> Here are some ideas which could be used to solve the problem differently.
>>
>> *)
>> Assuming each platform device has a driver for it!?
>> Then why don't you implement the ->runtime_suspend|resume() callbacks
>> on the driver level and call the SCI interface to power on/off the
>> device from there? This ought to be the most straight forward
>> solution.
>
>
> Straightforward yes but not a realistic option as we are using shared
> drivers from other platforms so sticking in platform specific code won't
> work.

Agree, but...

Historically, I think this was *one* of the reasons to why omap's
hw_mod PM domain was invented.
At that point we did not have the common clk framework, the pinctrl
framework, the phy framework, syscon, etc. These device resources can
now be handled by the drivers themselves via generic interfaces and
thus they remains to be portable.

My point is, let's be sure you don't drop this approach unless it's
really SoC specific code needed to deal with the device.

>
>>
>>
>> **)
>> You may also use genpd's (struct generic_pm_domain)
>> ->attach|detach_dev() callbacks to create the device specific data
>> (the SCI device ID). The ->attach_dev() callback are invoked when a
>> device gets attached to its PM domain (genpd) at probe time.
>>
>> Currently these callbacks are already being used by SoCs that uses the
>> PM clk API from a genpd client. That is needed to create the device
>> specific PM clock list. We would have to do something similar here for
>> the SCI device IDs.
>>
>> Then, you must also assign/implement the ->start() and ->stop()
>> callbacks of the struct gpd_dev_ops, which is a part of the genpd
>> struct. These callbacks can then easily invoke SoC specific code/APIs
>> and perhaps that is the issue with my first suggested approach!?
>
>
> I've taken a look at what you have suggested here and I think it could work
> for us, thanks for the suggestion, I will give it a shot, I think that this
> will work just as well from a functional perspective.

Okay, great!

>
>>
>>>
>>> Based on the ID set provided in patch 2 of this series I see 12 gaps, so
>>> we'd be wasting space the size of 12 genpds. The ID mapping will change
>>> on
>>> future SoCs, so this number could be larger. Do you think this is an
>>> acceptable solution? It allows us to play nice with the new genpd
>>> framework
>>> changes at the cost of wasting some space allocated to filler genpds.
>>
>>
>> There are other issues as well, which mostly has do to with a
>> unnecessary long execution path, involving taking mutexes etc in
>> genpd.
>>
>> All you actually need, is to be able to power on/off a device from a
>> driver's ->runtime_suspend|resume() callback. Don't you think?
>
>
> Yes, but I thought the point of these frameworks was that they let us avoid
> doing it manually with platform specific code inside the drivers. I'll look
> at the callbacks in the genpd framework instead, that seems like a good
> place to do it.

BTW, as you would need to store your device specific data somewhere,
we probably need to extend the struct pm_subsys_data or the "struct
pm_domain_data", to hold a "void *data".

Kind regards
Uffe

Re: [PATCH 3/3] soc: ti: Add ti_sci_pm_domains driver

[Ulf Hansson]

[...]

>
> One more idea...
>
> Since you don't really have a domain (a group of devices), what you
> really have is each device having an independent power switch, so as Ulf
> suggested, what you really need is for all the devices to share the same
> set of runtime PM callbacks that call SCI.  The only difference is the
> unique ID.
>
> Rather than using all of genpd, you could also just use a pm_domain
> which is what genpd is built on top of (and also omap_device, which
> you're probably familiar with also.)

Even if this would work as well, the downside would be that you need
to re-invent the parts related to the DT parsing, the probing/removal
and attaching/detaching of the device to the PM domain.

You probably don't want to go there... :-)

>
> That would allow you to keep the drivers completely generic, yet share
> all the SCI specific "domain" code inside a pm_domain.
>
> Kevin

Kind regards
Uffe

Re: [PATCH 3/3] soc: ti: Add ti_sci_pm_domains driver

[Kevin Hilman]

Ulf Hansson <ulf.hansson@linaro.org> writes:

> [...]
>
>>
>> One more idea...
>>
>> Since you don't really have a domain (a group of devices), what you
>> really have is each device having an independent power switch, so as Ulf
>> suggested, what you really need is for all the devices to share the same
>> set of runtime PM callbacks that call SCI.  The only difference is the
>> unique ID.
>>
>> Rather than using all of genpd, you could also just use a pm_domain
>> which is what genpd is built on top of (and also omap_device, which
>> you're probably familiar with also.)
>
> Even if this would work as well, the downside would be that you need
> to re-invent the parts related to the DT parsing, the probing/removal
> and attaching/detaching of the device to the PM domain.
>
> You probably don't want to go there... :-)

All you'd need to read from DT would be the device-specific ID for
TI-SCI, and that could be done at bind time with a notifier.  The, in
that same notifier, if a TI-SCI ID exists, it would get added to the
pm_domain.

Anyways, your original proposal is much preferred if it can work.  I'm
just throwing out another option because I really don't like one genpd
per device.

Kevin

Re: [PATCH 3/3] soc: ti: Add ti_sci_pm_domains driver

[Dave Gerlach]

Hi,
On 09/08/2016 12:38 PM, Kevin Hilman wrote:
> Ulf Hansson <ulf.hansson@linaro.org> writes:
>
>> [...]
>>
>>>
>>> One more idea...
>>>
>>> Since you don't really have a domain (a group of devices), what you
>>> really have is each device having an independent power switch, so as Ulf
>>> suggested, what you really need is for all the devices to share the same
>>> set of runtime PM callbacks that call SCI.  The only difference is the
>>> unique ID.
>>>
>>> Rather than using all of genpd, you could also just use a pm_domain
>>> which is what genpd is built on top of (and also omap_device, which
>>> you're probably familiar with also.)
>>
>> Even if this would work as well, the downside would be that you need
>> to re-invent the parts related to the DT parsing, the probing/removal
>> and attaching/detaching of the device to the PM domain.
>>
>> You probably don't want to go there... :-)
>
> All you'd need to read from DT would be the device-specific ID for
> TI-SCI, and that could be done at bind time with a notifier.  The, in
> that same notifier, if a TI-SCI ID exists, it would get added to the
> pm_domain.
>
> Anyways, your original proposal is much preferred if it can work.  I'm
> just throwing out another option because I really don't like one genpd
> per device.
>
> Kevin
>

I am first trying to leverage the dev_attach/detach and start/stop 
callbacks that Ulf suggested without creating a single genpd per device 
and it looks like it will work for us. I appreciate the alternative 
suggestions but I agree we'd like to leverage as much of the existing 
genpd framework as we can and avoid going down the omap_device style 
implementation path.

Regards,
Dave

Re: [PATCH 3/3] soc: ti: Add ti_sci_pm_domains driver

[Ulf Hansson]

On 8 September 2016 at 19:38, Kevin Hilman <khilman@baylibre.com> wrote:
> Ulf Hansson <ulf.hansson@linaro.org> writes:
>
>> [...]
>>
>>>
>>> One more idea...
>>>
>>> Since you don't really have a domain (a group of devices), what you
>>> really have is each device having an independent power switch, so as Ulf
>>> suggested, what you really need is for all the devices to share the same
>>> set of runtime PM callbacks that call SCI.  The only difference is the
>>> unique ID.
>>>
>>> Rather than using all of genpd, you could also just use a pm_domain
>>> which is what genpd is built on top of (and also omap_device, which
>>> you're probably familiar with also.)
>>
>> Even if this would work as well, the downside would be that you need
>> to re-invent the parts related to the DT parsing, the probing/removal
>> and attaching/detaching of the device to the PM domain.
>>
>> You probably don't want to go there... :-)
>
> All you'd need to read from DT would be the device-specific ID for
> TI-SCI, and that could be done at bind time with a notifier.  The, in
> that same notifier, if a TI-SCI ID exists, it would get added to the
> pm_domain.
>
> Anyways, your original proposal is much preferred if it can work.  I'm
> just throwing out another option because I really don't like one genpd
> per device.

Okay, then we are in full agreement!

BTW, I have just been trying to convince other people working on
Rockchip SoCs, to also avoid using one genpd per device. Feel free to
join those discussions [1] as well. :-)

Kind regards
Uffe

[1]
https://lkml.org/lkml/2016/9/1/377