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

[Andy Shevchenko]

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 at 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 at chromium.org>
> > > Reviewed-by: Bin Meng <bmeng.cn at gmail.com>
> > > Reviewed-by: Wolfgang Wallner <wolfgang.wallner at 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