Re: [PATCH 2/3] iio: pressure: Honeywell mpr pressure sensor

[Lars-Peter Clausen]

Hi,

Looks pretty good. Jonathan already covered most of it, a few additional 
comments.

On 4/1/23 02:10, Andreas Klinger wrote:
> [...]
> +struct mpr_data {
> +	struct device		*dev;
> +	void			*client;

Any reason not to use `struct i2c_client` for the type?

> +	struct mutex		lock;
> +	s32			pmin;
> +	s32			pmax;
> +	struct gpio_desc	*gpiod_reset;
> +	int			irq;
> +	struct completion	completion;
> +	s64			channel[2] __aligned(8);
> +};
> +
> [...]
> +static int mpr_read_pressure(struct mpr_data *data, s64 *press)
> +{
> +	int ret, i;
> +	u8 wdata[] = {0xAA, 0x00, 0x00};
> +	s32 status;
> +	int nloops = 10;
> +	char buf[5];
The tx buffer is `u8`, the rx buffer is `char`. This should be consistent.
> +	s64 press_cnt;
> +	s64 outputmin = 1677722;
> +	s64 outputmax = 15099494;
> +
> +	reinit_completion(&data->completion);
> +
> +	ret = i2c_master_send(data->client, wdata, sizeof(wdata));

The i2c family of transfer functions returns the number of bytes 
transferred, this can be less than what you expect if you get an early 
NACK. Its always good to check that all the data was transferred. E.g.

if (ret >= 0 && ret != sizeof(wdata))

   ret = -EIO;

Same for the receive later on.

[...]