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

Barnabás Czémán posted 4 patches 1 year, 6 months ago
There is a newer version of this series
[PATCH v3 3/4] dt-bindings: iio: imu: magnetometer: Add ak09118
Posted by Barnabás Czémán 1 year, 6 months ago
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

Re: [PATCH v3 3/4] dt-bindings: iio: imu: magnetometer: Add ak09118
Posted by Jonathan Cameron 1 year, 5 months ago
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 3/4] dt-bindings: iio: imu: magnetometer: Add ak09118
Posted by Krzysztof Kozlowski 1 year, 6 months ago
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
Posted by barnabas.czeman@mainlining.org 1 year, 6 months ago
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
Posted by Krzysztof Kozlowski 1 year, 6 months ago
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 3/4] dt-bindings: iio: imu: magnetometer: Add ak09118
Posted by Jonathan Cameron 1 year, 5 months ago
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
>