| Message ID | 1425638270-24903-2-git-send-email-yong.wu@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Yong, On Fri, Mar 6, 2015 at 6:37 PM, <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. 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@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 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@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 < 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@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. > + */ > +#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
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@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" +}; + +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@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. + */ +#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