[PATCH v3 10/29] pci: Adjust dm_pci_read_bar32() to return errors correctly

Simon Glass
2020-04-08T02:57:20Z

Hi Andy,

On Fri, 3 Apr 2020 at 05:22, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Mon, Mar 30, 2020 at 05:12:46PM -0600, Simon Glass wrote:
> > At present if reading a BAR returns 0xffffffff (e.g. the device is not
> > present) then the value is masked and a different value is returned.
> > This makes it harder to detect the problem when debugging.
>
> The above ('the device is not present') is actually not correct.
> BAR is not mandatory register and detection is described in PCI spec.

What change are you suggesting here? I suggest 'not present' as an
example of why this might happen.

>
> To get device presence one may have check Vendor ID / Device ID pair rather
> then BAR.
>
> > Update the function to avoid masking in this case.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > Reviewed-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
> > ---
> >
> > Changes in v3: None
> > Changes in v2: None
> >
> > drivers/pci/pci-uclass.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> > index ceb64517047..d2e10d6868a 100644
> > --- a/drivers/pci/pci-uclass.c
> > +++ b/drivers/pci/pci-uclass.c
> > @@ -1213,7 +1213,14 @@ u32 dm_pci_read_bar32(const struct udevice *dev, int barnum)
> >
> > bar = PCI_BASE_ADDRESS_0 + barnum * 4;
> > dm_pci_read_config32(dev, bar, &addr);
> > - if (addr & PCI_BASE_ADDRESS_SPACE_IO)
> > +
> > + /*
> > + * If we get an invalid address, return this so that comparisons with
> > + * FDT_ADDR_T_NONE work correctly
> > + */
> > + if (addr == 0xffffffff)
> > + return addr;
> > + else if (addr & PCI_BASE_ADDRESS_SPACE_IO)
> > return addr & PCI_BASE_ADDRESS_IO_MASK;
> > else
> > return addr & PCI_BASE_ADDRESS_MEM_MASK;
> > --
> > 2.26.0.rc2.310.g2932bb562d-goog
> >
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Regards,
Simon