2020-07-22 14:20:07

by Miles Chen

[permalink] [raw]
Subject: [PATCH v3] iommu/mediatek: check 4GB mode by reading infracfg

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 v2:
- determine compatible string by m4u_plat
- rebase to next-20200720
- add "---"

Change since v1:
- remove the phandle usage, search for infracfg instead [3]
- use infracfg instead of infracfg_regmap
- move infracfg definitaions to linux/soc/mediatek/infracfg.h
- 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 <[email protected]>
Cc: David Hildenbrand <[email protected]>
Cc: Yong Wu <[email protected]>
Cc: Yingjoe Chen <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Matthias Brugger <[email protected]>
Signed-off-by: Miles Chen <[email protected]>
---
drivers/iommu/mtk_iommu.c | 34 +++++++++++++++++++++++----
include/linux/soc/mediatek/infracfg.h | 3 +++
2 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 59e5a62a34db..9ec666168822 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 <[email protected]>
*/
-#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>

@@ -640,8 +642,11 @@ 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;
+ char *p;

data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
@@ -655,10 +660,29 @@ 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 (!MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_4GB_MODE))
- data->enable_4GB = false;
+ data->enable_4GB = false;
+ if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_4GB_MODE)) {
+ switch (data->plat_data->m4u_plat) {
+ case M4U_MT2712:
+ p = "mediatek,mt2712-infracfg";
+ break;
+ case M4U_MT8173:
+ p = "mediatek,mt8173-infracfg";
+ break;
+ default:
+ p = NULL;
+ }
+
+ infracfg = syscon_regmap_lookup_by_compatible(p);
+
+ 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


2020-07-22 15:23:48

by Matthias Brugger

[permalink] [raw]
Subject: Re: [PATCH v3] iommu/mediatek: check 4GB mode by reading infracfg



On 22/07/2020 16:19, 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.
>
> ---

That's wrong. The commit message would be cut after this '---' so we would loose
the Cc and Signed-of-by tags.

>
> Change since v2:
> - determine compatible string by m4u_plat
> - rebase to next-20200720
> - add "---"
>
> Change since v1:
> - remove the phandle usage, search for infracfg instead [3]
> - use infracfg instead of infracfg_regmap
> - move infracfg definitaions to linux/soc/mediatek/infracfg.h
> - 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

I think using links to lore.kernel.org would make sure that the URL does not
change over time. As the commit log will stay there for ever, but who konws what
happens with lkml.org

> [3] https://lkml.org/lkml/2020/7/15/1147
>
> Cc: Mike Rapoport <[email protected]>
> Cc: David Hildenbrand <[email protected]>
> Cc: Yong Wu <[email protected]>
> Cc: Yingjoe Chen <[email protected]>
> Cc: Christoph Hellwig <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Matthias Brugger <[email protected]>
> Signed-off-by: Miles Chen <[email protected]>

The formating should look like this:
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.

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

Cc: Mike Rapoport <[email protected]>
Cc: David Hildenbrand <[email protected]>
Cc: Yong Wu <[email protected]>
Cc: Yingjoe Chen <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Matthias Brugger <[email protected]>
Signed-off-by: Miles Chen <[email protected]>
---

Change since v2:
- determine compatible string by m4u_plat
- rebase to next-20200720
- add "---"

Change since v1:
- remove the phandle usage, search for infracfg instead
https://lkml.org/lkml/2020/7/15/1147
- use infracfg instead of infracfg_regmap
- move infracfg definitaions to linux/soc/mediatek/infracfg.h
- update enable_4GB only when has_4gb_mode



> ---
> drivers/iommu/mtk_iommu.c | 34 +++++++++++++++++++++++----
> include/linux/soc/mediatek/infracfg.h | 3 +++
> 2 files changed, 32 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 59e5a62a34db..9ec666168822 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 <[email protected]>
> */
> -#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>
>
> @@ -640,8 +642,11 @@ 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;
> + char *p;
>
> data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> if (!data)
> @@ -655,10 +660,29 @@ 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 (!MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_4GB_MODE))
> - data->enable_4GB = false;
> + data->enable_4GB = false;
> + if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_4GB_MODE)) {
> + switch (data->plat_data->m4u_plat) {
> + case M4U_MT2712:
> + p = "mediatek,mt2712-infracfg";
> + break;
> + case M4U_MT8173:
> + p = "mediatek,mt8173-infracfg";
> + break;
> + default:
> + p = NULL;
> + }
> +
> + infracfg = syscon_regmap_lookup_by_compatible(p);

if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_4GB_MODE)) {
switch (data->plat_data->m4u_plat) {
case M4U_MT2712:
infracfg = syscon_regmap_lookup_by_compatible("mediatek,mt2712-infracfg");
break;
case M4U_MT8173:
infracfg = syscon_regmap_lookup_by_compatible("mediatek,mt8173-infracfg");
break;
default:
infracfg = -ENODEV;
}
> +
> + 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,
>

2020-07-23 01:45:26

by Miles Chen

[permalink] [raw]
Subject: Re: [PATCH v3] iommu/mediatek: check 4GB mode by reading infracfg

On Wed, 2020-07-22 at 17:19 +0200, Matthias Brugger wrote:
>
> On 22/07/2020 16:19, 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.
> >
> > ---
>
> That's wrong. The commit message would be cut after this '---' so we would loose
> the Cc and Signed-of-by tags.

Thanks for the comment.
understood, I will fix that in patch v4.
>
> >
> > Change since v2:
> > - determine compatible string by m4u_plat
> > - rebase to next-20200720
> > - add "---"
> >
> > Change since v1:
> > - remove the phandle usage, search for infracfg instead [3]
> > - use infracfg instead of infracfg_regmap
> > - move infracfg definitaions to linux/soc/mediatek/infracfg.h
> > - update enable_4GB only when has_4gb_mode
> >
> > [1] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/3/733__;!!CTRNKA9wMg0ARbw!073_W_0qmeQnHgSGJRNPTbK2KnPa4VzaPqFBffFn12odyEL1LDaQtZEmrMY4tB_vBw$
> > [2] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/4/136__;!!CTRNKA9wMg0ARbw!073_W_0qmeQnHgSGJRNPTbK2KnPa4VzaPqFBffFn12odyEL1LDaQtZEmrMZ7PRs7yw$
>
> I think using links to lore.kernel.org would make sure that the URL does not
> change over time. As the commit log will stay there for ever, but who konws what
> happens with lkml.org

I will use lore.kernel.org links
>
> > [3] https://urldefense.com/v3/__https://lkml.org/lkml/2020/7/15/1147__;!!CTRNKA9wMg0ARbw!073_W_0qmeQnHgSGJRNPTbK2KnPa4VzaPqFBffFn12odyEL1LDaQtZEmrMYreY-qqA$
> >
> > Cc: Mike Rapoport <[email protected]>
> > Cc: David Hildenbrand <[email protected]>
> > Cc: Yong Wu <[email protected]>
> > Cc: Yingjoe Chen <[email protected]>
> > Cc: Christoph Hellwig <[email protected]>
> > Cc: Rob Herring <[email protected]>
> > Cc: Matthias Brugger <[email protected]>
> > Signed-off-by: Miles Chen <[email protected]>
>
> The formating should look like this:
> 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.
>
> [1] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/3/733__;!!CTRNKA9wMg0ARbw!073_W_0qmeQnHgSGJRNPTbK2KnPa4VzaPqFBffFn12odyEL1LDaQtZEmrMY4tB_vBw$
> [2] https://urldefense.com/v3/__https://lkml.org/lkml/2020/6/4/136__;!!CTRNKA9wMg0ARbw!073_W_0qmeQnHgSGJRNPTbK2KnPa4VzaPqFBffFn12odyEL1LDaQtZEmrMZ7PRs7yw$
>
> Cc: Mike Rapoport <[email protected]>
> Cc: David Hildenbrand <[email protected]>
> Cc: Yong Wu <[email protected]>
> Cc: Yingjoe Chen <[email protected]>
> Cc: Christoph Hellwig <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Matthias Brugger <[email protected]>
> Signed-off-by: Miles Chen <[email protected]>
> ---
>
> Change since v2:
> - determine compatible string by m4u_plat
> - rebase to next-20200720
> - add "---"
>
> Change since v1:
> - remove the phandle usage, search for infracfg instead
> https://urldefense.com/v3/__https://lkml.org/lkml/2020/7/15/1147__;!!CTRNKA9wMg0ARbw!073_W_0qmeQnHgSGJRNPTbK2KnPa4VzaPqFBffFn12odyEL1LDaQtZEmrMYreY-qqA$
> - use infracfg instead of infracfg_regmap
> - move infracfg definitaions to linux/soc/mediatek/infracfg.h
> - update enable_4GB only when has_4gb_mode
>
>
>
> > ---
> > drivers/iommu/mtk_iommu.c | 34 +++++++++++++++++++++++----
> > include/linux/soc/mediatek/infracfg.h | 3 +++
> > 2 files changed, 32 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > index 59e5a62a34db..9ec666168822 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 <[email protected]>
> > */
> > -#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>
> >
> > @@ -640,8 +642,11 @@ 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;
> > + char *p;
> >
> > data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> > if (!data)
> > @@ -655,10 +660,29 @@ 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 (!MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_4GB_MODE))
> > - data->enable_4GB = false;
> > + data->enable_4GB = false;
> > + if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_4GB_MODE)) {
> > + switch (data->plat_data->m4u_plat) {
> > + case M4U_MT2712:
> > + p = "mediatek,mt2712-infracfg";
> > + break;
> > + case M4U_MT8173:
> > + p = "mediatek,mt8173-infracfg";
> > + break;
> > + default:
> > + p = NULL;
> > + }
> > +
> > + infracfg = syscon_regmap_lookup_by_compatible(p);
>
> if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_4GB_MODE)) {
> switch (data->plat_data->m4u_plat) {
> case M4U_MT2712:
> infracfg = syscon_regmap_lookup_by_compatible("mediatek,mt2712-infracfg");
> break;
> case M4U_MT8173:
> infracfg = syscon_regmap_lookup_by_compatible("mediatek,mt8173-infracfg");
> break;
> default:
> infracfg = -ENODEV;
> }
> > +
> > + 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,
> >