>-----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