* [Patch 0/5] PPC64-HWBKPT: Hardware Breakpoint interfaces - ver XXIII
From: K.Prasad @ 2010-06-09 10:24 UTC (permalink / raw)
To: linuxppc-dev@ozlabs.org, Paul Mackerras
Cc: Michael Neuling, Benjamin Herrenschmidt, shaggy,
Frederic Weisbecker, David Gibson, Alan Stern, Roland McGrath
Hi All,
Please find a new version of the patchset that implement hardware
breakpoint interfaces for the PowerPC BookIII S processor. The changes are
few and as described below.
Changelog - ver XXIII
--------------------
(Version XXII: 20100528063924.GA8679@in.ibm.com)
- Detection of extraneous breakpoint exceptions is now done using a boolean flag
in 'struct arch_hw_breakpoint'.
- A dangling put_cpu() (remnant from previous patch versions) in
arch_unregister_hw_breakpoint() is now removed.
Kindly let me know your comments.
Thanks,
K.Prasad
Changelog - ver XXII
--------------------
(Version XXI: linuxppc-dev ref:20100525091314.GA29003@in.ibm.com)
- Extraneous breakpoint exceptions are now properly handled; causative
instruction will be single-stepped and debug register values restored.
- Restoration of breakpoints during signal handling through thread_change_pc()
had defects which are now fixed.
- Breakpoints are flushed through flush_ptrace_hw_breakpoint() call in both
flush_thread() and prepare_to_copy() functions. 'ptrace_bps[]' and
'last_hit_ubp' members are now promptly cleaned-up.
- Single-step exception is now conditionally emulated upon hitting
alignment_exception.
- Rebased to commit 31f46717997a83bdf6db0dd04810c0a329eb3148 of linux-2.6 tree.
Changelog - ver XXI
--------------------
(Version XX: linuxppc-dev ref:20100524103136.GA8131@in.ibm.com)
- Decision to emulate an instruction is now based on whether the causative
instruction is in user_mode() or not.
- Breakpoints don't have to be cleared during sigreturn. A 'double-hit' on
hw_breakpoint_handler() is harmless for non-ptrace instructions.
- Minor changes to aid code brevity.
Changelog - ver XX
--------------------
(Version XIX: linuxppc-dev ref: 20100524040137.GA20873@in.ibm.com)
- Non task-bound breakpoints will only be emulated. Breakpoint will be
unregistered with a warning if emulation fails.
Changelog - ver XIX
--------------------
(Version XVIII: linuxppc-dev ref: 20100512033055.GA6384@in.ibm.com)
- Increased coverage of breakpoints during concurrent alignment_exception
and signal handling (which previously had 'blind-spots').
- Support for kernel-thread breakpoints and kernel-space breakpoints inside the
context of a user-space process.
- Patches re-based to commit f4b87dee923342505e1ddba8d34ce9de33e75050, thereby
necessitating minor changes to arch_validate_hwbkpt_settings().
Changelog - ver XVIII
--------------------
(Version XVII: linuxppc-dev ref: 20100414034340.GA6571@in.ibm.com)
- Slight code restructuring for brevity and coding-style corrections.
- emulate_single_step() now notifies DIE_SSTEP to registered handlers;
causes single_step_dabr_instruction() to be invoked after alignment_exception.
- hw-breakpoint restoration variables are cleaned-up before unregistration
through arch_unregister_hw_breakpoint().
- SIGTRAP is no longer generated for non-ptrace user-space breakpoints.
Changelog - ver XVII
--------------------
(Version XVI: linuxppc-dev ref: 20100330095809.GA14403@in.ibm.com)
- CONFIG_HAVE_HW_BREAKPOINT is now used to define the scope of the new code
(in lieu of CONFIG_PPC_BOOK3S_64).
- CONFIG_HAVE_HW_BREAKPOINT is now dependant upon CONFIG_PERF_EVENTS and
CONFIG_PPC_BOOK3S_64 (to overcome build failures under certain configs).
- Included a target in arch/powerpc/lib/Makefile to build sstep.o when
HAVE_HW_BREAKPOINT.
- Added a dummy definition for hw_breakpoint_disable() when !HAVE_HW_BREAKPOINT.
- Tested builds under defconfigs for ppc64, cell and g5 (found no patch induced
failures).
Changelog - ver XVI
--------------------
(Version XV: linuxppc-dev ref: 20100323140639.GA21836@in.ibm.com)
- Used a new config option CONFIG_PPC_BOOK3S_64 (in lieu of
CONFIG_PPC64/CPU_FTR_HAS_DABR) to limit the scope of the new code.
- Disabled breakpoints before kexec of the machine using hw_breakpoint_disable().
- Minor optimisation in exception-64s.S to check for data breakpoint exceptions
in DSISR finally (after check for other causes) + changes in code comments and
representation of DSISR_DABRMATCH constant.
- Rebased to commit ae6be51ed01d6c4aaf249a207b4434bc7785853b of linux-2.6.
Changelog - ver XV
--------------------
(Version XIV: linuxppc-dev ref: 20100308181232.GA3406@in.ibm.com)
- Additional patch to disable interrupts during data breakpoint exception
handling.
- Moved HBP_NUM definition to cputable.h under a new CPU_FTR definition
(CPU_FTR_HAS_DABR).
- Filtering of extraneous exceptions (due to accesses outside symbol length) is
by-passed for ptrace requests.
- Removed flush_ptrace_hw_breakpoint() from __switch_to() due to incorrect
coding placement.
- Changes to code comments as per community reviews for previous version.
- Minor coding-style changes in hw_breakpoint.c as per review comments.
- Re-based to commit ae6be51ed01d6c4aaf249a207b4434bc7785853b of linux-2.6
Changelog - ver XIV
--------------------
(Version XIII: linuxppc-dev ref: 20100215055605.GB3670@in.ibm.com)
- Removed the 'name' field from 'struct arch_hw_breakpoint'.
- All callback invocations through bp->overflow_handler() are replaced with
perf_bp_event().
- Removed the check for pre-existing single-stepping mode in
hw_breakpoint_handler() as this check is unreliable while in kernel-space.
Side effect of this change is the non-triggering of hw-breakpoints while
single-stepping kernel through KGDB or Xmon.
- Minor code-cleanups and addition of comments in hw_breakpoint_handler() and
single_step_dabr_instruction().
- Re-based to commit 25cf84cf377c0aae5dbcf937ea89bc7893db5176 of linux-2.6
Changelog - ver XIII
--------------------
(Version XII: linuxppc-dev ref: 20100121084640.GA3252@in.ibm.com)
- Fixed a bug for user-space breakpoints (triggered following the failure of a
breakpoint request).
- Re-based on commit 724e6d3fe8003c3f60bf404bf22e4e331327c596 of linux-2.6
Changelog - ver XII
--------------------
(Version XI: linuxppc-dev ref: 20100119091234.GA9971@in.ibm.com)
- Unset MSR_SE only if kernel was not previously in single-step mode.
- Pre-emption is now enabled before returning from the hw-breakpoint exception
handler.
- Variables to track the source of single-step exception (breakpoint from kernel,
user-space vs single-stepping due to other requests) are added.
- Extraneous hw-breakpoint exceptions (due to memory accesses lying outside
monitored symbol length) is now done for both kernel and user-space
(previously only user-space).
- single_step_dabr_instruction() now returns NOTIFY_DONE if kernel was in
single-step mode even before the hw-breakpoint. This enables other users of
single-step mode to be notified of the exception.
- User-space instructions are not emulated from kernel-space, they are instead
single-stepped.
Changelog - ver XI
------------------
(Version X: linuxppc-dev ref: 20091211160144.GA23156@in.ibm.com)
- Conditionally unset MSR_SE in the single-step handler
- Added comments to explain the duration and need for pre-emption
disable following hw-breakpoint exception.
Changelog - ver X
------------------
- Re-write the PPC64 patches for the new implementation of hw-breakpoints that
uses the perf-layer.
- Rebased to commit 7622fc234190a37d4e9fe3ed944a2b61a63fca03 of -tip.
Changelog - ver IX
-------------------
- Invocation of user-defined callback will be 'trigger-after-execute' (except
for ptrace).
- Creation of a new global per-CPU breakpoint structure to help invocation of
user-defined callback from single-step handler.
(Changes made now)
- Validation before registration will fail only if the address does not match
the kernel symbol's (if specified) resolved address
(through kallsyms_lookup_name()).
- 'symbolsize' value is expected to within the range contained by the symbol's
starting address and the end of a double-word boundary (8 Bytes).
- PPC64's arch-dependant code is now aware of 'cpumask' in 'struct hw_breakpoint'
and can accomodate requests for a subset of CPUs in the system.
- Introduced arch_disable_hw_breakpoint() required for
<enable><disable>_hw_breakpoint() APIs.
Changelog - ver VIII
-------------------
- Reverting changes to allow one-shot breakpoints only for ptrace requests.
- Minor changes in sanity checking in arch_validate_hwbkpt_settings().
- put_cpu_no_resched() is no longer available. Converted to put_cpu().
Changelog - ver VII
-------------------
- Allow the one-shot behaviour for exception handlers to be defined by the user.
A new 'is_one_shot' flag is added to 'struct arch_hw_breakpoint'.
Changelog - ver VI
------------------
The task of identifying 'genuine' breakpoint exceptions from those caused by
'out-of-range' accesses turned out to be more tricky than originally thought.
Some changes to this effect were made in version IV of this patchset, but they
were not sufficient for user-space. Basically the breakpoint address received
through ptrace is always aligned to 8-bytes since ptrace receives an encoded
'data' (consisting of address | translation_enable | bkpt_type), and the size of
the symbol is not known. However for kernel-space addresses, the symbol-size can
be determined using kallsyms_lookup_size_offset() and this is used to check if
DAR (in the exception context) is
'bkpt_address <= DAR <= (bkpt_address + symbol_size)', failing which we conclude
it as a stray exception.
The following changes are made to enable check:
- Addition of a symbolsize field in 'struct arch_hw_breakpoint' field.
- Store the size of the 'watched' kernel symbol into 'symbolsize' field in
arch_store_info(0 routine.
- Verify if the above described condition is true when is_one_shot is FALSE in
hw_breakpoint_handler().
Changelog - ver V
------------------
- Breakpoint requests from ptrace (for user-space) are designed to be one-shot
in PPC64. The patch contains changes to retain this behaviour by returning early
in hw_breakpoint_handler() [without re-initialising DABR] and unregistering the
user-space request in ptrace_triggered(). It is safe to make a
unregister_user_hw_breakpoint() call from the breakpoint exception context
[through ptrace_triggered()] without giving rise to circular locking-dependancy.
This is because there can be no kernel code running on the CPU (which received
the exception) with the same spinlock held.
- Minor change in 'type' member of 'struct arch_hw_breakpoint' from u8 to 'int'.
Changelog - ver IV
------------------
- While DABR register requires double-word (8 bytes) aligned addresses, i.e.
the breakpoint is active over a range of 8 bytes, PPC64 allows byte-level
addressability. This may lead to stray exceptions which have to be ignored in
hw_breakpoint_handler(), when DAR != (Breakpoint request address). However DABR
will be populated with the requested breakpoint address aligned to the previous
double-word address. The code is now modified to store user-requested address
in 'bp->info.address' but update the DABR with a double-word aligned address.
- Please note that the Data Breakpoint facility in Xmon is broken as of 2.6.29
and the same has not been integrated into this facility as described in Ver I.
Changelog - ver III
------------------
- Patches are based on commit 08f16e060bf54bdc34f800ed8b5362cdeda75d8b of -tip
tree.
- The declarations in arch/powerpc/include/asm/hw_breakpoint.h are done only if
CONFIG_PPC64 is defined. This eliminates the need to conditionally include this
header file.
- load_debug_registers() is done in start_secondary() i.e. during CPU
initialisation.
- arch_check_va_<> routines in hw_breakpoint.c are now replaced with a much
simpler is_kernel_addr() check in arch_validate_hwbkpt_settings()
- Return code of hw_breakpoint_handler() when triggered due to Lazy debug
register switching is now changed to NOTIFY_STOP.
- The ptrace code no longer sets the TIF_DEBUG task flag as it is proposed to
be done in register_user_hw_breakpoint() routine.
- hw_breakpoint_handler() is now modified to use hbp_kernel_pos value to
determine if the trigger was a user/kernel space address. The DAR register
value is checked with the address stored in 'struct hw_breakpoint' to avoid
handling of exceptions that belong to kprobe/Xmon.
Changelog - ver II
------------------
- Split the monolithic patch into six logical patches
- Changed the signature of arch_check_va_in_<user><kernel>space functions. They
are now marked static.
- HB_NUM is now called as HBP_NUM (to preserve a consistent short-name
convention)
- Introduced hw_breakpoint_disable() and changes to kexec code to disable
breakpoints before a reboot.
- Minor changes in ptrace code to use macro-defined constants instead of
numbers.
- Introduced a new constant definition INSTRUCTION_LEN in reg.h
^ permalink raw reply
* [PATCH 3/5] Removing dead CONFIG_SMP_750
From: Christoph Egger @ 2010-06-09 10:00 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev,
linux-kernel; +Cc: vamos
In-Reply-To: <cover.1275925219.git.siccegge@cs.fau.de>
CONFIG_SMP_750 doesn't exist in Kconfig, therefore removing all
references for it from the source code.
Signed-off-by: Christoph Egger <siccegge@cs.fau.de>
---
arch/powerpc/mm/tlb_hash32.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/mm/tlb_hash32.c b/arch/powerpc/mm/tlb_hash32.c
index 8aaa8b7..925fecb 100644
--- a/arch/powerpc/mm/tlb_hash32.c
+++ b/arch/powerpc/mm/tlb_hash32.c
@@ -94,11 +94,7 @@ void tlb_flush(struct mmu_gather *tlb)
* the cache operations on the bus. Hence we need to use an IPI
* to get the other CPU(s) to invalidate their TLBs.
*/
-#ifdef CONFIG_SMP_750
-#define FINISH_FLUSH smp_send_tlb_invalidate(0)
-#else
#define FINISH_FLUSH do { } while (0)
-#endif
static void flush_range(struct mm_struct *mm, unsigned long start,
unsigned long end)
--
1.6.3.3
^ permalink raw reply related
* [PATCH 1/5] Removing dead BOOK3E_MMU_TLB_STATS
From: Christoph Egger @ 2010-06-09 9:59 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Kumar Gala,
Thadeu Lima de Souza Cascardo, Huang Weiyi, linuxppc-dev,
linux-kernel
Cc: vamos
In-Reply-To: <cover.1275925219.git.siccegge@cs.fau.de>
BOOK3E_MMU_TLB_STATS doesn't exist in Kconfig, therefore removing all
references for it from the source code.
Signed-off-by: Christoph Egger <siccegge@cs.fau.de>
---
arch/powerpc/include/asm/exception-64e.h | 38 ------------------------------
arch/powerpc/mm/tlb_low_64e.S | 9 -------
2 files changed, 0 insertions(+), 47 deletions(-)
diff --git a/arch/powerpc/include/asm/exception-64e.h b/arch/powerpc/include/asm/exception-64e.h
index 6d53f31..db74814 100644
--- a/arch/powerpc/include/asm/exception-64e.h
+++ b/arch/powerpc/include/asm/exception-64e.h
@@ -65,14 +65,7 @@
#define EX_TLB_MMUCR0 (12 * 8) /* Level 0 */
#define EX_TLB_MAS1 (12 * 8) /* Level 0 */
#define EX_TLB_MAS2 (13 * 8) /* Level 0 */
-#ifdef CONFIG_BOOK3E_MMU_TLB_STATS
-#define EX_TLB_R8 (14 * 8)
-#define EX_TLB_R9 (15 * 8)
-#define EX_TLB_LR (16 * 8)
-#define EX_TLB_SIZE (17 * 8)
-#else
#define EX_TLB_SIZE (14 * 8)
-#endif
#define START_EXCEPTION(label) \
.globl exc_##label##_book3e; \
@@ -157,36 +150,6 @@ exc_##label##_book3e:
addi r11,r13,PACA_EXTLB; \
TLB_MISS_RESTORE(r11)
-#ifdef CONFIG_BOOK3E_MMU_TLB_STATS
-#define TLB_MISS_PROLOG_STATS \
- mflr r10; \
- std r8,EX_TLB_R8(r12); \
- std r9,EX_TLB_R9(r12); \
- std r10,EX_TLB_LR(r12);
-#define TLB_MISS_RESTORE_STATS \
- ld r16,EX_TLB_LR(r12); \
- ld r9,EX_TLB_R9(r12); \
- ld r8,EX_TLB_R8(r12); \
- mtlr r16;
-#define TLB_MISS_STATS_D(name) \
- addi r9,r13,MMSTAT_DSTATS+name; \
- bl .tlb_stat_inc;
-#define TLB_MISS_STATS_I(name) \
- addi r9,r13,MMSTAT_ISTATS+name; \
- bl .tlb_stat_inc;
-#define TLB_MISS_STATS_X(name) \
- ld r8,PACA_EXTLB+EX_TLB_ESR(r13); \
- cmpdi cr2,r8,-1; \
- beq cr2,61f; \
- addi r9,r13,MMSTAT_DSTATS+name; \
- b 62f; \
-61: addi r9,r13,MMSTAT_ISTATS+name; \
-62: bl .tlb_stat_inc;
-#define TLB_MISS_STATS_SAVE_INFO \
- std r14,EX_TLB_ESR(r12); /* save ESR */ \
-
-
-#else
#define TLB_MISS_PROLOG_STATS
#define TLB_MISS_RESTORE_STATS
#define TLB_MISS_STATS_D(name)
@@ -194,7 +157,6 @@ exc_##label##_book3e:
#define TLB_MISS_STATS_X(name)
#define TLB_MISS_STATS_Y(name)
#define TLB_MISS_STATS_SAVE_INFO
-#endif
#define SET_IVOR(vector_number, vector_offset) \
li r3,vector_offset@l; \
diff --git a/arch/powerpc/mm/tlb_low_64e.S b/arch/powerpc/mm/tlb_low_64e.S
index 8b04c54..4d7d059 100644
--- a/arch/powerpc/mm/tlb_low_64e.S
+++ b/arch/powerpc/mm/tlb_low_64e.S
@@ -758,12 +758,3 @@ tlb_load_linear_fault:
1: TLB_MISS_EPILOG_ERROR_SPECIAL
b exc_instruction_storage_book3e
-
-#ifdef CONFIG_BOOK3E_MMU_TLB_STATS
-.tlb_stat_inc:
-1: ldarx r8,0,r9
- addi r8,r8,1
- stdcx. r8,0,r9
- bne- 1b
- blr
-#endif
--
1.6.3.3
^ permalink raw reply related
* [PATCH 2/5] Removing dead CONFIG_HIGHPTE
From: Christoph Egger @ 2010-06-09 9:59 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Albert Herranz,
Grant Likely, Li Yang, Tejun Heo, linuxppc-dev, linux-kernel
Cc: vamos
In-Reply-To: <cover.1275925219.git.siccegge@cs.fau.de>
CONFIG_HIGHPTE doesn't exist in Kconfig, therefore removing all
references for it from the source code.
Signed-off-by: Christoph Egger <siccegge@cs.fau.de>
---
arch/powerpc/mm/pgtable_32.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
index 9fc02dc..34347b2 100644
--- a/arch/powerpc/mm/pgtable_32.c
+++ b/arch/powerpc/mm/pgtable_32.c
@@ -115,11 +115,7 @@ pgtable_t pte_alloc_one(struct mm_struct *mm, unsigned long address)
{
struct page *ptepage;
-#ifdef CONFIG_HIGHPTE
- gfp_t flags = GFP_KERNEL | __GFP_HIGHMEM | __GFP_REPEAT | __GFP_ZERO;
-#else
gfp_t flags = GFP_KERNEL | __GFP_REPEAT | __GFP_ZERO;
-#endif
ptepage = alloc_pages(flags, 0);
if (!ptepage)
--
1.6.3.3
^ permalink raw reply related
* [PATCH 5/5] Removing dead CONFIG_PPC47x
From: Christoph Egger @ 2010-06-09 10:01 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Stephen Rothwell,
Torez Smith, Dave Kleikamp, Josh Boyer, linuxppc-dev,
linux-kernel
Cc: vamos
In-Reply-To: <cover.1275925219.git.siccegge@cs.fau.de>
CONFIG_PPC47x doesn't exist in Kconfig, therefore removing all
references for it from the source code.
Signed-off-by: Christoph Egger <siccegge@cs.fau.de>
---
arch/powerpc/mm/44x_mmu.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/mm/44x_mmu.c b/arch/powerpc/mm/44x_mmu.c
index d8c6efb..f70da7e 100644
--- a/arch/powerpc/mm/44x_mmu.c
+++ b/arch/powerpc/mm/44x_mmu.c
@@ -76,11 +76,7 @@ static void __init ppc44x_pin_tlb(unsigned int virt, unsigned int phys)
"tlbwe %1,%3,%5\n"
"tlbwe %0,%3,%6\n"
:
-#ifdef CONFIG_PPC47x
- : "r" (PPC47x_TLB2_S_RWX),
-#else
: "r" (PPC44x_TLB_SW | PPC44x_TLB_SR | PPC44x_TLB_SX | PPC44x_TLB_G),
-#endif
"r" (phys),
"r" (virt | PPC44x_TLB_VALID | PPC44x_TLB_256M),
"r" (entry),
--
1.6.3.3
^ permalink raw reply related
* [PATCH 4/5] Removing dead CONFIG_SERIAL_TEXT_DEBUG
From: Christoph Egger @ 2010-06-09 10:00 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev,
linux-kernel; +Cc: vamos
In-Reply-To: <cover.1275925219.git.siccegge@cs.fau.de>
CONFIG_SERIAL_TEXT_DEBUG doesn't exist in Kconfig, therefore removing all
references for it from the source code.
Signed-off-by: Christoph Egger <siccegge@cs.fau.de>
---
arch/powerpc/kernel/head_40x.S | 19 -------------------
1 files changed, 0 insertions(+), 19 deletions(-)
diff --git a/arch/powerpc/kernel/head_40x.S b/arch/powerpc/kernel/head_40x.S
index a90625f..1a10e4d 100644
--- a/arch/powerpc/kernel/head_40x.S
+++ b/arch/powerpc/kernel/head_40x.S
@@ -939,25 +939,6 @@ initial_mmu:
tlbwe r4,r0,TLB_DATA /* Load the data portion of the entry */
tlbwe r3,r0,TLB_TAG /* Load the tag portion of the entry */
-#if defined(CONFIG_SERIAL_TEXT_DEBUG) && defined(SERIAL_DEBUG_IO_BASE)
-
- /* Load a TLB entry for the UART, so that ppc4xx_progress() can use
- * the UARTs nice and early. We use a 4k real==virtual mapping. */
-
- lis r3,SERIAL_DEBUG_IO_BASE@h
- ori r3,r3,SERIAL_DEBUG_IO_BASE@l
- mr r4,r3
- clrrwi r4,r4,12
- ori r4,r4,(TLB_WR|TLB_I|TLB_M|TLB_G)
-
- clrrwi r3,r3,12
- ori r3,r3,(TLB_VALID | TLB_PAGESZ(PAGESZ_4K))
-
- li r0,0 /* TLB slot 0 */
- tlbwe r4,r0,TLB_DATA
- tlbwe r3,r0,TLB_TAG
-#endif /* CONFIG_SERIAL_DEBUG_TEXT && SERIAL_DEBUG_IO_BASE */
-
isync
/* Establish the exception vector base
--
1.6.3.3
^ permalink raw reply related
* Re: [PATCH 00/12] treewide: Remove unnecessary kmalloc casts
From: Pekka Enberg @ 2010-06-09 10:06 UTC (permalink / raw)
To: Milton Miller; +Cc: Joe Perches, Andrew Morton, linuxppc-dev, linux-kernel
In-Reply-To: <1276068001_13533@mail4.comsite.net>
Hi Milton,
On Wed, Jun 9, 2010 at 10:20 AM, Milton Miller <miltonm@bga.com> wrote:
> However, in this case you are removing casts that, while not necessary
> for C, are indeed there for a reason.
>
> Specifically, they are of the form
> =A0 type *p;
> =A0 <code>
> =A0 p =3D (type *)kmalloc(sizeof(type), ...);
>
> For example, from the powerpc patch:
>> goto out;
>> }
>> - tmp_part =3D (struct nvram_partition *)
>> - kmalloc(sizeof(struct nvram_partition), GFP_KERNEL);
>> + tmp_part =3D kmalloc(sizeof(struct nvram_partition), GFP_KERNEL);
>> err =3D -ENOMEM;
>
> The reason they casts are present is to guard against someone changing
> the type of p at the top of the function and not changing the type at
> the kmalloc.
If you're worried about that...
[snip]
> There may have been discussion of doing the above vs
> =A0 p =3D kmalloc(sizeof(*p), ...);
...it's better to use this form. There's actually a mention of this in
"Chapter 14: Allocating memory" of Documentation/CodingStyle. The
guard is not really a guard as someone can still change the "sizeof"
part to something else and the cast from "void *" will silently ignore
it.
Pekka
^ permalink raw reply
* [PATCH 0/5] Removing dead code
From: Christoph Egger @ 2010-06-09 9:58 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Kumar Gala,
Stephen Rothwell, Torez Smith, Dave Kleikamp, Josh Boyer,
Albert Herranz, Grant Likely, Li Yang, Tejun Heo,
Thadeu Lima de Souza Cascardo, Huang Weiyi, linuxppc-dev,
linux-kernel
Cc: vamos
Hi all!
As part of the VAMOS[0] research project at the University of
Erlangen we are looking at multiple integrity errors in linux'
configuration system.
I've been running a check on the arch/powerpc sourcetree
for
config Items not defined in Kconfig and found5 such chases. Sourcecode
blocks depending on these Items are not reachable from a vanilla
kernel -- dead code. I've seen such dead blocks made on purpose
e.g. while integrating new features into the kernel but generally
they're just useless.
Each of the patches in this patchset removes on such dead
config Item, I'd be glad if you consider applying them. I've been
doing deeper analysis of such issues before and can do so again but
I'm not so sure they were fastly usefull. However I've done a
testbuild on ppc with allmodconfig so it should at least build.
Please keep me informed of this patch getting confirmed /
merged so we can keep track of it.
Regards
Christoph Egger
[0] http://vamos1.informatik.uni-erlangen.de/
Christoph Egger (5):
Removing dead BOOK3E_MMU_TLB_STATS
Removing dead CONFIG_HIGHPTE
Removing dead CONFIG_SMP_750
Removing dead CONFIG_SERIAL_TEXT_DEBUG
Removing dead CONFIG_PPC47x
arch/powerpc/include/asm/exception-64e.h | 38 ------------------------------
arch/powerpc/kernel/head_40x.S | 19 ---------------
arch/powerpc/mm/44x_mmu.c | 4 ---
arch/powerpc/mm/pgtable_32.c | 4 ---
arch/powerpc/mm/tlb_hash32.c | 4 ---
arch/powerpc/mm/tlb_low_64e.S | 9 -------
6 files changed, 0 insertions(+), 78 deletions(-)
^ permalink raw reply
* Re: [PATCH 3/3] powerpc: enabled asymmetric SMT scheduling on POWER7
From: Benjamin Herrenschmidt @ 2010-06-09 8:54 UTC (permalink / raw)
To: Michael Neuling; +Cc: Peter Zijlstra, linuxppc-dev, linux-kernel
In-Reply-To: <20100608045702.31FB5CC8C7@localhost.localdomain>
On Tue, 2010-06-08 at 14:57 +1000, Michael Neuling wrote:
> The POWER7 core has dynamic SMT mode switching which is controlled by
> the hypervisor. There are 3 SMT modes:
> SMT1 uses thread 0
> SMT2 uses threads 0 & 1
> SMT4 uses threads 0, 1, 2 & 3
> When in any particular SMT mode, all threads have the same performance
> as each other (ie. at any moment in time, all threads perform the same).
>
> The SMT mode switching works such that when linux has threads 2 & 3 idle
> and 0 & 1 active, it will cede (H_CEDE hypercall) threads 2 and 3 in the
> idle loop and the hypervisor will automatically switch to SMT2 for that
> core (independent of other cores). The opposite is not true, so if
> threads 0 & 1 are idle and 2 & 3 are active, we will stay in SMT4 mode.
>
> Similarly if thread 0 is active and threads 1, 2 & 3 are idle, we'll go
> into SMT1 mode.
>
> If we can get the core into a lower SMT mode (SMT1 is best), the threads
> will perform better (since they share less core resources). Hence when
> we have idle threads, we want them to be the higher ones.
>
> This adds a feature bit for asymmetric packing to powerpc and then
> enables it on POWER7.
>
> Signed-off-by: Michael Neuling <mikey@neuling.org>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>
> ---
>
> arch/powerpc/include/asm/cputable.h | 3 ++-
> arch/powerpc/kernel/process.c | 9 +++++++++
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> Index: linux-2.6-ozlabs/arch/powerpc/include/asm/cputable.h
> ===================================================================
> --- linux-2.6-ozlabs.orig/arch/powerpc/include/asm/cputable.h
> +++ linux-2.6-ozlabs/arch/powerpc/include/asm/cputable.h
> @@ -195,6 +195,7 @@ extern const char *powerpc_base_platform
> #define CPU_FTR_SAO LONG_ASM_CONST(0x0020000000000000)
> #define CPU_FTR_CP_USE_DCBTZ LONG_ASM_CONST(0x0040000000000000)
> #define CPU_FTR_UNALIGNED_LD_STD LONG_ASM_CONST(0x0080000000000000)
> +#define CPU_FTR_ASYM_SMT LONG_ASM_CONST(0x0100000000000000)
>
> #ifndef __ASSEMBLY__
>
> @@ -409,7 +410,7 @@ extern const char *powerpc_base_platform
> CPU_FTR_MMCRA | CPU_FTR_SMT | \
> CPU_FTR_COHERENT_ICACHE | CPU_FTR_LOCKLESS_TLBIE | \
> CPU_FTR_PURR | CPU_FTR_SPURR | CPU_FTR_REAL_LE | \
> - CPU_FTR_DSCR | CPU_FTR_SAO)
> + CPU_FTR_DSCR | CPU_FTR_SAO | CPU_FTR_ASYM_SMT)
> #define CPU_FTRS_CELL (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
> CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
> CPU_FTR_ALTIVEC_COMP | CPU_FTR_MMCRA | CPU_FTR_SMT | \
> Index: linux-2.6-ozlabs/arch/powerpc/kernel/process.c
> ===================================================================
> --- linux-2.6-ozlabs.orig/arch/powerpc/kernel/process.c
> +++ linux-2.6-ozlabs/arch/powerpc/kernel/process.c
> @@ -1265,3 +1265,12 @@ unsigned long randomize_et_dyn(unsigned
>
> return ret;
> }
> +
> +int arch_sd_sibiling_asym_packing(void)
> +{
> + if (cpu_has_feature(CPU_FTR_ASYM_SMT)){
> + printk_once(KERN_INFO "Enabling Asymmetric SMT scheduling\n");
> + return SD_ASYM_PACKING;
> + }
> + return 0;
> +}
^ permalink raw reply
* Re: [PATCH 00/12] treewide: Remove unnecessary kmalloc casts
From: Joe Perches @ 2010-06-09 7:46 UTC (permalink / raw)
To: Milton Miller, Jiri Kosina; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1276068001_13533@mail4.comsite.net>
On Wed, 2010-06-09 at 02:20 -0500, Milton Miller wrote:
> On Mon Jun 07 2010 at around 23:53:18 EST, Joe Perches wrote:
> > Trivial cleanups
> The reason they casts are present is to guard against someone changing
> the type of p at the top of the function and not changing the type at
> the kmalloc.
That'd be true for most all of the kernel mallocs.
> I know some maintainers have already taken some of
> the series the last time you posted; could you please augment the
> CC list.
I did not send to trivial any of kmalloc cast removal patches that
were acked. The patches in this series were not acked, so I think
no CC list needs augmentation.
Original: http://lkml.org/lkml/2010/5/31/471
Newest: http://lkml.org/lkml/2010/6/7/428
cheers, Joe
^ permalink raw reply
* Re: [PATCH 00/12] treewide: Remove unnecessary kmalloc casts
From: Milton Miller @ 2010-06-09 7:20 UTC (permalink / raw)
To: Joe Perches; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1275968594.git.joe@perches.com>
On Mon Jun 07 2010 at around 23:53:18 EST, Joe Perches wrote:
> Trivial cleanups
>
> arch/cris: Remove unnecessary kmalloc casts
> arch/powerpc: Remove unnecessary kmalloc casts
> arch/x86/kernel/tlb_uv.c: Remove unnecessary kmalloc casts
> drivers/gpu/drm: Remove unnecessary kmalloc casts
> drivers/isdn/capi/capidrv.c: Remove unnecessary kmalloc casts
> drivers/media: Remove unnecesary kmalloc casts
> drivers/message/i2o/i20_config.c: Remove unnecessary kmalloc casts
> drivers/parisc/iosapic.c: Remove unnecessary kzalloc cast
> drivers/s390: Remove unnecessary kmalloc casts
> drivers/serial/icom.c: Remove unnecessary kmalloc casts
> drivers/usb/storage/isd200.c: Remove unnecessary kmalloc casts
> fs/ufs/util.c: Remove unnecessary kmalloc casts
>
Joe,
Thanks for doing cleanups.
However, in this case you are removing casts that, while not necessary
for C, are indeed there for a reason.
Specifically, they are of the form
type *p;
<code>
p = (type *)kmalloc(sizeof(type), ...);
For example, from the powerpc patch:
> goto out;
> }
> - tmp_part = (struct nvram_partition *)
> - kmalloc(sizeof(struct nvram_partition), GFP_KERNEL);
> + tmp_part = kmalloc(sizeof(struct nvram_partition), GFP_KERNEL);
> err = -ENOMEM;
The reason they casts are present is to guard against someone changing
the type of p at the top of the function and not changing the type at
the kmalloc.
This was discussed some years ago, possibly around the time kcalloc
got its underflow detection.
Should we create a kmalloc wrapper that enforces this type saftey?
While we have added static analysis tools that can check these things,
and we have slab redzoning options, they tend to be run in batch by
"someone else" or in response to debugging a problem.
There may have been discussion of doing the above vs
p = kmalloc(sizeof(*p), ...);
I don't remember if it was programmer preference or issues when p is
an array type.
I am not subscribed so I don't have the full list that you sent
these to, and I know some maintainers have already taken some of
the series the last time you posted; could you please augment the
CC list.
thanks,
milton
^ permalink raw reply
* Re: [PATCH 0/2] mpc5200 ac97 gpio reset
From: Wolfgang Denk @ 2010-06-09 6:13 UTC (permalink / raw)
To: Jon Smirl; +Cc: Mark Brown, linuxppc-dev, Eric Millbrandt
In-Reply-To: <AANLkTimIs90kR5uqhdBQ02oSd94dn08sOITm51Ylrl4-@mail.gmail.com>
Dear Jon Smirl,
In message <AANLkTimIs90kR5uqhdBQ02oSd94dn08sOITm51Ylrl4-@mail.gmail.com> you wrote:
>
> Would making a change in uboot be a better solution? Eric, can you
> verify if changing uboot also fixes the problem?
To me it seems better if the driver itself does what needs to be done
instead of relying on specific settings that may or may not be done in
U-Boot. Keep in mind that drivers may be loaded as modules, and that
we even see cases where the same port serves multiple purposes by
loading different driver modules (yes, this is not exactly a clever
idea, but hardware designers come up with such solutions).
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
The typical page layout program is nothing more than an electronic
light table for cutting and pasting documents.
^ permalink raw reply
* [PATCH] powerpc: rtas_flash cannot be a module
From: Anton Blanchard @ 2010-06-09 6:01 UTC (permalink / raw)
To: benh; +Cc: linuxppc-dev
When trying to flash a machine via the update_flash command, I received the
following error:
Restarting system.
FLASH: kernel bug...flash list header addr above 4GB
The code in question has a comment that the flash list should be in
the kernel data and therefore under 4GB:
/* NOTE: the "first" block list is a global var with no data
* blocks in the kernel data segment. We do this because
* we want to ensure this block_list addr is under 4GB.
*/
Unfortunately the Kconfig option is marked tristate which means the variable
may not be in the kernel data and could be above 4GB.
Change RTAS_FLASH to a bool. If we are worried about kernel footprint we
could move the problem variables out of the module and export them.
Signed-off-by: Anton Blanchard <anton@samba.org>
---
Index: linux-2.6/arch/powerpc/platforms/Kconfig
===================================================================
--- linux-2.6.orig/arch/powerpc/platforms/Kconfig 2010-06-09 15:44:53.635955260 +1000
+++ linux-2.6/arch/powerpc/platforms/Kconfig 2010-06-09 15:45:00.503453428 +1000
@@ -97,7 +97,7 @@ config RTAS_PROC
default y
config RTAS_FLASH
- tristate "Firmware flash interface"
+ bool "Firmware flash interface"
depends on PPC64 && RTAS_PROC
config MMIO_NVRAM
Index: linux-2.6/arch/powerpc/configs/ppc64_defconfig
===================================================================
--- linux-2.6.orig/arch/powerpc/configs/ppc64_defconfig 2010-06-09 15:46:25.394704486 +1000
+++ linux-2.6/arch/powerpc/configs/ppc64_defconfig 2010-06-09 15:46:37.083454943 +1000
@@ -264,7 +264,7 @@ CONFIG_U3_DART=y
CONFIG_PPC_RTAS=y
CONFIG_RTAS_ERROR_LOGGING=y
CONFIG_RTAS_PROC=y
-CONFIG_RTAS_FLASH=m
+CONFIG_RTAS_FLASH=y
CONFIG_PPC_PMI=m
CONFIG_MMIO_NVRAM=y
CONFIG_MPIC_U3_HT_IRQS=y
Index: linux-2.6/arch/powerpc/configs/pseries_defconfig
===================================================================
--- linux-2.6.orig/arch/powerpc/configs/pseries_defconfig 2010-06-09 15:46:25.364703092 +1000
+++ linux-2.6/arch/powerpc/configs/pseries_defconfig 2010-06-09 15:46:31.723454300 +1000
@@ -213,7 +213,7 @@ CONFIG_PPC_I8259=y
CONFIG_PPC_RTAS=y
CONFIG_RTAS_ERROR_LOGGING=y
CONFIG_RTAS_PROC=y
-CONFIG_RTAS_FLASH=m
+CONFIG_RTAS_FLASH=y
# CONFIG_MMIO_NVRAM is not set
CONFIG_IBMVIO=y
CONFIG_IBMEBUS=y
^ permalink raw reply
* Re: [PATCH 0/2] mpc5200 ac97 gpio reset
From: Jon Smirl @ 2010-06-09 0:24 UTC (permalink / raw)
To: Eric Millbrandt; +Cc: Mark Brown, linuxppc-dev
In-Reply-To: <1276015562-28928-1-git-send-email-emillbrandt@dekaresearch.com>
On Tue, Jun 8, 2010 at 12:46 PM, Eric Millbrandt
<emillbrandt@dekaresearch.com> wrote:
> These patches reimplement the reset fuction in the ac97 to use gpio pins
> instead of using the mpc5200 ac97 reset functionality in the psc. =A0This
> avoids a problem in which attached ac97 devices go into "test" mode appea=
r
> unresponsive.
I'm aware of this problem but I was unable to solve it when I first
wrote the driver. I didn't have access to a scope so I wasn't able to
figure out what was causing the lock up. I worked around it by
looping in reset until the chip did actually reset, but Mark wouldn't
let me submit that code.
A while ago Maximilian Mueth <maximilian.mueth@mp-ndt.de> sent me a
note saying another solution was to configure the PSC into AC97 mode
in uboot. His impression was that uboot was setting PSC into its
default mode. Then Linux booted and set the PSC into AC97 mode. The
transition from default mode into AC97 mode caused glitches on the
AC97 reset pins and triggered test mode.
Would making a change in uboot be a better solution? Eric, can you
verify if changing uboot also fixes the problem?
I'm glad were finally getting to the root cause of the problem.
>
> These patches were tested on a pcm030 baseboard and on custom hardware wi=
th
> a wm9715 audio codec/touchscreen controller.
>
> Eric Millbrandt
>
> ---
>
> Eric Millbrandt (2):
> =A0 =A0powerpc/5200: Export port-config
> =A0 =A0sound/soc: mpc5200_psc_ac97: Use gpio pins for cold reset.
>
> =A0arch/powerpc/boot/dts/lite5200.dts =A0 =A0 =A0 =A0 =A0 | =A0 =A03 +
> =A0arch/powerpc/boot/dts/lite5200b.dts =A0 =A0 =A0 =A0 =A0| =A0 =A03 +
> =A0arch/powerpc/boot/dts/pcm030.dts =A0 =A0 =A0 =A0 =A0 =A0 | =A0 =A03 +
> =A0arch/powerpc/boot/dts/pcm032.dts =A0 =A0 =A0 =A0 =A0 =A0 | =A0 =A03 +
> =A0arch/powerpc/include/asm/mpc52xx.h =A0 =A0 =A0 =A0 =A0 | =A0 =A02 +
> =A0arch/powerpc/platforms/52xx/mpc52xx_common.c | =A0 61 ++++++++++++++++=
+++
> =A0sound/soc/fsl/mpc5200_dma.h =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0| =A0 =
=A05 ++
> =A0sound/soc/fsl/mpc5200_psc_ac97.c =A0 =A0 =A0 =A0 =A0 =A0 | =A0 83 ++++=
++++++++++++++++++++-
> =A08 files changed, 159 insertions(+), 4 deletions(-)
>
> -DISCLAIMER: an automatically appended disclaimer may follow. By posting-
> -to a public e-mail mailing list I hereby grant permission to distribute-
> -and copy this message.-
>
>
> This e-mail and the information, including any attachments, it contains a=
re intended to be a confidential communication only to the person or entity=
to whom it is addressed and may contain information that is privileged. If=
the reader of this message is not the intended recipient, you are hereby n=
otified that any dissemination, distribution or copying of this communicati=
on is strictly prohibited. If you have received this communication in error=
, please immediately notify the sender and destroy the original message.
>
> Thank you.
>
> Please consider the environment before printing this email.
>
--=20
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* [PATCH 3/3] powerpc/85xx: Cleanup QE initialization for MPC85xxMDS boards
From: Anton Vorontsov @ 2010-06-08 19:55 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
The mpc85xx_mds_setup_arch() function is incomprehensible
and unmaintainable. Factor out all QE specific stuff into
mpc85xx_mds_qe_init() and mpc85xx_mds_reset_ucc_phys().
Also move QE stuff out of mpc85xx_mds_pic_init().
The diff is unreadable, but only because the code was so. ;-)
It should be better now, and less indented.
Signed-off-by: Anton Vorontsov <avorontsov@mvista.com>
---
arch/powerpc/platforms/85xx/mpc85xx_mds.c | 272 +++++++++++++++--------------
1 files changed, 143 insertions(+), 129 deletions(-)
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
index 9dadcff..c8be7b5 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
@@ -177,63 +177,89 @@ static void __init mpc85xx_publish_qe_devices(void)
of_platform_bus_probe(NULL, mpc85xx_qe_ids, NULL);
}
-#else
-static void __init mpc85xx_publish_qe_devices(void) { }
-#endif /* CONFIG_QUICC_ENGINE */
-static void __init mpc85xx_mds_setup_arch(void)
+static void __init mpc85xx_mds_reset_ucc_phys(void)
{
struct device_node *np;
- static u8 __iomem *bcsr_regs = NULL;
-#ifdef CONFIG_PCI
- struct pci_controller *hose;
-#endif
- dma_addr_t max = 0xffffffff;
-
- if (ppc_md.progress)
- ppc_md.progress("mpc85xx_mds_setup_arch()", 0);
+ static u8 __iomem *bcsr_regs;
/* Map BCSR area */
np = of_find_node_by_name(NULL, "bcsr");
- if (np != NULL) {
- struct resource res;
+ if (!np)
+ return;
- of_address_to_resource(np, 0, &res);
- bcsr_regs = ioremap(res.start, res.end - res.start +1);
- of_node_put(np);
- }
+ bcsr_regs = of_iomap(np, 0);
+ of_node_put(np);
+ if (!bcsr_regs)
+ return;
-#ifdef CONFIG_PCI
- for_each_node_by_type(np, "pci") {
- if (of_device_is_compatible(np, "fsl,mpc8540-pci") ||
- of_device_is_compatible(np, "fsl,mpc8548-pcie")) {
- struct resource rsrc;
- of_address_to_resource(np, 0, &rsrc);
- if ((rsrc.start & 0xfffff) == 0x8000)
- fsl_add_bridge(np, 1);
- else
- fsl_add_bridge(np, 0);
+ if (machine_is(mpc8568_mds)) {
+#define BCSR_UCC1_GETH_EN (0x1 << 7)
+#define BCSR_UCC2_GETH_EN (0x1 << 7)
+#define BCSR_UCC1_MODE_MSK (0x3 << 4)
+#define BCSR_UCC2_MODE_MSK (0x3 << 0)
- hose = pci_find_hose_for_OF_device(np);
- max = min(max, hose->dma_window_base_cur +
- hose->dma_window_size);
+ /* Turn off UCC1 & UCC2 */
+ clrbits8(&bcsr_regs[8], BCSR_UCC1_GETH_EN);
+ clrbits8(&bcsr_regs[9], BCSR_UCC2_GETH_EN);
+
+ /* Mode is RGMII, all bits clear */
+ clrbits8(&bcsr_regs[11], BCSR_UCC1_MODE_MSK |
+ BCSR_UCC2_MODE_MSK);
+
+ /* Turn UCC1 & UCC2 on */
+ setbits8(&bcsr_regs[8], BCSR_UCC1_GETH_EN);
+ setbits8(&bcsr_regs[9], BCSR_UCC2_GETH_EN);
+ } else if (machine_is(mpc8569_mds)) {
+#define BCSR7_UCC12_GETHnRST (0x1 << 2)
+#define BCSR8_UEM_MARVELL_RST (0x1 << 1)
+#define BCSR_UCC_RGMII (0x1 << 6)
+#define BCSR_UCC_RTBI (0x1 << 5)
+ /*
+ * U-Boot mangles interrupt polarity for Marvell PHYs,
+ * so reset built-in and UEM Marvell PHYs, this puts
+ * the PHYs into their normal state.
+ */
+ clrbits8(&bcsr_regs[7], BCSR7_UCC12_GETHnRST);
+ setbits8(&bcsr_regs[8], BCSR8_UEM_MARVELL_RST);
+
+ setbits8(&bcsr_regs[7], BCSR7_UCC12_GETHnRST);
+ clrbits8(&bcsr_regs[8], BCSR8_UEM_MARVELL_RST);
+
+ for (np = NULL; (np = of_find_compatible_node(np,
+ "network",
+ "ucc_geth")) != NULL;) {
+ const unsigned int *prop;
+ int ucc_num;
+
+ prop = of_get_property(np, "cell-index", NULL);
+ if (prop == NULL)
+ continue;
+
+ ucc_num = *prop - 1;
+
+ prop = of_get_property(np, "phy-connection-type", NULL);
+ if (prop == NULL)
+ continue;
+
+ if (strcmp("rtbi", (const char *)prop) == 0)
+ clrsetbits_8(&bcsr_regs[7 + ucc_num],
+ BCSR_UCC_RGMII, BCSR_UCC_RTBI);
}
+ } else if (machine_is(p1021_mds)) {
+#define BCSR11_ENET_MICRST (0x1 << 5)
+ /* Reset Micrel PHY */
+ clrbits8(&bcsr_regs[11], BCSR11_ENET_MICRST);
+ setbits8(&bcsr_regs[11], BCSR11_ENET_MICRST);
}
-#endif
-#ifdef CONFIG_SMP
- mpc85xx_smp_init();
-#endif
+ iounmap(bcsr_regs);
+}
-#ifdef CONFIG_SWIOTLB
- if (lmb_end_of_DRAM() > max) {
- ppc_swiotlb_enable = 1;
- set_pci_dma_ops(&swiotlb_dma_ops);
- ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb;
- }
-#endif
+static void __init mpc85xx_mds_qe_init(void)
+{
+ struct device_node *np;
-#ifdef CONFIG_QUICC_ENGINE
np = of_find_compatible_node(NULL, NULL, "fsl,qe");
if (!np) {
np = of_find_node_by_name(NULL, "qe");
@@ -260,70 +286,7 @@ static void __init mpc85xx_mds_setup_arch(void)
par_io_of_config(ucc);
}
- if (bcsr_regs) {
- if (machine_is(mpc8568_mds)) {
-#define BCSR_UCC1_GETH_EN (0x1 << 7)
-#define BCSR_UCC2_GETH_EN (0x1 << 7)
-#define BCSR_UCC1_MODE_MSK (0x3 << 4)
-#define BCSR_UCC2_MODE_MSK (0x3 << 0)
-
- /* Turn off UCC1 & UCC2 */
- clrbits8(&bcsr_regs[8], BCSR_UCC1_GETH_EN);
- clrbits8(&bcsr_regs[9], BCSR_UCC2_GETH_EN);
-
- /* Mode is RGMII, all bits clear */
- clrbits8(&bcsr_regs[11], BCSR_UCC1_MODE_MSK |
- BCSR_UCC2_MODE_MSK);
-
- /* Turn UCC1 & UCC2 on */
- setbits8(&bcsr_regs[8], BCSR_UCC1_GETH_EN);
- setbits8(&bcsr_regs[9], BCSR_UCC2_GETH_EN);
- } else if (machine_is(mpc8569_mds)) {
-#define BCSR7_UCC12_GETHnRST (0x1 << 2)
-#define BCSR8_UEM_MARVELL_RST (0x1 << 1)
-#define BCSR_UCC_RGMII (0x1 << 6)
-#define BCSR_UCC_RTBI (0x1 << 5)
- /*
- * U-Boot mangles interrupt polarity for Marvell PHYs,
- * so reset built-in and UEM Marvell PHYs, this puts
- * the PHYs into their normal state.
- */
- clrbits8(&bcsr_regs[7], BCSR7_UCC12_GETHnRST);
- setbits8(&bcsr_regs[8], BCSR8_UEM_MARVELL_RST);
-
- setbits8(&bcsr_regs[7], BCSR7_UCC12_GETHnRST);
- clrbits8(&bcsr_regs[8], BCSR8_UEM_MARVELL_RST);
-
- for (np = NULL; (np = of_find_compatible_node(np,
- "network",
- "ucc_geth")) != NULL;) {
- const unsigned int *prop;
- int ucc_num;
-
- prop = of_get_property(np, "cell-index", NULL);
- if (prop == NULL)
- continue;
-
- ucc_num = *prop - 1;
-
- prop = of_get_property(np, "phy-connection-type", NULL);
- if (prop == NULL)
- continue;
-
- if (strcmp("rtbi", (const char *)prop) == 0)
- clrsetbits_8(&bcsr_regs[7 + ucc_num],
- BCSR_UCC_RGMII, BCSR_UCC_RTBI);
- }
-
- } else if (machine_is(p1021_mds)) {
-#define BCSR11_ENET_MICRST (0x1 << 5)
- /* Reset Micrel PHY */
- clrbits8(&bcsr_regs[11], BCSR11_ENET_MICRST);
- setbits8(&bcsr_regs[11], BCSR11_ENET_MICRST);
- }
-
- iounmap(bcsr_regs);
- }
+ mpc85xx_mds_reset_ucc_phys();
if (machine_is(p1021_mds)) {
#define MPC85xx_PMUXCR_OFFSET 0x60
@@ -358,7 +321,79 @@ static void __init mpc85xx_mds_setup_arch(void)
}
}
+}
+
+static void __init mpc85xx_mds_qeic_init(void)
+{
+ struct device_node *np;
+
+ np = of_find_compatible_node(NULL, NULL, "fsl,qe");
+ if (!of_device_is_available(np)) {
+ of_node_put(np);
+ return;
+ }
+
+ np = of_find_compatible_node(NULL, NULL, "fsl,qe-ic");
+ if (!np) {
+ np = of_find_node_by_type(NULL, "qeic");
+ if (!np)
+ return;
+ }
+
+ if (machine_is(p1021_mds))
+ qe_ic_init(np, 0, qe_ic_cascade_low_mpic,
+ qe_ic_cascade_high_mpic);
+ else
+ qe_ic_init(np, 0, qe_ic_cascade_muxed_mpic, NULL);
+ of_node_put(np);
+}
+#else
+static void __init mpc85xx_publish_qe_devices(void) { }
+static void __init mpc85xx_mds_qe_init(void) { }
+static void __init mpc85xx_mds_qeic_init(void) { }
#endif /* CONFIG_QUICC_ENGINE */
+
+static void __init mpc85xx_mds_setup_arch(void)
+{
+#ifdef CONFIG_PCI
+ struct pci_controller *hose;
+#endif
+ dma_addr_t max = 0xffffffff;
+
+ if (ppc_md.progress)
+ ppc_md.progress("mpc85xx_mds_setup_arch()", 0);
+
+#ifdef CONFIG_PCI
+ for_each_node_by_type(np, "pci") {
+ if (of_device_is_compatible(np, "fsl,mpc8540-pci") ||
+ of_device_is_compatible(np, "fsl,mpc8548-pcie")) {
+ struct resource rsrc;
+ of_address_to_resource(np, 0, &rsrc);
+ if ((rsrc.start & 0xfffff) == 0x8000)
+ fsl_add_bridge(np, 1);
+ else
+ fsl_add_bridge(np, 0);
+
+ hose = pci_find_hose_for_OF_device(np);
+ max = min(max, hose->dma_window_base_cur +
+ hose->dma_window_size);
+ }
+ }
+#endif
+
+#ifdef CONFIG_SMP
+ mpc85xx_smp_init();
+#endif
+
+ mpc85xx_mds_qe_init();
+
+#ifdef CONFIG_SWIOTLB
+ if (lmb_end_of_DRAM() > max) {
+ ppc_swiotlb_enable = 1;
+ set_pci_dma_ops(&swiotlb_dma_ops);
+ ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb;
+ }
+#endif
}
@@ -465,28 +500,7 @@ static void __init mpc85xx_mds_pic_init(void)
of_node_put(np);
mpic_init(mpic);
-
-#ifdef CONFIG_QUICC_ENGINE
- np = of_find_compatible_node(NULL, NULL, "fsl,qe");
- if (!of_device_is_available(np)) {
- of_node_put(np);
- return;
- }
-
- np = of_find_compatible_node(NULL, NULL, "fsl,qe-ic");
- if (!np) {
- np = of_find_node_by_type(NULL, "qeic");
- if (!np)
- return;
- }
-
- if (machine_is(p1021_mds))
- qe_ic_init(np, 0, qe_ic_cascade_low_mpic,
- qe_ic_cascade_high_mpic);
- else
- qe_ic_init(np, 0, qe_ic_cascade_muxed_mpic, NULL);
- of_node_put(np);
-#endif /* CONFIG_QUICC_ENGINE */
+ mpc85xx_mds_qeic_init();
}
static int __init mpc85xx_mds_probe(void)
--
1.7.0.5
^ permalink raw reply related
* [PATCH 2/3] powerpc/85xx: Fix booting for P1021MDS boards
From: Anton Vorontsov @ 2010-06-08 19:55 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
P1021 processors have no dedicated ROM to store the QE microcode,
so the fimrware is stored externally, and it is U-Boot responsibility
to load it. It might be that the board is booting without QE, e.g.
currently U-Boot doesn't support QE for P1021MDS boards, which means
that QE isn't initialized, and so the board hangs early at boot.
This patch fixes the issue by marking QE as disabled and checking the
state in the probing code. U-Boot should fixup the state if it
initialized the QE.
Signed-off-by: Anton Vorontsov <avorontsov@mvista.com>
---
arch/powerpc/boot/dts/p1021mds.dts | 1 +
arch/powerpc/platforms/85xx/mpc85xx_mds.c | 43 +++++++++++++++++++++++++----
2 files changed, 38 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/boot/dts/p1021mds.dts b/arch/powerpc/boot/dts/p1021mds.dts
index 7fad2df..ad5b852 100644
--- a/arch/powerpc/boot/dts/p1021mds.dts
+++ b/arch/powerpc/boot/dts/p1021mds.dts
@@ -617,6 +617,7 @@
bus-frequency = <0>;
fsl,qe-num-riscs = <1>;
fsl,qe-num-snums = <28>;
+ status = "disabled"; /* no firmware loaded */
qeic: interrupt-controller@80 {
interrupt-controller;
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
index 35ab2b4..9dadcff 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
@@ -158,6 +158,29 @@ static int mpc8568_mds_phy_fixups(struct phy_device *phydev)
extern void __init mpc85xx_smp_init(void);
#endif
+#ifdef CONFIG_QUICC_ENGINE
+static struct of_device_id mpc85xx_qe_ids[] __initdata = {
+ { .type = "qe", },
+ { .compatible = "fsl,qe", },
+ { },
+};
+
+static void __init mpc85xx_publish_qe_devices(void)
+{
+ struct device_node *np;
+
+ np = of_find_compatible_node(NULL, NULL, "fsl,qe");
+ if (!of_device_is_available(np)) {
+ of_node_put(np);
+ return;
+ }
+
+ of_platform_bus_probe(NULL, mpc85xx_qe_ids, NULL);
+}
+#else
+static void __init mpc85xx_publish_qe_devices(void) { }
+#endif /* CONFIG_QUICC_ENGINE */
+
static void __init mpc85xx_mds_setup_arch(void)
{
struct device_node *np;
@@ -218,6 +241,11 @@ static void __init mpc85xx_mds_setup_arch(void)
return;
}
+ if (!of_device_is_available(np)) {
+ of_node_put(np);
+ return;
+ }
+
qe_reset();
of_node_put(np);
@@ -369,8 +397,6 @@ static struct of_device_id mpc85xx_ids[] = {
{ .type = "soc", },
{ .compatible = "soc", },
{ .compatible = "simple-bus", },
- { .type = "qe", },
- { .compatible = "fsl,qe", },
{ .compatible = "gianfar", },
{ .compatible = "fsl,rapidio-delta", },
{ .compatible = "fsl,mpc8548-guts", },
@@ -382,8 +408,6 @@ static struct of_device_id p1021_ids[] = {
{ .type = "soc", },
{ .compatible = "soc", },
{ .compatible = "simple-bus", },
- { .type = "qe", },
- { .compatible = "fsl,qe", },
{ .compatible = "gianfar", },
{},
};
@@ -395,16 +419,16 @@ static int __init mpc85xx_publish_devices(void)
if (machine_is(mpc8569_mds))
simple_gpiochip_init("fsl,mpc8569mds-bcsr-gpio");
- /* Publish the QE devices */
of_platform_bus_probe(NULL, mpc85xx_ids, NULL);
+ mpc85xx_publish_qe_devices();
return 0;
}
static int __init p1021_publish_devices(void)
{
- /* Publish the QE devices */
of_platform_bus_probe(NULL, p1021_ids, NULL);
+ mpc85xx_publish_qe_devices();
return 0;
}
@@ -443,12 +467,19 @@ static void __init mpc85xx_mds_pic_init(void)
mpic_init(mpic);
#ifdef CONFIG_QUICC_ENGINE
+ np = of_find_compatible_node(NULL, NULL, "fsl,qe");
+ if (!of_device_is_available(np)) {
+ of_node_put(np);
+ return;
+ }
+
np = of_find_compatible_node(NULL, NULL, "fsl,qe-ic");
if (!np) {
np = of_find_node_by_type(NULL, "qeic");
if (!np)
return;
}
+
if (machine_is(p1021_mds))
qe_ic_init(np, 0, qe_ic_cascade_low_mpic,
qe_ic_cascade_high_mpic);
--
1.7.0.5
^ permalink raw reply related
* [PATCH 1/3] powerpc/85xx: Fix SWIOTLB initalization for MPC85xxMDS boards
From: Anton Vorontsov @ 2010-06-08 19:55 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
The code inside '#ifdef CONFIG_QUICC_ENGINE' makes the
mpc85xx_mds_setup_arch() return early if no QE nodes present,
and so SWIOTLB is never initialized.
This patch fixes the issue by moving SWIOTLB code above
QE.
Signed-off-by: Anton Vorontsov <avorontsov@mvista.com>
---
arch/powerpc/platforms/85xx/mpc85xx_mds.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
index 8fe87fc..35ab2b4 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
@@ -202,6 +202,14 @@ static void __init mpc85xx_mds_setup_arch(void)
mpc85xx_smp_init();
#endif
+#ifdef CONFIG_SWIOTLB
+ if (lmb_end_of_DRAM() > max) {
+ ppc_swiotlb_enable = 1;
+ set_pci_dma_ops(&swiotlb_dma_ops);
+ ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb;
+ }
+#endif
+
#ifdef CONFIG_QUICC_ENGINE
np = of_find_compatible_node(NULL, NULL, "fsl,qe");
if (!np) {
@@ -323,14 +331,6 @@ static void __init mpc85xx_mds_setup_arch(void)
}
#endif /* CONFIG_QUICC_ENGINE */
-
-#ifdef CONFIG_SWIOTLB
- if (lmb_end_of_DRAM() > max) {
- ppc_swiotlb_enable = 1;
- set_pci_dma_ops(&swiotlb_dma_ops);
- ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb;
- }
-#endif
}
--
1.7.0.5
^ permalink raw reply related
* Re: [PATCH 6/6] of/device: populate platform_device (of_device) resource table on allocation
From: Anton Vorontsov @ 2010-06-08 19:48 UTC (permalink / raw)
To: Grant Likely
Cc: sfr, monstr, devicetree-discuss, microblaze-uclinux, jeremy.kerr,
linuxppc-dev
In-Reply-To: <AANLkTinHH-YhdWpH60Z2BYsiEjzLb_imrk6KUmQRH07P@mail.gmail.com>
On Tue, Jun 08, 2010 at 12:41:47PM -0600, Grant Likely wrote:
[...]
> >> This is a common pattern.
> >
> > This can't be true because it produces ugly casts and fragile
> > code all over the place -- which is exactly what everybody
> > tries to avoid in the kernel.
>
> Fragile? How? &var[1] *always* gives you a pointer to the first
> address after a structure. If the structure changes, then so does the
> offset. Heck, if the type of 'var' changes, then the offset changes
> in kind too. If anything, I should have also used sizeof(*res) in the
> kzalloc call so that the allocated size is protected against a type
> change to 'res' too.
You just introduced an unnamed structure of device + resources,
it isn't declared anywhere but in the code itself (either via
&foo[1] or buf + sizeof(*foo)).
You're not the only one who hacks (or at least have to
understand) the OF stuff, so let's try keep this stuff
readable?
I told you several ways of how to improve the code (based on
the ideas from drivers/base/, so the ideas aren't even mine,
fwiw).
> If you prefer, I can move the dev->resource assignment to immediately
> after the kzalloc to keep everything contained within 4 lines of each
> other.
Sure, that would be better, but I already said what would I
really prefer, i.e.
- A dedicated allocation;
- Or, at least, the same thing as drivers/base/platform.c does:
platform_object { platform_device; name[1]; };
> > But I heard of no such pattern for 'struct device + struct
> > resources' allocation without even some kind of _priv struct,
> > which is surely something new, and ugly.
>
> git grep '\*).*&[a-z1-9_]*\[1\]'
Ugh? This produces a lot of false positives. But OK, let's run it.
~/linux-2.6$ git grep '\*).*&[a-z1-9_]*\[1\]' | wc -l
164
^^^ Now let's presume that half of it (and not just a few hits)
is the ugly hacks that we're talking about. Then compare:
~/linux-2.6$ git grep 'struct.*_priv' | wc -l
22448
^^^ this is a common pattern.
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: [PATCH 6/6] of/device: populate platform_device (of_device) resource table on allocation
From: Grant Likely @ 2010-06-08 18:41 UTC (permalink / raw)
To: Anton Vorontsov
Cc: sfr, monstr, devicetree-discuss, microblaze-uclinux, jeremy.kerr,
linuxppc-dev
In-Reply-To: <20100608164645.GA15216@oksana.dev.rtsoft.ru>
On Tue, Jun 8, 2010 at 10:46 AM, Anton Vorontsov <cbouatmailru@gmail.com> w=
rote:
> On Tue, Jun 08, 2010 at 10:02:49AM -0600, Grant Likely wrote:
>> On Tue, Jun 8, 2010 at 9:57 AM, Anton Vorontsov <cbouatmailru@gmail.com>=
wrote:
>> > On Tue, Jun 08, 2010 at 08:26:43AM -0600, Grant Likely wrote:
>> > [...]
>> >> + =A0 =A0 dev =3D kzalloc(sizeof(*dev) + (sizeof(struct resource) * i=
), GFP_KERNEL);
>> >> =A0 =A0 =A0 if (!dev)
>> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 return NULL;
>> >> -
>> >> =A0 =A0 =A0 dev->dev.of_node =3D of_node_get(np);
>> >> =A0 =A0 =A0 dev->dev.dma_mask =3D &dev->archdata.dma_mask;
>> >> =A0 =A0 =A0 dev->dev.parent =3D parent;
>> >> =A0 =A0 =A0 dev->dev.release =3D of_release_dev;
>> >>
>> >> + =A0 =A0 /* Populate the resource table */
>> >> + =A0 =A0 if (num_irq || num_reg) {
>> >> + =A0 =A0 =A0 =A0 =A0 =A0 dev->resource =3D (void*)&dev[1];
>> >
>> > This is ugly. Why not allocate the memory specifically for
>> > dev->resource? Is this because you plan to get rid of
>> > of_release_dev(), and the generic release_dev() won't
>> > know if it should free the dev->resource? There must
>> > be a better way to handle this.
>>
>> Allocating in one big block means less potential memory fragmentation,
>> and the kernel can free it all at once.
>
> Are there any numbers of saved amount of memory so that we
> could compare?
>
> The "less memory fragmentation" is indeed potential, but
> introduction of obscure code is going on now at this precise
> moment.
It's not obscure. It's smaller and simpler code with fewer error paths.
>> This is a common pattern.
>
> This can't be true because it produces ugly casts and fragile
> code all over the place -- which is exactly what everybody
> tries to avoid in the kernel.
Fragile? How? &var[1] *always* gives you a pointer to the first
address after a structure. If the structure changes, then so does the
offset. Heck, if the type of 'var' changes, then the offset changes
in kind too. If anything, I should have also used sizeof(*res) in the
kzalloc call so that the allocated size is protected against a type
change to 'res' too.
If you prefer, I can move the dev->resource assignment to immediately
after the kzalloc to keep everything contained within 4 lines of each
other.
> Such a pattern is common when a driver allocates e.g. tx and rx
> buffers (of the same type) together, and then split the buffer
> into two pointers.
tx & rx buffers are different because they must be DMAable. That
imposes alignment requirements with kzalloc() guarantees.
> But I heard of no such pattern for 'struct device + struct
> resources' allocation without even some kind of _priv struct,
> which is surely something new, and ugly.
git grep '\*).*&[a-z1-9_]*\[1\]'
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* [PATCH 0/2] mpc5200 ac97 gpio reset
From: Eric Millbrandt @ 2010-06-08 16:46 UTC (permalink / raw)
To: Grant Likely; +Cc: Mark Brown, linuxppc-dev
These patches reimplement the reset fuction in the ac97 to use gpio pins
instead of using the mpc5200 ac97 reset functionality in the psc. This
avoids a problem in which attached ac97 devices go into "test" mode appear
unresponsive.
These patches were tested on a pcm030 baseboard and on custom hardware with
a wm9715 audio codec/touchscreen controller.
Eric Millbrandt
---
Eric Millbrandt (2):
powerpc/5200: Export port-config
sound/soc: mpc5200_psc_ac97: Use gpio pins for cold reset.
arch/powerpc/boot/dts/lite5200.dts | 3 +
arch/powerpc/boot/dts/lite5200b.dts | 3 +
arch/powerpc/boot/dts/pcm030.dts | 3 +
arch/powerpc/boot/dts/pcm032.dts | 3 +
arch/powerpc/include/asm/mpc52xx.h | 2 +
arch/powerpc/platforms/52xx/mpc52xx_common.c | 61 +++++++++++++++++++
sound/soc/fsl/mpc5200_dma.h | 5 ++
sound/soc/fsl/mpc5200_psc_ac97.c | 83 ++++++++++++++++++++++=
++-
8 files changed, 159 insertions(+), 4 deletions(-)
-DISCLAIMER: an automatically appended disclaimer may follow. By posting-
-to a public e-mail mailing list I hereby grant permission to distribute-
-and copy this message.-
This e-mail and the information, including any attachments, it contains are=
intended to be a confidential communication only to the person or entity t=
o whom it is addressed and may contain information that is privileged. If t=
he reader of this message is not the intended recipient, you are hereby not=
ified that any dissemination, distribution or copying of this communication=
is strictly prohibited. If you have received this communication in error, =
please immediately notify the sender and destroy the original message.
Thank you.
Please consider the environment before printing this email.
^ permalink raw reply
* [PATCH 1/2] powerpc/5200: Export port-config
From: Eric Millbrandt @ 2010-06-08 16:46 UTC (permalink / raw)
To: Grant Likely; +Cc: Mark Brown, linuxppc-dev, Eric Millbrandt
In-Reply-To: <1276015562-28928-1-git-send-email-emillbrandt@dekaresearch.com>
Allow device drivers to safely modify port-config. This allows device driv=
ers
access to gpio pins to manually bit-bang slave devices.
Signed-off-by: Eric Millbrandt <emillbrandt@dekaresearch.com>
---
arch/powerpc/include/asm/mpc52xx.h | 2 +
arch/powerpc/platforms/52xx/mpc52xx_common.c | 61 ++++++++++++++++++++++=
++++
2 files changed, 63 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/include/asm/mpc52xx.h b/arch/powerpc/include/asm/=
mpc52xx.h
index b664ce7..ebc5a3b 100644
--- a/arch/powerpc/include/asm/mpc52xx.h
+++ b/arch/powerpc/include/asm/mpc52xx.h
@@ -274,7 +274,9 @@ extern void mpc52xx_declare_of_platform_devices(void);
extern void mpc52xx_map_common_devices(void);
extern int mpc52xx_set_psc_clkdiv(int psc_id, int clkdiv);
extern unsigned int mpc52xx_get_xtal_freq(struct device_node *node);
+extern unsigned int mpc52xx_read_port_config(void);
extern void mpc52xx_restart(char *cmd);
+extern int mpc52xx_write_port_config(u32 mux, int operation);
/* mpc52xx_gpt.c */
struct mpc52xx_gpt_priv;
diff --git a/arch/powerpc/platforms/52xx/mpc52xx_common.c b/arch/powerpc/pl=
atforms/52xx/mpc52xx_common.c
index a46bad0..77c685a 100644
--- a/arch/powerpc/platforms/52xx/mpc52xx_common.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_common.c
@@ -37,6 +37,11 @@ static struct of_device_id mpc52xx_bus_ids[] __initdata =
=3D {
{}
};
+static struct of_device_id mpc52xx_gpio_simple[] =3D {
+ { .compatible =3D "fsl,mpc5200-gpio", },
+ {}
+};
+
/*
* This variable is mapped in mpc52xx_map_wdt() and used in mpc52xx_restar=
t().
* Permanent mapping is required because mpc52xx_restart() can be called
@@ -82,6 +87,15 @@ mpc5200_setup_xlb_arbiter(void)
iounmap(xlb);
}
+/*
+ * This variable is mapped in mpc52xx_write_port_config() and
+ * mpc52xx_read_port_config(). Permanent mapping is required because
+ * mpc52xx_map_and_lock_port_config() can be called from interrupt context
+ * while node mapping (which calls ioremap()) cannot be used at such point=
.
+ */
+static DEFINE_SPINLOCK(mpc52xx_port_config_lock);
+static u32 __iomem *port_config;
+
/**
* mpc52xx_declare_of_platform_devices: register internal devices and chil=
dren
* of the localplus bus to the of_plat=
form
@@ -117,6 +131,7 @@ void __init
mpc52xx_map_common_devices(void)
{
struct device_node *np;
+ const u32 *regaddr;
/* mpc52xx_wdt is mapped here and used in mpc52xx_restart,
* possibly from a interrupt context. wdt is only implement
@@ -135,6 +150,13 @@ mpc52xx_map_common_devices(void)
np =3D of_find_matching_node(NULL, mpc52xx_cdm_ids);
mpc52xx_cdm =3D of_iomap(np, 0);
of_node_put(np);
+
+ /* port_config register */
+ np =3D of_find_matching_node(NULL, mpc52xx_gpio_simple);
+ regaddr =3D of_get_address(np, 0, NULL, NULL);
+ port_config =3D ioremap((u32) of_translate_address(np, regaddr), 0x=
4);
+
+ of_node_put(np);
}
/**
@@ -233,3 +255,42 @@ mpc52xx_restart(char *cmd)
while (1);
}
+
+/**
+ * mpc52xx_write_port_config: Allow mutually exclusive access to the
+ * port_config register
+ *
+ * @newmux: value to write to port_config
+ * @operation: set to 0 for | or 1 for &
+ */
+int mpc52xx_write_port_config(u32 newmux, int operation)
+{
+ unsigned long flags;
+ u32 mux;
+
+ if (!port_config)
+ return -ENODEV;
+
+ spin_lock_irqsave(&mpc52xx_port_config_lock, flags);
+ mux =3D in_be32(port_config);
+ if (operation)
+ out_be32(port_config, mux & newmux);
+ else
+ out_be32(port_config, mux | newmux);
+ spin_unlock_irqrestore(&mpc52xx_port_config_lock, flags);
+
+ return 0;
+}
+EXPORT_SYMBOL(mpc52xx_write_port_config);
+
+/**
+ * mpc52xx_read_port_config: Return the value of port_config
+ */
+unsigned int mpc52xx_read_port_config(void)
+{
+ if (!port_config)
+ return -ENODEV;
+
+ return in_be32(port_config);
+}
+EXPORT_SYMBOL(mpc52xx_read_port_config);
--
1.6.3.1
This e-mail and the information, including any attachments, it contains are=
intended to be a confidential communication only to the person or entity t=
o whom it is addressed and may contain information that is privileged. If t=
he reader of this message is not the intended recipient, you are hereby not=
ified that any dissemination, distribution or copying of this communication=
is strictly prohibited. If you have received this communication in error, =
please immediately notify the sender and destroy the original message.
Thank you.
Please consider the environment before printing this email.
^ permalink raw reply related
* [PATCH 2/2] sound/soc: mpc5200_psc_ac97: Use gpio pins for cold reset.
From: Eric Millbrandt @ 2010-06-08 16:46 UTC (permalink / raw)
To: Grant Likely; +Cc: Mark Brown, linuxppc-dev, Eric Millbrandt
In-Reply-To: <1276015562-28928-2-git-send-email-emillbrandt@dekaresearch.com>
The implementation of the ac97 "cold" reset is flawed. If the sync and
output lines are high when reset is asserted the attached ac97 device
may go into test mode. Avoid this by reconfiguring the psc to gpio mode
and generating the reset manually.
Signed-off-by: Eric Millbrandt <emillbrandt@dekaresearch.com>
---
arch/powerpc/boot/dts/lite5200.dts | 3 +
arch/powerpc/boot/dts/lite5200b.dts | 3 +
arch/powerpc/boot/dts/pcm030.dts | 3 +
arch/powerpc/boot/dts/pcm032.dts | 3 +
sound/soc/fsl/mpc5200_dma.h | 5 ++
sound/soc/fsl/mpc5200_psc_ac97.c | 83 +++++++++++++++++++++++++++++++=
++--
6 files changed, 96 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/boot/dts/lite5200.dts b/arch/powerpc/boot/dts/lit=
e5200.dts
index 82ff2b1..cb4e49b 100644
--- a/arch/powerpc/boot/dts/lite5200.dts
+++ b/arch/powerpc/boot/dts/lite5200.dts
@@ -180,6 +180,9 @@
// compatible =3D "fsl,mpc5200-psc-ac97";
// cell-index =3D <1>;
// reg =3D <0x2200 0x100>;
+ // gpios =3D <&gpio 7 0 /* AC97_1_RES */
+ // &gpio_simple 29 0 /* AC97_1_SYNC */
+ // &gpio_simple 31 0>; /* AC97_1_SDATA_OUT=
*/
// interrupts =3D <2 2 0>;
//};
diff --git a/arch/powerpc/boot/dts/lite5200b.dts b/arch/powerpc/boot/dts/li=
te5200b.dts
index e45a63b..1fb0ac7 100644
--- a/arch/powerpc/boot/dts/lite5200b.dts
+++ b/arch/powerpc/boot/dts/lite5200b.dts
@@ -184,6 +184,9 @@
// compatible =3D "fsl,mpc5200b-psc-ac97","fsl,mpc5200=
-psc-ac97";
// cell-index =3D <1>;
// reg =3D <0x2200 0x100>;
+ // gpios =3D <&gpio 7 0 /* AC97_1_RES */
+ // &gpio_simple 29 0 /* AC97_1_SYNC */
+ // &gpio_simple 31 0>; /* AC97_1_SDATA_OUT=
*/
// interrupts =3D <2 2 0>;
//};
diff --git a/arch/powerpc/boot/dts/pcm030.dts b/arch/powerpc/boot/dts/pcm03=
0.dts
index 8a4ec30..0085e0f 100644
--- a/arch/powerpc/boot/dts/pcm030.dts
+++ b/arch/powerpc/boot/dts/pcm030.dts
@@ -189,6 +189,9 @@
compatible =3D "mpc5200b-psc-ac97","fsl,mpc5200b-ps=
c-ac97";
cell-index =3D <0>;
reg =3D <0x2000 0x100>;
+ gpios =3D <&gpio 7 0 /* AC97_1_RES */
+ &gpio_simple 29 0 /* AC97_1_SYNC */
+ &gpio_simple 31 0>; /* AC97_1_SDATA_OUT=
*/
interrupts =3D <2 1 0>;
};
diff --git a/arch/powerpc/boot/dts/pcm032.dts b/arch/powerpc/boot/dts/pcm03=
2.dts
index 85d857a..76f86d3 100644
--- a/arch/powerpc/boot/dts/pcm032.dts
+++ b/arch/powerpc/boot/dts/pcm032.dts
@@ -189,6 +189,9 @@
compatible =3D "fsl,mpc5200b-psc-ac97","fsl,mpc5200=
-psc-ac97";
cell-index =3D <0>;
reg =3D <0x2000 0x100>;
+ gpios =3D <&gpio 7 0 /* AC97_1_RES */
+ &gpio_simple 29 0 /* AC97_1_SYNC */
+ &gpio_simple 31 0>; /* AC97_1_SDATA_OUT=
*/
interrupts =3D <2 1 0>;
};
diff --git a/sound/soc/fsl/mpc5200_dma.h b/sound/soc/fsl/mpc5200_dma.h
index 22208b3..9fb0248 100644
--- a/sound/soc/fsl/mpc5200_dma.h
+++ b/sound/soc/fsl/mpc5200_dma.h
@@ -61,6 +61,11 @@ struct psc_dma {
int id;
unsigned int slots;
+ /* gpio pins locations for cold reset */
+ int reset_gpio;
+ int sync_gpio;
+ int out_gpio;
+
/* per-stream data */
struct psc_dma_stream playback;
struct psc_dma_stream capture;
diff --git a/sound/soc/fsl/mpc5200_psc_ac97.c b/sound/soc/fsl/mpc5200_psc_a=
c97.c
index e2ee220..bbbf860 100644
--- a/sound/soc/fsl/mpc5200_psc_ac97.c
+++ b/sound/soc/fsl/mpc5200_psc_ac97.c
@@ -9,8 +9,10 @@
* published by the Free Software Foundation.
*/
+#include <linux/gpio.h>
#include <linux/module.h>
#include <linux/of_device.h>
+#include <linux/of_gpio.h>
#include <linux/of_platform.h>
#include <linux/delay.h>
@@ -20,12 +22,17 @@
#include <asm/time.h>
#include <asm/delay.h>
+#include <asm/mpc52xx.h>
#include <asm/mpc52xx_psc.h>
#include "mpc5200_dma.h"
#include "mpc5200_psc_ac97.h"
#define DRV_NAME "mpc5200-psc-ac97"
+#define MPC52xx_GPIO_PSC1_MASK 0x7
+#define MPC52xx_GPIO_PSC2_MASK (0x7<<4)
+#define MPC52xx_AC97_PSC1 0x2
+#define MPC52xx_AC97_PSC2 (0x2<<4)
/* ALSA only supports a single AC97 device so static is recommend here */
static struct psc_dma *psc_dma;
@@ -100,19 +107,69 @@ static void psc_ac97_warm_reset(struct snd_ac97 *ac97=
)
{
struct mpc52xx_psc __iomem *regs =3D psc_dma->psc_regs;
+ mutex_lock(&psc_dma->mutex);
+
out_be32(®s->sicr, psc_dma->sicr | MPC52xx_PSC_SICR_AWR);
udelay(3);
out_be32(®s->sicr, psc_dma->sicr);
+
+ mutex_unlock(&psc_dma->mutex);
}
static void psc_ac97_cold_reset(struct snd_ac97 *ac97)
{
struct mpc52xx_psc __iomem *regs =3D psc_dma->psc_regs;
+ u32 gpio_mux;
+
+ mutex_lock(&psc_dma->mutex);
+
+ /* Reconfigure pin-muxing to gpio */
+ switch (psc_dma->id) {
+ case 0:
+ gpio_mux =3D MPC52xx_GPIO_PSC1_MASK; break;
+ case 1:
+ gpio_mux =3D MPC52xx_GPIO_PSC2_MASK; break;
+ default:
+ dev_err(psc_dma->dev,
+ "Unable to determine PSC, no cold-reset will be "
+ "performed\n");
+ return;
+ }
+
+ dev_err(psc_dma->dev, "cold reset\n");
+ mpc52xx_write_port_config(~gpio_mux, 1);
+
+ /* Assert cold reset */
+ gpio_direction_output(psc_dma->sync_gpio, 0);
+ gpio_direction_output(psc_dma->out_gpio, 0);
+ gpio_direction_output(psc_dma->reset_gpio, 0);
+
+ /* Notify the PSC that a cold reset is occurring */
+ out_be32(®s->sicr, 0);
+ udelay(2);
+
+ /* Deassert reset */
+ gpio_direction_output(psc_dma->reset_gpio, 1);
+ msleep(1);
+
+ /* Restore pin-muxing */
+ switch (psc_dma->id) {
+ case 0:
+ gpio_mux =3D MPC52xx_AC97_PSC1; break;
+ case 1:
+ gpio_mux =3D MPC52xx_AC97_PSC2; break;
+ default:
+ return;
+ }
+
+ mpc52xx_write_port_config(gpio_mux, 0);
+
+ /* Restore the serial interface mode to AC97 */
+ out_be32(®s->sicr, psc_dma->sicr);
+ out_8(®s->command, MPC52xx_PSC_TX_ENABLE | MPC52xx_PSC_RX_ENABLE=
);
+
+ mutex_unlock(&psc_dma->mutex);
- /* Do a cold reset */
- out_8(®s->op1, MPC52xx_PSC_OP_RES);
- udelay(10);
- out_8(®s->op0, MPC52xx_PSC_OP_RES);
msleep(1);
psc_ac97_warm_reset(ac97);
}
@@ -287,6 +344,24 @@ static int __devinit psc_ac97_of_probe(struct of_devic=
e *op,
regs =3D psc_dma->psc_regs;
ac97.private_data =3D psc_dma;
+ psc_dma->reset_gpio =3D of_get_gpio_flags(op->node, 0, NULL);
+ psc_dma->sync_gpio =3D of_get_gpio_flags(op->node, 1, NULL);
+ psc_dma->out_gpio =3D of_get_gpio_flags(op->node, 2, NULL);
+ if ((psc_dma->reset_gpio < 0) ||
+ (psc_dma->sync_gpio < 0) ||
+ (psc_dma->out_gpio < 0)) {
+ dev_err(&op->dev, "error: cannot get GPIO pins; "
+ "reset=3D%i sync=3D%i out=3D%i\n",
+ psc_dma->reset_gpio,
+ psc_dma->sync_gpio,
+ psc_dma->out_gpio);
+ return -ENODEV;
+ }
+
+ gpio_request(psc_dma->reset_gpio, "psc_dma-reset");
+ gpio_request(psc_dma->sync_gpio, "psc_dma-sync");
+ gpio_request(psc_dma->out_gpio, "psc_dma-out");
+
for (i =3D 0; i < ARRAY_SIZE(psc_ac97_dai); i++)
psc_ac97_dai[i].private_data =3D psc_dma;
--
1.6.3.1
This e-mail and the information, including any attachments, it contains are=
intended to be a confidential communication only to the person or entity t=
o whom it is addressed and may contain information that is privileged. If t=
he reader of this message is not the intended recipient, you are hereby not=
ified that any dissemination, distribution or copying of this communication=
is strictly prohibited. If you have received this communication in error, =
please immediately notify the sender and destroy the original message.
Thank you.
Please consider the environment before printing this email.
^ permalink raw reply related
* Re: [PATCH 6/6] of/device: populate platform_device (of_device) resource table on allocation
From: Anton Vorontsov @ 2010-06-08 16:46 UTC (permalink / raw)
To: Grant Likely
Cc: sfr, monstr, devicetree-discuss, microblaze-uclinux, jeremy.kerr,
linuxppc-dev
In-Reply-To: <AANLkTinZT4z93SWxjce5oRZzmaQHtVnO96zsXO9OX2UZ@mail.gmail.com>
On Tue, Jun 08, 2010 at 10:02:49AM -0600, Grant Likely wrote:
> On Tue, Jun 8, 2010 at 9:57 AM, Anton Vorontsov <cbouatmailru@gmail.com> wrote:
> > On Tue, Jun 08, 2010 at 08:26:43AM -0600, Grant Likely wrote:
> > [...]
> >> + dev = kzalloc(sizeof(*dev) + (sizeof(struct resource) * i), GFP_KERNEL);
> >> if (!dev)
> >> return NULL;
> >> -
> >> dev->dev.of_node = of_node_get(np);
> >> dev->dev.dma_mask = &dev->archdata.dma_mask;
> >> dev->dev.parent = parent;
> >> dev->dev.release = of_release_dev;
> >>
> >> + /* Populate the resource table */
> >> + if (num_irq || num_reg) {
> >> + dev->resource = (void*)&dev[1];
> >
> > This is ugly. Why not allocate the memory specifically for
> > dev->resource? Is this because you plan to get rid of
> > of_release_dev(), and the generic release_dev() won't
> > know if it should free the dev->resource? There must
> > be a better way to handle this.
>
> Allocating in one big block means less potential memory fragmentation,
> and the kernel can free it all at once.
Are there any numbers of saved amount of memory so that we
could compare?
The "less memory fragmentation" is indeed potential, but
introduction of obscure code is going on now at this precise
moment.
> This is a common pattern.
This can't be true because it produces ugly casts and fragile
code all over the place -- which is exactly what everybody
tries to avoid in the kernel.
Such a pattern is common when a driver allocates e.g. tx and rx
buffers (of the same type) together, and then split the buffer
into two pointers.
But I heard of no such pattern for 'struct device + struct
resources' allocation without even some kind of _priv struct,
which is surely something new, and ugly.
If you really want to avoid (an unproven) memory fragmentation,
you could do:
struct of_device_with_resources {
struct device dev;
struct resource resourses[0];
};
This at least will get rid of the casts and incomprehensible
"dev->resource = (void*)&dev[1];" construction.
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: [PATCH 6/6] of/device: populate platform_device (of_device) resource table on allocation
From: Grant Likely @ 2010-06-08 16:02 UTC (permalink / raw)
To: Anton Vorontsov
Cc: ben, sfr, monstr, microblaze-uclinux, devicetree-discuss,
jeremy.kerr, linuxppc-dev
In-Reply-To: <20100608155702.GA5419@oksana.dev.rtsoft.ru>
On Tue, Jun 8, 2010 at 9:57 AM, Anton Vorontsov <cbouatmailru@gmail.com> wr=
ote:
> On Tue, Jun 08, 2010 at 08:26:43AM -0600, Grant Likely wrote:
> [...]
>> + =A0 =A0 dev =3D kzalloc(sizeof(*dev) + (sizeof(struct resource) * i), =
GFP_KERNEL);
>> =A0 =A0 =A0 if (!dev)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 return NULL;
>> -
>> =A0 =A0 =A0 dev->dev.of_node =3D of_node_get(np);
>> =A0 =A0 =A0 dev->dev.dma_mask =3D &dev->archdata.dma_mask;
>> =A0 =A0 =A0 dev->dev.parent =3D parent;
>> =A0 =A0 =A0 dev->dev.release =3D of_release_dev;
>>
>> + =A0 =A0 /* Populate the resource table */
>> + =A0 =A0 if (num_irq || num_reg) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 dev->resource =3D (void*)&dev[1];
>
> This is ugly. Why not allocate the memory specifically for
> dev->resource? Is this because you plan to get rid of
> of_release_dev(), and the generic release_dev() won't
> know if it should free the dev->resource? There must
> be a better way to handle this.
Allocating in one big block means less potential memory fragmentation,
and the kernel can free it all at once. This is a common pattern.
> p.s.
>
> Just wonder what happened to of_gpio stuff? You blocked it
> in 2.6.34 for no reason saying "I'll pick it into my OF
> tree before the 2.6.35 merge window" and it's 2.6.36 merge
> window quite soon.
It's in my test-devicetree branch. I'll be posting for 2nd review in
the next few days and then adding to my devicetree-next branch
probably before the end of the week.
g.
^ permalink raw reply
* Re: [PATCH 6/6] of/device: populate platform_device (of_device) resource table on allocation
From: Anton Vorontsov @ 2010-06-08 15:57 UTC (permalink / raw)
To: Grant Likely
Cc: ben, sfr, monstr, microblaze-uclinux, devicetree-discuss,
jeremy.kerr, linuxppc-dev
In-Reply-To: <20100608142643.26088.61366.stgit@angua>
On Tue, Jun 08, 2010 at 08:26:43AM -0600, Grant Likely wrote:
[...]
> + dev = kzalloc(sizeof(*dev) + (sizeof(struct resource) * i), GFP_KERNEL);
> if (!dev)
> return NULL;
> -
> dev->dev.of_node = of_node_get(np);
> dev->dev.dma_mask = &dev->archdata.dma_mask;
> dev->dev.parent = parent;
> dev->dev.release = of_release_dev;
>
> + /* Populate the resource table */
> + if (num_irq || num_reg) {
> + dev->resource = (void*)&dev[1];
This is ugly. Why not allocate the memory specifically for
dev->resource? Is this because you plan to get rid of
of_release_dev(), and the generic release_dev() won't
know if it should free the dev->resource? There must
be a better way to handle this.
p.s.
[Two Minutes Hate for Grant.]
Just wonder what happened to of_gpio stuff? You blocked it
in 2.6.34 for no reason saying "I'll pick it into my OF
tree before the 2.6.35 merge window" and it's 2.6.36 merge
window quite soon.
It's still in origin/test-devicetree, which is obviously
not -next. What is your plan now?
Thanks,
> + dev->num_resources = num_reg + num_irq;
> + res = dev->resource;
> + for (i = 0; i < num_reg; i++, res++) {
> + rc = of_address_to_resource(np, i, res);
> + WARN_ON(rc);
> + }
> + for (i = 0; i < num_irq; i++, res++) {
> + rc = of_irq_to_resource(np, i, res);
> + WARN_ON(rc == NO_IRQ);
> + }
> + }
> +
> if (bus_id)
> dev_set_name(&dev->dev, "%s", bus_id);
> else
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ 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