Re: [PATCH] aspeed: Enable backend file for eeprom

[Cédric Le Goater]

On 7/28/22 09:20, John Wang wrote:
> Cédric Le Goater <clg@kaod.org> 于2022年7月28日周四 14:28写道：
> > Hello John,
> >
> > On 7/28/22 08:12, John Wang wrote:
> > > tested on a fp5280g2:
> > >
> > > $QEMU_BIN -machine fp5280g2-bmc \
> > >          -nographic \
> > >          -drive file="${IMAGE_PATH}",format=raw,if=mtd \
> > >          -drive file="eeprom.bin",format=raw,if=pflash,index=1 \
> > >          ${NIC}
> > >
> > > root@fp5280g2:/sys/bus/i2c/devices/1-0050# hexdump eeprom -C
> > > 00000000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> > > *
> > > ....
> > > ....
> > > 00000240  2c 87 a3 a4 1d d3 11 b2  02 d2 c2 9d 44 60 cf 3e  |,...........D`.>|
> > > 00000250  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> > >
> > > It's same as the "eeprom.bin"
> > >
> > > Signed-off-by: John Wang <wangzhiqiang02@inspur.com>
> > > Change-Id: I5c44785a028144b24aa0b22643266d83addc5eab
> > > ---
> > >    hw/arm/aspeed.c | 16 ++++++++++++----
> > >    1 file changed, 12 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> > > index 4193a3d23d..80aa687372 100644
> > > --- a/hw/arm/aspeed.c
> > > +++ b/hw/arm/aspeed.c
> > > @@ -431,12 +431,20 @@ static void aspeed_machine_init(MachineState *machine)
> > >        arm_load_kernel(ARM_CPU(first_cpu), machine, &aspeed_board_binfo);
> > >    }
> > >
> > > -static void at24c_eeprom_init(I2CBus *bus, uint8_t addr, uint32_t rsize)
> > > +static void at24c_eeprom_init(I2CBus *bus, uint8_t addr, uint32_t rsize,
> > > +                              int index)
> > >    {
> > >        I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", addr);
> > >        DeviceState *dev = DEVICE(i2c_dev);
> > >
> > > +    DriveInfo *dinfo = drive_get_by_index(IF_PFLASH, index);
> >
> > I don't think IF_PFLASH is the appropriate type.
>
> thanks
>
> > Jae proposed a similar patch with IF_NONE which should fit your need :
> >
> >     
> > 20220718175214.2087644-1-quic_jaehyoo@quicinc.com/">https://lore.kernel.org/all/20220718175214.2087644-1-quic_jaehyoo@quicinc.com/
> >
> > Could you please give it a try ?
>
> I tested on a fp5280g2-bmc, It's ok.  I would abandon my patch :）
>
> > It's available on my branch :
> >
> >     https://github.com/legoater/qemu/commits/aspeed-7.1
>
> I checked it, and will use this tree to module a new machine. :)

Or simply grab the patch if you only need one.

I don't think this is the correct approach for mainline. See this thread :

  
https://lore.kernel.org/all/CAFEAcA8sNjLsknea5Nt-tANEniFF2FYmjiV0xz=pr+vFwkX-gw@mail.gmail.com/t/

C.

> > Thanks,
> >
> > C.
> >
> >
> > > +    BlockBackend *blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL;
> > > +
> > > +    if (blk) {
> > > +        qdev_prop_set_drive(DEVICE(dev), "drive", blk);
> > > +    }
> > >        qdev_prop_set_uint32(dev, "rom-size", rsize);
> > > +
> > >        i2c_slave_realize_and_unref(i2c_dev, bus, &error_abort);
> > >    }
> > >
> > > @@ -685,7 +693,7 @@ static void fp5280g2_bmc_i2c_init(AspeedMachineState *bmc)
> > >        I2CSlave *i2c_mux;
> > >
> > >        /* The at24c256 */
> > > -    at24c_eeprom_init(aspeed_i2c_get_bus(&soc->i2c, 1), 0x50, 32768);
> > > +    at24c_eeprom_init(aspeed_i2c_get_bus(&soc->i2c, 1), 0x50, 32768, 1);
> > >
> > >        /* The fp5280g2 expects a TMP112 but a TMP105 is compatible */
> > >        i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 2), TYPE_TMP105,
> > > @@ -918,13 +926,13 @@ static void bletchley_bmc_i2c_init(AspeedMachineState 
> > > *bmc)
> > >        }
> > >
> > >        /* Bus 6 */
> > > -    at24c_eeprom_init(i2c[6], 0x56, 65536);
> > > +    at24c_eeprom_init(i2c[6], 0x56, 65536, 1);
> > >        /* Missing model: nxp,pcf85263 @ 0x51 , but ds1338 works enough */
> > >        i2c_slave_create_simple(i2c[6], "ds1338", 0x51);
> > >
> > >
> > >        /* Bus 7 */
> > > -    at24c_eeprom_init(i2c[7], 0x54, 65536);
> > > +    at24c_eeprom_init(i2c[7], 0x54, 65536, 2);
> > >
> > >        /* Bus 9 */
> > >        i2c_slave_create_simple(i2c[9], TYPE_TMP421, 0x4f);