qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 5/7] hw/nvram: Update at24c EEPROM init function in NPCM7x


From: Thomas Huth
Subject: Re: [PATCH v2 5/7] hw/nvram: Update at24c EEPROM init function in NPCM7xx boards
Date: Wed, 3 Nov 2021 10:13:14 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0

On 01/11/2021 18.47, Hao Wu wrote:


On Mon, Nov 1, 2021 at 10:41 AM Peter Maydell <peter.maydell@linaro.org <mailto:peter.maydell@linaro.org>> wrote:

    On Thu, 21 Oct 2021 at 19:40, Hao Wu <wuhaotsh@google.com
    <mailto:wuhaotsh@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 <wuhaotsh@google.com <mailto:wuhaotsh@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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]