* No ip addr on the Xilinx ML403
From: usysinc @ 2007-11-06 3:38 UTC (permalink / raw)
To: linuxppc-embedded
I have successfully booted the Linux 2.6.23 vertex kernel branch on the ML403
board. The hard_temac 3.00_a was implemented in the MHS file. The kernel was
configured with no Marvel PHY drivers; just the generic 10/100 Ethernet MII,
and 10/100/1000 Ethernet. The Temac configures correctly and I have verified
that DHCP is indeed functional, but no 'ip addr=' is visible with ifconfig
and can't ping anything other than localhost. Can anyone suggest a solution
to this configuration problem?
XTemac: using sgDMA mode.
[ 3.079060] XTemac: using TxDRE mode
[ 3.121940] XTemac: using RxDRE mode
[ 3.164839] XTemac: buffer descriptor size: 32768 (0x8000)
[ 3.231800] XTemac: (buffer_descriptor_init) phy: 0x3cb0000, virt:
0xff100000, size: 0x8000
[ 3.339873] eth%d: XTemac: PHY detected at address 0.
[ 3.400611] eth0: Dropping NETIF_F_SG since no checksum feature.
[ 3.475138] eth0: Xilinx TEMAC #0 at 0x81200000 mapped to 0xC5060000,
irq=0
[ 3.558779] eth0: XTemac id 1.0f, block id 5, type 8
.
.
.
# ifdown eth0
# ifup eth0
[ 20.004852] eth0: XTemac: Options: 0xb8f2
[ 32.012102] eth0: XTemac: Not able to set the speed to 1000 (status:
0x7949)
[ 34.089874] eth0: XTemac: We renegotiated the speed to: 100
[ 34.156549] eth0: XTemac: speed set to 100Mb/s
[ 34.209718] eth0: XTemac: Send Threshold = 16, Receive Threshold = 2
[ 34.285693] eth0: XTemac: Send Wait bound = 1, Receive Wait bound = 1
udhcpc (v1.7.2) started
udhcpc: script /usr/share/udhcpc/default.script failed: No such file or
directory
Sending discover...
Sending select for 192.168.0.2...
Lease of 192.168.0.2 obtained, lease time 86400
udhcpc: script /usr/share/udhcpc/default.script failed: No such file or
directory
# ifconfig
eth0 Link encap:Ethernet HWaddr 08:00:3E:26:15:59
UP BROADCAST RUNNING MTU:1500 Metric:1
RX packets:3 errors:0 dropped:0 overruns:0 frame:0
TX packets:2 errors:0 dropped:0 overruns:0 carrier:0
collisions:0 txqueuelen:1000
RX bytes:1240 (1.2 KiB) TX bytes:1180 (1.1 KiB)
lo Link encap:Local Loopback
inet addr:127.0.0.1 Mask:255.0.0.0
UP LOOPBACK RUNNING MTU:16436 Metric:1
RX packets:0 errors:0 dropped:0 overruns:0 frame:0
TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
collisions:0 txqueuelen:0
RX bytes:0 (0.0 B) TX bytes:0 (0.0 B)
- Brendan
--
View this message in context: http://www.nabble.com/No-ip-addr-on-the-Xilinx-ML403-tf4755897.html#a13600333
Sent from the linuxppc-embedded mailing list archive at Nabble.com.
^ permalink raw reply
* [RFC/PATCH] reduce load time for modules with lots of relocs
From: Nathan Lynch @ 2007-11-06 2:33 UTC (permalink / raw)
To: Medve Emilian-EMMEDVE1; +Cc: linuxppc-dev
In-Reply-To: <20071101235728.GE9695@localdomain>
Nathan Lynch wrote:
> Medve Emilian-EMMEDVE1 wrote:
> >
> > I have module with 36K relocation entries (I know, I know, how could
> > that be...) and the count_relocs() function takes ~17 seconds to crunch
> > through the relocation table from the .rela.text section. I don't think
> > I can reduce the number of entries in the relocation table (can I?) so
> > I'm thinking at improving the performance of count_relocs() (currently
> > O(n^2)). Does anybody have a simpler idea? Does anyone have any
> > constructive suggestion on how to improve the situation?
>
> This seems to come up every few months. There was a patch submitted
> here:
>
> http://ozlabs.org/pipermail/linuxppc-dev/2007-June/037641.html
I think this comes up often enough for us to fix it (IIRC unionfs
people complained about it long ago too), and it's kind of lame to
spend 17 seconds in the kernel without scheduling. So I dug up some
old patches that should reduce the complexity to O(n logn), using the
sort() function, which IMO is preferable to doing our own hash thing
as that patch from June does. Only the 64-bit code is tested. Does
this help your situation?
arch/powerpc/kernel/module_32.c | 85 +++++++++++++++++++++++++++++++---------
arch/powerpc/kernel/module_64.c | 80 ++++++++++++++++++++++++++++++-------
2 files changed, 132 insertions(+), 33 deletions(-)
diff --git a/arch/powerpc/kernel/module_32.c b/arch/powerpc/kernel/module_32.c
index 07a89a3..001b941 100644
--- a/arch/powerpc/kernel/module_32.c
+++ b/arch/powerpc/kernel/module_32.c
@@ -24,6 +24,7 @@
#include <linux/kernel.h>
#include <linux/cache.h>
#include <linux/bug.h>
+#include <linux/sort.h>
#include "setup.h"
@@ -50,26 +51,64 @@ void module_free(struct module *mod, void *module_region)
table entries. */
}
+static int reloc_cmp(const void *_a, const void *_b)
+{
+ const Elf32_Rela *a = *(Elf32_Rela **)_a, *b = *(Elf32_Rela **)_b;
+
+ if (a->r_info < b->r_info)
+ return -1;
+ else if (a->r_info > b->r_info)
+ return 1;
+ else if (a->r_addend < b->r_addend)
+ return -1;
+ else if (a->r_addend > b->r_addend)
+ return 1;
+
+ return 0;
+}
+
+
/* Count how many different relocations (different symbol, different
addend) */
static unsigned int count_relocs(const Elf32_Rela *rela, unsigned int num)
{
- unsigned int i, j, ret = 0;
+ unsigned int i, sorted_count = 0;
+ Elf32_Word last_info;
+ Elf32_Sword last_addend;
+ Elf32_Rela **sortbuf = NULL;
+
+ if (num == 0)
+ return 0;
+
+ sortbuf = vmalloc(num * sizeof(*sortbuf));
+
+ if (!sortbuf)
+ return -ENOMEM;
+
+ for (i = 0; i < num; i++)
+ sortbuf[i] = (Elf32_Rela *)&rela[i];
+
+ sort(sortbuf, i, sizeof(sortbuf[0]), reloc_cmp, NULL);
+
+ last_info = sortbuf[0]->r_info;
+ last_addend = sortbuf[0]->r_addend;
+ sorted_count = 1;
- /* Sure, this is order(n^2), but it's usually short, and not
- time critical */
for (i = 0; i < num; i++) {
- for (j = 0; j < i; j++) {
- /* If this addend appeared before, it's
- already been counted */
- if (ELF32_R_SYM(rela[i].r_info)
- == ELF32_R_SYM(rela[j].r_info)
- && rela[i].r_addend == rela[j].r_addend)
- break;
- }
- if (j == i) ret++;
+ /* If this r_info,r_addend tuple matches the previous
+ * entry, don't count it again
+ */
+ if (sortbuf[i]->r_info == last_info &&
+ sortbuf[i]->r_addend == last_addend)
+ continue;
+
+ last_info = sortbuf[i]->r_info;
+ last_addend = sortbuf[i]->r_addend;
+ sorted_count++;
}
- return ret;
+
+ vfree(sortbuf);
+ return sorted_count;
}
/* Get the potential trampolines size required of the init and
@@ -96,15 +135,19 @@ static unsigned long get_plt_size(const Elf32_Ehdr *hdr,
continue;
if (sechdrs[i].sh_type == SHT_RELA) {
+ int count;
DEBUGP("Found relocations in section %u\n", i);
DEBUGP("Ptr: %p. Number: %u\n",
(void *)hdr + sechdrs[i].sh_offset,
sechdrs[i].sh_size / sizeof(Elf32_Rela));
- ret += count_relocs((void *)hdr
+ count = count_relocs((void *)hdr
+ sechdrs[i].sh_offset,
sechdrs[i].sh_size
/ sizeof(Elf32_Rela))
* sizeof(struct ppc_plt_entry);
+ if (count < 0)
+ return count;
+ ret += count;
}
}
@@ -117,6 +160,7 @@ int module_frob_arch_sections(Elf32_Ehdr *hdr,
struct module *me)
{
unsigned int i;
+ int ret;
/* Find .plt and .init.plt sections */
for (i = 0; i < hdr->e_shnum; i++) {
@@ -131,10 +175,15 @@ int module_frob_arch_sections(Elf32_Ehdr *hdr,
}
/* Override their sizes */
- sechdrs[me->arch.core_plt_section].sh_size
- = get_plt_size(hdr, sechdrs, secstrings, 0);
- sechdrs[me->arch.init_plt_section].sh_size
- = get_plt_size(hdr, sechdrs, secstrings, 1);
+ ret = get_plt_size(hdr, sechdrs, secstrings, 0);
+ if (ret < 0)
+ return ret;
+ sechdrs[me->arch.core_plt_section].sh_size = ret;
+
+ ret = get_plt_size(hdr, sechdrs, secstrings, 1);
+ if (ret < 0)
+ return ret;
+ sechdrs[me->arch.init_plt_section].sh_size = ret;
return 0;
}
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 75c7c4f..bfc5b98 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -21,6 +21,7 @@
#include <linux/err.h>
#include <linux/vmalloc.h>
#include <linux/bug.h>
+#include <linux/sort.h>
#include <asm/module.h>
#include <asm/uaccess.h>
#include <asm/firmware.h>
@@ -77,28 +78,68 @@ static struct ppc64_stub_entry ppc64_stub =
0x4e, 0x80, 0x04, 0x20 /* bctr */
} };
+static int reloc_cmp(const void *_a, const void *_b)
+{
+ const Elf64_Rela *a = *(Elf64_Rela **)_a, *b = *(Elf64_Rela **)_b;
+
+ if (a->r_info < b->r_info)
+ return -1;
+ else if (a->r_info > b->r_info)
+ return 1;
+ else if (a->r_addend < b->r_addend)
+ return -1;
+ else if (a->r_addend > b->r_addend)
+ return 1;
+
+ return 0;
+}
+
/* Count how many different 24-bit relocations (different symbol,
different addend) */
-static unsigned int count_relocs(const Elf64_Rela *rela, unsigned int num)
+static int count_relocs(const Elf64_Rela *rela, unsigned int num)
{
unsigned int i, j, ret = 0;
+ Elf64_Xword last_info;
+ Elf64_Sxword last_addend;
+ Elf64_Rela **sortbuf = NULL;
+
+ if (num == 0)
+ return 0;
+
+ sortbuf = vmalloc(num * sizeof(*sortbuf));
- /* FIXME: Only count external ones --RR */
- /* Sure, this is order(n^2), but it's usually short, and not
- time critical */
- for (i = 0; i < num; i++) {
+ if (!sortbuf)
+ return -ENOMEM;
+
+ for (j = 0, i = 0; i < num; i++) {
/* Only count 24-bit relocs, others don't need stubs */
if (ELF64_R_TYPE(rela[i].r_info) != R_PPC_REL24)
continue;
- for (j = 0; j < i; j++) {
- /* If this addend appeared before, it's
- already been counted */
- if (rela[i].r_info == rela[j].r_info
- && rela[i].r_addend == rela[j].r_addend)
- break;
- }
- if (j == i) ret++;
+ sortbuf[j++] = (Elf64_Rela *)&rela[i];
}
+
+ if (j == 0)
+ goto out;
+
+ sort(sortbuf, j, sizeof(sortbuf[0]), reloc_cmp, NULL);
+
+ last_info = sortbuf[0]->r_info;
+ last_addend = sortbuf[0]->r_addend;
+ ret = 1;
+ for (i = 0; i < j; i++) {
+ /* If this r_info,r_addend tuple matches the previous
+ * entry, don't count it again
+ */
+ if (sortbuf[i]->r_info == last_info &&
+ sortbuf[i]->r_addend == last_addend)
+ continue;
+
+ last_info = sortbuf[i]->r_info;
+ last_addend = sortbuf[i]->r_addend;
+ ret++;
+ }
+out:
+ vfree(sortbuf);
return ret;
}
@@ -129,13 +170,17 @@ static unsigned long get_stubs_size(const Elf64_Ehdr *hdr,
/* Every relocated section... */
for (i = 1; i < hdr->e_shnum; i++) {
if (sechdrs[i].sh_type == SHT_RELA) {
+ int count;
DEBUGP("Found relocations in section %u\n", i);
DEBUGP("Ptr: %p. Number: %lu\n",
(void *)sechdrs[i].sh_addr,
sechdrs[i].sh_size / sizeof(Elf64_Rela));
- relocs += count_relocs((void *)sechdrs[i].sh_addr,
+ count = count_relocs((void *)sechdrs[i].sh_addr,
sechdrs[i].sh_size
/ sizeof(Elf64_Rela));
+ if (count < 0)
+ return count;
+ relocs += count;
}
}
@@ -173,6 +218,7 @@ int module_frob_arch_sections(Elf64_Ehdr *hdr,
struct module *me)
{
unsigned int i;
+ int stubs_size;
/* Find .toc and .stubs sections, symtab and strtab */
for (i = 1; i < hdr->e_shnum; i++) {
@@ -209,7 +255,11 @@ int module_frob_arch_sections(Elf64_Ehdr *hdr,
me->arch.toc_section = me->arch.stubs_section;
/* Override the stubs size */
- sechdrs[me->arch.stubs_section].sh_size = get_stubs_size(hdr, sechdrs);
+ stubs_size = get_stubs_size(hdr, sechdrs);
+ if (stubs_size < 0)
+ return stubs_size;
+
+ sechdrs[me->arch.stubs_section].sh_size = stubs_size;
return 0;
}
^ permalink raw reply related
* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Stephen Rothwell @ 2007-11-06 2:28 UTC (permalink / raw)
To: Jon Smirl; +Cc: Tjernlund, Jean Delvare, i2c, linuxppc-dev
In-Reply-To: <9e4733910711051734m7cf1fac0y5fb39feb37bad548@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1260 bytes --]
On Mon, 5 Nov 2007 20:34:51 -0500 "Jon Smirl" <jonsmirl@gmail.com> wrote:
>
> On 11/5/07, Scott Wood <scottwood@freescale.com> wrote:
> > > +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?
>
> This is a general question, if of_platform_driver doesn't need .name
> it should be removed from the structure.
I am in the process of doing that. However there a quite a few drivers
that need to be fixed first. In the meantime,
of_register_platform_driver will copy which ever of the name and owner
fields you fill in to the others, so we can convert over time. For new
drivers (and changing), please use the name and owner fields in the
embedded device_driver struct. (this is the same as done by the platform
drivers currently ...)
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* [PATCH] [POWERPC] pasemi: Don't reset mpic at boot
From: Olof Johansson @ 2007-11-06 2:21 UTC (permalink / raw)
To: linuxppc-dev
[POWERPC] pasemi: Don't reset mpic at boot
Due to an erratum, we don't want to reset the mpic at boot time. It can
sometimes cause problems with lost interrupts later on while running.
Signed-off-by: Olof Johansson <olof@lixom.net>
diff --git a/arch/powerpc/platforms/pasemi/setup.c b/arch/powerpc/platforms/pasemi/setup.c
index 857d238..bd85853 100644
--- a/arch/powerpc/platforms/pasemi/setup.c
+++ b/arch/powerpc/platforms/pasemi/setup.c
@@ -214,7 +214,7 @@ static __init void pas_init_IRQ(void)
printk(KERN_DEBUG "OpenPIC addr: %lx\n", openpic_addr);
mpic = mpic_alloc(mpic_node, openpic_addr,
- MPIC_PRIMARY|MPIC_LARGE_VECTORS|MPIC_WANTS_RESET,
+ MPIC_PRIMARY|MPIC_LARGE_VECTORS,
0, 0, " PAS-OPIC ");
BUG_ON(!mpic);
^ permalink raw reply related
* Re: [PATCH] powerpc: prpmc2800 - Don't trust documented memory size
From: Mark A. Greer @ 2007-11-06 1:59 UTC (permalink / raw)
To: Mark A. Greer; +Cc: linuxppc-dev
In-Reply-To: <20071106012805.GA20909@mag.az.mvista.com>
On Mon, Nov 05, 2007 at 06:28:05PM -0700, Mark A. Greer wrote:
> It turns out that the firmware on some PrPMC2800 processor modules
> initializes the memory controller for 512 MB even when there is more
> memory. As a simple work around, set the amount of memory in the
> device tree passed to the kernel to the lesser of what the memory
> controller is set up for and the actual amount of memory.
>
> Signed-off-by: Mark A. Greer <mgreer@mvista.com>
> ---
Paul, please ignore this patch. There is still something wrong.
Sorry for the noise.
Mark
^ permalink raw reply
* Re: [PATCH] [POWERPC] pasemi: Broaden specific references to 1682M
From: Olof Johansson @ 2007-11-06 2:01 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev, Doug Thompson
In-Reply-To: <cd9a9e67893406e486d2ffc7599e28b7@kernel.crashing.org>
On Tue, Nov 06, 2007 at 02:42:08AM +0100, Segher Boessenkool wrote:
> Hi Olof,
>
> Nice cleanup! One minor thing...
>
>> static struct of_device_id pasemi_bus_ids[] = {
>> + /* Unfortunately needed for legacy firmwares */
>> { .type = "localbus", },
>> { .type = "sdc", },
>> + { .compatible = "pasemi,localbus", },
>> + { .compatible = "pasemi,sdc", },
>
> The new comment is for the device_type entries only, right? You
> might want to make that more clear.
Good point. I'll clarify it with a new comment above the compatible
entries. I won't repost the patch since the change is minor, but include
it for the upstream push.
Thanks,
-Olof
^ permalink raw reply
* Re: [PATCH] [POWERPC] pasemi: Broaden specific references to 1682M
From: Segher Boessenkool @ 2007-11-06 1:42 UTC (permalink / raw)
To: Olof Johansson; +Cc: linuxppc-dev, Doug Thompson
In-Reply-To: <20071105032802.GA16613@lixom.net>
Hi Olof,
Nice cleanup! One minor thing...
> static struct of_device_id pasemi_bus_ids[] = {
> + /* Unfortunately needed for legacy firmwares */
> { .type = "localbus", },
> { .type = "sdc", },
> + { .compatible = "pasemi,localbus", },
> + { .compatible = "pasemi,sdc", },
The new comment is for the device_type entries only, right? You
might want to make that more clear.
Segher
^ permalink raw reply
* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Jon Smirl @ 2007-11-06 1:34 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:
> > +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?
This is a general question, if of_platform_driver doesn't need .name
it should be removed from the structure.
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* [PATCH] powerpc: prpmc2800 - Don't trust documented memory size
From: Mark A. Greer @ 2007-11-06 1:28 UTC (permalink / raw)
To: linuxppc-dev
It turns out that the firmware on some PrPMC2800 processor modules
initializes the memory controller for 512 MB even when there is more
memory. As a simple work around, set the amount of memory in the
device tree passed to the kernel to the lesser of what the memory
controller is set up for and the actual amount of memory.
Signed-off-by: Mark A. Greer <mgreer@mvista.com>
---
arch/powerpc/boot/prpmc2800.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/boot/prpmc2800.c b/arch/powerpc/boot/prpmc2800.c
index 9614e1d..559c45e 100644
--- a/arch/powerpc/boot/prpmc2800.c
+++ b/arch/powerpc/boot/prpmc2800.c
@@ -405,7 +405,10 @@ static void prpmc2800_fixups(void)
bip = prpmc2800_get_bip(); /* Get board info based on VPD */
- mem_size = (bip) ? bip->mem_size : mv64x60_get_mem_size(bridge_base);
+ mem_size = mv64x60_get_mem_size(bridge_base);
+ if (bip)
+ mem_size = min(mem_size, bip->mem_size);
+
prpmc2800_bridge_setup(mem_size); /* Do necessary bridge setup */
/* If the VPD doesn't match what we know about, just use the
^ permalink raw reply related
* Re: [PATCH] 4xx; Replace #includes of asm/of_platform.h with linux/of_platform.h.
From: Stephen Rothwell @ 2007-11-06 1:10 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev@ozlabs.org
In-Reply-To: <20071105175341.6ee15106@vader.jdub.homelinux.org>
[-- Attachment #1: Type: text/plain, Size: 455 bytes --]
On Mon, 5 Nov 2007 17:53:41 -0600 Josh Boyer <jwboyer@gmail.com> wrote:
>
> Yeah, this is fine with me too. But why haven't we put a #warning in
> asm/of_platform.h and asm/prom.h?
Because thee are way to many places to be fixed yet. And asm/prom.h needs to be explicitly included if you want to use the flattened device tree accessors.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Jon Smirl @ 2007-11-06 0:41 UTC (permalink / raw)
To: Scott Wood; +Cc: Tjernlund, linuxppc-dev, Jean Delvare, i2c
In-Reply-To: <472F8267.8070106@freescale.com>
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. 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.
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Jon Smirl @ 2007-11-06 0:33 UTC (permalink / raw)
To: Grant Likely; +Cc: Tjernlund, Jean Delvare, i2c, linuxppc-dev
In-Reply-To: <fa686aa40711051446q1abe886dh6225fa1ec675ef9b@mail.gmail.com>
On 11/5/07, Grant Likely <grant.likely@secretlab.ca> wrote:
> On 11/5/07, Scott Wood <scottwood@freescale.com> wrote:
> > 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 agree. This driver can and should have multiple bus bindings.
What non-of platform uses this driver?
I believe this is the last struct platform driver left for the
mpc5200. Fixing this one allows the platform bus to be removed. With a
device tree there isn't any reason to keep a platform bus, everything
should be on the of_platform bus.
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [PATCH] 4xx; Replace #includes of asm/of_platform.h with linux/of_platform.h.
From: Josh Boyer @ 2007-11-05 23:53 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: linuxppc-dev@ozlabs.org
In-Reply-To: <20071106094014.cd429711.sfr@canb.auug.org.au>
On Tue, 6 Nov 2007 09:40:14 +1100
Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> On Mon, 05 Nov 2007 15:43:43 -0600 Jon Loeliger <jdl@freescale.com> wrote:
> >
> > From: Jon Loeliger <jdl@freescale.com>
> >
> > Signed-off-by: Jon Loeliger <jdl@freescale.com>
>
> Acked-by: Stephen Rothwell <sfr@canb.auug.org.au>
Yeah, this is fine with me too. But why haven't we put a #warning in
asm/of_platform.h and asm/prom.h?
josh
^ permalink raw reply
* Re: libfdt: Fix sw_tree1 testcase
From: David Gibson @ 2007-11-05 23:42 UTC (permalink / raw)
To: Jon Loeliger; +Cc: linuxppc-dev
In-Reply-To: <E1Ip3bp-000260-Of@jdl.com>
On Mon, Nov 05, 2007 at 09:11:25AM -0600, Jon Loeliger wrote:
> So, like, the other day David Gibson mumbled:
> > 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>
>
> This change appears to cause a test to fail.
> I've not looked into it beyond "make check failed":
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>
Index: dtc/tests/run_tests.sh
===================================================================
--- dtc.orig/tests/run_tests.sh 2007-11-06 10:38:21.000000000 +1100
+++ dtc/tests/run_tests.sh 2007-11-06 10:39:37.000000000 +1100
@@ -71,6 +71,7 @@
run_test sw_tree1
tree1_tests sw_tree1.test.dtb
tree1_tests unfinished_tree1.test.dtb
+ run_test dtbs_equal_ordered test_tree1.dtb sw_tree1.test.dtb
# fdt_move tests
for tree in test_tree1.dtb sw_tree1.test.dtb unfinished_tree1.test.dtb; do
Index: dtc/tests/sw_tree1.c
===================================================================
--- dtc.orig/tests/sw_tree1.c 2007-11-05 16:59:13.000000000 +1100
+++ dtc/tests/sw_tree1.c 2007-11-06 10:39:37.000000000 +1100
@@ -64,7 +64,6 @@
CHECK(fdt_begin_node(fdt, "subsubnode"));
CHECK(fdt_property(fdt, "compatible", "subsubnode1\0subsubnode",
23));
- CHECK(fdt_property_string(fdt, "compatible", "subsubnode1\0"));
CHECK(fdt_property_typed(fdt, "prop-int", TEST_VALUE_1));
CHECK(fdt_end_node(fdt));
CHECK(fdt_end_node(fdt));
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Grant Likely @ 2007-11-05 23:03 UTC (permalink / raw)
To: Scott Wood; +Cc: Tjernlund, Jean Delvare, i2c, linuxppc-dev
In-Reply-To: <472F915F.9010400@freescale.com>
On 11/5/07, Scott Wood <scottwood@freescale.com> wrote:
> Matt Sealey wrote:
> > 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,
>
> I meant a controller device number (a.k.a. bus number), which (outside
> of documentation) is purely a Linux invention, and which is what
> cell-index was being used for above.
>
> > 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)
Actually, it is not intended for this. cell-index is not intended to
be able to say GPT1 instead of GPT@xxx (while that may be possible, it
is not the intent). Nor is it a holdover from the PSC code design.
It is designed to describe the internal structure of the SoC. The
5200 has 6 PSCs, and there are some chip registers which are shared
between all the PSCs (for clocking, IO mode, etc). It is not
sufficient to simply plop down a device tree node for each PSC because
it doesn't give enough information about which bits to use in the
shared regs. (For example, the port_config register)
> > 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.
>
> Right, that's what cell-index is for. This is different. :-)
Yes, this is correct.
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-05 22:46 UTC (permalink / raw)
To: Scott Wood; +Cc: Tjernlund, linuxppc-dev, Jean Delvare, i2c
In-Reply-To: <472F8267.8070106@freescale.com>
On 11/5/07, Scott Wood <scottwood@freescale.com> wrote:
> 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 agree. This driver can and should have multiple bus bindings.
> >>> 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.
cell-index is important. It describes the hardware, or more
specifically the layout of the SoC. The SoC has 2 i2c busses which
are numbered 0 and 1. This property should stay for the 5200.
However, that is the only purpose of it. cell-index does *not*
describe the system level bus number.
> > 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.
Yes, the purpose of cell-index is not to give an i2c bus number enumeration.
Cheers,
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195
^ permalink raw reply
* Re: [PATCH] 4xx; Replace #includes of asm/of_platform.h with linux/of_platform.h.
From: Stephen Rothwell @ 2007-11-05 22:40 UTC (permalink / raw)
To: Jon Loeliger; +Cc: linuxppc-dev@ozlabs.org
In-Reply-To: <1194299023.7158.59.camel@ld0161-tx32>
[-- Attachment #1: Type: text/plain, Size: 338 bytes --]
On Mon, 05 Nov 2007 15:43:43 -0600 Jon Loeliger <jdl@freescale.com> wrote:
>
> From: Jon Loeliger <jdl@freescale.com>
>
> Signed-off-by: Jon Loeliger <jdl@freescale.com>
Acked-by: Stephen Rothwell <sfr@canb.auug.org.au>
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] Replace some #includes of asm/of_platform.h with linux/of_platform.h.
From: Stephen Rothwell @ 2007-11-05 22:36 UTC (permalink / raw)
To: Jon Loeliger; +Cc: linuxppc-dev@ozlabs.org
In-Reply-To: <1194296705.7158.45.camel@ld0161-tx32>
[-- Attachment #1: Type: text/plain, Size: 465 bytes --]
Hi Jon,
Thanks for starting this.
On Mon, 05 Nov 2007 15:05:06 -0600 Jon Loeliger <jdl@freescale.com> wrote:
>
> From: Jon Loeliger <jdl@freescale.com>
>
> Signed-off-by: Jon Loeliger <jdl@freescale.com>
Acked-by: Stephen Rothwell <sfr@canb.auug.org.au>
> #include <asm/of_device.h>
asm/of_device.h -> linux/of_device.h as well? :-)
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [RFC] Rework of i2c-mpc.c - Freescale i2c driver
From: Scott Wood @ 2007-11-05 21:55 UTC (permalink / raw)
To: Matt Sealey; +Cc: Tjernlund, linuxppc-dev, Jean Delvare, i2c
In-Reply-To: <472F9086.2060606@genesi-usa.com>
Matt Sealey wrote:
> 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,
I meant a controller device number (a.k.a. bus number), which (outside
of documentation) is purely a Linux invention, and which is what
cell-index was being used for above.
> 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.
Right, that's what cell-index is for. This is different. :-)
-Scott
^ permalink raw reply
* 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
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