Re: [PATCH v2 5/9] dt-bindings: usb: dwc3: Document ST STM32MP2 DWC3 xHCI USB controller
Marek Vasut <[email protected]>
| Newsgroups | org.kernel.vger.linux-kernel,org.infradead.lists.linux-arm-kernel,org.infradead.lists.linux-phy,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-usb |
|---|---|
| Message-ID | <[email protected]> |
On 8/18/26 10:15 AM, Krzysztof Kozlowski wrote:
> On Sun, Aug 16, 2026 at 11:37:07PM +0200, Marek Vasut wrote:
>> +properties:
>> + compatible:
>> + const: st,stm32mp25-dwc3
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + access-controllers:
>> + maxItems: 1
>> +
>> + clocks:
>> + minItems: 3
>> + maxItems: 3
>> +
>> + clock-names:
>> + items:
>> + - const: ref
>> + - const: bus_early
>> + - const: suspend
>> +
>> + dr_mode:
>> + $ref: /schemas/types.yaml#/definitions/string
>> + enum: [host, peripheral, otg]
>> +
>> + interrupts:
>> + maxItems: 1
>> +
>> + phys:
>> + minItems: 1
>> + maxItems: 2
>> +
>> + phy-names:
>> + minItems: 1
>> + items:
>> + - const: usb2-phy
>> + - const: usb3-phy
>> +
>> + resets:
>> + minItems: 1
>
> Hm? You keep coming with some odd style, not present in any other files.
> Where do you see such code - property followed by minItems alone? This
> applies to other places as well.
This should clearly be maxItems: 1, fixed, thanks .
>> +
>> + st,syscfg:
>> + $ref: /schemas/types.yaml#/definitions/phandle-array
>> + description: Phandle to system configuration controller.
>> + items:
>> + - items:
>> + - description: phandle to syscfg
>> + - description: USB3DR control offset within syscfg
>> +
>> + st,enable-port-power-control:
>> + type: boolean
>> + description: Enable Host-Mode Port Power Control (bit-3 of capability param HCCPARAMS)
>
> Why wouldn't this be enavled always? Why is this a board-level property?
A board can have external USB power controller chip like TCPP02/TCPP03
and the DWC3 IP does not control the port power directly. This seems to
be common on the STM32MP2 . Hence this property, which disables the port
power control functionality in DWC3 IP and lets the chip do it instead.
>> +
>> + st,ovrcur-active-low:
>
> Don't re-invent stuff:
> st,over-current-active-low
It seems I can even use generic "over-current-active-low" .
>> + type: boolean
>> + description: Over-Current signal polarity is active-low
>> +
>> + st,vbusen-active-low:
>> + type: boolean
>> + description: VBUS-ENABLE signal polarity is active-low
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - clocks
>> + - clock-names
>> + - interrupts
>> + - phys
>> + - phy-names
>> + - resets
>> + - st,syscfg
>> +
>> +unevaluatedProperties: false
>
> So where did you reference any other schema - for properties here and
> for this unevaluatedProps?
I seem to be getting this one wrong all the time, so let me ask -- when
do I use unevaluatedProperties:false and when additionalProperties:false
, what is the rule of thumb here ?
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/clock/st,stm32mp25-rcc.h>
>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>> + #include <dt-bindings/reset/st,stm32mp25-rcc.h>
>> +
>> + usb3dr: usb@48300000 {
>
> Drop unused label
Done, thanks.