On 12/26/23 17:19, Jonathan Cameron wrote:
On Thu, 21 Dec 2023 17:33:09 +0100
Mårten Lindahl<marten.lindahl@xxxxxxxx> wrote:
The vcnl4040/vcnl4200 proximity sensor defaults to 12 bit data
resolution, but the chip also supports 16 bit data resolution, which is
called proximity high definition (PS_HD).
Make the vcnl4040/vcnl4200 proximity sensor use the high definition for
all data readings.
Signed-off-by: Mårten Lindahl<marten.lindahl@xxxxxxxx>
Hi Jonathan!
Hmm. Was about to apply this and had a nasty thought. Whilst proximity isn't
'scaled' as such because there is no absolute scale applied, I assume this change
divides the effective scale (so what someone may be applying in userspace) by 16?
So this might cause someone a visible userspace regression?
If so we may have to report it IIO_VAL_FRACTIONAL with the bottom set to 16
so we end up with a suitable fixed point value from sysfs.
Jonathan
Yes, your assumption is correct. And I found that this can easily pass
unnoticed, at least on our HW. To get full 16 bit data width the sensor
needs higher proximity integration time, current, and sample rate, or
else it will give a raw output very close to the 12 bit steps with a
maximum around 4150 compared to 12 bit max 4095. With our HW only few
would notice the difference.
Increasing integration time/current/sample rate is already supported
from sysfs, and to get the proper scaling I can use the
IIO_VAL_FRACTIONAL as you suggest, but then I also need to set these
higher values from the beginning (where PS_HD is set) or else sysfs
in_proximity_raw will give output "259.375000000" for 4150, instead of
"4095.937500000" for 65535. If not, userspace will have to change the
values manually before first read can be done.
Regarding the changed output format, is it ok to change it from "4095"
to "4095.937500000", without making users upset?
Kind regards
Mårten
---
drivers/iio/light/vcnl4000.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index fdf763a04b0b..4846f3b698b5 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -90,6 +90,7 @@
#define VCNL4040_PS_CONF1_PS_SHUTDOWN BIT(0)
#define VCNL4040_PS_CONF2_PS_IT GENMASK(3, 1) /* Proximity integration time */
#define VCNL4040_CONF1_PS_PERS GENMASK(5, 4) /* Proximity interrupt persistence setting */
+#define VCNL4040_PS_CONF2_PS_HD BIT(11) /* Proximity high definition */
#define VCNL4040_PS_CONF2_PS_INT GENMASK(9, 8) /* Proximity interrupt mode */
#define VCNL4040_PS_CONF3_MPS GENMASK(6, 5) /* Proximity multi pulse number */
#define VCNL4040_PS_MS_LED_I GENMASK(10, 8) /* Proximity current */
@@ -345,6 +346,7 @@ static int vcnl4200_set_power_state(struct vcnl4000_data *data, bool on)
static int vcnl4200_init(struct vcnl4000_data *data)
{
int ret, id;
+ u16 regval;
ret = i2c_smbus_read_word_data(data->client, VCNL4200_DEV_ID);
if (ret < 0)
@@ -389,6 +391,17 @@ static int vcnl4200_init(struct vcnl4000_data *data)
mutex_init(&data->vcnl4200_al.lock);
mutex_init(&data->vcnl4200_ps.lock);
+ /* Use 16 bits proximity sensor readings */
+ ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1);
+ if (ret >= 0) {
+ regval = (ret | VCNL4040_PS_CONF2_PS_HD);
+ ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1,
+ regval);
+ }
+
+ if (ret < 0)
+ dev_info(&data->client->dev, "Default to 12 bits sensor data");
+
ret = data->chip_spec->set_power_state(data, true);
if (ret < 0)
return ret;
---
base-commit: a39b6ac3781d46ba18193c9dbb2110f31e9bffe9
change-id: 20231221-vcnl4000-ps-hd-863f4f8fcea7
Best regards,
