On Thu, Sep 26, 2019 at 3:47 AM Andrew Murray <andrew.murray@xxxxxxx> wrote:
>
> On Tue, Sep 24, 2019 at 04:46:30PM -0500, Rob Herring wrote:
> > Now that the helpers provide the inbound resources in the host bridge
> > 'dma_ranges' resource list, convert Renesas R-Car PCIe host bridge to
> > use the resource list to setup the inbound addresses.
> >
> > Cc: Simon Horman <horms@xxxxxxxxxxxx>
> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx>
> > Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> > Signed-off-by: Rob Herring <robh@xxxxxxxxxx>
> > ---
> > drivers/pci/controller/pcie-rcar.c | 45 +++++++++++-------------------
> > 1 file changed, 16 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/pci/controller/pcie-rcar.c b/drivers/pci/controller/pcie-rcar.c
> > index b8d6e86a5539..453c931aaf77 100644
> > --- a/drivers/pci/controller/pcie-rcar.c
> > +++ b/drivers/pci/controller/pcie-rcar.c
> > @@ -1014,16 +1014,16 @@ static int rcar_pcie_get_resources(struct rcar_pcie *pcie)
> > }
> >
> > static int rcar_pcie_inbound_ranges(struct rcar_pcie *pcie,
> > - struct of_pci_range *range,
> > + struct resource_entry *entry,
> > int *index)
> > {
> > - u64 restype = range->flags;
> > - u64 cpu_addr = range->cpu_addr;
> > - u64 cpu_end = range->cpu_addr + range->size;
> > - u64 pci_addr = range->pci_addr;
> > + u64 restype = entry->res->flags;
> > + u64 cpu_addr = entry->res->start;
> > + u64 cpu_end = entry->res->end;
> > + u64 pci_addr = entry->res->start - entry->offset;
> > u32 flags = LAM_64BIT | LAR_ENABLE;
> > u64 mask;
> > - u64 size;
> > + u64 size = resource_size(entry->res);
> > int idx = *index;
> >
> > if (restype & IORESOURCE_PREFETCH)
> > @@ -1037,9 +1037,7 @@ static int rcar_pcie_inbound_ranges(struct rcar_pcie *pcie,
> > unsigned long nr_zeros = __ffs64(cpu_addr);
> > u64 alignment = 1ULL << nr_zeros;
> >
> > - size = min(range->size, alignment);
> > - } else {
> > - size = range->size;
> > + size = min(size, alignment);
> > }
>
> AFAICT the (if cpu_addr > 0) is here because the result of __ffs64 is undefined
> if no bits are set (according to the comment). However by removing the else
> statement we no longer guarantee that nr_zeros is defined.
You might want to read this again...
The 'if (cpu_addr > 0) {' is still there and nr_zeros is only under
that condition. We just init 'size' instead of setting it in the else
clause.
Rob
