On Fri, Apr 13, 2018 at 4:30 PM, Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> wrote: > pci_size() had a check that made it return 0 (i.e. an error) in some > cases. It took me a while to work out in which situations exactly this > check triggered: If writing 0xfffffffe to the BAR register doesn't > change it's value and the masked value isn't in the form 0b1....10...0. > > This patch replaces the check in pci_size() by a similar check in > setup_device() where it is more expected to happen. This allows to > simplify pci_size() considerably. > > The new check is a bit more strict as it doesn't consider the value the > BAR register holds initially and bails out iff the maximal base value > isn't in the expected format. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> > --- > drivers/pci/pci.c | 30 ++++++++++++++---------------- > 1 file changed, 14 insertions(+), 16 deletions(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 837e17aa8397..72689560d598 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -144,18 +144,10 @@ static struct pci_dev *alloc_pci_dev(void) > return dev; > } > > -static u32 pci_size(u32 base, u32 maxbase, u32 mask) > +static u32 pci_size(u32 maxbase) > { > - u32 size = maxbase & mask; > - if (!size) > - return 0; > - > - size = (size & ~(size-1)) - 1; > - > - if (base == maxbase && ((base | size) & mask) != mask) > - return 0; > - > - return size + 1; > + /* isolate right-most set bit */ > + return maxbase & ~(maxbase - 1); > } > > > @@ -169,7 +161,7 @@ static void setup_device(struct pci_dev *dev, int max_bar) > cmd & ~(PCI_COMMAND_IO | PCI_COMMAND_MEMORY)); > > for (bar = 0; bar < max_bar; bar++) { > - u32 orig, barval, size, mask; > + u32 orig, barval, maxbase, size, mask; > const char *type; > int restype; > unsigned long resflags; > @@ -206,13 +198,19 @@ static void setup_device(struct pci_dev *dev, int max_bar) > last = &last_mem; > } > > - size = pci_size(orig, barval, mask); > - if (!size) { > - pr_debug(" pbar%d: bad %s size (orig=%08x, barval=%08x, type=%s)\n", > - bar, type, orig, barval, type); > + maxbase = barval & mask; > + > + /* > + * if maxbase isn't in the form 0b1...10...0 there is something > + * fishy > + */ > + if ((maxbase | (maxbase - 1)) != (u32)-1 || maxbase == 0) { U32_MAX instead of (u32)-1 ? Thanks, Andrey Smirnov _______________________________________________ barebox mailing list barebox@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/barebox