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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: 
 <DM6PR07MB6154B687D5F3196D9FE3D2EFC596A@DM6PR07MB6154.namprd07.prod.outlook.com>
Date: Wed, 20 Dec 2023 09:00:21 +0000
From: Swapnil Kashinath Jakhade <sjakhade@...ence.com>
To: Roger Quadros <rogerq@...nel.org>,
        Krzysztof Kozlowski
	<krzysztof.kozlowski@...aro.org>,
        "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



> -----Original Message-----
> From: Roger Quadros <rogerq@...nel.org>
> Sent: Tuesday, December 19, 2023 2:39 AM
> To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>; 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>; s-vadapalli@...com
> 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 <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!
> 

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