mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 1/3] i2c-imx: change log level for No ACK
@ 2010-05-18  9:26 Eric Bénard
  2010-05-18  9:26 ` [PATCH 2/3] i2c: add i2cdev driver Eric Bénard
  0 siblings, 1 reply; 7+ messages in thread
From: Eric Bénard @ 2010-05-18  9:26 UTC (permalink / raw)
  To: barebox

Signed-off-by: Eric Bénard <eric@eukrea.com>
---
 drivers/i2c/i2c-imx.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/i2c/i2c-imx.c b/drivers/i2c/i2c-imx.c
index 03e3785..cc64d94 100644
--- a/drivers/i2c/i2c-imx.c
+++ b/drivers/i2c/i2c-imx.c
@@ -214,7 +214,7 @@ static int i2c_imx_acked(struct i2c_adapter *adapter)
 			break;
 
 		if (is_timeout(start, MSECOND)) {
-			dev_err(adapter->dev, "<%s> No ACK\n", __func__);
+			dev_dbg(adapter->dev, "<%s> No ACK\n", __func__);
 			return -EIO;
 		}
 	}
-- 
1.6.3.3


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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/3] i2c: add i2cdev driver
  2010-05-18  9:26 [PATCH 1/3] i2c-imx: change log level for No ACK Eric Bénard
@ 2010-05-18  9:26 ` Eric Bénard
  2010-05-18  9:26   ` [PATCH 3/3] commands: add i2c commands Eric Bénard
  2010-05-19 11:01   ` [PATCH 2/3] i2c: add i2cdev driver Sascha Hauer
  0 siblings, 2 replies; 7+ messages in thread
From: Eric Bénard @ 2010-05-18  9:26 UTC (permalink / raw)
  To: barebox

this driver is needed to introduce i2c commands

Signed-off-by: Eric Bénard <eric@eukrea.com>
---
 drivers/i2c/Kconfig  |    3 ++
 drivers/i2c/Makefile |    1 +
 drivers/i2c/i2cdev.c |   83 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 87 insertions(+), 0 deletions(-)
 create mode 100644 drivers/i2c/i2cdev.c

diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
index f1b2949..cbd2464 100644
--- a/drivers/i2c/Kconfig
+++ b/drivers/i2c/Kconfig
@@ -16,4 +16,7 @@ config DRIVER_I2C_MC9SDZ60
 config DRIVER_I2C_LP3972
 	bool "LP3972 driver"
 
+config DRIVER_I2C_I2CDEV
+	bool "I2CDEV driver"
+
 endif
diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
index 62d030b..a193f9c 100644
--- a/drivers/i2c/Makefile
+++ b/drivers/i2c/Makefile
@@ -5,3 +5,4 @@ obj-$(CONFIG_DRIVER_I2C_IMX) += i2c-imx.o
 obj-$(CONFIG_DRIVER_I2C_MC13892) += mc13892.o
 obj-$(CONFIG_DRIVER_I2C_MC9SDZ60) += mc9sdz60.o
 obj-$(CONFIG_DRIVER_I2C_LP3972) += lp3972.o
+obj-$(CONFIG_DRIVER_I2C_I2CDEV) += i2cdev.o
diff --git a/drivers/i2c/i2cdev.c b/drivers/i2c/i2cdev.c
new file mode 100644
index 0000000..194db6c
--- /dev/null
+++ b/drivers/i2c/i2cdev.c
@@ -0,0 +1,83 @@
+/*
+ * Copyright (C) 2007 Sascha Hauer, Pengutronix
+ *               2009 Marc Kleine-Budde <mkl@pengutronix.de>
+ *               2009 Eric Benard <eric@eukrea.com>
+ *
+ * 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
+ *
+ */
+
+#include <common.h>
+#include <init.h>
+#include <driver.h>
+#include <xfuncs.h>
+#include <errno.h>
+
+#include <i2c/i2c.h>
+
+#include <asm/byteorder.h>
+
+#define DRIVERNAME		"i2cdev"
+
+struct i2cdev_priv {
+	struct cdev		cdev;
+	struct i2c_client	*client;
+};
+
+#define to_i2cdev_priv(a)	container_of(a, struct i2cdev_priv, cdev)
+
+static struct i2cdev_priv *i2cdev_dev;
+
+struct i2c_client *i2cdev_get_client(void)
+{
+	if (!i2cdev_dev)
+		return NULL;
+
+	return i2cdev_dev->client;
+}
+
+static struct file_operations i2cdev_fops = {
+};
+
+static int i2cdev_probe(struct device_d *dev)
+{
+	if (i2cdev_dev)
+		return -EBUSY;
+
+	i2cdev_dev = xzalloc(sizeof(struct i2cdev_priv));
+	i2cdev_dev->cdev.name = DRIVERNAME;
+	i2cdev_dev->client = to_i2c_client(dev);
+	i2cdev_dev->cdev.size = 256;
+	i2cdev_dev->cdev.dev = dev;
+	i2cdev_dev->cdev.ops = &i2cdev_fops;
+
+	devfs_create(&i2cdev_dev->cdev);
+
+	return 0;
+}
+
+static struct driver_d i2cdev_driver = {
+	.name  = DRIVERNAME,
+	.probe = i2cdev_probe,
+};
+
+static int i2cdev_init(void)
+{
+	register_driver(&i2cdev_driver);
+	return 0;
+}
+
+device_initcall(i2cdev_init);
-- 
1.6.3.3


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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 3/3] commands: add i2c commands
  2010-05-18  9:26 ` [PATCH 2/3] i2c: add i2cdev driver Eric Bénard
@ 2010-05-18  9:26   ` Eric Bénard
  2010-05-18 10:04     ` Eric Bénard
  2010-05-19 10:17     ` Sascha Hauer
  2010-05-19 11:01   ` [PATCH 2/3] i2c: add i2cdev driver Sascha Hauer
  1 sibling, 2 replies; 7+ messages in thread
From: Eric Bénard @ 2010-05-18  9:26 UTC (permalink / raw)
  To: barebox

theses commands allow low level access to i2c bus and can be useful
to setup an i2c device without having to add code, compile and flash
barebox.

Signed-off-by: Eric Bénard <eric@eukrea.com>
---
 commands/Kconfig  |    8 ++
 commands/Makefile |    1 +
 commands/i2c.c    |  233 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 242 insertions(+), 0 deletions(-)
 create mode 100644 commands/i2c.c

diff --git a/commands/Kconfig b/commands/Kconfig
index 0c09f91..7115b18 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -314,4 +314,12 @@ config CMD_UNLZO
 	  Say yes here to get the unlzo command. lzo is a fast compression
 	  algorithm by Markus Franz Xaver Johannes Oberhumer.
 
+config CMD_I2C
+	bool
+	depends on DRIVER_I2C_I2CDEV
+	prompt "i2c commands"
+	help
+	  include i2c_probe, i2c_read and i2c_write commands to communicate
+	  on i2c bus.
+
 endmenu
diff --git a/commands/Makefile b/commands/Makefile
index 74b0994..3eef5de 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -48,3 +48,4 @@ obj-$(CONFIG_CMD_BMP)		+= bmp.o
 obj-$(CONFIG_USB_GADGET_DFU)	+= dfu.o
 obj-$(CONFIG_CMD_GPIO)		+= gpio.o
 obj-$(CONFIG_CMD_UNLZO)		+= unlzo.o
+obj-$(CONFIG_CMD_I2C)		+= i2c.o
diff --git a/commands/i2c.c b/commands/i2c.c
new file mode 100644
index 0000000..b4152eb
--- /dev/null
+++ b/commands/i2c.c
@@ -0,0 +1,233 @@
+/*
+ * i2c.c - i2c commands
+ *
+ * Copyright (c) 2010 Eric Bénard <eric@eukrea.Com>, Eukréa Electromatique
+ *
+ * originals hex1_bin and hex2_bin are
+ * (C) Copyright 2000 Wolfgang Denk, DENX Software Engineering, wd@denx.de.
+ *
+ * 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
+ */
+
+#include <common.h>
+#include <command.h>
+#include <errno.h>
+#include <malloc.h>
+#include <getopt.h>
+#include <i2c/i2c.h>
+
+struct i2c_client	*client;
+extern struct i2c_client *i2cdev_get_client(void);
+
+static int hex1_bin(char c)
+{
+	if (c >= '0' && c <= '9')
+		return c - '0';
+	if (c >= 'a' && c <= 'f')
+		return c + 10 - 'a';
+	if (c >= 'A' && c <= 'F')
+		return c + 10 - 'A';
+	return -1;
+}
+
+static int hex2_bin(char *s)
+{
+	int i, j;
+
+	if (*s == '0' && (*(s+1) == 'x'))
+		s += 2;
+	i = hex1_bin(*s++);
+	if (i < 0)
+		return -1;
+	j = hex1_bin(*s);
+	if (j < 0)
+		return -1;
+	return (i<<4) + j;
+}
+
+static int do_i2c_probe(struct command *cmdtp, int argc, char *argv[])
+{
+	struct i2c_client *client;
+	int startaddr = -1, stopaddr = -1, addr, ret;
+	u8 reg;
+
+	if (argc < 3)
+		return COMMAND_ERROR_USAGE;
+
+	startaddr = hex2_bin(argv[1]);
+	stopaddr = hex2_bin(argv[2]);
+	if ((startaddr == -1) || (stopaddr == -1))
+		return COMMAND_ERROR_USAGE;
+
+	client = i2cdev_get_client();
+	if (!client)
+		return -ENODEV;
+
+	printf("probing i2c range 0X%02x - 0x%02x :\n", startaddr, stopaddr);
+	for (addr = startaddr; addr != stopaddr; addr++) {
+		client->addr = addr;
+		ret = i2c_write_reg(client, 0x00, &reg, 0);
+		if (ret == 0)
+			printf("0x%02x ", addr);
+	}
+	printf("\n");
+	return 0;
+}
+
+static const __maybe_unused char cmd_i2c_probe_help[] =
+"Usage: i2c_probe 0xstartaddr 0xstopaddr\n"
+"probe a range of i2c addresses.\n";
+
+BAREBOX_CMD_START(i2c_probe)
+	.cmd		= do_i2c_probe,
+	.usage		= "probe for an i2c device",
+	BAREBOX_CMD_HELP(cmd_i2c_probe_help)
+BAREBOX_CMD_END
+
+static int do_i2c_write(struct command *cmdtp, int argc, char *argv[])
+{
+	struct i2c_client *client;
+	int addr = -1, reg = -1, count = -1, ret, opt;
+	void *buf = NULL;
+
+	if (argc < 5)
+		return COMMAND_ERROR_USAGE;
+
+	while ((opt = getopt(argc, argv, "a:c:r:")) > 0) {
+		switch (opt) {
+		case 'a':
+			addr = hex2_bin(optarg);
+			if (addr < 0)
+				return COMMAND_ERROR_USAGE;
+			break;
+		case 'c':
+			count = simple_strtoul(optarg, NULL, 0);
+			break;
+		case 'r':
+			reg = hex2_bin(optarg);
+			if (reg < 0)
+				return COMMAND_ERROR_USAGE;
+			break;
+		}
+	}
+	if ((argc - optind) != count)
+		return COMMAND_ERROR_USAGE;
+
+	client = i2cdev_get_client();
+	if (!client)
+		return -ENODEV;
+	client->addr = addr;
+
+	buf = malloc(count);
+	if (buf) {
+		int i;
+		u8 tmp;
+		for (i = 0; i < count; i++) {
+			tmp = hex2_bin(argv[optind+i]);
+			if (tmp < 1) {
+				free(buf);
+				return COMMAND_ERROR_USAGE;
+			}
+			*(u8 *) (buf + i) = tmp;
+		}
+		ret = i2c_write_reg(client, reg, buf, count);
+		if (ret != count) {
+			free(buf);
+			return ret;
+		}
+		free(buf);
+		printf("wrote %i bytes starting at reg 0x%02x to i2cdev \
+			0x%02x\n", count, reg, addr);
+		return 0;
+	}
+	return 1;
+}
+
+static const __maybe_unused char cmd_i2c_write_help[] =
+"Usage: i2c_write [OPTION] ... hexdatas\n"
+"write to i2c device.\n"
+"  -a 0x<addr>   i2c device address\n"
+"  -r 0x<reg>    start register\n"
+"  -c <count>  byte number to write\n";
+
+BAREBOX_CMD_START(i2c_write)
+	.cmd		= do_i2c_write,
+	.usage		= "write to an i2c device",
+	BAREBOX_CMD_HELP(cmd_i2c_write_help)
+BAREBOX_CMD_END
+
+static int do_i2c_read(struct command *cmdtp, int argc, char *argv[])
+{
+	struct i2c_client *client;
+	void *buf = NULL;
+	int count = -1, addr = -1, reg = -1, ret, opt;
+
+	if (argc < 4)
+		return COMMAND_ERROR_USAGE;
+
+	while ((opt = getopt(argc, argv, "a:c:r:")) > 0) {
+		switch (opt) {
+		case 'a':
+			addr = hex2_bin(optarg);
+			if (addr < 0)
+				return COMMAND_ERROR_USAGE;
+			break;
+		case 'c':
+			count = simple_strtoul(optarg, NULL, 0);
+			break;
+		case 'r':
+			reg = hex2_bin(optarg);
+			if (reg < 0)
+				return COMMAND_ERROR_USAGE;
+			break;
+		}
+	}
+
+	client = i2cdev_get_client();
+	if (!client)
+		return -ENODEV;
+	client->addr = addr;
+
+	buf = malloc(count);
+	if (buf) {
+		ret = i2c_read_reg(client, reg, buf, count);
+		if (ret == count) {
+			int i;
+			printf("read %i bytes starting at reg 0x%02x from \
+				i2cdev 0x%02x\n", count, reg, addr);
+			for (i = 0; i < count; i++)
+				printf("0x%02x ", *(u8 *) (buf + i));
+			printf("\n");
+			free(buf);
+			return 0;
+		}
+		free(buf);
+	}
+	return 1;
+}
+
+static const __maybe_unused char cmd_i2c_read_help[] =
+"Usage: i2c_read [OPTION]\n"
+"read i2c device.\n"
+"  -a 0x<addr>   i2c device address\n"
+"  -r 0x<reg>    start register\n"
+"  -c <count>  byte count\n";
+
+BAREBOX_CMD_START(i2c_read)
+	.cmd		= do_i2c_read,
+	.usage		= "read from an i2c device",
+	BAREBOX_CMD_HELP(cmd_i2c_read_help)
+BAREBOX_CMD_END
-- 
1.6.3.3


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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] commands: add i2c commands
  2010-05-18  9:26   ` [PATCH 3/3] commands: add i2c commands Eric Bénard
@ 2010-05-18 10:04     ` Eric Bénard
  2010-05-19 10:17     ` Sascha Hauer
  1 sibling, 0 replies; 7+ messages in thread
From: Eric Bénard @ 2010-05-18 10:04 UTC (permalink / raw)
  To: barebox

Le 18/05/2010 11:26, Eric Bénard a écrit :
> +static int do_i2c_write(struct command *cmdtp, int argc, char *argv[])
.../...
> +		for (i = 0; i<  count; i++) {
> +			tmp = hex2_bin(argv[optind+i]);
> +			if (tmp<  1) {
this should be < 0

Eric

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] commands: add i2c commands
  2010-05-18  9:26   ` [PATCH 3/3] commands: add i2c commands Eric Bénard
  2010-05-18 10:04     ` Eric Bénard
@ 2010-05-19 10:17     ` Sascha Hauer
  1 sibling, 0 replies; 7+ messages in thread
From: Sascha Hauer @ 2010-05-19 10:17 UTC (permalink / raw)
  To: Eric Bénard; +Cc: barebox

Hi Eric,

On Tue, May 18, 2010 at 11:26:48AM +0200, Eric Bénard wrote:
> theses commands allow low level access to i2c bus and can be useful
> to setup an i2c device without having to add code, compile and flash
> barebox.

This is surely a nice thing to have sometimes. Comments inline.


> 
> Signed-off-by: Eric Bénard <eric@eukrea.com>
> ---
>  commands/Kconfig  |    8 ++
>  commands/Makefile |    1 +
>  commands/i2c.c    |  233 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 242 insertions(+), 0 deletions(-)
>  create mode 100644 commands/i2c.c
> 
> diff --git a/commands/Kconfig b/commands/Kconfig
> index 0c09f91..7115b18 100644
> --- a/commands/Kconfig
> +++ b/commands/Kconfig
> @@ -314,4 +314,12 @@ config CMD_UNLZO
>  	  Say yes here to get the unlzo command. lzo is a fast compression
>  	  algorithm by Markus Franz Xaver Johannes Oberhumer.
>  
> +config CMD_I2C
> +	bool
> +	depends on DRIVER_I2C_I2CDEV
> +	prompt "i2c commands"
> +	help
> +	  include i2c_probe, i2c_read and i2c_write commands to communicate
> +	  on i2c bus.
> +
>  endmenu
> diff --git a/commands/Makefile b/commands/Makefile
> index 74b0994..3eef5de 100644
> --- a/commands/Makefile
> +++ b/commands/Makefile
> @@ -48,3 +48,4 @@ obj-$(CONFIG_CMD_BMP)		+= bmp.o
>  obj-$(CONFIG_USB_GADGET_DFU)	+= dfu.o
>  obj-$(CONFIG_CMD_GPIO)		+= gpio.o
>  obj-$(CONFIG_CMD_UNLZO)		+= unlzo.o
> +obj-$(CONFIG_CMD_I2C)		+= i2c.o
> diff --git a/commands/i2c.c b/commands/i2c.c
> new file mode 100644
> index 0000000..b4152eb
> --- /dev/null
> +++ b/commands/i2c.c
> @@ -0,0 +1,233 @@
> +/*
> + * i2c.c - i2c commands
> + *
> + * Copyright (c) 2010 Eric Bénard <eric@eukrea.Com>, Eukréa Electromatique
> + *
> + * originals hex1_bin and hex2_bin are
> + * (C) Copyright 2000 Wolfgang Denk, DENX Software Engineering, wd@denx.de.
> + *
> + * 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
> + */
> +
> +#include <common.h>
> +#include <command.h>
> +#include <errno.h>
> +#include <malloc.h>
> +#include <getopt.h>
> +#include <i2c/i2c.h>
> +
> +struct i2c_client	*client;
> +extern struct i2c_client *i2cdev_get_client(void);
> +
> +static int hex1_bin(char c)
> +{
> +	if (c >= '0' && c <= '9')
> +		return c - '0';
> +	if (c >= 'a' && c <= 'f')
> +		return c + 10 - 'a';
> +	if (c >= 'A' && c <= 'F')
> +		return c + 10 - 'A';
> +	return -1;
> +}
> +
> +static int hex2_bin(char *s)
> +{
> +	int i, j;
> +
> +	if (*s == '0' && (*(s+1) == 'x'))
> +		s += 2;
> +	i = hex1_bin(*s++);
> +	if (i < 0)
> +		return -1;
> +	j = hex1_bin(*s);
> +	if (j < 0)
> +		return -1;
> +	return (i<<4) + j;
> +}

Any reason not to use simple_strtoul here?

> +
> +static int do_i2c_probe(struct command *cmdtp, int argc, char *argv[])
> +{
> +	struct i2c_client *client;
> +	int startaddr = -1, stopaddr = -1, addr, ret;
> +	u8 reg;
> +
> +	if (argc < 3)
> +		return COMMAND_ERROR_USAGE;
> +
> +	startaddr = hex2_bin(argv[1]);
> +	stopaddr = hex2_bin(argv[2]);
> +	if ((startaddr == -1) || (stopaddr == -1))
> +		return COMMAND_ERROR_USAGE;
> +

You should also check for stopaddr >= startaddr, otherwise you can run
into a (nearly) infinite loop below. Or better check for addr <=
stopaddr below.

> +	client = i2cdev_get_client();
> +	if (!client)
> +		return -ENODEV;
> +
> +	printf("probing i2c range 0X%02x - 0x%02x :\n", startaddr, stopaddr);
> +	for (addr = startaddr; addr != stopaddr; addr++) {
> +		client->addr = addr;
> +		ret = i2c_write_reg(client, 0x00, &reg, 0);
> +		if (ret == 0)
> +			printf("0x%02x ", addr);
> +	}
> +	printf("\n");
> +	return 0;
> +}
> +
> +static const __maybe_unused char cmd_i2c_probe_help[] =
> +"Usage: i2c_probe 0xstartaddr 0xstopaddr\n"
> +"probe a range of i2c addresses.\n";
> +
> +BAREBOX_CMD_START(i2c_probe)
> +	.cmd		= do_i2c_probe,
> +	.usage		= "probe for an i2c device",
> +	BAREBOX_CMD_HELP(cmd_i2c_probe_help)
> +BAREBOX_CMD_END
> +
> +static int do_i2c_write(struct command *cmdtp, int argc, char *argv[])
> +{
> +	struct i2c_client *client;
> +	int addr = -1, reg = -1, count = -1, ret, opt;
> +	void *buf = NULL;

No need to set this to NULL.

> +
> +	if (argc < 5)
> +		return COMMAND_ERROR_USAGE;

You don't need this check.

> +
> +	while ((opt = getopt(argc, argv, "a:c:r:")) > 0) {
> +		switch (opt) {
> +		case 'a':
> +			addr = hex2_bin(optarg);
> +			if (addr < 0)
> +				return COMMAND_ERROR_USAGE;
> +			break;
> +		case 'c':
> +			count = simple_strtoul(optarg, NULL, 0);
> +			break;
> +		case 'r':
> +			reg = hex2_bin(optarg);
> +			if (reg < 0)
> +				return COMMAND_ERROR_USAGE;
> +			break;
> +		}
> +	}
> +	if ((argc - optind) != count)
> +		return COMMAND_ERROR_USAGE;

We do not need the count command line argument. Just do a
count = argc - optind.

You should move the addr < 0 and reg < 0 checks here. This also catches
the case when the user did not specify -a or -r.

> +
> +	client = i2cdev_get_client();
> +	if (!client)
> +		return -ENODEV;
> +	client->addr = addr;
> +
> +	buf = malloc(count);
> +	if (buf) {
> +		int i;
> +		u8 tmp;
> +		for (i = 0; i < count; i++) {
> +			tmp = hex2_bin(argv[optind+i]);
> +			if (tmp < 1) {

Shouldn't this be tmp < 0?
Anyway, simple_strtoul can do a better job here. You can explicitely
specify the base to 16 to get rid of the mandatory 0x prefix.

> +				free(buf);
> +				return COMMAND_ERROR_USAGE;
> +			}
> +			*(u8 *) (buf + i) = tmp;

u8 might a better type for buf.

> +		}
> +		ret = i2c_write_reg(client, reg, buf, count);
> +		if (ret != count) {
> +			free(buf);
> +			return ret;
> +		}
> +		free(buf);
> +		printf("wrote %i bytes starting at reg 0x%02x to i2cdev \
> +			0x%02x\n", count, reg, addr);
> +		return 0;
> +	}
> +	return 1;
> +}
> +
> +static const __maybe_unused char cmd_i2c_write_help[] =
> +"Usage: i2c_write [OPTION] ... hexdatas\n"
> +"write to i2c device.\n"
> +"  -a 0x<addr>   i2c device address\n"
> +"  -r 0x<reg>    start register\n"
> +"  -c <count>  byte number to write\n";
> +
> +BAREBOX_CMD_START(i2c_write)
> +	.cmd		= do_i2c_write,
> +	.usage		= "write to an i2c device",
> +	BAREBOX_CMD_HELP(cmd_i2c_write_help)
> +BAREBOX_CMD_END
> +
> +static int do_i2c_read(struct command *cmdtp, int argc, char *argv[])
> +{

Some comments above apply to this function aswell.

> +	struct i2c_client *client;
> +	void *buf = NULL;
> +	int count = -1, addr = -1, reg = -1, ret, opt;
> +
> +	if (argc < 4)
> +		return COMMAND_ERROR_USAGE;
> +
> +	while ((opt = getopt(argc, argv, "a:c:r:")) > 0) {
> +		switch (opt) {
> +		case 'a':
> +			addr = hex2_bin(optarg);
> +			if (addr < 0)
> +				return COMMAND_ERROR_USAGE;
> +			break;
> +		case 'c':
> +			count = simple_strtoul(optarg, NULL, 0);
> +			break;
> +		case 'r':
> +			reg = hex2_bin(optarg);
> +			if (reg < 0)
> +				return COMMAND_ERROR_USAGE;
> +			break;
> +		}
> +	}
> +
> +	client = i2cdev_get_client();
> +	if (!client)
> +		return -ENODEV;
> +	client->addr = addr;
> +
> +	buf = malloc(count);
> +	if (buf) {
> +		ret = i2c_read_reg(client, reg, buf, count);
> +		if (ret == count) {
> +			int i;
> +			printf("read %i bytes starting at reg 0x%02x from \
> +				i2cdev 0x%02x\n", count, reg, addr);
> +			for (i = 0; i < count; i++)
> +				printf("0x%02x ", *(u8 *) (buf + i));
> +			printf("\n");
> +			free(buf);
> +			return 0;
> +		}
> +		free(buf);
> +	}
> +	return 1;
> +}
> +
> +static const __maybe_unused char cmd_i2c_read_help[] =
> +"Usage: i2c_read [OPTION]\n"
> +"read i2c device.\n"
> +"  -a 0x<addr>   i2c device address\n"
> +"  -r 0x<reg>    start register\n"
> +"  -c <count>  byte count\n";
> +
> +BAREBOX_CMD_START(i2c_read)
> +	.cmd		= do_i2c_read,
> +	.usage		= "read from an i2c device",
> +	BAREBOX_CMD_HELP(cmd_i2c_read_help)
> +BAREBOX_CMD_END
> -- 
> 1.6.3.3
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

-- 
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-5555 |

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] i2c: add i2cdev driver
  2010-05-18  9:26 ` [PATCH 2/3] i2c: add i2cdev driver Eric Bénard
  2010-05-18  9:26   ` [PATCH 3/3] commands: add i2c commands Eric Bénard
@ 2010-05-19 11:01   ` Sascha Hauer
  2010-05-19 12:21     ` Sascha Hauer
  1 sibling, 1 reply; 7+ messages in thread
From: Sascha Hauer @ 2010-05-19 11:01 UTC (permalink / raw)
  To: Eric Bénard; +Cc: barebox

On Tue, May 18, 2010 at 11:26:47AM +0200, Eric Bénard wrote:
> this driver is needed to introduce i2c commands

This approach limits the i2cdev support to exactly one bus which is not
good. What we can do here is:

- collect all available adapters in a list in drivers/i2c/i2c.c
- implement i2c_get_adapter(int busnum) to get an adapter from its
  busnum
- then you can add a -b busnum argument to the i2c_read/i2c_write commands
  and use i2c_get_adapter() there. You don't need this file anymore then.

Sascha

> 
> Signed-off-by: Eric Bénard <eric@eukrea.com>
> ---
>  drivers/i2c/Kconfig  |    3 ++
>  drivers/i2c/Makefile |    1 +
>  drivers/i2c/i2cdev.c |   83 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 87 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/i2c/i2cdev.c
> 
> diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
> index f1b2949..cbd2464 100644
> --- a/drivers/i2c/Kconfig
> +++ b/drivers/i2c/Kconfig
> @@ -16,4 +16,7 @@ config DRIVER_I2C_MC9SDZ60
>  config DRIVER_I2C_LP3972
>  	bool "LP3972 driver"
>  
> +config DRIVER_I2C_I2CDEV
> +	bool "I2CDEV driver"
> +
>  endif
> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
> index 62d030b..a193f9c 100644
> --- a/drivers/i2c/Makefile
> +++ b/drivers/i2c/Makefile
> @@ -5,3 +5,4 @@ obj-$(CONFIG_DRIVER_I2C_IMX) += i2c-imx.o
>  obj-$(CONFIG_DRIVER_I2C_MC13892) += mc13892.o
>  obj-$(CONFIG_DRIVER_I2C_MC9SDZ60) += mc9sdz60.o
>  obj-$(CONFIG_DRIVER_I2C_LP3972) += lp3972.o
> +obj-$(CONFIG_DRIVER_I2C_I2CDEV) += i2cdev.o
> diff --git a/drivers/i2c/i2cdev.c b/drivers/i2c/i2cdev.c
> new file mode 100644
> index 0000000..194db6c
> --- /dev/null
> +++ b/drivers/i2c/i2cdev.c
> @@ -0,0 +1,83 @@
> +/*
> + * Copyright (C) 2007 Sascha Hauer, Pengutronix
> + *               2009 Marc Kleine-Budde <mkl@pengutronix.de>
> + *               2009 Eric Benard <eric@eukrea.com>
> + *
> + * 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
> + *
> + */
> +
> +#include <common.h>
> +#include <init.h>
> +#include <driver.h>
> +#include <xfuncs.h>
> +#include <errno.h>
> +
> +#include <i2c/i2c.h>
> +
> +#include <asm/byteorder.h>
> +
> +#define DRIVERNAME		"i2cdev"
> +
> +struct i2cdev_priv {
> +	struct cdev		cdev;
> +	struct i2c_client	*client;
> +};
> +
> +#define to_i2cdev_priv(a)	container_of(a, struct i2cdev_priv, cdev)
> +
> +static struct i2cdev_priv *i2cdev_dev;
> +
> +struct i2c_client *i2cdev_get_client(void)
> +{
> +	if (!i2cdev_dev)
> +		return NULL;
> +
> +	return i2cdev_dev->client;
> +}
> +
> +static struct file_operations i2cdev_fops = {
> +};
> +
> +static int i2cdev_probe(struct device_d *dev)
> +{
> +	if (i2cdev_dev)
> +		return -EBUSY;
> +
> +	i2cdev_dev = xzalloc(sizeof(struct i2cdev_priv));
> +	i2cdev_dev->cdev.name = DRIVERNAME;
> +	i2cdev_dev->client = to_i2c_client(dev);
> +	i2cdev_dev->cdev.size = 256;
> +	i2cdev_dev->cdev.dev = dev;
> +	i2cdev_dev->cdev.ops = &i2cdev_fops;
> +
> +	devfs_create(&i2cdev_dev->cdev);
> +
> +	return 0;
> +}
> +
> +static struct driver_d i2cdev_driver = {
> +	.name  = DRIVERNAME,
> +	.probe = i2cdev_probe,
> +};
> +
> +static int i2cdev_init(void)
> +{
> +	register_driver(&i2cdev_driver);
> +	return 0;
> +}
> +
> +device_initcall(i2cdev_init);
> -- 
> 1.6.3.3
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

-- 
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-5555 |

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] i2c: add i2cdev driver
  2010-05-19 11:01   ` [PATCH 2/3] i2c: add i2cdev driver Sascha Hauer
@ 2010-05-19 12:21     ` Sascha Hauer
  0 siblings, 0 replies; 7+ messages in thread
From: Sascha Hauer @ 2010-05-19 12:21 UTC (permalink / raw)
  To: Eric Bénard; +Cc: barebox

On Wed, May 19, 2010 at 01:01:24PM +0200, Sascha Hauer wrote:
> On Tue, May 18, 2010 at 11:26:47AM +0200, Eric Bénard wrote:
> > this driver is needed to introduce i2c commands
> 
> This approach limits the i2cdev support to exactly one bus which is not
> good. What we can do here is:
> 
> - collect all available adapters in a list in drivers/i2c/i2c.c
> - implement i2c_get_adapter(int busnum) to get an adapter from its
>   busnum

For your convenience, the following patch should do it (untested
currently)


From 541c2167200f2a21584d0fdc0353f130129cf120 Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Wed, 19 May 2010 14:20:07 +0200
Subject: [PATCH] i2c: implement i2c_get_adapter()

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/i2c/i2c.c |   24 +++++++++++++++++++++++-
 include/i2c/i2c.h |    3 +++
 2 files changed, 26 insertions(+), 1 deletions(-)

diff --git a/drivers/i2c/i2c.c b/drivers/i2c/i2c.c
index 32fd026..1fa114a 100644
--- a/drivers/i2c/i2c.c
+++ b/drivers/i2c/i2c.c
@@ -39,7 +39,7 @@ struct boardinfo {
 };
 
 static LIST_HEAD(board_list);
-
+static LIST_HEAD(adapter_list);
 
 /**
  * i2c_transfer - execute a single or combined I2C message
@@ -327,6 +327,23 @@ static void scan_boardinfo(struct i2c_adapter *adapter)
 }
 
 /**
+ *
+ * i2c_get_adapter - get an i2c adapter from its busnum
+ *
+ * @param	busnum	the desired bus number
+ *
+ */
+struct i2c_adapter *i2c_get_adapter(int busnum)
+{
+	struct i2c_adapter *adap;
+
+	list_for_each_entry(adap, &adapter_list, list)
+		if (adap->nr == busnum)
+			return adap;
+	return NULL;
+}
+
+/**
  * i2c_register_master - register I2C master controller
  *
  * @param	master	initialized master, originally from i2c_alloc_master()
@@ -345,6 +362,11 @@ static void scan_boardinfo(struct i2c_adapter *adapter)
  */
 int i2c_add_numbered_adapter(struct i2c_adapter *adapter)
 {
+	if (i2c_get_adapter(adapter->nr))
+		return -EBUSY;
+
+	list_add_tail(&adapter_list, &adapter->list);
+
 	/* populate children from any i2c device tables */
 	scan_boardinfo(adapter);
 
diff --git a/include/i2c/i2c.h b/include/i2c/i2c.h
index 0760fdc..6c216df 100644
--- a/include/i2c/i2c.h
+++ b/include/i2c/i2c.h
@@ -66,6 +66,7 @@ struct i2c_adapter {
 	struct device_d		*dev;	/* ptr to device */
 	int			nr;	/* bus number */
 	int (*master_xfer)(struct i2c_adapter *adap, struct i2c_msg *msgs, int num);
+	struct list_head	list;
 };
 
 

-- 
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-5555 |

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2010-05-19 12:21 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-18  9:26 [PATCH 1/3] i2c-imx: change log level for No ACK Eric Bénard
2010-05-18  9:26 ` [PATCH 2/3] i2c: add i2cdev driver Eric Bénard
2010-05-18  9:26   ` [PATCH 3/3] commands: add i2c commands Eric Bénard
2010-05-18 10:04     ` Eric Bénard
2010-05-19 10:17     ` Sascha Hauer
2010-05-19 11:01   ` [PATCH 2/3] i2c: add i2cdev driver Sascha Hauer
2010-05-19 12:21     ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox