On 04.10.2012, at 17:54, Andreas Färber wrote:
> Am 03.10.2012 13:50, schrieb Bharat Bhushan:
>> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
>> index 197411d..c7ae2b6 100644
>> --- a/hw/ppc/e500.c
>> +++ b/hw/ppc/e500.c
>> @@ -518,6 +518,7 @@ void ppce500_init(PPCE500Params *params)
>>
>> /* PCI */
>> dev = qdev_create(NULL, "e500-pcihost");
>> + qdev_prop_set_ptr(dev, "bar0_region", ccsr);
>> qdev_init_nofail(dev);
>> s = sysbus_from_qdev(dev);
>> sysbus_connect_irq(s, 0, mpic[pci_irq_nrs[0]]);
>
> Please...
>
>> diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c
>> index 92b1dc0..16e4af2 100644
>> --- a/hw/ppce500_pci.c
>> +++ b/hw/ppce500_pci.c
>> @@ -87,6 +87,7 @@ struct PPCE500PCIState {
>> /* mmio maps */
>> MemoryRegion container;
>> MemoryRegion iomem;
>> + void *bar0;
>> };
>>
>> typedef struct PPCE500PCIState PPCE500PCIState;
>
> ...do not do this. qdev_prop_set_ptr() is considered deprecated and we
> had long discussions how to solve this differently.
>
> Was there anything wrong with using a SysBusDevice for the CCSR to
> encapsulate the MemoryRegion?
Not at all. This was meant as a first shot, so we can slowly move towards the
CCSR-as-device model.
In fact, now that we have this code as far as it is, we can go over to tackle
it that way.
Bharat, mind to model a new CCSR device now that contains all the CCSR devices?
That one creates a memory region then. The PCI host controller gets a reference
to the CCSR device and from there can call a CCSR specific function to receive
the memory region pointer. And suddenly we have all of this solved :).
Alex
