[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/4] hw/pci-host/xilinx: QOM'ify the AXI-PCIe ho
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [Qemu-devel] [PATCH 3/4] hw/pci-host/xilinx: QOM'ify the AXI-PCIe host bridge |
Date: |
Mon, 18 Dec 2017 12:11:59 -0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 |
On 12/18/2017 04:15 AM, Marcel Apfelbaum wrote:
> On 17/12/2017 22:49, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
>> ---
>> hw/pci-host/xilinx-pcie.c | 19 +++++++++----------
>> 1 file changed, 9 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/pci-host/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c
>> index 7659253090..756db39fd5 100644
>> --- a/hw/pci-host/xilinx-pcie.c
>> +++ b/hw/pci-host/xilinx-pcie.c
>> @@ -267,24 +267,23 @@ static void
>> xilinx_pcie_root_config_write(PCIDevice *d, uint32_t address,
>> }
>> }
>> -static int xilinx_pcie_root_init(PCIDevice *dev)
>> +static void xilinx_pcie_root_realize(PCIDevice *pci, Error **errp)
>> {
>
> Same comment here, if "dev" seems not good enough maybe
> you can use the pci_dev convention.
>
>> - BusState *bus = qdev_get_parent_bus(DEVICE(dev));
>> + DeviceState *dev = DEVICE(pci);
>> + BusState *bus = qdev_get_parent_bus(dev);
>
> Not sure the above worth the effort :)
>
>> XilinxPCIEHost *s = XILINX_PCIE_HOST(bus->parent);
>> - pci_set_word(dev->config + PCI_COMMAND,
>> + pci_set_word(pci->config + PCI_COMMAND,
>> PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
>> - pci_set_word(dev->config + PCI_MEMORY_BASE, s->mmio_base >> 16);
>> - pci_set_word(dev->config + PCI_MEMORY_LIMIT,
>> + pci_set_word(pci->config + PCI_MEMORY_BASE, s->mmio_base >> 16);
>> + pci_set_word(pci->config + PCI_MEMORY_LIMIT,
>> ((s->mmio_base + s->mmio_size - 1) >> 16) & 0xfff0);
>> - pci_bridge_initfn(dev, TYPE_PCI_BUS);
>> + pci_bridge_initfn(pci, TYPE_PCI_BUS);
>> - if (pcie_endpoint_cap_v1_init(dev, 0x80) < 0) {
>> + if (pcie_endpoint_cap_v1_init(pci, 0x80) < 0) {
>> hw_error("Failed to initialize PCIe capability");
>
> I think here you need to use the error_setg (after/instead hw_error).
> It is supposed to return -1 if pcie capability was not added.
Oops I missed that, I'll fix in v2.
> Thanks,
> Marcel
>
>> }
>> -
>> - return 0;
>> }
>> static void xilinx_pcie_root_class_init(ObjectClass *klass, void
>> *data)
>> @@ -300,7 +299,7 @@ static void
>> xilinx_pcie_root_class_init(ObjectClass *klass, void *data)
>> k->class_id = PCI_CLASS_BRIDGE_HOST;
>> k->is_express = true;
>> k->is_bridge = true;
>> - k->init = xilinx_pcie_root_init;
>> + k->realize = xilinx_pcie_root_realize;
>> k->exit = pci_bridge_exitfn;
>> dc->reset = pci_bridge_reset;
>> k->config_read = xilinx_pcie_root_config_read;
>>