On Tue, Jan 17, 2023 at 10:32:15AM -0800, Peter Delevoryas wrote: > On Tue, Jan 17, 2023 at 09:00:34AM +0100, Philippe Mathieu-Daudé wrote: > > On 17/1/23 00:56, Peter Delevoryas wrote: > > > This helper is useful in board initialization because lets users > > > initialize and > > > realize an EEPROM on an I2C bus with a single function call. > > > > > > Signed-off-by: Peter Delevoryas <pe...@pjd.dev> > > > --- > > > hw/arm/aspeed.c | 10 +--------- > > > hw/arm/npcm7xx_boards.c | 20 +++++--------------- > > > hw/nvram/eeprom_at24c.c | 12 ++++++++++++ > > > include/hw/nvram/eeprom_at24c.h | 10 ++++++++++ > > > 4 files changed, 28 insertions(+), 24 deletions(-) > > > create mode 100644 include/hw/nvram/eeprom_at24c.h > > > > > diff --git a/include/hw/nvram/eeprom_at24c.h > > > b/include/hw/nvram/eeprom_at24c.h > > > new file mode 100644 > > > index 000000000000..79a36b53ca87 > > > --- /dev/null > > > +++ b/include/hw/nvram/eeprom_at24c.h > > > @@ -0,0 +1,10 @@ > > > +/* Copyright (c) Meta Platforms, Inc. and affiliates. */ > > > > What license for this copyright? > > Erg, yeah, thanks for calling this out, I did this wrong. Meta has some new > licensing guidelines and I misinterpreted them. Contributors are just supposed > to use whatever license the open-source project has, so I'll just change this > to say it's under GPL2, like the one I used in hw/arm/fby35.c > > > > > > +#ifndef EEPROM_AT24C_H > > > +#define EEPROM_AT24C_H > > > + > > > +#include "hw/i2c/i2c.h" > > > > /** > > * Create and realize an AT24C EEPROM device on the heap. > > * @bus: I2C bus to put it on > > * @addr: I2C address of the EEPROM slave when put on a bus > > * @rom_size: size of the EEPROM > > * > > * Create the device state structure, initialize it, put it on > > * the specified @bus, and drop the reference to it (the device > > * is realized). > > */ > > I2CSlave *at24c_eeprom_create_simple(I2CBus *bus, uint8_t addr, > > size_t rom_size); > > +1, I'll include this comment
Oh, to clarify though: I'm not going to include the rename to the function, maybe we could do that separately? I kinda want to avoid touching all the at24c_eeprom_init calls unless I really need to. I know it's just a simple sed, but also, smbus_eeprom_init is using the "init" suffix, so I'm not sure it's consistent, although "create_simple" probably _is_ more consistent with devices in general in QEMU. But anyways, main point, I just want to avoid making any unnecessary refactoring here, and renaming it completely seems unnecessary. > > > > > > +I2CSlave *at24c_eeprom_init(I2CBus *bus, uint8_t address, uint32_t > > > rom_size); > > > + > > > +#endif > > >