[U-Boot] [PATCH v3 06/10] MIPS: qemu-malta: add PCI support

[Gabor Juhos]

Hi Daniel,

Thank you for the review!

> 2013/5/22 Gabor Juhos <juhosg at openwrt.org>:
>> Qemu emulates the Galileo GT64120 System Controller
>> which provides a CPU bus to PCI bus bridge.
>>
>> The patch adds driver for this bridge and enables
>> PCI support for the emulated Malta board.
>>
>> Signed-off-by: Gabor Juhos <juhosg at openwrt.org>
>> Cc: Daniel Schwierzeck <daniel.schwierzeck at googlemail.com>
> 
> 
> ELDK-5.3 shows some warnings:
> 
> pci_indirect.c: In function 'indirect_read_config_byte':
> pci_indirect.c:101:1: warning: implicit declaration of function
> 'out_le32' [-Wimplicit-function-declaration]
> pci_indirect.c:101:1: warning: implicit declaration of function 'in_8'
> [-Wimplicit-function-declaration]
> pci_indirect.c: In function 'indirect_read_config_word':
> pci_indirect.c:102:1: warning: implicit declaration of function
> 'in_le16' [-Wimplicit-function-declaration]
> pci_indirect.c: In function 'indirect_read_config_dword':
> pci_indirect.c:103:1: warning: implicit declaration of function
> 'in_le32' [-Wimplicit-function-declaration]
> pci_indirect.c: In function 'indirect_write_config_byte':
> pci_indirect.c:109:1: warning: implicit declaration of function
> 'out_8' [-Wimplicit-function-declaration]
> pci_indirect.c: In function 'indirect_write_config_word':
> pci_indirect.c:110:1: warning: implicit declaration of function
> 'out_le16' [-Wimplicit-function-declaration]

Hm, you are right. My toolchain also shows the warning but I did not notice
those because I forgot to redirect stderr when I have generated the build logs.

The pci_indirect.c file is always compiled when CONFIG_PCI is defined although
it is not needed at all for Malta PCI support.

The issue can be resolved on a few different ways:

1. Extend the '#if !defined(__I386__)' directive in pci_indirect.c with a new
'&& !defined(__MIPS__)' condition. This would be the simplest solution but the
drawback of this is that indirect support will not be usable on any MIPS board.

2. Introduce a new 'CONFIG_PCI_INDIRECT_BRIDGE' option and only compile the
pci_indirect.c file if this option is present. Probably this is the best
solution however the new symbol should be added into the configuration of the
affected boards.

3. Introduce a new 'CONFIG_PCI_NO_INDIRECT_BRIDGE' option and use an '#ifndef
CONFIG_PCI_NO_INDIRECT_BRIDGE' directive in pci_indirect.c.

I'm unsure about which approach is preferred.

> pci_gt64120.c: In function 'gt64120_pci_init':
> pci_gt64120.c:178:1: warning: control reaches end of non-void function
> [-Wreturn-type]

<snip>

>> +int gt64120_pci_init(void *regs, unsigned long sys_bus, unsigned long sys_phys,
>> +                    unsigned long sys_size, unsigned long mem_bus,
>> +                    unsigned long mem_phys, unsigned long mem_size,
>> +                    unsigned long io_bus, unsigned long io_phys,
>> +                    unsigned long io_size)
>> +{
>> +       static struct gt64120_pci_controller global_gt;
>> +       struct gt64120_pci_controller *gt;
>> +       struct pci_controller *hose;
>> +
>> +       gt = &global_gt;
>> +       gt->regs = regs;
>> +
>> +       hose = &gt->hose;
>> +
>> +       hose->first_busno = 0;
>> +       hose->last_busno = 0;
>> +
>> +       /* System memory space */
>> +       pci_set_region(&hose->regions[0], sys_bus, sys_phys, sys_size,
>> +                      PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
>> +
>> +       /* PCI memory space */
>> +       pci_set_region(&hose->regions[1], mem_bus, mem_phys, mem_size,
>> +                      PCI_REGION_MEM);
>> +
>> +       /* PCI I/O space */
>> +       pci_set_region(&hose->regions[2], io_bus, io_phys, io_size,
>> +                      PCI_REGION_IO);
>> +
>> +       hose->region_count = 3;
>> +
>> +       pci_set_ops(hose,
>> +                   pci_hose_read_config_byte_via_dword,
>> +                   pci_hose_read_config_word_via_dword,
>> +                   gt_read_config_dword,
>> +                   pci_hose_write_config_byte_via_dword,
>> +                   pci_hose_write_config_word_via_dword,
>> +                   gt_write_config_dword);
>> +
>> +       pci_register_hose(hose);
>> +       hose->last_busno = pci_hose_scan(hose);
> 
> missing return keyword

Correct. However the function never fails, so I will convert it to void instead
of adding a 'return 0;' line.

> 
>> +}

-Gabor