[RFC,5/8] dt-bindings: net: pcs: add bindings for MediaTek USXGMII PCS
Commit Message
MediaTek's USXGMII can be found in the MT7988 SoC. We need to access
it in order to configure and monitor the Ethernet SerDes link in
USXGMII, 10GBase-R and 5GBase-R mode. By including a wrapped
legacy 1000Base-X/2500Base-X/Cisco SGMII LynxI PCS as well, those
interface modes are also available.
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
.../bindings/net/pcs/mediatek,usxgmii.yaml | 105 ++++++++++++++++++
1 file changed, 105 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/pcs/mediatek,usxgmii.yaml
Comments
On 09/11/2023 22:51, Daniel Golle wrote:
> MediaTek's USXGMII can be found in the MT7988 SoC. We need to access
> it in order to configure and monitor the Ethernet SerDes link in
> USXGMII, 10GBase-R and 5GBase-R mode. By including a wrapped
> legacy 1000Base-X/2500Base-X/Cisco SGMII LynxI PCS as well, those
> interface modes are also available.
>
A nit, subject: drop second/last, redundant "bindings for". The
"dt-bindings" prefix is already stating that these are bindings.
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
> .../bindings/net/pcs/mediatek,usxgmii.yaml | 105 ++++++++++++++++++
Use compatible as filename (especially that you do not expect it to grow).
> 1 file changed, 105 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/pcs/mediatek,usxgmii.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/pcs/mediatek,usxgmii.yaml b/Documentation/devicetree/bindings/net/pcs/mediatek,usxgmii.yaml
> new file mode 100644
> index 0000000000000..199cf47859e31
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/pcs/mediatek,usxgmii.yaml
> @@ -0,0 +1,105 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/pcs/mediatek,usxgmii.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek USXGMII PCS
MT7888 I guess?
> +
> +maintainers:
> + - Daniel Golle <daniel@makrotopia.org>
> +
> +description:
> + The MediaTek USXGMII PCS provides physical link control and status
> + for USXGMII, 10GBase-R and 5GBase-R links on the SerDes interfaces
> + provided by the PEXTP PHY.
> + In order to also support legacy 2500Base-X, 1000Base-X and Cisco
> + SGMII an existing mediatek,*-sgmiisys LynxI PCS is wrapped to
> + provide those interfaces modes on the same SerDes interfaces shared
> + with the USXGMII PCS.
> +
> +properties:
> + $nodename:
> + pattern: "^pcs@[0-9a-f]+$"
Drop, we do not enforce naming in individual device schemas.
> +
> + compatible:
> + const: mediatek,mt7988-usxgmiisys
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + items:
> + - description: USXGMII top-level clock
> + - description: SGMII top-level clock
> + - description: SGMII subsystem TX clock
> + - description: SGMII subsystem RX clock
> + - description: XFI PLL clock
> +
> + clock-names:
> + items:
> + - const: usxgmii
> + - const: sgmii_sel
> + - const: sgmii_tx
> + - const: sgmii_rx
> + - const: xfi_pll
> +
> + phys:
> + items:
> + - description: PEXTP SerDes PHY
> +
> + mediatek,sgmiisys:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + Phandle to the syscon node of the corresponding SGMII LynxI PCS.
"syscon node" is Linux term. Instead describe to what part of hardware
this phandle is and why do you need it.
> +
> + resets:
> + items:
> + - description: XFI reset
> + - description: SGMII reset
> +
> + reset-names:
> + items:
> + - const: xfi
> + - const: sgmii
> +
> + "#pcs-cells":
> + const: 0
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - phys
> + - mediatek,sgmiisys
> + - resets
> + - reset-names
> + - "#pcs-cells"
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/mediatek,mt7988-clk.h>
> + #include <dt-bindings/reset/mediatek,mt7988-resets.h>
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + usxgmiisys0: pcs@10080000 {
Odd/Messed indentation.
Use 4 spaces for example indentation.
Best regards,
Krzysztof
On Thu, Nov 09, 2023 at 09:51:47PM +0000, Daniel Golle wrote:
> MediaTek's USXGMII can be found in the MT7988 SoC. We need to access
> it in order to configure and monitor the Ethernet SerDes link in
> USXGMII, 10GBase-R and 5GBase-R mode. By including a wrapped
> legacy 1000Base-X/2500Base-X/Cisco SGMII LynxI PCS as well, those
> interface modes are also available.
>
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
> .../bindings/net/pcs/mediatek,usxgmii.yaml | 105 ++++++++++++++++++
> 1 file changed, 105 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/pcs/mediatek,usxgmii.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/pcs/mediatek,usxgmii.yaml b/Documentation/devicetree/bindings/net/pcs/mediatek,usxgmii.yaml
> new file mode 100644
> index 0000000000000..199cf47859e31
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/pcs/mediatek,usxgmii.yaml
> @@ -0,0 +1,105 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/pcs/mediatek,usxgmii.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek USXGMII PCS
> +
> +maintainers:
> + - Daniel Golle <daniel@makrotopia.org>
> +
> +description:
> + The MediaTek USXGMII PCS provides physical link control and status
> + for USXGMII, 10GBase-R and 5GBase-R links on the SerDes interfaces
> + provided by the PEXTP PHY.
> + In order to also support legacy 2500Base-X, 1000Base-X and Cisco
> + SGMII an existing mediatek,*-sgmiisys LynxI PCS is wrapped to
> + provide those interfaces modes on the same SerDes interfaces shared
> + with the USXGMII PCS.
> +
> +properties:
> + $nodename:
> + pattern: "^pcs@[0-9a-f]+$"
> +
> + compatible:
> + const: mediatek,mt7988-usxgmiisys
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + items:
> + - description: USXGMII top-level clock
> + - description: SGMII top-level clock
> + - description: SGMII subsystem TX clock
> + - description: SGMII subsystem RX clock
> + - description: XFI PLL clock
> +
> + clock-names:
> + items:
> + - const: usxgmii
> + - const: sgmii_sel
> + - const: sgmii_tx
> + - const: sgmii_rx
> + - const: xfi_pll
> +
> + phys:
> + items:
> + - description: PEXTP SerDes PHY
> +
> + mediatek,sgmiisys:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + Phandle to the syscon node of the corresponding SGMII LynxI PCS.
> +
> + resets:
> + items:
> + - description: XFI reset
> + - description: SGMII reset
> +
> + reset-names:
> + items:
> + - const: xfi
> + - const: sgmii
> +
> + "#pcs-cells":
There is no such property defined.
Rob
On Thu, Nov 09, 2023 at 09:51:47PM +0000, Daniel Golle wrote:
> MediaTek's USXGMII can be found in the MT7988 SoC. We need to access
> it in order to configure and monitor the Ethernet SerDes link in
> USXGMII, 10GBase-R and 5GBase-R mode. By including a wrapped
> legacy 1000Base-X/2500Base-X/Cisco SGMII LynxI PCS as well, those
> interface modes are also available.
I think this binding is based on the implementation than on hardware.
What I believe you have is this setup:
.---- LynxI PCS ----.
MAC ---+ +--- PEXTP --- world
`--- USXGMII PCS ---'
You are representing the PEXTP as a separate entity in DT, but then
you're representing the LynxI PCS and the USXGMII PCS as a single
block, which seems to be how you've decided to implement it.
Given that the LynxI PCS is already in use elsewhere in the Mediatek
range, I suggest that the LynxI PCS is one block of IP, and the USXGMII
PCS is a separate block of IP.
1) Would it not be better to model the two PCS seperately?
2) The addition of the SGMII reset needs more information - is this
controlling a reset for the LynxI block? If so, it should be part
of a LynxI PCS binding.
3) The PEXTP is presumably a separate block which can be shared between
several devices - for example, the LynxI, USXGMII, and probably SATA
and PCIe as well. From the 802.3's network model, the PEXTP is the
PMA/PMD.
From the point of view of 802.3's model, a network interface has
various layers such as the MAC, PCS and PMA/PMD, and sitting above
these layers is the management of the system. Rather than chasing
the data flow (which in a network device can be complex) wouldn't
it be better to continue with the 802.3 model as we are doing with
other devices, rather than trying to go with a new approach here?
new file mode 100644
@@ -0,0 +1,105 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/pcs/mediatek,usxgmii.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek USXGMII PCS
+
+maintainers:
+ - Daniel Golle <daniel@makrotopia.org>
+
+description:
+ The MediaTek USXGMII PCS provides physical link control and status
+ for USXGMII, 10GBase-R and 5GBase-R links on the SerDes interfaces
+ provided by the PEXTP PHY.
+ In order to also support legacy 2500Base-X, 1000Base-X and Cisco
+ SGMII an existing mediatek,*-sgmiisys LynxI PCS is wrapped to
+ provide those interfaces modes on the same SerDes interfaces shared
+ with the USXGMII PCS.
+
+properties:
+ $nodename:
+ pattern: "^pcs@[0-9a-f]+$"
+
+ compatible:
+ const: mediatek,mt7988-usxgmiisys
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ items:
+ - description: USXGMII top-level clock
+ - description: SGMII top-level clock
+ - description: SGMII subsystem TX clock
+ - description: SGMII subsystem RX clock
+ - description: XFI PLL clock
+
+ clock-names:
+ items:
+ - const: usxgmii
+ - const: sgmii_sel
+ - const: sgmii_tx
+ - const: sgmii_rx
+ - const: xfi_pll
+
+ phys:
+ items:
+ - description: PEXTP SerDes PHY
+
+ mediatek,sgmiisys:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ Phandle to the syscon node of the corresponding SGMII LynxI PCS.
+
+ resets:
+ items:
+ - description: XFI reset
+ - description: SGMII reset
+
+ reset-names:
+ items:
+ - const: xfi
+ - const: sgmii
+
+ "#pcs-cells":
+ const: 0
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - phys
+ - mediatek,sgmiisys
+ - resets
+ - reset-names
+ - "#pcs-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/mediatek,mt7988-clk.h>
+ #include <dt-bindings/reset/mediatek,mt7988-resets.h>
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ usxgmiisys0: pcs@10080000 {
+ compatible = "mediatek,mt7988-usxgmiisys";
+ reg = <0 0x10080000 0 0x1000>;
+ clocks = <&topckgen CLK_TOP_USXGMII_SBUS_0_SEL>,
+ <&topckgen CLK_TOP_SGM_0_SEL>,
+ <&sgmiisys0 CLK_SGM0_TX_EN>,
+ <&sgmiisys0 CLK_SGM0_RX_EN>,
+ <&xfi_pll CLK_XFIPLL_PLL_EN>;
+ clock-names = "usxgmii", "sgmii_sel", "sgmii_tx", "sgmii_rx", "xfi_pll";
+ resets = <&watchdog MT7988_TOPRGU_XFI0_GRST>,
+ <&watchdog MT7988_TOPRGU_SGMII0_GRST>;
+ reset-names = "xfi", "sgmii";
+ phys = <&xfi_pextp0>;
+ mediatek,sgmiisys = <&sgmiisys0>;
+ #pcs-cells = <0>;
+ };
+ };