LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Scott Wood @ 2007-11-06 17:02 UTC (permalink / raw)
  To: Jon Smirl; +Cc: Tjernlund, linuxppc-dev, Jean Delvare, i2c
In-Reply-To: <9e4733910711051641m5ff0e651ke9a43daa51fd14b@mail.gmail.com>

Jon Smirl wrote:
> On 11/5/07, Scott Wood <scottwood@freescale.com> wrote:
>>>>> One side effect is that legacy style i2c drivers won't work anymore.
>>>> If you mean legacy-style client drivers, why not?
>>> i2c_new_device() doesn't work with legacy-style client drivers.
>> No, but they should still work the old way.
> 
> I'm not in favor trying to support both legacy and new style i2c
> drivers.

I don't understand what it is that you did that would break support for 
legacy clients, though.

>  It took me all of five minutes to convert an existing legacy
> driver to the new style. Pretty much all you need to do is delete code
> (about 100 lines). So I'd recommend converting the drivers we are
> interest in instead of trying to support both types.

Sure, conversion is good, but that doesn't mean we want things to 
suddenly break for users.

-Scott

^ permalink raw reply

* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Jean Delvare @ 2007-11-06 17:29 UTC (permalink / raw)
  To: Scott Wood; +Cc: Tjernlund, linuxppc-dev, i2c
In-Reply-To: <472F8267.8070106@freescale.com>

Hi Scott, Jon,

On Mon, 05 Nov 2007 14:51:51 -0600, Scott Wood wrote:
> Jon Smirl wrote:
> > How about renaming the old driver file and leaving it hooked to ppc?
> > Then it would get deleted when ppc goes away. That would let work
> > progress on the powerpc version.
> 
> Or we could have one driver that has two probe methods.  I don't like 
> forking the driver.

I agree with Scott here, I don't want to fork the drivers. It is
possible (and easy) to support both methods in the same module, let's
just to that. See for example David Brownell's work on the lm75 driver:
http://lists.lm-sensors.org/pipermail/lm-sensors/2007-September/021270.html

> > i2c_new_device() doesn't work with legacy-style client drivers.
> 
> No, but they should still work the old way.

Definitely.

> > This is not hard to do but the i2c people will have to agree. I need
> > to change the i2c_driver structure to include the additional names.
> 
> I got a fair bit of resistance from them on the topic of multiple match 
> names for i2c clients.

Really? All I said is that you were a bit late in the game because this
had been discussed before. I know that David Brownell doesn't agree
with you (he designed what we have now), but me, I am still open to
discussing the matter, especially when more people complain about the
situation every month.

> >> We might as well just use i2c_new_device() instead of messing around
> >> with bus numbers.  Note that this is technically no longer platform
> >> code, so it's harder to justify claiming the static numberspace.
> > 
> > I was allowing control of the bus number with "cell-index" and
> > i2c_add_numbered_adapter().
> > Should I get rid of this and switch to i2c_add_adapter()?
> 
> Yes.

No! If you don't call i2c_add_numbered_adapter() then new-style i2c
clients will never work on your i2c adapter.

-- 
Jean Delvare

^ permalink raw reply

* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Jean Delvare @ 2007-11-06 17:32 UTC (permalink / raw)
  To: Matt Sealey; +Cc: Tjernlund, i2c, linuxppc-dev
In-Reply-To: <472F9086.2060606@genesi-usa.com>

On Mon, 05 Nov 2007 21:52:06 +0000, Matt Sealey wrote:
> Well, all i2c devices have a chip id you can probe for (...)

This statement is completely incorrect. I2C devices do NOT have
standard ID registers. Some devices have proprietary ID registers, some
don't, it's really up to the manfacturer.

-- 
Jean Delvare

^ permalink raw reply

* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Jon Smirl @ 2007-11-06 17:45 UTC (permalink / raw)
  To: Jean Delvare; +Cc: linuxppc-dev, Tjernlund, i2c
In-Reply-To: <20071106182953.3c1a57e3@hyperion.delvare>

On 11/6/07, Jean Delvare <khali@linux-fr.org> wrote:
> Hi Scott, Jon,
>
> On Mon, 05 Nov 2007 14:51:51 -0600, Scott Wood wrote:
> > Jon Smirl wrote:
> > > How about renaming the old driver file and leaving it hooked to ppc?
> > > Then it would get deleted when ppc goes away. That would let work
> > > progress on the powerpc version.
> >
> > Or we could have one driver that has two probe methods.  I don't like
> > forking the driver.
>
> I agree with Scott here, I don't want to fork the drivers. It is
> possible (and easy) to support both methods in the same module, let's
> just to that. See for example David Brownell's work on the lm75 driver:
> http://lists.lm-sensors.org/pipermail/lm-sensors/2007-September/021270.html

I agree that it is easy to make make a chip driver support both new
and old style.

But when I call i2c_new_device() on an old style chip driver it exits
saying that it doesn't work for the old style adapters. Checks for
is_newstyle_driver() are in the i2c_new_device code. That's what
caused me to rewrite the rtc-pcf8563 driver for the new style. This
probably related to probing, I have to pass the address in struct
i2c_board_info. The old style drivers don't support having their
address passed in.

This may be complicated by the fact that the rtc drivers I'm working
on are not probable. That's why I want to add device tree support for
them.

If this is going to work on an old style driver, how do I get the address to it?

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Scott Wood @ 2007-11-06 17:36 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Tjernlund, linuxppc-dev, i2c
In-Reply-To: <20071106182953.3c1a57e3@hyperion.delvare>

Jean Delvare wrote:
>>>> We might as well just use i2c_new_device() instead of messing around
>>>> with bus numbers.  Note that this is technically no longer platform
>>>> code, so it's harder to justify claiming the static numberspace.
>>> I was allowing control of the bus number with "cell-index" and
>>> i2c_add_numbered_adapter().
>>> Should I get rid of this and switch to i2c_add_adapter()?
>> Yes.
> 
> No! If you don't call i2c_add_numbered_adapter() then new-style i2c
> clients will never work on your i2c adapter.

I thought that was what i2c_new_device() was for?

By handling all the device tree stuff in the driver, it acts more like 
an add-on adapter than a platform device.

-Scott

^ permalink raw reply

* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Jean Delvare @ 2007-11-06 18:10 UTC (permalink / raw)
  To: Scott Wood; +Cc: Tjernlund, linuxppc-dev, i2c
In-Reply-To: <4730A617.9040502@freescale.com>

Hi Scott,

On Tue, 06 Nov 2007 11:36:23 -0600, Scott Wood wrote:
> Jean Delvare wrote:
> >>>> We might as well just use i2c_new_device() instead of messing around
> >>>> with bus numbers.  Note that this is technically no longer platform
> >>>> code, so it's harder to justify claiming the static numberspace.
> >>> I was allowing control of the bus number with "cell-index" and
> >>> i2c_add_numbered_adapter().
> >>> Should I get rid of this and switch to i2c_add_adapter()?
> >> Yes.
> > 
> > No! If you don't call i2c_add_numbered_adapter() then new-style i2c
> > clients will never work on your i2c adapter.
> 
> I thought that was what i2c_new_device() was for?

Sorry, I've not been completely clear. Yes, you can use
i2c_new_device() on an adapter that has been added with
i2c_add_adapter(). However, this requires that you have a reference to
that i2c_adapter, which is usually not the case with system-wide I2C
buses. Embedded platforms would rather use i2c_add_numbered_adapter(),
give a list of chips to i2c_register_board_info() and let i2c-core
instantiate them. i2c_new_device was primarily meant for multimedia
adapters.

> By handling all the device tree stuff in the driver, it acts more like 
> an add-on adapter than a platform device.

-- 
Jean Delvare

^ permalink raw reply

* [PATCH Rev 2] 8xxx: Convert #include of asm/of_{platform, device}.h into linux/of_{platform, device}.h.
From: Jon Loeliger @ 2007-11-06 18:11 UTC (permalink / raw)
  To: linuxppc-dev

From: Jon Loeliger <jdl@freescale.com>

Signed-off-by: Jon Loeliger <jdl@freescale.com>
Acked-by: Stephen Rothwell <sfr@canb.auug.org.au>
---

Chip away at some janitor work.
Catch both asm/of_platform.h and asm/of_device.h this time.
Add sfr's ACK.

 arch/powerpc/platforms/82xx/pq2fads.c     |    2 +-
 arch/powerpc/platforms/83xx/mpc832x_mds.c |    4 ++--
 arch/powerpc/platforms/83xx/mpc832x_rdb.c |    2 +-
 arch/powerpc/platforms/83xx/mpc836x_mds.c |    4 ++--
 arch/powerpc/platforms/85xx/mpc85xx_mds.c |    4 ++--
 5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/platforms/82xx/pq2fads.c b/arch/powerpc/platforms/82xx/pq2fads.c
index 4f457a9..1be5005 100644
--- a/arch/powerpc/platforms/82xx/pq2fads.c
+++ b/arch/powerpc/platforms/82xx/pq2fads.c
@@ -15,12 +15,12 @@
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/fsl_devices.h>
+#include <linux/of_platform.h>
 
 #include <asm/io.h>
 #include <asm/cpm2.h>
 #include <asm/udbg.h>
 #include <asm/machdep.h>
-#include <asm/of_platform.h>
 #include <asm/time.h>
 
 #include <sysdev/fsl_soc.h>
diff --git a/arch/powerpc/platforms/83xx/mpc832x_mds.c b/arch/powerpc/platforms/83xx/mpc832x_mds.c
index 972fa85..4ea1989 100644
--- a/arch/powerpc/platforms/83xx/mpc832x_mds.c
+++ b/arch/powerpc/platforms/83xx/mpc832x_mds.c
@@ -23,9 +23,9 @@
 #include <linux/seq_file.h>
 #include <linux/root_dev.h>
 #include <linux/initrd.h>
+#include <linux/of_platform.h>
+#include <linux/of_device.h>
 
-#include <asm/of_device.h>
-#include <asm/of_platform.h>
 #include <asm/system.h>
 #include <asm/atomic.h>
 #include <asm/time.h>
diff --git a/arch/powerpc/platforms/83xx/mpc832x_rdb.c b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
index fbca336..24e6ce6 100644
--- a/arch/powerpc/platforms/83xx/mpc832x_rdb.c
+++ b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
@@ -16,8 +16,8 @@
 
 #include <linux/pci.h>
 #include <linux/spi/spi.h>
+#include <linux/of_platform.h>
 
-#include <asm/of_platform.h>
 #include <asm/time.h>
 #include <asm/ipic.h>
 #include <asm/udbg.h>
diff --git a/arch/powerpc/platforms/83xx/mpc836x_mds.c b/arch/powerpc/platforms/83xx/mpc836x_mds.c
index 0f3855c..3781e3d 100644
--- a/arch/powerpc/platforms/83xx/mpc836x_mds.c
+++ b/arch/powerpc/platforms/83xx/mpc836x_mds.c
@@ -29,9 +29,9 @@
 #include <linux/seq_file.h>
 #include <linux/root_dev.h>
 #include <linux/initrd.h>
+#include <linux/of_platform.h>
+#include <linux/of_device.h>
 
-#include <asm/of_device.h>
-#include <asm/of_platform.h>
 #include <asm/system.h>
 #include <asm/atomic.h>
 #include <asm/time.h>
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
index 61b3eed..e6c63a5 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
@@ -30,9 +30,9 @@
 #include <linux/initrd.h>
 #include <linux/module.h>
 #include <linux/fsl_devices.h>
+#include <linux/of_platform.h>
+#include <linux/of_device.h>
 
-#include <asm/of_device.h>
-#include <asm/of_platform.h>
 #include <asm/system.h>
 #include <asm/atomic.h>
 #include <asm/time.h>
-- 
1.5.3.1.139.g9346b

^ permalink raw reply related

* [PATCH Rev 2] 4xx: Replace #includes of asm/of_platform.h with linux/of_platform.h.
From: Jon Loeliger @ 2007-11-06 18:13 UTC (permalink / raw)
  To: linuxppc-dev

From: Jon Loeliger <jdl@freescale.com>

Signed-off-by: Jon Loeliger <jdl@freescale.com>
Acked-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
Chip away at some janitor work.
Add sfr's ACK.

 arch/powerpc/platforms/40x/walnut.c  |    3 ++-
 arch/powerpc/platforms/44x/bamboo.c  |    3 ++-
 arch/powerpc/platforms/44x/ebony.c   |    3 ++-
 arch/powerpc/platforms/44x/sequoia.c |    3 ++-
 4 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/platforms/40x/walnut.c b/arch/powerpc/platforms/40x/walnut.c
index eb0c136..ff6db24 100644
--- a/arch/powerpc/platforms/40x/walnut.c
+++ b/arch/powerpc/platforms/40x/walnut.c
@@ -17,12 +17,13 @@
  */
 
 #include <linux/init.h>
+#include <linux/of_platform.h>
+
 #include <asm/machdep.h>
 #include <asm/prom.h>
 #include <asm/udbg.h>
 #include <asm/time.h>
 #include <asm/uic.h>
-#include <asm/of_platform.h>
 
 static struct of_device_id walnut_of_bus[] = {
 	{ .compatible = "ibm,plb3", },
diff --git a/arch/powerpc/platforms/44x/bamboo.c b/arch/powerpc/platforms/44x/bamboo.c
index 470e1a3..be23f11 100644
--- a/arch/powerpc/platforms/44x/bamboo.c
+++ b/arch/powerpc/platforms/44x/bamboo.c
@@ -14,12 +14,13 @@
  * option) any later version.
  */
 #include <linux/init.h>
+#include <linux/of_platform.h>
+
 #include <asm/machdep.h>
 #include <asm/prom.h>
 #include <asm/udbg.h>
 #include <asm/time.h>
 #include <asm/uic.h>
-#include <asm/of_platform.h>
 #include "44x.h"
 
 static struct of_device_id bamboo_of_bus[] = {
diff --git a/arch/powerpc/platforms/44x/ebony.c b/arch/powerpc/platforms/44x/ebony.c
index 40e18fc..6cd3476 100644
--- a/arch/powerpc/platforms/44x/ebony.c
+++ b/arch/powerpc/platforms/44x/ebony.c
@@ -17,12 +17,13 @@
  */
 
 #include <linux/init.h>
+#include <linux/of_platform.h>
+
 #include <asm/machdep.h>
 #include <asm/prom.h>
 #include <asm/udbg.h>
 #include <asm/time.h>
 #include <asm/uic.h>
-#include <asm/of_platform.h>
 
 #include "44x.h"
 
diff --git a/arch/powerpc/platforms/44x/sequoia.c b/arch/powerpc/platforms/44x/sequoia.c
index 30700b3..21a9dd1 100644
--- a/arch/powerpc/platforms/44x/sequoia.c
+++ b/arch/powerpc/platforms/44x/sequoia.c
@@ -14,12 +14,13 @@
  * option) any later version.
  */
 #include <linux/init.h>
+#include <linux/of_platform.h>
+
 #include <asm/machdep.h>
 #include <asm/prom.h>
 #include <asm/udbg.h>
 #include <asm/time.h>
 #include <asm/uic.h>
-#include <asm/of_platform.h>
 #include "44x.h"
 
 static struct of_device_id sequoia_of_bus[] = {
-- 
1.5.3.1.139.g9346b

^ permalink raw reply related

* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Jean Delvare @ 2007-11-06 18:17 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-dev, Tjernlund, i2c
In-Reply-To: <9e4733910711060945t35284f8atb36c0d6ed35abf71@mail.gmail.com>

Hi Jon,

On Tue, 6 Nov 2007 12:45:24 -0500, Jon Smirl wrote:
> On 11/6/07, Jean Delvare wrote:
> > I agree with Scott here, I don't want to fork the drivers. It is
> > possible (and easy) to support both methods in the same module, let's
> > just to that. See for example David Brownell's work on the lm75 driver:
> > http://lists.lm-sensors.org/pipermail/lm-sensors/2007-September/021270.html
> 
> I agree that it is easy to make make a chip driver support both new
> and old style.
> 
> But when I call i2c_new_device() on an old style chip driver it exits
> saying that it doesn't work for the old style adapters. Checks for
> is_newstyle_driver() are in the i2c_new_device code. That's what
> caused me to rewrite the rtc-pcf8563 driver for the new style. This
> probably related to probing, I have to pass the address in struct
> i2c_board_info. The old style drivers don't support having their
> address passed in.

I know that. The trick is to register two struct i2c_driver (again see
the lm75 example), one old-style, one new-style. I agree it's not very
elegant, but it works. Hopefully we can get rid of the old-style one
after some time, and it allows for a smooth transition.

> This may be complicated by the fact that the rtc drivers I'm working
> on are not probable. That's why I want to add device tree support for
> them.
> 
> If this is going to work on an old style driver, how do I get the
> address to it?

Old-style drivers probe for supported chips on all possible addresses
(for the chip in question). If the chip can't be probed, then module
parameters must be used. That's not terribly convenient, and new-style
drivers are much preferred in this case.

-- 
Jean Delvare

^ permalink raw reply

* Re: libfdt: Fix sw_tree1 testcase
From: Jon Loeliger @ 2007-11-06 18:21 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20071105234245.GA27067@localhost.localdomain>

So, like, the other day David Gibson mumbled:
> Crud, I screwed up and gave you an intermediate version of the patch
> which tried to do the same thing for rw_tree1.  For that to work, I'll
> need to write a dtbs_equal_notordered test.
> 
> Corrected version below.
> 
> libfdt: Fix sw_tree1 testcase
> 
> There is a bug in the sw_tree1 testcase / utility which puts two
> "compatible" properties in one node in the output tree.  This patch
> fixes the bug, and also adds a new test checking that the sw_tree1
> output is equal to test_tree1.dtb as its supposed to be, which should
> catch future errors of this type.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Applied.

Thanks,
jdl

^ permalink raw reply

* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Grant Likely @ 2007-11-06 18:26 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Tjernlund, i2c, linuxppc-dev
In-Reply-To: <20071106191039.37cd7053@hyperion.delvare>

On 11/6/07, Jean Delvare <khali@linux-fr.org> wrote:
> Hi Scott,
>
> On Tue, 06 Nov 2007 11:36:23 -0600, Scott Wood wrote:
> > Jean Delvare wrote:
> > >>>> We might as well just use i2c_new_device() instead of messing around
> > >>>> with bus numbers.  Note that this is technically no longer platform
> > >>>> code, so it's harder to justify claiming the static numberspace.
> > >>> I was allowing control of the bus number with "cell-index" and
> > >>> i2c_add_numbered_adapter().
> > >>> Should I get rid of this and switch to i2c_add_adapter()?
> > >> Yes.
> > >
> > > No! If you don't call i2c_add_numbered_adapter() then new-style i2c
> > > clients will never work on your i2c adapter.
> >
> > I thought that was what i2c_new_device() was for?
>
> Sorry, I've not been completely clear. Yes, you can use
> i2c_new_device() on an adapter that has been added with
> i2c_add_adapter(). However, this requires that you have a reference to
> that i2c_adapter, which is usually not the case with system-wide I2C
> buses. Embedded platforms would rather use i2c_add_numbered_adapter(),
> give a list of chips to i2c_register_board_info() and let i2c-core
> instantiate them. i2c_new_device was primarily meant for multimedia
> adapters.

*Some* embedded platforms would rather use i2c_add_numbered_adapter().  :-)

On powerpc, and other platforms which have a device tree, we don't
need to define a table of devices in the platform code because we've
already got a rich structure for describing such things.  The i2c
busses and i2c devices are grouped together in the device tree, so
when the i2c bus is probed, it should call out to common i2c device
tree parsing code to instantiate all the devices described in the
tree.

It would be awkward to describe the i2c bus in the device tree but
still have to use a static structure to describe the devices on that
bus.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

^ permalink raw reply

* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Grant Likely @ 2007-11-06 18:26 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Tjernlund, i2c, linuxppc-dev
In-Reply-To: <fa686aa40711061026i64f3022eh3bb820f645b9220e@mail.gmail.com>

On 11/6/07, Grant Likely <grant.likely@secretlab.ca> wrote:
> On 11/6/07, Jean Delvare <khali@linux-fr.org> wrote:
> > Hi Scott,
> >
> > On Tue, 06 Nov 2007 11:36:23 -0600, Scott Wood wrote:
> > > Jean Delvare wrote:
> > > >>>> We might as well just use i2c_new_device() instead of messing around
> > > >>>> with bus numbers.  Note that this is technically no longer platform
> > > >>>> code, so it's harder to justify claiming the static numberspace.
> > > >>> I was allowing control of the bus number with "cell-index" and
> > > >>> i2c_add_numbered_adapter().
> > > >>> Should I get rid of this and switch to i2c_add_adapter()?
> > > >> Yes.
> > > >
> > > > No! If you don't call i2c_add_numbered_adapter() then new-style i2c
> > > > clients will never work on your i2c adapter.
> > >
> > > I thought that was what i2c_new_device() was for?
> >
> > Sorry, I've not been completely clear. Yes, you can use
> > i2c_new_device() on an adapter that has been added with
> > i2c_add_adapter(). However, this requires that you have a reference to
> > that i2c_adapter, which is usually not the case with system-wide I2C
> > buses. Embedded platforms would rather use i2c_add_numbered_adapter(),
> > give a list of chips to i2c_register_board_info() and let i2c-core
> > instantiate them. i2c_new_device was primarily meant for multimedia
> > adapters.
>
> *Some* embedded platforms would rather use i2c_add_numbered_adapter().  :-)
>
> On powerpc, and other platforms which have a device tree, we don't

Specifically; an OF style device tree.  :-)

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

^ permalink raw reply

* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Scott Wood @ 2007-11-06 18:29 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Tjernlund, linuxppc-dev, i2c
In-Reply-To: <20071106191039.37cd7053@hyperion.delvare>

Jean Delvare wrote:
> Sorry, I've not been completely clear. Yes, you can use
> i2c_new_device() on an adapter that has been added with
> i2c_add_adapter(). However, this requires that you have a reference to
> that i2c_adapter, which is usually not the case with system-wide I2C
> buses.

But it is the case here, because the i2c driver knows about the device 
tree, and thus can pass the device tree node and the adapter struct to 
the enumeration function.

The driver should still do i2c_add_numbered_adapter() when using the 
non-OF platform device binding, in which case it gets the bus number 
from the platform data.

-Scott

^ permalink raw reply

* Re: [PATCH] using mii-bitbang on different processor ports - update the booting-without-of.txt-file
From: Scott Wood @ 2007-11-06 18:46 UTC (permalink / raw)
  To: Sergej Stepanov; +Cc: linuxppc-dev, jgarzik, netdev
In-Reply-To: <1194339117.3467.8.camel@p60635-ste.ids.de>

On Tue, Nov 06, 2007 at 09:51:57AM +0100, Sergej Stepanov wrote:
> The patch updates the booting-without-of.txt-file.
> There is a description for the case
> if mdio data and clock pins are on different processor ports.
> It is a extending for e-mail "[PATCH v3] using mii-bitbang on different processor ports".
> 
> Signed-off-by: Sergej Stepanov <Sergej.Stepanov@ids.de>
> --
> 
> diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
> index a96e853..497d8d8 100644
> --- a/Documentation/powerpc/booting-without-of.txt
> +++ b/Documentation/powerpc/booting-without-of.txt
> @@ -1956,6 +1956,12 @@ platforms are moved over to use the flattened-device-tree model.
>  		fsl,mdc-pin = <13>;
>  	};
>  
> +   The "reg"-property may have also depending on board design
> +   the following form:
> +	reg = <10d40 14 10d60 14>;
> +   In that case the pin for mdio data controlling is on the port C,
> +   and the pin for mdio clock controlling is on the port D.

It'd be better to explicitly say that the first resource is mdio, and the
second resource is mdc, rather than require the reader to know/look up which
corresponds to 10d40 and which to 10d60.

-Scott

^ permalink raw reply

* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Matt Sealey @ 2007-11-06 18:53 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Tjernlund, i2c, linuxppc-dev
In-Reply-To: <20071106183210.50058019@hyperion.delvare>

Jean Delvare wrote:
> On Mon, 05 Nov 2007 21:52:06 +0000, Matt Sealey wrote:
>> Well, all i2c devices have a chip id you can probe for (...)
> 
> This statement is completely incorrect. I2C devices do NOT have
> standard ID registers. Some devices have proprietary ID registers, some
> don't, it's really up to the manfacturer.

All I2C slave devices have to have a 7- or 10-bit address to identify them
by. They *may* not report what they ARE, but this is 9 times out of
10 a hardware design decision of soldering the chip to a board and
the address is then coded into device trees or hardcoded into drivers.

Whoever designed the board and has the datasheets knows the address
they're supposed to be at, and the device can accept this.

You simply cannot entertain an i2c bus with "anonymous and unnumbered
devices", every one has to have an address it responds to, however
it is defined, or it just does not work.

WRT cell-index this is an index of the bus on the chip (not the logical
i2c bus but the physical difference between two i2c controllers) and
then any i2c devices which need to be communicated with would be
child nodes, their reg property reflecting their slave address, is
that not correct?

-- 
Matt Sealey <matt@genesi-usa.com>
Genesi, Manager, Developer Relations

^ permalink raw reply

* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Jon Smirl @ 2007-11-06 19:02 UTC (permalink / raw)
  To: Stephen Rothwell; +Cc: Tjernlund, Jean Delvare, i2c, linuxppc-dev
In-Reply-To: <20071106154004.4f01a536.sfr@canb.auug.org.au>

Second pass at extending i2c core to accept strings of aliases for the
module. This version eliminate the need for separate name and type
fields when selecting a driver. PowerPC has to have a mapping from
device tree names to the i2c drivers, it makes sense to keep this
mapping inside the i2c driver.

Extend i2c-core to support lists of device tree compatible names when
matching drivers

From: Jon Smirl <jonsmirl@gmail.com>


---

 drivers/i2c/busses/i2c-mpc.c |   37 +++++++------------------------------
 drivers/i2c/i2c-core.c       |   35 ++++++++++++++++++-----------------
 drivers/rtc/rtc-pcf8563.c    |    1 +
 drivers/rtc/rtc-rs5c372.c    |    3 ++-
 include/linux/i2c.h          |   13 +++++++++----
 5 files changed, 37 insertions(+), 52 deletions(-)


diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index 4ddebe4..30420ad 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -312,34 +312,6 @@ static struct i2c_adapter mpc_ops =3D {
 =09.retries =3D 1
 };

-struct i2c_driver_device {
-=09char=09*of_device;
-=09char=09*i2c_driver;
-=09char=09*i2c_type;
-};
-
-static struct i2c_driver_device i2c_devices[] =3D {
-=09{"ricoh,rs5c372a", "rtc-rs5c372", "rs5c372a",},
-=09{"ricoh,rs5c372b", "rtc-rs5c372", "rs5c372b",},
-=09{"ricoh,rv5c386",  "rtc-rs5c372", "rv5c386",},
-=09{"ricoh,rv5c387a", "rtc-rs5c372", "rv5c387a",},
-=09{"epson,pcf8564", "rtc-pcf8563", "pcf8564",},
-};
-
-static int of_find_i2c_driver(struct device_node *node, struct
i2c_board_info *info)
-{
-=09int i;
-
-=09for (i =3D 0; i < ARRAY_SIZE(i2c_devices); i++) {
-=09=09if (!of_device_is_compatible(node, i2c_devices[i].of_device))
-=09=09=09continue;
-=09=09strncpy(info->driver_name, i2c_devices[i].i2c_driver, KOBJ_NAME_LEN)=
;
-=09=09strncpy(info->type, i2c_devices[i].i2c_type, I2C_NAME_SIZE);
-=09=09return 0;
-=09}
-=09return -ENODEV;
-}
-
 static void of_register_i2c_devices(struct i2c_adapter *adap, struct
device_node *adap_node)
 {
 =09struct device_node *node =3D NULL;
@@ -347,11 +319,12 @@ static void of_register_i2c_devices(struct
i2c_adapter *adap, struct device_node
 =09while ((node =3D of_get_next_child(adap_node, node))) {
 =09=09struct i2c_board_info info;
 =09=09const u32 *addr;
+=09=09const char *compatible;
 =09=09int len;

 =09=09addr =3D of_get_property(node, "reg", &len);
 =09=09if (!addr || len < sizeof(int) || *addr > (1 << 10) - 1) {
-=09=09=09printk(KERN_WARNING "i2c-mpc.c: invalid i2c device entry\n");
+=09=09=09printk(KERN_WARNING "i2c-mpc.c: invalid entry, missing reg attrib=
ute\n");
 =09=09=09continue;
 =09=09}

@@ -359,8 +332,12 @@ static void of_register_i2c_devices(struct
i2c_adapter *adap, struct device_node
 =09=09if (info.irq =3D=3D NO_IRQ)
 =09=09=09info.irq =3D -1;

-=09=09if (of_find_i2c_driver(node, &info) < 0)
+=09=09compatible =3D of_get_property(node, "compatible", &len);
+=09=09if (!compatible) {
+=09=09=09printk(KERN_WARNING "i2c-mpc.c: invalid entry, missing compatible
attribute\n");
 =09=09=09continue;
+=09=09}
+=09=09strncpy(info.name, compatible, sizeof(info.name));

 =09=09info.platform_data =3D NULL;
 =09=09info.addr =3D *addr;
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index d663e69..d9a70c2 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -17,7 +17,7 @@
     Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.=09=09     */
 /* -----------------------------------------------------------------------=
-- */

-/* With some changes from Ky=F6sti M=E4lkki <kmalkki@cc.hut.fi>.
+/* With some changes from Ky=C3=B6sti M=C3=80lkki <kmalkki@cc.hut.fi>.
    All SMBus-related things are written by Frodo Looijaard <frodol@dds.nl>
    SMBus 2.0 support by Mark Studebaker <mdsxyz123@yahoo.com> and
    Jean Delvare <khali@linux-fr.org> */
@@ -51,6 +51,7 @@ static int i2c_device_match(struct device *dev,
struct device_driver *drv)
 {
 =09struct i2c_client=09*client =3D to_i2c_client(dev);
 =09struct i2c_driver=09*driver =3D to_i2c_driver(drv);
+=09char const **alias;

 =09/* make legacy i2c drivers bypass driver model probing entirely;
 =09 * such drivers scan each i2c adapter/bus themselves.
@@ -60,8 +61,18 @@ static int i2c_device_match(struct device *dev,
struct device_driver *drv)

 =09/* new style drivers use the same kind of driver matching policy
 =09 * as platform devices or SPI:  compare device and driver IDs.
+=09 * Match against arrary of alias device tree names. When a match
+=09 * is found change the reference to point at the copy inside the
+=09 * chip driver allowing the caller's string to be freed.
 =09 */
-=09return strcmp(client->driver_name, drv->name) =3D=3D 0;
+=09alias =3D driver->aliases;
+=09while (*alias) {
+=09=09if (strcmp(client->name, *alias) =3D=3D 0) {
+=09=09=09return true;
+=09=09}
+=09=09alias++;
+=09}
+=09return 0;
 }

 #ifdef=09CONFIG_HOTPLUG
@@ -74,11 +85,11 @@ static int i2c_device_uevent(struct device *dev,
char **envp, int num_envp,
 =09int=09=09=09i =3D 0, length =3D 0;

 =09/* by definition, legacy drivers can't hotplug */
-=09if (dev->driver || !client->driver_name)
+=09if (dev->driver || !client->name)
 =09=09return 0;

 =09if (add_uevent_var(envp, num_envp, &i, buffer, buffer_size, &length,
-=09=09=09"MODALIAS=3D%s", client->driver_name))
+=09=09=09"MODALIAS=3D%s", client->name))
 =09=09return -ENOMEM;
 =09envp[i] =3D NULL;
 =09dev_dbg(dev, "uevent\n");
@@ -169,22 +180,15 @@ static void i2c_client_dev_release(struct device *dev=
)
 =09kfree(to_i2c_client(dev));
 }

-static ssize_t show_client_name(struct device *dev, struct
device_attribute *attr, char *buf)
-{
-=09struct i2c_client *client =3D to_i2c_client(dev);
-=09return sprintf(buf, "%s\n", client->name);
-}
-
 static ssize_t show_modalias(struct device *dev, struct
device_attribute *attr, char *buf)
 {
 =09struct i2c_client *client =3D to_i2c_client(dev);
-=09return client->driver_name
-=09=09? sprintf(buf, "%s\n", client->driver_name)
+=09return client->name
+=09=09? sprintf(buf, "%s\n", client->name)
 =09=09: 0;
 }

 static struct device_attribute i2c_dev_attrs[] =3D {
-=09__ATTR(name, S_IRUGO, show_client_name, NULL),
 =09/* modalias helps coldplug:  modprobe $(cat .../modalias) */
 =09__ATTR(modalias, S_IRUGO, show_modalias, NULL),
 =09{ },
@@ -233,10 +237,7 @@ i2c_new_device(struct i2c_adapter *adap, struct
i2c_board_info const *info)
 =09client->flags =3D info->flags;
 =09client->addr =3D info->addr;
 =09client->irq =3D info->irq;
-
-=09strlcpy(client->driver_name, info->driver_name,
-=09=09sizeof(client->driver_name));
-=09strlcpy(client->name, info->type, sizeof(client->name));
+=09strlcpy(client->name, info->name, sizeof(info->name));

 =09/* a new style driver may be bound to this device when we
 =09 * return from this function, or any later moment (e.g. maybe
diff --git a/drivers/rtc/rtc-pcf8563.c b/drivers/rtc/rtc-pcf8563.c
index b778d35..8162f77 100644
--- a/drivers/rtc/rtc-pcf8563.c
+++ b/drivers/rtc/rtc-pcf8563.c
@@ -266,6 +266,7 @@ static struct i2c_driver pcf8563_driver =3D {
 =09.driver=09=09=3D {
 =09=09.name=09=3D "rtc-pcf8563",
 =09},
+=09.aliases =3D (char const *[]){"philips,pcf8563", "epson,rtc8564", 0},
 =09.id=09=09=3D I2C_DRIVERID_PCF8563,
 =09.probe =3D &pcf8563_probe,
 =09.remove =3D &pcf8563_remove,
diff --git a/drivers/rtc/rtc-rs5c372.c b/drivers/rtc/rtc-rs5c372.c
index 6b67b50..b2da981 100644
--- a/drivers/rtc/rtc-rs5c372.c
+++ b/drivers/rtc/rtc-rs5c372.c
@@ -62,11 +62,11 @@


 enum rtc_type {
-=09rtc_undef =3D 0,
 =09rtc_rs5c372a,
 =09rtc_rs5c372b,
 =09rtc_rv5c386,
 =09rtc_rv5c387a,
+=09rtc_undef,
 };

 /* REVISIT:  this assumes that:
@@ -649,6 +649,7 @@ static struct i2c_driver rs5c372_driver =3D {
 =09.driver=09=09=3D {
 =09=09.name=09=3D "rtc-rs5c372",
 =09},
+=09.aliases=09=3D (char const
*[]){"ricoh,rs5c372a","ricoh,rs5c372b","ricoh,rv5c386","ricoh,rv5c387a",=09=
0},
 =09.probe=09=09=3D rs5c372_probe,
 =09.remove=09=09=3D rs5c372_remove,
 };
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 2a32f2f..4384cc1 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -107,6 +107,13 @@ extern s32 i2c_smbus_write_i2c_block_data(struct
i2c_client * client,
 struct i2c_driver {
 =09int id;
 =09unsigned int class;
+=09
+=09/* Alias names for the driver. Used to support device trees on
+=09 * the PowerPC architecture. Device tree names take the form of
+=09 * vendor,chip. For example "epson,rtc8564". Alias is a list of
+=09 * strings terminated by a zero entry.
+=09 */
+=09char const **aliases;

 =09/* Notifies the driver that a new bus has appeared. This routine
 =09 * can be used by the driver to test if the bus meets its conditions
@@ -146,7 +153,7 @@ struct i2c_driver {
 };
 #define to_i2c_driver(d) container_of(d, struct i2c_driver, driver)

-#define I2C_NAME_SIZE=0920
+#define I2C_NAME_SIZE=0940

 /**
  * struct i2c_client - represent an I2C slave device
@@ -181,7 +188,6 @@ struct i2c_client {
 =09=09=09=09=09/* to the client=09=09*/
 =09struct device dev;=09=09/* the device structure=09=09*/
 =09int irq;=09=09=09/* irq issued by device (or -1) */
-=09char driver_name[KOBJ_NAME_LEN];
 =09struct list_head list;
 =09struct completion released;
 };
@@ -225,8 +231,7 @@ static inline void i2c_set_clientdata (struct
i2c_client *dev, void *data)
  * with the adapter already known.
  */
 struct i2c_board_info {
-=09char=09=09driver_name[KOBJ_NAME_LEN];
-=09char=09=09type[I2C_NAME_SIZE];
+=09char =09=09name[I2C_NAME_SIZE];
 =09unsigned short=09flags;
 =09unsigned short=09addr;
 =09void=09=09*platform_data;

--=20
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply related

* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Jon Smirl @ 2007-11-06 19:07 UTC (permalink / raw)
  To: Jean Delvare; +Cc: linuxppc-dev, Tjernlund, i2c
In-Reply-To: <20071106191754.6ad2569c@hyperion.delvare>

Second pass on rework of mpc-i2c.c. This doesn't include code for
supporting both old and new style chip drivers. I'm still not a fan of
cluttering up mpc-i2c.c to support old style drivers since it so easy
to convert them to the new style. I'd rather just fix up the i2c
drivers used by chips on the PowerPC platform.

Convert i2c to of_platform_driver from platform_driver

From: Jon Smirl <jonsmirl@gmail.com>

Improve error returns
---

 arch/powerpc/sysdev/fsl_soc.c |  116 ---------------------------
 drivers/i2c/busses/i2c-mpc.c  |  178 +++++++++++++++++++++++++++++------------
 2 files changed, 126 insertions(+), 168 deletions(-)


diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index 1cf29c9..6f80216 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -306,122 +306,6 @@ err:

 arch_initcall(gfar_of_init);

-#ifdef CONFIG_I2C_BOARDINFO
-#include <linux/i2c.h>
-struct i2c_driver_device {
-	char	*of_device;
-	char	*i2c_driver;
-	char	*i2c_type;
-};
-
-static struct i2c_driver_device i2c_devices[] __initdata = {
-	{"ricoh,rs5c372a", "rtc-rs5c372", "rs5c372a",},
-	{"ricoh,rs5c372b", "rtc-rs5c372", "rs5c372b",},
-	{"ricoh,rv5c386",  "rtc-rs5c372", "rv5c386",},
-	{"ricoh,rv5c387a", "rtc-rs5c372", "rv5c387a",},
-};
-
-static int __init of_find_i2c_driver(struct device_node *node, struct
i2c_board_info *info)
-{
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(i2c_devices); i++) {
-		if (!of_device_is_compatible(node, i2c_devices[i].of_device))
-			continue;
-		strncpy(info->driver_name, i2c_devices[i].i2c_driver, KOBJ_NAME_LEN);
-		strncpy(info->type, i2c_devices[i].i2c_type, I2C_NAME_SIZE);
-		return 0;
-	}
-	return -ENODEV;
-}
-
-static void __init of_register_i2c_devices(struct device_node
*adap_node, int bus_num)
-{
-	struct device_node *node = NULL;
-
-	while ((node = of_get_next_child(adap_node, node))) {
-		struct i2c_board_info info;
-		const u32 *addr;
-		int len;
-
-		addr = of_get_property(node, "reg", &len);
-		if (!addr || len < sizeof(int) || *addr > (1 << 10) - 1) {
-			printk(KERN_WARNING "fsl_ioc.c: invalid i2c device entry\n");
-			continue;
-		}
-
-		info.irq = irq_of_parse_and_map(node, 0);
-		if (info.irq == NO_IRQ)
-			info.irq = -1;
-
-		if (of_find_i2c_driver(node, &info) < 0)
-			continue;
-
-		info.platform_data = NULL;
-		info.addr = *addr;
-
-		i2c_register_board_info(bus_num, &info, 1);
-	}
-}
-
-static int __init fsl_i2c_of_init(void)
-{
-	struct device_node *np;
-	unsigned int i;
-	struct platform_device *i2c_dev;
-	int ret;
-
-	for (np = NULL, i = 0;
-	     (np = of_find_compatible_node(np, "i2c", "fsl-i2c")) != NULL;
-	     i++) {
-		struct resource r[2];
-		struct fsl_i2c_platform_data i2c_data;
-		const unsigned char *flags = NULL;
-
-		memset(&r, 0, sizeof(r));
-		memset(&i2c_data, 0, sizeof(i2c_data));
-
-		ret = of_address_to_resource(np, 0, &r[0]);
-		if (ret)
-			goto err;
-
-		of_irq_to_resource(np, 0, &r[1]);
-
-		i2c_dev = platform_device_register_simple("fsl-i2c", i, r, 2);
-		if (IS_ERR(i2c_dev)) {
-			ret = PTR_ERR(i2c_dev);
-			goto err;
-		}
-
-		i2c_data.device_flags = 0;
-		flags = of_get_property(np, "dfsrr", NULL);
-		if (flags)
-			i2c_data.device_flags |= FSL_I2C_DEV_SEPARATE_DFSRR;
-
-		flags = of_get_property(np, "fsl5200-clocking", NULL);
-		if (flags)
-			i2c_data.device_flags |= FSL_I2C_DEV_CLOCK_5200;
-
-		ret =
-		    platform_device_add_data(i2c_dev, &i2c_data,
-					     sizeof(struct
-						    fsl_i2c_platform_data));
-		if (ret)
-			goto unreg;
-
-		of_register_i2c_devices(np, i);
-	}
-
-	return 0;
-
-unreg:
-	platform_device_unregister(i2c_dev);
-err:
-	return ret;
-}
-
-arch_initcall(fsl_i2c_of_init);
-#endif

 #ifdef CONFIG_PPC_83xx
 static int __init mpc83xx_wdt_init(void)
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index d8de4ac..4ddebe4 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -17,7 +17,7 @@
 #include <linux/module.h>
 #include <linux/sched.h>
 #include <linux/init.h>
-#include <linux/platform_device.h>
+#include <linux/of_platform.h>

 #include <asm/io.h>
 #include <linux/fsl_devices.h>
@@ -25,13 +25,13 @@
 #include <linux/interrupt.h>
 #include <linux/delay.h>

-#define MPC_I2C_ADDR  0x00
+#define DRV_NAME "mpc-i2c"
+
 #define MPC_I2C_FDR 	0x04
 #define MPC_I2C_CR	0x08
 #define MPC_I2C_SR	0x0c
 #define MPC_I2C_DR	0x10
 #define MPC_I2C_DFSRR 0x14
-#define MPC_I2C_REGION 0x20

 #define CCR_MEN  0x80
 #define CCR_MIEN 0x40
@@ -180,7 +180,7 @@ static void mpc_i2c_stop(struct mpc_i2c *i2c)
 static int mpc_write(struct mpc_i2c *i2c, int target,
 		     const u8 * data, int length, int restart)
 {
-	int i;
+	int i, result;
 	unsigned timeout = i2c->adap.timeout;
 	u32 flags = restart ? CCR_RSTA : 0;

@@ -192,15 +192,15 @@ static int mpc_write(struct mpc_i2c *i2c, int target,
 	/* Write target byte */
 	writeb((target << 1), i2c->base + MPC_I2C_DR);

-	if (i2c_wait(i2c, timeout, 1) < 0)
-		return -1;
+	if ((result = i2c_wait(i2c, timeout, 1)) < 0)
+		return result;

 	for (i = 0; i < length; i++) {
 		/* Write data byte */
 		writeb(data[i], i2c->base + MPC_I2C_DR);

-		if (i2c_wait(i2c, timeout, 1) < 0)
-			return -1;
+		if ((result = i2c_wait(i2c, timeout, 1)) < 0)
+			return result;
 	}

 	return 0;
@@ -210,7 +210,7 @@ static int mpc_read(struct mpc_i2c *i2c, int target,
 		    u8 * data, int length, int restart)
 {
 	unsigned timeout = i2c->adap.timeout;
-	int i;
+	int i, result;
 	u32 flags = restart ? CCR_RSTA : 0;

 	/* Start with MEN */
@@ -221,8 +221,8 @@ static int mpc_read(struct mpc_i2c *i2c, int target,
 	/* Write target address byte - this time with the read flag set */
 	writeb((target << 1) | 1, i2c->base + MPC_I2C_DR);

-	if (i2c_wait(i2c, timeout, 1) < 0)
-		return -1;
+	if ((result = i2c_wait(i2c, timeout, 1)) < 0)
+		return result;

 	if (length) {
 		if (length == 1)
@@ -234,8 +234,8 @@ static int mpc_read(struct mpc_i2c *i2c, int target,
 	}

 	for (i = 0; i < length; i++) {
-		if (i2c_wait(i2c, timeout, 0) < 0)
-			return -1;
+		if ((result = i2c_wait(i2c, timeout, 0)) < 0)
+			return result;

 		/* Generate txack on next to last byte */
 		if (i == length - 2)
@@ -312,74 +312,132 @@ static struct i2c_adapter mpc_ops = {
 	.retries = 1
 };

-static int fsl_i2c_probe(struct platform_device *pdev)
+struct i2c_driver_device {
+	char	*of_device;
+	char	*i2c_driver;
+	char	*i2c_type;
+};
+
+static struct i2c_driver_device i2c_devices[] = {
+	{"ricoh,rs5c372a", "rtc-rs5c372", "rs5c372a",},
+	{"ricoh,rs5c372b", "rtc-rs5c372", "rs5c372b",},
+	{"ricoh,rv5c386",  "rtc-rs5c372", "rv5c386",},
+	{"ricoh,rv5c387a", "rtc-rs5c372", "rv5c387a",},
+	{"epson,pcf8564", "rtc-pcf8563", "pcf8564",},
+};
+
+static int of_find_i2c_driver(struct device_node *node, struct
i2c_board_info *info)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(i2c_devices); i++) {
+		if (!of_device_is_compatible(node, i2c_devices[i].of_device))
+			continue;
+		strncpy(info->driver_name, i2c_devices[i].i2c_driver, KOBJ_NAME_LEN);
+		strncpy(info->type, i2c_devices[i].i2c_type, I2C_NAME_SIZE);
+		return 0;
+	}
+	return -ENODEV;
+}
+
+static void of_register_i2c_devices(struct i2c_adapter *adap, struct
device_node *adap_node)
+{
+	struct device_node *node = NULL;
+
+	while ((node = of_get_next_child(adap_node, node))) {
+		struct i2c_board_info info;
+		const u32 *addr;
+		int len;
+
+		addr = of_get_property(node, "reg", &len);
+		if (!addr || len < sizeof(int) || *addr > (1 << 10) - 1) {
+			printk(KERN_WARNING "i2c-mpc.c: invalid i2c device entry\n");
+			continue;
+		}
+
+		info.irq = irq_of_parse_and_map(node, 0);
+		if (info.irq == NO_IRQ)
+			info.irq = -1;
+
+		if (of_find_i2c_driver(node, &info) < 0)
+			continue;
+
+		info.platform_data = NULL;
+		info.addr = *addr;
+
+		i2c_new_device(adap, &info);
+	}
+}
+
+static int mpc_i2c_probe(struct of_device *op, const struct
of_device_id *match)
 {
 	int result = 0;
 	struct mpc_i2c *i2c;
-	struct fsl_i2c_platform_data *pdata;
-	struct resource *r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-
-	pdata = (struct fsl_i2c_platform_data *) pdev->dev.platform_data;

 	if (!(i2c = kzalloc(sizeof(*i2c), GFP_KERNEL))) {
 		return -ENOMEM;
 	}

-	i2c->irq = platform_get_irq(pdev, 0);
-	if (i2c->irq < 0) {
-		result = -ENXIO;
-		goto fail_get_irq;
-	}
-	i2c->flags = pdata->device_flags;
-	init_waitqueue_head(&i2c->queue);
+	if (of_get_property(op->node, "dfsrr", NULL))
+		i2c->flags |= FSL_I2C_DEV_SEPARATE_DFSRR;

-	i2c->base = ioremap((phys_addr_t)r->start, MPC_I2C_REGION);
+	if (of_device_is_compatible(op->node, "mpc5200-i2c"))
+		i2c->flags |= FSL_I2C_DEV_CLOCK_5200;
+
+	init_waitqueue_head(&i2c->queue);

+	i2c->base = of_iomap(op->node, 0);
 	if (!i2c->base) {
 		printk(KERN_ERR "i2c-mpc - failed to map controller\n");
 		result = -ENOMEM;
 		goto fail_map;
 	}

+	i2c->irq = irq_of_parse_and_map(op->node, 0);
+	if (i2c->irq < 0) {
+		result = -ENXIO;
+		goto fail_irq;
+	}
+	
 	if (i2c->irq != 0)
 		if ((result = request_irq(i2c->irq, mpc_i2c_isr,
-					  IRQF_SHARED, "i2c-mpc", i2c)) < 0) {
-			printk(KERN_ERR
-			       "i2c-mpc - failed to attach interrupt\n");
+					  	IRQF_SHARED, "i2c-mpc", i2c)) < 0) {
+			printk(KERN_ERR "i2c-mpc - failed to attach interrupt\n");
 			goto fail_irq;
 		}

 	mpc_i2c_setclock(i2c);
-	platform_set_drvdata(pdev, i2c);
+	
+	dev_set_drvdata(&op->dev, i2c);

 	i2c->adap = mpc_ops;
-	i2c->adap.nr = pdev->id;
 	i2c_set_adapdata(&i2c->adap, i2c);
-	i2c->adap.dev.parent = &pdev->dev;
-	if ((result = i2c_add_numbered_adapter(&i2c->adap)) < 0) {
+	i2c->adap.dev.parent = &op->dev;
+	if ((result = i2c_add_adapter(&i2c->adap)) < 0) {
 		printk(KERN_ERR "i2c-mpc - failed to add adapter\n");
 		goto fail_add;
 	}
+	
+	of_register_i2c_devices(&i2c->adap, op->node);

 	return result;

-      fail_add:
+fail_add:
 	if (i2c->irq != 0)
 		free_irq(i2c->irq, i2c);
-      fail_irq:
+fail_irq:
 	iounmap(i2c->base);
-      fail_map:
-      fail_get_irq:
+fail_map:
 	kfree(i2c);
 	return result;
 };

-static int fsl_i2c_remove(struct platform_device *pdev)
+static int mpc_i2c_remove(struct of_device *op)
 {
-	struct mpc_i2c *i2c = platform_get_drvdata(pdev);
+	struct mpc_i2c *i2c = dev_get_drvdata(&op->dev);

 	i2c_del_adapter(&i2c->adap);
-	platform_set_drvdata(pdev, NULL);
+	dev_set_drvdata(&op->dev, NULL);

 	if (i2c->irq != 0)
 		free_irq(i2c->irq, i2c);
@@ -389,28 +447,44 @@ static int fsl_i2c_remove(struct platform_device *pdev)
 	return 0;
 };

+static struct of_device_id mpc_i2c_of_match[] = {
+	{
+		.compatible	= "fsl-i2c",
+	},
+};
+MODULE_DEVICE_TABLE(of, mpc_i2c_of_match);
+
+
 /* Structure for a device driver */
-static struct platform_driver fsl_i2c_driver = {
-	.probe = fsl_i2c_probe,
-	.remove = fsl_i2c_remove,
-	.driver	= {
-		.owner = THIS_MODULE,
-		.name = "fsl-i2c",
+static struct of_platform_driver mpc_i2c_driver = {
+	.match_table	= mpc_i2c_of_match,
+	.probe		= mpc_i2c_probe,
+	.remove		= __devexit_p(mpc_i2c_remove),
+	.driver		= {
+		.owner	= THIS_MODULE,
+		.name	= DRV_NAME,
 	},
 };

-static int __init fsl_i2c_init(void)
+static int __init mpc_i2c_init(void)
 {
-	return platform_driver_register(&fsl_i2c_driver);
+	int rv;
+
+	rv = of_register_platform_driver(&mpc_i2c_driver);
+	if (rv) {
+		printk(KERN_ERR DRV_NAME " of_register_platform_driver failed (%i)\n", rv);
+		return rv;
+	}
+	return 0;
 }

-static void __exit fsl_i2c_exit(void)
+static void __exit mpc_i2c_exit(void)
 {
-	platform_driver_unregister(&fsl_i2c_driver);
+	of_unregister_platform_driver(&mpc_i2c_driver);
 }

-module_init(fsl_i2c_init);
-module_exit(fsl_i2c_exit);
+module_init(mpc_i2c_init);
+module_exit(mpc_i2c_exit);

 MODULE_AUTHOR("Adrian Cox <adrian@humboldt.co.uk>");
 MODULE_DESCRIPTION

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply related

* Re: [PATCH v2 08/12] [POWERPC] CM5200 DTS
From: Marian Balakowicz @ 2007-11-06 19:18 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20071105005007.GH19867@localhost.localdomain>

David Gibson wrote:
> On Sun, Nov 04, 2007 at 12:52:59AM +0100, Marian Balakowicz wrote:
>> Add device tree source file for CM5200 board.
> [snip]
>> +		rtc@800 {	// Real time clock
>> +			compatible = "mpc5200b-rtc","mpc5200-rtc";
>> +			device_type = "rtc";
> 
> No device_type here.
> 
> [snip]
>> +		spi@f00 {
>> +			device_type = "spi";
> 
> Definitely no device_type here.
> 
>> +			compatible = "mpc5200b-spi","mpc5200-spi";
>> +			reg = <f00 20>;
>> +			interrupts = <2 d 0 2 e 0>;
>> +			interrupt-parent = <&mpc5200_pic>;
>> +		};
> 
> [snip]
>> +		i2c@3d40 {
>> +			device_type = "i2c";
> 
> No device_type here.

OK, device_type removed for the above devices.

Thanks
m.

^ permalink raw reply

* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Jean Delvare @ 2007-11-06 19:34 UTC (permalink / raw)
  To: Grant Likely; +Cc: Tjernlund, i2c, linuxppc-dev
In-Reply-To: <fa686aa40711061026i64f3022eh3bb820f645b9220e@mail.gmail.com>

On Tue, 6 Nov 2007 11:26:14 -0700, Grant Likely wrote:
> On 11/6/07, Jean Delvare <khali@linux-fr.org> wrote:
> > Sorry, I've not been completely clear. Yes, you can use
> > i2c_new_device() on an adapter that has been added with
> > i2c_add_adapter(). However, this requires that you have a reference to
> > that i2c_adapter, which is usually not the case with system-wide I2C
> > buses. Embedded platforms would rather use i2c_add_numbered_adapter(),
> > give a list of chips to i2c_register_board_info() and let i2c-core
> > instantiate them. i2c_new_device was primarily meant for multimedia
> > adapters.
> 
> *Some* embedded platforms would rather use i2c_add_numbered_adapter().  :-)
> 
> On powerpc, and other platforms which have a device tree, we don't
> need to define a table of devices in the platform code because we've
> already got a rich structure for describing such things.  The i2c
> busses and i2c devices are grouped together in the device tree, so
> when the i2c bus is probed, it should call out to common i2c device
> tree parsing code to instantiate all the devices described in the
> tree.
> 
> It would be awkward to describe the i2c bus in the device tree but
> still have to use a static structure to describe the devices on that
> bus.

Ah, OK, thanks for the clarification. Then indeed using
i2c_add_adapter() will work fine, agreed.

-- 
Jean Delvare

^ permalink raw reply

* Re: [RFC/PATCH 0/2] powerpc: 64 bits irqtrace / lockdep support
From: Tim Pepper @ 2007-11-06 19:36 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, External List
In-Reply-To: <1192433296.924212.737979117370.qpush@grosgo>

On 10/14/07, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> This is 2 patches, one from Christoph Hellwig that adds the backtrace support,
> and one from Johannes Berg modified by me that adds the irq tracing support.
>
> This successfully boots a POWER5 pSeries machine. I'm going to run some more
> tests in the upcoming few days, so this is not an official submission (and it's
> too late for .24 merge window anyway) but it's looking good so far.

FYI: Boots and survives watching /proc/lock_stat during a kernel build
for me (on a 970 based blade).


Tim

^ permalink raw reply

* Re: [PATCH v2 06/12] [POWERPC] TQM5200 DTS
From: Marian Balakowicz @ 2007-11-06 19:50 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20071105004739.GG19867@localhost.localdomain>

David Gibson wrote:
> On Sun, Nov 04, 2007 at 12:52:47AM +0100, Marian Balakowicz wrote:
>> Add device tree source file for TQM5200 board.
> [snip]
>> +	soc5200@f0000000 {
> 
> soc@address is the new convention, I believe, but I guess you need
> compatibility with older bootstraps.

Correct, we stick with 'soc5200' due to hardcoded paths in U-boot.

> 
>> +		model = "fsl,mpc5200";
>> +		compatible = "mpc5200";
> 
> This compatible looks bogus; it should have the "fsl," at least.

Added 'fsl,'.

> [snip]
>> +		mpc5200_pic: pic@500 {
>> +			// 5200 interrupts are encoded into two levels;
>> +			interrupt-controller;
>> +			#interrupt-cells = <3>;
>> +			device_type = "interrupt-controller";
> 
> No device_type here.

Removed it.

> 
>> +			compatible = "mpc5200-pic";
>> +			reg = <500 80>;
>> +		};
>> +
>> +		gpt@600 {	// General Purpose Timer
>> +			compatible = "fsl,mpc5200-gpt";
>> +			cell-index = <0>;
> 
> Ok, is this actually a suitable usage for cell-index?  It should only
> be used when the cell-index number is used to program some soc-global
> register.  It should not be used just for ordering or logical-indexing
> purposes.

Rechecked that and it's not being used anywhere. Removed.

> [snip]
>> +		serial@2000 {		// PSC1
>> +			device_type = "serial";
>> +			compatible = "mpc5200-psc-uart";
>> +			port-number = <0>;  // Logical port assignment
>> +			cell-index = <0>;
> 
> Ditto w.r.t. cell-index.

Not used, removed.

> port-number also looks bogus - the device tree should not generally
> contain logical numbering information in this manner.  How and what
> uses the port-number property?

'port-number' is used by serial driver (mpc52xx_uart.c), if present it
 assigns a device number (/dev/PSC<port-number>) for given serial port.
That allows to override default auto-numbering.

>> +		sram@8000 {
>> +			compatible = "mpc5200-sram","sram";
> 
> Uh.. is there an "sram" binding?  "sram" doesn't look specific enough
> for a compatible property.

Right, removed "sram".

Thanks,
m.

^ permalink raw reply

* [PATCH] POWERPC: Use "is_power_of_2" macro for simplicity.
From: Robert P. J. Day @ 2007-11-06 17:11 UTC (permalink / raw)
  To: Linux PPC Mailing List


Signed-off-by: Robert P. J. Day <rpjday@crashcourse.ca>

---

diff --git a/arch/powerpc/lib/rheap.c b/arch/powerpc/lib/rheap.c
index 22c3b4f..53b4e91 100644
--- a/arch/powerpc/lib/rheap.c
+++ b/arch/powerpc/lib/rheap.c
@@ -19,6 +19,7 @@
 #include <linux/mm.h>
 #include <linux/err.h>
 #include <linux/slab.h>
+#include <linux/log2.h>

 #include <asm/rheap.h>

@@ -254,8 +255,7 @@ rh_info_t *rh_create(unsigned int alignment)
 {
 	rh_info_t *info;

-	/* Alignment must be a power of two */
-	if ((alignment & (alignment - 1)) != 0)
+	if (!is_power_of_2(alignment))
 		return ERR_PTR(-EINVAL);

 	info = kmalloc(sizeof(*info), GFP_KERNEL);
@@ -303,8 +303,7 @@ void rh_init(rh_info_t * info, unsigned int alignment, int max_blocks,
 	int i;
 	rh_block_t *blk;

-	/* Alignment must be a power of two */
-	if ((alignment & (alignment - 1)) != 0)
+	if (!is_power_of_2(alignment))
 		return;

 	info->alignment = alignment;
@@ -446,8 +445,7 @@ unsigned long rh_alloc_align(rh_info_t * info, int size, int alignment, const ch
 	rh_block_t *newblk;
 	unsigned long start, sp_size;

-	/* Validate size, and alignment must be power of two */
-	if (size <= 0 || (alignment & (alignment - 1)) != 0)
+	if (size <= 0 || !is_power_of_2(alignment))
 		return (unsigned long) -EINVAL;

 	/* Align to configured alignment */
-- 
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://crashcourse.ca
========================================================================

^ permalink raw reply related

* Re: [PATCH v2 11/12] [POWERPC] Promess Motion-PRO DTS
From: Marian Balakowicz @ 2007-11-06 20:02 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20071105005626.GI19867@localhost.localdomain>

David Gibson wrote:
> On Sun, Nov 04, 2007 at 12:53:17AM +0100, Marian Balakowicz wrote:
>> Add device tree source file for Motion-PRO board.
> [snip]
>> +		motionpro-statusled@660 {	// Motion-PRO status LED
>> +			compatible = "promess,motionpro-statusled";
>> +			cell-index = <6>;
>> +			reg = <660 10>;
>> +			interrupts = <1 f 0>;
>> +			interrupt-parent = <&mpc5200_pic>;
>> +			blink-delay = <64>; // 100 msec
>> +		};
>> +
>> +		motionpro-readyled@670 {	// Motion-PRO ready LED
>> +			compatible = "promess,motionpro-readyled";
>> +			cell-index = <7>;
> 
> These cell-index values for the LEDs look very strange.  How are they
> used?
[snip]
>> +
>> +		mscan@980 {
>> +			compatible = "mpc5200b-mscan","mpc5200-mscan";
>> +			cell-index = <1>;
> 
> As for serial and gpt, is cell-index really suitable here?

Ok, removed those unused "cell-index" properties.

> [snip]
>> +		spi@f00 {
>> +			device_type = "spi";
>> +			compatible = "mpc5200b-spi","mpc5200-spi";
>> +			reg = <f00 20>;
>> +			interrupts = <2 d 0 2 e 0>;
>> +			interrupt-parent = <&mpc5200_pic>;
>> +		};
> [snip]
>> +		// PSC2 in spi master mode 
>> +		spi@2200 {		// PSC2
>> +			device_type = "spi";
>> +			compatible = "mpc5200b-psc-spi","mpc5200-psc-spi";
>> +			cell-index = <1>;
> 
> cell-index present for one spi, but not the other makes be even more
> suspicious about it's applicability here...

These are two different SPIs, both are part of the SoC but first one
is dedicated SPI interface while the second one is PSC port in a SPI
mode. Driver for the second one (mpc52xx_psc_spi.c) is actually using
cell-index to get the PSC port number it's controlling.

> [snip]
>> +	lpb {
>> +		model = "fsl,lpb";
>> +		compatible = "fsl,lpb";
> 
> Is lpb another one of these chipselect/offset configurable bus bridge
> things?  If so, you should use a 2-cell addressing convention for the
> subnodes like fsl localbus and 4xx EBC.

Yes, it is, so I have switched to 2-cell addressing. Please see the v3
respin of the patches.


>> +		// 8-bit custom Anybus Module on LocalPlus Bus CS3
>> +		anybus50020000 {
> 
> Missing '@'.

Fixed.

Thanks,
m.

^ permalink raw reply

* [PATCH v3 00/13] [POWERPC] Add TQM5200/CM5200/Motion-PRO board support
From: Marian Balakowicz @ 2007-11-06 20:04 UTC (permalink / raw)
  To: linuxppc-dev

This is a third version of the patches that add arch/powerpc support for
three MPC5200 based boards: TQ-Components TQM5200, Schindler CM5200
and Promess Motion-PRO.

[POWERPC] Promess Motion-PRO defconfig
[POWERPC] Promess Motion-PRO DTS
[POWERPC] Motion-PRO: Add LED support.
[POWERPC] CM5200 defconfig
[POWERPC] CM5200 DTS
[POWERPC] TQM5200 defconfig
[POWERPC] TQM5200 DTS
[POWERPC] Use EXPORT_SYMBOL_GPL for 52xx common routines symbol export
[POWERPC] Export mpc52xx_map_node() routine symbol
[POWERPC] Add generic support for simple MPC5200 based boards
[POWERPC] Add common mpc52xx_setup_pci() routine
[POWERPC] Add 'fsl,lpb' bus type for MPC5200 LocalPlus Bus
[POWERPC] Add 'model: ...' line to common show_cpuinfo()

Please review them and schedule for 2.6.24-rc2 if you are ok with them.

Cheers,
m.

^ permalink raw reply

* [PATCH v3 01/13] [POWERPC] Add 'model: ...' line to common show_cpuinfo()
From: Marian Balakowicz @ 2007-11-06 20:04 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <20071106200446.10913.29338.stgit@hekate.izotz.org>

Print out 'model' property of '/' node as a machine name
in generic show_cpuinfo() routine.

Signed-off-by: Marian Balakowicz <m8@semihalf.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Acked-by: Olof Johansson <olof@lixom.net>
---

 arch/powerpc/kernel/setup-common.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)


diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
index 2de00f8..cb291f1 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -165,6 +165,8 @@ static int show_cpuinfo(struct seq_file *m, void *v)
 	unsigned short min;
 
 	if (cpu_id == NR_CPUS) {
+		struct device_node *root;
+		const char *model = NULL;
 #if defined(CONFIG_SMP) && defined(CONFIG_PPC32)
 		unsigned long bogosum = 0;
 		int i;
@@ -176,6 +178,13 @@ static int show_cpuinfo(struct seq_file *m, void *v)
 		seq_printf(m, "timebase\t: %lu\n", ppc_tb_freq);
 		if (ppc_md.name)
 			seq_printf(m, "platform\t: %s\n", ppc_md.name);
+		root = of_find_node_by_path("/");
+		if (root)
+			model = of_get_property(root, "model", NULL);
+		of_node_put(root);
+		if (model)
+			seq_printf(m, "model\t\t: %s\n", model);
+
 		if (ppc_md.show_cpuinfo != NULL)
 			ppc_md.show_cpuinfo(m);
 

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox