Re: [PATCH v3 1/2] mfd: add stmpe-i2c driver

2012-09-04 Thread Sascha Hauer
On Tue, Sep 04, 2012 at 11:58:45AM +0200, Steffen Trumtrar wrote:
> The stmpe mfds can be connected via i2c and spi. This driver provides the 
> basic
> infrastructure for the i2c kind. It can be added as a normal i2c-device in the
> board code. To enable functions a platform_data struct has to be provided, 
> that
> describes what parts of the chip are to be used.
> 
> Signed-off-by: Steffen Trumtrar 
> ---
> +static struct stmpe_client_info i2c_ci = {
> + .read_reg = stmpe_reg_read,
> + .write_reg = stmpe_reg_write,
> +};
> +
> +static int stmpe_probe(struct device_d *dev)
> +{
> + struct stmpe_platform_data *pdata = dev->platform_data;
> + struct stmpe *stmpe_dev;
> +
> + if (!pdata) {
> + dev_dbg(dev, "no platform data\n");
> + return -ENODEV;
> + }
> +
> + stmpe_dev = xzalloc(sizeof(struct stmpe));
> + stmpe_dev->cdev.name = DRIVERNAME;
> + stmpe_dev->client = to_i2c_client(dev);
> + stmpe_dev->cdev.size = 191; /* 191 known registers */
> + stmpe_dev->cdev.dev = dev;
> + stmpe_dev->cdev.ops = &stmpe_fops;
> + stmpe_dev->pdata = pdata;
> + dev->priv = stmpe_dev;
> + i2c_ci.stmpe = stmpe_dev;

This breaks for multiple instances of a stmpe device. You have to
allocate a static struct stmpe_client_info dynamically.

> +
> + if (pdata->blocks &= STMPE_BLOCK_GPIO)
> + add_generic_device("stmpe-gpio", 0, NULL, pdata->base, 0x10, 
> IORESOURCE_MEM, &i2c_ci);

This issues:

drivers/mfd/stmpe-i2c.c: In function 'stmpe_probe':
drivers/mfd/stmpe-i2c.c:135:3: warning: passing argument 4 of 
'add_generic_device' makes integer from pointer without a cast [enabled by 
default]
include/driver.h:197:18: note: expected 'resource_size_t' but argument is of 
type 'void *'

The base and size refer to the iomem space. As an i2c device we do not
have iomem here, so this is wrong.

Looking further at it it seems to be unused by the gpio driver anyway,
so you can just pass 0, 0 as iomem and size.


> +struct stmpe_platform_data {
> + enumstmpe_revision  revision;
> + enumstmpe_blocksblocks;
> + void__iomem *base;

This is unused, please remove.

> + int gpio_base;
> +};
> +
> +
> +extern int stmpe_reg_read(struct stmpe *priv, u32 reg, u8 *val);
> +extern int stmpe_reg_write(struct stmpe *priv, u32 reg, u8 val);
> +extern int stmpe_set_bits(struct stmpe *priv, u32 reg, u8 mask, u8 val);

no 'extern' for function prototypes please.

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


Re: [PATCH v3 1/2] mfd: add stmpe-i2c driver

2012-09-04 Thread Marc Kleine-Budde
On 09/04/2012 11:58 AM, Steffen Trumtrar wrote:
> The stmpe mfds can be connected via i2c and spi. This driver provides the 
> basic
> infrastructure for the i2c kind. It can be added as a normal i2c-device in the
> board code. To enable functions a platform_data struct has to be provided, 
> that
> describes what parts of the chip are to be used.
> 
> Signed-off-by: Steffen Trumtrar 
> ---
>  drivers/mfd/Kconfig |4 ++
>  drivers/mfd/Makefile|1 +
>  drivers/mfd/stmpe-i2c.c |  153 
> +++
>  include/mfd/stmpe-i2c.h |   56 +
>  4 files changed, 214 insertions(+)
>  create mode 100644 drivers/mfd/stmpe-i2c.c
>  create mode 100644 include/mfd/stmpe-i2c.h
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index af67935..20eef86 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -33,4 +33,8 @@ config I2C_TWL6030
>   select I2C_TWLCORE
>   bool "TWL6030 driver"
>  
> +config I2C_STMPE
> + depends on I2C
> + bool "STMPE-i2c driver"
> +
>  endmenu
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index e11223b..792ae2d 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_I2C_LP3972) += lp3972.o
>  obj-$(CONFIG_I2C_TWLCORE) += twl-core.o
>  obj-$(CONFIG_I2C_TWL4030) += twl4030.o
>  obj-$(CONFIG_I2C_TWL6030) += twl6030.o
> +obj-$(CONFIG_I2C_STMPE) += stmpe-i2c.o
> diff --git a/drivers/mfd/stmpe-i2c.c b/drivers/mfd/stmpe-i2c.c
> new file mode 100644
> index 000..53b3d06
> --- /dev/null
> +++ b/drivers/mfd/stmpe-i2c.c
> @@ -0,0 +1,153 @@
> +/*
> + * Copyright (C) 2012 Pengutronix
> + * Steffen Trumtrar 
> + *
> + * 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.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +
> +#define DRIVERNAME   "stmpe-i2c"
> +
> +#define to_stmpe(a)  container_of(a, struct stmpe, cdev)
> +
> +int stmpe_reg_read(struct stmpe *stmpe, u32 reg, u8 *val)
> +{
> + int ret;
> +
> + ret = i2c_read_reg(stmpe->client, reg, val, 1);
> +
> + return ret == 1 ? 0 : ret;
> +}
> +EXPORT_SYMBOL(stmpe_reg_read)
> +
> +int stmpe_reg_write(struct stmpe *stmpe, u32 reg, u8 val)
> +{
> + int ret;
> +
> + ret = i2c_write_reg(stmpe->client, reg, &val, 1);
> +
> + return ret == 1 ? 0 : ret;
> +}
> +EXPORT_SYMBOL(stmpe_reg_write)
> +
> +int stmpe_set_bits(struct stmpe *stmpe, u32 reg, u8 mask, u8 val)
> +{
> + u8 tmp;
> + int err;
> +
> + err = stmpe_reg_read(stmpe, reg, &tmp);
> + tmp = (tmp & ~mask) | val;
> +
> + if (!err)
> + err = stmpe_reg_write(stmpe, reg, tmp);
> +
> + return err;
> +}
> +EXPORT_SYMBOL(stmpe_set_bits);
> +
> +static ssize_t stmpe_read(struct cdev *cdev, void *_buf, size_t count, 
> loff_t offset, ulong flags)
> +{
> + struct stmpe *stmpe = to_stmpe(cdev);
> + u8 *buf = _buf;
> + size_t i = count;
> + int err;
> +
> + while (i) {
> + err = stmpe_reg_read(stmpe, offset, buf);
> + if (err)
> + return (ssize_t)err;
> + buf++;
> + i--;
> + offset++;
> + }
> +
> + return count;
> +}
> +
> +static ssize_t stmpe_write(struct cdev *cdev, const void *_buf, size_t 
> count, loff_t offset, ulong flags)
> +{
> + struct stmpe *stmpe = to_stmpe(cdev);
> + const u8 *buf = _buf;
> + size_t i = count;
> + int err;
> +
> + while (i) {
> + err = stmpe_reg_write(stmpe, offset, *buf);
> + if (err)
> + return (ssize_t)err;
> + buf++;
> + i--;
> + offset++;
> + }
> +
> + return count;
> +}
> +
> +static struct file_operations stmpe_fops = {
> + .lseek  = dev_lseek_default,
> + .read   = stmpe_read,
> + .write  = stmpe_write,
> +};
> +
> +static struct stmpe_client_info i2c_ci = {
> + .read_reg = stmpe_reg_read,
> + .write_reg = stmpe_reg_write,
> +};
> +
> +static int stmpe_probe(struct device_d *dev)
> +{
> + struct stmpe_platform_data *pdata = dev->platform_data;
> + struct stmpe *stmpe_dev;
> +
> + if (!pdata) {
> + dev_dbg(dev, "no platform data\n");
> + return -ENODEV;
> + }
> +
> + stmpe_dev = xzalloc(sizeof(struct stmpe));
> + stmpe_dev->cdev.name = DRIVERNAME;
> + stmpe_dev->client = to_i2c_client(dev);
> + stmpe_dev->cdev.size = 191; /

[PATCH v3 1/2] mfd: add stmpe-i2c driver

2012-09-04 Thread Steffen Trumtrar
The stmpe mfds can be connected via i2c and spi. This driver provides the basic
infrastructure for the i2c kind. It can be added as a normal i2c-device in the
board code. To enable functions a platform_data struct has to be provided, that
describes what parts of the chip are to be used.

Signed-off-by: Steffen Trumtrar 
---
 drivers/mfd/Kconfig |4 ++
 drivers/mfd/Makefile|1 +
 drivers/mfd/stmpe-i2c.c |  153 +++
 include/mfd/stmpe-i2c.h |   56 +
 4 files changed, 214 insertions(+)
 create mode 100644 drivers/mfd/stmpe-i2c.c
 create mode 100644 include/mfd/stmpe-i2c.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index af67935..20eef86 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -33,4 +33,8 @@ config I2C_TWL6030
select I2C_TWLCORE
bool "TWL6030 driver"
 
+config I2C_STMPE
+   depends on I2C
+   bool "STMPE-i2c driver"
+
 endmenu
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index e11223b..792ae2d 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_I2C_LP3972) += lp3972.o
 obj-$(CONFIG_I2C_TWLCORE) += twl-core.o
 obj-$(CONFIG_I2C_TWL4030) += twl4030.o
 obj-$(CONFIG_I2C_TWL6030) += twl6030.o
+obj-$(CONFIG_I2C_STMPE) += stmpe-i2c.o
diff --git a/drivers/mfd/stmpe-i2c.c b/drivers/mfd/stmpe-i2c.c
new file mode 100644
index 000..53b3d06
--- /dev/null
+++ b/drivers/mfd/stmpe-i2c.c
@@ -0,0 +1,153 @@
+/*
+ * Copyright (C) 2012 Pengutronix
+ * Steffen Trumtrar 
+ *
+ * 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.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#define DRIVERNAME "stmpe-i2c"
+
+#define to_stmpe(a)container_of(a, struct stmpe, cdev)
+
+int stmpe_reg_read(struct stmpe *stmpe, u32 reg, u8 *val)
+{
+   int ret;
+
+   ret = i2c_read_reg(stmpe->client, reg, val, 1);
+
+   return ret == 1 ? 0 : ret;
+}
+EXPORT_SYMBOL(stmpe_reg_read)
+
+int stmpe_reg_write(struct stmpe *stmpe, u32 reg, u8 val)
+{
+   int ret;
+
+   ret = i2c_write_reg(stmpe->client, reg, &val, 1);
+
+   return ret == 1 ? 0 : ret;
+}
+EXPORT_SYMBOL(stmpe_reg_write)
+
+int stmpe_set_bits(struct stmpe *stmpe, u32 reg, u8 mask, u8 val)
+{
+   u8 tmp;
+   int err;
+
+   err = stmpe_reg_read(stmpe, reg, &tmp);
+   tmp = (tmp & ~mask) | val;
+
+   if (!err)
+   err = stmpe_reg_write(stmpe, reg, tmp);
+
+   return err;
+}
+EXPORT_SYMBOL(stmpe_set_bits);
+
+static ssize_t stmpe_read(struct cdev *cdev, void *_buf, size_t count, loff_t 
offset, ulong flags)
+{
+   struct stmpe *stmpe = to_stmpe(cdev);
+   u8 *buf = _buf;
+   size_t i = count;
+   int err;
+
+   while (i) {
+   err = stmpe_reg_read(stmpe, offset, buf);
+   if (err)
+   return (ssize_t)err;
+   buf++;
+   i--;
+   offset++;
+   }
+
+   return count;
+}
+
+static ssize_t stmpe_write(struct cdev *cdev, const void *_buf, size_t count, 
loff_t offset, ulong flags)
+{
+   struct stmpe *stmpe = to_stmpe(cdev);
+   const u8 *buf = _buf;
+   size_t i = count;
+   int err;
+
+   while (i) {
+   err = stmpe_reg_write(stmpe, offset, *buf);
+   if (err)
+   return (ssize_t)err;
+   buf++;
+   i--;
+   offset++;
+   }
+
+   return count;
+}
+
+static struct file_operations stmpe_fops = {
+   .lseek  = dev_lseek_default,
+   .read   = stmpe_read,
+   .write  = stmpe_write,
+};
+
+static struct stmpe_client_info i2c_ci = {
+   .read_reg = stmpe_reg_read,
+   .write_reg = stmpe_reg_write,
+};
+
+static int stmpe_probe(struct device_d *dev)
+{
+   struct stmpe_platform_data *pdata = dev->platform_data;
+   struct stmpe *stmpe_dev;
+
+   if (!pdata) {
+   dev_dbg(dev, "no platform data\n");
+   return -ENODEV;
+   }
+
+   stmpe_dev = xzalloc(sizeof(struct stmpe));
+   stmpe_dev->cdev.name = DRIVERNAME;
+   stmpe_dev->client = to_i2c_client(dev);
+   stmpe_dev->cdev.size = 191; /* 191 known registers */
+   stmpe_dev->cdev.dev = dev;
+   stmpe_dev->cdev.ops = &stmpe_fops;
+   stmpe_dev->pdata = pdata;
+   dev->priv = stmpe_dev;
+   i2c_ci.stmpe = stmpe_dev;
+
+   if (pdata->blocks &= STMPE_BLOCK_GPIO)
+   add_generic_de