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

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

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

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

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

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