[Qemu-devel] [PATCH v2 0/3] e1000: allow model/device_id selection on command line

[Gabriel L. Somlo]

This started out as a single patch (now patch 2/3):

    Allow selection of different card models from the qemu
    command line, to better accomodate a wider range of guests.

New in v2:

  - moved check for 8257x out of the way of QOM, as suggested by
    Michael (patch 1/3)

  - resolved "Signed-off-by" misunderstanding and miscellaneous style
    issues (patch 2/3)

  - modified e1000 test to check for all supported models, as suggested
    by Andreas (patch 3/3). I used eepro100-test.c as an example for
    this change.

On Wed, May 21, 2014 at 12:04:57PM +0300, Michael S. Tsirkin wrote:
> On Wed, May 21, 2014 at 10:40:54AM +0200, Andreas F?rber wrote:
> >    static inline uint16_t maybe?
> 
> inline in c file is an optimization hint for a compiler, so
> really useless without a benchmark.
> It's useful in headers since some compilers warn about unused
> non-inline static functions.

I went with "static inline uint16_t" in both 1/3 and 2/3, since each
only get called once, and I'm only using a function instead of a
preprocessor macro for aesthetic reasons. I'm not religious about it
though, so if anyone still objects I can change it to whatever we all
agree on :)

> > > +    d->phy_reg[PHY_ID2] = e1000_phy_id2_init(dev_id);
> 
> I would just pass E1000State to e1000_phy_id2_init.

So I'm still passing "uint16_t device_id" instead of E1000State,
because e1000_phy_id2_init() replaces "enum { PHY_ID2_INIT = ...}",
which goes before the E1000State typedef in the source, and I wanted
to have the new code show up in the same spot in the patch as the code
it replaces. That, and we wouldn't save anything either way, I'd still
have to de-ref E1000State to get at the device_id, either in
e1000_reset() or in e1000_phy_id2_init(). Again, not religious about
it, so I can change it if you really want me to :)

Thanks again,
  Gabriel

Gabriel L. Somlo (3):
  e1000: avoid relying on device id (and soon, QOM) on data path
  e1000: allow command-line selection of card model
  tests: e1000: test additional device IDs

 hw/net/e1000.c     | 127 ++++++++++++++++++++++++++++++++++++++++++++++-------
 tests/e1000-test.c |  33 +++++++++++---
 2 files changed, 136 insertions(+), 24 deletions(-)

-- 
1.9.0