Re: [PATCH RFC net-next 1/2] net: pcs: xpcs: Add special code to operate in Microchip KSZ9477 switch

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Tue, Jan 28, 2025 at 12:21:28PM +0200, Vladimir Oltean wrote:
> On Tue, Jan 28, 2025 at 09:24:45AM +0000, Russell King (Oracle) wrote:
> > On Mon, Jan 27, 2025 at 07:32:25PM -0800, Tristram.Ha@microchip.com wrote:
> > > For 1000BaseX mode setting neg_mode to false works, but that does not
> > > work for SGMII mode.  Setting 0x18 value in register 0x1f8001 allows
> > > 1000BaseX mode to work with auto-negotiation enabled.
> > 
> > I'm not sure (a) exactly what the above paragraph is trying to tell me,
> > and (b) why setting the AN control register to 0x18, which should only
> > affect SGMII, has an effect on 1000BASE-X.
> > 
> > Note that a config word formatted for SGMII can result in a link with
> > 1000BASE-X to come up, but it is not correct. So, I highly recommend you
> > check the config word sent by the XPCS to the other end of the link.
> > Bit 0 of that will tell you whether it is SGMII-formatted or 802.3z
> > formatted.
> 
> I, too, am concerned about the sentence "setting neg_mode to false works".
> If this is talking about the only neg_mode field that is a boolean, aka
> struct phylink_pcs :: neg_mode, then setting it to false is not
> something driver customizable, it should be true for API compliance,
> and all that remains false in current kernel trees will eventually get
> converted to true, AFAIU. If 1000BaseX works by setting xpcs->pcs.neg_mode
> to false and not modifying anything else, it should be purely a
> coincidence that it "works", since that makes the driver comparisons
> with PHYLINK_PCS_NEG_* constants meaningless.

Another related issue that concerns me is:

         * Note: Since it is MAC side SGMII, there is no need to set
         *       SR_MII_AN_ADV. MAC side SGMII receives AN Tx Config from
         *       PHY about the link state change after C28 AN is completed
         *       between PHY and Link Partner. There is also no need to
         *       trigger AN restart for MAC-side SGMII.

However, 2a22b7ae2fa3 ("net: pcs: xpcs: adapt Wangxun NICs for SGMII
mode") added support for switching this to PHY-side SGMII, so this
comment is no longer true.

Again, I still wonder whether PHY-side is some kind of hack despite
my queries during the review of that change. Sadly, I didn't catch
that comment (and until recently, I didn't have any documentation
on these registers. I now have the KS9477 and SJA1105 manuals that
document some of the register set.)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!