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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

[...]



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux