diff mbox

[v9] mfd: Add anatop mfd driver

Message ID 1331797421-23731-1-git-send-email-paul.liu@linaro.org
State Accepted
Commit 75060a1d9dc016fb25524e65afba7ec86778084f
Headers show

Commit Message

Paul Liu March 15, 2012, 7:43 a.m. UTC
From: "Ying-Chun Liu (PaulLiu)" <paul.liu@linaro.org>

Anatop is a mfd chip embedded in Freescale i.MX6Q SoC.
Anatop provides regulators and thermal.
This driver handles the address space and the operation of the mfd device.

Signed-off-by: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org>
Acked-by: Shawn Guo <shawn.guo@linaro.org>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: Venu Byravarasu <vbyravarasu@nvidia.com>
Cc: Peter Korsgaard <jacmet@sunsite.dk>
---
 drivers/mfd/Kconfig        |    8 +++
 drivers/mfd/Makefile       |    1 +
 drivers/mfd/anatop-mfd.c   |  142 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/anatop.h |   40 ++++++++++++
 4 files changed, 191 insertions(+), 0 deletions(-)
 create mode 100644 drivers/mfd/anatop-mfd.c
 create mode 100644 include/linux/mfd/anatop.h

Comments

Arnd Bergmann March 15, 2012, 9:07 a.m. UTC | #1
On Thursday 15 March 2012, Ying-Chun Liu (PaulLiu) wrote:
> From: "Ying-Chun Liu (PaulLiu)" <paul.liu@linaro.org>
> 
> Anatop is a mfd chip embedded in Freescale i.MX6Q SoC.
> Anatop provides regulators and thermal.
> This driver handles the address space and the operation of the mfd device.

Hi Paul,

This looks like a very nice and clean driver, good work!

Very broadly speaking, I wonder whether we could use the regmap
infrastructure for these things in the future, but I would first
need to understand whether that is actually in the scope of regmap.

It seems that you just need a subset of what regmap provides,
so it could work, but it might not actually be better than what
you have now.

Mark, can you comment on that?

> +u32 anatop_get_bits(struct anatop *adata, u32 addr, int bit_shift,
> +		    int bit_width)
> +{
> +	u32 val, mask;
> +
> +	if (bit_width == 32)
> +		mask = ~0;
> +	else
> +		mask = (1 << bit_width) - 1;
> +
> +	val = readl(adata->ioreg + addr);
> +	val = (val >> bit_shift) & mask;
> +
> +	return val;
> +}
> +EXPORT_SYMBOL(anatop_get_bits);

I think the exports here should be EXPORT_SYMBOL_GPL. There is no reason
why an out of tree driver would ever use these.

> +static const struct of_device_id of_anatop_subdevice_match[] = {
> +	{ .compatible = "fsl,anatop-regulator", },
> +	{ .compatible = "fsl,anatop-thermal", },
> +	{ },
> +};
> +
> +static int __devinit of_anatop_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *np = dev->of_node;
> +	void *ioreg;
> +	struct anatop *drvdata;
> +
> +	ioreg = of_iomap(np, 0);
> +	if (!ioreg)
> +		return -EADDRNOTAVAIL;
> +	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> +	if (!drvdata)
> +		return -ENOMEM;
> +	drvdata->ioreg = ioreg;
> +	spin_lock_init(&drvdata->reglock);
> +	platform_set_drvdata(pdev, drvdata);
> +	of_platform_bus_probe(np, of_anatop_subdevice_match, dev);
> +
> +	return 0;
> +}

Why do you list the subdevices in of_anatop_subdevice_match()? I think you
should just use 

	of_platform_bus_probe(np, of_anatop_match, dev);

here, using the same match table that you have in the platform_driver.
That will automatically create platform devices for any children of this
device, so you don't have to update the list above when you get new
child drivers.

	Arnd
Mark Brown March 15, 2012, 12:09 p.m. UTC | #2
On Thu, Mar 15, 2012 at 09:07:29AM +0000, Arnd Bergmann wrote:

> Very broadly speaking, I wonder whether we could use the regmap
> infrastructure for these things in the future, but I would first
> need to understand whether that is actually in the scope of regmap.

> It seems that you just need a subset of what regmap provides,
> so it could work, but it might not actually be better than what
> you have now.

> Mark, can you comment on that?

There were some other mutterings about using regmap for memory mapped
devices, mostly from the point of view of building framework features
like this on top of it.  regmap currently makes some assumptions that
the I/O is going to be slow so approximately any amount of CPU time can
usefully be spent on avoiding I/O but we can probably do something about
that.  It also uses mutexes to lock I/O which might not be the most
sensible thing for memory mapped devices, but again that's solveable.
Right now there's no memory mapping support but there's no reason that
can't be added.

In short, it does seem sensible to want to have some support for this
for devices that use appropriate idioms.
Arnd Bergmann March 15, 2012, 3:52 p.m. UTC | #3
On Thursday 15 March 2012, Mark Brown wrote:
> There were some other mutterings about using regmap for memory mapped
> devices, mostly from the point of view of building framework features
> like this on top of it.  regmap currently makes some assumptions that
> the I/O is going to be slow so approximately any amount of CPU time can
> usefully be spent on avoiding I/O but we can probably do something about
> that.  It also uses mutexes to lock I/O which might not be the most
> sensible thing for memory mapped devices, but again that's solveable.
> Right now there's no memory mapping support but there's no reason that
> can't be added.
> 
> In short, it does seem sensible to want to have some support for this
> for devices that use appropriate idioms.

Ok, I see. 

I guess there is no reason to change anything in Paul's patch now, but
we can keep this in mind if we see a lot of similar drivers in the future.

Thanks,

	Arnd
diff mbox

Patch

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 82da448..c3a9f31 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -846,6 +846,14 @@  config MFD_INTEL_MSIC
 	  Passage) chip. This chip embeds audio, battery, GPIO, etc.
 	  devices used in Intel Medfield platforms.
 
+config MFD_ANATOP
+	bool "Support for Freescale i.MX on-chip ANATOP controller"
+	depends on SOC_IMX6Q
+	help
+	  Select this option to enable Freescale i.MX on-chip ANATOP
+	  MFD controller. This controller embeds regulator and
+	  thermal devices for Freescale i.MX platforms.
+
 endmenu
 endif
 
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 27430d3..42c8bf6 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -113,3 +113,4 @@  obj-$(CONFIG_TPS65911_COMPARATOR)	+= tps65911-comparator.o
 obj-$(CONFIG_MFD_AAT2870_CORE)	+= aat2870-core.o
 obj-$(CONFIG_MFD_INTEL_MSIC)	+= intel_msic.o
 obj-$(CONFIG_MFD_S5M_CORE)	+= s5m-core.o s5m-irq.o
+obj-$(CONFIG_MFD_ANATOP)	+= anatop-mfd.o
diff --git a/drivers/mfd/anatop-mfd.c b/drivers/mfd/anatop-mfd.c
new file mode 100644
index 0000000..4272e60
--- /dev/null
+++ b/drivers/mfd/anatop-mfd.c
@@ -0,0 +1,142 @@ 
+/*
+ * Anatop MFD driver
+ *
+ * Copyright (C) 2012 Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org>
+ * Copyright (C) 2012 Linaro
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *   (at your option) any later version.
+ *
+ *  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.
+ *
+ *  You should have received a copy of the GNU General Public License along
+ *  with this program; if not, write to the Free Software Foundation, Inc.,
+ *  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  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.
+ *
+ *  You should have received a copy of the GNU General Public License along
+ *  with this program; if not, write to the Free Software Foundation, Inc.,
+ *  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ */
+
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/of_address.h>
+#include <linux/mfd/anatop.h>
+
+u32 anatop_get_bits(struct anatop *adata, u32 addr, int bit_shift,
+		    int bit_width)
+{
+	u32 val, mask;
+
+	if (bit_width == 32)
+		mask = ~0;
+	else
+		mask = (1 << bit_width) - 1;
+
+	val = readl(adata->ioreg + addr);
+	val = (val >> bit_shift) & mask;
+
+	return val;
+}
+EXPORT_SYMBOL(anatop_get_bits);
+
+void anatop_set_bits(struct anatop *adata, u32 addr, int bit_shift,
+		     int bit_width, u32 data)
+{
+	u32 val, mask;
+
+	if (bit_width == 32)
+		mask = ~0;
+	else
+		mask = (1 << bit_width) - 1;
+
+	spin_lock(&adata->reglock);
+	val = readl(adata->ioreg + addr) & ~(mask << bit_shift);
+	writel((data << bit_shift) | val, adata->ioreg + addr);
+	spin_unlock(&adata->reglock);
+}
+EXPORT_SYMBOL(anatop_set_bits);
+
+static const struct of_device_id of_anatop_subdevice_match[] = {
+	{ .compatible = "fsl,anatop-regulator", },
+	{ .compatible = "fsl,anatop-thermal", },
+	{ },
+};
+
+static int __devinit of_anatop_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	void *ioreg;
+	struct anatop *drvdata;
+
+	ioreg = of_iomap(np, 0);
+	if (!ioreg)
+		return -EADDRNOTAVAIL;
+	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
+	if (!drvdata)
+		return -ENOMEM;
+	drvdata->ioreg = ioreg;
+	spin_lock_init(&drvdata->reglock);
+	platform_set_drvdata(pdev, drvdata);
+	of_platform_bus_probe(np, of_anatop_subdevice_match, dev);
+
+	return 0;
+}
+
+static int __devexit of_anatop_remove(struct platform_device *pdev)
+{
+	struct anatop *drvdata;
+	drvdata = platform_get_drvdata(pdev);
+	iounmap(drvdata->ioreg);
+	return 0;
+}
+
+static const struct of_device_id of_anatop_match[] = {
+	{ .compatible = "fsl,imx6q-anatop", },
+	{ },
+};
+
+static struct platform_driver anatop_of_driver = {
+	.driver = {
+		.name = "anatop-mfd",
+		.owner = THIS_MODULE,
+		.of_match_table = of_anatop_match,
+	},
+	.probe		= of_anatop_probe,
+	.remove		= of_anatop_remove,
+};
+
+static int __init anatop_init(void)
+{
+	return platform_driver_register(&anatop_of_driver);
+}
+postcore_initcall(anatop_init);
+
+static void __exit anatop_exit(void)
+{
+	platform_driver_unregister(&anatop_of_driver);
+}
+module_exit(anatop_exit);
+
+MODULE_AUTHOR("Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org>");
+MODULE_DESCRIPTION("ANATOP MFD driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/anatop.h b/include/linux/mfd/anatop.h
new file mode 100644
index 0000000..22c1007
--- /dev/null
+++ b/include/linux/mfd/anatop.h
@@ -0,0 +1,40 @@ 
+/*
+ * anatop.h - Anatop MFD driver
+ *
+ *  Copyright (C) 2012 Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org>
+ *  Copyright (C) 2012 Linaro
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#ifndef __LINUX_MFD_ANATOP_H
+#define __LINUX_MFD_ANATOP_H
+
+#include <linux/spinlock.h>
+
+/**
+ * anatop - MFD data
+ * @ioreg: ioremap register
+ * @reglock: spinlock for register read/write
+ */
+struct anatop {
+	void *ioreg;
+	spinlock_t reglock;
+};
+
+extern u32 anatop_get_bits(struct anatop *, u32, int, int);
+extern void anatop_set_bits(struct anatop *, u32, int, int, u32);
+
+#endif /*  __LINUX_MFD_ANATOP_H */