Thanks for the comment. I'll upload a new version using IF_OTHER as discussed in that thread.
I don't know if adding the assertion for type != IF_NONE is a good idea now so I didn't add that. If you think that's good I can add it as well. On Wed, Nov 3, 2021 at 2:13 AM Thomas Huth <th...@redhat.com> wrote: > On 01/11/2021 18.47, Hao Wu wrote: > > > > > > On Mon, Nov 1, 2021 at 10:41 AM Peter Maydell <peter.mayd...@linaro.org > > <mailto:peter.mayd...@linaro.org>> wrote: > > > > On Thu, 21 Oct 2021 at 19:40, Hao Wu <wuhao...@google.com > > <mailto:wuhao...@google.com>> wrote: > > > > > > We made 3 changes to the at24c_eeprom_init function in > > > npcm7xx_boards.c: > > > > > > 1. We allow the function to take a I2CBus* as parameter. This > allows > > > us to attach an EEPROM device behind an I2C mux which is not > > > possible with the old method. > > > > > > 2. We make at24c EEPROMs are backed by drives so that we can > > > specify the content of the EEPROMs. > > > > > > 3. Instead of using i2c address as unit number, This patch assigns > > > unique unit numbers for each eeproms in each board. This avoids > > > conflict in providing multiple eeprom contents with the same > address. > > > In the old method if we specify two drives with the same unit > number, > > > the following error will occur: `Device with id 'none85' > exists`. > > > > > > Signed-off-by: Hao Wu <wuhao...@google.com <mailto: > wuhao...@google.com>> > > > --- > > > hw/arm/npcm7xx_boards.c | 15 ++++++++++----- > > > 1 file changed, 10 insertions(+), 5 deletions(-) > > > > > > diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c > > > index a656169f61..cdb52b9922 100644 > > > --- a/hw/arm/npcm7xx_boards.c > > > +++ b/hw/arm/npcm7xx_boards.c > > > @@ -107,13 +107,18 @@ static I2CBus > *npcm7xx_i2c_get_bus(NPCM7xxState > > *soc, uint32_t num) > > > return I2C_BUS(qdev_get_child_bus(DEVICE(&soc->smbus[num]), > > "i2c-bus")); > > > } > > > > > > -static void at24c_eeprom_init(NPCM7xxState *soc, int bus, > uint8_t addr, > > > - uint32_t rsize) > > > +static void at24c_eeprom_init(I2CBus *i2c_bus, int bus, uint8_t > addr, > > > + uint32_t rsize, int unit_number) > > > { > > > - I2CBus *i2c_bus = npcm7xx_i2c_get_bus(soc, bus); > > > I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr); > > > DeviceState *dev = DEVICE(i2c_dev); > > > + BlockInterfaceType type = IF_NONE; > > > > Why make this a variable? We only use it in one place... > > > > You're right, we can actually inline it. > > Actually, please do *not* use IF_NONE for such stuff. See the discussion > here for details: > > https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg00970.html > > Thomas > >
