Re: [PATCH v2 1/2] dt-bindings: usb: Add documentation for AM62 USB Wrapper module

[Aswath Govindraju <a-govindraju@ti.com>]

Hi Krzysztof,

On 24/03/22 16:37, Krzysztof Kozlowski wrote:
> On 24/03/2022 08:34, Aswath Govindraju wrote:
>> Add bindings for the TI's AM62 wrapper module for the Synopsys USBSS-DRD
>> controller.
>>
>> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
>> ---
>>
>> Changes since v1:
>> - made correction in grammer of clocks property description
>>   and added maxItems in the interrupts property based on comments
>>   received from Roger
>> - corrected the title, fixed the description of
>>   ti,syscon-phy-pll-refclk, added pattern properties and child node
>>   in the example based on the comments from Krzysztof.
>>
>>  .../devicetree/bindings/usb/ti,am62-usb.yaml  | 117 ++++++++++++++++++
>>  1 file changed, 117 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/usb/ti,am62-usb.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/usb/ti,am62-usb.yaml b/Documentation/devicetree/bindings/usb/ti,am62-usb.yaml
>> new file mode 100644
>> index 000000000000..452bfdc6fb09
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/ti,am62-usb.yaml
>> @@ -0,0 +1,117 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/usb/ti,am62-usb.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: TI's AM62 wrapper module for the Synopsys USBSS-DRD controller
>> +
>> +maintainers:
>> +  - Aswath Govindraju <a-govindraju@ti.com>
>> +
>> +properties:
>> +  compatible:
>> +    const: ti,am62-usb
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  ranges: true
>> +
>> +  power-domains:
>> +    description:
>> +      PM domain provider node and an args specifier containing
>> +      the USB ISO device id value. See,
>> +      Documentation/devicetree/bindings/soc/ti/sci-pm-domain.yaml
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    description: Clock phandle to usb2_refclk
>> +    maxItems: 1
>> +
>> +  clock-names:
>> +    items:
>> +      - const: ref
>> +
>> +  id-gpio:
>> +    description:
>> +      GPIO to be used as ID pin
>> +    maxItems: 1
> 
> I have doubts about this. If you USB controller handles the ID pin, then
> probably this should be moved to usb-connector.yaml. I did not see
> id-gpio in any other USB controller blocks.
> 

Yes, the USB wrapper handles the ID pin operation only. It also reads
the status of VBUS by reading a register from its MMR and not using a
gpio. After evaluating the role the based on the states if id pin and
VBUS, this role is communicated to the dwc3 core driver using extcon.
There is no way for the dwc3 driver to detect the role on its own.


The usb-connector(drivers/usb/common/usb-conn-gpio.c) driver, seems to
be implemented for driving the VBUS, based on ID and VBUS pin status.
However, in case of the above implementation we need to communicate the
detected role to the dwc3 core driver. Also, the wrapper does not
control VBUS but it is the dwc3 core driver that drives the VBUS.
Therefore, I think the usb-connector implementation cannot be used here.

Thanks,
Aswath

> 
> Best regards,
> Krzysztof