iio: humidity: ens210: Extend I2C functionality check

[PATCH v7] iio: humidity: ens210: Extend I2C functionality check

Posted by Salah Triki 4 weeks, 1 day ago

The ENS210 driver requires several specific SMBus protocols to operate:
- WRITE_BYTE for sending commands.
- BYTE_DATA and WORD_DATA for register access.
- I2C_BLOCK_DATA for reading humidity/temperature results.

Update the probe function to explicitly check for these required
functionalities using standard macros. This ensures the driver only
binds to adapters that support the necessary transaction types.

Signed-off-by: Salah Triki <salah.triki@gmail.com>
---
Changes since v6:
- Replaced the incorrect use of I2C_FUNC_SMBUS_EMUL with an explicit 
  list of required SMBus functionalities as requested by Jonathan Cameron.
- Used combined macros (BYTE_DATA, WORD_DATA) to simplify the check.

Changes since v5:
- Changed patch title from "Fix missing I2C functionality checks" to
  "Simplify I2C functionality check" to reflect the new approach.
- Dropped the Fixes tag as the change is now considered hardening rather
  than a bug fix.
- Replaced individual functionality checks with a single check for
  I2C_FUNC_SMBUS_EMUL as suggested by Jonathan Cameron.

Changes since v4:
- Fixed the alignment and indentation of the I2C functionality check
  per Andy's review.

Changes since v3:
- Fixed the alignment and indentation of the I2C functionality check
  per Andy's review.

Changes since v2:
- Fixed the alignment and indentation of the I2C functionality check
  per Maxime's review.

Changes since v1:
- Updated the I2C functionality test to check for both required native
  operations and SMBus emulation (`I2C_FUNC_SMBUS_EMUL`)
 drivers/iio/humidity/ens210.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iio/humidity/ens210.c b/drivers/iio/humidity/ens210.c
index 77418d97f30d..d209f30d61ed 100644
--- a/drivers/iio/humidity/ens210.c
+++ b/drivers/iio/humidity/ens210.c
@@ -204,6 +204,8 @@ static int ens210_probe(struct i2c_client *client)
 	if (!i2c_check_functionality(client->adapter,
 				     I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
 				     I2C_FUNC_SMBUS_WRITE_BYTE |
+				     I2C_FUNC_SMBUS_BYTE_DATA |
+				     I2C_FUNC_SMBUS_WORD_DATA |
 				     I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
 		return dev_err_probe(&client->dev, -EOPNOTSUPP,
 			"adapter does not support some i2c transactions\n");
-- 
2.43.0

Re: [PATCH v7] iio: humidity: ens210: Extend I2C functionality check

Posted by Jonathan Cameron 3 weeks, 5 days ago

On Thu, 14 May 2026 09:41:54 +0100
Salah Triki <salah.triki@gmail.com> wrote:

> The ENS210 driver requires several specific SMBus protocols to operate:
> - WRITE_BYTE for sending commands.
> - BYTE_DATA and WORD_DATA for register access.
> - I2C_BLOCK_DATA for reading humidity/temperature results.
> 
> Update the probe function to explicitly check for these required
> functionalities using standard macros. This ensures the driver only
> binds to adapters that support the necessary transaction types.
> 
> Signed-off-by: Salah Triki <salah.triki@gmail.com>
> ---
> Changes since v6:
> - Replaced the incorrect use of I2C_FUNC_SMBUS_EMUL with an explicit 
>   list of required SMBus functionalities as requested by Jonathan Cameron.
> - Used combined macros (BYTE_DATA, WORD_DATA) to simplify the check.
> 
> Changes since v5:
> - Changed patch title from "Fix missing I2C functionality checks" to
>   "Simplify I2C functionality check" to reflect the new approach.
> - Dropped the Fixes tag as the change is now considered hardening rather
>   than a bug fix.
> - Replaced individual functionality checks with a single check for
>   I2C_FUNC_SMBUS_EMUL as suggested by Jonathan Cameron.
> 
> Changes since v4:
> - Fixed the alignment and indentation of the I2C functionality check
>   per Andy's review.
> 
> Changes since v3:
> - Fixed the alignment and indentation of the I2C functionality check
>   per Andy's review.
> 
> Changes since v2:
> - Fixed the alignment and indentation of the I2C functionality check
>   per Maxime's review.
> 
> Changes since v1:
> - Updated the I2C functionality test to check for both required native
>   operations and SMBus emulation (`I2C_FUNC_SMBUS_EMUL`)
>  drivers/iio/humidity/ens210.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/iio/humidity/ens210.c b/drivers/iio/humidity/ens210.c
> index 77418d97f30d..d209f30d61ed 100644
> --- a/drivers/iio/humidity/ens210.c
> +++ b/drivers/iio/humidity/ens210.c
> @@ -204,6 +204,8 @@ static int ens210_probe(struct i2c_client *client)
>  	if (!i2c_check_functionality(client->adapter,
>  				     I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
Sashiko pointed this out... Take a look at definition of I2C_FUNC_SMBUS_BYTE_DATA
and consider if the line above makes sense.  Check for other instances of
this.
>  				     I2C_FUNC_SMBUS_WRITE_BYTE |
> +				     I2C_FUNC_SMBUS_BYTE_DATA |
> +				     I2C_FUNC_SMBUS_WORD_DATA |
Sashiko suggests this one is tighter than needed.  Might be right - I didn't check!
https://sashiko.dev/#/patchset/20260514084154.298154-1-salah.triki%40gmail.com
>  				     I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
>  		return dev_err_probe(&client->dev, -EOPNOTSUPP,
>  			"adapter does not support some i2c transactions\n");

Re: [PATCH v7] iio: humidity: ens210: Extend I2C functionality check

Posted by Stepan Ionichev 3 weeks, 5 days ago

On Fri, May 16, 2026, Jonathan Cameron wrote:
> Sashiko pointed this out... Take a look at definition of I2C_FUNC_SMBUS_BYTE_DATA
> and consider if the line above makes sense.  Check for other instances of
> this.

grep across the driver shows what actually gets called:

  i2c_smbus_read_byte_data       -> I2C_FUNC_SMBUS_READ_BYTE_DATA
  i2c_smbus_write_byte_data      -> I2C_FUNC_SMBUS_WRITE_BYTE_DATA
  i2c_smbus_read_word_data       -> I2C_FUNC_SMBUS_READ_WORD_DATA
  i2c_smbus_read_i2c_block_data  -> I2C_FUNC_SMBUS_READ_I2C_BLOCK

i2c_smbus_write_byte() (without _data) is not called anywhere in the
driver, so the WRITE_BYTE flag in the current mask is dead. WORD_DATA
asks for both read and write word data, but only the read side is used.

Minimal correct mask:

	I2C_FUNC_SMBUS_BYTE_DATA |        /* read + write byte data */
	I2C_FUNC_SMBUS_READ_WORD_DATA |
	I2C_FUNC_SMBUS_READ_I2C_BLOCK

Stepan

Re: [PATCH v7] iio: humidity: ens210: Extend I2C functionality check

Posted by Jonathan Cameron 3 weeks, 5 days ago

On Sun, 17 May 2026 12:38:22 +0500
Stepan Ionichev <sozdayvek@gmail.com> wrote:

> On Fri, May 16, 2026, Jonathan Cameron wrote:
> > Sashiko pointed this out... Take a look at definition of I2C_FUNC_SMBUS_BYTE_DATA
> > and consider if the line above makes sense.  Check for other instances of
> > this.  
> 
> grep across the driver shows what actually gets called:
> 
>   i2c_smbus_read_byte_data       -> I2C_FUNC_SMBUS_READ_BYTE_DATA
>   i2c_smbus_write_byte_data      -> I2C_FUNC_SMBUS_WRITE_BYTE_DATA
>   i2c_smbus_read_word_data       -> I2C_FUNC_SMBUS_READ_WORD_DATA
>   i2c_smbus_read_i2c_block_data  -> I2C_FUNC_SMBUS_READ_I2C_BLOCK
> 
> i2c_smbus_write_byte() (without _data) is not called anywhere in the
> driver, so the WRITE_BYTE flag in the current mask is dead. WORD_DATA
> asks for both read and write word data, but only the read side is used.
> 
> Minimal correct mask:
> 
> 	I2C_FUNC_SMBUS_BYTE_DATA |        /* read + write byte data */
> 	I2C_FUNC_SMBUS_READ_WORD_DATA |
> 	I2C_FUNC_SMBUS_READ_I2C_BLOCK
Yes.
> 
> Stepan
>