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 patchset has dependency on below drivers:
Firmware Driver: https://patchwork.kernel.org/patch/10230773/
Jolly Shah (2):
dt-bindings: power: Add ZynqMP power domain bindings
drivers: soc: xilinx: Add ZynqMP power domain driver
.../devicetree/bindings/power/zynqmp-genpd.txt | 46 +++
drivers/soc/xilinx/Kconfig | 2 +
drivers/soc/xilinx/Makefile | 2 +
drivers/soc/xilinx/zynqmp/Kconfig | 16 +
drivers/soc/xilinx/zynqmp/Makefile | 4 +
drivers/soc/xilinx/zynqmp/pm_domains.c | 339 +++++++++++++++++++++
6 files changed, 409 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/zynqmp-genpd.txt
create mode 100644 drivers/soc/xilinx/zynqmp/Kconfig
create mode 100644 drivers/soc/xilinx/zynqmp/Makefile
create mode 100644 drivers/soc/xilinx/zynqmp/pm_domains.c
--
2.7.4
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: Jolly Shah <[email protected]>
Signed-off-by: Rajan Vaja <[email protected]>
---
drivers/soc/xilinx/Kconfig | 2 +
drivers/soc/xilinx/Makefile | 2 +
drivers/soc/xilinx/zynqmp/Kconfig | 16 ++
drivers/soc/xilinx/zynqmp/Makefile | 4 +
drivers/soc/xilinx/zynqmp/pm_domains.c | 339 +++++++++++++++++++++++++++++++++
5 files changed, 363 insertions(+)
create mode 100644 drivers/soc/xilinx/zynqmp/Kconfig
create mode 100644 drivers/soc/xilinx/zynqmp/Makefile
create mode 100644 drivers/soc/xilinx/zynqmp/pm_domains.c
diff --git a/drivers/soc/xilinx/Kconfig b/drivers/soc/xilinx/Kconfig
index 687c8f3..1c98d7f 100644
--- a/drivers/soc/xilinx/Kconfig
+++ b/drivers/soc/xilinx/Kconfig
@@ -17,4 +17,6 @@ config XILINX_VCU
To compile this driver as a module, choose M here: the
module will be called xlnx_vcu.
+source "drivers/soc/xilinx/zynqmp/Kconfig"
+
endmenu
diff --git a/drivers/soc/xilinx/Makefile b/drivers/soc/xilinx/Makefile
index dee8fd5..e132897 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_ARCH_ZYNQMP) += zynqmp/
diff --git a/drivers/soc/xilinx/zynqmp/Kconfig b/drivers/soc/xilinx/zynqmp/Kconfig
new file mode 100644
index 0000000..95d8665
--- /dev/null
+++ b/drivers/soc/xilinx/zynqmp/Kconfig
@@ -0,0 +1,16 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Kconfig for Xilinx zynqmp SoC
+
+menu "Zynq MPSoC SoC Drivers"
+ depends on ARCH_ZYNQMP
+
+config ZYNQMP_PM_DOMAINS
+ bool "Enable Zynq MPSoC generic PM domains"
+ default y
+ depends on PM
+ select PM_GENERIC_DOMAINS
+ help
+ Say yes to enable device power management through PM domains
+ In doubt, say N
+
+endmenu
diff --git a/drivers/soc/xilinx/zynqmp/Makefile b/drivers/soc/xilinx/zynqmp/Makefile
new file mode 100644
index 0000000..0bd522b
--- /dev/null
+++ b/drivers/soc/xilinx/zynqmp/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Makefile for Xilinx zynqmp SoC
+
+obj-$(CONFIG_ZYNQMP_PM_DOMAINS) += 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..7ddca8e
--- /dev/null
+++ b/drivers/soc/xilinx/zynqmp/pm_domains.c
@@ -0,0 +1,339 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * ZynqMP Generic PM domain support
+ *
+ * Copyright (C) 2015-2018 Xilinx, Inc.
+ *
+ * Davorin Mista <[email protected]>
+ * Jolly Shah <[email protected]>
+ * Rajan Vaja <[email protected]>
+ */
+
+#include <linux/err.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/list.h>
+#include <linux/firmware/xilinx/zynqmp/firmware.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, status = 0;
+ 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 status;
+
+ pd = container_of(domain, struct zynqmp_pm_domain, gpd);
+ for (i = 0; i < pd->node_id_num; i++) {
+ status = eemi_ops->set_requirement(pd->node_ids[i],
+ ZYNQMP_PM_CAPABILITY_ACCESS,
+ ZYNQMP_PM_MAX_QOS,
+ ZYNQMP_PM_REQUEST_ACK_BLOCKING);
+ if (status)
+ break;
+ }
+ return status;
+}
+
+/**
+ * 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, status = 0;
+ struct zynqmp_pm_domain *pd;
+ struct zynqmp_domain_device *zdev, *tmp;
+ u32 capabilities = 0;
+ bool may_wakeup = 0;
+ const struct zynqmp_eemi_ops *eemi_ops = zynqmp_pm_get_eemi_ops();
+
+ if (!eemi_ops || !eemi_ops->set_requirement)
+ return status;
+
+ 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--) {
+ status = 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 (status) {
+ pr_err("%s error %d, node %u\n", __func__, status,
+ pd->node_ids[i]);
+ return status;
+ }
+ }
+
+ return status;
+}
+
+/**
+ * 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, status;
+ 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++) {
+ status = 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 (status) {
+ pr_err("%s error %d, node %u\n", __func__,
+ status, pd->node_ids[i]);
+ list_del(&zdev->list);
+ zdev->dev = NULL;
+ devm_kfree(dev, zdev);
+ return status;
+ }
+ }
+
+ pd->flags |= ZYNQMP_PM_DOMAIN_REQUESTED;
+
+ 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, status;
+ 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++) {
+ status = eemi_ops->release_node(pd->node_ids[i]);
+ /* If releasing a node fails print the error and return */
+ if (status) {
+ pr_err("%s error %d, node %u\n", __func__, status,
+ pd->node_ids[i]);
+ return;
+ }
+ }
+
+ pd->flags &= ~ZYNQMP_PM_DOMAIN_REQUESTED;
+}
+
+/**
+ * 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 __init 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);
+ }
+
+ 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_gpd_of_match[] = {
+ { .compatible = "xlnx,zynqmp-genpd" },
+ {},
+};
+
+MODULE_DEVICE_TABLE(of, zynqmp_gpd_of_match);
+
+static struct platform_driver zynqmp_gpd_platform_driver = {
+ .driver = {
+ .name = "zynqmp_gpd",
+ .of_match_table = zynqmp_gpd_of_match,
+ },
+};
+
+static __init int zynqmp_gpd_init(void)
+{
+ return platform_driver_probe(&zynqmp_gpd_platform_driver,
+ zynqmp_gpd_probe);
+}
+subsys_initcall(zynqmp_gpd_init);
--
2.7.4
Add documentation to describe ZynqMP power domain bindings.
Signed-off-by: Jolly Shah <[email protected]>
Signed-off-by: Rajan Vaja <[email protected]>
---
.../devicetree/bindings/power/zynqmp-genpd.txt | 46 ++++++++++++++++++++++
1 file changed, 46 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/zynqmp-genpd.txt
diff --git a/Documentation/devicetree/bindings/power/zynqmp-genpd.txt b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
new file mode 100644
index 0000000..25f9711
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
@@ -0,0 +1,46 @@
+Device Tree bindings for Xilinx Zynq MPSoC PM domains
+
+The binding for zynqmp-genpd 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-genpd"
+
+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: List of domain identifiers of as defined by platform firmware. These
+ identifiers are passed to the PM firmware.
+
+Example:
+ zynqmp-genpd {
+ compatible = "xlnx,zynqmp-genpd";
+
+ 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 = <0x0>;
+ };
+ };
+
+ sata0: ahci@SATA_AHCI_HBA {
+ ...
+ power-domains = <&pd_sata>;
+ ...
+ };
--
2.7.4
Hi Jolly,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.16-rc3 next-20180302]
[cannot apply to xlnx/master mediatek/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Jolly-Shah/drivers-soc-xilinx-Add-support-for-ZynqMP-power-domain-driver/20180302-213151
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64
All errors (new ones prefixed by >>):
>> drivers/soc/xilinx/zynqmp/pm_domains.c:19:10: fatal error: linux/firmware/xilinx/zynqmp/firmware.h: No such file or directory
#include <linux/firmware/xilinx/zynqmp/firmware.h>
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
vim +19 drivers/soc/xilinx/zynqmp/pm_domains.c
> 19 #include <linux/firmware/xilinx/zynqmp/firmware.h>
20
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Tue, Feb 27, 2018 at 03:55:49PM -0800, Jolly Shah wrote:
> Add documentation to describe ZynqMP power domain bindings.
>
> Signed-off-by: Jolly Shah <[email protected]>
> Signed-off-by: Rajan Vaja <[email protected]>
> ---
> .../devicetree/bindings/power/zynqmp-genpd.txt | 46 ++++++++++++++++++++++
> 1 file changed, 46 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/power/zynqmp-genpd.txt
>
> diff --git a/Documentation/devicetree/bindings/power/zynqmp-genpd.txt b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
> new file mode 100644
> index 0000000..25f9711
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
> @@ -0,0 +1,46 @@
> +Device Tree bindings for Xilinx Zynq MPSoC PM domains
> +
> +The binding for zynqmp-genpd 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-genpd"
genpd is a Linux term. The DT should contain a power controller that
controls physical power islands on a chip.
> +
> +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: List of domain identifiers of as defined by platform firmware. These
> + identifiers are passed to the PM firmware.
> +
> +Example:
> + zynqmp-genpd {
> + compatible = "xlnx,zynqmp-genpd";
What's the control interface for controlling the domains?
> +
> + pd_usb0: pd-usb0 {
> + pd-id = <22>;
> + #power-domain-cells = <0>;
There's no need for all these sub nodes. Make #power-domain-cells 1 and
put the id in the cell value.
> + };
> +
> + pd_sata: pd-sata {
> + pd-id = <28>;
> + #power-domain-cells = <0>;
> + };
> +
> + pd_gpu: pd-gpu {
> + pd-id = <58 20 21>;
> + #power-domain-cells = <0x0>;
> + };
> + };
> +
> + sata0: ahci@SATA_AHCI_HBA {
> + ...
> + power-domains = <&pd_sata>;
> + ...
> + };
> --
> 2.7.4
>
Hi Rob, Jolly,
On Mon, Mar 5, 2018 at 11:39 PM, Rob Herring <[email protected]> wrote:
> On Tue, Feb 27, 2018 at 03:55:49PM -0800, Jolly Shah wrote:
>> Add documentation to describe ZynqMP power domain bindings.
>>
>> Signed-off-by: Jolly Shah <[email protected]>
>> Signed-off-by: Rajan Vaja <[email protected]>
>> ---
>> .../devicetree/bindings/power/zynqmp-genpd.txt | 46 ++++++++++++++++++++++
>> 1 file changed, 46 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/power/zynqmp-genpd.txt
>>
>> diff --git a/Documentation/devicetree/bindings/power/zynqmp-genpd.txt b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
>> new file mode 100644
>> index 0000000..25f9711
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
>> +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: List of domain identifiers of as defined by platform firmware. These
>> + identifiers are passed to the PM firmware.
>> +
>> +Example:
>> + zynqmp-genpd {
>> + compatible = "xlnx,zynqmp-genpd";
>
> What's the control interface for controlling the domains?
>> +
>> + pd_usb0: pd-usb0 {
>> + pd-id = <22>;
>> + #power-domain-cells = <0>;
>
> There's no need for all these sub nodes. Make #power-domain-cells 1 and
> put the id in the cell value.
That was my first reaction, too...
>
>> + };
>> +
>> + pd_sata: pd-sata {
>> + pd-id = <28>;
>> + #power-domain-cells = <0>;
>> + };
>> +
>> + pd_gpu: pd-gpu {
>> + pd-id = <58 20 21>;
... until I saw the above.
Controlling the GPU power area requires controlling 3 physical areas?
However, doing it this way may bite you in the future, if a need arises to
control a subset. And what about power up/down order?
>> + #power-domain-cells = <0x0>;
>> + };
>> + };
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi All,
On 2018-03-06 08:59, Geert Uytterhoeven wrote:
> Hi Rob, Jolly,
>
> On Mon, Mar 5, 2018 at 11:39 PM, Rob Herring <[email protected]> wrote:
>> On Tue, Feb 27, 2018 at 03:55:49PM -0800, Jolly Shah wrote:
>>> Add documentation to describe ZynqMP power domain bindings.
>>>
>>> Signed-off-by: Jolly Shah <[email protected]>
>>> Signed-off-by: Rajan Vaja <[email protected]>
>>> ---
>>> .../devicetree/bindings/power/zynqmp-genpd.txt | 46 ++++++++++++++++++++++
>>> 1 file changed, 46 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/power/zynqmp-genpd.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/power/zynqmp-genpd.txt b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
>>> new file mode 100644
>>> index 0000000..25f9711
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
>>> +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: List of domain identifiers of as defined by platform firmware. These
>>> + identifiers are passed to the PM firmware.
>>> +
>>> +Example:
>>> + zynqmp-genpd {
>>> + compatible = "xlnx,zynqmp-genpd";
>> What's the control interface for controlling the domains?
>>> +
>>> + pd_usb0: pd-usb0 {
>>> + pd-id = <22>;
>>> + #power-domain-cells = <0>;
>> There's no need for all these sub nodes. Make #power-domain-cells 1 and
>> put the id in the cell value.
> That was my first reaction, too...
>>> + };
>>> +
>>> + pd_sata: pd-sata {
>>> + pd-id = <28>;
>>> + #power-domain-cells = <0>;
>>> + };
>>> +
>>> + pd_gpu: pd-gpu {
>>> + pd-id = <58 20 21>;
> ... until I saw the above.
> Controlling the GPU power area requires controlling 3 physical areas?
>
> However, doing it this way may bite you in the future, if a need arises to
> control a subset. And what about power up/down order?
What about defining 3 separate domains and arranging them in parent-child
relationship? generic power domains already supports that and this allows
to nicely define the power on/off order.
>>> + #power-domain-cells = <0x0>;
>>> + };
>>> + };
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Hi Rob, Geert, Marek,
> -----Original Message-----
> From: Marek Szyprowski [mailto:[email protected]]
> Sent: Tuesday, March 06, 2018 12:06 AM
> To: Geert Uytterhoeven <[email protected]>; Rob Herring
> <[email protected]>
> Cc: Jolly Shah <[email protected]>; Matthias Brugger
> <[email protected]>; Andy Gross <[email protected]>; Shawn Guo
> <[email protected]>; Geert Uytterhoeven <[email protected]>;
> Björn Andersson <[email protected]>; [email protected];
> Michal Simek <[email protected]>; Mark Rutland
> <[email protected]>; Rajan Vaja <[email protected]>; open list:OPEN
> FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
> <[email protected]>; Linux ARM <linux-arm-
> [email protected]>; Linux Kernel Mailing List <linux-
> [email protected]>; Jolly Shah <[email protected]>
> Subject: Re: [PATCH 1/2] dt-bindings: power: Add ZynqMP power domain
> bindings
>
> Hi All,
>
> On 2018-03-06 08:59, Geert Uytterhoeven wrote:
> > Hi Rob, Jolly,
> >
> > On Mon, Mar 5, 2018 at 11:39 PM, Rob Herring <[email protected]> wrote:
> >> On Tue, Feb 27, 2018 at 03:55:49PM -0800, Jolly Shah wrote:
> >>> Add documentation to describe ZynqMP power domain bindings.
> >>>
> >>> Signed-off-by: Jolly Shah <[email protected]>
> >>> Signed-off-by: Rajan Vaja <[email protected]>
> >>> ---
> >>> .../devicetree/bindings/power/zynqmp-genpd.txt | 46
> ++++++++++++++++++++++
> >>> 1 file changed, 46 insertions(+)
> >>> create mode 100644
> >>> Documentation/devicetree/bindings/power/zynqmp-genpd.txt
> >>>
> >>> diff --git
> >>> a/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
> >>> b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
> >>> new file mode 100644
> >>> index 0000000..25f9711
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
> >>> +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: List of domain identifiers of as defined by platform firmware.
> These
> >>> + identifiers are passed to the PM firmware.
> >>> +
> >>> +Example:
> >>> + zynqmp-genpd {
> >>> + compatible = "xlnx,zynqmp-genpd";
> >> What's the control interface for controlling the domains?
> >>> +
> >>> + pd_usb0: pd-usb0 {
> >>> + pd-id = <22>;
> >>> + #power-domain-cells = <0>;
> >> There's no need for all these sub nodes. Make #power-domain-cells 1
> >> and put the id in the cell value.
> > That was my first reaction, too...
> >>> + };
> >>> +
> >>> + pd_sata: pd-sata {
> >>> + pd-id = <28>;
> >>> + #power-domain-cells = <0>;
> >>> + };
> >>> +
> >>> + pd_gpu: pd-gpu {
> >>> + pd-id = <58 20 21>;
> > ... until I saw the above.
> > Controlling the GPU power area requires controlling 3 physical areas?
> >
> > However, doing it this way may bite you in the future, if a need
> > arises to control a subset. And what about power up/down order?
>
> What about defining 3 separate domains and arranging them in parent-child
> relationship? generic power domains already supports that and this allows to
> nicely define the power on/off order.
>
> >>> + #power-domain-cells = <0x0>;
> >>> + };
> >>> + };
>
I agree it should be arranged in as parent child order to control subset or control order. Will incorporate those changes in next version.
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
Hi Marek,
> -----Original Message-----
> From: [email protected] [mailto:linux-kernel-
> [email protected]] On Behalf Of Jolly Shah
> Sent: Thursday, March 15, 2018 10:47 AM
> To: Marek Szyprowski <[email protected]>; Geert Uytterhoeven
> <[email protected]>; Rob Herring <[email protected]>
> Cc: Matthias Brugger <[email protected]>; Andy Gross
> <[email protected]>; Shawn Guo <[email protected]>; Geert
> Uytterhoeven <[email protected]>; Björn Andersson
> <[email protected]>; [email protected]; Michal Simek
> <[email protected]>; Mark Rutland <[email protected]>; Rajan
> Vaja <[email protected]>; open list:OPEN FIRMWARE AND FLATTENED
> DEVICE TREE BINDINGS <[email protected]>; Linux ARM <linux-arm-
> [email protected]>; Linux Kernel Mailing List <linux-
> [email protected]>
> Subject: RE: [PATCH 1/2] dt-bindings: power: Add ZynqMP power domain
> bindings
>
> [This sender failed our fraud detection checks and may not be who they appear
> to be. Learn about spoofing at http://aka.ms/LearnAboutSpoofing]
>
> Hi Rob, Geert, Marek,
>
> > -----Original Message-----
> > From: Marek Szyprowski [mailto:[email protected]]
> > Sent: Tuesday, March 06, 2018 12:06 AM
> > To: Geert Uytterhoeven <[email protected]>; Rob Herring
> > <[email protected]>
> > Cc: Jolly Shah <[email protected]>; Matthias Brugger
> > <[email protected]>; Andy Gross <[email protected]>; Shawn
> > Guo <[email protected]>; Geert Uytterhoeven
> > <[email protected]>; Björn Andersson
> > <[email protected]>; [email protected]; Michal Simek
> > <[email protected]>; Mark Rutland <[email protected]>; Rajan
> > Vaja <[email protected]>; open list:OPEN FIRMWARE AND FLATTENED
> DEVICE
> > TREE BINDINGS <[email protected]>; Linux ARM <linux-arm-
> > [email protected]>; Linux Kernel Mailing List <linux-
> > [email protected]>; Jolly Shah <[email protected]>
> > Subject: Re: [PATCH 1/2] dt-bindings: power: Add ZynqMP power domain
> > bindings
> >
> > Hi All,
> >
> > On 2018-03-06 08:59, Geert Uytterhoeven wrote:
> > > Hi Rob, Jolly,
> > >
> > > On Mon, Mar 5, 2018 at 11:39 PM, Rob Herring <[email protected]> wrote:
> > >> On Tue, Feb 27, 2018 at 03:55:49PM -0800, Jolly Shah wrote:
> > >>> Add documentation to describe ZynqMP power domain bindings.
> > >>>
> > >>> Signed-off-by: Jolly Shah <[email protected]>
> > >>> Signed-off-by: Rajan Vaja <[email protected]>
> > >>> ---
> > >>> .../devicetree/bindings/power/zynqmp-genpd.txt | 46
> > ++++++++++++++++++++++
> > >>> 1 file changed, 46 insertions(+)
> > >>> create mode 100644
> > >>> Documentation/devicetree/bindings/power/zynqmp-genpd.txt
> > >>>
> > >>> diff --git
> > >>> a/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
> > >>> b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
> > >>> new file mode 100644
> > >>> index 0000000..25f9711
> > >>> --- /dev/null
> > >>> +++ b/Documentation/devicetree/bindings/power/zynqmp-genpd.txt
> > >>> +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: List of domain identifiers of as defined by platform firmware.
> > These
> > >>> + identifiers are passed to the PM firmware.
> > >>> +
> > >>> +Example:
> > >>> + zynqmp-genpd {
> > >>> + compatible = "xlnx,zynqmp-genpd";
> > >> What's the control interface for controlling the domains?
> > >>> +
> > >>> + pd_usb0: pd-usb0 {
> > >>> + pd-id = <22>;
> > >>> + #power-domain-cells = <0>;
> > >> There's no need for all these sub nodes. Make #power-domain-cells 1
> > >> and put the id in the cell value.
> > > That was my first reaction, too...
> > >>> + };
> > >>> +
> > >>> + pd_sata: pd-sata {
> > >>> + pd-id = <28>;
> > >>> + #power-domain-cells = <0>;
> > >>> + };
> > >>> +
> > >>> + pd_gpu: pd-gpu {
> > >>> + pd-id = <58 20 21>;
> > > ... until I saw the above.
> > > Controlling the GPU power area requires controlling 3 physical areas?
> > >
> > > However, doing it this way may bite you in the future, if a need
> > > arises to control a subset. And what about power up/down order?
> >
> > What about defining 3 separate domains and arranging them in
> > parent-child relationship? generic power domains already supports that
> > and this allows to nicely define the power on/off order.
> >
> > >>> + #power-domain-cells = <0x0>;
> > >>> + };
> > >>> + };
> >
>
> I agree it should be arranged in as parent child order to control subset or control
> order. Will incorporate those changes in next version.
>
> > Best regards
> > --
> > Marek Szyprowski, PhD
> > Samsung R&D Institute Poland
As suggested, I tried out parent, child approach. However what I found is Genpd core takes care of parent child dependencies for power on off routines only. In our case, We need them in attach-detach routines too. In that case, we need to handle dependencies manually for those routines. Please suggest better approach, if any.
Thanks,
Jolly Shah
Hi Jolly,
On 2018-05-17 23:10, Jolly Shah wrote:
>>>>>> +Example:
>>>>>> + zynqmp-genpd {
>>>>>> + compatible = "xlnx,zynqmp-genpd";
>>>>> What's the control interface for controlling the domains?
>>>>>> +
>>>>>> + pd_usb0: pd-usb0 {
>>>>>> + pd-id = <22>;
>>>>>> + #power-domain-cells = <0>;
>>>>> There's no need for all these sub nodes. Make #power-domain-cells 1
>>>>> and put the id in the cell value.
>>>> That was my first reaction, too...
>>>>>> + };
>>>>>> +
>>>>>> + pd_sata: pd-sata {
>>>>>> + pd-id = <28>;
>>>>>> + #power-domain-cells = <0>;
>>>>>> + };
>>>>>> +
>>>>>> + pd_gpu: pd-gpu {
>>>>>> + pd-id = <58 20 21>;
>>>> ... until I saw the above.
>>>> Controlling the GPU power area requires controlling 3 physical areas?
>>>>
>>>> However, doing it this way may bite you in the future, if a need
>>>> arises to control a subset. And what about power up/down order?
>>> What about defining 3 separate domains and arranging them in
>>> parent-child relationship? generic power domains already supports that
>>> and this allows to nicely define the power on/off order.
>>>
>>>>>> + #power-domain-cells = <0x0>;
>>>>>> + };
>>>>>> + };
>> I agree it should be arranged in as parent child order to control subset or control
>> order. Will incorporate those changes in next version.
>
> As suggested, I tried out parent, child approach. However what I found is Genpd core takes care of parent child dependencies for power on off routines only. In our case, We need them in attach-detach routines too. In that case, we need to handle dependencies manually for those routines. Please suggest better approach, if any.
What do you mean to handle attach-detach?
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Hi Marek,
> -----Original Message-----
> From: Marek Szyprowski [mailto:[email protected]]
> Sent: Thursday, May 17, 2018 11:31 PM
> To: Jolly Shah <[email protected]>; Geert Uytterhoeven <geert@linux-
> m68k.org>; Rob Herring <[email protected]>
> Cc: Matthias Brugger <[email protected]>; Andy Gross
> <[email protected]>; Shawn Guo <[email protected]>; Geert
> Uytterhoeven <[email protected]>; Björn Andersson
> <[email protected]>; [email protected]; Michal Simek
> <[email protected]>; Mark Rutland <[email protected]>; Rajan
> Vaja <[email protected]>; open list:OPEN FIRMWARE AND FLATTENED
> DEVICE TREE BINDINGS <[email protected]>; Linux ARM <linux-arm-
> [email protected]>; Linux Kernel Mailing List <linux-
> [email protected]>
> Subject: Re: [PATCH 1/2] dt-bindings: power: Add ZynqMP power domain
> bindings
>
> Hi Jolly,
>
> On 2018-05-17 23:10, Jolly Shah wrote:
>
> >>>>>> +Example:
> >>>>>> + zynqmp-genpd {
> >>>>>> + compatible = "xlnx,zynqmp-genpd";
> >>>>> What's the control interface for controlling the domains?
> >>>>>> +
> >>>>>> + pd_usb0: pd-usb0 {
> >>>>>> + pd-id = <22>;
> >>>>>> + #power-domain-cells = <0>;
> >>>>> There's no need for all these sub nodes. Make #power-domain-cells
> >>>>> 1 and put the id in the cell value.
> >>>> That was my first reaction, too...
> >>>>>> + };
> >>>>>> +
> >>>>>> + pd_sata: pd-sata {
> >>>>>> + pd-id = <28>;
> >>>>>> + #power-domain-cells = <0>;
> >>>>>> + };
> >>>>>> +
> >>>>>> + pd_gpu: pd-gpu {
> >>>>>> + pd-id = <58 20 21>;
> >>>> ... until I saw the above.
> >>>> Controlling the GPU power area requires controlling 3 physical areas?
> >>>>
> >>>> However, doing it this way may bite you in the future, if a need
> >>>> arises to control a subset. And what about power up/down order?
> >>> What about defining 3 separate domains and arranging them in
> >>> parent-child relationship? generic power domains already supports
> >>> that and this allows to nicely define the power on/off order.
> >>>
> >>>>>> + #power-domain-cells = <0x0>;
> >>>>>> + };
> >>>>>> + };
> >> I agree it should be arranged in as parent child order to control
> >> subset or control order. Will incorporate those changes in next version.
> >
> > As suggested, I tried out parent, child approach. However what I found is
> Genpd core takes care of parent child dependencies for power on off routines
> only. In our case, We need them in attach-detach routines too. In that case, we
> need to handle dependencies manually for those routines. Please suggest better
> approach, if any.
>
> What do you mean to handle attach-detach?
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
For our power domain driver, we request usage of these nodes in attach routines and power them on in power on routine. So for below specific case, when attach_dev is called, all 3 nodes need to be requested.
> >>>>>> + pd_gpu: pd-gpu {
> >>>>>> + pd-id = <58 20 21>;
Thanks,
Jolly Shah
Hi Marek,
> -----Original Message-----
> From: Jolly Shah
> Sent: Friday, May 18, 2018 2:19 PM
> To: 'Marek Szyprowski' <[email protected]>
> Cc: Matthias Brugger <[email protected]>; Andy Gross
> <[email protected]>; Shawn Guo <[email protected]>; Geert
> Uytterhoeven <[email protected]>; Björn Andersson
> <[email protected]>; [email protected]; Michal Simek
> <[email protected]>; Mark Rutland <[email protected]>; Rajan
> Vaja <[email protected]>; open list:OPEN FIRMWARE AND FLATTENED
> DEVICE TREE BINDINGS <[email protected]>; Linux ARM <linux-arm-
> [email protected]>; Linux Kernel Mailing List <linux-
> [email protected]>; Geert Uytterhoeven <[email protected]>; Rob
> Herring <[email protected]>
> Subject: RE: [PATCH 1/2] dt-bindings: power: Add ZynqMP power domain
> bindings
>
> Hi Marek,
>
> > -----Original Message-----
> > From: Marek Szyprowski [mailto:[email protected]]
> > Sent: Thursday, May 17, 2018 11:31 PM
> > To: Jolly Shah <[email protected]>; Geert Uytterhoeven <geert@linux-
> > m68k.org>; Rob Herring <[email protected]>
> > Cc: Matthias Brugger <[email protected]>; Andy Gross
> > <[email protected]>; Shawn Guo <[email protected]>; Geert
> > Uytterhoeven <[email protected]>; Björn Andersson
> > <[email protected]>; [email protected]; Michal Simek
> > <[email protected]>; Mark Rutland <[email protected]>; Rajan
> > Vaja <[email protected]>; open list:OPEN FIRMWARE AND FLATTENED
> DEVICE
> > TREE BINDINGS <[email protected]>; Linux ARM <linux-arm-
> > [email protected]>; Linux Kernel Mailing List <linux-
> > [email protected]>
> > Subject: Re: [PATCH 1/2] dt-bindings: power: Add ZynqMP power domain
> > bindings
> >
> > Hi Jolly,
> >
> > On 2018-05-17 23:10, Jolly Shah wrote:
> >
> > >>>>>> +Example:
> > >>>>>> + zynqmp-genpd {
> > >>>>>> + compatible = "xlnx,zynqmp-genpd";
> > >>>>> What's the control interface for controlling the domains?
> > >>>>>> +
> > >>>>>> + pd_usb0: pd-usb0 {
> > >>>>>> + pd-id = <22>;
> > >>>>>> + #power-domain-cells = <0>;
> > >>>>> There's no need for all these sub nodes. Make
> > >>>>> #power-domain-cells
> > >>>>> 1 and put the id in the cell value.
> > >>>> That was my first reaction, too...
> > >>>>>> + };
> > >>>>>> +
> > >>>>>> + pd_sata: pd-sata {
> > >>>>>> + pd-id = <28>;
> > >>>>>> + #power-domain-cells = <0>;
> > >>>>>> + };
> > >>>>>> +
> > >>>>>> + pd_gpu: pd-gpu {
> > >>>>>> + pd-id = <58 20 21>;
> > >>>> ... until I saw the above.
> > >>>> Controlling the GPU power area requires controlling 3 physical areas?
> > >>>>
> > >>>> However, doing it this way may bite you in the future, if a need
> > >>>> arises to control a subset. And what about power up/down order?
> > >>> What about defining 3 separate domains and arranging them in
> > >>> parent-child relationship? generic power domains already supports
> > >>> that and this allows to nicely define the power on/off order.
> > >>>
> > >>>>>> + #power-domain-cells = <0x0>;
> > >>>>>> + };
> > >>>>>> + };
> > >> I agree it should be arranged in as parent child order to control
> > >> subset or control order. Will incorporate those changes in next version.
> > >
> > > As suggested, I tried out parent, child approach. However what I
> > > found is
> > Genpd core takes care of parent child dependencies for power on off
> > routines only. In our case, We need them in attach-detach routines
> > too. In that case, we need to handle dependencies manually for those
> > routines. Please suggest better approach, if any.
> >
> > What do you mean to handle attach-detach?
> >
> > Best regards
> > --
> > Marek Szyprowski, PhD
> > Samsung R&D Institute Poland
>
> For our power domain driver, we request usage of these nodes in attach
> routines and power them on in power on routine. So for below specific case,
> when attach_dev is called, all 3 nodes need to be requested.
>
> > >>>>>> + pd_gpu: pd-gpu {
> > >>>>>> + pd-id = <58 20 21>;
>
> Thanks,
> Jolly Shah
>
Please let me know your opinion.
Thanks,
Jolly Shah