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]
Date:   Wed, 30 Oct 2019 11:15:44 +0530
From:   Kishon Vijay Abraham I <kishon@...com>
To:     Rob Herring <robh@...nel.org>
CC:     Roger Quadros <rogerq@...com>, Jyri Sarha <jsarha@...com>,
        Anil Varughese <aniljoy@...ence.com>,
        <linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>
Subject: Re: [PATCH v2 13/14] dt-bindings: phy: Document WIZ (SERDES wrapper)
 bindings

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ