LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [RFC] [PATCH] allow low HZ values?
From: Thomas Gleixner @ 2010-10-11 22:33 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Marcio Saito, Jiri Slaby, John Stultz, x86, linux-kernel,
	Tim Pepper, Ingo Molnar, Paul Mackerras, H. Peter Anvin,
	Avantika Mathur, linuxppc-dev
In-Reply-To: <1286836212.5220.19.camel@pasglop>

On Tue, 12 Oct 2010, Benjamin Herrenschmidt wrote:

> On Mon, 2010-10-11 at 13:11 -0700, Tim Pepper wrote:
> > I'm not necessarily wanting to open up the age old question of "what is
> > a good HZ", but we were doing some testing on timer tick overheads for
> > HPC applications and this came up...
> 
> Note that this is also very useful when working on CPU prototypes
> implemented in FPGAs and running at something like 12Mhz :-)

/me hands benh 0.5$ for a FPGA upgrade

^ permalink raw reply

* Re: [RFC] [PATCH] allow low HZ values?
From: Benjamin Herrenschmidt @ 2010-10-11 22:30 UTC (permalink / raw)
  To: Tim Pepper
  Cc: Marcio Saito, Jiri Slaby, John Stultz, x86, linux-kernel,
	Ingo Molnar, Paul Mackerras, H. Peter Anvin, Thomas Gleixner,
	linuxppc-dev, Avantika Mathur
In-Reply-To: <20101011201121.GA953@tpepper-t61p.dolavim.us>

On Mon, 2010-10-11 at 13:11 -0700, Tim Pepper wrote:
> I'm not necessarily wanting to open up the age old question of "what is
> a good HZ", but we were doing some testing on timer tick overheads for
> HPC applications and this came up...

Note that this is also very useful when working on CPU prototypes
implemented in FPGAs and running at something like 12Mhz :-)

Cheers,
Ben.

> Below is a minimal hack at enabling lower HZ values.  The kernel builds
> and boots for me on x86_64 (simple laptop and kvm configs) and ppc64
> (misc. IBM System p) with each of the added HZ options.
> 
> There's explicit code checking HZ down to 12, but HZ<100 wasn't a config
> option.  We collected some data at 10, 12 and 25.  There'd been some
> question of whether 10 would even work or not but it looks fine in the
> relatively minimal testing we did.  We tried 12 since the code seemed
> to allow for it.  And 25 as a "safe" lower value.  The only difference
> observed under load (ie: no no idle HZ in play) was the expected timer
> tick happening less often.  There was definitely surprise that nothing
> else seemed to break anywhere, especially at 10.
> 
> Do people feel it is reasonable to have Kconfig bits to allow some lower
> HZ values?
> 
> If so, then there's the question of what breaks.  It's reasonable to
> think there are other going to be subtleties buried in code around
> assumptions on the likely range of HZ:
> 
> - I'm not sure that what I did in inet_timewait_sock.h and jiffies.h is
>   reasonable.
> - arch/x86/kernel/i8253.c throws a warning at line 43 (v2.6.36-rc7):
>       warning: large integer implicitly truncated to unsigned type
> - drivers/char/cyclades.c's cy_ioctl() warns:
>       drivers/char/cyclades.c:2761: warning: division by zero
> - drivers, drivers, drivers across all the arch's could use sanity checking
> 

^ permalink raw reply

* Re: [RFC] [PATCH] allow low HZ values?
From: Tim Pepper @ 2010-10-11 21:11 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Marcio Saito, John Stultz, Jiri Slaby, Peter Zijlstra, x86, LKML,
	Tim Pepper, Frederic Weisbecker, Ingo Molnar, Paul Mackerras,
	H. Peter Anvin, Avantika Mathur, linuxppc-dev
In-Reply-To: <alpine.LFD.2.00.1010112227190.2909@localhost6.localdomain6>

On Mon 11 Oct at 22:32:06 +0200 tglx@linutronix.de said:
> On Mon, 11 Oct 2010, Tim Pepper wrote:
> 
> > I'm not necessarily wanting to open up the age old question of "what is
> > a good HZ", but we were doing some testing on timer tick overheads for
> > HPC applications and this came up...
> 
> Yeah. This comes always up when the timer tick overhead on HPC is
> tested. And this patch is again the fundamentally wrong answer.

Yep.  Long term no hz is definitely the goal.  I'm not sufficiently
connected to the -rt space I guess to have followed that there's somebody
again looking in that direction.  The rfc patch was mostly just a minimal
is there anything simple we can do in the meantime exercise.

> We have told HPC folks for years that we need a kind of "NOHZ" mode
> for HPC where we can transparently switch off the tick when only one
> user space bound thread is active and switch back to normal once this
> thing terminates or goes into the kernel via a syscall.

I'd not heard of this in between NOHZ-y idea...sounds promising.
We'd talked about different non-idle no hz approaches in the past year
or so, some of which were on the veeery complicated side of the spectrum.

> Sigh, nothing
> happened ever except for repeating the same crap patches over and
> over.

I'll check out what Frederic is doing.  Thanks for the pointer and
apologies for the noise.

-- 
Tim Pepper  <lnxninja@linux.vnet.ibm.com>
IBM Linux Technology Center

^ permalink raw reply

* Re: PROBLEM: memory corrupting bug, bisected to 6dda9d55
From: Andrew Morton @ 2010-10-11 21:00 UTC (permalink / raw)
  To: Mel Gorman
  Cc: linuxppc-dev, linux-kernel, linux-mm, pacman, KOSAKI Motohiro,
	Christoph Lameter, Yinghai Lu
In-Reply-To: <20101011143022.GD30667@csn.ul.ie>

(cc linuxppc-dev@lists.ozlabs.org)

On Mon, 11 Oct 2010 15:30:22 +0100
Mel Gorman <mel@csn.ul.ie> wrote:

> On Sat, Oct 09, 2010 at 04:57:18AM -0500, pacman@kosh.dhis.org wrote:
> > (What a big Cc: list... scripts/get_maintainer.pl made me do it.)
> > 
> > This will be a long story with a weak conclusion, sorry about that, but it's
> > been a long bug-hunt.
> > 
> > With recent kernels I've seen a bug that appears to corrupt random 4-byte
> > chunks of memory. It's not easy to reproduce. It seems to happen only once
> > per boot, pretty quickly after userspace has gotten started, and sometimes it
> > doesn't happen at all.
> > 
> 
> A corruption of 4 bytes could be consistent with a pointer value being
> written to an incorrect location.

It's corruption of user memory, which is unusual.  I'd be wondering if
there was a pre-existing bug which 6dda9d55bf545013597 has exposed -
previously the corruption was hitting something harmless.  Something
like a missed CPU cache writeback or invalidate operation.

How sensitive/vulnerable is PPC32 to such things?

^ permalink raw reply

* Re: [RFC] [PATCH] allow low HZ values?
From: Thomas Gleixner @ 2010-10-11 20:32 UTC (permalink / raw)
  To: Tim Pepper
  Cc: Marcio Saito, John Stultz, Jiri Slaby, Peter Zijlstra, x86, LKML,
	Frederic Weisbecker, Ingo Molnar, Paul Mackerras, H. Peter Anvin,
	Avantika Mathur, linuxppc-dev
In-Reply-To: <20101011201121.GA953@tpepper-t61p.dolavim.us>

On Mon, 11 Oct 2010, Tim Pepper wrote:

> I'm not necessarily wanting to open up the age old question of "what is
> a good HZ", but we were doing some testing on timer tick overheads for
> HPC applications and this came up...

Yeah. This comes always up when the timer tick overhead on HPC is
tested. And this patch is again the fundamentally wrong answer.

We have told HPC folks for years that we need a kind of "NOHZ" mode
for HPC where we can transparently switch off the tick when only one
user space bound thread is active and switch back to normal once this
thing terminates or goes into the kernel via a syscall. Sigh, nothing
happened ever except for repeating the same crap patches over and
over.

FYI, Frederic is working on that right now. He will talk about it at
the plumbers RT microconf, so you might catch him there.

Thanks,

	tglx

^ permalink raw reply

* [RFC] [PATCH] allow low HZ values?
From: Tim Pepper @ 2010-10-11 20:11 UTC (permalink / raw)
  To: linux-kernel
  Cc: Marcio Saito, John Stultz, Jiri Slaby, x86, Ingo Molnar,
	Paul Mackerras, H. Peter Anvin, Thomas Gleixner, linuxppc-dev,
	Avantika Mathur

I'm not necessarily wanting to open up the age old question of "what is
a good HZ", but we were doing some testing on timer tick overheads for
HPC applications and this came up...

Below is a minimal hack at enabling lower HZ values.  The kernel builds
and boots for me on x86_64 (simple laptop and kvm configs) and ppc64
(misc. IBM System p) with each of the added HZ options.

There's explicit code checking HZ down to 12, but HZ<100 wasn't a config
option.  We collected some data at 10, 12 and 25.  There'd been some
question of whether 10 would even work or not but it looks fine in the
relatively minimal testing we did.  We tried 12 since the code seemed
to allow for it.  And 25 as a "safe" lower value.  The only difference
observed under load (ie: no no idle HZ in play) was the expected timer
tick happening less often.  There was definitely surprise that nothing
else seemed to break anywhere, especially at 10.

Do people feel it is reasonable to have Kconfig bits to allow some lower
HZ values?

If so, then there's the question of what breaks.  It's reasonable to
think there are other going to be subtleties buried in code around
assumptions on the likely range of HZ:

- I'm not sure that what I did in inet_timewait_sock.h and jiffies.h is
  reasonable.
- arch/x86/kernel/i8253.c throws a warning at line 43 (v2.6.36-rc7):
      warning: large integer implicitly truncated to unsigned type
- drivers/char/cyclades.c's cy_ioctl() warns:
      drivers/char/cyclades.c:2761: warning: division by zero
- drivers, drivers, drivers across all the arch's could use sanity checking

-- 
Tim Pepper  <lnxninja@linux.vnet.ibm.com>
IBM Linux Technology Center



Not-Signed-off-by: Tim Pepper <lnxninja@us.ibm.com>

diff --git a/include/linux/jiffies.h b/include/linux/jiffies.h
index 6811f4b..8c225b2 100644
--- a/include/linux/jiffies.h
+++ b/include/linux/jiffies.h
@@ -15,7 +15,9 @@
  * OSF/1 kernel. The SHIFT_HZ define expresses the same value as the
  * nearest power of two in order to avoid hardware multiply operations.
  */
-#if HZ >= 12 && HZ < 24
+#if HZ < 12
+# define SHIFT_HZ	3
+#elif HZ >= 12 && HZ < 24
 # define SHIFT_HZ	4
 #elif HZ >= 24 && HZ < 48
 # define SHIFT_HZ	5
diff --git a/include/net/inet_timewait_sock.h b/include/net/inet_timewait_sock.h
index a066fdd..1aba305 100644
--- a/include/net/inet_timewait_sock.h
+++ b/include/net/inet_timewait_sock.h
@@ -39,8 +39,9 @@ struct inet_hashinfo;
  * If time > 4sec, it is "slow" path, no recycling is required,
  * so that we select tick to get range about 4 seconds.
  */
-#if HZ <= 16 || HZ > 4096
-# error Unsupported: HZ <= 16 or HZ > 4096
+/* HACK HACK */
+#if HZ > 4096
+# error Unsupported: HZ > 4096
 #elif HZ <= 32
 # define INET_TWDR_RECYCLE_TICK (5 + 2 - INET_TWDR_RECYCLE_SLOTS_LOG)
 #elif HZ <= 64
diff --git a/kernel/Kconfig.hz b/kernel/Kconfig.hz
index 94fabd5..37302bf 100644
--- a/kernel/Kconfig.hz
+++ b/kernel/Kconfig.hz
@@ -15,6 +15,22 @@ choice
 	 environment leading to NR_CPUS * HZ number of timer interrupts
 	 per second.
 
+	config HZ_10
+		bool "10 HZ"
+	help
+	  10 Hz is extremely aggressive and may break things.
+
+	config HZ_12
+		bool "12 HZ"
+	help
+	  12 Hz because it's less aggressive than 10?
+
+	config HZ_25
+		bool "25 HZ"
+	help
+	  25 Hz is useful for reducing HPC application jitter caused by
+	  timer interrupts happening during a "fixed time quantum of work
+	  then barrier" style workload.
 
 	config HZ_100
 		bool "100 HZ"
@@ -49,6 +65,9 @@ endchoice
 
 config HZ
 	int
+	default 10 if HZ_10
+	default 12 if HZ_12
+	default 25 if HZ_25
 	default 100 if HZ_100
 	default 250 if HZ_250
 	default 300 if HZ_300

^ permalink raw reply related

* Re: [patch] ps3disk: passing wrong variable to bvec_kunmap_irq()
From: Jens Axboe @ 2010-10-11 19:42 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: cbe-oss-dev@lists.ozlabs.org, Martin K. Petersen, Geoff Levand,
	kernel-janitors@vger.kernel.org, FUJITA Tomonori,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20101011191335.GH5851@bicker>

On 2010-10-11 21:13, Dan Carpenter wrote:
> This should pass "buf" to bvec_kunmap_irq() instead of "bv".  The api is
> like kmap_atomic() instead of kmap().
> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> 
> diff --git a/drivers/block/ps3disk.c b/drivers/block/ps3disk.c
> index e9da874..03688c2 100644
> --- a/drivers/block/ps3disk.c
> +++ b/drivers/block/ps3disk.c
> @@ -113,7 +113,7 @@ static void ps3disk_scatter_gather(struct ps3_storage_device *dev,
>  			memcpy(buf, dev->bounce_buf+offset, size);
>  		offset += size;
>  		flush_kernel_dcache_page(bvec->bv_page);
> -		bvec_kunmap_irq(bvec, &flags);
> +		bvec_kunmap_irq(buf, &flags);
>  		i++;
>  	}
>  }

Thanks applied, that bug is all too common.

-- 
Jens Axboe

^ permalink raw reply

* [patch] ps3disk: passing wrong variable to bvec_kunmap_irq()
From: Dan Carpenter @ 2010-10-11 19:13 UTC (permalink / raw)
  To: Geoff Levand
  Cc: cbe-oss-dev, Martin K. Petersen, Jens Axboe, kernel-janitors,
	FUJITA Tomonori, linuxppc-dev

This should pass "buf" to bvec_kunmap_irq() instead of "bv".  The api is
like kmap_atomic() instead of kmap().

Signed-off-by: Dan Carpenter <error27@gmail.com>

diff --git a/drivers/block/ps3disk.c b/drivers/block/ps3disk.c
index e9da874..03688c2 100644
--- a/drivers/block/ps3disk.c
+++ b/drivers/block/ps3disk.c
@@ -113,7 +113,7 @@ static void ps3disk_scatter_gather(struct ps3_storage_device *dev,
 			memcpy(buf, dev->bounce_buf+offset, size);
 		offset += size;
 		flush_kernel_dcache_page(bvec->bv_page);
-		bvec_kunmap_irq(bvec, &flags);
+		bvec_kunmap_irq(buf, &flags);
 		i++;
 	}
 }

^ permalink raw reply related

* Re: Questions on interrupt vector assignment on MPC8641D
From: Scott Wood @ 2010-10-11 17:30 UTC (permalink / raw)
  To: david.hagood; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <9af5fd3c8f713d261c7acdc1d195fb84.squirrel@localhost>

On Mon, 11 Oct 2010 12:02:15 -0500
<david.hagood@gmail.com> wrote:

> Re-ordering your questions a bit:
> 
> > What board are you using?  What kernel?
> 
> One of 2 boards: Either an Embedded Planet or a Performance Tech uTCA
> board based on the MPC8641D, running the 2.6.26 as supplied by EP.

That's a very old kernel.  Have you contacted EP support?

You may be better off trying to migrate anything board-specific to the
latest kernel, rather than do new development on the vendor kernel,
especially if you're looking for community support, and definitely if
you're trying to do something that could be merged upstream to help
others.

The MPC8641D itself is supported in mainline Linux.

> > On Sat, 9 Oct 2010 10:52:49 -0500
> > Documentation/powerpc/dts-bindings/fsl/mpic.txt
> Not present in the version I have.

We didn't have a time machine, so we were only able to add things to new
releases, not old ones. :-)

Should it have been documented from the start?  Ideally, yes, and we're
pickier now than we were then about insisting on bindings being
documented before they're used.  But nothing's perfect.

> > If it's not in the dts, add it.  If for whatever reason that's not an
> > option, you can use irq_create_mapping() as I mentioned in the previous
> > e-mail.
> And as I've said previously, I have no good info on HOW to add the nodes,
> WHAT to add, or WHERE. You may as well be saying "Bargle the Narbog".

There are no nodes to add with irq_create_mapping().

Just bargle the narbog.

> And when I try to use irq_create_mapping() it seg faults, which doesn't
> exactly help me get my interrupt hooked up.

Well, I didn't know it was going to do that...  Your usage of
irq_create_mapping() seems fine, there's just something going wrong.
See my other reply about the number of MPIC interrupts.

> > If you grep arch/powerpc/boot/dts for msi in a reasonably recent kernel
> > you should find msi nodes.
> 
> ddhagood@WIC-102362:..Workspace/Linux_Kernel_for_PPC> grep msi
> arch/powerpc/boot/dts  -ir
> arch/powerpc/boot/dts/glacier.dts:			enable-msi-hole;
> arch/powerpc/boot/dts/taishan.dts:			enable-msi-hole;
> arch/powerpc/boot/dts/canyonlands.dts:			enable-msi-hole;
> arch/powerpc/boot/dts/katmai.dts:			enable-msi-hole;

"reasonably recent kernel"

$ grep -rI msi arch/powerpc/boot/dts/|wc -l
85

> > What did I get signed up for? :-)
> 
> It sounds like Tiejun thinks you might be working on, well, basically what
> I am working on - a generic interface to allow user space to support being
> a endpoint,

I am not working on that -- not sure how I gave that impression.

> with the ability to generate interrupts to the host root
> complex, get interrupts from the host root complex, provide memory to be
> accessed by the host root complex via the PPC's BARs, and to access the
> host root complex's PCI address space via the OATMU windows. Something to
> allow a person to use a PPC SOC as an endpoint, and to get on with the job
> of providing functionality rather than having to deal with esoterica.

That does sound like it would be a useful addition.

-Scott

^ permalink raw reply

* Re: Serial RapidIO Maintaintance read causes lock up
From: Bastiaan Nijkamp @ 2010-10-11 17:27 UTC (permalink / raw)
  To: John Traill, Bounine, Alexandre; +Cc: linuxppc-dev
In-Reply-To: <4CAB39F4.3030208@freescale.com>

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

Hi,

We have found a poorly documented jumper on our boards that force pci-mode
to 32bit instead of 64bit. It seems to have an effect on RapidIO aswell
since the host now completes the enumeration process including finding the
other board. However, as soon as the agent starts the peer discovery
process, ALL RapidIO related registers on the agent are set to 0xFF4A.
Which, offcourse, caused unpredicted behaviour. All other jumpers are set as
they should be, especially the reference clock (100Mhz) and linkspeed
settings (1.25Gbps). I am currently having some trouble understanding why
this happens.

I've double checked Accept All on both boards in the registers and on both
boards it has been set correctly before discovery/enumeration.

Here is the LAW configuration from u-boot for the agent and host:

Local Access Window Configuration
LAWBAR00: 0x00000000 LAWAR0x00: 0x80f0001b
        (EN: 1 TGT: 0x0f SIZE: 256 MiB)
LAWBAR01: 0x00080000 LAWAR0x01: 0x8000001c
        (EN: 1 TGT: 0x00 SIZE: 512 MiB)
LAWBAR02: 0x000e2000 LAWAR0x02: 0x80000016
        (EN: 1 TGT: 0x00 SIZE: 8 MiB)
LAWBAR03: 0x000f0000 LAWAR0x03: 0x8040001b
        (EN: 1 TGT: 0x04 SIZE: 256 MiB)
LAWBAR04: 0x000c0000 LAWAR0x04: 0x80c0001c
        (EN: 1 TGT: 0x0c SIZE: 512 MiB)
LAWBAR05: 0x00000000 LAWAR0x05: 0x00000000
        (EN: 0 TGT: 0x00 SIZE: 2 Bytes)
LAWBAR06: 0x00000000 LAWAR0x06: 0x00000000
        (EN: 0 TGT: 0x00 SIZE: 2 Bytes)
LAWBAR07: 0x00000000 LAWAR0x07: 0x00000000
        (EN: 0 TGT: 0x00 SIZE: 2 Bytes)
LAWBAR08: 0x00000000 LAWAR0x08: 0x00000000
        (EN: 0 TGT: 0x00 SIZE: 2 Bytes)
LAWBAR09: 0x00000000 LAWAR0x09: 0x00000000
        (EN: 0 TGT: 0x00 SIZE: 2 Bytes)

We have removed the RapidIO TLB Entries from u-boot.

Kind regards,
Bastiaan Nijkamp

[-- Attachment #2: Type: text/html, Size: 2202 bytes --]

^ permalink raw reply

* Re: Questions on interrupt vector assignment on MPC8641D
From: Scott Wood @ 2010-10-11 17:17 UTC (permalink / raw)
  To: tiejun.chen; +Cc: david.hagood, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <4CB2DEFB.90204@windriver.com>

On Mon, 11 Oct 2010 17:55:07 +0800
"tiejun.chen" <tiejun.chen@windriver.com> wrote:

> david.hagood@gmail.com wrote:
> > OK, using 224 as the MPIC interrupt number, and attempting to map it via
> > irq_create_mapping(0,224) gives me a kernel seg fault:
> 
> This should not be correct without initialing MSI for MPIC host. As I comment on
> another email, please refer to the file, arch/powerpc/sysdev/fsl_msi.c.

The MSI driver is not going to do anything that would change whether
that call to irq_create_mapping() works or not.  fsl_msi.c would also
probably require some changes to be useful in endpoint mode.

> > Reading the source (since these calls don't seem to have any other
> > documentation), it *looks* like it ought to be valid to call
> > irq_create_mapping with a null irq_host *host - which is good, since I
> > can't see anything obvious that would give me some other irq_host (again,
> > since none of this seems to be documented anywhere).

There's mpic stuff in the call trace, so the NULL host was OK.

Look in arch/powerpc/platforms/86xx/pic.c.  What is the second-to-last
parameter of mpc86xx_init_irq() in your kernel tree?  It's 256 in
current upstream -- this is the number of IRQ sources the MPIC driver
will handle.

-Scott

^ permalink raw reply

* Re: Questions on interrupt vector assignment on MPC8641D
From: david.hagood @ 2010-10-11 17:02 UTC (permalink / raw)
  To: Scott Wood; +Cc: david.hagood, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20101011105031.52a5c06a@udp111988uds.am.freescale.net>

Re-ordering your questions a bit:

> What board are you using?  What kernel?

One of 2 boards: Either an Embedded Planet or a Performance Tech uTCA
board based on the MPC8641D, running the 2.6.26 as supplied by EP.


> On Sat, 9 Oct 2010 10:52:49 -0500
> Documentation/powerpc/dts-bindings/fsl/mpic.txt
Not present in the version I have.

>
> Plus the chip manual, for the register offsets.
I have that now, so at least one part of the fog is a bit less dense.

> If it's not in the dts, add it.  If for whatever reason that's not an
> option, you can use irq_create_mapping() as I mentioned in the previous
> e-mail.
And as I've said previously, I have no good info on HOW to add the nodes,
WHAT to add, or WHERE. You may as well be saying "Bargle the Narbog".

And when I try to use irq_create_mapping() it seg faults, which doesn't
exactly help me get my interrupt hooked up.

(crotchety old man mode)Kids these day - when I was a kid, you just
grabbed the IRQ vector into your assembly code and away you went. Now GET
OFF MY YARD!(/crotchety old man mode).

> If you grep arch/powerpc/boot/dts for msi in a reasonably recent kernel
> you should find msi nodes.

ddhagood@WIC-102362:..Workspace/Linux_Kernel_for_PPC> grep msi
arch/powerpc/boot/dts  -ir
arch/powerpc/boot/dts/glacier.dts:			enable-msi-hole;
arch/powerpc/boot/dts/taishan.dts:			enable-msi-hole;
arch/powerpc/boot/dts/canyonlands.dts:			enable-msi-hole;
arch/powerpc/boot/dts/katmai.dts:			enable-msi-hole;

> What did I get signed up for? :-)

It sounds like Tiejun thinks you might be working on, well, basically what
I am working on - a generic interface to allow user space to support being
a endpoint, with the ability to generate interrupts to the host root
complex, get interrupts from the host root complex, provide memory to be
accessed by the host root complex via the PPC's BARs, and to access the
host root complex's PCI address space via the OATMU windows. Something to
allow a person to use a PPC SOC as an endpoint, and to get on with the job
of providing functionality rather than having to deal with esoterica.

I'm hoping to make an interface generic enough to support not only the
PPC, but other devices as well (things I cannot go into due to NDAs at
this time). As a systems programmer, I am tired of re-inventing the wheel
when dealing with being an endpoint - I want to invent the wheel, and push
it uphill to the greater Linux community so that a) others are spared the
pain I am suffering, and b) maybe other SOC vendors will implement my
interface (and thus I won't have to).

^ permalink raw reply

* Re: Questions on interrupt vector assignment on MPC8641D
From: Scott Wood @ 2010-10-11 15:50 UTC (permalink / raw)
  To: david.hagood; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <adaf806ee1c11621dbb51246bef0acd8.squirrel@localhost>

On Sat, 9 Oct 2010 10:52:49 -0500
<david.hagood@gmail.com> wrote:

> First of all - where is all of this documented? There seems to be a great
> deal of "oral tradition" type knowledge here, but is any of it actually
> written down somewhere? (see below for examples)

Documentation/powerpc/dts-bindings/fsl/mpic.txt

Plus the chip manual, for the register offsets.

> > On Thu, 7 Oct 2010 15:12:26 -0500
> > This is asking for the 256th specifier in the interrupts property in
> > the mpic node -- not what you want.
> 
> That was from some of the previous emails in this thread.
> 
> > Ideally you would have a node for your device with an interrupt
> > specifier that you could look up with irq_of_parse_and_map().
> 
> OK, and how do these devices come into being? From what I can tell, they
> are defined by uBoot, and if uBoot doesn't define it, then you are out of
> luck.

More commonly they're statically defined in the dts file, not
dynamically created by u-boot.

If it's not in the dts, add it.  If for whatever reason that's not an
option, you can use irq_create_mapping() as I mentioned in the previous
e-mail.

> * A set of APIs (irq_of_parse_and_map(), irq_create_mapping) that were,
> for all intents, undocumented (just "here's the parameters", no
> description of when and how to use them), that took a ???? and returned a
> ???? (because, with the documentation I had, that's basically all I could
> say about them).

Yes, there are many parts of the kernel that could use better
documentation.

> > BTW, the MSIs are already described in an msi node in the device tree.
> 
> As I stated previously - not that I can see.

What board are you using?  What kernel?

If you grep arch/powerpc/boot/dts for msi in a reasonably recent kernel
you should find msi nodes.

> It may be they are defined in PCI Root Complex mode, 

No, it's a separate node.

-Scott

^ permalink raw reply

* Re: Questions on interrupt vector assignment on MPC8641D
From: Scott Wood @ 2010-10-11 15:51 UTC (permalink / raw)
  To: tiejun.chen; +Cc: david.hagood, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <4CB2DE2B.7040504@windriver.com>

On Mon, 11 Oct 2010 17:51:39 +0800
"tiejun.chen" <tiejun.chen@windriver.com> wrote:

> Maybe you can check the file, ppc4xx_pci.c since ppc4xx also can support EP as I
> previously said. And sounds Scott will do something to support EP for Freescale
> chip.

What did I get signed up for? :-)

-Scott

^ permalink raw reply

* Re: Questions on interrupt vector assignment on MPC8641D
From: david.hagood @ 2010-10-11 14:44 UTC (permalink / raw)
  To: tiejun.chen; +Cc: Scott Wood, david.hagood, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <4CB2DE2B.7040504@windriver.com>


> You should define MSI device nodes on your target dts. And you can refer
> to the
> file, mpc8572ds.dts.

I see nothing in that file that defines any MSIs. I see code that looks
like it maps ROOT COMPLEX MODE interrupts on regular PCI interfaces, which
IS NOT WHAT I AM DOING.

Since it seems I have been unclear, let me state this as clearly as possible.

I AM DOING ENDPOINT MODE.


> I think you can check fsl_msi.c to figure out what you want.
Except that file doesn't exist in the 2.6.26 kernel with which I am working.

> Firstly you should use irq_of_parse_and_map()/irq_create_mapping() to map
> the
> real hardware irq to virtual irq. Then use request_irq() with the virtual
> irq to
> hook your interrupt handler.

Except I have no device to pass irq_of_parse_and_map, and when I call
irq_create_mapping it seg faults.

>
> Maybe you can check the file, ppc4xx_pci.c since ppc4xx also can support
> EP as I
> previously said.

It only "supports" EP mode in the sense that it sets the hardware up to
export a BAR, and does nothing else. It doesn't provide any means for any
other code to actually DO anything in that mode. It doesn't define a way
to be interrupted by the root complex, nor to do anything when that
interrupt happens. It doesn't supply any meaningful framework to get any
data from the Root Complex.

That's the whole reason I am trying to make a piece of code that actually
WILL make those things available in a useful fashion.

^ permalink raw reply

* Re: powerpc, fs_enet: scanning PHY after Linux is up
From: Holger brunck @ 2010-10-11 11:49 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, devicetree-discuss, hs, Detlev Zundel, netdev
In-Reply-To: <20101008170615.GC3863@angua.secretlab.ca>

Hi Grant,

On 10/08/2010 07:06 PM, Grant Likely wrote:
> On Fri, Oct 08, 2010 at 10:50:50AM +0200, Holger brunck wrote:
>>> On Wed, Oct 6, 2010 at 3:53 AM, Heiko Schocher <hs@denx.de> wrote:
>>
>>>> Wouldn;t it be good, just if we need a PHY (on calling fs_enet_open)
>>>> to look if there is one?
>>>>
>>>> Something like that (not tested):
>>>>
>>>> in drivers/net/fs_enet/fs_enet-main.c in fs_init_phy()
>>>> called from fs_enet_open():
>>>>
>>>> Do first:
>>>> phydev =  of_phy_find_device(fep->fpi->phy_node);
>>>>
>>>> Look if there is a driver (phy_dev->drv == NULL ?)
>>>>
>>>> If not, call new function
>>>> of_mdiobus_register_phy(mii_bus, fep->fpi->phy_node)
>>>> see below patch for it.
>>>>
>>>> If this succeeds, all is OK, and we can use this phy,
>>>> else ethernet not work.
>>>
>>> I don't like this approach because it muddies the concept of which
>>> device is actually responsible for managing the phys on the bus.  Is
>>> it managed by the mdio bus device or the Ethernet device?  It also has
>>> a potential race condition.  Whereas triggering a late driver bind
>>> will be safe.
>>>
>>> Alternately, I'd also be okay with a common method to trigger a
>>> reprobe of a particular phy from userspace, but I fear that would be a
>>> significantly more complex solution.
>>>
>>>>
>>>> !!just no idea, how to get mii_bus pointer ...
>>>
>>> You'd have to get the parent of the phy node, and then loop over all
>>> the registered mdio busses looking for a bus that uses that node.
>>>
>>
>> you say that you don't like the approach to probe the phy again in fs_enet_open,
>> but currently I don't understand what would be the alternate trigger point to
>> rescan the mdio bus?
> 
> Same trigger point, but different operation.  At fs_enet_open time,
> instead of registering the phy_device, the phy layer could sanity
> check the already registered phy_device, and refuse to connect to it
> if the phy isn't responding.  If it is responding, then it could
> re-attempt binding a phy_driver to it (although I just realized that
> this has other problems, such as correct module loading.  See below)
> 

ok.

>> I made a first patch to enhance the phy_device structure and rescan the mdio bus
>> at time of fs_enet_open (because I didn't see a better trigger point). The
>> advantage is that we got the mii_bus pointer and the phy addr stored in the
>> already created phy device structure and is therefore easy to use. See the patch
>> below for this modifications. Whats currently missing in the patch is to set the
>> phy_id if the phy was scanned later after phy_device creation. For the mgcoge
>> board it seems to solve our problem, but maybe I miss something important.
>>
>> Best regards
>> Holger Brunck
>>
>> diff --git a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c
>> index ec2f503..6bc117f 100644
>> --- a/drivers/net/fs_enet/fs_enet-main.c
>> +++ b/drivers/net/fs_enet/fs_enet-main.c
>> @@ -775,7 +774,8 @@ static int fs_enet_open(struct net_device *dev)
>>  {
>>         struct fs_enet_private *fep = netdev_priv(dev);
>>         int r;
>> -       int err;
>> +       int err = 0;
>> +       u32 phy_id = 0;
>>
>>         /* to initialize the fep->cur_rx,... */
>>         /* not doing this, will cause a crash in fs_enet_rx_napi */
>> @@ -795,13 +795,23 @@ static int fs_enet_open(struct net_device *dev)
>>                 return -EINVAL;
>>         }
>>
>> -       err = fs_init_phy(dev);
>> -       if (err) {
>> +       if (fep->phydev == NULL)
>> +               err = fs_init_phy(dev);
>> +
>> +       if (!err && (fep->phydev->available == false))
>> +               r = get_phy_id(fep->phydev->bus, fep->phydev->addr, &phy_id);
>> +
>> +       if (err || (phy_id == 0xffffffff)) {
>>                 free_irq(fep->interrupt, dev);
>>                 if (fep->fpi->use_napi)
>>                         napi_disable(&fep->napi);
>> -               return err;
>> +               if (err)
>> +                       return err;
>> +               else
>> +                       return -EINVAL;
>>         }
>> +       else
>> +               fep->phydev->available = true;
>>         phy_start(fep->phydev);
>>
>>         netif_start_queue(dev);
>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>> index adbc0fd..1f443cb 100644
>> --- a/drivers/net/phy/phy_device.c
>> +++ b/drivers/net/phy/phy_device.c
>> @@ -173,6 +173,10 @@ struct phy_device* phy_device_create(struct mii_bus *bus,
>> int addr, int phy_id)
>>         dev->dev.bus = &mdio_bus_type;
>>         dev->irq = bus->irq != NULL ? bus->irq[addr] : PHY_POLL;
>>         dev_set_name(&dev->dev, PHY_ID_FMT, bus->id, addr);
>> +       if (phy_id == 0xffffffff)
>> +               dev->available = false;
>> +       else
>> +               dev->available = true;
> 
> This flag shouldn't be necessary.  Just check whether or not
> phy_device->phy_id is sane at phy_attach_direct() time.  If it is
> mostly f's, then don't attach.
>

Yes, indeed it is unneeded. Thanks for pointing out.

>>
>>         dev->state = PHY_DOWN;
>>
>> @@ -232,13 +236,11 @@ struct phy_device * get_phy_device(struct mii_bus *bus,
>> int addr)
>>         int r;
>>
>>         r = get_phy_id(bus, addr, &phy_id);
>> -       if (r)
>> -               return ERR_PTR(r);
>>
>>         /* If the phy_id is mostly Fs, there is no device there */
>> -       if ((phy_id & 0x1fffffff) == 0x1fffffff)
>> -               return NULL;
>> -
>> +       if (((phy_id & 0x1fffffff) == 0x1fffffff) || r)
>> +               phy_id = 0xffffffff;
>> +       /* create phy even if the phy is currently not available */
>>         dev = phy_device_create(bus, addr, phy_id);
> 
> Cannot do it this way because many phylib users probe the bus for phys
> instead of the explicit creation used with the device tree.  There
> needs to be a method to explicitly skip this test when creating a phy;
> possibly by having the device tree code call phy_device_create()
> directly.
> 

Ah ok, every phy_device_create() call from of_mdiobus_register should skip this
test, because if a phy is described in the dts it is present (sooner or later)
and if phy_device_create is called from somewhere else I do this test as usual.
I adapted my patch accordingly.

> Hmmm.... I see another problem.  Deferred probing of the phy will
> potentially cause problems with module loading.  If the binding is
> deferred to phy connect time; then the phy driver may not have time to
> get loaded before the phy layer decides there is no driver and binds
> it to the generic one.  Blech.
> 
> Okay, so it seems like a method of explicitly triggering a phy_device
> rebind from userspace is necessary.  This could be done with a
> per-phy_device sysfs file I suppose.  Just an empty file that when
> read triggers a re-read of the phy id registers, and retries binding a
> driver, including the request_module call in phy_device_create().
> 

Okay I suspected that there is not an easy solution for our problem. Another
solution comes in my mind. If we defer the call to fs_enet_probe at startup. So
enhance the dts entry with something like an hotplug indication and then trigger
via an sysfs entry the call to fs_enet_probe if the phy is up... Other hotplug
devices should have similar problems...

However for our mgcoge board we can live with the fact that we can't load/unload
the driver dynamically. So I think we will go with this modified "out of tree"
patch for our board. Thanks for your support.

Best regards
Holger Brunck

^ permalink raw reply

* Re: Freescale P2020 / 85xx PCIe and Advance Error Reporting (AER) service problem
From: Benjamin Herrenschmidt @ 2010-10-11 11:32 UTC (permalink / raw)
  To: Eran Liberty; +Cc: linux-pci, linuxppc-dev
In-Reply-To: <4CB2E517.8020401@extricom.com>


> BUT if we take into consideration that:
> 1. Freescale is a serious dude in the hood and on the whole does a good 
> job with its products and their Linux support.

Sure but that's irrelevant to the technical problem at hand :-)

> 2. The P2020 does state it has an MSI mechanism support (although one is 
> not present as a PCIe capability header for some reason)

Then it's broken :-(

> 3. Errors in general and AER are major features in PCIe.
> 4. PCIe has been here quite a while and it is not new to Freescale or 
> anyone else.

Right but we don't do AER on ppc44x either, I know we should but for
some reason, AER hasn't been on anybody #1 priority list in embedded
world so far...

> I am much more inclined to believe that I have missed something by a 
> mile then that Freescale did. I just don't know what I am missing.

No, I think you haven't and we just need to fix it :-)

Cheers,
Ben.

> My device tree is a clone of "arch/ powerpc/ boot/ dts/ p2020rdb.dts"
> 
> It has a PCI node that looks like this:
> ----------------------------- snip -----------------------------
>     pci0: pcie@ffe09000 {
>         cell-index = <1>;
>         compatible = "fsl,mpc8548-pcie";
>         device_type = "pci";
>         #interrupt-cells = <1>;
>         #size-cells = <2>;
>         #address-cells = <3>;
>         reg = <0 0xffe09000 0 0x1000>;
>         bus-range = <0 255>;
>         ranges = <0x2000000 0x0 0xa0000000 0 0xa0000000 0x0 0x20000000
>               0x1000000 0x0 0x00000000 0 0xffc30000 0x0 0x10000>;
>         clock-frequency = <33333333>;
>         interrupt-parent = <&mpic>;
>         interrupts = <25 2>;
>         interrupt-map-mask = <0xf800 0x0 0x0 0x7>;
>         interrupt-map = <
>             /* IDSEL 0x0 */
>             0000 0x0 0x0 0x1 &mpic 0x4 0x1
>             0000 0x0 0x0 0x2 &mpic 0x5 0x1
>             0000 0x0 0x0 0x3 &mpic 0x6 0x1
>             0000 0x0 0x0 0x4 &mpic 0x7 0x1
>             >;
>         pcie@0 {
>             reg = <0x0 0x0 0x0 0x0 0x0>;
>             #size-cells = <2>;
>             #address-cells = <3>;
>             device_type = "pci";
>             ranges = <0x2000000 0x0 0xa0000000
>                   0x2000000 0x0 0xa0000000
>                   0x0 0x20000000
> 
>                   0x1000000 0x0 0x0
>                   0x1000000 0x0 0x0
>                   0x0 0x100000>;
>         };
>     };
> ----------------------------- snap -----------------------------
> 
> and under "soc" it has an MSI node that looks like that:
> ----------------------------- snip -----------------------------
>         msi@41600 {
>             compatible = "fsl,p2020-msi", "fsl,mpic-msi";
>             reg = <0x41600 0x80>;
>             msi-available-ranges = <0 0x100>;
>             interrupts = <
>                 0xe0 0
>                 0xe1 0
>                 0xe2 0
>                 0xe3 0
>                 0xe4 0
>                 0xe5 0
>                 0xe6 0
>                 0xe7 0>;
>             interrupt-parent = <&mpic>;
>         };
> ----------------------------- snap -----------------------------
> 
> -- Liberty
> 

^ permalink raw reply

* Re: Questions on interrupt vector assignment on MPC8641D
From: David Hagood @ 2010-10-11 11:30 UTC (permalink / raw)
  To: tiejun.chen; +Cc: Scott Wood, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <4CB2DE2B.7040504@windriver.com>

On Mon, 2010-10-11 at 17:51 +0800, tiejun.chen wrote:
> 
> You should define MSI device nodes on your target dts. And you can refer to the
> file, mpc8572ds.dts.
> 
> Often U-Boot dose not generate MSI information and embed that to dtb.
> 
> > 
> > But even assuming you can define these nodes at run time, as far as I can
> > see, you are right back to the question of "and how do I know what the
> > mappings are so I can create the node?" You haven't answered the question,
> > you've just moved where you are asking it.
> 
> I think you can check fsl_msi.c to figure out what you want.
<snip>

> Often most latest classic books/articles always use x86 code as an
example to
> clarify Linux. So you have to understand something on PPC via codes.
But I think
> you will benefit more information from the codes than documents :)
> 

RTFS. That wouldn't be bad advice, if the source were actually
commented. None of these APIS have any meaningful comments in them,
around them, or anywhere near them. You'd think we were back in the old
BASIC days, when comments occupied run-time memory. Code can tell me
WHAT is being done, but not WHY, and not what assumptions are being
made.

So when you say "You should define MSI device nodes on your target dts."
we go right back to my comment of "You haven't answered the question,
you've just moved where you are asking it."

> 
> Firstly you should use irq_of_parse_and_map()/irq_create_mapping() to map the
> real hardware irq to virtual irq. Then use request_irq() with the virtual irq to
> hook your interrupt handler.
And when I do that, I get a segfault as my follow-on mail reports.

> 
> Maybe you can check the file, ppc4xx_pci.c since ppc4xx also can support EP as I
> previously said. And sounds Scott will do something to support EP for Freescale
> chip.
I will look at that file.

> 
> Looks you want to your host root complex to trigger MSI to mpc8641 EP target? If
> so I'm a bit confused since MSIs should be delivered to to the Root Complex
> resided on your host.
I want the host to be able to interrupt the PPC. Since this is PCIe, the
only way that will be working is for the root complex to do a write to
somewhere on the PPC. This is basically the same mechanism as MSI, only
with the PPC as the target of the write rather than the root complex.
Obviously, the rest of the PCI devices will be sending interrupts to the
root complex, some via MSI. That has nothing to do with this discussion.

Since the PPC is in endpoint mode, the MSI hardware on the PPC won't be
used. The hardware is writable from the root complex via the PPC BAR0,
so I see no reason not to reuse it to trigger interrupts on the PPC from
the Root complex.

^ permalink raw reply

* Re: Freescale P2020 / 85xx PCIe and Advance Error Reporting (AER) service problem
From: Eran Liberty @ 2010-10-11 10:21 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linux-pci, linuxppc-dev
In-Reply-To: <1286756363.2463.517.camel@pasglop>

Benjamin Herrenschmidt wrote:
>>  - pcie_portdrv_probe() will be called for every BRIDGE class PCI device. P2020 PCIe is a PCI-PCI BRIDGE class so no problem here. 
>>  - The code will continue to check that we have PCI_CAP_ID_EXP capability, which we have and continue to pcie_port_device_register().
>>  - Now ,the function pcie_port_device_register() will FAIL. It will fail because it will call assign_interrupt_mode(), return with PCIE_PORT_NO_IRQ, and giveup with a reasonable remark in the code
>> "/*
>>   * Don't use service devices that require interrupts if there is
>>   * no way to generate them.
>>   */"
>>
>> So now the question is why calling assign_interrupt_mode() with the P2020 PCIe ROOT device return empty? Well...
>>  - First assign_interrupt_mode() will test for PCIE_PORT_MSIX_MODE. Freescale PCIe does not support this...
>>  - Second attampt is made to discover PCIE_PORT_MSI_MODE, which Freescale should support but the PCIe PCI_CAP_ID_MSI capability is published on the device side of the bridge and NOT on the PCIe ROOT device, which is the one probed and thus fails.
>>  - Last it attempts to look at "dev->pin" in order to set PCIE_PORT_INTx_MODE. On top of being the less recommended way (the old way), The Freescale PCIE ROOT device pin is not set anywhere.
>>
>> Failing all those the probe fails and the AER service is not activated for the PCIE device.
>
> So the question boils down to how does the bridge generate the AER
> interrupts. This should be documented in the FSL docs no ? The MSI in
> the child/device should be unrelated (it's your device MSI) no ? So the
> question is where's the missing interrupt.
>
> If it's a SoC interrupt, coming from the device-tree, then perhaps the
> generic AER code should be extended to recognize those.
>
> Cheers,
> Ben.
>
I agree...

BUT if we take into consideration that:
1. Freescale is a serious dude in the hood and on the whole does a good 
job with its products and their Linux support.
2. The P2020 does state it has an MSI mechanism support (although one is 
not present as a PCIe capability header for some reason)
3. Errors in general and AER are major features in PCIe.
4. PCIe has been here quite a while and it is not new to Freescale or 
anyone else.
I am much more inclined to believe that I have missed something by a 
mile then that Freescale did. I just don't know what I am missing.

My device tree is a clone of "arch/ powerpc/ boot/ dts/ p2020rdb.dts"

It has a PCI node that looks like this:
----------------------------- snip -----------------------------
    pci0: pcie@ffe09000 {
        cell-index = <1>;
        compatible = "fsl,mpc8548-pcie";
        device_type = "pci";
        #interrupt-cells = <1>;
        #size-cells = <2>;
        #address-cells = <3>;
        reg = <0 0xffe09000 0 0x1000>;
        bus-range = <0 255>;
        ranges = <0x2000000 0x0 0xa0000000 0 0xa0000000 0x0 0x20000000
              0x1000000 0x0 0x00000000 0 0xffc30000 0x0 0x10000>;
        clock-frequency = <33333333>;
        interrupt-parent = <&mpic>;
        interrupts = <25 2>;
        interrupt-map-mask = <0xf800 0x0 0x0 0x7>;
        interrupt-map = <
            /* IDSEL 0x0 */
            0000 0x0 0x0 0x1 &mpic 0x4 0x1
            0000 0x0 0x0 0x2 &mpic 0x5 0x1
            0000 0x0 0x0 0x3 &mpic 0x6 0x1
            0000 0x0 0x0 0x4 &mpic 0x7 0x1
            >;
        pcie@0 {
            reg = <0x0 0x0 0x0 0x0 0x0>;
            #size-cells = <2>;
            #address-cells = <3>;
            device_type = "pci";
            ranges = <0x2000000 0x0 0xa0000000
                  0x2000000 0x0 0xa0000000
                  0x0 0x20000000

                  0x1000000 0x0 0x0
                  0x1000000 0x0 0x0
                  0x0 0x100000>;
        };
    };
----------------------------- snap -----------------------------

and under "soc" it has an MSI node that looks like that:
----------------------------- snip -----------------------------
        msi@41600 {
            compatible = "fsl,p2020-msi", "fsl,mpic-msi";
            reg = <0x41600 0x80>;
            msi-available-ranges = <0 0x100>;
            interrupts = <
                0xe0 0
                0xe1 0
                0xe2 0
                0xe3 0
                0xe4 0
                0xe5 0
                0xe6 0
                0xe7 0>;
            interrupt-parent = <&mpic>;
        };
----------------------------- snap -----------------------------

-- Liberty

^ permalink raw reply

* Re: Questions on interrupt vector assignment on MPC8641D
From: tiejun.chen @ 2010-10-11  9:55 UTC (permalink / raw)
  To: david.hagood; +Cc: Scott Wood, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <8636b70ea34330679bebdaad187ccd68.squirrel@localhost>

david.hagood@gmail.com wrote:
> OK, using 224 as the MPIC interrupt number, and attempting to map it via
> irq_create_mapping(0,224) gives me a kernel seg fault:

This should not be correct without initialing MSI for MPIC host. As I comment on
another email, please refer to the file, arch/powerpc/sysdev/fsl_msi.c.

-Tiejun

> 
> Unable to handle kernel paging request for data at address 0x00000000
> Faulting instruction address: 0xc0016540
> Oops: Kernel access of bad area, sig: 11 [#1]
> PREEMPT SMP NR_CPUS=2 EP8641A
> Modules linked in: Endpoint_driver(+)
> NIP: c0016540 LR: c0050b38 CTR: c00163b8
> REGS: ef8c1ab0 TRAP: 0300   Not tainted  (2.6.26.2-ep1.10)
> MSR: 00001032 <ME,IR,DR>  CR: 24024482  XER: 00000000
> DAR: 00000000, DSISR: 40000000
> TASK = eedec6a0[60] 'insmod' THREAD: ef8c0000 CPU: 1
> GPR00: 00000000 ef8c1b60 eedec6a0 ffffffea 00000000 00000000 c035c17c
> 000000e0
> GPR08: 00400000 00000000 c035c020 00000000 24024422 100a7264 00000000
> 00000095
> GPR16: 00000095 00000000 0000000d 00000124 f102fd98 00000000 c004a4fc
> f102c000
> GPR24: 0000001a f1038400 00000000 c031e610 fffffffa 00009032 c031e5e0
> 000000e0
> NIP [c0016540] mpic_set_irq_type+0x188/0x1c4
> LR [c0050b38] set_irq_type+0x84/0xc8
> Call Trace:
> [ef8c1b60] [c0050b1c] set_irq_type+0x68/0xc8 (unreliable)
> [ef8c1b80] [c001572c] mpic_host_map+0xec/0xf4
> [ef8c1b90] [c00064a0] irq_setup_virq+0x60/0x98
> [ef8c1bb0] [c00065dc] irq_create_mapping+0x104/0x114
> [ef8c1bd0] [f1034798] Endpoint_device_ctor+0x258/0x3b8 [Endpoint_driver]
> 
> Reading the source (since these calls don't seem to have any other
> documentation), it *looks* like it ought to be valid to call
> irq_create_mapping with a null irq_host *host - which is good, since I
> can't see anything obvious that would give me some other irq_host (again,
> since none of this seems to be documented anywhere).
> 
> I am assuming the default host used by the system would be correct.
> 
> However, then I get this fault.
> 
> 
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
> 

^ permalink raw reply

* Re: Questions on interrupt vector assignment on MPC8641D
From: tiejun.chen @ 2010-10-11  9:51 UTC (permalink / raw)
  To: david.hagood; +Cc: Scott Wood, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <adaf806ee1c11621dbb51246bef0acd8.squirrel@localhost>

david.hagood@gmail.com wrote:
> First of all - where is all of this documented? There seems to be a great
> deal of "oral tradition" type knowledge here, but is any of it actually
> written down somewhere? (see below for examples)
> 
>> On Thu, 7 Oct 2010 15:12:26 -0500
>> This is asking for the 256th specifier in the interrupts property in
>> the mpic node -- not what you want.
> 
> That was from some of the previous emails in this thread.
> 
>> Ideally you would have a node for your device with an interrupt
>> specifier that you could look up with irq_of_parse_and_map().
> 
> OK, and how do these devices come into being? From what I can tell, they
> are defined by uBoot, and if uBoot doesn't define it, then you are out of
> luck.

You should define MSI device nodes on your target dts. And you can refer to the
file, mpc8572ds.dts.

Often U-Boot dose not generate MSI information and embed that to dtb.

> 
> But even assuming you can define these nodes at run time, as far as I can
> see, you are right back to the question of "and how do I know what the
> mappings are so I can create the node?" You haven't answered the question,
> you've just moved where you are asking it.

I think you can check fsl_msi.c to figure out what you want.
> 
> 
>> The vector/priority register (MSIVPR0) is at offset
>> 0x51c00.  Each interrupt source is 32 bytes.  The first interrupt
>> source is at 0x50000.
>>
>> So the interrupt number is (0x51c00 - 0x50000) / 32 = 224.
> 
> OK, so that's how you get the MPIC IRQ number. It's a pity that the key
> piece of information - "MPIC IRQ Numbers are defined by the interrupt
> vector table starting at 0x50000 and incrementing by 0x20 - see table 9.3
> of the MPD8641D manual for details" wasn't spelled out anywhere I could
> find.
> 
> Given the data I had: the kernel sources, the MPD8641D manual, and not
> much else, I had three unknowns:
> * a table of config registers, that could map an set of events into an
> arbitrary vector number.
> * The standard kernel "request_irq" API that takes that arbitrary vector
> number
> * A set of APIs (irq_of_parse_and_map(), irq_create_mapping) that were,
> for all intents, undocumented (just "here's the parameters", no
> description of when and how to use them), that took a ???? and returned a
> ???? (because, with the documentation I had, that's basically all I could
> say about them).

Firstly you should use irq_of_parse_and_map()/irq_create_mapping() to map the
real hardware irq to virtual irq. Then use request_irq() with the virtual irq to
hook your interrupt handler.

> 
> There was no documentation that said how to compute the MPIC interrupt
> vector, no documentation that said which API to use to convert that to an
> kernel IRQ, nothing.

Often most latest classic books/articles always use x86 code as an example to
clarify Linux. So you have to understand something on PPC via codes. But I think
you will benefit more information from the codes than documents :)

> 
>> BTW, the MSIs are already described in an msi node in the device tree.
> 
> As I stated previously - not that I can see. Neither does
> /proc/device_tree contain such a listing, nor does doing a
> "of_find_node_by_name(0,"msi");" nor "of_find_node_by_type(0,"msi");" find
> them.
> 
> It may be they are defined in PCI Root Complex mode, but again - I am
> writing code to handle Endpoint mode, which, as far as I can tell from the
> kernel sources, is NOT handled in any standardized way.

Maybe you can check the file, ppc4xx_pci.c since ppc4xx also can support EP as I
previously said. And sounds Scott will do something to support EP for Freescale
chip.

Looks you want to your host root complex to trigger MSI to mpc8641 EP target? If
so I'm a bit confused since MSIs should be delivered to to the Root Complex
resided on your host.

Tiejun

> 
> 
> Again, it would be nice were this sort of thing documented somewhere. If
> it is, I'd love a link - I've tried every combination of search terms I
> can think of to find it to no avail.
> 
> If it isn't documented, might I suggest that either a) creating a
> dedicated "Programming PPC Embedded devices and their components under
> Linux" be created, or b) an appropriate chapter or chapters be drafted and
> submitted to the Linux Device Drivers book.
> 
> 
> Don't take this message the wrong way - I do want to say "THANK YOU FOR
> THE HELP!" - I am just trying to point out why somebody with a fair amount
> of Linux kernel experience and a great deal of general embedded experience
> is having great trouble doing a simple thing.
> 
> I'll try creating a mapping, binding to it, and seeing what happens.
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
> 

^ permalink raw reply

* Re: Freescale P2020 / 85xx PCIe and Advance Error Reporting (AER) service problem
From: Benjamin Herrenschmidt @ 2010-10-11  0:19 UTC (permalink / raw)
  To: Eran Liberty
  Cc: Xianghua Xiao, linuxppc-dev, linux-pci, Tony Li, Linas Vepstas,
	ZHANG WEI
In-Reply-To: <4CADBD7B.3000506@extricom.com>


>  - pcie_portdrv_probe() will be called for every BRIDGE class PCI device. P2020 PCIe is a PCI-PCI BRIDGE class so no problem here. 
>  - The code will continue to check that we have PCI_CAP_ID_EXP capability, which we have and continue to pcie_port_device_register().
>  - Now ,the function pcie_port_device_register() will FAIL. It will fail because it will call assign_interrupt_mode(), return with PCIE_PORT_NO_IRQ, and giveup with a reasonable remark in the code
> "/*
>   * Don't use service devices that require interrupts if there is
>   * no way to generate them.
>   */"
> 
> So now the question is why calling assign_interrupt_mode() with the P2020 PCIe ROOT device return empty? Well...
>  - First assign_interrupt_mode() will test for PCIE_PORT_MSIX_MODE. Freescale PCIe does not support this...
>  - Second attampt is made to discover PCIE_PORT_MSI_MODE, which Freescale should support but the PCIe PCI_CAP_ID_MSI capability is published on the device side of the bridge and NOT on the PCIe ROOT device, which is the one probed and thus fails.
>  - Last it attempts to look at "dev->pin" in order to set PCIE_PORT_INTx_MODE. On top of being the less recommended way (the old way), The Freescale PCIE ROOT device pin is not set anywhere.
> 
> Failing all those the probe fails and the AER service is not activated for the PCIE device.

So the question boils down to how does the bridge generate the AER
interrupts. This should be documented in the FSL docs no ? The MSI in
the child/device should be unrelated (it's your device MSI) no ? So the
question is where's the missing interrupt.

If it's a SoC interrupt, coming from the device-tree, then perhaps the
generic AER code should be extended to recognize those.

Cheers,
Ben.

^ permalink raw reply

* Re: [RFC PATCH 03/11] ppc: Create ops to choose between direct window and iommu based on device mask
From: Benjamin Herrenschmidt @ 2010-10-10 23:41 UTC (permalink / raw)
  To: FUJITA Tomonori
  Cc: avorontsov, miltonm, linux-kernel, paulus, scottwood, nacc, akpm,
	linuxppc-dev
In-Reply-To: <20101011000903R.fujita.tomonori@lab.ntt.co.jp>

On Mon, 2010-10-11 at 00:09 +0900, FUJITA Tomonori wrote:
> On Sat, 09 Oct 2010 10:44:53 +1100
> Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> 
> > On Fri, 2010-10-08 at 10:33 -0700, Nishanth Aravamudan wrote:
> > > Also allow the coherent ops to be iommu if only the coherent mask is too
> > > small, mostly for driver that do not set set the coherent mask but also
> > > don't use the coherent api.
> > 
> > You are doing the transition at map_sg time which is a hot path, I don't
> > like that. Also you add all those "choose" variants of the dma ops...
> > not very nice at all.
> 
> Agreed, looks hacky.
> 
> 
> > You may want to look at the patches I posted to the list a while back
> > for doing direct DMA on Bimini:
> > 
> > [PATCH 1/2] powerpc/dma: Add optional platform override of dma_set_mask()
> 
> Would it be cleaner if each ppc dma_map_ops has the own set_dma_mask
> and dma_set_mask simply calls dma_map_ops->set_dma_mask?

I'm not sure I parse what you wrote above :-)

I did try with various methods back then, and what ended up sucking the
less was basically to hookup dma_set_mask() at the arch level.

In fact, it makes sense to the extent that the arch is the one that
knows that there are multiple regions configured potentially with
different capabilities.

You can still do the switch within the dma_ops->set_dma_mask if you want
I suppose, especially if you end up hitting different attribute regions
within a single bus or such but from my experience, it gets really hacky
with multiple ops structures etc...
> 
> > [PATCH 2/2] powerpc/dart_iommu: Support for 64-bit iommu bypass window on PCIe
> 

Cheers,
Ben.

^ permalink raw reply

* Re: Pegasos i8042 broken again
From: pacman @ 2010-10-10 17:32 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1286696147.2463.499.camel@pasglop>

Benjamin Herrenschmidt writes:
> 
> Those things really suck. They absolutely refuse to fix their FW for
> reasons I never quite managed to figure out.

The last time around, they did release a firmware patch (pegasos-dts-20071018)
to fix up the device tree enough to satisfy the kernel. Now that the kernel
has become dissatisfied again, maybe another patch will appear.

> 
> At this stage, I'd say the best is to add yet another pegasos workaround
> in prom_init that adds the missing compatible property.

This one would be more complex than the other fixes in prom_init. It's not
just the compatible property that's missing. The 8042 node in the device tree
has no children.

-- 
Alan Curry

^ permalink raw reply

* Re: [RFC PATCH 03/11] ppc: Create ops to choose between direct window and iommu based on device mask
From: FUJITA Tomonori @ 2010-10-10 15:09 UTC (permalink / raw)
  To: benh
  Cc: avorontsov, miltonm, linux-kernel, fujita.tomonori, paulus,
	scottwood, nacc, akpm, linuxppc-dev
In-Reply-To: <1286581493.2463.428.camel@pasglop>

On Sat, 09 Oct 2010 10:44:53 +1100
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Fri, 2010-10-08 at 10:33 -0700, Nishanth Aravamudan wrote:
> > Also allow the coherent ops to be iommu if only the coherent mask is too
> > small, mostly for driver that do not set set the coherent mask but also
> > don't use the coherent api.
> 
> You are doing the transition at map_sg time which is a hot path, I don't
> like that. Also you add all those "choose" variants of the dma ops...
> not very nice at all.

Agreed, looks hacky.


> You may want to look at the patches I posted to the list a while back
> for doing direct DMA on Bimini:
> 
> [PATCH 1/2] powerpc/dma: Add optional platform override of dma_set_mask()

Would it be cleaner if each ppc dma_map_ops has the own set_dma_mask
and dma_set_mask simply calls dma_map_ops->set_dma_mask?


> [PATCH 2/2] powerpc/dart_iommu: Support for 64-bit iommu bypass window on PCIe

^ 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