Re: [PATCH] isl29020: ambient light sensor

[Alan Cox <alan@lxorguk.ukuu.org.uk>]

On Fri, 22 Oct 2010 12:51:30 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Fri, 22 Oct 2010 13:46:07 +0100
> Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> 
> > From: Kalhan Trisal <kalhan.trisal@intel.com>
> > 
> > The LS driver will read the latest Lux measurement based upon the
> > light brightness and will report the LUX output through sysfs interface.
> > 
> > This hardware isn't quite the same as the ISL29003 so has a different driver.
> > 
> >
> > ...
> >
> > +static ssize_t als_lux_input_data_show(struct device *dev,
> > +			struct device_attribute *attr, char *buf)
> > +{
> > +	struct i2c_client *client = to_i2c_client(dev);
> > +	int ret_val, val;
> > +	unsigned long int lux;
> > +	int temp;
> > +
> > +	pm_runtime_get_sync(dev);
> > +	msleep(100);
> 
> The msleep() is a bit mysterious.  And long!  A little code comment
> explaining why it's here would illuminate things (heh, I kill me).

It takes that long to get a sensible answer from the device after you
power it up and sysfs doesn't keep enough context around to do poll etc.

It may be possible to shorten it if we've not been suspended but there
isn't a sane way I've found to track last suspended time nicely and race
free in the runtime_pm layer.

> Could/should we make the PM code disappear if !CONFIG_PM?  Such as

Looks sensible. Will add to it.

Alan