* [PATCH v3] of/irq: provide more wrappers for !CONFIG_OF
@ 2014-06-03 14:47 Arnd Bergmann
2014-06-03 15:40 ` Rob Herring
0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2014-06-03 14:47 UTC (permalink / raw)
To: devicetree
Cc: Rob Herring, Grant Likely, Lucas Stach, Bjorn Helgaas,
Magnus Damm, Geert Uytterhoeven, Ben Dooks, linux-pci, linux-sh
The pci-rcar driver is enabled for compile tests, and this has
now shown that the driver cannot build without CONFIG_OF,
following the inclusion of f8f2fe7355fb "PCI: rcar: Use new OF
interrupt mapping when possible":
drivers/built-in.o: In function `rcar_pci_map_irq':
:(.text+0x1cc7c): undefined reference to `of_irq_parse_and_map_pci'
pci/host/pcie-rcar.c: In function 'pci_dma_range_parser_init':
pci/host/pcie-rcar.c:875:2: error: implicit declaration of function 'of_n_addr_cells' [-Werror=implicit-function-declaration]
As pointed out by Ben Dooks and Geert Uytterhoeven, this is actually
supposed to build fine, which we can achieve if we make the
declaration of of_irq_parse_and_map_pci conditional on CONFIG_OF
and provide an empty inline function otherwise, as we do for
a lot of other of interfaces.
This lets us build the rcar_pci driver again without CONFIG_OF
for build testing. All platforms using this driver select OF,
so this doesn't change anything for the users.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: devicetree@vger.kernel.org
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Grant Likely <grant.likely@linaro.org>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Magnus Damm <damm@opensource.se>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Ben Dooks <ben.dooks@codethink.co.uk>
Cc: linux-pci@vger.kernel.org
Cc: linux-sh@vger.kernel.org
----
This replaces "[PATCH v2] of/irq: provide int of_irq_parse_and_map_pci
wrapper", since now the same driver requires additional interfaces.
We still want to be able to build the driver with CONFIG_OF disabled,
but now we need three functions instead of just one.
Rob, Grant, can you apply this as a bug fix, or provide comments?
diff --git a/include/linux/of.h b/include/linux/of.h
index 196b34c..7c29e6c 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -511,6 +511,9 @@ static inline struct device_node *of_get_cpu_node(int cpu,
return NULL;
}
+static inline int of_n_addr_cells(struct device_node *np) { return 0; }
+static inline int of_n_size_cells(struct device_node *np) { return 0; }
+
static inline int of_property_read_u64(const struct device_node *np,
const char *propname, u64 *out_value)
{
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index 1a1f5ff..dde3a4a 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -6,14 +6,44 @@
struct pci_dev;
struct of_phandle_args;
-int of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args *out_irq);
-int of_irq_parse_and_map_pci(const struct pci_dev *dev, u8 slot, u8 pin);
-
struct device_node;
+
+#ifdef CONFIG_OF
+int of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args *out_irq);
struct device_node *of_pci_find_child_device(struct device_node *parent,
unsigned int devfn);
int of_pci_get_devfn(struct device_node *np);
+int of_irq_parse_and_map_pci(const struct pci_dev *dev, u8 slot, u8 pin);
int of_pci_parse_bus_range(struct device_node *node, struct resource *res);
+#else
+static inline int of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args *out_irq)
+{
+ return 0;
+}
+
+static inline struct device_node *of_pci_find_child_device(struct device_node *parent,
+ unsigned int devfn)
+{
+ return NULL;
+}
+
+static inline int of_pci_get_devfn(struct device_node *np)
+{
+ return -EINVAL;
+}
+
+static inline int
+of_irq_parse_and_map_pci(const struct pci_dev *dev, u8 slot, u8 pin)
+{
+ return 0;
+}
+
+static inline int
+of_pci_parse_bus_range(struct device_node *node, struct resource *res)
+{
+ return -EINVAL;
+}
+#endif
#if defined(CONFIG_OF) && defined(CONFIG_PCI_MSI)
int of_pci_msi_chip_add(struct msi_chip *chip);
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v3] of/irq: provide more wrappers for !CONFIG_OF
2014-06-03 14:47 [PATCH v3] of/irq: provide more wrappers for !CONFIG_OF Arnd Bergmann
@ 2014-06-03 15:40 ` Rob Herring
2014-06-03 15:59 ` Arnd Bergmann
2014-06-04 9:58 ` Ben Dooks
0 siblings, 2 replies; 6+ messages in thread
From: Rob Herring @ 2014-06-03 15:40 UTC (permalink / raw)
To: Arnd Bergmann
Cc: devicetree@vger.kernel.org, Rob Herring, Grant Likely,
Lucas Stach, Bjorn Helgaas, Magnus Damm, Geert Uytterhoeven,
Ben Dooks, linux-pci@vger.kernel.org, SH-Linux
On Tue, Jun 3, 2014 at 9:47 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> The pci-rcar driver is enabled for compile tests, and this has
> now shown that the driver cannot build without CONFIG_OF,
> following the inclusion of f8f2fe7355fb "PCI: rcar: Use new OF
> interrupt mapping when possible":
>
> drivers/built-in.o: In function `rcar_pci_map_irq':
> :(.text+0x1cc7c): undefined reference to `of_irq_parse_and_map_pci'
> pci/host/pcie-rcar.c: In function 'pci_dma_range_parser_init':
> pci/host/pcie-rcar.c:875:2: error: implicit declaration of function 'of_n_addr_cells' [-Werror=implicit-function-declaration]
>
> As pointed out by Ben Dooks and Geert Uytterhoeven, this is actually
> supposed to build fine, which we can achieve if we make the
> declaration of of_irq_parse_and_map_pci conditional on CONFIG_OF
> and provide an empty inline function otherwise, as we do for
> a lot of other of interfaces.
>
> This lets us build the rcar_pci driver again without CONFIG_OF
> for build testing. All platforms using this driver select OF,
> so this doesn't change anything for the users.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: devicetree@vger.kernel.org
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Magnus Damm <damm@opensource.se>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Ben Dooks <ben.dooks@codethink.co.uk>
> Cc: linux-pci@vger.kernel.org
> Cc: linux-sh@vger.kernel.org
> ----
>
> This replaces "[PATCH v2] of/irq: provide int of_irq_parse_and_map_pci
> wrapper", since now the same driver requires additional interfaces.
> We still want to be able to build the driver with CONFIG_OF disabled,
> but now we need three functions instead of just one.
>
> Rob, Grant, can you apply this as a bug fix, or provide comments?
>
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 196b34c..7c29e6c 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -511,6 +511,9 @@ static inline struct device_node *of_get_cpu_node(int cpu,
> return NULL;
> }
>
> +static inline int of_n_addr_cells(struct device_node *np) { return 0; }
> +static inline int of_n_size_cells(struct device_node *np) { return 0; }
I'm fine with the rest, but I think these should always be used within
some higher level function.
I can't seem to find where this is used by rcar. BTW, why does rcar
pci DT support fail to have any ranges property?
Looking at one use in mvebu pci only confirms my position. An
of_for_each_ranges helper would be useful in that case.
Rob
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v3] of/irq: provide more wrappers for !CONFIG_OF
2014-06-03 15:40 ` Rob Herring
@ 2014-06-03 15:59 ` Arnd Bergmann
2014-06-03 16:36 ` Rob Herring
2014-06-04 9:58 ` Ben Dooks
1 sibling, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2014-06-03 15:59 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree@vger.kernel.org, Rob Herring, Grant Likely,
Lucas Stach, Bjorn Helgaas, Magnus Damm, Geert Uytterhoeven,
Ben Dooks, linux-pci@vger.kernel.org, SH-Linux, Phil Edworthy
On Tuesday 03 June 2014 10:40:34 Rob Herring wrote:
> > diff --git a/include/linux/of.h b/include/linux/of.h
> > index 196b34c..7c29e6c 100644
> > --- a/include/linux/of.h
> > +++ b/include/linux/of.h
> > @@ -511,6 +511,9 @@ static inline struct device_node *of_get_cpu_node(int cpu,
> > return NULL;
> > }
> >
> > +static inline int of_n_addr_cells(struct device_node *np) { return 0; }
> > +static inline int of_n_size_cells(struct device_node *np) { return 0; }
>
> I'm fine with the rest, but I think these should always be used within
> some higher level function.
Can you apply the rest already?
> I can't seem to find where this is used by rcar. BTW, why does rcar
> pci DT support fail to have any ranges property?
>
> Looking at one use in mvebu pci only confirms my position. An
> of_for_each_ranges helper would be useful in that case.
It's currently in the pci-next, the code using this looks like
+static int pci_dma_range_parser_init(struct of_pci_range_parser *parser,
+ struct device_node *node)
+{
+ const int na = 3, ns = 2;
+ int rlen;
+
+ parser->node = node;
+ parser->pna = of_n_addr_cells(node);
+ parser->np = parser->pna + na + ns;
+
+ parser->range = of_get_property(node, "dma-ranges", &rlen);
+ if (!parser->range)
+ return -ENOENT;
+
+ parser->end = parser->range + rlen / sizeof(__be32);
+ return 0;
+}
+
+static int rcar_pcie_parse_map_dma_ranges(struct rcar_pcie *pcie,
+ struct device_node *np)
+{
+ struct of_pci_range range;
+ struct of_pci_range_parser parser;
+ int index = 0;
+ int err;
+
+ if (pci_dma_range_parser_init(&parser, np))
+ return -EINVAL;
+
+ /* Get the dma-ranges from DT */
+ for_each_of_pci_range(&parser, &range) {
+ u64 end = range.cpu_addr + range.size - 1;
+ dev_dbg(pcie->dev, "0x%08x 0x%016llx..0x%016llx -> 0x%016llx\n",
+ range.flags, range.cpu_addr, end, range.pci_addr);
+
+ err = rcar_pcie_inbound_ranges(pcie, &range, &index);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
Should the first function just be moved into of-pci.c?
Arnd
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v3] of/irq: provide more wrappers for !CONFIG_OF
2014-06-03 15:59 ` Arnd Bergmann
@ 2014-06-03 16:36 ` Rob Herring
0 siblings, 0 replies; 6+ messages in thread
From: Rob Herring @ 2014-06-03 16:36 UTC (permalink / raw)
To: Arnd Bergmann
Cc: devicetree@vger.kernel.org, Rob Herring, Grant Likely,
Lucas Stach, Bjorn Helgaas, Magnus Damm, Geert Uytterhoeven,
Ben Dooks, linux-pci@vger.kernel.org, SH-Linux, Phil Edworthy
On Tue, Jun 3, 2014 at 10:59 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 03 June 2014 10:40:34 Rob Herring wrote:
>> > diff --git a/include/linux/of.h b/include/linux/of.h
>> > index 196b34c..7c29e6c 100644
>> > --- a/include/linux/of.h
>> > +++ b/include/linux/of.h
>> > @@ -511,6 +511,9 @@ static inline struct device_node *of_get_cpu_node(int cpu,
>> > return NULL;
>> > }
>> >
>> > +static inline int of_n_addr_cells(struct device_node *np) { return 0; }
>> > +static inline int of_n_size_cells(struct device_node *np) { return 0; }
>>
>> I'm fine with the rest, but I think these should always be used within
>> some higher level function.
>
> Can you apply the rest already?
Yes, sure.
>
>> I can't seem to find where this is used by rcar. BTW, why does rcar
>> pci DT support fail to have any ranges property?
>>
>> Looking at one use in mvebu pci only confirms my position. An
>> of_for_each_ranges helper would be useful in that case.
>
> It's currently in the pci-next, the code using this looks like
>
> +static int pci_dma_range_parser_init(struct of_pci_range_parser *parser,
> + struct device_node *node)
> +{
> + const int na = 3, ns = 2;
> + int rlen;
> +
> + parser->node = node;
> + parser->pna = of_n_addr_cells(node);
> + parser->np = parser->pna + na + ns;
> +
> + parser->range = of_get_property(node, "dma-ranges", &rlen);
> + if (!parser->range)
> + return -ENOENT;
> +
> + parser->end = parser->range + rlen / sizeof(__be32);
> + return 0;
> +}
> +
> +static int rcar_pcie_parse_map_dma_ranges(struct rcar_pcie *pcie,
> + struct device_node *np)
> +{
> + struct of_pci_range range;
> + struct of_pci_range_parser parser;
> + int index = 0;
> + int err;
> +
> + if (pci_dma_range_parser_init(&parser, np))
> + return -EINVAL;
> +
> + /* Get the dma-ranges from DT */
> + for_each_of_pci_range(&parser, &range) {
> + u64 end = range.cpu_addr + range.size - 1;
> + dev_dbg(pcie->dev, "0x%08x 0x%016llx..0x%016llx -> 0x%016llx\n",
> + range.flags, range.cpu_addr, end, range.pci_addr);
> +
> + err = rcar_pcie_inbound_ranges(pcie, &range, &index);
> + if (err)
> + return err;
> + }
> +
> + return 0;
> +}
>
> Should the first function just be moved into of-pci.c?
Yes, and probably some refactoring with the "ranges" version of it too.
Rob
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] of/irq: provide more wrappers for !CONFIG_OF
2014-06-03 15:40 ` Rob Herring
2014-06-03 15:59 ` Arnd Bergmann
@ 2014-06-04 9:58 ` Ben Dooks
2014-06-04 10:17 ` Arnd Bergmann
1 sibling, 1 reply; 6+ messages in thread
From: Ben Dooks @ 2014-06-04 9:58 UTC (permalink / raw)
To: Rob Herring, Arnd Bergmann
Cc: devicetree@vger.kernel.org, Rob Herring, Grant Likely,
Lucas Stach, Bjorn Helgaas, Magnus Damm, Geert Uytterhoeven,
linux-pci@vger.kernel.org, SH-Linux
On 03/06/14 16:40, Rob Herring wrote:
> On Tue, Jun 3, 2014 at 9:47 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> The pci-rcar driver is enabled for compile tests, and this has
>> now shown that the driver cannot build without CONFIG_OF,
>> following the inclusion of f8f2fe7355fb "PCI: rcar: Use new OF
>> interrupt mapping when possible":
>>
>> drivers/built-in.o: In function `rcar_pci_map_irq':
>> :(.text+0x1cc7c): undefined reference to `of_irq_parse_and_map_pci'
>> pci/host/pcie-rcar.c: In function 'pci_dma_range_parser_init':
>> pci/host/pcie-rcar.c:875:2: error: implicit declaration of function 'of_n_addr_cells' [-Werror=implicit-function-declaration]
>>
>> As pointed out by Ben Dooks and Geert Uytterhoeven, this is actually
>> supposed to build fine, which we can achieve if we make the
>> declaration of of_irq_parse_and_map_pci conditional on CONFIG_OF
>> and provide an empty inline function otherwise, as we do for
>> a lot of other of interfaces.
>>
>> This lets us build the rcar_pci driver again without CONFIG_OF
>> for build testing. All platforms using this driver select OF,
>> so this doesn't change anything for the users.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> Cc: devicetree@vger.kernel.org
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Grant Likely <grant.likely@linaro.org>
>> Cc: Lucas Stach <l.stach@pengutronix.de>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> Cc: Magnus Damm <damm@opensource.se>
>> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
>> Cc: Ben Dooks <ben.dooks@codethink.co.uk>
>> Cc: linux-pci@vger.kernel.org
>> Cc: linux-sh@vger.kernel.org
>> ----
>>
>> This replaces "[PATCH v2] of/irq: provide int of_irq_parse_and_map_pci
>> wrapper", since now the same driver requires additional interfaces.
>> We still want to be able to build the driver with CONFIG_OF disabled,
>> but now we need three functions instead of just one.
>>
>> Rob, Grant, can you apply this as a bug fix, or provide comments?
>>
>> diff --git a/include/linux/of.h b/include/linux/of.h
>> index 196b34c..7c29e6c 100644
>> --- a/include/linux/of.h
>> +++ b/include/linux/of.h
>> @@ -511,6 +511,9 @@ static inline struct device_node *of_get_cpu_node(int cpu,
>> return NULL;
>> }
>>
>> +static inline int of_n_addr_cells(struct device_node *np) { return 0; }
>> +static inline int of_n_size_cells(struct device_node *np) { return 0; }
>
> I'm fine with the rest, but I think these should always be used within
> some higher level function.
>
> I can't seem to find where this is used by rcar. BTW, why does rcar
> pci DT support fail to have any ranges property?
I think the driver provides all the necessary PCI information
internally as it started off as a platform-driver.
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v3] of/irq: provide more wrappers for !CONFIG_OF
2014-06-04 9:58 ` Ben Dooks
@ 2014-06-04 10:17 ` Arnd Bergmann
0 siblings, 0 replies; 6+ messages in thread
From: Arnd Bergmann @ 2014-06-04 10:17 UTC (permalink / raw)
To: Ben Dooks
Cc: Rob Herring, devicetree@vger.kernel.org, Rob Herring,
Grant Likely, Lucas Stach, Bjorn Helgaas, Magnus Damm,
Geert Uytterhoeven, linux-pci@vger.kernel.org, SH-Linux
On Wednesday 04 June 2014 10:58:15 Ben Dooks wrote:
> On 03/06/14 16:40, Rob Herring wrote:
> > On Tue, Jun 3, 2014 at 9:47 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> This replaces "[PATCH v2] of/irq: provide int of_irq_parse_and_map_pci
> >> wrapper", since now the same driver requires additional interfaces.
> >> We still want to be able to build the driver with CONFIG_OF disabled,
> >> but now we need three functions instead of just one.
> >>
> >> Rob, Grant, can you apply this as a bug fix, or provide comments?
> >>
> >> diff --git a/include/linux/of.h b/include/linux/of.h
> >> index 196b34c..7c29e6c 100644
> >> --- a/include/linux/of.h
> >> +++ b/include/linux/of.h
> >> @@ -511,6 +511,9 @@ static inline struct device_node *of_get_cpu_node(int cpu,
> >> return NULL;
> >> }
> >>
> >> +static inline int of_n_addr_cells(struct device_node *np) { return 0; }
> >> +static inline int of_n_size_cells(struct device_node *np) { return 0; }
> >
> > I'm fine with the rest, but I think these should always be used within
> > some higher level function.
> >
> > I can't seem to find where this is used by rcar. BTW, why does rcar
> > pci DT support fail to have any ranges property?
>
> I think the driver provides all the necessary PCI information
> internally as it started off as a platform-driver.
Actually it gets the memory resource from the 'reg' property instead
of parsing the 'ranges' property, which of course is not compliant
with the generic PCI binding.
It also sets up an I/O resource that is located at the same location
as the memory resource, which is just a bug but might be used to
work around problems of the PCI core when no I/O resource is present.
I think that should be fixed.
I also see that the ranges parser in the pcie-rcar driver still gets
I/O space wrong, we should probably move that to a common range
parser once the arm64 pci implementation is in place and we can
share more of the common helpers.
Arnd
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-06-04 10:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-03 14:47 [PATCH v3] of/irq: provide more wrappers for !CONFIG_OF Arnd Bergmann
2014-06-03 15:40 ` Rob Herring
2014-06-03 15:59 ` Arnd Bergmann
2014-06-03 16:36 ` Rob Herring
2014-06-04 9:58 ` Ben Dooks
2014-06-04 10:17 ` Arnd Bergmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).