From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joe Hershberger Date: Wed, 18 Dec 2019 02:11:48 +0000 Subject: [U-Boot] [PATCH] net/phy: Fix phy_connect() for phy addr 0 In-Reply-To: References: <20191105040439.23450-1-priyanka.jain@nxp.com> <20191107191543.GK19317@bill-the-cat> <423b0427-c8b3-8e2e-d64c-ae06d79214b6@denx.de> <4cecdc1d-cf11-e26f-0bb9-deeeb2bd4d95@denx.de> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Tue, Dec 17, 2019 at 1:04 PM Marek Vasut wrote: > > On 12/17/19 7:47 PM, Joe Hershberger wrote: > > On Tue, Dec 17, 2019 at 11:46 AM Marek Vasut 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 wrote: > >>>> > >>>> On 11/7/19 9:04 PM, Joe Hershberger wrote: > >>>>> On Thu, Nov 7, 2019 at 1:16 PM Tom Rini 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 > >>>>>> > >>>>>> Reported-by: tetsu-aoki via github > >>>>> > >>>>> Acked-by: Joe Hershberger > >>>> > >>>> 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.