[RFC PATCH 0/5] MT8173 IOMMU support

[RFC PATCH 0/5] MT8173 IOMMU support

[yong.wu]

  This is based on Robin Murphy's arm64: IOMMU-backed DMA mapping[1]. 
This patch adds support for m4u(Multimedia Memory Management Unit),
Currently it only support the m4u with 2 levels of page table on mt8173.

  Please check the hardware block diagram of Mediatek IOMMU.

              EMI (External Memory Interface)
               |
              m4u (Multimedia Memory Management Unit)
               |
              smi (Smart Multimedia Interface)
               |
        +---------------+-------
        |               |
        |               |
    vdec larb       disp larb      ... SoCs have different local arbiter(larb).
        |               |
        |               |
   +----+----+    +-----+-----+
   |    |    |    |     |     |    ...
   |    |    |    |     |     |    ...
   |    |    |    |     |     |    ...
  MC   PP   VLD  OVL0 RDMA0 WDMA0  ... 
  
  Normally we specify a local arbiter(larb) for each multimedia hardware like
display, video decode, video encode and camera. And there are different ports in
each larb. Take a example, there are some ports like MC, PP, UFO, VLD, AVC_MV,
PRED_RD, PRED_WR in video larb, all the ports are according to the video hardware.

	From the diagram, all the multimedia module connect with m4u via smi.
SMI is responsible to enable/disable iommu and control the clocks of each local
arbiter. If we should enable the iommu of video decode, it should config the 
video's ports. And if the video hardware work wether enable/disable iommu, 
it should enable the clock of its larb's clock. So we add a special driver for smi.

[1] http://lists.linuxfoundation.org/pipermail/iommu/2015-February/012236.html

Yong Wu (5):
  soc: mediatek: Add SMI driver
  iommu/mediatek: Add mt8173 IOMMU driver
  dt-bindings: mediatek: Add smi dts binding
  dt-bindings: iommu: Add binding for mediatek IOMMU
  dts: mt8173: Add iommu/smi nodes for mt8173

 .../devicetree/bindings/iommu/mediatek,iommu.txt   |  41 ++
 .../bindings/soc/mediatek/mediatek,smi.txt         |  17 +
 arch/arm64/boot/dts/mediatek/mt8173.dtsi           |  60 ++
 drivers/iommu/Kconfig                              |  11 +
 drivers/iommu/Makefile                             |   1 +
 drivers/iommu/mtk_iommu.c                          | 754 +++++++++++++++++++++
 drivers/iommu/mtk_iommu.h                          |  73 ++
 drivers/iommu/mtk_iommu_pagetable.c                | 439 ++++++++++++
 drivers/iommu/mtk_iommu_pagetable.h                |  49 ++
 drivers/soc/mediatek/Kconfig                       |   7 +
 drivers/soc/mediatek/Makefile                      |   1 +
 drivers/soc/mediatek/mt8173-smi.c                  | 143 ++++
 include/dt-bindings/iommu/mt8173-iommu-port.h      | 127 ++++
 include/linux/mtk-smi.h                            |  40 ++
 14 files changed, 1763 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
 create mode 100644 Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt
 create mode 100644 drivers/iommu/mtk_iommu.c
 create mode 100644 drivers/iommu/mtk_iommu.h
 create mode 100644 drivers/iommu/mtk_iommu_pagetable.c
 create mode 100644 drivers/iommu/mtk_iommu_pagetable.h
 create mode 100644 drivers/soc/mediatek/mt8173-smi.c
 create mode 100644 include/dt-bindings/iommu/mt8173-iommu-port.h
 create mode 100644 include/linux/mtk-smi.h

-- 
1.8.1.1.dirty

G:IT: [PATCH 5/5] dts: mt8173: Add iommu/smi nodes for mt8173

[PATCH 1/5] soc: mediatek: Add SMI driver

[yong.wu-NuS5LvNUpcJWk0Htik3J/w]

From: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>

    This patch add SMI(Smart Multimedia Interface) driver. This driver 
is responsible to enable/disable iommu and control the clocks of each 
local arbiter.

Signed-off-by: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 drivers/soc/mediatek/Kconfig      |   7 ++
 drivers/soc/mediatek/Makefile     |   1 +
 drivers/soc/mediatek/mt8173-smi.c | 143 ++++++++++++++++++++++++++++++++++++++
 include/linux/mtk-smi.h           |  40 +++++++++++
 4 files changed, 191 insertions(+)
 create mode 100644 drivers/soc/mediatek/mt8173-smi.c
 create mode 100644 include/linux/mtk-smi.h

diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
index 729f93e..27fb26c 100644
--- a/drivers/soc/mediatek/Kconfig
+++ b/drivers/soc/mediatek/Kconfig
@@ -20,3 +20,10 @@ config MT8173_PMIC_WRAP
 	  PMIC wrapper is a proprietary hardware in MT8173 to make
 	  communication protocols to access PMIC device.
 	  This driver implement access protocols for MT8173.
+
+config MTK_SMI
+        bool
+	help
+	  Smi help enable/disable iommu in mt8173 and control the
+	  clock of each local arbiter.
+	  It should be true while MTK_IOMMU enable.
diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
index 9b5709b..cdfe95c 100644
--- a/drivers/soc/mediatek/Makefile
+++ b/drivers/soc/mediatek/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_MT8135_PMIC_WRAP)		+= mt8135-pmic-wrap.o
 obj-$(CONFIG_MT8173_PMIC_WRAP)		+= mt8173-pmic-wrap.o
+obj-$(CONFIG_MTK_SMI)                   += mt8173-smi.o
diff --git a/drivers/soc/mediatek/mt8173-smi.c b/drivers/soc/mediatek/mt8173-smi.c
new file mode 100644
index 0000000..4e3fab9
--- /dev/null
+++ b/drivers/soc/mediatek/mt8173-smi.c
@@ -0,0 +1,143 @@
+/*
+ * Copyright (c) 2014-2015 MediaTek Inc.
+ * Author: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/mm.h>
+
+#define SMI_LARB_MMU_EN                 (0xf00)
+#define F_SMI_MMU_EN(port)              (1 << (port))
+
+struct mtk_smi_larb {
+	void __iomem *larb_base;
+	struct clk *larb_clk[3];/* each larb has 3 clk at most */
+};
+
+static const char * const mtk_smi_clk_name[] = {
+	"larb_sub0", "larb_sub1", "larb_sub2"
+};
+
+static const struct of_device_id mtk_smi_of_ids[] = {
+	{ .compatible = "mediatek,mt8173-smi-larb",
+	},
+	{}
+};
+
+int mtk_smi_larb_get(struct platform_device *plarbdev)
+{
+	struct mtk_smi_larb *larbpriv = dev_get_drvdata(&plarbdev->dev);
+	int i, ret = 0;
+
+	for (i = 0; i < 3; i++)
+		if (larbpriv->larb_clk[i]) {
+			ret = clk_prepare_enable(larbpriv->larb_clk[i]);
+			if (ret) {
+				dev_err(&plarbdev->dev,
+					"failed to enable larbclk%d:%d\n",
+					i, ret);
+				break;
+			}
+		}
+	return ret;
+}
+
+void mtk_smi_larb_put(struct platform_device *plarbdev)
+{
+	struct mtk_smi_larb *larbpriv = dev_get_drvdata(&plarbdev->dev);
+	int i;
+
+	for (i = 2; i >= 0; i--)
+		if (larbpriv->larb_clk[i])
+			clk_disable_unprepare(larbpriv->larb_clk[i]);
+}
+
+int mtk_smi_config_port(struct platform_device *plarbdev,
+			unsigned int larbportid)
+{
+	struct mtk_smi_larb *larbpriv = dev_get_drvdata(&plarbdev->dev);
+	int ret;
+	u32 reg;
+
+	ret = mtk_smi_larb_get(plarbdev);
+	if (ret)
+		return ret;
+
+	reg = readl(larbpriv->larb_base + SMI_LARB_MMU_EN);
+	reg &= ~F_SMI_MMU_EN(larbportid);
+	reg |= F_SMI_MMU_EN(larbportid);
+	writel(reg, larbpriv->larb_base + SMI_LARB_MMU_EN);
+
+	mtk_smi_larb_put(plarbdev);
+
+	return 0;
+}
+
+static int mtk_smi_probe(struct platform_device *pdev)
+{
+	struct mtk_smi_larb *larbpriv;
+	struct resource *res;
+	struct device *dev = &pdev->dev;
+	unsigned int i;
+
+	larbpriv = devm_kzalloc(dev, sizeof(struct mtk_smi_larb), GFP_KERNEL);
+	if (!larbpriv)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	larbpriv->larb_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(larbpriv->larb_base)) {
+		dev_err(dev, "larbbase %p err\n", larbpriv->larb_base);
+		return PTR_ERR(larbpriv->larb_base);
+	}
+
+	for (i = 0; i < 3; i++) {
+		larbpriv->larb_clk[i] = devm_clk_get(dev, mtk_smi_clk_name[i]);
+
+		if (IS_ERR(larbpriv->larb_clk[i])) {
+			if (i == 2) {/* some larb may have only 2 clock */
+				larbpriv->larb_clk[i] = NULL;
+			} else {
+				dev_err(dev, "clock-%d err: %p\n", i,
+					larbpriv->larb_clk[i]);
+				return PTR_ERR(larbpriv->larb_clk[i]);
+			}
+		}
+	}
+	dev_set_drvdata(dev, larbpriv);
+	return 0;
+}
+
+static int mtk_smi_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static struct platform_driver mtk_smi_driver = {
+	.probe	= mtk_smi_probe,
+	.remove	= mtk_smi_remove,
+	.driver	= {
+		.name = "mtksmi",
+		.of_match_table = mtk_smi_of_ids,
+	}
+};
+
+static int __init mtk_smi_init(void)
+{
+	return platform_driver_register(&mtk_smi_driver);
+}
+
+subsys_initcall(mtk_smi_init);
+
diff --git a/include/linux/mtk-smi.h b/include/linux/mtk-smi.h
new file mode 100644
index 0000000..1411f7b
--- /dev/null
+++ b/include/linux/mtk-smi.h
@@ -0,0 +1,40 @@
+/*
+ * Copyright (c) 2014-2015 MediaTek Inc.
+ * Author: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#ifndef MTK_IOMMU_SMI_H
+#define MTK_IOMMU_SMI_H
+#include <linux/platform_device.h>
+
+/*
+ * Enable iommu for each port, it is only for iommu.
+ *
+ * Returns 0 if successfully, others if failed.
+ */
+int mtk_smi_config_port(struct platform_device *pdev,
+			unsigned int larbportid);
+
+/*
+ * The multimedia module should call the two function below
+ * which help open/close the clock of the larb.
+ * so the client dtsi should add the larb like "larb = <&larb0>"
+ * to get platform_device.
+ *
+ * mtk_smi_larb_get should be called before the multimedia h/w work.
+ * mtk_smi_larb_put should be called after h/w done.
+ *
+ * Returns 0 if successfully, others if failed.
+ */
+int mtk_smi_larb_get(struct platform_device *plarbdev);
+void mtk_smi_larb_put(struct platform_device *plarbdev);
+
+#endif
-- 
1.8.1.1.dirty

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] soc: mediatek: Add SMI driver

[Daniel Kurtz]

Hi Yong,

On Fri, Mar 6, 2015 at 6:37 PM,  <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> From: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>
>     This patch add SMI(Smart Multimedia Interface) driver. This driver
> is responsible to enable/disable iommu and control the clocks of each
> local arbiter.

High-level:
Is there more to the smi (or smi-larb) driver, or is it always just a
1:1 wrapper for a particular m4u consumer?
In other words, instead of a separate driver, is it possible to move
this functionality into the m4u driver and/or the m4u consumers
directly?

>
> Signed-off-by: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/soc/mediatek/Kconfig      |   7 ++
>  drivers/soc/mediatek/Makefile     |   1 +
>  drivers/soc/mediatek/mt8173-smi.c | 143 ++++++++++++++++++++++++++++++++++++++
>  include/linux/mtk-smi.h           |  40 +++++++++++
>  4 files changed, 191 insertions(+)
>  create mode 100644 drivers/soc/mediatek/mt8173-smi.c
>  create mode 100644 include/linux/mtk-smi.h
>
> diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
> index 729f93e..27fb26c 100644
> --- a/drivers/soc/mediatek/Kconfig
> +++ b/drivers/soc/mediatek/Kconfig
> @@ -20,3 +20,10 @@ config MT8173_PMIC_WRAP
>           PMIC wrapper is a proprietary hardware in MT8173 to make
>           communication protocols to access PMIC device.
>           This driver implement access protocols for MT8173.
> +
> +config MTK_SMI
> +        bool
> +       help
> +         Smi help enable/disable iommu in mt8173 and control the
> +         clock of each local arbiter.
> +         It should be true while MTK_IOMMU enable.
> diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
> index 9b5709b..cdfe95c 100644
> --- a/drivers/soc/mediatek/Makefile
> +++ b/drivers/soc/mediatek/Makefile
> @@ -1,2 +1,3 @@
>  obj-$(CONFIG_MT8135_PMIC_WRAP)         += mt8135-pmic-wrap.o
>  obj-$(CONFIG_MT8173_PMIC_WRAP)         += mt8173-pmic-wrap.o
> +obj-$(CONFIG_MTK_SMI)                   += mt8173-smi.o
> diff --git a/drivers/soc/mediatek/mt8173-smi.c b/drivers/soc/mediatek/mt8173-smi.c
> new file mode 100644
> index 0000000..4e3fab9
> --- /dev/null
> +++ b/drivers/soc/mediatek/mt8173-smi.c
> @@ -0,0 +1,143 @@
> +/*
> + * Copyright (c) 2014-2015 MediaTek Inc.
> + * Author: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +#include <linux/io.h>
> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/mm.h>
> +
> +#define SMI_LARB_MMU_EN                 (0xf00)
> +#define F_SMI_MMU_EN(port)              (1 << (port))
> +
> +struct mtk_smi_larb {
> +       void __iomem *larb_base;
> +       struct clk *larb_clk[3];/* each larb has 3 clk at most */
> +};
> +
> +static const char * const mtk_smi_clk_name[] = {
> +       "larb_sub0", "larb_sub1", "larb_sub2"
> +};

The order and meaning of these clocks do not seem particularly important.
It seems a bit awkward to use these arbitrary names just so we can use
devm_clk_get() to get a variably sized array of clocks from .dts.
Can we eliminate the "clock-names" property, and just use a single
.dts proprety that lists an array of clocks?
Then you would also get an explicit clock count, and can remove the
NULL checking when iterating.
An example of a clock list without names is:

Clock list in .dts:
  https://lkml.org/lkml/2014/11/17/115
Filling in the clocks from .dts:
  https://lkml.org/lkml/2014/11/17/114

Unfortunately, those patches never made it out of list discussion into
a maintainer tree.

> +
> +static const struct of_device_id mtk_smi_of_ids[] = {
> +       { .compatible = "mediatek,mt8173-smi-larb",
> +       },
> +       {}
> +};

I find it a bit redundant to call the struct "mtk_smi_larb", and then
to prepend "larb_" to all of the fields.
In fact this whole driver is a bit confusing because it isn't clear if
this is an "smi" driver (of which only larb control has been
implemented) or is this an "smi_larb" driver (and potentially there
are other smi drivers).

Perhaps we can just call this an "smi_larb" driver, rename this file
to mt8173-smi-larb.c, and then doing something like:

struct mtk_smi_larb {
  void __iomem *base;
  struct clk *clk[3];    /* each smi_larb has at most 3 clocks */
};

static const struct of_device_id mtk_smi_larb_of_ids[] = {
       { .compatible = "mediatek,mt8173-smi-larb" },
       {}
};


> +
> +int mtk_smi_larb_get(struct platform_device *plarbdev)

Is there any reason to use "struct platform_device" here instead of
just "struct device"?

> +{
> +       struct mtk_smi_larb *larbpriv = dev_get_drvdata(&plarbdev->dev);
> +       int i, ret = 0;
> +
> +       for (i = 0; i < 3; i++)
> +               if (larbpriv->larb_clk[i]) {
> +                       ret = clk_prepare_enable(larbpriv->larb_clk[i]);

I think it is slightly nicer to prepare() all of these clocks in
probe(), and just do enable/disable at runtime.
That would allow these get/put routines to be called in atomic context.

> +                       if (ret) {
> +                               dev_err(&plarbdev->dev,
> +                                       "failed to enable larbclk%d:%d\n",
> +                                       i, ret);

Please disable any clocks on the error path that were previously enabled.

> +                               break;
> +                       }
> +               }
> +       return ret;
> +}
> +
> +void mtk_smi_larb_put(struct platform_device *plarbdev)
> +{
> +       struct mtk_smi_larb *larbpriv = dev_get_drvdata(&plarbdev->dev);
> +       int i;
> +
> +       for (i = 2; i >= 0; i--)
> +               if (larbpriv->larb_clk[i])
> +                       clk_disable_unprepare(larbpriv->larb_clk[i]);
> +}
> +
> +int mtk_smi_config_port(struct platform_device *plarbdev,
> +                       unsigned int larbportid)
> +{
> +       struct mtk_smi_larb *larbpriv = dev_get_drvdata(&plarbdev->dev);
> +       int ret;
> +       u32 reg;
> +
> +       ret = mtk_smi_larb_get(plarbdev);
> +       if (ret)
> +               return ret;
> +
> +       reg = readl(larbpriv->larb_base + SMI_LARB_MMU_EN);
> +       reg &= ~F_SMI_MMU_EN(larbportid);
> +       reg |= F_SMI_MMU_EN(larbportid);
> +       writel(reg, larbpriv->larb_base + SMI_LARB_MMU_EN);
> +
> +       mtk_smi_larb_put(plarbdev);
> +
> +       return 0;
> +}
> +
> +static int mtk_smi_probe(struct platform_device *pdev)
> +{
> +       struct mtk_smi_larb *larbpriv;
> +       struct resource *res;
> +       struct device *dev = &pdev->dev;
> +       unsigned int i;
> +
> +       larbpriv = devm_kzalloc(dev, sizeof(struct mtk_smi_larb), GFP_KERNEL);
> +       if (!larbpriv)
> +               return -ENOMEM;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       larbpriv->larb_base = devm_ioremap_resource(dev, res);
> +       if (IS_ERR(larbpriv->larb_base)) {
> +               dev_err(dev, "larbbase %p err\n", larbpriv->larb_base);
> +               return PTR_ERR(larbpriv->larb_base);
> +       }
> +
> +       for (i = 0; i < 3; i++) {
> +               larbpriv->larb_clk[i] = devm_clk_get(dev, mtk_smi_clk_name[i]);
> +
> +               if (IS_ERR(larbpriv->larb_clk[i])) {
> +                       if (i == 2) {/* some larb may have only 2 clock */
> +                               larbpriv->larb_clk[i] = NULL;

The assumption "any error on the third clock means this larb has only
2 clocks" is incorrect.
Please explicitly handle the case where a larb has less than 3 clocks
and not rely on any error from devm_clk_get.
Hopefully this can be handled more cleanly by using an array property
as described above.

> +                       } else {
> +                               dev_err(dev, "clock-%d err: %p\n", i,
> +                                       larbpriv->larb_clk[i]);
> +                               return PTR_ERR(larbpriv->larb_clk[i]);
> +                       }
> +               }
> +       }
> +       dev_set_drvdata(dev, larbpriv);
> +       return 0;
> +}
> +
> +static int mtk_smi_remove(struct platform_device *pdev)

I think you can just remove the empty remove handler (or unprepare the
clocks here).

Ok, that's it for now!

-Dan

> +{
> +       return 0;
> +}
> +
> +static struct platform_driver mtk_smi_driver = {
> +       .probe  = mtk_smi_probe,
> +       .remove = mtk_smi_remove,
> +       .driver = {
> +               .name = "mtksmi",
> +               .of_match_table = mtk_smi_of_ids,
> +       }
> +};
> +
> +static int __init mtk_smi_init(void)
> +{
> +       return platform_driver_register(&mtk_smi_driver);
> +}
> +
> +subsys_initcall(mtk_smi_init);
> +
> diff --git a/include/linux/mtk-smi.h b/include/linux/mtk-smi.h
> new file mode 100644
> index 0000000..1411f7b
> --- /dev/null
> +++ b/include/linux/mtk-smi.h
> @@ -0,0 +1,40 @@
> +/*
> + * Copyright (c) 2014-2015 MediaTek Inc.
> + * Author: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +#ifndef MTK_IOMMU_SMI_H
> +#define MTK_IOMMU_SMI_H
> +#include <linux/platform_device.h>
> +
> +/*
> + * Enable iommu for each port, it is only for iommu.
> + *
> + * Returns 0 if successfully, others if failed.
> + */
> +int mtk_smi_config_port(struct platform_device *pdev,
> +                       unsigned int larbportid);
> +
> +/*
> + * The multimedia module should call the two function below
> + * which help open/close the clock of the larb.
> + * so the client dtsi should add the larb like "larb = <&larb0>"
> + * to get platform_device.
> + *
> + * mtk_smi_larb_get should be called before the multimedia h/w work.
> + * mtk_smi_larb_put should be called after h/w done.
> + *
> + * Returns 0 if successfully, others if failed.
> + */
> +int mtk_smi_larb_get(struct platform_device *plarbdev);
> +void mtk_smi_larb_put(struct platform_device *plarbdev);
> +
> +#endif
> --
> 1.8.1.1.dirty

[RFC PATCH 0/5] MT8173 IOMMU support

[yong.wu-NuS5LvNUpcJWk0Htik3J/w]

  This is based on Robin Murphy's arm64: IOMMU-backed DMA mapping[1]. 
This patch adds support for m4u(Multimedia Memory Management Unit),
Currently it only support the m4u with 2 levels of page table on mt8173.

  Please check the hardware block diagram of Mediatek IOMMU.

              EMI (External Memory Interface)
               |
              m4u (Multimedia Memory Management Unit)
               |
              smi (Smart Multimedia Interface)
               |
        +---------------+-------
        |               |
        |               |
    vdec larb       disp larb      ... SoCs have different local arbiter(larb).
        |               |
        |               |
   +----+----+    +-----+-----+
   |    |    |    |     |     |    ...
   |    |    |    |     |     |    ...
   |    |    |    |     |     |    ...
  MC   PP   VLD  OVL0 RDMA0 WDMA0  ... 
  
  Normally we specify a local arbiter(larb) for each multimedia hardware like
display, video decode, video encode and camera. And there are different ports in
each larb. Take a example, there are some ports like MC, PP, UFO, VLD, AVC_MV,
PRED_RD, PRED_WR in video larb, all the ports are according to the video hardware.

	From the diagram, all the multimedia module connect with m4u via smi.
SMI is responsible to enable/disable iommu and control the clocks of each local
arbiter. If we should enable the iommu of video decode, it should config the 
video's ports. And if the video hardware work wether enable/disable iommu, 
it should enable the clock of its larb's clock. So we add a special driver for smi.

[1] http://lists.linuxfoundation.org/pipermail/iommu/2015-February/012236.html

Yong Wu (5):
  soc: mediatek: Add SMI driver
  iommu/mediatek: Add mt8173 IOMMU driver
  dt-bindings: mediatek: Add smi dts binding
  dt-bindings: iommu: Add binding for mediatek IOMMU
  dts: mt8173: Add iommu/smi nodes for mt8173

 .../devicetree/bindings/iommu/mediatek,iommu.txt   |  41 ++
 .../bindings/soc/mediatek/mediatek,smi.txt         |  17 +
 arch/arm64/boot/dts/mediatek/mt8173.dtsi           |  60 ++
 drivers/iommu/Kconfig                              |  11 +
 drivers/iommu/Makefile                             |   1 +
 drivers/iommu/mtk_iommu.c                          | 754 +++++++++++++++++++++
 drivers/iommu/mtk_iommu.h                          |  73 ++
 drivers/iommu/mtk_iommu_pagetable.c                | 439 ++++++++++++
 drivers/iommu/mtk_iommu_pagetable.h                |  49 ++
 drivers/soc/mediatek/Kconfig                       |   7 +
 drivers/soc/mediatek/Makefile                      |   1 +
 drivers/soc/mediatek/mt8173-smi.c                  | 143 ++++
 include/dt-bindings/iommu/mt8173-iommu-port.h      | 127 ++++
 include/linux/mtk-smi.h                            |  40 ++
 14 files changed, 1763 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
 create mode 100644 Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt
 create mode 100644 drivers/iommu/mtk_iommu.c
 create mode 100644 drivers/iommu/mtk_iommu.h
 create mode 100644 drivers/iommu/mtk_iommu_pagetable.c
 create mode 100644 drivers/iommu/mtk_iommu_pagetable.h
 create mode 100644 drivers/soc/mediatek/mt8173-smi.c
 create mode 100644 include/dt-bindings/iommu/mt8173-iommu-port.h
 create mode 100644 include/linux/mtk-smi.h

-- 
1.8.1.1.dirty

[PATCH 1/5] soc: mediatek: Add SMI driver

[yong.wu-NuS5LvNUpcJWk0Htik3J/w]

From: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>

    This patch add SMI(Smart Multimedia Interface) driver. This driver is
responsible to enable/disable iommu and control the clocks of each
local arbiter.

Signed-off-by: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 drivers/soc/mediatek/Kconfig      |   7 ++
 drivers/soc/mediatek/Makefile     |   1 +
 drivers/soc/mediatek/mt8173-smi.c | 143 ++++++++++++++++++++++++++++++++++++++
 include/linux/mtk-smi.h           |  40 +++++++++++
 4 files changed, 191 insertions(+)
 create mode 100644 drivers/soc/mediatek/mt8173-smi.c
 create mode 100644 include/linux/mtk-smi.h

diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
index 729f93e..27fb26c 100644
--- a/drivers/soc/mediatek/Kconfig
+++ b/drivers/soc/mediatek/Kconfig
@@ -20,3 +20,10 @@ config MT8173_PMIC_WRAP
 	  PMIC wrapper is a proprietary hardware in MT8173 to make
 	  communication protocols to access PMIC device.
 	  This driver implement access protocols for MT8173.
+
+config MTK_SMI
+        bool
+	help
+	  Smi help enable/disable iommu in mt8173 and control the
+	  clock of each local arbiter.
+	  It should be true while MTK_IOMMU enable.
diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
index 9b5709b..cdfe95c 100644
--- a/drivers/soc/mediatek/Makefile
+++ b/drivers/soc/mediatek/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_MT8135_PMIC_WRAP)		+= mt8135-pmic-wrap.o
 obj-$(CONFIG_MT8173_PMIC_WRAP)		+= mt8173-pmic-wrap.o
+obj-$(CONFIG_MTK_SMI)                   += mt8173-smi.o
diff --git a/drivers/soc/mediatek/mt8173-smi.c b/drivers/soc/mediatek/mt8173-smi.c
new file mode 100644
index 0000000..4e3fab9
--- /dev/null
+++ b/drivers/soc/mediatek/mt8173-smi.c
@@ -0,0 +1,143 @@
+/*
+ * Copyright (c) 2014-2015 MediaTek Inc.
+ * Author: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/mm.h>
+
+#define SMI_LARB_MMU_EN                 (0xf00)
+#define F_SMI_MMU_EN(port)              (1 << (port))
+
+struct mtk_smi_larb {
+	void __iomem *larb_base;
+	struct clk *larb_clk[3];/* each larb has 3 clk at most */
+};
+
+static const char * const mtk_smi_clk_name[] = {
+	"larb_sub0", "larb_sub1", "larb_sub2"
+};
+
+static const struct of_device_id mtk_smi_of_ids[] = {
+	{ .compatible = "mediatek,mt8173-smi-larb",
+	},
+	{}
+};
+
+int mtk_smi_larb_get(struct platform_device *plarbdev)
+{
+	struct mtk_smi_larb *larbpriv = dev_get_drvdata(&plarbdev->dev);
+	int i, ret = 0;
+
+	for (i = 0; i < 3; i++)
+		if (larbpriv->larb_clk[i]) {
+			ret = clk_prepare_enable(larbpriv->larb_clk[i]);
+			if (ret) {
+				dev_err(&plarbdev->dev,
+					"failed to enable larbclk%d:%d\n",
+					i, ret);
+				break;
+			}
+		}
+	return ret;
+}
+
+void mtk_smi_larb_put(struct platform_device *plarbdev)
+{
+	struct mtk_smi_larb *larbpriv = dev_get_drvdata(&plarbdev->dev);
+	int i;
+
+	for (i = 2; i >= 0; i--)
+		if (larbpriv->larb_clk[i])
+			clk_disable_unprepare(larbpriv->larb_clk[i]);
+}
+
+int mtk_smi_config_port(struct platform_device *plarbdev,
+			unsigned int larbportid)
+{
+	struct mtk_smi_larb *larbpriv = dev_get_drvdata(&plarbdev->dev);
+	int ret;
+	u32 reg;
+
+	ret = mtk_smi_larb_get(plarbdev);
+	if (ret)
+		return ret;
+
+	reg = readl(larbpriv->larb_base + SMI_LARB_MMU_EN);
+	reg &= ~F_SMI_MMU_EN(larbportid);
+	reg |= F_SMI_MMU_EN(larbportid);
+	writel(reg, larbpriv->larb_base + SMI_LARB_MMU_EN);
+
+	mtk_smi_larb_put(plarbdev);
+
+	return 0;
+}
+
+static int mtk_smi_probe(struct platform_device *pdev)
+{
+	struct mtk_smi_larb *larbpriv;
+	struct resource *res;
+	struct device *dev = &pdev->dev;
+	unsigned int i;
+
+	larbpriv = devm_kzalloc(dev, sizeof(struct mtk_smi_larb), GFP_KERNEL);
+	if (!larbpriv)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	larbpriv->larb_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(larbpriv->larb_base)) {
+		dev_err(dev, "larbbase %p err\n", larbpriv->larb_base);
+		return PTR_ERR(larbpriv->larb_base);
+	}
+
+	for (i = 0; i < 3; i++) {
+		larbpriv->larb_clk[i] = devm_clk_get(dev, mtk_smi_clk_name[i]);
+
+		if (IS_ERR(larbpriv->larb_clk[i])) {
+			if (i == 2) {/* some larb may have only 2 clock */
+				larbpriv->larb_clk[i] = NULL;
+			} else {
+				dev_err(dev, "clock-%d err: %p\n", i,
+					larbpriv->larb_clk[i]);
+				return PTR_ERR(larbpriv->larb_clk[i]);
+			}
+		}
+	}
+	dev_set_drvdata(dev, larbpriv);
+	return 0;
+}
+
+static int mtk_smi_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static struct platform_driver mtk_smi_driver = {
+	.probe	= mtk_smi_probe,
+	.remove	= mtk_smi_remove,
+	.driver	= {
+		.name = "mtksmi",
+		.of_match_table = mtk_smi_of_ids,
+	}
+};
+
+static int __init mtk_smi_init(void)
+{
+	return platform_driver_register(&mtk_smi_driver);
+}
+
+subsys_initcall(mtk_smi_init);
+
diff --git a/include/linux/mtk-smi.h b/include/linux/mtk-smi.h
new file mode 100644
index 0000000..1411f7b
--- /dev/null
+++ b/include/linux/mtk-smi.h
@@ -0,0 +1,40 @@
+/*
+ * Copyright (c) 2014-2015 MediaTek Inc.
+ * Author: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#ifndef MTK_IOMMU_SMI_H
+#define MTK_IOMMU_SMI_H
+#include <linux/platform_device.h>
+
+/*
+ * Enable iommu for each port, it is only for iommu.
+ *
+ * Returns 0 if successfully, others if failed.
+ */
+int mtk_smi_config_port(struct platform_device *pdev,
+			unsigned int larbportid);
+
+/*
+ * The multimedia module should call the two function below
+ * which help open/close the clock of the larb.
+ * so the client dtsi should add the larb like "larb = <&larb0>"
+ * to get platform_device.
+ *
+ * mtk_smi_larb_get should be called before the multimedia h/w work.
+ * mtk_smi_larb_put should be called after h/w done.
+ *
+ * Returns 0 if successfully, others if failed.
+ */
+int mtk_smi_larb_get(struct platform_device *plarbdev);
+void mtk_smi_larb_put(struct platform_device *plarbdev);
+
+#endif
-- 
1.8.1.1.dirty

Re: [PATCH 1/5] soc: mediatek: Add SMI driver

[Paul Bolle]

On Fri, 2015-03-06 at 18:48 +0800, yong.wu@mediatek.com wrote:
> --- a/drivers/soc/mediatek/Kconfig
> +++ b/drivers/soc/mediatek/Kconfig
> @@ -20,3 +20,10 @@ config MT8173_PMIC_WRAP
>  	  PMIC wrapper is a proprietary hardware in MT8173 to make
>  	  communication protocols to access PMIC device.
>  	  This driver implement access protocols for MT8173.
> +
> +config MTK_SMI
> +        bool

Nit: make this one tab instead of 8 spaces, please.

> +	help
> +	  Smi help enable/disable iommu in mt8173 and control the
> +	  clock of each local arbiter.
> +	  It should be true while MTK_IOMMU enable.

I don't think anyone using the *config tools will ever see this text, as
there's no prompt. So you might as well make this a comment or drop it
altogether.

Is this selected by more than just MTK_IOMMU (see 2/5)? If not, I think
MTK_SMI will be set and unset in lockstep with MTK_IOMMU. In other
words, you could as well use one Kconfig symbol.

Thanks,


Paul Bolle

Re: [PATCH 1/5] soc: mediatek: Add SMI driver

[Yong Wu]

Dear Paul,
      Thanks very much for your suggestion.

On Fri, 2015-03-06 at 12:30 +0100, Paul Bolle wrote:
> On Fri, 2015-03-06 at 18:48 +0800, yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org wrote:
> > --- a/drivers/soc/mediatek/Kconfig
> > +++ b/drivers/soc/mediatek/Kconfig
> > @@ -20,3 +20,10 @@ config MT8173_PMIC_WRAP
> >  	  PMIC wrapper is a proprietary hardware in MT8173 to make
> >  	  communication protocols to access PMIC device.
> >  	  This driver implement access protocols for MT8173.
> > +
> > +config MTK_SMI
> > +        bool
> 
> Nit: make this one tab instead of 8 spaces, please.
> 
> > +	help
> > +	  Smi help enable/disable iommu in mt8173 and control the
> > +	  clock of each local arbiter.
> > +	  It should be true while MTK_IOMMU enable.
> 
> I don't think anyone using the *config tools will ever see this text, as
> there's no prompt. So you might as well make this a comment or drop it
> altogether.
> 
     We could search it in the tool even though we don't see it. In next
version, I will try to make it a comment.
> Is this selected by more than just MTK_IOMMU (see 2/5)? If not, I think
> MTK_SMI will be set and unset in lockstep with MTK_IOMMU. In other
> words, you could as well use one Kconfig symbol.
> 
> Thanks,
    if we disable MTK_IOMMU, the MTK_SMI also should be selected.That is because 
if the multimedia h/w want to work, the clock of the local arbiters always should be opened.
> 
> Paul Bolle
> 

Re: [PATCH 1/5] soc: mediatek: Add SMI driver

[Paul Bolle]

Hi Yong,

Yong Wu schreef op ma 09-03-2015 om 19:57 [+0800]:
> On Fri, 2015-03-06 at 12:30 +0100, Paul Bolle wrote:
> > On Fri, 2015-03-06 at 18:48 +0800, yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org wrote:
> > > --- a/drivers/soc/mediatek/Kconfig
> > > +++ b/drivers/soc/mediatek/Kconfig
> > > @@ -20,3 +20,10 @@ config MT8173_PMIC_WRAP
> > >  	  PMIC wrapper is a proprietary hardware in MT8173 to make
> > >  	  communication protocols to access PMIC device.
> > >  	  This driver implement access protocols for MT8173.
> > > +
> > > +config MTK_SMI
> > > +        bool
> > 
> > Nit: make this one tab instead of 8 spaces, please.
> > 
> > > +	help
> > > +	  Smi help enable/disable iommu in mt8173 and control the
> > > +	  clock of each local arbiter.
> > > +	  It should be true while MTK_IOMMU enable.
> > 
> > I don't think anyone using the *config tools will ever see this text, as
> > there's no prompt. So you might as well make this a comment or drop it
> > altogether.
> > 
>      We could search it in the tool even though we don't see it. In next
> version, I will try to make it a comment.
> > Is this selected by more than just MTK_IOMMU (see 2/5)? If not, I think
> > MTK_SMI will be set and unset in lockstep with MTK_IOMMU. In other
> > words, you could as well use one Kconfig symbol.
> > 
>     if we disable MTK_IOMMU, the MTK_SMI also should be selected.That is because 
> if the multimedia h/w want to work, the clock of the local arbiters always should be opened.

This is a bit confusing, I'm afraid. Do you mean to say that it ought to
be possible for MTK_SMI to be 'y' even if MTK_IOMMU would be 'n'?


Paul Bolle

Re: [PATCH 1/5] soc: mediatek: Add SMI driver

[Arnd Bergmann]

On Monday 09 March 2015 18:59:37 Paul Bolle wrote:
> Hi Yong,
> 
> Yong Wu schreef op ma 09-03-2015 om 19:57 [+0800]:
> > On Fri, 2015-03-06 at 12:30 +0100, Paul Bolle wrote:
> > > On Fri, 2015-03-06 at 18:48 +0800, yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org wrote:
> > > > --- a/drivers/soc/mediatek/Kconfig
> > > > +++ b/drivers/soc/mediatek/Kconfig
> > > > @@ -20,3 +20,10 @@ config MT8173_PMIC_WRAP
> > > >  	  PMIC wrapper is a proprietary hardware in MT8173 to make
> > > >  	  communication protocols to access PMIC device.
> > > >  	  This driver implement access protocols for MT8173.
> > > > +
> > > > +config MTK_SMI
> > > > +        bool
> > > 
> > > Nit: make this one tab instead of 8 spaces, please.
> > > 
> > > > +	help
> > > > +	  Smi help enable/disable iommu in mt8173 and control the
> > > > +	  clock of each local arbiter.
> > > > +	  It should be true while MTK_IOMMU enable.
> > > 
> > > I don't think anyone using the *config tools will ever see this text, as
> > > there's no prompt. So you might as well make this a comment or drop it
> > > altogether.
> > > 
> >      We could search it in the tool even though we don't see it. In next
> > version, I will try to make it a comment.

No, please leave it the way it was. I prefer having help texts even
for silent options over using comments to do the same.

	Arnd

Re: [PATCH 1/5] soc: mediatek: Add SMI driver

[Yingjoe Chen]

On Mon, 2015-03-09 at 18:59 +0100, Paul Bolle wrote:
> Hi Yong,
> 
> Yong Wu schreef op ma 09-03-2015 om 19:57 [+0800]:
> > On Fri, 2015-03-06 at 12:30 +0100, Paul Bolle wrote:
> > > On Fri, 2015-03-06 at 18:48 +0800, yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org wrote:
> > > > --- a/drivers/soc/mediatek/Kconfig
> > > > +++ b/drivers/soc/mediatek/Kconfig
> > > > @@ -20,3 +20,10 @@ config MT8173_PMIC_WRAP
> > > >  	  PMIC wrapper is a proprietary hardware in MT8173 to make
> > > >  	  communication protocols to access PMIC device.
> > > >  	  This driver implement access protocols for MT8173.
> > > > +
> > > > +config MTK_SMI
> > > > +        bool
> > > 
> > > Nit: make this one tab instead of 8 spaces, please.
> > > 
> > > > +	help
> > > > +	  Smi help enable/disable iommu in mt8173 and control the
> > > > +	  clock of each local arbiter.
> > > > +	  It should be true while MTK_IOMMU enable.
> > > 
> > > I don't think anyone using the *config tools will ever see this text, as
> > > there's no prompt. So you might as well make this a comment or drop it
> > > altogether.
> > > 
> >      We could search it in the tool even though we don't see it. In next
> > version, I will try to make it a comment.
> > > Is this selected by more than just MTK_IOMMU (see 2/5)? If not, I think
> > > MTK_SMI will be set and unset in lockstep with MTK_IOMMU. In other
> > > words, you could as well use one Kconfig symbol.
> > > 
> >     if we disable MTK_IOMMU, the MTK_SMI also should be selected.That is because 
> > if the multimedia h/w want to work, the clock of the local arbiters always should be opened.
> 
> This is a bit confusing, I'm afraid. Do you mean to say that it ought to
> be possible for MTK_SMI to be 'y' even if MTK_IOMMU would be 'n'?

The SMI can be configured to bypass IOMMU and send traffic directly to
memory interface. It is possible to not use IOMMU and have display/MM to
use continuous memory only.

Besides MTK_IOMMU, we expect DRM, VDEC driver to select MTK_SMI as well.

Joe.C

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] soc: mediatek: Add SMI driver

[Yingjoe Chen]

On Fri, 2015-03-06 at 18:48 +0800, yong.wu@mediatek.com wrote:
> From: Yong Wu <yong.wu@mediatek.com>
> 
>     This patch add SMI(Smart Multimedia Interface) driver. This driver is
> responsible to enable/disable iommu and control the clocks of each
> local arbiter.
> 
> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> ---
>  drivers/soc/mediatek/Kconfig      |   7 ++
>  drivers/soc/mediatek/Makefile     |   1 +
>  drivers/soc/mediatek/mt8173-smi.c | 143 ++++++++++++++++++++++++++++++++++++++
>  include/linux/mtk-smi.h           |  40 +++++++++++
>  4 files changed, 191 insertions(+)
>  create mode 100644 drivers/soc/mediatek/mt8173-smi.c
>  create mode 100644 include/linux/mtk-smi.h
> 

Hi Arnd, Matthias,

For the SMI driver, we can't find a better place, so we put it in
drivers/soc/mediatek now. Please let us know if you have any suggestion
or concern. Thanks

You can find more description about SMI and how we use it in the cover
letter:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/328451.html

Joe.C

Re: [PATCH 1/5] soc: mediatek: Add SMI driver

[Arnd Bergmann]

On Monday 09 March 2015 11:26:52 Yingjoe Chen wrote:
> On Fri, 2015-03-06 at 18:48 +0800, yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org wrote:
> > From: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > 
> >     This patch add SMI(Smart Multimedia Interface) driver. This driver is
> > responsible to enable/disable iommu and control the clocks of each
> > local arbiter.
> > 
> > Signed-off-by: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > ---
> >  drivers/soc/mediatek/Kconfig      |   7 ++
> >  drivers/soc/mediatek/Makefile     |   1 +
> >  drivers/soc/mediatek/mt8173-smi.c | 143 ++++++++++++++++++++++++++++++++++++++
> >  include/linux/mtk-smi.h           |  40 +++++++++++
> >  4 files changed, 191 insertions(+)
> >  create mode 100644 drivers/soc/mediatek/mt8173-smi.c
> >  create mode 100644 include/linux/mtk-smi.h
> > 
> 
> Hi Arnd, Matthias,
> 
> For the SMI driver, we can't find a better place, so we put it in
> drivers/soc/mediatek now. Please let us know if you have any suggestion
> or concern. Thanks

>From what I understand from your description, I think it would better
fit in drivers/iommu. Another option is drivers/memory, which I think
is where the respective Tegra driver ended up.

	Arnd

Re: [PATCH 1/5] soc: mediatek: Add SMI driver

[Yingjoe Chen]

On Mon, 2015-03-09 at 22:56 +0100, Arnd Bergmann wrote:
> On Monday 09 March 2015 11:26:52 Yingjoe Chen wrote:
> > On Fri, 2015-03-06 at 18:48 +0800, yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org wrote:
> > > From: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > > 
> > >     This patch add SMI(Smart Multimedia Interface) driver. This driver is
> > > responsible to enable/disable iommu and control the clocks of each
> > > local arbiter.
> > > 
> > > Signed-off-by: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > > ---
> > >  drivers/soc/mediatek/Kconfig      |   7 ++
> > >  drivers/soc/mediatek/Makefile     |   1 +
> > >  drivers/soc/mediatek/mt8173-smi.c | 143 ++++++++++++++++++++++++++++++++++++++
> > >  include/linux/mtk-smi.h           |  40 +++++++++++
> > >  4 files changed, 191 insertions(+)
> > >  create mode 100644 drivers/soc/mediatek/mt8173-smi.c
> > >  create mode 100644 include/linux/mtk-smi.h
> > > 
> > 
> > Hi Arnd, Matthias,
> > 
> > For the SMI driver, we can't find a better place, so we put it in
> > drivers/soc/mediatek now. Please let us know if you have any suggestion
> > or concern. Thanks
> 
> From what I understand from your description, I think it would better
> fit in drivers/iommu. Another option is drivers/memory, which I think
> is where the respective Tegra driver ended up.

Hi Arnd,

The description above only describe what is implemented now. Besides
that, SMI HW also does:

- Prioritize/arbitrate memory requests between different ports & larbs.
- Bandwidth limiter
- Performance monitor

I think drivers/memory make more sense. I can't find info about
drivers/memory in MAINTAINERS. Can this go through ARM SoC tree if we
put SMI driver there?

Joe.C


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] soc: mediatek: Add SMI driver

[Arnd Bergmann]

On Tuesday 10 March 2015 14:27:04 Yingjoe Chen wrote:
> On Mon, 2015-03-09 at 22:56 +0100, Arnd Bergmann wrote:
> > On Monday 09 March 2015 11:26:52 Yingjoe Chen wrote:
> > > On Fri, 2015-03-06 at 18:48 +0800, yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org wrote:
> > > > From: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > > > 
> > > >     This patch add SMI(Smart Multimedia Interface) driver. This driver is
> > > > responsible to enable/disable iommu and control the clocks of each
> > > > local arbiter.
> > > > 
> > > > Signed-off-by: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > > > ---
> > > >  drivers/soc/mediatek/Kconfig      |   7 ++
> > > >  drivers/soc/mediatek/Makefile     |   1 +
> > > >  drivers/soc/mediatek/mt8173-smi.c | 143 ++++++++++++++++++++++++++++++++++++++
> > > >  include/linux/mtk-smi.h           |  40 +++++++++++
> > > >  4 files changed, 191 insertions(+)
> > > >  create mode 100644 drivers/soc/mediatek/mt8173-smi.c
> > > >  create mode 100644 include/linux/mtk-smi.h
> > > > 
> > > 
> > > Hi Arnd, Matthias,
> > > 
> > > For the SMI driver, we can't find a better place, so we put it in
> > > drivers/soc/mediatek now. Please let us know if you have any suggestion
> > > or concern. Thanks
> > 
> > From what I understand from your description, I think it would better
> > fit in drivers/iommu. Another option is drivers/memory, which I think
> > is where the respective Tegra driver ended up.
> 
> Hi Arnd,
> 
> The description above only describe what is implemented now. Besides
> that, SMI HW also does:
> 
> - Prioritize/arbitrate memory requests between different ports & larbs.
> - Bandwidth limiter
> - Performance monitor
> 
> I think drivers/memory make more sense. I can't find info about
> drivers/memory in MAINTAINERS. Can this go through ARM SoC tree if we
> put SMI driver there?
> 

Yes, that works for me. I'd like to see a review from Thierry Reding
though, since he wrote the Tegra driver that is used for very similar
hardware, and I want to make sure you two are on the same page with
the internal APIs.

	Arnd

Re: [PATCH 1/5] soc: mediatek: Add SMI driver

[Lucas Stach]

Hi Arnd,

Am Montag, den 09.03.2015, 22:56 +0100 schrieb Arnd Bergmann:
> On Monday 09 March 2015 11:26:52 Yingjoe Chen wrote:
> > On Fri, 2015-03-06 at 18:48 +0800, yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org wrote:
> > > From: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > > 
> > >     This patch add SMI(Smart Multimedia Interface) driver. This driver is
> > > responsible to enable/disable iommu and control the clocks of each
> > > local arbiter.
> > > 
> > > Signed-off-by: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > > ---
> > >  drivers/soc/mediatek/Kconfig      |   7 ++
> > >  drivers/soc/mediatek/Makefile     |   1 +
> > >  drivers/soc/mediatek/mt8173-smi.c | 143 ++++++++++++++++++++++++++++++++++++++
> > >  include/linux/mtk-smi.h           |  40 +++++++++++
> > >  4 files changed, 191 insertions(+)
> > >  create mode 100644 drivers/soc/mediatek/mt8173-smi.c
> > >  create mode 100644 include/linux/mtk-smi.h
> > > 
> > 
> > Hi Arnd, Matthias,
> > 
> > For the SMI driver, we can't find a better place, so we put it in
> > drivers/soc/mediatek now. Please let us know if you have any suggestion
> > or concern. Thanks
> 
> From what I understand from your description, I think it would better
> fit in drivers/iommu. Another option is drivers/memory, which I think
> is where the respective Tegra driver ended up.
> 
Note that this is not the IOMMU driver. M4U is the IOMMU unit. The SMI
unit is more of a bridge control for the internal buses where you can
control various properties for the masters on the bus.

One notable property (and why this driver is part of the IOMMU series)
is the control weather a master should go through the M4U unit for
translation or bypass it and talk directly to external memory
controller.

Regards,
Lucas

-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] soc: mediatek: Add SMI driver

[Sascha Hauer]

On Fri, Mar 06, 2015 at 06:48:16PM +0800, yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org wrote:
> From: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> 
>     This patch add SMI(Smart Multimedia Interface) driver. This driver is
> responsible to enable/disable iommu and control the clocks of each
> local arbiter.
> 
> Signed-off-by: Yong Wu <yong.wu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/soc/mediatek/Kconfig      |   7 ++
>  drivers/soc/mediatek/Makefile     |   1 +
>  drivers/soc/mediatek/mt8173-smi.c | 143 ++++++++++++++++++++++++++++++++++++++
>  include/linux/mtk-smi.h           |  40 +++++++++++

I just posted the power domain driver for prividing the domains this
driver uses. You should base your code on that.

> +#include <linux/mm.h>
> +
> +#define SMI_LARB_MMU_EN                 (0xf00)
> +#define F_SMI_MMU_EN(port)              (1 << (port))
> +
> +struct mtk_smi_larb {
> +	void __iomem *larb_base;
> +	struct clk *larb_clk[3];/* each larb has 3 clk at most */
> +};
> +
> +static const char * const mtk_smi_clk_name[] = {
> +	"larb_sub0", "larb_sub1", "larb_sub2"
> +};
> +
> +static const struct of_device_id mtk_smi_of_ids[] = {
> +	{ .compatible = "mediatek,mt8173-smi-larb",
> +	},

Add a newline after the opening brace.

> +int mtk_smi_config_port(struct platform_device *plarbdev,
> +			unsigned int larbportid)
> +{
> +	struct mtk_smi_larb *larbpriv = dev_get_drvdata(&plarbdev->dev);
> +	int ret;
> +	u32 reg;
> +
> +	ret = mtk_smi_larb_get(plarbdev);
> +	if (ret)
> +		return ret;
> +
> +	reg = readl(larbpriv->larb_base + SMI_LARB_MMU_EN);
> +	reg &= ~F_SMI_MMU_EN(larbportid);
> +	reg |= F_SMI_MMU_EN(larbportid);

This sets the very same bits that are cleared one line above. This is
surely not what you want.

> +	writel(reg, larbpriv->larb_base + SMI_LARB_MMU_EN);
> +
> +	mtk_smi_larb_put(plarbdev);
> +
> +	return 0;
> +}
> +
> +static int mtk_smi_probe(struct platform_device *pdev)
> +{
> +	struct mtk_smi_larb *larbpriv;
> +	struct resource *res;
> +	struct device *dev = &pdev->dev;
> +	unsigned int i;
> +
> +	larbpriv = devm_kzalloc(dev, sizeof(struct mtk_smi_larb), GFP_KERNEL);

sizeof(*larbpriv)

> +	if (!larbpriv)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	larbpriv->larb_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(larbpriv->larb_base)) {
> +		dev_err(dev, "larbbase %p err\n", larbpriv->larb_base);

You can print an error number with %ld and PTR_ERR(larbpriv->larb_base).
Printing the pointer is not that helpful.

> +/*
> + * Enable iommu for each port, it is only for iommu.
> + *
> + * Returns 0 if successfully, others if failed.
> + */
> +int mtk_smi_config_port(struct platform_device *pdev,
> +			unsigned int larbportid);
> +
> +/*
> + * The multimedia module should call the two function below
> + * which help open/close the clock of the larb.
> + * so the client dtsi should add the larb like "larb = <&larb0>"
> + * to get platform_device.
> + *
> + * mtk_smi_larb_get should be called before the multimedia h/w work.
> + * mtk_smi_larb_put should be called after h/w done.
> + *
> + * Returns 0 if successfully, others if failed.
> + */
> +int mtk_smi_larb_get(struct platform_device *plarbdev);
> +void mtk_smi_larb_put(struct platform_device *plarbdev);

The function documentation usually comes with the definition of a
function, not its declaration.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html