Messages in this thread | | | From | Kevin Hilman <> | Subject | Re: [PATCH v2 01/11] base: power: Add generic OF-based power domain look-up | Date | Fri, 14 Mar 2014 16:07:05 -0700 |
| |
Tomasz Figa <tomasz.figa@gmail.com> writes:
> This patch introduces generic code to perform power domain look-up using > device tree and automatically bind devices to their power domains. > Generic device tree binding is introduced to specify power domains of > devices in their device tree nodes. > > Backwards compatibility with legacy Samsung-specific power domain > bindings is provided, but for now the new code is not compiled when > CONFIG_ARCH_EXYNOS is selected to avoid collision with legacy code. This > will change as soon as Exynos power domain code gets converted to use > the generic framework in further patch. > > Signed-off-by: Tomasz Figa <tomasz.figa@gmail.com>
Reviewed-by: Kevin Hilman <khilman@linaro.org>
The approach and binding both look good to me, other than a few minor nits on comments and question on the locking below...
[...]
> @@ -2177,3 +2181,297 @@ void pm_genpd_init(struct generic_pm_domain *genpd, > list_add(&genpd->gpd_list_node, &gpd_list); > mutex_unlock(&gpd_list_lock); > } > + > +#ifdef CONFIG_PM_GENERIC_DOMAINS_OF > +/* > + * DEVICE TREE BASED POWER DOMAIN PROVIDERS
why all caps?
[...]
> +/* See of_genpd_get_from_provider(). */ > +static struct generic_pm_domain *__of_genpd_get_from_provider( > + struct of_phandle_args *genpdspec) > +{ > + struct of_genpd_provider *provider; > + struct generic_pm_domain *genpd = ERR_PTR(-EPROBE_DEFER); > + > + /* Check if we have such a provider in our array */
I think you want to take the mutex here...
> + list_for_each_entry(provider, &of_genpd_providers, link) { > + if (provider->node == genpdspec->np) > + genpd = provider->xlate(genpdspec, provider->data); > + if (!IS_ERR(genpd)) > + break; > + }
...and release it here, right?
[...]
> +/* > + * DEVICE<->DOMAIN BINDING USING DEVICE TREE LOOK-UP
hmm, more yelling?
Otherwise, looks good to me.
Kevin
| |