[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 6/6] hw/arm/aspeed: Init fby35 BMC FRUID EEPROM
From: |
Peter Delevoryas |
Subject: |
Re: [PATCH 6/6] hw/arm/aspeed: Init fby35 BMC FRUID EEPROM |
Date: |
Tue, 17 Jan 2023 09:49:55 -0800 |
On Tue, Jan 17, 2023 at 07:47:06AM +0100, Philippe Mathieu-Daudé wrote:
> On 16/1/23 18:23, Peter Delevoryas wrote:
> > On Mon, Jan 16, 2023 at 01:30:19PM +0100, Philippe Mathieu-Daudé wrote:
> > > On 14/1/23 18:01, Peter Delevoryas wrote:
> > > > Signed-off-by: Peter Delevoryas <peter@pjd.dev>
> > > > ---
> > > > hw/arm/aspeed.c | 49
> > > > +++++++++++++++++++++++++++++++++++++++++++++++++
> > > > 1 file changed, 49 insertions(+)
> > > >
> > > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> > > > index c929c61d582a..4ac8ff11a835 100644
> > > > --- a/hw/arm/aspeed.c
> > > > +++ b/hw/arm/aspeed.c
> > > > @@ -922,6 +922,52 @@ static void
> > > > bletchley_bmc_i2c_init(AspeedMachineState *bmc)
> > > > i2c_slave_create_simple(i2c[12], TYPE_PCA9552, 0x67);
> > > > }
> > > > +static const uint8_t fby35_bmc_fruid[] = {
> > > [...]
> > >
> > > > +};
> > > > +
> > > > static void fby35_i2c_init(AspeedMachineState *bmc)
> > > > {
> > > > AspeedSoCState *soc = &bmc->soc;
> > > > @@ -1363,6 +1409,9 @@ static void fby35_reset(MachineState *state,
> > > > ShutdownCause reason)
> > > > object_property_set_bool(OBJECT(gpio), "gpioB3", false,
> > > > &error_fatal);
> > > > object_property_set_bool(OBJECT(gpio), "gpioB4", false,
> > > > &error_fatal);
> > > > object_property_set_bool(OBJECT(gpio), "gpioB5", false,
> > > > &error_fatal);
> > > > +
> > > > + at24c_eeprom_write(aspeed_i2c_get_bus(&bmc->soc.i2c, 11),
> > > > + 0x54, 0, fby35_bmc_fruid,
> > > > sizeof(fby35_bmc_fruid));
> > >
> > > Why transfer the prom content on the i2c bus at each reset?
> > >
> > > In particular this looks wrong if the prom is initialized with a 'drive'
> > > block backend (using -global).
> >
> > Yeah, it looks like this might not be the right way to model it. I'm going
> > to try Cedric's suggestions.
>
> OK, but watch out this is a PROM, not a ROM, meaning it is legitimate
> for a guest to reprogram it, and expect the reprogrammed content after
> reset.
+1
>
> Shouldn't we put the 'fill default content if no -drive provided' option
> in the device's realize() handler, to avoid overwriting content possibly
> updated by guest before reset?
+1, yeah I think you're right, if somebody is providing a -drive option, we
should allow that to override everything else (default zero initialization,
init ROM initialization, etc).
Because, if they're providing a -drive, they shouldn't need to provide an
initial value, they can just initialize the file with the data they want.
[PATCH 4/6] hw/arm/npcm7xx: Remove local copy of at24c_eeprom_init, Peter Delevoryas, 2023/01/14
Re: [PATCH 0/6] hw/nvram/eeprom_at24c: Cleanup + FRUID EEPROM init example, Peter Delevoryas, 2023/01/14