* Re: [PATCH 10/61] bootwrapper: flatdevtree fixes
From: Milton Miller @ 2007-07-18 15:43 UTC (permalink / raw)
To: Scott Wood; +Cc: ppcdev, Paul Mackerras, David Gibson
In-Reply-To: <20070718013137.GA15217@ld0162-tx32.am.freescale.net>
On Wed Jul 18 11:33:08 EST 2007, Scott Wood wrote:
> 1. ft_create_node was returning the internal pointer rather than a
> phandle.
> 2. ft_find_device_rel was treating lookups relative to root as an
> error.
No, it is treating lookups relative to NULL as an error.
Your patch changes it to treat lookups relative to the NULL phandle as
relative to root.
I've no objections to the other part, can you split these?
>
> Signed-off-by: Scott Wood <scottwood at freescale.com>
> ---
> arch/powerpc/boot/flatdevtree.c | 12 ++++++++----
> 1 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/boot/flatdevtree.c
> b/arch/powerpc/boot/flatdevtree.c
> index b732644..5b69aeb 100644
> --- a/arch/powerpc/boot/flatdevtree.c
> +++ b/arch/powerpc/boot/flatdevtree.c
> @@ -659,9 +659,13 @@ void *ft_find_device_rel(struct ft_cxt *cxt,
> const void *top,
> {
> char *node;
>
> - node = ft_node_ph2node(cxt, top);
> - if (node == NULL)
> - return NULL;
> + if (top) {
> + node = ft_node_ph2node(cxt, top);
> + if (node == NULL)
> + return NULL;
> + } else {
> + node = ft_root_node(cxt);
> + }
>
milton
^ permalink raw reply
* Re: [PATCH 09/61] bootwrapper: Add dt_is_compatible().
From: Milton Miller @ 2007-07-18 15:42 UTC (permalink / raw)
To: Scott Wood; +Cc: ppcdev, Paul Mackerras, David Gibson
In-Reply-To: <20070718013137.GA15217@ld0162-tx32.am.freescale.net>
On Wed Jul 18 11:33:06 EST 2007, Scott Wood wrote:
> +int dt_is_compatible(void *node, const char *compat)
> +{
> + char *buf = (char *)prop_buf;
> + int compat_len = strlen(compat);
> + int len, pos;
> +
> + len = getprop(node, "compatible", buf, MAX_PROP_LEN);
> + if (len < 0)
> + return 0;
> +
> + for (pos = 0; pos + compat_len < len; pos++) {
> + if (!strcmp(buf + pos, compat))
> + return 1;
> +
> + while (buf[pos] && pos + compat_len < len)
> + pos++;
This is buggy: if you are searching for "ns16550" and the compatable is
"fsl,1234\0commons16550" this code will incorrectly says its
compatable.
Comparing pos < len instead will do the right thing, at the cost of a
few iterations of the loop.
> + }
> +
> + return 0;
milton
^ permalink raw reply
* Re: [PATCH 2/2] fix showing xmon help
From: Milton Miller @ 2007-07-18 15:51 UTC (permalink / raw)
To: Ishizaki Kou; +Cc: ppcdev, Paul Mackerras
In-Reply-To: <20070718.182640.-1300542340.kouish@swc.toshiba.co.jp>
On Wed Jul 18 19:26:40 EST 2007, Ishizaki Kou wrote:
> In some configuration, xmon help string is larger than xmon_printf
> buffer. We need not to use printf. This patch adds xmon_puts and
> change to use it to show help string.
[Since I'm requesting changes I'll suggest a new change log.]
In some configurations the xmon help string is larger than the
xmon_printf buffer. However, since its a fixed string we do not need
printf to format it. This patch adds xmon_puts and uses it to show the
help string.
> case '?':
> - printf(help_string);
> + xmon_puts(help_string);
> break;
>
nonstdio.h #defines printf to xmon_printf. Please add a similar line
for puts, and use the define here. (It will avoid an unnecessary
difference with the user space version).
Thanks,
milton
^ permalink raw reply
* Re: [PATCH 3/3] First cut at PReP support for arch/powerpc
From: Segher Boessenkool @ 2007-07-18 15:55 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <20070718013110.GA18251@localhost.localdomain>
>>> Too big for the list, the patch is at:
>>> http://ozlabs.org/~dgibson/home/prep-support
>>
>> Too lazy to split the patch into bite-size chunks, you mean ;-)
>
> Well... much as I like small patches, I don't really like having a big
> string of patches, each of which does basically nothing on its own,
> i.e. split up just for the sake of making smaller, rather than into
> separate logically separate changes.
Yeah I understand. It's just that I had to dig out the DTS
part :-)
>> + external-control;
>>
>> Really?
>
> No idea, just copied that from earlier work of Paulus'. Don't even
> know what the property means.
It seems to me a flat device tree could leave out all those
properties that can be derived from the PVR (except for the
few that are really useful for bootloaders and such -- cache
line size, 64-bit-or-not, what kind of MMU). Linux doesn't
use it anyway. Existing DTSs already leave away most.
>> + pci@80000000 {
>> + device_type = "pci";
>> + compatible = "prep";
>>
>> Is that specific enough?
>
> Well, AFAICT, the prep PCI code doesn't need any more info.
If PReP requires a specific programming model for the PCI
host bridge, that would be fine. But then compatible =
"prep-pci-bridge" or such, not just "prep"; everything on
your board is "prep", it would make matching a bit hard ;-)
>> I can't believe this "ranges" and interrupt mapping will
>> work on all PReP systems...
>
> Probably not, but it should work on a chunk of them. Like I say,
> there's still a good deal more that needs to be filled in from
> residual data or wherever.
Sure, I'm just pointing out things that seem problematic, I'm
not saying your code can't be merged because of that -- esp.
since it is a new port anyway (for arch/powerpc, that is).
>> What is the plan here -- have the bootwrapper build the
>> device tree / fill in the details from the residual data?
>
> Not sure at this stage if it will be best for the bootwrapper to build
> a complete tree from residual, or to have a dts skeleton with
> substantial chunks filled in by bootwrapper from residual.
Conceptually those two options are pretty much the same thing;
just try them out, see what is nicer for the implementation.
> I was
> intending to merge libfdt into the kernel for more flexible device
> tree manipulation before investigating that further.
Into the kernel wrapper, I think you mean?
Segher
^ permalink raw reply
* Re: [Bugme-new] [Bug 8778] New: Ocotea board: kernel reports access of bad area during boot with DEBUG_SLAB=y
From: Eugene Surovegin @ 2007-07-18 15:59 UTC (permalink / raw)
To: Josh Boyer
Cc: bart.vanassche, netdev, Andrew Morton,
bugme-daemon@kernel-bugs.osdl.org, linuxppc-embedded
In-Reply-To: <1184766070.3699.2.camel@zod.rchland.ibm.com>
On Wed, Jul 18, 2007 at 08:41:10AM -0500, Josh Boyer wrote:
> On Wed, 2007-07-18 at 01:34 -0700, Eugene Surovegin wrote:
> > On Wed, Jul 18, 2007 at 12:52:53AM -0700, Andrew Morton wrote:
> > > On Wed, 18 Jul 2007 00:07:50 -0700 (PDT) bugme-daemon@bugzilla.kernel.org wrote:
> > >
> > > > http://bugzilla.kernel.org/show_bug.cgi?id=8778
> > > >
> > > > Summary: Ocotea board: kernel reports access of bad area during
> > > > boot with DEBUG_SLAB=y
> >
> > Slab debugging is probably the culprit here. I had similar problem
> > couple of years ago, not sure something has changed since then,
> > haven't checked.
> >
> > When slab debugging was enabled it made memory allocations non L1
> > cache line aligned. This is very bad for DMA on non-coherent cache
> > arches (PPC440 is one of those archs).
> >
> > I have a hack for EMAC which tries to "workaround" this problem:
> > http://kernel.ebshome.net/emac_slab_debug.diff
> > which might help.
>
> Would you be opposed to including that patch in mainline?
Yes. I don't think it's the right way to fix this issue. IMO, the
right one is to fix slab allocator. You cannot change all drivers to
do this kind of cache flushing, and yes, I saw the same problem with
PCI based NIC I tried on Ocotea at the time.
--
Eugene
^ permalink raw reply
* Re: [PATCH 05/61] bootwrapper: Change cuImage .gitignore entry to cuImage.*
From: Scott Wood @ 2007-07-18 16:09 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20070718014200.GB18251@localhost.localdomain>
David Gibson wrote:
> On Tue, Jul 17, 2007 at 08:33:01PM -0500, Scott Wood wrote:
>
>>Signed-off-by: Scott Wood <scottwood@freescale.com>
>>---
>> arch/powerpc/boot/.gitignore | 4 +---
>> 1 files changed, 1 insertions(+), 3 deletions(-)
>>
>>diff --git a/arch/powerpc/boot/.gitignore b/arch/powerpc/boot/.gitignore
>>index eec7af7..3270335 100644
>>--- a/arch/powerpc/boot/.gitignore
>>+++ b/arch/powerpc/boot/.gitignore
>>@@ -18,9 +18,7 @@ kernel-vmlinux.strip.c
>> kernel-vmlinux.strip.gz
>> mktree
>> uImage
>>-cuImage
>>-cuImage.bin.gz
>>-cuImage.elf
>>+cuImage.*
>> zImage
>> zImage.chrp
>> zImage.coff
>
>
> Err... your new wildcard won't match plain 'cuImage'.
When is plain "cuImage" ever generated anymore?
-Scott
^ permalink raw reply
* Re: [PATCH 22/61] Rename mpc82xx_ads to mpc8272ads.
From: Scott Wood @ 2007-07-18 16:10 UTC (permalink / raw)
To: Mark Zhan; +Cc: linuxppc-dev
In-Reply-To: <1184728051.31656.35.camel@mark>
Mark Zhan wrote:
> Scott,
>
> It seems the old name "mpc82xx ads" is still used in your code.
> It will be nice that your file name match your code name, you know, less
> confusion.
See patch 24.
AIUI, it's generally preferred when moving large chunks of code to first
have a patch that just moves it, and then a subsequent one that makes
changes. I was also under the impression that git would generate a one
line file rename in the diff, but for some reason that didn't happen.
-Scott
^ permalink raw reply
* Re: PS3 Storage Driver O_DIRECT issue
From: Geert Uytterhoeven @ 2007-07-18 16:12 UTC (permalink / raw)
To: Olaf Hering
Cc: Jens Axboe, James E.J. Bottomley, linux-scsi, Alessandro Rubini,
Linux Kernel Development, Linux/PPC Development, Paul Mackerras
In-Reply-To: <20070713122752.GA854@aepfle.de>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1917 bytes --]
On Fri, 13 Jul 2007, Olaf Hering wrote:
> This driver (or the generic PS3 code) has appearently problems with
> O_DIRECT.
> glibc aborts parted because the malloc metadata get corrupted. While it
> is reproducible, the place where it crashes changes with every version
> of the debug attempt.
> I dont have a handle right now, all I know is that the metadata after a
> malloc area get overwritten with zeros.
>
>
> Can you have a look at this?
> parted /dev/ps3da
> print (a few times)
I could reproduce it with parted 1.8.0 and later.
The patch below fixes it (tested with 1.8.0, 1.8.2 (FC6), 1.9.0 (git)).
Apparently sometimes bio_cur_sectors(bio)*KERNEL_SECTOR_SIZE != bvec->bv_len
when using O_DIRECT. Is that true (or a bug?)?
If it's OK, I'll fold this patch into the main ps3disk patch.
---
drivers/block/ps3disk.c | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/block/ps3disk.c
+++ b/drivers/block/ps3disk.c
@@ -108,7 +108,7 @@ static void ps3disk_scatter_gather(struc
__func__, __LINE__, i, bio_segments(bio),
bio_sectors(bio), sector);
bio_for_each_segment(bvec, bio, j) {
- size = bio_cur_sectors(bio)*KERNEL_SECTOR_SIZE;
+ size = bvec->bv_len;
buf = __bio_kmap_atomic(bio, j, KM_IRQ0);
if (gather)
memcpy(dev->bounce_buf+offset, buf, size);
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
^ permalink raw reply
* Re: [PATCH 2/2] fix showing xmon help
From: Andreas Schwab @ 2007-07-18 16:12 UTC (permalink / raw)
To: Milton Miller; +Cc: ppcdev, Paul Mackerras
In-Reply-To: <7bd7b12c7bd2303039a850a1fee9634c@bga.com>
Milton Miller <miltonm@bga.com> writes:
>> case '?':
>> - printf(help_string);
>> + xmon_puts(help_string);
>> break;
>>
>
> nonstdio.h #defines printf to xmon_printf. Please add a similar line
> for puts, and use the define here. (It will avoid an unnecessary
> difference with the user space version).
User space puts add a newline, which this xmon_puts doesn't.
Andreas.
--
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: [PATCH 17/61] bootwrapper: Add 8xx support.
From: Scott Wood @ 2007-07-18 16:13 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20070718033026.GG18251@localhost.localdomain>
David Gibson wrote:
> On Tue, Jul 17, 2007 at 08:33:18PM -0500, Scott Wood wrote:
> [snip]
>
>>diff --git a/arch/powerpc/boot/cuboot-8xx.c b/arch/powerpc/boot/cuboot-8xx.c
>>new file mode 100644
>>index 0000000..35476a0
>>--- /dev/null
>>+++ b/arch/powerpc/boot/cuboot-8xx.c
>>@@ -0,0 +1,47 @@
>>+/*
>>+ * Old U-boot compatibility for 8xx
>>+ *
>>+ * Author: Scott Wood <scottwood@freescale.com>
>>+ *
>>+ * Copyright (c) 2007 Freescale Semiconductor, Inc.
>>+ *
>>+ * This program is free software; you can redistribute it and/or modify it
>>+ * under the terms of the GNU General Public License version 2 as published
>>+ * by the Free Software Foundation.
>>+ */
>>+
>>+#include "ops.h"
>>+#include "stdio.h"
>>+#include "cuboot.h"
>>+
>>+#define TARGET_8xx
>>+#define TARGET_HAS_ETH1
>
> ^^^^^^^^^^^^^^^
> Is that really true for all 8xx boards?
Well, no. Even on a given board, it depends on the version of u-boot.
There's nothing after enet1addr that the bootwrapper cares about,
though, so the only harm is if the device tree has a second network
interface but u-boot doesn't know about it, and the bootwrapper ends up
pulling in junk rather than leaving zeroes.
-Scott
^ permalink raw reply
* Re: [PATCH] Add StorCenter DTS first draft.
From: Segher Boessenkool @ 2007-07-18 16:13 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Jon Loeliger
In-Reply-To: <1184722754.25235.183.camel@localhost.localdomain>
>> + PowerPC,603e { /* Really 8241 */
>> + device_type = "cpu";
>> + reg = <0>;
>> + clock-frequency = <d# 200000000>; /* Hz */
>> + timebase-frequency = <d# 33333333>; /* Hz */
>> + bus-frequency = <0>;
>> + /* Following required by dtc but not used */
>> + i-cache-line-size = <0>;
>> + d-cache-line-size = <0>;
>> + i-cache-size = <4000>;
>> + d-cache-size = <4000>;
>> + };
>
> The 32 bits kernel may not be using those yet, but it will. 64 bits
> does
> already and I plan to merge those bits at one point.
Hrm, what does it use it for? Are you going to require
all other defined CPU properties as well (like the PowerPC
binding does)?
Segher
^ permalink raw reply
* Re: [PATCH 01/61] Use strcasecmp() rather than strncasecmp() when determining device node compatibility.
From: Scott Wood @ 2007-07-18 16:17 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <99CD25DB-EAAC-4CF3-AB14-D825AD11E499@kernel.crashing.org>
Kumar Gala wrote:
> Can these patches be broken into logical clumps?
Probably... I just wanted to get them out there quickly as there've
been numerous people lately inquiring about 82xx support and posting
patches.
> Would be easier to review and obviously the fs_enet patches need to go
> via jeff and netdev.
While I don't think there'll be a build break if the ethernet patches go
in separately, there'll be a functional breakage, so I don't want to
send them to Jeff until I'm confident that the rest of the patches will
be going in the same release.
-Scott
^ permalink raw reply
* Re: [PATCH] Add StorCenter DTS first draft.
From: Segher Boessenkool @ 2007-07-18 16:19 UTC (permalink / raw)
To: Jon Loeliger; +Cc: linuxppc-dev@ozlabs.org list
In-Reply-To: <E1IAvVm-0005yQ-LO@jdl.com>
>>> + compatible = "storcenter";
>>
>> Needs a manufacturer name in there.
>
> Right. Will use:
> compatible = "iomega,storcenter"
Okido.
>>> + PowerPC,603e { /* Really 8241 */
>>
>> So say "PowerPC,8241@0", or "PowerPC,e300@0" (or whatever
>> the CPU core in there is), or simply "cpu@0", following
>> the generic naming recommended practice.
>
> Well, its the 8241 SoC with a 603e core... (This is
> the same phrase currently being used on the Kurobox.)
> I'll use:
>
> PowerPC,8241@0 }
That might be best yes.
>>> + soc10x {
>>
>> Bad name. Where is the binding for this? I don't think
>> I saw it before.
>
> It's what is being used, again, by the Kurobox. I understand
> that doesn't make it "right", just precedented by now.
Sure, just trying to trick you into documenting it ;-)
> How about "soc8241@80000000" instead?
soc@ like suggested by Scott seems just fine.
>>> + compatible = "fsl-i2c";
>>
>> Needs to be more specific.
>
> Hmmm... Not sure what to use here then. There are many
> existing examples using "fsl-i2c" already. Granted, we've
> established that they could be wrong... Should this be
> more like this?:
>
> compatible = "fsl,mpc8241-i2c", "fsl-i2c";
That looks good yes. Or if the kernel side code for
recognising fsl,mpc8241-i2c gets merged in time, you
can leave out fsl-i2c from your device tree completely.
Segher
^ permalink raw reply
* Re: [PATCH 09/61] bootwrapper: Add dt_is_compatible().
From: Milton Miller @ 2007-07-18 16:20 UTC (permalink / raw)
To: Scott Wood; +Cc: ppcdev, Paul Mackerras, David Gibson
In-Reply-To: <20070718013306.GH15238@ld0162-tx32.am.freescale.net>
On Wed Jul 18 11:33:06 EST 2007, Scott Wood wrote:
> +int dt_is_compatible(void *node, const char *compat)
> +{
> + char *buf = (char *)prop_buf;
> + int compat_len = strlen(compat);
> + int len, pos;
> +
> + len = getprop(node, "compatible", buf, MAX_PROP_LEN);
> + if (len < 0)
> + return 0;
> +
> + for (pos = 0; pos + compat_len < len; pos++) {
> + if (!strcmp(buf + pos, compat))
> + return 1;
> +
> + while (buf[pos] && pos + compat_len < len)
> + pos++;
This is buggy: if you are searching for "ns16550" and the compatable is
"fsl,1234\0commons16550" this code will incorrectly says its
compatable.
Comparing pos < len instead will do the right thing, at the cost of a
few iterations of the loop.
> + }
> +
> + return 0;
milton
^ permalink raw reply
* Re: [PATCH 10/61] bootwrapper: flatdevtree fixes
From: Milton Miller @ 2007-07-18 16:20 UTC (permalink / raw)
To: Scott Wood; +Cc: ppcdev, Paul Mackerras, David Gibson
In-Reply-To: <20070718013308.GI15238@ld0162-tx32.am.freescale.net>
On Wed Jul 18 11:33:08 EST 2007, Scott Wood wrote:
> 1. ft_create_node was returning the internal pointer rather than a
> phandle.
> 2. ft_find_device_rel was treating lookups relative to root as an
> error.
No, it is treating lookups relative to NULL as an error.
Your patch changes it to treat lookups relative to the NULL phandle as
relative to root.
I've no objections to the other part, can you split these?
>
> Signed-off-by: Scott Wood <scottwood at freescale.com>
> ---
> arch/powerpc/boot/flatdevtree.c | 12 ++++++++----
> 1 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/boot/flatdevtree.c
> b/arch/powerpc/boot/flatdevtree.c
> index b732644..5b69aeb 100644
> --- a/arch/powerpc/boot/flatdevtree.c
> +++ b/arch/powerpc/boot/flatdevtree.c
> @@ -659,9 +659,13 @@ void *ft_find_device_rel(struct ft_cxt *cxt,
> const void *top,
> {
> char *node;
>
> - node = ft_node_ph2node(cxt, top);
> - if (node == NULL)
> - return NULL;
> + if (top) {
> + node = ft_node_ph2node(cxt, top);
> + if (node == NULL)
> + return NULL;
> + } else {
> + node = ft_root_node(cxt);
> + }
>
milton
^ permalink raw reply
* Re: [PATCH 06/61] 8xx: Don't call non-existent Soft_emulate_8xx from SoftwareEmulation.
From: Scott Wood @ 2007-07-18 16:22 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <3F44085F-8A67-4134-95A3-39897F2A84FD@kernel.crashing.org>
Kumar Gala wrote:
>
> On Jul 17, 2007, at 8:33 PM, Scott Wood wrote:
>
>> On arch/ppc, Soft_emulate_8xx was used when full math emulation was
>> turned off to emulate a minimal subset of floating point load/store
>> instructions, to avoid needing a soft-float toolchain. This function
>> is called, but not present, on arch/powerpc, causing a build error
>> if floating point emulation is turned off.
>>
>> As soft-float toolchains are now common, I'm deleting the call rather
>> than moving Soft_emulate_8xx over to arch/powerpc.
>
>
> We should move the Soft_emulate_8xx code over. I see no reason to
> break a usage model that existed in arch/ppc.
According to the comment in softemu8xx.c, the only reason for it was
that "it was easier than trying to get the libraries compiled for
software floating point." Given that soft-float toolchains are easily
had now, and that full emulation could be used if one really needs to
work with hard-float binaries, I don't really see the need for this...
If you want to move it over and test it, though, be my guest. :-)
-Scott
^ permalink raw reply
* Re: [Bugme-new] [Bug 8778] New: Ocotea board: kernel reports access of bad area during boot with DEBUG_SLAB=y
From: Josh Boyer @ 2007-07-18 16:28 UTC (permalink / raw)
To: Eugene Surovegin
Cc: bart.vanassche, netdev, Andrew Morton,
bugme-daemon@kernel-bugs.osdl.org, linuxppc-embedded
In-Reply-To: <20070718155940.GB29722@gate.ebshome.net>
On Wed, 2007-07-18 at 08:59 -0700, Eugene Surovegin wrote:
> On Wed, Jul 18, 2007 at 08:41:10AM -0500, Josh Boyer wrote:
> > On Wed, 2007-07-18 at 01:34 -0700, Eugene Surovegin wrote:
> > > On Wed, Jul 18, 2007 at 12:52:53AM -0700, Andrew Morton wrote:
> > > > On Wed, 18 Jul 2007 00:07:50 -0700 (PDT) bugme-daemon@bugzilla.kernel.org wrote:
> > > >
> > > > > http://bugzilla.kernel.org/show_bug.cgi?id=8778
> > > > >
> > > > > Summary: Ocotea board: kernel reports access of bad area during
> > > > > boot with DEBUG_SLAB=y
> > >
> > > Slab debugging is probably the culprit here. I had similar problem
> > > couple of years ago, not sure something has changed since then,
> > > haven't checked.
> > >
> > > When slab debugging was enabled it made memory allocations non L1
> > > cache line aligned. This is very bad for DMA on non-coherent cache
> > > arches (PPC440 is one of those archs).
> > >
> > > I have a hack for EMAC which tries to "workaround" this problem:
> > > http://kernel.ebshome.net/emac_slab_debug.diff
> > > which might help.
> >
> > Would you be opposed to including that patch in mainline?
>
> Yes. I don't think it's the right way to fix this issue. IMO, the
> right one is to fix slab allocator. You cannot change all drivers to
> do this kind of cache flushing, and yes, I saw the same problem with
> PCI based NIC I tried on Ocotea at the time.
Hm... good point. I'd still like to see if your patch works around the
reporter's problem.
josh
^ permalink raw reply
* [PATCH 19/61] 8xx: Fix CONFIG_PIN_TLB.
From: Scott Wood @ 2007-07-18 16:29 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20070718013137.GA15217@ld0162-tx32.am.freescale.net>
1. Only map 512K of the IMMR, rather than 8M, to avoid conflicting with
the default ioremap region.
2. The wrong register was being loaded into SPRN_MD_RPN.
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
Apparently I missed sending this one yesterday...
arch/powerpc/kernel/head_8xx.S | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index 7488f30..c1469f3 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -701,7 +701,7 @@ initial_mmu:
mtspr SPRN_MI_AP, r8
mtspr SPRN_MD_AP, r8
- /* Map another 8 MByte at the IMMR to get the processor
+ /* Map another 512 KByte at the IMMR to get the processor
* internal registers (among other things).
*/
#ifdef CONFIG_PIN_TLB
@@ -714,7 +714,7 @@ initial_mmu:
mr r8, r9 /* Create vaddr for TLB */
ori r8, r8, MD_EVALID /* Mark it valid */
mtspr SPRN_MD_EPN, r8
- li r8, MD_PS8MEG /* Set 8M byte page */
+ li r8, MD_PS512K /* Set 512K byte page */
ori r8, r8, MD_SVALID /* Make it valid */
mtspr SPRN_MD_TWC, r8
mr r8, r9 /* Create paddr for TLB */
@@ -736,13 +736,13 @@ initial_mmu:
mtspr SPRN_MD_TWC, r9
li r11, MI_BOOTINIT /* Create RPN for address 0 */
addis r11, r11, 0x0080 /* Add 8M */
- mtspr SPRN_MD_RPN, r8
+ mtspr SPRN_MD_RPN, r11
addis r8, r8, 0x0080 /* Add 8M */
mtspr SPRN_MD_EPN, r8
mtspr SPRN_MD_TWC, r9
addis r11, r11, 0x0080 /* Add 8M */
- mtspr SPRN_MD_RPN, r8
+ mtspr SPRN_MD_RPN, r11
#endif
/* Since the cache is enabled according to the information we
--
1.5.0.3
^ permalink raw reply related
* Re: [PATCH 30/61] fsl_soc: Update the way get_brgfreq() finds things in the device tree.
From: Scott Wood @ 2007-07-18 16:32 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <8DC2DA02-3D10-4D14-AC91-E34AEA68E04F@kernel.crashing.org>
Kumar Gala wrote:
>> diff --git a/arch/powerpc/boot/dts/mpc8272ads.dts b/arch/powerpc/
>> boot/dts/mpc8272ads.dts
>> index 4d09dca..16a77f4 100644
>> --- a/arch/powerpc/boot/dts/mpc8272ads.dts
>> +++ b/arch/powerpc/boot/dts/mpc8272ads.dts
>> @@ -119,12 +119,11 @@
>> #address-cells = <1>;
>> #size-cells = <1>;
>> #interrupt-cells = <2>;
>> - device_type = "cpm";
>> - model = "CPM2";
>> + compatible = "fsl,mpc8272-cpm", "fsl,cpm2", "fsl,cpm";
>
> Does 'fsl,cpm' really mean anything useful?
Yes. It's can't be used on its own to show the complete programming
model, but there are lots of common things that it does indicate.
get_brgfreq() uses it to locate nodes which have an fsl,brg-frequency
property.
>> ranges = <00000000 00000000 20000>;
>> reg = <0 20000>;
>> command-proc = <119c0>;
>> - brg-frequency = <17D7840>;
>> + fsl,brg-frequency = <d#25000000>;
>
> Leave brg-frequency, and make a note about it being deprecated.
The CPM binding is changed in so many other ways that are much harder to
make backward compatible that I don't really see much point in doing so
here.
> Also, take a look at QE it has a similar concept.
It'd be nice to extend this binding to include QE (and at some point
down the road, merge the code)... I just didn't have time this time around.
-Scott
^ permalink raw reply
* Re: [PATCH 32/61] mpc82xx: Move PQ2 restart and halt functions out of mpc8272-specific code.
From: Scott Wood @ 2007-07-18 16:33 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <5ED3F9B1-87FF-4046-ADFD-E7805925489A@kernel.crashing.org>
Kumar Gala wrote:
> Kill pq2_halt, if this is all it does just dont set ppc_md.halt() and
> get the generic behavior which is the same.
OK.
>> diff --git a/include/asm-ppc/immap_cpm2.h b/include/asm-ppc/ immap_cpm2.h
>> index 3c23d9c..8795bcc 100644
>> --- a/include/asm-ppc/immap_cpm2.h
>> +++ b/include/asm-ppc/immap_cpm2.h
>> @@ -10,6 +10,8 @@
>> #ifndef __IMMAP_CPM2__
>> #define __IMMAP_CPM2__
>>
>> +#include <linux/types.h>
>> +
>
>
> Why was this needed all of a sudden?
It uses types without including any headers. It's always been needed,
we just got away with it before because the file including immap_cpm2.h
included other stuff earlier.
-Scott
^ permalink raw reply
* Re: [PATCH 52/61] cpm_uart: Issue STOP_TX command before initializing console.
From: Scott Wood @ 2007-07-18 16:35 UTC (permalink / raw)
To: Vitaly Bordug; +Cc: linuxppc-dev
In-Reply-To: <20070718120022.1b007937@localhost.localdomain>
Vitaly Bordug wrote:
>>+ cpm_line_cr_cmd(pinfo, CPM_CR_STOP_TX);
>>+
>
>
> I am recalling exactly the contrary patch that removes stuff to get the non-console UARTs work.
> Let's better revalidate this once other pieces will be in before applying.
I tried non-console UARTs and had no problem... How would this
interfere with them? Especially as we're only doing it in the console
init path...
-Scott
^ permalink raw reply
* Re: [PATCH 46/61] mpc885ads: Rework initialization.
From: Scott Wood @ 2007-07-18 16:40 UTC (permalink / raw)
To: Vitaly Bordug; +Cc: linuxppc-dev
In-Reply-To: <20070718124402.18197206@localhost.localdomain>
Vitaly Bordug wrote:
>>-#ifdef CONFIG_SERIAL_CPM_SMC1
>>- clrbits32(bcsr_io, BCSR1_RS232EN_1);
>>- clrbits32(&cp->cp_simode, 0xe0000000 >> 17); /* brg1
>>*/
>>- tmpval8 = in_8(&(cp->cp_smc[0].smc_smcm)) | (SMCM_RX |
>>SMCM_TX);
>>- out_8(&(cp->cp_smc[0].smc_smcm), tmpval8);
>>- clrbits16(&cp->cp_smc[0].smc_smcmr, SMCMR_REN |
>>SMCMR_TEN); /* brg1 */ -#else
>>- setbits32(bcsr_io,BCSR1_RS232EN_1);
>>- out_be16(&cp->cp_smc[0].smc_smcmr, 0);
>>- out_8(&cp->cp_smc[0].smc_smce, 0);
>>-#endif
>
>
> these are not just for beauty: if corresponding not-used uart regs are not cleared, second one has a
> good chances to be hosed.
The CPM reset should take care of that.
>>-void init_scc_ioports(struct fs_platform_info *fpi)
>>-{
>>- int scc_no = fs_get_scc_index(fpi->fs_no);
>>+ /* The SCC3 enet registers overlap the SMC1 registers, so
>>+ * one of the two must be removed from the device tree.
>>+ */
>
> Unfortunately,
> this approach has very little chances to work. IIRC, SCC3 eth and SMC1 do have overlapping pins,
> and just removing it from the tree is not going to save the world because now we have all-in-one early condensed
> pins setup.
It may not save the world, but it's needed to keep the driver from
poking at the disabled device.
> Also, we have to be very careful with corresponding ports shutdown: say turn off smc1 if scc3 is enabled: most prolly
> it was turned on by the firmware/bootwrapper, and eth won't be alive.
The conflict is in register space, not pins -- why would the smc1 pins
matter?
-Scott
^ permalink raw reply
* Re: [PATCH 38/61] cpm2: Update device trees.
From: Scott Wood @ 2007-07-18 16:42 UTC (permalink / raw)
To: Vitaly Bordug; +Cc: linuxppc-dev
In-Reply-To: <20070718162240.557eea46@localhost.localdomain>
Vitaly Bordug wrote:
> On Tue, 17 Jul 2007 20:35:50 -0500
> Scott Wood wrote:
>
>
>>+ "fsl,cpm-uart";
>> reg = <11a00 20 8000 100>;
>>- current-speed = <1c200>;
>
> Hmm, how is it supposed to work without speed? I was testing my u-boot OF bindings for 82xx and spotted into it.
The same way all the other device trees without current-speed work --
one passes console=whatever on the command line.
Feel free to have u-boot add the actual current speed (rather than a
hardcoded default speed) if you want, though.
-Scott
^ permalink raw reply
* Re: [RFC][PATCH 6/8] Walnut DTS
From: Scott Wood @ 2007-07-18 16:47 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: Yoder Stuart-B08248, linuxppc-dev
In-Reply-To: <A86AB8AE-E421-457B-ABE8-9AD93CCFCDC8@kernel.crashing.org>
Segher Boessenkool wrote:
> Yes indeed. The problem with your suggested "obvious way"
I said it was obvious, not obviously correct. :-)
> is that you wouldn't get a unit address included if your
> interrupt-map points (for some entry) at your device tree
> parent, either. Not all that hypothetical.
Ah, good point. My inclination would be to, rather than check how we
got to the node, check whether it's the device's parent. If not, then
the presence of #address-cells (other than zero for compatibility) is an
error. Otherwise, #address-cells is used, and defaults are handled the
same as with reg/ranges translation.
-Scott
^ permalink raw reply
* Re: [PATCH 09/61] bootwrapper: Add dt_is_compatible().
From: Scott Wood @ 2007-07-18 18:24 UTC (permalink / raw)
To: Milton Miller; +Cc: ppcdev, Paul Mackerras, David Gibson
In-Reply-To: <c5856df6a88b8124fbb6a3cc6974314c@bga.com>
Milton Miller wrote:
>> + for (pos = 0; pos + compat_len < len; pos++) {
>> + if (!strcmp(buf + pos, compat))
>> + return 1;
>> +
>> + while (buf[pos] && pos + compat_len < len)
>> + pos++;
>
>
> This is buggy: if you are searching for "ns16550" and the compatable is
> "fsl,1234\0commons16550" this code will incorrectly says its compatable.
How so?
The first iteration will compare "ns16550" to "fsl,1234", find that they
don't match, and advance to the '\0'.
The second iteration will compare "ns16550" to "commons16550", find that
they don't match, and advance to the 'n'.
The third iteration will not happen, since the loop condition fails, and
not-compatible is returned.
> Comparing pos < len instead will do the right thing, at the cost of a
> few iterations of the loop.
I'll grant that the extra iterations aren't worth doing the strlen() --
I'm not sure why I did it that way.
-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