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
From: Dong Aisheng <redacted>
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 <redacted>
---
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(-)
@@ -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 at 020e0000 {+ compatible = "fsl,imx6q-iomuxc", "fsl,imx-syscon";+ reg = <0x020e0000 0x38>;+};
@@ -993,6 +993,14 @@ config MFD_ANATOPMFDcontroller.ThiscontrollerembedsregulatorandthermaldevicesforFreescalei.MXplatforms.+configMFD_IMX_SYSCON+bool"Freescale i.MX System Controller Register R/W Based on Regmap"+depends onARCH_MXC+selectREGMAP_MMIO+help+SelectthisoptiontoenableaccessFreescalei.MXsystemcontrol+registerslikeiomuxcgprandanatopviaregmap.+configMFD_PALMASbool"Support for the TI Palmas series chips"selectMFD_CORE
From: Dong Aisheng <redacted>
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 <redacted>
---
arch/arm/boot/dts/imx6q.dtsi | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
From: Dong Aisheng <redacted>
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 <redacted>
---
arch/arm/boot/dts/imx6q.dtsi | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
@@ -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");
@@ -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 */
From: Richard Zhao <hidden> Date: 2012-08-22 08:29:49
On Wed, Aug 22, 2012 at 03:18:42PM +0800, Dong Aisheng wrote:
quoted hunk
From: Dong Aisheng <redacted>
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 <redacted>
---
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(-)
@@ -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 at 020e0000 {+ compatible = "fsl,imx6q-iomuxc", "fsl,imx-syscon";
@@ -993,6 +993,14 @@ config MFD_ANATOPMFDcontroller.ThiscontrollerembedsregulatorandthermaldevicesforFreescalei.MXplatforms.+configMFD_IMX_SYSCON+bool"Freescale i.MX System Controller Register R/W Based on Regmap"+depends onARCH_MXC+selectREGMAP_MMIO+help+SelectthisoptiontoenableaccessFreescalei.MXsystemcontrol+registerslikeiomuxcgprandanatopviaregmap.+configMFD_PALMASbool"Support for the TI Palmas series chips"selectMFD_CORE
From: Richard Zhao <hidden> Date: 2012-08-22 08:53:09
On Wed, Aug 22, 2012 at 03:18:47PM +0800, Dong Aisheng wrote:
quoted hunk
From: Dong Aisheng <redacted>
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 <redacted>
---
arch/arm/boot/dts/imx6q.dtsi | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
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
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:
quoted
From: Dong Aisheng <redacted>
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 <redacted>
---
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(-)
@@ -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 at 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.
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.
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
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:
quoted
From: Dong Aisheng <redacted>
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 <redacted>
---
arch/arm/boot/dts/imx6q.dtsi | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
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
From: Mark Brown <hidden> Date: 2012-08-22 15:59:58
On Wed, Aug 22, 2012 at 03:18:45PM +0800, Dong Aisheng wrote:
From: Dong Aisheng <redacted>
Using standard imx syscon API to access anatop register.
Acked-by: Mark Brown <redacted>
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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120822/2b57f4e2/attachment.sig>
From: Mark Brown <hidden> Date: 2012-08-22 16:02:47
On Wed, Aug 22, 2012 at 03:18:42PM +0800, Dong Aisheng wrote:
From: Dong Aisheng <redacted>
Add regmap based imx syscon driver.
Nice to see more regmap-mmio usage!
Reviwed-by: Mark Brown [off-list ref]
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).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120822/bde6a801/attachment.sig>
From: Stephen Warren <hidden> Date: 2012-08-23 05:16:37
On 08/22/2012 04:57 AM, Dong Aisheng wrote:
On Wed, Aug 22, 2012 at 04:29:41PM +0800, Zhao Richard-B20223 wrote:
quoted
On Wed, Aug 22, 2012 at 03:18:42PM +0800, Dong Aisheng wrote:
quoted
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.
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.
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.)
From: Richard Zhao <hidden> Date: 2012-08-23 06:09:45
On Wed, Aug 22, 2012 at 11:16:33PM -0600, Stephen Warren wrote:
On 08/22/2012 04:57 AM, Dong Aisheng wrote:
quoted
On Wed, Aug 22, 2012 at 04:29:41PM +0800, Zhao Richard-B20223 wrote:
quoted
On Wed, Aug 22, 2012 at 03:18:42PM +0800, Dong Aisheng wrote:
quoted
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.
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.
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
On Thu, Aug 23, 2012 at 01:16:33PM +0800, Stephen Warren wrote:
On 08/22/2012 04:57 AM, Dong Aisheng wrote:
quoted
On Wed, Aug 22, 2012 at 04:29:41PM +0800, Zhao Richard-B20223 wrote:
quoted
On Wed, Aug 22, 2012 at 03:18:42PM +0800, Dong Aisheng wrote:
quoted
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.
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.
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:
quoted
From: Dong Aisheng <redacted>
Using standard imx syscon API to access anatop register.
Acked-by: Mark Brown <redacted>
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
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:
quoted
From: Dong Aisheng <redacted>
quoted
Add regmap based imx syscon driver.
Nice to see more regmap-mmio usage!
Reviwed-by: Mark Brown [off-list ref]
from a regmap point of view.
Thanks
quoted
+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
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
From: Mark Brown <hidden> Date: 2012-08-23 11:06:57
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:
quoted
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?
From: Mark Brown <hidden> Date: 2012-08-23 11:17:47
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:
quoted
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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120823/05ca2fe8/attachment.sig>
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)
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:
quoted
On Thu, Aug 23, 2012 at 12:02:41AM +0800, Mark Brown wrote:
quoted
quoted
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).
quoted
Do you mean something like:
regmap = syscon_regmap_dev_lookup(np, "fsl,anatop");
regmap_write(regmap, reg, val);
quoted
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
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:
quoted
On Wed, Aug 22, 2012 at 11:59:53PM +0800, Mark Brown wrote:
quoted
quoted
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.
quoted
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.
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.
On Wed, Aug 22, 2012 at 03:18:42PM +0800, Dong Aisheng wrote:
From: Dong Aisheng <redacted>
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 <redacted>
---
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