Subject: [PATCH v2 1/5] dt-bindings: phy: cadence-torrent: Add optional input reference clock for PLL1

Torrent PHY can have two input reference clocks. Update bindings
to support dual reference clock multilink configurations.

Signed-off-by: Swapnil Jakhade <[email protected]>
---
.../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).

clock-names:
minItems: 1
items:
- const: refclk
- - const: phy_en_refclk
+ - enum: [ pll1_refclk, phy_en_refclk ]

reg:
minItems: 1
--
2.25.1


2023-12-13 06:49:05

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] dt-bindings: phy: cadence-torrent: Add optional input reference clock for PLL1

On 12/12/2023 12:48, Swapnil Jakhade wrote:
> Torrent PHY can have two input reference clocks. Update bindings

It already supports two.

> to support dual reference clock multilink configurations.
>
> Signed-off-by: Swapnil Jakhade <[email protected]>
> ---
> .../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.

>
> 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.

Best regards,
Krzysztof

Subject: RE: [PATCH v2 1/5] dt-bindings: phy: cadence-torrent: Add optional input reference clock for PLL1

Hi Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <[email protected]>
> Sent: Wednesday, December 13, 2023 12:19 PM
> To: Swapnil Kashinath Jakhade <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]
> Cc: Milind Parab <[email protected]>; [email protected]; s-
> [email protected]
> 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.

> > to support dual reference clock multilink configurations.
> >
> > Signed-off-by: Swapnil Jakhade <[email protected]>
> > ---
> > .../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.

Thanks & regards,
Swapnil

> Best regards,
> Krzysztof

2023-12-14 07:22:24

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] dt-bindings: phy: cadence-torrent: Add optional input reference clock for PLL1

On 14/12/2023 08:02, Swapnil Kashinath Jakhade wrote:
> Hi Krzysztof,
>
>> -----Original Message-----
>> From: Krzysztof Kozlowski <[email protected]>
>> Sent: Wednesday, December 13, 2023 12:19 PM
>> To: Swapnil Kashinath Jakhade <[email protected]>; [email protected];
>> [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected]; linux-
>> [email protected]; [email protected]
>> Cc: Milind Parab <[email protected]>; [email protected]; s-
>> [email protected]
>> 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?

>
>>> to support dual reference clock multilink configurations.
>>>
>>> Signed-off-by: Swapnil Jakhade <[email protected]>
>>> ---
>>> .../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

2023-12-18 21:19:24

by Roger Quadros

[permalink] [raw]
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 <[email protected]>
>>> Sent: Wednesday, December 13, 2023 12:19 PM
>>> To: Swapnil Kashinath Jakhade <[email protected]>; [email protected];
>>> [email protected]; [email protected]; [email protected];
>>> [email protected]; [email protected]; linux-
>>> [email protected]; [email protected]
>>> Cc: Milind Parab <[email protected]>; [email protected]; s-
>>> [email protected]
>>> 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 <[email protected]>
>>>> ---
>>>> .../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

Subject: RE: [PATCH v2 1/5] dt-bindings: phy: cadence-torrent: Add optional input reference clock for PLL1



> -----Original Message-----
> From: Roger Quadros <[email protected]>
> Sent: Tuesday, December 19, 2023 2:39 AM
> To: Krzysztof Kozlowski <[email protected]>; Swapnil Kashinath
> Jakhade <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]
> Cc: Milind Parab <[email protected]>; [email protected]
> Subject: Re: [PATCH v2 1/5] dt-bindings: phy: cadence-torrent: Add optional
> input reference clock for PLL1
>
> EXTERNAL MAIL
>
>
>
> 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 <[email protected]>
> >>> Sent: Wednesday, December 13, 2023 12:19 PM
> >>> To: Swapnil Kashinath Jakhade <[email protected]>;
> [email protected];
> >>> [email protected]; [email protected];
> [email protected];
> >>> [email protected]; [email protected]; linux-
> >>> [email protected]; [email protected]
> >>> Cc: Milind Parab <[email protected]>; [email protected]; s-
> >>> [email protected]
> >>> 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!
>

Yes. This is correct. The two input reference clocks mentioned in commit
message refers to refclk and pll1_refclk.

Thanks & regards,
Swapnil
> >
> >>
> >>>> to support dual reference clock multilink configurations.
> >>>>
> >>>> Signed-off-by: Swapnil Jakhade <[email protected]>
> >>>> ---
> >>>> .../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

2023-12-20 10:00:40

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] dt-bindings: phy: cadence-torrent: Add optional input reference clock for PLL1



On 20/12/2023 11:00, Swapnil Kashinath Jakhade wrote:
>
>
>> -----Original Message-----
>> From: Roger Quadros <[email protected]>
>> Sent: Tuesday, December 19, 2023 2:39 AM
>> To: Krzysztof Kozlowski <[email protected]>; Swapnil Kashinath
>> Jakhade <[email protected]>; [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected];
>> [email protected]
>> Cc: Milind Parab <[email protected]>; [email protected]
>> Subject: Re: [PATCH v2 1/5] dt-bindings: phy: cadence-torrent: Add optional
>> input reference clock for PLL1
>>
>> EXTERNAL MAIL
>>
>>
>>
>> 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 <[email protected]>
>>>>> Sent: Wednesday, December 13, 2023 12:19 PM
>>>>> To: Swapnil Kashinath Jakhade <[email protected]>;
>> [email protected];
>>>>> [email protected]; [email protected];
>> [email protected];
>>>>> [email protected]; [email protected]; linux-
>>>>> [email protected]; [email protected]
>>>>> Cc: Milind Parab <[email protected]>; [email protected]; s-
>>>>> [email protected]
>>>>> 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!
>>
>
> Yes. This is correct. The two input reference clocks mentioned in commit
> message refers to refclk and pll1_refclk.

Thanks. Can you please clarify this in the DT documentation in next spin?

>
> Thanks & regards,
> Swapnil
>>>
>>>>
>>>>>> to support dual reference clock multilink configurations.
>>>>>>
>>>>>> Signed-off-by: Swapnil Jakhade <[email protected]>
>>>>>> ---
>>>>>> .../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

--
cheers,
-roger