On Tue, 2022-01-04 at 16:54 +0100, AngeloGioacchino Del Regno wrote:
> Il 23/09/21 13:58, Yong Wu ha scritto:
> > Currently the code for of_iommu_configure_dev_id is like this:
> >
> > static int of_iommu_configure_dev_id(struct device_node *master_np,
> > struct device *dev,
> > const u32 *id)
> > {
> > struct of_phandle_args iommu_spec = { .args_count = 1 };
> >
> > err = of_map_id(master_np, *id, "iommu-map",
> > "iommu-map-mask", &iommu_spec.np,
> > iommu_spec.args);
> > ...
> > }
> >
> > It supports only one id output. BUT our PCIe HW has two ID(one is
> > for
> > writing, the other is for reading). I'm not sure if we should
> > change
> > of_map_id to support output MAX_PHANDLE_ARGS.
> >
> > Here add the solution in ourselve drivers. If it's pcie case,
> > enable one
> > more bit.
> >
> > Not all infra iommu support PCIe, thus add a PCIe support flag
> > here.
> >
> > Signed-off-by: Yong Wu <yong.wu@xxxxxxxxxxxx>
> > ---
> > drivers/iommu/mtk_iommu.c | 21 ++++++++++++++++++++-
> > 1 file changed, 20 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > index 37d6dfb4feab..3f1fd8036345 100644
> > --- a/drivers/iommu/mtk_iommu.c
> > +++ b/drivers/iommu/mtk_iommu.c
> > @@ -20,6 +20,7 @@
> > #include <linux/of_address.h>
> > #include <linux/of_irq.h>
> > #include <linux/of_platform.h>
> > +#include <linux/pci.h>
> > #include <linux/platform_device.h>
> > #include <linux/pm_runtime.h>
> > #include <linux/regmap.h>
> > @@ -132,6 +133,7 @@
> > #define MTK_IOMMU_TYPE_MM (0x0 << 13)
> > #define MTK_IOMMU_TYPE_INFRA (0x1 << 13)
> > #define MTK_IOMMU_TYPE_MASK (0x3 << 13)
> > +#define IFA_IOMMU_PCIe_SUPPORT BIT(15)
>
> This definition looks like "breaking" the naming convention that's
> used in this
> driver... what about MTK_INFRA_IOMMU_PCIE_SUPPORT?
OK for me. I noticed the "PCIE" should called to "PCIe", thus renamed
this.
>
> >
> > #define MTK_IOMMU_HAS_FLAG(pdata, _x) (!!(((pdata)->flags) &
> > (_x)))
> >
> > @@ -401,8 +403,11 @@ static int mtk_iommu_config(struct
> > mtk_iommu_data *data, struct device *dev,
> > 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);
> > - peri_mmuen = enable ? peri_mmuen_msk : 0;
> > + /* PCIdev 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)
> > @@ -977,6 +982,15 @@ static int mtk_iommu_probe(struct
> > platform_device *pdev)
> > ret = component_master_add_with_match(dev,
> > &mtk_iommu_com_ops, match);
> > if (ret)
> > goto out_bus_set_null;
> > + } else if (MTK_IOMMU_IS_TYPE(data->plat_data,
> > MTK_IOMMU_TYPE_INFRA) &&
> > + MTK_IOMMU_HAS_FLAG(data->plat_data,
> > IFA_IOMMU_PCIe_SUPPORT)) {
> > + #ifdef CONFIG_PCI
>
> Please fix the indentation of this ifdef (do not indent).
Thanks. Will fix this.
>
> > + if (!iommu_present(&pci_bus_type)) {
> > + ret = bus_set_iommu(&pci_bus_type,
> > &mtk_iommu_ops);
> > + if (ret) /* PCIe fail don't affect
> > platform_bus. */
> > + goto out_list_del;
> > + }
> > + #endif
> > }
> > return ret;
> >
> > @@ -1007,6 +1021,11 @@ static int mtk_iommu_remove(struct
> > platform_device *pdev)
> > if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_MM)) {
> > device_link_remove(data->smicomm_dev, &pdev->dev);
> > component_master_del(&pdev->dev, &mtk_iommu_com_ops);
> > + } else if (MTK_IOMMU_IS_TYPE(data->plat_data,
> > MTK_IOMMU_TYPE_INFRA) &&
> > + MTK_IOMMU_HAS_FLAG(data->plat_data,
> > IFA_IOMMU_PCIe_SUPPORT)) {
> > + #ifdef CONFIG_PCI
>
> ditto.
>
> > + bus_set_iommu(&pci_bus_type, NULL);
> > + #endif
> > }
> > pm_runtime_disable(&pdev->dev);
> > devm_free_irq(&pdev->dev, data->irq, data);
> >
>
>
