Hi Yong,
On Fri, Mar 6, 2015 at 6:37 PM, yong...@mediatek.com wrote:
From: Yong Wu yong...@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.
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...@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
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 000..4e3fab9
--- /dev/null
+++ b/drivers/soc/mediatek/mt8173-smi.c
@@ -0,0 +1,143 @@
+/*
+ * Copyright (c) 2014-2015 MediaTek Inc.
+ * Author: Yong Wu yong...@mediatek.com
+ *
+ * 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
