Sorry I just realized I forgot to reply to the last item Gab > -----Original Message----- > From: Gabriele Paoloni > Sent: Monday, July 13, 2015 12:07 PM > To: 'Rob Herring' > Cc: Arnd Bergmann; Lorenzo Pieralisi; Wangzhou (B); Bjorn Helgaas; Rob > Herring; james.morse@xxxxxxx; Liviu Dudau; linux-pci@xxxxxxxxxxxxxxx; > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; > Yuanzhichang; Zhudacai; zhangjukuo; qiuzhenfa; Liguozhu (Kenneth) > Subject: RE: [PATCH] Store PCIe controllers address in struct > of_pci_range > > Hi Rob > > Many Thanks for your review > > > -----Original Message----- > > From: Rob Herring [mailto:robherring2@xxxxxxxxx] > > Sent: Friday, July 10, 2015 8:56 PM > > To: Gabriele Paoloni > > Cc: Arnd Bergmann; Lorenzo Pieralisi; Wangzhou (B); Bjorn Helgaas; > Rob > > Herring; james.morse@xxxxxxx; Liviu Dudau; linux-pci@xxxxxxxxxxxxxxx; > > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; > > Yuanzhichang; Zhudacai; zhangjukuo; qiuzhenfa; Liguozhu (Kenneth) > > Subject: Re: [PATCH] Store PCIe controllers address in struct > > of_pci_range > > > > On Fri, Jul 10, 2015 at 3:48 AM, Gabriele Paoloni > > <gabriele.paoloni@xxxxxxxxxx> wrote: > > > From: gabriele paoloni <gabriele.paoloni@xxxxxxxxxx> > > > > > > This patch is needed port PCIe designware to new DT parsing API > > > As discussed in > > > http://lists.infradead.org/pipermail/linux-arm-kernel/2015- > > January/317743.html > > > in designware we have a problem as the PCI addresses in the PCIe > > controller > > > address space are required in order to perform correct HW operation. > > > > > > In order to solve this problem commit > > > f4c55c5a3f7f68c06cc559ed7af8b2d017cbb0a7 "PCI: designware: > > > > Please abbreviate hashs to 12 characters. > > Sure, will do. > > > > > > Program ATU with untranslated address" added code to read the PCIe > > > controller start address directly from the DT ranges. > > > > > > In the new DT parsing API of_pci_get_host_bridge_resources() hides > > the > > > DT parser from the host controller drivers, so it is not possible > > > for drivers to parse values directly from the DT. > > > > > > In http://www.spinics.net/lists/linux-pci/msg42540.html we already > > tried > > > to use the new DT parsing API but there is a bug (obviously) in > > setting > > > the <*>_mod_base addresses > > > Applying this patch we can easily set "<*>_mod_base = win- > > >__res.start" > > > > > > This patch adds a new field in "struct of_pci_range" to store the > > > pci controller start address; it fills the field in > > of_pci_range_parser_one(); > > > in of_pci_get_host_bridge_resources() it retrieve the resource > entry > > > after it is created and added to the resource list and uses > > > entry->__res.start to store the pci controller address > > > > > > the patch is based on 4.2-rc1 > > > > > > Signed-off-by: Gabriele Paoloni <gabriele.paoloni@xxxxxxxxxx> > > > --- > > > drivers/of/address.c | 1 + > > > drivers/of/of_pci.c | 4 ++++ > > > drivers/pci/host/pcie-designware.c | 9 +++------ > > > include/linux/of_address.h | 1 + > > > 4 files changed, 9 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/of/address.c b/drivers/of/address.c > > > index 8bfda6a..52f9321 100644 > > > --- a/drivers/of/address.c > > > +++ b/drivers/of/address.c > > > @@ -265,6 +265,7 @@ struct of_pci_range > > *of_pci_range_parser_one(struct of_pci_range_parser *parser, > > > range->pci_addr = of_read_number(parser->range + 1, ns); > > > range->cpu_addr = of_translate_address(parser->node, > > > parser->range + na); > > > + range->pci_ctrl_addr = of_read_number(parser->range + na, > ns) ; > > > > This is wrong as the correct size to read is not "ns", but the parent > > bus #size-cells value. > > Ok I will replace "ns" with "of_n_size_cells(parser->node)" > > > > > I think "bus_addr" would be a better name. It is not the PCI > > controller's address (i.e. what is in reg prop). No "pci" because it > > has nothing to do with PCI bus addresses. > > Ok I will change the name > > > > > In general, this seems fragile as the dt address ranges/translation > > may not align with the h/w ranges. For example, what if you have 2 > > levels of translations and you happen to need it translated with just > > 1 level of translation. That said, I don't really have a better > > suggestion and I guess we can deal with that case if needed later. > > Ok so, we'll leave it like this for now > > > > > > range->size = of_read_number(parser->range + parser->pna + > na, > > ns); > > > > > > parser->range += parser->np; > > > diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c > > > index 5751dc5..2ccc749 100644 > > > --- a/drivers/of/of_pci.c > > > +++ b/drivers/of/of_pci.c > > > @@ -198,6 +198,7 @@ int of_pci_get_host_bridge_resources(struct > > device_node *dev, > > > > > > pr_debug("Parsing ranges property...\n"); > > > for_each_of_pci_range(&parser, &range) { > > > + struct resource_entry *entry; > > > /* Read next ranges element */ > > > if ((range.flags & IORESOURCE_TYPE_BITS) == > > IORESOURCE_IO) > > > snprintf(range_type, 4, " IO"); > > > @@ -240,6 +241,9 @@ int of_pci_get_host_bridge_resources(struct > > device_node *dev, > > > } > > > > > > pci_add_resource_offset(resources, res, res->start > - > > range.pci_addr); > > > + entry = list_last_entry(resources, struct > > resource_entry, node); > > > + /*we are using __res for storing the PCI controller > > address*/ > > > + entry->__res.start = range.pci_ctrl_addr; > > > > You will use this in a follow-up patch? I'd like to see this just > > split into core changes and DW changes. This looks like you are > making > > intermediate DW changes which will be removed in subsequent patches. Ok I will split it The changes in "drivers/pci/host/pcie-designware.c" can be removed from this patch; we can modify directly "[PATCH v2 2/4] PCI: designware: Add ARM64 support" in v3 patchset > > > > Rob ��.n��������+%������w��{.n����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
