linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] add imx-syscon driver for general registers access
@ 2012-08-22  7:18 Dong Aisheng
  2012-08-22  7:18 ` [PATCH 1/7] mfd: add imx syscon driver based on regmap Dong Aisheng
                   ` (6 more replies)
  0 siblings, 7 replies; 29+ messages in thread
From: Dong Aisheng @ 2012-08-22  7:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-kernel, linus.walleij, s.hauer, shawn.guo, kernel,
	grant.likely, rob.herring, sameo, lrg, broonie, richard.zhao,
	devicetree-discuss, swarren, paul.liu

This patch series mainly adds an imx-syscon driver which is used to access
general system controller registers like IOMUXC GPR and ANATOP,
after that, we convert all the exist private access general registers code to use
standard API from imx-syscon to access registers.
Finally we remove the old mfd anatop driver which is only for anatop register
access.

The patch series is based on linus's tree since commit 9160338.

Dong Aisheng (7):
  mfd: add imx syscon driver based on regmap
  ARM: imx6q: add iomuxc gpr support into imx-syscon
  ARM: imx6q: add anatop support into imx-syscon
  regulator: anatop-regulator: convert to use imx-syscon to access
    anatop register
  ARM: imx6q: convert to use imx-syscon to access anatop registers
  ARM: dts: imx6q: add simple-bus compatible string for anatop
  mfd: anatop-mfd: remove anatop driver

 .../devicetree/bindings/mfd/imx-syscon.txt         |   11 +
 arch/arm/boot/dts/imx6q.dtsi                       |   15 +-
 arch/arm/mach-imx/Kconfig                          |    2 +-
 arch/arm/mach-imx/mach-imx6q.c                     |   25 +--
 drivers/mfd/Kconfig                                |   14 +-
 drivers/mfd/Makefile                               |    2 +-
 drivers/mfd/anatop-mfd.c                           |  124 --------
 drivers/mfd/imx-syscon.c                           |  193 ++++++++++++
 drivers/regulator/Kconfig                          |    2 +-
 drivers/regulator/anatop-regulator.c               |   17 +-
 include/linux/fsl/imx6q-iomuxc-gpr.h               |  319 ++++++++++++++++++++
 include/linux/mfd/anatop.h                         |   40 ---
 include/linux/mfd/imx-syscon.h                     |   22 ++
 13 files changed, 585 insertions(+), 201 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/imx-syscon.txt
 delete mode 100644 drivers/mfd/anatop-mfd.c
 create mode 100644 drivers/mfd/imx-syscon.c
 create mode 100644 include/linux/fsl/imx6q-iomuxc-gpr.h
 delete mode 100644 include/linux/mfd/anatop.h
 create mode 100644 include/linux/mfd/imx-syscon.h



^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 1/7] mfd: add imx syscon driver based on regmap
  2012-08-22  7:18 [PATCH 0/7] add imx-syscon driver for general registers access Dong Aisheng
@ 2012-08-22  7:18 ` Dong Aisheng
  2012-08-22  8:29   ` Richard Zhao
                     ` (2 more replies)
  2012-08-22  7:18 ` [PATCH 2/7] ARM: imx6q: add iomuxc gpr support into imx-syscon Dong Aisheng
                   ` (5 subsequent siblings)
  6 siblings, 3 replies; 29+ messages in thread
From: Dong Aisheng @ 2012-08-22  7:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-kernel, linus.walleij, s.hauer, shawn.guo, kernel,
	grant.likely, rob.herring, sameo, lrg, broonie, richard.zhao,
	devicetree-discuss, swarren, paul.liu

From: Dong Aisheng <dong.aisheng@linaro.org>

Add regmap based imx syscon driver.
This is usually used for access misc bits in registers which does not belong
to a specific module, for example, IOMUXC GPR and ANATOP.
With this driver, we provide a standard API for client driver to call to
access registers which are registered into syscon.

Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
---
Currently it's just simply for IMX, however the driver really is not too
much imx specific.
If people want, we probably could extend it to support other platforms too.
---
 .../devicetree/bindings/mfd/imx-syscon.txt         |   11 +
 drivers/mfd/Kconfig                                |    8 +
 drivers/mfd/Makefile                               |    1 +
 drivers/mfd/imx-syscon.c                           |  193 ++++++++++++++++++++
 include/linux/mfd/imx-syscon.h                     |   22 +++
 5 files changed, 235 insertions(+), 0 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/imx-syscon.txt b/Documentation/devicetree/bindings/mfd/imx-syscon.txt
new file mode 100644
index 0000000..4a72994
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/imx-syscon.txt
@@ -0,0 +1,11 @@
+* Freescale IMX System Controller Registers R/W driver
+
+Required properties:
+- compatible: Should contain "fsl,imx-syscon".
+- reg: the register range can be access from imx-syscon
+
+Examples:
+gpr: iomuxc-gpr@020e0000 {
+	compatible = "fsl,imx6q-iomuxc", "fsl,imx-syscon";
+	reg = <0x020e0000 0x38>;
+};
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index b1a1462..20a050e 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -993,6 +993,14 @@ config MFD_ANATOP
 	  MFD controller. This controller embeds regulator and
 	  thermal devices for Freescale i.MX platforms.
 
+config MFD_IMX_SYSCON
+        bool "Freescale i.MX System Controller Register R/W Based on Regmap"
+        depends on ARCH_MXC
+        select REGMAP_MMIO
+        help
+          Select this option to enable access Freescale i.MX system control
+	  registers like iomuxc gpr and anatop via regmap.
+
 config MFD_PALMAS
 	bool "Support for the TI Palmas series chips"
 	select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 79dd22d..82c7ee1 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -131,4 +131,5 @@ obj-$(CONFIG_MFD_PALMAS)	+= palmas.o
 obj-$(CONFIG_MFD_RC5T583)	+= rc5t583.o rc5t583-irq.o
 obj-$(CONFIG_MFD_SEC_CORE)	+= sec-core.o sec-irq.o
 obj-$(CONFIG_MFD_ANATOP)	+= anatop-mfd.o
+obj-$(CONFIG_MFD_IMX_SYSCON)	+= imx-syscon.o
 obj-$(CONFIG_MFD_LM3533)	+= lm3533-core.o lm3533-ctrlbank.o
diff --git a/drivers/mfd/imx-syscon.c b/drivers/mfd/imx-syscon.c
new file mode 100644
index 0000000..141b456
--- /dev/null
+++ b/drivers/mfd/imx-syscon.c
@@ -0,0 +1,193 @@
+/*
+ * Freescale IMX System Control Driver
+ *
+ * Copyright (C) 2012 Freescale Semiconductor, Inc.
+ * Copyright (C) 2012 Linaro Ltd.
+ *
+ * Author: Dong Aisheng <dong.aisheng@linaro.org>
+ *
+ * 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.
+ */
+
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+static struct platform_driver imx_syscon_driver;
+
+struct imx_syscon {
+	struct device *dev;
+	void __iomem *base;
+	struct regmap *regmap;
+};
+
+static int imx_syscon_match(struct device *dev, void *data)
+{
+	struct imx_syscon *syscon = dev_get_drvdata(dev);
+	struct device_node *dn = data;
+
+	return (syscon->dev->of_node == dn) ? 1 : 0;
+}
+
+int imx_syscon_write(struct device_node *np, u32 reg, u32 val)
+{
+	struct device *dev;
+	struct imx_syscon *syscon;
+	int ret = 0;
+
+	dev = driver_find_device(&imx_syscon_driver.driver, NULL, np,
+				 imx_syscon_match);
+	if (!dev)
+		return -EPROBE_DEFER;
+
+	syscon = dev_get_drvdata(dev);
+	ret = regmap_write(syscon->regmap, reg, val);
+	if (ret)
+		dev_err(dev, "failed to write regmap(%s) reg 0x%x (%d)\n",
+			np->name, reg, ret);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(imx_syscon_write);
+
+int imx_syscon_read(struct device_node *np, u32 reg, u32 *val)
+{
+	struct device *dev;
+	struct imx_syscon *syscon;
+	int ret = 0;
+
+	dev = driver_find_device(&imx_syscon_driver.driver, NULL, np,
+				 imx_syscon_match);
+	if (!dev)
+		return -EPROBE_DEFER;
+
+	syscon = dev_get_drvdata(dev);
+	ret = regmap_read(syscon->regmap, reg, val);
+	if (ret)
+		dev_err(dev, "failed to read regmap(%s) reg 0x%x (%d)\n",
+			np->name, reg, ret);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(imx_syscon_read);
+
+int imx_syscon_update_bits(struct device_node *np, u32 reg,
+					u32 mask, u32 val)
+{
+	struct device *dev;
+	struct imx_syscon *syscon;
+	int ret = 0;
+
+	dev = driver_find_device(&imx_syscon_driver.driver, NULL, np,
+				 imx_syscon_match);
+	if (!dev)
+		return -EPROBE_DEFER;
+
+	syscon = dev_get_drvdata(dev);
+	ret = regmap_update_bits(syscon->regmap, reg, mask, val);
+	if (ret)
+		dev_err(dev, "failed to update regmap(%s) reg 0x%x (%d)\n",
+			np->name, reg, ret);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(imx_syscon_update_bits);
+
+static const struct of_device_id of_imx_syscon_match[] = {
+	{ .compatible = "fsl,imx-syscon", },
+	{ },
+};
+
+static struct regmap_config imx_syscon_regmap_config = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+};
+
+static int __devinit imx_syscon_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct imx_syscon *syscon;
+	struct resource res;
+	int ret;
+
+	if (!np)
+		return -ENOENT;
+
+	syscon = devm_kzalloc(&pdev->dev, sizeof(struct imx_syscon),
+			    GFP_KERNEL);
+	if (!syscon)
+		return -ENOMEM;
+
+	syscon->base = of_iomap(np, 0);
+	if (!syscon->base)
+		return -EADDRNOTAVAIL;
+
+	ret = of_address_to_resource(np, 0, &res);
+	if (ret)
+		return ret;
+
+	imx_syscon_regmap_config.max_register = res.end - res.start - 3;
+	syscon->regmap = devm_regmap_init_mmio(&pdev->dev, syscon->base,
+					&imx_syscon_regmap_config);
+	if (IS_ERR(syscon->regmap)) {
+		dev_err(&pdev->dev, "regmap init failed\n");
+		return PTR_ERR(syscon->regmap);
+	}
+
+	regcache_cache_only(syscon->regmap, false);
+
+	dev_info(dev, "syscon regmap start 0x%x end 0x%x registered\n",
+		res.start, res.end);
+
+	syscon->dev = &pdev->dev;
+	platform_set_drvdata(pdev, syscon);
+
+	return 0;
+}
+
+static int __devexit imx_syscon_remove(struct platform_device *pdev)
+{
+	struct imx_syscon *syscon;
+
+	syscon = platform_get_drvdata(pdev);
+	iounmap(syscon->base);
+	platform_set_drvdata(pdev, NULL);
+
+	return 0;
+}
+
+static struct platform_driver imx_syscon_driver = {
+	.driver = {
+		.name = "imx-syscon",
+		.owner = THIS_MODULE,
+		.of_match_table = of_imx_syscon_match,
+	},
+	.probe		= imx_syscon_probe,
+	.remove		= imx_syscon_remove,
+};
+
+static int __init imx_syscon_init(void)
+{
+	return platform_driver_register(&imx_syscon_driver);
+}
+postcore_initcall(imx_syscon_init);
+
+static void __exit anatop_exit(void)
+{
+	platform_driver_unregister(&imx_syscon_driver);
+}
+module_exit(anatop_exit);
+
+MODULE_AUTHOR("Dong Aisheng <dong.aisheng@linaro.org>");
+MODULE_DESCRIPTION("Freescale IMX System Control driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/imx-syscon.h b/include/linux/mfd/imx-syscon.h
new file mode 100644
index 0000000..be8b6db
--- /dev/null
+++ b/include/linux/mfd/imx-syscon.h
@@ -0,0 +1,22 @@
+/*
+ * Freescale IMX System Control Driver
+ *
+ * Copyright (C) 2012 Freescale Semiconductor, Inc.
+ * Copyright (C) 2012 Linaro Ltd.
+ *
+ * Author: Dong Aisheng <dong.aisheng@linaro.org>
+ *
+ * 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.
+ */
+
+#ifndef __LINUX_MFD_IMX_SYSCON_H__
+#define __LINUX_MFD_IMX_SYSCON_H__
+
+extern int imx_syscon_write(struct device_node *np, u32 reg, u32 val);
+extern int imx_syscon_read(struct device_node *np, u32 reg, u32 *val);
+extern int imx_syscon_update_bits(struct device_node *np, u32 reg,
+					u32 mask, u32 val);
+#endif /* __LINUX_MFD_IMX_SYSCON_H__ */
-- 
1.7.0.4



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 2/7] ARM: imx6q: add iomuxc gpr support into imx-syscon
  2012-08-22  7:18 [PATCH 0/7] add imx-syscon driver for general registers access Dong Aisheng
  2012-08-22  7:18 ` [PATCH 1/7] mfd: add imx syscon driver based on regmap Dong Aisheng
@ 2012-08-22  7:18 ` Dong Aisheng
  2012-08-22  7:18 ` [PATCH 3/7] ARM: imx6q: add anatop " Dong Aisheng
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 29+ messages in thread
From: Dong Aisheng @ 2012-08-22  7:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-kernel, linus.walleij, s.hauer, shawn.guo, kernel,
	grant.likely, rob.herring, sameo, lrg, broonie, richard.zhao,
	devicetree-discuss, swarren, paul.liu

From: Dong Aisheng <dong.aisheng@linaro.org>

Include headfile for easy using.

Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
---
 arch/arm/boot/dts/imx6q.dtsi         |    5 +
 include/linux/fsl/imx6q-iomuxc-gpr.h |  319 ++++++++++++++++++++++++++++++++++
 2 files changed, 324 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index fd57079..1306868 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -507,6 +507,11 @@
 				interrupts = <0 89 0x04 0 90 0x04>;
 			};
 
+			gpr: iomuxc-gpr@020e0000 {
+				compatible = "fsl,imx6q-iomuxc-gpr", "fsl,imx-syscon";
+				reg = <0x020e0000 0x38>;
+			};
+
 			iomuxc@020e0000 {
 				compatible = "fsl,imx6q-iomuxc";
 				reg = <0x020e0000 0x4000>;
diff --git a/include/linux/fsl/imx6q-iomuxc-gpr.h b/include/linux/fsl/imx6q-iomuxc-gpr.h
new file mode 100644
index 0000000..2c44573
--- /dev/null
+++ b/include/linux/fsl/imx6q-iomuxc-gpr.h
@@ -0,0 +1,319 @@
+/*
+ * Copyright (C) 2012 Freescale Semiconductor, Inc.
+ *
+ * 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.
+ */
+
+#ifndef __LINUX_IMX6Q_IOMUXC_GPR_H
+#define __LINUX_IMX6Q_IOMUXC_GPR_H
+
+#include <linux/bitops.h>
+
+#define IOMUXC_GPR0	0x00
+#define IOMUXC_GPR1	0x04
+#define IOMUXC_GPR2	0x08
+#define IOMUXC_GPR3	0x0c
+#define IOMUXC_GPR4	0x10
+#define IOMUXC_GPR5	0x14
+#define IOMUXC_GPR6	0x18
+#define IOMUXC_GPR7	0x1c
+#define IOMUXC_GPR8	0x20
+#define IOMUXC_GPR9	0x24
+#define IOMUXC_GPR10	0x28
+#define IOMUXC_GPR11	0x2c
+#define IOMUXC_GPR12	0x30
+#define IOMUXC_GPR13	0x34
+
+#define IMX6Q_GPR0_CLOCK_8_MUX_SEL_MASK		(0x3 << 30)
+#define IMX6Q_GPR0_CLOCK_8_MUX_SEL_AUDMUX_RXCLK_P7_MUXED	(0x0 << 30)
+#define IMX6Q_GPR0_CLOCK_8_MUX_SEL_AUDMUX_RXCLK_P7	(0x1 << 30)
+#define IMX6Q_GPR0_CLOCK_8_MUX_SEL_SSI3_SSI_SRCK	(0x2 << 30)
+#define IMX6Q_GPR0_CLOCK_8_MUX_SEL_SSI3_RX_BIT_CLK	(0x3 << 30)
+#define IMX6Q_GPR0_CLOCK_0_MUX_SEL_MASK		(0x3 << 28)
+#define IMX6Q_GPR0_CLOCK_0_MUX_SEL_ESAI1_IPP_IND_SCKR_MUXED	(0x0 << 28)
+#define IMX6Q_GPR0_CLOCK_0_MUX_SEL_ESAI1_IPP_IND_SCKR	(0x1 << 28)
+#define IMX6Q_GPR0_CLOCK_0_MUX_SEL_ESAI1_IPP_DO_SCKR	(0x2 << 28)
+#define IMX6Q_GPR0_CLOCK_B_MUX_SEL_MASK		(0x3 << 26)
+#define IMX6Q_GPR0_CLOCK_B_MUX_SEL_AUDMUX_TXCLK_P7_MUXED	(0x0 << 26)
+#define IMX6Q_GPR0_CLOCK_B_MUX_SEL_AUDMUX_TXCLK_P7	(0x1 << 26)
+#define IMX6Q_GPR0_CLOCK_B_MUX_SEL_SSI3_SSI_STCK	(0x2 << 26)
+#define IMX6Q_GPR0_CLOCK_B_MUX_SEL_SSI3_TX_BIT_CLK	(0x3 << 26)
+#define IMX6Q_GPR0_CLOCK_3_MUX_SEL_MASK		(0x3 << 24)
+#define IMX6Q_GPR0_CLOCK_3_MUX_SEL_AUDMUX_RXCLK_P7_MUXED	(0x3 << 24)
+#define IMX6Q_GPR0_CLOCK_3_MUX_SEL_AUDMUX_RXCLK_P7	(0x3 << 24)
+#define IMX6Q_GPR0_CLOCK_3_MUX_SEL_SSI3_SSI_SRCK	(0x3 << 24)
+#define IMX6Q_GPR0_CLOCK_3_MUX_SEL_SSI3_RX_BIT_CLK	(0x3 << 24)
+#define IMX6Q_GPR0_CLOCK_A_MUX_SEL_MASK		(0x3 << 22)
+#define IMX6Q_GPR0_CLOCK_A_MUX_SEL_AUDMUX_TXCLK_P2_MUXED	(0x0 << 22)
+#define IMX6Q_GPR0_CLOCK_A_MUX_SEL_AUDMUX_TXCLK_P2	(0x1 << 22)
+#define IMX6Q_GPR0_CLOCK_A_MUX_SEL_SSI2_SSI_STCK	(0x2 << 22)
+#define IMX6Q_GPR0_CLOCK_A_MUX_SEL_SSI2_TX_BIT_CLK	(0x3 << 22)
+#define IMX6Q_GPR0_CLOCK_2_MUX_SEL_MASK		(0x3 << 20)
+#define IMX6Q_GPR0_CLOCK_2_MUX_SEL_AUDMUX_RXCLK_P2_MUXED	(0x0 << 20)
+#define IMX6Q_GPR0_CLOCK_2_MUX_SEL_AUDMUX_RXCLK_P2	(0x1 << 20)
+#define IMX6Q_GPR0_CLOCK_2_MUX_SEL_SSI2_SSI_SRCK	(0x2 << 20)
+#define IMX6Q_GPR0_CLOCK_2_MUX_SEL_SSI2_RX_BIT_CLK	(0x3 << 20)
+#define IMX6Q_GPR0_CLOCK_9_MUX_SEL_MASK		(0x3 << 18)
+#define IMX6Q_GPR0_CLOCK_9_MUX_SEL_AUDMUX_TXCLK_P1_MUXED	(0x0 << 18)
+#define IMX6Q_GPR0_CLOCK_9_MUX_SEL_AUDMUX_TXCLK_P1	(0x1 << 18)
+#define IMX6Q_GPR0_CLOCK_9_MUX_SEL_SSI1_SSI_STCK	(0x2 << 18)
+#define IMX6Q_GPR0_CLOCK_9_MUX_SEL_SSI1_SSI_TX_BIT_CLK	(0x3 << 18)
+#define IMX6Q_GPR0_CLOCK_1_MUX_SEL_MASK		(0x3 << 16)
+#define IMX6Q_GPR0_CLOCK_1_MUX_SEL_AUDMUX_RXCLK_P1_MUXED	(0x0 << 16)
+#define IMX6Q_GPR0_CLOCK_1_MUX_SEL_AUDMUX_RXCLK_P1	(0x1 << 16)
+#define IMX6Q_GPR0_CLOCK_1_MUX_SEL_SSI1_SSI_SRCK	(0x2 << 16)
+#define IMX6Q_GPR0_CLOCK_1_MUX_SEL_SSI1_SSI_RX_BIT_CLK	(0x3 << 16)
+#define IMX6Q_GPR0_TX_CLK2_MUX_SEL_MASK		(0x3 << 14)
+#define IMX6Q_GPR0_TX_CLK2_MUX_SEL_ASRCK_CLK1	(0x0 << 14)
+#define IMX6Q_GPR0_TX_CLK2_MUX_SEL_ASRCK_CLK2	(0x1 << 14)
+#define IMX6Q_GPR0_TX_CLK2_MUX_SEL_ASRCK_CLK3	(0x2 << 14)
+#define IMX6Q_GPR0_DMAREQ_MUX_SEL7_MASK		BIT(7)
+#define IMX6Q_GPR0_DMAREQ_MUX_SEL7_SPDIF	0x0
+#define IMX6Q_GPR0_DMAREQ_MUX_SEL7_IOMUX	BIT(7)
+#define IMX6Q_GPR0_DMAREQ_MUX_SEL6_MASK		BIT(6)
+#define IMX6Q_GPR0_DMAREQ_MUX_SEL6_ESAI		0x0
+#define IMX6Q_GPR0_DMAREQ_MUX_SEL6_I2C3		BIT(6)
+#define IMX6Q_GPR0_DMAREQ_MUX_SEL5_MASK		BIT(5)
+#define IMX6Q_GPR0_DMAREQ_MUX_SEL5_ECSPI4	0x0
+#define IMX6Q_GPR0_DMAREQ_MUX_SEL5_EPIT2	BIT(5)
+#define IMX6Q_GPR0_DMAREQ_MUX_SEL4_MASK		BIT(4)
+#define IMX6Q_GPR0_DMAREQ_MUX_SEL4_ECSPI4	0x0
+#define IMX6Q_GPR0_DMAREQ_MUX_SEL4_I2C1		BIT(4)
+#define IMX6Q_GPR0_DMAREQ_MUX_SEL3_MASK		BIT(3)
+#define IMX6Q_GPR0_DMAREQ_MUX_SEL3_ECSPI2	0x0
+#define IMX6Q_GPR0_DMAREQ_MUX_SEL3_I2C1		BIT(3)
+#define IMX6Q_GPR0_DMAREQ_MUX_SEL2_MASK		BIT(2)
+#define IMX6Q_GPR0_DMAREQ_MUX_SEL2_ECSPI1	0x0
+#define IMX6Q_GPR0_DMAREQ_MUX_SEL2_I2C2		BIT(2)
+#define IMX6Q_GPR0_DMAREQ_MUX_SEL1_MASK		BIT(1)
+#define IMX6Q_GPR0_DMAREQ_MUX_SEL1_ECSPI1	0x0
+#define IMX6Q_GPR0_DMAREQ_MUX_SEL1_I2C3		BIT(1)
+#define IMX6Q_GPR0_DMAREQ_MUX_SEL0_MASK		BIT(0)
+#define IMX6Q_GPR0_DMAREQ_MUX_SEL0_IPU1		0x0
+#define IMX6Q_GPR0_DMAREQ_MUX_SEL0_IOMUX	BIT(0)
+
+#define IMX6Q_GPR1_PCIE_REQ_MASK		(0x3 << 30)
+#define IMX6Q_GPR1_PCIE_EXIT_L1			BIT(28)
+#define IMX6Q_GPR1_PCIE_RDY_L23			BIT(27)
+#define IMX6Q_GPR1_PCIE_ENTER_L1		BIT(26)
+#define IMX6Q_GPR1_MIPI_COLOR_SW		BIT(25)
+#define IMX6Q_GPR1_DPI_OFF			BIT(24)
+#define IMX6Q_GPR1_EXC_MON_MASK			BIT(22)
+#define IMX6Q_GPR1_EXC_MON_OKAY			0x0
+#define IMX6Q_GPR1_EXC_MON_SLVE			BIT(22)
+#define IMX6Q_GPR1_MIPI_IPU2_SEL_MASK		BIT(21)
+#define IMX6Q_GPR1_MIPI_IPU2_SEL_GASKET		0x0
+#define IMX6Q_GPR1_MIPI_IPU2_SEL_IOMUX		BIT(21)
+#define IMX6Q_GPR1_MIPI_IPU1_MUX_MASK		BIT(20)
+#define IMX6Q_GPR1_MIPI_IPU1_MUX_GASKET		0x0
+#define IMX6Q_GPR1_MIPI_IPU1_MUX_IOMUX		BIT(20)
+#define IMX6Q_GPR1_MIPI_IPU2_MUX_MASK		BIT(19)
+#define IMX6Q_GPR1_MIPI_IPU2_MUX_GASKET		0x0
+#define IMX6Q_GPR1_MIPI_IPU2_MUX_IOMUX		BIT(19)
+#define IMX6Q_GPR1_PCIE_TEST_PD			BIT(18)
+#define IMX6Q_GPR1_IPU_VPU_MUX_MASK		BIT(17)
+#define IMX6Q_GPR1_IPU_VPU_MUX_IPU1		0x0
+#define IMX6Q_GPR1_IPU_VPU_MUX_IPU2		BIT(17)
+#define IMX6Q_GPR1_PCIE_REF_CLK_EN		BIT(16)
+#define IMX6Q_GPR1_USB_EXP_MODE			BIT(15)
+#define IMX6Q_GPR1_PCIE_INT			BIT(14)
+#define IMX6Q_GPR1_USB_OTG_ID_SEL_MASK		BIT(13)
+#define IMX6Q_GPR1_USB_OTG_ID_SEL_ENET_RX_ER	0x0
+#define IMX6Q_GPR1_USB_OTG_ID_SEL_GPIO_1	BIT(13)
+#define IMX6Q_GPR1_GINT				BIT(12)
+#define IMX6Q_GPR1_ADDRS3_MASK			(0x3 << 10)
+#define IMX6Q_GPR1_ADDRS3_32MB			(0x0 << 10)
+#define IMX6Q_GPR1_ADDRS3_64MB			(0x1 << 10)
+#define IMX6Q_GPR1_ADDRS3_128MB			(0x2 << 10)
+#define IMX6Q_GPR1_ACT_CS3			BIT(9)
+#define IMX6Q_GPR1_ADDRS2_MASK			(0x3 << 7)
+#define IMX6Q_GPR1_ACT_CS2			BIT(6)
+#define IMX6Q_GPR1_ADDRS1_MASK			(0x3 << 4)
+#define IMX6Q_GPR1_ACT_CS1			BIT(3)
+#define IMX6Q_GPR1_ADDRS0_MASK			(0x3 << 1)
+#define IMX6Q_GPR1_ACT_CS0			BIT(0)
+
+#define IMX6Q_GPR2_COUNTER_RESET_VAL_MASK	(0x3 << 20)
+#define IMX6Q_GPR2_COUNTER_RESET_VAL_5		(0x0 << 20)
+#define IMX6Q_GPR2_COUNTER_RESET_VAL_3		(0x1 << 20)
+#define IMX6Q_GPR2_COUNTER_RESET_VAL_4		(0x2 << 20)
+#define IMX6Q_GPR2_COUNTER_RESET_VAL_6		(0x3 << 20)
+#define IMX6Q_GPR2_LVDS_CLK_SHIFT_MASK		(0x7 << 16)
+#define IMX6Q_GPR2_LVDS_CLK_SHIFT_0		(0x0 << 16)
+#define IMX6Q_GPR2_LVDS_CLK_SHIFT_1		(0x1 << 16)
+#define IMX6Q_GPR2_LVDS_CLK_SHIFT_2		(0x2 << 16)
+#define IMX6Q_GPR2_LVDS_CLK_SHIFT_3		(0x3 << 16)
+#define IMX6Q_GPR2_LVDS_CLK_SHIFT_4		(0x4 << 16)
+#define IMX6Q_GPR2_LVDS_CLK_SHIFT_5		(0x5 << 16)
+#define IMX6Q_GPR2_LVDS_CLK_SHIFT_6		(0x6 << 16)
+#define IMX6Q_GPR2_LVDS_CLK_SHIFT_7		(0x7 << 16)
+#define IMX6Q_GPR2_BGREF_RRMODE_MASK		BIT(15)
+#define IMX6Q_GPR2_BGREF_RRMODE_EXT_RESISTOR	0x0
+#define IMX6Q_GPR2_BGREF_RRMODE_INT_RESISTOR	BIT(15)
+#define IMX6Q_GPR2_DI1_VS_POLARITY_MASK		BIT(10)
+#define IMX6Q_GPR2_DI1_VS_POLARITY_ACTIVE_H	0x0
+#define IMX6Q_GPR2_DI1_VS_POLARITY_ACTIVE_L	BIT(10)
+#define IMX6Q_GPR2_DI0_VS_POLARITY_MASK		BIT(9)
+#define IMX6Q_GPR2_DI0_VS_POLARITY_ACTIVE_H	0x0
+#define IMX6Q_GPR2_DI0_VS_POLARITY_ACTIVE_L	BIT(9)
+#define IMX6Q_GPR2_BIT_MAPPING_CH1_MASK		BIT(8)
+#define IMX6Q_GPR2_BIT_MAPPING_CH1_SPWG		0x0
+#define IMX6Q_GPR2_BIT_MAPPING_CH1_JEIDA	BIT(8)
+#define IMX6Q_GPR2_DATA_WIDTH_CH1_MASK		BIT(7)
+#define IMX6Q_GPR2_DATA_WIDTH_CH1_18BIT		0x0
+#define IMX6Q_GPR2_DATA_WIDTH_CH1_24BIT		BIT(7)
+#define IMX6Q_GPR2_BIT_MAPPING_CH0_MASK		BIT(6)
+#define IMX6Q_GPR2_BIT_MAPPING_CH0_SPWG		0x0
+#define IMX6Q_GPR2_BIT_MAPPING_CH0_JEIDA	BIT(6)
+#define IMX6Q_GPR2_DATA_WIDTH_CH0_MASK		BIT(5)
+#define IMX6Q_GPR2_DATA_WIDTH_CH0_18BIT		0x0
+#define IMX6Q_GPR2_DATA_WIDTH_CH0_24BIT		BIT(5)
+#define IMX6Q_GPR2_SPLIT_MODE_EN		BIT(4)
+#define IMX6Q_GPR2_CH1_MODE_MASK		(0x3 << 2)
+#define IMX6Q_GPR2_CH1_MODE_DISABLE		(0x0 << 2)
+#define IMX6Q_GPR2_CH1_MODE_EN_ROUTE_DI0	(0x1 << 2)
+#define IMX6Q_GPR2_CH1_MODE_EN_ROUTE_DI1	(0x3 << 2)
+#define IMX6Q_GPR2_CH0_MODE_MASK		(0x3 << 0)
+#define IMX6Q_GPR2_CH0_MODE_DISABLE		(0x0 << 0)
+#define IMX6Q_GPR2_CH0_MODE_EN_ROUTE_DI0	(0x1 << 0)
+#define IMX6Q_GPR2_CH0_MODE_EN_ROUTE_DI1	(0x3 << 0)
+
+#define IMX6Q_GPR3_GPU_DBG_MASK			(0x3 << 29)
+#define IMX6Q_GPR3_GPU_DBG_GPU3D		(0x0 << 29)
+#define IMX6Q_GPR3_GPU_DBG_GPU2D		(0x1 << 29)
+#define IMX6Q_GPR3_GPU_DBG_OPENVG		(0x2 << 29)
+#define IMX6Q_GPR3_BCH_WR_CACHE_CTL		BIT(28)
+#define IMX6Q_GPR3_BCH_RD_CACHE_CTL		BIT(27)
+#define IMX6Q_GPR3_USDHCX_WR_CACHE_CTL		BIT(26)
+#define IMX6Q_GPR3_USDHCX_RD_CACHE_CTL		BIT(25)
+#define IMX6Q_GPR3_OCRAM_CTL_MASK		(0xf << 21)
+#define IMX6Q_GPR3_OCRAM_STATUS_MASK		(0xf << 17)
+#define IMX6Q_GPR3_CORE3_DBG_ACK_EN		BIT(16)
+#define IMX6Q_GPR3_CORE2_DBG_ACK_EN		BIT(15)
+#define IMX6Q_GPR3_CORE1_DBG_ACK_EN		BIT(14)
+#define IMX6Q_GPR3_CORE0_DBG_ACK_EN		BIT(13)
+#define IMX6Q_GPR3_TZASC2_BOOT_LOCK		BIT(12)
+#define IMX6Q_GPR3_TZASC1_BOOT_LOCK		BIT(11)
+#define IMX6Q_GPR3_IPU_DIAG_MASK		BIT(10)
+#define IMX6Q_GPR3_LVDS1_MUX_CTL_MASK		(0x3 << 8)
+#define IMX6Q_GPR3_LVDS1_MUX_CTL_IPU1_DI0	(0x0 << 8)
+#define IMX6Q_GPR3_LVDS1_MUX_CTL_IPU1_DI1	(0x1 << 8)
+#define IMX6Q_GPR3_LVDS1_MUX_CTL_IPU2_DI0	(0x2 << 8)
+#define IMX6Q_GPR3_LVDS1_MUX_CTL_IPU2_DI1	(0x3 << 8)
+#define IMX6Q_GPR3_LVDS0_MUX_CTL_MASK		(0x3 << 6)
+#define IMX6Q_GPR3_LVDS0_MUX_CTL_IPU1_DI0	(0x0 << 6)
+#define IMX6Q_GPR3_LVDS0_MUX_CTL_IPU1_DI1	(0x1 << 6)
+#define IMX6Q_GPR3_LVDS0_MUX_CTL_IPU2_DI0	(0x2 << 6)
+#define IMX6Q_GPR3_LVDS0_MUX_CTL_IPU2_DI1	(0x3 << 6)
+#define IMX6Q_GPR3_MIPI_MUX_CTL_MASK		(0x3 << 4)
+#define IMX6Q_GPR3_MIPI_MUX_CTL_IPU1_DI0	(0x0 << 4)
+#define IMX6Q_GPR3_MIPI_MUX_CTL_IPU1_DI1	(0x1 << 4)
+#define IMX6Q_GPR3_MIPI_MUX_CTL_IPU2_DI0	(0x2 << 4)
+#define IMX6Q_GPR3_MIPI_MUX_CTL_IPU2_DI1	(0x3 << 4)
+#define IMX6Q_GPR3_HDMI_MUX_CTL_MASK		(0x3 << 2)
+#define IMX6Q_GPR3_HDMI_MUX_CTL_IPU1_DI0	(0x0 << 2)
+#define IMX6Q_GPR3_HDMI_MUX_CTL_IPU1_DI1	(0x1 << 2)
+#define IMX6Q_GPR3_HDMI_MUX_CTL_IPU2_DI0	(0x2 << 2)
+#define IMX6Q_GPR3_HDMI_MUX_CTL_IPU2_DI1	(0x3 << 2)
+
+#define IMX6Q_GPR4_VDOA_WR_CACHE_SEL		BIT(31)
+#define IMX6Q_GPR4_VDOA_RD_CACHE_SEL		BIT(30)
+#define IMX6Q_GPR4_VDOA_WR_CACHE_VAL		BIT(29)
+#define IMX6Q_GPR4_VDOA_RD_CACHE_VAL		BIT(28)
+#define IMX6Q_GPR4_PCIE_WR_CACHE_SEL		BIT(27)
+#define IMX6Q_GPR4_PCIE_RD_CACHE_SEL		BIT(26)
+#define IMX6Q_GPR4_PCIE_WR_CACHE_VAL		BIT(25)
+#define IMX6Q_GPR4_PCIE_RD_CACHE_VAL		BIT(24)
+#define IMX6Q_GPR4_SDMA_STOP_ACK		BIT(19)
+#define IMX6Q_GPR4_CAN2_STOP_ACK		BIT(18)
+#define IMX6Q_GPR4_CAN1_STOP_ACK		BIT(17)
+#define IMX6Q_GPR4_ENET_STOP_ACK		BIT(16)
+#define IMX6Q_GPR4_SOC_VERSION_MASK		(0xff << 8)
+#define IMX6Q_GPR4_SOC_VERSION_OFF		0x8
+#define IMX6Q_GPR4_VPU_WR_CACHE_SEL		BIT(7)
+#define IMX6Q_GPR4_VPU_RD_CACHE_SEL		BIT(6)
+#define IMX6Q_GPR4_VPU_P_WR_CACHE_VAL		BIT(3)
+#define IMX6Q_GPR4_VPU_P_RD_CACHE_VAL_MASK	BIT(2)
+#define IMX6Q_GPR4_IPU_WR_CACHE_CTL		BIT(1)
+#define IMX6Q_GPR4_IPU_RD_CACHE_CTL		BIT(0)
+
+#define IMX6Q_GPR5_L2_CLK_STOP			BIT(8)
+
+#define IMX6Q_GPR9_TZASC2_BYP			BIT(1)
+#define IMX6Q_GPR9_TZASC1_BYP			BIT(0)
+
+#define IMX6Q_GPR10_LOCK_DBG_EN			BIT(29)
+#define IMX6Q_GPR10_LOCK_DBG_CLK_EN		BIT(28)
+#define IMX6Q_GPR10_LOCK_SEC_ERR_RESP		BIT(27)
+#define IMX6Q_GPR10_LOCK_OCRAM_TZ_ADDR		(0x3f << 21)
+#define IMX6Q_GPR10_LOCK_OCRAM_TZ_EN		BIT(20)
+#define IMX6Q_GPR10_LOCK_DCIC2_MUX_MASK		(0x3 << 18)
+#define IMX6Q_GPR10_LOCK_DCIC1_MUX_MASK		(0x3 << 16)
+#define IMX6Q_GPR10_DBG_EN			BIT(13)
+#define IMX6Q_GPR10_DBG_CLK_EN			BIT(12)
+#define IMX6Q_GPR10_SEC_ERR_RESP_MASK		BIT(11)
+#define IMX6Q_GPR10_SEC_ERR_RESP_OKEY		0x0
+#define IMX6Q_GPR10_SEC_ERR_RESP_SLVE		BIT(11)
+#define IMX6Q_GPR10_OCRAM_TZ_ADDR_MASK		(0x3f << 5)
+#define IMX6Q_GPR10_OCRAM_TZ_EN_MASK		BIT(4)
+#define IMX6Q_GPR10_DCIC2_MUX_CTL_MASK		(0x3 << 2)
+#define IMX6Q_GPR10_DCIC2_MUX_CTL_IPU1_DI0	(0x0 << 2)
+#define IMX6Q_GPR10_DCIC2_MUX_CTL_IPU1_DI1	(0x1 << 2)
+#define IMX6Q_GPR10_DCIC2_MUX_CTL_IPU2_DI0	(0x2 << 2)
+#define IMX6Q_GPR10_DCIC2_MUX_CTL_IPU2_DI1	(0x3 << 2)
+#define IMX6Q_GPR10_DCIC1_MUX_CTL_MASK		(0x3 << 0)
+#define IMX6Q_GPR10_DCIC1_MUX_CTL_IPU1_DI0	(0x0 << 0)
+#define IMX6Q_GPR10_DCIC1_MUX_CTL_IPU1_DI1	(0x1 << 0)
+#define IMX6Q_GPR10_DCIC1_MUX_CTL_IPU2_DI0	(0x2 << 0)
+#define IMX6Q_GPR10_DCIC1_MUX_CTL_IPU2_DI1	(0x3 << 0)
+
+#define IMX6Q_GPR12_ARMP_IPG_CLK_EN		BIT(27)
+#define IMX6Q_GPR12_ARMP_AHB_CLK_EN		BIT(26)
+#define IMX6Q_GPR12_ARMP_ATB_CLK_EN		BIT(25)
+#define IMX6Q_GPR12_ARMP_APB_CLK_EN		BIT(24)
+#define IMX6Q_GPR12_PCIE_CTL_2			BIT(10)
+
+#define IMX6Q_GPR13_SDMA_STOP_REQ		BIT(30)
+#define IMX6Q_GPR13_CAN2_STOP_REQ		BIT(29)
+#define IMX6Q_GPR13_CAN1_STOP_REQ		BIT(28)
+#define IMX6Q_GPR13_ENET_STOP_REQ		BIT(27)
+#define IMX6Q_GPR13_SATA_PHY_8_MASK		(0x7 << 24)
+#define IMX6Q_GPR13_SATA_PHY_8_0_5_DB		(0x0 << 24)
+#define IMX6Q_GPR13_SATA_PHY_8_1_0_DB		(0x1 << 24)
+#define IMX6Q_GPR13_SATA_PHY_8_1_5_DB		(0x2 << 24)
+#define IMX6Q_GPR13_SATA_PHY_8_2_0_DB		(0x3 << 24)
+#define IMX6Q_GPR13_SATA_PHY_8_2_5_DB		(0x4 << 24)
+#define IMX6Q_GPR13_SATA_PHY_8_3_0_DB		(0x5 << 24)
+#define IMX6Q_GPR13_SATA_PHY_8_3_5_DB		(0x6 << 24)
+#define IMX6Q_GPR13_SATA_PHY_8_4_0_DB		(0x7 << 24)
+#define IMX6Q_GPR13_SATA_PHY_7_MASK		(0x1f << 19)
+#define IMX6Q_GPR13_SATA_PHY_7_SATA1I		(0x10 << 19)
+#define IMX6Q_GPR13_SATA_PHY_7_SATA1M		(0x10 << 19)
+#define IMX6Q_GPR13_SATA_PHY_7_SATA1X		(0x1a << 19)
+#define IMX6Q_GPR13_SATA_PHY_7_SATA2I		(0x12 << 19)
+#define IMX6Q_GPR13_SATA_PHY_7_SATA2M		(0x12 << 19)
+#define IMX6Q_GPR13_SATA_PHY_7_SATA2X		(0x1a << 19)
+#define IMX6Q_GPR13_SATA_PHY_6_MASK		(0x7 << 16)
+#define IMX6Q_GPR13_SATA_SPEED_MASK		BIT(15)
+#define IMX6Q_GPR13_SATA_SPEED_1P5G		0x0
+#define IMX6Q_GPR13_SATA_SPEED_3P0G		BIT(15)
+#define IMX6Q_GPR13_SATA_PHY_5			BIT(14)
+#define IMX6Q_GPR13_SATA_PHY_4_MASK		(0x7 << 11)
+#define IMX6Q_GPR13_SATA_PHY_4_16_16		(0x0 << 11)
+#define IMX6Q_GPR13_SATA_PHY_4_14_16		(0x1 << 11)
+#define IMX6Q_GPR13_SATA_PHY_4_12_16		(0x2 << 11)
+#define IMX6Q_GPR13_SATA_PHY_4_10_16		(0x3 << 11)
+#define IMX6Q_GPR13_SATA_PHY_4_9_16		(0x4 << 11)
+#define IMX6Q_GPR13_SATA_PHY_4_8_16		(0x5 << 11)
+#define IMX6Q_GPR13_SATA_PHY_3_MASK		(0xf << 7)
+#define IMX6Q_GPR13_SATA_PHY_3_OFF		0x7
+#define IMX6Q_GPR13_SATA_PHY_2_MASK		(0x1f << 2)
+#define IMX6Q_GPR13_SATA_PHY_2_OFF		0x2
+#define IMX6Q_GPR13_SATA_PHY_1_MASK		(0x3 << 0)
+#define IMX6Q_GPR13_SATA_PHY_1_FAST		(0x0 << 0)
+#define IMX6Q_GPR13_SATA_PHY_1_MED		(0x1 << 0)
+#define IMX6Q_GPR13_SATA_PHY_1_SLOW		(0x2 << 0)
+
+#endif /* !__LINUX_IMX6Q_IOMUXC_GPR_H */
-- 
1.7.0.4



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 3/7] ARM: imx6q: add anatop support into imx-syscon
  2012-08-22  7:18 [PATCH 0/7] add imx-syscon driver for general registers access Dong Aisheng
  2012-08-22  7:18 ` [PATCH 1/7] mfd: add imx syscon driver based on regmap Dong Aisheng
  2012-08-22  7:18 ` [PATCH 2/7] ARM: imx6q: add iomuxc gpr support into imx-syscon Dong Aisheng
@ 2012-08-22  7:18 ` Dong Aisheng
  2012-08-22  7:18 ` [PATCH 4/7] regulator: anatop-regulator: convert to use imx-syscon to access anatop register Dong Aisheng
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 29+ messages in thread
From: Dong Aisheng @ 2012-08-22  7:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-kernel, linus.walleij, s.hauer, shawn.guo, kernel,
	grant.likely, rob.herring, sameo, lrg, broonie, richard.zhao,
	devicetree-discuss, swarren, paul.liu

From: Dong Aisheng <dong.aisheng@linaro.org>

There're a few anatop registers need to be accessed by different modules.
Add anatop registers into imx-syscon support for easy access.

Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
---
 arch/arm/boot/dts/imx6q.dtsi |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index 1306868..7732b70 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -378,8 +378,8 @@
 				interrupts = <0 87 0x04 0 88 0x04>;
 			};
 
-			anatop@020c8000 {
-				compatible = "fsl,imx6q-anatop";
+			anatop: anatop@020c8000 {
+				compatible = "fsl,imx6q-anatop", "fsl,imx-syscon";
 				reg = <0x020c8000 0x1000>;
 				interrupts = <0 49 0x04 0 54 0x04 0 127 0x04>;
 
-- 
1.7.0.4



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 4/7] regulator: anatop-regulator: convert to use imx-syscon to access anatop register
  2012-08-22  7:18 [PATCH 0/7] add imx-syscon driver for general registers access Dong Aisheng
                   ` (2 preceding siblings ...)
  2012-08-22  7:18 ` [PATCH 3/7] ARM: imx6q: add anatop " Dong Aisheng
@ 2012-08-22  7:18 ` Dong Aisheng
  2012-08-22 15:59   ` Mark Brown
  2012-08-23  5:21   ` Stephen Warren
  2012-08-22  7:18 ` [PATCH 5/7] ARM: imx6q: convert to use imx-syscon to access anatop registers Dong Aisheng
                   ` (2 subsequent siblings)
  6 siblings, 2 replies; 29+ messages in thread
From: Dong Aisheng @ 2012-08-22  7:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-kernel, linus.walleij, s.hauer, shawn.guo, kernel,
	grant.likely, rob.herring, sameo, lrg, broonie, richard.zhao,
	devicetree-discuss, swarren, paul.liu

From: Dong Aisheng <dong.aisheng@linaro.org>

Using standard imx syscon API to access anatop register.

Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
---
 arch/arm/boot/dts/imx6q.dtsi         |    6 ++++++
 drivers/regulator/Kconfig            |    2 +-
 drivers/regulator/anatop-regulator.c |   17 +++++++++++------
 3 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index 7732b70..7076be0 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -395,6 +395,7 @@
 					anatop-min-bit-val = <4>;
 					anatop-min-voltage = <800000>;
 					anatop-max-voltage = <1375000>;
+					fsl,anatop = <&anatop>;
 				};
 
 				regulator-3p0@120 {
@@ -409,6 +410,7 @@
 					anatop-min-bit-val = <0>;
 					anatop-min-voltage = <2625000>;
 					anatop-max-voltage = <3400000>;
+					fsl,anatop = <&anatop>;
 				};
 
 				regulator-2p5@130 {
@@ -423,6 +425,7 @@
 					anatop-min-bit-val = <0>;
 					anatop-min-voltage = <2000000>;
 					anatop-max-voltage = <2750000>;
+					fsl,anatop = <&anatop>;
 				};
 
 				regulator-vddcore@140 {
@@ -437,6 +440,7 @@
 					anatop-min-bit-val = <1>;
 					anatop-min-voltage = <725000>;
 					anatop-max-voltage = <1450000>;
+					fsl,anatop = <&anatop>;
 				};
 
 				regulator-vddpu@140 {
@@ -451,6 +455,7 @@
 					anatop-min-bit-val = <1>;
 					anatop-min-voltage = <725000>;
 					anatop-max-voltage = <1450000>;
+					fsl,anatop = <&anatop>;
 				};
 
 				regulator-vddsoc@140 {
@@ -465,6 +470,7 @@
 					anatop-min-bit-val = <1>;
 					anatop-min-voltage = <725000>;
 					anatop-max-voltage = <1450000>;
+					fsl,anatop = <&anatop>;
 				};
 			};
 
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 4e932cc..7ceea1a 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -112,7 +112,7 @@ config REGULATOR_DA9052
 
 config REGULATOR_ANATOP
 	tristate "Freescale i.MX on-chip ANATOP LDO regulators"
-	depends on MFD_ANATOP
+	depends on MFD_IMX_SYSCON
 	help
 	  Say y here to support Freescale i.MX on-chip ANATOP LDOs
 	  regulators. It is recommended that this option be
diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
index ce0fe72..e0b9ef1 100644
--- a/drivers/regulator/anatop-regulator.c
+++ b/drivers/regulator/anatop-regulator.c
@@ -21,19 +21,20 @@
 #include <linux/slab.h>
 #include <linux/device.h>
 #include <linux/module.h>
+#include <linux/mfd/imx-syscon.h>
 #include <linux/err.h>
 #include <linux/io.h>
 #include <linux/platform_device.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
-#include <linux/mfd/anatop.h>
+#include <linux/regmap.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/of_regulator.h>
 
 struct anatop_regulator {
 	const char *name;
 	u32 control_reg;
-	struct anatop *mfd;
+	struct device_node *anatop;
 	int vol_bit_shift;
 	int vol_bit_width;
 	int min_bit_val;
@@ -56,7 +57,8 @@ static int anatop_set_voltage_sel(struct regulator_dev *reg, unsigned selector)
 	mask = ((1 << anatop_reg->vol_bit_width) - 1) <<
 		anatop_reg->vol_bit_shift;
 	val <<= anatop_reg->vol_bit_shift;
-	anatop_write_reg(anatop_reg->mfd, anatop_reg->control_reg, val, mask);
+	imx_syscon_update_bits(anatop_reg->anatop, anatop_reg->control_reg,
+				mask, val);
 
 	return 0;
 }
@@ -69,7 +71,7 @@ static int anatop_get_voltage_sel(struct regulator_dev *reg)
 	if (!anatop_reg->control_reg)
 		return -ENOTSUPP;
 
-	val = anatop_read_reg(anatop_reg->mfd, anatop_reg->control_reg);
+	imx_syscon_read(anatop_reg->anatop, anatop_reg->control_reg, &val);
 	mask = ((1 << anatop_reg->vol_bit_width) - 1) <<
 		anatop_reg->vol_bit_shift;
 	val = (val & mask) >> anatop_reg->vol_bit_shift;
@@ -92,7 +94,6 @@ static int __devinit anatop_regulator_probe(struct platform_device *pdev)
 	struct regulator_dev *rdev;
 	struct anatop_regulator *sreg;
 	struct regulator_init_data *initdata;
-	struct anatop *anatopmfd = dev_get_drvdata(pdev->dev.parent);
 	struct regulator_config config = { };
 	int ret = 0;
 
@@ -109,7 +110,11 @@ static int __devinit anatop_regulator_probe(struct platform_device *pdev)
 	rdesc->ops = &anatop_rops;
 	rdesc->type = REGULATOR_VOLTAGE;
 	rdesc->owner = THIS_MODULE;
-	sreg->mfd = anatopmfd;
+
+	sreg->anatop = of_parse_phandle(np, "fsl,anatop", 0);
+	if (!sreg->anatop)
+		return -ENODEV;
+
 	ret = of_property_read_u32(np, "anatop-reg-offset",
 				   &sreg->control_reg);
 	if (ret) {
-- 
1.7.0.4



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 5/7] ARM: imx6q: convert to use imx-syscon to access anatop registers
  2012-08-22  7:18 [PATCH 0/7] add imx-syscon driver for general registers access Dong Aisheng
                   ` (3 preceding siblings ...)
  2012-08-22  7:18 ` [PATCH 4/7] regulator: anatop-regulator: convert to use imx-syscon to access anatop register Dong Aisheng
@ 2012-08-22  7:18 ` Dong Aisheng
  2012-08-22  7:18 ` [PATCH 6/7] ARM: dts: imx6q: add simple-bus compatible string for anatop Dong Aisheng
  2012-08-22  7:18 ` [PATCH 7/7] mfd: anatop-mfd: remove anatop driver Dong Aisheng
  6 siblings, 0 replies; 29+ messages in thread
From: Dong Aisheng @ 2012-08-22  7:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-kernel, linus.walleij, s.hauer, shawn.guo, kernel,
	grant.likely, rob.herring, sameo, lrg, broonie, richard.zhao,
	devicetree-discuss, swarren, paul.liu

From: Dong Aisheng <dong.aisheng@linaro.org>

Using standard imx syscon API to access anatop registers.

Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
---
 arch/arm/mach-imx/Kconfig      |    2 +-
 arch/arm/mach-imx/mach-imx6q.c |   25 ++++++-------------------
 2 files changed, 7 insertions(+), 20 deletions(-)

diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index afd542a..c7b2adb 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -839,7 +839,7 @@ config SOC_IMX6Q
 	select HAVE_IMX_MMDC
 	select HAVE_IMX_SRC
 	select HAVE_SMP
-	select MFD_ANATOP
+	select MFD_IMX_SYSCON
 	select PINCTRL
 	select PINCTRL_IMX6Q
 
diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index 5ec0608..32a9c9e 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -24,8 +24,9 @@
 #include <linux/of_platform.h>
 #include <linux/pinctrl/machine.h>
 #include <linux/phy.h>
+#include <linux/regmap.h>
 #include <linux/micrel_phy.h>
-#include <linux/mfd/anatop.h>
+#include <linux/mfd/imx-syscon.h>
 #include <asm/cpuidle.h>
 #include <asm/smp_twd.h>
 #include <asm/hardware/cache-l2x0.h>
@@ -121,20 +122,8 @@ static void __init imx6q_sabrelite_init(void)
 static void __init imx6q_usb_init(void)
 {
 	struct device_node *np;
-	struct platform_device *pdev = NULL;
-	struct anatop *adata = NULL;
 
 	np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-anatop");
-	if (np)
-		pdev = of_find_device_by_node(np);
-	if (pdev)
-		adata = platform_get_drvdata(pdev);
-	if (!adata) {
-		if (np)
-			of_node_put(np);
-		return;
-	}
-
 #define HW_ANADIG_USB1_CHRG_DETECT		0x000001b0
 #define HW_ANADIG_USB2_CHRG_DETECT		0x00000210
 
@@ -145,14 +134,12 @@ static void __init imx6q_usb_init(void)
 	 * The external charger detector needs to be disabled,
 	 * or the signal at DP will be poor
 	 */
-	anatop_write_reg(adata, HW_ANADIG_USB1_CHRG_DETECT,
+	imx_syscon_write(np, HW_ANADIG_USB1_CHRG_DETECT,
 			BM_ANADIG_USB_CHRG_DETECT_EN_B
-			| BM_ANADIG_USB_CHRG_DETECT_CHK_CHRG_B,
-			~0);
-	anatop_write_reg(adata, HW_ANADIG_USB2_CHRG_DETECT,
+			| BM_ANADIG_USB_CHRG_DETECT_CHK_CHRG_B);
+	imx_syscon_write(np, HW_ANADIG_USB2_CHRG_DETECT,
 			BM_ANADIG_USB_CHRG_DETECT_EN_B |
-			BM_ANADIG_USB_CHRG_DETECT_CHK_CHRG_B,
-			~0);
+			BM_ANADIG_USB_CHRG_DETECT_CHK_CHRG_B);
 
 	of_node_put(np);
 }
-- 
1.7.0.4



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 6/7] ARM: dts: imx6q: add simple-bus compatible string for anatop
  2012-08-22  7:18 [PATCH 0/7] add imx-syscon driver for general registers access Dong Aisheng
                   ` (4 preceding siblings ...)
  2012-08-22  7:18 ` [PATCH 5/7] ARM: imx6q: convert to use imx-syscon to access anatop registers Dong Aisheng
@ 2012-08-22  7:18 ` Dong Aisheng
  2012-08-22  8:52   ` Richard Zhao
  2012-08-22  7:18 ` [PATCH 7/7] mfd: anatop-mfd: remove anatop driver Dong Aisheng
  6 siblings, 1 reply; 29+ messages in thread
From: Dong Aisheng @ 2012-08-22  7:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-kernel, linus.walleij, s.hauer, shawn.guo, kernel,
	grant.likely, rob.herring, sameo, lrg, broonie, richard.zhao,
	devicetree-discuss, swarren, paul.liu

From: Dong Aisheng <dong.aisheng@linaro.org>

Originally the anatop regulator devices are populated by mfd anatop driver.
Since mfd anatop driver will be deleted later, we change to populate the
regulator devices by devicetree automatically.
This will cause some warning messages as follows during boot due to device
recreation: "vdd1p1: Failed to create debugfs directory"
But it does not break any function.
Later, we will remove mfd anatop driver which can get rid of this
error message.

Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
---
 arch/arm/boot/dts/imx6q.dtsi |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index 7076be0..426f735 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -379,7 +379,7 @@
 			};
 
 			anatop: anatop@020c8000 {
-				compatible = "fsl,imx6q-anatop", "fsl,imx-syscon";
+				compatible = "fsl,imx6q-anatop", "fsl,imx-syscon", "simple-bus";
 				reg = <0x020c8000 0x1000>;
 				interrupts = <0 49 0x04 0 54 0x04 0 127 0x04>;
 
-- 
1.7.0.4



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 7/7] mfd: anatop-mfd: remove anatop driver
  2012-08-22  7:18 [PATCH 0/7] add imx-syscon driver for general registers access Dong Aisheng
                   ` (5 preceding siblings ...)
  2012-08-22  7:18 ` [PATCH 6/7] ARM: dts: imx6q: add simple-bus compatible string for anatop Dong Aisheng
@ 2012-08-22  7:18 ` Dong Aisheng
  6 siblings, 0 replies; 29+ messages in thread
From: Dong Aisheng @ 2012-08-22  7:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-kernel, linus.walleij, s.hauer, shawn.guo, kernel,
	grant.likely, rob.herring, sameo, lrg, broonie, richard.zhao,
	devicetree-discuss, swarren, paul.liu

From: Dong Aisheng <dong.aisheng@linaro.org>

The anatop registers are accessed via imx syscon now, no one will use
mfd anatop driver anymore, remove it.

Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
---
 drivers/mfd/Kconfig        |    8 ---
 drivers/mfd/Makefile       |    1 -
 drivers/mfd/anatop-mfd.c   |  124 --------------------------------------------
 include/linux/mfd/anatop.h |   40 --------------
 4 files changed, 0 insertions(+), 173 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 20a050e..705a3d0 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -985,14 +985,6 @@ config MFD_STA2X11
 	depends on STA2X11
 	select MFD_CORE
 
-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.
-
 config MFD_IMX_SYSCON
         bool "Freescale i.MX System Controller Register R/W Based on Regmap"
         depends on ARCH_MXC
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 82c7ee1..c352dfe 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -130,6 +130,5 @@ obj-$(CONFIG_MFD_INTEL_MSIC)	+= intel_msic.o
 obj-$(CONFIG_MFD_PALMAS)	+= palmas.o
 obj-$(CONFIG_MFD_RC5T583)	+= rc5t583.o rc5t583-irq.o
 obj-$(CONFIG_MFD_SEC_CORE)	+= sec-core.o sec-irq.o
-obj-$(CONFIG_MFD_ANATOP)	+= anatop-mfd.o
 obj-$(CONFIG_MFD_IMX_SYSCON)	+= imx-syscon.o
 obj-$(CONFIG_MFD_LM3533)	+= lm3533-core.o lm3533-ctrlbank.o
diff --git a/drivers/mfd/anatop-mfd.c b/drivers/mfd/anatop-mfd.c
deleted file mode 100644
index 5576e07..0000000
--- a/drivers/mfd/anatop-mfd.c
+++ /dev/null
@@ -1,124 +0,0 @@
-/*
- * 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_read_reg(struct anatop *adata, u32 addr)
-{
-	return readl(adata->ioreg + addr);
-}
-EXPORT_SYMBOL_GPL(anatop_read_reg);
-
-void anatop_write_reg(struct anatop *adata, u32 addr, u32 data, u32 mask)
-{
-	u32 val;
-
-	data &= mask;
-
-	spin_lock(&adata->reglock);
-	val = readl(adata->ioreg + addr);
-	val &= ~mask;
-	val |= data;
-	writel(val, adata->ioreg + addr);
-	spin_unlock(&adata->reglock);
-}
-EXPORT_SYMBOL_GPL(anatop_write_reg);
-
-static const struct of_device_id of_anatop_match[] = {
-	{ .compatible = "fsl,imx6q-anatop", },
-	{ },
-};
-
-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_populate(np, NULL, NULL, 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 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
deleted file mode 100644
index 7f92acf..0000000
--- a/include/linux/mfd/anatop.h
+++ /dev/null
@@ -1,40 +0,0 @@
-/*
- * 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_read_reg(struct anatop *, u32);
-extern void anatop_write_reg(struct anatop *, u32, u32, u32);
-
-#endif /*  __LINUX_MFD_ANATOP_H */
-- 
1.7.0.4



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH 1/7] mfd: add imx syscon driver based on regmap
  2012-08-22  7:18 ` [PATCH 1/7] mfd: add imx syscon driver based on regmap Dong Aisheng
@ 2012-08-22  8:29   ` Richard Zhao
  2012-08-22 10:57     ` Dong Aisheng
  2012-08-22 16:02   ` Mark Brown
  2012-08-24  6:43   ` Shawn Guo
  2 siblings, 1 reply; 29+ messages in thread
From: Richard Zhao @ 2012-08-22  8:29 UTC (permalink / raw)
  To: Dong Aisheng
  Cc: linux-kernel, linux-arm-kernel, linus.walleij, s.hauer,
	shawn.guo, kernel, grant.likely, rob.herring, sameo, lrg,
	broonie, devicetree-discuss, swarren, paul.liu

On Wed, Aug 22, 2012 at 03:18:42PM +0800, Dong Aisheng wrote:
> From: Dong Aisheng <dong.aisheng@linaro.org>
> 
> Add regmap based imx syscon driver.
> This is usually used for access misc bits in registers which does not belong
> to a specific module, for example, IOMUXC GPR and ANATOP.
> With this driver, we provide a standard API for client driver to call to
> access registers which are registered into syscon.
> 
> Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
> ---
> Currently it's just simply for IMX, however the driver really is not too
> much imx specific.
> If people want, we probably could extend it to support other platforms too.
> ---
>  .../devicetree/bindings/mfd/imx-syscon.txt         |   11 +
>  drivers/mfd/Kconfig                                |    8 +
>  drivers/mfd/Makefile                               |    1 +
>  drivers/mfd/imx-syscon.c                           |  193 ++++++++++++++++++++
>  include/linux/mfd/imx-syscon.h                     |   22 +++
>  5 files changed, 235 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/imx-syscon.txt b/Documentation/devicetree/bindings/mfd/imx-syscon.txt
> new file mode 100644
> index 0000000..4a72994
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/imx-syscon.txt
> @@ -0,0 +1,11 @@
> +* Freescale IMX System Controller Registers R/W driver
> +
> +Required properties:
> +- compatible: Should contain "fsl,imx-syscon".
> +- reg: the register range can be access from imx-syscon
> +
> +Examples:
> +gpr: iomuxc-gpr@020e0000 {
> +	compatible = "fsl,imx6q-iomuxc", "fsl,imx-syscon";
why is it compatible with iomuxc?

> +	reg = <0x020e0000 0x38>;
> +};
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index b1a1462..20a050e 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -993,6 +993,14 @@ config MFD_ANATOP
>  	  MFD controller. This controller embeds regulator and
>  	  thermal devices for Freescale i.MX platforms.
>  
> +config MFD_IMX_SYSCON
> +        bool "Freescale i.MX System Controller Register R/W Based on Regmap"
> +        depends on ARCH_MXC
> +        select REGMAP_MMIO
> +        help
> +          Select this option to enable access Freescale i.MX system control
> +	  registers like iomuxc gpr and anatop via regmap.
> +
>  config MFD_PALMAS
>  	bool "Support for the TI Palmas series chips"
>  	select MFD_CORE
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 79dd22d..82c7ee1 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -131,4 +131,5 @@ obj-$(CONFIG_MFD_PALMAS)	+= palmas.o
>  obj-$(CONFIG_MFD_RC5T583)	+= rc5t583.o rc5t583-irq.o
>  obj-$(CONFIG_MFD_SEC_CORE)	+= sec-core.o sec-irq.o
>  obj-$(CONFIG_MFD_ANATOP)	+= anatop-mfd.o
> +obj-$(CONFIG_MFD_IMX_SYSCON)	+= imx-syscon.o
>  obj-$(CONFIG_MFD_LM3533)	+= lm3533-core.o lm3533-ctrlbank.o
> diff --git a/drivers/mfd/imx-syscon.c b/drivers/mfd/imx-syscon.c
> new file mode 100644
> index 0000000..141b456
> --- /dev/null
> +++ b/drivers/mfd/imx-syscon.c
> @@ -0,0 +1,193 @@
> +/*
> + * Freescale IMX System Control Driver
> + *
> + * Copyright (C) 2012 Freescale Semiconductor, Inc.
> + * Copyright (C) 2012 Linaro Ltd.
> + *
> + * Author: Dong Aisheng <dong.aisheng@linaro.org>
> + *
> + * 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.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +static struct platform_driver imx_syscon_driver;
> +
> +struct imx_syscon {
> +	struct device *dev;
> +	void __iomem *base;
> +	struct regmap *regmap;
> +};
> +
> +static int imx_syscon_match(struct device *dev, void *data)
> +{
> +	struct imx_syscon *syscon = dev_get_drvdata(dev);
> +	struct device_node *dn = data;
> +
> +	return (syscon->dev->of_node == dn) ? 1 : 0;
> +}
> +
> +int imx_syscon_write(struct device_node *np, u32 reg, u32 val)
For API function, is it better to use struct device rather not np?
 - it won't need to search dev in below code every time it access
   registers.
> +{
> +	struct device *dev;
> +	struct imx_syscon *syscon;
> +	int ret = 0;
> +
> +	dev = driver_find_device(&imx_syscon_driver.driver, NULL, np,
> +				 imx_syscon_match);
> +	if (!dev)
> +		return -EPROBE_DEFER;
> +
> +	syscon = dev_get_drvdata(dev);
> +	ret = regmap_write(syscon->regmap, reg, val);
> +	if (ret)
> +		dev_err(dev, "failed to write regmap(%s) reg 0x%x (%d)\n",
> +			np->name, reg, ret);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(imx_syscon_write);
> +
> +int imx_syscon_read(struct device_node *np, u32 reg, u32 *val)
> +{
> +	struct device *dev;
> +	struct imx_syscon *syscon;
> +	int ret = 0;
> +
> +	dev = driver_find_device(&imx_syscon_driver.driver, NULL, np,
> +				 imx_syscon_match);
> +	if (!dev)
> +		return -EPROBE_DEFER;
> +
> +	syscon = dev_get_drvdata(dev);
> +	ret = regmap_read(syscon->regmap, reg, val);
> +	if (ret)
> +		dev_err(dev, "failed to read regmap(%s) reg 0x%x (%d)\n",
> +			np->name, reg, ret);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(imx_syscon_read);
> +
> +int imx_syscon_update_bits(struct device_node *np, u32 reg,
> +					u32 mask, u32 val)
> +{
> +	struct device *dev;
> +	struct imx_syscon *syscon;
> +	int ret = 0;
> +
> +	dev = driver_find_device(&imx_syscon_driver.driver, NULL, np,
> +				 imx_syscon_match);
> +	if (!dev)
> +		return -EPROBE_DEFER;
> +
> +	syscon = dev_get_drvdata(dev);
> +	ret = regmap_update_bits(syscon->regmap, reg, mask, val);
> +	if (ret)
> +		dev_err(dev, "failed to update regmap(%s) reg 0x%x (%d)\n",
> +			np->name, reg, ret);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(imx_syscon_update_bits);
> +
> +static const struct of_device_id of_imx_syscon_match[] = {
> +	{ .compatible = "fsl,imx-syscon", },
> +	{ },
> +};
> +
> +static struct regmap_config imx_syscon_regmap_config = {
> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.reg_stride = 4,
> +};
> +
> +static int __devinit imx_syscon_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *np = dev->of_node;
> +	struct imx_syscon *syscon;
> +	struct resource res;
> +	int ret;
> +
> +	if (!np)
> +		return -ENOENT;
> +
> +	syscon = devm_kzalloc(&pdev->dev, sizeof(struct imx_syscon),
> +			    GFP_KERNEL);
> +	if (!syscon)
> +		return -ENOMEM;
> +
> +	syscon->base = of_iomap(np, 0);
no request? use devm_request_and_ioremap?

Thanks
Richard

> +	if (!syscon->base)
> +		return -EADDRNOTAVAIL;
> +
> +	ret = of_address_to_resource(np, 0, &res);
> +	if (ret)
> +		return ret;
> +
> +	imx_syscon_regmap_config.max_register = res.end - res.start - 3;
> +	syscon->regmap = devm_regmap_init_mmio(&pdev->dev, syscon->base,
> +					&imx_syscon_regmap_config);
> +	if (IS_ERR(syscon->regmap)) {
> +		dev_err(&pdev->dev, "regmap init failed\n");
> +		return PTR_ERR(syscon->regmap);
> +	}
> +
> +	regcache_cache_only(syscon->regmap, false);
> +
> +	dev_info(dev, "syscon regmap start 0x%x end 0x%x registered\n",
> +		res.start, res.end);
> +
> +	syscon->dev = &pdev->dev;
> +	platform_set_drvdata(pdev, syscon);
> +
> +	return 0;
> +}
> +
> +static int __devexit imx_syscon_remove(struct platform_device *pdev)
> +{
> +	struct imx_syscon *syscon;
> +
> +	syscon = platform_get_drvdata(pdev);
> +	iounmap(syscon->base);
> +	platform_set_drvdata(pdev, NULL);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver imx_syscon_driver = {
> +	.driver = {
> +		.name = "imx-syscon",
> +		.owner = THIS_MODULE,
> +		.of_match_table = of_imx_syscon_match,
> +	},
> +	.probe		= imx_syscon_probe,
> +	.remove		= imx_syscon_remove,
> +};
> +
> +static int __init imx_syscon_init(void)
> +{
> +	return platform_driver_register(&imx_syscon_driver);
> +}
> +postcore_initcall(imx_syscon_init);
> +
> +static void __exit anatop_exit(void)
> +{
> +	platform_driver_unregister(&imx_syscon_driver);
> +}
> +module_exit(anatop_exit);
> +
> +MODULE_AUTHOR("Dong Aisheng <dong.aisheng@linaro.org>");
> +MODULE_DESCRIPTION("Freescale IMX System Control driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mfd/imx-syscon.h b/include/linux/mfd/imx-syscon.h
> new file mode 100644
> index 0000000..be8b6db
> --- /dev/null
> +++ b/include/linux/mfd/imx-syscon.h
> @@ -0,0 +1,22 @@
> +/*
> + * Freescale IMX System Control Driver
> + *
> + * Copyright (C) 2012 Freescale Semiconductor, Inc.
> + * Copyright (C) 2012 Linaro Ltd.
> + *
> + * Author: Dong Aisheng <dong.aisheng@linaro.org>
> + *
> + * 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.
> + */
> +
> +#ifndef __LINUX_MFD_IMX_SYSCON_H__
> +#define __LINUX_MFD_IMX_SYSCON_H__
> +
> +extern int imx_syscon_write(struct device_node *np, u32 reg, u32 val);
> +extern int imx_syscon_read(struct device_node *np, u32 reg, u32 *val);
> +extern int imx_syscon_update_bits(struct device_node *np, u32 reg,
> +					u32 mask, u32 val);
> +#endif /* __LINUX_MFD_IMX_SYSCON_H__ */
> -- 
> 1.7.0.4
> 


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 6/7] ARM: dts: imx6q: add simple-bus compatible string for anatop
  2012-08-22  7:18 ` [PATCH 6/7] ARM: dts: imx6q: add simple-bus compatible string for anatop Dong Aisheng
@ 2012-08-22  8:52   ` Richard Zhao
  2012-08-22 11:02     ` Dong Aisheng
  0 siblings, 1 reply; 29+ messages in thread
From: Richard Zhao @ 2012-08-22  8:52 UTC (permalink / raw)
  To: Dong Aisheng
  Cc: linux-kernel, linux-arm-kernel, linus.walleij, s.hauer,
	shawn.guo, kernel, grant.likely, rob.herring, sameo, lrg,
	broonie, devicetree-discuss, swarren, paul.liu

On Wed, Aug 22, 2012 at 03:18:47PM +0800, Dong Aisheng wrote:
> From: Dong Aisheng <dong.aisheng@linaro.org>
> 
> Originally the anatop regulator devices are populated by mfd anatop driver.
> Since mfd anatop driver will be deleted later, we change to populate the
> regulator devices by devicetree automatically.
> This will cause some warning messages as follows during boot due to device
> recreation: "vdd1p1: Failed to create debugfs directory"
> But it does not break any function.
> Later, we will remove mfd anatop driver which can get rid of this
> error message.
> 
> Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
> ---
>  arch/arm/boot/dts/imx6q.dtsi |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
> index 7076be0..426f735 100644
> --- a/arch/arm/boot/dts/imx6q.dtsi
> +++ b/arch/arm/boot/dts/imx6q.dtsi
> @@ -379,7 +379,7 @@
>  			};
>  
>  			anatop: anatop@020c8000 {
> -				compatible = "fsl,imx6q-anatop", "fsl,imx-syscon";
> +				compatible = "fsl,imx6q-anatop", "fsl,imx-syscon", "simple-bus";
To prevent bisect break, it should merge with patch #4.
It's really strange to use simple-bus, because it's not a bus.
I like more the way how anatop driver handle it. Anatop driver populate
devices in its code.

Thanks
Richard
>  				reg = <0x020c8000 0x1000>;
>  				interrupts = <0 49 0x04 0 54 0x04 0 127 0x04>;
>  
> -- 
> 1.7.0.4
> 


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 1/7] mfd: add imx syscon driver based on regmap
  2012-08-22  8:29   ` Richard Zhao
@ 2012-08-22 10:57     ` Dong Aisheng
  2012-08-23  5:16       ` Stephen Warren
  0 siblings, 1 reply; 29+ messages in thread
From: Dong Aisheng @ 2012-08-22 10:57 UTC (permalink / raw)
  To: Zhao Richard-B20223
  Cc: linux-kernel, linux-arm-kernel, linus.walleij, s.hauer,
	shawn.guo, kernel, grant.likely, rob.herring, sameo, lrg,
	broonie, devicetree-discuss, swarren, paul.liu

On Wed, Aug 22, 2012 at 04:29:41PM +0800, Zhao Richard-B20223 wrote:
> On Wed, Aug 22, 2012 at 03:18:42PM +0800, Dong Aisheng wrote:
> > From: Dong Aisheng <dong.aisheng@linaro.org>
> > 
> > Add regmap based imx syscon driver.
> > This is usually used for access misc bits in registers which does not belong
> > to a specific module, for example, IOMUXC GPR and ANATOP.
> > With this driver, we provide a standard API for client driver to call to
> > access registers which are registered into syscon.
> > 
> > Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
> > ---
> > Currently it's just simply for IMX, however the driver really is not too
> > much imx specific.
> > If people want, we probably could extend it to support other platforms too.
> > ---
> >  .../devicetree/bindings/mfd/imx-syscon.txt         |   11 +
> >  drivers/mfd/Kconfig                                |    8 +
> >  drivers/mfd/Makefile                               |    1 +
> >  drivers/mfd/imx-syscon.c                           |  193 ++++++++++++++++++++
> >  include/linux/mfd/imx-syscon.h                     |   22 +++
> >  5 files changed, 235 insertions(+), 0 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/imx-syscon.txt b/Documentation/devicetree/bindings/mfd/imx-syscon.txt
> > new file mode 100644
> > index 0000000..4a72994
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/imx-syscon.txt
> > @@ -0,0 +1,11 @@
> > +* Freescale IMX System Controller Registers R/W driver
> > +
> > +Required properties:
> > +- compatible: Should contain "fsl,imx-syscon".
> > +- reg: the register range can be access from imx-syscon
> > +
> > +Examples:
> > +gpr: iomuxc-gpr@020e0000 {
> > +	compatible = "fsl,imx6q-iomuxc", "fsl,imx-syscon";
> why is it compatible with iomuxc?
> 
The first one usually is describing the device itself,
the second one is the required compatible string for using imx-sycon.

btw, it looks i'd better change "fsl,imx6q-iomuxc" to "fsl,imx6q-iomuxc-gpr"
since the later is the real case for us which is in patch 2/7.

> > +static int imx_syscon_match(struct device *dev, void *data)
> > +{
> > +	struct imx_syscon *syscon = dev_get_drvdata(dev);
> > +	struct device_node *dn = data;
> > +
> > +	return (syscon->dev->of_node == dn) ? 1 : 0;
> > +}
> > +
> > +int imx_syscon_write(struct device_node *np, u32 reg, u32 val)
> For API function, is it better to use struct device rather not np?
>  - it won't need to search dev in below code every time it access
>    registers.
The purpose is not require client driver to know the implementation details
of imx_syscon_{read/write} API, it's more easy to use since client only
needs pass the device node to which it wants to read/write.

For search dev, it doesn't look like a big issue since it only search devices
attached on the driver which is very quick.
And hide it in common API does not require every client driver to write
duplicated codes.

> > +static int __devinit imx_syscon_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct device_node *np = dev->of_node;
> > +	struct imx_syscon *syscon;
> > +	struct resource res;
> > +	int ret;
> > +
> > +	if (!np)
> > +		return -ENOENT;
> > +
> > +	syscon = devm_kzalloc(&pdev->dev, sizeof(struct imx_syscon),
> > +			    GFP_KERNEL);
> > +	if (!syscon)
> > +		return -ENOMEM;
> > +
> > +	syscon->base = of_iomap(np, 0);
> no request? use devm_request_and_ioremap?
> 
The io space registered into imx-sycon may be overlapped with other device,
e.g. iomuxc gpr overlapped with iomuxc. So we do not request it here.
There are also some exist examples, imx28 pinctrl with gpio devices contained.

Regards
Dong Aisheng


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 6/7] ARM: dts: imx6q: add simple-bus compatible string for anatop
  2012-08-22  8:52   ` Richard Zhao
@ 2012-08-22 11:02     ` Dong Aisheng
  0 siblings, 0 replies; 29+ messages in thread
From: Dong Aisheng @ 2012-08-22 11:02 UTC (permalink / raw)
  To: Zhao Richard-B20223
  Cc: linux-kernel, linux-arm-kernel, linus.walleij, s.hauer,
	shawn.guo, kernel, grant.likely, rob.herring, sameo, lrg,
	broonie, devicetree-discuss, swarren, paul.liu

On Wed, Aug 22, 2012 at 04:52:36PM +0800, Zhao Richard-B20223 wrote:
> On Wed, Aug 22, 2012 at 03:18:47PM +0800, Dong Aisheng wrote:
> > From: Dong Aisheng <dong.aisheng@linaro.org>
> > 
> > Originally the anatop regulator devices are populated by mfd anatop driver.
> > Since mfd anatop driver will be deleted later, we change to populate the
> > regulator devices by devicetree automatically.
> > This will cause some warning messages as follows during boot due to device
> > recreation: "vdd1p1: Failed to create debugfs directory"
> > But it does not break any function.
> > Later, we will remove mfd anatop driver which can get rid of this
> > error message.
> > 
> > Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
> > ---
> >  arch/arm/boot/dts/imx6q.dtsi |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
> > index 7076be0..426f735 100644
> > --- a/arch/arm/boot/dts/imx6q.dtsi
> > +++ b/arch/arm/boot/dts/imx6q.dtsi
> > @@ -379,7 +379,7 @@
> >  			};
> >  
> >  			anatop: anatop@020c8000 {
> > -				compatible = "fsl,imx6q-anatop", "fsl,imx-syscon";
> > +				compatible = "fsl,imx6q-anatop", "fsl,imx-syscon", "simple-bus";
> To prevent bisect break, it should merge with patch #4.
Yes, i will try it and merge them if needed.

> It's really strange to use simple-bus, because it's not a bus.
I can't say it's strange or not.
There are existing using examples, imx28.dtsi.

> I like more the way how anatop driver handle it. Anatop driver populate
> devices in its code.
The anatop mfd driver will be deleted later.
So the proper solution may be generating regulator devices automatically when call
of_platform_populate in mach code rather than populate it in driver itself.

Regards
Dong Aisheng


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 4/7] regulator: anatop-regulator: convert to use imx-syscon to access anatop register
  2012-08-22  7:18 ` [PATCH 4/7] regulator: anatop-regulator: convert to use imx-syscon to access anatop register Dong Aisheng
@ 2012-08-22 15:59   ` Mark Brown
  2012-08-23  7:15     ` Dong Aisheng
  2012-08-23  5:21   ` Stephen Warren
  1 sibling, 1 reply; 29+ messages in thread
From: Mark Brown @ 2012-08-22 15:59 UTC (permalink / raw)
  To: Dong Aisheng
  Cc: linux-kernel, linux-arm-kernel, linus.walleij, s.hauer,
	shawn.guo, kernel, grant.likely, rob.herring, sameo, lrg,
	richard.zhao, devicetree-discuss, swarren, paul.liu

[-- Attachment #1: Type: text/plain, Size: 394 bytes --]

On Wed, Aug 22, 2012 at 03:18:45PM +0800, Dong Aisheng wrote:
> From: Dong Aisheng <dong.aisheng@linaro.org>
> 
> Using standard imx syscon API to access anatop register.

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

With the conversion to regmap it'd also be good to convert the driver to
use the regmap helper functions for enable and voltage operations if
possible.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 1/7] mfd: add imx syscon driver based on regmap
  2012-08-22  7:18 ` [PATCH 1/7] mfd: add imx syscon driver based on regmap Dong Aisheng
  2012-08-22  8:29   ` Richard Zhao
@ 2012-08-22 16:02   ` Mark Brown
  2012-08-23  7:26     ` Dong Aisheng
  2012-08-24  6:43   ` Shawn Guo
  2 siblings, 1 reply; 29+ messages in thread
From: Mark Brown @ 2012-08-22 16:02 UTC (permalink / raw)
  To: Dong Aisheng
  Cc: linux-kernel, linux-arm-kernel, linus.walleij, s.hauer,
	shawn.guo, kernel, grant.likely, rob.herring, sameo, lrg,
	richard.zhao, devicetree-discuss, swarren, paul.liu

[-- Attachment #1: Type: text/plain, Size: 890 bytes --]

On Wed, Aug 22, 2012 at 03:18:42PM +0800, Dong Aisheng wrote:

> From: Dong Aisheng <dong.aisheng@linaro.org>

> Add regmap based imx syscon driver.

Nice to see more regmap-mmio usage!

Reviwed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

from a regmap point of view.

> +int imx_syscon_write(struct device_node *np, u32 reg, u32 val)
> +{
> +	struct device *dev;
> +	struct imx_syscon *syscon;
> +	int ret = 0;
> +
> +	dev = driver_find_device(&imx_syscon_driver.driver, NULL, np,
> +				 imx_syscon_match);
> +	if (!dev)
> +		return -EPROBE_DEFER;
> +
> +	syscon = dev_get_drvdata(dev);
> +	ret = regmap_write(syscon->regmap, reg, val);

It'd be good to provide a way of retrieving the regmap so that drivers
for subsystems with generic regmap code could use the framework features
(regulator is one example that I just mentioned in my other mail).

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 1/7] mfd: add imx syscon driver based on regmap
  2012-08-22 10:57     ` Dong Aisheng
@ 2012-08-23  5:16       ` Stephen Warren
  2012-08-23  6:09         ` Richard Zhao
  2012-08-23  7:06         ` Dong Aisheng
  0 siblings, 2 replies; 29+ messages in thread
From: Stephen Warren @ 2012-08-23  5:16 UTC (permalink / raw)
  To: Dong Aisheng
  Cc: Zhao Richard-B20223, linux-kernel, linux-arm-kernel,
	linus.walleij, s.hauer, shawn.guo, kernel, grant.likely,
	rob.herring, sameo, lrg, broonie, devicetree-discuss, paul.liu

On 08/22/2012 04:57 AM, Dong Aisheng wrote:
> On Wed, Aug 22, 2012 at 04:29:41PM +0800, Zhao Richard-B20223 wrote:
>> On Wed, Aug 22, 2012 at 03:18:42PM +0800, Dong Aisheng wrote:
>>> Add regmap based imx syscon driver.
>>> This is usually used for access misc bits in registers which does not belong
>>> to a specific module, for example, IOMUXC GPR and ANATOP.
>>> With this driver, we provide a standard API for client driver to call to
>>> access registers which are registered into syscon.

>>> +static int imx_syscon_match(struct device *dev, void *data)
>>> +{
>>> +	struct imx_syscon *syscon = dev_get_drvdata(dev);
>>> +	struct device_node *dn = data;
>>> +
>>> +	return (syscon->dev->of_node == dn) ? 1 : 0;
>>> +}
>>> +
>>> +int imx_syscon_write(struct device_node *np, u32 reg, u32 val)
>>
>> For API function, is it better to use struct device rather not np?
>>  - it won't need to search dev in below code every time it access
>>    registers.
>
> The purpose is not require client driver to know the implementation details
> of imx_syscon_{read/write} API, it's more easy to use since client only
> needs pass the device node to which it wants to read/write.
> 
> For search dev, it doesn't look like a big issue since it only search devices
> attached on the driver which is very quick.
> And hide it in common API does not require every client driver to write
> duplicated codes.

You could still implement a function:

struct device *imx_syscon_lookup(struct device_node *np)

... and require all clients to call that, and pass the dev to the other
functions. That'd still keep all the lookup code in one place, but
prevent it having to run every time, no matter how small it is.

I think such an API is required anyway, since client drivers need some
way to determine whether the imx_syscon driver is available yet, and if
not defer their probe until it is.

So, clients would do:

foo->syscon_dev = imx_syscon_lookup(np);
if (!foo->syscon_dev)
    return -EPROBE_DEFER;

rather than:

foo->syscon_np = np;

Not too much overhead/boiler-plate in each client driver.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 4/7] regulator: anatop-regulator: convert to use imx-syscon to access anatop register
  2012-08-22  7:18 ` [PATCH 4/7] regulator: anatop-regulator: convert to use imx-syscon to access anatop register Dong Aisheng
  2012-08-22 15:59   ` Mark Brown
@ 2012-08-23  5:21   ` Stephen Warren
  2012-08-23  6:12     ` Richard Zhao
  2012-08-23  7:32     ` Dong Aisheng
  1 sibling, 2 replies; 29+ messages in thread
From: Stephen Warren @ 2012-08-23  5:21 UTC (permalink / raw)
  To: Dong Aisheng
  Cc: linux-kernel, linux-arm-kernel, linus.walleij, s.hauer,
	shawn.guo, kernel, grant.likely, rob.herring, sameo, lrg,
	broonie, richard.zhao, devicetree-discuss, paul.liu

On 08/22/2012 01:18 AM, Dong Aisheng wrote:
> Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>

> diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c

> @@ -109,7 +110,11 @@ static int __devinit anatop_regulator_probe(struct platform_device *pdev)
>  	rdesc->ops = &anatop_rops;
>  	rdesc->type = REGULATOR_VOLTAGE;
>  	rdesc->owner = THIS_MODULE;
> -	sreg->mfd = anatopmfd;
> +
> +	sreg->anatop = of_parse_phandle(np, "fsl,anatop", 0);
> +	if (!sreg->anatop)
> +		return -ENODEV;

In fact, that imx_syscon_lookup function I proposed could even do the
of_parse_phandle() internally, so perhaps:

foo->syscon_dev = imx_syscon_lookup(np, "fsl,anatop", 0);
if (IS_ERR(foo->syscon_dev))
    return PTR_ERR(foo->syscon_dev);

with imx_syscon_lookup() internally knowing when to return EPROBE_DEFER
rather than any other permanent error code (e.g. for missing property,
bad phandle, etc.)

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 1/7] mfd: add imx syscon driver based on regmap
  2012-08-23  5:16       ` Stephen Warren
@ 2012-08-23  6:09         ` Richard Zhao
  2012-08-23  7:06         ` Dong Aisheng
  1 sibling, 0 replies; 29+ messages in thread
From: Richard Zhao @ 2012-08-23  6:09 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Dong Aisheng, Zhao Richard-B20223, linux-kernel,
	linux-arm-kernel, linus.walleij, s.hauer, shawn.guo, kernel,
	grant.likely, rob.herring, sameo, lrg, broonie,
	devicetree-discuss, paul.liu

On Wed, Aug 22, 2012 at 11:16:33PM -0600, Stephen Warren wrote:
> On 08/22/2012 04:57 AM, Dong Aisheng wrote:
> > On Wed, Aug 22, 2012 at 04:29:41PM +0800, Zhao Richard-B20223 wrote:
> >> On Wed, Aug 22, 2012 at 03:18:42PM +0800, Dong Aisheng wrote:
> >>> Add regmap based imx syscon driver.
> >>> This is usually used for access misc bits in registers which does not belong
> >>> to a specific module, for example, IOMUXC GPR and ANATOP.
> >>> With this driver, we provide a standard API for client driver to call to
> >>> access registers which are registered into syscon.
> 
> >>> +static int imx_syscon_match(struct device *dev, void *data)
> >>> +{
> >>> +	struct imx_syscon *syscon = dev_get_drvdata(dev);
> >>> +	struct device_node *dn = data;
> >>> +
> >>> +	return (syscon->dev->of_node == dn) ? 1 : 0;
> >>> +}
> >>> +
> >>> +int imx_syscon_write(struct device_node *np, u32 reg, u32 val)
> >>
> >> For API function, is it better to use struct device rather not np?
> >>  - it won't need to search dev in below code every time it access
> >>    registers.
> >
> > The purpose is not require client driver to know the implementation details
> > of imx_syscon_{read/write} API, it's more easy to use since client only
> > needs pass the device node to which it wants to read/write.
> > 
> > For search dev, it doesn't look like a big issue since it only search devices
> > attached on the driver which is very quick.
> > And hide it in common API does not require every client driver to write
> > duplicated codes.
> 
> You could still implement a function:
> 
> struct device *imx_syscon_lookup(struct device_node *np)
> 
> ... and require all clients to call that, and pass the dev to the other
> functions. That'd still keep all the lookup code in one place, but
> prevent it having to run every time, no matter how small it is.
> 
> I think such an API is required anyway, since client drivers need some
> way to determine whether the imx_syscon driver is available yet, and if
> not defer their probe until it is.
> 
> So, clients would do:
> 
> foo->syscon_dev = imx_syscon_lookup(np);
> if (!foo->syscon_dev)
>     return -EPROBE_DEFER;
> 
> rather than:
> 
> foo->syscon_np = np;
> 
> Not too much overhead/boiler-plate in each client driver.
I think it's good idea.

Thanks
Richard


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 4/7] regulator: anatop-regulator: convert to use imx-syscon to access anatop register
  2012-08-23  5:21   ` Stephen Warren
@ 2012-08-23  6:12     ` Richard Zhao
  2012-08-23 17:56       ` Stephen Warren
  2012-08-23  7:32     ` Dong Aisheng
  1 sibling, 1 reply; 29+ messages in thread
From: Richard Zhao @ 2012-08-23  6:12 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Dong Aisheng, linux-kernel, linux-arm-kernel, linus.walleij,
	s.hauer, shawn.guo, kernel, grant.likely, rob.herring, sameo,
	lrg, broonie, devicetree-discuss, paul.liu

On Wed, Aug 22, 2012 at 11:21:03PM -0600, Stephen Warren wrote:
> On 08/22/2012 01:18 AM, Dong Aisheng wrote:
> > Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
> 
> > diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
> 
> > @@ -109,7 +110,11 @@ static int __devinit anatop_regulator_probe(struct platform_device *pdev)
> >  	rdesc->ops = &anatop_rops;
> >  	rdesc->type = REGULATOR_VOLTAGE;
> >  	rdesc->owner = THIS_MODULE;
> > -	sreg->mfd = anatopmfd;
> > +
> > +	sreg->anatop = of_parse_phandle(np, "fsl,anatop", 0);
> > +	if (!sreg->anatop)
> > +		return -ENODEV;
> 
> In fact, that imx_syscon_lookup function I proposed could even do the
> of_parse_phandle() internally, so perhaps:
> 
> foo->syscon_dev = imx_syscon_lookup(np, "fsl,anatop", 0);
> if (IS_ERR(foo->syscon_dev))
>     return PTR_ERR(foo->syscon_dev);
> 
> with imx_syscon_lookup() internally knowing when to return EPROBE_DEFER
> rather than any other permanent error code (e.g. for missing property,
> bad phandle, etc.)
In some case that we access register in machine code, we don't have any
phandler. The node is got by find compatible or by path.

Thanks
Richard 


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 1/7] mfd: add imx syscon driver based on regmap
  2012-08-23  5:16       ` Stephen Warren
  2012-08-23  6:09         ` Richard Zhao
@ 2012-08-23  7:06         ` Dong Aisheng
  1 sibling, 0 replies; 29+ messages in thread
From: Dong Aisheng @ 2012-08-23  7:06 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Zhao Richard-B20223, linux-kernel, linux-arm-kernel,
	linus.walleij, s.hauer, shawn.guo, kernel, grant.likely,
	rob.herring, sameo, lrg, broonie, devicetree-discuss, paul.liu

On Thu, Aug 23, 2012 at 01:16:33PM +0800, Stephen Warren wrote:
> On 08/22/2012 04:57 AM, Dong Aisheng wrote:
> > On Wed, Aug 22, 2012 at 04:29:41PM +0800, Zhao Richard-B20223 wrote:
> >> On Wed, Aug 22, 2012 at 03:18:42PM +0800, Dong Aisheng wrote:
> >>> Add regmap based imx syscon driver.
> >>> This is usually used for access misc bits in registers which does not belong
> >>> to a specific module, for example, IOMUXC GPR and ANATOP.
> >>> With this driver, we provide a standard API for client driver to call to
> >>> access registers which are registered into syscon.
> 
> >>> +static int imx_syscon_match(struct device *dev, void *data)
> >>> +{
> >>> +	struct imx_syscon *syscon = dev_get_drvdata(dev);
> >>> +	struct device_node *dn = data;
> >>> +
> >>> +	return (syscon->dev->of_node == dn) ? 1 : 0;
> >>> +}
> >>> +
> >>> +int imx_syscon_write(struct device_node *np, u32 reg, u32 val)
> >>
> >> For API function, is it better to use struct device rather not np?
> >>  - it won't need to search dev in below code every time it access
> >>    registers.
> >
> > The purpose is not require client driver to know the implementation details
> > of imx_syscon_{read/write} API, it's more easy to use since client only
> > needs pass the device node to which it wants to read/write.
> > 
> > For search dev, it doesn't look like a big issue since it only search devices
> > attached on the driver which is very quick.
> > And hide it in common API does not require every client driver to write
> > duplicated codes.
> 
> You could still implement a function:
> 
> struct device *imx_syscon_lookup(struct device_node *np)
> 
> ... and require all clients to call that, and pass the dev to the other
> functions. That'd still keep all the lookup code in one place, but
> prevent it having to run every time, no matter how small it is.
> 
> I think such an API is required anyway, since client drivers need some
> way to determine whether the imx_syscon driver is available yet, and if
> not defer their probe until it is.
> 
> So, clients would do:
> 
> foo->syscon_dev = imx_syscon_lookup(np);
> if (!foo->syscon_dev)
>     return -EPROBE_DEFER;
> 
> rather than:
> 
> foo->syscon_np = np;
> 
> Not too much overhead/boiler-plate in each client driver.
> 
Looks like a good idea.

Regards
Dong Aisheng


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 4/7] regulator: anatop-regulator: convert to use imx-syscon to access anatop register
  2012-08-22 15:59   ` Mark Brown
@ 2012-08-23  7:15     ` Dong Aisheng
  2012-08-23 11:17       ` Mark Brown
  0 siblings, 1 reply; 29+ messages in thread
From: Dong Aisheng @ 2012-08-23  7:15 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-kernel, linux-arm-kernel, linus.walleij, s.hauer,
	shawn.guo, kernel, grant.likely, rob.herring, sameo, lrg,
	Zhao Richard-B20223, devicetree-discuss, swarren, paul.liu

On Wed, Aug 22, 2012 at 11:59:53PM +0800, Mark Brown wrote:
> On Wed, Aug 22, 2012 at 03:18:45PM +0800, Dong Aisheng wrote:
> > From: Dong Aisheng <dong.aisheng@linaro.org>
> > 
> > Using standard imx syscon API to access anatop register.
> 
> Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> 
Thanks

> With the conversion to regmap it'd also be good to convert the driver to
> use the regmap helper functions for enable and voltage operations if
> possible.

The anatop-regulator driver only implements set_voltage_sel/get_voltage_sel
which i have already converted, what do you mean others like
'for enable and voltage operations' i should also convert?

Regards
Dong Aisheng


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 1/7] mfd: add imx syscon driver based on regmap
  2012-08-22 16:02   ` Mark Brown
@ 2012-08-23  7:26     ` Dong Aisheng
  2012-08-23 11:06       ` Mark Brown
  0 siblings, 1 reply; 29+ messages in thread
From: Dong Aisheng @ 2012-08-23  7:26 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-kernel, linux-arm-kernel, linus.walleij, s.hauer,
	shawn.guo, kernel, grant.likely, rob.herring, sameo, lrg,
	Zhao Richard-B20223, devicetree-discuss, swarren, paul.liu

On Thu, Aug 23, 2012 at 12:02:41AM +0800, Mark Brown wrote:
> On Wed, Aug 22, 2012 at 03:18:42PM +0800, Dong Aisheng wrote:
> 
> > From: Dong Aisheng <dong.aisheng@linaro.org>
> 
> > Add regmap based imx syscon driver.
> 
> Nice to see more regmap-mmio usage!
> 
> Reviwed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> 
> from a regmap point of view.
> 
Thanks

> > +int imx_syscon_write(struct device_node *np, u32 reg, u32 val)
> > +{
> > +	struct device *dev;
> > +	struct imx_syscon *syscon;
> > +	int ret = 0;
> > +
> > +	dev = driver_find_device(&imx_syscon_driver.driver, NULL, np,
> > +				 imx_syscon_match);
> > +	if (!dev)
> > +		return -EPROBE_DEFER;
> > +
> > +	syscon = dev_get_drvdata(dev);
> > +	ret = regmap_write(syscon->regmap, reg, val);
> 
> It'd be good to provide a way of retrieving the regmap so that drivers
> for subsystems with generic regmap code could use the framework features
> (regulator is one example that I just mentioned in my other mail).

Do you mean something like:
regmap = syscon_regmap_dev_lookup(np, "fsl,anatop");
regmap_write(regmap, reg, val);

Then drivers can use generic regmap framework features rather than depend
on what imx-syscon implemented, is that correct?

Regards
Dong Aisheng


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 4/7] regulator: anatop-regulator: convert to use imx-syscon to access anatop register
  2012-08-23  5:21   ` Stephen Warren
  2012-08-23  6:12     ` Richard Zhao
@ 2012-08-23  7:32     ` Dong Aisheng
  1 sibling, 0 replies; 29+ messages in thread
From: Dong Aisheng @ 2012-08-23  7:32 UTC (permalink / raw)
  To: Stephen Warren
  Cc: linux-kernel, linux-arm-kernel, linus.walleij, s.hauer,
	shawn.guo, kernel, grant.likely, rob.herring, sameo, lrg,
	broonie, Zhao Richard-B20223, devicetree-discuss, paul.liu

On Thu, Aug 23, 2012 at 01:21:03PM +0800, Stephen Warren wrote:
> On 08/22/2012 01:18 AM, Dong Aisheng wrote:
> > Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
> 
> > diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
> 
> > @@ -109,7 +110,11 @@ static int __devinit anatop_regulator_probe(struct platform_device *pdev)
> >  	rdesc->ops = &anatop_rops;
> >  	rdesc->type = REGULATOR_VOLTAGE;
> >  	rdesc->owner = THIS_MODULE;
> > -	sreg->mfd = anatopmfd;
> > +
> > +	sreg->anatop = of_parse_phandle(np, "fsl,anatop", 0);
> > +	if (!sreg->anatop)
> > +		return -ENODEV;
> 
> In fact, that imx_syscon_lookup function I proposed could even do the
> of_parse_phandle() internally, so perhaps:
> 
> foo->syscon_dev = imx_syscon_lookup(np, "fsl,anatop", 0);
> if (IS_ERR(foo->syscon_dev))
>     return PTR_ERR(foo->syscon_dev);
> 
> with imx_syscon_lookup() internally knowing when to return EPROBE_DEFER
> rather than any other permanent error code (e.g. for missing property,
> bad phandle, etc.)
> 
This also looks reasonable to me.
btw, see my last reply to mark in another mail.
I'm not sure but Mark may want something slightly different as this one,
imx_syscon_lookup directly return regmap rather than dev, then we do not
need implement any register read/write API in imx-syscon driver, just
using the exist regmap r/w API is ok.

Regards
Dong Aisheng


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 1/7] mfd: add imx syscon driver based on regmap
  2012-08-23  7:26     ` Dong Aisheng
@ 2012-08-23 11:06       ` Mark Brown
  2012-08-24  2:28         ` Dong Aisheng
  0 siblings, 1 reply; 29+ messages in thread
From: Mark Brown @ 2012-08-23 11:06 UTC (permalink / raw)
  To: Dong Aisheng
  Cc: linux-kernel, linux-arm-kernel, linus.walleij, s.hauer,
	shawn.guo, kernel, grant.likely, rob.herring, sameo, lrg,
	Zhao Richard-B20223, devicetree-discuss, swarren, paul.liu

[-- Attachment #1: Type: text/plain, Size: 695 bytes --]

On Thu, Aug 23, 2012 at 03:26:30PM +0800, Dong Aisheng wrote:
> On Thu, Aug 23, 2012 at 12:02:41AM +0800, Mark Brown wrote:

> > It'd be good to provide a way of retrieving the regmap so that drivers
> > for subsystems with generic regmap code could use the framework features
> > (regulator is one example that I just mentioned in my other mail).

> Do you mean something like:
> regmap = syscon_regmap_dev_lookup(np, "fsl,anatop");
> regmap_write(regmap, reg, val);

> Then drivers can use generic regmap framework features rather than depend
> on what imx-syscon implemented, is that correct?

Yes, this is mainly for cases where the subsystem has helper functions
that can work with regmap.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 4/7] regulator: anatop-regulator: convert to use imx-syscon to access anatop register
  2012-08-23  7:15     ` Dong Aisheng
@ 2012-08-23 11:17       ` Mark Brown
  2012-08-24  2:29         ` Dong Aisheng
  0 siblings, 1 reply; 29+ messages in thread
From: Mark Brown @ 2012-08-23 11:17 UTC (permalink / raw)
  To: Dong Aisheng
  Cc: linux-kernel, linux-arm-kernel, linus.walleij, s.hauer,
	shawn.guo, kernel, grant.likely, rob.herring, sameo, lrg,
	Zhao Richard-B20223, devicetree-discuss, swarren, paul.liu

[-- Attachment #1: Type: text/plain, Size: 649 bytes --]

On Thu, Aug 23, 2012 at 03:15:04PM +0800, Dong Aisheng wrote:
> On Wed, Aug 22, 2012 at 11:59:53PM +0800, Mark Brown wrote:

> > With the conversion to regmap it'd also be good to convert the driver to
> > use the regmap helper functions for enable and voltage operations if
> > possible.

> The anatop-regulator driver only implements set_voltage_sel/get_voltage_sel
> which i have already converted, what do you mean others like
> 'for enable and voltage operations' i should also convert?

Those operations should ideally be converted to use the generic regmap
implementation now the device uses regmap - regmap_get_voltage_sel_regmap
and so on.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 4/7] regulator: anatop-regulator: convert to use imx-syscon to access anatop register
  2012-08-23  6:12     ` Richard Zhao
@ 2012-08-23 17:56       ` Stephen Warren
  2012-08-24  2:37         ` Dong Aisheng
  0 siblings, 1 reply; 29+ messages in thread
From: Stephen Warren @ 2012-08-23 17:56 UTC (permalink / raw)
  To: Richard Zhao
  Cc: Dong Aisheng, linux-kernel, linux-arm-kernel, linus.walleij,
	s.hauer, shawn.guo, kernel, grant.likely, rob.herring, sameo,
	lrg, broonie, devicetree-discuss, paul.liu

On 08/23/2012 12:12 AM, Richard Zhao wrote:
> On Wed, Aug 22, 2012 at 11:21:03PM -0600, Stephen Warren wrote:
>> On 08/22/2012 01:18 AM, Dong Aisheng wrote:
>>> Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
>>
>>> diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
>>
>>> @@ -109,7 +110,11 @@ static int __devinit anatop_regulator_probe(struct platform_device *pdev)
>>>  	rdesc->ops = &anatop_rops;
>>>  	rdesc->type = REGULATOR_VOLTAGE;
>>>  	rdesc->owner = THIS_MODULE;
>>> -	sreg->mfd = anatopmfd;
>>> +
>>> +	sreg->anatop = of_parse_phandle(np, "fsl,anatop", 0);
>>> +	if (!sreg->anatop)
>>> +		return -ENODEV;
>>
>> In fact, that imx_syscon_lookup function I proposed could even do the
>> of_parse_phandle() internally, so perhaps:
>>
>> foo->syscon_dev = imx_syscon_lookup(np, "fsl,anatop", 0);
>> if (IS_ERR(foo->syscon_dev))
>>     return PTR_ERR(foo->syscon_dev);
>>
>> with imx_syscon_lookup() internally knowing when to return EPROBE_DEFER
>> rather than any other permanent error code (e.g. for missing property,
>> bad phandle, etc.)
>
> In some case that we access register in machine code, we don't have any
> phandle. The node is got by find compatible or by path.

That sounds a little odd; why not just use a phandle consistently
everywhere?

Either way though, I could imagine still putting all the lookup code
into the syscon driver; just have different functions for the different
lookup methods:

imx_syscon_lookup_by_phandle(np, char *property_name)
imx_syscon_lookup_by_compatible(char *compatible
imx_syscon_lookup_by_path(char *node_path)

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 1/7] mfd: add imx syscon driver based on regmap
  2012-08-23 11:06       ` Mark Brown
@ 2012-08-24  2:28         ` Dong Aisheng
  0 siblings, 0 replies; 29+ messages in thread
From: Dong Aisheng @ 2012-08-24  2:28 UTC (permalink / raw)
  To: Mark Brown, linus.walleij
  Cc: linux-kernel, linux-arm-kernel, linus.walleij, s.hauer,
	shawn.guo, kernel, grant.likely, rob.herring, sameo, lrg,
	Zhao Richard-B20223, devicetree-discuss, swarren

On Thu, Aug 23, 2012 at 07:06:47PM +0800, Mark Brown wrote:
> On Thu, Aug 23, 2012 at 03:26:30PM +0800, Dong Aisheng wrote:
> > On Thu, Aug 23, 2012 at 12:02:41AM +0800, Mark Brown wrote:
> 
> > > It'd be good to provide a way of retrieving the regmap so that drivers
> > > for subsystems with generic regmap code could use the framework features
> > > (regulator is one example that I just mentioned in my other mail).
> 
> > Do you mean something like:
> > regmap = syscon_regmap_dev_lookup(np, "fsl,anatop");
> > regmap_write(regmap, reg, val);
> 
> > Then drivers can use generic regmap framework features rather than depend
> > on what imx-syscon implemented, is that correct?
> 
> Yes, this is mainly for cases where the subsystem has helper functions
> that can work with regmap.

Okay, then imx-syscon only implements regmap register mechanism and regmap
lookup mechanism, for accessors, client driver can directly use the generic
regmap API defined in include/linux/regmap.h.

Then it looks to me the driver is more like a generic feature which may also be
needed for other SoCs, IIRC, Tegra ahb and ux500 PRCMU,

Do you think if we should implement it in a more generic way at first?
e.g, drop 'imx-' prefix first.

Linus,
You're the first guy to raise the idea that we could implement a syscon
framework to generic register access, what's your comment on this?

Regards
Dong Aisheng


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 4/7] regulator: anatop-regulator: convert to use imx-syscon to access anatop register
  2012-08-23 11:17       ` Mark Brown
@ 2012-08-24  2:29         ` Dong Aisheng
  0 siblings, 0 replies; 29+ messages in thread
From: Dong Aisheng @ 2012-08-24  2:29 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-kernel, linux-arm-kernel, linus.walleij, s.hauer,
	shawn.guo, kernel, grant.likely, rob.herring, sameo, lrg,
	Zhao Richard-B20223, devicetree-discuss, swarren

On Thu, Aug 23, 2012 at 07:17:41PM +0800, Mark Brown wrote:
> On Thu, Aug 23, 2012 at 03:15:04PM +0800, Dong Aisheng wrote:
> > On Wed, Aug 22, 2012 at 11:59:53PM +0800, Mark Brown wrote:
> 
> > > With the conversion to regmap it'd also be good to convert the driver to
> > > use the regmap helper functions for enable and voltage operations if
> > > possible.
> 
> > The anatop-regulator driver only implements set_voltage_sel/get_voltage_sel
> > which i have already converted, what do you mean others like
> > 'for enable and voltage operations' i should also convert?
> 
> Those operations should ideally be converted to use the generic regmap
> implementation now the device uses regmap - regmap_get_voltage_sel_regmap
> and so on.

Okay, got it.

Regards
Dong Aisheng


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 4/7] regulator: anatop-regulator: convert to use imx-syscon to access anatop register
  2012-08-23 17:56       ` Stephen Warren
@ 2012-08-24  2:37         ` Dong Aisheng
  0 siblings, 0 replies; 29+ messages in thread
From: Dong Aisheng @ 2012-08-24  2:37 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Zhao Richard-B20223, linux-kernel, linux-arm-kernel,
	linus.walleij, s.hauer, shawn.guo, kernel, grant.likely,
	rob.herring, sameo, lrg, broonie, devicetree-discuss

On Fri, Aug 24, 2012 at 01:56:58AM +0800, Stephen Warren wrote:
> On 08/23/2012 12:12 AM, Richard Zhao wrote:
> > On Wed, Aug 22, 2012 at 11:21:03PM -0600, Stephen Warren wrote:
> >> On 08/22/2012 01:18 AM, Dong Aisheng wrote:
> >>> Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
> >>
> >>> diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
> >>
> >>> @@ -109,7 +110,11 @@ static int __devinit anatop_regulator_probe(struct platform_device *pdev)
> >>>  	rdesc->ops = &anatop_rops;
> >>>  	rdesc->type = REGULATOR_VOLTAGE;
> >>>  	rdesc->owner = THIS_MODULE;
> >>> -	sreg->mfd = anatopmfd;
> >>> +
> >>> +	sreg->anatop = of_parse_phandle(np, "fsl,anatop", 0);
> >>> +	if (!sreg->anatop)
> >>> +		return -ENODEV;
> >>
> >> In fact, that imx_syscon_lookup function I proposed could even do the
> >> of_parse_phandle() internally, so perhaps:
> >>
> >> foo->syscon_dev = imx_syscon_lookup(np, "fsl,anatop", 0);
> >> if (IS_ERR(foo->syscon_dev))
> >>     return PTR_ERR(foo->syscon_dev);
> >>
> >> with imx_syscon_lookup() internally knowing when to return EPROBE_DEFER
> >> rather than any other permanent error code (e.g. for missing property,
> >> bad phandle, etc.)
> >
> > In some case that we access register in machine code, we don't have any
> > phandle. The node is got by find compatible or by path.
> 
> That sounds a little odd; why not just use a phandle consistently
> everywhere?
Maybe for some places we do not have that device node, e.g:
arch/arm/mach-imx/mach-imx6q.c

> 
> Either way though, I could imagine still putting all the lookup code
> into the syscon driver; just have different functions for the different
> lookup methods:
> 
> imx_syscon_lookup_by_phandle(np, char *property_name)
Probably we do not need the left two lookup, below seems also ok if needed:
np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-anatop");
regmap = imx_syscon_lookup_by_phandle(np, property_name)
Then we do not need to handle how to find the compatible node.

> imx_syscon_lookup_by_compatible(char *compatible
> imx_syscon_lookup_by_path(char *node_path)

Regards
Dong Aisheng


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 1/7] mfd: add imx syscon driver based on regmap
  2012-08-22  7:18 ` [PATCH 1/7] mfd: add imx syscon driver based on regmap Dong Aisheng
  2012-08-22  8:29   ` Richard Zhao
  2012-08-22 16:02   ` Mark Brown
@ 2012-08-24  6:43   ` Shawn Guo
  2 siblings, 0 replies; 29+ messages in thread
From: Shawn Guo @ 2012-08-24  6:43 UTC (permalink / raw)
  To: Dong Aisheng
  Cc: linux-kernel, linux-arm-kernel, linus.walleij, s.hauer, kernel,
	grant.likely, rob.herring, sameo, lrg, broonie, richard.zhao,
	devicetree-discuss, swarren, paul.liu

On Wed, Aug 22, 2012 at 03:18:42PM +0800, Dong Aisheng wrote:
> From: Dong Aisheng <dong.aisheng@linaro.org>
> 
> Add regmap based imx syscon driver.
> This is usually used for access misc bits in registers which does not belong
> to a specific module, for example, IOMUXC GPR and ANATOP.
> With this driver, we provide a standard API for client driver to call to
> access registers which are registered into syscon.
> 
> Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
> ---
> Currently it's just simply for IMX, however the driver really is not too
> much imx specific.
> If people want, we probably could extend it to support other platforms too.

Right.  I do not see anything IMX specific there.  We should probably
at least give the driver a generic name from day one.

-- 
Regards,
Shawn

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2012-08-24  6:42 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-22  7:18 [PATCH 0/7] add imx-syscon driver for general registers access Dong Aisheng
2012-08-22  7:18 ` [PATCH 1/7] mfd: add imx syscon driver based on regmap Dong Aisheng
2012-08-22  8:29   ` Richard Zhao
2012-08-22 10:57     ` Dong Aisheng
2012-08-23  5:16       ` Stephen Warren
2012-08-23  6:09         ` Richard Zhao
2012-08-23  7:06         ` Dong Aisheng
2012-08-22 16:02   ` Mark Brown
2012-08-23  7:26     ` Dong Aisheng
2012-08-23 11:06       ` Mark Brown
2012-08-24  2:28         ` Dong Aisheng
2012-08-24  6:43   ` Shawn Guo
2012-08-22  7:18 ` [PATCH 2/7] ARM: imx6q: add iomuxc gpr support into imx-syscon Dong Aisheng
2012-08-22  7:18 ` [PATCH 3/7] ARM: imx6q: add anatop " Dong Aisheng
2012-08-22  7:18 ` [PATCH 4/7] regulator: anatop-regulator: convert to use imx-syscon to access anatop register Dong Aisheng
2012-08-22 15:59   ` Mark Brown
2012-08-23  7:15     ` Dong Aisheng
2012-08-23 11:17       ` Mark Brown
2012-08-24  2:29         ` Dong Aisheng
2012-08-23  5:21   ` Stephen Warren
2012-08-23  6:12     ` Richard Zhao
2012-08-23 17:56       ` Stephen Warren
2012-08-24  2:37         ` Dong Aisheng
2012-08-23  7:32     ` Dong Aisheng
2012-08-22  7:18 ` [PATCH 5/7] ARM: imx6q: convert to use imx-syscon to access anatop registers Dong Aisheng
2012-08-22  7:18 ` [PATCH 6/7] ARM: dts: imx6q: add simple-bus compatible string for anatop Dong Aisheng
2012-08-22  8:52   ` Richard Zhao
2012-08-22 11:02     ` Dong Aisheng
2012-08-22  7:18 ` [PATCH 7/7] mfd: anatop-mfd: remove anatop driver Dong Aisheng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).