On 20/11/17 17:51, Philippe Mathieu-Daudé wrote:
> Hi Mark,
>
> On 11/17/2017 10:42 AM, Mark Cave-Ayland wrote:
>> By making the special_base and mem_base values qdev properties, we can move
>> the remaining parts of pci_apb_init() into the pbm init() and realize()
>> functions.
>>
>> This finally allows us to instantiate the APB directly using standard qdev
>> create/init functions in sun4u.c.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>
>> ---
>> hw/pci-host/apb.c | 123
>> ++++++++++++++++++++++-----------------------
>> hw/sparc64/sun4u.c | 6 ++-
>> include/hw/pci-host/apb.h | 4 +-
>> 3 files changed, 68 insertions(+), 65 deletions(-)
>>
>> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
>> index 823661a..6c20285 100644
>> --- a/hw/pci-host/apb.c
>> +++ b/hw/pci-host/apb.c
>> @@ -611,41 +611,56 @@ static void apb_pci_bridge_realize(PCIDevice *dev,
>> Error **errp)
>> pci_bridge_update_mappings(PCI_BRIDGE(br));
>> }
>>
>> -APBState *pci_apb_init(hwaddr special_base,
>> - hwaddr mem_base)
>> +static void pci_pbm_reset(DeviceState *d)
>> {
>> - DeviceState *dev;
>> - SysBusDevice *s;
>> - PCIHostState *phb;
>> - APBState *d;
>> - IOMMUState *is;
>> + unsigned int i;
>> + APBState *s = APB_DEVICE(d);
>> +
>> + for (i = 0; i < 8; i++) {
>> + s->pci_irq_map[i] &= PBM_PCI_IMR_MASK;
>> + }
>> + for (i = 0; i < 32; i++) {
>> + s->obio_irq_map[i] &= PBM_PCI_IMR_MASK;
>> + }
>> +
>> + s->irq_request = NO_IRQ_REQUEST;
>> + s->pci_irq_in = 0ULL;
>> +
>> + if (s->nr_resets++ == 0) {
>> + /* Power on reset */
>> + s->reset_control = POR;
>> + }
>> +}
>> +
>> +static const MemoryRegionOps pci_config_ops = {
>> + .read = apb_pci_config_read,
>> + .write = apb_pci_config_write,
>> + .endianness = DEVICE_LITTLE_ENDIAN,
>> +};
>> +
>> +static void pci_pbm_realize(DeviceState *dev, Error **errp)
>> +{
>> + APBState *s = APB_DEVICE(dev);
>> + PCIHostState *phb = PCI_HOST_BRIDGE(dev);
>> + SysBusDevice *sbd = SYS_BUS_DEVICE(s);
>> PCIDevice *pci_dev;
>> + IOMMUState *is;
>>
>> - /* Ultrasparc PBM main bus */
>> - dev = qdev_create(NULL, TYPE_APB);
>> - d = APB_DEVICE(dev);
>> - phb = PCI_HOST_BRIDGE(dev);
>> - phb->bus = pci_register_bus(DEVICE(phb), "pci",
>> - pci_apb_set_irq, pci_apb_map_irq, d,
>> - &d->pci_mmio,
>> - &d->pci_ioport,
>> - 0, 32, TYPE_PCI_BUS);
>> - qdev_init_nofail(dev);
>> - s = SYS_BUS_DEVICE(dev);
>> /* apb_config */
>> - sysbus_mmio_map(s, 0, special_base);
>> + sysbus_mmio_map(sbd, 0, s->special_base);
>
> You might add a safety check than special_base is correctly initialize
> (compare to -1?)
I guess strictly speaking this would be good to have, however since the
same value is also hard-coded in OpenBIOS then you'll get a crash if you
change it to any other value anyhow. So for that reason I think it's
okay for the moment as it is.
ATB,
Mark.
