2012-08-22 07:37:59

by Dong Aisheng

[permalink] [raw]
Subject: [PATCH 0/7] add imx-syscon driver for general registers access

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


2012-08-22 07:38:08

by Dong Aisheng

[permalink] [raw]
Subject: [PATCH 1/7] mfd: add imx syscon driver based on regmap

From: Dong Aisheng <[email protected]>

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 <[email protected]>
---
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 <[email protected]>
+ *
+ * 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 <[email protected]>");
+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 <[email protected]>
+ *
+ * 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

2012-08-22 07:38:16

by Dong Aisheng

[permalink] [raw]
Subject: [PATCH 3/7] ARM: imx6q: add anatop support into imx-syscon

From: Dong Aisheng <[email protected]>

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 <[email protected]>
---
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

2012-08-22 07:38:26

by Dong Aisheng

[permalink] [raw]
Subject: [PATCH 6/7] ARM: dts: imx6q: add simple-bus compatible string for anatop

From: Dong Aisheng <[email protected]>

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 <[email protected]>
---
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

2012-08-22 07:38:31

by Dong Aisheng

[permalink] [raw]
Subject: [PATCH 5/7] ARM: imx6q: convert to use imx-syscon to access anatop registers

From: Dong Aisheng <[email protected]>

Using standard imx syscon API to access anatop registers.

Signed-off-by: Dong Aisheng <[email protected]>
---
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

2012-08-22 07:38:35

by Dong Aisheng

[permalink] [raw]
Subject: [PATCH 7/7] mfd: anatop-mfd: remove anatop driver

From: Dong Aisheng <[email protected]>

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

Signed-off-by: Dong Aisheng <[email protected]>
---
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) <[email protected]>
- * 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) <[email protected]>");
-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) <[email protected]>
- * 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

2012-08-22 07:39:19

by Dong Aisheng

[permalink] [raw]
Subject: [PATCH 4/7] regulator: anatop-regulator: convert to use imx-syscon to access anatop register

From: Dong Aisheng <[email protected]>

Using standard imx syscon API to access anatop register.

Signed-off-by: Dong Aisheng <[email protected]>
---
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

2012-08-22 07:39:50

by Dong Aisheng

[permalink] [raw]
Subject: [PATCH 2/7] ARM: imx6q: add iomuxc gpr support into imx-syscon

From: Dong Aisheng <[email protected]>

Include headfile for easy using.

Signed-off-by: Dong Aisheng <[email protected]>
---
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

2012-08-22 08:29:49

by Richard Zhao

[permalink] [raw]
Subject: Re: [PATCH 1/7] mfd: add imx syscon driver based on regmap

On Wed, Aug 22, 2012 at 03:18:42PM +0800, Dong Aisheng wrote:
> From: Dong Aisheng <[email protected]>
>
> 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 <[email protected]>
> ---
> 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 <[email protected]>
> + *
> + * 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 <[email protected]>");
> +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 <[email protected]>
> + *
> + * 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
>

2012-08-22 08:53:09

by Richard Zhao

[permalink] [raw]
Subject: Re: [PATCH 6/7] ARM: dts: imx6q: add simple-bus compatible string for anatop

On Wed, Aug 22, 2012 at 03:18:47PM +0800, Dong Aisheng wrote:
> From: Dong Aisheng <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
>

2012-08-22 11:16:38

by Dong Aisheng

[permalink] [raw]
Subject: Re: [PATCH 1/7] mfd: add imx syscon driver based on regmap

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 <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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

2012-08-22 11:21:15

by Dong Aisheng

[permalink] [raw]
Subject: Re: [PATCH 6/7] ARM: dts: imx6q: add simple-bus compatible string for anatop

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 <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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

2012-08-22 15:59:58

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 4/7] regulator: anatop-regulator: convert to use imx-syscon to access anatop register

On Wed, Aug 22, 2012 at 03:18:45PM +0800, Dong Aisheng wrote:
> From: Dong Aisheng <[email protected]>
>
> Using standard imx syscon API to access anatop register.

Acked-by: Mark Brown <[email protected]>

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.


Attachments:
(No filename) (384.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-08-22 16:02:47

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 1/7] mfd: add imx syscon driver based on regmap

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

> From: Dong Aisheng <[email protected]>

> Add regmap based imx syscon driver.

Nice to see more regmap-mmio usage!

Reviwed-by: Mark Brown <[email protected]>

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).


Attachments:
(No filename) (861.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-08-23 05:16:37

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH 1/7] mfd: add imx syscon driver based on regmap

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.

2012-08-23 05:21:09

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH 4/7] regulator: anatop-regulator: convert to use imx-syscon to access anatop register

On 08/22/2012 01:18 AM, Dong Aisheng wrote:
> Signed-off-by: Dong Aisheng <[email protected]>

> 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.)

2012-08-23 06:09:45

by Richard Zhao

[permalink] [raw]
Subject: Re: [PATCH 1/7] mfd: add imx syscon driver based on regmap

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

2012-08-23 06:12:07

by Richard Zhao

[permalink] [raw]
Subject: Re: [PATCH 4/7] regulator: anatop-regulator: convert to use imx-syscon to access anatop register

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 <[email protected]>
>
> > 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

2012-08-23 07:26:17

by Dong Aisheng

[permalink] [raw]
Subject: Re: [PATCH 1/7] mfd: add imx syscon driver based on regmap

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

2012-08-23 07:34:20

by Dong Aisheng

[permalink] [raw]
Subject: Re: [PATCH 4/7] regulator: anatop-regulator: convert to use imx-syscon to access anatop register

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 <[email protected]>
> >
> > Using standard imx syscon API to access anatop register.
>
> Acked-by: Mark Brown <[email protected]>
>
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

2012-08-23 07:45:57

by Dong Aisheng

[permalink] [raw]
Subject: Re: [PATCH 1/7] mfd: add imx syscon driver based on regmap

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 <[email protected]>
>
> > Add regmap based imx syscon driver.
>
> Nice to see more regmap-mmio usage!
>
> Reviwed-by: Mark Brown <[email protected]>
>
> 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

2012-08-23 07:51:19

by Dong Aisheng

[permalink] [raw]
Subject: Re: [PATCH 4/7] regulator: anatop-regulator: convert to use imx-syscon to access anatop register

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 <[email protected]>
>
> > 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

2012-08-23 11:06:57

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 1/7] mfd: add imx syscon driver based on regmap

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.


Attachments:
(No filename) (695.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-08-23 11:17:47

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 4/7] regulator: anatop-regulator: convert to use imx-syscon to access anatop register

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.


Attachments:
(No filename) (649.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-08-23 17:57:05

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH 4/7] regulator: anatop-regulator: convert to use imx-syscon to access anatop register

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 <[email protected]>
>>
>>> 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)

2012-08-24 02:47:39

by Dong Aisheng

[permalink] [raw]
Subject: Re: [PATCH 1/7] mfd: add imx syscon driver based on regmap

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

2012-08-24 02:49:14

by Dong Aisheng

[permalink] [raw]
Subject: Re: [PATCH 4/7] regulator: anatop-regulator: convert to use imx-syscon to access anatop register

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

2012-08-24 02:57:22

by Dong Aisheng

[permalink] [raw]
Subject: Re: [PATCH 4/7] regulator: anatop-regulator: convert to use imx-syscon to access anatop register

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 <[email protected]>
> >>
> >>> 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

2012-08-24 06:42:30

by Shawn Guo

[permalink] [raw]
Subject: Re: [PATCH 1/7] mfd: add imx syscon driver based on regmap

On Wed, Aug 22, 2012 at 03:18:42PM +0800, Dong Aisheng wrote:
> From: Dong Aisheng <[email protected]>
>
> 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 <[email protected]>
> ---
> 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