Re: [Qemu-devel] [PATCH 2/8] sam460ex: Clean up SPD EEPROM creation

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2377 bytes --]

On Wed, Jan 02, 2019 at 01:49:44PM +0100, BALATON Zoltan wrote:
> On Wed, 2 Jan 2019, David Gibson wrote:
> > On Wed, Jan 02, 2019 at 03:06:38AM +0100, BALATON Zoltan wrote:
> > > +    /* IIC controllers and devices */
> > >      dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600700, uic[0][2]);
> > > -    i2c[0] = PPC4xx_I2C(dev);
> > > -    object_property_set_bool(OBJECT(dev), true, "realized", NULL);
> > > -    smbus_eeprom_init(i2c[0]->bus, 8, smbus_eeprom_buf, smbus_eeprom_size);
> > > -    g_free(smbus_eeprom_buf);
> > > -    i2c_create_slave(i2c[0]->bus, "m41t80", 0x68);
> > > +    i2c = PPC4xx_I2C(dev)->bus;
> > > +    /* SPD EEPROM on RAM module */
> > > +    spd_data = spd_data_generate(DDR2, ram_sizes[0]);
> > > +    if (spd_data) {
> > > +        spd_data[20] = 4; /* SO-DIMM module */
> > > +        smbus_eeprom_init_one(i2c, 0x50, spd_data);
> > 
> > Any specific reason this case is handled outside spd_data_generate()?
> 
> As explained in previous reply I tried to keep number of options to the
> function to minimum and not handle every board specific case in that
> function. This board has SO-DIMM instead of regular DIMM socket but most of
> the SPD data is the same so it's easy enough to patch it here to match what
> real hardware may have. Otherwise spd_data_generate should have a lot of
> options for all different possibilites different boards might have.

Ok, fair enough.

> 
> (Also this is an example that unexpected configs might still work:
> 
> $ qemu-system-ppc -M sam460ex -m 64 -serial stdio
> qemu-system-ppc: warning: Memory size is too small for SDRAM type, adjusting type
> [...]
> DRAM:  64 MiB (ECC not enabled, 460 MHz, CL0)
> 
> This SoC has a DRAM controller which accepts both DDR and DDR2 and the
> firmware does not care either so it works even if this cannot happen on real
> hardware. The spd_data[20] is different for DDR2 and DDR/SDR but in the
> latter it's ~WE latency which does not matter either so it can be set
> unconditionally. If this was not working the board code could check
> spd_data[2] != DDR2 and then abort itself.)
> 
> Regards,
> BALATON Zoltan
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]