On Tue, 2022-10-18 at 14:42 +0800, Chengci.Xu wrote: > For reduce indention without functional change, prepare for MT8188. > If there are many port in a same larb, current flow will update > larb_mmu->mmu or update INFRA register for too many times. > So we save all port to portid_msk in the front of mtk_iommu_config(), > and then update only once for IOMMU configure. By this modification, > we can prevent MT8188 from sending to many SMC calls, avoiding enter > ATF for each port. > > Signed-off-by: Chengci.Xu <chengci.xu@xxxxxxxxxxxx> > --- > drivers/iommu/mtk_iommu.c | 60 ++++++++++++++++++++++--------------- > -- > 1 file changed, 34 insertions(+), 26 deletions(-) > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > index 5a4e00e4bbbc..fbaf401f34e0 100644 > --- a/drivers/iommu/mtk_iommu.c > +++ b/drivers/iommu/mtk_iommu.c > @@ -534,41 +534,49 @@ static int mtk_iommu_config(struct > mtk_iommu_data *data, struct device *dev, > unsigned int larbid, portid; > struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); > const struct mtk_iommu_iova_region *region; > - u32 peri_mmuen, peri_mmuen_msk; > + unsigned long portid_msk_ext; > + u32 portid_msk = 0; > int i, ret = 0; > > for (i = 0; i < fwspec->num_ids; ++i) { > - larbid = MTK_M4U_TO_LARB(fwspec->ids[i]); > portid = MTK_M4U_TO_PORT(fwspec->ids[i]); > + portid_msk |= BIT(portid); > + } > > - if (MTK_IOMMU_IS_TYPE(data->plat_data, > MTK_IOMMU_TYPE_MM)) { > - larb_mmu = &data->larb_imu[larbid]; > + if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_MM)) { > + /* All ports should be in the same larb. just use 0 > here */ > + larbid = MTK_M4U_TO_LARB(fwspec->ids[0]); > + larb_mmu = &data->larb_imu[larbid]; > + region = data->plat_data->iova_region + regionid; > > - region = data->plat_data->iova_region + > regionid; > + portid_msk_ext = portid_msk; > + for_each_set_bit(portid, &portid_msk_ext, 32) Why do we need define a new portid_msk_ext? Can't we use portid_msk directly? > larb_mmu->bank[portid] = upper_32_bits(region- > >iova_base); > > - dev_dbg(dev, "%s iommu for larb(%s) port %d > region %d rgn-bank %d.\n", > - enable ? "enable" : "disable", > dev_name(larb_mmu->dev), > - portid, regionid, larb_mmu- > >bank[portid]); > - > - if (enable) > - larb_mmu->mmu |= > MTK_SMI_MMU_EN(portid); > - else > - larb_mmu->mmu &= > ~MTK_SMI_MMU_EN(portid); > - } else if (MTK_IOMMU_IS_TYPE(data->plat_data, > MTK_IOMMU_TYPE_INFRA)) { > - peri_mmuen_msk = BIT(portid); > - /* PCI dev has only one output id, enable the > next writing bit for PCIe */ > - if (dev_is_pci(dev)) > - peri_mmuen_msk |= BIT(portid + 1); > - > - peri_mmuen = enable ? peri_mmuen_msk : 0; > - ret = regmap_update_bits(data->pericfg, > PERICFG_IOMMU_1, > - peri_mmuen_msk, > peri_mmuen); > - if (ret) > - dev_err(dev, "%s iommu(%s) inframaster > 0x%x fail(%d).\n", > - enable ? "enable" : "disable", > - dev_name(data->dev), > peri_mmuen_msk, ret); > + dev_dbg(dev, "%s iommu for larb(%s) port 0x%x region %d > rgn-bank %d.\n", > + enable ? "enable" : "disable", > dev_name(larb_mmu->dev), > + portid_msk, regionid, upper_32_bits(region- > >iova_base)); > + > + if (enable) > + larb_mmu->mmu |= portid_msk; > + else > + larb_mmu->mmu &= ~portid_msk; > + } else if (MTK_IOMMU_IS_TYPE(data->plat_data, > MTK_IOMMU_TYPE_INFRA)) { > + /* PCI dev has only one output id, enable the next > writing bit for PCIe */ > + if (dev_is_pci(dev)) { > + if (fwspec->num_ids != 1) { > + dev_err(dev, "PCI dev can only have one > port.\n"); > + return -ENODEV; > + } > + portid_msk |= BIT(portid + 1); > } > + > + ret = regmap_update_bits(data->pericfg, > PERICFG_IOMMU_1, > + portid_msk, enable ? > portid_msk : 0); > + if (ret) > + dev_err(dev, "%s iommu(%s) inframaster 0x%x > fail(%d).\n", > + enable ? "enable" : "disable", > + dev_name(data->dev), portid_msk, ret); > } > return ret; > }