[U-Boot-Users] [Patch]: A small patch for PPC4xx I2C driver

[U-Boot-Users] [Patch]: A small patch for PPC4xx I2C driver

[Vladimir Gurevich]

Hello,

Here is a small and obvious patch for PPC4xx I2C driver that
makes it "API-compatible" with other I2C drivers. I had to
do it in order to be able to use DS1307 driver, which worked
right out of the box after that (thanks, Steven Scholz!)

Comments, criticisms, suggestions are very welcome.
Vladimir Gurevich
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: i2c.diff
Url: http://lists.denx.de/pipermail/u-boot/attachments/20030207/157d1b55/attachment.txt 

[U-Boot-Users] [Patch]: A small patch for PPC4xx I2C driver

[Frank]

You should probably comment the write API. The "i2c_write"
function takes the value to be wriiten as a refernce but your
API implies the parameter could be a literal. You can't pass the
address of a literal...
--- Vladimir Gurevich <vag@paulidav.org> wrote:
> Hello,
> 
> Here is a small and obvious patch for PPC4xx I2C driver that
> makes it "API-compatible" with other I2C drivers. I had to
> do it in order to be able to use DS1307 driver, which worked
> right out of the box after that (thanks, Steven Scholz!)
> 
> Comments, criticisms, suggestions are very welcome.
> Vladimir Gurevich
> > Index: cpu/ppc4xx/i2c.c
>
===================================================================
> RCS file: /cvsroot/u-boot/u-boot/cpu/ppc4xx/i2c.c,v
> retrieving revision 1.1.1.1
> diff -c -b -w -r1.1.1.1 i2c.c
> *** cpu/ppc4xx/i2c.c	3 Nov 2002 00:32:03 -0000	1.1.1.1
> --- cpu/ppc4xx/i2c.c	8 Feb 2003 01:26:35 -0000
> ***************
> *** 414,417 ****
> --- 414,436 ----
>           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)
> + {
> + 	char 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);
> + }
>   #endif	/* CONFIG_HARD_I2C */
> 


__________________________________________________
Do you Yahoo!?
Yahoo! Mail Plus - Powerful. Affordable. Sign up now.
http://mailplus.yahoo.com

[U-Boot-Users] [Patch]: A small patch for PPC4xx I2C driver

[Vladimir Gurevich]

Hi Frank,

This code has been copied verbatim from common/soft_i2c.c
and is identical to the implementations in other drivers like
cpu/mpc8260/i2c.c for example. It is done this way
because the prototype for i2c_write is

int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len);

As you can see it requires an address of the buffer.

Since this is a real function and not a macro,
it will always work even if the actual parameter is
a literal, there is nothing wrong about it. Because the
prototype is

void i2c_reg_write(uchar i2c_addr, uchar reg, uchar val)

you don't have to pass an address of a value,
just the value itself.

If you look at the code that the compiler generates,
youll see it does the right thing (comments are mine):

(gdb) disas i2c_reg_write
Dump of assembler code for function i2c_reg_write:
# Make a stack frame
0x7fe8a80 <i2c_reg_write>:      stwu    r1,-16(r1)

# Store the return address on the stack
0x7fe8a84 <i2c_reg_write+4>:    mflr    r0
0x7fe8a88 <i2c_reg_write+8>:    stw     r0,20(r1)

# Store "val" on the stack
0x7fe8a8c <i2c_reg_write+12>:   stb     r5,8(r1)

# Set "alen" to 1
0x7fe8a90 <i2c_reg_write+16>:   li      r5,1

# set buffer = &val  (it is r1+8)
0x7fe8a94 <i2c_reg_write+20>:   addi    r6,r1,8

# set "len" to 1
0x7fe8a98 <i2c_reg_write+24>:   li      r7,1

# r3 has i2c_addr, that is chip
# r4 has reg
# i2c_write(chip, reg, 1, &val, 1)
0x7fe8a9c <i2c_reg_write+28>:   bl      0x7fe89a0 <i2c_write>

# restore the return address
0x7fe8aa0 <i2c_reg_write+32>:   lwz     r0,20(r1)
0x7fe8aa4 <i2c_reg_write+36>:   mtlr    r0

# remove the stack frame
0x7fe8aa8 <i2c_reg_write+40>:   addi    r1,r1,16

#return
0x7fe8aac <i2c_reg_write+44>:   blr
End of assembler dump.
(gdb)


Happy hacking,
Vladimir

[U-Boot-Users] [Patch]: A small patch for PPC4xx I2C driver

[Wolfgang Denk]

In message <20030208031610.16267.qmail@web13801.mail.yahoo.com> you wrote:
> You should probably comment the write API. The "i2c_write"
> function takes the value to be wriiten as a refernce but your
> API implies the parameter could be a literal. You can't pass the
> address of a literal...

??? What are you talking about?

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd at denx.de
Your own mileage may vary.

[U-Boot-Users] [Patch]: A small patch for PPC4xx I2C driver

[Wolfgang Denk]

In message <3E445E77.5050706@paulidav.org> you wrote:
> 
> Here is a small and obvious patch for PPC4xx I2C driver that
> makes it "API-compatible" with other I2C drivers. I had to
> do it in order to be able to use DS1307 driver, which worked
> right out of the box after that (thanks, Steven Scholz!)

Added. Sorry it took so long. Will show up on CVS in a couple of hours.

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd at denx.de
For those who like this sort of thing, this is the sort of thing they
like.                                               - Abraham Lincoln

[U-Boot-Users] [Patch]: A small patch for PPC4xx I2C driver

[Vladimir Gurevich]

Thanks, Wolfgang!

But what about DS1307 driver itself? I took Steven's
patch from the list, it applied cleanly and the driver
seems to work very well. At least, that was the main 
reason for the patch, anyway...

Best regards,
Vladimir

Wolfgang Denk wrote:
> In message <3E445E77.5050706@paulidav.org> you wrote:
> 
>>Here is a small and obvious patch for PPC4xx I2C driver that
>>makes it "API-compatible" with other I2C drivers. I had to
>>do it in order to be able to use DS1307 driver, which worked
>>right out of the box after that (thanks, Steven Scholz!)
> 
> 
> Added. Sorry it took so long. Will show up on CVS in a couple of hours.
> 
> Best regards,
> 
> Wolfgang Denk
> 

[U-Boot-Users] MC68360 Port

[Reinhard Meyer]

Hello,

has anyone ever thought about a port for the

<ducking down>
    MC68360 ?
</ducking down>

Although there is no use for booting LinuX there I like the features like
TFTP, good exploration possibilites of the hardware (md, imd, ports etc). I
think there is no need to limit U-Boot to LinuX-able processors only...

On that account, can anyone point me or send me a GNU toolchain for 68K? I
do not want to go through configuring AND compiling one myself if that is
avoidable.... ;)

Reinhard

[U-Boot-Users] MC68360 Port

[Wolfgang Denk]

Dear Reinhard,

in message <01c401c2e427$88e01700$6d4ba8c0@alb.sub.de> you wrote:
> 
> has anyone ever thought about a port for the
> 
> <ducking down>

Why?

>     MC68360 ?

Yes, of course. Just waiting for a customer who asks for it.

> Although there is no use for booting LinuX there I like the features like
> TFTP, good exploration possibilites of the hardware (md, imd, ports etc). I
> think there is no need to limit U-Boot to LinuX-able processors only...

100% correct.

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd at denx.de
Repeat after me:
Usenet is not a word processor; it's a medium where aesthetics count.
Mozilla is not a newsreader; it's a web browser.
Windows is not an operating system; it's a GUI on a program loader.
         -- Tom Christiansen in <6bq0g5$lr4$2@csnews.cs.colorado.edu>