On Tue, Dec 17, 2019 at 1:04 PM Marek Vasut marex@denx.de wrote:
On 12/17/19 7:47 PM, Joe Hershberger wrote:
On Tue, Dec 17, 2019 at 11:46 AM Marek Vasut marex@denx.de wrote:
On 12/17/19 5:25 PM, Joe Hershberger wrote:
Hi Marek,
Hi Joe,
On Tue, Dec 17, 2019 at 1:39 AM Marek Vasut marex@denx.de wrote:
On 11/7/19 9:04 PM, Joe Hershberger wrote:
On Thu, Nov 7, 2019 at 1:16 PM Tom Rini trini@konsulko.com wrote: > > On Tue, Nov 05, 2019 at 04:05:11AM +0000, Priyanka Jain wrote: > >> Fix 'mask' calculation in phy_connect() for phy addr '0'. >> 'mask' is getting set to '0xffffffff' for phy addr '0' >> in phy_connect() whereas expected value is '0'. >> >> >> Signed-off-by: Priyanka Jain priyanka.jain@nxp.com > > Reported-by: tetsu-aoki via github
Acked-by: Joe Hershberger joe.hershberger@ni.com
Sadly, this breaks systems where a PHY is at address 0. I have such an STM32MP1 system with LAN8720 PHY and since this patch, I cannot use ethernet. Please revert.
It seems like a case that shouldn't have worked before.
Eh? PHY at address 0 definitely did work before and must work now.
Agreed that a phy at address 0 should work. Not agreed that because the value "0" used to work due to a bug that it must still. Which of these is the statement you are making? Do we already agree or disagree?
I am saying that because a board worked on rc4 and does not work on rc5, this is a bug introduced by this patch in rc5 and must be fixed before the release.
The address 0 is a PHY broadcast address for some PHYs, it's a fixed address for other PHYs. Thus, a PHY at address 0 must work. If this is broken now, it's a bug.
The only thing this patch should change is to not access addresses other than 0. I read the data sheet for the LAN8720 and it doesn't mention anything about any broadcast behavior, so I'm not sure what you're trying to state here.
What about this board requires the mask to be all 'f's, other than specifying the wrong phy address? It seems that in your case the phy address is not actually 0 (or the computed mask would find it), but your board dts may be setting it to 0 as an "unknown" value, but the correct unknown value should be "-1". It seems the issue is with these boards.
Nope, the address is actually configured to 0 in hardware.
Can you double check that?
No, sorry, I know the hardware is fixed to 0. Checking it again will not change this fact.
It seems there is no phy driver for this in U-Boot so the generic behavior is being used. I'm at a disadvantage of not having this board to try. Can you revert this patch and run with debug enabled for drivers/net/phy/phy.c to determine what is happening for this board? I would appreciate you helping with this.
The code as is should compute a mask of "0x01" which should match the offset for address 0. If it really is at 0 in hardware, maybe there is a different bug. Otherwise I don't see how this patch would work for the author.
Reverting this patch makes things work again for me.