Re: [PATCH 1/2] dt-bindings: phy: spacemit: add regulator support to K1 USB2 PHY

[Krzysztof Kozlowski <krzk@kernel.org>]

On 07/02/2026 13:30, Ze Huang wrote:
> On Sat Feb 7, 2026 at 6:48 PM CST, Krzysztof Kozlowski wrote:
>> On Fri, Feb 06, 2026 at 06:00:08PM +0800, Chukun Pan wrote:
>>> Add an optional phy-supply property to describe the regulator
>>> supplying for USB VBUS.
>>
>> Why wasn't it there before? USB did not have VBUS?
> 
> Previously, the VBUS regulator was defined as regulator-always-on in
> the device tree as a workaround.

Bindings are supposed to be complete regardless what driver does (or
does not).

You must list all supplies with proper names.

If this regulator was missing, then what else?

> 
>>
>> Explanation is so incomplete I suspect you are patching broken things,
>> so as well this could be completely different hardware (e.g. there is no
>> regulator for this block but e.g. connector).
>>
> 
> usb ports under usb hub node can describe the topology well, but
> still regulator always-on is necessary as no driver toggles it.
> 
>     usb3 {
>      dr_mode = "host";
>      #address-cells = <1>;
>      #size-cells = <0>;
>      status = "okay";
> 
>      hub_2_0: hub@1 {
>       compatible = "usb2109,2817";
>       reg = <0x1>;
>       #address-cells = <1>;
>       #size-cells = <0>;
>       vdd-supply = <&usb3_vhub>;
>       peer-hub = <&hub_3_0>;
>       reset-gpios = <&gpio K1_GPIO(124) GPIO_ACTIVE_LOW>;
> 
>       port@1 {
>        reg = <1>;
>        vbus-supply = <&usb3_vbus>;
>       };
> 
>       port@2 {
>        reg = <2>;
>        vbus-supply = <&usb3_vbus>;
>       };
> 
>       ...
> 
>       hub_3_0: hub@2 {
>         ...
>       };
>     };
> 
> Here is the diagram for the USB2, USB3 controller on K1 Jupiter board [1] (page 21)
> 
>     +-----------------------+
>     |        K1 SoC         |
>     |                       |
>     |  +-----------------+  |   (USB 3.0)
>     |  | USB3 Controller |--|---------------------------------->+--------------------------+
>     |  +-----------------+  |                                   |        VL817 Hub         |
>     |                       |                                   |                          |---> [USB3 Ports]
>     |      HUB_PWREN        |                                   | +----------------------+ |     ^
>     |       (GPIO)  --------|---------------------------------->| | Enable Chip Power    | |     |
>     |                       |                                   | +----------------------+ |     |
>     |                       |                                   +--------------------------+     |
>     |      USB3_PWREN       |                                                                    |
>     |       (GPIO)   -------|----------------------------------------------------------> [VBUS of USB3 Ports]
>     |                       |
>     |                       |
>     |~~~~~~~~~~~~~~~~~~~~~~~|~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     |                       |
>     |  +-----------------+  |   (USB 2.0)
>     |  | USB2 Controller |--|---------------------------------->+--------------------------+
>     |  +-----------------+  |                                   |      FE1_1S Hub          |---> [USB2 Ports]
>     |                       |                                   +--------------------------+     ^
>     |   USB_HOST_PWREN_H    |                                                                    |
>     |       (GPIO)      ----|-----------------------------------------------------------> [VBUS of USB2 Ports]
>     |                       |
>     |                       |
>     +-----------------------+


I do not see here a "phy" regulator supply. Are you sure this is how
your pin or supply is called?

> 
> [1] https://github.com/milkv-jupiter/jupiter-files/blob/main/hardware/v1_0/jupiter-sch-v1_0.pdf

This page shows the connector, so not the phy.

> 
>>
>>>
>>> Signed-off-by: Chukun Pan <amadeus@jmu.edu.cn>
>>> ---
>>>  Documentation/devicetree/bindings/phy/spacemit,usb2-phy.yaml | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/phy/spacemit,usb2-phy.yaml b/Documentation/devicetree/bindings/phy/spacemit,usb2-phy.yaml
>>> index 43eaca90d88c..74a1cd5bcdbe 100644
>>> --- a/Documentation/devicetree/bindings/phy/spacemit,usb2-phy.yaml
>>> +++ b/Documentation/devicetree/bindings/phy/spacemit,usb2-phy.yaml
>>> @@ -19,6 +19,10 @@ properties:
>>>    clocks:
>>>      maxItems: 1
>>>  
>>> +  phy-supply:
>>> +    description:
>>> +      Phandle to a regulator that provides power to VBUS.
>>
> 
> "vbus-supply" should be more accurate.
> 
>> Drop redundant part. This cannot be anything else than phandle and
>> regulator.
>>
>> "VBUS power supply" for example.
>>
> 
>> But anyway, I don't have certainty that
>> this is correct hardware representation. It's your task to provide that.
>>
>> Best regards,
>> Krzysztof
> 
> I initially considered handling VBUS via the onboard_usb_dev driver, as
> we discussed previously for the BananaPi-F3 board (which uses a VL817
> Hub) [2]. I was waiting for Marco Felsch's patch series "onboard-dev USB
> hub host managed vbus" to land [3].
> 
> [2] https://lore.kernel.org/linux-riscv/aWJAT3n_KcND8bOz@monica.localdomain/
> [3] https://lore.kernel.org/all/20250911-v6-16-topic-usb-onboard-dev-v4-0-1af288125d74@pengutronix.de/
> 
> I am not certain if managing VBUS in the PHY driver (e.g.,
> phy-stm32-usbphyc.c and phy-rockchip-usb.c) is the standard method.
> I would not insist on the "onboard_usb_dev managed vbus" if this patch
> (PHY managed) is considered proper way.

I am not talking about driver here at all.

Please start describing in the bindings your complete hardware, not your
drivers.

That's a no for this patch, because phy does not have such supply.

Best regards,
Krzysztof