arm/dts: Tegra30: Add device tree support for SMMU
Subsystems (3)
PATCH 1/1 arm/dts: Tegra30: Add device tree support for SMMU
From: Hiroshi DOYU <[email protected]>
Add device tree support for Tegra30 IOMMU(SMMU).
--- .../devicetree/bindings/arm/tegra/tegra30-smmu.txt | 19 +++++++++++++++++++ arch/arm/boot/dts/tegra30.dtsi | 10 ++++++++++ 2 files changed, 29 insertions(+), 0 deletions(-)
Documentation/devicetree/bindings/arm/tegra/tegra30-smmu.txt new file +19 -0
@@ -0,0 +1,19 @@ +NVIDIA Tegra 30 IOMMU H/W, SMMU (System Memory Management Unit) + +Required properties: +- compatible : "nvidia,tegra30-smmu" +- reg : Should contain MC registers location and length +- reg : Should contain AHB Arbitration registers and length +- reg : Should contain virtual address space range +- interrupts : Should contain MC General interrupt + +Example: + smmu: smmu@7000f000 { + compatible = "nvidia,tegra30-smmu"; + reg = < 0x7000f000 0x400 /* controller registers */ + 0x6000c000 0x150 /* AHB Arbitration registers */ + 0x00001000 0x3ffff000 >;/* Virtual address space range + * Exclude the 1st & last page + */ + interrupts = < 0 13 0x40 >; + };
arch/arm/boot/dts/tegra30.dtsi +10 -0
@@ -183,4 +183,14 @@ reg = < 0x70000868 0xd0 /* Pad control registers */ 0x70003000 0x3e0 >; /* Mux registers */ }; + + smmu: smmu@7000f000 { + compatible = "nvidia,tegra30-smmu"; + reg = < 0x7000f000 0x400 /* controller registers */ + 0x6000c000 0x150 /* AHB Arbitration registers */ + 0x00001000 0x3ffff000 >;/* Virtual address space range + * Exclude the 1st & last page + */ + interrupts = < 0 13 0x40 >; + }; };
Signature · 1 line
1.7.5.4
A message that is not in the archive (sent off-list or not yet received): 20120413.132205.1766846402066131036.hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org
Quoted 44 lines · Hiroshi Doyu
From: Hiroshi DOYU <[email protected]>
Add device tree support for Tegra30 IOMMU(SMMU).
Signed-off-by Hiroshi DOYU <[email protected]>--- .../devicetree/bindings/arm/tegra/tegra30-smmu.txt | 19 +++++++++++++++++++ arch/arm/boot/dts/tegra30.dtsi | 10 ++++++++++ 2 files changed, 29 insertions(+), 0 deletions(-)Documentation/devicetree/bindings/arm/tegra/tegra30-smmu.txt new file +19
-0@@ -0,0 +1,19 @@ +NVIDIA Tegra 30 IOMMU H/W, SMMU (System Memory Management Unit) + +Required properties: +- compatible : "nvidia,tegra30-smmu" +- reg : Should contain MC registers location and length +- reg : Should contain AHB Arbitration registers and length +- reg : Should contain virtual address space range +- interrupts : Should contain MC General interrupt + +Example: + smmu: smmu@7000f000 { + compatible = "nvidia,tegra30-smmu"; + reg = < 0x7000f000 0x400 /* controller registers */ + 0x6000c000 0x150 /* AHB Arbitration registers */ + 0x00001000 0x3ffff000 >;/* Virtual address space range + * Exclude the 1st & last page + */ + interrupts = < 0 13 0x40 >; + };arch/arm/boot/dts/tegra30.dtsi +1
-0@@ -183,4 +183,14 @@ reg = < 0x70000868 0xd0 /* Pad control registers */ 0x70003000 0x3e0 >; /* Mux registers */ }; +
arch/arm/boot/dts/tegra30.dtsi line 196
Quoted 10 lines
arch/arm/boot/dts/tegra30.dtsi +9
-0+ smmu: smmu@7000f000 { + compatible = "nvidia,tegra30-smmu"; + reg = < 0x7000f000 0x400 /* controller registers */ + 0x6000c000 0x150 /* AHB Arbitration registers */ + 0x00001000 0x3ffff000 >;/* Virtual address space range + * Exclude the 1st & last page + */ + interrupts = < 0 13 0x40 >; + }; };
Why is the virtual address space range limited to 1 GiB? What is the reason for the exclusion of the first and last pages?
Thierry
_______________________________________________
iommu mailing list iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
A message that is not in the archive (sent off-list or not yet received): 20120413110331.GA11605-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/[email protected]
Hi Thierry,
From: Thierry Reding <[email protected]>
Subject: Re: [PATCH 1/1] arm/dts: Tegra30: Add device tree support for SMMU
Date: Fri, 13 Apr 2012 13:03:32 +0200
Message-ID: <20120413110331.GA11605-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/[email protected]>
Quoted 52 lines
* PGP Signed by an unknown key
Quoted 49 lines · Hiroshi Doyu
From: Hiroshi DOYU <[email protected]>
Add device tree support for Tegra30 IOMMU(SMMU).
Signed-off-by Hiroshi DOYU <[email protected]>--- .../devicetree/bindings/arm/tegra/tegra30-smmu.txt | 19 +++++++++++++++++++ arch/arm/boot/dts/tegra30.dtsi | 10 ++++++++++ 2 files changed, 29 insertions(+), 0 deletions(-)Documentation/devicetree/bindings/arm/tegra/tegra30-smmu.txt new file +19
-0@@ -0,0 +1,19 @@ +NVIDIA Tegra 30 IOMMU H/W, SMMU (System Memory Management Unit) + +Required properties: +- compatible : "nvidia,tegra30-smmu" +- reg : Should contain MC registers location and length +- reg : Should contain AHB Arbitration registers and length +- reg : Should contain virtual address space range +- interrupts : Should contain MC General interrupt + +Example: + smmu: smmu@7000f000 { + compatible = "nvidia,tegra30-smmu"; + reg = < 0x7000f000 0x400 /* controller registers */ + 0x6000c000 0x150 /* AHB Arbitration registers */ + 0x00001000 0x3ffff000 >;/* Virtual address space range + * Exclude the 1st & last page + */ + interrupts = < 0 13 0x40 >; + };arch/arm/boot/dts/tegra30.dtsi +6
-0@@ -183,4 +183,14 @@ reg = < 0x70000868 0xd0 /* Pad control registers */ 0x70003000 0x3e0 >; /* Mux registers */ }; + + smmu: smmu@7000f000 { + compatible = "nvidia,tegra30-smmu"; + reg = < 0x7000f000 0x400 /* controller registers */ + 0x6000c000 0x150 /* AHB Arbitration registers */ + 0x00001000 0x3ffff000 >;/* Virtual address space range
Quoted 8 lines
arch/arm/boot/dts/tegra30.dtsi line 196
Quoted 5 lines
arch/arm/boot/dts/tegra30.dtsi +4
-0+ * Exclude the 1st & last page + */ + interrupts = < 0 13 0x40 >; + }; };Why is the virtual address space range limited to 1 GiB? What is the reason for the exclusion of the first and last pages?
It's because physical RAM is located 2-4GB, and we may want to use those area 1-1(V==P) mapping in some cases. This could be extended with larger RAM without 1-1 mapping theoretically. So far 1GB seems to be enough.
The 1st page for AVP vector, and the last one is required by another H/W entity.
A message that is not in the archive (sent off-list or not yet received): 20120413.143104.1310879848248152784.hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org
Quoted 16 lines · Hiroshi Doyu
Quoted 14 lines · Thierry Reding
arch/arm/boot/dts/tegra30.dtsi line 196
Quoted 10 lines · Hiroshi Doyu
arch/arm/boot/dts/tegra30.dtsi +9
-0+ smmu: smmu@7000f000 { + compatible = "nvidia,tegra30-smmu"; + reg = < 0x7000f000 0x400 /* controller registers */ + 0x6000c000 0x150 /* AHB Arbitration registers */ + 0x00001000 0x3ffff000 >;/* Virtual address space range + * Exclude the 1st & last page + */ + interrupts = < 0 13 0x40 >; + }; };Why is the virtual address space range limited to 1 GiB? What is the reason for the exclusion of the first and last pages?
Quoted 4 lines
It's because physical RAM is located 2-4GB, and we may want to use those area 1-1(V==P) mapping in some cases. This could be extended with larger RAM without 1-1 mapping theoretically. So far 1GB seems to be enough.
I'm thinking that this would be better off in a separate property so that it's easier for boards to override it.
Quoted 2 lines
The 1st page for AVP vector, and the last one is required by another H/W entity.
I would expect such peculiarities to be handled by the driver internally. That way users wouldn't have to know or care about these kind of details. If it can't be handled by the driver then at least it should be mentioned explicitly in the binding documentation.
Thierry
A message that is not in the archive (sent off-list or not yet received): 20120413121510.GD11605-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/[email protected]
Quoted 17 lines · Thierry Reding
Quoted 16 lines · Hiroshi Doyu
Quoted 14 lines · Thierry Reding
Review
Quoted 10 lines · Hiroshi Doyu
(quoted diff) +9
-0+ smmu: smmu@7000f000 { + compatible = "nvidia,tegra30-smmu"; + reg = < 0x7000f000 0x400 /* controller registers */ + 0x6000c000 0x150 /* AHB Arbitration registers */ + 0x00001000 0x3ffff000 >;/* Virtual address space range + * Exclude the 1st & last page + */ + interrupts = < 0 13 0x40 >; + }; };Why is the virtual address space range limited to 1 GiB? What is the reason for the exclusion of the first and last pages?
Quoted 7 lines
Quoted 4 lines
It's because physical RAM is located 2-4GB, and we may want to use those area 1-1(V==P) mapping in some cases. This could be extended with larger RAM without 1-1 mapping theoretically. So far 1GB seems to be enough.
I'm thinking that this would be better off in a separate property so that it's easier for boards to override it.
Yes, and for another reason: The entries in the reg property are supposed to be within the bus address space that contains the device, whereas here this 3rd reg entry is being used to define something about an entirely unrelated address space - the virtual address space seen by SMMU clients.
Quoted 7 lines
Quoted 2 lines
The 1st page for AVP vector, and the last one is required by another H/W entity.
I would expect such peculiarities to be handled by the driver internally. That way users wouldn't have to know or care about these kind of details. If it can't be handled by the driver then at least it should be mentioned explicitly in the binding documentation.
I agree, but:
Why do those pages even need special handling? Doesn't the AVP get its own address space ID, and then can set it up however it wants?
A message that is not in the archive (sent off-list or not yet received): [email protected]
From: Stephen Warren <[email protected]>
Subject: Re: [PATCH 1/1] arm/dts: Tegra30: Add device tree support for SMMU
Date: Fri, 13 Apr 2012 21:25:20 +0200
Message-ID: <[email protected]>
Quoted 23 lines · Thierry Reding
Quoted 22 lines · Hiroshi Doyu
Quoted 20 lines
Quoted 14 lines · Thierry Reding
Review
Quoted 10 lines · Hiroshi Doyu
(quoted diff) +9
-0+ smmu: smmu@7000f000 { + compatible = "nvidia,tegra30-smmu"; + reg = < 0x7000f000 0x400 /* controller registers */ + 0x6000c000 0x150 /* AHB Arbitration registers */ + 0x00001000 0x3ffff000 >;/* Virtual address space range + * Exclude the 1st & last page + */ + interrupts = < 0 13 0x40 >; + }; };Why is the virtual address space range limited to 1 GiB? What is the reason for the exclusion of the first and last pages?
It's because physical RAM is located 2-4GB, and we may want to use those area 1-1(V==P) mapping in some cases. This could be extended with larger RAM without 1-1 mapping theoretically. So far 1GB seems to be enough.
Quoted 8 lines
Quoted 2 lines
I'm thinking that this would be better off in a separate property so that it's easier for boards to override it.
Yes, and for another reason: The entries in the reg property are supposed to be within the bus address space that contains the device, whereas here this 3rd reg entry is being used to define something about an entirely unrelated address space - the virtual address space seen by SMMU clients.
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 }
Quoted 8 lines
Quoted 7 lines
Quoted 2 lines
The 1st page for AVP vector, and the last one is required by another H/W entity.
I would expect such peculiarities to be handled by the driver internally. That way users wouldn't have to know or care about these kind of details. If it can't be handled by the driver then at least it should be mentioned explicitly in the binding documentation.
Quoted 4 lines
I agree, but:
Why do those pages even need special handling? Doesn't the AVP get its own address space ID, and then can set it up however it wants?
This sounds better than dealing with prefixed mappings at booting. At least we need to pass # of ASIDs, and client should claim one of them.
A message that is not in the archive (sent off-list or not yet received): 20120416.141238.381304581863200083.hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org
Quoted 17 lines · Hiroshi Doyu
What about using "dma-window" property to specify IOVA range in dtsi as below?
arch/powerpc/platforms/cell/iommu.c:698 static int
__initcell_iommu_get_window(structdevice_node*np,
699 unsigned long *base,
700 unsigned long *size)
701 {
702 const void*dma_window;
703 unsigned long index;
704705 /* 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;
Quoted 6 lines
711 }
712
713of_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.
Arnd
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 25 lines · On Mon, 16 Apr 2012 17:34:19 +0200 Arnd Bergmann…
Quoted 17 lines · Hiroshi Doyu
What about using "dma-window" property to specify IOVA range in dtsi as below?
arch/powerpc/platforms/cell/iommu.c:698 static int
__initcell_iommu_get_window(structdevice_node*np,
699 unsigned long *base,
700 unsigned long *size)
701 {
702 const void*dma_window;
703 unsigned long index;
704705 /* 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;Quoted 6 lines
711 }
712
713of_parse_dma_window(np, dma_window, &index, base, size);
714 return 0;
715 }
Quoted 4 lines
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.
A message that is not in the archive (sent off-list or not yet received): 20120418081746.995724e27bdc726049cd4965-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org
Quoted 28 lines · Hiroshi Doyu
Quoted 25 lines · On Mon, 16 Apr 2012 17:34:19 +0200 Arnd Bergmann…
Quoted 23 lines · Hiroshi Doyu
What about using "dma-window" property to specify IOVA range in dtsi as below?
arch/powerpc/platforms/cell/iommu.c:698 static int
__initcell_iommu_get_window(structdevice_node*np,
699 unsigned long *base,
700 unsigned long *size)
701 {
702 const void*dma_window;
703 unsigned long index;
704705 /* 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
713of_parse_dma_window(np, dma_window, &index, base, size);
714 return 0;
715 }
Quoted 7 lines
Quoted 4 lines
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.
Thierry
PATCH 1/1 arm/dts: Tegra30: Add device tree support for SMMU
From: Thierry Reding <[email protected]>
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: <[email protected]>
Quoted 36 lines
* PGP Signed by an unknown key
Quoted 33 lines · Hiroshi Doyu
Quoted 29 lines · On Mon, 16 Apr 2012 17:34:19 +0200 Arnd Bergmann <[email protected]> wrote:
Quoted 17 lines · Hiroshi Doyu
What about using "dma-window" property to specify IOVA range in dtsi as below?
arch/powerpc/platforms/cell/iommu.c:698 static int
__initcell_iommu_get_window(structdevice_node*np,
699 unsigned long *base,
700 unsigned long *size)
701 {
702 const void*dma_window;
703 unsigned long index;
704705 /* 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;Quoted 6 lines
711 }
712
713of_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.
Quoted 7 lines
Quoted 2 lines
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;)
PATCH 1/1 arm/dts: Tegra30: Add device tree support for SMMU
Quoted 6 lines · Hiroshi Doyu
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
PATCH 1/1 arm/dts: Tegra30: Add device tree support for SMMU
From: Arnd Bergmann <[email protected]>
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: <[email protected]>
Quoted 6 lines · Hiroshi Doyu
Quoted 5 lines
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"(?)
Quoted 6 lines
Quoted 1 line
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_windowfunction is currently only defined in powerpc specific code, and should get moved todrivers/offromarch/powerpc/kernel/prom_parse.c.
Something like below?
At least, I verified that this works with "tegra-smmu".
Quoted 1 line
From
67c1dd493637c9e972d04e061a8e67049687021aMon Sep 17 00:00:00 2001
From: Hiroshi DOYU <[email protected]>
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.
--- 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(-)
drivers/of/Kconfig +4 -0
@@ -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
drivers/of/Makefile +1 -0
@@ -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
drivers/of/of_dma.c new file +35 -0
@@ -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); +} +
include/linux/of_address.h +10 -0
@@ -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_piostatic 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 */
Signature · 1 line
1.7.5.4
PATCH 1/1 arm/dts: Tegra30: Add device tree support for SMMU
drivers/of/of_dma.c line 2
Quoted 8 lines · Hiroshi Doyu
drivers/of/of_dma.c new file +2
-0@@ -0,0 +1,35 @@ +/* +* Stealed from:
"Stolen from"
drivers/of/of_dma.c line 12
Quoted 10 lines
drivers/of/of_dma.c new file +10
-0+* "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.
drivers/of/of_dma.c line 20
Quoted 8 lines
drivers/of/of_dma.c new file +8
-0+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++);
drivers/of/of_dma.c line 26
Quoted 6 lines
drivers/of/of_dma.c new file +6
-0+ + 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.
drivers/of/of_dma.c line 32
Quoted 6 lines
drivers/of/of_dma.c new file +6
-0+ *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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120418/e02078d4/attachment.sig>
PATCH 1/1 dt: Add general DMA window parser
Hi,
Try again with fixing Thierry's commnets, Thanks.
Quoted 1 line
From
9a632c24949e46df197a216ca95f684edd1db693Mon Sep 17 00:00:00 2001
From: Hiroshi DOYU <[email protected]>
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.
--- 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(-)
drivers/of/Kconfig +4 -0
@@ -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
drivers/of/Makefile +1 -0
@@ -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
drivers/of/of_dma.c new file +35 -0
@@ -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); +} +
include/linux/of_address.h +10 -0
@@ -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_piostatic 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 */
Signature · 1 line
1.7.5.4
PATCH 1/1 dt: Add general DMA window parser
drivers/of/of_dma.c line 26
Quoted 1 line · Hiroshi Doyu
drivers/of/of_dma.c new file +1
-0+ 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.
drivers/of/of_dma.c line 32
Quoted 1 line
drivers/of/of_dma.c new file +1
-0+ cells = prop ? *(__be32 *)prop :of_n_size_cells (dn);
Similarly:
cells = prop ? be32_to_cpup(prop) : of_n_size_cells(dn);
Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120418/d8de2877/attachment.sig>
PATCH 1/1 dt: Add general DMA window parser
From: Thierry Reding <[email protected]>
Subject: Re: [PATCH 1/1] dt: Add general DMA window parser
Date: Wed, 18 Apr 2012 12:26:29 +0200
Message-ID: <[email protected]>
Quoted 10 lines
* PGP Signed by an unknown key
Review
Quoted 1 line · Hiroshi Doyu
(quoted diff) +1
-0+ 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.
Quoted 1 line
From
1ee8a9b3a839456b170af74b11a4304bfda965c4Mon Sep 17 00:00:00 2001
From: Hiroshi DOYU <[email protected]>
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.
--- 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(-)
drivers/of/Kconfig +4 -0
@@ -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
drivers/of/Makefile +1 -0
@@ -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
drivers/of/of_dma.c new file +35 -0
@@ -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); +} +
include/linux/of_address.h +10 -0
@@ -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_piostatic 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 */
Signature · 1 line
1.7.5.4
PATCH 1/1 dt: Add general DMA window parser
Quoted 10 lines · Hiroshi Doyu
drivers/of/of_dma.c new file +4
-0@@ -0,0 +1,35 @@ +/* +* Stolen from: +* "arch/microblaze/kernel/prom_parse.c" +* "arch/powerpc/kernel/prom_parse.c"
drivers/of/of_dma.c line 14
Quoted 10 lines
drivers/of/of_dma.c new file +10
-0+*/ + +#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.
drivers/of/of_dma.c line 27
Quoted 13 lines
drivers/of/of_dma.c new file +13
-0+ + 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.
drivers/of/of_dma.c line 33
Quoted 6 lines
drivers/of/of_dma.c new file +6
-0+ + 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.
drivers/of/of_dma.c line 35
Quoted 2 lines
drivers/of/of_dma.c new file +2
-0+} +
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120418/44c7e319/attachment-0001.sig>
PATCH 1/1 dt: Add general DMA window parser
From: Thierry Reding <[email protected]>
Subject: Re: [PATCH 1/1] dt: Add general DMA window parser
Date: Wed, 18 Apr 2012 12:54:23 +0200
Message-ID: <[email protected]>
Quoted 53 lines
* PGP Signed by an unknown key
Quoted 10 lines · Hiroshi Doyu
drivers/of/of_dma.c new file +4
-0@@ -0,0 +1,35 @@ +/* +* Stolen from: +* "arch/microblaze/kernel/prom_parse.c" +* "arch/powerpc/kernel/prom_parse.c" drivers/of/of_dma.c line 14
Quoted 10 lines
drivers/of/of_dma.c new file +10
-0+*/ + +#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.
drivers/of/of_dma.c line 27
Quoted 13 lines
drivers/of/of_dma.c new file +13
-0+ + 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.
drivers/of/of_dma.c line 33
Quoted 6 lines
drivers/of/of_dma.c new file +6
-0+ + 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.
Quoted 8 lines
drivers/of/of_dma.c line 35
Quoted 2 lines
drivers/of/of_dma.c new file +2
-0+} +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.
PATCH 1/1 dt: Add general DMA window parser
In reply to Hiroshi Doyuinclude/linux/of_address.h extern void __iomem *of_iomap(struct device_node *device, int index); · line 26
Quoted 12 lines · Hiroshi Doyu
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.
include/linux/of_address.h +3
-0+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);
PATCH 1/1 dt: Add general DMA window parser
Quoted 21 lines · Stephen Warren
include/linux/of_address.h extern void __iomem *of_iomap(struct device_node *device, int index); · line 26
Quoted 12 lines · Hiroshi Doyu
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.
include/linux/of_address.h +3
-0+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_phandleandof_get_named_gpio_flags. Should this new API be similar? E.g.:
Quoted 4 lines
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120418/fa4e567b/attachment.sig>
PATCH 1/1 dt: Add general DMA window parser
Quoted 24 lines · On Wed, 18 Apr 2012 21:39:45 +0200 Thierry Reding…
* PGP Signed by an unknown key
Quoted 21 lines · Stephen Warren
include/linux/of_address.h extern void __iomem *of_iomap(struct device_node *device, int index); · line 26
Quoted 12 lines · Hiroshi Doyu
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.
include/linux/of_address.h +3
-0+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_phandleandof_get_named_gpio_flags. Should this new API be similar? E.g.:
Quoted 6 lines
Quoted 4 lines
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.
PATCH 1/1 dt: Add general DMA window parser
In reply to Stephen WarrenQuoted 10 lines · On Wed, 18 Apr 2012 19:27:58 +0200 Stephen Warren <[email protected]> wrote:
Quoted 8 lines · Hiroshi Doyu
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.
Quoted 10 lines
include/linux/of_address.h extern void __iomem *of_iomap(struct device_node *device, int index); · line 26
Quoted 3 lines
include/linux/of_address.h +3
-0+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_phandleandof_get_named_gpio_flags. Should this new API be similar? E.g.:
Quoted 4 lines
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"?
PATCH v2 1/1 dt: Add general DMA window parser
From: Hiroshi DOYU <[email protected]>
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.
---
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(-)
drivers/of/Kconfig +4 -0
@@ -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
drivers/of/Makefile +1 -0
@@ -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
drivers/of/of_dma.c new file +67 -0
67 lines not on this page · raw message
include/linux/of_address.h +13 -0
@@ -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_piostatic 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 */
Signature · 1 line
1.7.5.4
PATCH v2 1/1 dt: Add general DMA window parser
Quoted 7 lines · Hiroshi Doyu
From: Hiroshi DOYU <[email protected]>
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.
drivers/of/of_dma.c line 30
Quoted 10 lines
drivers/of/of_dma.c new file +10
-0+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?
Quoted 1 line
include/linux/of_address.h +0
-0
...
include/linux/of_address.h extern void __iomem *of_iomap(struct device_node *device, int index); · line 27
Quoted 4 lines
include/linux/of_address.h +4
-0+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?
Quoted 5 lines · Hiroshi Doyu
From: Hiroshi DOYU <[email protected]>
Add device tree support for Tegra30 IOMMU(SMMU).
+++
b/Documentation/devicetree/bindings/arm/tegra/tegra30-smmu.txt
I personally like the documentation to be named after the full compatible value, so nvidia,tegra30-smmu.txt.
Documentation/devicetree/bindings/arm/tegra/tegra30-smmu.txt line 7
Quoted 8 lines
Documentation/devicetree/bindings/arm/tegra/tegra30-smmu.txt new file +7
-0@@ -0,0 +1,19 @@ +NVIDIA Tegra 30 IOMMU H/W, SMMU (System Memory Management Unit) + +Required properties: +- compatible : "nvidia,tegra30-smmu" +- reg : Should contain MC registers location and length +- reg : Should contain AHB Arbitration registers and length +- reg : Should contain virtual address space range
This looks like 3 properties with the same name. It seems common to write something more like:
reg : Should contain the register address and length for each fo the MC and AHB arbitration registers.
But why does the SMMU driver expect to control the AHB arbitration registers? They seem unrelated to the SMMU.
As I said in my previous email, I think the VA space size should be a separate property.
A message that is not in the archive (sent off-list or not yet received): [email protected]
From: Stephen Warren <[email protected]>
Subject: Re: [PATCH 1/1] arm/dts: Tegra30: Add device tree support for SMMU
Date: Fri, 13 Apr 2012 21:33:47 +0200
Message-ID: <[email protected]>
Quoted 9 lines · Hiroshi Doyu
Quoted 5 lines
From: Hiroshi DOYU <[email protected]>
Add device tree support for Tegra30 IOMMU(SMMU).
+++
b/Documentation/devicetree/bindings/arm/tegra/tegra30-smmu.txtI personally like the documentation to be named after the full compatible value, so nvidia,tegra30-smmu.txt.
OK
Quoted 12 lines
Documentation/devicetree/bindings/arm/tegra/tegra30-smmu.txt line 7
Quoted 8 lines
Documentation/devicetree/bindings/arm/tegra/tegra30-smmu.txt new file +7
-0@@ -0,0 +1,19 @@ +NVIDIA Tegra 30 IOMMU H/W, SMMU (System Memory Management Unit) + +Required properties: +- compatible : "nvidia,tegra30-smmu" +- reg : Should contain MC registers location and length +- reg : Should contain AHB Arbitration registers and length +- reg : Should contain virtual address space rangeThis looks like 3 properties with the same name. It seems common to write something more like:
Quoted 2 lines
reg : Should contain the register address and length for each fo the MC and AHB arbitration registers.
OK, this looks better.
Quoted 2 lines
But why does the SMMU driver expect to control the AHB arbitration registers? They seem unrelated to the SMMU.
It's necessary to inform AHB that SMMU is ready to use. There's a dedicated bit for SMMU in this AHB arbitration register.
Quoted 2 lines
As I said in my previous email, I think the VA space size should be a separate property.
OK, I'll update.
A message that is not in the archive (sent off-list or not yet received): 20120416.131059.331282986467061278.hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org
Quoted 3 lines · Hiroshi Doyu
Stephen Warren wrote at Fri, 13 Apr 2012 21:33:47 +0200:
Quoted 2 lines · Hiroshi Doyu
Quoted 1 line
Add device tree support for Tegra30 IOMMU(SMMU).
...
Quoted 5 lines
Quoted 2 lines
But why does the SMMU driver expect to control the AHB arbitration registers? They seem unrelated to the SMMU.
It's necessary to inform AHB that SMMU is ready to use. There's a dedicated bit for SMMU in this AHB arbitration register.
Shouldn't there be a dedicated driver for the AHB arbitration registers that the SMMU driver calls into to achieve this? IIRC, the AHB arbitration registers support much more than the SMMU enable, and if we ever need to touch those other features, going to the SMMU driver to do so probably wouldn't make sense.
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 8 lines · On Mon, 16 Apr 2012 18:07:06 +0200 Stephen Warren…
Quoted 3 lines · Hiroshi Doyu
Stephen Warren wrote at Fri, 13 Apr 2012 21:33:47 +0200:
Quoted 2 lines · Hiroshi Doyu
Quoted 1 line
Add device tree support for Tegra30 IOMMU(SMMU).
...
Quoted 3 lines
Quoted 2 lines
But why does the SMMU driver expect to control the AHB arbitration registers? They seem unrelated to the SMMU.
Quoted 8 lines
Quoted 2 lines
It's necessary to inform AHB that SMMU is ready to use. There's a dedicated bit for SMMU in this AHB arbitration register.
Shouldn't there be a dedicated driver for the AHB arbitration registers that the SMMU driver calls into to achieve this? IIRC, the AHB arbitration registers support much more than the SMMU enable, and if we ever need to touch those other features, going to the SMMU driver to do so probably wouldn't make sense.
Agree. Sounds like the way to go.
We haven't upstreamed AHB driver yet. So is it ok to remove AHB entry from SMMU dt and add AHB function call when upstreaming AHB driver? I'll work on that.
A message that is not in the archive (sent off-list or not yet received): 20120418081017.d3c630c6f7d09cd4ed039854-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org
Quoted 20 lines · Hiroshi Doyu
Quoted 8 lines · On Mon, 16 Apr 2012 18:07:06 +0200 Stephen Warren…
Quoted 3 lines · Hiroshi Doyu
Stephen Warren wrote at Fri, 13 Apr 2012 21:33:47 +0200:
Quoted 2 lines · Hiroshi Doyu
Quoted 1 line
Add device tree support for Tegra30 IOMMU(SMMU).
...
Quoted 3 lines
Quoted 2 lines
But why does the SMMU driver expect to control the AHB arbitration registers? They seem unrelated to the SMMU.
Quoted 8 lines
Quoted 2 lines
It's necessary to inform AHB that SMMU is ready to use. There's a dedicated bit for SMMU in this AHB arbitration register.
Shouldn't there be a dedicated driver for the AHB arbitration registers that the SMMU driver calls into to achieve this? IIRC, the AHB arbitration registers support much more than the SMMU enable, and if we ever need to touch those other features, going to the SMMU driver to do so probably wouldn't make sense.
Quoted 5 lines
Agree. Sounds like the way to go.
We haven't upstreamed AHB driver yet. So is it ok to remove AHB entry from SMMU dt and add AHB function call when upstreaming AHB driver? I'll work on that.
I think that seems reasonable for now, yes. I assume you mean implementing e.g. arch/arm/mach-tegra/tegra30-ahb.c, similar to how, say, the fuse APIs work.