On Wed, Sep 13, 2023 at 10:57 PM Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote: > > On 13/09/2023 23:14, Max Filippov wrote: > > Add documentation for the ESP32S3 ACM controller. > > A nit, subject: drop second/last, redundant "bindings". The > "dt-bindings" prefix is already stating that these are bindings. Ok. > > Signed-off-by: Max Filippov <jcmvbkbc@xxxxxxxxx> > > --- > > .../bindings/serial/esp,esp32-acm.yaml | 40 +++++++++++++++++++ > > 1 file changed, 40 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml > > > > diff --git a/Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml b/Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml > > new file mode 100644 > > index 000000000000..dafbae38aa64 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/serial/esp,esp32-acm.yaml > > @@ -0,0 +1,40 @@ > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > + > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/serial/esp,esp32-acm.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: ESP32S3 ACM controller > > + > > +maintainers: > > + - Max Filippov <jcmvbkbc@xxxxxxxxx> > > + > > +description: | > > Do not need '|' unless you need to preserve formatting. Ok. > > + ESP32S3 ACM controller is a communication device found in the ESP32S3 > > What is "ACM"? It's an 'Abstract Control Model' as in USB CDC-ACM: 'Communication Device Class - Abstract Control Model'. > Why is this in serial? Only serial controllers are in serial. Because it's a serial communication device. The SoC TRM calls this peripheral 'USB Serial', but the USB part is fixed and is not controllable on the SoC side. When you plug it into a host USB socket you get a serial port called ttyACM on the host. > The description is very vague, way too vague. Is the following better? Fixed function USB CDC-ACM device controller of the Espressif ESP32S3 SoC. > > + SoC that is connected to one of its USB controllers. > > Same comments as previous patch. > > > + > > +properties: > > + compatible: > > + const: esp,esp32s3-acm > > + > > + reg: > > + maxItems: 1 > > + > > + interrupts: > > + maxItems: 1 > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + acm@60038000 { > > + compatible = "esp,esp32s3-acm"; > > Use 4 spaces for example indentation. Ok. > > + reg = <0x60038000 0x1000>; > > + interrupts = <96 3 0>; > > Same comments as previous patch. These are not IRQ flags. In any case the contents of the IRQ specification cells is not relevant here, right? -- Thanks. -- Max
