[U-Boot] [PATCH] cmd_pci: Check for VendorID earlier

[Simon Glass]

Hi Fabio,

On 9 October 2015 at 14:22, Fabio Estevam <festevam at gmail.com> wrote:
> Hi Simon,
>
> On Fri, Oct 9, 2015 at 10:01 AM, Simon Glass <sjg at chromium.org> wrote:
>
>> I'm just surprised that it is failing when there is nothing there. I
>> think it should succeed (and read 0xffff).
>>
>> What board is this? Can you find the code that is returning this
>> error? It may be a call to pci_set_ops() which sets read_word().
>
> It is a mx6qsabresd board.
>
> I don't have the PCI device at the moment to give it a try, but
> looking at the code, in pcie_imx we have:
>
>     pci_set_ops(hose,
>             pci_hose_read_config_byte_via_dword,
>             pci_hose_read_config_word_via_dword,
>             imx_pcie_read_config,
>             pci_hose_write_config_byte_via_dword,
>             pci_hose_write_config_word_via_dword,
>             imx_pcie_write_config);
>
> Then in drivers/pci/pci.c:
>
> #define PCI_READ_VIA_DWORD_OP(size, type, off_mask)            \
> int pci_hose_read_config_##size##_via_dword(struct pci_controller *hose,\
>                     pci_dev_t dev,            \
>                     int offset, type val)        \
> {                                    \
>     u32 val32;                            \
>                                     \
>     if (pci_hose_read_config_dword(hose, dev, offset & 0xfc, &val32) < 0) {    \
>         *val = -1;                        \
>         return -1;                        \
>     }                                \
>                                     \
>     *val = (val32 >> ((offset & (int)off_mask) * 8));        \
>                                     \
>     return 0;                            \
> }
>
> ,which returns -1 (and also *val = -1) in the case of errror, which
> matches the values that I was reading yesterday

If you look down one more level, these end up calling
imx_pcie_read_config() which calls imx_pcie_addr_valid():

static int imx_pcie_addr_valid(pci_dev_t d)
{
   if ((PCI_BUS(d) == 0) && (PCI_DEV(d) > 1))
      return -EINVAL;
   if ((PCI_BUS(d) == 1) && (PCI_DEV(d) > 0))
      return -EINVAL;
   return 0;
}

I can understand the bus check, but why return an access error if the
device does not exist on the bus? That seems like a bug to me.

Regards,
Simon