lkml.org 
[lkml]   [2020]   [Sep]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH v2 3/3] PM / Domains: Add support for PM domain on/off notifiers for genpd
Date
Hi Ulf,

> Subject: [PATCH v2 3/3] PM / Domains: Add support for PM domain on/off
> notifiers for genpd
>
> A device may have specific HW constraints that must be obeyed to, before its
> corresponding PM domain (genpd) can be powered off - and vice verse at
> power on. These constraints can't be managed through the regular runtime
> PM based deployment for a device, because the access pattern for it, isn't
> always request based. In other words, using the runtime PM callbacks to deal
> with the constraints doesn't work for these cases.

Could the notification be added before/after power on, and before/after power
off? not just after power on and before power off?

Our SoC has a requirement that before power on/off the specific module,
the corresponding clk needs to be on to make sure the hardware async
bridge could finish handshake.

So we need clk_prepare_on/off to guard power on and power off as below:

clk_prepare_on
power on
clk_prepare_off

clk_prepare_on
power off
clk_prepare_off

Thanks,
Peng.

>
> For these reasons, let's instead add a PM domain power on/off notification
> mechanism to genpd. To add/remove a notifier for a device, the device must
> already have been attached to the genpd, which also means that it needs to
> be a part of the PM domain topology.
>
> To add/remove a notifier, let's introduce two genpd specific functions:
> - dev_pm_genpd_add|remove_notifier()
>
> Note that, to further clarify when genpd power on/off notifiers may be used,
> one can compare with the existing CPU_CLUSTER_PM_ENTER|EXIT notifiers.
> In the long run, the genpd power on/off notifiers should be able to replace
> them, but that requires additional genpd based platform support for the
> current users.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>
> Changes in v2:
> - Improved error handling from fired notifications, according to
> suggestions by Lina Iyer.
>
> ---
> drivers/base/power/domain.c | 142
> ++++++++++++++++++++++++++++++++++--
> include/linux/pm_domain.h | 15 ++++
> 2 files changed, 152 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 0198af358503..f001ac6326fb 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -497,7 +497,7 @@ static int genpd_power_off(struct
> generic_pm_domain *genpd, bool one_dev_on,
> struct pm_domain_data *pdd;
> struct gpd_link *link;
> unsigned int not_suspended = 0;
> - int ret;
> + int ret, nr_calls = 0;
>
> /*
> * Do not try to power off the domain in the following situations:
> @@ -545,13 +545,22 @@ static int genpd_power_off(struct
> generic_pm_domain *genpd, bool one_dev_on,
> if (!genpd->gov)
> genpd->state_idx = 0;
>
> + /* Notify consumers that we are about to power off. */
> + ret = __raw_notifier_call_chain(&genpd->power_notifiers,
> + GENPD_STATE_OFF, NULL, -1, &nr_calls);
> + ret = notifier_to_errno(ret);
> + if (ret)
> + goto busy;
> +
> /* Don't power off, if a child domain is waiting to power on. */
> - if (atomic_read(&genpd->sd_count) > 0)
> - return -EBUSY;
> + if (atomic_read(&genpd->sd_count) > 0) {
> + ret = -EBUSY;
> + goto busy;
> + }
>
> ret = _genpd_power_off(genpd, true);
> if (ret)
> - return ret;
> + goto busy;
>
> genpd->status = GENPD_STATE_OFF;
> genpd_update_accounting(genpd);
> @@ -564,6 +573,12 @@ static int genpd_power_off(struct
> generic_pm_domain *genpd, bool one_dev_on,
> }
>
> return 0;
> +busy:
> + if (nr_calls)
> + __raw_notifier_call_chain(&genpd->power_notifiers,
> + GENPD_STATE_ON, NULL,
> + nr_calls - 1, NULL);
> + return ret;
> }
>
> /**
> @@ -609,6 +624,9 @@ static int genpd_power_on(struct
> generic_pm_domain *genpd, unsigned int depth)
> genpd->status = GENPD_STATE_ON;
> genpd_update_accounting(genpd);
>
> + /* Inform consumers that we have powered on. */
> + raw_notifier_call_chain(&genpd->power_notifiers, GENPD_STATE_ON,
> +NULL);
> +
> return 0;
>
> err:
> @@ -938,6 +956,7 @@ static void genpd_sync_power_off(struct
> generic_pm_domain *genpd, bool use_lock,
> unsigned int depth)
> {
> struct gpd_link *link;
> + int err, nr_calls = 0;
>
> if (!genpd_status_on(genpd) || genpd_is_always_on(genpd))
> return;
> @@ -948,8 +967,15 @@ static void genpd_sync_power_off(struct
> generic_pm_domain *genpd, bool use_lock,
>
> /* Choose the deepest state when suspending */
> genpd->state_idx = genpd->state_count - 1;
> +
> + /* Notify consumers that we are about to power off. */
> + err = __raw_notifier_call_chain(&genpd->power_notifiers,
> + GENPD_STATE_OFF, NULL, -1, &nr_calls);
> + if (notifier_to_errno(err))
> + goto err;
> +
> if (_genpd_power_off(genpd, false))
> - return;
> + goto err;
>
> genpd->status = GENPD_STATE_OFF;
>
> @@ -964,6 +990,13 @@ static void genpd_sync_power_off(struct
> generic_pm_domain *genpd, bool use_lock,
> if (use_lock)
> genpd_unlock(link->parent);
> }
> +
> + return;
> +err:
> + if (nr_calls)
> + __raw_notifier_call_chain(&genpd->power_notifiers,
> + GENPD_STATE_ON, NULL,
> + nr_calls - 1, NULL);
> }
>
> /**
> @@ -998,6 +1031,9 @@ static void genpd_sync_power_on(struct
> generic_pm_domain *genpd, bool use_lock,
>
> _genpd_power_on(genpd, false);
> genpd->status = GENPD_STATE_ON;
> +
> + /* Inform consumers that we have powered on. */
> + raw_notifier_call_chain(&genpd->power_notifiers, GENPD_STATE_ON,
> +NULL);
> }
>
> /**
> @@ -1592,6 +1628,101 @@ int pm_genpd_remove_device(struct device
> *dev) } EXPORT_SYMBOL_GPL(pm_genpd_remove_device);
>
> +/**
> + * dev_pm_genpd_add_notifier - Add a genpd power on/off notifier for
> +@dev
> + *
> + * @dev: Device that should be associated with the notifier
> + * @nb: The notifier block to register
> + *
> + * Users may call this function to add a genpd power on/off notifier
> +for an
> + * attached @dev. Only one notifier per device is allowed. The notifier
> +is
> + * sent when genpd is powering on/off the PM domain.
> + *
> + * It is assumed that the user guarantee that the genpd wouldn't be
> +detached
> + * while this routine is getting called.
> + *
> + * Returns 0 on success and negative error values on failures.
> + */
> +int dev_pm_genpd_add_notifier(struct device *dev, struct notifier_block
> +*nb) {
> + struct generic_pm_domain *genpd;
> + struct generic_pm_domain_data *gpd_data;
> + int ret;
> +
> + genpd = dev_to_genpd_safe(dev);
> + if (!genpd)
> + return -ENODEV;
> +
> + if (WARN_ON(!dev->power.subsys_data ||
> + !dev->power.subsys_data->domain_data))
> + return -EINVAL;
> +
> + gpd_data = to_gpd_data(dev->power.subsys_data->domain_data);
> + if (gpd_data->power_nb)
> + return -EEXIST;
> +
> + genpd_lock(genpd);
> + ret = raw_notifier_chain_register(&genpd->power_notifiers, nb);
> + genpd_unlock(genpd);
> +
> + if (ret) {
> + dev_warn(dev, "failed to add notifier for PM domain %s\n",
> + genpd->name);
> + return ret;
> + }
> +
> + gpd_data->power_nb = nb;
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_genpd_add_notifier);
> +
> +/**
> + * dev_pm_genpd_remove_notifier - Remove a genpd power on/off notifier
> +for @dev
> + *
> + * @dev: Device that is associated with the notifier
> + *
> + * Users may call this function to remove a genpd power on/off notifier
> +for an
> + * attached @dev.
> + *
> + * It is assumed that the user guarantee that the genpd wouldn't be
> +detached
> + * while this routine is getting called.
> + *
> + * Returns 0 on success and negative error values on failures.
> + */
> +int dev_pm_genpd_remove_notifier(struct device *dev) {
> + struct generic_pm_domain *genpd;
> + struct generic_pm_domain_data *gpd_data;
> + int ret;
> +
> + genpd = dev_to_genpd_safe(dev);
> + if (!genpd)
> + return -ENODEV;
> +
> + if (WARN_ON(!dev->power.subsys_data ||
> + !dev->power.subsys_data->domain_data))
> + return -EINVAL;
> +
> + gpd_data = to_gpd_data(dev->power.subsys_data->domain_data);
> + if (!gpd_data->power_nb)
> + return -ENODEV;
> +
> + genpd_lock(genpd);
> + ret = raw_notifier_chain_unregister(&genpd->power_notifiers,
> + gpd_data->power_nb);
> + genpd_unlock(genpd);
> +
> + if (ret) {
> + dev_warn(dev, "failed to remove notifier for PM domain %s\n",
> + genpd->name);
> + return ret;
> + }
> +
> + gpd_data->power_nb = NULL;
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_genpd_remove_notifier);
> +
> static int genpd_add_subdomain(struct generic_pm_domain *genpd,
> struct generic_pm_domain *subdomain) { @@
> -1762,6 +1893,7 @@ int pm_genpd_init(struct generic_pm_domain *genpd,
> INIT_LIST_HEAD(&genpd->parent_links);
> INIT_LIST_HEAD(&genpd->child_links);
> INIT_LIST_HEAD(&genpd->dev_list);
> + RAW_INIT_NOTIFIER_HEAD(&genpd->power_notifiers);
> genpd_lock_init(genpd);
> genpd->gov = gov;
> INIT_WORK(&genpd->power_off_work, genpd_power_off_work_fn); diff
> --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index
> 66f3c5d64d81..3b2b561ce846 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -112,6 +112,7 @@ struct generic_pm_domain {
> cpumask_var_t cpus; /* A cpumask of the attached CPUs */
> int (*power_off)(struct generic_pm_domain *domain);
> int (*power_on)(struct generic_pm_domain *domain);
> + struct raw_notifier_head power_notifiers; /* Power on/off notifiers */
> struct opp_table *opp_table; /* OPP table of the genpd */
> unsigned int (*opp_to_performance_state)(struct generic_pm_domain
> *genpd,
> struct dev_pm_opp *opp);
> @@ -178,6 +179,7 @@ struct generic_pm_domain_data {
> struct pm_domain_data base;
> struct gpd_timing_data td;
> struct notifier_block nb;
> + struct notifier_block *power_nb;
> int cpu;
> unsigned int performance_state;
> void *data;
> @@ -204,6 +206,8 @@ int pm_genpd_init(struct generic_pm_domain
> *genpd,
> struct dev_power_governor *gov, bool is_off); int
> pm_genpd_remove(struct generic_pm_domain *genpd); int
> dev_pm_genpd_set_performance_state(struct device *dev, unsigned int
> state);
> +int dev_pm_genpd_add_notifier(struct device *dev, struct notifier_block
> +*nb); int dev_pm_genpd_remove_notifier(struct device *dev);
>
> extern struct dev_power_governor simple_qos_governor; extern struct
> dev_power_governor pm_domain_always_on_gov; @@ -251,6 +255,17 @@
> static inline int dev_pm_genpd_set_performance_state(struct device *dev,
> return -ENOTSUPP;
> }
>
> +static inline int dev_pm_genpd_add_notifier(struct device *dev,
> + struct notifier_block *nb)
> +{
> + return -ENOTSUPP;
> +}
> +
> +static inline int dev_pm_genpd_remove_notifier(struct device *dev) {
> + return -ENOTSUPP;
> +}
> +
> #define simple_qos_governor (*(struct dev_power_governor
> *)(NULL))
> #define pm_domain_always_on_gov (*(struct dev_power_governor
> *)(NULL))
> #endif
> --
> 2.25.1

\
 
 \ /
  Last update: 2020-09-25 08:08    [W:0.098 / U:2.328 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site