diff mbox series

[v3,22/33] iommu/mediatek: Add PCIe support

Message ID 20210923115840.17813-23-yong.wu@mediatek.com (mailing list archive)
State New, archived
Headers show
Series MT8195 IOMMU SUPPORT | expand

Commit Message

Yong Wu (吴勇) Sept. 23, 2021, 11:58 a.m. UTC
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@mediatek.com>
---
 drivers/iommu/mtk_iommu.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

Comments

AngeloGioacchino Del Regno Jan. 4, 2022, 3:54 p.m. UTC | #1
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@mediatek.com>
> ---
>   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?

>   
>   #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).

> +		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);
>
Yong Wu (吴勇) Jan. 9, 2022, 2:47 a.m. UTC | #2
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@mediatek.com>
> > ---
> >   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);
> > 
> 
>
diff mbox series

Patch

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)
 
 #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
+		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
+		bus_set_iommu(&pci_bus_type, NULL);
+		#endif
 	}
 	pm_runtime_disable(&pdev->dev);
 	devm_free_irq(&pdev->dev, data->irq, data);