hw/net: cadence_gem: Fix MDIO_OP_xxx values

[PATCH for 8.2.1] hw/net: cadence_gem: Fix MDIO_OP_xxx values

Posted by Bin Meng 2 years, 1 month ago

Testing upstream U-Boot with 'sifive_u' machine we see:

  => dhcp
  ethernet@10090000: PHY present at 0
  Could not get PHY for ethernet@10090000: addr 0
  phy_connect failed

This has been working till QEMU 8.1 but broken since QEMU 8.2.

Fixes: 1b09eeb122aa ("hw/net/cadence_gem: use FIELD to describe PHYMNTNC register fields")
Reported-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Signed-off-by: Bin Meng <bmeng@tinylab.org>

---

 hw/net/cadence_gem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 296bba238e..472ce9c8cf 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -199,8 +199,8 @@ REG32(PHYMNTNC, 0x34) /* Phy Maintenance reg */
     FIELD(PHYMNTNC, PHY_ADDR, 23, 5)
     FIELD(PHYMNTNC, OP, 28, 2)
     FIELD(PHYMNTNC, ST, 30, 2)
-#define MDIO_OP_READ    0x3
-#define MDIO_OP_WRITE   0x2
+#define MDIO_OP_READ    0x2
+#define MDIO_OP_WRITE   0x1
 
 REG32(RXPAUSE, 0x38) /* RX Pause Time reg */
 REG32(TXPAUSE, 0x3c) /* TX Pause Time reg */
-- 
2.34.1

Re: [PATCH for 8.2.1] hw/net: cadence_gem: Fix MDIO_OP_xxx values

Posted by Michael Tokarev 2 years, 1 month ago

02.01.2024 17:18, Bin Meng:
> Testing upstream U-Boot with 'sifive_u' machine we see:
> 
>    => dhcp
>    ethernet@10090000: PHY present at 0
>    Could not get PHY for ethernet@10090000: addr 0
>    phy_connect failed
> 
> This has been working till QEMU 8.1 but broken since QEMU 8.2.
> 
> Fixes: 1b09eeb122aa ("hw/net/cadence_gem: use FIELD to describe PHYMNTNC register fields")
> Reported-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>

Applied to trivial-patches tree, and marked to be picked up for stable.

Thanks,

/mjt

Re: [PATCH for 8.2.1] hw/net: cadence_gem: Fix MDIO_OP_xxx values

Posted by Philippe Mathieu-Daudé 2 years, 1 month ago

On 2/1/24 15:18, Bin Meng wrote:
> Testing upstream U-Boot with 'sifive_u' machine we see:
> 
>    => dhcp
>    ethernet@10090000: PHY present at 0
>    Could not get PHY for ethernet@10090000: addr 0
>    phy_connect failed
> 
> This has been working till QEMU 8.1 but broken since QEMU 8.2.

s/till/until/?

> Fixes: 1b09eeb122aa ("hw/net/cadence_gem: use FIELD to describe PHYMNTNC register fields")
> Reported-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> 
> ---
> 
>   hw/net/cadence_gem.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index 296bba238e..472ce9c8cf 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -199,8 +199,8 @@ REG32(PHYMNTNC, 0x34) /* Phy Maintenance reg */
>       FIELD(PHYMNTNC, PHY_ADDR, 23, 5)
>       FIELD(PHYMNTNC, OP, 28, 2)
>       FIELD(PHYMNTNC, ST, 30, 2)
> -#define MDIO_OP_READ    0x3
> -#define MDIO_OP_WRITE   0x2
> +#define MDIO_OP_READ    0x2
> +#define MDIO_OP_WRITE   0x1
>   
>   REG32(RXPAUSE, 0x38) /* RX Pause Time reg */
>   REG32(TXPAUSE, 0x3c) /* TX Pause Time reg */

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH for 8.2.1] hw/net: cadence_gem: Fix MDIO_OP_xxx values

Posted by Heinrich Schuchardt 2 years, 1 month ago

On 1/2/24 16:08, Philippe Mathieu-Daudé wrote:
> On 2/1/24 15:18, Bin Meng wrote:
>> Testing upstream U-Boot with 'sifive_u' machine we see:
>>
>>    => dhcp
>>    ethernet@10090000: PHY present at 0
>>    Could not get PHY for ethernet@10090000: addr 0
>>    phy_connect failed
>>
>> This has been working till QEMU 8.1 but broken since QEMU 8.2.
>
> s/till/until/?

These are synonyms. Till is more informal. No need to change.

>
>> Fixes: 1b09eeb122aa ("hw/net/cadence_gem: use FIELD to describe
>> PHYMNTNC register fields")
>> Reported-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> Signed-off-by: Bin Meng <bmeng@tinylab.org>
>>
>> ---
>>
>>   hw/net/cadence_gem.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
>> index 296bba238e..472ce9c8cf 100644
>> --- a/hw/net/cadence_gem.c
>> +++ b/hw/net/cadence_gem.c
>> @@ -199,8 +199,8 @@ REG32(PHYMNTNC, 0x34) /* Phy Maintenance reg */
>>       FIELD(PHYMNTNC, PHY_ADDR, 23, 5)
>>       FIELD(PHYMNTNC, OP, 28, 2)
>>       FIELD(PHYMNTNC, ST, 30, 2)
>> -#define MDIO_OP_READ    0x3
>> -#define MDIO_OP_WRITE   0x2
>> +#define MDIO_OP_READ    0x2
>> +#define MDIO_OP_WRITE   0x1
>>   REG32(RXPAUSE, 0x38) /* RX Pause Time reg */
>>   REG32(TXPAUSE, 0x3c) /* TX Pause Time reg */
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>

Thank you Bin for the fix.

With the fix I was able to download a file via TFTP to U-Boot
sifive_unleashed_defconfig on the emulated board. Cf.
docs/system/riscv/sifive_u.rst.

Tested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Re: [PATCH for 8.2.1] hw/net: cadence_gem: Fix MDIO_OP_xxx values

Posted by Philippe Mathieu-Daudé 2 years, 1 month ago

On 2/1/24 19:32, Heinrich Schuchardt wrote:
> On 1/2/24 16:08, Philippe Mathieu-Daudé wrote:
>> On 2/1/24 15:18, Bin Meng wrote:
>>> Testing upstream U-Boot with 'sifive_u' machine we see:
>>>
>>>    => dhcp
>>>    ethernet@10090000: PHY present at 0
>>>    Could not get PHY for ethernet@10090000: addr 0
>>>    phy_connect failed
>>>
>>> This has been working till QEMU 8.1 but broken since QEMU 8.2.
>>
>> s/till/until/?
> 
> These are synonyms. Till is more informal. No need to change.

Not obvious for non-native/fluent informal English speakers.

> 
>>
>>> Fixes: 1b09eeb122aa ("hw/net/cadence_gem: use FIELD to describe
>>> PHYMNTNC register fields")
>>> Reported-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>> Signed-off-by: Bin Meng <bmeng@tinylab.org>
>>>
>>> ---
>>>
>>>   hw/net/cadence_gem.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)