[PATCH 0/7] Exynos: MFC driver: reserved memory cleanup and IOMMU support

[PATCH 0/7] Exynos: MFC driver: reserved memory cleanup and IOMMU support

[Marek Szyprowski]

Hello,

This patchset finally perform cleanup of custom code in s5p-mfc codec
driver. The first part is removal of custom, driver specific code for
intializing and handling of reserved memory. Instead, a generic code for
reserved memory regions is used. Then, once it is done, the proper setup
of DMA parameters (max segment size) is applied for all multimedia
devices found on Exynos SoCs to let them properly handle shared buffers
mapped into contiguous DMA address space. The last patch adds support
for IOMMU to MFC driver. Some additional code is needed because of
specific requirements of MFC device firmware (see patch 7 for more
details). When no IOMMU is available, the code fallbacks to generic
reserved memory regions.

After applying this patchset, MFC device works correctly when IOMMU is
either enabled or disabled.

Patches have been tested on top of linux-next from 20151207. I would
prefer to merge patches 1-2 via Samsung tree and patches 3-7 via media
tree (there are no compile-time dependencies between patches 1-2 and
3-7). Patches have been tested on Odroid U3 (Exynos 4412 based) and
Odroid XU3 (Exynos 5422 based) boards.

Best regards
Marek Szyprowski
Samsung R&D Institute Poland


Patch summary:

Marek Szyprowski (7):
  ARM: Exynos: convert MFC device to generic reserved memory bindings
  ARM: dts: exynos4412-odroid*: enable MFC device
  of: reserved_mem: add support for named reserved mem nodes
  media: vb2-dma-contig: add helper for setting dma max seg size
  media: set proper max seg size for devices on Exynos SoCs
  media: s5p-mfc: replace custom reserved memory init code with generic
    one
  media: s5p-mfc: add iommu support

 .../devicetree/bindings/media/s5p-mfc.txt          |  16 +--
 arch/arm/boot/dts/exynos4210-origen.dts            |  22 ++-
 arch/arm/boot/dts/exynos4210-smdkv310.dts          |  22 ++-
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi    |  24 ++++
 arch/arm/boot/dts/exynos4412-origen.dts            |  22 ++-
 arch/arm/boot/dts/exynos4412-smdk4412.dts          |  22 ++-
 arch/arm/boot/dts/exynos5250-arndale.dts           |  22 ++-
 arch/arm/boot/dts/exynos5250-smdk5250.dts          |  22 ++-
 arch/arm/boot/dts/exynos5250-spring.dts            |  22 ++-
 arch/arm/boot/dts/exynos5420-arndale-octa.dts      |  22 ++-
 arch/arm/boot/dts/exynos5420-smdk5420.dts          |  22 ++-
 arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi |  22 ++-
 arch/arm/mach-exynos/Makefile                      |   2 -
 arch/arm/mach-exynos/exynos.c                      |  19 ---
 arch/arm/mach-exynos/mfc.h                         |  16 ---
 arch/arm/mach-exynos/s5p-dev-mfc.c                 |  94 -------------
 drivers/media/platform/exynos-gsc/gsc-core.c       |   1 +
 drivers/media/platform/exynos4-is/fimc-core.c      |   1 +
 drivers/media/platform/exynos4-is/fimc-is.c        |   1 +
 drivers/media/platform/exynos4-is/fimc-lite.c      |   1 +
 drivers/media/platform/s5p-g2d/g2d.c               |   1 +
 drivers/media/platform/s5p-jpeg/jpeg-core.c        |   1 +
 drivers/media/platform/s5p-mfc/s5p_mfc.c           | 153 ++++++++++++---------
 drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h     |  79 +++++++++++
 drivers/media/platform/s5p-tv/mixer_video.c        |   1 +
 drivers/media/v4l2-core/videobuf2-dma-contig.c     |  15 ++
 drivers/of/of_reserved_mem.c                       | 101 +++++++++++---
 include/linux/of_reserved_mem.h                    |   6 +-
 include/media/videobuf2-dma-contig.h               |   1 +
 29 files changed, 505 insertions(+), 248 deletions(-)
 delete mode 100644 arch/arm/mach-exynos/mfc.h
 delete mode 100644 arch/arm/mach-exynos/s5p-dev-mfc.c
 create mode 100644 drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h

-- 
1.9.2

[PATCH 1/7] ARM: Exynos: convert MFC device to generic reserved memory bindings

[Marek Szyprowski]

This patch replaces custom properties for definining reserved memory
regions with generic reserved memory bindings. All custom code for
handling MFC-specific reserved memory can be now removed from Exynos-DT
generic board code.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 .../devicetree/bindings/media/s5p-mfc.txt          | 16 ++--
 arch/arm/boot/dts/exynos4210-origen.dts            | 22 ++++-
 arch/arm/boot/dts/exynos4210-smdkv310.dts          | 22 ++++-
 arch/arm/boot/dts/exynos4412-origen.dts            | 22 ++++-
 arch/arm/boot/dts/exynos4412-smdk4412.dts          | 22 ++++-
 arch/arm/boot/dts/exynos5250-arndale.dts           | 22 ++++-
 arch/arm/boot/dts/exynos5250-smdk5250.dts          | 22 ++++-
 arch/arm/boot/dts/exynos5250-spring.dts            | 22 ++++-
 arch/arm/boot/dts/exynos5420-arndale-octa.dts      | 22 ++++-
 arch/arm/boot/dts/exynos5420-smdk5420.dts          | 22 ++++-
 arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 22 ++++-
 arch/arm/mach-exynos/Makefile                      |  2 -
 arch/arm/mach-exynos/exynos.c                      | 19 -----
 arch/arm/mach-exynos/mfc.h                         | 16 ----
 arch/arm/mach-exynos/s5p-dev-mfc.c                 | 94 ----------------------
 15 files changed, 208 insertions(+), 159 deletions(-)
 delete mode 100644 arch/arm/mach-exynos/mfc.h
 delete mode 100644 arch/arm/mach-exynos/s5p-dev-mfc.c

diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt b/Documentation/devicetree/bindings/media/s5p-mfc.txt
index 2d5787eac91a..4603673c593b 100644
--- a/Documentation/devicetree/bindings/media/s5p-mfc.txt
+++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt
@@ -21,16 +21,16 @@ Required properties:
   - clock-names : from common clock binding: must contain "mfc",
 		  corresponding to entry in the clocks property.
 
-  - samsung,mfc-r : Base address of the first memory bank used by MFC
-		    for DMA contiguous memory allocation and its size.
-
-  - samsung,mfc-l : Base address of the second memory bank used by MFC
-		    for DMA contiguous memory allocation and its size.
-
 Optional properties:
   - power-domains : power-domain property defined with a phandle
 			   to respective power domain.
 
+  - memory-region : from reserved memory binding: phandles to two reserved
+	memory regions: accessed by "left" and "right" mfc memory bus
+	interfaces, used when no SYSMMU support is available
+  - memory-region-names : from reserved memory binding: must be "left"
+	and "right"
+
 Example:
 SoC specific DT entry:
 
@@ -46,6 +46,6 @@ mfc: codec@13400000 {
 Board specific DT entry:
 
 codec@13400000 {
-	samsung,mfc-r = <0x43000000 0x800000>;
-	samsung,mfc-l = <0x51000000 0x800000>;
+	memory-region = <&mfc_left>, <&mfc_right>;
+	memory-region-names = "left", "right";
 };
diff --git a/arch/arm/boot/dts/exynos4210-origen.dts b/arch/arm/boot/dts/exynos4210-origen.dts
index 5821ad87e32c..4b7637dfa392 100644
--- a/arch/arm/boot/dts/exynos4210-origen.dts
+++ b/arch/arm/boot/dts/exynos4210-origen.dts
@@ -30,6 +30,24 @@
 		       0x70000000 0x10000000>;
 	};
 
+	reserved-memory {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		mfc_left: region@51000000 {
+			compatible = "shared-dma-pool";
+			no-map;
+			reg = <0x51000000 0x800000>;
+		};
+
+		mfc_right: region@43000000 {
+			compatible = "shared-dma-pool";
+			no-map;
+			reg = <0x43000000 0x800000>;
+		};
+	};
+
 	chosen {
 		bootargs ="root=/dev/ram0 rw ramdisk=8192 initrd=0x41000000,8M console=ttySAC2,115200 init=/linuxrc";
 		stdout-path = &serial_2;
@@ -288,8 +306,8 @@
 };
 
 &mfc {
-	samsung,mfc-r = <0x43000000 0x800000>;
-	samsung,mfc-l = <0x51000000 0x800000>;
+	memory-region = <&mfc_left>, <&mfc_right>;
+	memory-region-names = "left", "right";
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/exynos4210-smdkv310.dts b/arch/arm/boot/dts/exynos4210-smdkv310.dts
index 104cbb33d2bb..efafc5721817 100644
--- a/arch/arm/boot/dts/exynos4210-smdkv310.dts
+++ b/arch/arm/boot/dts/exynos4210-smdkv310.dts
@@ -26,6 +26,24 @@
 		reg = <0x40000000 0x80000000>;
 	};
 
+	reserved-memory {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		mfc_left: region@51000000 {
+			compatible = "shared-dma-pool";
+			no-map;
+			reg = <0x51000000 0x800000>;
+		};
+
+		mfc_right: region@43000000 {
+			compatible = "shared-dma-pool";
+			no-map;
+			reg = <0x43000000 0x800000>;
+		};
+	};
+
 	chosen {
 		bootargs = "root=/dev/ram0 rw ramdisk=8192 initrd=0x41000000,8M console=ttySAC1,115200 init=/linuxrc";
 		stdout-path = &serial_1;
@@ -133,8 +151,8 @@
 };
 
 &mfc {
-	samsung,mfc-r = <0x43000000 0x800000>;
-	samsung,mfc-l = <0x51000000 0x800000>;
+	memory-region = <&mfc_left>, <&mfc_right>;
+	memory-region-names = "left", "right";
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/exynos4412-origen.dts b/arch/arm/boot/dts/exynos4412-origen.dts
index 9e2e24c6177a..fb9291e371bc 100644
--- a/arch/arm/boot/dts/exynos4412-origen.dts
+++ b/arch/arm/boot/dts/exynos4412-origen.dts
@@ -25,6 +25,24 @@
 		reg = <0x40000000 0x40000000>;
 	};
 
+	reserved-memory {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		mfc_left: region@51000000 {
+			compatible = "shared-dma-pool";
+			no-map;
+			reg = <0x51000000 0x800000>;
+		};
+
+		mfc_right: region@43000000 {
+			compatible = "shared-dma-pool";
+			no-map;
+			reg = <0x43000000 0x800000>;
+		};
+	};
+
 	chosen {
 		bootargs ="console=ttySAC2,115200";
 		stdout-path = &serial_2;
@@ -466,8 +484,8 @@
 };
 
 &mfc {
-	samsung,mfc-r = <0x43000000 0x800000>;
-	samsung,mfc-l = <0x51000000 0x800000>;
+	memory-region = <&mfc_left>, <&mfc_right>;
+	memory-region-names = "left", "right";
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/exynos4412-smdk4412.dts b/arch/arm/boot/dts/exynos4412-smdk4412.dts
index a130ab39fa77..4df182b52665 100644
--- a/arch/arm/boot/dts/exynos4412-smdk4412.dts
+++ b/arch/arm/boot/dts/exynos4412-smdk4412.dts
@@ -23,6 +23,24 @@
 		reg = <0x40000000 0x40000000>;
 	};
 
+	reserved-memory {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		mfc_left: region@51000000 {
+			compatible = "shared-dma-pool";
+			no-map;
+			reg = <0x51000000 0x800000>;
+		};
+
+		mfc_right: region@43000000 {
+			compatible = "shared-dma-pool";
+			no-map;
+			reg = <0x43000000 0x800000>;
+		};
+	};
+
 	chosen {
 		bootargs ="root=/dev/ram0 rw ramdisk=8192 initrd=0x41000000,8M console=ttySAC1,115200 init=/linuxrc";
 		stdout-path = &serial_1;
@@ -112,8 +130,8 @@
 };
 
 &mfc {
-	samsung,mfc-r = <0x43000000 0x800000>;
-	samsung,mfc-l = <0x51000000 0x800000>;
+	memory-region = <&mfc_left>, <&mfc_right>;
+	memory-region-names = "left", "right";
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/exynos5250-arndale.dts b/arch/arm/boot/dts/exynos5250-arndale.dts
index c000532c1444..30ada98cd078 100644
--- a/arch/arm/boot/dts/exynos5250-arndale.dts
+++ b/arch/arm/boot/dts/exynos5250-arndale.dts
@@ -23,6 +23,24 @@
 		reg = <0x40000000 0x80000000>;
 	};
 
+	reserved-memory {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		mfc_left: region@51000000 {
+			compatible = "shared-dma-pool";
+			no-map;
+			reg = <0x51000000 0x800000>;
+		};
+
+		mfc_right: region@43000000 {
+			compatible = "shared-dma-pool";
+			no-map;
+			reg = <0x43000000 0x800000>;
+		};
+	};
+
 	chosen {
 		bootargs = "console=ttySAC2,115200";
 	};
@@ -518,8 +536,8 @@
 };
 
 &mfc {
-	samsung,mfc-r = <0x43000000 0x800000>;
-	samsung,mfc-l = <0x51000000 0x800000>;
+	memory-region = <&mfc_left>, <&mfc_right>;
+	memory-region-names = "left", "right";
 };
 
 &mmc_0 {
diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts b/arch/arm/boot/dts/exynos5250-smdk5250.dts
index 0f5dcd418af8..88fe16dda188 100644
--- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
+++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
@@ -25,6 +25,24 @@
 		reg = <0x40000000 0x80000000>;
 	};
 
+	reserved-memory {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		mfc_left: region@51000000 {
+			compatible = "shared-dma-pool";
+			no-map;
+			reg = <0x51000000 0x800000>;
+		};
+
+		mfc_right: region@43000000 {
+			compatible = "shared-dma-pool";
+			no-map;
+			reg = <0x43000000 0x800000>;
+		};
+	};
+
 	chosen {
 		bootargs = "root=/dev/ram0 rw ramdisk=8192 initrd=0x41000000,8M console=ttySAC2,115200 init=/linuxrc";
 	};
@@ -346,8 +364,8 @@
 };
 
 &mfc {
-	samsung,mfc-r = <0x43000000 0x800000>;
-	samsung,mfc-l = <0x51000000 0x800000>;
+	memory-region = <&mfc_left>, <&mfc_right>;
+	memory-region-names = "left", "right";
 };
 
 &mmc_0 {
diff --git a/arch/arm/boot/dts/exynos5250-spring.dts b/arch/arm/boot/dts/exynos5250-spring.dts
index c1edd6d038a9..607d133ac1a9 100644
--- a/arch/arm/boot/dts/exynos5250-spring.dts
+++ b/arch/arm/boot/dts/exynos5250-spring.dts
@@ -23,6 +23,24 @@
 		reg = <0x40000000 0x80000000>;
 	};
 
+	reserved-memory {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		mfc_left: region@51000000 {
+			compatible = "shared-dma-pool";
+			no-map;
+			reg = <0x51000000 0x800000>;
+		};
+
+		mfc_right: region@43000000 {
+			compatible = "shared-dma-pool";
+			no-map;
+			reg = <0x43000000 0x800000>;
+		};
+	};
+
 	chosen {
 		bootargs = "console=tty1";
 		stdout-path = "serial3:115200n8";
@@ -427,8 +445,8 @@
 };
 
 &mfc {
-	samsung,mfc-r = <0x43000000 0x800000>;
-	samsung,mfc-l = <0x51000000 0x800000>;
+	memory-region = <&mfc_left>, <&mfc_right>;
+	memory-region-names = "left", "right";
 };
 
 &mmc_0 {
diff --git a/arch/arm/boot/dts/exynos5420-arndale-octa.dts b/arch/arm/boot/dts/exynos5420-arndale-octa.dts
index 4ecef6981d5c..cdaf49a1bd5b 100644
--- a/arch/arm/boot/dts/exynos5420-arndale-octa.dts
+++ b/arch/arm/boot/dts/exynos5420-arndale-octa.dts
@@ -24,6 +24,24 @@
 		reg = <0x20000000 0x80000000>;
 	};
 
+	reserved-memory {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		mfc_left: region@51000000 {
+			compatible = "shared-dma-pool";
+			no-map;
+			reg = <0x51000000 0x800000>;
+		};
+
+		mfc_right: region@43000000 {
+			compatible = "shared-dma-pool";
+			no-map;
+			reg = <0x43000000 0x800000>;
+		};
+	};
+
 	chosen {
 		bootargs = "console=ttySAC3,115200";
 	};
@@ -345,8 +363,8 @@
 };
 
 &mfc {
-	samsung,mfc-r = <0x43000000 0x800000>;
-	samsung,mfc-l = <0x51000000 0x800000>;
+	memory-region = <&mfc_left>, <&mfc_right>;
+	memory-region-names = "left", "right";
 };
 
 &mmc_0 {
diff --git a/arch/arm/boot/dts/exynos5420-smdk5420.dts b/arch/arm/boot/dts/exynos5420-smdk5420.dts
index ac35aefd320f..159e6d9ae0f8 100644
--- a/arch/arm/boot/dts/exynos5420-smdk5420.dts
+++ b/arch/arm/boot/dts/exynos5420-smdk5420.dts
@@ -21,6 +21,24 @@
 		reg = <0x20000000 0x80000000>;
 	};
 
+	reserved-memory {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		mfc_left: region@51000000 {
+			compatible = "shared-dma-pool";
+			no-map;
+			reg = <0x51000000 0x800000>;
+		};
+
+		mfc_right: region@43000000 {
+			compatible = "shared-dma-pool";
+			no-map;
+			reg = <0x43000000 0x800000>;
+		};
+	};
+
 	chosen {
 		bootargs = "console=ttySAC2,115200 init=/linuxrc";
 	};
@@ -355,8 +373,8 @@
 };
 
 &mfc {
-	samsung,mfc-r = <0x43000000 0x800000>;
-	samsung,mfc-l = <0x51000000 0x800000>;
+	memory-region = <&mfc_left>, <&mfc_right>;
+	memory-region-names = "left", "right";
 };
 
 &mmc_0 {
diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
index 9134217446b8..864cf1180129 100644
--- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
+++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
@@ -23,6 +23,24 @@
 		reg = <0x40000000 0x7EA00000>;
 	};
 
+	reserved-memory {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		mfc_left: region@51000000 {
+			compatible = "shared-dma-pool";
+			no-map;
+			reg = <0x51000000 0x800000>;
+		};
+
+		mfc_right: region@43000000 {
+			compatible = "shared-dma-pool";
+			no-map;
+			reg = <0x43000000 0x800000>;
+		};
+	};
+
 	chosen {
 		linux,stdout-path = &serial_2;
 	};
@@ -319,8 +337,8 @@
 };
 
 &mfc {
-	samsung,mfc-r = <0x43000000 0x800000>;
-	samsung,mfc-l = <0x51000000 0x800000>;
+	memory-region = <&mfc_left>, <&mfc_right>;
+	memory-region-names = "left", "right";
 };
 
 &mmc_0 {
diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
index 2f306767cdfe..bcefb5473ee4 100644
--- a/arch/arm/mach-exynos/Makefile
+++ b/arch/arm/mach-exynos/Makefile
@@ -23,5 +23,3 @@ AFLAGS_sleep.o			:=-Wa,-march=armv7-a$(plus_sec)
 
 obj-$(CONFIG_EXYNOS5420_MCPM)	+= mcpm-exynos.o
 CFLAGS_mcpm-exynos.o		+= -march=armv7-a
-
-obj-$(CONFIG_S5P_DEV_MFC)	+= s5p-dev-mfc.o
diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index 1c47aee31e9c..662f329e37cd 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -30,7 +30,6 @@
 #include <mach/map.h>
 
 #include "common.h"
-#include "mfc.h"
 #include "regs-pmu.h"
 
 void __iomem *pmu_base_addr;
@@ -290,23 +289,6 @@ static char const *const exynos_dt_compat[] __initconst = {
 	NULL
 };
 
-static void __init exynos_reserve(void)
-{
-#ifdef CONFIG_S5P_DEV_MFC
-	int i;
-	char *mfc_mem[] = {
-		"samsung,mfc-v5",
-		"samsung,mfc-v6",
-		"samsung,mfc-v7",
-		"samsung,mfc-v8",
-	};
-
-	for (i = 0; i < ARRAY_SIZE(mfc_mem); i++)
-		if (of_scan_flat_dt(s5p_fdt_alloc_mfc_mem, mfc_mem[i]))
-			break;
-#endif
-}
-
 static void __init exynos_dt_fixup(void)
 {
 	/*
@@ -328,6 +310,5 @@ DT_MACHINE_START(EXYNOS_DT, "SAMSUNG EXYNOS (Flattened Device Tree)")
 	.init_machine	= exynos_dt_machine_init,
 	.init_late	= exynos_init_late,
 	.dt_compat	= exynos_dt_compat,
-	.reserve	= exynos_reserve,
 	.dt_fixup	= exynos_dt_fixup,
 MACHINE_END
diff --git a/arch/arm/mach-exynos/mfc.h b/arch/arm/mach-exynos/mfc.h
deleted file mode 100644
index dec93cd5b3c6..000000000000
--- a/arch/arm/mach-exynos/mfc.h
+++ /dev/null
@@ -1,16 +0,0 @@
-/*
- * Copyright (C) 2013 Samsung Electronics Co.Ltd
- *
- * This program is free software; you can redistribute  it and/or modify it
- * under  the terms of  the GNU General  Public License as published by the
- * Free Software Foundation;  either version 2 of the  License, or (at your
- * option) any later version.
- */
-
-#ifndef __MACH_EXYNOS_MFC_H
-#define __MACH_EXYNOS_MFC_H __FILE__
-
-int __init s5p_fdt_alloc_mfc_mem(unsigned long node, const char *uname,
-				int depth, void *data);
-
-#endif /* __MACH_EXYNOS_MFC_H */
diff --git a/arch/arm/mach-exynos/s5p-dev-mfc.c b/arch/arm/mach-exynos/s5p-dev-mfc.c
deleted file mode 100644
index 0b04b6b0fa30..000000000000
--- a/arch/arm/mach-exynos/s5p-dev-mfc.c
+++ /dev/null
@@ -1,94 +0,0 @@
-/*
- * Copyright (C) 2010-2011 Samsung Electronics Co.Ltd
- *
- * Base S5P MFC resource and device definitions
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-
-#include <linux/kernel.h>
-#include <linux/interrupt.h>
-#include <linux/platform_device.h>
-#include <linux/dma-mapping.h>
-#include <linux/memblock.h>
-#include <linux/ioport.h>
-#include <linux/of_fdt.h>
-#include <linux/of.h>
-
-static struct platform_device s5p_device_mfc_l;
-static struct platform_device s5p_device_mfc_r;
-
-struct s5p_mfc_dt_meminfo {
-	unsigned long	loff;
-	unsigned long	lsize;
-	unsigned long	roff;
-	unsigned long	rsize;
-	char		*compatible;
-};
-
-struct s5p_mfc_reserved_mem {
-	phys_addr_t	base;
-	unsigned long	size;
-	struct device	*dev;
-};
-
-static struct s5p_mfc_reserved_mem s5p_mfc_mem[2] __initdata;
-
-
-static void __init s5p_mfc_reserve_mem(phys_addr_t rbase, unsigned int rsize,
-				phys_addr_t lbase, unsigned int lsize)
-{
-	int i;
-
-	s5p_mfc_mem[0].dev = &s5p_device_mfc_r.dev;
-	s5p_mfc_mem[0].base = rbase;
-	s5p_mfc_mem[0].size = rsize;
-
-	s5p_mfc_mem[1].dev = &s5p_device_mfc_l.dev;
-	s5p_mfc_mem[1].base = lbase;
-	s5p_mfc_mem[1].size = lsize;
-
-	for (i = 0; i < ARRAY_SIZE(s5p_mfc_mem); i++) {
-		struct s5p_mfc_reserved_mem *area = &s5p_mfc_mem[i];
-		if (memblock_remove(area->base, area->size)) {
-			printk(KERN_ERR "Failed to reserve memory for MFC device (%ld bytes at 0x%08lx)\n",
-			       area->size, (unsigned long) area->base);
-			area->base = 0;
-		}
-	}
-}
-
-int __init s5p_fdt_alloc_mfc_mem(unsigned long node, const char *uname,
-				int depth, void *data)
-{
-	const __be32 *prop;
-	int len;
-	struct s5p_mfc_dt_meminfo mfc_mem;
-
-	if (!data)
-		return 0;
-
-	if (!of_flat_dt_is_compatible(node, data))
-		return 0;
-
-	prop = of_get_flat_dt_prop(node, "samsung,mfc-l", &len);
-	if (!prop || (len != 2 * sizeof(unsigned long)))
-		return 0;
-
-	mfc_mem.loff = be32_to_cpu(prop[0]);
-	mfc_mem.lsize = be32_to_cpu(prop[1]);
-
-	prop = of_get_flat_dt_prop(node, "samsung,mfc-r", &len);
-	if (!prop || (len != 2 * sizeof(unsigned long)))
-		return 0;
-
-	mfc_mem.roff = be32_to_cpu(prop[0]);
-	mfc_mem.rsize = be32_to_cpu(prop[1]);
-
-	s5p_mfc_reserve_mem(mfc_mem.roff, mfc_mem.rsize,
-			mfc_mem.loff, mfc_mem.lsize);
-
-	return 1;
-}
-- 
1.9.2

[PATCH 2/7] ARM: dts: exynos4412-odroid*: enable MFC device

[Marek Szyprowski]

Enable support for Multimedia Codec (MFC) device for all Exynos4412-based
Odroid boards.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
index 395c3ca9601e..90b952e29ebf 100644
--- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
+++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
@@ -18,6 +18,24 @@
 		stdout-path = &serial_1;
 	};
 
+	reserved-memory {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		mfc_left: region@77000000 {
+			compatible = "shared-dma-pool";
+			reusable;
+			reg = <0x77000000 0x1000000>;
+		};
+
+		mfc_right: region@78000000 {
+			compatible = "shared-dma-pool";
+			reusable;
+			reg = <0x78000000 0x1000000>;
+		};
+	};
+
 	firmware@0204F000 {
 		compatible = "samsung,secure-firmware";
 		reg = <0x0204F000 0x1000>;
@@ -447,6 +465,12 @@
 	clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
 };
 
+&mfc {
+	memory-region = <&mfc_left>, <&mfc_right>;
+	memory-region-names = "left", "right";
+	status = "okay";
+};
+
 &mixer {
 	status = "okay";
 };
-- 
1.9.2

[PATCH 3/7] of: reserved_mem: add support for named reserved mem nodes

[Marek Szyprowski]

This patch allows device drivers to use more than one reserved memory
region assigned to given device. When NULL name is passed to
of_reserved_mem_device_init(), the default (first) region is used.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/of/of_reserved_mem.c    | 101 +++++++++++++++++++++++++++++++---------
 include/linux/of_reserved_mem.h |   6 ++-
 2 files changed, 84 insertions(+), 23 deletions(-)

diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 1a3556a9e9ea..0a0b23b73004 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -21,6 +21,7 @@
 #include <linux/sizes.h>
 #include <linux/of_reserved_mem.h>
 #include <linux/sort.h>
+#include <linux/slab.h>
 
 #define MAX_RESERVED_REGIONS	16
 static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS];
@@ -287,31 +288,84 @@ static inline struct reserved_mem *__find_rmem(struct device_node *node)
 	return NULL;
 }
 
+static struct reserved_mem *__node_to_rmem(struct device_node *node,
+					   const char *name)
+{
+	struct reserved_mem *rmem;
+	struct device_node *target;
+	int idx = 0;
+
+	if (!node)
+		return NULL;
+
+	if (name) {
+		idx = of_property_match_string(node,
+					       "memory-region-names", name);
+		if (idx < 0)
+			return NULL;
+	}
+
+	target = of_parse_phandle(node, "memory-region", idx);
+	if (!target)
+		return NULL;
+	rmem = __find_rmem(target);
+	of_node_put(target);
+
+	return rmem;
+}
+
+struct rmem_assigned_device {
+	struct device *dev;
+	struct reserved_mem *rmem;
+	struct list_head list;
+};
+
+static LIST_HEAD(of_rmem_assigned_device_list);
+static DEFINE_MUTEX(of_rmem_assigned_device_mutex);
+
 /**
  * of_reserved_mem_device_init() - assign reserved memory region to given device
+ * @dev:	Pointer to the device to configure
+ * @np:		Pointer to the device_node with 'reserved-memory' property
+ * @name:	Optional name of the selected region (can be NULL)
+ *
+ * This function assigns respective DMA-mapping operations based on reserved
+ * memory regionspecified by 'memory-region' property in @np node, named @name
+ * to the @dev device. When NULL name is provided, the default (first) memory
+ * region is used. When driver needs to use more than one reserved memory
+ * region, it should allocate child devices and initialize regions by name for
+ * each of child device.
  *
- * This function assign memory region pointed by "memory-region" device tree
- * property to the given device.
+ * Returns error code or zero on success.
  */
-int of_reserved_mem_device_init(struct device *dev)
+int of_reserved_mem_device_init(struct device *dev, struct device_node *np,
+				const char *name)
 {
+	struct rmem_assigned_device *rd;
 	struct reserved_mem *rmem;
-	struct device_node *np;
 	int ret;
 
-	np = of_parse_phandle(dev->of_node, "memory-region", 0);
-	if (!np)
-		return -ENODEV;
-
-	rmem = __find_rmem(np);
-	of_node_put(np);
-
+	rmem = __node_to_rmem(np, name);
 	if (!rmem || !rmem->ops || !rmem->ops->device_init)
 		return -EINVAL;
 
+	rd = kmalloc(sizeof(struct rmem_assigned_device), GFP_KERNEL);
+	if (!rd)
+		return -ENOMEM;
+
 	ret = rmem->ops->device_init(rmem, dev);
-	if (ret == 0)
+	if (ret == 0) {
+		rd->dev = dev;
+		rd->rmem = rmem;
+
+		mutex_lock(&of_rmem_assigned_device_mutex);
+		list_add(&rd->list, &of_rmem_assigned_device_list);
+		mutex_unlock(&of_rmem_assigned_device_mutex);
+
 		dev_info(dev, "assigned reserved memory node %s\n", rmem->name);
+	} else {
+		kfree(rd);
+	}
 
 	return ret;
 }
@@ -319,21 +373,26 @@ EXPORT_SYMBOL_GPL(of_reserved_mem_device_init);
 
 /**
  * of_reserved_mem_device_release() - release reserved memory device structures
+ * @dev:	Pointer to the device to deconfigure
  *
  * This function releases structures allocated for memory region handling for
  * the given device.
  */
 void of_reserved_mem_device_release(struct device *dev)
 {
-	struct reserved_mem *rmem;
-	struct device_node *np;
-
-	np = of_parse_phandle(dev->of_node, "memory-region", 0);
-	if (!np)
-		return;
-
-	rmem = __find_rmem(np);
-	of_node_put(np);
+	struct rmem_assigned_device *rd;
+	struct reserved_mem *rmem = NULL;
+
+	mutex_lock(&of_rmem_assigned_device_mutex);
+	list_for_each_entry(rd, &of_rmem_assigned_device_list, list) {
+		if (rd->dev == dev) {
+			rmem = rd->rmem;
+			list_del(&rd->list);
+			kfree(rd);
+			break;
+		}
+	}
+	mutex_unlock(&of_rmem_assigned_device_mutex);
 
 	if (!rmem || !rmem->ops || !rmem->ops->device_release)
 		return;
diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
index ad2f67054372..05dd38490657 100644
--- a/include/linux/of_reserved_mem.h
+++ b/include/linux/of_reserved_mem.h
@@ -28,14 +28,16 @@ typedef int (*reservedmem_of_init_fn)(struct reserved_mem *rmem);
 	_OF_DECLARE(reservedmem, name, compat, init, reservedmem_of_init_fn)
 
 #ifdef CONFIG_OF_RESERVED_MEM
-int of_reserved_mem_device_init(struct device *dev);
+int of_reserved_mem_device_init(struct device *dev, struct device_node *np,
+				const char *name);
 void of_reserved_mem_device_release(struct device *dev);
 
 void fdt_init_reserved_mem(void);
 void fdt_reserved_mem_save_node(unsigned long node, const char *uname,
 			       phys_addr_t base, phys_addr_t size);
 #else
-static inline int of_reserved_mem_device_init(struct device *dev)
+static inline int of_reserved_mem_device_init(struct device *dev,
+				struct device_node *np, const char *name)
 {
 	return -ENOSYS;
 }
-- 
1.9.2

[PATCH 4/7] media: vb2-dma-contig: add helper for setting dma max seg size

[Marek Szyprowski]

Add a helper function for device drivers to set DMA's max_seg_size.
Setting it to largest possible value lets DMA-mapping API always create
contiguous mappings in DMA address space. This is essential for all
devices, which use dma-contig videobuf2 memory allocator and shared
buffers.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
Changelog:
v3:
- make this code a helper function instead of chaning max_seg_size
  unconditionally on vb2_dma_contig_init_ctx

v2:
- set max segment size only if a new dma params structure has been
  allocated, as suggested by Laurent Pinchart
---
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 15 +++++++++++++++
 include/media/videobuf2-dma-contig.h           |  1 +
 2 files changed, 16 insertions(+)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index c33127284cfe..628518dc3aad 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -742,6 +742,21 @@ void vb2_dma_contig_cleanup_ctx(void *alloc_ctx)
 }
 EXPORT_SYMBOL_GPL(vb2_dma_contig_cleanup_ctx);
 
+int vb2_dma_contig_set_max_seg_size(struct device *dev, unsigned int size)
+{
+	if (!dev->dma_parms) {
+		dev->dma_parms = devm_kzalloc(dev, sizeof(dev->dma_parms),
+					      GFP_KERNEL);
+		if (!dev->dma_parms)
+			return -ENOMEM;
+	}
+	if (dma_get_max_seg_size(dev) < size)
+		return dma_set_max_seg_size(dev, size);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(vb2_dma_contig_set_max_seg_size);
+
 MODULE_DESCRIPTION("DMA-contig memory handling routines for videobuf2");
 MODULE_AUTHOR("Pawel Osciak <pawel@osciak.com>");
 MODULE_LICENSE("GPL");
diff --git a/include/media/videobuf2-dma-contig.h b/include/media/videobuf2-dma-contig.h
index c33dfa69d7ab..0e6ba644939e 100644
--- a/include/media/videobuf2-dma-contig.h
+++ b/include/media/videobuf2-dma-contig.h
@@ -26,6 +26,7 @@ vb2_dma_contig_plane_dma_addr(struct vb2_buffer *vb, unsigned int plane_no)
 
 void *vb2_dma_contig_init_ctx(struct device *dev);
 void vb2_dma_contig_cleanup_ctx(void *alloc_ctx);
+int vb2_dma_contig_set_max_seg_size(struct device *dev, unsigned int size);
 
 extern const struct vb2_mem_ops vb2_dma_contig_memops;
 
-- 
1.9.2

[PATCH 5/7] media: set proper max seg size for devices on Exynos SoCs

[Marek Szyprowski]

All multimedia devices found on Exynos SoCs support only contiguous
buffers, so set DMA max segment size to DMA_BIT_MASK(32) to let memory
allocator to correctly create contiguous memory mappings.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/media/platform/exynos-gsc/gsc-core.c  | 1 +
 drivers/media/platform/exynos4-is/fimc-core.c | 1 +
 drivers/media/platform/exynos4-is/fimc-is.c   | 1 +
 drivers/media/platform/exynos4-is/fimc-lite.c | 1 +
 drivers/media/platform/s5p-g2d/g2d.c          | 1 +
 drivers/media/platform/s5p-jpeg/jpeg-core.c   | 1 +
 drivers/media/platform/s5p-mfc/s5p_mfc.c      | 2 ++
 drivers/media/platform/s5p-tv/mixer_video.c   | 1 +
 8 files changed, 9 insertions(+)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c
index 9b9e423e4fc4..4f90be43b5a9 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -1140,6 +1140,7 @@ static int gsc_probe(struct platform_device *pdev)
 		goto err_m2m;
 
 	/* Initialize continious memory allocator */
+	vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32));
 	gsc->alloc_ctx = vb2_dma_contig_init_ctx(dev);
 	if (IS_ERR(gsc->alloc_ctx)) {
 		ret = PTR_ERR(gsc->alloc_ctx);
diff --git a/drivers/media/platform/exynos4-is/fimc-core.c b/drivers/media/platform/exynos4-is/fimc-core.c
index cef2a7f07cdb..368e19b50498 100644
--- a/drivers/media/platform/exynos4-is/fimc-core.c
+++ b/drivers/media/platform/exynos4-is/fimc-core.c
@@ -1019,6 +1019,7 @@ static int fimc_probe(struct platform_device *pdev)
 	}
 
 	/* Initialize contiguous memory allocator */
+	vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32));
 	fimc->alloc_ctx = vb2_dma_contig_init_ctx(dev);
 	if (IS_ERR(fimc->alloc_ctx)) {
 		ret = PTR_ERR(fimc->alloc_ctx);
diff --git a/drivers/media/platform/exynos4-is/fimc-is.c b/drivers/media/platform/exynos4-is/fimc-is.c
index 49658ca39e51..123772fa0241 100644
--- a/drivers/media/platform/exynos4-is/fimc-is.c
+++ b/drivers/media/platform/exynos4-is/fimc-is.c
@@ -841,6 +841,7 @@ static int fimc_is_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto err_pm;
 
+	vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32));
 	is->alloc_ctx = vb2_dma_contig_init_ctx(dev);
 	if (IS_ERR(is->alloc_ctx)) {
 		ret = PTR_ERR(is->alloc_ctx);
diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c
index 6f76afd909c4..9cfd2221f53d 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite.c
@@ -1564,6 +1564,7 @@ static int fimc_lite_probe(struct platform_device *pdev)
 			goto err_sd;
 	}
 
+	vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32));
 	fimc->alloc_ctx = vb2_dma_contig_init_ctx(dev);
 	if (IS_ERR(fimc->alloc_ctx)) {
 		ret = PTR_ERR(fimc->alloc_ctx);
diff --git a/drivers/media/platform/s5p-g2d/g2d.c b/drivers/media/platform/s5p-g2d/g2d.c
index e1936d9d27da..31f6c233b146 100644
--- a/drivers/media/platform/s5p-g2d/g2d.c
+++ b/drivers/media/platform/s5p-g2d/g2d.c
@@ -681,6 +681,7 @@ static int g2d_probe(struct platform_device *pdev)
 		goto put_clk_gate;
 	}
 
+	vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
 	dev->alloc_ctx = vb2_dma_contig_init_ctx(&pdev->dev);
 	if (IS_ERR(dev->alloc_ctx)) {
 		ret = PTR_ERR(dev->alloc_ctx);
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 4a608cbe0fdb..6bd92f014a23 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -2839,6 +2839,7 @@ static int s5p_jpeg_probe(struct platform_device *pdev)
 		goto device_register_rollback;
 	}
 
+	vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
 	jpeg->alloc_ctx = vb2_dma_contig_init_ctx(&pdev->dev);
 	if (IS_ERR(jpeg->alloc_ctx)) {
 		v4l2_err(&jpeg->v4l2_dev, "Failed to init memory allocator\n");
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 81ffb67e6d66..8fcecf8a9a17 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -1164,11 +1164,13 @@ static int s5p_mfc_probe(struct platform_device *pdev)
 		}
 	}
 
+	vb2_dma_contig_set_max_seg_size(dev->mem_dev_l, DMA_BIT_MASK(32));
 	dev->alloc_ctx[0] = vb2_dma_contig_init_ctx(dev->mem_dev_l);
 	if (IS_ERR(dev->alloc_ctx[0])) {
 		ret = PTR_ERR(dev->alloc_ctx[0]);
 		goto err_res;
 	}
+	vb2_dma_contig_set_max_seg_size(dev->mem_dev_r, DMA_BIT_MASK(32));
 	dev->alloc_ctx[1] = vb2_dma_contig_init_ctx(dev->mem_dev_r);
 	if (IS_ERR(dev->alloc_ctx[1])) {
 		ret = PTR_ERR(dev->alloc_ctx[1]);
diff --git a/drivers/media/platform/s5p-tv/mixer_video.c b/drivers/media/platform/s5p-tv/mixer_video.c
index dc1c679e136c..1d9c2d5a10e7 100644
--- a/drivers/media/platform/s5p-tv/mixer_video.c
+++ b/drivers/media/platform/s5p-tv/mixer_video.c
@@ -80,6 +80,7 @@ int mxr_acquire_video(struct mxr_device *mdev,
 		goto fail;
 	}
 
+	vb2_dma_contig_set_max_seg_size(mdev->dev, DMA_BIT_MASK(32));
 	mdev->alloc_ctx = vb2_dma_contig_init_ctx(mdev->dev);
 	if (IS_ERR(mdev->alloc_ctx)) {
 		mxr_err(mdev, "could not acquire vb2 allocator\n");
-- 
1.9.2

[PATCH 6/7] media: s5p-mfc: replace custom reserved memory init code with generic one

[Marek Szyprowski]

This patch removes custom code for initialization and handling of
reserved memory regions in s5p-mfc driver and replaces it with generic
named reserved memory regions specified in device tree.

s5p-mfc driver now handles two reserved memory regions: "left" and
"right", defined by generic reserved memory bindings. Support for non-dt
platform has been removed, because all supported platforms have been
converted to device tree.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/media/platform/s5p-mfc/s5p_mfc.c | 129 +++++++++++++++----------------
 1 file changed, 62 insertions(+), 67 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 8fcecf8a9a17..55c557f835f2 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -22,6 +22,7 @@
 #include <media/v4l2-event.h>
 #include <linux/workqueue.h>
 #include <linux/of.h>
+#include <linux/of_reserved_mem.h>
 #include <media/videobuf2-v4l2.h>
 #include "s5p_mfc_common.h"
 #include "s5p_mfc_ctrl.h"
@@ -1043,55 +1044,67 @@ static const struct v4l2_file_operations s5p_mfc_fops = {
 	.mmap = s5p_mfc_mmap,
 };
 
-static int match_child(struct device *dev, void *data)
+/* DMA memory related helper functions */
+static void s5p_mfc_memdev_release(struct device *dev)
 {
-	if (!dev_name(dev))
-		return 0;
-	return !strcmp(dev_name(dev), (char *)data);
+	of_reserved_mem_device_release(dev);
 }
 
-static void *mfc_get_drv_data(struct platform_device *pdev);
-
-static int s5p_mfc_alloc_memdevs(struct s5p_mfc_dev *dev)
+static struct device *s5p_mfc_alloc_memdev(struct device *dev, const char *name)
 {
-	unsigned int mem_info[2] = { };
+	struct device *child;
+	int ret;
 
-	dev->mem_dev_l = devm_kzalloc(&dev->plat_dev->dev,
-			sizeof(struct device), GFP_KERNEL);
-	if (!dev->mem_dev_l) {
-		mfc_err("Not enough memory\n");
-		return -ENOMEM;
-	}
-	device_initialize(dev->mem_dev_l);
-	of_property_read_u32_array(dev->plat_dev->dev.of_node,
-			"samsung,mfc-l", mem_info, 2);
-	if (dma_declare_coherent_memory(dev->mem_dev_l, mem_info[0],
-				mem_info[0], mem_info[1],
-				DMA_MEMORY_MAP | DMA_MEMORY_EXCLUSIVE) == 0) {
-		mfc_err("Failed to declare coherent memory for\n"
-		"MFC device\n");
-		return -ENOMEM;
+	child = devm_kzalloc(dev, sizeof(struct device), GFP_KERNEL);
+	if (!child)
+		return NULL;
+
+	device_initialize(child);
+	dev_set_name(child, "%s:%s", dev_name(dev), name);
+	child->parent = dev;
+	child->bus = dev->bus;
+	child->coherent_dma_mask = dev->coherent_dma_mask;
+	child->dma_mask = dev->dma_mask;
+	child->release = s5p_mfc_memdev_release;
+
+	if (device_add(child) == 0) {
+		ret = of_reserved_mem_device_init(child, dev->of_node, name);
+		if (ret == 0)
+			return child;
 	}
 
-	dev->mem_dev_r = devm_kzalloc(&dev->plat_dev->dev,
-			sizeof(struct device), GFP_KERNEL);
-	if (!dev->mem_dev_r) {
-		mfc_err("Not enough memory\n");
-		return -ENOMEM;
-	}
-	device_initialize(dev->mem_dev_r);
-	of_property_read_u32_array(dev->plat_dev->dev.of_node,
-			"samsung,mfc-r", mem_info, 2);
-	if (dma_declare_coherent_memory(dev->mem_dev_r, mem_info[0],
-				mem_info[0], mem_info[1],
-				DMA_MEMORY_MAP | DMA_MEMORY_EXCLUSIVE) == 0) {
-		pr_err("Failed to declare coherent memory for\n"
-		"MFC device\n");
-		return -ENOMEM;
+	put_device(child);
+	return NULL;
+}
+
+static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_dev)
+{
+	struct device *dev = &mfc_dev->plat_dev->dev;
+
+	/*
+	 * Create and initialize virtual devices for accessing
+	 * reserved memory regions.
+	 */
+	mfc_dev->mem_dev_l = s5p_mfc_alloc_memdev(dev, "left");
+	if (!mfc_dev->mem_dev_l)
+		return -ENODEV;
+	mfc_dev->mem_dev_r = s5p_mfc_alloc_memdev(dev, "right");
+	if (!mfc_dev->mem_dev_r) {
+		device_unregister(mfc_dev->mem_dev_l);
+		return -ENODEV;
 	}
+
 	return 0;
 }
 
+static void s5p_mfc_unconfigure_dma_memory(struct s5p_mfc_dev *mfc_dev)
+{
+	device_unregister(mfc_dev->mem_dev_l);
+	device_unregister(mfc_dev->mem_dev_r);
+}
+
+static void *mfc_get_drv_data(struct platform_device *pdev);
+
 /* MFC probe function */
 static int s5p_mfc_probe(struct platform_device *pdev)
 {
@@ -1117,12 +1130,6 @@ static int s5p_mfc_probe(struct platform_device *pdev)
 
 	dev->variant = mfc_get_drv_data(pdev);
 
-	ret = s5p_mfc_init_pm(dev);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "failed to get mfc clock source\n");
-		return ret;
-	}
-
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 
 	dev->regs_base = devm_ioremap_resource(&pdev->dev, res);
@@ -1143,25 +1150,16 @@ static int s5p_mfc_probe(struct platform_device *pdev)
 		goto err_res;
 	}
 
-	if (pdev->dev.of_node) {
-		ret = s5p_mfc_alloc_memdevs(dev);
-		if (ret < 0)
-			goto err_res;
-	} else {
-		dev->mem_dev_l = device_find_child(&dev->plat_dev->dev,
-				"s5p-mfc-l", match_child);
-		if (!dev->mem_dev_l) {
-			mfc_err("Mem child (L) device get failed\n");
-			ret = -ENODEV;
-			goto err_res;
-		}
-		dev->mem_dev_r = device_find_child(&dev->plat_dev->dev,
-				"s5p-mfc-r", match_child);
-		if (!dev->mem_dev_r) {
-			mfc_err("Mem child (R) device get failed\n");
-			ret = -ENODEV;
-			goto err_res;
-		}
+	ret = s5p_mfc_configure_dma_memory(dev);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to configure DMA memory\n");
+		return ret;
+	}
+
+	ret = s5p_mfc_init_pm(dev);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to get mfc clock source\n");
+		return ret;
 	}
 
 	vb2_dma_contig_set_max_seg_size(dev->mem_dev_l, DMA_BIT_MASK(32));
@@ -1295,10 +1293,7 @@ static int s5p_mfc_remove(struct platform_device *pdev)
 	s5p_mfc_release_firmware(dev);
 	vb2_dma_contig_cleanup_ctx(dev->alloc_ctx[0]);
 	vb2_dma_contig_cleanup_ctx(dev->alloc_ctx[1]);
-	if (pdev->dev.of_node) {
-		put_device(dev->mem_dev_l);
-		put_device(dev->mem_dev_r);
-	}
+	s5p_mfc_unconfigure_dma_memory(dev);
 
 	s5p_mfc_final_pm(dev);
 	return 0;
-- 
1.9.2

[PATCH 7/7] media: s5p-mfc: add iommu support

[Marek Szyprowski]

This patch adds support for IOMMU to s5p-mfc device driver. MFC firmware
is limited and it cannot use the default configuration. If IOMMU is
available, the patch disables the default DMA address space
configuration and creates a new address space of size limited to 256M
and base address set to 0x20000000.

For now the same address space is shared by both 'left' and 'right'
memory channels, because the DMA/IOMMU frameworks do not support
configuring them separately. This is not optimal, but besides limiting
total address space available has no other drawbacks (MFC firmware
supports 256M of address space per each channel).

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/media/platform/s5p-mfc/s5p_mfc.c       | 24 ++++++++
 drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h | 79 ++++++++++++++++++++++++++
 2 files changed, 103 insertions(+)
 create mode 100644 drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 55c557f835f2..dfbaadd22a3d 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -30,6 +30,7 @@
 #include "s5p_mfc_dec.h"
 #include "s5p_mfc_enc.h"
 #include "s5p_mfc_intr.h"
+#include "s5p_mfc_iommu.h"
 #include "s5p_mfc_opr.h"
 #include "s5p_mfc_cmd.h"
 #include "s5p_mfc_pm.h"
@@ -1082,6 +1083,22 @@ static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_dev)
 	struct device *dev = &mfc_dev->plat_dev->dev;
 
 	/*
+	 * When IOMMU is available, we cannot use the default configuration,
+	 * because of MFC firmware requirements: address space limited to
+	 * 256M and non-zero default start address.
+	 * This is still simplified, not optimal configuration, but for now
+	 * IOMMU core doesn't allow to configure device's IOMMUs channel
+	 * separately.
+	 */
+	if (exynos_is_iommu_available(dev)) {
+		int ret = exynos_configure_iommu(dev, S5P_MFC_IOMMU_DMA_BASE,
+						 S5P_MFC_IOMMU_DMA_SIZE);
+		if (ret == 0)
+			mfc_dev->mem_dev_l = mfc_dev->mem_dev_r = dev;
+		return ret;
+	}
+
+	/*
 	 * Create and initialize virtual devices for accessing
 	 * reserved memory regions.
 	 */
@@ -1099,6 +1116,13 @@ static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_dev)
 
 static void s5p_mfc_unconfigure_dma_memory(struct s5p_mfc_dev *mfc_dev)
 {
+	struct device *dev = &mfc_dev->plat_dev->dev;
+
+	if (exynos_is_iommu_available(dev)) {
+		exynos_unconfigure_iommu(dev);
+		return;
+	}
+
 	device_unregister(mfc_dev->mem_dev_l);
 	device_unregister(mfc_dev->mem_dev_r);
 }
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h b/drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h
new file mode 100644
index 000000000000..5d1d1c2922e8
--- /dev/null
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h
@@ -0,0 +1,79 @@
+/*
+ * Copyright (C) 2015 Samsung Electronics Co.Ltd
+ * Authors: Marek Szyprowski <m.szyprowski@samsung.com>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+#ifndef S5P_MFC_IOMMU_H_
+#define S5P_MFC_IOMMU_H_
+
+#define S5P_MFC_IOMMU_DMA_BASE	0x20000000lu
+#define S5P_MFC_IOMMU_DMA_SIZE	SZ_256M
+
+#ifdef CONFIG_EXYNOS_IOMMU
+
+#include <asm/dma-iommu.h>
+
+static inline bool exynos_is_iommu_available(struct device *dev)
+{
+	return dev->archdata.iommu != NULL;
+}
+
+static inline void exynos_unconfigure_iommu(struct device *dev)
+{
+	struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev);
+
+	arm_iommu_detach_device(dev);
+	arm_iommu_release_mapping(mapping);
+}
+
+static inline int exynos_configure_iommu(struct device *dev,
+					 unsigned int base, unsigned int size)
+{
+	struct dma_iommu_mapping *mapping = NULL;
+	int ret;
+
+	/* Disable the default mapping created by device core */
+	if (to_dma_iommu_mapping(dev))
+		exynos_unconfigure_iommu(dev);
+
+	mapping = arm_iommu_create_mapping(dev->bus, base, size);
+	if (IS_ERR(mapping)) {
+		pr_warn("Failed to create IOMMU mapping for device %s\n",
+			dev_name(dev));
+		return PTR_ERR(mapping);
+	}
+
+	ret = arm_iommu_attach_device(dev, mapping);
+	if (ret) {
+		pr_warn("Failed to attached device %s to IOMMU_mapping\n",
+				dev_name(dev));
+		arm_iommu_release_mapping(mapping);
+		return ret;
+	}
+
+	return 0;
+}
+
+#else
+
+static inline bool exynos_is_iommu_available(struct device *dev)
+{
+	return false;
+}
+
+static inline int exynos_configure_iommu(struct device *dev,
+					 unsigned int base, unsigned int size)
+{
+	return -ENOSYS;
+}
+
+static inline void exynos_unconfigure_iommu(struct device *dev) { }
+
+#endif
+
+#endif /* S5P_MFC_IOMMU_H_ */
-- 
1.9.2

Re: [PATCH 0/7] Exynos: MFC driver: reserved memory cleanup and IOMMU support

[Marek Szyprowski]

Hello,

On 2015-12-07 13:08, Marek Szyprowski wrote:
> This patchset finally perform cleanup of custom code in s5p-mfc codec
> driver. The first part is removal of custom, driver specific code for
> intializing and handling of reserved memory. Instead, a generic code for
> reserved memory regions is used. Then, once it is done, the proper setup
> of DMA parameters (max segment size) is applied for all multimedia
> devices found on Exynos SoCs to let them properly handle shared buffers
> mapped into contiguous DMA address space. The last patch adds support
> for IOMMU to MFC driver. Some additional code is needed because of
> specific requirements of MFC device firmware (see patch 7 for more
> details). When no IOMMU is available, the code fallbacks to generic
> reserved memory regions.
>
> After applying this patchset, MFC device works correctly when IOMMU is
> either enabled or disabled.
>
> Patches have been tested on top of linux-next from 20151207. I would
> prefer to merge patches 1-2 via Samsung tree and patches 3-7 via media
> tree (there are no compile-time dependencies between patches 1-2 and
> 3-7). Patches have been tested on Odroid U3 (Exynos 4412 based) and
> Odroid XU3 (Exynos 5422 based) boards.

One more notice: this is an updated version of the old patch initially
posted here:
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-August/189259.html

The main change since that is adaptation for generic reserved memory
bindings, which have been merged a while ago and added support for IOMMU.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH 3/7] of: reserved_mem: add support for named reserved mem nodes

[Rob Herring]

On Mon, Dec 7, 2015 at 6:08 AM, Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
> This patch allows device drivers to use more than one reserved memory
> region assigned to given device. When NULL name is passed to
> of_reserved_mem_device_init(), the default (first) region is used.

Every property that's an array does not need a name property. Just use
indexes please.

>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/of/of_reserved_mem.c    | 101 +++++++++++++++++++++++++++++++---------
>  include/linux/of_reserved_mem.h |   6 ++-
>  2 files changed, 84 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 1a3556a9e9ea..0a0b23b73004 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -21,6 +21,7 @@
>  #include <linux/sizes.h>
>  #include <linux/of_reserved_mem.h>
>  #include <linux/sort.h>
> +#include <linux/slab.h>
>
>  #define MAX_RESERVED_REGIONS   16
>  static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS];
> @@ -287,31 +288,84 @@ static inline struct reserved_mem *__find_rmem(struct device_node *node)
>         return NULL;
>  }
>
> +static struct reserved_mem *__node_to_rmem(struct device_node *node,
> +                                          const char *name)
> +{
> +       struct reserved_mem *rmem;
> +       struct device_node *target;
> +       int idx = 0;
> +
> +       if (!node)
> +               return NULL;
> +
> +       if (name) {
> +               idx = of_property_match_string(node,
> +                                              "memory-region-names", name);
> +               if (idx < 0)
> +                       return NULL;
> +       }
> +
> +       target = of_parse_phandle(node, "memory-region", idx);
> +       if (!target)
> +               return NULL;
> +       rmem = __find_rmem(target);
> +       of_node_put(target);
> +
> +       return rmem;
> +}
> +
> +struct rmem_assigned_device {
> +       struct device *dev;
> +       struct reserved_mem *rmem;
> +       struct list_head list;
> +};
> +
> +static LIST_HEAD(of_rmem_assigned_device_list);
> +static DEFINE_MUTEX(of_rmem_assigned_device_mutex);

Not that this is a fast or contended path, but I think a spinlock
would be more appropriate here.

> +
>  /**
>   * of_reserved_mem_device_init() - assign reserved memory region to given device
> + * @dev:       Pointer to the device to configure
> + * @np:                Pointer to the device_node with 'reserved-memory' property
> + * @name:      Optional name of the selected region (can be NULL)
> + *
> + * This function assigns respective DMA-mapping operations based on reserved
> + * memory regionspecified by 'memory-region' property in @np node, named @name
> + * to the @dev device. When NULL name is provided, the default (first) memory
> + * region is used. When driver needs to use more than one reserved memory
> + * region, it should allocate child devices and initialize regions by name for
> + * each of child device.
>   *
> - * This function assign memory region pointed by "memory-region" device tree
> - * property to the given device.
> + * Returns error code or zero on success.
>   */
> -int of_reserved_mem_device_init(struct device *dev)
> +int of_reserved_mem_device_init(struct device *dev, struct device_node *np,
> +                               const char *name)
>  {
> +       struct rmem_assigned_device *rd;
>         struct reserved_mem *rmem;
> -       struct device_node *np;
>         int ret;
>
> -       np = of_parse_phandle(dev->of_node, "memory-region", 0);
> -       if (!np)
> -               return -ENODEV;
> -
> -       rmem = __find_rmem(np);
> -       of_node_put(np);
> -
> +       rmem = __node_to_rmem(np, name);
>         if (!rmem || !rmem->ops || !rmem->ops->device_init)
>                 return -EINVAL;
>
> +       rd = kmalloc(sizeof(struct rmem_assigned_device), GFP_KERNEL);
> +       if (!rd)
> +               return -ENOMEM;
> +
>         ret = rmem->ops->device_init(rmem, dev);
> -       if (ret == 0)
> +       if (ret == 0) {
> +               rd->dev = dev;
> +               rd->rmem = rmem;
> +
> +               mutex_lock(&of_rmem_assigned_device_mutex);
> +               list_add(&rd->list, &of_rmem_assigned_device_list);
> +               mutex_unlock(&of_rmem_assigned_device_mutex);
> +
>                 dev_info(dev, "assigned reserved memory node %s\n", rmem->name);
> +       } else {
> +               kfree(rd);
> +       }
>
>         return ret;
>  }
> @@ -319,21 +373,26 @@ EXPORT_SYMBOL_GPL(of_reserved_mem_device_init);
>
>  /**
>   * of_reserved_mem_device_release() - release reserved memory device structures
> + * @dev:       Pointer to the device to deconfigure
>   *
>   * This function releases structures allocated for memory region handling for
>   * the given device.
>   */
>  void of_reserved_mem_device_release(struct device *dev)
>  {
> -       struct reserved_mem *rmem;
> -       struct device_node *np;
> -
> -       np = of_parse_phandle(dev->of_node, "memory-region", 0);
> -       if (!np)
> -               return;
> -
> -       rmem = __find_rmem(np);
> -       of_node_put(np);
> +       struct rmem_assigned_device *rd;
> +       struct reserved_mem *rmem = NULL;
> +
> +       mutex_lock(&of_rmem_assigned_device_mutex);
> +       list_for_each_entry(rd, &of_rmem_assigned_device_list, list) {
> +               if (rd->dev == dev) {
> +                       rmem = rd->rmem;
> +                       list_del(&rd->list);
> +                       kfree(rd);
> +                       break;

Is this function supposed to be called multiple times to release each
region. That's not very obvious and which region it removes undefined
for the call as it is just the first entry it finds.

Either both functions should init/release all regions or a single
specified region. I suppose there could be reasons not to init all
regions, but would expect that is the exception.

Rob

Re: [PATCH 3/7] of: reserved_mem: add support for named reserved mem nodes

[Marek Szyprowski]

Hello,

On 2015-12-08 15:58, Rob Herring wrote:
> On Mon, Dec 7, 2015 at 6:08 AM, Marek Szyprowski
> <m.szyprowski@samsung.com> wrote:
>> This patch allows device drivers to use more than one reserved memory
>> region assigned to given device. When NULL name is passed to
>> of_reserved_mem_device_init(), the default (first) region is used.
> Every property that's an array does not need a name property. Just use
> indexes please.

Okay, I will update the patch and add support for indices in the main
implementation as well as a wrapper, which accepts "name" parameter.

>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>   drivers/of/of_reserved_mem.c    | 101 +++++++++++++++++++++++++++++++---------
>>   include/linux/of_reserved_mem.h |   6 ++-
>>   2 files changed, 84 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
>> index 1a3556a9e9ea..0a0b23b73004 100644
>> --- a/drivers/of/of_reserved_mem.c
>> +++ b/drivers/of/of_reserved_mem.c
>> @@ -21,6 +21,7 @@
>>   #include <linux/sizes.h>
>>   #include <linux/of_reserved_mem.h>
>>   #include <linux/sort.h>
>> +#include <linux/slab.h>
>>
>>   #define MAX_RESERVED_REGIONS   16
>>   static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS];
>> @@ -287,31 +288,84 @@ static inline struct reserved_mem *__find_rmem(struct device_node *node)
>>          return NULL;
>>   }
>>
>> +static struct reserved_mem *__node_to_rmem(struct device_node *node,
>> +                                          const char *name)
>> +{
>> +       struct reserved_mem *rmem;
>> +       struct device_node *target;
>> +       int idx = 0;
>> +
>> +       if (!node)
>> +               return NULL;
>> +
>> +       if (name) {
>> +               idx = of_property_match_string(node,
>> +                                              "memory-region-names", name);
>> +               if (idx < 0)
>> +                       return NULL;
>> +       }
>> +
>> +       target = of_parse_phandle(node, "memory-region", idx);
>> +       if (!target)
>> +               return NULL;
>> +       rmem = __find_rmem(target);
>> +       of_node_put(target);
>> +
>> +       return rmem;
>> +}
>> +
>> +struct rmem_assigned_device {
>> +       struct device *dev;
>> +       struct reserved_mem *rmem;
>> +       struct list_head list;
>> +};
>> +
>> +static LIST_HEAD(of_rmem_assigned_device_list);
>> +static DEFINE_MUTEX(of_rmem_assigned_device_mutex);
> Not that this is a fast or contended path, but I think a spinlock
> would be more appropriate here.

This is not meant to be called really often and for all kinds on 
initialization lists
and structures I saw that mutexes are used instead of spinlocks. There 
is no intention
to let this function to be called from atomic context.

>
>> +
>>   /**
>>    * of_reserved_mem_device_init() - assign reserved memory region to given device
>> + * @dev:       Pointer to the device to configure
>> + * @np:                Pointer to the device_node with 'reserved-memory' property
>> + * @name:      Optional name of the selected region (can be NULL)
>> + *
>> + * This function assigns respective DMA-mapping operations based on reserved
>> + * memory regionspecified by 'memory-region' property in @np node, named @name
>> + * to the @dev device. When NULL name is provided, the default (first) memory
>> + * region is used. When driver needs to use more than one reserved memory
>> + * region, it should allocate child devices and initialize regions by name for
>> + * each of child device.
>>    *
>> - * This function assign memory region pointed by "memory-region" device tree
>> - * property to the given device.
>> + * Returns error code or zero on success.
>>    */
>> -int of_reserved_mem_device_init(struct device *dev)
>> +int of_reserved_mem_device_init(struct device *dev, struct device_node *np,
>> +                               const char *name)
>>   {
>> +       struct rmem_assigned_device *rd;
>>          struct reserved_mem *rmem;
>> -       struct device_node *np;
>>          int ret;
>>
>> -       np = of_parse_phandle(dev->of_node, "memory-region", 0);
>> -       if (!np)
>> -               return -ENODEV;
>> -
>> -       rmem = __find_rmem(np);
>> -       of_node_put(np);
>> -
>> +       rmem = __node_to_rmem(np, name);
>>          if (!rmem || !rmem->ops || !rmem->ops->device_init)
>>                  return -EINVAL;
>>
>> +       rd = kmalloc(sizeof(struct rmem_assigned_device), GFP_KERNEL);
>> +       if (!rd)
>> +               return -ENOMEM;
>> +
>>          ret = rmem->ops->device_init(rmem, dev);
>> -       if (ret == 0)
>> +       if (ret == 0) {
>> +               rd->dev = dev;
>> +               rd->rmem = rmem;
>> +
>> +               mutex_lock(&of_rmem_assigned_device_mutex);
>> +               list_add(&rd->list, &of_rmem_assigned_device_list);
>> +               mutex_unlock(&of_rmem_assigned_device_mutex);
>> +
>>                  dev_info(dev, "assigned reserved memory node %s\n", rmem->name);
>> +       } else {
>> +               kfree(rd);
>> +       }
>>
>>          return ret;
>>   }
>> @@ -319,21 +373,26 @@ EXPORT_SYMBOL_GPL(of_reserved_mem_device_init);
>>
>>   /**
>>    * of_reserved_mem_device_release() - release reserved memory device structures
>> + * @dev:       Pointer to the device to deconfigure
>>    *
>>    * This function releases structures allocated for memory region handling for
>>    * the given device.
>>    */
>>   void of_reserved_mem_device_release(struct device *dev)
>>   {
>> -       struct reserved_mem *rmem;
>> -       struct device_node *np;
>> -
>> -       np = of_parse_phandle(dev->of_node, "memory-region", 0);
>> -       if (!np)
>> -               return;
>> -
>> -       rmem = __find_rmem(np);
>> -       of_node_put(np);
>> +       struct rmem_assigned_device *rd;
>> +       struct reserved_mem *rmem = NULL;
>> +
>> +       mutex_lock(&of_rmem_assigned_device_mutex);
>> +       list_for_each_entry(rd, &of_rmem_assigned_device_list, list) {
>> +               if (rd->dev == dev) {
>> +                       rmem = rd->rmem;
>> +                       list_del(&rd->list);
>> +                       kfree(rd);
>> +                       break;
> Is this function supposed to be called multiple times to release each
> region. That's not very obvious and which region it removes undefined
> for the call as it is just the first entry it finds.
>
> Either both functions should init/release all regions or a single
> specified region. I suppose there could be reasons not to init all
> regions, but would expect that is the exception.

Current implementation allows only to initialize (assign) one region for
given struct device. I can add a check if given device has been already
initialized if needed.

To let device driver to use more regions, additional child devices need
to be allocated and then for each separate call of
of_reserved_mem_device_init() is needed. This is a limitation of current
dma-mapping subsystem, for which the only 'key' for selecting dma/memory
address space is a struct device pointer. Maybe later dma-mapping will
be extended to support named/intexed dma address spaces, but for now
this is the only solution.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH 3/7] of: reserved_mem: add support for named reserved mem nodes

[Rob Herring]

On Wed, Dec 9, 2015 at 4:08 AM, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> Hello,
>
> On 2015-12-08 15:58, Rob Herring wrote:
>>
>> On Mon, Dec 7, 2015 at 6:08 AM, Marek Szyprowski
>> <m.szyprowski@samsung.com> wrote:
>>>
>>> This patch allows device drivers to use more than one reserved memory
>>> region assigned to given device. When NULL name is passed to
>>> of_reserved_mem_device_init(), the default (first) region is used.
>>
>> Every property that's an array does not need a name property. Just use
>> indexes please.
>
>
> Okay, I will update the patch and add support for indices in the main
> implementation as well as a wrapper, which accepts "name" parameter.
>
>
>>
>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> ---
>>>   drivers/of/of_reserved_mem.c    | 101
>>> +++++++++++++++++++++++++++++++---------
>>>   include/linux/of_reserved_mem.h |   6 ++-
>>>   2 files changed, 84 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
>>> index 1a3556a9e9ea..0a0b23b73004 100644
>>> --- a/drivers/of/of_reserved_mem.c
>>> +++ b/drivers/of/of_reserved_mem.c
>>> @@ -21,6 +21,7 @@
>>>   #include <linux/sizes.h>
>>>   #include <linux/of_reserved_mem.h>
>>>   #include <linux/sort.h>
>>> +#include <linux/slab.h>
>>>
>>>   #define MAX_RESERVED_REGIONS   16
>>>   static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS];
>>> @@ -287,31 +288,84 @@ static inline struct reserved_mem
>>> *__find_rmem(struct device_node *node)
>>>          return NULL;
>>>   }
>>>
>>> +static struct reserved_mem *__node_to_rmem(struct device_node *node,
>>> +                                          const char *name)
>>> +{
>>> +       struct reserved_mem *rmem;
>>> +       struct device_node *target;
>>> +       int idx = 0;
>>> +
>>> +       if (!node)
>>> +               return NULL;
>>> +
>>> +       if (name) {
>>> +               idx = of_property_match_string(node,
>>> +                                              "memory-region-names",
>>> name);
>>> +               if (idx < 0)
>>> +                       return NULL;
>>> +       }
>>> +
>>> +       target = of_parse_phandle(node, "memory-region", idx);
>>> +       if (!target)
>>> +               return NULL;
>>> +       rmem = __find_rmem(target);
>>> +       of_node_put(target);
>>> +
>>> +       return rmem;
>>> +}
>>> +
>>> +struct rmem_assigned_device {
>>> +       struct device *dev;
>>> +       struct reserved_mem *rmem;
>>> +       struct list_head list;
>>> +};
>>> +
>>> +static LIST_HEAD(of_rmem_assigned_device_list);
>>> +static DEFINE_MUTEX(of_rmem_assigned_device_mutex);
>>
>> Not that this is a fast or contended path, but I think a spinlock
>> would be more appropriate here.
>
>
> This is not meant to be called really often and for all kinds on
> initialization lists
> and structures I saw that mutexes are used instead of spinlocks. There is no
> intention
> to let this function to be called from atomic context.
>
>
>>
>>> +
>>>   /**
>>>    * of_reserved_mem_device_init() - assign reserved memory region to
>>> given device
>>> + * @dev:       Pointer to the device to configure
>>> + * @np:                Pointer to the device_node with 'reserved-memory'
>>> property
>>> + * @name:      Optional name of the selected region (can be NULL)
>>> + *
>>> + * This function assigns respective DMA-mapping operations based on
>>> reserved
>>> + * memory regionspecified by 'memory-region' property in @np node, named
>>> @name
>>> + * to the @dev device. When NULL name is provided, the default (first)
>>> memory
>>> + * region is used. When driver needs to use more than one reserved
>>> memory
>>> + * region, it should allocate child devices and initialize regions by
>>> name for
>>> + * each of child device.
>>>    *
>>> - * This function assign memory region pointed by "memory-region" device
>>> tree
>>> - * property to the given device.
>>> + * Returns error code or zero on success.
>>>    */
>>> -int of_reserved_mem_device_init(struct device *dev)
>>> +int of_reserved_mem_device_init(struct device *dev, struct device_node
>>> *np,
>>> +                               const char *name)
>>>   {
>>> +       struct rmem_assigned_device *rd;
>>>          struct reserved_mem *rmem;
>>> -       struct device_node *np;
>>>          int ret;
>>>
>>> -       np = of_parse_phandle(dev->of_node, "memory-region", 0);
>>> -       if (!np)
>>> -               return -ENODEV;
>>> -
>>> -       rmem = __find_rmem(np);
>>> -       of_node_put(np);
>>> -
>>> +       rmem = __node_to_rmem(np, name);
>>>          if (!rmem || !rmem->ops || !rmem->ops->device_init)
>>>                  return -EINVAL;
>>>
>>> +       rd = kmalloc(sizeof(struct rmem_assigned_device), GFP_KERNEL);
>>> +       if (!rd)
>>> +               return -ENOMEM;
>>> +
>>>          ret = rmem->ops->device_init(rmem, dev);
>>> -       if (ret == 0)
>>> +       if (ret == 0) {
>>> +               rd->dev = dev;
>>> +               rd->rmem = rmem;
>>> +
>>> +               mutex_lock(&of_rmem_assigned_device_mutex);
>>> +               list_add(&rd->list, &of_rmem_assigned_device_list);
>>> +               mutex_unlock(&of_rmem_assigned_device_mutex);
>>> +
>>>                  dev_info(dev, "assigned reserved memory node %s\n",
>>> rmem->name);
>>> +       } else {
>>> +               kfree(rd);
>>> +       }
>>>
>>>          return ret;
>>>   }
>>> @@ -319,21 +373,26 @@ EXPORT_SYMBOL_GPL(of_reserved_mem_device_init);
>>>
>>>   /**
>>>    * of_reserved_mem_device_release() - release reserved memory device
>>> structures
>>> + * @dev:       Pointer to the device to deconfigure
>>>    *
>>>    * This function releases structures allocated for memory region
>>> handling for
>>>    * the given device.
>>>    */
>>>   void of_reserved_mem_device_release(struct device *dev)
>>>   {
>>> -       struct reserved_mem *rmem;
>>> -       struct device_node *np;
>>> -
>>> -       np = of_parse_phandle(dev->of_node, "memory-region", 0);
>>> -       if (!np)
>>> -               return;
>>> -
>>> -       rmem = __find_rmem(np);
>>> -       of_node_put(np);
>>> +       struct rmem_assigned_device *rd;
>>> +       struct reserved_mem *rmem = NULL;
>>> +
>>> +       mutex_lock(&of_rmem_assigned_device_mutex);
>>> +       list_for_each_entry(rd, &of_rmem_assigned_device_list, list) {
>>> +               if (rd->dev == dev) {
>>> +                       rmem = rd->rmem;
>>> +                       list_del(&rd->list);
>>> +                       kfree(rd);
>>> +                       break;
>>
>> Is this function supposed to be called multiple times to release each
>> region. That's not very obvious and which region it removes undefined
>> for the call as it is just the first entry it finds.
>>
>> Either both functions should init/release all regions or a single
>> specified region. I suppose there could be reasons not to init all
>> regions, but would expect that is the exception.
>
>
> Current implementation allows only to initialize (assign) one region for
> given struct device. I can add a check if given device has been already
> initialized if needed.
>
> To let device driver to use more regions, additional child devices need
> to be allocated and then for each separate call of
> of_reserved_mem_device_init() is needed. This is a limitation of current
> dma-mapping subsystem, for which the only 'key' for selecting dma/memory
> address space is a struct device pointer. Maybe later dma-mapping will
> be extended to support named/intexed dma address spaces, but for now
> this is the only solution.

That is assuming each region is a 
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>