2011-05-05 00:21:57

by Jorge Eduardo Candelaria

[permalink] [raw]
Subject: [PATCHv2 5/5] TPS65911: Comparator: Add comparator driver

This driver adds functionality to the tps65911 chip driver.

Two of the comparators are configurable by software and measures
VCCS voltage to detect high or low voltage scenarios.

Signed-off-by: Jorge Eduardo Candelaria <[email protected]>
---
drivers/mfd/Kconfig | 7 ++
drivers/mfd/Makefile | 1 +
drivers/mfd/tps65911-comparator.c | 188 +++++++++++++++++++++++++++++++++++++
include/linux/mfd/tps65910.h | 4 +
4 files changed, 200 insertions(+), 0 deletions(-)
create mode 100644 drivers/mfd/tps65911-comparator.c

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 65930a7..8c748f4 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -679,6 +679,13 @@ config MFD_TPS65910
if you say yes here you get support for the TPS65910 series of
Power Management chips.

+config TPS65911_COMPARATOR
+ tristate "TPS65911 Comparator"
+ depends on MFD_TPS65910
+ help
+ if you say yes here you get support for the TPS65910 comparator
+ module.
+
endif # MFD_SUPPORT

menu "Multimedia Capabilities Port drivers"
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index c771eed..320de3e 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -89,3 +89,4 @@ obj-$(CONFIG_MFD_WL1273_CORE) += wl1273-core.o
obj-$(CONFIG_MFD_CS5535) += cs5535-mfd.o
obj-$(CONFIG_MFD_OMAP_USB_HOST) += omap-usb-host.o
obj-$(CONFIG_MFD_TPS65910) += tps65910.o tps65910-gpio.o tps65910-irq.o
+obj-$(CONFIG_TPS65911_COMP) += tps65911-comparator.o
diff --git a/drivers/mfd/tps65911-comparator.c b/drivers/mfd/tps65911-comparator.c
new file mode 100644
index 0000000..b8b985b
--- /dev/null
+++ b/drivers/mfd/tps65911-comparator.c
@@ -0,0 +1,188 @@
+/*
+ * tps65910.c -- TI TPS6591x
+ *
+ * Copyright 2010 Texas Instruments Inc.
+ *
+ * Author: Jorge Eduardo Candelaria <[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/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/debugfs.h>
+#include <linux/gpio.h>
+#include <linux/mfd/tps65910.h>
+
+#define COMP 0
+#define COMP1 1
+#define COMP2 2
+
+/* Comparator 1 voltage selection table in milivolts */
+static const u16 COMP_VSEL_TABLE[] = {
+ 0, 2500, 2500, 2500, 2500, 2550, 2600, 2650,
+ 2700, 2750, 2800, 2850, 2900, 2950, 3000, 3050,
+ 3100, 3150, 3200, 3250, 3300, 3350, 3400, 3450,
+ 3500,
+};
+
+struct comparator {
+ const char *name;
+ int reg;
+ int uV_max;
+ const u16 *vsel_table;
+};
+
+static struct comparator tps_comparators[] = {
+ {
+ .name = "COMP1",
+ .reg = TPS65911_VMBCH,
+ .uV_max = 3500,
+ .vsel_table = COMP_VSEL_TABLE,
+ },
+ {
+ .name = "COMP2",
+ .reg = TPS65911_VMBCH2,
+ .uV_max = 3500,
+ .vsel_table = COMP_VSEL_TABLE,
+ },
+};
+
+static int comp_threshold_set(struct tps65910 *tps65910, int id, int voltage)
+{
+ struct comparator tps_comp = tps_comparators[id];
+ int curr_voltage = 0;
+ int ret;
+ u8 index = 0, val;
+
+ if (id == COMP)
+ return 0;
+
+ while (curr_voltage < tps_comp.uV_max) {
+ curr_voltage = tps_comp.vsel_table[index];
+ if (curr_voltage >= voltage)
+ break;
+ else if (curr_voltage < voltage)
+ index ++;
+ }
+
+ if (curr_voltage > tps_comp.uV_max)
+ return -EINVAL;
+
+ val = index << 1;
+ ret = tps65910->write(tps65910, tps_comp.reg, 1, &val);
+
+ return ret;
+}
+
+static int comp_threshold_get(struct tps65910 *tps65910, int id)
+{
+ struct comparator tps_comp = tps_comparators[id];
+ int ret;
+ u8 val;
+
+ if (id == COMP)
+ return 0;
+
+ ret = tps65910->read(tps65910, tps_comp.reg, 1, &val);
+ if (ret < 0)
+ return ret;
+
+ val >>= 1;
+ return tps_comp.vsel_table[val];
+}
+
+static ssize_t comp_threshold_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct tps65910 *tps65910 = dev_get_drvdata(dev->parent);
+ struct attribute comp_attr = attr->attr;
+ int id, uVolt;
+
+ if (!strcmp(comp_attr.name, "comp1_threshold"))
+ id = COMP1;
+ else if (!strcmp(comp_attr.name, "comp2_threshold"))
+ id = COMP2;
+ else
+ return -EINVAL;
+
+ uVolt = comp_threshold_get(tps65910, id);
+
+ return sprintf(buf, "%d\n", uVolt);
+}
+
+static DEVICE_ATTR(comp1_threshold, S_IRUGO, comp_threshold_show, NULL);
+static DEVICE_ATTR(comp2_threshold, S_IRUGO, comp_threshold_show, NULL);
+
+static __devinit int tps65911_comparator_probe(struct platform_device *pdev)
+{
+ struct tps65910 *tps65910 = dev_get_drvdata(pdev->dev.parent);
+ struct tps65910_platform_data *pdata = dev_get_platdata(tps65910->dev);
+ int ret;
+
+ ret = comp_threshold_set(tps65910, COMP1, pdata->vmbch_threshold);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "cannot set COMP1 threshold\n");
+ return ret;
+ }
+
+ ret = comp_threshold_set(tps65910, COMP2, pdata->vmbch2_threshold);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "cannot set COMP2 theshold\n");
+ return ret;
+ }
+
+ /* Create sysfs entry */
+ ret = device_create_file(&pdev->dev, &dev_attr_comp1_threshold);
+ if (ret < 0)
+ dev_err(&pdev->dev, "failed to add COMP1 sysfs file\n");
+
+ ret = device_create_file(&pdev->dev, &dev_attr_comp2_threshold);
+ if (ret < 0)
+ dev_err(&pdev->dev, "failed to add COMP2 sysfs file\n");
+
+ return ret;
+}
+
+static __devexit int tps65911_comparator_remove(struct platform_device *pdev)
+{
+ struct tps65910 *tps65910;
+
+ tps65910 = dev_get_drvdata(pdev->dev.parent);
+
+ return 0;
+}
+
+static struct platform_driver tps65911_comparator_driver = {
+ .driver = {
+ .name = "tps65911-comparator",
+ .owner = THIS_MODULE,
+ },
+ .probe = tps65911_comparator_probe,
+ .remove = __devexit_p(tps65911_comparator_remove),
+};
+
+static int __init tps65911_comparator_init(void)
+{
+ return platform_driver_register(&tps65911_comparator_driver);
+}
+subsys_initcall(tps65911_comparator_init);
+
+static void __exit tps65911_comparator_exit(void)
+{
+ platform_driver_unregister(&tps65911_comparator_driver);
+}
+module_exit(tps65911_comparator_exit);
+
+MODULE_AUTHOR("Jorge Eduardo Candelaria <[email protected]>");
+MODULE_DESCRIPTION("TPS65911 comparator driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:tps65911-comparator");
diff --git a/include/linux/mfd/tps65910.h b/include/linux/mfd/tps65910.h
index 5f77006..8bb85b9 100644
--- a/include/linux/mfd/tps65910.h
+++ b/include/linux/mfd/tps65910.h
@@ -121,6 +121,8 @@
#define TPS65911_LDO6 0x35
#define TPS65911_LDO4 0x36
#define TPS65911_LDO3 0x37
+#define TPS65911_VMBCH 0x6A
+#define TPS65911_VMBCH2 0x6B

/*
* List of register bitfields for component TPS65910
@@ -746,6 +748,8 @@ struct tps65910_board {
int gpio_base;
int irq;
int irq_base;
+ int vmbch_threshold;
+ int vmbch2_threshold;
struct regulator_init_data *tps65910_pmic_init_data;
};

--
1.7.1


2011-05-13 16:59:36

by Samuel Ortiz

[permalink] [raw]
Subject: Re: [PATCHv2 5/5] TPS65911: Comparator: Add comparator driver

Hi Jorge,

On Wed, May 04, 2011 at 07:21:46PM -0500, Jorge Eduardo Candelaria wrote:
> This driver adds functionality to the tps65911 chip driver.
>
> Two of the comparators are configurable by software and measures
> VCCS voltage to detect high or low voltage scenarios.
A few comments:

> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 65930a7..8c748f4 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -679,6 +679,13 @@ config MFD_TPS65910
> if you say yes here you get support for the TPS65910 series of
> Power Management chips.
>
> +config TPS65911_COMPARATOR
> + tristate "TPS65911 Comparator"
> + depends on MFD_TPS65910
> + help
> + if you say yes here you get support for the TPS65910 comparator
> + module.
> +
I still don't know what it compares and why I would want to enable that.


> endif # MFD_SUPPORT
>
> menu "Multimedia Capabilities Port drivers"
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index c771eed..320de3e 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -89,3 +89,4 @@ obj-$(CONFIG_MFD_WL1273_CORE) += wl1273-core.o
> obj-$(CONFIG_MFD_CS5535) += cs5535-mfd.o
> obj-$(CONFIG_MFD_OMAP_USB_HOST) += omap-usb-host.o
> obj-$(CONFIG_MFD_TPS65910) += tps65910.o tps65910-gpio.o tps65910-irq.o
> +obj-$(CONFIG_TPS65911_COMP) += tps65911-comparator.o
> diff --git a/drivers/mfd/tps65911-comparator.c b/drivers/mfd/tps65911-comparator.c
> new file mode 100644
> index 0000000..b8b985b
> --- /dev/null
> +++ b/drivers/mfd/tps65911-comparator.c
> @@ -0,0 +1,188 @@
> +/*
> + * tps65910.c -- TI TPS6591x
> + *
> + * Copyright 2010 Texas Instruments Inc.
> + *
> + * Author: Jorge Eduardo Candelaria <[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/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/debugfs.h>
> +#include <linux/gpio.h>
> +#include <linux/mfd/tps65910.h>
> +
> +#define COMP 0
> +#define COMP1 1
> +#define COMP2 2
> +
> +/* Comparator 1 voltage selection table in milivolts */
> +static const u16 COMP_VSEL_TABLE[] = {
> + 0, 2500, 2500, 2500, 2500, 2550, 2600, 2650,
> + 2700, 2750, 2800, 2850, 2900, 2950, 3000, 3050,
> + 3100, 3150, 3200, 3250, 3300, 3350, 3400, 3450,
> + 3500,
> +};
> +
> +struct comparator {
> + const char *name;
> + int reg;
> + int uV_max;
> + const u16 *vsel_table;
> +};
> +
> +static struct comparator tps_comparators[] = {
> + {
> + .name = "COMP1",
> + .reg = TPS65911_VMBCH,
> + .uV_max = 3500,
> + .vsel_table = COMP_VSEL_TABLE,
> + },
> + {
> + .name = "COMP2",
> + .reg = TPS65911_VMBCH2,
> + .uV_max = 3500,
> + .vsel_table = COMP_VSEL_TABLE,
> + },
> +};
This driver really looks like it could belong to drivers/regulator.

Cheers,
Samuel.

--
Intel Open Source Technology Centre
http://oss.intel.com/

2011-05-14 22:29:45

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCHv2 5/5] TPS65911: Comparator: Add comparator driver

On Fri, May 13, 2011 at 06:59:32PM +0200, Samuel Ortiz wrote:
> On Wed, May 04, 2011 at 07:21:46PM -0500, Jorge Eduardo Candelaria wrote:

> > +config TPS65911_COMPARATOR
> > + tristate "TPS65911 Comparator"
> > + depends on MFD_TPS65910
> > + help
> > + if you say yes here you get support for the TPS65910 comparator
> > + module.

> I still don't know what it compares and why I would want to enable that.

It'll be comparing two voltages - it's quite a common feature for PMICs
with auxadcs. I think I said in one of my previous reviews that it
probably shouldn't be user visible in Kconfig as some other driver will
need to be written to call it so that driver could just select the
symbol.

> This driver really looks like it could belong to drivers/regulator.

It's measuring rather than producing.

2011-05-22 20:41:11

by Samuel Ortiz

[permalink] [raw]
Subject: Re: [PATCHv2 5/5] TPS65911: Comparator: Add comparator driver

On Sat, May 14, 2011 at 03:29:45PM -0700, Mark Brown wrote:
> On Fri, May 13, 2011 at 06:59:32PM +0200, Samuel Ortiz wrote:
> > On Wed, May 04, 2011 at 07:21:46PM -0500, Jorge Eduardo Candelaria wrote:
>
> > > +config TPS65911_COMPARATOR
> > > + tristate "TPS65911 Comparator"
> > > + depends on MFD_TPS65910
> > > + help
> > > + if you say yes here you get support for the TPS65910 comparator
> > > + module.
>
> > I still don't know what it compares and why I would want to enable that.
>
> It'll be comparing two voltages - it's quite a common feature for PMICs
> with auxadcs. I think I said in one of my previous reviews that it
> probably shouldn't be user visible in Kconfig as some other driver will
> need to be written to call it so that driver could just select the
> symbol.
>
> > This driver really looks like it could belong to drivers/regulator.
>
> It's measuring rather than producing.
Sure, but I still don't see how it belongs to drivers/mfd/.

Cheers,
Samuel.

--
Intel Open Source Technology Centre
http://oss.intel.com/

2011-05-22 22:46:31

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCHv2 5/5] TPS65911: Comparator: Add comparator driver

On Sun, May 22, 2011 at 10:40:57PM +0200, Samuel Ortiz wrote:
> On Sat, May 14, 2011 at 03:29:45PM -0700, Mark Brown wrote:

> > It's measuring rather than producing.
> Sure, but I still don't see how it belongs to drivers/mfd/.

It doesn't, that was my point about presenting a generic API for this.
It's just ending up in drivers/mfd for want of a better place to put it.