* [PATCH 3/4 v2] powerpc: Better scheduling of CR save code in system call path
From: Anton Blanchard @ 2012-04-05 13:44 UTC (permalink / raw)
To: benh, paulus; +Cc: linuxppc-dev
In-Reply-To: <20120405142557.398f14d2@kryten>
At the moment system call entry looks like:
crclr so
...
mfcr r9
...
std r9,_CCR(r1)
commit bd19c8994a82 ([POWERPC] system call micro optimisation) put
some space between the crclr and mfcr in order to avoid a stall.
There is still a stall seen between the mfcr and std. We can avoid
the crclr by doing it in a GPR with rlwinm which gives us more room
to better schedule the sequence.
Signed-off-by: Anton Blanchard <anton@samba.org>
---
v2:
Milton pointed out that rlwinm copies the low 32bits into the top
32bits and this might confuse ptracers. We tossed around a few ideas
but his suggestion of using rldimi sounds good to me.
Index: linux-build/arch/powerpc/kernel/entry_64.S
===================================================================
--- linux-build.orig/arch/powerpc/kernel/entry_64.S 2012-04-05 14:03:05.123678148 +1000
+++ linux-build/arch/powerpc/kernel/entry_64.S 2012-04-05 23:38:09.182681076 +1000
@@ -63,15 +63,9 @@ system_call_common:
std r0,GPR0(r1)
std r10,GPR1(r1)
ACCOUNT_CPU_USER_ENTRY(r10, r11)
- /*
- * This "crclr so" clears CR0.SO, which is the error indication on
- * return from this system call. There must be no cmp instruction
- * between it and the "mfcr r9" below, otherwise if XER.SO is set,
- * CR0.SO will get set, causing all system calls to appear to fail.
- */
- crclr so
std r2,GPR2(r1)
std r3,GPR3(r1)
+ mfcr r2
std r4,GPR4(r1)
std r5,GPR5(r1)
std r6,GPR6(r1)
@@ -84,15 +78,19 @@ system_call_common:
std r11,GPR12(r1)
std r11,_XER(r1)
std r9,GPR13(r1)
- mfcr r9
mflr r10
+ /*
+ * This clears CR0.SO (bit 28), which is the error indication on
+ * return from this system call.
+ */
+ rldimi r2,r11,28,(63-28)
li r11,0xc01
- std r9,_CCR(r1)
std r10,_LINK(r1)
std r11,_TRAP(r1)
mfctr r10
std r10,_CTR(r1)
std r3,ORIG_GPR3(r1)
+ std r2,_CCR(r1)
ld r2,PACATOC(r13)
addi r9,r1,STACK_FRAME_OVERHEAD
ld r11,exception_marker@toc(r2)
^ permalink raw reply
* Re: [PATCH v5 06/27] irq_domain/powerpc: eliminate irq_map; use irq_alloc_desc() instead
From: Andreas Schwab @ 2012-04-05 12:35 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1333401070.30734.55.camel__46349.1482347765$1333401169$gmane$org@pasglop>
Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:
> Hrm, odd.. I'll dbl check today that I didn't b0rk something in my test
> (like loading the wrong kernel :-) I'll also try with your config.
Do you have anything that triggers loading the sound drivers? That's
exactly the point where the irq problems start.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: [PATCH v5 06/27] irq_domain/powerpc: eliminate irq_map; use irq_alloc_desc() instead
From: Andreas Schwab @ 2012-04-05 10:51 UTC (permalink / raw)
To: Grant Likely
Cc: devicetree-discuss, linux-kernel, Milton Miller, Rob Herring,
Thomas Gleixner, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20120404154020.40C513E09D5@localhost>
Grant Likely <grant.likely@secretlab.ca> writes:
> I bet it is NR_IRQS related. You have SPARSE_IRQ enabled, which means
> the maximum number of irq_descs is IRQ_BITMAP_BITS (NR_IRQS + 8192).
The actual definition uses NR_IRQS + 8196. Guess that's a typo. (Does
it really make sense to add NR_IRQS here?)
> diff --git a/arch/powerpc/include/asm/irq.h b/arch/powerpc/include/asm/irq.h
> index cf417e51..9edf499 100644
> --- a/arch/powerpc/include/asm/irq.h
> +++ b/arch/powerpc/include/asm/irq.h
> @@ -20,7 +20,7 @@
>
> /* Define a way to iterate across irqs. */
> #define for_each_irq(i) \
> - for ((i) = 0; (i) < NR_IRQS; ++(i))
> + for ((i) = 0; (i) < nr_irqs; ++(i))
There are exactly two uses of for_each_irq, one is related to cpu
hotplug, the other to kexec, so that cannot make any difference.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* [PATCH 4/4] powerpc: No need to preserve count register across system call
From: Anton Blanchard @ 2012-04-05 4:26 UTC (permalink / raw)
To: benh, paulus; +Cc: linuxppc-dev
In-Reply-To: <20120405142327.6038bbb4@kryten>
The count register is volatile so we don't need to preserve it.
Store zero to the entry in the exception frame.
Signed-off-by: Anton Blanchard <anton@samba.org>
---
Index: linux-build/arch/powerpc/kernel/entry_64.S
===================================================================
--- linux-build.orig/arch/powerpc/kernel/entry_64.S 2012-03-22 22:47:15.023504471 +1100
+++ linux-build/arch/powerpc/kernel/entry_64.S 2012-03-22 22:47:17.899556262 +1100
@@ -77,6 +77,7 @@ system_call_common:
std r11,GPR11(r1)
std r11,GPR12(r1)
std r11,_XER(r1)
+ std r11,_CTR(r1)
std r9,GPR13(r1)
mflr r10
li r11,0xc01
@@ -87,8 +88,6 @@ system_call_common:
rlwinm r2,r2,0,4,2
std r10,_LINK(r1)
std r11,_TRAP(r1)
- mfctr r10
- std r10,_CTR(r1)
std r3,ORIG_GPR3(r1)
std r2,_CCR(r1)
ld r2,PACATOC(r13)
^ permalink raw reply
* [PATCH 3/4] powerpc: Better scheduling of CR save code in system call path
From: Anton Blanchard @ 2012-04-05 4:25 UTC (permalink / raw)
To: benh, paulus; +Cc: linuxppc-dev
In-Reply-To: <20120405142327.6038bbb4@kryten>
At the moment system call entry looks like:
crclr so
...
mfcr r9
...
std r9,_CCR(r1)
commit bd19c8994a82 ([POWERPC] system call micro optimisation) put
some space between the crclr and mfcr in order to avoid a stall.
There is still a stall seen between the mfcr and std. We can avoid
the crclr by doing it in a GPR with rlwinm which gives us more room
to better schedule the sequence.
Signed-off-by: Anton Blanchard <anton@samba.org>
---
Index: linux-build/arch/powerpc/kernel/entry_64.S
===================================================================
--- linux-build.orig/arch/powerpc/kernel/entry_64.S 2012-03-22 22:47:08.379384827 +1100
+++ linux-build/arch/powerpc/kernel/entry_64.S 2012-03-22 22:47:15.023504471 +1100
@@ -63,15 +63,9 @@ system_call_common:
std r0,GPR0(r1)
std r10,GPR1(r1)
ACCOUNT_CPU_USER_ENTRY(r10, r11)
- /*
- * This "crclr so" clears CR0.SO, which is the error indication on
- * return from this system call. There must be no cmp instruction
- * between it and the "mfcr r9" below, otherwise if XER.SO is set,
- * CR0.SO will get set, causing all system calls to appear to fail.
- */
- crclr so
std r2,GPR2(r1)
std r3,GPR3(r1)
+ mfcr r2
std r4,GPR4(r1)
std r5,GPR5(r1)
std r6,GPR6(r1)
@@ -84,15 +78,19 @@ system_call_common:
std r11,GPR12(r1)
std r11,_XER(r1)
std r9,GPR13(r1)
- mfcr r9
mflr r10
li r11,0xc01
- std r9,_CCR(r1)
+ /*
+ * This clears CR0.SO, which is the error indication on return
+ * from this system call.
+ */
+ rlwinm r2,r2,0,4,2
std r10,_LINK(r1)
std r11,_TRAP(r1)
mfctr r10
std r10,_CTR(r1)
std r3,ORIG_GPR3(r1)
+ std r2,_CCR(r1)
ld r2,PACATOC(r13)
addi r9,r1,STACK_FRAME_OVERHEAD
ld r11,exception_marker@toc(r2)
^ permalink raw reply
* [PATCH 2/4] powerpc: No need to save XER in a system call
From: Anton Blanchard @ 2012-04-05 4:24 UTC (permalink / raw)
To: benh, paulus; +Cc: linuxppc-dev
In-Reply-To: <20120405142327.6038bbb4@kryten>
The XER is a volatile register so there is no need to save and restore
it over a system call - zero it out in the exception stack frame
instead.
This should fix a 5 cycle stall of the mfxer/std seen on POWER7.
Signed-off-by: Anton Blanchard <anton@samba.org>
---
Index: linux-build/arch/powerpc/kernel/entry_64.S
===================================================================
--- linux-build.orig/arch/powerpc/kernel/entry_64.S 2012-03-22 22:47:04.479314599 +1100
+++ linux-build/arch/powerpc/kernel/entry_64.S 2012-03-22 22:47:08.379384827 +1100
@@ -82,6 +82,7 @@ system_call_common:
std r11,GPR10(r1)
std r11,GPR11(r1)
std r11,GPR12(r1)
+ std r11,_XER(r1)
std r9,GPR13(r1)
mfcr r9
mflr r10
@@ -89,9 +90,7 @@ system_call_common:
std r9,_CCR(r1)
std r10,_LINK(r1)
std r11,_TRAP(r1)
- mfxer r9
mfctr r10
- std r9,_XER(r1)
std r10,_CTR(r1)
std r3,ORIG_GPR3(r1)
ld r2,PACATOC(r13)
^ permalink raw reply
* [PATCH 1/4] powerpc: Hide some system call labels from profile tools
From: Anton Blanchard @ 2012-04-05 4:23 UTC (permalink / raw)
To: benh, paulus, linuxppc-dev
syscall_dotrace_cont and syscall_error_cont tend to complicate perf
output so make them local.
Signed-off-by: Anton Blanchard <anton@samba.org>
---
Index: linux-build/arch/powerpc/kernel/entry_64.S
===================================================================
--- linux-build.orig/arch/powerpc/kernel/entry_64.S 2012-03-22 22:41:21.209137690 +1100
+++ linux-build/arch/powerpc/kernel/entry_64.S 2012-03-22 22:47:04.479314599 +1100
@@ -154,7 +154,7 @@ END_FW_FTR_SECTION_IFSET(FW_FEATURE_SPLP
ld r10,TI_FLAGS(r11)
andi. r11,r10,_TIF_SYSCALL_T_OR_A
bne- syscall_dotrace
-syscall_dotrace_cont:
+.Lsyscall_dotrace_cont:
cmpldi 0,r0,NR_syscalls
bge- syscall_enosys
@@ -211,7 +211,7 @@ syscall_exit:
cmpld r3,r11
ld r5,_CCR(r1)
bge- syscall_error
-syscall_error_cont:
+.Lsyscall_error_cont:
ld r7,_NIP(r1)
BEGIN_FTR_SECTION
stdcx. r0,0,r1 /* to clear the reservation */
@@ -246,7 +246,7 @@ syscall_error:
oris r5,r5,0x1000 /* Set SO bit in CR */
neg r3,r3
std r5,_CCR(r1)
- b syscall_error_cont
+ b .Lsyscall_error_cont
/* Traced system call support */
syscall_dotrace:
@@ -268,7 +268,7 @@ syscall_dotrace:
addi r9,r1,STACK_FRAME_OVERHEAD
clrrdi r10,r1,THREAD_SHIFT
ld r10,TI_FLAGS(r10)
- b syscall_dotrace_cont
+ b .Lsyscall_dotrace_cont
syscall_enosys:
li r3,-ENOSYS
^ permalink raw reply
* Re: [PATCH] powerpc/boot: Only build board support files when required.
From: Geoff Levand @ 2012-04-05 3:44 UTC (permalink / raw)
To: Tony Breeds
Cc: Frank Svendsbøe, Wolfgang Denk, robert.karl.berger,
LinuxPPC-dev
In-Reply-To: <20120403005558.GA9194@thor.bakeyournoodle.com>
On Tue, 2012-04-03 at 10:55 +1000, Tony Breeds wrote:
> Currently we build all board files regardless of the final zImage
> target. This is sub-optimal (in terms on compilation) and leads to
> problems in one platform needlessly causing failures for other
> platforms.
>
> Use the Kconfig variables to selectively construct this board files to
> build.
>
> +ifeq ($(CONFIG_PPC_PS3),y)
> +src-plat += ps3-head.S ps3-hvcall.S ps3.c
> +endif
PS3 part looks good.
Acked-by: Geoff Levand <geoff@infradead.org>
^ permalink raw reply
* Re: kilauea compilation breaks with v3.3 kernel and ELDK 4.2
From: Stephen Rothwell @ 2012-04-05 0:02 UTC (permalink / raw)
To: Wolfgang Denk; +Cc: linuxppc-dev, robert.karl.berger, Frank Svendsbøe
In-Reply-To: <20120404192101.2A558200243@gemini.denx.de>
[-- Attachment #1: Type: text/plain, Size: 781 bytes --]
On Wed, 04 Apr 2012 21:21:01 +0200 Wolfgang Denk <wd@denx.de> wrote:
>
> The kernel README says nothing about binutils requirements, the only
> tool related statement is "Make sure you have at least gcc 3.2
> available." Actually I doubt if gcc 3.2 wouldbuild a working kernel
> image.
>
> ELDK 4.2 is based on gcc version 4.2.2 / binutils version 2.17.50.0.12
> 20070128. This is obviously to old for this code. I do not see an
> actual problem with that - nobody can expect that we support old tol
> chain versions forever.
The requirements are documented in Documentation/Changes where we say we
need gcc 3.2 and binutils 2.12. Not that that is very relevant to this
discussion. ;-)
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* RE: [PATCH 3/4] powerpc/mpic: Move internal interrupt source vector allocation to a separate function.
From: Benjamin Herrenschmidt @ 2012-04-04 23:29 UTC (permalink / raw)
To: Sethi Varun-B16395; +Cc: Linuxppc-dev@lists.ozlabs.org
In-Reply-To: <C5ECD7A89D1DC44195F34B25E172658D0B77F9@039-SN2MPN1-013.039d.mgd.msft.net>
On Wed, 2012-04-04 at 19:09 +0000, Sethi Varun-B16395 wrote:
> > I think that is more due to how you added the MPIC error interrupts
> and
> > issues w/that code. If you are treating the MPIC error interrupts
> as a
> > cascade than they should have a distinct linux IRQ space from the
> > standard MPIC interrupts. This is how the MSIs work (as an
> example).
> In case of error interrupts we are depending on the mpic_host_maps
> for mapping and interrupt specifier translations. There is no separate
> initialization as in case of MSIs. That's the reason I am treating
> error interrupts as mpic internal interrupt sources.
It makes sense to have an allocator for MPIC vectors but it should be a
single allocator shared with the MSI code.
Cheers,
Ben.
^ permalink raw reply
* Re: kilauea compilation breaks with v3.3 kernel and ELDK 4.2
From: Wolfgang Denk @ 2012-04-04 19:21 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev, robert.karl.berger, Frank Svendsbøe
In-Reply-To: <CA+5PVA6jQgme0sS=i8yYX6b_R5179+BtWPh7QXCGNfoLTCt46A@mail.gmail.com>
Dear Josh,
In message <CA+5PVA6jQgme0sS=i8yYX6b_R5179+BtWPh7QXCGNfoLTCt46A@mail.gmail.com> you wrote:
>
> > The kernel shouldn't have tried to build that instruction on 8xx, though
> > I suppose if it's in arch/powerpc/boot, we are a bit too eager at
> > building everything including what's not relevant, we might to be a bit
> > more careful at excluding 4xx stuff on a 8xx kernel.
>
> It's still a binutils issue. Sounds like the toolchain being used to
> build the 8xx kernel is specifically built for 8xx. A generally built
> binutils should have worked fine (assuming it was new enough), since
> we pass -mcpu=405.
The problem is the "assuming it was new enough" part.
The kernel README says nothing about binutils requirements, the only
tool related statement is "Make sure you have at least gcc 3.2
available." Actually I doubt if gcc 3.2 wouldbuild a working kernel
image.
ELDK 4.2 is based on gcc version 4.2.2 / binutils version 2.17.50.0.12
20070128. This is obviously to old for this code. I do not see an
actual problem with that - nobody can expect that we support old tol
chain versions forever.
But then, I think if we make assumptions about tool versions, we
should add appropriate tests and issue helpful error messages. Here,
we should issue an error "binutils versions x.y.z or later needed" or
similar.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
I have made mistakes, but have never made the mistake of claiming I
never made one. - James G. Bennet
^ permalink raw reply
* RE: [PATCH 3/4] powerpc/mpic: Move internal interrupt source vector allocation to a separate function.
From: Sethi Varun-B16395 @ 2012-04-04 19:09 UTC (permalink / raw)
To: Kumar Gala; +Cc: Linuxppc-dev@lists.ozlabs.org
In-Reply-To: <BA2352CD-6C54-4D33-86DB-F55DBDBA6E5F@kernel.crashing.org>
> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> Sent: Tuesday, March 27, 2012 7:32 PM
> To: Sethi Varun-B16395
> Cc: Linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH 3/4] powerpc/mpic: Move internal interrupt source
> vector allocation to a separate function.
>=20
>=20
> On Mar 27, 2012, at 8:52 AM, Sethi Varun-B16395 wrote:
>=20
> >
> >
> >> -----Original Message-----
> >> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> >> Sent: Tuesday, March 27, 2012 6:55 PM
> >> To: Sethi Varun-B16395
> >> Cc: Linuxppc-dev@lists.ozlabs.org
> >> Subject: Re: [PATCH 3/4] powerpc/mpic: Move internal interrupt source
> >> vector allocation to a separate function.
> >>
> >>
> >> On Mar 27, 2012, at 7:16 AM, Varun Sethi wrote:
> >>
> >>> Allocate vector numbers for MPIC internal interrupt sources (IPIs
> >>> and
> >>> Timers) in a separate function.
> >>>
> >>
> >> Explain why you are making this change.
> >
> >
> > [Sethi Varun-B16395] With the current code it becomes fairly difficult
> > to add new internal interrupt sources. In my case I had to add 32
> > additional interrupt sources corresponding to the MPIC error
> > interrupts. It's more convenient doing the internal interrupt source
> allocation using a loop.
>=20
> I think that is more due to how you added the MPIC error interrupts and
> issues w/that code. If you are treating the MPIC error interrupts as a
> cascade than they should have a distinct linux IRQ space from the
> standard MPIC interrupts. This is how the MSIs work (as an example).
In case of error interrupts we are depending on the mpic_host_maps
for mapping and interrupt specifier translations. There is no separate
initialization as in case of MSIs. That's the reason I am treating
error interrupts as mpic internal interrupt sources.
-Varun
^ permalink raw reply
* Re: 3.4.0-rc1: No init found
From: Christian Kujau @ 2012-04-04 16:45 UTC (permalink / raw)
To: Suzuki K. Poulose; +Cc: linuxppc-dev, LKML
In-Reply-To: <4F7C4058.6090405@in.ibm.com>
On Wed, 4 Apr 2012 at 18:06, Suzuki K. Poulose wrote:
> > INFO: Uncompressed kernel (size 0x6d4b80) overlaps the address of the
> > wrapper(0x400000)
> > INFO: Fixing the link_address of wrapper to (0x700000)
> > Building modules, stage 2.
> > MODPOST 24 modules
> > ------------
> >
> > I started to see these messages in January (around Linux 3.2.0), but never
> > investigated what it was since the produced kernels continued to boot just
> > fine.
>
> The above change was added by me. The message is printed when the 'wrapper'
> script finds that decompressed kernel overlaps the 'bootstrap code' which does
> the decompression. So it shifts the 'address' of the bootstrap code to the
> next higher MB. As such it is harmless.
OK, good to know that it's harmless. Thanks for the explanation.
Christian.
--
BOFH excuse #256:
You need to install an RTFM interface.
^ permalink raw reply
* Re: [PATCH v5 06/27] irq_domain/powerpc: eliminate irq_map; use irq_alloc_desc() instead
From: Grant Likely @ 2012-04-04 15:40 UTC (permalink / raw)
To: Andreas Schwab
Cc: devicetree-discuss, linux-kernel, Milton Miller, Rob Herring,
Thomas Gleixner, linuxppc-dev, linux-arm-kernel
In-Reply-To: <m2y5qdow0v.fsf@igel.home>
On Tue, 03 Apr 2012 14:11:12 +0200, Andreas Schwab <schwab@linux-m68k.org> wrote:
> Grant Likely <grant.likely@secretlab.ca> writes:
>
> > Can you dump out /debug/powerpc/virq_mapping from both before and
> > after the irq_map patch is applied?
>
> before:
> virq hwirq chip name chip data host name
> 16 0x00000 MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 21 0x00001 MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 24 0x00002 MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 25 0x00019 MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 26 0x0001a MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 27 0x0001b MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 28 0x0001c MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 29 0x0003d MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 30 0x0001e MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 31 0x0003c MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 39 0x00027 MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 40 0x00028 MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 41 0x00029 MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 42 0x0002a MPIC 2 0xc00000017a011000 /u3@0,f8000000/mpic@f8040000
> 47 0x0002f MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 59 0x000fb MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 60 0x000fc MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 61 0x000fd MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 62 0x000fe MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 63 0x0003f MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>
> after:
> virq hwirq chip name chip data host name
> 16 0x00000 MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 21 0x00001 MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 24 0x00002 MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 25 0x00019 MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 26 0x0001a MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 27 0x0001b MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 28 0x0001c MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 30 0x0001e MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 39 0x00027 MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 40 0x00028 MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 41 0x00029 MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 42 0x0002a MPIC 2 0xc00000017a011000 /u3@0,f8000000/mpic@f8040000
> 47 0x0002f MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 59 0x000fb MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 60 0x000fc MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 61 0x000fd MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 62 0x000fe MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 63 0x0003f MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 64 0x0003d MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
> 65 0x0003c MPIC 1 0xc00000017a010000 /ht@0,f2000000/pci@1/mac-io@7/mpic@40000
>
> But I have NR_IRQS=64. Bounds checking missing? Irqs 64/65 are related
> to the sound chip (headphone-detect and line-out-detect).
I bet it is NR_IRQS related. You have SPARSE_IRQ enabled, which means
the maximum number of irq_descs is IRQ_BITMAP_BITS (NR_IRQS + 8192).
The old powerpc code was strictly limited to NR_IRQS, but the new code
uses irq_alloc_descs() which isn't. Yet I can see places in the
powerpc code that depends specifically on the value of NR_IRQS. The
for_each_irq() macro for instance. I think all the users there can be
switched to using for_each_irq_desc().
Can you attach console output logs for each of configs above and also
with NR_IRQS=128? That might give me some clues as to which specific
code is causing the issues. Also, as a quick test, try changing
for_each_irq_desc() to use "nr_irqs" instead of "NR_IRQS". nr_irqs is
kept up to date with the real maximum number of irqs allocated in the
system:
diff --git a/arch/powerpc/include/asm/irq.h b/arch/powerpc/include/asm/irq.h
index cf417e51..9edf499 100644
--- a/arch/powerpc/include/asm/irq.h
+++ b/arch/powerpc/include/asm/irq.h
@@ -20,7 +20,7 @@
/* Define a way to iterate across irqs. */
#define for_each_irq(i) \
- for ((i) = 0; (i) < NR_IRQS; ++(i))
+ for ((i) = 0; (i) < nr_irqs; ++(i))
extern atomic_t ppc_n_lost_interrupts;
g.
>
> When reconfiguring with NR_IRQS=128 interrupts are working again, but I
> still see a lot of spurious interrupts, and the X server is still broken
> (no input works, but I still don't know whether that is an unrelated
> bug).
>
> This is a sample of /proc/interrupts from 3.3 (with NR_IRQS=64):
> CPU0 CPU1
> 16: 2039 6070 MPIC 1 Level sata_svw
> 21: 0 0 MPIC 1 Edge i2sbus: i2s-a (tx)
> 22: 12 20 MPIC 1 Level
> 23: 14 18 MPIC 1 Level
> 24: 0 0 MPIC 1 Edge i2sbus: i2s-a (rx)
> 25: 3 0 MPIC 1 Level VIA-PMU
> 26: 16 62 MPIC 1 Level keywest i2c
> 27: 0 1 MPIC 1 Level ohci_hcd:usb2
> 28: 0 1 MPIC 1 Level ohci_hcd:usb3
> 29: 0 0 MPIC 1 Edge headphone-detect
> 30: 0 0 MPIC 1 Level i2sbus: i2s-a (control)
> 31: 0 0 MPIC 1 Edge line-output-detect
> 39: 22 64 MPIC 1 Level pata-pci-macio
> 40: 0 2 MPIC 1 Level firewire_ohci
> 41: 52 147 MPIC 1 Level eth0
> 42: 1732 5053 MPIC 2 Level keywest i2c
> 47: 0 0 MPIC 1 Level GPIO1 ADB
> 59: 0 0 MPIC 1 Edge ipi call function
> 60: 2064 1940 MPIC 1 Edge ipi reschedule
> 61: 3406 945 MPIC 1 Edge ipi call function single
> 62: 0 0 MPIC 1 Edge ipi debugger
> 63: 39 91 MPIC 1 Level ehci_hcd:usb1, ohci_hcd:usb4, ohci_hcd:usb5
> LOC: 3503 3719 Local timer interrupts
> SPU: 2 0 Spurious interrupts
> CNT: 0 0 Performance monitoring interrupts
> MCE: 0 0 Machine check exceptions
>
> This is a sample of /proc/interrupts from 3.4-rc1 (with NR_IRQS=128):
> CPU0 CPU1
> 16: 2603 7596 MPIC 1 Level sata_svw
> 21: 1 0 MPIC 1 Edge i2sbus: i2s-a (tx)
> 22: 13 19 MPIC 1 Level
> 23: 8 24 MPIC 1 Level
> 24: 0 1 MPIC 1 Edge i2sbus: i2s-a (rx)
> 25: 2 1 MPIC 1 Level VIA-PMU
> 26: 21 57 MPIC 1 Level keywest i2c
> 27: 0 1 MPIC 1 Level ohci_hcd:usb2
> 28: 0 1 MPIC 1 Level ohci_hcd:usb3
> 30: 0 0 MPIC 1 Level i2sbus: i2s-a (control)
> 39: 39 131 MPIC 1 Level pata-pci-macio
> 40: 2 2 MPIC 1 Level firewire_ohci
> 41: 93 268 MPIC 1 Level eth0
> 42: 8569 24140 MPIC 2 Level keywest i2c
> 47: 0 0 MPIC 1 Level GPIO1 ADB
> 60: 1 0 MPIC 1 Edge line-output-detect
> 61: 1 0 MPIC 1 Edge headphone-detect
> 63: 153 502 MPIC 1 Level ehci_hcd:usb1, ohci_hcd:usb4, ohci_hcd:usb5
> 123: 0 0 MPIC 1 Edge ipi call function
> 124: 1978 2349 MPIC 1 Edge ipi reschedule
> 125: 2356 1816 MPIC 1 Edge ipi call function single
> 126: 0 0 MPIC 1 Edge ipi debugger
> LOC: 4417 7985 Local timer interrupts
> SPU: 9586 25811 Spurious interrupts
> CNT: 0 0 Performance monitoring interrupts
> MCE: 0 0 Machine check exceptions
>
> Andreas.
>
> --
> Andreas Schwab, schwab@linux-m68k.org
> GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
> "And now for something completely different."
--
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies,Ltd.
^ permalink raw reply related
* Re: [alsa-devel] [PATCH] powerpc: select PPC_CLOCK unconditionally for FSL_SOC
From: Shawn Guo @ 2012-04-04 15:06 UTC (permalink / raw)
To: Kumar Gala; +Cc: alsa-devel, linuxppc-dev, Timur Tabi
In-Reply-To: <9F34EEF5-830B-4115-8BB0-DE9FF794BDB9@kernel.crashing.org>
On 4 April 2012 23:00, Kumar Gala <galak@kernel.crashing.org> wrote:
...
> What timeframe are you looking for this to go in? 3.4 or 3.5?
>
3.5
Thanks,
Shawn
^ permalink raw reply
* Re: [PATCH] powerpc: select PPC_CLOCK unconditionally for FSL_SOC
From: Kumar Gala @ 2012-04-04 15:00 UTC (permalink / raw)
To: Shawn Guo; +Cc: alsa-devel, linuxppc-dev, Timur Tabi
In-Reply-To: <20120404133246.GA25168@S2101-09.ap.freescale.net>
On Apr 4, 2012, at 8:32 AM, Shawn Guo wrote:
> Kumar,
>
> Gentle ping ...
>
> Regards,
> Shawn
Was on a bit of travel to nowhere, but that's a different story.
What timeframe are you looking for this to go in? 3.4 or 3.5?
- k
>
> On Fri, Mar 30, 2012 at 01:38:56PM +0800, Shawn Guo wrote:
>> Freescale PowerPC SoCs share a number of IP blocks with Freescale
>> ARM/IMX SoCs, FlexCAN, SSI, FEC, eSDHC, USB, etc. There are some
>> effort consolidating those drivers to make them work for both
>> architectures.
>>
>> One outstanding difference between two architectures is ARM/IMX will
>> turn off module clocks during platform initialization for power saving
>> and expects drivers manage clocks using clk API, while PowerPC
>> mostly does not do that, and thus does not always build in clk API.
>>
>> Listing all those driver Kconfig options in "select PPC_CLOCK if" seems
>> not scalable for long term maintenance, and could easily introduce
>> Kconfig recursive dependency. This patch chooses to select PPC_CLOCK
>> unconditionally for FSL_SOC to always build clk API for PowerPC in.
>>
>> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
>> ---
>> arch/powerpc/Kconfig | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index feab3ba..63fa7fb 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -661,7 +661,7 @@ config SBUS
>> config FSL_SOC
>> bool
>> select HAVE_CAN_FLEXCAN if NET && CAN
>> - select PPC_CLOCK if CAN_FLEXCAN
>> + select PPC_CLOCK
>>
>> config FSL_PCI
>> bool
>> --
>> 1.7.5.4
>>
^ permalink raw reply
* Re: [PATCH] powerpc: select PPC_CLOCK unconditionally for FSL_SOC
From: Shawn Guo @ 2012-04-04 13:32 UTC (permalink / raw)
To: Kumar Gala; +Cc: alsa-devel, linuxppc-dev, Timur Tabi
In-Reply-To: <1333085936-8977-1-git-send-email-shawn.guo@linaro.org>
Kumar,
Gentle ping ...
Regards,
Shawn
On Fri, Mar 30, 2012 at 01:38:56PM +0800, Shawn Guo wrote:
> Freescale PowerPC SoCs share a number of IP blocks with Freescale
> ARM/IMX SoCs, FlexCAN, SSI, FEC, eSDHC, USB, etc. There are some
> effort consolidating those drivers to make them work for both
> architectures.
>
> One outstanding difference between two architectures is ARM/IMX will
> turn off module clocks during platform initialization for power saving
> and expects drivers manage clocks using clk API, while PowerPC
> mostly does not do that, and thus does not always build in clk API.
>
> Listing all those driver Kconfig options in "select PPC_CLOCK if" seems
> not scalable for long term maintenance, and could easily introduce
> Kconfig recursive dependency. This patch chooses to select PPC_CLOCK
> unconditionally for FSL_SOC to always build clk API for PowerPC in.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
> arch/powerpc/Kconfig | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index feab3ba..63fa7fb 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -661,7 +661,7 @@ config SBUS
> config FSL_SOC
> bool
> select HAVE_CAN_FLEXCAN if NET && CAN
> - select PPC_CLOCK if CAN_FLEXCAN
> + select PPC_CLOCK
>
> config FSL_PCI
> bool
> --
> 1.7.5.4
>
^ permalink raw reply
* Re: [RFC] powerpc/fsl-pci: Document the "fsl, has-isa" property for Freescale PCI
From: Kumar Gala @ 2012-04-04 13:08 UTC (permalink / raw)
To: Jia Hongtao; +Cc: devicetree-discuss, linuxppc-dev
In-Reply-To: <1333263396-23932-1-git-send-email-B38951@freescale.com>
On Apr 1, 2012, at 1:56 AM, Jia Hongtao wrote:
> If PCI is primary bus we should set isa_io/mem_base when parsing PCI =
bridge
> resources from device tree. The previous way to check the primary bus =
based
> on a hard-coded address named primary_phb_addr. Now we add a property =
named
> "fsl,has-isa" into device tree. In kernel we use this property to find =
out
> the bus is primary or not. This way is more flexible.
>=20
> Signed-off-by: Jia Hongtao <B38951@freescale.com>
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
> .../devicetree/bindings/powerpc/fsl/pci.txt | 36 =
++++++++++++++++++++
> 1 files changed, 36 insertions(+), 0 deletions(-)
> create mode 100644 =
Documentation/devicetree/bindings/powerpc/fsl/pci.txt
This isn't freescale specific, its linux specific. If anything the =
property should be linux,has-isa.
But in general I dont think this is a good idea. In truth one could =
search the device tree for:
device_type =3D "isa";
to try and set this dynamically.
- k
^ permalink raw reply
* Re: [PATCH v5 06/27] irq_domain/powerpc: eliminate irq_map; use irq_alloc_desc() instead
From: Andreas Schwab @ 2012-04-04 12:51 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: devicetree-discuss, linux-kernel, Rob Herring, Milton Miller,
Thomas Gleixner, linuxppc-dev, linux-arm-kernel
In-Reply-To: <1333489422.3040.11.camel@pasglop>
Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:
> Do you see a relevant difference in the X log ?
Nothing at all.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: 3.4.0-rc1: No init found
From: Suzuki K. Poulose @ 2012-04-04 12:36 UTC (permalink / raw)
To: Christian Kujau; +Cc: linuxppc-dev, LKML, Al Viro
In-Reply-To: <alpine.DEB.2.01.1204030953320.4930@trent.utfs.org>
On 04/03/2012 10:48 PM, Christian Kujau wrote:
> On Tue, 3 Apr 2012 at 18:08, Benjamin Herrenschmidt wrote:
>> I have observed this randomly on the G5 ... sometimes, if I try again,
>> it works... it's very very odd. There is some kind of race maybe with
>> async startup ? Or a problem with the vfs path walking ? It's certainly
>> not easily reproducable for me, it goes away from one boot to the next.
>
> It's 100% reproducible for me. This PowerBook G4 (1.25Ghz) is not the
> fastes though, maybe a race triggers more easily here...?
>
>>> PS: Unfortunately I cannot boot into the old (3.3-rc7) kernel
>>> right now (which is still installed via "yaboot" and present in
>>> /boot), because of this:
>>> http://nerdbynature.de/bits/3.4.0-rc1/init/mac-invalid-memory.JPG
>>> Booting into Debian's "squeeze" kernel (2.6.32) which resides in
>>> the same /boot directory succeeds.
>>
>> Hrm, did it used to boot ?
>
> I'm using the "backup" kernel only when the new one has an issue, so I
> have not tested it for a while, but it used to work, for sure.
>
>> Can you do printenv in OF and tell me what
>> your load-base, real-base, virt-base etc... are ?
>
> load-base is 0x800000, real-base and virt-base is set to "-1", please see
> http://nerdbynature.de/bits/3.4.0-rc1/init/printenv-1.JPG
>
> Not sure if this is related, but at the end of each kernel compilation,
> the following messages are printed:
>
> ------------
> SYSMAP System.map
> SYSMAP .tmp_System.map
> WRAP arch/powerpc/boot/zImage.pmac
> INFO: Uncompressed kernel (size 0x6e52f8) overlaps the address of the wrapper(0x400000)
> INFO: Fixing the link_address of wrapper to (0x700000)
> WRAP arch/powerpc/boot/zImage.coff
> INFO: Uncompressed kernel (size 0x6e52f8) overlaps the address of the wrapper(0x500000)
> INFO: Fixing the link_address of wrapper to (0x700000)
> WRAP arch/powerpc/boot/zImage.miboot
> INFO: Uncompressed kernel (size 0x6d4b80) overlaps the address of the wrapper(0x400000)
> INFO: Fixing the link_address of wrapper to (0x700000)
> Building modules, stage 2.
> MODPOST 24 modules
> ------------
>
> I started to see these messages in January (around Linux 3.2.0), but never
> investigated what it was since the produced kernels continued to boot just
> fine.
The above change was added by me. The message is printed when the
'wrapper' script finds that decompressed kernel overlaps the 'bootstrap
code' which does the decompression. So it shifts the 'address' of the
bootstrap code to the next higher MB. As such it is harmless.
Thanks
Suzuki
^ permalink raw reply
* [PATCH 1/1] dts:bluestone: Add support 2 SATA ports
From: Thang Q. Nguyen @ 2012-04-04 8:38 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Vinh Nguyen Huu Tuong,
Josh Boyer
Cc: linux-kernel, linuxppc-dev, Thang Q. Nguyen
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1701 bytes --]
Adding 2 SATA nodes on Bluestone device tree file.
Signed-off-by: Thang Q. Nguyen <tqnguyen@apm.com>
---
arch/powerpc/boot/dts/bluestone.dts | 21 +++++++++++++++++++++
1 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/boot/dts/bluestone.dts b/arch/powerpc/boot/dts/bluestone.dts
index cfa23bf..803fda6 100644
--- a/arch/powerpc/boot/dts/bluestone.dts
+++ b/arch/powerpc/boot/dts/bluestone.dts
@@ -155,6 +155,27 @@
/*RXDE*/ 0x5 0x4>;
};
+ /* SATA DWC devices */
+ SATA0: sata@bffd1000 {
+ compatible = "amcc,sata-apm821xx";
+ reg = <4 0xbffd1000 0x800 /* SATA0 */
+ 4 0xbffd0800 0x400>; /* AHBDMA */
+ dma-channel=<0>;
+ interrupt-parent = <&UIC0>;
+ interrupts = <26 4 /* SATA0 */
+ 25 4>; /* AHBDMA */
+ };
+
+ SATA1: sata@bffd1800 {
+ compatible = "amcc,sata-apm821xx";
+ reg = <4 0xbffd1800 0x800 /* SATA1 */
+ 4 0xbffd0800 0x400>; /* AHBDMA */
+ dma-channel=<1>;
+ interrupt-parent = <&UIC0>;
+ interrupts = <27 4 /* SATA1 */
+ 25 4>; /* AHBDMA */
+ };
+
POB0: opb {
compatible = "ibm,opb";
#address-cells = <1>;
--
1.7.3.4
CONFIDENTIALITY NOTICE: This e-mail message, including any attachments,
is for the sole use of the intended recipient(s) and contains information
that is confidential and proprietary to AppliedMicro Corporation or its subsidiaries.
It is to be used solely for the purpose of furthering the parties' business relationship.
All unauthorized review, use, disclosure or distribution is prohibited.
If you are not the intended recipient, please contact the sender by reply e-mail
and destroy all copies of the original message.
^ permalink raw reply related
* RE: [PATCH 1/1] Add support 2 SATA ports for Maui and change filename from sata_dwc_460ex.c to sata_dwc_4xx.c
From: Thang Nguyen @ 2012-04-04 8:18 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: devicetree-discuss, linux-kernel, Rob Herring, linux-ide,
Paul Mackerras, Jeff Garzik, linuxppc-dev
In-Reply-To: <4F7AE584.3050805@mvista.com>
Thanks Sergei for quick response.
In the original driver (sata_dwc_460ex.c), an instance of the
sata_dwc_host_priv structure is declared and used globally. This will not
require functions to have ata_port structure or alternative structures
(ata_link, hsdev,...) to be declared in the function calls.
However, for supporting 2 ports, we can't use the sata_dwc_host_priv
structure as functions need to know what SATA port they are working on.
This is the reason why you see the ata_port, ata_link, sata_dwc_device,
sata_dwc_device_port,... structures declared in functions.
I will update the patch as your feedback and separate also the
bluestone.dts device tree to the another patch
Thanks and best regards,
Thang Nguyen -
Applied Micro
-----Original Message-----
From: Sergei Shtylyov [mailto:sshtylyov@mvista.com]
Sent: Tuesday, April 03, 2012 6:57 PM
To: Thang Q. Nguyen
Cc: Benjamin Herrenschmidt; Paul Mackerras; Jeff Garzik; Grant Likely; Rob
Herring; linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org;
linux-ide@vger.kernel.org; devicetree-discuss@lists.ozlabs.org
Subject: Re: [PATCH 1/1] Add support 2 SATA ports for Maui and change
filename from sata_dwc_460ex.c to sata_dwc_4xx.c
Hello.
On 03-04-2012 14:12, Thang Q. Nguyen wrote:
> Signed-off-by: Thang Q. Nguyen<tqnguyen@apm.com>
> ---
> Changes for v2:
> - Use git rename feature to change the driver to the newname and
for
> easier review.
> arch/powerpc/boot/dts/bluestone.dts | 21 +
> drivers/ata/Makefile | 2 +-
> drivers/ata/{sata_dwc_460ex.c => sata_dwc_4xx.c} | 1371
++++++++++++++--------
> 3 files changed, 904 insertions(+), 490 deletions(-)
> rename drivers/ata/{sata_dwc_460ex.c => sata_dwc_4xx.c} (56%)
You submitted a magapatch doing several things at once (some even
needlessly) and even in two areas of the kernel. This needs proper
splitting/description.
> diff --git a/arch/powerpc/boot/dts/bluestone.dts
b/arch/powerpc/boot/dts/bluestone.dts
> index cfa23bf..803fda6 100644
> --- a/arch/powerpc/boot/dts/bluestone.dts
> +++ b/arch/powerpc/boot/dts/bluestone.dts
> @@ -155,6 +155,27 @@
> /*RXDE*/ 0x5 0x4>;
> };
>
> + /* SATA DWC devices */
> + SATA0: sata@bffd1000 {
> + compatible = "amcc,sata-apm821xx";
> + reg =<4 0xbffd1000 0x800 /* SATA0 */
> + 4 0xbffd0800 0x400>; /* AHBDMA */
> + dma-channel=<0>;
> + interrupt-parent =<&UIC0>;
> + interrupts =<26 4 /* SATA0 */
> + 25 4>; /* AHBDMA */
> + };
> +
> + SATA1: sata@bffd1800 {
> + compatible = "amcc,sata-apm821xx";
> + reg =<4 0xbffd1800 0x800 /* SATA1 */
> + 4 0xbffd0800 0x400>; /* AHBDMA */
> + dma-channel=<1>;
> + interrupt-parent =<&UIC0>;
> + interrupts =<27 4 /* SATA1 */
> + 25 4>; /* AHBDMA */
> + };
> +
> POB0: opb {
> compatible = "ibm,opb";
> #address-cells =<1>;
The above should be in a separate patch for PPC people, of course.
> diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_4xx.c
> similarity index 56%
> rename from drivers/ata/sata_dwc_460ex.c
> rename to drivers/ata/sata_dwc_4xx.c
> index 69f7cde..bdbb35a 100644
> --- a/drivers/ata/sata_dwc_460ex.c
> +++ b/drivers/ata/sata_dwc_4xx.c
> @@ -1,5 +1,5 @@
> /*
> - * drivers/ata/sata_dwc_460ex.c
> + * drivers/ata/sata_dwc_4xx.c
This line should be removed altogether.
> *
> * Synopsys DesignWare Cores (DWC) SATA host driver
> *
[...]
> @@ -135,13 +146,12 @@ enum {
> DMA_CTL_LLP_DSTEN = 0x08000000, /* Blk chain enable Dst */
> };
>
> -#define DMA_CTL_BLK_TS(size) ((size)& 0x000000FFF) /* Blk
Transfer size */
> +#define DMA_CTL_BLK_TS(size) ((size)& 0x000000FFF) /* Blk Transfer
size */
Avoid random whitespoace changes.
> #define DMA_CHANNEL(ch) (0x00000001<< (ch)) /* Select
channel */
> /* Enable channel */
> -#define DMA_ENABLE_CHAN(ch) ((0x00000001<< (ch)) |
\
> - ((0x000000001<< (ch))<< 8))
> +#define DMA_ENABLE_CHAN(ch) (0x00000101<< (ch))
> /* Disable channel */
> -#define DMA_DISABLE_CHAN(ch) (0x00000000 | ((0x000000001<<
(ch))<< 8))
> +#define DMA_DISABLE_CHAN(ch) (0x000000100<< (ch))
> /* Transfer Type& Flow Controller */
These cleanups are not related to adding support for 2 channels
> @@ -298,43 +313,32 @@ struct sata_dwc_device_port {
> #define HSDEV_FROM_QC(qc) ((struct sata_dwc_device *)\
> (qc)->ap->host->private_data)
> #define HSDEV_FROM_HSDEVP(p) ((struct sata_dwc_device *)\
> - (hsdevp)->hsdev)
> + (hsdevp)->hsdev)
Avoid random whitespoace changes.
> +/*
> + * Globals
> + */
> +static struct sata_dwc_device *dwc_dev_list[2];
> +static struct ahb_dma_regs *sata_dma_regs;
This assumes that the system only has single controller, doesn't it?
> /*
> - * Function: get_burst_length_encode
> - * arguments: datalength: length in bytes of data
> - * returns value to be programmed in register corresponding to data
length
> + * Calculate value to be programmed in register corresponding to data
length
> * This value is effectively the log(base 2) of the length
> */
> -static int get_burst_length_encode(int datalength)
> +static int get_burst_length_encode(int datalength)
Is it releated to adding support to 2 ports?
> {
> int items = datalength>> 2; /* div by 4 to get lword count */
>
> @@ -414,152 +416,205 @@ static int get_burst_length_encode(int
datalength)
> return 0;
> }
>
> -static void clear_chan_interrupts(int c)
> +/*
> + * Clear channel interrupt. No interrupt for the specified channel
> + * generated until it is enabled again.
> + */
> +static void clear_chan_interrupts(int c)
> {
> - out_le32(&(host_pvt.sata_dma_regs->interrupt_clear.tfr.low),
> - DMA_CHANNEL(c));
> - out_le32(&(host_pvt.sata_dma_regs->interrupt_clear.block.low),
> + out_le32(&(sata_dma_regs->interrupt_clear.tfr.low),
DMA_CHANNEL(c));
> + out_le32(&(sata_dma_regs->interrupt_clear.block.low),
DMA_CHANNEL(c));
> + out_le32(&(sata_dma_regs->interrupt_clear.srctran.low),
> DMA_CHANNEL(c));
> - out_le32(&(host_pvt.sata_dma_regs->interrupt_clear.srctran.low),
> - DMA_CHANNEL(c));
> - out_le32(&(host_pvt.sata_dma_regs->interrupt_clear.dsttran.low),
> - DMA_CHANNEL(c));
> - out_le32(&(host_pvt.sata_dma_regs->interrupt_clear.error.low),
> + out_le32(&(sata_dma_regs->interrupt_clear.dsttran.low),
> DMA_CHANNEL(c));
> + out_le32(&(sata_dma_regs->interrupt_clear.error.low),
DMA_CHANNEL(c));
() with & are not necessary.
> }
>
> /*
> - * Function: dma_request_channel
> - * arguments: None
> - * returns channel number if available else -1
> - * This function assigns the next available DMA channel from the list
to the
> - * requester
> + * Check if the selected DMA channel is currently enabled.
> */
> -static int dma_request_channel(void)
> +static int sata_dwc_dma_chk_en(int ch)
> {
> - int i;
> + u32 dma_chan_reg;
> + /* Read the DMA channel register */
> + dma_chan_reg = in_le32(&(sata_dma_regs->dma_chan_en.low));
> + /* Check if it is currently enabled */
> + if (dma_chan_reg & DMA_CHANNEL(ch))
> + return 1;
> + return 0;
> +}
Is this related to the claimed subject of adding support for 2 ports?
>
> - for (i = 0; i< DMA_NUM_CHANS; i++) {
> - if
(!(in_le32(&(host_pvt.sata_dma_regs->dma_chan_en.low))&\
> - DMA_CHANNEL(i)))
> - return i;
> +/*
> + * Terminate the current DMA transfer
> + *
> + * Refer to the "Abnormal Transfer Termination" section
> + * Disable the corresponding bit in the ChEnReg register
> + * and poll that register to until the channel is terminated.
> + */
> +static void sata_dwc_dma_terminate(struct ata_port *ap, int dma_ch)
> +{
> + int enabled = sata_dwc_dma_chk_en(dma_ch);
> + /* If the channel is currenly in use, release it. */
> + if (enabled) {
> + dev_dbg(ap->dev,
> + "%s terminate DMA on channel=%d (mask=0x%08x)
...",
> + __func__, dma_ch, DMA_DISABLE_CHAN(dma_ch));
> + dev_dbg(ap->dev, "ChEnReg=0x%08x\n",
> + in_le32(&(sata_dma_regs->dma_chan_en.low)));
> + /* Disable the selected channel */
> + out_le32(&(sata_dma_regs->dma_chan_en.low),
> + DMA_DISABLE_CHAN(dma_ch));
> +
> + /* Wait for the channel is disabled */
> + do {
> + enabled = sata_dwc_dma_chk_en(dma_ch);
> + ndelay(1000);
> + } while (enabled);
> + dev_dbg(ap->dev, "done\n");
> }
> - dev_err(host_pvt.dwc_dev, "%s NO channel chan_en: 0x%08x\n",
__func__,
> - in_le32(&(host_pvt.sata_dma_regs->dma_chan_en.low)));
> +}
Same question.
> +
> +/*
> + * Check if the DMA channel is currently available for transferring
data
> + * on the specified ata_port.
> + */
> +static int dma_request_channel(struct ata_port *ap)
> +{
> + struct sata_dwc_device *hsdev = HSDEV_FROM_AP(ap);
> +
> + /* Check if the channel is not currently in use */
> + if (!(in_le32(&(sata_dma_regs->dma_chan_en.low))&\
> + DMA_CHANNEL(hsdev->dma_channel)))
> + return hsdev->dma_channel;
> +
> + dev_err(ap->dev, "%s Channel %d is currently in use\n", __func__,
> + hsdev->dma_channel);
> return -1;
> }
Same question.
> +/*
> + * Registers ISR for a particular DMA channel interrupt
> + */
> +static int dma_request_interrupts(struct sata_dwc_device *hsdev, int
irq)
> +{
> + int retval;
> +
> + /* Unmask error interrupt */
> + out_le32(&sata_dma_regs->interrupt_mask.error.low,
> + in_le32(&sata_dma_regs->interrupt_mask.error.low) |
> + DMA_ENABLE_CHAN(hsdev->dma_channel));
> +
> + /* Unmask end-of-transfer interrupt */
> + out_le32(&sata_dma_regs->interrupt_mask.tfr.low,
> + in_le32(&sata_dma_regs->interrupt_mask.tfr.low) |
> + DMA_ENABLE_CHAN(hsdev->dma_channel));
> +
> + retval = request_irq(irq, dma_dwc_handler, IRQF_SHARED, "SATA
DMA",
> + hsdev);
> if (retval) {
> - dev_err(host_pvt.dwc_dev, "%s: could not get IRQ %d\n",
> + dev_err(hsdev->dev, "%s: could not get IRQ %d\n",\
> __func__, irq);
> return -ENODEV;
> }
>
> /* Mark this interrupt as requested */
> hsdev->irq_dma = irq;
> +
> return 0;
> }
Same question.
> /*
> - * Function: dma_dwc_exit
> - * arguments: None
> - * returns status
> - * This function exits the SATA DMA driver
> - */
> -static void dma_dwc_exit(struct sata_dwc_device *hsdev)
> -{
> - dev_dbg(host_pvt.dwc_dev, "%s:\n", __func__);
> - if (host_pvt.sata_dma_regs) {
> - iounmap(host_pvt.sata_dma_regs);
> - host_pvt.sata_dma_regs = NULL;
> - }
> -
> - if (hsdev->irq_dma) {
> - free_irq(hsdev->irq_dma, hsdev);
> - hsdev->irq_dma = 0;
> - }
> -}
Same question.
> static int sata_dwc_scr_read(struct ata_link *link, unsigned int scr,
u32 *val)
> {
> - if (scr> SCR_NOTIFICATION) {
> + if (unlikely(scr> SCR_NOTIFICATION)) {
> dev_err(link->ap->dev, "%s: Incorrect SCR offset
0x%02x\n",
> __func__, scr);
> return -EINVAL;
> }
>
> *val = in_le32((void *)link->ap->ioaddr.scr_addr + (scr * 4));
> - dev_dbg(link->ap->dev, "%s: id=%d reg=%d val=val=0x%08x\n",
> + dev_dbg(link->ap->dev, "%s: id=%d reg=%d val=0x%08x\n",
> __func__, link->ap->print_id, scr, *val);
>
> return 0;
> @@ -828,7 +867,7 @@ static int sata_dwc_scr_write(struct ata_link *link,
unsigned int scr, u32 val)
> {
> dev_dbg(link->ap->dev, "%s: id=%d reg=%d val=val=0x%08x\n",
> __func__, link->ap->print_id, scr, val);
> - if (scr> SCR_NOTIFICATION) {
> + if (unlikely(scr> SCR_NOTIFICATION)) {
> dev_err(link->ap->dev, "%s: Incorrect SCR offset
0x%02x\n",
> __func__, scr);
> return -EINVAL;
Same question.
> @@ -838,23 +877,24 @@ static int sata_dwc_scr_write(struct ata_link
*link, unsigned int scr, u32 val)
> return 0;
> }
>
> -static u32 core_scr_read(unsigned int scr)
> +static u32 core_scr_read(struct ata_port *ap, unsigned int scr)
> {
> - return in_le32((void __iomem *)(host_pvt.scr_addr_sstatus) +\
> - (scr * 4));
> + struct sata_dwc_device *hsdev = HSDEV_FROM_AP(ap);
Insert empty line here, please.
> + return in_le32((void __iomem *)hsdev->scr_base + (scr * 4));
> }
>
> -static void core_scr_write(unsigned int scr, u32 val)
> +
> +static void core_scr_write(struct ata_port *ap, unsigned int scr, u32
val)
> {
> - out_le32((void __iomem *)(host_pvt.scr_addr_sstatus) + (scr * 4),
> - val);
> + struct sata_dwc_device *hsdev = HSDEV_FROM_AP(ap);
And here.
> + out_le32((void __iomem *)hsdev->scr_base + (scr * 4), val);
> }
>
> -static void clear_serror(void)
> +static void clear_serror(struct ata_port *ap)
> {
> - u32 val;
> - val = core_scr_read(SCR_ERROR);
> - core_scr_write(SCR_ERROR, val);
> + struct sata_dwc_device *hsdev = HSDEV_FROM_AP(ap);
And here.
> + out_le32((void __iomem *)hsdev->scr_base + 4,
> + in_le32((void __iomem *)hsdev->scr_base + 4));
>
> }
>
> @@ -864,12 +904,105 @@ static void clear_interrupt_bit(struct
sata_dwc_device *hsdev, u32 bit)
> in_le32(&hsdev->sata_dwc_regs->intpr));
> }
>
> +/*
> + * Porting the ata_bus_softreset function from the libata-sff.c
library.
> + */
> +static int sata_dwc_bus_softreset(struct ata_port *ap, unsigned int
devmask,
> + unsigned long deadline)
> +{
> + struct ata_ioports *ioaddr =&ap->ioaddr;
> +
> + DPRINTK("ata%u: bus reset via SRST\n", ap->print_id);
> +
> + /* Software reset. causes dev0 to be selected */
> + iowrite8(ap->ctl, ioaddr->ctl_addr);
> + udelay(20); /* FIXME: flush */
> + iowrite8(ap->ctl | ATA_SRST, ioaddr->ctl_addr);
> + udelay(20); /* FIXME: flush */
> + iowrite8(ap->ctl, ioaddr->ctl_addr);
> + ap->last_ctl = ap->ctl;
> +
> + /* Wait the port to become ready */
> + return ata_sff_wait_after_reset(&ap->link, devmask, deadline);
> +}
I don't see
> +
> +/*
> + * Do soft reset on the current SATA link.
> + */
> +static int sata_dwc_softreset(struct ata_link *link, unsigned int
*classes,
> + unsigned long deadline)
> +{
> + int rc;
> + u8 err;
> + struct ata_port *ap = link->ap;
> + unsigned int devmask = 0;
Why delcare it at all if it's always 0?
> + struct sata_dwc_device *hsdev = HSDEV_FROM_AP(ap);
> +
> + /* Select device 0 again */
> + ap->ops->sff_dev_select(ap, 0);
> +
> + DPRINTK("about to softreset, devmask=%x\n", devmask);
> + rc = sata_dwc_bus_softreset(ap, devmask, deadline);
> +
> + /* If link is occupied, -ENODEV too is an error */
> + if (rc && (rc != -ENODEV || sata_scr_valid(link))) {
> + ata_link_printk(link, KERN_ERR, "SRST failed(errno=%d)\n",
rc);
> + return rc;
> + }
> +
> + /* Determine by signature whether we have ATA or ATAPI devices */
> + classes[0] = ata_sff_dev_classify(&link->device[0],
> + devmask& (1<< 0),&err);
Always 0, and it should be 1.
> + DPRINTK("EXIT, classes[0]=%u [1]=%u\n", classes[0], classes[1]);
classes[1] will always be empty.
> + clear_serror(link->ap);
> +
> + /* Terminate DMA if it is currently in use */
> + sata_dwc_dma_terminate(link->ap, hsdev->dma_channel);
Isn't it too late?
> +
> + return rc;
> +}
> +
> +/*
> + * Reset all internal parameters to default value.
> + * This function should be called in hardreset
> + */
> +static void dwc_reset_internal_params(struct ata_port *ap)
> +{
> + struct sata_dwc_device_port *hsdevp = HSDEVP_FROM_AP(ap);
> + int tag;
Empty line here please.
> + for (tag = 0; tag< SATA_DWC_QCMD_MAX; tag++)
> + hsdevp->dma_pending[tag] = SATA_DWC_DMA_PENDING_NONE;
> +
> + hsdevp->sata_dwc_sactive_issued = 0;
> + hsdevp->sata_dwc_sactive_queued = 0;
> +}
> +
> +static int sata_dwc_hardreset(struct ata_link *link, unsigned int
*classes,
> + unsigned long deadline)
> +{
> + int rc;
> + const unsigned long *timing =
sata_ehc_deb_timing(&link->eh_context);
> + bool online;
> +
> + /* Reset internal parameters to default values */
> + dwc_reset_internal_params(link->ap);
> +
> + /* Call standard hard reset */
> + rc = sata_link_hardreset(link, timing, deadline,&online, NULL);
> +
> + /* Reconfigure the port after hard reset */
> + if (ata_link_online(link))
> + sata_dwc_init_port(link->ap);
> +
> + return online ? -EAGAIN : rc;
> +}
> +
What does this have to do with adding support for 2 ports again?
> @@ -918,11 +1049,7 @@ static void sata_dwc_error_intr(struct ata_port
*ap,
> }
>
> /*
> - * Function : sata_dwc_isr
> - * arguments : irq, void *dev_instance, struct pt_regs *regs
> - * Return value : irqreturn_t - status of IRQ
> * This Interrupt handler called via port ops registered function.
> - * .irq_handler = sata_dwc_isr
> */
> static irqreturn_t sata_dwc_isr(int irq, void *dev_instance)
> {
> @@ -930,14 +1057,14 @@ static irqreturn_t sata_dwc_isr(int irq, void
*dev_instance)
> struct sata_dwc_device *hsdev = HSDEV_FROM_HOST(host);
> struct ata_port *ap;
> struct ata_queued_cmd *qc;
> - unsigned long flags;
> u8 status, tag;
> - int handled, num_processed, port = 0;
> - uint intpr, sactive, sactive2, tag_mask;
> + int handled, port = 0;
> + int num_lli;
> + uint intpr, sactive, tag_mask;
> struct sata_dwc_device_port *hsdevp;
> - host_pvt.sata_dwc_sactive_issued = 0;
> + u32 mask;
>
> - spin_lock_irqsave(&host->lock, flags);
> + spin_lock(&host->lock);
>
> /* Read the interrupt register */
> intpr = in_le32(&hsdev->sata_dwc_regs->intpr);
> @@ -958,38 +1085,61 @@ static irqreturn_t sata_dwc_isr(int irq, void
*dev_instance)
> /* Check for DMA SETUP FIS (FP DMA) interrupt */
> if (intpr& SATA_DWC_INTPR_NEWFP) {
> clear_interrupt_bit(hsdev, SATA_DWC_INTPR_NEWFP);
> + if (ap->qc_allocated == 0x0) {
> + handled = 1;
> + goto DONE;
> + }
>
> tag = (u8)(in_le32(&hsdev->sata_dwc_regs->fptagr));
> + mask = qcmd_tag_to_mask(tag);
> dev_dbg(ap->dev, "%s: NEWFP tag=%d\n", __func__, tag);
> - if (hsdevp->cmd_issued[tag] != SATA_DWC_CMD_ISSUED_PEND)
> + if ((hsdevp->sata_dwc_sactive_queued& mask) == 0)
> dev_warn(ap->dev, "CMD tag=%d not pending?\n",
tag);
>
> - host_pvt.sata_dwc_sactive_issued |= qcmd_tag_to_mask(tag);
> -
> qc = ata_qc_from_tag(ap, tag);
> /*
> * Start FP DMA for NCQ command. At this point the tag is
the
> * active tag. It is the tag that matches the command
about to
> * be completed.
> */
> - qc->ap->link.active_tag = tag;
> - sata_dwc_bmdma_start_by_tag(qc, tag);
> + if (qc) {
> + hsdevp->sata_dwc_sactive_issued |= mask;
> + /* Prevent to issue more commands */
> + qc->ap->link.active_tag = tag;
> + qc->dev->link->sactive |= (1<< qc->tag);
> + num_lli = map_sg_to_lli(ap, qc->sg, qc->n_elem, \
> + hsdevp->llit[tag], hsdevp->llit_dma[tag],
\
> + (void
*__iomem)(&hsdev->sata_dwc_regs->dmadr), \
> + qc->dma_dir);
> + sata_dwc_bmdma_start_by_tag(qc, tag);
> + wmb();
> + qc->ap->hsm_task_state = HSM_ST_LAST;
> + } else {
> + hsdevp->sata_dwc_sactive_issued&= ~mask;
> + dev_warn(ap->dev, "No QC available for tag %d (intpr="
> + "0x%08x, qc_allocated=0x%08x, qc_active=0x%08x)\n",
tag,\
> + intpr, ap->qc_allocated, ap->qc_active);
Indent the above preperly with tabs.
> + }
>
[...]
> @@ -1167,70 +1245,51 @@ static void sata_dwc_clear_dmacr(struct
sata_dwc_device_port *hsdevp, u8 tag)
> }
> }
>
> -static void sata_dwc_dma_xfer_complete(struct ata_port *ap, u32
check_status)
> -{
> - struct ata_queued_cmd *qc;
> - struct sata_dwc_device_port *hsdevp = HSDEVP_FROM_AP(ap);
> - struct sata_dwc_device *hsdev = HSDEV_FROM_AP(ap);
> - u8 tag = 0;
> -
> - tag = ap->link.active_tag;
> - qc = ata_qc_from_tag(ap, tag);
> - if (!qc) {
> - dev_err(ap->dev, "failed to get qc");
> - return;
> - }
> -
> -#ifdef DEBUG_NCQ
> - if (tag> 0) {
> - dev_info(ap->dev, "%s tag=%u cmd=0x%02x dma dir=%s
proto=%s "
> - "dmacr=0x%08x\n", __func__, qc->tag,
qc->tf.command,
> - get_dma_dir_descript(qc->dma_dir),
> - get_prot_descript(qc->tf.protocol),
> - in_le32(&(hsdev->sata_dwc_regs->dmacr)));
> - }
> -#endif
> -
> - if (ata_is_dma(qc->tf.protocol)) {
> - if (hsdevp->dma_pending[tag] == SATA_DWC_DMA_PENDING_NONE)
{
> - dev_err(ap->dev, "%s DMA protocol RX and TX DMA
not "
> - "pending dmacr: 0x%08x\n", __func__,
> - in_le32(&(hsdev->sata_dwc_regs->dmacr)));
> - }
> -
> - hsdevp->dma_pending[tag] = SATA_DWC_DMA_PENDING_NONE;
> - sata_dwc_qc_complete(ap, qc, check_status);
> - ap->link.active_tag = ATA_TAG_POISON;
> - } else {
> - sata_dwc_qc_complete(ap, qc, check_status);
> - }
> -}
What does this chenge have to do with the claimed target of thye
patch.
> static int sata_dwc_qc_complete(struct ata_port *ap, struct
ata_queued_cmd *qc,
> u32 check_status)
> {
> - u8 status = 0;
> - u32 mask = 0x0;
> + u8 status;
> + int i;
> u8 tag = qc->tag;
> struct sata_dwc_device_port *hsdevp = HSDEVP_FROM_AP(ap);
> - host_pvt.sata_dwc_sactive_queued = 0;
> + u32 serror;
> dev_dbg(ap->dev, "%s checkstatus? %x\n", __func__, check_status);
>
> - if (hsdevp->dma_pending[tag] == SATA_DWC_DMA_PENDING_TX)
> - dev_err(ap->dev, "TX DMA PENDING\n");
> - else if (hsdevp->dma_pending[tag] == SATA_DWC_DMA_PENDING_RX)
> - dev_err(ap->dev, "RX DMA PENDING\n");
> + /* Check main status, clearing INTRQ */
> + status = ap->ops->sff_check_status(ap);
> +
> + if (check_status) {
> + i = 0;
> + while (status& ATA_BUSY) {
> + if (++i> 10)
> + break;
> + status = ap->ops->sff_check_altstatus(ap);
> + };
> +
> + if (unlikely(status& ATA_BUSY))
> + dev_err(ap->dev, "QC complete cmd=0x%02x STATUS
BUSY "
> + "(0x%02x) [%d]\n", qc->tf.command, status,
i);
> + serror = core_scr_read(ap, SCR_ERROR);
> + if (unlikely(serror& SATA_DWC_SERROR_ERR_BITS))
> + dev_err(ap->dev, "****** SERROR=0x%08x ******\n",
> + serror);
> + }
> dev_dbg(ap->dev, "QC complete cmd=0x%02x status=0x%02x ata%u:"
> " protocol=%d\n", qc->tf.command, status, ap->print_id,
> qc->tf.protocol);
>
> - /* clear active bit */
> - mask = (~(qcmd_tag_to_mask(tag)));
> - host_pvt.sata_dwc_sactive_queued =
(host_pvt.sata_dwc_sactive_queued) \
> - & mask;
> - host_pvt.sata_dwc_sactive_issued =
(host_pvt.sata_dwc_sactive_issued) \
> - & mask;
> - ata_qc_complete(qc);
> + hsdevp->sata_dwc_sactive_issued&= ~qcmd_tag_to_mask(tag);
> +
> + /* Complete taskfile transaction (does not read SCR registers) */
> + if (ata_is_atapi(qc->tf.protocol))
> + ata_sff_hsm_move(ap, qc, status, 0);
> + else
> + ata_qc_complete(qc);
> +
> + if (hsdevp->sata_dwc_sactive_queued == 0)
> + ap->link.active_tag = ATA_TAG_POISON;
> +
> return 0;
> }
Same question.
> +static void sata_dwc_init_port(struct ata_port *ap)
> +{
> + struct sata_dwc_device *hsdev = HSDEV_FROM_AP(ap);
> +
> + /* Configure DMA */
> + if (ap->port_no == 0) {
> + dev_dbg(ap->dev, "%s: clearing TXCHEN, RXCHEN in DMAC\n",
> + __func__);
> +
> + /* Clear all transmit/receive bits */
> + out_le32(&hsdev->sata_dwc_regs->dmacr,
> + SATA_DWC_DMACR_TXRXCH_CLEAR);
> +
> + dev_dbg(ap->dev, "%s: setting burst size DBTSR\n",
__func__);
> + out_le32(&hsdev->sata_dwc_regs->dbtsr,
> + (SATA_DWC_DBTSR_MWR(AHB_DMA_BRST_DFLT) |
> + SATA_DWC_DBTSR_MRD(AHB_DMA_BRST_DFLT)));
Why not put the above init. in a separate function, instead of
associating
with channehl 0?
> + }
> +
> + /* Enable interrupts */
> + sata_dwc_enable_interrupts(hsdev);
> +}
> +
> static void sata_dwc_setup_port(struct ata_ioports *port, unsigned
long base)
> {
> port->cmd_addr = (void *)base + 0x00;
> @@ -1276,10 +1359,7 @@ static void sata_dwc_setup_port(struct
ata_ioports *port, unsigned long base)
> }
>
> /*
> - * Function : sata_dwc_port_start
> - * arguments : struct ata_ioports *port
> - * Return value : returns 0 if success, error code otherwise
> - * This function allocates the scatter gather LLI table for AHB DMA
> + * Allocates the scatter gather LLI table for AHB DMA
> */
> static int sata_dwc_port_start(struct ata_port *ap)
> {
> @@ -1287,6 +1367,7 @@ static int sata_dwc_port_start(struct ata_port
*ap)
> struct sata_dwc_device *hsdev;
> struct sata_dwc_device_port *hsdevp = NULL;
> struct device *pdev;
> + u32 sstatus;
> int i;
>
> hsdev = HSDEV_FROM_AP(ap);
> @@ -1308,12 +1389,10 @@ static int sata_dwc_port_start(struct ata_port
*ap)
> err = -ENOMEM;
> goto CLEANUP;
> }
> + memset(hsdevp, 0, sizeof(*hsdevp));
We already called kzalloc(), so the allocated buffer is already
cleared.
> hsdevp->hsdev = hsdev;
>
> - for (i = 0; i< SATA_DWC_QCMD_MAX; i++)
> - hsdevp->cmd_issued[i] = SATA_DWC_CMD_ISSUED_NOT;
> -
> - ap->bmdma_prd = 0; /* set these so libata doesn't use them */
> + ap->bmdma_prd = 0; /* set these so libata doesn't use them */
Avoid random whitespace changes.
> @@ -1347,32 +1426,47 @@ static int sata_dwc_port_start(struct ata_port
*ap)
> }
>
> /* Clear any error bits before libata starts issuing commands */
> - clear_serror();
> + clear_serror(ap);
> ap->private_data = hsdevp;
> +
> + /* Are we in Gen I or II */
> + sstatus = core_scr_read(ap, SCR_STATUS);
> + switch (SATA_DWC_SCR0_SPD_GET(sstatus)) {
> + case 0x0:
> + dev_info(ap->dev, "**** No neg speed (nothing
attached?)\n");
> + break;
> + case 0x1:
> + dev_info(ap->dev, "**** GEN I speed rate negotiated\n");
> + break;
> + case 0x2:
> + dev_info(ap->dev, "**** GEN II speed rate negotiated\n");
> + break;
> + }
> +
libata will negoptiate the speed, why this is needed?
> static void sata_dwc_port_stop(struct ata_port *ap)
> {
> int i;
> - struct sata_dwc_device *hsdev = HSDEV_FROM_AP(ap);
> struct sata_dwc_device_port *hsdevp = HSDEVP_FROM_AP(ap);
>
> dev_dbg(ap->dev, "%s: ap->id = %d\n", __func__, ap->print_id);
>
> - if (hsdevp&& hsdev) {
> - /* deallocate LLI table */
> + if (hsdevp) {
> + /* De-allocate LLI table */
> for (i = 0; i< SATA_DWC_QCMD_MAX; i++) {
> dma_free_coherent(ap->host->dev,
> - SATA_DWC_DMAC_LLI_TBL_SZ,
> - hsdevp->llit[i],
hsdevp->llit_dma[i]);
> + SATA_DWC_DMAC_LLI_TBL_SZ,
> + hsdevp->llit[i], hsdevp->llit_dma[i]);
It was properly indented before.
> @@ -1381,15 +1475,76 @@ static void sata_dwc_port_stop(struct ata_port
*ap)
> }
>
> /*
> - * Function : sata_dwc_exec_command_by_tag
> - * arguments : ata_port *ap, ata_taskfile *tf, u8 tag, u32 cmd_issued
> - * Return value : None
> - * This function keeps track of individual command tag ids and calls
> - * ata_exec_command in libata
> + * As our SATA is master only, no dev_select function needed.
> + * This just overwrite the ata_sff_dev_select() function in
> + * libata-sff
> + */
> +void sata_dwc_dev_select(struct ata_port *ap, unsigned int device)
> +{
> + ndelay(100);
Why?
> +}
> +
> +/**
> + * Filter ATAPI cmds which are unsuitable for DMA.
> + *
> + * The bmdma engines cannot handle speculative data sizes
> + * (bytecount under/over flow). So only allow DMA for
> + * data transfer commands with known data sizes.
> + */
> +static int sata_dwc_check_atapi_dma(struct ata_queued_cmd *qc)
> +{
> + struct scsi_cmnd *scmd = qc->scsicmd;
> + int pio = 1; /* ATAPI DMA disabled by default */
> + unsigned int lba;
> +
> + if (scmd) {
> + switch (scmd->cmnd[0]) {
> + case WRITE_6:
> + case WRITE_10:
> + case WRITE_12:
> + case READ_6:
> + case READ_10:
> + case READ_12:
> + pio = 0; /* DMA is safe */
> + break;
> + }
> +
> + /* Command WRITE_10 with LBA between -45150 (FFFF4FA2)
> + * and -1 (FFFFFFFF) shall use PIO mode */
> + if (scmd->cmnd[0] == WRITE_10) {
> + lba = (scmd->cmnd[2]<< 24) |
> + (scmd->cmnd[3]<< 16) |
> + (scmd->cmnd[4]<< 8) |
> + scmd->cmnd[5];
> + if (lba>= 0xFFFF4FA2)
> + pio = 1;
> + }
> + /*
> + * WORK AROUND: Fix DMA issue when blank CD/DVD disc
> + * in the drive and user use the 'fdisk -l' command.
> + * No DMA data returned so we can not complete the QC.
> + */
> + if (scmd->cmnd[0] == READ_10) {
> + lba = (scmd->cmnd[2]<< 24) |
> + (scmd->cmnd[3]<< 16) |
> + (scmd->cmnd[4]<< 8) |
> + scmd->cmnd[5];
> + if (lba< 0x20)
> + pio = 1;
> + }
> + }
> + dev_dbg(qc->ap->dev, "%s - using %s mode for command
cmd=0x%02x\n", \
> + __func__, (pio ? "PIO" : "DMA"), scmd->cmnd[0]);
> + return pio;
> +}
ATAPI support is a different matter then 2-port support. Needs to be
in a
separate patch.
> @@ -1437,42 +1588,54 @@ static void sata_dwc_bmdma_start_by_tag(struct
ata_queued_cmd *qc, u8 tag)
[...]
> dev_dbg(ap->dev, "%s qc=%p tag: %x cmd: 0x%02x dma_dir: %s "
> "start_dma? %x\n", __func__, qc, tag, qc->tf.command,
> get_dma_dir_descript(qc->dma_dir), start_dma);
> - sata_dwc_tf_dump(&(qc->tf));
> + sata_dwc_tf_dump(hsdev->dev, &(qc->tf));
() with & not necessary.
>
> + /* Enable to start DMA transfer */
> if (start_dma) {
> - reg = core_scr_read(SCR_ERROR);
> - if (reg& SATA_DWC_SERROR_ERR_BITS) {
> + reg = core_scr_read(ap, SCR_ERROR);
> + if (unlikely(reg& SATA_DWC_SERROR_ERR_BITS)) {
> dev_err(ap->dev, "%s: ****** SError=0x%08x
******\n",
> __func__, reg);
libata will print SError register...
> }
>
> - if (dir == DMA_TO_DEVICE)
> + if (dir == DMA_TO_DEVICE) {
> out_le32(&hsdev->sata_dwc_regs->dmacr,
> SATA_DWC_DMACR_TXCHEN);
> - else
> + } else {
> out_le32(&hsdev->sata_dwc_regs->dmacr,
> SATA_DWC_DMACR_RXCHEN);
> + }
>
> /* Enable AHB DMA transfer on the specified channel */
> - dma_dwc_xfer_start(dma_chan);
> + dwc_dma_xfer_start(dma_chan);
> + hsdevp->sata_dwc_sactive_queued&= ~qcmd_tag_to_mask(tag);
> }
> }
> @@ -1490,34 +1653,98 @@ static void sata_dwc_bmdma_start(struct
ata_queued_cmd *qc)
> sata_dwc_bmdma_start_by_tag(qc, tag);
> }
>
> +static u8 sata_dwc_dma_status(struct ata_port *ap)
> +{
> + u32 status = 0;
> + u32 tfr_reg, err_reg;
> + struct sata_dwc_device *hsdev = HSDEV_FROM_AP(ap);
> +
> + /* Check DMA register for status */
> + tfr_reg = in_le32(&(sata_dma_regs->interrupt_status.tfr.low));
> + err_reg = in_le32(&(sata_dma_regs->interrupt_status.error.low));
> +
> + if (unlikely(err_reg& DMA_CHANNEL(hsdev->dma_channel)))
> + status = ATA_DMA_ERR | ATA_DMA_INTR;
Error bit in BMIDE (SFF-8038i) specification doesn't cause interrupt.
> + else if (tfr_reg& DMA_CHANNEL(hsdev->dma_channel))
> + status = ATA_DMA_INTR;
> + return status;
> +}
> +
[...]
> +
> +int sata_dwc_qc_defer(struct ata_queued_cmd *qc)
> +{
> + struct ata_port *ap = qc->ap;
> + struct sata_dwc_device_port *hsdevp = HSDEVP_FROM_AP(ap);
> + u8 status;
> + int ret;
> +
> + dev_dbg(qc->ap->dev, "%s -\n", __func__);
> + ret = ata_std_qc_defer(qc);
> + if (ret) {
> + printk(KERN_DEBUG "STD Defer %s cmd %s tag=%d\n",
> + (ret == ATA_DEFER_LINK) ? "LINK" : "PORT",
> + ata_get_cmd_descript(qc->tf.command), qc->tag);
> + return ret;
> + }
> +
> + /* Check the SATA host for busy status */
> + if (ata_is_ncq(qc->tf.protocol)) {
> + status = ap->ops->sff_check_altstatus(ap);
> + if (status& ATA_BUSY) {
> + dev_dbg(ap->dev,
> + "Defer PORT cmd %s tag=%d as host is
busy\n",
> + ata_get_cmd_descript(qc->tf.command),
qc->tag);
> + return ATA_DEFER_PORT;/*HOST BUSY*/
> + }
> +
> + /* This will prevent collision error */
> + if (hsdevp->sata_dwc_sactive_issued) {
> + dev_dbg(ap->dev, "Defer PORT cmd %s with tag %d "
\
> + "because another dma xfer is
outstanding\n",
> + ata_get_cmd_descript(qc->tf.command),
qc->tag);
What kind of NCQ is this if you can't start another comamnd when some
are
active already?!
> +
> + return ATA_DEFER_PORT;/*DEVICE&HOST BUSY*/
> + }
> +
> + }
> +
> + return 0;
> +}
> +void sata_dwc_exec_command(struct ata_port *ap, const struct
ata_taskfile *tf)
> +{
> + iowrite8(tf->command, ap->ioaddr.command_addr);
> + /* If we have an mmio device with no ctl and no altstatus
> + * method, this will fail. No such devices are known to exist.
> + */
> + if (ap->ioaddr.altstatus_addr)
Isn't it always set?
> + ioread8(ap->ioaddr.altstatus_addr);
> +
> + ndelay(400);
> }
Why duplicate the standard sff_exec_command() method at all?
> @@ -1525,6 +1752,8 @@ static unsigned int sata_dwc_qc_issue(struct
ata_queued_cmd *qc)
> u32 sactive;
> u8 tag = qc->tag;
> struct ata_port *ap = qc->ap;
> + struct sata_dwc_device_port *hsdevp = HSDEVP_FROM_AP(qc->ap);
> + u8 status;
>
> #ifdef DEBUG_NCQ
> if (qc->tag> 0 || ap->link.sactive> 1)
> @@ -1541,50 +1770,148 @@ static unsigned int sata_dwc_qc_issue(struct
ata_queued_cmd *qc)
> sata_dwc_qc_prep_by_tag(qc, tag);
>
> if (ata_is_ncq(qc->tf.protocol)) {
> - sactive = core_scr_read(SCR_ACTIVE);
> + status = ap->ops->sff_check_altstatus(ap);
> + if (status& ATA_BUSY) {
> + /* Ignore the QC when device is BUSY */
> + sactive = core_scr_read(qc->ap, SCR_ACTIVE);
> + dev_info(ap->dev, "Ignore current QC as device
BUSY"
> + "tag=%d, sactive=0x%08x)\n", qc->tag,
sactive);
> + return AC_ERR_SYSTEM;
> + }
> +
> + if (hsdevp->sata_dwc_sactive_issued)
> + return AC_ERR_SYSTEM;
Very strange NCQ... was there a point in implementing it at all?
> +
> + sactive = core_scr_read(qc->ap, SCR_ACTIVE);
> sactive |= (0x00000001<< tag);
> - core_scr_write(SCR_ACTIVE, sactive);
> + qc->dev->link->sactive |= (0x00000001<< tag);
() not needed.
> + core_scr_write(qc->ap, SCR_ACTIVE, sactive);
>
> dev_dbg(qc->ap->dev, "%s: tag=%d ap->link.sactive = 0x%08x
"
> - "sactive=0x%08x\n", __func__, tag,
qc->ap->link.sactive,
> + "sactive=0x%x\n", __func__, tag,
qc->ap->link.sactive,
Why?
> sactive);
>
> ap->ops->sff_tf_load(ap,&qc->tf);
> - sata_dwc_exec_command_by_tag(ap,&qc->tf, qc->tag,
> - SATA_DWC_CMD_ISSUED_PEND);
> + sata_dwc_exec_command_by_tag(ap,&qc->tf, qc->tag);
> } else {
> - ata_sff_qc_issue(qc);
> + ap->link.active_tag = qc->tag;
> + /* Pass QC to libata-sff to process */
> + ata_bmdma_qc_issue(qc);
This don't have to do with the claimed subject of the patch.
> }
> return 0;
> }
>
> /*
> - * Function : sata_dwc_qc_prep
> - * arguments : ata_queued_cmd *qc
> - * Return value : None
> - * qc_prep for a particular queued command
> + * Prepare for a particular queued command
> */
>
> static void sata_dwc_qc_prep(struct ata_queued_cmd *qc)
> {
> - if ((qc->dma_dir == DMA_NONE) || (qc->tf.protocol ==
ATA_PROT_PIO))
> + if ((qc->dma_dir == DMA_NONE) || (qc->tf.protocol == ATA_PROT_PIO)
> + || (qc->tf.protocol == ATAPI_PROT_PIO))
Adding support for ATAPI is another matter than adding support for two
ports. Should be in a patch of its own.
> return;
>
> #ifdef DEBUG_NCQ
> if (qc->tag> 0)
> dev_info(qc->ap->dev, "%s: qc->tag=%d
ap->active_tag=0x%08x\n",
> __func__, qc->tag, qc->ap->link.active_tag);
> -
> - return ;
> #endif
> }
>
> +/*
> + * Get the QC currently used for transferring data
> + */
> +static struct ata_queued_cmd *sata_dwc_get_active_qc(struct ata_port
*ap)
> +{
> + struct ata_queued_cmd *qc;
> +
> + qc = ata_qc_from_tag(ap, ap->link.active_tag);
> + if (qc&& !(qc->tf.flags& ATA_TFLAG_POLLING))
> + return qc;
> + return NULL;
> +}
> +
> +/*
> + * dwc_lost_interrupt - check and process if interrupt is lost.
> + * @ap: ATA port
> + *
> + * Process the command when it is timeout.
> + * Check to see if interrupt is lost. If yes, complete the qc.
> + */
> +static void sata_dwc_lost_interrupt(struct ata_port *ap)
> +{
> + u8 status;
> + struct sata_dwc_device *hsdev = HSDEV_FROM_AP(ap);
> + struct ata_queued_cmd *qc;
> +
> + dev_dbg(ap->dev, "%s -\n", __func__);
> + /* Only one outstanding command per SFF channel */
> + qc = sata_dwc_get_active_qc(ap);
> + /* We cannot lose an interrupt on a non-existent or polled command
*/
> + if (!qc)
> + return;
> +
> + /* See if the controller thinks it is still busy - if so the
command
> + isn't a lost IRQ but is still in progress */
> + status = ap->ops->sff_check_altstatus(ap);
> + if (status& ATA_BUSY) {
> + ata_port_printk(ap, KERN_INFO, "%s - ATA_BUSY\n",
__func__);
> + return;
> + }
> +
> + /* There was a command running, we are no longer busy and we have
> + no interrupt. */
> + ata_link_printk(qc->dev->link, KERN_WARNING,
> + "lost interrupt (Status 0x%x)\n", status);
> +
> + if (sata_dwc_dma_chk_en(hsdev->dma_channel)) {
> + /* When DMA does transfer does not complete,
> + see if DMA fails */
> + qc->err_mask |= AC_ERR_DEV;
> + ap->hsm_task_state = HSM_ST_ERR;
> + sata_dwc_dma_terminate(ap, hsdev->dma_channel);
> + }
> + sata_dwc_qc_complete(ap, qc, 1);
> +}
> +
> +
> static void sata_dwc_error_handler(struct ata_port *ap)
> {
> - ap->link.flags |= ATA_LFLAG_NO_HRST;
> + bool thaw = false;
> + struct ata_queued_cmd *qc;
> + u8 status = ap->ops->bmdma_status(ap);
> +
> + qc = sata_dwc_get_active_qc(ap);
> + /* In case of DMA timeout, process it. */
> + if (qc && ata_is_dma(qc->tf.protocol)) {
> + if ((qc->err_mask == AC_ERR_TIMEOUT)
> + && (status & ATA_DMA_ERR)) {
> + qc->err_mask = AC_ERR_HOST_BUS;
> + thaw = true;
> + }
> +
> + if (thaw) {
> + ap->ops->sff_check_status(ap);
> + if (ap->ops->sff_irq_clear)
> + ap->ops->sff_irq_clear(ap);
> + }
> + }
> + if (thaw)
> + ata_eh_thaw_port(ap);
> +
> ata_sff_error_handler(ap);
> }
>
I don't think this goes well with adding support for 2 ports. Seems to
be
material for another patch.
[...]
> +u8 sata_dwc_check_status(struct ata_port *ap)
> +{
> + return ioread8(ap->ioaddr.status_addr);
> +}
This method is equivalent to ata_sff_check_status(), why redefine it?
> +
> +u8 sata_dwc_check_altstatus(struct ata_port *ap)
> +{
> + return ioread8(ap->ioaddr.altstatus_addr);
> +}
This method is optional. The above is equivalent to the default
implementnation, why redefine it?
> @@ -1604,7 +1931,10 @@ static struct ata_port_operations sata_dwc_ops =
{
> .inherits =&ata_sff_port_ops,
>
> .error_handler = sata_dwc_error_handler,
> + .softreset = sata_dwc_softreset,
> + .hardreset = sata_dwc_hardreset,
>
> + .qc_defer = sata_dwc_qc_defer,
> .qc_prep = sata_dwc_qc_prep,
> .qc_issue = sata_dwc_qc_issue,
>
> @@ -1614,8 +1944,17 @@ static struct ata_port_operations sata_dwc_ops =
{
> .port_start = sata_dwc_port_start,
> .port_stop = sata_dwc_port_stop,
>
> + .check_atapi_dma = sata_dwc_check_atapi_dma,
> .bmdma_setup = sata_dwc_bmdma_setup,
> .bmdma_start = sata_dwc_bmdma_start,
> + .bmdma_status = sata_dwc_dma_status,
> +
> + .sff_dev_select = sata_dwc_dev_select,
> + .sff_check_status = sata_dwc_check_status,
> + .sff_check_altstatus = sata_dwc_check_altstatus,
> + .sff_exec_command = sata_dwc_exec_command,
> +
> + .lost_interrupt = sata_dwc_lost_interrupt,
> };
>
> static const struct ata_port_info sata_dwc_port_info[] = {
> @@ -1639,21 +1978,49 @@ static int sata_dwc_probe(struct platform_device
*ofdev)
> struct ata_port_info pi = sata_dwc_port_info[0];
> const struct ata_port_info *ppi[] = {&pi, NULL };
>
Why empty line here?
> + const unsigned int *dma_chan;
> +
> /* Allocate DWC SATA device */
> - hsdev = kzalloc(sizeof(*hsdev), GFP_KERNEL);
> + hsdev = kmalloc(sizeof(*hsdev), GFP_KERNEL);
Why change kzalloc() to kmalloc() if you do memset() later?
> if (hsdev == NULL) {
> dev_err(&ofdev->dev, "kmalloc failed for hsdev\n");
> err = -ENOMEM;
> - goto error;
> + goto error_out_5;
> }
> + memset(hsdev, 0, sizeof(*hsdev));
>
[...]
> + /* Identify SATA controller index from the cell-index property */
Comment don't match the code?
> + dma_chan = of_get_property(ofdev->dev.of_node, "dma-channel",
NULL);
> + if (dma_chan) {
> + dev_notice(&ofdev->dev, "Getting DMA channel %d\n",
*dma_chan);
> + hsdev->dma_channel = *dma_chan;
> + } else {
> + hsdev->dma_channel = 0;
> + }
> +
[...]
> @@ -1777,7 +2159,18 @@ static struct platform_driver sata_dwc_driver = {
> .remove = sata_dwc_remove,
> };
>
> -module_platform_driver(sata_dwc_driver);
> +static int __init sata_dwc_init(void)
> +{
> + return platform_driver_register(&sata_dwc_driver);
> +}
> +
> +static void __exit sata_dwc_exit(void)
> +{
> + platform_driver_unregister(&sata_dwc_driver);
> +}
> +
> +module_init(sata_dwc_init);
> +module_exit(sata_dwc_exit);
Why you changed this from module_platfrom_driver()?
MBR, Sergei
CONFIDENTIALITY NOTICE: This e-mail message, including any attachments,
is for the sole use of the intended recipient(s) and contains information
that is confidential and proprietary to AppliedMicro Corporation or its subsidiaries.
It is to be used solely for the purpose of furthering the parties' business relationship.
All unauthorized review, use, disclosure or distribution is prohibited.
If you are not the intended recipient, please contact the sender by reply e-mail
and destroy all copies of the original message.
^ permalink raw reply
* Re: [PATCH 3/5] powerpc: dts: klondike: Add UART nodes
From: Tanmay Inamdar @ 2012-04-04 5:39 UTC (permalink / raw)
To: Josh Boyer; +Cc: devicetree-discuss, linuxppc-dev, linux-kernel
In-Reply-To: <CA+5PVA4_PJ2T7LGM_N_WemQKY2u6bLg0QFk4wPpd=FGESx7eiQ@mail.gmail.com>
On Wed, Apr 4, 2012 at 6:19 AM, Josh Boyer <jwboyer@gmail.com> wrote:
> On Tue, Apr 3, 2012 at 11:57 AM, Grant Likely <grant.likely@secretlab.ca>=
wrote:
>> On Mon, =A02 Apr 2012 12:09:05 +0530, Tanmay Inamdar <tinamdar@apm.com> =
wrote:
>>> Adding UART nodes in Klondike device tree file.
>>>
>>> Signed-off-by: Tanmay Inamdar <tinamdar@apm.com>
>>> ---
>>> :100644 100644 4ff2852... d5bf2e1... M =A0 =A0 =A0 =A0arch/powerpc/boot=
/dts/klondike.dts
>>> =A0arch/powerpc/boot/dts/klondike.dts | =A0 24 ++++++++++++++++++++++++
>>> =A01 files changed, 24 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/powerpc/boot/dts/klondike.dts b/arch/powerpc/boot/dts=
/klondike.dts
>>> index 4ff2852..d5bf2e1 100644
>>> --- a/arch/powerpc/boot/dts/klondike.dts
>>> +++ b/arch/powerpc/boot/dts/klondike.dts
>>> @@ -222,6 +222,30 @@
>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 #size-cells=
=3D <1>;
>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ranges;
>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 clock-frequ=
ency =3D <0>;
>>> +
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 UART0: serial=
@50001000 {
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0device_type =3D "serial";
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0compatible =3D "ns16550";
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0reg =3D <0x50001000 0x00000100>;
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0virtual-reg =3D <0x50001000>;
>>
>> Why do you need virtual-reg? =A0The kernel should handle all iomapping
>> properly without this.
>
> Right. =A0It's been used for platforms that actually use the
> zImage/treeImage wrapper for printf support, but this uses u-boot as
> far as I know. =A0It shouldn't need it.
>
Agreed. 'virtual-reg' is not required here. I will remove it in next
version of patch.
> josh
CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, =
is for the sole use of the intended recipient(s) and contains information=
=A0
that is confidential and proprietary to AppliedMicro Corporation or its s=
ubsidiaries. =
It is to be used solely for the purpose of furthering the parties' busine=
ss relationship. =
All unauthorized review, use, disclosure or distribution is prohibited. =
If you are not the intended recipient, please contact the sender by reply=
e-mail =
and destroy all copies of the original message.
=0D
^ permalink raw reply
* [PATCH] powerpc: Option FB_FSL_DIU is not really optional for mpc512x
From: Paul Gortmaker @ 2012-04-04 3:15 UTC (permalink / raw)
To: agust; +Cc: paulus, linuxppc-dev, Paul Gortmaker
In powerpc randconfig builds, this keeps showing up:
CC arch/powerpc/platforms/512x/mpc512x_shared.o
arch/powerpc/platforms/512x/mpc512x_shared.c:70:9: warning: 'enum fsl_diu_monitor_port' declared inside parameter list
arch/powerpc/platforms/512x/mpc512x_shared.c:70:9: warning: its scope is only this definition or declaration, which is probably not what you want
arch/powerpc/platforms/512x/mpc512x_shared.c:69:56: error: parameter 1 ('port') has incomplete type
arch/powerpc/platforms/512x/mpc512x_shared.c:69:5: warning: function declaration isn't a prototype
arch/powerpc/platforms/512x/mpc512x_shared.c:84:9: warning: 'enum fsl_diu_monitor_port' declared inside parameter list
arch/powerpc/platforms/512x/mpc512x_shared.c:83:56: error: parameter 1 ('port') has incomplete type
arch/powerpc/platforms/512x/mpc512x_shared.c:83:6: warning: function declaration isn't a prototype
arch/powerpc/platforms/512x/mpc512x_shared.c:88:36: warning: 'enum fsl_diu_monitor_port' declared inside parameter list
arch/powerpc/platforms/512x/mpc512x_shared.c:88:57: error: parameter 1 ('port') has incomplete type
arch/powerpc/platforms/512x/mpc512x_shared.c:88:6: warning: function declaration isn't a prototype
arch/powerpc/platforms/512x/mpc512x_shared.c:187:54: error: parameter 1 ('port') has incomplete type
arch/powerpc/platforms/512x/mpc512x_shared.c:187:1: error: return type is an incomplete type
arch/powerpc/platforms/512x/mpc512x_shared.c:187:1: warning: function declaration isn't a prototype
arch/powerpc/platforms/512x/mpc512x_shared.c: In function 'mpc512x_valid_monitor_port':
arch/powerpc/platforms/512x/mpc512x_shared.c:189:9: error: 'FSL_DIU_PORT_DVI' undeclared (first use in this function)
arch/powerpc/platforms/512x/mpc512x_shared.c:189:9: note: each undeclared identifier is reported only once for each function it appears in
arch/powerpc/platforms/512x/mpc512x_shared.c:189:2: warning: 'return' with a value, in function returning void
make[2]: *** [arch/powerpc/platforms/512x/mpc512x_shared.o] Error 1
The reason is that mpc512x_shared.c has a couple token #ifdef
on FB_FSL_DIU/FB_FSL_DIU_MODULE, but they don't come close to
masking all the DIU dependencies, as the above fail shows.
Rather than sprinkle more pointless #ifdef in this file, just
remove the existing two, and make FB_FSL_DIU part of the
dependency. The mpc512x_defconfig already has the line
"CONFIG_FB_FSL_DIU=y" so this change should be zero impact
on real world configs.
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
diff --git a/arch/powerpc/platforms/512x/Kconfig b/arch/powerpc/platforms/512x/Kconfig
index c169998..b62508b 100644
--- a/arch/powerpc/platforms/512x/Kconfig
+++ b/arch/powerpc/platforms/512x/Kconfig
@@ -2,6 +2,7 @@ config PPC_MPC512x
bool "512x-based boards"
depends on 6xx
select FSL_SOC
+ select FB_FSL_DIU
select IPIC
select PPC_CLOCK
select PPC_PCI_CHOICE
diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerpc/platforms/512x/mpc512x_shared.c
index cfe958e..1650e09 100644
--- a/arch/powerpc/platforms/512x/mpc512x_shared.c
+++ b/arch/powerpc/platforms/512x/mpc512x_shared.c
@@ -191,8 +191,6 @@ mpc512x_valid_monitor_port(enum fsl_diu_monitor_port port)
static struct fsl_diu_shared_fb __attribute__ ((__aligned__(8))) diu_shared_fb;
-#if defined(CONFIG_FB_FSL_DIU) || \
- defined(CONFIG_FB_FSL_DIU_MODULE)
static inline void mpc512x_free_bootmem(struct page *page)
{
__ClearPageReserved(page);
@@ -220,7 +218,6 @@ void mpc512x_release_bootmem(void)
}
diu_ops.release_bootmem = NULL;
}
-#endif
/*
* Check if DIU was pre-initialized. If so, perform steps
@@ -323,15 +320,12 @@ void __init mpc512x_setup_diu(void)
}
}
-#if defined(CONFIG_FB_FSL_DIU) || \
- defined(CONFIG_FB_FSL_DIU_MODULE)
diu_ops.get_pixel_format = mpc512x_get_pixel_format;
diu_ops.set_gamma_table = mpc512x_set_gamma_table;
diu_ops.set_monitor_port = mpc512x_set_monitor_port;
diu_ops.set_pixel_clock = mpc512x_set_pixel_clock;
diu_ops.valid_monitor_port = mpc512x_valid_monitor_port;
diu_ops.release_bootmem = mpc512x_release_bootmem;
-#endif
}
void __init mpc512x_init_IRQ(void)
--
1.7.9.1
^ permalink raw reply related
* Need Xserve G5 (or power supply for it)
From: Benjamin Herrenschmidt @ 2012-04-04 1:37 UTC (permalink / raw)
To: linuxppc-dev
Hi folks !
My good old Xserve is dead... it looks like the PSU thought it's hard to
tell...
I'm right in the middle of reworking the thermal control driver for
those beasts (PowerMac7,2 PowerMac7,3 and RackMac3,1) so I will need to
test on one of these.
Anybody around with one of them who could help me with the testing ?
(Somebody with a spare PSU willing to ship it to .au would be even
better).
Thanks !
Ben.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox