linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/6] MT8173 IOMMU SUPPORT
@ 2015-10-09  2:23 Yong Wu
  2015-10-09  2:23 ` [PATCH v5 1/6] dt-bindings: iommu: Add binding for mediatek IOMMU Yong Wu
                   ` (6 more replies)
  0 siblings, 7 replies; 23+ messages in thread
From: Yong Wu @ 2015-10-09  2:23 UTC (permalink / raw)
  To: Joerg Roedel, Thierry Reding, Mark Rutland, Matthias Brugger
  Cc: Robin Murphy, Will Deacon, Daniel Kurtz, Tomasz Figa,
	Lucas Stach, Rob Herring, Catalin Marinas, linux-mediatek,
	Sasha Hauer, srv_heupstream, devicetree, linux-kernel,
	linux-arm-kernel, iommu, pebolle, arnd, mitchelh, Sricharan R,
	youhua.li, k.zhang, kendrick.hsu

  This patch set adds support for m4u(Multimedia Memory Management Unit),
Currently it only support the m4u with 2 levels of pagetable on mt8173.

  It's based on Robin Murphy's latest ARM64 DMA-v6[1]. The dsti is based
on MTK clock patch[2].
 
  Please check the hardware block diagram of Mediatek IOMMU.
 
              EMI (External Memory Interface)
               |
              m4u (Multimedia Memory Management Unit)
               |
              smi (Smart Multimedia Interface)
               |
        +---------------+-------
        |               |
        |               |
    vdec larb       disp larb      ... SoCs have different local arbiter(larb).
        |               |
        |               |
   +----+----+    +-----+-----+
   |    |    |    |     |     |    ...
   |    |    |    |     |     |    ...
   |    |    |    |     |     |    ...
  MC   PP   VLD  OVL0 RDMA0 WDMA0  ... There are different ports in each larb.
  
  Normally we specify a local arbiter(larb) for each multimedia hardware like
display, video decode, video encode and camera. And there are different ports in
each larb. Take a example, there are some ports like MC, PP, UFO, VLD, AVC_MV,
PRED_RD in video larb, all the ports are according to the video hardware.
 
  From the diagram, all the multimedia module connect with m4u via smi.
SMI is responsible to enable/disable iommu and control the clocks for each local
arbiter. If we would like to enable the iommu of video decode, the video decode
HW ports have to be configed. And if the video hardware work whether
enable/disable iommu, it need enable the clock of its larb's clock. And smi also
help bandwidth control for each local. So we add a special driver for smi and
locate it drivers/memory.

v5:
-rebase onto v4.3-rc1.
-About MTK iommu: don't return the same domain while domain_alloc, change the
 domain's flow according to the M4U HW.
-About Short-descriptor: Improve many error-handles, NO_PERMS quirk, and 
 add TLBI_MAP quirk following Will's Suggestion; Add io-pgtable don't use 
 dma_to_phys; Add a loop in arm_short_unmap since iommu_unmap don't care the 
 physical address align.
-About SMI driver: Add a help funcion for some similar code. Add PROPRE_DEFER
 for power-domain as MTK SCPSYS is module_init currently.

v4: http://lists.linuxfoundation.org/pipermail/iommu/2015-August/013903.html
-use only one iommu domain here based on the Robin's DMA-v5:
 http://lists.linuxfoundation.org/pipermail/iommu/2015-July/013900.html
-remove flush_pgtable.
-change writel to writel_relaxed.
-about Short-descriptor: move dma_map_single into io-pgtable-arm-short.
 Improve the flow of free pgtable and add NO_XN+NO_PERMS quirk following
 Will's suggestion.
-Change two sytle issues in dtsi according to Daniel's suggestion.

v3: http://lists.linuxfoundation.org/pipermail/iommu/2015-July/013632.html
-rebased onto v4.2-rc1
-improve iommu flow based on the Robin's DMA v3:
 http://lists.linuxfoundation.org/pipermail/iommu/2015-July/013597.html
-change mtk iommu-cells from 1 to 2.
-about Short-descriptor: add split function; add self-test; add some other bits like nG,
 XN according to the spec; add SUPERSECTION and MTK quirk; move io_pgtable_ops_to_pgtable
 out from LPAE to the header file.
-about SMI: move from driver/soc/mediatek to driver/memory; change the clocks from
 clk[2] to clk_apb and clk_smi; add pm.
-add iommu suspend/resume to backup/restore register.

v2: http://lists.linuxfoundation.org/pipermail/iommu/2015-May/013028.html
-add arm short descriptor support.
-seperate smi common from smi and change the clock-names according
 to smi HW.
-delete the hardcode of the port-names in mt8173.
 replace this with larb-portes-nr in dtsi.
-fix some coding style issues.

v1: http://lists.infradead.org/pipermail/linux-mediatek/2015-March/000058.html
-initial version.

[1]: http://lists.linuxfoundation.org/pipermail/iommu/2015-October/014504.html
[2]: http://lists.infradead.org/pipermail/linux-mediatek/2015-August/001962.html

Yong Wu (6):
  dt-bindings: iommu: Add binding for mediatek IOMMU
  dt-bindings: mediatek: Add smi dts binding
  iommu: add ARM short descriptor page table allocator
  memory: mediatek: Add SMI driver
  iommu/mediatek: Add mt8173 IOMMU driver
  dts: mt8173: Add iommu/smi nodes for mt8173

 .../devicetree/bindings/iommu/mediatek,iommu.txt   |  61 ++
 .../memory-controllers/mediatek,smi-larb.txt       |  25 +
 .../bindings/memory-controllers/mediatek,smi.txt   |  24 +
 arch/arm64/boot/dts/mediatek/mt8173.dtsi           |  81 ++
 drivers/iommu/Kconfig                              |  33 +
 drivers/iommu/Makefile                             |   2 +
 drivers/iommu/io-pgtable-arm-short.c               | 827 +++++++++++++++++++++
 drivers/iommu/io-pgtable-arm.c                     |   3 -
 drivers/iommu/io-pgtable.c                         |   3 +
 drivers/iommu/io-pgtable.h                         |  18 +-
 drivers/iommu/mtk_iommu.c                          | 767 +++++++++++++++++++
 drivers/memory/Kconfig                             |   8 +
 drivers/memory/Makefile                            |   1 +
 drivers/memory/mtk-smi.c                           | 274 +++++++
 include/dt-bindings/memory/mt8173-larb-port.h      | 105 +++
 include/soc/mediatek/smi.h                         |  60 ++
 16 files changed, 2288 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi.txt
 create mode 100644 drivers/iommu/io-pgtable-arm-short.c
 create mode 100644 drivers/iommu/mtk_iommu.c
 create mode 100644 drivers/memory/mtk-smi.c
 create mode 100644 include/dt-bindings/memory/mt8173-larb-port.h
 create mode 100644 include/soc/mediatek/smi.h

-- 
1.8.1.1.dirty


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v5 1/6] dt-bindings: iommu: Add binding for mediatek IOMMU
  2015-10-09  2:23 [PATCH v5 0/6] MT8173 IOMMU SUPPORT Yong Wu
@ 2015-10-09  2:23 ` Yong Wu
  2015-10-09  2:23 ` [PATCH v5 2/6] dt-bindings: mediatek: Add smi dts binding Yong Wu
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 23+ messages in thread
From: Yong Wu @ 2015-10-09  2:23 UTC (permalink / raw)
  To: Joerg Roedel, Thierry Reding, Mark Rutland, Matthias Brugger
  Cc: Robin Murphy, Will Deacon, Daniel Kurtz, Tomasz Figa,
	Lucas Stach, Rob Herring, Catalin Marinas, linux-mediatek,
	Sasha Hauer, srv_heupstream, devicetree, linux-kernel,
	linux-arm-kernel, iommu, pebolle, arnd, mitchelh, Sricharan R,
	youhua.li, k.zhang, kendrick.hsu, Yong Wu

This patch add mediatek iommu dts binding document.

Signed-off-by: Yong Wu <yong.wu@mediatek.com>
---
 .../devicetree/bindings/iommu/mediatek,iommu.txt   |  61 ++++++++++++
 include/dt-bindings/memory/mt8173-larb-port.h      | 105 +++++++++++++++++++++
 2 files changed, 166 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
 create mode 100644 include/dt-bindings/memory/mt8173-larb-port.h

diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt b/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
new file mode 100644
index 0000000..d515e47
--- /dev/null
+++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
@@ -0,0 +1,61 @@
+* Mediatek IOMMU Architecture Implementation
+
+  Mediatek Socs may contain a implementation of Multimedia Memory
+Management Unit(M4U),which use ARM Short-descriptor translation table
+to achieve address translation.
+
+  The IOMMU Hardware Block Diagram, please check below:
+
+              EMI (External Memory Interface)
+               |
+              m4u (Multimedia Memory Management Unit)
+               |
+              smi (Smart Multimedia Interface)
+               |
+        +---------------+-------
+        |               |
+        |               |
+    vdec larb       disp larb      ... SoCs have different local arbiter(larb).
+        |               |
+        |               |
+   +----+----+    +-----+-----+
+   |    |    |    |     |     |    ...
+   |    |    |    |     |     |    ...
+   |    |    |    |     |     |    ...
+  MC   PP   VLD  OVL0 RDMA0 WDMA0  ... There are different ports in each larb.
+
+  As above, The Multimedia HW will go through SMI and M4U while it
+access EMI. SMI is a brige between m4u and the Multimedia HW. It contain
+smi local arbiter and smi common. It will control whether the Multimedia
+HW should go though the m4u for translation or bypass it and talk
+directly with EMI. And also SMI help control the clocks for each
+local arbiter.
+  Normally we specify a local arbiter(larb) for each multimedia HW
+like display, video decode, and camera. And there are different ports
+in each larb. Take a example, There are some ports like MC, PP, VLD in the
+video decode local arbiter, all the ports are according to the video HW.
+
+Required properties:
+- compatible : must be "mediatek,mt8173-m4u".
+- reg : m4u register base and size.
+- interrupts : the interrupt of m4u.
+- clocks : must contain one entry for each clock-names.
+- clock-names : must be "bclk", It is the block clock of m4u.
+- mediatek,larb : List of phandle to the local arbiters in the current Socs.
+	Refer to bindings/memory-controllers/mediatek,smi-larb.txt. It must sort
+	according to the local arbiter index, like larb0, larb1, larb2...
+- iommu-cells : must be 2. There are 2 cells needed to enable/disable iommu.
+	The first one is local arbiter index(larbid), and the other is port
+	index(portid) within local arbiter. Specifies the larbid and portid as
+	defined in dt-binding/memory/mt8173-larb-port.h.
+
+Example:
+	iommu: iommu@10205000 {
+		compatible = "mediatek,mt8173-m4u";
+		reg = <0 0x10205000 0 0x1000>;
+		interrupts = <GIC_SPI 139 IRQ_TYPE_LEVEL_LOW>;
+		clocks = <&infracfg CLK_INFRA_M4U>;
+		clock-names = "bclk";
+		mediatek,larb = <&larb0 &larb1 &larb2 &larb3 &larb4 &larb5>;
+		#iommu-cells = <2>;
+	};
diff --git a/include/dt-bindings/memory/mt8173-larb-port.h b/include/dt-bindings/memory/mt8173-larb-port.h
new file mode 100644
index 0000000..8e5b716
--- /dev/null
+++ b/include/dt-bindings/memory/mt8173-larb-port.h
@@ -0,0 +1,105 @@
+/*
+ * Copyright (c) 2014-2015 MediaTek Inc.
+ * Author: Yong Wu <yong.wu@mediatek.com>
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#ifndef __DTS_IOMMU_PORT_MT8173_H
+#define __DTS_IOMMU_PORT_MT8173_H
+
+#define M4U_LARB0_ID			0
+#define M4U_LARB1_ID			1
+#define M4U_LARB2_ID			2
+#define M4U_LARB3_ID			3
+#define M4U_LARB4_ID			4
+#define M4U_LARB5_ID			5
+
+/* larb0 */
+#define M4U_PORT_DISP_OVL0		0
+#define M4U_PORT_DISP_RDMA0		1
+#define M4U_PORT_DISP_WDMA0		2
+#define M4U_PORT_DISP_OD_R		3
+#define M4U_PORT_DISP_OD_W		4
+#define M4U_PORT_MDP_RDMA0		5
+#define M4U_PORT_MDP_WDMA		6
+#define M4U_PORT_MDP_WROT0		7
+
+/* larb1 */
+#define M4U_PORT_HW_VDEC_MC_EXT		0
+#define M4U_PORT_HW_VDEC_PP_EXT		1
+#define M4U_PORT_HW_VDEC_UFO_EXT	2
+#define M4U_PORT_HW_VDEC_VLD_EXT	3
+#define M4U_PORT_HW_VDEC_VLD2_EXT	4
+#define M4U_PORT_HW_VDEC_AVC_MV_EXT	5
+#define M4U_PORT_HW_VDEC_PRED_RD_EXT	6
+#define M4U_PORT_HW_VDEC_PRED_WR_EXT	7
+#define M4U_PORT_HW_VDEC_PPWRAP_EXT	8
+#define M4U_PORT_HW_VDEC_TILE		9
+
+/* larb2 */
+#define M4U_PORT_IMGO			0
+#define M4U_PORT_RRZO			1
+#define M4U_PORT_AAO			2
+#define M4U_PORT_LCSO			3
+#define M4U_PORT_ESFKO			4
+#define M4U_PORT_IMGO_D			5
+#define M4U_PORT_LSCI			6
+#define M4U_PORT_LSCI_D			7
+#define M4U_PORT_BPCI			8
+#define M4U_PORT_BPCI_D			9
+#define M4U_PORT_UFDI			10
+#define M4U_PORT_IMGI			11
+#define M4U_PORT_IMG2O			12
+#define M4U_PORT_IMG3O			13
+#define M4U_PORT_VIPI			14
+#define M4U_PORT_VIP2I			15
+#define M4U_PORT_VIP3I			16
+#define M4U_PORT_LCEI			17
+#define M4U_PORT_RB			18
+#define M4U_PORT_RP			19
+#define M4U_PORT_WR			20
+
+/* larb3 */
+#define M4U_PORT_VENC_RCPU		0
+#define M4U_PORT_VENC_REC		1
+#define M4U_PORT_VENC_BSDMA		2
+#define M4U_PORT_VENC_SV_COMV		3
+#define M4U_PORT_VENC_RD_COMV		4
+#define M4U_PORT_JPGENC_RDMA		5
+#define M4U_PORT_JPGENC_BSDMA		6
+#define M4U_PORT_JPGDEC_WDMA		7
+#define M4U_PORT_JPGDEC_BSDMA		8
+#define M4U_PORT_VENC_CUR_LUMA		9
+#define M4U_PORT_VENC_CUR_CHROMA	10
+#define M4U_PORT_VENC_REF_LUMA		11
+#define M4U_PORT_VENC_REF_CHROMA	12
+#define M4U_PORT_VENC_NBM_RDMA		13
+#define M4U_PORT_VENC_NBM_WDMA		14
+
+/* larb4 */
+#define M4U_PORT_DISP_OVL1		0
+#define M4U_PORT_DISP_RDMA1		1
+#define M4U_PORT_DISP_RDMA2		2
+#define M4U_PORT_DISP_WDMA1		3
+#define M4U_PORT_MDP_RDMA1		4
+#define M4U_PORT_MDP_WROT1		5
+
+/* larb5 */
+#define M4U_PORT_VENC_RCPU_SET2		0
+#define M4U_PORT_VENC_REC_FRM_SET2	1
+#define M4U_PORT_VENC_REF_LUMA_SET2	2
+#define M4U_PORT_VENC_REC_CHROMA_SET2	3
+#define M4U_PORT_VENC_BSDMA_SET2	4
+#define M4U_PORT_VENC_CUR_LUMA_SET2	5
+#define M4U_PORT_VENC_CUR_CHROMA_SET2	6
+#define M4U_PORT_VENC_RD_COMA_SET2	7
+#define M4U_PORT_VENC_SV_COMA_SET2	8
+
+#endif
-- 
1.8.1.1.dirty


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 2/6] dt-bindings: mediatek: Add smi dts binding
  2015-10-09  2:23 [PATCH v5 0/6] MT8173 IOMMU SUPPORT Yong Wu
  2015-10-09  2:23 ` [PATCH v5 1/6] dt-bindings: iommu: Add binding for mediatek IOMMU Yong Wu
@ 2015-10-09  2:23 ` Yong Wu
  2015-10-09  2:23 ` [PATCH v5 3/6] iommu: add ARM short descriptor page table allocator Yong Wu
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 23+ messages in thread
From: Yong Wu @ 2015-10-09  2:23 UTC (permalink / raw)
  To: Joerg Roedel, Thierry Reding, Mark Rutland, Matthias Brugger
  Cc: Robin Murphy, Will Deacon, Daniel Kurtz, Tomasz Figa,
	Lucas Stach, Rob Herring, Catalin Marinas, linux-mediatek,
	Sasha Hauer, srv_heupstream, devicetree, linux-kernel,
	linux-arm-kernel, iommu, pebolle, arnd, mitchelh, Sricharan R,
	youhua.li, k.zhang, kendrick.hsu, Yong Wu

This patch add smi binding document.

Signed-off-by: Yong Wu <yong.wu@mediatek.com>
---
 .../memory-controllers/mediatek,smi-larb.txt       | 25 ++++++++++++++++++++++
 .../bindings/memory-controllers/mediatek,smi.txt   | 24 +++++++++++++++++++++
 2 files changed, 49 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,smi.txt

diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
new file mode 100644
index 0000000..55ff3b7
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
@@ -0,0 +1,25 @@
+SMI (Smart Multimedia Interface) Local Arbiter
+
+The hardware block diagram please check bindings/iommu/mediatek,iommu.txt
+
+Required properties:
+- compatible : must be "mediatek,mt8173-smi-larb"
+- reg : the register and size of this local arbiter.
+- mediatek,smi : a phandle to the smi_common node.
+- power-domains : a phandle to the power domain of this local arbiter.
+- clocks : Must contain an entry for each entry in clock-names.
+- clock-names: must contain 2 entries, as follows:
+  - "apb" : Advanced Peripheral Bus clock, It's the clock for setting
+	    the register.
+  - "smi" : It's the clock for transfer data and command.
+
+Example:
+	larb1: larb@16010000 {
+		compatible = "mediatek,mt8173-smi-larb";
+		reg = <0 0x16010000 0 0x1000>;
+		mediatek,smi = <&smi_common>;
+		power-domains = <&scpsys MT8173_POWER_DOMAIN_VDEC>;
+		clocks = <&vdecsys CLK_VDEC_CKEN>,
+			 <&vdecsys CLK_VDEC_LARB_CKEN>;
+		clock-names = "apb", "smi";
+	};
diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi.txt b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi.txt
new file mode 100644
index 0000000..f54e91c
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi.txt
@@ -0,0 +1,24 @@
+SMI (Smart Multimedia Interface)
+
+The hardware block diagram please check bindings/iommu/mediatek,iommu.txt
+
+Required properties:
+- compatible : must be "mediatek,mt8173-smi"
+- reg : the register and size of the SMI block.
+- power-domains : a phandle to the power domain of this local arbiter.
+- clocks : Must contain an entry for each entry in clock-names.
+- clock-names : must contain 2 entries, as follows:
+  - "apb" : Advanced Peripheral Bus clock, It's the clock for setting
+	    the register.
+  - "smi" : It's the clock for transfer data and command.
+  They may be the same if both source clock are the same.
+
+Example:
+	smi_common: smi@14022000 {
+		compatible = "mediatek,mt8173-smi";
+		reg = <0 0x14022000 0 0x1000>;
+		power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+		clocks = <&mmsys CLK_MM_SMI_COMMON>,
+			 <&mmsys CLK_MM_SMI_COMMON>;
+		clock-names = "apb", "smi";
+	};
-- 
1.8.1.1.dirty


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 3/6] iommu: add ARM short descriptor page table allocator
  2015-10-09  2:23 [PATCH v5 0/6] MT8173 IOMMU SUPPORT Yong Wu
  2015-10-09  2:23 ` [PATCH v5 1/6] dt-bindings: iommu: Add binding for mediatek IOMMU Yong Wu
  2015-10-09  2:23 ` [PATCH v5 2/6] dt-bindings: mediatek: Add smi dts binding Yong Wu
@ 2015-10-09  2:23 ` Yong Wu
  2015-10-14 12:54   ` Joerg Roedel
  2015-11-06  8:42   ` Yong Wu
  2015-10-09  2:23 ` [PATCH v5 4/6] memory: mediatek: Add SMI driver Yong Wu
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 23+ messages in thread
From: Yong Wu @ 2015-10-09  2:23 UTC (permalink / raw)
  To: Joerg Roedel, Thierry Reding, Mark Rutland, Matthias Brugger
  Cc: Robin Murphy, Will Deacon, Daniel Kurtz, Tomasz Figa,
	Lucas Stach, Rob Herring, Catalin Marinas, linux-mediatek,
	Sasha Hauer, srv_heupstream, devicetree, linux-kernel,
	linux-arm-kernel, iommu, pebolle, arnd, mitchelh, Sricharan R,
	youhua.li, k.zhang, kendrick.hsu, Yong Wu

This patch is for ARM Short Descriptor Format.

Signed-off-by: Yong Wu <yong.wu@mediatek.com>
---
 drivers/iommu/Kconfig                |  18 +
 drivers/iommu/Makefile               |   1 +
 drivers/iommu/io-pgtable-arm-short.c | 827 +++++++++++++++++++++++++++++++++++
 drivers/iommu/io-pgtable-arm.c       |   3 -
 drivers/iommu/io-pgtable.c           |   3 +
 drivers/iommu/io-pgtable.h           |  18 +-
 6 files changed, 866 insertions(+), 4 deletions(-)
 create mode 100644 drivers/iommu/io-pgtable-arm-short.c

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 4664c2a..a7920fb 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -40,6 +40,24 @@ config IOMMU_IO_PGTABLE_LPAE_SELFTEST
 
 	  If unsure, say N here.
 
+config IOMMU_IO_PGTABLE_SHORT
+	bool "ARMv7/v8 Short Descriptor Format"
+	select IOMMU_IO_PGTABLE
+	depends on HAS_DMA && (ARM || ARM64 || COMPILE_TEST)
+	help
+	  Enable support for the ARM Short-descriptor pagetable format.
+	  This allocator supports 2 levels of translation tables, which
+	  enables a 32-bit memory map based on memory sections or pages.
+
+config IOMMU_IO_PGTABLE_SHORT_SELFTEST
+	bool "Short Descriptor selftests"
+	depends on IOMMU_IO_PGTABLE_SHORT
+	help
+	  Enable self-tests for Short-descriptor page table allocator.
+	  This performs a series of page-table consistency checks during boot.
+
+	  If unsure, say N here.
+
 endmenu
 
 config IOMMU_IOVA
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index c6dcc51..06df3e6 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_IOMMU_API) += iommu-traces.o
 obj-$(CONFIG_IOMMU_API) += iommu-sysfs.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE) += io-pgtable.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o
+obj-$(CONFIG_IOMMU_IO_PGTABLE_SHORT) += io-pgtable-arm-short.o
 obj-$(CONFIG_IOMMU_IOVA) += iova.o
 obj-$(CONFIG_OF_IOMMU)	+= of_iommu.o
 obj-$(CONFIG_MSM_IOMMU) += msm_iommu.o msm_iommu_dev.o
diff --git a/drivers/iommu/io-pgtable-arm-short.c b/drivers/iommu/io-pgtable-arm-short.c
new file mode 100644
index 0000000..6337c61
--- /dev/null
+++ b/drivers/iommu/io-pgtable-arm-short.c
@@ -0,0 +1,827 @@
+/*
+ * Copyright (c) 2014-2015 MediaTek Inc.
+ * Author: Yong Wu <yong.wu@mediatek.com>
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/iommu.h>
+#include <linux/errno.h>
+#include "io-pgtable.h"
+
+typedef u32 arm_short_iopte;
+
+struct arm_short_io_pgtable {
+	struct io_pgtable	iop;
+	struct kmem_cache	*pgtable_cached;
+	size_t			pgd_size;
+	void			*pgd;
+};
+
+#define io_pgtable_to_data(x)			\
+	container_of((x), struct arm_short_io_pgtable, iop)
+
+#define io_pgtable_ops_to_data(x)		\
+	io_pgtable_to_data(io_pgtable_ops_to_pgtable(x))
+
+#define io_pgtable_cfg_to_pgtable(x)		\
+	container_of((x), struct io_pgtable, cfg)
+
+#define io_pgtable_cfg_to_data(x)		\
+	io_pgtable_to_data(io_pgtable_cfg_to_pgtable(x))
+
+#define ARM_SHORT_PGDIR_SHIFT			20
+#define ARM_SHORT_PAGE_SHIFT			12
+#define ARM_SHORT_PTRS_PER_PTE			\
+	(1 << (ARM_SHORT_PGDIR_SHIFT - ARM_SHORT_PAGE_SHIFT))
+#define ARM_SHORT_BYTES_PER_PTE			\
+	(ARM_SHORT_PTRS_PER_PTE * sizeof(arm_short_iopte))
+
+/* level 1 pagetable */
+#define ARM_SHORT_PGD_TYPE_PGTABLE		BIT(0)
+#define ARM_SHORT_PGD_TYPE_SECTION		BIT(1)
+#define ARM_SHORT_PGD_B				BIT(2)
+#define ARM_SHORT_PGD_C				BIT(3)
+#define ARM_SHORT_PGD_PGTABLE_NS		BIT(3)
+#define ARM_SHORT_PGD_SECTION_XN		BIT(4)
+#define ARM_SHORT_PGD_IMPLE			BIT(9)
+#define ARM_SHORT_PGD_RD_WR			(3 << 10)
+#define ARM_SHORT_PGD_RDONLY			BIT(15)
+#define ARM_SHORT_PGD_S				BIT(16)
+#define ARM_SHORT_PGD_nG			BIT(17)
+#define ARM_SHORT_PGD_SUPERSECTION		BIT(18)
+#define ARM_SHORT_PGD_SECTION_NS		BIT(19)
+
+#define ARM_SHORT_PGD_TYPE_SUPERSECTION		\
+	(ARM_SHORT_PGD_TYPE_SECTION | ARM_SHORT_PGD_SUPERSECTION)
+#define ARM_SHORT_PGD_SECTION_TYPE_MSK		\
+	(ARM_SHORT_PGD_TYPE_SECTION | ARM_SHORT_PGD_SUPERSECTION)
+#define ARM_SHORT_PGD_PGTABLE_TYPE_MSK		\
+	(ARM_SHORT_PGD_TYPE_SECTION | ARM_SHORT_PGD_TYPE_PGTABLE)
+#define ARM_SHORT_PGD_TYPE_IS_PGTABLE(pgd)	\
+	(((pgd) & ARM_SHORT_PGD_PGTABLE_TYPE_MSK) == ARM_SHORT_PGD_TYPE_PGTABLE)
+#define ARM_SHORT_PGD_TYPE_IS_SECTION(pgd)	\
+	(((pgd) & ARM_SHORT_PGD_SECTION_TYPE_MSK) == ARM_SHORT_PGD_TYPE_SECTION)
+#define ARM_SHORT_PGD_TYPE_IS_SUPERSECTION(pgd)	\
+	(((pgd) & ARM_SHORT_PGD_SECTION_TYPE_MSK) == \
+	ARM_SHORT_PGD_TYPE_SUPERSECTION)
+#define ARM_SHORT_PGD_PGTABLE_MSK		(~(ARM_SHORT_BYTES_PER_PTE - 1))
+#define ARM_SHORT_PGD_SECTION_MSK		(~(SZ_1M - 1))
+#define ARM_SHORT_PGD_SUPERSECTION_MSK		(~(SZ_16M - 1))
+
+/* level 2 pagetable */
+#define ARM_SHORT_PTE_TYPE_LARGE		BIT(0)
+#define ARM_SHORT_PTE_SMALL_XN			BIT(0)
+#define ARM_SHORT_PTE_TYPE_SMALL		BIT(1)
+#define ARM_SHORT_PTE_B				BIT(2)
+#define ARM_SHORT_PTE_C				BIT(3)
+#define ARM_SHORT_PTE_RD_WR			(3 << 4)
+#define ARM_SHORT_PTE_RDONLY			BIT(9)
+#define ARM_SHORT_PTE_S				BIT(10)
+#define ARM_SHORT_PTE_nG			BIT(11)
+#define ARM_SHORT_PTE_LARGE_XN			BIT(15)
+#define ARM_SHORT_PTE_LARGE_MSK			(~(SZ_64K - 1))
+#define ARM_SHORT_PTE_SMALL_MSK			(~(SZ_4K - 1))
+#define ARM_SHORT_PTE_TYPE_MSK			\
+	(ARM_SHORT_PTE_TYPE_LARGE | ARM_SHORT_PTE_TYPE_SMALL)
+/* Bit[0] in small page is the XN bit */
+#define ARM_SHORT_PTE_TYPE_IS_SMALLPAGE(pte)	\
+	(((pte) & ARM_SHORT_PTE_TYPE_SMALL) == ARM_SHORT_PTE_TYPE_SMALL)
+#define ARM_SHORT_PTE_TYPE_IS_LARGEPAGE(pte)	\
+	(((pte) & ARM_SHORT_PTE_TYPE_MSK) == ARM_SHORT_PTE_TYPE_LARGE)
+
+#define ARM_SHORT_PGD_IDX(a)			((a) >> ARM_SHORT_PGDIR_SHIFT)
+#define ARM_SHORT_PTE_IDX(a)			\
+	(((a) >> ARM_SHORT_PAGE_SHIFT) & (ARM_SHORT_PTRS_PER_PTE - 1))
+
+#define ARM_SHORT_GET_PGTABLE_VA(pgd)		\
+	(phys_to_virt((pgd) & ARM_SHORT_PGD_PGTABLE_MSK))
+
+/* Get the prot of large page for split */
+#define ARM_SHORT_PTE_GET_PROT_LARGE(pte)	\
+	(((pte) & (~ARM_SHORT_PTE_LARGE_MSK)) & ~ARM_SHORT_PTE_TYPE_MSK)
+
+#define ARM_SHORT_PGD_GET_PROT(pgd)		\
+	(((pgd) & (~ARM_SHORT_PGD_SECTION_MSK)) & ~ARM_SHORT_PGD_SUPERSECTION)
+
+static bool selftest_running;
+
+static arm_short_iopte *
+arm_short_get_pte_in_pgd(arm_short_iopte pgd, unsigned int iova)
+{
+	arm_short_iopte *pte;
+
+	pte = ARM_SHORT_GET_PGTABLE_VA(pgd);
+	pte += ARM_SHORT_PTE_IDX(iova);
+	return pte;
+}
+
+static dma_addr_t __arm_short_dma_addr(void *va)
+{
+	return (dma_addr_t)virt_to_phys(va);
+}
+
+static int
+__arm_short_set_pte(arm_short_iopte *ptep, arm_short_iopte pte,
+		    unsigned int ptenr, struct io_pgtable_cfg *cfg)
+{
+	int i;
+
+	for (i = 0; i < ptenr; i++) {
+		if (ptep[i] && pte) {
+			/* Someone else may have allocated for this pte */
+			WARN_ON(!selftest_running);
+			goto err_exist_pte;
+		}
+		ptep[i] = pte;
+	}
+
+	if (selftest_running)
+		return 0;
+
+	dma_sync_single_for_device(cfg->iommu_dev, __arm_short_dma_addr(ptep),
+				   sizeof(*ptep) * ptenr, DMA_TO_DEVICE);
+	return 0;
+
+err_exist_pte:
+	while (i--) {
+		ptep[i] = 0;
+		if (!selftest_running)
+			dma_sync_single_for_device(
+				cfg->iommu_dev, __arm_short_dma_addr(ptep + i),
+				sizeof(*ptep), DMA_TO_DEVICE);
+	}
+	return -EEXIST;
+}
+
+static void *
+__arm_short_alloc_pgtable(size_t size, gfp_t gfp, bool pgd,
+			  struct io_pgtable_cfg *cfg)
+{
+	struct arm_short_io_pgtable *data;
+	struct device *dev = cfg->iommu_dev;
+	dma_addr_t dma;
+	void *va;
+
+	if (pgd) {/* lvl1 pagetable */
+		va = alloc_pages_exact(size, gfp);
+	} else {  /* lvl2 pagetable */
+		data = io_pgtable_cfg_to_data(cfg);
+		va = kmem_cache_zalloc(data->pgtable_cached, gfp);
+	}
+
+	if (!va)
+		return NULL;
+
+	if (selftest_running)
+		return va;
+
+	dma = dma_map_single(dev, va, size, DMA_TO_DEVICE);
+	if (dma_mapping_error(dev, dma))
+		goto out_free;
+
+	if (dma != virt_to_phys(va))
+		goto out_unmap;
+
+	if (!pgd)
+		kmemleak_ignore(va);
+
+	return va;
+
+out_unmap:
+	dev_err(dev, "Cannot accommodate DMA translation for IOMMU page tables\n");
+	dma_unmap_single(dev, dma, size, DMA_TO_DEVICE);
+out_free:
+	if (pgd)
+		free_pages_exact(va, size);
+	else
+		kmem_cache_free(data->pgtable_cached, va);
+	return NULL;
+}
+
+static void
+__arm_short_free_pgtable(void *va, size_t size, bool pgd,
+			 struct io_pgtable_cfg *cfg)
+{
+	struct arm_short_io_pgtable *data;
+
+	if (!selftest_running)
+		dma_unmap_single(cfg->iommu_dev, __arm_short_dma_addr(va),
+				 size, DMA_TO_DEVICE);
+
+	if (pgd) {
+		free_pages_exact(va, size);
+	} else {
+		data = io_pgtable_cfg_to_data(cfg);
+		kmem_cache_free(data->pgtable_cached, va);
+	}
+}
+
+static arm_short_iopte
+__arm_short_pte_prot(struct arm_short_io_pgtable *data, int prot, bool large)
+{
+	arm_short_iopte pteprot;
+	int quirk = data->iop.cfg.quirks;
+
+	pteprot = ARM_SHORT_PTE_S | ARM_SHORT_PTE_nG;
+	pteprot |= large ? ARM_SHORT_PTE_TYPE_LARGE :
+				ARM_SHORT_PTE_TYPE_SMALL;
+	if (prot & IOMMU_CACHE)
+		pteprot |=  ARM_SHORT_PTE_B | ARM_SHORT_PTE_C;
+
+	if (!(quirk & IO_PGTABLE_QUIRK_NO_PERMS)) {
+		if (prot & IOMMU_NOEXEC) {
+			pteprot |= large ? ARM_SHORT_PTE_LARGE_XN :
+				ARM_SHORT_PTE_SMALL_XN;
+		}
+		pteprot |= ARM_SHORT_PTE_RD_WR;
+		if (!(prot & IOMMU_WRITE) && (prot & IOMMU_READ))
+			pteprot |= ARM_SHORT_PTE_RDONLY;
+	}
+
+	return pteprot;
+}
+
+static arm_short_iopte
+__arm_short_pgd_prot(struct arm_short_io_pgtable *data, int prot, bool super)
+{
+	arm_short_iopte pgdprot;
+	int quirk = data->iop.cfg.quirks;
+
+	pgdprot = ARM_SHORT_PGD_S | ARM_SHORT_PGD_nG;
+	pgdprot |= super ? ARM_SHORT_PGD_TYPE_SUPERSECTION :
+				ARM_SHORT_PGD_TYPE_SECTION;
+	if (prot & IOMMU_CACHE)
+		pgdprot |= ARM_SHORT_PGD_C | ARM_SHORT_PGD_B;
+	if (quirk & IO_PGTABLE_QUIRK_ARM_NS)
+		pgdprot |= ARM_SHORT_PGD_SECTION_NS;
+
+	if (!(quirk & IO_PGTABLE_QUIRK_NO_PERMS)) {
+		if (prot & IOMMU_NOEXEC)
+			pgdprot |= ARM_SHORT_PGD_SECTION_XN;
+		pgdprot |= ARM_SHORT_PGD_RD_WR;
+		if (!(prot & IOMMU_WRITE) && (prot & IOMMU_READ))
+			pgdprot |= ARM_SHORT_PGD_RDONLY;
+	}
+
+	return pgdprot;
+}
+
+static arm_short_iopte
+__arm_short_pte_prot_split(struct arm_short_io_pgtable *data,
+			   arm_short_iopte pgdprot,
+			   arm_short_iopte pteprot_large,
+			   bool large)
+{
+	arm_short_iopte pteprot = 0;
+
+	pteprot = ARM_SHORT_PTE_S | ARM_SHORT_PTE_nG;
+	pteprot |= large ? ARM_SHORT_PTE_TYPE_LARGE :
+				ARM_SHORT_PTE_TYPE_SMALL;
+
+	/* Get the pte prot while large page split to small page */
+	if (!pgdprot && !large) {
+		pteprot |= pteprot_large & ~ARM_SHORT_PTE_SMALL_MSK;
+		if (pteprot_large & ARM_SHORT_PTE_LARGE_XN)
+			pteprot |= ARM_SHORT_PTE_SMALL_XN;
+	}
+
+	/* Section to pte prot */
+	if (pgdprot & ARM_SHORT_PGD_C)
+		pteprot |= ARM_SHORT_PTE_C;
+	if (pgdprot & ARM_SHORT_PGD_B)
+		pteprot |= ARM_SHORT_PTE_B;
+	if (pgdprot & ARM_SHORT_PGD_nG)
+		pteprot |= ARM_SHORT_PTE_nG;
+	if (pgdprot & ARM_SHORT_PGD_SECTION_XN)
+		pteprot |= large ? ARM_SHORT_PTE_LARGE_XN :
+				ARM_SHORT_PTE_SMALL_XN;
+	if (pgdprot & ARM_SHORT_PGD_RD_WR)
+		pteprot |= ARM_SHORT_PTE_RD_WR;
+	if (pgdprot & ARM_SHORT_PGD_RDONLY)
+		pteprot |= ARM_SHORT_PTE_RDONLY;
+
+	return pteprot;
+}
+
+static arm_short_iopte
+__arm_short_pgtable_prot(struct arm_short_io_pgtable *data)
+{
+	arm_short_iopte pgdprot = 0;
+
+	pgdprot = ARM_SHORT_PGD_TYPE_PGTABLE;
+	if (data->iop.cfg.quirks & IO_PGTABLE_QUIRK_ARM_NS)
+		pgdprot |= ARM_SHORT_PGD_PGTABLE_NS;
+	return pgdprot;
+}
+
+static int
+_arm_short_map(struct arm_short_io_pgtable *data,
+	       unsigned int iova, size_t size, phys_addr_t paddr,
+	       arm_short_iopte pgdprot, arm_short_iopte pteprot)
+{
+	struct io_pgtable_cfg *cfg = &data->iop.cfg;
+	const struct iommu_gather_ops *tlb = cfg->tlb;
+	arm_short_iopte *pgd = data->pgd, *pte;
+	void *cookie = data->iop.cookie, *pgtable_new = NULL;
+	unsigned int pte_nr;
+	int ret;
+
+	pgd += ARM_SHORT_PGD_IDX(iova);
+
+	if (!pteprot) { /* section or supersection */
+		pte = pgd;
+		pteprot = pgdprot;
+		pte_nr = (size == SZ_1M) ? 1 : 16;
+	} else {        /* page or largepage */
+		if (!(*pgd)) {
+			pgtable_new = __arm_short_alloc_pgtable(
+					ARM_SHORT_BYTES_PER_PTE,
+					GFP_ATOMIC, false, cfg);
+			if (unlikely(!pgtable_new))
+				return -ENOMEM;
+			pgdprot |= virt_to_phys(pgtable_new);
+			__arm_short_set_pte(pgd, pgdprot, 1, cfg);
+		}
+		pte = arm_short_get_pte_in_pgd(*pgd, iova);
+		pte_nr = (size == SZ_4K) ? 1 : 16;
+	}
+
+	pteprot |= (arm_short_iopte)paddr;
+	ret = __arm_short_set_pte(pte, pteprot, pte_nr, cfg);
+	if (ret && pgtable_new)
+		goto err_unmap_pgd;
+
+	if (cfg->quirks & IO_PGTABLE_QUIRK_TLBI_ON_MAP) {
+		tlb->tlb_add_flush(iova, size, true, cookie);
+		tlb->tlb_sync(cookie);
+	}
+	return ret;
+
+err_unmap_pgd:
+	__arm_short_set_pte(pgd, 0, 1, cfg);
+	tlb->tlb_add_flush(iova, SZ_1M, false, cookie);/* Flush whole the pgd */
+	tlb->tlb_sync(cookie);
+	__arm_short_free_pgtable(pgtable_new, ARM_SHORT_BYTES_PER_PTE,
+				 false, cfg);
+	return ret;
+}
+
+static int arm_short_map(struct io_pgtable_ops *ops, unsigned long iova,
+			 phys_addr_t paddr, size_t size, int prot)
+{
+	struct arm_short_io_pgtable *data = io_pgtable_ops_to_data(ops);
+	arm_short_iopte pgdprot = 0, pteprot = 0;
+	bool large;
+
+	/* If no access, then nothing to do */
+	if (!(prot & (IOMMU_READ | IOMMU_WRITE)))
+		return 0;
+
+	if (WARN_ON((iova | paddr) & (size - 1)))
+		return -EINVAL;
+
+	switch (size) {
+	case SZ_4K:
+	case SZ_64K:
+		large = (size == SZ_64K) ? true : false;
+		pteprot = __arm_short_pte_prot(data, prot, large);
+		pgdprot = __arm_short_pgtable_prot(data);
+		break;
+
+	case SZ_1M:
+	case SZ_16M:
+		large = (size == SZ_16M) ? true : false;
+		pgdprot = __arm_short_pgd_prot(data, prot, large);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return _arm_short_map(data, iova, size, paddr, pgdprot, pteprot);
+}
+
+static phys_addr_t arm_short_iova_to_phys(struct io_pgtable_ops *ops,
+					  unsigned long iova)
+{
+	struct arm_short_io_pgtable *data = io_pgtable_ops_to_data(ops);
+	arm_short_iopte *pte, *pgd = data->pgd;
+	phys_addr_t pa = 0;
+
+	pgd += ARM_SHORT_PGD_IDX(iova);
+
+	if (ARM_SHORT_PGD_TYPE_IS_PGTABLE(*pgd)) {
+		pte = arm_short_get_pte_in_pgd(*pgd, iova);
+
+		if (ARM_SHORT_PTE_TYPE_IS_LARGEPAGE(*pte)) {
+			pa = (*pte) & ARM_SHORT_PTE_LARGE_MSK;
+			pa |= iova & ~ARM_SHORT_PTE_LARGE_MSK;
+		} else if (ARM_SHORT_PTE_TYPE_IS_SMALLPAGE(*pte)) {
+			pa = (*pte) & ARM_SHORT_PTE_SMALL_MSK;
+			pa |= iova & ~ARM_SHORT_PTE_SMALL_MSK;
+		}
+	} else if (ARM_SHORT_PGD_TYPE_IS_SECTION(*pgd)) {
+		pa = (*pgd) & ARM_SHORT_PGD_SECTION_MSK;
+		pa |= iova & ~ARM_SHORT_PGD_SECTION_MSK;
+	} else if (ARM_SHORT_PGD_TYPE_IS_SUPERSECTION(*pgd)) {
+		pa = (*pgd) & ARM_SHORT_PGD_SUPERSECTION_MSK;
+		pa |= iova & ~ARM_SHORT_PGD_SUPERSECTION_MSK;
+	}
+
+	return pa;
+}
+
+static bool __arm_short_pgtable_empty(arm_short_iopte *pgd)
+{
+	arm_short_iopte *pte;
+	int i;
+
+	pte = ARM_SHORT_GET_PGTABLE_VA(*pgd);
+	for (i = 0; i < ARM_SHORT_PTRS_PER_PTE; i++) {
+		if (pte[i])
+			return false;
+	}
+
+	return true;
+}
+
+static int
+arm_short_split_blk_unmap(struct io_pgtable_ops *ops, unsigned int iova,
+			  size_t size, size_t blk_size,
+			  arm_short_iopte pgdprotup, arm_short_iopte pteprotup)
+{
+	struct arm_short_io_pgtable *data = io_pgtable_ops_to_data(ops);
+	struct io_pgtable_cfg *cfg = &data->iop.cfg;
+	unsigned long *pgbitmap = &cfg->pgsize_bitmap;
+	unsigned int blk_base, blk_start, blk_end, i;
+	arm_short_iopte pgdprot, pteprot;
+	phys_addr_t blk_paddr;
+	size_t mapsize = 0, nextmapsize;
+	int ret;
+
+	/* Find the nearest mapsize */
+	for (i = find_first_bit(pgbitmap, BITS_PER_LONG);
+	     i < BITS_PER_LONG && ((1 << i) < blk_size) &&
+	     IS_ALIGNED(size, 1 << i);
+	     i = find_next_bit(pgbitmap, BITS_PER_LONG, i + 1))
+		mapsize = 1 << i;
+
+	if (WARN_ON(!mapsize))
+		return 0; /* Bytes unmapped */
+	nextmapsize = 1 << i;
+
+	blk_base = iova & ~(blk_size - 1);
+	blk_start = blk_base;
+	blk_end = blk_start + blk_size;
+	blk_paddr = arm_short_iova_to_phys(ops, blk_base);
+
+	for (; blk_start < blk_end;
+	     blk_start += mapsize, blk_paddr += mapsize) {
+		/* Unmap! */
+		if (blk_start == iova)
+			continue;
+
+		/* Try to upper map */
+		if (blk_base != blk_start &&
+		    IS_ALIGNED(blk_start | blk_paddr, nextmapsize) &&
+		    mapsize != nextmapsize) {
+			mapsize = nextmapsize;
+			i = find_next_bit(pgbitmap, BITS_PER_LONG, i + 1);
+			if (i < BITS_PER_LONG)
+				nextmapsize = 1 << i;
+		}
+
+		if (mapsize == SZ_1M) {
+			pgdprot = pgdprotup;
+			pgdprot |= __arm_short_pgd_prot(data, 0, false);
+			pteprot = 0;
+		} else { /* small or large page */
+			pgdprot = (blk_size == SZ_64K) ? 0 : pgdprotup;
+			pteprot = __arm_short_pte_prot_split(
+					data, pgdprot, pteprotup,
+					mapsize == SZ_64K);
+			pgdprot = __arm_short_pgtable_prot(data);
+		}
+
+		ret = _arm_short_map(data, blk_start, mapsize,
+				     blk_paddr, pgdprot, pteprot);
+		if (ret)
+			return 0;/* Bytes unmapped */
+	}
+
+	return size;
+}
+
+static int arm_short_unmap(struct io_pgtable_ops *ops,
+			   unsigned long iova,
+			   size_t size)
+{
+	struct arm_short_io_pgtable *data = io_pgtable_ops_to_data(ops);
+	struct io_pgtable_cfg *cfg = &data->iop.cfg;
+	void *cookie = data->iop.cookie;
+	arm_short_iopte *pgd_base = data->pgd;
+	arm_short_iopte *pgd, *pte = NULL;
+	arm_short_iopte pgd_tmp, pte_tmp = 0;
+	unsigned int blk_base, blk_size;
+	int unmap_size = 0;
+	bool pgtempty;
+
+	do {
+		pgd = pgd_base + ARM_SHORT_PGD_IDX(iova);
+		blk_size = 0;
+		pgtempty = false;
+
+		/* Get block size */
+		if (ARM_SHORT_PGD_TYPE_IS_PGTABLE(*pgd)) {
+			pte = arm_short_get_pte_in_pgd(*pgd, iova);
+
+			if (ARM_SHORT_PTE_TYPE_IS_SMALLPAGE(*pte))
+				blk_size = SZ_4K;
+			else if (ARM_SHORT_PTE_TYPE_IS_LARGEPAGE(*pte))
+				blk_size = SZ_64K;
+		} else if (ARM_SHORT_PGD_TYPE_IS_SECTION(*pgd)) {
+			blk_size = SZ_1M;
+		} else if (ARM_SHORT_PGD_TYPE_IS_SUPERSECTION(*pgd)) {
+			blk_size = SZ_16M;
+		}
+
+		if (WARN_ON(!blk_size))
+			return 0;
+
+		/* Unmap the pgd/pte of the block base */
+		blk_base = iova & ~(blk_size - 1);
+		pgd = pgd_base + ARM_SHORT_PGD_IDX(blk_base);
+		pgd_tmp = *pgd;
+
+		if (blk_size == SZ_4K || blk_size == SZ_64K) {
+			pte = arm_short_get_pte_in_pgd(*pgd, blk_base);
+			pte_tmp = *pte;
+			__arm_short_set_pte(pte, 0, blk_size / SZ_4K, cfg);
+
+			pgtempty = __arm_short_pgtable_empty(pgd);
+			if (pgtempty)
+				__arm_short_set_pte(pgd, 0, 1, cfg);
+		} else if (blk_size == SZ_1M || blk_size == SZ_16M) {
+			__arm_short_set_pte(pgd, 0, blk_size / SZ_1M, cfg);
+		}
+
+		cfg->tlb->tlb_add_flush(blk_base, blk_size, true, cookie);
+		cfg->tlb->tlb_sync(cookie);
+
+		if (pgtempty)/* Free lvl2 pgtable after tlb-flush */
+			__arm_short_free_pgtable(
+					ARM_SHORT_GET_PGTABLE_VA(pgd_tmp),
+					ARM_SHORT_BYTES_PER_PTE, false, cfg);
+
+		/*
+		 * If the unmap size that from the pgsize_bitmap is more
+		 * than the current blk_size, unmap it continuously.
+		 */
+		if (blk_size <= size) {
+			iova += blk_size;
+			size -= blk_size;
+			unmap_size += blk_size;
+			continue;
+		} else { /* Split this block */
+			return arm_short_split_blk_unmap(
+					ops, iova, size, blk_size,
+					ARM_SHORT_PGD_GET_PROT(pgd_tmp),
+					ARM_SHORT_PTE_GET_PROT_LARGE(pte_tmp));
+		}
+	} while (size);
+
+	return unmap_size;
+}
+
+static struct io_pgtable *
+arm_short_alloc_pgtable(struct io_pgtable_cfg *cfg, void *cookie)
+{
+	struct arm_short_io_pgtable *data;
+
+	if (cfg->ias > 32 || cfg->oas > 32)
+		return NULL;
+
+	cfg->pgsize_bitmap &=
+		(cfg->quirks & IO_PGTABLE_QUIRK_SHORT_SUPERSECTION) ?
+		(SZ_4K | SZ_64K | SZ_1M | SZ_16M) : (SZ_4K | SZ_64K | SZ_1M);
+
+	if (!selftest_running && cfg->iommu_dev->dma_pfn_offset) {
+		dev_err(cfg->iommu_dev, "Cannot accommodate DMA offset for IOMMU page tables\n");
+		return NULL;
+	}
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return NULL;
+
+	data->pgd_size = SZ_16K;
+	data->pgd = __arm_short_alloc_pgtable(
+					data->pgd_size,
+					GFP_KERNEL | __GFP_ZERO | GFP_DMA,
+					true, cfg);
+	if (!data->pgd)
+		goto out_free_data;
+	wmb();/* Ensure the empty pgd is visible before any actual TTBR write */
+
+	data->pgtable_cached = kmem_cache_create(
+					"io-pgtable-arm-short",
+					 ARM_SHORT_BYTES_PER_PTE,
+					 ARM_SHORT_BYTES_PER_PTE,
+					 SLAB_CACHE_DMA, NULL);
+	if (!data->pgtable_cached)
+		goto out_free_pgd;
+
+	/* TTBRs */
+	cfg->arm_short_cfg.ttbr[0] = virt_to_phys(data->pgd);
+	cfg->arm_short_cfg.ttbr[1] = 0;
+	cfg->arm_short_cfg.tcr = 0;
+	cfg->arm_short_cfg.nmrr = 0;
+	cfg->arm_short_cfg.prrr = 0;
+	/* The access flag is not supported as SCTLR isn't implemented */
+
+	data->iop.ops = (struct io_pgtable_ops) {
+		.map		= arm_short_map,
+		.unmap		= arm_short_unmap,
+		.iova_to_phys	= arm_short_iova_to_phys,
+	};
+
+	return &data->iop;
+
+out_free_pgd:
+	__arm_short_free_pgtable(data->pgd, data->pgd_size, true, cfg);
+out_free_data:
+	kfree(data);
+	return NULL;
+}
+
+static void arm_short_free_pgtable(struct io_pgtable *iop)
+{
+	struct arm_short_io_pgtable *data = io_pgtable_to_data(iop);
+
+	kmem_cache_destroy(data->pgtable_cached);
+	__arm_short_free_pgtable(data->pgd, data->pgd_size,
+				 true, &data->iop.cfg);
+	kfree(data);
+}
+
+struct io_pgtable_init_fns io_pgtable_arm_short_init_fns = {
+	.alloc	= arm_short_alloc_pgtable,
+	.free	= arm_short_free_pgtable,
+};
+
+#ifdef CONFIG_IOMMU_IO_PGTABLE_SHORT_SELFTEST
+
+static struct io_pgtable_cfg *cfg_cookie;
+
+static void dummy_tlb_flush_all(void *cookie)
+{
+	WARN_ON(cookie != cfg_cookie);
+}
+
+static void dummy_tlb_add_flush(unsigned long iova, size_t size, bool leaf,
+				void *cookie)
+{
+	WARN_ON(cookie != cfg_cookie);
+	WARN_ON(!(size & cfg_cookie->pgsize_bitmap));
+}
+
+static void dummy_tlb_sync(void *cookie)
+{
+	WARN_ON(cookie != cfg_cookie);
+}
+
+static struct iommu_gather_ops dummy_tlb_ops = {
+	.tlb_flush_all	= dummy_tlb_flush_all,
+	.tlb_add_flush	= dummy_tlb_add_flush,
+	.tlb_sync	= dummy_tlb_sync,
+};
+
+#define __FAIL(ops)	({				\
+		WARN(1, "selftest: test failed\n");	\
+		selftest_running = false;		\
+		-EFAULT;				\
+})
+
+static int __init arm_short_do_selftests(void)
+{
+	struct io_pgtable_ops *ops;
+	struct io_pgtable_cfg cfg = {
+		.tlb = &dummy_tlb_ops,
+		.oas = 32,
+		.ias = 32,
+		.quirks = IO_PGTABLE_QUIRK_ARM_NS |
+			IO_PGTABLE_QUIRK_SHORT_SUPERSECTION,
+		.pgsize_bitmap = SZ_4K | SZ_64K | SZ_1M | SZ_16M,
+	};
+	unsigned int iova, size, iova_start;
+	unsigned int i, loopnr = 0;
+
+	selftest_running = true;
+
+	cfg_cookie = &cfg;
+
+	ops = alloc_io_pgtable_ops(ARM_SHORT_DESC, &cfg, &cfg);
+	if (!ops) {
+		pr_err("Failed to alloc short desc io pgtable\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * Initial sanity checks.
+	 * Empty page tables shouldn't provide any translations.
+	 */
+	if (ops->iova_to_phys(ops, 42))
+		return __FAIL(ops);
+
+	if (ops->iova_to_phys(ops, SZ_1G + 42))
+		return __FAIL(ops);
+
+	if (ops->iova_to_phys(ops, SZ_2G + 42))
+		return __FAIL(ops);
+
+	/*
+	 * Distinct mappings of different granule sizes.
+	 */
+	iova = 0;
+	i = find_first_bit(&cfg.pgsize_bitmap, BITS_PER_LONG);
+	while (i != BITS_PER_LONG) {
+		size = 1UL << i;
+		if (ops->map(ops, iova, iova, size, IOMMU_READ |
+						    IOMMU_WRITE |
+						    IOMMU_NOEXEC |
+						    IOMMU_CACHE))
+			return __FAIL(ops);
+
+		/* Overlapping mappings */
+		if (!ops->map(ops, iova, iova + size, size,
+			      IOMMU_READ | IOMMU_NOEXEC))
+			return __FAIL(ops);
+
+		if (ops->iova_to_phys(ops, iova + 42) != (iova + 42))
+			return __FAIL(ops);
+
+		iova += SZ_16M;
+		i++;
+		i = find_next_bit(&cfg.pgsize_bitmap, BITS_PER_LONG, i);
+		loopnr++;
+	}
+
+	/* Partial unmap */
+	i = 1;
+	size = 1UL << __ffs(cfg.pgsize_bitmap);
+	while (i < loopnr) {
+		iova_start = i * SZ_16M;
+		if (ops->unmap(ops, iova_start + size, size) != size)
+			return __FAIL(ops);
+
+		/* Remap of partial unmap */
+		if (ops->map(ops, iova_start + size, size, size, IOMMU_READ))
+			return __FAIL(ops);
+
+		if (ops->iova_to_phys(ops, iova_start + size + 42)
+		    != (size + 42))
+			return __FAIL(ops);
+		i++;
+	}
+
+	/* Full unmap */
+	iova = 0;
+	i = find_first_bit(&cfg.pgsize_bitmap, BITS_PER_LONG);
+	while (i != BITS_PER_LONG) {
+		size = 1UL << i;
+
+		if (ops->unmap(ops, iova, size) != size)
+			return __FAIL(ops);
+
+		if (ops->iova_to_phys(ops, iova + 42))
+			return __FAIL(ops);
+
+		/* Remap full block */
+		if (ops->map(ops, iova, iova, size, IOMMU_WRITE))
+			return __FAIL(ops);
+
+		if (ops->iova_to_phys(ops, iova + 42) != (iova + 42))
+			return __FAIL(ops);
+
+		iova += SZ_16M;
+		i++;
+		i = find_next_bit(&cfg.pgsize_bitmap, BITS_PER_LONG, i);
+	}
+
+	free_io_pgtable_ops(ops);
+
+	selftest_running = false;
+
+	pr_info("arm-short io-pgtable: self test ok\n");
+	return 0;
+}
+subsys_initcall(arm_short_do_selftests);
+#endif
diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 73c0748..0d7bcfc 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -38,9 +38,6 @@
 #define io_pgtable_to_data(x)						\
 	container_of((x), struct arm_lpae_io_pgtable, iop)
 
-#define io_pgtable_ops_to_pgtable(x)					\
-	container_of((x), struct io_pgtable, ops)
-
 #define io_pgtable_ops_to_data(x)					\
 	io_pgtable_to_data(io_pgtable_ops_to_pgtable(x))
 
diff --git a/drivers/iommu/io-pgtable.c b/drivers/iommu/io-pgtable.c
index 6f2e319..e7b0b1a 100644
--- a/drivers/iommu/io-pgtable.c
+++ b/drivers/iommu/io-pgtable.c
@@ -33,6 +33,9 @@ io_pgtable_init_table[IO_PGTABLE_NUM_FMTS] =
 	[ARM_64_LPAE_S1] = &io_pgtable_arm_64_lpae_s1_init_fns,
 	[ARM_64_LPAE_S2] = &io_pgtable_arm_64_lpae_s2_init_fns,
 #endif
+#ifdef CONFIG_IOMMU_IO_PGTABLE_SHORT
+	[ARM_SHORT_DESC] = &io_pgtable_arm_short_init_fns,
+#endif
 };
 
 struct io_pgtable_ops *alloc_io_pgtable_ops(enum io_pgtable_fmt fmt,
diff --git a/drivers/iommu/io-pgtable.h b/drivers/iommu/io-pgtable.h
index ac9e234..af09467 100644
--- a/drivers/iommu/io-pgtable.h
+++ b/drivers/iommu/io-pgtable.h
@@ -9,6 +9,7 @@ enum io_pgtable_fmt {
 	ARM_32_LPAE_S2,
 	ARM_64_LPAE_S1,
 	ARM_64_LPAE_S2,
+	ARM_SHORT_DESC,
 	IO_PGTABLE_NUM_FMTS,
 };
 
@@ -45,7 +46,10 @@ struct iommu_gather_ops {
  *                 page table walker.
  */
 struct io_pgtable_cfg {
-	#define IO_PGTABLE_QUIRK_ARM_NS	(1 << 0)	/* Set NS bit in PTEs */
+	#define IO_PGTABLE_QUIRK_ARM_NS		BIT(0) /* Set NS bit in PTEs */
+	#define IO_PGTABLE_QUIRK_NO_PERMS	BIT(1) /* No AP/XN bits */
+	#define IO_PGTABLE_QUIRK_TLBI_ON_MAP	BIT(2) /* TLB Inv. on map */
+	#define IO_PGTABLE_QUIRK_SHORT_SUPERSECTION	BIT(3)
 	int				quirks;
 	unsigned long			pgsize_bitmap;
 	unsigned int			ias;
@@ -65,6 +69,14 @@ struct io_pgtable_cfg {
 			u64	vttbr;
 			u64	vtcr;
 		} arm_lpae_s2_cfg;
+
+		struct {
+			u32	ttbr[2];
+			u32	tcr;
+			u32	nmrr;
+			u32	prrr;
+			u32	sctlr;
+		} arm_short_cfg;
 	};
 };
 
@@ -131,6 +143,9 @@ struct io_pgtable {
 	struct io_pgtable_ops	ops;
 };
 
+#define io_pgtable_ops_to_pgtable(x)		\
+	container_of((x), struct io_pgtable, ops)
+
 /**
  * struct io_pgtable_init_fns - Alloc/free a set of page tables for a
  *                              particular format.
@@ -147,5 +162,6 @@ extern struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s1_init_fns;
 extern struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s2_init_fns;
 extern struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s1_init_fns;
 extern struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s2_init_fns;
+extern struct io_pgtable_init_fns io_pgtable_arm_short_init_fns;
 
 #endif /* __IO_PGTABLE_H */
-- 
1.8.1.1.dirty


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 4/6] memory: mediatek: Add SMI driver
  2015-10-09  2:23 [PATCH v5 0/6] MT8173 IOMMU SUPPORT Yong Wu
                   ` (2 preceding siblings ...)
  2015-10-09  2:23 ` [PATCH v5 3/6] iommu: add ARM short descriptor page table allocator Yong Wu
@ 2015-10-09  2:23 ` Yong Wu
  2015-10-27 13:24   ` Robin Murphy
  2015-10-09  2:23 ` [PATCH v5 5/6] iommu/mediatek: Add mt8173 IOMMU driver Yong Wu
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 23+ messages in thread
From: Yong Wu @ 2015-10-09  2:23 UTC (permalink / raw)
  To: Joerg Roedel, Thierry Reding, Mark Rutland, Matthias Brugger
  Cc: Robin Murphy, Will Deacon, Daniel Kurtz, Tomasz Figa,
	Lucas Stach, Rob Herring, Catalin Marinas, linux-mediatek,
	Sasha Hauer, srv_heupstream, devicetree, linux-kernel,
	linux-arm-kernel, iommu, pebolle, arnd, mitchelh, Sricharan R,
	youhua.li, k.zhang, kendrick.hsu, Yong Wu

This patch add SMI(Smart Multimedia Interface) driver. This driver
is responsible to enable/disable iommu and control the clocks of each
local arbiter

Signed-off-by: Yong Wu <yong.wu@mediatek.com>
---
 drivers/memory/Kconfig     |   8 ++
 drivers/memory/Makefile    |   1 +
 drivers/memory/mtk-smi.c   | 274 +++++++++++++++++++++++++++++++++++++++++++++
 include/soc/mediatek/smi.h |  60 ++++++++++
 4 files changed, 343 insertions(+)
 create mode 100644 drivers/memory/mtk-smi.c
 create mode 100644 include/soc/mediatek/smi.h

diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
index c6a644b..6eab27c 100644
--- a/drivers/memory/Kconfig
+++ b/drivers/memory/Kconfig
@@ -108,6 +108,14 @@ config JZ4780_NEMC
 	  the Ingenic JZ4780. This controller is used to handle external
 	  memory devices such as NAND and SRAM.
 
+config MTK_SMI
+	bool
+	depends on ARCH_MEDIATEK || COMPILE_TEST
+	help
+	  This driver is for the Memory Controller module in MediaTek SoCs,
+	  mainly help enable/disable iommu and control the clock for each
+	  local arbiter.
+
 source "drivers/memory/tegra/Kconfig"
 
 endif
diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
index 1c46af5..890bdf4 100644
--- a/drivers/memory/Makefile
+++ b/drivers/memory/Makefile
@@ -15,5 +15,6 @@ obj-$(CONFIG_FSL_IFC)		+= fsl_ifc.o
 obj-$(CONFIG_MVEBU_DEVBUS)	+= mvebu-devbus.o
 obj-$(CONFIG_TEGRA20_MC)	+= tegra20-mc.o
 obj-$(CONFIG_JZ4780_NEMC)	+= jz4780-nemc.o
+obj-$(CONFIG_MTK_SMI)		+= mtk-smi.o
 
 obj-$(CONFIG_TEGRA_MC)		+= tegra/
diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
new file mode 100644
index 0000000..2d9d61b
--- /dev/null
+++ b/drivers/memory/mtk-smi.c
@@ -0,0 +1,274 @@
+/*
+ * Copyright (c) 2014-2015 MediaTek Inc.
+ * Author: Yong Wu <yong.wu@mediatek.com>
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/pm_runtime.h>
+#include <soc/mediatek/smi.h>
+
+#define SMI_LARB_MMU_EN		0xf00
+#define F_SMI_MMU_EN(port)	BIT(port)
+
+struct mtk_smi_data {
+	struct clk		*clk_apb;
+	struct clk		*clk_smi;
+};
+
+struct mtk_larb_data { /* larb: local arbiter */
+	void __iomem		*base;
+	struct clk		*clk_apb;
+	struct clk		*clk_smi;
+	struct device		*smidev;
+};
+
+struct mtk_larb_mmu {
+	u32			mmu;
+};
+
+static int __mtk_smi_get(struct device *dev, struct clk *apb, struct clk *smi)
+{
+	int ret;
+
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0)
+		return ret;
+
+	ret = clk_prepare_enable(apb);
+	if (ret) {
+		dev_err(dev, "Failed to enable the apb clock\n");
+		goto err_put_pm;
+	}
+	ret = clk_prepare_enable(smi);
+	if (ret) {
+		dev_err(dev, "Failed to enable the smi clock\n");
+		goto err_disable_apb;
+	}
+	return 0;
+
+err_disable_apb:
+	clk_disable_unprepare(apb);
+err_put_pm:
+	pm_runtime_put(dev);
+	return ret;
+}
+
+static void __mtk_smi_put(struct device *dev, struct clk *apb, struct clk *smi)
+{
+	clk_disable_unprepare(smi);
+	clk_disable_unprepare(apb);
+	pm_runtime_put(dev);
+}
+
+int mtk_smi_larb_get(struct device *larbdev)
+{
+	struct mtk_larb_data *larbdata = dev_get_drvdata(larbdev);
+	struct mtk_smi_data *smidata = dev_get_drvdata(larbdata->smidev);
+	struct mtk_larb_mmu *mmucfg = larbdev->platform_data;
+	int ret;
+
+	/* Enable smidev's power and clockes firstly, then enable the larb's. */
+	ret = __mtk_smi_get(larbdata->smidev, smidata->clk_apb,
+			    smidata->clk_smi);
+	if (ret)
+		return ret;
+
+	ret = __mtk_smi_get(larbdev, larbdata->clk_apb, larbdata->clk_smi);
+	if (ret)
+		goto err_put_smi;
+
+	/* config iommu */
+	writel_relaxed(mmucfg->mmu, larbdata->base + SMI_LARB_MMU_EN);
+
+	return ret;
+
+err_put_smi:
+	__mtk_smi_put(larbdata->smidev, smidata->clk_apb, smidata->clk_smi);
+	return ret;
+}
+
+void mtk_smi_larb_put(struct device *larbdev)
+{
+	struct mtk_larb_data *larbdata = dev_get_drvdata(larbdev);
+	struct mtk_smi_data *smidata = dev_get_drvdata(larbdata->smidev);
+
+	__mtk_smi_put(larbdev, larbdata->clk_apb, larbdata->clk_smi);
+	__mtk_smi_put(larbdata->smidev, smidata->clk_apb, smidata->clk_smi);
+}
+
+int mtk_smi_config_port(struct device *larbdev, unsigned int larbportid,
+			bool enable)
+{
+	struct mtk_larb_mmu *mmucfg = larbdev->platform_data;
+
+	if (!mmucfg) {
+		mmucfg = kzalloc(sizeof(*mmucfg), GFP_KERNEL);
+		if (!mmucfg)
+			return -ENOMEM;
+		larbdev->platform_data = mmucfg;
+	}
+
+	dev_dbg(larbdev, "%s iommu port id %d\n",
+		enable ? "enable" : "disable", larbportid);
+
+	if (enable)
+		mmucfg->mmu |= F_SMI_MMU_EN(larbportid);
+	else
+		mmucfg->mmu &= ~F_SMI_MMU_EN(larbportid);
+
+	return 0;
+}
+
+static int mtk_smi_larb_probe(struct platform_device *pdev)
+{
+	struct mtk_larb_data *larbdata;
+	struct resource *res;
+	struct device *dev = &pdev->dev;
+	struct device_node *smi_node;
+	struct platform_device *smi_pdev;
+
+	if (!dev->pm_domain)
+		return -EPROBE_DEFER;
+
+	larbdata = devm_kzalloc(dev, sizeof(*larbdata), GFP_KERNEL);
+	if (!larbdata)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	larbdata->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(larbdata->base))
+		return PTR_ERR(larbdata->base);
+
+	larbdata->clk_apb = devm_clk_get(dev, "apb");
+	if (IS_ERR(larbdata->clk_apb))
+		return PTR_ERR(larbdata->clk_apb);
+
+	larbdata->clk_smi = devm_clk_get(dev, "smi");
+	if (IS_ERR(larbdata->clk_smi))
+		return PTR_ERR(larbdata->clk_smi);
+
+	smi_node = of_parse_phandle(dev->of_node, "mediatek,smi", 0);
+	if (!smi_node)
+		return -EINVAL;
+
+	smi_pdev = of_find_device_by_node(smi_node);
+	of_node_put(smi_node);
+	if (smi_pdev) {
+		larbdata->smidev = &smi_pdev->dev;
+	} else {
+		dev_err(dev, "Failed to get the smi_common device\n");
+		return -EINVAL;
+	}
+
+	pm_runtime_enable(dev);
+	dev_set_drvdata(dev, larbdata);
+	return 0;
+}
+
+static int mtk_smi_larb_remove(struct platform_device *pdev)
+{
+	struct mtk_larb_mmu *mmucfg = pdev->dev.platform_data;
+
+	kfree(mmucfg);
+	pm_runtime_disable(&pdev->dev);
+	return 0;
+}
+
+static const struct of_device_id mtk_smi_larb_of_ids[] = {
+	{ .compatible = "mediatek,mt8173-smi-larb",},
+	{}
+};
+
+static struct platform_driver mtk_smi_larb_driver = {
+	.probe	= mtk_smi_larb_probe,
+	.remove = mtk_smi_larb_remove,
+	.driver	= {
+		.name = "mtk-smi-larb",
+		.of_match_table = mtk_smi_larb_of_ids,
+	}
+};
+
+static int mtk_smi_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mtk_smi_data *smidata;
+	int ret;
+
+	if (!dev->pm_domain)
+		return -EPROBE_DEFER;
+
+	smidata = devm_kzalloc(dev, sizeof(*smidata), GFP_KERNEL);
+	if (!smidata)
+		return -ENOMEM;
+
+	smidata->clk_apb = devm_clk_get(dev, "apb");
+	if (IS_ERR(smidata->clk_apb))
+		return PTR_ERR(smidata->clk_apb);
+
+	smidata->clk_smi = devm_clk_get(dev, "smi");
+	if (IS_ERR(smidata->clk_smi))
+		return PTR_ERR(smidata->clk_smi);
+
+	pm_runtime_enable(dev);
+	dev_set_drvdata(dev, smidata);
+	return ret;
+}
+
+static int mtk_smi_remove(struct platform_device *pdev)
+{
+	pm_runtime_disable(&pdev->dev);
+	return 0;
+}
+
+static const struct of_device_id mtk_smi_of_ids[] = {
+	{ .compatible = "mediatek,mt8173-smi",},
+	{}
+};
+
+static struct platform_driver mtk_smi_driver = {
+	.probe	= mtk_smi_probe,
+	.remove = mtk_smi_remove,
+	.driver	= {
+		.name = "mtk-smi",
+		.of_match_table = mtk_smi_of_ids,
+	}
+};
+
+static int __init mtk_smi_init(void)
+{
+	int ret;
+
+	ret = platform_driver_register(&mtk_smi_driver);
+	if (ret != 0) {
+		pr_err("Failed to register SMI driver\n");
+		return ret;
+	}
+
+	ret = platform_driver_register(&mtk_smi_larb_driver);
+	if (ret != 0) {
+		pr_err("Failed to register SMI-LARB driver\n");
+		goto err_unreg_smi;
+	}
+	return ret;
+
+err_unreg_smi:
+	platform_driver_unregister(&mtk_smi_driver);
+	return ret;
+}
+module_init(mtk_smi_init);
diff --git a/include/soc/mediatek/smi.h b/include/soc/mediatek/smi.h
new file mode 100644
index 0000000..189d575
--- /dev/null
+++ b/include/soc/mediatek/smi.h
@@ -0,0 +1,60 @@
+/*
+ * Copyright (c) 2014-2015 MediaTek Inc.
+ * Author: Yong Wu <yong.wu@mediatek.com>
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#ifndef MTK_IOMMU_SMI_H
+#define MTK_IOMMU_SMI_H
+
+#include <linux/device.h>
+
+#ifdef CONFIG_MTK_SMI
+
+/*
+ * Record the iommu info for each port in the local arbiter.
+ * It is only for iommu.
+ *
+ * Returns 0 if successfully, others if failed.
+ */
+int mtk_smi_config_port(struct device *larbdev, unsigned int larbportid,
+			bool enable);
+/*
+ * The two function below config iommu and enable/disable the clock
+ * for the larb.
+ *
+ * mtk_smi_larb_get must be called before the multimedia HW work.
+ * mtk_smi_larb_put must be called after HW done.
+ * Both should be called in non-atomic context.
+ *
+ * Returns 0 if successfully, others if failed.
+ */
+int mtk_smi_larb_get(struct device *larbdev);
+void mtk_smi_larb_put(struct device *larbdev);
+
+#else
+
+static int
+mtk_smi_config_port(struct device *larbdev, unsigned int larbportid,
+		    bool enable)
+{
+	return 0;
+}
+
+static inline int mtk_smi_larb_get(struct device *larbdev)
+{
+	return 0;
+}
+
+static inline void mtk_smi_larb_put(struct device *larbdev) { }
+
+#endif
+
+#endif
-- 
1.8.1.1.dirty


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 5/6] iommu/mediatek: Add mt8173 IOMMU driver
  2015-10-09  2:23 [PATCH v5 0/6] MT8173 IOMMU SUPPORT Yong Wu
                   ` (3 preceding siblings ...)
  2015-10-09  2:23 ` [PATCH v5 4/6] memory: mediatek: Add SMI driver Yong Wu
@ 2015-10-09  2:23 ` Yong Wu
  2015-10-14 12:53   ` Joerg Roedel
  2015-10-27 13:25   ` Robin Murphy
  2015-10-09  2:23 ` [PATCH v5 6/6] dts: mt8173: Add iommu/smi nodes for mt8173 Yong Wu
  2015-10-14 12:56 ` [PATCH v5 0/6] MT8173 IOMMU SUPPORT Joerg Roedel
  6 siblings, 2 replies; 23+ messages in thread
From: Yong Wu @ 2015-10-09  2:23 UTC (permalink / raw)
  To: Joerg Roedel, Thierry Reding, Mark Rutland, Matthias Brugger
  Cc: Robin Murphy, Will Deacon, Daniel Kurtz, Tomasz Figa,
	Lucas Stach, Rob Herring, Catalin Marinas, linux-mediatek,
	Sasha Hauer, srv_heupstream, devicetree, linux-kernel,
	linux-arm-kernel, iommu, pebolle, arnd, mitchelh, Sricharan R,
	youhua.li, k.zhang, kendrick.hsu, Yong Wu

This patch adds support for mediatek m4u (MultiMedia Memory Management
Unit).

Signed-off-by: Yong Wu <yong.wu@mediatek.com>
---
 drivers/iommu/Kconfig     |  15 +
 drivers/iommu/Makefile    |   1 +
 drivers/iommu/mtk_iommu.c | 767 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 783 insertions(+)
 create mode 100644 drivers/iommu/mtk_iommu.c

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index a7920fb..b964364 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -387,4 +387,19 @@ config ARM_SMMU_V3
 	  Say Y here if your system includes an IOMMU device implementing
 	  the ARM SMMUv3 architecture.
 
+config MTK_IOMMU
+	bool "MTK IOMMU Support"
+	depends on ARCH_MEDIATEK || COMPILE_TEST
+	select IOMMU_API
+	select IOMMU_DMA
+	select IOMMU_IO_PGTABLE_SHORT
+	select MEMORY
+	select MTK_SMI
+	help
+	  Support for the M4U on certain Mediatek SOCs. M4U is MultiMedia
+	  Memory Management Unit. This option enables remapping of DMA memory
+	  accesses for the multimedia subsystem.
+
+	  If unsure, say N here.
+
 endif # IOMMU_SUPPORT
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 06df3e6..f4f2f2c 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_ROCKCHIP_IOMMU) += rockchip-iommu.o
 obj-$(CONFIG_TEGRA_IOMMU_GART) += tegra-gart.o
 obj-$(CONFIG_TEGRA_IOMMU_SMMU) += tegra-smmu.o
 obj-$(CONFIG_EXYNOS_IOMMU) += exynos-iommu.o
+obj-$(CONFIG_MTK_IOMMU) += mtk_iommu.o
 obj-$(CONFIG_SHMOBILE_IOMMU) += shmobile-iommu.o
 obj-$(CONFIG_SHMOBILE_IPMMU) += shmobile-ipmmu.o
 obj-$(CONFIG_FSL_PAMU) += fsl_pamu.o fsl_pamu_domain.o
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
new file mode 100644
index 0000000..39839f7
--- /dev/null
+++ b/drivers/iommu/mtk_iommu.c
@@ -0,0 +1,767 @@
+/*
+ * Copyright (c) 2014-2015 MediaTek Inc.
+ * Author: Yong Wu <yong.wu@mediatek.com>
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/iommu.h>
+#include <linux/dma-iommu.h>
+#include <linux/of_iommu.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/list.h>
+#include <linux/clk.h>
+#include <linux/iopoll.h>
+#include <soc/mediatek/smi.h>
+#include "io-pgtable.h"
+
+#define REG_MMU_PT_BASE_ADDR			0x000
+
+#define REG_MMU_INVALIDATE			0x020
+#define F_ALL_INVLD				0x2
+#define F_MMU_INV_RANGE				0x1
+
+#define REG_MMU_INVLD_START_A			0x024
+#define REG_MMU_INVLD_END_A			0x028
+
+#define REG_MMU_INV_SEL				0x038
+#define F_INVLD_EN0				BIT(0)
+#define F_INVLD_EN1				BIT(1)
+
+#define REG_MMU_STANDARD_AXI_MODE		0x048
+#define REG_MMU_DCM_DIS				0x050
+
+#define REG_MMU_CTRL_REG			0x110
+#define F_MMU_PREFETCH_RT_REPLACE_MOD		BIT(4)
+#define F_MMU_TF_PROTECT_SEL(prot)		(((prot) & 0x3) << 5)
+#define F_COHERENCE_EN				BIT(8)
+
+#define REG_MMU_IVRP_PADDR			0x114
+#define F_MMU_IVRP_PA_SET(pa)			((pa) >> 1)
+
+#define REG_MMU_INT_CONTROL0			0x120
+#define F_L2_MULIT_HIT_EN			BIT(0)
+#define F_TABLE_WALK_FAULT_INT_EN		BIT(1)
+#define F_PREETCH_FIFO_OVERFLOW_INT_EN		BIT(2)
+#define F_MISS_FIFO_OVERFLOW_INT_EN		BIT(3)
+#define F_PREFETCH_FIFO_ERR_INT_EN		BIT(5)
+#define F_MISS_FIFO_ERR_INT_EN			BIT(6)
+#define F_INT_L2_CLR_BIT			BIT(12)
+
+#define REG_MMU_INT_MAIN_CONTROL		0x124
+#define F_INT_TRANSLATION_FAULT			BIT(0)
+#define F_INT_MAIN_MULTI_HIT_FAULT		BIT(1)
+#define F_INT_INVALID_PA_FAULT			BIT(2)
+#define F_INT_ENTRY_REPLACEMENT_FAULT		BIT(3)
+#define F_INT_TLB_MISS_FAULT			BIT(4)
+#define F_INT_MISS_TRANSATION_FIFO_FAULT	BIT(5)
+#define F_INT_PRETETCH_TRANSATION_FIFO_FAULT	BIT(6)
+
+#define REG_MMU_CPE_DONE			0x12C
+
+#define REG_MMU_FAULT_ST1			0x134
+
+#define REG_MMU_FAULT_VA			0x13c
+#define F_MMU_FAULT_VA_MSK			0xfffff000
+#define F_MMU_FAULT_VA_WRITE_BIT		BIT(1)
+#define F_MMU_FAULT_VA_LAYER_BIT		BIT(0)
+
+#define REG_MMU_INVLD_PA			0x140
+#define REG_MMU_INT_ID				0x150
+#define F_MMU0_INT_ID_LARB_ID(a)		(((a) >> 7) & 0x7)
+#define F_MMU0_INT_ID_PORT_ID(a)		(((a) >> 2) & 0x1f)
+
+#define MTK_PROTECT_PA_ALIGN			128
+#define MTK_IOMMU_LARB_MAX_NR			8
+#define MTK_IOMMU_REG_NR			10
+
+struct mtk_iommu_suspend_reg {
+	u32				standard_axi_mode;
+	u32				dcm_dis;
+	u32				ctrl_reg;
+	u32				ivrp_paddr;
+	u32				int_control0;
+	u32				int_main_control;
+};
+
+struct mtk_iommu_data {
+	void __iomem			*base;
+	int				irq;
+	struct device			*dev;
+	struct device			*larbdev[MTK_IOMMU_LARB_MAX_NR];
+	struct clk			*bclk;
+	phys_addr_t			protect_base; /* protect memory base */
+	int				larb_nr;/* local arbiter number */
+	struct mtk_iommu_suspend_reg	reg;
+};
+
+struct mtk_iommu_domain {
+	struct imu_pgd_t		*pgd;
+	spinlock_t			pgtlock; /* lock for page table */
+
+	struct io_pgtable_cfg		cfg;
+	struct io_pgtable_ops		*iop;
+
+	struct mtk_iommu_data		*data;
+	struct iommu_domain		domain;
+};
+
+struct mtk_iommu_client_priv {
+	struct list_head		client;
+	unsigned int			larbid;
+	unsigned int			portid;
+	struct device			*m4udev;
+};
+
+static struct iommu_ops mtk_iommu_ops;
+
+static struct mtk_iommu_domain *to_mtk_domain(struct iommu_domain *dom)
+{
+	return container_of(dom, struct mtk_iommu_domain, domain);
+}
+
+static void mtk_iommu_clear_intr(const struct mtk_iommu_data *data)
+{
+	u32 val;
+
+	val = readl_relaxed(data->base + REG_MMU_INT_CONTROL0);
+	val |= F_INT_L2_CLR_BIT;
+	writel_relaxed(val, data->base + REG_MMU_INT_CONTROL0);
+}
+
+static void mtk_iommu_tlb_flush_all(void *cookie)
+{
+	struct mtk_iommu_domain *domain = cookie;
+	void __iomem *base;
+
+	base = domain->data->base;
+	writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0, base + REG_MMU_INV_SEL);
+	writel_relaxed(F_ALL_INVLD, base + REG_MMU_INVALIDATE);
+	mb();/* Make sure flush all done */
+}
+
+static void mtk_iommu_tlb_add_flush(unsigned long iova, size_t size,
+				    bool leaf, void *cookie)
+{
+	struct mtk_iommu_domain *domain = cookie;
+	void __iomem *base = domain->data->base;
+	unsigned int iova_start = iova, iova_end = iova + size - 1;
+
+	writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0, base + REG_MMU_INV_SEL);
+
+	writel_relaxed(iova_start, base + REG_MMU_INVLD_START_A);
+	writel_relaxed(iova_end, base + REG_MMU_INVLD_END_A);
+	writel_relaxed(F_MMU_INV_RANGE, base + REG_MMU_INVALIDATE);
+}
+
+static void mtk_iommu_tlb_sync(void *cookie)
+{
+	struct mtk_iommu_domain *domain = cookie;
+	void __iomem *base = domain->data->base;
+	int ret;
+	u32 tmp;
+
+	ret = readl_poll_timeout_atomic(base + REG_MMU_CPE_DONE, tmp,
+					tmp != 0, 10, 1000000);
+	if (ret) {
+		dev_warn(domain->data->dev,
+			 "Partial TLB flush timed out, falling back to full flush\n");
+		mtk_iommu_tlb_flush_all(cookie);
+	}
+	writel_relaxed(0, base + REG_MMU_CPE_DONE);
+}
+
+static struct iommu_gather_ops mtk_iommu_gather_ops = {
+	.tlb_flush_all = mtk_iommu_tlb_flush_all,
+	.tlb_add_flush = mtk_iommu_tlb_add_flush,
+	.tlb_sync = mtk_iommu_tlb_sync,
+};
+
+static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
+{
+	struct mtk_iommu_domain *mtkdom = dev_id;
+	struct mtk_iommu_data *data = mtkdom->data;
+	u32 int_state, regval, fault_iova, fault_pa;
+	unsigned int fault_larb, fault_port;
+	bool layer, write;
+
+	int_state = readl_relaxed(data->base + REG_MMU_FAULT_ST1);
+
+	/* Read error info */
+	fault_iova = readl_relaxed(data->base + REG_MMU_FAULT_VA);
+	layer = fault_iova & F_MMU_FAULT_VA_LAYER_BIT;
+	write = fault_iova & F_MMU_FAULT_VA_WRITE_BIT;
+	fault_iova &= F_MMU_FAULT_VA_MSK;
+	fault_pa = readl_relaxed(data->base + REG_MMU_INVLD_PA);
+	regval = readl_relaxed(data->base + REG_MMU_INT_ID);
+	fault_larb = F_MMU0_INT_ID_LARB_ID(regval);
+	fault_port = F_MMU0_INT_ID_PORT_ID(regval);
+
+	if (report_iommu_fault(&mtkdom->domain, data->dev, fault_iova,
+			       write ? IOMMU_FAULT_WRITE : IOMMU_FAULT_READ)) {
+		dev_err_ratelimited(
+			data->dev,
+			"fault type=0x%x iova=0x%x pa=0x%x larb=%d port=%d layer=%d %s\n",
+			int_state, fault_iova, fault_pa, fault_larb, fault_port,
+			layer, write ? "write" : "read");
+	}
+
+	mtk_iommu_clear_intr(data);
+	mtk_iommu_tlb_flush_all(mtkdom);
+
+	return IRQ_HANDLED;
+}
+
+static int mtk_iommu_parse_dt(struct platform_device *pdev,
+			      struct mtk_iommu_data *data)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *ofnode;
+	struct resource *res;
+	int i;
+
+	ofnode = dev->of_node;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	data->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(data->base))
+		return PTR_ERR(data->base);
+
+	data->irq = platform_get_irq(pdev, 0);
+	if (data->irq < 0)
+		return data->irq;
+
+	data->bclk = devm_clk_get(dev, "bclk");
+	if (IS_ERR(data->bclk))
+		return PTR_ERR(data->bclk);
+
+	data->larb_nr = of_count_phandle_with_args(
+					ofnode, "mediatek,larb", NULL);
+	if (data->larb_nr < 0)
+		return data->larb_nr;
+
+	for (i = 0; i < data->larb_nr; i++) {
+		struct device_node *larbnode;
+		struct platform_device *plarbdev;
+
+		larbnode = of_parse_phandle(ofnode, "mediatek,larb", i);
+		if (!larbnode)
+			return -EINVAL;
+
+		plarbdev = of_find_device_by_node(larbnode);
+		of_node_put(larbnode);
+		if (!plarbdev)
+			return -EPROBE_DEFER;
+		data->larbdev[i] = &plarbdev->dev;
+	}
+
+	return 0;
+}
+
+static int mtk_iommu_hw_init(const struct mtk_iommu_domain *mtkdom)
+{
+	struct mtk_iommu_data *data = mtkdom->data;
+	void __iomem *base = data->base;
+	u32 regval;
+	int ret;
+
+	ret = clk_prepare_enable(data->bclk);
+	if (ret) {
+		dev_err(data->dev, "Failed to enable iommu clk(%d)\n", ret);
+		return ret;
+	}
+
+	writel_relaxed(mtkdom->cfg.arm_short_cfg.ttbr[0],
+		       base + REG_MMU_PT_BASE_ADDR);
+
+	regval = F_MMU_PREFETCH_RT_REPLACE_MOD |
+		F_MMU_TF_PROTECT_SEL(2) |
+		F_COHERENCE_EN;
+	writel_relaxed(regval, base + REG_MMU_CTRL_REG);
+
+	regval = F_L2_MULIT_HIT_EN |
+		F_TABLE_WALK_FAULT_INT_EN |
+		F_PREETCH_FIFO_OVERFLOW_INT_EN |
+		F_MISS_FIFO_OVERFLOW_INT_EN |
+		F_PREFETCH_FIFO_ERR_INT_EN |
+		F_MISS_FIFO_ERR_INT_EN;
+	writel_relaxed(regval, base + REG_MMU_INT_CONTROL0);
+
+	regval = F_INT_TRANSLATION_FAULT |
+		F_INT_MAIN_MULTI_HIT_FAULT |
+		F_INT_INVALID_PA_FAULT |
+		F_INT_ENTRY_REPLACEMENT_FAULT |
+		F_INT_TLB_MISS_FAULT |
+		F_INT_MISS_TRANSATION_FIFO_FAULT |
+		F_INT_PRETETCH_TRANSATION_FIFO_FAULT;
+	writel_relaxed(regval, base + REG_MMU_INT_MAIN_CONTROL);
+
+	regval = ALIGN(data->protect_base, MTK_PROTECT_PA_ALIGN);
+	regval = F_MMU_IVRP_PA_SET(regval);
+	writel_relaxed(regval, base + REG_MMU_IVRP_PADDR);
+
+	writel_relaxed(0, base + REG_MMU_DCM_DIS);
+	writel_relaxed(0, base + REG_MMU_STANDARD_AXI_MODE);
+
+	if (devm_request_irq(data->dev, data->irq, mtk_iommu_isr, 0,
+			     dev_name(data->dev), (void *)mtkdom)) {
+		writel_relaxed(0, base + REG_MMU_PT_BASE_ADDR);
+		clk_disable_unprepare(data->bclk);
+		dev_err(data->dev, "Failed @ IRQ-%d Request\n", data->irq);
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+static int mtk_iommu_config(struct mtk_iommu_domain *mtkdom,
+			    struct device *dev, bool enable)
+{
+	struct mtk_iommu_data *data = mtkdom->data;
+	struct mtk_iommu_client_priv *head, *cur, *next;
+
+	head = dev->archdata.iommu;
+	list_for_each_entry_safe(cur, next, &head->client, client) {
+		if (cur->larbid >= data->larb_nr) {
+			dev_err(data->dev, "Invalid larb:%d\n", cur->larbid);
+			return -EINVAL;
+		}
+
+		mtk_smi_config_port(data->larbdev[cur->larbid],
+				    cur->portid, enable);
+		if (!enable) {
+			list_del(&cur->client);
+			kfree(cur);
+		}
+	}
+
+	if (!enable) {
+		kfree(head);
+		dev->archdata.iommu = NULL;
+	}
+	return 0;
+}
+
+static int mtk_iommu_init_domain_context(struct mtk_iommu_domain *dom)
+{
+	int ret;
+
+	if (dom->iop)
+		return 0;
+
+	spin_lock_init(&dom->pgtlock);
+	dom->cfg.quirks = IO_PGTABLE_QUIRK_ARM_NS |
+			IO_PGTABLE_QUIRK_NO_PERMS |
+			IO_PGTABLE_QUIRK_TLBI_ON_MAP |
+			IO_PGTABLE_QUIRK_SHORT_SUPERSECTION;
+	dom->cfg.pgsize_bitmap = mtk_iommu_ops.pgsize_bitmap,
+	dom->cfg.ias = 32;
+	dom->cfg.oas = 32;
+	dom->cfg.tlb = &mtk_iommu_gather_ops;
+	dom->cfg.iommu_dev = dom->data->dev;
+
+	dom->iop = alloc_io_pgtable_ops(ARM_SHORT_DESC, &dom->cfg, dom);
+	if (!dom->iop) {
+		dev_err(dom->data->dev, "Failed to alloc io pgtable\n");
+		return -EINVAL;
+	}
+
+	/* Update our support page sizes bitmap */
+	mtk_iommu_ops.pgsize_bitmap = dom->cfg.pgsize_bitmap;
+
+	ret = mtk_iommu_hw_init(dom);
+	if (ret)
+		free_io_pgtable_ops(dom->iop);
+
+	return ret;
+}
+
+static struct iommu_domain *mtk_iommu_domain_alloc(unsigned type)
+{
+	struct mtk_iommu_domain *priv;
+
+	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
+		return NULL;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return NULL;
+
+	if (type == IOMMU_DOMAIN_DMA && iommu_get_dma_cookie(&priv->domain)) {
+		kfree(priv);
+		return NULL;
+	}
+
+	priv->domain.geometry.aperture_start = 0;
+	priv->domain.geometry.aperture_end = DMA_BIT_MASK(32);
+	priv->domain.geometry.force_aperture = true;
+
+	return &priv->domain;
+}
+
+static void mtk_iommu_domain_free(struct iommu_domain *domain)
+{
+	if (domain->type == IOMMU_DOMAIN_DMA)
+		iommu_put_dma_cookie(domain);
+	kfree(to_mtk_domain(domain));
+}
+
+static int mtk_iommu_attach_device(struct iommu_domain *domain,
+				   struct device *dev)
+{
+	struct mtk_iommu_domain *priv = to_mtk_domain(domain), *m4udom;
+	struct iommu_group *group;
+	struct mtk_iommu_client_priv *clientpriv;
+	struct device *m4udev;
+	int ret;
+
+	clientpriv = dev->archdata.iommu;
+	if (!clientpriv)
+		return -ENODEV;
+	m4udev = clientpriv->m4udev;
+
+	/*
+	 * There is a domain for each a iommu device in normal case.
+	 * But MTK only has one iommu domain called the m4u domain which all
+	 * the multimedia HW share. Here we reserve one as the m4u domain and
+	 * free the others.
+	 *
+	 * And the attach_device that from __iommu_setup_dma_ops
+	 * will be called earlier than probe.
+	 */
+	m4udom = dev_get_drvdata(m4udev);
+	if (!m4udom)
+		dev_set_drvdata(m4udev, priv);
+	else if (m4udom != priv)
+		iommu_domain_free(domain);
+
+	group = iommu_group_get(dev);
+	if (!group)
+		return 0;
+	iommu_group_put(group);
+
+	/* Initial the m4u domain context which is from the add_device */
+	ret = mtk_iommu_init_domain_context(priv);
+	if (ret)
+		return ret;
+
+	return mtk_iommu_config(priv, dev, true);
+}
+
+static void mtk_iommu_detach_device(struct iommu_domain *domain,
+				    struct device *dev)
+{
+	mtk_iommu_config(to_mtk_domain(domain), dev, false);
+}
+
+static int mtk_iommu_map(struct iommu_domain *domain, unsigned long iova,
+			 phys_addr_t paddr, size_t size, int prot)
+{
+	struct mtk_iommu_domain *priv = to_mtk_domain(domain);
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&priv->pgtlock, flags);
+	ret = priv->iop->map(priv->iop, iova, paddr, size, prot);
+	spin_unlock_irqrestore(&priv->pgtlock, flags);
+
+	return ret;
+}
+
+static size_t mtk_iommu_unmap(struct iommu_domain *domain,
+			      unsigned long iova, size_t size)
+{
+	struct mtk_iommu_domain *priv = to_mtk_domain(domain);
+	unsigned long flags;
+	size_t unmapsize;
+
+	spin_lock_irqsave(&priv->pgtlock, flags);
+	unmapsize = priv->iop->unmap(priv->iop, iova, size);
+	spin_unlock_irqrestore(&priv->pgtlock, flags);
+
+	return unmapsize;
+}
+
+static phys_addr_t mtk_iommu_iova_to_phys(struct iommu_domain *domain,
+					  dma_addr_t iova)
+{
+	struct mtk_iommu_domain *priv = to_mtk_domain(domain);
+	unsigned long flags;
+	phys_addr_t pa;
+
+	spin_lock_irqsave(&priv->pgtlock, flags);
+	pa = priv->iop->iova_to_phys(priv->iop, iova);
+	spin_unlock_irqrestore(&priv->pgtlock, flags);
+
+	return pa;
+}
+
+static int mtk_iommu_add_device(struct device *dev)
+{
+	struct iommu_group *group;
+	struct mtk_iommu_client_priv *priv;
+	struct mtk_iommu_domain *m4udom;
+	struct iommu_domain *domain;
+	int ret;
+
+	if (!dev->archdata.iommu) /* Not a iommu client device */
+		return -ENODEV;
+
+	group = iommu_group_get(dev);
+	if (!group) {
+		group = iommu_group_alloc();
+		if (IS_ERR(group)) {
+			dev_err(dev, "Failed to allocate IOMMU group\n");
+			return PTR_ERR(group);
+		}
+	}
+
+	ret = iommu_group_add_device(group, dev);
+	if (ret) {
+		dev_err(dev, "Failed to add IOMMU group\n");
+		goto err_group_put;
+	}
+
+	domain = iommu_get_domain_for_dev(dev);
+	if (!domain) {
+		/*
+		 * Get the m4u iommu domain from the m4u device.
+		 * Attach all the client devices into the m4u domain.
+		 */
+		priv = dev->archdata.iommu;
+		m4udom = dev_get_drvdata(priv->m4udev);
+		ret = iommu_attach_group(&m4udom->domain, group);
+		if (ret)
+			dev_err(dev, "Failed to attach IOMMU group\n");
+	}
+
+err_group_put:
+	iommu_group_put(group);
+	return ret;
+}
+
+static void mtk_iommu_remove_device(struct device *dev)
+{
+	if (!dev->archdata.iommu)
+		return;
+
+	iommu_group_remove_device(dev);
+}
+
+static int mtk_iommu_of_xlate(struct device *dev, struct of_phandle_args *args)
+{
+	struct mtk_iommu_client_priv *head, *priv, *next;
+	struct platform_device *m4updev;
+
+	if (args->args_count != 2) {
+		dev_err(dev, "invalid #iommu-cells(%d) property for IOMMU\n",
+			args->args_count);
+		return -EINVAL;
+	}
+
+	if (!dev->archdata.iommu) {
+		/* Get the m4u device */
+		m4updev = of_find_device_by_node(args->np);
+		of_node_put(args->np);
+		if (WARN_ON(!m4updev))
+			return -EINVAL;
+
+		head = kzalloc(sizeof(*head), GFP_KERNEL);
+		if (!head)
+			return -ENOMEM;
+
+		dev->archdata.iommu = head;
+		INIT_LIST_HEAD(&head->client);
+		head->m4udev = &m4updev->dev;
+	} else {
+		head = dev->archdata.iommu;
+	}
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		goto err_free_mem;
+
+	priv->larbid = args->args[0];
+	priv->portid = args->args[1];
+	list_add_tail(&priv->client, &head->client);
+
+	return 0;
+
+err_free_mem:
+	list_for_each_entry_safe(priv, next, &head->client, client)
+		kfree(priv);
+	kfree(head);
+	dev->archdata.iommu = NULL;
+	return -ENOMEM;
+}
+
+static struct iommu_ops mtk_iommu_ops = {
+	.domain_alloc	= mtk_iommu_domain_alloc,
+	.domain_free	= mtk_iommu_domain_free,
+	.attach_dev	= mtk_iommu_attach_device,
+	.detach_dev	= mtk_iommu_detach_device,
+	.map		= mtk_iommu_map,
+	.unmap		= mtk_iommu_unmap,
+	.map_sg		= default_iommu_map_sg,
+	.iova_to_phys	= mtk_iommu_iova_to_phys,
+	.add_device	= mtk_iommu_add_device,
+	.remove_device	= mtk_iommu_remove_device,
+	.of_xlate	= mtk_iommu_of_xlate,
+	.pgsize_bitmap	= SZ_4K | SZ_64K | SZ_1M | SZ_16M,
+};
+
+static const struct of_device_id mtk_iommu_of_ids[] = {
+	{ .compatible = "mediatek,mt8173-m4u", },
+	{}
+};
+
+static int mtk_iommu_init_fn(struct device_node *np)
+{
+	struct platform_device *pdev;
+
+	pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
+	if (IS_ERR(pdev))
+		return PTR_ERR(pdev);
+
+	of_iommu_set_ops(np, &mtk_iommu_ops);
+
+	return 0;
+}
+
+IOMMU_OF_DECLARE(mtkm4u, "mediatek,mt8173-m4u", mtk_iommu_init_fn);
+
+static int mtk_iommu_probe(struct platform_device *pdev)
+{
+	struct mtk_iommu_data   *data;
+	struct device           *dev = &pdev->dev;
+	struct mtk_iommu_domain *m4udom;
+	void __iomem	        *protect;
+	int                     ret;
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+	data->dev = dev;
+
+	/* Protect memory. HW will access here while translation fault.*/
+	protect = devm_kzalloc(dev, MTK_PROTECT_PA_ALIGN * 2, GFP_KERNEL);
+	if (!protect)
+		return -ENOMEM;
+	data->protect_base = virt_to_phys(protect);
+
+	ret = mtk_iommu_parse_dt(pdev, data);
+	if (ret)
+		return ret;
+
+	m4udom = dev_get_drvdata(dev);
+	if (m4udom)
+		m4udom->data = data;
+
+	return 0;
+}
+
+static int mtk_iommu_remove(struct platform_device *pdev)
+{
+	struct mtk_iommu_domain *mtkdom = dev_get_drvdata(&pdev->dev);
+
+	if (!mtkdom)
+		return 0;
+
+	free_io_pgtable_ops(mtkdom->iop); /* Destroy domain context */
+	clk_disable_unprepare(mtkdom->data->bclk);
+	return 0;
+}
+
+static int mtk_iommu_suspend(struct device *dev)
+{
+	struct mtk_iommu_domain *mtkdom = dev_get_drvdata(dev);
+	struct mtk_iommu_suspend_reg *reg;
+	void __iomem *base;
+
+	if (!mtkdom)
+		return 0;
+
+	reg = &mtkdom->data->reg;
+	base = mtkdom->data->base;
+	reg->standard_axi_mode = readl_relaxed(base +
+					       REG_MMU_STANDARD_AXI_MODE);
+	reg->dcm_dis = readl_relaxed(base + REG_MMU_DCM_DIS);
+	reg->ctrl_reg = readl_relaxed(base + REG_MMU_CTRL_REG);
+	reg->ivrp_paddr = readl_relaxed(base + REG_MMU_IVRP_PADDR);
+	reg->int_control0 = readl_relaxed(base + REG_MMU_INT_CONTROL0);
+	reg->int_main_control = readl_relaxed(base + REG_MMU_INT_MAIN_CONTROL);
+	return 0;
+}
+
+static int mtk_iommu_resume(struct device *dev)
+{
+	struct mtk_iommu_domain *mtkdom = dev_get_drvdata(dev);
+	struct mtk_iommu_suspend_reg *reg;
+	void __iomem *base;
+
+	if (!mtkdom)
+		return 0;
+
+	reg = &mtkdom->data->reg;
+	base = mtkdom->data->base;
+	writel_relaxed(mtkdom->cfg.arm_short_cfg.ttbr[0],
+		       base + REG_MMU_PT_BASE_ADDR);
+	writel_relaxed(reg->standard_axi_mode,
+		       base + REG_MMU_STANDARD_AXI_MODE);
+	writel_relaxed(reg->dcm_dis, base + REG_MMU_DCM_DIS);
+	writel_relaxed(reg->ctrl_reg, base + REG_MMU_CTRL_REG);
+	writel_relaxed(reg->ivrp_paddr, base + REG_MMU_IVRP_PADDR);
+	writel_relaxed(reg->int_control0, base + REG_MMU_INT_CONTROL0);
+	writel_relaxed(reg->int_main_control, base + REG_MMU_INT_MAIN_CONTROL);
+	return 0;
+}
+
+const struct dev_pm_ops mtk_iommu_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(mtk_iommu_suspend, mtk_iommu_resume)
+};
+
+static struct platform_driver mtk_iommu_driver = {
+	.probe	= mtk_iommu_probe,
+	.remove	= mtk_iommu_remove,
+	.driver	= {
+		.name = "mtk-iommu",
+		.of_match_table = mtk_iommu_of_ids,
+		.pm = &mtk_iommu_pm_ops,
+	}
+};
+
+static int __init mtk_iommu_init(void)
+{
+	int ret;
+
+	ret = platform_driver_register(&mtk_iommu_driver);
+	if (ret) {
+		pr_err("%s: Failed to register driver\n", __func__);
+		return ret;
+	}
+
+	if (!iommu_present(&platform_bus_type))
+		bus_set_iommu(&platform_bus_type, &mtk_iommu_ops);
+
+	return 0;
+}
+
+static void __exit mtk_iommu_exit(void)
+{
+	return platform_driver_unregister(&mtk_iommu_driver);
+}
+
+subsys_initcall(mtk_iommu_init);
+module_exit(mtk_iommu_exit);
-- 
1.8.1.1.dirty


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v5 6/6] dts: mt8173: Add iommu/smi nodes for mt8173
  2015-10-09  2:23 [PATCH v5 0/6] MT8173 IOMMU SUPPORT Yong Wu
                   ` (4 preceding siblings ...)
  2015-10-09  2:23 ` [PATCH v5 5/6] iommu/mediatek: Add mt8173 IOMMU driver Yong Wu
@ 2015-10-09  2:23 ` Yong Wu
  2015-10-14 12:56 ` [PATCH v5 0/6] MT8173 IOMMU SUPPORT Joerg Roedel
  6 siblings, 0 replies; 23+ messages in thread
From: Yong Wu @ 2015-10-09  2:23 UTC (permalink / raw)
  To: Joerg Roedel, Thierry Reding, Mark Rutland, Matthias Brugger
  Cc: Robin Murphy, Will Deacon, Daniel Kurtz, Tomasz Figa,
	Lucas Stach, Rob Herring, Catalin Marinas, linux-mediatek,
	Sasha Hauer, srv_heupstream, devicetree, linux-kernel,
	linux-arm-kernel, iommu, pebolle, arnd, mitchelh, Sricharan R,
	youhua.li, k.zhang, kendrick.hsu, Yong Wu

This patch add the iommu/larbs nodes for mt8173

Signed-off-by: Yong Wu <yong.wu@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8173.dtsi | 81 ++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index 42540b2..524d305 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -14,6 +14,7 @@
 #include <dt-bindings/clock/mt8173-clk.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/memory/mt8173-larb-port.h>
 #include <dt-bindings/power/mt8173-power.h>
 #include <dt-bindings/reset-controller/mt8173-resets.h>
 #include "mt8173-pinfunc.h"
@@ -265,6 +266,17 @@
 			reg = <0 0x10200620 0 0x20>;
 		};
 
+		iommu: iommu@10205000 {
+			compatible = "mediatek,mt8173-m4u";
+			reg = <0 0x10205000 0 0x1000>;
+			interrupts = <GIC_SPI 139 IRQ_TYPE_LEVEL_LOW>;
+			clocks = <&infracfg CLK_INFRA_M4U>;
+			clock-names = "bclk";
+			mediatek,larb = <&larb0 &larb1 &larb2
+					 &larb3 &larb4 &larb5>;
+			#iommu-cells = <2>;
+		};
+
 		apmixedsys: clock-controller@10209000 {
 			compatible = "mediatek,mt8173-apmixedsys";
 			reg = <0 0x10209000 0 0x1000>;
@@ -514,29 +526,98 @@
 			#clock-cells = <1>;
 		};
 
+		larb0: larb@14021000 {
+			compatible = "mediatek,mt8173-smi-larb";
+			reg = <0 0x14021000 0 0x1000>;
+			mediatek,smi = <&smi_common>;
+			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			clocks = <&mmsys CLK_MM_SMI_LARB0>,
+				 <&mmsys CLK_MM_SMI_LARB0>;
+			clock-names = "apb", "smi";
+		};
+
+		smi_common: smi@14022000 {
+			compatible = "mediatek,mt8173-smi";
+			reg = <0 0x14022000 0 0x1000>;
+			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			clocks = <&mmsys CLK_MM_SMI_COMMON>,
+				 <&mmsys CLK_MM_SMI_COMMON>;
+			clock-names = "apb", "smi";
+		};
+
+		larb4: larb@14027000 {
+			compatible = "mediatek,mt8173-smi-larb";
+			reg = <0 0x14027000 0 0x1000>;
+			mediatek,smi = <&smi_common>;
+			power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
+			clocks = <&mmsys CLK_MM_SMI_LARB4>,
+				 <&mmsys CLK_MM_SMI_LARB4>;
+			clock-names = "apb", "smi";
+		};
+
 		imgsys: clock-controller@15000000 {
 			compatible = "mediatek,mt8173-imgsys", "syscon";
 			reg = <0 0x15000000 0 0x1000>;
 			#clock-cells = <1>;
 		};
 
+		larb2: larb@15001000 {
+			compatible = "mediatek,mt8173-smi-larb";
+			reg = <0 0x15001000 0 0x1000>;
+			mediatek,smi = <&smi_common>;
+			power-domains = <&scpsys MT8173_POWER_DOMAIN_ISP>;
+			clocks = <&imgsys CLK_IMG_LARB2_SMI>,
+				 <&imgsys CLK_IMG_LARB2_SMI>;
+			clock-names = "apb", "smi";
+		};
+
 		vdecsys: clock-controller@16000000 {
 			compatible = "mediatek,mt8173-vdecsys", "syscon";
 			reg = <0 0x16000000 0 0x1000>;
 			#clock-cells = <1>;
 		};
 
+		larb1: larb@16010000 {
+			compatible = "mediatek,mt8173-smi-larb";
+			reg = <0 0x16010000 0 0x1000>;
+			mediatek,smi = <&smi_common>;
+			power-domains = <&scpsys MT8173_POWER_DOMAIN_VDEC>;
+			clocks = <&vdecsys CLK_VDEC_CKEN>,
+				 <&vdecsys CLK_VDEC_LARB_CKEN>;
+			clock-names = "apb", "smi";
+		};
+
 		vencsys: clock-controller@18000000 {
 			compatible = "mediatek,mt8173-vencsys", "syscon";
 			reg = <0 0x18000000 0 0x1000>;
 			#clock-cells = <1>;
 		};
 
+		larb3: larb@18001000 {
+			compatible = "mediatek,mt8173-smi-larb";
+			reg = <0 0x18001000 0 0x1000>;
+			mediatek,smi = <&smi_common>;
+			power-domains = <&scpsys MT8173_POWER_DOMAIN_VENC>;
+			clocks = <&vencsys CLK_VENC_CKE1>,
+				 <&vencsys CLK_VENC_CKE0>;
+			clock-names = "apb", "smi";
+		};
+
 		vencltsys: clock-controller@19000000 {
 			compatible = "mediatek,mt8173-vencltsys", "syscon";
 			reg = <0 0x19000000 0 0x1000>;
 			#clock-cells = <1>;
 		};
+
+		larb5: larb@19001000 {
+			compatible = "mediatek,mt8173-smi-larb";
+			reg = <0 0x19001000 0 0x1000>;
+			mediatek,smi = <&smi_common>;
+			power-domains = <&scpsys MT8173_POWER_DOMAIN_VENC_LT>;
+			clocks = <&vencltsys CLK_VENCLT_CKE1>,
+				 <&vencltsys CLK_VENCLT_CKE0>;
+			clock-names = "apb", "smi";
+		};
 	};
 };
 
-- 
1.8.1.1.dirty


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH v5 5/6] iommu/mediatek: Add mt8173 IOMMU driver
  2015-10-09  2:23 ` [PATCH v5 5/6] iommu/mediatek: Add mt8173 IOMMU driver Yong Wu
@ 2015-10-14 12:53   ` Joerg Roedel
  2015-10-26  5:23     ` Yong Wu
  2015-10-27 13:25   ` Robin Murphy
  1 sibling, 1 reply; 23+ messages in thread
From: Joerg Roedel @ 2015-10-14 12:53 UTC (permalink / raw)
  To: Yong Wu
  Cc: Thierry Reding, Mark Rutland, Matthias Brugger, Robin Murphy,
	Will Deacon, Daniel Kurtz, Tomasz Figa, Lucas Stach, Rob Herring,
	Catalin Marinas, linux-mediatek, Sasha Hauer, srv_heupstream,
	devicetree, linux-kernel, linux-arm-kernel, iommu, pebolle, arnd,
	mitchelh, Sricharan R, youhua.li, k.zhang, kendrick.hsu

On Fri, Oct 09, 2015 at 10:23:07AM +0800, Yong Wu wrote:
> +	/*
> +	 * There is a domain for each a iommu device in normal case.
> +	 * But MTK only has one iommu domain called the m4u domain which all
> +	 * the multimedia HW share. Here we reserve one as the m4u domain and
> +	 * free the others.
> +	 *
> +	 * And the attach_device that from __iommu_setup_dma_ops
> +	 * will be called earlier than probe.
> +	 */

Okay, with this being the case, you need to put all devices behind one
IOMMU into the same iommu-group, because the IOMMU can't really isolate
the devices from each other.

> +static int mtk_iommu_add_device(struct device *dev)
> +{
> +	struct iommu_group *group;
> +	struct mtk_iommu_client_priv *priv;
> +	struct mtk_iommu_domain *m4udom;
> +	struct iommu_domain *domain;
> +	int ret;
> +
> +	if (!dev->archdata.iommu) /* Not a iommu client device */
> +		return -ENODEV;
> +
> +	group = iommu_group_get(dev);
> +	if (!group) {
> +		group = iommu_group_alloc();
> +		if (IS_ERR(group)) {
> +			dev_err(dev, "Failed to allocate IOMMU group\n");
> +			return PTR_ERR(group);
> +		}
> +	}
> +
> +	ret = iommu_group_add_device(group, dev);
> +	if (ret) {
> +		dev_err(dev, "Failed to add IOMMU group\n");
> +		goto err_group_put;
> +	}
> +
> +	domain = iommu_get_domain_for_dev(dev);
> +	if (!domain) {
> +		/*
> +		 * Get the m4u iommu domain from the m4u device.
> +		 * Attach all the client devices into the m4u domain.
> +		 */
> +		priv = dev->archdata.iommu;
> +		m4udom = dev_get_drvdata(priv->m4udev);
> +		ret = iommu_attach_group(&m4udom->domain, group);
> +		if (ret)
> +			dev_err(dev, "Failed to attach IOMMU group\n");
> +	}
> +
> +err_group_put:
> +	iommu_group_put(group);
> +	return ret;
> +}

Here it looks like you are allocating one group for each device. As I
said, all devices need to be in one group.



	Joerg


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v5 3/6] iommu: add ARM short descriptor page table allocator
  2015-10-09  2:23 ` [PATCH v5 3/6] iommu: add ARM short descriptor page table allocator Yong Wu
@ 2015-10-14 12:54   ` Joerg Roedel
  2015-10-15 17:20     ` Will Deacon
  2015-11-06  8:42   ` Yong Wu
  1 sibling, 1 reply; 23+ messages in thread
From: Joerg Roedel @ 2015-10-14 12:54 UTC (permalink / raw)
  To: Yong Wu
  Cc: Thierry Reding, Mark Rutland, Matthias Brugger, Robin Murphy,
	Will Deacon, Daniel Kurtz, Tomasz Figa, Lucas Stach, Rob Herring,
	Catalin Marinas, linux-mediatek, Sasha Hauer, srv_heupstream,
	devicetree, linux-kernel, linux-arm-kernel, iommu, pebolle, arnd,
	mitchelh, Sricharan R, youhua.li, k.zhang, kendrick.hsu

On Fri, Oct 09, 2015 at 10:23:05AM +0800, Yong Wu wrote:
> This patch is for ARM Short Descriptor Format.
> 
> Signed-off-by: Yong Wu <yong.wu@mediatek.com>

I think it would be good if Will Deacon could have a look on that.

Will, any comments on this patch?



	Joerg



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v5 0/6] MT8173 IOMMU SUPPORT
  2015-10-09  2:23 [PATCH v5 0/6] MT8173 IOMMU SUPPORT Yong Wu
                   ` (5 preceding siblings ...)
  2015-10-09  2:23 ` [PATCH v5 6/6] dts: mt8173: Add iommu/smi nodes for mt8173 Yong Wu
@ 2015-10-14 12:56 ` Joerg Roedel
  2015-10-22  4:40   ` Yong Wu
  6 siblings, 1 reply; 23+ messages in thread
From: Joerg Roedel @ 2015-10-14 12:56 UTC (permalink / raw)
  To: Yong Wu
  Cc: Thierry Reding, Mark Rutland, Matthias Brugger, Robin Murphy,
	Will Deacon, Daniel Kurtz, Tomasz Figa, Lucas Stach, Rob Herring,
	Catalin Marinas, linux-mediatek, Sasha Hauer, srv_heupstream,
	devicetree, linux-kernel, linux-arm-kernel, iommu, pebolle, arnd,
	mitchelh, Sricharan R, youhua.li, k.zhang, kendrick.hsu

On Fri, Oct 09, 2015 at 10:23:02AM +0800, Yong Wu wrote:
> Yong Wu (6):
>   dt-bindings: iommu: Add binding for mediatek IOMMU
>   dt-bindings: mediatek: Add smi dts binding
>   iommu: add ARM short descriptor page table allocator
>   memory: mediatek: Add SMI driver
>   iommu/mediatek: Add mt8173 IOMMU driver
>   dts: mt8173: Add iommu/smi nodes for mt8173
> 
>  .../devicetree/bindings/iommu/mediatek,iommu.txt   |  61 ++
>  .../memory-controllers/mediatek,smi-larb.txt       |  25 +
>  .../bindings/memory-controllers/mediatek,smi.txt   |  24 +
>  arch/arm64/boot/dts/mediatek/mt8173.dtsi           |  81 ++
>  drivers/iommu/Kconfig                              |  33 +
>  drivers/iommu/Makefile                             |   2 +
>  drivers/iommu/io-pgtable-arm-short.c               | 827 +++++++++++++++++++++
>  drivers/iommu/io-pgtable-arm.c                     |   3 -
>  drivers/iommu/io-pgtable.c                         |   3 +
>  drivers/iommu/io-pgtable.h                         |  18 +-
>  drivers/iommu/mtk_iommu.c                          | 767 +++++++++++++++++++
>  drivers/memory/Kconfig                             |   8 +
>  drivers/memory/Makefile                            |   1 +
>  drivers/memory/mtk-smi.c                           | 274 +++++++
>  include/dt-bindings/memory/mt8173-larb-port.h      | 105 +++
>  include/soc/mediatek/smi.h                         |  60 ++

So this contains not only IOMMU code. Are the patches dependent on each
other or can the iommu parts be merged independently?



	Joerg


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v5 3/6] iommu: add ARM short descriptor page table allocator
  2015-10-14 12:54   ` Joerg Roedel
@ 2015-10-15 17:20     ` Will Deacon
  0 siblings, 0 replies; 23+ messages in thread
From: Will Deacon @ 2015-10-15 17:20 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Yong Wu, Thierry Reding, Mark Rutland, Matthias Brugger,
	Robin Murphy, Daniel Kurtz, Tomasz Figa, Lucas Stach,
	Rob Herring, Catalin Marinas, linux-mediatek, Sasha Hauer,
	srv_heupstream, devicetree, linux-kernel, linux-arm-kernel,
	iommu, pebolle, arnd, mitchelh, Sricharan R, youhua.li, k.zhang,
	kendrick.hsu

On Wed, Oct 14, 2015 at 02:54:19PM +0200, Joerg Roedel wrote:
> On Fri, Oct 09, 2015 at 10:23:05AM +0800, Yong Wu wrote:
> > This patch is for ARM Short Descriptor Format.
> > 
> > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> 
> I think it would be good if Will Deacon could have a look on that.
> 
> Will, any comments on this patch?

Unfortunately, I haven't found time to take a good look at this version
(this patch usually takes the best part of a day to review properly)
and I'm away on holiday all next week.

I can review it when I'm back and have got back on top of email, but
that probably doesn't work well with 4.4, unfortunately. Robin and/or
Catalin may be able to review it in my absence, but it's hard work...

Will

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v5 0/6] MT8173 IOMMU SUPPORT
  2015-10-14 12:56 ` [PATCH v5 0/6] MT8173 IOMMU SUPPORT Joerg Roedel
@ 2015-10-22  4:40   ` Yong Wu
  2015-10-23  9:26     ` Joerg Roedel
  0 siblings, 1 reply; 23+ messages in thread
From: Yong Wu @ 2015-10-22  4:40 UTC (permalink / raw)
  To: Joerg Roedel, Matthias Brugger, arnd
  Cc: Thierry Reding, Mark Rutland, Robin Murphy, Will Deacon,
	Daniel Kurtz, Tomasz Figa, Lucas Stach, Rob Herring,
	Catalin Marinas, linux-mediatek, Sasha Hauer, srv_heupstream,
	devicetree, linux-kernel, linux-arm-kernel, iommu, pebolle,
	mitchelh, Sricharan R

On Wed, 2015-10-14 at 14:56 +0200, Joerg Roedel wrote:
> On Fri, Oct 09, 2015 at 10:23:02AM +0800, Yong Wu wrote:
> > Yong Wu (6):
> >   dt-bindings: iommu: Add binding for mediatek IOMMU
> >   dt-bindings: mediatek: Add smi dts binding
> >   iommu: add ARM short descriptor page table allocator
> >   memory: mediatek: Add SMI driver
> >   iommu/mediatek: Add mt8173 IOMMU driver
> >   dts: mt8173: Add iommu/smi nodes for mt8173
> > 
> >  .../devicetree/bindings/iommu/mediatek,iommu.txt   |  61 ++
> >  .../memory-controllers/mediatek,smi-larb.txt       |  25 +
> >  .../bindings/memory-controllers/mediatek,smi.txt   |  24 +
> >  arch/arm64/boot/dts/mediatek/mt8173.dtsi           |  81 ++
> >  drivers/iommu/Kconfig                              |  33 +
> >  drivers/iommu/Makefile                             |   2 +
> >  drivers/iommu/io-pgtable-arm-short.c               | 827 +++++++++++++++++++++
> >  drivers/iommu/io-pgtable-arm.c                     |   3 -
> >  drivers/iommu/io-pgtable.c                         |   3 +
> >  drivers/iommu/io-pgtable.h                         |  18 +-
> >  drivers/iommu/mtk_iommu.c                          | 767 +++++++++++++++++++
> >  drivers/memory/Kconfig                             |   8 +
> >  drivers/memory/Makefile                            |   1 +
> >  drivers/memory/mtk-smi.c                           | 274 +++++++
> >  include/dt-bindings/memory/mt8173-larb-port.h      | 105 +++
> >  include/soc/mediatek/smi.h                         |  60 ++
> 
> So this contains not only IOMMU code. Are the patches dependent on each
> other or can the iommu parts be merged independently?
> 
> 	Joerg
> 
Hi Joerg,
     Sorry for reply late.
     The ARM short descriptor is independent. This one can be merged
independently.

     But the mtk-iommu depend on the drivers/memory/mtk-smi.c(mtk-iommu
has called a function of the mtk-smi).
     So if there is dependence here, How should we do to merge them?



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v5 0/6] MT8173 IOMMU SUPPORT
  2015-10-22  4:40   ` Yong Wu
@ 2015-10-23  9:26     ` Joerg Roedel
  2015-11-24  5:58       ` Yong Wu
  0 siblings, 1 reply; 23+ messages in thread
From: Joerg Roedel @ 2015-10-23  9:26 UTC (permalink / raw)
  To: Yong Wu
  Cc: Matthias Brugger, arnd, Thierry Reding, Mark Rutland,
	Robin Murphy, Will Deacon, Daniel Kurtz, Tomasz Figa,
	Lucas Stach, Rob Herring, Catalin Marinas, linux-mediatek,
	Sasha Hauer, srv_heupstream, devicetree, linux-kernel,
	linux-arm-kernel, iommu, pebolle, mitchelh, Sricharan R

On Thu, Oct 22, 2015 at 12:40:02PM +0800, Yong Wu wrote:
>      But the mtk-iommu depend on the drivers/memory/mtk-smi.c(mtk-iommu
> has called a function of the mtk-smi).
>      So if there is dependence here, How should we do to merge them?

I can surely merge mtk-smi too, if it gets proper Reviewed-by and
Acked-by tags from the maintainer(s).


	Joerg


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v5 5/6] iommu/mediatek: Add mt8173 IOMMU driver
  2015-10-14 12:53   ` Joerg Roedel
@ 2015-10-26  5:23     ` Yong Wu
  0 siblings, 0 replies; 23+ messages in thread
From: Yong Wu @ 2015-10-26  5:23 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Thierry Reding, Mark Rutland, Matthias Brugger, Robin Murphy,
	Will Deacon, Daniel Kurtz, Tomasz Figa, Lucas Stach, Rob Herring,
	Catalin Marinas, linux-mediatek, Sasha Hauer, srv_heupstream,
	devicetree, linux-kernel, linux-arm-kernel, iommu, pebolle, arnd,
	mitchelh, Sricharan R, youhua.li, k.zhang, kendrick.hsu

On Wed, 2015-10-14 at 14:53 +0200, Joerg Roedel wrote:
> On Fri, Oct 09, 2015 at 10:23:07AM +0800, Yong Wu wrote:
> > +	/*
> > +	 * There is a domain for each a iommu device in normal case.
> > +	 * But MTK only has one iommu domain called the m4u domain which all
> > +	 * the multimedia HW share. Here we reserve one as the m4u domain and
> > +	 * free the others.
> > +	 *
> > +	 * And the attach_device that from __iommu_setup_dma_ops
> > +	 * will be called earlier than probe.
> > +	 */
> 
> Okay, with this being the case, you need to put all devices behind one
> IOMMU into the same iommu-group, because the IOMMU can't really isolate
> the devices from each other.
> 
> > +static int mtk_iommu_add_device(struct device *dev)
> > +{
> > +	struct iommu_group *group;
> > +	struct mtk_iommu_client_priv *priv;
> > +	struct mtk_iommu_domain *m4udom;
> > +	struct iommu_domain *domain;
> > +	int ret;
> > +
> > +	if (!dev->archdata.iommu) /* Not a iommu client device */
> > +		return -ENODEV;
> > +
> > +	group = iommu_group_get(dev);
> > +	if (!group) {
> > +		group = iommu_group_alloc();
> > +		if (IS_ERR(group)) {
> > +			dev_err(dev, "Failed to allocate IOMMU group\n");
> > +			return PTR_ERR(group);
> > +		}
> > +	}
> > +
> > +	ret = iommu_group_add_device(group, dev);
> > +	if (ret) {
> > +		dev_err(dev, "Failed to add IOMMU group\n");
> > +		goto err_group_put;
> > +	}
> > +
> > +	domain = iommu_get_domain_for_dev(dev);
> > +	if (!domain) {
> > +		/*
> > +		 * Get the m4u iommu domain from the m4u device.
> > +		 * Attach all the client devices into the m4u domain.
> > +		 */
> > +		priv = dev->archdata.iommu;
> > +		m4udom = dev_get_drvdata(priv->m4udev);
> > +		ret = iommu_attach_group(&m4udom->domain, group);
> > +		if (ret)
> > +			dev_err(dev, "Failed to attach IOMMU group\n");
> > +	}
> > +
> > +err_group_put:
> > +	iommu_group_put(group);
> > +	return ret;
> > +}
> 
> Here it looks like you are allocating one group for each device. As I
> said, all devices need to be in one group.
> 
> 	Joerg
> 

Thanks for this suggestion. I have put all the iommu client devices into
the same iommu group, the code looks like below.
And I will send this in the next version after the Short descriptor is
reviewed.


static int mtk_iommu_add_device(struct device *dev)
{
	struct iommu_group *group;
	struct mtk_iommu_client_priv *priv;
	struct mtk_iommu_domain *m4udom;
	struct iommu_domain *domain;
	int ret;

	priv = dev->archdata.iommu;
	if (!priv) /* Not a iommu client device */
		return -ENODEV;
	m4udom = dev_get_drvdata(priv->m4udev);

	group = iommu_group_get(dev);
	if (!group) {
		/*
		 * All the iommu client devices are in the m4u domain,
		 * they all are in the same m4u iommu-group too here.
		 */
		if (!m4udom->m4u_group) {
			group = iommu_group_alloc();
			if (IS_ERR(group)) {
				dev_err(dev, "Failed to allocate IOMMU group\n");
				return PTR_ERR(group);
			}
			m4udom->m4u_group = group;
		} else {
			group = m4udom->m4u_group;
		}
	}

	ret = iommu_group_add_device(group, dev);
	if (ret) {
		dev_err(dev, "Failed to add IOMMU group\n");
		goto err_group_put;
	}

	domain = iommu_get_domain_for_dev(dev);
	if (!domain)
		ret = iommu_attach_group(&m4udom->domain, group);

err_group_put:
	iommu_group_put(group);
	return ret;
}


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v5 4/6] memory: mediatek: Add SMI driver
  2015-10-09  2:23 ` [PATCH v5 4/6] memory: mediatek: Add SMI driver Yong Wu
@ 2015-10-27 13:24   ` Robin Murphy
  2015-10-31  8:32     ` Yong Wu
  0 siblings, 1 reply; 23+ messages in thread
From: Robin Murphy @ 2015-10-27 13:24 UTC (permalink / raw)
  To: Yong Wu, Joerg Roedel, Thierry Reding, Mark Rutland, Matthias Brugger
  Cc: Will Deacon, Daniel Kurtz, Tomasz Figa, Lucas Stach, Rob Herring,
	Catalin Marinas, linux-mediatek, Sasha Hauer, srv_heupstream,
	devicetree, linux-kernel, linux-arm-kernel, iommu, pebolle, arnd,
	mitchelh, Sricharan R, youhua.li, k.zhang, kendrick.hsu

On 09/10/15 03:23, Yong Wu wrote:
[...]
> +static int mtk_smi_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct mtk_smi_data *smidata;
> +	int ret;
> +
> +	if (!dev->pm_domain)
> +		return -EPROBE_DEFER;
> +
> +	smidata = devm_kzalloc(dev, sizeof(*smidata), GFP_KERNEL);
> +	if (!smidata)
> +		return -ENOMEM;
> +
> +	smidata->clk_apb = devm_clk_get(dev, "apb");
> +	if (IS_ERR(smidata->clk_apb))
> +		return PTR_ERR(smidata->clk_apb);
> +
> +	smidata->clk_smi = devm_clk_get(dev, "smi");
> +	if (IS_ERR(smidata->clk_smi))
> +		return PTR_ERR(smidata->clk_smi);
> +
> +	pm_runtime_enable(dev);
> +	dev_set_drvdata(dev, smidata);
> +	return ret;

ret is used uninitialised here, but you might as well just "return 0;" 
and get rid of the variable entirely.

> +}

Robin.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v5 5/6] iommu/mediatek: Add mt8173 IOMMU driver
  2015-10-09  2:23 ` [PATCH v5 5/6] iommu/mediatek: Add mt8173 IOMMU driver Yong Wu
  2015-10-14 12:53   ` Joerg Roedel
@ 2015-10-27 13:25   ` Robin Murphy
  2015-10-31  8:28     ` Yong Wu
  1 sibling, 1 reply; 23+ messages in thread
From: Robin Murphy @ 2015-10-27 13:25 UTC (permalink / raw)
  To: Yong Wu, Joerg Roedel, Thierry Reding, Mark Rutland, Matthias Brugger
  Cc: Will Deacon, Daniel Kurtz, Tomasz Figa, Lucas Stach, Rob Herring,
	Catalin Marinas, linux-mediatek, Sasha Hauer, srv_heupstream,
	devicetree, linux-kernel, linux-arm-kernel, iommu, pebolle, arnd,
	mitchelh, Sricharan R, youhua.li, k.zhang, kendrick.hsu

On 09/10/15 03:23, Yong Wu wrote:
[...]
> +#include <linux/io.h>
> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +#include <linux/module.h>
> +#include <linux/iommu.h>
> +#include <linux/dma-iommu.h>
> +#include <linux/of_iommu.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +#include <linux/list.h>
> +#include <linux/clk.h>
> +#include <linux/iopoll.h>

Nit: ordering?

> +#include <soc/mediatek/smi.h>
> +#include "io-pgtable.h"

[...]
> +struct mtk_iommu_data {
> +	void __iomem			*base;
> +	int				irq;
> +	struct device			*dev;
> +	struct device			*larbdev[MTK_IOMMU_LARB_MAX_NR];
> +	struct clk			*bclk;
> +	phys_addr_t			protect_base; /* protect memory base */
> +	int				larb_nr;/* local arbiter number */
> +	struct mtk_iommu_suspend_reg	reg;
> +};

I think I've finally got my head round the way this hardware works - 
each LARB can be configured to block or allow transactions from the 
client device behind each port, but they _don't_ otherwise pass any 
information downstream such that the M4U itself can identify individual 
transactions, right? If that is indeed the case, then Joerg is totally 
correct that all clients of one M4U should be in a single group, so you 
might as well keep a handy iommu_group pointer here. I'll refer back to 
that idea later...

[...]
> +static void mtk_iommu_clear_intr(const struct mtk_iommu_data *data)
> +{
> +	u32 val;
> +
> +	val = readl_relaxed(data->base + REG_MMU_INT_CONTROL0);
> +	val |= F_INT_L2_CLR_BIT;
> +	writel_relaxed(val, data->base + REG_MMU_INT_CONTROL0);
> +}

Do you anticipate any other callers of this? AFAICS these 3 lines could 
just be rolled into mtk_iommu_isr().

> +static void mtk_iommu_tlb_flush_all(void *cookie)
> +{
> +	struct mtk_iommu_domain *domain = cookie;
> +	void __iomem *base;
> +
> +	base = domain->data->base;
> +	writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0, base + REG_MMU_INV_SEL);
> +	writel_relaxed(F_ALL_INVLD, base + REG_MMU_INVALIDATE);
> +	mb();/* Make sure flush all done */

If it's purely to make sure the write has completed, would wmb() be 
sufficient here?

> +}
> +
> +static void mtk_iommu_tlb_add_flush(unsigned long iova, size_t size,
> +				    bool leaf, void *cookie)
> +{
> +	struct mtk_iommu_domain *domain = cookie;
> +	void __iomem *base = domain->data->base;
> +	unsigned int iova_start = iova, iova_end = iova + size - 1;

Nit: why not simply name the argument iova_start in the first place, or 
just use iova below?

> +	writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0, base + REG_MMU_INV_SEL);
> +
> +	writel_relaxed(iova_start, base + REG_MMU_INVLD_START_A);
> +	writel_relaxed(iova_end, base + REG_MMU_INVLD_END_A);
> +	writel_relaxed(F_MMU_INV_RANGE, base + REG_MMU_INVALIDATE);
> +}
> +
> +static void mtk_iommu_tlb_sync(void *cookie)
> +{
> +	struct mtk_iommu_domain *domain = cookie;
> +	void __iomem *base = domain->data->base;
> +	int ret;
> +	u32 tmp;
> +
> +	ret = readl_poll_timeout_atomic(base + REG_MMU_CPE_DONE, tmp,
> +					tmp != 0, 10, 1000000);
> +	if (ret) {
> +		dev_warn(domain->data->dev,
> +			 "Partial TLB flush timed out, falling back to full flush\n");
> +		mtk_iommu_tlb_flush_all(cookie);
> +	}
> +	writel_relaxed(0, base + REG_MMU_CPE_DONE);

Do you still need this writeback in the ret==0 case when you've already 
read CPE_DONE as 0, or should this be inside the condition? (in which 
case you could also use an early return to lose the indent)

> +}
[...]
> +static int mtk_iommu_hw_init(const struct mtk_iommu_domain *mtkdom)
> +{
> +	struct mtk_iommu_data *data = mtkdom->data;
> +	void __iomem *base = data->base;
> +	u32 regval;
> +	int ret;
> +
> +	ret = clk_prepare_enable(data->bclk);
> +	if (ret) {
> +		dev_err(data->dev, "Failed to enable iommu clk(%d)\n", ret);
> +		return ret;
> +	}

I'm not sure about the asymmetry here; the clock gets enabled when 
attaching clients to a domain, but not disabled until the IOMMU itself 
is torn down in mtk_iommu_remove() (i.e. never). It seems like either 
the clock should be enabled in mtk_iommu_probe(), or disabled in domain 
detach.

> +	writel_relaxed(mtkdom->cfg.arm_short_cfg.ttbr[0],
> +		       base + REG_MMU_PT_BASE_ADDR);
> +
> +	regval = F_MMU_PREFETCH_RT_REPLACE_MOD |
> +		F_MMU_TF_PROTECT_SEL(2) |
> +		F_COHERENCE_EN;
> +	writel_relaxed(regval, base + REG_MMU_CTRL_REG);
> +
> +	regval = F_L2_MULIT_HIT_EN |
> +		F_TABLE_WALK_FAULT_INT_EN |
> +		F_PREETCH_FIFO_OVERFLOW_INT_EN |
> +		F_MISS_FIFO_OVERFLOW_INT_EN |
> +		F_PREFETCH_FIFO_ERR_INT_EN |
> +		F_MISS_FIFO_ERR_INT_EN;
> +	writel_relaxed(regval, base + REG_MMU_INT_CONTROL0);
> +
> +	regval = F_INT_TRANSLATION_FAULT |
> +		F_INT_MAIN_MULTI_HIT_FAULT |
> +		F_INT_INVALID_PA_FAULT |
> +		F_INT_ENTRY_REPLACEMENT_FAULT |
> +		F_INT_TLB_MISS_FAULT |
> +		F_INT_MISS_TRANSATION_FIFO_FAULT |
> +		F_INT_PRETETCH_TRANSATION_FIFO_FAULT;
> +	writel_relaxed(regval, base + REG_MMU_INT_MAIN_CONTROL);
> +
> +	regval = ALIGN(data->protect_base, MTK_PROTECT_PA_ALIGN);
> +	regval = F_MMU_IVRP_PA_SET(regval);

 From the look of it, it might not hurt to just fold the ALIGN() into 
the F_MMU_IVRP_PA_SET() macro itself.

> +	writel_relaxed(regval, base + REG_MMU_IVRP_PADDR);
> +
> +	writel_relaxed(0, base + REG_MMU_DCM_DIS);
> +	writel_relaxed(0, base + REG_MMU_STANDARD_AXI_MODE);
> +
> +	if (devm_request_irq(data->dev, data->irq, mtk_iommu_isr, 0,
> +			     dev_name(data->dev), (void *)mtkdom)) {
> +		writel_relaxed(0, base + REG_MMU_PT_BASE_ADDR);
> +		clk_disable_unprepare(data->bclk);
> +		dev_err(data->dev, "Failed @ IRQ-%d Request\n", data->irq);
> +		return -ENODEV;
> +	}

Maybe balance this with a devm_free_irq() in mtk_iommu_domain_free()? 
(otherwise it's hanging around forever since the platform bus never 
seems to get destroyed)

> +	return 0;
> +}
> +
> +static int mtk_iommu_config(struct mtk_iommu_domain *mtkdom
> +			    struct device *dev, bool enable)
> +{
> +	struct mtk_iommu_data *data = mtkdom->data;
> +	struct mtk_iommu_client_priv *head, *cur, *next;
> +
> +	head = dev->archdata.iommu;
> +	list_for_each_entry_safe(cur, next, &head->client, client) {
> +		if (cur->larbid >= data->larb_nr) {
> +			dev_err(data->dev, "Invalid larb:%d\n", cur->larbid);
> +			return -EINVAL;
> +		}
> +
> +		mtk_smi_config_port(data->larbdev[cur->larbid],
> +				    cur->portid, enable);
> +		if (!enable) {
> +			list_del(&cur->client);
> +			kfree(cur);
> +		}

This list wasn't created by attach_device(), so it doesn't look right 
that detach_device() should cause it to be freed - I think this teardown 
belongs in mtk_iommu_remove_device(), as the counterpoint to the 
of_xlate/add_device operation.

> +	}
> +
> +	if (!enable) {
> +		kfree(head);
> +		dev->archdata.iommu = NULL;

Ditto.

> +	}
> +	return 0;
> +}
[...]
> +static int mtk_iommu_attach_device(struct iommu_domain *domain,
> +				   struct device *dev)
> +{
> +	struct mtk_iommu_domain *priv = to_mtk_domain(domain), *m4udom;
> +	struct iommu_group *group;
> +	struct mtk_iommu_client_priv *clientpriv;
> +	struct device *m4udev;
> +	int ret;
> +
> +	clientpriv = dev->archdata.iommu;
> +	if (!clientpriv)
> +		return -ENODEV;
> +	m4udev = clientpriv->m4udev;
> +
> +	/*
> +	 * There is a domain for each a iommu device in normal case.
> +	 * But MTK only has one iommu domain called the m4u domain which all
> +	 * the multimedia HW share. Here we reserve one as the m4u domain and
> +	 * free the others.
> +	 *
> +	 * And the attach_device that from __iommu_setup_dma_ops
> +	 * will be called earlier than probe.
> +	 */
> +	m4udom = dev_get_drvdata(m4udev);
> +	if (!m4udom)
> +		dev_set_drvdata(m4udev, priv);
> +	else if (m4udom != priv)
> +		iommu_domain_free(domain);

With the client devices in a single group, then I realise we shouldn't 
actually need any special handling of domains at all - we can freely 
create multiple domains, and since the group can only be attached to one 
at a time, all we do is point the hardware at the relevant page table on 
attach, and reset it on detach. That should make life somewhat easier, 
and means we no longer have to subvert the IOMMU API like this.

> +	group = iommu_group_get(dev);
> +	if (!group)

Either way you shouldn't need this - you've already bailed out if this 
isn't one of your client devices (via the dev->archdata.iommu check), 
and if it is, then it already has a group by virtue of 
mtk_iommu_add_device()...

> +		return 0;

...and regardless, indicating success without attaching anything to 
anything looks very off.

> +	iommu_group_put(group);
> +
> +	/* Initial the m4u domain context which is from the add_device */
> +	ret = mtk_iommu_init_domain_context(priv);
> +	if (ret)
> +		return ret;
> +
> +	return mtk_iommu_config(priv, dev, true);
> +}
[...]
> +static int mtk_iommu_add_device(struct device *dev)
> +{
> +	struct iommu_group *group;
> +	struct mtk_iommu_client_priv *priv;
> +	struct mtk_iommu_domain *m4udom;
> +	struct iommu_domain *domain;
> +	int ret;
> +
> +	if (!dev->archdata.iommu) /* Not a iommu client device */
> +		return -ENODEV;
> +
> +	group = iommu_group_get(dev);

If this became just a case of looking up mtk_iommu_data->group in 
archdata.iommu and adding this device to it, then everything else here 
should be able to go away - the arch code will create a default domain 
for the first device in the group, then sees each subsequent device 
appear in that domain as you add them, so just sets their dma_ops 
without any further interference (I have tested multi-device groups!)

> +	if (!group) {
> +		group = iommu_group_alloc();
> +		if (IS_ERR(group)) {
> +			dev_err(dev, "Failed to allocate IOMMU group\n");
> +			return PTR_ERR(group);
> +		}
> +	}

(although you might still need the lazy group allocation here if 
mtk_iommu_probe() turns out to run too early to do it).

> +	ret = iommu_group_add_device(group, dev);
> +	if (ret) {
> +		dev_err(dev, "Failed to add IOMMU group\n");
> +		goto err_group_put;
> +	}
> +
> +	domain = iommu_get_domain_for_dev(dev);
> +	if (!domain) {
> +		/*
> +		 * Get the m4u iommu domain from the m4u device.
> +		 * Attach all the client devices into the m4u domain.
> +		 */
> +		priv = dev->archdata.iommu;
> +		m4udom = dev_get_drvdata(priv->m4udev);
> +		ret = iommu_attach_group(&m4udom->domain, group);
> +		if (ret)
> +			dev_err(dev, "Failed to attach IOMMU group\n");
> +	}
> +
> +err_group_put:
> +	iommu_group_put(group);
> +	return ret;
> +}
[...]
> +static int mtk_iommu_init_fn(struct device_node *np)
> +{
> +	struct platform_device *pdev;
> +
> +	pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);

Hmm, is it OK that the driver isn't yet registered at this point? If you 
can guarantee that none of the client devices will also be registering 
their drivers at subsys_initcall level, then I guess it works out 
reasonably safe in practice, but it still smells a bit racy.

> +	if (IS_ERR(pdev))
> +		return PTR_ERR(pdev);
> +
> +	of_iommu_set_ops(np, &mtk_iommu_ops);
> +
> +	return 0;
> +}
> +
> +IOMMU_OF_DECLARE(mtkm4u, "mediatek,mt8173-m4u", mtk_iommu_init_fn);
> +
> +static int mtk_iommu_probe(struct platform_device *pdev)
> +{
> +	struct mtk_iommu_data   *data;
> +	struct device           *dev = &pdev->dev;
> +	struct mtk_iommu_domain *m4udom;
> +	void __iomem	        *protect;
> +	int                     ret;
> +
> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +	data->dev = dev;
> +
> +	/* Protect memory. HW will access here while translation fault.*/
> +	protect = devm_kzalloc(dev, MTK_PROTECT_PA_ALIGN * 2, GFP_KERNEL);
> +	if (!protect)
> +		return -ENOMEM;
> +	data->protect_base = virt_to_phys(protect);
> +
> +	ret = mtk_iommu_parse_dt(pdev, data);
> +	if (ret)
> +		return ret;

Hopefully you could allocate your group here and avoid the extra 
complication in add_device, but it might be problematic if the early 
device creation means this gets called before sysfs is fully up and running.

> +	m4udom = dev_get_drvdata(dev);
> +	if (m4udom)
> +		m4udom->data = data;
> +
> +	return 0;
> +}
[...]
> +static int mtk_iommu_resume(struct device *dev)
> +{
> +	struct mtk_iommu_domain *mtkdom = dev_get_drvdata(dev);
> +	struct mtk_iommu_suspend_reg *reg;
> +	void __iomem *base;
> +
> +	if (!mtkdom)
> +		return 0;
> +
> +	reg = &mtkdom->data->reg;
> +	base = mtkdom->data->base;
> +	writel_relaxed(mtkdom->cfg.arm_short_cfg.ttbr[0],
> +		       base + REG_MMU_PT_BASE_ADDR);
> +	writel_relaxed(reg->standard_axi_mode,
> +		       base + REG_MMU_STANDARD_AXI_MODE);
> +	writel_relaxed(reg->dcm_dis, base + REG_MMU_DCM_DIS);
> +	writel_relaxed(reg->ctrl_reg, base + REG_MMU_CTRL_REG);
> +	writel_relaxed(reg->ivrp_paddr, base + REG_MMU_IVRP_PADDR);

On closer inspection, it looks pretty cheap to recalculate this one from 
data->protect_base, so perhaps that could be one less thing to save.

> +	writel_relaxed(reg->int_control0, base + REG_MMU_INT_CONTROL0);
> +	writel_relaxed(reg->int_main_control, base + REG_MMU_INT_MAIN_CONTROL);
> +	return 0;
> +}

Robin.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v5 5/6] iommu/mediatek: Add mt8173 IOMMU driver
  2015-10-27 13:25   ` Robin Murphy
@ 2015-10-31  8:28     ` Yong Wu
  0 siblings, 0 replies; 23+ messages in thread
From: Yong Wu @ 2015-10-31  8:28 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Joerg Roedel, Thierry Reding, Mark Rutland, Matthias Brugger,
	Will Deacon, Daniel Kurtz, Tomasz Figa, Lucas Stach, Rob Herring,
	Catalin Marinas, linux-mediatek, Sasha Hauer, srv_heupstream,
	devicetree, linux-kernel, linux-arm-kernel, iommu, pebolle, arnd,
	mitchelh, Sricharan R, youhua.li, k.zhang, kendrick.hsu

On Tue, 2015-10-27 at 13:25 +0000, Robin Murphy wrote:
> On 09/10/15 03:23, Yong Wu wrote:
> [...]
> > +#include <linux/io.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/module.h>
> > +#include <linux/iommu.h>
> > +#include <linux/dma-iommu.h>
> > +#include <linux/of_iommu.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/list.h>
> > +#include <linux/clk.h>
> > +#include <linux/iopoll.h>
> 
> Nit: ordering?

Yes. Thanks. I will order them in next time.

> 
> > +#include <soc/mediatek/smi.h>
> > +#include "io-pgtable.h"
> 
> [...]
> > +struct mtk_iommu_data {
> > +	void __iomem			*base;
> > +	int				irq;
> > +	struct device			*dev;
> > +	struct device			*larbdev[MTK_IOMMU_LARB_MAX_NR];
> > +	struct clk			*bclk;
> > +	phys_addr_t			protect_base; /* protect memory base */
> > +	int				larb_nr;/* local arbiter number */
> > +	struct mtk_iommu_suspend_reg	reg;
> > +};
> 
> I think I've finally got my head round the way this hardware works - 
> each LARB can be configured to block or allow transactions from the 
> client device behind each port, but they _don't_ otherwise pass any 
> information downstream such that the M4U itself can identify individual 
> transactions, right? If that is indeed the case, then Joerg is totally 
> correct that all clients of one M4U should be in a single group, so you 
> might as well keep a handy iommu_group pointer here. I'll refer back to 
> that idea later...

I will put them into a single group and rebase Joerg's patchset(iommu:
Make core iommu-groups code more generic) too. see later.

> 
> [...]
> > +static void mtk_iommu_clear_intr(const struct mtk_iommu_data *data)
> > +{
> > +	u32 val;
> > +
> > +	val = readl_relaxed(data->base + REG_MMU_INT_CONTROL0);
> > +	val |= F_INT_L2_CLR_BIT;
> > +	writel_relaxed(val, data->base + REG_MMU_INT_CONTROL0);
> > +}
> 
> Do you anticipate any other callers of this? AFAICS these 3 lines could 
> just be rolled into mtk_iommu_isr().

Yes. Thanks. We can put it into the isr.

> 
> > +static void mtk_iommu_tlb_flush_all(void *cookie)
> > +{
> > +	struct mtk_iommu_domain *domain = cookie;
> > +	void __iomem *base;
> > +
> > +	base = domain->data->base;
> > +	writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0, base + REG_MMU_INV_SEL);
> > +	writel_relaxed(F_ALL_INVLD, base + REG_MMU_INVALIDATE);
> > +	mb();/* Make sure flush all done */
> 
> If it's purely to make sure the write has completed, would wmb() be 
> sufficient here?

YES.

> 
> > +}
> > +
> > +static void mtk_iommu_tlb_add_flush(unsigned long iova, size_t size,
> > +				    bool leaf, void *cookie)
> > +{
> > +	struct mtk_iommu_domain *domain = cookie;
> > +	void __iomem *base = domain->data->base;
> > +	unsigned int iova_start = iova, iova_end = iova + size - 1;
> 
> Nit: why not simply name the argument iova_start in the first place, or 
> just use iova below?

I will delete them and just use "iova" below.

> 
> > +	writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0, base + REG_MMU_INV_SEL);
> > +
> > +	writel_relaxed(iova_start, base + REG_MMU_INVLD_START_A);
> > +	writel_relaxed(iova_end, base + REG_MMU_INVLD_END_A);
> > +	writel_relaxed(F_MMU_INV_RANGE, base + REG_MMU_INVALIDATE);
> > +}
> > +
> > +static void mtk_iommu_tlb_sync(void *cookie)
> > +{
> > +	struct mtk_iommu_domain *domain = cookie;
> > +	void __iomem *base = domain->data->base;
> > +	int ret;
> > +	u32 tmp;
> > +
> > +	ret = readl_poll_timeout_atomic(base + REG_MMU_CPE_DONE, tmp,
> > +					tmp != 0, 10, 1000000);
> > +	if (ret) {
> > +		dev_warn(domain->data->dev,
> > +			 "Partial TLB flush timed out, falling back to full flush\n");
> > +		mtk_iommu_tlb_flush_all(cookie);
> > +	}
> > +	writel_relaxed(0, base + REG_MMU_CPE_DONE);
> 
> Do you still need this writeback in the ret==0 case when you've already 
> read CPE_DONE as 0, or should this be inside the condition? (in which 
> case you could also use an early return to lose the indent)

Yes. I still need this writeback in the ret==0 case.

REG_MMU_CPE_DONE is the state of CPE. It's 1 while the non-secure range
invalidation is done. Write 1'b0 is for the purpose of clearing the
current status, then we could check the next status again. So "write 1"
is also needed in ret==0 case.

so I will keep it and add a comment for more readable here.

writel_relaxed(0, base + REG_MMU_CPE_DONE);/* Clear the CPE state */

> 
> > +}
> [...]
> > +static int mtk_iommu_hw_init(const struct mtk_iommu_domain *mtkdom)
> > +{
> > +	struct mtk_iommu_data *data = mtkdom->data;
> > +	void __iomem *base = data->base;
> > +	u32 regval;
> > +	int ret;
> > +
> > +	ret = clk_prepare_enable(data->bclk);
> > +	if (ret) {
> > +		dev_err(data->dev, "Failed to enable iommu clk(%d)\n", ret);
> > +		return ret;
> > +	}
> 
> I'm not sure about the asymmetry here; the clock gets enabled when 
> attaching clients to a domain, but not disabled until the IOMMU itself 
> is torn down in mtk_iommu_remove() (i.e. never). It seems like either 
> the clock should be enabled in mtk_iommu_probe(), or disabled in domain 
> detach.

>From your comment, I realize that some places here is not asymmetry.
I will move this mtk_iommu_hw_init into mtk_iommu_probe.

> 
> > +	writel_relaxed(mtkdom->cfg.arm_short_cfg.ttbr[0],
> > +		       base + REG_MMU_PT_BASE_ADDR);
> > +
> > +	regval = F_MMU_PREFETCH_RT_REPLACE_MOD |
> > +		F_MMU_TF_PROTECT_SEL(2) |
> > +		F_COHERENCE_EN;
> > +	writel_relaxed(regval, base + REG_MMU_CTRL_REG);
> > +
> > +	regval = F_L2_MULIT_HIT_EN |
> > +		F_TABLE_WALK_FAULT_INT_EN |
> > +		F_PREETCH_FIFO_OVERFLOW_INT_EN |
> > +		F_MISS_FIFO_OVERFLOW_INT_EN |
> > +		F_PREFETCH_FIFO_ERR_INT_EN |
> > +		F_MISS_FIFO_ERR_INT_EN;
> > +	writel_relaxed(regval, base + REG_MMU_INT_CONTROL0);
> > +
> > +	regval = F_INT_TRANSLATION_FAULT |
> > +		F_INT_MAIN_MULTI_HIT_FAULT |
> > +		F_INT_INVALID_PA_FAULT |
> > +		F_INT_ENTRY_REPLACEMENT_FAULT |
> > +		F_INT_TLB_MISS_FAULT |
> > +		F_INT_MISS_TRANSATION_FIFO_FAULT |
> > +		F_INT_PRETETCH_TRANSATION_FIFO_FAULT;
> > +	writel_relaxed(regval, base + REG_MMU_INT_MAIN_CONTROL);
> > +
> > +	regval = ALIGN(data->protect_base, MTK_PROTECT_PA_ALIGN);
> > +	regval = F_MMU_IVRP_PA_SET(regval);
> 
>  From the look of it, it might not hurt to just fold the ALIGN() into 
> the F_MMU_IVRP_PA_SET() macro itself.

      If the ALIGN is moved into the macro, that line will be over 80
chars. so I seperate them.
      In the next version I will align data->protect_base in probe,
then the code here will be more simply. and it also could be used in
mtk_iommu_resume.
      
> 
> > +	writel_relaxed(regval, base + REG_MMU_IVRP_PADDR);
> > +
> > +	writel_relaxed(0, base + REG_MMU_DCM_DIS);
> > +	writel_relaxed(0, base + REG_MMU_STANDARD_AXI_MODE);
> > +
> > +	if (devm_request_irq(data->dev, data->irq, mtk_iommu_isr, 0,
> > +			     dev_name(data->dev), (void *)mtkdom)) {
> > +		writel_relaxed(0, base + REG_MMU_PT_BASE_ADDR);
> > +		clk_disable_unprepare(data->bclk);
> > +		dev_err(data->dev, "Failed @ IRQ-%d Request\n", data->irq);
> > +		return -ENODEV;
> > +	}
> 
> Maybe balance this with a devm_free_irq() in mtk_iommu_domain_free()? 
> (otherwise it's hanging around forever since the platform bus never 
> seems to get destroyed)

I will move the mtk_iommu_hw_init into mtk_iommu_probe, and add
devm_free_irq into mtk_iommu_remove.
(I cann't add devm_free_irq in mtk_iommu_domain_free and detach_device)

> 
> > +	return 0;
> > +}
> > +
> > +static int mtk_iommu_config(struct mtk_iommu_domain *mtkdom
> > +			    struct device *dev, bool enable)
> > +{
> > +	struct mtk_iommu_data *data = mtkdom->data;
> > +	struct mtk_iommu_client_priv *head, *cur, *next;
> > +
> > +	head = dev->archdata.iommu;
> > +	list_for_each_entry_safe(cur, next, &head->client, client) {
> > +		if (cur->larbid >= data->larb_nr) {
> > +			dev_err(data->dev, "Invalid larb:%d\n", cur->larbid);
> > +			return -EINVAL;
> > +		}
> > +
> > +		mtk_smi_config_port(data->larbdev[cur->larbid],
> > +				    cur->portid, enable);
> > +		if (!enable) {
> > +			list_del(&cur->client);
> > +			kfree(cur);
> > +		}
> 
> This list wasn't created by attach_device(), so it doesn't look right 
> that detach_device() should cause it to be freed - I think this teardown 
> belongs in mtk_iommu_remove_device(), as the counterpoint to the 
> of_xlate/add_device operation.

It is also asymmetry here. I will move kfree into
mtk_iommu_remove_device.

> 
> > +	}
> > +
> > +	if (!enable) {
> > +		kfree(head);
> > +		dev->archdata.iommu = NULL;
> 
> Ditto.
> 
> > +	}
> > +	return 0;
> > +}
> [...]
> > +static int mtk_iommu_attach_device(struct iommu_domain *domain,
> > +				   struct device *dev)
> > +{
> > +	struct mtk_iommu_domain *priv = to_mtk_domain(domain), *m4udom;
> > +	struct iommu_group *group;
> > +	struct mtk_iommu_client_priv *clientpriv;
> > +	struct device *m4udev;
> > +	int ret;
> > +
> > +	clientpriv = dev->archdata.iommu;
> > +	if (!clientpriv)
> > +		return -ENODEV;
> > +	m4udev = clientpriv->m4udev;
> > +
> > +	/*
> > +	 * There is a domain for each a iommu device in normal case.
> > +	 * But MTK only has one iommu domain called the m4u domain which all
> > +	 * the multimedia HW share. Here we reserve one as the m4u domain and
> > +	 * free the others.
> > +	 *
> > +	 * And the attach_device that from __iommu_setup_dma_ops
> > +	 * will be called earlier than probe.
> > +	 */
> > +	m4udom = dev_get_drvdata(m4udev);
> > +	if (!m4udom)
> > +		dev_set_drvdata(m4udev, priv);
> > +	else if (m4udom != priv)
> > +		iommu_domain_free(domain);
> 
> With the client devices in a single group, then I realise we shouldn't 
> actually need any special handling of domains at all - we can freely 
> create multiple domains, and since the group can only be attached to one 
> at a time, all we do is point the hardware at the relevant page table on 
> attach, and reset it on detach. That should make life somewhat easier, 
> and means we no longer have to subvert the IOMMU API like this.

"we can freely create multiple domains", 
Do we need to free the unnecessary domain here?

This function also is a little complex after changed to a single group.
see below.

> 
> > +	group = iommu_group_get(dev);
> > +	if (!group)
> 
> Either way you shouldn't need this - you've already bailed out if this 
> isn't one of your client devices (via the dev->archdata.iommu check), 
> and if it is, then it already has a group by virtue of 
> mtk_iommu_add_device()...

I think I can not use "dev->archdata.iommu" to break here.
Like the comment "the attach_device that from __iommu_setup_dma_ops
will be called earlier than probe."

If I use dev->archdata.iommu, the code will also go down. But at that
time some data has not been initialization(dev_get_drvdata is null).

So I use iommu_group_get to guarantee the sequence: The code could go
down only after the mtk_iommu_probe.


> 
> > +		return 0;
> 
> ...and regardless, indicating success without attaching anything to 
> anything looks very off.

I have to return "0" here.
I need the DMA help update dev->archdata.dma_ops into iommu_dma_ops.
(If I don't return 0 here, the do_iommu_attach will never update
archdata->dma_ops for the client device.)

If I am wrong, please tell me. Thanks.

> 
> > +	iommu_group_put(group);
> > +
> > +	/* Initial the m4u domain context which is from the add_device */
> > +	ret = mtk_iommu_init_domain_context(priv);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return mtk_iommu_config(priv, dev, true);
> > +}
> [...]
> > +static int mtk_iommu_add_device(struct device *dev)
> > +{
> > +	struct iommu_group *group;
> > +	struct mtk_iommu_client_priv *priv;
> > +	struct mtk_iommu_domain *m4udom;
> > +	struct iommu_domain *domain;
> > +	int ret;
> > +
> > +	if (!dev->archdata.iommu) /* Not a iommu client device */
> > +		return -ENODEV;
> > +
> > +	group = iommu_group_get(dev);
> 
> If this became just a case of looking up mtk_iommu_data->group in 
> archdata.iommu and adding this device to it, then everything else here 
> should be able to go away - the arch code will create a default domain 
> for the first device in the group, then sees each subsequent device 
> appear in that domain as you add them, so just sets their dma_ops 
> without any further interference (I have tested multi-device groups!)

Do you mean this function "iommu_group_get_for_dev" which will create a
default domain?
I also notice that Joerg's "iommu: Make core iommu-groups code more
generic" patchset has a device_group callback, then I could return the
same iommu group easily.
So I add this patchset and test, then the code flow is changed greatly.
see below.

> 
> > +	if (!group) {
> > +		group = iommu_group_alloc();
> > +		if (IS_ERR(group)) {
> > +			dev_err(dev, "Failed to allocate IOMMU group\n");
> > +			return PTR_ERR(group);
> > +		}
> > +	}
> 
> (although you might still need the lazy group allocation here if 
> mtk_iommu_probe() turns out to run too early to do it).
> 
> > +	ret = iommu_group_add_device(group, dev);
> > +	if (ret) {
> > +		dev_err(dev, "Failed to add IOMMU group\n");
> > +		goto err_group_put;
> > +	}
> > +
> > +	domain = iommu_get_domain_for_dev(dev);
> > +	if (!domain) {
> > +		/*
> > +		 * Get the m4u iommu domain from the m4u device.
> > +		 * Attach all the client devices into the m4u domain.
> > +		 */
> > +		priv = dev->archdata.iommu;
> > +		m4udom = dev_get_drvdata(priv->m4udev);
> > +		ret = iommu_attach_group(&m4udom->domain, group);
> > +		if (ret)
> > +			dev_err(dev, "Failed to attach IOMMU group\n");
> > +	}
> > +
> > +err_group_put:
> > +	iommu_group_put(group);
> > +	return ret;
> > +}
> [...]
> > +static int mtk_iommu_init_fn(struct device_node *np)
> > +{
> > +	struct platform_device *pdev;
> > +
> > +	pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
> 
> Hmm, is it OK that the driver isn't yet registered at this point? If you 
> can guarantee that none of the client devices will also be registering 
> their drivers at subsys_initcall level, then I guess it works out 
> reasonably safe in practice, but it still smells a bit racy.

Do you means that we could delete of_platform_device_create here if I
can guarantee none of the client devices is registered at
subsys_initcall level?

I think all the iommu client devices should be called after our iommu
device, but it will be more safe if there is of_platform_device_create
here.

sorry, I may misunderstand here.

> 
> > +	if (IS_ERR(pdev))
> > +		return PTR_ERR(pdev);
> > +
> > +	of_iommu_set_ops(np, &mtk_iommu_ops);
> > +
> > +	return 0;
> > +}
> > +
> > +IOMMU_OF_DECLARE(mtkm4u, "mediatek,mt8173-m4u", mtk_iommu_init_fn);
> > +
> > +static int mtk_iommu_probe(struct platform_device *pdev)
> > +{
> > +	struct mtk_iommu_data   *data;
> > +	struct device           *dev = &pdev->dev;
> > +	struct mtk_iommu_domain *m4udom;
> > +	void __iomem	        *protect;
> > +	int                     ret;
> > +
> > +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> > +	if (!data)
> > +		return -ENOMEM;
> > +	data->dev = dev;
> > +
> > +	/* Protect memory. HW will access here while translation fault.*/
> > +	protect = devm_kzalloc(dev, MTK_PROTECT_PA_ALIGN * 2, GFP_KERNEL);
> > +	if (!protect)
> > +		return -ENOMEM;
> > +	data->protect_base = virt_to_phys(protect);
> > +
> > +	ret = mtk_iommu_parse_dt(pdev, data);
> > +	if (ret)
> > +		return ret;
> 
> Hopefully you could allocate your group here and avoid the extra 
> complication in add_device, but it might be problematic if the early 
> device creation means this gets called before sysfs is fully up and running.

If I use Joerg's patch, then we don't need allocate the group here.

> 
> > +	m4udom = dev_get_drvdata(dev);
> > +	if (m4udom)
> > +		m4udom->data = data;
> > +
> > +	return 0;
> > +}
> [...]
> > +static int mtk_iommu_resume(struct device *dev)
> > +{
> > +	struct mtk_iommu_domain *mtkdom = dev_get_drvdata(dev);
> > +	struct mtk_iommu_suspend_reg *reg;
> > +	void __iomem *base;
> > +
> > +	if (!mtkdom)
> > +		return 0;
> > +
> > +	reg = &mtkdom->data->reg;
> > +	base = mtkdom->data->base;
> > +	writel_relaxed(mtkdom->cfg.arm_short_cfg.ttbr[0],
> > +		       base + REG_MMU_PT_BASE_ADDR);
> > +	writel_relaxed(reg->standard_axi_mode,
> > +		       base + REG_MMU_STANDARD_AXI_MODE);
> > +	writel_relaxed(reg->dcm_dis, base + REG_MMU_DCM_DIS);
> > +	writel_relaxed(reg->ctrl_reg, base + REG_MMU_CTRL_REG);
> > +	writel_relaxed(reg->ivrp_paddr, base + REG_MMU_IVRP_PADDR);
> 
> On closer inspection, it looks pretty cheap to recalculate this one from 
> data->protect_base, so perhaps that could be one less thing to save.

Yes, It's OK.

> 
> > +	writel_relaxed(reg->int_control0, base + REG_MMU_INT_CONTROL0);
> > +	writel_relaxed(reg->int_main_control, base + REG_MMU_INT_MAIN_CONTROL);
> > +	return 0;
> > +}
> 
> Robin.
> 

After add Joerg's patchset[1], I test locally and it looks ok.

I copy most of the code here for your confirm.
If there is some place I don't understand your comment exactly, please
help tell me.
(I will sent it as the next version after the Short-desc is reviewed.)

[1]:
http://lists.linuxfoundation.org/pipermail/iommu/2015-October/014764.html

//===========================

struct mtk_iommu_domain {
	struct imu_pgd_t		*pgd;
	spinlock_t			pgtlock; /* lock for page table */

	struct io_pgtable_cfg		cfg;
	struct io_pgtable_ops		*iop;

	struct iommu_domain		domain;
};

struct mtk_iommu_data {
	void __iomem			*base;
	int				irq;
	struct device			*dev;
	struct device			*larbdev[MTK_IOMMU_LARB_MAX_NR];
	struct clk			*bclk;
	phys_addr_t			protect_base; /* protect memory base */
	int				larb_nr;/* local arbiter number */
	struct mtk_iommu_suspend_reg	reg;
	struct mtk_iommu_domain		*m4udom;
	struct iommu_group		*m4ugroup;
};

static void mtk_iommu_tlb_flush_all(void *cookie)
{
	struct mtk_iommu_data *data = cookie;
	void __iomem *base = data->base;

	writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0, base + REG_MMU_INV_SEL);
	writel_relaxed(F_ALL_INVLD, base + REG_MMU_INVALIDATE);
	wmb();/* Make sure tlb flush all done */
}

static void mtk_iommu_tlb_add_flush(unsigned long iova, size_t size,
				    bool leaf, void *cookie)
{
	struct mtk_iommu_data *data = cookie;
	void __iomem *base = data->base;

	writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0, base + REG_MMU_INV_SEL);

	writel_relaxed(iova, base + REG_MMU_INVLD_START_A);
	writel_relaxed(iova + size - 1, base + REG_MMU_INVLD_END_A);
	writel_relaxed(F_MMU_INV_RANGE, base + REG_MMU_INVALIDATE);
}

static void mtk_iommu_tlb_sync(void *cookie)
{
	struct mtk_iommu_data *data = cookie;
	void __iomem *base = data->base;
	int ret;
	u32 tmp;

	ret = readl_poll_timeout_atomic(base + REG_MMU_CPE_DONE, tmp,
					tmp != 0, 10, 1000000);
	if (ret) {
		dev_warn(data->dev,
			 "Partial TLB flush timed out, falling back to full flush\n");
		mtk_iommu_tlb_flush_all(cookie);
	}
	writel_relaxed(0, base + REG_MMU_CPE_DONE); /* Clear the CPE status */
}

static struct iommu_gather_ops mtk_iommu_gather_ops = {
	.tlb_flush_all = mtk_iommu_tlb_flush_all,
	.tlb_add_flush = mtk_iommu_tlb_add_flush,
	.tlb_sync = mtk_iommu_tlb_sync,
};

static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
{
	struct mtk_iommu_data *data = dev_id;
	struct mtk_iommu_domain *mtkdom = data->m4udom;
	u32 int_state, regval, fault_iova, fault_pa;
	unsigned int fault_larb, fault_port;
	bool layer, write;

	/* Read error info from registers */
	int_state = readl_relaxed(data->base + REG_MMU_FAULT_ST1);
	fault_iova = readl_relaxed(data->base + REG_MMU_FAULT_VA);
	layer = fault_iova & F_MMU_FAULT_VA_LAYER_BIT;
	write = fault_iova & F_MMU_FAULT_VA_WRITE_BIT;
	fault_iova &= F_MMU_FAULT_VA_MSK;
	fault_pa = readl_relaxed(data->base + REG_MMU_INVLD_PA);
	regval = readl_relaxed(data->base + REG_MMU_INT_ID);
	fault_larb = F_MMU0_INT_ID_LARB_ID(regval);
	fault_port = F_MMU0_INT_ID_PORT_ID(regval);

	if (report_iommu_fault(&mtkdom->domain, data->dev, fault_iova,
			       write ? IOMMU_FAULT_WRITE : IOMMU_FAULT_READ)) {
		dev_err_ratelimited(
			data->dev,
			"fault type=0x%x iova=0x%x pa=0x%x larb=%d port=%d layer=%d %s\n",
			int_state, fault_iova, fault_pa, fault_larb, fault_port,
			layer, write ? "write" : "read");
	}

	/* Interrupt clear */
	regval = readl_relaxed(data->base + REG_MMU_INT_CONTROL0);
	regval |= F_INT_CLR_BIT;
	writel_relaxed(regval, data->base + REG_MMU_INT_CONTROL0);

	mtk_iommu_tlb_flush_all(data);

	return IRQ_HANDLED;
}

static int mtk_iommu_parse_dt(struct platform_device *pdev,
			      struct mtk_iommu_data *data)
{
	[...]
}

static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
{
	void __iomem *base = data->base;
	u32 regval;
	int ret;

	ret = clk_prepare_enable(data->bclk);
	if (ret) {
		dev_err(data->dev, "Failed to enable iommu bclk(%d)\n", ret);
		return ret;
	}

	regval = F_MMU_PREFETCH_RT_REPLACE_MOD |
		F_MMU_TF_PROTECT_SEL(2) |
		F_COHERENCE_EN;
	writel_relaxed(regval, base + REG_MMU_CTRL_REG);

	regval = F_L2_MULIT_HIT_EN |
		F_TABLE_WALK_FAULT_INT_EN |
		F_PREETCH_FIFO_OVERFLOW_INT_EN |
		F_MISS_FIFO_OVERFLOW_INT_EN |
		F_PREFETCH_FIFO_ERR_INT_EN |
		F_MISS_FIFO_ERR_INT_EN;
	writel_relaxed(regval, base + REG_MMU_INT_CONTROL0);

	regval = F_INT_TRANSLATION_FAULT |
		F_INT_MAIN_MULTI_HIT_FAULT |
		F_INT_INVALID_PA_FAULT |
		F_INT_ENTRY_REPLACEMENT_FAULT |
		F_INT_TLB_MISS_FAULT |
		F_INT_MISS_TRANSATION_FIFO_FAULT |
		F_INT_PRETETCH_TRANSATION_FIFO_FAULT;
	writel_relaxed(regval, base + REG_MMU_INT_MAIN_CONTROL);

	regval = F_MMU_IVRP_PA_SET(data->protect_base);
	writel_relaxed(regval, base + REG_MMU_IVRP_PADDR);

	writel_relaxed(0, base + REG_MMU_DCM_DIS);
	writel_relaxed(0, base + REG_MMU_STANDARD_AXI_MODE);

	if (devm_request_irq(data->dev, data->irq, mtk_iommu_isr, 0,
			     dev_name(data->dev), (void *)data)) {
		writel_relaxed(0, base + REG_MMU_PT_BASE_ADDR);
		clk_disable_unprepare(data->bclk);
		dev_err(data->dev, "Failed @ IRQ-%d Request\n", data->irq);
		return -ENODEV;
	}

	return 0;
}

static int mtk_iommu_config(struct mtk_iommu_data *data,
			    struct device *dev, bool enable)
{
	struct mtk_iommu_client_priv *head, *cur, *next;
	int ret;

	head = dev->archdata.iommu;
	list_for_each_entry_safe(cur, next, &head->client, client) {
		if (cur->larbid >= data->larb_nr) {
			dev_err(data->dev, "Invalid larb: %d\n", cur->larbid);
			return -EINVAL;
		}

		ret = mtk_smi_config_port(data->larbdev[cur->larbid],
					  cur->portid, enable);
		if (ret)
			return ret;
	}

	return 0;
}

static int mtk_iommu_init_domain_context(struct mtk_iommu_data *data)
{
	struct mtk_iommu_domain *dom = data->m4udom;

	if (dom->iop)
		return 0;

	spin_lock_init(&dom->pgtlock);
	dom->cfg.quirks = IO_PGTABLE_QUIRK_ARM_NS |
			IO_PGTABLE_QUIRK_NO_PERMS |
			IO_PGTABLE_QUIRK_TLBI_ON_MAP |
			IO_PGTABLE_QUIRK_SHORT_SUPERSECTION;
	dom->cfg.pgsize_bitmap = mtk_iommu_ops.pgsize_bitmap,
	dom->cfg.ias = 32;
	dom->cfg.oas = 32;
	dom->cfg.tlb = &mtk_iommu_gather_ops;
	dom->cfg.iommu_dev = data->dev;

	dom->iop = alloc_io_pgtable_ops(ARM_SHORT_DESC, &dom->cfg, data);
	if (!dom->iop) {
		dev_err(data->dev, "Failed to alloc io pgtable\n");
		return -EINVAL;
	}

	/* Update our support page sizes bitmap */
	mtk_iommu_ops.pgsize_bitmap = dom->cfg.pgsize_bitmap;

        /* Update the pagetable base address */
	writel_relaxed(data->m4udom->cfg.arm_short_cfg.ttbr[0],
		       data->base + REG_MMU_PT_BASE_ADDR);
	return 0;
}

static struct iommu_domain *mtk_iommu_domain_alloc(unsigned type)
{
	struct mtk_iommu_domain *priv;

	if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
		return NULL;

	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
	if (!priv)
		return NULL;

	if (type == IOMMU_DOMAIN_DMA && iommu_get_dma_cookie(&priv->domain)) {
		kfree(priv);
		return NULL;
	}

	priv->domain.geometry.aperture_start = 0;
	priv->domain.geometry.aperture_end = DMA_BIT_MASK(32);
	priv->domain.geometry.force_aperture = true;

	return &priv->domain;
}

static void mtk_iommu_domain_free(struct iommu_domain *domain)
{
	if (domain->type == IOMMU_DOMAIN_DMA)
		iommu_put_dma_cookie(domain);
	kfree(to_mtk_domain(domain));
}

static int mtk_iommu_attach_device(struct iommu_domain *domain,
				   struct device *dev)
{
	struct mtk_iommu_domain *dom = to_mtk_domain(domain);
	struct iommu_group *group;
	struct mtk_iommu_client_priv *priv = dev->archdata.iommu;
	struct mtk_iommu_data *m4udata;
	int ret;

	if (!priv)
		return -ENODEV;

	group = iommu_group_get(dev);
	if (!group) {
		/* __iommu_setup_dma_ops will create a domain for each a device
		 * defaultly, But MTK only has only one iommu domain called the
		 * m4u domain which all the multimedia HW share.
		 * We don't need its default domain here.
		 */
		iommu_domain_free(domain);
		return 0;
	}
	iommu_group_put(group);

	/*
	 * The iommu core will create a default domain for each a iommu group.
	 * This default domain is used for the m4u domain here.
	 */
	m4udata = dev_get_drvdata(priv->m4udev);
	if (!m4udata->m4udom) {
		m4udata->m4udom = dom;
		ret = iommu_dma_init_domain(domain, 0, DMA_BIT_MASK(32));

       <<<=== iommu_group_get_for_dev don't help initialize the domain,
I have to initialize it here.

		if (ret)
			return ret;
	}

	/*
	 * All the client devices are in the same group, they also are in the
	 * same domain. confirm it here.
	 */
	WARN_ON(dom != m4udata->m4udom);

	ret = mtk_iommu_init_domain_context(m4udata);
	if (ret)
		return ret;

	return mtk_iommu_config(m4udata, dev, true);
}

static void mtk_iommu_detach_device(struct iommu_domain *domain,
				    struct device *dev)
{
	struct mtk_iommu_client_priv *priv = dev->archdata.iommu;
	struct mtk_iommu_data *m4udata;
	struct iommu_group *group;

	if (!priv)
		return;

	group = iommu_group_get(dev);
	if (!group)
		return;
	iommu_group_put(group);

	m4udata = dev_get_drvdata(priv->m4udev);
	mtk_iommu_config(m4udata, dev, false);
}

static int mtk_iommu_map(struct iommu_domain *domain, unsigned long
iova,
			 phys_addr_t paddr, size_t size, int prot)
{
	struct mtk_iommu_domain *dom = to_mtk_domain(domain);
	unsigned long flags;
	int ret;

	spin_lock_irqsave(&dom->pgtlock, flags);
	ret = dom->iop->map(dom->iop, iova, paddr, size, prot);
	spin_unlock_irqrestore(&dom->pgtlock, flags);

	return ret;
}

static size_t mtk_iommu_unmap(struct iommu_domain *domain,
			      unsigned long iova, size_t size)
{
	struct mtk_iommu_domain *dom = to_mtk_domain(domain);
	unsigned long flags;
	size_t unmapsize;

	spin_lock_irqsave(&dom->pgtlock, flags);
	unmapsize = dom->iop->unmap(dom->iop, iova, size);
	spin_unlock_irqrestore(&dom->pgtlock, flags);

	return unmapsize;
}

static phys_addr_t mtk_iommu_iova_to_phys(struct iommu_domain *domain,
					  dma_addr_t iova)
{
	struct mtk_iommu_domain *dom = to_mtk_domain(domain);
	unsigned long flags;
	phys_addr_t pa;

	spin_lock_irqsave(&dom->pgtlock, flags);
	pa = dom->iop->iova_to_phys(dom->iop, iova);
	spin_unlock_irqrestore(&dom->pgtlock, flags);

	return pa;
}

static int mtk_iommu_add_device(struct device *dev)
{
	struct iommu_group *group;

	if (!dev->archdata.iommu) /* Not a iommu client device */
		return -ENODEV;

	group = iommu_group_get_for_dev(dev);
	if (IS_ERR(group))
		return PTR_ERR(group);

	iommu_group_put(group);
	return 0;
}

static void mtk_iommu_remove_device(struct device *dev)
{
	struct mtk_iommu_client_priv *head, *cur, *next;

	head = dev->archdata.iommu;
	if (!head)
		return;

	list_for_each_entry_safe(cur, next, &head->client, client) {
		list_del(&cur->client);
		kfree(cur);
	}
	kfree(head);
	dev->archdata.iommu = NULL;

	iommu_group_remove_device(dev);
}

static struct iommu_group *mtk_iommu_device_group(struct device *dev)
{
	struct mtk_iommu_data *m4udata;
	struct mtk_iommu_client_priv *priv;

	priv = dev->archdata.iommu;
	if (!priv)
		return ERR_PTR(-ENODEV);
	m4udata = dev_get_drvdata(priv->m4udev);

	/* All the client devices are in the same m4u iommu-group */
	if (!m4udata->m4ugroup) {
		m4udata->m4ugroup = iommu_group_alloc();
		if (IS_ERR(m4udata->m4ugroup))
			dev_err(dev, "Failed to allocate M4U IOMMU group\n");
	}
	return m4udata->m4ugroup;
}

static int mtk_iommu_of_xlate(struct device *dev, struct of_phandle_args
*args)
{
[...]
}

static struct iommu_ops mtk_iommu_ops = {
	.domain_alloc	= mtk_iommu_domain_alloc,
	.domain_free	= mtk_iommu_domain_free,
	.attach_dev	= mtk_iommu_attach_device,
	.detach_dev	= mtk_iommu_detach_device,
	.map		= mtk_iommu_map,
	.unmap		= mtk_iommu_unmap,
	.map_sg		= default_iommu_map_sg,
	.iova_to_phys	= mtk_iommu_iova_to_phys,
	.add_device	= mtk_iommu_add_device,
	.remove_device	= mtk_iommu_remove_device,
	.device_group	= mtk_iommu_device_group,
	.of_xlate	= mtk_iommu_of_xlate,
	.pgsize_bitmap	= SZ_4K | SZ_64K | SZ_1M | SZ_16M,
};

static const struct of_device_id mtk_iommu_of_ids[] = {
	{ .compatible = "mediatek,mt8173-m4u", },
	{}
};

static int mtk_iommu_init_fn(struct device_node *np)
{
	struct platform_device *pdev;

	pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
	if (IS_ERR(pdev))
		return PTR_ERR(pdev);

	of_iommu_set_ops(np, &mtk_iommu_ops);

	return 0;
}

IOMMU_OF_DECLARE(mtkm4u, "mediatek,mt8173-m4u", mtk_iommu_init_fn);

static int mtk_iommu_probe(struct platform_device *pdev)
{
	struct mtk_iommu_data   *data;
	struct device           *dev = &pdev->dev;
	void __iomem	        *protect;
	int                     ret;

	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
	if (!data)
		return -ENOMEM;
	data->dev = dev;

	/* Protect memory. HW will access here while translation fault.*/
	protect = devm_kzalloc(dev, MTK_PROTECT_PA_ALIGN * 2, GFP_KERNEL);
	if (!protect)
		return -ENOMEM;
	data->protect_base = ALIGN(virt_to_phys(protect),
MTK_PROTECT_PA_ALIGN);

	ret = mtk_iommu_parse_dt(pdev, data);
	if (ret)
		return ret;

	ret = mtk_iommu_hw_init(data);
	if (ret)
		return ret;

	dev_set_drvdata(dev, data);

	return 0;
}

static int mtk_iommu_remove(struct platform_device *pdev)
{
	struct mtk_iommu_data *data = dev_get_drvdata(&pdev->dev);

	free_io_pgtable_ops(data->m4udom->iop); /* Destroy domain context */
       <<<===== the free domain-context is also asymmetric too, but I
can not move it into detach_device, the detach_device may be called the
client device. the domain cann't be freed by a client device.

	clk_disable_unprepare(data->bclk);
	devm_free_irq(&pdev->dev, data->irq, data);
	return 0;
}

//===========================



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v5 4/6] memory: mediatek: Add SMI driver
  2015-10-27 13:24   ` Robin Murphy
@ 2015-10-31  8:32     ` Yong Wu
  0 siblings, 0 replies; 23+ messages in thread
From: Yong Wu @ 2015-10-31  8:32 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Joerg Roedel, Thierry Reding, Mark Rutland, Matthias Brugger,
	Will Deacon, Daniel Kurtz, Tomasz Figa, Lucas Stach, Rob Herring,
	Catalin Marinas, linux-mediatek, Sasha Hauer, srv_heupstream,
	devicetree, linux-kernel, linux-arm-kernel, iommu, pebolle, arnd,
	mitchelh, Sricharan R, youhua.li, k.zhang, kendrick.hsu

On Tue, 2015-10-27 at 13:24 +0000, Robin Murphy wrote:
> On 09/10/15 03:23, Yong Wu wrote:
> [...]
> > +static int mtk_smi_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct mtk_smi_data *smidata;
> > +	int ret;
> > +
> > +	if (!dev->pm_domain)
> > +		return -EPROBE_DEFER;
> > +
> > +	smidata = devm_kzalloc(dev, sizeof(*smidata), GFP_KERNEL);
> > +	if (!smidata)
> > +		return -ENOMEM;
> > +
> > +	smidata->clk_apb = devm_clk_get(dev, "apb");
> > +	if (IS_ERR(smidata->clk_apb))
> > +		return PTR_ERR(smidata->clk_apb);
> > +
> > +	smidata->clk_smi = devm_clk_get(dev, "smi");
> > +	if (IS_ERR(smidata->clk_smi))
> > +		return PTR_ERR(smidata->clk_smi);
> > +
> > +	pm_runtime_enable(dev);
> > +	dev_set_drvdata(dev, smidata);
> > +	return ret;
> 
> ret is used uninitialised here, but you might as well just "return 0;" 
> and get rid of the variable entirely.

Thanks. I will change to "return 0" directly.

> 
> > +}
> 
> Robin.
> 



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v5 3/6] iommu: add ARM short descriptor page table allocator
  2015-10-09  2:23 ` [PATCH v5 3/6] iommu: add ARM short descriptor page table allocator Yong Wu
  2015-10-14 12:54   ` Joerg Roedel
@ 2015-11-06  8:42   ` Yong Wu
  2015-11-06 11:42     ` Will Deacon
  1 sibling, 1 reply; 23+ messages in thread
From: Yong Wu @ 2015-11-06  8:42 UTC (permalink / raw)
  To: Will Deacon, Robin Murphy, Catalin Marinas
  Cc: Joerg Roedel, Thierry Reding, Mark Rutland, Matthias Brugger,
	Daniel Kurtz, Tomasz Figa, Lucas Stach, Rob Herring,
	linux-mediatek, Sasha Hauer, srv_heupstream, devicetree,
	linux-kernel, linux-arm-kernel, iommu, pebolle, arnd, mitchelh,
	Sricharan R, youhua.li, k.zhang, kendrick.hsu

On Fri, 2015-10-09 at 10:23 +0800, Yong Wu wrote:
> This patch is for ARM Short Descriptor Format.
> 
> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> ---

Hi Will, Robin,
   Is there any comment about this patch?
   As our project request, We are going to prepare the next version.
Thanks very much.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v5 3/6] iommu: add ARM short descriptor page table allocator
  2015-11-06  8:42   ` Yong Wu
@ 2015-11-06 11:42     ` Will Deacon
  0 siblings, 0 replies; 23+ messages in thread
From: Will Deacon @ 2015-11-06 11:42 UTC (permalink / raw)
  To: Yong Wu
  Cc: Robin Murphy, Catalin Marinas, Joerg Roedel, Thierry Reding,
	Mark Rutland, Matthias Brugger, Daniel Kurtz, Tomasz Figa,
	Lucas Stach, Rob Herring, linux-mediatek, Sasha Hauer,
	srv_heupstream, devicetree, linux-kernel, linux-arm-kernel,
	iommu, pebolle, arnd, mitchelh, Sricharan R, youhua.li, k.zhang,
	kendrick.hsu

On Fri, Nov 06, 2015 at 04:42:52PM +0800, Yong Wu wrote:
> On Fri, 2015-10-09 at 10:23 +0800, Yong Wu wrote:
> > This patch is for ARM Short Descriptor Format.
> > 
> > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > ---
> 
> Hi Will, Robin,
>    Is there any comment about this patch?
>    As our project request, We are going to prepare the next version.

Robin has been looking at the code after he ran into problems using it
in conjunction with arm-smmu. I'd expect him to post something after the
merge window, so you may as well hold-off until then.

Will

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v5 0/6] MT8173 IOMMU SUPPORT
  2015-10-23  9:26     ` Joerg Roedel
@ 2015-11-24  5:58       ` Yong Wu
  2015-11-24 10:38         ` Thierry Reding
  0 siblings, 1 reply; 23+ messages in thread
From: Yong Wu @ 2015-11-24  5:58 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: Matthias Brugger, arnd, Thierry Reding, Mark Rutland,
	Robin Murphy, Will Deacon, Daniel Kurtz, Tomasz Figa,
	Lucas Stach, Rob Herring, Catalin Marinas, linux-mediatek,
	Sasha Hauer, srv_heupstream, devicetree, linux-kernel,
	linux-arm-kernel, iommu, pebolle, mitchelh, Sricharan R,
	Daniel Thompson

On Fri, 2015-10-23 at 11:26 +0200, Joerg Roedel wrote:
> On Thu, Oct 22, 2015 at 12:40:02PM +0800, Yong Wu wrote:
> >      But the mtk-iommu depend on the drivers/memory/mtk-smi.c(mtk-iommu
> > has called a function of the mtk-smi).
> >      So if there is dependence here, How should we do to merge them?
> 
> I can surely merge mtk-smi too, if it gets proper Reviewed-by and
> Acked-by tags from the maintainer(s).
> 
> 
> 	Joerg
> 
Hi Joerg,

   About the driver/memory/, We don't know who is his maintainer.
MAINTAINERS file don't have drivers/memory maintainer.
>From the history in drivers/memory/ it looks like most of the
drivers land with an ack from the architecture maintainer.
And Matthias Brugger is our "ARM/Mediatek SoC support" maintainer.

Then do you mean that we need Matthias's ACK or whom others?

By the way, I will send the next version after Robin reposting
Short-descriptor. maybe in next week. 



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v5 0/6] MT8173 IOMMU SUPPORT
  2015-11-24  5:58       ` Yong Wu
@ 2015-11-24 10:38         ` Thierry Reding
  2015-11-25  1:30           ` Yong Wu
  0 siblings, 1 reply; 23+ messages in thread
From: Thierry Reding @ 2015-11-24 10:38 UTC (permalink / raw)
  To: Yong Wu
  Cc: Joerg Roedel, Matthias Brugger, arnd, Mark Rutland, Robin Murphy,
	Will Deacon, Daniel Kurtz, Tomasz Figa, Lucas Stach, Rob Herring,
	Catalin Marinas, linux-mediatek, Sasha Hauer, srv_heupstream,
	devicetree, linux-kernel, linux-arm-kernel, iommu, pebolle,
	mitchelh, Sricharan R, Daniel Thompson

[-- Attachment #1: Type: text/plain, Size: 1138 bytes --]

On Tue, Nov 24, 2015 at 01:58:13PM +0800, Yong Wu wrote:
> On Fri, 2015-10-23 at 11:26 +0200, Joerg Roedel wrote:
> > On Thu, Oct 22, 2015 at 12:40:02PM +0800, Yong Wu wrote:
> > >      But the mtk-iommu depend on the drivers/memory/mtk-smi.c(mtk-iommu
> > > has called a function of the mtk-smi).
> > >      So if there is dependence here, How should we do to merge them?
> > 
> > I can surely merge mtk-smi too, if it gets proper Reviewed-by and
> > Acked-by tags from the maintainer(s).
> > 
> > 
> > 	Joerg
> > 
> Hi Joerg,
> 
>    About the driver/memory/, We don't know who is his maintainer.
> MAINTAINERS file don't have drivers/memory maintainer.
> From the history in drivers/memory/ it looks like most of the
> drivers land with an ack from the architecture maintainer.
> And Matthias Brugger is our "ARM/Mediatek SoC support" maintainer.
> 
> Then do you mean that we need Matthias's ACK or whom others?

Yes, I think the sub-architecture maintainer's ACK is probably going to
be as good as it gets. Historically drivers/memory hasn't had enough of
a common ground to instate a framework.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v5 0/6] MT8173 IOMMU SUPPORT
  2015-11-24 10:38         ` Thierry Reding
@ 2015-11-25  1:30           ` Yong Wu
  0 siblings, 0 replies; 23+ messages in thread
From: Yong Wu @ 2015-11-25  1:30 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Mark Rutland, Catalin Marinas, Will Deacon, Daniel Thompson,
	Joerg Roedel, devicetree, arnd, Robin Murphy, Tomasz Figa,
	Rob Herring, linux-mediatek, Matthias Brugger, linux-arm-kernel,
	pebolle, srv_heupstream, Sricharan R, linux-kernel, iommu,
	Daniel Kurtz, Sasha Hauer, mitchelh, Lucas Stach

On Tue, 2015-11-24 at 11:38 +0100, Thierry Reding wrote:
> On Tue, Nov 24, 2015 at 01:58:13PM +0800, Yong Wu wrote:
> > On Fri, 2015-10-23 at 11:26 +0200, Joerg Roedel wrote:
> > > On Thu, Oct 22, 2015 at 12:40:02PM +0800, Yong Wu wrote:
> > > >      But the mtk-iommu depend on the drivers/memory/mtk-smi.c(mtk-iommu
> > > > has called a function of the mtk-smi).
> > > >      So if there is dependence here, How should we do to merge them?
> > > 
> > > I can surely merge mtk-smi too, if it gets proper Reviewed-by and
> > > Acked-by tags from the maintainer(s).
> > > 
> > > 
> > > 	Joerg
> > > 
> > Hi Joerg,
> > 
> >    About the driver/memory/, We don't know who is his maintainer.
> > MAINTAINERS file don't have drivers/memory maintainer.
> > From the history in drivers/memory/ it looks like most of the
> > drivers land with an ack from the architecture maintainer.
> > And Matthias Brugger is our "ARM/Mediatek SoC support" maintainer.
> > 
> > Then do you mean that we need Matthias's ACK or whom others?
> 
> Yes, I think the sub-architecture maintainer's ACK is probably going to
> be as good as it gets. Historically drivers/memory hasn't had enough of
> a common ground to instate a framework.
> 
> Thierry

Got it.
Thanks very much for your reply.

> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek



^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2015-11-25  1:31 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-09  2:23 [PATCH v5 0/6] MT8173 IOMMU SUPPORT Yong Wu
2015-10-09  2:23 ` [PATCH v5 1/6] dt-bindings: iommu: Add binding for mediatek IOMMU Yong Wu
2015-10-09  2:23 ` [PATCH v5 2/6] dt-bindings: mediatek: Add smi dts binding Yong Wu
2015-10-09  2:23 ` [PATCH v5 3/6] iommu: add ARM short descriptor page table allocator Yong Wu
2015-10-14 12:54   ` Joerg Roedel
2015-10-15 17:20     ` Will Deacon
2015-11-06  8:42   ` Yong Wu
2015-11-06 11:42     ` Will Deacon
2015-10-09  2:23 ` [PATCH v5 4/6] memory: mediatek: Add SMI driver Yong Wu
2015-10-27 13:24   ` Robin Murphy
2015-10-31  8:32     ` Yong Wu
2015-10-09  2:23 ` [PATCH v5 5/6] iommu/mediatek: Add mt8173 IOMMU driver Yong Wu
2015-10-14 12:53   ` Joerg Roedel
2015-10-26  5:23     ` Yong Wu
2015-10-27 13:25   ` Robin Murphy
2015-10-31  8:28     ` Yong Wu
2015-10-09  2:23 ` [PATCH v5 6/6] dts: mt8173: Add iommu/smi nodes for mt8173 Yong Wu
2015-10-14 12:56 ` [PATCH v5 0/6] MT8173 IOMMU SUPPORT Joerg Roedel
2015-10-22  4:40   ` Yong Wu
2015-10-23  9:26     ` Joerg Roedel
2015-11-24  5:58       ` Yong Wu
2015-11-24 10:38         ` Thierry Reding
2015-11-25  1:30           ` Yong Wu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).