On Sat, 30 Mar 2024 04:01:26 +0800 wefu@xxxxxxxxxx wrote: > From: Wei Fu <wefu@xxxxxxxxxx> > > Document devicetree bindings for the XuanTie TH1520 AP sub-system ADC. > > Signed-off-by: Wei Fu <wefu@xxxxxxxxxx> Hi. Few comments inline. Given Fugang Duan is listed as a maintainer I'd also like an ack ideally from them. BTW your +CC list seems a bit random. Thanks, Jonathan > --- > .../bindings/iio/adc/thead,th1520.yaml | 52 +++++++++++++++++++ > 1 file changed, 52 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iio/adc/thead,th1520.yaml b/Documentation/devicetree/bindings/iio/adc/thead,th1520.yaml > new file mode 100644 > index 000000000000..80890ce62810 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/thead,th1520.yaml > @@ -0,0 +1,52 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/adc/thead,th1520.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: XuanTie TH1520 Analog to Digital Converter (ADC) > + > +maintainers: > + - Fugang Duan <duanfugang.dfg@xxxxxxxxxxxxxxxxx> > + - Wei Fu <wefu@xxxxxxxxxx> > + > +description: | > + 12-Bit Analog to Digital Converter (ADC) on XuanTie TH1520 > + > +properties: > + compatible: > + const: thead,th1520 See the example - doesn't match. Should be -adc here as well as this is just part of the SoC. > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + clocks: > + maxItems: 1 > + > + clock-names: > + const: adc No objection to having names, but if there is only 1 why is it required? > + > +required: > + - compatible > + - reg > + - interrupts > + - clocks > + - clock-names > + - status Status doesn't need to be here > + > +additionalProperties: false > + > +examples: > + - | > + adc: adc@0xfffff51000 { > + compatible = "thead,th1520-adc"; As the bot pointed out, spaces not tabs for these - Please run the suggested tests in that email before sending a v2. > + reg = <0xff 0xfff51000 0x0 0x1000>; > + interrupts = <61 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&aonsys_clk>; > + clock-names = "adc"; > + /* ADC pin is proprietary,no need to config pinctrl */ > + status = "disabled"; No need for a status entry in the example. > + };
