LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] powerpc ptrace block-step
From: Benjamin Herrenschmidt @ 2009-05-29  5:03 UTC (permalink / raw)
  To: Roland McGrath; +Cc: linuxppc-dev, utrace-devel
In-Reply-To: <20090401215903.DE872FC3AB@magilla.sf.frob.com>

On Wed, 2009-04-01 at 14:59 -0700, Roland McGrath wrote:
> Maynard asked about user_enable_block_step() support on powerpc.
> This is the old patch I've posted before.  I haven't even tried
> to compile it lately, but it rebased cleanly.
> 
> AFAIK the only reason this didn't go in several months ago was waiting
> for someone to decide what the right arch_has_block_step() condition was,
> i.e. if it needs to check some cpu_feature or chip identifier bits.
> 
> I had hoped that I had passed the buck then to ppc folks to figure that out
> and make it so.  But it does not appear to have happened.
> 
> Note you can drop the #define PTRACE_SINGLEBLOCK if you want to be
> conservative and not touch the user (ptrace) ABI yet.  Then Maynard
> could beat on it with internal uses (utrace) before you worry about
> whether userland expects the new ptrace request macro to exist.

So the patch had some issues, such as missing clearing of DBCR0 bits,
missing changes to code in traps.c to properly identify the new cause
of debug interrupts, etc...

I've spinned a new version, I'll post it as soon as I got to do some
quick tests. It will then go into the next merge window hopefully.

Note: I've verified, blockstep seems to be implemented by all the core
variants -except- the old 601.

Cheers,
Ben.

^ permalink raw reply

* Re: [Patch 4/6] Modify process and processor handling code to recognise hardware debug registers
From: David Gibson @ 2009-05-29  4:29 UTC (permalink / raw)
  To: K.Prasad
  Cc: Michael Neuling, Benjamin Herrenschmidt, linuxppc-dev, Alan Stern,
	paulus, Roland McGrath
In-Reply-To: <20090525011650.GE11078@in.ibm.com>

On Mon, May 25, 2009 at 06:46:50AM +0530, K.Prasad wrote:
> Modify process handling code to recognise hardware debug registers during copy
> and flush operations. Introduce a new TIF_DEBUG task flag to indicate a
> process's use of debug register. Load the debug register values into a
> new CPU during initialisation.

[snip]
> Index: linux-2.6-tip.hbkpt/arch/powerpc/kernel/process.c
> ===================================================================
> --- linux-2.6-tip.hbkpt.orig/arch/powerpc/kernel/process.c
> +++ linux-2.6-tip.hbkpt/arch/powerpc/kernel/process.c
> @@ -50,6 +50,7 @@
>  #include <asm/syscalls.h>
>  #ifdef CONFIG_PPC64
>  #include <asm/firmware.h>
> +#include <asm/hw_breakpoint.h>
>  #endif
>  #include <linux/kprobes.h>
>  #include <linux/kdebug.h>
> @@ -254,8 +255,10 @@ void do_dabr(struct pt_regs *regs, unsig
>  			11, SIGSEGV) == NOTIFY_STOP)
>  		return;
>  
> +#ifndef CONFIG_PPC64
>  	if (debugger_dabr_match(regs))
>  		return;
> +#endif
>  
>  	/* Clear the DAC and struct entries.  One shot trigger */
>  #if defined(CONFIG_BOOKE)
> @@ -372,8 +375,13 @@ struct task_struct *__switch_to(struct t
>  
>  #endif /* CONFIG_SMP */
>  
> +#ifdef CONFIG_PPC64
> +		if (unlikely(test_tsk_thread_flag(new, TIF_DEBUG)))
> +			arch_install_thread_hw_breakpoint(new);
> +#else
>  	if (unlikely(__get_cpu_var(current_dabr) != new->thread.dabr))
>  		set_dabr(new->thread.dabr);
> +#endif /* CONFIG_PPC64 */
>  
>  #if defined(CONFIG_BOOKE)
>  	/* If new thread DAC (HW breakpoint) is the same then leave it */
> @@ -550,6 +558,10 @@ void show_regs(struct pt_regs * regs)
>  void exit_thread(void)
>  {
>  	discard_lazy_cpu_state();
> +#ifdef CONFIG_PPC64
> +	if (unlikely(test_tsk_thread_flag(current, TIF_DEBUG)))
> +		flush_thread_hw_breakpoint(current);
> +#endif /* CONFIG_PPC64 */
>  }
>  
>  void flush_thread(void)
> @@ -605,6 +617,9 @@ int copy_thread(unsigned long clone_flag
>  	struct pt_regs *childregs, *kregs;
>  	extern void ret_from_fork(void);
>  	unsigned long sp = (unsigned long)task_stack_page(p) + THREAD_SIZE;
> +#ifdef CONFIG_PPC64
> +	struct task_struct *tsk = current;

I don't see any point to adding this variable, just reference current
directly  below.

> +#endif
>  
>  	CHECK_FULL_REGS(regs);
>  	/* Copy registers */
> @@ -672,6 +687,9 @@ int copy_thread(unsigned long clone_flag
>  	 * function.
>   	 */
>  	kregs->nip = *((unsigned long *)ret_from_fork);
> +
> +	if (unlikely(test_tsk_thread_flag(tsk, TIF_DEBUG)))
> +		copy_thread_hw_breakpoint(tsk, p, clone_flags);
>  #else
>  	kregs->nip = (unsigned long)ret_from_fork;
>  #endif

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [Patch 2/6] Introduce PPC64 specific Hardware Breakpoint interfaces
From: David Gibson @ 2009-05-29  4:18 UTC (permalink / raw)
  To: K.Prasad
  Cc: Michael Neuling, Benjamin Herrenschmidt, linuxppc-dev, Alan Stern,
	paulus, Roland McGrath
In-Reply-To: <20090525011522.GC11078@in.ibm.com>

On Mon, May 25, 2009 at 06:45:22AM +0530, K.Prasad wrote:
> Introduce PPC64 implementation for the generic hardware breakpoint interfaces
> defined in kernel/hw_breakpoint.c. Enable the HAVE_HW_BREAKPOINT flag and the
> Makefile.

[snip]
> +/* Store the kernel-space breakpoint address value */
> +static unsigned long kdabr;
> +
> +/*
> + * Temporarily stores address for DABR before it is written by the
> + * single-step handler routine
> + */
> +static DEFINE_PER_CPU(unsigned long, dabr_data);
> +
> +void arch_update_kernel_hw_breakpoint(void *unused)
> +{
> +	struct hw_breakpoint *bp;
> +
> +	/* Check if there is nothing to update */
> +	if (hbp_kernel_pos == HBP_NUM)
> +		return;
> +
> +	per_cpu(this_hbp_kernel[hbp_kernel_pos], get_cpu()) = bp =
> +						hbp_kernel[hbp_kernel_pos];
> +	if (bp == NULL)
> +		kdabr = 0;
> +	else
> +		kdabr = (bp->info.address & ~HW_BREAKPOINT_ALIGN) |
> +			bp->info.type | DABR_TRANSLATION;
> +	set_dabr(kdabr);
> +	put_cpu_no_resched();
> +}
> +
> +/*
> + * Install the thread breakpoints in their debug registers.
> + */
> +void arch_install_thread_hw_breakpoint(struct task_struct *tsk)
> +{
> +	set_dabr(tsk->thread.dabr);
> +}
> +
> +/*
> + * Install the debug register values for just the kernel, no thread.
> + */
> +void arch_uninstall_thread_hw_breakpoint()
> +{
> +	set_dabr(0);
> +}
> +
> +/*
> + * Store a breakpoint's encoded address, length, and type.
> + */
> +int arch_store_info(struct hw_breakpoint *bp, struct task_struct *tsk)
> +{
> +	/*
> +	 * User-space requests will always have the address field populated
> +	* Symbol names from user-space are rejected
> +	*/
> +	if (tsk && bp->info.name)
> +		return -EINVAL;
> +	/*
> +	 * User-space requests will always have the address field populated
> +	 * For kernel-addresses, either the address or symbol name can be
> +	 * specified.
> +	 */
> +	if (bp->info.name)
> +		bp->info.address = (unsigned long)
> +					kallsyms_lookup_name(bp->info.name);
> +	if (bp->info.address)
> +		return 0;
> +	return -EINVAL;
> +}
> +
> +/*
> + * Validate the arch-specific HW Breakpoint register settings
> + */
> +int arch_validate_hwbkpt_settings(struct hw_breakpoint *bp,
> +						struct task_struct *tsk)
> +{
> +	int is_kernel, ret = -EINVAL;
> +
> +	if (!bp)
> +		return ret;
> +
> +	switch (bp->info.type) {
> +	case HW_BREAKPOINT_READ:
> +	case HW_BREAKPOINT_WRITE:
> +	case HW_BREAKPOINT_RW:
> +		break;
> +	default:
> +		return ret;
> +	}
> +
> +	if (bp->triggered)
> +		ret = arch_store_info(bp, tsk);
> +
> +	is_kernel = is_kernel_addr(bp->info.address);
> +	if ((tsk && is_kernel) || (!tsk && !is_kernel))
> +		return -EINVAL;
> +
> +	return ret;
> +}
> +
> +void arch_update_user_hw_breakpoint(int pos, struct task_struct *tsk)
> +{
> +	struct thread_struct *thread = &(tsk->thread);
> +	struct hw_breakpoint *bp = thread->hbp[0];
> +
> +	if (bp)
> +		thread->dabr = (bp->info.address & ~HW_BREAKPOINT_ALIGN) |
> +				bp->info.type | DABR_TRANSLATION;
> +	else
> +		thread->dabr = 0;
> +}
> +
> +void arch_flush_thread_hw_breakpoint(struct task_struct *tsk)
> +{
> +	struct thread_struct *thread = &(tsk->thread);
> +
> +	thread->dabr = 0;
> +}
> +
> +/*
> + * Handle debug exception notifications.
> + */
> +int __kprobes hw_breakpoint_handler(struct die_args *args)
> +{
> +	int rc = NOTIFY_STOP;
> +	struct hw_breakpoint *bp;
> +	struct pt_regs *regs = args->regs;
> +	unsigned long dar = regs->dar;
> +	int cpu, stepped = 1;
> +
> +	/* Disable breakpoints during exception handling */
> +	set_dabr(0);
> +
> +	cpu = get_cpu();
> +	/* Determine whether kernel- or user-space address is the trigger */
> +	bp = (hbp_kernel_pos == HBP_NUM) ? current->thread.hbp[0] :
> +					per_cpu(this_hbp_kernel[0], cpu);
> +	/*
> +	 * bp can be NULL due to lazy debug register switching
> +	 * or due to the delay between updates of hbp_kernel_pos
> +	 * and this_hbp_kernel.
> +	 */
> +	if (!bp)
> +		goto out;
> +
> +	if (dar == bp->info.address)
> +		per_cpu(dabr_data, cpu) = (hbp_kernel_pos == HBP_NUM) ?
> +						current->thread.dabr : kdabr;
> +	else {
> +		/*
> +		 * This exception is triggered not because of a memory access on
> +		 * the monitored variable but in the double-word address range
> +		 * in which it is contained. We will consume this exception,
> +		 * considering it as 'noise'.
> +		 */
> +		rc = NOTIFY_STOP;
> +		goto out;
> +	}
> +	(bp->triggered)(bp, regs);

This will fire the handler function before the instruction has
executed.  I remember seeing a comment in the other patchset saying
the function would be triggered after execution, but I'm not sure if
that was in generic of x86-specific code.

> +
> +	stepped = emulate_step(regs, regs->nip);
> +	/*
> +	 * Single-step the causative instruction manually if
> +	 * emulate_step() could not execute it
> +	 */
> +	if (stepped == 0) {
> +		regs->msr |= MSR_SE;
> +		goto out;
> +	}
> +	set_dabr(per_cpu(dabr_data, cpu));
> +	per_cpu(dabr_data, cpu) = 0;

This curly arrangement of put_cpu() / get_cpu() could probably do with
some more comments...

> +out:
> +	/* Enable pre-emption only if single-stepping is finished */
> +	if (stepped)
> +		put_cpu_no_resched();
> +	return rc;
> +}
> +
> +/*
> + * Handle single-step exceptions following a DABR hit.
> + */
> +int __kprobes single_step_dabr_instruction(struct die_args *args)
> +{
> +	struct pt_regs *regs = args->regs;
> +	int cpu = get_cpu();
> +	int ret = NOTIFY_DONE;
> +	siginfo_t info;
> +	unsigned long this_dabr_data = per_cpu(dabr_data, cpu);
> +
> +	/*
> +	 * Check if we are single-stepping as a result of a
> +	 * previous HW Breakpoint exception
> +	 */
> +	if (this_dabr_data == 0)
> +		goto out;
> +
> +	regs->msr &= ~MSR_SE;
> +	/* Deliver signal to user-space */
> +	if (this_dabr_data < TASK_SIZE) {
> +		info.si_signo = SIGTRAP;
> +		info.si_errno = 0;
> +		info.si_code = TRAP_HWBKPT;
> +		info.si_addr = (void __user *)(per_cpu(dabr_data, cpu));
> +		force_sig_info(SIGTRAP, &info, current);
> +	}

Ok, this is a behaviour change - the old do_dabr() code fired the
SIGTRAP before the instruction completed, but this will fire it
after.  It seems simpler and safer to move this into ptrace's
triggered function.

> +
> +	set_dabr(this_dabr_data);
> +	per_cpu(dabr_data, cpu) = 0;
> +	ret = NOTIFY_STOP;
> +	/*
> +	 * If single-stepped after hw_breakpoint_handler(), pre-emption is
> +	 * already disabled.
> +	 */
> +	put_cpu_no_resched();
> +
> +out:
> +	/*
> +	 * A put_cpu_no_resched() call is required to complement the get_cpu()
> +	 * call used initially
> +	 */
> +	put_cpu_no_resched();
> +	return ret;
> +}
> +
> +/*
> + * Handle debug exception notifications.
> + */
> +int __kprobes hw_breakpoint_exceptions_notify(
> +		struct notifier_block *unused, unsigned long val, void *data)
> +{
> +	int ret = NOTIFY_DONE;
> +
> +	switch (val) {
> +	case DIE_DABR_MATCH:
> +		ret = hw_breakpoint_handler(data);
> +		break;
> +	case DIE_SSTEP:
> +		ret = single_step_dabr_instruction(data);
> +		break;
> +	}
> +
> +	return ret;
> +}
-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [Patch 1/6] Prepare the PowerPC platform for HW Breakpoint infrastructure
From: David Gibson @ 2009-05-29  3:20 UTC (permalink / raw)
  To: K.Prasad
  Cc: Michael Neuling, Benjamin Herrenschmidt, linuxppc-dev, Alan Stern,
	paulus, Roland McGrath
In-Reply-To: <20090525011423.GB11078@in.ibm.com>

On Mon, May 25, 2009 at 06:44:23AM +0530, K.Prasad wrote:
> Prepare the PowerPC code for HW Breakpoint infrastructure patches by including
> relevant constant definitions and function declarations.
> 
> Signed-off-by: K.Prasad <prasad@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/hw_breakpoint.h |   57 +++++++++++++++++++++++++++++++
>  arch/powerpc/include/asm/processor.h     |    1 
>  arch/powerpc/include/asm/reg.h           |    3 +
>  3 files changed, 61 insertions(+)
> 
> Index: linux-2.6-tip.hbkpt/arch/powerpc/include/asm/hw_breakpoint.h
> ===================================================================
> --- /dev/null
> +++ linux-2.6-tip.hbkpt/arch/powerpc/include/asm/hw_breakpoint.h
> @@ -0,0 +1,57 @@
> +#ifndef	_PPC64_HW_BREAKPOINT_H
> +#define	_PPC64_HW_BREAKPOINT_H
> +
> +#ifdef	__KERNEL__
> +#define	__ARCH_HW_BREAKPOINT_H
> +#ifdef CONFIG_PPC64
> +
> +struct arch_hw_breakpoint {
> +	char		*name; /* Contains name of the symbol to set bkpt */
> +	unsigned long	address;
> +	u8		type;

You might as well make this an int, it will get padded out to 4 bytes
long anyway.

> +};
> +
> +#include <linux/kdebug.h>
> +#include <asm/reg.h>
> +#include <asm-generic/hw_breakpoint.h>
> +
> +#define HW_BREAKPOINT_READ DABR_DATA_READ
> +#define HW_BREAKPOINT_WRITE DABR_DATA_WRITE
> +#define HW_BREAKPOINT_RW (DABR_DATA_READ | DABR_DATA_WRITE)
> +
> +#define HW_BREAKPOINT_ALIGN 0x7
> +#define HW_BREAKPOINT_LEN INSTRUCTION_LEN
> +
> +extern struct hw_breakpoint *hbp_kernel[HBP_NUM];
> +DECLARE_PER_CPU(struct hw_breakpoint*, this_hbp_kernel[HBP_NUM]);
> +extern unsigned int hbp_user_refcount[HBP_NUM];
> +
> +extern void arch_install_thread_hw_breakpoint(struct task_struct *tsk);
> +extern void arch_uninstall_thread_hw_breakpoint(void);
> +extern int arch_validate_hwbkpt_settings(struct hw_breakpoint *bp,
> +						struct task_struct *tsk);
> +extern void arch_update_user_hw_breakpoint(int pos, struct task_struct *tsk);
> +extern void arch_flush_thread_hw_breakpoint(struct task_struct *tsk);
> +extern void arch_update_kernel_hw_breakpoint(void *);
> +extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
> +				     unsigned long val, void *data);
> +
> +extern void flush_thread_hw_breakpoint(struct task_struct *tsk);
> +extern int copy_thread_hw_breakpoint(struct task_struct *tsk,
> +		struct task_struct *child, unsigned long clone_flags);
> +extern void load_debug_registers(void );

It looks as though a lot of these arch hooks ought to have prototypes
in the generic part of the infrastructure.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Fwd: serial8250: too much work
From: Alemao @ 2009-05-29  2:57 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <d970ff420905281057kefbf339y29257154c142186a@mail.gmail.com>

Im facing some problems with serial, getting flooded with this
message:

---
serial8250: too much work for irq16
---

Something I notice, in my .dts I have the following lines:

serial0: serial@4500, interrupts =3D <9 =A00x8>
serial1: serial@4600, interrupts =3D <10 0x8>
spi: =A0 =A0   spi@7000, =A0 =A0  interrupts =3D <16 0x8>

But when kernel starts:

---
Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
serial8250.0: ttyS0 at MMIO 0xe0004500 (irq =3D 16) is a 16550A
---

Why IRQ 16? Shouldn't it be IRQ 9?

I traced functions calls in 8250.c driver:

---
serial8250_interrup() -> serial8250_handle_port() -> transmit_chars()
---

"too much work" is for transmiting, but there's nothing to transmit.

In my MPC8343 board I have only serial0 connected to a MAX3232, serial1
is floating. Maybe some electromagnetic interference in the bus?

I found this comments in linux/drivers/serial/8250.h, and tried to use
same defines that they used, but no success:

/*
=A0* Digital did something really horribly wrong with the OUT1 and OUT2
=A0* lines on at least some ALPHA's. =A0The failure mode is that if either
=A0* is cleared, the machine locks up with endless interrupts.
=A0*/

/*
=A0* WindRiver did something similarly broken on their SBC8560 board. The
=A0* UART tristates its IRQ output while OUT2 is clear, but they pulled
=A0* the interrupt line _up_ instead of down, so if we register the IRQ
=A0* while the UART is in that state, we die in an IRQ storm.
=A0*/


I also tried removing serial1 and spi from .dts, but didn't work.

Any sugestions?

Cheers,

--
Alemao

^ permalink raw reply

* Re: 8544 external interrupt configuration problems
From: Nancy Isaac @ 2009-05-29  1:48 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <20090528200215.GA9793@b07421-ec1.am.freescale.net>

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

Thank you for your response.  My responses below.

On Thu, May 28, 2009 at 1:02 PM, Scott Wood <scottwood@freescale.com> wrote:

> On Thu, May 28, 2009 at 12:05:52PM -0700, Nancy Isaac wrote:
> > My device tree has the following entry for my fpga:
> >
> > CpuCpld\@f0000000{
> >                         compatible = "MPC8544DS";
> >                         device_type = "CpuCpld";
> >                         reg = <f0000000 00000040>;
> >                         interrupts = <41 2 42 2 43 2>;
> >                         interrupt-parent = <&mpic>;
> > };
>
> Where did you get those interrupt numbers from?  External interrupt
> numbers start at zero on MPIC.


<NI> I  tried several  things and this is the last set I tried. These
numbers start at the end of the internal interrupts.

I also tried specifying 1, 2 and 3 because it looks like there is just an
array of these interrupts and the interrupts are just added to this array as
they are registered with the hwirq and the virq is the index into the array
where the hwirq was added.  I get slightly different results but still not
accurate.

For example, in the device tree, I had setup interrupts = <1 2 2 2 3 2>; for
irq1, 2 and 3 which are all level sensitive interrupts. These map to virq
0x12, 0x13 and 0x14 .  I verify that there is no interrupt present by
looking at the registers in the FPGA and also by connecting a scope to the
signal.  Immediately after I finish registering the irqs listed above, I get
an interrupt  and my print statement in the do_IRQ function returns irq
number 0x12.  This happens even if I've disabled the interrupt at the FPGA.
Then I forced an interrupt to occur at the FPGA on irq2 and the do_irq
function still reports that the irq occured for 0x12.

>
> Is the level/sense information correct?


[NI] Yes, these interrupts are supposed to be active low, level sensitive
interrupts.


> > My driver does the mapping to the virq:
> >
> >  np = of_find_node_by_type(NULL, "CpuCpld");
> >     if (!np) {
> >         ret = -ENODEV;
> >     }
> >     cpldCpuDrv->MateIntIrq = irq_of_parse_and_map(np, 0);
> >     cpldCpuDrv->FtaIrq =  irq_of_parse_and_map(np, 1);
> >     cpldCpuDrv->ExtractIrq =  irq_of_parse_and_map(np, 2);
> >     cpldCpuDrv->XauiIrq =  irq_of_parse_and_map(np, 0);
> >
> >     of_node_put(np);
>
> Looks good (other than that you should be using compatible (with a more
> specific name) rather than device_type).
>
> > Does anyone know what the virq should be for these external interrupts?
>
> They're dynamically assigned.
>
> > I've tried specifying the actual irq numbers 1,2 and 3 and that doesn't
> work
> > either.
>
> Specifying them where?  In the device tree or as virq numbers?  Never
> hard-code virq numbers.


[NI] I specify this in the device tree.

>
>
> > Is there some PCI configuration that's getting in the way?  I've tried
> > disabling the FSL_PCIE and I get the same behavior.
>
> Barring an unusual bug, PCI should have nothing to do with the interrupt
> routing of things that aren't on the PCI bus.


[NI]. For 8544, the external interrupts can be shared with PCI Express. I
thought that if I am somehow enabling PCI Express, maybe it would explain
this behavior.


>
>
> -Scott
>

Thanks
Nancy

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

^ permalink raw reply

* Re: MPC8272- Porting HDLC driver from 2.6.14 to 2.6.27- "no_irq_chip" error
From: Daniel Ng @ 2009-05-29  0:46 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linuxppc-dev@ozlabs.org, Norbert van Bolhuis
In-Reply-To: <20090528123311.GA3112@pengutronix.de>

On Thu, May 28, 2009 at 10:33 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:
>> this is an example of how a simple 8313 Periodic Interval Timer (PIT) kernel driver
>> registers for the PIT IRQ (Interrupt ID 65)
>>
>> #define PIT_IRQ 65
>>
>>     virq = irq_create_mapping(NULL, PIT_IRQ);
>>     set_irq_type(virq, IRQ_TYPE_LEVEL_LOW);
>>
>>     if(request_irq(virq, (irq_handler_t)timerEvent, 0, "timer2", (void *)0)) {
>>         printk(KERN_ERR "request_irq() returned error for irq=%d virq=%d\n", PIT_IRQ, virq);
>>     }
>
> It is some time ago, but when I did something similar I needed the
> following patch in order to use NULL for irq_create_mapping(). Have a
> try, and if it is still needed (as it looks from a glimpse), then maybe
> we should get it merged?
>
> ===
>
> From: Wolfram Sang <w.sang@pengutronix.de>
> Subject: [PATCH] powerpc/cpm2: make cpm2_pic the default host
>
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> ---
>  arch/powerpc/sysdev/cpm2_pic.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/cpm2_pic.c b/arch/powerpc/sysdev/cpm2_pic.c
> index 78f1f7c..7a7d4e5 100644
> --- a/arch/powerpc/sysdev/cpm2_pic.c
> +++ b/arch/powerpc/sysdev/cpm2_pic.c
> @@ -272,4 +272,5 @@ void cpm2_pic_init(struct device_node *node)
>                printk(KERN_ERR "CPM2 PIC: failed to allocate irq host!\n");
>                return;
>        }
> +       irq_set_default_host(cpm2_pic_host);
>  }

Thanks guys.

I can confirm that Wolfram's patch above is required to get Norbet's
suggestion to work. Without the patch, I still get the original
symptoms. I have tested this on 2.6.27.19.

Daniel

^ permalink raw reply

* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by  read i2c_msg
From: Esben Haabendal @ 2009-05-28 21:22 UTC (permalink / raw)
  To: Peter Korsgaard; +Cc: linuxppc-dev, linux-i2c
In-Reply-To: <87octd5415.fsf@macbook.be.48ers.dk>

Peter Korsgaard wrote:
>>>>>> "Esben" == Esben Haabendal <esbenhaabendal@gmail.com> writes:
>>>>>>             
>
> Hi,
>
>  Esben> It's strange, that line looks perfectly fine when I check the
>  Esben> mail in my GMail inbox and the outbox from the account I sent
>  Esben> it from.
>
> Well, it is here and in the archive:
> http://ozlabs.org/pipermail/linuxppc-dev/2009-May/072274.html
>   
If you look at
http://www.mail-archive.com/linux-i2c@vger.kernel.org/msg01050.html
instead, the patch is not broken.  I find it more likely that the ozlabs.org
archive is breaking my lines, than mail-archive.com putting broken lines
together again.
> Please consider using git send-email for patches.
>   
I used git imap-send and thunderbird to do it.

I will consider git send-email in the future, although I don't think
my original e-mail were broken ;-)

But

^ permalink raw reply

* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg
From: Peter Korsgaard @ 2009-05-28 21:08 UTC (permalink / raw)
  To: Esben Haabendal; +Cc: Esben Haabendal, linuxppc-dev, linux-i2c, Ben Dooks
In-Reply-To: <d2b9ea600905281341s24efa4e3l6277a8aa5dec5bd@mail.gmail.com>

>>>>> "Esben" == Esben Haabendal <esbenhaabendal@gmail.com> writes:

Hi,

 Esben> It's strange, that line looks perfectly fine when I check the
 Esben> mail in my GMail inbox and the outbox from the account I sent
 Esben> it from.

Well, it is here and in the archive:
http://ozlabs.org/pipermail/linuxppc-dev/2009-May/072274.html

Please consider using git send-email for patches.

-- 
Bye, Peter Korsgaard

^ permalink raw reply

* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg
From: Esben Haabendal @ 2009-05-28 20:41 UTC (permalink / raw)
  To: Peter Korsgaard; +Cc: Esben Haabendal, linuxppc-dev, linux-i2c, Ben Dooks
In-Reply-To: <87ws8155md.fsf@macbook.be.48ers.dk>

> =A0Esben> I've checked both my copy in my "Sent" folder and the copy
> =A0Esben> received from the list, and I cannot see any "line break"
> =A0Esben> breakage of the patch.
>
> I guess Wolfram referred to the context line which was clearly word wrapp=
ed:
>
> @@ -456,17 +456,22 @@ static int mpc_xfer(struct i2c_adapter *adap,
> struct i2c_msg *msgs, int num)
>
> The other lines look fine.

It's strange, that line looks perfectly fine when I check the mail in my GM=
ail
inbox and the outbox from the account I sent it from.

/Esben
--=20
Esben Haabendal, Senior Software Consultant
Dor=E9Development ApS, Ved Stranden 1, 9560 Hadsund, DK-Denmark
Phone: +45 51 92 53 93, E-mail: eha@doredevelopment.dk
WWW: http://www.doredevelopment.dk

^ permalink raw reply

* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg
From: Peter Korsgaard @ 2009-05-28 20:34 UTC (permalink / raw)
  To: Esben Haabendal; +Cc: Esben Haabendal, linuxppc-dev, linux-i2c, Ben Dooks
In-Reply-To: <d2b9ea600905281310p16f47ed0i454e25bf256d3010@mail.gmail.com>

>>>>> "Esben" == Esben Haabendal <esbenhaabendal@gmail.com> writes:

Hi,

 >> I wanted to test it, but it does not apply due to line breaks (check
 >> @@-line). Also, I don't really have the time to dig into the topic, so I
 >> would only test it and give a tested-by-tag if it doesn't break anything
 >> here. I think Joakim would be a good candidate for an acked-by .

 Esben> I've checked both my copy in my "Sent" folder and the copy
 Esben> received from the list, and I cannot see any "line break"
 Esben> breakage of the patch.

I guess Wolfram referred to the context line which was clearly word wrapped:

@@ -456,17 +456,22 @@ static int mpc_xfer(struct i2c_adapter *adap,
struct i2c_msg *msgs, int num)

The other lines look fine.

-- 
Bye, Peter Korsgaard

^ permalink raw reply

* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg
From: Esben Haabendal @ 2009-05-28 20:15 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev, linux-i2c
In-Reply-To: <fa686aa40905281231o26c74a13v250bcedbd066e77b@mail.gmail.com>

On Thu, May 28, 2009 at 9:31 PM, Grant Likely <grant.likely@secretlab.ca> w=
rote:
> On Tue, May 26, 2009 at 5:30 AM, Esben Haabendal wrote:
>> On Tue, May 19, 2009 at 7:22 AM, Esben Haabendal wrote:
>>> This fixes MAL (arbitration lost) bug caused by illegal use of
>>> RSTA (repeated START) after STOP condition generated after last byte
>>> of reads. With this patch, it is possible to do an i2c_transfer() with
>>> additional i2c_msg's following the I2C_M_RD messages.
>>>
>>> It still needs to be resolved if it is possible to fix this issue
>>> by removing the STOP condition after reads in a robust way.
>>>
>>> Signed-off-by: Esben Haabendal <eha@doredevelopment.dk>
>>> ---
>>> =A0drivers/i2c/busses/i2c-mpc.c | =A0 =A09 +++++++--
>>> =A01 files changed, 7 insertions(+), 2 deletions(-)
>>
>> Any blockers to get this accepted?
>
> It helps if you cc: developers/maintainers of the device. =A0ie. Kumar
> for mpc8xxx, me for 52xx.
>
> This is the first time I noticed your posting. =A0It will take me a few
> days before I get a chance to review it.

Kumar, will you take a look at this patch?

/Esben

^ permalink raw reply

* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg
From: Esben Haabendal @ 2009-05-28 20:10 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linuxppc-dev, Esben Haabendal, linux-i2c, Ben Dooks
In-Reply-To: <20090528171726.GE3112@pengutronix.de>

On Thu, May 28, 2009 at 7:17 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:
>> > Any blockers to get this accepted?
>>
>> It would be nice to get an ack from someone who can actually test
>> the driver before getting this merged.
>
> I wanted to test it, but it does not apply due to line breaks (check
> @@-line). Also, I don't really have the time to dig into the topic, so I
> would only test it and give a tested-by-tag if it doesn't break anything
> here. I think Joakim would be a good candidate for an acked-by .

I've checked both my copy in my "Sent" folder and the copy received
from the list,
and I cannot see any "line break" breakage of the patch.

It must be your mail client that is messing with it, so I cannot really do it
much better in that way.

If necessary, I can push a branch to a public repo you can pull from.

/Esben

^ permalink raw reply

* Re: 8544 external interrupt configuration problems
From: Scott Wood @ 2009-05-28 20:02 UTC (permalink / raw)
  To: Nancy Isaac; +Cc: linuxppc-dev
In-Reply-To: <1bcc666d0905281205p63ee8119td5578a749bce8377@mail.gmail.com>

On Thu, May 28, 2009 at 12:05:52PM -0700, Nancy Isaac wrote:
> My device tree has the following entry for my fpga:
> 
> CpuCpld\@f0000000{
>                         compatible = "MPC8544DS";
>                         device_type = "CpuCpld";
>                         reg = <f0000000 00000040>;
>                         interrupts = <41 2 42 2 43 2>;
>                         interrupt-parent = <&mpic>;
> };

Where did you get those interrupt numbers from?  External interrupt
numbers start at zero on MPIC.

Is the level/sense information correct?

> My driver does the mapping to the virq:
> 
>  np = of_find_node_by_type(NULL, "CpuCpld");
>     if (!np) {
>         ret = -ENODEV;
>     }
>     cpldCpuDrv->MateIntIrq = irq_of_parse_and_map(np, 0);
>     cpldCpuDrv->FtaIrq =  irq_of_parse_and_map(np, 1);
>     cpldCpuDrv->ExtractIrq =  irq_of_parse_and_map(np, 2);
>     cpldCpuDrv->XauiIrq =  irq_of_parse_and_map(np, 0);
> 
>     of_node_put(np);

Looks good (other than that you should be using compatible (with a more
specific name) rather than device_type).

> Does anyone know what the virq should be for these external interrupts?

They're dynamically assigned.

> I've tried specifying the actual irq numbers 1,2 and 3 and that doesn't work
> either.

Specifying them where?  In the device tree or as virq numbers?  Never
hard-code virq numbers.

> Is there some PCI configuration that's getting in the way?  I've tried
> disabling the FSL_PCIE and I get the same behavior.

Barring an unusual bug, PCI should have nothing to do with the interrupt
routing of things that aren't on the PCI bus.

-Scott

^ permalink raw reply

* Re: [PATCH] powerpc: tiny memcpy_(to|from)io optimisation
From: Albrecht Dreß @ 2009-05-28 19:50 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <OF508468F5.9BFFC168-ONC12575C4.005846FD-C12575C4.00592585@transmode.se>

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

Am 28.05.09 18:13 schrieb(en) Joakim Tjernlund:
> hmm, these do look a bit unoptimal anyway. Any reason not to write  
> them something like below(written by me for uClibc long time ago).  
> You will have to add eieio()/sync

No (and I wasn't aware of the PPC pre-inc vs. post-inc stuff) - I just  
stumbled over this while fixing mtd accesses to the MPC5200's Local Bus  
in 16-bit mode which doesn't allow byte accesses.  And I didn't want to  
go too deep into this as the real fix for me is actually somewhat  
different...

> /* PPC can do pre increment and load/store, but not post increment  
> and load/store.
>    Therefore use *++ptr instead of *ptr++. */
[snip]
>  copy_chunks:
> 	do {
> 		/* make gcc to load all data, then store it */
> 		tmp1 = *(unsigned long *)(tmp_from+4);
> 		tmp_from += 8;
> 		tmp2 = *(unsigned long *)tmp_from;
> 		*(unsigned long *)(tmp_to+4) = tmp1;
> 		tmp_to += 8;
> 		*(unsigned long *)tmp_to = tmp2;
> 	} while (--chunks);

Is this the same for all PPC cores, i.e. do they all benefit from  
loading/storing 8 instead of 4 bytes?

Best, Albrecht.

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

^ permalink raw reply

* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg
From: Grant Likely @ 2009-05-28 19:31 UTC (permalink / raw)
  To: Esben Haabendal; +Cc: linuxppc-dev, linux-i2c, Esben Haabendal
In-Reply-To: <d2b9ea600905260430i72290901p9266de4cade049c1@mail.gmail.com>

On Tue, May 26, 2009 at 5:30 AM, Esben Haabendal
<esbenhaabendal@gmail.com> wrote:
> On Tue, May 19, 2009 at 7:22 AM, Esben Haabendal <eha@doredevelopment.dk>=
 wrote:
>> This fixes MAL (arbitration lost) bug caused by illegal use of
>> RSTA (repeated START) after STOP condition generated after last byte
>> of reads. With this patch, it is possible to do an i2c_transfer() with
>> additional i2c_msg's following the I2C_M_RD messages.
>>
>> It still needs to be resolved if it is possible to fix this issue
>> by removing the STOP condition after reads in a robust way.
>>
>> Signed-off-by: Esben Haabendal <eha@doredevelopment.dk>
>> ---
>> =A0drivers/i2c/busses/i2c-mpc.c | =A0 =A09 +++++++--
>> =A01 files changed, 7 insertions(+), 2 deletions(-)
>
> Any blockers to get this accepted?

It helps if you cc: developers/maintainers of the device.  ie. Kumar
for mpc8xxx, me for 52xx.

This is the first time I noticed your posting.  It will take me a few
days before I get a chance to review it.

g.

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

^ permalink raw reply

* [PATCH] fsldma: do not clear bandwidth control bits on the 83xx controller
From: Ira Snyder @ 2009-05-28 19:26 UTC (permalink / raw)
  To: linuxppc-dev, Li Yang, Dan Williams

The 83xx controller does not support the external pause feature. The bit
in the mode register that controls external pause on the 85xx controller
happens to be part of the bandwidth control settings for the 83xx
controller.

This patch fixes the driver so that it only clears the external pause bit
if the hardware is the 85xx controller. When driving the 83xx controller,
the bit is left untouched. This follows the existing convention that mode
registers settings are not touched unless necessary.

Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
---

There is currently no interface to change the bandwidth control setting.
I have changed the bandwidth control setting by requesting exclusive
access to the DMA channel (via dma_request_channel()) then, since I know
the channel is idle, I just add the value I need.

I have tested this using the DMA_SLAVE patch I posted last week, and
verified that the bandwidth control bits do not change on the 83xx
controller.

The conditional could be changed from "== 85XX" to "!= 83XX", as they
are equivalent in the current driver. I'm not sure which is better.

 drivers/dma/fsldma.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index b2fd35e..a662aac 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -148,10 +148,11 @@ static void dma_start(struct fsl_dma_chan *fsl_chan)
 	if (fsl_chan->feature & FSL_DMA_CHAN_PAUSE_EXT) {
 		DMA_OUT(fsl_chan, &fsl_chan->reg_base->bcr, 0, 32);
 		mr_set |= FSL_DMA_MR_EMP_EN;
-	} else
+	} else if ((fsl_chan->feature & FSL_DMA_IP_MASK) == FSL_DMA_IP_85XX) {
 		DMA_OUT(fsl_chan, &fsl_chan->reg_base->mr,
 			DMA_IN(fsl_chan, &fsl_chan->reg_base->mr, 32)
 				& ~FSL_DMA_MR_EMP_EN, 32);
+	}
 
 	if (fsl_chan->feature & FSL_DMA_CHAN_START_EXT)
 		mr_set |= FSL_DMA_MR_EMS_EN;
-- 
1.5.4.3

^ permalink raw reply related

* [PATCH] fsldma: enable external start for the 83xx controller
From: Ira Snyder @ 2009-05-28 19:20 UTC (permalink / raw)
  To: linuxppc-dev, Li Yang, Dan Williams

The 83xx controller has external start capability, but lacks external pause
capability. Hook up the external start function pointer for the 83xx
controller.

Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
---

Though there is no way to enable external start in the mainline driver,
the DMA_SLAVE patch I posted last week does give the ability to enable
external start.

I have tested this patch along with the DMA_SLAVE patch using a driver
for some custom hardware that uses the external DMA control lines.

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

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index db07113..b2fd35e 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1099,9 +1099,9 @@ static int __devinit fsl_dma_chan_probe(struct fsl_dma_device *fdev,
 
 	switch (new_fsl_chan->feature & FSL_DMA_IP_MASK) {
 	case FSL_DMA_IP_85XX:
-		new_fsl_chan->toggle_ext_start = fsl_chan_toggle_ext_start;
 		new_fsl_chan->toggle_ext_pause = fsl_chan_toggle_ext_pause;
 	case FSL_DMA_IP_83XX:
+		new_fsl_chan->toggle_ext_start = fsl_chan_toggle_ext_start;
 		new_fsl_chan->set_src_loop_size = fsl_chan_set_src_loop_size;
 		new_fsl_chan->set_dest_loop_size = fsl_chan_set_dest_loop_size;
 	}
-- 
1.5.4.3

^ permalink raw reply related

* 8544 external interrupt configuration problems
From: Nancy Isaac @ 2009-05-28 19:05 UTC (permalink / raw)
  To: linuxppc-dev

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

Hi,

I have a custom board that's using powerpc 8544. I am trying to setup the
external interrupts using the device tree and I am having no luck getting
the interrupt.  I think my problem is that I don't have the hwirq to virq
mapping correctly. We are using the freescale 8544ds as our base.  We have a
PCI bus but we are not using PCIE.  In my kernel configuration, I have
CONFIG_PCIEPORTBUS disabled.  The PCI interrupt is connected to IRQ0 and it
is working fine.  But, my FPGA has three interrupts, IRQ1, IRQ2 and IRQ3.  I
configure this in the device tree and I've tried many different numbers for
the interrupt.

In the KConfig for ppc85xx, the configuration is the following

config MPC85xx_DS
    bool "Freescale MPC8544 DS"
    #select PPC_I8259
    select DEFAULT_UIMAGE
    select FSL_PCIE

config MPC85xx
    bool
    select PPC_UDBG_16550
    select PPC_INDIRECT_PCI
    select FSL_PCIE
    select SERIAL_8250_SHARE_IRQ if SERIAL_8250
    default y if MPC8540_ADS || MPC85xx_CDS || MPC8560_ADS \
        || MPC85xx_MDS || MPC85xx_DS

My .config has the following configuration for PCI ( I am showing the PCI
configuration b/c 8544 seems to have different interrupt setup when PCIE is
configured)

CONFIG_ZONE_DMA=y
# CONFIG_MPIC_WEIRD is not set
# CONFIG_PPC_I8259 is not set
CONFIG_PPC_INDIRECT_PCI=y
CONFIG_FSL_SOC=y
CONFIG_FSL_PCIE=y
CONFIG_PCI=y
CONFIG_PCI_DOMAINS=y
# CONFIG_PCIEPORTBUS is not set
CONFIG_PCI_DEBUG=y

My device tree has the following entry for my fpga:

CpuCpld\@f0000000{
                        compatible = "MPC8544DS";
                        device_type = "CpuCpld";
                        reg = <f0000000 00000040>;
                        interrupts = <41 2 42 2 43 2>;
                        interrupt-parent = <&mpic>;
};

My driver does the mapping to the virq:

 np = of_find_node_by_type(NULL, "CpuCpld");
    if (!np) {
        ret = -ENODEV;
    }
    cpldCpuDrv->MateIntIrq = irq_of_parse_and_map(np, 0);
    cpldCpuDrv->FtaIrq =  irq_of_parse_and_map(np, 1);
    cpldCpuDrv->ExtractIrq =  irq_of_parse_and_map(np, 2);
    cpldCpuDrv->XauiIrq =  irq_of_parse_and_map(np, 0);

    of_node_put(np);

Does anyone know what the virq should be for these external interrupts?
I've tried specifying the actual irq numbers 1,2 and 3 and that doesn't work
either.
Is there some PCI configuration that's getting in the way?  I've tried
disabling the FSL_PCIE and I get the same behavior.

Any help or pointers will be appreciated.

Thanks
Nancy

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

^ permalink raw reply

* Re: [PATCH V3 4/4] Fabric bindings for STAC9766 on the Efika
From: Jon Smirl @ 2009-05-28 18:58 UTC (permalink / raw)
  To: Peter Korsgaard; +Cc: linuxppc-dev, alsa-devel, broonie
In-Reply-To: <87eiu972ff.fsf@macbook.be.48ers.dk>

On Thu, May 28, 2009 at 10:00 AM, Peter Korsgaard <jacmet@sunsite.dk> wrote=
:
>>>>>> "Jon" =3D=3D Jon Smirl <jonsmirl@gmail.com> writes:
>
> Hi,
>
> =A0Jon> Fabric bindings for STAC9766 AC97 codec on the Efika.
> =A0Jon> Signed-off-by: Jon Smirl <jonsmirl@gmail.com>
> =A0Jon> ---
> =A0Jon> =A0sound/soc/fsl/Kconfig =A0 =A0 =A0 =A0 =A0 =A0 =A0| =A0 =A08 ++=
+
> =A0Jon> =A0sound/soc/fsl/Makefile =A0 =A0 =A0 =A0 =A0 =A0 | =A0 =A01
> =A0Jon> =A0sound/soc/fsl/efika-audio-fabric.c | =A0 95 ++++++++++++++++++=
++++++++++++++++++
> =A0Jon> =A03 files changed, 104 insertions(+), 0 deletions(-)
> =A0Jon> =A0create mode 100644 sound/soc/fsl/efika-audio-fabric.c
>
> =A0Jon> diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
> =A0Jon> index 79579ae..f571c6e 100644
> =A0Jon> --- a/sound/soc/fsl/Kconfig
> =A0Jon> +++ b/sound/soc/fsl/Kconfig
> =A0Jon> @@ -47,3 +47,11 @@ config SND_MPC52xx_SOC_PCM030
> =A0Jon> =A0 help
> =A0Jon> =A0 =A0 Say Y if you want to add support for sound on the Phytec =
pcm030 baseboard.
>
> =A0Jon> +config SND_MPC52xx_SOC_EFIKA
> =A0Jon> + tristate "SoC AC97 Audio support for bbplan Efika and STAC9766"
> =A0Jon> + depends on PPC_EFIKA
> =A0Jon> + select SND_SOC_MPC5200_AC97
> =A0Jon> + select SND_SOC_STAC9766
> =A0Jon> + help
> =A0Jon> + =A0 Say Y if you want to add support for sound on the Efika.
> =A0Jon> +
>
> Wouldn't it make more sense to make this default y when it has such
> specific dependencies and is so deep down in the tree - Most efika
> users probably want to enable this if they have enabled ALSA and
> SND_SOC?

I can change it in the next round of updates. Right now I'd like to
get some people testing it and maybe doing some patches to enhance it.

For example I know he code has a problem where about 5% of the time
the codec doesn't reset right on boot. Someone needs to put an
oscilloscope on the codec chip and tell me why it is failing.

>
> --
> Bye, Peter Korsgaard
>



--=20
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Kernel bug in 2.6.23...was: RE: How to debug a hung multi-core system....
From: Morrison, Tom @ 2009-05-28 18:46 UTC (permalink / raw)
  To: Morrison, Tom, Kumar Gala
  Cc: Geary Sean-R60898, linuxppc-dev, Young, Andrew, Brown, Jeff
In-Reply-To: <BD261180E6D35F4D9D32F3E44FD3D90110F53C57@EMPBEDEX.empirix.com>

Kumar,

To follow up on our postings from late last week...
(which I was expecting a response (but never got) from you)...

-----

We (well, mostly a very bright engineer who was very persistent)=20
have(has) found the origin of how the kernel TLB got corrupted.

We tracked down the problem to a programming bug in the DataStorage
exception handler for our kernel (2.6.23). We have looked at newer
kernels, and have noticed that this piece of processing has changed,=20
but let me explain to you what happened (and the conditions that=20
caused the problem on our MPC8572E (running SMP)...

If you follow the logic of in this version of the kernel, it reads=20
the SPRN_DEAR into register R10, and then does some operations=20
(including a tlbsx operation (which uses R10)), and then attempts
to update the associated PTE entry.

Well, if you have REALLY bad luck, sometime between the time you=20
took this exception and try to update the PTE for this page, the=20
other core has decided to invalidate this page's PTE. The good=20
part is the kernel recognizes this unlucky case.

Unfortunately, in this 'bad luck' case, a kernel bug was=20
Introduced. The kernel uses R10 for some processing (puts
the physical address associated with this virtual page) and=20
then branches up 'above' the tlbsx operation to try again=20

...without restoring R10 to the SPRN_DEAR required by the tlbsx
operation...

This means, that even though the kernel recognized this exceptional
problem, it NEVER did the right thing, and instead, the kernel would=20
(attempt) to modify the unlucky TLB virtual address that corresponds=20
to the physical address of the original DataStorage exception.

The only way we caught this is that we also had a second piece of=20
'bad luck' by having that physical address map to the virtual address
of the kernel (0xC0000000), and thus, when it loops back to try again,
it gets the kernel page(s) from the tlbsx operation, and modifies=20
permissions on the kernel pages and thus causing an InstructionStore=20
Exception (forever).

We fixed this in our kernel by just restoring R10 to SPRN_DEAR value
just before it loops back, something like this:

=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D
              ....
	mtspr	SPRN_MAS1, r13
	tlbwe

	/* because we did NOT find in PTE */
	/* r10 was changed - so we need   */
	/* to re-load it here to work     */
	mfspr	r10, SPRN_DEAR   	  /* restore the faulting
address */
	b	5b		/* Try again */
             ....
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D

That's the short and long of it...and 4 weeks of very stressful
problems...

I am wondering why nobody has found this problem before - are we the
first to be this unlucky? I am not sure that is a good thing!

Comments? Suggestions? What else should I be doing with this
information?

Tom Morrison
Principal Software Engineer
EMPIRIX=20
20 Crosby Drive - Bedford, MA  01730
p: 781.266.3567 f: 781.266.3670=20
email: tmorrison@empirix.com=20
www.empirix.com


>> -----Original Message-----
>> From: Morrison, Tom
>> Sent: Thursday, May 21, 2009 11:24 AM
>> To: Morrison, Tom; Kumar Gala
>> Cc: linuxppc-dev@ozlabs.org; Young, Andrew; Brown, Jeff; Geary Sean-
>> R60898
>> Subject: RE: How to debug a hung multi-core system....
>>=20
>> Just had a little conference with several co-workers...to go over
results
>>=20
>> We think that LT0 (the one that maps the kernel) has been corrupted:
>>=20
>>        Entry  EPN          RPN    TID  TMASK   WIMGE  TSIZ U0:3  X0:1
>>
---------------------------------------------------------------
>>        LT0  C0000000     00000000 00     0FF     04     9     0     0
>>=20
>>        PID  TS  PROT SHEN   UR   UW   UX   SR   SW   SX  TIDZ VAL
>>
---------------------------------------------------------------
>>        0    0    P    P    E    E    D    E    E    D    D    V
>>=20
>> Is absolutely wrong - this is TLB for the kernel - and as you can see
>> ...it does NOT have execution privileges (and in fact the user space
>> HAS executive privileges for this area (complete opposite of what it
>> should be)...
>>=20
>> This is why it is stuck AT that instruction (can't even single step
>> from that location)..
>>=20
>> (one of) The first problem(s) is how can/when did this TLB get
corrupted!
>>=20
>> Tom

^ permalink raw reply

* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg
From: Joakim Tjernlund @ 2009-05-28 18:43 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linuxppc-dev, Esben Haabendal, Esben Haabendal, Ben Dooks,
	linux-i2c
In-Reply-To: <20090528171726.GE3112@pengutronix.de>

Wolfram Sang <w.sang@pengutronix.de> wrote on 28/05/2009 19:17:26:
>
> > > Any blockers to get this accepted?
> >
> > It would be nice to get an ack from someone who can actually test
> > the driver before getting this merged.
>
> I wanted to test it, but it does not apply due to line breaks (check
> @@-line). Also, I don't really have the time to dig into the topic, so I
> would only test it and give a tested-by-tag if it doesn't break anything
> here. I think Joakim would be a good candidate for an acked-by .

It sure looks OK, even at a closer look :)
Acked-by:  Joakim Tjernlund <Joakim.Tjernlund@transmode.se>

^ permalink raw reply

* Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg
From: Wolfram Sang @ 2009-05-28 17:17 UTC (permalink / raw)
  To: Ben Dooks; +Cc: linuxppc-dev, Esben Haabendal, linux-i2c, Esben Haabendal
In-Reply-To: <20090526213351.GG23114@fluff.org.uk>

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

> > Any blockers to get this accepted?
> 
> It would be nice to get an ack from someone who can actually test
> the driver before getting this merged.

I wanted to test it, but it does not apply due to line breaks (check
@@-line). Also, I don't really have the time to dig into the topic, so I
would only test it and give a tested-by-tag if it doesn't break anything
here. I think Joakim would be a good candidate for an acked-by .

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

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

^ permalink raw reply

* Re: [PATCH] powerpc: tiny memcpy_(to|from)io optimisation
From: Joakim Tjernlund @ 2009-05-28 16:13 UTC (permalink / raw)
  To: Albrecht Dreß; +Cc: Linux PPC Development
In-Reply-To: <1243454441.3489.1@antares>


>
> This trivial patch changes memcpy_(to|from)io as to transfer as many
> 32-bit words as possible in 32-bit accesses (in the current solution,=

> the last 32-bit word was transferred as 4 byte accesses).
>
> Signed-off-by: Albrecht Dre=DF <albrecht.dress@arcor.de>
> ---
>
> diff -urpN -X linux-2.6.29.1.orig/Documentation/dontdiff
> linux-2.6.29.1.orig/arch/powerpc/kernel/io.c
> linux-2.6.29.1/arch/powerpc/kernel/io.c
> --- linux-2.6.29.1.orig/arch/powerpc/kernel/io.c   2009-04-02
> 22:55:27.000000000 +0200
> +++ linux-2.6.29.1/arch/powerpc/kernel/io.c   2009-05-27
> 11:36:09.000000000 +0200
> @@ -161,7 +161,7 @@ void _memcpy_fromio(void *dest, const vo
>         dest++;
>         n--;
>      }
> -   while(n > 4) {
> +   while(n >=3D 4) {
>         *((u32 *)dest) =3D *((volatile u32 *)vsrc);
>         eieio();
>         vsrc +=3D 4;
> @@ -190,7 +190,7 @@ void _memcpy_toio(volatile void __iomem
>         vdest++;
>         n--;
>      }
> -   while(n > 4) {
> +   while(n >=3D 4) {
>         *((volatile u32 *)vdest) =3D *((volatile u32 *)src);
>         src +=3D 4;
>         vdest +=3D 4;

hmm, these do look a bit unoptimal anyway. Any reason not to write
them something like below(written by me for uClibc long time ago). You =
will
have to add eieio()/sync

void *memcpy(void *to, const void *from, size_t n)
/* PPC can do pre increment and load/store, but not post increment and =
load/store.
   Therefore use *++ptr instead of *ptr++. */
{
	unsigned long rem, chunks, tmp1, tmp2;
	unsigned char *tmp_to;
	unsigned char *tmp_from =3D (unsigned char *)from;

	chunks =3D n / 8;
	tmp_from -=3D 4;
	tmp_to =3D to - 4;
	if (!chunks)
		goto lessthan8;
	rem =3D (unsigned long )tmp_to % 4;
	if (rem)
		goto align;
 copy_chunks:
	do {
		/* make gcc to load all data, then store it */
		tmp1 =3D *(unsigned long *)(tmp_from+4);
		tmp_from +=3D 8;
		tmp2 =3D *(unsigned long *)tmp_from;
		*(unsigned long *)(tmp_to+4) =3D tmp1;
		tmp_to +=3D 8;
		*(unsigned long *)tmp_to =3D tmp2;
	} while (--chunks);
 lessthan8:
	n =3D n % 8;
	if (n >=3D 4) {
		*(unsigned long *)(tmp_to+4) =3D *(unsigned long *)(tmp_from+4);
		tmp_from +=3D 4;
		tmp_to +=3D 4;
		n =3D n-4;
	}
	if (!n ) return to;
	tmp_from +=3D 3;
	tmp_to +=3D 3;
	do {
		*++tmp_to =3D *++tmp_from;
	} while (--n);

	return to;
 align:
	rem =3D 4 - rem;
	n =3D n - rem;
	do {
		*(tmp_to+4) =3D *(tmp_from+4);
		++tmp_from;
		++tmp_to;
	} while (--rem);
	chunks =3D n / 8;
	if (chunks)
		goto copy_chunks;
	goto lessthan8;
}=

^ permalink raw reply

* Re: [PATCH V3 4/4] Fabric bindings for STAC9766 on the Efika
From: Peter Korsgaard @ 2009-05-28 14:00 UTC (permalink / raw)
  To: Jon Smirl; +Cc: linuxppc-dev, alsa-devel, broonie
In-Reply-To: <20090525013853.3073.78627.stgit@terra>

>>>>> "Jon" == Jon Smirl <jonsmirl@gmail.com> writes:

Hi,

 Jon> Fabric bindings for STAC9766 AC97 codec on the Efika.
 Jon> Signed-off-by: Jon Smirl <jonsmirl@gmail.com>
 Jon> ---
 Jon>  sound/soc/fsl/Kconfig              |    8 +++
 Jon>  sound/soc/fsl/Makefile             |    1 
 Jon>  sound/soc/fsl/efika-audio-fabric.c |   95 ++++++++++++++++++++++++++++++++++++
 Jon>  3 files changed, 104 insertions(+), 0 deletions(-)
 Jon>  create mode 100644 sound/soc/fsl/efika-audio-fabric.c

 Jon> diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
 Jon> index 79579ae..f571c6e 100644
 Jon> --- a/sound/soc/fsl/Kconfig
 Jon> +++ b/sound/soc/fsl/Kconfig
 Jon> @@ -47,3 +47,11 @@ config SND_MPC52xx_SOC_PCM030
 Jon>  	help
 Jon>  	  Say Y if you want to add support for sound on the Phytec pcm030 baseboard.
 
 Jon> +config SND_MPC52xx_SOC_EFIKA
 Jon> +	tristate "SoC AC97 Audio support for bbplan Efika and STAC9766"
 Jon> +	depends on PPC_EFIKA
 Jon> +	select SND_SOC_MPC5200_AC97
 Jon> +	select SND_SOC_STAC9766
 Jon> +	help
 Jon> +	  Say Y if you want to add support for sound on the Efika.
 Jon> +

Wouldn't it make more sense to make this default y when it has such
specific dependencies and is so deep down in the tree - Most efika
users probably want to enable this if they have enabled ALSA and
SND_SOC?

-- 
Bye, Peter Korsgaard

^ permalink raw reply


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