[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9a86efae-2855-484e-be01-cf5fffdf9f95@kernel.org>
Date: Mon, 18 Dec 2023 23:09:15 +0200
From: Roger Quadros <rogerq@...nel.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
Swapnil Kashinath Jakhade <sjakhade@...ence.com>,
"vkoul@...nel.org" <vkoul@...nel.org>, "kishon@...nel.org"
<kishon@...nel.org>, "robh+dt@...nel.org" <robh+dt@...nel.org>,
"krzysztof.kozlowski+dt@...aro.org" <krzysztof.kozlowski+dt@...aro.org>,
"conor+dt@...nel.org" <conor+dt@...nel.org>,
"linux-phy@...ts.infradead.org" <linux-phy@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Cc: Milind Parab <mparab@...ence.com>, "s-vadapalli@...com"
<s-vadapalli@...com>
Subject: Re: [PATCH v2 1/5] dt-bindings: phy: cadence-torrent: Add optional
input reference clock for PLL1
On 14/12/2023 09:22, Krzysztof Kozlowski wrote:
> On 14/12/2023 08:02, Swapnil Kashinath Jakhade wrote:
>> Hi Krzysztof,
>>
>>> -----Original Message-----
>>> From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
>>> Sent: Wednesday, December 13, 2023 12:19 PM
>>> To: Swapnil Kashinath Jakhade <sjakhade@...ence.com>; vkoul@...nel.org;
>>> kishon@...nel.org; robh+dt@...nel.org; krzysztof.kozlowski+dt@...aro.org;
>>> conor+dt@...nel.org; linux-phy@...ts.infradead.org; linux-
>>> kernel@...r.kernel.org; devicetree@...r.kernel.org
>>> Cc: Milind Parab <mparab@...ence.com>; rogerq@...nel.org; s-
>>> vadapalli@...com
>>> Subject: Re: [PATCH v2 1/5] dt-bindings: phy: cadence-torrent: Add optional
>>> input reference clock for PLL1
>>>
>>> EXTERNAL MAIL
>>>
>>>
>>> On 12/12/2023 12:48, Swapnil Jakhade wrote:
>>>> Torrent PHY can have two input reference clocks. Update bindings
>>>
>>> It already supports two.
>>>
>>
>> Thanks for your comments.
>> refclk and pll1_refclk are the two input reference clocks for the PLLs.
>> phy_en_refclk is used to enable output reference clock in some cases.
>
> Why input clock is used to enable output reference clock?
Looking at the driver code, "phy_en_refclk" is used at 2 places only to
set the parent of 2 clocks apparently called "received reference clock"
and "derived reference clock", either of which can be used to enable an
optional reference clock output feature.
My understanding is that it is a separate refclk input than 'refclk' (pll0)
or 'pll1_refclk' which can be used to enable a reference clock output feature.
So this PHY can support a total of 3 input reference clocks:
"refclk" is PLL0 reference clock and is required on all platforms
"pll1_refclk" is PLL1 reference clock and is required on some platforms
"phy_en_refclk" is reference for output reference clock generator and is optional on all platforms
Swapnil, can you please confirm my understanding? Thanks!
>
>>
>>>> to support dual reference clock multilink configurations.
>>>>
>>>> Signed-off-by: Swapnil Jakhade <sjakhade@...ence.com>
>>>> ---
>>>> .../devicetree/bindings/phy/phy-cadence-torrent.yaml | 6 +++---
>>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/phy/phy-cadence-
>>> torrent.yaml b/Documentation/devicetree/bindings/phy/phy-cadence-
>>> torrent.yaml
>>>> index dfb31314face..98946f549895 100644
>>>> --- a/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
>>>> +++ b/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
>>>> @@ -35,14 +35,14 @@ properties:
>>>> minItems: 1
>>>> maxItems: 2
>>>> description:
>>>> - PHY reference clock for 1 item. Must contain an entry in clock-names.
>>>> - Optional Parent to enable output reference clock.
>>>> + PHY input reference clocks - refclk & pll1_refclk (optional).
>>>> + Optional Parent to enable output reference clock (phy_en_refclk).
>>>
>>> So third clock? But you allow only two? Confusing.
>>>
>>
>> Yes, if both refclk and pll1_refclk are present, phy_en_refclk can't be used.
>>
>>>>
>>>> clock-names:
>>>> minItems: 1
>>>> items:
>>>> - const: refclk
>>>> - - const: phy_en_refclk
>>>> + - enum: [ pll1_refclk, phy_en_refclk ]
>>>
>>> This does not match your commit msg. You already had two clocks there.
>>>
>> Yes, but refclk was the single input reference clock used for PLLs earlier.
>> As stated in commit message, a new input reference clock (pll1_refclk) is added here.
>
> existing phy_en_refclk is also input reference clock, isn't it?
>
>>
>
> Best regards,
> Krzysztof
>
--
cheers,
-roger
Powered by blists - more mailing lists