| Message ID | 20241203091540.3695650-2-j2anfernee@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: adc: add Nuvoton NCT720x ADC driver | expand |
On 03/12/2024 10:15, Eason Yang wrote: > This adds a binding specification for the Nuvoton NCT7201/NCT7202 Please do not use "This commit/patch/change", but imperative mood. See longer explanation here: https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95 A nit, subject: drop second/last, redundant "bindings". The "dt-bindings" prefix is already stating that these are bindings. See also: https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 > > Signed-off-by: Eason Yang <j2anfernee@gmail.com> > --- > .../bindings/iio/adc/nuvoton,nct720x.yaml | 40 +++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 41 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > new file mode 100644 > index 000000000000..2ed1e15b953b > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > @@ -0,0 +1,40 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/adc/nuvoton,nct720x.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Nuvoton nct7202 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 No other properties? No resources? I think you skipped quite a lot from previous review. > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + nct7202@1d { Nothing improved here. <form letter> This is a friendly reminder during the review process. It seems my or other reviewer's previous comments were not fully addressed. Maybe the feedback got lost between the quotes, maybe you just forgot to apply it. Please go back to the previous discussion and either implement all requested changes or keep discussing them. Thank you. </form letter> Best regards, Krzysztof
Dear Krzysztof Kozlowski, Thank you for your kind feedback. Krzysztof Kozlowski <krzk@kernel.org> 於 2024年12月3日 週二 下午5:25寫道： > > On 03/12/2024 10:15, Eason Yang wrote: > > This adds a binding specification for the Nuvoton NCT7201/NCT7202 > > > Please do not use "This commit/patch/change", but imperative mood. See > longer explanation here: > https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95 > > A nit, subject: drop second/last, redundant "bindings". The > "dt-bindings" prefix is already stating that these are bindings. > See also: > https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 > I read the submit patch rule and understand how to rewrite it. > > > > Signed-off-by: Eason Yang <j2anfernee@gmail.com> > > --- > > .../bindings/iio/adc/nuvoton,nct720x.yaml | 40 +++++++++++++++++++ > > MAINTAINERS | 1 + > > 2 files changed, 41 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > > > diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > new file mode 100644 > > index 000000000000..2ed1e15b953b > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml > > @@ -0,0 +1,40 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/iio/adc/nuvoton,nct720x.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Nuvoton nct7202 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 > > > No other properties? No resources? > The difference is to remove read-vin-data-size property and default use read word vin data. > I think you skipped quite a lot from previous review. > > > + > > +required: > > + - compatible > > + - reg > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + i2c { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + nct7202@1d { > > Nothing improved here. > > <form letter> > This is a friendly reminder during the review process. > > It seems my or other reviewer's previous comments were not fully > addressed. Maybe the feedback got lost between the quotes, maybe you > just forgot to apply it. Please go back to the previous discussion and > either implement all requested changes or keep discussing them. > > Thank you. > </form letter> > Thanks for your friendly reminder. It's impolite not to reply to every reviewer's comment. I would keep discussing with reviewers and apply the changes in the next version. > > Best regards, > Krzysztof
On 04/12/2024 04:10, Yu-Hsian Yang wrote: >>> + reg: >>> + maxItems: 1 >> >> >> No other properties? No resources? >> > > The difference is to remove read-vin-data-size property and default > use read word vin data. No supplies? No interrupts? Best regards, Krzysztof
Dear Krzysztof Kozlowski, Krzysztof Kozlowski <krzk@kernel.org> 於 2024年12月4日 週三 下午3:13寫道： > > On 04/12/2024 04:10, Yu-Hsian Yang wrote: > >>> + reg: > >>> + maxItems: 1 > >> > >> > >> No other properties? No resources? > >> > > > > The difference is to remove read-vin-data-size property and default > > use read word vin data. > > > No supplies? No interrupts? > We would add interrupts and reset-gpios but not include in required block. Add these two properties in todo list. + interrupts: + maxItems: 1 + reset-gpios: + description: + Reset pin for the device. + maxItems: 1 Besides, I found a mistake that the Node name should follow. > + nct7202@1d { So, correct it as + adc@1d { > > Best regards, > Krzysztof
diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml new file mode 100644 index 000000000000..2ed1e15b953b --- /dev/null +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml @@ -0,0 +1,40 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/adc/nuvoton,nct720x.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Nuvoton nct7202 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 + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + nct7202@1d { + compatible = "nuvoton,nct7202"; + reg = <0x1d>; + }; + }; diff --git a/MAINTAINERS b/MAINTAINERS index 1e930c7a58b1..bea10a846475 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,nct720x.yaml F: Documentation/devicetree/bindings/rtc/nuvoton,nct3018y.yaml F: arch/arm/boot/dts/nuvoton/nuvoton-npcm* F: arch/arm/mach-npcm/
This adds a binding specification for the Nuvoton NCT7201/NCT7202 Signed-off-by: Eason Yang <j2anfernee@gmail.com> --- .../bindings/iio/adc/nuvoton,nct720x.yaml | 40 +++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 41 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,nct720x.yaml