RE: [PATCH v1 1/3] dt-bindings: usb: Add HPE GXP UDCG Controller

[Yu, Richard]

>> +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.

Here we are working with virtual devices that are created and have no
physical presence. We have modeled our code off of ASPEED's VHUB
implementation to comply with the implementation in OpenBMC.

>> + 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.

We will remove references to EHCI in code and documentation. It has been
modeled to following ASPEEDs approach as mentioned above.

>> + 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?

That is correct. We can configure this VHUB Controller to have one to
8 virtual ports. This is similar to the aspeed virtual USB HUB
"aspeed,vhub-downstream-ports" moving forward in the next patch
we are going to use "hpe,vhub-downstream-ports"

>> + $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.

We intend to change this to "hpe,vhub-generic-endpoints" following
ASPEED's "aspeed,vhub-generic-endpoints"

>> +examples:
>> + - |
>> + udcg@80400800 {


> Node names should be generic. See also an explanation and list of
...
We will use usb-hub.

Thank you for your feedback,
Richard Yu