linux-kernel - Re: [PATCH 2/2] net: phy: dp83869: Fix RGMII-SGMII and 1000BASE-X

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Wed, 15 May 2024 10:45:17 +0200 (CEST)
From: Thomas Geßler <gessler_t@...eckmann-gmbh.de>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
cc: Thomas Gessler <thomas.gessler@...eckmann-gmbh.de>, 
    Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>, 
    "David S. Miller" <davem@...emloft.net>, 
    Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, 
    Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org, 
    linux-kernel@...r.kernel.org, MD Danish Anwar <danishanwar@...com>, 
    Ravi Gunasekaran <r-gunasekaran@...com>
Subject: Re: [PATCH 2/2] net: phy: dp83869: Fix RGMII-SGMII and 1000BASE-X

On Tue, 14 May 2024, Russell King (Oracle) wrote:
> On Tue, May 14, 2024 at 02:27:28PM +0200, Thomas Gessler wrote:
> > This patch adds
> > device-specific get_features(), config_aneg(), aneg_done(), and
> > read_status() functions for these modes. They are based on the genphy_*
> > versions with the correct registers and fall back to the genphy_*
> > versions for other modes.
> 
> I'm reading this, and wondering... do you have a use case for this,
> or are you adding it because "the PHY supports this" ?

We use this chip in both modes. The driver did not work for the 1000BASE-X
and RGMII-to-SGMII modes out of the box, so I started a thread in the TI
forum and tried to get some info there.

https://e2e.ti.com/support/interface-group/interface/f/interface-forum/1320180/dp83869hm-link-not-working-with-rgmii---sgmii-bridge---1000base-t-using-linux/

This led to the development of this patch, which makes the modes work for
our use cases.

> If you don't have a use case for this, then it would be better not to
> add support for it at this stage, otherwise it may restrict what we can
> do in the future when coming up with a solution for stacked PHY support.

I understand, it would be fine for me to leave this unmerged for now,
especially because of the unclear RGMII-to-SGMII situation, and continue
patching this locally. The only problem I see for other users is that the
driver ostensibly supports all modes the chip supports and can enable each
of them with device-tree settings. Several of the modes, however, can
simply not work because the driver accesses the wrong registers (BMCR/BMSR
instead of the specialized FX_CTRL/FX_STS). This is the main reason why
the custom config_aneg(), read_status() etc. are necessary.

Thomas