diff mbox series

[v2,1/2] dt-bindings: phy: mediatek,mt7988-xfi-tphy: add new bindings

Message ID 3251ac3db1a739e0c18ded0a824edae981c1e2df.1707153425.git.daniel@makrotopia.org
State Superseded
Headers show
Series [v2,1/2] dt-bindings: phy: mediatek,mt7988-xfi-tphy: add new bindings | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success

Commit Message

Daniel Golle Feb. 5, 2024, 5:28 p.m. UTC 
Add bindings for the MediaTek XFI Ethernet SerDes T-PHY found in the
MediaTek MT7988 SoC which can operate at various interfaces modes:

via USXGMII PCS:
 * USXGMII
 * 10GBase-R
 * 5GBase-R

via LynxI SGMII PCS:
 * 2500Base-X
 * 1000Base-X
 * Cisco SGMII (MAC side)

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v2: unify filename and compatible as requested

 .../phy/mediatek,mt7988-xfi-tphy.yaml         | 77 +++++++++++++++++++
 1 file changed, 77 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/mediatek,mt7988-xfi-tphy.yaml

Comments

AngeloGioacchino Del Regno Feb. 6, 2024, 9:28 a.m. UTC | #1 
Il 05/02/24 18:29, Daniel Golle ha scritto:
> Add driver for MediaTek's XFI T-PHY which can be found in the MT7988
> SoC. The XFI T-PHY is a 10 Gigabit/s Ethernet SerDes PHY with muxes on
> the internal side to be used with either USXGMII PCS or LynxI PCS,
> depending on the selected PHY interface mode.
> 
> The PHY can operates only in PHY_MODE_ETHERNET, the submode is one of
> PHY_INTERFACE_MODE_* corresponding to the supported modes:
> 
>   * USXGMII                 \
>   * 10GBase-R                }- USXGMII PCS - XGDM  \
>   * 5GBase-R                /                        \
>                                                       }- Ethernet MAC
>   * 2500Base-X              \                        /
>   * 1000Base-X               }- LynxI PCS - GDM     /
>   * Cisco SGMII (MAC side)  /
> 
> In order to work-around a performance issue present on the first of
> two XFI T-PHYs present in MT7988, special tuning is applied which can be
> selected by adding the 'mediatek,usxgmii-performance-errata' property to
> the device tree node.
> 
> There is no documentation for most registers used for the
> analog/tuning part, however, most of the registers have been partially
> reverse-engineered from MediaTek's SDK implementation (an opaque
> sequence of 32-bit register writes) and descriptions for all relevant
> digital registers and bits such as resets and muxes have been supplied
> by MediaTek.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
AngeloGioacchino Del Regno Feb. 6, 2024, 9:28 a.m. UTC | #2 
Il 05/02/24 18:28, Daniel Golle ha scritto:
> Add bindings for the MediaTek XFI Ethernet SerDes T-PHY found in the
> MediaTek MT7988 SoC which can operate at various interfaces modes:
> 
> via USXGMII PCS:
>   * USXGMII
>   * 10GBase-R
>   * 5GBase-R
> 
> via LynxI SGMII PCS:
>   * 2500Base-X
>   * 1000Base-X
>   * Cisco SGMII (MAC side)
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Krzysztof Kozlowski Feb. 6, 2024, 10:53 a.m. UTC | #3 
On 05/02/2024 18:28, Daniel Golle wrote:
> Add bindings for the MediaTek XFI Ethernet SerDes T-PHY found in the
> MediaTek MT7988 SoC which can operate at various interfaces modes:
> 
> via USXGMII PCS:
>  * USXGMII
>  * 10GBase-R
>  * 5GBase-R
> 
> via LynxI SGMII PCS:
>  * 2500Base-X
>  * 1000Base-X
>  * Cisco SGMII (MAC side)
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
> v2: unify filename and compatible as requested

Several comments, from me and Rob, were ignored. Please respond to them.

Best regards,
Krzysztof
Daniel Golle Feb. 6, 2024, 3:31 p.m. UTC | #4 
Hi Krzysztof,

On Tue, Feb 06, 2024 at 11:53:55AM +0100, Krzysztof Kozlowski wrote:
> On 05/02/2024 18:28, Daniel Golle wrote:
> > Add bindings for the MediaTek XFI Ethernet SerDes T-PHY found in the
> > MediaTek MT7988 SoC which can operate at various interfaces modes:
> > 
> > via USXGMII PCS:
> >  * USXGMII
> >  * 10GBase-R
> >  * 5GBase-R
> > 
> > via LynxI SGMII PCS:
> >  * 2500Base-X
> >  * 1000Base-X
> >  * Cisco SGMII (MAC side)
> > 
> > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > ---
> > v2: unify filename and compatible as requested
> 
> Several comments, from me and Rob, were ignored. Please respond to them.

I'm sorry if I have missed something. I just checked again on
patchwork, just in case I would have missed an email reply to this or
any of the preceding posts of this patch as part of the old series
going to netdev.

Comments you have made which I have addressed:
 - removed $nodename
 - use compatible as filename

And the only thing I found that I didn't either fix or reply to is this:
> Can you explain what is this issue and errata about (except performance)?

Not overwriting that (undocumented) value in that (undocumented)
register results in 10GBase-R having performance issues according to a
commit in MediaTek's SDK, see here:

https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/a500d94cd%5E%21/#F0

Maybe Bc or SkyLake of MediaTek (added to Cc) can explain this in more
detail?


What I did miss was Rob's comment at the very bottom of this reply:
> What is PEXTP?

I can again only answer by referencing to MediaTek's SDK sources:

https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/refs/heads/master/21.02/files/target/linux/mediatek/files-5.4/drivers/net/ethernet/mediatek/mtk_sgmii.c#96

Here this reset is called XFI_PEXTP0_GRST.

I personally find that name confusing (as this PHY has nothing to do with
_P_ci _EX_press) and have tried to get rid of it where it isn't either part
of official documentation or already merged drivers (like Sam's clock driver).

If there have been any other issues with this patch which I'm not aware
of, please point them out to me.


Thank you


Best regards


Daniel
Bc-bocun Chen (陳柏村) Feb. 7, 2024, 7:14 a.m. UTC | #5 
Hi Daniel,
Hi Krzysztof,

On Tue, 2024-02-06 at 15:31 +0000, Daniel Golle wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  Hi Krzysztof,
> 
> On Tue, Feb 06, 2024 at 11:53:55AM +0100, Krzysztof Kozlowski wrote:
> > On 05/02/2024 18:28, Daniel Golle wrote:
> > > Add bindings for the MediaTek XFI Ethernet SerDes T-PHY found in
> the
> > > MediaTek MT7988 SoC which can operate at various interfaces
> modes:
> > > 
> > > via USXGMII PCS:
> > >  * USXGMII
> > >  * 10GBase-R
> > >  * 5GBase-R
> > > 
> > > via LynxI SGMII PCS:
> > >  * 2500Base-X
> > >  * 1000Base-X
> > >  * Cisco SGMII (MAC side)
> > > 
> > > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > > ---
> > > v2: unify filename and compatible as requested
> > 
> > Several comments, from me and Rob, were ignored. Please respond to
> them.
> 
> I'm sorry if I have missed something. I just checked again on
> patchwork, just in case I would have missed an email reply to this or
> any of the preceding posts of this patch as part of the old series
> going to netdev.
> 
> Comments you have made which I have addressed:
>  - removed $nodename
>  - use compatible as filename
> 
> And the only thing I found that I didn't either fix or reply to is
> this:
> > Can you explain what is this issue and errata about (except
> performance)?
> 
> Not overwriting that (undocumented) value in that (undocumented)
> register results in 10GBase-R having performance issues according to
> a
> commit in MediaTek's SDK, see here:
> 
> 
https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/a500d94cd%5E%21/#F0
> 
> Maybe Bc or SkyLake of MediaTek (added to Cc) can explain this in
> more
> detail?

Our QA team has identified a potential performance issue when
connecting XFI Port0 with an SFP optical fiber, which may violate the
SFP specification.
Therefore, we have adjusted the PHYA setting for this port.

> 
> What I did miss was Rob's comment at the very bottom of this reply:
> > What is PEXTP?
> 
> I can again only answer by referencing to MediaTek's SDK sources:
> 
> 
https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/refs/heads/master/21.02/files/target/linux/mediatek/files-5.4/drivers/net/ethernet/mediatek/mtk_sgmii.c#96
> 
> Here this reset is called XFI_PEXTP0_GRST.
> 
> I personally find that name confusing (as this PHY has nothing to do
> with
> _P_ci _EX_press) and have tried to get rid of it where it isn't
> either part
> of official documentation or already merged drivers (like Sam's clock
> driver).
> 
> If there have been any other issues with this patch which I'm not
> aware
> of, please point them out to me.
> 
> 
> Thank you
> 
> 
> Best regards
> 
> 
> Daniel

Thanks
BC
Krzysztof Kozlowski Feb. 7, 2024, 3:07 p.m. UTC | #6 
On 06/02/2024 16:31, Daniel Golle wrote:
> Hi Krzysztof,
> 
> On Tue, Feb 06, 2024 at 11:53:55AM +0100, Krzysztof Kozlowski wrote:
>> On 05/02/2024 18:28, Daniel Golle wrote:
>>> Add bindings for the MediaTek XFI Ethernet SerDes T-PHY found in the
>>> MediaTek MT7988 SoC which can operate at various interfaces modes:
>>>
>>> via USXGMII PCS:
>>>  * USXGMII
>>>  * 10GBase-R
>>>  * 5GBase-R
>>>
>>> via LynxI SGMII PCS:
>>>  * 2500Base-X
>>>  * 1000Base-X
>>>  * Cisco SGMII (MAC side)
>>>
>>> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
>>> ---
>>> v2: unify filename and compatible as requested
>>
>> Several comments, from me and Rob, were ignored. Please respond to them.
> 
> I'm sorry if I have missed something. I just checked again on
> patchwork, just in case I would have missed an email reply to this or
> any of the preceding posts of this patch as part of the old series
> going to netdev.
> 
> Comments you have made which I have addressed:
>  - removed $nodename
>  - use compatible as filename
> 
> And the only thing I found that I didn't either fix or reply to is this:
>> Can you explain what is this issue and errata about (except performance)?
> 
> Not overwriting that (undocumented) value in that (undocumented)
> register results in 10GBase-R having performance issues according to a
> commit in MediaTek's SDK, see here:
> 
> https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/a500d94cd%5E%21/#F0
> 
> Maybe Bc or SkyLake of MediaTek (added to Cc) can explain this in more
> detail?
> 
> 
> What I did miss was Rob's comment at the very bottom of this reply:
>> What is PEXTP?
> 
> I can again only answer by referencing to MediaTek's SDK sources:
> 
> https://git01.mediatek.com/plugins/gitiles/openwrt/feeds/mtk-openwrt-feeds/+/refs/heads/master/21.02/files/target/linux/mediatek/files-5.4/drivers/net/ethernet/mediatek/mtk_sgmii.c#96
> 
> Here this reset is called XFI_PEXTP0_GRST.
> 
> I personally find that name confusing (as this PHY has nothing to do with
> _P_ci _EX_press) and have tried to get rid of it where it isn't either part
> of official documentation or already merged drivers (like Sam's clock driver).
> 
> If there have been any other issues with this patch which I'm not aware
> of, please point them out to me.

These both cases should be explained in the binding somehow.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/phy/mediatek,mt7988-xfi-tphy.yaml b/Documentation/devicetree/bindings/phy/mediatek,mt7988-xfi-tphy.yaml
new file mode 100644
index 0000000000000..e897118dcf7e6
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/mediatek,mt7988-xfi-tphy.yaml
@@ -0,0 +1,77 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/mediatek,mt7988-xfi-tphy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek XFI T-PHY
+
+maintainers:
+  - Daniel Golle <daniel@makrotopia.org>
+
+description:
+  The MediaTek XFI SerDes T-PHY provides the physical SerDes lanes
+  used by the (10G/5G) USXGMII PCS and (1G/2.5G) LynxI PCS found in
+  MediaTek's 10G-capabale MT7988 SoC.
+
+properties:
+  compatible:
+    const: mediatek,mt7988-xfi-tphy
+
+  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:
+      One instance of the T-PHY 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>
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      phy@11f20000 {
+        compatible = "mediatek,mt7988-xfi-tphy";
+        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 14>;
+        mediatek,usxgmii-performance-errata;
+        #phy-cells = <0>;
+      };
+    };
+
+...