Re: [Qemu-devel] modelling omap_gpmc with the hierarchical memory API

[Anthony Liguori <anthony@codemonkey.ws>]

On 08/02/2011 04:29 PM, Avi Kivity wrote:
> On 08/03/2011 12:06 AM, Anthony Liguori wrote:
>>
>> The qdev level should be the common base that makes sense for *all*
>> qdev devices. IRQ management does not belong in DeviceState because
>> what you do for a simple LCD is not what you do for an MSI-X capable
>> PCI device.
>>
>> This is what QOM properties tries to address. It should be possible to
>> create a simple device, and register plugs/sockets for GPIO pins
>> without pushing GPIO knowledge into the base class.
>>
>> In a QDev world, the right approach is to have a GpioDevice base class
>> that implements this sort of logic for devices where it makes sense.
>> That's what SysBusDevice sort of wants to be but it somehow ended up
>> as yet another base class for everything.
>>
>
> Doesn't this end up requiring multiple inheritance, and a ton of
> boilerplate in addition?

I don't see a ton of boiler plate nor do I see multiple inheritance. 
Either you have devices that live on a bus that interact with other 
devices via the bus interface or you have devices that interact through 
Pin interactions.

But pushing everything into qdev is wrong.  This is a classic trap with 
object oriented modelling.  Inheritance is supposed to model is-a 
relationships, not "may-implement in some subclasses".  Not only does it 
cause unnecessary bloat, it causes brittleness.

What in the world would DeviceState::num_gpio_out mean for a PCI device? 
  It doesn't make sense at all which means it doesn't belong in DeviceState.

Unless we're modelling the pin inputs and outputs for every device that 
we possibly support..  But we're in for a world of hurt if that's what 
our goals are.  Connecting PCI devices to their busses will be, 
interesting, to say the least :-)

Regards,

Anthony Liguori


>
> I'm in favour of throwing everything into qdev so it's easy to use. If a
> device doesn't have gpio, why, 0 is a valid integer.
>
>