Hi Chao,
This is also ok for me. Only two format nitpick.
On Mon, 2020-06-29 at 15:13 +0800, Chao Hao wrote:
> Given the fact that we are adding more and more plat_data bool values,
> it would make sense to use a u32 flags register and add the appropriate
> macro definitions to set and check for a flag present.
> No functional change.
>
> Suggested-by: Matthias Brugger <matthias.bgg@xxxxxxxxx>
> Signed-off-by: Chao Hao <chao.hao@xxxxxxxxxxxx>
> ---
[snip]
> static const struct mtk_iommu_plat_data mt2712_data = {
> .m4u_plat = M4U_MT2712,
> - .has_4gb_mode = true,
> - .has_bclk = true,
> - .has_vld_pa_rng = true,
> + .flags = HAS_4GB_MODE |
> + HAS_BCLK |
> + HAS_VLD_PA_RNG,
short enough. we can put it in one line?
> .larbid_remap = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9},
> };
>
> static const struct mtk_iommu_plat_data mt8173_data = {
> .m4u_plat = M4U_MT8173,
> - .has_4gb_mode = true,
> - .has_bclk = true,
> - .reset_axi = true,
> + .flags = HAS_4GB_MODE |
> + HAS_BCLK |
> + RESET_AXI,
> .larbid_remap = {0, 1, 2, 3, 4, 5}, /* Linear mapping. */
> };
>
> static const struct mtk_iommu_plat_data mt8183_data = {
> .m4u_plat = M4U_MT8183,
> - .reset_axi = true,
> + .flags = RESET_AXI,
> .larbid_remap = {0, 4, 5, 6, 7, 2, 3, 1},
> };
>
> diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
> index 1b6ea839b92c..7cc39f729263 100644
> --- a/drivers/iommu/mtk_iommu.h
> +++ b/drivers/iommu/mtk_iommu.h
> @@ -17,6 +17,15 @@
> #include <linux/spinlock.h>
> #include <soc/mediatek/smi.h>
>
> +#define HAS_4GB_MODE BIT(0)
> +/* HW will use the EMI clock if there isn't the "bclk". */
> +#define HAS_BCLK BIT(1)
> +#define HAS_VLD_PA_RNG BIT(2)
> +#define RESET_AXI BIT(3)
> +
> +#define MTK_IOMMU_HAS_FLAG(pdata, _x) \
> + ((((pdata)->flags) & (_x)) == (_x))
If these definitions are not used in mtk_iommu_v1.c(also no this plan),
then we can put them in the mtk_iommu.c.
BTW, the patch title "modify the usage of mtk_iommu_plat_data structure"
isn't so clear, we could write what the detailed modification is.
something like:
iommu/mediatek: Use a u32 flags to describe different HW features
> +
> struct mtk_iommu_suspend_reg {
> u32 misc_ctrl;
> u32 dcm_dis;
> @@ -36,12 +45,7 @@ enum mtk_iommu_plat {
>
> struct mtk_iommu_plat_data {
> enum mtk_iommu_plat m4u_plat;
> - bool has_4gb_mode;
> -
> - /* HW will use the EMI clock if there isn't the "bclk". */
> - bool has_bclk;
> - bool has_vld_pa_rng;
> - bool reset_axi;
> + u32 flags;
> unsigned char larbid_remap[MTK_LARB_NR_MAX];
> };
>
