| Message ID | 20230621160857.3400747-1-m.felsch@pengutronix.de (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | dt-bindings: iio: adc: ti,ads1015: fix datarate max value and meaning | expand |
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 >
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>
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 > > >
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
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.
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
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
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 >
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
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(-)