Re: [PATCH v4 10/18] iommu/mediatek: Add mt8183 IOMMU support

2018-12-21 Thread Yong Wu
On Fri, 2018-12-21 at 19:31 +0100, Matthias Brugger wrote:
> 
> On 08/12/2018 09:39, Yong Wu wrote:
> > The M4U IP blocks in mt8183 is MediaTek's generation2 M4U which use
> > the ARM Short-descriptor like mt8173, and most of the HW registers
> > are the same.
> > 
> > Here list main differences between mt8183 and mt8173/mt2712:
> > 1) mt8183 has only one M4U HW like mt8173 while mt2712 has two.
> > 2) mt8183 don't have the "bclk" clock, it use the EMI clock instead.
> > 3) mt8183 can support the dram over 4GB, but it doesn't call this "4GB
> > mode".
> > 4) mt8183 pgtable base register(0x0) extend bit[1:0] which represent
> > the bit[33:32] in the physical address of the pgtable base, But the
> > standard ttbr0[1] means the S bit which is enabled defaultly, Hence,
> > we add a mask.
> > 5) mt8183 HW has a GALS modules, SMI should enable "has_gals" support.
> > 6) the larb-id in smi-common is remapped. M4U should enable
> > larbid_remapped support.
> > 
> > Signed-off-by: Yong Wu 
> > ---
[...]
> > +static const struct mtk_iommu_plat_data mt8183_data = {
> > +   .m4u_plat= M4U_MT8183,
> > +   .larbid_remap_enable = true,
> > +   .larbid_remapped = {0, 4, 5, 6, 7, 2, 3, 1},
> 
> Aren't we reinventing the wheel here?
> Why can't we use larb-id to get the correct id insteaf of providing another 
> data
> structure for the remapping?

Sorry, The remapping id is arbitrary, there is no rule to get it from
the larb-id.

>From Nicolas's comment, I plan to delete "larbid_remap_enable" and only
use "larbid_remap". The other SoCs use the linear mapping here.

In addition, I have to apologize that here will may be improved for
mt2712. There are 2 smi-common(smi-common0 and smi-common1) in mt2712,
actually the remapping relationship for smi-common1 is also different.
If it is really needed, I plan to change it from "larbid_remap" to
"larbid_remap[2]" which 0 is for smi-common0 and 1 is for smi-common1.
Of course, it doesn't affect the iommu functions and only prints the
error log when IOMMU translation fault.

> 
> Regards,
> Matthias

[...]


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


Re: [PATCH v4 10/18] iommu/mediatek: Add mt8183 IOMMU support

2018-12-21 Thread Yong Wu
On Sat, 2018-12-22 at 08:31 +0800, Nicolas Boichat wrote:
> On Fri, Dec 21, 2018 at 4:02 PM Yong Wu  wrote:
> >
> > On Fri, 2018-12-21 at 12:43 +0800, Nicolas Boichat wrote:
> > > On Sat, Dec 8, 2018 at 4:42 PM Yong Wu  wrote:
> > > >
> > > > The M4U IP blocks in mt8183 is MediaTek's generation2 M4U which use
> > > > the ARM Short-descriptor like mt8173, and most of the HW registers
> > > > are the same.
> > > >
> > > > Here list main differences between mt8183 and mt8173/mt2712:
> > > > 1) mt8183 has only one M4U HW like mt8173 while mt2712 has two.
> > > > 2) mt8183 don't have the "bclk" clock, it use the EMI clock instead.
> > > > 3) mt8183 can support the dram over 4GB, but it doesn't call this "4GB
> > > > mode".
> > > > 4) mt8183 pgtable base register(0x0) extend bit[1:0] which represent
> > > > the bit[33:32] in the physical address of the pgtable base, But the
> > > > standard ttbr0[1] means the S bit which is enabled defaultly, Hence,
> > > > we add a mask.
> > > > 5) mt8183 HW has a GALS modules, SMI should enable "has_gals" support.
> > > > 6) the larb-id in smi-common is remapped. M4U should enable
> > > > larbid_remapped support.
> > > >
> > > > Signed-off-by: Yong Wu 
> > > > ---
> > > >  drivers/iommu/mtk_iommu.c | 31 ++-
> > > >  drivers/iommu/mtk_iommu.h |  1 +
> > > >  drivers/memory/mtk-smi.c  | 19 +++
> > > >  3 files changed, 42 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > > > index 8ab3b69..d91a554 100644
> > > > --- a/drivers/iommu/mtk_iommu.c
> > > > +++ b/drivers/iommu/mtk_iommu.c
> > > > @@ -36,6 +36,7 @@
> > > >  #include "mtk_iommu.h"
> > > >
> > > >  #define REG_MMU_PT_BASE_ADDR   0x000
> > > > +#define MMU_PT_ADDR_MASK   GENMASK(31, 7)
> > > >
> > > >  #define REG_MMU_INVALIDATE 0x020
> > > >  #define F_ALL_INVLD0x2
> > > > @@ -54,7 +55,7 @@
> > > >  #define REG_MMU_CTRL_REG   0x110
> > > >  #define F_MMU_PREFETCH_RT_REPLACE_MOD  BIT(4)
> > > >  #define F_MMU_TF_PROTECT_SEL_SHIFT(data) \
> > > > -   ((data)->plat_data->m4u_plat == M4U_MT2712 ? 4 : 5)
> > > > +   ((data)->plat_data->m4u_plat == M4U_MT8173 ? 5 : 4)
> > >
> > > Should the shift value be a member of plat_data instead?
> >
> > It's also ok.
> > This TF_PROTECT_SEL MACRO looks a bit complex. I can use a new patch to
> > refactor it.
> 
> SGTM.
> 
> > >
> > > >  /* It's named by F_MMU_TF_PROT_SEL in mt2712. */
> > > >  #define F_MMU_TF_PROTECT_SEL(prot, data) \
> > > > (((prot) & 0x3) << F_MMU_TF_PROTECT_SEL_SHIFT(data))
> > > > @@ -347,7 +348,7 @@ static int mtk_iommu_attach_device(struct 
> > > > iommu_domain *domain,
> > > > /* Update the pgtable base address register of the M4U HW */
> > > > if (!data->m4u_dom) {
> > > > data->m4u_dom = dom;
> > > > -   writel(dom->cfg.arm_v7s_cfg.ttbr[0],
> > > > +   writel(dom->cfg.arm_v7s_cfg.ttbr[0] & MMU_PT_ADDR_MASK,
> > > >data->base + REG_MMU_PT_BASE_ADDR);
> > > > }
> > > >
> > > > @@ -510,6 +511,7 @@ static int mtk_iommu_of_xlate(struct device *dev, 
> > > > struct of_phandle_args *args)
> > > >
> > > >  static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
> > > >  {
> > > > +   enum mtk_iommu_plat m4u_plat = data->plat_data->m4u_plat;
> > > > u32 regval;
> > > > int ret;
> > > >
> > > > @@ -520,7 +522,7 @@ static int mtk_iommu_hw_init(const struct 
> > > > mtk_iommu_data *data)
> > > > }
> > > >
> > > > regval = F_MMU_TF_PROTECT_SEL(2, data);
> > > > -   if (data->plat_data->m4u_plat == M4U_MT8173)
> > > > +   if (m4u_plat == M4U_MT8173)
> > > > regval |= F_MMU_PREFETCH_RT_REPLACE_MOD;
> > > > writel_relaxed(regval, data->base + REG_MMU_CTRL_REG);
> > > >
> > > > @@ -541,14 +543,14 @@ static int mtk_iommu_hw_init(const struct 
> > > > mtk_iommu_data *data)
> > > > F_INT_PRETETCH_TRANSATION_FIFO_FAULT;
> > > > writel_relaxed(regval, data->base + REG_MMU_INT_MAIN_CONTROL);
> > > >
> > > > -   if (data->plat_data->m4u_plat == M4U_MT8173)
> > > > +   if (m4u_plat == M4U_MT8173)
> > > > regval = (data->protect_base >> 1) | (data->enable_4GB 
> > > > << 31);
> > > > else
> > > > regval = lower_32_bits(data->protect_base) |
> > > >  upper_32_bits(data->protect_base);
> > > > writel_relaxed(regval, data->base + REG_MMU_IVRP_PADDR);
> > > >
> > > > -   if (data->enable_4GB && data->plat_data->m4u_plat != 
> > > > M4U_MT8173) {
> > > > +   if (data->enable_4GB && m4u_plat == M4U_MT2712) {
> > > > /*
> > > >  * If 4GB mode is enabled, the validate PA range is from
> > > >  * 0x1__ to 0x1__. here record 

Re: [PATCH v4 10/18] iommu/mediatek: Add mt8183 IOMMU support

2018-12-21 Thread Nicolas Boichat
On Fri, Dec 21, 2018 at 4:02 PM Yong Wu  wrote:
>
> On Fri, 2018-12-21 at 12:43 +0800, Nicolas Boichat wrote:
> > On Sat, Dec 8, 2018 at 4:42 PM Yong Wu  wrote:
> > >
> > > The M4U IP blocks in mt8183 is MediaTek's generation2 M4U which use
> > > the ARM Short-descriptor like mt8173, and most of the HW registers
> > > are the same.
> > >
> > > Here list main differences between mt8183 and mt8173/mt2712:
> > > 1) mt8183 has only one M4U HW like mt8173 while mt2712 has two.
> > > 2) mt8183 don't have the "bclk" clock, it use the EMI clock instead.
> > > 3) mt8183 can support the dram over 4GB, but it doesn't call this "4GB
> > > mode".
> > > 4) mt8183 pgtable base register(0x0) extend bit[1:0] which represent
> > > the bit[33:32] in the physical address of the pgtable base, But the
> > > standard ttbr0[1] means the S bit which is enabled defaultly, Hence,
> > > we add a mask.
> > > 5) mt8183 HW has a GALS modules, SMI should enable "has_gals" support.
> > > 6) the larb-id in smi-common is remapped. M4U should enable
> > > larbid_remapped support.
> > >
> > > Signed-off-by: Yong Wu 
> > > ---
> > >  drivers/iommu/mtk_iommu.c | 31 ++-
> > >  drivers/iommu/mtk_iommu.h |  1 +
> > >  drivers/memory/mtk-smi.c  | 19 +++
> > >  3 files changed, 42 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > > index 8ab3b69..d91a554 100644
> > > --- a/drivers/iommu/mtk_iommu.c
> > > +++ b/drivers/iommu/mtk_iommu.c
> > > @@ -36,6 +36,7 @@
> > >  #include "mtk_iommu.h"
> > >
> > >  #define REG_MMU_PT_BASE_ADDR   0x000
> > > +#define MMU_PT_ADDR_MASK   GENMASK(31, 7)
> > >
> > >  #define REG_MMU_INVALIDATE 0x020
> > >  #define F_ALL_INVLD0x2
> > > @@ -54,7 +55,7 @@
> > >  #define REG_MMU_CTRL_REG   0x110
> > >  #define F_MMU_PREFETCH_RT_REPLACE_MOD  BIT(4)
> > >  #define F_MMU_TF_PROTECT_SEL_SHIFT(data) \
> > > -   ((data)->plat_data->m4u_plat == M4U_MT2712 ? 4 : 5)
> > > +   ((data)->plat_data->m4u_plat == M4U_MT8173 ? 5 : 4)
> >
> > Should the shift value be a member of plat_data instead?
>
> It's also ok.
> This TF_PROTECT_SEL MACRO looks a bit complex. I can use a new patch to
> refactor it.

SGTM.

> >
> > >  /* It's named by F_MMU_TF_PROT_SEL in mt2712. */
> > >  #define F_MMU_TF_PROTECT_SEL(prot, data) \
> > > (((prot) & 0x3) << F_MMU_TF_PROTECT_SEL_SHIFT(data))
> > > @@ -347,7 +348,7 @@ static int mtk_iommu_attach_device(struct 
> > > iommu_domain *domain,
> > > /* Update the pgtable base address register of the M4U HW */
> > > if (!data->m4u_dom) {
> > > data->m4u_dom = dom;
> > > -   writel(dom->cfg.arm_v7s_cfg.ttbr[0],
> > > +   writel(dom->cfg.arm_v7s_cfg.ttbr[0] & MMU_PT_ADDR_MASK,
> > >data->base + REG_MMU_PT_BASE_ADDR);
> > > }
> > >
> > > @@ -510,6 +511,7 @@ static int mtk_iommu_of_xlate(struct device *dev, 
> > > struct of_phandle_args *args)
> > >
> > >  static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
> > >  {
> > > +   enum mtk_iommu_plat m4u_plat = data->plat_data->m4u_plat;
> > > u32 regval;
> > > int ret;
> > >
> > > @@ -520,7 +522,7 @@ static int mtk_iommu_hw_init(const struct 
> > > mtk_iommu_data *data)
> > > }
> > >
> > > regval = F_MMU_TF_PROTECT_SEL(2, data);
> > > -   if (data->plat_data->m4u_plat == M4U_MT8173)
> > > +   if (m4u_plat == M4U_MT8173)
> > > regval |= F_MMU_PREFETCH_RT_REPLACE_MOD;
> > > writel_relaxed(regval, data->base + REG_MMU_CTRL_REG);
> > >
> > > @@ -541,14 +543,14 @@ static int mtk_iommu_hw_init(const struct 
> > > mtk_iommu_data *data)
> > > F_INT_PRETETCH_TRANSATION_FIFO_FAULT;
> > > writel_relaxed(regval, data->base + REG_MMU_INT_MAIN_CONTROL);
> > >
> > > -   if (data->plat_data->m4u_plat == M4U_MT8173)
> > > +   if (m4u_plat == M4U_MT8173)
> > > regval = (data->protect_base >> 1) | (data->enable_4GB << 
> > > 31);
> > > else
> > > regval = lower_32_bits(data->protect_base) |
> > >  upper_32_bits(data->protect_base);
> > > writel_relaxed(regval, data->base + REG_MMU_IVRP_PADDR);
> > >
> > > -   if (data->enable_4GB && data->plat_data->m4u_plat != M4U_MT8173) {
> > > +   if (data->enable_4GB && m4u_plat == M4U_MT2712) {
> > > /*
> > >  * If 4GB mode is enabled, the validate PA range is from
> > >  * 0x1__ to 0x1__. here record bit[32:30].
> > > @@ -558,8 +560,11 @@ static int mtk_iommu_hw_init(const struct 
> > > mtk_iommu_data *data)
> > > }
> > > writel_relaxed(0, data->base + REG_MMU_DCM_DIS);
> > >
> > > -   /* It's MISC control register whose default value is ok except 
> > > 

Re: [PATCH v4 10/18] iommu/mediatek: Add mt8183 IOMMU support

2018-12-21 Thread Matthias Brugger



On 08/12/2018 09:39, Yong Wu wrote:
> The M4U IP blocks in mt8183 is MediaTek's generation2 M4U which use
> the ARM Short-descriptor like mt8173, and most of the HW registers
> are the same.
> 
> Here list main differences between mt8183 and mt8173/mt2712:
> 1) mt8183 has only one M4U HW like mt8173 while mt2712 has two.
> 2) mt8183 don't have the "bclk" clock, it use the EMI clock instead.
> 3) mt8183 can support the dram over 4GB, but it doesn't call this "4GB
> mode".
> 4) mt8183 pgtable base register(0x0) extend bit[1:0] which represent
> the bit[33:32] in the physical address of the pgtable base, But the
> standard ttbr0[1] means the S bit which is enabled defaultly, Hence,
> we add a mask.
> 5) mt8183 HW has a GALS modules, SMI should enable "has_gals" support.
> 6) the larb-id in smi-common is remapped. M4U should enable
> larbid_remapped support.
> 
> Signed-off-by: Yong Wu 
> ---
>  drivers/iommu/mtk_iommu.c | 31 ++-
>  drivers/iommu/mtk_iommu.h |  1 +
>  drivers/memory/mtk-smi.c  | 19 +++
>  3 files changed, 42 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 8ab3b69..d91a554 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -36,6 +36,7 @@
>  #include "mtk_iommu.h"
>  
>  #define REG_MMU_PT_BASE_ADDR 0x000
> +#define MMU_PT_ADDR_MASK GENMASK(31, 7)
>  
>  #define REG_MMU_INVALIDATE   0x020
>  #define F_ALL_INVLD  0x2
> @@ -54,7 +55,7 @@
>  #define REG_MMU_CTRL_REG 0x110
>  #define F_MMU_PREFETCH_RT_REPLACE_MODBIT(4)
>  #define F_MMU_TF_PROTECT_SEL_SHIFT(data) \
> - ((data)->plat_data->m4u_plat == M4U_MT2712 ? 4 : 5)
> + ((data)->plat_data->m4u_plat == M4U_MT8173 ? 5 : 4)
>  /* It's named by F_MMU_TF_PROT_SEL in mt2712. */
>  #define F_MMU_TF_PROTECT_SEL(prot, data) \
>   (((prot) & 0x3) << F_MMU_TF_PROTECT_SEL_SHIFT(data))
> @@ -347,7 +348,7 @@ static int mtk_iommu_attach_device(struct iommu_domain 
> *domain,
>   /* Update the pgtable base address register of the M4U HW */
>   if (!data->m4u_dom) {
>   data->m4u_dom = dom;
> - writel(dom->cfg.arm_v7s_cfg.ttbr[0],
> + writel(dom->cfg.arm_v7s_cfg.ttbr[0] & MMU_PT_ADDR_MASK,
>  data->base + REG_MMU_PT_BASE_ADDR);
>   }
>  
> @@ -510,6 +511,7 @@ static int mtk_iommu_of_xlate(struct device *dev, struct 
> of_phandle_args *args)
>  
>  static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
>  {
> + enum mtk_iommu_plat m4u_plat = data->plat_data->m4u_plat;
>   u32 regval;
>   int ret;
>  
> @@ -520,7 +522,7 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data 
> *data)
>   }
>  
>   regval = F_MMU_TF_PROTECT_SEL(2, data);
> - if (data->plat_data->m4u_plat == M4U_MT8173)
> + if (m4u_plat == M4U_MT8173)
>   regval |= F_MMU_PREFETCH_RT_REPLACE_MOD;
>   writel_relaxed(regval, data->base + REG_MMU_CTRL_REG);
>  
> @@ -541,14 +543,14 @@ static int mtk_iommu_hw_init(const struct 
> mtk_iommu_data *data)
>   F_INT_PRETETCH_TRANSATION_FIFO_FAULT;
>   writel_relaxed(regval, data->base + REG_MMU_INT_MAIN_CONTROL);
>  
> - if (data->plat_data->m4u_plat == M4U_MT8173)
> + if (m4u_plat == M4U_MT8173)
>   regval = (data->protect_base >> 1) | (data->enable_4GB << 31);
>   else
>   regval = lower_32_bits(data->protect_base) |
>upper_32_bits(data->protect_base);
>   writel_relaxed(regval, data->base + REG_MMU_IVRP_PADDR);
>  
> - if (data->enable_4GB && data->plat_data->m4u_plat != M4U_MT8173) {
> + if (data->enable_4GB && m4u_plat == M4U_MT2712) {
>   /*
>* If 4GB mode is enabled, the validate PA range is from
>* 0x1__ to 0x1__. here record bit[32:30].
> @@ -558,8 +560,11 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data 
> *data)
>   }
>   writel_relaxed(0, data->base + REG_MMU_DCM_DIS);
>  
> - /* It's MISC control register whose default value is ok except mt8173.*/
> - if (data->plat_data->m4u_plat == M4U_MT8173)
> + /*
> +  * It's MISC control register whose default value is ok
> +  * except mt8173 and mt8183.
> +  */
> + if (m4u_plat == M4U_MT8173 || m4u_plat == M4U_MT8183)
>   writel_relaxed(0, data->base + REG_MMU_STANDARD_AXI_MODE);
>  
>   if (devm_request_irq(data->dev, data->irq, mtk_iommu_isr, 0,
> @@ -713,6 +718,7 @@ static int __maybe_unused mtk_iommu_resume(struct device 
> *dev)
>  {
>   struct mtk_iommu_data *data = dev_get_drvdata(dev);
>   struct mtk_iommu_suspend_reg *reg = >reg;
> + struct mtk_iommu_domain *m4u_dom = data->m4u_dom;
>   void __iomem *base = data->base;
>   int ret;
>  
> @@ -728,8 +734,8 @@ static int __maybe_unused 

Re: [PATCH v4 10/18] iommu/mediatek: Add mt8183 IOMMU support

2018-12-21 Thread Yong Wu
On Fri, 2018-12-21 at 12:43 +0800, Nicolas Boichat wrote:
> On Sat, Dec 8, 2018 at 4:42 PM Yong Wu  wrote:
> >
> > The M4U IP blocks in mt8183 is MediaTek's generation2 M4U which use
> > the ARM Short-descriptor like mt8173, and most of the HW registers
> > are the same.
> >
> > Here list main differences between mt8183 and mt8173/mt2712:
> > 1) mt8183 has only one M4U HW like mt8173 while mt2712 has two.
> > 2) mt8183 don't have the "bclk" clock, it use the EMI clock instead.
> > 3) mt8183 can support the dram over 4GB, but it doesn't call this "4GB
> > mode".
> > 4) mt8183 pgtable base register(0x0) extend bit[1:0] which represent
> > the bit[33:32] in the physical address of the pgtable base, But the
> > standard ttbr0[1] means the S bit which is enabled defaultly, Hence,
> > we add a mask.
> > 5) mt8183 HW has a GALS modules, SMI should enable "has_gals" support.
> > 6) the larb-id in smi-common is remapped. M4U should enable
> > larbid_remapped support.
> >
> > Signed-off-by: Yong Wu 
> > ---
> >  drivers/iommu/mtk_iommu.c | 31 ++-
> >  drivers/iommu/mtk_iommu.h |  1 +
> >  drivers/memory/mtk-smi.c  | 19 +++
> >  3 files changed, 42 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > index 8ab3b69..d91a554 100644
> > --- a/drivers/iommu/mtk_iommu.c
> > +++ b/drivers/iommu/mtk_iommu.c
> > @@ -36,6 +36,7 @@
> >  #include "mtk_iommu.h"
> >
> >  #define REG_MMU_PT_BASE_ADDR   0x000
> > +#define MMU_PT_ADDR_MASK   GENMASK(31, 7)
> >
> >  #define REG_MMU_INVALIDATE 0x020
> >  #define F_ALL_INVLD0x2
> > @@ -54,7 +55,7 @@
> >  #define REG_MMU_CTRL_REG   0x110
> >  #define F_MMU_PREFETCH_RT_REPLACE_MOD  BIT(4)
> >  #define F_MMU_TF_PROTECT_SEL_SHIFT(data) \
> > -   ((data)->plat_data->m4u_plat == M4U_MT2712 ? 4 : 5)
> > +   ((data)->plat_data->m4u_plat == M4U_MT8173 ? 5 : 4)
> 
> Should the shift value be a member of plat_data instead?

It's also ok.
This TF_PROTECT_SEL MACRO looks a bit complex. I can use a new patch to
refactor it.

> 
> >  /* It's named by F_MMU_TF_PROT_SEL in mt2712. */
> >  #define F_MMU_TF_PROTECT_SEL(prot, data) \
> > (((prot) & 0x3) << F_MMU_TF_PROTECT_SEL_SHIFT(data))
> > @@ -347,7 +348,7 @@ static int mtk_iommu_attach_device(struct iommu_domain 
> > *domain,
> > /* Update the pgtable base address register of the M4U HW */
> > if (!data->m4u_dom) {
> > data->m4u_dom = dom;
> > -   writel(dom->cfg.arm_v7s_cfg.ttbr[0],
> > +   writel(dom->cfg.arm_v7s_cfg.ttbr[0] & MMU_PT_ADDR_MASK,
> >data->base + REG_MMU_PT_BASE_ADDR);
> > }
> >
> > @@ -510,6 +511,7 @@ static int mtk_iommu_of_xlate(struct device *dev, 
> > struct of_phandle_args *args)
> >
> >  static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
> >  {
> > +   enum mtk_iommu_plat m4u_plat = data->plat_data->m4u_plat;
> > u32 regval;
> > int ret;
> >
> > @@ -520,7 +522,7 @@ static int mtk_iommu_hw_init(const struct 
> > mtk_iommu_data *data)
> > }
> >
> > regval = F_MMU_TF_PROTECT_SEL(2, data);
> > -   if (data->plat_data->m4u_plat == M4U_MT8173)
> > +   if (m4u_plat == M4U_MT8173)
> > regval |= F_MMU_PREFETCH_RT_REPLACE_MOD;
> > writel_relaxed(regval, data->base + REG_MMU_CTRL_REG);
> >
> > @@ -541,14 +543,14 @@ static int mtk_iommu_hw_init(const struct 
> > mtk_iommu_data *data)
> > F_INT_PRETETCH_TRANSATION_FIFO_FAULT;
> > writel_relaxed(regval, data->base + REG_MMU_INT_MAIN_CONTROL);
> >
> > -   if (data->plat_data->m4u_plat == M4U_MT8173)
> > +   if (m4u_plat == M4U_MT8173)
> > regval = (data->protect_base >> 1) | (data->enable_4GB << 
> > 31);
> > else
> > regval = lower_32_bits(data->protect_base) |
> >  upper_32_bits(data->protect_base);
> > writel_relaxed(regval, data->base + REG_MMU_IVRP_PADDR);
> >
> > -   if (data->enable_4GB && data->plat_data->m4u_plat != M4U_MT8173) {
> > +   if (data->enable_4GB && m4u_plat == M4U_MT2712) {
> > /*
> >  * If 4GB mode is enabled, the validate PA range is from
> >  * 0x1__ to 0x1__. here record bit[32:30].
> > @@ -558,8 +560,11 @@ static int mtk_iommu_hw_init(const struct 
> > mtk_iommu_data *data)
> > }
> > writel_relaxed(0, data->base + REG_MMU_DCM_DIS);
> >
> > -   /* It's MISC control register whose default value is ok except 
> > mt8173.*/
> > -   if (data->plat_data->m4u_plat == M4U_MT8173)
> > +   /*
> > +* It's MISC control register whose default value is ok
> > +* except mt8173 and mt8183.
> > +*/
> > +   if (m4u_plat == M4U_MT8173 || m4u_plat == M4U_MT8183)
> 
> 

Re: [PATCH v4 10/18] iommu/mediatek: Add mt8183 IOMMU support

2018-12-20 Thread Nicolas Boichat
On Sat, Dec 8, 2018 at 4:42 PM Yong Wu  wrote:
>
> The M4U IP blocks in mt8183 is MediaTek's generation2 M4U which use
> the ARM Short-descriptor like mt8173, and most of the HW registers
> are the same.
>
> Here list main differences between mt8183 and mt8173/mt2712:
> 1) mt8183 has only one M4U HW like mt8173 while mt2712 has two.
> 2) mt8183 don't have the "bclk" clock, it use the EMI clock instead.
> 3) mt8183 can support the dram over 4GB, but it doesn't call this "4GB
> mode".
> 4) mt8183 pgtable base register(0x0) extend bit[1:0] which represent
> the bit[33:32] in the physical address of the pgtable base, But the
> standard ttbr0[1] means the S bit which is enabled defaultly, Hence,
> we add a mask.
> 5) mt8183 HW has a GALS modules, SMI should enable "has_gals" support.
> 6) the larb-id in smi-common is remapped. M4U should enable
> larbid_remapped support.
>
> Signed-off-by: Yong Wu 
> ---
>  drivers/iommu/mtk_iommu.c | 31 ++-
>  drivers/iommu/mtk_iommu.h |  1 +
>  drivers/memory/mtk-smi.c  | 19 +++
>  3 files changed, 42 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 8ab3b69..d91a554 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -36,6 +36,7 @@
>  #include "mtk_iommu.h"
>
>  #define REG_MMU_PT_BASE_ADDR   0x000
> +#define MMU_PT_ADDR_MASK   GENMASK(31, 7)
>
>  #define REG_MMU_INVALIDATE 0x020
>  #define F_ALL_INVLD0x2
> @@ -54,7 +55,7 @@
>  #define REG_MMU_CTRL_REG   0x110
>  #define F_MMU_PREFETCH_RT_REPLACE_MOD  BIT(4)
>  #define F_MMU_TF_PROTECT_SEL_SHIFT(data) \
> -   ((data)->plat_data->m4u_plat == M4U_MT2712 ? 4 : 5)
> +   ((data)->plat_data->m4u_plat == M4U_MT8173 ? 5 : 4)

Should the shift value be a member of plat_data instead?

>  /* It's named by F_MMU_TF_PROT_SEL in mt2712. */
>  #define F_MMU_TF_PROTECT_SEL(prot, data) \
> (((prot) & 0x3) << F_MMU_TF_PROTECT_SEL_SHIFT(data))
> @@ -347,7 +348,7 @@ static int mtk_iommu_attach_device(struct iommu_domain 
> *domain,
> /* Update the pgtable base address register of the M4U HW */
> if (!data->m4u_dom) {
> data->m4u_dom = dom;
> -   writel(dom->cfg.arm_v7s_cfg.ttbr[0],
> +   writel(dom->cfg.arm_v7s_cfg.ttbr[0] & MMU_PT_ADDR_MASK,
>data->base + REG_MMU_PT_BASE_ADDR);
> }
>
> @@ -510,6 +511,7 @@ static int mtk_iommu_of_xlate(struct device *dev, struct 
> of_phandle_args *args)
>
>  static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
>  {
> +   enum mtk_iommu_plat m4u_plat = data->plat_data->m4u_plat;
> u32 regval;
> int ret;
>
> @@ -520,7 +522,7 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data 
> *data)
> }
>
> regval = F_MMU_TF_PROTECT_SEL(2, data);
> -   if (data->plat_data->m4u_plat == M4U_MT8173)
> +   if (m4u_plat == M4U_MT8173)
> regval |= F_MMU_PREFETCH_RT_REPLACE_MOD;
> writel_relaxed(regval, data->base + REG_MMU_CTRL_REG);
>
> @@ -541,14 +543,14 @@ static int mtk_iommu_hw_init(const struct 
> mtk_iommu_data *data)
> F_INT_PRETETCH_TRANSATION_FIFO_FAULT;
> writel_relaxed(regval, data->base + REG_MMU_INT_MAIN_CONTROL);
>
> -   if (data->plat_data->m4u_plat == M4U_MT8173)
> +   if (m4u_plat == M4U_MT8173)
> regval = (data->protect_base >> 1) | (data->enable_4GB << 31);
> else
> regval = lower_32_bits(data->protect_base) |
>  upper_32_bits(data->protect_base);
> writel_relaxed(regval, data->base + REG_MMU_IVRP_PADDR);
>
> -   if (data->enable_4GB && data->plat_data->m4u_plat != M4U_MT8173) {
> +   if (data->enable_4GB && m4u_plat == M4U_MT2712) {
> /*
>  * If 4GB mode is enabled, the validate PA range is from
>  * 0x1__ to 0x1__. here record bit[32:30].
> @@ -558,8 +560,11 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data 
> *data)
> }
> writel_relaxed(0, data->base + REG_MMU_DCM_DIS);
>
> -   /* It's MISC control register whose default value is ok except 
> mt8173.*/
> -   if (data->plat_data->m4u_plat == M4U_MT8173)
> +   /*
> +* It's MISC control register whose default value is ok
> +* except mt8173 and mt8183.
> +*/
> +   if (m4u_plat == M4U_MT8173 || m4u_plat == M4U_MT8183)

Again, should this be a field in plat_data?

> writel_relaxed(0, data->base + REG_MMU_STANDARD_AXI_MODE);
>
> if (devm_request_irq(data->dev, data->irq, mtk_iommu_isr, 0,
> @@ -713,6 +718,7 @@ static int __maybe_unused mtk_iommu_resume(struct device 
> *dev)
>  {
> struct mtk_iommu_data *data = dev_get_drvdata(dev);
> struct mtk_iommu_suspend_reg 

[PATCH v4 10/18] iommu/mediatek: Add mt8183 IOMMU support

2018-12-08 Thread Yong Wu
The M4U IP blocks in mt8183 is MediaTek's generation2 M4U which use
the ARM Short-descriptor like mt8173, and most of the HW registers
are the same.

Here list main differences between mt8183 and mt8173/mt2712:
1) mt8183 has only one M4U HW like mt8173 while mt2712 has two.
2) mt8183 don't have the "bclk" clock, it use the EMI clock instead.
3) mt8183 can support the dram over 4GB, but it doesn't call this "4GB
mode".
4) mt8183 pgtable base register(0x0) extend bit[1:0] which represent
the bit[33:32] in the physical address of the pgtable base, But the
standard ttbr0[1] means the S bit which is enabled defaultly, Hence,
we add a mask.
5) mt8183 HW has a GALS modules, SMI should enable "has_gals" support.
6) the larb-id in smi-common is remapped. M4U should enable
larbid_remapped support.

Signed-off-by: Yong Wu 
---
 drivers/iommu/mtk_iommu.c | 31 ++-
 drivers/iommu/mtk_iommu.h |  1 +
 drivers/memory/mtk-smi.c  | 19 +++
 3 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 8ab3b69..d91a554 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -36,6 +36,7 @@
 #include "mtk_iommu.h"
 
 #define REG_MMU_PT_BASE_ADDR   0x000
+#define MMU_PT_ADDR_MASK   GENMASK(31, 7)
 
 #define REG_MMU_INVALIDATE 0x020
 #define F_ALL_INVLD0x2
@@ -54,7 +55,7 @@
 #define REG_MMU_CTRL_REG   0x110
 #define F_MMU_PREFETCH_RT_REPLACE_MOD  BIT(4)
 #define F_MMU_TF_PROTECT_SEL_SHIFT(data) \
-   ((data)->plat_data->m4u_plat == M4U_MT2712 ? 4 : 5)
+   ((data)->plat_data->m4u_plat == M4U_MT8173 ? 5 : 4)
 /* It's named by F_MMU_TF_PROT_SEL in mt2712. */
 #define F_MMU_TF_PROTECT_SEL(prot, data) \
(((prot) & 0x3) << F_MMU_TF_PROTECT_SEL_SHIFT(data))
@@ -347,7 +348,7 @@ static int mtk_iommu_attach_device(struct iommu_domain 
*domain,
/* Update the pgtable base address register of the M4U HW */
if (!data->m4u_dom) {
data->m4u_dom = dom;
-   writel(dom->cfg.arm_v7s_cfg.ttbr[0],
+   writel(dom->cfg.arm_v7s_cfg.ttbr[0] & MMU_PT_ADDR_MASK,
   data->base + REG_MMU_PT_BASE_ADDR);
}
 
@@ -510,6 +511,7 @@ static int mtk_iommu_of_xlate(struct device *dev, struct 
of_phandle_args *args)
 
 static int mtk_iommu_hw_init(const struct mtk_iommu_data *data)
 {
+   enum mtk_iommu_plat m4u_plat = data->plat_data->m4u_plat;
u32 regval;
int ret;
 
@@ -520,7 +522,7 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data 
*data)
}
 
regval = F_MMU_TF_PROTECT_SEL(2, data);
-   if (data->plat_data->m4u_plat == M4U_MT8173)
+   if (m4u_plat == M4U_MT8173)
regval |= F_MMU_PREFETCH_RT_REPLACE_MOD;
writel_relaxed(regval, data->base + REG_MMU_CTRL_REG);
 
@@ -541,14 +543,14 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data 
*data)
F_INT_PRETETCH_TRANSATION_FIFO_FAULT;
writel_relaxed(regval, data->base + REG_MMU_INT_MAIN_CONTROL);
 
-   if (data->plat_data->m4u_plat == M4U_MT8173)
+   if (m4u_plat == M4U_MT8173)
regval = (data->protect_base >> 1) | (data->enable_4GB << 31);
else
regval = lower_32_bits(data->protect_base) |
 upper_32_bits(data->protect_base);
writel_relaxed(regval, data->base + REG_MMU_IVRP_PADDR);
 
-   if (data->enable_4GB && data->plat_data->m4u_plat != M4U_MT8173) {
+   if (data->enable_4GB && m4u_plat == M4U_MT2712) {
/*
 * If 4GB mode is enabled, the validate PA range is from
 * 0x1__ to 0x1__. here record bit[32:30].
@@ -558,8 +560,11 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data 
*data)
}
writel_relaxed(0, data->base + REG_MMU_DCM_DIS);
 
-   /* It's MISC control register whose default value is ok except mt8173.*/
-   if (data->plat_data->m4u_plat == M4U_MT8173)
+   /*
+* It's MISC control register whose default value is ok
+* except mt8173 and mt8183.
+*/
+   if (m4u_plat == M4U_MT8173 || m4u_plat == M4U_MT8183)
writel_relaxed(0, data->base + REG_MMU_STANDARD_AXI_MODE);
 
if (devm_request_irq(data->dev, data->irq, mtk_iommu_isr, 0,
@@ -713,6 +718,7 @@ static int __maybe_unused mtk_iommu_resume(struct device 
*dev)
 {
struct mtk_iommu_data *data = dev_get_drvdata(dev);
struct mtk_iommu_suspend_reg *reg = >reg;
+   struct mtk_iommu_domain *m4u_dom = data->m4u_dom;
void __iomem *base = data->base;
int ret;
 
@@ -728,8 +734,8 @@ static int __maybe_unused mtk_iommu_resume(struct device 
*dev)
writel_relaxed(reg->int_control0, base + REG_MMU_INT_CONTROL0);
writel_relaxed(reg->int_main_control, base +