1. Patchew
  2. linux
  3. [PATCH 0/4] iio: proximity: vl53l0x: power sequencing, optional VDDIO, raw I2C ID read
  4. View patch

[PATCH 4/4] iio: proximity: vl53l0x-i2c: Use raw I2C access and read full device ID

Petr Hodina via B4 Relay posted 4 patches 3 weeks ago
[PATCH 4/4] iio: proximity: vl53l0x-i2c: Use raw I2C access and read full device ID
Posted by Petr Hodina via B4 Relay 3 weeks ago 
From: Petr Hodina <petr.hodina@protonmail.com>

Replace SMBus byte reads with raw I2C transfers when reading the device
identification registers.

The VL53L0X exposes its model and revision as 16-bit registers, which are
more accurately accessed using standard I2C send/receive operations.
This also avoids depending on SMBus byte data support, which is not
guaranteed on all I2C adapters.

Read and log both model and revision IDs, and validate the model ID
during probe to ensure the expected device is present.

Signed-off-by: Petr Hodina <petr.hodina@protonmail.com>
---
 drivers/iio/proximity/vl53l0x-i2c.c | 45 +++++++++++++++++++++++++++++++------
 1 file changed, 38 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/proximity/vl53l0x-i2c.c b/drivers/iio/proximity/vl53l0x-i2c.c
index 6901ce7dd835..a2de4cc16a43 100644
--- a/drivers/iio/proximity/vl53l0x-i2c.c
+++ b/drivers/iio/proximity/vl53l0x-i2c.c
@@ -320,11 +320,35 @@ static const struct iio_trigger_ops vl53l0x_trigger_ops = {
 	.validate_device = iio_trigger_validate_own_device,
 };
 
+
+static int vl53l0x_read_word(struct i2c_client *client, u8 reg, u16 *val)
+{
+	int ret;
+	u8 buf[2];
+
+	ret = i2c_master_send(client, &reg, 1);
+	if (ret < 0)
+		return ret;
+	if (ret != 1)
+		return -EIO;
+
+	ret = i2c_master_recv(client, buf, 2);
+	if (ret < 0)
+		return ret;
+	if (ret != 2)
+		return -EIO;
+
+	*val = (buf[0] << 8) | buf[1];
+
+	return 0;
+}
+
 static int vl53l0x_probe(struct i2c_client *client)
 {
 	struct vl53l0x_data *data;
 	struct iio_dev *indio_dev;
 	int ret;
+	u16 model, rev;
 
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
 	if (!indio_dev)
@@ -339,13 +363,6 @@ static int vl53l0x_probe(struct i2c_client *client)
 				     I2C_FUNC_SMBUS_BYTE_DATA))
 		return -EOPNOTSUPP;
 
-	ret = i2c_smbus_read_byte_data(data->client, VL_REG_IDENTIFICATION_MODEL_ID);
-	if (ret < 0)
-		return -EINVAL;
-
-	if (ret != VL53L0X_MODEL_ID_VAL)
-		dev_info(&client->dev, "Unknown model id: 0x%x", ret);
-
 	data->vdd_supply = devm_regulator_get(&client->dev, "vdd");
 	if (IS_ERR(data->vdd_supply))
 		return dev_err_probe(&client->dev, PTR_ERR(data->vdd_supply),
@@ -372,6 +389,20 @@ static int vl53l0x_probe(struct i2c_client *client)
 	if (ret)
 		return ret;
 
+	ret = vl53l0x_read_word(client, 0xC0, &model);
+	if (ret)
+		return dev_err_probe(&client->dev, ret, "Failed to read model ID\n");
+
+	ret = vl53l0x_read_word(client, 0xC2, &rev);
+	if (ret)
+		return dev_err_probe(&client->dev, ret, "Failed to read revision ID\n");
+
+	dev_info(&client->dev, "VL53L0X model=0x%04x rev=0x%04x\n", model, rev);
+
+	if ((model >> 8) != VL53L0X_MODEL_ID_VAL)
+		return dev_err_probe(&client->dev, -ENODEV,
+			"Unexpected model ID: 0x%04x\n", model);
+
 	indio_dev->name = "vl53l0x";
 	indio_dev->info = &vl53l0x_info;
 	indio_dev->channels = vl53l0x_channels;

-- 
2.52.0
Re: [PATCH 4/4] iio: proximity: vl53l0x-i2c: Use raw I2C access and read full device ID
Posted by Jonathan Cameron 2 weeks, 3 days ago 
On Mon, 19 Jan 2026 18:19:58 +0100
Petr Hodina via B4 Relay <devnull+petr.hodina.protonmail.com@kernel.org> wrote:

> From: Petr Hodina <petr.hodina@protonmail.com>
> 
> Replace SMBus byte reads with raw I2C transfers when reading the device
> identification registers.
> 
> The VL53L0X exposes its model and revision as 16-bit registers, which are
> more accurately accessed using standard I2C send/receive operations.
> This also avoids depending on SMBus byte data support, which is not
> guaranteed on all I2C adapters.

Hmm. I thought it was emulated wherever possible.  See below for
request for a specific comment on what the subtle difference is.
I'm also wondering from this description if it actually works with
smbus but  isn't documented as doing so?
> 
> Read and log both model and revision IDs, and validate the model ID
> during probe to ensure the expected device is present.
> 
> Signed-off-by: Petr Hodina <petr.hodina@protonmail.com>
> ---
>  drivers/iio/proximity/vl53l0x-i2c.c | 45 +++++++++++++++++++++++++++++++------
>  1 file changed, 38 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/proximity/vl53l0x-i2c.c b/drivers/iio/proximity/vl53l0x-i2c.c
> index 6901ce7dd835..a2de4cc16a43 100644
> --- a/drivers/iio/proximity/vl53l0x-i2c.c
> +++ b/drivers/iio/proximity/vl53l0x-i2c.c
> @@ -320,11 +320,35 @@ static const struct iio_trigger_ops vl53l0x_trigger_ops = {
>  	.validate_device = iio_trigger_validate_own_device,
>  };
>  
> +
> +static int vl53l0x_read_word(struct i2c_client *client, u8 reg, u16 *val)
> +{
> +	int ret;
> +	u8 buf[2];
> +
> +	ret = i2c_master_send(client, &reg, 1);

Add a comment that a stop is needed. Otherwise this would
be i2c_smbus_read_word_swapped()

Sigh, seems ST liked to make everyone's life a little harder ;)


> +	if (ret < 0)
> +		return ret;
> +	if (ret != 1)
> +		return -EIO;
> +
> +	ret = i2c_master_recv(client, buf, 2);
> +	if (ret < 0)
> +		return ret;
> +	if (ret != 2)
> +		return -EIO;
> +
> +	*val = (buf[0] << 8) | buf[1];
> +
> +	return 0;
> +}
> +
>  static int vl53l0x_probe(struct i2c_client *client)
>  {
>  	struct vl53l0x_data *data;
>  	struct iio_dev *indio_dev;
>  	int ret;
> +	u16 model, rev;
>  
>  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>  	if (!indio_dev)
> @@ -339,13 +363,6 @@ static int vl53l0x_probe(struct i2c_client *client)
>  				     I2C_FUNC_SMBUS_BYTE_DATA))
>  		return -EOPNOTSUPP;
>  
> -	ret = i2c_smbus_read_byte_data(data->client, VL_REG_IDENTIFICATION_MODEL_ID);
> -	if (ret < 0)
> -		return -EINVAL;
> -
> -	if (ret != VL53L0X_MODEL_ID_VAL)
> -		dev_info(&client->dev, "Unknown model id: 0x%x", ret);
> -
>  	data->vdd_supply = devm_regulator_get(&client->dev, "vdd");
>  	if (IS_ERR(data->vdd_supply))
>  		return dev_err_probe(&client->dev, PTR_ERR(data->vdd_supply),
> @@ -372,6 +389,20 @@ static int vl53l0x_probe(struct i2c_client *client)
>  	if (ret)
>  		return ret;
>  
> +	ret = vl53l0x_read_word(client, 0xC0, &model);
defines for the register addresses.
I'm curious why they got bigger in a patch that doesn't mention it in the
patch description.

> +	if (ret)
> +		return dev_err_probe(&client->dev, ret, "Failed to read model ID\n");
Given these long lines, it would be good to have a precursor patch
struct device *dev = &client->dev;
and use that throughout probe()

> +
> +	ret = vl53l0x_read_word(client, 0xC2, &rev);
> +	if (ret)
> +		return dev_err_probe(&client->dev, ret, "Failed to read revision ID\n");
> +
> +	dev_info(&client->dev, "VL53L0X model=0x%04x rev=0x%04x\n", model, rev);
> +
> +	if ((model >> 8) != VL53L0X_MODEL_ID_VAL)
> +		return dev_err_probe(&client->dev, -ENODEV,
> +			"Unexpected model ID: 0x%04x\n", model);
See comment on other patch. This breaks DT fallback compatibles and
the benefit they bring for new compatible parts being able to be run with 
old kernels.  Just warn and carry on with whatever the firmware told you this
was.

> +
>  	indio_dev->name = "vl53l0x";
>  	indio_dev->info = &vl53l0x_info;
>  	indio_dev->channels = vl53l0x_channels;
>
Re: [PATCH 4/4] iio: proximity: vl53l0x-i2c: Use raw I2C access and read full device ID
Posted by Krzysztof Kozlowski 2 weeks, 6 days ago 
On 19/01/2026 18:19, Petr Hodina via B4 Relay wrote:
> diff --git a/drivers/iio/proximity/vl53l0x-i2c.c b/drivers/iio/proximity/vl53l0x-i2c.c
> index 6901ce7dd835..a2de4cc16a43 100644
> --- a/drivers/iio/proximity/vl53l0x-i2c.c
> +++ b/drivers/iio/proximity/vl53l0x-i2c.c
> @@ -320,11 +320,35 @@ static const struct iio_trigger_ops vl53l0x_trigger_ops = {
>  	.validate_device = iio_trigger_validate_own_device,
>  };
>  
> +

Do not introduce stray blank lines.

> +static int vl53l0x_read_word(struct i2c_client *client, u8 reg, u16 *val)
> +{
> +	int ret;
> +	u8 buf[2];
> +
> +	ret = i2c_master_send(client, &reg, 1);
> +	if (ret < 0)
> +		return ret;
> +	if (ret != 1)
> +		return -EIO;
> +
> +	ret = i2c_master_recv(client, buf, 2);
> +	if (ret < 0)
> +		return ret;
> +	if (ret != 2)
> +		return -EIO;
> +
> +	*val = (buf[0] << 8) | buf[1];
> +
> +	return 0;
> +}
> +
>  static int vl53l0x_probe(struct i2c_client *client)
>  {
>  	struct vl53l0x_data *data;
>  	struct iio_dev *indio_dev;
>  	int ret;
> +	u16 model, rev;
>  
>  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>  	if (!indio_dev)
> @@ -339,13 +363,6 @@ static int vl53l0x_probe(struct i2c_client *client)
>  				     I2C_FUNC_SMBUS_BYTE_DATA))
>  		return -EOPNOTSUPP;
>  
> -	ret = i2c_smbus_read_byte_data(data->client, VL_REG_IDENTIFICATION_MODEL_ID);
> -	if (ret < 0)
> -		return -EINVAL;
> -
> -	if (ret != VL53L0X_MODEL_ID_VAL)
> -		dev_info(&client->dev, "Unknown model id: 0x%x", ret);
> -
>  	data->vdd_supply = devm_regulator_get(&client->dev, "vdd");
>  	if (IS_ERR(data->vdd_supply))
>  		return dev_err_probe(&client->dev, PTR_ERR(data->vdd_supply),
> @@ -372,6 +389,20 @@ static int vl53l0x_probe(struct i2c_client *client)
>  	if (ret)
>  		return ret;
>  
> +	ret = vl53l0x_read_word(client, 0xC0, &model);
> +	if (ret)
> +		return dev_err_probe(&client->dev, ret, "Failed to read model ID\n");
> +
> +	ret = vl53l0x_read_word(client, 0xC2, &rev);
> +	if (ret)
> +		return dev_err_probe(&client->dev, ret, "Failed to read revision ID\n");
> +
> +	dev_info(&client->dev, "VL53L0X model=0x%04x rev=0x%04x\n", model, rev);

dev_dbg

This does not look like useful printk message. Drivers should be silent
on success:
https://elixir.bootlin.com/linux/v6.15-rc7/source/Documentation/process/coding-style.rst#L913
https://elixir.bootlin.com/linux/v6.15-rc7/source/Documentation/process/debugging/driver_development_debugging_guide.rst#L79

Best regards,
Krzysztof
Re: [PATCH 4/4] iio: proximity: vl53l0x-i2c: Use raw I2C access and read full device ID
Posted by Andy Shevchenko 3 weeks ago 
On Mon, Jan 19, 2026 at 06:19:58PM +0100, Petr Hodina via B4 Relay wrote:

> Replace SMBus byte reads with raw I2C transfers when reading the device
> identification registers.
> 
> The VL53L0X exposes its model and revision as 16-bit registers, which are
> more accurately accessed using standard I2C send/receive operations.
> This also avoids depending on SMBus byte data support, which is not
> guaranteed on all I2C adapters.
> 
> Read and log both model and revision IDs, and validate the model ID
> during probe to ensure the expected device is present.

...

>  };
>  
> +

Stray blank line addition.

> +static int vl53l0x_read_word(struct i2c_client *client, u8 reg, u16 *val)
> +{
> +	int ret;
> +	u8 buf[2];

Here and everywhere else, please, keep the reversed xmas tree ordering.

> +	ret = i2c_master_send(client, &reg, 1);
> +	if (ret < 0)
> +		return ret;
> +	if (ret != 1)
> +		return -EIO;
> +
> +	ret = i2c_master_recv(client, buf, 2);

sizeof(buf)

> +	if (ret < 0)
> +		return ret;
> +	if (ret != 2)

Ditto.

> +		return -EIO;
> +
> +	*val = (buf[0] << 8) | buf[1];

Actually define

	__be16 buf;

and use

	*val = be16_to_cpu(buf);

> +	return 0;
> +}

...

>  static int vl53l0x_probe(struct i2c_client *client)
>  {
>  	struct vl53l0x_data *data;
>  	struct iio_dev *indio_dev;
>  	int ret;
> +	u16 model, rev;
>  

Use reversed xmas tree ordering.

-- 
With Best Regards,
Andy Shevchenko

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.