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.
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;
> }
>
>
