[PATCH 4/4] pci: don't hide hard to understand sanity check in size calculation

[Andrey Smirnov]

On Fri, Apr 13, 2018 at 4:30 PM, Uwe Kleine-König
<u.kleine-koenig at pengutronix.de> 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 at pengutronix.de>
> ---
>  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