On 09/09/2013 09:05 PM, Wei Ni wrote:
On 09/10/2013 04:39 AM, Mark Brown wrote:
* PGP Signed by an unknown key
On Mon, Sep 09, 2013 at 09:17:35AM -0700, Guenter Roeck wrote:
On Mon, Sep 09, 2013 at 05:02:37PM +0100, Mark Brown wrote:
It does, though it gets complicated trying to use it for a case like
this since you can't really tell if the regulator was powered on
immediately before the device got probed by another device on the bus.
Why not ? Just keep a timestamp.
The support is a callback on state changes; we could keep a timestamp
but there's still going to be race conditions around bootloaders. It's
doable though.
On a higher level, I wonder if such functionality should be added in the i2c
subsystem and not in i2c client drivers. Has anyone thought about this ?
I'm not sure what the subsystem would do for such delays? It's fairly
common for things that need this to also want to do things like
manipulate GPIOs as part of the power on sequence so the applicability
is relatively limited, plus it's not even I2C specific, the same applies
to other buses so it ought to be a driver core thing.
Possibly. I just thought about i2c since it also takes care of basic
devicetree bindings. Something along the line of
if devicetree bindings for this device declare one or more
regulators, enable those regulators before calling the driver
probe function.
That's definitely a driver core thing, not I2C - there's nothing
specific to I2C in there at all, needing power is pretty generic. I
have considered this before, something along the lines of what we have
for pinctrl, but unfortunately the generic case isn't quite generic
enough to make it easy. It'd need to be an explicit list of regulators
(partly just to make it opt in and avoid breaking things) and you'd want
to have a way of handling the different suspend/resume behaviour that
devices want. There's a few patterns there.
It's definitely something I think about from time to time and it would
be useful to factor things out, the issue is getting a good enough model
of what's going on.
There was some work on a generic helper for power on sequences but it
stalled since it wasn't accepted for the original purpose (LCD panel
power ons IIRC).
Too bad. I think it could be kept quite simple, though, by handling it
through the regulator subsystem as suggested above. A generic binding
for a per-regulator and per-device poweron delay should solve that
and possibly even make it transparent to the actual driver code.
Lots of things have a GPIO for reset too, and some want clocks too. For
maximum usefulness this should be cross subsystem. I suspect the reset
controller API may be able to handle some of it.
The regulator power on delays are already handled transparently, by the
time regulator_enable() returns the ramp should be finished.
I think the regulator should encoded its own startup delay. Each
individual device should handle its own requirements for delay after
power is stable.
The regulator_enable() will handle the delays for the regulator device.
And adding the msleep(25) is for lm90 device. If without delay,
sometimes the device can't work properly. If read lm90 register
immediately after enabling regulator, the reading may be failed.
I'm not sure if 25ms is the right value, I read the LM90 SPEC, the max
of "SMBus Clock Low Time" is 25ms, so I supposed that it may need about
25ms to stable after power on.
Problem is that you are always waiting, even if the same regulator was
turned on already, and even if it is a dummy regulator.
Imagine every driver doing that. Booting would take forever, just because of
unnecessary delays all over the place. There has to be a better solution
which does not include a mandatory and potentially unnecessary wait time
in the driver. At a previous company we had a design with literally dozens
of those chip. You really want to force such a boot delay on every user ?
But essentially you don't even know if it is needed; you are just guessing.
That is not an acceptable reason to add such a delay, mandatory or not.
Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
