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 03/18] memory: mtk-smi: Use a general config_port interface

2018-12-21 Thread Yong Wu
On Fri, 2018-12-21 at 18:47 +0100, Matthias Brugger wrote:
> 
> On 08/12/2018 09:39, Yong Wu wrote:
> > The config_port of mt2712 and mt8183 are the same. Use a general
> > config_port interface instead.
> > 
> > In addition, in mt2712, larb8 and larb9 are the bdpsys larbs which
> > are not the normal larb, their register space are different from the
> > normal one. thus, we can not call the general config_port. In mt8183,
> > IPU0/1 and CCU connect with smi-common directly, they also are not
> > the normal larb. Hence, we add a "larb_special_mask" for these special
> > larbs.
> > 
> > This is also a preparing patch for adding mt8183 SMI support.
> > 
> > Signed-off-by: Yong Wu 
> > ---
> >  drivers/memory/mtk-smi.c | 12 +---
> >  1 file changed, 5 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> > index 8f2d152..3b9ad0e 100644
> > --- a/drivers/memory/mtk-smi.c
> > +++ b/drivers/memory/mtk-smi.c
> > @@ -53,6 +53,7 @@ struct mtk_smi_larb_gen {
> > bool need_larbid;
> > int port_in_larb[MTK_LARB_NR_MAX + 1];
> > void (*config_port)(struct device *);
> > +   unsigned int larb_special_mask; /* The special larbs mask. */
> 
> I'm not really happy with the name larb_special_mask but I can't think of
> anything else. The comment is not needed as it just rewords the name of the
> variable.

Thanks this comment. then I reword more detail instead this common
"special", How about "larb_to_common_directly_mask"?

> 
> Other then that (or even without changing anything):
> 
> Reviewed-by: Matthias Brugger 

Thanks.

> 
> >  };
> >  
> >  struct mtk_smi {
> > @@ -176,17 +177,13 @@ void mtk_smi_larb_put(struct device *larbdev)
> > return -ENODEV;
> >  }
> >  
> > -static void mtk_smi_larb_config_port_mt2712(struct device *dev)
> > +static void mtk_smi_larb_config_port_gen2_general(struct device *dev)
> >  {
> > struct mtk_smi_larb *larb = dev_get_drvdata(dev);
> > u32 reg;
> > int i;
> >  
> > -   /*
> > -* larb 8/9 is the bdpsys larb, the iommu_en is enabled defaultly.
> > -* Don't need to set it again.
> > -*/
> > -   if (larb->larbid == 8 || larb->larbid == 9)
> > +   if (BIT(larb->larbid) & larb->larb_gen->larb_special_mask)
> > return;
> >  
> > for_each_set_bit(i, (unsigned long *)larb->mmu, 32) {
> > @@ -261,7 +258,8 @@ static void mtk_smi_larb_config_port_gen1(struct device 
> > *dev)
> >  
> >  static const struct mtk_smi_larb_gen mtk_smi_larb_mt2712 = {
> > .need_larbid = true,
> > -   .config_port = mtk_smi_larb_config_port_mt2712,
> > +   .config_port   = mtk_smi_larb_config_port_gen2_general,
> > +   .larb_special_mask = BIT(8) | BIT(9),  /* bdpsys */
> >  };
> >  
> >  static const struct of_device_id mtk_smi_larb_of_ids[] = {
> > 


___
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 04/18] memory: mtk-smi: Use a struct for the platform data for smi-common

2018-12-21 Thread Matthias Brugger



On 08/12/2018 09:39, Yong Wu wrote:
> Use a struct as the platform special data instead of the enumeration.
> 
> Also there is a minor change that moving the position of
> "enum mtk_smi_gen" definition, this is because we expect define
> "struct mtk_smi_common_plat" before it is referred.
> 
> This is a preparing patch for mt8183.
> 
> Signed-off-by: Yong Wu 

Reviewed-by: Matthias Brugger 

> ---
>  drivers/memory/mtk-smi.c | 35 ---
>  1 file changed, 24 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> index 3b9ad0e..a5ddd42 100644
> --- a/drivers/memory/mtk-smi.c
> +++ b/drivers/memory/mtk-smi.c
> @@ -49,6 +49,15 @@
>  #define SMI_LARB_NONSEC_CON(id)  (0x380 + ((id) * 4))
>  #define F_MMU_EN BIT(0)
>  
> +enum mtk_smi_gen {
> + MTK_SMI_GEN1,
> + MTK_SMI_GEN2
> +};
> +
> +struct mtk_smi_common_plat {
> + enum mtk_smi_gen gen;
> +};
> +
>  struct mtk_smi_larb_gen {
>   bool need_larbid;
>   int port_in_larb[MTK_LARB_NR_MAX + 1];
> @@ -61,6 +70,8 @@ struct mtk_smi {
>   struct clk  *clk_apb, *clk_smi;
>   struct clk  *clk_async; /*only needed by mt2701*/
>   void __iomem*smi_ao_base;
> +
> + const struct mtk_smi_common_plat *plat;
>  };
>  
>  struct mtk_smi_larb { /* larb: local arbiter */
> @@ -72,11 +83,6 @@ struct mtk_smi_larb { /* larb: local arbiter */
>   u32 *mmu;
>  };
>  
> -enum mtk_smi_gen {
> - MTK_SMI_GEN1,
> - MTK_SMI_GEN2
> -};
> -
>  static int mtk_smi_enable(const struct mtk_smi *smi)
>  {
>   int ret;
> @@ -351,18 +357,26 @@ static int mtk_smi_larb_remove(struct platform_device 
> *pdev)
>   }
>  };
>  
> +static const struct mtk_smi_common_plat mtk_smi_common_gen1 = {
> + .gen = MTK_SMI_GEN1,
> +};
> +
> +static const struct mtk_smi_common_plat mtk_smi_common_gen2 = {
> + .gen = MTK_SMI_GEN2,
> +};
> +
>  static const struct of_device_id mtk_smi_common_of_ids[] = {
>   {
>   .compatible = "mediatek,mt8173-smi-common",
> - .data = (void *)MTK_SMI_GEN2
> + .data = _smi_common_gen2,
>   },
>   {
>   .compatible = "mediatek,mt2701-smi-common",
> - .data = (void *)MTK_SMI_GEN1
> + .data = _smi_common_gen1,
>   },
>   {
>   .compatible = "mediatek,mt2712-smi-common",
> - .data = (void *)MTK_SMI_GEN2
> + .data = _smi_common_gen2,
>   },
>   {}
>  };
> @@ -372,13 +386,13 @@ static int mtk_smi_common_probe(struct platform_device 
> *pdev)
>   struct device *dev = >dev;
>   struct mtk_smi *common;
>   struct resource *res;
> - enum mtk_smi_gen smi_gen;
>   int ret;
>  
>   common = devm_kzalloc(dev, sizeof(*common), GFP_KERNEL);
>   if (!common)
>   return -ENOMEM;
>   common->dev = dev;
> + common->plat = of_device_get_match_data(dev);
>  
>   common->clk_apb = devm_clk_get(dev, "apb");
>   if (IS_ERR(common->clk_apb))
> @@ -394,8 +408,7 @@ static int mtk_smi_common_probe(struct platform_device 
> *pdev)
>* clock into emi clock domain, but for mtk smi gen2, there's no smi ao
>* base.
>*/
> - smi_gen = (enum mtk_smi_gen)of_device_get_match_data(dev);
> - if (smi_gen == MTK_SMI_GEN1) {
> + if (common->plat->gen == MTK_SMI_GEN1) {
>   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   common->smi_ao_base = devm_ioremap_resource(dev, res);
>   if (IS_ERR(common->smi_ao_base))
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v4 03/18] memory: mtk-smi: Use a general config_port interface

2018-12-21 Thread Matthias Brugger



On 08/12/2018 09:39, Yong Wu wrote:
> The config_port of mt2712 and mt8183 are the same. Use a general
> config_port interface instead.
> 
> In addition, in mt2712, larb8 and larb9 are the bdpsys larbs which
> are not the normal larb, their register space are different from the
> normal one. thus, we can not call the general config_port. In mt8183,
> IPU0/1 and CCU connect with smi-common directly, they also are not
> the normal larb. Hence, we add a "larb_special_mask" for these special
> larbs.
> 
> This is also a preparing patch for adding mt8183 SMI support.
> 
> Signed-off-by: Yong Wu 
> ---
>  drivers/memory/mtk-smi.c | 12 +---
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> index 8f2d152..3b9ad0e 100644
> --- a/drivers/memory/mtk-smi.c
> +++ b/drivers/memory/mtk-smi.c
> @@ -53,6 +53,7 @@ struct mtk_smi_larb_gen {
>   bool need_larbid;
>   int port_in_larb[MTK_LARB_NR_MAX + 1];
>   void (*config_port)(struct device *);
> + unsigned int larb_special_mask; /* The special larbs mask. */

I'm not really happy with the name larb_special_mask but I can't think of
anything else. The comment is not needed as it just rewords the name of the
variable.

Other then that (or even without changing anything):

Reviewed-by: Matthias Brugger 

>  };
>  
>  struct mtk_smi {
> @@ -176,17 +177,13 @@ void mtk_smi_larb_put(struct device *larbdev)
>   return -ENODEV;
>  }
>  
> -static void mtk_smi_larb_config_port_mt2712(struct device *dev)
> +static void mtk_smi_larb_config_port_gen2_general(struct device *dev)
>  {
>   struct mtk_smi_larb *larb = dev_get_drvdata(dev);
>   u32 reg;
>   int i;
>  
> - /*
> -  * larb 8/9 is the bdpsys larb, the iommu_en is enabled defaultly.
> -  * Don't need to set it again.
> -  */
> - if (larb->larbid == 8 || larb->larbid == 9)
> + if (BIT(larb->larbid) & larb->larb_gen->larb_special_mask)
>   return;
>  
>   for_each_set_bit(i, (unsigned long *)larb->mmu, 32) {
> @@ -261,7 +258,8 @@ static void mtk_smi_larb_config_port_gen1(struct device 
> *dev)
>  
>  static const struct mtk_smi_larb_gen mtk_smi_larb_mt2712 = {
>   .need_larbid = true,
> - .config_port = mtk_smi_larb_config_port_mt2712,
> + .config_port   = mtk_smi_larb_config_port_gen2_general,
> + .larb_special_mask = BIT(8) | BIT(9),  /* bdpsys */
>  };
>  
>  static const struct of_device_id mtk_smi_larb_of_ids[] = {
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v4 02/18] iommu/mediatek: Use a struct as the platform data

2018-12-21 Thread Matthias Brugger



On 08/12/2018 09:39, Yong Wu wrote:
> Use a struct as the platform special data instead of the enumeration.
> This is a prepare patch for adding mt8183 iommu support.
> 
> Signed-off-by: Yong Wu 
> ---

Reviewed-by: Matthias Brugger 

>  drivers/iommu/mtk_iommu.c | 24 
>  drivers/iommu/mtk_iommu.h |  6 +-
>  2 files changed, 21 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 44bd5b9..9a2225b 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -54,7 +54,7 @@
>  #define REG_MMU_CTRL_REG 0x110
>  #define F_MMU_PREFETCH_RT_REPLACE_MODBIT(4)
>  #define F_MMU_TF_PROTECT_SEL_SHIFT(data) \
> - ((data)->m4u_plat == M4U_MT2712 ? 4 : 5)
> + ((data)->plat_data->m4u_plat == M4U_MT2712 ? 4 : 5)
>  /* 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))
> @@ -517,7 +517,7 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data 
> *data)
>   }
>  
>   regval = F_MMU_TF_PROTECT_SEL(2, data);
> - if (data->m4u_plat == M4U_MT8173)
> + if (data->plat_data->m4u_plat == M4U_MT8173)
>   regval |= F_MMU_PREFETCH_RT_REPLACE_MOD;
>   writel_relaxed(regval, data->base + REG_MMU_CTRL_REG);
>  
> @@ -538,14 +538,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->m4u_plat == M4U_MT8173)
> + if (data->plat_data->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->m4u_plat != M4U_MT8173) {
> + if (data->enable_4GB && data->plat_data->m4u_plat != M4U_MT8173) {
>   /*
>* If 4GB mode is enabled, the validate PA range is from
>* 0x1__ to 0x1__. here record bit[32:30].
> @@ -556,7 +556,7 @@ 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->m4u_plat == M4U_MT8173)
> + if (data->plat_data->m4u_plat == M4U_MT8173)
>   writel_relaxed(0, data->base + REG_MMU_STANDARD_AXI_MODE);
>  
>   if (devm_request_irq(data->dev, data->irq, mtk_iommu_isr, 0,
> @@ -589,7 +589,7 @@ static int mtk_iommu_probe(struct platform_device *pdev)
>   if (!data)
>   return -ENOMEM;
>   data->dev = dev;
> - data->m4u_plat = (enum mtk_iommu_plat)of_device_get_match_data(dev);
> + data->plat_data = of_device_get_match_data(dev);
>  
>   /* Protect memory. HW will access here while translation fault.*/
>   protect = devm_kzalloc(dev, MTK_PROTECT_PA_ALIGN * 2, GFP_KERNEL);
> @@ -733,9 +733,17 @@ static int __maybe_unused mtk_iommu_resume(struct device 
> *dev)
>   SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(mtk_iommu_suspend, mtk_iommu_resume)
>  };
>  
> +static const struct mtk_iommu_plat_data mt2712_data = {
> + .m4u_plat = M4U_MT2712,
> +};
> +
> +static const struct mtk_iommu_plat_data mt8173_data = {
> + .m4u_plat = M4U_MT8173,
> +};
> +
>  static const struct of_device_id mtk_iommu_of_ids[] = {
> - { .compatible = "mediatek,mt2712-m4u", .data = (void *)M4U_MT2712},
> - { .compatible = "mediatek,mt8173-m4u", .data = (void *)M4U_MT8173},
> + { .compatible = "mediatek,mt2712-m4u", .data = _data},
> + { .compatible = "mediatek,mt8173-m4u", .data = _data},
>   {}
>  };
>  
> diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
> index 778498b..333a0ef 100644
> --- a/drivers/iommu/mtk_iommu.h
> +++ b/drivers/iommu/mtk_iommu.h
> @@ -41,6 +41,10 @@ enum mtk_iommu_plat {
>   M4U_MT8173,
>  };
>  
> +struct mtk_iommu_plat_data {
> + enum mtk_iommu_plat m4u_plat;
> +};
> +
>  struct mtk_iommu_domain;
>  
>  struct mtk_iommu_data {
> @@ -57,7 +61,7 @@ struct mtk_iommu_data {
>   booltlb_flush_active;
>  
>   struct iommu_device iommu;
> - enum mtk_iommu_plat m4u_plat;
> + const struct mtk_iommu_plat_data *plat_data;
>  
>   struct list_headlist;
>  };
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: question about broadcast invalidation and atc invalidation

2018-12-21 Thread Jean-Philippe Brucker
Hi,

On 21/12/2018 09:31, Zhongmiao wrote:
> Hi Jean:
>   Now Soft need to make sure build atc inv cmd to smmu after Execute 
> "TLBI" (dvm sync). I don't think it's very reasonable. Why  Smmu The 
> smmu automatically sends the inv atc command when smmu receive 
> broadcast invalidation ?

I'd also like to see this done by hardware, but it's not
straightforward. I see two difficulties with turning broadcast TLBI into
ATC invalidation:

* TLBI uses VMID and ASID to target the TLB entries to invalidate, but
ATS uses RID and PASID. The SMMU would need to do a reverse lookup to
convert VMID:ASID into RID:PASID, and there may be multiple RID:PASID
pairs associated to one VMID:ASID.

* ATS flow control. Each endpoint has a limited number of in-flight ATC
invalidate requests. If the CPU is sending TLBI too fast, the SMMU would
have to buffer them or back-pressure the interconnect, while waiting for
in-flight ATC invalidate to complete.

So for the moment the architecture doesn't support this. We use MMU
notifiers to convert VMID:ASID into RID:PASID and wait for ATC
invalidation to complete.

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


Re: [GIT PULL] dma-mapping updates for Linux 4.21

2018-12-21 Thread Christoph Hellwig
On Fri, Dec 21, 2018 at 10:45:49AM +0100, Christoph Hellwig wrote:
> The following changes since commit c9d76d0655c06b8c1f944e46c4fd9e9cf4b331c0:
> 
>   dma-mapping: fix return type of dma_set_max_seg_size() (2018-11-27 08:39:52 
> +0100)

And that actually is the first commit already in the tree, sorry.

Updated git-request-pull output attached:


The following changes since commit ef78e5ec9214376c5cb989f5da70b02d0c117b66:

  ia64: export node_distance function (2018-11-26 18:30:40 -0800)

are available in the Git repository at:

  git://git.infradead.org/users/hch/dma-mapping.git tags/dma-mapping-4.21

for you to fetch changes up to 8b1cce9f5832a8eda17d37a3c49fb7dd2d650f46:

  dma-mapping: fix inverted logic in dma_supported (2018-12-20 17:47:55 +0100)


DMA mapping updates for Linux 4.21

A huge update this time, but a lot of that is just consolidating or
removing code:

 - provide a common DMA_MAPPING_ERROR definition and avoid indirect
   calls for dma_map_* error checking
 - use direct calls for the DMA direct mapping case, avoiding huge
   retpoline overhead for high performance workloads
 - merge the swiotlb dma_map_ops into dma-direct
 - provide a generic remapping DMA consistent allocator for architectures
   that have devices that perform DMA that is not cache coherent. Based
   on the existing arm64 implementation and also used for csky now.
 - improve the dma-debug infrastructure, including dynamic allocation
   of entries (Robin Murphy)
 - default to providing chaining scatterlist everywhere, with opt-outs
   for the few architectures (alpha, parisc, most arm32 variants) that
   can't cope with it
 - misc sparc32 dma-related cleanups
 - remove the dma_mark_clean arch hook used by swiotlb on ia64 and
   replace it with the generic noncoherent infrastructure
 - fix the return type of dma_set_max_seg_size (Niklas Söderlund)
 - move the dummy dma ops for not DMA capable devices from arm64 to
   common code (Robin Murphy)
 - ensure dma_alloc_coherent returns zeroed memory to avoid kernel data
   leaks through userspace.  We already did this for most common
   architectures, but this ensures we do it everywhere.
   dma_zalloc_coherent has been deprecated and can hopefully be
   removed after -rc1 with a coccinelle script.


Christoph Hellwig (60):
  dma-direct: provide page based alloc/free helpers
  dma-direct: reject highmem pages from dma_alloc_from_contiguous
  dma-mapping: move the remap helpers to a separate file
  dma-mapping: move the arm64 noncoherent alloc/free support to common code
  dma-mapping: support highmem in the generic remap allocator
  dma-remap: support DMA_ATTR_NO_KERNEL_MAPPING
  csky: don't select DMA_NONCOHERENT_OPS
  csky: don't use GFP_DMA in atomic_pool_init
  csky: use the generic remapping dma alloc implementation
  dma-mapping: provide a generic DMA_MAPPING_ERROR
  dma-direct: remove the mapping_error dma_map_ops method
  arm: remove the mapping_error dma_map_ops method
  powerpc/iommu: remove the mapping_error dma_map_ops method
  mips/jazz: remove the mapping_error dma_map_ops method
  s390: remove the mapping_error dma_map_ops method
  sparc: remove the mapping_error dma_map_ops method
  parisc/ccio: remove the mapping_error dma_map_ops method
  parisc/sba_iommu: remove the mapping_error dma_map_ops method
  arm64: remove the dummy_dma_ops mapping_error method
  alpha: remove the mapping_error dma_map_ops method
  ia64/sba_iommu: improve internal map_page users
  ia64/sba_iommu: remove the mapping_error dma_map_ops method
  ia64/sn: remove the mapping_error dma_map_ops method
  x86/amd_gart: remove the mapping_error dma_map_ops method
  x86/calgary: remove the mapping_error dma_map_ops method
  iommu: remove the mapping_error dma_map_ops method
  iommu/intel: small map_page cleanup
  iommu/vt-d: remove the mapping_error dma_map_ops method
  iommu/dma-iommu: remove the mapping_error dma_map_ops method
  xen-swiotlb: remove the mapping_error dma_map_ops method
  dma-mapping: remove the mapping_error dma_map_ops method
  dma-mapping: return an error code from dma_mapping_error
  arch: switch the default on ARCH_HAS_SG_CHAIN
  sparc: remove not needed sbus_dma_ops methods
  sparc: factor the dma coherent mapping into helper
  sparc: remove the sparc32_dma_ops indirection
  sparc: remove not required includes from dma-mapping.h
  sparc: move the leon PCI memory space comment to 
  sparc: merge 32-bit and 64-bit version of pci.h
  sparc: use DT node full_name in sparc_dma_alloc_resource
  dma-mapping: remove a pointless memset in dma_atomic_pool_init
  dma-mapping: simplify the dma_sync_single_range_for_{cpu,device} 
implementation
  dma-mapping: merge dma_unmap_page_attrs 

[GIT PULL] dma-mapping updates for Linux 4.21

2018-12-21 Thread Christoph Hellwig
Hi Linus,

an early pull request as requested.  Besides the usual contextual
conflicts in Kconfig files that can be solved by taking both updates
and applying them manually there are some real conflicts this time:

  - the RISC-V tree has some updates to
Documentation/features/io/sg-chain/arch-support.txt, which is
removed in this tree.  It should be put to rest.
  - the sparc64 tree has various updates to replace the name field
with the full_name one in the OF device, including in code
that gets refctored in the dma-mapping tree.  I've applied an
equivalent change to the refactored version, so take the
dma-mapping tree version of arch/sparc/kernel/ioport.c.


The following changes since commit c9d76d0655c06b8c1f944e46c4fd9e9cf4b331c0:

  dma-mapping: fix return type of dma_set_max_seg_size() (2018-11-27 08:39:52 
+0100)

are available in the Git repository at:

  git://git.infradead.org/users/hch/dma-mapping.git tags/dma-mapping-4.21

for you to fetch changes up to 8b1cce9f5832a8eda17d37a3c49fb7dd2d650f46:

  dma-mapping: fix inverted logic in dma_supported (2018-12-20 17:47:55 +0100)


DMA mapping updates for Linux 4.21

A huge update this time, but a lot of that is just consolidating or
removing code:

 - provide a common DMA_MAPPING_ERROR definition and avoid indirect
   calls for dma_map_* error checking
 - use direct calls for the DMA direct mapping case, avoiding huge
   retpoline overhead for high performance workloads
 - merge the swiotlb dma_map_ops into dma-direct
 - provide a generic remapping DMA consistent allocator for architectures
   that have devices that perform DMA that is not cache coherent. Based
   on the existing arm64 implementation and also used for csky now.
 - improve the dma-debug infrastructure, including dynamic allocation
   of entries (Robin Murphy)
 - default to providing chaining scatterlist everywhere, with opt-outs
   for the few architectures (alpha, parisc, most arm32 variants) that
   can't cope with it
 - misc sparc32 dma-related cleanups
 - remove the dma_mark_clean arch hook used by swiotlb on ia64 and
   replace it with the generic noncoherent infrastructure
 - fix the return type of dma_set_max_seg_size (Niklas Söderlund)
 - move the dummy dma ops for not DMA capable devices from arm64 to
   common code (Robin Murphy)
 - ensure dma_alloc_coherent returns zeroed memory to avoid kernel data
   leaks through userspace.  We already did this for most common
   architectures, but this ensures we do it everywhere.
   dma_zalloc_coherent has been deprecated and can hopefully be
   removed after -rc1 with a coccinelle script.


Christoph Hellwig (60):
  dma-direct: provide page based alloc/free helpers
  dma-direct: reject highmem pages from dma_alloc_from_contiguous
  dma-mapping: move the remap helpers to a separate file
  dma-mapping: move the arm64 noncoherent alloc/free support to common code
  dma-mapping: support highmem in the generic remap allocator
  dma-remap: support DMA_ATTR_NO_KERNEL_MAPPING
  csky: don't select DMA_NONCOHERENT_OPS
  csky: don't use GFP_DMA in atomic_pool_init
  csky: use the generic remapping dma alloc implementation
  dma-mapping: provide a generic DMA_MAPPING_ERROR
  dma-direct: remove the mapping_error dma_map_ops method
  arm: remove the mapping_error dma_map_ops method
  powerpc/iommu: remove the mapping_error dma_map_ops method
  mips/jazz: remove the mapping_error dma_map_ops method
  s390: remove the mapping_error dma_map_ops method
  sparc: remove the mapping_error dma_map_ops method
  parisc/ccio: remove the mapping_error dma_map_ops method
  parisc/sba_iommu: remove the mapping_error dma_map_ops method
  arm64: remove the dummy_dma_ops mapping_error method
  alpha: remove the mapping_error dma_map_ops method
  ia64/sba_iommu: improve internal map_page users
  ia64/sba_iommu: remove the mapping_error dma_map_ops method
  ia64/sn: remove the mapping_error dma_map_ops method
  x86/amd_gart: remove the mapping_error dma_map_ops method
  x86/calgary: remove the mapping_error dma_map_ops method
  iommu: remove the mapping_error dma_map_ops method
  iommu/intel: small map_page cleanup
  iommu/vt-d: remove the mapping_error dma_map_ops method
  iommu/dma-iommu: remove the mapping_error dma_map_ops method
  xen-swiotlb: remove the mapping_error dma_map_ops method
  dma-mapping: remove the mapping_error dma_map_ops method
  dma-mapping: return an error code from dma_mapping_error
  arch: switch the default on ARCH_HAS_SG_CHAIN
  sparc: remove not needed sbus_dma_ops methods
  sparc: factor the dma coherent mapping into helper
  sparc: remove the sparc32_dma_ops indirection
  sparc: remove not required includes from dma-mapping.h
  sparc: 

Re: [PATCH v4 08/18] iommu/mediatek: Add larb-id remapped support

2018-12-21 Thread Yong Wu
Hi Nicolas,

Thanks for the review of this patchset.

On Fri, 2018-12-21 at 11:35 +0800, Nicolas Boichat wrote:
> On Sat, Dec 8, 2018 at 4:42 PM Yong Wu  wrote:
> >
> > The larb-id may be remapped in the smi-common, this means the
> > larb-id reported in the mtk_iommu_isr isn't the real larb-id,
> >
> > Take mt8183 as a example:
> >M4U
> > |
> > -
> > |   SMI common  |
> > -0-7-5-6-1-2--3-4- <- Id remapped
> >  | | | | | |  | |
> > larb0 larb1 IPU0  IPU1 larb4 larb5  larb6  CCU
> > disp  vdec  img   cam   venc  imgcam
> > As above, larb0 connects with the id 0 in smi-common.
> >   larb1 connects with the id 7 in smi-common.
> >   ...
> > If the larb-id reported in the isr is 7, actually it's larb1(vdec).
> > In order to output the right larb-id in the isr, we add a larb-id
> > remapping relationship in this patch.
> >
> > This also is a preparing patch for mt8183.
> >
> > Signed-off-by: Yong Wu 
> > ---
> >  drivers/iommu/mtk_iommu.c | 3 +++
> >  drivers/iommu/mtk_iommu.h | 4 
> >  2 files changed, 7 insertions(+)
> >
> > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > index eda062a..8ab3b69 100644
> > --- a/drivers/iommu/mtk_iommu.c
> > +++ b/drivers/iommu/mtk_iommu.c
> > @@ -220,6 +220,9 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
> > fault_larb = F_MMU0_INT_ID_LARB_ID(regval);
> > fault_port = F_MMU0_INT_ID_PORT_ID(regval);
> >
> > +   if (data->plat_data->larbid_remap_enable)
> > +   fault_larb = data->plat_data->larbid_remapped[fault_larb];
> > +
> > if (report_iommu_fault(>domain, data->dev, fault_iova,
> >write ? IOMMU_FAULT_WRITE : 
> > IOMMU_FAULT_READ)) {
> > dev_err_ratelimited(
> > diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
> > index b8749ac..3877050 100644
> > --- a/drivers/iommu/mtk_iommu.h
> > +++ b/drivers/iommu/mtk_iommu.h
> > @@ -47,6 +47,10 @@ struct mtk_iommu_plat_data {
> >
> > /* HW will use the EMI clock if there isn't the "bclk". */
> > boolhas_bclk;
> > +
> > +   /* The larb-id may be remapped in the smi-common. */
> > +   boollarbid_remap_enable;
> > +   unsigned intlarbid_remapped[MTK_LARB_NR_MAX];
> 
> Wouldn't it be a little simpler if you just had
> larbid_remap[MTK_LARB_NR_MAX] (no larbid_remap_enable), and just set
> it to {0, 1, 2, 3, 4, 5, 6, 7} in platforms that don't need
> complicated remapping?

Actually I'd like the original way(Print the larb-id from the register
directly if larb-id is not remapped). But this solution is also ok for
me.

> 
> Also, unsigned char/u8 array would be enough.

OK. "unsigned char" is enough.
Originally I think "int" or "long" may be better to access in ARM/ARM64.

> 
> >  };
> >
> >  struct mtk_iommu_domain;
> > --
> > 1.9.1
> >


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


Re: [PATCH v4 13/18] memory: mtk-smi: Add bus_sel for mt8183

2018-12-21 Thread Yong Wu
On Fri, 2018-12-21 at 12:47 +0800, Nicolas Boichat wrote:
> On Sat, Dec 8, 2018 at 4:43 PM Yong Wu  wrote:
> >
> > There are 2 mmu cells in a M4U HW. we could adjust some larbs entering
> > mmu0 or mmu1 to balance the bandwidth via the smi-common register
> > SMI_BUS_SEL(0x220)(Each larb occupy 2 bits).
> >
> > In mt8183, For better performance, we switch larb1/2/5/7 to enter
> > mmu1 while the others still keep enter mmu0.
> >
> > In mt8173 and mt2712, we don't get the performance issue,
> > Keep its default value(0x0), that means all the larbs enter mmu0.
> >
> > Signed-off-by: Yong Wu 
> > ---
> >  drivers/memory/mtk-smi.c | 22 --
> >  1 file changed, 20 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> > index ee6165e..88eb61a 100644
> > --- a/drivers/memory/mtk-smi.c
> > +++ b/drivers/memory/mtk-smi.c
> > @@ -49,6 +49,12 @@
> >  #define SMI_LARB_NONSEC_CON(id)(0x380 + ((id) * 4))
> >  #define F_MMU_EN   BIT(0)
> >
> > +/* SMI COMMON */
> > +#define SMI_BUS_SEL0x220
> > +#define SMI_BUS_LARB_SHIFT(larbid) ((larbid) << 1)
> > +/* All are MMU0 defaultly. Only specialize mmu1 here. */
> > +#define F_MMU1_LARB(larbid)(0x1 << SMI_BUS_LARB_SHIFT(larbid))
> > +
> >  enum mtk_smi_gen {
> > MTK_SMI_GEN1,
> > MTK_SMI_GEN2
> > @@ -57,6 +63,7 @@ enum mtk_smi_gen {
> >  struct mtk_smi_common_plat {
> > enum mtk_smi_gen gen;
> > bool has_gals;
> > +   u32  bus_sel; /* Balance some larbs to enter mmu0 or 
> > mmu1 */
> >  };
> >
> >  struct mtk_smi_larb_gen {
> > @@ -72,8 +79,8 @@ struct mtk_smi {
> > struct clk  *clk_apb, *clk_smi;
> > struct clk  *clk_gals0, *clk_gals1;
> > struct clk  *clk_async; /*only needed by 
> > mt2701*/
> > -   void __iomem*smi_ao_base;
> > -
> > +   void __iomem*smi_ao_base; /* only for gen1 */
> > +   void __iomem*base;/* only for gen2 */
> > const struct mtk_smi_common_plat *plat;
> >  };
> >
> > @@ -409,6 +416,8 @@ static int __maybe_unused mtk_smi_larb_suspend(struct 
> > device *dev)
> >  static const struct mtk_smi_common_plat mtk_smi_common_mt8183 = {
> > .gen  = MTK_SMI_GEN2,
> > .has_gals = true,
> > +   .bus_sel  = F_MMU1_LARB(1) | F_MMU1_LARB(2) | F_MMU1_LARB(5) |
> > +   F_MMU1_LARB(7),
> 
> Maybe it's ok for now, but I wonder if this is something that should
> be specified in device tree? Maybe different applications will want
> different larb split between MMU0 and MMU1?

Good question. Many Thanks. This value is recommended from the HW DE. In
some SoCs, It may be different in different scenarios. and in some SoCs
like mt8183, the bus_sel always use a fixed value.

I guess it should not be in device tree if it may be changed in
different applications.

BTW, Is there some existed IOCTL command or the similar interface that
can tell the kernel driver(SMI here) which scenario it is currently,
like it is playing video or the camera is running. 

Currently the SMI driver is so simple, it doesn't know which application
it is. All use the HW default value. of course, the default also is ok,
only the bandwidth may be not so good.

For the bus_sel of mt8183, I think it is ok right now. Sure, it can be
improved if we can change it dynamically.

> 
> >  };
> >
> >  static const struct of_device_id mtk_smi_common_of_ids[] = {
> > @@ -481,6 +490,11 @@ static int mtk_smi_common_probe(struct platform_device 
> > *pdev)
> > ret = clk_prepare_enable(common->clk_async);
> > if (ret)
> > return ret;
> > +   } else {
> > +   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +   common->base = devm_ioremap_resource(dev, res);
> > +   if (IS_ERR(common->base))
> > +   return PTR_ERR(common->base);
> > }
> > pm_runtime_enable(dev);
> > platform_set_drvdata(pdev, common);
> > @@ -496,6 +510,7 @@ static int mtk_smi_common_remove(struct platform_device 
> > *pdev)
> >  static int __maybe_unused mtk_smi_common_resume(struct device *dev)
> >  {
> > struct mtk_smi *common = dev_get_drvdata(dev);
> > +   u32 bus_sel = common->plat->bus_sel;
> > int ret;
> >
> > ret = mtk_smi_clk_enable(common);
> > @@ -503,6 +518,9 @@ static int __maybe_unused mtk_smi_common_resume(struct 
> > device *dev)
> > dev_err(common->dev, "Failed to enable clock(%d).\n", ret);
> > return ret;
> > }
> > +
> > +   if (common->plat->gen == MTK_SMI_GEN2 && bus_sel)
> > +   writel(bus_sel, common->base + SMI_BUS_SEL);
> > return 0;
> >  }
> >
> > --
> > 1.9.1
> >



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)
> 
>