All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
@ 2020-07-21  2:16 ` Miles Chen
  0 siblings, 0 replies; 32+ messages in thread
From: Miles Chen @ 2020-07-21  2:16 UTC (permalink / raw)
  To: Joerg Roedel, Matthias Brugger, Rob Herring
  Cc: iommu, linux-arm-kernel, linux-mediatek, linux-kernel,
	wsd_upstream, Miles Chen, Mike Rapoport, David Hildenbrand,
	Yong Wu, Yingjoe Chen, Christoph Hellwig, Chao Hao

In previous discussion [1] and [2], we found that it is risky to
use max_pfn or totalram_pages to tell if 4GB mode is enabled.

Check 4GB mode by reading infracfg register, remove the usage
of the un-exported symbol max_pfn.

This is a step towards building mtk_iommu as a kernel module.

Change since v1:
1. remove the phandle usage, search for infracfg instead [3]
2. use infracfg instead of infracfg_regmap
3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
4. update enable_4GB only when has_4gb_mode

[1] https://lkml.org/lkml/2020/6/3/733
[2] https://lkml.org/lkml/2020/6/4/136
[3] https://lkml.org/lkml/2020/7/15/1147

Cc: Mike Rapoport <rppt@linux.ibm.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Yong Wu <yong.wu@mediatek.com>
Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Yong Wu <yong.wu@mediatek.com>
Cc: Chao Hao <chao.hao@mediatek.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Matthias Brugger <matthias.bgg@gmail.com>
Signed-off-by: Miles Chen <miles.chen@mediatek.com>
---
 drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
 include/linux/soc/mediatek/infracfg.h |  3 +++
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 2be96f1cdbd2..16765f532853 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -3,7 +3,6 @@
  * Copyright (c) 2015-2016 MediaTek Inc.
  * Author: Yong Wu <yong.wu@mediatek.com>
  */
-#include <linux/memblock.h>
 #include <linux/bug.h>
 #include <linux/clk.h>
 #include <linux/component.h>
@@ -15,13 +14,16 @@
 #include <linux/iommu.h>
 #include <linux/iopoll.h>
 #include <linux/list.h>
+#include <linux/mfd/syscon.h>
 #include <linux/of_address.h>
 #include <linux/of_iommu.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
+#include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
+#include <linux/soc/mediatek/infracfg.h>
 #include <asm/barrier.h>
 #include <soc/mediatek/smi.h>
 
@@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
 	struct resource         *res;
 	resource_size_t		ioaddr;
 	struct component_match  *match = NULL;
+	struct regmap		*infracfg;
 	void                    *protect;
 	int                     i, larb_nr, ret;
+	u32			val;
 
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
@@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
 
-	/* Whether the current dram is over 4GB */
-	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
-	if (!data->plat_data->has_4gb_mode)
-		data->enable_4GB = false;
+	data->enable_4GB = false;
+	if (data->plat_data->has_4gb_mode) {
+		infracfg = syscon_regmap_lookup_by_compatible(
+				"mediatek,mt8173-infracfg");
+		if (IS_ERR(infracfg)) {
+			infracfg = syscon_regmap_lookup_by_compatible(
+					"mediatek,mt2712-infracfg");
+			if (IS_ERR(infracfg))
+				return PTR_ERR(infracfg);
+
+		}
+		ret = regmap_read(infracfg, REG_INFRA_MISC, &val);
+		if (ret)
+			return ret;
+		data->enable_4GB = !!(val & F_DDR_4GB_SUPPORT_EN);
+	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	data->base = devm_ioremap_resource(dev, res);
diff --git a/include/linux/soc/mediatek/infracfg.h b/include/linux/soc/mediatek/infracfg.h
index fd25f0148566..233463d789c6 100644
--- a/include/linux/soc/mediatek/infracfg.h
+++ b/include/linux/soc/mediatek/infracfg.h
@@ -32,6 +32,9 @@
 #define MT7622_TOP_AXI_PROT_EN_WB		(BIT(2) | BIT(6) | \
 						 BIT(7) | BIT(8))
 
+#define REG_INFRA_MISC				0xf00
+#define F_DDR_4GB_SUPPORT_EN			BIT(13)
+
 int mtk_infracfg_set_bus_protection(struct regmap *infracfg, u32 mask,
 		bool reg_update);
 int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask,
-- 
2.18.0

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

* [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
@ 2020-07-21  2:16 ` Miles Chen
  0 siblings, 0 replies; 32+ messages in thread
From: Miles Chen @ 2020-07-21  2:16 UTC (permalink / raw)
  To: Joerg Roedel, Matthias Brugger, Rob Herring
  Cc: wsd_upstream, David Hildenbrand, iommu, linux-kernel, Chao Hao,
	Miles Chen, linux-mediatek, Yingjoe Chen, Mike Rapoport,
	Christoph Hellwig, linux-arm-kernel

In previous discussion [1] and [2], we found that it is risky to
use max_pfn or totalram_pages to tell if 4GB mode is enabled.

Check 4GB mode by reading infracfg register, remove the usage
of the un-exported symbol max_pfn.

This is a step towards building mtk_iommu as a kernel module.

Change since v1:
1. remove the phandle usage, search for infracfg instead [3]
2. use infracfg instead of infracfg_regmap
3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
4. update enable_4GB only when has_4gb_mode

[1] https://lkml.org/lkml/2020/6/3/733
[2] https://lkml.org/lkml/2020/6/4/136
[3] https://lkml.org/lkml/2020/7/15/1147

Cc: Mike Rapoport <rppt@linux.ibm.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Yong Wu <yong.wu@mediatek.com>
Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Yong Wu <yong.wu@mediatek.com>
Cc: Chao Hao <chao.hao@mediatek.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Matthias Brugger <matthias.bgg@gmail.com>
Signed-off-by: Miles Chen <miles.chen@mediatek.com>
---
 drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
 include/linux/soc/mediatek/infracfg.h |  3 +++
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 2be96f1cdbd2..16765f532853 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -3,7 +3,6 @@
  * Copyright (c) 2015-2016 MediaTek Inc.
  * Author: Yong Wu <yong.wu@mediatek.com>
  */
-#include <linux/memblock.h>
 #include <linux/bug.h>
 #include <linux/clk.h>
 #include <linux/component.h>
@@ -15,13 +14,16 @@
 #include <linux/iommu.h>
 #include <linux/iopoll.h>
 #include <linux/list.h>
+#include <linux/mfd/syscon.h>
 #include <linux/of_address.h>
 #include <linux/of_iommu.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
+#include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
+#include <linux/soc/mediatek/infracfg.h>
 #include <asm/barrier.h>
 #include <soc/mediatek/smi.h>
 
@@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
 	struct resource         *res;
 	resource_size_t		ioaddr;
 	struct component_match  *match = NULL;
+	struct regmap		*infracfg;
 	void                    *protect;
 	int                     i, larb_nr, ret;
+	u32			val;
 
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
@@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
 
-	/* Whether the current dram is over 4GB */
-	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
-	if (!data->plat_data->has_4gb_mode)
-		data->enable_4GB = false;
+	data->enable_4GB = false;
+	if (data->plat_data->has_4gb_mode) {
+		infracfg = syscon_regmap_lookup_by_compatible(
+				"mediatek,mt8173-infracfg");
+		if (IS_ERR(infracfg)) {
+			infracfg = syscon_regmap_lookup_by_compatible(
+					"mediatek,mt2712-infracfg");
+			if (IS_ERR(infracfg))
+				return PTR_ERR(infracfg);
+
+		}
+		ret = regmap_read(infracfg, REG_INFRA_MISC, &val);
+		if (ret)
+			return ret;
+		data->enable_4GB = !!(val & F_DDR_4GB_SUPPORT_EN);
+	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	data->base = devm_ioremap_resource(dev, res);
diff --git a/include/linux/soc/mediatek/infracfg.h b/include/linux/soc/mediatek/infracfg.h
index fd25f0148566..233463d789c6 100644
--- a/include/linux/soc/mediatek/infracfg.h
+++ b/include/linux/soc/mediatek/infracfg.h
@@ -32,6 +32,9 @@
 #define MT7622_TOP_AXI_PROT_EN_WB		(BIT(2) | BIT(6) | \
 						 BIT(7) | BIT(8))
 
+#define REG_INFRA_MISC				0xf00
+#define F_DDR_4GB_SUPPORT_EN			BIT(13)
+
 int mtk_infracfg_set_bus_protection(struct regmap *infracfg, u32 mask,
 		bool reg_update);
 int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask,
-- 
2.18.0
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
@ 2020-07-21  2:16 ` Miles Chen
  0 siblings, 0 replies; 32+ messages in thread
From: Miles Chen @ 2020-07-21  2:16 UTC (permalink / raw)
  To: Joerg Roedel, Matthias Brugger, Rob Herring
  Cc: wsd_upstream, David Hildenbrand, iommu, linux-kernel, Chao Hao,
	Miles Chen, linux-mediatek, Yong Wu, Yingjoe Chen, Mike Rapoport,
	Christoph Hellwig, linux-arm-kernel

In previous discussion [1] and [2], we found that it is risky to
use max_pfn or totalram_pages to tell if 4GB mode is enabled.

Check 4GB mode by reading infracfg register, remove the usage
of the un-exported symbol max_pfn.

This is a step towards building mtk_iommu as a kernel module.

Change since v1:
1. remove the phandle usage, search for infracfg instead [3]
2. use infracfg instead of infracfg_regmap
3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
4. update enable_4GB only when has_4gb_mode

[1] https://lkml.org/lkml/2020/6/3/733
[2] https://lkml.org/lkml/2020/6/4/136
[3] https://lkml.org/lkml/2020/7/15/1147

Cc: Mike Rapoport <rppt@linux.ibm.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Yong Wu <yong.wu@mediatek.com>
Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Yong Wu <yong.wu@mediatek.com>
Cc: Chao Hao <chao.hao@mediatek.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Matthias Brugger <matthias.bgg@gmail.com>
Signed-off-by: Miles Chen <miles.chen@mediatek.com>
---
 drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
 include/linux/soc/mediatek/infracfg.h |  3 +++
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 2be96f1cdbd2..16765f532853 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -3,7 +3,6 @@
  * Copyright (c) 2015-2016 MediaTek Inc.
  * Author: Yong Wu <yong.wu@mediatek.com>
  */
-#include <linux/memblock.h>
 #include <linux/bug.h>
 #include <linux/clk.h>
 #include <linux/component.h>
@@ -15,13 +14,16 @@
 #include <linux/iommu.h>
 #include <linux/iopoll.h>
 #include <linux/list.h>
+#include <linux/mfd/syscon.h>
 #include <linux/of_address.h>
 #include <linux/of_iommu.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
+#include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
+#include <linux/soc/mediatek/infracfg.h>
 #include <asm/barrier.h>
 #include <soc/mediatek/smi.h>
 
@@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
 	struct resource         *res;
 	resource_size_t		ioaddr;
 	struct component_match  *match = NULL;
+	struct regmap		*infracfg;
 	void                    *protect;
 	int                     i, larb_nr, ret;
+	u32			val;
 
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
@@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
 
-	/* Whether the current dram is over 4GB */
-	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
-	if (!data->plat_data->has_4gb_mode)
-		data->enable_4GB = false;
+	data->enable_4GB = false;
+	if (data->plat_data->has_4gb_mode) {
+		infracfg = syscon_regmap_lookup_by_compatible(
+				"mediatek,mt8173-infracfg");
+		if (IS_ERR(infracfg)) {
+			infracfg = syscon_regmap_lookup_by_compatible(
+					"mediatek,mt2712-infracfg");
+			if (IS_ERR(infracfg))
+				return PTR_ERR(infracfg);
+
+		}
+		ret = regmap_read(infracfg, REG_INFRA_MISC, &val);
+		if (ret)
+			return ret;
+		data->enable_4GB = !!(val & F_DDR_4GB_SUPPORT_EN);
+	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	data->base = devm_ioremap_resource(dev, res);
diff --git a/include/linux/soc/mediatek/infracfg.h b/include/linux/soc/mediatek/infracfg.h
index fd25f0148566..233463d789c6 100644
--- a/include/linux/soc/mediatek/infracfg.h
+++ b/include/linux/soc/mediatek/infracfg.h
@@ -32,6 +32,9 @@
 #define MT7622_TOP_AXI_PROT_EN_WB		(BIT(2) | BIT(6) | \
 						 BIT(7) | BIT(8))
 
+#define REG_INFRA_MISC				0xf00
+#define F_DDR_4GB_SUPPORT_EN			BIT(13)
+
 int mtk_infracfg_set_bus_protection(struct regmap *infracfg, u32 mask,
 		bool reg_update);
 int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask,
-- 
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

* [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
@ 2020-07-21  2:16 ` Miles Chen
  0 siblings, 0 replies; 32+ messages in thread
From: Miles Chen @ 2020-07-21  2:16 UTC (permalink / raw)
  To: Joerg Roedel, Matthias Brugger, Rob Herring
  Cc: wsd_upstream, David Hildenbrand, iommu, linux-kernel, Chao Hao,
	Miles Chen, linux-mediatek, Yong Wu, Yingjoe Chen, Mike Rapoport,
	Christoph Hellwig, linux-arm-kernel

In previous discussion [1] and [2], we found that it is risky to
use max_pfn or totalram_pages to tell if 4GB mode is enabled.

Check 4GB mode by reading infracfg register, remove the usage
of the un-exported symbol max_pfn.

This is a step towards building mtk_iommu as a kernel module.

Change since v1:
1. remove the phandle usage, search for infracfg instead [3]
2. use infracfg instead of infracfg_regmap
3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
4. update enable_4GB only when has_4gb_mode

[1] https://lkml.org/lkml/2020/6/3/733
[2] https://lkml.org/lkml/2020/6/4/136
[3] https://lkml.org/lkml/2020/7/15/1147

Cc: Mike Rapoport <rppt@linux.ibm.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Yong Wu <yong.wu@mediatek.com>
Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Yong Wu <yong.wu@mediatek.com>
Cc: Chao Hao <chao.hao@mediatek.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Matthias Brugger <matthias.bgg@gmail.com>
Signed-off-by: Miles Chen <miles.chen@mediatek.com>
---
 drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
 include/linux/soc/mediatek/infracfg.h |  3 +++
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 2be96f1cdbd2..16765f532853 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -3,7 +3,6 @@
  * Copyright (c) 2015-2016 MediaTek Inc.
  * Author: Yong Wu <yong.wu@mediatek.com>
  */
-#include <linux/memblock.h>
 #include <linux/bug.h>
 #include <linux/clk.h>
 #include <linux/component.h>
@@ -15,13 +14,16 @@
 #include <linux/iommu.h>
 #include <linux/iopoll.h>
 #include <linux/list.h>
+#include <linux/mfd/syscon.h>
 #include <linux/of_address.h>
 #include <linux/of_iommu.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
+#include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
+#include <linux/soc/mediatek/infracfg.h>
 #include <asm/barrier.h>
 #include <soc/mediatek/smi.h>
 
@@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
 	struct resource         *res;
 	resource_size_t		ioaddr;
 	struct component_match  *match = NULL;
+	struct regmap		*infracfg;
 	void                    *protect;
 	int                     i, larb_nr, ret;
+	u32			val;
 
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
@@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
 
-	/* Whether the current dram is over 4GB */
-	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
-	if (!data->plat_data->has_4gb_mode)
-		data->enable_4GB = false;
+	data->enable_4GB = false;
+	if (data->plat_data->has_4gb_mode) {
+		infracfg = syscon_regmap_lookup_by_compatible(
+				"mediatek,mt8173-infracfg");
+		if (IS_ERR(infracfg)) {
+			infracfg = syscon_regmap_lookup_by_compatible(
+					"mediatek,mt2712-infracfg");
+			if (IS_ERR(infracfg))
+				return PTR_ERR(infracfg);
+
+		}
+		ret = regmap_read(infracfg, REG_INFRA_MISC, &val);
+		if (ret)
+			return ret;
+		data->enable_4GB = !!(val & F_DDR_4GB_SUPPORT_EN);
+	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	data->base = devm_ioremap_resource(dev, res);
diff --git a/include/linux/soc/mediatek/infracfg.h b/include/linux/soc/mediatek/infracfg.h
index fd25f0148566..233463d789c6 100644
--- a/include/linux/soc/mediatek/infracfg.h
+++ b/include/linux/soc/mediatek/infracfg.h
@@ -32,6 +32,9 @@
 #define MT7622_TOP_AXI_PROT_EN_WB		(BIT(2) | BIT(6) | \
 						 BIT(7) | BIT(8))
 
+#define REG_INFRA_MISC				0xf00
+#define F_DDR_4GB_SUPPORT_EN			BIT(13)
+
 int mtk_infracfg_set_bus_protection(struct regmap *infracfg, u32 mask,
 		bool reg_update);
 int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask,
-- 
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
  2020-07-21  2:16 ` Miles Chen
  (?)
  (?)
@ 2020-07-21  9:10   ` David Hildenbrand
  -1 siblings, 0 replies; 32+ messages in thread
From: David Hildenbrand @ 2020-07-21  9:10 UTC (permalink / raw)
  To: Miles Chen, Joerg Roedel, Matthias Brugger, Rob Herring
  Cc: iommu, linux-arm-kernel, linux-mediatek, linux-kernel,
	wsd_upstream, Mike Rapoport, Yong Wu, Yingjoe Chen,
	Christoph Hellwig, Chao Hao

On 21.07.20 04:16, Miles Chen wrote:
> In previous discussion [1] and [2], we found that it is risky to
> use max_pfn or totalram_pages to tell if 4GB mode is enabled.
> 
> Check 4GB mode by reading infracfg register, remove the usage
> of the un-exported symbol max_pfn.
> 
> This is a step towards building mtk_iommu as a kernel module.
> 
> Change since v1:
> 1. remove the phandle usage, search for infracfg instead [3]
> 2. use infracfg instead of infracfg_regmap
> 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
> 4. update enable_4GB only when has_4gb_mode

Nit: We tend to place such information below the "---" (adding a second
one) such that this information is discarded when the patch is picked up.

> 
> [1] https://lkml.org/lkml/2020/6/3/733
> [2] https://lkml.org/lkml/2020/6/4/136
> [3] https://lkml.org/lkml/2020/7/15/1147
> 
> Cc: Mike Rapoport <rppt@linux.ibm.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Yong Wu <yong.wu@mediatek.com>
> Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Yong Wu <yong.wu@mediatek.com>
> Cc: Chao Hao <chao.hao@mediatek.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Matthias Brugger <matthias.bgg@gmail.com>
> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> ---
>  drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
>  include/linux/soc/mediatek/infracfg.h |  3 +++
>  2 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 2be96f1cdbd2..16765f532853 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -3,7 +3,6 @@
>   * Copyright (c) 2015-2016 MediaTek Inc.
>   * Author: Yong Wu <yong.wu@mediatek.com>
>   */
> -#include <linux/memblock.h>
>  #include <linux/bug.h>
>  #include <linux/clk.h>
>  #include <linux/component.h>
> @@ -15,13 +14,16 @@
>  #include <linux/iommu.h>
>  #include <linux/iopoll.h>
>  #include <linux/list.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/of_address.h>
>  #include <linux/of_iommu.h>
>  #include <linux/of_irq.h>
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
> +#include <linux/regmap.h>
>  #include <linux/slab.h>
>  #include <linux/spinlock.h>
> +#include <linux/soc/mediatek/infracfg.h>
>  #include <asm/barrier.h>
>  #include <soc/mediatek/smi.h>
>  
> @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>  	struct resource         *res;
>  	resource_size_t		ioaddr;
>  	struct component_match  *match = NULL;
> +	struct regmap		*infracfg;
>  	void                    *protect;
>  	int                     i, larb_nr, ret;
> +	u32			val;
>  
>  	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>  	if (!data)
> @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
>  
> -	/* Whether the current dram is over 4GB */
> -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
> -	if (!data->plat_data->has_4gb_mode)
> -		data->enable_4GB = false;
> +	data->enable_4GB = false;
> +	if (data->plat_data->has_4gb_mode) {
> +		infracfg = syscon_regmap_lookup_by_compatible(
> +				"mediatek,mt8173-infracfg");
> +		if (IS_ERR(infracfg)) {
> +			infracfg = syscon_regmap_lookup_by_compatible(
> +					"mediatek,mt2712-infracfg");
> +			if (IS_ERR(infracfg))
> +				return PTR_ERR(infracfg);
> +
> +		}
> +		ret = regmap_read(infracfg, REG_INFRA_MISC, &val);
> +		if (ret)
> +			return ret;
> +		data->enable_4GB = !!(val & F_DDR_4GB_SUPPORT_EN);

(I am absolutely not familiar with syscon_regmap_lookup_by ..., I am
missing some context, so sorry for the stupid questions)

Who sets the regmap value and based on what? Who decides that 4GB mode
is supported or not? And who decides if 4GB mode is *required* or not?

-- 
Thanks,

David / dhildenb


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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
@ 2020-07-21  9:10   ` David Hildenbrand
  0 siblings, 0 replies; 32+ messages in thread
From: David Hildenbrand @ 2020-07-21  9:10 UTC (permalink / raw)
  To: Miles Chen, Joerg Roedel, Matthias Brugger, Rob Herring
  Cc: wsd_upstream, linux-kernel, Mike Rapoport, Chao Hao, iommu,
	linux-mediatek, Yingjoe Chen, Christoph Hellwig,
	linux-arm-kernel

On 21.07.20 04:16, Miles Chen wrote:
> In previous discussion [1] and [2], we found that it is risky to
> use max_pfn or totalram_pages to tell if 4GB mode is enabled.
> 
> Check 4GB mode by reading infracfg register, remove the usage
> of the un-exported symbol max_pfn.
> 
> This is a step towards building mtk_iommu as a kernel module.
> 
> Change since v1:
> 1. remove the phandle usage, search for infracfg instead [3]
> 2. use infracfg instead of infracfg_regmap
> 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
> 4. update enable_4GB only when has_4gb_mode

Nit: We tend to place such information below the "---" (adding a second
one) such that this information is discarded when the patch is picked up.

> 
> [1] https://lkml.org/lkml/2020/6/3/733
> [2] https://lkml.org/lkml/2020/6/4/136
> [3] https://lkml.org/lkml/2020/7/15/1147
> 
> Cc: Mike Rapoport <rppt@linux.ibm.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Yong Wu <yong.wu@mediatek.com>
> Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Yong Wu <yong.wu@mediatek.com>
> Cc: Chao Hao <chao.hao@mediatek.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Matthias Brugger <matthias.bgg@gmail.com>
> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> ---
>  drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
>  include/linux/soc/mediatek/infracfg.h |  3 +++
>  2 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 2be96f1cdbd2..16765f532853 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -3,7 +3,6 @@
>   * Copyright (c) 2015-2016 MediaTek Inc.
>   * Author: Yong Wu <yong.wu@mediatek.com>
>   */
> -#include <linux/memblock.h>
>  #include <linux/bug.h>
>  #include <linux/clk.h>
>  #include <linux/component.h>
> @@ -15,13 +14,16 @@
>  #include <linux/iommu.h>
>  #include <linux/iopoll.h>
>  #include <linux/list.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/of_address.h>
>  #include <linux/of_iommu.h>
>  #include <linux/of_irq.h>
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
> +#include <linux/regmap.h>
>  #include <linux/slab.h>
>  #include <linux/spinlock.h>
> +#include <linux/soc/mediatek/infracfg.h>
>  #include <asm/barrier.h>
>  #include <soc/mediatek/smi.h>
>  
> @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>  	struct resource         *res;
>  	resource_size_t		ioaddr;
>  	struct component_match  *match = NULL;
> +	struct regmap		*infracfg;
>  	void                    *protect;
>  	int                     i, larb_nr, ret;
> +	u32			val;
>  
>  	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>  	if (!data)
> @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
>  
> -	/* Whether the current dram is over 4GB */
> -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
> -	if (!data->plat_data->has_4gb_mode)
> -		data->enable_4GB = false;
> +	data->enable_4GB = false;
> +	if (data->plat_data->has_4gb_mode) {
> +		infracfg = syscon_regmap_lookup_by_compatible(
> +				"mediatek,mt8173-infracfg");
> +		if (IS_ERR(infracfg)) {
> +			infracfg = syscon_regmap_lookup_by_compatible(
> +					"mediatek,mt2712-infracfg");
> +			if (IS_ERR(infracfg))
> +				return PTR_ERR(infracfg);
> +
> +		}
> +		ret = regmap_read(infracfg, REG_INFRA_MISC, &val);
> +		if (ret)
> +			return ret;
> +		data->enable_4GB = !!(val & F_DDR_4GB_SUPPORT_EN);

(I am absolutely not familiar with syscon_regmap_lookup_by ..., I am
missing some context, so sorry for the stupid questions)

Who sets the regmap value and based on what? Who decides that 4GB mode
is supported or not? And who decides if 4GB mode is *required* or not?

-- 
Thanks,

David / dhildenb

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
@ 2020-07-21  9:10   ` David Hildenbrand
  0 siblings, 0 replies; 32+ messages in thread
From: David Hildenbrand @ 2020-07-21  9:10 UTC (permalink / raw)
  To: Miles Chen, Joerg Roedel, Matthias Brugger, Rob Herring
  Cc: wsd_upstream, linux-kernel, Mike Rapoport, Chao Hao, iommu,
	linux-mediatek, Yong Wu, Yingjoe Chen, Christoph Hellwig,
	linux-arm-kernel

On 21.07.20 04:16, Miles Chen wrote:
> In previous discussion [1] and [2], we found that it is risky to
> use max_pfn or totalram_pages to tell if 4GB mode is enabled.
> 
> Check 4GB mode by reading infracfg register, remove the usage
> of the un-exported symbol max_pfn.
> 
> This is a step towards building mtk_iommu as a kernel module.
> 
> Change since v1:
> 1. remove the phandle usage, search for infracfg instead [3]
> 2. use infracfg instead of infracfg_regmap
> 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
> 4. update enable_4GB only when has_4gb_mode

Nit: We tend to place such information below the "---" (adding a second
one) such that this information is discarded when the patch is picked up.

> 
> [1] https://lkml.org/lkml/2020/6/3/733
> [2] https://lkml.org/lkml/2020/6/4/136
> [3] https://lkml.org/lkml/2020/7/15/1147
> 
> Cc: Mike Rapoport <rppt@linux.ibm.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Yong Wu <yong.wu@mediatek.com>
> Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Yong Wu <yong.wu@mediatek.com>
> Cc: Chao Hao <chao.hao@mediatek.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Matthias Brugger <matthias.bgg@gmail.com>
> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> ---
>  drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
>  include/linux/soc/mediatek/infracfg.h |  3 +++
>  2 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 2be96f1cdbd2..16765f532853 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -3,7 +3,6 @@
>   * Copyright (c) 2015-2016 MediaTek Inc.
>   * Author: Yong Wu <yong.wu@mediatek.com>
>   */
> -#include <linux/memblock.h>
>  #include <linux/bug.h>
>  #include <linux/clk.h>
>  #include <linux/component.h>
> @@ -15,13 +14,16 @@
>  #include <linux/iommu.h>
>  #include <linux/iopoll.h>
>  #include <linux/list.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/of_address.h>
>  #include <linux/of_iommu.h>
>  #include <linux/of_irq.h>
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
> +#include <linux/regmap.h>
>  #include <linux/slab.h>
>  #include <linux/spinlock.h>
> +#include <linux/soc/mediatek/infracfg.h>
>  #include <asm/barrier.h>
>  #include <soc/mediatek/smi.h>
>  
> @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>  	struct resource         *res;
>  	resource_size_t		ioaddr;
>  	struct component_match  *match = NULL;
> +	struct regmap		*infracfg;
>  	void                    *protect;
>  	int                     i, larb_nr, ret;
> +	u32			val;
>  
>  	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>  	if (!data)
> @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
>  
> -	/* Whether the current dram is over 4GB */
> -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
> -	if (!data->plat_data->has_4gb_mode)
> -		data->enable_4GB = false;
> +	data->enable_4GB = false;
> +	if (data->plat_data->has_4gb_mode) {
> +		infracfg = syscon_regmap_lookup_by_compatible(
> +				"mediatek,mt8173-infracfg");
> +		if (IS_ERR(infracfg)) {
> +			infracfg = syscon_regmap_lookup_by_compatible(
> +					"mediatek,mt2712-infracfg");
> +			if (IS_ERR(infracfg))
> +				return PTR_ERR(infracfg);
> +
> +		}
> +		ret = regmap_read(infracfg, REG_INFRA_MISC, &val);
> +		if (ret)
> +			return ret;
> +		data->enable_4GB = !!(val & F_DDR_4GB_SUPPORT_EN);

(I am absolutely not familiar with syscon_regmap_lookup_by ..., I am
missing some context, so sorry for the stupid questions)

Who sets the regmap value and based on what? Who decides that 4GB mode
is supported or not? And who decides if 4GB mode is *required* or not?

-- 
Thanks,

David / dhildenb


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

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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
@ 2020-07-21  9:10   ` David Hildenbrand
  0 siblings, 0 replies; 32+ messages in thread
From: David Hildenbrand @ 2020-07-21  9:10 UTC (permalink / raw)
  To: Miles Chen, Joerg Roedel, Matthias Brugger, Rob Herring
  Cc: wsd_upstream, linux-kernel, Mike Rapoport, Chao Hao, iommu,
	linux-mediatek, Yong Wu, Yingjoe Chen, Christoph Hellwig,
	linux-arm-kernel

On 21.07.20 04:16, Miles Chen wrote:
> In previous discussion [1] and [2], we found that it is risky to
> use max_pfn or totalram_pages to tell if 4GB mode is enabled.
> 
> Check 4GB mode by reading infracfg register, remove the usage
> of the un-exported symbol max_pfn.
> 
> This is a step towards building mtk_iommu as a kernel module.
> 
> Change since v1:
> 1. remove the phandle usage, search for infracfg instead [3]
> 2. use infracfg instead of infracfg_regmap
> 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
> 4. update enable_4GB only when has_4gb_mode

Nit: We tend to place such information below the "---" (adding a second
one) such that this information is discarded when the patch is picked up.

> 
> [1] https://lkml.org/lkml/2020/6/3/733
> [2] https://lkml.org/lkml/2020/6/4/136
> [3] https://lkml.org/lkml/2020/7/15/1147
> 
> Cc: Mike Rapoport <rppt@linux.ibm.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Yong Wu <yong.wu@mediatek.com>
> Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Yong Wu <yong.wu@mediatek.com>
> Cc: Chao Hao <chao.hao@mediatek.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Matthias Brugger <matthias.bgg@gmail.com>
> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> ---
>  drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
>  include/linux/soc/mediatek/infracfg.h |  3 +++
>  2 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 2be96f1cdbd2..16765f532853 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -3,7 +3,6 @@
>   * Copyright (c) 2015-2016 MediaTek Inc.
>   * Author: Yong Wu <yong.wu@mediatek.com>
>   */
> -#include <linux/memblock.h>
>  #include <linux/bug.h>
>  #include <linux/clk.h>
>  #include <linux/component.h>
> @@ -15,13 +14,16 @@
>  #include <linux/iommu.h>
>  #include <linux/iopoll.h>
>  #include <linux/list.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/of_address.h>
>  #include <linux/of_iommu.h>
>  #include <linux/of_irq.h>
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
> +#include <linux/regmap.h>
>  #include <linux/slab.h>
>  #include <linux/spinlock.h>
> +#include <linux/soc/mediatek/infracfg.h>
>  #include <asm/barrier.h>
>  #include <soc/mediatek/smi.h>
>  
> @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>  	struct resource         *res;
>  	resource_size_t		ioaddr;
>  	struct component_match  *match = NULL;
> +	struct regmap		*infracfg;
>  	void                    *protect;
>  	int                     i, larb_nr, ret;
> +	u32			val;
>  
>  	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>  	if (!data)
> @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
>  
> -	/* Whether the current dram is over 4GB */
> -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
> -	if (!data->plat_data->has_4gb_mode)
> -		data->enable_4GB = false;
> +	data->enable_4GB = false;
> +	if (data->plat_data->has_4gb_mode) {
> +		infracfg = syscon_regmap_lookup_by_compatible(
> +				"mediatek,mt8173-infracfg");
> +		if (IS_ERR(infracfg)) {
> +			infracfg = syscon_regmap_lookup_by_compatible(
> +					"mediatek,mt2712-infracfg");
> +			if (IS_ERR(infracfg))
> +				return PTR_ERR(infracfg);
> +
> +		}
> +		ret = regmap_read(infracfg, REG_INFRA_MISC, &val);
> +		if (ret)
> +			return ret;
> +		data->enable_4GB = !!(val & F_DDR_4GB_SUPPORT_EN);

(I am absolutely not familiar with syscon_regmap_lookup_by ..., I am
missing some context, so sorry for the stupid questions)

Who sets the regmap value and based on what? Who decides that 4GB mode
is supported or not? And who decides if 4GB mode is *required* or not?

-- 
Thanks,

David / dhildenb


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
  2020-07-21  2:16 ` Miles Chen
  (?)
  (?)
@ 2020-07-21  9:40   ` Matthias Brugger
  -1 siblings, 0 replies; 32+ messages in thread
From: Matthias Brugger @ 2020-07-21  9:40 UTC (permalink / raw)
  To: Miles Chen, Joerg Roedel, Rob Herring
  Cc: iommu, linux-arm-kernel, linux-mediatek, linux-kernel,
	wsd_upstream, Mike Rapoport, David Hildenbrand, Yong Wu,
	Yingjoe Chen, Christoph Hellwig, Chao Hao



On 21/07/2020 04:16, Miles Chen wrote:
> In previous discussion [1] and [2], we found that it is risky to
> use max_pfn or totalram_pages to tell if 4GB mode is enabled.
> 
> Check 4GB mode by reading infracfg register, remove the usage
> of the un-exported symbol max_pfn.
> 
> This is a step towards building mtk_iommu as a kernel module.
> 
> Change since v1:
> 1. remove the phandle usage, search for infracfg instead [3]
> 2. use infracfg instead of infracfg_regmap
> 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
> 4. update enable_4GB only when has_4gb_mode
> 
> [1] https://lkml.org/lkml/2020/6/3/733
> [2] https://lkml.org/lkml/2020/6/4/136
> [3] https://lkml.org/lkml/2020/7/15/1147
> 
> Cc: Mike Rapoport <rppt@linux.ibm.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Yong Wu <yong.wu@mediatek.com>
> Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Yong Wu <yong.wu@mediatek.com>
> Cc: Chao Hao <chao.hao@mediatek.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Matthias Brugger <matthias.bgg@gmail.com>
> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> ---
>   drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
>   include/linux/soc/mediatek/infracfg.h |  3 +++
>   2 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 2be96f1cdbd2..16765f532853 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -3,7 +3,6 @@
>    * Copyright (c) 2015-2016 MediaTek Inc.
>    * Author: Yong Wu <yong.wu@mediatek.com>
>    */
> -#include <linux/memblock.h>
>   #include <linux/bug.h>
>   #include <linux/clk.h>
>   #include <linux/component.h>
> @@ -15,13 +14,16 @@
>   #include <linux/iommu.h>
>   #include <linux/iopoll.h>
>   #include <linux/list.h>
> +#include <linux/mfd/syscon.h>
>   #include <linux/of_address.h>
>   #include <linux/of_iommu.h>
>   #include <linux/of_irq.h>
>   #include <linux/of_platform.h>
>   #include <linux/platform_device.h>
> +#include <linux/regmap.h>
>   #include <linux/slab.h>
>   #include <linux/spinlock.h>
> +#include <linux/soc/mediatek/infracfg.h>
>   #include <asm/barrier.h>
>   #include <soc/mediatek/smi.h>
>   
> @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>   	struct resource         *res;
>   	resource_size_t		ioaddr;
>   	struct component_match  *match = NULL;
> +	struct regmap		*infracfg;
>   	void                    *protect;
>   	int                     i, larb_nr, ret;
> +	u32			val;
>   
>   	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>   	if (!data)
> @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>   		return -ENOMEM;
>   	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
>   
> -	/* Whether the current dram is over 4GB */
> -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
> -	if (!data->plat_data->has_4gb_mode)
> -		data->enable_4GB = false;
> +	data->enable_4GB = false;
> +	if (data->plat_data->has_4gb_mode) {
> +		infracfg = syscon_regmap_lookup_by_compatible(
> +				"mediatek,mt8173-infracfg");
> +		if (IS_ERR(infracfg)) {
> +			infracfg = syscon_regmap_lookup_by_compatible(
> +					"mediatek,mt2712-infracfg");
> +			if (IS_ERR(infracfg))
> +				return PTR_ERR(infracfg);

I think we should check m4u_plat instead to decide which compatible we have to 
look for.
Another option would be to add a general compatible something like 
"mtk-infracfg" and search for that. That would need an update of all DTS having 
a infracfg compatible right now. After thinking twice, this would break newer 
kernel with older device tree, so maybe it's better to go with m4u_plat switch 
statement.

Regards,
Matthias

> +
> +		}
> +		ret = regmap_read(infracfg, REG_INFRA_MISC, &val);
> +		if (ret)
> +			return ret;
> +		data->enable_4GB = !!(val & F_DDR_4GB_SUPPORT_EN);
> +	}
>   
>   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   	data->base = devm_ioremap_resource(dev, res);
> diff --git a/include/linux/soc/mediatek/infracfg.h b/include/linux/soc/mediatek/infracfg.h
> index fd25f0148566..233463d789c6 100644
> --- a/include/linux/soc/mediatek/infracfg.h
> +++ b/include/linux/soc/mediatek/infracfg.h
> @@ -32,6 +32,9 @@
>   #define MT7622_TOP_AXI_PROT_EN_WB		(BIT(2) | BIT(6) | \
>   						 BIT(7) | BIT(8))
>   
> +#define REG_INFRA_MISC				0xf00
> +#define F_DDR_4GB_SUPPORT_EN			BIT(13)
> +
>   int mtk_infracfg_set_bus_protection(struct regmap *infracfg, u32 mask,
>   		bool reg_update);
>   int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask,
> 

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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
@ 2020-07-21  9:40   ` Matthias Brugger
  0 siblings, 0 replies; 32+ messages in thread
From: Matthias Brugger @ 2020-07-21  9:40 UTC (permalink / raw)
  To: Miles Chen, Joerg Roedel, Rob Herring
  Cc: wsd_upstream, David Hildenbrand, linux-kernel, Mike Rapoport,
	Chao Hao, iommu, linux-mediatek, Yingjoe Chen, Christoph Hellwig,
	linux-arm-kernel



On 21/07/2020 04:16, Miles Chen wrote:
> In previous discussion [1] and [2], we found that it is risky to
> use max_pfn or totalram_pages to tell if 4GB mode is enabled.
> 
> Check 4GB mode by reading infracfg register, remove the usage
> of the un-exported symbol max_pfn.
> 
> This is a step towards building mtk_iommu as a kernel module.
> 
> Change since v1:
> 1. remove the phandle usage, search for infracfg instead [3]
> 2. use infracfg instead of infracfg_regmap
> 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
> 4. update enable_4GB only when has_4gb_mode
> 
> [1] https://lkml.org/lkml/2020/6/3/733
> [2] https://lkml.org/lkml/2020/6/4/136
> [3] https://lkml.org/lkml/2020/7/15/1147
> 
> Cc: Mike Rapoport <rppt@linux.ibm.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Yong Wu <yong.wu@mediatek.com>
> Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Yong Wu <yong.wu@mediatek.com>
> Cc: Chao Hao <chao.hao@mediatek.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Matthias Brugger <matthias.bgg@gmail.com>
> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> ---
>   drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
>   include/linux/soc/mediatek/infracfg.h |  3 +++
>   2 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 2be96f1cdbd2..16765f532853 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -3,7 +3,6 @@
>    * Copyright (c) 2015-2016 MediaTek Inc.
>    * Author: Yong Wu <yong.wu@mediatek.com>
>    */
> -#include <linux/memblock.h>
>   #include <linux/bug.h>
>   #include <linux/clk.h>
>   #include <linux/component.h>
> @@ -15,13 +14,16 @@
>   #include <linux/iommu.h>
>   #include <linux/iopoll.h>
>   #include <linux/list.h>
> +#include <linux/mfd/syscon.h>
>   #include <linux/of_address.h>
>   #include <linux/of_iommu.h>
>   #include <linux/of_irq.h>
>   #include <linux/of_platform.h>
>   #include <linux/platform_device.h>
> +#include <linux/regmap.h>
>   #include <linux/slab.h>
>   #include <linux/spinlock.h>
> +#include <linux/soc/mediatek/infracfg.h>
>   #include <asm/barrier.h>
>   #include <soc/mediatek/smi.h>
>   
> @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>   	struct resource         *res;
>   	resource_size_t		ioaddr;
>   	struct component_match  *match = NULL;
> +	struct regmap		*infracfg;
>   	void                    *protect;
>   	int                     i, larb_nr, ret;
> +	u32			val;
>   
>   	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>   	if (!data)
> @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>   		return -ENOMEM;
>   	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
>   
> -	/* Whether the current dram is over 4GB */
> -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
> -	if (!data->plat_data->has_4gb_mode)
> -		data->enable_4GB = false;
> +	data->enable_4GB = false;
> +	if (data->plat_data->has_4gb_mode) {
> +		infracfg = syscon_regmap_lookup_by_compatible(
> +				"mediatek,mt8173-infracfg");
> +		if (IS_ERR(infracfg)) {
> +			infracfg = syscon_regmap_lookup_by_compatible(
> +					"mediatek,mt2712-infracfg");
> +			if (IS_ERR(infracfg))
> +				return PTR_ERR(infracfg);

I think we should check m4u_plat instead to decide which compatible we have to 
look for.
Another option would be to add a general compatible something like 
"mtk-infracfg" and search for that. That would need an update of all DTS having 
a infracfg compatible right now. After thinking twice, this would break newer 
kernel with older device tree, so maybe it's better to go with m4u_plat switch 
statement.

Regards,
Matthias

> +
> +		}
> +		ret = regmap_read(infracfg, REG_INFRA_MISC, &val);
> +		if (ret)
> +			return ret;
> +		data->enable_4GB = !!(val & F_DDR_4GB_SUPPORT_EN);
> +	}
>   
>   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   	data->base = devm_ioremap_resource(dev, res);
> diff --git a/include/linux/soc/mediatek/infracfg.h b/include/linux/soc/mediatek/infracfg.h
> index fd25f0148566..233463d789c6 100644
> --- a/include/linux/soc/mediatek/infracfg.h
> +++ b/include/linux/soc/mediatek/infracfg.h
> @@ -32,6 +32,9 @@
>   #define MT7622_TOP_AXI_PROT_EN_WB		(BIT(2) | BIT(6) | \
>   						 BIT(7) | BIT(8))
>   
> +#define REG_INFRA_MISC				0xf00
> +#define F_DDR_4GB_SUPPORT_EN			BIT(13)
> +
>   int mtk_infracfg_set_bus_protection(struct regmap *infracfg, u32 mask,
>   		bool reg_update);
>   int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask,
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
@ 2020-07-21  9:40   ` Matthias Brugger
  0 siblings, 0 replies; 32+ messages in thread
From: Matthias Brugger @ 2020-07-21  9:40 UTC (permalink / raw)
  To: Miles Chen, Joerg Roedel, Rob Herring
  Cc: wsd_upstream, David Hildenbrand, linux-kernel, Mike Rapoport,
	Chao Hao, iommu, linux-mediatek, Yong Wu, Yingjoe Chen,
	Christoph Hellwig, linux-arm-kernel



On 21/07/2020 04:16, Miles Chen wrote:
> In previous discussion [1] and [2], we found that it is risky to
> use max_pfn or totalram_pages to tell if 4GB mode is enabled.
> 
> Check 4GB mode by reading infracfg register, remove the usage
> of the un-exported symbol max_pfn.
> 
> This is a step towards building mtk_iommu as a kernel module.
> 
> Change since v1:
> 1. remove the phandle usage, search for infracfg instead [3]
> 2. use infracfg instead of infracfg_regmap
> 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
> 4. update enable_4GB only when has_4gb_mode
> 
> [1] https://lkml.org/lkml/2020/6/3/733
> [2] https://lkml.org/lkml/2020/6/4/136
> [3] https://lkml.org/lkml/2020/7/15/1147
> 
> Cc: Mike Rapoport <rppt@linux.ibm.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Yong Wu <yong.wu@mediatek.com>
> Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Yong Wu <yong.wu@mediatek.com>
> Cc: Chao Hao <chao.hao@mediatek.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Matthias Brugger <matthias.bgg@gmail.com>
> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> ---
>   drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
>   include/linux/soc/mediatek/infracfg.h |  3 +++
>   2 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 2be96f1cdbd2..16765f532853 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -3,7 +3,6 @@
>    * Copyright (c) 2015-2016 MediaTek Inc.
>    * Author: Yong Wu <yong.wu@mediatek.com>
>    */
> -#include <linux/memblock.h>
>   #include <linux/bug.h>
>   #include <linux/clk.h>
>   #include <linux/component.h>
> @@ -15,13 +14,16 @@
>   #include <linux/iommu.h>
>   #include <linux/iopoll.h>
>   #include <linux/list.h>
> +#include <linux/mfd/syscon.h>
>   #include <linux/of_address.h>
>   #include <linux/of_iommu.h>
>   #include <linux/of_irq.h>
>   #include <linux/of_platform.h>
>   #include <linux/platform_device.h>
> +#include <linux/regmap.h>
>   #include <linux/slab.h>
>   #include <linux/spinlock.h>
> +#include <linux/soc/mediatek/infracfg.h>
>   #include <asm/barrier.h>
>   #include <soc/mediatek/smi.h>
>   
> @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>   	struct resource         *res;
>   	resource_size_t		ioaddr;
>   	struct component_match  *match = NULL;
> +	struct regmap		*infracfg;
>   	void                    *protect;
>   	int                     i, larb_nr, ret;
> +	u32			val;
>   
>   	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>   	if (!data)
> @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>   		return -ENOMEM;
>   	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
>   
> -	/* Whether the current dram is over 4GB */
> -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
> -	if (!data->plat_data->has_4gb_mode)
> -		data->enable_4GB = false;
> +	data->enable_4GB = false;
> +	if (data->plat_data->has_4gb_mode) {
> +		infracfg = syscon_regmap_lookup_by_compatible(
> +				"mediatek,mt8173-infracfg");
> +		if (IS_ERR(infracfg)) {
> +			infracfg = syscon_regmap_lookup_by_compatible(
> +					"mediatek,mt2712-infracfg");
> +			if (IS_ERR(infracfg))
> +				return PTR_ERR(infracfg);

I think we should check m4u_plat instead to decide which compatible we have to 
look for.
Another option would be to add a general compatible something like 
"mtk-infracfg" and search for that. That would need an update of all DTS having 
a infracfg compatible right now. After thinking twice, this would break newer 
kernel with older device tree, so maybe it's better to go with m4u_plat switch 
statement.

Regards,
Matthias

> +
> +		}
> +		ret = regmap_read(infracfg, REG_INFRA_MISC, &val);
> +		if (ret)
> +			return ret;
> +		data->enable_4GB = !!(val & F_DDR_4GB_SUPPORT_EN);
> +	}
>   
>   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   	data->base = devm_ioremap_resource(dev, res);
> diff --git a/include/linux/soc/mediatek/infracfg.h b/include/linux/soc/mediatek/infracfg.h
> index fd25f0148566..233463d789c6 100644
> --- a/include/linux/soc/mediatek/infracfg.h
> +++ b/include/linux/soc/mediatek/infracfg.h
> @@ -32,6 +32,9 @@
>   #define MT7622_TOP_AXI_PROT_EN_WB		(BIT(2) | BIT(6) | \
>   						 BIT(7) | BIT(8))
>   
> +#define REG_INFRA_MISC				0xf00
> +#define F_DDR_4GB_SUPPORT_EN			BIT(13)
> +
>   int mtk_infracfg_set_bus_protection(struct regmap *infracfg, u32 mask,
>   		bool reg_update);
>   int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask,
> 

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

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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
@ 2020-07-21  9:40   ` Matthias Brugger
  0 siblings, 0 replies; 32+ messages in thread
From: Matthias Brugger @ 2020-07-21  9:40 UTC (permalink / raw)
  To: Miles Chen, Joerg Roedel, Rob Herring
  Cc: wsd_upstream, David Hildenbrand, linux-kernel, Mike Rapoport,
	Chao Hao, iommu, linux-mediatek, Yong Wu, Yingjoe Chen,
	Christoph Hellwig, linux-arm-kernel



On 21/07/2020 04:16, Miles Chen wrote:
> In previous discussion [1] and [2], we found that it is risky to
> use max_pfn or totalram_pages to tell if 4GB mode is enabled.
> 
> Check 4GB mode by reading infracfg register, remove the usage
> of the un-exported symbol max_pfn.
> 
> This is a step towards building mtk_iommu as a kernel module.
> 
> Change since v1:
> 1. remove the phandle usage, search for infracfg instead [3]
> 2. use infracfg instead of infracfg_regmap
> 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
> 4. update enable_4GB only when has_4gb_mode
> 
> [1] https://lkml.org/lkml/2020/6/3/733
> [2] https://lkml.org/lkml/2020/6/4/136
> [3] https://lkml.org/lkml/2020/7/15/1147
> 
> Cc: Mike Rapoport <rppt@linux.ibm.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Yong Wu <yong.wu@mediatek.com>
> Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Yong Wu <yong.wu@mediatek.com>
> Cc: Chao Hao <chao.hao@mediatek.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Matthias Brugger <matthias.bgg@gmail.com>
> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> ---
>   drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
>   include/linux/soc/mediatek/infracfg.h |  3 +++
>   2 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 2be96f1cdbd2..16765f532853 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -3,7 +3,6 @@
>    * Copyright (c) 2015-2016 MediaTek Inc.
>    * Author: Yong Wu <yong.wu@mediatek.com>
>    */
> -#include <linux/memblock.h>
>   #include <linux/bug.h>
>   #include <linux/clk.h>
>   #include <linux/component.h>
> @@ -15,13 +14,16 @@
>   #include <linux/iommu.h>
>   #include <linux/iopoll.h>
>   #include <linux/list.h>
> +#include <linux/mfd/syscon.h>
>   #include <linux/of_address.h>
>   #include <linux/of_iommu.h>
>   #include <linux/of_irq.h>
>   #include <linux/of_platform.h>
>   #include <linux/platform_device.h>
> +#include <linux/regmap.h>
>   #include <linux/slab.h>
>   #include <linux/spinlock.h>
> +#include <linux/soc/mediatek/infracfg.h>
>   #include <asm/barrier.h>
>   #include <soc/mediatek/smi.h>
>   
> @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>   	struct resource         *res;
>   	resource_size_t		ioaddr;
>   	struct component_match  *match = NULL;
> +	struct regmap		*infracfg;
>   	void                    *protect;
>   	int                     i, larb_nr, ret;
> +	u32			val;
>   
>   	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>   	if (!data)
> @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>   		return -ENOMEM;
>   	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
>   
> -	/* Whether the current dram is over 4GB */
> -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
> -	if (!data->plat_data->has_4gb_mode)
> -		data->enable_4GB = false;
> +	data->enable_4GB = false;
> +	if (data->plat_data->has_4gb_mode) {
> +		infracfg = syscon_regmap_lookup_by_compatible(
> +				"mediatek,mt8173-infracfg");
> +		if (IS_ERR(infracfg)) {
> +			infracfg = syscon_regmap_lookup_by_compatible(
> +					"mediatek,mt2712-infracfg");
> +			if (IS_ERR(infracfg))
> +				return PTR_ERR(infracfg);

I think we should check m4u_plat instead to decide which compatible we have to 
look for.
Another option would be to add a general compatible something like 
"mtk-infracfg" and search for that. That would need an update of all DTS having 
a infracfg compatible right now. After thinking twice, this would break newer 
kernel with older device tree, so maybe it's better to go with m4u_plat switch 
statement.

Regards,
Matthias

> +
> +		}
> +		ret = regmap_read(infracfg, REG_INFRA_MISC, &val);
> +		if (ret)
> +			return ret;
> +		data->enable_4GB = !!(val & F_DDR_4GB_SUPPORT_EN);
> +	}
>   
>   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   	data->base = devm_ioremap_resource(dev, res);
> diff --git a/include/linux/soc/mediatek/infracfg.h b/include/linux/soc/mediatek/infracfg.h
> index fd25f0148566..233463d789c6 100644
> --- a/include/linux/soc/mediatek/infracfg.h
> +++ b/include/linux/soc/mediatek/infracfg.h
> @@ -32,6 +32,9 @@
>   #define MT7622_TOP_AXI_PROT_EN_WB		(BIT(2) | BIT(6) | \
>   						 BIT(7) | BIT(8))
>   
> +#define REG_INFRA_MISC				0xf00
> +#define F_DDR_4GB_SUPPORT_EN			BIT(13)
> +
>   int mtk_infracfg_set_bus_protection(struct regmap *infracfg, u32 mask,
>   		bool reg_update);
>   int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask,
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
  2020-07-21  9:40   ` Matthias Brugger
  (?)
  (?)
@ 2020-07-21 11:24     ` Yong Wu
  -1 siblings, 0 replies; 32+ messages in thread
From: Yong Wu @ 2020-07-21 11:24 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Miles Chen, Joerg Roedel, Rob Herring, iommu, linux-arm-kernel,
	linux-mediatek, linux-kernel, wsd_upstream, Mike Rapoport,
	David Hildenbrand, Yingjoe Chen, Christoph Hellwig, Chao Hao

On Tue, 2020-07-21 at 11:40 +0200, Matthias Brugger wrote:
> 
> On 21/07/2020 04:16, Miles Chen wrote:
> > In previous discussion [1] and [2], we found that it is risky to
> > use max_pfn or totalram_pages to tell if 4GB mode is enabled.
> > 
> > Check 4GB mode by reading infracfg register, remove the usage
> > of the un-exported symbol max_pfn.
> > 
> > This is a step towards building mtk_iommu as a kernel module.
> > 
> > Change since v1:
> > 1. remove the phandle usage, search for infracfg instead [3]
> > 2. use infracfg instead of infracfg_regmap
> > 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
> > 4. update enable_4GB only when has_4gb_mode
> > 
> > [1] https://lkml.org/lkml/2020/6/3/733
> > [2] https://lkml.org/lkml/2020/6/4/136
> > [3] https://lkml.org/lkml/2020/7/15/1147
> > 
> > Cc: Mike Rapoport <rppt@linux.ibm.com>
> > Cc: David Hildenbrand <david@redhat.com>
> > Cc: Yong Wu <yong.wu@mediatek.com>
> > Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Yong Wu <yong.wu@mediatek.com>
> > Cc: Chao Hao <chao.hao@mediatek.com>
> > Cc: Rob Herring <robh@kernel.org>
> > Cc: Matthias Brugger <matthias.bgg@gmail.com>
> > Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> > ---
> >   drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
> >   include/linux/soc/mediatek/infracfg.h |  3 +++
> >   2 files changed, 24 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > index 2be96f1cdbd2..16765f532853 100644
> > --- a/drivers/iommu/mtk_iommu.c
> > +++ b/drivers/iommu/mtk_iommu.c
> > @@ -3,7 +3,6 @@
> >    * Copyright (c) 2015-2016 MediaTek Inc.
> >    * Author: Yong Wu <yong.wu@mediatek.com>
> >    */
> > -#include <linux/memblock.h>
> >   #include <linux/bug.h>
> >   #include <linux/clk.h>
> >   #include <linux/component.h>
> > @@ -15,13 +14,16 @@
> >   #include <linux/iommu.h>
> >   #include <linux/iopoll.h>
> >   #include <linux/list.h>
> > +#include <linux/mfd/syscon.h>
> >   #include <linux/of_address.h>
> >   #include <linux/of_iommu.h>
> >   #include <linux/of_irq.h>
> >   #include <linux/of_platform.h>
> >   #include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> >   #include <linux/slab.h>
> >   #include <linux/spinlock.h>
> > +#include <linux/soc/mediatek/infracfg.h>
> >   #include <asm/barrier.h>
> >   #include <soc/mediatek/smi.h>
> >   
> > @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >   	struct resource         *res;
> >   	resource_size_t		ioaddr;
> >   	struct component_match  *match = NULL;
> > +	struct regmap		*infracfg;
> >   	void                    *protect;
> >   	int                     i, larb_nr, ret;
> > +	u32			val;
> >   
> >   	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> >   	if (!data)
> > @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >   		return -ENOMEM;
> >   	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
> >   
> > -	/* Whether the current dram is over 4GB */
> > -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
> > -	if (!data->plat_data->has_4gb_mode)
> > -		data->enable_4GB = false;
> > +	data->enable_4GB = false;
> > +	if (data->plat_data->has_4gb_mode) {
> > +		infracfg = syscon_regmap_lookup_by_compatible(
> > +				"mediatek,mt8173-infracfg");
> > +		if (IS_ERR(infracfg)) {
> > +			infracfg = syscon_regmap_lookup_by_compatible(
> > +					"mediatek,mt2712-infracfg");
> > +			if (IS_ERR(infracfg))
> > +				return PTR_ERR(infracfg);
> 
> I think we should check m4u_plat instead to decide which compatible we have to 
> look for.
> Another option would be to add a general compatible something like 
> "mtk-infracfg" and search for that. That would need an update of all DTS having 
> a infracfg compatible right now. After thinking twice, this would break newer 
> kernel with older device tree, so maybe it's better to go with m4u_plat switch 
> statement.

Add a "char *infracfg" in the plat_data, Use the mt2712, mt8173
corresponding string in it. If it is NULL, It means the "enable_4GB"
always is false. Then we also can remove the flag "has_4gb_mode".

is this OK?

> 
> Regards,
> Matthias
> 
> > +
> > +		}
> > +		ret = regmap_read(infracfg, REG_INFRA_MISC, &val);
> > +		if (ret)
> > +			return ret;
> > +		data->enable_4GB = !!(val & F_DDR_4GB_SUPPORT_EN);
> > +	}
> >   
> >   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >   	data->base = devm_ioremap_resource(dev, res);
> > diff --git a/include/linux/soc/mediatek/infracfg.h b/include/linux/soc/mediatek/infracfg.h
> > index fd25f0148566..233463d789c6 100644
> > --- a/include/linux/soc/mediatek/infracfg.h
> > +++ b/include/linux/soc/mediatek/infracfg.h
> > @@ -32,6 +32,9 @@
> >   #define MT7622_TOP_AXI_PROT_EN_WB		(BIT(2) | BIT(6) | \
> >   						 BIT(7) | BIT(8))
> >   
> > +#define REG_INFRA_MISC				0xf00
> > +#define F_DDR_4GB_SUPPORT_EN			BIT(13)
> > +
> >   int mtk_infracfg_set_bus_protection(struct regmap *infracfg, u32 mask,
> >   		bool reg_update);
> >   int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask,
> > 


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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
@ 2020-07-21 11:24     ` Yong Wu
  0 siblings, 0 replies; 32+ messages in thread
From: Yong Wu @ 2020-07-21 11:24 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Rob Herring, wsd_upstream, David Hildenbrand, iommu,
	linux-kernel, Chao Hao, Miles Chen, linux-mediatek, Yingjoe Chen,
	Mike Rapoport, Christoph Hellwig, linux-arm-kernel

On Tue, 2020-07-21 at 11:40 +0200, Matthias Brugger wrote:
> 
> On 21/07/2020 04:16, Miles Chen wrote:
> > In previous discussion [1] and [2], we found that it is risky to
> > use max_pfn or totalram_pages to tell if 4GB mode is enabled.
> > 
> > Check 4GB mode by reading infracfg register, remove the usage
> > of the un-exported symbol max_pfn.
> > 
> > This is a step towards building mtk_iommu as a kernel module.
> > 
> > Change since v1:
> > 1. remove the phandle usage, search for infracfg instead [3]
> > 2. use infracfg instead of infracfg_regmap
> > 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
> > 4. update enable_4GB only when has_4gb_mode
> > 
> > [1] https://lkml.org/lkml/2020/6/3/733
> > [2] https://lkml.org/lkml/2020/6/4/136
> > [3] https://lkml.org/lkml/2020/7/15/1147
> > 
> > Cc: Mike Rapoport <rppt@linux.ibm.com>
> > Cc: David Hildenbrand <david@redhat.com>
> > Cc: Yong Wu <yong.wu@mediatek.com>
> > Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Yong Wu <yong.wu@mediatek.com>
> > Cc: Chao Hao <chao.hao@mediatek.com>
> > Cc: Rob Herring <robh@kernel.org>
> > Cc: Matthias Brugger <matthias.bgg@gmail.com>
> > Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> > ---
> >   drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
> >   include/linux/soc/mediatek/infracfg.h |  3 +++
> >   2 files changed, 24 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > index 2be96f1cdbd2..16765f532853 100644
> > --- a/drivers/iommu/mtk_iommu.c
> > +++ b/drivers/iommu/mtk_iommu.c
> > @@ -3,7 +3,6 @@
> >    * Copyright (c) 2015-2016 MediaTek Inc.
> >    * Author: Yong Wu <yong.wu@mediatek.com>
> >    */
> > -#include <linux/memblock.h>
> >   #include <linux/bug.h>
> >   #include <linux/clk.h>
> >   #include <linux/component.h>
> > @@ -15,13 +14,16 @@
> >   #include <linux/iommu.h>
> >   #include <linux/iopoll.h>
> >   #include <linux/list.h>
> > +#include <linux/mfd/syscon.h>
> >   #include <linux/of_address.h>
> >   #include <linux/of_iommu.h>
> >   #include <linux/of_irq.h>
> >   #include <linux/of_platform.h>
> >   #include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> >   #include <linux/slab.h>
> >   #include <linux/spinlock.h>
> > +#include <linux/soc/mediatek/infracfg.h>
> >   #include <asm/barrier.h>
> >   #include <soc/mediatek/smi.h>
> >   
> > @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >   	struct resource         *res;
> >   	resource_size_t		ioaddr;
> >   	struct component_match  *match = NULL;
> > +	struct regmap		*infracfg;
> >   	void                    *protect;
> >   	int                     i, larb_nr, ret;
> > +	u32			val;
> >   
> >   	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> >   	if (!data)
> > @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >   		return -ENOMEM;
> >   	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
> >   
> > -	/* Whether the current dram is over 4GB */
> > -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
> > -	if (!data->plat_data->has_4gb_mode)
> > -		data->enable_4GB = false;
> > +	data->enable_4GB = false;
> > +	if (data->plat_data->has_4gb_mode) {
> > +		infracfg = syscon_regmap_lookup_by_compatible(
> > +				"mediatek,mt8173-infracfg");
> > +		if (IS_ERR(infracfg)) {
> > +			infracfg = syscon_regmap_lookup_by_compatible(
> > +					"mediatek,mt2712-infracfg");
> > +			if (IS_ERR(infracfg))
> > +				return PTR_ERR(infracfg);
> 
> I think we should check m4u_plat instead to decide which compatible we have to 
> look for.
> Another option would be to add a general compatible something like 
> "mtk-infracfg" and search for that. That would need an update of all DTS having 
> a infracfg compatible right now. After thinking twice, this would break newer 
> kernel with older device tree, so maybe it's better to go with m4u_plat switch 
> statement.

Add a "char *infracfg" in the plat_data, Use the mt2712, mt8173
corresponding string in it. If it is NULL, It means the "enable_4GB"
always is false. Then we also can remove the flag "has_4gb_mode".

is this OK?

> 
> Regards,
> Matthias
> 
> > +
> > +		}
> > +		ret = regmap_read(infracfg, REG_INFRA_MISC, &val);
> > +		if (ret)
> > +			return ret;
> > +		data->enable_4GB = !!(val & F_DDR_4GB_SUPPORT_EN);
> > +	}
> >   
> >   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >   	data->base = devm_ioremap_resource(dev, res);
> > diff --git a/include/linux/soc/mediatek/infracfg.h b/include/linux/soc/mediatek/infracfg.h
> > index fd25f0148566..233463d789c6 100644
> > --- a/include/linux/soc/mediatek/infracfg.h
> > +++ b/include/linux/soc/mediatek/infracfg.h
> > @@ -32,6 +32,9 @@
> >   #define MT7622_TOP_AXI_PROT_EN_WB		(BIT(2) | BIT(6) | \
> >   						 BIT(7) | BIT(8))
> >   
> > +#define REG_INFRA_MISC				0xf00
> > +#define F_DDR_4GB_SUPPORT_EN			BIT(13)
> > +
> >   int mtk_infracfg_set_bus_protection(struct regmap *infracfg, u32 mask,
> >   		bool reg_update);
> >   int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask,
> > 

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
@ 2020-07-21 11:24     ` Yong Wu
  0 siblings, 0 replies; 32+ messages in thread
From: Yong Wu @ 2020-07-21 11:24 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Rob Herring, wsd_upstream, David Hildenbrand, Joerg Roedel,
	iommu, linux-kernel, Chao Hao, Miles Chen, linux-mediatek,
	Yingjoe Chen, Mike Rapoport, Christoph Hellwig, linux-arm-kernel

On Tue, 2020-07-21 at 11:40 +0200, Matthias Brugger wrote:
> 
> On 21/07/2020 04:16, Miles Chen wrote:
> > In previous discussion [1] and [2], we found that it is risky to
> > use max_pfn or totalram_pages to tell if 4GB mode is enabled.
> > 
> > Check 4GB mode by reading infracfg register, remove the usage
> > of the un-exported symbol max_pfn.
> > 
> > This is a step towards building mtk_iommu as a kernel module.
> > 
> > Change since v1:
> > 1. remove the phandle usage, search for infracfg instead [3]
> > 2. use infracfg instead of infracfg_regmap
> > 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
> > 4. update enable_4GB only when has_4gb_mode
> > 
> > [1] https://lkml.org/lkml/2020/6/3/733
> > [2] https://lkml.org/lkml/2020/6/4/136
> > [3] https://lkml.org/lkml/2020/7/15/1147
> > 
> > Cc: Mike Rapoport <rppt@linux.ibm.com>
> > Cc: David Hildenbrand <david@redhat.com>
> > Cc: Yong Wu <yong.wu@mediatek.com>
> > Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Yong Wu <yong.wu@mediatek.com>
> > Cc: Chao Hao <chao.hao@mediatek.com>
> > Cc: Rob Herring <robh@kernel.org>
> > Cc: Matthias Brugger <matthias.bgg@gmail.com>
> > Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> > ---
> >   drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
> >   include/linux/soc/mediatek/infracfg.h |  3 +++
> >   2 files changed, 24 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > index 2be96f1cdbd2..16765f532853 100644
> > --- a/drivers/iommu/mtk_iommu.c
> > +++ b/drivers/iommu/mtk_iommu.c
> > @@ -3,7 +3,6 @@
> >    * Copyright (c) 2015-2016 MediaTek Inc.
> >    * Author: Yong Wu <yong.wu@mediatek.com>
> >    */
> > -#include <linux/memblock.h>
> >   #include <linux/bug.h>
> >   #include <linux/clk.h>
> >   #include <linux/component.h>
> > @@ -15,13 +14,16 @@
> >   #include <linux/iommu.h>
> >   #include <linux/iopoll.h>
> >   #include <linux/list.h>
> > +#include <linux/mfd/syscon.h>
> >   #include <linux/of_address.h>
> >   #include <linux/of_iommu.h>
> >   #include <linux/of_irq.h>
> >   #include <linux/of_platform.h>
> >   #include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> >   #include <linux/slab.h>
> >   #include <linux/spinlock.h>
> > +#include <linux/soc/mediatek/infracfg.h>
> >   #include <asm/barrier.h>
> >   #include <soc/mediatek/smi.h>
> >   
> > @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >   	struct resource         *res;
> >   	resource_size_t		ioaddr;
> >   	struct component_match  *match = NULL;
> > +	struct regmap		*infracfg;
> >   	void                    *protect;
> >   	int                     i, larb_nr, ret;
> > +	u32			val;
> >   
> >   	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> >   	if (!data)
> > @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >   		return -ENOMEM;
> >   	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
> >   
> > -	/* Whether the current dram is over 4GB */
> > -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
> > -	if (!data->plat_data->has_4gb_mode)
> > -		data->enable_4GB = false;
> > +	data->enable_4GB = false;
> > +	if (data->plat_data->has_4gb_mode) {
> > +		infracfg = syscon_regmap_lookup_by_compatible(
> > +				"mediatek,mt8173-infracfg");
> > +		if (IS_ERR(infracfg)) {
> > +			infracfg = syscon_regmap_lookup_by_compatible(
> > +					"mediatek,mt2712-infracfg");
> > +			if (IS_ERR(infracfg))
> > +				return PTR_ERR(infracfg);
> 
> I think we should check m4u_plat instead to decide which compatible we have to 
> look for.
> Another option would be to add a general compatible something like 
> "mtk-infracfg" and search for that. That would need an update of all DTS having 
> a infracfg compatible right now. After thinking twice, this would break newer 
> kernel with older device tree, so maybe it's better to go with m4u_plat switch 
> statement.

Add a "char *infracfg" in the plat_data, Use the mt2712, mt8173
corresponding string in it. If it is NULL, It means the "enable_4GB"
always is false. Then we also can remove the flag "has_4gb_mode".

is this OK?

> 
> Regards,
> Matthias
> 
> > +
> > +		}
> > +		ret = regmap_read(infracfg, REG_INFRA_MISC, &val);
> > +		if (ret)
> > +			return ret;
> > +		data->enable_4GB = !!(val & F_DDR_4GB_SUPPORT_EN);
> > +	}
> >   
> >   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >   	data->base = devm_ioremap_resource(dev, res);
> > diff --git a/include/linux/soc/mediatek/infracfg.h b/include/linux/soc/mediatek/infracfg.h
> > index fd25f0148566..233463d789c6 100644
> > --- a/include/linux/soc/mediatek/infracfg.h
> > +++ b/include/linux/soc/mediatek/infracfg.h
> > @@ -32,6 +32,9 @@
> >   #define MT7622_TOP_AXI_PROT_EN_WB		(BIT(2) | BIT(6) | \
> >   						 BIT(7) | BIT(8))
> >   
> > +#define REG_INFRA_MISC				0xf00
> > +#define F_DDR_4GB_SUPPORT_EN			BIT(13)
> > +
> >   int mtk_infracfg_set_bus_protection(struct regmap *infracfg, u32 mask,
> >   		bool reg_update);
> >   int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask,
> > 

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

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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
@ 2020-07-21 11:24     ` Yong Wu
  0 siblings, 0 replies; 32+ messages in thread
From: Yong Wu @ 2020-07-21 11:24 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Rob Herring, wsd_upstream, David Hildenbrand, Joerg Roedel,
	iommu, linux-kernel, Chao Hao, Miles Chen, linux-mediatek,
	Yingjoe Chen, Mike Rapoport, Christoph Hellwig, linux-arm-kernel

On Tue, 2020-07-21 at 11:40 +0200, Matthias Brugger wrote:
> 
> On 21/07/2020 04:16, Miles Chen wrote:
> > In previous discussion [1] and [2], we found that it is risky to
> > use max_pfn or totalram_pages to tell if 4GB mode is enabled.
> > 
> > Check 4GB mode by reading infracfg register, remove the usage
> > of the un-exported symbol max_pfn.
> > 
> > This is a step towards building mtk_iommu as a kernel module.
> > 
> > Change since v1:
> > 1. remove the phandle usage, search for infracfg instead [3]
> > 2. use infracfg instead of infracfg_regmap
> > 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
> > 4. update enable_4GB only when has_4gb_mode
> > 
> > [1] https://lkml.org/lkml/2020/6/3/733
> > [2] https://lkml.org/lkml/2020/6/4/136
> > [3] https://lkml.org/lkml/2020/7/15/1147
> > 
> > Cc: Mike Rapoport <rppt@linux.ibm.com>
> > Cc: David Hildenbrand <david@redhat.com>
> > Cc: Yong Wu <yong.wu@mediatek.com>
> > Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Yong Wu <yong.wu@mediatek.com>
> > Cc: Chao Hao <chao.hao@mediatek.com>
> > Cc: Rob Herring <robh@kernel.org>
> > Cc: Matthias Brugger <matthias.bgg@gmail.com>
> > Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> > ---
> >   drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
> >   include/linux/soc/mediatek/infracfg.h |  3 +++
> >   2 files changed, 24 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > index 2be96f1cdbd2..16765f532853 100644
> > --- a/drivers/iommu/mtk_iommu.c
> > +++ b/drivers/iommu/mtk_iommu.c
> > @@ -3,7 +3,6 @@
> >    * Copyright (c) 2015-2016 MediaTek Inc.
> >    * Author: Yong Wu <yong.wu@mediatek.com>
> >    */
> > -#include <linux/memblock.h>
> >   #include <linux/bug.h>
> >   #include <linux/clk.h>
> >   #include <linux/component.h>
> > @@ -15,13 +14,16 @@
> >   #include <linux/iommu.h>
> >   #include <linux/iopoll.h>
> >   #include <linux/list.h>
> > +#include <linux/mfd/syscon.h>
> >   #include <linux/of_address.h>
> >   #include <linux/of_iommu.h>
> >   #include <linux/of_irq.h>
> >   #include <linux/of_platform.h>
> >   #include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> >   #include <linux/slab.h>
> >   #include <linux/spinlock.h>
> > +#include <linux/soc/mediatek/infracfg.h>
> >   #include <asm/barrier.h>
> >   #include <soc/mediatek/smi.h>
> >   
> > @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >   	struct resource         *res;
> >   	resource_size_t		ioaddr;
> >   	struct component_match  *match = NULL;
> > +	struct regmap		*infracfg;
> >   	void                    *protect;
> >   	int                     i, larb_nr, ret;
> > +	u32			val;
> >   
> >   	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> >   	if (!data)
> > @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >   		return -ENOMEM;
> >   	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
> >   
> > -	/* Whether the current dram is over 4GB */
> > -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
> > -	if (!data->plat_data->has_4gb_mode)
> > -		data->enable_4GB = false;
> > +	data->enable_4GB = false;
> > +	if (data->plat_data->has_4gb_mode) {
> > +		infracfg = syscon_regmap_lookup_by_compatible(
> > +				"mediatek,mt8173-infracfg");
> > +		if (IS_ERR(infracfg)) {
> > +			infracfg = syscon_regmap_lookup_by_compatible(
> > +					"mediatek,mt2712-infracfg");
> > +			if (IS_ERR(infracfg))
> > +				return PTR_ERR(infracfg);
> 
> I think we should check m4u_plat instead to decide which compatible we have to 
> look for.
> Another option would be to add a general compatible something like 
> "mtk-infracfg" and search for that. That would need an update of all DTS having 
> a infracfg compatible right now. After thinking twice, this would break newer 
> kernel with older device tree, so maybe it's better to go with m4u_plat switch 
> statement.

Add a "char *infracfg" in the plat_data, Use the mt2712, mt8173
corresponding string in it. If it is NULL, It means the "enable_4GB"
always is false. Then we also can remove the flag "has_4gb_mode".

is this OK?

> 
> Regards,
> Matthias
> 
> > +
> > +		}
> > +		ret = regmap_read(infracfg, REG_INFRA_MISC, &val);
> > +		if (ret)
> > +			return ret;
> > +		data->enable_4GB = !!(val & F_DDR_4GB_SUPPORT_EN);
> > +	}
> >   
> >   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >   	data->base = devm_ioremap_resource(dev, res);
> > diff --git a/include/linux/soc/mediatek/infracfg.h b/include/linux/soc/mediatek/infracfg.h
> > index fd25f0148566..233463d789c6 100644
> > --- a/include/linux/soc/mediatek/infracfg.h
> > +++ b/include/linux/soc/mediatek/infracfg.h
> > @@ -32,6 +32,9 @@
> >   #define MT7622_TOP_AXI_PROT_EN_WB		(BIT(2) | BIT(6) | \
> >   						 BIT(7) | BIT(8))
> >   
> > +#define REG_INFRA_MISC				0xf00
> > +#define F_DDR_4GB_SUPPORT_EN			BIT(13)
> > +
> >   int mtk_infracfg_set_bus_protection(struct regmap *infracfg, u32 mask,
> >   		bool reg_update);
> >   int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask,
> > 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
  2020-07-21 11:24     ` Yong Wu
  (?)
  (?)
@ 2020-07-21 21:19       ` Matthias Brugger
  -1 siblings, 0 replies; 32+ messages in thread
From: Matthias Brugger @ 2020-07-21 21:19 UTC (permalink / raw)
  To: Yong Wu
  Cc: Miles Chen, Joerg Roedel, Rob Herring, iommu, linux-arm-kernel,
	linux-mediatek, linux-kernel, wsd_upstream, Mike Rapoport,
	David Hildenbrand, Yingjoe Chen, Christoph Hellwig, Chao Hao



On 21/07/2020 13:24, Yong Wu wrote:
> On Tue, 2020-07-21 at 11:40 +0200, Matthias Brugger wrote:
>>
>> On 21/07/2020 04:16, Miles Chen wrote:
>>> In previous discussion [1] and [2], we found that it is risky to
>>> use max_pfn or totalram_pages to tell if 4GB mode is enabled.
>>>
>>> Check 4GB mode by reading infracfg register, remove the usage
>>> of the un-exported symbol max_pfn.
>>>
>>> This is a step towards building mtk_iommu as a kernel module.
>>>
>>> Change since v1:
>>> 1. remove the phandle usage, search for infracfg instead [3]
>>> 2. use infracfg instead of infracfg_regmap
>>> 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
>>> 4. update enable_4GB only when has_4gb_mode
>>>
>>> [1] https://lkml.org/lkml/2020/6/3/733
>>> [2] https://lkml.org/lkml/2020/6/4/136
>>> [3] https://lkml.org/lkml/2020/7/15/1147
>>>
>>> Cc: Mike Rapoport <rppt@linux.ibm.com>
>>> Cc: David Hildenbrand <david@redhat.com>
>>> Cc: Yong Wu <yong.wu@mediatek.com>
>>> Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
>>> Cc: Christoph Hellwig <hch@lst.de>
>>> Cc: Yong Wu <yong.wu@mediatek.com>
>>> Cc: Chao Hao <chao.hao@mediatek.com>
>>> Cc: Rob Herring <robh@kernel.org>
>>> Cc: Matthias Brugger <matthias.bgg@gmail.com>
>>> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
>>> ---
>>>    drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
>>>    include/linux/soc/mediatek/infracfg.h |  3 +++
>>>    2 files changed, 24 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
>>> index 2be96f1cdbd2..16765f532853 100644
>>> --- a/drivers/iommu/mtk_iommu.c
>>> +++ b/drivers/iommu/mtk_iommu.c
>>> @@ -3,7 +3,6 @@
>>>     * Copyright (c) 2015-2016 MediaTek Inc.
>>>     * Author: Yong Wu <yong.wu@mediatek.com>
>>>     */
>>> -#include <linux/memblock.h>
>>>    #include <linux/bug.h>
>>>    #include <linux/clk.h>
>>>    #include <linux/component.h>
>>> @@ -15,13 +14,16 @@
>>>    #include <linux/iommu.h>
>>>    #include <linux/iopoll.h>
>>>    #include <linux/list.h>
>>> +#include <linux/mfd/syscon.h>
>>>    #include <linux/of_address.h>
>>>    #include <linux/of_iommu.h>
>>>    #include <linux/of_irq.h>
>>>    #include <linux/of_platform.h>
>>>    #include <linux/platform_device.h>
>>> +#include <linux/regmap.h>
>>>    #include <linux/slab.h>
>>>    #include <linux/spinlock.h>
>>> +#include <linux/soc/mediatek/infracfg.h>
>>>    #include <asm/barrier.h>
>>>    #include <soc/mediatek/smi.h>
>>>    
>>> @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>>>    	struct resource         *res;
>>>    	resource_size_t		ioaddr;
>>>    	struct component_match  *match = NULL;
>>> +	struct regmap		*infracfg;
>>>    	void                    *protect;
>>>    	int                     i, larb_nr, ret;
>>> +	u32			val;
>>>    
>>>    	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>>>    	if (!data)
>>> @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>>>    		return -ENOMEM;
>>>    	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
>>>    
>>> -	/* Whether the current dram is over 4GB */
>>> -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
>>> -	if (!data->plat_data->has_4gb_mode)
>>> -		data->enable_4GB = false;
>>> +	data->enable_4GB = false;
>>> +	if (data->plat_data->has_4gb_mode) {
>>> +		infracfg = syscon_regmap_lookup_by_compatible(
>>> +				"mediatek,mt8173-infracfg");
>>> +		if (IS_ERR(infracfg)) {
>>> +			infracfg = syscon_regmap_lookup_by_compatible(
>>> +					"mediatek,mt2712-infracfg");
>>> +			if (IS_ERR(infracfg))
>>> +				return PTR_ERR(infracfg);
>>
>> I think we should check m4u_plat instead to decide which compatible we have to
>> look for.
>> Another option would be to add a general compatible something like
>> "mtk-infracfg" and search for that. That would need an update of all DTS having
>> a infracfg compatible right now. After thinking twice, this would break newer
>> kernel with older device tree, so maybe it's better to go with m4u_plat switch
>> statement.
> 
> Add a "char *infracfg" in the plat_data, Use the mt2712, mt8173
> corresponding string in it. If it is NULL, It means the "enable_4GB"
> always is false. Then we also can remove the flag "has_4gb_mode".
> 
> is this OK?
> 

It's an option, but I personally find that a bit hacky.

Regards,
Matthias

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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
@ 2020-07-21 21:19       ` Matthias Brugger
  0 siblings, 0 replies; 32+ messages in thread
From: Matthias Brugger @ 2020-07-21 21:19 UTC (permalink / raw)
  To: Yong Wu
  Cc: Rob Herring, wsd_upstream, David Hildenbrand, iommu,
	linux-kernel, Chao Hao, Miles Chen, linux-mediatek, Yingjoe Chen,
	Mike Rapoport, Christoph Hellwig, linux-arm-kernel



On 21/07/2020 13:24, Yong Wu wrote:
> On Tue, 2020-07-21 at 11:40 +0200, Matthias Brugger wrote:
>>
>> On 21/07/2020 04:16, Miles Chen wrote:
>>> In previous discussion [1] and [2], we found that it is risky to
>>> use max_pfn or totalram_pages to tell if 4GB mode is enabled.
>>>
>>> Check 4GB mode by reading infracfg register, remove the usage
>>> of the un-exported symbol max_pfn.
>>>
>>> This is a step towards building mtk_iommu as a kernel module.
>>>
>>> Change since v1:
>>> 1. remove the phandle usage, search for infracfg instead [3]
>>> 2. use infracfg instead of infracfg_regmap
>>> 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
>>> 4. update enable_4GB only when has_4gb_mode
>>>
>>> [1] https://lkml.org/lkml/2020/6/3/733
>>> [2] https://lkml.org/lkml/2020/6/4/136
>>> [3] https://lkml.org/lkml/2020/7/15/1147
>>>
>>> Cc: Mike Rapoport <rppt@linux.ibm.com>
>>> Cc: David Hildenbrand <david@redhat.com>
>>> Cc: Yong Wu <yong.wu@mediatek.com>
>>> Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
>>> Cc: Christoph Hellwig <hch@lst.de>
>>> Cc: Yong Wu <yong.wu@mediatek.com>
>>> Cc: Chao Hao <chao.hao@mediatek.com>
>>> Cc: Rob Herring <robh@kernel.org>
>>> Cc: Matthias Brugger <matthias.bgg@gmail.com>
>>> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
>>> ---
>>>    drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
>>>    include/linux/soc/mediatek/infracfg.h |  3 +++
>>>    2 files changed, 24 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
>>> index 2be96f1cdbd2..16765f532853 100644
>>> --- a/drivers/iommu/mtk_iommu.c
>>> +++ b/drivers/iommu/mtk_iommu.c
>>> @@ -3,7 +3,6 @@
>>>     * Copyright (c) 2015-2016 MediaTek Inc.
>>>     * Author: Yong Wu <yong.wu@mediatek.com>
>>>     */
>>> -#include <linux/memblock.h>
>>>    #include <linux/bug.h>
>>>    #include <linux/clk.h>
>>>    #include <linux/component.h>
>>> @@ -15,13 +14,16 @@
>>>    #include <linux/iommu.h>
>>>    #include <linux/iopoll.h>
>>>    #include <linux/list.h>
>>> +#include <linux/mfd/syscon.h>
>>>    #include <linux/of_address.h>
>>>    #include <linux/of_iommu.h>
>>>    #include <linux/of_irq.h>
>>>    #include <linux/of_platform.h>
>>>    #include <linux/platform_device.h>
>>> +#include <linux/regmap.h>
>>>    #include <linux/slab.h>
>>>    #include <linux/spinlock.h>
>>> +#include <linux/soc/mediatek/infracfg.h>
>>>    #include <asm/barrier.h>
>>>    #include <soc/mediatek/smi.h>
>>>    
>>> @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>>>    	struct resource         *res;
>>>    	resource_size_t		ioaddr;
>>>    	struct component_match  *match = NULL;
>>> +	struct regmap		*infracfg;
>>>    	void                    *protect;
>>>    	int                     i, larb_nr, ret;
>>> +	u32			val;
>>>    
>>>    	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>>>    	if (!data)
>>> @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>>>    		return -ENOMEM;
>>>    	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
>>>    
>>> -	/* Whether the current dram is over 4GB */
>>> -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
>>> -	if (!data->plat_data->has_4gb_mode)
>>> -		data->enable_4GB = false;
>>> +	data->enable_4GB = false;
>>> +	if (data->plat_data->has_4gb_mode) {
>>> +		infracfg = syscon_regmap_lookup_by_compatible(
>>> +				"mediatek,mt8173-infracfg");
>>> +		if (IS_ERR(infracfg)) {
>>> +			infracfg = syscon_regmap_lookup_by_compatible(
>>> +					"mediatek,mt2712-infracfg");
>>> +			if (IS_ERR(infracfg))
>>> +				return PTR_ERR(infracfg);
>>
>> I think we should check m4u_plat instead to decide which compatible we have to
>> look for.
>> Another option would be to add a general compatible something like
>> "mtk-infracfg" and search for that. That would need an update of all DTS having
>> a infracfg compatible right now. After thinking twice, this would break newer
>> kernel with older device tree, so maybe it's better to go with m4u_plat switch
>> statement.
> 
> Add a "char *infracfg" in the plat_data, Use the mt2712, mt8173
> corresponding string in it. If it is NULL, It means the "enable_4GB"
> always is false. Then we also can remove the flag "has_4gb_mode".
> 
> is this OK?
> 

It's an option, but I personally find that a bit hacky.

Regards,
Matthias
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
@ 2020-07-21 21:19       ` Matthias Brugger
  0 siblings, 0 replies; 32+ messages in thread
From: Matthias Brugger @ 2020-07-21 21:19 UTC (permalink / raw)
  To: Yong Wu
  Cc: Rob Herring, wsd_upstream, David Hildenbrand, Joerg Roedel,
	iommu, linux-kernel, Chao Hao, Miles Chen, linux-mediatek,
	Yingjoe Chen, Mike Rapoport, Christoph Hellwig, linux-arm-kernel



On 21/07/2020 13:24, Yong Wu wrote:
> On Tue, 2020-07-21 at 11:40 +0200, Matthias Brugger wrote:
>>
>> On 21/07/2020 04:16, Miles Chen wrote:
>>> In previous discussion [1] and [2], we found that it is risky to
>>> use max_pfn or totalram_pages to tell if 4GB mode is enabled.
>>>
>>> Check 4GB mode by reading infracfg register, remove the usage
>>> of the un-exported symbol max_pfn.
>>>
>>> This is a step towards building mtk_iommu as a kernel module.
>>>
>>> Change since v1:
>>> 1. remove the phandle usage, search for infracfg instead [3]
>>> 2. use infracfg instead of infracfg_regmap
>>> 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
>>> 4. update enable_4GB only when has_4gb_mode
>>>
>>> [1] https://lkml.org/lkml/2020/6/3/733
>>> [2] https://lkml.org/lkml/2020/6/4/136
>>> [3] https://lkml.org/lkml/2020/7/15/1147
>>>
>>> Cc: Mike Rapoport <rppt@linux.ibm.com>
>>> Cc: David Hildenbrand <david@redhat.com>
>>> Cc: Yong Wu <yong.wu@mediatek.com>
>>> Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
>>> Cc: Christoph Hellwig <hch@lst.de>
>>> Cc: Yong Wu <yong.wu@mediatek.com>
>>> Cc: Chao Hao <chao.hao@mediatek.com>
>>> Cc: Rob Herring <robh@kernel.org>
>>> Cc: Matthias Brugger <matthias.bgg@gmail.com>
>>> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
>>> ---
>>>    drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
>>>    include/linux/soc/mediatek/infracfg.h |  3 +++
>>>    2 files changed, 24 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
>>> index 2be96f1cdbd2..16765f532853 100644
>>> --- a/drivers/iommu/mtk_iommu.c
>>> +++ b/drivers/iommu/mtk_iommu.c
>>> @@ -3,7 +3,6 @@
>>>     * Copyright (c) 2015-2016 MediaTek Inc.
>>>     * Author: Yong Wu <yong.wu@mediatek.com>
>>>     */
>>> -#include <linux/memblock.h>
>>>    #include <linux/bug.h>
>>>    #include <linux/clk.h>
>>>    #include <linux/component.h>
>>> @@ -15,13 +14,16 @@
>>>    #include <linux/iommu.h>
>>>    #include <linux/iopoll.h>
>>>    #include <linux/list.h>
>>> +#include <linux/mfd/syscon.h>
>>>    #include <linux/of_address.h>
>>>    #include <linux/of_iommu.h>
>>>    #include <linux/of_irq.h>
>>>    #include <linux/of_platform.h>
>>>    #include <linux/platform_device.h>
>>> +#include <linux/regmap.h>
>>>    #include <linux/slab.h>
>>>    #include <linux/spinlock.h>
>>> +#include <linux/soc/mediatek/infracfg.h>
>>>    #include <asm/barrier.h>
>>>    #include <soc/mediatek/smi.h>
>>>    
>>> @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>>>    	struct resource         *res;
>>>    	resource_size_t		ioaddr;
>>>    	struct component_match  *match = NULL;
>>> +	struct regmap		*infracfg;
>>>    	void                    *protect;
>>>    	int                     i, larb_nr, ret;
>>> +	u32			val;
>>>    
>>>    	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>>>    	if (!data)
>>> @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>>>    		return -ENOMEM;
>>>    	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
>>>    
>>> -	/* Whether the current dram is over 4GB */
>>> -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
>>> -	if (!data->plat_data->has_4gb_mode)
>>> -		data->enable_4GB = false;
>>> +	data->enable_4GB = false;
>>> +	if (data->plat_data->has_4gb_mode) {
>>> +		infracfg = syscon_regmap_lookup_by_compatible(
>>> +				"mediatek,mt8173-infracfg");
>>> +		if (IS_ERR(infracfg)) {
>>> +			infracfg = syscon_regmap_lookup_by_compatible(
>>> +					"mediatek,mt2712-infracfg");
>>> +			if (IS_ERR(infracfg))
>>> +				return PTR_ERR(infracfg);
>>
>> I think we should check m4u_plat instead to decide which compatible we have to
>> look for.
>> Another option would be to add a general compatible something like
>> "mtk-infracfg" and search for that. That would need an update of all DTS having
>> a infracfg compatible right now. After thinking twice, this would break newer
>> kernel with older device tree, so maybe it's better to go with m4u_plat switch
>> statement.
> 
> Add a "char *infracfg" in the plat_data, Use the mt2712, mt8173
> corresponding string in it. If it is NULL, It means the "enable_4GB"
> always is false. Then we also can remove the flag "has_4gb_mode".
> 
> is this OK?
> 

It's an option, but I personally find that a bit hacky.

Regards,
Matthias

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

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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
@ 2020-07-21 21:19       ` Matthias Brugger
  0 siblings, 0 replies; 32+ messages in thread
From: Matthias Brugger @ 2020-07-21 21:19 UTC (permalink / raw)
  To: Yong Wu
  Cc: Rob Herring, wsd_upstream, David Hildenbrand, Joerg Roedel,
	iommu, linux-kernel, Chao Hao, Miles Chen, linux-mediatek,
	Yingjoe Chen, Mike Rapoport, Christoph Hellwig, linux-arm-kernel



On 21/07/2020 13:24, Yong Wu wrote:
> On Tue, 2020-07-21 at 11:40 +0200, Matthias Brugger wrote:
>>
>> On 21/07/2020 04:16, Miles Chen wrote:
>>> In previous discussion [1] and [2], we found that it is risky to
>>> use max_pfn or totalram_pages to tell if 4GB mode is enabled.
>>>
>>> Check 4GB mode by reading infracfg register, remove the usage
>>> of the un-exported symbol max_pfn.
>>>
>>> This is a step towards building mtk_iommu as a kernel module.
>>>
>>> Change since v1:
>>> 1. remove the phandle usage, search for infracfg instead [3]
>>> 2. use infracfg instead of infracfg_regmap
>>> 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
>>> 4. update enable_4GB only when has_4gb_mode
>>>
>>> [1] https://lkml.org/lkml/2020/6/3/733
>>> [2] https://lkml.org/lkml/2020/6/4/136
>>> [3] https://lkml.org/lkml/2020/7/15/1147
>>>
>>> Cc: Mike Rapoport <rppt@linux.ibm.com>
>>> Cc: David Hildenbrand <david@redhat.com>
>>> Cc: Yong Wu <yong.wu@mediatek.com>
>>> Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
>>> Cc: Christoph Hellwig <hch@lst.de>
>>> Cc: Yong Wu <yong.wu@mediatek.com>
>>> Cc: Chao Hao <chao.hao@mediatek.com>
>>> Cc: Rob Herring <robh@kernel.org>
>>> Cc: Matthias Brugger <matthias.bgg@gmail.com>
>>> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
>>> ---
>>>    drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
>>>    include/linux/soc/mediatek/infracfg.h |  3 +++
>>>    2 files changed, 24 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
>>> index 2be96f1cdbd2..16765f532853 100644
>>> --- a/drivers/iommu/mtk_iommu.c
>>> +++ b/drivers/iommu/mtk_iommu.c
>>> @@ -3,7 +3,6 @@
>>>     * Copyright (c) 2015-2016 MediaTek Inc.
>>>     * Author: Yong Wu <yong.wu@mediatek.com>
>>>     */
>>> -#include <linux/memblock.h>
>>>    #include <linux/bug.h>
>>>    #include <linux/clk.h>
>>>    #include <linux/component.h>
>>> @@ -15,13 +14,16 @@
>>>    #include <linux/iommu.h>
>>>    #include <linux/iopoll.h>
>>>    #include <linux/list.h>
>>> +#include <linux/mfd/syscon.h>
>>>    #include <linux/of_address.h>
>>>    #include <linux/of_iommu.h>
>>>    #include <linux/of_irq.h>
>>>    #include <linux/of_platform.h>
>>>    #include <linux/platform_device.h>
>>> +#include <linux/regmap.h>
>>>    #include <linux/slab.h>
>>>    #include <linux/spinlock.h>
>>> +#include <linux/soc/mediatek/infracfg.h>
>>>    #include <asm/barrier.h>
>>>    #include <soc/mediatek/smi.h>
>>>    
>>> @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>>>    	struct resource         *res;
>>>    	resource_size_t		ioaddr;
>>>    	struct component_match  *match = NULL;
>>> +	struct regmap		*infracfg;
>>>    	void                    *protect;
>>>    	int                     i, larb_nr, ret;
>>> +	u32			val;
>>>    
>>>    	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>>>    	if (!data)
>>> @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>>>    		return -ENOMEM;
>>>    	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
>>>    
>>> -	/* Whether the current dram is over 4GB */
>>> -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
>>> -	if (!data->plat_data->has_4gb_mode)
>>> -		data->enable_4GB = false;
>>> +	data->enable_4GB = false;
>>> +	if (data->plat_data->has_4gb_mode) {
>>> +		infracfg = syscon_regmap_lookup_by_compatible(
>>> +				"mediatek,mt8173-infracfg");
>>> +		if (IS_ERR(infracfg)) {
>>> +			infracfg = syscon_regmap_lookup_by_compatible(
>>> +					"mediatek,mt2712-infracfg");
>>> +			if (IS_ERR(infracfg))
>>> +				return PTR_ERR(infracfg);
>>
>> I think we should check m4u_plat instead to decide which compatible we have to
>> look for.
>> Another option would be to add a general compatible something like
>> "mtk-infracfg" and search for that. That would need an update of all DTS having
>> a infracfg compatible right now. After thinking twice, this would break newer
>> kernel with older device tree, so maybe it's better to go with m4u_plat switch
>> statement.
> 
> Add a "char *infracfg" in the plat_data, Use the mt2712, mt8173
> corresponding string in it. If it is NULL, It means the "enable_4GB"
> always is false. Then we also can remove the flag "has_4gb_mode".
> 
> is this OK?
> 

It's an option, but I personally find that a bit hacky.

Regards,
Matthias

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
  2020-07-21  9:10   ` David Hildenbrand
  (?)
  (?)
@ 2020-07-22  7:15     ` Miles Chen
  -1 siblings, 0 replies; 32+ messages in thread
From: Miles Chen @ 2020-07-22  7:15 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Joerg Roedel, Matthias Brugger, Rob Herring, iommu,
	linux-arm-kernel, linux-mediatek, linux-kernel, wsd_upstream,
	Mike Rapoport, Yong Wu, Yingjoe Chen, Christoph Hellwig,
	Chao Hao

On Tue, 2020-07-21 at 11:10 +0200, David Hildenbrand wrote:
> On 21.07.20 04:16, Miles Chen wrote:
> > In previous discussion [1] and [2], we found that it is risky to
> > use max_pfn or totalram_pages to tell if 4GB mode is enabled.
> > 
> > Check 4GB mode by reading infracfg register, remove the usage
> > of the un-exported symbol max_pfn.
> > 
> > This is a step towards building mtk_iommu as a kernel module.
> > 
> > Change since v1:
> > 1. remove the phandle usage, search for infracfg instead [3]
> > 2. use infracfg instead of infracfg_regmap
> > 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
> > 4. update enable_4GB only when has_4gb_mode
> 
> Nit: We tend to place such information below the "---" (adding a second
> one) such that this information is discarded when the patch is picked up.

Thanks, I'll add a "---" before the "Change since..." in patch v3.
> 
> > 
> > [1] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/3/733__;!!CTRNKA9wMg0ARbw!wrP2H0azuZkTHSnM_ETki4kT_FMF0Cl7abQX8tIHX0iTr65JkvMCe4jDt84d_YhBrA$ 
> > [2] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/4/136__;!!CTRNKA9wMg0ARbw!wrP2H0azuZkTHSnM_ETki4kT_FMF0Cl7abQX8tIHX0iTr65JkvMCe4jDt86IGhmouQ$ 
> > [3] https://urldefense.com/v3/__https://lkml.org/lkml/2020/7/15/1147__;!!CTRNKA9wMg0ARbw!wrP2H0azuZkTHSnM_ETki4kT_FMF0Cl7abQX8tIHX0iTr65JkvMCe4jDt855z4xdqw$ 
> > 
> > Cc: Mike Rapoport <rppt@linux.ibm.com>
> > Cc: David Hildenbrand <david@redhat.com>
> > Cc: Yong Wu <yong.wu@mediatek.com>
> > Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Yong Wu <yong.wu@mediatek.com>
> > Cc: Chao Hao <chao.hao@mediatek.com>
> > Cc: Rob Herring <robh@kernel.org>
> > Cc: Matthias Brugger <matthias.bgg@gmail.com>
> > Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> > ---
> >  drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
> >  include/linux/soc/mediatek/infracfg.h |  3 +++
> >  2 files changed, 24 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > index 2be96f1cdbd2..16765f532853 100644
> > --- a/drivers/iommu/mtk_iommu.c
> > +++ b/drivers/iommu/mtk_iommu.c
> > @@ -3,7 +3,6 @@
> >   * Copyright (c) 2015-2016 MediaTek Inc.
> >   * Author: Yong Wu <yong.wu@mediatek.com>
> >   */
> > -#include <linux/memblock.h>
> >  #include <linux/bug.h>
> >  #include <linux/clk.h>
> >  #include <linux/component.h>
> > @@ -15,13 +14,16 @@
> >  #include <linux/iommu.h>
> >  #include <linux/iopoll.h>
> >  #include <linux/list.h>
> > +#include <linux/mfd/syscon.h>
> >  #include <linux/of_address.h>
> >  #include <linux/of_iommu.h>
> >  #include <linux/of_irq.h>
> >  #include <linux/of_platform.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> >  #include <linux/slab.h>
> >  #include <linux/spinlock.h>
> > +#include <linux/soc/mediatek/infracfg.h>
> >  #include <asm/barrier.h>
> >  #include <soc/mediatek/smi.h>
> >  
> > @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >  	struct resource         *res;
> >  	resource_size_t		ioaddr;
> >  	struct component_match  *match = NULL;
> > +	struct regmap		*infracfg;
> >  	void                    *protect;
> >  	int                     i, larb_nr, ret;
> > +	u32			val;
> >  
> >  	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> >  	if (!data)
> > @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >  		return -ENOMEM;
> >  	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
> >  
> > -	/* Whether the current dram is over 4GB */
> > -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
> > -	if (!data->plat_data->has_4gb_mode)
> > -		data->enable_4GB = false;
> > +	data->enable_4GB = false;
> > +	if (data->plat_data->has_4gb_mode) {
> > +		infracfg = syscon_regmap_lookup_by_compatible(
> > +				"mediatek,mt8173-infracfg");
> > +		if (IS_ERR(infracfg)) {
> > +			infracfg = syscon_regmap_lookup_by_compatible(
> > +					"mediatek,mt2712-infracfg");
> > +			if (IS_ERR(infracfg))
> > +				return PTR_ERR(infracfg);
> > +
> > +		}
> > +		ret = regmap_read(infracfg, REG_INFRA_MISC, &val);
> > +		if (ret)
> > +			return ret;
> > +		data->enable_4GB = !!(val & F_DDR_4GB_SUPPORT_EN);
> 
> (I am absolutely not familiar with syscon_regmap_lookup_by ..., I am
> missing some context, so sorry for the stupid questions)
> 
> Who sets the regmap value and based on what? Who decides that 4GB mode
> is supported or not? And who decides if 4GB mode is *required* or not?
> 


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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
@ 2020-07-22  7:15     ` Miles Chen
  0 siblings, 0 replies; 32+ messages in thread
From: Miles Chen @ 2020-07-22  7:15 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Rob Herring, wsd_upstream, linux-kernel, Mike Rapoport, Chao Hao,
	iommu, linux-mediatek, Matthias Brugger, Yingjoe Chen,
	Christoph Hellwig, linux-arm-kernel

On Tue, 2020-07-21 at 11:10 +0200, David Hildenbrand wrote:
> On 21.07.20 04:16, Miles Chen wrote:
> > In previous discussion [1] and [2], we found that it is risky to
> > use max_pfn or totalram_pages to tell if 4GB mode is enabled.
> > 
> > Check 4GB mode by reading infracfg register, remove the usage
> > of the un-exported symbol max_pfn.
> > 
> > This is a step towards building mtk_iommu as a kernel module.
> > 
> > Change since v1:
> > 1. remove the phandle usage, search for infracfg instead [3]
> > 2. use infracfg instead of infracfg_regmap
> > 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
> > 4. update enable_4GB only when has_4gb_mode
> 
> Nit: We tend to place such information below the "---" (adding a second
> one) such that this information is discarded when the patch is picked up.

Thanks, I'll add a "---" before the "Change since..." in patch v3.
> 
> > 
> > [1] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/3/733__;!!CTRNKA9wMg0ARbw!wrP2H0azuZkTHSnM_ETki4kT_FMF0Cl7abQX8tIHX0iTr65JkvMCe4jDt84d_YhBrA$ 
> > [2] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/4/136__;!!CTRNKA9wMg0ARbw!wrP2H0azuZkTHSnM_ETki4kT_FMF0Cl7abQX8tIHX0iTr65JkvMCe4jDt86IGhmouQ$ 
> > [3] https://urldefense.com/v3/__https://lkml.org/lkml/2020/7/15/1147__;!!CTRNKA9wMg0ARbw!wrP2H0azuZkTHSnM_ETki4kT_FMF0Cl7abQX8tIHX0iTr65JkvMCe4jDt855z4xdqw$ 
> > 
> > Cc: Mike Rapoport <rppt@linux.ibm.com>
> > Cc: David Hildenbrand <david@redhat.com>
> > Cc: Yong Wu <yong.wu@mediatek.com>
> > Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Yong Wu <yong.wu@mediatek.com>
> > Cc: Chao Hao <chao.hao@mediatek.com>
> > Cc: Rob Herring <robh@kernel.org>
> > Cc: Matthias Brugger <matthias.bgg@gmail.com>
> > Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> > ---
> >  drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
> >  include/linux/soc/mediatek/infracfg.h |  3 +++
> >  2 files changed, 24 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > index 2be96f1cdbd2..16765f532853 100644
> > --- a/drivers/iommu/mtk_iommu.c
> > +++ b/drivers/iommu/mtk_iommu.c
> > @@ -3,7 +3,6 @@
> >   * Copyright (c) 2015-2016 MediaTek Inc.
> >   * Author: Yong Wu <yong.wu@mediatek.com>
> >   */
> > -#include <linux/memblock.h>
> >  #include <linux/bug.h>
> >  #include <linux/clk.h>
> >  #include <linux/component.h>
> > @@ -15,13 +14,16 @@
> >  #include <linux/iommu.h>
> >  #include <linux/iopoll.h>
> >  #include <linux/list.h>
> > +#include <linux/mfd/syscon.h>
> >  #include <linux/of_address.h>
> >  #include <linux/of_iommu.h>
> >  #include <linux/of_irq.h>
> >  #include <linux/of_platform.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> >  #include <linux/slab.h>
> >  #include <linux/spinlock.h>
> > +#include <linux/soc/mediatek/infracfg.h>
> >  #include <asm/barrier.h>
> >  #include <soc/mediatek/smi.h>
> >  
> > @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >  	struct resource         *res;
> >  	resource_size_t		ioaddr;
> >  	struct component_match  *match = NULL;
> > +	struct regmap		*infracfg;
> >  	void                    *protect;
> >  	int                     i, larb_nr, ret;
> > +	u32			val;
> >  
> >  	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> >  	if (!data)
> > @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >  		return -ENOMEM;
> >  	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
> >  
> > -	/* Whether the current dram is over 4GB */
> > -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
> > -	if (!data->plat_data->has_4gb_mode)
> > -		data->enable_4GB = false;
> > +	data->enable_4GB = false;
> > +	if (data->plat_data->has_4gb_mode) {
> > +		infracfg = syscon_regmap_lookup_by_compatible(
> > +				"mediatek,mt8173-infracfg");
> > +		if (IS_ERR(infracfg)) {
> > +			infracfg = syscon_regmap_lookup_by_compatible(
> > +					"mediatek,mt2712-infracfg");
> > +			if (IS_ERR(infracfg))
> > +				return PTR_ERR(infracfg);
> > +
> > +		}
> > +		ret = regmap_read(infracfg, REG_INFRA_MISC, &val);
> > +		if (ret)
> > +			return ret;
> > +		data->enable_4GB = !!(val & F_DDR_4GB_SUPPORT_EN);
> 
> (I am absolutely not familiar with syscon_regmap_lookup_by ..., I am
> missing some context, so sorry for the stupid questions)
> 
> Who sets the regmap value and based on what? Who decides that 4GB mode
> is supported or not? And who decides if 4GB mode is *required* or not?
> 

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
@ 2020-07-22  7:15     ` Miles Chen
  0 siblings, 0 replies; 32+ messages in thread
From: Miles Chen @ 2020-07-22  7:15 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Rob Herring, wsd_upstream, Joerg Roedel, linux-kernel,
	Mike Rapoport, Chao Hao, iommu, linux-mediatek, Yong Wu,
	Matthias Brugger, Yingjoe Chen, Christoph Hellwig,
	linux-arm-kernel

On Tue, 2020-07-21 at 11:10 +0200, David Hildenbrand wrote:
> On 21.07.20 04:16, Miles Chen wrote:
> > In previous discussion [1] and [2], we found that it is risky to
> > use max_pfn or totalram_pages to tell if 4GB mode is enabled.
> > 
> > Check 4GB mode by reading infracfg register, remove the usage
> > of the un-exported symbol max_pfn.
> > 
> > This is a step towards building mtk_iommu as a kernel module.
> > 
> > Change since v1:
> > 1. remove the phandle usage, search for infracfg instead [3]
> > 2. use infracfg instead of infracfg_regmap
> > 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
> > 4. update enable_4GB only when has_4gb_mode
> 
> Nit: We tend to place such information below the "---" (adding a second
> one) such that this information is discarded when the patch is picked up.

Thanks, I'll add a "---" before the "Change since..." in patch v3.
> 
> > 
> > [1] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/3/733__;!!CTRNKA9wMg0ARbw!wrP2H0azuZkTHSnM_ETki4kT_FMF0Cl7abQX8tIHX0iTr65JkvMCe4jDt84d_YhBrA$ 
> > [2] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/4/136__;!!CTRNKA9wMg0ARbw!wrP2H0azuZkTHSnM_ETki4kT_FMF0Cl7abQX8tIHX0iTr65JkvMCe4jDt86IGhmouQ$ 
> > [3] https://urldefense.com/v3/__https://lkml.org/lkml/2020/7/15/1147__;!!CTRNKA9wMg0ARbw!wrP2H0azuZkTHSnM_ETki4kT_FMF0Cl7abQX8tIHX0iTr65JkvMCe4jDt855z4xdqw$ 
> > 
> > Cc: Mike Rapoport <rppt@linux.ibm.com>
> > Cc: David Hildenbrand <david@redhat.com>
> > Cc: Yong Wu <yong.wu@mediatek.com>
> > Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Yong Wu <yong.wu@mediatek.com>
> > Cc: Chao Hao <chao.hao@mediatek.com>
> > Cc: Rob Herring <robh@kernel.org>
> > Cc: Matthias Brugger <matthias.bgg@gmail.com>
> > Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> > ---
> >  drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
> >  include/linux/soc/mediatek/infracfg.h |  3 +++
> >  2 files changed, 24 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > index 2be96f1cdbd2..16765f532853 100644
> > --- a/drivers/iommu/mtk_iommu.c
> > +++ b/drivers/iommu/mtk_iommu.c
> > @@ -3,7 +3,6 @@
> >   * Copyright (c) 2015-2016 MediaTek Inc.
> >   * Author: Yong Wu <yong.wu@mediatek.com>
> >   */
> > -#include <linux/memblock.h>
> >  #include <linux/bug.h>
> >  #include <linux/clk.h>
> >  #include <linux/component.h>
> > @@ -15,13 +14,16 @@
> >  #include <linux/iommu.h>
> >  #include <linux/iopoll.h>
> >  #include <linux/list.h>
> > +#include <linux/mfd/syscon.h>
> >  #include <linux/of_address.h>
> >  #include <linux/of_iommu.h>
> >  #include <linux/of_irq.h>
> >  #include <linux/of_platform.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> >  #include <linux/slab.h>
> >  #include <linux/spinlock.h>
> > +#include <linux/soc/mediatek/infracfg.h>
> >  #include <asm/barrier.h>
> >  #include <soc/mediatek/smi.h>
> >  
> > @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >  	struct resource         *res;
> >  	resource_size_t		ioaddr;
> >  	struct component_match  *match = NULL;
> > +	struct regmap		*infracfg;
> >  	void                    *protect;
> >  	int                     i, larb_nr, ret;
> > +	u32			val;
> >  
> >  	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> >  	if (!data)
> > @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >  		return -ENOMEM;
> >  	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
> >  
> > -	/* Whether the current dram is over 4GB */
> > -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
> > -	if (!data->plat_data->has_4gb_mode)
> > -		data->enable_4GB = false;
> > +	data->enable_4GB = false;
> > +	if (data->plat_data->has_4gb_mode) {
> > +		infracfg = syscon_regmap_lookup_by_compatible(
> > +				"mediatek,mt8173-infracfg");
> > +		if (IS_ERR(infracfg)) {
> > +			infracfg = syscon_regmap_lookup_by_compatible(
> > +					"mediatek,mt2712-infracfg");
> > +			if (IS_ERR(infracfg))
> > +				return PTR_ERR(infracfg);
> > +
> > +		}
> > +		ret = regmap_read(infracfg, REG_INFRA_MISC, &val);
> > +		if (ret)
> > +			return ret;
> > +		data->enable_4GB = !!(val & F_DDR_4GB_SUPPORT_EN);
> 
> (I am absolutely not familiar with syscon_regmap_lookup_by ..., I am
> missing some context, so sorry for the stupid questions)
> 
> Who sets the regmap value and based on what? Who decides that 4GB mode
> is supported or not? And who decides if 4GB mode is *required* or not?
> 

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

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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
@ 2020-07-22  7:15     ` Miles Chen
  0 siblings, 0 replies; 32+ messages in thread
From: Miles Chen @ 2020-07-22  7:15 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Rob Herring, wsd_upstream, Joerg Roedel, linux-kernel,
	Mike Rapoport, Chao Hao, iommu, linux-mediatek, Yong Wu,
	Matthias Brugger, Yingjoe Chen, Christoph Hellwig,
	linux-arm-kernel

On Tue, 2020-07-21 at 11:10 +0200, David Hildenbrand wrote:
> On 21.07.20 04:16, Miles Chen wrote:
> > In previous discussion [1] and [2], we found that it is risky to
> > use max_pfn or totalram_pages to tell if 4GB mode is enabled.
> > 
> > Check 4GB mode by reading infracfg register, remove the usage
> > of the un-exported symbol max_pfn.
> > 
> > This is a step towards building mtk_iommu as a kernel module.
> > 
> > Change since v1:
> > 1. remove the phandle usage, search for infracfg instead [3]
> > 2. use infracfg instead of infracfg_regmap
> > 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
> > 4. update enable_4GB only when has_4gb_mode
> 
> Nit: We tend to place such information below the "---" (adding a second
> one) such that this information is discarded when the patch is picked up.

Thanks, I'll add a "---" before the "Change since..." in patch v3.
> 
> > 
> > [1] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/3/733__;!!CTRNKA9wMg0ARbw!wrP2H0azuZkTHSnM_ETki4kT_FMF0Cl7abQX8tIHX0iTr65JkvMCe4jDt84d_YhBrA$ 
> > [2] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/4/136__;!!CTRNKA9wMg0ARbw!wrP2H0azuZkTHSnM_ETki4kT_FMF0Cl7abQX8tIHX0iTr65JkvMCe4jDt86IGhmouQ$ 
> > [3] https://urldefense.com/v3/__https://lkml.org/lkml/2020/7/15/1147__;!!CTRNKA9wMg0ARbw!wrP2H0azuZkTHSnM_ETki4kT_FMF0Cl7abQX8tIHX0iTr65JkvMCe4jDt855z4xdqw$ 
> > 
> > Cc: Mike Rapoport <rppt@linux.ibm.com>
> > Cc: David Hildenbrand <david@redhat.com>
> > Cc: Yong Wu <yong.wu@mediatek.com>
> > Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Yong Wu <yong.wu@mediatek.com>
> > Cc: Chao Hao <chao.hao@mediatek.com>
> > Cc: Rob Herring <robh@kernel.org>
> > Cc: Matthias Brugger <matthias.bgg@gmail.com>
> > Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> > ---
> >  drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
> >  include/linux/soc/mediatek/infracfg.h |  3 +++
> >  2 files changed, 24 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > index 2be96f1cdbd2..16765f532853 100644
> > --- a/drivers/iommu/mtk_iommu.c
> > +++ b/drivers/iommu/mtk_iommu.c
> > @@ -3,7 +3,6 @@
> >   * Copyright (c) 2015-2016 MediaTek Inc.
> >   * Author: Yong Wu <yong.wu@mediatek.com>
> >   */
> > -#include <linux/memblock.h>
> >  #include <linux/bug.h>
> >  #include <linux/clk.h>
> >  #include <linux/component.h>
> > @@ -15,13 +14,16 @@
> >  #include <linux/iommu.h>
> >  #include <linux/iopoll.h>
> >  #include <linux/list.h>
> > +#include <linux/mfd/syscon.h>
> >  #include <linux/of_address.h>
> >  #include <linux/of_iommu.h>
> >  #include <linux/of_irq.h>
> >  #include <linux/of_platform.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> >  #include <linux/slab.h>
> >  #include <linux/spinlock.h>
> > +#include <linux/soc/mediatek/infracfg.h>
> >  #include <asm/barrier.h>
> >  #include <soc/mediatek/smi.h>
> >  
> > @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >  	struct resource         *res;
> >  	resource_size_t		ioaddr;
> >  	struct component_match  *match = NULL;
> > +	struct regmap		*infracfg;
> >  	void                    *protect;
> >  	int                     i, larb_nr, ret;
> > +	u32			val;
> >  
> >  	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> >  	if (!data)
> > @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >  		return -ENOMEM;
> >  	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
> >  
> > -	/* Whether the current dram is over 4GB */
> > -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
> > -	if (!data->plat_data->has_4gb_mode)
> > -		data->enable_4GB = false;
> > +	data->enable_4GB = false;
> > +	if (data->plat_data->has_4gb_mode) {
> > +		infracfg = syscon_regmap_lookup_by_compatible(
> > +				"mediatek,mt8173-infracfg");
> > +		if (IS_ERR(infracfg)) {
> > +			infracfg = syscon_regmap_lookup_by_compatible(
> > +					"mediatek,mt2712-infracfg");
> > +			if (IS_ERR(infracfg))
> > +				return PTR_ERR(infracfg);
> > +
> > +		}
> > +		ret = regmap_read(infracfg, REG_INFRA_MISC, &val);
> > +		if (ret)
> > +			return ret;
> > +		data->enable_4GB = !!(val & F_DDR_4GB_SUPPORT_EN);
> 
> (I am absolutely not familiar with syscon_regmap_lookup_by ..., I am
> missing some context, so sorry for the stupid questions)
> 
> Who sets the regmap value and based on what? Who decides that 4GB mode
> is supported or not? And who decides if 4GB mode is *required* or not?
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
  2020-07-21 21:19       ` Matthias Brugger
  (?)
  (?)
@ 2020-07-22  7:17         ` Miles Chen
  -1 siblings, 0 replies; 32+ messages in thread
From: Miles Chen @ 2020-07-22  7:17 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Yong Wu, Joerg Roedel, Rob Herring, iommu, linux-arm-kernel,
	linux-mediatek, linux-kernel, wsd_upstream, Mike Rapoport,
	David Hildenbrand, Yingjoe Chen, Christoph Hellwig, Chao Hao

On Tue, 2020-07-21 at 23:19 +0200, Matthias Brugger wrote:
> 
> On 21/07/2020 13:24, Yong Wu wrote:
> > On Tue, 2020-07-21 at 11:40 +0200, Matthias Brugger wrote:
> >>
> >> On 21/07/2020 04:16, Miles Chen wrote:
> >>> In previous discussion [1] and [2], we found that it is risky to
> >>> use max_pfn or totalram_pages to tell if 4GB mode is enabled.
> >>>
> >>> Check 4GB mode by reading infracfg register, remove the usage
> >>> of the un-exported symbol max_pfn.
> >>>
> >>> This is a step towards building mtk_iommu as a kernel module.
> >>>
> >>> Change since v1:
> >>> 1. remove the phandle usage, search for infracfg instead [3]
> >>> 2. use infracfg instead of infracfg_regmap
> >>> 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
> >>> 4. update enable_4GB only when has_4gb_mode
> >>>
> >>> [1] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/3/733__;!!CTRNKA9wMg0ARbw!w5YjY83YRL9_ijgXHwB1x2Dnb5BqiFUI8H5IAyAWWFMvUJKI9Qbj_zta2AaiFZejiQ$ 
> >>> [2] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/4/136__;!!CTRNKA9wMg0ARbw!w5YjY83YRL9_ijgXHwB1x2Dnb5BqiFUI8H5IAyAWWFMvUJKI9Qbj_zta2Aa9U2yQyg$ 
> >>> [3] https://urldefense.com/v3/__https://lkml.org/lkml/2020/7/15/1147__;!!CTRNKA9wMg0ARbw!w5YjY83YRL9_ijgXHwB1x2Dnb5BqiFUI8H5IAyAWWFMvUJKI9Qbj_zta2Aaxpk_Wjw$ 
> >>>
> >>> Cc: Mike Rapoport <rppt@linux.ibm.com>
> >>> Cc: David Hildenbrand <david@redhat.com>
> >>> Cc: Yong Wu <yong.wu@mediatek.com>
> >>> Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
> >>> Cc: Christoph Hellwig <hch@lst.de>
> >>> Cc: Yong Wu <yong.wu@mediatek.com>
> >>> Cc: Chao Hao <chao.hao@mediatek.com>
> >>> Cc: Rob Herring <robh@kernel.org>
> >>> Cc: Matthias Brugger <matthias.bgg@gmail.com>
> >>> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> >>> ---
> >>>    drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
> >>>    include/linux/soc/mediatek/infracfg.h |  3 +++
> >>>    2 files changed, 24 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> >>> index 2be96f1cdbd2..16765f532853 100644
> >>> --- a/drivers/iommu/mtk_iommu.c
> >>> +++ b/drivers/iommu/mtk_iommu.c
> >>> @@ -3,7 +3,6 @@
> >>>     * Copyright (c) 2015-2016 MediaTek Inc.
> >>>     * Author: Yong Wu <yong.wu@mediatek.com>
> >>>     */
> >>> -#include <linux/memblock.h>
> >>>    #include <linux/bug.h>
> >>>    #include <linux/clk.h>
> >>>    #include <linux/component.h>
> >>> @@ -15,13 +14,16 @@
> >>>    #include <linux/iommu.h>
> >>>    #include <linux/iopoll.h>
> >>>    #include <linux/list.h>
> >>> +#include <linux/mfd/syscon.h>
> >>>    #include <linux/of_address.h>
> >>>    #include <linux/of_iommu.h>
> >>>    #include <linux/of_irq.h>
> >>>    #include <linux/of_platform.h>
> >>>    #include <linux/platform_device.h>
> >>> +#include <linux/regmap.h>
> >>>    #include <linux/slab.h>
> >>>    #include <linux/spinlock.h>
> >>> +#include <linux/soc/mediatek/infracfg.h>
> >>>    #include <asm/barrier.h>
> >>>    #include <soc/mediatek/smi.h>
> >>>    
> >>> @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >>>    	struct resource         *res;
> >>>    	resource_size_t		ioaddr;
> >>>    	struct component_match  *match = NULL;
> >>> +	struct regmap		*infracfg;
> >>>    	void                    *protect;
> >>>    	int                     i, larb_nr, ret;
> >>> +	u32			val;
> >>>    
> >>>    	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> >>>    	if (!data)
> >>> @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >>>    		return -ENOMEM;
> >>>    	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
> >>>    
> >>> -	/* Whether the current dram is over 4GB */
> >>> -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
> >>> -	if (!data->plat_data->has_4gb_mode)
> >>> -		data->enable_4GB = false;
> >>> +	data->enable_4GB = false;
> >>> +	if (data->plat_data->has_4gb_mode) {
> >>> +		infracfg = syscon_regmap_lookup_by_compatible(
> >>> +				"mediatek,mt8173-infracfg");
> >>> +		if (IS_ERR(infracfg)) {
> >>> +			infracfg = syscon_regmap_lookup_by_compatible(
> >>> +					"mediatek,mt2712-infracfg");
> >>> +			if (IS_ERR(infracfg))
> >>> +				return PTR_ERR(infracfg);
> >>
> >> I think we should check m4u_plat instead to decide which compatible we have to
> >> look for.
> >> Another option would be to add a general compatible something like
> >> "mtk-infracfg" and search for that. That would need an update of all DTS having
> >> a infracfg compatible right now. After thinking twice, this would break newer
> >> kernel with older device tree, so maybe it's better to go with m4u_plat switch
> >> statement.
> > 
> > Add a "char *infracfg" in the plat_data, Use the mt2712, mt8173
> > corresponding string in it. If it is NULL, It means the "enable_4GB"
> > always is false. Then we also can remove the flag "has_4gb_mode".
> > 
> > is this OK?
> > 
> 
> It's an option, but I personally find that a bit hacky.

Thanks Yong and Matthias for your comment.
I will try adding a char *infracfg in patch v3.


> 
> Regards,
> Matthias


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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
@ 2020-07-22  7:17         ` Miles Chen
  0 siblings, 0 replies; 32+ messages in thread
From: Miles Chen @ 2020-07-22  7:17 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Rob Herring, wsd_upstream, David Hildenbrand, linux-kernel,
	Mike Rapoport, Chao Hao, iommu, linux-mediatek, Yingjoe Chen,
	Christoph Hellwig, linux-arm-kernel

On Tue, 2020-07-21 at 23:19 +0200, Matthias Brugger wrote:
> 
> On 21/07/2020 13:24, Yong Wu wrote:
> > On Tue, 2020-07-21 at 11:40 +0200, Matthias Brugger wrote:
> >>
> >> On 21/07/2020 04:16, Miles Chen wrote:
> >>> In previous discussion [1] and [2], we found that it is risky to
> >>> use max_pfn or totalram_pages to tell if 4GB mode is enabled.
> >>>
> >>> Check 4GB mode by reading infracfg register, remove the usage
> >>> of the un-exported symbol max_pfn.
> >>>
> >>> This is a step towards building mtk_iommu as a kernel module.
> >>>
> >>> Change since v1:
> >>> 1. remove the phandle usage, search for infracfg instead [3]
> >>> 2. use infracfg instead of infracfg_regmap
> >>> 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
> >>> 4. update enable_4GB only when has_4gb_mode
> >>>
> >>> [1] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/3/733__;!!CTRNKA9wMg0ARbw!w5YjY83YRL9_ijgXHwB1x2Dnb5BqiFUI8H5IAyAWWFMvUJKI9Qbj_zta2AaiFZejiQ$ 
> >>> [2] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/4/136__;!!CTRNKA9wMg0ARbw!w5YjY83YRL9_ijgXHwB1x2Dnb5BqiFUI8H5IAyAWWFMvUJKI9Qbj_zta2Aa9U2yQyg$ 
> >>> [3] https://urldefense.com/v3/__https://lkml.org/lkml/2020/7/15/1147__;!!CTRNKA9wMg0ARbw!w5YjY83YRL9_ijgXHwB1x2Dnb5BqiFUI8H5IAyAWWFMvUJKI9Qbj_zta2Aaxpk_Wjw$ 
> >>>
> >>> Cc: Mike Rapoport <rppt@linux.ibm.com>
> >>> Cc: David Hildenbrand <david@redhat.com>
> >>> Cc: Yong Wu <yong.wu@mediatek.com>
> >>> Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
> >>> Cc: Christoph Hellwig <hch@lst.de>
> >>> Cc: Yong Wu <yong.wu@mediatek.com>
> >>> Cc: Chao Hao <chao.hao@mediatek.com>
> >>> Cc: Rob Herring <robh@kernel.org>
> >>> Cc: Matthias Brugger <matthias.bgg@gmail.com>
> >>> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> >>> ---
> >>>    drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
> >>>    include/linux/soc/mediatek/infracfg.h |  3 +++
> >>>    2 files changed, 24 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> >>> index 2be96f1cdbd2..16765f532853 100644
> >>> --- a/drivers/iommu/mtk_iommu.c
> >>> +++ b/drivers/iommu/mtk_iommu.c
> >>> @@ -3,7 +3,6 @@
> >>>     * Copyright (c) 2015-2016 MediaTek Inc.
> >>>     * Author: Yong Wu <yong.wu@mediatek.com>
> >>>     */
> >>> -#include <linux/memblock.h>
> >>>    #include <linux/bug.h>
> >>>    #include <linux/clk.h>
> >>>    #include <linux/component.h>
> >>> @@ -15,13 +14,16 @@
> >>>    #include <linux/iommu.h>
> >>>    #include <linux/iopoll.h>
> >>>    #include <linux/list.h>
> >>> +#include <linux/mfd/syscon.h>
> >>>    #include <linux/of_address.h>
> >>>    #include <linux/of_iommu.h>
> >>>    #include <linux/of_irq.h>
> >>>    #include <linux/of_platform.h>
> >>>    #include <linux/platform_device.h>
> >>> +#include <linux/regmap.h>
> >>>    #include <linux/slab.h>
> >>>    #include <linux/spinlock.h>
> >>> +#include <linux/soc/mediatek/infracfg.h>
> >>>    #include <asm/barrier.h>
> >>>    #include <soc/mediatek/smi.h>
> >>>    
> >>> @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >>>    	struct resource         *res;
> >>>    	resource_size_t		ioaddr;
> >>>    	struct component_match  *match = NULL;
> >>> +	struct regmap		*infracfg;
> >>>    	void                    *protect;
> >>>    	int                     i, larb_nr, ret;
> >>> +	u32			val;
> >>>    
> >>>    	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> >>>    	if (!data)
> >>> @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >>>    		return -ENOMEM;
> >>>    	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
> >>>    
> >>> -	/* Whether the current dram is over 4GB */
> >>> -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
> >>> -	if (!data->plat_data->has_4gb_mode)
> >>> -		data->enable_4GB = false;
> >>> +	data->enable_4GB = false;
> >>> +	if (data->plat_data->has_4gb_mode) {
> >>> +		infracfg = syscon_regmap_lookup_by_compatible(
> >>> +				"mediatek,mt8173-infracfg");
> >>> +		if (IS_ERR(infracfg)) {
> >>> +			infracfg = syscon_regmap_lookup_by_compatible(
> >>> +					"mediatek,mt2712-infracfg");
> >>> +			if (IS_ERR(infracfg))
> >>> +				return PTR_ERR(infracfg);
> >>
> >> I think we should check m4u_plat instead to decide which compatible we have to
> >> look for.
> >> Another option would be to add a general compatible something like
> >> "mtk-infracfg" and search for that. That would need an update of all DTS having
> >> a infracfg compatible right now. After thinking twice, this would break newer
> >> kernel with older device tree, so maybe it's better to go with m4u_plat switch
> >> statement.
> > 
> > Add a "char *infracfg" in the plat_data, Use the mt2712, mt8173
> > corresponding string in it. If it is NULL, It means the "enable_4GB"
> > always is false. Then we also can remove the flag "has_4gb_mode".
> > 
> > is this OK?
> > 
> 
> It's an option, but I personally find that a bit hacky.

Thanks Yong and Matthias for your comment.
I will try adding a char *infracfg in patch v3.


> 
> Regards,
> Matthias

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
@ 2020-07-22  7:17         ` Miles Chen
  0 siblings, 0 replies; 32+ messages in thread
From: Miles Chen @ 2020-07-22  7:17 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Rob Herring, wsd_upstream, David Hildenbrand, Joerg Roedel,
	linux-kernel, Mike Rapoport, Chao Hao, iommu, linux-mediatek,
	Yong Wu, Yingjoe Chen, Christoph Hellwig, linux-arm-kernel

On Tue, 2020-07-21 at 23:19 +0200, Matthias Brugger wrote:
> 
> On 21/07/2020 13:24, Yong Wu wrote:
> > On Tue, 2020-07-21 at 11:40 +0200, Matthias Brugger wrote:
> >>
> >> On 21/07/2020 04:16, Miles Chen wrote:
> >>> In previous discussion [1] and [2], we found that it is risky to
> >>> use max_pfn or totalram_pages to tell if 4GB mode is enabled.
> >>>
> >>> Check 4GB mode by reading infracfg register, remove the usage
> >>> of the un-exported symbol max_pfn.
> >>>
> >>> This is a step towards building mtk_iommu as a kernel module.
> >>>
> >>> Change since v1:
> >>> 1. remove the phandle usage, search for infracfg instead [3]
> >>> 2. use infracfg instead of infracfg_regmap
> >>> 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
> >>> 4. update enable_4GB only when has_4gb_mode
> >>>
> >>> [1] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/3/733__;!!CTRNKA9wMg0ARbw!w5YjY83YRL9_ijgXHwB1x2Dnb5BqiFUI8H5IAyAWWFMvUJKI9Qbj_zta2AaiFZejiQ$ 
> >>> [2] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/4/136__;!!CTRNKA9wMg0ARbw!w5YjY83YRL9_ijgXHwB1x2Dnb5BqiFUI8H5IAyAWWFMvUJKI9Qbj_zta2Aa9U2yQyg$ 
> >>> [3] https://urldefense.com/v3/__https://lkml.org/lkml/2020/7/15/1147__;!!CTRNKA9wMg0ARbw!w5YjY83YRL9_ijgXHwB1x2Dnb5BqiFUI8H5IAyAWWFMvUJKI9Qbj_zta2Aaxpk_Wjw$ 
> >>>
> >>> Cc: Mike Rapoport <rppt@linux.ibm.com>
> >>> Cc: David Hildenbrand <david@redhat.com>
> >>> Cc: Yong Wu <yong.wu@mediatek.com>
> >>> Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
> >>> Cc: Christoph Hellwig <hch@lst.de>
> >>> Cc: Yong Wu <yong.wu@mediatek.com>
> >>> Cc: Chao Hao <chao.hao@mediatek.com>
> >>> Cc: Rob Herring <robh@kernel.org>
> >>> Cc: Matthias Brugger <matthias.bgg@gmail.com>
> >>> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> >>> ---
> >>>    drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
> >>>    include/linux/soc/mediatek/infracfg.h |  3 +++
> >>>    2 files changed, 24 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> >>> index 2be96f1cdbd2..16765f532853 100644
> >>> --- a/drivers/iommu/mtk_iommu.c
> >>> +++ b/drivers/iommu/mtk_iommu.c
> >>> @@ -3,7 +3,6 @@
> >>>     * Copyright (c) 2015-2016 MediaTek Inc.
> >>>     * Author: Yong Wu <yong.wu@mediatek.com>
> >>>     */
> >>> -#include <linux/memblock.h>
> >>>    #include <linux/bug.h>
> >>>    #include <linux/clk.h>
> >>>    #include <linux/component.h>
> >>> @@ -15,13 +14,16 @@
> >>>    #include <linux/iommu.h>
> >>>    #include <linux/iopoll.h>
> >>>    #include <linux/list.h>
> >>> +#include <linux/mfd/syscon.h>
> >>>    #include <linux/of_address.h>
> >>>    #include <linux/of_iommu.h>
> >>>    #include <linux/of_irq.h>
> >>>    #include <linux/of_platform.h>
> >>>    #include <linux/platform_device.h>
> >>> +#include <linux/regmap.h>
> >>>    #include <linux/slab.h>
> >>>    #include <linux/spinlock.h>
> >>> +#include <linux/soc/mediatek/infracfg.h>
> >>>    #include <asm/barrier.h>
> >>>    #include <soc/mediatek/smi.h>
> >>>    
> >>> @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >>>    	struct resource         *res;
> >>>    	resource_size_t		ioaddr;
> >>>    	struct component_match  *match = NULL;
> >>> +	struct regmap		*infracfg;
> >>>    	void                    *protect;
> >>>    	int                     i, larb_nr, ret;
> >>> +	u32			val;
> >>>    
> >>>    	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> >>>    	if (!data)
> >>> @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >>>    		return -ENOMEM;
> >>>    	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
> >>>    
> >>> -	/* Whether the current dram is over 4GB */
> >>> -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
> >>> -	if (!data->plat_data->has_4gb_mode)
> >>> -		data->enable_4GB = false;
> >>> +	data->enable_4GB = false;
> >>> +	if (data->plat_data->has_4gb_mode) {
> >>> +		infracfg = syscon_regmap_lookup_by_compatible(
> >>> +				"mediatek,mt8173-infracfg");
> >>> +		if (IS_ERR(infracfg)) {
> >>> +			infracfg = syscon_regmap_lookup_by_compatible(
> >>> +					"mediatek,mt2712-infracfg");
> >>> +			if (IS_ERR(infracfg))
> >>> +				return PTR_ERR(infracfg);
> >>
> >> I think we should check m4u_plat instead to decide which compatible we have to
> >> look for.
> >> Another option would be to add a general compatible something like
> >> "mtk-infracfg" and search for that. That would need an update of all DTS having
> >> a infracfg compatible right now. After thinking twice, this would break newer
> >> kernel with older device tree, so maybe it's better to go with m4u_plat switch
> >> statement.
> > 
> > Add a "char *infracfg" in the plat_data, Use the mt2712, mt8173
> > corresponding string in it. If it is NULL, It means the "enable_4GB"
> > always is false. Then we also can remove the flag "has_4gb_mode".
> > 
> > is this OK?
> > 
> 
> It's an option, but I personally find that a bit hacky.

Thanks Yong and Matthias for your comment.
I will try adding a char *infracfg in patch v3.


> 
> Regards,
> Matthias

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

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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
@ 2020-07-22  7:17         ` Miles Chen
  0 siblings, 0 replies; 32+ messages in thread
From: Miles Chen @ 2020-07-22  7:17 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Rob Herring, wsd_upstream, David Hildenbrand, Joerg Roedel,
	linux-kernel, Mike Rapoport, Chao Hao, iommu, linux-mediatek,
	Yong Wu, Yingjoe Chen, Christoph Hellwig, linux-arm-kernel

On Tue, 2020-07-21 at 23:19 +0200, Matthias Brugger wrote:
> 
> On 21/07/2020 13:24, Yong Wu wrote:
> > On Tue, 2020-07-21 at 11:40 +0200, Matthias Brugger wrote:
> >>
> >> On 21/07/2020 04:16, Miles Chen wrote:
> >>> In previous discussion [1] and [2], we found that it is risky to
> >>> use max_pfn or totalram_pages to tell if 4GB mode is enabled.
> >>>
> >>> Check 4GB mode by reading infracfg register, remove the usage
> >>> of the un-exported symbol max_pfn.
> >>>
> >>> This is a step towards building mtk_iommu as a kernel module.
> >>>
> >>> Change since v1:
> >>> 1. remove the phandle usage, search for infracfg instead [3]
> >>> 2. use infracfg instead of infracfg_regmap
> >>> 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
> >>> 4. update enable_4GB only when has_4gb_mode
> >>>
> >>> [1] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/3/733__;!!CTRNKA9wMg0ARbw!w5YjY83YRL9_ijgXHwB1x2Dnb5BqiFUI8H5IAyAWWFMvUJKI9Qbj_zta2AaiFZejiQ$ 
> >>> [2] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/4/136__;!!CTRNKA9wMg0ARbw!w5YjY83YRL9_ijgXHwB1x2Dnb5BqiFUI8H5IAyAWWFMvUJKI9Qbj_zta2Aa9U2yQyg$ 
> >>> [3] https://urldefense.com/v3/__https://lkml.org/lkml/2020/7/15/1147__;!!CTRNKA9wMg0ARbw!w5YjY83YRL9_ijgXHwB1x2Dnb5BqiFUI8H5IAyAWWFMvUJKI9Qbj_zta2Aaxpk_Wjw$ 
> >>>
> >>> Cc: Mike Rapoport <rppt@linux.ibm.com>
> >>> Cc: David Hildenbrand <david@redhat.com>
> >>> Cc: Yong Wu <yong.wu@mediatek.com>
> >>> Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
> >>> Cc: Christoph Hellwig <hch@lst.de>
> >>> Cc: Yong Wu <yong.wu@mediatek.com>
> >>> Cc: Chao Hao <chao.hao@mediatek.com>
> >>> Cc: Rob Herring <robh@kernel.org>
> >>> Cc: Matthias Brugger <matthias.bgg@gmail.com>
> >>> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> >>> ---
> >>>    drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
> >>>    include/linux/soc/mediatek/infracfg.h |  3 +++
> >>>    2 files changed, 24 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> >>> index 2be96f1cdbd2..16765f532853 100644
> >>> --- a/drivers/iommu/mtk_iommu.c
> >>> +++ b/drivers/iommu/mtk_iommu.c
> >>> @@ -3,7 +3,6 @@
> >>>     * Copyright (c) 2015-2016 MediaTek Inc.
> >>>     * Author: Yong Wu <yong.wu@mediatek.com>
> >>>     */
> >>> -#include <linux/memblock.h>
> >>>    #include <linux/bug.h>
> >>>    #include <linux/clk.h>
> >>>    #include <linux/component.h>
> >>> @@ -15,13 +14,16 @@
> >>>    #include <linux/iommu.h>
> >>>    #include <linux/iopoll.h>
> >>>    #include <linux/list.h>
> >>> +#include <linux/mfd/syscon.h>
> >>>    #include <linux/of_address.h>
> >>>    #include <linux/of_iommu.h>
> >>>    #include <linux/of_irq.h>
> >>>    #include <linux/of_platform.h>
> >>>    #include <linux/platform_device.h>
> >>> +#include <linux/regmap.h>
> >>>    #include <linux/slab.h>
> >>>    #include <linux/spinlock.h>
> >>> +#include <linux/soc/mediatek/infracfg.h>
> >>>    #include <asm/barrier.h>
> >>>    #include <soc/mediatek/smi.h>
> >>>    
> >>> @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >>>    	struct resource         *res;
> >>>    	resource_size_t		ioaddr;
> >>>    	struct component_match  *match = NULL;
> >>> +	struct regmap		*infracfg;
> >>>    	void                    *protect;
> >>>    	int                     i, larb_nr, ret;
> >>> +	u32			val;
> >>>    
> >>>    	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> >>>    	if (!data)
> >>> @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> >>>    		return -ENOMEM;
> >>>    	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
> >>>    
> >>> -	/* Whether the current dram is over 4GB */
> >>> -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
> >>> -	if (!data->plat_data->has_4gb_mode)
> >>> -		data->enable_4GB = false;
> >>> +	data->enable_4GB = false;
> >>> +	if (data->plat_data->has_4gb_mode) {
> >>> +		infracfg = syscon_regmap_lookup_by_compatible(
> >>> +				"mediatek,mt8173-infracfg");
> >>> +		if (IS_ERR(infracfg)) {
> >>> +			infracfg = syscon_regmap_lookup_by_compatible(
> >>> +					"mediatek,mt2712-infracfg");
> >>> +			if (IS_ERR(infracfg))
> >>> +				return PTR_ERR(infracfg);
> >>
> >> I think we should check m4u_plat instead to decide which compatible we have to
> >> look for.
> >> Another option would be to add a general compatible something like
> >> "mtk-infracfg" and search for that. That would need an update of all DTS having
> >> a infracfg compatible right now. After thinking twice, this would break newer
> >> kernel with older device tree, so maybe it's better to go with m4u_plat switch
> >> statement.
> > 
> > Add a "char *infracfg" in the plat_data, Use the mt2712, mt8173
> > corresponding string in it. If it is NULL, It means the "enable_4GB"
> > always is false. Then we also can remove the flag "has_4gb_mode".
> > 
> > is this OK?
> > 
> 
> It's an option, but I personally find that a bit hacky.

Thanks Yong and Matthias for your comment.
I will try adding a char *infracfg in patch v3.


> 
> Regards,
> Matthias

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
  2020-07-22  7:17         ` Miles Chen
  (?)
  (?)
@ 2020-07-22  9:37           ` Miles Chen
  -1 siblings, 0 replies; 32+ messages in thread
From: Miles Chen @ 2020-07-22  9:37 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Yong Wu, Joerg Roedel, Rob Herring, iommu, linux-arm-kernel,
	linux-mediatek, linux-kernel, wsd_upstream, Mike Rapoport,
	David Hildenbrand, Yingjoe Chen, Christoph Hellwig, Chao Hao

On Wed, 2020-07-22 at 15:17 +0800, Miles Chen wrote:
> On Tue, 2020-07-21 at 23:19 +0200, Matthias Brugger wrote:
> > 
> > On 21/07/2020 13:24, Yong Wu wrote:
> > > On Tue, 2020-07-21 at 11:40 +0200, Matthias Brugger wrote:
> > >>
> > >> On 21/07/2020 04:16, Miles Chen wrote:
> > >>> In previous discussion [1] and [2], we found that it is risky to
> > >>> use max_pfn or totalram_pages to tell if 4GB mode is enabled.
> > >>>
> > >>> Check 4GB mode by reading infracfg register, remove the usage
> > >>> of the un-exported symbol max_pfn.
> > >>>
> > >>> This is a step towards building mtk_iommu as a kernel module.
> > >>>
> > >>> Change since v1:
> > >>> 1. remove the phandle usage, search for infracfg instead [3]
> > >>> 2. use infracfg instead of infracfg_regmap
> > >>> 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
> > >>> 4. update enable_4GB only when has_4gb_mode
> > >>>
> > >>> [1] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/3/733__;!!CTRNKA9wMg0ARbw!w5YjY83YRL9_ijgXHwB1x2Dnb5BqiFUI8H5IAyAWWFMvUJKI9Qbj_zta2AaiFZejiQ$ 
> > >>> [2] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/4/136__;!!CTRNKA9wMg0ARbw!w5YjY83YRL9_ijgXHwB1x2Dnb5BqiFUI8H5IAyAWWFMvUJKI9Qbj_zta2Aa9U2yQyg$ 
> > >>> [3] https://urldefense.com/v3/__https://lkml.org/lkml/2020/7/15/1147__;!!CTRNKA9wMg0ARbw!w5YjY83YRL9_ijgXHwB1x2Dnb5BqiFUI8H5IAyAWWFMvUJKI9Qbj_zta2Aaxpk_Wjw$ 
> > >>>
> > >>> Cc: Mike Rapoport <rppt@linux.ibm.com>
> > >>> Cc: David Hildenbrand <david@redhat.com>
> > >>> Cc: Yong Wu <yong.wu@mediatek.com>
> > >>> Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
> > >>> Cc: Christoph Hellwig <hch@lst.de>
> > >>> Cc: Yong Wu <yong.wu@mediatek.com>
> > >>> Cc: Chao Hao <chao.hao@mediatek.com>
> > >>> Cc: Rob Herring <robh@kernel.org>
> > >>> Cc: Matthias Brugger <matthias.bgg@gmail.com>
> > >>> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> > >>> ---
> > >>>    drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
> > >>>    include/linux/soc/mediatek/infracfg.h |  3 +++
> > >>>    2 files changed, 24 insertions(+), 5 deletions(-)
> > >>>
> > >>> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > >>> index 2be96f1cdbd2..16765f532853 100644
> > >>> --- a/drivers/iommu/mtk_iommu.c
> > >>> +++ b/drivers/iommu/mtk_iommu.c
> > >>> @@ -3,7 +3,6 @@
> > >>>     * Copyright (c) 2015-2016 MediaTek Inc.
> > >>>     * Author: Yong Wu <yong.wu@mediatek.com>
> > >>>     */
> > >>> -#include <linux/memblock.h>
> > >>>    #include <linux/bug.h>
> > >>>    #include <linux/clk.h>
> > >>>    #include <linux/component.h>
> > >>> @@ -15,13 +14,16 @@
> > >>>    #include <linux/iommu.h>
> > >>>    #include <linux/iopoll.h>
> > >>>    #include <linux/list.h>
> > >>> +#include <linux/mfd/syscon.h>
> > >>>    #include <linux/of_address.h>
> > >>>    #include <linux/of_iommu.h>
> > >>>    #include <linux/of_irq.h>
> > >>>    #include <linux/of_platform.h>
> > >>>    #include <linux/platform_device.h>
> > >>> +#include <linux/regmap.h>
> > >>>    #include <linux/slab.h>
> > >>>    #include <linux/spinlock.h>
> > >>> +#include <linux/soc/mediatek/infracfg.h>
> > >>>    #include <asm/barrier.h>
> > >>>    #include <soc/mediatek/smi.h>
> > >>>    
> > >>> @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> > >>>    	struct resource         *res;
> > >>>    	resource_size_t		ioaddr;
> > >>>    	struct component_match  *match = NULL;
> > >>> +	struct regmap		*infracfg;
> > >>>    	void                    *protect;
> > >>>    	int                     i, larb_nr, ret;
> > >>> +	u32			val;
> > >>>    
> > >>>    	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> > >>>    	if (!data)
> > >>> @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> > >>>    		return -ENOMEM;
> > >>>    	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
> > >>>    
> > >>> -	/* Whether the current dram is over 4GB */
> > >>> -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
> > >>> -	if (!data->plat_data->has_4gb_mode)
> > >>> -		data->enable_4GB = false;
> > >>> +	data->enable_4GB = false;
> > >>> +	if (data->plat_data->has_4gb_mode) {
> > >>> +		infracfg = syscon_regmap_lookup_by_compatible(
> > >>> +				"mediatek,mt8173-infracfg");
> > >>> +		if (IS_ERR(infracfg)) {
> > >>> +			infracfg = syscon_regmap_lookup_by_compatible(
> > >>> +					"mediatek,mt2712-infracfg");
> > >>> +			if (IS_ERR(infracfg))
> > >>> +				return PTR_ERR(infracfg);
> > >>
> > >> I think we should check m4u_plat instead to decide which compatible we have to
> > >> look for.
> > >> Another option would be to add a general compatible something like
> > >> "mtk-infracfg" and search for that. That would need an update of all DTS having
> > >> a infracfg compatible right now. After thinking twice, this would break newer
> > >> kernel with older device tree, so maybe it's better to go with m4u_plat switch
> > >> statement.
> > > 
> > > Add a "char *infracfg" in the plat_data, Use the mt2712, mt8173
> > > corresponding string in it. If it is NULL, It means the "enable_4GB"
> > > always is false. Then we also can remove the flag "has_4gb_mode".
> > > 
> > > is this OK?
> > > 
> > 
> > It's an option, but I personally find that a bit hacky.
> 
> Thanks Yong and Matthias for your comment.
> I will try adding a char *infracfg in patch v3.
> 

I misunderstood the comment.
I should check m4u_plat to decide which string to look for, not adding a
char *infracfg in patch v3.


Miles
> 
> > 
> > Regards,
> > Matthias
> 


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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
@ 2020-07-22  9:37           ` Miles Chen
  0 siblings, 0 replies; 32+ messages in thread
From: Miles Chen @ 2020-07-22  9:37 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Rob Herring, wsd_upstream, David Hildenbrand, linux-kernel,
	Mike Rapoport, Chao Hao, iommu, linux-mediatek, Yingjoe Chen,
	Christoph Hellwig, linux-arm-kernel

On Wed, 2020-07-22 at 15:17 +0800, Miles Chen wrote:
> On Tue, 2020-07-21 at 23:19 +0200, Matthias Brugger wrote:
> > 
> > On 21/07/2020 13:24, Yong Wu wrote:
> > > On Tue, 2020-07-21 at 11:40 +0200, Matthias Brugger wrote:
> > >>
> > >> On 21/07/2020 04:16, Miles Chen wrote:
> > >>> In previous discussion [1] and [2], we found that it is risky to
> > >>> use max_pfn or totalram_pages to tell if 4GB mode is enabled.
> > >>>
> > >>> Check 4GB mode by reading infracfg register, remove the usage
> > >>> of the un-exported symbol max_pfn.
> > >>>
> > >>> This is a step towards building mtk_iommu as a kernel module.
> > >>>
> > >>> Change since v1:
> > >>> 1. remove the phandle usage, search for infracfg instead [3]
> > >>> 2. use infracfg instead of infracfg_regmap
> > >>> 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
> > >>> 4. update enable_4GB only when has_4gb_mode
> > >>>
> > >>> [1] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/3/733__;!!CTRNKA9wMg0ARbw!w5YjY83YRL9_ijgXHwB1x2Dnb5BqiFUI8H5IAyAWWFMvUJKI9Qbj_zta2AaiFZejiQ$ 
> > >>> [2] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/4/136__;!!CTRNKA9wMg0ARbw!w5YjY83YRL9_ijgXHwB1x2Dnb5BqiFUI8H5IAyAWWFMvUJKI9Qbj_zta2Aa9U2yQyg$ 
> > >>> [3] https://urldefense.com/v3/__https://lkml.org/lkml/2020/7/15/1147__;!!CTRNKA9wMg0ARbw!w5YjY83YRL9_ijgXHwB1x2Dnb5BqiFUI8H5IAyAWWFMvUJKI9Qbj_zta2Aaxpk_Wjw$ 
> > >>>
> > >>> Cc: Mike Rapoport <rppt@linux.ibm.com>
> > >>> Cc: David Hildenbrand <david@redhat.com>
> > >>> Cc: Yong Wu <yong.wu@mediatek.com>
> > >>> Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
> > >>> Cc: Christoph Hellwig <hch@lst.de>
> > >>> Cc: Yong Wu <yong.wu@mediatek.com>
> > >>> Cc: Chao Hao <chao.hao@mediatek.com>
> > >>> Cc: Rob Herring <robh@kernel.org>
> > >>> Cc: Matthias Brugger <matthias.bgg@gmail.com>
> > >>> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> > >>> ---
> > >>>    drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
> > >>>    include/linux/soc/mediatek/infracfg.h |  3 +++
> > >>>    2 files changed, 24 insertions(+), 5 deletions(-)
> > >>>
> > >>> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > >>> index 2be96f1cdbd2..16765f532853 100644
> > >>> --- a/drivers/iommu/mtk_iommu.c
> > >>> +++ b/drivers/iommu/mtk_iommu.c
> > >>> @@ -3,7 +3,6 @@
> > >>>     * Copyright (c) 2015-2016 MediaTek Inc.
> > >>>     * Author: Yong Wu <yong.wu@mediatek.com>
> > >>>     */
> > >>> -#include <linux/memblock.h>
> > >>>    #include <linux/bug.h>
> > >>>    #include <linux/clk.h>
> > >>>    #include <linux/component.h>
> > >>> @@ -15,13 +14,16 @@
> > >>>    #include <linux/iommu.h>
> > >>>    #include <linux/iopoll.h>
> > >>>    #include <linux/list.h>
> > >>> +#include <linux/mfd/syscon.h>
> > >>>    #include <linux/of_address.h>
> > >>>    #include <linux/of_iommu.h>
> > >>>    #include <linux/of_irq.h>
> > >>>    #include <linux/of_platform.h>
> > >>>    #include <linux/platform_device.h>
> > >>> +#include <linux/regmap.h>
> > >>>    #include <linux/slab.h>
> > >>>    #include <linux/spinlock.h>
> > >>> +#include <linux/soc/mediatek/infracfg.h>
> > >>>    #include <asm/barrier.h>
> > >>>    #include <soc/mediatek/smi.h>
> > >>>    
> > >>> @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> > >>>    	struct resource         *res;
> > >>>    	resource_size_t		ioaddr;
> > >>>    	struct component_match  *match = NULL;
> > >>> +	struct regmap		*infracfg;
> > >>>    	void                    *protect;
> > >>>    	int                     i, larb_nr, ret;
> > >>> +	u32			val;
> > >>>    
> > >>>    	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> > >>>    	if (!data)
> > >>> @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> > >>>    		return -ENOMEM;
> > >>>    	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
> > >>>    
> > >>> -	/* Whether the current dram is over 4GB */
> > >>> -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
> > >>> -	if (!data->plat_data->has_4gb_mode)
> > >>> -		data->enable_4GB = false;
> > >>> +	data->enable_4GB = false;
> > >>> +	if (data->plat_data->has_4gb_mode) {
> > >>> +		infracfg = syscon_regmap_lookup_by_compatible(
> > >>> +				"mediatek,mt8173-infracfg");
> > >>> +		if (IS_ERR(infracfg)) {
> > >>> +			infracfg = syscon_regmap_lookup_by_compatible(
> > >>> +					"mediatek,mt2712-infracfg");
> > >>> +			if (IS_ERR(infracfg))
> > >>> +				return PTR_ERR(infracfg);
> > >>
> > >> I think we should check m4u_plat instead to decide which compatible we have to
> > >> look for.
> > >> Another option would be to add a general compatible something like
> > >> "mtk-infracfg" and search for that. That would need an update of all DTS having
> > >> a infracfg compatible right now. After thinking twice, this would break newer
> > >> kernel with older device tree, so maybe it's better to go with m4u_plat switch
> > >> statement.
> > > 
> > > Add a "char *infracfg" in the plat_data, Use the mt2712, mt8173
> > > corresponding string in it. If it is NULL, It means the "enable_4GB"
> > > always is false. Then we also can remove the flag "has_4gb_mode".
> > > 
> > > is this OK?
> > > 
> > 
> > It's an option, but I personally find that a bit hacky.
> 
> Thanks Yong and Matthias for your comment.
> I will try adding a char *infracfg in patch v3.
> 

I misunderstood the comment.
I should check m4u_plat to decide which string to look for, not adding a
char *infracfg in patch v3.


Miles
> 
> > 
> > Regards,
> > Matthias
> 

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
@ 2020-07-22  9:37           ` Miles Chen
  0 siblings, 0 replies; 32+ messages in thread
From: Miles Chen @ 2020-07-22  9:37 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Rob Herring, wsd_upstream, David Hildenbrand, Joerg Roedel,
	linux-kernel, Mike Rapoport, Chao Hao, iommu, linux-mediatek,
	Yong Wu, Yingjoe Chen, Christoph Hellwig, linux-arm-kernel

On Wed, 2020-07-22 at 15:17 +0800, Miles Chen wrote:
> On Tue, 2020-07-21 at 23:19 +0200, Matthias Brugger wrote:
> > 
> > On 21/07/2020 13:24, Yong Wu wrote:
> > > On Tue, 2020-07-21 at 11:40 +0200, Matthias Brugger wrote:
> > >>
> > >> On 21/07/2020 04:16, Miles Chen wrote:
> > >>> In previous discussion [1] and [2], we found that it is risky to
> > >>> use max_pfn or totalram_pages to tell if 4GB mode is enabled.
> > >>>
> > >>> Check 4GB mode by reading infracfg register, remove the usage
> > >>> of the un-exported symbol max_pfn.
> > >>>
> > >>> This is a step towards building mtk_iommu as a kernel module.
> > >>>
> > >>> Change since v1:
> > >>> 1. remove the phandle usage, search for infracfg instead [3]
> > >>> 2. use infracfg instead of infracfg_regmap
> > >>> 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
> > >>> 4. update enable_4GB only when has_4gb_mode
> > >>>
> > >>> [1] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/3/733__;!!CTRNKA9wMg0ARbw!w5YjY83YRL9_ijgXHwB1x2Dnb5BqiFUI8H5IAyAWWFMvUJKI9Qbj_zta2AaiFZejiQ$ 
> > >>> [2] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/4/136__;!!CTRNKA9wMg0ARbw!w5YjY83YRL9_ijgXHwB1x2Dnb5BqiFUI8H5IAyAWWFMvUJKI9Qbj_zta2Aa9U2yQyg$ 
> > >>> [3] https://urldefense.com/v3/__https://lkml.org/lkml/2020/7/15/1147__;!!CTRNKA9wMg0ARbw!w5YjY83YRL9_ijgXHwB1x2Dnb5BqiFUI8H5IAyAWWFMvUJKI9Qbj_zta2Aaxpk_Wjw$ 
> > >>>
> > >>> Cc: Mike Rapoport <rppt@linux.ibm.com>
> > >>> Cc: David Hildenbrand <david@redhat.com>
> > >>> Cc: Yong Wu <yong.wu@mediatek.com>
> > >>> Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
> > >>> Cc: Christoph Hellwig <hch@lst.de>
> > >>> Cc: Yong Wu <yong.wu@mediatek.com>
> > >>> Cc: Chao Hao <chao.hao@mediatek.com>
> > >>> Cc: Rob Herring <robh@kernel.org>
> > >>> Cc: Matthias Brugger <matthias.bgg@gmail.com>
> > >>> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> > >>> ---
> > >>>    drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
> > >>>    include/linux/soc/mediatek/infracfg.h |  3 +++
> > >>>    2 files changed, 24 insertions(+), 5 deletions(-)
> > >>>
> > >>> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > >>> index 2be96f1cdbd2..16765f532853 100644
> > >>> --- a/drivers/iommu/mtk_iommu.c
> > >>> +++ b/drivers/iommu/mtk_iommu.c
> > >>> @@ -3,7 +3,6 @@
> > >>>     * Copyright (c) 2015-2016 MediaTek Inc.
> > >>>     * Author: Yong Wu <yong.wu@mediatek.com>
> > >>>     */
> > >>> -#include <linux/memblock.h>
> > >>>    #include <linux/bug.h>
> > >>>    #include <linux/clk.h>
> > >>>    #include <linux/component.h>
> > >>> @@ -15,13 +14,16 @@
> > >>>    #include <linux/iommu.h>
> > >>>    #include <linux/iopoll.h>
> > >>>    #include <linux/list.h>
> > >>> +#include <linux/mfd/syscon.h>
> > >>>    #include <linux/of_address.h>
> > >>>    #include <linux/of_iommu.h>
> > >>>    #include <linux/of_irq.h>
> > >>>    #include <linux/of_platform.h>
> > >>>    #include <linux/platform_device.h>
> > >>> +#include <linux/regmap.h>
> > >>>    #include <linux/slab.h>
> > >>>    #include <linux/spinlock.h>
> > >>> +#include <linux/soc/mediatek/infracfg.h>
> > >>>    #include <asm/barrier.h>
> > >>>    #include <soc/mediatek/smi.h>
> > >>>    
> > >>> @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> > >>>    	struct resource         *res;
> > >>>    	resource_size_t		ioaddr;
> > >>>    	struct component_match  *match = NULL;
> > >>> +	struct regmap		*infracfg;
> > >>>    	void                    *protect;
> > >>>    	int                     i, larb_nr, ret;
> > >>> +	u32			val;
> > >>>    
> > >>>    	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> > >>>    	if (!data)
> > >>> @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> > >>>    		return -ENOMEM;
> > >>>    	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
> > >>>    
> > >>> -	/* Whether the current dram is over 4GB */
> > >>> -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
> > >>> -	if (!data->plat_data->has_4gb_mode)
> > >>> -		data->enable_4GB = false;
> > >>> +	data->enable_4GB = false;
> > >>> +	if (data->plat_data->has_4gb_mode) {
> > >>> +		infracfg = syscon_regmap_lookup_by_compatible(
> > >>> +				"mediatek,mt8173-infracfg");
> > >>> +		if (IS_ERR(infracfg)) {
> > >>> +			infracfg = syscon_regmap_lookup_by_compatible(
> > >>> +					"mediatek,mt2712-infracfg");
> > >>> +			if (IS_ERR(infracfg))
> > >>> +				return PTR_ERR(infracfg);
> > >>
> > >> I think we should check m4u_plat instead to decide which compatible we have to
> > >> look for.
> > >> Another option would be to add a general compatible something like
> > >> "mtk-infracfg" and search for that. That would need an update of all DTS having
> > >> a infracfg compatible right now. After thinking twice, this would break newer
> > >> kernel with older device tree, so maybe it's better to go with m4u_plat switch
> > >> statement.
> > > 
> > > Add a "char *infracfg" in the plat_data, Use the mt2712, mt8173
> > > corresponding string in it. If it is NULL, It means the "enable_4GB"
> > > always is false. Then we also can remove the flag "has_4gb_mode".
> > > 
> > > is this OK?
> > > 
> > 
> > It's an option, but I personally find that a bit hacky.
> 
> Thanks Yong and Matthias for your comment.
> I will try adding a char *infracfg in patch v3.
> 

I misunderstood the comment.
I should check m4u_plat to decide which string to look for, not adding a
char *infracfg in patch v3.


Miles
> 
> > 
> > Regards,
> > Matthias
> 

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

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

* Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
@ 2020-07-22  9:37           ` Miles Chen
  0 siblings, 0 replies; 32+ messages in thread
From: Miles Chen @ 2020-07-22  9:37 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Rob Herring, wsd_upstream, David Hildenbrand, Joerg Roedel,
	linux-kernel, Mike Rapoport, Chao Hao, iommu, linux-mediatek,
	Yong Wu, Yingjoe Chen, Christoph Hellwig, linux-arm-kernel

On Wed, 2020-07-22 at 15:17 +0800, Miles Chen wrote:
> On Tue, 2020-07-21 at 23:19 +0200, Matthias Brugger wrote:
> > 
> > On 21/07/2020 13:24, Yong Wu wrote:
> > > On Tue, 2020-07-21 at 11:40 +0200, Matthias Brugger wrote:
> > >>
> > >> On 21/07/2020 04:16, Miles Chen wrote:
> > >>> In previous discussion [1] and [2], we found that it is risky to
> > >>> use max_pfn or totalram_pages to tell if 4GB mode is enabled.
> > >>>
> > >>> Check 4GB mode by reading infracfg register, remove the usage
> > >>> of the un-exported symbol max_pfn.
> > >>>
> > >>> This is a step towards building mtk_iommu as a kernel module.
> > >>>
> > >>> Change since v1:
> > >>> 1. remove the phandle usage, search for infracfg instead [3]
> > >>> 2. use infracfg instead of infracfg_regmap
> > >>> 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
> > >>> 4. update enable_4GB only when has_4gb_mode
> > >>>
> > >>> [1] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/3/733__;!!CTRNKA9wMg0ARbw!w5YjY83YRL9_ijgXHwB1x2Dnb5BqiFUI8H5IAyAWWFMvUJKI9Qbj_zta2AaiFZejiQ$ 
> > >>> [2] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/4/136__;!!CTRNKA9wMg0ARbw!w5YjY83YRL9_ijgXHwB1x2Dnb5BqiFUI8H5IAyAWWFMvUJKI9Qbj_zta2Aa9U2yQyg$ 
> > >>> [3] https://urldefense.com/v3/__https://lkml.org/lkml/2020/7/15/1147__;!!CTRNKA9wMg0ARbw!w5YjY83YRL9_ijgXHwB1x2Dnb5BqiFUI8H5IAyAWWFMvUJKI9Qbj_zta2Aaxpk_Wjw$ 
> > >>>
> > >>> Cc: Mike Rapoport <rppt@linux.ibm.com>
> > >>> Cc: David Hildenbrand <david@redhat.com>
> > >>> Cc: Yong Wu <yong.wu@mediatek.com>
> > >>> Cc: Yingjoe Chen <yingjoe.chen@mediatek.com>
> > >>> Cc: Christoph Hellwig <hch@lst.de>
> > >>> Cc: Yong Wu <yong.wu@mediatek.com>
> > >>> Cc: Chao Hao <chao.hao@mediatek.com>
> > >>> Cc: Rob Herring <robh@kernel.org>
> > >>> Cc: Matthias Brugger <matthias.bgg@gmail.com>
> > >>> Signed-off-by: Miles Chen <miles.chen@mediatek.com>
> > >>> ---
> > >>>    drivers/iommu/mtk_iommu.c             | 26 +++++++++++++++++++++-----
> > >>>    include/linux/soc/mediatek/infracfg.h |  3 +++
> > >>>    2 files changed, 24 insertions(+), 5 deletions(-)
> > >>>
> > >>> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > >>> index 2be96f1cdbd2..16765f532853 100644
> > >>> --- a/drivers/iommu/mtk_iommu.c
> > >>> +++ b/drivers/iommu/mtk_iommu.c
> > >>> @@ -3,7 +3,6 @@
> > >>>     * Copyright (c) 2015-2016 MediaTek Inc.
> > >>>     * Author: Yong Wu <yong.wu@mediatek.com>
> > >>>     */
> > >>> -#include <linux/memblock.h>
> > >>>    #include <linux/bug.h>
> > >>>    #include <linux/clk.h>
> > >>>    #include <linux/component.h>
> > >>> @@ -15,13 +14,16 @@
> > >>>    #include <linux/iommu.h>
> > >>>    #include <linux/iopoll.h>
> > >>>    #include <linux/list.h>
> > >>> +#include <linux/mfd/syscon.h>
> > >>>    #include <linux/of_address.h>
> > >>>    #include <linux/of_iommu.h>
> > >>>    #include <linux/of_irq.h>
> > >>>    #include <linux/of_platform.h>
> > >>>    #include <linux/platform_device.h>
> > >>> +#include <linux/regmap.h>
> > >>>    #include <linux/slab.h>
> > >>>    #include <linux/spinlock.h>
> > >>> +#include <linux/soc/mediatek/infracfg.h>
> > >>>    #include <asm/barrier.h>
> > >>>    #include <soc/mediatek/smi.h>
> > >>>    
> > >>> @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> > >>>    	struct resource         *res;
> > >>>    	resource_size_t		ioaddr;
> > >>>    	struct component_match  *match = NULL;
> > >>> +	struct regmap		*infracfg;
> > >>>    	void                    *protect;
> > >>>    	int                     i, larb_nr, ret;
> > >>> +	u32			val;
> > >>>    
> > >>>    	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> > >>>    	if (!data)
> > >>> @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> > >>>    		return -ENOMEM;
> > >>>    	data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
> > >>>    
> > >>> -	/* Whether the current dram is over 4GB */
> > >>> -	data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
> > >>> -	if (!data->plat_data->has_4gb_mode)
> > >>> -		data->enable_4GB = false;
> > >>> +	data->enable_4GB = false;
> > >>> +	if (data->plat_data->has_4gb_mode) {
> > >>> +		infracfg = syscon_regmap_lookup_by_compatible(
> > >>> +				"mediatek,mt8173-infracfg");
> > >>> +		if (IS_ERR(infracfg)) {
> > >>> +			infracfg = syscon_regmap_lookup_by_compatible(
> > >>> +					"mediatek,mt2712-infracfg");
> > >>> +			if (IS_ERR(infracfg))
> > >>> +				return PTR_ERR(infracfg);
> > >>
> > >> I think we should check m4u_plat instead to decide which compatible we have to
> > >> look for.
> > >> Another option would be to add a general compatible something like
> > >> "mtk-infracfg" and search for that. That would need an update of all DTS having
> > >> a infracfg compatible right now. After thinking twice, this would break newer
> > >> kernel with older device tree, so maybe it's better to go with m4u_plat switch
> > >> statement.
> > > 
> > > Add a "char *infracfg" in the plat_data, Use the mt2712, mt8173
> > > corresponding string in it. If it is NULL, It means the "enable_4GB"
> > > always is false. Then we also can remove the flag "has_4gb_mode".
> > > 
> > > is this OK?
> > > 
> > 
> > It's an option, but I personally find that a bit hacky.
> 
> Thanks Yong and Matthias for your comment.
> I will try adding a char *infracfg in patch v3.
> 

I misunderstood the comment.
I should check m4u_plat to decide which string to look for, not adding a
char *infracfg in patch v3.


Miles
> 
> > 
> > Regards,
> > Matthias
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, other threads:[~2020-07-22  9:39 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-21  2:16 [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg Miles Chen
2020-07-21  2:16 ` Miles Chen
2020-07-21  2:16 ` Miles Chen
2020-07-21  2:16 ` Miles Chen
2020-07-21  9:10 ` David Hildenbrand
2020-07-21  9:10   ` David Hildenbrand
2020-07-21  9:10   ` David Hildenbrand
2020-07-21  9:10   ` David Hildenbrand
2020-07-22  7:15   ` Miles Chen
2020-07-22  7:15     ` Miles Chen
2020-07-22  7:15     ` Miles Chen
2020-07-22  7:15     ` Miles Chen
2020-07-21  9:40 ` Matthias Brugger
2020-07-21  9:40   ` Matthias Brugger
2020-07-21  9:40   ` Matthias Brugger
2020-07-21  9:40   ` Matthias Brugger
2020-07-21 11:24   ` Yong Wu
2020-07-21 11:24     ` Yong Wu
2020-07-21 11:24     ` Yong Wu
2020-07-21 11:24     ` Yong Wu
2020-07-21 21:19     ` Matthias Brugger
2020-07-21 21:19       ` Matthias Brugger
2020-07-21 21:19       ` Matthias Brugger
2020-07-21 21:19       ` Matthias Brugger
2020-07-22  7:17       ` Miles Chen
2020-07-22  7:17         ` Miles Chen
2020-07-22  7:17         ` Miles Chen
2020-07-22  7:17         ` Miles Chen
2020-07-22  9:37         ` Miles Chen
2020-07-22  9:37           ` Miles Chen
2020-07-22  9:37           ` Miles Chen
2020-07-22  9:37           ` Miles Chen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.