Re: [Qemu-devel] [PATCH v2 2/3] e1000: allow command-line selection of card model

[Peter Crosthwaite]

On Fri, May 30, 2014 at 2:00 AM, Gabriel L. Somlo <address@hidden> wrote:
> 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"

Well class_init is open coded and gives you a chance to implement
semantics like that when translating the Info struct to the actual
usable classes. If you do it that early stages, then all your classes
will have correct default phy_id2 with only one LOC needed. I am
however assuming 0 is not a valid phy_id2?

> and a default "is_8257x = false"

And a "false" or 0 default cost you nothing, as you just rely on
static const structs init to 0 anyway.

Regards,
Peter