| 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
| ||
|
Message-ID: <CAL_Jsq+7zpDDcVzTKSufzuCWnRcLZ0h+y0TpsJE=G+pbuhWtvw@mail.gmail.com>
Date: Fri, 24 Apr 2020 14:55:40 -0500
From: Rob Herring <robh+dt@...nel.org>
To: Florinel Iordache <florinel.iordache@....com>
Cc: David Miller <davem@...emloft.net>,
netdev <netdev@...r.kernel.org>, Andrew Lunn <andrew@...n.ch>,
Florian Fainelli <f.fainelli@...il.com>,
Heiner Kallweit <hkallweit1@...il.com>,
Russell King <linux@...linux.org.uk>,
devicetree@...r.kernel.org,
Linux Doc Mailing List <linux-doc@...r.kernel.org>,
Mark Rutland <mark.rutland@....com>,
Jakub Kicinski <kuba@...nel.org>,
Jonathan Corbet <corbet@....net>,
Shawn Guo <shawnguo@...nel.org>,
Yang-Leo Li <leoyang.li@....com>,
"Madalin Bucur (OSS)" <madalin.bucur@....nxp.com>,
Ioana Ciornei <ioana.ciornei@....com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next v2 2/9] dt-bindings: net: add backplane dt bindings
On Fri, Apr 24, 2020 at 7:46 AM Florinel Iordache
<florinel.iordache@....com> wrote:
>
> Add ethernet backplane device tree bindings
For a new, common binding, you've got to do better than this. Bindings
need to stand on their own. I need a h/w block diagram or something
because I know little about "ethernet backplane".
>
> Signed-off-by: Florinel Iordache <florinel.iordache@....com>
> ---
> .../bindings/net/ethernet-controller.yaml | 3 +-
> .../devicetree/bindings/net/ethernet-phy.yaml | 50 +++++++++++++++++++++
> .../devicetree/bindings/net/serdes-lane.yaml | 51 ++++++++++++++++++++++
> Documentation/devicetree/bindings/net/serdes.yaml | 44 +++++++++++++++++++
> 4 files changed, 147 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/devicetree/bindings/net/serdes-lane.yaml
> create mode 100644 Documentation/devicetree/bindings/net/serdes.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> index ac471b6..541cee5 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> @@ -93,8 +93,9 @@ properties:
> - rxaui
> - xaui
>
> - # 10GBASE-KR, XFI, SFI
> + # 10GBASE-KR, 40GBASE-KR4, XFI, SFI
> - 10gbase-kr
> + - 40gbase-kr4
> - usxgmii
>
> phy-mode:
> diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> index 5aa141c..436b5a7 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> @@ -161,6 +161,42 @@ properties:
> description:
> Specifies a reference to a node representing a SFP cage.
>
> + eq-algorithm:
> + description:
> + Specifies the desired equalization algorithm to be used
> + by the KR link training
> + oneOf:
> + - const: fixed
> + description:
> + Backplane KR using fixed coefficients meaning no
> + equalization algorithm
> + - const: bee
> + description:
> + Backplane KR using 3-Taps Bit Edge Equalization (BEE)
> + algorithm
> +
> + eq-init:
eq-coefficients?
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + minItems: 3
> + maxItems: 3
> + description:
> + Triplet of KR coefficients. Specifies the initialization
> + values for standard KR equalization coefficients used by
> + the link training (pre-cursor, main-cursor, post-cursor)
items:
- description: pre-cursor
- description: main-cursor
...
Is 0-2^32 valid data? If not, add some constraints.
> +
> + eq-params:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + description:
> + Variable size array of KR parameters. Specifies the HW
> + specific parameters used by the link training.
DT is not a dumping ground for magic register values.
eq-init vs. eq-params is pretty vague as to what they are.
I fail to see how these properties are related to $subject. Should be
a separate patch.
> +
> + lane-handle:
> + $ref: /schemas/types.yaml#definitions/phandle
> + description:
> + Specifies a reference (or array of references) to a node
> + representing the desired SERDES lane (or lanes) used in
> + backplane mode.
> +
> required:
> - reg
>
> @@ -183,3 +219,17 @@ examples:
> reset-deassert-us = <2000>;
> };
> };
> + - |
> + ethernet {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ethernet-phy@0 {
> + compatible = "ethernet-phy-ieee802.3-c45";
> + reg = <0x0>;
> + lane-handle = <&lane_d>;
> + eq-algorithm = "fixed";
> + eq-init = <0x2 0x29 0x5>;
> + eq-params = <0>;
> + };
> + };
> diff --git a/Documentation/devicetree/bindings/net/serdes-lane.yaml b/Documentation/devicetree/bindings/net/serdes-lane.yaml
> new file mode 100644
> index 0000000..ce3581e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/serdes-lane.yaml
> @@ -0,0 +1,51 @@
> +# SPDX-License-Identifier: GPL-2.0
Dual license new bindings:
(GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/serdes-lane.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Serdes Lane Binding
> +
> +maintainers:
> + - Florinel Iordache <florinel.iordache@....com>
> +
> +properties:
> + $nodename:
> + pattern: "^lane(@[a-f0-9]+)?$"
> +
> + compatible:
> + oneOf:
> + - const: lane-10g
> + description: Lane part of a 10G SerDes module
> + - const: lane-28g
> + description: Lane part of a 28G SerDes module
> +
> + reg:
> + description:
> + Registers memory map offset and size for this lane
> +
> + reg-names:
> + description:
> + Names of the register map given in "reg" node.
> +
> +examples:
> + - |
> + serdes1: serdes@...0000 {
> + compatible = "serdes-10g";
Do you have a datasheet for this device as bindings describe devices?
I assume not because serdes is a protocol, not a device. AFAIK,
there's no standard programming interface for 'serdes' as that is
about the only time we have any sort of genericish compatible strings.
The compatible string at a minimum should tell me what the programming
model for the registers are.
> + reg = <0x0 0x1ea0000 0 0x00002000>;
> + reg-names = "serdes", "serdes-10g";
The default address and sizes are 1 cell. So you have addr 0 with size
0x1ea0000 and then addr 0 with size 0x2000.
> + little-endian;
> +
> + #address-cells = <1>;
> + #size-cells = <1>;
> + lane_a: lane@800 {
> + compatible = "lane-10g";
> + reg = <0x800 0x40>;
> + reg-names = "lane", "serdes-lane";
Not valid. You have 1 entry (with a addr and size) for reg, but 2
entries for reg-names.
40G is made up of 4 10G lanes, right? Do all 40G serdes phys have
separate register regions for each lane? You can't assume lanes and DT
nodes are 1-1.
As lanes have to be child nodes, these 2 schemas should be 1. Though I
don't think lane nodes will survive.
> + };
> + lane_b: lane@840 {
> + compatible = "lane-10g";
> + reg = <0x840 0x40>;
> + reg-names = "lane", "serdes-lane";
> + };
> + };
> diff --git a/Documentation/devicetree/bindings/net/serdes.yaml b/Documentation/devicetree/bindings/net/serdes.yaml
> new file mode 100644
> index 0000000..fd3da85
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/serdes.yaml
> @@ -0,0 +1,44 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/serdes.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Serdes Module Binding
> +
> +maintainers:
> + - Florinel Iordache <florinel.iordache@....com>
> +
> +properties:
> + $nodename:
> + pattern: "^serdes(@[a-f0-9]+)?$"
> +
> + compatible:
> + oneOf:
> + - const: serdes-10g
> + description: SerDes module type of 10G
> + - const: serdes-28g
> + description: SerDes module type of 28G
> +
> + reg:
> + description:
> + Registers memory map offset and size for this serdes module
> +
> + reg-names:
> + description:
> + Names of the register map given in "reg" node.
> +
> + little-endian:
> + description:
> + Specifies the endianness of serdes module
> + For complete definition see
> + Documentation/devicetree/bindings/common-properties.txt
> +
> +examples:
> + - |
> + serdes1: serdes@...0000 {
> + compatible = "serdes-10g";
> + reg = <0x0 0x1ea0000 0 0x00002000>;
> + reg-names = "serdes", "serdes-10g";
> + little-endian;
> + };
> --
> 1.9.1
>
Powered by blists - more mailing lists