All of lore.kernel.org
 help / color / mirror / Atom feed
* Device probe order (i2c regulator vs. platform device)
@ 2010-03-24  7:10 Marek Szyprowski
  2010-03-24  9:19 ` Andy Green
  2010-03-24 10:21 ` Mark Brown
  0 siblings, 2 replies; 19+ messages in thread
From: Marek Szyprowski @ 2010-03-24  7:10 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

I've encountered a problem with regulator framework and the device probe order. In my system there is a pmic chip connected thought i2c bus and a platform device (let's call it A) that depends on the regulator device (for proper probing pmic chip must enable voltage to the device A). In the current configuration the i2c driver is also a platform device. However during the system initialization the device A is probed before the i2c driver would register pmic chip and its regulators.

How I can delay probing the device A to the moment when the regulator device will be available in the system? Is there any generic was of changing the device probe order? 

Best regards
--
Marek Szyprowski
Samsung Poland R&D Center

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

* Device probe order (i2c regulator vs. platform device)
  2010-03-24  7:10 Device probe order (i2c regulator vs. platform device) Marek Szyprowski
@ 2010-03-24  9:19 ` Andy Green
  2010-03-24 11:11   ` Marek Szyprowski
  2010-03-24 11:32   ` Mark Brown
  2010-03-24 10:21 ` Mark Brown
  1 sibling, 2 replies; 19+ messages in thread
From: Andy Green @ 2010-03-24  9:19 UTC (permalink / raw)
  To: linux-arm-kernel

On 03/24/10 07:10, Somebody in the thread at some point said:
> Hello,
>
> I've encountered a problem with regulator framework and the device
> probe order. In my system there is a pmic chip connected thought i2c
> bus and a platform device (let's call it A) that depends on the
> regulator device (for proper probing pmic chip must enable voltage to
> the device A). In the current configuration the i2c driver is also a
> platform device. However during the system initialization the device
> A is probed before the i2c driver would register pmic chip and its
> regulators.
>
> How I can delay probing the device A to the moment when the regulator
> device will be available in the system? Is there any generic was of
> changing the device probe order?

Delaying registering devices is only half the problem.  You must also 
correct these device's parent dev to be the PMU's.

Otherwise your suspend and resume ordering turns to crap and you will 
immediately be in Hell, for example the PMIC may go down taking away 
power from the SD Card before you finished talking to it.

With correct device parent ordering, then children are guaranteed to be 
suspended before the parent, it means the PMIC won't go down until all 
these guys registered as dependent on it have nicely gone down.

What I have been doing is adding a platform callback in the pmic 
platform_data which is called at the end of pmic probe.

In the machine file, in the callback I then register everything that 
relies on PMIC power arrangements, and correct the device parent at to 
be the PMIC at the same time.

It would be very good is this was regularized somehow into a standard 
API because it's absolutely a requirement for many boards that their 
devices ARE dependent on PMU as a parent.

-Andy

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

* Device probe order (i2c regulator vs. platform device)
  2010-03-24  7:10 Device probe order (i2c regulator vs. platform device) Marek Szyprowski
  2010-03-24  9:19 ` Andy Green
@ 2010-03-24 10:21 ` Mark Brown
  2010-03-24 10:57   ` Andy Green
  2010-03-24 11:02   ` Marek Szyprowski
  1 sibling, 2 replies; 19+ messages in thread
From: Mark Brown @ 2010-03-24 10:21 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Mar 24, 2010 at 08:10:38AM +0100, Marek Szyprowski wrote:

[Reflowed into 80 columns - please fix your mail client to wrap text, it
makes it very much easier to read your messages.]

> I've encountered a problem with regulator framework and the device
> probe order. In my system there is a pmic chip connected thought i2c bus
> and a platform device (let's call it A) that depends on the regulator
> device (for proper probing pmic chip must enable voltage to the device
> A). In the current configuration the i2c driver is also a platform
> device. However during the system initialization the device A is probed
> before the i2c driver would register pmic chip and its regulators.

This is (if I'm parsing what you say above correctly) a very common case
- if you look at most of the existing PMIC core and regulator drivers
you'll see that their initcalls are subsys_initcall(), and similarly for
the I2C controller drivers they use.  This means that at boot the PMICs
come up before pretty much any other device.

> How I can delay probing the device A to the moment when the regulator
> device will be available in the system? Is there any generic was of
> changing the device probe order?

The solution above is the other way around - it ensures that the
regulators come up first - but achieves the same effect.  Unfortunately
it's not as generic as would be best but it resolves the issues.

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

* Device probe order (i2c regulator vs. platform device)
  2010-03-24 10:21 ` Mark Brown
@ 2010-03-24 10:57   ` Andy Green
  2010-03-24 11:02   ` Marek Szyprowski
  1 sibling, 0 replies; 19+ messages in thread
From: Andy Green @ 2010-03-24 10:57 UTC (permalink / raw)
  To: linux-arm-kernel

On 03/24/10 10:21, Somebody in the thread at some point said:

> The solution above is the other way around - it ensures that the
> regulators come up first - but achieves the same effect.  Unfortunately
> it's not as generic as would be best but it resolves the issues.

How does it "resolve the issue" of uncontrolled race during suspend and 
resume?

-Andy

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

* Device probe order (i2c regulator vs. platform device)
  2010-03-24 10:21 ` Mark Brown
  2010-03-24 10:57   ` Andy Green
@ 2010-03-24 11:02   ` Marek Szyprowski
  2010-03-24 11:08     ` Mark Brown
  1 sibling, 1 reply; 19+ messages in thread
From: Marek Szyprowski @ 2010-03-24 11:02 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

On Wednesday, March 24, 2010 11:22 AM Mark Brown wrote:

> On Wed, Mar 24, 2010 at 08:10:38AM +0100, Marek Szyprowski wrote:
> 
> > I've encountered a problem with regulator framework and the device
> > probe order. In my system there is a pmic chip connected thought i2c bus
> > and a platform device (let's call it A) that depends on the regulator
> > device (for proper probing pmic chip must enable voltage to the device
> > A). In the current configuration the i2c driver is also a platform
> > device. However during the system initialization the device A is probed
> > before the i2c driver would register pmic chip and its regulators.
> 
> This is (if I'm parsing what you say above correctly) a very common case
> - if you look at most of the existing PMIC core and regulator drivers
> you'll see that their initcalls are subsys_initcall(), and similarly for
> the I2C controller drivers they use.  This means that at boot the PMICs
> come up before pretty much any other device.

Ok, I get this idea, but this requires to move the i2c bus initialization
also to subsys_initcall. We use generic i2c-gpio driver. I suspect there
might be some unpredicted consequences on some other systems if we push
the patch that changes it init to subsys_initcall. I have no idea if the
gpiolib calls are available on all systems during the subsys_initcall.

Best regards
--
Marek Szyprowski
Samsung Poland R&D Center

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

* Device probe order (i2c regulator vs. platform device)
  2010-03-24 11:02   ` Marek Szyprowski
@ 2010-03-24 11:08     ` Mark Brown
  2010-03-24 11:27       ` Andy Green
  0 siblings, 1 reply; 19+ messages in thread
From: Mark Brown @ 2010-03-24 11:08 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Mar 24, 2010 at 12:02:30PM +0100, Marek Szyprowski wrote:

> Ok, I get this idea, but this requires to move the i2c bus initialization
> also to subsys_initcall. We use generic i2c-gpio driver. I suspect there
> might be some unpredicted consequences on some other systems if we push
> the patch that changes it init to subsys_initcall. I have no idea if the
> gpiolib calls are available on all systems during the subsys_initcall.

It's been OK for other I2C controllers.  GPIOs are another thing I'd
expect to see available early since they're in a similar position where
lots of other devices end up needing them in their probe so it'd seem
surprising if there were a problem with bringing it up early.

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

* Device probe order (i2c regulator vs. platform device)
  2010-03-24  9:19 ` Andy Green
@ 2010-03-24 11:11   ` Marek Szyprowski
  2010-03-24 12:50     ` Andy Green
  2010-03-24 11:32   ` Mark Brown
  1 sibling, 1 reply; 19+ messages in thread
From: Marek Szyprowski @ 2010-03-24 11:11 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

On Wednesday, March 24, 2010 10:20 AM Andy Green wrote:

> On 03/24/10 07:10, Somebody in the thread at some point said:
> > Hello,
> >
> > I've encountered a problem with regulator framework and the device
> > probe order. In my system there is a pmic chip connected thought i2c
> > bus and a platform device (let's call it A) that depends on the
> > regulator device (for proper probing pmic chip must enable voltage to
> > the device A). In the current configuration the i2c driver is also a
> > platform device. However during the system initialization the device
> > A is probed before the i2c driver would register pmic chip and its
> > regulators.
> >
> > How I can delay probing the device A to the moment when the regulator
> > device will be available in the system? Is there any generic was of
> > changing the device probe order?
> 
> Delaying registering devices is only half the problem.  You must also
> correct these device's parent dev to be the PMU's.
>
> Otherwise your suspend and resume ordering turns to crap and you will
> immediately be in Hell, for example the PMIC may go down taking away
> power from the SD Card before you finished talking to it.
> 
> With correct device parent ordering, then children are guaranteed to be
> suspended before the parent, it means the PMIC won't go down until all
> these guys registered as dependent on it have nicely gone down.

That's true.
 
> What I have been doing is adding a platform callback in the pmic
> platform_data which is called at the end of pmic probe.
> 
> In the machine file, in the callback I then register everything that
> relies on PMIC power arrangements, and correct the device parent at to
> be the PMIC at the same time.
> 
> It would be very good is this was regularized somehow into a standard
> API because it's absolutely a requirement for many boards that their
> devices ARE dependent on PMU as a parent.

Right, this is a really common case. Maybe this callback could be added
to the regulator framework as well?

On the other hand a more generic solution might be needed, because besides
the PMU there might be some other dependences between various devices that
not possible to be ensured in the current framework (I'm thinking of v4l2
subdevs that link 2 separate devices together, but currently are used only
with i2c clients).

Best regards
--
Marek Szyprowski
Samsung Poland R&D Center

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

* Device probe order (i2c regulator vs. platform device)
  2010-03-24 11:08     ` Mark Brown
@ 2010-03-24 11:27       ` Andy Green
  2010-03-24 12:19         ` Mark Brown
  0 siblings, 1 reply; 19+ messages in thread
From: Andy Green @ 2010-03-24 11:27 UTC (permalink / raw)
  To: linux-arm-kernel

On 03/24/10 11:08, Somebody in the thread at some point said:
> On Wed, Mar 24, 2010 at 12:02:30PM +0100, Marek Szyprowski wrote:
>
>> Ok, I get this idea, but this requires to move the i2c bus initialization
>> also to subsys_initcall. We use generic i2c-gpio driver. I suspect there
>> might be some unpredicted consequences on some other systems if we push
>> the patch that changes it init to subsys_initcall. I have no idea if the
>> gpiolib calls are available on all systems during the subsys_initcall.
>
> It's been OK for other I2C controllers.  GPIOs are another thing I'd
> expect to see available early since they're in a similar position where
> lots of other devices end up needing them in their probe so it'd seem
> surprising if there were a problem with bringing it up early.

You're missing the point.

On any board with a PMU if you look at the schematic you will see most 
of the assets on the board are totally dependent on the PMU power 
arrangements.

Take for example the SD card socket with its own regulator from the PMU.

In Linux reasonably enough MMC is not related to I2C at all in the 
default case.  Therefore during suspend, nothing stops I2C child suspend 
completing before the MMC stack suspend completes.

In that case, MMC commands designed to put the card to idle nicely ping 
off a dead card because its power is gone already.  And that is an 
uncontrolled race that varied suspend by suspend according to the phase 
of the moon.

The dependency of the SD slot on the PMU is not notional, it's totally 
real in hardware.

The solution is to make explicit the device hierarchy relationship using 
the parent member of struct device.  On a typical board, it means most 
of your devices are children on the PMU -- and that reflects the reality.

To do that, you need a post-PMU-probe callback into the machine file, or 
some even nicer API arrangement you're in a good position to invent :-)

-Andy

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

* Device probe order (i2c regulator vs. platform device)
  2010-03-24  9:19 ` Andy Green
  2010-03-24 11:11   ` Marek Szyprowski
@ 2010-03-24 11:32   ` Mark Brown
  2010-03-24 12:11     ` Andy Green
  1 sibling, 1 reply; 19+ messages in thread
From: Mark Brown @ 2010-03-24 11:32 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Mar 24, 2010 at 09:19:58AM +0000, Andy Green wrote:

> Delaying registering devices is only half the problem.  You must
> also correct these device's parent dev to be the PMU's.

> Otherwise your suspend and resume ordering turns to crap and you
> will immediately be in Hell, for example the PMIC may go down taking
> away power from the SD Card before you finished talking to it.

This has not been a practical issue in systems - it is very rare to
explicitly suspend the regulator part of the PMIC.  Obviously the PMIC
suspend will usually involve killing the power to the processor so it is
almost always triggered from hardware as part of the processor poweroff
after software has halted.

It'd be nice to explicitly save and restore the regulator state over
suspend and resume for devices that don't support that natively but
that's mostly orthogonal and mostly just flops out of devices doing
their own management at suspend and resume anyway.

> It would be very good is this was regularized somehow into a
> standard API because it's absolutely a requirement for many boards
> that their devices ARE dependent on PMU as a parent.

The only system I'm aware of which had any stuff like this was the
GTA02.  I never investigated to figure out why the code there was doing
what it was.

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

* Device probe order (i2c regulator vs. platform device)
  2010-03-24 11:32   ` Mark Brown
@ 2010-03-24 12:11     ` Andy Green
  2010-03-24 14:01       ` Mark Brown
  0 siblings, 1 reply; 19+ messages in thread
From: Andy Green @ 2010-03-24 12:11 UTC (permalink / raw)
  To: linux-arm-kernel

On 03/24/10 11:32, Somebody in the thread at some point said:
> On Wed, Mar 24, 2010 at 09:19:58AM +0000, Andy Green wrote:
>
>> Delaying registering devices is only half the problem.  You must
>> also correct these device's parent dev to be the PMU's.
>
>> Otherwise your suspend and resume ordering turns to crap and you
>> will immediately be in Hell, for example the PMIC may go down taking
>> away power from the SD Card before you finished talking to it.
>
> This has not been a practical issue in systems - it is very rare to

You evidently know about GTA02...

> explicitly suspend the regulator part of the PMIC.  Obviously the PMIC
> suspend will usually involve killing the power to the processor so it is
> almost always triggered from hardware as part of the processor poweroff
> after software has halted.

That doesn't have to follow at all, there are various kinds of suspend 
and it's legal for the PMU to have a policy to turn off various assets 
at the various levels of power saving.  Nor if I understood it is 
regulator API requiring a handshake signal scheme from the CPU to manage 
suspend, it's open how it is done and supports regulators and CPUs that 
don't have that arrangement.

> It'd be nice to explicitly save and restore the regulator state over
> suspend and resume for devices that don't support that natively but
> that's mostly orthogonal and mostly just flops out of devices doing
> their own management at suspend and resume anyway.
>
>> It would be very good is this was regularized somehow into a
>> standard API because it's absolutely a requirement for many boards
>> that their devices ARE dependent on PMU as a parent.
>
> The only system I'm aware of which had any stuff like this was the
> GTA02.  I never investigated to figure out why the code there was doing
> what it was.

I wouldn't be so quick to assert that, you do not know all the code out 
there in order to tell.  I seem to read threads regularly here about 
furrowed brows on unstable or broken suspend that can easily be due to 
this kind of race.

Anyway, you reject it's a problem, understood :-)

-Andy

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

* Device probe order (i2c regulator vs. platform device)
  2010-03-24 11:27       ` Andy Green
@ 2010-03-24 12:19         ` Mark Brown
  2010-03-24 12:29           ` Andy Green
  0 siblings, 1 reply; 19+ messages in thread
From: Mark Brown @ 2010-03-24 12:19 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Mar 24, 2010 at 11:27:32AM +0000, Andy Green wrote:

> You're missing the point.

No, I'm not.  Much of what you're saying appears to be predicated on the
idea that there is some explicit suspend and resume activity done by the
PMIC driver which needs to happen before anything else can run.

> On any board with a PMU if you look at the schematic you will see
> most of the assets on the board are totally dependent on the PMU
> power arrangements.

Right, clearly - and the fact that this includes things like the
processor and the RAM makes life a lot eaiser for us.

> Take for example the SD card socket with its own regulator from the PMU.

> In Linux reasonably enough MMC is not related to I2C at all in the
> default case.  Therefore during suspend, nothing stops I2C child
> suspend completing before the MMC stack suspend completes.

This only makes a practical difference if the I2C device suspend
actually exists and does something which affects power.  If it's not
doing anything then it really doesn't make any practical difference and
we don't need to worry about it in software.  This is where the CPU and
RAM involvement help us by requiring the PMICs handle much of this in
hardware.

For the bus controller itself there's a much more general issue which
most platforms resolve by either using the _noirq suspend operations,
not suspending at all (on some platforms there's just not anything to
do), or suspending as part of the arch suspend work.  I2C tends to be
used in enough applications that cause sequencing issues that this is
required anyway and has often been solved before anyone tries to use
software controlled regulators.

Note also that the suspend and resume ordering flow from the ordering
with which things appear at system startup so solving the problem at
startup tends to mean you've coped with things well enough.  See the
thread on allowing us to explicitly specify interdependencies for the
device graph that occurred during the last release cycle for Linus'
analysis of this.

> The solution is to make explicit the device hierarchy relationship
> using the parent member of struct device.  On a typical board, it
> means most of your devices are children on the PMU -- and that
> reflects the reality.

It's not just devices that depend on the PMU, it's other things like the
CPU and RAM that don't even appear in the device tree as well.

> To do that, you need a post-PMU-probe callback into the machine
> file, or some even nicer API arrangement you're in a good position
> to invent :-)

This isn't really regulator specific, it's a generic problem that
affects other areas too (this sort of multi-chip arrangement is hardly
unusual in embedded cases, and I understand there are some similar
issues on PCs) and the regulator API doesn't have the information to
deal with anyway.  You're really looking for something at the device
core level here.

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

* Device probe order (i2c regulator vs. platform device)
  2010-03-24 12:19         ` Mark Brown
@ 2010-03-24 12:29           ` Andy Green
  0 siblings, 0 replies; 19+ messages in thread
From: Andy Green @ 2010-03-24 12:29 UTC (permalink / raw)
  To: linux-arm-kernel

On 03/24/10 12:19, Somebody in the thread at some point said:

(snipped what's covered elsewhere)

> For the bus controller itself there's a much more general issue which
> most platforms resolve by either using the _noirq suspend operations,
> not suspending at all (on some platforms there's just not anything to
> do), or suspending as part of the arch suspend work.  I2C tends to be
> used in enough applications that cause sequencing issues that this is
> required anyway and has often been solved before anyone tries to use
> software controlled regulators.

I dunno what that solution looks like but I doubt it's very pretty.  I 
have several times seen "other driver is in suspend" flags that defeat 
the operation if so, then there is unpredictable behaviour (even if some 
deadlock is avoided).

If the devices are explicit children of the PMU then there can never be 
a race about doing i2c-based activity that the PMU relys on.

> Note also that the suspend and resume ordering flow from the ordering
> with which things appear at system startup so solving the problem at
> startup tends to mean you've coped with things well enough.  See the
> thread on allowing us to explicitly specify interdependencies for the
> device graph that occurred during the last release cycle for Linus'
> analysis of this.

Yeah I agree totally, but to enforce the system startup order in a solid 
way it's also needed to have probe callbacks.

>> To do that, you need a post-PMU-probe callback into the machine
>> file, or some even nicer API arrangement you're in a good position
>> to invent :-)
>
> This isn't really regulator specific, it's a generic problem that
> affects other areas too (this sort of multi-chip arrangement is hardly
> unusual in embedded cases, and I understand there are some similar
> issues on PCs) and the regulator API doesn't have the information to
> deal with anyway.  You're really looking for something at the device
> core level here.

That's right, you just reminded me of another non-PMU race where 
framebuffer device raced on LCM controller chip driver suspend, and IIRC 
framebuffer device again raced on backlight which raced on PMU 
availability to set the level.

All of that nightmare dispersed into order once the device hierarchy was 
set properly (which required probe callbacks at init-time).

So I think you have it correct, some kind of generic probe callback 
allowing init staging and parent setting would solve everything.

-Andy

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

* Device probe order (i2c regulator vs. platform device)
  2010-03-24 11:11   ` Marek Szyprowski
@ 2010-03-24 12:50     ` Andy Green
  2010-03-24 13:11       ` Mark Brown
  0 siblings, 1 reply; 19+ messages in thread
From: Andy Green @ 2010-03-24 12:50 UTC (permalink / raw)
  To: linux-arm-kernel

On 03/24/10 11:11, Somebody in the thread at some point said:

Hi -

> Right, this is a really common case. Maybe this callback could be added
> to the regulator framework as well?
>
> On the other hand a more generic solution might be needed, because besides
> the PMU there might be some other dependences between various devices that
> not possible to be ensured in the current framework (I'm thinking of v4l2
> subdevs that link 2 separate devices together, but currently are used only
> with i2c clients).

Right, it's a generic issue with needing to delay other device 
registration until something else has completed probe, not just PMU. 
PMU is just the most common parent.

A callback is needed after probe() returned without error somehow.

-Andy

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

* Device probe order (i2c regulator vs. platform device)
  2010-03-24 12:50     ` Andy Green
@ 2010-03-24 13:11       ` Mark Brown
  2010-03-24 13:22         ` Andy Green
  0 siblings, 1 reply; 19+ messages in thread
From: Mark Brown @ 2010-03-24 13:11 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Mar 24, 2010 at 12:50:46PM +0000, Andy Green wrote:
> On 03/24/10 11:11, Somebody in the thread at some point said:
> 
> Hi -
> 
> >Right, this is a really common case. Maybe this callback could be added
> >to the regulator framework as well?

Looks like there's somethin up with the quoting configuration in your
mail client.  Apart from the bit where it's removing the name of the
person you're quoting it appears to have dropped a level of indentation
off the above quote, which is from you rather than me.

> >On the other hand a more generic solution might be needed, because besides
> >the PMU there might be some other dependences between various devices that
> >not possible to be ensured in the current framework (I'm thinking of v4l2
> >subdevs that link 2 separate devices together, but currently are used only
> >with i2c clients).

> Right, it's a generic issue with needing to delay other device
> registration until something else has completed probe, not just PMU.
> PMU is just the most common parent.

> A callback is needed after probe() returned without error somehow.

I rather suspect that the approach you suggest would have some usability
issues, especially on PCs which don't by and by large have custom board
code at all.  Something more data driven, or handled more in drivers, is
probably in order if this does get done.  Anyway, this should probably
be discussed elsewhere.

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

* Device probe order (i2c regulator vs. platform device)
  2010-03-24 13:11       ` Mark Brown
@ 2010-03-24 13:22         ` Andy Green
  0 siblings, 0 replies; 19+ messages in thread
From: Andy Green @ 2010-03-24 13:22 UTC (permalink / raw)
  To: linux-arm-kernel

On 03/24/10 13:11, Somebody in the thread at some point said:
> On Wed, Mar 24, 2010 at 12:50:46PM +0000, Andy Green wrote:
>> On 03/24/10 11:11, Somebody in the thread at some point said:
>>
>> Hi -
>>
>>> Right, this is a really common case. Maybe this callback could be added
>>> to the regulator framework as well?
>
> Looks like there's somethin up with the quoting configuration in your

Must be bugs in rawhide thunderbird.

>> Right, it's a generic issue with needing to delay other device
>> registration until something else has completed probe, not just PMU.
>> PMU is just the most common parent.
>
>> A callback is needed after probe() returned without error somehow.
>
> I rather suspect that the approach you suggest would have some usability
> issues, especially on PCs which don't by and by large have custom board
> code at all.  Something more data driven, or handled more in drivers, is
> probably in order if this does get done.  Anyway, this should probably
> be discussed elsewhere.

Fine, at the moment there are no arrangements for it and you have to go 
hack callbacks in case by case, so anything would be better.

I agree it's not arm-specific so this isn't the place, but it'll be sad 
if it just disappears off the radar.

-Andy

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

* Device probe order (i2c regulator vs. platform device)
  2010-03-24 12:11     ` Andy Green
@ 2010-03-24 14:01       ` Mark Brown
  2010-03-24 14:38         ` Andy Green
  0 siblings, 1 reply; 19+ messages in thread
From: Mark Brown @ 2010-03-24 14:01 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Mar 24, 2010 at 12:11:41PM +0000, Andy Green wrote:
> On 03/24/10 11:32, Somebody in the thread at some point said:
> >On Wed, Mar 24, 2010 at 09:19:58AM +0000, Andy Green wrote:

> >>Otherwise your suspend and resume ordering turns to crap and you
> >>will immediately be in Hell, for example the PMIC may go down taking
> >>away power from the SD Card before you finished talking to it.

> >This has not been a practical issue in systems - it is very rare to

> You evidently know about GTA02...

Not really, like I said I never really looked at what was going on there
(and the problem was never reported upstream so...).

> >explicitly suspend the regulator part of the PMIC.  Obviously the PMIC
> >suspend will usually involve killing the power to the processor so it is
> >almost always triggered from hardware as part of the processor poweroff
> >after software has halted.

> That doesn't have to follow at all, there are various kinds of
> suspend and it's legal for the PMU to have a policy to turn off
> various assets at the various levels of power saving.  Nor if I
> understood it is regulator API requiring a handshake signal scheme
> from the CPU to manage suspend, it's open how it is done and
> supports regulators and CPUs that don't have that arrangement.

It's not really the regulator API that needs the hardware handshake,
it's the hardware itself.  Everything we've seen thus far either wants
to do something to the core system power rails during suspend mode which
would immediately crash if done while the system is running or doesn't
want to do anything at all, in which case it's just device level stuff
going on which they can deal with.  Either way, the suspend of the PMIC
devices themselves isn't that interesting for sequencing.

At present we don't have any regulators with suspend or resume
operations.

> >The only system I'm aware of which had any stuff like this was the
> >GTA02.  I never investigated to figure out why the code there was doing
> >what it was.

> I wouldn't be so quick to assert that, you do not know all the code
> out there in order to tell.

I really am very sure that nobody has ever reported any issues here -
the only reason I know about the GTA02 is from following development.
There may be non-regulator issues, or there may be unreported issues but
nobody's come forward with those.

Until we have real systems we can look at I'm not really sure it's worth
worrying about since there's a risk of desigining something that doesn't
fit well with what people actually need, and there's always the chance
that something like the device layer will solve the problem before
anyone notices.

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

* Device probe order (i2c regulator vs. platform device)
  2010-03-24 14:01       ` Mark Brown
@ 2010-03-24 14:38         ` Andy Green
  2010-03-25 10:22           ` Mark Brown
  0 siblings, 1 reply; 19+ messages in thread
From: Andy Green @ 2010-03-24 14:38 UTC (permalink / raw)
  To: linux-arm-kernel

On 03/24/10 14:01, Somebody in the thread at some point said:

>>> The only system I'm aware of which had any stuff like this was the
>>> GTA02.  I never investigated to figure out why the code there was doing
>>> what it was.
>
>> I wouldn't be so quick to assert that, you do not know all the code
>> out there in order to tell.
>
> I really am very sure that nobody has ever reported any issues here -

There's plenty of out-of-tree code.  I'm not trying to blame you for 
something, just it's too strong to say there's no problem here because 
you personally didn't see it in your neck of the woods (especially when 
I spent many weeks fighting exactly that on GTA02).

There problem's generic -->

> Until we have real systems we can look at I'm not really sure it's worth
> worrying about since there's a risk of desigining something that doesn't
> fit well with what people actually need, and there's always the chance
> that something like the device layer will solve the problem before
> anyone notices.

No I agree after both Marek's and your discussion (and remembering the 
other races I had seen), the problem is bigger than regulator API and 
the device layer would be the right place.

You're right someone needs to bring it up elsewhere.

-Andy

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

* Device probe order (i2c regulator vs. platform device)
  2010-03-24 14:38         ` Andy Green
@ 2010-03-25 10:22           ` Mark Brown
  2010-03-25 10:52             ` Andy Green
  0 siblings, 1 reply; 19+ messages in thread
From: Mark Brown @ 2010-03-25 10:22 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Mar 24, 2010 at 02:38:31PM +0000, Andy Green wrote:
> On 03/24/10 14:01, Somebody in the thread at some point said:

> >I really am very sure that nobody has ever reported any issues here -

> There's plenty of out-of-tree code.  I'm not trying to blame you for

I'm really wary of trusting out of tree reports since a high proportion
of the time they turn out to be due to people not being aware of some
existing support or things like API misuse.  New issues do crop up, but
anecdotes about issues that have been diagnosed but not reported need to
be taken with a grain of salt.

Like I say, the regulator drivers just don't do anything in suspend
in mainline which means that ordering issues from them seem a bit
suspicious.

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

* Device probe order (i2c regulator vs. platform device)
  2010-03-25 10:22           ` Mark Brown
@ 2010-03-25 10:52             ` Andy Green
  0 siblings, 0 replies; 19+ messages in thread
From: Andy Green @ 2010-03-25 10:52 UTC (permalink / raw)
  To: linux-arm-kernel

On 03/25/10 10:22, Somebody in the thread at some point said:
> On Wed, Mar 24, 2010 at 02:38:31PM +0000, Andy Green wrote:
>> On 03/24/10 14:01, Somebody in the thread at some point said:
>
>>> I really am very sure that nobody has ever reported any issues here -
>
>> There's plenty of out-of-tree code.  I'm not trying to blame you for
>
> I'm really wary of trusting out of tree reports since a high proportion
> of the time they turn out to be due to people not being aware of some
> existing support or things like API misuse.  New issues do crop up, but
> anecdotes about issues that have been diagnosed but not reported need to
> be taken with a grain of salt.

I can see your point of view, but you sound...

> Like I say, the regulator drivers just don't do anything in suspend
> in mainline which means that ordering issues from them seem a bit
> suspicious.

The I2C bus goes down at some point in suspend, then you can't talk to 
the PMU or other I2C device any more.

If there are other drivers that are trying to do something themselves 
their power when they go down, or indeed ask for anything else from 
another driver who is on a separate bus and they don't have any formal 
relationship with it hierarchy-wise, then it's racing with the other 
driver or the bus going down.

In that case, those drivers who want to ask for things in suspend or 
resume, or who trigger indirectly those requests at that time, need to 
mark themselves as being dependencies of the thing they're reliant on 
talking to.  Then everything is taken care of.

GTA02 had a bunch of these kind of relationships... it's possible they 
were all evil and could be eliminated but actually framebuffer vs LCM 
controller vs PMU vs backlight driver race I dunno what else you're 
meant to do except make the relationship explicit and everything is 
cool.  Backlight really is dependent on PMU being there if it wants to 
change state while it goes down (eg, fade down itself), correct thing 
really does seem to be make the dependency explicit via the device 
parent stuff.

If you have a better way, I am standing by ready to learn something.

-Andy

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

end of thread, other threads:[~2010-03-25 10:52 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-03-24  7:10 Device probe order (i2c regulator vs. platform device) Marek Szyprowski
2010-03-24  9:19 ` Andy Green
2010-03-24 11:11   ` Marek Szyprowski
2010-03-24 12:50     ` Andy Green
2010-03-24 13:11       ` Mark Brown
2010-03-24 13:22         ` Andy Green
2010-03-24 11:32   ` Mark Brown
2010-03-24 12:11     ` Andy Green
2010-03-24 14:01       ` Mark Brown
2010-03-24 14:38         ` Andy Green
2010-03-25 10:22           ` Mark Brown
2010-03-25 10:52             ` Andy Green
2010-03-24 10:21 ` Mark Brown
2010-03-24 10:57   ` Andy Green
2010-03-24 11:02   ` Marek Szyprowski
2010-03-24 11:08     ` Mark Brown
2010-03-24 11:27       ` Andy Green
2010-03-24 12:19         ` Mark Brown
2010-03-24 12:29           ` Andy Green

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.