On 18/07/2024 20:05, Bartosz Golaszewski wrote:
On Thu, Jul 18, 2024 at 7:42 PM Jon Hunter <jonathanh@xxxxxxxxxx> wrote:
On 18/07/2024 15:59, Bartosz Golaszewski wrote:
...
TBH I only observed the issue on AQR115C. I don't have any other model
to test with. Is it fine to fix it by implementing
aqr115_fill_interface_modes() that would first wait for this register
to return non-0 and then call aqr107_fill_interface_modes()?
I am doing a bit more testing. We have seen a few issues with this PHY
driver and so I am wondering if we also need something similar for the
AQR113C variant too.
Interestingly, the product brief for these PHYs [0] do show that both
the AQR113C and AQR115C both support 10M. So I wonder if it is our
ethernet controller that is not supporting 10M? I will check on this too.
Oh you have an 113c? I didn't get this. Yeah, weird, all docs say it
should support 10M. In fact all AQR PHYs should hence my initial
change.
Yes we have an AQR113C. I agree it should support this, but for whatever
reason this is not advertised. I do see that 10M is advertised as
supported by the network ...
Link partner advertised link modes: 10baseT/Half 10baseT/Full
100baseT/Half 100baseT/Full
1000baseT/Full
My PC that is on the same network supports 10M, but just not this Tegra
device. I am checking to see if this is expected for this device.
I sent a patch for you to test. I think that even if it doesn't fully
fix the issue you're observing, it's worth picking it up as it reduces
the impact of the workaround I introduced.
Thanks! I will test this tonight.
I'll be off next week so I'm sending it quickly with the hope it will be useful.
OK thanks for letting me know.
Another thought I had, which is also quite timely, is that I have
recently been testing a patch [0] as I found that this actually resolves
an issue where we occasionally see our device fail to get an IP address.
This was sent out over a year ago and sadly we failed to follow up :-(
Russell was concerned if this would make the function that was being
changed fail if it did not have the link (if I am understanding the
comments correctly). However, looking at the code now, I see that the
aqr107_read_status() function checks if '!phydev->link' before we poll
the TX ready status, and so I am wondering if this change is OK? From my
testing it does work. I would be interested to know if this may also
resolve your issue?
With this change [0] I have been able to do 500 boots on our board and
verify that the ethernet controller is able to get an IP address every
time. Without this change it would fail to get an IP address anywhere
from 1-100 boots typically.
I will test your patch in the same way, but I am wondering if both are
trying to address the same sort of issue?
The patch you linked does not fix the suspend/resume either. :(
Thanks for testing! I have verified that the patch you sent resolves the
issue introduced by this patch for Tegra. And likewise this patch does
not resolve the long-standing issue (not related to this change) that we
have been observing.
Cheers
Jon
--
nvpublic
