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

Andy Shevchenko
2020-04-08T16:58:23Z

On Tue, Apr 07, 2020 at 08:57:20PM -0600, Simon Glass wrote:
> 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.

I suggest to follow PCI spec.
Thus, the code below is fragile and working by luck.

> > 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

--
With Best Regards,
Andy Shevchenko