diff mbox

[U-Boot] i2c: Fix chip address.0

Message ID 1296202977-21878-1-git-send-email-Joakim.Tjernlund@transmode.se
State Not Applicable
Headers show

Commit Message

Joakim Tjernlund Jan. 28, 2011, 8:22 a.m. UTC 
According to the i2c command alen 0 should be allowed
but this is not allowed in the implementation, fix.
Also cleanup all cmd_usage cases while at it. It is cleaner
to return cmd_usage(cmdtp); instead of
{
  cmd_usage(cmdtp);
  return 1;
}

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---
 common/cmd_i2c.c |  107 +++++++++++++++++++++---------------------------------
 1 files changed, 41 insertions(+), 66 deletions(-)

Comments

Heiko Schocher Jan. 31, 2011, 7:18 a.m. UTC | #1 
Hello Joakim,

Joakim Tjernlund write:
> According to the i2c command alen 0 should be allowed
> but this is not allowed in the implementation, fix.
> Also cleanup all cmd_usage cases while at it. It is cleaner
> to return cmd_usage(cmdtp); instead of
> {
>   cmd_usage(cmdtp);
>   return 1;
> }
> 
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> ---
>  common/cmd_i2c.c |  107 +++++++++++++++++++++---------------------------------
>  1 files changed, 41 insertions(+), 66 deletions(-)

Hmm... did you use actual top of tree?

Your patch didn;t apply, also I think the issues your patch try to
fix, should be fixed in TOT:

- actual TOT common/cmd_i2c.c uses "return cmd_usage(cmdtp);"
- alen of 0 should also be possible, fixed in commit
  7a92e53cd0d8164d518216bebd52679603d6ffe9 from Reinhard Meyer.

Please try actual TOT

bye,
Heiko
Joakim Tjernlund Jan. 31, 2011, 7:29 a.m. UTC | #2 
Heiko Schocher <hs@denx.de> wrote on 2011/01/31 08:18:56:
>
> Hello Joakim,
>
> Joakim Tjernlund write:
> > According to the i2c command alen 0 should be allowed
> > but this is not allowed in the implementation, fix.
> > Also cleanup all cmd_usage cases while at it. It is cleaner
> > to return cmd_usage(cmdtp); instead of
> > {
> >   cmd_usage(cmdtp);
> >   return 1;
> > }
> >
> > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> > ---
> >  common/cmd_i2c.c |  107 +++++++++++++++++++++---------------------------------
> >  1 files changed, 41 insertions(+), 66 deletions(-)
>
> Hmm... did you use actual top of tree?

Ahh, I used my current tree and it is a little behind.
I see it fixed in TOT.

    Jocke
diff mbox

Patch

diff --git a/common/cmd_i2c.c b/common/cmd_i2c.c
index 8b9c2c9..a7d0dd8 100644
--- a/common/cmd_i2c.c
+++ b/common/cmd_i2c.c
@@ -88,7 +88,7 @@ 
  */
 static uchar	i2c_dp_last_chip;
 static uint	i2c_dp_last_addr;
-static uint	i2c_dp_last_alen;
+static int	i2c_dp_last_alen;
 static uint	i2c_dp_last_length = 0x10;
 
 static uchar	i2c_mm_last_chip;
@@ -154,7 +154,7 @@  int i2c_set_bus_speed(unsigned int)
  * get_alen: small parser helper function to get address length
  * returns the address length,or 0 on error
  */
-static uint get_alen(char *arg)
+static int get_alen(char *arg)
 {
 	int	j;
 	int	alen;
@@ -164,7 +164,7 @@  static uint get_alen(char *arg)
 		if (arg[j] == '.') {
 			alen = arg[j+1] - '0';
 			if (alen > 3) {
-				return 0;
+				return -1;
 			}
 			break;
 		} else if (arg[j] == '\0')
@@ -181,13 +181,12 @@  static uint get_alen(char *arg)
 static int do_i2c_read ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 {
 	u_char	chip;
-	uint	devaddr, alen, length;
+	uint	devaddr, length;
+	int	alen;
 	u_char  *memaddr;
 
-	if (argc != 5) {
-		cmd_usage(cmdtp);
-		return 1;
-	}
+	if (argc != 5)
+		return cmd_usage(cmdtp);
 
 	/*
 	 * I2C chip address
@@ -200,10 +199,8 @@  static int do_i2c_read ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	 */
 	devaddr = simple_strtoul(argv[2], NULL, 16);
 	alen = get_alen(argv[2]);
-	if (alen == 0) {
-		cmd_usage(cmdtp);
-		return 1;
-	}
+	if (alen < 0)
+		return cmd_usage(cmdtp);
 
 	/*
 	 * Length is the number of objects, not number of bytes.
@@ -229,7 +226,8 @@  static int do_i2c_read ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 static int do_i2c_md ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 {
 	u_char	chip;
-	uint	addr, alen, length;
+	uint	addr, length;
+	int	alen;
 	int	j, nbytes, linebytes;
 
 	/* We use the last specified parameters, unless new ones are
@@ -240,10 +238,8 @@  static int do_i2c_md ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	alen   = i2c_dp_last_alen;
 	length = i2c_dp_last_length;
 
-	if (argc < 3) {
-		cmd_usage(cmdtp);
-		return 1;
-	}
+	if (argc < 3)
+		return cmd_usage(cmdtp);
 
 	if ((flag & CMD_FLAG_REPEAT) == 0) {
 		/*
@@ -261,10 +257,8 @@  static int do_i2c_md ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 		 */
 		addr = simple_strtoul(argv[2], NULL, 16);
 		alen = get_alen(argv[2]);
-		if (alen == 0) {
-			cmd_usage(cmdtp);
-			return 1;
-		}
+		if (alen < 0)
+			return cmd_usage(cmdtp);
 
 		/*
 		 * If another parameter, it is the length to display.
@@ -328,14 +322,12 @@  static int do_i2c_mw ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 {
 	uchar	chip;
 	ulong	addr;
-	uint	alen;
+	int	alen;
 	uchar	byte;
 	int	count;
 
-	if ((argc < 4) || (argc > 5)) {
-		cmd_usage(cmdtp);
-		return 1;
-	}
+	if ((argc < 4) || (argc > 5))
+		return cmd_usage(cmdtp);
 
 	/*
 	 * Chip is always specified.
@@ -347,10 +339,8 @@  static int do_i2c_mw ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	 */
 	addr = simple_strtoul(argv[2], NULL, 16);
 	alen = get_alen(argv[2]);
-	if (alen == 0) {
-		cmd_usage(cmdtp);
-		return 1;
-	}
+	if (alen < 0)
+		return cmd_usage(cmdtp);
 
 	/*
 	 * Value to write is always specified.
@@ -392,16 +382,14 @@  static int do_i2c_crc (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 {
 	uchar	chip;
 	ulong	addr;
-	uint	alen;
+	int	alen;
 	int	count;
 	uchar	byte;
 	ulong	crc;
 	ulong	err;
 
-	if (argc < 4) {
-		cmd_usage(cmdtp);
-		return 1;
-	}
+	if (argc < 4)
+		return cmd_usage(cmdtp);
 
 	/*
 	 * Chip is always specified.
@@ -413,10 +401,8 @@  static int do_i2c_crc (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	 */
 	addr = simple_strtoul(argv[2], NULL, 16);
 	alen = get_alen(argv[2]);
-	if (alen == 0) {
-		cmd_usage(cmdtp);
-		return 1;
-	}
+	if (alen < 0)
+		return cmd_usage(cmdtp);
 
 	/*
 	 * Count is always specified
@@ -456,16 +442,14 @@  mod_i2c_mem(cmd_tbl_t *cmdtp, int incrflag, int flag, int argc, char *argv[])
 {
 	uchar	chip;
 	ulong	addr;
-	uint	alen;
+	int	alen;
 	ulong	data;
 	int	size = 1;
 	int	nbytes;
 	extern char console_buffer[];
 
-	if (argc != 3) {
-		cmd_usage(cmdtp);
-		return 1;
-	}
+	if (argc != 3)
+		return cmd_usage(cmdtp);
 
 #ifdef CONFIG_BOOT_RETRY_TIME
 	reset_cmd_timeout();	/* got a good command to get here */
@@ -495,10 +479,8 @@  mod_i2c_mem(cmd_tbl_t *cmdtp, int incrflag, int flag, int argc, char *argv[])
 		 */
 		addr = simple_strtoul(argv[2], NULL, 16);
 		alen = get_alen(argv[2]);
-		if (alen == 0) {
-			cmd_usage(cmdtp);
-			return 1;
-		}
+		if (alen < 0)
+			return cmd_usage(cmdtp);
 	}
 
 	/*
@@ -622,16 +604,14 @@  static int do_i2c_probe (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 static int do_i2c_loop(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 {
 	u_char	chip;
-	ulong	alen;
+	int	alen;
 	uint	addr;
 	uint	length;
 	u_char	bytes[16];
 	int	delay;
 
-	if (argc < 3) {
-		cmd_usage(cmdtp);
-		return 1;
-	}
+	if (argc < 3)
+		return cmd_usage(cmdtp);
 
 	/*
 	 * Chip is always specified.
@@ -643,10 +623,8 @@  static int do_i2c_loop(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	 */
 	addr = simple_strtoul(argv[2], NULL, 16);
 	alen = get_alen(argv[2]);
-	if (alen == 0) {
-		cmd_usage(cmdtp);
-		return 1;
-	}
+	if (alen < 0)
+		return cmd_usage(cmdtp);
 
 	/*
 	 * Length is the number of objects, not number of bytes.
@@ -784,10 +762,9 @@  static int do_sdram (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 		"32 MiB", "16 MiB", "8 MiB", "4 MiB"
 	};
 
-	if (argc < 2) {
-		cmd_usage(cmdtp);
-		return 1;
-	}
+	if (argc < 2)
+		return cmd_usage(cmdtp);
+
 	/*
 	 * Chip is always specified.
 	 */
@@ -1322,12 +1299,10 @@  static int do_i2c(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 
 	c = find_cmd_tbl(argv[0], &cmd_i2c_sub[0], ARRAY_SIZE(cmd_i2c_sub));
 
-	if (c) {
+	if (c)
 		return  c->cmd(cmdtp, flag, argc, argv);
-	} else {
-		cmd_usage(cmdtp);
-		return 1;
-	}
+	else
+		return cmd_usage(cmdtp);
 }
 
 /***************************************************/