* [PATCH 1/4] hw/net/lan9118: Extract lan9118_phy
2024-10-05 20:57 [PATCH 0/4] Consolidate lan9118 phy implementations Bernhard Beschow
@ 2024-10-05 20:57 ` Bernhard Beschow
2024-10-14 12:47 ` Peter Maydell
2024-10-05 20:57 ` [PATCH 2/4] hw/net/lan9118_phy: Reuse in imx_fec and consolidate implementations Bernhard Beschow
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Bernhard Beschow @ 2024-10-05 20:57 UTC (permalink / raw)
To: qemu-devel
Cc: Jason Wang, qemu-arm, Peter Maydell, Paolo Bonzini,
Bernhard Beschow
A very similar implementation of the same device exists in imx_fec. Prepare for
a common implementation by extracting the code into its own files.
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
include/hw/net/lan9118_phy.h | 31 ++++++++
hw/net/lan9118.c | 133 ++++++-----------------------------
hw/net/lan9118_phy.c | 117 ++++++++++++++++++++++++++++++
hw/net/Kconfig | 4 ++
hw/net/meson.build | 1 +
5 files changed, 174 insertions(+), 112 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..50e3559b12
--- /dev/null
+++ b/include/hw/net/lan9118_phy.h
@@ -0,0 +1,31 @@
+/*
+ * 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 "hw/irq.h"
+
+typedef struct Lan9118PhyState {
+ uint32_t status;
+ uint32_t control;
+ uint32_t advertise;
+ uint32_t ints;
+ uint32_t int_mask;
+ IRQState irq;
+ bool link_down;
+} Lan9118PhyState;
+
+void lan9118_phy_update_link(Lan9118PhyState *s, bool link_down);
+void lan9118_phy_reset(Lan9118PhyState *s);
+uint32_t lan9118_phy_read(Lan9118PhyState *s, int reg);
+void lan9118_phy_write(Lan9118PhyState *s, int reg, uint32_t val);
+
+#endif
diff --git a/hw/net/lan9118.c b/hw/net/lan9118.c
index db28a0ef30..07702e8b4d 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,7 @@ 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;
int32_t eeprom_writable;
uint8_t eeprom[128];
@@ -301,11 +290,11 @@ 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_UINT32(mii.status, lan9118_state),
+ VMSTATE_UINT32(mii.control, lan9118_state),
+ VMSTATE_UINT32(mii.advertise, lan9118_state),
+ VMSTATE_UINT32(mii.ints, lan9118_state),
+ VMSTATE_UINT32(mii.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 +374,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 +386,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,7 +446,7 @@ static void lan9118_reset(DeviceState *d)
s->read_word_n = 0;
s->write_word_n = 0;
- phy_reset(s);
+ lan9118_phy_reset(&s->mii);
s->eeprom_writable = 0;
lan9118_reload_eeprom(s);
@@ -678,7 +646,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 +802,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 +835,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 +1032,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);
@@ -1383,6 +1289,9 @@ static void lan9118_realize(DeviceState *dev, Error **errp)
object_get_typename(OBJECT(dev)), dev->id,
&dev->mem_reentrancy_guard, s);
qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
+
+ qemu_init_irq(&s->mii.irq, lan9118_update_irq, s, 0);
+
s->eeprom[0] = 0xa5;
for (i = 0; i < 6; i++) {
s->eeprom[i + 1] = s->conf.macaddr.a[i];
diff --git a/hw/net/lan9118_phy.c b/hw/net/lan9118_phy.c
new file mode 100644
index 0000000000..ced9afce28
--- /dev/null
+++ b/hw/net/lan9118_phy.c
@@ -0,0 +1,117 @@
+/*
+ * 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 "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));
+}
+
+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->status = 0x7809;
+ s->control = 0x3000;
+ s->advertise = 0x01e1;
+ s->int_mask = 0;
+ s->ints = 0;
+ lan9118_phy_update_link(s, s->link_down);
+}
+
+uint32_t lan9118_phy_read(Lan9118PhyState *s, int reg)
+{
+ uint32_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, uint32_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);
+ }
+}
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.46.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 1/4] hw/net/lan9118: Extract lan9118_phy
2024-10-05 20:57 ` [PATCH 1/4] hw/net/lan9118: Extract lan9118_phy Bernhard Beschow
@ 2024-10-14 12:47 ` Peter Maydell
2024-10-14 18:38 ` Bernhard Beschow
0 siblings, 1 reply; 13+ messages in thread
From: Peter Maydell @ 2024-10-14 12:47 UTC (permalink / raw)
To: Bernhard Beschow; +Cc: qemu-devel, Jason Wang, qemu-arm, Paolo Bonzini
On Sat, 5 Oct 2024 at 21: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 the code into its own files.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
> include/hw/net/lan9118_phy.h | 31 ++++++++
> hw/net/lan9118.c | 133 ++++++-----------------------------
> hw/net/lan9118_phy.c | 117 ++++++++++++++++++++++++++++++
> hw/net/Kconfig | 4 ++
> hw/net/meson.build | 1 +
> 5 files changed, 174 insertions(+), 112 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..50e3559b12
> --- /dev/null
> +++ b/include/hw/net/lan9118_phy.h
> @@ -0,0 +1,31 @@
> +/*
> + * 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 "hw/irq.h"
> +
> +typedef struct Lan9118PhyState {
> + uint32_t status;
> + uint32_t control;
> + uint32_t advertise;
> + uint32_t ints;
> + uint32_t int_mask;
> + IRQState irq;
> + bool link_down;
> +} Lan9118PhyState;
This takes state that was in a QOM object, and moves it
into something that's kind of a device but not a QOM
object. I think we should avoid that, because at some
point somebody's going to have to QOMify this.
Making this a QOM device is a bit awkward for migration
compatibility, unfortunately.
thanks
-- PMM
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/4] hw/net/lan9118: Extract lan9118_phy
2024-10-14 12:47 ` Peter Maydell
@ 2024-10-14 18:38 ` Bernhard Beschow
2024-10-15 9:27 ` Peter Maydell
0 siblings, 1 reply; 13+ messages in thread
From: Bernhard Beschow @ 2024-10-14 18:38 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, Jason Wang, qemu-arm, Paolo Bonzini
Am 14. Oktober 2024 12:47:52 UTC schrieb Peter Maydell <peter.maydell@linaro.org>:
>On Sat, 5 Oct 2024 at 21: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 the code into its own files.
>>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>> include/hw/net/lan9118_phy.h | 31 ++++++++
>> hw/net/lan9118.c | 133 ++++++-----------------------------
>> hw/net/lan9118_phy.c | 117 ++++++++++++++++++++++++++++++
>> hw/net/Kconfig | 4 ++
>> hw/net/meson.build | 1 +
>> 5 files changed, 174 insertions(+), 112 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..50e3559b12
>> --- /dev/null
>> +++ b/include/hw/net/lan9118_phy.h
>> @@ -0,0 +1,31 @@
>> +/*
>> + * 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 "hw/irq.h"
>> +
>> +typedef struct Lan9118PhyState {
>> + uint32_t status;
>> + uint32_t control;
>> + uint32_t advertise;
>> + uint32_t ints;
>> + uint32_t int_mask;
>> + IRQState irq;
>> + bool link_down;
>> +} Lan9118PhyState;
>
>This takes state that was in a QOM object, and moves it
>into something that's kind of a device but not a QOM
>object. I think we should avoid that, because at some
>point somebody's going to have to QOMify this.
>
>Making this a QOM device is a bit awkward for migration
>compatibility, unfortunately.
Do we care about migration compatibility here? Or is it sufficient to check the version? In the latter case I could QOMify it.
Best regards,
Bernhard
>
>thanks
>-- PMM
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/4] hw/net/lan9118: Extract lan9118_phy
2024-10-14 18:38 ` Bernhard Beschow
@ 2024-10-15 9:27 ` Peter Maydell
2024-10-15 16:49 ` Bernhard Beschow
2024-10-16 21:44 ` Bernhard Beschow
0 siblings, 2 replies; 13+ messages in thread
From: Peter Maydell @ 2024-10-15 9:27 UTC (permalink / raw)
To: Bernhard Beschow; +Cc: qemu-devel, Jason Wang, qemu-arm, Paolo Bonzini
On Mon, 14 Oct 2024 at 19:50, Bernhard Beschow <shentey@gmail.com> wrote:
>
>
>
> Am 14. Oktober 2024 12:47:52 UTC schrieb Peter Maydell <peter.maydell@linaro.org>:
> >> +typedef struct Lan9118PhyState {
> >> + uint32_t status;
> >> + uint32_t control;
> >> + uint32_t advertise;
> >> + uint32_t ints;
> >> + uint32_t int_mask;
> >> + IRQState irq;
> >> + bool link_down;
> >> +} Lan9118PhyState;
> >
> >This takes state that was in a QOM object, and moves it
> >into something that's kind of a device but not a QOM
> >object. I think we should avoid that, because at some
> >point somebody's going to have to QOMify this.
> >
> >Making this a QOM device is a bit awkward for migration
> >compatibility, unfortunately.
>
> Do we care about migration compatibility here? Or is it
> sufficient to check the version? In the latter case I could
> QOMify it.
Doing a quick grep it looks like the lan9118 is only
used in a set of Arm boards and none of them are ones where
we care about migration across versions. So I think we're
ok to break compat with a version-bump. We should mention
the affected boards in the commit message.
-- PMM
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/4] hw/net/lan9118: Extract lan9118_phy
2024-10-15 9:27 ` Peter Maydell
@ 2024-10-15 16:49 ` Bernhard Beschow
2024-10-16 21:44 ` Bernhard Beschow
1 sibling, 0 replies; 13+ messages in thread
From: Bernhard Beschow @ 2024-10-15 16:49 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, Jason Wang, qemu-arm, Paolo Bonzini
Am 15. Oktober 2024 09:27:40 UTC schrieb Peter Maydell <peter.maydell@linaro.org>:
>On Mon, 14 Oct 2024 at 19:50, Bernhard Beschow <shentey@gmail.com> wrote:
>>
>>
>>
>> Am 14. Oktober 2024 12:47:52 UTC schrieb Peter Maydell <peter.maydell@linaro.org>:
>> >> +typedef struct Lan9118PhyState {
>> >> + uint32_t status;
>> >> + uint32_t control;
>> >> + uint32_t advertise;
>> >> + uint32_t ints;
>> >> + uint32_t int_mask;
>> >> + IRQState irq;
>> >> + bool link_down;
>> >> +} Lan9118PhyState;
>> >
>> >This takes state that was in a QOM object, and moves it
>> >into something that's kind of a device but not a QOM
>> >object. I think we should avoid that, because at some
>> >point somebody's going to have to QOMify this.
>> >
>> >Making this a QOM device is a bit awkward for migration
>> >compatibility, unfortunately.
>>
>> Do we care about migration compatibility here? Or is it
>> sufficient to check the version? In the latter case I could
>> QOMify it.
>
>
>Doing a quick grep it looks like the lan9118 is only
>used in a set of Arm boards and none of them are ones where
>we care about migration across versions.
Four i.mx boards using imx_fec will also be affected. None is versioned afaics.
>So I think we're
>ok to break compat with a version-bump. We should mention
>the affected boards in the commit message.
Will do.
Thanks,
Bernhard
>
>-- PMM
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/4] hw/net/lan9118: Extract lan9118_phy
2024-10-15 9:27 ` Peter Maydell
2024-10-15 16:49 ` Bernhard Beschow
@ 2024-10-16 21:44 ` Bernhard Beschow
1 sibling, 0 replies; 13+ messages in thread
From: Bernhard Beschow @ 2024-10-16 21:44 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, Jason Wang, qemu-arm, Paolo Bonzini
Am 15. Oktober 2024 09:27:40 UTC schrieb Peter Maydell <peter.maydell@linaro.org>:
>On Mon, 14 Oct 2024 at 19:50, Bernhard Beschow <shentey@gmail.com> wrote:
>>
>>
>>
>> Am 14. Oktober 2024 12:47:52 UTC schrieb Peter Maydell <peter.maydell@linaro.org>:
>> >> +typedef struct Lan9118PhyState {
>> >> + uint32_t status;
>> >> + uint32_t control;
>> >> + uint32_t advertise;
>> >> + uint32_t ints;
>> >> + uint32_t int_mask;
>> >> + IRQState irq;
>> >> + bool link_down;
>> >> +} Lan9118PhyState;
>> >
>> >This takes state that was in a QOM object, and moves it
>> >into something that's kind of a device but not a QOM
>> >object. I think we should avoid that, because at some
>> >point somebody's going to have to QOMify this.
>> >
>> >Making this a QOM device is a bit awkward for migration
>> >compatibility, unfortunately.
>>
>> Do we care about migration compatibility here? Or is it
>> sufficient to check the version? In the latter case I could
>> QOMify it.
>
>
>Doing a quick grep it looks like the lan9118 is only
>used in a set of Arm boards and none of them are ones where
>we care about migration across versions. So I think we're
>ok to break compat with a version-bump. We should mention
>the affected boards in the commit message.
V2 sent: <https://lore.kernel.org/qemu-devel/20241016212407.139390-1-shentey@gmail.com/>
Best regards,
Bernhard
>
>-- PMM
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/4] hw/net/lan9118_phy: Reuse in imx_fec and consolidate implementations
2024-10-05 20:57 [PATCH 0/4] Consolidate lan9118 phy implementations Bernhard Beschow
2024-10-05 20:57 ` [PATCH 1/4] hw/net/lan9118: Extract lan9118_phy Bernhard Beschow
@ 2024-10-05 20:57 ` Bernhard Beschow
2024-10-05 20:57 ` [PATCH 3/4] hw/net/lan9118_phy: Reuse MII constants Bernhard Beschow
` (2 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Bernhard Beschow @ 2024-10-05 20:57 UTC (permalink / raw)
To: qemu-devel
Cc: Jason Wang, qemu-arm, Peter Maydell, Paolo Bonzini,
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.
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
include/hw/net/imx_fec.h | 7 +-
hw/net/imx_fec.c | 141 ++++-----------------------------------
hw/net/lan9118_phy.c | 80 ++++++++++++++++------
hw/net/Kconfig | 1 +
hw/net/trace-events | 10 +--
5 files changed, 81 insertions(+), 158 deletions(-)
diff --git a/include/hw/net/imx_fec.h b/include/hw/net/imx_fec.h
index 2d13290c78..5f1d30f56b 100644
--- a/include/hw/net/imx_fec.h
+++ b/include/hw/net/imx_fec.h
@@ -31,6 +31,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(IMXFECState, IMX_FEC)
#define TYPE_IMX_ENET "imx.enet"
#include "hw/sysbus.h"
+#include "hw/net/lan9118_phy.h"
#include "net/net.h"
#define ENET_EIR 1
@@ -264,11 +265,7 @@ 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;
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..e981116640 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -209,11 +209,11 @@ static const VMStateDescription vmstate_imx_eth = {
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_UINT32(mii.status, IMXFECState),
+ VMSTATE_UINT32(mii.control, IMXFECState),
+ VMSTATE_UINT32(mii.advertise, IMXFECState),
+ VMSTATE_UINT32(mii.ints, IMXFECState),
+ VMSTATE_UINT32(mii.int_mask, IMXFECState),
VMSTATE_END_OF_LIST()
},
.subsections = (const VMStateDescription * const []) {
@@ -222,14 +222,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 +230,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 +260,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 +282,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)
@@ -684,7 +569,7 @@ static void imx_eth_reset(DeviceState *d)
memset(s->tx_descriptor, 0, sizeof(s->tx_descriptor));
/* We also reset the PHY */
- imx_phy_reset(s);
+ lan9118_phy_reset(&s->mii);
}
static uint32_t imx_default_read(IMXFECState *s, uint32_t index)
@@ -1336,6 +1221,8 @@ static void imx_eth_realize(DeviceState *dev, Error **errp)
dev->id, &dev->mem_reentrancy_guard, s);
qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a);
+
+ qemu_init_irq(&s->mii.irq, imx_phy_update_irq, s, 0);
}
static Property imx_eth_properties[] = {
diff --git a/hw/net/lan9118_phy.c b/hw/net/lan9118_phy.c
index ced9afce28..1ab4bbc900 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
@@ -14,6 +16,7 @@
#include "hw/net/lan9118_phy.h"
#include "hw/irq.h"
#include "qemu/log.h"
+#include "trace.h"
#define PHY_INT_ENERGYON (1 << 7)
#define PHY_INT_AUTONEG_COMPLETE (1 << 6)
@@ -34,9 +37,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;
@@ -46,6 +51,8 @@ void lan9118_phy_update_link(Lan9118PhyState *s, bool link_down)
void lan9118_phy_reset(Lan9118PhyState *s)
{
+ trace_lan9118_phy_reset();
+
s->status = 0x7809;
s->control = 0x3000;
s->advertise = 0x01e1;
@@ -60,58 +67,87 @@ uint32_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, uint32_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;
}
}
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 4c6687923e..b8e0076833 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(uint32_t val, int reg) "[0x%02x] -> 0x%04" PRIx32
+lan9118_phy_write(uint32_t val, int reg) "[0x%02x] <- 0x%04" PRIx32
+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"
@@ -426,12 +432,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.46.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 3/4] hw/net/lan9118_phy: Reuse MII constants
2024-10-05 20:57 [PATCH 0/4] Consolidate lan9118 phy implementations Bernhard Beschow
2024-10-05 20:57 ` [PATCH 1/4] hw/net/lan9118: Extract lan9118_phy Bernhard Beschow
2024-10-05 20:57 ` [PATCH 2/4] hw/net/lan9118_phy: Reuse in imx_fec and consolidate implementations Bernhard Beschow
@ 2024-10-05 20:57 ` Bernhard Beschow
2024-10-14 12:56 ` Peter Maydell
2024-10-05 20:57 ` [PATCH 4/4] hw/net/lan9118_phy: Add missing 100 mbps full duplex advertisement Bernhard Beschow
2024-10-12 14:07 ` [PATCH 0/4] Consolidate lan9118 phy implementations Bernhard Beschow
4 siblings, 1 reply; 13+ messages in thread
From: Bernhard Beschow @ 2024-10-05 20:57 UTC (permalink / raw)
To: qemu-devel
Cc: Jason Wang, qemu-arm, Peter Maydell, Paolo Bonzini,
Bernhard Beschow
Prefer named constants over magic values for better readability.
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
include/hw/net/mii.h | 6 +++++
hw/net/lan9118_phy.c | 59 +++++++++++++++++++++++++++-----------------
2 files changed, 43 insertions(+), 22 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 1ab4bbc900..1c2f30c81a 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 "qemu/log.h"
#include "trace.h"
@@ -38,11 +39,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;
}
@@ -53,9 +54,18 @@ void lan9118_phy_reset(Lan9118PhyState *s)
{
trace_lan9118_phy_reset();
- s->status = 0x7809;
- s->control = 0x3000;
- s->advertise = 0x01e1;
+ 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->control = MII_BMCR_AUTOEN | MII_BMCR_SPEED100;
+ 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);
@@ -66,26 +76,26 @@ uint32_t lan9118_phy_read(Lan9118PhyState *s, int reg)
uint32_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 */
+ case MII_ANLPAR:
val = 0x0f71;
break;
- case 6: /* Auto-neg Expansion */
- val = 1;
+ case MII_ANER:
+ val = MII_ANER_NWAY;
break;
case 29: /* Interrupt source. */
val = s->ints;
@@ -120,19 +130,24 @@ void lan9118_phy_write(Lan9118PhyState *s, int reg, uint32_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;
--
2.46.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 4/4] hw/net/lan9118_phy: Add missing 100 mbps full duplex advertisement
2024-10-05 20:57 [PATCH 0/4] Consolidate lan9118 phy implementations Bernhard Beschow
` (2 preceding siblings ...)
2024-10-05 20:57 ` [PATCH 3/4] hw/net/lan9118_phy: Reuse MII constants Bernhard Beschow
@ 2024-10-05 20:57 ` Bernhard Beschow
2024-10-14 12:52 ` Peter Maydell
2024-10-12 14:07 ` [PATCH 0/4] Consolidate lan9118 phy implementations Bernhard Beschow
4 siblings, 1 reply; 13+ messages in thread
From: Bernhard Beschow @ 2024-10-05 20:57 UTC (permalink / raw)
To: qemu-devel
Cc: Jason Wang, qemu-arm, Peter Maydell, Paolo Bonzini,
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.
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 1c2f30c81a..4815a4da35 100644
--- a/hw/net/lan9118_phy.c
+++ b/hw/net/lan9118_phy.c
@@ -145,8 +145,8 @@ void lan9118_phy_write(Lan9118PhyState *s, int reg, uint32_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.46.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 4/4] hw/net/lan9118_phy: Add missing 100 mbps full duplex advertisement
2024-10-05 20:57 ` [PATCH 4/4] hw/net/lan9118_phy: Add missing 100 mbps full duplex advertisement Bernhard Beschow
@ 2024-10-14 12:52 ` Peter Maydell
0 siblings, 0 replies; 13+ messages in thread
From: Peter Maydell @ 2024-10-14 12:52 UTC (permalink / raw)
To: Bernhard Beschow; +Cc: qemu-devel, Jason Wang, qemu-arm, Paolo Bonzini
On Sat, 5 Oct 2024 at 21:58, Bernhard Beschow <shentey@gmail.com> wrote:
>
> 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.
>
> 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 1c2f30c81a..4815a4da35 100644
> --- a/hw/net/lan9118_phy.c
> +++ b/hw/net/lan9118_phy.c
> @@ -145,8 +145,8 @@ void lan9118_phy_write(Lan9118PhyState *s, int reg, uint32_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;
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/4] Consolidate lan9118 phy implementations
2024-10-05 20:57 [PATCH 0/4] Consolidate lan9118 phy implementations Bernhard Beschow
` (3 preceding siblings ...)
2024-10-05 20:57 ` [PATCH 4/4] hw/net/lan9118_phy: Add missing 100 mbps full duplex advertisement Bernhard Beschow
@ 2024-10-12 14:07 ` Bernhard Beschow
4 siblings, 0 replies; 13+ messages in thread
From: Bernhard Beschow @ 2024-10-12 14:07 UTC (permalink / raw)
To: qemu-devel; +Cc: Jason Wang, qemu-arm, Peter Maydell, Paolo Bonzini
Am 5. Oktober 2024 20:57:44 UTC schrieb Bernhard Beschow <shentey@gmail.com>:
>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.
>
Ping
>
>
>Bernhard Beschow (4):
>
> hw/net/lan9118: Extract lan9118_phy
>
> hw/net/lan9118_phy: Reuse in imx_fec and consolidate implementations
>
> hw/net/lan9118_phy: Reuse MII constants
>
> hw/net/lan9118_phy: Add missing 100 mbps full duplex advertisement
>
>
>
> include/hw/net/imx_fec.h | 7 +-
>
> include/hw/net/lan9118_phy.h | 31 +++++++
>
> include/hw/net/mii.h | 6 ++
>
> hw/net/imx_fec.c | 141 +++--------------------------
>
> hw/net/lan9118.c | 133 +++++----------------------
>
> hw/net/lan9118_phy.c | 168 +++++++++++++++++++++++++++++++++++
>
> hw/net/Kconfig | 5 ++
>
> hw/net/meson.build | 1 +
>
> hw/net/trace-events | 10 ++-
>
> 9 files changed, 254 insertions(+), 248 deletions(-)
>
> create mode 100644 include/hw/net/lan9118_phy.h
>
> create mode 100644 hw/net/lan9118_phy.c
>
>
>
>-- >
>2.46.2
>
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread