[PATCH] dt-bindings: iio: adc: ti,ads1015: fix datarate max value and meaning

[PATCH] dt-bindings: iio: adc: ti,ads1015: fix datarate max value and meaning

[Marco Felsch]

Datarate (dr) is a 3-bit wide register field. Values from 0 to 7 are
allowed for all devices but only for the ADS1115 devices a value of 7
does make a difference.

While on it fix the description of the datarate for ADS1115 devices as
well.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 .../devicetree/bindings/iio/adc/ti,ads1015.yaml   | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
index 2127d639a7683..e004659099c19 100644
--- a/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
@@ -78,9 +78,9 @@ patternProperties:
       ti,datarate:
         $ref: /schemas/types.yaml#/definitions/uint32
         minimum: 0
-        maximum: 6
+        maximum: 7
         description: |
-          Data acquisition rate in samples per second
+          Data acquisition rate in samples per second for ADS1015, TLA2024
           0: 128
           1: 250
           2: 490
@@ -88,6 +88,17 @@ patternProperties:
           4: 1600 (default)
           5: 2400
           6: 3300
+          7: 3300
+
+          Data acquisition rate in samples per second for ADS1115
+          0: 8
+          1: 16
+          2: 32
+          3: 64
+          4: 128 (default)
+          5: 250
+          6: 475
+          7: 860
 
     required:
       - reg
-- 
2.39.2

Re: [PATCH] dt-bindings: iio: adc: ti,ads1015: fix datarate max value and meaning

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2000 bytes --]

On Wed, Jun 21, 2023 at 06:08:57PM +0200, Marco Felsch wrote:
> Datarate (dr) is a 3-bit wide register field. Values from 0 to 7 are
> allowed for all devices but only for the ADS1115 devices a value of 7
> does make a difference.
> 
> While on it fix the description of the datarate for ADS1115 devices as
> well.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  .../devicetree/bindings/iio/adc/ti,ads1015.yaml   | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
> index 2127d639a7683..e004659099c19 100644
> --- a/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
> @@ -78,9 +78,9 @@ patternProperties:
>        ti,datarate:
>          $ref: /schemas/types.yaml#/definitions/uint32
>          minimum: 0
> -        maximum: 6
> +        maximum: 7
>          description: |
> -          Data acquisition rate in samples per second
> +          Data acquisition rate in samples per second for ADS1015, TLA2024
>            0: 128
>            1: 250
>            2: 490
> @@ -88,6 +88,17 @@ patternProperties:
>            4: 1600 (default)
>            5: 2400
>            6: 3300
> +          7: 3300
> +
> +          Data acquisition rate in samples per second for ADS1115
> +          0: 8
> +          1: 16
> +          2: 32
> +          3: 64
> +          4: 128 (default)
> +          5: 250
> +          6: 475
> +          7: 860

I'll leave this one to Rob or Krzysztof to ack/review, but this does
seem like as good an opportunity as any to migrate to a property that
allows you to put the actual data acquisition rate in & not have to add
new key-value mappings to the binding to support devices with differing
schemes.

>  
>      required:
>        - reg
> -- 
> 2.39.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] dt-bindings: iio: adc: ti,ads1015: fix datarate max value and meaning

[Rob Herring]

On Wed, 21 Jun 2023 18:08:57 +0200, Marco Felsch wrote:
> Datarate (dr) is a 3-bit wide register field. Values from 0 to 7 are
> allowed for all devices but only for the ADS1115 devices a value of 7
> does make a difference.
> 
> While on it fix the description of the datarate for ADS1115 devices as
> well.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  .../devicetree/bindings/iio/adc/ti,ads1015.yaml   | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH] dt-bindings: iio: adc: ti,ads1015: fix datarate max value and meaning

[Jonathan Cameron]

On Wed, 21 Jun 2023 21:41:05 +0100
Conor Dooley <conor@kernel.org> wrote:

> On Wed, Jun 21, 2023 at 06:08:57PM +0200, Marco Felsch wrote:
> > Datarate (dr) is a 3-bit wide register field. Values from 0 to 7 are
> > allowed for all devices but only for the ADS1115 devices a value of 7
> > does make a difference.
> > 
> > While on it fix the description of the datarate for ADS1115 devices as
> > well.
> > 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> >  .../devicetree/bindings/iio/adc/ti,ads1015.yaml   | 15 +++++++++++++--
> >  1 file changed, 13 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
> > index 2127d639a7683..e004659099c19 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
> > +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
> > @@ -78,9 +78,9 @@ patternProperties:
> >        ti,datarate:
> >          $ref: /schemas/types.yaml#/definitions/uint32
> >          minimum: 0
> > -        maximum: 6
> > +        maximum: 7
> >          description: |
> > -          Data acquisition rate in samples per second
> > +          Data acquisition rate in samples per second for ADS1015, TLA2024
> >            0: 128
> >            1: 250
> >            2: 490
> > @@ -88,6 +88,17 @@ patternProperties:
> >            4: 1600 (default)
> >            5: 2400
> >            6: 3300
> > +          7: 3300
> > +
> > +          Data acquisition rate in samples per second for ADS1115
> > +          0: 8
> > +          1: 16
> > +          2: 32
> > +          3: 64
> > +          4: 128 (default)
> > +          5: 250
> > +          6: 475
> > +          7: 860  
> 
> I'll leave this one to Rob or Krzysztof to ack/review, but this does
> seem like as good an opportunity as any to migrate to a property that
> allows you to put the actual data acquisition rate in & not have to add
> new key-value mappings to the binding to support devices with differing
> schemes.

I agree a value would have been better, but now we are where we are,
I'm not sure it's worth the churn of changing it - particularly as the
driver will need to support the old binding for every anyway.

Jonathan

> 
> >  
> >      required:
> >        - reg
> > -- 
> > 2.39.2
> >   
> 

Re: [PATCH] dt-bindings: iio: adc: ti,ads1015: fix datarate max value and meaning

[Marco Felsch]

On 23-07-02, Jonathan Cameron wrote:
> On Wed, 21 Jun 2023 21:41:05 +0100
> Conor Dooley <conor@kernel.org> wrote:
> 
> > On Wed, Jun 21, 2023 at 06:08:57PM +0200, Marco Felsch wrote:
> > > Datarate (dr) is a 3-bit wide register field. Values from 0 to 7 are
> > > allowed for all devices but only for the ADS1115 devices a value of 7
> > > does make a difference.
> > > 
> > > While on it fix the description of the datarate for ADS1115 devices as
> > > well.
> > > 
> > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > > ---
> > >  .../devicetree/bindings/iio/adc/ti,ads1015.yaml   | 15 +++++++++++++--
> > >  1 file changed, 13 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
> > > index 2127d639a7683..e004659099c19 100644
> > > --- a/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
> > > +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
> > > @@ -78,9 +78,9 @@ patternProperties:
> > >        ti,datarate:
> > >          $ref: /schemas/types.yaml#/definitions/uint32
> > >          minimum: 0
> > > -        maximum: 6
> > > +        maximum: 7
> > >          description: |
> > > -          Data acquisition rate in samples per second
> > > +          Data acquisition rate in samples per second for ADS1015, TLA2024
> > >            0: 128
> > >            1: 250
> > >            2: 490
> > > @@ -88,6 +88,17 @@ patternProperties:
> > >            4: 1600 (default)
> > >            5: 2400
> > >            6: 3300
> > > +          7: 3300
> > > +
> > > +          Data acquisition rate in samples per second for ADS1115
> > > +          0: 8
> > > +          1: 16
> > > +          2: 32
> > > +          3: 64
> > > +          4: 128 (default)
> > > +          5: 250
> > > +          6: 475
> > > +          7: 860  
> > 
> > I'll leave this one to Rob or Krzysztof to ack/review, but this does
> > seem like as good an opportunity as any to migrate to a property that
> > allows you to put the actual data acquisition rate in & not have to add
> > new key-value mappings to the binding to support devices with differing
> > schemes.
> 
> I agree a value would have been better, but now we are where we are,
> I'm not sure it's worth the churn of changing it - particularly as the
> driver will need to support the old binding for every anyway.

Yep, this would be an API change :/

Regards,
  Marco

Re: [PATCH] dt-bindings: iio: adc: ti,ads1015: fix datarate max value and meaning

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2880 bytes --]

On Mon, Jul 03, 2023 at 08:46:26AM +0200, Marco Felsch wrote:
> On 23-07-02, Jonathan Cameron wrote:
> > On Wed, 21 Jun 2023 21:41:05 +0100
> > Conor Dooley <conor@kernel.org> wrote:
> > 
> > > On Wed, Jun 21, 2023 at 06:08:57PM +0200, Marco Felsch wrote:
> > > > Datarate (dr) is a 3-bit wide register field. Values from 0 to 7 are
> > > > allowed for all devices but only for the ADS1115 devices a value of 7
> > > > does make a difference.
> > > > 
> > > > While on it fix the description of the datarate for ADS1115 devices as
> > > > well.
> > > > 
> > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > > > ---
> > > >  .../devicetree/bindings/iio/adc/ti,ads1015.yaml   | 15 +++++++++++++--
> > > >  1 file changed, 13 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
> > > > index 2127d639a7683..e004659099c19 100644
> > > > --- a/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
> > > > +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1015.yaml
> > > > @@ -78,9 +78,9 @@ patternProperties:
> > > >        ti,datarate:
> > > >          $ref: /schemas/types.yaml#/definitions/uint32
> > > >          minimum: 0
> > > > -        maximum: 6
> > > > +        maximum: 7
> > > >          description: |
> > > > -          Data acquisition rate in samples per second
> > > > +          Data acquisition rate in samples per second for ADS1015, TLA2024
> > > >            0: 128
> > > >            1: 250
> > > >            2: 490
> > > > @@ -88,6 +88,17 @@ patternProperties:
> > > >            4: 1600 (default)
> > > >            5: 2400
> > > >            6: 3300
> > > > +          7: 3300
> > > > +
> > > > +          Data acquisition rate in samples per second for ADS1115
> > > > +          0: 8
> > > > +          1: 16
> > > > +          2: 32
> > > > +          3: 64
> > > > +          4: 128 (default)
> > > > +          5: 250
> > > > +          6: 475
> > > > +          7: 860  
> > > 
> > > I'll leave this one to Rob or Krzysztof to ack/review, but this does
> > > seem like as good an opportunity as any to migrate to a property that
> > > allows you to put the actual data acquisition rate in & not have to add
> > > new key-value mappings to the binding to support devices with differing
> > > schemes.
> > 
> > I agree a value would have been better, but now we are where we are,
> > I'm not sure it's worth the churn of changing it - particularly as the
> > driver will need to support the old binding for every anyway.
> 
> Yep, this would be an API change :/

Of course, but so what you have in these patches anyway. Change being
the operative word, not break ;)

Either way, I passed the buck to Rob and Krzysztof on this one anyway.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] dt-bindings: iio: adc: ti,ads1015: fix datarate max value and meaning

[Krzysztof Kozlowski]

On 21/06/2023 18:08, Marco Felsch wrote:
> Datarate (dr) is a 3-bit wide register field. Values from 0 to 7 are
> allowed for all devices but only for the ADS1115 devices a value of 7
> does make a difference.
> 
> While on it fix the description of the datarate for ADS1115 devices as
> well.
> 


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH] dt-bindings: iio: adc: ti,ads1015: fix datarate max value and meaning

[Krzysztof Kozlowski]

On 03/07/2023 18:14, Conor Dooley wrote:
>>>>> @@ -88,6 +88,17 @@ patternProperties:
>>>>>            4: 1600 (default)
>>>>>            5: 2400
>>>>>            6: 3300
>>>>> +          7: 3300
>>>>> +
>>>>> +          Data acquisition rate in samples per second for ADS1115
>>>>> +          0: 8
>>>>> +          1: 16
>>>>> +          2: 32
>>>>> +          3: 64
>>>>> +          4: 128 (default)
>>>>> +          5: 250
>>>>> +          6: 475
>>>>> +          7: 860  
>>>>
>>>> I'll leave this one to Rob or Krzysztof to ack/review, but this does
>>>> seem like as good an opportunity as any to migrate to a property that
>>>> allows you to put the actual data acquisition rate in & not have to add
>>>> new key-value mappings to the binding to support devices with differing
>>>> schemes.
>>>
>>> I agree a value would have been better, but now we are where we are,
>>> I'm not sure it's worth the churn of changing it - particularly as the
>>> driver will need to support the old binding for every anyway.
>>
>> Yep, this would be an API change :/
> 
> Of course, but so what you have in these patches anyway. Change being
> the operative word, not break ;)
> 
> Either way, I passed the buck to Rob and Krzysztof on this one anyway.

It's fine. Device-specific, so not common, properties can be expressing
programming model (register value).
https://lore.kernel.org/linux-devicetree/20230412133921.GA2017891-robh@kernel.org/

Such properties are usually less readable in DTS, so they have clear
disadvantage. However using here common units would require mapping in
the driver which is additional churn.

From every rule there are also exceptions. For example consider common
regulator values or number of samples where hwmon core is ready to parse
it properly:
https://lore.kernel.org/linux-devicetree/82d76824-ef5b-23f9-149e-2c5d9f88e94a@kernel.org/T/#mb2808172369a9960890a2de538464ca68dc86455

That's not the case here, so it's fine.

Best regards,
Krzysztof

Re: [PATCH] dt-bindings: iio: adc: ti,ads1015: fix datarate max value and meaning

[Jonathan Cameron]

On Mon, 3 Jul 2023 18:18:28 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 21/06/2023 18:08, Marco Felsch wrote:
> > Datarate (dr) is a 3-bit wide register field. Values from 0 to 7 are
> > allowed for all devices but only for the ADS1115 devices a value of 7
> > does make a difference.
> > 
> > While on it fix the description of the datarate for ADS1115 devices as
> > well.
> >   
> 
> 
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Applied,

Thanks,

Jonathan

> 
> Best regards,
> Krzysztof
>