* [RFCv2 0/2] rpi: add support for rpi power domain driver
@ 2015-11-03 22:45 Alexander Aring
[not found] ` <1446590711-18928-1-git-send-email-alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-11-03 22:45 ` [RFCv2 2/2] rpi: add support to enable usb power domain Alexander Aring
0 siblings, 2 replies; 14+ messages in thread
From: Alexander Aring @ 2015-11-03 22:45 UTC (permalink / raw)
To: linux-rpi-kernel
Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, swarren,
lee, eric, linux, f.fainelli, rjui, sbranden, rjw, khilman,
ulf.hansson, len.brown, pavel, gregkh, devicetree,
linux-arm-kernel, linux-pm, bcm-kernel-feedback-list, kernel,
Alexander Aring
Hi,
this patch series contains at first a patch for the power domain subsystem
which offers to uninit generic power domains when init was called before.
The RPi Power-Domains need to be enabled/disabled by interacting with the
RPi firmware which can fail. To cleanup the probing we need to undo the
power domains again which was registered before.
- Alex
Alexander Aring (2):
power: domain: add pm_genpd_uninit
rpi: add support to enable usb power domain
.../bindings/arm/bcm/raspberrypi,bcm2835-power.txt | 25 +++
arch/arm/boot/dts/bcm2835-rpi.dtsi | 11 ++
arch/arm/boot/dts/bcm2835.dtsi | 2 +-
arch/arm/mach-bcm/Kconfig | 10 ++
arch/arm/mach-bcm/Makefile | 1 +
arch/arm/mach-bcm/raspberrypi-power.c | 180 +++++++++++++++++++++
drivers/base/power/domain.c | 15 ++
include/dt-bindings/arm/raspberrypi-power.h | 14 ++
include/linux/pm_domain.h | 4 +
9 files changed, 261 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt
create mode 100644 arch/arm/mach-bcm/raspberrypi-power.c
create mode 100644 include/dt-bindings/arm/raspberrypi-power.h
--
2.6.1
^ permalink raw reply [flat|nested] 14+ messages in thread[parent not found: <1446590711-18928-1-git-send-email-alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* [RFCv2 1/2] power: domain: add pm_genpd_uninit [not found] ` <1446590711-18928-1-git-send-email-alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2015-11-03 22:45 ` Alexander Aring 2015-11-05 9:01 ` Ulf Hansson 0 siblings, 1 reply; 14+ messages in thread From: Alexander Aring @ 2015-11-03 22:45 UTC (permalink / raw) To: linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg, galak-sgV2jX0FEOL9JmXXK+q4OQ, swarren-3lzwWm7+Weoh9ZMKESR00Q, lee-DgEjT+Ai2ygdnm+yROfE0A, eric-WhKQ6XTQaPysTnJN9+BGXg, linux-lFZ/pmaqli7XmaaqVzeoHQ, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, rjui-dY08KVG/lbpWk0Htik3J/w, sbranden-dY08KVG/lbpWk0Htik3J/w, rjw-LthD3rsA81gm4RdzfppkhA, khilman-DgEjT+Ai2ygdnm+yROfE0A, ulf.hansson-QSEj5FYQhm4dnm+yROfE0A, len.brown-ral2JQCrhuEAvxtiuMwx3w, pavel-+ZI9xUNit7I, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-pm-u79uwXL29TY76Z2rM5mHXA, bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w, kernel-bIcnvbaLZ9MEGnE8C9+IrQ, Alexander Aring This patch adds function pm_genpd_uninit for undo a pm_genpd_init. This is useful for multiple power domains while probing. If the probing fails after one pm_genpd_init was called we need to undo all previous registrations of generic pm domains inside the gpd_list list. There is a check on IS_ERR_OR_NULL(genpd) which is useful to check again registered power domains and not registered domains, the driver can use this mechanism to have an array with registered and non-registered power domains, where non-registered power domains are NULL. Cc: Rafael J. Wysocki <rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org> Cc: Kevin Hilman <khilman-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> Cc: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> Cc: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org> Cc: Len Brown <len.brown-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> Signed-off-by: Alexander Aring <alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> --- drivers/base/power/domain.c | 15 +++++++++++++++ include/linux/pm_domain.h | 4 ++++ 2 files changed, 19 insertions(+) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 16550c6..65b9d1a 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -1730,6 +1730,21 @@ void pm_genpd_init(struct generic_pm_domain *genpd, } EXPORT_SYMBOL_GPL(pm_genpd_init); +/** + * pm_genpd_uninit - Uninitialize a generic I/O PM domain object. + * @genpd: PM domain object to initialize. + */ +void pm_genpd_uninit(struct generic_pm_domain *genpd) +{ + if (IS_ERR_OR_NULL(genpd)) + return; + + mutex_lock(&gpd_list_lock); + list_del(&genpd->gpd_list_node); + mutex_unlock(&gpd_list_lock); +} +EXPORT_SYMBOL_GPL(pm_genpd_uninit); + #ifdef CONFIG_PM_GENERIC_DOMAINS_OF /* * Device Tree based PM domain providers. diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index b1cf7e7..45d4f7a 100644 --- a/include/linux/pm_domain.h +++ b/include/linux/pm_domain.h @@ -143,6 +143,7 @@ extern int pm_genpd_detach_cpuidle(struct generic_pm_domain *genpd); extern int pm_genpd_name_detach_cpuidle(const char *name); extern void pm_genpd_init(struct generic_pm_domain *genpd, struct dev_power_governor *gov, bool is_off); +extern void pm_genpd_uninit(struct generic_pm_domain *genpd); extern int pm_genpd_poweron(struct generic_pm_domain *genpd); extern int pm_genpd_name_poweron(const char *domain_name); @@ -212,6 +213,9 @@ static inline void pm_genpd_init(struct generic_pm_domain *genpd, struct dev_power_governor *gov, bool is_off) { } +static inline void pm_genpd_uninit(struct generic_pm_domain *genpd) +{ +} static inline int pm_genpd_poweron(struct generic_pm_domain *genpd) { return -ENOSYS; -- 2.6.1 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [RFCv2 1/2] power: domain: add pm_genpd_uninit 2015-11-03 22:45 ` [RFCv2 1/2] power: domain: add pm_genpd_uninit Alexander Aring @ 2015-11-05 9:01 ` Ulf Hansson 2015-11-05 14:34 ` Alexander Aring 0 siblings, 1 reply; 14+ messages in thread From: Ulf Hansson @ 2015-11-05 9:01 UTC (permalink / raw) To: Alexander Aring Cc: linux-rpi-kernel, Rob Herring, Paweł Moll, Mark Rutland, Ian Campbell, Kumar Gala, Stephen Warren, Lee Jones, Eric Anholt, Russell King - ARM Linux, Florian Fainelli, Ray Jui, Scott Branden, Rafael J. Wysocki, Kevin Hilman, Len Brown, Pavel Machek, Greg Kroah-Hartman, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org On 3 November 2015 at 23:45, Alexander Aring <alex.aring@gmail.com> wrote: > This patch adds function pm_genpd_uninit for undo a pm_genpd_init. This > is useful for multiple power domains while probing. If the probing fails > after one pm_genpd_init was called we need to undo all previous > registrations of generic pm domains inside the gpd_list list. Yes, agree. Although I think it's a bit mote complicated than what you suggest in this simple approach. :-) > > There is a check on IS_ERR_OR_NULL(genpd) which is useful to check again > registered power domains and not registered domains, the driver can use > this mechanism to have an array with registered and non-registered power > domains, where non-registered power domains are NULL. > > Cc: Rafael J. Wysocki <rjw@rjwysocki.net> > Cc: Kevin Hilman <khilman@kernel.org> > Cc: Ulf Hansson <ulf.hansson@linaro.org> > Cc: Pavel Machek <pavel@ucw.cz> > Cc: Len Brown <len.brown@intel.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Alexander Aring <alex.aring@gmail.com> > --- > drivers/base/power/domain.c | 15 +++++++++++++++ > include/linux/pm_domain.h | 4 ++++ > 2 files changed, 19 insertions(+) > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index 16550c6..65b9d1a 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -1730,6 +1730,21 @@ void pm_genpd_init(struct generic_pm_domain *genpd, > } > EXPORT_SYMBOL_GPL(pm_genpd_init); > > +/** > + * pm_genpd_uninit - Uninitialize a generic I/O PM domain object. > + * @genpd: PM domain object to initialize. > + */ > +void pm_genpd_uninit(struct generic_pm_domain *genpd) > +{ > + if (IS_ERR_OR_NULL(genpd)) > + return; > + > + mutex_lock(&gpd_list_lock); > + list_del(&genpd->gpd_list_node); > + mutex_unlock(&gpd_list_lock); This is too fragile. You don't protect from the cases below. 1. The genpd may have devices attached to it. 2. The genpd may have subdomains. To deal with these case, that's when it becomes more complex which I guess is the reason to why nobody really cared until now. Moreover, I think there are some more structures to "uninitialize" besides just unlinking the genpd struct from the gpd list. For example a mutex_destroy() should be done. > +} > +EXPORT_SYMBOL_GPL(pm_genpd_uninit); > + > #ifdef CONFIG_PM_GENERIC_DOMAINS_OF > /* > * Device Tree based PM domain providers. > diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h > index b1cf7e7..45d4f7a 100644 > --- a/include/linux/pm_domain.h > +++ b/include/linux/pm_domain.h > @@ -143,6 +143,7 @@ extern int pm_genpd_detach_cpuidle(struct generic_pm_domain *genpd); > extern int pm_genpd_name_detach_cpuidle(const char *name); > extern void pm_genpd_init(struct generic_pm_domain *genpd, > struct dev_power_governor *gov, bool is_off); > +extern void pm_genpd_uninit(struct generic_pm_domain *genpd); > > extern int pm_genpd_poweron(struct generic_pm_domain *genpd); > extern int pm_genpd_name_poweron(const char *domain_name); > @@ -212,6 +213,9 @@ static inline void pm_genpd_init(struct generic_pm_domain *genpd, > struct dev_power_governor *gov, bool is_off) > { > } > +static inline void pm_genpd_uninit(struct generic_pm_domain *genpd) > +{ > +} > static inline int pm_genpd_poweron(struct generic_pm_domain *genpd) > { > return -ENOSYS; > -- > 2.6.1 > Kind regards Uffe ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFCv2 1/2] power: domain: add pm_genpd_uninit 2015-11-05 9:01 ` Ulf Hansson @ 2015-11-05 14:34 ` Alexander Aring 2015-11-11 18:00 ` Alexander Aring 2015-11-11 20:29 ` Ulf Hansson 0 siblings, 2 replies; 14+ messages in thread From: Alexander Aring @ 2015-11-05 14:34 UTC (permalink / raw) To: Ulf Hansson Cc: linux-rpi-kernel, Rob Herring, Paweł Moll, Mark Rutland, Ian Campbell, Kumar Gala, Stephen Warren, Lee Jones, Eric Anholt, Russell King - ARM Linux, Florian Fainelli, Ray Jui, Scott Branden, Rafael J. Wysocki, Kevin Hilman, Len Brown, Pavel Machek, Greg Kroah-Hartman, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org On Thu, Nov 05, 2015 at 10:01:32AM +0100, Ulf Hansson wrote: > On 3 November 2015 at 23:45, Alexander Aring <alex.aring@gmail.com> wrote: > > This patch adds function pm_genpd_uninit for undo a pm_genpd_init. This > > is useful for multiple power domains while probing. If the probing fails > > after one pm_genpd_init was called we need to undo all previous > > registrations of generic pm domains inside the gpd_list list. > > Yes, agree. Although I think it's a bit mote complicated than what you > suggest in this simple approach. :-) > ok. > > > > There is a check on IS_ERR_OR_NULL(genpd) which is useful to check again > > registered power domains and not registered domains, the driver can use > > this mechanism to have an array with registered and non-registered power > > domains, where non-registered power domains are NULL. > > > > Cc: Rafael J. Wysocki <rjw@rjwysocki.net> > > Cc: Kevin Hilman <khilman@kernel.org> > > Cc: Ulf Hansson <ulf.hansson@linaro.org> > > Cc: Pavel Machek <pavel@ucw.cz> > > Cc: Len Brown <len.brown@intel.com> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Signed-off-by: Alexander Aring <alex.aring@gmail.com> > > --- > > drivers/base/power/domain.c | 15 +++++++++++++++ > > include/linux/pm_domain.h | 4 ++++ > > 2 files changed, 19 insertions(+) > > > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > > index 16550c6..65b9d1a 100644 > > --- a/drivers/base/power/domain.c > > +++ b/drivers/base/power/domain.c > > @@ -1730,6 +1730,21 @@ void pm_genpd_init(struct generic_pm_domain *genpd, > > } > > EXPORT_SYMBOL_GPL(pm_genpd_init); > > > > +/** > > + * pm_genpd_uninit - Uninitialize a generic I/O PM domain object. > > + * @genpd: PM domain object to initialize. > > + */ > > +void pm_genpd_uninit(struct generic_pm_domain *genpd) > > +{ > > + if (IS_ERR_OR_NULL(genpd)) > > + return; > > + > > + mutex_lock(&gpd_list_lock); > > + list_del(&genpd->gpd_list_node); > > + mutex_unlock(&gpd_list_lock); > > This is too fragile. You don't protect from the cases below. > > 1. The genpd may have devices attached to it. > 2. The genpd may have subdomains. > > To deal with these case, that's when it becomes more complex which I > guess is the reason to why nobody really cared until now. > Can we not just undo the things which pm_genpd_init does, then let the driver to deal with all other uninitialize of e.g. "subdomains" which might the driver has initialize before? We know there are no slaves or something else which is empty because pm_genpd_init runs INIT_LIST_HEAD, or not? To make such handling above I would add some: "pm_genpd_uninit_recursive" function, which cleanup everything from a power domain, e.g. subdomains, etc and other lists. What do you suggest to me for e.g. the raspberrypi power domain driver, also simple ignore such error handling? For my current use-case I can remove the failure between pm_genpd_init by simple getting all "power states" (the bool is_off for pm_genpd_init), before calling pm_genpd_init. In this case I don't have any failure between calling pm_genpd_init when another pm_genpd_init was before. Looks like this: 1. get all states from firmware to get parameter "is_off", which can fail. We should get here the states for the power domains which we want to register only. 2. Then call pm_genpd_init, which cannot be fail between another pm_genpd_init. (There are only void functions in the middle, which cannot fail). If I call pm_genpd_init, I use "is_off" variable from the step of "1.". But then I have still the call of "of_genpd_add_provider_onecell" at the end of probing which can fail. So this is also not a valid solution, because I need to undo everything before. > Moreover, I think there are some more structures to "uninitialize" > besides just unlinking the genpd struct from the gpd list. For example > a mutex_destroy() should be done. > yes, of course. - Alex ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFCv2 1/2] power: domain: add pm_genpd_uninit 2015-11-05 14:34 ` Alexander Aring @ 2015-11-11 18:00 ` Alexander Aring 2015-11-11 20:33 ` Ulf Hansson 2015-11-11 20:29 ` Ulf Hansson 1 sibling, 1 reply; 14+ messages in thread From: Alexander Aring @ 2015-11-11 18:00 UTC (permalink / raw) To: Ulf Hansson Cc: linux-rpi-kernel, Rob Herring, Paweł Moll, Mark Rutland, Ian Campbell, Kumar Gala, Stephen Warren, Lee Jones, Eric Anholt, Russell King - ARM Linux, Florian Fainelli, Ray Jui, Scott Branden, Rafael J. Wysocki, Kevin Hilman, Len Brown, Pavel Machek, Greg Kroah-Hartman, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org Hi, On Thu, Nov 05, 2015 at 03:34:45PM +0100, Alexander Aring wrote: > > What do you suggest to me for e.g. the raspberrypi power domain driver, > also simple ignore such error handling? > ping, I also can add some WARN_ON_ONCE, if the list for sub-domains, etc. are not empty. This would then report about wrong use of pm_genpd_uninit. - Alex ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFCv2 1/2] power: domain: add pm_genpd_uninit 2015-11-11 18:00 ` Alexander Aring @ 2015-11-11 20:33 ` Ulf Hansson 2015-11-13 12:56 ` Alexander Aring 0 siblings, 1 reply; 14+ messages in thread From: Ulf Hansson @ 2015-11-11 20:33 UTC (permalink / raw) To: Alexander Aring Cc: linux-rpi-kernel, Rob Herring, Paweł Moll, Mark Rutland, Ian Campbell, Kumar Gala, Stephen Warren, Lee Jones, Eric Anholt, Russell King - ARM Linux, Florian Fainelli, Ray Jui, Scott Branden, Rafael J. Wysocki, Kevin Hilman, Len Brown, Pavel Machek, Greg Kroah-Hartman, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org On 11 November 2015 at 19:00, Alexander Aring <alex.aring@gmail.com> wrote: > Hi, > > On Thu, Nov 05, 2015 at 03:34:45PM +0100, Alexander Aring wrote: >> >> What do you suggest to me for e.g. the raspberrypi power domain driver, >> also simple ignore such error handling? >> > > ping, I also can add some WARN_ON_ONCE, if the list for sub-domains, > etc. are not empty. This would then report about wrong use of > pm_genpd_uninit. > > - Alex Sorry for the delay. I think what you suggest would be an okay solution, at least it will improve the current behaviour. We should verify for sub-domains, attached devices, and if the genpd has an of-provider. That's all I can think of right now, but there may be other things as well. Kind regards Uffe ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFCv2 1/2] power: domain: add pm_genpd_uninit 2015-11-11 20:33 ` Ulf Hansson @ 2015-11-13 12:56 ` Alexander Aring 0 siblings, 0 replies; 14+ messages in thread From: Alexander Aring @ 2015-11-13 12:56 UTC (permalink / raw) To: Ulf Hansson Cc: linux-rpi-kernel, Rob Herring, Paweł Moll, Mark Rutland, Ian Campbell, Kumar Gala, Stephen Warren, Lee Jones, Eric Anholt, Russell King - ARM Linux, Florian Fainelli, Ray Jui, Scott Branden, Rafael J. Wysocki, Kevin Hilman, Len Brown, Pavel Machek, Greg Kroah-Hartman, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org Hi, On Wed, Nov 11, 2015 at 09:33:40PM +0100, Ulf Hansson wrote: > On 11 November 2015 at 19:00, Alexander Aring <alex.aring@gmail.com> wrote: > > Hi, > > > > On Thu, Nov 05, 2015 at 03:34:45PM +0100, Alexander Aring wrote: > >> > >> What do you suggest to me for e.g. the raspberrypi power domain driver, > >> also simple ignore such error handling? > >> > > > > ping, I also can add some WARN_ON_ONCE, if the list for sub-domains, > > etc. are not empty. This would then report about wrong use of > > pm_genpd_uninit. > > > > - Alex > > Sorry for the delay. > > I think what you suggest would be an okay solution, at least it will > improve the current behaviour. > > We should verify for sub-domains, attached devices, and if the genpd > has an of-provider. That's all I can think of right now, but there may > be other things as well. > okay, I added now: WARN_ON_ONCE(!list_empty(genpd->master_links) || !list_empty(genpd->slave_links) || !list_empty(genpd->dev_list)); So far I understand is master/slave something about domains/subdomains, the dev_list fis for atteched devices. But how can I check "if the genpd has an of-provider", the "struct generic_pm_domain" doesn't know a "of-provider". There is a static list "of_genpd_providers", do you want to iterate over all and then doing some matching algorithmn? Or do you want to add something inside "struct generic_pm_domain", so a genpd knows about the "of-provider". I am currently confused about how to check on "of-provider". - Alex ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFCv2 1/2] power: domain: add pm_genpd_uninit 2015-11-05 14:34 ` Alexander Aring 2015-11-11 18:00 ` Alexander Aring @ 2015-11-11 20:29 ` Ulf Hansson 1 sibling, 0 replies; 14+ messages in thread From: Ulf Hansson @ 2015-11-11 20:29 UTC (permalink / raw) To: Alexander Aring Cc: linux-rpi-kernel, Rob Herring, Paweł Moll, Mark Rutland, Ian Campbell, Kumar Gala, Stephen Warren, Lee Jones, Eric Anholt, Russell King - ARM Linux, Florian Fainelli, Ray Jui, Scott Branden, Rafael J. Wysocki, Kevin Hilman, Len Brown, Pavel Machek, Greg Kroah-Hartman, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org [...] >> > >> > +/** >> > + * pm_genpd_uninit - Uninitialize a generic I/O PM domain object. >> > + * @genpd: PM domain object to initialize. >> > + */ >> > +void pm_genpd_uninit(struct generic_pm_domain *genpd) >> > +{ >> > + if (IS_ERR_OR_NULL(genpd)) >> > + return; >> > + >> > + mutex_lock(&gpd_list_lock); >> > + list_del(&genpd->gpd_list_node); >> > + mutex_unlock(&gpd_list_lock); >> >> This is too fragile. You don't protect from the cases below. >> >> 1. The genpd may have devices attached to it. >> 2. The genpd may have subdomains. >> >> To deal with these case, that's when it becomes more complex which I >> guess is the reason to why nobody really cared until now. >> > > Can we not just undo the things which pm_genpd_init does, then let the > driver to deal with all other uninitialize of e.g. "subdomains" which might > the driver has initialize before? We know there are no slaves or something > else which is empty because pm_genpd_init runs INIT_LIST_HEAD, or not? > > To make such handling above I would add some: > > "pm_genpd_uninit_recursive" function, which cleanup everything from a > power domain, e.g. subdomains, etc and other lists. > > What do you suggest to me for e.g. the raspberrypi power domain driver, > also simple ignore such error handling? No, let's give it try so see if can improve the situation. > > > For my current use-case I can remove the failure between pm_genpd_init by > simple getting all "power states" (the bool is_off for pm_genpd_init), > before calling pm_genpd_init. In this case I don't have any failure between > calling pm_genpd_init when another pm_genpd_init was before. > > Looks like this: > > 1. get all states from firmware to get parameter "is_off", which can fail. > We should get here the states for the power domains which we want to > register only. > > 2. Then call pm_genpd_init, which cannot be fail between another > pm_genpd_init. (There are only void functions in the middle, which cannot > fail). If I call pm_genpd_init, I use "is_off" variable from the step > of "1.". > > > But then I have still the call of "of_genpd_add_provider_onecell" at the end > of probing which can fail. So this is also not a valid solution, because > I need to undo everything before. Okay, got it. > >> Moreover, I think there are some more structures to "uninitialize" >> besides just unlinking the genpd struct from the gpd list. For example >> a mutex_destroy() should be done. >> > > yes, of course. > > - Alex Kind regards Uffe ^ permalink raw reply [flat|nested] 14+ messages in thread
* [RFCv2 2/2] rpi: add support to enable usb power domain 2015-11-03 22:45 [RFCv2 0/2] rpi: add support for rpi power domain driver Alexander Aring [not found] ` <1446590711-18928-1-git-send-email-alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2015-11-03 22:45 ` Alexander Aring [not found] ` <1446590711-18928-3-git-send-email-alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2015-11-05 13:35 ` Rob Herring 1 sibling, 2 replies; 14+ messages in thread From: Alexander Aring @ 2015-11-03 22:45 UTC (permalink / raw) To: linux-rpi-kernel Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, swarren, lee, eric, linux, f.fainelli, rjui, sbranden, rjw, khilman, ulf.hansson, len.brown, pavel, gregkh, devicetree, linux-arm-kernel, linux-pm, bcm-kernel-feedback-list, kernel, Alexander Aring This patch adds support for RPi several Power Domains and enable support to enable the USB Power Domain when it's not enabled before. This patch based on Eric Anholt's patch to support Power Domains. He had an issue about -EPROBE_DEFER inside the power domain subsystem, this issue was solved by commit <311fa6a> ("PM / Domains: Return -EPROBE_DEFER if we fail to init or turn-on domain"). It was tested with barebox and the following scripts before booting linux: /env/a_off: # cat /env/a_off #turn off which are enabled by default regulator -n bcm2835_mci0 -s disable regulator -n uart0-pl0110 -s disable /env/a_on: # cat /env/a_on #turn off which are enabled by default regulator -n bcm2835_mci0 -s disable regulator -n uart0-pl0110 -s disable regulator -n bcm2835_mci0 -s enable regulator -n uart0-pl0110 -s enable regulator -n uart0-pl0111 -s enable regulator -n bcm2835_usb -s enable regulator -n bcm2835_i2c0 -s enable regulator -n bcm2835_i2c1 -s enable regulator -n bcm2835_i2c2 -s enable regulator -n bcm2835_spi -s enable regulator -n bcm2835_ccp2tx -s enable regulator -n bcm2835_dsi -s enable /env/b: # cat /env/b sh /env/a_on regulator -n bcm2835_mci0 -s disable regulator -n uart0-pl0110 -s disable regulator -n uart0-pl0111 -s disable regulator -n bcm2835_usb -s disable regulator -n bcm2835_i2c0 -s disable regulator -n bcm2835_i2c1 -s disable regulator -n bcm2835_i2c2 -s disable regulator -n bcm2835_spi -s disable regulator -n bcm2835_ccp2tx -s disable regulator -n bcm2835_dsi -s disable /env/c: # cat /env/c sh ./env/b regulator -n bcm2835_mci0 -s enable regulator -n uart0-pl0110 -s enable regulator -n uart0-pl0111 -s enable regulator -n bcm2835_usb -s enable regulator -n bcm2835_i2c0 -s enable regulator -n bcm2835_i2c1 -s enable regulator -n bcm2835_i2c2 -s enable regulator -n bcm2835_spi -s enable regulator -n bcm2835_ccp2tx -s enable regulator -n bcm2835_dsi -s enable These scripts enables/disable all regulators inside the bootloader. It was running with a "hard" and "soft" reset without any issues. These testcases should fit to Stephen Warren suggestions: "(a) before having explicitly turned the power domain on or off at all (b) after having turned it on (c) after having turned it off, and for all power domains." Cc: Stephen Warren <swarren@wwwdotorg.org> Cc: Lee Jones <lee@kernel.org> Cc: Eric Anholt <eric@anholt.net> Signed-off-by: Alexander Aring <alex.aring@gmail.com> --- changes since v2: - add pm_genpd_uninit to handle probing failure. - move power domain drive to his own driver in arch/arm/mach-bcm/ Also add own devicetree node for this driver, "raspberrypi,bcm2835-power". - Removing all power domains which might exists for the firmware API but we currently have no use-case for it. I tried to keep the same domain numbering in generic power domains subsystem like they are offered from the firmware API. This works, all power_domains which are NULL inside the array of genpd_onecell_data.domains[#] will be ignored. - Adding Eric Anholt and me to the authors. - Creating devicetree documentation for the power domain driver. - fix error handling in raspberrypi_firmware_set_power. - Remove comment about mapping between power domains array, this is not necessary anymore. I add a "enabled" attribute to raspberrypi_power_domain which indicates if a domain should be registered or not (zeroed values does not indicate such handling, but enabled is false then). - remove "goto mbox" not necessary anymore because an own driver implementation. - Update devicetrees for changes in v2. .../bindings/arm/bcm/raspberrypi,bcm2835-power.txt | 25 +++ arch/arm/boot/dts/bcm2835-rpi.dtsi | 11 ++ arch/arm/boot/dts/bcm2835.dtsi | 2 +- arch/arm/mach-bcm/Kconfig | 10 ++ arch/arm/mach-bcm/Makefile | 1 + arch/arm/mach-bcm/raspberrypi-power.c | 180 +++++++++++++++++++++ include/dt-bindings/arm/raspberrypi-power.h | 14 ++ 7 files changed, 242 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt create mode 100644 arch/arm/mach-bcm/raspberrypi-power.c create mode 100644 include/dt-bindings/arm/raspberrypi-power.h diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt new file mode 100644 index 0000000..c3abc24 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt @@ -0,0 +1,25 @@ +Raspberry Pi power domain driver + +Required properties: + +- compatible: Should be "raspberrypi,bcm2835-power". +- firmware: Reference to the RPi firmware device node. +- #power-domain-cells: Should be <1>, we providing multiple power domains. + +The valid defines for power domain are: + + RPI_POWER_DOMAIN_USB + +Example: + +power: power { + compatible = "raspberrypi,bcm2835-power"; + firmware = <&firmware>; + #power-domain-cells = <1>; +}; + +Example for using power domain: + +&usb { + power-domains = <&power RPI_POWER_DOMAIN_USB>; +}; diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi b/arch/arm/boot/dts/bcm2835-rpi.dtsi index ab5474e..d9b16d1 100644 --- a/arch/arm/boot/dts/bcm2835-rpi.dtsi +++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi @@ -1,3 +1,4 @@ +#include <dt-bindings/arm/raspberrypi-power.h> #include "bcm2835.dtsi" / { @@ -20,6 +21,12 @@ compatible = "raspberrypi,bcm2835-firmware"; mboxes = <&mailbox>; }; + + power: power { + compatible = "raspberrypi,bcm2835-power"; + firmware = <&firmware>; + #power-domain-cells = <1>; + }; }; }; @@ -56,3 +63,7 @@ status = "okay"; bus-width = <4>; }; + +&usb { + power-domains = <&power RPI_POWER_DOMAIN_USB>; +}; diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi index 301c73f..3c899b3 100644 --- a/arch/arm/boot/dts/bcm2835.dtsi +++ b/arch/arm/boot/dts/bcm2835.dtsi @@ -149,7 +149,7 @@ status = "disabled"; }; - usb@7e980000 { + usb: usb@7e980000 { compatible = "brcm,bcm2835-usb"; reg = <0x7e980000 0x10000>; interrupts = <1 9>; diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig index 1319c3c..244475e5 100644 --- a/arch/arm/mach-bcm/Kconfig +++ b/arch/arm/mach-bcm/Kconfig @@ -120,6 +120,16 @@ config ARCH_BCM2835 This enables support for the Broadcom BCM2835 SoC. This SoC is used in the Raspberry Pi and Roku 2 devices. +config RASPBERRY_POWER + bool "Raspberry Pi power domain driver" + depends on ARCH_BCM2835 + depends on RASPBERRYPI_FIRMWARE + select PM_GENERIC_DOMAINS if PM + select PM_GENERIC_DOMAINS_OF if PM + help + This enables support for the RPi power domains which can be enabled + or disabled via the RPi firmware. + config ARCH_BCM_63XX bool "Broadcom BCM63xx DSL SoC" if ARCH_MULTI_V7 depends on MMU diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile index 1780a3f..283295e 100644 --- a/arch/arm/mach-bcm/Makefile +++ b/arch/arm/mach-bcm/Makefile @@ -33,6 +33,7 @@ endif # BCM2835 obj-$(CONFIG_ARCH_BCM2835) += board_bcm2835.o +obj-$(CONFIG_RASPBERRY_POWER) += raspberrypi-power.o # BCM5301X obj-$(CONFIG_ARCH_BCM_5301X) += bcm_5301x.o diff --git a/arch/arm/mach-bcm/raspberrypi-power.c b/arch/arm/mach-bcm/raspberrypi-power.c new file mode 100644 index 0000000..531300f --- /dev/null +++ b/arch/arm/mach-bcm/raspberrypi-power.c @@ -0,0 +1,180 @@ +/* + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * Authors: + * (C) 2015 Pengutronix, Alexander Aring <aar@pengutronix.de> + * Eric Anholt <eric@anholt.net> + */ + +#include <linux/module.h> +#include <linux/pm_domain.h> +#include <linux/of_platform.h> +#include <linux/platform_device.h> +#include <dt-bindings/arm/raspberrypi-power.h> +#include <soc/bcm2835/raspberrypi-firmware.h> + +#define RPI_POWER_DOMAIN(_domain, _name) \ + [_domain] = { \ + .domain = _domain, \ + .enabled = true, \ + .base = { \ + .name = _name, \ + .power_off = raspberrypi_domain_off, \ + .power_on = raspberrypi_domain_on, \ + }, \ + } + +struct raspberrypi_power_domain { + u32 domain; + bool enabled; + struct generic_pm_domain base; +}; + +struct rpi_power_domain_packet { + u32 domain; + u32 on; +} __packet; + +static struct rpi_firmware *fw; +static struct genpd_onecell_data rpi_genpd_xlate; + +/* + * Asks the firmware to enable or disable power on a specific power + * domain. + */ +static int raspberrypi_firmware_set_power(u32 domain, bool on) +{ + struct rpi_power_domain_packet packet; + + packet.domain = domain; + packet.on = on; + return rpi_firmware_property(fw, RPI_FIRMWARE_SET_POWER_STATE, &packet, + sizeof(packet)); +} + +/* Asks the firmware to if power is on for a specific power domain. */ +static int raspberrypi_firmware_power_is_on(u32 domain) +{ + struct rpi_power_domain_packet packet; + int ret; + + packet.domain = domain; + ret = rpi_firmware_property(fw, RPI_FIRMWARE_GET_POWER_STATE, &packet, + sizeof(packet)); + if (ret < 0) + return ret; + + return packet.on & BIT(0); +} + +static int raspberrypi_domain_off(struct generic_pm_domain *domain) +{ + struct raspberrypi_power_domain *raspberrpi_domain = + container_of(domain, struct raspberrypi_power_domain, base); + + return raspberrypi_firmware_set_power(raspberrpi_domain->domain, false); +} + +static int raspberrypi_domain_on(struct generic_pm_domain *domain) +{ + struct raspberrypi_power_domain *raspberrpi_domain = + container_of(domain, struct raspberrypi_power_domain, base); + + return raspberrypi_firmware_set_power(raspberrpi_domain->domain, true); +} + +static struct raspberrypi_power_domain rpi_power_domains[] = { + RPI_POWER_DOMAIN(RPI_POWER_DOMAIN_USB, "USB"), +}; + +static int rpi_power_probe(struct platform_device *pdev) +{ + struct device_node *fw_np; + struct device *dev = &pdev->dev; + struct generic_pm_domain **power_domains; + int i, ret, num_domains = ARRAY_SIZE(rpi_power_domains); + + fw_np = of_parse_phandle(pdev->dev.of_node, "firmware", 0); + if (!fw_np) { + dev_err(&pdev->dev, "no firmware node\n"); + return -ENODEV; + } + + fw = rpi_firmware_get(fw_np); + if (!fw) + return -EPROBE_DEFER; + + power_domains = devm_kzalloc(dev, sizeof(*power_domains) * num_domains, + GFP_KERNEL); + if (!power_domains) + return -ENOMEM; + + rpi_genpd_xlate.domains = power_domains; + rpi_genpd_xlate.num_domains = num_domains; + + for (i = 0; i < num_domains; i++) { + bool is_off; + + if (!rpi_power_domains[i].enabled) + continue; + + /* get the initial state */ + ret = raspberrypi_firmware_power_is_on(rpi_power_domains[i].domain); + if (ret < 0) + goto uninit_pm; + + /* pm_genpd_init needs is_off, invert the logic here */ + is_off = !ret; + pm_genpd_init(&rpi_power_domains[i].base, NULL, is_off); + /* let power_domains array know about the registered pm */ + power_domains[i] = &rpi_power_domains[i].base; + } + + ret = of_genpd_add_provider_onecell(dev->of_node, &rpi_genpd_xlate); + if (ret < 0) + goto uninit_pm; + + return 0; + +uninit_pm: + for (i = 0; i < num_domains; i++) + pm_genpd_uninit(power_domains[i]); + + return ret; +} + +static int rpi_power_remove(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + int i; + + for (i = 0; i < rpi_genpd_xlate.num_domains; i++) + pm_genpd_uninit(rpi_genpd_xlate.domains[i]); + + of_genpd_del_provider(dev->of_node); + + return 0; +} + +static const struct of_device_id rpi_power_of_match[] = { + { .compatible = "raspberrypi,bcm2835-power", }, + {}, +}; +MODULE_DEVICE_TABLE(of, rpi_power_of_match); + +static struct platform_driver rpi_power_driver = { + .driver = { + .name = "raspberrypi-power", + .of_match_table = rpi_power_of_match, + }, + .probe = rpi_power_probe, + .remove = rpi_power_remove, +}; +module_platform_driver(rpi_power_driver); + +MODULE_AUTHOR("Alexander Aring <aar@pengutronix.de>"); +MODULE_AUTHOR("Eric Anholt <eric@anholt.net>"); +MODULE_DESCRIPTION("Raspberry Pi power domain driver"); +MODULE_LICENSE("GPL v2"); diff --git a/include/dt-bindings/arm/raspberrypi-power.h b/include/dt-bindings/arm/raspberrypi-power.h new file mode 100644 index 0000000..51f0772 --- /dev/null +++ b/include/dt-bindings/arm/raspberrypi-power.h @@ -0,0 +1,14 @@ +/* + * Copyright © 2015 Broadcom + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#ifndef _DT_BINDINGS_ARM_BCM2835_MBOX_POWER_H +#define _DT_BINDINGS_ARM_BCM2835_MBOX_POWER_H + +#define RPI_POWER_DOMAIN_USB 3 + +#endif /* _DT_BINDINGS_ARM_BCM2835_MBOX_POWER_H */ -- 2.6.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
[parent not found: <1446590711-18928-3-git-send-email-alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [RFCv2 2/2] rpi: add support to enable usb power domain [not found] ` <1446590711-18928-3-git-send-email-alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2015-11-05 7:15 ` Stefan Wahren 2015-11-05 14:14 ` Alexander Aring [not found] ` <563B0217.4030307-saaNCTdWVBT7BZbvpMY5sg@public.gmane.org> 0 siblings, 2 replies; 14+ messages in thread From: Stefan Wahren @ 2015-11-05 7:15 UTC (permalink / raw) To: Alexander Aring Cc: linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, mark.rutland-5wv7dgnIgG8, ulf.hansson-QSEj5FYQhm4dnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA, pavel-+ZI9xUNit7I, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, linux-lFZ/pmaqli7XmaaqVzeoHQ, khilman-DgEjT+Ai2ygdnm+yROfE0A, bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w, len.brown-ral2JQCrhuEAvxtiuMwx3w, pawel.moll-5wv7dgnIgG8, rjui-dY08KVG/lbpWk0Htik3J/w, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg, sbranden-dY08KVG/lbpWk0Htik3J/w, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, linux-pm-u79uwXL29TY76Z2rM5mHXA, rjw-LthD3rsA81gm4RdzfppkhA, kernel-bIcnvbaLZ9MEGnE8C9+IrQ, galak-sgV2jX0FEOL9JmXXK+q4OQ Hi Alexander, i think this subject should better start with "ARM:". Am 03.11.2015 um 23:45 schrieb Alexander Aring: > This patch adds support for RPi several Power Domains and enable support > to enable the USB Power Domain when it's not enabled before. > > This patch based on Eric Anholt's patch to support Power Domains. He had > an issue about -EPROBE_DEFER inside the power domain subsystem, this > issue was solved by commit <311fa6a> ("PM / Domains: Return -EPROBE_DEFER > if we fail to init or turn-on domain"). > >[...] > diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt > new file mode 100644 > index 0000000..c3abc24 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt > @@ -0,0 +1,25 @@ > +Raspberry Pi power domain driver > + > +Required properties: > + > +- compatible: Should be "raspberrypi,bcm2835-power". > +- firmware: Reference to the RPi firmware device node. > +- #power-domain-cells: Should be <1>, we providing multiple power domains. > + > +The valid defines for power domain are: > + > + RPI_POWER_DOMAIN_USB > + > +Example: > + > +power: power { > + compatible = "raspberrypi,bcm2835-power"; > + firmware = <&firmware>; > + #power-domain-cells = <1>; > +}; > + > +Example for using power domain: > + > +&usb { > + power-domains = <&power RPI_POWER_DOMAIN_USB>; > +}; Refering to Documentation/devicetree/bindings/submitting-patches.txt binding doc should be a separate patch. > diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi b/arch/arm/boot/dts/bcm2835-rpi.dtsi > index ab5474e..d9b16d1 100644 > --- a/arch/arm/boot/dts/bcm2835-rpi.dtsi > +++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi > @@ -1,3 +1,4 @@ > +#include <dt-bindings/arm/raspberrypi-power.h> > #include "bcm2835.dtsi" > > / { > @@ -20,6 +21,12 @@ > compatible = "raspberrypi,bcm2835-firmware"; > mboxes = <&mailbox>; > }; > + > + power: power { > + compatible = "raspberrypi,bcm2835-power"; > + firmware = <&firmware>; > + #power-domain-cells = <1>; > + }; > }; > }; > > @@ -56,3 +63,7 @@ > status = "okay"; > bus-width = <4>; > }; > + > +&usb { > + power-domains = <&power RPI_POWER_DOMAIN_USB>; > +}; > diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi > index 301c73f..3c899b3 100644 > --- a/arch/arm/boot/dts/bcm2835.dtsi > +++ b/arch/arm/boot/dts/bcm2835.dtsi > @@ -149,7 +149,7 @@ > status = "disabled"; > }; > > - usb@7e980000 { > + usb: usb@7e980000 { > compatible = "brcm,bcm2835-usb"; > reg = <0x7e980000 0x10000>; > interrupts = <1 9>; > diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig > index 1319c3c..244475e5 100644 > --- a/arch/arm/mach-bcm/Kconfig > +++ b/arch/arm/mach-bcm/Kconfig > @@ -120,6 +120,16 @@ config ARCH_BCM2835 > This enables support for the Broadcom BCM2835 SoC. This SoC is > used in the Raspberry Pi and Roku 2 devices. > > +config RASPBERRY_POWER RASPBERRYPI_POWER? > + bool "Raspberry Pi power domain driver" > + depends on ARCH_BCM2835 > + depends on RASPBERRYPI_FIRMWARE > + select PM_GENERIC_DOMAINS if PM Since PM_GENERIC_DOMAINS_OF depends on PM_GENERIC_DOMAINS this line should be redundant. > + select PM_GENERIC_DOMAINS_OF if PM > + help > + This enables support for the RPi power domains which can be enabled > + or disabled via the RPi firmware. > + > config ARCH_BCM_63XX > bool "Broadcom BCM63xx DSL SoC" if ARCH_MULTI_V7 > depends on MMU > diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile > index 1780a3f..283295e 100644 > --- a/arch/arm/mach-bcm/Makefile > +++ b/arch/arm/mach-bcm/Makefile > @@ -33,6 +33,7 @@ endif > > # BCM2835 > obj-$(CONFIG_ARCH_BCM2835) += board_bcm2835.o > +obj-$(CONFIG_RASPBERRY_POWER) += raspberrypi-power.o > > # BCM5301X > obj-$(CONFIG_ARCH_BCM_5301X) += bcm_5301x.o > diff --git a/arch/arm/mach-bcm/raspberrypi-power.c b/arch/arm/mach-bcm/raspberrypi-power.c > new file mode 100644 > index 0000000..531300f > --- /dev/null > +++ b/arch/arm/mach-bcm/raspberrypi-power.c > @@ -0,0 +1,180 @@ > +/* > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * Authors: > + * (C) 2015 Pengutronix, Alexander Aring <aar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> > + * Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org> > + */ > + > +#include <linux/module.h> > +#include <linux/pm_domain.h> > +#include <linux/of_platform.h> > +#include <linux/platform_device.h> Please sort alphabetical. > +#include <dt-bindings/arm/raspberrypi-power.h> > +#include <soc/bcm2835/raspberrypi-firmware.h> > + > +#define RPI_POWER_DOMAIN(_domain, _name) \ > + [_domain] = { \ > + .domain = _domain, \ > + .enabled = true, \ > + .base = { \ > + .name = _name, \ > + .power_off = raspberrypi_domain_off, \ > + .power_on = raspberrypi_domain_on, \ > + }, \ > + } > + > +struct raspberrypi_power_domain { > + u32 domain; > + bool enabled; > + struct generic_pm_domain base; > +}; > + > +struct rpi_power_domain_packet { > + u32 domain; > + u32 on; > +} __packet; It would be nice to use consequently rpi_ as prefix instead of raspberrypi_ . > [...] > diff --git a/include/dt-bindings/arm/raspberrypi-power.h b/include/dt-bindings/arm/raspberrypi-power.h > new file mode 100644 > index 0000000..51f0772 > --- /dev/null > +++ b/include/dt-bindings/arm/raspberrypi-power.h > @@ -0,0 +1,14 @@ > +/* > + * Copyright © 2015 Broadcom > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > + > +#ifndef _DT_BINDINGS_ARM_BCM2835_MBOX_POWER_H > +#define _DT_BINDINGS_ARM_BCM2835_MBOX_POWER_H This needs renaming. Thanks Stefan > + > +#define RPI_POWER_DOMAIN_USB 3 > + > +#endif /* _DT_BINDINGS_ARM_BCM2835_MBOX_POWER_H */ > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFCv2 2/2] rpi: add support to enable usb power domain 2015-11-05 7:15 ` Stefan Wahren @ 2015-11-05 14:14 ` Alexander Aring [not found] ` <563B0217.4030307-saaNCTdWVBT7BZbvpMY5sg@public.gmane.org> 1 sibling, 0 replies; 14+ messages in thread From: Alexander Aring @ 2015-11-05 14:14 UTC (permalink / raw) To: Stefan Wahren Cc: linux-rpi-kernel, mark.rutland, ulf.hansson, devicetree, pavel, f.fainelli, linux, khilman, bcm-kernel-feedback-list, len.brown, pawel.moll, rjui, robh+dt, linux-arm-kernel, ijc+devicetree, sbranden, gregkh, linux-pm, rjw, kernel, galak On Thu, Nov 05, 2015 at 08:15:35AM +0100, Stefan Wahren wrote: > Hi Alexander, > > i think this subject should better start with "ARM:". > ok. I will change every thing which you recognised. Thanks. - Alex ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <563B0217.4030307-saaNCTdWVBT7BZbvpMY5sg@public.gmane.org>]
* Re: [RFCv2 2/2] rpi: add support to enable usb power domain [not found] ` <563B0217.4030307-saaNCTdWVBT7BZbvpMY5sg@public.gmane.org> @ 2015-11-13 12:22 ` Alexander Aring 0 siblings, 0 replies; 14+ messages in thread From: Alexander Aring @ 2015-11-13 12:22 UTC (permalink / raw) To: Stefan Wahren Cc: linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, mark.rutland-5wv7dgnIgG8, ulf.hansson-QSEj5FYQhm4dnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA, pavel-+ZI9xUNit7I, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, linux-lFZ/pmaqli7XmaaqVzeoHQ, khilman-DgEjT+Ai2ygdnm+yROfE0A, bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w, len.brown-ral2JQCrhuEAvxtiuMwx3w, pawel.moll-5wv7dgnIgG8, rjui-dY08KVG/lbpWk0Htik3J/w, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg, sbranden-dY08KVG/lbpWk0Htik3J/w, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, linux-pm-u79uwXL29TY76Z2rM5mHXA, rjw-LthD3rsA81gm4RdzfppkhA, kernel-bIcnvbaLZ9MEGnE8C9+IrQ, galak-sgV2jX0FEOL9JmXXK+q4OQ On Thu, Nov 05, 2015 at 08:15:35AM +0100, Stefan Wahren wrote: ... > >+ bool "Raspberry Pi power domain driver" > >+ depends on ARCH_BCM2835 > >+ depends on RASPBERRYPI_FIRMWARE > >+ select PM_GENERIC_DOMAINS if PM > > Since PM_GENERIC_DOMAINS_OF depends on PM_GENERIC_DOMAINS this line should > be redundant. > I can't remove the line: select PM_GENERIC_DOMAINS if PM here. It's true that "PM_GENERIC_DOMAINS_OF depends on PM_GENERIC_DOMAINS", but Kconfig can't handle the dependency here with select. That's why sometimes people teaches me "select is evil". I will get: warning: (RASPBERRYPI_POWER) selects PM_GENERIC_DOMAINS_OF which has unmet direct dependencies (PM_GENERIC_DOMAINS && OF) otherwise. Nevertheless we can't also use "depends on PM_GENERIC_DOMAINS if PM" because, PM_GENERIC_DOMAINS is a hidden entry (has no Kconfig prompt). - Alex -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFCv2 2/2] rpi: add support to enable usb power domain 2015-11-03 22:45 ` [RFCv2 2/2] rpi: add support to enable usb power domain Alexander Aring [not found] ` <1446590711-18928-3-git-send-email-alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2015-11-05 13:35 ` Rob Herring 2015-11-05 14:12 ` Alexander Aring 1 sibling, 1 reply; 14+ messages in thread From: Rob Herring @ 2015-11-05 13:35 UTC (permalink / raw) To: Alexander Aring Cc: linux-rpi-kernel, pawel.moll, mark.rutland, ijc+devicetree, galak, swarren, lee, eric, linux, f.fainelli, rjui, sbranden, rjw, khilman, ulf.hansson, len.brown, pavel, gregkh, devicetree, linux-arm-kernel, linux-pm, bcm-kernel-feedback-list, kernel On Tue, Nov 03, 2015 at 11:45:11PM +0100, Alexander Aring wrote: > This patch adds support for RPi several Power Domains and enable support > to enable the USB Power Domain when it's not enabled before. > > This patch based on Eric Anholt's patch to support Power Domains. He had > an issue about -EPROBE_DEFER inside the power domain subsystem, this > issue was solved by commit <311fa6a> ("PM / Domains: Return -EPROBE_DEFER > if we fail to init or turn-on domain"). > > It was tested with barebox and the following scripts before booting > linux: > > /env/a_off: > > # cat /env/a_off > #turn off which are enabled by default > regulator -n bcm2835_mci0 -s disable > regulator -n uart0-pl0110 -s disable > > /env/a_on: > > # cat /env/a_on > #turn off which are enabled by default > regulator -n bcm2835_mci0 -s disable > regulator -n uart0-pl0110 -s disable > > regulator -n bcm2835_mci0 -s enable > regulator -n uart0-pl0110 -s enable > regulator -n uart0-pl0111 -s enable > regulator -n bcm2835_usb -s enable > regulator -n bcm2835_i2c0 -s enable > regulator -n bcm2835_i2c1 -s enable > regulator -n bcm2835_i2c2 -s enable > regulator -n bcm2835_spi -s enable > regulator -n bcm2835_ccp2tx -s enable > regulator -n bcm2835_dsi -s enable > > /env/b: > > # cat /env/b > sh /env/a_on > > regulator -n bcm2835_mci0 -s disable > regulator -n uart0-pl0110 -s disable > regulator -n uart0-pl0111 -s disable > regulator -n bcm2835_usb -s disable > regulator -n bcm2835_i2c0 -s disable > regulator -n bcm2835_i2c1 -s disable > regulator -n bcm2835_i2c2 -s disable > regulator -n bcm2835_spi -s disable > regulator -n bcm2835_ccp2tx -s disable > regulator -n bcm2835_dsi -s disable > > /env/c: > > # cat /env/c > sh ./env/b > > regulator -n bcm2835_mci0 -s enable > regulator -n uart0-pl0110 -s enable > regulator -n uart0-pl0111 -s enable > regulator -n bcm2835_usb -s enable > regulator -n bcm2835_i2c0 -s enable > regulator -n bcm2835_i2c1 -s enable > regulator -n bcm2835_i2c2 -s enable > regulator -n bcm2835_spi -s enable > regulator -n bcm2835_ccp2tx -s enable > regulator -n bcm2835_dsi -s enable > > These scripts enables/disable all regulators inside the bootloader. It > was running with a "hard" and "soft" reset without any issues. These > testcases should fit to Stephen Warren suggestions: > > "(a) before having explicitly turned the power domain on or off at all (b) > after having turned it on (c) after having turned it off, and for all > power domains." > > Cc: Stephen Warren <swarren@wwwdotorg.org> > Cc: Lee Jones <lee@kernel.org> > Cc: Eric Anholt <eric@anholt.net> > Signed-off-by: Alexander Aring <alex.aring@gmail.com> > --- > changes since v2: > - add pm_genpd_uninit to handle probing failure. > - move power domain drive to his own driver in arch/arm/mach-bcm/ > Also add own devicetree node for this driver, "raspberrypi,bcm2835-power". > - Removing all power domains which might exists for the firmware API but > we currently have no use-case for it. I tried to keep the same domain > numbering in generic power domains subsystem like they are offered from > the firmware API. This works, all power_domains which are NULL inside > the array of genpd_onecell_data.domains[#] will be ignored. > - Adding Eric Anholt and me to the authors. > - Creating devicetree documentation for the power domain driver. > - fix error handling in raspberrypi_firmware_set_power. > - Remove comment about mapping between power domains array, this is not > necessary anymore. I add a "enabled" attribute to raspberrypi_power_domain > which indicates if a domain should be registered or not (zeroed values > does not indicate such handling, but enabled is false then). > - remove "goto mbox" not necessary anymore because an own driver > implementation. > - Update devicetrees for changes in v2. > > .../bindings/arm/bcm/raspberrypi,bcm2835-power.txt | 25 +++ > arch/arm/boot/dts/bcm2835-rpi.dtsi | 11 ++ > arch/arm/boot/dts/bcm2835.dtsi | 2 +- > arch/arm/mach-bcm/Kconfig | 10 ++ > arch/arm/mach-bcm/Makefile | 1 + > arch/arm/mach-bcm/raspberrypi-power.c | 180 +++++++++++++++++++++ > include/dt-bindings/arm/raspberrypi-power.h | 14 ++ > 7 files changed, 242 insertions(+), 1 deletion(-) > create mode 100644 Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt > create mode 100644 arch/arm/mach-bcm/raspberrypi-power.c > create mode 100644 include/dt-bindings/arm/raspberrypi-power.h > > diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt > new file mode 100644 > index 0000000..c3abc24 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt > @@ -0,0 +1,25 @@ > +Raspberry Pi power domain driver > + > +Required properties: > + > +- compatible: Should be "raspberrypi,bcm2835-power". These are board or chip level power domains? If the latter, the vendor should not be raspberrypi, but Broadcom. If the former, then it should describe the board rather than the chip. Rob ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFCv2 2/2] rpi: add support to enable usb power domain 2015-11-05 13:35 ` Rob Herring @ 2015-11-05 14:12 ` Alexander Aring 0 siblings, 0 replies; 14+ messages in thread From: Alexander Aring @ 2015-11-05 14:12 UTC (permalink / raw) To: Rob Herring Cc: linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, pawel.moll-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg, galak-sgV2jX0FEOL9JmXXK+q4OQ, swarren-3lzwWm7+Weoh9ZMKESR00Q, lee-DgEjT+Ai2ygdnm+yROfE0A, eric-WhKQ6XTQaPysTnJN9+BGXg, linux-lFZ/pmaqli7XmaaqVzeoHQ, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, rjui-dY08KVG/lbpWk0Htik3J/w, sbranden-dY08KVG/lbpWk0Htik3J/w, rjw-LthD3rsA81gm4RdzfppkhA, khilman-DgEjT+Ai2ygdnm+yROfE0A, ulf.hansson-QSEj5FYQhm4dnm+yROfE0A, len.brown-ral2JQCrhuEAvxtiuMwx3w, pavel-+ZI9xUNit7I, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-pm-u79uwXL29TY76Z2rM5mHXA, bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w, kernel-bIcnvbaLZ9MEGnE8C9+IrQ Hi, On Thu, Nov 05, 2015 at 07:35:10AM -0600, Rob Herring wrote: > On Tue, Nov 03, 2015 at 11:45:11PM +0100, Alexander Aring wrote: ... > > > > diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt > > new file mode 100644 > > index 0000000..c3abc24 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt > > @@ -0,0 +1,25 @@ > > +Raspberry Pi power domain driver > > + > > +Required properties: > > + > > +- compatible: Should be "raspberrypi,bcm2835-power". > > These are board or chip level power domains? If the latter, the vendor > should not be raspberrypi, but Broadcom. If the former, then it should > describe the board rather than the chip. > the access to the power domains is provided by the firmware. The firmware is RPi specific. On an other bcm2835 with another firmware it would be working complete different. Everything which is accessable over the firmware is RPi specific. - Alex -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2015-11-13 12:56 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-03 22:45 [RFCv2 0/2] rpi: add support for rpi power domain driver Alexander Aring
[not found] ` <1446590711-18928-1-git-send-email-alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-11-03 22:45 ` [RFCv2 1/2] power: domain: add pm_genpd_uninit Alexander Aring
2015-11-05 9:01 ` Ulf Hansson
2015-11-05 14:34 ` Alexander Aring
2015-11-11 18:00 ` Alexander Aring
2015-11-11 20:33 ` Ulf Hansson
2015-11-13 12:56 ` Alexander Aring
2015-11-11 20:29 ` Ulf Hansson
2015-11-03 22:45 ` [RFCv2 2/2] rpi: add support to enable usb power domain Alexander Aring
[not found] ` <1446590711-18928-3-git-send-email-alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-11-05 7:15 ` Stefan Wahren
2015-11-05 14:14 ` Alexander Aring
[not found] ` <563B0217.4030307-saaNCTdWVBT7BZbvpMY5sg@public.gmane.org>
2015-11-13 12:22 ` Alexander Aring
2015-11-05 13:35 ` Rob Herring
2015-11-05 14:12 ` Alexander Aring
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).