* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Matt Sealey @ 2007-11-05 21:52 UTC (permalink / raw)
To: Scott Wood; +Cc: Tjernlund, linuxppc-dev, Jean Delvare, i2c
In-Reply-To: <472F8267.8070106@freescale.com>
Scott Wood wrote:
> Jon Smirl wrote:
>>>> cell-index = <1>;
>>> What is cell-index for?
>> I was using it to control the bus number, is that the wrong attribute?
>
> It shouldn't be specified at all -- the hardware has no concept of a
> device number.
Well, all i2c devices have a chip id you can probe for, as for buses I
think cell-index is a holdover from the way the PSC code is organised
on the MPC5200 for example - if you have multiple buses which use the
same registers, for example. It's redundant on the PSC's for programming
because they all use different register offsets but if you move to
other devices like the GPTs, then it is then useful for debugging (it
is far more interesting to say GPT1 than GPT @ offset to match the)
and in general for tweaking OTHER parts of the chip (for instance
the CDM - very relevant!) which use single registers to control entire
swathes of units.
This way if you are in any doubt you can tell which one you should
be futzing with in other parts of the chip without complicated logic
based on MBAR offsets from the manual (magic numbers hinder the
maintainability and portability of the code). It's not relevant for
i2c but like I said, still valid, useful information..
--
Matt Sealey <matt@genesi-usa.com>
Genesi, Manager, Developer Relations
^ permalink raw reply
* [PATCH] 4xx; Replace #includes of asm/of_platform.h with linux/of_platform.h.
From: Jon Loeliger @ 2007-11-05 21:43 UTC (permalink / raw)
To: linuxppc-dev@ozlabs.org
From: Jon Loeliger <jdl@freescale.com>
Signed-off-by: Jon Loeliger <jdl@freescale.com>
---
Chip away at some janitor work.
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
^ permalink raw reply related
* Re: mmap question on ppc440
From: Josh Boyer @ 2007-11-05 21:16 UTC (permalink / raw)
To: Steven A. Falco; +Cc: linuxppc-dev
In-Reply-To: <472F8503.2080705@harris.com>
On Mon, 05 Nov 2007 16:02:59 -0500
"Steven A. Falco" <sfalco@harris.com> wrote:
> I am attempting to access the CPLD on the AMCC Sequoia board from
> user-land. I open /dev/mem, and mmap it, then try to access the
> resulting pointer. That works fine when accessing physical addresses
> that correspond to RAM, but as soon as I try to access the CPLD at
> physical address 0xc0000000, I get an infinite machine check.
That's because the CPLD is actually at physical address 0x1C0000000.
Yay for 36-bit physical addresses.
josh
^ permalink raw reply
* [PATCH] Replace some #includes of asm/of_platform.h with linux/of_platform.h.
From: Jon Loeliger @ 2007-11-05 21:05 UTC (permalink / raw)
To: linuxppc-dev@ozlabs.org
From: Jon Loeliger <jdl@freescale.com>
Signed-off-by: Jon Loeliger <jdl@freescale.com>
---
Chip away at some janitor work.
arch/powerpc/platforms/82xx/pq2fads.c | 2 +-
arch/powerpc/platforms/83xx/mpc832x_mds.c | 2 +-
arch/powerpc/platforms/83xx/mpc832x_rdb.c | 2 +-
arch/powerpc/platforms/83xx/mpc836x_mds.c | 2 +-
arch/powerpc/platforms/85xx/mpc85xx_mds.c | 2 +-
5 files changed, 5 insertions(+), 5 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..3ea59af 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 <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..ec1b03b 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 <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..1d81d22 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 <asm/of_device.h>
-#include <asm/of_platform.h>
#include <asm/system.h>
#include <asm/atomic.h>
#include <asm/time.h>
--
1.5.3
^ permalink raw reply related
* mmap question on ppc440
From: Steven A. Falco @ 2007-11-05 21:02 UTC (permalink / raw)
To: linuxppc-dev
I am attempting to access the CPLD on the AMCC Sequoia board from
user-land. I open /dev/mem, and mmap it, then try to access the
resulting pointer. That works fine when accessing physical addresses
that correspond to RAM, but as soon as I try to access the CPLD at
physical address 0xc0000000, I get an infinite machine check.
I've done this successfully on the ARM architecture, and I've found
examples where people do this on PPC, so I would expect this to work.
Here are a few successful reads:
bash-3.00# ./mm -r -a 0
paddr 00000000, map_base 0x30018000
00000000: c0348200
bash-3.00# ./mm -r -a 100
paddr 00000000, map_base 0x30018100
00000100: 7c0004ac
And here is the machine check:
bash-3.00# ./mm -r -a c0000000
paddr c0000000, Machine check in kernel mode.
Machine check in kernel mode.
Machine check in kernel mode.
Machine check in kernel mode.
My code looks like this (I'll post the whole program if anybody wants to
see it:
if((fd = open("/dev/mem", O_RDWR | O_SYNC)) == -1) {
fprintf(stderr, "cannot open\n");
exit(1);
}
offset = addr & MAP_MASK;
paddr = addr & ~MAP_MASK;
map_base = (unsigned long *)mmap(NULL, MAP_SIZE,
PROT_READ | PROT_WRITE, MAP_SHARED, fd, paddr);
Is it possible to access devices like this from user-land? If so, what
am I doing wrong?
Thanks,
Steve
^ permalink raw reply
* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Scott Wood @ 2007-11-05 20:51 UTC (permalink / raw)
To: Jon Smirl; +Cc: Tjernlund, linuxppc-dev, Jean Delvare, i2c
In-Reply-To: <9e4733910711051230w2d90a710idec3dcfc2e0f5c16@mail.gmail.com>
Jon Smirl wrote:
> On 11/5/07, Scott Wood <scottwood@freescale.com> wrote:
>> Jon Smirl wrote:
>>> This is my first pass at reworking the Freescale i2c driver. It
>>> switches the driver from being a platform driver to an open firmware
>>> one. I've checked it out on my hardware and it is working.
>> We may want to hold off on this until arch/ppc goes away (or at least
>> all users of this driver in arch/ppc).
>
> 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'm working on this because I'm adding codecs under powerpc that use
> i2c and I want consistent device tree use.
We already support i2c clients in the device tree, via the code in
fsl_soc.c.
>>> cell-index = <1>;
>> What is cell-index for?
>
> I was using it to control the bus number, is that the wrong attribute?
It shouldn't be specified at all -- the hardware has no concept of a
device number.
>>> rtc@32 {
>>> device_type = "rtc";
>> This isn't really necessary.
>
> Code doesn't access it. I copied it from a pre-existing device tree.
I'm just trying to keep the damage from spreading. :-)
>>> 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.
>>> Or push these strings into the chip drivers and modify
>>> i2c-core to also match on the device tree style names.
>> That would be ideal; it just hasn't been done yet.
>
> 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.
>> Most of this code should be made generic and placed in drivers/of.
>
> How so, it is specific to adding i2c drivers.
I meant generic with respect to the type of i2c controller, not generic
to all device types. :-)
It could be drivers/of/i2c.c, or drivers/i2c/of.c, etc.
>> 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.
>>> + i2c->base = ioremap((phys_addr_t)mem.start, MPC_I2C_REGION);
>>> if (!i2c->base) {
>>> printk(KERN_ERR "i2c-mpc - failed to map controller\n");
>> Use of_iomap().
>
> I didn't write this, how should it be done? MPC_I2C_REGION can be
> eliminated by using mem.start - mem.end.
i2c->base = of_iomap(op->node, 0);
of_address_to_resource() and ioremap() are combined into one.
>> Let's take this opportunity to stop matching on device_type here
>> (including updating booting-without-of.txt).
>
> Remove the .type line and leave .compatible?
Yes.
>>> +static struct of_platform_driver mpc_i2c_driver = {
>>> + .owner = THIS_MODULE,
>>> + .name = DRV_NAME,
>>> + .match_table = mpc_i2c_of_match,
>>> + .probe = mpc_i2c_probe,
>>> + .remove = __devexit_p(mpc_i2c_remove),
>>> + .driver = {
>>> + .name = DRV_NAME,
>>> },
>>> };
>> Do we still need .name if we have .driver.name?
>
> Yes, i2c-core is matching on mpc_i2c_driver.name, i2c-core would need
> to be changed to use mpc_i2c_driver.driver.name.
How can i2c-core be matching on a field in an OF-specific struct?
-Scott
^ permalink raw reply
* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Jon Smirl @ 2007-11-05 20:41 UTC (permalink / raw)
To: i2c, linuxppc-dev; +Cc: Tjernlund, Jean Delvare
In-Reply-To: <9e4733910711050714l2aa3a5eeqf5327c3e0d8ca490@mail.gmail.com>
On 11/5/07, Jon Smirl <jonsmirl@gmail.com> wrote:
> This is my first pass at reworking the Freescale i2c driver. It
> switches the driver from being a platform driver to an open firmware
> one. I've checked it out on my hardware and it is working.
Is there any way to have the i2c chip modules match on the device tree
and be automatically loaded? This is complicated by the fact that
these modules are used on other platforms.
If there is a way to do this for the i2c bus I can apply the same code
to the audio bus as well.
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Jon Smirl @ 2007-11-05 20:30 UTC (permalink / raw)
To: Scott Wood; +Cc: Tjernlund, linuxppc-dev, Jean Delvare, i2c
In-Reply-To: <472F7247.9070106@freescale.com>
On 11/5/07, Scott Wood <scottwood@freescale.com> wrote:
> Jon Smirl wrote:
> > This is my first pass at reworking the Freescale i2c driver. It
> > switches the driver from being a platform driver to an open firmware
> > one. I've checked it out on my hardware and it is working.
>
> We may want to hold off on this until arch/ppc goes away (or at least
> all users of this driver in arch/ppc).
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.
I'm working on this because I'm adding codecs under powerpc that use
i2c and I want consistent device tree use.
> > You can specific i2c devices on a bus by adding child nodes for them
> > in the device tree. It does not know how to autoload the i2c chip
> > driver modules.
> >
> > i2c@3d40 {
> > device_type = "i2c";
> > compatible = "mpc5200b-i2c\0mpc5200-i2c\0fsl-i2c";
>
> dtc supports the "mpc5200b-i2c", "mpc5200-i2c", "fsl-i2c" syntax.
>
> > cell-index = <1>;
>
> What is cell-index for?
I was using it to control the bus number, is that the wrong attribute?
> > fsl5200-clocking;
>
> As Matt pointed out, this is redundant.
>
> > rtc@32 {
> > device_type = "rtc";
>
> This isn't really necessary.
Code doesn't access it. I copied it from a pre-existing device tree.
> > 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.
>
> > The driver contains a table for mapping device tree style names to
> > linux kernel ones.
>
> Can we please put this in common code instead?
>
> > +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",},
> > +};
>
> At the very least, include the entries that are already being used with
> this driver in fsl_soc.c.
This is the same table from fsl_soc.c it has been moved. The data
really belongs in the i2c drivers if I can figure out how to get it
there.
> > I'd like to get rid of this table. There are two obvious solutions,
> > use names in the device tree that match up with the linux device
> > driver names.
>
> This was proposed and rejected a while ago. For one thing, some drivers
> handle multiple chips, and we shouldn't base device tree bindings on
> what groupings Linux happens to make in what driver supports what.
>
> > Or push these strings into the chip drivers and modify
> > i2c-core to also match on the device tree style names.
>
> That would be ideal; it just hasn't been done yet.
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.
> A middle ground for now would be to keep one table in drivers/of or
> something.
>
> > Without one of
> > these changes the table is going to need a mapping for every i2c
> > device on the market.
>
> Nah, only one for every i2c device Linux supports. :-)
>
> > 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",},
> > -};
>
> This is obviously not based on head-of-tree -- there are several more
> entries in this table.
It is based on 2.6.23. Head of tree hasn't been working very well for
me. I'll rebase it when I can get things working again.
> > diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
> > index d8de4ac..5ceb936 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 <asm/of_platform.h>
>
> Should be linux/of_platform.h
changed
>
> > @@ -180,7 +182,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 +194,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;
>
> This is a separate change from the OF-ization -- at least note it in the
> changelog.
done
>
> > + 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);
> > + printk("i2c driver is %s\n", info->driver_name);
>
> Should be KERN_DEBUG, if it stays at all.
deleted
>
> > +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);
> > + }
> > +}
>
> Most of this code should be made generic and placed in drivers/of.
How so, it is specific to adding i2c drivers.
>
> > + index = of_get_property(op->node, "cell-index", NULL);
> > + if (!index || *index > 5) {
> > + printk(KERN_ERR "mpc_i2c_probe: Device node %s has invalid "
> > + "cell-index property\n", op->node->full_name);
> > + return -EINVAL;
> > + }
> > +
>
> 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()?
>
> > + i2c->base = ioremap((phys_addr_t)mem.start, MPC_I2C_REGION);
> > if (!i2c->base) {
> > printk(KERN_ERR "i2c-mpc - failed to map controller\n");
>
> Use of_iomap().
I didn't write this, how should it be done? MPC_I2C_REGION can be
eliminated by using mem.start - mem.end.
>
> > if (i2c->irq != 0)
>
> if (i2c->irq != NO_IRQ)
>
> > +static struct of_device_id mpc_i2c_of_match[] = {
> > + {
> > + .type = "i2c",
> > + .compatible = "fsl-i2c",
> > + },
> > +};
> > +MODULE_DEVICE_TABLE(of, mpc_i2c_of_match);
>
> Let's take this opportunity to stop matching on device_type here
> (including updating booting-without-of.txt).
Remove the .type line and leave .compatible?
>
> > +static struct of_platform_driver mpc_i2c_driver = {
> > + .owner = THIS_MODULE,
> > + .name = DRV_NAME,
> > + .match_table = mpc_i2c_of_match,
> > + .probe = mpc_i2c_probe,
> > + .remove = __devexit_p(mpc_i2c_remove),
> > + .driver = {
> > + .name = DRV_NAME,
> > },
> > };
>
> Do we still need .name if we have .driver.name?
Yes, i2c-core is matching on mpc_i2c_driver.name, i2c-core would need
to be changed to use mpc_i2c_driver.driver.name.
>
> > diff --git a/drivers/rtc/rtc-pcf8563.c b/drivers/rtc/rtc-pcf8563.c
> > index 0242d80..b778d35 100644
> > --- a/drivers/rtc/rtc-pcf8563.c
> > +++ b/drivers/rtc/rtc-pcf8563.c
>
> This should be a separate patch.
It will be in final form.
>
> -Scott
>
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* [PATCH 07/12] arch/ppc: Remove duplicate includes.
From: Lucas Woods @ 2007-11-05 20:14 UTC (permalink / raw)
To: kernel-janitors; +Cc: linuxppc-dev, Andrew Morton
Signed-off-by: Lucas Woods <woodzy@gmail.com>
---
arch/ppc/kernel/setup.c | 1 -
arch/ppc/platforms/85xx/stx_gp3.c | 1 -
arch/ppc/syslib/gt64260_pic.c | 1 -
arch/ppc/syslib/mpc52xx_pic.c | 1 -
arch/ppc/syslib/mv64360_pic.c | 1 -
arch/ppc/syslib/ppc83xx_setup.c | 1 -
arch/ppc/xmon/start.c | 1 -
7 files changed, 0 insertions(+), 7 deletions(-)
diff --git a/arch/ppc/kernel/setup.c b/arch/ppc/kernel/setup.c
index aac88c2..79a03d0 100644
--- a/arch/ppc/kernel/setup.c
+++ b/arch/ppc/kernel/setup.c
@@ -37,7 +37,6 @@
#include <asm/nvram.h>
#include <asm/xmon.h>
#include <asm/ocp.h>
-#include <asm/prom.h>
#define USES_PPC_SYS (defined(CONFIG_85xx) || defined(CONFIG_83xx) || \
defined(CONFIG_MPC10X_BRIDGE) || defined(CONFIG_8260) || \
diff --git a/arch/ppc/platforms/85xx/stx_gp3.c b/arch/ppc/platforms/85xx/stx_gp3.c
index b1f5b73..731b40e 100644
--- a/arch/ppc/platforms/85xx/stx_gp3.c
+++ b/arch/ppc/platforms/85xx/stx_gp3.c
@@ -50,7 +50,6 @@
#include <asm/irq.h>
#include <asm/immap_85xx.h>
#include <asm/cpm2.h>
-#include <asm/mpc85xx.h>
#include <asm/ppc_sys.h>
#include <syslib/cpm2_pic.h>
diff --git a/arch/ppc/syslib/gt64260_pic.c b/arch/ppc/syslib/gt64260_pic.c
index e84d432..3b4fcca 100644
--- a/arch/ppc/syslib/gt64260_pic.c
+++ b/arch/ppc/syslib/gt64260_pic.c
@@ -35,7 +35,6 @@
#include <linux/interrupt.h>
#include <linux/sched.h>
#include <linux/signal.h>
-#include <linux/stddef.h>
#include <linux/delay.h>
#include <linux/irq.h>
diff --git a/arch/ppc/syslib/mpc52xx_pic.c b/arch/ppc/syslib/mpc52xx_pic.c
index af35a31..f58149c 100644
--- a/arch/ppc/syslib/mpc52xx_pic.c
+++ b/arch/ppc/syslib/mpc52xx_pic.c
@@ -20,7 +20,6 @@
#include <linux/init.h>
#include <linux/sched.h>
#include <linux/signal.h>
-#include <linux/stddef.h>
#include <linux/delay.h>
#include <linux/irq.h>
diff --git a/arch/ppc/syslib/mv64360_pic.c b/arch/ppc/syslib/mv64360_pic.c
index 4b7a333..2dd2dc5 100644
--- a/arch/ppc/syslib/mv64360_pic.c
+++ b/arch/ppc/syslib/mv64360_pic.c
@@ -36,7 +36,6 @@
#include <linux/init.h>
#include <linux/sched.h>
#include <linux/signal.h>
-#include <linux/stddef.h>
#include <linux/delay.h>
#include <linux/irq.h>
#include <linux/interrupt.h>
diff --git a/arch/ppc/syslib/ppc83xx_setup.c b/arch/ppc/syslib/ppc83xx_setup.c
index ec466db..ea37291 100644
--- a/arch/ppc/syslib/ppc83xx_setup.c
+++ b/arch/ppc/syslib/ppc83xx_setup.c
@@ -41,7 +41,6 @@
#include <syslib/ppc83xx_setup.h>
#if defined(CONFIG_PCI)
-#include <asm/delay.h>
#include <syslib/ppc83xx_pci.h>
#endif
diff --git a/arch/ppc/xmon/start.c b/arch/ppc/xmon/start.c
index 8f0b953..9056fe5 100644
--- a/arch/ppc/xmon/start.c
+++ b/arch/ppc/xmon/start.c
@@ -10,7 +10,6 @@
#include <linux/sysrq.h>
#include <linux/bitops.h>
#include <asm/xmon.h>
-#include <asm/machdep.h>
#include <asm/errno.h>
#include <asm/processor.h>
#include <asm/delay.h>
--
^ permalink raw reply related
* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Scott Wood @ 2007-11-05 20:06 UTC (permalink / raw)
To: Jon Smirl; +Cc: Tjernlund, Jean Delvare, i2c, linuxppc-dev
In-Reply-To: <9e4733910711051204g1efd2911sffbeef956b9e9f01@mail.gmail.com>
Jon Smirl wrote:
> On 11/5/07, Scott Wood <scottwood@freescale.com> wrote:
>> Jon Smirl wrote:
>>> I can change it to remove fsl5200-clocking if someone can tell me if
>>> this is only needed on the mpc5200b.
>> Well, it appears to be needed on the mpc5200 (no 'b')...
>
> What is the recommend way to check for a mpc5200 or mpc5200b?
Just check for mpc5200-i2c -- it's listed in the mpc5200b dts as well.
-Scott
^ permalink raw reply
* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Grant Likely @ 2007-11-05 20:11 UTC (permalink / raw)
To: Matt Sealey; +Cc: Tjernlund, linuxppc-dev, Jean Delvare, i2c
In-Reply-To: <472F6D5D.3030804@genesi-usa.com>
On 11/5/07, Matt Sealey <matt@genesi-usa.com> wrote:
> Jon Smirl wrote:
>
> > i2c@3d40 {
> > device_type = "i2c";
> > compatible = "mpc5200b-i2c\0mpc5200-i2c\0fsl-i2c";
> > cell-index = <1>;
> > reg = <3d40 40>;
> > interrupts = <2 10 0>;
> > interrupt-parent = <&mpc5200_pic>;
> > fsl5200-clocking;
> >
> > rtc@32 {
> > device_type = "rtc";
> > compatible = "epson,pcf8564";
> > reg = <51>;
> > };
> > };
>
> My only comment would be that the fsl5200-clocking property is
> totally redundant.
>
> Drivers can look at the compatible property (mpc5200b-i2c and
> mpc5200-i2c) to match up what special needs the driver may need.
> Even if it was just fsl-i2c, it could/should be implicit that
> this device is the onboard i2c and the parent node is ostensibly
> going to be marked as an MPC52xx SoC.. or it can look for the
> mpc5200-cdm node. There is no reason to invent a property just
> so you can do a property search when it replaces code of the
> same size to do a node or compatible search..
Yeah, I agree. Drop the fsl-clocking property. The hardware is
adequately described without it.
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: Jon Smirl @ 2007-11-05 20:04 UTC (permalink / raw)
To: Scott Wood; +Cc: Tjernlund, Jean Delvare, i2c, linuxppc-dev
In-Reply-To: <472F754C.1090202@freescale.com>
On 11/5/07, Scott Wood <scottwood@freescale.com> wrote:
> Jon Smirl wrote:
> > I can change it to remove fsl5200-clocking if someone can tell me if
> > this is only needed on the mpc5200b.
>
> Well, it appears to be needed on the mpc5200 (no 'b')...
What is the recommend way to check for a mpc5200 or mpc5200b?
>
> > This driver also supports the
> > MPC107/Tsi107/MPC8240/MPC8245 and MPC85xx/MPC8641.
>
> You forgot mpc83xx. :-)
>
> > Do any of
> > these other chips need fsl5200-clocking?
>
> None of them have it set in arch/powerpc/boot/dts.
>
> -Scott
>
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Grant Likely @ 2007-11-05 20:03 UTC (permalink / raw)
To: Jon Smirl; +Cc: Tjernlund, linuxppc-dev, Jean Delvare, i2c
In-Reply-To: <9e4733910711050714l2aa3a5eeqf5327c3e0d8ca490@mail.gmail.com>
On 11/5/07, Jon Smirl <jonsmirl@gmail.com> wrote:
> This is my first pass at reworking the Freescale i2c driver. It
> switches the driver from being a platform driver to an open firmware
> one. I've checked it out on my hardware and it is working.
Isn't this device core also used in the fsl coldfire socs?
If so, it needs to have both platform bus and of_platform bus
bindings. This is easy to do as long as you keep device probing and
initialization in separate routines. See drivers/serial/uartlite.c
for an example.
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: Scott Wood @ 2007-11-05 19:55 UTC (permalink / raw)
To: Jon Smirl; +Cc: Tjernlund, Jean Delvare, i2c, linuxppc-dev
In-Reply-To: <9e4733910711051151p18311586h133ac70917714466@mail.gmail.com>
Jon Smirl wrote:
> I can change it to remove fsl5200-clocking if someone can tell me if
> this is only needed on the mpc5200b.
Well, it appears to be needed on the mpc5200 (no 'b')...
> This driver also supports the
> MPC107/Tsi107/MPC8240/MPC8245 and MPC85xx/MPC8641.
You forgot mpc83xx. :-)
> Do any of
> these other chips need fsl5200-clocking?
None of them have it set in arch/powerpc/boot/dts.
-Scott
^ permalink raw reply
* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Jon Smirl @ 2007-11-05 19:51 UTC (permalink / raw)
To: Matt Sealey; +Cc: Tjernlund, linuxppc-dev, Jean Delvare, i2c
In-Reply-To: <472F6D5D.3030804@genesi-usa.com>
On 11/5/07, Matt Sealey <matt@genesi-usa.com> wrote:
> Jon Smirl wrote:
>
> > i2c@3d40 {
> > device_type = "i2c";
> > compatible = "mpc5200b-i2c\0mpc5200-i2c\0fsl-i2c";
> > cell-index = <1>;
> > reg = <3d40 40>;
> > interrupts = <2 10 0>;
> > interrupt-parent = <&mpc5200_pic>;
> > fsl5200-clocking;
> >
> > rtc@32 {
> > device_type = "rtc";
> > compatible = "epson,pcf8564";
> > reg = <51>;
> > };
> > };
>
> My only comment would be that the fsl5200-clocking property is
> totally redundant.
>
> Drivers can look at the compatible property (mpc5200b-i2c and
> mpc5200-i2c) to match up what special needs the driver may need.
> Even if it was just fsl-i2c, it could/should be implicit that
> this device is the onboard i2c and the parent node is ostensibly
> going to be marked as an MPC52xx SoC.. or it can look for the
> mpc5200-cdm node. There is no reason to invent a property just
> so you can do a property search when it replaces code of the
> same size to do a node or compatible search..
fsl5200-clocking is used to set FSL_I2C_DEV_CLOCK_5200
} else if (i2c->flags & FSL_I2C_DEV_CLOCK_5200)
writeb(0x3f, i2c->base + MPC_I2C_FDR);
else
writel(0x1031, i2c->base + MPC_I2C_FDR);
I can change it to remove fsl5200-clocking if someone can tell me if
this is only needed on the mpc5200b. This driver also supports the
MPC107/Tsi107/MPC8240/MPC8245 and MPC85xx/MPC8641. Do any of
these other chips need fsl5200-clocking?
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Scott Wood @ 2007-11-05 19:43 UTC (permalink / raw)
To: Jon Smirl; +Cc: Tjernlund, linuxppc-dev, Jean Delvare, i2c
In-Reply-To: <9e4733910711050714l2aa3a5eeqf5327c3e0d8ca490@mail.gmail.com>
Jon Smirl wrote:
> This is my first pass at reworking the Freescale i2c driver. It
> switches the driver from being a platform driver to an open firmware
> one. I've checked it out on my hardware and it is working.
We may want to hold off on this until arch/ppc goes away (or at least
all users of this driver in arch/ppc).
> You can specific i2c devices on a bus by adding child nodes for them
> in the device tree. It does not know how to autoload the i2c chip
> driver modules.
>
> i2c@3d40 {
> device_type = "i2c";
> compatible = "mpc5200b-i2c\0mpc5200-i2c\0fsl-i2c";
dtc supports the "mpc5200b-i2c", "mpc5200-i2c", "fsl-i2c" syntax.
> cell-index = <1>;
What is cell-index for?
> fsl5200-clocking;
As Matt pointed out, this is redundant.
> rtc@32 {
> device_type = "rtc";
This isn't really necessary.
> One side effect is that legacy style i2c drivers won't work anymore.
If you mean legacy-style client drivers, why not?
> The driver contains a table for mapping device tree style names to
> linux kernel ones.
Can we please put this in common code instead?
> +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",},
> +};
At the very least, include the entries that are already being used with
this driver in fsl_soc.c.
> I'd like to get rid of this table. There are two obvious solutions,
> use names in the device tree that match up with the linux device
> driver names.
This was proposed and rejected a while ago. For one thing, some drivers
handle multiple chips, and we shouldn't base device tree bindings on
what groupings Linux happens to make in what driver supports what.
> Or push these strings into the chip drivers and modify
> i2c-core to also match on the device tree style names.
That would be ideal; it just hasn't been done yet.
A middle ground for now would be to keep one table in drivers/of or
something.
> Without one of
> these changes the table is going to need a mapping for every i2c
> device on the market.
Nah, only one for every i2c device Linux supports. :-)
> 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",},
> -};
This is obviously not based on head-of-tree -- there are several more
entries in this table.
> diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
> index d8de4ac..5ceb936 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 <asm/of_platform.h>
Should be linux/of_platform.h
> @@ -180,7 +182,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 +194,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;
This is a separate change from the OF-ization -- at least note it in the
changelog.
> + 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);
> + printk("i2c driver is %s\n", info->driver_name);
Should be KERN_DEBUG, if it stays at all.
> +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);
> + }
> +}
Most of this code should be made generic and placed in drivers/of.
> + index = of_get_property(op->node, "cell-index", NULL);
> + if (!index || *index > 5) {
> + printk(KERN_ERR "mpc_i2c_probe: Device node %s has invalid "
> + "cell-index property\n", op->node->full_name);
> + return -EINVAL;
> + }
> +
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.
> + i2c->base = ioremap((phys_addr_t)mem.start, MPC_I2C_REGION);
> if (!i2c->base) {
> printk(KERN_ERR "i2c-mpc - failed to map controller\n");
Use of_iomap().
> if (i2c->irq != 0)
if (i2c->irq != NO_IRQ)
> +static struct of_device_id mpc_i2c_of_match[] = {
> + {
> + .type = "i2c",
> + .compatible = "fsl-i2c",
> + },
> +};
> +MODULE_DEVICE_TABLE(of, mpc_i2c_of_match);
Let's take this opportunity to stop matching on device_type here
(including updating booting-without-of.txt).
> +static struct of_platform_driver mpc_i2c_driver = {
> + .owner = THIS_MODULE,
> + .name = DRV_NAME,
> + .match_table = mpc_i2c_of_match,
> + .probe = mpc_i2c_probe,
> + .remove = __devexit_p(mpc_i2c_remove),
> + .driver = {
> + .name = DRV_NAME,
> },
> };
Do we still need .name if we have .driver.name?
> diff --git a/drivers/rtc/rtc-pcf8563.c b/drivers/rtc/rtc-pcf8563.c
> index 0242d80..b778d35 100644
> --- a/drivers/rtc/rtc-pcf8563.c
> +++ b/drivers/rtc/rtc-pcf8563.c
This should be a separate patch.
-Scott
^ permalink raw reply
* Re: [PATCH 11/16] Use of_get_next_child() in eeh_restore_bars()
From: Linas Vepstas @ 2007-11-05 19:23 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Stephen Rothwell, David S.Miller, linuxppc-dev
In-Reply-To: <1193629573.7280.11.camel@concordia>
On Mon, Oct 29, 2007 at 02:46:13PM +1100, Michael Ellerman wrote:
>
> On Fri, 2007-10-26 at 17:29 +1000, Stephen Rothwell wrote:
> > On Fri, 26 Oct 2007 16:54:43 +1000 (EST) Michael Ellerman <michael@ellerman.id.au> wrote:
> > >
> > > - dn = pdn->node->child;
> > > - while (dn) {
> > > + for (dn = NULL; (dn = of_get_next_child(pdn->node, dn));)
> >
> > Just wondering if we need
> >
> > #define for_each_child_node(dn, parent) \
> > for (dn = of_get_next_child(parent, NULL); dn; \
> > dn = of_get_next_child(parent, dn))
Yes, I like this much better too, if for no other reason than
the for-loop tructure is more orthodox.
> Should we perhaps make it for_each_child_device_node() ?
foreach_of_device_node_child() or
of_foreach_device_node_child()
^ permalink raw reply
* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Matt Sealey @ 2007-11-05 19:22 UTC (permalink / raw)
To: Jon Smirl; +Cc: Tjernlund, linuxppc-dev, Jean Delvare, i2c
In-Reply-To: <9e4733910711050714l2aa3a5eeqf5327c3e0d8ca490@mail.gmail.com>
Jon Smirl wrote:
> i2c@3d40 {
> device_type = "i2c";
> compatible = "mpc5200b-i2c\0mpc5200-i2c\0fsl-i2c";
> cell-index = <1>;
> reg = <3d40 40>;
> interrupts = <2 10 0>;
> interrupt-parent = <&mpc5200_pic>;
> fsl5200-clocking;
>
> rtc@32 {
> device_type = "rtc";
> compatible = "epson,pcf8564";
> reg = <51>;
> };
> };
My only comment would be that the fsl5200-clocking property is
totally redundant.
Drivers can look at the compatible property (mpc5200b-i2c and
mpc5200-i2c) to match up what special needs the driver may need.
Even if it was just fsl-i2c, it could/should be implicit that
this device is the onboard i2c and the parent node is ostensibly
going to be marked as an MPC52xx SoC.. or it can look for the
mpc5200-cdm node. There is no reason to invent a property just
so you can do a property search when it replaces code of the
same size to do a node or compatible search..
--
Matt Sealey <matt@genesi-usa.com>
Genesi, Manager, Developer Relations
^ permalink raw reply
* Re: [PATCH] [POWERPC] pasemi: Broaden specific references to 1682M
From: Olof Johansson @ 2007-11-05 19:22 UTC (permalink / raw)
To: Doug Thompson; +Cc: linuxppc-dev, Doug Thompson
In-Reply-To: <283711.31809.qm@web50103.mail.re2.yahoo.com>
On Mon, Nov 05, 2007 at 10:57:24AM -0800, Doug Thompson wrote:
>
> I assume then that this patch will move upstream via the POWERPC path, is that right?
Yes, I'll take care of feeding it upstream through that path.
-Olof
^ permalink raw reply
* Re: [RFC] [POWERPC] Add support for PHY-less fs_enet operation
From: Scott Wood @ 2007-11-05 19:04 UTC (permalink / raw)
To: Jochen Friedrich; +Cc: Jeff Garzik, linuxppc-embedded@ozlabs.org
In-Reply-To: <472CCA98.4000808@scram.de>
Jochen Friedrich wrote:
> diff --git a/drivers/net/fs_enet/fs_enet-main.c
> b/drivers/net/fs_enet/fs_enet-main.c
> index f2a4d39..e142eff 100644
> --- a/drivers/net/fs_enet/fs_enet-main.c
> +++ b/drivers/net/fs_enet/fs_enet-main.c
> @@ -702,13 +702,16 @@ static void fs_timeout(struct net_device *dev)
> spin_lock_irqsave(&fep->lock, flags);
>
> if (dev->flags & IFF_UP) {
> - phy_stop(fep->phydev);
> + if (fep->phydev)
> + phy_stop(fep->phydev);
> (*fep->ops->stop)(dev);
> (*fep->ops->restart)(dev);
> - phy_start(fep->phydev);
> + if (fep->phydev)
> + phy_start(fep->phydev);
> }
It might be nice if phy_stop/phy_start/etc. were made to accept NULL
parameters as no-ops to make things easier on drivers that support
phyless operation...
-Scott
^ permalink raw reply
* Re: [PATCH] [POWERPC] pasemi: Broaden specific references to 1682M
From: Doug Thompson @ 2007-11-05 18:57 UTC (permalink / raw)
To: Olof Johansson, linuxppc-dev; +Cc: Doug Thompson
In-Reply-To: <20071105032802.GA16613@lixom.net>
I assume then that this patch will move upstream via the POWERPC path, is that right?
Signed-off-by: Doug Thompson <dougthompson@xmission.com>
--- Olof Johansson <olof@lixom.net> wrote:
> [POWERPC] pasemi: Broaden specific references to 1682M
>
> There will be more product numbers in the future than just PA6T-1682M,
> but they will share much of the features. Remove some of the explicit
> references and compatibility checks with 1682M, and replace most of them
> with the more generic term "PWRficient".
>
>
> Signed-off-by: Olof Johansson <olof@lixom.net>
>
> ---
>
> This one touches drivers/char/hw_random and drivers/edac, but I'd prefer
> to just merge it up through the powerpc merge path since the changes
> are trivial.
>
> (Michael, Doug, if you disagree let me know and I can submit separate
> patches. This is 2.6.25 material anyway).
>
>
> -Olof
>
>
> diff --git a/arch/powerpc/platforms/pasemi/Kconfig b/arch/powerpc/platforms/pasemi/Kconfig
> index 735e153..2f4dd6e 100644
> --- a/arch/powerpc/platforms/pasemi/Kconfig
> +++ b/arch/powerpc/platforms/pasemi/Kconfig
> @@ -17,7 +17,7 @@ config PPC_PASEMI_IOMMU
> bool "PA Semi IOMMU support"
> depends on PPC_PASEMI
> help
> - IOMMU support for PA6T-1682M
> + IOMMU support for PA Semi PWRficient
>
> config PPC_PASEMI_IOMMU_DMA_FORCE
> bool "Force DMA engine to use IOMMU"
> diff --git a/arch/powerpc/platforms/pasemi/cpufreq.c b/arch/powerpc/platforms/pasemi/cpufreq.c
> index 1cfb8b0..8caa166 100644
> --- a/arch/powerpc/platforms/pasemi/cpufreq.c
> +++ b/arch/powerpc/platforms/pasemi/cpufreq.c
> @@ -147,7 +147,10 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
> if (!cpu)
> goto out;
>
> - dn = of_find_compatible_node(NULL, "sdc", "1682m-sdc");
> + dn = of_find_compatible_node(NULL, NULL, "1682m-sdc");
> + if (!dn)
> + dn = of_find_compatible_node(NULL, NULL,
> + "pasemi,pwrficient-sdc");
> if (!dn)
> goto out;
> err = of_address_to_resource(dn, 0, &res);
> @@ -160,7 +163,10 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
> goto out;
> }
>
> - dn = of_find_compatible_node(NULL, "gizmo", "1682m-gizmo");
> + dn = of_find_compatible_node(NULL, NULL, "1682m-gizmo");
> + if (!dn)
> + dn = of_find_compatible_node(NULL, NULL,
> + "pasemi,pwrficient-gizmo");
> if (!dn) {
> err = -ENODEV;
> goto out_unmap_sdcasr;
> @@ -292,7 +298,8 @@ static struct cpufreq_driver pas_cpufreq_driver = {
>
> static int __init pas_cpufreq_init(void)
> {
> - if (!machine_is_compatible("PA6T-1682M"))
> + if (!machine_is_compatible("PA6T-1682M") &&
> + !machine_is_compatible("pasemi,pwrficient"))
> return -ENODEV;
>
> return cpufreq_register_driver(&pas_cpufreq_driver);
> diff --git a/arch/powerpc/platforms/pasemi/gpio_mdio.c
> b/arch/powerpc/platforms/pasemi/gpio_mdio.c
> index 95d0c78..b029804 100644
> --- a/arch/powerpc/platforms/pasemi/gpio_mdio.c
> +++ b/arch/powerpc/platforms/pasemi/gpio_mdio.c
> @@ -333,7 +333,10 @@ int gpio_mdio_init(void)
> {
> struct device_node *np;
>
> - np = of_find_compatible_node(NULL, "gpio", "1682m-gpio");
> + np = of_find_compatible_node(NULL, NULL, "1682m-gpio");
> + if (!np)
> + np = of_find_compatible_node(NULL, NULL,
> + "pasemi,pwrficient-gpio");
> if (!np)
> return -ENODEV;
> gpio_regs = of_iomap(np, 0);
> diff --git a/arch/powerpc/platforms/pasemi/setup.c b/arch/powerpc/platforms/pasemi/setup.c
> index 3a5d112..aeafe98 100644
> --- a/arch/powerpc/platforms/pasemi/setup.c
> +++ b/arch/powerpc/platforms/pasemi/setup.c
> @@ -362,8 +362,11 @@ static inline void pasemi_pcmcia_init(void)
>
>
> static struct of_device_id pasemi_bus_ids[] = {
> + /* Unfortunately needed for legacy firmwares */
> { .type = "localbus", },
> { .type = "sdc", },
> + { .compatible = "pasemi,localbus", },
> + { .compatible = "pasemi,sdc", },
> {},
> };
>
> @@ -389,7 +392,8 @@ static int __init pas_probe(void)
> {
> unsigned long root = of_get_flat_dt_root();
>
> - if (!of_flat_dt_is_compatible(root, "PA6T-1682M"))
> + if (!of_flat_dt_is_compatible(root, "PA6T-1682M") &&
> + !of_flat_dt_is_compatible(root, "pasemi,pwrficient"))
> return 0;
>
> hpte_init_native();
> @@ -400,7 +404,7 @@ static int __init pas_probe(void)
> }
>
> define_machine(pasemi) {
> - .name = "PA Semi PA6T-1682M",
> + .name = "PA Semi PWRficient",
> .probe = pas_probe,
> .setup_arch = pas_setup_arch,
> .init_early = pas_init_early,
> diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
> index 2d7cd48..6bbd4fa 100644
> --- a/drivers/char/hw_random/Kconfig
> +++ b/drivers/char/hw_random/Kconfig
> @@ -98,7 +98,7 @@ config HW_RANDOM_PASEMI
> default HW_RANDOM
> ---help---
> This driver provides kernel-side support for the Random Number
> - Generator hardware found on PA6T-1682M processor.
> + Generator hardware found on PA Semi PWRficient SoCs.
>
> To compile this driver as a module, choose M here: the
> module will be called pasemi-rng.
> diff --git a/drivers/char/hw_random/pasemi-rng.c b/drivers/char/hw_random/pasemi-rng.c
> index fa6040b..24ae307 100644
> --- a/drivers/char/hw_random/pasemi-rng.c
> +++ b/drivers/char/hw_random/pasemi-rng.c
> @@ -126,10 +126,9 @@ static int __devexit rng_remove(struct of_device *dev)
> }
>
> static struct of_device_id rng_match[] = {
> - {
> - .compatible = "1682m-rng",
> - },
> - {},
> + { .compatible = "1682m-rng", },
> + { .compatible = "pasemi,pwrficient-rng", },
> + { },
> };
>
> static struct of_platform_driver rng_driver = {
> diff --git a/drivers/edac/pasemi_edac.c b/drivers/edac/pasemi_edac.c
> index 9007d06..9032091 100644
> --- a/drivers/edac/pasemi_edac.c
> +++ b/drivers/edac/pasemi_edac.c
> @@ -225,7 +225,7 @@ static int __devinit pasemi_edac_probe(struct pci_dev *pdev,
> EDAC_FLAG_NONE;
> mci->mod_name = MODULE_NAME;
> mci->dev_name = pci_name(pdev);
> - mci->ctl_name = "pasemi,1682m-mc";
> + mci->ctl_name = "pasemi,pwrficient-mc";
> mci->edac_check = pasemi_edac_check;
> mci->ctl_page_to_phys = NULL;
> pci_read_config_dword(pdev, MCCFG_SCRUB, &scrub);
> @@ -297,4 +297,4 @@ module_exit(pasemi_edac_exit);
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Egor Martovetsky <egor@pasemi.com>");
> -MODULE_DESCRIPTION("MC support for PA Semi PA6T-1682M memory controller");
> +MODULE_DESCRIPTION("MC support for PA Semi PWRficient memory controller");
>
W1DUG
^ permalink raw reply
* [PATCH 5/5] powerpc: handle mpc8360 rev. 2.1 RGMII timing erratum
From: Kim Phillips @ 2007-11-05 18:15 UTC (permalink / raw)
To: Li Yang, Kumar Gala, netdev, linuxppc-dev; +Cc: paulus, jgarzik
if on a rev. 2.1, adjust UCC clock and data timing characteristics
as specified in the rev.2.1 erratum #2.
Signed-off-by: Kim Phillips <kim.phillips@freescale.com>
---
arch/powerpc/platforms/83xx/mpc836x_mds.c | 31 ++++++++++++++++++++++++++--
1 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/83xx/mpc836x_mds.c b/arch/powerpc/platforms/83xx/mpc836x_mds.c
index 0f3855c..0a72260 100644
--- a/arch/powerpc/platforms/83xx/mpc836x_mds.c
+++ b/arch/powerpc/platforms/83xx/mpc836x_mds.c
@@ -96,14 +96,39 @@ static void __init mpc836x_mds_setup_arch(void)
if ((np = of_find_compatible_node(NULL, "network", "ucc_geth"))
!= NULL){
+ uint svid;
+
/* Reset the Ethernet PHY */
- bcsr_regs[9] &= ~0x20;
+#define BCSR9_GETHRST 0x20
+ clrbits8(&bcsr_regs[9], BCSR9_GETHRST);
udelay(1000);
- bcsr_regs[9] |= 0x20;
+ setbits8(&bcsr_regs[9], BCSR9_GETHRST);
+
+ /* handle mpc8360ea rev.2.1 erratum 2: RGMII Timing */
+ svid = mfspr(SPRN_SVR);
+ if (svid == 0x80480021) {
+ void __iomem *immap;
+
+ immap = ioremap(get_immrbase() + 0x14a8, 8);
+
+ /*
+ * IMMR + 0x14A8[4:5] = 11 (clk delay for UCC 2)
+ * IMMR + 0x14A8[18:19] = 11 (clk delay for UCC 1)
+ */
+ setbits32(immap, 0x0c003000);
+
+ /*
+ * IMMR + 0x14AC[20:27] = 10101010
+ * (data delay for both UCC's)
+ */
+ clrsetbits_be32(immap + 4, 0xff0, 0xaa0);
+
+ iounmap(immap);
+ }
+
iounmap(bcsr_regs);
of_node_put(np);
}
-
#endif /* CONFIG_QUICC_ENGINE */
}
--
1.5.2.2
^ permalink raw reply related
* [PATCH 4/5] ucc_geth: handle passing of RX-only and TX-only internal delay PHY connection type parameters
From: Kim Phillips @ 2007-11-05 18:15 UTC (permalink / raw)
To: Li Yang, Kumar Gala, netdev, linuxppc-dev; +Cc: paulus, jgarzik
Extend the RGMII-Internal Delay specification case to include
TX-only and RX-only variants.
Signed-off-by: Kim Phillips <kim.phillips@freescale.com>
---
drivers/net/ucc_geth.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index a3ff270..7f68990 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -1460,6 +1460,8 @@ static int adjust_enet_interface(struct ucc_geth_private *ugeth)
if ((ugeth->phy_interface == PHY_INTERFACE_MODE_RMII) ||
(ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII) ||
(ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII_ID) ||
+ (ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII_RXID) ||
+ (ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID) ||
(ugeth->phy_interface == PHY_INTERFACE_MODE_RTBI)) {
upsmr |= UPSMR_RPM;
switch (ugeth->max_speed) {
@@ -1557,6 +1559,8 @@ static void adjust_link(struct net_device *dev)
if ((ugeth->phy_interface == PHY_INTERFACE_MODE_RMII) ||
(ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII) ||
(ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII_ID) ||
+ (ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII_RXID) ||
+ (ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID) ||
(ugeth->phy_interface == PHY_INTERFACE_MODE_RTBI)) {
if (phydev->speed == SPEED_10)
upsmr |= UPSMR_R10M;
@@ -3795,6 +3799,10 @@ static phy_interface_t to_phy_interface(const char *phy_connection_type)
return PHY_INTERFACE_MODE_RGMII;
if (strcasecmp(phy_connection_type, "rgmii-id") == 0)
return PHY_INTERFACE_MODE_RGMII_ID;
+ if (strcasecmp(phy_connection_type, "rgmii-txid") == 0)
+ return PHY_INTERFACE_MODE_RGMII_TXID;
+ if (strcasecmp(phy_connection_type, "rgmii-rxid") == 0)
+ return PHY_INTERFACE_MODE_RGMII_RXID;
if (strcasecmp(phy_connection_type, "rtbi") == 0)
return PHY_INTERFACE_MODE_RTBI;
@@ -3889,6 +3897,8 @@ static int ucc_geth_probe(struct of_device* ofdev, const struct of_device_id *ma
case PHY_INTERFACE_MODE_GMII:
case PHY_INTERFACE_MODE_RGMII:
case PHY_INTERFACE_MODE_RGMII_ID:
+ case PHY_INTERFACE_MODE_RGMII_RXID:
+ case PHY_INTERFACE_MODE_RGMII_TXID:
case PHY_INTERFACE_MODE_TBI:
case PHY_INTERFACE_MODE_RTBI:
max_speed = SPEED_1000;
--
1.5.2.2
^ permalink raw reply related
* [PATCH 3/5] phylib: marvell: add support for TX-only and RX-only Internal Delay
From: Kim Phillips @ 2007-11-05 18:15 UTC (permalink / raw)
To: Li Yang, Kumar Gala, netdev, linuxppc-dev; +Cc: paulus, jgarzik
Previously, Internal Delay specification implied the delay be
applied to both TX and RX. This patch allows for separate TX/RX-only
internal delay specification.
Signed-off-by: Kim Phillips <kim.phillips@freescale.com>
---
drivers/net/phy/marvell.c | 26 +++++++++++++++++---------
1 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index d2ede5f..55eb5c1 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -143,21 +143,29 @@ static int m88e1111_config_init(struct phy_device *phydev)
int err;
if ((phydev->interface == PHY_INTERFACE_MODE_RGMII) ||
- (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)) {
+ (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) ||
+ (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) ||
+ (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)) {
int temp;
- if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
- temp = phy_read(phydev, MII_M1111_PHY_EXT_CR);
- if (temp < 0)
- return temp;
+ temp = phy_read(phydev, MII_M1111_PHY_EXT_CR);
+ if (temp < 0)
+ return temp;
+ if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
temp |= (MII_M1111_RX_DELAY | MII_M1111_TX_DELAY);
-
- err = phy_write(phydev, MII_M1111_PHY_EXT_CR, temp);
- if (err < 0)
- return err;
+ } else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
+ temp &= ~MII_M1111_TX_DELAY;
+ temp |= MII_M1111_RX_DELAY;
+ } else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
+ temp &= ~MII_M1111_RX_DELAY;
+ temp |= MII_M1111_TX_DELAY;
}
+ err = phy_write(phydev, MII_M1111_PHY_EXT_CR, temp);
+ if (err < 0)
+ return err;
+
temp = phy_read(phydev, MII_M1111_PHY_EXT_SR);
if (temp < 0)
return temp;
--
1.5.2.2
^ permalink raw reply related
* [PATCH 2/5] phylib: add PHY interface modes for internal delay for tx and rx only
From: Kim Phillips @ 2007-11-05 18:15 UTC (permalink / raw)
To: Li Yang, Kumar Gala, netdev, linuxppc-dev; +Cc: paulus, jgarzik
Allow phylib specification of cases where hardware needs to configure
PHYs for Internal Delay only on either RX or TX (not both).
Signed-off-by: Kim Phillips <kim.phillips@freescale.com>
---
include/linux/phy.h | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/include/linux/phy.h b/include/linux/phy.h
index f0742b6..e10763d 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -58,6 +58,8 @@ typedef enum {
PHY_INTERFACE_MODE_RMII,
PHY_INTERFACE_MODE_RGMII,
PHY_INTERFACE_MODE_RGMII_ID,
+ PHY_INTERFACE_MODE_RGMII_RXID,
+ PHY_INTERFACE_MODE_RGMII_TXID,
PHY_INTERFACE_MODE_RTBI
} phy_interface_t;
--
1.5.2.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox