From db8ecd5ec8da8a46e224728831759ec5adad98b9 Mon Sep 17 00:00:00 2001 From: Nick Ward Date: Fri, 18 Nov 2022 11:34:47 +1100 Subject: [PATCH] drivers: sensor: vcnl4040: fix sensor register init The driver has a read register, modify value, write value flow due to needing to modify nibbles of registers at different times. The issue this driver previously had was new incoming configurations from the driver would be corrupted by previous configurations written into the vcnl4040 (when there's been no power off). This would be in the case when a device continuously power on and a developer is tweaking the vcnl4040's configuration or when deploying firmware updates with a new vcnl4040 configuration. Signed-off-by: Nick Ward --- drivers/sensor/vcnl4040/vcnl4040.c | 96 ++++++++++-------------------- drivers/sensor/vcnl4040/vcnl4040.h | 2 + 2 files changed, 32 insertions(+), 66 deletions(-) diff --git a/drivers/sensor/vcnl4040/vcnl4040.c b/drivers/sensor/vcnl4040/vcnl4040.c index 9180f6756de..efac2444036 100644 --- a/drivers/sensor/vcnl4040/vcnl4040.c +++ b/drivers/sensor/vcnl4040/vcnl4040.c @@ -43,7 +43,7 @@ int vcnl4040_write(const struct device *dev, uint8_t reg, uint16_t value) ret = i2c_write_dt(&config->i2c, buf, sizeof(buf)); if (ret < 0) { - LOG_ERR("write: %u", ret); + LOG_ERR("write[%02X]: %u", reg, ret); return ret; } @@ -119,67 +119,20 @@ static int vcnl4040_channel_get(const struct device *dev, return ret; } -static int vcnl4040_proxy_setup(const struct device *dev) +static int vcnl4040_reg_setup(const struct device *dev) { const struct vcnl4040_config *config = dev->config; - uint16_t conf = 0; - - if (vcnl4040_read(dev, VCNL4040_REG_PS_MS, &conf)) { - LOG_ERR("Could not read proximity config"); - return -EIO; - } - - /* Set LED current */ - conf |= config->led_i << VCNL4040_LED_I_POS; - - if (vcnl4040_write(dev, VCNL4040_REG_PS_MS, conf)) { - LOG_ERR("Could not write proximity config"); - return -EIO; - } - - if (vcnl4040_read(dev, VCNL4040_REG_PS_CONF, &conf)) { - LOG_ERR("Could not read proximity config"); - return -EIO; - } - - /* Set PS_HD */ - conf |= VCNL4040_PS_HD_MASK; - /* Set duty cycle */ - conf |= config->led_dc << VCNL4040_PS_DUTY_POS; - /* Set integration time */ - conf |= config->proxy_it << VCNL4040_PS_IT_POS; - /* Clear proximity shutdown */ - conf &= ~VCNL4040_PS_SD_MASK; - - if (vcnl4040_write(dev, VCNL4040_REG_PS_CONF, conf)) { - LOG_ERR("Could not write proximity config"); - return -EIO; - } - - return 0; -} + uint16_t value[VCNL4040_RW_REG_COUNT] = { 0 }; + uint8_t reg; + int ret = 0; #ifdef CONFIG_VCNL4040_ENABLE_ALS -static int vcnl4040_ambient_setup(const struct device *dev) -{ - const struct vcnl4040_config *config = dev->config; struct vcnl4040_data *data = dev->data; - uint16_t conf = 0; - - if (vcnl4040_read(dev, VCNL4040_REG_ALS_CONF, &conf)) { - LOG_ERR("Could not read proximity config"); - return -EIO; - } /* Set ALS integration time */ - conf |= config->als_it << VCNL4040_ALS_IT_POS; + value[VCNL4040_REG_ALS_CONF] = config->als_it << VCNL4040_ALS_IT_POS; /* Clear ALS shutdown */ - conf &= ~VCNL4040_ALS_SD_MASK; - - if (vcnl4040_write(dev, VCNL4040_REG_ALS_CONF, conf)) { - LOG_ERR("Could not write proximity config"); - return -EIO; - } + value[VCNL4040_REG_ALS_CONF] &= ~VCNL4040_ALS_SD_MASK; /* * scale the lux depending on the value of the integration time @@ -205,11 +158,29 @@ static int vcnl4040_ambient_setup(const struct device *dev) config->als_it); break; } - - return 0; -} +#else + value[VCNL4040_REG_ALS_CONF] = VCNL4040_ALS_SD_MASK; /* default */ #endif + /* Set PS_HD */ + value[VCNL4040_REG_PS_CONF] = VCNL4040_PS_HD_MASK; + /* Set duty cycle */ + value[VCNL4040_REG_PS_CONF] |= config->led_dc << VCNL4040_PS_DUTY_POS; + /* Set integration time */ + value[VCNL4040_REG_PS_CONF] |= config->proxy_it << VCNL4040_PS_IT_POS; + /* Clear proximity shutdown */ + value[VCNL4040_REG_PS_CONF] &= ~VCNL4040_PS_SD_MASK; + + /* Set LED current */ + value[VCNL4040_REG_PS_MS] = config->led_i << VCNL4040_LED_I_POS; + + for (reg = 0; reg < ARRAY_SIZE(value); reg++) { + ret |= vcnl4040_write(dev, reg, value[reg]); + } + + return ret; +} + #ifdef CONFIG_PM_DEVICE static int vcnl4040_pm_action(const struct device *dev, enum pm_device_action action) @@ -295,18 +266,11 @@ static int vcnl4040_init(const struct device *dev) return -EIO; } - if (vcnl4040_proxy_setup(dev)) { - LOG_ERR("Failed to setup proximity functionality"); + if (vcnl4040_reg_setup(dev)) { + LOG_ERR("register setup"); return -EIO; } -#ifdef CONFIG_VCNL4040_ENABLE_ALS - if (vcnl4040_ambient_setup(dev)) { - LOG_ERR("Failed to setup ambient light functionality"); - return -EIO; - } -#endif - k_sem_init(&data->sem, 0, K_SEM_MAX_LIMIT); #if CONFIG_VCNL4040_TRIGGER diff --git a/drivers/sensor/vcnl4040/vcnl4040.h b/drivers/sensor/vcnl4040/vcnl4040.h index 9df6c2aaba2..d5192310f09 100644 --- a/drivers/sensor/vcnl4040/vcnl4040.h +++ b/drivers/sensor/vcnl4040/vcnl4040.h @@ -27,6 +27,8 @@ #define VCNL4040_REG_INT_FLAG 0x0B #define VCNL4040_REG_DEVICE_ID 0x0C +#define VCNL4040_RW_REG_COUNT 0x08 /* [0x00, 0x07] */ + #define VCNL4040_DEFAULT_ID 0x0186 #define VCNL4040_LED_I_POS 8