[RFC][PATCH] I2C driver

[RFC][PATCH] I2C driver

[Steven A. Falco]

The following patch enables building the I2C driver for 4xx chips.
Tested on a Sequoia board.  Comments invited.

Signed-off-by: Steven A. Falco <sfalco@harris.com>
---
 drivers/i2c/busses/Kconfig      |    7 +++++++
 drivers/i2c/busses/i2c-ibm_of.c |    5 ++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index e9f88fe..6444030 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -247,6 +247,13 @@ config I2C_PIIX4
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-piix4.
 
+config I2C_IBM_OF
+	tristate "IBM PPC 4xx open-firmware driver"
+	depends on 4xx
+	help
+	  Say Y here if you want to use the open-firmware driver for
+          IIC peripheral found on embedded IBM PPC 4xx based systems.
+
 config I2C_IBM_IIC
 	tristate "IBM PPC 4xx on-chip I2C interface"
 	depends on 4xx
diff --git a/drivers/i2c/busses/i2c-ibm_of.c b/drivers/i2c/busses/i2c-ibm_of.c
index 08440ab..caa895d 100644
--- a/drivers/i2c/busses/i2c-ibm_of.c
+++ b/drivers/i2c/busses/i2c-ibm_of.c
@@ -699,9 +699,7 @@ static int __init of_find_i2c_driver(struct device_node *node,
 	for (i = 0; i < ARRAY_SIZE(i2c_devices); i++) {
 		if (!of_device_is_compatible(node, i2c_devices[i].of_device))
 			continue;
-		if (strlcpy(info->driver_name, i2c_devices[i].i2c_driver,
-			    KOBJ_NAME_LEN) >= KOBJ_NAME_LEN ||
-		    strlcpy(info->type, i2c_devices[i].i2c_type,
+		if (strlcpy(info->type, i2c_devices[i].i2c_type,
 			    I2C_NAME_SIZE) >= I2C_NAME_SIZE)
 			return -ENOMEM;
 		return 0;
@@ -906,6 +904,7 @@ static const struct of_device_id ibm_iic_match[] = {
 	{ .compatible = "ibm,iic-405ex", },
 	{ .compatible = "ibm,iic-405exr", },
 	{ .compatible = "ibm,iic-405gp", },
+	{ .compatible = "ibm,iic-440epx", },
 	{ .compatible = "ibm,iic-440gp", },
 	{ .compatible = "ibm,iic-440gpx", },
 	{ .compatible = "ibm,iic-440grx", },
-- 
1.5.5.1

Re: [RFC][PATCH] I2C driver

[Josh Boyer]

On Thu, 2008-08-21 at 12:21 -0400, Steven A. Falco wrote:
> The following patch enables building the I2C driver for 4xx chips.
> Tested on a Sequoia board.  Comments invited.
> 
> Signed-off-by: Steven A. Falco <sfalco@harris.com>
> ---
>  drivers/i2c/busses/Kconfig      |    7 +++++++
>  drivers/i2c/busses/i2c-ibm_of.c |    5 ++---
>  2 files changed, 9 insertions(+), 3 deletions(-)

That file doesn't even exist in the mainline kernel:

[jwboyer@yoda linux-2.6]$ find drivers/i2c/ -name *ibm*
drivers/i2c/busses/i2c-ibm_iic.c
drivers/i2c/busses/i2c-ibm_iic.h
[jwboyer@yoda linux-2.6]$ 

josh

Re: [RFC][PATCH] I2C driver

[Steven A. Falco]

Josh Boyer wrote:
> On Thu, 2008-08-21 at 12:21 -0400, Steven A. Falco wrote:
>> The following patch enables building the I2C driver for 4xx chips.
>> Tested on a Sequoia board.  Comments invited.
>>
>> Signed-off-by: Steven A. Falco <sfalco@harris.com>
>> ---
>>  drivers/i2c/busses/Kconfig      |    7 +++++++
>>  drivers/i2c/busses/i2c-ibm_of.c |    5 ++---
>>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> That file doesn't even exist in the mainline kernel:
> 
> [jwboyer@yoda linux-2.6]$ find drivers/i2c/ -name *ibm*
> drivers/i2c/busses/i2c-ibm_iic.c
> drivers/i2c/busses/i2c-ibm_iic.h
> [jwboyer@yoda linux-2.6]$ 
> 
> josh
> 
> 

Interesting.  Ok - I found the problem with the driver you *did* find:

diff --git a/drivers/i2c/busses/i2c-ibm_iic.c b/drivers/i2c/busses/i2c-ibm_iic.c
index 85dbf34..569161d 100644
--- a/drivers/i2c/busses/i2c-ibm_iic.c
+++ b/drivers/i2c/busses/i2c-ibm_iic.c
@@ -989,6 +989,7 @@ static int __devexit iic_remove(struct of_device *ofdev)
 static const struct of_device_id ibm_iic_match[] = {
 	{ .compatible = "ibm,iic-405ex", },
 	{ .compatible = "ibm,iic-405gp", },
+	{ .compatible = "ibm,iic-440epx", },
 	{ .compatible = "ibm,iic-440gp", },
 	{ .compatible = "ibm,iic-440gpx", },
 	{ .compatible = "ibm,iic-440grx", },

However, while the i2c-ibm_of.c driver works with the sequoia .dts,
i2c-ibm_iic.c does not, because it is looking for an "index" property,
which is not in the .dts file.  I added one:

diff --git a/arch/powerpc/boot/dts/sequoia.dts b/arch/powerpc/boot/dts/sequoia.dts
index 72d6756..538aadb 100644
--- a/arch/powerpc/boot/dts/sequoia.dts
+++ b/arch/powerpc/boot/dts/sequoia.dts
@@ -248,6 +248,7 @@
 				reg = <ef600700 14>;
 				interrupt-parent = <&UIC0>;
 				interrupts = <2 4>;
+				index = <0>;
 			};
 
 			IIC1: i2c@ef600800 {
@@ -255,6 +256,7 @@
 				reg = <ef600800 14>;
 				interrupt-parent = <&UIC0>;
 				interrupts = <7 4>;
+				index = <1>;
 			};
 
 			ZMII0: emac-zmii@ef600d00 {

However, something is still not right.  Using the mystery i2c-ibm_of.c
driver, I get:

i2c /dev entries driver
IBM IIC driver v2.1
m41t80_probe called
rtc-m41t80 0-0068: chip found, driver version 0.05
rtc-m41t80 0-0068: rtc core: registered m41t80 as rtc0
ibm-iic(/plb/opb/i2c@ef600700): using standard (100 kHz) mode
ibm-iic(/plb/opb/i2c@ef600800): using standard (100 kHz) mode

But with the i2c-ibm_iic.c driver, I get:

i2c /dev entries driver
ibm-iic 1ef600700.i2c: using standard (100 kHz) mode
ibm-iic 1ef600800.i2c: using standard (100 kHz) mode

I.e. it doesn't find the rtc (which I added to my sequoia board).

	Steve

Re: [RFC][PATCH] I2C driver

[Josh Boyer]

On Thu, 21 Aug 2008 13:06:24 -0400
"Steven A. Falco" <sfalco@harris.com> wrote:

> Josh Boyer wrote:
> > On Thu, 2008-08-21 at 12:21 -0400, Steven A. Falco wrote:
> >> The following patch enables building the I2C driver for 4xx chips.
> >> Tested on a Sequoia board.  Comments invited.
> >>
> >> Signed-off-by: Steven A. Falco <sfalco@harris.com>
> >> ---
> >>  drivers/i2c/busses/Kconfig      |    7 +++++++
> >>  drivers/i2c/busses/i2c-ibm_of.c |    5 ++---
> >>  2 files changed, 9 insertions(+), 3 deletions(-)
> > 
> > That file doesn't even exist in the mainline kernel:
> > 
> > [jwboyer@yoda linux-2.6]$ find drivers/i2c/ -name *ibm*
> > drivers/i2c/busses/i2c-ibm_iic.c
> > drivers/i2c/busses/i2c-ibm_iic.h
> > [jwboyer@yoda linux-2.6]$ 
> > 
> > josh
> > 
> > 
> 
> Interesting.  Ok - I found the problem with the driver you *did* find:

.26 doesn't have what can be considered awesome i2c support.  But the
latest git trees are much better.  See git commits:

d3dc685eb5ef64aa695dabb74f00440ec3ab6796
and
b1204e6ec16468ebf89d9d818bfe425ca7adcdf3

They deal with the problems you highlighted with the listing and index
stuff.  There might be more issues outstanding, but using the latest
kernel is going to get you better support and prevent you from finding
problems that are already fixed :)

josh

Re: [RFC][PATCH] I2C driver

[Sean MacLennan]

On Thu, 21 Aug 2008 13:06:24 -0400
"Steven A. Falco" <sfalco@harris.com> wrote:

> However, while the i2c-ibm_of.c driver works with the sequoia .dts,
> i2c-ibm_iic.c does not, because it is looking for an "index" property,
> which is not in the .dts file.  I added one:

I don't know where i2c-ibm_of.c came from.... but in the 2.6.26 kernel
you need the index property.

The 2.6.27 kernel has a fully OF aware i2c-ibm_iic driver. You do
not need the index. And it will walk the device tree and setup the
devices for you.

Cheers,
   Sean

Re: [RFC][PATCH] I2C driver

[Stefan Roese]

On Thursday 21 August 2008, Sean MacLennan wrote:
> On Thu, 21 Aug 2008 13:06:24 -0400
>
> "Steven A. Falco" <sfalco@harris.com> wrote:
> > However, while the i2c-ibm_of.c driver works with the sequoia .dts,
> > i2c-ibm_iic.c does not, because it is looking for an "index" property,
> > which is not in the .dts file.  I added one:
>
> I don't know where i2c-ibm_of.c came from....

It's a temporary DENX version for arch/powerpc which we created, since it took 
too long to get something accepted upstream (I remember endless discussions 
about index properties...). And we needed something functional quite a while 
ago.

> but in the 2.6.26 kernel 
> you need the index property.
>
> The 2.6.27 kernel has a fully OF aware i2c-ibm_iic driver. You do
> not need the index. And it will walk the device tree and setup the
> devices for you.

Yes. Since 2.6.27 the "original" IBM I2C driver should be enough. We will drop 
our local special version soon.

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@denx.de
=====================================================================

Re: [RFC][PATCH] I2C driver

[Steven A. Falco]

Stefan Roese wrote:
> On Thursday 21 August 2008, Sean MacLennan wrote:
>> On Thu, 21 Aug 2008 13:06:24 -0400
>>
>> "Steven A. Falco" <sfalco@harris.com> wrote:
>>> However, while the i2c-ibm_of.c driver works with the sequoia .dts,
>>> i2c-ibm_iic.c does not, because it is looking for an "index" property,
>>> which is not in the .dts file.  I added one:
>> I don't know where i2c-ibm_of.c came from....
> 
> It's a temporary DENX version for arch/powerpc which we created, since it took 
> too long to get something accepted upstream (I remember endless discussions 
> about index properties...). And we needed something functional quite a while 
> ago.
> 
>> but in the 2.6.26 kernel 
>> you need the index property.
>>
>> The 2.6.27 kernel has a fully OF aware i2c-ibm_iic driver. You do
>> not need the index. And it will walk the device tree and setup the
>> devices for you.
> 
> Yes. Since 2.6.27 the "original" IBM I2C driver should be enough. We will drop 
> our local special version soon.
> 
> 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@denx.de
> =====================================================================
> 

I was able to cherry-pick the i2c-ibm_iic driver from .27 into .26, so
I withdraw my comments regarding the soon-to-be-discarded i2c-ibm_of
driver.

	Steve