[PATCH 0/8] MT2712 IOMMU SUPPORT

[PATCH 0/8] MT2712 IOMMU SUPPORT

[Yong Wu]

This patchset mainly adds support for M4U of mt2712.

The M4U in mt2712 is MTK's generation2 M4U which use the
Short-descriptor like mt8173. The main difference is that there are 2
M4Us and 2 smi-commons in mt2712, while there is only 1 M4U and 1
smi-common in mt8173. The purpose is for balance the bandwidth.

The mt2712 M4U-SMI HW diagram is as below:

                            EMI
                             |
              -------------------------------
              |                             |
             M4U0                          M4U1
              |                             |
         smi-common0                   smi-common1
              |                             |
  -------------------------       --------------------
  |     |     |     |     |       |         |        |
  |     |     |     |     |       |         |        |
larb0 larb1 larb2 larb3 larb6    larb4    larb5     larb7
disp0 vdec  cam   venc   jpg  mdp1/disp1 mdp2/disp2  mdp3->larb names

This patchset is based on v4.13-rc1, Also it base on Robin's[1],
Honghui[2],Arvind[3]. Currently it don't contain the dtsi part
as the ccf/power-domain has not been ready.

The patch 1/2 adds the support of MT2712 IOMMU support,
the patch 3/4 improve the m4u flow for mt2712,
the last patch 5/6/7/8 mainly fix bug or improve code.

[1]:https://patchwork.kernel.org/patch/9828671/
[2]:https://patchwork.kernel.org/patch/9880223/
[3]:https://patchwork.kernel.org/patch/9892759/

Yong Wu (8):
  dt-bindings: mediatek: Add binding for mt2712 IOMMU and SMI
  iommu/mediatek: Add mt2712 IOMMU support
  iommu/mediatek: Merge 2 M4U HWs into one iommu domain
  iommu/mediatek: Move pgtable allocation into domain_alloc
  iommu/mediatek: Disable iommu clock when system suspend
  iommu/mediatek: Enlarge the validate PA range for 4GB mode
  memory: mtk-smi: Rearrange some function position alphabetically
  memory: mtk-smi: Degrade SMI init to module_init

 .../devicetree/bindings/iommu/mediatek,iommu.txt   |   6 +-
 .../memory-controllers/mediatek,smi-common.txt     |   6 +-
 .../memory-controllers/mediatek,smi-larb.txt       |   5 +-
 drivers/iommu/mtk_iommu.c                          | 184 ++++++++++++++-------
 drivers/iommu/mtk_iommu.h                          |   9 +
 drivers/memory/mtk-smi.c                           | 108 +++++++-----
 include/dt-bindings/memory/mt2712-larb-port.h      |  91 ++++++++++
 7 files changed, 305 insertions(+), 104 deletions(-)
 create mode 100644 include/dt-bindings/memory/mt2712-larb-port.h

-- 
1.9.1

[PATCH 3/8] iommu/mediatek: Merge 2 M4U HWs into one iommu domain

[Yong Wu]

In theory, If there are 2 M4U HWs, there should be 2 IOMMU domains.
But one IOMMU domain(4GB iova range) is enough for us currently,
It's unnecessary to maintain 2 pagetables.

Besides, This patch can simplify our consumer code largely. They don't
need map a iova range from one domain into another, They can share the
iova address easily.

Signed-off-by: Yong Wu <yong.wu@mediatek.com>
---
 drivers/iommu/mtk_iommu.c | 98 ++++++++++++++++++++++++++++++++++-------------
 drivers/iommu/mtk_iommu.h |  2 +
 2 files changed, 73 insertions(+), 27 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index da6cedb..e3848e1 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -106,6 +106,27 @@ struct mtk_iommu_domain {
 
 static struct iommu_ops mtk_iommu_ops;
 
+static LIST_HEAD(m4ulist); /* List all the M4U HWs */
+
+#define for_each_m4u(data) list_for_each_entry(data, &m4ulist, list)
+
+/*
+ * There may be 1 or 2 M4U HWs, But we always expect they are in the same domain
+ * for the performance.
+ *
+ * Here always return the mtk_iommu_data of the first probed M4U where the
+ * iommu domain information is recorded.
+ */
+static struct mtk_iommu_data *mtk_iommu_get_m4u_data(void)
+{
+	struct mtk_iommu_data *data;
+
+	for_each_m4u(data)
+		return data;
+
+	return NULL;
+}
+
 static struct mtk_iommu_domain *to_mtk_domain(struct iommu_domain *dom)
 {
 	return container_of(dom, struct mtk_iommu_domain, domain);
@@ -113,47 +134,57 @@ static struct mtk_iommu_domain *to_mtk_domain(struct iommu_domain *dom)
 
 static void mtk_iommu_tlb_flush_all(void *cookie)
 {
-	struct mtk_iommu_data *data = cookie;
+	struct mtk_iommu_data *data;
 
-	writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0, data->base + REG_MMU_INV_SEL);
-	writel_relaxed(F_ALL_INVLD, data->base + REG_MMU_INVALIDATE);
-	wmb(); /* Make sure the tlb flush all done */
+	for_each_m4u(data) {
+		writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
+			       data->base + REG_MMU_INV_SEL);
+		writel_relaxed(F_ALL_INVLD, data->base + REG_MMU_INVALIDATE);
+		wmb(); /* Make sure the tlb flush all done */
+	}
 }
 
 static void mtk_iommu_tlb_add_flush_nosync(unsigned long iova, size_t size,
 					   size_t granule, bool leaf,
 					   void *cookie)
 {
-	struct mtk_iommu_data *data = cookie;
+	struct mtk_iommu_data *data;
 
-	writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0, data->base + REG_MMU_INV_SEL);
+	for_each_m4u(data) {
+		writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
+			       data->base + REG_MMU_INV_SEL);
 
-	writel_relaxed(iova, data->base + REG_MMU_INVLD_START_A);
-	writel_relaxed(iova + size - 1, data->base + REG_MMU_INVLD_END_A);
-	writel_relaxed(F_MMU_INV_RANGE, data->base + REG_MMU_INVALIDATE);
-	data->tlb_flush_active = true;
+		writel_relaxed(iova, data->base + REG_MMU_INVLD_START_A);
+		writel_relaxed(iova + size - 1,
+			       data->base + REG_MMU_INVLD_END_A);
+		writel_relaxed(F_MMU_INV_RANGE,
+			       data->base + REG_MMU_INVALIDATE);
+		data->tlb_flush_active = true;
+	}
 }
 
 static void mtk_iommu_tlb_sync(void *cookie)
 {
-	struct mtk_iommu_data *data = cookie;
+	struct mtk_iommu_data *data;
 	int ret;
 	u32 tmp;
 
-	/* Avoid timing out if there's nothing to wait for */
-	if (!data->tlb_flush_active)
-		return;
+	for_each_m4u(data) {
+		/* Avoid timing out if there's nothing to wait for */
+		if (!data->tlb_flush_active)
+			return;
 
-	ret = readl_poll_timeout_atomic(data->base + REG_MMU_CPE_DONE, tmp,
-					tmp != 0, 10, 100000);
-	if (ret) {
-		dev_warn(data->dev,
-			 "Partial TLB flush timed out, falling back to full flush\n");
-		mtk_iommu_tlb_flush_all(cookie);
+		ret = readl_poll_timeout_atomic(data->base + REG_MMU_CPE_DONE,
+						tmp, tmp != 0, 10, 100000);
+		if (ret) {
+			dev_warn(data->dev,
+				 "Partial TLB flush timed out, falling back to full flush\n");
+			mtk_iommu_tlb_flush_all(cookie);
+		}
+		/* Clear the CPE status */
+		writel_relaxed(0, data->base + REG_MMU_CPE_DONE);
+		data->tlb_flush_active = false;
 	}
-	/* Clear the CPE status */
-	writel_relaxed(0, data->base + REG_MMU_CPE_DONE);
-	data->tlb_flush_active = false;
 }
 
 static const struct iommu_gather_ops mtk_iommu_gather_ops = {
@@ -290,10 +321,11 @@ static int mtk_iommu_attach_device(struct iommu_domain *domain,
 				   struct device *dev)
 {
 	struct mtk_iommu_domain *dom = to_mtk_domain(domain);
-	struct mtk_iommu_data *data = dev->iommu_fwspec->iommu_priv;
+	struct mtk_iommu_data *curdata = dev->iommu_fwspec->iommu_priv;
+	struct mtk_iommu_data *data = mtk_iommu_get_m4u_data();
 	int ret;
 
-	if (!data)
+	if (!data || !curdata)
 		return -ENODEV;
 
 	if (!data->m4u_dom) {
@@ -305,7 +337,17 @@ static int mtk_iommu_attach_device(struct iommu_domain *domain,
 		}
 	}
 
-	mtk_iommu_config(data, dev, true);
+	/*
+	 * Update the pgtable base address register of another M4U HW with the
+	 * existed pgtable if there are more than one M4U HW.
+	 */
+	if (!curdata->m4u_dom) {
+		curdata->m4u_dom = data->m4u_dom;
+		writel(data->m4u_dom->cfg.arm_v7s_cfg.ttbr[0],
+		       curdata->base + REG_MMU_PT_BASE_ADDR);
+	}
+
+	mtk_iommu_config(curdata, dev, true);
 	return 0;
 }
 
@@ -397,7 +439,7 @@ static void mtk_iommu_remove_device(struct device *dev)
 
 static struct iommu_group *mtk_iommu_device_group(struct device *dev)
 {
-	struct mtk_iommu_data *data = dev->iommu_fwspec->iommu_priv;
+	struct mtk_iommu_data *data = mtk_iommu_get_m4u_data();
 
 	if (!data)
 		return ERR_PTR(-ENODEV);
@@ -606,6 +648,8 @@ static int mtk_iommu_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	list_add_tail(&data->list, &m4ulist);
+
 	if (!iommu_present(&platform_bus_type))
 		bus_set_iommu(&platform_bus_type, &mtk_iommu_ops);
 
diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
index cd729a3..7d9ec49 100644
--- a/drivers/iommu/mtk_iommu.h
+++ b/drivers/iommu/mtk_iommu.h
@@ -57,6 +57,8 @@ struct mtk_iommu_data {
 
 	struct iommu_device		iommu;
 	enum mtk_iommu_type		m4u_type;
+
+	struct list_head		list;
 };
 
 static inline int compare_of(struct device *dev, void *data)
-- 
1.9.1

[PATCH 1/8] dt-bindings: mediatek: Add binding for mt2712 IOMMU and SMI

[Yong Wu]

This patch adds decriptions for mt2712 IOMMU and SMI.

In order to balance the bandwidth, mt2712 has two M4Us, two
smi-commons, 8 smi-larbs. and mt2712 is also MTK IOMMU gen2 which
use Short-Descriptor translation table format.

The mt2712 M4U-SMI HW diagram is as below:

                            EMI
                             |
              -------------------------------
              |                             |
             M4U0                          M4U1
              |                             |
         smi-common0                   smi-common1
              |                             |
  -------------------------       --------------------
  |     |     |     |     |       |         |        |
  |     |     |     |     |       |         |        |
larb0 larb1 larb2 larb3 larb6    larb4    larb5     larb7
disp0 vdec  cam   venc   jpg  mdp1/disp1 mdp2/disp2  mdp3->larb names

All the connections are HW fixed, SW can NOT adjust it.

Signed-off-by: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 .../devicetree/bindings/iommu/mediatek,iommu.txt   |  6 +-
 .../memory-controllers/mediatek,smi-common.txt     |  6 +-
 .../memory-controllers/mediatek,smi-larb.txt       |  5 +-
 include/dt-bindings/memory/mt2712-larb-port.h      | 91 ++++++++++++++++++++++
 4 files changed, 102 insertions(+), 6 deletions(-)
 create mode 100644 include/dt-bindings/memory/mt2712-larb-port.h

diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt b/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
index 53c20ca..df5db73 100644
--- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
+++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
@@ -40,6 +40,7 @@ video decode local arbiter, all these ports are according to the video HW.
 Required properties:
 - compatible : must be one of the following string:
 	"mediatek,mt2701-m4u" for mt2701 which uses generation one m4u HW.
+	"mediatek,mt2712-m4u" for mt2712 which uses generation two m4u HW.
 	"mediatek,mt8173-m4u" for mt8173 which uses generation two m4u HW.
 - reg : m4u register base and size.
 - interrupts : the interrupt of m4u.
@@ -50,8 +51,9 @@ Required properties:
 	according to the local arbiter index, like larb0, larb1, larb2...
 - iommu-cells : must be 1. This is the mtk_m4u_id according to the HW.
 	Specifies the mtk_m4u_id as defined in
-	dt-binding/memory/mt2701-larb-port.h for mt2701 and
-	dt-binding/memory/mt8173-larb-port.h for mt8173
+	dt-binding/memory/mt2701-larb-port.h for mt2701,
+	dt-binding/memory/mt2712-larb-port.h for mt2712, and
+	dt-binding/memory/mt8173-larb-port.h for mt8173.
 
 Example:
 	iommu: iommu@10205000 {
diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
index aa614b2..615abdd 100644
--- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
+++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
@@ -2,8 +2,9 @@ SMI (Smart Multimedia Interface) Common
 
 The hardware block diagram please check bindings/iommu/mediatek,iommu.txt
 
-Mediatek SMI have two generations of HW architecture, mt8173 uses the second
-generation of SMI HW while mt2701 uses the first generation HW of SMI.
+Mediatek SMI have two generations of HW architecture, mt2712 and mt8173 use
+the second generation of SMI HW while mt2701 uses the first generation HW of
+SMI.
 
 There's slight differences between the two SMI, for generation 2, the
 register which control the iommu port is at each larb's register base. But
@@ -15,6 +16,7 @@ not needed for SMI generation 2.
 Required properties:
 - compatible : must be one of :
 	"mediatek,mt2701-smi-common"
+	"mediatek,mt2712-smi-common"
 	"mediatek,mt8173-smi-common"
 - reg : the register and size of the SMI block.
 - power-domains : a phandle to the power domain of this local arbiter.
diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
index ddf46b8..083155c 100644
--- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
+++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
@@ -4,8 +4,9 @@ The hardware block diagram please check bindings/iommu/mediatek,iommu.txt
 
 Required properties:
 - compatible : must be one of :
-		"mediatek,mt8173-smi-larb"
 		"mediatek,mt2701-smi-larb"
+		"mediatek,mt2712-smi-larb"
+		"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.
@@ -15,7 +16,7 @@ Required properties:
 	    the register.
   - "smi" : It's the clock for transfer data and command.
 
-Required property for mt2701:
+Required property for mt2701 and mt2712:
 - mediatek,larb-id :the hardware id of this larb.
 
 Example:
diff --git a/include/dt-bindings/memory/mt2712-larb-port.h b/include/dt-bindings/memory/mt2712-larb-port.h
new file mode 100644
index 0000000..4d74536
--- /dev/null
+++ b/include/dt-bindings/memory/mt2712-larb-port.h
@@ -0,0 +1,91 @@
+/*
+ * Copyright (c) 2017 MediaTek Inc.
+ * Author: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
+ *
+ * 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_MT2712_H
+#define __DTS_IOMMU_PORT_MT2712_H
+
+#define MTK_M4U_ID(larb, port)		(((larb) << 5) | (port))
+
+#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
+#define M4U_LARB6_ID			6
+#define M4U_LARB7_ID			7
+
+/* larb0 */
+#define M4U_PORT_DISP_OVL0		MTK_M4U_ID(M4U_LARB0_ID, 0)
+#define M4U_PORT_DISP_RDMA0		MTK_M4U_ID(M4U_LARB0_ID, 1)
+#define M4U_PORT_DISP_WDMA0		MTK_M4U_ID(M4U_LARB0_ID, 2)
+#define M4U_PORT_DISP_OD_R		MTK_M4U_ID(M4U_LARB0_ID, 3)
+#define M4U_PORT_DISP_OD_W		MTK_M4U_ID(M4U_LARB0_ID, 4)
+#define M4U_PORT_MDP_RDMA0		MTK_M4U_ID(M4U_LARB0_ID, 5)
+#define M4U_PORT_MDP_WDMA		MTK_M4U_ID(M4U_LARB0_ID, 6)
+
+/* larb1 */
+#define M4U_PORT_HW_VDEC_MC_EXT		MTK_M4U_ID(M4U_LARB1_ID, 0)
+#define M4U_PORT_HW_VDEC_PP_EXT		MTK_M4U_ID(M4U_LARB1_ID, 1)
+#define M4U_PORT_HW_VDEC_UFO_EXT	MTK_M4U_ID(M4U_LARB1_ID, 2)
+#define M4U_PORT_HW_VDEC_VLD_EXT	MTK_M4U_ID(M4U_LARB1_ID, 3)
+#define M4U_PORT_HW_VDEC_VLD2_EXT	MTK_M4U_ID(M4U_LARB1_ID, 4)
+#define M4U_PORT_HW_VDEC_AVC_MV_EXT	MTK_M4U_ID(M4U_LARB1_ID, 5)
+#define M4U_PORT_HW_VDEC_PRED_RD_EXT	MTK_M4U_ID(M4U_LARB1_ID, 6)
+#define M4U_PORT_HW_VDEC_PRED_WR_EXT	MTK_M4U_ID(M4U_LARB1_ID, 7)
+#define M4U_PORT_HW_VDEC_PPWRAP_EXT	MTK_M4U_ID(M4U_LARB1_ID, 8)
+#define M4U_PORT_HW_VDEC_TILE		MTK_M4U_ID(M4U_LARB1_ID, 9)
+#define M4U_PORT_HW_IMG_RESZ_EXT	MTK_M4U_ID(M4U_LARB1_ID, 10)
+
+/* larb2 */
+#define M4U_PORT_CAM_DMA0		MTK_M4U_ID(M4U_LARB2_ID, 0)
+#define M4U_PORT_CAM_DMA1		MTK_M4U_ID(M4U_LARB2_ID, 1)
+#define M4U_PORT_CAM_DMA2		MTK_M4U_ID(M4U_LARB2_ID, 2)
+
+/* larb3 */
+#define M4U_PORT_VENC_RCPU		MTK_M4U_ID(M4U_LARB3_ID, 0)
+#define M4U_PORT_VENC_REC		MTK_M4U_ID(M4U_LARB3_ID, 1)
+#define M4U_PORT_VENC_BSDMA		MTK_M4U_ID(M4U_LARB3_ID, 2)
+#define M4U_PORT_VENC_SV_COMV		MTK_M4U_ID(M4U_LARB3_ID, 3)
+#define M4U_PORT_VENC_RD_COMV		MTK_M4U_ID(M4U_LARB3_ID, 4)
+#define M4U_PORT_VENC_CUR_CHROMA	MTK_M4U_ID(M4U_LARB3_ID, 5)
+#define M4U_PORT_VENC_REF_CHROMA	MTK_M4U_ID(M4U_LARB3_ID, 6)
+#define M4U_PORT_VENC_CUR_LUMA		MTK_M4U_ID(M4U_LARB3_ID, 7)
+#define M4U_PORT_VENC_REF_LUMA		MTK_M4U_ID(M4U_LARB3_ID, 8)
+
+/* larb4 */
+#define M4U_PORT_DISP_OVL1		MTK_M4U_ID(M4U_LARB4_ID, 0)
+#define M4U_PORT_DISP_RDMA1		MTK_M4U_ID(M4U_LARB4_ID, 1)
+#define M4U_PORT_DISP_WDMA1		MTK_M4U_ID(M4U_LARB4_ID, 2)
+#define M4U_PORT_DISP_OD1_R		MTK_M4U_ID(M4U_LARB4_ID, 3)
+#define M4U_PORT_DISP_OD1_W		MTK_M4U_ID(M4U_LARB4_ID, 4)
+#define M4U_PORT_MDP_RDMA1		MTK_M4U_ID(M4U_LARB4_ID, 5)
+#define M4U_PORT_MDP_WROT1		MTK_M4U_ID(M4U_LARB4_ID, 6)
+
+/* larb5 */
+#define M4U_PORT_DISP_OVL2		MTK_M4U_ID(M4U_LARB5_ID, 0)
+#define M4U_PORT_DISP_WDMA2		MTK_M4U_ID(M4U_LARB5_ID, 1)
+#define M4U_PORT_MDP_RDMA2		MTK_M4U_ID(M4U_LARB5_ID, 2)
+#define M4U_PORT_MDP_WROT0		MTK_M4U_ID(M4U_LARB5_ID, 3)
+
+/* larb6 */
+#define M4U_PORT_JPGDEC_WDMA_0		MTK_M4U_ID(M4U_LARB6_ID, 0)
+#define M4U_PORT_JPGDEC_WDMA_1		MTK_M4U_ID(M4U_LARB6_ID, 1)
+#define M4U_PORT_JPGDEC_BSDMA_0		MTK_M4U_ID(M4U_LARB6_ID, 2)
+#define M4U_PORT_JPGDEC_BSDMA_1		MTK_M4U_ID(M4U_LARB6_ID, 3)
+
+/* larb7 */
+#define M4U_PORT_MDP_RDMA3		MTK_M4U_ID(M4U_LARB7_ID, 0)
+#define M4U_PORT_MDP_WROT2		MTK_M4U_ID(M4U_LARB7_ID, 1)
+
+#endif
-- 
1.9.1

Re: [PATCH 1/8] dt-bindings: mediatek: Add binding for mt2712 IOMMU and SMI

[Rob Herring]

On Fri, Aug 11, 2017 at 05:56:10PM +0800, Yong Wu wrote:
> This patch adds decriptions for mt2712 IOMMU and SMI.
> 
> In order to balance the bandwidth, mt2712 has two M4Us, two
> smi-commons, 8 smi-larbs. and mt2712 is also MTK IOMMU gen2 which
> use Short-Descriptor translation table format.

s/use/uses/

> 
> The mt2712 M4U-SMI HW diagram is as below:
> 
>                             EMI
>                              |
>               -------------------------------
>               |                             |
>              M4U0                          M4U1
>               |                             |
>          smi-common0                   smi-common1
>               |                             |
>   -------------------------       --------------------
>   |     |     |     |     |       |         |        |
>   |     |     |     |     |       |         |        |
> larb0 larb1 larb2 larb3 larb6    larb4    larb5     larb7
> disp0 vdec  cam   venc   jpg  mdp1/disp1 mdp2/disp2  mdp3->larb names
> 
> All the connections are HW fixed, SW can NOT adjust it.
> 
> Signed-off-by: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
>  .../devicetree/bindings/iommu/mediatek,iommu.txt   |  6 +-
>  .../memory-controllers/mediatek,smi-common.txt     |  6 +-
>  .../memory-controllers/mediatek,smi-larb.txt       |  5 +-
>  include/dt-bindings/memory/mt2712-larb-port.h      | 91 ++++++++++++++++++++++
>  4 files changed, 102 insertions(+), 6 deletions(-)
>  create mode 100644 include/dt-bindings/memory/mt2712-larb-port.h

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

[PATCH 2/8] iommu/mediatek: Add mt2712 IOMMU support

[Yong Wu]

The M4U IP blocks in mt2712 is MTK's generation2 M4U which use the
Short-descriptor like mt8173, and most of the HW registers are the
same.

The difference is that there are 2 M4U HWs in mt2712 while there's
only one in mt8173. The purpose of 2 M4U HWs is for balance the
bandwidth.

Normally if there are 2 M4U HWs, there should be 2 iommu domains,
each M4U has a iommu domain.

Signed-off-by: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
This patch also include a minor issue:
suspend while there is no iommu client. it will hang because
there is no iommu domain at that time.
---
 drivers/iommu/mtk_iommu.c | 48 ++++++++++++++++++++++++++++++++---------------
 drivers/iommu/mtk_iommu.h |  7 +++++++
 drivers/memory/mtk-smi.c  | 40 ++++++++++++++++++++++++++++++++++++---
 3 files changed, 77 insertions(+), 18 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 91c6d36..da6cedb 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -54,7 +54,9 @@
 
 #define REG_MMU_CTRL_REG			0x110
 #define F_MMU_PREFETCH_RT_REPLACE_MOD		BIT(4)
+/* The TF-protect-select is bit[5:4] in mt2712 while it's bit[6:5] in mt8173.*/
 #define F_MMU_TF_PROTECT_SEL(prot)		(((prot) & 0x3) << 5)
+#define F_MMU_TF_PROT_SEL(prot)			(((prot) & 0x3) << 4)
 
 #define REG_MMU_IVRP_PADDR			0x114
 #define F_MMU_IVRP_PA_SET(pa, ext)		(((pa) >> 1) | ((!!(ext)) << 31))
@@ -301,10 +303,6 @@ static int mtk_iommu_attach_device(struct iommu_domain *domain,
 			data->m4u_dom = NULL;
 			return ret;
 		}
-	} else if (data->m4u_dom != dom) {
-		/* All the client devices should be in the same m4u domain */
-		dev_err(dev, "try to attach into the error iommu domain\n");
-		return -EPERM;
 	}
 
 	mtk_iommu_config(data, dev, true);
@@ -464,8 +462,12 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
 		return ret;
 	}
 
-	regval = F_MMU_PREFETCH_RT_REPLACE_MOD |
-		F_MMU_TF_PROTECT_SEL(2);
+	if (data->m4u_type == M4U_MT8173) {
+		regval = F_MMU_PREFETCH_RT_REPLACE_MOD |
+			F_MMU_TF_PROTECT_SEL(2);
+	} else {
+		regval = F_MMU_TF_PROT_SEL(2);
+	}
 	writel_relaxed(regval, data->base + REG_MMU_CTRL_REG);
 
 	regval = F_L2_MULIT_HIT_EN |
@@ -487,9 +489,11 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
 
 	writel_relaxed(F_MMU_IVRP_PA_SET(data->protect_base, data->enable_4GB),
 		       data->base + REG_MMU_IVRP_PADDR);
-
 	writel_relaxed(0, data->base + REG_MMU_DCM_DIS);
-	writel_relaxed(0, data->base + REG_MMU_STANDARD_AXI_MODE);
+
+	/* It's MISC control register whose default value is ok except mt8173.*/
+	if (data->m4u_type == M4U_MT8173)
+		writel_relaxed(0, data->base + REG_MMU_STANDARD_AXI_MODE);
 
 	if (devm_request_irq(data->dev, data->irq, mtk_iommu_isr, 0,
 			     dev_name(data->dev), (void *)data)) {
@@ -521,6 +525,7 @@ static int mtk_iommu_probe(struct platform_device *pdev)
 	if (!data)
 		return -ENOMEM;
 	data->dev = dev;
+	data->m4u_type = (enum mtk_iommu_type)of_device_get_match_data(dev);
 
 	/* Protect memory. HW will access here while translation fault.*/
 	protect = devm_kzalloc(dev, MTK_PROTECT_PA_ALIGN * 2, GFP_KERNEL);
@@ -554,6 +559,7 @@ static int mtk_iommu_probe(struct platform_device *pdev)
 	for (i = 0; i < larb_nr; i++) {
 		struct device_node *larbnode;
 		struct platform_device *plarbdev;
+		unsigned int id;
 
 		larbnode = of_parse_phandle(dev->of_node, "mediatek,larbs", i);
 		if (!larbnode)
@@ -562,6 +568,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
 		if (!of_device_is_available(larbnode))
 			continue;
 
+		ret = of_property_read_u32(larbnode, "mediatek,larb-id", &id);
+		if (ret)/* The id is consecutive if there is no this property */
+			id = i;
+
 		plarbdev = of_find_device_by_node(larbnode);
 		if (!plarbdev) {
 			plarbdev = of_platform_device_create(
@@ -572,7 +582,7 @@ static int mtk_iommu_probe(struct platform_device *pdev)
 				return -EPROBE_DEFER;
 			}
 		}
-		data->smi_imu.larb_imu[i].dev = &plarbdev->dev;
+		data->smi_imu.larb_imu[id].dev = &plarbdev->dev;
 
 		component_match_add_release(dev, &match, release_of,
 					    compare_of, larbnode);
@@ -640,8 +650,6 @@ static int __maybe_unused mtk_iommu_resume(struct device *dev)
 	struct mtk_iommu_suspend_reg *reg = &data->reg;
 	void __iomem *base = data->base;
 
-	writel_relaxed(data->m4u_dom->cfg.arm_v7s_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);
@@ -650,15 +658,19 @@ static int __maybe_unused mtk_iommu_resume(struct device *dev)
 	writel_relaxed(reg->int_main_control, base + REG_MMU_INT_MAIN_CONTROL);
 	writel_relaxed(F_MMU_IVRP_PA_SET(data->protect_base, data->enable_4GB),
 		       base + REG_MMU_IVRP_PADDR);
+	if (data->m4u_dom)
+		writel(data->m4u_dom->cfg.arm_v7s_cfg.ttbr[0],
+		       base + REG_MMU_PT_BASE_ADDR);
 	return 0;
 }
 
-const struct dev_pm_ops mtk_iommu_pm_ops = {
+static const struct dev_pm_ops mtk_iommu_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(mtk_iommu_suspend, mtk_iommu_resume)
 };
 
 static const struct of_device_id mtk_iommu_of_ids[] = {
-	{ .compatible = "mediatek,mt8173-m4u", },
+	{ .compatible = "mediatek,mt2712-m4u", .data = (void *)M4U_MT2712},
+	{ .compatible = "mediatek,mt8173-m4u", .data = (void *)M4U_MT8173},
 	{}
 };
 
@@ -667,16 +679,20 @@ static int __maybe_unused mtk_iommu_resume(struct device *dev)
 	.remove	= mtk_iommu_remove,
 	.driver	= {
 		.name = "mtk-iommu",
-		.of_match_table = mtk_iommu_of_ids,
+		.of_match_table = of_match_ptr(mtk_iommu_of_ids),
 		.pm = &mtk_iommu_pm_ops,
 	}
 };
 
 static int mtk_iommu_init_fn(struct device_node *np)
 {
+	static bool init_done;
 	int ret;
 	struct platform_device *pdev;
 
+	if (init_done)
+		return 0;
+
 	pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
 	if (!pdev)
 		return -ENOMEM;
@@ -686,8 +702,10 @@ static int mtk_iommu_init_fn(struct device_node *np)
 		pr_err("%s: Failed to register driver\n", __func__);
 		return ret;
 	}
+	init_done = true;
 
 	return 0;
 }
 
-IOMMU_OF_DECLARE(mtkm4u, "mediatek,mt8173-m4u", mtk_iommu_init_fn);
+IOMMU_OF_DECLARE(mt8173m4u, "mediatek,mt8173-m4u", mtk_iommu_init_fn);
+IOMMU_OF_DECLARE(mt2712m4u, "mediatek,mt2712-m4u", mtk_iommu_init_fn);
diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
index c06cc91..cd729a3 100644
--- a/drivers/iommu/mtk_iommu.h
+++ b/drivers/iommu/mtk_iommu.h
@@ -34,6 +34,12 @@ struct mtk_iommu_suspend_reg {
 	u32				int_main_control;
 };
 
+enum mtk_iommu_type {
+	M4U_MT2701,
+	M4U_MT2712,
+	M4U_MT8173,
+};
+
 struct mtk_iommu_domain;
 
 struct mtk_iommu_data {
@@ -50,6 +56,7 @@ struct mtk_iommu_data {
 	bool				tlb_flush_active;
 
 	struct iommu_device		iommu;
+	enum mtk_iommu_type		m4u_type;
 };
 
 static inline int compare_of(struct device *dev, void *data)
diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index 13f8c45..ec06d2b 100644
--- a/drivers/memory/mtk-smi.c
+++ b/drivers/memory/mtk-smi.c
@@ -23,7 +23,10 @@
 #include <soc/mediatek/smi.h>
 #include <dt-bindings/memory/mt2701-larb-port.h>
 
+/* mt8173 */
 #define SMI_LARB_MMU_EN		0xf00
+
+/* mt2701 */
 #define REG_SMI_SECUR_CON_BASE		0x5c0
 
 /* every register control 8 port, register offset 0x4 */
@@ -41,6 +44,10 @@
 /* mt2701 domain should be set to 3 */
 #define SMI_SECUR_CON_VAL_DOMAIN(id)	(0x3 << ((((id) & 0x7) << 2) + 1))
 
+/* mt2712 */
+#define SMI_LARB_NONSEC_CON(id)	(0x380 + (id * 4))
+#define F_MMU_EN		BIT(0)
+
 struct mtk_smi_larb_gen {
 	bool need_larbid;
 	int port_in_larb[MTK_LARB_NR_MAX + 1];
@@ -149,7 +156,7 @@ void mtk_smi_larb_put(struct device *larbdev)
 	struct mtk_smi_iommu *smi_iommu = data;
 	unsigned int         i;
 
-	for (i = 0; i < smi_iommu->larb_nr; i++) {
+	for (i = 0; i < MTK_LARB_NR_MAX; i++) {
 		if (dev == smi_iommu->larb_imu[i].dev) {
 			/* The 'mmu' may be updated in iommu-attach/detach. */
 			larb->mmu = &smi_iommu->larb_imu[i].mmu;
@@ -159,13 +166,28 @@ void mtk_smi_larb_put(struct device *larbdev)
 	return -ENODEV;
 }
 
-static void mtk_smi_larb_config_port(struct device *dev)
+static void mtk_smi_larb_config_port_mt8173(struct device *dev)
 {
 	struct mtk_smi_larb *larb = dev_get_drvdata(dev);
 
 	writel(*larb->mmu, larb->base + SMI_LARB_MMU_EN);
 }
 
+static void mtk_smi_larb_config_port_mt2712(struct device *dev)
+{
+	struct mtk_smi_larb *larb = dev_get_drvdata(dev);
+	u32 reg;
+	int i;
+
+	for (i = 0; i < 32; i++) {
+		if (*larb->mmu & BIT(i)) {
+			reg = readl_relaxed(larb->base +
+					    SMI_LARB_NONSEC_CON(i));
+			reg |= F_MMU_EN;
+			writel(reg, larb->base + SMI_LARB_NONSEC_CON(i));
+		}
+	}
+}
 
 static void mtk_smi_larb_config_port_gen1(struct device *dev)
 {
@@ -211,7 +233,11 @@ static void mtk_smi_larb_config_port_gen1(struct device *dev)
 
 static const struct mtk_smi_larb_gen mtk_smi_larb_mt8173 = {
 	/* mt8173 do not need the port in larb */
-	.config_port = mtk_smi_larb_config_port,
+	.config_port = mtk_smi_larb_config_port_mt8173,
+};
+
+static const struct mtk_smi_larb_gen mtk_smi_larb_mt2712 = {
+	.config_port = mtk_smi_larb_config_port_mt2712,
 };
 
 static const struct mtk_smi_larb_gen mtk_smi_larb_mt2701 = {
@@ -232,6 +258,10 @@ static void mtk_smi_larb_config_port_gen1(struct device *dev)
 		.compatible = "mediatek,mt2701-smi-larb",
 		.data = &mtk_smi_larb_mt2701
 	},
+	{
+		.compatible = "mediatek,mt2712-smi-larb",
+		.data = &mtk_smi_larb_mt2712
+	},
 	{}
 };
 
@@ -318,6 +348,10 @@ static int mtk_smi_larb_remove(struct platform_device *pdev)
 		.compatible = "mediatek,mt2701-smi-common",
 		.data = (void *)MTK_SMI_GEN1
 	},
+	{
+		.compatible = "mediatek,mt2712-smi-common",
+		.data = (void *)MTK_SMI_GEN2
+	},
 	{}
 };
 
-- 
1.9.1

Re: [PATCH 2/8] iommu/mediatek: Add mt2712 IOMMU support

[Robin Murphy]

On 11/08/17 10:56, Yong Wu wrote:
> The M4U IP blocks in mt2712 is MTK's generation2 M4U which use the
> Short-descriptor like mt8173, and most of the HW registers are the
> same.
> 
> The difference is that there are 2 M4U HWs in mt2712 while there's
> only one in mt8173. The purpose of 2 M4U HWs is for balance the
> bandwidth.

Heh, I never imagined that theoretical argument I made against global
data in the original driver would become reality so soon :D

> Normally if there are 2 M4U HWs, there should be 2 iommu domains,
> each M4U has a iommu domain.
> 
> Signed-off-by: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
> This patch also include a minor issue:
> suspend while there is no iommu client. it will hang because
> there is no iommu domain at that time.
> ---
>  drivers/iommu/mtk_iommu.c | 48 ++++++++++++++++++++++++++++++++---------------
>  drivers/iommu/mtk_iommu.h |  7 +++++++
>  drivers/memory/mtk-smi.c  | 40 ++++++++++++++++++++++++++++++++++++---
>  3 files changed, 77 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 91c6d36..da6cedb 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -54,7 +54,9 @@
>  
>  #define REG_MMU_CTRL_REG			0x110
>  #define F_MMU_PREFETCH_RT_REPLACE_MOD		BIT(4)
> +/* The TF-protect-select is bit[5:4] in mt2712 while it's bit[6:5] in mt8173.*/
>  #define F_MMU_TF_PROTECT_SEL(prot)		(((prot) & 0x3) << 5)
> +#define F_MMU_TF_PROT_SEL(prot)			(((prot) & 0x3) << 4)

In my opinion PROTECT vs. PROT here is too confusing for its own good...

>  #define REG_MMU_IVRP_PADDR			0x114
>  #define F_MMU_IVRP_PA_SET(pa, ext)		(((pa) >> 1) | ((!!(ext)) << 31))
> @@ -301,10 +303,6 @@ static int mtk_iommu_attach_device(struct iommu_domain *domain,
>  			data->m4u_dom = NULL;
>  			return ret;
>  		}
> -	} else if (data->m4u_dom != dom) {
> -		/* All the client devices should be in the same m4u domain */
> -		dev_err(dev, "try to attach into the error iommu domain\n");
> -		return -EPERM;
>  	}
>  
>  	mtk_iommu_config(data, dev, true);
> @@ -464,8 +462,12 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
>  		return ret;
>  	}
>  
> -	regval = F_MMU_PREFETCH_RT_REPLACE_MOD |
> -		F_MMU_TF_PROTECT_SEL(2);
> +	if (data->m4u_type == M4U_MT8173) {
> +		regval = F_MMU_PREFETCH_RT_REPLACE_MOD |
> +			F_MMU_TF_PROTECT_SEL(2);
> +	} else {
> +		regval = F_MMU_TF_PROT_SEL(2);

...and it would be a bit more obvious to just use
F_MMU_TF_PROTECT_SEL(2) >> 1 here (with the comment from above).
Alternatively, the really bullet-proof option would be something like:

#define F_MMU_TF_PROTECT_SEL_SHIFT(data) \
	((data)->m4u_type == MT2172 ? 4 : 5)
#define F_MMU_TF_PROTECT_SEL(prot, data) \
	((prot) & 0x3) << F_MMU_TF_PROTECT_SEL_SHIFT(data))

> +	}
>  	writel_relaxed(regval, data->base + REG_MMU_CTRL_REG);
>  
>  	regval = F_L2_MULIT_HIT_EN |
> @@ -487,9 +489,11 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
>  
>  	writel_relaxed(F_MMU_IVRP_PA_SET(data->protect_base, data->enable_4GB),
>  		       data->base + REG_MMU_IVRP_PADDR);
> -
>  	writel_relaxed(0, data->base + REG_MMU_DCM_DIS);
> -	writel_relaxed(0, data->base + REG_MMU_STANDARD_AXI_MODE);
> +
> +	/* It's MISC control register whose default value is ok except mt8173.*/
> +	if (data->m4u_type == M4U_MT8173)
> +		writel_relaxed(0, data->base + REG_MMU_STANDARD_AXI_MODE);
>  
>  	if (devm_request_irq(data->dev, data->irq, mtk_iommu_isr, 0,
>  			     dev_name(data->dev), (void *)data)) {
> @@ -521,6 +525,7 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>  	if (!data)
>  		return -ENOMEM;
>  	data->dev = dev;
> +	data->m4u_type = (enum mtk_iommu_type)of_device_get_match_data(dev);
>  
>  	/* Protect memory. HW will access here while translation fault.*/
>  	protect = devm_kzalloc(dev, MTK_PROTECT_PA_ALIGN * 2, GFP_KERNEL);
> @@ -554,6 +559,7 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>  	for (i = 0; i < larb_nr; i++) {
>  		struct device_node *larbnode;
>  		struct platform_device *plarbdev;
> +		unsigned int id;

Strictly, this should be u32...

>  
>  		larbnode = of_parse_phandle(dev->of_node, "mediatek,larbs", i);
>  		if (!larbnode)
> @@ -562,6 +568,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>  		if (!of_device_is_available(larbnode))
>  			continue;
>  
> +		ret = of_property_read_u32(larbnode, "mediatek,larb-id", &id);
> +		if (ret)/* The id is consecutive if there is no this property */
> +			id = i;

...but wouldn't it make more sense for the SMI driver to handle this?
Admittedly it looks like only this driver knows the default IDs thanks
to the ordering of the phandle args, but the SMI driver could at least
initialise larb->larbid to some sentinel value which could be detected
and replaced with i here.

> +
>  		plarbdev = of_find_device_by_node(larbnode);
>  		if (!plarbdev) {
>  			plarbdev = of_platform_device_create(
> @@ -572,7 +582,7 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>  				return -EPROBE_DEFER;
>  			}
>  		}
> -		data->smi_imu.larb_imu[i].dev = &plarbdev->dev;
> +		data->smi_imu.larb_imu[id].dev = &plarbdev->dev;

Changing the way the larb_imu array is indexed also seems to create a
worrying inconsistency with mtk_iommu_v1.

>  		component_match_add_release(dev, &match, release_of,
>  					    compare_of, larbnode);
> @@ -640,8 +650,6 @@ static int __maybe_unused mtk_iommu_resume(struct device *dev)
>  	struct mtk_iommu_suspend_reg *reg = &data->reg;
>  	void __iomem *base = data->base;
>  
> -	writel_relaxed(data->m4u_dom->cfg.arm_v7s_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);
> @@ -650,15 +658,19 @@ static int __maybe_unused mtk_iommu_resume(struct device *dev)
>  	writel_relaxed(reg->int_main_control, base + REG_MMU_INT_MAIN_CONTROL);
>  	writel_relaxed(F_MMU_IVRP_PA_SET(data->protect_base, data->enable_4GB),
>  		       base + REG_MMU_IVRP_PADDR);
> +	if (data->m4u_dom)
> +		writel(data->m4u_dom->cfg.arm_v7s_cfg.ttbr[0],
> +		       base + REG_MMU_PT_BASE_ADDR);
>  	return 0;
>  }
>  
> -const struct dev_pm_ops mtk_iommu_pm_ops = {
> +static const struct dev_pm_ops mtk_iommu_pm_ops = {
>  	SET_SYSTEM_SLEEP_PM_OPS(mtk_iommu_suspend, mtk_iommu_resume)
>  };
>  
>  static const struct of_device_id mtk_iommu_of_ids[] = {
> -	{ .compatible = "mediatek,mt8173-m4u", },
> +	{ .compatible = "mediatek,mt2712-m4u", .data = (void *)M4U_MT2712},
> +	{ .compatible = "mediatek,mt8173-m4u", .data = (void *)M4U_MT8173},
>  	{}
>  };
>  
> @@ -667,16 +679,20 @@ static int __maybe_unused mtk_iommu_resume(struct device *dev)
>  	.remove	= mtk_iommu_remove,
>  	.driver	= {
>  		.name = "mtk-iommu",
> -		.of_match_table = mtk_iommu_of_ids,
> +		.of_match_table = of_match_ptr(mtk_iommu_of_ids),
>  		.pm = &mtk_iommu_pm_ops,
>  	}
>  };
>  
>  static int mtk_iommu_init_fn(struct device_node *np)
>  {
> +	static bool init_done;
>  	int ret;
>  	struct platform_device *pdev;
>  
> +	if (init_done)
> +		return 0;
> +

Actually, you can simply get rid of the whole init_fn now - the
IOMMU_OF_DECLARE() table only remains as a way to identify built-in
drivers for the probe-deferral decision. Hopefully the dodgy-looking
forced creation of plarbdev in probe could go away as well.

>  	pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
>  	if (!pdev)
>  		return -ENOMEM;
> @@ -686,8 +702,10 @@ static int mtk_iommu_init_fn(struct device_node *np)
>  		pr_err("%s: Failed to register driver\n", __func__);
>  		return ret;
>  	}
> +	init_done = true;
>  
>  	return 0;
>  }
>  
> -IOMMU_OF_DECLARE(mtkm4u, "mediatek,mt8173-m4u", mtk_iommu_init_fn);
> +IOMMU_OF_DECLARE(mt8173m4u, "mediatek,mt8173-m4u", mtk_iommu_init_fn);
> +IOMMU_OF_DECLARE(mt2712m4u, "mediatek,mt2712-m4u", mtk_iommu_init_fn);
> diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
> index c06cc91..cd729a3 100644
> --- a/drivers/iommu/mtk_iommu.h
> +++ b/drivers/iommu/mtk_iommu.h
> @@ -34,6 +34,12 @@ struct mtk_iommu_suspend_reg {
>  	u32				int_main_control;
>  };
>  
> +enum mtk_iommu_type {
> +	M4U_MT2701,
> +	M4U_MT2712,
> +	M4U_MT8173,
> +};
> +
>  struct mtk_iommu_domain;
>  
>  struct mtk_iommu_data {
> @@ -50,6 +56,7 @@ struct mtk_iommu_data {
>  	bool				tlb_flush_active;
>  
>  	struct iommu_device		iommu;
> +	enum mtk_iommu_type		m4u_type;
>  };
>  
>  static inline int compare_of(struct device *dev, void *data)
> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> index 13f8c45..ec06d2b 100644
> --- a/drivers/memory/mtk-smi.c
> +++ b/drivers/memory/mtk-smi.c
> @@ -23,7 +23,10 @@
>  #include <soc/mediatek/smi.h>
>  #include <dt-bindings/memory/mt2701-larb-port.h>
>  
> +/* mt8173 */
>  #define SMI_LARB_MMU_EN		0xf00
> +
> +/* mt2701 */
>  #define REG_SMI_SECUR_CON_BASE		0x5c0
>  
>  /* every register control 8 port, register offset 0x4 */
> @@ -41,6 +44,10 @@
>  /* mt2701 domain should be set to 3 */
>  #define SMI_SECUR_CON_VAL_DOMAIN(id)	(0x3 << ((((id) & 0x7) << 2) + 1))
>  
> +/* mt2712 */
> +#define SMI_LARB_NONSEC_CON(id)	(0x380 + (id * 4))
> +#define F_MMU_EN		BIT(0)
> +
>  struct mtk_smi_larb_gen {
>  	bool need_larbid;
>  	int port_in_larb[MTK_LARB_NR_MAX + 1];
> @@ -149,7 +156,7 @@ void mtk_smi_larb_put(struct device *larbdev)
>  	struct mtk_smi_iommu *smi_iommu = data;
>  	unsigned int         i;
>  
> -	for (i = 0; i < smi_iommu->larb_nr; i++) {
> +	for (i = 0; i < MTK_LARB_NR_MAX; i++) {

This initially looked suspicious, but I guess it's related to the
earlier change to indexing. As a result we seem to have a bit of a
redundant mess where some things are using larb->larbid and others are
relying on inferring it from the index in larb_imu.

I'm not sure whether it would end up better to use larbid consistently
everywhere, or to convert everything to make make larb_imu officially a
sparse array indexed by ID (and thus remove smi_iommu->larb_nr and
larb->larbid), but a weird mix of both is not a great idea.

>  		if (dev == smi_iommu->larb_imu[i].dev) {
>  			/* The 'mmu' may be updated in iommu-attach/detach. */
>  			larb->mmu = &smi_iommu->larb_imu[i].mmu;
> @@ -159,13 +166,28 @@ void mtk_smi_larb_put(struct device *larbdev)
>  	return -ENODEV;
>  }
>  
> -static void mtk_smi_larb_config_port(struct device *dev)
> +static void mtk_smi_larb_config_port_mt8173(struct device *dev)
>  {
>  	struct mtk_smi_larb *larb = dev_get_drvdata(dev);
>  
>  	writel(*larb->mmu, larb->base + SMI_LARB_MMU_EN);
>  }
>  
> +static void mtk_smi_larb_config_port_mt2712(struct device *dev)
> +{
> +	struct mtk_smi_larb *larb = dev_get_drvdata(dev);
> +	u32 reg;
> +	int i;
> +
> +	for (i = 0; i < 32; i++) {
> +		if (*larb->mmu & BIT(i)) {

Seeing this immediately make me think of:

	unsigned long enable = *larb->mmu;

	for_each_set_bit(i, &enable, 32) {
		...
	}

but maybe that's overkill :/

Robin.

> +			reg = readl_relaxed(larb->base +
> +					    SMI_LARB_NONSEC_CON(i));
> +			reg |= F_MMU_EN;
> +			writel(reg, larb->base + SMI_LARB_NONSEC_CON(i));
> +		}
> +	}
> +}
>  
>  static void mtk_smi_larb_config_port_gen1(struct device *dev)
>  {
> @@ -211,7 +233,11 @@ static void mtk_smi_larb_config_port_gen1(struct device *dev)
>  
>  static const struct mtk_smi_larb_gen mtk_smi_larb_mt8173 = {
>  	/* mt8173 do not need the port in larb */
> -	.config_port = mtk_smi_larb_config_port,
> +	.config_port = mtk_smi_larb_config_port_mt8173,
> +};
> +
> +static const struct mtk_smi_larb_gen mtk_smi_larb_mt2712 = {
> +	.config_port = mtk_smi_larb_config_port_mt2712,
>  };
>  
>  static const struct mtk_smi_larb_gen mtk_smi_larb_mt2701 = {
> @@ -232,6 +258,10 @@ static void mtk_smi_larb_config_port_gen1(struct device *dev)
>  		.compatible = "mediatek,mt2701-smi-larb",
>  		.data = &mtk_smi_larb_mt2701
>  	},
> +	{
> +		.compatible = "mediatek,mt2712-smi-larb",
> +		.data = &mtk_smi_larb_mt2712
> +	},
>  	{}
>  };
>  
> @@ -318,6 +348,10 @@ static int mtk_smi_larb_remove(struct platform_device *pdev)
>  		.compatible = "mediatek,mt2701-smi-common",
>  		.data = (void *)MTK_SMI_GEN1
>  	},
> +	{
> +		.compatible = "mediatek,mt2712-smi-common",
> +		.data = (void *)MTK_SMI_GEN2
> +	},
>  	{}
>  };
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/8] iommu/mediatek: Add mt2712 IOMMU support

[Yong Wu]

On Fri, 2017-08-11 at 18:24 +0100, Robin Murphy wrote:
> On 11/08/17 10:56, Yong Wu wrote:
> > The M4U IP blocks in mt2712 is MTK's generation2 M4U which use the
> > Short-descriptor like mt8173, and most of the HW registers are the
> > same.
> > 
> > The difference is that there are 2 M4U HWs in mt2712 while there's
> > only one in mt8173. The purpose of 2 M4U HWs is for balance the
> > bandwidth.
> 
> Heh, I never imagined that theoretical argument I made against global
> data in the original driver would become reality so soon :D

Sorry for this.

The global data you said should be "static LIST_HEAD(m4ulist); "

Actually, After this patch, the mt2712 IOMMU can work successfully.

In order to simplify the IOMMU client code, we add that global data to
merge 2 IOMMU domains into one, then they don't need map the iova range
from a iommu domain into another.

I'm not familiar with SMMU. The SMMU should have so many iommu
domains(each context bank has one.). So how the SMMU deal with this
case: Does the modules in different context banks need share the iova?
How does they share the iova?

> 
> > Normally if there are 2 M4U HWs, there should be 2 iommu domains,
> > each M4U has a iommu domain.
> > 
> > Signed-off-by: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > ---
> > This patch also include a minor issue:
> > suspend while there is no iommu client. it will hang because
> > there is no iommu domain at that time.
> > ---
> >  drivers/iommu/mtk_iommu.c | 48 ++++++++++++++++++++++++++++++++---------------
> >  drivers/iommu/mtk_iommu.h |  7 +++++++
> >  drivers/memory/mtk-smi.c  | 40 ++++++++++++++++++++++++++++++++++++---
> >  3 files changed, 77 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > index 91c6d36..da6cedb 100644
> > --- a/drivers/iommu/mtk_iommu.c
> > +++ b/drivers/iommu/mtk_iommu.c
> > @@ -54,7 +54,9 @@
> >  
> >  #define REG_MMU_CTRL_REG			0x110
> >  #define F_MMU_PREFETCH_RT_REPLACE_MOD		BIT(4)
> > +/* The TF-protect-select is bit[5:4] in mt2712 while it's bit[6:5] in mt8173.*/
> >  #define F_MMU_TF_PROTECT_SEL(prot)		(((prot) & 0x3) << 5)
> > +#define F_MMU_TF_PROT_SEL(prot)			(((prot) & 0x3) << 4)
> 
> In my opinion PROTECT vs. PROT here is too confusing for its own good...

.Both the strings are from the coda released by our HW DE..

And your suggestion looks better. I will use:

/* mt2712 is named by F_MMU_TF_PROT_SEL */
#define F_MMU_TF_PROTECT_SEL_SHIFT(data) \
 	((data)->m4u_type == MT2172 ? 4 : 5)
#define F_MMU_TF_PROTECT_SEL(prot, data) \
 	((prot) & 0x3) << F_MMU_TF_PROTECT_SEL_SHIFT(data))

> 
> >  #define REG_MMU_IVRP_PADDR			0x114
> >  #define F_MMU_IVRP_PA_SET(pa, ext)		(((pa) >> 1) | ((!!(ext)) << 31))
> > @@ -301,10 +303,6 @@ static int mtk_iommu_attach_device(struct iommu_domain *domain,
> >  			data->m4u_dom = NULL;
> >  			return ret;
> >  		}
> > -	} else if (data->m4u_dom != dom) {
> > -		/* All the client devices should be in the same m4u domain */
> > -		dev_err(dev, "try to attach into the error iommu domain\n");
> > -		return -EPERM;
> >  	}
> >  
> >  	mtk_iommu_config(data, dev, true);
> > @@ -464,8 +462,12 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
> >  		return ret;
> >  	}
> >  
> > -	regval = F_MMU_PREFETCH_RT_REPLACE_MOD |
> > -		F_MMU_TF_PROTECT_SEL(2);
> > +	if (data->m4u_type == M4U_MT8173) {
> > +		regval = F_MMU_PREFETCH_RT_REPLACE_MOD |
> > +			F_MMU_TF_PROTECT_SEL(2);
> > +	} else {
> > +		regval = F_MMU_TF_PROT_SEL(2);
> 
> ...and it would be a bit more obvious to just use
> F_MMU_TF_PROTECT_SEL(2) >> 1 here (with the comment from above).
> Alternatively, the really bullet-proof option would be something like:
> 
> #define F_MMU_TF_PROTECT_SEL_SHIFT(data) \
> 	((data)->m4u_type == MT2172 ? 4 : 5)
> #define F_MMU_TF_PROTECT_SEL(prot, data) \
> 	((prot) & 0x3) << F_MMU_TF_PROTECT_SEL_SHIFT(data))
> 
> > +	}
> >  	writel_relaxed(regval, data->base + REG_MMU_CTRL_REG);
> >  
> >  	regval = F_L2_MULIT_HIT_EN |
> > @@ -487,9 +489,11 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
> >  
> >  	writel_relaxed(F_MMU_IVRP_PA_SET(data->protect_base, data->enable_4GB),
> >  		       data->base + REG_MMU_IVRP_PADDR);
> > -
> >  	writel_relaxed(0, data->base + REG_MMU_DCM_DIS);
> > -	writel_relaxed(0, data->base + REG_MMU_STANDARD_AXI_MODE);
> > +
> > +	/* It's MISC control register whose default value is ok except mt8173.*/
> > +	if (data->m4u_type == M4U_MT8173)
> > +		writel_relaxed(0, data->base + REG_MMU_STANDARD_AXI_MODE);
> >  
> >  	if (devm_request_irq(data->dev, data->irq, mtk_iommu_isr, 0,
> >  			     dev_name(data->dev), (void *)data)) {
> > @@ -521,6 +525,7 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >  	if (!data)
> >  		return -ENOMEM;
> >  	data->dev = dev;
> > +	data->m4u_type = (enum mtk_iommu_type)of_device_get_match_data(dev);
> >  
> >  	/* Protect memory. HW will access here while translation fault.*/
> >  	protect = devm_kzalloc(dev, MTK_PROTECT_PA_ALIGN * 2, GFP_KERNEL);
> > @@ -554,6 +559,7 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >  	for (i = 0; i < larb_nr; i++) {
> >  		struct device_node *larbnode;
> >  		struct platform_device *plarbdev;
> > +		unsigned int id;
> 
> Strictly, this should be u32...

OK.

> 
> >  
> >  		larbnode = of_parse_phandle(dev->of_node, "mediatek,larbs", i);
> >  		if (!larbnode)
> > @@ -562,6 +568,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >  		if (!of_device_is_available(larbnode))
> >  			continue;
> >  
> > +		ret = of_property_read_u32(larbnode, "mediatek,larb-id", &id);
> > +		if (ret)/* The id is consecutive if there is no this property */
> > +			id = i;
> 
> ...but wouldn't it make more sense for the SMI driver to handle this?
> Admittedly it looks like only this driver knows the default IDs thanks
> to the ordering of the phandle args, but the SMI driver could at least
> initialise larb->larbid to some sentinel value which could be detected
> and replaced with i here.

>From the HW diagram before, the "mediatek,larbs" is
"mediatek,larbs = <&larb0 &larb1 &larb2 &larb3 &larb6>" in M4U0. and
"mediatek,larbs = <&larb4 &larb5 &larb7>" in M4U1.

Both are not consecutive..this is different with mt8173 and mt2701.

The problem is that: In the mtk_iommu_config, it will get the larbid via
"larbid = MTK_M4U_TO_LARB(fwspec->ids[i])".
This function will get the right larbid.

Thus I have to initialize the array "data->smi_imu.larb_imu" with the
real lardid here.

SMI driver will probe defer because it is delayed by MTK power-domain.
It will be very late to initialize the larb-id. So currently IOMMU
initialize the array "smi_imu.larb_imu" according to the real larbid,
and later SMI driver read the array to get the iommu information about
each larb.
Thus, is there some other way the SMI driver can help handle this?

About "the ordering of the phandle args", I have requested it must 
sort in the binding description of "mediatek,larbs".

> 
> > +
> >  		plarbdev = of_find_device_by_node(larbnode);
> >  		if (!plarbdev) {
> >  			plarbdev = of_platform_device_create(
> > @@ -572,7 +582,7 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >  				return -EPROBE_DEFER;
> >  			}
> >  		}
> > -		data->smi_imu.larb_imu[i].dev = &plarbdev->dev;
> > +		data->smi_imu.larb_imu[id].dev = &plarbdev->dev;
> 
> Changing the way the larb_imu array is indexed also seems to create a
> worrying inconsistency with mtk_iommu_v1.
> 
> >  		component_match_add_release(dev, &match, release_of,
> >  					    compare_of, larbnode);
> > @@ -640,8 +650,6 @@ static int __maybe_unused mtk_iommu_resume(struct device *dev)
> >  	struct mtk_iommu_suspend_reg *reg = &data->reg;
> >  	void __iomem *base = data->base;
> >  
> > -	writel_relaxed(data->m4u_dom->cfg.arm_v7s_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);
> > @@ -650,15 +658,19 @@ static int __maybe_unused mtk_iommu_resume(struct device *dev)
> >  	writel_relaxed(reg->int_main_control, base + REG_MMU_INT_MAIN_CONTROL);
> >  	writel_relaxed(F_MMU_IVRP_PA_SET(data->protect_base, data->enable_4GB),
> >  		       base + REG_MMU_IVRP_PADDR);
> > +	if (data->m4u_dom)
> > +		writel(data->m4u_dom->cfg.arm_v7s_cfg.ttbr[0],
> > +		       base + REG_MMU_PT_BASE_ADDR);
> >  	return 0;
> >  }
> >  
> > -const struct dev_pm_ops mtk_iommu_pm_ops = {
> > +static const struct dev_pm_ops mtk_iommu_pm_ops = {
> >  	SET_SYSTEM_SLEEP_PM_OPS(mtk_iommu_suspend, mtk_iommu_resume)
> >  };
> >  
> >  static const struct of_device_id mtk_iommu_of_ids[] = {
> > -	{ .compatible = "mediatek,mt8173-m4u", },
> > +	{ .compatible = "mediatek,mt2712-m4u", .data = (void *)M4U_MT2712},
> > +	{ .compatible = "mediatek,mt8173-m4u", .data = (void *)M4U_MT8173},
> >  	{}
> >  };
> >  
> > @@ -667,16 +679,20 @@ static int __maybe_unused mtk_iommu_resume(struct device *dev)
> >  	.remove	= mtk_iommu_remove,
> >  	.driver	= {
> >  		.name = "mtk-iommu",
> > -		.of_match_table = mtk_iommu_of_ids,
> > +		.of_match_table = of_match_ptr(mtk_iommu_of_ids),
> >  		.pm = &mtk_iommu_pm_ops,
> >  	}
> >  };
> >  
> >  static int mtk_iommu_init_fn(struct device_node *np)
> >  {
> > +	static bool init_done;
> >  	int ret;
> >  	struct platform_device *pdev;
> >  
> > +	if (init_done)
> > +		return 0;
> > +
> 
> Actually, you can simply get rid of the whole init_fn now - the

Thanks. It looks ok after I change here to subsys_initcall.

> IOMMU_OF_DECLARE() table only remains as a way to identify built-in
> drivers for the probe-deferral decision. Hopefully the dodgy-looking

Do you means that IOMMU_OF_DECLARE is needed by probe-deferral drivers?

I tested a deferal driver without IOMMU_OF_DEDCALRE.It could also enter
our of_late. all the flow looks normal.

> forced creation of plarbdev in probe could go away as well.

Yes. I can get rid of it.

> 
> >  	pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
> >  	if (!pdev)
> >  		return -ENOMEM;
> > @@ -686,8 +702,10 @@ static int mtk_iommu_init_fn(struct device_node *np)
> >  		pr_err("%s: Failed to register driver\n", __func__);
> >  		return ret;
> >  	}
> > +	init_done = true;
> >  
> >  	return 0;
> >  }
> >  
> > -IOMMU_OF_DECLARE(mtkm4u, "mediatek,mt8173-m4u", mtk_iommu_init_fn);
> > +IOMMU_OF_DECLARE(mt8173m4u, "mediatek,mt8173-m4u", mtk_iommu_init_fn);
> > +IOMMU_OF_DECLARE(mt2712m4u, "mediatek,mt2712-m4u", mtk_iommu_init_fn);
> > diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
> > index c06cc91..cd729a3 100644
> > --- a/drivers/iommu/mtk_iommu.h
> > +++ b/drivers/iommu/mtk_iommu.h
> > @@ -34,6 +34,12 @@ struct mtk_iommu_suspend_reg {
> >  	u32				int_main_control;
> >  };
> >  
> > +enum mtk_iommu_type {
> > +	M4U_MT2701,
> > +	M4U_MT2712,
> > +	M4U_MT8173,
> > +};
> > +
> >  struct mtk_iommu_domain;
> >  
> >  struct mtk_iommu_data {
> > @@ -50,6 +56,7 @@ struct mtk_iommu_data {
> >  	bool				tlb_flush_active;
> >  
> >  	struct iommu_device		iommu;
> > +	enum mtk_iommu_type		m4u_type;
> >  };
> >  
> >  static inline int compare_of(struct device *dev, void *data)
> > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> > index 13f8c45..ec06d2b 100644
> > --- a/drivers/memory/mtk-smi.c
> > +++ b/drivers/memory/mtk-smi.c
> > @@ -23,7 +23,10 @@
> >  #include <soc/mediatek/smi.h>
> >  #include <dt-bindings/memory/mt2701-larb-port.h>
> >  
> > +/* mt8173 */
> >  #define SMI_LARB_MMU_EN		0xf00
> > +
> > +/* mt2701 */
> >  #define REG_SMI_SECUR_CON_BASE		0x5c0
> >  
> >  /* every register control 8 port, register offset 0x4 */
> > @@ -41,6 +44,10 @@
> >  /* mt2701 domain should be set to 3 */
> >  #define SMI_SECUR_CON_VAL_DOMAIN(id)	(0x3 << ((((id) & 0x7) << 2) + 1))
> >  
> > +/* mt2712 */
> > +#define SMI_LARB_NONSEC_CON(id)	(0x380 + (id * 4))
> > +#define F_MMU_EN		BIT(0)
> > +
> >  struct mtk_smi_larb_gen {
> >  	bool need_larbid;
> >  	int port_in_larb[MTK_LARB_NR_MAX + 1];
> > @@ -149,7 +156,7 @@ void mtk_smi_larb_put(struct device *larbdev)
> >  	struct mtk_smi_iommu *smi_iommu = data;
> >  	unsigned int         i;
> >  
> > -	for (i = 0; i < smi_iommu->larb_nr; i++) {
> > +	for (i = 0; i < MTK_LARB_NR_MAX; i++) {
> 
> This initially looked suspicious, but I guess it's related to the
> earlier change to indexing. As a result we seem to have a bit of a
> redundant mess where some things are using larb->larbid and others are
> relying on inferring it from the index in larb_imu.
> 
> I'm not sure whether it would end up better to use larbid consistently
> everywhere, or to convert everything to make make larb_imu officially a
> sparse array indexed by ID (and thus remove smi_iommu->larb_nr and
> larb->larbid), but a weird mix of both is not a great idea.

see above.

> 
> >  		if (dev == smi_iommu->larb_imu[i].dev) {
> >  			/* The 'mmu' may be updated in iommu-attach/detach. */
> >  			larb->mmu = &smi_iommu->larb_imu[i].mmu;
> > @@ -159,13 +166,28 @@ void mtk_smi_larb_put(struct device *larbdev)
> >  	return -ENODEV;
> >  }
> >  
> > -static void mtk_smi_larb_config_port(struct device *dev)
> > +static void mtk_smi_larb_config_port_mt8173(struct device *dev)
> >  {
> >  	struct mtk_smi_larb *larb = dev_get_drvdata(dev);
> >  
> >  	writel(*larb->mmu, larb->base + SMI_LARB_MMU_EN);
> >  }
> >  
> > +static void mtk_smi_larb_config_port_mt2712(struct device *dev)
> > +{
> > +	struct mtk_smi_larb *larb = dev_get_drvdata(dev);
> > +	u32 reg;
> > +	int i;
> > +
> > +	for (i = 0; i < 32; i++) {
> > +		if (*larb->mmu & BIT(i)) {
> 
> Seeing this immediately make me think of:
> 
> 	unsigned long enable = *larb->mmu;
> 
> 	for_each_set_bit(i, &enable, 32) {
> 		...
> 	}
> 
> but maybe that's overkill :/

Good enough..I will use "u32" to instead of "unsigned long" above.

> 
> Robin.
> 
> > +			reg = readl_relaxed(larb->base +
> > +					    SMI_LARB_NONSEC_CON(i));
> > +			reg |= F_MMU_EN;
> > +			writel(reg, larb->base + SMI_LARB_NONSEC_CON(i));
> > +		}
> > +	}
> > +}
> >  
> >  static void mtk_smi_larb_config_port_gen1(struct device *dev)
> >  {
> > @@ -211,7 +233,11 @@ static void mtk_smi_larb_config_port_gen1(struct device *dev)
> >  
> >  static const struct mtk_smi_larb_gen mtk_smi_larb_mt8173 = {
> >  	/* mt8173 do not need the port in larb */
> > -	.config_port = mtk_smi_larb_config_port,
> > +	.config_port = mtk_smi_larb_config_port_mt8173,
> > +};
> > +
> > +static const struct mtk_smi_larb_gen mtk_smi_larb_mt2712 = {
> > +	.config_port = mtk_smi_larb_config_port_mt2712,
> >  };
> >  
> >  static const struct mtk_smi_larb_gen mtk_smi_larb_mt2701 = {
> > @@ -232,6 +258,10 @@ static void mtk_smi_larb_config_port_gen1(struct device *dev)
> >  		.compatible = "mediatek,mt2701-smi-larb",
> >  		.data = &mtk_smi_larb_mt2701
> >  	},
> > +	{
> > +		.compatible = "mediatek,mt2712-smi-larb",
> > +		.data = &mtk_smi_larb_mt2712
> > +	},
> >  	{}
> >  };
> >  
> > @@ -318,6 +348,10 @@ static int mtk_smi_larb_remove(struct platform_device *pdev)
> >  		.compatible = "mediatek,mt2701-smi-common",
> >  		.data = (void *)MTK_SMI_GEN1
> >  	},
> > +	{
> > +		.compatible = "mediatek,mt2712-smi-common",
> > +		.data = (void *)MTK_SMI_GEN2
> > +	},
> >  	{}
> >  };
> >  
> > 
> 


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/8] iommu/mediatek: Move pgtable allocation into domain_alloc

[Yong Wu]

After adding the global list for M4U HW, We get a chance to
move the pagetable allocation into the mtk_iommu_domain_alloc.
Let the domain_alloc do the right thing.

This patch is for fixing this problem[1].
[1]: https://patchwork.codeaurora.org/patch/53987/

Signed-off-by: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 drivers/iommu/mtk_iommu.c | 50 +++++++++++++++++++----------------------------
 1 file changed, 20 insertions(+), 30 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index e3848e1..e81ed9a 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -254,9 +254,9 @@ static void mtk_iommu_config(struct mtk_iommu_data *data,
 	}
 }
 
-static int mtk_iommu_domain_finalise(struct mtk_iommu_data *data)
+static int mtk_iommu_domain_finalise(struct mtk_iommu_domain *dom)
 {
-	struct mtk_iommu_domain *dom = data->m4u_dom;
+	struct mtk_iommu_data *data = mtk_iommu_get_m4u_data();
 
 	spin_lock_init(&dom->pgtlock);
 
@@ -282,9 +282,6 @@ static int mtk_iommu_domain_finalise(struct mtk_iommu_data *data)
 
 	/* Update our support page sizes bitmap */
 	dom->domain.pgsize_bitmap = dom->cfg.pgsize_bitmap;
-
-	writel(data->m4u_dom->cfg.arm_v7s_cfg.ttbr[0],
-	       data->base + REG_MMU_PT_BASE_ADDR);
 	return 0;
 }
 
@@ -299,20 +296,28 @@ static struct iommu_domain *mtk_iommu_domain_alloc(unsigned type)
 	if (!dom)
 		return NULL;
 
-	if (iommu_get_dma_cookie(&dom->domain)) {
-		kfree(dom);
-		return NULL;
-	}
+	if (iommu_get_dma_cookie(&dom->domain))
+		goto  free_dom;
+
+	if (mtk_iommu_domain_finalise(dom))
+		goto  free_dom;
 
 	dom->domain.geometry.aperture_start = 0;
 	dom->domain.geometry.aperture_end = DMA_BIT_MASK(32);
 	dom->domain.geometry.force_aperture = true;
 
 	return &dom->domain;
+
+free_dom:
+	kfree(dom);
+	return NULL;
 }
 
 static void mtk_iommu_domain_free(struct iommu_domain *domain)
 {
+	struct mtk_iommu_domain *dom = to_mtk_domain(domain);
+
+	free_io_pgtable_ops(dom->iop);
 	iommu_put_dma_cookie(domain);
 	kfree(to_mtk_domain(domain));
 }
@@ -321,33 +326,19 @@ static int mtk_iommu_attach_device(struct iommu_domain *domain,
 				   struct device *dev)
 {
 	struct mtk_iommu_domain *dom = to_mtk_domain(domain);
-	struct mtk_iommu_data *curdata = dev->iommu_fwspec->iommu_priv;
-	struct mtk_iommu_data *data = mtk_iommu_get_m4u_data();
-	int ret;
+	struct mtk_iommu_data *data = dev->iommu_fwspec->iommu_priv;
 
-	if (!data || !curdata)
+	if (!data)
 		return -ENODEV;
 
+	/* Update the pgtable base address register of the M4U HW */
 	if (!data->m4u_dom) {
 		data->m4u_dom = dom;
-		ret = mtk_iommu_domain_finalise(data);
-		if (ret) {
-			data->m4u_dom = NULL;
-			return ret;
-		}
-	}
-
-	/*
-	 * Update the pgtable base address register of another M4U HW with the
-	 * existed pgtable if there are more than one M4U HW.
-	 */
-	if (!curdata->m4u_dom) {
-		curdata->m4u_dom = data->m4u_dom;
-		writel(data->m4u_dom->cfg.arm_v7s_cfg.ttbr[0],
-		       curdata->base + REG_MMU_PT_BASE_ADDR);
+		writel(dom->cfg.arm_v7s_cfg.ttbr[0],
+		       data->base + REG_MMU_PT_BASE_ADDR);
 	}
 
-	mtk_iommu_config(curdata, dev, true);
+	mtk_iommu_config(data, dev, true);
 	return 0;
 }
 
@@ -666,7 +657,6 @@ static int mtk_iommu_remove(struct platform_device *pdev)
 	if (iommu_present(&platform_bus_type))
 		bus_set_iommu(&platform_bus_type, NULL);
 
-	free_io_pgtable_ops(data->m4u_dom->iop);
 	clk_disable_unprepare(data->bclk);
 	devm_free_irq(&pdev->dev, data->irq, data);
 	component_master_del(&pdev->dev, &mtk_iommu_com_ops);
-- 
1.9.1

[PATCH 5/8] iommu/mediatek: Disable iommu clock when system suspend

[Yong Wu]

When system suspend, infra power domain may be off, and the iommu's
clock must be disabled when system off, or the iommu's bclk clock maybe
disabled after system resume.

Signed-off-by: Honghui Zhang <honghui.zhang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
Signed-off-by: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 drivers/iommu/mtk_iommu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index e81ed9a..b7c8e52 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -675,6 +675,7 @@ static int __maybe_unused mtk_iommu_suspend(struct device *dev)
 	reg->ctrl_reg = readl_relaxed(base + REG_MMU_CTRL_REG);
 	reg->int_control0 = readl_relaxed(base + REG_MMU_INT_CONTROL0);
 	reg->int_main_control = readl_relaxed(base + REG_MMU_INT_MAIN_CONTROL);
+	clk_disable_unprepare(data->bclk);
 	return 0;
 }
 
@@ -684,6 +685,7 @@ static int __maybe_unused mtk_iommu_resume(struct device *dev)
 	struct mtk_iommu_suspend_reg *reg = &data->reg;
 	void __iomem *base = data->base;
 
+	clk_prepare_enable(data->bclk);
 	writel_relaxed(reg->standard_axi_mode,
 		       base + REG_MMU_STANDARD_AXI_MODE);
 	writel_relaxed(reg->dcm_dis, base + REG_MMU_DCM_DIS);
@@ -699,7 +701,7 @@ static int __maybe_unused mtk_iommu_resume(struct device *dev)
 }
 
 static const struct dev_pm_ops mtk_iommu_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(mtk_iommu_suspend, mtk_iommu_resume)
+	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(mtk_iommu_suspend, mtk_iommu_resume)
 };
 
 static const struct of_device_id mtk_iommu_of_ids[] = {
-- 
1.9.1

Re: [PATCH 5/8] iommu/mediatek: Disable iommu clock when system suspend

[Arvind Yadav]

Hi Youn,


On Friday 11 August 2017 03:26 PM, Yong Wu wrote:
> When system suspend, infra power domain may be off, and the iommu's
> clock must be disabled when system off, or the iommu's bclk clock maybe
> disabled after system resume.
>
> Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> ---
>   drivers/iommu/mtk_iommu.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index e81ed9a..b7c8e52 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -675,6 +675,7 @@ static int __maybe_unused mtk_iommu_suspend(struct device *dev)
>   	reg->ctrl_reg = readl_relaxed(base + REG_MMU_CTRL_REG);
>   	reg->int_control0 = readl_relaxed(base + REG_MMU_INT_CONTROL0);
>   	reg->int_main_control = readl_relaxed(base + REG_MMU_INT_MAIN_CONTROL);
> +	clk_disable_unprepare(data->bclk);
>   	return 0;
>   }
>   
> @@ -684,6 +685,7 @@ static int __maybe_unused mtk_iommu_resume(struct device *dev)
>   	struct mtk_iommu_suspend_reg *reg = &data->reg;
>   	void __iomem *base = data->base;
>   
> +	clk_prepare_enable(data->bclk);
Please handle return value of clk_prepare_enable. It can fail
>   	writel_relaxed(reg->standard_axi_mode,
>   		       base + REG_MMU_STANDARD_AXI_MODE);
>   	writel_relaxed(reg->dcm_dis, base + REG_MMU_DCM_DIS);
> @@ -699,7 +701,7 @@ static int __maybe_unused mtk_iommu_resume(struct device *dev)
>   }
>   
>   static const struct dev_pm_ops mtk_iommu_pm_ops = {
> -	SET_SYSTEM_SLEEP_PM_OPS(mtk_iommu_suspend, mtk_iommu_resume)
> +	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(mtk_iommu_suspend, mtk_iommu_resume)
>   };
>   
>   static const struct of_device_id mtk_iommu_of_ids[] = {
~arvind

Re: [PATCH 5/8] iommu/mediatek: Disable iommu clock when system suspend

[Yong Wu]

On Fri, 2017-08-11 at 16:39 +0530, Arvind Yadav wrote:
> Hi Youn,
> 
> 
> On Friday 11 August 2017 03:26 PM, Yong Wu wrote:
> > When system suspend, infra power domain may be off, and the iommu's
> > clock must be disabled when system off, or the iommu's bclk clock maybe
> > disabled after system resume.
> >
> > Signed-off-by: Honghui Zhang <honghui.zhang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > Signed-off-by: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > ---
> >   drivers/iommu/mtk_iommu.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > index e81ed9a..b7c8e52 100644
> > --- a/drivers/iommu/mtk_iommu.c
> > +++ b/drivers/iommu/mtk_iommu.c
> > @@ -675,6 +675,7 @@ static int __maybe_unused mtk_iommu_suspend(struct device *dev)
> >   	reg->ctrl_reg = readl_relaxed(base + REG_MMU_CTRL_REG);
> >   	reg->int_control0 = readl_relaxed(base + REG_MMU_INT_CONTROL0);
> >   	reg->int_main_control = readl_relaxed(base + REG_MMU_INT_MAIN_CONTROL);
> > +	clk_disable_unprepare(data->bclk);
> >   	return 0;
> >   }
> >   
> > @@ -684,6 +685,7 @@ static int __maybe_unused mtk_iommu_resume(struct device *dev)
> >   	struct mtk_iommu_suspend_reg *reg = &data->reg;
> >   	void __iomem *base = data->base;
> >   
> > +	clk_prepare_enable(data->bclk);
> Please handle return value of clk_prepare_enable. It can fail

Thanks for the reminding. I will add it in next version.

> >   	writel_relaxed(reg->standard_axi_mode,
> >   		       base + REG_MMU_STANDARD_AXI_MODE);
> >   	writel_relaxed(reg->dcm_dis, base + REG_MMU_DCM_DIS);
> > @@ -699,7 +701,7 @@ static int __maybe_unused mtk_iommu_resume(struct device *dev)
> >   }
> >   
> >   static const struct dev_pm_ops mtk_iommu_pm_ops = {
> > -	SET_SYSTEM_SLEEP_PM_OPS(mtk_iommu_suspend, mtk_iommu_resume)
> > +	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(mtk_iommu_suspend, mtk_iommu_resume)
> >   };
> >   
> >   static const struct of_device_id mtk_iommu_of_ids[] = {
> ~arvind


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 6/8] iommu/mediatek: Enlarge the validate PA range for 4GB mode

[Yong Wu]

This patch is for 4GB mode, mainly for 4 issues:
1) Fix a 4GB bug:
   if the dram base is 0x4000_0000, the dram size is 0xc000_0000.
   then the code just meet a corner case because max_pfn is
   0x10_0000.
   data->enable_4GB = !!(max_pfn > (0xffffffffUL >> PAGE_SHIFT));
   It's true at the case above. That isn't unexpected.
2) In mt2712, there is a new register for the 4GB PA range(0x118)
   we should enlarge the max PA range, or the HW will report
   error.
   The dram range is from 0x1_0000_0000 to 0x1_ffff_ffff in the 4GB
   mode, we cut out the bit[32:30] of the SA(Start address) and
   EA(End address) into this REG_MMU_VLD_PA_RNG(0x118).
3) In mt2712, the register(0x13c) is extended for 4GB mode.
   bit[7:6] indicate the valid PA[32:33]. Thus, we don't mask the
   value and print it directly for debug.
4) if 4GB is enabled, the dram PA range is from 0x1_0000_0000 to
   0x1_ffff_ffff. Thus, the PA from iova_to_pa should also '|' BIT(32)

Signed-off-by: Honghui Zhang <honghui.zhang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
Signed-off-by: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
this patch also include bug-fix for mt8173 and mt2712.
I also don't split them.
---
 drivers/iommu/mtk_iommu.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index b7c8e52..a3817e0 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -60,6 +60,8 @@
 
 #define REG_MMU_IVRP_PADDR			0x114
 #define F_MMU_IVRP_PA_SET(pa, ext)		(((pa) >> 1) | ((!!(ext)) << 31))
+#define REG_MMU_VLD_PA_RNG			0x118
+#define F_MMU_VLD_PA_RNG(EA, SA)		(((EA) << 8) | (SA))
 
 #define REG_MMU_INT_CONTROL0			0x120
 #define F_L2_MULIT_HIT_EN			BIT(0)
@@ -84,7 +86,6 @@
 #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)
 
@@ -206,7 +207,6 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
 	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);
@@ -385,6 +385,7 @@ 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);
+	struct mtk_iommu_data *data = mtk_iommu_get_m4u_data();
 	unsigned long flags;
 	phys_addr_t pa;
 
@@ -392,6 +393,9 @@ static phys_addr_t mtk_iommu_iova_to_phys(struct iommu_domain *domain,
 	pa = dom->iop->iova_to_phys(dom->iop, iova);
 	spin_unlock_irqrestore(&dom->pgtlock, flags);
 
+	if (data->enable_4GB)
+		pa |= BIT(32);
+
 	return pa;
 }
 
@@ -522,6 +526,14 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
 
 	writel_relaxed(F_MMU_IVRP_PA_SET(data->protect_base, data->enable_4GB),
 		       data->base + REG_MMU_IVRP_PADDR);
+	if (data->enable_4GB && data->m4u_type != M4U_MT8173) {
+		/*
+		 * If 4GB mode is enabled, the validate PA range is from
+		 * 0x1_0000_0000 to 0x1_ffff_ffff. here record bit[32:30].
+		 */
+		regval = F_MMU_VLD_PA_RNG(7, 4);
+		writel_relaxed(regval, data->base + REG_MMU_VLD_PA_RNG);
+	}
 	writel_relaxed(0, data->base + REG_MMU_DCM_DIS);
 
 	/* It's MISC control register whose default value is ok except mt8173.*/
@@ -567,7 +579,7 @@ static int mtk_iommu_probe(struct platform_device *pdev)
 	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
 
 	/* Whether the current dram is over 4GB */
-	data->enable_4GB = !!(max_pfn > (0xffffffffUL >> PAGE_SHIFT));
+	data->enable_4GB = !!(max_pfn > (BIT(32) >> PAGE_SHIFT));
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	data->base = devm_ioremap_resource(dev, res);
-- 
1.9.1

[PATCH 7/8] memory: mtk-smi: Rearrange some function position alphabetically

[Yong Wu]

Only adjust some code position in Soc numerical order, from mt2701,
mt2712 to mt8173.

Besides, 3 minor changes:
1) fix a coding style issue:
    CHECK: Alignment should match open parenthesis
    +               writel(reg_val,
    +                       common->smi_ao_base
2) change from readl to readl_relaxed in gen1_config_port.
3) change the type "larbid" from "int" to "unsigned int" to meet
   the requirement of of_property_read_u32.

Signed-off-by: Yong Wu <yong.wu@mediatek.com>
---
 drivers/memory/mtk-smi.c | 105 +++++++++++++++++++++++------------------------
 1 file changed, 52 insertions(+), 53 deletions(-)

diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index ec06d2b..ce27bdf 100644
--- a/drivers/memory/mtk-smi.c
+++ b/drivers/memory/mtk-smi.c
@@ -23,9 +23,6 @@
 #include <soc/mediatek/smi.h>
 #include <dt-bindings/memory/mt2701-larb-port.h>
 
-/* mt8173 */
-#define SMI_LARB_MMU_EN		0xf00
-
 /* mt2701 */
 #define REG_SMI_SECUR_CON_BASE		0x5c0
 
@@ -48,16 +45,19 @@
 #define SMI_LARB_NONSEC_CON(id)	(0x380 + (id * 4))
 #define F_MMU_EN		BIT(0)
 
+/* mt8173 */
+#define SMI_LARB_MMU_EN		0xf00
+
 struct mtk_smi_larb_gen {
-	bool need_larbid;
-	int port_in_larb[MTK_LARB_NR_MAX + 1];
-	void (*config_port)(struct device *);
+	bool	need_larbid;
+	int	port_in_larb[MTK_LARB_NR_MAX + 1];/* Only needed by smi gen1 */
+	void	(*config_port)(struct device *);
 };
 
 struct mtk_smi {
 	struct device			*dev;
 	struct clk			*clk_apb, *clk_smi;
-	struct clk			*clk_async; /*only needed by mt2701*/
+	struct clk			*clk_async; /*only needed by smi gen1*/
 	void __iomem			*smi_ao_base;
 };
 
@@ -66,7 +66,7 @@ struct mtk_smi_larb { /* larb: local arbiter */
 	void __iomem			*base;
 	struct device			*smi_common_dev;
 	const struct mtk_smi_larb_gen	*larb_gen;
-	int				larbid;
+	unsigned int			larbid;
 	u32				*mmu;
 };
 
@@ -166,28 +166,16 @@ void mtk_smi_larb_put(struct device *larbdev)
 	return -ENODEV;
 }
 
-static void mtk_smi_larb_config_port_mt8173(struct device *dev)
+static void
+mtk_smi_larb_unbind(struct device *dev, struct device *master, void *data)
 {
-	struct mtk_smi_larb *larb = dev_get_drvdata(dev);
-
-	writel(*larb->mmu, larb->base + SMI_LARB_MMU_EN);
+	/* Do nothing as the iommu is always enabled. */
 }
 
-static void mtk_smi_larb_config_port_mt2712(struct device *dev)
-{
-	struct mtk_smi_larb *larb = dev_get_drvdata(dev);
-	u32 reg;
-	int i;
-
-	for (i = 0; i < 32; i++) {
-		if (*larb->mmu & BIT(i)) {
-			reg = readl_relaxed(larb->base +
-					    SMI_LARB_NONSEC_CON(i));
-			reg |= F_MMU_EN;
-			writel(reg, larb->base + SMI_LARB_NONSEC_CON(i));
-		}
-	}
-}
+static const struct component_ops mtk_smi_larb_component_ops = {
+	.bind = mtk_smi_larb_bind,
+	.unbind = mtk_smi_larb_unbind,
+};
 
 static void mtk_smi_larb_config_port_gen1(struct device *dev)
 {
@@ -209,36 +197,39 @@ static void mtk_smi_larb_config_port_gen1(struct device *dev)
 			/* do not need to enable m4u for this port */
 			continue;
 		}
-		reg_val = readl(common->smi_ao_base
+		reg_val = readl_relaxed(common->smi_ao_base
 			+ REG_SMI_SECUR_CON_ADDR(m4u_port_id));
 		reg_val &= SMI_SECUR_CON_VAL_MSK(m4u_port_id);
 		reg_val |= sec_con_val;
 		reg_val |= SMI_SECUR_CON_VAL_DOMAIN(m4u_port_id);
 		writel(reg_val,
-			common->smi_ao_base
-			+ REG_SMI_SECUR_CON_ADDR(m4u_port_id));
+		       common->smi_ao_base +
+		       REG_SMI_SECUR_CON_ADDR(m4u_port_id));
 	}
 }
 
-static void
-mtk_smi_larb_unbind(struct device *dev, struct device *master, void *data)
+static void mtk_smi_larb_config_port_mt2712(struct device *dev)
 {
-	/* Do nothing as the iommu is always enabled. */
-}
+	struct mtk_smi_larb *larb = dev_get_drvdata(dev);
+	u32 reg;
+	int i;
 
-static const struct component_ops mtk_smi_larb_component_ops = {
-	.bind = mtk_smi_larb_bind,
-	.unbind = mtk_smi_larb_unbind,
-};
+	for (i = 0; i < 32; i++) {
+		if (*larb->mmu & BIT(i)) {
+			reg = readl_relaxed(larb->base +
+					    SMI_LARB_NONSEC_CON(i));
+			reg |= F_MMU_EN;
+			writel(reg, larb->base + SMI_LARB_NONSEC_CON(i));
+		}
+	}
+}
 
-static const struct mtk_smi_larb_gen mtk_smi_larb_mt8173 = {
-	/* mt8173 do not need the port in larb */
-	.config_port = mtk_smi_larb_config_port_mt8173,
-};
+static void mtk_smi_larb_config_port_mt8173(struct device *dev)
+{
+	struct mtk_smi_larb *larb = dev_get_drvdata(dev);
 
-static const struct mtk_smi_larb_gen mtk_smi_larb_mt2712 = {
-	.config_port = mtk_smi_larb_config_port_mt2712,
-};
+	writel(*larb->mmu, larb->base + SMI_LARB_MMU_EN);
+}
 
 static const struct mtk_smi_larb_gen mtk_smi_larb_mt2701 = {
 	.need_larbid = true,
@@ -249,12 +240,16 @@ static void mtk_smi_larb_config_port_gen1(struct device *dev)
 	.config_port = mtk_smi_larb_config_port_gen1,
 };
 
+static const struct mtk_smi_larb_gen mtk_smi_larb_mt2712 = {
+	.config_port = mtk_smi_larb_config_port_mt2712,
+};
+
+static const struct mtk_smi_larb_gen mtk_smi_larb_mt8173 = {
+	.config_port = mtk_smi_larb_config_port_mt8173,
+};
+
 static const struct of_device_id mtk_smi_larb_of_ids[] = {
 	{
-		.compatible = "mediatek,mt8173-smi-larb",
-		.data = &mtk_smi_larb_mt8173
-	},
-	{
 		.compatible = "mediatek,mt2701-smi-larb",
 		.data = &mtk_smi_larb_mt2701
 	},
@@ -262,6 +257,10 @@ static void mtk_smi_larb_config_port_gen1(struct device *dev)
 		.compatible = "mediatek,mt2712-smi-larb",
 		.data = &mtk_smi_larb_mt2712
 	},
+	{
+		.compatible = "mediatek,mt8173-smi-larb",
+		.data = &mtk_smi_larb_mt8173
+	},
 	{}
 };
 
@@ -341,10 +340,6 @@ static int mtk_smi_larb_remove(struct platform_device *pdev)
 
 static const struct of_device_id mtk_smi_common_of_ids[] = {
 	{
-		.compatible = "mediatek,mt8173-smi-common",
-		.data = (void *)MTK_SMI_GEN2
-	},
-	{
 		.compatible = "mediatek,mt2701-smi-common",
 		.data = (void *)MTK_SMI_GEN1
 	},
@@ -352,6 +347,10 @@ static int mtk_smi_larb_remove(struct platform_device *pdev)
 		.compatible = "mediatek,mt2712-smi-common",
 		.data = (void *)MTK_SMI_GEN2
 	},
+	{
+		.compatible = "mediatek,mt8173-smi-common",
+		.data = (void *)MTK_SMI_GEN2
+	},
 	{}
 };
 
-- 
1.9.1

Re: [PATCH 7/8] memory: mtk-smi: Rearrange some function position alphabetically

[Robin Murphy]

On 11/08/17 10:56, Yong Wu wrote:
> Only adjust some code position in Soc numerical order, from mt2701,
> mt2712 to mt8173.
> 
> Besides, 3 minor changes:
> 1) fix a coding style issue:
>     CHECK: Alignment should match open parenthesis
>     +               writel(reg_val,
>     +                       common->smi_ao_base
> 2) change from readl to readl_relaxed in gen1_config_port.
> 3) change the type "larbid" from "int" to "unsigned int" to meet
>    the requirement of of_property_read_u32.

If moving existing code around is really necessary, do it as a
preliminary patch *before* any material changes (and arguably separate
even from the whitespace and comment updates) - those diffs are usually
hard to review as-is, so being able to check you get binary-identical
object files before and after is reassuring. A "cleanup" patch shouldn't
need to touch code added in the same series, and it certainly shouldn't
have significant things like the readl_relaxed() change hidden in it.

Robin.

> Signed-off-by: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/memory/mtk-smi.c | 105 +++++++++++++++++++++++------------------------
>  1 file changed, 52 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> index ec06d2b..ce27bdf 100644
> --- a/drivers/memory/mtk-smi.c
> +++ b/drivers/memory/mtk-smi.c
> @@ -23,9 +23,6 @@
>  #include <soc/mediatek/smi.h>
>  #include <dt-bindings/memory/mt2701-larb-port.h>
>  
> -/* mt8173 */
> -#define SMI_LARB_MMU_EN		0xf00
> -
>  /* mt2701 */
>  #define REG_SMI_SECUR_CON_BASE		0x5c0
>  
> @@ -48,16 +45,19 @@
>  #define SMI_LARB_NONSEC_CON(id)	(0x380 + (id * 4))
>  #define F_MMU_EN		BIT(0)
>  
> +/* mt8173 */
> +#define SMI_LARB_MMU_EN		0xf00
> +
>  struct mtk_smi_larb_gen {
> -	bool need_larbid;
> -	int port_in_larb[MTK_LARB_NR_MAX + 1];
> -	void (*config_port)(struct device *);
> +	bool	need_larbid;
> +	int	port_in_larb[MTK_LARB_NR_MAX + 1];/* Only needed by smi gen1 */
> +	void	(*config_port)(struct device *);
>  };
>  
>  struct mtk_smi {
>  	struct device			*dev;
>  	struct clk			*clk_apb, *clk_smi;
> -	struct clk			*clk_async; /*only needed by mt2701*/
> +	struct clk			*clk_async; /*only needed by smi gen1*/
>  	void __iomem			*smi_ao_base;
>  };
>  
> @@ -66,7 +66,7 @@ struct mtk_smi_larb { /* larb: local arbiter */
>  	void __iomem			*base;
>  	struct device			*smi_common_dev;
>  	const struct mtk_smi_larb_gen	*larb_gen;
> -	int				larbid;
> +	unsigned int			larbid;
>  	u32				*mmu;
>  };
>  
> @@ -166,28 +166,16 @@ void mtk_smi_larb_put(struct device *larbdev)
>  	return -ENODEV;
>  }
>  
> -static void mtk_smi_larb_config_port_mt8173(struct device *dev)
> +static void
> +mtk_smi_larb_unbind(struct device *dev, struct device *master, void *data)
>  {
> -	struct mtk_smi_larb *larb = dev_get_drvdata(dev);
> -
> -	writel(*larb->mmu, larb->base + SMI_LARB_MMU_EN);
> +	/* Do nothing as the iommu is always enabled. */
>  }
>  
> -static void mtk_smi_larb_config_port_mt2712(struct device *dev)
> -{
> -	struct mtk_smi_larb *larb = dev_get_drvdata(dev);
> -	u32 reg;
> -	int i;
> -
> -	for (i = 0; i < 32; i++) {
> -		if (*larb->mmu & BIT(i)) {
> -			reg = readl_relaxed(larb->base +
> -					    SMI_LARB_NONSEC_CON(i));
> -			reg |= F_MMU_EN;
> -			writel(reg, larb->base + SMI_LARB_NONSEC_CON(i));
> -		}
> -	}
> -}
> +static const struct component_ops mtk_smi_larb_component_ops = {
> +	.bind = mtk_smi_larb_bind,
> +	.unbind = mtk_smi_larb_unbind,
> +};
>  
>  static void mtk_smi_larb_config_port_gen1(struct device *dev)
>  {
> @@ -209,36 +197,39 @@ static void mtk_smi_larb_config_port_gen1(struct device *dev)
>  			/* do not need to enable m4u for this port */
>  			continue;
>  		}
> -		reg_val = readl(common->smi_ao_base
> +		reg_val = readl_relaxed(common->smi_ao_base
>  			+ REG_SMI_SECUR_CON_ADDR(m4u_port_id));
>  		reg_val &= SMI_SECUR_CON_VAL_MSK(m4u_port_id);
>  		reg_val |= sec_con_val;
>  		reg_val |= SMI_SECUR_CON_VAL_DOMAIN(m4u_port_id);
>  		writel(reg_val,
> -			common->smi_ao_base
> -			+ REG_SMI_SECUR_CON_ADDR(m4u_port_id));
> +		       common->smi_ao_base +
> +		       REG_SMI_SECUR_CON_ADDR(m4u_port_id));
>  	}
>  }
>  
> -static void
> -mtk_smi_larb_unbind(struct device *dev, struct device *master, void *data)
> +static void mtk_smi_larb_config_port_mt2712(struct device *dev)
>  {
> -	/* Do nothing as the iommu is always enabled. */
> -}
> +	struct mtk_smi_larb *larb = dev_get_drvdata(dev);
> +	u32 reg;
> +	int i;
>  
> -static const struct component_ops mtk_smi_larb_component_ops = {
> -	.bind = mtk_smi_larb_bind,
> -	.unbind = mtk_smi_larb_unbind,
> -};
> +	for (i = 0; i < 32; i++) {
> +		if (*larb->mmu & BIT(i)) {
> +			reg = readl_relaxed(larb->base +
> +					    SMI_LARB_NONSEC_CON(i));
> +			reg |= F_MMU_EN;
> +			writel(reg, larb->base + SMI_LARB_NONSEC_CON(i));
> +		}
> +	}
> +}
>  
> -static const struct mtk_smi_larb_gen mtk_smi_larb_mt8173 = {
> -	/* mt8173 do not need the port in larb */
> -	.config_port = mtk_smi_larb_config_port_mt8173,
> -};
> +static void mtk_smi_larb_config_port_mt8173(struct device *dev)
> +{
> +	struct mtk_smi_larb *larb = dev_get_drvdata(dev);
>  
> -static const struct mtk_smi_larb_gen mtk_smi_larb_mt2712 = {
> -	.config_port = mtk_smi_larb_config_port_mt2712,
> -};
> +	writel(*larb->mmu, larb->base + SMI_LARB_MMU_EN);
> +}
>  
>  static const struct mtk_smi_larb_gen mtk_smi_larb_mt2701 = {
>  	.need_larbid = true,
> @@ -249,12 +240,16 @@ static void mtk_smi_larb_config_port_gen1(struct device *dev)
>  	.config_port = mtk_smi_larb_config_port_gen1,
>  };
>  
> +static const struct mtk_smi_larb_gen mtk_smi_larb_mt2712 = {
> +	.config_port = mtk_smi_larb_config_port_mt2712,
> +};
> +
> +static const struct mtk_smi_larb_gen mtk_smi_larb_mt8173 = {
> +	.config_port = mtk_smi_larb_config_port_mt8173,
> +};
> +
>  static const struct of_device_id mtk_smi_larb_of_ids[] = {
>  	{
> -		.compatible = "mediatek,mt8173-smi-larb",
> -		.data = &mtk_smi_larb_mt8173
> -	},
> -	{
>  		.compatible = "mediatek,mt2701-smi-larb",
>  		.data = &mtk_smi_larb_mt2701
>  	},
> @@ -262,6 +257,10 @@ static void mtk_smi_larb_config_port_gen1(struct device *dev)
>  		.compatible = "mediatek,mt2712-smi-larb",
>  		.data = &mtk_smi_larb_mt2712
>  	},
> +	{
> +		.compatible = "mediatek,mt8173-smi-larb",
> +		.data = &mtk_smi_larb_mt8173
> +	},
>  	{}
>  };
>  
> @@ -341,10 +340,6 @@ static int mtk_smi_larb_remove(struct platform_device *pdev)
>  
>  static const struct of_device_id mtk_smi_common_of_ids[] = {
>  	{
> -		.compatible = "mediatek,mt8173-smi-common",
> -		.data = (void *)MTK_SMI_GEN2
> -	},
> -	{
>  		.compatible = "mediatek,mt2701-smi-common",
>  		.data = (void *)MTK_SMI_GEN1
>  	},
> @@ -352,6 +347,10 @@ static int mtk_smi_larb_remove(struct platform_device *pdev)
>  		.compatible = "mediatek,mt2712-smi-common",
>  		.data = (void *)MTK_SMI_GEN2
>  	},
> +	{
> +		.compatible = "mediatek,mt8173-smi-common",
> +		.data = (void *)MTK_SMI_GEN2
> +	},
>  	{}
>  };
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/8] memory: mtk-smi: Rearrange some function position alphabetically

[Yong Wu]

On Fri, 2017-08-11 at 19:09 +0100, Robin Murphy wrote:
> On 11/08/17 10:56, Yong Wu wrote:
> > Only adjust some code position in Soc numerical order, from mt2701,
> > mt2712 to mt8173.
> > 
> > Besides, 3 minor changes:
> > 1) fix a coding style issue:
> >     CHECK: Alignment should match open parenthesis
> >     +               writel(reg_val,
> >     +                       common->smi_ao_base
> > 2) change from readl to readl_relaxed in gen1_config_port.
> > 3) change the type "larbid" from "int" to "unsigned int" to meet
> >    the requirement of of_property_read_u32.
> 
> If moving existing code around is really necessary, do it as a
> preliminary patch *before* any material changes (and arguably separate
> even from the whitespace and comment updates) - those diffs are usually
> hard to review as-is, so being able to check you get binary-identical
> object files before and after is reassuring. A "cleanup" patch shouldn't
> need to touch code added in the same series, and it certainly shouldn't
> have significant things like the readl_relaxed() change hidden in it.

OK. This patch is really not easy to read.

This patch-set is mainly for supporting MT2712 IOMMU. thus, I will use a
patch "readl_relaxed replace readl" instead of this one.

About the cleanup patch, I can send it in future if necessary.

> 
> Robin.
> 
> > Signed-off-by: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > ---
> >  drivers/memory/mtk-smi.c | 105 +++++++++++++++++++++++------------------------
> >  1 file changed, 52 insertions(+), 53 deletions(-)
> > 
[...]


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 8/8] memory: mtk-smi: Degrade SMI init to module_init

[Yong Wu]

The initialization of Mediatek power manager(SCPSYS) is
builtin_platform_driver, and SMI must depend on power-domain.
Thus, currently subsys_initcall for SMI is unnecessary, SMI will be
always probe defered by power-domain. Degrade it to module_init.

In addition, there are two small changes.
1) Delete this two lines.
    	if (!dev->pm_domain)
		return -EPROBE_DEFER;
   This is not helpful. the platform driver framework guarantee this.
   The "dev_pm_domain_attach" in the "platform_drv_probe" will return
   EPROBE_DEFER if its powerdomain is not ready.

2) Add the probe-defer for the smi-larb device should waiting for
   smi-common.
   In mt2712, there are 2 smi-common, 8 smi-larb. All will be
   probe-defered by the power-domain, there is seldom case that
   smi-larb probe done before smi-common. then it will hang like
   this:

   Unable to handle kernel NULL pointer dereference at virtual address
00000000 pgd = ffffff800a4e0000
[00000000] *pgd=00000000beffe003[   17.610026] , *pud=00000000beffe003
...
[<ffffff800897fe04>] mtk_smi_enable+0x1c/0xd0
[<ffffff800897fee8>] mtk_smi_larb_get+0x30/0x98
[<ffffff80088edfa8>] mtk_mipicsi0_resume+0x38/0x1b8
[<ffffff8008634f44>] pm_generic_runtime_resume+0x3c/0x58
[<ffffff8008644ff8>] __genpd_runtime_resume+0x38/0x98
[<ffffff8008647434>] genpd_runtime_resume+0x164/0x220
[<ffffff80086372f8>] __rpm_callback+0x78/0xa0
[<ffffff8008637358>] rpm_callback+0x38/0xa0
[<ffffff8008638a4c>] rpm_resume+0x4a4/0x6f8
[<ffffff8008638d04>] __pm_runtime_resume+0x64/0xa0
[<ffffff80088ed05c>] mtk_mipicsi0_probe+0x40c/0xb70
[<ffffff800862cdc0>] platform_drv_probe+0x58/0xc0
[<ffffff800862a514>] driver_probe_device+0x284/0x438
[<ffffff800862a8ac>] __device_attach_driver+0xb4/0x160
[<ffffff8008627d58>] bus_for_each_drv+0x68/0xa8
[<ffffff800862a0a4>] __device_attach+0xd4/0x168
[<ffffff800862a9d4>] device_initial_probe+0x24/0x30
[<ffffff80086291d8>] bus_probe_device+0xa0/0xa8
[<ffffff8008629784>] deferred_probe_work_func+0x94/0xf0
[<ffffff80080f03a8>] process_one_work+0x1d8/0x6e0

Signed-off-by: Yong Wu <yong.wu@mediatek.com>
---
 drivers/memory/mtk-smi.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index ce27bdf..1a0286f 100644
--- a/drivers/memory/mtk-smi.c
+++ b/drivers/memory/mtk-smi.c
@@ -16,6 +16,7 @@
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/io.h>
+#include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
@@ -273,9 +274,6 @@ static int mtk_smi_larb_probe(struct platform_device *pdev)
 	struct platform_device *smi_pdev;
 	int err;
 
-	if (!dev->pm_domain)
-		return -EPROBE_DEFER;
-
 	larb = devm_kzalloc(dev, sizeof(*larb), GFP_KERNEL);
 	if (!larb)
 		return -ENOMEM;
@@ -311,6 +309,8 @@ static int mtk_smi_larb_probe(struct platform_device *pdev)
 	smi_pdev = of_find_device_by_node(smi_node);
 	of_node_put(smi_node);
 	if (smi_pdev) {
+		if (!platform_get_drvdata(smi_pdev))
+			return -EPROBE_DEFER;
 		larb->smi_common_dev = &smi_pdev->dev;
 	} else {
 		dev_err(dev, "Failed to get the smi_common device\n");
@@ -362,9 +362,6 @@ static int mtk_smi_common_probe(struct platform_device *pdev)
 	enum mtk_smi_gen smi_gen;
 	int ret;
 
-	if (!dev->pm_domain)
-		return -EPROBE_DEFER;
-
 	common = devm_kzalloc(dev, sizeof(*common), GFP_KERNEL);
 	if (!common)
 		return -ENOMEM;
@@ -441,4 +438,4 @@ static int __init mtk_smi_init(void)
 	return ret;
 }
 
-subsys_initcall(mtk_smi_init);
+module_init(mtk_smi_init);
-- 
1.9.1