Stephen Warren <[email protected]> wrote @ Wed, 20 Jun 2012 19:11:51 +0200:
> On 06/20/2012 01:16 AM, Hiroshi DOYU wrote:
> > From: Hiroshi Doyu <[email protected]>
> >
> > This code was based on:
> > "arch/microblaze/kernel/prom_parse.c"
> > "arch/powerpc/kernel/prom_parse.c"
> >
> > Can be promoted as a global function for general use to replace
> > "of_parse_dma_window()" in the above. This supports different formats
> > flexibly. "prefix" can be configured if any. "busno" and "index" are
> > optionally specified. Set NULL and 0 if not used.
> >
> > Signed-off-by: Hiroshi DOYU <[email protected]>
> > ---
> > Based on the discussion:
> > http://marc.info/?l=linux-tegra&m=133732046606458&w=2
>
> Hmmm. This function really should be in some common location and
> available for all drivers to use. Can't we add it to that common
> location from the start? What prevented the earlier patch that did this
> from getting merged into 3.5?
It's because there was no feedback against the original patches(the
common location ones, *1,*2) from DT side.
*1: http://article.gmane.org/gmane.linux.ports.tegra/4468
*2: https://lkml.org/lkml/2012/4/30/153
> One thing that might help here would be /not/ to add the common code to
> drivers/of/of_dma.c as was done in the earlier revisions of this patch -
> I believe that Grant has been trying to push subsystem-specific OF
> functionality into files in those individual subsystems, so that
> drivers/of can be kept for core support. Perhaps this patch should
> create drivers/iommu/of_iommu.c or similar?
"drivers/iommu/of_iommu.c" seems quite reasonable for this function.
> But I wonder: Is this function likely to be useful outside of
> drivers/iommu/ - you mentioned that similar code already exists in the
> two arch-specific prom_parse.c files; where are the existing users of
> those functions. If not in drivers/iommu/, then probably drivers/iommu/
> isn't a good place to put the new common function...
There are the following 3 users of of_parse_dma_window() as below. All
of them are IOMMU related, but they are architecture specific ones,
not for the standard IOMMU API. I guess that the current trend is to
convert Arch specific IOMMU API to the standard one basically.
arch/powerpc/kernel/vio.c of_parse_dma_window(dev->dev.of_node, dma_window,
arch/powerpc/platforms/cell/iommu.c of_parse_dma_window(np, dma_window, &index, base, size);
arch/powerpc/platforms/pseries/iommu.c of_parse_dma_window(dn, dma_window, &tbl->it_index, &offset, &size);
I think that the common "dma-window" DT parser is necessary for the
standard IOMMU because "dma-window" info is dealt as
DOMAIN_ATTR_GEOMETRY in the following Joerg's patch too.
[PATCH 0/5] IOMMU: Make IOMMU-API ready for GART-like hardware
https://lkml.org/lkml/2012/1/19/170
If it's ok to have of_get_dma_window() in "drivers/iommu/of_iommu.c",
I'll post that version.
Any comment?
On 06/21/2012 12:46 AM, Hiroshi Doyu wrote:
...
> There are the following 3 users of of_parse_dma_window() as below. All
> of them are IOMMU related, but they are architecture specific ones,
> not for the standard IOMMU API. I guess that the current trend is to
> convert Arch specific IOMMU API to the standard one basically.
>
> arch/powerpc/kernel/vio.c of_parse_dma_window(dev->dev.of_node, dma_window,
> arch/powerpc/platforms/cell/iommu.c of_parse_dma_window(np, dma_window, &index, base, size);
> arch/powerpc/platforms/pseries/iommu.c of_parse_dma_window(dn, dma_window, &tbl->it_index, &offset, &size);
>
> I think that the common "dma-window" DT parser is necessary for the
> standard IOMMU because "dma-window" info is dealt as
> DOMAIN_ATTR_GEOMETRY in the following Joerg's patch too.
>
> [PATCH 0/5] IOMMU: Make IOMMU-API ready for GART-like hardware
> https://lkml.org/lkml/2012/1/19/170
>
> If it's ok to have of_get_dma_window() in "drivers/iommu/of_iommu.c",
> I'll post that version.
Sounds sane to me.
From: Hiroshi DOYU <[email protected]>
This code was based on:
"arch/microblaze/kernel/prom_parse.c"
"arch/powerpc/kernel/prom_parse.c"
Can replace "of_parse_dma_window()" in the above. This supports
different formats flexibly. "prefix" can be configured if any. "busno"
and "index" are optionally specified. Set NULL and 0 if not used.
Signed-off-by: Hiroshi DOYU <[email protected]>
---
Update:
Move of_get_dma_window() in drivers/iommu/of_iommu.c
Based on the discussion:
http://marc.info/?l=linux-tegra&m=133732046606458&w=2
---
drivers/iommu/Kconfig | 4 ++
drivers/iommu/Makefile | 1 +
drivers/iommu/of_iommu.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++
include/linux/of_iommu.h | 21 +++++++++++
4 files changed, 116 insertions(+), 0 deletions(-)
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 3408937..4826af6 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -13,6 +13,10 @@ menuconfig IOMMU_SUPPORT
if IOMMU_SUPPORT
+config OF_IOMMU
+ def_bool y
+ depends on OF
+
# MSM IOMMU support
config MSM_IOMMU
bool "MSM IOMMU Support"
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 76e54ef..14a4d5f 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -1,4 +1,5 @@
obj-$(CONFIG_IOMMU_API) += iommu.o
+obj-$(CONFIG_OF_IOMMU) += of_iommu.o
obj-$(CONFIG_MSM_IOMMU) += msm_iommu.o msm_iommu_dev.o
obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o amd_iommu_init.o
obj-$(CONFIG_AMD_IOMMU_V2) += amd_iommu_v2.o
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
new file mode 100644
index 0000000..ee249bc
--- /dev/null
+++ b/drivers/iommu/of_iommu.c
@@ -0,0 +1,90 @@
+/*
+ * OF helpers for IOMMU
+ *
+ * Copyright (c) 2012, NVIDIA CORPORATION. All rights reserved.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <linux/export.h>
+#include <linux/limits.h>
+#include <linux/of.h>
+
+/**
+ * of_get_dma_window - Parse *dma-window property and returns 0 if found.
+ *
+ * @dn: device node
+ * @prefix: prefix for property name if any
+ * @index: index to start to parse
+ * @busno: Returns busno if supported. Otherwise pass NULL
+ * @addr: Returns address that DMA starts
+ * @size: Returns the range that DMA can handle
+ *
+ * This supports different formats flexibly. "prefix" can be
+ * configured if any. "busno" and "index" are optionally
+ * specified. Set 0(or NULL) if not used.
+ */
+int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
+ unsigned long *busno, dma_addr_t *addr, size_t *size)
+{
+ const __be32 *dma_window, *end;
+ int bytes, cur_index = 0;
+ char propname[NAME_MAX], addrname[NAME_MAX], sizename[NAME_MAX];
+
+ if (!dn || !addr || !size)
+ return -EINVAL;
+
+ if (!prefix)
+ prefix = "";
+
+ snprintf(propname, sizeof(propname), "%sdma-window", prefix);
+ snprintf(addrname, sizeof(addrname), "%s#dma-address-cells", prefix);
+ snprintf(sizename, sizeof(sizename), "%s#dma-size-cells", prefix);
+
+ 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 one cell if supported */
+ if (busno)
+ *busno = be32_to_cpup(dma_window++);
+
+ prop = of_get_property(dn, addrname, 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, sizename, 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_GPL(of_get_dma_window);
diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
new file mode 100644
index 0000000..51a560f
--- /dev/null
+++ b/include/linux/of_iommu.h
@@ -0,0 +1,21 @@
+#ifndef __OF_IOMMU_H
+#define __OF_IOMMU_H
+
+#ifdef CONFIG_OF_IOMMU
+
+extern int of_get_dma_window(struct device_node *dn, const char *prefix,
+ int index, unsigned long *busno, dma_addr_t *addr,
+ size_t *size);
+
+#else
+
+static inline int of_get_dma_window(struct device_node *dn, const char *prefix,
+ int index, unsigned long *busno, dma_addr_t *addr,
+ size_t *size)
+{
+ return -EINVAL;
+}
+
+#endif /* CONFIG_OF_IOMMU */
+
+#endif /* __OF_IOMMU_H */
--
1.7.5.4
On 06/21/2012 02:30 AM, Hiroshi Doyu wrote:
> From: Hiroshi DOYU <[email protected]>
>
> This code was based on:
> "arch/microblaze/kernel/prom_parse.c"
> "arch/powerpc/kernel/prom_parse.c"
>
> Can replace "of_parse_dma_window()" in the above. This supports
> different formats flexibly. "prefix" can be configured if any. "busno"
> and "index" are optionally specified. Set NULL and 0 if not used.
>
> Signed-off-by: Hiroshi DOYU <[email protected]>
Acked-by: Stephen Warren <[email protected]>
BTW, I only see patches 1 & 2 of 5 in this series; did you only mean to
post just 2 patches or did the others get lost on the way to my system
somehow?
On Thu, 21 Jun 2012 18:18:59 +0200
Stephen Warren <[email protected]> wrote:
> On 06/21/2012 02:30 AM, Hiroshi Doyu wrote:
> > From: Hiroshi DOYU <[email protected]>
> >
> > This code was based on:
> > "arch/microblaze/kernel/prom_parse.c"
> > "arch/powerpc/kernel/prom_parse.c"
> >
> > Can replace "of_parse_dma_window()" in the above. This supports
> > different formats flexibly. "prefix" can be configured if any. "busno"
> > and "index" are optionally specified. Set NULL and 0 if not used.
> >
> > Signed-off-by: Hiroshi DOYU <[email protected]>
>
> Acked-by: Stephen Warren <[email protected]>
Thanks.
> BTW, I only see patches 1 & 2 of 5 in this series; did you only mean to
> post just 2 patches or did the others get lost on the way to my system
> somehow?
I reposted only [1/5] and [2/5] as "v2" because the rests are
same. Probably it was better to send a whole series again. Sorry for
confusion.
On Mon, Jun 25, 2012 at 08:15:43AM +0300, Hiroshi Doyu wrote:
> On Thu, 21 Jun 2012 18:18:59 +0200
> Stephen Warren <[email protected]> wrote:
>
> > On 06/21/2012 02:30 AM, Hiroshi Doyu wrote:
> > > From: Hiroshi DOYU <[email protected]>
> > >
> > > This code was based on:
> > > "arch/microblaze/kernel/prom_parse.c"
> > > "arch/powerpc/kernel/prom_parse.c"
> > >
> > > Can replace "of_parse_dma_window()" in the above. This supports
> > > different formats flexibly. "prefix" can be configured if any. "busno"
> > > and "index" are optionally specified. Set NULL and 0 if not used.
> > >
> > > Signed-off-by: Hiroshi DOYU <[email protected]>
> >
> > Acked-by: Stephen Warren <[email protected]>
>
> Thanks.
>
> > BTW, I only see patches 1 & 2 of 5 in this series; did you only mean to
> > post just 2 patches or did the others get lost on the way to my system
> > somehow?
>
> I reposted only [1/5] and [2/5] as "v2" because the rests are
> same. Probably it was better to send a whole series again. Sorry for
> confusion.
Please repost the whole series with the Acks included as a new thread so
that I can easily apply them.
Joerg
--
AMD Operating System Research Center
Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632