diff mbox series

[PULL,39/41] hw/net/cadence_gem: use FIELD to describe PHYMNTNC register fields

Message ID 20231027143942.3413881-40-peter.maydell@linaro.org
State Accepted
Commit 1b09eeb122aaf1666a9c5aa789eb219443772685
Headers show
Series [PULL,01/41] target/arm: Correct minor errors in Cortex-A710 definition | expand

Commit Message

Peter Maydell Oct. 27, 2023, 2:39 p.m. UTC 
From: Luc Michel <luc.michel@amd.com>

Use the FIELD macro to describe the PHYMNTNC register fields.

Signed-off-by: Luc Michel <luc.michel@amd.com>
Reviewed-by: sai.pavan.boddu@amd.com
Message-id: 20231017194422.4124691-10-luc.michel@amd.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/net/cadence_gem.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

Comments

Guenter Roeck Jan. 19, 2024, 10:32 p.m. UTC | #1 
Hi,

On Fri, Oct 27, 2023 at 03:39:40PM +0100, Peter Maydell wrote:
> From: Luc Michel <luc.michel@amd.com>
> 
> Use the FIELD macro to describe the PHYMNTNC register fields.
> 
> Signed-off-by: Luc Michel <luc.michel@amd.com>
> Reviewed-by: sai.pavan.boddu@amd.com
> Message-id: 20231017194422.4124691-10-luc.michel@amd.com
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

With qemu v8.2.0 and this patch in place, I get the following error when trying
to enable the network interface on the xilinx-zynq-a9 emulation.

macb e000b000.ethernet eth0: validation of  with support 00,00000000,00005000,00006000 and advertisement 00,00000000,00000000,00000000 failed: -EINVAL
macb e000b000.ethernet eth0: Could not attach PHY (-22)

The problem is gone after reverting this patch. Note that I also had
to revert "hw/net/cadence_gem: perform PHY access on write only", but
that alone did not fix the problem.

Guenter
Guenter Roeck Jan. 19, 2024, 10:36 p.m. UTC | #2 
On Fri, Jan 19, 2024 at 02:32:47PM -0800, Guenter Roeck wrote:
> Hi,
> 
> On Fri, Oct 27, 2023 at 03:39:40PM +0100, Peter Maydell wrote:
> > From: Luc Michel <luc.michel@amd.com>
> > 
> > Use the FIELD macro to describe the PHYMNTNC register fields.
> > 
> > Signed-off-by: Luc Michel <luc.michel@amd.com>
> > Reviewed-by: sai.pavan.boddu@amd.com
> > Message-id: 20231017194422.4124691-10-luc.michel@amd.com
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> 
> With qemu v8.2.0 and this patch in place, I get the following error when trying
> to enable the network interface on the xilinx-zynq-a9 emulation.
> 
> macb e000b000.ethernet eth0: validation of  with support 00,00000000,00005000,00006000 and advertisement 00,00000000,00000000,00000000 failed: -EINVAL
> macb e000b000.ethernet eth0: Could not attach PHY (-22)
> 
> The problem is gone after reverting this patch. Note that I also had
> to revert "hw/net/cadence_gem: perform PHY access on write only", but
> that alone did not fix the problem.
> 

Never mind, it looks like the problem was fixed with commit 0c7ffc977195c1.
Sorry for the noise.

Guenter
diff mbox series

Patch

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index dffcc64df25..373d3ee0712 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -194,6 +194,14 @@  REG32(IDR, 0x2c) /* Interrupt Disable reg */
 REG32(IMR, 0x30) /* Interrupt Mask reg */
 
 REG32(PHYMNTNC, 0x34) /* Phy Maintenance reg */
+    FIELD(PHYMNTNC, DATA, 0, 16)
+    FIELD(PHYMNTNC, REG_ADDR, 18, 5)
+    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
+
 REG32(RXPAUSE, 0x38) /* RX Pause Time reg */
 REG32(TXPAUSE, 0x3c) /* TX Pause Time reg */
 REG32(TXPARTIALSF, 0x40) /* TX Partial Store and Forward */
@@ -342,13 +350,6 @@  REG32(TYPE2_COMPARE_0_WORD_1, 0x704)
 
 
 
-#define GEM_PHYMNTNC_OP_R      0x20000000 /* read operation */
-#define GEM_PHYMNTNC_OP_W      0x10000000 /* write operation */
-#define GEM_PHYMNTNC_ADDR      0x0F800000 /* Address bits */
-#define GEM_PHYMNTNC_ADDR_SHFT 23
-#define GEM_PHYMNTNC_REG       0x007C0000 /* register bits */
-#define GEM_PHYMNTNC_REG_SHIFT 18
-
 /* Marvell PHY definitions */
 #define BOARD_PHY_ADDRESS    0 /* PHY address we will emulate a device at */
 
@@ -1541,12 +1542,12 @@  static uint64_t gem_read(void *opaque, hwaddr offset, unsigned size)
         /* The interrupts get updated at the end of the function. */
         break;
     case R_PHYMNTNC:
-        if (retval & GEM_PHYMNTNC_OP_R) {
+        if (FIELD_EX32(retval, PHYMNTNC, OP) == MDIO_OP_READ) {
             uint32_t phy_addr, reg_num;
 
-            phy_addr = (retval & GEM_PHYMNTNC_ADDR) >> GEM_PHYMNTNC_ADDR_SHFT;
+            phy_addr = FIELD_EX32(retval, PHYMNTNC, PHY_ADDR);
             if (phy_addr == s->phy_addr) {
-                reg_num = (retval & GEM_PHYMNTNC_REG) >> GEM_PHYMNTNC_REG_SHIFT;
+                reg_num = FIELD_EX32(retval, PHYMNTNC, REG_ADDR);
                 retval &= 0xFFFF0000;
                 retval |= gem_phy_read(s, reg_num);
             } else {
@@ -1664,12 +1665,12 @@  static void gem_write(void *opaque, hwaddr offset, uint64_t val,
         s->sar_active[(offset - R_SPADDR1HI) / 2] = true;
         break;
     case R_PHYMNTNC:
-        if (val & GEM_PHYMNTNC_OP_W) {
+        if (FIELD_EX32(val, PHYMNTNC, OP) == MDIO_OP_WRITE) {
             uint32_t phy_addr, reg_num;
 
-            phy_addr = (val & GEM_PHYMNTNC_ADDR) >> GEM_PHYMNTNC_ADDR_SHFT;
+            phy_addr = FIELD_EX32(val, PHYMNTNC, PHY_ADDR);
             if (phy_addr == s->phy_addr) {
-                reg_num = (val & GEM_PHYMNTNC_REG) >> GEM_PHYMNTNC_REG_SHIFT;
+                reg_num = FIELD_EX32(val, PHYMNTNC, REG_ADDR);
                 gem_phy_write(s, reg_num, val);
             }
         }