Message-ID:
 <DM3PR11MB8736B75C8846DE3816F42935ECB12@DM3PR11MB8736.namprd11.prod.outlook.com>
Date: Sat, 12 Apr 2025 00:18:33 +0000
From: <Tristram.Ha@...rochip.com>
To: <olteanv@...il.com>
CC: <linux@...linux.org.uk>, <andrew@...n.ch>, <davem@...emloft.net>,
	<edumazet@...gle.com>, <hkallweit1@...il.com>, <kuba@...nel.org>,
	<netdev@...r.kernel.org>, <pabeni@...hat.com>,
	<UNGLinuxDriver@...rochip.com>, <Woojung.Huh@...rochip.com>
Subject: RE: [PATCH RFC net-next 0/4] net: xpcs: cleanups and partial support
 for KSZ9477

> Subject: Re: [PATCH RFC net-next 0/4] net: xpcs: cleanups and partial support for
> KSZ9477
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content
> is safe
> 
> Hi Tristram,
> 
> On Tue, Mar 18, 2025 at 07:59:07PM +0000, Tristram.Ha@...rochip.com wrote:
> > Sorry for the long delay.  After discussing with the Microchip design
> > team we come up with this explanation for setting the
> > DW_VR_MII_TX_CONFIG_PHY_SIDE_SGMII (bit 3) of DW_VR_MII_AN_CTRL
> (0x8001)
> > register in 1000Base-X mode to make it work with AN on.
> >
> > KSZ9477 has the older version of 1000Base-X Synopsys IP which works in
> > 1000Base-X mode without AN.
> 
> I was unaware of the existence of such Synopsys IP. In which relevant
> way is it "older"? Does it specifically claim it supports 1000Base-X,
> but only without AN? Or if not, is it an SGMII-only base IP, then? The
> two are compatible when AN is disabled and the SGMII IP is configured
> for 1Gbps, and can be mistaken for one another.
> 
> You're making it sound as if "1000Base-X" support was patched by the
> Microchip integration over a base PCS IP which did not support it.

The 1000Base-X config word is sent correctly by KSZ9477 and link partner
can receive it.  This is verified by changing the register MII_ADVERTISE
and observing the link partner's MII_LPA register for the same value with
additional bit 0x4000 set.

The KSZ9477 1000Base-X AN is broken and requires the workaround mentioned
in the app note.  Mainly the MII_ADVERTISE register needs to be written
once after PCS mode is changed before restarting the auto-negotiation for
the config word to be sent correctly.

> > When AN is on the port does not pass traffic because it does not
> > detect a link.
> 
> Which port does not detect a link? The KSZ9477 port or its link partner?

The link detection issue is for the local port device.  The link can be
detected properly, and the interrupt can even signal the link up is
completed.  It is just the port will not forward traffic as the link
status is not passed to the local device.  Setting the 2 bits mentioned
before allows the port to know about the link.  This is just a side
effect and is not an intended design as it is really the hardware's
problem not to pass the link status to the local device.

> > Setting DW_VR_MII_TX_CONFIG_PHY_SIDE_SGMII allows the link to be
> > turned on by either setting DW_VR_MII_SGMII_LINK_STS (bit 4) or
> > DW_VR_MII_DIG_CTRL1_PHY_MODE_CTRL (bit 0) in DW_VR_MII_DIG_CTRL1
> > (0x8000) register.  After that the port can pass traffic.
> 
> Can you comment on whether Microchip has given these bits some
> integration-specific meaning, or whether their meaning from the base IP,
> summarized by me in this table taken from the XPCS data book, has been
> preserved unchanged?
> https://lore.kernel.org/netdev/20250128152324.3p2ccnxoz5xta7ct@skbuf/
> 
> So far, the only noted fact is that they take effect also for
> PCS_MODE=0b00 (1000Base-X), and not just for PCS_MODE=0b10 (SGMII),
> as originally intended in the base IP. But we don't exactly know what
> they do. Just that the Microchip IP wants them set to one.
> 
> If their meaning is otherwise the same, all data available to me points
> to the conclusion that the "1000Base-X with autoneg on" mode in KSZ9477
> is actually SGMII with a config word customized by software, and with
> some conditions commented out from the base IP, to allow the code word
> to be customizable even in PCS_MODE=0b00.
> 
> If the above conclusion is true, I think we need to pay more attention
> at whether the transmitted config word is truly 1000Base-X compatible,
> as Russell pointed out here:
> https://lore.kernel.org/netdev/Z5iiXWkhm2OvbjOx@shell.armlinux.org.uk/#t
> The discussion there got pretty involved and branched out into other
> topics, but I couldn't find a response from you on that specific second
> paragraph.
> 
> > This is still a specific KSZ9477 problem so it may be safer to put this
> > code under "if (xpcs->info.pma == MICROCHIP_KSZ9477_PMD_ID)" check.
> 
> If the meaning of these fields was not kept 100% intact by Microchip
> during their integration, including the context in which they are valid,
> then I 100% agree. But I would still like an answer to the questions above.

Since this is a KSZ9477 specific problem is it okay to just program those
bits inside the KSZ9477 DSA driver when the PCS mode is changed from
SGMII to 1000Base-X?  Then the XPCS driver does not need to concern with
this code change.  If a SFP does not work then it is the fault of KSZ9477
hardware and its driver.

Powered by blists - more mailing lists