Hi Peter,
Thanks for the QOM crash course !
So I did s/TYPE_E1000/TYPE_E1000_BASE/ as suggested by Stefan, and
also moved PCI_VENDOR_ID_INTEL back into e1000_class_init() and away
from e1000_devices[] as you suggested. That's going to be part of
v3, as soon as we're done talking about QOM, see below :)
On Wed, May 28, 2014 at 11:02:10PM +1000, Peter Crosthwaite wrote:
> So I would guess with a bit more QOMification of the subtypes, it's
> possible to promote the phy_id2 to class data and remove the (late)
> lookup giving some decoupling between PCI dev id and this phy id.
>
> I'll make some notes through the patch on the basic idea.
>
> You could also do it for your is_8257x flag, just set it true in the
> .info's that matter. Then grab it from class to obj at init time
> (slightly optional, but avoids QOM-in-data-path).
I've attached a new patch (could be fourth in the series, or merged in
with the main dev_id patch. My only concern regarding a replacement of
e1000_phy_id2_init() and e1000_is_8257x() is that now I'll *have* to
include a .phy_id2 (and possibly a .is_8257x) field with every entry
in the e1000_devices[] array (unless there's a way to set a default
"phy_id2 = 0xc20" and a default "is_8257x = false" somewhere, and
inherit it unless explicitly overridden for the relevant device IDs
during initialization ?
Note that we're not even supporting any 8257x cards, so the switch
statement is just there for documentation purposes at this time...
Thanks much,
--Gabriel
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 7a1a681..33a93ba 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -160,11 +160,21 @@ typedef struct E1000State_st {
bool is_8257x;
} E1000State;
+typedef struct E1000DeviceClass {
+ PCIDeviceClass parent_class;
+ uint16_t phy_id2;
+} E1000DeviceClass;
+
#define TYPE_E1000_BASE "e1000-base"
#define E1000(obj) \
OBJECT_CHECK(E1000State, (obj), TYPE_E1000_BASE)
+#define E1000_DEVICE_CLASS(klass) \
+ OBJECT_CLASS_CHECK(E1000DeviceClass, (klass), TYPE_E1000_BASE)
+#define E1000_DEVICE_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(E1000DeviceClass, (obj), TYPE_E1000_BASE)
+
#define defreg(x) x = (E1000_##x>>2)
enum {
defreg(CTRL), defreg(EECD), defreg(EERD), defreg(GPRC),
@@ -384,7 +394,7 @@ rxbufsize(uint32_t v)
static void e1000_reset(void *opaque)
{
E1000State *d = opaque;
- PCIDeviceClass *pdc = PCI_DEVICE_GET_CLASS(d);
+ E1000DeviceClass *edc = E1000_DEVICE_GET_CLASS(d);
uint8_t *macaddr = d->conf.macaddr.a;
int i;
@@ -395,7 +405,7 @@ static void e1000_reset(void *opaque)
d->mit_ide = 0;
memset(d->phy_reg, 0, sizeof d->phy_reg);
memmove(d->phy_reg, phy_reg_init, sizeof phy_reg_init);
- d->phy_reg[PHY_ID2] = e1000_phy_id2_init(pdc->device_id);
+ d->phy_reg[PHY_ID2] = edc->phy_id2;
memset(d->mac_reg, 0, sizeof d->mac_reg);
memmove(d->mac_reg, mac_reg_init, sizeof mac_reg_init);
d->rxbuf_min_shift = 1;
@@ -1613,12 +1623,14 @@ typedef struct E1000Info_st {
const char *name;
uint16_t device_id;
uint8_t revision;
+ uint16_t phy_id2;
} E1000Info;
static void e1000_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+ E1000DeviceClass *e = E1000_DEVICE_CLASS(klass);
const E1000Info *info = data;
k->init = pci_e1000_init;
@@ -1627,6 +1639,7 @@ static void e1000_class_init(ObjectClass *klass, void
*data)
k->vendor_id = PCI_VENDOR_ID_INTEL;
k->device_id = info->device_id;
k->revision = info->revision;
+ e->phy_id2 = info->phy_id2;
k->class_id = PCI_CLASS_NETWORK_ETHERNET;
set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
dc->desc = "Intel Gigabit Ethernet";
@@ -1639,29 +1652,33 @@ static const TypeInfo e1000_base_info = {
.name = TYPE_E1000_BASE,
.parent = TYPE_PCI_DEVICE,
.instance_size = sizeof(E1000State),
+ .class_size = sizeof(E1000DeviceClass),
.abstract = true,
};
+static const TypeInfo e1000_default_info = {
+ .name = "e1000",
+ .parent = "e1000-82540em",
+};
+
static const E1000Info e1000_devices[] = {
{
- .name = "e1000", /* default, alias for "e1000-82540em" */
- .device_id = E1000_DEV_ID_82540EM,
- .revision = 0x03,
- },
- {
.name = "e1000-82540em",
.device_id = E1000_DEV_ID_82540EM,
.revision = 0x03,
+ .phy_id2 = 0xc20,
},
{
.name = "e1000-82544gc",
.device_id = E1000_DEV_ID_82544GC_COPPER,
.revision = 0x03,
+ .phy_id2 = 0xc30,
},
{
.name = "e1000-82545em",
.device_id = E1000_DEV_ID_82545EM_COPPER,
.revision = 0x03,
+ .phy_id2 = 0xc20,
},
};
@@ -1670,6 +1687,7 @@ static void e1000_register_types(void)
int i;
type_register_static(&e1000_base_info);
+ type_register_static(&e1000_default_info);
for (i = 0; i < ARRAY_SIZE(e1000_devices); i++) {
const E1000Info *info = &e1000_devices[i];
TypeInfo type_info = {};
