Re: [PATCH] kernel 2.6.11.6 - I2C adaptor for ColdFire 5282 CPU

2005-04-18 Thread Jean Delvare 
Hi Derek,

My comments on your code:

> +Changes:
> +v0.1 26 March 2005
> + Initial Release - developed on uClinux with 2.6.9 kernel
> +v0.2 11 April 2005
> + Coding style changes
> +

Changelogs are not welcome in kernel code.

> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "i2c-mcf5282.h"

I can't see that you need config.h, errno.h nor string.h.

> + /* printk(KERN_DEBUG "%s I2DR is %.2x \n", __FUNCTION__, *rxData); */

Please don't include dead code.

> + timeout = 500;

You use a timeout of 500 in various places. I wonder if you shouldn't
have a define for it - in case it needs to be altered later.

> + while (timeout-- && !(*MCF5282_I2C_I2SR & MCF5282_I2C_I2SR_IIF))
> + udelay(1);
> + if (timeout <= 0)
> + printk(KERN_WARNING "%s - I2C IIF never set \n", __FUNCTION__);

These constructs are error-prone. I personally prefer:
while(--timeout && ...)
...
if (!timeout)
...

> + rc += mcf5282_write_data(data->word & 0x00FF);
> + rc += mcf5282_write_data((data->word & 0x00FF) >> 8);

Looks like a bug to me.

> +static s32 mcf5282_i2c_access(struct i2c_adapter *adap, u16 addr,
> +   unsigned short flags, char read_write,
> +   u8 command, int size, union i2c_smbus_data *data)
> (...)
> + printk(KERN_WARNING "Not support I2C_SMBUS_PROC_CALL yet \n");

Don't use printk when you can user dev_{dbg,warn,info,err}. Also, please
no space before newline.

> +static u32 mcf5282_func(struct i2c_adapter *adapter)
> +{
> + return I2C_FUNC_SMBUS_QUICK |
> +I2C_FUNC_SMBUS_BYTE |
> +I2C_FUNC_SMBUS_PROC_CALL |
> +I2C_FUNC_SMBUS_BYTE_DATA |
> +I2C_FUNC_SMBUS_WORD_DATA |
> +I2C_FUNC_SMBUS_BLOCK_DATA;
> +};

Don't say you support I2C_FUNC_SMBUS_PROC_CALL and
I2C_FUNC_SMBUS_BLOCK_DATA when you in fact don't. Not supporting some
functions is no problem, just leave them out for now. You can always
submit patches later.

> --- linux-2.6.11.6/drivers/i2c/busses/i2c-mcf5282.h   1969-12-31 
> 19:00:00.0 -0500
> +++ linux_dev/drivers/i2c/busses/i2c-mcf5282.h2005-04-12 
> 20:45:00.0 -0400

I see no reason to have a separate .h file for this.

> --- linux-2.6.11.6/drivers/i2c/busses/Kconfig 2005-03-25 22:28:29.0 
> -0500
> +++ linux_dev/drivers/i2c/busses/Kconfig  2005-04-12 20:45:24.0 
> -0400
> @@ -39,6 +39,16 @@ config I2C_ALI15X3
> This driver can also be built as a module.  If so, the module
> will be called i2c-ali15x3.
>  
> +config I2C_MCF5282

Please keep the entries in somewhat alphabetical order.

> --- linux-2.6.11.6/drivers/i2c/busses/Makefile2005-03-25 
> 22:28:39.0 -0500
> +++ linux_dev/drivers/i2c/busses/Makefile 2005-04-13 18:52:03.0 
> -0400
> @@ -40,6 +40,7 @@ obj-$(CONFIG_I2C_VIAPRO)+= i2c-viapro.o
>  obj-$(CONFIG_I2C_VOODOO3)+= i2c-voodoo3.o
>  obj-$(CONFIG_SCx200_ACB) += scx200_acb.o
>  obj-$(CONFIG_SCx200_I2C) += scx200_i2c.o
> +obj-$(CONFIG_I2C_MCF5282)+= i2c-mcf5282.o

Ditto.

> +#define MCF5282_I2C_I2CR_IEN(0x80)   /* I2C enable */
> +#define MCF5282_I2C_I2CR_IIEN   (0x40)  /* interrupt enable */
> +#define MCF5282_I2C_I2CR_MSTA   (0x20)  /* master/slave mode */
> +#define MCF5282_I2C_I2CR_MTX(0x10)  /* transmit/receive mode */
> +#define MCF5282_I2C_I2CR_TXAK   (0x08)  /* transmit acknowledge enable */
> +#define MCF5282_I2C_I2CR_RSTA   (0x04)  /* repeat start */
> +
> +#define MCF5282_I2C_I2SR_ICF(0x80)  /* data transfer bit */
> +#define MCF5282_I2C_I2SR_IAAS   (0x40)  /* I2C addressed as a slave */
> +#define MCF5282_I2C_I2SR_IBB(0x20)  /* I2C bus busy */
> +#define MCF5282_I2C_I2SR_IAL(0x10)  /* aribitration lost */
> +#define MCF5282_I2C_I2SR_SRW(0x04)  /* slave read/write */
> +#define MCF5282_I2C_I2SR_IIF(0x02)  /* I2C interrupt */
> +#define MCF5282_I2C_I2SR_RXAK   (0x01)  /* received acknowledge */

Parentheses are not needed here.

Thanks,
-- 
Jean Delvare
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] kernel 2.6.11.6 - I2C adaptor for ColdFire 5282 CPU

2005-04-17 Thread Greg KH 
On Wed, Apr 13, 2005 at 09:12:53PM -0400, Derek Cheung wrote:
> OK, hope this patch can satisfy everyone :-)
> 
> The following is the diffstat of the enclosed patch file:
> 
>  drivers/i2c/busses/Kconfig   |   10 
>  drivers/i2c/busses/Makefile  |1 
>  drivers/i2c/busses/i2c-mcf5282.c |  414
> +++
>  drivers/i2c/busses/i2c-mcf5282.h |   46 
>  include/asm-m68knommu/m528xsim.h |   42 +++
>  5 files changed, 513 insertions(+)
> 
> I did:
> 
> a) remove all trailing spaces in the files
> b) re-align the switch statement
> c) change a return statement
> d) change some white space intents to TABs
> e) insert a break for the I2C_SMBUS_PROC_CALL, thanks for spotting it
> f) fix the mcf5282lite wording in Kconfig
> 
> I did not:
> 
> g) use the ioremap. This is because Coldfire is a CPU without MMU and
> there is no difference between virtual and physical memory. In fact, the
> ioremap routine in the m68knommu is simply a stub routine that returns
> the input address argument for compatibility reason. Also, all other
> Coldfire CPU include files such as the m5307sim.h uses the volatile
> declaration method. 
> So, I hope this is acceptable to the Linux kernel maintainers

No, do not do this.  Even the i386 platform can get away with doing this
kind of io memory addressing, but drivers do not do that, as they are
not portable.  The first time someone wants to use this kind of i2c
adapter on a non-coldfire chip, they will have to rewrite the driver,
not acceptable.

Also, you did not include a good Changelog entry, nor a Signed-off-by:
line, and you attached the file in a mime attachment.

Please fix these issues up.

thanks,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] kernel 2.6.11.6 - I2C adaptor for ColdFire 5282 CPU

2005-04-13 Thread Derek Cheung 
OK, hope this patch can satisfy everyone :-)

The following is the diffstat of the enclosed patch file:

 drivers/i2c/busses/Kconfig   |   10 
 drivers/i2c/busses/Makefile  |1 
 drivers/i2c/busses/i2c-mcf5282.c |  414
+++
 drivers/i2c/busses/i2c-mcf5282.h |   46 
 include/asm-m68knommu/m528xsim.h |   42 +++
 5 files changed, 513 insertions(+)

I did:

a) remove all trailing spaces in the files
b) re-align the switch statement
c) change a return statement
d) change some white space intents to TABs
e) insert a break for the I2C_SMBUS_PROC_CALL, thanks for spotting it
f) fix the mcf5282lite wording in Kconfig

I did not:

g) use the ioremap. This is because Coldfire is a CPU without MMU and
there is no difference between virtual and physical memory. In fact, the
ioremap routine in the m68knommu is simply a stub routine that returns
the input address argument for compatibility reason. Also, all other
Coldfire CPU include files such as the m5307sim.h uses the volatile
declaration method. 
So, I hope this is acceptable to the Linux kernel maintainers

Please let me know if there is any question.

Regards
Derek


-Original Message-
From: Greg KH [mailto:[EMAIL PROTECTED] 
Sent: April 11, 2005 4:03 PM
To: Derek Cheung
Cc: 'Randy.Dunlap'; 'Andrew Morton'; Linux-kernel@vger.kernel.org
Subject: Re: [PATCH] kernel 2.6.11.6 - I2C adaptor for ColdFire 5282 CPU

On Sun, Apr 10, 2005 at 12:47:42PM -0400, Derek Cheung wrote:
> Enclosed please find the updated patch that incorporates changes for
all
> the comments I received.

You did not cc: the sensors mailing list, nor fix all of the coding
style issues.

> The volatile declaration in the m528xsim.h is needed because the
> declaration refers to the ColdFire 5282 register mapping.

Shouldn't you be calling ioremap, and not directly accessing a specific
register location through a pointer?  That's how all other arches do
this.

thanks,

greg k-h


linux_patch_submit3
Description: Binary data

Re: [PATCH] kernel 2.6.11.6 - I2C adaptor for ColdFire 5282 CPU

2005-04-11 Thread Greg KH 
On Sun, Apr 10, 2005 at 12:47:42PM -0400, Derek Cheung wrote:
> Enclosed please find the updated patch that incorporates changes for all
> the comments I received.

You did not cc: the sensors mailing list, nor fix all of the coding
style issues.

> The volatile declaration in the m528xsim.h is needed because the
> declaration refers to the ColdFire 5282 register mapping.

Shouldn't you be calling ioremap, and not directly accessing a specific
register location through a pointer?  That's how all other arches do
this.

thanks,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] kernel 2.6.11.6 - I2C adaptor for ColdFire 5282 CPU

2005-04-10 Thread Randy.Dunlap 
On Sun, 10 Apr 2005 12:47:42 -0400 Derek Cheung wrote:

| Enclosed please find the updated patch that incorporates changes for all
| the comments I received.

(yes, almost all)

| The volatile declaration in the m528xsim.h is needed because the
| declaration refers to the ColdFire 5282 register mapping. The volatile
| declaration is actually not needed in my I2C driver but someone may
| include the m528xsim.h file in his/her applications and we need to force
| the compiler not to do any optimization on the register mapping.

Did you also send it to the I2C mailing list like Greg asked you to do?

More comments:

diffstat summary, like so, would be nice:
 drivers/i2c/busses/Kconfig   |   10
 drivers/i2c/busses/Makefile  |2
 drivers/i2c/busses/i2c-mcf5282.c |  419 +++
 drivers/i2c/busses/i2c-mcf5282.h |   46 
 include/asm-m68knommu/m528xsim.h |   42 +++
 5 files changed, 519 insertions(+)


+   int i, len, rc = 0;
+u8 rxData, tempRxData[2];
Use tabs, not spaces.  Happens other places too.

+switch (size) {
+case I2C_SMBUS_QUICK:
Don't need to indent case statements... (repeating myself).

+case I2C_SMBUS_PROC_CALL:
+   /* dev_info(&adap->dev, "size = 
+   I2C_SMBUS_PROC_CALL \n"); */
No break needed here ?
+   case I2C_SMBUS_WORD_DATA:

+   if (rc < 0) 
+   return -1;
+   else
+   return 0;
There are several of these.  Why not just return rc ?


Kconfig says that the module (if selected) will be called
i2c-mcf5282lite, but Makefile builds
+obj-$(CONFIG_I2C_MCF5282LITE)   += i2c-mcf5282.o
(i.e., no "lite").


---
~Randy
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] kernel 2.6.11.6 - I2C adaptor for ColdFire 5282 CPU

2005-04-10 Thread Andrew Morton 
"Derek Cheung" <[EMAIL PROTECTED]> wrote:
>
> Enclosed please find the updated patch that incorporates changes for all
>  the comments I received.
> 
>  The volatile declaration in the m528xsim.h is needed because the
>  declaration refers to the ColdFire 5282 register mapping. The volatile
>  declaration is actually not needed in my I2C driver but someone may
>  include the m528xsim.h file in his/her applications and we need to force
>  the compiler not to do any optimization on the register mapping.

- Please reissue the changelog each time you reissue a patch.

- This patch adds tons of trailing whitespace.

- It breaks the x86 build.  I did this:

--- 25/drivers/i2c/busses/Kconfig~i2c-adaptor-for-coldfire-5282-cpu-fix 
2005-04-10 16:52:08.0 -0700
+++ 25-akpm/drivers/i2c/busses/Kconfig  2005-04-10 16:52:18.0 -0700
@@ -31,7 +31,7 @@ config I2C_ALI1563
 
 config I2C_MCF5282LITE
 tristate "MCF5282Lite"
-depends on I2C && EXPERIMENTAL
+depends on I2C && EXPERIMENTAL && PPC
 help
   If you say yes to this option, support will be included for the
   I2C on the ColdFire MCF5282Lite Development Board
_


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] kernel 2.6.11.6 - I2C adaptor for ColdFire 5282 CPU

2005-04-10 Thread Derek Cheung 
Enclosed please find the updated patch that incorporates changes for all
the comments I received.

The volatile declaration in the m528xsim.h is needed because the
declaration refers to the ColdFire 5282 register mapping. The volatile
declaration is actually not needed in my I2C driver but someone may
include the m528xsim.h file in his/her applications and we need to force
the compiler not to do any optimization on the register mapping.

Regards
Derek

-Original Message-
From: Randy.Dunlap [mailto:[EMAIL PROTECTED] 
Sent: April 5, 2005 11:44 PM
To: Derek Cheung
Cc: 'Andrew Morton'; [EMAIL PROTECTED]; Linux-kernel@vger.kernel.org
Subject: Re: [PATCH] kernel 2.6.11.6 - I2C adaptor for ColdFire 5282 CPU

Derek Cheung wrote:
> 
>> Below please find the patch file I "diff" against Linux 2.6.11.6. It
>> contains the I2C adaptor for ColdFire 5282 CPU. Since most ColdFire
> CPU
>> shares the same I2C register set, the code can be easily adopted for
>> other ColdFire CPUs for I2C operations.
>>
>> I have tested the code on a ColdFire 5282Lite CPU board
>> (http://www.axman.com/Pages/cml-5282LITE.html) running uClinux 2.6.9
>> with LM75 and DS1621 temperature sensor chips. As advised by David
>> McCullough, the code will be incorporated in the next uClinux
> release.
> 
>> The patch contains:
>>
>> linux/drivers/i2c/busses
>>  i2c-mcf5282.c (new file)

Limit source code lines to 80 characters (including comment lines).

+static int mcf5282_read_data():

+   if (ackType == NACK)
+   *MCF5282_I2C_I2CR |= MCF5282_I2C_I2CR_TXAK; //
generate NA
+   else
+*MCF5282_I2C_I2CR &= ~MCF5282_I2C_I2CR_TXAK;// 
generate ACK

The 2 assignments above should begin in the same column.
Also, kernel comment style is C /* ... */, not C++ (or C99) // style.

+if (timeout <= 0)
+printk("%s - I2C IIF never set. Timeout is %d \n", 
__FUNCTION__, timeout);

All printk() calls should have a KERN_WARNING or KERN_ERR or
KERN_DEBUG level used in it...

+   if (timeout <= 0 )

No space before the closing ')'.

+static int mcf5282_write_data():

+if (timeout <=0)
should be (add a space)
+if (timeout <= 0)

+   if (timeout <= 0 )
Drop space before ')'

Drop the debugging printk's and DEREK_DEBUG blocks.

+switch (size) {
+case I2C_SMBUS_QUICK:
We usually don't indent the 'case' line to save one indent level.
It helps when using 8-space tabs.

+   // this is not yet ready!!!
Put blocks like this inside
#if 0
or
#if NOT_READY_YET
#endif
blocks.

+static u32 mcf5282_func(struct i2c_adapter *adapter)
+{
+   return(I2C_FUNC_SMBUS_QUICK |
+  I2C_FUNC_SMBUS_BYTE |
+  I2C_FUNC_SMBUS_PROC_CALL |
+  I2C_FUNC_SMBUS_BYTE_DATA |
+  I2C_FUNC_SMBUS_WORD_DATA |
+  I2C_FUNC_SMBUS_BLOCK_DATA);
+};
Don't use parens on return statements.


+static int __init i2c_mcf5282_init():
is not driver registration needed?  I don't know the I2C
subsystem, so maybe not...



Big Question:  does most Coldfire or I2C use volatile so heavily,
or is it just this one driver that does that?  Volatile here
semms very overused.


-- 
~Randy


linux_patch_submit2
Description: Binary data

Re: [PATCH] kernel 2.6.11.6 - I2C adaptor for ColdFire 5282 CPU

2005-04-07 Thread Matt Mackall 
On Tue, Apr 05, 2005 at 08:10:27PM -0700, Randy.Dunlap wrote:
> There is a fairly up-to-date dontdiff file available at
> http://developer.osdl.org/rddunlap/doc/dontdiff-osdl

Can we stash a copy in Documentation?

-- 
Mathematics is the supreme nostalgia of our time.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] kernel 2.6.11.6 - I2C adaptor for ColdFire 5282 CPU

2005-04-06 Thread Greg KH 
On Tue, Apr 05, 2005 at 08:43:45PM -0700, Randy.Dunlap wrote:
> Big Question:  does most Coldfire or I2C use volatile so heavily,
> or is it just this one driver that does that?  Volatile here
> semms very overused.

It's not a i2c issue, volatile should not be needed here at all.

thanks,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] kernel 2.6.11.6 - I2C adaptor for ColdFire 5282 CPU

2005-04-05 Thread Randy.Dunlap 
Derek Cheung wrote:

Below please find the patch file I "diff" against Linux 2.6.11.6. It
contains the I2C adaptor for ColdFire 5282 CPU. Since most ColdFire
CPU
shares the same I2C register set, the code can be easily adopted for
other ColdFire CPUs for I2C operations.
I have tested the code on a ColdFire 5282Lite CPU board
(http://www.axman.com/Pages/cml-5282LITE.html) running uClinux 2.6.9
with LM75 and DS1621 temperature sensor chips. As advised by David
McCullough, the code will be incorporated in the next uClinux
release.
The patch contains:
linux/drivers/i2c/busses
i2c-mcf5282.c (new file)
Limit source code lines to 80 characters (including comment lines).
+static int mcf5282_read_data():
+	if (ackType == NACK)
+		*MCF5282_I2C_I2CR |= MCF5282_I2C_I2CR_TXAK; // generate NA
+	else
+*MCF5282_I2C_I2CR &= ~MCF5282_I2C_I2CR_TXAK;// 
generate ACK

The 2 assignments above should begin in the same column.
Also, kernel comment style is C /* ... */, not C++ (or C99) // style.
+if (timeout <= 0)
+printk("%s - I2C IIF never set. Timeout is %d \n", 
__FUNCTION__, timeout);

All printk() calls should have a KERN_WARNING or KERN_ERR or
KERN_DEBUG level used in it...
+   if (timeout <= 0 )
No space before the closing ')'.
+static int mcf5282_write_data():
+if (timeout <=0)
should be (add a space)
+if (timeout <= 0)
+   if (timeout <= 0 )
Drop space before ')'
Drop the debugging printk's and DEREK_DEBUG blocks.
+switch (size) {
+case I2C_SMBUS_QUICK:
We usually don't indent the 'case' line to save one indent level.
It helps when using 8-space tabs.
+   // this is not yet ready!!!
Put blocks like this inside
#if 0
or
#if NOT_READY_YET
#endif
blocks.
+static u32 mcf5282_func(struct i2c_adapter *adapter)
+{
+   return(I2C_FUNC_SMBUS_QUICK |
+  I2C_FUNC_SMBUS_BYTE |
+  I2C_FUNC_SMBUS_PROC_CALL |
+  I2C_FUNC_SMBUS_BYTE_DATA |
+  I2C_FUNC_SMBUS_WORD_DATA |
+  I2C_FUNC_SMBUS_BLOCK_DATA);
+};
Don't use parens on return statements.
+static int __init i2c_mcf5282_init():
is not driver registration needed?  I don't know the I2C
subsystem, so maybe not...

Big Question:  does most Coldfire or I2C use volatile so heavily,
or is it just this one driver that does that?  Volatile here
semms very overused.
--
~Randy
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] kernel 2.6.11.6 - I2C adaptor for ColdFire 5282 CPU

2005-04-05 Thread Greg KH 
On Tue, Apr 05, 2005 at 10:18:15PM -0400, Derek Cheung wrote:
> Hi Greg and Andrew,

After you fix the diff issues that Randy pointed out, please be sure to
 CC: the sensors mailing list as found in the MAINTAINERS file.  I know
they will be able to give you feedback on the code.

thanks,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] kernel 2.6.11.6 - I2C adaptor for ColdFire 5282 CPU

2005-04-05 Thread Randy.Dunlap 
Derek Cheung wrote:

Below please find the patch file I "diff" against Linux 2.6.11.6. It
contains the I2C adaptor for ColdFire 5282 CPU. Since most ColdFire
CPU
shares the same I2C register set, the code can be easily adopted for
other ColdFire CPUs for I2C operations.
I have tested the code on a ColdFire 5282Lite CPU board
(http://www.axman.com/Pages/cml-5282LITE.html) running uClinux 2.6.9
with LM75 and DS1621 temperature sensor chips. As advised by David
McCullough, the code will be incorporated in the next uClinux
release.
The patch contains:
linux/drivers/i2c/busses
i2c-mcf5282.c (new file)
i2c-mcf5282.h (new file)
Kconfig (modified)
Makefile (modified)
It also includes Kconfig.orig & Makefile.orig &
m528xsim.h.orig .
You should use
  diff -X dontdiff
where dontdiff is a filename to exclude the listed files,
where dontdiff includes *.orig .
There is a fairly up-to-date dontdiff file available at
http://developer.osdl.org/rddunlap/doc/dontdiff-osdl
A diffstat summary would (hereby requested in the future)
would let us see which files are modified and how much
they are modified:
 drivers/i2c/busses/Kconfig|   10
 drivers/i2c/busses/Kconfig.orig   |  489 
++
 drivers/i2c/busses/Makefile   |2
 drivers/i2c/busses/Makefile.orig  |   46 +++
 drivers/i2c/busses/i2c-mcf5282.c  |  407 

 drivers/i2c/busses/i2c-mcf5282.h  |   45 +++
 include/asm-m68knommu/m528xsim.h  |  112 +++
 include/asm-m68knommu/m528xsim.h.orig |   45 +++
 8 files changed, 1156 insertions(+)


 
linux/include/asm-m68knommu
 		m528xsim.h (modified)

Please let me know if you have any questions.

The patch was very wordwrapped by your email client.  Please fix that up
(first email the patch to yourself and test that the result still
applies OK) or
resend as an email attachment.
linux_dev/include/asm-m68knommu/m528xsim.h:
some spaces in this expression (& elsewhere) would make it
easier to read:
+#define MCF5282_I2C_I2ADR_ADDR(x) 
(((x)&0x7F)<<0x01)

Oh, it's not even used don't need it then.
And this one is not used:
+#define MCF5282_I2C_I2FDR_IC(x) (((x)&0x3F))
Lots of the bit-level definitions aren't used and usually aren't
added unless used.
Comment (7) doesn't match name (hm, and it's not used):
+/* Interrupt Control Register 7 */
+#define MCF5282_INTC0_ICR17 (volatile u8 *) (MCF_IPSBAR + 0x0C51)
These are not used -- but if they were, we generally like to
have macro expressions wrapped in parentheses:
+#define MCF5282_QSPI_QMRMCF_IPSBAR + 0x0340
+#define MCF5282_QSPI_QDLYR  MCF_IPSBAR + 0x0344
+#define MCF5282_QSPI_QWRMCF_IPSBAR + 0x0348
+#define MCF5282_QSPI_QIRMCF_IPSBAR + 0x034C
+#define MCF5282_QSPI_QARMCF_IPSBAR + 0x0350
+#define MCF5282_QSPI_QDRMCF_IPSBAR + 0x0354
+#define MCF5282_QSPI_QCRMCF_IPSBAR + 0x0354
i2c-mcf5282.h:
Please limit line lengths to 80 characters in source files:  e.g.:
+static int mcf5282_i2c_start(const char read_write, const u16 
target_address, const enum I2C_START_TYPE i2c_start);

What is this one for?
+void dumpReg(char *, u16 addr, u8 data);
I'm looking over the primary .c file separately now.
--
~Randy
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] kernel 2.6.11.6 - I2C adaptor for ColdFire 5282 CPU

2005-04-05 Thread Derek Cheung 
Thanks Andrew. Enclosed please find the patch file.

Regards, 
Derek

-Original Message-
From: Andrew Morton [mailto:[EMAIL PROTECTED] 
Sent: April 5, 2005 10:22 PM
To: Derek Cheung
Cc: [EMAIL PROTECTED]; Linux-kernel@vger.kernel.org
Subject: Re: [PATCH] kernel 2.6.11.6 - I2C adaptor for ColdFire 5282 CPU

"Derek Cheung" <[EMAIL PROTECTED]> wrote:
>
>  Below please find the patch file I "diff" against Linux 2.6.11.6. It
>  contains the I2C adaptor for ColdFire 5282 CPU. Since most ColdFire
CPU
>  shares the same I2C register set, the code can be easily adopted for
>  other ColdFire CPUs for I2C operations.
> 
>  I have tested the code on a ColdFire 5282Lite CPU board
>  (http://www.axman.com/Pages/cml-5282LITE.html) running uClinux 2.6.9
>  with LM75 and DS1621 temperature sensor chips. As advised by David
>  McCullough, the code will be incorporated in the next uClinux
release.
> 
>  The patch contains:
> 
>  linux/drivers/i2c/busses
>   i2c-mcf5282.c (new file)
>   i2c-mcf5282.h (new file)
>   Kconfig (modified)
>   Makefile (modified)
>   
>  linux/include/asm-m68knommu
>   m528xsim.h (modified)
> 
>  Please let me know if you have any questions.

The patch was very wordwrapped by your email client.  Please fix that up
(first email the patch to yourself and test that the result still
applies OK) or
resend as an email attachment.

Thanks.


linux_patch
Description: Binary data

Re: [PATCH] kernel 2.6.11.6 - I2C adaptor for ColdFire 5282 CPU

2005-04-05 Thread Andrew Morton 
"Derek Cheung" <[EMAIL PROTECTED]> wrote:
>
>  Below please find the patch file I "diff" against Linux 2.6.11.6. It
>  contains the I2C adaptor for ColdFire 5282 CPU. Since most ColdFire CPU
>  shares the same I2C register set, the code can be easily adopted for
>  other ColdFire CPUs for I2C operations.
> 
>  I have tested the code on a ColdFire 5282Lite CPU board
>  (http://www.axman.com/Pages/cml-5282LITE.html) running uClinux 2.6.9
>  with LM75 and DS1621 temperature sensor chips. As advised by David
>  McCullough, the code will be incorporated in the next uClinux release.
> 
>  The patch contains:
> 
>  linux/drivers/i2c/busses
>   i2c-mcf5282.c (new file)
>   i2c-mcf5282.h (new file)
>   Kconfig (modified)
>   Makefile (modified)
>   
>  linux/include/asm-m68knommu
>   m528xsim.h (modified)
> 
>  Please let me know if you have any questions.

The patch was very wordwrapped by your email client.  Please fix that up
(first email the patch to yourself and test that the result still applies OK) or
resend as an email attachment.

Thanks.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] kernel 2.6.11.6 - I2C adaptor for ColdFire 5282 CPU

2005-04-05 Thread Derek Cheung 
Hi Greg and Andrew,

Below please find the patch file I "diff" against Linux 2.6.11.6. It
contains the I2C adaptor for ColdFire 5282 CPU. Since most ColdFire CPU
shares the same I2C register set, the code can be easily adopted for
other ColdFire CPUs for I2C operations.

I have tested the code on a ColdFire 5282Lite CPU board
(http://www.axman.com/Pages/cml-5282LITE.html) running uClinux 2.6.9
with LM75 and DS1621 temperature sensor chips. As advised by David
McCullough, the code will be incorporated in the next uClinux release.

The patch contains:

linux/drivers/i2c/busses
i2c-mcf5282.c (new file)
i2c-mcf5282.h (new file)
Kconfig (modified)
Makefile (modified)
 
linux/include/asm-m68knommu
m528xsim.h (modified)

Please let me know if you have any questions.

Regards
Derek

Signed-off-by: Derek CL Cheung [EMAIL PROTECTED]
===

diff -uprN -X dontdiff linux-2.6.11.6/drivers/i2c/busses/i2c-mcf5282.c
linux_dev/drivers/i2c/busses/i2c-mcf5282.c
--- linux-2.6.11.6/drivers/i2c/busses/i2c-mcf5282.c 1969-12-31
19:00:00.0 -0500
+++ linux_dev/drivers/i2c/busses/i2c-mcf5282.c  2005-04-05
19:21:55.0 -0400
@@ -0,0 +1,407 @@
+/*
+i2c-mcf5282.c - Part of lm_sensors, Linux kernel modules for
hardware monitoring
+
+Copyright (c) 2005, Derek CL Cheung <[EMAIL PROTECTED]>
+

+
+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., 675 Mass Ave, Cambridge, MA 02139, USA.
+
+Changes:
+v0.1   26 March 2005
+   Initial Release - developed on uClinux with 2.6.9 kernel
+
+
+This I2C adaptor supports the ColdFire 5282 CPU I2C module. Since
most Coldfire
+CPUs' I2C module use the same register set (e.g., MCF5249), the
code is very
+portable and re-usable to other Coldfire CPUs.
+
+The transmission frequency is set at about 100KHz for the 5282Lite
CPU board with
+8MHz crystal. If the CPU board uses different system clock
frequency, you should
+change the following line:
+static int __init i2c_mcf5282_init(void)
+{
+.
+// Set transmission frequency 0x15 = ~100kHz
+*MCF5282_I2C_I2FDR = 0x15;
+
+}
+
+Remember to perform a dummy read to set the ColdFire CPU's I2C
module for read before
+reading the actual byte from a device
+
+The I2C_SM_BUS_BLOCK_DATA function are not yet ready but most
lm_senors do not care
+
+*/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "i2c-mcf5282.h"
+
+
+static struct i2c_algorithm mcf5282_algorithm = {
+   .name   = "MCF5282 I2C algorithm",
+   .id = I2C_ALGO_SMBUS,
+   .smbus_xfer = mcf5282_i2c_access,
+   .functionality  = mcf5282_func,
+};
+
+
+static struct i2c_adapter mcf5282_adapter = {
+   .owner  = THIS_MODULE,
+   .class  = I2C_CLASS_HWMON,
+   .algo   = &mcf5282_algorithm,
+   .name   = "MCF5282 I2C Adapter",
+};
+
+
+/*
+ *  read one byte data from the I2C bus
+ */
+static int mcf5282_read_data(u8 * const rxData, const enum I2C_ACK_TYPE
ackType) {
+
+int timeout;
+
+   *MCF5282_I2C_I2CR &= ~MCF5282_I2C_I2CR_MTX; //
master receive mode
+
+   if (ackType == NACK)
+   *MCF5282_I2C_I2CR |= MCF5282_I2C_I2CR_TXAK; //
generate NA
+   else
+*MCF5282_I2C_I2CR &= ~MCF5282_I2C_I2CR_TXAK;//
generate ACK
+
+
+   // read data from the I2C bus
+   *rxData = *MCF5282_I2C_I2DR; 
+
+   // printk(">>> %s I2DR data is %.2x \n", __FUNCTION__, *rxData);
+
+// wait for data transfer to complete 
+timeout = 500;
+while (timeout-- && !(*MCF5282_I2C_I2SR &
MCF5282_I2C_I2SR_IIF))
+udelay(1);
+if (timeout <= 0) 
+printk("%s - I2C IIF never set. Timeout is %d \n",
__FUNCTION__, timeout);
+
+
+// reset the interrupt bit 
+*MCF5282_I2C_I2SR &= ~MCF5282_I2C_I2SR_IIF;
+
+   if (timeout <= 0 ) 
+return -1;
+else 
+