Subject: Re: [PATCH] enhanced i2c driver for MPC8xx/MPC8260 CPM ...

From: Murray Jensen <[email-masked]>
Date: Fri, 31 May 2002 11:32:41 +1000

On Fri, 31 May 2002 10:09:00 +1000, Rosen Jeremy <[email-masked]> writes:
>Hello, I have just applied your patch, and would like to add a few comments

I'm glad someone has tried it...

>I beleive you use the denx kernel, which is not 100% compatible with the
>2.4.4-devel one.

We've hit a snag straight away - I use linuxppc_2_4_devel grabbed via bitkeeper
from "bk://ppc.bkbits.net/linuxppc_2_4_devel"

>In particular, commproc.h is in asm/ in the devel kernel.

That's where it is in mine also. Actually, its in "include/asm-ppc" -
"include/asm" is just a symlink.

Oh, I see the problem - I didn't update the #include in
"drivers/i2c/i2c-algo-cpm.c" - *sigh* I thought I had done this - the others in
"drivers/i2c/i2c-cpm.c" and "drivers/i2c/i2c-rpx.c" were done.

>The patch did not apply properly, and I had to do some trivial editing to
>make
>it work (note that my last pull from 2.4-devel is quite old)

My last pull from "bk://ppc.bkbits.net/linuxppc_2_4_devel" was a couple of
weeks ago. If yours is very old, you will probably have trouble with at least
"arch/ppc/kernel/ppc_ksyms.c" (but it should be obvious what is required in
there).

>you also need to include commproc.h in i2c-algo-cpm.h, because this is where
>iic_t is defined

I think the file that includes <linux/i2c-algo-cpm.h> is expected to include
commproc.h, or cpm_8260.h, as required. What is the policy on these things?
Should every include file include all headers it needs? Anyone?

The original <linux/i2c-algo-8xx.h> file only included <linux/i2c.h>, yet
referenced the "i2c8xx_t" and "cpm8xx_t" types. I simply did the same.

>in arch/ppc/8xx_io/commproc.c you reference a cp variable that oesn't exist, I
>changed it to cpmp, which is probably what you  meant

Yes, indeed - you are correct (when I made those additions, there was a cp
local variable - it got removed later).

OK, as I said in my message, I haven't tested this on the 8xx platform.
Obviously, it needed it - but I think we can work together to get it going.

>you include the same header twice in i2c-algo-cpm : asm/mpc8xx.h

It is only included once in my source.

I have attached two patches - the first is a further patch to the previously
posted patch, the second is the entire thing in one patch. These fix the obvious
compile problems you point out.

Note that I have a separate repository which has the vanilla linuxppc_2_4_devel
as it's parent, and only contains these i2c patches. The last time I pulled was
a couple of weeks ago - there shouldn't be any of my other local stuff
interfering with this.

Should we take this discussion off the list now? Cheers!
								Murray...
--
Murray Jensen, CSIRO Manufacturing Sci & Tech,         Phone: +61 3 9662 7763
Locked Bag No. 9, Preston, Vic, 3072, Australia.         Fax: +61 3 9662 7853
Internet: [email-masked]

Hymod project: http://www.msa.cmst.csiro.au/projects/Hymod/