From: Jonathan Cameron <jic23@kernel.org>
To: Song Qiang <songqiang1304521@gmail.com>
Cc: Rob Herring <robh+dt@kernel.org>,
Lars-Peter Clausen <lars@metafoo.de>,
Michael Hennerich <Michael.Hennerich@analog.com>,
Stefan Popa <stefan.popa@analog.com>,
Hartmut Knaack <knaack.h@gmx.de>,
Peter Meerwald <pmeerw@pmeerw.net>,
Liam Girdwood <lgirdwood@gmail.com>,
Mark Brown <broonie@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
"open list:IIO SUBSYSTEM AND DRIVERS" <linux-iio@vger.kernel.org>,
devicetree@vger.kernel.org,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/3] dt-bindings: iio: adc: add support for AD5940
Date: Sat, 16 Nov 2019 14:17:19 +0000 [thread overview]
Message-ID: <20191116141719.35a9208e@archlinux> (raw)
In-Reply-To: <c4f649be-de4f-1762-7748-88ebac6d0d53@gmail.com>
On Fri, 15 Nov 2019 01:06:32 +0800
Song Qiang <songqiang1304521@gmail.com> wrote:
> Thanks, I'll see how to use 'lable' to pass name of channels, other
> problems will be fixed in the next patch.
For a channel, it will be a new thing. We can introduce it in a
similar fashion to you did for channel-name but ultimately I think we
want to make it a standard part of the IIO core.
However, for either to happen the key thing is a documentation patch
describing it so that we can get the discussion going!
Thanks,
Jonathan
>
> yours,
> Song Qiang
>
> On 11/14/19 9:28 AM, Rob Herring wrote:
> > On Fri, Nov 8, 2019 at 7:09 AM Song Qiang <songqiang1304521@gmail.com> wrote:
> >>
> >> Add yaml devicetree description file and a header file for
> >> helping configure positive and negtive input of AD5940.
> >>
> >> Signed-off-by: Song Qiang <songqiang1304521@gmail.com>
> >> ---
> >> .../bindings/iio/adc/adi,ad5940.yaml | 240 ++++++++++++++++++
> >> include/dt-bindings/iio/adc/adi,ad5940.h | 52 ++++
> >> 2 files changed, 292 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad5940.yaml
> >> create mode 100644 include/dt-bindings/iio/adc/adi,ad5940.h
> >>
> >> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad5940.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad5940.yaml
> >> new file mode 100644
> >> index 000000000000..f7f034fdd8ec
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad5940.yaml
> >> @@ -0,0 +1,240 @@
> >> +# SPDX-License-Identifier: GPL-2.0
> >> +# Copyright 2019 Analog Devices Inc.
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/bindings/iio/adc/adi,ad5940.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: Analog Devices AD5940 Device Tree Bindings
> >> +
> >> +maintainers:
> >> + - Song Qiang <songqiang1304521@gmail.com>
> >> +
> >> +description: |
> >> + Analog Devices AD5940 High Precision, Impedance, and Electrochemical Front End.
> >> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD5940.pdf
> >> +
> >> +properties:
> >> + compatible:
> >> + enum:
> >> + - adi,ad5940
> >> +
> >> + reg:
> >> + maxItems: 1
> >> +
> >> + vref-supply:
> >> + description:
> >> + The regulator to be used to supply the reference voltage.
> >> + maxItems: 1
> >> +
> >> + adi,interrupt-io:
> >> + description:
> >> + Output GPIO index of interrupt controller of AD5940.
> >> + maxItems: 1
> >> + allOf:
> >> + - $ref: /schemas/types.yaml#/definitions/uint32
> >> + - enum: [0, 3, 6, 7]
> >> +
> >> + '#address-cells':
> >> + const: 1
> >> +
> >> + '#size-cells':
> >> + const: 0
> >> +
> >> +required:
> >> + - compatible
> >> + - reg
> >> + - interrupts
> >> + - adi,interrupt-io
> >> +
> >> +patternProperties:
> >> + # 'channel@0-255'
> >> + "^channel@([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])$":
> >> + type: object
> >> + description: |
> >> + Represents the external channels which are connected to the ADC.
> >> + See Documentation/devicetree/bindings/iio/adc/adc.txt.
> >> + properties:
> >> + reg:
> >> + description:
> >> + Index of this channel, must be starting from 0.
> >> + maxItems: 1
> >
> > Drop maxItems and do:
> >
> > items:
> > - minimum: 0
> > maximum: 255
> >
> >> +
> >> + diff-channels:
> >> + description:
> >> + Positive input and negtive input of the ADC buffer of this channel.
> >> + Input candidates are defined in include/dt-bindings/iio/adc/adi,ad5940.h.
> >> + minItems: 2
> >> + maxItems: 2
> >> + items:
> >> + - description: Positive input channel
> >> + - enum:
> >> + - AD5940_ADC_INPUTP_EXCITATION
> >
> > You can't have defines in the schema. minimum/maximum might work better here.
> >
> >> + - AD5940_ADC_INPUTP_FLOATING
> >> + - AD5940_ADC_INPUTP_HSTIA
> >> + - AD5940_ADC_INPUTP_LPTIA_LP
> >> + - AD5940_ADC_INPUTP_AIN0
> >> + - AD5940_ADC_INPUTP_AIN1
> >> + - AD5940_ADC_INPUTP_AIN2
> >> + - AD5940_ADC_INPUTP_AIN3
> >> + - AD5940_ADC_INPUTP_AVDD_2
> >> + - AD5940_ADC_INPUTP_DVDD_2
> >> + - AD5940_ADC_INPUTP_AVDD_REG_2
> >> + - AD5940_ADC_INPUTP_TEMP
> >> + - AD5940_ADC_INPUTP_VBIAS_CAP
> >> + - AD5940_ADC_INPUTP_DE0
> >> + - AD5940_ADC_INPUTP_SE0
> >> + - AD5940_ADC_INPUTP_VREF_2V5_2
> >> + - AD5940_ADC_INPUTP_VREF_1V82
> >> + - AD5940_ADC_INPUTP_P_TEMP_N
> >> + - AD5940_ADC_INPUTP_AIN4
> >> + - AD5940_ADC_INPUTP_AIN6
> >> + - AD5940_ADC_INPUTP_VZERO
> >> + - AD5940_ADC_INPUTP_VBIAS0
> >> + - AD5940_ADC_INPUTP_VCE0
> >> + - AD5940_ADC_INPUTP_VRE0
> >> + - AD5940_ADC_INPUTP_VCE0_2
> >> + - AD5940_ADC_INPUTP_LPTIA
> >> + - AD5940_ADC_INPUTP_AGND_REF
> >> +
> >> + - description: Negtive input channel
> >> + - enum:
> >> + # Negtive input candidates
> >> + - AD5940_ADC_INPUTN_FLOATING
> >> + - AD5940_ADC_INPUTN_HSTIA
> >> + - AD5940_ADC_INPUTN_LPTIA
> >> + - AD5940_ADC_INPUTN_AIN0
> >> + - AD5940_ADC_INPUTN_AIN1
> >> + - AD5940_ADC_INPUTN_AIN2
> >> + - AD5940_ADC_INPUTN_AIN3
> >> + - AD5940_ADC_INPUTN_VBIAS_CA8
> >> + - AD5940_ADC_INPUTN_TEMP_N
> >> + - AD5940_ADC_INPUTN_AIN4
> >> + - AD5940_ADC_INPUTN_AIN6
> >> + - AD5940_ADC_INPUTN_VZERO
> >> + - AD5940_ADC_INPUTN_VBIAS0
> >> + - AD5940_ADC_INPUTN_EXCITATION
> >
> > You've defined that diff-channels is 4 items. I don't think that's
> > what you want. Each enum and description should be under a single '-'.
> >
> >> +
> >> + channel-name:
> >
> > Perhaps standard property 'label' should be used here. Seems like a
> > common thing.
> >
> >> + description:
> >> + Any string format name you would like to assign to this channel.
> >> + maxItems: 1
> >> +
> >> + required:
> >> + - reg
> >> + - diff-channels
> >> + - channel-name
> >> +
> >> +examples:
> >> + - |
> >> + ad5940: ad5940@0 {
> >> + compatible = "adi,ad5940";
> >> + reg = <0>;
> >> + spi-max-frequency = <16000000>;
> >> + vref-supply = <&adc_vref>;
> >> + interrupt-parent = <&gpio>;
> >> + interrupts = <24 2>;
> >> +
> >> + adi,interrupt-io = <0>;
> >> +
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> +
> >> + channel@0 {
> >> + reg = <0>;
> >> + diff-channels = <AD5940_ADC_INPUTP_VCE0
> >> + AD5940_ADC_INPUTN_VBIAS0>;
> >
> > You'll need the header included for this to build. Run 'make
> > dt_binding_check' on this.
> >
> >> + channel-name = "Vce-Vbias";
> >> + };
> >> +
> >> + channel@1 {
> >> + reg = <1>;
> >> + diff-channels = <AD5940_ADC_INPUTP_VRE0
> >> + AD5940_ADC_INPUTN_VBIAS0>;
> >> + channel-name = "Vre-Vbias";
> >> + };
> >> +
> >> + channel@2 {
> >> + reg = <2>;
> >> + diff-channels = <AD5940_ADC_INPUTP_SE0
> >> + AD5940_ADC_INPUTN_VBIAS0>;
> >> + channel-name = "Vse-Vbias";
> >> + };
> >> +
> >> + channel@3 {
> >> + reg = <3>;
> >> + diff-channels = <AD5940_ADC_INPUTP_DE0
> >> + AD5940_ADC_INPUTN_VBIAS0>;
> >> + channel-name = "Vde-Vbias";
> >> + };
> >> +
> >> + channel@4 {
> >> + reg = <4>;
> >> + diff-channels = <AD5940_ADC_INPUTP_AIN0
> >> + AD5940_ADC_INPUTN_VBIAS0>;
> >> + channel-name = "ain0-Vbias";
> >> + };
> >> +
> >> + channel@5 {
> >> + reg = <5>;
> >> + diff-channels = <AD5940_ADC_INPUTP_AIN1
> >> + AD5940_ADC_INPUTN_VBIAS0>;
> >> + channel-name = "ain1-Vbias";
> >> + };
> >> +
> >> + channel@6 {
> >> + reg = <6>;
> >> + diff-channels = <AD5940_ADC_INPUTP_AIN2
> >> + AD5940_ADC_INPUTN_VBIAS0>;
> >> + channel-name = "ain2-Vbias";
> >> + };
> >> +
> >> + channel@7 {
> >> + reg = <7>;
> >> + diff-channels = <AD5940_ADC_INPUTP_AIN3
> >> + AD5940_ADC_INPUTN_VBIAS0>;
> >> + channel-name = "ain3-Vbias";
> >> + };
> >> +
> >> + channel@8 {
> >> + reg = <8>;
> >> + diff-channels = <AD5940_ADC_INPUTP_AIN4
> >> + AD5940_ADC_INPUTN_VBIAS0>;
> >> + channel-name = "ain4-Vbias";
> >> + };
> >> +
> >> + channel@9 {
> >> + reg = <9>;
> >> + diff-channels = <AD5940_ADC_INPUTP_AIN6
> >> + AD5940_ADC_INPUTN_VBIAS0>;
> >> + channel-name = "ain6-Vbias";
> >> + };
> >> +
> >> + channel@10 {
> >> + reg = <10>;
> >> + diff-channels = <AD5940_ADC_INPUTP_LPTIA_LP
> >> + AD5940_ADC_INPUTN_LPTIA>;
> >> + channel-name = "Low power TIA DC";
> >> + };
> >> +
> >> + channel@11 {
> >> + reg = <11>;
> >> + diff-channels = <AD5940_ADC_INPUTP_LPTIA
> >> + AD5940_ADC_INPUTN_LPTIA>;
> >> + channel-name = "Low power TIA AC";
> >> + };
> >> +
> >> + channel@12 {
> >> + reg = <12>;
> >> + diff-channels = <AD5940_ADC_INPUTP_HSTIA
> >> + AD5940_ADC_INPUTN_HSTIA>;
> >> + channel-name = "High Speed TIA";
> >> + };
> >> +
> >> + channel@13 {
> >> + reg = <13>;
> >> + diff-channels = <AD5940_ADC_INPUTP_TEMP
> >> + AD5940_ADC_INPUTN_VBIAS0>;
> >> + channel-name = "Temperature";
> >> + };
> >> + };
> >> diff --git a/include/dt-bindings/iio/adc/adi,ad5940.h b/include/dt-bindings/iio/adc/adi,ad5940.h
> >> new file mode 100644
> >> index 000000000000..c17826f2f654
> >> --- /dev/null
> >> +++ b/include/dt-bindings/iio/adc/adi,ad5940.h
> >> @@ -0,0 +1,52 @@
> >> +/* SPDX-License-Identifier: GPL-2.0 */
> >> +/*
> >> + * This header provides constants for configuring the AD5940 AFE
> >> + */
> >> +
> >> +#ifndef _DT_BINDINGS_IIO_ADC_AD5940_H
> >> +#define _DT_BINDINGS_IIO_ADC_AD5940_H
> >> +
> >> +#define AD5940_ADC_INPUTN_FLOATING 0
> >> +#define AD5940_ADC_INPUTN_HSTIA 1
> >> +#define AD5940_ADC_INPUTN_LPTIA 2
> >> +#define AD5940_ADC_INPUTN_AIN0 4
> >> +#define AD5940_ADC_INPUTN_AIN1 5
> >> +#define AD5940_ADC_INPUTN_AIN2 6
> >> +#define AD5940_ADC_INPUTN_AIN3 7
> >> +#define AD5940_ADC_INPUTN_VBIAS_CA8 10
> >> +#define AD5940_ADC_INPUTN_TEMP_N 11
> >> +#define AD5940_ADC_INPUTN_AIN4 12
> >> +#define AD5940_ADC_INPUTN_AIN6 14
> >> +#define AD5940_ADC_INPUTN_VZERO 16
> >> +#define AD5940_ADC_INPUTN_VBIAS0 17
> >> +#define AD5940_ADC_INPUTN_EXCITATION 20
> >> +
> >> +#define AD5940_ADC_INPUTP_FLOATING 0
> >> +#define AD5940_ADC_INPUTP_HSTIA 1
> >> +#define AD5940_ADC_INPUTP_LPTIA_LP 2
> >> +#define AD5940_ADC_INPUTP_AIN0 4
> >> +#define AD5940_ADC_INPUTP_AIN1 5
> >> +#define AD5940_ADC_INPUTP_AIN2 6
> >> +#define AD5940_ADC_INPUTP_AIN3 7
> >> +#define AD5940_ADC_INPUTP_AVDD_2 8
> >> +#define AD5940_ADC_INPUTP_DVDD_2 9
> >> +#define AD5940_ADC_INPUTP_AVDD_REG_2 10
> >> +#define AD5940_ADC_INPUTP_TEMP 11
> >> +#define AD5940_ADC_INPUTP_VBIAS_CAP 12
> >> +#define AD5940_ADC_INPUTP_DE0 13
> >> +#define AD5940_ADC_INPUTP_SE0 14
> >> +#define AD5940_ADC_INPUTP_VREF_2V5_2 16
> >> +#define AD5940_ADC_INPUTP_VREF_1V82 18
> >> +#define AD5940_ADC_INPUTP_P_TEMP_N 19
> >> +#define AD5940_ADC_INPUTP_AIN4 20
> >> +#define AD5940_ADC_INPUTP_AIN6 22
> >> +#define AD5940_ADC_INPUTP_VZERO 23
> >> +#define AD5940_ADC_INPUTP_VBIAS0 24
> >> +#define AD5940_ADC_INPUTP_VCE0 25
> >> +#define AD5940_ADC_INPUTP_VRE0 26
> >> +#define AD5940_ADC_INPUTP_VCE0_2 31
> >> +#define AD5940_ADC_INPUTP_LPTIA 33
> >> +#define AD5940_ADC_INPUTP_AGND_REF 35
> >> +#define AD5940_ADC_INPUTP_EXCITATION 36
> >> +
> >> +#endif /* _DT_BINDINGS_IIO_ADC_AD5940 */
> >> --
> >> 2.17.1
> >>
prev parent reply other threads:[~2019-11-16 14:17 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-08 13:09 [PATCH 1/3] dt-bindings: iio: adc: add support for AD5940 Song Qiang
2019-11-08 13:09 ` [PATCH 2/3] iio: adc: add driver " Song Qiang
2019-11-10 16:55 ` Jonathan Cameron
2019-11-14 14:05 ` Song Qiang
2019-11-16 14:08 ` Jonathan Cameron
2019-11-08 13:09 ` [PATCH 3/3] MAINTAINERS: add maintainer " Song Qiang
2019-11-10 16:26 ` [PATCH 1/3] dt-bindings: iio: adc: add support " Jonathan Cameron
2019-11-14 13:32 ` Song Qiang
2019-11-14 14:01 ` Ardelean, Alexandru
2019-11-16 14:15 ` Jonathan Cameron
2019-11-14 1:28 ` Rob Herring
2019-11-14 17:06 ` Song Qiang
2019-11-16 14:17 ` Jonathan Cameron [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191116141719.35a9208e@archlinux \
--to=jic23@kernel.org \
--cc=Michael.Hennerich@analog.com \
--cc=broonie@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=knaack.h@gmx.de \
--cc=lars@metafoo.de \
--cc=lgirdwood@gmail.com \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=pmeerw@pmeerw.net \
--cc=robh+dt@kernel.org \
--cc=songqiang1304521@gmail.com \
--cc=stefan.popa@analog.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.