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 09:24, Stanley Chang[昌育德] wrote:
>>> +  realtek,usb-ctrl:
>>> +    description: The phandle of syscon used to control USB PHY power
>> domain.
>>> +    $ref: /schemas/types.yaml#/definitions/phandle
>>
>> No, we have power-domains for this.
> 
> 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?

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

> 
>>> +      For most Relatek SoCs, one XHCI controller only support one the USB
>> 2.0
>>> +      phy. For RTD1395 SoC, the one XHCI controller has two USB 2.0
>> PHYs.
>>> +    properties:
>>> +      realtek,page0-param:
>>> +        description: PHY parameter at page 0. The data are the pair of
>> the
>>> +          offset and value.
>>
>> This needs to be specific. What the heck is "PHY parameter"?
>>
>>> +        $ref: /schemas/types.yaml#/definitions/uint32-array
>>
>> Array? Then maxItems.
> I have found other document.
> It should be a uint32-matrix.
> I will add the maxItems.

Entire property should be dropped.

> 
>>> +
>>> +      realtek,page1-param:
>>> +        description: PHY parameter at page 1. The data are the pair of
>> the
>>> +          offset and value.
>>> +        $ref: /schemas/types.yaml#/definitions/uint32-array
>>> +
>>> +      realtek,page2-param:
>>> +        description: PHY parameter at page 2. The data are the pair of
>> the
>>> +          offset and value. If the PHY support the page 2 parameter.
>>> +        $ref: /schemas/types.yaml#/definitions/uint32-array
>>> +
>>> +      realtek,support-page2-param:
>>> +        description: Set this flag if PHY support page 2 parameter.
>>
>> Why this cannot be deducted from compatible?
> It can identify by compatible.

So drop it.

> 
>>
>>> +        type: boolean
>>> +
>>> +      realtek,do-toggle:
>>> +        description: Set this flag to enable PHY parameter toggle when
>> port
>>> +          status change.
>>
>> Do not instruct OS what to do. Explain why this is a hardware characteristic.
> 
> In my original intention, we hope that this property can be used to control the phy driver do parameter toggle.
> Is it a hardware characteristic? I don't think it's exactly a hardware feature.
> Maybe it can be specified by the compatible.

Drop it.

> 
>>> +        type: boolean
>>> +
>>> +      realtek,do-toggle-driving:
>>> +        description: Set this flag to enable PHY parameter toggle for
>> adjust
>>> +          the driving when port status change.
>>
>> Do not instruct OS what to do. Explain why this is a hardware characteristic.
>>
>>
>>> +        type: boolean
>>> +
>>> +      realtek,check-efuse:
>>> +        description: Enable to update PHY parameter from reading otp
>> table.
>>
>> Do not instruct OS what to do. Explain why this is a hardware characteristic.
> 
> Same above.

So drop all of these.


Best regards,
Krzysztof