From: "Ying-Chun Liu (PaulLiu)" <[email protected]>
Anatop is a mfd chip embedded in Freescale i.MX6Q SoC.
Anatop provides regulators and thermal.
This driver handles the address space and the operation of the mfd device.
Signed-off-by: Ying-Chun Liu (PaulLiu) <[email protected]>
Cc: Samuel Ortiz <[email protected]>
---
drivers/mfd/Kconfig | 6 ++
drivers/mfd/Makefile | 1 +
drivers/mfd/anatop-mfd.c | 152 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/mfd/anatop.h | 39 +++++++++++
4 files changed, 198 insertions(+), 0 deletions(-)
create mode 100644 drivers/mfd/anatop-mfd.c
create mode 100644 include/linux/mfd/anatop.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index cd13e9f..4f71627 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -830,6 +830,12 @@ config MFD_INTEL_MSIC
Passage) chip. This chip embeds audio, battery, GPIO, etc.
devices used in Intel Medfield platforms.
+config MFD_ANATOP
+ bool "Support for Anatop"
+ depends on SOC_IMX6Q
+ help
+ Select this option to enable Anatop MFD driver.
+
endmenu
endif
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index b953bab..8e11060 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -112,3 +112,4 @@ obj-$(CONFIG_TPS65911_COMPARATOR) += tps65911-comparator.o
obj-$(CONFIG_MFD_AAT2870_CORE) += aat2870-core.o
obj-$(CONFIG_MFD_INTEL_MSIC) += intel_msic.o
obj-$(CONFIG_MFD_S5M_CORE) += s5m-core.o s5m-irq.o
+obj-$(CONFIG_MFD_ANATOP) += anatop-mfd.o
diff --git a/drivers/mfd/anatop-mfd.c b/drivers/mfd/anatop-mfd.c
new file mode 100644
index 0000000..58c6054
--- /dev/null
+++ b/drivers/mfd/anatop-mfd.c
@@ -0,0 +1,152 @@
+/*
+ * 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>
+
+static u32 anatop_read(struct anatop *adata, u32 addr, int bit_shift, int bits)
+{
+ u32 val;
+ int mask;
+ if (bits == 32)
+ mask = 0xff;
+ else
+ mask = (1 << bits) - 1;
+
+ val = ioread32(adata->ioreg+addr);
+ val = (val >> bit_shift) & mask;
+ return val;
+}
+
+static void anatop_write(struct anatop *adata, u32 addr, int bit_shift,
+ int bits, u32 data)
+{
+ u32 val;
+ int mask;
+ if (bits == 32)
+ mask = 0xff;
+ else
+ mask = (1 << bits) - 1;
+
+ val = ioread32(adata->ioreg+addr) & ~(mask << bit_shift);
+ iowrite32((data << bit_shift) | val, adata->ioreg);
+}
+
+static const struct of_device_id of_anatop_regulator_match[] = {
+ {
+ .compatible = "fsl,anatop-regulator",
+ },
+ {
+ .compatible = "fsl,anatop-thermal",
+ },
+ { },
+};
+
+static int of_anatop_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+ u64 ofaddr;
+ u64 ofsize;
+ void *ioreg;
+ struct anatop *drvdata;
+ int ret = 0;
+ const __be32 *rval;
+
+ rval = of_get_address(np, 0, &ofsize, NULL);
+ if (rval)
+ ofaddr = be32_to_cpu(*rval);
+ else
+ return -EINVAL;
+
+ ioreg = ioremap(ofaddr, ofsize);
+ if (!ioreg)
+ return -EINVAL;
+ drvdata = devm_kzalloc(dev, sizeof(struct anatop), GFP_KERNEL);
+ if (!drvdata)
+ return -EINVAL;
+ drvdata->ioreg = ioreg;
+ drvdata->read = anatop_read;
+ drvdata->write = anatop_write;
+ platform_set_drvdata(pdev, drvdata);
+ of_platform_bus_probe(np, of_anatop_regulator_match, dev);
+ return ret;
+}
+
+static int of_anatop_remove(struct platform_device *pdev)
+{
+ return 0;
+}
+
+static const struct of_device_id of_anatop_match[] = {
+ {
+ .compatible = "fsl,imx6q-anatop",
+ },
+ { },
+};
+
+static struct platform_driver anatop_of_driver = {
+ .driver = {
+ .name = "anatop-mfd",
+ .owner = THIS_MODULE,
+ .of_match_table = of_anatop_match,
+ },
+ .probe = of_anatop_probe,
+ .remove = of_anatop_remove,
+};
+
+static int __init anatop_init(void)
+{
+ int ret;
+ ret = platform_driver_register(&anatop_of_driver);
+ return ret;
+}
+
+static void __exit anatop_exit(void)
+{
+ platform_driver_unregister(&anatop_of_driver);
+}
+
+postcore_initcall(anatop_init);
+module_exit(anatop_exit);
+
+MODULE_AUTHOR("Ying-Chun Liu (PaulLiu)");
+MODULE_DESCRIPTION("ANATOP MFD driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/anatop.h b/include/linux/mfd/anatop.h
new file mode 100644
index 0000000..4425538
--- /dev/null
+++ b/include/linux/mfd/anatop.h
@@ -0,0 +1,39 @@
+/*
+ * 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
+
+/**
+ * anatop - MFD data
+ * @ioreg: ioremap register
+ * @read: function to read bits from the device
+ * @write: function to write bits to the device
+ */
+struct anatop {
+ void *ioreg;
+ u32 (*read) (struct anatop *adata, u32 addr, int bit_shift, int bits);
+ void (*write) (struct anatop *adata, u32 addr, int bit_shift,
+ int bits, u32 data);
+
+};
+
+#endif /* __LINUX_MFD_ANATOP_H */
--
1.7.8.3
From: "Ying-Chun Liu (PaulLiu)" <[email protected]>
Anatop is an integrated regulator inside i.MX6 SoC.
There are 3 digital regulators which controls PU, CORE (ARM), and SOC.
And 3 analog regulators which controls 1P1, 2P5, 3P0 (USB).
This patch adds the Anatop regulator driver.
Signed-off-by: Nancy Chen <[email protected]>
Signed-off-by: Ying-Chun Liu (PaulLiu) <[email protected]>
Cc: Mark Brown <[email protected]>
Cc: Liam Girdwood <[email protected]>
---
drivers/regulator/Kconfig | 6 +
drivers/regulator/Makefile | 1 +
drivers/regulator/anatop-regulator.c | 204 ++++++++++++++++++++++++++++
include/linux/regulator/anatop-regulator.h | 49 +++++++
4 files changed, 260 insertions(+), 0 deletions(-)
create mode 100644 drivers/regulator/anatop-regulator.c
create mode 100644 include/linux/regulator/anatop-regulator.h
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 7a61b17..5fbcda2 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -335,5 +335,11 @@ config REGULATOR_AAT2870
If you have a AnalogicTech AAT2870 say Y to enable the
regulator driver.
+config REGULATOR_ANATOP
+ tristate "ANATOP LDO regulators"
+ depends on MFD_ANATOP
+ help
+ Say y here to support ANATOP LDOs regulators.
+
endif
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 503bac8..8440871 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -48,5 +48,6 @@ obj-$(CONFIG_REGULATOR_AB8500) += ab8500.o
obj-$(CONFIG_REGULATOR_DB8500_PRCMU) += db8500-prcmu.o
obj-$(CONFIG_REGULATOR_TPS65910) += tps65910-regulator.o
obj-$(CONFIG_REGULATOR_AAT2870) += aat2870-regulator.o
+obj-$(CONFIG_REGULATOR_ANATOP) += anatop-regulator.o
ccflags-$(CONFIG_REGULATOR_DEBUG) += -DDEBUG
diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
new file mode 100644
index 0000000..d800c88
--- /dev/null
+++ b/drivers/regulator/anatop-regulator.c
@@ -0,0 +1,204 @@
+/*
+ * Copyright (C) 2011 Freescale Semiconductor, Inc. All Rights Reserved.
+ */
+
+/*
+ * 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/slab.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/anatop-regulator.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/regulator/of_regulator.h>
+
+static int anatop_set_voltage(struct regulator_dev *reg, int min_uV,
+ int max_uV, unsigned *selector)
+{
+ struct anatop_regulator *anatop_reg = rdev_get_drvdata(reg);
+ u32 val, sel;
+ int uv;
+
+ uv = min_uV;
+ pr_debug("%s: uv %d, min %d, max %d\n", __func__,
+ uv, anatop_reg->rdata->min_voltage,
+ anatop_reg->rdata->max_voltage);
+
+ if (uv < anatop_reg->rdata->min_voltage
+ || uv > anatop_reg->rdata->max_voltage) {
+ if (max_uV > anatop_reg->rdata->min_voltage)
+ uv = anatop_reg->rdata->min_voltage;
+ else
+ return -EINVAL;
+ }
+
+ if (anatop_reg->rdata->control_reg) {
+ sel = (uv - anatop_reg->rdata->min_voltage) / 25000;
+ val = anatop_reg->rdata->min_bit_val + sel;
+ *selector = sel;
+ pr_debug("%s: calculated val %d\n", __func__, val);
+ anatop_reg->rdata->mfd->write(anatop_reg->rdata->mfd,
+ anatop_reg->rdata->control_reg,
+ anatop_reg->rdata->vol_bit_shift,
+ anatop_reg->rdata->vol_bit_size,
+ val);
+ return 0;
+ } else {
+ return -ENOTSUPP;
+ }
+}
+
+static int anatop_get_voltage_sel(struct regulator_dev *reg)
+{
+ struct anatop_regulator *anatop_reg = rdev_get_drvdata(reg);
+ int selector;
+ struct anatop_regulator_data *rdata = anatop_reg->rdata;
+
+ if (rdata->control_reg) {
+ u32 val = rdata->mfd->read(rdata->mfd,
+ rdata->control_reg,
+ rdata->vol_bit_shift,
+ rdata->vol_bit_size);
+ selector = val - rdata->min_bit_val;
+ return selector;
+ } else {
+ return -ENOTSUPP;
+ }
+}
+
+static int anatop_list_voltage(struct regulator_dev *dev, unsigned selector)
+{
+ struct anatop_regulator *anatop_reg = rdev_get_drvdata(dev);
+ int uv;
+ struct anatop_regulator_data *rdata = anatop_reg->rdata;
+
+ uv = rdata->min_voltage + selector * 25000;
+ pr_debug("vddio = %d, selector = %u\n", uv, selector);
+ return uv;
+}
+
+static struct regulator_ops anatop_rops = {
+ .set_voltage = anatop_set_voltage,
+ .get_voltage_sel = anatop_get_voltage_sel,
+ .list_voltage = anatop_list_voltage,
+};
+
+int anatop_regulator_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+ struct regulator_desc *rdesc;
+ struct regulator_dev *rdev;
+ struct anatop_regulator *sreg;
+ struct regulator_init_data *initdata;
+ struct anatop *anatopmfd = dev_get_drvdata(pdev->dev.parent);
+ const __be32 *rval;
+ u64 ofsize;
+
+ initdata = of_get_regulator_init_data(dev, dev->of_node);
+ sreg = devm_kzalloc(dev, sizeof(struct anatop_regulator), GFP_KERNEL);
+ if (!sreg)
+ return -EINVAL;
+ rdesc = devm_kzalloc(dev, sizeof(struct regulator_desc), GFP_KERNEL);
+ if (!rdesc)
+ return -EINVAL;
+ sreg->initdata = initdata;
+ sreg->regulator = rdesc;
+ memset(rdesc, 0, sizeof(struct regulator_desc));
+ rdesc->name = kstrdup(of_get_property(np, "regulator-name", NULL),
+ GFP_KERNEL);
+ rdesc->ops = &anatop_rops;
+ rdesc->type = REGULATOR_VOLTAGE;
+ rdesc->owner = THIS_MODULE;
+ sreg->rdata = devm_kzalloc(dev,
+ sizeof(struct anatop_regulator_data),
+ GFP_KERNEL);
+ sreg->rdata->name = kstrdup(rdesc->name, GFP_KERNEL);
+ rval = of_get_address(np, 0, &ofsize, NULL);
+ if (rval) {
+ sreg->rdata->control_reg = be32_to_cpu(rval[0]);
+ sreg->rdata->vol_bit_shift = be32_to_cpu(rval[1]);
+ }
+ sreg->rdata->mfd = anatopmfd;
+ sreg->rdata->vol_bit_size = (int)ofsize;
+ rval = of_get_property(np, "min-bit-val", NULL);
+ if (rval)
+ sreg->rdata->min_bit_val = be32_to_cpu(*rval);
+ rval = of_get_property(np, "min-voltage", NULL);
+ if (rval)
+ sreg->rdata->min_voltage = be32_to_cpu(*rval);
+ rval = of_get_property(np, "max-voltage", NULL);
+ if (rval)
+ sreg->rdata->max_voltage = be32_to_cpu(*rval);
+
+ /* register regulator */
+ rdev = regulator_register(rdesc, &pdev->dev,
+ initdata, sreg, pdev->dev.of_node);
+ platform_set_drvdata(pdev, rdev);
+
+ if (IS_ERR(rdev)) {
+ dev_err(&pdev->dev, "failed to register %s\n",
+ rdesc->name);
+ return PTR_ERR(rdev);
+ }
+
+ return 0;
+}
+
+int anatop_regulator_remove(struct platform_device *pdev)
+{
+ struct regulator_dev *rdev = platform_get_drvdata(pdev);
+ regulator_unregister(rdev);
+ return 0;
+}
+
+struct of_device_id __devinitdata of_anatop_regulator_match_tbl[] = {
+ { .compatible = "fsl,anatop-regulator", },
+ { /* end */ }
+};
+
+
+struct platform_driver anatop_regulator = {
+ .driver = {
+ .name = "anatop_regulator",
+ .owner = THIS_MODULE,
+ .of_match_table = of_anatop_regulator_match_tbl,
+ },
+ .probe = anatop_regulator_probe,
+ .remove = anatop_regulator_remove,
+};
+
+int anatop_regulator_init(void)
+{
+ return platform_driver_register(&anatop_regulator);
+}
+
+void anatop_regulator_exit(void)
+{
+ platform_driver_unregister(&anatop_regulator);
+}
+
+postcore_initcall(anatop_regulator_init);
+module_exit(anatop_regulator_exit);
+
+MODULE_AUTHOR("Freescale Semiconductor, Inc.");
+MODULE_DESCRIPTION("ANATOP Regulator driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/regulator/anatop-regulator.h b/include/linux/regulator/anatop-regulator.h
new file mode 100644
index 0000000..089d519
--- /dev/null
+++ b/include/linux/regulator/anatop-regulator.h
@@ -0,0 +1,49 @@
+/*
+ * Copyright (C) 2011 Freescale Semiconductor, Inc. All Rights Reserved.
+ */
+
+/*
+ * 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.
+ */
+
+#ifndef __ANATOP_REGULATOR_H
+#define __ANATOP_REGULATOR_H
+#include <linux/regulator/driver.h>
+#include <linux/mfd/anatop.h>
+
+struct anatop_regulator {
+ struct regulator_desc *regulator;
+ struct regulator_init_data *initdata;
+ struct anatop_regulator_data *rdata;
+};
+
+
+struct anatop_regulator_data {
+ const char *name;
+
+ u32 control_reg;
+ struct anatop *mfd;
+ int vol_bit_shift;
+ int vol_bit_size;
+ int min_bit_val;
+ int min_voltage;
+ int max_voltage;
+};
+
+int anatop_register_regulator(
+ struct anatop_regulator *reg_data, int reg,
+ struct regulator_init_data *initdata);
+
+#endif /* __ANATOP_REGULATOR_H */
--
1.7.8.3
On Thu, Feb 09, 2012 at 04:51:26AM +0800, Ying-Chun Liu (PaulLiu) wrote:
Overall this is looking pretty good, a few issues but relatively minor.
> + if (uv < anatop_reg->rdata->min_voltage
> + || uv > anatop_reg->rdata->max_voltage) {
> + if (max_uV > anatop_reg->rdata->min_voltage)
> + uv = anatop_reg->rdata->min_voltage;
> + else
> + return -EINVAL;
> + }
This looks buggy (and is quite hard to follow). The check for the
voltage being above the minimum voltage is fine but surely if the
voltage is too high then the first if statement will be true and max_uV
will be greater than the minimum voltage so the second if will be true.
This will cause us to choose the minimum voltage that the regulator can
do which is less than the minimum voltage requested.
You probably shouldn't be checking for the upper end of the range at all
here...
> + if (anatop_reg->rdata->control_reg) {
> + sel = (uv - anatop_reg->rdata->min_voltage) / 25000;
> + val = anatop_reg->rdata->min_bit_val + sel;
> + *selector = sel;
> + pr_debug("%s: calculated val %d\n", __func__, val);
...instead check that selector is in range here.
> + anatop_reg->rdata->mfd->write(anatop_reg->rdata->mfd,
> + anatop_reg->rdata->control_reg,
> + anatop_reg->rdata->vol_bit_shift,
> + anatop_reg->rdata->vol_bit_size,
> + val);
Just have a write() function in the MFD.
> + memset(rdesc, 0, sizeof(struct regulator_desc));
> + rdesc->name = kstrdup(of_get_property(np, "regulator-name", NULL),
> + GFP_KERNEL);
You should add binding documentation for the device since it's OF based.
> + sreg->rdata->name = kstrdup(rdesc->name, GFP_KERNEL);
Can't you just point to rdesc->name?
> + if (IS_ERR(rdev)) {
> + dev_err(&pdev->dev, "failed to register %s\n",
> + rdesc->name);
> + return PTR_ERR(rdev);
> + }
All your kstrdup()s need to be unwound in error and free cases.
> +int anatop_regulator_init(void)
> +{
> + return platform_driver_register(&anatop_regulator);
> +}
> +
> +void anatop_regulator_exit(void)
> +{
> + platform_driver_unregister(&anatop_regulator);
> +}
> +
> +postcore_initcall(anatop_regulator_init);
> +module_exit(anatop_regulator_exit);
These should be adjacent to the functions.
> +MODULE_AUTHOR("Freescale Semiconductor, Inc.");
Either omit this or put one or more humans as the author.
> +struct anatop_regulator {
> + struct regulator_desc *regulator;
> + struct regulator_init_data *initdata;
> + struct anatop_regulator_data *rdata;
> +};
May as well just merge the regulator data in here - it's not ever used
except with a 1:1 relationship between them. Could also directly
embed the desc and init_data, then you just have one allocation for the
data rather than a series to error check.
> +int anatop_register_regulator(
> + struct anatop_regulator *reg_data, int reg,
> + struct regulator_init_data *initdata);
This looks like it's not defined any more so could be removed and since
you only appear to support OF the entire header could be merged into the
driver.
On Thu, Feb 09, 2012 at 04:51:25AM +0800, Ying-Chun Liu (PaulLiu) wrote:
> From: "Ying-Chun Liu (PaulLiu)" <[email protected]>
>
> Anatop is a mfd chip embedded in Freescale i.MX6Q SoC.
> Anatop provides regulators and thermal.
> This driver handles the address space and the operation of the mfd device.
>
> Signed-off-by: Ying-Chun Liu (PaulLiu) <[email protected]>
> Cc: Samuel Ortiz <[email protected]>
> ---
> drivers/mfd/Kconfig | 6 ++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/anatop-mfd.c | 152 ++++++++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/anatop.h | 39 +++++++++++
> 4 files changed, 198 insertions(+), 0 deletions(-)
> create mode 100644 drivers/mfd/anatop-mfd.c
> create mode 100644 include/linux/mfd/anatop.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index cd13e9f..4f71627 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -830,6 +830,12 @@ config MFD_INTEL_MSIC
> Passage) chip. This chip embeds audio, battery, GPIO, etc.
> devices used in Intel Medfield platforms.
>
> +config MFD_ANATOP
> + bool "Support for Anatop"
> + depends on SOC_IMX6Q
> + help
> + Select this option to enable Anatop MFD driver.
> +
> endmenu
> endif
>
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index b953bab..8e11060 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -112,3 +112,4 @@ obj-$(CONFIG_TPS65911_COMPARATOR) += tps65911-comparator.o
> obj-$(CONFIG_MFD_AAT2870_CORE) += aat2870-core.o
> obj-$(CONFIG_MFD_INTEL_MSIC) += intel_msic.o
> obj-$(CONFIG_MFD_S5M_CORE) += s5m-core.o s5m-irq.o
> +obj-$(CONFIG_MFD_ANATOP) += anatop-mfd.o
> diff --git a/drivers/mfd/anatop-mfd.c b/drivers/mfd/anatop-mfd.c
> new file mode 100644
> index 0000000..58c6054
> --- /dev/null
> +++ b/drivers/mfd/anatop-mfd.c
> @@ -0,0 +1,152 @@
> +/*
> + * 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>
> +
> +static u32 anatop_read(struct anatop *adata, u32 addr, int bit_shift, int bits)
> +{
> + u32 val;
> + int mask;
Nit: it may be nice to put a blank line here.
> + if (bits == 32)
> + mask = 0xff;
Shouldn't it be ~0?
> + else
> + mask = (1 << bits) - 1;
> +
> + val = ioread32(adata->ioreg+addr);
Why not just readl()? Nit: put space before and after '+'.
> + val = (val >> bit_shift) & mask;
Nit: it may be nice to put a blank line here.
> + return val;
> +}
> +
> +static void anatop_write(struct anatop *adata, u32 addr, int bit_shift,
> + int bits, u32 data)
> +{
> + u32 val;
> + int mask;
> + if (bits == 32)
> + mask = 0xff;
> + else
> + mask = (1 << bits) - 1;
> +
> + val = ioread32(adata->ioreg+addr) & ~(mask << bit_shift);
> + iowrite32((data << bit_shift) | val, adata->ioreg);
Same comments on anatop_read apply on anatop_write here.
> +}
> +
> +static const struct of_device_id of_anatop_regulator_match[] = {
A better naming of of_anatop_regulator_match, since it covers not only
regulator but also thermal as below?
> + {
> + .compatible = "fsl,anatop-regulator",
> + },
Nit: since you only have one line here, it could be just:
{ .compatible = "fsl,anatop-regulator", },
> + {
> + .compatible = "fsl,anatop-thermal",
> + },
> + { },
> +};
> +
> +static int of_anatop_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + u64 ofaddr;
> + u64 ofsize;
> + void *ioreg;
> + struct anatop *drvdata;
> + int ret = 0;
> + const __be32 *rval;
> +
---
> + rval = of_get_address(np, 0, &ofsize, NULL);
> + if (rval)
> + ofaddr = be32_to_cpu(*rval);
> + else
> + return -EINVAL;
> +
> + ioreg = ioremap(ofaddr, ofsize);
---
Above lines can just be of_iomap(np, 0);
> + if (!ioreg)
> + return -EINVAL;
> + drvdata = devm_kzalloc(dev, sizeof(struct anatop), GFP_KERNEL);
> + if (!drvdata)
> + return -EINVAL;
> + drvdata->ioreg = ioreg;
> + drvdata->read = anatop_read;
> + drvdata->write = anatop_write;
> + platform_set_drvdata(pdev, drvdata);
> + of_platform_bus_probe(np, of_anatop_regulator_match, dev);
Nit: it may be nice to put a blank line here.
> + return ret;
The 'ret' is used nowhere, so the above line can just be:
return 0;
> +}
> +
> +static int of_anatop_remove(struct platform_device *pdev)
> +{
Nothing to do here? At least iounmap?
> + return 0;
> +}
> +
> +static const struct of_device_id of_anatop_match[] = {
> + {
> + .compatible = "fsl,imx6q-anatop",
> + },
{ .compatible = "fsl,imx6q-anatop", },
> + { },
> +};
> +
> +static struct platform_driver anatop_of_driver = {
> + .driver = {
> + .name = "anatop-mfd",
> + .owner = THIS_MODULE,
> + .of_match_table = of_anatop_match,
> + },
> + .probe = of_anatop_probe,
> + .remove = of_anatop_remove,
> +};
> +
> +static int __init anatop_init(void)
> +{
> + int ret;
> + ret = platform_driver_register(&anatop_of_driver);
> + return ret;
return platform_driver_register(&anatop_of_driver);
> +}
> +
> +static void __exit anatop_exit(void)
> +{
> + platform_driver_unregister(&anatop_of_driver);
> +}
> +
> +postcore_initcall(anatop_init);
> +module_exit(anatop_exit);
> +
> +MODULE_AUTHOR("Ying-Chun Liu (PaulLiu)");
Missing your email address here.
> +MODULE_DESCRIPTION("ANATOP MFD driver");
> +MODULE_LICENSE("GPL");
"GPL v2" for better?
> diff --git a/include/linux/mfd/anatop.h b/include/linux/mfd/anatop.h
> new file mode 100644
> index 0000000..4425538
> --- /dev/null
> +++ b/include/linux/mfd/anatop.h
> @@ -0,0 +1,39 @@
> +/*
> + * anatop.h - Anatop MFD driver
> + *
> + * Copyright (C) 2012 Ying-Chun Liu (PaulLiu) <[email protected]>
It would nice to use a consistent email address through the patch.
Regards,
Shawn
> + * 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
> +
> +/**
> + * anatop - MFD data
> + * @ioreg: ioremap register
> + * @read: function to read bits from the device
> + * @write: function to write bits to the device
> + */
> +struct anatop {
> + void *ioreg;
> + u32 (*read) (struct anatop *adata, u32 addr, int bit_shift, int bits);
> + void (*write) (struct anatop *adata, u32 addr, int bit_shift,
> + int bits, u32 data);
> +
> +};
> +
> +#endif /* __LINUX_MFD_ANATOP_H */
> --
> 1.7.8.3
>
On Thu, Feb 09, 2012 at 04:51:26AM +0800, Ying-Chun Liu (PaulLiu) wrote:
> From: "Ying-Chun Liu (PaulLiu)" <[email protected]>
>
> Anatop is an integrated regulator inside i.MX6 SoC.
> There are 3 digital regulators which controls PU, CORE (ARM), and SOC.
> And 3 analog regulators which controls 1P1, 2P5, 3P0 (USB).
> This patch adds the Anatop regulator driver.
>
> Signed-off-by: Nancy Chen <[email protected]>
> Signed-off-by: Ying-Chun Liu (PaulLiu) <[email protected]>
> Cc: Mark Brown <[email protected]>
> Cc: Liam Girdwood <[email protected]>
> ---
> drivers/regulator/Kconfig | 6 +
> drivers/regulator/Makefile | 1 +
> drivers/regulator/anatop-regulator.c | 204 ++++++++++++++++++++++++++++
> include/linux/regulator/anatop-regulator.h | 49 +++++++
> 4 files changed, 260 insertions(+), 0 deletions(-)
> create mode 100644 drivers/regulator/anatop-regulator.c
> create mode 100644 include/linux/regulator/anatop-regulator.h
>
> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> index 7a61b17..5fbcda2 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -335,5 +335,11 @@ config REGULATOR_AAT2870
> If you have a AnalogicTech AAT2870 say Y to enable the
> regulator driver.
>
> +config REGULATOR_ANATOP
> + tristate "ANATOP LDO regulators"
> + depends on MFD_ANATOP
> + help
> + Say y here to support ANATOP LDOs regulators.
> +
> endif
>
> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
> index 503bac8..8440871 100644
> --- a/drivers/regulator/Makefile
> +++ b/drivers/regulator/Makefile
> @@ -48,5 +48,6 @@ obj-$(CONFIG_REGULATOR_AB8500) += ab8500.o
> obj-$(CONFIG_REGULATOR_DB8500_PRCMU) += db8500-prcmu.o
> obj-$(CONFIG_REGULATOR_TPS65910) += tps65910-regulator.o
> obj-$(CONFIG_REGULATOR_AAT2870) += aat2870-regulator.o
> +obj-$(CONFIG_REGULATOR_ANATOP) += anatop-regulator.o
>
> ccflags-$(CONFIG_REGULATOR_DEBUG) += -DDEBUG
> diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
> new file mode 100644
> index 0000000..d800c88
> --- /dev/null
> +++ b/drivers/regulator/anatop-regulator.c
> @@ -0,0 +1,204 @@
> +/*
> + * Copyright (C) 2011 Freescale Semiconductor, Inc. All Rights Reserved.
> + */
> +
> +/*
> + * 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/slab.h>
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/regulator/anatop-regulator.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/regulator/of_regulator.h>
> +
> +static int anatop_set_voltage(struct regulator_dev *reg, int min_uV,
> + int max_uV, unsigned *selector)
> +{
> + struct anatop_regulator *anatop_reg = rdev_get_drvdata(reg);
> + u32 val, sel;
> + int uv;
> +
> + uv = min_uV;
> + pr_debug("%s: uv %d, min %d, max %d\n", __func__,
I would suggest dev_dbg in device driver.
> + uv, anatop_reg->rdata->min_voltage,
> + anatop_reg->rdata->max_voltage);
> +
> + if (uv < anatop_reg->rdata->min_voltage
> + || uv > anatop_reg->rdata->max_voltage) {
> + if (max_uV > anatop_reg->rdata->min_voltage)
> + uv = anatop_reg->rdata->min_voltage;
> + else
> + return -EINVAL;
> + }
> +
> + if (anatop_reg->rdata->control_reg) {
> + sel = (uv - anatop_reg->rdata->min_voltage) / 25000;
> + val = anatop_reg->rdata->min_bit_val + sel;
> + *selector = sel;
> + pr_debug("%s: calculated val %d\n", __func__, val);
> + anatop_reg->rdata->mfd->write(anatop_reg->rdata->mfd,
> + anatop_reg->rdata->control_reg,
> + anatop_reg->rdata->vol_bit_shift,
> + anatop_reg->rdata->vol_bit_size,
> + val);
> + return 0;
> + } else {
> + return -ENOTSUPP;
> + }
> +}
> +
> +static int anatop_get_voltage_sel(struct regulator_dev *reg)
> +{
> + struct anatop_regulator *anatop_reg = rdev_get_drvdata(reg);
> + int selector;
> + struct anatop_regulator_data *rdata = anatop_reg->rdata;
> +
> + if (rdata->control_reg) {
> + u32 val = rdata->mfd->read(rdata->mfd,
> + rdata->control_reg,
> + rdata->vol_bit_shift,
> + rdata->vol_bit_size);
> + selector = val - rdata->min_bit_val;
> + return selector;
> + } else {
> + return -ENOTSUPP;
> + }
> +}
> +
> +static int anatop_list_voltage(struct regulator_dev *dev, unsigned selector)
> +{
> + struct anatop_regulator *anatop_reg = rdev_get_drvdata(dev);
> + int uv;
> + struct anatop_regulator_data *rdata = anatop_reg->rdata;
> +
> + uv = rdata->min_voltage + selector * 25000;
> + pr_debug("vddio = %d, selector = %u\n", uv, selector);
> + return uv;
> +}
> +
> +static struct regulator_ops anatop_rops = {
> + .set_voltage = anatop_set_voltage,
> + .get_voltage_sel = anatop_get_voltage_sel,
> + .list_voltage = anatop_list_voltage,
> +};
> +
> +int anatop_regulator_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + struct regulator_desc *rdesc;
> + struct regulator_dev *rdev;
> + struct anatop_regulator *sreg;
> + struct regulator_init_data *initdata;
> + struct anatop *anatopmfd = dev_get_drvdata(pdev->dev.parent);
> + const __be32 *rval;
> + u64 ofsize;
> +
> + initdata = of_get_regulator_init_data(dev, dev->of_node);
> + sreg = devm_kzalloc(dev, sizeof(struct anatop_regulator), GFP_KERNEL);
> + if (!sreg)
> + return -EINVAL;
> + rdesc = devm_kzalloc(dev, sizeof(struct regulator_desc), GFP_KERNEL);
> + if (!rdesc)
> + return -EINVAL;
> + sreg->initdata = initdata;
> + sreg->regulator = rdesc;
> + memset(rdesc, 0, sizeof(struct regulator_desc));
> + rdesc->name = kstrdup(of_get_property(np, "regulator-name", NULL),
> + GFP_KERNEL);
We can probably reuse the regulator's node name here to save property
regulator-name.
> + rdesc->ops = &anatop_rops;
> + rdesc->type = REGULATOR_VOLTAGE;
> + rdesc->owner = THIS_MODULE;
> + sreg->rdata = devm_kzalloc(dev,
> + sizeof(struct anatop_regulator_data),
> + GFP_KERNEL);
> + sreg->rdata->name = kstrdup(rdesc->name, GFP_KERNEL);
> + rval = of_get_address(np, 0, &ofsize, NULL);
> + if (rval) {
> + sreg->rdata->control_reg = be32_to_cpu(rval[0]);
> + sreg->rdata->vol_bit_shift = be32_to_cpu(rval[1]);
I'm not sure you can (ab)use 'reg' property for bit shift. We may
need to have a property for it.
> + }
> + sreg->rdata->mfd = anatopmfd;
> + sreg->rdata->vol_bit_size = (int)ofsize;
> + rval = of_get_property(np, "min-bit-val", NULL);
> + if (rval)
> + sreg->rdata->min_bit_val = be32_to_cpu(*rval);
> + rval = of_get_property(np, "min-voltage", NULL);
> + if (rval)
> + sreg->rdata->min_voltage = be32_to_cpu(*rval);
> + rval = of_get_property(np, "max-voltage", NULL);
> + if (rval)
> + sreg->rdata->max_voltage = be32_to_cpu(*rval);
We need a sensible binding document to understand those. But at least,
shouldn't min-voltage and max-voltage be retrieved as the common
regulator binding documented in
Documentation/devicetree/bindings/regulator/regulator.txt?
> +
> + /* register regulator */
> + rdev = regulator_register(rdesc, &pdev->dev,
> + initdata, sreg, pdev->dev.of_node);
> + platform_set_drvdata(pdev, rdev);
> +
> + if (IS_ERR(rdev)) {
> + dev_err(&pdev->dev, "failed to register %s\n",
> + rdesc->name);
> + return PTR_ERR(rdev);
> + }
> +
> + return 0;
> +}
> +
> +int anatop_regulator_remove(struct platform_device *pdev)
> +{
> + struct regulator_dev *rdev = platform_get_drvdata(pdev);
> + regulator_unregister(rdev);
> + return 0;
> +}
> +
> +struct of_device_id __devinitdata of_anatop_regulator_match_tbl[] = {
> + { .compatible = "fsl,anatop-regulator", },
> + { /* end */ }
> +};
> +
> +
One blank line is enough.
> +struct platform_driver anatop_regulator = {
> + .driver = {
> + .name = "anatop_regulator",
> + .owner = THIS_MODULE,
> + .of_match_table = of_anatop_regulator_match_tbl,
> + },
> + .probe = anatop_regulator_probe,
> + .remove = anatop_regulator_remove,
> +};
> +
> +int anatop_regulator_init(void)
> +{
> + return platform_driver_register(&anatop_regulator);
> +}
> +
> +void anatop_regulator_exit(void)
> +{
> + platform_driver_unregister(&anatop_regulator);
> +}
> +
> +postcore_initcall(anatop_regulator_init);
> +module_exit(anatop_regulator_exit);
> +
> +MODULE_AUTHOR("Freescale Semiconductor, Inc.");
> +MODULE_DESCRIPTION("ANATOP Regulator driver");
> +MODULE_LICENSE("GPL");
"GPL v2"?
> diff --git a/include/linux/regulator/anatop-regulator.h b/include/linux/regulator/anatop-regulator.h
> new file mode 100644
> index 0000000..089d519
> --- /dev/null
> +++ b/include/linux/regulator/anatop-regulator.h
> @@ -0,0 +1,49 @@
> +/*
> + * Copyright (C) 2011 Freescale Semiconductor, Inc. All Rights Reserved.
> + */
> +
> +/*
> + * 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.
> + */
> +
> +#ifndef __ANATOP_REGULATOR_H
> +#define __ANATOP_REGULATOR_H
Have a blank line here.
> +#include <linux/regulator/driver.h>
> +#include <linux/mfd/anatop.h>
> +
> +struct anatop_regulator {
> + struct regulator_desc *regulator;
> + struct regulator_init_data *initdata;
> + struct anatop_regulator_data *rdata;
> +};
> +
> +
Drop one blank line here.
> +struct anatop_regulator_data {
> + const char *name;
> +
Nit: drop this blank line.
> + u32 control_reg;
> + struct anatop *mfd;
> + int vol_bit_shift;
> + int vol_bit_size;
> + int min_bit_val;
> + int min_voltage;
> + int max_voltage;
> +};
> +
It seems to me that anatop_regulator_data should be put before
anatop_regulator.
Regards,
Shawn
> +int anatop_register_regulator(
> + struct anatop_regulator *reg_data, int reg,
> + struct regulator_init_data *initdata);
> +
> +#endif /* __ANATOP_REGULATOR_H */
> --
> 1.7.8.3
>
On Fri, Feb 10, 2012 at 10:36:38PM -0800, Shawn Guo wrote:
> On Thu, Feb 09, 2012 at 04:51:26AM +0800, Ying-Chun Liu (PaulLiu) wrote:
> > + rval = of_get_property(np, "min-voltage", NULL);
> > + if (rval)
> > + sreg->rdata->min_voltage = be32_to_cpu(*rval);
> > + rval = of_get_property(np, "max-voltage", NULL);
> > + if (rval)
> > + sreg->rdata->max_voltage = be32_to_cpu(*rval);
> We need a sensible binding document to understand those. But at least,
> shouldn't min-voltage and max-voltage be retrieved as the common
> regulator binding documented in
> Documentation/devicetree/bindings/regulator/regulator.txt?
Normally this would be a bad idea as the set of voltages that can safely
be used on a given board might differ from those which are supported by
the device. However in this case you might be OK as this is all
internal to the SoC and so presumably won't vary from board to board.
Hi Paul,
I didn't get patch #2, so I don't get to see how the read/write functions ar
eused for example.
On Thu, Feb 09, 2012 at 04:51:25AM +0800, Ying-Chun Liu (PaulLiu) wrote:
> From: "Ying-Chun Liu (PaulLiu)" <[email protected]>
>
> Anatop is a mfd chip embedded in Freescale i.MX6Q SoC.
> Anatop provides regulators and thermal.
> This driver handles the address space and the operation of the mfd device.
A few comments here:
> +static u32 anatop_read(struct anatop *adata, u32 addr, int bit_shift, int bits)
> +{
> + u32 val;
> + int mask;
> + if (bits == 32)
> + mask = 0xff;
> + else
> + mask = (1 << bits) - 1;
> +
> + val = ioread32(adata->ioreg+addr);
> + val = (val >> bit_shift) & mask;
> + return val;
> +}
> +
> +static void anatop_write(struct anatop *adata, u32 addr, int bit_shift,
> + int bits, u32 data)
> +{
> + u32 val;
> + int mask;
> + if (bits == 32)
> + mask = 0xff;
> + else
> + mask = (1 << bits) - 1;
> +
> + val = ioread32(adata->ioreg+addr) & ~(mask << bit_shift);
> + iowrite32((data << bit_shift) | val, adata->ioreg);
> +}
Don't you need some sort of read/write atomic routine as well ? Locking would
be needed then...
> +static const struct of_device_id of_anatop_regulator_match[] = {
> + {
> + .compatible = "fsl,anatop-regulator",
> + },
> + {
> + .compatible = "fsl,anatop-thermal",
> + },
> + { },
> +};
> +
> +static int of_anatop_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + u64 ofaddr;
> + u64 ofsize;
> + void *ioreg;
> + struct anatop *drvdata;
> + int ret = 0;
> + const __be32 *rval;
> +
> + rval = of_get_address(np, 0, &ofsize, NULL);
> + if (rval)
> + ofaddr = be32_to_cpu(*rval);
> + else
> + return -EINVAL;
> +
> + ioreg = ioremap(ofaddr, ofsize);
> + if (!ioreg)
> + return -EINVAL;
> + drvdata = devm_kzalloc(dev, sizeof(struct anatop), GFP_KERNEL);
> + if (!drvdata)
> + return -EINVAL;
> + drvdata->ioreg = ioreg;
> + drvdata->read = anatop_read;
> + drvdata->write = anatop_write;
> + platform_set_drvdata(pdev, drvdata);
> + of_platform_bus_probe(np, of_anatop_regulator_match, dev);
> + return ret;
> +}
So it seems that your driver here does nothing but extending your device tree
definition. Correct me if I'm wrong, aren't you trying to fix a broken device
tree definition here ?
Cheers,
Samuel.
--
Intel Open Source Technology Centre
http://oss.intel.com/