Re: [PATCH v3 2/2] iio: proximity: Add driver for Vishay VCNL36829

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 16 Oct 2026 09:22:18 +0800
Liu Yufei <lyf98405@gmail.com> wrote:

> From: Liu Yufei <lyf98405@gmail.com>
> 
> Add an IIO driver for the Vishay VCNL36829 proximity sensor with
> integrated VCSEL, connected over I2C.
> 
> At probe the driver enables the vdd and vddio supplies, waits for the
> sensor to power up, checks the device ID, configures the VCSEL current
> and photodiodes, and turns the proximity engine on. The sensor is put
> back into shutdown when the device is removed.
> 
> The following attributes are exposed:
>   - in_proximity0_raw
>   - in_proximity_integration_time
>   - in_proximity_integration_time_available
> 
> Datasheet: https://www.vishay.com/docs/80580/vcnl36829um.pdf
> Assisted-by: LLM
> Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> Signed-off-by: Liu Yufei <lyf98405@gmail.com>
> ---
> v3:
> - Use dev_info() instead of dev_warn() for an unknown device ID
>   (Joshua Crofts).
> - Wrap the write_raw() arguments on a logical boundary (Andy Shevchenko).
> v2:
> - Address review comments from Andy (see cover letter for details).
> 
Nice little driver. I'll tweak the first thing whilst applying.

Applied series to the testing branch of iio.git. That will be rebased on rc1
once available at which point I'll push it out for linux-next to pick up.

> diff --git a/drivers/iio/proximity/vcnl36829.c b/drivers/iio/proximity/vcnl36829.c
> new file mode 100644
> index 000000000..217a32aad
> --- /dev/null
> +++ b/drivers/iio/proximity/vcnl36829.c

> +
> +static const int vcnl36829_it_range[] = {0, 25, 0, 25, 0, 375};
Trivial but preferred style for IIO has { 0, ... 375 };
I'll tidy that up.


> +
> +static int vcnl36829_read_raw(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan,
> +			      int *val, int *val2, long mask)
> +{
> +	struct vcnl36829_data *data = iio_priv(indio_dev);
> +	struct device *dev = regmap_get_device(data->regmap);
> +	struct regmap *regmap = data->regmap;
> +	unsigned int reg;
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = regmap_read(regmap, chan->address, &reg);
> +		if (ret) {
> +			dev_err(dev, "Data read failed: %d\n", ret);
> +			return ret;
> +		}
> +		*val = reg;
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_INT_TIME:
> +		ret = regmap_read(regmap, VCNL36829_PS_CONF2, &reg);
> +		if (ret) {
> +			dev_err(dev, "Data read failed: %d\n", ret);
> +			return ret;
> +		}
> +		/*
> +		 * PS_IT field 0 would mean 12.5us, which does not fit the
> +		 * 25us step used here. The driver never programs field 0:
> +		 * probe sets 25us and write_raw rejects anything below 25us,
> +		 * so the field is always >= 1 and val2 = field * 25 holds.

I suppose we can relax that later without it being a regression if
it turns out someone wants to use 12.5 usecs.

> +		 */
> +		*val = 0;
> +		*val2 = FIELD_GET(VCNL36829_PS_IT_MSK, reg) * 25;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}


> +static int vcnl36829_probe(struct i2c_client *client)
> +{

> +
> +	ret = regmap_read(regmap, VCNL36829_DEV_ID, &reg);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to read device ID\n");
> +	if ((reg & 0xFF) != VCNL36829_DEV_ID_VAL)

Not that important, but for consistency I'd have given that a field name,
despite it not having one on the datasheet..

> +		dev_info(dev, "Unknown device ID 0x%x\n", reg & 0xFF);
>