* [PATCH v3 1/5] hw/net/lan9118: Extract lan9118_phy
2024-11-02 12:57 [PATCH v3 0/5] Consolidate lan9118 phy implementations Bernhard Beschow
@ 2024-11-02 12:57 ` Bernhard Beschow
2024-11-04 15:35 ` Guenter Roeck
2024-12-02 14:31 ` Peter Maydell
2024-11-02 12:57 ` [PATCH v3 2/5] hw/net/lan9118_phy: Reuse in imx_fec and consolidate implementations Bernhard Beschow
` (4 subsequent siblings)
5 siblings, 2 replies; 15+ messages in thread
From: Bernhard Beschow @ 2024-11-02 12:57 UTC (permalink / raw)
To: qemu-devel
Cc: Jason Wang, Guenter Roeck, qemu-arm, Paolo Bonzini, Peter Maydell,
Bernhard Beschow
A very similar implementation of the same device exists in imx_fec. Prepare for
a common implementation by extracting a device model into its own files.
Some migration state has been moved into the new device model which breaks
migration compatibility for the following machines:
* smdkc210
* realview-*
* vexpress-*
* kzm
* mps2-*
While breaking migration ABI, fix the size of the MII registers to be 16 bit,
as defined by IEEE 802.3u.
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
include/hw/net/lan9118_phy.h | 37 ++++++++
hw/net/lan9118.c | 137 +++++-----------------------
hw/net/lan9118_phy.c | 169 +++++++++++++++++++++++++++++++++++
hw/net/Kconfig | 4 +
hw/net/meson.build | 1 +
5 files changed, 233 insertions(+), 115 deletions(-)
create mode 100644 include/hw/net/lan9118_phy.h
create mode 100644 hw/net/lan9118_phy.c
diff --git a/include/hw/net/lan9118_phy.h b/include/hw/net/lan9118_phy.h
new file mode 100644
index 0000000000..af12fc33d5
--- /dev/null
+++ b/include/hw/net/lan9118_phy.h
@@ -0,0 +1,37 @@
+/*
+ * SMSC LAN9118 PHY emulation
+ *
+ * Copyright (c) 2009 CodeSourcery, LLC.
+ * Written by Paul Brook
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef HW_NET_LAN9118_PHY_H
+#define HW_NET_LAN9118_PHY_H
+
+#include "qom/object.h"
+#include "hw/sysbus.h"
+
+#define TYPE_LAN9118_PHY "lan9118-phy"
+OBJECT_DECLARE_SIMPLE_TYPE(Lan9118PhyState, LAN9118_PHY)
+
+typedef struct Lan9118PhyState {
+ SysBusDevice parent_obj;
+
+ uint16_t status;
+ uint16_t control;
+ uint16_t advertise;
+ uint16_t ints;
+ uint16_t int_mask;
+ qemu_irq irq;
+ bool link_down;
+} Lan9118PhyState;
+
+void lan9118_phy_update_link(Lan9118PhyState *s, bool link_down);
+void lan9118_phy_reset(Lan9118PhyState *s);
+uint16_t lan9118_phy_read(Lan9118PhyState *s, int reg);
+void lan9118_phy_write(Lan9118PhyState *s, int reg, uint16_t val);
+
+#endif
diff --git a/hw/net/lan9118.c b/hw/net/lan9118.c
index db28a0ef30..99e87b7178 100644
--- a/hw/net/lan9118.c
+++ b/hw/net/lan9118.c
@@ -16,6 +16,7 @@
#include "net/net.h"
#include "net/eth.h"
#include "hw/irq.h"
+#include "hw/net/lan9118_phy.h"
#include "hw/net/lan9118.h"
#include "hw/ptimer.h"
#include "hw/qdev-properties.h"
@@ -139,14 +140,6 @@ do { printf("lan9118: " fmt , ## __VA_ARGS__); } while (0)
#define MAC_CR_RXEN 0x00000004
#define MAC_CR_RESERVED 0x7f404213
-#define PHY_INT_ENERGYON 0x80
-#define PHY_INT_AUTONEG_COMPLETE 0x40
-#define PHY_INT_FAULT 0x20
-#define PHY_INT_DOWN 0x10
-#define PHY_INT_AUTONEG_LP 0x08
-#define PHY_INT_PARFAULT 0x04
-#define PHY_INT_AUTONEG_PAGE 0x02
-
#define GPT_TIMER_EN 0x20000000
/*
@@ -228,11 +221,8 @@ struct lan9118_state {
uint32_t mac_mii_data;
uint32_t mac_flow;
- uint32_t phy_status;
- uint32_t phy_control;
- uint32_t phy_advertise;
- uint32_t phy_int;
- uint32_t phy_int_mask;
+ Lan9118PhyState mii;
+ IRQState mii_irq;
int32_t eeprom_writable;
uint8_t eeprom[128];
@@ -274,8 +264,8 @@ struct lan9118_state {
static const VMStateDescription vmstate_lan9118 = {
.name = "lan9118",
- .version_id = 2,
- .minimum_version_id = 1,
+ .version_id = 3,
+ .minimum_version_id = 3,
.fields = (const VMStateField[]) {
VMSTATE_PTIMER(timer, lan9118_state),
VMSTATE_UINT32(irq_cfg, lan9118_state),
@@ -301,11 +291,6 @@ static const VMStateDescription vmstate_lan9118 = {
VMSTATE_UINT32(mac_mii_acc, lan9118_state),
VMSTATE_UINT32(mac_mii_data, lan9118_state),
VMSTATE_UINT32(mac_flow, lan9118_state),
- VMSTATE_UINT32(phy_status, lan9118_state),
- VMSTATE_UINT32(phy_control, lan9118_state),
- VMSTATE_UINT32(phy_advertise, lan9118_state),
- VMSTATE_UINT32(phy_int, lan9118_state),
- VMSTATE_UINT32(phy_int_mask, lan9118_state),
VMSTATE_INT32(eeprom_writable, lan9118_state),
VMSTATE_UINT8_ARRAY(eeprom, lan9118_state, 128),
VMSTATE_INT32(tx_fifo_size, lan9118_state),
@@ -385,9 +370,11 @@ static void lan9118_reload_eeprom(lan9118_state *s)
lan9118_mac_changed(s);
}
-static void phy_update_irq(lan9118_state *s)
+static void lan9118_update_irq(void *opaque, int n, int level)
{
- if (s->phy_int & s->phy_int_mask) {
+ lan9118_state *s = opaque;
+
+ if (level) {
s->int_sts |= PHY_INT;
} else {
s->int_sts &= ~PHY_INT;
@@ -395,33 +382,10 @@ static void phy_update_irq(lan9118_state *s)
lan9118_update(s);
}
-static void phy_update_link(lan9118_state *s)
-{
- /* Autonegotiation status mirrors link status. */
- if (qemu_get_queue(s->nic)->link_down) {
- s->phy_status &= ~0x0024;
- s->phy_int |= PHY_INT_DOWN;
- } else {
- s->phy_status |= 0x0024;
- s->phy_int |= PHY_INT_ENERGYON;
- s->phy_int |= PHY_INT_AUTONEG_COMPLETE;
- }
- phy_update_irq(s);
-}
-
static void lan9118_set_link(NetClientState *nc)
{
- phy_update_link(qemu_get_nic_opaque(nc));
-}
-
-static void phy_reset(lan9118_state *s)
-{
- s->phy_status = 0x7809;
- s->phy_control = 0x3000;
- s->phy_advertise = 0x01e1;
- s->phy_int_mask = 0;
- s->phy_int = 0;
- phy_update_link(s);
+ lan9118_phy_update_link(&LAN9118(qemu_get_nic_opaque(nc))->mii,
+ nc->link_down);
}
static void lan9118_reset(DeviceState *d)
@@ -478,8 +442,6 @@ static void lan9118_reset(DeviceState *d)
s->read_word_n = 0;
s->write_word_n = 0;
- phy_reset(s);
-
s->eeprom_writable = 0;
lan9118_reload_eeprom(s);
}
@@ -678,7 +640,7 @@ static void do_tx_packet(lan9118_state *s)
uint32_t status;
/* FIXME: Honor TX disable, and allow queueing of packets. */
- if (s->phy_control & 0x4000) {
+ if (s->mii.control & 0x4000) {
/* This assumes the receive routine doesn't touch the VLANClient. */
qemu_receive_packet(qemu_get_queue(s->nic), s->txp->data, s->txp->len);
} else {
@@ -834,68 +796,6 @@ static void tx_fifo_push(lan9118_state *s, uint32_t val)
}
}
-static uint32_t do_phy_read(lan9118_state *s, int reg)
-{
- uint32_t val;
-
- switch (reg) {
- case 0: /* Basic Control */
- return s->phy_control;
- case 1: /* Basic Status */
- return s->phy_status;
- case 2: /* ID1 */
- return 0x0007;
- case 3: /* ID2 */
- return 0xc0d1;
- case 4: /* Auto-neg advertisement */
- return s->phy_advertise;
- case 5: /* Auto-neg Link Partner Ability */
- return 0x0f71;
- case 6: /* Auto-neg Expansion */
- return 1;
- /* TODO 17, 18, 27, 29, 30, 31 */
- case 29: /* Interrupt source. */
- val = s->phy_int;
- s->phy_int = 0;
- phy_update_irq(s);
- return val;
- case 30: /* Interrupt mask */
- return s->phy_int_mask;
- default:
- qemu_log_mask(LOG_GUEST_ERROR,
- "do_phy_read: PHY read reg %d\n", reg);
- return 0;
- }
-}
-
-static void do_phy_write(lan9118_state *s, int reg, uint32_t val)
-{
- switch (reg) {
- case 0: /* Basic Control */
- if (val & 0x8000) {
- phy_reset(s);
- break;
- }
- s->phy_control = val & 0x7980;
- /* Complete autonegotiation immediately. */
- if (val & 0x1000) {
- s->phy_status |= 0x0020;
- }
- break;
- case 4: /* Auto-neg advertisement */
- s->phy_advertise = (val & 0x2d7f) | 0x80;
- break;
- /* TODO 17, 18, 27, 31 */
- case 30: /* Interrupt mask */
- s->phy_int_mask = val & 0xff;
- phy_update_irq(s);
- break;
- default:
- qemu_log_mask(LOG_GUEST_ERROR,
- "do_phy_write: PHY write reg %d = 0x%04x\n", reg, val);
- }
-}
-
static void do_mac_write(lan9118_state *s, int reg, uint32_t val)
{
switch (reg) {
@@ -929,9 +829,9 @@ static void do_mac_write(lan9118_state *s, int reg, uint32_t val)
if (val & 2) {
DPRINTF("PHY write %d = 0x%04x\n",
(val >> 6) & 0x1f, s->mac_mii_data);
- do_phy_write(s, (val >> 6) & 0x1f, s->mac_mii_data);
+ lan9118_phy_write(&s->mii, (val >> 6) & 0x1f, s->mac_mii_data);
} else {
- s->mac_mii_data = do_phy_read(s, (val >> 6) & 0x1f);
+ s->mac_mii_data = lan9118_phy_read(&s->mii, (val >> 6) & 0x1f);
DPRINTF("PHY read %d = 0x%04x\n",
(val >> 6) & 0x1f, s->mac_mii_data);
}
@@ -1126,7 +1026,7 @@ static void lan9118_writel(void *opaque, hwaddr offset,
break;
case CSR_PMT_CTRL:
if (val & 0x400) {
- phy_reset(s);
+ lan9118_phy_reset(&s->mii);
}
s->pmt_ctrl &= ~0x34e;
s->pmt_ctrl |= (val & 0x34e);
@@ -1373,6 +1273,13 @@ static void lan9118_realize(DeviceState *dev, Error **errp)
const MemoryRegionOps *mem_ops =
s->mode_16bit ? &lan9118_16bit_mem_ops : &lan9118_mem_ops;
+ qemu_init_irq(&s->mii_irq, lan9118_update_irq, s, 0);
+ object_initialize_child(OBJECT(s), "mii", &s->mii, TYPE_LAN9118_PHY);
+ if (!sysbus_realize_and_unref(SYS_BUS_DEVICE(&s->mii), errp)) {
+ return;
+ }
+ qdev_connect_gpio_out(DEVICE(&s->mii), 0, &s->mii_irq);
+
memory_region_init_io(&s->mmio, OBJECT(dev), mem_ops, s,
"lan9118-mmio", 0x100);
sysbus_init_mmio(sbd, &s->mmio);
diff --git a/hw/net/lan9118_phy.c b/hw/net/lan9118_phy.c
new file mode 100644
index 0000000000..b22c3c2855
--- /dev/null
+++ b/hw/net/lan9118_phy.c
@@ -0,0 +1,169 @@
+/*
+ * SMSC LAN9118 PHY emulation
+ *
+ * Copyright (c) 2009 CodeSourcery, LLC.
+ * Written by Paul Brook
+ *
+ * This code is licensed under the GNU GPL v2
+ *
+ * Contributions after 2012-01-13 are licensed under the terms of the
+ * GNU GPL, version 2 or (at your option) any later version.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/net/lan9118_phy.h"
+#include "hw/irq.h"
+#include "hw/resettable.h"
+#include "migration/vmstate.h"
+#include "qemu/log.h"
+
+#define PHY_INT_ENERGYON (1 << 7)
+#define PHY_INT_AUTONEG_COMPLETE (1 << 6)
+#define PHY_INT_FAULT (1 << 5)
+#define PHY_INT_DOWN (1 << 4)
+#define PHY_INT_AUTONEG_LP (1 << 3)
+#define PHY_INT_PARFAULT (1 << 2)
+#define PHY_INT_AUTONEG_PAGE (1 << 1)
+
+static void lan9118_phy_update_irq(Lan9118PhyState *s)
+{
+ qemu_set_irq(s->irq, !!(s->ints & s->int_mask));
+}
+
+uint16_t lan9118_phy_read(Lan9118PhyState *s, int reg)
+{
+ uint16_t val;
+
+ switch (reg) {
+ case 0: /* Basic Control */
+ return s->control;
+ case 1: /* Basic Status */
+ return s->status;
+ case 2: /* ID1 */
+ return 0x0007;
+ case 3: /* ID2 */
+ return 0xc0d1;
+ case 4: /* Auto-neg advertisement */
+ return s->advertise;
+ case 5: /* Auto-neg Link Partner Ability */
+ return 0x0f71;
+ case 6: /* Auto-neg Expansion */
+ return 1;
+ /* TODO 17, 18, 27, 29, 30, 31 */
+ case 29: /* Interrupt source. */
+ val = s->ints;
+ s->ints = 0;
+ lan9118_phy_update_irq(s);
+ return val;
+ case 30: /* Interrupt mask */
+ return s->int_mask;
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "lan9118_phy_read: PHY read reg %d\n", reg);
+ return 0;
+ }
+}
+
+void lan9118_phy_write(Lan9118PhyState *s, int reg, uint16_t val)
+{
+ switch (reg) {
+ case 0: /* Basic Control */
+ if (val & 0x8000) {
+ lan9118_phy_reset(s);
+ break;
+ }
+ s->control = val & 0x7980;
+ /* Complete autonegotiation immediately. */
+ if (val & 0x1000) {
+ s->status |= 0x0020;
+ }
+ break;
+ case 4: /* Auto-neg advertisement */
+ s->advertise = (val & 0x2d7f) | 0x80;
+ break;
+ /* TODO 17, 18, 27, 31 */
+ case 30: /* Interrupt mask */
+ s->int_mask = val & 0xff;
+ lan9118_phy_update_irq(s);
+ break;
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "lan9118_phy_write: PHY write reg %d = 0x%04x\n", reg, val);
+ }
+}
+
+void lan9118_phy_update_link(Lan9118PhyState *s, bool link_down)
+{
+ s->link_down = link_down;
+
+ /* Autonegotiation status mirrors link status. */
+ if (link_down) {
+ s->status &= ~0x0024;
+ s->ints |= PHY_INT_DOWN;
+ } else {
+ s->status |= 0x0024;
+ s->ints |= PHY_INT_ENERGYON;
+ s->ints |= PHY_INT_AUTONEG_COMPLETE;
+ }
+ lan9118_phy_update_irq(s);
+}
+
+void lan9118_phy_reset(Lan9118PhyState *s)
+{
+ s->control = 0x3000;
+ s->status = 0x7809;
+ s->advertise = 0x01e1;
+ s->int_mask = 0;
+ s->ints = 0;
+ lan9118_phy_update_link(s, s->link_down);
+}
+
+static void lan9118_phy_reset_hold(Object *obj, ResetType type)
+{
+ Lan9118PhyState *s = LAN9118_PHY(obj);
+
+ lan9118_phy_reset(s);
+}
+
+static void lan9118_phy_init(Object *obj)
+{
+ Lan9118PhyState *s = LAN9118_PHY(obj);
+
+ qdev_init_gpio_out(DEVICE(s), &s->irq, 1);
+}
+
+static const VMStateDescription vmstate_lan9118_phy = {
+ .name = "lan9118-phy",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (const VMStateField[]) {
+ VMSTATE_UINT16(control, Lan9118PhyState),
+ VMSTATE_UINT16(status, Lan9118PhyState),
+ VMSTATE_UINT16(advertise, Lan9118PhyState),
+ VMSTATE_UINT16(ints, Lan9118PhyState),
+ VMSTATE_UINT16(int_mask, Lan9118PhyState),
+ VMSTATE_BOOL(link_down, Lan9118PhyState),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static void lan9118_phy_class_init(ObjectClass *klass, void *data)
+{
+ ResettableClass *rc = RESETTABLE_CLASS(klass);
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ rc->phases.hold = lan9118_phy_reset_hold;
+ dc->vmsd = &vmstate_lan9118_phy;
+}
+
+static const TypeInfo types[] = {
+ {
+ .name = TYPE_LAN9118_PHY,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(Lan9118PhyState),
+ .instance_init = lan9118_phy_init,
+ .class_init = lan9118_phy_class_init,
+ }
+};
+
+DEFINE_TYPES(types)
diff --git a/hw/net/Kconfig b/hw/net/Kconfig
index 7fcc0d7faa..6b2ff2f937 100644
--- a/hw/net/Kconfig
+++ b/hw/net/Kconfig
@@ -62,8 +62,12 @@ config VMXNET3_PCI
config SMC91C111
bool
+config LAN9118_PHY
+ bool
+
config LAN9118
bool
+ select LAN9118_PHY
select PTIMER
config NE2000_ISA
diff --git a/hw/net/meson.build b/hw/net/meson.build
index 00a9e9dd51..3bb5d749a8 100644
--- a/hw/net/meson.build
+++ b/hw/net/meson.build
@@ -19,6 +19,7 @@ system_ss.add(when: 'CONFIG_VMXNET3_PCI', if_true: files('vmxnet3.c'))
system_ss.add(when: 'CONFIG_SMC91C111', if_true: files('smc91c111.c'))
system_ss.add(when: 'CONFIG_LAN9118', if_true: files('lan9118.c'))
+system_ss.add(when: 'CONFIG_LAN9118_PHY', if_true: files('lan9118_phy.c'))
system_ss.add(when: 'CONFIG_NE2000_ISA', if_true: files('ne2000-isa.c'))
system_ss.add(when: 'CONFIG_OPENCORES_ETH', if_true: files('opencores_eth.c'))
system_ss.add(when: 'CONFIG_XGMAC', if_true: files('xgmac.c'))
--
2.47.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v3 1/5] hw/net/lan9118: Extract lan9118_phy
2024-11-02 12:57 ` [PATCH v3 1/5] hw/net/lan9118: Extract lan9118_phy Bernhard Beschow
@ 2024-11-04 15:35 ` Guenter Roeck
2024-12-02 14:31 ` Peter Maydell
1 sibling, 0 replies; 15+ messages in thread
From: Guenter Roeck @ 2024-11-04 15:35 UTC (permalink / raw)
To: Bernhard Beschow
Cc: qemu-devel, Jason Wang, qemu-arm, Paolo Bonzini, Peter Maydell
On Sat, Nov 02, 2024 at 01:57:20PM +0100, Bernhard Beschow wrote:
> A very similar implementation of the same device exists in imx_fec. Prepare for
> a common implementation by extracting a device model into its own files.
>
> Some migration state has been moved into the new device model which breaks
> migration compatibility for the following machines:
> * smdkc210
> * realview-*
> * vexpress-*
> * kzm
> * mps2-*
>
> While breaking migration ABI, fix the size of the MII registers to be 16 bit,
> as defined by IEEE 802.3u.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Tested-by: Guenter Roeck <linux@roeck-us.net>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/5] hw/net/lan9118: Extract lan9118_phy
2024-11-02 12:57 ` [PATCH v3 1/5] hw/net/lan9118: Extract lan9118_phy Bernhard Beschow
2024-11-04 15:35 ` Guenter Roeck
@ 2024-12-02 14:31 ` Peter Maydell
1 sibling, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2024-12-02 14:31 UTC (permalink / raw)
To: Bernhard Beschow
Cc: qemu-devel, Jason Wang, Guenter Roeck, qemu-arm, Paolo Bonzini
On Sat, 2 Nov 2024 at 12:57, Bernhard Beschow <shentey@gmail.com> wrote:
>
> A very similar implementation of the same device exists in imx_fec. Prepare for
> a common implementation by extracting a device model into its own files.
>
> Some migration state has been moved into the new device model which breaks
> migration compatibility for the following machines:
> * smdkc210
> * realview-*
> * vexpress-*
> * kzm
> * mps2-*
>
> While breaking migration ABI, fix the size of the MII registers to be 16 bit,
> as defined by IEEE 802.3u.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 2/5] hw/net/lan9118_phy: Reuse in imx_fec and consolidate implementations
2024-11-02 12:57 [PATCH v3 0/5] Consolidate lan9118 phy implementations Bernhard Beschow
2024-11-02 12:57 ` [PATCH v3 1/5] hw/net/lan9118: Extract lan9118_phy Bernhard Beschow
@ 2024-11-02 12:57 ` Bernhard Beschow
2024-11-04 15:35 ` Guenter Roeck
2024-12-02 14:32 ` Peter Maydell
2024-11-02 12:57 ` [PATCH v3 3/5] hw/net/lan9118_phy: Fix off-by-one error in MII_ANLPAR register Bernhard Beschow
` (3 subsequent siblings)
5 siblings, 2 replies; 15+ messages in thread
From: Bernhard Beschow @ 2024-11-02 12:57 UTC (permalink / raw)
To: qemu-devel
Cc: Jason Wang, Guenter Roeck, qemu-arm, Paolo Bonzini, Peter Maydell,
Bernhard Beschow
imx_fec models the same PHY as lan9118_phy. The code is almost the same with
imx_fec having more logging and tracing. Merge these improvements into
lan9118_phy and reuse in imx_fec to fix the code duplication.
Some migration state how resides in the new device model which breaks migration
compatibility for the following machines:
* imx25-pdk
* sabrelite
* mcimx7d-sabre
* mcimx6ul-evk
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
include/hw/net/imx_fec.h | 9 ++-
hw/net/imx_fec.c | 146 ++++-----------------------------------
hw/net/lan9118_phy.c | 82 ++++++++++++++++------
hw/net/Kconfig | 1 +
hw/net/trace-events | 10 +--
5 files changed, 85 insertions(+), 163 deletions(-)
diff --git a/include/hw/net/imx_fec.h b/include/hw/net/imx_fec.h
index 2d13290c78..83b21637ee 100644
--- a/include/hw/net/imx_fec.h
+++ b/include/hw/net/imx_fec.h
@@ -31,6 +31,8 @@ OBJECT_DECLARE_SIMPLE_TYPE(IMXFECState, IMX_FEC)
#define TYPE_IMX_ENET "imx.enet"
#include "hw/sysbus.h"
+#include "hw/net/lan9118_phy.h"
+#include "hw/irq.h"
#include "net/net.h"
#define ENET_EIR 1
@@ -264,11 +266,8 @@ struct IMXFECState {
uint32_t tx_descriptor[ENET_TX_RING_NUM];
uint32_t tx_ring_num;
- uint32_t phy_status;
- uint32_t phy_control;
- uint32_t phy_advertise;
- uint32_t phy_int;
- uint32_t phy_int_mask;
+ Lan9118PhyState mii;
+ IRQState mii_irq;
uint32_t phy_num;
bool phy_connected;
struct IMXFECState *phy_consumer;
diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index 6294d29202..4ee6f74206 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -203,17 +203,12 @@ static const VMStateDescription vmstate_imx_eth_txdescs = {
static const VMStateDescription vmstate_imx_eth = {
.name = TYPE_IMX_FEC,
- .version_id = 2,
- .minimum_version_id = 2,
+ .version_id = 3,
+ .minimum_version_id = 3,
.fields = (const VMStateField[]) {
VMSTATE_UINT32_ARRAY(regs, IMXFECState, ENET_MAX),
VMSTATE_UINT32(rx_descriptor, IMXFECState),
VMSTATE_UINT32(tx_descriptor[0], IMXFECState),
- VMSTATE_UINT32(phy_status, IMXFECState),
- VMSTATE_UINT32(phy_control, IMXFECState),
- VMSTATE_UINT32(phy_advertise, IMXFECState),
- VMSTATE_UINT32(phy_int, IMXFECState),
- VMSTATE_UINT32(phy_int_mask, IMXFECState),
VMSTATE_END_OF_LIST()
},
.subsections = (const VMStateDescription * const []) {
@@ -222,14 +217,6 @@ static const VMStateDescription vmstate_imx_eth = {
},
};
-#define PHY_INT_ENERGYON (1 << 7)
-#define PHY_INT_AUTONEG_COMPLETE (1 << 6)
-#define PHY_INT_FAULT (1 << 5)
-#define PHY_INT_DOWN (1 << 4)
-#define PHY_INT_AUTONEG_LP (1 << 3)
-#define PHY_INT_PARFAULT (1 << 2)
-#define PHY_INT_AUTONEG_PAGE (1 << 1)
-
static void imx_eth_update(IMXFECState *s);
/*
@@ -238,47 +225,19 @@ static void imx_eth_update(IMXFECState *s);
* For now we don't handle any GPIO/interrupt line, so the OS will
* have to poll for the PHY status.
*/
-static void imx_phy_update_irq(IMXFECState *s)
-{
- imx_eth_update(s);
-}
-
-static void imx_phy_update_link(IMXFECState *s)
+static void imx_phy_update_irq(void *opaque, int n, int level)
{
- /* Autonegotiation status mirrors link status. */
- if (qemu_get_queue(s->nic)->link_down) {
- trace_imx_phy_update_link("down");
- s->phy_status &= ~0x0024;
- s->phy_int |= PHY_INT_DOWN;
- } else {
- trace_imx_phy_update_link("up");
- s->phy_status |= 0x0024;
- s->phy_int |= PHY_INT_ENERGYON;
- s->phy_int |= PHY_INT_AUTONEG_COMPLETE;
- }
- imx_phy_update_irq(s);
+ imx_eth_update(opaque);
}
static void imx_eth_set_link(NetClientState *nc)
{
- imx_phy_update_link(IMX_FEC(qemu_get_nic_opaque(nc)));
-}
-
-static void imx_phy_reset(IMXFECState *s)
-{
- trace_imx_phy_reset();
-
- s->phy_status = 0x7809;
- s->phy_control = 0x3000;
- s->phy_advertise = 0x01e1;
- s->phy_int_mask = 0;
- s->phy_int = 0;
- imx_phy_update_link(s);
+ lan9118_phy_update_link(&IMX_FEC(qemu_get_nic_opaque(nc))->mii,
+ nc->link_down);
}
static uint32_t imx_phy_read(IMXFECState *s, int reg)
{
- uint32_t val;
uint32_t phy = reg / 32;
if (!s->phy_connected) {
@@ -296,54 +255,7 @@ static uint32_t imx_phy_read(IMXFECState *s, int reg)
reg %= 32;
- switch (reg) {
- case 0: /* Basic Control */
- val = s->phy_control;
- break;
- case 1: /* Basic Status */
- val = s->phy_status;
- break;
- case 2: /* ID1 */
- val = 0x0007;
- break;
- case 3: /* ID2 */
- val = 0xc0d1;
- break;
- case 4: /* Auto-neg advertisement */
- val = s->phy_advertise;
- break;
- case 5: /* Auto-neg Link Partner Ability */
- val = 0x0f71;
- break;
- case 6: /* Auto-neg Expansion */
- val = 1;
- break;
- case 29: /* Interrupt source. */
- val = s->phy_int;
- s->phy_int = 0;
- imx_phy_update_irq(s);
- break;
- case 30: /* Interrupt mask */
- val = s->phy_int_mask;
- break;
- case 17:
- case 18:
- case 27:
- case 31:
- qemu_log_mask(LOG_UNIMP, "[%s.phy]%s: reg %d not implemented\n",
- TYPE_IMX_FEC, __func__, reg);
- val = 0;
- break;
- default:
- qemu_log_mask(LOG_GUEST_ERROR, "[%s.phy]%s: Bad address at offset %d\n",
- TYPE_IMX_FEC, __func__, reg);
- val = 0;
- break;
- }
-
- trace_imx_phy_read(val, phy, reg);
-
- return val;
+ return lan9118_phy_read(&s->mii, reg);
}
static void imx_phy_write(IMXFECState *s, int reg, uint32_t val)
@@ -365,39 +277,7 @@ static void imx_phy_write(IMXFECState *s, int reg, uint32_t val)
reg %= 32;
- trace_imx_phy_write(val, phy, reg);
-
- switch (reg) {
- case 0: /* Basic Control */
- if (val & 0x8000) {
- imx_phy_reset(s);
- } else {
- s->phy_control = val & 0x7980;
- /* Complete autonegotiation immediately. */
- if (val & 0x1000) {
- s->phy_status |= 0x0020;
- }
- }
- break;
- case 4: /* Auto-neg advertisement */
- s->phy_advertise = (val & 0x2d7f) | 0x80;
- break;
- case 30: /* Interrupt mask */
- s->phy_int_mask = val & 0xff;
- imx_phy_update_irq(s);
- break;
- case 17:
- case 18:
- case 27:
- case 31:
- qemu_log_mask(LOG_UNIMP, "[%s.phy)%s: reg %d not implemented\n",
- TYPE_IMX_FEC, __func__, reg);
- break;
- default:
- qemu_log_mask(LOG_GUEST_ERROR, "[%s.phy]%s: Bad address at offset %d\n",
- TYPE_IMX_FEC, __func__, reg);
- break;
- }
+ lan9118_phy_write(&s->mii, reg, val);
}
static void imx_fec_read_bd(IMXFECBufDesc *bd, dma_addr_t addr)
@@ -682,9 +562,6 @@ static void imx_eth_reset(DeviceState *d)
s->rx_descriptor = 0;
memset(s->tx_descriptor, 0, sizeof(s->tx_descriptor));
-
- /* We also reset the PHY */
- imx_phy_reset(s);
}
static uint32_t imx_default_read(IMXFECState *s, uint32_t index)
@@ -1329,6 +1206,13 @@ static void imx_eth_realize(DeviceState *dev, Error **errp)
sysbus_init_irq(sbd, &s->irq[0]);
sysbus_init_irq(sbd, &s->irq[1]);
+ qemu_init_irq(&s->mii_irq, imx_phy_update_irq, s, 0);
+ object_initialize_child(OBJECT(s), "mii", &s->mii, TYPE_LAN9118_PHY);
+ if (!sysbus_realize_and_unref(SYS_BUS_DEVICE(&s->mii), errp)) {
+ return;
+ }
+ qdev_connect_gpio_out(DEVICE(&s->mii), 0, &s->mii_irq);
+
qemu_macaddr_default_if_unset(&s->conf.macaddr);
s->nic = qemu_new_nic(&imx_eth_net_info, &s->conf,
diff --git a/hw/net/lan9118_phy.c b/hw/net/lan9118_phy.c
index b22c3c2855..d2dcd732ac 100644
--- a/hw/net/lan9118_phy.c
+++ b/hw/net/lan9118_phy.c
@@ -4,6 +4,8 @@
* Copyright (c) 2009 CodeSourcery, LLC.
* Written by Paul Brook
*
+ * Copyright (c) 2013 Jean-Christophe Dubois. <jcd@tribudubois.net>
+ *
* This code is licensed under the GNU GPL v2
*
* Contributions after 2012-01-13 are licensed under the terms of the
@@ -16,6 +18,7 @@
#include "hw/resettable.h"
#include "migration/vmstate.h"
#include "qemu/log.h"
+#include "trace.h"
#define PHY_INT_ENERGYON (1 << 7)
#define PHY_INT_AUTONEG_COMPLETE (1 << 6)
@@ -36,59 +39,88 @@ uint16_t lan9118_phy_read(Lan9118PhyState *s, int reg)
switch (reg) {
case 0: /* Basic Control */
- return s->control;
+ val = s->control;
+ break;
case 1: /* Basic Status */
- return s->status;
+ val = s->status;
+ break;
case 2: /* ID1 */
- return 0x0007;
+ val = 0x0007;
+ break;
case 3: /* ID2 */
- return 0xc0d1;
+ val = 0xc0d1;
+ break;
case 4: /* Auto-neg advertisement */
- return s->advertise;
+ val = s->advertise;
+ break;
case 5: /* Auto-neg Link Partner Ability */
- return 0x0f71;
+ val = 0x0f71;
+ break;
case 6: /* Auto-neg Expansion */
- return 1;
- /* TODO 17, 18, 27, 29, 30, 31 */
+ val = 1;
+ break;
case 29: /* Interrupt source. */
val = s->ints;
s->ints = 0;
lan9118_phy_update_irq(s);
- return val;
+ break;
case 30: /* Interrupt mask */
- return s->int_mask;
+ val = s->int_mask;
+ break;
+ case 17:
+ case 18:
+ case 27:
+ case 31:
+ qemu_log_mask(LOG_UNIMP, "%s: reg %d not implemented\n",
+ __func__, reg);
+ val = 0;
+ break;
default:
- qemu_log_mask(LOG_GUEST_ERROR,
- "lan9118_phy_read: PHY read reg %d\n", reg);
- return 0;
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad address at offset %d\n",
+ __func__, reg);
+ val = 0;
+ break;
}
+
+ trace_lan9118_phy_read(val, reg);
+
+ return val;
}
void lan9118_phy_write(Lan9118PhyState *s, int reg, uint16_t val)
{
+ trace_lan9118_phy_write(val, reg);
+
switch (reg) {
case 0: /* Basic Control */
if (val & 0x8000) {
lan9118_phy_reset(s);
- break;
- }
- s->control = val & 0x7980;
- /* Complete autonegotiation immediately. */
- if (val & 0x1000) {
- s->status |= 0x0020;
+ } else {
+ s->control = val & 0x7980;
+ /* Complete autonegotiation immediately. */
+ if (val & 0x1000) {
+ s->status |= 0x0020;
+ }
}
break;
case 4: /* Auto-neg advertisement */
s->advertise = (val & 0x2d7f) | 0x80;
break;
- /* TODO 17, 18, 27, 31 */
case 30: /* Interrupt mask */
s->int_mask = val & 0xff;
lan9118_phy_update_irq(s);
break;
+ case 17:
+ case 18:
+ case 27:
+ case 31:
+ qemu_log_mask(LOG_UNIMP, "%s: reg %d not implemented\n",
+ __func__, reg);
+ break;
default:
- qemu_log_mask(LOG_GUEST_ERROR,
- "lan9118_phy_write: PHY write reg %d = 0x%04x\n", reg, val);
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad address at offset %d\n",
+ __func__, reg);
+ break;
}
}
@@ -98,9 +130,11 @@ void lan9118_phy_update_link(Lan9118PhyState *s, bool link_down)
/* Autonegotiation status mirrors link status. */
if (link_down) {
+ trace_lan9118_phy_update_link("down");
s->status &= ~0x0024;
s->ints |= PHY_INT_DOWN;
} else {
+ trace_lan9118_phy_update_link("up");
s->status |= 0x0024;
s->ints |= PHY_INT_ENERGYON;
s->ints |= PHY_INT_AUTONEG_COMPLETE;
@@ -110,6 +144,8 @@ void lan9118_phy_update_link(Lan9118PhyState *s, bool link_down)
void lan9118_phy_reset(Lan9118PhyState *s)
{
+ trace_lan9118_phy_reset();
+
s->control = 0x3000;
s->status = 0x7809;
s->advertise = 0x01e1;
@@ -137,8 +173,8 @@ static const VMStateDescription vmstate_lan9118_phy = {
.version_id = 1,
.minimum_version_id = 1,
.fields = (const VMStateField[]) {
- VMSTATE_UINT16(control, Lan9118PhyState),
VMSTATE_UINT16(status, Lan9118PhyState),
+ VMSTATE_UINT16(control, Lan9118PhyState),
VMSTATE_UINT16(advertise, Lan9118PhyState),
VMSTATE_UINT16(ints, Lan9118PhyState),
VMSTATE_UINT16(int_mask, Lan9118PhyState),
diff --git a/hw/net/Kconfig b/hw/net/Kconfig
index 6b2ff2f937..7f80218d10 100644
--- a/hw/net/Kconfig
+++ b/hw/net/Kconfig
@@ -93,6 +93,7 @@ config ALLWINNER_SUN8I_EMAC
config IMX_FEC
bool
+ select LAN9118_PHY
config CADENCE
bool
diff --git a/hw/net/trace-events b/hw/net/trace-events
index 91a3d0c054..e694d5823a 100644
--- a/hw/net/trace-events
+++ b/hw/net/trace-events
@@ -10,6 +10,12 @@ allwinner_sun8i_emac_set_link(bool active) "Set link: active=%u"
allwinner_sun8i_emac_read(uint64_t offset, uint64_t val) "MMIO read: offset=0x%" PRIx64 " value=0x%" PRIx64
allwinner_sun8i_emac_write(uint64_t offset, uint64_t val) "MMIO write: offset=0x%" PRIx64 " value=0x%" PRIx64
+# lan9118_phy.c
+lan9118_phy_read(uint16_t val, int reg) "[0x%02x] -> 0x%04" PRIx16
+lan9118_phy_write(uint16_t val, int reg) "[0x%02x] <- 0x%04" PRIx16
+lan9118_phy_update_link(const char *s) "%s"
+lan9118_phy_reset(void) ""
+
# lance.c
lance_mem_readw(uint64_t addr, uint32_t ret) "addr=0x%"PRIx64"val=0x%04x"
lance_mem_writew(uint64_t addr, uint32_t val) "addr=0x%"PRIx64"val=0x%04x"
@@ -428,12 +434,8 @@ i82596_set_multicast(uint16_t count) "Added %d multicast entries"
i82596_channel_attention(void *s) "%p: Received CHANNEL ATTENTION"
# imx_fec.c
-imx_phy_read(uint32_t val, int phy, int reg) "0x%04"PRIx32" <= phy[%d].reg[%d]"
imx_phy_read_num(int phy, int configured) "read request from unconfigured phy %d (configured %d)"
-imx_phy_write(uint32_t val, int phy, int reg) "0x%04"PRIx32" => phy[%d].reg[%d]"
imx_phy_write_num(int phy, int configured) "write request to unconfigured phy %d (configured %d)"
-imx_phy_update_link(const char *s) "%s"
-imx_phy_reset(void) ""
imx_fec_read_bd(uint64_t addr, int flags, int len, int data) "tx_bd 0x%"PRIx64" flags 0x%04x len %d data 0x%08x"
imx_enet_read_bd(uint64_t addr, int flags, int len, int data, int options, int status) "tx_bd 0x%"PRIx64" flags 0x%04x len %d data 0x%08x option 0x%04x status 0x%04x"
imx_eth_tx_bd_busy(void) "tx_bd ran out of descriptors to transmit"
--
2.47.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v3 2/5] hw/net/lan9118_phy: Reuse in imx_fec and consolidate implementations
2024-11-02 12:57 ` [PATCH v3 2/5] hw/net/lan9118_phy: Reuse in imx_fec and consolidate implementations Bernhard Beschow
@ 2024-11-04 15:35 ` Guenter Roeck
2024-12-02 14:32 ` Peter Maydell
1 sibling, 0 replies; 15+ messages in thread
From: Guenter Roeck @ 2024-11-04 15:35 UTC (permalink / raw)
To: Bernhard Beschow
Cc: qemu-devel, Jason Wang, qemu-arm, Paolo Bonzini, Peter Maydell
On Sat, Nov 02, 2024 at 01:57:21PM +0100, Bernhard Beschow wrote:
> imx_fec models the same PHY as lan9118_phy. The code is almost the same with
> imx_fec having more logging and tracing. Merge these improvements into
> lan9118_phy and reuse in imx_fec to fix the code duplication.
>
> Some migration state how resides in the new device model which breaks migration
> compatibility for the following machines:
> * imx25-pdk
> * sabrelite
> * mcimx7d-sabre
> * mcimx6ul-evk
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Tested-by: Guenter Roeck <linux@roeck-us.net>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/5] hw/net/lan9118_phy: Reuse in imx_fec and consolidate implementations
2024-11-02 12:57 ` [PATCH v3 2/5] hw/net/lan9118_phy: Reuse in imx_fec and consolidate implementations Bernhard Beschow
2024-11-04 15:35 ` Guenter Roeck
@ 2024-12-02 14:32 ` Peter Maydell
1 sibling, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2024-12-02 14:32 UTC (permalink / raw)
To: Bernhard Beschow
Cc: qemu-devel, Jason Wang, Guenter Roeck, qemu-arm, Paolo Bonzini
On Sat, 2 Nov 2024 at 12:57, Bernhard Beschow <shentey@gmail.com> wrote:
>
> imx_fec models the same PHY as lan9118_phy. The code is almost the same with
> imx_fec having more logging and tracing. Merge these improvements into
> lan9118_phy and reuse in imx_fec to fix the code duplication.
>
> Some migration state how resides in the new device model which breaks migration
> compatibility for the following machines:
> * imx25-pdk
> * sabrelite
> * mcimx7d-sabre
> * mcimx6ul-evk
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 3/5] hw/net/lan9118_phy: Fix off-by-one error in MII_ANLPAR register
2024-11-02 12:57 [PATCH v3 0/5] Consolidate lan9118 phy implementations Bernhard Beschow
2024-11-02 12:57 ` [PATCH v3 1/5] hw/net/lan9118: Extract lan9118_phy Bernhard Beschow
2024-11-02 12:57 ` [PATCH v3 2/5] hw/net/lan9118_phy: Reuse in imx_fec and consolidate implementations Bernhard Beschow
@ 2024-11-02 12:57 ` Bernhard Beschow
2024-11-04 15:35 ` Guenter Roeck
2024-12-02 14:31 ` Peter Maydell
2024-11-02 12:57 ` [PATCH v3 4/5] hw/net/lan9118_phy: Reuse MII constants Bernhard Beschow
` (2 subsequent siblings)
5 siblings, 2 replies; 15+ messages in thread
From: Bernhard Beschow @ 2024-11-02 12:57 UTC (permalink / raw)
To: qemu-devel
Cc: Jason Wang, Guenter Roeck, qemu-arm, Paolo Bonzini, Peter Maydell,
Bernhard Beschow
Turns 0x70 into 0xe0 (== 0x70 << 1) which adds the missing MII_ANLPAR_TX and
fixes the MSB of selector field to be zero, as specified in the datasheet.
Fixes: 2a424990170b "LAN9118 emulation"
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
hw/net/lan9118_phy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/net/lan9118_phy.c b/hw/net/lan9118_phy.c
index d2dcd732ac..d8fc9ddd3a 100644
--- a/hw/net/lan9118_phy.c
+++ b/hw/net/lan9118_phy.c
@@ -54,7 +54,7 @@ uint16_t lan9118_phy_read(Lan9118PhyState *s, int reg)
val = s->advertise;
break;
case 5: /* Auto-neg Link Partner Ability */
- val = 0x0f71;
+ val = 0x0fe1;
break;
case 6: /* Auto-neg Expansion */
val = 1;
--
2.47.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v3 3/5] hw/net/lan9118_phy: Fix off-by-one error in MII_ANLPAR register
2024-11-02 12:57 ` [PATCH v3 3/5] hw/net/lan9118_phy: Fix off-by-one error in MII_ANLPAR register Bernhard Beschow
@ 2024-11-04 15:35 ` Guenter Roeck
2024-12-02 14:31 ` Peter Maydell
1 sibling, 0 replies; 15+ messages in thread
From: Guenter Roeck @ 2024-11-04 15:35 UTC (permalink / raw)
To: Bernhard Beschow
Cc: qemu-devel, Jason Wang, qemu-arm, Paolo Bonzini, Peter Maydell
On Sat, Nov 02, 2024 at 01:57:22PM +0100, Bernhard Beschow wrote:
> Turns 0x70 into 0xe0 (== 0x70 << 1) which adds the missing MII_ANLPAR_TX and
> fixes the MSB of selector field to be zero, as specified in the datasheet.
>
> Fixes: 2a424990170b "LAN9118 emulation"
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Tested-by: Guenter Roeck <linux@roeck-us.net>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 3/5] hw/net/lan9118_phy: Fix off-by-one error in MII_ANLPAR register
2024-11-02 12:57 ` [PATCH v3 3/5] hw/net/lan9118_phy: Fix off-by-one error in MII_ANLPAR register Bernhard Beschow
2024-11-04 15:35 ` Guenter Roeck
@ 2024-12-02 14:31 ` Peter Maydell
1 sibling, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2024-12-02 14:31 UTC (permalink / raw)
To: Bernhard Beschow
Cc: qemu-devel, Jason Wang, Guenter Roeck, qemu-arm, Paolo Bonzini
On Sat, 2 Nov 2024 at 12:57, Bernhard Beschow <shentey@gmail.com> wrote:
>
> Turns 0x70 into 0xe0 (== 0x70 << 1) which adds the missing MII_ANLPAR_TX and
> fixes the MSB of selector field to be zero, as specified in the datasheet.
>
> Fixes: 2a424990170b "LAN9118 emulation"
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
> hw/net/lan9118_phy.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/net/lan9118_phy.c b/hw/net/lan9118_phy.c
> index d2dcd732ac..d8fc9ddd3a 100644
> --- a/hw/net/lan9118_phy.c
> +++ b/hw/net/lan9118_phy.c
> @@ -54,7 +54,7 @@ uint16_t lan9118_phy_read(Lan9118PhyState *s, int reg)
> val = s->advertise;
> break;
> case 5: /* Auto-neg Link Partner Ability */
> - val = 0x0f71;
> + val = 0x0fe1;
> break;
> case 6: /* Auto-neg Expansion */
> val = 1;
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 4/5] hw/net/lan9118_phy: Reuse MII constants
2024-11-02 12:57 [PATCH v3 0/5] Consolidate lan9118 phy implementations Bernhard Beschow
` (2 preceding siblings ...)
2024-11-02 12:57 ` [PATCH v3 3/5] hw/net/lan9118_phy: Fix off-by-one error in MII_ANLPAR register Bernhard Beschow
@ 2024-11-02 12:57 ` Bernhard Beschow
2024-11-04 15:36 ` Guenter Roeck
2024-11-02 12:57 ` [PATCH v3 5/5] hw/net/lan9118_phy: Add missing 100 mbps full duplex advertisement Bernhard Beschow
2024-12-02 14:33 ` [PATCH v3 0/5] Consolidate lan9118 phy implementations Peter Maydell
5 siblings, 1 reply; 15+ messages in thread
From: Bernhard Beschow @ 2024-11-02 12:57 UTC (permalink / raw)
To: qemu-devel
Cc: Jason Wang, Guenter Roeck, qemu-arm, Paolo Bonzini, Peter Maydell,
Bernhard Beschow
Prefer named constants over magic values for better readability.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
include/hw/net/mii.h | 6 +++++
hw/net/lan9118_phy.c | 63 ++++++++++++++++++++++++++++----------------
2 files changed, 46 insertions(+), 23 deletions(-)
diff --git a/include/hw/net/mii.h b/include/hw/net/mii.h
index f7feddac9b..55bf7c92a1 100644
--- a/include/hw/net/mii.h
+++ b/include/hw/net/mii.h
@@ -71,6 +71,7 @@
#define MII_BMSR_JABBER (1 << 1) /* Jabber detected */
#define MII_BMSR_EXTCAP (1 << 0) /* Ext-reg capability */
+#define MII_ANAR_RFAULT (1 << 13) /* Say we can detect faults */
#define MII_ANAR_PAUSE_ASYM (1 << 11) /* Try for asymmetric pause */
#define MII_ANAR_PAUSE (1 << 10) /* Try for pause */
#define MII_ANAR_TXFD (1 << 8)
@@ -78,6 +79,7 @@
#define MII_ANAR_10FD (1 << 6)
#define MII_ANAR_10 (1 << 5)
#define MII_ANAR_CSMACD (1 << 0)
+#define MII_ANAR_SELECT (0x001f) /* Selector bits */
#define MII_ANLPAR_ACK (1 << 14)
#define MII_ANLPAR_PAUSEASY (1 << 11) /* can pause asymmetrically */
@@ -112,6 +114,10 @@
#define RTL8201CP_PHYID1 0x0000
#define RTL8201CP_PHYID2 0x8201
+/* SMSC LAN9118 */
+#define SMSCLAN9118_PHYID1 0x0007
+#define SMSCLAN9118_PHYID2 0xc0d1
+
/* RealTek 8211E */
#define RTL8211E_PHYID1 0x001c
#define RTL8211E_PHYID2 0xc915
diff --git a/hw/net/lan9118_phy.c b/hw/net/lan9118_phy.c
index d8fc9ddd3a..874dae4155 100644
--- a/hw/net/lan9118_phy.c
+++ b/hw/net/lan9118_phy.c
@@ -14,6 +14,7 @@
#include "qemu/osdep.h"
#include "hw/net/lan9118_phy.h"
+#include "hw/net/mii.h"
#include "hw/irq.h"
#include "hw/resettable.h"
#include "migration/vmstate.h"
@@ -38,26 +39,28 @@ uint16_t lan9118_phy_read(Lan9118PhyState *s, int reg)
uint16_t val;
switch (reg) {
- case 0: /* Basic Control */
+ case MII_BMCR:
val = s->control;
break;
- case 1: /* Basic Status */
+ case MII_BMSR:
val = s->status;
break;
- case 2: /* ID1 */
- val = 0x0007;
+ case MII_PHYID1:
+ val = SMSCLAN9118_PHYID1;
break;
- case 3: /* ID2 */
- val = 0xc0d1;
+ case MII_PHYID2:
+ val = SMSCLAN9118_PHYID2;
break;
- case 4: /* Auto-neg advertisement */
+ case MII_ANAR:
val = s->advertise;
break;
- case 5: /* Auto-neg Link Partner Ability */
- val = 0x0fe1;
+ case MII_ANLPAR:
+ val = MII_ANLPAR_PAUSEASY | MII_ANLPAR_PAUSE | MII_ANLPAR_T4 |
+ MII_ANLPAR_TXFD | MII_ANLPAR_TX | MII_ANLPAR_10FD |
+ MII_ANLPAR_10 | MII_ANLPAR_CSMACD;
break;
- case 6: /* Auto-neg Expansion */
- val = 1;
+ case MII_ANER:
+ val = MII_ANER_NWAY;
break;
case 29: /* Interrupt source. */
val = s->ints;
@@ -92,19 +95,24 @@ void lan9118_phy_write(Lan9118PhyState *s, int reg, uint16_t val)
trace_lan9118_phy_write(val, reg);
switch (reg) {
- case 0: /* Basic Control */
- if (val & 0x8000) {
+ case MII_BMCR:
+ if (val & MII_BMCR_RESET) {
lan9118_phy_reset(s);
} else {
- s->control = val & 0x7980;
+ s->control = val & (MII_BMCR_LOOPBACK | MII_BMCR_SPEED100 |
+ MII_BMCR_AUTOEN | MII_BMCR_PDOWN | MII_BMCR_FD |
+ MII_BMCR_CTST);
/* Complete autonegotiation immediately. */
- if (val & 0x1000) {
- s->status |= 0x0020;
+ if (val & MII_BMCR_AUTOEN) {
+ s->status |= MII_BMSR_AN_COMP;
}
}
break;
- case 4: /* Auto-neg advertisement */
- s->advertise = (val & 0x2d7f) | 0x80;
+ case MII_ANAR:
+ s->advertise = (val & (MII_ANAR_RFAULT | MII_ANAR_PAUSE_ASYM |
+ MII_ANAR_PAUSE | MII_ANAR_10FD | MII_ANAR_10 |
+ MII_ANAR_SELECT))
+ | MII_ANAR_TX;
break;
case 30: /* Interrupt mask */
s->int_mask = val & 0xff;
@@ -131,11 +139,11 @@ void lan9118_phy_update_link(Lan9118PhyState *s, bool link_down)
/* Autonegotiation status mirrors link status. */
if (link_down) {
trace_lan9118_phy_update_link("down");
- s->status &= ~0x0024;
+ s->status &= ~(MII_BMSR_AN_COMP | MII_BMSR_LINK_ST);
s->ints |= PHY_INT_DOWN;
} else {
trace_lan9118_phy_update_link("up");
- s->status |= 0x0024;
+ s->status |= MII_BMSR_AN_COMP | MII_BMSR_LINK_ST;
s->ints |= PHY_INT_ENERGYON;
s->ints |= PHY_INT_AUTONEG_COMPLETE;
}
@@ -146,9 +154,18 @@ void lan9118_phy_reset(Lan9118PhyState *s)
{
trace_lan9118_phy_reset();
- s->control = 0x3000;
- s->status = 0x7809;
- s->advertise = 0x01e1;
+ s->control = MII_BMCR_AUTOEN | MII_BMCR_SPEED100;
+ s->status = MII_BMSR_100TX_FD
+ | MII_BMSR_100TX_HD
+ | MII_BMSR_10T_FD
+ | MII_BMSR_10T_HD
+ | MII_BMSR_AUTONEG
+ | MII_BMSR_EXTCAP;
+ s->advertise = MII_ANAR_TXFD
+ | MII_ANAR_TX
+ | MII_ANAR_10FD
+ | MII_ANAR_10
+ | MII_ANAR_CSMACD;
s->int_mask = 0;
s->ints = 0;
lan9118_phy_update_link(s, s->link_down);
--
2.47.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v3 5/5] hw/net/lan9118_phy: Add missing 100 mbps full duplex advertisement
2024-11-02 12:57 [PATCH v3 0/5] Consolidate lan9118 phy implementations Bernhard Beschow
` (3 preceding siblings ...)
2024-11-02 12:57 ` [PATCH v3 4/5] hw/net/lan9118_phy: Reuse MII constants Bernhard Beschow
@ 2024-11-02 12:57 ` Bernhard Beschow
2024-11-04 15:36 ` Guenter Roeck
2024-12-02 14:33 ` [PATCH v3 0/5] Consolidate lan9118 phy implementations Peter Maydell
5 siblings, 1 reply; 15+ messages in thread
From: Bernhard Beschow @ 2024-11-02 12:57 UTC (permalink / raw)
To: qemu-devel
Cc: Jason Wang, Guenter Roeck, qemu-arm, Paolo Bonzini, Peter Maydell,
Bernhard Beschow
The real device advertises this mode and the device model already advertises
100 mbps half duplex and 10 mbps full+half duplex. So advertise this mode to
make the model more realistic.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
hw/net/lan9118_phy.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/net/lan9118_phy.c b/hw/net/lan9118_phy.c
index 874dae4155..5c53a4a1e3 100644
--- a/hw/net/lan9118_phy.c
+++ b/hw/net/lan9118_phy.c
@@ -110,8 +110,8 @@ void lan9118_phy_write(Lan9118PhyState *s, int reg, uint16_t val)
break;
case MII_ANAR:
s->advertise = (val & (MII_ANAR_RFAULT | MII_ANAR_PAUSE_ASYM |
- MII_ANAR_PAUSE | MII_ANAR_10FD | MII_ANAR_10 |
- MII_ANAR_SELECT))
+ MII_ANAR_PAUSE | MII_ANAR_TXFD | MII_ANAR_10FD |
+ MII_ANAR_10 | MII_ANAR_SELECT))
| MII_ANAR_TX;
break;
case 30: /* Interrupt mask */
--
2.47.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v3 0/5] Consolidate lan9118 phy implementations
2024-11-02 12:57 [PATCH v3 0/5] Consolidate lan9118 phy implementations Bernhard Beschow
` (4 preceding siblings ...)
2024-11-02 12:57 ` [PATCH v3 5/5] hw/net/lan9118_phy: Add missing 100 mbps full duplex advertisement Bernhard Beschow
@ 2024-12-02 14:33 ` Peter Maydell
5 siblings, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2024-12-02 14:33 UTC (permalink / raw)
To: Bernhard Beschow
Cc: qemu-devel, Jason Wang, Guenter Roeck, qemu-arm, Paolo Bonzini
On Sat, 2 Nov 2024 at 12:57, Bernhard Beschow <shentey@gmail.com> wrote:
>
> hw/net/imx_fec and hw/net/lan9118 implement the same Ethernet PHY with similar
> but not quite the same code. This series consolidates the implementations into
> one to fix code duplication. It then continues to make the code more readable by
> reusing some existing constants.
>
> Having a dedicated module for the PHY allows it to be reused in even further
> device models.
>
> Testing done:
> * Run my usual sabrelite load and verify that network works.
> * Build and run Buildroot's qemu_arm_vexpress_defconfig which includes lan9118
> and check on serial console that IP address gets assigned.
>
> v3:
> * Don't add SOFT reset, use device-specific method instead (Peter)
> * Reuse MII constants for MII_ANLPAR register which was previously missed
> * Fix off-by-one error in MII_ANLPAR register which prevented MII constants to
> be reused
>
> v2:
> * QOM'ify + mention boards whose migratability will be broken (Peter)
I've added this series to my list of patches to add to
target-arm.next once 9.2 is released and we reopen the
tree for 10.0.
thanks
-- PMM
^ permalink raw reply [flat|nested] 15+ messages in thread