linux-kernel - Re: [PATCH] light sensor: Add a calibscale file to the isl29018 light sensor driver.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1309299480.10209.13.camel@rklein-linux2>
Date:	Tue, 28 Jun 2011 15:18:00 -0700
From:	Rhyland Klein <rklein@...dia.com>
To:	Bryan Freed <bfreed@...omium.org>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
	"jic23@....ac.uk" <jic23@....ac.uk>,
	"gregkh@...e.de" <gregkh@...e.de>, "arnd@...db.de" <arnd@...db.de>
Subject: Re: [PATCH] light sensor: Add a calibscale file to the isl29018
 light sensor driver.

On Tue, 2011-06-28 at 15:00 -0700, Bryan Freed wrote:
> Defaulting to 1, this gives a way to amplify the lux value before being
> reduced by the programmed adc_bit shift.
> Only support whole numbers right now.  When this driver is converted to the new
> IIO_CHAN framework, it will be easy to support the framework's pseudo float.
> 
> Signed-off-by: Bryan Freed <bfreed@...omium.org>
> ---
>  drivers/staging/iio/light/isl29018.c |   35 +++++++++++++++++++++++++++++++++-
>  1 files changed, 34 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/isl29018.c b/drivers/staging/iio/light/isl29018.c
> index 4794ffd..cd88311 100644
> --- a/drivers/staging/iio/light/isl29018.c
> +++ b/drivers/staging/iio/light/isl29018.c
> @@ -57,6 +57,7 @@ struct isl29018_chip {
>  	struct iio_dev		*indio_dev;
>  	struct i2c_client	*client;
>  	struct mutex		lock;
> +	unsigned int		lux_scale;
>  	unsigned int		range;
>  	unsigned int		adc_bit;
>  	int			prox_scheme;
> @@ -166,7 +167,7 @@ static int isl29018_read_lux(struct i2c_client *client, int *lux)
>  	if (lux_data < 0)
>  		return lux_data;
>  
> -	*lux = (lux_data * chip->range) >> chip->adc_bit;
> +	*lux = (lux_data * chip->range * chip->lux_scale) >> chip->adc_bit;

This calibscale property changes the output of the _input property.
Properties defined with _input I believe are designed to return values
in SI units. Wouldn't changing the scale, say to 1000, change the units
from like lux to say kilolux? Wouldn't it be more appropriate then to
return this calibrated value through illuminance_raw ?

>  
>  	return 0;
>  }
> @@ -264,6 +265,34 @@ static ssize_t get_sensor_data(struct device *dev, char *buf, int mode)
>  }
>  
>  /* Sysfs interface */
> +/* lux_scale */
> +static ssize_t show_lux_scale(struct device *dev,
> +			struct device_attribute *attr, char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct isl29018_chip *chip = indio_dev->dev_data;
> +
> +	return sprintf(buf, "%d\n", chip->lux_scale);
> +}
> +
> +static ssize_t store_lux_scale(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t count)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct isl29018_chip *chip = indio_dev->dev_data;
> +	unsigned long lval;
> +
> +	lval = simple_strtoul(buf, NULL, 10);
> +	if (lval == 0)
> +		return -EINVAL;
> +
> +	mutex_lock(&chip->lock);
> +	chip->lux_scale = lval;
> +	mutex_unlock(&chip->lock);
> +
> +	return count;
> +}
> +
>  /* range */
>  static ssize_t show_range(struct device *dev,
>  			struct device_attribute *attr, char *buf)
> @@ -412,6 +441,8 @@ static IIO_DEVICE_ATTR(proximity_on_chip_ambient_infrared_supression,
>  					show_prox_infrared_supression,
>  					store_prox_infrared_supression, 0);
>  static IIO_DEVICE_ATTR(illuminance0_input, S_IRUGO, show_lux, NULL, 0);
> +static IIO_DEVICE_ATTR(illuminance0_calibscale, S_IRUGO | S_IWUSR,
> +					show_lux_scale, store_lux_scale, 0);
>  static IIO_DEVICE_ATTR(intensity_infrared_raw, S_IRUGO, show_ir, NULL, 0);
>  static IIO_DEVICE_ATTR(proximity_raw, S_IRUGO, show_proxim_ir, NULL, 0);
>  
> @@ -424,6 +455,7 @@ static struct attribute *isl29018_attributes[] = {
>  	ISL29018_CONST_ATTR(adc_resolution_available),
>  	ISL29018_DEV_ATTR(proximity_on_chip_ambient_infrared_supression),
>  	ISL29018_DEV_ATTR(illuminance0_input),
> +	ISL29018_DEV_ATTR(illuminance0_calibscale),
>  	ISL29018_DEV_ATTR(intensity_infrared_raw),
>  	ISL29018_DEV_ATTR(proximity_raw),
>  	NULL
> @@ -478,6 +510,7 @@ static int __devinit isl29018_probe(struct i2c_client *client,
>  
>  	mutex_init(&chip->lock);
>  
> +	chip->lux_scale = 1;
>  	chip->range = 1000;
>  	chip->adc_bit = 16;
>  

Also, please update
drivers/staging/iio/Documentation/sysfs-bus-iio-light with definitions
for the calibscale property so that it is properly documented there.

-rhyland

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/