Re: [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family

Re: [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family

[Krzysztof Kozlowski]

On 26/12/2023 04:47, cy_huang@richtek.com wrote:
> From: ChiYuan Huang <cy_huang@richtek.com>
> 
> Add compatible support for RTQ6053 and RTQ6059.
> 
> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> ---
>  .../devicetree/bindings/iio/adc/richtek,rtq6056.yaml         | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
> index 88e008629ea8..d1e1f36d1972 100644
> --- a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
> @@ -25,7 +25,10 @@ description: |
>  
>  properties:
>    compatible:
> -    const: richtek,rtq6056
> +    enum:
> +      - richtek,rtq6053
> +      - richtek,rtq6056

Aren't these devices compatible? Your driver change says they are, so
express compatibility with list here (and oneOf).


Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family

[ChiYuan Huang]

On Tue, Dec 26, 2023 at 10:18:47AM +0100, Krzysztof Kozlowski wrote:
> On 26/12/2023 04:47, cy_huang@richtek.com wrote:
> > From: ChiYuan Huang <cy_huang@richtek.com>
> > 
> > Add compatible support for RTQ6053 and RTQ6059.
> > 
> > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> > ---
> >  .../devicetree/bindings/iio/adc/richtek,rtq6056.yaml         | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
> > index 88e008629ea8..d1e1f36d1972 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
> > +++ b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
> > @@ -25,7 +25,10 @@ description: |
> >  
> >  properties:
> >    compatible:
> > -    const: richtek,rtq6056
> > +    enum:
> > +      - richtek,rtq6053
> > +      - richtek,rtq6056
> 
> Aren't these devices compatible? Your driver change says they are, so
> express compatibility with list here (and oneOf).
>
Thanks, I try to take other binding as the reference. One more question.
If rtq6053 is compatible with rtq6056, there's only chip package type difference.
Do I need to seperate it into a dedicated enum element?
Or just put it into one item and said this part number is compatible with rtq6056? 
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family

[Krzysztof Kozlowski]

On 26/12/2023 12:19, ChiYuan Huang wrote:
> On Tue, Dec 26, 2023 at 10:18:47AM +0100, Krzysztof Kozlowski wrote:
>> On 26/12/2023 04:47, cy_huang@richtek.com wrote:
>>> From: ChiYuan Huang <cy_huang@richtek.com>
>>>
>>> Add compatible support for RTQ6053 and RTQ6059.
>>>
>>> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
>>> ---
>>>  .../devicetree/bindings/iio/adc/richtek,rtq6056.yaml         | 5 ++++-
>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
>>> index 88e008629ea8..d1e1f36d1972 100644
>>> --- a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
>>> +++ b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
>>> @@ -25,7 +25,10 @@ description: |
>>>  
>>>  properties:
>>>    compatible:
>>> -    const: richtek,rtq6056
>>> +    enum:
>>> +      - richtek,rtq6053
>>> +      - richtek,rtq6056
>>
>> Aren't these devices compatible? Your driver change says they are, so
>> express compatibility with list here (and oneOf).
>>
> Thanks, I try to take other binding as the reference. One more question.
> If rtq6053 is compatible with rtq6056, there's only chip package type difference.
> Do I need to seperate it into a dedicated enum element?
> Or just put it into one item and said this part number is compatible with rtq6056? 

See example-schema. You need enum and items, both in oneOf:.

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family

[Jonathan Cameron]

On Tue, 26 Dec 2023 10:18:47 +0100
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 26/12/2023 04:47, cy_huang@richtek.com wrote:
> > From: ChiYuan Huang <cy_huang@richtek.com>
> > 
> > Add compatible support for RTQ6053 and RTQ6059.
> > 
> > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> > ---
> >  .../devicetree/bindings/iio/adc/richtek,rtq6056.yaml         | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
> > index 88e008629ea8..d1e1f36d1972 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
> > +++ b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
> > @@ -25,7 +25,10 @@ description: |
> >  
> >  properties:
> >    compatible:
> > -    const: richtek,rtq6056
> > +    enum:
> > +      - richtek,rtq6053
> > +      - richtek,rtq6056  
> 
> Aren't these devices compatible? Your driver change says they are, so
> express compatibility with list here (and oneOf).

I'm not seeing this thread on lore.kernel.org or in my local email. 
Perhaps something went wrong?

Jonathan


> 
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family

[ChiYuan Huang]

On Tue, Dec 26, 2023 at 01:12:50PM +0100, Krzysztof Kozlowski wrote:
> On 26/12/2023 12:19, ChiYuan Huang wrote:
> > On Tue, Dec 26, 2023 at 10:18:47AM +0100, Krzysztof Kozlowski wrote:
> >> On 26/12/2023 04:47, cy_huang@richtek.com wrote:
> >>> From: ChiYuan Huang <cy_huang@richtek.com>
> >>>
> >>> Add compatible support for RTQ6053 and RTQ6059.
> >>>
> >>> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> >>> ---
> >>>  .../devicetree/bindings/iio/adc/richtek,rtq6056.yaml         | 5 ++++-
> >>>  1 file changed, 4 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
> >>> index 88e008629ea8..d1e1f36d1972 100644
> >>> --- a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
> >>> +++ b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
> >>> @@ -25,7 +25,10 @@ description: |
> >>>  
> >>>  properties:
> >>>    compatible:
> >>> -    const: richtek,rtq6056
> >>> +    enum:
> >>> +      - richtek,rtq6053
> >>> +      - richtek,rtq6056
> >>
> >> Aren't these devices compatible? Your driver change says they are, so
> >> express compatibility with list here (and oneOf).
> >>
> > Thanks, I try to take other binding as the reference. One more question.
> > If rtq6053 is compatible with rtq6056, there's only chip package type difference.
> > Do I need to seperate it into a dedicated enum element?
> > Or just put it into one item and said this part number is compatible with rtq6056? 
> 
> See example-schema. You need enum and items, both in oneOf:.
>
After reading the 'example-schema', I Still cannot understand what the special case items
means.

But in my case, is the below change correct?
[Diff]
properties:
   compatible:
-    enum:
-      - richtek,rtq6053
-      - richtek,rtq6056
-      - richtek,rtq6059
+    oneOf:
+      - items:
+          - enum:
+              - richtek,rtq6053
+              - richtek,rtq6056
+              - richtek,rtq6059
 
> Best regards,
> Krzysztof
> 

Re: [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family

[ChiYuan Huang]

On Tue, Dec 26, 2023 at 04:24:50PM +0000, Jonathan Cameron wrote:
> On Tue, 26 Dec 2023 10:18:47 +0100
> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> 
> > On 26/12/2023 04:47, cy_huang@richtek.com wrote:
> > > From: ChiYuan Huang <cy_huang@richtek.com>
> > > 
> > > Add compatible support for RTQ6053 and RTQ6059.
> > > 
> > > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> > > ---
> > >  .../devicetree/bindings/iio/adc/richtek,rtq6056.yaml         | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
> > > index 88e008629ea8..d1e1f36d1972 100644
> > > --- a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
> > > +++ b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
> > > @@ -25,7 +25,10 @@ description: |
> > >  
> > >  properties:
> > >    compatible:
> > > -    const: richtek,rtq6056
> > > +    enum:
> > > +      - richtek,rtq6053
> > > +      - richtek,rtq6056  
> > 
> > Aren't these devices compatible? Your driver change says they are, so
> > express compatibility with list here (and oneOf).
> 
> I'm not seeing this thread on lore.kernel.org or in my local email. 
> Perhaps something went wrong?
> 
I also cannot find the original message in lore.kernel.org.
But I checked the to/cc list. Nothing is lost.

Still not sure what happened.

Best Regards,
ChiYuan
> Jonathan
> 
> 
> > 
> > 
> > Best regards,
> > Krzysztof
> > 
> 

Re: [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family

[ChiYuan Huang]

On Thu, Dec 28, 2023 at 11:26:04AM +0800, ChiYuan Huang wrote:
> On Tue, Dec 26, 2023 at 04:24:50PM +0000, Jonathan Cameron wrote:
> > On Tue, 26 Dec 2023 10:18:47 +0100
> > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> > 
> > > On 26/12/2023 04:47, cy_huang@richtek.com wrote:
> > > > From: ChiYuan Huang <cy_huang@richtek.com>
> > > > 
> > > > Add compatible support for RTQ6053 and RTQ6059.
> > > > 
> > > > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> > > > ---
> > > >  .../devicetree/bindings/iio/adc/richtek,rtq6056.yaml         | 5 ++++-
> > > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
> > > > index 88e008629ea8..d1e1f36d1972 100644
> > > > --- a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
> > > > +++ b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
> > > > @@ -25,7 +25,10 @@ description: |
> > > >  
> > > >  properties:
> > > >    compatible:
> > > > -    const: richtek,rtq6056
> > > > +    enum:
> > > > +      - richtek,rtq6053
> > > > +      - richtek,rtq6056  
> > > 
> > > Aren't these devices compatible? Your driver change says they are, so
> > > express compatibility with list here (and oneOf).
> > 
> > I'm not seeing this thread on lore.kernel.org or in my local email. 
> > Perhaps something went wrong?
> > 
> I also cannot find the original message in lore.kernel.org.
> But I checked the to/cc list. Nothing is lost.
> 
> Still not sure what happened.
Ok, it seems the only difference is that I forget to add --thread in 
git format-patch. But not sure for that.
When I send v2 patch for the binding fix, I'll check it again.

Sorry for the inconvenience.
> 
> Best Regards,
> ChiYuan
> > Jonathan
> > 
> > 
> > > 
> > > 
> > > Best regards,
> > > Krzysztof
> > > 
> > 

[PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family

[cy_huang]

From: ChiYuan Huang <cy_huang@richtek.com>

Add compatible support for RTQ6053 and RTQ6059.

Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
---
 .../devicetree/bindings/iio/adc/richtek,rtq6056.yaml         | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
index 88e008629ea8..d1e1f36d1972 100644
--- a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
@@ -25,7 +25,10 @@ description: |
 
 properties:
   compatible:
-    const: richtek,rtq6056
+    enum:
+      - richtek,rtq6053
+      - richtek,rtq6056
+      - richtek,rtq6059
 
   reg:
     maxItems: 1
-- 
2.34.1

Re: [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family

[Krzysztof Kozlowski]

On 28/12/2023 04:19, ChiYuan Huang wrote:
> On Tue, Dec 26, 2023 at 01:12:50PM +0100, Krzysztof Kozlowski wrote:
>> On 26/12/2023 12:19, ChiYuan Huang wrote:
>>> On Tue, Dec 26, 2023 at 10:18:47AM +0100, Krzysztof Kozlowski wrote:
>>>> On 26/12/2023 04:47, cy_huang@richtek.com wrote:
>>>>> From: ChiYuan Huang <cy_huang@richtek.com>
>>>>>
>>>>> Add compatible support for RTQ6053 and RTQ6059.
>>>>>
>>>>> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
>>>>> ---
>>>>>  .../devicetree/bindings/iio/adc/richtek,rtq6056.yaml         | 5 ++++-
>>>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
>>>>> index 88e008629ea8..d1e1f36d1972 100644
>>>>> --- a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
>>>>> +++ b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
>>>>> @@ -25,7 +25,10 @@ description: |
>>>>>  
>>>>>  properties:
>>>>>    compatible:
>>>>> -    const: richtek,rtq6056
>>>>> +    enum:
>>>>> +      - richtek,rtq6053
>>>>> +      - richtek,rtq6056
>>>>
>>>> Aren't these devices compatible? Your driver change says they are, so
>>>> express compatibility with list here (and oneOf).
>>>>
>>> Thanks, I try to take other binding as the reference. One more question.
>>> If rtq6053 is compatible with rtq6056, there's only chip package type difference.
>>> Do I need to seperate it into a dedicated enum element?
>>> Or just put it into one item and said this part number is compatible with rtq6056? 
>>
>> See example-schema. You need enum and items, both in oneOf:.
>>
> After reading the 'example-schema', I Still cannot understand what the special case items
> means.

What is "special case items"?

> 
> But in my case, is the below change correct?
> [Diff]
> properties:
>    compatible:
> -    enum:
> -      - richtek,rtq6053
> -      - richtek,rtq6056
> -      - richtek,rtq6059
> +    oneOf:
> +      - items:
> +          - enum:
> +              - richtek,rtq6053
> +              - richtek,rtq6056
> +              - richtek,rtq6059

This changes nothing, you still have just one item. The example-schema
has exactly that case, so why you are coding it differently?

Anyway, test your DTS with the fallback, you will see that above does
not work.

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family

[Krzysztof Kozlowski]

On 28/12/2023 04:51, cy_huang@richtek.com wrote:
> From: ChiYuan Huang <cy_huang@richtek.com>
> 
> Add compatible support for RTQ6053 and RTQ6059.
> 
> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>

Why do you resend the same patches after two days ignoring the feedback?
No versioning, no changelog?

NAK.

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.

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family

[ChiYuan Huang]

On Thu, Dec 28, 2023 at 08:09:35AM +0100, Krzysztof Kozlowski wrote:
> On 28/12/2023 04:19, ChiYuan Huang wrote:
> > On Tue, Dec 26, 2023 at 01:12:50PM +0100, Krzysztof Kozlowski wrote:
> >> On 26/12/2023 12:19, ChiYuan Huang wrote:
> >>> On Tue, Dec 26, 2023 at 10:18:47AM +0100, Krzysztof Kozlowski wrote:
> >>>> On 26/12/2023 04:47, cy_huang@richtek.com wrote:
> >>>>> From: ChiYuan Huang <cy_huang@richtek.com>
> >>>>>
> >>>>> Add compatible support for RTQ6053 and RTQ6059.
> >>>>>
> >>>>> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> >>>>> ---
> >>>>>  .../devicetree/bindings/iio/adc/richtek,rtq6056.yaml         | 5 ++++-
> >>>>>  1 file changed, 4 insertions(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
> >>>>> index 88e008629ea8..d1e1f36d1972 100644
> >>>>> --- a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
> >>>>> +++ b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
> >>>>> @@ -25,7 +25,10 @@ description: |
> >>>>>  
> >>>>>  properties:
> >>>>>    compatible:
> >>>>> -    const: richtek,rtq6056
> >>>>> +    enum:
> >>>>> +      - richtek,rtq6053
> >>>>> +      - richtek,rtq6056
> >>>>
> >>>> Aren't these devices compatible? Your driver change says they are, so
> >>>> express compatibility with list here (and oneOf).
> >>>>
> >>> Thanks, I try to take other binding as the reference. One more question.
> >>> If rtq6053 is compatible with rtq6056, there's only chip package type difference.
> >>> Do I need to seperate it into a dedicated enum element?
> >>> Or just put it into one item and said this part number is compatible with rtq6056? 
> >>
> >> See example-schema. You need enum and items, both in oneOf:.
> >>
> > After reading the 'example-schema', I Still cannot understand what the special case items
> > means.
> 
> What is "special case items"?
> 
I may misunderstand something. The special case is the 'fallback' that you mentaioned in
the last.
> > 
> > But in my case, is the below change correct?
> > [Diff]
> > properties:
> >    compatible:
> > -    enum:
> > -      - richtek,rtq6053
> > -      - richtek,rtq6056
> > -      - richtek,rtq6059
> > +    oneOf:
> > +      - items:
> > +          - enum:
> > +              - richtek,rtq6053
> > +              - richtek,rtq6056
> > +              - richtek,rtq6059
> 
> This changes nothing, you still have just one item. The example-schema
> has exactly that case, so why you are coding it differently?
> 
> Anyway, test your DTS with the fallback, you will see that above does
> not work.
> 
I rewrite the below one and tested. it seems correct.

[Diff]
   compatible:
-    enum:
-      - richtek,rtq6053
-      - richtek,rtq6056
-      - richtek,rtq6059
+    oneOf:
+      - enum:
+          - richtek,rtq6053
+          - richtek,rtq6059
+      - items:
+          - const: richtek,rtq6056

Just one more question. If rtq6053 is fully compatibie with rtq6056, does it need to be put
into oneOf enum or be just put into items enum and use the 'fallback' mechanism?

If so, the 'richtek,rtq6053' in of_device_id match can be removed. Though it just remove
one line, but less-changed would be better.

ChiYuan.
Best regards,
> Krzysztof
> 

Re: [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family

[Krzysztof Kozlowski]

On 28/12/2023 08:58, ChiYuan Huang wrote:
> On Thu, Dec 28, 2023 at 08:09:35AM +0100, Krzysztof Kozlowski wrote:
>> On 28/12/2023 04:19, ChiYuan Huang wrote:
>>> On Tue, Dec 26, 2023 at 01:12:50PM +0100, Krzysztof Kozlowski wrote:
>>>> On 26/12/2023 12:19, ChiYuan Huang wrote:
>>>>> On Tue, Dec 26, 2023 at 10:18:47AM +0100, Krzysztof Kozlowski wrote:
>>>>>> On 26/12/2023 04:47, cy_huang@richtek.com wrote:
>>>>>>> From: ChiYuan Huang <cy_huang@richtek.com>
>>>>>>>
>>>>>>> Add compatible support for RTQ6053 and RTQ6059.
>>>>>>>
>>>>>>> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
>>>>>>> ---
>>>>>>>  .../devicetree/bindings/iio/adc/richtek,rtq6056.yaml         | 5 ++++-
>>>>>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
>>>>>>> index 88e008629ea8..d1e1f36d1972 100644
>>>>>>> --- a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
>>>>>>> +++ b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
>>>>>>> @@ -25,7 +25,10 @@ description: |
>>>>>>>  
>>>>>>>  properties:
>>>>>>>    compatible:
>>>>>>> -    const: richtek,rtq6056
>>>>>>> +    enum:
>>>>>>> +      - richtek,rtq6053
>>>>>>> +      - richtek,rtq6056
>>>>>>
>>>>>> Aren't these devices compatible? Your driver change says they are, so
>>>>>> express compatibility with list here (and oneOf).
>>>>>>
>>>>> Thanks, I try to take other binding as the reference. One more question.
>>>>> If rtq6053 is compatible with rtq6056, there's only chip package type difference.
>>>>> Do I need to seperate it into a dedicated enum element?
>>>>> Or just put it into one item and said this part number is compatible with rtq6056? 
>>>>
>>>> See example-schema. You need enum and items, both in oneOf:.
>>>>
>>> After reading the 'example-schema', I Still cannot understand what the special case items
>>> means.
>>
>> What is "special case items"?
>>
> I may misunderstand something. The special case is the 'fallback' that you mentaioned in
> the last.
>>>
>>> But in my case, is the below change correct?
>>> [Diff]
>>> properties:
>>>    compatible:
>>> -    enum:
>>> -      - richtek,rtq6053
>>> -      - richtek,rtq6056
>>> -      - richtek,rtq6059
>>> +    oneOf:
>>> +      - items:
>>> +          - enum:
>>> +              - richtek,rtq6053
>>> +              - richtek,rtq6056
>>> +              - richtek,rtq6059
>>
>> This changes nothing, you still have just one item. The example-schema
>> has exactly that case, so why you are coding it differently?
>>
>> Anyway, test your DTS with the fallback, you will see that above does
>> not work.
>>
> I rewrite the below one and tested. it seems correct.
> 
> [Diff]
>    compatible:
> -    enum:
> -      - richtek,rtq6053
> -      - richtek,rtq6056
> -      - richtek,rtq6059
> +    oneOf:
> +      - enum:
> +          - richtek,rtq6053
> +          - richtek,rtq6059
> +      - items:
> +          - const: richtek,rtq6056

You still need two items here to express compatibility. What is
compatible with what? It must be rtq6053 compatible with rtq6056,
because you cannot break the ABI, can you?

> 
> Just one more question. If rtq6053 is fully compatibie with rtq6056, does it need to be put
> into oneOf enum or be just put into items enum and use the 'fallback' mechanism?

The fallback is just a term. The point is to have a list of two
compatibles. See: Devicetree specification, writing-bindings and
numerous presentations about writing DTS.

> 
> If so, the 'richtek,rtq6053' in of_device_id match can be removed. Though it just remove
> one line, but less-changed would be better.

The device using fallback should be removed the driver of_device_id, so
6053 goes away.


Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: iio: adc: rtq6056: add support for the whole RTQ6056 family

[ChiYuan Huang]

Hi, Krzysztof:
On Thu, Dec 28, 2023 at 09:06:30AM +0100, Krzysztof Kozlowski wrote:
> On 28/12/2023 08:58, ChiYuan Huang wrote:
> > On Thu, Dec 28, 2023 at 08:09:35AM +0100, Krzysztof Kozlowski wrote:
> >> On 28/12/2023 04:19, ChiYuan Huang wrote:
> >>> On Tue, Dec 26, 2023 at 01:12:50PM +0100, Krzysztof Kozlowski wrote:
> >>>> On 26/12/2023 12:19, ChiYuan Huang wrote:
> >>>>> On Tue, Dec 26, 2023 at 10:18:47AM +0100, Krzysztof Kozlowski wrote:
> >>>>>> On 26/12/2023 04:47, cy_huang@richtek.com wrote:
> >>>>>>> From: ChiYuan Huang <cy_huang@richtek.com>
> >>>>>>>
> >>>>>>> Add compatible support for RTQ6053 and RTQ6059.
> >>>>>>>
> >>>>>>> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> >>>>>>> ---
> >>>>>>>  .../devicetree/bindings/iio/adc/richtek,rtq6056.yaml         | 5 ++++-
> >>>>>>>  1 file changed, 4 insertions(+), 1 deletion(-)
> >>>>>>>
> >>>>>>> diff --git a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
> >>>>>>> index 88e008629ea8..d1e1f36d1972 100644
> >>>>>>> --- a/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
> >>>>>>> +++ b/Documentation/devicetree/bindings/iio/adc/richtek,rtq6056.yaml
> >>>>>>> @@ -25,7 +25,10 @@ description: |
> >>>>>>>  
> >>>>>>>  properties:
> >>>>>>>    compatible:
> >>>>>>> -    const: richtek,rtq6056
> >>>>>>> +    enum:
> >>>>>>> +      - richtek,rtq6053
> >>>>>>> +      - richtek,rtq6056
> >>>>>>
> >>>>>> Aren't these devices compatible? Your driver change says they are, so
> >>>>>> express compatibility with list here (and oneOf).
> >>>>>>
> >>>>> Thanks, I try to take other binding as the reference. One more question.
> >>>>> If rtq6053 is compatible with rtq6056, there's only chip package type difference.
> >>>>> Do I need to seperate it into a dedicated enum element?
> >>>>> Or just put it into one item and said this part number is compatible with rtq6056? 
> >>>>
> >>>> See example-schema. You need enum and items, both in oneOf:.
> >>>>
> >>> After reading the 'example-schema', I Still cannot understand what the special case items
> >>> means.
> >>
> >> What is "special case items"?
> >>
> > I may misunderstand something. The special case is the 'fallback' that you mentaioned in
> > the last.
> >>>
> >>> But in my case, is the below change correct?
> >>> [Diff]
> >>> properties:
> >>>    compatible:
> >>> -    enum:
> >>> -      - richtek,rtq6053
> >>> -      - richtek,rtq6056
> >>> -      - richtek,rtq6059
> >>> +    oneOf:
> >>> +      - items:
> >>> +          - enum:
> >>> +              - richtek,rtq6053
> >>> +              - richtek,rtq6056
> >>> +              - richtek,rtq6059
> >>
> >> This changes nothing, you still have just one item. The example-schema
> >> has exactly that case, so why you are coding it differently?
> >>
> >> Anyway, test your DTS with the fallback, you will see that above does
> >> not work.
> >>
> > I rewrite the below one and tested. it seems correct.
> > 
> > [Diff]
> >    compatible:
> > -    enum:
> > -      - richtek,rtq6053
> > -      - richtek,rtq6056
> > -      - richtek,rtq6059
> > +    oneOf:
> > +      - enum:
> > +          - richtek,rtq6053
> > +          - richtek,rtq6059
> > +      - items:
> > +          - const: richtek,rtq6056
> 
> You still need two items here to express compatibility. What is
> compatible with what? It must be rtq6053 compatible with rtq6056,
> because you cannot break the ABI, can you?
> 
> > 
> > Just one more question. If rtq6053 is fully compatibie with rtq6056, does it need to be put
> > into oneOf enum or be just put into items enum and use the 'fallback' mechanism?
> 
> The fallback is just a term. The point is to have a list of two
> compatibles. See: Devicetree specification, writing-bindings and
> numerous presentations about writing DTS.
> 
> > 
> > If so, the 'richtek,rtq6053' in of_device_id match can be removed. Though it just remove
> > one line, but less-changed would be better.
> 
> The device using fallback should be removed the driver of_device_id, so
> 6053 goes away.
>
Thanks for all the information. If not misunderstanding, I'll remove "dt match id" for 6053.

And the binding fix in v2 will be 
 properties:
   compatible:
-    enum:
-      - richtek,rtq6053
-      - richtek,rtq6056
-      - richtek,rtq6059
+    oneOf:
+      - enum:
+          - richtek,rtq6056
+          - richtek,rtq6059
+      - items:
+          - enum:
+              - richtek,rtq6053
+          - const: richtek,rtq6056
 
This make the 6053 compatible with 6056 with DT fallback.

> Best regards,
> Krzysztof
>