iio: light: Adding driver for ISL29018 ALS

PATCH Superseded [email protected] 4 participants 7 messages Oct 7, 2010 – Oct 9, 2010 4 messages not in the archive

Subsystems (4)

Staging IIO I2C subsystem host drivers Staging - industrial io

lore

Share & tools

lore

[email protected]

author

2010-10-07 19:48 UTC #0 lore raw

PATCH v5 1/1 iio: light: Adding driver for ISL29018 ALS

From: Rhyland Klein <[email protected]>

adding support for the ISL 29018 ambient light and proximity sensor.

Addressed comments from reviews by Jonathan Cameron and Joe Perches
* Removed some excess dbg prints that only printed function name
* Renamed some properties to make them more descriptive
* Added a property to list available adc resolutions
* Defined arrays for resolutions/ranges as static const
* Change loops initialization to memset for extensibility.
* used sizeof() instead of ARRAY_SIZE() to be safer
* Added a property to list available adc ranges

Signed-off-by Rhyland Klein <[email protected]>

Acked-by Jonathan Cameron <jic23-KWPb1pKIrIJaa/[email protected]>

---
 .../staging/iio/Documentation/sysfs-bus-iio-light  |   64 +++
 drivers/staging/iio/light/Kconfig                  |   12 +
 drivers/staging/iio/light/Makefile                 |    1 +
 drivers/staging/iio/light/isl29018.c               |  563 ++++++++++++++++++++
 4 files changed, 640 insertions(+), 0 deletions(-)
 create mode 100644 drivers/staging/iio/Documentation/sysfs-bus-iio-light
 create mode 100644 drivers/staging/iio/light/isl29018.c

drivers/staging/iio/Documentation/sysfs-bus-iio-light new file +64 -0

new file mode 100644
index 0000000..5d84856
@@ -0,0 +1,64 @@
+
+What:		/sys/bus/iio/devices/device[n]/range
+KernelVersion:	2.6.37
+Contact:	[email protected]
+Description:
+		Hardware dependent ADC Full Scale Range used for some ambient
+		light sensors in calculating lux.
+
+What:		/sys/bus/iio/devices/device[n]/range_available
+KernelVersion:	2.6.37
+Contact:	[email protected]
+Description:
+		Hardware dependent supported vales for ADC Full Scale Range.
+
+What:		/sys/bus/iio/devices/device[n]/adc_resolution
+KernelVersion:	2.6.37
+Contact:	[email protected]
+Description:
+		Hardware dependent ADC resolution of the ambient light sensor
+		used in calculating the lux.
+
+What:		/sys/bus/iio/devices/device[n]/adc_resolution_available
+KernelVersion:	2.6.37
+Contact:	[email protected]
+Description:
+		Hardware dependent list of possible values supported for the
+		adc_resolution of the given sensor.
+
+What:		/sys/bus/iio/devices/device[n]/illuminance0[_input|_raw]
+KernelVersion:	2.6.35
+Contact:	[email protected]
+Description:
+		This should return the calculated lux from the light sensor. If
+		it comes back in SI units, it should also include _input else it
+		should include _raw to signify it is not in SI units.
+
+What:		/sys/.../device[n]/proximity_on_chip_ambient_infrared_supression
+KernelVersion:	2.6.37
+Contact:	[email protected]
+Description:
+		Hardware dependent mode for an ALS device to calculate the value
+		in proximity mode. When this is enabled, then the device should
+		use a infrared sensor reading to remove infrared noise from the
+		proximity reading. If this is not enabled, the driver can still
+		do this calculation manually by reading the infrared sensor
+		value and doing the negation in sw.
+
+What:		/sys/bus/iio/devices/device[n]/proximity[_input|_raw]
+KernelVersion:	2.6.37
+Contact:	[email protected]
+Description:
+		This property is supported by proximity sensors and should be
+		used to return the value of a reading by the sensor. If this
+		value is returned in SI units, it should also include _input
+		but if it is not, then it should include _raw.
+
+What:		/sys/bus/iio/devices/device[n]/intensity_infrared[_input|_raw]
+KernelVersion:	2.6.37
+Contact:	[email protected]
+Description:
+		This property is supported by sensors that have an infrared
+		sensing mode. This value should be the output from a reading
+		and if expressed in SI units, should include _input. If this
+		value is not in SI units, then it should include _raw.

drivers/staging/iio/light/Kconfig +12 -0

index 3ddc478..36d8bbe 100644
@@ -12,3 +12,15 @@ config SENSORS_TSL2563
 
 	 This driver can also be built as a module.  If so, the module
 	 will be called tsl2563.
+
+config SENSORS_ISL29018
+        tristate "ISL 29018 light and proximity sensor"
+        depends on I2C
+        default n
+        help
+         If you say yes here you get support for ambient light sensing and
+         proximity infrared sensing from Intersil ISL29018.
+         This driver will provide the measurements of ambient light intensity
+         in lux, proximity infrared sensing and normal infrared sensing.
+         Data from sensor is accessible via sysfs.
+

drivers/staging/iio/light/Makefile +1 -0

index 30f3300..9142c0e 100644
@@ -3,3 +3,4 @@
 #
 
 obj-$(CONFIG_SENSORS_TSL2563)	+= tsl2563.o
+obj-$(CONFIG_SENSORS_ISL29018)	+= isl29018.o

drivers/staging/iio/light/isl29018.c new file +563 -0

563 lines not on this page · raw message

Signature · 1 line

1.7.0.4

A message that is not in the archive (sent off-list or not yet received): 1286480883-25589-1-git-send-email-rklein-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org

Rory Filer

2010-10-07 21:37 UTC #2 lore raw

Newbie Question About Dev-Interface Method

I'm working on a proof-of-concept device which contains several chips connected to the same adapter (/dev/ic2-0) on the I2C bus. Some of these have kernel-space drivers (e.g. GPIO Expander) and some don't. For the ones that don't I've opted to use the method described in the kernel Documentation/i2c/dev-interface document. This works great and does all I need for now. This is the general procedure I'm following as outlined in kernel - Documentation/i2c/dev-interface.

- open /dev/i2c-N
- Set the chip's address by calling ioctl(file, I2C_SLAVE, address)
- interact with the chip using read/write or smb calls, etc.
- close the file when done

I'm assuming that a kernel space chip driver like our gpio expander (pca953x.c) also uses the same adapter driver as my user-space application since that expander chip is on the same bus. My question is, what happens if a kernel driver is using the bus at the same time my user-mode application is? Even if the user space app. may not be running when the chip driver runs, the adapter has an address written into it via the ioctl() call, I noted above. Can unpredictable results occur in this scenario? I think I could protect against it by writing a thread to serialize accesses to the I2C bus, but that only works if my entire application is running inside one process. I'm hoping someone on the list might be able to advise.

Regards,

Rory

A message that is not in the archive (sent off-list or not yet received): 8F90F944E50427428C60E12A34A309D2308DA559B4-3qf8vkpM5jTbmvMHnzRVpW3Pdy6AhKVLXbPIYa3/[email protected]

Jean Delvare

2010-10-08 06:57 UTC #4 lore raw

Re: Newbie Question About Dev-Interface Method

Hi Rory,

Quoted 8 lines · Rory Filer

I'm working on a proof-of-concept device which contains several chips connected to the same adapter (/dev/ic2-0) on the I2C bus. Some of these have kernel-space drivers (e.g. GPIO Expander) and some don't. For the ones that don't I've opted to use the method described in the kernel Documentation/i2c/dev-interface document. This works great and does all I need for now. This is the general procedure I'm following as outlined in kernel - Documentation/i2c/dev-interface.
- open /dev/i2c-N
- Set the chip's address by calling ioctl(file, I2C_SLAVE, address)
- interact with the chip using read/write or smb calls, etc.
- close the file when done
I'm assuming that a kernel space chip driver like our gpio expander (pca953x.c) also uses the same adapter driver as my user-space application since that expander chip is on the same bus.

Let's clarify right away: i2c-dev (which handles /dev/i2c-* nodes) is NOT an adapter driver. It is a bridge driver between user-space and your adapter driver. Your adapter driver is most likely one living in drivers/i2c/busses, and depends on the actual hardware you run on.

Quoted 1 line

My question is, what happens if a kernel driver is using the bus at the same time my user-mode application is?

It will work fine.

Quoted 1 line

Even if the user space app. may not be running when the chip driver runs, the adapter has an address written into it via the ioctl() call, I noted above.

Incorrect. The address isn't written to the adapter, it is a setting local to i2c-dev and even specific to the file handle you got when opening a /dev/i2c-* node. Each subsequent read, write or smbus call in your user-space driver automatically includes the slave address, which is only valid for the read, write or smbus call in question.

Quoted 1 line

Can unpredictable results occur in this scenario?

No.

Quoted 1 line

I think I could protect against it by writing a thread to serialize accesses to the I2C bus, but that only works if my entire application is running inside one process. I'm hoping someone on the list might be able to advise.

The i2c-core module already serializes all accesses to every given i2c adapter through i2c_transfer() and i2c_smbus_xfer().

Signature · 2 lines

Jean Delvare
http://khali.linux-fr.org/wishlist.html

A message that is not in the archive (sent off-list or not yet received): [email protected]

Rory Filer

2010-10-09 02:03 UTC #6 lore raw

RE: Newbie Question About Dev-Interface Method

Quoted 8 lines

Quoted 1 line
I'm assuming that a kernel space chip driver like our gpio expander
(pca953x.c) also uses the same adapter driver as my user-space application since that expander chip is on the same bus.
Let's clarify right away: i2c-dev (which handles /dev/i2c-* nodes) is NOT an adapter driver. It is a bridge driver between user-space and your adapter driver. Your adapter driver is most likely one living in drivers/i2c/busses, and depends on the actual hardware you run on.

OK, thanks that's clear. I was confused by the terminology in the dev-interface document. When it said to open an adapter, I thought that meant I was using the associated adapter driver.

Quoted 6 lines

Incorrect. The address isn't written to the adapter, it is a setting local to i2c-dev and even specific to the file handle you got when opening a /dev/i2c-* node. Each subsequent read, write or smbus call in your user-space driver automatically includes the slave address, which is only valid for the read, write or smbus call in question.

Excellent! - in the words of a famous Swiss restaurant's wine menu, this leaves me nothing to hope for!

Jean, thank you for your help, it saved me a ton pouring over source code and allowed me to get on with the job at hand. Much appreciated.

Regards,

Rory

A message that is not in the archive (sent off-list or not yet received): 8F90F944E50427428C60E12A34A309D2308DAD1B7F-3qf8vkpM5jTbmvMHnzRVpW3Pdy6AhKVLXbPIYa3/[email protected]

Jean Delvare

2010-10-09 07:33 UTC #8 lore raw

Re: Newbie Question About Dev-Interface Method

Quoted 4 lines · Rory Filer

Quoted 4 lines
Quoted 1 line
I'm assuming that a kernel space chip driver like our gpio expander
(pca953x.c) also uses the same adapter driver as my user-space application since that expander chip is on the same bus.

Quoted 8 lines

Quoted 4 lines
Let's clarify right away: i2c-dev (which handles /dev/i2c-* nodes) is NOT an adapter driver. It is a bridge driver between user-space and your adapter driver. Your adapter driver is most likely one living in drivers/i2c/busses, and depends on the actual hardware you run on.
OK, thanks that's clear. I was confused by the terminology in the dev-interface document. When it said to open an adapter, I thought that meant I was using the associated adapter driver.

Well, you end up using the underlying adapter driver. i2c-dev is only a glue driver between user-space and the actual in-kernel i2c adapter driver. See
https://i2c.wiki.kernel.org/index.php/Driver_Architecture
for a diagram of the architecture.

Quoted 5 lines

Quoted 5 lines
Incorrect. The address isn't written to the adapter, it is a setting local to i2c-dev and even specific to the file handle you got when opening a /dev/i2c-* node. Each subsequent read, write or smbus call in your user-space driver automatically includes the slave address, which is only valid for the read, write or smbus call in question.

I realize my wording may be a little misleading. When you set a slave address using the I2C_SLAVE ioctl, the address is valid for _all_ subsequent read, write or smbus calls on the filehandle, until either the file is closed or ioctl(I2C_SLAVE) is called again, not only the following one.

Quoted 5 lines

Excellent! - in the words of a famous Swiss restaurant's wine menu, this leaves me nothing to hope for!
Jean, thank you for your help, it saved me a ton pouring over source code and allowed me to get on with the job at hand. Much appreciated.

You're welcome.

Signature · 2 lines

Jean Delvare
http://khali.linux-fr.org/wishlist.html

Greg KH

2010-10-07 21:41 UTC #9 lore raw

Re: [PATCH v5] staging: iio: light: Adding driver for ISL29018 ALS

In reply to [email protected]

Quoted 13 lines · On Thu, Oct 07, 2010 at 12:48:03PM -0700…

From: Rhyland Klein <[email protected]>
adding support for the ISL 29018 ambient light and proximity sensor.
Addressed comments from reviews by Jonathan Cameron and Joe Perches
* Removed some excess dbg prints that only printed function name
* Renamed some properties to make them more descriptive
* Added a property to list available adc resolutions
* Defined arrays for resolutions/ranges as static const
* Change loops initialization to memset for extensibility.
* used sizeof() instead of ARRAY_SIZE() to be safer
* Added a property to list available adc ranges

Quoted 2 lines

Signed-off-by Rhyland Klein <[email protected]>
Acked-by Jonathan Cameron <jic23-KWPb1pKIrIJaa/[email protected]>

Too bad no one actually compiled this driver:
CC [M] drivers/staging/iio/light/isl29018.o
drivers/staging/iio/light/isl29018.c:420:8: error: ‘show_prox_infrared_suppression’ undeclared here (not in a function) drivers/staging/iio/light/isl29018.c:433:1: error: ‘iio_dev_attr_range_available’ undeclared here (not in a function) drivers/staging/iio/light/isl29018.c:435:1: error: ‘iio_dev_attr_adc_resolution_available’ undeclared here (not in a function) drivers/staging/iio/light/isl29018.c: In function ‘isl29018_chip_init’:
drivers/staging/iio/light/isl29018.c:451:6: warning: unused variable ‘i’ drivers/staging/iio/light/isl29018.c: At top level:
drivers/staging/iio/light/isl29018.c:350:16: warning: ‘show_prox_infrared_supression’ defined but not used drivers/staging/iio/light/isl29018.c:416:1: warning: ‘iio_const_attr_range_available’ defined but not used drivers/staging/iio/light/isl29018.c:417:1: warning: ‘iio_const_attr_adc_resolution_available’ defined but not used make[2]: *** [drivers/staging/iio/light/isl29018.o] Error 1
make[1]: *** [drivers/staging/iio/light] Error 2
make: *** [_module_drivers/staging/iio] Error 2

Please fix this up before resending it.

thanks,

greg k-h

Rhyland Klein

author

2010-10-07 22:39 UTC #10 lore raw

RE: [PATCH v5] staging: iio: light: Adding driver for ISL29018 ALS

Sorry resending new patch (compiles fine for me).

Quoted 52 lines

-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Thursday, October 07, 2010 2:42 PM
To: Rhyland Klein
Cc: [email protected]; [email protected]; Andrew Chew; [email protected]; linux- [email protected]; [email protected]; linux- [email protected]; Laxman Dewangan
Subject: Re: [PATCH v5] staging: iio: light: Adding driver for ISL29018 ALS
Quoted 13 lines · On Thu, Oct 07, 2010 at 12:48:03PM -0700, [email protected] wrote:
From: Rhyland Klein <[email protected]>
adding support for the ISL 29018 ambient light and proximity sensor.
Addressed comments from reviews by Jonathan Cameron and Joe Perches
* Removed some excess dbg prints that only printed function name
* Renamed some properties to make them more descriptive
* Added a property to list available adc resolutions
* Defined arrays for resolutions/ranges as static const
* Change loops initialization to memset for extensibility.
* used sizeof() instead of ARRAY_SIZE() to be safer
* Added a property to list available adc ranges
Quoted 2 lines
Signed-off-by Rhyland Klein <[email protected]>
Acked-by Jonathan Cameron <[email protected]>
Too bad no one actually compiled this driver:
CC [M] drivers/staging/iio/light/isl29018.o
drivers/staging/iio/light/isl29018.c:420:8: error:
‘show_prox_infrared_suppression’ undeclared here (not in a function) drivers/staging/iio/light/isl29018.c:433:1: error:
‘iio_dev_attr_range_available’ undeclared here (not in a function) drivers/staging/iio/light/isl29018.c:435:1: error:
‘iio_dev_attr_adc_resolution_available’ undeclared here (not in a function) drivers/staging/iio/light/isl29018.c: In function ‘isl29018_chip_init’:
drivers/staging/iio/light/isl29018.c:451:6: warning: unused variable ‘i’ drivers/staging/iio/light/isl29018.c: At top level:
drivers/staging/iio/light/isl29018.c:350:16: warning:
‘show_prox_infrared_supression’ defined but not used drivers/staging/iio/light/isl29018.c:416:1: warning:
‘iio_const_attr_range_available’ defined but not used drivers/staging/iio/light/isl29018.c:417:1: warning:
‘iio_const_attr_adc_resolution_available’ defined but not used make[2]: *** [drivers/staging/iio/light/isl29018.o] Error 1 make[1]: *** [drivers/staging/iio/light] Error 2
make: *** [_module_drivers/staging/iio] Error 2
Please fix this up before resending it.
thanks,
greg k-h