Messages in this thread | | | From | Max Filippov <> | Date | Thu, 14 Sep 2023 13:47:13 -0700 | Subject | Re: [PATCH 3/4] dt-bindings: serial: document esp32s3-acm bindings |
| |
On Wed, Sep 13, 2023 at 10:57 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 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@gmail.com> > > --- > > .../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@gmail.com> > > + > > +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
| |