diff mbox series

[10/11] hw/net/cadence_gem: perform PHY access on write only

Message ID 20231017194422.4124691-11-luc.michel@amd.com (mailing list archive)
State New, archived
Headers show
Series Various updates for the Cadence GEM model | expand

Commit Message

Luc Michel Oct. 17, 2023, 7:44 p.m. UTC
The MDIO access is done only on a write to the PHYMNTNC register. A
subsequent read is used to retrieve the result but does not trigger an
MDIO access by itself.

Refactor the PHY access logic to perform all accesses (MDIO reads and
writes) at PHYMNTNC write time.

Signed-off-by: Luc Michel <luc.michel@amd.com>
---
 hw/net/cadence_gem.c | 56 ++++++++++++++++++++++++++------------------
 1 file changed, 33 insertions(+), 23 deletions(-)

Comments

Sai Pavan Boddu Oct. 18, 2023, 10:35 a.m. UTC | #1
>-----Original Message-----
>From: Luc Michel <luc.michel@amd.com>
>Sent: Wednesday, October 18, 2023 1:14 AM
>To: qemu-devel@nongnu.org
>Cc: Michel, Luc <Luc.Michel@amd.com>; qemu-arm@nongnu.org; Edgar E .
>Iglesias <edgar.iglesias@gmail.com>; Alistair Francis <alistair@alistair23.me>;
>Peter Maydell <peter.maydell@linaro.org>; Jason Wang
><jasowang@redhat.com>; Philippe Mathieu-Daudé <philmd@linaro.org>;
>Iglesias, Francisco <francisco.iglesias@amd.com>; Konrad, Frederic
><Frederic.Konrad@amd.com>; Boddu, Sai Pavan
><sai.pavan.boddu@amd.com>
>Subject: [PATCH 10/11] hw/net/cadence_gem: perform PHY access on write
>only
>
>The MDIO access is done only on a write to the PHYMNTNC register. A
>subsequent read is used to retrieve the result but does not trigger an MDIO
>access by itself.
>
>Refactor the PHY access logic to perform all accesses (MDIO reads and
>writes) at PHYMNTNC write time.
>
>Signed-off-by: Luc Michel <luc.michel@amd.com>

Reviewed-by: sai.pavan.boddu@amd.com



>---
> hw/net/cadence_gem.c | 56 ++++++++++++++++++++++++++------------------
> 1 file changed, 33 insertions(+), 23 deletions(-)
>
>diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c index
>4c5fe10316..21146f4242 100644
>--- a/hw/net/cadence_gem.c
>+++ b/hw/net/cadence_gem.c
>@@ -1519,10 +1519,42 @@ static void gem_phy_write(CadenceGEMState *s,
>unsigned reg_num, uint16_t val)
>         break;
>     }
>     s->phy_regs[reg_num] = val;
> }
>
>+static void gem_handle_phy_access(CadenceGEMState *s) {
>+    uint32_t val = s->regs[R_PHYMNTNC];
>+    uint32_t phy_addr, reg_num;
>+
>+    phy_addr = FIELD_EX32(val, PHYMNTNC, PHY_ADDR);
>+
>+    if (phy_addr != s->phy_addr) {
>+        /* no phy at this address */
>+        if (FIELD_EX32(val, PHYMNTNC, OP) == MDIO_OP_READ) {
>+            s->regs[R_PHYMNTNC] = FIELD_DP32(val, PHYMNTNC, DATA, 0xffff);
>+        }
>+        return;
>+    }
>+
>+    reg_num = FIELD_EX32(val, PHYMNTNC, REG_ADDR);
>+
>+    switch (FIELD_EX32(val, PHYMNTNC, OP)) {
>+    case MDIO_OP_READ:
>+        s->regs[R_PHYMNTNC] = FIELD_DP32(val, PHYMNTNC, DATA,
>+                                         gem_phy_read(s, reg_num));
>+        break;
>+
>+    case MDIO_OP_WRITE:
>+        gem_phy_write(s, reg_num, val);
>+        break;
>+
>+    default:
>+        break; /* only clause 22 operations are supported */
>+    }
>+}
>+
> /*
>  * gem_read32:
>  * Read a GEM register.
>  */
> static uint64_t gem_read(void *opaque, hwaddr offset, unsigned size) @@ -
>1539,24 +1571,10 @@ static uint64_t gem_read(void *opaque, hwaddr
>offset, unsigned size)
>     switch (offset) {
>     case R_ISR:
>         DB_PRINT("lowering irqs on ISR read\n");
>         /* The interrupts get updated at the end of the function. */
>         break;
>-    case R_PHYMNTNC:
>-        if (FIELD_EX32(retval, PHYMNTNC, OP) == MDIO_OP_READ) {
>-            uint32_t phy_addr, reg_num;
>-
>-            phy_addr = FIELD_EX32(retval, PHYMNTNC, PHY_ADDR);
>-            if (phy_addr == s->phy_addr) {
>-                reg_num = FIELD_EX32(retval, PHYMNTNC, REG_ADDR);
>-                retval &= 0xFFFF0000;
>-                retval |= gem_phy_read(s, reg_num);
>-            } else {
>-                retval |= 0xFFFF; /* No device at this address */
>-            }
>-        }
>-        break;
>     }
>
>     /* Squash read to clear bits */
>     s->regs[offset] &= ~(s->regs_rtc[offset]);
>
>@@ -1663,19 +1681,11 @@ static void gem_write(void *opaque, hwaddr
>offset, uint64_t val,
>     case R_SPADDR3HI:
>     case R_SPADDR4HI:
>         s->sar_active[(offset - R_SPADDR1HI) / 2] = true;
>         break;
>     case R_PHYMNTNC:
>-        if (FIELD_EX32(val, PHYMNTNC, OP) == MDIO_OP_WRITE) {
>-            uint32_t phy_addr, reg_num;
>-
>-            phy_addr = FIELD_EX32(val, PHYMNTNC, PHY_ADDR);
>-            if (phy_addr == s->phy_addr) {
>-                reg_num = FIELD_EX32(val, PHYMNTNC, REG_ADDR);
>-                gem_phy_write(s, reg_num, val);
>-            }
>-        }
>+        gem_handle_phy_access(s);
>         break;
>     }
>
>     DB_PRINT("newval: 0x%08x\n", s->regs[offset]);  }
>--
>2.39.2
diff mbox series

Patch

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 4c5fe10316..21146f4242 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -1519,10 +1519,42 @@  static void gem_phy_write(CadenceGEMState *s, unsigned reg_num, uint16_t val)
         break;
     }
     s->phy_regs[reg_num] = val;
 }
 
+static void gem_handle_phy_access(CadenceGEMState *s)
+{
+    uint32_t val = s->regs[R_PHYMNTNC];
+    uint32_t phy_addr, reg_num;
+
+    phy_addr = FIELD_EX32(val, PHYMNTNC, PHY_ADDR);
+
+    if (phy_addr != s->phy_addr) {
+        /* no phy at this address */
+        if (FIELD_EX32(val, PHYMNTNC, OP) == MDIO_OP_READ) {
+            s->regs[R_PHYMNTNC] = FIELD_DP32(val, PHYMNTNC, DATA, 0xffff);
+        }
+        return;
+    }
+
+    reg_num = FIELD_EX32(val, PHYMNTNC, REG_ADDR);
+
+    switch (FIELD_EX32(val, PHYMNTNC, OP)) {
+    case MDIO_OP_READ:
+        s->regs[R_PHYMNTNC] = FIELD_DP32(val, PHYMNTNC, DATA,
+                                         gem_phy_read(s, reg_num));
+        break;
+
+    case MDIO_OP_WRITE:
+        gem_phy_write(s, reg_num, val);
+        break;
+
+    default:
+        break; /* only clause 22 operations are supported */
+    }
+}
+
 /*
  * gem_read32:
  * Read a GEM register.
  */
 static uint64_t gem_read(void *opaque, hwaddr offset, unsigned size)
@@ -1539,24 +1571,10 @@  static uint64_t gem_read(void *opaque, hwaddr offset, unsigned size)
     switch (offset) {
     case R_ISR:
         DB_PRINT("lowering irqs on ISR read\n");
         /* The interrupts get updated at the end of the function. */
         break;
-    case R_PHYMNTNC:
-        if (FIELD_EX32(retval, PHYMNTNC, OP) == MDIO_OP_READ) {
-            uint32_t phy_addr, reg_num;
-
-            phy_addr = FIELD_EX32(retval, PHYMNTNC, PHY_ADDR);
-            if (phy_addr == s->phy_addr) {
-                reg_num = FIELD_EX32(retval, PHYMNTNC, REG_ADDR);
-                retval &= 0xFFFF0000;
-                retval |= gem_phy_read(s, reg_num);
-            } else {
-                retval |= 0xFFFF; /* No device at this address */
-            }
-        }
-        break;
     }
 
     /* Squash read to clear bits */
     s->regs[offset] &= ~(s->regs_rtc[offset]);
 
@@ -1663,19 +1681,11 @@  static void gem_write(void *opaque, hwaddr offset, uint64_t val,
     case R_SPADDR3HI:
     case R_SPADDR4HI:
         s->sar_active[(offset - R_SPADDR1HI) / 2] = true;
         break;
     case R_PHYMNTNC:
-        if (FIELD_EX32(val, PHYMNTNC, OP) == MDIO_OP_WRITE) {
-            uint32_t phy_addr, reg_num;
-
-            phy_addr = FIELD_EX32(val, PHYMNTNC, PHY_ADDR);
-            if (phy_addr == s->phy_addr) {
-                reg_num = FIELD_EX32(val, PHYMNTNC, REG_ADDR);
-                gem_phy_write(s, reg_num, val);
-            }
-        }
+        gem_handle_phy_access(s);
         break;
     }
 
     DB_PRINT("newval: 0x%08x\n", s->regs[offset]);
 }