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

[Joshua Crofts <joshua.crofts1@gmail.com>]

On Tue,  6 Oct 2026 18:09:43 +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
> Signed-off-by: Liu Yufei <lyf98405@gmail.com>
> ---

...

> +static int vcnl36829_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct vcnl36829_data *data;
> +	struct iio_dev *indio_dev;
> +	struct regmap *regmap;
> +	unsigned int reg;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	regmap = devm_regmap_init_i2c(client, &vcnl36829_regmap_config);
> +	if (IS_ERR(regmap))
> +		return dev_err_probe(dev, PTR_ERR(regmap), "Regmap setup failed\n");
> +
> +	data = iio_priv(indio_dev);
> +	data->regmap = regmap;
> +
> +	indio_dev->name = "vcnl36829";
> +	indio_dev->info = &vcnl36829_info;
> +	indio_dev->channels = vcnl36829_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(vcnl36829_channels);
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	ret = devm_regulator_get_enable(dev, "vdd");
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to enable vdd\n");
> +
> +	ret = devm_regulator_get_enable(dev, "vddio");
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to enable vddio\n");
> +
> +	/*
> +	 * The datasheet does not specify a power-up settling time.
> +	 * Wait 3 ms after enabling the supplies before accessing the
> +	 * device, as a conservative value.
> +	 */
> +	fsleep(3 * USEC_PER_MSEC);
> +
> +	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)
> +		dev_warn(dev, "Unknown device ID 0x%x\n", reg & 0xFF);

I'd rather be soft and do dev_info() instead (perhaps this could
be tweaked when applying).

Otherwise this looks good to me!

Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>

-- 
Kind regards,
Joshua Crofts