Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752463AbaJ0MDs (ORCPT ); Mon, 27 Oct 2014 08:03:48 -0400 Received: from foss-mx-na.foss.arm.com ([217.140.108.86]:48944 "EHLO foss-mx-na.foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751500AbaJ0MDq (ORCPT ); Mon, 27 Oct 2014 08:03:46 -0400 Date: Mon, 27 Oct 2014 12:03:35 +0000 From: Will Deacon To: Lorenzo Pieralisi Cc: "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "linux-pci@vger.kernel.org" , Liviu Dudau , "suravee.suthikulpanit@amd.com" , Arnd Bergmann , Bjorn Helgaas Subject: Re: [PATCH RFC 2/2] drivers: pci: convert generic host controller to DT resource parsing API Message-ID: <20141027120335.GQ8768@arm.com> References: <1414077787-20633-1-git-send-email-lorenzo.pieralisi@arm.com> <1414077787-20633-2-git-send-email-lorenzo.pieralisi@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1414077787-20633-2-git-send-email-lorenzo.pieralisi@arm.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Oct 23, 2014 at 04:23:07PM +0100, Lorenzo Pieralisi wrote: > In order to consolidate DT configuration for PCI host controllers in the > kernel, a new API (ie of_pci_get_host_bridge_resources()) was developed > to allow parsing and assigning IO/BUS/MEM resources from DT, removing > duplicated code present in the majority of pci host driver implementations. > > This patch converts the existing PCI generic host controller driver to > the new API. Most of the code parsing ranges and creating resources is > now delegated to of_pci_get_host_bridge_resources() API. > > The PCI host controller code carries out resources filtering on the > resulting resource list and maps IO space by using the newly introduced > pci_ioremap_iospace() API. > > New code supports only one IO resource per generic host controller, which > should cater for all existing host controller configurations. > > Cc: Arnd Bergmann > Cc: Will Deacon > Cc: Bjorn Helgaas > Signed-off-by: Lorenzo Pieralisi > --- > drivers/pci/host/pci-host-generic.c | 120 ++++++++---------------------------- > 1 file changed, 27 insertions(+), 93 deletions(-) Looks fine to me -- I assume you tested this under a 32-bit kernel? Acked-by: Will Deacon Will > diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c > index 1e1a80f..1895907 100644 > --- a/drivers/pci/host/pci-host-generic.c > +++ b/drivers/pci/host/pci-host-generic.c > @@ -32,7 +32,7 @@ struct gen_pci_cfg_bus_ops { > > struct gen_pci_cfg_windows { > struct resource res; > - struct resource bus_range; > + struct resource *bus_range; > void __iomem **win; > > const struct gen_pci_cfg_bus_ops *ops; > @@ -50,7 +50,7 @@ static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus, > { > struct pci_sys_data *sys = bus->sysdata; > struct gen_pci *pci = sys->private_data; > - resource_size_t idx = bus->number - pci->cfg.bus_range.start; > + resource_size_t idx = bus->number - pci->cfg.bus_range->start; > > return pci->cfg.win[idx] + ((devfn << 8) | where); > } > @@ -66,7 +66,7 @@ static void __iomem *gen_pci_map_cfg_bus_ecam(struct pci_bus *bus, > { > struct pci_sys_data *sys = bus->sysdata; > struct gen_pci *pci = sys->private_data; > - resource_size_t idx = bus->number - pci->cfg.bus_range.start; > + resource_size_t idx = bus->number - pci->cfg.bus_range->start; > > return pci->cfg.win[idx] + ((devfn << 12) | where); > } > @@ -138,106 +138,50 @@ static const struct of_device_id gen_pci_of_match[] = { > }; > MODULE_DEVICE_TABLE(of, gen_pci_of_match); > > -static int gen_pci_calc_io_offset(struct device *dev, > - struct of_pci_range *range, > - struct resource *res, > - resource_size_t *offset) > -{ > - static atomic_t wins = ATOMIC_INIT(0); > - int err, idx, max_win; > - unsigned int window; > - > - if (!PAGE_ALIGNED(range->cpu_addr)) > - return -EINVAL; > - > - max_win = (IO_SPACE_LIMIT + 1) / SZ_64K; > - idx = atomic_inc_return(&wins); > - if (idx > max_win) > - return -ENOSPC; > - > - window = (idx - 1) * SZ_64K; > - err = pci_ioremap_io(window, range->cpu_addr); > - if (err) > - return err; > - > - of_pci_range_to_resource(range, dev->of_node, res); > - res->start = window; > - res->end = res->start + range->size - 1; > - *offset = window - range->pci_addr; > - return 0; > -} > - > -static int gen_pci_calc_mem_offset(struct device *dev, > - struct of_pci_range *range, > - struct resource *res, > - resource_size_t *offset) > -{ > - of_pci_range_to_resource(range, dev->of_node, res); > - *offset = range->cpu_addr - range->pci_addr; > - return 0; > -} > - > static void gen_pci_release_of_pci_ranges(struct gen_pci *pci) > { > - struct pci_host_bridge_window *win; > - > - list_for_each_entry(win, &pci->resources, list) > - release_resource(win->res); > - > pci_free_resource_list(&pci->resources); > } > > static int gen_pci_parse_request_of_pci_ranges(struct gen_pci *pci) > { > - struct of_pci_range range; > - struct of_pci_range_parser parser; > int err, res_valid = 0; > struct device *dev = pci->host.dev.parent; > struct device_node *np = dev->of_node; > + resource_size_t iobase; > + struct pci_host_bridge_window *win; > > - if (of_pci_range_parser_init(&parser, np)) { > - dev_err(dev, "missing \"ranges\" property\n"); > - return -EINVAL; > - } > + err = of_pci_get_host_bridge_resources(np, 0, 0xff, &pci->resources, > + &iobase); > + if (err) > + return err; > > - for_each_of_pci_range(&parser, &range) { > - struct resource *parent, *res; > - resource_size_t offset; > - u32 restype = range.flags & IORESOURCE_TYPE_BITS; > + list_for_each_entry(win, &pci->resources, list) { > + struct resource *parent, *res = win->res; > > - res = devm_kmalloc(dev, sizeof(*res), GFP_KERNEL); > - if (!res) { > - err = -ENOMEM; > - goto out_release_res; > - } > - > - switch (restype) { > + switch (resource_type(res)) { > case IORESOURCE_IO: > parent = &ioport_resource; > - err = gen_pci_calc_io_offset(dev, &range, res, &offset); > + err = pci_remap_iospace(res, iobase); > + if (err) { > + dev_warn(dev, "error %d: failed to map resource %pR\n", > + err, res); > + continue; > + } > break; > case IORESOURCE_MEM: > parent = &iomem_resource; > - err = gen_pci_calc_mem_offset(dev, &range, res, &offset); > - res_valid |= !(res->flags & IORESOURCE_PREFETCH || err); > + res_valid |= !(res->flags & IORESOURCE_PREFETCH); > break; > + case IORESOURCE_BUS: > + pci->cfg.bus_range = res; > default: > - err = -EINVAL; > continue; > } > > - if (err) { > - dev_warn(dev, > - "error %d: failed to add resource [type 0x%x, %lld bytes]\n", > - err, restype, range.size); > - continue; > - } > - > - err = request_resource(parent, res); > + err = devm_request_resource(dev, parent, res); > if (err) > goto out_release_res; > - > - pci_add_resource_offset(&pci->resources, res, offset); > } > > if (!res_valid) { > @@ -262,14 +206,6 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci) > struct device *dev = pci->host.dev.parent; > struct device_node *np = dev->of_node; > > - if (of_pci_parse_bus_range(np, &pci->cfg.bus_range)) > - pci->cfg.bus_range = (struct resource) { > - .name = np->name, > - .start = 0, > - .end = 0xff, > - .flags = IORESOURCE_BUS, > - }; > - > err = of_address_to_resource(np, 0, &pci->cfg.res); > if (err) { > dev_err(dev, "missing \"reg\" property\n"); > @@ -277,12 +213,12 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci) > } > > /* Limit the bus-range to fit within reg */ > - bus_max = pci->cfg.bus_range.start + > + bus_max = pci->cfg.bus_range->start + > (resource_size(&pci->cfg.res) >> pci->cfg.ops->bus_shift) - 1; > - pci->cfg.bus_range.end = min_t(resource_size_t, pci->cfg.bus_range.end, > - bus_max); > + pci->cfg.bus_range->end = min_t(resource_size_t, > + pci->cfg.bus_range->end, bus_max); > > - pci->cfg.win = devm_kcalloc(dev, resource_size(&pci->cfg.bus_range), > + pci->cfg.win = devm_kcalloc(dev, resource_size(pci->cfg.bus_range), > sizeof(*pci->cfg.win), GFP_KERNEL); > if (!pci->cfg.win) > return -ENOMEM; > @@ -293,7 +229,7 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci) > "Configuration Space")) > return -ENOMEM; > > - bus_range = &pci->cfg.bus_range; > + bus_range = pci->cfg.bus_range; > for (busn = bus_range->start; busn <= bus_range->end; ++busn) { > u32 idx = busn - bus_range->start; > u32 sz = 1 << pci->cfg.ops->bus_shift; > @@ -305,8 +241,6 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci) > return -ENOMEM; > } > > - /* Register bus resource */ > - pci_add_resource(&pci->resources, bus_range); > return 0; > } > > -- > 2.1.2 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/