Re: [PATCH 4/4]-V2 Add MFD driver for TPS6507x family of multi-function chips and move TPS6507x regulator driver from being stand-alone to using the MFD driver.

2010-04-05 Thread Mark Brown
On Fri, Apr 02, 2010 at 03:37:33PM -0600, Todd Fischer wrote:
 Add MFD driver for TPS6507x family of multi-function chips.  Move TPS6507x 
 regulator driver from being stand-alone driver to using the MFD TPS6507x 
 driver.
 
 Signed-off-by: Todd Fischer todd.fisc...@ridgerun.com

One issue...

 +static int tps6507x_i2c_read_device(struct tps6507x_dev *tps6507x, char reg,
 +   int bytes, void *dest)
 +{
 + struct i2c_client *i2c = tps6507x-i2c_client;
 + int ret;
 +
 + ret = i2c_master_send(i2c, reg, 1);
 + if (ret  0)
 + return ret;
 +
 + ret = i2c_master_recv(i2c, dest, bytes);
 + if (ret  0)
 + return ret;
 + if (ret != bytes)
 + return -EIO;
 + return 0;
 +}

Your register I/O functions don't have anything protecting them against
concurrent access.  This isn't really an issue for the writes by
themselves since they do a single transaction on the I2C bus so the I2C
layer concurrency protection ought to be enough but for reads you need
to send the register address first then read back the data, opening up
an issue.  A simple mutex in the read and write functions ought to cover
this.
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source


[PATCH 4/4]-V2 Add MFD driver for TPS6507x family of multi-function chips and move TPS6507x regulator driver from being stand-alone to using the MFD driver.

2010-04-02 Thread Todd Fischer
Add MFD driver for TPS6507x family of multi-function chips.  Move TPS6507x 
regulator driver from being stand-alone driver to using the MFD TPS6507x driver.

Signed-off-by: Todd Fischer todd.fisc...@ridgerun.com
---
 drivers/mfd/Kconfig|   12 +++
 drivers/mfd/Makefile   |3 +-
 drivers/mfd/tps6507x.c |  145 ++
 drivers/regulator/tps6507x-regulator.c |  153 +++
 include/linux/mfd/tps6507x.h   |   22 +
 5 files changed, 255 insertions(+), 80 deletions(-)
 create mode 100644 drivers/mfd/tps6507x.c

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 2a5a0b7..1e301ad 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -111,6 +111,18 @@ config TPS65010
  This driver can also be built as a module.  If so, the module
  will be called tps65010.
 
+config TPS6507X
+   tristate TPS6507x Power Management / Touch Screen chips
+   select MFD_CORE
+   depends on I2C
+   help
+ If you say yes here you get support for the TPS6507x series of
+ Power Management / Touch Screen chips.  These include voltage
+ regulators, lithium ion/polymer battery charging, touch screen
+ and other features that are often used in portable devices.
+ This driver can also be built as a module.  If so, the module
+ will be called tps6507x.
+
 config MENELAUS
bool Texas Instruments TWL92330/Menelaus PM chip
depends on I2C=y  ARCH_OMAP2
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 22715ad..2bebc95 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_MFD_WM8350_I2C)  += wm8350-i2c.o
 obj-$(CONFIG_MFD_WM8994)   += wm8994-core.o
 
 obj-$(CONFIG_TPS65010) += tps65010.o
+obj-$(CONFIG_TPS6507X) += tps6507x.o
 obj-$(CONFIG_MENELAUS) += menelaus.o
 
 obj-$(CONFIG_TWL4030_CORE) += twl-core.o twl4030-irq.o twl6030-irq.o
@@ -62,4 +63,4 @@ obj-$(CONFIG_AB3100_OTP)  += ab3100-otp.o
 obj-$(CONFIG_AB4500_CORE)  += ab4500-core.o
 obj-$(CONFIG_MFD_TIMBERDALE)+= timberdale.o
 obj-$(CONFIG_PMIC_ADP5520) += adp5520.o
-obj-$(CONFIG_LPC_SCH)  += lpc_sch.o
\ No newline at end of file
+obj-$(CONFIG_LPC_SCH)  += lpc_sch.o
diff --git a/drivers/mfd/tps6507x.c b/drivers/mfd/tps6507x.c
new file mode 100644
index 000..edda9ff
--- /dev/null
+++ b/drivers/mfd/tps6507x.c
@@ -0,0 +1,145 @@
+/*
+ * tps6507x.c  --  TPS6507x chip family multi-function driver
+ *
+ *  Copyright (c) 2010 RidgeRun (todd.fisc...@ridgerun.com)
+ *
+ * Author: Todd Fischer
+ * todd.fisc...@ridgerun.com
+ *
+ * Credits:
+ *
+ *Using code from wm831x-*.c, Wolfson Microelectronics PLC.
+ *
+ * For licencing details see kernel-base/COPYING
+ *
+ */
+
+#include linux/module.h
+#include linux/moduleparam.h
+#include linux/init.h
+#include linux/i2c.h
+#include linux/mfd/core.h
+#include linux/mfd/tps6507x.h
+
+static struct mfd_cell tps6507x_devs[] = {
+   {
+   .name = tps6507x-pmic,
+   },
+};
+
+
+static int tps6507x_i2c_read_device(struct tps6507x_dev *tps6507x, char reg,
+ int bytes, void *dest)
+{
+   struct i2c_client *i2c = tps6507x-i2c_client;
+   int ret;
+
+   ret = i2c_master_send(i2c, reg, 1);
+   if (ret  0)
+   return ret;
+
+   ret = i2c_master_recv(i2c, dest, bytes);
+   if (ret  0)
+   return ret;
+   if (ret != bytes)
+   return -EIO;
+   return 0;
+}
+
+static int tps6507x_i2c_write_device(struct tps6507x_dev *tps6507x, char reg,
+  int bytes, void *src)
+{
+   struct i2c_client *i2c = tps6507x-i2c_client;
+   /* we add 1 byte for device register */
+   u8 msg[(TPS6507X_MAX_REGISTER  1) + 1];
+   int ret;
+
+   if (bytes  ((TPS6507X_MAX_REGISTER  1) + 1))
+   return -EINVAL;
+
+   msg[0] = reg;
+   memcpy(msg[1], src, bytes);
+
+   ret = i2c_master_send(i2c, msg, bytes + 1);
+   if (ret  0)
+   return ret;
+   if (ret != bytes + 1)
+   return -EIO;
+   return 0;
+}
+
+static int tps6507x_i2c_probe(struct i2c_client *i2c,
+   const struct i2c_device_id *id)
+{
+   struct tps6507x_dev *tps6507x;
+   int ret = 0;
+
+   tps6507x = kzalloc(sizeof(struct tps6507x_dev), GFP_KERNEL);
+   if (tps6507x == NULL) {
+   kfree(i2c);
+   return -ENOMEM;
+   }
+
+   i2c_set_clientdata(i2c, tps6507x);
+   tps6507x-dev = i2c-dev;
+   tps6507x-i2c_client = i2c;
+   tps6507x-read_dev = tps6507x_i2c_read_device;
+   tps6507x-write_dev = tps6507x_i2c_write_device;
+
+   ret = mfd_add_devices(tps6507x-dev, -1,
+ tps6507x_devs, ARRAY_SIZE(tps6507x_devs),
+ NULL, 0);
+
+