On Thu, Jun 27, 2024 at 08:35:16PM GMT, Bartosz Golaszewski wrote: > On Thu, Jun 27, 2024 at 7:07 PM Andrew Halaney <ahalaney@xxxxxxxxxx> wrote: > > > > Although, this isn't tied to _just_ 2500basex here. If I boot the > > sa8775p-ride (r2 version, with a marvell 88ea1512 phy attached via > > sgmii, not indicating 2500basex) wouldn't all this get exercised? Right > > now the devicetree doesn't indicate inband signalling, but I tried that > > over here with Russell's clean up a week or two ago and things at least > > came up ok (which made me think all the INTEGRATED_PCS stuff wasn't needed, > > and I'm not totally positive my test proved inband signalling worked, > > but I thought it did...): > > > > Am I getting this right? You added `managed = "in-band-status"' to > Rev2 DTS and it still worked? > > > https://lore.kernel.org/netdev/zzevmhmwxrhs5yfv5srvcjxrue2d7wu7vjqmmoyd5mp6kgur54@jvmuv7bxxhqt/ > > > > based on Russell's comments, I feel if I was to use his series over > > there, add 'managed = "in-band-status"' to the dts, and then apply this > > series, the link would not come up anymore. > > This works on rev2/rev1 (no way to tell which one it actually is, shouldn't matter), here's a branch I just whipped up to replicate the setup I had when making the comments in above link: https://gitlab.com/ahalaney/kernel-automotive-9/-/commits/russell-cleanups-and-inband The last commit has some dmesg/ethtool output etc to show things working. I reverted recent changes to stmmac just to apply cleanly. I tried the patches Serge added on top of that series, but that was causing the link to cycle up/down, so I dropped those and went back to just Russell's patches to recreate the setup I had when leaving the comment. I need to try with Serge's stuff again when I find a moment and see if I can work out why the link starts going up/down with those + some compiler fixups and removing INTEGRATED_PCS flags. For what its worth, here's the branch, logs are in the last commit: https://gitlab.com/ahalaney/kernel-automotive-9/-/commits/russell-plus-serge-plus-inband-link-cycles Without Russell's patches the link doesn't come up after switching to 'managed = "in-band-status"' otherwise I'd look into switching the dts to inband signalling now instead of after those cleanups land. > > Because I can confirm that it doesn't on Rev 3. :( > > So to explain myself: I tried to follow Andrew Lunn's suggestion about > unifying this and the existing ethqos_set_func_clk_en() bits as they > seem to address a similar issue. > > I'm working with limited information here as well regarding this issue > so I figured this could work but you're right - if we ever need > in-band signalling, then it won't work. It's late here so let me get > back to it tomorrow. No worries, I understand how this goes, stmmac is tricky and getting information/documentation and understanding it can be tough. I appreciate you trying to get this squared away. > > > Total side note, but I'm wondering if the sa8775p-ride dts should > > specify 'managed = "in-band-status"'. > > > > I'll check this at the source. > Thanks!