* [RFC Patch 6/6] Adapt kexec and samples code to recognise PPC64 hardware breakpoint usage
From: K.Prasad @ 2009-05-14 13:46 UTC (permalink / raw)
To: linuxppc-dev
Cc: Michael Neuling, Benjamin Herrenschmidt, Alan Stern, paulus,
K.Prasad, Roland McGrath
In-Reply-To: <20090514133312.360702378@prasadkr_t60p.in.ibm.com>
Modify kexec code to disable DABR registers before a reboot. Adapt the samples
code to populate PPC64-arch specific fields.
Signed-off-by: K.Prasad <prasad@linux.vnet.ibm.com>
---
arch/powerpc/kernel/machine_kexec_64.c | 6
samples/hw_breakpoint/data_breakpoint.c | 4
Index: linux-2.6-tip.hbkpt/arch/powerpc/kernel/machine_kexec_64.c
===================================================================
--- linux-2.6-tip.hbkpt.orig/arch/powerpc/kernel/machine_kexec_64.c 2009-05-14 00:17:24.000000000 +0530
+++ linux-2.6-tip.hbkpt/arch/powerpc/kernel/machine_kexec_64.c 2009-05-14 09:48:09.000000000 +0530
@@ -24,6 +24,7 @@
#include <asm/sections.h> /* _end */
#include <asm/prom.h>
#include <asm/smp.h>
+#include <asm/hw_breakpoint.h>
int default_machine_kexec_prepare(struct kimage *image)
{
@@ -214,6 +215,9 @@
put_cpu();
local_irq_disable();
+#ifdef CONFIG_PPC64
+ hw_breakpoint_disable();
+#endif
}
#else /* ! SMP */
@@ -233,6 +237,9 @@
if (ppc_md.kexec_cpu_down)
ppc_md.kexec_cpu_down(0, 0);
local_irq_disable();
+#ifdef CONFIG_PPC64
+ hw_breakpoint_disable();
+#endif
}
#endif /* SMP */
Index: linux-2.6-tip.hbkpt/samples/hw_breakpoint/data_breakpoint.c
===================================================================
--- linux-2.6-tip.hbkpt.orig/samples/hw_breakpoint/data_breakpoint.c 2009-05-14 00:17:24.000000000 +0530
+++ linux-2.6-tip.hbkpt/samples/hw_breakpoint/data_breakpoint.c 2009-05-14 00:58:06.000000000 +0530
@@ -54,6 +54,10 @@
sample_hbp.info.type = HW_BREAKPOINT_WRITE;
sample_hbp.info.len = HW_BREAKPOINT_LEN_4;
#endif /* CONFIG_X86 */
+#ifdef CONFIG_PPC64
+ sample_hbp.info.name = ksym_name;
+ sample_hbp.info.type = DABR_DATA_WRITE;
+#endif /* CONFIG_PPC64 */
sample_hbp.triggered = (void *)sample_hbp_handler;
^ permalink raw reply
* Re: [RFC Patch 6/6] Adapt kexec and samples code to recognise PPC64 hardware breakpoint usage
From: Geert Uytterhoeven @ 2009-05-14 13:59 UTC (permalink / raw)
To: K.Prasad
Cc: Michael Neuling, Benjamin Herrenschmidt, linuxppc-dev, Alan Stern,
paulus, Roland McGrath
In-Reply-To: <20090514134614.GG14229@in.ibm.com>
On Thu, 14 May 2009, K.Prasad wrote:
> Modify kexec code to disable DABR registers before a reboot. Adapt the samples
> code to populate PPC64-arch specific fields.
>
> Signed-off-by: K.Prasad <prasad@linux.vnet.ibm.com>
> ---
> arch/powerpc/kernel/machine_kexec_64.c | 6
> samples/hw_breakpoint/data_breakpoint.c | 4
>
> Index: linux-2.6-tip.hbkpt/arch/powerpc/kernel/machine_kexec_64.c
> ===================================================================
> --- linux-2.6-tip.hbkpt.orig/arch/powerpc/kernel/machine_kexec_64.c 2009-05-14 00:17:24.000000000 +0530
> +++ linux-2.6-tip.hbkpt/arch/powerpc/kernel/machine_kexec_64.c 2009-05-14 09:48:09.000000000 +0530
> @@ -24,6 +24,7 @@
> #include <asm/sections.h> /* _end */
> #include <asm/prom.h>
> #include <asm/smp.h>
> +#include <asm/hw_breakpoint.h>
>
> int default_machine_kexec_prepare(struct kimage *image)
> {
> @@ -214,6 +215,9 @@
> put_cpu();
>
> local_irq_disable();
> +#ifdef CONFIG_PPC64
^^^^^^^^^^^^^^^^^^^
> + hw_breakpoint_disable();
> +#endif
^^^^^^
What about providing a dummy definition of hw_breakpoint_disable()
in <asm/hw_breakpoint.h> if !CONFIG_PPC64?
and if you make it safe to always include <asm/hw_breakpoint.h>, you can
get rid of the #ifdef in e.g.
--- linux-2.6-tip.hbkpt.orig/arch/powerpc/kernel/ptrace.c
+++ linux-2.6-tip.hbkpt/arch/powerpc/kernel/ptrace.c
@@ -37,6 +37,9 @@
#include <asm/page.h>
#include <asm/pgtable.h>
#include <asm/system.h>
+#ifdef CONFIG_PPC64
+#include <asm/hw_breakpoint.h>
+#endif
With kind regards,
Geert Uytterhoeven
Software Architect
Techsoft Centre
Technology and Software Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010
^ permalink raw reply
* Re: [PATCH 02/12] fs_enet: Add MPC5121 FEC support.
From: Grant Likely @ 2009-05-14 14:00 UTC (permalink / raw)
To: Piotr Zięcik
Cc: Becky Bruce, Wolfgang Denk, Detlev Zundel, John Rigby, netdev,
linuxppc-dev, Scott Wood
In-Reply-To: <200905141438.23348.kosmo@semihalf.com>
2009/5/14 Piotr Zi=EAcik <kosmo@semihalf.com>:
> Thursday 07 May 2009 00:39:25 Grant Likely napisa=B3(a):
>> >> 512x are enabled in the same kernel?
>> >
>> > Hm... both architectures look sufficiently different to me that I
>> > don't see sense in trying such a thing. Do you think that needs to be
>> > supported?
>>
>> Yes! =A0:-) =A0It's not hard to do and it keeps the driver cleaner
>> (IMNSHO). =A0I don't think it is quite possible at the moment due to
>> cache coherency issues, but with Becky's recently merged dma ops
>> changes it should be fixable.
>
> Could you elaborate on the cache coherency issues in MPC5121
> FEC context? Especially how these issues are related to the driver
> binary compatibility.
>
> MPC5121 support was added to drivers/net/fs_enet. MPC52xx uses
> drivers/net/fec_mpc52xx.c. Do you think that creating one universal
> driver from these two is now possible? You said that it should be easy,
> however you also said that cache coherency issues makes this imposible.
Not impossible. Hard.
The mpc5200 is a cache coherent part. Bestcomm memory accesses are
noticed (snooped) by the cache, and it will flush out cache lines
appropriately to maintain coherency.
The mpc5121 bus design is non-coherent. The e300 core is essentially
the same as on the 5200, and the core can do snooping, but the
multiport memory controller on the 5121 doesn't support bus snooping
so the cache is not automatically maintained in a consistent state.
On this part Linux needs to manually flush the relevant cache lines
before initiating DMA transfers, and invalidiate them afterwards.
All of this doesn't actually affect the driver code at all. It's all
handled by the kernel and the DMA apis. What it does affect is
multiplatform kernels. The DMA behaviour is set at compile time, not
run time, depending on the setting of CONFIG_NON_COHERENT_CACHE (see
arch/powerpc/platforms/Kconfig.cputype). A kernel which has it on
won't run properly on a platform which has it off, and visa-versa.
So, while the MPC5200 and MPC5121 (and MPC83xx) all have the same
core, it isn't currently possible to build a single kernel which will
boot on both the MPC5200 and the MPC5121. The solution (i think) is
to kill CONFIG_NON_COHERENT_CACHE and have the platform setup code set
the appropriate dmaops at platform init time.
Cheers,
g.
>
> --
> Best Regards.
> Piotr Ziecik
>
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [RFC Patch 6/6] Adapt kexec and samples code to recognise PPC64 hardware breakpoint usage
From: Michael Ellerman @ 2009-05-14 14:11 UTC (permalink / raw)
To: K.Prasad
Cc: Michael Neuling, Benjamin Herrenschmidt, linuxppc-dev, Alan Stern,
Geert Uytterhoeven, paulus, K.Prasad, Roland McGrath
In-Reply-To: <alpine.LRH.2.00.0905141557260.2225@vixen.sonytel.be>
[-- Attachment #1: Type: text/plain, Size: 1518 bytes --]
On Thu, 2009-05-14 at 15:59 +0200, Geert Uytterhoeven wrote:
> On Thu, 14 May 2009, K.Prasad wrote:
> > Modify kexec code to disable DABR registers before a reboot. Adapt the samples
> > code to populate PPC64-arch specific fields.
> >
> > Signed-off-by: K.Prasad <prasad@linux.vnet.ibm.com>
> > ---
> > arch/powerpc/kernel/machine_kexec_64.c | 6
> > samples/hw_breakpoint/data_breakpoint.c | 4
> >
> > Index: linux-2.6-tip.hbkpt/arch/powerpc/kernel/machine_kexec_64.c
> > ===================================================================
> > --- linux-2.6-tip.hbkpt.orig/arch/powerpc/kernel/machine_kexec_64.c 2009-05-14 00:17:24.000000000 +0530
> > +++ linux-2.6-tip.hbkpt/arch/powerpc/kernel/machine_kexec_64.c 2009-05-14 09:48:09.000000000 +0530
> > @@ -24,6 +24,7 @@
> > #include <asm/sections.h> /* _end */
> > #include <asm/prom.h>
> > #include <asm/smp.h>
> > +#include <asm/hw_breakpoint.h>
> >
> > int default_machine_kexec_prepare(struct kimage *image)
> > {
> > @@ -214,6 +215,9 @@
> > put_cpu();
> >
> > local_irq_disable();
> > +#ifdef CONFIG_PPC64
> ^^^^^^^^^^^^^^^^^^^
> > + hw_breakpoint_disable();
> > +#endif
> ^^^^^^
>
> What about providing a dummy definition of hw_breakpoint_disable()
> in <asm/hw_breakpoint.h> if !CONFIG_PPC64?
That would be good.
What would be better, is to notice that machine_kexec_64.c is only ever
built for 64-bit - hence the name. And so no ifdefs or anything else is
required.
cheers
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [RFC Patch 6/6] Adapt kexec and samples code to recognise PPC64 hardware breakpoint usage
From: Geert Uytterhoeven @ 2009-05-14 14:18 UTC (permalink / raw)
To: Michael Ellerman
Cc: Michael Neuling, Benjamin Herrenschmidt, linuxppc-dev, paulus,
Alan Stern, K.Prasad, Roland McGrath
In-Reply-To: <1242310317.8608.1.camel@concordia>
On Fri, 15 May 2009, Michael Ellerman wrote:
> On Thu, 2009-05-14 at 15:59 +0200, Geert Uytterhoeven wrote:
> > On Thu, 14 May 2009, K.Prasad wrote:
> > > Modify kexec code to disable DABR registers before a reboot. Adapt the samples
> > > code to populate PPC64-arch specific fields.
> > >
> > > Signed-off-by: K.Prasad <prasad@linux.vnet.ibm.com>
> > > ---
> > > arch/powerpc/kernel/machine_kexec_64.c | 6
> > > samples/hw_breakpoint/data_breakpoint.c | 4
> > >
> > > Index: linux-2.6-tip.hbkpt/arch/powerpc/kernel/machine_kexec_64.c
> > > ===================================================================
> > > --- linux-2.6-tip.hbkpt.orig/arch/powerpc/kernel/machine_kexec_64.c 2009-05-14 00:17:24.000000000 +0530
> > > +++ linux-2.6-tip.hbkpt/arch/powerpc/kernel/machine_kexec_64.c 2009-05-14 09:48:09.000000000 +0530
> > > @@ -24,6 +24,7 @@
> > > #include <asm/sections.h> /* _end */
> > > #include <asm/prom.h>
> > > #include <asm/smp.h>
> > > +#include <asm/hw_breakpoint.h>
> > >
> > > int default_machine_kexec_prepare(struct kimage *image)
> > > {
> > > @@ -214,6 +215,9 @@
> > > put_cpu();
> > >
> > > local_irq_disable();
> > > +#ifdef CONFIG_PPC64
> > ^^^^^^^^^^^^^^^^^^^
> > > + hw_breakpoint_disable();
> > > +#endif
> > ^^^^^^
> >
> > What about providing a dummy definition of hw_breakpoint_disable()
> > in <asm/hw_breakpoint.h> if !CONFIG_PPC64?
>
> That would be good.
>
> What would be better, is to notice that machine_kexec_64.c is only ever
> built for 64-bit - hence the name. And so no ifdefs or anything else is
> required.
Right ;-)
But that can't be said for other files, like e.g. arch/powerpc/kernel/ptrace.c.
With kind regards,
Geert Uytterhoeven
Software Architect
Techsoft Centre
Technology and Software Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010
^ permalink raw reply
* Re: [RFC Patch 2/6] Introduce PPC64 specific Hardware Breakpoint interfaces
From: Michael Ellerman @ 2009-05-14 14:50 UTC (permalink / raw)
To: K.Prasad
Cc: Michael Neuling, Benjamin Herrenschmidt, linuxppc-dev, Alan Stern,
paulus, Roland McGrath
In-Reply-To: <20090514134439.GC14229@in.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 7522 bytes --]
On Thu, 2009-05-14 at 19:14 +0530, K.Prasad wrote:
> plain text document attachment (ppc64_arch_hwbkpt_implementation_02)
> Introduce PPC64 implementation for the generic hardware breakpoint interfaces
> defined in kernel/hw_breakpoint.c. Enable the HAVE_HW_BREAKPOINT flag and the
> Makefile.
Hi, some comments inline ...
> Index: linux-2.6-tip.hbkpt/arch/powerpc/kernel/hw_breakpoint.c
> ===================================================================
> --- /dev/null
> +++ linux-2.6-tip.hbkpt/arch/powerpc/kernel/hw_breakpoint.c
> @@ -0,0 +1,281 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
> + *
> + * Copyright (C) 2009 IBM Corporation
> + */
Don't use (C), either use a proper ©, or just skip it. I don't know
why :)
> +/*
> + * HW_breakpoint: a unified kernel/user-space hardware breakpoint facility,
> + * using the CPU's debug registers.
> + */
This comment would normally go at the top of the file.
> +
> +#include <linux/notifier.h>
> +#include <linux/kallsyms.h>
> +#include <linux/kprobes.h>
> +#include <linux/percpu.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/sched.h>
> +#include <linux/init.h>
> +#include <linux/smp.h>
> +
> +#include <asm/hw_breakpoint.h>
> +#include <asm/processor.h>
> +#include <asm/sstep.h>
> +
> +/* 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);
How does this relate to the existing current_dabr per-cpu variable?
> +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;
Should that be hbp_kernel_pos >= HBP_NUM, you're checking array bounds
right?
> + bp = hbp_kernel[hbp_kernel_pos];
> + if (bp == NULL)
> + kdabr = 0;
> + else
> + kdabr = bp->info.address | bp->info.type | DABR_TRANSLATION;
> + set_dabr(kdabr);
> +}
> +
> +/*
> + * Install the thread breakpoints in their debug registers.
> + */
> +void arch_install_thread_hw_breakpoint(struct task_struct *tsk)
> +{
> + set_dabr(tsk->thread.dabr);
Can we avoid setting this value if it's not necessary? It might require
an hcall. See for example what we do in __switch_to().
> +/*
> + * Check for virtual address in user space.
> + */
> +static int arch_check_va_in_userspace(unsigned long va)
> +{
> + return (!(is_kernel_addr(va)));
> +}
> +
> +/*
> + * Check for virtual address in kernel space.
> + */
> +static int arch_check_va_in_kernelspace(unsigned long va)
> +{
> + return is_kernel_addr(va);
> +}
You don't need these two routines. Just use is_kernel_addr() directly,
that way people will know what the code is doing without having to
lookup these new functions.
> +/*
> + * Store a breakpoint's encoded address, length, and type.
> + */
This doesn't "store" in the sense I was thinking, it actually does a
lookup and returns info in the arg.
> +int arch_store_info(struct hw_breakpoint *bp)
> +{
> + /*
> + * User-space requests will always have the address field populated
> + * For kernel-addresses, either the address or symbol name can be
> + * specified.
> + */
Do user-space requests never have the name populated? Otherwise aren't
you overwriting the supplied address with the one from kallsyms?
> + 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 ret = -EINVAL;
> +
> + if (!bp)
> + return ret;
> +
> + switch (bp->info.type) {
> + case DABR_DATA_READ:
> + break;
> + case DABR_DATA_WRITE:
> + break;
> + case DABR_DATA_RW:
> + break;
You only need the final break here.
> + default:
> + return ret;
> + }
> +
> + if (bp->triggered)
> + ret = arch_store_info(bp);
Shouldn't you check ret here, bp->info.address might be bogus.
> +
> + /* Check for double word alignment - 8 bytes */
> + if (bp->info.address & HW_BREAKPOINT_ALIGN)
> + return -EINVAL;
> +
> + /* Check that the virtual address is in the proper range */
> + if (tsk) {
> + if (!arch_check_va_in_userspace(bp->info.address))
> + return -EFAULT;
> + } else {
> + if (!arch_check_va_in_kernelspace(bp->info.address))
> + return -EFAULT;
> + }
Which becomes:
is_kernel = is_kernel_addr(bp->info.address);
if (tsk && is_kernel || !tsk && !is_kernel)
return -EFAULT;
> +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 | bp->info.type |
> + DABR_TRANSLATION;
2nd place I've seen that pattern.
> + 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;
> + int cpu, stepped, is_kernel;
> +
> + /* Disable breakpoints during exception handling */
> + set_dabr(0);
> +
> + dar = regs->dar & (~HW_BREAKPOINT_ALIGN);
> + is_kernel = (dar >= TASK_SIZE) ? 1 : 0;
is_kernel_addr() ?
> + if (is_kernel)
> + bp = hbp_kernel[0];
> + else {
> + bp = current->thread.hbp[0];
> + /* Lazy debug register switching */
> + if (!bp)
> + return rc;
What if we keep hitting this case?
> + rc = NOTIFY_DONE;
> + }
> +
> + (bp->triggered)(bp, regs);
> +
> + cpu = get_cpu();
> + if (is_kernel)
> + per_cpu(dabr_data, cpu) = kdabr;
> + else
> + per_cpu(dabr_data, cpu) = current->thread.dabr;
> +
> + 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));
> +out:
> + /* Enable pre-emption only if single-stepping is finished */
> + if (stepped)
> + put_cpu_no_resched();
> + return rc;
> +}
Gotta run, laptop battery running out! :)
cheers
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* [PATCH] sata_fsl: Fix the command description of FSL SATA controller
From: Kumar Gala @ 2009-05-14 14:47 UTC (permalink / raw)
To: Jeff Garzik; +Cc: linux-ide, linux-kernel, linuxppc-dev
From: Dave Liu <daveliu@freescale.com>
The bit 11 of command description is reserved bit in Freescale
SATA controller and needs to be set to '1'. This is needed to
make sure the last write from the controller to the buffer
descriptor is seen before an interrupt is raised.
Signed-off-by: Dave Liu <daveliu@freescale.com>
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
Jeff, please pick this up as a bug fix for 2.6.30
drivers/ata/sata_fsl.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
index 5edd654..36b8629 100644
--- a/drivers/ata/sata_fsl.c
+++ b/drivers/ata/sata_fsl.c
@@ -205,6 +205,7 @@ struct cmdhdr_tbl_entry {
* Description information bitdefs
*/
enum {
+ CMD_DESC_RES = (1 << 11),
VENDOR_SPECIFIC_BIST = (1 << 10),
CMD_DESC_SNOOP_ENABLE = (1 << 9),
FPDMA_QUEUED_CMD = (1 << 8),
@@ -388,7 +389,7 @@ static void sata_fsl_qc_prep(struct ata_queued_cmd *qc)
void __iomem *hcr_base = host_priv->hcr_base;
unsigned int tag = sata_fsl_tag(qc->tag, hcr_base);
struct command_desc *cd;
- u32 desc_info = CMD_DESC_SNOOP_ENABLE;
+ u32 desc_info = CMD_DESC_RES | CMD_DESC_SNOOP_ENABLE;
u32 num_prde = 0;
u32 ttl_dwords = 0;
dma_addr_t cd_paddr;
@@ -841,7 +842,7 @@ issue_srst:
/* device reset/SRST is a control register update FIS, uses tag0 */
sata_fsl_setup_cmd_hdr_entry(pp, 0,
- SRST_CMD | CMD_DESC_SNOOP_ENABLE, 0, 0, 5);
+ SRST_CMD | CMD_DESC_RES | CMD_DESC_SNOOP_ENABLE, 0, 0, 5);
tf.ctl |= ATA_SRST; /* setup SRST bit in taskfile control reg */
ata_tf_to_fis(&tf, pmp, 0, cfis);
@@ -887,7 +888,8 @@ issue_srst:
* using ATA signature D2H register FIS to the host controller.
*/
- sata_fsl_setup_cmd_hdr_entry(pp, 0, CMD_DESC_SNOOP_ENABLE, 0, 0, 5);
+ sata_fsl_setup_cmd_hdr_entry(pp, 0, CMD_DESC_RES | CMD_DESC_SNOOP_ENABLE,
+ 0, 0, 5);
tf.ctl &= ~ATA_SRST; /* 2nd H2D Ctl. register FIS */
ata_tf_to_fis(&tf, pmp, 0, cfis);
--
1.6.0.6
^ permalink raw reply related
* Re: [RFC Patch 4/6] Modify process handling code to handle hardware debug registers
From: Michael Ellerman @ 2009-05-14 14:54 UTC (permalink / raw)
To: K.Prasad
Cc: Michael Neuling, Benjamin Herrenschmidt, linuxppc-dev, Alan Stern,
paulus, Roland McGrath
In-Reply-To: <20090514134528.GE14229@in.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 2069 bytes --]
On Thu, 2009-05-14 at 19:15 +0530, K.Prasad wrote:
> plain text document attachment (ppc64_modify_process_handling_04)
> 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)))
> + switch_to_thread_hw_breakpoint(new);
> +#else
To avoid all these ifdefs in the code we need something like this in a
header:
static inline int task_uses_debug_regs(struct task_struct *tsk)
{
#ifdef CONFIG_PPC64
return test_tsk_thread_flag(tsk, TIF_DEBUG);
#else
return 0;
#endif
}
> @@ -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;
> +#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);
If you just use current here you don't need to define tsk above.
cheers
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [RFC Patch 6/6] Adapt kexec and samples code to recognise PPC64 hardware breakpoint usage
From: Michael Ellerman @ 2009-05-14 14:55 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Michael Neuling, Benjamin Herrenschmidt, linuxppc-dev, paulus,
Alan Stern, K.Prasad, Roland McGrath
In-Reply-To: <alpine.LRH.2.00.0905141617480.2225@vixen.sonytel.be>
[-- Attachment #1: Type: text/plain, Size: 1980 bytes --]
On Thu, 2009-05-14 at 16:18 +0200, Geert Uytterhoeven wrote:
> On Fri, 15 May 2009, Michael Ellerman wrote:
> > On Thu, 2009-05-14 at 15:59 +0200, Geert Uytterhoeven wrote:
> > > On Thu, 14 May 2009, K.Prasad wrote:
> > > > Modify kexec code to disable DABR registers before a reboot. Adapt the samples
> > > > code to populate PPC64-arch specific fields.
> > > >
> > > > Signed-off-by: K.Prasad <prasad@linux.vnet.ibm.com>
> > > > ---
> > > > arch/powerpc/kernel/machine_kexec_64.c | 6
> > > > samples/hw_breakpoint/data_breakpoint.c | 4
> > > >
> > > > Index: linux-2.6-tip.hbkpt/arch/powerpc/kernel/machine_kexec_64.c
> > > > ===================================================================
> > > > --- linux-2.6-tip.hbkpt.orig/arch/powerpc/kernel/machine_kexec_64.c 2009-05-14 00:17:24.000000000 +0530
> > > > +++ linux-2.6-tip.hbkpt/arch/powerpc/kernel/machine_kexec_64.c 2009-05-14 09:48:09.000000000 +0530
> > > > @@ -24,6 +24,7 @@
> > > > #include <asm/sections.h> /* _end */
> > > > #include <asm/prom.h>
> > > > #include <asm/smp.h>
> > > > +#include <asm/hw_breakpoint.h>
> > > >
> > > > int default_machine_kexec_prepare(struct kimage *image)
> > > > {
> > > > @@ -214,6 +215,9 @@
> > > > put_cpu();
> > > >
> > > > local_irq_disable();
> > > > +#ifdef CONFIG_PPC64
> > > ^^^^^^^^^^^^^^^^^^^
> > > > + hw_breakpoint_disable();
> > > > +#endif
> > > ^^^^^^
> > >
> > > What about providing a dummy definition of hw_breakpoint_disable()
> > > in <asm/hw_breakpoint.h> if !CONFIG_PPC64?
> >
> > That would be good.
> >
> > What would be better, is to notice that machine_kexec_64.c is only ever
> > built for 64-bit - hence the name. And so no ifdefs or anything else is
> > required.
>
> Right ;-)
>
> But that can't be said for other files, like e.g. arch/powerpc/kernel/ptrace.c.
Yeah you're right, the rest of this series could use lots of #ifdefs in
headers love.
cheers
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH v7] introduce macro spin_event_timeout()
From: Grant Likely @ 2009-05-14 15:13 UTC (permalink / raw)
To: Timur Tabi; +Cc: linuxppc-dev, smaclennan, scottwood
In-Reply-To: <1241560385-26868-1-git-send-email-timur@freescale.com>
On Tue, May 5, 2009 at 3:53 PM, Timur Tabi <timur@freescale.com> wrote:
> The macro spin_event_timeout() takes a condition and timeout value
> (in microseconds) as parameters. =A0It spins until either the condition i=
s true
> or the timeout expires. =A0It returns the result of the condition when th=
e loop
> was terminated.
>
> This primary purpose of this macro is to poll on a hardware register unti=
l a
> status bit changes. =A0The timeout ensures that the loop still terminates=
if the
> bit doesn't change as expected. =A0This macro makes it easier for driver
> developers to perform this kind of operation properly.
>
> Signed-off-by: Timur Tabi <timur@freescale.com>
This version looks good to me. Who's the first user? (I'd like to see
that something is ready to use this before merging it)
Acked-by: Grant Likely <grant.likely@secretlab.ca>
> ---
>
> v7: add if-statement to use cpu_relax() if the delay is 0. =A0gcc will op=
timize
> out the if-statement if 'delay' is a constant.
>
> I'm making this a PowerPC-specific patch because I want to use
> tb_ticks_per_usec, which does not exist on all other platforms. =A0I don'=
t want
> to use jiffies because jiffies works only when interrupts are enabled, an=
d
> the resolution may not be fine enough.
>
> =A0arch/powerpc/include/asm/delay.h | =A0 32 ++++++++++++++++++++++++++++=
++++
> =A01 files changed, 32 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/delay.h b/arch/powerpc/include/asm/=
delay.h
> index f9200a6..2bde26f 100644
> --- a/arch/powerpc/include/asm/delay.h
> +++ b/arch/powerpc/include/asm/delay.h
> @@ -2,6 +2,8 @@
> =A0#define _ASM_POWERPC_DELAY_H
> =A0#ifdef __KERNEL__
>
> +#include <asm/time.h>
> +
> =A0/*
> =A0* Copyright 1996, Paul Mackerras.
> =A0*
> @@ -30,5 +32,35 @@ extern void udelay(unsigned long usecs);
> =A0#define mdelay(n) =A0 =A0 =A0udelay((n) * 1000)
> =A0#endif
>
> +/**
> + * spin_event_timeout - spin until a condition gets true or a timeout el=
apses
> + * @condition: a C expression to evalate
> + * @timeout: timeout, in microseconds
> + * @delay: the number of microseconds to delay between eache evaluation =
of
> + * =A0 =A0 =A0 =A0 @condition
> + * @rc: the last value of the condition
> + *
> + * The process spins until the condition evaluates to true (non-zero) or=
the
> + * timeout elapses. =A0Upon exit, @rc contains the value of the conditio=
n. This
> + * allows you to test the condition without incurring any side effects.
> + *
> + * This primary purpose of this macro is to poll on a hardware register
> + * until a status bit changes. =A0The timeout ensures that the loop stil=
l
> + * terminates even if the bit never changes. =A0The delay is for devices=
that
> + * need a delay in between successive reads.
> + *
> + * gcc will optimize out the if-statement if @delay is a constant.
> + */
> +#define spin_event_timeout(condition, timeout, delay, rc) =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 \
> +{ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
=A0 \
> + =A0 =A0 =A0 unsigned long __loops =3D tb_ticks_per_usec * timeout; =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0\
> + =A0 =A0 =A0 unsigned long __start =3D get_tbl(); =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0\
> + =A0 =A0 =A0 while (!(rc =3D (condition)) && (tb_ticks_since(__start) <=
=3D __loops)) \
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (delay) =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0\
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 udelay(delay); =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0\
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 else =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
\
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 cpu_relax(); =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0\
> +}
> +
> =A0#endif /* __KERNEL__ */
> =A0#endif /* _ASM_POWERPC_DELAY_H */
> --
> 1.6.0.6
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH] powerpc/ftrace: Use pr_devel() in ftrace.c
From: Steven Rostedt @ 2009-05-14 15:49 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <e805665453162bf2eb90b402e41fc7b672fcf639.1242282612.git.michael@ellerman.id.au>
I'm fine with this patch, but it should go via the PPC tree.
Acked-by: Steven Rostedt <rostedt@goodmis.org>
-- Steve
On Thu, 14 May 2009, Michael Ellerman wrote:
> pr_debug() can now result in code being generated even when #DEBUG
> is not defined. That's not really desirable in the ftrace code
> which we want to be snappy.
>
> With CONFIG_DYNAMIC_DEBUG=y:
>
> size before:
> text data bss dec hex filename
> 3334 672 4 4010 faa arch/powerpc/kernel/ftrace.o
>
> size after:
> text data bss dec hex filename
> 2616 360 4 2980 ba4 arch/powerpc/kernel/ftrace.o
>
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> ---
> arch/powerpc/kernel/ftrace.c | 20 ++++++++++----------
> 1 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/kernel/ftrace.c b/arch/powerpc/kernel/ftrace.c
> index 70e2a73..5b078ee 100644
> --- a/arch/powerpc/kernel/ftrace.c
> +++ b/arch/powerpc/kernel/ftrace.c
> @@ -157,7 +157,7 @@ __ftrace_make_nop(struct module *mod,
> * 0xe8, 0x4c, 0x00, 0x28, ld r2,40(r12)
> */
>
> - pr_debug("ip:%lx jumps to %lx r2: %lx", ip, tramp, mod->arch.toc);
> + pr_devel("ip:%lx jumps to %lx r2: %lx", ip, tramp, mod->arch.toc);
>
> /* Find where the trampoline jumps to */
> if (probe_kernel_read(jmp, (void *)tramp, sizeof(jmp))) {
> @@ -165,7 +165,7 @@ __ftrace_make_nop(struct module *mod,
> return -EFAULT;
> }
>
> - pr_debug(" %08x %08x", jmp[0], jmp[1]);
> + pr_devel(" %08x %08x", jmp[0], jmp[1]);
>
> /* verify that this is what we expect it to be */
> if (((jmp[0] & 0xffff0000) != 0x3d820000) ||
> @@ -181,18 +181,18 @@ __ftrace_make_nop(struct module *mod,
> offset = ((unsigned)((unsigned short)jmp[0]) << 16) +
> (int)((short)jmp[1]);
>
> - pr_debug(" %x ", offset);
> + pr_devel(" %x ", offset);
>
> /* get the address this jumps too */
> tramp = mod->arch.toc + offset + 32;
> - pr_debug("toc: %lx", tramp);
> + pr_devel("toc: %lx", tramp);
>
> if (probe_kernel_read(jmp, (void *)tramp, 8)) {
> printk(KERN_ERR "Failed to read %lx\n", tramp);
> return -EFAULT;
> }
>
> - pr_debug(" %08x %08x\n", jmp[0], jmp[1]);
> + pr_devel(" %08x %08x\n", jmp[0], jmp[1]);
>
> ptr = ((unsigned long)jmp[0] << 32) + jmp[1];
>
> @@ -269,7 +269,7 @@ __ftrace_make_nop(struct module *mod,
> * 0x4e, 0x80, 0x04, 0x20 bctr
> */
>
> - pr_debug("ip:%lx jumps to %lx", ip, tramp);
> + pr_devel("ip:%lx jumps to %lx", ip, tramp);
>
> /* Find where the trampoline jumps to */
> if (probe_kernel_read(jmp, (void *)tramp, sizeof(jmp))) {
> @@ -277,7 +277,7 @@ __ftrace_make_nop(struct module *mod,
> return -EFAULT;
> }
>
> - pr_debug(" %08x %08x ", jmp[0], jmp[1]);
> + pr_devel(" %08x %08x ", jmp[0], jmp[1]);
>
> /* verify that this is what we expect it to be */
> if (((jmp[0] & 0xffff0000) != 0x3d600000) ||
> @@ -293,7 +293,7 @@ __ftrace_make_nop(struct module *mod,
> if (tramp & 0x8000)
> tramp -= 0x10000;
>
> - pr_debug(" %lx ", tramp);
> + pr_devel(" %lx ", tramp);
>
> if (tramp != addr) {
> printk(KERN_ERR
> @@ -402,7 +402,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
> /* ld r2,40(r1) */
> op[1] = 0xe8410028;
>
> - pr_debug("write to %lx\n", rec->ip);
> + pr_devel("write to %lx\n", rec->ip);
>
> if (probe_kernel_write((void *)ip, op, MCOUNT_INSN_SIZE * 2))
> return -EPERM;
> @@ -442,7 +442,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
> return -EINVAL;
> }
>
> - pr_debug("write to %lx\n", rec->ip);
> + pr_devel("write to %lx\n", rec->ip);
>
> if (probe_kernel_write((void *)ip, &op, MCOUNT_INSN_SIZE))
> return -EPERM;
> --
> 1.6.2.1
>
>
^ permalink raw reply
* Re: [PATCH v7] introduce macro spin_event_timeout()
From: Timur Tabi @ 2009-05-14 16:03 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev, smaclennan, scottwood
In-Reply-To: <fa686aa40905140813l5caf7085r64bce935be0a22d3@mail.gmail.com>
Grant Likely wrote:
> This version looks good to me. Who's the first user? (I'd like to see
> that something is ready to use this before merging it)
I was going to do it the other way around - wait until this patch is merged, and then update some code to use it. After all, it's easier to use a macro if it already exists in a header file. :-)
Grepping for "while.*in_be" finds lots of candidates.
I was originally planning to use this macro in sound/soc/fsl/fsl_ssi.c, but I eliminated the need to poll on a register with commit a4d11fe50c238a7da5225d1399314c3505cbd792 ("ASoC: remove trigger delay in Freescale MPC8610 sound driver"). So now I need to find a new guinea pig.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH v7] introduce macro spin_event_timeout()
From: Grant Likely @ 2009-05-14 16:10 UTC (permalink / raw)
To: Timur Tabi; +Cc: linuxppc-dev, smaclennan, scottwood
In-Reply-To: <4A0C40D4.1090508@freescale.com>
On Thu, May 14, 2009 at 10:03 AM, Timur Tabi <timur@freescale.com> wrote:
> Grant Likely wrote:
>
>> This version looks good to me. =A0Who's the first user? (I'd like to see
>> that something is ready to use this before merging it)
>
> I was going to do it the other way around - wait until this patch is merg=
ed, and then update some code to use it. =A0After all, it's easier to use a=
macro if it already exists in a header file. :-)
Heh, I think you miss my meaning. Of course this patch must be merged
before any users. I just want to hold off merging it until I see
viable patches on the mailing list which depend on it.
In other words, write your patches which use it and submit the lot as
a patch series with this patch as the first. That gives some evidence
that this macro will actually be used and useful.
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* [RFC] Using the DMAEngine API for scatter/gather operations
From: Ira Snyder @ 2009-05-14 16:13 UTC (permalink / raw)
To: Dan Williams, Li Yang, Zhang Wei, linuxppc-dev
Hello all.
I'm working on a driver for a custom board (based heavily on the
MPC8349EMDS board) to dump data out of onboard FPGA's. I need to use the
onboard DMA controller for this, mostly due to data transfer rate and
timing requirements.
Of course, I'd like to "play nice" with the existing Linux
infrastructure in my driver, even though I have no plans to submit the
driver upstream (since it is only useful for our custom board).
I have been using the DMAEngine API to interact with the DMA controller,
and it works without any problems. However it seems that it is missing
one of the major features that is supported by almost all DMA
controllers: scatter/gather support.
What I'd really like to do is give the controller a scatterlist and list
of hardware addresses, and have it set up the transfer. The Freescale
hardware can handle this, and I could program it by hand to do so, but I
don't want to invent my own DMA API.
I'm aware that the DMAEngine API was designed to offload memcpy, and
pretty much nothing else. Is there any chance that such a change would
be accepted into the mainline kernel? Any opinions on what such an API
should look like?
Thanks,
Ira
^ permalink raw reply
* Re: [RFC] Using the DMAEngine API for scatter/gather operations
From: Dan Williams @ 2009-05-14 16:27 UTC (permalink / raw)
To: Ira Snyder; +Cc: linuxppc-dev@ozlabs.org, Li Yang, Zhang Wei
In-Reply-To: <20090514161353.GA4379@ovro.caltech.edu>
On Thu, 2009-05-14 at 09:13 -0700, Ira Snyder wrote:
> Hello all.
>
> I'm working on a driver for a custom board (based heavily on the
> MPC8349EMDS board) to dump data out of onboard FPGA's. I need to use the
> onboard DMA controller for this, mostly due to data transfer rate and
> timing requirements.
>
> Of course, I'd like to "play nice" with the existing Linux
> infrastructure in my driver, even though I have no plans to submit the
> driver upstream (since it is only useful for our custom board).
>
> I have been using the DMAEngine API to interact with the DMA controller,
> and it works without any problems. However it seems that it is missing
> one of the major features that is supported by almost all DMA
> controllers: scatter/gather support.
>
> What I'd really like to do is give the controller a scatterlist and list
> of hardware addresses, and have it set up the transfer. The Freescale
> hardware can handle this, and I could program it by hand to do so, but I
> don't want to invent my own DMA API.
>
> I'm aware that the DMAEngine API was designed to offload memcpy, and
> pretty much nothing else. Is there any chance that such a change would
> be accepted into the mainline kernel? Any opinions on what such an API
> should look like?
>
Have you taken a look at the 'slave' interface implemented by dw_dmac
and other drivers? It has the following interface:
struct dma_async_tx_descriptor *(*device_prep_slave_sg)(
struct dma_chan *chan, struct scatterlist *sgl,
unsigned int sg_len, enum dma_data_direction direction,
unsigned long flags);
Haavard uses this interface to do sgl transfers on behalf of the
atmel-mci driver (drivers/mmc/host/atmel-mci.c).
--
Dan
^ permalink raw reply
* Re: Please pull from 'merge' branch for 2.6.30
From: Grant Likely @ 2009-05-14 16:41 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <Pine.LNX.4.64.0905131731260.772@localhost.localdomain>
Hi Ben,
Here are some more commits which got missed in the last merge cycle.
I've based this on top of Kumar's pull request from yesterday to
minimize merge commits.
Note: This pull request replaces my previous pull request. I rebased
the series since some of the commits in my previous request have
already been merged. If you've already pulled my last request then
you don't need to pull this one.
Cheers,
g.
The following changes since commit 2e15eedffaae555085071c68cb94b9eeed2245be=
:
Kumar Gala (1):
powerpc/8xx: Update defconfigs
are available in the git repository at:
git://git.secretlab.ca/git/linux-2.6 merge
Grant Likely (2):
powerpc/boot: add simpleImage.* to clean-files list
powerpc/virtex: Add uImage to the default images list
John Linn (1):
powerpc/virtex: Fix duplicate level irq events.
arch/powerpc/boot/Makefile | 2 +-
arch/powerpc/platforms/40x/Kconfig | 1 +
arch/powerpc/platforms/44x/Kconfig | 1 +
arch/powerpc/sysdev/xilinx_intc.c | 117 ++++++++++++++++++++++++++++++++=
---
4 files changed, 110 insertions(+), 11 deletions(-)
On Wed, May 13, 2009 at 4:31 PM, Kumar Gala <galak@kernel.crashing.org> wro=
te:
> Just added defconfig updates for the normal FSL set of HW.
>
> - k
>
> Please pull from 'merge' branch of
>
> =A0 =A0 =A0 =A0master.kernel.org:/pub/scm/linux/kernel/git/galak/powerpc.=
git merge
>
> to receive the following updates:
>
> =A0arch/powerpc/configs/83xx/asp8347_defconfig =A0 =A0 =A0 | =A0104 ++++-
> =A0arch/powerpc/configs/83xx/mpc8313_rdb_defconfig =A0 | =A0114 ++++--
> =A0arch/powerpc/configs/83xx/mpc8315_rdb_defconfig =A0 | =A0114 ++++--
> =A0arch/powerpc/configs/83xx/mpc832x_mds_defconfig =A0 | =A0107 ++++--
> =A0arch/powerpc/configs/83xx/mpc832x_rdb_defconfig =A0 | =A0113 ++++--
> =A0arch/powerpc/configs/83xx/mpc834x_itx_defconfig =A0 | =A0108 ++++--
> =A0arch/powerpc/configs/83xx/mpc834x_itxgp_defconfig | =A0108 ++++--
> =A0arch/powerpc/configs/83xx/mpc834x_mds_defconfig =A0 | =A0103 ++++-
> =A0arch/powerpc/configs/83xx/mpc836x_mds_defconfig =A0 | =A0108 ++++--
> =A0arch/powerpc/configs/83xx/mpc836x_rdk_defconfig =A0 | =A0 96 ++++-
> =A0arch/powerpc/configs/83xx/mpc837x_mds_defconfig =A0 | =A0104 ++++-
> =A0arch/powerpc/configs/83xx/mpc837x_rdb_defconfig =A0 | =A0111 ++++--
> =A0arch/powerpc/configs/83xx/sbc834x_defconfig =A0 =A0 =A0 | =A0 96 ++++-
> =A0arch/powerpc/configs/85xx/ksi8560_defconfig =A0 =A0 =A0 | =A0 86 +++-
> =A0arch/powerpc/configs/85xx/mpc8540_ads_defconfig =A0 | =A0 82 +++-
> =A0arch/powerpc/configs/85xx/mpc8560_ads_defconfig =A0 | =A0 89 +++--
> =A0arch/powerpc/configs/85xx/mpc85xx_cds_defconfig =A0 | =A0 88 +++--
> =A0arch/powerpc/configs/85xx/sbc8548_defconfig =A0 =A0 =A0 | =A0 84 +++-
> =A0arch/powerpc/configs/85xx/sbc8560_defconfig =A0 =A0 =A0 | =A0 83 +++-
> =A0arch/powerpc/configs/85xx/socrates_defconfig =A0 =A0 =A0| =A0385 +++++=
+++++++++++------
> =A0arch/powerpc/configs/85xx/stx_gp3_defconfig =A0 =A0 =A0 | =A0105 ++++-=
-
> =A0arch/powerpc/configs/85xx/tqm8540_defconfig =A0 =A0 =A0 | =A0100 ++++-
> =A0arch/powerpc/configs/85xx/tqm8541_defconfig =A0 =A0 =A0 | =A0101 ++++-
> =A0arch/powerpc/configs/85xx/tqm8548_defconfig =A0 =A0 =A0 | =A0 64 ++-
> =A0arch/powerpc/configs/85xx/tqm8555_defconfig =A0 =A0 =A0 | =A0101 ++++-
> =A0arch/powerpc/configs/85xx/tqm8560_defconfig =A0 =A0 =A0 | =A0101 ++++-
> =A0arch/powerpc/configs/86xx/gef_ppc9a_defconfig =A0 =A0 | =A0 92 ++++-
> =A0arch/powerpc/configs/86xx/gef_sbc310_defconfig =A0 =A0| =A0 92 ++++-
> =A0arch/powerpc/configs/86xx/gef_sbc610_defconfig =A0 =A0| =A0122 +++++-
> =A0arch/powerpc/configs/86xx/mpc8610_hpcd_defconfig =A0| =A0100 ++++-
> =A0arch/powerpc/configs/86xx/mpc8641_hpcn_defconfig =A0| =A0211 ++++-----=
---
> =A0arch/powerpc/configs/86xx/sbc8641d_defconfig =A0 =A0 =A0| =A0114 ++++-=
-
> =A0arch/powerpc/configs/adder875_defconfig =A0 =A0 =A0 =A0 =A0 | =A0 65 +=
+-
> =A0arch/powerpc/configs/c2k_defconfig =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0| =
=A0128 +++++--
> =A0arch/powerpc/configs/ep8248e_defconfig =A0 =A0 =A0 =A0 =A0 =A0| =A0 74=
+++-
> =A0arch/powerpc/configs/ep88xc_defconfig =A0 =A0 =A0 =A0 =A0 =A0 | =A0 63=
++-
> =A0arch/powerpc/configs/linkstation_defconfig =A0 =A0 =A0 =A0| =A0 96 +++=
+-
> =A0arch/powerpc/configs/mgcoge_defconfig =A0 =A0 =A0 =A0 =A0 =A0 | =A0 74=
+++-
> =A0arch/powerpc/configs/mgsuvd_defconfig =A0 =A0 =A0 =A0 =A0 =A0 | =A0 72=
+++-
> =A0arch/powerpc/configs/mpc7448_hpc2_defconfig =A0 =A0 =A0 | =A0 91 +++--
> =A0arch/powerpc/configs/mpc8272_ads_defconfig =A0 =A0 =A0 =A0| =A0 77 +++=
-
> =A0arch/powerpc/configs/mpc83xx_defconfig =A0 =A0 =A0 =A0 =A0 =A0| =A0115=
++++--
> =A0arch/powerpc/configs/mpc866_ads_defconfig =A0 =A0 =A0 =A0 | =A0 77 +++=
-
> =A0arch/powerpc/configs/mpc86xx_defconfig =A0 =A0 =A0 =A0 =A0 =A0| =A0211=
++++--------
> =A0arch/powerpc/configs/mpc885_ads_defconfig =A0 =A0 =A0 =A0 | =A0 63 ++-
> =A0arch/powerpc/configs/pq2fads_defconfig =A0 =A0 =A0 =A0 =A0 =A0| =A0 90=
+++--
> =A0arch/powerpc/configs/prpmc2800_defconfig =A0 =A0 =A0 =A0 =A0| =A0120 +=
++++-
> =A0arch/powerpc/configs/storcenter_defconfig =A0 =A0 =A0 =A0 | =A0 71 ++-=
-
> =A0arch/powerpc/sysdev/fsl_soc.c =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
| =A0 46 --
> =A049 files changed, 3630 insertions(+), 1489 deletions(-)
>
> Anton Vorontsov (1):
> =A0 =A0 =A0powerpc/fsl_soc: Remove mpc83xx_wdt_init, again
>
> Kumar Gala (5):
> =A0 =A0 =A0powerpc/83xx: Update defconfigs
> =A0 =A0 =A0powerpc/85xx: Update defconfigs
> =A0 =A0 =A0powerpc/86xx: Update defconfigs
> =A0 =A0 =A0powerpc/embedded6xx: Update defconfigs
> =A0 =A0 =A0powerpc/8xx: Update defconfigs
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH] i2c-mpc: generate START condition after STOP caused by read i2c_msg
From: Joakim Tjernlund @ 2009-05-14 16:52 UTC (permalink / raw)
To: Esben Haabendal; +Cc: linuxppc-dev
In-Reply-To: <d2b9ea600905140317u7a4b9a70s574568d79a1ce7cc@mail.gmail.com>
Esben Haabendal <esbenhaabendal@gmail.com> wrote on 14/05/2009 12:17:48=
:
> On Thu, May 14, 2009 at 11:58 AM, Joakim Tjernlund
> <joakim.tjernlund@transmode.se> wrote:
> >>
> >> The problem is that after the STOP condition, the following i2c_ms=
g will be
> >> attempted with a repeated START, which according to specification =
will
> >> cause a MAL, which also happens. =A0My MPC8360ERM reads:
> >>
> >> "Attempting a repeated START at the wrong time (or if the bus is o=
wned
> >> by another master), results in loss of arbitration."
> >>
> >> Which I know read as it that we must own the I2C bus when using RS=
TA.
> >
> > I agree with the theory, but isn't the problem here that STOP is pe=
rformed in
> > the middle of this transaction?
> > Remove the STOP and RSTA will work I think.
>
> That was also my first idea.
>
> But at least for my CS4265, reads does not work without a STOP follow=
ing the
> last byte. I guess I am not the first to experience this, as indicate=
d
> by the current
> i2c-mpc.c implementation.
> As far as I can understand the I2C specification, a STOP should not b=
e required
> after reads, but I am not sure.
>
> The cost of the additional STOP is really small. The only issue I ca=
n see,
> is that with the STOP in the midle of an i2c_transfer, the I2C bus is=
released.
> This naturally also happens with the current implementation, so I bel=
ieve
> that this patch is a clean improvement.
>
> Anyways, if someone can figure out how to achive this without STOP,
> I will be happy (re)test with my hardware setup here.
>From just playing a little, I think the below patch will get you far.
However, I had to change this too to make it stable:
static void mpc_i2c_start(struct mpc_i2c *i2c)
{
/* Clear arbitration */
writeb(0, i2c->base + MPC_I2C_SR);
#if 1 // fix, not sure why
writeccr(i2c, 0);
udelay(5);
#endif
/* Start with MEN */
writeccr(i2c, CCR_MEN);
}
PS.
I added back the linuxppc list, let it stay.
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.=
c
index 6c1cddd..d7edcd2 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -189,9 +189,6 @@ static int mpc_write(struct mpc_i2c *i2c, int targe=
t,
unsigned timeout =3D i2c->adap.timeout;
u32 flags =3D restart ? CCR_RSTA : 0;
- /* Start with MEN */
- if (!restart)
- writeccr(i2c, CCR_MEN);
/* Start as master */
writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA | CCR_MTX | flags);
/* Write target byte */
@@ -220,9 +217,6 @@ static int mpc_read(struct mpc_i2c *i2c, int target=
,
int i, result;
u32 flags =3D restart ? CCR_RSTA : 0;
- /* Start with MEN */
- if (!restart)
- writeccr(i2c, CCR_MEN);
/* Switch to read - restart */
writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA | CCR_MTX | flags);
/* Write target address byte - this time with the read flag set */
@@ -241,18 +235,15 @@ static int mpc_read(struct mpc_i2c *i2c, int targ=
et,
readb(i2c->base + MPC_I2C_DR);
}
- for (i =3D 0; i < length; i++) {
+ for (i =3D length; i ; --i) {
result =3D i2c_wait(i2c, timeout, 0);
if (result < 0)
return result;
- /* Generate txack on next to last byte */
- if (i =3D=3D length - 2)
+ /* Generate txack on next to last byte(s) */
+ if (i <=3D 2)
writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA | CCR_TXAK);
- /* Generate stop on last byte */
- if (i =3D=3D length - 1)
- writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_TXAK);
- data[i] =3D readb(i2c->base + MPC_I2C_DR);
+ data[length-i] =3D readb(i2c->base + MPC_I2C_DR);
}
return length;
=
^ permalink raw reply related
* Re: [RFC] Using the DMAEngine API for scatter/gather operations
From: Ira Snyder @ 2009-05-14 16:55 UTC (permalink / raw)
To: Dan Williams; +Cc: linuxppc-dev@ozlabs.org, Li Yang, Zhang Wei
In-Reply-To: <1242318437.6797.3.camel@dwillia2-mobl1.amr.corp.intel.com>
On Thu, May 14, 2009 at 09:27:17AM -0700, Dan Williams wrote:
> On Thu, 2009-05-14 at 09:13 -0700, Ira Snyder wrote:
> > Hello all.
> >
> > I'm working on a driver for a custom board (based heavily on the
> > MPC8349EMDS board) to dump data out of onboard FPGA's. I need to use the
> > onboard DMA controller for this, mostly due to data transfer rate and
> > timing requirements.
> >
> > Of course, I'd like to "play nice" with the existing Linux
> > infrastructure in my driver, even though I have no plans to submit the
> > driver upstream (since it is only useful for our custom board).
> >
> > I have been using the DMAEngine API to interact with the DMA controller,
> > and it works without any problems. However it seems that it is missing
> > one of the major features that is supported by almost all DMA
> > controllers: scatter/gather support.
> >
> > What I'd really like to do is give the controller a scatterlist and list
> > of hardware addresses, and have it set up the transfer. The Freescale
> > hardware can handle this, and I could program it by hand to do so, but I
> > don't want to invent my own DMA API.
> >
> > I'm aware that the DMAEngine API was designed to offload memcpy, and
> > pretty much nothing else. Is there any chance that such a change would
> > be accepted into the mainline kernel? Any opinions on what such an API
> > should look like?
> >
>
> Have you taken a look at the 'slave' interface implemented by dw_dmac
> and other drivers? It has the following interface:
>
> struct dma_async_tx_descriptor *(*device_prep_slave_sg)(
> struct dma_chan *chan, struct scatterlist *sgl,
> unsigned int sg_len, enum dma_data_direction direction,
> unsigned long flags);
>
> Haavard uses this interface to do sgl transfers on behalf of the
> atmel-mci driver (drivers/mmc/host/atmel-mci.c).
>
I had started looking at it, though the Freescale device does not
implement DMA_SLAVE support. I was a bit discouraged by the changelog
for commit dc0ee643, which says that the interface is for "DMA with
hardware handshaking", which I do not have. There is not anything
special about accessing the FPGA's, they appear in the processor's
memory map and can be read from/written to just like RAM.
I'll try studying the dw_dmac implementation and atmel-mci example.
Thanks for the response,
Ira
^ permalink raw reply
* Re: [RFC] Hardware Breakpoint interfaces implementation for PPC64
From: K.Prasad @ 2009-05-14 18:52 UTC (permalink / raw)
To: Josh Boyer, linuxppc-dev, Benjamin Herrenschmidt, paulus
In-Reply-To: <20090513025717.GN24338@yookeroo.seuss>
On Wed, May 13, 2009 at 12:57:17PM +1000, David Gibson wrote:
> On Wed, May 13, 2009 at 01:55:45AM +0530, K.Prasad wrote:
> > On Tue, May 12, 2009 at 07:51:49AM -0400, Josh Boyer wrote:
> > > On Tue, May 12, 2009 at 01:33:55AM +0530, K.Prasad wrote:
> [snip]
>
> It seems to me that a kind of minimal requirement for a sensible
> generic debug interface is that if no processes actually ask to use
> the debug regs, then we should never touch them in the hardware. This
> means that debugging hacks in the kernel can just use the debug regs
> directly and don't have to go through the interface to avoid having
> their stuff clobbered on context switch.
>
All that we do additionally for kernel users (when no process is
currently using the debug regs) is to account for the usage just so that
any new requests (kernel or user-space) are denied.
> --
> 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
Thanks,
K.Prasad
^ permalink raw reply
* Re: [RFC Patch 6/6] Adapt kexec and samples code to recognise PPC64hardware breakpoint usage
From: K.Prasad @ 2009-05-14 19:15 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Michael Neuling, Benjamin Herrenschmidt, linuxppc-dev, Alan Stern,
paulus, Roland McGrath
In-Reply-To: <alpine.LRH.2.00.0905141557260.2225@vixen.sonytel.be>
On Thu, May 14, 2009 at 03:59:45PM +0200, Geert Uytterhoeven wrote:
> On Thu, 14 May 2009, K.Prasad wrote:
> > Modify kexec code to disable DABR registers before a reboot. Adapt the samples
> > code to populate PPC64-arch specific fields.
> >
> > Signed-off-by: K.Prasad <prasad@linux.vnet.ibm.com>
> > ---
> > arch/powerpc/kernel/machine_kexec_64.c | 6
> > samples/hw_breakpoint/data_breakpoint.c | 4
> >
> > Index: linux-2.6-tip.hbkpt/arch/powerpc/kernel/machine_kexec_64.c
> > ===================================================================
> > --- linux-2.6-tip.hbkpt.orig/arch/powerpc/kernel/machine_kexec_64.c 2009-05-14 00:17:24.000000000 +0530
> > +++ linux-2.6-tip.hbkpt/arch/powerpc/kernel/machine_kexec_64.c 2009-05-14 09:48:09.000000000 +0530
> > @@ -24,6 +24,7 @@
> > #include <asm/sections.h> /* _end */
> > #include <asm/prom.h>
> > #include <asm/smp.h>
> > +#include <asm/hw_breakpoint.h>
> >
> > int default_machine_kexec_prepare(struct kimage *image)
> > {
> > @@ -214,6 +215,9 @@
> > put_cpu();
> >
> > local_irq_disable();
> > +#ifdef CONFIG_PPC64
> ^^^^^^^^^^^^^^^^^^^
> > + hw_breakpoint_disable();
> > +#endif
> ^^^^^^
>
> What about providing a dummy definition of hw_breakpoint_disable()
> in <asm/hw_breakpoint.h> if !CONFIG_PPC64?
>
Yes, that's a good idea. I will change it.
Thanks,
K.Prasad
^ permalink raw reply
* Re: [RFC Patch 2/6] Introduce PPC64 specific Hardware Breakpointinterfaces
From: K.Prasad @ 2009-05-14 19:50 UTC (permalink / raw)
To: Michael Ellerman
Cc: Michael Neuling, Benjamin Herrenschmidt, linuxppc-dev, Alan Stern,
paulus, Roland McGrath
In-Reply-To: <1242312611.8608.32.camel@concordia>
On Fri, May 15, 2009 at 12:50:11AM +1000, Michael Ellerman wrote:
> On Thu, 2009-05-14 at 19:14 +0530, K.Prasad wrote:
> > plain text document attachment (ppc64_arch_hwbkpt_implementation_02)
> > Introduce PPC64 implementation for the generic hardware breakpoint interfaces
> > defined in kernel/hw_breakpoint.c. Enable the HAVE_HW_BREAKPOINT flag and the
> > Makefile.
>
> Hi, some comments inline ...
>
>
Thanks for reviewing the code.
> > Index: linux-2.6-tip.hbkpt/arch/powerpc/kernel/hw_breakpoint.c
> > ===================================================================
> > --- /dev/null
> > +++ linux-2.6-tip.hbkpt/arch/powerpc/kernel/hw_breakpoint.c
> > @@ -0,0 +1,281 @@
> > +/*
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, write to the Free Software
> > + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
> > + *
> > + * Copyright (C) 2009 IBM Corporation
> > + */
>
> Don't use (C), either use a proper ©, or just skip it. I don't know
> why :)
>
Ok.
> > +/*
> > + * HW_breakpoint: a unified kernel/user-space hardware breakpoint facility,
> > + * using the CPU's debug registers.
> > + */
>
> This comment would normally go at the top of the file.
>
Ok.
> > +
> > +#include <linux/notifier.h>
> > +#include <linux/kallsyms.h>
> > +#include <linux/kprobes.h>
> > +#include <linux/percpu.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/sched.h>
> > +#include <linux/init.h>
> > +#include <linux/smp.h>
> > +
> > +#include <asm/hw_breakpoint.h>
> > +#include <asm/processor.h>
> > +#include <asm/sstep.h>
> > +
> > +/* 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);
>
> How does this relate to the existing current_dabr per-cpu variable?
>
The present infrastructure assumes that kernel-space (through xmon and
KGDB) and user-space requests happen at mutually exclusive times.
current_dabr in arch/powerpc/kernel/process.c stores the value of DABR
as requested by the current process and helps initiate a set_dabr() call
if the new incoming process's DABR value is different (when neither of
them use DABR, the values are 0, hence no set_dabr() call).
The per-cpu 'dabr_data' seen above is used to store the value of DABR
temporarily between a HW Breakpoint exception handler
hw_breakpoint_handler() (where the breakpoint exception might be
temporarily disabled if emulate_step() failed) and a single-step
exception handler single_step_dabr_instruction() in which the DABR value
is restored from per-cpu 'dabr_data'.
This is required because PowerPC triggers DABR match exception before
execution of the causative instruction (such as load/store) and if the
DABR value remains enabled, the system will enter into an infinite loop.
> > +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;
>
> Should that be hbp_kernel_pos >= HBP_NUM, you're checking array bounds
> right?
>
In short, no. If hbp_kernel_pos > HBP_NUM it is only indicative of erroneous
code and not a condition that the system could enter.
To explain 'hbp_kernel_pos' further, it is that variable which points to
the last numbered debug register occupied by the kernel (which is more
meaningful on a processor having more than one debug register).
In the above code, in arch_update_kernel_hw_breakpoint()
"hbp_kernel_pos == HBP_NUM" condition would be satisfied when
invoked from load_debug_registers() at initialisation time (kernel
requests are serviced starting from highest numbered register).
Having said that, I find that I've missed invoking
load_debug_registers() [a part of kernel/hw_breakpoint.c] in
start_secondary(). Thanks for raising this question which helped me
identify the missing invocation, I will add the same.
> > + bp = hbp_kernel[hbp_kernel_pos];
> > + if (bp == NULL)
> > + kdabr = 0;
> > + else
> > + kdabr = bp->info.address | bp->info.type | DABR_TRANSLATION;
> > + set_dabr(kdabr);
> > +}
> > +
> > +/*
> > + * Install the thread breakpoints in their debug registers.
> > + */
> > +void arch_install_thread_hw_breakpoint(struct task_struct *tsk)
> > +{
> > + set_dabr(tsk->thread.dabr);
>
> Can we avoid setting this value if it's not necessary? It might require
> an hcall. See for example what we do in __switch_to().
>
I see that you're referring to this code in __switch_to() :
if (unlikely(__get_cpu_var(current_dabr) != new->thread.dabr))
set_dabr(new->thread.dabr);
arch_install_thread_hw_breakpoint()<--switch_to_thread_hw_breakpoint()
<--__switch_to() implementation is also similar.
In __switch_to(),
if (unlikely(test_tsk_thread_flag(new, TIF_DEBUG)))
switch_to_thread_hw_breakpoint(new);
happens only when TIF_DEBUG flag is set. This flag is cleared when the
process unregisters any breakpoints it had requested earlier. So, the
set_dabr() call is avoided for processes not using the debug register.
> > +/*
> > + * Check for virtual address in user space.
> > + */
> > +static int arch_check_va_in_userspace(unsigned long va)
> > +{
> > + return (!(is_kernel_addr(va)));
> > +}
> > +
> > +/*
> > + * Check for virtual address in kernel space.
> > + */
> > +static int arch_check_va_in_kernelspace(unsigned long va)
> > +{
> > + return is_kernel_addr(va);
> > +}
>
> You don't need these two routines. Just use is_kernel_addr() directly,
> that way people will know what the code is doing without having to
> lookup these new functions.
>
These functions are here due to 'legacy' reasons (from x86 code) , but I
see that it can be made a lot simpler by using is_kernel() for PPC64. I
will change the code on the lines of what you've suggested below.
But on processors supporting variable range of addresses, the above
functions may be required. Well, it can be added when required!
> > +/*
> > + * Store a breakpoint's encoded address, length, and type.
> > + */
>
> This doesn't "store" in the sense I was thinking, it actually does a
> lookup and returns info in the arg.
>
> > +int arch_store_info(struct hw_breakpoint *bp)
> > +{
> > + /*
> > + * User-space requests will always have the address field populated
> > + * For kernel-addresses, either the address or symbol name can be
> > + * specified.
> > + */
>
> Do user-space requests never have the name populated? Otherwise aren't
> you overwriting the supplied address with the one from kallsyms?
>
I see that I should also check for an empty bp->info.name when "if (tsk)"
is TRUE. I will add the same.
> > + 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 ret = -EINVAL;
> > +
> > + if (!bp)
> > + return ret;
> > +
> > + switch (bp->info.type) {
> > + case DABR_DATA_READ:
> > + break;
> > + case DABR_DATA_WRITE:
> > + break;
> > + case DABR_DATA_RW:
> > + break;
>
> You only need the final break here.
>
Yes, will change.
> > + default:
> > + return ret;
> > + }
> > +
> > + if (bp->triggered)
> > + ret = arch_store_info(bp);
>
> Shouldn't you check ret here, bp->info.address might be bogus.
Only when bp->info.name is set for user-space. The additional check I
described above should help.
> > +
> > + /* Check for double word alignment - 8 bytes */
> > + if (bp->info.address & HW_BREAKPOINT_ALIGN)
> > + return -EINVAL;
> > +
> > + /* Check that the virtual address is in the proper range */
> > + if (tsk) {
> > + if (!arch_check_va_in_userspace(bp->info.address))
> > + return -EFAULT;
> > + } else {
> > + if (!arch_check_va_in_kernelspace(bp->info.address))
> > + return -EFAULT;
> > + }
>
> Which becomes:
>
> is_kernel = is_kernel_addr(bp->info.address);
> if (tsk && is_kernel || !tsk && !is_kernel)
> return -EFAULT;
>
Ok. I will change them.
> > +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 | bp->info.type |
> > + DABR_TRANSLATION;
>
> 2nd place I've seen that pattern.
>
I don't recognise what issue you're referring to. Can you elaborate?
> > + 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;
> > + int cpu, stepped, is_kernel;
> > +
> > + /* Disable breakpoints during exception handling */
> > + set_dabr(0);
> > +
> > + dar = regs->dar & (~HW_BREAKPOINT_ALIGN);
> > + is_kernel = (dar >= TASK_SIZE) ? 1 : 0;
>
> is_kernel_addr() ?
>
Ok.
> > + if (is_kernel)
> > + bp = hbp_kernel[0];
> > + else {
> > + bp = current->thread.hbp[0];
> > + /* Lazy debug register switching */
> > + if (!bp)
> > + return rc;
>
> What if we keep hitting this case?
>
In short - no, they will not recur because we are returning after a
set_dabr(0).
The reason a breakpoint exception can trigger because of lazy debug
register switching is because the DABR value is not cleared when the
incoming process does not have TIF_DEBUG set i.e. does not use
breakpoint register. If the outgoing process used DABR while the
incoming process did not, and happened to read/write on the same address
such an exception can arise. It is only then that the DABR value is
cleared.
> > + rc = NOTIFY_DONE;
> > + }
> > +
> > + (bp->triggered)(bp, regs);
> > +
> > + cpu = get_cpu();
> > + if (is_kernel)
> > + per_cpu(dabr_data, cpu) = kdabr;
> > + else
> > + per_cpu(dabr_data, cpu) = current->thread.dabr;
> > +
> > + 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));
> > +out:
> > + /* Enable pre-emption only if single-stepping is finished */
> > + if (stepped)
> > + put_cpu_no_resched();
> > + return rc;
> > +}
>
> Gotta run, laptop battery running out! :)
>
> cheers
>
Thanks again for your reviews. I will post another patchset with the
changes I've agreed above.
Thanks,
K.Prasad
^ permalink raw reply
* Re: [RFC Patch 2/6] Introduce PPC64 specific Hardware Breakpointinterfaces
From: Alan Stern @ 2009-05-14 20:20 UTC (permalink / raw)
To: K.Prasad
Cc: Michael Neuling, Benjamin Herrenschmidt, linuxppc-dev, paulus,
Roland McGrath
In-Reply-To: <20090514195044.GD30963@in.ibm.com>
On Fri, 15 May 2009, K.Prasad wrote:
> I see that you're referring to this code in __switch_to() :
> if (unlikely(__get_cpu_var(current_dabr) != new->thread.dabr))
> set_dabr(new->thread.dabr);
>
> arch_install_thread_hw_breakpoint()<--switch_to_thread_hw_breakpoint()
> <--__switch_to() implementation is also similar.
>
> In __switch_to(),
> if (unlikely(test_tsk_thread_flag(new, TIF_DEBUG)))
> switch_to_thread_hw_breakpoint(new);
>
> happens only when TIF_DEBUG flag is set. This flag is cleared when the
> process unregisters any breakpoints it had requested earlier. So, the
> set_dabr() call is avoided for processes not using the debug register.
In the x86 code, shouldn't arch_update_user_hw_breakpoint set or clear
TIF_DEBUG, depending on whether or not there are any user breakpoints
remaining?
> > > +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;
> > > + int cpu, stepped, is_kernel;
> > > +
> > > + /* Disable breakpoints during exception handling */
> > > + set_dabr(0);
> > > +
> > > + dar = regs->dar & (~HW_BREAKPOINT_ALIGN);
> > > + is_kernel = (dar >= TASK_SIZE) ? 1 : 0;
> >
> > is_kernel_addr() ?
> >
>
> Ok.
Shouldn't this test hbp_kernel_pos instead?
> > > + if (is_kernel)
> > > + bp = hbp_kernel[0];
> > > + else {
> > > + bp = current->thread.hbp[0];
> > > + /* Lazy debug register switching */
> > > + if (!bp)
> > > + return rc;
Shouldn't this test be moved outside the "if" statement, as in the x86
code?
Alan Stern
^ permalink raw reply
* Re: [RFC Patch 6/6] Adapt kexec and samples code to recognise PPC64 hardware breakpoint usage
From: Alan Stern @ 2009-05-14 20:21 UTC (permalink / raw)
To: K.Prasad
Cc: linuxppc-dev, Michael Neuling, Benjamin Herrenschmidt, paulus,
Roland McGrath
In-Reply-To: <20090514134614.GG14229@in.ibm.com>
On Thu, 14 May 2009, K.Prasad wrote:
> Index: linux-2.6-tip.hbkpt/samples/hw_breakpoint/data_breakpoint.c
> ===================================================================
> --- linux-2.6-tip.hbkpt.orig/samples/hw_breakpoint/data_breakpoint.c 2009-05-14 00:17:24.000000000 +0530
> +++ linux-2.6-tip.hbkpt/samples/hw_breakpoint/data_breakpoint.c 2009-05-14 00:58:06.000000000 +0530
> @@ -54,6 +54,10 @@
> sample_hbp.info.type = HW_BREAKPOINT_WRITE;
> sample_hbp.info.len = HW_BREAKPOINT_LEN_4;
> #endif /* CONFIG_X86 */
> +#ifdef CONFIG_PPC64
> + sample_hbp.info.name = ksym_name;
> + sample_hbp.info.type = DABR_DATA_WRITE;
This should be HW_BREAKPOINT_WRITE, not DABR_DATA_WRITE.
Alan Stern
^ permalink raw reply
* [PATCH v2] powerpc/fsl: Setup PCI inbound window based on actual amount of memory
From: Kumar Gala @ 2009-05-14 21:00 UTC (permalink / raw)
To: linuxppc-dev
Previouslly we just always set the inbound window to 2G. This was
broken for systems with >2G. If a system has >=4G we will need
SWIOTLB support to handle that case.
We now allocate PCICSRBAR/PEXCSRBAR right below the lowest PCI outbound
address for MMIO or the 4G boundary (if the lowest PCI address is above
4G).
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
* Fixed up the warn/err msgs
* use a idx to keep track of which window we are setting up
- k
arch/powerpc/sysdev/fsl_pci.c | 130 ++++++++++++++++++++++++++++++++++------
arch/powerpc/sysdev/fsl_pci.h | 6 ++-
2 files changed, 115 insertions(+), 21 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index b20171d..0de91c6 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -23,6 +23,8 @@
#include <linux/string.h>
#include <linux/init.h>
#include <linux/bootmem.h>
+#include <linux/lmb.h>
+#include <linux/log2.h>
#include <asm/io.h>
#include <asm/prom.h>
@@ -96,7 +98,13 @@ static void __init setup_pci_atmu(struct pci_controller *hose,
struct resource *rsrc)
{
struct ccsr_pci __iomem *pci;
- int i, j, n;
+ int i, j, n, mem_log, win_idx = 2;
+ u64 mem, sz, paddr_hi = 0;
+ u64 paddr_lo = ULLONG_MAX;
+ u32 pcicsrbar = 0, pcicsrbar_sz;
+ u32 piwar = PIWAR_EN | PIWAR_PF | PIWAR_TGI_LOCAL |
+ PIWAR_READ_SNOOP | PIWAR_WRITE_SNOOP;
+ char *name = hose->dn->full_name;
pr_debug("PCI memory map start 0x%016llx, size 0x%016llx\n",
(u64)rsrc->start, (u64)rsrc->end - (u64)rsrc->start + 1);
@@ -117,6 +125,9 @@ static void __init setup_pci_atmu(struct pci_controller *hose,
if (!(hose->mem_resources[i].flags & IORESOURCE_MEM))
continue;
+ paddr_lo = min(paddr_lo, (u64)hose->mem_resources[i].start);
+ paddr_hi = max(paddr_hi, (u64)hose->mem_resources[i].end);
+
n = setup_one_atmu(pci, j, &hose->mem_resources[i],
hose->pci_mem_offset);
@@ -147,14 +158,105 @@ static void __init setup_pci_atmu(struct pci_controller *hose,
}
}
- /* Setup 2G inbound Memory Window @ 1 */
- out_be32(&pci->piw[2].pitar, 0x00000000);
- out_be32(&pci->piw[2].piwbar,0x00000000);
- out_be32(&pci->piw[2].piwar, PIWAR_2G);
+ /* convert to pci address space */
+ paddr_hi -= hose->pci_mem_offset;
+ paddr_lo -= hose->pci_mem_offset;
+
+ if (paddr_hi == paddr_lo) {
+ pr_err("%s: No outbound window space\n", name);
+ return ;
+ }
+
+ if (paddr_lo == 0) {
+ pr_err("%s: No space for inbound window\n", name);
+ return ;
+ }
+
+ /* setup PCSRBAR/PEXCSRBAR */
+ early_write_config_dword(hose, 0, 0, PCI_BASE_ADDRESS_0, 0xffffffff);
+ early_read_config_dword(hose, 0, 0, PCI_BASE_ADDRESS_0, &pcicsrbar_sz);
+ pcicsrbar_sz = ~pcicsrbar_sz + 1;
+
+ if (paddr_hi < (0x100000000ull - pcicsrbar_sz) ||
+ (paddr_lo > 0x100000000ull))
+ pcicsrbar = 0x100000000ull - pcicsrbar_sz;
+ else
+ pcicsrbar = (paddr_lo - pcicsrbar_sz) & -pcicsrbar_sz;
+ early_write_config_dword(hose, 0, 0, PCI_BASE_ADDRESS_0, pcicsrbar);
+
+ paddr_lo = min(paddr_lo, (u64)pcicsrbar);
- /* Save the base address and size covered by inbound window mappings */
- hose->dma_window_base_cur = 0x00000000;
- hose->dma_window_size = 0x80000000;
+ pr_info("%s: PCICSRBAR @ 0x%x\n", name, pcicsrbar);
+
+ /* Setup inbound mem window */
+ mem = lmb_end_of_DRAM();
+ sz = min(mem, paddr_lo);
+ mem_log = __ilog2_u64(sz);
+
+ /* PCIe can overmap inbound & outbound since RX & TX are separated */
+ if (early_find_capability(hose, 0, 0, PCI_CAP_ID_EXP)) {
+ /* Size window to exact size if power-of-two or one size up */
+ if ((1ull << mem_log) != mem) {
+ if ((1ull << mem_log) > mem)
+ pr_info("%s: Setting PCI inbound window "
+ "greater than memory size\n", name);
+ mem_log++;
+ }
+
+ piwar |= (mem_log - 1);
+
+ /* Setup inbound memory window */
+ out_be32(&pci->piw[win_idx].pitar, 0x00000000);
+ out_be32(&pci->piw[win_idx].piwbar, 0x00000000);
+ out_be32(&pci->piw[win_idx].piwar, piwar);
+ win_idx--;
+
+ hose->dma_window_base_cur = 0x00000000;
+ hose->dma_window_size = (resource_size_t)sz;
+ } else {
+ u64 paddr = 0;
+
+ /* Setup inbound memory window */
+ out_be32(&pci->piw[win_idx].pitar, paddr >> 12);
+ out_be32(&pci->piw[win_idx].piwbar, paddr >> 12);
+ out_be32(&pci->piw[win_idx].piwar, (piwar | (mem_log - 1)));
+ win_idx--;
+
+ paddr += 1ull << mem_log;
+ sz -= 1ull << mem_log;
+
+ if (sz) {
+ mem_log = __ilog2_u64(sz);
+ piwar |= (mem_log - 1);
+
+ out_be32(&pci->piw[win_idx].pitar, paddr >> 12);
+ out_be32(&pci->piw[win_idx].piwbar, paddr >> 12);
+ out_be32(&pci->piw[win_idx].piwar, piwar);
+ win_idx--;
+
+ paddr += 1ull << mem_log;
+ }
+
+ hose->dma_window_base_cur = 0x00000000;
+ hose->dma_window_size = (resource_size_t)paddr;
+ }
+
+ if (hose->dma_window_size < mem) {
+#ifndef CONFIG_SWIOTLB
+ pr_err("%s: ERROR: Memory size exceeds PCI ATMU ability to "
+ "map - enable CONFIG_SWIOTLB to avoid dma errors.\n",
+ name);
+#endif
+ /* adjusting outbound windows could reclaim space in mem map */
+ if (paddr_hi < 0xffffffffull)
+ pr_warning("%s: WARNING: Outbound window cfg leaves "
+ "gaps in memory map. Adjusting the memory map "
+ "could reduce unnecessary bounce buffering.\n",
+ name);
+
+ pr_info("%s: DMA window size is 0x%llx\n", name,
+ (u64)hose->dma_window_size);
+ }
iounmap(pci);
}
@@ -180,16 +282,6 @@ static void __init setup_pci_cmd(struct pci_controller *hose)
}
}
-static void __init setup_pci_pcsrbar(struct pci_controller *hose)
-{
-#ifdef CONFIG_PCI_MSI
- phys_addr_t immr_base;
-
- immr_base = get_immrbase();
- early_write_config_dword(hose, 0, 0, PCI_BASE_ADDRESS_0, immr_base);
-#endif
-}
-
void fsl_pcibios_fixup_bus(struct pci_bus *bus)
{
struct pci_controller *hose = (struct pci_controller *) bus->sysdata;
@@ -273,8 +365,6 @@ int __init fsl_add_bridge(struct device_node *dev, int is_primary)
/* Setup PEX window registers */
setup_pci_atmu(hose, &rsrc);
- /* Setup PEXCSRBAR */
- setup_pci_pcsrbar(hose);
return 0;
}
diff --git a/arch/powerpc/sysdev/fsl_pci.h b/arch/powerpc/sysdev/fsl_pci.h
index 13f30c2..a9d8bbe 100644
--- a/arch/powerpc/sysdev/fsl_pci.h
+++ b/arch/powerpc/sysdev/fsl_pci.h
@@ -16,7 +16,11 @@
#define PCIE_LTSSM 0x0404 /* PCIE Link Training and Status */
#define PCIE_LTSSM_L0 0x16 /* L0 state */
-#define PIWAR_2G 0xa0f5501e /* Enable, Prefetch, Local Mem, Snoop R/W, 2G */
+#define PIWAR_EN 0x80000000 /* Enable */
+#define PIWAR_PF 0x20000000 /* prefetch */
+#define PIWAR_TGI_LOCAL 0x00f00000 /* target - local memory */
+#define PIWAR_READ_SNOOP 0x00050000
+#define PIWAR_WRITE_SNOOP 0x00005000
/* PCI/PCI Express outbound window reg */
struct pci_outbound_window_regs {
--
1.6.0.6
^ permalink raw reply related
* Re: [PATCH] [PowerPC] MPC8272ADS: fix device tree for 8 MB flash, size
From: Scott Wood @ 2009-05-14 21:48 UTC (permalink / raw)
To: Heiko Schocher; +Cc: linuxppc-dev, Wolfgang Denk, linux-kernel
In-Reply-To: <4A0A683B.5010209@denx.de>
On Wed, May 13, 2009 at 08:27:07AM +0200, Heiko Schocher wrote:
> Hello Wolfgang,
>
> > The current device tree for the MPC8272ADS assumes a mapping of 32 MB
> > of NOR flash at 0xFE00.0000, while there are actually only 8 MB on
> > the boards, mapped at 0xFF80.0000. When booting an uImage with such a
> > device tree, the kernel crashes because 0xFE00.0000 is not mapped.
>
> Wouldn;t it be better, if u-boot fixes the device tree entries?
We should proabbly leave out the ranges altogether, and have u-boot
populate it from the mappings it establishes.
> I think, u-boot should know, where the flash begins and ends, and
> because this is maybe a dynamic variable for this board, it should
> be better, if u-boot fixes this, so no need for adding a device tree
> for every board variant.
Flash is on a SIMM on this board, and the board manual says it's
expandable to 32 MiB. However, I suspect that the current DTS was just
an error as I based it on a board that had not had its flash SIMM
modified. That specific flash SIMM is no longer working (or perhaps just
got its contents corrupted -- one of these days I may hook up a BDI and
try to reflash), so I can't go back and check.
I don't see how current u-boot would accomodate more than 8MiB flash on
this board (there's some detection in board/freescale/mpc8260ads/flash.c,
but I don't see any setting of BR0 besides the preliminary value at
0xff800000).
-Scott
^ 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