On 06/07/2023 23:59, richard.yu@xxxxxxx wrote: > From: Richard Yu <richard.yu@xxxxxxx> > > Provide access to the two register regions for GXP Virtual EHCI > controller through the hpe,gxp-udcg binding. Thank you for your patch. There is something to discuss/improve. > > Signed-off-by: Richard Yu <richard.yu@xxxxxxx> > --- > .../devicetree/bindings/usb/hpe,gxp-udcg.yaml | 70 +++++++++++++++++++ > 1 file changed, 70 insertions(+) > create mode 100644 Documentation/devicetree/bindings/usb/hpe,gxp-udcg.yaml > > diff --git a/Documentation/devicetree/bindings/usb/hpe,gxp-udcg.yaml b/Documentation/devicetree/bindings/usb/hpe,gxp-udcg.yaml > new file mode 100644 > index 000000000000..e6746374f97d > --- /dev/null > +++ b/Documentation/devicetree/bindings/usb/hpe,gxp-udcg.yaml > @@ -0,0 +1,70 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/usb/hpe,gxp-udcg.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: HPE GXP USB Virtual EHCI controller The word "virtual" in bindings pretty often raises questions, because we describe usually real hardware, not virtual. Some explanation in description would be useful. > + > +maintainers: > + - Nick Hawkins <nick.hawkins@xxxxxxx> > + - Richard Yu <richard.yu@xxxxxxx> > + > +description: |+ Drop |+ > + The HPE GXP USB Virtual EHCI Controller implements 1 set of USB EHCI > + register and several sets of device and endpoint registers to support > + the virtual EHCI's downstream USB devices. > + If this is EHCI controller, then I would expect here reference to usb-hcd. > +properties: > + compatible: > + enum: > + - hpe,gxp-udcg > + > + reg: > + items: > + - description: UDC Global (UDCG) config controller > + - description: UDC Invidual config/interrupt controllers > + > + reg-names: > + items: > + - const: udcg > + - const: udc > + > + interrupts: > + maxItems: 1 > + > + hpe,vehci-downstream-ports: > + description: Number of downstream ports supported by the GXP Why do you need this property in DT and what exactly does it represent? You have one device - EHCI controller - and on some boards it is further customized? Even though it is the same device? > + $ref: /schemas/types.yaml#/definitions/uint32 > + default: 4 > + minimum: 1 > + maximum: 8 > + > + hpe,vehci-generic-endpoints: > + description: Number of generic endpoints supported by the GXP > + $ref: /schemas/types.yaml#/definitions/uint32 Same concerns. > + default: 16 > + minimum: 1 > + maximum: 16 > + > +required: > + - compatible > + - reg > + - reg-names > + - interrupts > + - hpe,vehci-downstream-ports > + - hpe,vehci-generic-endpoints > + > +additionalProperties: false > + > +examples: > + - | > + udcg@80400800 { Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation Best regards, Krzysztof
