LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 6/6] powerpc: Enable sparse irq_descs on powerpc
From: Grant Likely @ 2009-10-15  0:33 UTC (permalink / raw)
  To: michael; +Cc: linuxppc-dev
In-Reply-To: <1255564276.9651.14.camel@concordia>

On Wed, Oct 14, 2009 at 5:51 PM, Michael Ellerman
<michael@ellerman.id.au> wrote:
> On Wed, 2009-10-14 at 12:44 -0600, Grant Likely wrote:
>> Why not make sparse IRQs manditory for all platforms? =A0Is there a
>> performance concern with doing so? =A0From a maintenance perspective,
>> I'd rather see IRQ descs manged in one way only to keep the code
>> simple.
>
> I agree on the maintenance angle. My thinking was we'd run with it
> optional but default y for a release or two, and if no one complains we
> can make it mandatory.
>
> It does make some code paths bigger, and looking up an irq_desc is going
> to take slightly more cycles. I don't think it's a big issue, but I
> thought we should try it for a while before making it mandatory. The
> code has only been tested on x86 and sh as far as I know.

No guts, no glory.  I say throw it into linux-next to give it some
time before the next merge window.  I don't think you'll get any
better results by having it optional for a few releases (in fact, I
suspect that people who do have problems will just end up turning it
off and waiting for someone else to report/fix the problems).  If this
is the direction IRQ handling is going, then just make the change and
force any bugs to be dealt with before the next release.

> ps. thanks for the review

You're welcome.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH 6/6] powerpc: Enable sparse irq_descs on powerpc
From: Michael Ellerman @ 2009-10-14 23:51 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev
In-Reply-To: <fa686aa40910141144y1776c476uc8e26e06eb57e6ac@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1262 bytes --]

On Wed, 2009-10-14 at 12:44 -0600, Grant Likely wrote:
> On Tue, Oct 13, 2009 at 11:45 PM, Michael Ellerman
> <michael@ellerman.id.au> wrote:
> > Defining CONFIG_SPARSE_IRQ enables generic code that gets rid of the
> > static irq_desc array, and replaces it with an array of pointers to
> > irq_descs.
> >
> > It also allows node local allocation of irq_descs, however we
> > currently don't have the information available to do that, so we just
> > allocate them on all on node 0.
> >
> > Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> 
> Why not make sparse IRQs manditory for all platforms?  Is there a
> performance concern with doing so?  From a maintenance perspective,
> I'd rather see IRQ descs manged in one way only to keep the code
> simple.

I agree on the maintenance angle. My thinking was we'd run with it
optional but default y for a release or two, and if no one complains we
can make it mandatory.

It does make some code paths bigger, and looking up an irq_desc is going
to take slightly more cycles. I don't think it's a big issue, but I
thought we should try it for a while before making it mandatory. The
code has only been tested on x86 and sh as far as I know.

cheers

ps. thanks for the review

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCH 1/6] powerpc: Make NR_IRQS a CONFIG option
From: Michael Ellerman @ 2009-10-14 23:47 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev
In-Reply-To: <fa686aa40910141159l402304e9y22a6b2d7d8d79759@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1328 bytes --]

On Wed, 2009-10-14 at 12:59 -0600, Grant Likely wrote:
> On Tue, Oct 13, 2009 at 11:44 PM, Michael Ellerman
> <michael@ellerman.id.au> wrote:
> > The irq_desc array consumes quite a lot of space, and for systems
> > that don't need or can't have 512 irqs it's just wasted space.
> >
> > The first 16 are reserved for ISA, so the minimum of 32 is really
> > 16 - and no one has asked for more than 512 so leave that as the
> > maximum.
> 
> Does it really make sense to have this as a user twiddlable value?
> Especially when many users just don't have the background to know what
> an appropriate value is here and will get it wrong?  I believe your
> sparse IRQ patch has a bigger impact anyway on systems where memory is
> tight.

We have users? But yes I think it's reasonable, there's a million other
options people can fiddle with and break their kernel, I don't see that
this is much different.

The sparse IRQ patch has a bigger difference on the size of the irq_desc
array, but there are still other things that are statically sized based
on NR_IRQs. So if you're building an machine-specific kernel and you
know you're only going to have N interrupts, then this will give you a
bigger saving.

But I'm not super fussed, if other people think it's too dangerous we
can drop it.

cheers



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: linux-next: tree build failure
From: Hollis Blanchard @ 2009-10-14 22:57 UTC (permalink / raw)
  To: Jan Beulich
  Cc: sfr, Rusty Russell, linux-kernel, kvm-ppc, linux-next, akpm,
	linuxppc-dev
In-Reply-To: <1255115648.2546.71.camel@slab.beaverton.ibm.com>

On Fri, 2009-10-09 at 12:14 -0700, Hollis Blanchard wrote:
> Rusty's version of BUILD_BUG_ON() does indeed fix the build break, and
> also exposes the bug in kvmppc_account_exit_stat(). So to recap:
> 
> original: built but didn't work
> Jan's: doesn't build
> Rusty's: builds and works
> 
> Where do you want to go from here?

Jan, what are your thoughts? Your BUILD_BUG_ON patch has broken the
build, and we still need to fix it.

-- 
Hollis Blanchard
IBM Linux Technology Center

^ permalink raw reply

* Re: [PATCH 4/8] 8xx: Fixup DAR from buggy dcbX instructions.
From: Joakim Tjernlund @ 2009-10-14 22:09 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Scott Wood, linuxppc-dev@ozlabs.org, Rex Feany
In-Reply-To: <1255557130.2347.58.camel@pasglop>

Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 14/10/2009 23:52:10:
>
> On Wed, 2009-10-14 at 23:41 +0200, Joakim Tjernlund wrote:
> > Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 14/10/2009 23:17:09:
> > >
> > > On Wed, 2009-10-14 at 16:14 -0500, Scott Wood wrote:
> > > >
> > > > I think the last working version was a little older than that -- and it's quite
> > > > possible that there was underlying badness even earlier that just recently got
> > > > exposed.  I think I want to just debug it and find out what's really going on.
> > >
> > > That would be good :-)
> > >
> > > I've been itching to do that but without HW it's not trivial :-)
> >
> > Meanwhile, how about the tlb asm you promised me? :)
> > It will be a challenge I think since you only have 2 GPRs
> > I guess it would be possible to stash yet another reg since it
> > will fit in the cache line already used by the TLB handlers.
>
> Let's just get it working first :-)

Chicken :):)

^ permalink raw reply

* Re: [PATCH v2] net/fec_mpc52xx: Fix kernel panic on FEC error
From: David Miller @ 2009-10-14 22:10 UTC (permalink / raw)
  To: grant.likely; +Cc: bones, netdev, linuxppc-dev
In-Reply-To: <20091014174224.29221.18830.stgit@angua>

From: Grant Likely <grant.likely@secretlab.ca>
Date: Wed, 14 Oct 2009 11:43:43 -0600

> From: John Bonesio <bones@secretlab.ca>
> 
> The MDIO bus cannot be accessed at interrupt context, but on an FEC
> error, the fec_mpc52xx driver reset function also tries to reset the
> PHY.  Since the error is detected at IRQ context, and the PHY functions
> try to sleep, the kernel ends up panicking.
> 
> Resetting the PHY on an FEC error isn't even necessary.  This patch
> solves the problem by removing the PHY reset entirely.
> 
> Signed-off-by: John Bonesio <bones@secretlab.ca>
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] net: Fix OF platform drivers coldplug/hotplug when compiled as modules
From: David Miller @ 2009-10-14 21:54 UTC (permalink / raw)
  To: avorontsov; +Cc: linuxppc-dev, netdev
In-Reply-To: <20091008121512.GA2390@oksana.dev.rtsoft.ru>

From: Anton Vorontsov <avorontsov@ru.mvista.com>
Date: Thu, 8 Oct 2009 16:15:12 +0400

> Some OF platform drivers are missing module device tables, so they won't
> load automatically on boot. This patch fixes the issue by adding proper
> MODULE_DEVICE_TABLE() macros to the drivers.
> 
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>

Applied, thanks!

^ permalink raw reply

* [PATCH] hvc_console: returning 0 from put_chars is not an error
From: Timur Tabi @ 2009-10-14 21:53 UTC (permalink / raw)
  To: linuxppc-dev, scottwood, borntraeger, brueckner

hvc_console_print() calls the HVC client driver's put_chars() callback
to write some characters to the console.  If the callback returns 0, that
indicates that no characters were written (perhaps the output buffer is
full), but hvc_console_print() treats that as an error and discards the
rest of the buffer.

So change hvc_console_print() to just loop and call put_chars() again if it
returns a 0 return code.

This change makes hvc_console_print() behave more like hvc_push(), which does
check for a 0 return code and re-schedules itself.

Signed-off-by: Timur Tabi <timur@freescale.com>
---

This patch might cause a hang in drivers that return 0 in case of error, 
instead of a negative number, but those drivers are broken anyway.  This 
patch might fix drivers that return 0 to indicate that they're busy, such as
arch/powerpc/platforms/pseries/hvconsole.c.  It will break drivers that
return 0 if their output buffer is full, but where those buffers cannot be
emptied while the kernel is in a loop.

 drivers/char/hvc_console.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/char/hvc_console.c b/drivers/char/hvc_console.c
index 25ce15b..0c94907 100644
--- a/drivers/char/hvc_console.c
+++ b/drivers/char/hvc_console.c
@@ -161,7 +161,7 @@ static void hvc_console_print(struct console *co, const char *b,
 			}
 		} else {
 			r = cons_ops[index]->put_chars(vtermnos[index], c, i);
-			if (r <= 0) {
+			if (r < 0) {
 				/* throw away chars on error */
 				i = 0;
 			} else if (r > 0) {
-- 
1.6.5

^ permalink raw reply related

* Re: [PATCH 4/8] 8xx: Fixup DAR from buggy dcbX instructions.
From: Benjamin Herrenschmidt @ 2009-10-14 21:52 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: Scott Wood, linuxppc-dev@ozlabs.org, Rex Feany
In-Reply-To: <OFDAF14BF7.E3D4EBC9-ONC125764F.0076DF84-C125764F.00772817@transmode.se>

On Wed, 2009-10-14 at 23:41 +0200, Joakim Tjernlund wrote:
> Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 14/10/2009 23:17:09:
> >
> > On Wed, 2009-10-14 at 16:14 -0500, Scott Wood wrote:
> > >
> > > I think the last working version was a little older than that -- and it's quite
> > > possible that there was underlying badness even earlier that just recently got
> > > exposed.  I think I want to just debug it and find out what's really going on.
> >
> > That would be good :-)
> >
> > I've been itching to do that but without HW it's not trivial :-)
> 
> Meanwhile, how about the tlb asm you promised me? :)
> It will be a challenge I think since you only have 2 GPRs
> I guess it would be possible to stash yet another reg since it
> will fit in the cache line already used by the TLB handlers.

Let's just get it working first :-)

Ben.

^ permalink raw reply

* Re: [PATCH 4/8] 8xx: Fixup DAR from buggy dcbX instructions.
From: Joakim Tjernlund @ 2009-10-14 21:41 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Scott Wood, linuxppc-dev@ozlabs.org, Rex Feany
In-Reply-To: <1255555029.2347.55.camel@pasglop>

Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 14/10/2009 23:17:09:
>
> On Wed, 2009-10-14 at 16:14 -0500, Scott Wood wrote:
> >
> > I think the last working version was a little older than that -- and it's quite
> > possible that there was underlying badness even earlier that just recently got
> > exposed.  I think I want to just debug it and find out what's really going on.
>
> That would be good :-)
>
> I've been itching to do that but without HW it's not trivial :-)

Meanwhile, how about the tlb asm you promised me? :)
It will be a challenge I think since you only have 2 GPRs
I guess it would be possible to stash yet another reg since it
will fit in the cache line already used by the TLB handlers.

 Jocke

^ permalink raw reply

* Re: Support for S29JL064 in MPC8272ADS?
From: Scott Wood @ 2009-10-14 21:40 UTC (permalink / raw)
  To: Roberto Guerra; +Cc: linuxppc-dev
In-Reply-To: <7c4144600910141434y26972dbbnb776c4ef89d15a17@mail.gmail.com>

Roberto Guerra wrote:
> I've been learning how to modify the dts from
> http://www.mjmwired.net/kernel/Documentation/powerpc/dts-bindings/mtd-physmap.txt#49
> The original mpc8272ads.dts represents four 8-bit JEDEC Sharp flash
> chips in 1 SIMM module:
> [snip]        localbus@f0010100 {
>                 compatible = "fsl,mpc8280-localbus",
>                              "fsl,pq2-localbus";
>                 #address-cells = <2>;
>                 #size-cells = <1>;
>                 reg = <f0010100 60>;
> 
>                 ranges = <0 0 fe000000 00800000
>                           1 0 f4500000 00008000
>                           8 0 f8200000 00008000>;
> 
>                 flash@0,0 {
>                         compatible = "jedec-flash";
>                         reg = <0 0 800000>;
>                         bank-width = <4>;
>                         device-width = <1>;
>                 };
> [snip]
> My board (based on the PQ2FADS, using the MPC8272ADS BSP)

Don't base anything on the BSPs, unless there's something in them that you 
really need that isn't upstream.  There is pq2fads support in current upstream 
kernels.

> uses one
> 16-bit Spansion (AMD) CFI chip at addresses FF800000 through FFFFFFFF.
> It probably needs to be represented this way (I've only made changes
> to the "flash" section.
> [snip]
>                 flash@0,0 {
>                         compatible = "amd, s29jl064h", "cfi-flash";
>                         reg = <0 0 800000>;
>                         bank-width = <2>;
>                         device-width = <2>;
>                 };
> [snip]
> However, I don't know what would be the correct addresses to type
> after "localbus", "flash" and "reg". Is this enough information to
> define my dts?

The flash node looks good, other than that there shouldn't be a space after "amd,".

In the localbus node, change fe000000 to ff800000.  Remove or change the other 
ranges entries if they don't describe your board's chipselects.

If your IMMR is somewhere other than 0xf0000000, update the f0010100 to match.

-Scott

^ permalink raw reply

* Re: Support for S29JL064 in MPC8272ADS?
From: Roberto Guerra @ 2009-10-14 21:34 UTC (permalink / raw)
  To: linuxppc-dev, Scott Wood
In-Reply-To: <20091009181617.GA14304@loki.buserror.net>

On Fri, Oct 9, 2009 at 2:16 PM, Scott Wood <scottwood@freescale.com> wrote:
> On Fri, Oct 09, 2009 at 01:59:58PM -0400, Roberto Guerra wrote:
>> No. I did not. My FDT was simplified from the stock MPC8272ADS:
>> =3D> fdt list
>> / {
>> =A0 =A0 =A0 =A0 model =3D "pq2fads";
>> =A0 =A0 =A0 =A0 compatible =3D "fsl,pq2fads";
>> =A0 =A0 =A0 =A0 #address-cells =3D <0x00000001>;
>> =A0 =A0 =A0 =A0 #size-cells =3D <0x00000001>;
>> =A0 =A0 =A0 =A0 cpus {
>> =A0 =A0 =A0 =A0 };
>> =A0 =A0 =A0 =A0 memory {
>> =A0 =A0 =A0 =A0 };
>> =A0 =A0 =A0 =A0 soc@f0000000 {
>> =A0 =A0 =A0 =A0 };
>> =A0 =A0 =A0 =A0 chosen {
>> =A0 =A0 =A0 =A0 };
>> };
>
> You need more than that. =A0What happened to all the content of those nod=
es?
>
>> I am searching how I could add the mtd branch between the "soc" and
>> the "chosen".
>
> Look at the localbus node on the mpc8272ads dts.
>
> Look at (and use) a recent upstream kernel, if you're not already.
>
> -Scott
>

I've been learning how to modify the dts from
http://www.mjmwired.net/kernel/Documentation/powerpc/dts-bindings/mtd-physm=
ap.txt#49
The original mpc8272ads.dts represents four 8-bit JEDEC Sharp flash
chips in 1 SIMM module:
[snip]        localbus@f0010100 {
                compatible =3D "fsl,mpc8280-localbus",
                             "fsl,pq2-localbus";
                #address-cells =3D <2>;
                #size-cells =3D <1>;
                reg =3D <f0010100 60>;

                ranges =3D <0 0 fe000000 00800000
                          1 0 f4500000 00008000
                          8 0 f8200000 00008000>;

                flash@0,0 {
                        compatible =3D "jedec-flash";
                        reg =3D <0 0 800000>;
                        bank-width =3D <4>;
                        device-width =3D <1>;
                };
[snip]
My board (based on the PQ2FADS, using the MPC8272ADS BSP) uses one
16-bit Spansion (AMD) CFI chip at addresses FF800000 through FFFFFFFF.
It probably needs to be represented this way (I've only made changes
to the "flash" section.
[snip]
                flash@0,0 {
                        compatible =3D "amd, s29jl064h", "cfi-flash";
                        reg =3D <0 0 800000>;
                        bank-width =3D <2>;
                        device-width =3D <2>;
                };
[snip]
However, I don't know what would be the correct addresses to type
after "localbus", "flash" and "reg". Is this enough information to
define my dts?
Thanks,
Roberto

^ permalink raw reply

* Re: [PATCH] powerpc: Prevent unsigned wrap in htab_dt_scan_page_sizes()
From: Paul Mackerras @ 2009-10-14 21:29 UTC (permalink / raw)
  To: Roel Kluin; +Cc: Andrew Morton, linuxppc-dev
In-Reply-To: <4AD5C01E.8000600@gmail.com>

Roel Kluin writes:

> Check to prevent unsigned wrap of size before subtraction.
> 
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> Is this maybe better or are we certain that size can't wrap?

Patch looks good, though while you're at it, you could add a space
after the "while".

Acked-by: Paul Mackerras <paulus@samba.org>

^ permalink raw reply

* Re: i2c-powermac fails
From: Benjamin Herrenschmidt @ 2009-10-14 21:26 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Paul Mackerras, Tim Shepard, linuxppc-dev
In-Reply-To: <20091014230252.0d0cba8d@hyperion.delvare>

On Wed, 2009-10-14 at 23:02 +0200, Jean Delvare wrote:
> Hi all,
> 
> On Tue, 13 Oct 2009 11:49:48 +0200, Jean Delvare wrote:
> > I2C bus being setup too fast sounds more likely. It might be worth
> > adding an arbitrary delay after initialization, just to see if it
> > helps. Not sure where though, as I'm not familiar with the Powermac
> > initialization steps. Maybe right before i2c_add_adapter() in
> > i2c_powermac_probe?
> 
> Tim, can you please give a try to this patch? Obviously your machine
> will take 5 additional seconds to boot, and this isn't meant as a real
> fix, but if it helps, this will be an interesting hint for further
> debugging attempts.

Oh, I was actually thinking about the frequency of the I2C clock :-)

Cheers,
Ben.

> --- kernel32.orig/drivers/macintosh/therm_adt746x.c
> +++ kernel32/drivers/macintosh/therm_adt746x.c
> @@ -380,6 +380,7 @@ static int probe_thermostat(struct i2c_c
>  	if (thermostat)
>  		return 0;
>  
> +	msleep(5000);
>  	th = kzalloc(sizeof(struct thermostat), GFP_KERNEL);
>  	if (!th)
>  		return -ENOMEM;
> 
> 

^ permalink raw reply

* Re: [PATCH 4/8] 8xx: Fixup DAR from buggy dcbX instructions.
From: Benjamin Herrenschmidt @ 2009-10-14 21:17 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev@ozlabs.org, Rex Feany
In-Reply-To: <4AD63F48.8030301@freescale.com>

On Wed, 2009-10-14 at 16:14 -0500, Scott Wood wrote:
> 
> I think the last working version was a little older than that -- and it's quite 
> possible that there was underlying badness even earlier that just recently got 
> exposed.  I think I want to just debug it and find out what's really going on.

That would be good :-)

I've been itching to do that but without HW it's not trivial :-)

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH 4/8] 8xx: Fixup DAR from buggy dcbX instructions.
From: Scott Wood @ 2009-10-14 21:14 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: Rex Feany, linuxppc-dev@ozlabs.org
In-Reply-To: <OF459A0F65.D3201748-ONC125764F.00738958-C125764F.00744CF0@transmode.se>

Joakim Tjernlund wrote:
>> With that, I don't see the hard lockup, but things get stuck during
> 
> You needed both to loose the hard lockup? I would think
> it should be enough to revert the "various copy routines" stuff?

No, but when I just reverted the patch and didn't change the TLB error handler, 
I got some other weirdness (assertion failure in some userspace program).  It 
may have been coincidental, though.

>> I think there's something else going on in the 2.6 8xx code that needs
>> to be fixed before we can tell what the impact of these patches is.
>> I'll look into it.
> 
> Great because I am really out of ideas. Perhaps back down to 2.6.30 and test
> from there?

I think the last working version was a little older than that -- and it's quite 
possible that there was underlying badness even earlier that just recently got 
exposed.  I think I want to just debug it and find out what's really going on.

-Scott

^ permalink raw reply

* Re: [PATCH 4/8] 8xx: Fixup DAR from buggy dcbX instructions.
From: Joakim Tjernlund @ 2009-10-14 21:10 UTC (permalink / raw)
  To: Scott Wood; +Cc: Rex Feany, linuxppc-dev@ozlabs.org
In-Reply-To: <4AD63301.6090307@freescale.com>


Scott Wood <scottwood@freescale.com> wrote on 14/10/2009 22:22:25:
>
> Joakim Tjernlund wrote:
> > Scott Wood <scottwood@freescale.com> wrote on 14/10/2009 21:23:02:
> >> Joakim Tjernlund wrote:
> >>> BTW, you could add a test and printk in do_page_fault on address 0x000000f0.
> >>> if that ever hits there is a problem with dcbX fixup.
> >> It doesn't get any 0xf0 faults.
> >>
> >> FWIW, I'm not seeing the segfault any more, but I still get the lockup.
> >
> > Have you reverted
> >  8xx: start using dcbX instructions in various copy routines ?
> >
> > After that you could stick a
> >  b DataAccess
> >
> >  directly in the DTLB error handler to skip and dcbX fixups.
>
> With that, I don't see the hard lockup, but things get stuck during

You needed both to loose the hard lockup? I would think
it should be enough to revert the "various copy routines" stuff?
I figure that these routines aren't working in 8xx for other reasons
since they haven't been used on 8xx since at least early 2.4.

> bootup with everything idle.  I see this even if I revert everything but
> the "invalidate non present TLBs" patch, and I was seeing similar things
> sometimes with the other tlbil_va hacks.

OK, something else is up.

>
> I think there's something else going on in the 2.6 8xx code that needs
> to be fixed before we can tell what the impact of these patches is.
> I'll look into it.

Great because I am really out of ideas. Perhaps back down to 2.6.30 and test
from there?

^ permalink raw reply

* Re: i2c-powermac fails
From: Jean Delvare @ 2009-10-14 21:02 UTC (permalink / raw)
  To: Tim Shepard; +Cc: Paul Mackerras, linuxppc-dev
In-Reply-To: <20091013114948.03608b7f@hyperion.delvare>

Hi all,

On Tue, 13 Oct 2009 11:49:48 +0200, Jean Delvare wrote:
> I2C bus being setup too fast sounds more likely. It might be worth
> adding an arbitrary delay after initialization, just to see if it
> helps. Not sure where though, as I'm not familiar with the Powermac
> initialization steps. Maybe right before i2c_add_adapter() in
> i2c_powermac_probe?

Tim, can you please give a try to this patch? Obviously your machine
will take 5 additional seconds to boot, and this isn't meant as a real
fix, but if it helps, this will be an interesting hint for further
debugging attempts.

--- kernel32.orig/drivers/macintosh/therm_adt746x.c
+++ kernel32/drivers/macintosh/therm_adt746x.c
@@ -380,6 +380,7 @@ static int probe_thermostat(struct i2c_c
 	if (thermostat)
 		return 0;
 
+	msleep(5000);
 	th = kzalloc(sizeof(struct thermostat), GFP_KERNEL);
 	if (!th)
 		return -ENOMEM;


-- 
Jean Delvare

^ permalink raw reply

* Re: [PATCH 4/8] 8xx: Fixup DAR from buggy dcbX instructions.
From: Scott Wood @ 2009-10-14 20:22 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: Rex Feany, linuxppc-dev@ozlabs.org
In-Reply-To: <OF22DCA2B2.1ADC3BC0-ONC125764F.006DE50F-C125764F.006E305A@transmode.se>

Joakim Tjernlund wrote:
> Scott Wood <scottwood@freescale.com> wrote on 14/10/2009 21:23:02:
>> Joakim Tjernlund wrote:
>>> BTW, you could add a test and printk in do_page_fault on address 0x000000f0.
>>> if that ever hits there is a problem with dcbX fixup.
>> It doesn't get any 0xf0 faults.
>>
>> FWIW, I'm not seeing the segfault any more, but I still get the lockup.
> 
> Have you reverted
>  8xx: start using dcbX instructions in various copy routines ?
> 
> After that you could stick a
>  b DataAccess
> 
>  directly in the DTLB error handler to skip and dcbX fixups.

With that, I don't see the hard lockup, but things get stuck during 
bootup with everything idle.  I see this even if I revert everything but 
the "invalidate non present TLBs" patch, and I was seeing similar things 
sometimes with the other tlbil_va hacks.

I think there's something else going on in the 2.6 8xx code that needs 
to be fixed before we can tell what the impact of these patches is. 
I'll look into it.

-Scott

^ permalink raw reply

* Re: UBIFS problem on MPC8536DS
From: Felix Radensky @ 2009-10-14 20:12 UTC (permalink / raw)
  To: Scott Wood
  Cc: linuxppc-dev@ozlabs.org, linux-mtd@lists.infradead.org,
	Adrian Hunter
In-Reply-To: <4AD614E4.6030005@freescale.com>

Hi, Scott

Scott Wood wrote:
> Felix Radensky wrote:
>> Yes, NAND and NOR are on the same local bus controller.
>>
>> Maybe powerpc folks can provide some insight here.
>> Is it possible that simultaneous access to NOR and NAND
>> on MPC8536 can result in NAND timeouts ?
>
> I've heard other reports of such problems with eLBC, but was unable to 
> reproduce it myself last time I tried.  Could you reduce this down to 
> a minimal set of specific reproduction instructions (e.g. eliminate 
> UBI if possible, or else explain how to set up UBI)?
>
> -Scott
>
I use u-boot-2009.08 and linux-2.6.31 compiled with ELDK 4.2
UBI (under Device Drivers->MTD->UBI) and UBIFS
(under File Systems->Miscellaneous filesystems) should be enabled
in kernel configuration. I use the following configuration options 
(default ones):

CONFIG_MTD_UBI=y
CONFIG_MTD_UBI_WL_THRESHOLD=4096
CONFIG_MTD_UBI_BEB_RESERVE=1
# CONFIG_MTD_UBI_GLUEBI is not set
# UBI debugging options
# CONFIG_MTD_UBI_DEBUG is not set
# CONFIG_JFFS2_RUBIN is not set
CONFIG_UBIFS_FS=y
# CONFIG_UBIFS_FS_XATTR is not set
# CONFIG_UBIFS_FS_ADVANCED_COMPR is not set
CONFIG_UBIFS_FS_LZO=y
CONFIG_UBIFS_FS_ZLIB=y
# CONFIG_UBIFS_FS_DEBUG is not set

Root file system is mounted over NFS (also ELDK 4.2). udev is not used.
mtd-utils (which include ubi tools) are from latest 
git://git.infradead.org/mtd-utils.git

I've added the following eLBC node to mpc8536ds.dts

    localbus@ffe05000 {
        #address-cells = <2>;
        #size-cells = <1>;
        compatible = "fsl,mpc8536-elbc", "fsl,elbc", "simple-bus";
        reg = <0xffe05000 0x1000>;
        interrupts = <19 2>;
        interrupt-parent = <&mpic>;

        ranges = <0x0 0x0 0xe8000000 0x08000000
              0x1 0x0 0xe8000000 0x08000000
              0x2 0x0 0xffa00000 0x00040000
              0x3 0x0 0xffdf0000 0x00008000
              0x4 0x0 0xffa40000 0x00040000
              0x5 0x0 0xffa80000 0x00040000
              0x6 0x0 0xffac0000 0x00040000>;

        nor@0,0 {
            #address-cells = <1>;
            #size-cells = <1>;
            compatible = "cfi-flash";
            reg = <0x0 0x0 0x8000000>;
            bank-width = <2>;
            device-width = <1>;

            partition@0 {
                label = "ramdisk";
                reg = <0x0 0x03000000>;
                read-only;
            };

            partition@3000000 {
                label = "diagnostics";
                reg = <0x03000000 0x00e00000>;
                read-only;
            };

            partition@3e00000 {
                label = "dink";
                reg = <0x03e00000 0x00200000>;
                read-only;
            };

            partition@4000000 {
                label = "kernel";
                reg = <0x04000000 0x00400000>;
                read-only;
            };

            partition@4400000 {
                label = "jffs2";
                reg = <0x04400000 0x03b00000>;
            };

            partition@7f00000 {
                label = "dtb";
                reg = <0x07f00000 0x00080000>;
                read-only;
            };

            partition@7f80000 {
                label = "u-boot";
                reg = <0x07f80000 0x00080000>;
                read-only;
            };
        };

        nand@2,0 {
            compatible = "fsl,mpc8536-fcm-nand",
                     "fsl,elbc-fcm-nand";
            reg = <0x2 0x0 0x40000>;
        };

        nand@4,0 {
            compatible = "fsl,mpc8536-fcm-nand",
                     "fsl,elbc-fcm-nand";
            reg = <0x4 0x0 0x40000>;
        };

        nand@5,0 {
            compatible = "fsl,mpc8536-fcm-nand",
                     "fsl,elbc-fcm-nand";
            reg = <0x5 0x0 0x40000>;
        };

        nand@6,0 {
            compatible = "fsl,mpc8536-fcm-nand",
                     "fsl,elbc-fcm-nand";
            reg = <0x6 0x0 0x40000>;
        };

        board-control@3,0 {
            compatible = "fsl,mpc8536ds-fpga-pixis";
            reg = <0x3 0x0 0x8000>;
        };
    };

I use the script below to reproduce the problem. rfs.tar.bz2 is 350MiB.

nand_dev=mtd7
nand_num=7
nor_dev=mtd4

flash_eraseall /dev/$nand_dev || exit 1
ubiformat /dev/$nand_dev || exit 1
ubictrl_major=`cat /sys/class/misc/ubi_ctrl/dev | cut -d: -f1`
ubictrl_minor=`cat /sys/class/misc/ubi_ctrl/dev | cut -d: -f2`
rm -f /dev/ubi_ctrl
mknod /dev/ubi_ctrl c $ubictrl_major $ubictrl_minor
ubiattach /dev/ubi_ctrl -m $nand_num  || exit 1
ubi0_major=`cat /sys/class/ubi/ubi0/dev | cut -d: -f1`
ubi0_minor=`cat /sys/class/ubi/ubi0/dev | cut -d: -f2`
rm -f /dev/ubi0
mknod /dev/ubi0 c $ubi0_major $ubi0_minor
ubimkvol /dev/ubi0 -N rootfs -s 800MiB  || exit 1
mkdir /mnt/rfs
mount -t ubifs ubi0:rootfs /mnt/rfs  || exit 1
tar xf /root/rfs.tar.bz2 -C /mnt/rfs
flash_eraseall /dev/$nor_dev || exit 1

Felix.

^ permalink raw reply

* Re: [PATCH 4/8] 8xx: Fixup DAR from buggy dcbX instructions.
From: Joakim Tjernlund @ 2009-10-14 20:03 UTC (permalink / raw)
  To: Scott Wood; +Cc: Rex Feany, linuxppc-dev@ozlabs.org
In-Reply-To: <4AD62516.6090301@freescale.com>

Scott Wood <scottwood@freescale.com> wrote on 14/10/2009 21:23:02:
> Joakim Tjernlund wrote:
> > BTW, you could add a test and printk in do_page_fault on address 0x000000f0.
> > if that ever hits there is a problem with dcbX fixup.
>
> It doesn't get any 0xf0 faults.
>
> FWIW, I'm not seeing the segfault any more, but I still get the lockup.

Have you reverted
 8xx: start using dcbX instructions in various copy routines ?

After that you could stick a
 b DataAccess

 directly in the DTLB error handler to skip and dcbX fixups.

^ permalink raw reply

* Re: [PATCH v2] net/fec_mpc52xx: Fix kernel panic on FEC error
From: Grant Likely @ 2009-10-14 19:32 UTC (permalink / raw)
  To: Wolfgang Grandegger; +Cc: John Bonesio, netdev, linuxppc-dev, davem
In-Reply-To: <4AD62247.4020102@grandegger.com>

On Wed, Oct 14, 2009 at 1:11 PM, Wolfgang Grandegger <wg@grandegger.com> wr=
ote:
> Grant Likely wrote:
>> From: John Bonesio <bones@secretlab.ca>
>>
>> The MDIO bus cannot be accessed at interrupt context, but on an FEC
>> error, the fec_mpc52xx driver reset function also tries to reset the
>> PHY. =A0Since the error is detected at IRQ context, and the PHY function=
s
>> try to sleep, the kernel ends up panicking.
>>
>> Resetting the PHY on an FEC error isn't even necessary. =A0This patch
>> solves the problem by removing the PHY reset entirely.
>
> There is also no need to free and re-allocate the RX buffers in
> mpc52xx_fec_reset().

Write and test a patch for me!  :-)

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH] therm_adt746x: Don't access non-existing register
From: Tim Shepard @ 2009-10-14 18:59 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Colin Leroy, linuxppc-dev, Paul Mackerras
In-Reply-To: <20091014173132.2894d0fd@hyperion.delvare>


Jean,

I tried one reboot with your patch, failed as usual.
(Patch sounds like a good idea though.)

			-Tim Shepard
			 shep@alum.mit.edu



tree<11>$ git describe
v2.6.31.1-6-g0b193bc
tree<12>$ git diff HEAD^ HEAD
diff --git a/drivers/macintosh/therm_adt746x.c b/drivers/macintosh/therm_adt746x.c
index 0554dae..bf9ed2d 100644
--- a/drivers/macintosh/therm_adt746x.c
+++ b/drivers/macintosh/therm_adt746x.c
@@ -366,7 +366,7 @@ static int probe_thermostat(struct i2c_client *client,
        i2c_set_clientdata(client, th);
        th->clt = client;
 
-       rc = read_reg(th, 0);
+       rc = read_reg(th, CONFIG_REG);
        if (rc < 0) {
                dev_err(&client->dev, "Thermostat failed to read config!\n");
                kfree(th);
tree<13>$ dmesg | head
Using PowerMac machine description
Total memory = 1536MB; using 4096kB for hash table (at cfc00000)
Linux version 2.6.31.1-00006-g0b193bc (shep@tree) (gcc version 4.3.2 (Debian 4.3.2-1.1) ) #9 Wed Oct 14 13:41:40 EDT 2009
Found UniNorth memory controller & host bridge @ 0xf8000000 revision: 0xd9
Mapped at 0xff7c0000
Found a Intrepid mac-io controller, rev: 0, mapped at 0xff740000
Processor NAP mode on idle enabled.
PowerMac motherboard: PowerBook G4 15"
console [udbg0] enabled
Found UniNorth PCI host bridge at 0x00000000f0000000. Firmware bus number: 0->1
tree<14>$ dmesg | grep adt
adt746x: version 1 (supported)
adt746x: Thermostat bus: 0, address: 0x2e, limit_adjust: 0, fan_speed: -1
therm_adt746x 7-002e: Thermostat failed to read config!
tree<15>$ 

^ permalink raw reply related

* Re: [PATCH 4/8] 8xx: Fixup DAR from buggy dcbX instructions.
From: Scott Wood @ 2009-10-14 19:23 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: Rex Feany, linuxppc-dev@ozlabs.org
In-Reply-To: <OF64D89B73.6073827F-ONC125764F.00687BC4-C125764F.0068D4DC@transmode.se>

Joakim Tjernlund wrote:
> BTW, you could add a test and printk in do_page_fault on address 0x000000f0.
> if that ever hits there is a problem with dcbX fixup.

It doesn't get any 0xf0 faults.

FWIW, I'm not seeing the segfault any more, but I still get the lockup.

-Scott

^ permalink raw reply

* Re: [PATCH v2] net/fec_mpc52xx: Fix kernel panic on FEC error
From: Wolfgang Grandegger @ 2009-10-14 19:11 UTC (permalink / raw)
  To: Grant Likely; +Cc: John Bonesio, netdev, linuxppc-dev, davem
In-Reply-To: <20091014174224.29221.18830.stgit@angua>

Grant Likely wrote:
> From: John Bonesio <bones@secretlab.ca>
> 
> The MDIO bus cannot be accessed at interrupt context, but on an FEC
> error, the fec_mpc52xx driver reset function also tries to reset the
> PHY.  Since the error is detected at IRQ context, and the PHY functions
> try to sleep, the kernel ends up panicking.
> 
> Resetting the PHY on an FEC error isn't even necessary.  This patch
> solves the problem by removing the PHY reset entirely.

There is also no need to free and re-allocate the RX buffers in
mpc52xx_fec_reset().

Wolfgang.

^ permalink raw reply


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