Re: [Qemu-devel] [RFC][MIPS][PATCH 1/6] Initial support of bonito north bridge used by fulong mini pc

[chen huacai]

>>  +    s->pci = qemu_mallocz(sizeof(*s->pci));
>>  +    assert(s->pci != NULL);
>>  +    bonito_state = s;
>>  +
>>  +    /* get the north bridge pci bus */
>>  +    s->pci->bus = pci_register_bus(NULL, "pci", pci_bonito_set_irq,
>>  +                                   pci_bonito_map_irq, pic, 0x28, 32);
>>  +
>>  +    /* set the north bridge register mapping */
>>  +    s->bonito_reg_handle = cpu_register_io_memory(bonito_read,
>>  bonito_write, s);
>>  +    s->bonito_reg_start = BONITO_INTERNAL_REG_BASE;
>
> Usually the devices don't specify their addresses, but these are
> passed from the board level.

I'm a bit confusing here, bonito internal registers are mapped to a
fixed physical address according to specification.


>>  +    /*add PCI io space */
>>  +    /*PCI IO Space  0x1fd0 0000 - 0x1fd1 0000 */
>>  +    if (s->bonito_pciio_length)
>>  +    {
>>  +        cpu_register_physical_memory(s->bonito_pciio_start,
>>  +                                     s->bonito_pciio_length,
>>  IO_MEM_UNASSIGNED);
>
> Why would this be needed?

This is borrowed from gt64xxx.c


>>  +    d = pci_register_device(s->pci->bus, "Bonito PCI Bus", 
>> sizeof(PCIDevice),
>>  +                            0, bonito_read_config, bonito_write_config);
>>  +
>>  +    pci_config_set_vendor_id(d->config, 0xdf53); //Bonito North Bridge
>>  +    pci_config_set_device_id(d->config, 0x00d5);
>
> Please put the above constants to hw/pci_ids.h.

Bonito north bridge is built on FPGA now, VENDOR_ID/DEVICE_ID are
temporary value so I didn't put them in pci_ids.h

For your other comments I'll improve my code, thanks.

Best regards,

Huacai Chen