Re: [Qemu-devel] [PATCH 3/3] nvram: at24c: use a sane default for "rom-size"

[Wolfram Sang]

Hi Philippe,

> >  static Property at24c_eeprom_props[] = {
> > -    DEFINE_PROP_UINT32("rom-size", EEPROMState, rsize, 0),
> > +    DEFINE_PROP_UINT32("rom-size", EEPROMState, rsize, 128),
> 
> This patch should goes before your 2/3 in your series.

I don't mind much, but why? My reasoning was "let's first fix the cause
and then the symptom"?

> Can you add a #define for this value? Such AT24C_ROMSIZE_MIN.

Can do, of course. But won't that give room for regressions because
people are already using it with lower values?

Ideally, we would have a "model" variable. The model type would define
the size of the memory. The "rom-size" variable could then be kept as is
(except for the 0 bugfix) or deprecated?

Thanks for the review,

   Wolfram

signature.asc
Description: PGP signature