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@xxxxxxxxxxxxxx> >>> --- >>> 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
