Re: [PATCH v3 1/2] hwmon: (lm90) Add power control

From: Mark Brown
Date: Mon Sep 09 2013 - 12:03:41 EST


On Mon, Sep 09, 2013 at 08:50:43AM -0700, Guenter Roeck wrote:
> On Mon, Sep 09, 2013 at 02:50:22PM +0100, Mark Brown wrote:

> > And indeed it does this (well, it does whatever the driver says in terms
> > of delay). However it is possible that the lm90 needs this time for
> > itself - if it's doing some sort of initialisation or callibration
> > sequence then that'll happen after the supplies come up. 25ms did seem
> > rather long, especially for such simple devices, but it's not beyond the
> > bounds of possibility.

> Even then it would be unreasonable to enforce such a delay for every instance
> of this driver, even if the regulator is a dummy one and/or has been enabled
> already. Many if not almost all users of the driver work just fine as-is,
> without additional enforced delay (and, for that matter, without regulator ;).

> If there is a delay, it would have to be optional: Only wait if the regulator
> was really turned on by the call and not already active. I don't know if the
> regulator subsystem has this capability; if not, maybe it should.

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.

> 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.

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).

Attachment: signature.asc
Description: Digital signature