Re: [PATCH v1 2/2] iio: adc: add MAX40080 current-sense amplifier driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The driver enables I2C PEC without verifying adapter support and sets the I2C_CLIENT_PEC flag before configuring the device to expect it.
- [Medium] The driver modifies volatile hardware state during initialization but lacks PM suspend/resume hooks to restore this state, causing device failure after a suspend cycle.
--

commit 7c1a04214c6c99c028e92e65daff698c598ebaff
Author: Stefan Popa <stefan.popa@analog.com>

iio: adc: add MAX40080 current-sense amplifier driver

The MAX40080 is a bidirectional current-sense amplifier with an
integrated 12-bit ADC and an I2C/SMBus interface. It measures the
voltage across an external shunt resistor and the input bus voltage,
storing the results in an internal FIFO.

> diff --git a/drivers/iio/adc/max40080.c b/drivers/iio/adc/max40080.c
> new file mode 100644
> index 0000000000000..441e1ce3dcffd
> --- /dev/null
> +++ b/drivers/iio/adc/max40080.c
> @@ -0,0 +1,587 @@

[ ... ]

> +static int max40080_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct iio_dev *indio_dev;
> +	struct max40080_state *st;
> +	int ret;
> +
> +	if (!i2c_check_functionality(client->adapter,
> +				     I2C_FUNC_SMBUS_WORD_DATA |
> +				     I2C_FUNC_SMBUS_I2C_BLOCK |
> +				     I2C_FUNC_SMBUS_QUICK))
> +		return -EOPNOTSUPP;

[Severity: High]
Should this check also verify I2C_FUNC_SMBUS_PEC support?

If the adapter lacks PEC support, it will not send PEC bytes. However, since the
driver writes MAX40080_PEC_EN_MSK to the device during initialization, the device
will expect a PEC byte on all subsequent transactions and discard any writes that
lack it. 

Won't this render the device unresponsive on adapters without PEC?

> +
> +	client->flags |= I2C_CLIENT_PEC;

[Severity: High]
Is it safe to unconditionally set the I2C_CLIENT_PEC flag before the hardware is
configured to accept PEC bytes?

Since the device powers up with PEC disabled, it expects a standard 2-byte SMBus
word write. If the host appends a 3rd PEC byte during the initial configuration
in max40080_init(), won't the device NACK it, causing the initial configuration
to fail and probe to abort?

[ ... ]

> +static struct i2c_driver max40080_driver = {
> +	.driver = {
> +		.name = "max40080",
> +		.of_match_table = max40080_of_match,
> +	},
> +	.probe = max40080_probe,
> +	.id_table = max40080_i2c_ids,
> +};

[Severity: Medium]
Does this driver need PM suspend/resume hooks to restore hardware state?

max40080_init() programs the device into MAX40080_SINGLE_MODE, enables PEC, and
configures the FIFO. If the system enters a sleep state where power to the
sensor is cut, the device resets to its power-on defaults (STANDBY_MODE, PEC
disabled).

Upon resume, any subsequent read would trigger an I2C Quick Command (which the
device ignores in STANDBY_MODE) and poll indefinitely for the data valid bit,
resulting in a timeout. 

Additionally, the software would send PEC bytes which the reset device would
reject.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703102941.1141341-1-stefan.popa@analog.com?part=2