| Message ID | 20220513151411.395744-3-angelogioacchino.delregno@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | MediaTek Helio X10 MT6795 - M4U/IOMMU Support | expand |
On Fri, 2022-05-13 at 17:14 +0200, AngeloGioacchino Del Regno wrote: > Add support for the M4Us found in the MT6795 Helio X10 SoC. > > Signed-off-by: AngeloGioacchino Del Regno < > angelogioacchino.delregno@collabora.com> > --- > drivers/iommu/mtk_iommu.c | 20 +++++++++++++++++++- > 1 file changed, 19 insertions(+), 1 deletion(-) > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > index 71b2ace74cd6..3d802dd3f377 100644 > --- a/drivers/iommu/mtk_iommu.c > +++ b/drivers/iommu/mtk_iommu.c > @@ -157,6 +157,7 @@ > enum mtk_iommu_plat { > M4U_MT2712, > M4U_MT6779, > + M4U_MT6795, > M4U_MT8167, > M4U_MT8173, > M4U_MT8183, > @@ -953,7 +954,8 @@ static int mtk_iommu_hw_init(const struct > mtk_iommu_data *data, unsigned int ban > * Global control settings are in bank0. May re-init these > global registers > * since no sure if there is bank0 consumers. > */ > - if (data->plat_data->m4u_plat == M4U_MT8173) { > + if (data->plat_data->m4u_plat == M4U_MT6795 || > + data->plat_data->m4u_plat == M4U_MT8173) { > regval = F_MMU_PREFETCH_RT_REPLACE_MOD | > F_MMU_TF_PROT_TO_PROGRAM_ADDR_MT8173; > } else { > @@ -1138,6 +1140,9 @@ static int mtk_iommu_probe(struct > platform_device *pdev) > case M4U_MT2712: > p = "mediatek,mt2712-infracfg"; > break; > + case M4U_MT6795: > + p = "mediatek,mt6795-infracfg"; > + break; > case M4U_MT8173: > p = "mediatek,mt8173-infracfg"; > break; > @@ -1404,6 +1409,18 @@ static const struct mtk_iommu_plat_data > mt6779_data = { > .larbid_remap = {{0}, {1}, {2}, {3}, {5}, {7, 8}, {10}, {9}}, > }; > > +static const struct mtk_iommu_plat_data mt6795_data = { > + .m4u_plat = M4U_MT6795, > + .flags = HAS_4GB_MODE | HAS_BCLK | RESET_AXI | > + HAS_LEGACY_IVRP_PADDR | MTK_IOMMU_TYPE_MM, > + .inv_sel_reg = REG_MMU_INV_SEL_GEN1, > + .banks_num = 1, > + .banks_enable = {true}, > + .iova_region = single_domain, > + .iova_region_nr = ARRAY_SIZE(single_domain), > + .larbid_remap = {{0}, {1}, {2}, {3}, {4}}, /* Linear mapping. > */ > +}; This is nearly same with mt8173_data. mt8173 has one more larb than mt6795, its larbid_remap is also ok for mt6795. thus it looks we could use mt8173 as the backward compatible. compatible = "mediatek,mt6795-m4u", "mediatek,mt8173-m4u"; After this, the only thing is about "mediatek,mt6795-infracfg". we have to try again with mediatek,mt6795-infracfg after mediatek,mt8173- infracfg fail. I think we should allow the backward case in 4GB mode judgment if we have. What's your opinion? or some other suggestion? Thanks. > + > static const struct mtk_iommu_plat_data mt8167_data = { > .m4u_plat = M4U_MT8167, > .flags = RESET_AXI | HAS_LEGACY_IVRP_PADDR | > MTK_IOMMU_TYPE_MM, > @@ -1515,6 +1532,7 @@ static const struct mtk_iommu_plat_data > mt8195_data_vpp = { > static const struct of_device_id mtk_iommu_of_ids[] = { > { .compatible = "mediatek,mt2712-m4u", .data = &mt2712_data}, > { .compatible = "mediatek,mt6779-m4u", .data = &mt6779_data}, > + { .compatible = "mediatek,mt6795-m4u", .data = &mt6795_data}, > { .compatible = "mediatek,mt8167-m4u", .data = &mt8167_data}, > { .compatible = "mediatek,mt8173-m4u", .data = &mt8173_data}, > { .compatible = "mediatek,mt8183-m4u", .data = &mt8183_data},
Il 17/05/22 11:08, Yong Wu ha scritto: > On Fri, 2022-05-13 at 17:14 +0200, AngeloGioacchino Del Regno wrote: >> Add support for the M4Us found in the MT6795 Helio X10 SoC. >> >> Signed-off-by: AngeloGioacchino Del Regno < >> angelogioacchino.delregno@collabora.com> >> --- >> drivers/iommu/mtk_iommu.c | 20 +++++++++++++++++++- >> 1 file changed, 19 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c >> index 71b2ace74cd6..3d802dd3f377 100644 >> --- a/drivers/iommu/mtk_iommu.c >> +++ b/drivers/iommu/mtk_iommu.c >> @@ -157,6 +157,7 @@ >> enum mtk_iommu_plat { >> M4U_MT2712, >> M4U_MT6779, >> + M4U_MT6795, >> M4U_MT8167, >> M4U_MT8173, >> M4U_MT8183, >> @@ -953,7 +954,8 @@ static int mtk_iommu_hw_init(const struct >> mtk_iommu_data *data, unsigned int ban >> * Global control settings are in bank0. May re-init these >> global registers >> * since no sure if there is bank0 consumers. >> */ >> - if (data->plat_data->m4u_plat == M4U_MT8173) { >> + if (data->plat_data->m4u_plat == M4U_MT6795 || >> + data->plat_data->m4u_plat == M4U_MT8173) { >> regval = F_MMU_PREFETCH_RT_REPLACE_MOD | >> F_MMU_TF_PROT_TO_PROGRAM_ADDR_MT8173; >> } else { >> @@ -1138,6 +1140,9 @@ static int mtk_iommu_probe(struct >> platform_device *pdev) >> case M4U_MT2712: >> p = "mediatek,mt2712-infracfg"; >> break; >> + case M4U_MT6795: >> + p = "mediatek,mt6795-infracfg"; >> + break; >> case M4U_MT8173: >> p = "mediatek,mt8173-infracfg"; >> break; >> @@ -1404,6 +1409,18 @@ static const struct mtk_iommu_plat_data >> mt6779_data = { >> .larbid_remap = {{0}, {1}, {2}, {3}, {5}, {7, 8}, {10}, {9}}, >> }; >> >> +static const struct mtk_iommu_plat_data mt6795_data = { >> + .m4u_plat = M4U_MT6795, >> + .flags = HAS_4GB_MODE | HAS_BCLK | RESET_AXI | >> + HAS_LEGACY_IVRP_PADDR | MTK_IOMMU_TYPE_MM, >> + .inv_sel_reg = REG_MMU_INV_SEL_GEN1, >> + .banks_num = 1, >> + .banks_enable = {true}, >> + .iova_region = single_domain, >> + .iova_region_nr = ARRAY_SIZE(single_domain), >> + .larbid_remap = {{0}, {1}, {2}, {3}, {4}}, /* Linear mapping. >> */ >> +}; > > This is nearly same with mt8173_data. mt8173 has one more larb than > mt6795, its larbid_remap is also ok for mt6795. > I think that we should be explicit about the larbid_remap property, since mt6795 has one less larb, we should explicitly say that like I did there... that's only for human readability I admit ... but, still, I wouldn't want to see people thinking that MT6795 has 6 LARBs because they've read that larbid_remap having 6 entries. > thus it looks we could use mt8173 as the backward compatible. > compatible = "mediatek,mt6795-m4u", > "mediatek,mt8173-m4u"; > > After this, the only thing is about "mediatek,mt6795-infracfg". we have > to try again with mediatek,mt6795-infracfg after mediatek,mt8173- > infracfg fail. I think we should allow the backward case in 4GB mode > judgment if we have. > > What's your opinion? or some other suggestion? > Thanks. I know, I may have a plan for that, but I wanted to have a good reason to propose such a thing, as if it's just about two SoCs needing that, there would be no good reason to get things done differently. ...so, in order to provide a good cleanup, we have two possible roads to follow here: either we add a generic "mediatek,infracfg" compatible to the infra node (but I don't like that), or we can do it like it was previously done in mtk-pm-domains.c (I prefer that approach): iommu: iommu@somewhere { ... something ... mediatek,infracfg = <&infracfg>; }; infracfg = syscon_regmap_lookup_by_compatible(node, "mediatek,infracfg"); if (IS_ERR(infracfg)) { /* try with the older way */ switch (...) { case .... p = "mediatek,mt2712-infracfg"; ... blah blah ... } /* legacy also failed, ouch! */ if (IS_ERR(infracfg)) return PTR_ERR(infracfg); } ret = regmap_read ... etc etc etc Cheers, Angelo
On 17/05/2022 11:26, AngeloGioacchino Del Regno wrote: > Il 17/05/22 11:08, Yong Wu ha scritto: >> On Fri, 2022-05-13 at 17:14 +0200, AngeloGioacchino Del Regno wrote: >>> Add support for the M4Us found in the MT6795 Helio X10 SoC. >>> >>> Signed-off-by: AngeloGioacchino Del Regno < >>> angelogioacchino.delregno@collabora.com> >>> --- >>> drivers/iommu/mtk_iommu.c | 20 +++++++++++++++++++- >>> 1 file changed, 19 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c >>> index 71b2ace74cd6..3d802dd3f377 100644 >>> --- a/drivers/iommu/mtk_iommu.c >>> +++ b/drivers/iommu/mtk_iommu.c >>> @@ -157,6 +157,7 @@ >>> enum mtk_iommu_plat { >>> M4U_MT2712, >>> M4U_MT6779, >>> + M4U_MT6795, >>> M4U_MT8167, >>> M4U_MT8173, >>> M4U_MT8183, >>> @@ -953,7 +954,8 @@ static int mtk_iommu_hw_init(const struct >>> mtk_iommu_data *data, unsigned int ban >>> * Global control settings are in bank0. May re-init these >>> global registers >>> * since no sure if there is bank0 consumers. >>> */ >>> - if (data->plat_data->m4u_plat == M4U_MT8173) { >>> + if (data->plat_data->m4u_plat == M4U_MT6795 || >>> + data->plat_data->m4u_plat == M4U_MT8173) { >>> regval = F_MMU_PREFETCH_RT_REPLACE_MOD | >>> F_MMU_TF_PROT_TO_PROGRAM_ADDR_MT8173; >>> } else { >>> @@ -1138,6 +1140,9 @@ static int mtk_iommu_probe(struct >>> platform_device *pdev) >>> case M4U_MT2712: >>> p = "mediatek,mt2712-infracfg"; >>> break; >>> + case M4U_MT6795: >>> + p = "mediatek,mt6795-infracfg"; >>> + break; >>> case M4U_MT8173: >>> p = "mediatek,mt8173-infracfg"; >>> break; >>> @@ -1404,6 +1409,18 @@ static const struct mtk_iommu_plat_data >>> mt6779_data = { >>> .larbid_remap = {{0}, {1}, {2}, {3}, {5}, {7, 8}, {10}, {9}}, >>> }; >>> +static const struct mtk_iommu_plat_data mt6795_data = { >>> + .m4u_plat = M4U_MT6795, >>> + .flags = HAS_4GB_MODE | HAS_BCLK | RESET_AXI | >>> + HAS_LEGACY_IVRP_PADDR | MTK_IOMMU_TYPE_MM, >>> + .inv_sel_reg = REG_MMU_INV_SEL_GEN1, >>> + .banks_num = 1, >>> + .banks_enable = {true}, >>> + .iova_region = single_domain, >>> + .iova_region_nr = ARRAY_SIZE(single_domain), >>> + .larbid_remap = {{0}, {1}, {2}, {3}, {4}}, /* Linear mapping. >>> */ >>> +}; >> >> This is nearly same with mt8173_data. mt8173 has one more larb than >> mt6795, its larbid_remap is also ok for mt6795. >> > > I think that we should be explicit about the larbid_remap property, > since mt6795 has one less larb, we should explicitly say that like > I did there... that's only for human readability I admit ... but, > still, I wouldn't want to see people thinking that MT6795 has 6 LARBs > because they've read that larbid_remap having 6 entries. > >> thus it looks we could use mt8173 as the backward compatible. >> compatible = "mediatek,mt6795-m4u", >> "mediatek,mt8173-m4u"; >> >> After this, the only thing is about "mediatek,mt6795-infracfg". we have >> to try again with mediatek,mt6795-infracfg after mediatek,mt8173- >> infracfg fail. I think we should allow the backward case in 4GB mode >> judgment if we have. >> >> What's your opinion? or some other suggestion? >> Thanks. > > I know, I may have a plan for that, but I wanted to have a good reason to > propose such a thing, as if it's just about two SoCs needing that, there > would be no good reason to get things done differently. > > ...so, in order to provide a good cleanup, we have two possible roads to > follow here: either we add a generic "mediatek,infracfg" compatible to the > infra node (but I don't like that), or we can do it like it was previously > done in mtk-pm-domains.c (I prefer that approach): > > iommu: iommu@somewhere { > ... something ... > mediatek,infracfg = <&infracfg>; > }; > > infracfg = syscon_regmap_lookup_by_compatible(node, "mediatek,infracfg"); > if (IS_ERR(infracfg)) { > /* try with the older way */ > switch (...) { > case .... p = "mediatek,mt2712-infracfg"; > ... blah blah ... > } > /* legacy also failed, ouch! */ > if (IS_ERR(infracfg)) > return PTR_ERR(infracfg); > } > > ret = regmap_read ... etc etc etc > I prefer that approach as well. Regards, Matthias > Cheers, > Angelo
On Tue, 2022-05-17 at 11:26 +0200, AngeloGioacchino Del Regno wrote: > Il 17/05/22 11:08, Yong Wu ha scritto: > > On Fri, 2022-05-13 at 17:14 +0200, AngeloGioacchino Del Regno > > wrote: > > > Add support for the M4Us found in the MT6795 Helio X10 SoC. > > > > > > Signed-off-by: AngeloGioacchino Del Regno < > > > angelogioacchino.delregno@collabora.com> > > > --- > > > drivers/iommu/mtk_iommu.c | 20 +++++++++++++++++++- > > > 1 file changed, 19 insertions(+), 1 deletion(-) [...] > > > +static const struct mtk_iommu_plat_data mt6795_data = { > > > + .m4u_plat = M4U_MT6795, > > > + .flags = HAS_4GB_MODE | HAS_BCLK | RESET_AXI | > > > + HAS_LEGACY_IVRP_PADDR | MTK_IOMMU_TYPE_MM, > > > + .inv_sel_reg = REG_MMU_INV_SEL_GEN1, > > > + .banks_num = 1, > > > + .banks_enable = {true}, > > > + .iova_region = single_domain, > > > + .iova_region_nr = ARRAY_SIZE(single_domain), > > > + .larbid_remap = {{0}, {1}, {2}, {3}, {4}}, /* Linear mapping. > > > */ > > > +}; > > > > This is nearly same with mt8173_data. mt8173 has one more larb than > > mt6795, its larbid_remap is also ok for mt6795. > > > > I think that we should be explicit about the larbid_remap property, > since mt6795 has one less larb, we should explicitly say that like > I did there... that's only for human readability I admit ... but, > still, I wouldn't want to see people thinking that MT6795 has 6 LARBs > because they've read that larbid_remap having 6 entries. In the linear mapping case, It does help. Strictly larbid_remap is two- dimensional array now, It doesn't indicate how many larbs this SoC has. If someone would like to know this, he could read the mtxxx-larb- port.h. We also could document the larb numbers in the binding for readability. Anyway, It is not a big problem. A new structure is ok for me. I just complain there are too many structures, specially in the internal branch which contains many non-upstream SoCs, and there may be several IOMMU HWs in one SoC. thus I'd like to group it personally. > > > thus it looks we could use mt8173 as the backward compatible. > > compatible = "mediatek,mt6795-m4u", > > "mediatek,mt8173-m4u"; > > > > After this, the only thing is about "mediatek,mt6795-infracfg". we > > have > > to try again with mediatek,mt6795-infracfg after mediatek,mt8173- > > infracfg fail. I think we should allow the backward case in 4GB > > mode > > judgment if we have. > > > > What's your opinion? or some other suggestion? > > Thanks. > > I know, I may have a plan for that, but I wanted to have a good > reason to > propose such a thing, as if it's just about two SoCs needing that, > there > would be no good reason to get things done differently. > > ...so, in order to provide a good cleanup, we have two possible roads > to > follow here: either we add a generic "mediatek,infracfg" compatible > to the > infra node (but I don't like that), or we can do it like it was > previously > done in mtk-pm-domains.c (I prefer that approach): > > iommu: iommu@somewhere { > ... something ... > mediatek,infracfg = <&infracfg>; We like this too. But this was not suggested from Rob long before. https://lore.kernel.org/linux-mediatek/20200715205120.GA778876@bogus/ > }; > > infracfg = syscon_regmap_lookup_by_compatible(node, > "mediatek,infracfg"); > if (IS_ERR(infracfg)) { > /* try with the older way */ > switch (...) { > case .... p = "mediatek,mt2712-infracfg"; > ... blah blah ... > } > /* legacy also failed, ouch! */ > if (IS_ERR(infracfg)) > return PTR_ERR(infracfg); > } > > ret = regmap_read ... etc etc etc > > Cheers, > Angelo
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c index 71b2ace74cd6..3d802dd3f377 100644 --- a/drivers/iommu/mtk_iommu.c +++ b/drivers/iommu/mtk_iommu.c @@ -157,6 +157,7 @@ enum mtk_iommu_plat { M4U_MT2712, M4U_MT6779, + M4U_MT6795, M4U_MT8167, M4U_MT8173, M4U_MT8183, @@ -953,7 +954,8 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data *data, unsigned int ban * Global control settings are in bank0. May re-init these global registers * since no sure if there is bank0 consumers. */ - if (data->plat_data->m4u_plat == M4U_MT8173) { + if (data->plat_data->m4u_plat == M4U_MT6795 || + data->plat_data->m4u_plat == M4U_MT8173) { regval = F_MMU_PREFETCH_RT_REPLACE_MOD | F_MMU_TF_PROT_TO_PROGRAM_ADDR_MT8173; } else { @@ -1138,6 +1140,9 @@ static int mtk_iommu_probe(struct platform_device *pdev) case M4U_MT2712: p = "mediatek,mt2712-infracfg"; break; + case M4U_MT6795: + p = "mediatek,mt6795-infracfg"; + break; case M4U_MT8173: p = "mediatek,mt8173-infracfg"; break; @@ -1404,6 +1409,18 @@ static const struct mtk_iommu_plat_data mt6779_data = { .larbid_remap = {{0}, {1}, {2}, {3}, {5}, {7, 8}, {10}, {9}}, }; +static const struct mtk_iommu_plat_data mt6795_data = { + .m4u_plat = M4U_MT6795, + .flags = HAS_4GB_MODE | HAS_BCLK | RESET_AXI | + HAS_LEGACY_IVRP_PADDR | MTK_IOMMU_TYPE_MM, + .inv_sel_reg = REG_MMU_INV_SEL_GEN1, + .banks_num = 1, + .banks_enable = {true}, + .iova_region = single_domain, + .iova_region_nr = ARRAY_SIZE(single_domain), + .larbid_remap = {{0}, {1}, {2}, {3}, {4}}, /* Linear mapping. */ +}; + static const struct mtk_iommu_plat_data mt8167_data = { .m4u_plat = M4U_MT8167, .flags = RESET_AXI | HAS_LEGACY_IVRP_PADDR | MTK_IOMMU_TYPE_MM, @@ -1515,6 +1532,7 @@ static const struct mtk_iommu_plat_data mt8195_data_vpp = { static const struct of_device_id mtk_iommu_of_ids[] = { { .compatible = "mediatek,mt2712-m4u", .data = &mt2712_data}, { .compatible = "mediatek,mt6779-m4u", .data = &mt6779_data}, + { .compatible = "mediatek,mt6795-m4u", .data = &mt6795_data}, { .compatible = "mediatek,mt8167-m4u", .data = &mt8167_data}, { .compatible = "mediatek,mt8173-m4u", .data = &mt8173_data}, { .compatible = "mediatek,mt8183-m4u", .data = &mt8183_data},
Add support for the M4Us found in the MT6795 Helio X10 SoC. Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> --- drivers/iommu/mtk_iommu.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)