Re: [PATCH v3] iio: add vcnl4000 combined ALS and proximity sensor

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

> > +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_?

patch to remove SENSORS_ for IIO is following

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

regards, p.

-- 

Peter Meerwald
+43-664-2444418 (mobile)
--
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


[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux