Re: [PATCH] arm/aspeed: rainier: Add i2c eeproms and muxes

[Joel Stanley]

On Mon, 28 Jun 2021 at 04:49, Joel Stanley <joel@jms.id.au> wrote:
>
> On Mon, 28 Jun 2021 at 04:33, Andrew Jeffery <andrew@aj.id.au> wrote:
> >
> >
> >
> > On Fri, 25 Jun 2021, at 14:36, Joel Stanley wrote:
> > > These are the devices documented by the Rainier device tree. With this
> > > we can see the guest discovering the multiplexers and probing the eeprom
> > > devices:
> > >
> > >  i2c i2c-2: Added multiplexed i2c bus 16
> > >  i2c i2c-2: Added multiplexed i2c bus 17
> > >  i2c i2c-2: Added multiplexed i2c bus 18
> > >  i2c i2c-2: Added multiplexed i2c bus 19
> > >  i2c-mux-gpio i2cmux: 4 port mux on 1e78a180.i2c-bus adapter
> > >  at24 20-0050: 8192 byte 24c64 EEPROM, writable, 1 bytes/write
> > >  i2c i2c-4: Added multiplexed i2c bus 20
> > >  at24 21-0051: 8192 byte 24c64 EEPROM, writable, 1 bytes/write
> > >  i2c i2c-4: Added multiplexed i2c bus 21
> > >  at24 22-0052: 8192 byte 24c64 EEPROM, writable, 1 bytes/write
> > >
> > > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > > ---
> > >  hw/arm/aspeed.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 56 insertions(+)
> > >
> > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> > > index 1301e8fdffb2..7ed22294c6eb 100644
> > > --- a/hw/arm/aspeed.c
> > > +++ b/hw/arm/aspeed.c
> > > @@ -677,6 +677,10 @@ static void g220a_bmc_i2c_init(AspeedMachineState 
> > > *bmc)
> > >  static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
> > >  {
> > >      AspeedSoCState *soc = &bmc->soc;
> > > +    I2CSlave *i2c_mux;
> > > +
> > > +    smbus_eeprom_init_one(aspeed_i2c_get_bus(&soc->i2c, 0), 0x51,
> > > +                          g_malloc0(32 * 1024));
> > >
> > >      /* The rainier expects a TMP275 but a TMP105 is compatible */
> > >      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 4), 
> > > TYPE_TMP105,
> > > @@ -685,11 +689,25 @@ static void rainier_bmc_i2c_init(AspeedMachineState 
> > > *bmc)
> > >                       0x49);
> > >      i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 4), 
> > > TYPE_TMP105,
> > >                       0x4a);
> > > +    i2c_mux = i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 4),
> > > +                                      "pca9546", 0x70);
> > > +    smbus_eeprom_init_one(pca954x_i2c_get_bus(i2c_mux, 0), 0x50,
> > > +                          g_malloc0(64 * 1024));
> >
> > The EEPROMs described in the Rainier devicetree are Atmel AT2x devices and
> > not SMBus EEPROMs. The SMBus EEPROM model uses SMBus block reads and
> > writes which are not what the AT2x driver issues. If you try to read or
> > write data under Linux from the EEPROMs in this patch you just get
> > corrupt results. So as far as I can see the patch needs to be reworked.
>
> That's fine, these are just there so the drivers can probe. As you can
> see the devices have no data in them either, so it's nowhere near an
> incomplete model.

nowhere near a complete model.

>
> If you want to extend the modelling to have the correct VPD data, and
> allow writes where applicable, that would be welcome.
>
> Cheers,
>
> Joel