Re: [PATCH v2 4/5] hw/arm/aspeed: Add aspeed_eeprom.c

[Peter Delevoryas <peter@pjd.dev>]

On Tue, Jan 17, 2023 at 09:08:57AM +0100, Philippe Mathieu-Daudé wrote:
> On 17/1/23 08:39, Cédric Le Goater wrote:
> > On 1/17/23 00:56, Peter Delevoryas wrote:
> > > - Create aspeed_eeprom.c and aspeed_eeprom.h
> > > - Include aspeed_eeprom.c in CONFIG_ASPEED meson source files
> > > - Include aspeed_eeprom.h in aspeed.c
> > > - Add fby35_bmc_fruid data
> > > - Use new at24c_eeprom_init_rom helper to initialize BMC FRUID
> > > EEPROM with data
> > >    from aspeed_eeprom.c
> [...]
> 
> > > diff --git a/hw/arm/aspeed_eeprom.h b/hw/arm/aspeed_eeprom.h
> > > new file mode 100644
> > > index 000000000000..89860e37d007
> > > --- /dev/null
> > > +++ b/hw/arm/aspeed_eeprom.h
> > > @@ -0,0 +1,11 @@
> > > +/* Copyright (c) Meta Platforms, Inc. and affiliates. */
> 
> Missing license.

+1, will fix

> 
> > > +#ifndef ASPEED_EEPROM_H
> > > +#define ASPEED_EEPROM_H
> > > +
> > > +#include "qemu/osdep.h"
> > > +
> > > +extern const uint8_t fby35_bmc_fruid[];
> > 
> > 
> > may be define the array with an explicit size to avoid the size variable ?
> > I don't see any good solution.
>  /* Return rom_size and set rombufptr, or return 0 */
>  size_t aspeed_get_default_rom_content(const char *machine_typename,
>                                        const void **rombufptr);
> 
> ?


Hmmm I don't think this would work, cause actually there are more FRUID
EEPROM's than just this one. I only added this one in this commit, but there's
also FRUID EEPROM's from the network card and baseboard. I'll include those 2
EEPROM's in the next version to illustrate.