On Mon, Feb 18, 2019 at 11:26:30AM +0100, Antoine Tenart wrote: > Hi Russell, > > On Fri, Feb 15, 2019 at 05:12:24PM +0000, Russell King - ARM Linux admin > wrote: > > On Fri, Feb 15, 2019 at 04:32:38PM +0100, Antoine Tenart wrote: > > > The documentation advises to set the XPCS in reset while reconfiguring > > > the serdes lanes. This seems to be a good thing to do, but the PPv2 > > > driver wasn't doing it. This patch fixes it. > > > > Hmm. That statment seems to have some ambiguity in it - we do two > > "reconfigurations" - one may be upon initialisation, where the lane > > is already configured for 10Gbase-KR, and we're re-initialising it > > for the same mode. The other case is when we're switching between > > 10Gbase-KR and SGMII, or as will be the case with 2.5G support for > > the Alaska PHYs, 2500base-X. > > The configuration at the lane at boot time is dependent to the > firmware or bootloader configuration. On the mcbin, the lane may be > configured in 10Gbase-KR, but it could be configured in SGMII as well. > The configuration upon initialization and the re-configuration are quite > similar then, as we might change mode as well at boot time. > > You're right in that we might be re-configuring the lane for the same > exact mode at boot time, if it was already configured in the same mode. > > > Does this apply to reconfiguration of the serdes lane between > > 10Gbase-KR and slower modes? > > This applies only when configuring a line in a 10G mode, > mvpp22_gop_init_10gkr isn't called otherwise. > > When switching to an non-10G mode we might want to put the XPCS in reset > though, which is not done today with this patch.
I'm merely pointing out the discrepency between the commit message and what is actually being done. I'm not particularly concerned about what happens at boot. We have four different transitions a port can go through, all of which reconfigure the serdes lanes: 1. 10gkr -> 10gkr 2. 10gkr -> non-10gkr 3. non-10gkr -> non-10gkr 4. non-10gkr -> 10gkr With this patch, XPCS is only placed into reset during the reconfiguration for cases 1 and 4. Case 3 doesn't matter (the XPCS should already be in reset right?) Case 2 isn't covered, and this leaves a rather big hole. It seems to me that if the documentation states that the XPCS needs to be placed in reset while the serdes is reconfigured, then what should be happening is: - at boot, place the XPCS into reset. - when we configure for 10gkr, release the reset once we've finished configuring the serdes. - when we configure away from 10gkr, place the XPCS back into reset before configuring the serdes. Merely placing the XPCS into reset while we configure the serdes for 10gkr doesn't seem to be "fixing" the driver to conform to your commit message opening statement. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up