| Message ID | 20241226055313.2841977-2-j2anfernee@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: adc: add Nuvoton NCT7201 ADC driver | expand |
On Thu, Dec 26, 2024 at 01:53:12PM +0800, Eason Yang wrote: > Adds a binding specification for the Nuvoton NCT7201/NCT7202 I gave you link to exact line with exact text to use. Read it again and use it, instead inventing your own wording. The documentation does not say "Adds" but explicitly asks you to say "Add". Why using different? Subject: nothing improved. > > Signed-off-by: Eason Yang <j2anfernee@gmail.com> > --- > .../bindings/iio/adc/nuvoton,nct7201.yaml | 49 +++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 50 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml > > diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml > new file mode 100644 > index 000000000000..08b52258e4af > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml > @@ -0,0 +1,49 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/adc/nuvoton,nct7201.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Nuvoton nct7201 and similar ADCs > + > +maintainers: > + - Eason Yang <j2anfernee@gmail.com> > + > +description: | > + Family of ADCs with i2c interface. > + > +properties: > + compatible: > + enum: > + - nuvoton,nct7201 > + - nuvoton,nct7202 Devices aren't compatible? Explain in the commit msg why they aren't or use proper compatibility (oneOf, see numerous other bindings or example-schema). > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + reset-gpios: > + description: > + Reset pin for the device. Drop description, obvious. > + maxItems: 1 > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + adc@1d { > + compatible = "nuvoton,nct7202"; > + reg = <0x1d>; Make the example complete: add interrupts and reset-gpios. Best regards, Krzysztof
Dear Krzysztof, Thanks for your comments. Krzysztof Kozlowski <krzk@kernel.org> 於 2024年12月27日 週五 下午4:17寫道： > > On Thu, Dec 26, 2024 at 01:53:12PM +0800, Eason Yang wrote: > > Adds a binding specification for the Nuvoton NCT7201/NCT7202 > > > I gave you link to exact line with exact text to use. Read it again and > use it, instead inventing your own wording. The documentation does not > say "Adds" but explicitly asks you to say "Add". Why using different? > > Subject: nothing improved. > > > > > Signed-off-by: Eason Yang <j2anfernee@gmail.com> > > --- > > .../bindings/iio/adc/nuvoton,nct7201.yaml | 49 +++++++++++++++++++ > > MAINTAINERS | 1 + > > 2 files changed, 50 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml > > > > diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml > > new file mode 100644 > > index 000000000000..08b52258e4af > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml > > @@ -0,0 +1,49 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/iio/adc/nuvoton,nct7201.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Nuvoton nct7201 and similar ADCs > > + > > +maintainers: > > + - Eason Yang <j2anfernee@gmail.com> > > + > > +description: | > > + Family of ADCs with i2c interface. > > + > > +properties: > > + compatible: > > + enum: > > + - nuvoton,nct7201 > > + - nuvoton,nct7202 > > Devices aren't compatible? Explain in the commit msg why they aren't or > use proper compatibility (oneOf, see numerous other bindings or example-schema). > > + compatible: - enum: - - nuvoton,nct7201 - - nuvoton,nct7202 + oneOf: + - const: nuvoton,nct7201 + - const: nuvoton,nct7202 > > + > > + reg: > > + maxItems: 1 > > + > > + interrupts: > > + maxItems: 1 > > + > > + reset-gpios: > > + description: > > + Reset pin for the device. > > Drop description, obvious. > > > + maxItems: 1 > > + > > +required: > > + - compatible > > + - reg > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + i2c { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + adc@1d { > > + compatible = "nuvoton,nct7202"; > > + reg = <0x1d>; > > > Make the example complete: add interrupts and reset-gpios. > Add interrupts and reset-gpios example, + #include <dt-bindings/gpio/gpio.h> i2c { #address-cells = <1>; #size-cells = <0>; adc@1d { compatible = "nuvoton,nct7202"; reg = <0x1d>; + interrupt-parent = <&gpio3>; + interrupts = <30 IRQ_TYPE_LEVEL_LOW>; + reset-gpios = <&gpio3 28 GPIO_ACTIVE_LOW>; }; > Best regards, > Krzysztof >
On 06/01/2025 08:22, Yu-Hsian Yang wrote: >>> --- >>> .../bindings/iio/adc/nuvoton,nct7201.yaml | 49 +++++++++++++++++++ >>> MAINTAINERS | 1 + >>> 2 files changed, 50 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml >>> new file mode 100644 >>> index 000000000000..08b52258e4af >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml >>> @@ -0,0 +1,49 @@ >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/iio/adc/nuvoton,nct7201.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Nuvoton nct7201 and similar ADCs >>> + >>> +maintainers: >>> + - Eason Yang <j2anfernee@gmail.com> >>> + >>> +description: | >>> + Family of ADCs with i2c interface. >>> + >>> +properties: >>> + compatible: >>> + enum: >>> + - nuvoton,nct7201 >>> + - nuvoton,nct7202 >> >> Devices aren't compatible? Explain in the commit msg why they aren't or >> use proper compatibility (oneOf, see numerous other bindings or example-schema). >> >> > > + compatible: > - enum: > - - nuvoton,nct7201 > - - nuvoton,nct7202 > + oneOf: > + - const: nuvoton,nct7201 > + - const: nuvoton,nct7202 Nothing improved. I referenced 'oneOf' for the case of compatibility. Don't use it to code enum in different way. Address the comment and questions. You did not respond to several comments, so I assume you are going to implement/fix all of pointed out things. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml new file mode 100644 index 000000000000..08b52258e4af --- /dev/null +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml @@ -0,0 +1,49 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/adc/nuvoton,nct7201.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Nuvoton nct7201 and similar ADCs + +maintainers: + - Eason Yang <j2anfernee@gmail.com> + +description: | + Family of ADCs with i2c interface. + +properties: + compatible: + enum: + - nuvoton,nct7201 + - nuvoton,nct7202 + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + reset-gpios: + description: + Reset pin for the device. + maxItems: 1 + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + adc@1d { + compatible = "nuvoton,nct7202"; + reg = <0x1d>; + }; + }; +... diff --git a/MAINTAINERS b/MAINTAINERS index 910305c11e8a..9c5328cbef17 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2792,6 +2792,7 @@ L: openbmc@lists.ozlabs.org (moderated for non-subscribers) S: Supported F: Documentation/devicetree/bindings/*/*/*npcm* F: Documentation/devicetree/bindings/*/*npcm* +F: Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml F: Documentation/devicetree/bindings/rtc/nuvoton,nct3018y.yaml F: arch/arm/boot/dts/nuvoton/nuvoton-npcm* F: arch/arm/mach-npcm/
Adds a binding specification for the Nuvoton NCT7201/NCT7202 Signed-off-by: Eason Yang <j2anfernee@gmail.com> --- .../bindings/iio/adc/nuvoton,nct7201.yaml | 49 +++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 50 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml