Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <7df8ffb6-a544-d10e-5273-fd6c4b368b20@linaro.org>
Date:   Thu, 8 Jun 2023 09:49:01 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.2
Subject: Re: [PATCH v3 4/5] dt-bindings: phy: realtek: Add the doc about the
 Realtek SoC USB 2.0 PHY
Content-Language: en-US
To:     =?UTF-8?B?U3RhbmxleSBDaGFuZ1vmmIzogrLlvrdd?= 
        <stanley_chang@realtek.com>
Cc:     Vinod Koul <vkoul@kernel.org>,
        Kishon Vijay Abraham I <kishon@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        Alan Stern <stern@rowland.harvard.edu>,
        Ray Chi <raychi@google.com>,
        Matthias Kaehlcke <mka@chromium.org>,
        Douglas Anderson <dianders@chromium.org>,
        Michael Grzeschik <m.grzeschik@pengutronix.de>,
        Mathias Nyman <mathias.nyman@linux.intel.com>,
        Flavio Suligoi <f.suligoi@asem.it>,
        "linux-phy@lists.infradead.org" <linux-phy@lists.infradead.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>
References: <20230607062500.24669-1-stanley_chang@realtek.com>
 <20230607062500.24669-4-stanley_chang@realtek.com>
 <7cce1d72-6b4d-9fff-32bc-942193388134@linaro.org>
 <8a88cbee5c6245f2941c700b2bb30697@realtek.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <8a88cbee5c6245f2941c700b2bb30697@realtek.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

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