| Message ID | b875f693f6d4367a610a12ef324584f3bf3a1c1c.1702352117.git.daniel@makrotopia.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add support for 10G Ethernet SerDes on MT7988 | expand |
On Tue, 12 Dec 2023 03:46:26 +0000, Daniel Golle wrote: > Add bindings for the MediaTek PEXTP Ethernet SerDes PHY found in the > MediaTek MT7988 SoC which can operate at various interfaces modes: > > * USXGMII > * 10GBase-R > * 5GBase-R > * 2500Base-X > * 1000Base-X > * Cisco SGMII (MAC side) > > Signed-off-by: Daniel Golle <daniel@makrotopia.org> > --- > .../bindings/phy/mediatek,xfi-pextp.yaml | 80 +++++++++++++++++++ > 1 file changed, 80 insertions(+) > create mode 100644 Documentation/devicetree/bindings/phy/mediatek,xfi-pextp.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: ./Documentation/devicetree/bindings/phy/mediatek,xfi-pextp.yaml:33:16: [error] string value is redundantly quoted with any quotes (quoted-strings) ./Documentation/devicetree/bindings/phy/mediatek,xfi-pextp.yaml:34:16: [error] string value is redundantly quoted with any quotes (quoted-strings) dtschema/dtc warnings/errors: Documentation/devicetree/bindings/phy/mediatek,xfi-pextp.example.dts:18:18: fatal error: dt-bindings/clock/mediatek,mt7988-clk.h: No such file or directory 18 | #include <dt-bindings/clock/mediatek,mt7988-clk.h> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compilation terminated. make[2]: *** [scripts/Makefile.lib:419: Documentation/devicetree/bindings/phy/mediatek,xfi-pextp.example.dtb] Error 1 make[2]: *** Waiting for unfinished jobs.... make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1424: dt_binding_check] Error 2 make: *** [Makefile:234: __sub-make] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/b875f693f6d4367a610a12ef324584f3bf3a1c1c.1702352117.git.daniel@makrotopia.org The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On Tue, Dec 12, 2023 at 03:46:26AM +0000, Daniel Golle wrote: > + mediatek,usxgmii-performance-errata: > + $ref: /schemas/types.yaml#/definitions/flag > + description: > + USXGMII0 on MT7988 suffers from a performance problem in 10GBase-R > + mode which needs a work-around in the driver. The work-around is > + enabled using this flag. Why do you need a property for this if you know that it is present on the MT7988?
On Tue, Dec 12, 2023 at 04:21:38PM +0000, Conor Dooley wrote: > On Tue, Dec 12, 2023 at 03:46:26AM +0000, Daniel Golle wrote: > > > + mediatek,usxgmii-performance-errata: > > + $ref: /schemas/types.yaml#/definitions/flag > > + description: > > + USXGMII0 on MT7988 suffers from a performance problem in 10GBase-R > > + mode which needs a work-around in the driver. The work-around is > > + enabled using this flag. > > Why do you need a property for this if you know that it is present on > the MT7988? Because it is only present in one of the two SerDes channels. Channel 0 needs the work-around, Channel 1 doesn't. See also this commit in the vendor driver for reference[1]. We previously discussed that[2] and it was decided that a property would be the prefered way to represent this as there aren't any other per-instance differences which would justify another compatible. [1]: https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/a500d94cd47e279015ce22947e1ce396a7516598%5E%21/#F0 [2]: https://patchwork.kernel.org/comment/25592845/
On Tue, Dec 12, 2023 at 04:42:45PM +0000, Daniel Golle wrote: > On Tue, Dec 12, 2023 at 04:21:38PM +0000, Conor Dooley wrote: > > On Tue, Dec 12, 2023 at 03:46:26AM +0000, Daniel Golle wrote: > > > > > + mediatek,usxgmii-performance-errata: > > > + $ref: /schemas/types.yaml#/definitions/flag > > > + description: > > > + USXGMII0 on MT7988 suffers from a performance problem in 10GBase-R > > > + mode which needs a work-around in the driver. The work-around is > > > + enabled using this flag. > > > > Why do you need a property for this if you know that it is present on > > the MT7988? > > Because it is only present in one of the two SerDes channels. > Channel 0 needs the work-around, Channel 1 doesn't. > > See also this commit in the vendor driver for reference[1]. > > We previously discussed that[2] and it was decided that a property > would be the prefered way to represent this as there aren't any other > per-instance differences which would justify another compatible. Please put it in the commit message so that when the next version shows up, Krzysztof doesn't show up and question the property for the third time. Also, on another note, this series is aimed at net-next but half the series is fixed for incorrect bindings. Why not net?
> Because it is only present in one of the two SerDes channels. > Channel 0 needs the work-around, Channel 1 doesn't. Does the channel know its own number? Andrew
On Wed, Dec 13, 2023 at 02:20:45PM +0100, Andrew Lunn wrote: > > Because it is only present in one of the two SerDes channels. > > Channel 0 needs the work-around, Channel 1 doesn't. > > Does the channel know its own number? As far as I know we can't infer the channel number by any of the registers default values. We could infer it from the base address, and that would kinda defy the purpose of having Device Tree to begin with in my feeling at least, but it would be possible, of course.
diff --git a/Documentation/devicetree/bindings/phy/mediatek,xfi-pextp.yaml b/Documentation/devicetree/bindings/phy/mediatek,xfi-pextp.yaml new file mode 100644 index 0000000000000..58c93368141e2 --- /dev/null +++ b/Documentation/devicetree/bindings/phy/mediatek,xfi-pextp.yaml @@ -0,0 +1,80 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/phy/mediatek,xfi-pextp.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: MediaTek XFI PEXTP SerDes PHY + +maintainers: + - Daniel Golle <daniel@makrotopia.org> + +description: + The MediaTek XFI PEXTP SerDes PHY provides the physical SerDes lanes + used by the MediaTek USXGMII PCS. + +properties: + $nodename: + pattern: "^phy@[0-9a-f]+$" + + compatible: + const: mediatek,mt7988-xfi-pextp + + reg: + maxItems: 1 + + clocks: + items: + - description: XFI PHY clock + - description: XFI register clock + + clock-names: + items: + - const: "xfipll" + - const: "topxtal" + + resets: + items: + - description: PEXTP reset + + mediatek,usxgmii-performance-errata: + $ref: /schemas/types.yaml#/definitions/flag + description: + USXGMII0 on MT7988 suffers from a performance problem in 10GBase-R + mode which needs a work-around in the driver. The work-around is + enabled using this flag. + + "#phy-cells": + const: 0 + +required: + - compatible + - reg + - clocks + - clock-names + - resets + - "#phy-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>; + + phy@11f20000 { + compatible = "mediatek,mt7988-xfi-pextp"; + reg = <0 0x11f20000 0 0x10000>; + clocks = <&xfi_pll CLK_XFIPLL_PLL_EN>, + <&topckgen CLK_TOP_XFI_PHY_0_XTAL_SEL>; + clock-names = "xfipll", "topxtal"; + resets = <&watchdog MT7988_TOPRGU_XFI_PEXTP0_GRST>; + mediatek,usxgmii-performance-errata; + #phy-cells = <0>; + }; + }; + +...
Add bindings for the MediaTek PEXTP Ethernet SerDes PHY found in the MediaTek MT7988 SoC which can operate at various interfaces modes: * USXGMII * 10GBase-R * 5GBase-R * 2500Base-X * 1000Base-X * Cisco SGMII (MAC side) Signed-off-by: Daniel Golle <daniel@makrotopia.org> --- .../bindings/phy/mediatek,xfi-pextp.yaml | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/mediatek,xfi-pextp.yaml