| Message ID | 20210708235618.1541335-1-liambeguin@gmail.com |
|---|---|
| Headers | show |
| Series | AD7949 Fixes | expand |
Hi Liam, The series looks good. Just some minor comments from my side... Thanks! - Nuno Sá > From: Liam Beguin <liambeguin@gmail.com> > Sent: Friday, July 9, 2021 1:56 AM > To: liambeguin@gmail.com; lars@metafoo.de; Hennerich, Michael > <Michael.Hennerich@analog.com>; jic23@kernel.org; charles- > antoine.couret@essensium.com > Cc: linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org; > devicetree@vger.kernel.org; robh+dt@kernel.org > Subject: [PATCH v1 0/4] AD7949 Fixes > > [External] > > While working on another series[1] I ran into issues where my SPI > controller would fail to handle 14-bit and 16-bit SPI messages. This > addresses that issue and adds support for selecting a different voltage > reference source from the devicetree. > > This is base on a series[2] that seems to not have made it all the way, > and was tested on an ad7689. > > [1] > https://urldefense.com/v3/__https://patchwork.kernel.org/project/li > nux-iio/list/?series=511545__;!!A3Ni8CS0y2Y!rF7O-WxC7HZM4hv_o- > zYPtVCmZlDoASzPvamwwQkH9llVS8J-GpfQltvDBz7gQ$ > [2] > https://urldefense.com/v3/__https://patchwork.kernel.org/project/li > nux- > iio/list/?series=116971&state=*2A&archive=both__;JQ!!A3Ni8CS0y2Y! > rF7O-WxC7HZM4hv_o-zYPtVCmZlDoASzPvamwwQkH9llVS8J-GpfQluZ- > w3LYA$ > > Thanks for your time, > Liam > > Liam Beguin (4): > iio: adc: ad7949: define and use bitfield names > iio: adc: ad7949: fix spi messages on non 14-bit controllers > iio: adc: ad7949: add support for internal vref > dt-bindings: iio: adc: ad7949: add adi,reference-source > > .../bindings/iio/adc/adi,ad7949.yaml | 22 ++ > drivers/iio/adc/ad7949.c | 191 +++++++++++++++--- > 2 files changed, 181 insertions(+), 32 deletions(-) > > > base-commit: 6cbb3aa0f9d5d23221df787cf36f74d3866fdb78 > -- > 2.30.1.489.g328c10930387
> -----Original Message----- > From: Liam Beguin <liambeguin@gmail.com> > Sent: Friday, July 9, 2021 1:56 AM > To: liambeguin@gmail.com; lars@metafoo.de; Hennerich, Michael > <Michael.Hennerich@analog.com>; jic23@kernel.org; charles- > antoine.couret@essensium.com > Cc: linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org; > devicetree@vger.kernel.org; robh+dt@kernel.org > Subject: [PATCH v1 4/4] dt-bindings: iio: adc: ad7949: add > adi,reference-source > > [External] > > From: Liam Beguin <lvb@xiphos.com> > > Add bindings documentation for the adi,reference-source property. > This property is required to properly configure the ADC sample request > based on which reference source should be used for the calculation. > > Signed-off-by: Liam Beguin <lvb@xiphos.com> > --- > .../bindings/iio/adc/adi,ad7949.yaml | 22 +++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/iio/adc/adi,ad7949.yaml > b/Documentation/devicetree/bindings/iio/adc/adi,ad7949.yaml > index 9b56bd4d5510..3f4629281cc8 100644 > --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7949.yaml > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7949.yaml > @@ -35,6 +35,28 @@ properties: > "#io-channel-cells": > const: 1 > > + adi,reference-select: > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + - enum: [0, 1, 2, 3, 6, 7] > + > + default: 7 > + description: | > + Select the reference voltage source to use when converting > samples. > + Acceptable values are: > + - 0: Internal reference and temperature sensor enabled. > + Vref=2.5V, buffered output > + - 1: Internal reference and temperature sensor enabled. > + Vref=4.096V, buffered output > + - 2: Use external reference, temperature sensor enabled. > + Internal buffer disabled > + - 3: Use external reference, internal buffer and temperature > sensor > + enabled. > + - 6: Use external reference, internal buffer and temperature > sensor > + disabled. > + - 7: Use external reference, internal buffer enabled. > + Internal reference and temperature sensor disabled. I think typically the description comes first. I also don't think you need the 'allOf'(not even sure if it will pass the binding check)... Just have '$ref' and 'enum' on the same level. - Nuno Sá > required: > - compatible > - reg > -- > 2.30.1.489.g328c10930387
Hi Nuno, On Fri Jul 9, 2021 at 4:15 AM EDT, Sa, Nuno wrote: > > > > -----Original Message----- > > From: Liam Beguin <liambeguin@gmail.com> > > Sent: Friday, July 9, 2021 1:56 AM > > To: liambeguin@gmail.com; lars@metafoo.de; Hennerich, Michael > > <Michael.Hennerich@analog.com>; jic23@kernel.org; charles- > > antoine.couret@essensium.com > > Cc: linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org; > > devicetree@vger.kernel.org; robh+dt@kernel.org > > Subject: [PATCH v1 4/4] dt-bindings: iio: adc: ad7949: add > > adi,reference-source > > > > [External] > > > > From: Liam Beguin <lvb@xiphos.com> > > > > Add bindings documentation for the adi,reference-source property. > > This property is required to properly configure the ADC sample request > > based on which reference source should be used for the calculation. > > > > Signed-off-by: Liam Beguin <lvb@xiphos.com> > > --- > > .../bindings/iio/adc/adi,ad7949.yaml | 22 +++++++++++++++++++ > > 1 file changed, 22 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/iio/adc/adi,ad7949.yaml > > b/Documentation/devicetree/bindings/iio/adc/adi,ad7949.yaml > > index 9b56bd4d5510..3f4629281cc8 100644 > > --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7949.yaml > > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7949.yaml > > @@ -35,6 +35,28 @@ properties: > > "#io-channel-cells": > > const: 1 > > > > + adi,reference-select: > > + allOf: > > + - $ref: /schemas/types.yaml#/definitions/uint32 > > + - enum: [0, 1, 2, 3, 6, 7] > > + > > + default: 7 > > + description: | > > + Select the reference voltage source to use when converting > > samples. > > + Acceptable values are: > > + - 0: Internal reference and temperature sensor enabled. > > + Vref=2.5V, buffered output > > + - 1: Internal reference and temperature sensor enabled. > > + Vref=4.096V, buffered output > > + - 2: Use external reference, temperature sensor enabled. > > + Internal buffer disabled > > + - 3: Use external reference, internal buffer and temperature > > sensor > > + enabled. > > + - 6: Use external reference, internal buffer and temperature > > sensor > > + disabled. > > + - 7: Use external reference, internal buffer enabled. > > + Internal reference and temperature sensor disabled. > > I think typically the description comes first. I also don't think you > need the 'allOf'(not even sure if it will pass the binding check)... > Just have '$ref' and 'enum' on the same level. > Understood, I can reorder the patches so that the bindings come first. I thought I based that part on the `example-schema.yaml`, but looking at it again, it seems like you're right and the AllOf isn't required. I did run the bindings check on this, but I'll fix it. Thanks, Liam > - Nuno Sá > > > required: > > - compatible > > - reg > > -- > > 2.30.1.489.g328c10930387
On Fri Jul 9, 2021 at 4:15 AM EDT, Sa, Nuno wrote: > > > > -----Original Message----- > > From: Liam Beguin <liambeguin@gmail.com> > > Sent: Friday, July 9, 2021 1:56 AM > > To: liambeguin@gmail.com; lars@metafoo.de; Hennerich, Michael > > <Michael.Hennerich@analog.com>; jic23@kernel.org; charles- > > antoine.couret@essensium.com > > Cc: linux-kernel@vger.kernel.org; linux-iio@vger.kernel.org; > > devicetree@vger.kernel.org; robh+dt@kernel.org > > Subject: [PATCH v1 4/4] dt-bindings: iio: adc: ad7949: add > > adi,reference-source > > > > [External] > > > > From: Liam Beguin <lvb@xiphos.com> > > > > Add bindings documentation for the adi,reference-source property. > > This property is required to properly configure the ADC sample request > > based on which reference source should be used for the calculation. > > > > Signed-off-by: Liam Beguin <lvb@xiphos.com> > > --- > > .../bindings/iio/adc/adi,ad7949.yaml | 22 +++++++++++++++++++ > > 1 file changed, 22 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/iio/adc/adi,ad7949.yaml > > b/Documentation/devicetree/bindings/iio/adc/adi,ad7949.yaml > > index 9b56bd4d5510..3f4629281cc8 100644 > > --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7949.yaml > > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7949.yaml > > @@ -35,6 +35,28 @@ properties: > > "#io-channel-cells": > > const: 1 > > > > + adi,reference-select: > > + allOf: > > + - $ref: /schemas/types.yaml#/definitions/uint32 > > + - enum: [0, 1, 2, 3, 6, 7] > > + > > + default: 7 > > + description: | > > + Select the reference voltage source to use when converting > > samples. > > + Acceptable values are: > > + - 0: Internal reference and temperature sensor enabled. > > + Vref=2.5V, buffered output > > + - 1: Internal reference and temperature sensor enabled. > > + Vref=4.096V, buffered output > > + - 2: Use external reference, temperature sensor enabled. > > + Internal buffer disabled > > + - 3: Use external reference, internal buffer and temperature > > sensor > > + enabled. > > + - 6: Use external reference, internal buffer and temperature > > sensor > > + disabled. > > + - 7: Use external reference, internal buffer enabled. > > + Internal reference and temperature sensor disabled. > > I think typically the description comes first. I also don't think you > need the 'allOf'(not even sure if it will pass the binding check)... > Just have '$ref' and 'enum' on the same level. > I realize I misread your comment on the order of the description. I'll move it at the top of the binding definition. Liam > - Nuno Sá > > > required: > > - compatible > > - reg > > -- > > 2.30.1.489.g328c10930387