[PATCH v2 0/3] drivers: soc: xilinx: Add support for ZynqMP power domain driver

[PATCH v2 0/3] drivers: soc: xilinx: Add support for ZynqMP power domain driver

[Jolly Shah]

The zynqmp power domain driver communicates the usage requirements
for logical power domains / devices to the platform FW.
FW is responsible for choosing appropriate power states,
taking Linux' usage information into account.

This patch series is based on top of Xilinx firmware patch set:
https://patchwork.kernel.org/cover/10555405/

v2:
 - Rebased on top of latest firmware driver patch series
 - Updated driver name from zynqmp-genpd to zynqmp-power-controller
 - Updated device tree bindings to move power controller node under firmware node

Jolly Shah (1):
  drivers: soc: xilinx: Add ZynqMP power domain driver

Rajan Vaja (2):
  dt-bindings: power: Add ZynqMP power domain bindings
  firmware: xilinx: Add APIs to control node status/power

 .../firmware/xilinx/xlnx,zynqmp-firmware.txt       |  47 +++
 drivers/firmware/xilinx/zynqmp.c                   |  58 ++++
 drivers/soc/xilinx/Kconfig                         |   9 +
 drivers/soc/xilinx/Makefile                        |   2 +
 drivers/soc/xilinx/zynqmp_pm_domains.c             | 352 +++++++++++++++++++++
 include/linux/firmware/xlnx-zynqmp.h               |  26 ++
 6 files changed, 494 insertions(+)
 create mode 100644 drivers/soc/xilinx/zynqmp_pm_domains.c

-- 
2.7.4

[PATCH v2 1/3] dt-bindings: power: Add ZynqMP power domain bindings

[Jolly Shah]

From: Rajan Vaja <rajan.vaja@xilinx.com>

Add documentation to describe ZynqMP power domain bindings.

Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
Signed-off-by: Jolly Shah <jollys@xilinx.com>
---
 .../firmware/xilinx/xlnx,zynqmp-firmware.txt       | 47 ++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt
index d215d15..5fa10a0 100644
--- a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt
+++ b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt
@@ -64,6 +64,29 @@ Output clocks are registered based on clock information received
 from firmware. Output clocks indexes are mentioned in
 include/dt-bindings/clock/xlnx,zynqmp-clk.h.
 
+-----------------------------------------------------------
+Device Tree Bindings for the Xilinx Zynq MPSoC PM domains
+-----------------------------------------------------------
+The binding for zynqmp-power-controller follow the common
+generic PM domain binding[1].
+
+[1] Documentation/devicetree/bindings/power/power_domain.txt
+
+== Zynq MPSoC Generic PM Domain Node ==
+
+Required properties:
+ - compatible:	Must be: "xlnx,zynqmp-power-controller"
+
+This node contains a number of subnodes, each representing a single PM domain
+that PM domain consumer devices reference.
+
+== PM Domain Nodes ==
+
+Required properties:
+ - #power-domain-cells:	Number of cells in a PM domain specifier. Must be 0.
+ - pd-id:		Domain identifier as defined by platform firmware.
+			This identifier is passed to the PM firmware.
+
 -------
 Example
 -------
@@ -78,5 +101,29 @@ firmware {
 			clocks = <&pss_ref_clk>, <&video_clk>, <&pss_alt_ref_clk>, <&aux_ref_clk>, <&gt_crx_ref_clk>;
 			clock-names = "pss_ref_clk", "video_clk", "pss_alt_ref_clk","aux_ref_clk", "gt_crx_ref_clk";
 		};
+		zynqmp-power-controller {
+			compatible = "xlnx,zynqmp-power-controller";
+
+			pd_usb0: pd-usb0 {
+				pd-id = <22>;
+				#power-domain-cells = <0>;
+			};
+
+			pd_sata: pd-sata {
+				pd-id = <28>;
+				#power-domain-cells = <0>;
+			};
+
+			pd_gpu : pd-gpu {
+				pd-id = <58 20 21>;
+				#power-domain-cells = <0>;
+			};
+		};
 	};
 };
+
+sata0: ahci@SATA_AHCI_HBA {
+	...
+	power-domains = <&pd_sata>;
+	...
+};
-- 
2.7.4

[PATCH v2 2/3] firmware: xilinx: Add APIs to control node status/power

[Jolly Shah]

From: Rajan Vaja <rajan.vaja@xilinx.com>

Add Xilinx ZynqMP firmware APIs to control node status
and power. These APIs allows turning on/off power domain
and setting capabilities of devices present in power domain.

Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
Signed-off-by: Jolly Shah <jollys@xilinx.com>
---
 drivers/firmware/xilinx/zynqmp.c     | 58 ++++++++++++++++++++++++++++++++++++
 include/linux/firmware/xlnx-zynqmp.h | 26 ++++++++++++++++
 2 files changed, 84 insertions(+)

diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
index ce6c746..c54696f 100644
--- a/drivers/firmware/xilinx/zynqmp.c
+++ b/drivers/firmware/xilinx/zynqmp.c
@@ -447,6 +447,61 @@ static int zynqmp_pm_clock_getparent(u32 clock_id, u32 *parent_id)
 	return ret;
 }
 
+/**
+ * zynqmp_pm_request_node() - Request a node with specific capabilities
+ * @node:		Node ID of the slave
+ * @capabilities:	Requested capabilities of the slave
+ * @qos:		Quality of service (not supported)
+ * @ack:		Flag to specify whether acknowledge is requested
+ *
+ * This function is used by master to request particular node from firmware.
+ * Every master must request node before using it.
+ *
+ * Return: Returns status, either success or error+reason
+ */
+static int zynqmp_pm_request_node(const u32 node, const u32 capabilities,
+				  const u32 qos,
+				  const enum zynqmp_pm_request_ack ack)
+{
+	return zynqmp_pm_invoke_fn(PM_REQUEST_NODE, node, capabilities,
+				   qos, ack, NULL);
+}
+
+/**
+ * zynqmp_pm_release_node() - Release a node
+ * @node:	Node ID of the slave
+ *
+ * This function is used by master to inform firmware that master
+ * has released node. Once released, master must not use that node
+ * without re-request.
+ *
+ * Return: Returns status, either success or error+reason
+ */
+static int zynqmp_pm_release_node(const u32 node)
+{
+	return zynqmp_pm_invoke_fn(PM_RELEASE_NODE, node, 0, 0, 0, NULL);
+}
+
+/**
+ * zynqmp_pm_set_requirement() - PM call to set requirement for PM slaves
+ * @node:		Node ID of the slave
+ * @capabilities:	Requested capabilities of the slave
+ * @qos:		Quality of service (not supported)
+ * @ack:		Flag to specify whether acknowledge is requested
+ *
+ * This API function is to be used for slaves a PU already has requested
+ * to change its capabilities.
+ *
+ * Return: Returns status, either success or error+reason
+ */
+static int zynqmp_pm_set_requirement(const u32 node, const u32 capabilities,
+				     const u32 qos,
+				     const enum zynqmp_pm_request_ack ack)
+{
+	return zynqmp_pm_invoke_fn(PM_SET_REQUIREMENT, node, capabilities,
+				   qos, ack, NULL);
+}
+
 static const struct zynqmp_eemi_ops eemi_ops = {
 	.get_api_version = zynqmp_pm_get_api_version,
 	.ioctl = zynqmp_pm_ioctl,
@@ -460,6 +515,9 @@ static const struct zynqmp_eemi_ops eemi_ops = {
 	.clock_getrate = zynqmp_pm_clock_getrate,
 	.clock_setparent = zynqmp_pm_clock_setparent,
 	.clock_getparent = zynqmp_pm_clock_getparent,
+	.request_node = zynqmp_pm_request_node,
+	.release_node = zynqmp_pm_release_node,
+	.set_requirement = zynqmp_pm_set_requirement,
 };
 
 /**
diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
index a3ef7d6..6a1bdcb 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -32,8 +32,19 @@
 /* Number of 32bits values in payload */
 #define PAYLOAD_ARG_CNT	4U
 
+#define ZYNQMP_PM_MAX_QOS	100U
+
+/* Node capabilities */
+#define	ZYNQMP_PM_CAPABILITY_ACCESS	0x1U
+#define	ZYNQMP_PM_CAPABILITY_CONTEXT	0x2U
+#define	ZYNQMP_PM_CAPABILITY_WAKEUP	0x4U
+#define	ZYNQMP_PM_CAPABILITY_POWER	0x8U
+
 enum pm_api_id {
 	PM_GET_API_VERSION = 1,
+	PM_REQUEST_NODE = 13,
+	PM_RELEASE_NODE,
+	PM_SET_REQUIREMENT,
 	PM_IOCTL = 34,
 	PM_QUERY_DATA,
 	PM_CLOCK_ENABLE,
@@ -75,6 +86,12 @@ enum pm_query_id {
 	PM_QID_CLOCK_GET_NUM_CLOCKS = 12,
 };
 
+enum zynqmp_pm_request_ack {
+	ZYNQMP_PM_REQUEST_ACK_NO = 1,
+	ZYNQMP_PM_REQUEST_ACK_BLOCKING,
+	ZYNQMP_PM_REQUEST_ACK_NON_BLOCKING,
+};
+
 /**
  * struct zynqmp_pm_query_data - PM query data
  * @qid:	query ID
@@ -102,6 +119,15 @@ struct zynqmp_eemi_ops {
 	int (*clock_getrate)(u32 clock_id, u64 *rate);
 	int (*clock_setparent)(u32 clock_id, u32 parent_id);
 	int (*clock_getparent)(u32 clock_id, u32 *parent_id);
+	int (*request_node)(const u32 node,
+			    const u32 capabilities,
+			    const u32 qos,
+			    const enum zynqmp_pm_request_ack ack);
+	int (*release_node)(const u32 node);
+	int (*set_requirement)(const u32 node,
+			       const u32 capabilities,
+			       const u32 qos,
+			       const enum zynqmp_pm_request_ack ack);
 };
 
 #if IS_REACHABLE(CONFIG_ARCH_ZYNQMP)
-- 
2.7.4

[PATCH v2 3/3] drivers: soc: xilinx: Add ZynqMP power domain driver

[Jolly Shah]

From: Jolly Shah <jolly.shah@xilinx.com>

The zynqmp-genpd driver communicates the usage requirements
for logical power domains / devices to the platform FW.
FW is responsible for choosing appropriate power states,
taking Linux' usage information into account.

Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
Signed-off-by: Jolly Shah <jollys@xilinx.com>
---
 drivers/soc/xilinx/Kconfig             |   9 +
 drivers/soc/xilinx/Makefile            |   2 +
 drivers/soc/xilinx/zynqmp_pm_domains.c | 352 +++++++++++++++++++++++++++++++++
 3 files changed, 363 insertions(+)
 create mode 100644 drivers/soc/xilinx/zynqmp_pm_domains.c

diff --git a/drivers/soc/xilinx/Kconfig b/drivers/soc/xilinx/Kconfig
index 687c8f3..964b205 100644
--- a/drivers/soc/xilinx/Kconfig
+++ b/drivers/soc/xilinx/Kconfig
@@ -17,4 +17,13 @@ config XILINX_VCU
 	  To compile this driver as a module, choose M here: the
 	  module will be called xlnx_vcu.
 
+config ZYNQMP_PM_DOMAINS
+	bool "Enable Zynq MPSoC generic PM domains"
+	default y
+	depends on PM && ARCH_ZYNQMP
+	select PM_GENERIC_DOMAINS
+	help
+	  Say yes to enable device power management through PM domains
+	  If in doubt, say N.
+
 endmenu
diff --git a/drivers/soc/xilinx/Makefile b/drivers/soc/xilinx/Makefile
index dee8fd5..f468d1b 100644
--- a/drivers/soc/xilinx/Makefile
+++ b/drivers/soc/xilinx/Makefile
@@ -1,2 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_XILINX_VCU)	+= xlnx_vcu.o
+
+obj-$(CONFIG_ZYNQMP_PM_DOMAINS) += zynqmp_pm_domains.o
diff --git a/drivers/soc/xilinx/zynqmp_pm_domains.c b/drivers/soc/xilinx/zynqmp_pm_domains.c
new file mode 100644
index 0000000..aab462c
--- /dev/null
+++ b/drivers/soc/xilinx/zynqmp_pm_domains.c
@@ -0,0 +1,352 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ZynqMP Generic PM domain support
+ *
+ *  Copyright (C) 2015-2018 Xilinx, Inc.
+ *
+ *  Davorin Mista <davorin.mista@aggios.com>
+ *  Jolly Shah <jollys@xilinx.com>
+ *  Rajan Vaja <rajan.vaja@xilinx.com>
+ */
+
+#include <linux/err.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/slab.h>
+
+#include <linux/firmware/xlnx-zynqmp.h>
+
+/* Flag stating if PM nodes mapped to the PM domain has been requested */
+#define ZYNQMP_PM_DOMAIN_REQUESTED	BIT(0)
+
+/**
+ * struct zynqmp_pm_domain - Wrapper around struct generic_pm_domain
+ * @gpd:		Generic power domain
+ * @dev_list:		List of devices belong to power domain
+ * @node_ids:		PM node IDs corresponding to device(s) inside PM domain
+ * @node_id_num:	Number of PM node IDs
+ * @flags:		ZynqMP PM domain flags
+ */
+struct zynqmp_pm_domain {
+	struct generic_pm_domain gpd;
+	struct list_head dev_list;
+	u32 *node_ids;
+	int node_id_num;
+	u8 flags;
+};
+
+/*
+ * struct zynqmp_domain_device - Device node present in power domain
+ * @dev:	Device
+ * &list:	List member for the devices in domain list
+ */
+struct zynqmp_domain_device {
+	struct device *dev;
+	struct list_head list;
+};
+
+/**
+ * zynqmp_gpd_is_active_wakeup_path() - Check if device is in wakeup source
+ *					path
+ * @dev:	Device to check for wakeup source path
+ * @not_used:	Data member (not required)
+ *
+ * This function is checks device's child hierarchy and checks if any device is
+ * set as wakeup source.
+ *
+ * Return: 1 if device is in wakeup source path else 0
+ */
+static int zynqmp_gpd_is_active_wakeup_path(struct device *dev, void *not_used)
+{
+	int may_wakeup;
+
+	may_wakeup = device_may_wakeup(dev);
+	if (may_wakeup)
+		return may_wakeup;
+
+	return device_for_each_child(dev, NULL,
+			zynqmp_gpd_is_active_wakeup_path);
+}
+
+/**
+ * zynqmp_gpd_power_on() - Power on PM domain
+ * @domain:	Generic PM domain
+ *
+ * This function is called before devices inside a PM domain are resumed, to
+ * power on PM domain.
+ *
+ * Return: 0 on success, error code otherwise
+ */
+static int zynqmp_gpd_power_on(struct generic_pm_domain *domain)
+{
+	int i, ret;
+	struct zynqmp_pm_domain *pd;
+	const struct zynqmp_eemi_ops *eemi_ops = zynqmp_pm_get_eemi_ops();
+
+	if (!eemi_ops || !eemi_ops->set_requirement)
+		return -ENXIO;
+
+	pd = container_of(domain, struct zynqmp_pm_domain, gpd);
+	for (i = 0; i < pd->node_id_num; i++) {
+		ret = eemi_ops->set_requirement(pd->node_ids[i],
+						ZYNQMP_PM_CAPABILITY_ACCESS,
+						ZYNQMP_PM_MAX_QOS,
+						ZYNQMP_PM_REQUEST_ACK_BLOCKING);
+		if (ret) {
+			pr_err("%s() %s set requirement for node %d failed: %d\n",
+			       __func__, domain->name, pd->node_ids[i], ret);
+			return ret;
+		}
+	}
+
+	pr_debug("%s() Powered on %s domain\n", __func__, domain->name);
+
+	return 0;
+}
+
+/**
+ * zynqmp_gpd_power_off() - Power off PM domain
+ * @domain:	Generic PM domain
+ *
+ * This function is called after devices inside a PM domain are suspended, to
+ * power off PM domain.
+ *
+ * Return: 0 on success, error code otherwise
+ */
+static int zynqmp_gpd_power_off(struct generic_pm_domain *domain)
+{
+	int i, ret;
+	struct zynqmp_pm_domain *pd;
+	struct zynqmp_domain_device *zdev, *tmp;
+	u32 capabilities = 0;
+	bool may_wakeup;
+	const struct zynqmp_eemi_ops *eemi_ops = zynqmp_pm_get_eemi_ops();
+
+	if (!eemi_ops || !eemi_ops->set_requirement)
+		return -ENXIO;
+
+	pd = container_of(domain, struct zynqmp_pm_domain, gpd);
+
+	/* If domain is already released there is nothing to be done */
+	if (!(pd->flags & ZYNQMP_PM_DOMAIN_REQUESTED))
+		return 0;
+
+	list_for_each_entry_safe(zdev, tmp, &pd->dev_list, list) {
+		/* If device is in wakeup path, set capability to WAKEUP */
+		may_wakeup = zynqmp_gpd_is_active_wakeup_path(zdev->dev, NULL);
+		if (may_wakeup) {
+			dev_dbg(zdev->dev, "device is in wakeup path in %s\n",
+				domain->name);
+			capabilities = ZYNQMP_PM_CAPABILITY_WAKEUP;
+			break;
+		}
+	}
+
+	for (i = pd->node_id_num - 1; i >= 0; i--) {
+		ret = eemi_ops->set_requirement(pd->node_ids[i],
+						   capabilities, 0,
+						   ZYNQMP_PM_REQUEST_ACK_NO);
+		/**
+		 * If powering down of any node inside this domain fails,
+		 * report and return the error
+		 */
+		if (ret) {
+			pr_err("%s() %s set requirement for node %d failed: %d\n",
+			       __func__, domain->name, pd->node_ids[i], ret);
+			return ret;
+		}
+	}
+
+	pr_debug("%s() Powered off %s domain\n", __func__, domain->name);
+
+	return 0;
+}
+
+/**
+ * zynqmp_gpd_attach_dev() - Attach device to the PM domain
+ * @domain:	Generic PM domain
+ * @dev:	Device to attach
+ *
+ * Return: 0 on success, error code otherwise
+ */
+static int zynqmp_gpd_attach_dev(struct generic_pm_domain *domain,
+				 struct device *dev)
+{
+	int i, ret;
+	struct zynqmp_pm_domain *pd;
+	struct zynqmp_domain_device *zdev;
+	const struct zynqmp_eemi_ops *eemi_ops = zynqmp_pm_get_eemi_ops();
+
+	if (!eemi_ops || !eemi_ops->request_node)
+		return -ENXIO;
+
+	pd = container_of(domain, struct zynqmp_pm_domain, gpd);
+
+	zdev = devm_kzalloc(dev, sizeof(*zdev), GFP_KERNEL);
+	if (!zdev)
+		return -ENOMEM;
+
+	zdev->dev = dev;
+	list_add(&zdev->list, &pd->dev_list);
+
+	/* If this is not the first device to attach there is nothing to do */
+	if (domain->device_count)
+		return 0;
+
+	for (i = 0; i < pd->node_id_num; i++) {
+		ret = eemi_ops->request_node(pd->node_ids[i], 0, 0,
+						ZYNQMP_PM_REQUEST_ACK_BLOCKING);
+		/* If requesting a node fails print and return the error */
+		if (ret) {
+			pr_err("%s() %s request failed for node %d: %d\n",
+			       __func__, domain->name, pd->node_ids[i], ret);
+			list_del(&zdev->list);
+			zdev->dev = NULL;
+			devm_kfree(dev, zdev);
+			return ret;
+		}
+	}
+
+	pd->flags |= ZYNQMP_PM_DOMAIN_REQUESTED;
+
+	pr_debug("%s() %s attached to %s domain\n", __func__,
+		 dev_name(dev), domain->name);
+
+	return 0;
+}
+
+/**
+ * zynqmp_gpd_detach_dev() - Detach device from the PM domain
+ * @domain:	Generic PM domain
+ * @dev:	Device to detach
+ */
+static void zynqmp_gpd_detach_dev(struct generic_pm_domain *domain,
+				  struct device *dev)
+{
+	int i, ret;
+	struct zynqmp_pm_domain *pd;
+	struct zynqmp_domain_device *zdev, *tmp;
+	const struct zynqmp_eemi_ops *eemi_ops = zynqmp_pm_get_eemi_ops();
+
+	if (!eemi_ops || !eemi_ops->release_node)
+		return;
+
+	pd = container_of(domain, struct zynqmp_pm_domain, gpd);
+
+	list_for_each_entry_safe(zdev, tmp, &pd->dev_list, list)
+		if (zdev->dev == dev) {
+			list_del(&zdev->list);
+			zdev->dev = NULL;
+			devm_kfree(dev, zdev);
+		}
+
+	/* If this is not the last device to detach there is nothing to do */
+	if (domain->device_count)
+		return;
+
+	for (i = 0; i < pd->node_id_num; i++) {
+		ret = eemi_ops->release_node(pd->node_ids[i]);
+		/* If releasing a node fails print the error and return */
+		if (ret) {
+			pr_err("%s() %s release failed for node %d: %d\n",
+			       __func__, domain->name, pd->node_ids[i], ret);
+			return;
+		}
+	}
+
+	pd->flags &= ~ZYNQMP_PM_DOMAIN_REQUESTED;
+
+	pr_debug("%s() %s detached from %s domain\n", __func__,
+		 dev_name(dev), domain->name);
+}
+
+/**
+ * zynqmp_gpd_probe() - Initialize ZynqMP specific PM domains
+ * @pdev:	Platform device pointer
+ *
+ * Description:	This function populates struct zynqmp_pm_domain for each PM
+ * domain and initalizes generic PM domain. If the "pd-id" DT property
+ * of a certain domain is missing or invalid, that domain will be skipped.
+ *
+ * Return: 0 on success, error code otherwise
+ */
+static int zynqmp_gpd_probe(struct platform_device *pdev)
+{
+	int ret;
+	struct device_node *child_err, *child, *np = pdev->dev.of_node;
+
+	for_each_child_of_node(np, child) {
+		struct zynqmp_pm_domain *pd;
+
+		pd = devm_kzalloc(&pdev->dev, sizeof(*pd), GFP_KERNEL);
+		if (!pd) {
+			ret = -ENOMEM;
+			goto err_cleanup;
+		}
+
+		ret = of_property_count_u32_elems(child, "pd-id");
+		if (ret <= 0)
+			goto err_cleanup;
+
+		pd->node_id_num = ret;
+		pd->node_ids = devm_kcalloc(&pdev->dev, ret,
+					    sizeof(*pd->node_ids), GFP_KERNEL);
+		if (!pd->node_ids) {
+			ret = -ENOMEM;
+			goto err_cleanup;
+		}
+
+		ret = of_property_read_u32_array(child, "pd-id", pd->node_ids,
+						 pd->node_id_num);
+		if (ret)
+			goto err_cleanup;
+
+		pd->gpd.name = kstrdup(child->name, GFP_KERNEL);
+		pd->gpd.power_off = zynqmp_gpd_power_off;
+		pd->gpd.power_on = zynqmp_gpd_power_on;
+		pd->gpd.attach_dev = zynqmp_gpd_attach_dev;
+		pd->gpd.detach_dev = zynqmp_gpd_detach_dev;
+
+		/* Mark all PM domains as initially powered off */
+		pm_genpd_init(&pd->gpd, NULL, true);
+
+		ret = of_genpd_add_provider_simple(child, &pd->gpd);
+		if (ret)
+			goto err_cleanup;
+
+		INIT_LIST_HEAD(&pd->dev_list);
+
+		pr_debug("%s() %s PM domain registered\n",
+			 __func__, child->name);
+	}
+
+	return 0;
+
+err_cleanup:
+	child_err = child;
+	for_each_child_of_node(np, child) {
+		if (child == child_err)
+			break;
+		of_genpd_del_provider(child);
+	}
+
+	return ret;
+}
+
+static const struct of_device_id zynqmp_power_domain_of_match[] = {
+	{.compatible = "xlnx,zynqmp-power-controller"},
+	{},
+};
+MODULE_DEVICE_TABLE(of, zynqmp_power_domain_of_match);
+
+static struct platform_driver zynqmp_power_domain_driver = {
+	.driver	= {
+		.name = "zynqmp_power_controller",
+		.of_match_table = zynqmp_power_domain_of_match,
+	},
+	.probe = zynqmp_gpd_probe,
+};
+module_platform_driver(zynqmp_power_domain_driver);
-- 
2.7.4

Re: [PATCH v2 1/3] dt-bindings: power: Add ZynqMP power domain bindings

[Rob Herring]

On Thu, Aug 16, 2018 at 12:21:42PM -0700, Jolly Shah wrote:
> From: Rajan Vaja <rajan.vaja@xilinx.com>
> 
> Add documentation to describe ZynqMP power domain bindings.
> 
> Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
> Signed-off-by: Jolly Shah <jollys@xilinx.com>
> ---
>  .../firmware/xilinx/xlnx,zynqmp-firmware.txt       | 47 ++++++++++++++++++++++

This should be with all the other power domain bindings.

>  1 file changed, 47 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt
> index d215d15..5fa10a0 100644
> --- a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt
> +++ b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmware.txt
> @@ -64,6 +64,29 @@ Output clocks are registered based on clock information received
>  from firmware. Output clocks indexes are mentioned in
>  include/dt-bindings/clock/xlnx,zynqmp-clk.h.
>  
> +-----------------------------------------------------------
> +Device Tree Bindings for the Xilinx Zynq MPSoC PM domains
> +-----------------------------------------------------------
> +The binding for zynqmp-power-controller follow the common
> +generic PM domain binding[1].
> +
> +[1] Documentation/devicetree/bindings/power/power_domain.txt
> +
> +== Zynq MPSoC Generic PM Domain Node ==
> +
> +Required properties:
> + - compatible:	Must be: "xlnx,zynqmp-power-controller"
> +
> +This node contains a number of subnodes, each representing a single PM domain
> +that PM domain consumer devices reference.
> +
> +== PM Domain Nodes ==
> +
> +Required properties:
> + - #power-domain-cells:	Number of cells in a PM domain specifier. Must be 0.
> + - pd-id:		Domain identifier as defined by platform firmware.
> +			This identifier is passed to the PM firmware.

Make this a cell for the power domain consumer.

> +
>  -------
>  Example
>  -------
> @@ -78,5 +101,29 @@ firmware {
>  			clocks = <&pss_ref_clk>, <&video_clk>, <&pss_alt_ref_clk>, <&aux_ref_clk>, <&gt_crx_ref_clk>;
>  			clock-names = "pss_ref_clk", "video_clk", "pss_alt_ref_clk","aux_ref_clk", "gt_crx_ref_clk";
>  		};
> +		zynqmp-power-controller {
> +			compatible = "xlnx,zynqmp-power-controller";
> +
> +			pd_usb0: pd-usb0 {
> +				pd-id = <22>;
> +				#power-domain-cells = <0>;
> +			};
> +
> +			pd_sata: pd-sata {
> +				pd-id = <28>;
> +				#power-domain-cells = <0>;
> +			};
> +
> +			pd_gpu : pd-gpu {
> +				pd-id = <58 20 21>;
> +				#power-domain-cells = <0>;
> +			};
> +		};
>  	};
>  };
> +
> +sata0: ahci@SATA_AHCI_HBA {

Don't use defines in unit-addresses (or reg for that matter). It's 
pointless indirection.

> +	...
> +	power-domains = <&pd_sata>;
> +	...
> +};
> -- 
> 2.7.4
> 

RE: [PATCH v2 1/3] dt-bindings: power: Add ZynqMP power domain bindings

[Jolly Shah]

Hi Rob,

> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: Monday, August 20, 2018 12:46 PM
> To: Jolly Shah <JOLLYS@xilinx.com>
> Cc: matthias.bgg@gmail.com; andy.gross@linaro.org; shawnguo@kernel.org;
> geert+renesas@glider.be; bjorn.andersson@linaro.org;
> sean.wang@mediatek.com; m.szyprowski@samsung.com; Michal Simek
> <michals@xilinx.com>; mark.rutland@arm.com; Rajan Vaja
> <RAJANV@xilinx.com>; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Rajan Vaja
> <RAJANV@xilinx.com>; Jolly Shah <JOLLYS@xilinx.com>
> Subject: Re: [PATCH v2 1/3] dt-bindings: power: Add ZynqMP power domain
> bindings
> 
> On Thu, Aug 16, 2018 at 12:21:42PM -0700, Jolly Shah wrote:
> > From: Rajan Vaja <rajan.vaja@xilinx.com>
> >
> > Add documentation to describe ZynqMP power domain bindings.
> >
> > Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
> > Signed-off-by: Jolly Shah <jollys@xilinx.com>
> > ---
> >  .../firmware/xilinx/xlnx,zynqmp-firmware.txt       | 47
> ++++++++++++++++++++++
> 
> This should be with all the other power domain bindings.
> 

The firmware node has clock, reset and power domain subnodes. Please suggest the right location for binding.

> >  1 file changed, 47 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmwa
> > re.txt
> > b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmwa
> > re.txt
> > index d215d15..5fa10a0 100644
> > ---
> > a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmwa
> > re.txt
> > +++ b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-fi
> > +++ rmware.txt
> > @@ -64,6 +64,29 @@ Output clocks are registered based on clock
> > information received  from firmware. Output clocks indexes are
> > mentioned in  include/dt-bindings/clock/xlnx,zynqmp-clk.h.
> >
> > +-----------------------------------------------------------
> > +Device Tree Bindings for the Xilinx Zynq MPSoC PM domains
> > +-----------------------------------------------------------
> > +The binding for zynqmp-power-controller follow the common generic PM
> > +domain binding[1].
> > +
> > +[1] Documentation/devicetree/bindings/power/power_domain.txt
> > +
> > +== Zynq MPSoC Generic PM Domain Node ==
> > +
> > +Required properties:
> > + - compatible:	Must be: "xlnx,zynqmp-power-controller"
> > +
> > +This node contains a number of subnodes, each representing a single
> > +PM domain that PM domain consumer devices reference.
> > +
> > +== PM Domain Nodes ==
> > +
> > +Required properties:
> > + - #power-domain-cells:	Number of cells in a PM domain specifier. Must
> be 0.
> > + - pd-id:		Domain identifier as defined by platform firmware.
> > +			This identifier is passed to the PM firmware.
> 
> Make this a cell for the power domain consumer.
> 

I am not clear here. Do you suggest to move pd-id property under consumer nodes? Right now, consumer nodes use power-domains property as below:

usb0: usb0@ff9d0000 {
                        #address-cells = <2>;
	          ...................................................
                        power-domains = <&pd_usb0>;
	};

Thanks,
Jolly Shah

> > +
> >  -------
> >  Example
> >  -------
> > @@ -78,5 +101,29 @@ firmware {
> >  			clocks = <&pss_ref_clk>, <&video_clk>,
> <&pss_alt_ref_clk>, <&aux_ref_clk>, <&gt_crx_ref_clk>;
> >  			clock-names = "pss_ref_clk", "video_clk",
> "pss_alt_ref_clk","aux_ref_clk", "gt_crx_ref_clk";
> >  		};
> > +		zynqmp-power-controller {
> > +			compatible = "xlnx,zynqmp-power-controller";
> > +
> > +			pd_usb0: pd-usb0 {
> > +				pd-id = <22>;
> > +				#power-domain-cells = <0>;
> > +			};
> > +
> > +			pd_sata: pd-sata {
> > +				pd-id = <28>;
> > +				#power-domain-cells = <0>;
> > +			};
> > +
> > +			pd_gpu : pd-gpu {
> > +				pd-id = <58 20 21>;
> > +				#power-domain-cells = <0>;
> > +			};
> > +		};
> >  	};
> >  };
> > +
> > +sata0: ahci@SATA_AHCI_HBA {
> 
> Don't use defines in unit-addresses (or reg for that matter). It's pointless
> indirection.
> 
> > +	...
> > +	power-domains = <&pd_sata>;
> > +	...
> > +};
> > --
> > 2.7.4
> >

RE: [PATCH v2 1/3] dt-bindings: power: Add ZynqMP power domain bindings

[Jolly Shah]

Hi Rob,

> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: Monday, August 20, 2018 12:46 PM
> To: Jolly Shah <JOLLYS@xilinx.com>
> Cc: matthias.bgg@gmail.com; andy.gross@linaro.org; shawnguo@kernel.org;
> geert+renesas@glider.be; bjorn.andersson@linaro.org;
> sean.wang@mediatek.com; m.szyprowski@samsung.com; Michal Simek
> <michals@xilinx.com>; mark.rutland@arm.com; Rajan Vaja
> <RAJANV@xilinx.com>; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Rajan Vaja
> <RAJANV@xilinx.com>; Jolly Shah <JOLLYS@xilinx.com>
> Subject: Re: [PATCH v2 1/3] dt-bindings: power: Add ZynqMP power domain
> bindings
> 
> On Thu, Aug 16, 2018 at 12:21:42PM -0700, Jolly Shah wrote:
> > From: Rajan Vaja <rajan.vaja@xilinx.com>
> >
> > Add documentation to describe ZynqMP power domain bindings.
> >
> > Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
> > Signed-off-by: Jolly Shah <jollys@xilinx.com>
> > ---
> >  .../firmware/xilinx/xlnx,zynqmp-firmware.txt       | 47
> ++++++++++++++++++++++
> 
> This should be with all the other power domain bindings.
> 
> >  1 file changed, 47 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmwa
> > re.txt
> > b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmwa
> > re.txt
> > index d215d15..5fa10a0 100644
> > ---
> > a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmwa
> > re.txt
> > +++ b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-fi
> > +++ rmware.txt
> > @@ -64,6 +64,29 @@ Output clocks are registered based on clock
> > information received  from firmware. Output clocks indexes are
> > mentioned in  include/dt-bindings/clock/xlnx,zynqmp-clk.h.
> >
> > +-----------------------------------------------------------
> > +Device Tree Bindings for the Xilinx Zynq MPSoC PM domains
> > +-----------------------------------------------------------
> > +The binding for zynqmp-power-controller follow the common generic PM
> > +domain binding[1].
> > +
> > +[1] Documentation/devicetree/bindings/power/power_domain.txt
> > +
> > +== Zynq MPSoC Generic PM Domain Node ==
> > +
> > +Required properties:
> > + - compatible:	Must be: "xlnx,zynqmp-power-controller"
> > +
> > +This node contains a number of subnodes, each representing a single
> > +PM domain that PM domain consumer devices reference.
> > +
> > +== PM Domain Nodes ==
> > +
> > +Required properties:
> > + - #power-domain-cells:	Number of cells in a PM domain specifier. Must
> be 0.
> > + - pd-id:		Domain identifier as defined by platform firmware.
> > +			This identifier is passed to the PM firmware.
> 
> Make this a cell for the power domain consumer.
[Jolly] We have more than one Ids for GPU device. Also they don't have parent child relationship and hence are defined as flat hierarchy. (shown in example below)

Thanks,
Jolly Shah

> 
> > +
> >  -------
> >  Example
> >  -------
> > @@ -78,5 +101,29 @@ firmware {
> >  			clocks = <&pss_ref_clk>, <&video_clk>,
> <&pss_alt_ref_clk>, <&aux_ref_clk>, <&gt_crx_ref_clk>;
> >  			clock-names = "pss_ref_clk", "video_clk",
> "pss_alt_ref_clk","aux_ref_clk", "gt_crx_ref_clk";
> >  		};
> > +		zynqmp-power-controller {
> > +			compatible = "xlnx,zynqmp-power-controller";
> > +
> > +			pd_usb0: pd-usb0 {
> > +				pd-id = <22>;
> > +				#power-domain-cells = <0>;
> > +			};
> > +
> > +			pd_sata: pd-sata {
> > +				pd-id = <28>;
> > +				#power-domain-cells = <0>;
> > +			};
> > +
> > +			pd_gpu : pd-gpu {
> > +				pd-id = <58 20 21>;
> > +				#power-domain-cells = <0>;
> > +			};
> > +		};
> >  	};
> >  };
> > +
> > +sata0: ahci@SATA_AHCI_HBA {
> 
> Don't use defines in unit-addresses (or reg for that matter). It's pointless
> indirection.
> 
> > +	...
> > +	power-domains = <&pd_sata>;
> > +	...
> > +};
> > --
> > 2.7.4
> >

Re: [PATCH v2 1/3] dt-bindings: power: Add ZynqMP power domain bindings

[Rob Herring]

On Thu, Sep 13, 2018 at 12:51 PM Jolly Shah <JOLLYS@xilinx.com> wrote:
>
> Hi Rob,
>
> > -----Original Message-----
> > From: Rob Herring [mailto:robh@kernel.org]
> > Sent: Monday, August 20, 2018 12:46 PM
> > To: Jolly Shah <JOLLYS@xilinx.com>
> > Cc: matthias.bgg@gmail.com; andy.gross@linaro.org; shawnguo@kernel.org;
> > geert+renesas@glider.be; bjorn.andersson@linaro.org;
> > sean.wang@mediatek.com; m.szyprowski@samsung.com; Michal Simek
> > <michals@xilinx.com>; mark.rutland@arm.com; Rajan Vaja
> > <RAJANV@xilinx.com>; devicetree@vger.kernel.org; linux-arm-
> > kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Rajan Vaja
> > <RAJANV@xilinx.com>; Jolly Shah <JOLLYS@xilinx.com>
> > Subject: Re: [PATCH v2 1/3] dt-bindings: power: Add ZynqMP power domain
> > bindings
> >
> > On Thu, Aug 16, 2018 at 12:21:42PM -0700, Jolly Shah wrote:
> > > From: Rajan Vaja <rajan.vaja@xilinx.com>
> > >
> > > Add documentation to describe ZynqMP power domain bindings.
> > >
> > > Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
> > > Signed-off-by: Jolly Shah <jollys@xilinx.com>
> > > ---
> > >  .../firmware/xilinx/xlnx,zynqmp-firmware.txt       | 47
> > ++++++++++++++++++++++
> >
> > This should be with all the other power domain bindings.
> >
> > >  1 file changed, 47 insertions(+)
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmwa
> > > re.txt
> > > b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmwa
> > > re.txt
> > > index d215d15..5fa10a0 100644
> > > ---
> > > a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-firmwa
> > > re.txt
> > > +++ b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-fi
> > > +++ rmware.txt
> > > @@ -64,6 +64,29 @@ Output clocks are registered based on clock
> > > information received  from firmware. Output clocks indexes are
> > > mentioned in  include/dt-bindings/clock/xlnx,zynqmp-clk.h.
> > >
> > > +-----------------------------------------------------------
> > > +Device Tree Bindings for the Xilinx Zynq MPSoC PM domains
> > > +-----------------------------------------------------------
> > > +The binding for zynqmp-power-controller follow the common generic PM
> > > +domain binding[1].
> > > +
> > > +[1] Documentation/devicetree/bindings/power/power_domain.txt
> > > +
> > > +== Zynq MPSoC Generic PM Domain Node ==
> > > +
> > > +Required properties:
> > > + - compatible:     Must be: "xlnx,zynqmp-power-controller"
> > > +
> > > +This node contains a number of subnodes, each representing a single
> > > +PM domain that PM domain consumer devices reference.
> > > +
> > > +== PM Domain Nodes ==
> > > +
> > > +Required properties:
> > > + - #power-domain-cells:    Number of cells in a PM domain specifier. Must
> > be 0.
> > > + - pd-id:          Domain identifier as defined by platform firmware.
> > > +                   This identifier is passed to the PM firmware.
> >
> > Make this a cell for the power domain consumer.
> [Jolly] We have more than one Ids for GPU device. Also they don't have parent child relationship and hence are defined as flat hierarchy. (shown in example below)

Then the gpu node should have:

power-domains = <&pd 58 &pd 20 &pd 21>;

Also, for this and the firmware reset binding, there is no reason that
I see to make these all subnodes. A single firmware node can be a
provider of multiple functions. You only need child nodes if the
sub-functions have their own resources (clks, irqs, etc.). IOW, don't
create nodes just because you want to instantiate drivers that way. DT
is not the only way to instantiate devices for drivers.

Rob

RE: [PATCH v2 1/3] dt-bindings: power: Add ZynqMP power domain bindings

[Jolly Shah]

Hi Rob,

> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: Tuesday, September 25, 2018 9:15 AM
> To: Jolly Shah <JOLLYS@xilinx.com>
> Cc: Matthias Brugger <matthias.bgg@gmail.com>; Andy Gross
> <andy.gross@linaro.org>; Shawn Guo <shawnguo@kernel.org>; Geert
> Uytterhoeven <geert+renesas@glider.be>; Bjorn Andersson
> <bjorn.andersson@linaro.org>; Sean Wang <sean.wang@mediatek.com>;
> Marek Szyprowski <m.szyprowski@samsung.com>; Michal Simek
> <michals@xilinx.com>; Mark Rutland <mark.rutland@arm.com>; Rajan Vaja
> <RAJANV@xilinx.com>; devicetree@vger.kernel.org; moderated
> list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE <linux-arm-
> kernel@lists.infradead.org>; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v2 1/3] dt-bindings: power: Add ZynqMP power domain
> bindings
> 
> On Thu, Sep 13, 2018 at 12:51 PM Jolly Shah <JOLLYS@xilinx.com> wrote:
> >
> > Hi Rob,
> >
> > > -----Original Message-----
> > > From: Rob Herring [mailto:robh@kernel.org]
> > > Sent: Monday, August 20, 2018 12:46 PM
> > > To: Jolly Shah <JOLLYS@xilinx.com>
> > > Cc: matthias.bgg@gmail.com; andy.gross@linaro.org;
> > > shawnguo@kernel.org;
> > > geert+renesas@glider.be; bjorn.andersson@linaro.org;
> > > sean.wang@mediatek.com; m.szyprowski@samsung.com; Michal Simek
> > > <michals@xilinx.com>; mark.rutland@arm.com; Rajan Vaja
> > > <RAJANV@xilinx.com>; devicetree@vger.kernel.org; linux-arm-
> > > kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Rajan Vaja
> > > <RAJANV@xilinx.com>; Jolly Shah <JOLLYS@xilinx.com>
> > > Subject: Re: [PATCH v2 1/3] dt-bindings: power: Add ZynqMP power
> > > domain bindings
> > >
> > > On Thu, Aug 16, 2018 at 12:21:42PM -0700, Jolly Shah wrote:
> > > > From: Rajan Vaja <rajan.vaja@xilinx.com>
> > > >
> > > > Add documentation to describe ZynqMP power domain bindings.
> > > >
> > > > Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
> > > > Signed-off-by: Jolly Shah <jollys@xilinx.com>
> > > > ---
> > > >  .../firmware/xilinx/xlnx,zynqmp-firmware.txt       | 47
> > > ++++++++++++++++++++++
> > >
> > > This should be with all the other power domain bindings.
> > >
> > > >  1 file changed, 47 insertions(+)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-fi
> > > > rmwa
> > > > re.txt
> > > > b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-fi
> > > > rmwa
> > > > re.txt
> > > > index d215d15..5fa10a0 100644
> > > > ---
> > > > a/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqmp-fi
> > > > rmwa
> > > > re.txt
> > > > +++ b/Documentation/devicetree/bindings/firmware/xilinx/xlnx,zynqm
> > > > +++ p-fi
> > > > +++ rmware.txt
> > > > @@ -64,6 +64,29 @@ Output clocks are registered based on clock
> > > > information received  from firmware. Output clocks indexes are
> > > > mentioned in  include/dt-bindings/clock/xlnx,zynqmp-clk.h.
> > > >
> > > > +-----------------------------------------------------------
> > > > +Device Tree Bindings for the Xilinx Zynq MPSoC PM domains
> > > > +-----------------------------------------------------------
> > > > +The binding for zynqmp-power-controller follow the common generic
> > > > +PM domain binding[1].
> > > > +
> > > > +[1] Documentation/devicetree/bindings/power/power_domain.txt
> > > > +
> > > > +== Zynq MPSoC Generic PM Domain Node ==
> > > > +
> > > > +Required properties:
> > > > + - compatible:     Must be: "xlnx,zynqmp-power-controller"
> > > > +
> > > > +This node contains a number of subnodes, each representing a
> > > > +single PM domain that PM domain consumer devices reference.
> > > > +
> > > > +== PM Domain Nodes ==
> > > > +
> > > > +Required properties:
> > > > + - #power-domain-cells:    Number of cells in a PM domain specifier. Must
> > > be 0.
> > > > + - pd-id:          Domain identifier as defined by platform firmware.
> > > > +                   This identifier is passed to the PM firmware.
> > >
> > > Make this a cell for the power domain consumer.
> > [Jolly] We have more than one Ids for GPU device. Also they don't have
> > parent child relationship and hence are defined as flat hierarchy.
> > (shown in example below)
> 
> Then the gpu node should have:
> 
> power-domains = <&pd 58 &pd 20 &pd 21>;
> 
> Also, for this and the firmware reset binding, there is no reason that I see to
> make these all subnodes. A single firmware node can be a provider of multiple
> functions. You only need child nodes if the sub-functions have their own
> resources (clks, irqs, etc.). IOW, don't create nodes just because you want to
> instantiate drivers that way. DT is not the only way to instantiate devices for
> drivers.
> 

Got it. Pushed v3 with suggested changes. Please review.

> Rob