[U-Boot] [PATCH] i2c: merge all i2c_reg_read() and i2c_reg_write() into inline functions

[U-Boot] [PATCH] i2c: merge all i2c_reg_read() and i2c_reg_write() into inline functions

[Timur Tabi]

All implementations of the functions i2c_reg_read() and i2c_reg_write() are
identical.  We can save space and simplify the code by converting these
functions into inlines and putting them in i2c.h.

Signed-off-by: Timur Tabi <timur@freescale.com>
---

I'm posting this patch because I'm enhancing the I2C routines to support
multiple I2C busses more easily, but I need to clean up the existing code
first.

 cpu/arm920t/at91rm9200/i2c.c  |   14 --------------
 cpu/arm926ejs/davinci/i2c.c   |   17 -----------------
 cpu/blackfin/i2c.c            |   16 ----------------
 cpu/mpc512x/i2c.c             |   17 -----------------
 cpu/mpc5xxx/i2c.c             |   16 ----------------
 cpu/mpc8220/i2c.c             |   16 ----------------
 cpu/mpc824x/drivers/i2c/i2c.c |   14 --------------
 cpu/mpc8260/i2c.c             |   16 ----------------
 cpu/mpc8xx/i2c.c              |   20 --------------------
 cpu/ppc4xx/i2c.c              |   20 --------------------
 cpu/pxa/i2c.c                 |   15 ---------------
 drivers/i2c/fsl_i2c.c         |   16 ----------------
 drivers/i2c/soft_i2c.c        |   19 -------------------
 include/i2c.h                 |   15 +++++++++++++--
 14 files changed, 13 insertions(+), 218 deletions(-)

diff --git a/cpu/arm920t/at91rm9200/i2c.c b/cpu/arm920t/at91rm9200/i2c.c
index 90f95df..fb4725d 100644
--- a/cpu/arm920t/at91rm9200/i2c.c
+++ b/cpu/arm920t/at91rm9200/i2c.c
@@ -189,20 +189,6 @@ i2c_init(int speed, int slaveaddr)
 	return;
 }
 
-uchar i2c_reg_read(uchar i2c_addr, uchar reg)
-{
-	unsigned char buf;
-
-	i2c_read(i2c_addr, reg, 1, &buf, 1);
-
-	return(buf);
-}
-
-void i2c_reg_write(uchar i2c_addr, uchar reg, uchar val)
-{
-	i2c_write(i2c_addr, reg, 1, &val, 1);
-}
-
 int i2c_set_bus_speed(unsigned int speed)
 {
 	return -1;
diff --git a/cpu/arm926ejs/davinci/i2c.c b/cpu/arm926ejs/davinci/i2c.c
index af9dc03..1029b03 100644
--- a/cpu/arm926ejs/davinci/i2c.c
+++ b/cpu/arm926ejs/davinci/i2c.c
@@ -331,21 +331,4 @@ int i2c_write(u_int8_t chip, u_int32_t addr, int alen, u_int8_t *buf, int len)
 	return(0);
 }
 
-
-u_int8_t i2c_reg_read(u_int8_t chip, u_int8_t reg)
-{
-	u_int8_t	tmp;
-
-	i2c_read(chip, reg, 1, &tmp, 1);
-	return(tmp);
-}
-
-
-void i2c_reg_write(u_int8_t chip, u_int8_t reg, u_int8_t val)
-{
-	u_int8_t	tmp;
-
-	i2c_write(chip, reg, 1, &tmp, 1);
-}
-
 #endif /* CONFIG_DRIVER_DAVINCI_I2C */
diff --git a/cpu/blackfin/i2c.c b/cpu/blackfin/i2c.c
index 60f03d4..2a3e223 100644
--- a/cpu/blackfin/i2c.c
+++ b/cpu/blackfin/i2c.c
@@ -425,20 +425,4 @@ int i2c_write(uchar chip, uint addr, int alen, uchar * buffer, int len)
 
 }
 
-uchar i2c_reg_read(uchar chip, uchar reg)
-{
-	uchar buf;
-
-	PRINTD("i2c_reg_read: chip=0x%02x, reg=0x%02x\n", chip, reg);
-	i2c_read(chip, reg, 0, &buf, 1);
-	return (buf);
-}
-
-void i2c_reg_write(uchar chip, uchar reg, uchar val)
-{
-	PRINTD("i2c_reg_write: chip=0x%02x, reg=0x%02x, val=0x%02x\n", chip,
-			reg, val);
-	i2c_write(chip, reg, 0, &val, 1);
-}
-
 #endif /* CONFIG_HARD_I2C */
diff --git a/cpu/mpc512x/i2c.c b/cpu/mpc512x/i2c.c
index 56ba443..95cc942 100644
--- a/cpu/mpc512x/i2c.c
+++ b/cpu/mpc512x/i2c.c
@@ -382,23 +382,6 @@ Done:
 	return ret;
 }
 
-uchar i2c_reg_read (uchar chip, uchar reg)
-{
-	uchar buf;
-
-	i2c_read (chip, reg, 1, &buf, 1);
-
-	return buf;
-}
-
-void i2c_reg_write (uchar chip, uchar reg, uchar val)
-{
-	i2c_write (chip, reg, 1, &val, 1);
-
-	return;
-}
-
-
 int i2c_set_bus_num (unsigned int bus)
 {
 	if (bus >= I2C_BUS_CNT) {
diff --git a/cpu/mpc5xxx/i2c.c b/cpu/mpc5xxx/i2c.c
index 0f02e78..015256f 100644
--- a/cpu/mpc5xxx/i2c.c
+++ b/cpu/mpc5xxx/i2c.c
@@ -380,20 +380,4 @@ Done:
 	return ret;
 }
 
-uchar i2c_reg_read(uchar chip, uchar reg)
-{
-	uchar buf;
-
-	i2c_read(chip, reg, 1, &buf, 1);
-
-	return buf;
-}
-
-void i2c_reg_write(uchar chip, uchar reg, uchar val)
-{
-	i2c_write(chip, reg, 1, &val, 1);
-
-	return;
-}
-
 #endif	/* CONFIG_HARD_I2C */
diff --git a/cpu/mpc8220/i2c.c b/cpu/mpc8220/i2c.c
index d67936d..76ecdf1 100644
--- a/cpu/mpc8220/i2c.c
+++ b/cpu/mpc8220/i2c.c
@@ -387,20 +387,4 @@ int i2c_write (uchar chip, uint addr, int alen, uchar * buf, int len)
 	return ret;
 }
 
-uchar i2c_reg_read (uchar chip, uchar reg)
-{
-	uchar buf;
-
-	i2c_read (chip, reg, 1, &buf, 1);
-
-	return buf;
-}
-
-void i2c_reg_write (uchar chip, uchar reg, uchar val)
-{
-	i2c_write (chip, reg, 1, &val, 1);
-
-	return;
-}
-
 #endif /* CONFIG_HARD_I2C */
diff --git a/cpu/mpc824x/drivers/i2c/i2c.c b/cpu/mpc824x/drivers/i2c/i2c.c
index 3add687..48d754e 100644
--- a/cpu/mpc824x/drivers/i2c/i2c.c
+++ b/cpu/mpc824x/drivers/i2c/i2c.c
@@ -267,18 +267,4 @@ int i2c_probe (uchar chip)
 	return i2c_read (chip, 0, 1, (uchar *) &tmp, 1);
 }
 
-uchar i2c_reg_read (uchar i2c_addr, uchar reg)
-{
-	uchar buf[1];
-
-	i2c_read (i2c_addr, reg, 1, buf, 1);
-
-	return (buf[0]);
-}
-
-void i2c_reg_write (uchar i2c_addr, uchar reg, uchar val)
-{
-	i2c_write (i2c_addr, reg, 1, &val, 1);
-}
-
 #endif /* CONFIG_HARD_I2C */
diff --git a/cpu/mpc8260/i2c.c b/cpu/mpc8260/i2c.c
index c3af7b6..71dac3b 100644
--- a/cpu/mpc8260/i2c.c
+++ b/cpu/mpc8260/i2c.c
@@ -749,20 +749,4 @@ i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len)
 	return 0;
 }
 
-uchar
-i2c_reg_read(uchar chip, uchar reg)
-{
-	uchar buf;
-
-	i2c_read(chip, reg, 1, &buf, 1);
-
-	return (buf);
-}
-
-void
-i2c_reg_write(uchar chip, uchar reg, uchar val)
-{
-	i2c_write(chip, reg, 1, &val, 1);
-}
-
 #endif	/* CONFIG_HARD_I2C */
diff --git a/cpu/mpc8xx/i2c.c b/cpu/mpc8xx/i2c.c
index f05b666..0a2fe95 100644
--- a/cpu/mpc8xx/i2c.c
+++ b/cpu/mpc8xx/i2c.c
@@ -717,24 +717,4 @@ int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len)
 	return 0;
 }
 
-uchar
-i2c_reg_read(uchar i2c_addr, uchar reg)
-{
-	uchar buf;
-
-	i2c_init(CFG_I2C_SPEED, CFG_I2C_SLAVE);
-
-	i2c_read(i2c_addr, reg, 1, &buf, 1);
-
-	return (buf);
-}
-
-void
-i2c_reg_write(uchar i2c_addr, uchar reg, uchar val)
-{
-	i2c_init(CFG_I2C_SPEED, CFG_I2C_SLAVE);
-
-	i2c_write(i2c_addr, reg, 1, &val, 1);
-}
-
 #endif	/* CONFIG_HARD_I2C */
diff --git a/cpu/ppc4xx/i2c.c b/cpu/ppc4xx/i2c.c
index d8be2ce..39c3fef 100644
--- a/cpu/ppc4xx/i2c.c
+++ b/cpu/ppc4xx/i2c.c
@@ -420,26 +420,6 @@ int i2c_write(uchar chip, uint addr, int alen, uchar * buffer, int len)
 	return (i2c_transfer(0, chip<<1, &xaddr[4-alen], alen, buffer, len ) != 0);
 }
 
-/*-----------------------------------------------------------------------
- * Read a register
- */
-uchar i2c_reg_read(uchar i2c_addr, uchar reg)
-{
-	uchar buf;
-
-	i2c_read(i2c_addr, reg, 1, &buf, 1);
-
-	return (buf);
-}
-
-/*-----------------------------------------------------------------------
- * Write a register
- */
-void i2c_reg_write(uchar i2c_addr, uchar reg, uchar val)
-{
-	i2c_write(i2c_addr, reg, 1, &val, 1);
-}
-
 #if defined(CONFIG_I2C_MULTI_BUS)
 /*
  * Functions for multiple I2C bus handling
diff --git a/cpu/pxa/i2c.c b/cpu/pxa/i2c.c
index df537c4..539ca25 100644
--- a/cpu/pxa/i2c.c
+++ b/cpu/pxa/i2c.c
@@ -455,19 +455,4 @@ int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len)
 
 }
 
-uchar i2c_reg_read (uchar chip, uchar reg)
-{
-	uchar buf;
-
-	PRINTD(("i2c_reg_read(chip=0x%02x, reg=0x%02x)\n",chip,reg));
-	i2c_read(chip, reg, 1, &buf, 1);
-	return (buf);
-}
-
-void  i2c_reg_write(uchar chip, uchar reg, uchar val)
-{
-	PRINTD(("i2c_reg_write(chip=0x%02x, reg=0x%02x, val=0x%02x)\n",chip,reg,val));
-	i2c_write(chip, reg, 1, &val, 1);
-}
-
 #endif	/* CONFIG_HARD_I2C */
diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c
index 264553d..a8ab0f0 100644
--- a/drivers/i2c/fsl_i2c.c
+++ b/drivers/i2c/fsl_i2c.c
@@ -368,22 +368,6 @@ i2c_probe(uchar chip)
 	return i2c_read(chip, 0, 0, NULL, 0);
 }
 
-uchar
-i2c_reg_read(uchar i2c_addr, uchar reg)
-{
-	uchar buf[1];
-
-	i2c_read(i2c_addr, reg, 1, buf, 1);
-
-	return buf[0];
-}
-
-void
-i2c_reg_write(uchar i2c_addr, uchar reg, uchar val)
-{
-	i2c_write(i2c_addr, reg, 1, &val, 1);
-}
-
 int i2c_set_bus_num(unsigned int bus)
 {
 #ifdef CFG_I2C2_OFFSET
diff --git a/drivers/i2c/soft_i2c.c b/drivers/i2c/soft_i2c.c
index 23db2ee..4d7abdd 100644
--- a/drivers/i2c/soft_i2c.c
+++ b/drivers/i2c/soft_i2c.c
@@ -402,22 +402,3 @@ int  i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len)
 	return(failures);
 }
 
-/*-----------------------------------------------------------------------
- * Read a register
- */
-uchar i2c_reg_read(uchar i2c_addr, uchar reg)
-{
-	uchar buf;
-
-	i2c_read(i2c_addr, reg, 1, &buf, 1);
-
-	return(buf);
-}
-
-/*-----------------------------------------------------------------------
- * Write a register
- */
-void i2c_reg_write(uchar i2c_addr, uchar reg, uchar val)
-{
-	i2c_write(i2c_addr, reg, 1, &val, 1);
-}
diff --git a/include/i2c.h b/include/i2c.h
index a51c164..169e487 100644
--- a/include/i2c.h
+++ b/include/i2c.h
@@ -100,8 +100,19 @@ int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len);
 /*
  * Utility routines to read/write registers.
  */
-uchar i2c_reg_read (uchar chip, uchar reg);
-void  i2c_reg_write(uchar chip, uchar reg, uchar val);
+static inline u8 i2c_reg_read(u8 addr, u8 reg)
+{
+	u8 buf;
+
+	i2c_read(addr, reg, 1, &buf, 1);
+
+	return buf;
+}
+
+static inline void i2c_reg_write(u8 addr, u8 reg, u8 val)
+{
+	i2c_write(addr, reg, 1, &val, 1);
+}
 
 /*
  * Functions for setting the current I2C bus and its speed
-- 
1.5.5

[U-Boot] [PATCH] i2c: merge all i2c_reg_read() and i2c_reg_write() into inline functions

[Timur Tabi]

Wolfgang,

Who is the maintainer for I2C?  I don't know who needs to ACK this
patch for it to go in, and I don't know who's supposed to pick it up,
either.

On Thu, Oct 2, 2008 at 10:06 AM, Timur Tabi <timur@freescale.com> wrote:
> All implementations of the functions i2c_reg_read() and i2c_reg_write() are
> identical.  We can save space and simplify the code by converting these
> functions into inlines and putting them in i2c.h.
>
> Signed-off-by: Timur Tabi <timur@freescale.com>

-- 
Timur Tabi
Linux kernel developer at Freescale

[U-Boot] [PATCH] i2c: merge all i2c_reg_read() and i2c_reg_write() into inline functions

[Wolfgang Denk]

Dear Timur,

In message <ed82fe3e0810031525n784bd609gf3d28b8e65992d1@mail.gmail.com> you wrote:
> 
> Who is the maintainer for I2C?  I don't know who needs to ACK this
> patch for it to go in, and I don't know who's supposed to pick it up,
> either.

In case there is no specific custodian, I will be the default.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
In general, if you think something isn't in Perl, try it out, because
it usually is :-) - Larry Wall in <1991Jul31.174523.9447@netlabs.com>

[U-Boot] [PATCH] i2c: merge all i2c_reg_read() and i2c_reg_write() into inline functions

[Wolfgang Denk]

Dear Timur Tabi,

In message <1222959963-5697-1-git-send-email-timur@freescale.com> you wrote:
> All implementations of the functions i2c_reg_read() and i2c_reg_write() are
> identical.  We can save space and simplify the code by converting these
> functions into inlines and putting them in i2c.h.

Actually they are not identical:

> +++ b/cpu/arm926ejs/davinci/i2c.c
> @@ -331,21 +331,4 @@ int i2c_write(u_int8_t chip, u_int32_t addr, int alen, u_int8_t *buf, int len)
...
> -u_int8_t i2c_reg_read(u_int8_t chip, u_int8_t reg)
> -{
> -	u_int8_t	tmp;
> -
> -	i2c_read(chip, reg, 1, &tmp, 1);
> -	return(tmp);
> -}
...
> -void i2c_reg_write(u_int8_t chip, u_int8_t reg, u_int8_t val)
> -{
> -	u_int8_t	tmp;
> -
> -	i2c_write(chip, reg, 1, &tmp, 1);
> -}
> -
>  #endif /* CONFIG_DRIVER_DAVINCI_I2C */
> diff --git a/cpu/blackfin/i2c.c b/cpu/blackfin/i2c.c
> index 60f03d4..2a3e223 100644
> --- a/cpu/blackfin/i2c.c
> +++ b/cpu/blackfin/i2c.c
> @@ -425,20 +425,4 @@ int i2c_write(uchar chip, uint addr, int alen, uchar * buffer, int len)
...
> -uchar i2c_reg_read(uchar chip, uchar reg)
> -{
> -	uchar buf;
> -
> -	PRINTD("i2c_reg_read: chip=0x%02x, reg=0x%02x\n", chip, reg);
> -	i2c_read(chip, reg, 0, &buf, 1);
> -	return (buf);
> -}
> -
> -void i2c_reg_write(uchar chip, uchar reg, uchar val)
> -{
> -	PRINTD("i2c_reg_write: chip=0x%02x, reg=0x%02x, val=0x%02x\n", chip,
> -			reg, val);
> -	i2c_write(chip, reg, 0, &val, 1);
> -}
> -

This does not exactly look identical to me.


And when you claim "We can save space" - which sort of space are  you
talking about? Dos this reduce the memory footprint of the code?


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The main thing is the play itself. I swear that greed for  money  has
nothing  to  do with it, although heaven knows I am sorely in need of
money.                                           - Feodor Dostoyevsky

[U-Boot] [PATCH] i2c: merge all i2c_reg_read() and i2c_reg_write() into inline functions

[Timur Tabi]

Wolfgang Denk wrote:

>> -void i2c_reg_write(uchar chip, uchar reg, uchar val)
>> -{
>> -	PRINTD("i2c_reg_write: chip=0x%02x, reg=0x%02x, val=0x%02x\n", chip,
>> -			reg, val);
>> -	i2c_write(chip, reg, 0, &val, 1);
>> -}
>> -
> 
> This does not exactly look identical to me.

The PRINTD is irrelevant.  If these platforms really want to debug single I2C
operations, they can add the code back.  It's just debug code, so I would think
that it's not worth sacrificing the improved code simplicity just for that.

> And when you claim "We can save space" - which sort of space are  you
> talking about? Dos this reduce the memory footprint of the code?

I ran a couple tests, and u-boot.bin didn't change in size, but technically, it
should be smaller because the compiler will optimize the code into a normal call
to i2c_read or i2c_write.

Either way, at least the source code is smaller.

-- 
Timur Tabi
Linux kernel developer at Freescale

[U-Boot] [PATCH] i2c: merge all i2c_reg_read() and i2c_reg_write() into inline functions

[Wolfgang Denk]

Dear Timur,

In message <48E6A9D5.4050307@freescale.com> you wrote:
> 
> The PRINTD is irrelevant.  If these platforms really want to debug single I2C
> operations, they can add the code back.  It's just debug code, so I would think
> that it's not worth sacrificing the improved code simplicity just for that.

This is your opinion. I think you cannot really speak for  others.  I
think  you  should  ask  the respective maintainers of the code first
before changing their code.

> > And when you claim "We can save space" - which sort of space are  you
> > talking about? Dos this reduce the memory footprint of the code?
> 
> I ran a couple tests, and u-boot.bin didn't change in size, but technically, it
> should be smaller because the compiler will optimize the code into a normal call
> to i2c_read or i2c_write.

Hm... why didn't the size change, then?


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Quantum Mechanics is God's version of "Trust me."

[U-Boot] [PATCH] i2c: merge all i2c_reg_read() and i2c_reg_write() into inline functions

[Timur Tabi]

On Fri, Oct 3, 2008 at 6:39 PM, Wolfgang Denk <wd@denx.de> wrote:
> This is your opinion. I think you cannot really speak for  others.  I
> think  you  should  ask  the respective maintainers of the code first
> before changing their code.

Ok.

> Hm... why didn't the size change, then?

My guess: alignment.

-- 
Timur Tabi
Linux kernel developer at Freescale

[U-Boot] [PATCH] i2c: merge all i2c_reg_read() and i2c_reg_write() into inline functions

[Timur Tabi]

On Fri, Oct 3, 2008 at 6:39 PM, Wolfgang Denk <wd@denx.de> wrote:

> This is your opinion. I think you cannot really speak for  others.  I
> think  you  should  ask  the respective maintainers of the code first
> before changing their code.

I think the MAINTAINERS file might be out of date with respect to the
blackfin and pxa boards.  It lists Mike Frysinger and the blackfin
maintainer and Cliff Brake as the pxa maintainer, but the mailing
lists archives and the git repository show that these two individuals
have not been involved in any patches for these platforms for the past
six months.

So who are the maintainers for blackfin and pxa?

-- 
Timur Tabi
Linux kernel developer at Freescale

[U-Boot] [PATCH] i2c: merge all i2c_reg_read() and i2c_reg_write() into inline functions

[Stefan Roese]

On Sunday 05 October 2008, Timur Tabi wrote:
> On Fri, Oct 3, 2008 at 6:39 PM, Wolfgang Denk <wd@denx.de> wrote:
> > This is your opinion. I think you cannot really speak for  others.  I
> > think  you  should  ask  the respective maintainers of the code first
> > before changing their code.
>
> I think the MAINTAINERS file might be out of date with respect to the
> blackfin and pxa boards.  It lists Mike Frysinger and the blackfin
> maintainer and Cliff Brake as the pxa maintainer, but the mailing
> lists archives and the git repository show that these two individuals
> have not been involved in any patches for these platforms for the past
> six months.
>
> So who are the maintainers for blackfin and pxa?

I'm pretty sure that Mike is still responsible for Blackfin. Not sure about 
Cliff.

BTW: I second your patch. So from me:

Acked-by: Stefan Roese <sr@denx.de>

Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH] i2c: merge all i2c_reg_read() and i2c_reg_write() into inline functions

[Timur Tabi]

Mike and Cliff,

This I2C patch converts some I2C functions t inline, but in the case
of blackfin and pxa, it removes the PRINTD() call as well.  If you're
okay with that, please ACK this patch.

You can find the original patch in the mailing list archive here:
http://lists.denx.de/pipermail/u-boot/2008-October/041173.html

On Thu, Oct 2, 2008 at 10:06 AM, Timur Tabi <timur@freescale.com> wrote:
> All implementations of the functions i2c_reg_read() and i2c_reg_write() are
> identical.  We can save space and simplify the code by converting these
> functions into inlines and putting them in i2c.h.
>
> Signed-off-by: Timur Tabi <timur@freescale.com>
> ---
>
> I'm posting this patch because I'm enhancing the I2C routines to support
> multiple I2C busses more easily, but I need to clean up the existing code
> first.
>
>  cpu/arm920t/at91rm9200/i2c.c  |   14 --------------
>  cpu/arm926ejs/davinci/i2c.c   |   17 -----------------
>  cpu/blackfin/i2c.c            |   16 ----------------
>  cpu/mpc512x/i2c.c             |   17 -----------------
>  cpu/mpc5xxx/i2c.c             |   16 ----------------
>  cpu/mpc8220/i2c.c             |   16 ----------------
>  cpu/mpc824x/drivers/i2c/i2c.c |   14 --------------
>  cpu/mpc8260/i2c.c             |   16 ----------------
>  cpu/mpc8xx/i2c.c              |   20 --------------------
>  cpu/ppc4xx/i2c.c              |   20 --------------------
>  cpu/pxa/i2c.c                 |   15 ---------------
>  drivers/i2c/fsl_i2c.c         |   16 ----------------
>  drivers/i2c/soft_i2c.c        |   19 -------------------
>  include/i2c.h                 |   15 +++++++++++++--
>  14 files changed, 13 insertions(+), 218 deletions(-)

-- 
Timur Tabi
Linux kernel developer at Freescale

[U-Boot] [PATCH] i2c: merge all i2c_reg_read() and i2c_reg_write() into inline functions

[Wolfgang Denk]

Dear Timur,

In message <1222959963-5697-1-git-send-email-timur@freescale.com> you wrote:
> All implementations of the functions i2c_reg_read() and i2c_reg_write() are
> identical.  We can save space and simplify the code by converting these
> functions into inlines and putting them in i2c.h.
> 
> Signed-off-by: Timur Tabi <timur@freescale.com>
> ---
> 
> I'm posting this patch because I'm enhancing the I2C routines to support
> multiple I2C busses more easily, but I need to clean up the existing code
> first.

What about resubmitting this patch, with adding a debug() to the new,
common i2c_reg_read() and i2c_reg_write() funtions?


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"If people are good only because they fear punishment, and  hope  for
reward, then we are a sorry lot indeed."            - Albert Einstein

[U-Boot] [PATCH] i2c: merge all i2c_reg_read() and i2c_reg_write() into inline functions

[Timur Tabi]

Wolfgang Denk wrote:

> What about resubmitting this patch, with adding a debug() to the new,
> common i2c_reg_read() and i2c_reg_write() funtions?

I can do that.  First, however, I need Mike Frysinger to help me resolve the
Blackfin version of i2c_reg_read(), which passes 0 instead of 1 as the length.
I've emailed him twice about this, so I'm not sure why he's ignoring me.

-- 
Timur Tabi
Linux kernel developer at Freescale

[U-Boot] [PATCH] i2c: merge all i2c_reg_read() and i2c_reg_write() into inline functions

[Mike Frysinger]

On Tuesday 14 October 2008, Timur Tabi wrote:
> Wolfgang Denk wrote:
> > What about resubmitting this patch, with adding a debug() to the new,
> > common i2c_reg_read() and i2c_reg_write() funtions?
>
> I can do that.  First, however, I need Mike Frysinger to help me resolve
> the Blackfin version of i2c_reg_read(), which passes 0 instead of 1 as the
> length.

it's broken and i fixed it in the Blackfin i2c read write.  feel free to make 
the change and ignore the weirdness.

> I've emailed him twice about this, so I'm not sure why he's 
> ignoring me.

i havent been processing any @gentoo.org e-mail ... not really ignoring you if 
i havent read your e-mail ;)
-mike

[U-Boot] [PATCH] i2c: merge all i2c_reg_read() and i2c_reg_write() into inline functions

[Timur Tabi]

On Sun, Oct 19, 2008 at 4:18 PM, Mike Frysinger <vapier@gentoo.org> wrote:

> it's broken and i fixed it in the Blackfin i2c read write.  feel free to make
> the change and ignore the weirdness.

Ah, thank you!  I'll do it after WD pulls your repo.

> i havent been processing any @gentoo.org e-mail ... not really ignoring you if
> i havent read your e-mail ;)

I don't understand - the commits to your U-Boot repo all have the
@gentoo.org address, and your entry in the MAINTAINERS file also uses
that address.  In fact, everywhere I look I see that address.  What
address should I be using to contact you?

-- 
Timur Tabi
Linux kernel developer at Freescale

[U-Boot] [PATCH] i2c: merge all i2c_reg_read() and i2c_reg_write() into inline functions

[Mike Frysinger]

On Sunday 19 October 2008, Timur Tabi wrote:
> On Sun, Oct 19, 2008 at 4:18 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> > it's broken and i fixed it in the Blackfin i2c read write.  feel free to
> > make the change and ignore the weirdness.
>
> Ah, thank you!  I'll do it after WD pulls your repo.

dont wait ... it'll be a bit before i post it, so it'll be easier if i just 
rebase against that

> > i havent been processing any @gentoo.org e-mail ... not really ignoring
> > you if i havent read your e-mail ;)
>
> I don't understand - the commits to your U-Boot repo all have the
> @gentoo.org address, and your entry in the MAINTAINERS file also uses
> that address.  In fact, everywhere I look I see that address.  What
> address should I be using to contact you?

git activity doesnt correspond to e-mail activity ... that is the right e-mail 
to contact, ive just been too busy to check it lately
-mike

[U-Boot] [PATCH] i2c: merge all i2c_reg_read() and i2c_reg_write() into inline functions

[Timur Tabi]

Mike Frysinger wrote:

> dont wait ... it'll be a bit before i post it, so it'll be easier if i just 
> rebase against that

If I post the patch that assumes that that i2c_reg_read() passes 1 instead of 0,
it'll break blackfin.

-- 
Timur Tabi
Linux kernel developer at Freescale

[U-Boot] [PATCH] i2c: merge all i2c_reg_read() and i2c_reg_write() into inline functions

[Mike Frysinger]

On Monday 20 October 2008, Timur Tabi wrote:
> Mike Frysinger wrote:
> > dont wait ... it'll be a bit before i post it, so it'll be easier if i
> > just rebase against that
>
> If I post the patch that assumes that that i2c_reg_read() passes 1 instead
> of 0, it'll break blackfin.

no it wont.  as i said, the code that's in there today is broken.  you cant 
break what's already broken.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20081021/23f015a5/attachment.pgp