Hi Kishon/Rob
My comments are below.
> -----Original Message-----
> From: Kishon Vijay Abraham I <kishon@xxxxxx>
> Sent: Wednesday, October 30, 2019 11:06 AM
> To: Rob Herring <robh@xxxxxxxxxx>; Anil Joy Varughese
> <aniljoy@xxxxxxxxxxx>
> Cc: Roger Quadros <rogerq@xxxxxx>; Jyri Sarha <jsarha@xxxxxx>; linux-
> kernel@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v2 01/14] dt-bindings: phy: Sierra: Add bindings for Sierra
> in TI's J721E
>
> EXTERNAL MAIL
>
>
> Hi Rob,
>
> On 30/10/19 12:29 AM, Rob Herring wrote:
> > On Wed, Oct 23, 2019 at 06:27:22PM +0530, Kishon Vijay Abraham I wrote:
> >> Add DT binding documentation for Sierra PHY IP used in TI's J721E
> >> SoC.
> >>
> >> Signed-off-by: Kishon Vijay Abraham I <kishon@xxxxxx>
> >> ---
> >> .../devicetree/bindings/phy/phy-cadence-sierra.txt | 13
> >> ++++++++-----
> >> 1 file changed, 8 insertions(+), 5 deletions(-)
> >>
> >> diff --git
> >> a/Documentation/devicetree/bindings/phy/phy-cadence-sierra.txt
> >> b/Documentation/devicetree/bindings/phy/phy-cadence-sierra.txt
> >> index 6e1b47bfce43..bf90ef7e005e 100644
> >> --- a/Documentation/devicetree/bindings/phy/phy-cadence-sierra.txt
> >> +++ b/Documentation/devicetree/bindings/phy/phy-cadence-sierra.txt
> >> @@ -2,21 +2,24 @@ Cadence Sierra PHY
> >> -----------------------
> >>
> >> Required properties:
> >> -- compatible: cdns,sierra-phy-t0
> >> -- clocks: Must contain an entry in clock-names.
> >> - See ../clocks/clock-bindings.txt for details.
> >> -- clock-names: Must be "phy_clk"
> >> +- compatible: Must be "cdns,sierra-phy-t0" for Sierra in Cadence
> platform
> >> + Must be "ti,sierra-phy-t0" for Sierra in TI's J721E SoC.
> >> - resets: Must contain an entry for each in reset-names.
> >> See ../reset/reset.txt for details.
> >> - reset-names: Must include "sierra_reset" and "sierra_apb".
> >> "sierra_reset" must control the reset line to the PHY.
> >> "sierra_apb" must control the reset line to the APB PHY
> >> - interface.
> >> + interface ("sierra_apb" is optional).
> >> - reg: register range for the PHY.
> >> - #address-cells: Must be 1
> >> - #size-cells: Must be 0
> >>
> >> Optional properties:
> >> +- clocks: Must contain an entry in clock-names.
> >> + See ../clocks/clock-bindings.txt for details.
> >> +- clock-names: Must be "phy_clk". Must contain "cmn_refclk"
> and
> >> + "cmn_refclk1" for configuring the frequency of the
> >> + clock to the lanes.
> >
> > I don't understand how the same block can have completely different
> > clocks. Did the original binding forget some?
> >
> > TI needs 0, 1 or 3 clocks? Reads like it could be any.
>
> For TI, phy_clk is not needed. Anil, can you clarify what this clock actually
> corresponds to? Is it a functional clock of PHY?
When we had designed the DT binding for Sierra we thought of using phy_clk as a common interface for the clock inputs and there was no specific requirement for splitting it into multiple clocks then and also we had used a simulation environment for testing our IP. We can deprecate the phy_clk property.
Thanks,
Anil
> Sierra SERDES actually has a number of clocks which can be configured. The
> initial dt-binding didn't model all these clocks. The "cmn_refclk" and
> "cmn_refclk1" are used to program the dividers withing the Sierra. The actual
> registers for programming the dividers are in the Sierra wrapper though. The
> original Sierra driver and dt-binding didn't try to change the default divider
> values.
>
> Thanks
> Kishon
> >
> > Rob
> >
