-----Original Message-----
From: Florian Fainelli <f.fainelli@gmail.com>
Sent: Tuesday, June 23, 2020 1:21 AM
To: Florinel Iordache <redacted>; davem@davemloft.net;
netdev@vger.kernel.org; andrew@lunn.ch; hkallweit1@gmail.com;
linux@armlinux.org.uk
Cc: devicetree@vger.kernel.org; linux-doc@vger.kernel.org;
robh+dt@kernel.org; mark.rutland@arm.com; kuba@kernel.org;
corbet@lwn.net; shawnguo@kernel.org; Leo Li [off-list ref]; Madalin
Bucur (OSS) [off-list ref]; Ioana Ciornei
[off-list ref]; linux-kernel@vger.kernel.org
Subject: [EXT] Re: [PATCH net-next v3 2/7] dt-bindings: net: add backplane dt
bindings

Caution: EXT Email

On 6/22/20 6:35 AM, Florinel Iordache wrote:

quoted

Add ethernet backplane device tree bindings

Signed-off-by: Florinel Iordache <redacted>
---

[snip]

quoted

+properties:
+  $nodename:
+    pattern: "^serdes(@[a-f0-9]+)?$"
+
+  compatible:
+    oneOf:
+      - const: serdes-10g
+        description: SerDes module type of 10G

Since this appears to be memory mapped in your case, it does not sound like
"serdes-10g" alone is going to be sufficient, should not we have a SoC specific
compatible string if nothing else?

My intention was to make it generic enough to be used by any SerDes (Serializer/Deserializer) block.
So I was thinking that specifying serdes as HW block and the type: 10g (or 28g for example) should be enough.
I could add SoC specific (or family of SoC) to the compatible string
like for example Freescale/NXP QorIQ Soc: "fsl,ls1046a-serdes-10g" or "fsl,qoriq-serdes-10g"

quoted

+
+  reg:
+    description:
+      Registers memory map offset and size for this serdes module
+
+  reg-names:
+    description:
+      Names of the register map given in "reg" node.

You would also need to describe how many of these two properties are
expected.

Only one memory map is required since the memory maps for lanes are individually described
(as it is documented in serdes-lane.yaml).
I will specify this.

quoted

+
+  little-endian:
+    description:
+      Specifies the endianness of serdes module
+      For complete definition see
+      Documentation/devicetree/bindings/common-properties.txt

This is redundant with the default binding then, and if it is not already in the
common YAML binding, can you please add little-endian and native-endian
added there?

The endianness of the serdes block must be specified as little-endian or big-endian.
The serdes endianness may be different than the cores endianness.
This is also the case for QorIQ LS1043/LS1046 platforms with ARM cores which
are little endian but serdes block is big endian.
So endianness must be specified in device tree in order for the driver to know how to access it.
This is the generic binding description (with an example below)
but for LS1046 platform for example we should put: big-endian
(as it is in the last patch: 0007-arm64-dts-add-serdes-and-mdio-description.patch
in file: /arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi )

quoted

+
+examples:
+  - |
+    serdes1: serdes@1ea0000 {
+        compatible = "serdes-10g";
+        reg = <0x0 0x1ea0000 0 0x00002000>;
+        reg-names = "serdes", "serdes-10g";
+        little-endian;
+    };

--
Florian

Thank you for feedback
Florinel.