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@xxxxxx> 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@xxxxxx> >>>> [jsarha@xxxxxx: Add separate compatible for Sierra(16G) and Torrent(10G) >>>> SERDES] >>>> Signed-off-by: Jyri Sarha <jsarha@xxxxxx> >>>> --- >>>> .../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@xxxxxx> >>>> + >>>> +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
