LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: How to define an I2C-to-SPI bridge device ?
From: Grant Likely @ 2011-03-31  3:43 UTC (permalink / raw)
  To: Andre Schwarz; +Cc: LinuxPPC List
In-Reply-To: <4D92071E.3060906@matrix-vision.de>

On Tue, Mar 29, 2011 at 06:21:50PM +0200, Andre Schwarz wrote:
> On 03/25/2011 10:28 AM, Andre Schwarz wrote:
> >Grant, Anton,
> 
> got it.
> providing modalis = "spidev" within spi_board_info works like a charme ...

Good.  Glad you got it sorted out.

g.

^ permalink raw reply

* Re: mmotm threatens ppc preemption again
From: Benjamin Herrenschmidt @ 2011-03-31  0:52 UTC (permalink / raw)
  To: Jeremy Fitzhardinge
  Cc: linuxppc-dev, Andrew Morton, Hugh Dickins, Peter Zijlstra
In-Reply-To: <4D939B88.5020707@goop.org>

On Wed, 2011-03-30 at 14:07 -0700, Jeremy Fitzhardinge wrote:
> 
>    1. Change the locking rules for kernel updates to also require a pte lock
>    2. Special-case batched kernel updates to include a pte lock
>    3. Make Power deal with preemption during a batched kernel update
>    4. Never do batched kernel updates on Power
>    5. Never do batched kernel updates (the current state) 

We deal with preemption already since the PTL turns into a mutex on -rt,
so we could bring that patch into mainline. The easiest approach however
for now would be to not do the kernel batched updates on kernel
(solution 4), and I can sort it out later if I want to enable it.

The problem is that it's hard for me to "fix" that with the current
accessors as arch_enter_lazy_mmu_mode() don't get any argument that
could point me to which mm is being operated on.

Jeremy, I haven't had a chance to look at your patches in detail, do
you just use those accessors or do you create new ones for batching
kernel updates in which case powerpc could just make them do nothing ?

Else, we could have one patch that adds an mm argument accross the tree,
it shouldn't be too hard.

Later on, I can bring in the stuff from -rt stuff to enable lazy
batching of kernel pages on power if I wish to do so.

Cheers,
Ben

^ permalink raw reply

* Re: [PATCH v2 5/5] xhci: Remove recursive call to xhci_handle_event
From: Matt Evans @ 2011-03-30 23:10 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: sarah.a.sharp, linuxppc-dev, linux-usb, Sergei Shtylyov
In-Reply-To: <20110329200017.GA25480@dtor-ws.eng.vmware.com>

On 30/03/11 07:00, Dmitry Torokhov wrote:
> On Sunday, March 27, 2011 09:53:00 pm Matt Evans wrote:
>> @@ -2282,7 +2284,7 @@ hw_died:
>>  	/* FIXME this should be a delayed service routine
>>  	 * that clears the EHB.
>>  	 */
>> -	xhci_handle_event(xhci);
>> +	while (xhci_handle_event(xhci)) {}
>>
> 
> I must admit I dislike the style with empty loop bodies, do you think
> we could have something like below instead?

Well, although I don't mind empty while()s at all (they're clean and obvious
IMHO) I would remove an empty blightful while loop with something like this:

do {
	ret = xhci_handle_event(xhci);
} while (ret > 0);

;-) (Not sure that refactoring its contents into the IRQ handler is a good idea,
if that area's going to be revisited soon to extend error
handling/reporting.)

Cheers,


Matt

^ permalink raw reply

* Re: mmotm threatens ppc preemption again
From: Jeremy Fitzhardinge @ 2011-03-30 21:07 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linuxppc-dev, Hugh Dickins, Peter Zijlstra
In-Reply-To: <20110330135332.9c322e40.akpm@linux-foundation.org>

On 03/30/2011 01:53 PM, Andrew Morton wrote:
> On Mon, 21 Mar 2011 13:22:30 +1100
> Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
>> On Sun, 2011-03-20 at 19:20 -0700, Hugh Dickins wrote:
>>>> As long as the races to avoid are between map/unmap vs. access, yes, it
>>>> -should- be fine, and we used to not do demand faulting on kernel space
>>>> (but for how long ?). I'm wondering why we don't just stick a ptl in
>>>> there or is there a good reason why we can't ?
>>> We can - but we usually prefer to avoid unnecessary locking.
>>> An arch function which locks init_mm.page_table_lock on powerpc,
>>> but does nothing on others? 
>> That still means gratuitous differences between how the normal and
>> kernel page tables are handled. Maybe that's not worth bothering ...
> So what will we do here?  I still have
>
> mm-remove-unused-token-argument-from-apply_to_page_range-callback.patch
> mm-add-apply_to_page_range_batch.patch
> ioremap-use-apply_to_page_range_batch-for-ioremap_page_range.patch
> vmalloc-use-plain-pte_clear-for-unmaps.patch
> vmalloc-use-apply_to_page_range_batch-for-vunmap_page_range.patch
> vmalloc-use-apply_to_page_range_batch-for-vmap_page_range_noflush.patch
> vmalloc-use-apply_to_page_range_batch-in-alloc_vm_area.patch
> xen-mmu-use-apply_to_page_range_batch-in-xen_remap_domain_mfn_range.patch
> xen-grant-table-use-apply_to_page_range_batch.patch
>
> floating around and at some stage they may cause merge problems.

Well, my understanding of the situation is:

   1. There's a basic asymmetry between user and kernel pagetables, in
      that the former has a standard pte locking scheme, whereas the
      latter uses ad-hoc locking depending on what particular subsystem
      is doing the changes (presumably to its own private piece of
      kernel virtual address space)
   2. Power was assuming that all lazy mmu updates were done under
      spinlock, or are at least non-preemptable.  This is incompatible
      with 1), but it was moot because no kernel updates were done lazily
   3. These patches add the first instance of lazy mmu updates, which
      reveals the mismatch between Power's expectations and the actual
      locking rules.


So the options are:

   1. Change the locking rules for kernel updates to also require a pte lock
   2. Special-case batched kernel updates to include a pte lock
   3. Make Power deal with preemption during a batched kernel update
   4. Never do batched kernel updates on Power
   5. Never do batched kernel updates (the current state)


1 seems like a big change.
2 is pretty awkward, and has the side-effect of increasing preemption
latencies (since if you're doing enough updates to be worth batching,
you'll be disabling preemption for a longish time).
I don't know how complex 3 is; I guess it depends on the details of the
batched hashtable update thingy.
4 looks like it should be simple.
5 is the default do-nothing state, but it seems unfair on anyone who can
actually take advantage of batched updates.

Ben, how hard would something like 3 or 4 be to implement?

Thanks,
    J

^ permalink raw reply

* Re: mmotm threatens ppc preemption again
From: Andrew Morton @ 2011-03-30 20:53 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: linuxppc-dev, Jeremy Fitzhardinge, Hugh Dickins, Peter Zijlstra
In-Reply-To: <1300674150.2402.207.camel@pasglop>

On Mon, 21 Mar 2011 13:22:30 +1100
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Sun, 2011-03-20 at 19:20 -0700, Hugh Dickins wrote:
> > > As long as the races to avoid are between map/unmap vs. access, yes, it
> > > -should- be fine, and we used to not do demand faulting on kernel space
> > > (but for how long ?). I'm wondering why we don't just stick a ptl in
> > > there or is there a good reason why we can't ?
> > 
> > We can - but we usually prefer to avoid unnecessary locking.
> > An arch function which locks init_mm.page_table_lock on powerpc,
> > but does nothing on others? 
> 
> That still means gratuitous differences between how the normal and
> kernel page tables are handled. Maybe that's not worth bothering ...

So what will we do here?  I still have

mm-remove-unused-token-argument-from-apply_to_page_range-callback.patch
mm-add-apply_to_page_range_batch.patch
ioremap-use-apply_to_page_range_batch-for-ioremap_page_range.patch
vmalloc-use-plain-pte_clear-for-unmaps.patch
vmalloc-use-apply_to_page_range_batch-for-vunmap_page_range.patch
vmalloc-use-apply_to_page_range_batch-for-vmap_page_range_noflush.patch
vmalloc-use-apply_to_page_range_batch-in-alloc_vm_area.patch
xen-mmu-use-apply_to_page_range_batch-in-xen_remap_domain_mfn_range.patch
xen-grant-table-use-apply_to_page_range_batch.patch

floating around and at some stage they may cause merge problems.

^ permalink raw reply

* Re: [PATCH] POWER: perf_event: Skip updating kernel counters if register value shrinks
From: Eric B Munson @ 2011-03-30 18:36 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: a.p.zijlstra, linux-kernel, paulus, anton, acme, mingo,
	linuxppc-dev
In-Reply-To: <1301433165.2402.689.camel@pasglop>

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

On Wed, 30 Mar 2011, Benjamin Herrenschmidt wrote:

> On Tue, 2011-03-29 at 10:25 -0400, Eric B Munson wrote:
> > Here I made the assumption that the hardware would never remove more events in
> > a speculative roll back than it had added.  This is not a situation I
> > encoutered in my limited testing, so I didn't think underflow was possible.  I
> > will send out a V2 using the signed 32 bit delta and remeber to CC stable
> > this time. 
> 
> I'm not thinking about underflow but rollover... or that isn't possible
> with those counters ? IE. They don't wrap back to 0 after hitting
> ffffffff ?
> 

They do roll over to 0 after ffffffff, but I thought that case was already
covered by the perf_event_interrupt.  Are you concerned that we will reset a
counter and speculative roll back will underflow that counter?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply

* Re: checking status semantics with compatible functions
From: Grant Likely @ 2011-03-30 14:31 UTC (permalink / raw)
  To: Kumar Gala; +Cc: Linuxppc-dev list, devicetree-discuss, Prabhakar Kushwaha
In-Reply-To: <C1B3133E-0EEE-49E9-9E3A-35026CFA78E9@kernel.crashing.org>

On Wed, Mar 30, 2011 at 03:26:29AM -0500, Kumar Gala wrote:
> [Adding proper lists on CC]
> 
> - k
> 
> On Mar 30, 2011, at 3:07 AM, Kumar Gala wrote:
> 
> > Grant,
> > 
> > We have a few helper functions like:
> > 
> > drivers/of/base.c:EXPORT_SYMBOL(of_device_is_compatible);
> > drivers/of/base.c:EXPORT_SYMBOL(of_find_compatible_node);
> > 
> > That we use in places like arch/powerpc/platform/85xx/mpc85xx_ds.c:
> > 
> >        for_each_node_by_type(np, "pci") {
> >                if (of_device_is_compatible(np, "fsl,mpc8540-pci") ||
> >                    of_device_is_compatible(np, "fsl,mpc8548-pcie") ||
> >                    of_device_is_compatible(np, "fsl,p2020-pcie")) {
> > 
> > It seems like we should check 'status' in these cases with
> > 'of_device_is_available'.  I'm thinking that we should have
> > versions of the helpers that also do the 'of_device_is_available'
> > checking.  Or should we just change the helpers to call
> > of_device_is_available?

Yes, of_device_is_available() should be checked, but it should not be
added directly to of_device_is_compatible().  I'm okay with adding
a helper variant that does the of_device_is_compatible() check.

In that particular case, I'd also suggest using for_each_matching_node().

g.

^ permalink raw reply

* Re: [PATCH] powerpc: rename get_irq_desc_data and get_irq_desc_chip
From: Thomas Gleixner @ 2011-03-30 13:32 UTC (permalink / raw)
  To: Richard Cochran; +Cc: Lennert Buytenhek, linuxppc-dev, linux-kernel
In-Reply-To: <20110330132504.GA31832@riccoc20.at.omicron.at>

On Wed, 30 Mar 2011, Richard Cochran wrote:

> These two functions disappeared in commit
> 
>     0c6f8a8b917ad361319c8ace3e9f28e69bfdb4c1
>     "genirq: Remove compat code"
> 
> but they still exist in qe_ic.h.
> This patch renames the function to their new names.

oops. I really need to figure out to teach coccinelle not to ignore
header files.
 

^ permalink raw reply

* [PATCH] powerpc: rename get_irq_desc_data and get_irq_desc_chip
From: Richard Cochran @ 2011-03-30 13:25 UTC (permalink / raw)
  To: linux-kernel; +Cc: Lennert Buytenhek, Thomas Gleixner, linuxppc-dev

These two functions disappeared in commit

    0c6f8a8b917ad361319c8ace3e9f28e69bfdb4c1
    "genirq: Remove compat code"

but they still exist in qe_ic.h.
This patch renames the function to their new names.

Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
 arch/powerpc/include/asm/qe_ic.h |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/include/asm/qe_ic.h b/arch/powerpc/include/asm/qe_ic.h
index 9e2cb20..f706164 100644
--- a/arch/powerpc/include/asm/qe_ic.h
+++ b/arch/powerpc/include/asm/qe_ic.h
@@ -81,7 +81,7 @@ int qe_ic_set_high_priority(unsigned int virq, unsigned int priority, int high);
 static inline void qe_ic_cascade_low_ipic(unsigned int irq,
 					  struct irq_desc *desc)
 {
-	struct qe_ic *qe_ic = get_irq_desc_data(desc);
+	struct qe_ic *qe_ic = irq_desc_get_handler_data(desc);
 	unsigned int cascade_irq = qe_ic_get_low_irq(qe_ic);
 
 	if (cascade_irq != NO_IRQ)
@@ -91,7 +91,7 @@ static inline void qe_ic_cascade_low_ipic(unsigned int irq,
 static inline void qe_ic_cascade_high_ipic(unsigned int irq,
 					   struct irq_desc *desc)
 {
-	struct qe_ic *qe_ic = get_irq_desc_data(desc);
+	struct qe_ic *qe_ic = irq_desc_get_handler_data(desc);
 	unsigned int cascade_irq = qe_ic_get_high_irq(qe_ic);
 
 	if (cascade_irq != NO_IRQ)
@@ -101,9 +101,9 @@ static inline void qe_ic_cascade_high_ipic(unsigned int irq,
 static inline void qe_ic_cascade_low_mpic(unsigned int irq,
 					  struct irq_desc *desc)
 {
-	struct qe_ic *qe_ic = get_irq_desc_data(desc);
+	struct qe_ic *qe_ic = irq_desc_get_handler_data(desc);
 	unsigned int cascade_irq = qe_ic_get_low_irq(qe_ic);
-	struct irq_chip *chip = get_irq_desc_chip(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
 
 	if (cascade_irq != NO_IRQ)
 		generic_handle_irq(cascade_irq);
@@ -114,9 +114,9 @@ static inline void qe_ic_cascade_low_mpic(unsigned int irq,
 static inline void qe_ic_cascade_high_mpic(unsigned int irq,
 					   struct irq_desc *desc)
 {
-	struct qe_ic *qe_ic = get_irq_desc_data(desc);
+	struct qe_ic *qe_ic = irq_desc_get_handler_data(desc);
 	unsigned int cascade_irq = qe_ic_get_high_irq(qe_ic);
-	struct irq_chip *chip = get_irq_desc_chip(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
 
 	if (cascade_irq != NO_IRQ)
 		generic_handle_irq(cascade_irq);
@@ -127,9 +127,9 @@ static inline void qe_ic_cascade_high_mpic(unsigned int irq,
 static inline void qe_ic_cascade_muxed_mpic(unsigned int irq,
 					    struct irq_desc *desc)
 {
-	struct qe_ic *qe_ic = get_irq_desc_data(desc);
+	struct qe_ic *qe_ic = irq_desc_get_handler_data(desc);
 	unsigned int cascade_irq;
-	struct irq_chip *chip = get_irq_desc_chip(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
 
 	cascade_irq = qe_ic_get_high_irq(qe_ic);
 	if (cascade_irq == NO_IRQ)
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH v4]PPC4xx: Adding PCI(E) MSI support
From: Rupjyoti Sarmah @ 2011-03-30  9:10 UTC (permalink / raw)
  To: linuxppc-dev, linux-kernel; +Cc: rsarmah

This patch adds MSI support for 440SPe, 460Ex, 460Sx and 405Ex.

Signed-off-by: Rupjyoti Sarmah <rsarmah@apm.com>
Signed-off-by: Tirumala R Marri <tmarri@apm.com>
---
v4:
 * Updated the coding style as per  recommendation by Philipp
 * The goto style code removed as per suggestion
v3:
 * Rebased to Josh next tree
 * Cleanup and remove some unwanted log msg.
 * Remove list member and its references.
 * Keep msi_data local reference.
v2:
  * Remove or add blank lines at appropriate places.
  * Added BITMAP as it is easy to request and free the MSIs
  * Removed UPPER_4BITS_OF36BIT & LOWER_32BITS_OF36BIT;
  * Remove unused feature variable.
  * Remove initialization of "virq".
  * remove static int_no varaible and replace with bitmap.
  * Eliminated reading count from DTS tree and added a macro.
  * Remove printK.
  * Remove else in setup_irqs.
  * Free interrupts in teardown_msi_interrupts().
  * Print contraints in check_device().
  * Replace ioremap with of_iomap().
  * Use msi_data in setup_pcieh_hw().
  * Don't unmap in the setup_pcieh_hw().
  * don't use WARN_ON.
  * Remove ppc4xx_msi_ids[].
---
 arch/powerpc/boot/dts/canyonlands.dts |   18 ++
 arch/powerpc/boot/dts/katmai.dts      |   18 ++
 arch/powerpc/boot/dts/kilauea.dts     |   28 ++++
 arch/powerpc/boot/dts/redwood.dts     |   20 +++
 arch/powerpc/platforms/40x/Kconfig    |    2 +
 arch/powerpc/platforms/44x/Kconfig    |    6 +
 arch/powerpc/sysdev/Kconfig           |    7 +
 arch/powerpc/sysdev/Makefile          |    1 +
 arch/powerpc/sysdev/ppc4xx_msi.c      |  277 +++++++++++++++++++++++++++++++++
 9 files changed, 377 insertions(+), 0 deletions(-)
 create mode 100644 arch/powerpc/sysdev/ppc4xx_msi.c

diff --git a/arch/powerpc/boot/dts/canyonlands.dts b/arch/powerpc/boot/dts/canyonlands.dts
index 2779f08..22dd6ae 100644
--- a/arch/powerpc/boot/dts/canyonlands.dts
+++ b/arch/powerpc/boot/dts/canyonlands.dts
@@ -530,5 +530,23 @@
 				0x0 0x0 0x0 0x3 &UIC3 0x12 0x4 /* swizzled int C */
 				0x0 0x0 0x0 0x4 &UIC3 0x13 0x4 /* swizzled int D */>;
 		};
+
+		MSI: ppc4xx-msi@C10000000 {
+			compatible = "amcc,ppc4xx-msi", "ppc4xx-msi";
+			reg = < 0xC 0x10000000 0x100>;
+			sdr-base = <0x36C>;
+			msi-data = <0x00000000>;
+			msi-mask = <0x44440000>;
+			interrupt-count = <3>;
+			interrupts = <0 1 2 3>;
+			interrupt-parent = <&UIC3>;
+			#interrupt-cells = <1>;
+			#address-cells = <0>;
+			#size-cells = <0>;
+			interrupt-map = <0 &UIC3 0x18 1
+					1 &UIC3 0x19 1
+					2 &UIC3 0x1A 1
+					3 &UIC3 0x1B 1>;
+		};
 	};
 };
diff --git a/arch/powerpc/boot/dts/katmai.dts b/arch/powerpc/boot/dts/katmai.dts
index 7c3be5e..f913dbe 100644
--- a/arch/powerpc/boot/dts/katmai.dts
+++ b/arch/powerpc/boot/dts/katmai.dts
@@ -442,6 +442,24 @@
 				0x0 0x0 0x0 0x4 &UIC3 0xb 0x4 /* swizzled int D */>;
 		};
 
+		MSI: ppc4xx-msi@400300000 {
+				compatible = "amcc,ppc4xx-msi", "ppc4xx-msi";
+				reg = < 0x4 0x00300000 0x100>;
+				sdr-base = <0x3B0>;
+				msi-data = <0x00000000>;
+				msi-mask = <0x44440000>;
+				interrupt-count = <3>;
+				interrupts =<0 1 2 3>;
+				interrupt-parent = <&UIC0>;
+				#interrupt-cells = <1>;
+				#address-cells = <0>;
+				#size-cells = <0>;
+				interrupt-map = <0 &UIC0 0xC 1
+					1 &UIC0 0x0D 1
+					2 &UIC0 0x0E 1
+					3 &UIC0 0x0F 1>;
+		};
+
 		I2O: i2o@400100000 {
 			compatible = "ibm,i2o-440spe";
 			reg = <0x00000004 0x00100000 0x100>;
diff --git a/arch/powerpc/boot/dts/kilauea.dts b/arch/powerpc/boot/dts/kilauea.dts
index 89edb16..1613d6e 100644
--- a/arch/powerpc/boot/dts/kilauea.dts
+++ b/arch/powerpc/boot/dts/kilauea.dts
@@ -403,5 +403,33 @@
 				0x0 0x0 0x0 0x3 &UIC2 0xd 0x4 /* swizzled int C */
 				0x0 0x0 0x0 0x4 &UIC2 0xe 0x4 /* swizzled int D */>;
 		};
+
+		MSI: ppc4xx-msi@C10000000 {
+			compatible = "amcc,ppc4xx-msi", "ppc4xx-msi";
+			reg = < 0x0 0xEF620000 0x100>;
+			sdr-base = <0x4B0>;
+			msi-data = <0x00000000>;
+			msi-mask = <0x44440000>;
+			interrupt-count = <12>;
+			interrupts = <0 1 2 3 4 5 6 7 8 9 0xA 0xB 0xC 0xD>;
+			interrupt-parent = <&UIC2>;
+			#interrupt-cells = <1>;
+			#address-cells = <0>;
+			#size-cells = <0>;
+			interrupt-map = <0 &UIC2 0x10 1
+					1 &UIC2 0x11 1
+					2 &UIC2 0x12 1
+					2 &UIC2 0x13 1
+					2 &UIC2 0x14 1
+					2 &UIC2 0x15 1
+					2 &UIC2 0x16 1
+					2 &UIC2 0x17 1
+					2 &UIC2 0x18 1
+					2 &UIC2 0x19 1
+					2 &UIC2 0x1A 1
+					2 &UIC2 0x1B 1
+					2 &UIC2 0x1C 1
+					3 &UIC2 0x1D 1>;
+		};
 	};
 };
diff --git a/arch/powerpc/boot/dts/redwood.dts b/arch/powerpc/boot/dts/redwood.dts
index 81636c0..d86a3a4 100644
--- a/arch/powerpc/boot/dts/redwood.dts
+++ b/arch/powerpc/boot/dts/redwood.dts
@@ -358,8 +358,28 @@
 				0x0 0x0 0x0 0x4 &UIC3 0xb 0x4 /* swizzled int D */>;
 		};
 
+		MSI: ppc4xx-msi@400300000 {
+				compatible = "amcc,ppc4xx-msi", "ppc4xx-msi";
+				reg = < 0x4 0x00300000 0x100
+					0x4 0x00300000 0x100>;
+				sdr-base = <0x3B0>;
+				msi-data = <0x00000000>;
+				msi-mask = <0x44440000>;
+				interrupt-count = <3>;
+				interrupts =<0 1 2 3>;
+				interrupt-parent = <&UIC0>;
+				#interrupt-cells = <1>;
+				#address-cells = <0>;
+				#size-cells = <0>;
+				interrupt-map = <0 &UIC0 0xC 1
+					1 &UIC0 0x0D 1
+					2 &UIC0 0x0E 1
+					3 &UIC0 0x0F 1>;
+		};
+
 	};
 
+
 	chosen {
 		linux,stdout-path = "/plb/opb/serial@ef600200";
 	};
diff --git a/arch/powerpc/platforms/40x/Kconfig b/arch/powerpc/platforms/40x/Kconfig
index b721764..92aeee6 100644
--- a/arch/powerpc/platforms/40x/Kconfig
+++ b/arch/powerpc/platforms/40x/Kconfig
@@ -57,6 +57,8 @@ config KILAUEA
 	select 405EX
 	select PPC40x_SIMPLE
 	select PPC4xx_PCI_EXPRESS
+	select PCI_MSI
+	select 4xx_MSI
 	help
 	  This option enables support for the AMCC PPC405EX evaluation board.
 
diff --git a/arch/powerpc/platforms/44x/Kconfig b/arch/powerpc/platforms/44x/Kconfig
index f485fc5..f10cc7f 100644
--- a/arch/powerpc/platforms/44x/Kconfig
+++ b/arch/powerpc/platforms/44x/Kconfig
@@ -74,6 +74,8 @@ config KATMAI
 	select 440SPe
 	select PCI
 	select PPC4xx_PCI_EXPRESS
+	select PCI_MSI
+	select 4xx_MSI
 	help
 	  This option enables support for the AMCC PPC440SPe evaluation board.
 
@@ -118,6 +120,8 @@ config CANYONLANDS
 	select 460EX
 	select PCI
 	select PPC4xx_PCI_EXPRESS
+	select PCI_MSI
+	select 4xx_MSI
 	select IBM_NEW_EMAC_RGMII
 	select IBM_NEW_EMAC_ZMII
 	help
@@ -144,6 +148,8 @@ config REDWOOD
 	select 460SX
 	select PCI
 	select PPC4xx_PCI_EXPRESS
+	select PCI_MSI
+	select 4xx_MSI
 	help
 	  This option enables support for the AMCC PPC460SX Redwood board.
 
diff --git a/arch/powerpc/sysdev/Kconfig b/arch/powerpc/sysdev/Kconfig
index 3965828..32f5a40 100644
--- a/arch/powerpc/sysdev/Kconfig
+++ b/arch/powerpc/sysdev/Kconfig
@@ -7,8 +7,15 @@ config PPC4xx_PCI_EXPRESS
 	depends on PCI && 4xx
 	default n
 
+config 4xx_MSI
+	bool
+	depends on PCI_MSI
+	depends on PCI && 4xx
+	default n
+
 config PPC_MSI_BITMAP
 	bool
 	depends on PCI_MSI
 	default y if MPIC
 	default y if FSL_PCI
+	default y if 4xx_MSI
diff --git a/arch/powerpc/sysdev/Makefile b/arch/powerpc/sysdev/Makefile
index 1e0c933..5d92e29 100644
--- a/arch/powerpc/sysdev/Makefile
+++ b/arch/powerpc/sysdev/Makefile
@@ -41,6 +41,7 @@ obj-$(CONFIG_OF_RTC)		+= of_rtc.o
 ifeq ($(CONFIG_PCI),y)
 obj-$(CONFIG_4xx)		+= ppc4xx_pci.o
 endif
+obj-$(CONFIG_4xx_MSI)		+= ppc4xx_msi.o
 obj-$(CONFIG_PPC4xx_CPM)	+= ppc4xx_cpm.o
 obj-$(CONFIG_PPC4xx_GPIO)	+= ppc4xx_gpio.o
 
diff --git a/arch/powerpc/sysdev/ppc4xx_msi.c b/arch/powerpc/sysdev/ppc4xx_msi.c
new file mode 100644
index 0000000..ab69edb
--- /dev/null
+++ b/arch/powerpc/sysdev/ppc4xx_msi.c
@@ -0,0 +1,277 @@
+/*
+ * Adding PCI-E MSI support for PPC4XX SoCs.
+ *
+ * Copyright (c) 2010, Applied Micro Circuits Corporation
+ * Authors:	Tirumala R Marri <tmarri@apm.com>
+ *		Feng Kan <fkan@apm.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <linux/irq.h>
+#include <linux/bootmem.h>
+#include <linux/pci.h>
+#include <linux/msi.h>
+#include <linux/of_platform.h>
+#include <linux/interrupt.h>
+#include <asm/prom.h>
+#include <asm/hw_irq.h>
+#include <asm/ppc-pci.h>
+#include <boot/dcr.h>
+#include <asm/dcr-regs.h>
+#include <asm/msi_bitmap.h>
+
+#define PEIH_TERMADH	0x00
+#define PEIH_TERMADL	0x08
+#define PEIH_MSIED	0x10
+#define PEIH_MSIMK	0x18
+#define PEIH_MSIASS	0x20
+#define PEIH_FLUSH0	0x30
+#define PEIH_FLUSH1	0x38
+#define PEIH_CNTRST	0x48
+#define NR_MSI_IRQS	4
+
+struct ppc4xx_msi {
+	u32 msi_addr_lo;
+	u32 msi_addr_hi;
+	void __iomem *msi_regs;
+	int msi_virqs[NR_MSI_IRQS];
+	struct msi_bitmap bitmap;
+	struct device_node *msi_dev;
+};
+
+static struct ppc4xx_msi ppc4xx_msi;
+
+static int ppc4xx_msi_init_allocator(struct platform_device *dev,
+		struct ppc4xx_msi *msi_data)
+{
+	int err;
+
+	err = msi_bitmap_alloc(&msi_data->bitmap, NR_MSI_IRQS,
+			      dev->dev.of_node);
+	if (err)
+		return err;
+
+	err = msi_bitmap_reserve_dt_hwirqs(&msi_data->bitmap);
+	if (err < 0) {
+		msi_bitmap_free(&msi_data->bitmap);
+		return err;
+	}
+
+	return 0;
+}
+
+static int ppc4xx_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
+{
+	int int_no = -ENOMEM;
+	unsigned int virq;
+	struct msi_msg msg;
+	struct msi_desc *entry;
+	struct ppc4xx_msi *msi_data = &ppc4xx_msi;
+
+	list_for_each_entry(entry, &dev->msi_list, list) {
+		int_no = msi_bitmap_alloc_hwirqs(&msi_data->bitmap, 1);
+		if (int_no >= 0)
+			break;
+		if (int_no < 0) {
+			pr_debug("%s: fail allocating msi interrupt\n",
+					__func__);
+		}
+		virq = irq_of_parse_and_map(msi_data->msi_dev, int_no);
+		if (virq == NO_IRQ) {
+			dev_err(&dev->dev, "%s: fail mapping irq\n", __func__);
+			msi_bitmap_free_hwirqs(&msi_data->bitmap, int_no, 1);
+			return -ENOSPC;
+		}
+		set_irq_data(virq, (void *)int_no);
+		dev_dbg(&dev->dev, "%s: virq = %d\n", __func__, virq);
+
+		/* Setup msi address space */
+		msg.address_hi = msi_data->msi_addr_hi;
+		msg.address_lo = msi_data->msi_addr_lo;
+
+		set_irq_msi(virq, entry);
+		msg.data = int_no;
+		write_msi_msg(virq, &msg);
+	}
+	return 0;
+}
+
+void ppc4xx_teardown_msi_irqs(struct pci_dev *dev)
+{
+	struct msi_desc *entry;
+	struct ppc4xx_msi *msi_data = &ppc4xx_msi;
+
+	dev_dbg(&dev->dev, "PCIE-MSI: tearing down msi irqs\n");
+
+	list_for_each_entry(entry, &dev->msi_list, list) {
+		if (entry->irq == NO_IRQ)
+			continue;
+		set_irq_msi(entry->irq, NULL);
+		msi_bitmap_free_hwirqs(&msi_data->bitmap,
+				virq_to_hw(entry->irq), 1);
+		irq_dispose_mapping(entry->irq);
+	}
+}
+
+static int ppc4xx_msi_check_device(struct pci_dev *pdev, int nvec, int type)
+{
+	dev_dbg(&pdev->dev, "PCIE-MSI:%s called. vec %x type %d\n",
+		__func__, nvec, type);
+	if (type == PCI_CAP_ID_MSIX)
+		pr_debug("ppc4xx msi: MSI-X untested, trying anyway.\n");
+
+	return 0;
+}
+
+static int ppc4xx_setup_pcieh_hw(struct platform_device *dev,
+				 struct resource res, struct ppc4xx_msi *msi)
+{
+	const u32 *msi_data;
+	const u32 *msi_mask;
+	const u32 *sdr_addr;
+	dma_addr_t msi_phys;
+	void *msi_virt;
+
+	sdr_addr = of_get_property(dev->dev.of_node, "sdr-base", NULL);
+	if (!sdr_addr)
+		return -1;
+
+	SDR0_WRITE(sdr_addr, (u64)res.start >> 32);	 /*HIGH addr */
+	SDR0_WRITE(sdr_addr + 1, res.start & 0xFFFFFFFF); /* Low addr */
+
+
+	msi->msi_dev = of_find_node_by_name(NULL, "ppc4xx-msi");
+	if (msi->msi_dev)
+		return -ENODEV;
+
+	msi->msi_regs = of_iomap(msi->msi_dev, 0);
+	if (!msi->msi_regs) {
+		dev_err(&dev->dev, "of_iomap problem failed\n");
+		return -ENOMEM;
+	}
+	dev_dbg(&dev->dev, "PCIE-MSI: msi register mapped 0x%x 0x%x\n",
+		(u32) (msi->msi_regs + PEIH_TERMADH), (u32) (msi->msi_regs));
+
+	msi_virt = dma_alloc_coherent(&dev->dev, 64, &msi_phys, GFP_KERNEL);
+	msi->msi_addr_hi = 0x0;
+	msi->msi_addr_lo = (u32) msi_phys;
+	dev_dbg(&dev->dev, "PCIE-MSI: msi address 0x%x\n", msi->msi_addr_lo);
+
+	/* Progam the Interrupt handler Termination addr registers */
+	out_be32(msi->msi_regs + PEIH_TERMADH, msi->msi_addr_hi);
+	out_be32(msi->msi_regs + PEIH_TERMADL, msi->msi_addr_lo);
+
+	msi_data = of_get_property(dev->dev.of_node, "msi-data", NULL);
+	if (!msi_data)
+		return -1;
+	msi_mask = of_get_property(dev->dev.of_node, "msi-mask", NULL);
+	if (!msi_mask)
+		return -1;
+	/* Program MSI Expected data and Mask bits */
+	out_be32(msi->msi_regs + PEIH_MSIED, *msi_data);
+	out_be32(msi->msi_regs + PEIH_MSIMK, *msi_mask);
+
+	return 0;
+}
+
+static int ppc4xx_of_msi_remove(struct platform_device *dev)
+{
+	struct ppc4xx_msi *msi = dev->dev.platform_data;
+	int i;
+	int virq;
+
+	for (i = 0; i < NR_MSI_IRQS; i++) {
+		virq = msi->msi_virqs[i];
+		if (virq != NO_IRQ)
+			irq_dispose_mapping(virq);
+	}
+
+	if (msi->bitmap.bitmap)
+		msi_bitmap_free(&msi->bitmap);
+	iounmap(msi->msi_regs);
+	of_node_put(msi->msi_dev);
+	kfree(msi);
+
+	return 0;
+}
+
+static int __devinit ppc4xx_msi_probe(struct platform_device *dev)
+{
+	struct ppc4xx_msi *msi;
+	struct resource res;
+	int err = 0;
+
+	msi = &ppc4xx_msi;/*keep the msi data for further use*/
+
+	dev_dbg(&dev->dev, "PCIE-MSI: Setting up MSI support...\n");
+
+	msi = kzalloc(sizeof(struct ppc4xx_msi), GFP_KERNEL);
+	if (!msi) {
+		dev_err(&dev->dev, "No memory for MSI structure\n");
+		return -ENOMEM;
+	}
+	dev->dev.platform_data = msi;
+
+	/* Get MSI ranges */
+	err = of_address_to_resource(dev->dev.of_node, 0, &res);
+	if (err) {
+		dev_err(&dev->dev, "%s resource error!\n",
+			dev->dev.of_node->full_name);
+		goto error_out;
+	}
+
+	if (ppc4xx_setup_pcieh_hw(dev, res, msi))
+		goto error_out;
+
+	err = ppc4xx_msi_init_allocator(dev, msi);
+	if (err) {
+		dev_err(&dev->dev, "Error allocating MSI bitmap\n");
+		goto error_out;
+	}
+
+	ppc_md.setup_msi_irqs = ppc4xx_setup_msi_irqs;
+	ppc_md.teardown_msi_irqs = ppc4xx_teardown_msi_irqs;
+	ppc_md.msi_check_device = ppc4xx_msi_check_device;
+	return err;
+
+error_out:
+	ppc4xx_of_msi_remove(dev);
+	return err;
+}
+static const struct of_device_id ppc4xx_msi_ids[] = {
+	{
+		.compatible = "amcc,ppc4xx-msi",
+	},
+	{}
+};
+static struct platform_driver ppc4xx_msi_driver = {
+	.probe = ppc4xx_msi_probe,
+	.remove = ppc4xx_of_msi_remove,
+	.driver = {
+		   .name = "ppc4xx-msi",
+		   .owner = THIS_MODULE,
+		   .of_match_table = ppc4xx_msi_ids,
+		   },
+
+};
+
+static __init int ppc4xx_msi_init(void)
+{
+	return platform_driver_register(&ppc4xx_msi_driver);
+}
+
+subsys_initcall(ppc4xx_msi_init);
-- 
1.5.6.3

^ permalink raw reply related

* Re: checking status semantics with compatible functions
From: Kumar Gala @ 2011-03-30  8:26 UTC (permalink / raw)
  To: Kumar Gala; +Cc: Linuxppc-dev list, devicetree-discuss, Prabhakar Kushwaha
In-Reply-To: <9BC4C90E-090F-4961-A242-10D769C08738@kernel.crashing.org>

[Adding proper lists on CC]

- k

On Mar 30, 2011, at 3:07 AM, Kumar Gala wrote:

> Grant,
>=20
> We have a few helper functions like:
>=20
> drivers/of/base.c:EXPORT_SYMBOL(of_device_is_compatible);
> drivers/of/base.c:EXPORT_SYMBOL(of_find_compatible_node);
>=20
> That we use in places like arch/powerpc/platform/85xx/mpc85xx_ds.c:
>=20
>        for_each_node_by_type(np, "pci") {
>                if (of_device_is_compatible(np, "fsl,mpc8540-pci") ||
>                    of_device_is_compatible(np, "fsl,mpc8548-pcie") ||
>                    of_device_is_compatible(np, "fsl,p2020-pcie")) {
>=20
> It seems like we should check 'status' in these cases with =
'of_device_is_available'.  I'm thinking that we should have versions of =
the helpers that also do the 'of_device_is_available' checking.  Or =
should we just change the helpers to call of_device_is_available?
>=20
> - k
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

^ permalink raw reply

* Re: [PATCH] powerpc: dtc: remove obsolete .gitignore entries
From: Wolfram Sang @ 2011-03-30  8:05 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <1299948250-32084-1-git-send-email-w.sang@pengutronix.de>

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

On Sat, Mar 12, 2011 at 05:44:10PM +0100, Wolfram Sang wrote:
> dtc was moved and .gitignores have been added to the new location. So, we can
> delete the old, forgotten ones.
> 
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Ping. Is this one more for Ben or more for Grant?

> ---
>  arch/powerpc/boot/.gitignore         |    1 -
>  arch/powerpc/boot/dtc-src/.gitignore |    3 ---
>  2 files changed, 0 insertions(+), 4 deletions(-)
>  delete mode 100644 arch/powerpc/boot/dtc-src/.gitignore
> 
> diff --git a/arch/powerpc/boot/.gitignore b/arch/powerpc/boot/.gitignore
> index 3d80c3e..12da77e 100644
> --- a/arch/powerpc/boot/.gitignore
> +++ b/arch/powerpc/boot/.gitignore
> @@ -1,5 +1,4 @@
>  addnote
> -dtc
>  empty.c
>  hack-coff
>  infblock.c
> diff --git a/arch/powerpc/boot/dtc-src/.gitignore b/arch/powerpc/boot/dtc-src/.gitignore
> deleted file mode 100644
> index a7c3f94..0000000
> --- a/arch/powerpc/boot/dtc-src/.gitignore
> +++ /dev/null
> @@ -1,3 +0,0 @@
> -dtc-lexer.lex.c
> -dtc-parser.tab.c
> -dtc-parser.tab.h
> -- 
> 1.7.2.3
> 

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* RE: [PATCH v2 5/5] xhci: Remove recursive call to xhci_handle_event
From: David Laight @ 2011-03-30  7:51 UTC (permalink / raw)
  To: Dmitry Torokhov, Matt Evans
  Cc: sarah.a.sharp, linuxppc-dev, linux-usb, Sergei Shtylyov
In-Reply-To: <20110329200017.GA25480@dtor-ws.eng.vmware.com>

=20
> > -	xhci_handle_event(xhci);
> > +	while (xhci_handle_event(xhci)) {}
> >=20
>=20
> I must admit I dislike the style with empty loop bodies...

I would use an explicit 'continue;' for the body
of an otherwise empty loop.

	David

^ permalink raw reply

* [git pull] Please pull powerpc.git merge branch
From: Benjamin Herrenschmidt @ 2011-03-30  2:08 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linuxppc-dev list, Andrew Morton, Linux Kernel list

Hi Linus !

Here are a few post -rc1 fixes for powerpc.

Mostly some more fallout from the irq core rework, new syscalls wireup,
defconfig fixes and minor bits. The only feature-esque thing in there
would be me adding dma_mmap_coherent(), it fixes various audio devices
on our older non cache-coherent embedded platforms.

Freescale folks: I've manually added two patches to fix some FSL 8xx and
QE irq build problems caused by the upstream irq rework to the branch.
Please verify that it's all correct and if not send followup patches.

Linus: I didn't wait for hera to sync to the mirrors before sending this
email (I usually do but I have to run and it's very slow today).

The following changes since commit 0ce790e7d736cedc563e1fb4e998babf5a4dbc3d:

  Linux 2.6.39-rc1 (2011-03-29 12:09:47 -0700)

are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git ..BRANCH.NOT.VERIFIED..

Anton Blanchard (2):
      powerpc: Fix accounting of softirq time when idle
      powerpc/pseries: Enable Chelsio network and iWARP drivers

Benjamin Herrenschmidt (4):
      powerpc: Implement dma_mmap_coherent()
      powerpc/mm: Move the STAB0 location to 0x8000 to make room in low memory
      powerpc/8xx: Fix another breakage from the irq_data conversion
      powerpc/qe_ic: Fix another breakage from the irq_data conversion

Jim Keniston (1):
      powerpc/nvram: Don't overwrite oops/panic report on normal shutdown

Milton Miller (3):
      powerpc/xics: Fix numberspace mismatch from irq_desc conversion
      powerpc/xics: Use hwirq for xics domain irq number
      powerpc/pseries/smp: query-cpu-stopped-state support won't change

Scott Wood (1):
      powerpc: ARCH_PFN_OFFSET should be unsigned long

Stephen Rothwell (2):
      powerpc: Restore some misc devices to our configs
      powerpc: Wire up new syscalls

Thomas Gleixner (1):
      powerpc/cell: Use handle_edge_eoi_irq for real

Varun Sethi (1):
      powerpc/booke: Correct the SPRN_MAS5 definition.

 arch/powerpc/configs/44x/warp_defconfig          |    1 +
 arch/powerpc/configs/52xx/motionpro_defconfig    |    1 +
 arch/powerpc/configs/86xx/gef_ppc9a_defconfig    |    1 +
 arch/powerpc/configs/86xx/gef_sbc310_defconfig   |    1 +
 arch/powerpc/configs/86xx/gef_sbc610_defconfig   |    1 +
 arch/powerpc/configs/86xx/mpc8641_hpcn_defconfig |    1 +
 arch/powerpc/configs/e55xx_smp_defconfig         |    1 +
 arch/powerpc/configs/linkstation_defconfig       |    1 +
 arch/powerpc/configs/mpc512x_defconfig           |    1 +
 arch/powerpc/configs/mpc5200_defconfig           |    1 +
 arch/powerpc/configs/mpc85xx_defconfig           |    1 +
 arch/powerpc/configs/mpc85xx_smp_defconfig       |    1 +
 arch/powerpc/configs/mpc86xx_defconfig           |    1 +
 arch/powerpc/configs/pasemi_defconfig            |    1 +
 arch/powerpc/configs/ppc6xx_defconfig            |    1 +
 arch/powerpc/configs/pseries_defconfig           |    3 +
 arch/powerpc/include/asm/dma-mapping.h           |    6 ++
 arch/powerpc/include/asm/mmu-hash64.h            |    2 +-
 arch/powerpc/include/asm/page.h                  |    2 +-
 arch/powerpc/include/asm/qe_ic.h                 |   16 +++---
 arch/powerpc/include/asm/reg_booke.h             |    2 +-
 arch/powerpc/include/asm/systbl.h                |    4 +
 arch/powerpc/include/asm/unistd.h                |    6 ++-
 arch/powerpc/kernel/dma.c                        |   18 ++++++
 arch/powerpc/kernel/exceptions-64s.S             |   28 ++++----
 arch/powerpc/kernel/time.c                       |    2 +-
 arch/powerpc/mm/dma-noncoherent.c                |   20 ++++++
 arch/powerpc/platforms/cell/interrupt.c          |    2 +-
 arch/powerpc/platforms/pseries/nvram.c           |   24 +++++++
 arch/powerpc/platforms/pseries/smp.c             |    4 +-
 arch/powerpc/platforms/pseries/xics.c            |   71 +++++++++++-----------
 arch/powerpc/sysdev/mpc8xx_pic.c                 |    2 +-
 32 files changed, 161 insertions(+), 66 deletions(-)

Cheers,
Ben.


 

^ permalink raw reply

* Re: linux-next: build failure after merge of the Linus' tree
From: Benjamin Herrenschmidt @ 2011-03-30  1:56 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: ppc-dev, Thomas Gleixner, linux-next, Linus, linux-kernel
In-Reply-To: <20110330110027.b83a1d20.sfr@canb.auug.org.au>

On Wed, 2011-03-30 at 11:00 +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the Linus' tree, today's linux-next build (powerpc
> ppc64_defconfig) failed like this:
> 
> kernel/irq/chip.c: In function 'handle_edge_eoi_irq':
> kernel/irq/chip.c:517: warning: label 'out_unlock' defined but not used
> kernel/irq/chip.c:503: error: label 'out_eoi' used but not defined

There's a fix in tip/irq/urgent ...

Cheers,
Ben.

> Caused by commit 0521c8fbb3da ("genirq: Provide edge_eoi flow handler")
> which was clearly not even built with CONFIG_IRQ_EDGE_EOI_HANDLER defined.
> 
> I applied this fixup patch:
> 
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Wed, 30 Mar 2011 10:55:12 +1100
> Subject: [PATCH] genirq: fix CONFIG_IRQ_EDGE_EOI_HANDLER build
> 
> Fixes these errors:
> 
> kernel/irq/chip.c: In function 'handle_edge_eoi_irq':
> kernel/irq/chip.c:517: warning: label 'out_unlock' defined but not used
> kernel/irq/chip.c:503: error: label 'out_eoi' used but not defined
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  kernel/irq/chip.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index 616ec1c..1dafc86 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -514,7 +514,7 @@ void handle_edge_eoi_irq(unsigned int irq, struct irq_desc *desc)
>  	} while ((desc->istate & IRQS_PENDING) &&
>  		 !irqd_irq_disabled(&desc->irq_data));
>  
> -out_unlock:
> +out_eoi:
>  	chip->irq_eoi(&desc->irq_data);
>  	raw_spin_unlock(&desc->lock);
>  }
> -- 
> 1.7.4.1
> 
> 

^ permalink raw reply

* Re: linux-next: build failure after merge of the Linus' tree
From: Benjamin Herrenschmidt @ 2011-03-30  1:57 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: ppc-dev, Thomas Gleixner, linux-next, Linus, linux-kernel
In-Reply-To: <20110330110045.48fb0ccf.sfr@canb.auug.org.au>

On Wed, 2011-03-30 at 11:00 +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the Linus' tree, today's linux-next build (powerpc
> ppc64_defconfig) failed like this:
> 
> arch/powerpc/platforms/cell/interrupt.c: In function 'iic_host_map':
> arch/powerpc/platforms/cell/interrupt.c:247: error: 'handle_iic_irq' undeclared (first use in this function)

There's a fix in powerpc "merge" for which I'm about to send Linus a
pull request :-)

Cheers,
Ben.

> Caused by commit f9ba4475f95b ("powerpc: cell: Use the core flow handler").
> 
> I applied the following patch:
> 
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Wed, 30 Mar 2011 10:48:28 +1100
> Subject: [PATCH] powerpc/cell: fixup for removal of handle_iic_irq
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  arch/powerpc/platforms/cell/interrupt.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/cell/interrupt.c b/arch/powerpc/platforms/cell/interrupt.c
> index a19bec0..44cfd1b 100644
> --- a/arch/powerpc/platforms/cell/interrupt.c
> +++ b/arch/powerpc/platforms/cell/interrupt.c
> @@ -244,7 +244,7 @@ static int iic_host_map(struct irq_host *h, unsigned int virq,
>  		break;
>  	case IIC_IRQ_TYPE_IOEXC:
>  		irq_set_chip_and_handler(virq, &iic_ioexc_chip,
> -					 handle_iic_irq);
> +					 handle_edge_eoi_irq);
>  		break;
>  	default:
>  		irq_set_chip_and_handler(virq, &iic_chip, handle_edge_eoi_irq);
> -- 
> 1.7.4.1
> 

^ permalink raw reply

* linux-next: build failure after merge of the Linus' tree
From: Stephen Rothwell @ 2011-03-30  0:00 UTC (permalink / raw)
  To: Linus; +Cc: Thomas Gleixner, linux-next, ppc-dev, linux-kernel

Hi all,

After merging the Linus' tree, today's linux-next build (powerpc
ppc64_defconfig) failed like this:

arch/powerpc/platforms/cell/interrupt.c: In function 'iic_host_map':
arch/powerpc/platforms/cell/interrupt.c:247: error: 'handle_iic_irq' undeclared (first use in this function)

Caused by commit f9ba4475f95b ("powerpc: cell: Use the core flow handler").

I applied the following patch:

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Wed, 30 Mar 2011 10:48:28 +1100
Subject: [PATCH] powerpc/cell: fixup for removal of handle_iic_irq

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 arch/powerpc/platforms/cell/interrupt.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/platforms/cell/interrupt.c b/arch/powerpc/platforms/cell/interrupt.c
index a19bec0..44cfd1b 100644
--- a/arch/powerpc/platforms/cell/interrupt.c
+++ b/arch/powerpc/platforms/cell/interrupt.c
@@ -244,7 +244,7 @@ static int iic_host_map(struct irq_host *h, unsigned int virq,
 		break;
 	case IIC_IRQ_TYPE_IOEXC:
 		irq_set_chip_and_handler(virq, &iic_ioexc_chip,
-					 handle_iic_irq);
+					 handle_edge_eoi_irq);
 		break;
 	default:
 		irq_set_chip_and_handler(virq, &iic_chip, handle_edge_eoi_irq);
-- 
1.7.4.1

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

^ permalink raw reply related

* linux-next: build failure after merge of the Linus' tree
From: Stephen Rothwell @ 2011-03-30  0:00 UTC (permalink / raw)
  To: Linus; +Cc: Thomas Gleixner, linux-next, ppc-dev, linux-kernel

Hi all,

After merging the Linus' tree, today's linux-next build (powerpc
ppc64_defconfig) failed like this:

kernel/irq/chip.c: In function 'handle_edge_eoi_irq':
kernel/irq/chip.c:517: warning: label 'out_unlock' defined but not used
kernel/irq/chip.c:503: error: label 'out_eoi' used but not defined

Caused by commit 0521c8fbb3da ("genirq: Provide edge_eoi flow handler")
which was clearly not even built with CONFIG_IRQ_EDGE_EOI_HANDLER defined.

I applied this fixup patch:

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Wed, 30 Mar 2011 10:55:12 +1100
Subject: [PATCH] genirq: fix CONFIG_IRQ_EDGE_EOI_HANDLER build

Fixes these errors:

kernel/irq/chip.c: In function 'handle_edge_eoi_irq':
kernel/irq/chip.c:517: warning: label 'out_unlock' defined but not used
kernel/irq/chip.c:503: error: label 'out_eoi' used but not defined

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 kernel/irq/chip.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 616ec1c..1dafc86 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -514,7 +514,7 @@ void handle_edge_eoi_irq(unsigned int irq, struct irq_desc *desc)
 	} while ((desc->istate & IRQS_PENDING) &&
 		 !irqd_irq_disabled(&desc->irq_data));
 
-out_unlock:
+out_eoi:
 	chip->irq_eoi(&desc->irq_data);
 	raw_spin_unlock(&desc->lock);
 }
-- 
1.7.4.1


-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

^ permalink raw reply related

* Re: [PATCH v2 3/5] xhci: Add rmb() between reading event validity & event data access.
From: Benjamin Herrenschmidt @ 2011-03-29 21:23 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: sarah.a.sharp, linuxppc-dev, linux-usb, Matt Evans
In-Reply-To: <201103291156.47822.dmitry.torokhov@gmail.com>

On Tue, 2011-03-29 at 11:56 -0700, Dmitry Torokhov wrote:
> > +     /*
> > +      * Barrier between reading the TRB_CYCLE (valid) flag above
> and any
> > +      * speculative reads of the event's flags/data below.
> > +      */
> > +     rmb();
> >       /* FIXME: Handle more event types. */
> >       switch ((le32_to_cpu(event->event_cmd.flags) &
> TRB_TYPE_BITMASK)) {
> 
> Isn't it the same memory that is being read the first time around? How
> reordering could happen here? 

We don't try to enforce ordering specifically with the subsequent read
of the flags that is visible in the snipped above (indeed that's the
same address and so hopefully should be in program order), but all
subsequent reads which could have been performed speculatively such as
the rest of the event and including whatever other in memory information
the chip might have updated prior to sending the event.

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH V2] POWER: perf_event: Skip updating kernel counters if register value shrinks
From: Benjamin Herrenschmidt @ 2011-03-29 21:14 UTC (permalink / raw)
  To: Eric B Munson
  Cc: a.p.zijlstra, linux-kernel, paulus, anton, acme, mingo,
	linuxppc-dev, stable
In-Reply-To: <1301410302-7270-1-git-send-email-emunson@mgebm.net>

\
> diff --git a/arch/powerpc/kernel/perf_event.c b/arch/powerpc/kernel/perf_event.c
> index 97e0ae4..0a5178f 100644
> --- a/arch/powerpc/kernel/perf_event.c
> +++ b/arch/powerpc/kernel/perf_event.c
> @@ -416,6 +416,15 @@ static void power_pmu_read(struct perf_event *event)
>  		prev = local64_read(&event->hw.prev_count);
>  		barrier();
>  		val = read_pmc(event->hw.idx);
> +		/*
> +		 * POWER7 can roll back counter values, if the new value is
> +		 * smaller than the previous value it will cause the delta
> +		 * and the counter to have bogus values.  If this is the
> +		 * case skip updating anything until the counter grows again.
> +		 * This can lead to a small lack of precision in the counters.
> +		 */
> +		if (val < prev)
> +			return;
>  	} while (local64_cmpxchg(&event->hw.prev_count, prev, val) != prev);

So if the counters can rollover, the above is still wrong... prev could
be ffffffff and val could be 1 ... for example. In this case you really
need to substract and get the sign of the result I'd think...

Cheers,
Ben.

>  	/* The counters are only 32 bits wide */
> @@ -439,7 +448,8 @@ static void freeze_limited_counters(struct cpu_hw_events *cpuhw,
>  				    unsigned long pmc5, unsigned long pmc6)
>  {
>  	struct perf_event *event;
> -	u64 val, prev, delta;
> +	u64 val, prev;
> +	s32 delta;
>  	int i;
>  
>  	for (i = 0; i < cpuhw->n_limited; ++i) {
> @@ -449,8 +459,13 @@ static void freeze_limited_counters(struct cpu_hw_events *cpuhw,
>  		val = (event->hw.idx == 5) ? pmc5 : pmc6;
>  		prev = local64_read(&event->hw.prev_count);
>  		event->hw.idx = 0;
> -		delta = (val - prev) & 0xfffffffful;
> -		local64_add(delta, &event->count);
> +		/*
> +		 * The PerfMon registers are only 32 bits wide so the
> +		 * delta should not overflow.
> +		 */
> +		delta = val - prev;
> +		if (delta > 0)
> +			local64_add(delta, &event->count);
>  	}
>  }
>  
> @@ -458,14 +473,16 @@ static void thaw_limited_counters(struct cpu_hw_events *cpuhw,
>  				  unsigned long pmc5, unsigned long pmc6)
>  {
>  	struct perf_event *event;
> -	u64 val;
> +	u64 val, prev;
>  	int i;
>  
>  	for (i = 0; i < cpuhw->n_limited; ++i) {
>  		event = cpuhw->limited_counter[i];
>  		event->hw.idx = cpuhw->limited_hwidx[i];
>  		val = (event->hw.idx == 5) ? pmc5 : pmc6;
> -		local64_set(&event->hw.prev_count, val);
> +		prev = local64_read(&event->hw.prev_count);
> +		if (val > prev)
> +			local64_set(&event->hw.prev_count, val);
>  		perf_event_update_userpage(event);
>  	}
>  }
> @@ -1187,7 +1204,8 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
>  			       struct pt_regs *regs, int nmi)
>  {
>  	u64 period = event->hw.sample_period;
> -	s64 prev, delta, left;
> +	s64 prev, left;
> +	s32 delta;
>  	int record = 0;
>  
>  	if (event->hw.state & PERF_HES_STOPPED) {
> @@ -1197,7 +1215,9 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
>  
>  	/* we don't have to worry about interrupts here */
>  	prev = local64_read(&event->hw.prev_count);
> -	delta = (val - prev) & 0xfffffffful;
> +	delta = val - prev;
> +	if (delta < 0)
> +		delta = 0;
>  	local64_add(delta, &event->count);
>  
>  	/*

^ permalink raw reply

* Re: [PATCH] POWER: perf_event: Skip updating kernel counters if register value shrinks
From: Benjamin Herrenschmidt @ 2011-03-29 21:12 UTC (permalink / raw)
  To: Eric B Munson
  Cc: a.p.zijlstra, linux-kernel, paulus, anton, acme, mingo,
	linuxppc-dev
In-Reply-To: <20110329142519.GA3527@mgebm.net>

On Tue, 2011-03-29 at 10:25 -0400, Eric B Munson wrote:
> Here I made the assumption that the hardware would never remove more events in
> a speculative roll back than it had added.  This is not a situation I
> encoutered in my limited testing, so I didn't think underflow was possible.  I
> will send out a V2 using the signed 32 bit delta and remeber to CC stable
> this time. 

I'm not thinking about underflow but rollover... or that isn't possible
with those counters ? IE. They don't wrap back to 0 after hitting
ffffffff ?

Cheers,
Ben.

^ permalink raw reply

* [patch 1/1] powerpc: cell: Use handle_edge_eoi_irq for real
From: Thomas Gleixner @ 2011-03-29 20:37 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: mingo

Missed one instance when moving that to the core code.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: michael@ellerman.id.au
Cc: mingo@elte.hu
---
 arch/powerpc/platforms/cell/interrupt.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6-tip/arch/powerpc/platforms/cell/interrupt.c
===================================================================
--- linux-2.6-tip.orig/arch/powerpc/platforms/cell/interrupt.c
+++ linux-2.6-tip/arch/powerpc/platforms/cell/interrupt.c
@@ -244,7 +244,7 @@ static int iic_host_map(struct irq_host 
 		break;
 	case IIC_IRQ_TYPE_IOEXC:
 		irq_set_chip_and_handler(virq, &iic_ioexc_chip,
-					 handle_iic_irq);
+					 handle_edge_eoi_irq);
 		break;
 	default:
 		irq_set_chip_and_handler(virq, &iic_chip, handle_edge_eoi_irq);

^ permalink raw reply

* Re: [PATCH v2 5/5] xhci: Remove recursive call to xhci_handle_event
From: Dmitry Torokhov @ 2011-03-29 20:00 UTC (permalink / raw)
  To: Matt Evans; +Cc: sarah.a.sharp, linuxppc-dev, linux-usb, Sergei Shtylyov
In-Reply-To: <4D90142C.4050206@ozlabs.org>

On Sunday, March 27, 2011 09:53:00 pm Matt Evans wrote:
> @@ -2282,7 +2284,7 @@ hw_died:
>  	/* FIXME this should be a delayed service routine
>  	 * that clears the EHB.
>  	 */
> -	xhci_handle_event(xhci);
> +	while (xhci_handle_event(xhci)) {}
> 

I must admit I dislike the style with empty loop bodies, do you think
we could have something like below instead?

Thanks!

-- 
Dmitry

From: Dmitry Torokhov <dtor@vmware.com>
Subject: [PATCH] USB: xhci: avoid recursion in xhci_handle_event

Instead of having xhci_handle_event call itself if there are more
outstanding TRBs push the loop logic up one level, into xhci_irq().

xchI_handle_event() does not check for presence of event_ring
anymore since the ring is always allocated. Also, encountering
a TRB that does not belong to OS is a normal condition that
causes us to stop processing and exit ISR and so is not reported
in xhci->error_bitmask.

Also consolidate handling of the event handler busy flag in
xhci_irq().

Reported-by: Matt Evans <matt@ozlabs.org>
Signed-off-by: Dmitry Torokhov <dtor@vmware.com>
---
 drivers/usb/host/xhci-ring.c |   77 +++++++++++++++---------------------------
 1 files changed, 27 insertions(+), 50 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 0e30281..8e6d8fa 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2131,26 +2131,12 @@ cleanup:
  * This function handles all OS-owned events on the event ring.  It may drop
  * xhci->lock between event processing (e.g. to pass up port status changes).
  */
-static void xhci_handle_event(struct xhci_hcd *xhci)
+static void xhci_handle_event(struct xhci_hcd *xhci, union xhci_trb *event)
 {
-	union xhci_trb *event;
-	int update_ptrs = 1;
+	bool update_ptrs = true;
 	int ret;
 
 	xhci_dbg(xhci, "In %s\n", __func__);
-	if (!xhci->event_ring || !xhci->event_ring->dequeue) {
-		xhci->error_bitmask |= 1 << 1;
-		return;
-	}
-
-	event = xhci->event_ring->dequeue;
-	/* Does the HC or OS own the TRB? */
-	if ((event->event_cmd.flags & TRB_CYCLE) !=
-			xhci->event_ring->cycle_state) {
-		xhci->error_bitmask |= 1 << 2;
-		return;
-	}
-	xhci_dbg(xhci, "%s - OS owns TRB\n", __func__);
 
 	/* FIXME: Handle more event types. */
 	switch ((event->event_cmd.flags & TRB_TYPE_BITMASK)) {
@@ -2163,7 +2149,7 @@ static void xhci_handle_event(struct xhci_hcd *xhci)
 		xhci_dbg(xhci, "%s - calling handle_port_status\n", __func__);
 		handle_port_status(xhci, event);
 		xhci_dbg(xhci, "%s - returned from handle_port_status\n", __func__);
-		update_ptrs = 0;
+		update_ptrs = false;
 		break;
 	case TRB_TYPE(TRB_TRANSFER):
 		xhci_dbg(xhci, "%s - calling handle_tx_event\n", __func__);
@@ -2172,7 +2158,7 @@ static void xhci_handle_event(struct xhci_hcd *xhci)
 		if (ret < 0)
 			xhci->error_bitmask |= 1 << 9;
 		else
-			update_ptrs = 0;
+			update_ptrs = false;
 		break;
 	default:
 		if ((event->event_cmd.flags & TRB_TYPE_BITMASK) >= TRB_TYPE(48))
@@ -2180,21 +2166,9 @@ static void xhci_handle_event(struct xhci_hcd *xhci)
 		else
 			xhci->error_bitmask |= 1 << 3;
 	}
-	/* Any of the above functions may drop and re-acquire the lock, so check
-	 * to make sure a watchdog timer didn't mark the host as non-responsive.
-	 */
-	if (xhci->xhc_state & XHCI_STATE_DYING) {
-		xhci_dbg(xhci, "xHCI host dying, returning from "
-				"event handler.\n");
-		return;
-	}
 
 	if (update_ptrs)
-		/* Update SW event ring dequeue pointer */
 		inc_deq(xhci, xhci->event_ring, true);
-
-	/* Are there more items on the event ring? */
-	xhci_handle_event(xhci);
 }
 
 /*
@@ -2258,34 +2232,37 @@ hw_died:
 		xhci_writel(xhci, irq_pending, &xhci->ir_set->irq_pending);
 	}
 
-	if (xhci->xhc_state & XHCI_STATE_DYING) {
-		xhci_dbg(xhci, "xHCI dying, ignoring interrupt. "
-				"Shouldn't IRQs be disabled?\n");
-		/* Clear the event handler busy flag (RW1C);
-		 * the event ring should be empty.
-		 */
-		temp_64 = xhci_read_64(xhci, &xhci->ir_set->erst_dequeue);
-		xhci_write_64(xhci, temp_64 | ERST_EHB,
-				&xhci->ir_set->erst_dequeue);
-		spin_unlock(&xhci->lock);
-
-		return IRQ_HANDLED;
-	}
-
 	event_ring_deq = xhci->event_ring->dequeue;
+
 	/* FIXME this should be a delayed service routine
 	 * that clears the EHB.
 	 */
-	xhci_handle_event(xhci);
+	xhci_dbg(xhci, "%s - Begin processing TRBs\n", __func__);
+
+	while (!(xhci->xhc_state & XHCI_STATE_DYING)) {
+		union xhci_trb *event = xhci->event_ring->dequeue;
+
+		/* Does the HC or OS own the TRB? */
+		if ((event->event_cmd.flags & TRB_CYCLE) !=
+				xhci->event_ring->cycle_state) {
+			break;
+		}
+
+		xhci_dbg(xhci, "%s - OS owns TRB\n", __func__);
+		xhci_handle_event(xhci, event);
+	}
 
 	temp_64 = xhci_read_64(xhci, &xhci->ir_set->erst_dequeue);
-	/* If necessary, update the HW's version of the event ring deq ptr. */
-	if (event_ring_deq != xhci->event_ring->dequeue) {
+
+	if (unlikely(xhci->xhc_state & XHCI_STATE_DYING)) {
+		xhci_dbg(xhci, "%s: xHCI is dying, exiting ISR\n", __func__);
+	} else if (event_ring_deq != xhci->event_ring->dequeue) {
+		/* Update the HW's version of the event ring deq ptr. */
 		deq = xhci_trb_virt_to_dma(xhci->event_ring->deq_seg,
-				xhci->event_ring->dequeue);
+					xhci->event_ring->dequeue);
 		if (deq == 0)
-			xhci_warn(xhci, "WARN something wrong with SW event "
-					"ring dequeue ptr.\n");
+			xhci_warn(xhci,
+				  "WARN something wrong with SW event ring dequeue ptr.\n");
 		/* Update HC event ring dequeue pointer */
 		temp_64 &= ERST_PTR_MASK;
 		temp_64 |= ((u64) deq & (u64) ~ERST_PTR_MASK);
-- 
1.7.4.1

^ permalink raw reply related

* Re: [PATCH v2 3/5] xhci: Add rmb() between reading event validity & event data access.
From: Dmitry Torokhov @ 2011-03-29 18:56 UTC (permalink / raw)
  To: Matt Evans; +Cc: sarah.a.sharp, linuxppc-dev, linux-usb
In-Reply-To: <4D901429.5010807@ozlabs.org>

On Sunday, March 27, 2011 09:52:57 pm Matt Evans wrote:
> On weakly-ordered systems, the reading of an event's content must occur
> after reading the event's validity.
> 
> Signed-off-by: Matt Evans <matt@ozlabs.org>
> ---
> Segher, thanks for the comment; explanation added.
> 
>  drivers/usb/host/xhci-ring.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 45f3b77..d6aa880 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -2152,6 +2152,11 @@ static void xhci_handle_event(struct xhci_hcd
> *xhci) }
>  	xhci_dbg(xhci, "%s - OS owns TRB\n", __func__);
> 
> +	/*
> +	 * Barrier between reading the TRB_CYCLE (valid) flag above and any
> +	 * speculative reads of the event's flags/data below.
> +	 */
> +	rmb();
>  	/* FIXME: Handle more event types. */
>  	switch ((le32_to_cpu(event->event_cmd.flags) & TRB_TYPE_BITMASK)) {

Isn't it the same memory that is being read the first time around? How 
reordering could happen here?

Thanks.


-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v2 5/5] xhci: Remove recursive call to xhci_handle_event
From: Sarah Sharp @ 2011-03-29 18:34 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: linuxppc-dev, linux-usb, Sergei Shtylyov, Matt Evans
In-Reply-To: <1301356734.2402.657.camel@pasglop>

On Tue, Mar 29, 2011 at 10:58:54AM +1100, Benjamin Herrenschmidt wrote:
> On Mon, 2011-03-28 at 15:34 -0700, Sarah Sharp wrote:
> > 
> > What I'd like to do is take out the read of the status register out of
> > the interrupt handler (which is killing performance), and make it only
> > check the status register when xhci_handle_event() returns a negative
> > error status.  If the status register shows the host controller has a
> > critical error, the driver should call usb_hcd_died().
> 
> Be careful with removing that read...
> 
> Without MSIs, that read is what guarantees that all pending DMA writes
> by the xHCI have been "flushed" before you start poking at memory.
> 
> IE. If the chip writes an event and sends an LSI, without that read, you
> might get the interrupt before the writes to memory have completed and
> your driver will "miss" the event.
> 
> With MSIs (provided they are not broken on your PCI host bridge of
> course, this is typically the #1 cause of MSI breakage), you don't need
> that as the MSI itself is a DMA store by the device which is ordered
> after the stores done to update the event. So by the time you get the
> MSI interrupt, you -should- have all the updates visible in memory.
> 
> But that means that your PCI host bridge is doing the right thing, by
> ensuring whatever queues to the coherency domain it has have been
> properly flushed before it signals the interrupts caused by the MSI to
> the processors. Hopefully most systems get that right nowadays.
> 
> Point is: you need to keep that read if MSIs aren't enabled.

Sorry for the sloppy language, yes, I understand I still need the status
register read if only legacy IRQs are enabled.

Sarah Sharp

^ 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