From: Ulf Hansson <ulf.hansson@linaro.org> To: Lina Iyer <lina.iyer@linaro.org> Cc: Kevin Hilman <khilman@kernel.org>, "Rafael J. Wysocki" <rjw@rjwysocki.net>, "linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, Andy Gross <andy.gross@linaro.org>, Stephen Boyd <sboyd@codeaurora.org>, "linux-arm-msm@vger.kernel.org" <linux-arm-msm@vger.kernel.org>, Brendan Jackman <brendan.jackman@arm.com>, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>, Sudeep Holla <sudeep.holla@arm.com>, Juri Lelli <Juri.Lelli@arm.com>, Marc Titinger <mtitinger+renesas@baylibre.com> Subject: Re: [PATCH v2 3/8] PM / Domains: Allow domain power states to be read from DT Date: Mon, 10 Oct 2016 12:01:44 +0200 [thread overview] Message-ID: <CAPDyKFo62XuwsoqqPyFa7FYMutd2ng-xt9f7aFh_2T6ab=vAZg@mail.gmail.com> (raw) In-Reply-To: <1475879821-8035-4-git-send-email-lina.iyer@linaro.org> On 8 October 2016 at 00:36, Lina Iyer <lina.iyer@linaro.org> wrote: > This patch allows domains to define idle states in the DT. SoC's can > define domain idle states in DT using the "domain-idle-states" property > of the domain provider. Add API to read the idle states from DT that can > be set in the genpd object. > > This patch is based on the original patch by Marc Titinger. > > Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > Signed-off-by: Lina Iyer <lina.iyer@linaro.org> > --- > drivers/base/power/domain.c | 95 +++++++++++++++++++++++++++++++++++++++++++++ > include/linux/pm_domain.h | 8 ++++ > 2 files changed, 103 insertions(+) > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index 4e87170..4208b67 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -1917,6 +1917,101 @@ out: > return ret ? -EPROBE_DEFER : 0; > } > EXPORT_SYMBOL_GPL(genpd_dev_pm_attach); > + > +static const struct of_device_id idle_state_match[] = { > + { .compatible = "arm,idle-state", }, > + { } > +}; > + > +static int genpd_parse_state(struct genpd_power_state *genpd_state, > + struct device_node *state_node) > +{ > + int err; > + u32 residency; > + u32 entry_latency, exit_latency; > + const struct of_device_id *match_id; > + > + match_id = of_match_node(idle_state_match, state_node); > + if (!match_id) > + return -EINVAL; > + > + err = of_property_read_u32(state_node, "entry-latency-us", > + &entry_latency); > + if (err) { > + pr_debug(" * %s missing entry-latency-us property\n", > + state_node->full_name); > + return -EINVAL; > + } > + > + err = of_property_read_u32(state_node, "exit-latency-us", > + &exit_latency); > + if (err) { > + pr_debug(" * %s missing exit-latency-us property\n", > + state_node->full_name); > + return -EINVAL; > + } > + > + err = of_property_read_u32(state_node, "min-residency-us", &residency); > + if (!err) > + genpd_state->residency_ns = 1000 * residency; > + > + genpd_state->power_on_latency_ns = 1000 * exit_latency; > + genpd_state->power_off_latency_ns = 1000 * entry_latency; > + > + return 0; > +} > + > +/** > + * of_genpd_parse_idle_states: Return array of idle states for the genpd. > + * > + * @dn: The genpd device node > + * @states: The pointer to which the state array will be saved. > + * @n: The count of elements in the array returned from this function. > + * > + * Returns the device states parsed from the OF node. The memory for the states > + * is allocated by this function and is the responsibility of the caller to > + * free the memory after use. > + */ > +int of_genpd_parse_idle_states(struct device_node *dn, > + struct genpd_power_state **states, int *n) Instead of taking **states as a parameter, let's instead return it as a pointer for the allocated struct. In case of failures, let's return ERR_PTR(). > +{ > + struct genpd_power_state *st; > + struct device_node *np; > + int i = 0; > + int err, ret; > + int count; > + struct of_phandle_iterator it; > + > + count = of_count_phandle_with_args(dn, "domain-idle-states", NULL); If count is zero or an error, we should return an error code (ERR_PTR()). Right? > + > + st = kcalloc(count, sizeof(*st), GFP_KERNEL); > + if (!st) > + return -ENOMEM; > + > + /* Loop over the phandles until all the requested entry is found */ > + of_for_each_phandle(&it, err, dn, "domain-idle-states", NULL, 0) { > + np = of_node_get(it.node); I don't think you need to increment the usage count for the device node as that is already managed by of_for_each_phandle(). It's only in the error case below, when it's needed. > + ret = genpd_parse_state(&st[i++], np); > + if (ret) { > + pr_err > + ("Parsing idle state node %s failed with err %d\n", > + np->full_name, ret); > + of_node_put(np); > + goto fail; The goto seems unnecessary. Why not deal with all error handling here and return the error code? > + } > + of_node_put(np); According the comment above, you should be able to remove this. > + } > + > + *n = count; > + *states = st; > + > + return 0; > +fail: > + kfree(st); > + return ret; > +} > +EXPORT_SYMBOL(of_genpd_parse_idle_states); Please use EXPORT_SYMBOL_GPL() instead. > + > #endif /* CONFIG_PM_GENERIC_DOMAINS_OF */ > > > diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h > index f4492eb..b489496 100644 > --- a/include/linux/pm_domain.h > +++ b/include/linux/pm_domain.h > @@ -205,6 +205,8 @@ extern int of_genpd_add_device(struct of_phandle_args *args, > extern int of_genpd_add_subdomain(struct of_phandle_args *parent, > struct of_phandle_args *new_subdomain); > extern struct generic_pm_domain *of_genpd_remove_last(struct device_node *np); > +extern int of_genpd_parse_idle_states(struct device_node *dn, > + struct genpd_power_state **states, int *n); > > int genpd_dev_pm_attach(struct device *dev); > #else /* !CONFIG_PM_GENERIC_DOMAINS_OF */ > @@ -234,6 +236,12 @@ static inline int of_genpd_add_subdomain(struct of_phandle_args *parent, > return -ENODEV; > } > > +static inline int of_genpd_parse_idle_states(struct device_node *dn, > + struct genpd_power_state **states, int *n) > +{ > + return -ENODEV; > +} > + > static inline int genpd_dev_pm_attach(struct device *dev) > { > return -ENODEV; > -- > 2.7.4 > Kind regards Uffe
WARNING: multiple messages have this Message-ID (diff)
From: ulf.hansson@linaro.org (Ulf Hansson) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v2 3/8] PM / Domains: Allow domain power states to be read from DT Date: Mon, 10 Oct 2016 12:01:44 +0200 [thread overview] Message-ID: <CAPDyKFo62XuwsoqqPyFa7FYMutd2ng-xt9f7aFh_2T6ab=vAZg@mail.gmail.com> (raw) In-Reply-To: <1475879821-8035-4-git-send-email-lina.iyer@linaro.org> On 8 October 2016 at 00:36, Lina Iyer <lina.iyer@linaro.org> wrote: > This patch allows domains to define idle states in the DT. SoC's can > define domain idle states in DT using the "domain-idle-states" property > of the domain provider. Add API to read the idle states from DT that can > be set in the genpd object. > > This patch is based on the original patch by Marc Titinger. > > Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > Signed-off-by: Lina Iyer <lina.iyer@linaro.org> > --- > drivers/base/power/domain.c | 95 +++++++++++++++++++++++++++++++++++++++++++++ > include/linux/pm_domain.h | 8 ++++ > 2 files changed, 103 insertions(+) > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index 4e87170..4208b67 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -1917,6 +1917,101 @@ out: > return ret ? -EPROBE_DEFER : 0; > } > EXPORT_SYMBOL_GPL(genpd_dev_pm_attach); > + > +static const struct of_device_id idle_state_match[] = { > + { .compatible = "arm,idle-state", }, > + { } > +}; > + > +static int genpd_parse_state(struct genpd_power_state *genpd_state, > + struct device_node *state_node) > +{ > + int err; > + u32 residency; > + u32 entry_latency, exit_latency; > + const struct of_device_id *match_id; > + > + match_id = of_match_node(idle_state_match, state_node); > + if (!match_id) > + return -EINVAL; > + > + err = of_property_read_u32(state_node, "entry-latency-us", > + &entry_latency); > + if (err) { > + pr_debug(" * %s missing entry-latency-us property\n", > + state_node->full_name); > + return -EINVAL; > + } > + > + err = of_property_read_u32(state_node, "exit-latency-us", > + &exit_latency); > + if (err) { > + pr_debug(" * %s missing exit-latency-us property\n", > + state_node->full_name); > + return -EINVAL; > + } > + > + err = of_property_read_u32(state_node, "min-residency-us", &residency); > + if (!err) > + genpd_state->residency_ns = 1000 * residency; > + > + genpd_state->power_on_latency_ns = 1000 * exit_latency; > + genpd_state->power_off_latency_ns = 1000 * entry_latency; > + > + return 0; > +} > + > +/** > + * of_genpd_parse_idle_states: Return array of idle states for the genpd. > + * > + * @dn: The genpd device node > + * @states: The pointer to which the state array will be saved. > + * @n: The count of elements in the array returned from this function. > + * > + * Returns the device states parsed from the OF node. The memory for the states > + * is allocated by this function and is the responsibility of the caller to > + * free the memory after use. > + */ > +int of_genpd_parse_idle_states(struct device_node *dn, > + struct genpd_power_state **states, int *n) Instead of taking **states as a parameter, let's instead return it as a pointer for the allocated struct. In case of failures, let's return ERR_PTR(). > +{ > + struct genpd_power_state *st; > + struct device_node *np; > + int i = 0; > + int err, ret; > + int count; > + struct of_phandle_iterator it; > + > + count = of_count_phandle_with_args(dn, "domain-idle-states", NULL); If count is zero or an error, we should return an error code (ERR_PTR()). Right? > + > + st = kcalloc(count, sizeof(*st), GFP_KERNEL); > + if (!st) > + return -ENOMEM; > + > + /* Loop over the phandles until all the requested entry is found */ > + of_for_each_phandle(&it, err, dn, "domain-idle-states", NULL, 0) { > + np = of_node_get(it.node); I don't think you need to increment the usage count for the device node as that is already managed by of_for_each_phandle(). It's only in the error case below, when it's needed. > + ret = genpd_parse_state(&st[i++], np); > + if (ret) { > + pr_err > + ("Parsing idle state node %s failed with err %d\n", > + np->full_name, ret); > + of_node_put(np); > + goto fail; The goto seems unnecessary. Why not deal with all error handling here and return the error code? > + } > + of_node_put(np); According the comment above, you should be able to remove this. > + } > + > + *n = count; > + *states = st; > + > + return 0; > +fail: > + kfree(st); > + return ret; > +} > +EXPORT_SYMBOL(of_genpd_parse_idle_states); Please use EXPORT_SYMBOL_GPL() instead. > + > #endif /* CONFIG_PM_GENERIC_DOMAINS_OF */ > > > diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h > index f4492eb..b489496 100644 > --- a/include/linux/pm_domain.h > +++ b/include/linux/pm_domain.h > @@ -205,6 +205,8 @@ extern int of_genpd_add_device(struct of_phandle_args *args, > extern int of_genpd_add_subdomain(struct of_phandle_args *parent, > struct of_phandle_args *new_subdomain); > extern struct generic_pm_domain *of_genpd_remove_last(struct device_node *np); > +extern int of_genpd_parse_idle_states(struct device_node *dn, > + struct genpd_power_state **states, int *n); > > int genpd_dev_pm_attach(struct device *dev); > #else /* !CONFIG_PM_GENERIC_DOMAINS_OF */ > @@ -234,6 +236,12 @@ static inline int of_genpd_add_subdomain(struct of_phandle_args *parent, > return -ENODEV; > } > > +static inline int of_genpd_parse_idle_states(struct device_node *dn, > + struct genpd_power_state **states, int *n) > +{ > + return -ENODEV; > +} > + > static inline int genpd_dev_pm_attach(struct device *dev) > { > return -ENODEV; > -- > 2.7.4 > Kind regards Uffe
next prev parent reply other threads:[~2016-10-10 10:01 UTC|newest] Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-10-07 22:36 [PATCH v2 0/8] PM / Domains: DT support for domain idle states & atomic PM domains Lina Iyer 2016-10-07 22:36 ` Lina Iyer 2016-10-07 22:36 ` [PATCH v2 1/8] PM / Domains: Make genpd state allocation dynamic Lina Iyer 2016-10-07 22:36 ` Lina Iyer 2016-10-10 8:40 ` Ulf Hansson 2016-10-10 8:40 ` Ulf Hansson 2016-10-10 15:05 ` Lina Iyer 2016-10-10 15:05 ` Lina Iyer 2016-10-07 22:36 ` [PATCH v2 2/8] PM / Domain: Add residency property to genpd states Lina Iyer 2016-10-07 22:36 ` Lina Iyer 2016-10-07 22:36 ` [PATCH v2 3/8] PM / Domains: Allow domain power states to be read from DT Lina Iyer 2016-10-07 22:36 ` Lina Iyer 2016-10-10 10:01 ` Ulf Hansson [this message] 2016-10-10 10:01 ` Ulf Hansson 2016-10-10 15:03 ` Lina Iyer 2016-10-10 15:03 ` Lina Iyer 2016-10-10 21:24 ` Ulf Hansson 2016-10-10 21:24 ` Ulf Hansson 2016-10-07 22:36 ` [PATCH v2 4/8] PM / Domains: Save the fwnode in genpd_power_state Lina Iyer 2016-10-07 22:36 ` Lina Iyer 2016-10-10 10:03 ` Ulf Hansson 2016-10-10 10:03 ` Ulf Hansson 2016-10-07 22:36 ` [PATCH v2 5/8] dt/bindings: Update binding for PM domain idle states Lina Iyer 2016-10-07 22:36 ` Lina Iyer 2016-10-10 15:45 ` Sudeep Holla 2016-10-10 15:45 ` Sudeep Holla 2016-10-10 16:43 ` Lina Iyer 2016-10-10 16:43 ` Lina Iyer [not found] ` <20161010164342.GC44885-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2016-10-10 17:13 ` Sudeep Holla 2016-10-10 17:13 ` Sudeep Holla 2016-10-10 22:13 ` Lina Iyer 2016-10-10 22:13 ` Lina Iyer 2016-10-11 11:29 ` Sudeep Holla 2016-10-11 11:29 ` Sudeep Holla 2016-10-10 17:19 ` Sudeep Holla 2016-10-10 17:19 ` Sudeep Holla 2016-10-07 22:36 ` [PATCH v2 6/8] PM / Domains: Abstract genpd locking Lina Iyer 2016-10-07 22:36 ` Lina Iyer 2016-10-07 22:37 ` [PATCH v2 7/8] PM / Domains: Support IRQ safe PM domains Lina Iyer 2016-10-07 22:37 ` Lina Iyer 2016-10-10 11:04 ` Ulf Hansson 2016-10-10 11:04 ` Ulf Hansson 2016-10-07 22:37 ` [PATCH v2 8/8] PM / doc: Update device documentation for devices in " Lina Iyer 2016-10-07 22:37 ` Lina Iyer 2016-10-10 11:06 ` Ulf Hansson 2016-10-10 11:06 ` Ulf Hansson
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAPDyKFo62XuwsoqqPyFa7FYMutd2ng-xt9f7aFh_2T6ab=vAZg@mail.gmail.com' \ --to=ulf.hansson@linaro.org \ --cc=Juri.Lelli@arm.com \ --cc=andy.gross@linaro.org \ --cc=brendan.jackman@arm.com \ --cc=khilman@kernel.org \ --cc=lina.iyer@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=lorenzo.pieralisi@arm.com \ --cc=mtitinger+renesas@baylibre.com \ --cc=rjw@rjwysocki.net \ --cc=sboyd@codeaurora.org \ --cc=sudeep.holla@arm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.