Messages in this thread | | | Date | Sat, 30 Mar 2024 16:14:46 +0000 | From | Jonathan Cameron <> | Subject | Re: [PATCH 3/3] dt-bindings: adc: Document XuanTie TH1520 ADC |
| |
On Sat, 30 Mar 2024 04:01:26 +0800 wefu@redhat.com wrote:
> From: Wei Fu <wefu@redhat.com> > > Document devicetree bindings for the XuanTie TH1520 AP sub-system ADC. > > Signed-off-by: Wei Fu <wefu@redhat.com>
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@linux.alibaba.com> > + - Wei Fu <wefu@redhat.com> > + > +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.
> + };
| |