Messages in this thread | | | Date | Tue, 7 Jun 2022 15:45:32 +0100 | From | "Russell King (Oracle)" <> | Subject | Re: [PATCH net] net: dsa: realtek: rtl8365mb: fix GMII caps for ports with internal PHY |
| |
On Tue, Jun 07, 2022 at 02:17:44PM +0000, Alvin Šipraga wrote: > On Tue, Jun 07, 2022 at 03:05:40PM +0100, Russell King (Oracle) wrote: > > On Tue, Jun 07, 2022 at 10:52:48AM -0300, Luiz Angelo Daros de Luca wrote: > > > > > > Luiz, Russel: > > > > > > > > > > > > Commit a5dba0f207e5 ought to have had a Fixes: tag I think, because it > > > > > > claims to have been fixing a regression in the net-next tree - is that > > > > > > right? I seem to have missed both referenced commits when they were > > > > > > posted and never hit this issue personally. I only found things now > > > > > > during some other refactoring and the test for GMII looked weird to me > > > > > > so I went and investigated. > > > > > > > > > > > > Could you please help me identify that Fixes: tag? Just for my own > > > > > > understanding of what caused this added requirement for GMII on ports > > > > > > with internal PHY. > > > > > > > > > > I have absolutely no idea. I don't think any "requirement" has ever been > > > > > added - phylib has always defaulted to GMII, so as the driver stood when > > > > > it was first submitted on Oct 18 2021, I don't see how it could have > > > > > worked, unless the DT it was being tested with specified a phy-mode of > > > > > "internal". As you were the one who submitted it, you would have a > > > > > better idea. > > > > > > > > > > The only suggestion I have is to bisect to find out exactly what caused > > > > > the GMII vs INTERNAL issue to crop up. > > > > > > > > Alright, thanks for the quick response. Maybe Luiz has a better idea, otherwise > > > > I will try bisecting if I find the time. > > > > > > I don't know. I just got hit by the issue after a rebase (sorry, I > > > don't know exactly from which commit I was rebasing). > > > But I did test the net (!-next) and left a working commit note. You > > > can diff 3dd7d40b43..a5dba0f20. > > > If I'm to guess, I would blame: > > > > > > 21bd64bd717de: net: dsa: consolidate phylink creation > > > > Why do you suspect that commit? I fail to see any functional change in > > that commit that would cause the problem. > > Agree, seems like the referenced commit makes no functional change. > > But thanks for the range of commits Luiz, I found one that looks like the > culprit. It's small so I will reproduce the whole thing below. Will test later.
This one I agree could well be the culpret, but it means that the original premise that PHY_INTERFACE_MODE_INTERNAL was being used is incorrect - it's actually been relying on using PHY_INTERFACE_MODE_NA.
It instead means that PHY_INTERFACE_MODE_NA was being used, which really isn't good, because PHY_INTERFACE_MODE_NA internally inside phylink has always had a special meaning - that being with the validate step which has been used to get _all_ possible modes from the MAC. This was never intended to be used for anything except phylink's internal use to retrieve that information from the MAC driver to make decisions about what mode(s) a SFP should use.
So yes, this is most likely the culpret, and if proven, please use it for the Fixes: tag for any fixes to drivers that incorrectly relied upon that behaviour.
Thanks.
-- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
| |