| Message ID | 20191023125735.4713-1-kishon@ti.com |
|---|---|
| Headers | show |
| Series | PHY: Add support for SERDES in TI's J721E SoC | expand |
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@ti.com> > --- > .../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. Rob
On Wed, Oct 23, 2019 at 06:27:34PM +0530, Kishon Vijay Abraham I wrote: > Add DT binding documentation for WIZ (SERDES wrapper). WIZ is *NOT* a > PHY but a wrapper used to configure some of the input signals to the > SERDES. It is used with both Sierra(16G) and Torrent(10G) serdes. > > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > [jsarha@ti.com: Add separate compatible for Sierra(16G) and Torrent(10G) > SERDES] > Signed-off-by: Jyri Sarha <jsarha@ti.com> > --- > .../bindings/phy/ti,phy-j721e-wiz.yaml | 159 ++++++++++++++++++ > 1 file changed, 159 insertions(+) > create mode 100644 Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml > > diff --git a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml > new file mode 100644 > index 000000000000..8a1eccee6c1d > --- /dev/null > +++ b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml > @@ -0,0 +1,159 @@ > +# SPDX-License-Identifier: (GPL-2.0) (GPL-2.0-only OR BSD-2-Clause) for new bindings please. > +# Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/ > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/phy/ti,phy-j721e-wiz.yaml#" > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > + > +title: TI J721E WIZ (SERDES Wrapper) > + > +maintainers: > + - Kishon Vijay Abraham I <kishon@ti.com> > + > +properties: > + compatible: > + oneOf: > + - items: > + - enum: > + - ti,j721e-wiz-16g > + - ti,j721e-wiz-10g You can drop oneOf and items. > + > + power-domains: > + maxItems: 1 > + > + clocks: > + maxItems: 3 > + description: clock-specifier to represent input to the WIZ > + > + clock-names: > + items: > + - const: fck > + - const: core_ref_clk > + - const: ext_ref_clk > + > + num-lanes: > + maxItems: 1 > + minimum: 1 > + maximum: 4 You've mixed array and scalar schema keywords. Drop maxItems. Update dtschema and run 'make dt_binding_check'. We should catch that now. > + > + "#address-cells": > + const: 2 > + > + "#size-cells": > + const: 2 > + > + "#reset-cells": > + const: 1 > + > + ranges: true > + > + assigned-clocks: > + maxItems: 2 > + > + assigned-clock-parents: > + maxItems: 2 > + > +patternProperties: > + "^pll[0|1]_refclk$": > + type: object > + description: | > + WIZ node should have subnodes for each of the PLLs present in > + the SERDES. > + > + "^cmn_refclk1?$": > + type: object > + description: | > + WIZ node should have subnodes for each of the PMA common refclock > + provided by the SERDES. > + > + "^refclk_dig$": > + type: object > + description: | > + WIZ node should have subnode for refclk_dig to select the reference > + clock source for the reference clock used in the PHY and PMA digital > + logic. > + > + "^serdes@[0-9a-f]+$": > + type: object > + description: | > + WIZ node should have '1' subnode for the SERDES. It could be either > + Sierra SERDES or Torrent SERDES. Sierra SERDES should follow the > + bindings specified in > + Documentation/devicetree/bindings/phy/phy-cadence-sierra.txt > + Torrent SERDES should follow the bindings specified in > + Documentation/devicetree/bindings/phy/phy-cadence-dp.txt > + > +required: > + - compatible > + - power-domains > + - clocks > + - clock-names > + - num-lanes > + - "#address-cells" > + - "#size-cells" > + - "#reset-cells" > + > +examples: > + - | > + #include <dt-bindings/soc/ti,sci_pm_domain.h> > + > + wiz@5000000 { > + compatible = "ti,j721e-wiz-16g"; > + #address-cells = <2>; > + #size-cells = <2>; Really need 64-bits of address space for the child nodes? > + power-domains = <&k3_pds 292 TI_SCI_PD_EXCLUSIVE>; > + clocks = <&k3_clks 292 5>, <&k3_clks 292 11>, <&dummy_cmn_refclk>; > + clock-names = "fck", "core_ref_clk", "ext_ref_clk"; > + assigned-clocks = <&k3_clks 292 11>, <&k3_clks 292 0>; > + assigned-clock-parents = <&k3_clks 292 15>, <&k3_clks 292 4>; > + num-lanes = <2>; > + #reset-cells = <1>; Unless you have additional registers, I'm not a fan of wrapper nodes. > + > + pll0_refclk { > + clocks = <&k3_clks 293 13>, <&dummy_cmn_refclk>; > + clock-output-names = "wiz1_pll0_refclk"; > + #clock-cells = <0>; > + assigned-clocks = <&wiz1_pll0_refclk>; > + assigned-clock-parents = <&k3_clks 293 13>; > + }; > + > + pll1_refclk { > + clocks = <&k3_clks 293 0>, <&dummy_cmn_refclk1>; > + clock-output-names = "wiz1_pll1_refclk"; > + #clock-cells = <0>; > + assigned-clocks = <&wiz1_pll1_refclk>; > + assigned-clock-parents = <&k3_clks 293 0>; > + }; > + > + cmn_refclk { > + clocks = <&wiz1_refclk_dig>; > + clock-output-names = "wiz1_cmn_refclk"; > + #clock-cells = <0>; > + }; > + > + cmn_refclk1 { > + clocks = <&wiz1_pll1_refclk>; > + clock-output-names = "wiz1_cmn_refclk1"; > + #clock-cells = <0>; > + }; > + > + refclk_dig { > + clocks = <&k3_clks 292 11>, <&k3_clks 292 0>, <&dummy_cmn_refclk>, <&dummy_cmn_refclk1>; > + clock-output-names = "wiz0_refclk_dig"; > + #clock-cells = <0>; > + assigned-clocks = <&wiz0_refclk_dig>; > + assigned-clock-parents = <&k3_clks 292 11>; > + }; How are all these clocks programmed? > + > + serdes@5000000 { > + compatible = "cdns,ti,sierra-phy-t0"; > + reg-names = "serdes"; > + reg = <0x00 0x5000000 0x00 0x10000>; > + #address-cells = <1>; > + #size-cells = <0>; > + resets = <&serdes_wiz0 0>; > + reset-names = "sierra_reset"; > + clocks = <&wiz0_cmn_refclk>, <&wiz0_cmn_refclk1>; > + clock-names = "cmn_refclk", "cmn_refclk1"; > + }; > + }; > -- > 2.17.1 >
Hi, On 30/10/19 12:38 AM, Rob Herring wrote: > On Wed, Oct 23, 2019 at 06:27:34PM +0530, Kishon Vijay Abraham I wrote: >> Add DT binding documentation for WIZ (SERDES wrapper). WIZ is *NOT* a >> PHY but a wrapper used to configure some of the input signals to the >> SERDES. It is used with both Sierra(16G) and Torrent(10G) serdes. >> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> >> [jsarha@ti.com: Add separate compatible for Sierra(16G) and Torrent(10G) >> SERDES] >> Signed-off-by: Jyri Sarha <jsarha@ti.com> >> --- >> .../bindings/phy/ti,phy-j721e-wiz.yaml | 159 ++++++++++++++++++ >> 1 file changed, 159 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml >> >> diff --git a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml >> new file mode 100644 >> index 000000000000..8a1eccee6c1d >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml >> @@ -0,0 +1,159 @@ >> +# SPDX-License-Identifier: (GPL-2.0) > > (GPL-2.0-only OR BSD-2-Clause) for new bindings please. > >> +# Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/ >> +%YAML 1.2 >> +--- >> +$id: "http://devicetree.org/schemas/phy/ti,phy-j721e-wiz.yaml#" >> +$schema: "http://devicetree.org/meta-schemas/core.yaml#" >> + >> +title: TI J721E WIZ (SERDES Wrapper) >> + >> +maintainers: >> + - Kishon Vijay Abraham I <kishon@ti.com> >> + >> +properties: >> + compatible: >> + oneOf: >> + - items: >> + - enum: >> + - ti,j721e-wiz-16g >> + - ti,j721e-wiz-10g > > You can drop oneOf and items. > >> + >> + power-domains: >> + maxItems: 1 >> + >> + clocks: >> + maxItems: 3 >> + description: clock-specifier to represent input to the WIZ >> + >> + clock-names: >> + items: >> + - const: fck >> + - const: core_ref_clk >> + - const: ext_ref_clk >> + >> + num-lanes: >> + maxItems: 1 >> + minimum: 1 >> + maximum: 4 > > You've mixed array and scalar schema keywords. Drop maxItems. > > Update dtschema and run 'make dt_binding_check'. We should catch that > now. Sure. > >> + >> + "#address-cells": >> + const: 2 >> + >> + "#size-cells": >> + const: 2 >> + >> + "#reset-cells": >> + const: 1 >> + >> + ranges: true >> + >> + assigned-clocks: >> + maxItems: 2 >> + >> + assigned-clock-parents: >> + maxItems: 2 >> + >> +patternProperties: >> + "^pll[0|1]_refclk$": >> + type: object >> + description: | >> + WIZ node should have subnodes for each of the PLLs present in >> + the SERDES. >> + >> + "^cmn_refclk1?$": >> + type: object >> + description: | >> + WIZ node should have subnodes for each of the PMA common refclock >> + provided by the SERDES. >> + >> + "^refclk_dig$": >> + type: object >> + description: | >> + WIZ node should have subnode for refclk_dig to select the reference >> + clock source for the reference clock used in the PHY and PMA digital >> + logic. >> + >> + "^serdes@[0-9a-f]+$": >> + type: object >> + description: | >> + WIZ node should have '1' subnode for the SERDES. It could be either >> + Sierra SERDES or Torrent SERDES. Sierra SERDES should follow the >> + bindings specified in >> + Documentation/devicetree/bindings/phy/phy-cadence-sierra.txt >> + Torrent SERDES should follow the bindings specified in >> + Documentation/devicetree/bindings/phy/phy-cadence-dp.txt >> + >> +required: >> + - compatible >> + - power-domains >> + - clocks >> + - clock-names >> + - num-lanes >> + - "#address-cells" >> + - "#size-cells" >> + - "#reset-cells" >> + >> +examples: >> + - | >> + #include <dt-bindings/soc/ti,sci_pm_domain.h> >> + >> + wiz@5000000 { >> + compatible = "ti,j721e-wiz-16g"; >> + #address-cells = <2>; >> + #size-cells = <2>; > > Really need 64-bits of address space for the child nodes? hmm, the register space for the child nodes are in the 32-bit address space region. I'll fix this. > >> + power-domains = <&k3_pds 292 TI_SCI_PD_EXCLUSIVE>; >> + clocks = <&k3_clks 292 5>, <&k3_clks 292 11>, <&dummy_cmn_refclk>; >> + clock-names = "fck", "core_ref_clk", "ext_ref_clk"; >> + assigned-clocks = <&k3_clks 292 11>, <&k3_clks 292 0>; >> + assigned-clock-parents = <&k3_clks 292 15>, <&k3_clks 292 4>; >> + num-lanes = <2>; >> + #reset-cells = <1>; > > Unless you have additional registers, I'm not a fan of wrapper nodes. The wrapper node has TI specific registers while the child node has Cadence Sierra specific registers. It also has clock nodes which are input to the Sierra IP. > >> + >> + pll0_refclk { >> + clocks = <&k3_clks 293 13>, <&dummy_cmn_refclk>; >> + clock-output-names = "wiz1_pll0_refclk"; >> + #clock-cells = <0>; >> + assigned-clocks = <&wiz1_pll0_refclk>; >> + assigned-clock-parents = <&k3_clks 293 13>; >> + }; >> + >> + pll1_refclk { >> + clocks = <&k3_clks 293 0>, <&dummy_cmn_refclk1>; >> + clock-output-names = "wiz1_pll1_refclk"; >> + #clock-cells = <0>; >> + assigned-clocks = <&wiz1_pll1_refclk>; >> + assigned-clock-parents = <&k3_clks 293 0>; >> + }; >> + >> + cmn_refclk { >> + clocks = <&wiz1_refclk_dig>; >> + clock-output-names = "wiz1_cmn_refclk"; >> + #clock-cells = <0>; >> + }; >> + >> + cmn_refclk1 { >> + clocks = <&wiz1_pll1_refclk>; >> + clock-output-names = "wiz1_cmn_refclk1"; >> + #clock-cells = <0>; >> + }; >> + >> + refclk_dig { >> + clocks = <&k3_clks 292 11>, <&k3_clks 292 0>, <&dummy_cmn_refclk>, <&dummy_cmn_refclk1>; >> + clock-output-names = "wiz0_refclk_dig"; >> + #clock-cells = <0>; >> + assigned-clocks = <&wiz0_refclk_dig>; >> + assigned-clock-parents = <&k3_clks 292 11>; >> + }; > > How are all these clocks programmed? All these are programmed in the WIZ driver which is implemented in 14/14 of this series. Thanks Kishon
Hi Rob, On 31/10/19 12:56 AM, Rob Herring wrote: > On Wed, Oct 30, 2019 at 12:46 AM Kishon Vijay Abraham I <kishon@ti.com> wrote: >> >> Hi, >> >> On 30/10/19 12:38 AM, Rob Herring wrote: >>> On Wed, Oct 23, 2019 at 06:27:34PM +0530, Kishon Vijay Abraham I wrote: >>>> Add DT binding documentation for WIZ (SERDES wrapper). WIZ is *NOT* a >>>> PHY but a wrapper used to configure some of the input signals to the >>>> SERDES. It is used with both Sierra(16G) and Torrent(10G) serdes. >>>> >>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> >>>> [jsarha@ti.com: Add separate compatible for Sierra(16G) and Torrent(10G) >>>> SERDES] >>>> Signed-off-by: Jyri Sarha <jsarha@ti.com> >>>> --- >>>> .../bindings/phy/ti,phy-j721e-wiz.yaml | 159 ++++++++++++++++++ >>>> 1 file changed, 159 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml >>>> >>>> diff --git a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml >>>> new file mode 100644 >>>> index 000000000000..8a1eccee6c1d >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml >>>> @@ -0,0 +1,159 @@ >>>> +# SPDX-License-Identifier: (GPL-2.0) >>> >>> (GPL-2.0-only OR BSD-2-Clause) for new bindings please. >>> >>>> +# Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/ >>>> +%YAML 1.2 >>>> +--- >>>> +$id: "http://devicetree.org/schemas/phy/ti,phy-j721e-wiz.yaml#" >>>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#" >>>> + >>>> +title: TI J721E WIZ (SERDES Wrapper) >>>> + >>>> +maintainers: >>>> + - Kishon Vijay Abraham I <kishon@ti.com> >>>> + >>>> +properties: >>>> + compatible: >>>> + oneOf: >>>> + - items: >>>> + - enum: >>>> + - ti,j721e-wiz-16g >>>> + - ti,j721e-wiz-10g >>> >>> You can drop oneOf and items. >>> >>>> + >>>> + power-domains: >>>> + maxItems: 1 >>>> + >>>> + clocks: >>>> + maxItems: 3 >>>> + description: clock-specifier to represent input to the WIZ >>>> + >>>> + clock-names: >>>> + items: >>>> + - const: fck >>>> + - const: core_ref_clk >>>> + - const: ext_ref_clk >>>> + >>>> + num-lanes: >>>> + maxItems: 1 >>>> + minimum: 1 >>>> + maximum: 4 >>> >>> You've mixed array and scalar schema keywords. Drop maxItems. >>> >>> Update dtschema and run 'make dt_binding_check'. We should catch that >>> now. >> >> Sure. >>> >>>> + >>>> + "#address-cells": >>>> + const: 2 >>>> + >>>> + "#size-cells": >>>> + const: 2 >>>> + >>>> + "#reset-cells": >>>> + const: 1 >>>> + >>>> + ranges: true >>>> + >>>> + assigned-clocks: >>>> + maxItems: 2 >>>> + >>>> + assigned-clock-parents: >>>> + maxItems: 2 >>>> + >>>> +patternProperties: >>>> + "^pll[0|1]_refclk$": >>>> + type: object >>>> + description: | >>>> + WIZ node should have subnodes for each of the PLLs present in >>>> + the SERDES. >>>> + >>>> + "^cmn_refclk1?$": >>>> + type: object >>>> + description: | >>>> + WIZ node should have subnodes for each of the PMA common refclock >>>> + provided by the SERDES. >>>> + >>>> + "^refclk_dig$": >>>> + type: object >>>> + description: | >>>> + WIZ node should have subnode for refclk_dig to select the reference >>>> + clock source for the reference clock used in the PHY and PMA digital >>>> + logic. >>>> + >>>> + "^serdes@[0-9a-f]+$": >>>> + type: object >>>> + description: | >>>> + WIZ node should have '1' subnode for the SERDES. It could be either >>>> + Sierra SERDES or Torrent SERDES. Sierra SERDES should follow the >>>> + bindings specified in >>>> + Documentation/devicetree/bindings/phy/phy-cadence-sierra.txt >>>> + Torrent SERDES should follow the bindings specified in >>>> + Documentation/devicetree/bindings/phy/phy-cadence-dp.txt >>>> + >>>> +required: >>>> + - compatible >>>> + - power-domains >>>> + - clocks >>>> + - clock-names >>>> + - num-lanes >>>> + - "#address-cells" >>>> + - "#size-cells" >>>> + - "#reset-cells" >>>> + >>>> +examples: >>>> + - | >>>> + #include <dt-bindings/soc/ti,sci_pm_domain.h> >>>> + >>>> + wiz@5000000 { >>>> + compatible = "ti,j721e-wiz-16g"; >>>> + #address-cells = <2>; >>>> + #size-cells = <2>; >>> >>> Really need 64-bits of address space for the child nodes? >> >> hmm, the register space for the child nodes are in the 32-bit address space >> region. I'll fix this. >>> >>>> + power-domains = <&k3_pds 292 TI_SCI_PD_EXCLUSIVE>; >>>> + clocks = <&k3_clks 292 5>, <&k3_clks 292 11>, <&dummy_cmn_refclk>; >>>> + clock-names = "fck", "core_ref_clk", "ext_ref_clk"; >>>> + assigned-clocks = <&k3_clks 292 11>, <&k3_clks 292 0>; >>>> + assigned-clock-parents = <&k3_clks 292 15>, <&k3_clks 292 4>; >>>> + num-lanes = <2>; >>>> + #reset-cells = <1>; >>> >>> Unless you have additional registers, I'm not a fan of wrapper nodes. >> >> The wrapper node has TI specific registers while the child node has Cadence >> Sierra specific registers. It also has clock nodes which are input to the >> Sierra IP. > > Yeah? Where's 'reg'? The TI specific PHY registers use some of the reserved space within the Cadence region. So the WIZ wrapper driver will get the address from the "serdes" child node. > >>> >>>> + >>>> + pll0_refclk { >>>> + clocks = <&k3_clks 293 13>, <&dummy_cmn_refclk>; >>>> + clock-output-names = "wiz1_pll0_refclk"; >>>> + #clock-cells = <0>; >>>> + assigned-clocks = <&wiz1_pll0_refclk>; >>>> + assigned-clock-parents = <&k3_clks 293 13>; >>>> + }; >>>> + >>>> + pll1_refclk { >>>> + clocks = <&k3_clks 293 0>, <&dummy_cmn_refclk1>; >>>> + clock-output-names = "wiz1_pll1_refclk"; >>>> + #clock-cells = <0>; >>>> + assigned-clocks = <&wiz1_pll1_refclk>; >>>> + assigned-clock-parents = <&k3_clks 293 0>; >>>> + }; >>>> + >>>> + cmn_refclk { >>>> + clocks = <&wiz1_refclk_dig>; >>>> + clock-output-names = "wiz1_cmn_refclk"; >>>> + #clock-cells = <0>; >>>> + }; >>>> + >>>> + cmn_refclk1 { >>>> + clocks = <&wiz1_pll1_refclk>; >>>> + clock-output-names = "wiz1_cmn_refclk1"; >>>> + #clock-cells = <0>; >>>> + }; >>>> + >>>> + refclk_dig { >>>> + clocks = <&k3_clks 292 11>, <&k3_clks 292 0>, <&dummy_cmn_refclk>, <&dummy_cmn_refclk1>; >>>> + clock-output-names = "wiz0_refclk_dig"; >>>> + #clock-cells = <0>; >>>> + assigned-clocks = <&wiz0_refclk_dig>; >>>> + assigned-clock-parents = <&k3_clks 292 11>; >>>> + }; >>> >>> How are all these clocks programmed? >> >> All these are programmed in the WIZ driver which is implemented in 14/14 of >> this series. > > Not what I meant... How does one access the h/w because there's > nothing defined here to do so. As mentioned above the WIZ wrapper driver gets the address from "serdes" child node and use it for programming all these clocks. Thanks Kishon
Hi Kishon/Rob My comments are below. > -----Original Message----- > From: Kishon Vijay Abraham I <kishon@ti.com> > Sent: Wednesday, October 30, 2019 11:06 AM > To: Rob Herring <robh@kernel.org>; Anil Joy Varughese > <aniljoy@cadence.com> > Cc: Roger Quadros <rogerq@ti.com>; Jyri Sarha <jsarha@ti.com>; linux- > kernel@vger.kernel.org; devicetree@vger.kernel.org > 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@ti.com> > >> --- > >> .../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 > >
TI's J721E SoC uses Cadence Sierra SERDES for USB, PCIe and SGMII. TI has a wrapper named WIZ to control input signals to Sierra and Torrent SERDES. This patch series: 1) Add support to WIZ module present in TI's J721E SoC 2) Adapt Cadence Sierra PHY driver to be used for J721E SoC Changes from v1: *) Change the dt binding Documentation of WIZ wrapper to YAML format *) Fix an issue in Sierra while doimg rmmod Anil Varughese (1): phy: cadence: Sierra: Configure both lane cdb and common cdb registers for external SSC Kishon Vijay Abraham I (13): dt-bindings: phy: Sierra: Add bindings for Sierra in TI's J721E phy: cadence: Sierra: Make "phy_clk" and "sierra_apb" optional resources phy: cadence: Sierra: Use "regmap" for read and write to Sierra registers phy: cadence: Sierra: Add support for SERDES_16G used in J721E SoC phy: cadence: Sierra: Make cdns_sierra_phy_init() as phy_ops phy: cadence: Sierra: Modify register macro names to be in sync with Sierra user guide phy: cadence: Sierra: Get reset control "array" for each link phy: cadence: Sierra: Check for PLL lock during PHY power on phy: cadence: Sierra: Change MAX_LANES of Sierra to 16 phy: cadence: Sierra: Set cmn_refclk/cmn_refclk1 frequency to 25MHz phy: cadence: Sierra: Use correct dev pointer in cdns_sierra_phy_remove() dt-bindings: phy: Document WIZ (SERDES wrapper) bindings phy: ti: j721e-wiz: Add support for WIZ module present in TI J721E SoC .../bindings/phy/phy-cadence-sierra.txt | 13 +- .../bindings/phy/ti,phy-j721e-wiz.yaml | 159 +++ drivers/phy/cadence/phy-cadence-sierra.c | 697 +++++++++++--- drivers/phy/ti/Kconfig | 15 + drivers/phy/ti/Makefile | 1 + drivers/phy/ti/phy-j721e-wiz.c | 904 ++++++++++++++++++ 6 files changed, 1650 insertions(+), 139 deletions(-) create mode 100644 Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml create mode 100644 drivers/phy/ti/phy-j721e-wiz.c -- 2.17.1