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

[Thomas Geßler]

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