From: Liviu Dudau <liviu@dudau.co.uk> To: Rob Herring <robherring2@gmail.com> Cc: Liviu Dudau <Liviu.Dudau@arm.com>, Bjorn Helgaas <bhelgaas@google.com>, Catalin Marinas <Catalin.Marinas@arm.com>, Will Deacon <Will.Deacon@arm.com>, Benjamin Herrenschmidt <benh@kernel.crashing.org>, Arnd Bergmann <arnd@arndb.de>, Russell King <linux@arm.linux.org.uk>, Tanmay Inamdar <tinamdar@apm.com>, Grant Likely <grant.likely@secretlab.ca>, Sinan Kaya <okaya@codeaurora.org>, Jingoo Han <jg1.han@samsung.com>, Kukjin Kim <kgene.kim@samsung.com>, Suravee Suthikulanit <suravee.suthikulpanit@amd.com>, linux-pci <linux-pci@vger.kernel.org>, linux-arch <linux-arch@vger.kernel.org>, Device Tree ML <devicetree@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>, LAKML <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH v9 04/12] PCI: OF: Fix the conversion of IO ranges into IO resources. Date: Fri, 22 Aug 2014 14:06:40 +0100 [thread overview] Message-ID: <20140822130640.GM13147@bart.dudau.co.uk> (raw) In-Reply-To: <CAL_JsqJm6qWNdrJ2F8xddFUj46y42n6hQVu1nkTaA-5Tbg--XQ@mail.gmail.com> On Thu, Aug 21, 2014 at 11:08:48PM -0500, Rob Herring wrote: > On Tue, Aug 12, 2014 at 11:25 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote: > > The ranges property for a host bridge controller in DT describes > > the mapping between the PCI bus address and the CPU physical address. > > The resources framework however expects that the IO resources start > > at a pseudo "port" address 0 (zero) and have a maximum size of IO_SPACE_LIMIT. > > The conversion from pci ranges to resources failed to take that into account. > > > > In the process move the function into drivers/of/address.c as it now > > depends on pci_address_to_pio() code and make it return an error code. > > > > Cc: Grant Likely <grant.likely@linaro.org> > > Cc: Rob Herring <robh+dt@kernel.org> > > Humm, this says I'm cc'ed, but I'm not which defeats the point of > recording the Cc's in the commit. Appologies, I've screwed up my git send-email arguments. > > I still have the same concerns that this will break existing users. > Are you sure integrator is the only platform affected? microblaze and powerpc have their similar handcoded routine for parsing ranges where they pre-compute the io_base and adjust the values again when registering resources. I'm not absolutely sure they are not broken as I lack the appropriate platforms to test (I've been asking for an FPGA engineer to build me a microblaze image with all the bits included but haven't received anything yet and it is possible Xilinx has now shifted their interests towards ARM + PCI as the ML605 board that I have seems to have been discontinued). mips is doing the same thing and I believe is not affected, pci-host-generic.c was adjusting the returned values afterwards so that will not be needed and Lorenzo has a patch for the driver to adapt it to this series anyway. pcie-designware.c also recalculates the io.start and io.end values, so that's fine for now. The only ones that I believe are still affected are pci-tegra.c and pcie-rcar.c for which I will need to provide a patch similar to integrator unless the code gets converted to the new range parsing. Best regards, Liviu > > Rob > > > Cc: Arnd Bergmann <arnd@arndb.de> > > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com> > > --- > > drivers/of/address.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ > > include/linux/of_address.h | 13 ++----------- > > 2 files changed, 48 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/of/address.c b/drivers/of/address.c > > index 4dab700..3735ac7 100644 > > --- a/drivers/of/address.c > > +++ b/drivers/of/address.c > > @@ -906,3 +906,49 @@ bool of_dma_is_coherent(struct device_node *np) > > return false; > > } > > EXPORT_SYMBOL_GPL(of_dma_is_coherent); > > + > > +/* > > + * of_pci_range_to_resource - Create a resource from an of_pci_range > > + * @range: the PCI range that describes the resource > > + * @np: device node where the range belongs to > > + * @res: pointer to a valid resource that will be updated to > > + * reflect the values contained in the range. > > + * > > + * Returns EINVAL if the range cannot be converted to resource. > > + * > > + * Note that if the range is an IO range, the resource will be converted > > + * using pci_address_to_pio() which can fail if it is called too early or > > + * if the range cannot be matched to any host bridge IO space (our case here). > > + * To guard against that we try to register the IO range first. > > + * If that fails we know that pci_address_to_pio() will do too. > > + */ > > +int of_pci_range_to_resource(struct of_pci_range *range, > > + struct device_node *np, struct resource *res) > > +{ > > + int err; > > + res->flags = range->flags; > > + res->parent = res->child = res->sibling = NULL; > > + res->name = np->full_name; > > + > > + if (res->flags & IORESOURCE_IO) { > > + unsigned long port = -1; > > + err = pci_register_io_range(range->cpu_addr, range->size); > > + if (err) > > + goto invalid_range; > > + port = pci_address_to_pio(range->cpu_addr); > > + if (port == (unsigned long)-1) { > > + err = -EINVAL; > > + goto invalid_range; > > + } > > + res->start = port; > > + } else { > > + res->start = range->cpu_addr; > > + } > > + res->end = res->start + range->size - 1; > > + return 0; > > + > > +invalid_range: > > + res->start = (resource_size_t)OF_BAD_ADDR; > > + res->end = (resource_size_t)OF_BAD_ADDR; > > + return err; > > +} > > diff --git a/include/linux/of_address.h b/include/linux/of_address.h > > index 28e6836..6015f21 100644 > > --- a/include/linux/of_address.h > > +++ b/include/linux/of_address.h > > @@ -23,17 +23,8 @@ struct of_pci_range { > > #define for_each_of_pci_range(parser, range) \ > > for (; of_pci_range_parser_one(parser, range);) > > > > -static inline void of_pci_range_to_resource(struct of_pci_range *range, > > - struct device_node *np, > > - struct resource *res) > > -{ > > - res->flags = range->flags; > > - res->start = range->cpu_addr; > > - res->end = range->cpu_addr + range->size - 1; > > - res->parent = res->child = res->sibling = NULL; > > - res->name = np->full_name; > > -} > > - > > +extern int of_pci_range_to_resource(struct of_pci_range *range, > > + struct device_node *np, struct resource *res); > > /* Translate a DMA address from device space to CPU space */ > > extern u64 of_translate_dma_address(struct device_node *dev, > > const __be32 *in_addr); > > -- > > 2.0.4 > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- ------------------- .oooO ( ) \ ( Oooo. \_) ( ) ) / (_/ One small step for me ...
WARNING: multiple messages have this Message-ID (diff)
From: liviu@dudau.co.uk (Liviu Dudau) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v9 04/12] PCI: OF: Fix the conversion of IO ranges into IO resources. Date: Fri, 22 Aug 2014 14:06:40 +0100 [thread overview] Message-ID: <20140822130640.GM13147@bart.dudau.co.uk> (raw) In-Reply-To: <CAL_JsqJm6qWNdrJ2F8xddFUj46y42n6hQVu1nkTaA-5Tbg--XQ@mail.gmail.com> On Thu, Aug 21, 2014 at 11:08:48PM -0500, Rob Herring wrote: > On Tue, Aug 12, 2014 at 11:25 AM, Liviu Dudau <Liviu.Dudau@arm.com> wrote: > > The ranges property for a host bridge controller in DT describes > > the mapping between the PCI bus address and the CPU physical address. > > The resources framework however expects that the IO resources start > > at a pseudo "port" address 0 (zero) and have a maximum size of IO_SPACE_LIMIT. > > The conversion from pci ranges to resources failed to take that into account. > > > > In the process move the function into drivers/of/address.c as it now > > depends on pci_address_to_pio() code and make it return an error code. > > > > Cc: Grant Likely <grant.likely@linaro.org> > > Cc: Rob Herring <robh+dt@kernel.org> > > Humm, this says I'm cc'ed, but I'm not which defeats the point of > recording the Cc's in the commit. Appologies, I've screwed up my git send-email arguments. > > I still have the same concerns that this will break existing users. > Are you sure integrator is the only platform affected? microblaze and powerpc have their similar handcoded routine for parsing ranges where they pre-compute the io_base and adjust the values again when registering resources. I'm not absolutely sure they are not broken as I lack the appropriate platforms to test (I've been asking for an FPGA engineer to build me a microblaze image with all the bits included but haven't received anything yet and it is possible Xilinx has now shifted their interests towards ARM + PCI as the ML605 board that I have seems to have been discontinued). mips is doing the same thing and I believe is not affected, pci-host-generic.c was adjusting the returned values afterwards so that will not be needed and Lorenzo has a patch for the driver to adapt it to this series anyway. pcie-designware.c also recalculates the io.start and io.end values, so that's fine for now. The only ones that I believe are still affected are pci-tegra.c and pcie-rcar.c for which I will need to provide a patch similar to integrator unless the code gets converted to the new range parsing. Best regards, Liviu > > Rob > > > Cc: Arnd Bergmann <arnd@arndb.de> > > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com> > > --- > > drivers/of/address.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ > > include/linux/of_address.h | 13 ++----------- > > 2 files changed, 48 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/of/address.c b/drivers/of/address.c > > index 4dab700..3735ac7 100644 > > --- a/drivers/of/address.c > > +++ b/drivers/of/address.c > > @@ -906,3 +906,49 @@ bool of_dma_is_coherent(struct device_node *np) > > return false; > > } > > EXPORT_SYMBOL_GPL(of_dma_is_coherent); > > + > > +/* > > + * of_pci_range_to_resource - Create a resource from an of_pci_range > > + * @range: the PCI range that describes the resource > > + * @np: device node where the range belongs to > > + * @res: pointer to a valid resource that will be updated to > > + * reflect the values contained in the range. > > + * > > + * Returns EINVAL if the range cannot be converted to resource. > > + * > > + * Note that if the range is an IO range, the resource will be converted > > + * using pci_address_to_pio() which can fail if it is called too early or > > + * if the range cannot be matched to any host bridge IO space (our case here). > > + * To guard against that we try to register the IO range first. > > + * If that fails we know that pci_address_to_pio() will do too. > > + */ > > +int of_pci_range_to_resource(struct of_pci_range *range, > > + struct device_node *np, struct resource *res) > > +{ > > + int err; > > + res->flags = range->flags; > > + res->parent = res->child = res->sibling = NULL; > > + res->name = np->full_name; > > + > > + if (res->flags & IORESOURCE_IO) { > > + unsigned long port = -1; > > + err = pci_register_io_range(range->cpu_addr, range->size); > > + if (err) > > + goto invalid_range; > > + port = pci_address_to_pio(range->cpu_addr); > > + if (port == (unsigned long)-1) { > > + err = -EINVAL; > > + goto invalid_range; > > + } > > + res->start = port; > > + } else { > > + res->start = range->cpu_addr; > > + } > > + res->end = res->start + range->size - 1; > > + return 0; > > + > > +invalid_range: > > + res->start = (resource_size_t)OF_BAD_ADDR; > > + res->end = (resource_size_t)OF_BAD_ADDR; > > + return err; > > +} > > diff --git a/include/linux/of_address.h b/include/linux/of_address.h > > index 28e6836..6015f21 100644 > > --- a/include/linux/of_address.h > > +++ b/include/linux/of_address.h > > @@ -23,17 +23,8 @@ struct of_pci_range { > > #define for_each_of_pci_range(parser, range) \ > > for (; of_pci_range_parser_one(parser, range);) > > > > -static inline void of_pci_range_to_resource(struct of_pci_range *range, > > - struct device_node *np, > > - struct resource *res) > > -{ > > - res->flags = range->flags; > > - res->start = range->cpu_addr; > > - res->end = range->cpu_addr + range->size - 1; > > - res->parent = res->child = res->sibling = NULL; > > - res->name = np->full_name; > > -} > > - > > +extern int of_pci_range_to_resource(struct of_pci_range *range, > > + struct device_node *np, struct resource *res); > > /* Translate a DMA address from device space to CPU space */ > > extern u64 of_translate_dma_address(struct device_node *dev, > > const __be32 *in_addr); > > -- > > 2.0.4 > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel at lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- ------------------- .oooO ( ) \ ( Oooo. \_) ( ) ) / (_/ One small step for me ...
next prev parent reply other threads:[~2014-08-22 13:09 UTC|newest] Thread overview: 112+ messages / expand[flat|nested] mbox.gz Atom feed top 2014-08-12 16:25 [PATCH v9 00/12] Support for creating generic PCI host bridges from DT Liviu Dudau 2014-08-12 16:25 ` Liviu Dudau 2014-08-12 16:25 ` Liviu Dudau 2014-08-12 16:25 ` [PATCH v9 01/12] Fix ioport_map() for !CONFIG_GENERIC_IOMAP cases Liviu Dudau 2014-08-12 16:25 ` Liviu Dudau 2014-08-12 16:25 ` Liviu Dudau 2014-08-12 16:25 ` [PATCH v9 02/12] PCI: OF: Parse and map the IRQ when adding the PCI device Liviu Dudau 2014-08-12 16:25 ` Liviu Dudau 2014-08-12 16:25 ` Liviu Dudau 2014-08-14 14:58 ` Wei Yang 2014-08-14 14:58 ` Wei Yang 2014-08-14 15:49 ` Liviu Dudau 2014-08-14 15:49 ` Liviu Dudau 2014-08-15 8:56 ` Wei Yang 2014-08-15 8:56 ` Wei Yang 2014-08-15 10:30 ` Liviu Dudau 2014-08-15 10:30 ` Liviu Dudau 2014-08-15 10:30 ` Liviu Dudau 2014-08-18 1:44 ` Wei Yang 2014-08-18 1:44 ` Wei Yang 2014-08-18 21:26 ` Liviu Dudau 2014-08-18 21:26 ` Liviu Dudau 2014-08-18 14:25 ` Catalin Marinas 2014-08-18 14:25 ` Catalin Marinas 2014-08-18 21:30 ` Liviu Dudau 2014-08-18 21:30 ` Liviu Dudau 2014-08-18 22:09 ` Catalin Marinas 2014-08-18 22:09 ` Catalin Marinas 2014-08-19 12:39 ` Arnd Bergmann 2014-08-19 12:39 ` Arnd Bergmann 2014-08-19 1:44 ` Wei Yang 2014-08-19 1:44 ` Wei Yang 2014-08-19 12:05 ` Liviu Dudau 2014-08-19 12:05 ` Liviu Dudau 2014-08-12 16:25 ` [PATCH v9 03/12] PCI: Introduce helper functions to deal with PCI I/O ranges Liviu Dudau 2014-08-12 16:25 ` Liviu Dudau 2014-08-12 16:25 ` Liviu Dudau 2014-08-18 14:26 ` Catalin Marinas 2014-08-18 14:26 ` Catalin Marinas 2014-08-18 14:26 ` Catalin Marinas 2014-08-18 21:34 ` Liviu Dudau 2014-08-18 21:34 ` Liviu Dudau 2014-08-18 21:52 ` Catalin Marinas 2014-08-18 21:52 ` Catalin Marinas 2014-08-22 4:59 ` Rob Herring 2014-08-22 4:59 ` Rob Herring 2014-08-22 4:59 ` Rob Herring 2014-09-02 3:43 ` Yijing Wang 2014-09-02 3:43 ` Yijing Wang 2014-08-12 16:25 ` [PATCH v9 04/12] PCI: OF: Fix the conversion of IO ranges into IO resources Liviu Dudau 2014-08-12 16:25 ` Liviu Dudau 2014-08-22 4:08 ` Rob Herring 2014-08-22 4:08 ` Rob Herring 2014-08-22 4:08 ` Rob Herring 2014-08-22 13:06 ` Liviu Dudau [this message] 2014-08-22 13:06 ` Liviu Dudau 2014-08-24 23:27 ` Rob Herring 2014-08-24 23:27 ` Rob Herring 2014-09-05 22:11 ` Bjorn Helgaas 2014-09-05 22:11 ` Bjorn Helgaas 2014-08-12 16:25 ` [PATCH v9 05/12] ARM: Define PCI_IOBASE as the base of virtual PCI IO space Liviu Dudau 2014-08-12 16:25 ` Liviu Dudau 2014-08-12 16:25 ` Liviu Dudau 2014-08-12 16:25 ` [PATCH v9 06/12] ARM: integrator: Correct usage of of_pci_range_to_resource() Liviu Dudau 2014-08-12 16:25 ` Liviu Dudau 2014-08-12 16:25 ` Liviu Dudau 2014-09-05 22:08 ` Bjorn Helgaas 2014-09-05 22:08 ` Bjorn Helgaas 2014-09-08 12:25 ` Liviu Dudau 2014-09-08 12:25 ` Liviu Dudau 2014-09-22 12:47 ` Linus Walleij 2014-09-22 12:47 ` Linus Walleij 2014-09-22 13:36 ` Liviu Dudau 2014-09-22 13:36 ` Liviu Dudau 2014-08-12 16:25 ` [PATCH v9 07/12] PCI: Create pci_host_bridge before its associated bus in pci_create_root_bus Liviu Dudau 2014-08-12 16:25 ` Liviu Dudau 2014-08-12 16:25 ` [PATCH v9 08/12] PCI: Introduce generic domain handling for PCI busses Liviu Dudau 2014-08-12 16:25 ` Liviu Dudau 2014-08-12 16:25 ` [PATCH v9 09/12] OF: Introduce helper function for getting PCI domain_nr Liviu Dudau 2014-08-12 16:25 ` Liviu Dudau 2014-08-12 16:25 ` [PATCH v9 10/12] OF: PCI: Add support for creating a generic host_bridge from DT Liviu Dudau 2014-08-12 16:25 ` Liviu Dudau 2014-08-12 16:25 ` Liviu Dudau 2014-08-12 16:25 ` [PATCH v9 11/12] arm64: Add pgprot_device() interface for device mappings Liviu Dudau 2014-08-12 16:25 ` Liviu Dudau 2014-08-12 16:25 ` Liviu Dudau 2014-08-13 9:59 ` Catalin Marinas 2014-08-13 9:59 ` Catalin Marinas 2014-08-12 16:25 ` [PATCH v9 12/12] PCI: Introduce pci_remap_iospace() for remapping PCI I/O bus resources into CPU space Liviu Dudau 2014-08-12 16:25 ` Liviu Dudau 2014-08-12 16:25 ` Liviu Dudau 2014-08-13 10:01 ` Catalin Marinas 2014-08-13 10:01 ` Catalin Marinas 2014-08-13 10:33 ` Liviu Dudau 2014-08-13 10:33 ` Liviu Dudau 2014-08-13 10:53 ` Catalin Marinas 2014-08-13 10:53 ` Catalin Marinas 2014-08-22 4:16 ` Rob Herring 2014-08-22 4:16 ` Rob Herring 2014-08-22 4:16 ` Rob Herring 2014-08-22 12:43 ` Liviu Dudau 2014-08-22 12:43 ` Liviu Dudau 2014-08-23 16:57 ` Rob Herring 2014-08-23 16:57 ` Rob Herring 2014-08-23 16:57 ` Rob Herring 2014-08-18 14:26 ` [PATCH v9 00/12] Support for creating generic PCI host bridges from DT Catalin Marinas 2014-08-18 14:26 ` Catalin Marinas 2014-08-18 21:35 ` Liviu Dudau 2014-08-18 21:35 ` Liviu Dudau 2014-08-27 16:24 ` Robert Richter 2014-08-27 16:24 ` Robert Richter 2014-08-27 16:24 ` Robert Richter
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20140822130640.GM13147@bart.dudau.co.uk \ --to=liviu@dudau.co.uk \ --cc=Catalin.Marinas@arm.com \ --cc=Liviu.Dudau@arm.com \ --cc=Will.Deacon@arm.com \ --cc=arnd@arndb.de \ --cc=benh@kernel.crashing.org \ --cc=bhelgaas@google.com \ --cc=devicetree@vger.kernel.org \ --cc=grant.likely@secretlab.ca \ --cc=jg1.han@samsung.com \ --cc=kgene.kim@samsung.com \ --cc=linux-arch@vger.kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=linux@arm.linux.org.uk \ --cc=okaya@codeaurora.org \ --cc=robherring2@gmail.com \ --cc=suravee.suthikulpanit@amd.com \ --cc=tinamdar@apm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.