Re: [Qemu-devel] [PATCH v4 0/2] ati-vga: Implement DDC and EDID info from monitor

[Corey Minyard]

On Wed, Mar 20, 2019 at 12:39:44PM +0100, BALATON Zoltan wrote:
> On Tue, 19 Mar 2019, Corey Minyard wrote:
> > On Sat, Mar 16, 2019 at 03:23:09PM +0100, BALATON Zoltan wrote:
> > > On Sat, 16 Mar 2019, BALATON Zoltan wrote:
> > > > Version 4 try to fix an ASan warning about leaking bitbang_i2c.
> > > 
> > > I still get ASan warning but it's about gpio_i2c_init in bitbang_i2c.c 
> > > which
> > > I haven't changed nor using so I think this is pre-existing problem in
> > > bitbang_i2c.c which was exposed by touching it now so I'm not trying to 
> > > fix.
> > > (Maybe adding an exit function to free the allocated bitbang_i2c in object
> > > state could help but I think it still leaks the bus so I'm not sure how to
> > > handle it correctly. Maybe this should be QOMified somehow but cannot be a
> > > device because it's not connected to a bus but embedded in other device
> > > models.)
> > 
> > It can inherit from I2CBus and then specific devices could inherit from
> > the bitbang interface.  That's almost certainly the right way to do this,
> 
> It's OK for a QOMified bitbang_i2c to inherit from I2CBus but I don't think
> other devices can inherit from it. I'm using it from devices that are PCI
> (ati-vga) or SysBus devices (ppc4xx_i2c but also the gpio_i2c) but happen to
> have an i2c component so they can't inherit from bitbang_i2c only refer to
> an instance of it.

Yes, you are right, I realized that after I sent this.

> 
> > but that design issue isn't cause by your changes.
> > 
> > I'll try to poke at this a bit to see if I can clean it up.
> 
> I wonder if we really need a QOM object for this or if it would be enough to
> provide a bitbang_i2c_free function that dereferences the I2CBus which we
> can call to finalise a bitbang_i2c created with bitbang_i2c_init?

That would probably work.

If this is going to be used on x86, then a QOM object is required to
allow migration.  Outside of that, I think it's more correct form in
qemu, but not required.

-corey

> 
> Regards,
> BALATON Zoltan