[PATCH v3 0/4] Add support for AK09918

[PATCH v3 0/4] Add support for AK09918

[Barnabás Czémán]

Add support for AK09918 which is register and scaling compatible with
AK09912.

It was tested in Xiaomi Redmi 5 Plus (vince).

magnetometer@c {
  compatible = "asahi-kasei,ak09918", "asahi-kasei,ak09912";
  reg = <0x0c>;
  vdd-supply = <&pm8953_l6>;
  mount-matrix = "1", "0", "0",
                 "0", "1", "0",
                 "0", "0", "1";
};

Add a fix for data reading according to datasheet [1] (9.4.3.2.) 
ST2 register have to be read out after read measurment data as third step
because ST2 will realasing the lock on the measurment data. Without it
the next reading will fail.

[1] https://www.akm.com/content/dam/documents/products/electronic-compass/ak09918c/ak09918c-en-datasheet.pdf

Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
---
Changes in v3:
- Relax failure on unknown device id
for support more register compatible variants.
- Change to fallback compatible.
- Make ST2 to be always read after measuremnt read.
- Reword fix commit with more explanation.
- Link to v2: https://lore.kernel.org/r/20240806-ak09918-v2-0-c300da66c198@mainlining.org

Changes in v2:
- Remove unnecessary ak09918 compatbile.
- Link to v1: https://lore.kernel.org/r/20240805-ak09918-v1-0-70837eebd7d8@mainlining.org

---
Barnabás Czémán (2):
      iio: magnetometer: ak8975: Relax failure on unknown id
      iio: magnetometer: ak8975: Fix reading for ak099xx sensors

Danila Tikhonov (2):
      dt-bindings: iio: imu: magnetometer: Add ak09118
      iio: magnetometer: ak8975: Add AK09118 support

 .../iio/magnetometer/asahi-kasei,ak8975.yaml       |  3 +
 drivers/iio/magnetometer/Kconfig                   |  2 +-
 drivers/iio/magnetometer/ak8975.c                  | 73 ++++++++++++++++------
 3 files changed, 57 insertions(+), 21 deletions(-)
---
base-commit: 61c01d2e181adfba02fe09764f9fca1de2be0dbe
change-id: 20240805-ak09918-4a6cfef91c32

Best regards,
-- 
Barnabás Czémán <barnabas.czeman@mainlining.org>

[PATCH v3 1/4] iio: magnetometer: ak8975: Relax failure on unknown id

[Barnabás Czémán]

Relax failure when driver gets an unknown device id for
allow probe for register compatible devices.

Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
---
 drivers/iio/magnetometer/ak8975.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index dd466c5fa621..6357666edd34 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -484,10 +484,10 @@ static int ak8975_who_i_am(struct i2c_client *client,
 		if (wia_val[1] == AK09916_DEVICE_ID)
 			return 0;
 		break;
-	default:
-		dev_err(&client->dev, "Type %d unknown\n", type);
 	}
-	return -ENODEV;
+
+	dev_info(&client->dev, "Device ID %x is unknown.\n", wia_val[1]);
+	return 0;
 }
 
 /*

-- 
2.46.0

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

[Barnabás Czémán]

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@mainlining.org>
---
 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);
 	return ret;
 }
 

-- 
2.46.0

[PATCH v3 3/4] dt-bindings: iio: imu: magnetometer: Add ak09118

[Barnabás Czémán]

From: Danila Tikhonov <danila@jiaxyga.com>

Document asahi-kasei,ak09918 compatible.

Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
---
 .../devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml       | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml b/Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml
index 9790f75fc669..ff93a935363f 100644
--- a/Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml
+++ b/Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml
@@ -18,6 +18,9 @@ properties:
           - asahi-kasei,ak09911
           - asahi-kasei,ak09912
           - asahi-kasei,ak09916
+      - items:
+          - const: asahi-kasei,ak09918
+          - const: asahi-kasei,ak09912
       - enum:
           - ak8975
           - ak8963

-- 
2.46.0

[PATCH v3 4/4] iio: magnetometer: ak8975: Add AK09118 support

[Barnabás Czémán]

From: Danila Tikhonov <danila@jiaxyga.com>

Add additional AK09118 to the magnetometer driver which has the same
register mapping and scaling as the AK09112 device.

Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
---
 drivers/iio/magnetometer/Kconfig  |  2 +-
 drivers/iio/magnetometer/ak8975.c | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/magnetometer/Kconfig b/drivers/iio/magnetometer/Kconfig
index cd2917d71904..8eb718f5e50f 100644
--- a/drivers/iio/magnetometer/Kconfig
+++ b/drivers/iio/magnetometer/Kconfig
@@ -39,7 +39,7 @@ config AK8975
 	select IIO_TRIGGERED_BUFFER
 	help
 	  Say yes here to build support for Asahi Kasei AK8975, AK8963,
-	  AK09911, AK09912 or AK09916 3-Axis Magnetometer.
+	  AK09911, AK09912, AK09916 or AK09918 3-Axis Magnetometer.
 
 	  To compile this driver as a module, choose M here: the module
 	  will be called ak8975.
diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index f8355170f529..f9af5bffc9e3 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -78,6 +78,7 @@
  */
 #define AK09912_REG_WIA1		0x00
 #define AK09912_REG_WIA2		0x01
+#define AK09918_DEVICE_ID		0x0C
 #define AK09916_DEVICE_ID		0x09
 #define AK09912_DEVICE_ID		0x04
 #define AK09911_DEVICE_ID		0x05
@@ -209,6 +210,7 @@ enum asahi_compass_chipset {
 	AK09911,
 	AK09912,
 	AK09916,
+	AK09918,
 };
 
 enum ak_ctrl_reg_addr {
@@ -371,6 +373,31 @@ static const struct ak_def ak_def_array[] = {
 			AK09912_REG_HXL,
 			AK09912_REG_HYL,
 			AK09912_REG_HZL},
+	},
+	[AK09918] = {
+		.type = AK09918,
+		.raw_to_gauss = ak09912_raw_to_gauss,
+		.range = 32752,
+		.ctrl_regs = {
+			AK09912_REG_ST1,
+			AK09912_REG_ST2,
+			AK09912_REG_CNTL2,
+			AK09912_REG_ASAX,
+			AK09912_MAX_REGS},
+		.ctrl_masks = {
+			AK09912_REG_ST1_DRDY_MASK,
+			AK09912_REG_ST2_HOFL_MASK,
+			0,
+			AK09912_REG_CNTL2_MODE_MASK},
+		.ctrl_modes = {
+			AK09912_REG_CNTL_MODE_POWER_DOWN,
+			AK09912_REG_CNTL_MODE_ONCE,
+			AK09912_REG_CNTL_MODE_SELF_TEST,
+			AK09912_REG_CNTL_MODE_FUSE_ROM},
+		.data_regs = {
+			AK09912_REG_HXL,
+			AK09912_REG_HYL,
+			AK09912_REG_HZL},
 	}
 };
 
@@ -452,6 +479,7 @@ static int ak8975_who_i_am(struct i2c_client *client,
 	/*
 	 * Signature for each device:
 	 * Device   |  WIA1      |  WIA2
+	 * AK09918  |  DEVICE_ID_|  AK09918_DEVICE_ID
 	 * AK09916  |  DEVICE_ID_|  AK09916_DEVICE_ID
 	 * AK09912  |  DEVICE_ID |  AK09912_DEVICE_ID
 	 * AK09911  |  DEVICE_ID |  AK09911_DEVICE_ID
@@ -484,6 +512,10 @@ static int ak8975_who_i_am(struct i2c_client *client,
 		if (wia_val[1] == AK09916_DEVICE_ID)
 			return 0;
 		break;
+	case AK09918:
+		if (wia_val[1] == AK09918_DEVICE_ID)
+			return 0;
+		break;
 	}
 
 	dev_info(&client->dev, "Device ID %x is unknown.\n", wia_val[1]);
@@ -1066,6 +1098,7 @@ static const struct i2c_device_id ak8975_id[] = {
 	{"ak09911", (kernel_ulong_t)&ak_def_array[AK09911] },
 	{"ak09912", (kernel_ulong_t)&ak_def_array[AK09912] },
 	{"ak09916", (kernel_ulong_t)&ak_def_array[AK09916] },
+	{"ak09918", (kernel_ulong_t)&ak_def_array[AK09918] },
 	{}
 };
 MODULE_DEVICE_TABLE(i2c, ak8975_id);
@@ -1081,6 +1114,7 @@ static const struct of_device_id ak8975_of_match[] = {
 	{ .compatible = "ak09912", .data = &ak_def_array[AK09912] },
 	{ .compatible = "asahi-kasei,ak09916", .data = &ak_def_array[AK09916] },
 	{ .compatible = "ak09916", .data = &ak_def_array[AK09916] },
+	{ .compatible = "asahi-kasei,ak09918", .data = &ak_def_array[AK09918] },
 	{}
 };
 MODULE_DEVICE_TABLE(of, ak8975_of_match);

-- 
2.46.0

Re: [PATCH v3 3/4] dt-bindings: iio: imu: magnetometer: Add ak09118

[Krzysztof Kozlowski]

On 09/08/2024 22:25, Barnabás Czémán wrote:
> From: Danila Tikhonov <danila@jiaxyga.com>
> 
> Document asahi-kasei,ak09918 compatible.

Not much improved here.

<form letter>
This is a friendly reminder during the review process.

It seems my or other reviewer's previous comments were not fully
addressed. Maybe the feedback got lost between the quotes, maybe you
just forgot to apply it. Please go back to the previous discussion and
either implement all requested changes or keep discussing them.

Thank you.
</form letter>

> 
> Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
> Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
> ---
>  .../devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml       | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml b/Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml
> index 9790f75fc669..ff93a935363f 100644
> --- a/Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml
> +++ b/Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml
> @@ -18,6 +18,9 @@ properties:
>            - asahi-kasei,ak09911
>            - asahi-kasei,ak09912
>            - asahi-kasei,ak09916
> +      - items:
> +          - const: asahi-kasei,ak09918
> +          - const: asahi-kasei,ak09912

Why? Your driver suggests it might not be compatible... Can device bind
using ak09912 and operate up to ak09912 extend?

Best regards,
Krzysztof

Re: [PATCH v3 3/4] dt-bindings: iio: imu: magnetometer: Add ak09118

[barnabas.czeman]

On 2024-08-10 14:15, Krzysztof Kozlowski wrote:
> On 09/08/2024 22:25, Barnabás Czémán wrote:
>> From: Danila Tikhonov <danila@jiaxyga.com>
>> 
>> Document asahi-kasei,ak09918 compatible.
> 
> Not much improved here.
I have removed Reviewed-by because fallback compatible is a different 
approach
and I would not mind second look.
> 
> <form letter>
> This is a friendly reminder during the review process.
> 
> It seems my or other reviewer's previous comments were not fully
> addressed. Maybe the feedback got lost between the quotes, maybe you
> just forgot to apply it. Please go back to the previous discussion and
> either implement all requested changes or keep discussing them.
> 
> Thank you.
> </form letter>
> 
>> 
>> Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
>> Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
>> ---
>>  .../devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml      
>>  | 3 +++
>>  1 file changed, 3 insertions(+)
>> 
>> diff --git 
>> a/Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml 
>> b/Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml
>> index 9790f75fc669..ff93a935363f 100644
>> --- 
>> a/Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml
>> +++ 
>> b/Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml
>> @@ -18,6 +18,9 @@ properties:
>>            - asahi-kasei,ak09911
>>            - asahi-kasei,ak09912
>>            - asahi-kasei,ak09916
>> +      - items:
>> +          - const: asahi-kasei,ak09918
>> +          - const: asahi-kasei,ak09912
> 
> Why? Your driver suggests it might not be compatible... Can device bind
> using ak09912 and operate up to ak09912 extend?
It is register compatible and it can bind on 09112, as I understand 
fallback compatible
was a request from Connor and Jonathan in the previous round.
> 
> Best regards,
> Krzysztof

Re: [PATCH v3 3/4] dt-bindings: iio: imu: magnetometer: Add ak09118

[Krzysztof Kozlowski]

On 11/08/2024 20:28, barnabas.czeman@mainlining.org wrote:
> On 2024-08-10 14:15, Krzysztof Kozlowski wrote:
>> On 09/08/2024 22:25, Barnabás Czémán wrote:
>>> From: Danila Tikhonov <danila@jiaxyga.com>
>>>
>>> Document asahi-kasei,ak09918 compatible.
>>
>> Not much improved here.
> I have removed Reviewed-by because fallback compatible is a different 
> approach
> and I would not mind second look.

You received specific comments. You ignored them, so I replied that you
ignored them. And your excuse is that you ask for review? This does not
work like this.  Read CAREFULLY form letter below.

>>
>> <form letter>
>> This is a friendly reminder during the review process.
>>
>> It seems my or other reviewer's previous comments were not fully
>> addressed. Maybe the feedback got lost between the quotes, maybe you
>> just forgot to apply it. Please go back to the previous discussion and
>> either implement all requested changes or keep discussing them.
>>
>> Thank you.
>> </form letter>
>>
>>>
>>> Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
>>> Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
>>> ---
>>>  .../devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml      
>>>  | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git 
>>> a/Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml 
>>> b/Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml
>>> index 9790f75fc669..ff93a935363f 100644
>>> --- 
>>> a/Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml
>>> +++ 
>>> b/Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml
>>> @@ -18,6 +18,9 @@ properties:
>>>            - asahi-kasei,ak09911
>>>            - asahi-kasei,ak09912
>>>            - asahi-kasei,ak09916
>>> +      - items:
>>> +          - const: asahi-kasei,ak09918
>>> +          - const: asahi-kasei,ak09912
>>
>> Why? Your driver suggests it might not be compatible... Can device bind
>> using ak09912 and operate up to ak09912 extend?
> It is register compatible and it can bind on 09112, as I understand 
> fallback compatible

ok

> was a request from Connor and Jonathan in the previous round.

Not entirely, you should read comments more carefully.

Best regards,
Krzysztof

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

[Jonathan Cameron]

On Fri, 09 Aug 2024 22:25:40 +0200
Barnabás Czémán <barnabas.czeman@mainlining.org> 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@mainlining.org>
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;
>  }
>  
> 

Re: [PATCH v3 3/4] dt-bindings: iio: imu: magnetometer: Add ak09118

[Jonathan Cameron]

On Mon, 12 Aug 2024 08:17:52 +0200
Krzysztof Kozlowski <krzk@kernel.org> wrote:

> On 11/08/2024 20:28, barnabas.czeman@mainlining.org wrote:
> > On 2024-08-10 14:15, Krzysztof Kozlowski wrote:  
> >> On 09/08/2024 22:25, Barnabás Czémán wrote:  
> >>> From: Danila Tikhonov <danila@jiaxyga.com>
> >>>
> >>> Document asahi-kasei,ak09918 compatible.  
> >>
> >> Not much improved here.  
> > I have removed Reviewed-by because fallback compatible is a different 
> > approach
> > and I would not mind second look.  
> 
> You received specific comments. You ignored them, so I replied that you
> ignored them. And your excuse is that you ask for review? This does not
> work like this.  Read CAREFULLY form letter below.
> 
> >>
> >> <form letter>
> >> This is a friendly reminder during the review process.
> >>
> >> It seems my or other reviewer's previous comments were not fully
> >> addressed. Maybe the feedback got lost between the quotes, maybe you
> >> just forgot to apply it. Please go back to the previous discussion and
> >> either implement all requested changes or keep discussing them.
> >>
> >> Thank you.
> >> </form letter>
> >>  
> >>>
> >>> Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
> >>> Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
> >>> ---
> >>>  .../devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml      
> >>>  | 3 +++
> >>>  1 file changed, 3 insertions(+)
> >>>
> >>> diff --git 
> >>> a/Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml 
> >>> b/Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml
> >>> index 9790f75fc669..ff93a935363f 100644
> >>> --- 
> >>> a/Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml
> >>> +++ 
> >>> b/Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml
> >>> @@ -18,6 +18,9 @@ properties:
> >>>            - asahi-kasei,ak09911
> >>>            - asahi-kasei,ak09912
> >>>            - asahi-kasei,ak09916
> >>> +      - items:
> >>> +          - const: asahi-kasei,ak09918
> >>> +          - const: asahi-kasei,ak09912  
> >>
> >> Why? Your driver suggests it might not be compatible... Can device bind
> >> using ak09912 and operate up to ak09912 extend?  
> > It is register compatible and it can bind on 09112, as I understand 
> > fallback compatible  
> 
> ok
> 
> > was a request from Connor and Jonathan in the previous round.  
> 
> Not entirely, you should read comments more carefully.
Given the device specific data is only different in terms of the ID
register value, a fallback seems fine, but you should add to this
patch description something to say that this device is register
compatible etc.

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v3 3/4] dt-bindings: iio: imu: magnetometer: Add ak09118

[Jonathan Cameron]

On Fri, 09 Aug 2024 22:25:41 +0200
Barnabás Czémán <barnabas.czeman@mainlining.org> wrote:

> From: Danila Tikhonov <danila@jiaxyga.com>
> 
> Document asahi-kasei,ak09918 compatible.
> 
> Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
> Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
Patch title should not contain imu as this isn't one.

> ---
>  .../devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml       | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml b/Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml
> index 9790f75fc669..ff93a935363f 100644
> --- a/Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml
> +++ b/Documentation/devicetree/bindings/iio/magnetometer/asahi-kasei,ak8975.yaml
> @@ -18,6 +18,9 @@ properties:
>            - asahi-kasei,ak09911
>            - asahi-kasei,ak09912
>            - asahi-kasei,ak09916
> +      - items:
> +          - const: asahi-kasei,ak09918
> +          - const: asahi-kasei,ak09912
>        - enum:
>            - ak8975
>            - ak8963
> 

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

[barnabas.czeman]

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@mainlining.org> 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@mainlining.org>
> 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;
>>  }
>> 
>>