[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <22330d85-1d5c-48de-b006-9176f275328e@quicinc.com>
Date: Fri, 29 Dec 2023 12:21:15 -0800
From: Nikunj Kela <quic_nkela@...cinc.com>
To: Ulf Hansson <ulf.hansson@...aro.org>,
"Rafael J . Wysocki"
<rafael@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"Viresh
Kumar" <viresh.kumar@...aro.org>, <linux-pm@...r.kernel.org>
CC: Sudeep Holla <sudeep.holla@....com>, Kevin Hilman <khilman@...nel.org>,
Konrad Dybcio <konrad.dybcio@...aro.org>,
Bjorn Andersson
<andersson@...nel.org>,
Nikunj Kela <nkela@...cinc.com>,
Prasad Sodagudi
<psodagud@...cinc.com>,
Stephan Gerhold <stephan@...hold.net>,
Ben Horgan
<Ben.Horgan@....com>, <linux-kernel@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>,
<linux-remoteproc@...r.kernel.org>, <linux-media@...r.kernel.org>
Subject: Re: [PATCH 1/5] PM: domains: Add helper functions to attach/detach
multiple PM domains
On 12/28/2023 3:41 AM, Ulf Hansson wrote:
> Attaching/detaching of a device to multiple PM domains has started to
> become a common operation for many drivers, typically during ->probe() and
> ->remove(). In most cases, this has lead to lots of boilerplate code in the
> drivers.
>
> To fixup up the situation, let's introduce a pair of helper functions,
> dev_pm_domain_attach|detach_list(), that driver can use instead of the
> open-coding. Note that, it seems reasonable to limit the support for these
> helpers to DT based platforms, at it's the only valid use case for now.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@...aro.org>
> ---
> drivers/base/power/common.c | 133 ++++++++++++++++++++++++++++++++++++
> include/linux/pm_domain.h | 38 +++++++++++
> 2 files changed, 171 insertions(+)
>
> diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c
> index 44ec20918a4d..1ef51889fc6f 100644
> --- a/drivers/base/power/common.c
> +++ b/drivers/base/power/common.c
> @@ -167,6 +167,114 @@ struct device *dev_pm_domain_attach_by_name(struct device *dev,
> }
> EXPORT_SYMBOL_GPL(dev_pm_domain_attach_by_name);
>
> +/**
> + * dev_pm_domain_attach_list - Associate a device with its PM domains.
> + * @dev: The device used to lookup the PM domains for.
> + * @data: The data used for attaching to the PM domains.
> + * @list: An out-parameter with an allocated list of attached PM domains.
> + *
> + * This function helps to attach a device to its multiple PM domains. The
> + * caller, which is typically a driver's probe function, may provide a list of
> + * names for the PM domains that we should try to attach the device to, but it
> + * may also provide an empty list, in case the attach should be done for all of
> + * the available PM domains.
> + *
> + * Callers must ensure proper synchronization of this function with power
> + * management callbacks.
> + *
> + * Returns the number of attached PM domains or a negative error code in case of
> + * a failure. Note that, to detach the list of PM domains, the driver shall call
> + * dev_pm_domain_detach_list(), typically during the remove phase.
> + */
> +int dev_pm_domain_attach_list(struct device *dev,
> + const struct dev_pm_domain_attach_data *data,
> + struct dev_pm_domain_list **list)
> +{
> + struct device_node *np = dev->of_node;
> + struct dev_pm_domain_list *pds;
> + struct device *pd_dev = NULL;
> + int ret, i, num_pds = 0;
> + bool by_id = true;
> + u32 link_flags = data && data->pd_flags & PD_FLAG_NO_DEV_LINK ? 0 :
> + DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME;
> +
> + if (dev->pm_domain)
> + return -EEXIST;
> +
> + /* For now this is limited to OF based platforms. */
> + if (!np)
> + return 0;
> +
> + if (data && data->pd_names) {
> + num_pds = data->num_pd_names;
> + by_id = false;
> + } else {
> + num_pds = of_count_phandle_with_args(np, "power-domains",
> + "#power-domain-cells");
> + }
> +
> + if (num_pds <= 0)
> + return 0;
> +
> + pds = devm_kzalloc(dev, sizeof(*pds), GFP_KERNEL);
> + if (!pds)
> + return -ENOMEM;
> +
> + pds->pd_devs = devm_kcalloc(dev, num_pds, sizeof(*pds->pd_devs),
> + GFP_KERNEL);
> + if (!pds->pd_devs)
> + return -ENOMEM;
> +
> + pds->pd_links = devm_kcalloc(dev, num_pds, sizeof(*pds->pd_links),
> + GFP_KERNEL);
> + if (!pds->pd_links)
> + return -ENOMEM;
> +
> + if (link_flags && data->pd_flags & PD_FLAG_DEV_LINK_ON)
Since data is optional, this check results in crash if data is NULL. Thanks
> + link_flags |= DL_FLAG_RPM_ACTIVE;
> +
> + for (i = 0; i < num_pds; i++) {
> + if (by_id)
> + pd_dev = dev_pm_domain_attach_by_id(dev, i);
> + else
> + pd_dev = dev_pm_domain_attach_by_name(dev,
> + data->pd_names[i]);
> + if (IS_ERR_OR_NULL(pd_dev)) {
> + ret = pd_dev ? PTR_ERR(pd_dev) : -ENODEV;
> + goto err_attach;
> + }
> +
> + if (link_flags) {
> + struct device_link *link;
> +
> + link = device_link_add(dev, pd_dev, link_flags);
> + if (!link) {
> + ret = -ENODEV;
> + goto err_link;
> + }
> +
> + pds->pd_links[i] = link;
> + }
> +
> + pds->pd_devs[i] = pd_dev;
> + }
> +
> + pds->num_pds = num_pds;
> + *list = pds;
> + return num_pds;
> +
> +err_link:
> + dev_pm_domain_detach(pd_dev, true);
> +err_attach:
> + while (--i >= 0) {
> + if (pds->pd_links[i])
> + device_link_del(pds->pd_links[i]);
> + dev_pm_domain_detach(pds->pd_devs[i], true);
> + }
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_domain_attach_list);
> +
> /**
> * dev_pm_domain_detach - Detach a device from its PM domain.
> * @dev: Device to detach.
> @@ -187,6 +295,31 @@ void dev_pm_domain_detach(struct device *dev, bool power_off)
> }
> EXPORT_SYMBOL_GPL(dev_pm_domain_detach);
>
> +/**
> + * dev_pm_domain_detach_list - Detach a list of PM domains.
> + * @list: The list of PM domains to detach.
> + *
> + * This function reverse the actions from dev_pm_domain_attach_list().
> + * Typically it should be invoked during the remove phase from drivers.
> + *
> + * Callers must ensure proper synchronization of this function with power
> + * management callbacks.
> + */
> +void dev_pm_domain_detach_list(struct dev_pm_domain_list *list)
> +{
> + int i;
> +
> + if (!list)
> + return;
> +
> + for (i = 0; i < list->num_pds; i++) {
> + if (list->pd_links[i])
> + device_link_del(list->pd_links[i]);
> + dev_pm_domain_detach(list->pd_devs[i], true);
> + }
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_domain_detach_list);
> +
> /**
> * dev_pm_domain_start - Start the device through its PM domain.
> * @dev: Device to start.
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 34663d0d5c55..6b71fb69c349 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -19,6 +19,33 @@
> #include <linux/cpumask.h>
> #include <linux/time64.h>
>
> +/*
> + * Flags to control the behaviour when attaching a device to its PM domains.
> + *
> + * PD_FLAG_NO_DEV_LINK: As the default behaviour creates a device-link
> + * for every PM domain that gets attached, this
> + * flag can be used to skip that.
> + *
> + * PD_FLAG_DEV_LINK_ON: Add the DL_FLAG_RPM_ACTIVE to power-on the
> + * supplier and its PM domain when creating the
> + * device-links.
> + *
> + */
> +#define PD_FLAG_NO_DEV_LINK BIT(0)
> +#define PD_FLAG_DEV_LINK_ON BIT(1)
> +
> +struct dev_pm_domain_attach_data {
> + const char * const *pd_names;
> + const u32 num_pd_names;
> + const u32 pd_flags;
> +};
> +
> +struct dev_pm_domain_list {
> + struct device **pd_devs;
> + struct device_link **pd_links;
> + u32 num_pds;
> +};
> +
> /*
> * Flags to control the behaviour of a genpd.
> *
> @@ -432,7 +459,11 @@ 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);
> +int dev_pm_domain_attach_list(struct device *dev,
> + const struct dev_pm_domain_attach_data *data,
> + struct dev_pm_domain_list **list);
> void dev_pm_domain_detach(struct device *dev, bool power_off);
> +void dev_pm_domain_detach_list(struct dev_pm_domain_list *list);
> int dev_pm_domain_start(struct device *dev);
> void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd);
> int dev_pm_domain_set_performance_state(struct device *dev, unsigned int state);
> @@ -451,7 +482,14 @@ static inline struct device *dev_pm_domain_attach_by_name(struct device *dev,
> {
> return NULL;
> }
> +static inline int dev_pm_domain_attach_list(struct device *dev,
> + const struct dev_pm_domain_attach_data *data,
> + struct dev_pm_domain_list **list)
> +{
> + return 0;
> +}
> static inline void dev_pm_domain_detach(struct device *dev, bool power_off) {}
> +static inline void dev_pm_domain_detach_list(struct dev_pm_domain_list *list) {}
> static inline int dev_pm_domain_start(struct device *dev)
> {
> return 0;
Powered by blists - more mailing lists