| Message ID | 20191023125735.4713-14-kishon@ti.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | PHY: Add support for SERDES in TI's J721E SoC | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | fail | build log |
Hi Rob, On 23/10/19 6:27 PM, 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> Since you've reviewed the other patches posted after this one, wanted to check if this somehow slipped out of your radar. Thanks Kishon > --- > .../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) > +# 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 > + > + 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 > + > + "#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>; > + 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>; > + > + 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>; > + }; > + > + 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"; > + }; > + }; >
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
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'? > > > >> + > >> + 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. Rob
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
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) +# 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 + + 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 + + "#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>; + 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>; + + 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>; + }; + + 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"; + }; + };