Re: [uClinux-dev] Re: [PATCH v2] m68knommu: driver for Freescale Coldfire I2C controller.

2010-03-12 Thread Lennart Sorensen
On Fri, Mar 12, 2010 at 12:04:53PM +0100, Philippe De Muyter wrote:
> Hello all,
> 
> On Mon, Jan 25, 2010 at 11:56:30AM -0800, Steven King wrote:
> > > >
> > > > Add support for the I2C controller used on Freescale/Motorola Coldfire
> > > > MCUs.
> > > >
> > > > Signed-off-by: Steven King 
> 
> What's the status of this ?
> 
> I need to use i2c for a coldfire uclinux project (with a mcf5484) and I
> now have 3 different coldfire i2c drivers, none of which is in mainline.
> 
> i2c-mcf.c (from uClinux-dist-20090618, but not in 
> http://git.kernel.org/?p=linux/kernel/git/gerg/m68knommu.git;a=summary)

It seems I use that one based on the filename.

> i2c-mcf548x.c (from ltib-m5475evb-20080808, found as "Linux BSP for 
> MCF5484LITE, MCF5475/85EVB" at 
> http://www.freescale.com/webapp/sps/site/prod_summary.jsp?code=MCF548X&fpsp=1&tab=Design_Tools_Tab)
> i2c-coldfire.c (from http://lkml.org/lkml/2010/1/11/165)
> 
> I like to work with mainline sources to be be able to contribute to and 
> benefit
> from collective work.
> 
> Which one has the best chances to be put in mainline ?

No idea.

I am using this one on a 5271:

http://www.bitshrine.org/gpp/0006-I2C-device-driver.patch
http://www.bitshrine.org/gpp/0043-I2C-bug-fix.patch

No problems so far.

This is the patch that I use on top of
git://git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k.git

diff -urN 
build-2.6.29-1-m68k-uclinux-target1-nfs/arch/m68k/include/asm/m527xsim.h 
build-2.6.29-1-m68k-uclinux-target1/arch/m68k/include/asm/m527xsim.h
--- build-2.6.29-1-m68k-uclinux-target1-nfs/arch/m68k/include/asm/m527xsim.h
2010-02-12 11:49:19.0 -0500
+++ build-2.6.29-1-m68k-uclinux-target1/arch/m68k/include/asm/m527xsim.h
2010-02-12 12:10:59.0 -0500
@@ -248,5 +248,44 @@
 #defineMCF_RCR_SWRESET 0x80/* Software reset bit */
 #defineMCF_RCR_FRCSTOUT0x40/* Force external reset 
*/
 
+/*
+*
+* I2C Module (I2C) (Stolen from m5301xsim.h)
+*
+*/
+/* Register read/write macros */
+#ifdef CONFIG_M5271
+#define MCF_I2C_I2ADR   (volatile unsigned char *)(MCF_MBAR + 0x300)
+#define MCF_I2C_I2FDR   (volatile unsigned char *)(MCF_MBAR + 0x304)
+#define MCF_I2C_I2CR(volatile unsigned char *)(MCF_MBAR + 0x308)
+#define MCF_I2C_I2SR(volatile unsigned char *)(MCF_MBAR + 0x30C)
+#define MCF_I2C_I2DR(volatile unsigned char *)(MCF_MBAR + 0x310)
+
+/* Bit definitions and macros for I2AR */
+#define MCF_I2C_I2AR_ADR(x) (((x)&0x7F)<<1)
+
+/* Bit definitions and macros for I2FDR */
+#define MCF_I2C_I2FDR_IC(x) (((x)&0x3F))
+
+/* Bit definitions and macros for I2CR */
+#define MCF_I2C_I2CR_RSTA   (0x04)
+#define MCF_I2C_I2CR_TXAK   (0x08)
+#define MCF_I2C_I2CR_MTX(0x10)
+#define MCF_I2C_I2CR_MSTA   (0x20)
+#define MCF_I2C_I2CR_IIEN   (0x40)
+#define MCF_I2C_I2CR_IEN(0x80)
+
+/* Bit definitions and macros for I2SR */
+#define MCF_I2C_I2SR_RXAK   (0x01)
+#define MCF_I2C_I2SR_IIF(0x02)
+#define MCF_I2C_I2SR_SRW(0x04)
+#define MCF_I2C_I2SR_IAL(0x10)
+#define MCF_I2C_I2SR_IBB(0x20)
+#define MCF_I2C_I2SR_IAAS   (0x40)
+#define MCF_I2C_I2SR_ICF(0x80)
+
+/* Bit definitions and macros for I2DR */
+#define MCF_I2C_I2DR_DATA(x)(x)
+#endif /* I2C Module for CONFIG_M5271 */
 //
 #endif /* m527xsim_h */
diff -urN build-2.6.29-1-m68k-uclinux-target1-nfs/drivers/i2c/busses/Kconfig 
build-2.6.29-1-m68k-uclinux-target1/drivers/i2c/busses/Kconfig
--- build-2.6.29-1-m68k-uclinux-target1-nfs/drivers/i2c/busses/Kconfig  
2010-02-12 11:49:21.0 -0500
+++ build-2.6.29-1-m68k-uclinux-target1/drivers/i2c/busses/Kconfig  
2010-02-12 12:10:59.0 -0500
@@ -417,6 +417,16 @@
  This driver is deprecated and will be dropped soon. Use i2c-gpio
  instead.
 
+config I2C_MCF
+   tristate "MCF ColdFire"
+   depends on I2C && EXPERIMENTAL
+   help
+ If you say yes to this option, support will be included for the
+ I2C on most ColdFire CPUs
+
+ This driver can also be built as a module.  If so, the module
+ will be called i2c-mcf.
+
 config I2C_MPC
tristate "MPC107/824x/85xx/52xx/86xx"
depends on PPC32
diff -urN build-2.6.29-1-m68k-uclinux-target1-nfs/drivers/i2c/busses/Makefile 
build-2.6.29-1-m68k-uclinux-target1/drivers/i2c/busses/Makefile
--- build-2.6.29-1-m68k-uclinux-target1-nfs/drivers/i2c/busses/Makefile 
2010-02-12 11:49:21.0 -0500
+++ build-2.6.29-1-m68k-uclinux-target1/drivers/i2c/busses/Makefile 
2010-02-12 12:10:59.0 -0500
@@ -71,6 +71,7 @@
 obj-$(CONFIG_I2C_STUB) += i2c-stub.o
 obj-$(CONFIG_SCx200_ACB)   += scx200_acb.o
 obj-$(CONFIG_SCx200_I2C)

Re: [uClinux-dev] Re: [PATCH v2] m68knommu: driver for Freescale Coldfire I2C controller.

2010-03-12 Thread Philippe De Muyter
Hello all,

On Mon, Jan 25, 2010 at 11:56:30AM -0800, Steven King wrote:
> > >
> > > Add support for the I2C controller used on Freescale/Motorola Coldfire
> > > MCUs.
> > >
> > > Signed-off-by: Steven King 

What's the status of this ?

I need to use i2c for a coldfire uclinux project (with a mcf5484) and I
now have 3 different coldfire i2c drivers, none of which is in mainline.

i2c-mcf.c (from uClinux-dist-20090618, but not in 
http://git.kernel.org/?p=linux/kernel/git/gerg/m68knommu.git;a=summary)
i2c-mcf548x.c (from ltib-m5475evb-20080808, found as "Linux BSP for 
MCF5484LITE, MCF5475/85EVB" at 
http://www.freescale.com/webapp/sps/site/prod_summary.jsp?code=MCF548X&fpsp=1&tab=Design_Tools_Tab)
i2c-coldfire.c (from http://lkml.org/lkml/2010/1/11/165)

I like to work with mainline sources to be be able to contribute to and benefit
from collective work.

Which one has the best chances to be put in mainline ?

Best regards

Philippe
___
uClinux-dev mailing list
uClinux-dev@uclinux.org
http://mailman.uclinux.org/mailman/listinfo/uclinux-dev
This message was resent by uclinux-dev@uclinux.org
To unsubscribe see:
http://mailman.uclinux.org/mailman/options/uclinux-dev


[uClinux-dev] Re: [PATCH v2] m68knommu: driver for Freescale Coldfire I2C controller.

2010-01-25 Thread Steven King
On Sunday 24 January 2010 07:15:19 you wrote:
> On Mon, Jan 11, 2010 at 10:24:05AM -0800, Steven King wrote:
> > Changes for this version:
> > rename drivers/i2c/busses/i2c-mcf.c to drivers/i2c/busses/i2c-coldfire.c
> > use I2C_COLDFIRE in drivers/i2c/busses/{Kconfig,Makefile}
> >
> > --
> >
> > Add support for the I2C controller used on Freescale/Motorola Coldfire
> > MCUs.
> >
> > Signed-off-by: Steven King 
>
> The commit messsage should go first, the changelog and other stuff
> that won't go in should go beflore the --- line.

My bad, I think I was paying more attention to making sure this mailer didn't 
line wrap on me.  I can repost if you want.

> > +static irqreturn_t mcfi2c_irq_handler(int this_irq, void *dev_id)
> > +{
> > +   struct mcfi2c *mcfi2c = dev_id;
> > +
> > +   /* clear interrupt */
> > +   mcfi2c_wr_sr(mcfi2c, 0);
> > +   complete(&mcfi2c->completion);
> > +
> > +   return IRQ_HANDLED;
> > +}
>
> I'm interested in why you don't just handle the interrupt here and
> wake the thread once all the data is handled?

No particular reason; when I started working on this I looked at how some of 
the other drivers in drivers/i2c/busses were implemented, found one whose 
workings I could understand without knowing anything about that particular 
SoC (which one I don't remember) and used it as a model to adapt the i2c code 
I use with other Freescale/Motorola MCUs that don't run linux (there is very 
little difference in the i2c controller on an 8-bit hc08, 16 bit hc11/12, 
32bit Coldfire v1 and the Coldfire v2[+] that are currently supported).  So 
other than having easy to understand (for me atleast) code that shared a lot 
of similarity with the code I use for the some of the other  systems I code 
for, no real reason.  That and I find statefull irq handlers disconcerting.

> > +   status = request_irq(mcfi2c->irq, mcfi2c_irq_handler, IRQF_DISABLED,
> > +pdev->name, mcfi2c);
>
> do you really need IRQF_DISABLED here? your irq handler hardly does
> anything.

Yes, without it, I was getting a spurious interrupt (been there, done that).

> > +   if (status) {
> > +   dev_dbg(&pdev->dev, "request_irq failed\n");
> > +   goto fail2;
> > +   }
> > +
> > +   mcfi2c->clk = clk_get(&pdev->dev, "i2c_clk");
>
> hmm, think the default device clock should be findable by clk_get(dev,
> NULL).

Interesting.   Again, I had looked at the existing code in drivers/i2c/busses 
to see what the other drivers were doing and most were passing an id string.

Its a trivial change so its no big deal, but I'm curious, absent anything in 
Documentation, if the bulk of the existing code isn't documentation for the 
correct use of an api, then what is?

-- 
Steven King -- sfking at fdwdc dot com
___
uClinux-dev mailing list
uClinux-dev@uclinux.org
http://mailman.uclinux.org/mailman/listinfo/uclinux-dev
This message was resent by uclinux-dev@uclinux.org
To unsubscribe see:
http://mailman.uclinux.org/mailman/options/uclinux-dev