On Thu, Sep 26, 2019 at 07:53:20AM -0500, Rob Herring wrote:
> 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.
Ah yes, apologies for the noise, thanks for your patience.
Andrew Murray
>
> Rob
