[RFC PATCH 0/2] Introduce multi PM domains helpers
From: Daniel Baluta daniel.baluta@nxp.com
i.MX8QXP/i.MX8QM has IPs that need multiple power domains to be up in order to work. In order to help drivers, we introduce multi PM domains helpers that are able to activate/deactivate multi PM domains.
First patch introduces the helpers and second patch demonstrates how a driver can use them instead of hardcoding the PM domains handling.
Daniel Baluta (2): PM / domains: Introduce multi PM domains helpers ASoC: SOF: Use multi PM domains helpers
drivers/base/power/common.c | 93 +++++++++++++++++++++++++++++++++++++ include/linux/pm_domain.h | 19 ++++++++ sound/soc/sof/imx/imx8.c | 54 +++------------------ 3 files changed, 118 insertions(+), 48 deletions(-)
From: Daniel Baluta daniel.baluta@nxp.com
This patch introduces helpers support for multi PM domains.
API consists of:
1) dev_multi_pm_attach - powers up all PM domains associated with a given device. Because we can attach one PM domain per device, we create virtual devices (children of initial device) and associate PM domains one per virtual device.
2) dev_multi_pm_detach - detaches all virtual devices from PM domains attached with.
Signed-off-by: Daniel Baluta daniel.baluta@nxp.com --- drivers/base/power/common.c | 93 +++++++++++++++++++++++++++++++++++++ include/linux/pm_domain.h | 19 ++++++++ 2 files changed, 112 insertions(+)
diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c index bbddb267c2e6..a90cc6b476e4 100644 --- a/drivers/base/power/common.c +++ b/drivers/base/power/common.c @@ -228,3 +228,96 @@ void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd) device_pm_check_callbacks(dev); } EXPORT_SYMBOL_GPL(dev_pm_domain_set); + +/** + * dev_multi_pm_attach - power up device associated power domains + * @dev: The device used to lookup the PM domains + * + * Parse device's OF node to find all PM domains specifiers. For each power + * domain found, create a virtual device and associate it with the + * current power domain. + * + * This function should typically be invoked by a driver during the + * probe phase, in the case its device requires power management through + * multiple PM domains. + * + * Returns a pointer to @dev_multi_pm_domain_data if successfully attached PM + * domains, NULL if 0 or 1 PM domains specified, else an ERR_PTR() in case of + * failures. + */ +struct dev_multi_pm_domain_data *dev_multi_pm_attach(struct device *dev) +{ + struct dev_multi_pm_domain_data *mpd, *retp; + int num_domains; + int i; + + num_domains = of_count_phandle_with_args(dev->of_node, "power-domains", + "#power-domain-cells"); + if (num_domains < 2) + return NULL; + + mpd = devm_kzalloc(dev, GFP_KERNEL, sizeof(*mpd)); + if (!mpd) + return ERR_PTR(-ENOMEM); + + mpd->dev = dev; + mpd->num_domains = num_domains; + + mpd->virt_devs = devm_kmalloc_array(dev, mpd->num_domains, + sizeof(*mpd->virt_devs), + GFP_KERNEL); + if (!mpd->virt_devs) + return ERR_PTR(-ENOMEM); + + mpd->links = devm_kmalloc_array(dev, mpd->num_domains, + sizeof(*mpd->links), GFP_KERNEL); + if (!mpd->links) + return ERR_PTR(-ENOMEM); + + for (i = 0; i < mpd->num_domains; i++) { + mpd->virt_devs[i] = dev_pm_domain_attach_by_id(dev, i); + if (IS_ERR(mpd->virt_devs[i])) { + retp = (struct dev_multi_pm_domain_data *) + mpd->virt_devs[i]; + goto exit_unroll_pm; + } + mpd->links[i] = device_link_add(dev, mpd->virt_devs[i], + DL_FLAG_STATELESS | + DL_FLAG_PM_RUNTIME | + DL_FLAG_RPM_ACTIVE); + if (!mpd->links[i]) { + retp = ERR_PTR(-ENOMEM); + dev_pm_domain_detach(mpd->virt_devs[i], false); + goto exit_unroll_pm; + } + } + return mpd; + +exit_unroll_pm: + while (--i >= 0) { + device_link_del(mpd->links[i]); + dev_pm_domain_detach(mpd->virt_devs[i], false); + } + + return retp; +} +EXPORT_SYMBOL(dev_multi_pm_attach); + +/** + * dev_multi_pm_detach - Detach a device from its PM domains. + * Each multi power domain is attached to a virtual children device + * + * @mpd: multi power domains data, contains the association between + * virtul device and PM domain + * + */ +void dev_multi_pm_detach(struct dev_multi_pm_domain_data *mpd) +{ + int i; + + for (i = 0; i < mpd->num_domains; i++) { + device_link_del(mpd->links[i]); + dev_pm_domain_detach(mpd->virt_devs[i], false); + } +} +EXPORT_SYMBOL(dev_multi_pm_detach); diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index 9ec78ee53652..5bcb35150af2 100644 --- a/include/linux/pm_domain.h +++ b/include/linux/pm_domain.h @@ -183,6 +183,13 @@ struct generic_pm_domain_data { void *data; };
+struct dev_multi_pm_domain_data { + struct device *dev; /* parent device */ + struct device **virt_devs; /* virtual children links */ + struct device_link **links; /* links parent <-> virtual children */ + int num_domains; +}; + #ifdef CONFIG_PM_GENERIC_DOMAINS static inline struct generic_pm_domain_data *to_gpd_data(struct pm_domain_data *pdd) { @@ -369,18 +376,27 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
#ifdef CONFIG_PM int dev_pm_domain_attach(struct device *dev, bool power_on); +struct dev_multi_pm_domain_data *dev_multi_pm_attach(struct device *dev); struct device *dev_pm_domain_attach_by_id(struct device *dev, unsigned int index); struct device *dev_pm_domain_attach_by_name(struct device *dev, const char *name); void dev_pm_domain_detach(struct device *dev, bool power_off); int dev_pm_domain_start(struct device *dev); +void dev_multi_pm_detach(struct dev_multi_pm_domain_data *mpd); void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd); + #else static inline int dev_pm_domain_attach(struct device *dev, bool power_on) { return 0; } + +struct dev_multi_pm_domain_data *dev_multi_pm_attach(struct device *dev) +{ + return NULL; +} + static inline struct device *dev_pm_domain_attach_by_id(struct device *dev, unsigned int index) { @@ -396,6 +412,9 @@ static inline int dev_pm_domain_start(struct device *dev) { return 0; } + +void dev_multi_pm_detach(struct dev_multi_pm_domain_data *mpd) {} + static inline void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd) {} #endif
On Mon, Mar 2, 2020 at 12:59 PM Daniel Baluta daniel.baluta@oss.nxp.com wrote:
From: Daniel Baluta daniel.baluta@nxp.com
This patch introduces helpers support for multi PM domains.
API consists of:
- dev_multi_pm_attach - powers up all PM domains associated with a given
device. Because we can attach one PM domain per device, we create virtual devices (children of initial device) and associate PM domains one per virtual device.
- dev_multi_pm_detach - detaches all virtual devices from PM domains
attached with.
Signed-off-by: Daniel Baluta daniel.baluta@nxp.com
drivers/base/power/common.c | 93 +++++++++++++++++++++++++++++++++++++ include/linux/pm_domain.h | 19 ++++++++ 2 files changed, 112 insertions(+)
diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c index bbddb267c2e6..a90cc6b476e4 100644 --- a/drivers/base/power/common.c +++ b/drivers/base/power/common.c @@ -228,3 +228,96 @@ void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd) device_pm_check_callbacks(dev); } EXPORT_SYMBOL_GPL(dev_pm_domain_set);
+/**
- dev_multi_pm_attach - power up device associated power domains
- @dev: The device used to lookup the PM domains
- Parse device's OF node to find all PM domains specifiers. For each
power
- domain found, create a virtual device and associate it with the
- current power domain.
- This function should typically be invoked by a driver during the
- probe phase, in the case its device requires power management through
- multiple PM domains.
- Returns a pointer to @dev_multi_pm_domain_data if successfully
attached PM
- domains, NULL if 0 or 1 PM domains specified, else an ERR_PTR() in
case of
- failures.
- */
+struct dev_multi_pm_domain_data *dev_multi_pm_attach(struct device *dev) +{
struct dev_multi_pm_domain_data *mpd, *retp;int num_domains;int i;num_domains = of_count_phandle_with_args(dev->of_node,"power-domains",
"#power-domain-cells");if (num_domains < 2)
Hi Daniel,
Just out of curiosity, should it be an error when num_domains is 1? Is it an error because the expectation is that the caller would use dev_pm_domain_attach() in that case?
+ return NULL;
mpd = devm_kzalloc(dev, GFP_KERNEL, sizeof(*mpd));if (!mpd)return ERR_PTR(-ENOMEM);mpd->dev = dev;mpd->num_domains = num_domains;mpd->virt_devs = devm_kmalloc_array(dev, mpd->num_domains,sizeof(*mpd->virt_devs),GFP_KERNEL);if (!mpd->virt_devs)return ERR_PTR(-ENOMEM);mpd->links = devm_kmalloc_array(dev, mpd->num_domains,sizeof(*mpd->links), GFP_KERNEL);if (!mpd->links)return ERR_PTR(-ENOMEM);for (i = 0; i < mpd->num_domains; i++) {mpd->virt_devs[i] = dev_pm_domain_attach_by_id(dev, i);if (IS_ERR(mpd->virt_devs[i])) {retp = (struct dev_multi_pm_domain_data *)mpd->virt_devs[i];
Should retp be PTR_ERR(mpd->virt_devs[i]) here? Thanks, Ranjani
Hello Ranjani,
On Mon, Mar 2, 2020 at 11:24 PM Sridharan, Ranjani ranjani.sridharan@intel.com wrote:
- */
+struct dev_multi_pm_domain_data *dev_multi_pm_attach(struct device *dev) +{
struct dev_multi_pm_domain_data *mpd, *retp;int num_domains;int i;num_domains = of_count_phandle_with_args(dev->of_node, "power-domains","#power-domain-cells");if (num_domains < 2)Hi Daniel,
Just out of curiosity, should it be an error when num_domains is 1? Is it an error because the expectation is that the caller would use dev_pm_domain_attach() in that case?
NULL here doesn't really mean an error. It means that we don't need to handle Power domains because as you said the caller already used dev_pm_domain_attach.
Similar with this:
$ drivers/base/power/domain.c +2504
int genpd_dev_pm_attach(struct device *dev)
/ * Devices with multiple PM domains must be attached separately, as we * can only attach one PM domain per device. */ if (of_count_phandle_with_args(dev->of_node, "power-domains", "#power-domain-cells") != 1) return 0;
Will update the description for when this function returns a NULL.
return NULL;mpd = devm_kzalloc(dev, GFP_KERNEL, sizeof(*mpd));if (!mpd)return ERR_PTR(-ENOMEM);mpd->dev = dev;mpd->num_domains = num_domains;mpd->virt_devs = devm_kmalloc_array(dev, mpd->num_domains,sizeof(*mpd->virt_devs),GFP_KERNEL);if (!mpd->virt_devs)return ERR_PTR(-ENOMEM);mpd->links = devm_kmalloc_array(dev, mpd->num_domains,sizeof(*mpd->links), GFP_KERNEL);if (!mpd->links)return ERR_PTR(-ENOMEM);for (i = 0; i < mpd->num_domains; i++) {mpd->virt_devs[i] = dev_pm_domain_attach_by_id(dev, i);if (IS_ERR(mpd->virt_devs[i])) {retp = (struct dev_multi_pm_domain_data *)mpd->virt_devs[i];Should retp be PTR_ERR(mpd->virt_devs[i]) here?
PTR_ERR returns a long but our function needs to return struct dev_multi_pm_domain_data *.
Thanks, Ranjani
From: Daniel Baluta daniel.baluta@nxp.com
Use dev_multi_pm_attach / dev_multi_pm_detach instead of the hardcoded version.
Signed-off-by: Daniel Baluta daniel.baluta@nxp.com --- sound/soc/sof/imx/imx8.c | 54 +++++----------------------------------- 1 file changed, 6 insertions(+), 48 deletions(-)
diff --git a/sound/soc/sof/imx/imx8.c b/sound/soc/sof/imx/imx8.c index b692752b2178..ca740538a2d5 100644 --- a/sound/soc/sof/imx/imx8.c +++ b/sound/soc/sof/imx/imx8.c @@ -51,10 +51,7 @@ struct imx8_priv { struct imx_sc_ipc *sc_ipc;
/* Power domain handling */ - int num_domains; - struct device **pd_dev; - struct device_link **link; - + struct dev_multi_pm_domain_data *mpd; };
static void imx8_get_reply(struct snd_sof_dev *sdev) @@ -207,7 +204,6 @@ static int imx8_probe(struct snd_sof_dev *sdev) struct resource res; u32 base, size; int ret = 0; - int i;
priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); if (!priv) @@ -218,39 +214,9 @@ static int imx8_probe(struct snd_sof_dev *sdev) priv->sdev = sdev;
/* power up device associated power domains */ - priv->num_domains = of_count_phandle_with_args(np, "power-domains", - "#power-domain-cells"); - if (priv->num_domains < 0) { - dev_err(sdev->dev, "no power-domains property in %pOF\n", np); - return priv->num_domains; - } - - priv->pd_dev = devm_kmalloc_array(&pdev->dev, priv->num_domains, - sizeof(*priv->pd_dev), GFP_KERNEL); - if (!priv->pd_dev) - return -ENOMEM; - - priv->link = devm_kmalloc_array(&pdev->dev, priv->num_domains, - sizeof(*priv->link), GFP_KERNEL); - if (!priv->link) - return -ENOMEM; - - for (i = 0; i < priv->num_domains; i++) { - priv->pd_dev[i] = dev_pm_domain_attach_by_id(&pdev->dev, i); - if (IS_ERR(priv->pd_dev[i])) { - ret = PTR_ERR(priv->pd_dev[i]); - goto exit_unroll_pm; - } - priv->link[i] = device_link_add(&pdev->dev, priv->pd_dev[i], - DL_FLAG_STATELESS | - DL_FLAG_PM_RUNTIME | - DL_FLAG_RPM_ACTIVE); - if (!priv->link[i]) { - ret = -ENOMEM; - dev_pm_domain_detach(priv->pd_dev[i], false); - goto exit_unroll_pm; - } - } + priv->mpd = dev_multi_pm_attach(&pdev->dev); + if (IS_ERR(priv->mpd)) + return PTR_ERR(priv->mpd);
ret = imx_scu_get_handle(&priv->sc_ipc); if (ret) { @@ -329,25 +295,17 @@ static int imx8_probe(struct snd_sof_dev *sdev) exit_pdev_unregister: platform_device_unregister(priv->ipc_dev); exit_unroll_pm: - while (--i >= 0) { - device_link_del(priv->link[i]); - dev_pm_domain_detach(priv->pd_dev[i], false); - } - + dev_multi_pm_detach(priv->mpd); return ret; }
static int imx8_remove(struct snd_sof_dev *sdev) { struct imx8_priv *priv = (struct imx8_priv *)sdev->private; - int i;
platform_device_unregister(priv->ipc_dev);
- for (i = 0; i < priv->num_domains; i++) { - device_link_del(priv->link[i]); - dev_pm_domain_detach(priv->pd_dev[i], false); - } + dev_multi_pm_detach(priv->mpd);
return 0; }
On Mon, Mar 2, 2020 at 1:00 PM Daniel Baluta daniel.baluta@oss.nxp.com wrote:
From: Daniel Baluta daniel.baluta@nxp.com
Use dev_multi_pm_attach / dev_multi_pm_detach instead of the hardcoded version.
Signed-off-by: Daniel Baluta daniel.baluta@nxp.com
sound/soc/sof/imx/imx8.c | 54 +++++----------------------------------- 1 file changed, 6 insertions(+), 48 deletions(-)
diff --git a/sound/soc/sof/imx/imx8.c b/sound/soc/sof/imx/imx8.c index b692752b2178..ca740538a2d5 100644 --- a/sound/soc/sof/imx/imx8.c +++ b/sound/soc/sof/imx/imx8.c @@ -51,10 +51,7 @@ struct imx8_priv { struct imx_sc_ipc *sc_ipc;
/* Power domain handling */
int num_domains;struct device **pd_dev;struct device_link **link;
struct dev_multi_pm_domain_data *mpd;};
static void imx8_get_reply(struct snd_sof_dev *sdev) @@ -207,7 +204,6 @@ static int imx8_probe(struct snd_sof_dev *sdev) struct resource res; u32 base, size; int ret = 0;
int i; priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); if (!priv)@@ -218,39 +214,9 @@ static int imx8_probe(struct snd_sof_dev *sdev) priv->sdev = sdev;
/* power up device associated power domains */
priv->num_domains = of_count_phandle_with_args(np, "power-domains","#power-domain-cells");
if (priv->num_domains < 0) {dev_err(sdev->dev, "no power-domains property in %pOF\n",np);
return priv->num_domains;}priv->pd_dev = devm_kmalloc_array(&pdev->dev, priv->num_domains,sizeof(*priv->pd_dev),GFP_KERNEL);
if (!priv->pd_dev)return -ENOMEM;priv->link = devm_kmalloc_array(&pdev->dev, priv->num_domains,sizeof(*priv->link), GFP_KERNEL);if (!priv->link)return -ENOMEM;for (i = 0; i < priv->num_domains; i++) {priv->pd_dev[i] = dev_pm_domain_attach_by_id(&pdev->dev,i);
if (IS_ERR(priv->pd_dev[i])) {ret = PTR_ERR(priv->pd_dev[i]);goto exit_unroll_pm;}priv->link[i] = device_link_add(&pdev->dev,priv->pd_dev[i],
DL_FLAG_STATELESS |DL_FLAG_PM_RUNTIME |DL_FLAG_RPM_ACTIVE);if (!priv->link[i]) {ret = -ENOMEM;dev_pm_domain_detach(priv->pd_dev[i], false);goto exit_unroll_pm;}}
priv->mpd = dev_multi_pm_attach(&pdev->dev);if (IS_ERR(priv->mpd))return PTR_ERR(priv->mpd); ret = imx_scu_get_handle(&priv->sc_ipc); if (ret) {@@ -329,25 +295,17 @@ static int imx8_probe(struct snd_sof_dev *sdev) exit_pdev_unregister: platform_device_unregister(priv->ipc_dev); exit_unroll_pm:
Can we also rename the label to exit_pm_detach maybe? It is no longer an unroll anymore right?
Thanks, Ranjani
On Mon, Mar 2, 2020 at 11:26 PM Sridharan, Ranjani ranjani.sridharan@intel.com wrote:
On Mon, Mar 2, 2020 at 1:00 PM Daniel Baluta daniel.baluta@oss.nxp.com wrote:
From: Daniel Baluta daniel.baluta@nxp.com
Use dev_multi_pm_attach / dev_multi_pm_detach instead of the hardcoded version.
Signed-off-by: Daniel Baluta daniel.baluta@nxp.com
sound/soc/sof/imx/imx8.c | 54 +++++----------------------------------- 1 file changed, 6 insertions(+), 48 deletions(-)
diff --git a/sound/soc/sof/imx/imx8.c b/sound/soc/sof/imx/imx8.c index b692752b2178..ca740538a2d5 100644 --- a/sound/soc/sof/imx/imx8.c +++ b/sound/soc/sof/imx/imx8.c @@ -51,10 +51,7 @@ struct imx8_priv { struct imx_sc_ipc *sc_ipc;
/* Power domain handling */
int num_domains;struct device **pd_dev;struct device_link **link;
struct dev_multi_pm_domain_data *mpd;};
static void imx8_get_reply(struct snd_sof_dev *sdev) @@ -207,7 +204,6 @@ static int imx8_probe(struct snd_sof_dev *sdev) struct resource res; u32 base, size; int ret = 0;
int i; priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); if (!priv)@@ -218,39 +214,9 @@ static int imx8_probe(struct snd_sof_dev *sdev) priv->sdev = sdev;
/* power up device associated power domains */
priv->num_domains = of_count_phandle_with_args(np, "power-domains","#power-domain-cells");if (priv->num_domains < 0) {dev_err(sdev->dev, "no power-domains property in %pOF\n", np);return priv->num_domains;}priv->pd_dev = devm_kmalloc_array(&pdev->dev, priv->num_domains,sizeof(*priv->pd_dev), GFP_KERNEL);if (!priv->pd_dev)return -ENOMEM;priv->link = devm_kmalloc_array(&pdev->dev, priv->num_domains,sizeof(*priv->link), GFP_KERNEL);if (!priv->link)return -ENOMEM;for (i = 0; i < priv->num_domains; i++) {priv->pd_dev[i] = dev_pm_domain_attach_by_id(&pdev->dev, i);if (IS_ERR(priv->pd_dev[i])) {ret = PTR_ERR(priv->pd_dev[i]);goto exit_unroll_pm;}priv->link[i] = device_link_add(&pdev->dev, priv->pd_dev[i],DL_FLAG_STATELESS |DL_FLAG_PM_RUNTIME |DL_FLAG_RPM_ACTIVE);if (!priv->link[i]) {ret = -ENOMEM;dev_pm_domain_detach(priv->pd_dev[i], false);goto exit_unroll_pm;}}
priv->mpd = dev_multi_pm_attach(&pdev->dev);if (IS_ERR(priv->mpd))return PTR_ERR(priv->mpd); ret = imx_scu_get_handle(&priv->sc_ipc); if (ret) {@@ -329,25 +295,17 @@ static int imx8_probe(struct snd_sof_dev *sdev) exit_pdev_unregister: platform_device_unregister(priv->ipc_dev); exit_unroll_pm:
Can we also rename the label to exit_pm_detach maybe? It is no longer an unroll anymore right?
Sure, will do in v2.
participants (3)
-
Daniel Baluta -
Daniel Baluta -
Sridharan, Ranjani