Re: [PATCH] dt-bindings: usb: add yaml file for maxim,max3421

[Krzysztof Kozlowski <krzk@kernel.org>]

On 09/10/2025 03:15, Rodrigo Gobbi wrote:
> Convert maxim,max3421.txt to yaml format with a few extra properties like


Here and in subject, please do not use yaml at all. Look at other
commits, this is supposed to be:

dt-bindings: usb: maxim,max3421: convert to DT schema

(and all other things like "file for" are redundant")

> maxim,vbus-en-pin, maxim,gpx-pin, reset pin and supplies. Also add a

Why new properties? You must explain not only the difference but WHY you
are doing this.

WHY is the most important question/answer.

> maxim,max3421e compatible with a fallback, since the actually PN is with
> the 'e' suffix.

We don't add PNs usually, unless there is a need. So again, why?

> 
> Signed-off-by: Rodrigo Gobbi <rodrigo.gobbi.7@gmail.com>
> ---
> I've converted the txt into yaml with a few extra things as mentioned
> in the commit msg. All of them were extracted from the datasheet and also
> looking at the current state of the driver.
> 
> About the maintainer ref inside yaml, I'll quote the driver author:
> 
> Dear @David Mosberger, the binding file for this driver was not converted
> to YAML format. This patch address this. I've noticed you were the original
> driver author, so I`m "quoting" you at the maintainer ref inside yaml.
> I would appreciate your comment or suggestion over this topic.
> 
> Tks and regards to all.
> ---
>  .../devicetree/bindings/usb/maxim,3421.yaml   | 88 +++++++++++++++++++
>  .../devicetree/bindings/usb/maxim,max3421.txt | 23 -----
>  2 files changed, 88 insertions(+), 23 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/usb/maxim,3421.yaml
>  delete mode 100644 Documentation/devicetree/bindings/usb/maxim,max3421.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/maxim,3421.yaml b/Documentation/devicetree/bindings/usb/maxim,3421.yaml
> new file mode 100644
> index 000000000000..bccb22be74ff
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/maxim,3421.yaml
> @@ -0,0 +1,88 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/maxim,3421.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MAXIM MAX3421e USB Peripheral/Host Controller
> +
> +maintainers:
> +  - David Mosberger <davidm@egauge.net>
> +
> +description: |
> +  The controller provides USB2.0 compliant with Full Speed or Low Speed when in
> +  the host mode. At peripheral, it operates at Full Speed. At both cases, it
> +  uses a SPI interface.
> +  Datasheet at:
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/max3421e.pdf
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - maxim,max3421e
> +          - const: maxim,max3421
> +      - const: maxim,max3421
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  spi-max-frequency:
> +    $ref: /schemas/types.yaml#/definitions/uint32

No, drop $ref. Do you see any binding like that? No, there is none.

> +    maximum: 26000000
> +    description:
> +      SPI interface that operates up to 26MHz in Hz.

Drop description, again, look at other bindings.

> +
> +  maxim,vbus-en-pin:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    description:
> +      One of eight GPOUT pins to control external VBUS power and the polarity
> +      of the active level. It's an array of GPIO number and the active level of it.
> +    minItems: 2
> +    maxItems: 2
> +
> +  maxim,gpx-pin:

I don't understand. There is no need for this property. Drop.


> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: A property to define the behavior of the GPX pin, which is an
> +      output that may be selected in a 4-way multiplexer between OPERATE(0),
> +      VBUS_DETECT(1), BUSACT/INIRQ(2) and SOF(3) signals.
> +    enum: [0, 1, 2, 3]
> +    default: 0
> +
> +  reset-gpios:
> +    description: Active low to clear all of the internal registers except for
> +                 PINCTL (R17), USBCTL (R15), and SPI logic.
> +
> +  vdd-supply: true
> +
> +  vlogic-supply: true
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - maxim,vbus-en-pin
> +

Missing ref to spi props.

> +additionalProperties: false

unevaluatedProperties instead.


Best regards,
Krzysztof