Add documentation for the ESP32S3 ACM controller.
Signed-off-by: Max Filippov <[email protected]>
---
.../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 <[email protected]>
+
+description: |
+ ESP32S3 ACM controller is a communication device found in the ESP32S3
+ SoC that is connected to one of its USB controllers.
+
+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";
+ reg = <0x60038000 0x1000>;
+ interrupts = <96 3 0>;
+ };
--
2.30.2
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.
>
> Signed-off-by: Max Filippov <[email protected]>
> ---
> .../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 <[email protected]>
> +
> +description: |
Do not need '|' unless you need to preserve formatting.
> + ESP32S3 ACM controller is a communication device found in the ESP32S3
What is "ACM"? Why is this in serial? Only serial controllers are in
serial. The description is very vague, way too vague.
> + 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.
> + reg = <0x60038000 0x1000>;
> + interrupts = <96 3 0>;
Same comments as previous patch.
> + };
Best regards,
Krzysztof
On Wed, Sep 13, 2023 at 10:57 PM Krzysztof Kozlowski
<[email protected]> 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 <[email protected]>
> > ---
> > .../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 <[email protected]>
> > +
> > +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
On 14/09/2023 22:47, Max Filippov wrote:
> On Wed, Sep 13, 2023 at 10:57 PM Krzysztof Kozlowski
> <[email protected]> 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 <[email protected]>
>>> ---
>>> .../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 <[email protected]>
>>> +
>>> +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.
Yes.
>
>>> + 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 {
So this must be named "serial" now. ACM describes how this is interfaces
to the SoC, right? Otherwise it would not be in "serial" directory and
you would not be able to put serial devices as children.
>>> + 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?
Yes, if 0 is not an IRQ flag :)
>
Best regards,
Krzysztof