On 6/17/2012 2:18 PM, Peter Meerwald wrote:
Hello,
Couple of nitpicks inline. My biggest question about
this driver is why are you submitting it to staging?
Can't see any reason it can't go directly into drivers/iio/light.
thank you for reviewing; reason for staging is that I consider the driver
incomplete because IR current control and proximity thresholding/event
handling is missing -- I plan to add these as soon as I have figured out
how to do that properly in IIO; I moved the driver out of staging
nevertheless
cool. I'm keen to move everything we possibly can out of staging
(when ready!) to make for a vaguely sensible bit of code management in
the long run.
+config SENSORS_VCNL4000
+ tristate "VCNL4000 combined ALS and proximity sensor"
How did we end up with that SENSORS prefix. I think it might be a legacy of
a few drivers that were intially submitted to hwmon and moved here.
Probably best to drop it.
drivers/iio/light/Kconfig has CONFIG_SENSORS_LM3533
drivers/staging/iio/light/Kconfig has 3x CONFIG_SENSORS, 2x CONFIG_
drivers/staging/iio/magnetometer has CONFIG_SENSORS_
hwmon/ and misc/ uses CONFIG_SENSORS; I wonder how many sensor drivers do
not fly SENSORS_?
The misc ones are mostly drivers that were initially submitted to hwmon (or
I guess have copied the sensors bit off drivers that were.) Same is true of
the ones in IIO.
patch to remove SENSORS_ for IIO is following
Be careful with this one, whilst it would be nice, it may break lots of
peoples config
files. Probably best to leave be for existing drivers.
+ .type = IIO_INTENSITY,
+ .info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT |
+ IIO_CHAN_INFO_SCALE_SEPARATE_BIT,
IIO_INTENSITY was originally introduced because we had a set of light sensors
where their was no simple relationship between them and anything in SI units.
If we can convert to illuminance as here via a simple scaling and offset
then I'd be tempted to make this IIO_LIGHT (and hence illuminance_raw)
with illuminance_scale and illuminance_offset
I have switched it to IIO_LIGHT, I was taking the conservative route with
IIO_INTENSITY; I have to trust the data sheet...
Don't we all from time to time. Sometimes they are even right!
regards, p.
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
