Re: [PATCH v3 4/5] dt-bindings: phy: realtek: Add the doc about the Realtek SoC USB 2.0 PHY

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 08/06/2023 10:21, Stanley Chang[昌育德] wrote:
> 
>>> Maybe I use the word "control power domain" is not well, I just want to
>> control the ldo of usb phy.
>>> Revised:
>>> The phandle of syscon used to control the ldo of USB PHY.
>>
>> Isn't this still a power domain?
> 
> I only control a register, it is not needed a driver of power domain.

Aren't many power domains just a registers? What about other drivers?
Don't you want in other driver control LDO of something else? And in
other something else?

> 
> 
>>>
>>>>> +
>>>>> +patternProperties:
>>>>> +  "^phy@[0-3]+$":
>>>>> +    type: object
>>>>> +    description:
>>>>> +      Each sub-node is a PHY device for one XHCI controller.
>>>>
>>>> I don't think it is true. You claim above that you have 0 as
>>>> phy-cells, means you have one phy. Here you say you can have up to 4 phys.
>>>
>>> I mean the driver can support up to 4 phys.
>>
>> What driver can or cannot do, does not matter. This is about hardware.
>>
>>> For RTD1295 has only one phy.
>>> For RTD1395 has two phys.
>>
>> Two phys? So how do you reference them when cells=0?
> 
> 
> About RTD1395 SoCs USB
>   XHCI controller#1 -- usb2phy -- phy#0
>                           |- phy#1
> One xhci controller map to one phy driver.
> And one phy driver have two phys (phy@0 and phy@1).
> 
> Maybe the "phy" name is confusing.
> This "phy" not mean a phy driver.

We do not talk about drivers, but DTS and hardware.

> Would "port" be more appropriate? 
> 
> For example,
> Using phy@0 and phy@1:
>     usb_port1_usb2phy: usb-phy@13c14 {
>         compatible = "realtek,rtd1395-usb2phy", "realtek,usb2phy";
>         reg = <0x132c4 0x4>, <0x31280 0x8>;
>         #address-cells = <1>;
>         #size-cells = <0>;
>         #phy-cells = <0>;
>         realtek,usb-ctrl = <&usb_ctrl>;
> 
>         phy@0 {
>             reg = <0>;

So such child is a NAK... you have nothing here. But it's unrelated topic.

>         };
>         phy@1 {
>             reg = <1>;
>         };
>     };
> 
> Change: port@0 and port@1
>     usb_port1_usb2phy: usb-phy@13c14 {
>         compatible = "realtek,rtd1395-usb2phy", "realtek,usb2phy";
>         reg = <0x132c4 0x4>, <0x31280 0x8>;
>         #address-cells = <1>;
>         #size-cells = <0>;
>         #phy-cells = <0>;
>         realtek,usb-ctrl = <&usb_ctrl>;
> 
>         prot@0 {
>             reg = <0>;
>         };
>         port@1 {
>             reg = <1>;
>         };
>     };

This is not the answer. This is the provider. How do you reference it
from the consumer.

Upstream your entire DTS. It's frustrating to try to understand your DTS
from pieces of information you are sharing. Also very time consuming and
you are not the only one sending patches for review...

Best regards,
Krzysztof