Re: [PATCH v3 2/4] iio: magnetometer: ak8975: Fix reading for ak099xx sensors

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

 



On 2024-08-17 14:26, Jonathan Cameron wrote:
On Fri, 09 Aug 2024 22:25:40 +0200
Barnabás Czémán <barnabas.czeman@xxxxxxxxxxxxxx> wrote:

Move ST2 reading with overflow handling after measurement data
reading.
ST2 register read have to be read after read measurment data,
because it means end of the reading and realease the lock on the data.
Remove ST2 read skip on interrupt based waiting because ST2 required to
be read out at and of the axis read.

Signed-off-by: Barnabás Czémán <barnabas.czeman@xxxxxxxxxxxxxx>
This still needs a fixes tag to tell us when the bug was first
introduced into the driver so that we know how far to back port it.
I have got some test results from a device with ak09916 and that was working
without the fix so it seems only ak09918 is strict about ST2.
In theory all ak099xx should work like this so:
Fixes: 57e73a423b1e (iio: ak8975: add ak09911 and ak09912 support)

One other comment inline.

Thanks,

Jonathan

---
drivers/iio/magnetometer/ak8975.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 6357666edd34..f8355170f529 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -692,22 +692,7 @@ static int ak8975_start_read_axis(struct ak8975_data *data,
 	if (ret < 0)
 		return ret;

- /* This will be executed only for non-interrupt based waiting case */
-	if (ret & data->def->ctrl_masks[ST1_DRDY]) {
-		ret = i2c_smbus_read_byte_data(client,
-					       data->def->ctrl_regs[ST2]);
-		if (ret < 0) {
-			dev_err(&client->dev, "Error in reading ST2\n");
-			return ret;
-		}
-		if (ret & (data->def->ctrl_masks[ST2_DERR] |
-			   data->def->ctrl_masks[ST2_HOFL])) {
-			dev_err(&client->dev, "ST2 status error 0x%x\n", ret);
-			return -EINVAL;
-		}
-	}
-
-	return 0;
+	return !data->def->ctrl_regs[ST1_DRDY];
 }

 /* Retrieve raw flux value for one of the x, y, or z axis.  */
@@ -734,6 +719,20 @@ static int ak8975_read_axis(struct iio_dev *indio_dev, int index, int *val)
 	if (ret < 0)
 		goto exit;

+	/* Read out ST2 for release lock */
+	ret = i2c_smbus_read_byte_data(client, data->def->ctrl_regs[ST2]);
+	if (ret < 0) {
+		dev_err(&client->dev, "Error in reading ST2\n");
+		goto exit;
+	}
+
+	if (ret & (data->def->ctrl_masks[ST2_DERR] |
+		   data->def->ctrl_masks[ST2_HOFL])) {
+		dev_err(&client->dev, "ST2 status error 0x%x\n", ret);
+		ret = -EINVAL;
+		goto exit;
+	}
+
 	mutex_unlock(&data->lock);

 	pm_runtime_mark_last_busy(&data->client->dev);
@@ -746,7 +745,7 @@ static int ak8975_read_axis(struct iio_dev *indio_dev, int index, int *val)

 exit:
 	mutex_unlock(&data->lock);
-	dev_err(&client->dev, "Error in reading axis\n");
+	dev_err(&client->dev, "Error in reading axis %d\n", ret);

In most of the paths above there is already a detailed print. I'd not bother
adding the return value to this one.  It just adds noise to the patch.

 	return ret;
 }






[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux