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

From: Krzysztof Kozlowski
Date: Thu Jun 08 2023 - 03:49:13 EST


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