Re: [PATCH v3 2/3] memory: mtk-smi: Add enable IOMMU SMC command for MM master

Re: [PATCH v3 2/3] memory: mtk-smi: Add enable IOMMU SMC command for MM master

[Yong Wu]

On Wed, 2022-07-27 at 18:45 +0800, Chengci.Xu wrote:
> For concerns about security, the register to enable/disable IOMMU of
> SMI LARB should only be configured in secure world. Thus, we add some
> SMC command for multimedia master to enable/disable MM IOMMU in ATF
> by
> setting the register of SMI LARB. This function is prepared for
> MT8188.
> 
> Signed-off-by: Chengci.Xu <chengci.xu@mediatek.com>
> ---
>  drivers/memory/mtk-smi.c                 | 11 +++++++++++
>  include/linux/soc/mediatek/mtk_sip_svc.h |  3 +++
>  include/soc/mediatek/smi.h               |  7 +++++++
>  3 files changed, 21 insertions(+)
> 
> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> index d7cb7ead2ac7..41ce66c39123 100644
> --- a/drivers/memory/mtk-smi.c
> +++ b/drivers/memory/mtk-smi.c
> @@ -3,6 +3,7 @@
>   * Copyright (c) 2015-2016 MediaTek Inc.
>   * Author: Yong Wu <yong.wu@mediatek.com>
>   */
> +#include <linux/arm-smccc.h>
>  #include <linux/clk.h>
>  #include <linux/component.h>
>  #include <linux/device.h>
> @@ -14,6 +15,7 @@
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/soc/mediatek/mtk_sip_svc.h>
>  #include <soc/mediatek/smi.h>
>  #include <dt-bindings/memory/mt2701-larb-port.h>
>  #include <dt-bindings/memory/mtk-memory-port.h>
> @@ -89,6 +91,7 @@
>  #define MTK_SMI_FLAG_THRT_UPDATE	BIT(0)
>  #define MTK_SMI_FLAG_SW_FLAG		BIT(1)
>  #define MTK_SMI_FLAG_SLEEP_CTL		BIT(2)
> +#define MTK_SMI_FLAG_SEC_REG		BIT(3)

Rename more detailly. This is about the configuring port registers.

something like: MTK_SMI_FLAG_CFG_PORT_SEC_CTL

>  #define MTK_SMI_CAPS(flags, _x)		(!!((flags) & (_x)))
>  
>  struct mtk_smi_reg_pair {
> @@ -235,6 +238,7 @@ static void
> mtk_smi_larb_config_port_gen2_general(struct device *dev)
>  	struct mtk_smi_larb *larb = dev_get_drvdata(dev);
>  	u32 reg, flags_general = larb->larb_gen->flags_general;
>  	const u8 *larbostd = larb->larb_gen->ostd ? larb->larb_gen-
> >ostd[larb->larbid] : NULL;
> +	struct arm_smccc_res res;
>  	int i;
>  
>  	if (BIT(larb->larbid) & larb->larb_gen-
> >larb_direct_to_common_mask)
> @@ -259,6 +263,13 @@ static void
> mtk_smi_larb_config_port_gen2_general(struct device *dev)
>  		reg |= BANK_SEL(larb->bank[i]);
>  		writel(reg, larb->base + SMI_LARB_NONSEC_CON(i));
>  	}
> +
> +	if (MTK_SMI_CAPS(flags_general, MTK_SMI_FLAG_SEC_REG)) {
> +		arm_smccc_smc(MTK_SIP_KERNEL_IOMMU_CONTROL,
> IOMMU_ATF_CMD_CONFIG_SMI_LARB,
> +			      larb->larbid, (u32)(*larb->mmu), 0, 0, 0,
> 0, &res);
> +		if (res.a0 != 0)
> +			dev_err(dev, "Enable iommu fail, ret %ld\n",
> res.a0);
> +	}

In this case, Do we still need write SMI_LARB_NONSEC_CON above? If no,
  
      if (MTK_SMI_CAPS(flags_general, MTK_SMI_FLAG_SEC_REG)) {
          xx
          return;
      }
      and put this before updating SMI_LARB_NONSEC_CON.

If yes. we should add some comment.

And, Could we ignore the fail result? 

>  }
>  
>  static const u8 mtk_smi_larb_mt8195_ostd[][SMI_LARB_PORT_NR_MAX] = {
> diff --git a/include/linux/soc/mediatek/mtk_sip_svc.h
> b/include/linux/soc/mediatek/mtk_sip_svc.h
> index 082398e0cfb1..0761128b4354 100644
> --- a/include/linux/soc/mediatek/mtk_sip_svc.h
> +++ b/include/linux/soc/mediatek/mtk_sip_svc.h
> @@ -22,4 +22,7 @@
>  	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, MTK_SIP_SMC_CONVENTION,
> \
>  			   ARM_SMCCC_OWNER_SIP, fn_id)
>  
> +/* IOMMU related SMC call */
> +#define MTK_SIP_KERNEL_IOMMU_CONTROL	MTK_SIP_SMC_CMD(0x514)
> +
>  #endif
> diff --git a/include/soc/mediatek/smi.h b/include/soc/mediatek/smi.h
> index 11f7d6b59642..8c781b7bd88d 100644
> --- a/include/soc/mediatek/smi.h
> +++ b/include/soc/mediatek/smi.h
> @@ -9,6 +9,13 @@
>  #include <linux/bitops.h>
>  #include <linux/device.h>
>  
> +/* IOMMU & SMI ATF CMD */
> +
> +enum IOMMU_ATF_CMD {
> +	IOMMU_ATF_CMD_CONFIG_SMI_LARB,		/* For mm master to
> en/disable iommu */
> +	IOMMU_ATF_CMD_COUNT,

IOMMU_ATF_CMD_MAX?
> +};
> +

This enum only is used in IOMMU and SMI. Put it below inside the macro
CONFIG_MTK_SMI.


>  #if IS_ENABLED(CONFIG_MTK_SMI)
>  
>  #define MTK_SMI_MMU_EN(port)	BIT(port)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 2/3] memory: mtk-smi: Add enable IOMMU SMC command for MM master

[Chengci.Xu]

On Thu, 2022-07-28 at 14:58 +0800, Yong Wu wrote:
> On Wed, 2022-07-27 at 18:45 +0800, Chengci.Xu wrote:
> > For concerns about security, the register to enable/disable IOMMU
> > of
> > SMI LARB should only be configured in secure world. Thus, we add
> > some
> > SMC command for multimedia master to enable/disable MM IOMMU in ATF
> > by
> > setting the register of SMI LARB. This function is prepared for
> > MT8188.
> > 
> > Signed-off-by: Chengci.Xu <chengci.xu@mediatek.com>
> > ---
> >  drivers/memory/mtk-smi.c                 | 11 +++++++++++
> >  include/linux/soc/mediatek/mtk_sip_svc.h |  3 +++
> >  include/soc/mediatek/smi.h               |  7 +++++++
> >  3 files changed, 21 insertions(+)
> > 
> > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> > index d7cb7ead2ac7..41ce66c39123 100644
> > --- a/drivers/memory/mtk-smi.c
> > +++ b/drivers/memory/mtk-smi.c
> > @@ -3,6 +3,7 @@
> >   * Copyright (c) 2015-2016 MediaTek Inc.
> >   * Author: Yong Wu <yong.wu@mediatek.com>
> >   */
> > +#include <linux/arm-smccc.h>
> >  #include <linux/clk.h>
> >  #include <linux/component.h>
> >  #include <linux/device.h>
> > @@ -14,6 +15,7 @@
> >  #include <linux/of_platform.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/pm_runtime.h>
> > +#include <linux/soc/mediatek/mtk_sip_svc.h>
> >  #include <soc/mediatek/smi.h>
> >  #include <dt-bindings/memory/mt2701-larb-port.h>
> >  #include <dt-bindings/memory/mtk-memory-port.h>
> > @@ -89,6 +91,7 @@
> >  #define MTK_SMI_FLAG_THRT_UPDATE	BIT(0)
> >  #define MTK_SMI_FLAG_SW_FLAG		BIT(1)
> >  #define MTK_SMI_FLAG_SLEEP_CTL		BIT(2)
> > +#define MTK_SMI_FLAG_SEC_REG		BIT(3)
> 
> Rename more detailly. This is about the configuring port registers.
> 
> something like: MTK_SMI_FLAG_CFG_PORT_SEC_CTL

OK, rename thie item to MTK_SMI_FLAG_CFG_PORT_SEC_CTL.

> 
> >  #define MTK_SMI_CAPS(flags, _x)		(!!((flags) & (_x)))
> >  
> >  struct mtk_smi_reg_pair {
> > @@ -235,6 +238,7 @@ static void
> > mtk_smi_larb_config_port_gen2_general(struct device *dev)
> >  	struct mtk_smi_larb *larb = dev_get_drvdata(dev);
> >  	u32 reg, flags_general = larb->larb_gen->flags_general;
> >  	const u8 *larbostd = larb->larb_gen->ostd ? larb->larb_gen-
> > > ostd[larb->larbid] : NULL;
> > 
> > +	struct arm_smccc_res res;
> >  	int i;
> >  
> >  	if (BIT(larb->larbid) & larb->larb_gen-
> > > larb_direct_to_common_mask)
> > 
> > @@ -259,6 +263,13 @@ static void
> > mtk_smi_larb_config_port_gen2_general(struct device *dev)
> >  		reg |= BANK_SEL(larb->bank[i]);
> >  		writel(reg, larb->base + SMI_LARB_NONSEC_CON(i));
> >  	}
> > +
> > +	if (MTK_SMI_CAPS(flags_general, MTK_SMI_FLAG_SEC_REG)) {
> > +		arm_smccc_smc(MTK_SIP_KERNEL_IOMMU_CONTROL,
> > IOMMU_ATF_CMD_CONFIG_SMI_LARB,
> > +			      larb->larbid, (u32)(*larb->mmu), 0, 0, 0,
> > 0, &res);
> > +		if (res.a0 != 0)
> > +			dev_err(dev, "Enable iommu fail, ret %ld\n",
> > res.a0);
> > +	}
> 
> In this case, Do we still need write SMI_LARB_NONSEC_CON above? If
> no,
>   
>       if (MTK_SMI_CAPS(flags_general, MTK_SMI_FLAG_SEC_REG)) {
>           xx
>           return;
>       }
>       and put this before updating SMI_LARB_NONSEC_CON.
> 
> If yes. we should add some comment.

If we send an invalid parameter or something unpredictable happened,
such as ATF does not have related code, this SMC call will returns an
error, which means SMI-larb's register may in a HW default state.

In MT8188 we still need write SMI_LARB_NONSEC_CON for the HW default
value will enable IOMMU when SMC call failed. If we not write this
register, bank select function will lost. Thus, all the master that
with IOMMU default enable will lost the high bit[33:32], and send an
IOVA of 0~4G to IOMMU. That is abnormal. 

So I think it's better to add some comment.

> 
> And, Could we ignore the fail result? 
> 

This failure should not be ignored. If some masters want to disable
IOMMU and use PA to access DRAM for special case, this failure will
left IOMMU in enable state and lead to translation fault.

So I will add a return value for this function.

> >  }
> >  
> >  static const u8 mtk_smi_larb_mt8195_ostd[][SMI_LARB_PORT_NR_MAX] =
> > {
> > diff --git a/include/linux/soc/mediatek/mtk_sip_svc.h
> > b/include/linux/soc/mediatek/mtk_sip_svc.h
> > index 082398e0cfb1..0761128b4354 100644
> > --- a/include/linux/soc/mediatek/mtk_sip_svc.h
> > +++ b/include/linux/soc/mediatek/mtk_sip_svc.h
> > @@ -22,4 +22,7 @@
> >  	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, MTK_SIP_SMC_CONVENTION,
> > \
> >  			   ARM_SMCCC_OWNER_SIP, fn_id)
> >  
> > +/* IOMMU related SMC call */
> > +#define MTK_SIP_KERNEL_IOMMU_CONTROL	MTK_SIP_SMC_CMD(0x514)
> > +
> >  #endif
> > diff --git a/include/soc/mediatek/smi.h
> > b/include/soc/mediatek/smi.h
> > index 11f7d6b59642..8c781b7bd88d 100644
> > --- a/include/soc/mediatek/smi.h
> > +++ b/include/soc/mediatek/smi.h
> > @@ -9,6 +9,13 @@
> >  #include <linux/bitops.h>
> >  #include <linux/device.h>
> >  
> > +/* IOMMU & SMI ATF CMD */
> > +
> > +enum IOMMU_ATF_CMD {
> > +	IOMMU_ATF_CMD_CONFIG_SMI_LARB,		/* For mm master to
> > en/disable iommu */
> > +	IOMMU_ATF_CMD_COUNT,
> 
> IOMMU_ATF_CMD_MAX?
> > +};
> > +
> 
> This enum only is used in IOMMU and SMI. Put it below inside the
> macro
> CONFIG_MTK_SMI.
> 

OK, got it.

> 
> >  #if IS_ENABLED(CONFIG_MTK_SMI)
> >  
> >  #define MTK_SMI_MMU_EN(port)	BIT(port)
> 
> 


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

Re: [PATCH v3 2/3] memory: mtk-smi: Add enable IOMMU SMC command for MM master

[AngeloGioacchino Del Regno]

Il 27/07/22 12:45, Chengci.Xu ha scritto:
> For concerns about security, the register to enable/disable IOMMU of
> SMI LARB should only be configured in secure world. Thus, we add some
> SMC command for multimedia master to enable/disable MM IOMMU in ATF by
> setting the register of SMI LARB. This function is prepared for MT8188.
> 
> Signed-off-by: Chengci.Xu <chengci.xu@mediatek.com>
> ---
>   drivers/memory/mtk-smi.c                 | 11 +++++++++++
>   include/linux/soc/mediatek/mtk_sip_svc.h |  3 +++
>   include/soc/mediatek/smi.h               |  7 +++++++
>   3 files changed, 21 insertions(+)
> 
> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> index d7cb7ead2ac7..41ce66c39123 100644
> --- a/drivers/memory/mtk-smi.c
> +++ b/drivers/memory/mtk-smi.c
> @@ -3,6 +3,7 @@
>    * Copyright (c) 2015-2016 MediaTek Inc.
>    * Author: Yong Wu <yong.wu@mediatek.com>
>    */
> +#include <linux/arm-smccc.h>
>   #include <linux/clk.h>
>   #include <linux/component.h>
>   #include <linux/device.h>
> @@ -14,6 +15,7 @@
>   #include <linux/of_platform.h>
>   #include <linux/platform_device.h>
>   #include <linux/pm_runtime.h>
> +#include <linux/soc/mediatek/mtk_sip_svc.h>
>   #include <soc/mediatek/smi.h>
>   #include <dt-bindings/memory/mt2701-larb-port.h>
>   #include <dt-bindings/memory/mtk-memory-port.h>
> @@ -89,6 +91,7 @@
>   #define MTK_SMI_FLAG_THRT_UPDATE	BIT(0)
>   #define MTK_SMI_FLAG_SW_FLAG		BIT(1)
>   #define MTK_SMI_FLAG_SLEEP_CTL		BIT(2)
> +#define MTK_SMI_FLAG_SEC_REG		BIT(3)
>   #define MTK_SMI_CAPS(flags, _x)		(!!((flags) & (_x)))
>   
>   struct mtk_smi_reg_pair {
> @@ -235,6 +238,7 @@ static void mtk_smi_larb_config_port_gen2_general(struct device *dev)
>   	struct mtk_smi_larb *larb = dev_get_drvdata(dev);
>   	u32 reg, flags_general = larb->larb_gen->flags_general;
>   	const u8 *larbostd = larb->larb_gen->ostd ? larb->larb_gen->ostd[larb->larbid] : NULL;
> +	struct arm_smccc_res res;
>   	int i;
>   
>   	if (BIT(larb->larbid) & larb->larb_gen->larb_direct_to_common_mask)
> @@ -259,6 +263,13 @@ static void mtk_smi_larb_config_port_gen2_general(struct device *dev)
>   		reg |= BANK_SEL(larb->bank[i]);
>   		writel(reg, larb->base + SMI_LARB_NONSEC_CON(i));
>   	}
> +
> +	if (MTK_SMI_CAPS(flags_general, MTK_SMI_FLAG_SEC_REG)) {
> +		arm_smccc_smc(MTK_SIP_KERNEL_IOMMU_CONTROL, IOMMU_ATF_CMD_CONFIG_SMI_LARB,
> +			      larb->larbid, (u32)(*larb->mmu), 0, 0, 0, 0, &res);
> +		if (res.a0 != 0)
> +			dev_err(dev, "Enable iommu fail, ret %ld\n", res.a0);

This means that the system will eventually crash or anyway be unstable: in this
case, we should not allow further interaction with the IOMMUs and/or SMI.

So, if you place this here, you will have to change this function to return
something for the caller to take action.

> +	}
>   }
>   
>   static const u8 mtk_smi_larb_mt8195_ostd[][SMI_LARB_PORT_NR_MAX] = {
> diff --git a/include/linux/soc/mediatek/mtk_sip_svc.h b/include/linux/soc/mediatek/mtk_sip_svc.h
> index 082398e0cfb1..0761128b4354 100644
> --- a/include/linux/soc/mediatek/mtk_sip_svc.h
> +++ b/include/linux/soc/mediatek/mtk_sip_svc.h
> @@ -22,4 +22,7 @@
>   	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, MTK_SIP_SMC_CONVENTION, \
>   			   ARM_SMCCC_OWNER_SIP, fn_id)
>   
> +/* IOMMU related SMC call */
> +#define MTK_SIP_KERNEL_IOMMU_CONTROL	MTK_SIP_SMC_CMD(0x514)
> +
>   #endif
> diff --git a/include/soc/mediatek/smi.h b/include/soc/mediatek/smi.h
> index 11f7d6b59642..8c781b7bd88d 100644
> --- a/include/soc/mediatek/smi.h
> +++ b/include/soc/mediatek/smi.h
> @@ -9,6 +9,13 @@
>   #include <linux/bitops.h>
>   #include <linux/device.h>
>   
> +/* IOMMU & SMI ATF CMD */
> +
> +enum IOMMU_ATF_CMD {

Why do you have an enumeration when you're defining just one command?
Is it expected to have more?

Besides, the enum name should be lower case, and...

> +	IOMMU_ATF_CMD_CONFIG_SMI_LARB,		/* For mm master to en/disable iommu */
> +	IOMMU_ATF_CMD_COUNT,

if IOMMU_ATF_CMD_COUNT means "end of this enumeration", please call it
IOMMU_ATF_CMD_MAX instead.

> +};
> +
>   #if IS_ENABLED(CONFIG_MTK_SMI)
>   
>   #define MTK_SMI_MMU_EN(port)	BIT(port)


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

Re: [PATCH v3 2/3] memory: mtk-smi: Add enable IOMMU SMC command for MM master

[Chengci.Xu]

On Thu, 2022-07-28 at 13:11 +0200, AngeloGioacchino Del Regno wrote:
> Il 27/07/22 12:45, Chengci.Xu ha scritto:
> > For concerns about security, the register to enable/disable IOMMU
> > of
> > SMI LARB should only be configured in secure world. Thus, we add
> > some
> > SMC command for multimedia master to enable/disable MM IOMMU in ATF
> > by
> > setting the register of SMI LARB. This function is prepared for
> > MT8188.
> > 
> > Signed-off-by: Chengci.Xu <chengci.xu@mediatek.com>
> > ---
> >   drivers/memory/mtk-smi.c                 | 11 +++++++++++
> >   include/linux/soc/mediatek/mtk_sip_svc.h |  3 +++
> >   include/soc/mediatek/smi.h               |  7 +++++++
> >   3 files changed, 21 insertions(+)
> > 
> > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> > index d7cb7ead2ac7..41ce66c39123 100644
> > --- a/drivers/memory/mtk-smi.c
> > +++ b/drivers/memory/mtk-smi.c
> > @@ -3,6 +3,7 @@
> >    * Copyright (c) 2015-2016 MediaTek Inc.
> >    * Author: Yong Wu <yong.wu@mediatek.com>
> >    */
> > +#include <linux/arm-smccc.h>
> >   #include <linux/clk.h>
> >   #include <linux/component.h>
> >   #include <linux/device.h>
> > @@ -14,6 +15,7 @@
> >   #include <linux/of_platform.h>
> >   #include <linux/platform_device.h>
> >   #include <linux/pm_runtime.h>
> > +#include <linux/soc/mediatek/mtk_sip_svc.h>
> >   #include <soc/mediatek/smi.h>
> >   #include <dt-bindings/memory/mt2701-larb-port.h>
> >   #include <dt-bindings/memory/mtk-memory-port.h>
> > @@ -89,6 +91,7 @@
> >   #define MTK_SMI_FLAG_THRT_UPDATE	BIT(0)
> >   #define MTK_SMI_FLAG_SW_FLAG		BIT(1)
> >   #define MTK_SMI_FLAG_SLEEP_CTL		BIT(2)
> > +#define MTK_SMI_FLAG_SEC_REG		BIT(3)
> >   #define MTK_SMI_CAPS(flags, _x)		(!!((flags) & (_x)))
> >   
> >   struct mtk_smi_reg_pair {
> > @@ -235,6 +238,7 @@ static void
> > mtk_smi_larb_config_port_gen2_general(struct device *dev)
> >   	struct mtk_smi_larb *larb = dev_get_drvdata(dev);
> >   	u32 reg, flags_general = larb->larb_gen->flags_general;
> >   	const u8 *larbostd = larb->larb_gen->ostd ? larb->larb_gen-
> > >ostd[larb->larbid] : NULL;
> > +	struct arm_smccc_res res;
> >   	int i;
> >   
> >   	if (BIT(larb->larbid) & larb->larb_gen-
> > >larb_direct_to_common_mask)
> > @@ -259,6 +263,13 @@ static void
> > mtk_smi_larb_config_port_gen2_general(struct device *dev)
> >   		reg |= BANK_SEL(larb->bank[i]);
> >   		writel(reg, larb->base + SMI_LARB_NONSEC_CON(i));
> >   	}
> > +
> > +	if (MTK_SMI_CAPS(flags_general, MTK_SMI_FLAG_SEC_REG)) {
> > +		arm_smccc_smc(MTK_SIP_KERNEL_IOMMU_CONTROL,
> > IOMMU_ATF_CMD_CONFIG_SMI_LARB,
> > +			      larb->larbid, (u32)(*larb->mmu), 0, 0, 0,
> > 0, &res);
> > +		if (res.a0 != 0)
> > +			dev_err(dev, "Enable iommu fail, ret %ld\n",
> > res.a0);
> 
> This means that the system will eventually crash or anyway be
> unstable: in this
> case, we should not allow further interaction with the IOMMUs and/or
> SMI.
> 
> So, if you place this here, you will have to change this function to
> return
> something for the caller to take action.

Thanks, a new patch will add a return value for this function.
And, we will return -EINVAL when enable iommu failed.

> 
> > +	}
> >   }
> >   
> >   static const u8 mtk_smi_larb_mt8195_ostd[][SMI_LARB_PORT_NR_MAX]
> > = {
> > diff --git a/include/linux/soc/mediatek/mtk_sip_svc.h
> > b/include/linux/soc/mediatek/mtk_sip_svc.h
> > index 082398e0cfb1..0761128b4354 100644
> > --- a/include/linux/soc/mediatek/mtk_sip_svc.h
> > +++ b/include/linux/soc/mediatek/mtk_sip_svc.h
> > @@ -22,4 +22,7 @@
> >   	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, MTK_SIP_SMC_CONVENTION,
> > \
> >   			   ARM_SMCCC_OWNER_SIP, fn_id)
> >   
> > +/* IOMMU related SMC call */
> > +#define MTK_SIP_KERNEL_IOMMU_CONTROL	MTK_SIP_SMC_CMD(0x514)
> > +
> >   #endif
> > diff --git a/include/soc/mediatek/smi.h
> > b/include/soc/mediatek/smi.h
> > index 11f7d6b59642..8c781b7bd88d 100644
> > --- a/include/soc/mediatek/smi.h
> > +++ b/include/soc/mediatek/smi.h
> > @@ -9,6 +9,13 @@
> >   #include <linux/bitops.h>
> >   #include <linux/device.h>
> >   
> > +/* IOMMU & SMI ATF CMD */
> > +
> > +enum IOMMU_ATF_CMD {
> 
> Why do you have an enumeration when you're defining just one command?
> Is it expected to have more?

Yes, we will have another command for INFRA IOMMU enable/disable.

> 
> Besides, the enum name should be lower case, and...

Thanks, got it.

> 
> > +	IOMMU_ATF_CMD_CONFIG_SMI_LARB,		/* For mm master to
> > en/disable iommu */
> > +	IOMMU_ATF_CMD_COUNT,
> 
> if IOMMU_ATF_CMD_COUNT means "end of this enumeration", please call
> it
> IOMMU_ATF_CMD_MAX instead.

Thanks, got it.

> 
> > +};
> > +
> >   #if IS_ENABLED(CONFIG_MTK_SMI)
> >   
> >   #define MTK_SMI_MMU_EN(port)	BIT(port)
> 
> 


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

Re: [PATCH v3 3/3] memory: mtk-smi: mt8188: Add SMI Support

[Yong Wu]

On Wed, 2022-07-27 at 18:45 +0800, Chengci.Xu wrote:
> Add mt8188 smi common & larb support
> 
> Signed-off-by: Chengci.Xu <chengci.xu@mediatek.com>

Reviewed-by: Yong Wu <yong.wu@mediatek.com>

> ---
>  drivers/memory/mtk-smi.c | 71
> ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 3/3] memory: mtk-smi: mt8188: Add SMI Support

[AngeloGioacchino Del Regno]

Il 27/07/22 12:45, Chengci.Xu ha scritto:
> Add mt8188 smi common & larb support
> 
> Signed-off-by: Chengci.Xu <chengci.xu@mediatek.com>
> Reviewed-by: Yong Wu <yong.wu@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>


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

Re: [PATCH v3 1/3] dt-bindings: memory: mediatek: Add mt8188 smi binding

[AngeloGioacchino Del Regno]

Il 27/07/22 12:45, Chengci.Xu ha scritto:
> Add mt8188 smi supporting in the bindings.
> 
> In mt8188, there are two smi-common HW, one is for vdo(video output),
> the other is for vpp(video processing pipe). They connect with different
> smi-larbs, then some setting(bus_sel) is different. Differentiate them
> with the compatible string.
> 
> Something like this:
> 
>     IOMMU(VDO)          IOMMU(VPP)
>         |                   |
> SMI_COMMON_VDO       SMI_COMMON_VPP
> ----------------     ----------------
>     |     |   ...       |     |    ...
>   larb0 larb2 ...     larb1 larb3  ...
> 
> Signed-off-by: Chengci.Xu <chengci.xu@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>


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