* Re: [PATCH/RFC] ARM: amba: Remove AMBA level regulator support
@ 2012-04-01 17:22 ` Linus Walleij
0 siblings, 0 replies; 10+ messages in thread
From: Linus Walleij @ 2012-04-01 17:22 UTC (permalink / raw)
To: Mark Brown
Cc: Russell King, Grant Likely, linux-arm-kernel, spi-devel-general,
linux-kernel
On Sun, Apr 1, 2012 at 5:29 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> The AMBA bus regulator support is being used to model on/off switches
> for power domains which isn't terribly idiomatic for modern kernels with
> the generic power domain code and creates integration problems on platforms
> which don't use regulators for their power domains as it's hard to tell
> the difference between a regulator that is needed but failed to be provided
> and one that isn't supposed to be there (though DT does make that easier).
>
> Platforms that wish to use the regulator API to manage their power domains
> can indirect via the power domain interface.
I don't see how this solves the problem of AMBA PrimeCell probing.
If you look at the code in bus.c, you can see that it acquires and
enables the regulator - as it does with the clock.
The reason is that is does this *before* the device can probe,
since it needs to map up the memory to read out some magic
values at the end to figure out what device it is in the first place.
We need the current code replaced with something that
enables a power domain before probe instead, then implement
these power domains for the in-kernel AMBA devices that need it.
Is the default behaviour of power domains such that they will
be enabled as soon as devices are registered but before
any buses probe()? Because that is what is needed in this case.
(AMBA devices are special in this way: no other ARM things
support auto-detection of devices using magic numbers,
basically the DT stuff came about because noone was using
a thing like this.)
> The impact should be minimal since currently there are no mainline
> systems which actually provide a vcore regulator so none need updating.
Oh yes there are:
drivers/mfd/db8500-prcmu.c
This driver registers a number of voltage domain regulators,
among those:
static struct regulator_consumer_supply db8500_vape_consumers[] = {
(...)
REGULATOR_SUPPLY("vcore", "sdi0"),
REGULATOR_SUPPLY("vcore", "sdi1"),
REGULATOR_SUPPLY("vcore", "sdi2"),
REGULATOR_SUPPLY("vcore", "sdi3"),
REGULATOR_SUPPLY("vcore", "sdi4"),
(...)
REGULATOR_SUPPLY("vcore", "uart0"),
REGULATOR_SUPPLY("vcore", "uart1"),
REGULATOR_SUPPLY("vcore", "uart2"),
(I know the supplies should probably be moved up to the
platform but there they are.) The first array is MMC controllers
and the second is a number of UARTs.
IIRC the machine will not boot (i.e. these drivers cannot even
probe) without these regulators in place, so they get enabled by
the AMBA bus code.
So we need something that not just removes stuff from the
AMBA bus, but also adds a better power domain mechanism
and fixes up taking the above regulators.
That said I'm all for replacing it - but I'd need to figure out the
details on how to do that.
We do have code for ux500 power domains. If it will will be
enough to handle this I can try to hack it up and submit it.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH/RFC] ARM: amba: Remove AMBA level regulator support
@ 2012-04-01 17:22 ` Linus Walleij
0 siblings, 0 replies; 10+ messages in thread
From: Linus Walleij @ 2012-04-01 17:22 UTC (permalink / raw)
To: linux-arm-kernel
On Sun, Apr 1, 2012 at 5:29 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> The AMBA bus regulator support is being used to model on/off switches
> for power domains which isn't terribly idiomatic for modern kernels with
> the generic power domain code and creates integration problems on platforms
> which don't use regulators for their power domains as it's hard to tell
> the difference between a regulator that is needed but failed to be provided
> and one that isn't supposed to be there (though DT does make that easier).
>
> Platforms that wish to use the regulator API to manage their power domains
> can indirect via the power domain interface.
I don't see how this solves the problem of AMBA PrimeCell probing.
If you look at the code in bus.c, you can see that it acquires and
enables the regulator - as it does with the clock.
The reason is that is does this *before* the device can probe,
since it needs to map up the memory to read out some magic
values at the end to figure out what device it is in the first place.
We need the current code replaced with something that
enables a power domain before probe instead, then implement
these power domains for the in-kernel AMBA devices that need it.
Is the default behaviour of power domains such that they will
be enabled as soon as devices are registered but before
any buses probe()? Because that is what is needed in this case.
(AMBA devices are special in this way: no other ARM things
support auto-detection of devices using magic numbers,
basically the DT stuff came about because noone was using
a thing like this.)
> The impact should be minimal since currently there are no mainline
> systems which actually provide a vcore regulator so none need updating.
Oh yes there are:
drivers/mfd/db8500-prcmu.c
This driver registers a number of voltage domain regulators,
among those:
static struct regulator_consumer_supply db8500_vape_consumers[] = {
(...)
REGULATOR_SUPPLY("vcore", "sdi0"),
REGULATOR_SUPPLY("vcore", "sdi1"),
REGULATOR_SUPPLY("vcore", "sdi2"),
REGULATOR_SUPPLY("vcore", "sdi3"),
REGULATOR_SUPPLY("vcore", "sdi4"),
(...)
REGULATOR_SUPPLY("vcore", "uart0"),
REGULATOR_SUPPLY("vcore", "uart1"),
REGULATOR_SUPPLY("vcore", "uart2"),
(I know the supplies should probably be moved up to the
platform but there they are.) The first array is MMC controllers
and the second is a number of UARTs.
IIRC the machine will not boot (i.e. these drivers cannot even
probe) without these regulators in place, so they get enabled by
the AMBA bus code.
So we need something that not just removes stuff from the
AMBA bus, but also adds a better power domain mechanism
and fixes up taking the above regulators.
That said I'm all for replacing it - but I'd need to figure out the
details on how to do that.
We do have code for ux500 power domains. If it will will be
enough to handle this I can try to hack it up and submit it.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH/RFC] ARM: amba: Remove AMBA level regulator support
2012-04-01 17:22 ` Linus Walleij
@ 2012-04-01 18:54 ` Mark Brown
-1 siblings, 0 replies; 10+ messages in thread
From: Mark Brown @ 2012-04-01 18:54 UTC (permalink / raw)
To: Linus Walleij
Cc: Russell King, Grant Likely, linux-arm-kernel, spi-devel-general,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 3787 bytes --]
On Sun, Apr 01, 2012 at 07:22:46PM +0200, Linus Walleij wrote:
> On Sun, Apr 1, 2012 at 5:29 PM, Mark Brown
> > The AMBA bus regulator support is being used to model on/off switches
> > for power domains which isn't terribly idiomatic for modern kernels with
> I don't see how this solves the problem of AMBA PrimeCell probing.
I think you're misunderstanding what this fixes. The main problem it
addresses is that as things stand platforms which have no intention of
using regulators to model power domains really ought to be providing
dummy vcores for their AMBA devices (or the AMBA bus should be doing
that). Removing regulator usage from the AMBA core code obviously
accomplishes this, avoiding disruption to these platforms.
It does also remove the dodgy ignore the error idiom, but that's pretty
much secondary.
> We need the current code replaced with something that
> enables a power domain before probe instead, then implement
> these power domains for the in-kernel AMBA devices that need it.
> Is the default behaviour of power domains such that they will
> be enabled as soon as devices are registered but before
> any buses probe()? Because that is what is needed in this case.
Yes, this should be the case (TBH I can't actually remember if you have
to do the default in your power domain code or you just get to pick the
default state and the power domain figures it out). If you think about
it power domains would be pretty useless if they didn't do something
sensible here - it's not like the need to power things on before
interacting with them is specific to AMBA. As I keep saying they're
*really* simple to use and very idiomatic, drivers don't ever need to
interact with them directly at all. They just do system and runtime
power management.
> (AMBA devices are special in this way: no other ARM things
> support auto-detection of devices using magic numbers,
> basically the DT stuff came about because noone was using
> a thing like this.)
There's at least PCI and USB as well, with more to come very soon, and
on-SoC platform devices behave in very much the same way really.
> > The impact should be minimal since currently there are no mainline
> > systems which actually provide a vcore regulator so none need updating.
> Oh yes there are:
> drivers/mfd/db8500-prcmu.c
Oh, ick. As you say this stuff would obviously be expected to be in
arch/arm along with the rest of the platform code and quite frankly I'm
surprised that AMBA is producing textual dev_names - it's rather unusual
for a bus not to use the numeric IDs which it enumerates with which is
why I missed the code when I grepped. The names aren't a problem, just
surprising.
> IIRC the machine will not boot (i.e. these drivers cannot even
> probe) without these regulators in place, so they get enabled by
> the AMBA bus code.
> So we need something that not just removes stuff from the
> AMBA bus, but also adds a better power domain mechanism
> and fixes up taking the above regulators.
> That said I'm all for replacing it - but I'd need to figure out the
> details on how to do that.
> We do have code for ux500 power domains. If it will will be
> enough to handle this I can try to hack it up and submit it.
This should be totally trivial, just shift your existing regulator stuff
into the power domain and out of AMBA.
Though actually given the fact that only the pl022 driver supports
turning the domain on and off at runtime and that's not one of the
drivers bound to the switchable supply in this system you'll probably
see zero impact on actual systems if you just constrain vape to be
always_on in the first instance, then work to optimise later. With the
current mainline code I'd expect there to be at least one AMBA device
which forces the power on anyway.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH/RFC] ARM: amba: Remove AMBA level regulator support
@ 2012-04-01 18:54 ` Mark Brown
0 siblings, 0 replies; 10+ messages in thread
From: Mark Brown @ 2012-04-01 18:54 UTC (permalink / raw)
To: linux-arm-kernel
On Sun, Apr 01, 2012 at 07:22:46PM +0200, Linus Walleij wrote:
> On Sun, Apr 1, 2012 at 5:29 PM, Mark Brown
> > The AMBA bus regulator support is being used to model on/off switches
> > for power domains which isn't terribly idiomatic for modern kernels with
> I don't see how this solves the problem of AMBA PrimeCell probing.
I think you're misunderstanding what this fixes. The main problem it
addresses is that as things stand platforms which have no intention of
using regulators to model power domains really ought to be providing
dummy vcores for their AMBA devices (or the AMBA bus should be doing
that). Removing regulator usage from the AMBA core code obviously
accomplishes this, avoiding disruption to these platforms.
It does also remove the dodgy ignore the error idiom, but that's pretty
much secondary.
> We need the current code replaced with something that
> enables a power domain before probe instead, then implement
> these power domains for the in-kernel AMBA devices that need it.
> Is the default behaviour of power domains such that they will
> be enabled as soon as devices are registered but before
> any buses probe()? Because that is what is needed in this case.
Yes, this should be the case (TBH I can't actually remember if you have
to do the default in your power domain code or you just get to pick the
default state and the power domain figures it out). If you think about
it power domains would be pretty useless if they didn't do something
sensible here - it's not like the need to power things on before
interacting with them is specific to AMBA. As I keep saying they're
*really* simple to use and very idiomatic, drivers don't ever need to
interact with them directly at all. They just do system and runtime
power management.
> (AMBA devices are special in this way: no other ARM things
> support auto-detection of devices using magic numbers,
> basically the DT stuff came about because noone was using
> a thing like this.)
There's at least PCI and USB as well, with more to come very soon, and
on-SoC platform devices behave in very much the same way really.
> > The impact should be minimal since currently there are no mainline
> > systems which actually provide a vcore regulator so none need updating.
> Oh yes there are:
> drivers/mfd/db8500-prcmu.c
Oh, ick. As you say this stuff would obviously be expected to be in
arch/arm along with the rest of the platform code and quite frankly I'm
surprised that AMBA is producing textual dev_names - it's rather unusual
for a bus not to use the numeric IDs which it enumerates with which is
why I missed the code when I grepped. The names aren't a problem, just
surprising.
> IIRC the machine will not boot (i.e. these drivers cannot even
> probe) without these regulators in place, so they get enabled by
> the AMBA bus code.
> So we need something that not just removes stuff from the
> AMBA bus, but also adds a better power domain mechanism
> and fixes up taking the above regulators.
> That said I'm all for replacing it - but I'd need to figure out the
> details on how to do that.
> We do have code for ux500 power domains. If it will will be
> enough to handle this I can try to hack it up and submit it.
This should be totally trivial, just shift your existing regulator stuff
into the power domain and out of AMBA.
Though actually given the fact that only the pl022 driver supports
turning the domain on and off at runtime and that's not one of the
drivers bound to the switchable supply in this system you'll probably
see zero impact on actual systems if you just constrain vape to be
always_on in the first instance, then work to optimise later. With the
current mainline code I'd expect there to be at least one AMBA device
which forces the power on anyway.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120401/f72e642f/attachment.sig>
^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <CACRpkdb6+2SLm1Ojw2jNkK9ejW66fw2Pcit-b3AL67wP4Hr_ow-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH/RFC] ARM: amba: Remove AMBA level regulator support
2012-04-01 17:22 ` Linus Walleij
(?)
@ 2012-04-01 19:32 ` Rabin Vincent
-1 siblings, 0 replies; 10+ messages in thread
From: Rabin Vincent @ 2012-04-01 19:32 UTC (permalink / raw)
To: Linus Walleij
Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Mark Brown,
Russell King, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
On Sun, Apr 1, 2012 at 22:52, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Sun, Apr 1, 2012 at 5:29 PM, Mark Brown
> <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org> wrote:
>> The impact should be minimal since currently there are no mainline
>> systems which actually provide a vcore regulator so none need updating.
>
> Oh yes there are:
> drivers/mfd/db8500-prcmu.c
>
> This driver registers a number of voltage domain regulators,
> among those:
>
> static struct regulator_consumer_supply db8500_vape_consumers[] = {
> (...)
> REGULATOR_SUPPLY("vcore", "sdi0"),
> REGULATOR_SUPPLY("vcore", "sdi1"),
> REGULATOR_SUPPLY("vcore", "sdi2"),
> REGULATOR_SUPPLY("vcore", "sdi3"),
> REGULATOR_SUPPLY("vcore", "sdi4"),
> (...)
> REGULATOR_SUPPLY("vcore", "uart0"),
> REGULATOR_SUPPLY("vcore", "uart1"),
> REGULATOR_SUPPLY("vcore", "uart2"),
>
> IIRC the machine will not boot (i.e. these drivers cannot even
> probe) without these regulators in place, so they get enabled by
> the AMBA bus code.
These vcore "regulators" do nothing but increment a variable which is
write-only in mainline, so the machine will boot and drivers will probe
fine with Mark's patch.
------------------------------------------------------------------------------
This SF email is sponsosred by:
Try Windows Azure free for 90 days Click Here
http://p.sf.net/sfu/sfd2d-msazure
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH/RFC] ARM: amba: Remove AMBA level regulator support
@ 2012-04-01 19:32 ` Rabin Vincent
0 siblings, 0 replies; 10+ messages in thread
From: Rabin Vincent @ 2012-04-01 19:32 UTC (permalink / raw)
To: Linus Walleij
Cc: Mark Brown, spi-devel-general, Russell King, linux-kernel,
linux-arm-kernel
On Sun, Apr 1, 2012 at 22:52, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Sun, Apr 1, 2012 at 5:29 PM, Mark Brown
> <broonie@opensource.wolfsonmicro.com> wrote:
>> The impact should be minimal since currently there are no mainline
>> systems which actually provide a vcore regulator so none need updating.
>
> Oh yes there are:
> drivers/mfd/db8500-prcmu.c
>
> This driver registers a number of voltage domain regulators,
> among those:
>
> static struct regulator_consumer_supply db8500_vape_consumers[] = {
> (...)
> REGULATOR_SUPPLY("vcore", "sdi0"),
> REGULATOR_SUPPLY("vcore", "sdi1"),
> REGULATOR_SUPPLY("vcore", "sdi2"),
> REGULATOR_SUPPLY("vcore", "sdi3"),
> REGULATOR_SUPPLY("vcore", "sdi4"),
> (...)
> REGULATOR_SUPPLY("vcore", "uart0"),
> REGULATOR_SUPPLY("vcore", "uart1"),
> REGULATOR_SUPPLY("vcore", "uart2"),
>
> IIRC the machine will not boot (i.e. these drivers cannot even
> probe) without these regulators in place, so they get enabled by
> the AMBA bus code.
These vcore "regulators" do nothing but increment a variable which is
write-only in mainline, so the machine will boot and drivers will probe
fine with Mark's patch.
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH/RFC] ARM: amba: Remove AMBA level regulator support
@ 2012-04-01 19:32 ` Rabin Vincent
0 siblings, 0 replies; 10+ messages in thread
From: Rabin Vincent @ 2012-04-01 19:32 UTC (permalink / raw)
To: linux-arm-kernel
On Sun, Apr 1, 2012 at 22:52, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Sun, Apr 1, 2012 at 5:29 PM, Mark Brown
> <broonie@opensource.wolfsonmicro.com> wrote:
>> The impact should be minimal since currently there are no mainline
>> systems which actually provide a vcore regulator so none need updating.
>
> Oh yes there are:
> drivers/mfd/db8500-prcmu.c
>
> This driver registers a number of voltage domain regulators,
> among those:
>
> static struct regulator_consumer_supply db8500_vape_consumers[] = {
> ? ? ? (...)
> ? ? ? ?REGULATOR_SUPPLY("vcore", "sdi0"),
> ? ? ? ?REGULATOR_SUPPLY("vcore", "sdi1"),
> ? ? ? ?REGULATOR_SUPPLY("vcore", "sdi2"),
> ? ? ? ?REGULATOR_SUPPLY("vcore", "sdi3"),
> ? ? ? ?REGULATOR_SUPPLY("vcore", "sdi4"),
> ? ? ? ?(...)
> ? ? ? ?REGULATOR_SUPPLY("vcore", "uart0"),
> ? ? ? ?REGULATOR_SUPPLY("vcore", "uart1"),
> ? ? ? ?REGULATOR_SUPPLY("vcore", "uart2"),
>
> IIRC the machine will not boot (i.e. these drivers cannot even
> probe) without these regulators in place, so they get enabled by
> the AMBA bus code.
These vcore "regulators" do nothing but increment a variable which is
write-only in mainline, so the machine will boot and drivers will probe
fine with Mark's patch.
^ permalink raw reply [flat|nested] 10+ messages in thread