On Thu, Jun 23, 2022 at 10:16 AM Cédric Le Goater <
clg@kaod.org> wrote:
On 6/23/22 17:28, Patrick Venture wrote:
>
>
> On Wed, Jun 22, 2022 at 10:48 AM Jae Hyun Yoo <quic_jaehyoo@quicinc.com <mailto:quic_jaehyoo@quicinc.com>> wrote:
>
> From: Graeme Gregory <quic_ggregory@quicinc.com <mailto:quic_ggregory@quicinc.com>>
>
> The FRU device uses the index 0 device on bus IF_NONE.
>
> -drive file=$FRU,format=raw,if=none
>
> file must match FRU size of 128k
>
> Signed-off-by: Graeme Gregory <quic_ggregory@quicinc.com <mailto:quic_ggregory@quicinc.com>>
> ---
> hw/arm/aspeed.c | 22 +++++++++++++++++-----
> 1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 785cc543d046..36d6b2c33e48 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -992,17 +992,29 @@ static void fby35_i2c_init(AspeedMachineState *bmc)
> */
> }
>
> +static void qcom_dc_scm_fru_init(I2CBus *bus, uint8_t addr, uint32_t rsize)
> +{
> + I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr);
> + DeviceState *dev = DEVICE(i2c_dev);
> + /* Use First Index for DC-SCM FRU */
> + DriveInfo *dinfo = drive_get(IF_NONE, 0, 0);
> +
> + qdev_prop_set_uint32(dev, "rom-size", rsize);
> +
> + if (dinfo) {
> + qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
> + }
> +
> + i2c_slave_realize_and_unref(i2c_dev, bus, &error_abort);
> +}
>
>
> We've sent a similar patch up for the at24c but in its own file -- but looking at this, we could likely expand it to suite our cases as well - is there a reason it's named qcom_dc_scm_fru_init? Presumably that's to handle the drive_get parameters. If you make it use `drive_get(IF_NONE, bus, unit);` You'll be able to associate a drive via parameters like you aim to.
I have seen various attempts to populate the Aspeed eeproms with
data. The simplest is the g220a_bmc_i2c_init() approach. Some are
generating C code from the .bin files and compiling the result in
QEMU. Some were generating elf sections, linking them in QEMU and
passing the pointer as an eeprom buf.
The drive approach is the best I think, but the device/drive
association depends on the drive order on the command line when
devices are created by the platform.
We could may be extend the GenericLoaderState for eeproms ?
or introduce a routine which would look for a file under a (pc-bios)
per-machine directory and fill the eeprom buf with its contents ?
Any idea ?
So we have this in our eeprom_at24c module because we use it with Aspeed and Nuvoton:
void at24c_eeprom_init_one(I2CBus *i2c_bus, int bus, uint8_t addr,
uint32_t rsize, int unit_number)
{
I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr);
DeviceState *dev = DEVICE(i2c_dev);
BlockInterfaceType type = IF_NONE;
DriveInfo *dinfo;
dinfo = drive_get(type, bus, unit_number);
if (dinfo) {
qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
}
qdev_prop_set_uint32(dev, "rom-size", rsize);
i2c_slave_realize_and_unref(i2c_dev, i2c_bus, &error_abort);
}
With this style call in the board:
<snip>
/* mb_fru */
at24c_eeprom_init_one(npcm7xx_i2c_get_bus(soc, 5), 5, 0x50, 8192, 0);
<snip>
/* fan_fru */
at24c_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 2), 5, 0x51, 8192, 1);
/* hsbp_fru */
at24c_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 3), 5, 0x52, 8192, 2);
<snip>
And then we call it with the bus and unit, the pair of those has to be unique for the drive parameter to work:
-drive file=/export/hda3/tmp/mb_fru@50,if=none,bus=5,unit=0,snapshot=off,format=raw
-drive file=/export/hda3/tmp/fan_fru@51,if=none,bus=5,unit=1,snapshot=off,format=raw
-drive file=/export/hda3/tmp/hsbp_fru@52,if=none,bus=5,unit=2,snapshot=off,format=raw
The above code snippet is what, I'm fairly certain we had sent up for review before, and we can send it again if you want.
Thanks,
C.