All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] regulator: core: Keep boot_on regulators powered during init
@ 2012-04-23  9:37 Ulf Hansson
  2012-04-23 10:18 ` Mark Brown
  0 siblings, 1 reply; 20+ messages in thread
From: Ulf Hansson @ 2012-04-23  9:37 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, linux-kernel
  Cc: Mattias Wallin, Jonas Aberg, Lee Jones, Ulf Hansson

Regulators which has boot_on constraints set, will now remain
powered after regulator_init_complete is done.

In this case we leave the enable->disable operation to be
handled by the regulator consumer instead.

Signed-off-by: Ulf Hansson <ulf.hansson@stericsson.com>
---
 drivers/regulator/core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 1caada2..c5af6d2 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3393,7 +3393,7 @@ static int __init regulator_init_complete(void)
 		ops = rdev->desc->ops;
 		c = rdev->constraints;
 
-		if (!ops->disable || (c && c->always_on))
+		if (!ops->disable || (c && (c->always_on || c->boot_on)))
 			continue;
 
 		mutex_lock(&rdev->mutex);
-- 
1.7.9


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH] regulator: core: Keep boot_on regulators powered during init
  2012-04-23  9:37 [PATCH] regulator: core: Keep boot_on regulators powered during init Ulf Hansson
@ 2012-04-23 10:18 ` Mark Brown
  2012-04-23 10:52   ` Ulf Hansson
  0 siblings, 1 reply; 20+ messages in thread
From: Mark Brown @ 2012-04-23 10:18 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Liam Girdwood, linux-kernel, Mattias Wallin, Jonas Aberg, Lee Jones

[-- Attachment #1: Type: text/plain, Size: 921 bytes --]

On Mon, Apr 23, 2012 at 11:37:53AM +0200, Ulf Hansson wrote:

> Regulators which has boot_on constraints set, will now remain
> powered after regulator_init_complete is done.

This would be a bug.  All boot_on means is that the regulator was turned
on during boot, the regulator is free to vary after that.

> In this case we leave the enable->disable operation to be
> handled by the regulator consumer instead.

Which would be a bug if the consumer wasn't the thing that took the
reference to the regulator in the first place.  Remember, regulators can
be shared so the consumer can't disable a regulator it didn't enable in
the first place (unless it used regulator_get_exclusive() but the use
cases for that are a little suspicious so it'd be worth taking a careful
look before using it).  A consumer can't tell if the regulator was left
enabled by the firmware on boot or if it has been enabled by another
consumer.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] regulator: core: Keep boot_on regulators powered during init
  2012-04-23 10:18 ` Mark Brown
@ 2012-04-23 10:52   ` Ulf Hansson
  2012-04-23 11:05     ` Mark Brown
  0 siblings, 1 reply; 20+ messages in thread
From: Ulf Hansson @ 2012-04-23 10:52 UTC (permalink / raw)
  To: Mark Brown
  Cc: Liam Girdwood, linux-kernel, Mattias WALLIN, Jonas ABERG, Lee Jones

Hi Mark,

Thanks for a quick reply.

On 04/23/2012 12:18 PM, Mark Brown wrote:
> On Mon, Apr 23, 2012 at 11:37:53AM +0200, Ulf Hansson wrote:
>
>> Regulators which has boot_on constraints set, will now remain
>> powered after regulator_init_complete is done.
>
> This would be a bug.  All boot_on means is that the regulator was turned
> on during boot, the regulator is free to vary after that.

The idea is to prevent the late_init_call "regulator_init_complete" from 
disabling a regulator that "recently" were enabled due to it's boot_on 
constraints.

>
>> In this case we leave the enable->disable operation to be
>> handled by the regulator consumer instead.
>
> Which would be a bug if the consumer wasn't the thing that took the
> reference to the regulator in the first place.  Remember, regulators can
> be shared so the consumer can't disable a regulator it didn't enable in
> the first place (unless it used regulator_get_exclusive() but the use
> cases for that are a little suspicious so it'd be worth taking a careful
> look before using it).  A consumer can't tell if the regulator was left
> enabled by the firmware on boot or if it has been enabled by another
> consumer.

I realize that using boot_on, which has been around for quite some time 
could have problems.  If not using the existing boot_on constraint, do 
you have an idea of how to accomplish what I want? Should I invent a new 
constraint option to be used in regulator_init_complete!?

Kind regards
Ulf Hansson

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] regulator: core: Keep boot_on regulators powered during init
  2012-04-23 10:52   ` Ulf Hansson
@ 2012-04-23 11:05     ` Mark Brown
  2012-04-23 12:21       ` Ulf Hansson
  0 siblings, 1 reply; 20+ messages in thread
From: Mark Brown @ 2012-04-23 11:05 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Liam Girdwood, linux-kernel, Mattias WALLIN, Jonas ABERG, Lee Jones

[-- Attachment #1: Type: text/plain, Size: 604 bytes --]

On Mon, Apr 23, 2012 at 12:52:05PM +0200, Ulf Hansson wrote:

> I realize that using boot_on, which has been around for quite some
> time could have problems.  If not using the existing boot_on
> constraint, do you have an idea of how to accomplish what I want?
> Should I invent a new constraint option to be used in
> regulator_init_complete!?

To be honest I don't entirely understand what your goal is at the system
level - the current idea is that either the regulator will be marked as
always_on or it should be enabled by a consumer.  What is the scenario
in which neither of these is sufficient?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] regulator: core: Keep boot_on regulators powered during init
  2012-04-23 11:05     ` Mark Brown
@ 2012-04-23 12:21       ` Ulf Hansson
  2012-04-23 12:25         ` Mark Brown
  0 siblings, 1 reply; 20+ messages in thread
From: Ulf Hansson @ 2012-04-23 12:21 UTC (permalink / raw)
  To: Mark Brown
  Cc: Liam Girdwood, linux-kernel, Mattias WALLIN, Jonas ABERG, Lee Jones

On 04/23/2012 01:05 PM, Mark Brown wrote:
> On Mon, Apr 23, 2012 at 12:52:05PM +0200, Ulf Hansson wrote:
>
>> I realize that using boot_on, which has been around for quite some
>> time could have problems.  If not using the existing boot_on
>> constraint, do you have an idea of how to accomplish what I want?
>> Should I invent a new constraint option to be used in
>> regulator_init_complete!?
>
> To be honest I don't entirely understand what your goal is at the system
> level - the current idea is that either the regulator will be marked as
> always_on or it should be enabled by a consumer.  What is the scenario
> in which neither of these is sufficient?

The consumer do not want to enable the regulator directly from its 
device probe routine, it is handled through a scheduled work.

Moreover the regulator shall not be switched off unless the consumer 
work decides that this is OK.

So, we actually will have a race were the work _might_ be able to 
preventing the late_init_call (regulator_init_complete) from disabling 
the regulator if has reached the point were it has enabled the regulator.

Hopes this clarifies the background a bit more.


Kind regards
Ulf Hansson


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] regulator: core: Keep boot_on regulators powered during init
  2012-04-23 12:21       ` Ulf Hansson
@ 2012-04-23 12:25         ` Mark Brown
  2012-04-23 12:45           ` Ulf Hansson
  0 siblings, 1 reply; 20+ messages in thread
From: Mark Brown @ 2012-04-23 12:25 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Liam Girdwood, linux-kernel, Mattias WALLIN, Jonas ABERG, Lee Jones

[-- Attachment #1: Type: text/plain, Size: 785 bytes --]

On Mon, Apr 23, 2012 at 02:21:49PM +0200, Ulf Hansson wrote:
> On 04/23/2012 01:05 PM, Mark Brown wrote:

> >To be honest I don't entirely understand what your goal is at the system
> >level - the current idea is that either the regulator will be marked as
> >always_on or it should be enabled by a consumer.  What is the scenario
> >in which neither of these is sufficient?

> The consumer do not want to enable the regulator directly from its
> device probe routine, it is handled through a scheduled work.

> Moreover the regulator shall not be switched off unless the consumer
> work decides that this is OK.

Cann you be a bit more concrete?  What is this device and why does it
have these requirements?  The most obvious thing to do here is to run
the work in the probe routine.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] regulator: core: Keep boot_on regulators powered during init
  2012-04-23 12:25         ` Mark Brown
@ 2012-04-23 12:45           ` Ulf Hansson
  2012-04-23 18:01             ` Mark Brown
  0 siblings, 1 reply; 20+ messages in thread
From: Ulf Hansson @ 2012-04-23 12:45 UTC (permalink / raw)
  To: Mark Brown
  Cc: Liam Girdwood, linux-kernel, Mattias WALLIN, Jonas ABERG, Lee Jones

On 04/23/2012 02:25 PM, Mark Brown wrote:
> On Mon, Apr 23, 2012 at 02:21:49PM +0200, Ulf Hansson wrote:
>> On 04/23/2012 01:05 PM, Mark Brown wrote:
>
>>> To be honest I don't entirely understand what your goal is at the system
>>> level - the current idea is that either the regulator will be marked as
>>> always_on or it should be enabled by a consumer.  What is the scenario
>>> in which neither of these is sufficient?
>
>> The consumer do not want to enable the regulator directly from its
>> device probe routine, it is handled through a scheduled work.
>
>> Moreover the regulator shall not be switched off unless the consumer
>> work decides that this is OK.
>
> Cann you be a bit more concrete?  What is this device and why does it
> have these requirements?  The most obvious thing to do here is to run
> the work in the probe routine.

This issue is related to how eMMC cards is working and powered.

A typical mmc host driver is when finished it's probe routine, trigger 
of a so called mmc_rescan work which is taking care of detecting and 
initializing the eMMC card. It is a quite complicated procedure which 
preferably not to be handled from the host driver's probe function.

If the platform already have booted from the eMMC card, the card is 
already powered an initialized from a bootloader. Cutting the power 
without first notifying the card by sending commands to it, must be 
prevented and also violates the eMMC specification. In the end, it will 
mean that the detect and initialization procedure fails.

Kind regards
Ulf Hansson

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] regulator: core: Keep boot_on regulators powered during init
  2012-04-23 12:45           ` Ulf Hansson
@ 2012-04-23 18:01             ` Mark Brown
  2012-04-24  8:09               ` Ulf Hansson
  2012-04-25 15:34               ` Jassi Brar
  0 siblings, 2 replies; 20+ messages in thread
From: Mark Brown @ 2012-04-23 18:01 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Liam Girdwood, linux-kernel, Mattias WALLIN, Jonas ABERG, Lee Jones

[-- Attachment #1: Type: text/plain, Size: 1091 bytes --]

On Mon, Apr 23, 2012 at 02:45:10PM +0200, Ulf Hansson wrote:

> A typical mmc host driver is when finished it's probe routine,
> trigger of a so called mmc_rescan work which is taking care of
> detecting and initializing the eMMC card. It is a quite complicated
> procedure which preferably not to be handled from the host driver's
> probe function.

> If the platform already have booted from the eMMC card, the card is
> already powered an initialized from a bootloader. Cutting the power
> without first notifying the card by sending commands to it, must be
> prevented and also violates the eMMC specification. In the end, it
> will mean that the detect and initialization procedure fails.

Can the driver use is_enabled() in the probe routine to check the
current status during probe and hand off appropriately?  The issue here
seems like it's the fact that the driver isn't managing to bootstrapping
of its state well.

Worst case seems to be that the card will be briefly powered during boot
then turned off again after enumeration which doesn't seem like the end
of the world to me.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] regulator: core: Keep boot_on regulators powered during init
  2012-04-23 18:01             ` Mark Brown
@ 2012-04-24  8:09               ` Ulf Hansson
  2012-04-24 10:56                 ` Mark Brown
  2012-04-25 15:34               ` Jassi Brar
  1 sibling, 1 reply; 20+ messages in thread
From: Ulf Hansson @ 2012-04-24  8:09 UTC (permalink / raw)
  To: Mark Brown
  Cc: Liam Girdwood, linux-kernel, Mattias WALLIN, Jonas ABERG, Lee Jones

On 04/23/2012 08:01 PM, Mark Brown wrote:
> On Mon, Apr 23, 2012 at 02:45:10PM +0200, Ulf Hansson wrote:
>
>> A typical mmc host driver is when finished it's probe routine,
>> trigger of a so called mmc_rescan work which is taking care of
>> detecting and initializing the eMMC card. It is a quite complicated
>> procedure which preferably not to be handled from the host driver's
>> probe function.
>
>> If the platform already have booted from the eMMC card, the card is
>> already powered an initialized from a bootloader. Cutting the power
>> without first notifying the card by sending commands to it, must be
>> prevented and also violates the eMMC specification. In the end, it
>> will mean that the detect and initialization procedure fails.
>
> Can the driver use is_enabled() in the probe routine to check the
> current status during probe and hand off appropriately?  The issue here
> seems like it's the fact that the driver isn't managing to bootstrapping
> of its state well.

Well, it is not as simple as that. An mmc host driver is just a driver 
for handling a certain mmc IP. Uper layers handles the (e)MMC/SD/SDIO 
protocol including controlling power the card. Moreover the complicated 
detect procedure is handled in a work.

In principle what you are proposing will mean that each mmc host driver 
will have to "flush" the rescan work from probe. This will have horrid 
impact on boot time since rescan can take several hundred of 
milliseconds for each eMMC/SD/SDIO card. Is is far better to handle the 
rescan in parallel works.

I really think it would be much beneficial to be able to tell the late 
init call (regulator_init_completet) to back off from disabling this 
regulator. If not using boot_on, we can invent another regulator 
constraint for this. What do you think of this?

>
> Worst case seems to be that the card will be briefly powered during boot
> then turned off again after enumeration which doesn't seem like the end
> of the world to me.

It is really crucial that the regulator is not switched off in an 
uncontrolled manner. It will mean viloating eMMC spec and in many cases 
the hw is not able to reset the eMMC and thus the detect procedure will 
fail. Likely the eMMC holds root file system then the platform wont boot...

Kind regards
Ulf Hansson

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] regulator: core: Keep boot_on regulators powered during init
  2012-04-24  8:09               ` Ulf Hansson
@ 2012-04-24 10:56                 ` Mark Brown
  2012-04-24 12:43                   ` Ulf Hansson
  0 siblings, 1 reply; 20+ messages in thread
From: Mark Brown @ 2012-04-24 10:56 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Liam Girdwood, linux-kernel, Mattias WALLIN, Jonas ABERG, Lee Jones

[-- Attachment #1: Type: text/plain, Size: 2261 bytes --]

On Tue, Apr 24, 2012 at 10:09:40AM +0200, Ulf Hansson wrote:
> On 04/23/2012 08:01 PM, Mark Brown wrote:

> >Can the driver use is_enabled() in the probe routine to check the
> >current status during probe and hand off appropriately?  The issue here
> >seems like it's the fact that the driver isn't managing to bootstrapping
> >of its state well.

> Well, it is not as simple as that. An mmc host driver is just a
> driver for handling a certain mmc IP. Uper layers handles the
> (e)MMC/SD/SDIO protocol including controlling power the card.
> Moreover the complicated detect procedure is handled in a work.

> In principle what you are proposing will mean that each mmc host
> driver will have to "flush" the rescan work from probe. This will
> have horrid impact on boot time since rescan can take several
> hundred of milliseconds for each eMMC/SD/SDIO card. Is is far better
> to handle the rescan in parallel works.

No, that's not what I'm suggesting - all I'm suggesting is that the
driver uses is_enabled() in probe() to check if the regulator is on, if
it is then it grabs a reference to it.  Then, when it's figured out
what's going on, it can drop the reference again if it's not needed.

> I really think it would be much beneficial to be able to tell the
> late init call (regulator_init_completet) to back off from disabling
> this regulator. If not using boot_on, we can invent another
> regulator constraint for this. What do you think of this?

This just seems awfully fragile and very much dependant on things like
having the driver actually enabled to clean up later.

> >Worst case seems to be that the card will be briefly powered during boot
> >then turned off again after enumeration which doesn't seem like the end
> >of the world to me.

> It is really crucial that the regulator is not switched off in an
> uncontrolled manner. It will mean viloating eMMC spec and in many
> cases the hw is not able to reset the eMMC and thus the detect
> procedure will fail. Likely the eMMC holds root file system then the
> platform wont boot...

Right, but I'm talking about uncontrolled enables not disables - worst
case is you'll have to do an ordered shutdown you wouldn't otherwise
have to do but that doesn't seem like the end of the world.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] regulator: core: Keep boot_on regulators powered during init
  2012-04-24 10:56                 ` Mark Brown
@ 2012-04-24 12:43                   ` Ulf Hansson
  2012-04-25  8:02                     ` Mark Brown
  0 siblings, 1 reply; 20+ messages in thread
From: Ulf Hansson @ 2012-04-24 12:43 UTC (permalink / raw)
  To: Mark Brown
  Cc: Liam Girdwood, linux-kernel, Mattias WALLIN, Jonas ABERG, Lee Jones

On 04/24/2012 12:56 PM, Mark Brown wrote:
> On Tue, Apr 24, 2012 at 10:09:40AM +0200, Ulf Hansson wrote:
>> On 04/23/2012 08:01 PM, Mark Brown wrote:
>
>>> Can the driver use is_enabled() in the probe routine to check the
>>> current status during probe and hand off appropriately?  The issue here
>>> seems like it's the fact that the driver isn't managing to bootstrapping
>>> of its state well.
>
>> Well, it is not as simple as that. An mmc host driver is just a
>> driver for handling a certain mmc IP. Uper layers handles the
>> (e)MMC/SD/SDIO protocol including controlling power the card.
>> Moreover the complicated detect procedure is handled in a work.
>
>> In principle what you are proposing will mean that each mmc host
>> driver will have to "flush" the rescan work from probe. This will
>> have horrid impact on boot time since rescan can take several
>> hundred of milliseconds for each eMMC/SD/SDIO card. Is is far better
>> to handle the rescan in parallel works.
>
> No, that's not what I'm suggesting - all I'm suggesting is that the
> driver uses is_enabled() in probe() to check if the regulator is on, if
> it is then it grabs a reference to it.  Then, when it's figured out
> what's going on, it can drop the reference again if it's not needed.

The problem is how it should "figure out what is going on", all that 
stuff is handled from the protocol layer.

So if grabbing a reference, there is no good point in the code were I 
can drop it. Moreover _every_ host driver needs to handle this. It will 
likely become a "hack" is my first impression.

>> I really think it would be much beneficial to be able to tell the
>> late init call (regulator_init_completet) to back off from disabling
>> this regulator. If not using boot_on, we can invent another
>> regulator constraint for this. What do you think of this?
>
> This just seems awfully fragile and very much dependant on things like
> having the driver actually enabled to clean up later.

Setting this constraint is not done be "default", it could be clearly be 
stated that the consumer must handle the enable/disable, otherwise the 
regulator will be left in the state it was when the kernel booted.

>
>>> Worst case seems to be that the card will be briefly powered during boot
>>> then turned off again after enumeration which doesn't seem like the end
>>> of the world to me.
>
>> It is really crucial that the regulator is not switched off in an
>> uncontrolled manner. It will mean viloating eMMC spec and in many
>> cases the hw is not able to reset the eMMC and thus the detect
>> procedure will fail. Likely the eMMC holds root file system then the
>> platform wont boot...
>
> Right, but I'm talking about uncontrolled enables not disables - worst
> case is you'll have to do an ordered shutdown you wouldn't otherwise
> have to do but that doesn't seem like the end of the world.


Kind regards
Ulf Hansson

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] regulator: core: Keep boot_on regulators powered during init
  2012-04-24 12:43                   ` Ulf Hansson
@ 2012-04-25  8:02                     ` Mark Brown
  2012-04-25  9:37                       ` Ulf Hansson
  0 siblings, 1 reply; 20+ messages in thread
From: Mark Brown @ 2012-04-25  8:02 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Liam Girdwood, linux-kernel, Mattias WALLIN, Jonas ABERG, Lee Jones

[-- Attachment #1: Type: text/plain, Size: 1247 bytes --]

On Tue, Apr 24, 2012 at 02:43:20PM +0200, Ulf Hansson wrote:

> So if grabbing a reference, there is no good point in the code were
> I can drop it. Moreover _every_ host driver needs to handle this. It
> will likely become a "hack" is my first impression.

If it's something that every host driver needs to do then just factor it
into the framework and we're done...  The stuff you're trying to put in
the regulator API feels equally like it's a bodge and it seems to me
like we've just not thought of the best way for the MMC stack to figure
out and keep track of if it needs a regulator or not.

> >This just seems awfully fragile and very much dependant on things like
> >having the driver actually enabled to clean up later.

> Setting this constraint is not done be "default", it could be
> clearly be stated that the consumer must handle the enable/disable,
> otherwise the regulator will be left in the state it was when the
> kernel booted.

Right, but the whole point in having full constraints is to avoid that.
Users are supposed to set constraints to grant permissions for things,
not to work around internal problems in the rest of the stack.  If I
could see a general use case for the feature...  but I'm having trouble
doing that.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] regulator: core: Keep boot_on regulators powered during init
  2012-04-25  8:02                     ` Mark Brown
@ 2012-04-25  9:37                       ` Ulf Hansson
  2012-04-25  9:58                         ` Mark Brown
  0 siblings, 1 reply; 20+ messages in thread
From: Ulf Hansson @ 2012-04-25  9:37 UTC (permalink / raw)
  To: Mark Brown
  Cc: Liam Girdwood, linux-kernel, Mattias WALLIN, Jonas ABERG, Lee Jones

On 04/25/2012 10:02 AM, Mark Brown wrote:
> On Tue, Apr 24, 2012 at 02:43:20PM +0200, Ulf Hansson wrote:
>
>> So if grabbing a reference, there is no good point in the code were
>> I can drop it. Moreover _every_ host driver needs to handle this. It
>> will likely become a "hack" is my first impression.
>
> If it's something that every host driver needs to do then just factor it
> into the framework and we're done...  The stuff you're trying to put in
> the regulator API feels equally like it's a bodge and it seems to me
> like we've just not thought of the best way for the MMC stack to figure
> out and keep track of if it needs a regulator or not.
>
>>> This just seems awfully fragile and very much dependant on things like
>>> having the driver actually enabled to clean up later.
>
>> Setting this constraint is not done be "default", it could be
>> clearly be stated that the consumer must handle the enable/disable,
>> otherwise the regulator will be left in the state it was when the
>> kernel booted.
>
> Right, but the whole point in having full constraints is to avoid that.
> Users are supposed to set constraints to grant permissions for things,
> not to work around internal problems in the rest of the stack.  If I
> could see a general use case for the feature...  but I'm having trouble
> doing that.

Maybe you have convinced me now :-) I will therefore start thinking of a 
patch on the mmc framework instead. I will include you if/when I send 
out the patch to the mmc-list, just for reference if that is ok with you?

Some final thoughts (please comment if you like):
We already have the boot_on constraint, which to me is similar to what a 
new kind of "boot keep state" constraint would be. I think it would be 
no more odd than what boot_on already is. Maybe not a good argument, but 
still..


Kind regards
Ulf Hansson

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] regulator: core: Keep boot_on regulators powered during init
  2012-04-25  9:37                       ` Ulf Hansson
@ 2012-04-25  9:58                         ` Mark Brown
  2012-04-25 16:45                           ` Jassi Brar
  0 siblings, 1 reply; 20+ messages in thread
From: Mark Brown @ 2012-04-25  9:58 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Liam Girdwood, linux-kernel, Mattias WALLIN, Jonas ABERG,
	Lee Jones, Jassi Brar

[-- Attachment #1: Type: text/plain, Size: 1511 bytes --]

On Wed, Apr 25, 2012 at 11:37:36AM +0200, Ulf Hansson wrote:

> Maybe you have convinced me now :-) I will therefore start thinking
> of a patch on the mmc framework instead. I will include you if/when
> I send out the patch to the mmc-list, just for reference if that is
> ok with you?

Yes, please - there've been some issues with the way regulators are used
in MMC for a while.  There's definitely some stuff that needs to be
worked through here.

I've also added Jassi who it just occurred to me was talking about some
vaugley similar stuff relatively recently (though I'm not sure it was
MMC).  Jassi, the issue here is working out if an MMC device is powered
at boot so we can skip probing then shutting it down cleanly.

> Some final thoughts (please comment if you like):
> We already have the boot_on constraint, which to me is similar to
> what a new kind of "boot keep state" constraint would be. I think it
> would be no more odd than what boot_on already is. Maybe not a good
> argument, but still..

Yeah, the main use case for boot_on is to support things like automatic
enumeration of devices - for buses where we can enumerate devices when
they're plugged in we normally don't explicitly register them so if we
boot with the regulator disabled we need some way of allowing the device
to appear on the bus.  Don't know that that actually gets much use
though.  It is also used by regulators which for some reason can't
figure out their initial state, there's a few examples of that in
mainline.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] regulator: core: Keep boot_on regulators powered during init
  2012-04-23 18:01             ` Mark Brown
  2012-04-24  8:09               ` Ulf Hansson
@ 2012-04-25 15:34               ` Jassi Brar
  2012-04-25 15:44                 ` Ulf Hansson
  2012-04-26  8:35                 ` Mark Brown
  1 sibling, 2 replies; 20+ messages in thread
From: Jassi Brar @ 2012-04-25 15:34 UTC (permalink / raw)
  To: Mark Brown
  Cc: Ulf Hansson, Liam Girdwood, linux-kernel, Mattias WALLIN,
	Jonas ABERG, Lee Jones

> On Mon, Apr 23, 2012 at 02:45:10PM +0200, Ulf Hansson wrote:
>
>> If the platform already have booted from the eMMC card, the card is
>> already powered an initialized from a bootloader. Cutting the power
>> without first notifying the card by sending commands to it, must be
>> prevented and also violates the eMMC specification. In the end, it
>> will mean that the detect and initialization procedure fails.
>
 Shouldn't the bootloader be responsible for ensuring MMC is
put down after using it and before passing control to the kernel.

 Even if the bootloader didn't power off properly, unless there is
some outstanding data to be written when the kernel is passed
control, perhaps it's not that serious?

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] regulator: core: Keep boot_on regulators powered during init
  2012-04-25 15:34               ` Jassi Brar
@ 2012-04-25 15:44                 ` Ulf Hansson
  2012-04-25 16:31                   ` Jassi Brar
  2012-04-26  8:35                 ` Mark Brown
  1 sibling, 1 reply; 20+ messages in thread
From: Ulf Hansson @ 2012-04-25 15:44 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Mark Brown, Liam Girdwood, linux-kernel, Mattias WALLIN,
	Jonas ABERG, Lee Jones

On 04/25/2012 05:34 PM, Jassi Brar wrote:
>> On Mon, Apr 23, 2012 at 02:45:10PM +0200, Ulf Hansson wrote:
>>
>>> If the platform already have booted from the eMMC card, the card is
>>> already powered an initialized from a bootloader. Cutting the power
>>> without first notifying the card by sending commands to it, must be
>>> prevented and also violates the eMMC specification. In the end, it
>>> will mean that the detect and initialization procedure fails.
>>
>   Shouldn't the bootloader be responsible for ensuring MMC is
> put down after using it and before passing control to the kernel.

This is kind of complicated. eMMC is powered by two regulators. VCC and 
VCCQ. Cutting VCC uncontrolled is not allowed. If doing so anyway there 
is two options to recover.

1. Cut VCCQ as well and do full reinit of the eMMC
2. Pull a hw-reset pin to the eMMC and do a full reinit of the eMMC.

In some cases neither is possible due to HW constraints. Thus we have to 
make sure VCC is not cut.

>
>   Even if the bootloader didn't power off properly, unless there is
> some outstanding data to be written when the kernel is passed
> control, perhaps it's not that serious?

The kernel will not be to re-init the eMMC... quite serious. :-)

Kind regards
Ulf Hansson

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] regulator: core: Keep boot_on regulators powered during init
  2012-04-25 15:44                 ` Ulf Hansson
@ 2012-04-25 16:31                   ` Jassi Brar
  0 siblings, 0 replies; 20+ messages in thread
From: Jassi Brar @ 2012-04-25 16:31 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Mark Brown, Liam Girdwood, linux-kernel, Mattias WALLIN,
	Jonas ABERG, Lee Jones

On 25 April 2012 21:14, Ulf Hansson <ulf.hansson@stericsson.com> wrote:
> On 04/25/2012 05:34 PM, Jassi Brar wrote:
>>>
>>  Shouldn't the bootloader be responsible for ensuring MMC is
>> put down after using it and before passing control to the kernel.
>
> This is kind of complicated. eMMC is powered by two regulators. VCC and
> VCCQ. Cutting VCC uncontrolled is not allowed. If doing so anyway there is
> two options to recover.
>
> 1. Cut VCCQ as well and do full reinit of the eMMC
> 2. Pull a hw-reset pin to the eMMC and do a full reinit of the eMMC.
>
> In some cases neither is possible due to HW constraints. Thus we have to
> make sure VCC is not cut.
>
I meant power-down to whatever extent your h/w permits. The rest you'll
have to take care by appropriately flagging supplies in the kernel.

If not full power-off, couldn't your bootloader atleast send eMMC the necessary
"power-down sequence of commands", so that it responds well to next
power-up sequence of commands ?

The point being, the board files, in both your bootloader and kernel, ought to
know the h/w constraints and program the regulator api accordingly.

Generalizing the case, IMHO it's not feasible for every stack in the kernel to
assume it has to first cleanly shutdown the h/w before probing it.

>>  Even if the bootloader didn't power off properly, unless there is
>> some outstanding data to be written when the kernel is passed
>> control, perhaps it's not that serious?
>
> The kernel will not be to re-init the eMMC... quite serious. :-)
>
Not unless your board files flag the supplies accordingly, always_on,
and the bootloader sent eMMC the "deadly command seq" ;)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] regulator: core: Keep boot_on regulators powered during init
  2012-04-25  9:58                         ` Mark Brown
@ 2012-04-25 16:45                           ` Jassi Brar
  0 siblings, 0 replies; 20+ messages in thread
From: Jassi Brar @ 2012-04-25 16:45 UTC (permalink / raw)
  To: Mark Brown
  Cc: Ulf Hansson, Liam Girdwood, linux-kernel, Mattias WALLIN,
	Jonas ABERG, Lee Jones

On Wed, Apr 25, 2012 at 3:28 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Apr 25, 2012 at 11:37:36AM +0200, Ulf Hansson wrote:
>
>> Maybe you have convinced me now :-) I will therefore start thinking
>> of a patch on the mmc framework instead. I will include you if/when
>> I send out the patch to the mmc-list, just for reference if that is
>> ok with you?
>
> Yes, please - there've been some issues with the way regulators are used
> in MMC for a while.  There's definitely some stuff that needs to be
> worked through here.
>
> I've also added Jassi who it just occurred to me was talking about some
> vaugley similar stuff relatively recently (though I'm not sure it was
> MMC).  Jassi, the issue here is working out if an MMC device is powered
> at boot so we can skip probing then shutting it down cleanly.
>
Mark, thanks for notifying me.
But my problem was purely related to dummy - I wanted to determine if
a supply is physically absent when I have support for dummy regulator enabled.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] regulator: core: Keep boot_on regulators powered during init
  2012-04-25 15:34               ` Jassi Brar
  2012-04-25 15:44                 ` Ulf Hansson
@ 2012-04-26  8:35                 ` Mark Brown
  2012-04-26  9:10                   ` Jassi Brar
  1 sibling, 1 reply; 20+ messages in thread
From: Mark Brown @ 2012-04-26  8:35 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Ulf Hansson, Liam Girdwood, linux-kernel, Mattias WALLIN,
	Jonas ABERG, Lee Jones

[-- Attachment #1: Type: text/plain, Size: 335 bytes --]

On Wed, Apr 25, 2012 at 09:04:01PM +0530, Jassi Brar wrote:

>  Shouldn't the bootloader be responsible for ensuring MMC is
> put down after using it and before passing control to the kernel.

You might want to try to ensure that bootloaders always do the right
thing but I prefer to work on more achievable goals like world peace. :/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] regulator: core: Keep boot_on regulators powered during init
  2012-04-26  8:35                 ` Mark Brown
@ 2012-04-26  9:10                   ` Jassi Brar
  0 siblings, 0 replies; 20+ messages in thread
From: Jassi Brar @ 2012-04-26  9:10 UTC (permalink / raw)
  To: Mark Brown
  Cc: Ulf Hansson, Liam Girdwood, linux-kernel, Mattias WALLIN,
	Jonas ABERG, Lee Jones

On 26 April 2012 14:05, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Apr 25, 2012 at 09:04:01PM +0530, Jassi Brar wrote:
>
>>  Shouldn't the bootloader be responsible for ensuring MMC is
>> put down after using it and before passing control to the kernel.
>
> You might want to try to ensure that bootloaders always do the right
> thing but I prefer to work on more achievable goals like world peace. :/
>
>From what Ulf described so far, I am not sure if even gagging the regulator
api could be seen as a foolproof solution to the problem - he still doesn't
account for the necessary 'power-down sequence of commands' without
which the eMMC might fail attempts by the mmc stack to reset and initialize.

Appeasements and treaties could help achieve peace, I am just not sure
if former is a good option this time :)

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2012-04-26  9:10 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-23  9:37 [PATCH] regulator: core: Keep boot_on regulators powered during init Ulf Hansson
2012-04-23 10:18 ` Mark Brown
2012-04-23 10:52   ` Ulf Hansson
2012-04-23 11:05     ` Mark Brown
2012-04-23 12:21       ` Ulf Hansson
2012-04-23 12:25         ` Mark Brown
2012-04-23 12:45           ` Ulf Hansson
2012-04-23 18:01             ` Mark Brown
2012-04-24  8:09               ` Ulf Hansson
2012-04-24 10:56                 ` Mark Brown
2012-04-24 12:43                   ` Ulf Hansson
2012-04-25  8:02                     ` Mark Brown
2012-04-25  9:37                       ` Ulf Hansson
2012-04-25  9:58                         ` Mark Brown
2012-04-25 16:45                           ` Jassi Brar
2012-04-25 15:34               ` Jassi Brar
2012-04-25 15:44                 ` Ulf Hansson
2012-04-25 16:31                   ` Jassi Brar
2012-04-26  8:35                 ` Mark Brown
2012-04-26  9:10                   ` Jassi Brar

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.