ping ? On Wed, May 21, 2014 at 02:27:41PM -0400, Gabriel L. Somlo wrote: > 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)
http://patchwork.ozlabs.org/patch/351274/ > - resolved "Signed-off-by" misunderstanding and miscellaneous style > issues (patch 2/3) http://patchwork.ozlabs.org/patch/351273/ > - 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. http://patchwork.ozlabs.org/patch/351275/ > 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 >
