2023-12-26 09:19:05

by Krzysztof Kozlowski

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

On 26/12/2023 04:47, [email protected] wrote:
> From: ChiYuan Huang <[email protected]>
>
> Add compatible support for RTQ6053 and RTQ6059.
>
> Signed-off-by: ChiYuan Huang <[email protected]>
> ---
> .../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



2023-12-26 11:20:03

by ChiYuan Huang

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

On Tue, Dec 26, 2023 at 10:18:47AM +0100, Krzysztof Kozlowski wrote:
> On 26/12/2023 04:47, [email protected] wrote:
> > From: ChiYuan Huang <[email protected]>
> >
> > Add compatible support for RTQ6053 and RTQ6059.
> >
> > Signed-off-by: ChiYuan Huang <[email protected]>
> > ---
> > .../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
>

2023-12-26 12:13:04

by Krzysztof Kozlowski

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

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, [email protected] wrote:
>>> From: ChiYuan Huang <[email protected]>
>>>
>>> Add compatible support for RTQ6053 and RTQ6059.
>>>
>>> Signed-off-by: ChiYuan Huang <[email protected]>
>>> ---
>>> .../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


2023-12-26 16:25:11

by Jonathan Cameron

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

On Tue, 26 Dec 2023 10:18:47 +0100
Krzysztof Kozlowski <[email protected]> wrote:

> On 26/12/2023 04:47, [email protected] wrote:
> > From: ChiYuan Huang <[email protected]>
> >
> > Add compatible support for RTQ6053 and RTQ6059.
> >
> > Signed-off-by: ChiYuan Huang <[email protected]>
> > ---
> > .../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
>


2023-12-28 03:20:55

by ChiYuan Huang

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

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, [email protected] wrote:
> >>> From: ChiYuan Huang <[email protected]>
> >>>
> >>> Add compatible support for RTQ6053 and RTQ6059.
> >>>
> >>> Signed-off-by: ChiYuan Huang <[email protected]>
> >>> ---
> >>> .../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
>

2023-12-28 03:26:46

by ChiYuan Huang

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

On Tue, Dec 26, 2023 at 04:24:50PM +0000, Jonathan Cameron wrote:
> On Tue, 26 Dec 2023 10:18:47 +0100
> Krzysztof Kozlowski <[email protected]> wrote:
>
> > On 26/12/2023 04:47, [email protected] wrote:
> > > From: ChiYuan Huang <[email protected]>
> > >
> > > Add compatible support for RTQ6053 and RTQ6059.
> > >
> > > Signed-off-by: ChiYuan Huang <[email protected]>
> > > ---
> > > .../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
> >
>

2023-12-28 03:37:56

by ChiYuan Huang

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

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 <[email protected]> wrote:
> >
> > > On 26/12/2023 04:47, [email protected] wrote:
> > > > From: ChiYuan Huang <[email protected]>
> > > >
> > > > Add compatible support for RTQ6053 and RTQ6059.
> > > >
> > > > Signed-off-by: ChiYuan Huang <[email protected]>
> > > > ---
> > > > .../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
> > >
> >

2023-12-28 07:09:50

by Krzysztof Kozlowski

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

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, [email protected] wrote:
>>>>> From: ChiYuan Huang <[email protected]>
>>>>>
>>>>> Add compatible support for RTQ6053 and RTQ6059.
>>>>>
>>>>> Signed-off-by: ChiYuan Huang <[email protected]>
>>>>> ---
>>>>> .../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


2023-12-28 07:59:08

by ChiYuan Huang

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

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, [email protected] wrote:
> >>>>> From: ChiYuan Huang <[email protected]>
> >>>>>
> >>>>> Add compatible support for RTQ6053 and RTQ6059.
> >>>>>
> >>>>> Signed-off-by: ChiYuan Huang <[email protected]>
> >>>>> ---
> >>>>> .../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
>

2023-12-28 08:06:47

by Krzysztof Kozlowski

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

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, [email protected] wrote:
>>>>>>> From: ChiYuan Huang <[email protected]>
>>>>>>>
>>>>>>> Add compatible support for RTQ6053 and RTQ6059.
>>>>>>>
>>>>>>> Signed-off-by: ChiYuan Huang <[email protected]>
>>>>>>> ---
>>>>>>> .../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


2023-12-28 08:45:07

by ChiYuan Huang

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

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, [email protected] wrote:
> >>>>>>> From: ChiYuan Huang <[email protected]>
> >>>>>>>
> >>>>>>> Add compatible support for RTQ6053 and RTQ6059.
> >>>>>>>
> >>>>>>> Signed-off-by: ChiYuan Huang <[email protected]>
> >>>>>>> ---
> >>>>>>> .../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
>