[U-Boot] [PATCH] net/phy: Fix phy_connect() for phy addr 0

[Marek Vasut]

On 12/18/19 12:35 AM, Tom Rini wrote:
> On Tue, Dec 17, 2019, 2:04 PM Marek Vasut <marex at 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 at 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 at 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 at 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 at nxp.com>
>>>>>>>>
>>>>>>>> Reported-by: tetsu-aoki via github
>>>>>>>
>>>>>>> Acked-by: Joe Hershberger <joe.hershberger at 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.
>>
>>>>> 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.
>>
>>> 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.
>>
> 
> Ok but breaking other boards again to fix your board is also unacceptable.
> It's not a theoretical case that something else failed previously.

At least it restores the behavior in -rc4 , so that is the last option
if there is no better patch.