On Mon, May 06, 2024 at 08:51:59AM GMT, Krzysztof Kozlowski wrote: > On 05/05/2024 03:52, Inochi Amaoto wrote: > > The USB phy of Sophgo CV18XX series SoC needs to sense a pin called > > "VBUS_DET" to get the right operation mode. If this pin is not > > connected, it only supports setting the mode manually. > > > > Add USB phy bindings for Sophgo CV18XX/SG200X series SoC. > > ... > > > + > > + clock-names: > > + items: > > + - const: phy > > + - const: app > > + - const: stb > > + - const: lpm > > + > > + vbus_det-gpios: > > No underscores. > Thanks. > > + description: GPIO to the USB OTG VBUS detect pin. This should not be > > + defined if vbus_det pin and switch pin are connected, which may > > + break the VBUS detection. > > Why is this property of the PHY? VBUS pin goes to the connector, doesn't > it? It looks like you combined two or three (!!!) bindings into one. > Yes, but I am not sure which is the best to write this bindings. The topology of USB likes this: controller -- phy -- switch --> (host) port/hub --> (device) port The vbus-detect connect to the device port, but it will change the mode for both phy and switch. And the switch is just a switching circuit. I am pretty confused on how to split this binding. I think it may like the following: phy { switch { /* This is the switch in the follows */ connector1 { /* host port */ }; connector2 { /* device port*/ /* the vbus pin is here */ }; }; }; Could you share some suggestion on this? > > + maxItems: 1 > > + > > + sophgo,switch-gpios: > > + description: GPIO array for the phy to control connected switch. For > > Switch? This is a binding of the phy, not switch... > You are right, I think this switch may be more like a pattern device. > > + host mode, the driver will set these GPIOs to low one by one. For > > Yeah, you mention driver which further confirms this is a property for > driver, not hardware. > > Describe your hardware, not driver behavior. OK, I will remove this. > > > > + device mode, the driver will set these GPIOs to high in reverse > > + order. For a reference design, see item description. > > + minItems: 1 > > + items: > > + - description: USB switch operation mode > > + - description: USB switch host power control > > + > > +required: > > + - compatible > > + - "#phy-cells" > > + - clocks > > + - clock-names > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + phy@48 { > > + compatible = "sophgo,cv1800-usb-phy"; > > + reg = <0x48 0x4>; > > + #phy-cells = <0>; > > + clocks = <&clk 92>, <&clk 93>, > > + <&clk 94>, <&clk 95>; > > + clock-names = "phy", "app", "stb", "lpm"; > > Make the example complete. > > > > Best regards, > Krzysztof >