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.
[...]
