From: Thierry Reding thierry.reding@avionic-design.de Subject: Re: [PATCH 1/1] arm/dts: Tegra30: Add device tree support for SMMU Date: Wed, 18 Apr 2012 07:41:40 +0200 Message-ID: 20120418054140.GA17506@avionic-0098.adnet.avionic-design.de
PGP Signed by an unknown key
Hiroshi Doyu wrote:
On Mon, 16 Apr 2012 17:34:19 +0200 Arnd Bergmann arnd@arndb.de wrote:
On Monday 16 April 2012, Hiroshi Doyu wrote:
What about using "dma-window" property to specify IOVA range in dtsi as below?
arch/powerpc/platforms/cell/iommu.c:
698 static int __init cell_iommu_get_window(struct device_node *np, 699 unsigned long *base, 700 unsigned long *size) 701 { 702 const void *dma_window; 703 unsigned long index; 704 705 /* Use ibm,dma-window if available, else, hard code ! */ 706 dma_window = of_get_property(np, "ibm,dma-window", NULL); 707 if (dma_window == NULL) { ^^^^^^^^^^^^^^ 708 *base = 0; 709 *size = 0x80000000u; 710 return -ENODEV; 711 } 712 713 of_parse_dma_window(np, dma_window, &index, base, size); 714 return 0; 715 }
Yes, that's the right way to do it, but I would use the more generic "dma-window" name rather than "ibm,dma-window", which was originally introduced for pseries and for some reason copied into the cell qs2x firmware.
Which name is better, "dma-window" or "iova-window"? Considering DMA IOMMU mapping API, "dma-window" may sound appropriate to me.
But the window isn't necessarily used for DMA transfers. I was going to propose "aperture", but that doesn't seem quite right since it is a very GART-specific term. "iova-window" sounds like the best fit for what it really is.
If we can consider IOMMU as one of the implementation of DMA(Direct Memory Access), the prefix "dma-(window)" may make sense here. Then, we don't have to introduce a new concept "IO virtual address(iova)" in addition to the existing "bus address"(?)
Anyway, either name would be ok for me;)
On Wednesday 18 April 2012, Hiroshi Doyu wrote:
If we can consider IOMMU as one of the implementation of DMA(Direct Memory Access), the prefix "dma-(window)" may make sense here. Then, we don't have to introduce a new concept "IO virtual address(iova)" in addition to the existing "bus address"(?)
Anyway, either name would be ok for me;)
I would just use dma-window, without the "ibm," prefix but following the same conventions. Note that the of_parse_dma_window function is currently only defined in powerpc specific code, and should get moved to drivers/of from arch/powerpc/kernel/prom_parse.c.
Arnd
From: Arnd Bergmann arnd@arndb.de Subject: Re: [PATCH 1/1] arm/dts: Tegra30: Add device tree support for SMMU Date: Wed, 18 Apr 2012 09:31:53 +0200 Message-ID: 201204180731.54064.arnd@arndb.de
On Wednesday 18 April 2012, Hiroshi Doyu wrote:
If we can consider IOMMU as one of the implementation of DMA(Direct Memory Access), the prefix "dma-(window)" may make sense here. Then, we don't have to introduce a new concept "IO virtual address(iova)" in addition to the existing "bus address"(?)
Anyway, either name would be ok for me;)
I would just use dma-window, without the "ibm," prefix but following the same conventions. Note that the of_parse_dma_window function is currently only defined in powerpc specific code, and should get moved to drivers/of from arch/powerpc/kernel/prom_parse.c.
Something like below?
At least, I verified that this works with "tegra-smmu".
From 67c1dd493637c9e972d04e061a8e67049687021a Mon Sep 17 00:00:00 2001
From: Hiroshi DOYU hdoyu@nvidia.com Date: Wed, 18 Apr 2012 12:09:03 +0300 Subject: [PATCH 1/1] dt: Add general DMA window parser
This code was stolen from: "arch/microblaze/kernel/prom_parse.c" "arch/powerpc/kernel/prom_parse.c"
Once "ibm," prefix is removed from dts file. This generic one could replace the originals.
Signed-off-by: Hiroshi DOYU hdoyu@nvidia.com --- drivers/of/Kconfig | 4 ++++ drivers/of/Makefile | 1 + drivers/of/of_dma.c | 35 +++++++++++++++++++++++++++++++++++ include/linux/of_address.h | 10 ++++++++++ 4 files changed, 50 insertions(+), 0 deletions(-)
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig index dfba3e6..3b0298b 100644 --- a/drivers/of/Kconfig +++ b/drivers/of/Kconfig @@ -83,4 +83,8 @@ config OF_MTD depends on MTD def_bool y
+config OF_DMA + depends on HAS_DMA + def_bool y + endmenu # OF diff --git a/drivers/of/Makefile b/drivers/of/Makefile index e027f44..711ff5b 100644 --- a/drivers/of/Makefile +++ b/drivers/of/Makefile @@ -11,3 +11,4 @@ obj-$(CONFIG_OF_MDIO) += of_mdio.o obj-$(CONFIG_OF_PCI) += of_pci.o obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o obj-$(CONFIG_OF_MTD) += of_mtd.o +obj-$(CONFIG_OF_DMA) += of_dma.o diff --git a/drivers/of/of_dma.c b/drivers/of/of_dma.c new file mode 100644 index 0000000..1db1ccd --- /dev/null +++ b/drivers/of/of_dma.c @@ -0,0 +1,35 @@ +/* + * Stealed from: + * "arch/microblaze/kernel/prom_parse.c" + * "arch/powerpc/kernel/prom_parse.c" + */ + +#include <linux/of_address.h> + +void of_parse_dma_window(struct device_node *dn, const void *dma_window_prop, + unsigned long *busno, unsigned long *phys, unsigned long *size) +{ + const u32 *dma_window; + u32 cells; + const unsigned char *prop; + + dma_window = dma_window_prop; + + /* busno is always one cell */ + if (busno) + *busno = *(dma_window++); + + prop = of_get_property(dn, "#dma-address-cells", NULL); + if (!prop) + prop = of_get_property(dn, "#address-cells", NULL); + + cells = prop ? *(u32 *)prop : of_n_addr_cells(dn); + *phys = of_read_number(dma_window, cells); + + dma_window += cells; + + prop = of_get_property(dn, "#dma-size-cells", NULL); + cells = prop ? *(u32 *)prop : of_n_size_cells(dn); + *size = of_read_number(dma_window, cells); +} + diff --git a/include/linux/of_address.h b/include/linux/of_address.h index 01b925a..2a0f7c6 100644 --- a/include/linux/of_address.h +++ b/include/linux/of_address.h @@ -21,6 +21,10 @@ extern void __iomem *of_iomap(struct device_node *device, int index); extern const u32 *of_get_address(struct device_node *dev, int index, u64 *size, unsigned int *flags);
+extern void of_parse_dma_window(struct device_node *dn, + const void *dma_window_prop, unsigned long *busno, + unsigned long *phys, unsigned long *size); + #ifndef pci_address_to_pio static inline unsigned long pci_address_to_pio(phys_addr_t addr) { return -1; } #define pci_address_to_pio pci_address_to_pio @@ -48,6 +52,12 @@ static inline const u32 *of_get_address(struct device_node *dev, int index, { return NULL; } + +static inline void of_parse_dma_window(struct device_node *dn, + const void *dma_window_prop, unsigned long *busno, + unsigned long *phys, unsigned long *size) +{ +} #endif /* CONFIG_OF_ADDRESS */
* Hiroshi Doyu wrote:
diff --git a/drivers/of/of_dma.c b/drivers/of/of_dma.c new file mode 100644 index 0000000..1db1ccd --- /dev/null +++ b/drivers/of/of_dma.c @@ -0,0 +1,35 @@ +/*
- Stealed from:
"Stolen from"
- "arch/microblaze/kernel/prom_parse.c"
- "arch/powerpc/kernel/prom_parse.c"
- */
+#include <linux/of_address.h>
+void of_parse_dma_window(struct device_node *dn, const void *dma_window_prop,
unsigned long *busno, unsigned long *phys, unsigned long *size)+{
- const u32 *dma_window;
Should be __be32.
- u32 cells;
- const unsigned char *prop;
- dma_window = dma_window_prop;
- /* busno is always one cell */
- if (busno)
*busno = *(dma_window++);
This needs endianness conversion:
*busno = be32_to_cpup(dma_window++);
- prop = of_get_property(dn, "#dma-address-cells", NULL);
- if (!prop)
prop = of_get_property(dn, "#address-cells", NULL);- cells = prop ? *(u32 *)prop : of_n_addr_cells(dn);
Same here.
- *phys = of_read_number(dma_window, cells);
- dma_window += cells;
- prop = of_get_property(dn, "#dma-size-cells", NULL);
- cells = prop ? *(u32 *)prop : of_n_size_cells(dn);
And here.
Thierry
Hi,
Try again with fixing Thierry's commnets, Thanks.
From 9a632c24949e46df197a216ca95f684edd1db693 Mon Sep 17 00:00:00 2001
From: Hiroshi DOYU hdoyu@nvidia.com Date: Wed, 18 Apr 2012 12:09:03 +0300 Subject: [PATCH 1/1] dt: Add general DMA window parser
This code was stolen from: "arch/microblaze/kernel/prom_parse.c" "arch/powerpc/kernel/prom_parse.c"
Once "ibm," prefix is removed from dts file. This generic one could replace the originals.
Signed-off-by: Hiroshi DOYU hdoyu@nvidia.com --- drivers/of/Kconfig | 4 ++++ drivers/of/Makefile | 1 + drivers/of/of_dma.c | 35 +++++++++++++++++++++++++++++++++++ include/linux/of_address.h | 10 ++++++++++ 4 files changed, 50 insertions(+), 0 deletions(-)
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig index dfba3e6..3b0298b 100644 --- a/drivers/of/Kconfig +++ b/drivers/of/Kconfig @@ -83,4 +83,8 @@ config OF_MTD depends on MTD def_bool y
+config OF_DMA + depends on HAS_DMA + def_bool y + endmenu # OF diff --git a/drivers/of/Makefile b/drivers/of/Makefile index e027f44..711ff5b 100644 --- a/drivers/of/Makefile +++ b/drivers/of/Makefile @@ -11,3 +11,4 @@ obj-$(CONFIG_OF_MDIO) += of_mdio.o obj-$(CONFIG_OF_PCI) += of_pci.o obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o obj-$(CONFIG_OF_MTD) += of_mtd.o +obj-$(CONFIG_OF_DMA) += of_dma.o diff --git a/drivers/of/of_dma.c b/drivers/of/of_dma.c new file mode 100644 index 0000000..a34db5a --- /dev/null +++ b/drivers/of/of_dma.c @@ -0,0 +1,35 @@ +/* + * Stolen from: + * "arch/microblaze/kernel/prom_parse.c" + * "arch/powerpc/kernel/prom_parse.c" + */ + +#include <linux/of_address.h> + +void of_parse_dma_window(struct device_node *dn, const void *dma_window_prop, + unsigned long *busno, unsigned long *phys, unsigned long *size) +{ + const __be32 *dma_window; + u32 cells; + const unsigned char *prop; + + dma_window = dma_window_prop; + + /* busno is always one cell */ + if (busno) + *busno = be32_to_cpup(dma_window++); + + prop = of_get_property(dn, "#dma-address-cells", NULL); + if (!prop) + prop = of_get_property(dn, "#address-cells", NULL); + + cells = prop ? *(__be32 *)prop : of_n_addr_cells(dn); + *phys = of_read_number(dma_window, cells); + + dma_window += cells; + + prop = of_get_property(dn, "#dma-size-cells", NULL); + cells = prop ? *(__be32 *)prop : of_n_size_cells(dn); + *size = of_read_number(dma_window, cells); +} + diff --git a/include/linux/of_address.h b/include/linux/of_address.h index 01b925a..2a0f7c6 100644 --- a/include/linux/of_address.h +++ b/include/linux/of_address.h @@ -21,6 +21,10 @@ extern void __iomem *of_iomap(struct device_node *device, int index); extern const u32 *of_get_address(struct device_node *dev, int index, u64 *size, unsigned int *flags);
+extern void of_parse_dma_window(struct device_node *dn, + const void *dma_window_prop, unsigned long *busno, + unsigned long *phys, unsigned long *size); + #ifndef pci_address_to_pio static inline unsigned long pci_address_to_pio(phys_addr_t addr) { return -1; } #define pci_address_to_pio pci_address_to_pio @@ -48,6 +52,12 @@ static inline const u32 *of_get_address(struct device_node *dev, int index, { return NULL; } + +static inline void of_parse_dma_window(struct device_node *dn, + const void *dma_window_prop, unsigned long *busno, + unsigned long *phys, unsigned long *size) +{ +} #endif /* CONFIG_OF_ADDRESS */
* Hiroshi Doyu wrote:
- cells = prop ? *(__be32 *)prop : of_n_addr_cells(dn);
I think this needs to be:
cells = prop ? be32_to_cpup(prop) : of_n_addr_cells(dn);
Only casting isn't enough, you need the bytes to be swapped.
- cells = prop ? *(__be32 *)prop : of_n_size_cells(dn);
Similarly:
cells = prop ? be32_to_cpup(prop) : of_n_size_cells(dn);
Thierry
From: Thierry Reding thierry.reding@avionic-design.de Subject: Re: [PATCH 1/1] dt: Add general DMA window parser Date: Wed, 18 Apr 2012 12:26:29 +0200 Message-ID: 20120418102629.GA14533@avionic-0098.mockup.avionic-design.de
PGP Signed by an unknown key
Hiroshi Doyu wrote:
- cells = prop ? *(__be32 *)prop : of_n_addr_cells(dn);
I think this needs to be:
cells = prop ? be32_to_cpup(prop) : of_n_addr_cells(dn);
Only casting isn't enough, you need the bytes to be swapped.
Right. Try again. Sorry for spamming.
From 1ee8a9b3a839456b170af74b11a4304bfda965c4 Mon Sep 17 00:00:00 2001
From: Hiroshi DOYU hdoyu@nvidia.com Date: Wed, 18 Apr 2012 12:09:03 +0300 Subject: [PATCH 1/1] dt: Add general DMA window parser
This code was stolen from: "arch/microblaze/kernel/prom_parse.c" "arch/powerpc/kernel/prom_parse.c"
Once "ibm," prefix is removed from dts file. This generic one could replace the originals.
Signed-off-by: Hiroshi DOYU hdoyu@nvidia.com --- drivers/of/Kconfig | 4 ++++ drivers/of/Makefile | 1 + drivers/of/of_dma.c | 35 +++++++++++++++++++++++++++++++++++ include/linux/of_address.h | 10 ++++++++++ 4 files changed, 50 insertions(+), 0 deletions(-)
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig index dfba3e6..3b0298b 100644 --- a/drivers/of/Kconfig +++ b/drivers/of/Kconfig @@ -83,4 +83,8 @@ config OF_MTD depends on MTD def_bool y
+config OF_DMA + depends on HAS_DMA + def_bool y + endmenu # OF diff --git a/drivers/of/Makefile b/drivers/of/Makefile index e027f44..711ff5b 100644 --- a/drivers/of/Makefile +++ b/drivers/of/Makefile @@ -11,3 +11,4 @@ obj-$(CONFIG_OF_MDIO) += of_mdio.o obj-$(CONFIG_OF_PCI) += of_pci.o obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o obj-$(CONFIG_OF_MTD) += of_mtd.o +obj-$(CONFIG_OF_DMA) += of_dma.o diff --git a/drivers/of/of_dma.c b/drivers/of/of_dma.c new file mode 100644 index 0000000..45c9e88 --- /dev/null +++ b/drivers/of/of_dma.c @@ -0,0 +1,35 @@ +/* + * Stolen from: + * "arch/microblaze/kernel/prom_parse.c" + * "arch/powerpc/kernel/prom_parse.c" + */ + +#include <linux/of_address.h> + +void of_parse_dma_window(struct device_node *dn, const void *dma_window_prop, + unsigned long *busno, unsigned long *phys, unsigned long *size) +{ + const __be32 *dma_window; + u32 cells; + const unsigned char *prop; + + dma_window = dma_window_prop; + + /* busno is always one cell */ + if (busno) + *busno = be32_to_cpup(dma_window++); + + prop = of_get_property(dn, "#dma-address-cells", NULL); + if (!prop) + prop = of_get_property(dn, "#address-cells", NULL); + + cells = prop ? be32_to_cpup(prop) : of_n_addr_cells(dn); + *phys = of_read_number(dma_window, cells); + + dma_window += cells; + + prop = of_get_property(dn, "#dma-size-cells", NULL); + cells = prop ? be32_to_cpup(prop) : of_n_size_cells(dn); + *size = of_read_number(dma_window, cells); +} + diff --git a/include/linux/of_address.h b/include/linux/of_address.h index 01b925a..2a0f7c6 100644 --- a/include/linux/of_address.h +++ b/include/linux/of_address.h @@ -21,6 +21,10 @@ extern void __iomem *of_iomap(struct device_node *device, int index); extern const u32 *of_get_address(struct device_node *dev, int index, u64 *size, unsigned int *flags);
+extern void of_parse_dma_window(struct device_node *dn, + const void *dma_window_prop, unsigned long *busno, + unsigned long *phys, unsigned long *size); + #ifndef pci_address_to_pio static inline unsigned long pci_address_to_pio(phys_addr_t addr) { return -1; } #define pci_address_to_pio pci_address_to_pio @@ -48,6 +52,12 @@ static inline const u32 *of_get_address(struct device_node *dev, int index, { return NULL; } + +static inline void of_parse_dma_window(struct device_node *dn, + const void *dma_window_prop, unsigned long *busno, + unsigned long *phys, unsigned long *size) +{ +} #endif /* CONFIG_OF_ADDRESS */
* Hiroshi Doyu wrote:
diff --git a/drivers/of/of_dma.c b/drivers/of/of_dma.c new file mode 100644 index 0000000..45c9e88 --- /dev/null +++ b/drivers/of/of_dma.c @@ -0,0 +1,35 @@ +/*
- Stolen from:
- "arch/microblaze/kernel/prom_parse.c"
- "arch/powerpc/kernel/prom_parse.c"
- */
+#include <linux/of_address.h>
+void of_parse_dma_window(struct device_node *dn, const void *dma_window_prop,
unsigned long *busno, unsigned long *phys, unsigned long *size)+{
- const __be32 *dma_window;
- u32 cells;
- const unsigned char *prop;
There's no need for this to be const unsigned char *, const void * will do just as well.
- dma_window = dma_window_prop;
- /* busno is always one cell */
- if (busno)
*busno = be32_to_cpup(dma_window++);- prop = of_get_property(dn, "#dma-address-cells", NULL);
- if (!prop)
prop = of_get_property(dn, "#address-cells", NULL);- cells = prop ? be32_to_cpup(prop) : of_n_addr_cells(dn);
- *phys = of_read_number(dma_window, cells);
This should probably fail gracefully if phys == NULL, similar to what you do for busno.
- dma_window += cells;
- prop = of_get_property(dn, "#dma-size-cells", NULL);
- cells = prop ? be32_to_cpup(prop) : of_n_size_cells(dn);
- *size = of_read_number(dma_window, cells);
Same here.
+}
And you might want to add a EXPORT_SYMBOL(of_parse_dma_window) here so the function can be used from modules.
Sorry for having you go through another round. I should have looked more closely before.
Thierry
From: Thierry Reding thierry.reding@avionic-design.de Subject: Re: [PATCH 1/1] dt: Add general DMA window parser Date: Wed, 18 Apr 2012 12:54:23 +0200 Message-ID: 20120418105423.GA5667@avionic-0098.mockup.avionic-design.de
PGP Signed by an unknown key
Hiroshi Doyu wrote:
diff --git a/drivers/of/of_dma.c b/drivers/of/of_dma.c new file mode 100644 index 0000000..45c9e88 --- /dev/null +++ b/drivers/of/of_dma.c @@ -0,0 +1,35 @@ +/*
- Stolen from:
- "arch/microblaze/kernel/prom_parse.c"
- "arch/powerpc/kernel/prom_parse.c"
- */
+#include <linux/of_address.h>
+void of_parse_dma_window(struct device_node *dn, const void *dma_window_prop,
unsigned long *busno, unsigned long *phys, unsigned long *size)+{
- const __be32 *dma_window;
- u32 cells;
- const unsigned char *prop;
There's no need for this to be const unsigned char *, const void * will do just as well.
- dma_window = dma_window_prop;
- /* busno is always one cell */
- if (busno)
*busno = be32_to_cpup(dma_window++);- prop = of_get_property(dn, "#dma-address-cells", NULL);
- if (!prop)
prop = of_get_property(dn, "#address-cells", NULL);- cells = prop ? be32_to_cpup(prop) : of_n_addr_cells(dn);
- *phys = of_read_number(dma_window, cells);
This should probably fail gracefully if phys == NULL, similar to what you do for busno.
- dma_window += cells;
- prop = of_get_property(dn, "#dma-size-cells", NULL);
- cells = prop ? be32_to_cpup(prop) : of_n_size_cells(dn);
- *size = of_read_number(dma_window, cells);
Same here.
+}
And you might want to add a EXPORT_SYMBOL(of_parse_dma_window) here so the function can be used from modules.
Sorry for having you go through another round. I should have looked more closely before.
No problem, I'll wait for another comments, and post again later. Thank you for reivew.
On 04/18/2012 04:19 AM, Hiroshi Doyu wrote:
Subject: [PATCH 1/1] dt: Add general DMA window parser
This code was stolen from: "arch/microblaze/kernel/prom_parse.c" "arch/powerpc/kernel/prom_parse.c"
Once "ibm," prefix is removed from dts file. This generic one could replace the originals.
+extern void of_parse_dma_window(struct device_node *dn,
const void *dma_window_prop, unsigned long *busno,unsigned long *phys, unsigned long *size);
At least some other of_*() parsing functions take the property name rather than the property pointer, and also take an index into the property in order to support multiple entries in it. See for example of_parse_phandle and of_get_named_gpio_flags. Should this new API be similar? E.g.:
extern void of_parse_dma_window(struct device_node *np, const char *propname, int index, unsigned long *busno, unsigned long *phys, unsigned long *size);
* Stephen Warren wrote:
On 04/18/2012 04:19 AM, Hiroshi Doyu wrote:
Subject: [PATCH 1/1] dt: Add general DMA window parser
This code was stolen from: "arch/microblaze/kernel/prom_parse.c" "arch/powerpc/kernel/prom_parse.c"
Once "ibm," prefix is removed from dts file. This generic one could replace the originals.
+extern void of_parse_dma_window(struct device_node *dn,
const void *dma_window_prop, unsigned long *busno,unsigned long *phys, unsigned long *size);At least some other of_*() parsing functions take the property name rather than the property pointer, and also take an index into the property in order to support multiple entries in it. See for example of_parse_phandle and of_get_named_gpio_flags. Should this new API be similar? E.g.:
extern void of_parse_dma_window(struct device_node *np, const char *propname, int index, unsigned long *busno, unsigned long *phys, unsigned long *size);
In that case the function should return int for proper error handling.
Thierry
On Wed, 18 Apr 2012 21:39:45 +0200 Thierry Reding thierry.reding@avionic-design.de wrote:
PGP Signed by an unknown key
Stephen Warren wrote:
On 04/18/2012 04:19 AM, Hiroshi Doyu wrote:
Subject: [PATCH 1/1] dt: Add general DMA window parser
This code was stolen from: "arch/microblaze/kernel/prom_parse.c" "arch/powerpc/kernel/prom_parse.c"
Once "ibm," prefix is removed from dts file. This generic one could replace the originals.
+extern void of_parse_dma_window(struct device_node *dn,
const void *dma_window_prop, unsigned long *busno,unsigned long *phys, unsigned long *size);At least some other of_*() parsing functions take the property name rather than the property pointer, and also take an index into the property in order to support multiple entries in it. See for example of_parse_phandle and of_get_named_gpio_flags. Should this new API be similar? E.g.:
extern void of_parse_dma_window(struct device_node *np, const char *propname, int index, unsigned long *busno, unsigned long *phys, unsigned long *size);
In that case the function should return int for proper error handling.
Good point. I'll.
On Wed, 18 Apr 2012 19:27:58 +0200 Stephen Warren swarren@wwwdotorg.org wrote:
On 04/18/2012 04:19 AM, Hiroshi Doyu wrote:
Subject: [PATCH 1/1] dt: Add general DMA window parser
This code was stolen from: "arch/microblaze/kernel/prom_parse.c" "arch/powerpc/kernel/prom_parse.c"
Once "ibm," prefix is removed from dts file. This generic one could replace the originals.
It seems that the "ibm," prefix is not used in .dts files, but it comes from the PAPR compliant firmware and cannot be changed.
The easiest solution would be to make the function understand both variants.
+extern void of_parse_dma_window(struct device_node *dn,
const void *dma_window_prop, unsigned long *busno,unsigned long *phys, unsigned long *size);At least some other of_*() parsing functions take the property name rather than the property pointer, and also take an index into the property in order to support multiple entries in it. See for example of_parse_phandle and of_get_named_gpio_flags. Should this new API be similar? E.g.:
extern void of_parse_dma_window(struct device_node *np, const char *propname, int index, unsigned long *busno, unsigned long *phys, unsigned long *size);
At least, I can add the code checking the return value of this function, which won't change the existing code.
Does anyone know the format of "ibm,dma-window"?
From: Hiroshi DOYU hdoyu@nvidia.com
This code was based on: "arch/microblaze/kernel/prom_parse.c" "arch/powerpc/kernel/prom_parse.c"
"ibm," prefix could be supported with some modification.
Signed-off-by: Hiroshi DOYU hdoyu@nvidia.com --- v2: - Add several error checks. (Thierry Reding) - Use property name given to func. (Stephen Warren) - Support multiple entries. (Stephen Warren)
--- drivers/of/Kconfig | 4 ++ drivers/of/Makefile | 1 + drivers/of/of_dma.c | 67 ++++++++++++++++++++++++++++++++++++++++++++ include/linux/of_address.h | 13 ++++++++ 4 files changed, 85 insertions(+), 0 deletions(-)
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig index dfba3e6..3b0298b 100644 --- a/drivers/of/Kconfig +++ b/drivers/of/Kconfig @@ -83,4 +83,8 @@ config OF_MTD depends on MTD def_bool y
+config OF_DMA + depends on HAS_DMA + def_bool y + endmenu # OF diff --git a/drivers/of/Makefile b/drivers/of/Makefile index e027f44..711ff5b 100644 --- a/drivers/of/Makefile +++ b/drivers/of/Makefile @@ -11,3 +11,4 @@ obj-$(CONFIG_OF_MDIO) += of_mdio.o obj-$(CONFIG_OF_PCI) += of_pci.o obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o obj-$(CONFIG_OF_MTD) += of_mtd.o +obj-$(CONFIG_OF_DMA) += of_dma.o diff --git a/drivers/of/of_dma.c b/drivers/of/of_dma.c new file mode 100644 index 0000000..80bd7aa --- /dev/null +++ b/drivers/of/of_dma.c @@ -0,0 +1,67 @@ +/* + * Copyright (c) 2012, NVIDIA CORPORATION. All rights reserved. + * + * Based on: + * "arch/microblaze/kernel/prom_parse.c" + * "arch/powerpc/kernel/prom_parse.c" + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + */ + +#include <linux/export.h> +#include <linux/of_address.h> + +int of_parse_dma_window(struct device_node *dn, + const char *propname, int index, + unsigned long *busno, + dma_addr_t *addr, size_t *size) +{ + const __be32 *dma_window, *end; + int bytes, cur_index = 0; + + if (!dn || !propname || !addr || !size) + return -EINVAL; + + dma_window = of_get_property(dn, propname, &bytes); + if (!dma_window) + return -ENODEV; + end = dma_window + bytes / sizeof(*dma_window); + + while (dma_window < end) { + u32 cells; + const void *prop; + + /* busno is always one cell */ + if (busno) + *busno = be32_to_cpup(dma_window++); + + prop = of_get_property(dn, "#dma-address-cells", NULL); + if (!prop) + prop = of_get_property(dn, "#address-cells", NULL); + + cells = prop ? be32_to_cpup(prop) : of_n_addr_cells(dn); + if (!cells) + return -EINVAL; + *addr = of_read_number(dma_window, cells); + dma_window += cells; + + prop = of_get_property(dn, "#dma-size-cells", NULL); + cells = prop ? be32_to_cpup(prop) : of_n_size_cells(dn); + if (!cells) + return -EINVAL; + *size = of_read_number(dma_window, cells); + dma_window += cells; + + if (cur_index++ == index) + break; + } + return 0; +} +EXPORT_SYMBOL(of_parse_dma_window); diff --git a/include/linux/of_address.h b/include/linux/of_address.h index 01b925a..1138cd3 100644 --- a/include/linux/of_address.h +++ b/include/linux/of_address.h @@ -21,6 +21,11 @@ extern void __iomem *of_iomap(struct device_node *device, int index); extern const u32 *of_get_address(struct device_node *dev, int index, u64 *size, unsigned int *flags);
+extern int of_parse_dma_window(struct device_node *dev, + const char *propname, int index, + unsigned long *busno, + dma_addr_t *phys, size_t *size); + #ifndef pci_address_to_pio static inline unsigned long pci_address_to_pio(phys_addr_t addr) { return -1; } #define pci_address_to_pio pci_address_to_pio @@ -48,6 +53,14 @@ static inline const u32 *of_get_address(struct device_node *dev, int index, { return NULL; } + +extern int of_parse_dma_window(struct device_node *dev, + const char *propname, int index, + unsigned long *busno, + dma_addr_t *phys, size_t *size) +{ + return 0; +} #endif /* CONFIG_OF_ADDRESS */
On 04/23/2012 05:53 AM, Hiroshi Doyu wrote:
From: Hiroshi DOYU hdoyu@nvidia.com
This code was based on: "arch/microblaze/kernel/prom_parse.c" "arch/powerpc/kernel/prom_parse.c"
"ibm," prefix could be supported with some modification.
That'd probably be a good idea. If it isn't supported, then this code can't be used as a replacement for the PPC and Microblaze code.
+int of_parse_dma_window(struct device_node *dn,
const char *propname, int index,unsigned long *busno,dma_addr_t *addr, size_t *size)+{
- const __be32 *dma_window, *end;
- int bytes, cur_index = 0;
- if (!dn || !propname || !addr || !size)
return -EINVAL;
Instead of erroring out if (!propname), perhaps:
if (!propname) propname = "dma-window";
so that only drivers using bindings before this common binding was defined actually need to write out the property name?
diff --git a/include/linux/of_address.h b/include/linux/of_address.h
...
+extern int of_parse_dma_window(struct device_node *dev,
const char *propname, int index,unsigned long *busno,dma_addr_t *phys, size_t *size);
This function has the same name as the PPC and Microblaze code it's based on. Is that going to cause compile and link errors due to duplicate symbol definitions?
linaro-mm-sig@lists.linaro.org
-
Arnd Bergmann -
Hiroshi Doyu -
Stephen Warren -
Thierry Reding