* [PATCH RFC 0/5] Dove PMU support @ 2014-04-27 13:23 ` Russell King - ARM Linux 0 siblings, 0 replies; 49+ messages in thread From: Russell King - ARM Linux @ 2014-04-27 13:23 UTC (permalink / raw) To: devicetree, linux-arm-kernel, linux-pm, Mark Rutland, Pawel Moll, Rafael J. Wysocki, Rob Herring, Sebastian Hesselbarth The following series of patches add better PMU support for Dove. This has been developed on the Cubox, and tested in non-DT and DT modes. This also improves the interrupt handling over the existing code: the existing code ends up calling the interrupt handlers twice for every interrupt raised, because the interrupt clear-down is done at the wrong point - we need to clear down the interrupt in the device first, then clear it down in the controller. The problem this gives is that it can be racy (see comments in the driver) so we're careful about how we do that to minimise the window. I've included all patches here - the initial set are targetted towards adding DT support, with the final adding the non-DT support. There is a call to the initialisation function missing for DT mode - I'd like the mvebu people to comment on how that should be handled, as it needs to be done pretty early. Also included are two PM domain changes: the first I've discussed with Rafael who seems happy with it. The second is necessary because we have no way to know if a generic PM domain is associated with a device or whether something else making use of the PM domain is installed in the dev->pm_domain pointer, so this allows that decision to be made by core PM code. This is more a "this is where I'm at" with this stuff than a real submission, nevertheless comments on how to get it ready for submission would be welcome. I'd like to get this off my plate ASAP. arch/arm/Kconfig | 1 + arch/arm/boot/dts/dove.dtsi | 7 + arch/arm/mach-dove/Makefile | 1 + arch/arm/mach-dove/common.c | 2 + arch/arm/mach-dove/common.h | 1 + arch/arm/mach-dove/include/mach/pm.h | 17 -- arch/arm/mach-dove/irq.c | 87 ------ arch/arm/mach-dove/pmu.c | 531 +++++++++++++++++++++++++++++++++++ drivers/base/power/domain.c | 8 +- 9 files changed, 547 insertions(+), 108 deletions(-) -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH RFC 0/5] Dove PMU support @ 2014-04-27 13:23 ` Russell King - ARM Linux 0 siblings, 0 replies; 49+ messages in thread From: Russell King - ARM Linux @ 2014-04-27 13:23 UTC (permalink / raw) To: linux-arm-kernel The following series of patches add better PMU support for Dove. This has been developed on the Cubox, and tested in non-DT and DT modes. This also improves the interrupt handling over the existing code: the existing code ends up calling the interrupt handlers twice for every interrupt raised, because the interrupt clear-down is done at the wrong point - we need to clear down the interrupt in the device first, then clear it down in the controller. The problem this gives is that it can be racy (see comments in the driver) so we're careful about how we do that to minimise the window. I've included all patches here - the initial set are targetted towards adding DT support, with the final adding the non-DT support. There is a call to the initialisation function missing for DT mode - I'd like the mvebu people to comment on how that should be handled, as it needs to be done pretty early. Also included are two PM domain changes: the first I've discussed with Rafael who seems happy with it. The second is necessary because we have no way to know if a generic PM domain is associated with a device or whether something else making use of the PM domain is installed in the dev->pm_domain pointer, so this allows that decision to be made by core PM code. This is more a "this is where I'm at" with this stuff than a real submission, nevertheless comments on how to get it ready for submission would be welcome. I'd like to get this off my plate ASAP. arch/arm/Kconfig | 1 + arch/arm/boot/dts/dove.dtsi | 7 + arch/arm/mach-dove/Makefile | 1 + arch/arm/mach-dove/common.c | 2 + arch/arm/mach-dove/common.h | 1 + arch/arm/mach-dove/include/mach/pm.h | 17 -- arch/arm/mach-dove/irq.c | 87 ------ arch/arm/mach-dove/pmu.c | 531 +++++++++++++++++++++++++++++++++++ drivers/base/power/domain.c | 8 +- 9 files changed, 547 insertions(+), 108 deletions(-) -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ^ permalink raw reply [flat|nested] 49+ messages in thread
[parent not found: <20140427132312.GC26756-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>]
* [PATCH 1/5] pm: domains: quieten down generic pm domains 2014-04-27 13:23 ` Russell King - ARM Linux @ 2014-04-27 13:28 ` Russell King -1 siblings, 0 replies; 49+ messages in thread From: Russell King @ 2014-04-27 13:28 UTC (permalink / raw) To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Sebastian Hesselbarth Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, linux-pm-u79uwXL29TY76Z2rM5mHXA, Mark Rutland, Pawel Moll, Rafael J. Wysocki, Rob Herring Signed-off-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org> --- drivers/base/power/domain.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index bfb8955c406c..ea91ea0e137b 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -42,7 +42,7 @@ struct gpd_timing_data *__td = &dev_gpd_data(dev)->td; \ if (!__retval && __elapsed > __td->field) { \ __td->field = __elapsed; \ - dev_warn(dev, name " latency exceeded, new value %lld ns\n", \ + dev_dbg(dev, name " latency exceeded, new value %lld ns\n", \ __elapsed); \ genpd->max_off_time_changed = true; \ __td->constraint_changed = true; \ @@ -242,7 +242,7 @@ static int __pm_genpd_poweron(struct generic_pm_domain *genpd) genpd->max_off_time_changed = true; genpd_recalc_cpu_exit_latency(genpd); if (genpd->name) - pr_warning("%s: Power-on latency exceeded, " + pr_debug("%s: Power-on latency exceeded, " "new value %lld ns\n", genpd->name, elapsed_ns); } @@ -566,7 +566,7 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd) genpd->power_off_latency_ns = elapsed_ns; genpd->max_off_time_changed = true; if (genpd->name) - pr_warning("%s: Power-off latency exceeded, " + pr_debug("%s: Power-off latency exceeded, " "new value %lld ns\n", genpd->name, elapsed_ns); } -- 1.8.3.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] 49+ messages in thread
* [PATCH 1/5] pm: domains: quieten down generic pm domains @ 2014-04-27 13:28 ` Russell King 0 siblings, 0 replies; 49+ messages in thread From: Russell King @ 2014-04-27 13:28 UTC (permalink / raw) To: linux-arm-kernel Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> --- drivers/base/power/domain.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index bfb8955c406c..ea91ea0e137b 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -42,7 +42,7 @@ struct gpd_timing_data *__td = &dev_gpd_data(dev)->td; \ if (!__retval && __elapsed > __td->field) { \ __td->field = __elapsed; \ - dev_warn(dev, name " latency exceeded, new value %lld ns\n", \ + dev_dbg(dev, name " latency exceeded, new value %lld ns\n", \ __elapsed); \ genpd->max_off_time_changed = true; \ __td->constraint_changed = true; \ @@ -242,7 +242,7 @@ static int __pm_genpd_poweron(struct generic_pm_domain *genpd) genpd->max_off_time_changed = true; genpd_recalc_cpu_exit_latency(genpd); if (genpd->name) - pr_warning("%s: Power-on latency exceeded, " + pr_debug("%s: Power-on latency exceeded, " "new value %lld ns\n", genpd->name, elapsed_ns); } @@ -566,7 +566,7 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd) genpd->power_off_latency_ns = elapsed_ns; genpd->max_off_time_changed = true; if (genpd->name) - pr_warning("%s: Power-off latency exceeded, " + pr_debug("%s: Power-off latency exceeded, " "new value %lld ns\n", genpd->name, elapsed_ns); } -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 1/5] pm: domains: quieten down generic pm domains 2014-04-27 13:28 ` Russell King @ 2014-04-30 23:46 ` Rafael J. Wysocki -1 siblings, 0 replies; 49+ messages in thread From: Rafael J. Wysocki @ 2014-04-30 23:46 UTC (permalink / raw) To: Russell King Cc: linux-arm-kernel, Sebastian Hesselbarth, devicetree, linux-pm, Mark Rutland, Pawel Moll, Rob Herring On Sunday, April 27, 2014 02:28:50 PM Russell King wrote: > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> Acked-by: Rafael J. Wysocki <rjw@rjwysocki.net> Since you need this one and [2/5] for the rest of the patchset, please feel free to take them through your tree if that's convenient. > --- > drivers/base/power/domain.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index bfb8955c406c..ea91ea0e137b 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -42,7 +42,7 @@ > struct gpd_timing_data *__td = &dev_gpd_data(dev)->td; \ > if (!__retval && __elapsed > __td->field) { \ > __td->field = __elapsed; \ > - dev_warn(dev, name " latency exceeded, new value %lld ns\n", \ > + dev_dbg(dev, name " latency exceeded, new value %lld ns\n", \ > __elapsed); \ > genpd->max_off_time_changed = true; \ > __td->constraint_changed = true; \ > @@ -242,7 +242,7 @@ static int __pm_genpd_poweron(struct generic_pm_domain *genpd) > genpd->max_off_time_changed = true; > genpd_recalc_cpu_exit_latency(genpd); > if (genpd->name) > - pr_warning("%s: Power-on latency exceeded, " > + pr_debug("%s: Power-on latency exceeded, " > "new value %lld ns\n", genpd->name, > elapsed_ns); > } > @@ -566,7 +566,7 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd) > genpd->power_off_latency_ns = elapsed_ns; > genpd->max_off_time_changed = true; > if (genpd->name) > - pr_warning("%s: Power-off latency exceeded, " > + pr_debug("%s: Power-off latency exceeded, " > "new value %lld ns\n", genpd->name, > elapsed_ns); > } > -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 1/5] pm: domains: quieten down generic pm domains @ 2014-04-30 23:46 ` Rafael J. Wysocki 0 siblings, 0 replies; 49+ messages in thread From: Rafael J. Wysocki @ 2014-04-30 23:46 UTC (permalink / raw) To: linux-arm-kernel On Sunday, April 27, 2014 02:28:50 PM Russell King wrote: > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> Acked-by: Rafael J. Wysocki <rjw@rjwysocki.net> Since you need this one and [2/5] for the rest of the patchset, please feel free to take them through your tree if that's convenient. > --- > drivers/base/power/domain.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index bfb8955c406c..ea91ea0e137b 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -42,7 +42,7 @@ > struct gpd_timing_data *__td = &dev_gpd_data(dev)->td; \ > if (!__retval && __elapsed > __td->field) { \ > __td->field = __elapsed; \ > - dev_warn(dev, name " latency exceeded, new value %lld ns\n", \ > + dev_dbg(dev, name " latency exceeded, new value %lld ns\n", \ > __elapsed); \ > genpd->max_off_time_changed = true; \ > __td->constraint_changed = true; \ > @@ -242,7 +242,7 @@ static int __pm_genpd_poweron(struct generic_pm_domain *genpd) > genpd->max_off_time_changed = true; > genpd_recalc_cpu_exit_latency(genpd); > if (genpd->name) > - pr_warning("%s: Power-on latency exceeded, " > + pr_debug("%s: Power-on latency exceeded, " > "new value %lld ns\n", genpd->name, > elapsed_ns); > } > @@ -566,7 +566,7 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd) > genpd->power_off_latency_ns = elapsed_ns; > genpd->max_off_time_changed = true; > if (genpd->name) > - pr_warning("%s: Power-off latency exceeded, " > + pr_debug("%s: Power-off latency exceeded, " > "new value %lld ns\n", genpd->name, > elapsed_ns); > } > -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH 1/5] pm: domains: quieten down generic pm domains 2014-04-30 23:46 ` Rafael J. Wysocki @ 2014-05-02 9:24 ` Ulf Hansson -1 siblings, 0 replies; 49+ messages in thread From: Ulf Hansson @ 2014-05-02 9:24 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Russell King, linux-arm-kernel, Sebastian Hesselbarth, devicetree, linux-pm, Mark Rutland, Pawel Moll, Rob Herring On 1 May 2014 01:46, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > On Sunday, April 27, 2014 02:28:50 PM Russell King wrote: >> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > > Acked-by: Rafael J. Wysocki <rjw@rjwysocki.net> > > Since you need this one and [2/5] for the rest of the patchset, please feel > free to take them through your tree if that's convenient. Unless it complicates things for Russell, I would prefer if this could go via Rafael's tree. I have a quite big patchset for the PM domain, it would help me if I have a single point to base my work upon. Additionally, you have Tomasz Figa patchset below to consider for the PM domain code. [PATCH v3 0/3] Generic Device Tree based power domain look-up Kind regards Ulf Hansson > >> --- >> drivers/base/power/domain.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c >> index bfb8955c406c..ea91ea0e137b 100644 >> --- a/drivers/base/power/domain.c >> +++ b/drivers/base/power/domain.c >> @@ -42,7 +42,7 @@ >> struct gpd_timing_data *__td = &dev_gpd_data(dev)->td; \ >> if (!__retval && __elapsed > __td->field) { \ >> __td->field = __elapsed; \ >> - dev_warn(dev, name " latency exceeded, new value %lld ns\n", \ >> + dev_dbg(dev, name " latency exceeded, new value %lld ns\n", \ >> __elapsed); \ >> genpd->max_off_time_changed = true; \ >> __td->constraint_changed = true; \ >> @@ -242,7 +242,7 @@ static int __pm_genpd_poweron(struct generic_pm_domain *genpd) >> genpd->max_off_time_changed = true; >> genpd_recalc_cpu_exit_latency(genpd); >> if (genpd->name) >> - pr_warning("%s: Power-on latency exceeded, " >> + pr_debug("%s: Power-on latency exceeded, " >> "new value %lld ns\n", genpd->name, >> elapsed_ns); >> } >> @@ -566,7 +566,7 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd) >> genpd->power_off_latency_ns = elapsed_ns; >> genpd->max_off_time_changed = true; >> if (genpd->name) >> - pr_warning("%s: Power-off latency exceeded, " >> + pr_debug("%s: Power-off latency exceeded, " >> "new value %lld ns\n", genpd->name, >> elapsed_ns); >> } >> > > -- > I speak only for myself. > Rafael J. Wysocki, Intel Open Source Technology Center. > -- > To unsubscribe from this list: send the line "unsubscribe linux-pm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 1/5] pm: domains: quieten down generic pm domains @ 2014-05-02 9:24 ` Ulf Hansson 0 siblings, 0 replies; 49+ messages in thread From: Ulf Hansson @ 2014-05-02 9:24 UTC (permalink / raw) To: linux-arm-kernel On 1 May 2014 01:46, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > On Sunday, April 27, 2014 02:28:50 PM Russell King wrote: >> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > > Acked-by: Rafael J. Wysocki <rjw@rjwysocki.net> > > Since you need this one and [2/5] for the rest of the patchset, please feel > free to take them through your tree if that's convenient. Unless it complicates things for Russell, I would prefer if this could go via Rafael's tree. I have a quite big patchset for the PM domain, it would help me if I have a single point to base my work upon. Additionally, you have Tomasz Figa patchset below to consider for the PM domain code. [PATCH v3 0/3] Generic Device Tree based power domain look-up Kind regards Ulf Hansson > >> --- >> drivers/base/power/domain.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c >> index bfb8955c406c..ea91ea0e137b 100644 >> --- a/drivers/base/power/domain.c >> +++ b/drivers/base/power/domain.c >> @@ -42,7 +42,7 @@ >> struct gpd_timing_data *__td = &dev_gpd_data(dev)->td; \ >> if (!__retval && __elapsed > __td->field) { \ >> __td->field = __elapsed; \ >> - dev_warn(dev, name " latency exceeded, new value %lld ns\n", \ >> + dev_dbg(dev, name " latency exceeded, new value %lld ns\n", \ >> __elapsed); \ >> genpd->max_off_time_changed = true; \ >> __td->constraint_changed = true; \ >> @@ -242,7 +242,7 @@ static int __pm_genpd_poweron(struct generic_pm_domain *genpd) >> genpd->max_off_time_changed = true; >> genpd_recalc_cpu_exit_latency(genpd); >> if (genpd->name) >> - pr_warning("%s: Power-on latency exceeded, " >> + pr_debug("%s: Power-on latency exceeded, " >> "new value %lld ns\n", genpd->name, >> elapsed_ns); >> } >> @@ -566,7 +566,7 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd) >> genpd->power_off_latency_ns = elapsed_ns; >> genpd->max_off_time_changed = true; >> if (genpd->name) >> - pr_warning("%s: Power-off latency exceeded, " >> + pr_debug("%s: Power-off latency exceeded, " >> "new value %lld ns\n", genpd->name, >> elapsed_ns); >> } >> > > -- > I speak only for myself. > Rafael J. Wysocki, Intel Open Source Technology Center. > -- > To unsubscribe from this list: send the line "unsubscribe linux-pm" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH 1/5] pm: domains: quieten down generic pm domains 2014-05-02 9:24 ` Ulf Hansson @ 2014-05-04 22:36 ` Rafael J. Wysocki -1 siblings, 0 replies; 49+ messages in thread From: Rafael J. Wysocki @ 2014-05-04 22:36 UTC (permalink / raw) To: Ulf Hansson Cc: Russell King, linux-arm-kernel, Sebastian Hesselbarth, devicetree, linux-pm, Mark Rutland, Pawel Moll, Rob Herring On Friday, May 02, 2014 11:24:45 AM Ulf Hansson wrote: > On 1 May 2014 01:46, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > On Sunday, April 27, 2014 02:28:50 PM Russell King wrote: > >> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > > > > Acked-by: Rafael J. Wysocki <rjw@rjwysocki.net> > > > > Since you need this one and [2/5] for the rest of the patchset, please feel > > free to take them through your tree if that's convenient. > > Unless it complicates things for Russell, I would prefer if this could > go via Rafael's tree. > > I have a quite big patchset for the PM domain, it would help me if I > have a single point to base my work upon. No problem for me, but I'm not sure about what Russell things. > Additionally, you have Tomasz Figa patchset below to consider for the > PM domain code. > > [PATCH v3 0/3] Generic Device Tree based power domain look-up This one seems to have been under discussion recently, however. Thanks! -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 1/5] pm: domains: quieten down generic pm domains @ 2014-05-04 22:36 ` Rafael J. Wysocki 0 siblings, 0 replies; 49+ messages in thread From: Rafael J. Wysocki @ 2014-05-04 22:36 UTC (permalink / raw) To: linux-arm-kernel On Friday, May 02, 2014 11:24:45 AM Ulf Hansson wrote: > On 1 May 2014 01:46, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > On Sunday, April 27, 2014 02:28:50 PM Russell King wrote: > >> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > > > > Acked-by: Rafael J. Wysocki <rjw@rjwysocki.net> > > > > Since you need this one and [2/5] for the rest of the patchset, please feel > > free to take them through your tree if that's convenient. > > Unless it complicates things for Russell, I would prefer if this could > go via Rafael's tree. > > I have a quite big patchset for the PM domain, it would help me if I > have a single point to base my work upon. No problem for me, but I'm not sure about what Russell things. > Additionally, you have Tomasz Figa patchset below to consider for the > PM domain code. > > [PATCH v3 0/3] Generic Device Tree based power domain look-up This one seems to have been under discussion recently, however. Thanks! -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 49+ messages in thread
[parent not found: <1412882.XTDX0QPJ6V-sKB8Sp2ER+y1GS7QM15AGw@public.gmane.org>]
* Re: [PATCH 1/5] pm: domains: quieten down generic pm domains 2014-05-04 22:36 ` Rafael J. Wysocki @ 2015-02-13 11:54 ` Russell King - ARM Linux -1 siblings, 0 replies; 49+ messages in thread From: Russell King - ARM Linux @ 2015-02-13 11:54 UTC (permalink / raw) To: Rafael J. Wysocki, Ulf Hansson Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Sebastian Hesselbarth, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-pm-u79uwXL29TY76Z2rM5mHXA, Mark Rutland, Pawel Moll, Rob Herring On Mon, May 05, 2014 at 12:36:25AM +0200, Rafael J. Wysocki wrote: > On Friday, May 02, 2014 11:24:45 AM Ulf Hansson wrote: > > On 1 May 2014 01:46, Rafael J. Wysocki <rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org> wrote: > > > On Sunday, April 27, 2014 02:28:50 PM Russell King wrote: > > >> Signed-off-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org> > > > > > > Acked-by: Rafael J. Wysocki <rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org> > > > > > > Since you need this one and [2/5] for the rest of the patchset, please feel > > > free to take them through your tree if that's convenient. > > > > Unless it complicates things for Russell, I would prefer if this could > > go via Rafael's tree. > > > > I have a quite big patchset for the PM domain, it would help me if I > > have a single point to base my work upon. > > No problem for me, but I'm not sure about what Russell things. Well, it seems nothing happened with these patches, and they're still something I have (and I've just updated them to changes in 3.19.) What are we going to do about them? -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. -- 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] 49+ messages in thread
* [PATCH 1/5] pm: domains: quieten down generic pm domains @ 2015-02-13 11:54 ` Russell King - ARM Linux 0 siblings, 0 replies; 49+ messages in thread From: Russell King - ARM Linux @ 2015-02-13 11:54 UTC (permalink / raw) To: linux-arm-kernel On Mon, May 05, 2014 at 12:36:25AM +0200, Rafael J. Wysocki wrote: > On Friday, May 02, 2014 11:24:45 AM Ulf Hansson wrote: > > On 1 May 2014 01:46, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > > On Sunday, April 27, 2014 02:28:50 PM Russell King wrote: > > >> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > > > > > > Acked-by: Rafael J. Wysocki <rjw@rjwysocki.net> > > > > > > Since you need this one and [2/5] for the rest of the patchset, please feel > > > free to take them through your tree if that's convenient. > > > > Unless it complicates things for Russell, I would prefer if this could > > go via Rafael's tree. > > > > I have a quite big patchset for the PM domain, it would help me if I > > have a single point to base my work upon. > > No problem for me, but I'm not sure about what Russell things. Well, it seems nothing happened with these patches, and they're still something I have (and I've just updated them to changes in 3.19.) What are we going to do about them? -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 2/5] pm: domains: avoid potential oops in pm_genpd_remove_device() 2014-04-27 13:23 ` Russell King - ARM Linux @ 2014-04-27 13:28 ` Russell King -1 siblings, 0 replies; 49+ messages in thread From: Russell King @ 2014-04-27 13:28 UTC (permalink / raw) To: linux-arm-kernel, Sebastian Hesselbarth Cc: devicetree, linux-pm, Mark Rutland, Pawel Moll, Rafael J. Wysocki, Rob Herring pm_genpd_remove_device() should only be called with valid and present pm domain. There are circumstances where we may end up with something that isn't a generic PM domain in dev->pm_domain (eg, vga_switcheroo stuff.) Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> --- drivers/base/power/domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index ea91ea0e137b..9d8faecc060c 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -1531,7 +1531,7 @@ int pm_genpd_remove_device(struct generic_pm_domain *genpd, dev_dbg(dev, "%s()\n", __func__); - if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev) + if (!pm_genpd_present(genpd) || IS_ERR_OR_NULL(dev) || IS_ERR_OR_NULL(dev->pm_domain) || pd_to_genpd(dev->pm_domain) != genpd) return -EINVAL; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH 2/5] pm: domains: avoid potential oops in pm_genpd_remove_device() @ 2014-04-27 13:28 ` Russell King 0 siblings, 0 replies; 49+ messages in thread From: Russell King @ 2014-04-27 13:28 UTC (permalink / raw) To: linux-arm-kernel pm_genpd_remove_device() should only be called with valid and present pm domain. There are circumstances where we may end up with something that isn't a generic PM domain in dev->pm_domain (eg, vga_switcheroo stuff.) Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> --- drivers/base/power/domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index ea91ea0e137b..9d8faecc060c 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -1531,7 +1531,7 @@ int pm_genpd_remove_device(struct generic_pm_domain *genpd, dev_dbg(dev, "%s()\n", __func__); - if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev) + if (!pm_genpd_present(genpd) || IS_ERR_OR_NULL(dev) || IS_ERR_OR_NULL(dev->pm_domain) || pd_to_genpd(dev->pm_domain) != genpd) return -EINVAL; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 2/5] pm: domains: avoid potential oops in pm_genpd_remove_device() 2014-04-27 13:28 ` Russell King @ 2014-04-30 23:46 ` Rafael J. Wysocki -1 siblings, 0 replies; 49+ messages in thread From: Rafael J. Wysocki @ 2014-04-30 23:46 UTC (permalink / raw) To: Russell King Cc: linux-arm-kernel, Sebastian Hesselbarth, devicetree, linux-pm, Mark Rutland, Pawel Moll, Rob Herring On Sunday, April 27, 2014 02:28:55 PM Russell King wrote: > pm_genpd_remove_device() should only be called with valid and present > pm domain. There are circumstances where we may end up with something > that isn't a generic PM domain in dev->pm_domain (eg, vga_switcheroo > stuff.) > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> Acked-by: Rafael J. Wysocki <rjw@rjwysocki.net> > --- > drivers/base/power/domain.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index ea91ea0e137b..9d8faecc060c 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -1531,7 +1531,7 @@ int pm_genpd_remove_device(struct generic_pm_domain *genpd, > > dev_dbg(dev, "%s()\n", __func__); > > - if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev) > + if (!pm_genpd_present(genpd) || IS_ERR_OR_NULL(dev) > || IS_ERR_OR_NULL(dev->pm_domain) > || pd_to_genpd(dev->pm_domain) != genpd) > return -EINVAL; > -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 2/5] pm: domains: avoid potential oops in pm_genpd_remove_device() @ 2014-04-30 23:46 ` Rafael J. Wysocki 0 siblings, 0 replies; 49+ messages in thread From: Rafael J. Wysocki @ 2014-04-30 23:46 UTC (permalink / raw) To: linux-arm-kernel On Sunday, April 27, 2014 02:28:55 PM Russell King wrote: > pm_genpd_remove_device() should only be called with valid and present > pm domain. There are circumstances where we may end up with something > that isn't a generic PM domain in dev->pm_domain (eg, vga_switcheroo > stuff.) > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> Acked-by: Rafael J. Wysocki <rjw@rjwysocki.net> > --- > drivers/base/power/domain.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index ea91ea0e137b..9d8faecc060c 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -1531,7 +1531,7 @@ int pm_genpd_remove_device(struct generic_pm_domain *genpd, > > dev_dbg(dev, "%s()\n", __func__); > > - if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev) > + if (!pm_genpd_present(genpd) || IS_ERR_OR_NULL(dev) > || IS_ERR_OR_NULL(dev->pm_domain) > || pd_to_genpd(dev->pm_domain) != genpd) > return -EINVAL; > -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 3/5] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets 2014-04-27 13:23 ` Russell King - ARM Linux @ 2014-04-27 13:29 ` Russell King -1 siblings, 0 replies; 49+ messages in thread From: Russell King @ 2014-04-27 13:29 UTC (permalink / raw) To: linux-arm-kernel, Sebastian Hesselbarth Cc: devicetree, linux-pm, Mark Rutland, Pawel Moll, Rafael J. Wysocki, Rob Herring The PMU device contains an interrupt controller, power control and resets. The interrupt controller is a little sub-standard in that there is no race free way to clear down pending interrupts, so we try to avoid problems by reducing the window as much as possible, and clearing as infrequently as possible. The interrupt support is implemented using an IRQ domain, and the parent interrupt referenced in the standard DT way. The power domains and reset support is closely related - there is a defined sequence for powering down a domain which is tightly coupled with asserting the reset. Hence, it makes sense to group these two together. This patch adds the core PMU driver: power domains must be defined in the DT file in order to make use of them. The reset controller can be referenced in the standard way for reset controllers. Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> --- arch/arm/Kconfig | 1 + arch/arm/mach-dove/Makefile | 1 + arch/arm/mach-dove/common.c | 2 + arch/arm/mach-dove/common.h | 1 + arch/arm/mach-dove/include/mach/pm.h | 17 -- arch/arm/mach-dove/irq.c | 87 ------- arch/arm/mach-dove/pmu.c | 457 +++++++++++++++++++++++++++++++++++ 7 files changed, 462 insertions(+), 104 deletions(-) create mode 100644 arch/arm/mach-dove/pmu.c diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 15949459611f..cec3ff2dfad4 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -534,6 +534,7 @@ config ARCH_DOVE select PINCTRL select PINCTRL_DOVE select PLAT_ORION_LEGACY + select PM_GENERIC_DOMAINS if PM select USB_ARCH_HAS_EHCI help Support for the Marvell Dove SoC 88AP510 diff --git a/arch/arm/mach-dove/Makefile b/arch/arm/mach-dove/Makefile index cbc5c0618788..8e59c57dfa3c 100644 --- a/arch/arm/mach-dove/Makefile +++ b/arch/arm/mach-dove/Makefile @@ -1,4 +1,5 @@ obj-y += common.o +obj-$(CONFIG_PM_GENERIC_DOMAINS)+= pmu.o obj-$(CONFIG_DOVE_LEGACY) += irq.o mpp.o obj-$(CONFIG_PCI) += pcie.o obj-$(CONFIG_MACH_DOVE_DB) += dove-db-setup.o diff --git a/arch/arm/mach-dove/common.c b/arch/arm/mach-dove/common.c index 0d1a89298ece..195871c87819 100644 --- a/arch/arm/mach-dove/common.c +++ b/arch/arm/mach-dove/common.c @@ -377,6 +377,8 @@ void __init dove_setup_cpu_wins(void) void __init dove_init(void) { + dove_init_pmu(); + pr_info("Dove 88AP510 SoC, TCLK = %d MHz.\n", (dove_tclk + 499999) / 1000000); diff --git a/arch/arm/mach-dove/common.h b/arch/arm/mach-dove/common.h index 1d725224d146..261e0e995daa 100644 --- a/arch/arm/mach-dove/common.h +++ b/arch/arm/mach-dove/common.h @@ -45,5 +45,6 @@ void dove_i2c_init(void); void dove_sdio0_init(void); void dove_sdio1_init(void); void dove_restart(enum reboot_mode, const char *); +int dove_init_pmu(void); #endif diff --git a/arch/arm/mach-dove/include/mach/pm.h b/arch/arm/mach-dove/include/mach/pm.h index b47f75038686..625a89c15c1f 100644 --- a/arch/arm/mach-dove/include/mach/pm.h +++ b/arch/arm/mach-dove/include/mach/pm.h @@ -51,22 +51,5 @@ #define CLOCK_GATING_GIGA_PHY_MASK (1 << CLOCK_GATING_BIT_GIGA_PHY) #define PMU_INTERRUPT_CAUSE (DOVE_PMU_VIRT_BASE + 0x50) -#define PMU_INTERRUPT_MASK (DOVE_PMU_VIRT_BASE + 0x54) - -static inline int pmu_to_irq(int pin) -{ - if (pin < NR_PMU_IRQS) - return pin + IRQ_DOVE_PMU_START; - - return -EINVAL; -} - -static inline int irq_to_pmu(int irq) -{ - if (IRQ_DOVE_PMU_START <= irq && irq < NR_IRQS) - return irq - IRQ_DOVE_PMU_START; - - return -EINVAL; -} #endif diff --git a/arch/arm/mach-dove/irq.c b/arch/arm/mach-dove/irq.c index bc4344aa1009..ca14d45a699b 100644 --- a/arch/arm/mach-dove/irq.c +++ b/arch/arm/mach-dove/irq.c @@ -7,86 +7,14 @@ * License version 2. This program is licensed "as is" without any * warranty of any kind, whether express or implied. */ - -#include <linux/kernel.h> #include <linux/init.h> #include <linux/irq.h> -#include <linux/gpio.h> #include <linux/io.h> -#include <asm/mach/arch.h> #include <plat/irq.h> -#include <asm/mach/irq.h> -#include <mach/pm.h> #include <mach/bridge-regs.h> #include <plat/orion-gpio.h> #include "common.h" -static void pmu_irq_mask(struct irq_data *d) -{ - int pin = irq_to_pmu(d->irq); - u32 u; - - u = readl(PMU_INTERRUPT_MASK); - u &= ~(1 << (pin & 31)); - writel(u, PMU_INTERRUPT_MASK); -} - -static void pmu_irq_unmask(struct irq_data *d) -{ - int pin = irq_to_pmu(d->irq); - u32 u; - - u = readl(PMU_INTERRUPT_MASK); - u |= 1 << (pin & 31); - writel(u, PMU_INTERRUPT_MASK); -} - -static void pmu_irq_ack(struct irq_data *d) -{ - int pin = irq_to_pmu(d->irq); - u32 u; - - /* - * The PMU mask register is not RW0C: it is RW. This means that - * the bits take whatever value is written to them; if you write - * a '1', you will set the interrupt. - * - * Unfortunately this means there is NO race free way to clear - * these interrupts. - * - * So, let's structure the code so that the window is as small as - * possible. - */ - u = ~(1 << (pin & 31)); - u &= readl_relaxed(PMU_INTERRUPT_CAUSE); - writel_relaxed(u, PMU_INTERRUPT_CAUSE); -} - -static struct irq_chip pmu_irq_chip = { - .name = "pmu_irq", - .irq_mask = pmu_irq_mask, - .irq_unmask = pmu_irq_unmask, - .irq_ack = pmu_irq_ack, -}; - -static void pmu_irq_handler(unsigned int irq, struct irq_desc *desc) -{ - unsigned long cause = readl(PMU_INTERRUPT_CAUSE); - - cause &= readl(PMU_INTERRUPT_MASK); - if (cause == 0) { - do_bad_IRQ(irq, desc); - return; - } - - for (irq = 0; irq < NR_PMU_IRQS; irq++) { - if (!(cause & (1 << irq))) - continue; - irq = pmu_to_irq(irq); - generic_handle_irq(irq); - } -} - static int __initdata gpio0_irqs[4] = { IRQ_DOVE_GPIO_0_7, IRQ_DOVE_GPIO_8_15, @@ -110,8 +38,6 @@ static int __initdata gpio2_irqs[4] = { void __init dove_init_irq(void) { - int i; - orion_irq_init(0, IRQ_VIRT_BASE + IRQ_MASK_LOW_OFF); orion_irq_init(32, IRQ_VIRT_BASE + IRQ_MASK_HIGH_OFF); @@ -126,17 +52,4 @@ void __init dove_init_irq(void) orion_gpio_init(NULL, 64, 8, DOVE_GPIO2_VIRT_BASE, 0, IRQ_DOVE_GPIO_START + 64, gpio2_irqs); - - /* - * Mask and clear PMU interrupts - */ - writel(0, PMU_INTERRUPT_MASK); - writel(0, PMU_INTERRUPT_CAUSE); - - for (i = IRQ_DOVE_PMU_START; i < NR_IRQS; i++) { - irq_set_chip_and_handler(i, &pmu_irq_chip, handle_level_irq); - irq_set_status_flags(i, IRQ_LEVEL); - set_irq_flags(i, IRQF_VALID); - } - irq_set_chained_handler(IRQ_DOVE_PMU, pmu_irq_handler); } diff --git a/arch/arm/mach-dove/pmu.c b/arch/arm/mach-dove/pmu.c new file mode 100644 index 000000000000..0b3201fa2d5c --- /dev/null +++ b/arch/arm/mach-dove/pmu.c @@ -0,0 +1,457 @@ +/* + * Marvell Dove PMU support + */ +#include <linux/io.h> +#include <linux/irq.h> +#include <linux/irqdomain.h> +#include <linux/of.h> +#include <linux/of_irq.h> +#include <linux/of_address.h> +#include <linux/platform_device.h> +#include <linux/pm_domain.h> +#include <linux/reset.h> +#include <linux/reset-controller.h> +#include <linux/sched.h> +#include <linux/slab.h> +#include <linux/spinlock.h> + +#include <asm/mach/irq.h> + +#include <mach/hardware.h> +#include <mach/pm.h> + +#define PMC_SW_RST 0x30 +#define PMC_IRQ_CAUSE 0x50 +#define PMC_IRQ_MASK 0x54 + +#define PMU_PWR 0x10 +#define PMU_PWR_DOWN_GPU BIT(2) +#define PMU_PWR_DOWN_VPU BIT(3) +#define PMU_ISO 0x58 +#define PMU_ISO_VPU BIT(0) +#define PMU_ISO_GPU BIT(1) +#define PMU_ISO_CPU BIT(2) +#define PMU_ISO_CORE BIT(3) + +struct pmu_data { + spinlock_t lock; + struct device_node *of_node; + void __iomem *pmc_base; + void __iomem *pmu_base; + struct irq_chip_generic *irq_gc; + struct irq_domain *irq_domain; +#ifdef CONFIG_RESET_CONTROLLER + struct reset_controller_dev reset; +#endif +}; + +/* + * The PMU contains a register to reset various subsystems within the + * SoC. Export this as a reset controller. + */ +#ifdef CONFIG_RESET_CONTROLLER +#define rcdev_to_pmu(rcdev) container_of(rcdev, struct pmu_data, reset) + +static int pmu_reset_reset(struct reset_controller_dev *rc, unsigned long id) +{ + struct pmu_data *pmu = rcdev_to_pmu(rc); + unsigned long flags; + u32 val; + + spin_lock_irqsave(&pmu->lock, flags); + val = readl_relaxed(pmu->pmc_base + PMC_SW_RST); + writel_relaxed(val & ~BIT(id), pmu->pmc_base + PMC_SW_RST); + writel_relaxed(val | BIT(id), pmu->pmc_base + PMC_SW_RST); + spin_unlock_irqrestore(&pmu->lock, flags); + + return 0; +} + +static int pmu_reset_assert(struct reset_controller_dev *rc, unsigned long id) +{ + struct pmu_data *pmu = rcdev_to_pmu(rc); + unsigned long flags; + u32 val = ~BIT(id); + + spin_lock_irqsave(&pmu->lock, flags); + val &= readl_relaxed(pmu->pmc_base + PMC_SW_RST); + writel_relaxed(val, pmu->pmc_base + PMC_SW_RST); + spin_unlock_irqrestore(&pmu->lock, flags); + + return 0; +} + +static int pmu_reset_deassert(struct reset_controller_dev *rc, unsigned long id) +{ + struct pmu_data *pmu = rcdev_to_pmu(rc); + unsigned long flags; + u32 val = BIT(id); + + spin_lock_irqsave(&pmu->lock, flags); + val |= readl_relaxed(pmu->pmc_base + PMC_SW_RST); + writel_relaxed(val, pmu->pmc_base + PMC_SW_RST); + spin_unlock_irqrestore(&pmu->lock, flags); + + return 0; +} + +static struct reset_control_ops pmu_reset_ops = { + .reset = pmu_reset_reset, + .assert = pmu_reset_assert, + .deassert = pmu_reset_deassert, +}; + +static struct reset_controller_dev pmu_reset __initdata = { + .ops = &pmu_reset_ops, + .owner = THIS_MODULE, + .nr_resets = 32, +}; + +static void __init pmu_reset_init(struct pmu_data *pmu) +{ + int ret; + + pmu->reset = pmu_reset; + pmu->reset.of_node = pmu->of_node; + + ret = reset_controller_register(&pmu->reset); + if (ret) + pr_err("pmu: %s failed: %d\n", "reset_controller_register", ret); +} +#else +static void __init pmu_reset_init(struct pmu_data *pmu) +{ +} +#endif + +struct pmu_domain { + struct pmu_data *pmu; + u32 pwr_mask; + u32 rst_mask; + u32 iso_mask; + struct generic_pm_domain base; +}; + +#define to_pmu_domain(dom) container_of(dom, struct pmu_domain, base) + +/* + * This deals with the "old" Marvell sequence of bringing a power domain + * down/up, which is: apply power, release reset, disable isolators. + * + * Later devices apparantly use a different sequence: power up, disable + * isolators, assert repair signal, enable SRMA clock, enable AXI clock, + * enable module clock, deassert reset. + * + * Note: reading the assembly, it seems that the IO accessors have an + * unfortunate side-effect - they cause memory already read into registers + * for the if () to be re-read for the bit-set or bit-clear operation. + * The code is written to avoid this. + */ +static int pmu_domain_power_off(struct generic_pm_domain *domain) +{ + struct pmu_domain *pmu_dom = to_pmu_domain(domain); + struct pmu_data *pmu = pmu_dom->pmu; + unsigned long flags; + unsigned int val; + void __iomem *pmu_base = pmu->pmu_base; + void __iomem *pmc_base = pmu->pmc_base; + + spin_lock_irqsave(&pmu->lock, flags); + + /* Enable isolators */ + if (pmu_dom->iso_mask) { + val = ~pmu_dom->iso_mask; + val &= readl_relaxed(pmu_base + PMU_ISO); + writel_relaxed(val, pmu_base + PMU_ISO); + } + + /* Reset unit */ + if (pmu_dom->rst_mask) { + val = ~pmu_dom->rst_mask; + val &= readl_relaxed(pmc_base + PMC_SW_RST); + writel_relaxed(val, pmc_base + PMC_SW_RST); + } + + /* Power down */ + val = readl_relaxed(pmu_base + PMU_PWR) | pmu_dom->pwr_mask; + writel_relaxed(val, pmu_base + PMU_PWR); + + spin_unlock_irqrestore(&pmu->lock, flags); + + return 0; +} + +static int pmu_domain_power_on(struct generic_pm_domain *domain) +{ + struct pmu_domain *pmu_dom = to_pmu_domain(domain); + struct pmu_data *pmu = pmu_dom->pmu; + unsigned long flags; + unsigned int val; + void __iomem *pmu_base = pmu->pmu_base; + void __iomem *pmc_base = pmu->pmc_base; + + spin_lock_irqsave(&pmu->lock, flags); + + /* Power on */ + val = ~pmu_dom->pwr_mask & readl_relaxed(pmu_base + PMU_PWR); + writel_relaxed(val, pmu_base + PMU_PWR); + + /* Release reset */ + if (pmu_dom->rst_mask) { + val = pmu_dom->rst_mask; + val |= readl_relaxed(pmc_base + PMC_SW_RST); + writel_relaxed(val, pmc_base + PMC_SW_RST); + } + + /* Disable isolators */ + if (pmu_dom->iso_mask) { + val = pmu_dom->iso_mask; + val |= readl_relaxed(pmu_base + PMU_ISO); + writel_relaxed(val, pmu_base + PMU_ISO); + } + + spin_unlock_irqrestore(&pmu->lock, flags); + + return 0; +} + +static void __pmu_domain_register(struct pmu_domain *domain) +{ + unsigned int val = readl_relaxed(domain->pmu->pmu_base + PMU_PWR); + + domain->base.dev_irq_safe = true; + domain->base.power_off = pmu_domain_power_off; + domain->base.power_on = pmu_domain_power_on; + + pm_genpd_init(&domain->base, NULL, !(val & domain->pwr_mask)); +} + +static void pmu_add_genpd_of(struct device *dev) +{ + struct device_node *node; + + node = of_parse_phandle(dev->of_node, "marvell,power-domain", 0); + if (!node) + return; + + while (1) { + if (pm_genpd_of_add_device(node, dev) != -EAGAIN) + break; + cond_resched(); + } +} + +static void pmu_remove_genpd(struct device *dev) +{ + struct generic_pm_domain *genpd = dev_to_genpd(dev); + + while (1) { + if (pm_genpd_remove_device(genpd, dev) != -EAGAIN) + break; + cond_resched(); + } +} + +static int pmu_platform_call(struct notifier_block *nb, + unsigned long event, void *data) +{ + struct device *dev = data; + + switch (event) { + case BUS_NOTIFY_ADD_DEVICE: + if (dev->of_node) + pmu_add_genpd_of(dev); + break; + + case BUS_NOTIFY_DEL_DEVICE: + pmu_remove_genpd(dev); + break; + } + return NOTIFY_OK; +} + +static struct notifier_block platform_nb = { + .notifier_call = pmu_platform_call, +}; + +/* PMU IRQ controller */ +static void pmu_irq_handler(unsigned int irq, struct irq_desc *desc) +{ + struct pmu_data *pmu = irq_get_handler_data(irq); + struct irq_chip_generic *gc = pmu->irq_gc; + struct irq_domain *domain = pmu->irq_domain; + void __iomem *base = gc->reg_base; + u32 stat = readl_relaxed(base + PMC_IRQ_CAUSE) & gc->mask_cache; + u32 done = ~0; + + if (stat == 0) { + do_bad_IRQ(irq, desc); + return; + } + + while (stat) { + u32 hwirq = fls(stat) - 1; + + stat &= ~(1 << hwirq); + done &= ~(1 << hwirq); + + generic_handle_irq(irq_find_mapping(domain, hwirq)); + } + + /* + * The PMU mask register is not RW0C: it is RW. This means that + * the bits take whatever value is written to them; if you write + * a '1', you will set the interrupt. + * + * Unfortunately this means there is NO race free way to clear + * these interrupts. + * + * So, let's structure the code so that the window is as small as + * possible. + */ + irq_gc_lock(gc); + done &= readl_relaxed(base + PMC_IRQ_CAUSE); + writel_relaxed(done, base + PMC_IRQ_CAUSE); + irq_gc_unlock(gc); +} + +static int __init dove_init_pmu_irq(struct pmu_data *pmu, int irq) +{ + const char *name = "pmu_irq"; + struct irq_chip_generic *gc; + struct irq_domain *domain; + int ret; + + /* mask and clear all interrupts */ + writel(0, pmu->pmc_base + PMC_IRQ_MASK); + writel(0, pmu->pmc_base + PMC_IRQ_CAUSE); + + domain = irq_domain_add_linear(pmu->of_node, NR_PMU_IRQS, + &irq_generic_chip_ops, NULL); + if (!domain) { + pr_err("%s: unable to add irq domain\n", name); + return -ENOMEM; + } + + ret = irq_alloc_domain_generic_chips(domain, NR_PMU_IRQS, 1, name, + handle_level_irq, + IRQ_NOREQUEST | IRQ_NOPROBE, 0, + IRQ_GC_INIT_MASK_CACHE); + if (ret) { + pr_err("%s: unable to alloc irq domain gc: %d\n", name, ret); + irq_domain_remove(domain); + return ret; + } + + gc = irq_get_domain_generic_chip(domain, 0); + gc->reg_base = pmu->pmc_base; + gc->chip_types[0].regs.mask = PMC_IRQ_MASK; + gc->chip_types[0].chip.irq_mask = irq_gc_mask_clr_bit; + gc->chip_types[0].chip.irq_unmask = irq_gc_mask_set_bit; + + pmu->irq_domain = domain; + pmu->irq_gc = gc; + + /* If no of_node, populate the domain */ + if (!pmu->of_node) + irq_domain_associate_many(pmu->irq_domain, IRQ_DOVE_PMU_START, + 0, NR_PMU_IRQS); + + irq_set_handler_data(irq, pmu); + irq_set_chained_handler(irq, pmu_irq_handler); + + return 0; +} + +/* + * pmu { + * compatible = "marvell,pmu"; + * reg = <0xd0000 0x8000> <0xd8000 0x8000>; + * interrupts = <33>; + * #reset-cells = 1; + * vpu_domain: vpu-domain { + * marvell,pmu_pwr_mask = <0x00000008>; + * marvell,pmu_iso_mask = <0x00000001>; + * resets = <&pmu 16>; + * }; + * gpu_domain: gpu-domain { + * marvell,pmu_pwr_mask = <0x00000004>; + * marvell,pmu_iso_mask = <0x00000002>; + * resets = <&pmu 18>; + * }; + * }; + */ +int __init dove_init_pmu(void) +{ + struct device_node *np_pmu, *np; + struct pmu_data *pmu; + int ret, parent_irq; + + /* Lookup the PMU node */ + np_pmu = of_find_compatible_node(NULL, NULL, "marvell,pmu"); + if (!np_pmu) + return 0; + + pmu = kzalloc(sizeof(*pmu), GFP_KERNEL); + if (!pmu) + return -ENOMEM; + + spin_lock_init(&pmu->lock); + pmu->of_node = np_pmu; + pmu->pmc_base = of_iomap(pmu->of_node, 0); + pmu->pmu_base = of_iomap(pmu->of_node, 1); + if (!pmu->pmc_base || !pmu->pmu_base) { + pr_err("%s: failed to map PMU\n", np_pmu->name); + iounmap(pmu->pmu_base); + iounmap(pmu->pmc_base); + kfree(pmu); + return -ENOMEM; + } + + parent_irq = irq_of_parse_and_map(pmu->of_node, 0); + if (!parent_irq) + pr_err("%s: no interrupt specified\n", np_pmu->name); + + pmu_reset_init(pmu); + + for_each_available_child_of_node(pmu->of_node, np) { + struct of_phandle_args args; + struct pmu_domain *domain; + + domain = kzalloc(sizeof(*domain), GFP_KERNEL); + if (!domain) + break; + + domain->pmu = pmu; + domain->base.of_node = np; + domain->base.name = kstrdup(np->name, GFP_KERNEL); + if (!domain->base.name) { + kfree(domain); + break; + } + + of_property_read_u32(np, "marvell,pmu_pwr_mask", + &domain->pwr_mask); + of_property_read_u32(np, "marvell,pmu_iso_mask", + &domain->iso_mask); + + ret = of_parse_phandle_with_args(np, "resets", "#reset-cells", + 0, &args); + if (ret == 0) { + if (args.np == pmu->of_node) + domain->rst_mask = BIT(args.args[0]); + of_node_put(args.np); + } + + __pmu_domain_register(domain); + } + pm_genpd_poweroff_unused(); + + ret = dove_init_pmu_irq(pmu, parent_irq); + if (ret) + pr_err("dove_init_pmu_irq() failed: %d\n", ret); + + bus_register_notifier(&platform_bus_type, &platform_nb); + + return 0; +} -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH 3/5] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets @ 2014-04-27 13:29 ` Russell King 0 siblings, 0 replies; 49+ messages in thread From: Russell King @ 2014-04-27 13:29 UTC (permalink / raw) To: linux-arm-kernel The PMU device contains an interrupt controller, power control and resets. The interrupt controller is a little sub-standard in that there is no race free way to clear down pending interrupts, so we try to avoid problems by reducing the window as much as possible, and clearing as infrequently as possible. The interrupt support is implemented using an IRQ domain, and the parent interrupt referenced in the standard DT way. The power domains and reset support is closely related - there is a defined sequence for powering down a domain which is tightly coupled with asserting the reset. Hence, it makes sense to group these two together. This patch adds the core PMU driver: power domains must be defined in the DT file in order to make use of them. The reset controller can be referenced in the standard way for reset controllers. Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> --- arch/arm/Kconfig | 1 + arch/arm/mach-dove/Makefile | 1 + arch/arm/mach-dove/common.c | 2 + arch/arm/mach-dove/common.h | 1 + arch/arm/mach-dove/include/mach/pm.h | 17 -- arch/arm/mach-dove/irq.c | 87 ------- arch/arm/mach-dove/pmu.c | 457 +++++++++++++++++++++++++++++++++++ 7 files changed, 462 insertions(+), 104 deletions(-) create mode 100644 arch/arm/mach-dove/pmu.c diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 15949459611f..cec3ff2dfad4 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -534,6 +534,7 @@ config ARCH_DOVE select PINCTRL select PINCTRL_DOVE select PLAT_ORION_LEGACY + select PM_GENERIC_DOMAINS if PM select USB_ARCH_HAS_EHCI help Support for the Marvell Dove SoC 88AP510 diff --git a/arch/arm/mach-dove/Makefile b/arch/arm/mach-dove/Makefile index cbc5c0618788..8e59c57dfa3c 100644 --- a/arch/arm/mach-dove/Makefile +++ b/arch/arm/mach-dove/Makefile @@ -1,4 +1,5 @@ obj-y += common.o +obj-$(CONFIG_PM_GENERIC_DOMAINS)+= pmu.o obj-$(CONFIG_DOVE_LEGACY) += irq.o mpp.o obj-$(CONFIG_PCI) += pcie.o obj-$(CONFIG_MACH_DOVE_DB) += dove-db-setup.o diff --git a/arch/arm/mach-dove/common.c b/arch/arm/mach-dove/common.c index 0d1a89298ece..195871c87819 100644 --- a/arch/arm/mach-dove/common.c +++ b/arch/arm/mach-dove/common.c @@ -377,6 +377,8 @@ void __init dove_setup_cpu_wins(void) void __init dove_init(void) { + dove_init_pmu(); + pr_info("Dove 88AP510 SoC, TCLK = %d MHz.\n", (dove_tclk + 499999) / 1000000); diff --git a/arch/arm/mach-dove/common.h b/arch/arm/mach-dove/common.h index 1d725224d146..261e0e995daa 100644 --- a/arch/arm/mach-dove/common.h +++ b/arch/arm/mach-dove/common.h @@ -45,5 +45,6 @@ void dove_i2c_init(void); void dove_sdio0_init(void); void dove_sdio1_init(void); void dove_restart(enum reboot_mode, const char *); +int dove_init_pmu(void); #endif diff --git a/arch/arm/mach-dove/include/mach/pm.h b/arch/arm/mach-dove/include/mach/pm.h index b47f75038686..625a89c15c1f 100644 --- a/arch/arm/mach-dove/include/mach/pm.h +++ b/arch/arm/mach-dove/include/mach/pm.h @@ -51,22 +51,5 @@ #define CLOCK_GATING_GIGA_PHY_MASK (1 << CLOCK_GATING_BIT_GIGA_PHY) #define PMU_INTERRUPT_CAUSE (DOVE_PMU_VIRT_BASE + 0x50) -#define PMU_INTERRUPT_MASK (DOVE_PMU_VIRT_BASE + 0x54) - -static inline int pmu_to_irq(int pin) -{ - if (pin < NR_PMU_IRQS) - return pin + IRQ_DOVE_PMU_START; - - return -EINVAL; -} - -static inline int irq_to_pmu(int irq) -{ - if (IRQ_DOVE_PMU_START <= irq && irq < NR_IRQS) - return irq - IRQ_DOVE_PMU_START; - - return -EINVAL; -} #endif diff --git a/arch/arm/mach-dove/irq.c b/arch/arm/mach-dove/irq.c index bc4344aa1009..ca14d45a699b 100644 --- a/arch/arm/mach-dove/irq.c +++ b/arch/arm/mach-dove/irq.c @@ -7,86 +7,14 @@ * License version 2. This program is licensed "as is" without any * warranty of any kind, whether express or implied. */ - -#include <linux/kernel.h> #include <linux/init.h> #include <linux/irq.h> -#include <linux/gpio.h> #include <linux/io.h> -#include <asm/mach/arch.h> #include <plat/irq.h> -#include <asm/mach/irq.h> -#include <mach/pm.h> #include <mach/bridge-regs.h> #include <plat/orion-gpio.h> #include "common.h" -static void pmu_irq_mask(struct irq_data *d) -{ - int pin = irq_to_pmu(d->irq); - u32 u; - - u = readl(PMU_INTERRUPT_MASK); - u &= ~(1 << (pin & 31)); - writel(u, PMU_INTERRUPT_MASK); -} - -static void pmu_irq_unmask(struct irq_data *d) -{ - int pin = irq_to_pmu(d->irq); - u32 u; - - u = readl(PMU_INTERRUPT_MASK); - u |= 1 << (pin & 31); - writel(u, PMU_INTERRUPT_MASK); -} - -static void pmu_irq_ack(struct irq_data *d) -{ - int pin = irq_to_pmu(d->irq); - u32 u; - - /* - * The PMU mask register is not RW0C: it is RW. This means that - * the bits take whatever value is written to them; if you write - * a '1', you will set the interrupt. - * - * Unfortunately this means there is NO race free way to clear - * these interrupts. - * - * So, let's structure the code so that the window is as small as - * possible. - */ - u = ~(1 << (pin & 31)); - u &= readl_relaxed(PMU_INTERRUPT_CAUSE); - writel_relaxed(u, PMU_INTERRUPT_CAUSE); -} - -static struct irq_chip pmu_irq_chip = { - .name = "pmu_irq", - .irq_mask = pmu_irq_mask, - .irq_unmask = pmu_irq_unmask, - .irq_ack = pmu_irq_ack, -}; - -static void pmu_irq_handler(unsigned int irq, struct irq_desc *desc) -{ - unsigned long cause = readl(PMU_INTERRUPT_CAUSE); - - cause &= readl(PMU_INTERRUPT_MASK); - if (cause == 0) { - do_bad_IRQ(irq, desc); - return; - } - - for (irq = 0; irq < NR_PMU_IRQS; irq++) { - if (!(cause & (1 << irq))) - continue; - irq = pmu_to_irq(irq); - generic_handle_irq(irq); - } -} - static int __initdata gpio0_irqs[4] = { IRQ_DOVE_GPIO_0_7, IRQ_DOVE_GPIO_8_15, @@ -110,8 +38,6 @@ static int __initdata gpio2_irqs[4] = { void __init dove_init_irq(void) { - int i; - orion_irq_init(0, IRQ_VIRT_BASE + IRQ_MASK_LOW_OFF); orion_irq_init(32, IRQ_VIRT_BASE + IRQ_MASK_HIGH_OFF); @@ -126,17 +52,4 @@ void __init dove_init_irq(void) orion_gpio_init(NULL, 64, 8, DOVE_GPIO2_VIRT_BASE, 0, IRQ_DOVE_GPIO_START + 64, gpio2_irqs); - - /* - * Mask and clear PMU interrupts - */ - writel(0, PMU_INTERRUPT_MASK); - writel(0, PMU_INTERRUPT_CAUSE); - - for (i = IRQ_DOVE_PMU_START; i < NR_IRQS; i++) { - irq_set_chip_and_handler(i, &pmu_irq_chip, handle_level_irq); - irq_set_status_flags(i, IRQ_LEVEL); - set_irq_flags(i, IRQF_VALID); - } - irq_set_chained_handler(IRQ_DOVE_PMU, pmu_irq_handler); } diff --git a/arch/arm/mach-dove/pmu.c b/arch/arm/mach-dove/pmu.c new file mode 100644 index 000000000000..0b3201fa2d5c --- /dev/null +++ b/arch/arm/mach-dove/pmu.c @@ -0,0 +1,457 @@ +/* + * Marvell Dove PMU support + */ +#include <linux/io.h> +#include <linux/irq.h> +#include <linux/irqdomain.h> +#include <linux/of.h> +#include <linux/of_irq.h> +#include <linux/of_address.h> +#include <linux/platform_device.h> +#include <linux/pm_domain.h> +#include <linux/reset.h> +#include <linux/reset-controller.h> +#include <linux/sched.h> +#include <linux/slab.h> +#include <linux/spinlock.h> + +#include <asm/mach/irq.h> + +#include <mach/hardware.h> +#include <mach/pm.h> + +#define PMC_SW_RST 0x30 +#define PMC_IRQ_CAUSE 0x50 +#define PMC_IRQ_MASK 0x54 + +#define PMU_PWR 0x10 +#define PMU_PWR_DOWN_GPU BIT(2) +#define PMU_PWR_DOWN_VPU BIT(3) +#define PMU_ISO 0x58 +#define PMU_ISO_VPU BIT(0) +#define PMU_ISO_GPU BIT(1) +#define PMU_ISO_CPU BIT(2) +#define PMU_ISO_CORE BIT(3) + +struct pmu_data { + spinlock_t lock; + struct device_node *of_node; + void __iomem *pmc_base; + void __iomem *pmu_base; + struct irq_chip_generic *irq_gc; + struct irq_domain *irq_domain; +#ifdef CONFIG_RESET_CONTROLLER + struct reset_controller_dev reset; +#endif +}; + +/* + * The PMU contains a register to reset various subsystems within the + * SoC. Export this as a reset controller. + */ +#ifdef CONFIG_RESET_CONTROLLER +#define rcdev_to_pmu(rcdev) container_of(rcdev, struct pmu_data, reset) + +static int pmu_reset_reset(struct reset_controller_dev *rc, unsigned long id) +{ + struct pmu_data *pmu = rcdev_to_pmu(rc); + unsigned long flags; + u32 val; + + spin_lock_irqsave(&pmu->lock, flags); + val = readl_relaxed(pmu->pmc_base + PMC_SW_RST); + writel_relaxed(val & ~BIT(id), pmu->pmc_base + PMC_SW_RST); + writel_relaxed(val | BIT(id), pmu->pmc_base + PMC_SW_RST); + spin_unlock_irqrestore(&pmu->lock, flags); + + return 0; +} + +static int pmu_reset_assert(struct reset_controller_dev *rc, unsigned long id) +{ + struct pmu_data *pmu = rcdev_to_pmu(rc); + unsigned long flags; + u32 val = ~BIT(id); + + spin_lock_irqsave(&pmu->lock, flags); + val &= readl_relaxed(pmu->pmc_base + PMC_SW_RST); + writel_relaxed(val, pmu->pmc_base + PMC_SW_RST); + spin_unlock_irqrestore(&pmu->lock, flags); + + return 0; +} + +static int pmu_reset_deassert(struct reset_controller_dev *rc, unsigned long id) +{ + struct pmu_data *pmu = rcdev_to_pmu(rc); + unsigned long flags; + u32 val = BIT(id); + + spin_lock_irqsave(&pmu->lock, flags); + val |= readl_relaxed(pmu->pmc_base + PMC_SW_RST); + writel_relaxed(val, pmu->pmc_base + PMC_SW_RST); + spin_unlock_irqrestore(&pmu->lock, flags); + + return 0; +} + +static struct reset_control_ops pmu_reset_ops = { + .reset = pmu_reset_reset, + .assert = pmu_reset_assert, + .deassert = pmu_reset_deassert, +}; + +static struct reset_controller_dev pmu_reset __initdata = { + .ops = &pmu_reset_ops, + .owner = THIS_MODULE, + .nr_resets = 32, +}; + +static void __init pmu_reset_init(struct pmu_data *pmu) +{ + int ret; + + pmu->reset = pmu_reset; + pmu->reset.of_node = pmu->of_node; + + ret = reset_controller_register(&pmu->reset); + if (ret) + pr_err("pmu: %s failed: %d\n", "reset_controller_register", ret); +} +#else +static void __init pmu_reset_init(struct pmu_data *pmu) +{ +} +#endif + +struct pmu_domain { + struct pmu_data *pmu; + u32 pwr_mask; + u32 rst_mask; + u32 iso_mask; + struct generic_pm_domain base; +}; + +#define to_pmu_domain(dom) container_of(dom, struct pmu_domain, base) + +/* + * This deals with the "old" Marvell sequence of bringing a power domain + * down/up, which is: apply power, release reset, disable isolators. + * + * Later devices apparantly use a different sequence: power up, disable + * isolators, assert repair signal, enable SRMA clock, enable AXI clock, + * enable module clock, deassert reset. + * + * Note: reading the assembly, it seems that the IO accessors have an + * unfortunate side-effect - they cause memory already read into registers + * for the if () to be re-read for the bit-set or bit-clear operation. + * The code is written to avoid this. + */ +static int pmu_domain_power_off(struct generic_pm_domain *domain) +{ + struct pmu_domain *pmu_dom = to_pmu_domain(domain); + struct pmu_data *pmu = pmu_dom->pmu; + unsigned long flags; + unsigned int val; + void __iomem *pmu_base = pmu->pmu_base; + void __iomem *pmc_base = pmu->pmc_base; + + spin_lock_irqsave(&pmu->lock, flags); + + /* Enable isolators */ + if (pmu_dom->iso_mask) { + val = ~pmu_dom->iso_mask; + val &= readl_relaxed(pmu_base + PMU_ISO); + writel_relaxed(val, pmu_base + PMU_ISO); + } + + /* Reset unit */ + if (pmu_dom->rst_mask) { + val = ~pmu_dom->rst_mask; + val &= readl_relaxed(pmc_base + PMC_SW_RST); + writel_relaxed(val, pmc_base + PMC_SW_RST); + } + + /* Power down */ + val = readl_relaxed(pmu_base + PMU_PWR) | pmu_dom->pwr_mask; + writel_relaxed(val, pmu_base + PMU_PWR); + + spin_unlock_irqrestore(&pmu->lock, flags); + + return 0; +} + +static int pmu_domain_power_on(struct generic_pm_domain *domain) +{ + struct pmu_domain *pmu_dom = to_pmu_domain(domain); + struct pmu_data *pmu = pmu_dom->pmu; + unsigned long flags; + unsigned int val; + void __iomem *pmu_base = pmu->pmu_base; + void __iomem *pmc_base = pmu->pmc_base; + + spin_lock_irqsave(&pmu->lock, flags); + + /* Power on */ + val = ~pmu_dom->pwr_mask & readl_relaxed(pmu_base + PMU_PWR); + writel_relaxed(val, pmu_base + PMU_PWR); + + /* Release reset */ + if (pmu_dom->rst_mask) { + val = pmu_dom->rst_mask; + val |= readl_relaxed(pmc_base + PMC_SW_RST); + writel_relaxed(val, pmc_base + PMC_SW_RST); + } + + /* Disable isolators */ + if (pmu_dom->iso_mask) { + val = pmu_dom->iso_mask; + val |= readl_relaxed(pmu_base + PMU_ISO); + writel_relaxed(val, pmu_base + PMU_ISO); + } + + spin_unlock_irqrestore(&pmu->lock, flags); + + return 0; +} + +static void __pmu_domain_register(struct pmu_domain *domain) +{ + unsigned int val = readl_relaxed(domain->pmu->pmu_base + PMU_PWR); + + domain->base.dev_irq_safe = true; + domain->base.power_off = pmu_domain_power_off; + domain->base.power_on = pmu_domain_power_on; + + pm_genpd_init(&domain->base, NULL, !(val & domain->pwr_mask)); +} + +static void pmu_add_genpd_of(struct device *dev) +{ + struct device_node *node; + + node = of_parse_phandle(dev->of_node, "marvell,power-domain", 0); + if (!node) + return; + + while (1) { + if (pm_genpd_of_add_device(node, dev) != -EAGAIN) + break; + cond_resched(); + } +} + +static void pmu_remove_genpd(struct device *dev) +{ + struct generic_pm_domain *genpd = dev_to_genpd(dev); + + while (1) { + if (pm_genpd_remove_device(genpd, dev) != -EAGAIN) + break; + cond_resched(); + } +} + +static int pmu_platform_call(struct notifier_block *nb, + unsigned long event, void *data) +{ + struct device *dev = data; + + switch (event) { + case BUS_NOTIFY_ADD_DEVICE: + if (dev->of_node) + pmu_add_genpd_of(dev); + break; + + case BUS_NOTIFY_DEL_DEVICE: + pmu_remove_genpd(dev); + break; + } + return NOTIFY_OK; +} + +static struct notifier_block platform_nb = { + .notifier_call = pmu_platform_call, +}; + +/* PMU IRQ controller */ +static void pmu_irq_handler(unsigned int irq, struct irq_desc *desc) +{ + struct pmu_data *pmu = irq_get_handler_data(irq); + struct irq_chip_generic *gc = pmu->irq_gc; + struct irq_domain *domain = pmu->irq_domain; + void __iomem *base = gc->reg_base; + u32 stat = readl_relaxed(base + PMC_IRQ_CAUSE) & gc->mask_cache; + u32 done = ~0; + + if (stat == 0) { + do_bad_IRQ(irq, desc); + return; + } + + while (stat) { + u32 hwirq = fls(stat) - 1; + + stat &= ~(1 << hwirq); + done &= ~(1 << hwirq); + + generic_handle_irq(irq_find_mapping(domain, hwirq)); + } + + /* + * The PMU mask register is not RW0C: it is RW. This means that + * the bits take whatever value is written to them; if you write + * a '1', you will set the interrupt. + * + * Unfortunately this means there is NO race free way to clear + * these interrupts. + * + * So, let's structure the code so that the window is as small as + * possible. + */ + irq_gc_lock(gc); + done &= readl_relaxed(base + PMC_IRQ_CAUSE); + writel_relaxed(done, base + PMC_IRQ_CAUSE); + irq_gc_unlock(gc); +} + +static int __init dove_init_pmu_irq(struct pmu_data *pmu, int irq) +{ + const char *name = "pmu_irq"; + struct irq_chip_generic *gc; + struct irq_domain *domain; + int ret; + + /* mask and clear all interrupts */ + writel(0, pmu->pmc_base + PMC_IRQ_MASK); + writel(0, pmu->pmc_base + PMC_IRQ_CAUSE); + + domain = irq_domain_add_linear(pmu->of_node, NR_PMU_IRQS, + &irq_generic_chip_ops, NULL); + if (!domain) { + pr_err("%s: unable to add irq domain\n", name); + return -ENOMEM; + } + + ret = irq_alloc_domain_generic_chips(domain, NR_PMU_IRQS, 1, name, + handle_level_irq, + IRQ_NOREQUEST | IRQ_NOPROBE, 0, + IRQ_GC_INIT_MASK_CACHE); + if (ret) { + pr_err("%s: unable to alloc irq domain gc: %d\n", name, ret); + irq_domain_remove(domain); + return ret; + } + + gc = irq_get_domain_generic_chip(domain, 0); + gc->reg_base = pmu->pmc_base; + gc->chip_types[0].regs.mask = PMC_IRQ_MASK; + gc->chip_types[0].chip.irq_mask = irq_gc_mask_clr_bit; + gc->chip_types[0].chip.irq_unmask = irq_gc_mask_set_bit; + + pmu->irq_domain = domain; + pmu->irq_gc = gc; + + /* If no of_node, populate the domain */ + if (!pmu->of_node) + irq_domain_associate_many(pmu->irq_domain, IRQ_DOVE_PMU_START, + 0, NR_PMU_IRQS); + + irq_set_handler_data(irq, pmu); + irq_set_chained_handler(irq, pmu_irq_handler); + + return 0; +} + +/* + * pmu { + * compatible = "marvell,pmu"; + * reg = <0xd0000 0x8000> <0xd8000 0x8000>; + * interrupts = <33>; + * #reset-cells = 1; + * vpu_domain: vpu-domain { + * marvell,pmu_pwr_mask = <0x00000008>; + * marvell,pmu_iso_mask = <0x00000001>; + * resets = <&pmu 16>; + * }; + * gpu_domain: gpu-domain { + * marvell,pmu_pwr_mask = <0x00000004>; + * marvell,pmu_iso_mask = <0x00000002>; + * resets = <&pmu 18>; + * }; + * }; + */ +int __init dove_init_pmu(void) +{ + struct device_node *np_pmu, *np; + struct pmu_data *pmu; + int ret, parent_irq; + + /* Lookup the PMU node */ + np_pmu = of_find_compatible_node(NULL, NULL, "marvell,pmu"); + if (!np_pmu) + return 0; + + pmu = kzalloc(sizeof(*pmu), GFP_KERNEL); + if (!pmu) + return -ENOMEM; + + spin_lock_init(&pmu->lock); + pmu->of_node = np_pmu; + pmu->pmc_base = of_iomap(pmu->of_node, 0); + pmu->pmu_base = of_iomap(pmu->of_node, 1); + if (!pmu->pmc_base || !pmu->pmu_base) { + pr_err("%s: failed to map PMU\n", np_pmu->name); + iounmap(pmu->pmu_base); + iounmap(pmu->pmc_base); + kfree(pmu); + return -ENOMEM; + } + + parent_irq = irq_of_parse_and_map(pmu->of_node, 0); + if (!parent_irq) + pr_err("%s: no interrupt specified\n", np_pmu->name); + + pmu_reset_init(pmu); + + for_each_available_child_of_node(pmu->of_node, np) { + struct of_phandle_args args; + struct pmu_domain *domain; + + domain = kzalloc(sizeof(*domain), GFP_KERNEL); + if (!domain) + break; + + domain->pmu = pmu; + domain->base.of_node = np; + domain->base.name = kstrdup(np->name, GFP_KERNEL); + if (!domain->base.name) { + kfree(domain); + break; + } + + of_property_read_u32(np, "marvell,pmu_pwr_mask", + &domain->pwr_mask); + of_property_read_u32(np, "marvell,pmu_iso_mask", + &domain->iso_mask); + + ret = of_parse_phandle_with_args(np, "resets", "#reset-cells", + 0, &args); + if (ret == 0) { + if (args.np == pmu->of_node) + domain->rst_mask = BIT(args.args[0]); + of_node_put(args.np); + } + + __pmu_domain_register(domain); + } + pm_genpd_poweroff_unused(); + + ret = dove_init_pmu_irq(pmu, parent_irq); + if (ret) + pr_err("dove_init_pmu_irq() failed: %d\n", ret); + + bus_register_notifier(&platform_bus_type, &platform_nb); + + return 0; +} -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH 3/5] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets 2014-04-27 13:29 ` Russell King @ 2014-04-28 11:55 ` Ulf Hansson -1 siblings, 0 replies; 49+ messages in thread From: Ulf Hansson @ 2014-04-28 11:55 UTC (permalink / raw) To: Russell King Cc: linux-arm-kernel, Sebastian Hesselbarth, devicetree, linux-pm, Mark Rutland, Pawel Moll, Rafael J. Wysocki, Rob Herring, Tomasz Figa On 27 April 2014 15:29, Russell King <rmk+kernel@arm.linux.org.uk> wrote: > The PMU device contains an interrupt controller, power control and > resets. The interrupt controller is a little sub-standard in that > there is no race free way to clear down pending interrupts, so we try > to avoid problems by reducing the window as much as possible, and > clearing as infrequently as possible. > > The interrupt support is implemented using an IRQ domain, and the > parent interrupt referenced in the standard DT way. > > The power domains and reset support is closely related - there is a > defined sequence for powering down a domain which is tightly coupled > with asserting the reset. Hence, it makes sense to group these two > together. > > This patch adds the core PMU driver: power domains must be defined in > the DT file in order to make use of them. The reset controller can > be referenced in the standard way for reset controllers. Hi Russell, This patch would be simplified if this was based upon the not yet merged patchset from Tomasz Figa, "[PATCH v3 0/3] Generic Device Tree based power domain look-up". For example you would likely not need to add some of the marvel specific DT bindings, and you wouldn’t need the bus_notifiers to add devices to the power domain. I guess I just though it could be useful input to consider while going forward, unless you already knew. Kind regards Ulf Hansson ^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 3/5] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets @ 2014-04-28 11:55 ` Ulf Hansson 0 siblings, 0 replies; 49+ messages in thread From: Ulf Hansson @ 2014-04-28 11:55 UTC (permalink / raw) To: linux-arm-kernel On 27 April 2014 15:29, Russell King <rmk+kernel@arm.linux.org.uk> wrote: > The PMU device contains an interrupt controller, power control and > resets. The interrupt controller is a little sub-standard in that > there is no race free way to clear down pending interrupts, so we try > to avoid problems by reducing the window as much as possible, and > clearing as infrequently as possible. > > The interrupt support is implemented using an IRQ domain, and the > parent interrupt referenced in the standard DT way. > > The power domains and reset support is closely related - there is a > defined sequence for powering down a domain which is tightly coupled > with asserting the reset. Hence, it makes sense to group these two > together. > > This patch adds the core PMU driver: power domains must be defined in > the DT file in order to make use of them. The reset controller can > be referenced in the standard way for reset controllers. Hi Russell, This patch would be simplified if this was based upon the not yet merged patchset from Tomasz Figa, "[PATCH v3 0/3] Generic Device Tree based power domain look-up". For example you would likely not need to add some of the marvel specific DT bindings, and you wouldn?t need the bus_notifiers to add devices to the power domain. I guess I just though it could be useful input to consider while going forward, unless you already knew. Kind regards Ulf Hansson ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH 3/5] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets 2014-04-28 11:55 ` Ulf Hansson @ 2014-04-28 12:17 ` Russell King - ARM Linux -1 siblings, 0 replies; 49+ messages in thread From: Russell King - ARM Linux @ 2014-04-28 12:17 UTC (permalink / raw) To: Ulf Hansson Cc: linux-arm-kernel, Sebastian Hesselbarth, devicetree, linux-pm, Mark Rutland, Pawel Moll, Rafael J. Wysocki, Rob Herring, Tomasz Figa On Mon, Apr 28, 2014 at 01:55:40PM +0200, Ulf Hansson wrote: > On 27 April 2014 15:29, Russell King <rmk+kernel@arm.linux.org.uk> wrote: > > The PMU device contains an interrupt controller, power control and > > resets. The interrupt controller is a little sub-standard in that > > there is no race free way to clear down pending interrupts, so we try > > to avoid problems by reducing the window as much as possible, and > > clearing as infrequently as possible. > > > > The interrupt support is implemented using an IRQ domain, and the > > parent interrupt referenced in the standard DT way. > > > > The power domains and reset support is closely related - there is a > > defined sequence for powering down a domain which is tightly coupled > > with asserting the reset. Hence, it makes sense to group these two > > together. > > > > This patch adds the core PMU driver: power domains must be defined in > > the DT file in order to make use of them. The reset controller can > > be referenced in the standard way for reset controllers. > > Hi Russell, > > This patch would be simplified if this was based upon the not yet > merged patchset from Tomasz Figa, "[PATCH v3 0/3] Generic Device Tree > based power domain look-up". > > For example you would likely not need to add some of the marvel > specific DT bindings, and you wouldn’t need the bus_notifiers to add > devices to the power domain. I guess I just though it could be useful > input to consider while going forward, unless you already knew. Does that apply to 3.14? -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 3/5] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets @ 2014-04-28 12:17 ` Russell King - ARM Linux 0 siblings, 0 replies; 49+ messages in thread From: Russell King - ARM Linux @ 2014-04-28 12:17 UTC (permalink / raw) To: linux-arm-kernel On Mon, Apr 28, 2014 at 01:55:40PM +0200, Ulf Hansson wrote: > On 27 April 2014 15:29, Russell King <rmk+kernel@arm.linux.org.uk> wrote: > > The PMU device contains an interrupt controller, power control and > > resets. The interrupt controller is a little sub-standard in that > > there is no race free way to clear down pending interrupts, so we try > > to avoid problems by reducing the window as much as possible, and > > clearing as infrequently as possible. > > > > The interrupt support is implemented using an IRQ domain, and the > > parent interrupt referenced in the standard DT way. > > > > The power domains and reset support is closely related - there is a > > defined sequence for powering down a domain which is tightly coupled > > with asserting the reset. Hence, it makes sense to group these two > > together. > > > > This patch adds the core PMU driver: power domains must be defined in > > the DT file in order to make use of them. The reset controller can > > be referenced in the standard way for reset controllers. > > Hi Russell, > > This patch would be simplified if this was based upon the not yet > merged patchset from Tomasz Figa, "[PATCH v3 0/3] Generic Device Tree > based power domain look-up". > > For example you would likely not need to add some of the marvel > specific DT bindings, and you wouldn?t need the bus_notifiers to add > devices to the power domain. I guess I just though it could be useful > input to consider while going forward, unless you already knew. Does that apply to 3.14? -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH 3/5] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets 2014-04-28 11:55 ` Ulf Hansson @ 2015-02-13 13:29 ` Russell King - ARM Linux -1 siblings, 0 replies; 49+ messages in thread From: Russell King - ARM Linux @ 2015-02-13 13:29 UTC (permalink / raw) To: Ulf Hansson Cc: linux-arm-kernel, Sebastian Hesselbarth, devicetree, linux-pm, Mark Rutland, Pawel Moll, Rafael J. Wysocki, Rob Herring, Tomasz Figa On Mon, Apr 28, 2014 at 01:55:40PM +0200, Ulf Hansson wrote: > On 27 April 2014 15:29, Russell King <rmk+kernel@arm.linux.org.uk> wrote: > > The PMU device contains an interrupt controller, power control and > > resets. The interrupt controller is a little sub-standard in that > > there is no race free way to clear down pending interrupts, so we try > > to avoid problems by reducing the window as much as possible, and > > clearing as infrequently as possible. > > > > The interrupt support is implemented using an IRQ domain, and the > > parent interrupt referenced in the standard DT way. > > > > The power domains and reset support is closely related - there is a > > defined sequence for powering down a domain which is tightly coupled > > with asserting the reset. Hence, it makes sense to group these two > > together. > > > > This patch adds the core PMU driver: power domains must be defined in > > the DT file in order to make use of them. The reset controller can > > be referenced in the standard way for reset controllers. > > Hi Russell, > > This patch would be simplified if this was based upon the not yet > merged patchset from Tomasz Figa, "[PATCH v3 0/3] Generic Device Tree > based power domain look-up". > > For example you would likely not need to add some of the marvel > specific DT bindings, and you wouldn’t need the bus_notifiers to add > devices to the power domain. I guess I just though it could be useful > input to consider while going forward, unless you already knew. In 3.19, I notice something of an odd behaviour. My vMeta driver has runtime PM support enabled. When I explicitly register the PM domain in the pmu driver via a bus notifier, I see: root@cubox:~# cat /sys/kernel/debug/pm_genpd/pm_genpd_summary domain status slaves /device runtime status ---------------------------------------------------------------------- gpu-domain on /devices/platform/vivante/etnaviv-gpu,2d active vpu-domain off /devices/platform/mbus/mbus:internal-regs/f1c00000.video-decoder suspended But when I disable that, and let the generic code do the registration, I instead get: root@cubox:~# cat /sys/kernel/debug/pm_genpd/pm_genpd_summary domain status slaves /device runtime status ---------------------------------------------------------------------- gpu-domain on /devices/platform/vivante/etnaviv-gpu,2d active vpu-domain on /devices/platform/mbus/mbus:internal-regs/f1c00000.video-decoder suspended The difference being that the vpu domain remains powered. The only difference code-wise seems to be when genpd_dev_pm_attach() is called. In the working case, it's before the device is considered for probing. In the non-working case, it's just before the device is probed. With debugging enabled in the PM domain code, with the former case I get: Added domain provider from /mbus/internal-regs/power-management@d0000/vpu-domain platform f1c00000.video-decoder: adding to PM domain vpu-domain platform f1c00000.video-decoder: __pm_genpd_add_device() With the latter non-working case: Added domain provider from /mbus/internal-regs/power-management@d0000/vpu-domain ... ap510-vmeta f1c00000.video-decoder: adding to PM domain vpu-domain ap510-vmeta f1c00000.video-decoder: __pm_genpd_add_device() vpu-domain: Power-on latency exceeded, new value 1578 ns Neither of these debug messages provide much hint as to what the difference is, or the cause of the PM domain code being de-sync'd with its devices. Maybe the PM code needs more debugging in it, and maybe the debugfs file should always be present if debugfs support is enabled? -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 3/5] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets @ 2015-02-13 13:29 ` Russell King - ARM Linux 0 siblings, 0 replies; 49+ messages in thread From: Russell King - ARM Linux @ 2015-02-13 13:29 UTC (permalink / raw) To: linux-arm-kernel On Mon, Apr 28, 2014 at 01:55:40PM +0200, Ulf Hansson wrote: > On 27 April 2014 15:29, Russell King <rmk+kernel@arm.linux.org.uk> wrote: > > The PMU device contains an interrupt controller, power control and > > resets. The interrupt controller is a little sub-standard in that > > there is no race free way to clear down pending interrupts, so we try > > to avoid problems by reducing the window as much as possible, and > > clearing as infrequently as possible. > > > > The interrupt support is implemented using an IRQ domain, and the > > parent interrupt referenced in the standard DT way. > > > > The power domains and reset support is closely related - there is a > > defined sequence for powering down a domain which is tightly coupled > > with asserting the reset. Hence, it makes sense to group these two > > together. > > > > This patch adds the core PMU driver: power domains must be defined in > > the DT file in order to make use of them. The reset controller can > > be referenced in the standard way for reset controllers. > > Hi Russell, > > This patch would be simplified if this was based upon the not yet > merged patchset from Tomasz Figa, "[PATCH v3 0/3] Generic Device Tree > based power domain look-up". > > For example you would likely not need to add some of the marvel > specific DT bindings, and you wouldn?t need the bus_notifiers to add > devices to the power domain. I guess I just though it could be useful > input to consider while going forward, unless you already knew. In 3.19, I notice something of an odd behaviour. My vMeta driver has runtime PM support enabled. When I explicitly register the PM domain in the pmu driver via a bus notifier, I see: root at cubox:~# cat /sys/kernel/debug/pm_genpd/pm_genpd_summary domain status slaves /device runtime status ---------------------------------------------------------------------- gpu-domain on /devices/platform/vivante/etnaviv-gpu,2d active vpu-domain off /devices/platform/mbus/mbus:internal-regs/f1c00000.video-decoder suspended But when I disable that, and let the generic code do the registration, I instead get: root at cubox:~# cat /sys/kernel/debug/pm_genpd/pm_genpd_summary domain status slaves /device runtime status ---------------------------------------------------------------------- gpu-domain on /devices/platform/vivante/etnaviv-gpu,2d active vpu-domain on /devices/platform/mbus/mbus:internal-regs/f1c00000.video-decoder suspended The difference being that the vpu domain remains powered. The only difference code-wise seems to be when genpd_dev_pm_attach() is called. In the working case, it's before the device is considered for probing. In the non-working case, it's just before the device is probed. With debugging enabled in the PM domain code, with the former case I get: Added domain provider from /mbus/internal-regs/power-management at d0000/vpu-domain platform f1c00000.video-decoder: adding to PM domain vpu-domain platform f1c00000.video-decoder: __pm_genpd_add_device() With the latter non-working case: Added domain provider from /mbus/internal-regs/power-management at d0000/vpu-domain ... ap510-vmeta f1c00000.video-decoder: adding to PM domain vpu-domain ap510-vmeta f1c00000.video-decoder: __pm_genpd_add_device() vpu-domain: Power-on latency exceeded, new value 1578 ns Neither of these debug messages provide much hint as to what the difference is, or the cause of the PM domain code being de-sync'd with its devices. Maybe the PM code needs more debugging in it, and maybe the debugfs file should always be present if debugfs support is enabled? -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH 3/5] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets 2015-02-13 13:29 ` Russell King - ARM Linux @ 2015-02-13 14:11 ` Russell King - ARM Linux -1 siblings, 0 replies; 49+ messages in thread From: Russell King - ARM Linux @ 2015-02-13 14:11 UTC (permalink / raw) To: Ulf Hansson Cc: Mark Rutland, devicetree, Pawel Moll, linux-pm, Rafael J. Wysocki, Tomasz Figa, Rob Herring, linux-arm-kernel, Sebastian Hesselbarth On Fri, Feb 13, 2015 at 01:29:25PM +0000, Russell King - ARM Linux wrote: > On Mon, Apr 28, 2014 at 01:55:40PM +0200, Ulf Hansson wrote: > > On 27 April 2014 15:29, Russell King <rmk+kernel@arm.linux.org.uk> wrote: > > > The PMU device contains an interrupt controller, power control and > > > resets. The interrupt controller is a little sub-standard in that > > > there is no race free way to clear down pending interrupts, so we try > > > to avoid problems by reducing the window as much as possible, and > > > clearing as infrequently as possible. > > > > > > The interrupt support is implemented using an IRQ domain, and the > > > parent interrupt referenced in the standard DT way. > > > > > > The power domains and reset support is closely related - there is a > > > defined sequence for powering down a domain which is tightly coupled > > > with asserting the reset. Hence, it makes sense to group these two > > > together. > > > > > > This patch adds the core PMU driver: power domains must be defined in > > > the DT file in order to make use of them. The reset controller can > > > be referenced in the standard way for reset controllers. > > > > Hi Russell, > > > > This patch would be simplified if this was based upon the not yet > > merged patchset from Tomasz Figa, "[PATCH v3 0/3] Generic Device Tree > > based power domain look-up". > > > > For example you would likely not need to add some of the marvel > > specific DT bindings, and you wouldn’t need the bus_notifiers to add > > devices to the power domain. I guess I just though it could be useful > > input to consider while going forward, unless you already knew. > > In 3.19, I notice something of an odd behaviour. > > My vMeta driver has runtime PM support enabled. When I explicitly register > the PM domain in the pmu driver via a bus notifier, I see: > > root@cubox:~# cat /sys/kernel/debug/pm_genpd/pm_genpd_summary > domain status slaves > /device runtime status > ---------------------------------------------------------------------- > gpu-domain on > /devices/platform/vivante/etnaviv-gpu,2d active > vpu-domain off > /devices/platform/mbus/mbus:internal-regs/f1c00000.video-decoder suspended > > But when I disable that, and let the generic code do the registration, > I instead get: > > root@cubox:~# cat /sys/kernel/debug/pm_genpd/pm_genpd_summary > domain status slaves > /device runtime status > ---------------------------------------------------------------------- > gpu-domain on > /devices/platform/vivante/etnaviv-gpu,2d active > vpu-domain on > /devices/platform/mbus/mbus:internal-regs/f1c00000.video-decoder suspended > > The difference being that the vpu domain remains powered. > > The only difference code-wise seems to be when genpd_dev_pm_attach() is > called. In the working case, it's before the device is considered for > probing. In the non-working case, it's just before the device is probed. > > With debugging enabled in the PM domain code, with the former case I get: > > Added domain provider from /mbus/internal-regs/power-management@d0000/vpu-domain > platform f1c00000.video-decoder: adding to PM domain vpu-domain > platform f1c00000.video-decoder: __pm_genpd_add_device() > > With the latter non-working case: > > Added domain provider from /mbus/internal-regs/power-management@d0000/vpu-domain > ... > ap510-vmeta f1c00000.video-decoder: adding to PM domain vpu-domain > ap510-vmeta f1c00000.video-decoder: __pm_genpd_add_device() > vpu-domain: Power-on latency exceeded, new value 1578 ns > > Neither of these debug messages provide much hint as to what the > difference is, or the cause of the PM domain code being de-sync'd > with its devices. > > Maybe the PM code needs more debugging in it, and maybe the debugfs > file should always be present if debugfs support is enabled? The vmeta driver does this in its probe function: pm_runtime_use_autosuspend(vi->dev); pm_runtime_set_autosuspend_delay(vi->dev, 100); pm_runtime_enable(vi->dev); since it doesn't touch the hardware, and the hardware starts off at boot time in "suspended" mode. I think what's going on is that there's a difference in the expectations from the PM domain code vs the runtime PM code. I refer to section 5 of the runtime PM documentation: | 5. Runtime PM Initialization, Device Probing and Removal | | Initially, the runtime PM is disabled for all devices, which means that the | majority of the runtime PM helper functions described in Section 4 will return | -EAGAIN until pm_runtime_enable() is called for the device. | | In addition to that, the initial runtime PM status of all devices is | 'suspended', but it need not reflect the actual physical state of the device. | Thus, if the device is initially active (i.e. it is able to process I/O), its | runtime PM status must be changed to 'active', with the help of | pm_runtime_set_active(), before pm_runtime_enable() is called for the device. However, the PM domain code seems to always power up the PM domain when a device is attached to it: int genpd_dev_pm_attach(struct device *dev) { ... pm_genpd_poweron(pd); return 0; } EXPORT_SYMBOL_GPL(genpd_dev_pm_attach); So, the PM domain code ends up disagreeing with the runtime PM code about the state of the device. I think your commit (2ed127697eb1 "PM / Domains: Power on the PM domain right after attach completes") is fundamentally wrong. The assertion you make in there is built upon the assumption that every driver will call pm_runtime_set_active(), which is not an assumption you can make. Instead, you should be doing is to hook into __pm_runtime_set_status() and use that to trigger the PM domain power up so that the runtime PM and PM domain state is always in step with each other. What I'm certain of is that the current situation is just totally crazy. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH 3/5] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets @ 2015-02-13 14:11 ` Russell King - ARM Linux 0 siblings, 0 replies; 49+ messages in thread From: Russell King - ARM Linux @ 2015-02-13 14:11 UTC (permalink / raw) To: linux-arm-kernel On Fri, Feb 13, 2015 at 01:29:25PM +0000, Russell King - ARM Linux wrote: > On Mon, Apr 28, 2014 at 01:55:40PM +0200, Ulf Hansson wrote: > > On 27 April 2014 15:29, Russell King <rmk+kernel@arm.linux.org.uk> wrote: > > > The PMU device contains an interrupt controller, power control and > > > resets. The interrupt controller is a little sub-standard in that > > > there is no race free way to clear down pending interrupts, so we try > > > to avoid problems by reducing the window as much as possible, and > > > clearing as infrequently as possible. > > > > > > The interrupt support is implemented using an IRQ domain, and the > > > parent interrupt referenced in the standard DT way. > > > > > > The power domains and reset support is closely related - there is a > > > defined sequence for powering down a domain which is tightly coupled > > > with asserting the reset. Hence, it makes sense to group these two > > > together. > > > > > > This patch adds the core PMU driver: power domains must be defined in > > > the DT file in order to make use of them. The reset controller can > > > be referenced in the standard way for reset controllers. > > > > Hi Russell, > > > > This patch would be simplified if this was based upon the not yet > > merged patchset from Tomasz Figa, "[PATCH v3 0/3] Generic Device Tree > > based power domain look-up". > > > > For example you would likely not need to add some of the marvel > > specific DT bindings, and you wouldn?t need the bus_notifiers to add > > devices to the power domain. I guess I just though it could be useful > > input to consider while going forward, unless you already knew. > > In 3.19, I notice something of an odd behaviour. > > My vMeta driver has runtime PM support enabled. When I explicitly register > the PM domain in the pmu driver via a bus notifier, I see: > > root at cubox:~# cat /sys/kernel/debug/pm_genpd/pm_genpd_summary > domain status slaves > /device runtime status > ---------------------------------------------------------------------- > gpu-domain on > /devices/platform/vivante/etnaviv-gpu,2d active > vpu-domain off > /devices/platform/mbus/mbus:internal-regs/f1c00000.video-decoder suspended > > But when I disable that, and let the generic code do the registration, > I instead get: > > root at cubox:~# cat /sys/kernel/debug/pm_genpd/pm_genpd_summary > domain status slaves > /device runtime status > ---------------------------------------------------------------------- > gpu-domain on > /devices/platform/vivante/etnaviv-gpu,2d active > vpu-domain on > /devices/platform/mbus/mbus:internal-regs/f1c00000.video-decoder suspended > > The difference being that the vpu domain remains powered. > > The only difference code-wise seems to be when genpd_dev_pm_attach() is > called. In the working case, it's before the device is considered for > probing. In the non-working case, it's just before the device is probed. > > With debugging enabled in the PM domain code, with the former case I get: > > Added domain provider from /mbus/internal-regs/power-management at d0000/vpu-domain > platform f1c00000.video-decoder: adding to PM domain vpu-domain > platform f1c00000.video-decoder: __pm_genpd_add_device() > > With the latter non-working case: > > Added domain provider from /mbus/internal-regs/power-management at d0000/vpu-domain > ... > ap510-vmeta f1c00000.video-decoder: adding to PM domain vpu-domain > ap510-vmeta f1c00000.video-decoder: __pm_genpd_add_device() > vpu-domain: Power-on latency exceeded, new value 1578 ns > > Neither of these debug messages provide much hint as to what the > difference is, or the cause of the PM domain code being de-sync'd > with its devices. > > Maybe the PM code needs more debugging in it, and maybe the debugfs > file should always be present if debugfs support is enabled? The vmeta driver does this in its probe function: pm_runtime_use_autosuspend(vi->dev); pm_runtime_set_autosuspend_delay(vi->dev, 100); pm_runtime_enable(vi->dev); since it doesn't touch the hardware, and the hardware starts off at boot time in "suspended" mode. I think what's going on is that there's a difference in the expectations from the PM domain code vs the runtime PM code. I refer to section 5 of the runtime PM documentation: | 5. Runtime PM Initialization, Device Probing and Removal | | Initially, the runtime PM is disabled for all devices, which means that the | majority of the runtime PM helper functions described in Section 4 will return | -EAGAIN until pm_runtime_enable() is called for the device. | | In addition to that, the initial runtime PM status of all devices is | 'suspended', but it need not reflect the actual physical state of the device. | Thus, if the device is initially active (i.e. it is able to process I/O), its | runtime PM status must be changed to 'active', with the help of | pm_runtime_set_active(), before pm_runtime_enable() is called for the device. However, the PM domain code seems to always power up the PM domain when a device is attached to it: int genpd_dev_pm_attach(struct device *dev) { ... pm_genpd_poweron(pd); return 0; } EXPORT_SYMBOL_GPL(genpd_dev_pm_attach); So, the PM domain code ends up disagreeing with the runtime PM code about the state of the device. I think your commit (2ed127697eb1 "PM / Domains: Power on the PM domain right after attach completes") is fundamentally wrong. The assertion you make in there is built upon the assumption that every driver will call pm_runtime_set_active(), which is not an assumption you can make. Instead, you should be doing is to hook into __pm_runtime_set_status() and use that to trigger the PM domain power up so that the runtime PM and PM domain state is always in step with each other. What I'm certain of is that the current situation is just totally crazy. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH 3/5] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets 2015-02-13 14:11 ` Russell King - ARM Linux @ 2015-02-13 16:12 ` Russell King - ARM Linux -1 siblings, 0 replies; 49+ messages in thread From: Russell King - ARM Linux @ 2015-02-13 16:12 UTC (permalink / raw) To: Ulf Hansson Cc: Mark Rutland, devicetree, Pawel Moll, linux-pm, Rafael J. Wysocki, Tomasz Figa, Rob Herring, linux-arm-kernel, Sebastian Hesselbarth On Fri, Feb 13, 2015 at 02:11:52PM +0000, Russell King - ARM Linux wrote: > I think what's going on is that there's a difference in the expectations > from the PM domain code vs the runtime PM code. I refer to section 5 > of the runtime PM documentation: > > | 5. Runtime PM Initialization, Device Probing and Removal > | > | Initially, the runtime PM is disabled for all devices, which means that the > | majority of the runtime PM helper functions described in Section 4 will return > | -EAGAIN until pm_runtime_enable() is called for the device. > | > | In addition to that, the initial runtime PM status of all devices is > | 'suspended', but it need not reflect the actual physical state of the device. > | Thus, if the device is initially active (i.e. it is able to process I/O), its > | runtime PM status must be changed to 'active', with the help of > | pm_runtime_set_active(), before pm_runtime_enable() is called for the device. > > However, the PM domain code seems to always power up the PM domain when > a device is attached to it: > > int genpd_dev_pm_attach(struct device *dev) > { > ... > pm_genpd_poweron(pd); > > return 0; > } > EXPORT_SYMBOL_GPL(genpd_dev_pm_attach); > > So, the PM domain code ends up disagreeing with the runtime PM code about > the state of the device. > > I think your commit (2ed127697eb1 "PM / Domains: Power on the PM domain > right after attach completes") is fundamentally wrong. The assertion > you make in there is built upon the assumption that every driver will > call pm_runtime_set_active(), which is not an assumption you can make. > > Instead, you should be doing is to hook into __pm_runtime_set_status() > and use that to trigger the PM domain power up so that the runtime PM > and PM domain state is always in step with each other. > > What I'm certain of is that the current situation is just totally crazy. There are around 150 drivers in the kernel tree which do not call pm_runtime_set_active() before calling pm_runtime_enable(), so the assertion in the PM domains commit above is wrong. Some of them are only saved because they do a pm_runtime_get() immediately after pm_runtime_enable(), but that's in no way guaranteed - others do neither. So, for this to work properly, we need to address this issue _correctly_ rather than papering over the problem. Here's a patch which solves the issue for me along the lines which I described above. I'm introducing a new callback - runtime_set_status() which the PM domain code uses to be notified of the runtime PM status updates while RPM is disabled. This callback will only occur from __pm_runtime_set_status(), which can only be used while runtime PM is disabled. I believe it's safe - if we think that a PM domain is powered down, and the runtime PM status is then set active, it's clear that the PM domain absolutely must transition to active mode as well. If the runtime PM status is set to suspended, then the PM domain code can transition to off state. I've left some of my debugging in place in the patch below as that's exactly the code I've tested. diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 751a8859a4ab..1616faadf904 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -5,7 +5,7 @@ * * This file is released under the GPLv2. */ - +#define DEBUG #include <linux/kernel.h> #include <linux/io.h> #include <linux/platform_device.h> @@ -158,6 +158,8 @@ static int genpd_power_on(struct generic_pm_domain *genpd) s64 elapsed_ns; int ret; + pr_debug("%s: %s()\n", genpd->name, __func__); + if (!genpd->power_on) return 0; @@ -219,6 +221,10 @@ static int __pm_genpd_poweron(struct generic_pm_domain *genpd) DEFINE_WAIT(wait); int ret = 0; + pr_debug("%s: %s() status %u prepared %d spo %u\n", + genpd->name, __func__, genpd->status, genpd->prepared_count, + genpd->suspend_power_off); + /* If the domain's master is being waited for, we have to wait too. */ for (;;) { prepare_to_wait(&genpd->status_wait_queue, &wait, @@ -741,6 +747,20 @@ static int pm_genpd_runtime_resume(struct device *dev) return 0; } +static int pm_genpd_runtime_set_status(struct device *dev) +{ + int ret; + + dev_dbg(dev, "%s()\n", __func__); + + if (pm_runtime_suspended(dev)) + ret = pm_genpd_runtime_suspend(dev); + else + ret = pm_genpd_runtime_resume(dev); + + return ret; +} + static bool pd_ignore_unused; static int __init pd_ignore_unused_setup(char *__unused) { @@ -1907,6 +1927,7 @@ void pm_genpd_init(struct generic_pm_domain *genpd, genpd->max_off_time_changed = true; genpd->domain.ops.runtime_suspend = pm_genpd_runtime_suspend; genpd->domain.ops.runtime_resume = pm_genpd_runtime_resume; + genpd->domain.ops.runtime_set_status = pm_genpd_runtime_set_status; genpd->domain.ops.prepare = pm_genpd_prepare; genpd->domain.ops.suspend = pm_genpd_suspend; genpd->domain.ops.suspend_late = pm_genpd_suspend_late; @@ -2223,7 +2244,6 @@ int genpd_dev_pm_attach(struct device *dev) } dev->pm_domain->detach = genpd_dev_pm_detach; - pm_genpd_poweron(pd); return 0; } diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 5070c4fe8542..a958cae67a37 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -981,6 +981,7 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status) struct device *parent = dev->parent; unsigned long flags; bool notify_parent = false; + pm_callback_t callback; int error = 0; if (status != RPM_ACTIVE && status != RPM_SUSPENDED) @@ -1029,6 +1030,10 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status) out_set: __update_runtime_status(dev, status); dev->power.runtime_error = 0; + if (dev->power.no_callbacks) + goto out; + callback = RPM_GET_CALLBACK(dev, runtime_set_status); + rpm_callback(callback, dev); out: spin_unlock_irqrestore(&dev->power.lock, flags); diff --git a/include/linux/pm.h b/include/linux/pm.h index 8b5976364619..ee098585d577 100644 --- a/include/linux/pm.h +++ b/include/linux/pm.h @@ -316,6 +316,7 @@ struct dev_pm_ops { int (*runtime_suspend)(struct device *dev); int (*runtime_resume)(struct device *dev); int (*runtime_idle)(struct device *dev); + int (*runtime_set_status)(struct device *dev); }; #ifdef CONFIG_PM_SLEEP -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH 3/5] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets @ 2015-02-13 16:12 ` Russell King - ARM Linux 0 siblings, 0 replies; 49+ messages in thread From: Russell King - ARM Linux @ 2015-02-13 16:12 UTC (permalink / raw) To: linux-arm-kernel On Fri, Feb 13, 2015 at 02:11:52PM +0000, Russell King - ARM Linux wrote: > I think what's going on is that there's a difference in the expectations > from the PM domain code vs the runtime PM code. I refer to section 5 > of the runtime PM documentation: > > | 5. Runtime PM Initialization, Device Probing and Removal > | > | Initially, the runtime PM is disabled for all devices, which means that the > | majority of the runtime PM helper functions described in Section 4 will return > | -EAGAIN until pm_runtime_enable() is called for the device. > | > | In addition to that, the initial runtime PM status of all devices is > | 'suspended', but it need not reflect the actual physical state of the device. > | Thus, if the device is initially active (i.e. it is able to process I/O), its > | runtime PM status must be changed to 'active', with the help of > | pm_runtime_set_active(), before pm_runtime_enable() is called for the device. > > However, the PM domain code seems to always power up the PM domain when > a device is attached to it: > > int genpd_dev_pm_attach(struct device *dev) > { > ... > pm_genpd_poweron(pd); > > return 0; > } > EXPORT_SYMBOL_GPL(genpd_dev_pm_attach); > > So, the PM domain code ends up disagreeing with the runtime PM code about > the state of the device. > > I think your commit (2ed127697eb1 "PM / Domains: Power on the PM domain > right after attach completes") is fundamentally wrong. The assertion > you make in there is built upon the assumption that every driver will > call pm_runtime_set_active(), which is not an assumption you can make. > > Instead, you should be doing is to hook into __pm_runtime_set_status() > and use that to trigger the PM domain power up so that the runtime PM > and PM domain state is always in step with each other. > > What I'm certain of is that the current situation is just totally crazy. There are around 150 drivers in the kernel tree which do not call pm_runtime_set_active() before calling pm_runtime_enable(), so the assertion in the PM domains commit above is wrong. Some of them are only saved because they do a pm_runtime_get() immediately after pm_runtime_enable(), but that's in no way guaranteed - others do neither. So, for this to work properly, we need to address this issue _correctly_ rather than papering over the problem. Here's a patch which solves the issue for me along the lines which I described above. I'm introducing a new callback - runtime_set_status() which the PM domain code uses to be notified of the runtime PM status updates while RPM is disabled. This callback will only occur from __pm_runtime_set_status(), which can only be used while runtime PM is disabled. I believe it's safe - if we think that a PM domain is powered down, and the runtime PM status is then set active, it's clear that the PM domain absolutely must transition to active mode as well. If the runtime PM status is set to suspended, then the PM domain code can transition to off state. I've left some of my debugging in place in the patch below as that's exactly the code I've tested. diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 751a8859a4ab..1616faadf904 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -5,7 +5,7 @@ * * This file is released under the GPLv2. */ - +#define DEBUG #include <linux/kernel.h> #include <linux/io.h> #include <linux/platform_device.h> @@ -158,6 +158,8 @@ static int genpd_power_on(struct generic_pm_domain *genpd) s64 elapsed_ns; int ret; + pr_debug("%s: %s()\n", genpd->name, __func__); + if (!genpd->power_on) return 0; @@ -219,6 +221,10 @@ static int __pm_genpd_poweron(struct generic_pm_domain *genpd) DEFINE_WAIT(wait); int ret = 0; + pr_debug("%s: %s() status %u prepared %d spo %u\n", + genpd->name, __func__, genpd->status, genpd->prepared_count, + genpd->suspend_power_off); + /* If the domain's master is being waited for, we have to wait too. */ for (;;) { prepare_to_wait(&genpd->status_wait_queue, &wait, @@ -741,6 +747,20 @@ static int pm_genpd_runtime_resume(struct device *dev) return 0; } +static int pm_genpd_runtime_set_status(struct device *dev) +{ + int ret; + + dev_dbg(dev, "%s()\n", __func__); + + if (pm_runtime_suspended(dev)) + ret = pm_genpd_runtime_suspend(dev); + else + ret = pm_genpd_runtime_resume(dev); + + return ret; +} + static bool pd_ignore_unused; static int __init pd_ignore_unused_setup(char *__unused) { @@ -1907,6 +1927,7 @@ void pm_genpd_init(struct generic_pm_domain *genpd, genpd->max_off_time_changed = true; genpd->domain.ops.runtime_suspend = pm_genpd_runtime_suspend; genpd->domain.ops.runtime_resume = pm_genpd_runtime_resume; + genpd->domain.ops.runtime_set_status = pm_genpd_runtime_set_status; genpd->domain.ops.prepare = pm_genpd_prepare; genpd->domain.ops.suspend = pm_genpd_suspend; genpd->domain.ops.suspend_late = pm_genpd_suspend_late; @@ -2223,7 +2244,6 @@ int genpd_dev_pm_attach(struct device *dev) } dev->pm_domain->detach = genpd_dev_pm_detach; - pm_genpd_poweron(pd); return 0; } diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 5070c4fe8542..a958cae67a37 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -981,6 +981,7 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status) struct device *parent = dev->parent; unsigned long flags; bool notify_parent = false; + pm_callback_t callback; int error = 0; if (status != RPM_ACTIVE && status != RPM_SUSPENDED) @@ -1029,6 +1030,10 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status) out_set: __update_runtime_status(dev, status); dev->power.runtime_error = 0; + if (dev->power.no_callbacks) + goto out; + callback = RPM_GET_CALLBACK(dev, runtime_set_status); + rpm_callback(callback, dev); out: spin_unlock_irqrestore(&dev->power.lock, flags); diff --git a/include/linux/pm.h b/include/linux/pm.h index 8b5976364619..ee098585d577 100644 --- a/include/linux/pm.h +++ b/include/linux/pm.h @@ -316,6 +316,7 @@ struct dev_pm_ops { int (*runtime_suspend)(struct device *dev); int (*runtime_resume)(struct device *dev); int (*runtime_idle)(struct device *dev); + int (*runtime_set_status)(struct device *dev); }; #ifdef CONFIG_PM_SLEEP -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH 4/5] ARM: dove: add Dove PMU DT entries to dove.dtsi 2014-04-27 13:23 ` Russell King - ARM Linux @ 2014-04-27 13:29 ` Russell King -1 siblings, 0 replies; 49+ messages in thread From: Russell King @ 2014-04-27 13:29 UTC (permalink / raw) To: linux-arm-kernel, Sebastian Hesselbarth Cc: devicetree, linux-pm, Mark Rutland, Pawel Moll, Rafael J. Wysocki, Rob Herring Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> --- arch/arm/boot/dts/dove.dtsi | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/arch/arm/boot/dts/dove.dtsi b/arch/arm/boot/dts/dove.dtsi index 187fd46b7b5e..64773e96e7f6 100644 --- a/arch/arm/boot/dts/dove.dtsi +++ b/arch/arm/boot/dts/dove.dtsi @@ -367,6 +367,13 @@ status = "disabled"; }; + pmu: power-management@d0000 { + compatible = "marvell,pmu"; + reg = <0xd0000 0x8000>, <0xd8000 0x8000>; + interrupts = <33>; + #reset-cells = <1>; + }; + thermal: thermal-diode@d001c { compatible = "marvell,dove-thermal"; reg = <0xd001c 0x0c>, <0xd005c 0x08>; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH 4/5] ARM: dove: add Dove PMU DT entries to dove.dtsi @ 2014-04-27 13:29 ` Russell King 0 siblings, 0 replies; 49+ messages in thread From: Russell King @ 2014-04-27 13:29 UTC (permalink / raw) To: linux-arm-kernel Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> --- arch/arm/boot/dts/dove.dtsi | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/arch/arm/boot/dts/dove.dtsi b/arch/arm/boot/dts/dove.dtsi index 187fd46b7b5e..64773e96e7f6 100644 --- a/arch/arm/boot/dts/dove.dtsi +++ b/arch/arm/boot/dts/dove.dtsi @@ -367,6 +367,13 @@ status = "disabled"; }; + pmu: power-management at d0000 { + compatible = "marvell,pmu"; + reg = <0xd0000 0x8000>, <0xd8000 0x8000>; + interrupts = <33>; + #reset-cells = <1>; + }; + thermal: thermal-diode at d001c { compatible = "marvell,dove-thermal"; reg = <0xd001c 0x0c>, <0xd005c 0x08>; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH 5/5] ARM: dove: add non-DT PMU support 2014-04-27 13:23 ` Russell King - ARM Linux @ 2014-04-27 13:29 ` Russell King -1 siblings, 0 replies; 49+ messages in thread From: Russell King @ 2014-04-27 13:29 UTC (permalink / raw) To: linux-arm-kernel, Sebastian Hesselbarth Cc: devicetree, linux-pm, Mark Rutland, Pawel Moll, Rafael J. Wysocki, Rob Herring Since Dove has non-DT support still, this patch adds the static data to the PMU driver in order to initialise PMU domains. Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> --- arch/arm/mach-dove/pmu.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 1 deletion(-) diff --git a/arch/arm/mach-dove/pmu.c b/arch/arm/mach-dove/pmu.c index 0b3201fa2d5c..9832044dbab9 100644 --- a/arch/arm/mach-dove/pmu.c +++ b/arch/arm/mach-dove/pmu.c @@ -226,6 +226,20 @@ static void __pmu_domain_register(struct pmu_domain *domain) pm_genpd_init(&domain->base, NULL, !(val & domain->pwr_mask)); } +static void pmu_domain_register(struct pmu_data *pmu, + const struct pmu_domain *pmu_dom) +{ + struct pmu_domain *domain; + + domain = kmemdup(pmu_dom, sizeof(*domain), GFP_KERNEL); + if (!domain) + return; + + domain->pmu = pmu; + + __pmu_domain_register(domain); +} + static void pmu_add_genpd_of(struct device *dev) { struct device_node *node; @@ -241,6 +255,15 @@ static void pmu_add_genpd_of(struct device *dev) } } +static void pmu_add_genpd_name(const char *name, struct device *dev) +{ + while (1) { + if (pm_genpd_name_add_device(name, dev) != -EAGAIN) + break; + cond_resched(); + } +} + static void pmu_remove_genpd(struct device *dev) { struct generic_pm_domain *genpd = dev_to_genpd(dev); @@ -256,11 +279,19 @@ static int pmu_platform_call(struct notifier_block *nb, unsigned long event, void *data) { struct device *dev = data; + const char *name = NULL; + + if (strcmp(dev_name(dev), "ap510-vmeta.0") == 0) + name = "vpu"; + else if (strcmp(dev_name(dev), "galcore.0") == 0) + name = "gpu"; switch (event) { case BUS_NOTIFY_ADD_DEVICE: if (dev->of_node) pmu_add_genpd_of(dev); + else if (name) + pmu_add_genpd_name(name, dev); break; case BUS_NOTIFY_DEL_DEVICE: @@ -363,6 +394,49 @@ static int __init dove_init_pmu_irq(struct pmu_data *pmu, int irq) return 0; } +static struct pmu_data pmu_data = { + .lock = __SPIN_LOCK_UNLOCKED(&pmu.lock), + .pmc_base = DOVE_PMU_VIRT_BASE, + .pmu_base = DOVE_PMU_VIRT_BASE + 0x8000, +}; + +static struct pmu_domain vpu_domain = { + .pwr_mask = PMU_PWR_DOWN_VPU, + .rst_mask = PMU_SW_RST_VIDEO_MASK, + .iso_mask = PMU_ISO_VPU, + .base = { + .name = "vpu", + }, +}; + +static struct pmu_domain gpu_domain = { + .pwr_mask = PMU_PWR_DOWN_GPU, + .rst_mask = PMU_SW_RST_GPU_MASK, + .iso_mask = PMU_ISO_GPU, + .base = { + .name = "gpu", + }, +}; + +static int __init dove_pmu_init_legacy(void) +{ + struct pmu_data *pmu = &pmu_data; + int ret, parent_irq; + + pmu_reset_init(pmu); + pmu_domain_register(pmu, &vpu_domain); + pmu_domain_register(pmu, &gpu_domain); + pm_genpd_poweroff_unused(); + + ret = dove_init_pmu_irq(pmu, IRQ_DOVE_PMU); + if (ret) + pr_err("dove_init_pmu_irq() failed: %d\n", ret); + + bus_register_notifier(&platform_bus_type, &platform_nb); + + return 0; +} + /* * pmu { * compatible = "marvell,pmu"; @@ -390,7 +464,7 @@ int __init dove_init_pmu(void) /* Lookup the PMU node */ np_pmu = of_find_compatible_node(NULL, NULL, "marvell,pmu"); if (!np_pmu) - return 0; + return dove_pmu_init_legacy(); pmu = kzalloc(sizeof(*pmu), GFP_KERNEL); if (!pmu) -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH 5/5] ARM: dove: add non-DT PMU support @ 2014-04-27 13:29 ` Russell King 0 siblings, 0 replies; 49+ messages in thread From: Russell King @ 2014-04-27 13:29 UTC (permalink / raw) To: linux-arm-kernel Since Dove has non-DT support still, this patch adds the static data to the PMU driver in order to initialise PMU domains. Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> --- arch/arm/mach-dove/pmu.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 1 deletion(-) diff --git a/arch/arm/mach-dove/pmu.c b/arch/arm/mach-dove/pmu.c index 0b3201fa2d5c..9832044dbab9 100644 --- a/arch/arm/mach-dove/pmu.c +++ b/arch/arm/mach-dove/pmu.c @@ -226,6 +226,20 @@ static void __pmu_domain_register(struct pmu_domain *domain) pm_genpd_init(&domain->base, NULL, !(val & domain->pwr_mask)); } +static void pmu_domain_register(struct pmu_data *pmu, + const struct pmu_domain *pmu_dom) +{ + struct pmu_domain *domain; + + domain = kmemdup(pmu_dom, sizeof(*domain), GFP_KERNEL); + if (!domain) + return; + + domain->pmu = pmu; + + __pmu_domain_register(domain); +} + static void pmu_add_genpd_of(struct device *dev) { struct device_node *node; @@ -241,6 +255,15 @@ static void pmu_add_genpd_of(struct device *dev) } } +static void pmu_add_genpd_name(const char *name, struct device *dev) +{ + while (1) { + if (pm_genpd_name_add_device(name, dev) != -EAGAIN) + break; + cond_resched(); + } +} + static void pmu_remove_genpd(struct device *dev) { struct generic_pm_domain *genpd = dev_to_genpd(dev); @@ -256,11 +279,19 @@ static int pmu_platform_call(struct notifier_block *nb, unsigned long event, void *data) { struct device *dev = data; + const char *name = NULL; + + if (strcmp(dev_name(dev), "ap510-vmeta.0") == 0) + name = "vpu"; + else if (strcmp(dev_name(dev), "galcore.0") == 0) + name = "gpu"; switch (event) { case BUS_NOTIFY_ADD_DEVICE: if (dev->of_node) pmu_add_genpd_of(dev); + else if (name) + pmu_add_genpd_name(name, dev); break; case BUS_NOTIFY_DEL_DEVICE: @@ -363,6 +394,49 @@ static int __init dove_init_pmu_irq(struct pmu_data *pmu, int irq) return 0; } +static struct pmu_data pmu_data = { + .lock = __SPIN_LOCK_UNLOCKED(&pmu.lock), + .pmc_base = DOVE_PMU_VIRT_BASE, + .pmu_base = DOVE_PMU_VIRT_BASE + 0x8000, +}; + +static struct pmu_domain vpu_domain = { + .pwr_mask = PMU_PWR_DOWN_VPU, + .rst_mask = PMU_SW_RST_VIDEO_MASK, + .iso_mask = PMU_ISO_VPU, + .base = { + .name = "vpu", + }, +}; + +static struct pmu_domain gpu_domain = { + .pwr_mask = PMU_PWR_DOWN_GPU, + .rst_mask = PMU_SW_RST_GPU_MASK, + .iso_mask = PMU_ISO_GPU, + .base = { + .name = "gpu", + }, +}; + +static int __init dove_pmu_init_legacy(void) +{ + struct pmu_data *pmu = &pmu_data; + int ret, parent_irq; + + pmu_reset_init(pmu); + pmu_domain_register(pmu, &vpu_domain); + pmu_domain_register(pmu, &gpu_domain); + pm_genpd_poweroff_unused(); + + ret = dove_init_pmu_irq(pmu, IRQ_DOVE_PMU); + if (ret) + pr_err("dove_init_pmu_irq() failed: %d\n", ret); + + bus_register_notifier(&platform_bus_type, &platform_nb); + + return 0; +} + /* * pmu { * compatible = "marvell,pmu"; @@ -390,7 +464,7 @@ int __init dove_init_pmu(void) /* Lookup the PMU node */ np_pmu = of_find_compatible_node(NULL, NULL, "marvell,pmu"); if (!np_pmu) - return 0; + return dove_pmu_init_legacy(); pmu = kzalloc(sizeof(*pmu), GFP_KERNEL); if (!pmu) -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH RFC 0/5] Dove PMU support 2014-04-27 13:23 ` Russell King - ARM Linux @ 2014-04-28 7:47 ` Sebastian Hesselbarth -1 siblings, 0 replies; 49+ messages in thread From: Sebastian Hesselbarth @ 2014-04-28 7:47 UTC (permalink / raw) To: Russell King - ARM Linux, devicetree, linux-arm-kernel, linux-pm, Mark Rutland, Pawel Moll, Rafael J. Wysocki, Rob Herring On 04/27/2014 03:23 PM, Russell King - ARM Linux wrote: > The following series of patches add better PMU support for Dove. This > has been developed on the Cubox, and tested in non-DT and DT modes. > > This also improves the interrupt handling over the existing code: the > existing code ends up calling the interrupt handlers twice for every > interrupt raised, because the interrupt clear-down is done at the > wrong point - we need to clear down the interrupt in the device first, > then clear it down in the controller. > > The problem this gives is that it can be racy (see comments in the > driver) so we're careful about how we do that to minimise the window. > > I've included all patches here - the initial set are targetted towards > adding DT support, with the final adding the non-DT support. There is > a call to the initialisation function missing for DT mode - I'd like > the mvebu people to comment on how that should be handled, as it needs > to be done pretty early. > > Also included are two PM domain changes: the first I've discussed with > Rafael who seems happy with it. The second is necessary because we > have no way to know if a generic PM domain is associated with a device > or whether something else making use of the PM domain is installed in > the dev->pm_domain pointer, so this allows that decision to be made by > core PM code. > > This is more a "this is where I'm at" with this stuff than a real > submission, nevertheless comments on how to get it ready for submission > would be welcome. I'd like to get this off my plate ASAP. Russell, thanks for dropping those patches. I know you are packed with a bunch of other patch sets, so if you agree, I can pick up your Dove related patches and finish them. One thing that comes into my mind is, that we moved Dove DT to mach-mvebu starting with v3.15-rc1 so we need to find a better place for the driver than mach-dove. Sebastian > arch/arm/Kconfig | 1 + > arch/arm/boot/dts/dove.dtsi | 7 + > arch/arm/mach-dove/Makefile | 1 + > arch/arm/mach-dove/common.c | 2 + > arch/arm/mach-dove/common.h | 1 + > arch/arm/mach-dove/include/mach/pm.h | 17 -- > arch/arm/mach-dove/irq.c | 87 ------ > arch/arm/mach-dove/pmu.c | 531 +++++++++++++++++++++++++++++++++++ > drivers/base/power/domain.c | 8 +- > 9 files changed, 547 insertions(+), 108 deletions(-) > ^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH RFC 0/5] Dove PMU support @ 2014-04-28 7:47 ` Sebastian Hesselbarth 0 siblings, 0 replies; 49+ messages in thread From: Sebastian Hesselbarth @ 2014-04-28 7:47 UTC (permalink / raw) To: linux-arm-kernel On 04/27/2014 03:23 PM, Russell King - ARM Linux wrote: > The following series of patches add better PMU support for Dove. This > has been developed on the Cubox, and tested in non-DT and DT modes. > > This also improves the interrupt handling over the existing code: the > existing code ends up calling the interrupt handlers twice for every > interrupt raised, because the interrupt clear-down is done at the > wrong point - we need to clear down the interrupt in the device first, > then clear it down in the controller. > > The problem this gives is that it can be racy (see comments in the > driver) so we're careful about how we do that to minimise the window. > > I've included all patches here - the initial set are targetted towards > adding DT support, with the final adding the non-DT support. There is > a call to the initialisation function missing for DT mode - I'd like > the mvebu people to comment on how that should be handled, as it needs > to be done pretty early. > > Also included are two PM domain changes: the first I've discussed with > Rafael who seems happy with it. The second is necessary because we > have no way to know if a generic PM domain is associated with a device > or whether something else making use of the PM domain is installed in > the dev->pm_domain pointer, so this allows that decision to be made by > core PM code. > > This is more a "this is where I'm at" with this stuff than a real > submission, nevertheless comments on how to get it ready for submission > would be welcome. I'd like to get this off my plate ASAP. Russell, thanks for dropping those patches. I know you are packed with a bunch of other patch sets, so if you agree, I can pick up your Dove related patches and finish them. One thing that comes into my mind is, that we moved Dove DT to mach-mvebu starting with v3.15-rc1 so we need to find a better place for the driver than mach-dove. Sebastian > arch/arm/Kconfig | 1 + > arch/arm/boot/dts/dove.dtsi | 7 + > arch/arm/mach-dove/Makefile | 1 + > arch/arm/mach-dove/common.c | 2 + > arch/arm/mach-dove/common.h | 1 + > arch/arm/mach-dove/include/mach/pm.h | 17 -- > arch/arm/mach-dove/irq.c | 87 ------ > arch/arm/mach-dove/pmu.c | 531 +++++++++++++++++++++++++++++++++++ > drivers/base/power/domain.c | 8 +- > 9 files changed, 547 insertions(+), 108 deletions(-) > ^ permalink raw reply [flat|nested] 49+ messages in thread
[parent not found: <535E079B.6010701-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH RFC 0/5] Dove PMU support 2014-04-28 7:47 ` Sebastian Hesselbarth @ 2014-04-28 8:31 ` Andrew Lunn -1 siblings, 0 replies; 49+ messages in thread From: Andrew Lunn @ 2014-04-28 8:31 UTC (permalink / raw) To: Sebastian Hesselbarth Cc: Russell King - ARM Linux, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-pm-u79uwXL29TY76Z2rM5mHXA, Mark Rutland, Pawel Moll, Rafael J. Wysocki, Rob Herring > Russell, > > thanks for dropping those patches. I know you are packed with a bunch > of other patch sets, so if you agree, I can pick up your Dove related > patches and finish them. > > One thing that comes into my mind is, that we moved Dove DT to > mach-mvebu starting with v3.15-rc1 so we need to find a better place > for the driver than mach-dove. Create drivers/pmu ? The cpufreq driver also needs access to registers within the pmu range. Should it be part of pmu.c, or should we export a regmap which cpufreq can use? Andrew -- 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] 49+ messages in thread
* [PATCH RFC 0/5] Dove PMU support @ 2014-04-28 8:31 ` Andrew Lunn 0 siblings, 0 replies; 49+ messages in thread From: Andrew Lunn @ 2014-04-28 8:31 UTC (permalink / raw) To: linux-arm-kernel > Russell, > > thanks for dropping those patches. I know you are packed with a bunch > of other patch sets, so if you agree, I can pick up your Dove related > patches and finish them. > > One thing that comes into my mind is, that we moved Dove DT to > mach-mvebu starting with v3.15-rc1 so we need to find a better place > for the driver than mach-dove. Create drivers/pmu ? The cpufreq driver also needs access to registers within the pmu range. Should it be part of pmu.c, or should we export a regmap which cpufreq can use? Andrew ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH RFC 0/5] Dove PMU support 2014-04-28 8:31 ` Andrew Lunn @ 2014-04-29 9:15 ` Sebastian Hesselbarth -1 siblings, 0 replies; 49+ messages in thread From: Sebastian Hesselbarth @ 2014-04-29 9:15 UTC (permalink / raw) To: Andrew Lunn Cc: Russell King - ARM Linux, devicetree, linux-arm-kernel, linux-pm, Mark Rutland, Pawel Moll, Rafael J. Wysocki, Rob Herring On 04/28/2014 10:31 AM, Andrew Lunn wrote: >> Russell, >> >> thanks for dropping those patches. I know you are packed with a bunch >> of other patch sets, so if you agree, I can pick up your Dove related >> patches and finish them. >> >> One thing that comes into my mind is, that we moved Dove DT to >> mach-mvebu starting with v3.15-rc1 so we need to find a better place >> for the driver than mach-dove. > > Create drivers/pmu ? Hmm, I see no other folder it could sit in. Maybe, yes. > The cpufreq driver also needs access to registers within the pmu > range. Should it be part of pmu.c, or should we export a regmap which > cpufreq can use? Not only cpufreq but also pinctrl as the first 16 mpps can have PMU related functions mapped to them. Syscon should do the trick. Sebastian ^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH RFC 0/5] Dove PMU support @ 2014-04-29 9:15 ` Sebastian Hesselbarth 0 siblings, 0 replies; 49+ messages in thread From: Sebastian Hesselbarth @ 2014-04-29 9:15 UTC (permalink / raw) To: linux-arm-kernel On 04/28/2014 10:31 AM, Andrew Lunn wrote: >> Russell, >> >> thanks for dropping those patches. I know you are packed with a bunch >> of other patch sets, so if you agree, I can pick up your Dove related >> patches and finish them. >> >> One thing that comes into my mind is, that we moved Dove DT to >> mach-mvebu starting with v3.15-rc1 so we need to find a better place >> for the driver than mach-dove. > > Create drivers/pmu ? Hmm, I see no other folder it could sit in. Maybe, yes. > The cpufreq driver also needs access to registers within the pmu > range. Should it be part of pmu.c, or should we export a regmap which > cpufreq can use? Not only cpufreq but also pinctrl as the first 16 mpps can have PMU related functions mapped to them. Syscon should do the trick. Sebastian ^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH RFC 0/5] Dove PMU support 2014-04-29 9:15 ` Sebastian Hesselbarth @ 2014-06-15 15:25 ` Russell King - ARM Linux -1 siblings, 0 replies; 49+ messages in thread From: Russell King - ARM Linux @ 2014-06-15 15:25 UTC (permalink / raw) To: Sebastian Hesselbarth Cc: Andrew Lunn, devicetree, linux-arm-kernel, linux-pm, Mark Rutland, Pawel Moll, Rafael J. Wysocki, Rob Herring On Tue, Apr 29, 2014 at 11:15:27AM +0200, Sebastian Hesselbarth wrote: > On 04/28/2014 10:31 AM, Andrew Lunn wrote: >>> Russell, >>> >>> thanks for dropping those patches. I know you are packed with a bunch >>> of other patch sets, so if you agree, I can pick up your Dove related >>> patches and finish them. >>> >>> One thing that comes into my mind is, that we moved Dove DT to >>> mach-mvebu starting with v3.15-rc1 so we need to find a better place >>> for the driver than mach-dove. >> >> Create drivers/pmu ? > > Hmm, I see no other folder it could sit in. Maybe, yes. > >> The cpufreq driver also needs access to registers within the pmu >> range. Should it be part of pmu.c, or should we export a regmap which >> cpufreq can use? > > Not only cpufreq but also pinctrl as the first 16 mpps can have PMU > related functions mapped to them. Syscon should do the trick. The thing I don't like about regmap/syscon for this stuff is that it seems pretty easy to violate the statement in 8.6.2.4 when you lock individual register accesses. This statement states that the power-up/power-down sequence for VMeta and the GPU must be completed one at a time. In other words, you can't interleave the power control of these units (which is what could happen if you rely on the locking provided by regmap/syscon.) I'd prefer to take that a little further to avoid any unintended consequences (as indeed I have done in this patch) and ensure that these sequences are complete before any other system is controlled. As for DT, I should probably state that I'm far from happy with the DT situation on Dove. When I've tried it, I've encountered problems with the HDMI output - the picture spends more time blanked than displaying. I've no idea what is causing that, I've been through the SI5351 registers, the TDA998x registers, the LCD controller registers, and I can't find any reason for it. Yet, boot the same kernel without DT and it works fine. Boot back using DT, and it's unstable again. I can't detect any difference on the actual HDMI signals themselves either. The HDMI clock seems stable and of the correct frequency. There is definitely some difference between booting with DT and booting with legacy stuff that seems to upset HDMI. For me, the /only/ reliably working system is one where DT is not involved. This means I remain opposed to any solutions which can't be used in a non-DT environment - at least until the HDMI issue can be resolved. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH RFC 0/5] Dove PMU support @ 2014-06-15 15:25 ` Russell King - ARM Linux 0 siblings, 0 replies; 49+ messages in thread From: Russell King - ARM Linux @ 2014-06-15 15:25 UTC (permalink / raw) To: linux-arm-kernel On Tue, Apr 29, 2014 at 11:15:27AM +0200, Sebastian Hesselbarth wrote: > On 04/28/2014 10:31 AM, Andrew Lunn wrote: >>> Russell, >>> >>> thanks for dropping those patches. I know you are packed with a bunch >>> of other patch sets, so if you agree, I can pick up your Dove related >>> patches and finish them. >>> >>> One thing that comes into my mind is, that we moved Dove DT to >>> mach-mvebu starting with v3.15-rc1 so we need to find a better place >>> for the driver than mach-dove. >> >> Create drivers/pmu ? > > Hmm, I see no other folder it could sit in. Maybe, yes. > >> The cpufreq driver also needs access to registers within the pmu >> range. Should it be part of pmu.c, or should we export a regmap which >> cpufreq can use? > > Not only cpufreq but also pinctrl as the first 16 mpps can have PMU > related functions mapped to them. Syscon should do the trick. The thing I don't like about regmap/syscon for this stuff is that it seems pretty easy to violate the statement in 8.6.2.4 when you lock individual register accesses. This statement states that the power-up/power-down sequence for VMeta and the GPU must be completed one at a time. In other words, you can't interleave the power control of these units (which is what could happen if you rely on the locking provided by regmap/syscon.) I'd prefer to take that a little further to avoid any unintended consequences (as indeed I have done in this patch) and ensure that these sequences are complete before any other system is controlled. As for DT, I should probably state that I'm far from happy with the DT situation on Dove. When I've tried it, I've encountered problems with the HDMI output - the picture spends more time blanked than displaying. I've no idea what is causing that, I've been through the SI5351 registers, the TDA998x registers, the LCD controller registers, and I can't find any reason for it. Yet, boot the same kernel without DT and it works fine. Boot back using DT, and it's unstable again. I can't detect any difference on the actual HDMI signals themselves either. The HDMI clock seems stable and of the correct frequency. There is definitely some difference between booting with DT and booting with legacy stuff that seems to upset HDMI. For me, the /only/ reliably working system is one where DT is not involved. This means I remain opposed to any solutions which can't be used in a non-DT environment - at least until the HDMI issue can be resolved. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ^ permalink raw reply [flat|nested] 49+ messages in thread
* Dove DT and HDMI on v3.16-rc1 (was: Re: [PATCH RFC 0/5] Dove PMU support) 2014-06-15 15:25 ` Russell King - ARM Linux (?) @ 2014-06-18 14:11 ` Sebastian Hesselbarth 2014-06-18 14:34 ` Ezequiel Garcia 2014-06-21 19:39 ` Ezequiel Garcia -1 siblings, 2 replies; 49+ messages in thread From: Sebastian Hesselbarth @ 2014-06-18 14:11 UTC (permalink / raw) To: linux-arm-kernel On 06/15/2014 05:25 PM, Russell King - ARM Linux wrote: > As for DT, I should probably state that I'm far from happy with the DT > situation on Dove. When I've tried it, I've encountered problems with > the HDMI output - the picture spends more time blanked than displaying. > I've no idea what is causing that, I've been through the SI5351 > registers, the TDA998x registers, the LCD controller registers, and I > can't find any reason for it. Yet, boot the same kernel without DT > and it works fine. Boot back using DT, and it's unstable again. > > I can't detect any difference on the actual HDMI signals themselves > either. The HDMI clock seems stable and of the correct frequency. > > There is definitely some difference between booting with DT and booting > with legacy stuff that seems to upset HDMI. > > For me, the /only/ reliably working system is one where DT is not > involved. This means I remain opposed to any solutions which can't > be used in a non-DT environment - at least until the HDMI issue can > be resolved. Russell, as said on IRC, I really want this to be sorted out. I prepared a Dove DT HDMI quick-hack and pushed it to https://github.com/shesselba/linux-dove.git dove-drm-v3.16-rc1 Using your libdrm-armada and xf86-video-armada on Ubuntu raring armhf, I can run Xfce4 on 1920x1080p60 on Dove Cubox without any blanking issues. Using xrandr -s <mode> will fail after 2-3 times but that shouldn't be related to the issues you see. Please test above branch with kernel config at https://dl.dropboxusercontent.com/u/59928252/config-dove-v3.16-rc1-hdmi and report back if issues are still there. Anyone having a Dove CuBox ready, please also test. Russell has a git branch for the required video lib and drivers. Sebastian ^ permalink raw reply [flat|nested] 49+ messages in thread
* Dove DT and HDMI on v3.16-rc1 (was: Re: [PATCH RFC 0/5] Dove PMU support) 2014-06-18 14:11 ` Dove DT and HDMI on v3.16-rc1 (was: Re: [PATCH RFC 0/5] Dove PMU support) Sebastian Hesselbarth @ 2014-06-18 14:34 ` Ezequiel Garcia 2014-06-18 15:01 ` Russell King - ARM Linux 2014-06-21 19:39 ` Ezequiel Garcia 1 sibling, 1 reply; 49+ messages in thread From: Ezequiel Garcia @ 2014-06-18 14:34 UTC (permalink / raw) To: linux-arm-kernel On 18 Jun 04:11 PM, Sebastian Hesselbarth wrote: > On 06/15/2014 05:25 PM, Russell King - ARM Linux wrote: > >As for DT, I should probably state that I'm far from happy with the DT > >situation on Dove. When I've tried it, I've encountered problems with > >the HDMI output - the picture spends more time blanked than displaying. > >I've no idea what is causing that, I've been through the SI5351 > >registers, the TDA998x registers, the LCD controller registers, and I > >can't find any reason for it. Yet, boot the same kernel without DT > >and it works fine. Boot back using DT, and it's unstable again. > > > >I can't detect any difference on the actual HDMI signals themselves > >either. The HDMI clock seems stable and of the correct frequency. > > > >There is definitely some difference between booting with DT and booting > >with legacy stuff that seems to upset HDMI. > > > >For me, the /only/ reliably working system is one where DT is not > >involved. This means I remain opposed to any solutions which can't > >be used in a non-DT environment - at least until the HDMI issue can > >be resolved. > > Russell, > > as said on IRC, I really want this to be sorted out. I prepared a Dove > DT HDMI quick-hack and pushed it to > > https://github.com/shesselba/linux-dove.git dove-drm-v3.16-rc1 > I can probably give a quick test to this using Qt over FB, or just plain fb-test / libdrm tests. But you are already doing a more involved test, so maybe my tests are too simple. > Using your libdrm-armada and xf86-video-armada on Ubuntu raring armhf, > I can run Xfce4 on 1920x1080p60 on Dove Cubox without any blanking > issues. Using xrandr -s <mode> will fail after 2-3 times but that > shouldn't be related to the issues you see. > > Please test above branch with kernel config at > https://dl.dropboxusercontent.com/u/59928252/config-dove-v3.16-rc1-hdmi > and report back if issues are still there. > > Anyone having a Dove CuBox ready, please also test. Russell has a git > branch for the required video lib and drivers. > Can you point me at those? -- Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 49+ messages in thread
* Dove DT and HDMI on v3.16-rc1 (was: Re: [PATCH RFC 0/5] Dove PMU support) 2014-06-18 14:34 ` Ezequiel Garcia @ 2014-06-18 15:01 ` Russell King - ARM Linux 2014-06-18 15:10 ` Dove DT and HDMI on v3.16-rc1 Sebastian Hesselbarth 2014-06-18 16:09 ` Dove DT and HDMI on v3.16-rc1 (was: Re: [PATCH RFC 0/5] Dove PMU support) Nicolas Pitre 0 siblings, 2 replies; 49+ messages in thread From: Russell King - ARM Linux @ 2014-06-18 15:01 UTC (permalink / raw) To: linux-arm-kernel On Wed, Jun 18, 2014 at 11:34:48AM -0300, Ezequiel Garcia wrote: > On 18 Jun 04:11 PM, Sebastian Hesselbarth wrote: > > Using your libdrm-armada and xf86-video-armada on Ubuntu raring armhf, > > I can run Xfce4 on 1920x1080p60 on Dove Cubox without any blanking > > issues. Using xrandr -s <mode> will fail after 2-3 times but that > > shouldn't be related to the issues you see. > > > > Please test above branch with kernel config at > > https://dl.dropboxusercontent.com/u/59928252/config-dove-v3.16-rc1-hdmi > > and report back if issues are still there. > > > > Anyone having a Dove CuBox ready, please also test. Russell has a git > > branch for the required video lib and drivers. > > > > Can you point me at those? Beware - the machine is rather aged and slow by todays standards ((c)git is rather heavy weight): http://ftp.arm.linux.org.uk/cgit/ The xf86-video-armada.git tree is not trivial to build as there's dependencies on the Vivante galcore libraries - and even if you have them, there's then there's non-trivial changes to the kernel-side driver to support dmabuf imports. Welcome to the world of closed source graphics libraries making open source hard... I've been wishing for etnaviv for a while now... Even though the kernel side is supposed to be GPL, I'm really not happy distributing it or publishing changes as there are a number of files with headers which seem to be GPL-incompatible (which I've eliminated from my tree through updates) but the problem is wonderful git keeps them as history... and in my tree it's a massive 93 patches, against an old version of the code (0.8.0.1998) which I then sort-of updated to the version OLPC were carrying towards the start of 2013. It's probably best if Sebastian walks you through getting it up and running as my experience is with my kernels and userspace, which has all the necessary hacks in. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ^ permalink raw reply [flat|nested] 49+ messages in thread
* Dove DT and HDMI on v3.16-rc1 2014-06-18 15:01 ` Russell King - ARM Linux @ 2014-06-18 15:10 ` Sebastian Hesselbarth 2014-06-18 15:30 ` Russell King - ARM Linux 2014-06-18 16:09 ` Dove DT and HDMI on v3.16-rc1 (was: Re: [PATCH RFC 0/5] Dove PMU support) Nicolas Pitre 1 sibling, 1 reply; 49+ messages in thread From: Sebastian Hesselbarth @ 2014-06-18 15:10 UTC (permalink / raw) To: linux-arm-kernel On 06/18/2014 05:01 PM, Russell King - ARM Linux wrote: > On Wed, Jun 18, 2014 at 11:34:48AM -0300, Ezequiel Garcia wrote: >> On 18 Jun 04:11 PM, Sebastian Hesselbarth wrote: >>> Using your libdrm-armada and xf86-video-armada on Ubuntu raring armhf, >>> I can run Xfce4 on 1920x1080p60 on Dove Cubox without any blanking >>> issues. Using xrandr -s <mode> will fail after 2-3 times but that >>> shouldn't be related to the issues you see. >>> >>> Please test above branch with kernel config at >>> https://dl.dropboxusercontent.com/u/59928252/config-dove-v3.16-rc1-hdmi >>> and report back if issues are still there. >>> >>> Anyone having a Dove CuBox ready, please also test. Russell has a git >>> branch for the required video lib and drivers. >>> >> >> Can you point me at those? > > Beware - the machine is rather aged and slow by todays standards ((c)git > is rather heavy weight): > > http://ftp.arm.linux.org.uk/cgit/ > > The xf86-video-armada.git tree is not trivial to build as there's > dependencies on the Vivante galcore libraries - and even if you have > them, there's then there's non-trivial changes to the kernel-side driver > to support dmabuf imports. Welcome to the world of closed source graphics > libraries making open source hard... I've been wishing for etnaviv for > a while now... > > Even though the kernel side is supposed to be GPL, I'm really not happy > distributing it or publishing changes as there are a number of files with > headers which seem to be GPL-incompatible (which I've eliminated from my > tree through updates) but the problem is wonderful git keeps them as > history... and in my tree it's a massive 93 patches, against an old > version of the code (0.8.0.1998) which I then sort-of updated to the > version OLPC were carrying towards the start of 2013. > > It's probably best if Sebastian walks you through getting it up and > running as my experience is with my kernels and userspace, which has all > the necessary hacks in. I basically followed Russell's guide from http://www.home.arm.linux.org.uk/~rmk/cubox/carrier-1-readme.txt with this binary/include package from SolidRun http://download.solid-run.com/pub/solidrun/cubox/packages/marvell-opengl/marvell-opengl-hardfp-new-headers.zip xf86-video-armada needs some missing #include "picturestr.h" for raring, but whole process has been very straight forward. Feel free to bug me on IRC anytime you hit some build errors or anything else. Sebastian ^ permalink raw reply [flat|nested] 49+ messages in thread
* Dove DT and HDMI on v3.16-rc1 2014-06-18 15:10 ` Dove DT and HDMI on v3.16-rc1 Sebastian Hesselbarth @ 2014-06-18 15:30 ` Russell King - ARM Linux 2014-06-18 15:39 ` Sebastian Hesselbarth 0 siblings, 1 reply; 49+ messages in thread From: Russell King - ARM Linux @ 2014-06-18 15:30 UTC (permalink / raw) To: linux-arm-kernel On Wed, Jun 18, 2014 at 05:10:32PM +0200, Sebastian Hesselbarth wrote: > xf86-video-armada needs some missing > #include "picturestr.h" > for raring, but whole process has been very straight forward. I'm only aware of this (sorry, copy'n'pasted) which I found when trying to build against ubuntu 14.04. If further patches are needed to make it compatible with more Xorg versions, please ensure that they find their way into my inbox... diff --git a/src/common_drm.h b/src/common_drm.h index b0c3cd4..3bc5dc6 100644 --- a/src/common_drm.h +++ b/src/common_drm.h @@ -1,8 +1,8 @@ #ifndef COMMON_DRM_H #define COMMON_DRM_H -#include "compat-api.h" #include "xf86Crtc.h" +#include "compat-api.h" struct common_crtc_info { int drm_fd; diff --git a/src/vivante_unaccel_render.c b/src/vivante_unaccel_render.c index ad4d5cf..0496d7a 100644 --- a/src/vivante_unaccel_render.c +++ b/src/vivante_unaccel_render.c @@ -11,11 +11,11 @@ #include "config.h" #endif -#include "compat-api.h" #include "fb.h" #include "fbpict.h" #include "mipict.h" +#include "compat-api.h" #include "vivante_unaccel.h" #include "vivante_utils.h" -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ^ permalink raw reply related [flat|nested] 49+ messages in thread
* Dove DT and HDMI on v3.16-rc1 2014-06-18 15:30 ` Russell King - ARM Linux @ 2014-06-18 15:39 ` Sebastian Hesselbarth 2014-06-18 15:59 ` Russell King - ARM Linux 0 siblings, 1 reply; 49+ messages in thread From: Sebastian Hesselbarth @ 2014-06-18 15:39 UTC (permalink / raw) To: linux-arm-kernel On 06/18/2014 05:30 PM, Russell King - ARM Linux wrote: > On Wed, Jun 18, 2014 at 05:10:32PM +0200, Sebastian Hesselbarth wrote: >> xf86-video-armada needs some missing >> #include "picturestr.h" >> for raring, but whole process has been very straight forward. > > I'm only aware of this (sorry, copy'n'pasted) which I found when trying > to build against ubuntu 14.04. If further patches are needed to make it > compatible with more Xorg versions, please ensure that they find their > way into my inbox... Your patch also makes the xorg driver build fine on raring. When I encountered the build error, I just tried the first hit on Google - I didn't investigate it at all. Sebastian ^ permalink raw reply [flat|nested] 49+ messages in thread
* Dove DT and HDMI on v3.16-rc1 2014-06-18 15:39 ` Sebastian Hesselbarth @ 2014-06-18 15:59 ` Russell King - ARM Linux 0 siblings, 0 replies; 49+ messages in thread From: Russell King - ARM Linux @ 2014-06-18 15:59 UTC (permalink / raw) To: linux-arm-kernel On Wed, Jun 18, 2014 at 05:39:08PM +0200, Sebastian Hesselbarth wrote: > On 06/18/2014 05:30 PM, Russell King - ARM Linux wrote: >> On Wed, Jun 18, 2014 at 05:10:32PM +0200, Sebastian Hesselbarth wrote: >>> xf86-video-armada needs some missing >>> #include "picturestr.h" >>> for raring, but whole process has been very straight forward. >> >> I'm only aware of this (sorry, copy'n'pasted) which I found when trying >> to build against ubuntu 14.04. If further patches are needed to make it >> compatible with more Xorg versions, please ensure that they find their >> way into my inbox... > > Your patch also makes the xorg driver build fine on raring. > When I encountered the build error, I just tried the first hit on Google > - I didn't investigate it at all. Thanks, just pushed that change out. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ^ permalink raw reply [flat|nested] 49+ messages in thread
* Dove DT and HDMI on v3.16-rc1 (was: Re: [PATCH RFC 0/5] Dove PMU support) 2014-06-18 15:01 ` Russell King - ARM Linux 2014-06-18 15:10 ` Dove DT and HDMI on v3.16-rc1 Sebastian Hesselbarth @ 2014-06-18 16:09 ` Nicolas Pitre 1 sibling, 0 replies; 49+ messages in thread From: Nicolas Pitre @ 2014-06-18 16:09 UTC (permalink / raw) To: linux-arm-kernel On Wed, 18 Jun 2014, Russell King - ARM Linux wrote: > Even though the kernel side is supposed to be GPL, I'm really not happy > distributing it or publishing changes as there are a number of files with > headers which seem to be GPL-incompatible (which I've eliminated from my > tree through updates) but the problem is wonderful git keeps them as > history... and in my tree it's a massive 93 patches, against an old > version of the code (0.8.0.1998) which I then sort-of updated to the > version OLPC were carrying towards the start of 2013. If you want to remove some files from git history, you may look at the git-filter-branch man page. The first provided examples are most likely what you'd need. Nicolas ^ permalink raw reply [flat|nested] 49+ messages in thread
* Dove DT and HDMI on v3.16-rc1 (was: Re: [PATCH RFC 0/5] Dove PMU support) 2014-06-18 14:11 ` Dove DT and HDMI on v3.16-rc1 (was: Re: [PATCH RFC 0/5] Dove PMU support) Sebastian Hesselbarth 2014-06-18 14:34 ` Ezequiel Garcia @ 2014-06-21 19:39 ` Ezequiel Garcia 1 sibling, 0 replies; 49+ messages in thread From: Ezequiel Garcia @ 2014-06-21 19:39 UTC (permalink / raw) To: linux-arm-kernel On 18 Jun 04:11 PM, Sebastian Hesselbarth wrote: [..] > > as said on IRC, I really want this to be sorted out. I prepared a Dove > DT HDMI quick-hack and pushed it to > > https://github.com/shesselba/linux-dove.git dove-drm-v3.16-rc1 > Tested the above branch using mvebu_v7_defconfig + Cubox-specific options and ran a few basic tests. I know it's not much, but here they are: * The bootlogo shows up * Legacy fbdev tests (fb-test, fb-test-rect, mplayer) worked fine in the default mode (1920x1080 in my case). * Set a few different modes with LibDRM modetest tool: modetest -M armada-drm -s 18:640x480 modetest -M armada-drm -s 18:800x600 ... modetest -M armada-drm -s 18:1920x1080 -- Ezequiel Garc?a, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 49+ messages in thread
end of thread, other threads:[~2015-02-13 16:12 UTC | newest] Thread overview: 49+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-04-27 13:23 [PATCH RFC 0/5] Dove PMU support Russell King - ARM Linux 2014-04-27 13:23 ` Russell King - ARM Linux [not found] ` <20140427132312.GC26756-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org> 2014-04-27 13:28 ` [PATCH 1/5] pm: domains: quieten down generic pm domains Russell King 2014-04-27 13:28 ` Russell King 2014-04-30 23:46 ` Rafael J. Wysocki 2014-04-30 23:46 ` Rafael J. Wysocki 2014-05-02 9:24 ` Ulf Hansson 2014-05-02 9:24 ` Ulf Hansson 2014-05-04 22:36 ` Rafael J. Wysocki 2014-05-04 22:36 ` Rafael J. Wysocki [not found] ` <1412882.XTDX0QPJ6V-sKB8Sp2ER+y1GS7QM15AGw@public.gmane.org> 2015-02-13 11:54 ` Russell King - ARM Linux 2015-02-13 11:54 ` Russell King - ARM Linux 2014-04-27 13:28 ` [PATCH 2/5] pm: domains: avoid potential oops in pm_genpd_remove_device() Russell King 2014-04-27 13:28 ` Russell King 2014-04-30 23:46 ` Rafael J. Wysocki 2014-04-30 23:46 ` Rafael J. Wysocki 2014-04-27 13:29 ` [PATCH 3/5] ARM: dove: create a proper PMU driver for power domains, PMU IRQs and resets Russell King 2014-04-27 13:29 ` Russell King 2014-04-28 11:55 ` Ulf Hansson 2014-04-28 11:55 ` Ulf Hansson 2014-04-28 12:17 ` Russell King - ARM Linux 2014-04-28 12:17 ` Russell King - ARM Linux 2015-02-13 13:29 ` Russell King - ARM Linux 2015-02-13 13:29 ` Russell King - ARM Linux 2015-02-13 14:11 ` Russell King - ARM Linux 2015-02-13 14:11 ` Russell King - ARM Linux 2015-02-13 16:12 ` Russell King - ARM Linux 2015-02-13 16:12 ` Russell King - ARM Linux 2014-04-27 13:29 ` [PATCH 4/5] ARM: dove: add Dove PMU DT entries to dove.dtsi Russell King 2014-04-27 13:29 ` Russell King 2014-04-27 13:29 ` [PATCH 5/5] ARM: dove: add non-DT PMU support Russell King 2014-04-27 13:29 ` Russell King 2014-04-28 7:47 ` [PATCH RFC 0/5] Dove " Sebastian Hesselbarth 2014-04-28 7:47 ` Sebastian Hesselbarth [not found] ` <535E079B.6010701-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2014-04-28 8:31 ` Andrew Lunn 2014-04-28 8:31 ` Andrew Lunn 2014-04-29 9:15 ` Sebastian Hesselbarth 2014-04-29 9:15 ` Sebastian Hesselbarth 2014-06-15 15:25 ` Russell King - ARM Linux 2014-06-15 15:25 ` Russell King - ARM Linux 2014-06-18 14:11 ` Dove DT and HDMI on v3.16-rc1 (was: Re: [PATCH RFC 0/5] Dove PMU support) Sebastian Hesselbarth 2014-06-18 14:34 ` Ezequiel Garcia 2014-06-18 15:01 ` Russell King - ARM Linux 2014-06-18 15:10 ` Dove DT and HDMI on v3.16-rc1 Sebastian Hesselbarth 2014-06-18 15:30 ` Russell King - ARM Linux 2014-06-18 15:39 ` Sebastian Hesselbarth 2014-06-18 15:59 ` Russell King - ARM Linux 2014-06-18 16:09 ` Dove DT and HDMI on v3.16-rc1 (was: Re: [PATCH RFC 0/5] Dove PMU support) Nicolas Pitre 2014-06-21 19:39 ` Ezequiel Garcia
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.