* Re: device trees.
From: David H. Lynch Jr. @ 2009-05-12 2:34 UTC (permalink / raw)
To: Stephen Neuendorffer, linuxppc-dev
In-Reply-To: <20090512005554.EEE1019D009B@mail129-dub.bigfish.com>
Stephen Neuendorffer wrote:
>
>> Many of our systems are LX systems but currently we are not running
>> Linux on them.
>>
>
> Master SelectMap, I presume? What FPGA family?
> Does the FPGA have access to the CPLD after boot, other than through the
> configuration pins?
>
>
One of the skills I have not had time to develop - because Pico has
plenty of qualified firmware/hardware people and not enough OS people,
is fluency with Xilinx tools.
FPGA Families - currently Spartan, virtex 4 and virtex 5.
LX & FX, in all kinds of sizes. So that as an example our E14 (1st
generation cardbus) comes in FX40's and FX60's and LX## .......
I am presuming that is what you mean.
Access to the hardware on the cards can be weird. As our cards are often
hosted, that means the firmware is often setup to allow host and target
access to hardware. In others only the host or only the target does.
What I know is that if I send a few magic values to the CPLD and then
start reading the bit file out of flash, I will trigger the CPLD to
reload the FPGA from the bitfile I selected.
> OK, so that means the boot monitor can open sector 2 of the flash and
> read info, right? (Or wherever else the bitstream is coming from.
On powerup the CPLD boots from sector 2. IF it is rebooted by the host
or target it can reboot from any flash sector.
In the "normal" setup the target has indirect access to the entire NOR
flash.
> Can
> the CPLD store one 32 bit int that the new bitstream can come back and
> read later?
>
Off the top of my head I do nto know, but if you are saying I need to
find someway of preserving a 32bit value through rebooting the FPGA I
can find a way to do it.
>
>
> OK, so the key question seems to be *when* the bitstream is associated
> with the
> device tree. If at bitstream generation time, you can prepend the .dtb
> to the bitstream. As long as the dtb doesn't contain the magic
> bitstream start code, you can go back and access it later.
>
You really mean prepend ? I was presuming that things would work better
if it was appended ?
Regardless, I have the means to know exactly what bit file is currently
loaded, and I can then look it up in the NOR Flash.
I can glue the dtb and the bit file together in anyway that will make
xilinx happy.
If it is prepended the only case I care about is the power up sequence,
because that must start loading the bit file at sector 2.
We do cope with the scenario where the sector 2 bit file is completely
screwed up. The CPLD STARTS trying to load at sector 2, but it will
continue to the end of flash until something actually loads. The dead
card scenario is sector 2 is the start of a valid but non-functional bit
file.
At that point you must load a bit image using JTAG and then write a good
bit file to flash.
Alright lets say I prepend. I am loosely familar with the magic start
code. Does that need to be aligned in anyway ?
And just for the sake of argument lets say I append the dtb. Do I need
some padding between the bitfile and the dtb to keep the
FPGA from loading the dtb as firmware ? Is there a magic stop sequence ?
Is the load terminated by the length of the bits.
> Is this sounding reasonable?
>
>
I am sure there is something I can work with here.
Thanks.
> Steve
>
> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
>
>
>
--
Dave Lynch DLA Systems
Software Development: Embedded Linux
717.627.3770 dhlii@dlasys.net http://www.dlasys.net
fax: 1.253.369.9244 Cell: 1.717.587.7774
Over 25 years' experience in platforms, languages, and technologies too numerous to list.
"Any intelligent fool can make things bigger and more complex... It takes a touch of genius - and a lot of courage to move in the opposite direction."
Albert Einstein
^ permalink raw reply
* Re: device trees.
From: Michael Ellerman @ 2009-05-12 2:37 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev, David H. Lynch Jr.
In-Reply-To: <20090511225631.GA15371@yookeroo.seuss>
[-- Attachment #1: Type: text/plain, Size: 2365 bytes --]
On Tue, 2009-05-12 at 08:56 +1000, David Gibson wrote:
> On Mon, May 11, 2009 at 05:38:16PM -0400, David H. Lynch Jr. wrote:
> > Grant Likely wrote:
> > >> Anyway, all I was looking for was a leg up on figuring out how to do
> > >> what I want with them. Rather than starting from scratch.
> > >> I am not looking to be convinced that I am approaching this all wrong.
> > >> If you are happy with what you have - great. I am not.
> > >> While I was not looking to restart a great debate over device trees
> > >> - I do not actually think they are a bad idea.
> > >>
> > >
> > > I still don't understand what you're worried about starting an arguing
> > > about. Pretty much any of the PowerPC maintainers can point at warts
> > > and problems in the current handling of device trees. I'm not
> > > particularly happy with simpleImage (and I wrote it), but it takes
> > > time and effort to write something more capable.
> > >
> > I was not trying to start an argument, my initial question was
> >
> > "Is there an example somewhere that shows building a device tree on the fly ?"
> >
> > The responses have questioned why I want to do that rather than how can I do that.
> > I was not actually seeking a debate over the merit of device trees, or u-boot, or libdft, or ....
>
> Probably the closest things to suitable examples here are
> arch/powerpc/kernel/prom_init.c, which builds the dtb from the "live"
> OF device tree. At the moment that's a bit of a special case, but
> there's no inherent reason that logic couldn't be moved to the
> bootwrapper.
>
> The other, not in the main tree, is Paul's restodt, which builds a
> device tree from old-style PReP residual information. I recall it was
> sent to the list as a userspace program (we were gathering information
> on the sorts of PReP out there). I think Paul made at least a first
> cut at fusing it into the bootwrapper, but I don't know if that ever
> got sent out to the list.
There's also the iseries code, which is not pretty, but shows a very
minimal example of constructing a device tree on the fly, some of it
based on static config and some that's not.
The routines for constructing DT nodes and properties is about 110
lines, and most of that is brackets.
arch/powerpc/platforms/iseries/dt.c
cheers
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* get data failed with DMA
From: Sauce.Cheng @ 2009-05-12 1:59 UTC (permalink / raw)
To: linuxppc-dev
i tried to fetch data from periphial memories with DMA
the flow as following
flags = claim_dma_lock();
disable_dma(DMA_CH);
clear_dma_ff();
set_dma_count(DMA_CH, 4);
set_dma_addr(DMA_CH, virt_to_bus(kv_buf));
release_dma_lock(flags);
enable_dma(DMA_CH);
the code hault at disable_dma(DMA_CH)
in addition, i defined DMA_CH as 0 for channel 0
'kv_buf' is an kernel space virtual address. i can red the correct value of
periphal from this address
i supposed the fault along with the configured of registers about DMA in
processer. but i am not sure.
give some suggestion about this, thanks!
--
View this message in context: http://www.nabble.com/get-data-failed-with-DMA-tp23494879p23494879.html
Sent from the linuxppc-dev mailing list archive at Nabble.com.
^ permalink raw reply
* Re: device trees.
From: David Gibson @ 2009-05-12 1:12 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev, David H. Lynch Jr.
In-Reply-To: <fa686aa40905111609l3feaf555taadf048714030a1@mail.gmail.com>
On Mon, May 11, 2009 at 05:09:27PM -0600, Grant Likely wrote:
> On Mon, May 11, 2009 at 3:38 PM, David H. Lynch Jr. <dhlii@dlasys.net> wrote:
> > Grant Likely wrote:
> >>
> >> What do you mean by "one size fits all solution?"
> >>
> >> The kernel doesn't care where the device tree comes from. All it
> >> cares about is that by the time the kernel is started the device tree
> >> must be fully formed and populated. It can be completely pre-canned
> >> (like simpleImage), it can be modified by firmware (like u-boot), or
> >> it can be generated from scratch (like with real OpenFirmware). There
> >> is lots of flexibility on how to handle it.
> >>
> >>
> > First device trees are now the ONLY means of passing information to the
> > kernel.
> > By definition that means it is a one size fits all solution.
> > While there is nothing inherently wrong with that, solutions intended to
> > meet all circumstances need to be
> > simple, powerful, and flexible. They need to work well 100% of the time
> > not 98%.
> >
> > Not only is the device tree expected to pass static hardware
> > configuration information, but it is the sole means of passing anything.
> > As an example Command lines are to be in the device tree.
> > Everything is supposed to be in the device tree, whether that
> > information is static or dynamic, whether it is hardware information,
> > or user choices.
>
> It is the sole means of passing anything *to the kernel*. You can
> pass whatever you like to the bootwrapper. :-)
>
> > That means that whether you are in a Sun or Apple Desktop or a
> > system with the no flash and barely enough resources to run Linux,
> > you still may have to manipulate the device tree.
>
> ...or if you really are constrained, then define a format for the data
> you want to pass, pass it to the bootwrapper along with the device
> tree, and use the bootwrapper (which already has libfdt) to update the
> device tree. cuImage targets do this to support older u-boots which
> don't understand device trees. You are not forced to put device tree
> support in your firmware.
>
> In other words; having your bootloader support FDT is preferred, but
> not required.
I wouldn't even go so far as to say it's preferred. IMO, people have
gone a bit prematurely keen on moving devtree handling into the
firmware. Putting it in the firmware has a number of advantages, but
it also has a number of non-trivial disadvantages.
--
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: [RFC] Hardware Breakpoint interfaces implementation for PPC64
From: Michael Neuling @ 2009-05-12 0:56 UTC (permalink / raw)
To: prasad; +Cc: linuxppc-dev, Benjamin Herrenschmidt, paulus
In-Reply-To: <20090511200355.GA17988@in.ibm.com>
> Hi PPC Dev folks,
> Please find a patch below that implements the proposed Hardware
> Breakpoint interfaces for PPC64 architecture.
>
> As a brief introduction, the proposed Hardware Breakpoint
> infrastructure provides generic in-kernel interfaces to which users
> from kernel- and user-space can request for breakpoint registers. An
> ftrace plugin that can trace accesses to data variables is also part
> of the generic HW Breakpoint interface patchset. The latest submission
> for this patchset along with an x86 implementation can be found here:
> http://lkml.org/lkml/2009/5/11/159.
>
> The following are the salient features of the PPC64 patch.
>
> - Arch-specific definitions for kernel and user-space requests are
> defined in arch/powerpc/kernel/hw_breakpoint.c
> - Ptrace is converted to use the HW Breakpoint interfaces
Will we fall back to use the old method if more than one address is
being watched?
> - The ftrace plugin called ksym_tracer is tested to work fine. For
> instance when tracing pid_max kernel variable, the following was
> obtained as output
Could you split the patch into a few pieces which implement these
different parts. The smaller logical chucks will make it easier to
review?
>
> # cat trace
> # tracer: ksym_tracer
> #
> # TASK-PID CPU# Symbol Type Function
> # | | | | |
> bash 4502 3 pid_max RW .do_proc_dointvec_minmax_
conv+0x78/0x10c
> bash 4502 3 pid_max RW .do_proc_dointvec_minmax_
conv+0xa0/0x10c
> bash 4502 3 pid_max RW .alloc_pid+0x8c/0x4a4
>
> There are however a few limitations/caveats of the patch as identified
> below:
>
> - The patch is currently implemented only for PPC64 architecture. Other
> architectures (especially Book-E implementations are expected to
> happen in due course).
>
> - HW Breakpoints over data addresses through Xmon (using "bd" command)
> and the proposed HW Breakpoint interfaces can now operate in a
> mutually exclusive manner. Xmon's integration is pending and is
> dependant on successful triggering of breakpoints through "bd<ops>".
> (Note: On a Power5 machine running 2.6.29, Xmon could not trigger HW
> Breakpoints when tested).
>
> Kindly let me know your comments.
>
> Thanks,
> K.Prasad
>
>
> Signed-off-by: K.Prasad <prasad@linux.vnet.ibm.com>
> ---
> arch/powerpc/Kconfig | 1
> arch/powerpc/include/asm/hw_breakpoint.h | 52 +++++
> arch/powerpc/include/asm/processor.h | 1
> arch/powerpc/include/asm/reg.h | 2
> arch/powerpc/include/asm/thread_info.h | 2
> arch/powerpc/kernel/Makefile | 2
> arch/powerpc/kernel/hw_breakpoint.c | 271 ++++++++++++++++++++++++++++
+++
> arch/powerpc/kernel/process.c | 18 ++
> arch/powerpc/kernel/ptrace.c | 48 +++++
> arch/powerpc/mm/fault.c | 14 -
> samples/hw_breakpoint/data_breakpoint.c | 4
> 12 files changed, 423 insertions(+), 9 deletions(-)
You've not touched prace32.c. Can we use this for 32 bit apps on a 64
bit kernel?
>
> Index: linux-2.6-tip.hbkpt/arch/powerpc/Kconfig
> ===================================================================
> --- linux-2.6-tip.hbkpt.orig/arch/powerpc/Kconfig
> +++ linux-2.6-tip.hbkpt/arch/powerpc/Kconfig
> @@ -125,6 +125,7 @@ config PPC
> select USE_GENERIC_SMP_HELPERS if SMP
> select HAVE_OPROFILE
> select HAVE_SYSCALL_WRAPPERS if PPC64
> + select HAVE_HW_BREAKPOINT if PPC64
>
> config EARLY_PRINTK
> bool
> Index: linux-2.6-tip.hbkpt/arch/powerpc/kernel/Makefile
> ===================================================================
> --- linux-2.6-tip.hbkpt.orig/arch/powerpc/kernel/Makefile
> +++ linux-2.6-tip.hbkpt/arch/powerpc/kernel/Makefile
> @@ -33,7 +33,7 @@ obj-$(CONFIG_PPC64) += setup_64.o sys_p
> signal_64.o ptrace32.o \
> paca.o cpu_setup_ppc970.o \
> cpu_setup_pa6t.o \
> - firmware.o nvram_64.o
> + firmware.o nvram_64.o hw_breakpoint.o
> obj64-$(CONFIG_RELOCATABLE) += reloc_64.o
> obj-$(CONFIG_PPC64) += vdso64/
> obj-$(CONFIG_ALTIVEC) += vecemu.o vector.o
> 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,271 @@
> +/*
> + * 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
> + */
> +
> +/*
> + * HW_breakpoint: a unified kernel/user-space hardware breakpoint facility,
> + * using the CPU's debug registers.
> + */
> +
> +#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);
> +
> +void arch_update_kernel_hw_breakpoint(void *unused)
> +{
> + struct hw_breakpoint *bp;
> +
> + /* Check if there is nothing to update */
> + if (hbp_kernel_pos == HB_NUM)
> + return;
> + 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);
> +}
> +
> +/*
> + * Install the debug register values for just the kernel, no thread.
> + */
> +void arch_uninstall_thread_hw_breakpoint()
> +{
> + set_dabr(0);
> +}
> +
> +/*
> + * Check for virtual address in user space.
> + */
> +int arch_check_va_in_userspace(unsigned long va, u8 hbp_len)
> +{
> + return (va <= TASK_SIZE - HW_BREAKPOINT_LEN);
> +}
You pass ing hbp_len, but then use HW_BREAKPOINT_LEN? Is that right?
> +
> +/*
> + * Check for virtual address in kernel space.
> + */
> +int arch_check_va_in_kernelspace(unsigned long va, u8 hbp_len)
> +{
> + return (va >= TASK_SIZE) && ((va + HW_BREAKPOINT_LEN - 1) >= TASK_SIZE)
;
Can you use is_kernel_addr() here?
> +}
> +
> +/*
> + * Store a breakpoint's encoded address, length, and type.
> + */
> +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.
> + */
> + 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;
> + default:
> + return ret;
> + }
> +
> + if (bp->triggered)
> + ret = arch_store_info(bp);
> +
> + /* Check for double word alignment - 8 bytes */
> + if (bp->info.address & HW_BREAKPOINT_ALIGN)
> + 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 | 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;
> + 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;
> +
> + if (is_kernel)
> + bp = hbp_kernel[0];
> + else {
> + bp = current->thread.hbp[0];
> + /* Lazy debug register switching */
> + if (!bp)
> + return rc;
> + 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;
> + }
This is where we backout to single step mode?
> +
> + 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;
> +}
> +
+/*
> + * 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);
> + }
> +
> + set_dabr(this_dabr_data);
> + per_cpu(dabr_data, cpu) = 0;
> + ret = NOTIFY_STOP;
> + put_cpu_no_resched();
> +
> +out:
> + put_cpu_no_resched();
This looks wrong. put_cpu_no_resched() twice?
> + 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;
> +}
> 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,52 @@
> +#ifndef _PPC64_HW_BREAKPOINT_H
> +#define _PPC64_HW_BREAKPOINT_H
> +
> +#ifdef __KERNEL__
> +#define __ARCH_HW_BREAKPOINT_H
> +
> +struct arch_hw_breakpoint {
> + char *name; /* Contains name of the symbol to set bkpt */
> + unsigned long address;
> + u8 type;
> +};
Can you reorder this to pack the struct better (ie. put the unsigned
long first).
> +
> +#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_RW
> +
> +#define HW_BREAKPOINT_ALIGN 0x7
> +#define HW_BREAKPOINT_LEN 4
What is HW_BREAKPOINT_LEN?
Obviouslt all instructions on ppc64 are 4 bytes. This seems to be
defined in a few places, like here and MCOUNT_INSN_SIZE (ftrace.h). Can
you create a #define for this generically and get all refers to use this
instead of #define-ing 4 everywhere.
> +
> +extern struct hw_breakpoint *hbp_kernel[HB_NUM];
> +extern unsigned int hbp_user_refcount[HB_NUM];
> +
> +/*
> + * Ptrace support: breakpoint trigger routine.
> + */
> +extern int __modify_user_hw_breakpoint(int pos, struct task_struct *tsk,
> + struct hw_breakpoint *bp);
> +
> +extern void arch_install_thread_hw_breakpoint(struct task_struct *tsk);
> +extern void arch_uninstall_thread_hw_breakpoint(void);
> +extern int arch_check_va_in_userspace(unsigned long va, u8 hbp_len);
> +extern int arch_check_va_in_kernelspace(unsigned long va, u8 hbp_len);
> +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 switch_to_thread_hw_breakpoint(struct task_struct *tsk);
> +
> +#endif /* __KERNEL__ */
> +#endif /* _PPC64_HW_BREAKPOINT_H */
> +
> Index: linux-2.6-tip.hbkpt/arch/powerpc/include/asm/processor.h
> ===================================================================
> --- linux-2.6-tip.hbkpt.orig/arch/powerpc/include/asm/processor.h
> +++ linux-2.6-tip.hbkpt/arch/powerpc/include/asm/processor.h
> @@ -177,6 +177,7 @@ struct thread_struct {
> #ifdef CONFIG_PPC64
> unsigned long start_tb; /* Start purr when proc switched in */
> unsigned long accum_tb; /* Total accumilated purr for process *
/
> + struct hw_breakpoint *hbp[HB_NUM];
> #endif
> unsigned long dabr; /* Data address breakpoint register */
> #ifdef CONFIG_ALTIVEC
> Index: linux-2.6-tip.hbkpt/arch/powerpc/kernel/ptrace.c
> ===================================================================
> --- 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
>
> /*
> * does not yet catch signals sent when the child dies.
> @@ -735,9 +738,22 @@ void user_disable_single_step(struct tas
> clear_tsk_thread_flag(task, TIF_SINGLESTEP);
> }
>
> +static void ptrace_triggered(struct hw_breakpoint *bp, struct pt_regs *regs)
> +{
> + /*
> + * The SIGTRAP signal is generated automatically for us in do_dabr().
> + * We don't have to do anything here
> + */
> +}
> +
> int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
> unsigned long data)
> {
> +#ifdef CONFIG_PPC64
> + struct thread_struct *thread = &(task->thread);
> + struct hw_breakpoint *bp;
> + int ret;
> +#endif
> /* For ppc64 we support one DABR and no IABR's at the moment (ppc64).
> * For embedded processors we support one DAC and no IAC's at the
> * moment.
> @@ -767,6 +783,38 @@ int ptrace_set_debugreg(struct task_stru
> if (data && !(data & DABR_TRANSLATION))
> return -EIO;
>
> +#ifdef CONFIG_PPC64
> + bp = thread->hbp[0];
> + if ((data & ~0x7UL) == 0) {
Should use HW_BREAKPOINT_ALIGN here instead of 0x7? Is 0 some special
command? Seems to be lots of special numbers here related to using data
(later you mask is 0x3). What is happening here? What is contained in
the data parameter? It overloads the type and the address?
> + if (bp) {
> + unregister_user_hw_breakpoint(task, bp);
> + kfree(bp);
> + thread->hbp[0] = NULL;
> + }
> + return 0;
> + }
> +
> + if (bp) {
> + bp->info.type = data & 0x3UL;
> + task->thread.dabr = bp->info.address =
> + (data & ~HW_BREAKPOINT_ALIGN);
> + return __modify_user_hw_breakpoint(0, task, bp);
> + }
> + bp = kzalloc(sizeof(struct hw_breakpoint), GFP_KERNEL);
When a processes ends, will this be correctly freed? Should it be freed
in arch_flush_thread_hw_breakpoint?
> + if (!bp)
> + return -ENOMEM;
> +
> + /* Store the type of breakpoint */
> + bp->info.type = data & 0x3UL;
> + bp->triggered = ptrace_triggered;
> + task->thread.dabr = bp->info.address = (data & ~HW_BREAKPOINT_ALIGN);
> +
> + ret = register_user_hw_breakpoint(task, bp);
> + if (ret)
> + return ret;
> + set_tsk_thread_flag(task, TIF_DEBUG);
> +#endif /* CONFIG_PPC64 */
> +
> /* Move contents to the DABR register */
> task->thread.dabr = data;
>
> 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
> 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;
> +#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
> Index: linux-2.6-tip.hbkpt/samples/hw_breakpoint/data_breakpoint.c
> ===================================================================
> --- linux-2.6-tip.hbkpt.orig/samples/hw_breakpoint/data_breakpoint.c
> +++ linux-2.6-tip.hbkpt/samples/hw_breakpoint/data_breakpoint.c
> @@ -54,6 +54,10 @@ static int __init hw_break_module_init(v
> 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;
>
> Index: linux-2.6-tip.hbkpt/arch/powerpc/include/asm/thread_info.h
> ===================================================================
> --- linux-2.6-tip.hbkpt.orig/arch/powerpc/include/asm/thread_info.h
> +++ linux-2.6-tip.hbkpt/arch/powerpc/include/asm/thread_info.h
> @@ -114,6 +114,7 @@ static inline struct thread_info *curren
> #define TIF_FREEZE 14 /* Freezing for suspend */
> #define TIF_RUNLATCH 15 /* Is the runlatch enabled? */
> #define TIF_ABI_PENDING 16 /* 32/64 bit switch needed */
> +#define TIF_DEBUG 17 /* uses debug registers */
>
> /* as above, but as bit values */
> #define _TIF_SYSCALL_TRACE (1<<TIF_SYSCALL_TRACE)
> @@ -132,6 +133,7 @@ static inline struct thread_info *curren
> #define _TIF_FREEZE (1<<TIF_FREEZE)
> #define _TIF_RUNLATCH (1<<TIF_RUNLATCH)
> #define _TIF_ABI_PENDING (1<<TIF_ABI_PENDING)
> +#define _TIF_DEBUG (1<<TIF_DEBUG)
> #define _TIF_SYSCALL_T_OR_A (_TIF_SYSCALL_TRACE|_TIF_SYSCALL_AUDIT|_TIF_SEC
COMP)
>
> #define _TIF_USER_WORK_MASK (_TIF_SIGPENDING | _TIF_NEED_RESCHED | \
> Index: linux-2.6-tip.hbkpt/arch/powerpc/include/asm/reg.h
> ===================================================================
> --- linux-2.6-tip.hbkpt.orig/arch/powerpc/include/asm/reg.h
> +++ linux-2.6-tip.hbkpt/arch/powerpc/include/asm/reg.h
> @@ -184,9 +184,11 @@
> #define CTRL_TE 0x00c00000 /* thread enable */
> #define CTRL_RUNLATCH 0x1
> #define SPRN_DABR 0x3F5 /* Data Address Breakpoint Register */
> +#define HB_NUM 1 /* Number of physical HW breakpoint registers *
/
You've shortedned "hardware breakpoint" to "hbp" elsewhere. Can you be
consistent here so change this to HBP_NUM?
> #define DABR_TRANSLATION (1UL << 2)
> #define DABR_DATA_WRITE (1UL << 1)
> #define DABR_DATA_READ (1UL << 0)
> +#define DABR_DATA_RW (3UL << 0)
> #define SPRN_DABR2 0x13D /* e300 */
> #define SPRN_DABRX 0x3F7 /* Data Address Breakpoint Register Extension *
/
> #define DABRX_USER (1UL << 0)
> Index: linux-2.6-tip.hbkpt/arch/powerpc/mm/fault.c
> ===================================================================
> --- linux-2.6-tip.hbkpt.orig/arch/powerpc/mm/fault.c
> +++ linux-2.6-tip.hbkpt/arch/powerpc/mm/fault.c
> @@ -137,6 +137,12 @@ int __kprobes do_page_fault(struct pt_re
> error_code &= 0x48200000;
> else
> is_write = error_code & DSISR_ISSTORE;
> +
> + if (error_code & DSISR_DABRMATCH) {
> + /* DABR match */
> + do_dabr(regs, address, error_code);
> + return 0;
> + }
> #else
> is_write = error_code & ESR_DST;
> #endif /* CONFIG_4xx || CONFIG_BOOKE */
> @@ -151,14 +157,6 @@ int __kprobes do_page_fault(struct pt_re
> if (!user_mode(regs) && (address >= TASK_SIZE))
> return SIGSEGV;
>
> -#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE))
> - if (error_code & DSISR_DABRMATCH) {
> - /* DABR match */
> - do_dabr(regs, address, error_code);
> - return 0;
> - }
> -#endif /* !(CONFIG_4xx || CONFIG_BOOKE)*/
> -
> if (in_atomic() || mm == NULL) {
> if (!user_mode(regs))
> return SIGSEGV;
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>
^ permalink raw reply
* Re: device trees.
From: David Gibson @ 2009-05-11 23:27 UTC (permalink / raw)
To: David H. Lynch Jr.; +Cc: linuxppc-dev, Timur Tabi
In-Reply-To: <4A085862.5070303@dlasys.net>
On Mon, May 11, 2009 at 12:54:58PM -0400, David H. Lynch Jr. wrote:
> Timur Tabi wrote:
> > On Mon, May 11, 2009 at 1:32 AM, David H. Lynch Jr. <dhlii@dlasys.net> wrote:
> >
> > So all you need to do is have your boot loader create a device tree
> > from scratch. If you're using U-Boot, then you can already do this by
> > making the appropriate libfdt calls. Otherwise, you should probably
> > add libfdt to your boot loader.
> >
> As I mentioned before, we do nto use u-boot. I am not looking to
> start a debate on it either, but it does not meet a number of our needs,
> and would require significant architectural changes to do so. The
> difference between it and devicetrees is that u-boot is avaiable to us
> if we want, I did port u-boot to our hardware at one point and it did
> everything it promised, but u-boot is optional, device trees are not.
> I do not have to re-architect u-boot to fit into 16k of bram, or
> load bit files or .....
> If I want to move past 2.6.26 I have to not only use device trees
> but actually make them work in a way that will function as we need with
> our systems.
> It is likely I will use libdft as a starting point, but I can not
> see it as more than a short term solution. libdft is orders of magnitude
> large than our entire monitor, and it is a toolkit rather than the whole
> solution.
I'm very willing to make changes to libfdt, if it helps make it more
useful to you without breaking it for other people. I'm a little
horrified at the size of libfdt, and a bit baffled as to why it's
ending up that large. From some initial poking around, it looks like
-fPIC inflates the code size a *lot* (~3x on x86, which I know is not
what we're dealing with here, but was the easiest for a quick test).
So if you don't need that in your context, that could save a heap.
The separation into separate .o files is also deliberate, so that
portions you don't use can be omitted. I have considered breaking it
up into many more .c files, so that you can omit individual not-needed
functions.
--
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: device trees.
From: David Gibson @ 2009-05-11 22:56 UTC (permalink / raw)
To: David H. Lynch Jr.; +Cc: linuxppc-dev
In-Reply-To: <4A089AC8.9080704@dlasys.net>
On Mon, May 11, 2009 at 05:38:16PM -0400, David H. Lynch Jr. wrote:
> Grant Likely wrote:
> >
> > What do you mean by "one size fits all solution?"
> >
> > The kernel doesn't care where the device tree comes from. All it
> > cares about is that by the time the kernel is started the device tree
> > must be fully formed and populated. It can be completely pre-canned
> > (like simpleImage), it can be modified by firmware (like u-boot), or
> > it can be generated from scratch (like with real OpenFirmware). There
> > is lots of flexibility on how to handle it.
> First device trees are now the ONLY means of passing information to
> the kernel.
That's not really true in practice. Yes, the device tree is the only
way to pass information to the kernel proper, but having a
bootwrapper, built along with the kernel, which translates information
in some other form into a device tree is a perfectly reasonable
solution for the right circumstances.
> By definition that means it is a one size fits all
> solution. While there is nothing inherently wrong with that,
> solutions intended to meet all circumstances need to be simple,
> powerful, and flexible. They need to work well 100% of the time not
> 98%.
>
> Not only is the device tree expected to pass static hardware
> configuration information, but it is the sole means of passing anything.
> As an example Command lines are to be in the device tree.
> Everything is supposed to be in the device tree, whether that
> information is static or dynamic, whether it is hardware information,
> or user choices.
>
> That means that whether you are in a Sun or Apple Desktop or a
> system with the no flash and barely enough resources to run Linux,
> you still may have to manipulate the device tree.
Compared to running Linux, manipulating the device tree is a complete
triviality, so I don't see what the problem is here.
[snip]
> Welding the bit file to the dtb might solve 75% of my issues,
> And it probably would get me to the point where I could move
> forward and live with
> the remaining issues untile I was inspired to solve them.
> but it does not solve everything. It is increasingly clear to me
> that I am going to have to
> manipulate device trees.
That's probably true. But you don't necessarily have to do it within
your BRAM firmware - you can do it inside the Linux bootwrapper.
> >> Anyway, all I was looking for was a leg up on figuring out how to do
> >> what I want with them. Rather than starting from scratch.
> >> I am not looking to be convinced that I am approaching this all wrong.
> >> If you are happy with what you have - great. I am not.
> >> While I was not looking to restart a great debate over device trees
> >> - I do not actually think they are a bad idea.
> >>
> >
> > I still don't understand what you're worried about starting an arguing
> > about. Pretty much any of the PowerPC maintainers can point at warts
> > and problems in the current handling of device trees. I'm not
> > particularly happy with simpleImage (and I wrote it), but it takes
> > time and effort to write something more capable.
> >
> I was not trying to start an argument, my initial question was
>
> "Is there an example somewhere that shows building a device tree on the fly ?"
>
> The responses have questioned why I want to do that rather than how can I do that.
> I was not actually seeking a debate over the merit of device trees, or u-boot, or libdft, or ....
Probably the closest things to suitable examples here are
arch/powerpc/kernel/prom_init.c, which builds the dtb from the "live"
OF device tree. At the moment that's a bit of a special case, but
there's no inherent reason that logic couldn't be moved to the
bootwrapper.
The other, not in the main tree, is Paul's restodt, which builds a
device tree from old-style PReP residual information. I recall it was
sent to the list as a userspace program (we were gathering information
on the sorts of PReP out there). I think Paul made at least a first
cut at fusing it into the bootwrapper, but I don't know if that ever
got sent out to the list.
--
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: question about softirqs
From: Chris Friesen @ 2009-05-12 0:43 UTC (permalink / raw)
To: David Miller; +Cc: linuxppc-dev, Ingo Molnar, paulus, netdev
In-Reply-To: <20090511.162436.193717082.davem@davemloft.net>
This started out as a thread on the ppc list, but on the suggestion of
DaveM and Paul Mackerras I'm expanding the receiver list a bit.
Currently, if a softirq is raised in process context the
TIF_RESCHED_PENDING flag gets set and on return to userspace we run the
scheduler, expecting it to switch to ksoftirqd to handle the softirqd
processing.
I think I see a possible problem with this. Suppose I have a SCHED_FIFO
task spinning on recvmsg() with MSG_DONTWAIT set. Under the scenario
above, schedule() would re-run the spinning task rather than ksoftirqd,
thus preventing any incoming packets from being sent up the stack until
we get a real hardware interrupt--which could be a whole jiffy if
interrupt mitigation is enabled in the net device.
DaveM pointed out that if we're doing transmits we're likely to hit
local_bh_enable(), which would process the softirq work. However, I
think we may still have a problem in the above rx-only scenario--or is
it too contrived to matter?
Thanks,
Chris
^ permalink raw reply
* Re: device trees.
From: Grant Likely @ 2009-05-12 0:04 UTC (permalink / raw)
To: Stephen Neuendorffer; +Cc: linuxppc-dev, David H. Lynch Jr.
In-Reply-To: <20090511231915.007AC1430052@mail201-wa4.bigfish.com>
On Mon, May 11, 2009 at 5:19 PM, Stephen Neuendorffer
<stephen.neuendorffer@xilinx.com> wrote:
>
>> >> =A0 =A0The best alternative to creating the device tree dynamically
> would
>> >> be to
>> >> =A0 =A0append the devicetree to the bitimage in a way the boot loader
> could
>> >> always find it.
>> >>
>> >
>> > That sounds like a good solution to me.
>> >
>> =A0 =A0 I am glad you like it. If Xilinx would like to offer any advice =
as
>> to how to prepend a device tree to the end of a bit file without
>> =A0 =A0 foreclosing any of their future plans or .... I would be happy t=
o
>> look at implementing it.
>
> There are options here...
>
> For the benefit of the mailing list: they are tied into how the FPGA
> configuration is loaded, and the
> particular FPGA family. =A0By far the most portable/generic option is to
> simply use an initialized BRAM for this.
>
> David: If you would like to have a discussion regarding particular
> design tradeoffs, I'd be happy to, but since I doubt there is anyone on
> this list who is interested in the vagaries of FPGA configuration
> methods, I suggest we have the discussion privately.
I disagree. There are lots of lurkers on this list who care about
virtex stuff, not to mention non-lurkers like me. :-) I would at
least like to be a fly on the wall of any such discussion.
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* RE: device trees.
From: Stephen Neuendorffer @ 2009-05-11 23:19 UTC (permalink / raw)
To: David H. Lynch Jr., grant.likely; +Cc: linuxppc-dev
In-Reply-To: <4A089AC8.9080704@dlasys.net>
> >> The best alternative to creating the device tree dynamically
would
> >> be to
> >> append the devicetree to the bitimage in a way the boot loader
could
> >> always find it.
> >>
> >
> > That sounds like a good solution to me.
> >
> I am glad you like it. If Xilinx would like to offer any advice as
> to how to prepend a device tree to the end of a bit file without
> foreclosing any of their future plans or .... I would be happy to
> look at implementing it.
There are options here...
For the benefit of the mailing list: they are tied into how the FPGA
configuration is loaded, and the
particular FPGA family. By far the most portable/generic option is to
simply use an initialized BRAM for this.
David: If you would like to have a discussion regarding particular
design tradeoffs, I'd be happy to, but since I doubt there is anyone on
this list who is interested in the vagaries of FPGA configuration
methods, I suggest we have the discussion privately.
Steve
This email and any attachments are intended for the sole use of the named r=
ecipient(s) and contain(s) confidential information that may be proprietary=
, privileged or copyrighted under applicable law. If you are not the intend=
ed recipient, do not read, copy, or forward this email message or any attac=
hments. Delete this email message and any attachments immediately.
^ permalink raw reply
* Re: question about softirqs
From: Paul Mackerras @ 2009-05-11 23:34 UTC (permalink / raw)
To: Chris Friesen; +Cc: linuxppc-dev, David Miller
In-Reply-To: <4A086DB2.8040703@nortel.com>
Chris Friesen writes:
> Suppose I have a SCHED_FIFO task spinning on recvmsg() with MSG_DONTWAIT
> set (and maybe doing other stuff if there are no messages). In this
> case, schedule() would re-run the spinning task rather than running
> ksoftirqd. This could prevent any incoming packets from actually being
> sent up the stack until we get a real hardware interrupt--which could be
> a whole jiffy if interrupt mitigation is enabled in the net device.
I suggest you ask Ingo Molnar about that.
> (And maybe longer if NOHZ is enabled.)
We still have a timer interrupt every jiffy when stuff is running; we
only turn off the timer interrupts when idle.
Paul.
^ permalink raw reply
* Re: question about softirqs
From: David Miller @ 2009-05-11 23:24 UTC (permalink / raw)
To: cfriesen; +Cc: linuxppc-dev, paulus
In-Reply-To: <4A086DB2.8040703@nortel.com>
From: "Chris Friesen" <cfriesen@nortel.com>
Date: Mon, 11 May 2009 12:25:54 -0600
> David Miller wrote:
>
>> You know, for networking over loopback (one of the only real cases
>> that even matters, if we get a hard interrupt then the return from
>> that would process any softints), we probably make out just fine
>> anyways. As long as we hit a local_bh_enable() (and in the return
>> path from device transmit that's exceedingly likely as all of the
>> networking locking is BH safe) we'll run the softints from that and
>> thus long before we get to syscall return.
>
> What about the issue I raised earlier? (I don't think you were copied
> at that point.)
I'm sure all of the networking experts on linuxppc-dev will have
an answer.
And yes that was sarcasm :-) You need to ask this on netdev or similar
list.
^ permalink raw reply
* Re: [PATCH] i2c-cpm: Pass dev ptr to dma_*_coherent rather than NULL
From: Ben Dooks @ 2009-05-11 23:18 UTC (permalink / raw)
To: Mark Ware; +Cc: Linuxppc-dev Development, Ben Dooks, linux-i2c
In-Reply-To: <4A07A982.40504@elphinstone.net>
On Mon, May 11, 2009 at 02:28:50PM +1000, Mark Ware wrote:
> Ben Dooks wrote:
>> On Wed, Apr 29, 2009 at 08:43:14AM -0500, Kumar Gala wrote:
>>> On Apr 22, 2009, at 4:56 PM, Ben Dooks wrote:
>>>
>>>> On Tue, Apr 21, 2009 at 10:11:51AM -0500, Kumar Gala wrote:
>>>>> On Apr 21, 2009, at 7:49 AM, Mark Ware wrote:
>>>>>
>>>>>> Recent DMA changes result in a BUG() when NULL is passed to
>>>>>> dma_alloc_coherent in place of a device.
>>>>>>
>>>>>> Signed-off-by: Mark Ware <mware@elphinstone.net>
>>>>>> ---
>>>>>>
>>>>>> This patch fixes the BUG() during boot that has appeared during the
>>>>>> 2.6.30 window. It has been tested and appears correct on my
>>>>>> 8280 based
>>>>>> board.
>>>>>> Sent to both linuxppc-dev and linux-i2c, since I'm not sure where it
>>>>>> belongs.
>>>>>>
>>>>>>
>>>>>> drivers/i2c/busses/i2c-cpm.c | 14 ++++++++------
>>>>>> 1 files changed, 8 insertions(+), 6 deletions(-)
>>>>> Acked-by: Kumar Gala <galak@kernel.crashing.org>
>>>>>
>>>>> Ben, I'm expecting you to pick this up unless you tell me otherwise.
>>>> Yes.
>>> This go in yet?
>>
>> I've had to do a manual apply due to some changes in the
>> driver, so can someone please do a build of my git tree
>> at:
>>
>> git://aeryn.fluff.org.uk/bjdooks/linux.git i2c-for-2630-rc5
>>
>> or tell me which arch and defconfig to build.
>>
>
> Ping. Is there anything still blocking this?
sorry, have been ill recently, will get this sorted and
pushed.
--
Ben (ben@fluff.org, http://www.fluff.org/)
'a smiley only costs 4 bytes'
^ permalink raw reply
* Re: device trees.
From: Grant Likely @ 2009-05-11 23:09 UTC (permalink / raw)
To: David H. Lynch Jr.; +Cc: linuxppc-dev
In-Reply-To: <4A089AC8.9080704@dlasys.net>
On Mon, May 11, 2009 at 3:38 PM, David H. Lynch Jr. <dhlii@dlasys.net> wrot=
e:
> Grant Likely wrote:
>>
>> What do you mean by "one size fits all solution?"
>>
>> The kernel doesn't care where the device tree comes from. =A0All it
>> cares about is that by the time the kernel is started the device tree
>> must be fully formed and populated. =A0It can be completely pre-canned
>> (like simpleImage), it can be modified by firmware (like u-boot), or
>> it can be generated from scratch (like with real OpenFirmware). =A0There
>> is lots of flexibility on how to handle it.
>>
>>
> First device trees are now the ONLY means of =A0passing information to th=
e
> kernel.
> By definition that means it is a one size fits all solution.
> While there is nothing inherently wrong with that, solutions intended to
> meet all circumstances need to be
> simple, powerful, and flexible. They need to work well 100% of the time
> not 98%.
>
> Not only is the device tree expected to pass static hardware
> configuration information, but it is the sole means of passing anything.
> As an example Command lines are to be in the device tree.
> Everything is supposed to be in the device tree, whether that
> information is static or dynamic, whether it is hardware information,
> or user choices.
It is the sole means of passing anything *to the kernel*. You can
pass whatever you like to the bootwrapper. :-)
> =A0 =A0That means that whether you are in a Sun or Apple Desktop or a
> system with the no flash and barely enough resources to run Linux,
> you still may have to manipulate the device tree.
...or if you really are constrained, then define a format for the data
you want to pass, pass it to the bootwrapper along with the device
tree, and use the bootwrapper (which already has libfdt) to update the
device tree. cuImage targets do this to support older u-boots which
don't understand device trees. You are not forced to put device tree
support in your firmware.
In other words; having your bootloader support FDT is preferred, but
not required.
Word of warning; if you do define your own format, be very very very
careful. Otherwise you end up with something subtly broken and not
future proof. This is one of the reasons adding FDT support to your
firmware is preferred; these issues have already been thought about.
> =A0 =A0Though I still have an issue. One of the problem with 98% solution=
s,
> is that they result in a chain of work arrounds for
> =A0 =A0the other 2%. Instead of one case or two cases, you end up with a
> dozen cases each handling increasingly tiny slivers.
> =A0 =A0And it becomes increasingly easy to claim that the problem is with
> the slivers not the broad solution.
No, it's not. Everything you need to do can be done within the
bootwrapper if you so desire, passing data in whatever format you
like.
> =A0 =A0I am not sure what you are saying here ?
> =A0 =A0By firmware do you mean bootloader ? Or do you mean bitfiles ?
Yes, I'm using the terms "firmware" and "bootloader" interchangeably.
>> I still don't understand what you're worried about starting an arguing
>> about. =A0Pretty much any of the PowerPC maintainers can point at warts
>> and problems in the current handling of device trees. =A0I'm not
>> particularly happy with simpleImage (and I wrote it), but it takes
>> time and effort to write something more capable.
>>
> =A0 =A0I was not trying to start an argument, my initial question was
>
> =A0 "Is there an example somewhere that shows building a device tree on t=
he fly ?"
>
> =A0 The responses have questioned why I want to do that rather than how c=
an I do that.
> =A0 I was not actually seeking a debate over the merit of device trees, o=
r u-boot, or libdft, or ....
... but the questions were necessary to understand your problem set.
I cannot give good advice without understanding.
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH] fix sata_sil compilation on non-DMI platforms
From: Benjamin Herrenschmidt @ 2009-05-11 22:31 UTC (permalink / raw)
To: Alan Cox
Cc: Jeff Garzik, linuxppc-dev, Konstantinos Margaritis, linux-ide,
rmk, Lennart Sorensen, Lennert Buytenhek
In-Reply-To: <20090511194410.46641214@lxorguk.ukuu.org.uk>
On Mon, 2009-05-11 at 19:44 +0100, Alan Cox wrote:
> > Ideally the DMI subsystem should be provided wrappers for platforms
> > without DMI, rendering patches like this unnecessary.
>
> Interesting question - is the PPC OpenFirmware machine information
> mappable onto the DMI space ? or the various static bits of name
> information in the various ARM and the like machine descriptions.
>
> If not which bits are similar enough we could replace dmi at the high
> level with an abstract interface for system/vendor/... that was ?
The one thing we could try to map would be the device-tree "compatible"
property which would contain vendor,system tuples from more specific to
more generic with which the machine is compatible with.
Of course I would argue the other way around and create a device-tree
from the DMI data :-)
Cheers,
Ben.
^ permalink raw reply
* Re: device trees.
From: Benjamin Herrenschmidt @ 2009-05-11 22:29 UTC (permalink / raw)
To: David H. Lynch Jr.; +Cc: linuxppc-dev
In-Reply-To: <4A089AC8.9080704@dlasys.net>
On Mon, 2009-05-11 at 17:38 -0400, David H. Lynch Jr. wrote:
> Not only is the device tree expected to pass static hardware
> configuration information, but it is the sole means of passing
> anything.
> As an example Command lines are to be in the device tree.
> Everything is supposed to be in the device tree, whether that
> information is static or dynamic, whether it is hardware information,
> or user choices.
>
> That means that whether you are in a Sun or Apple Desktop or a
> system with the no flash and barely enough resources to run Linux,
> you still may have to manipulate the device tree.
To some extend, but the user choice information is well isolated, it's
all in /chosen, so it's really only that node that needs to be modified
or created from scratch by the bootloader.
Thus, your bootloader can pick the right static dtb for a given bitfile,
and then just use libfdt to generate the right /chosen node with other
informations.
Ben.
^ permalink raw reply
* Re: device trees.
From: Benjamin Herrenschmidt @ 2009-05-11 22:25 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev, dhlii
In-Reply-To: <fa686aa40905102108l5c964893y2dfb84763e13511@mail.gmail.com>
> > Not pretty, but it does more or less what you're talking about. Would
> > need some work to get it going on !pseries obviously.
>
> Heh, I didn't even know this existed. :-)
>
> Thinking about this more, it seems to me that the tricky bit would be
> figuring out how to drop all references to a node before it is pruned
> from the tree. of_platform_devices would probably be the easiest
> because the bus could walked before pruning the node, but there are
> also references on the i2c, spi and mdio busses that must be dealt
> with appropriately.
Also, the way userspace manipulates the tree via /proc/ppc64 is quite
gross... Some time ago we discussed about moving that to the kernel
since the firmware call that gives us the new bits of tree could be done
there in a relatively generic way but that's only one option.
Maybe it's time to have a proper devicetreefs so one can use standard fs
APIs to create nodes & properties though we would need a trick to turn
them from "temporary" to "online" and similar for offlining. Maybe with
mv from/to a "staging" directory that isn't "live" vs. the kernel.
Cheers,
Ben.
^ permalink raw reply
* Re: device trees.
From: David H. Lynch Jr. @ 2009-05-11 21:38 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev
In-Reply-To: <fa686aa40905111047p5f350b0dh796564cc1383a336@mail.gmail.com>
Grant Likely wrote:
>
> What do you mean by "one size fits all solution?"
>
> The kernel doesn't care where the device tree comes from. All it
> cares about is that by the time the kernel is started the device tree
> must be fully formed and populated. It can be completely pre-canned
> (like simpleImage), it can be modified by firmware (like u-boot), or
> it can be generated from scratch (like with real OpenFirmware). There
> is lots of flexibility on how to handle it.
>
>
First device trees are now the ONLY means of passing information to the
kernel.
By definition that means it is a one size fits all solution.
While there is nothing inherently wrong with that, solutions intended to
meet all circumstances need to be
simple, powerful, and flexible. They need to work well 100% of the time
not 98%.
Not only is the device tree expected to pass static hardware
configuration information, but it is the sole means of passing anything.
As an example Command lines are to be in the device tree.
Everything is supposed to be in the device tree, whether that
information is static or dynamic, whether it is hardware information,
or user choices.
That means that whether you are in a Sun or Apple Desktop or a
system with the no flash and barely enough resources to run Linux,
you still may have to manipulate the device tree.
>> The best alternative to creating the device tree dynamically would
>> be to
>> append the devicetree to the bitimage in a way the boot loader could
>> always find it.
>>
>
> That sounds like a good solution to me.
>
I am glad you like it. If Xilinx would like to offer any advice as
to how to prepend a device tree to the end of a bit file without
foreclosing any of their future plans or .... I would be happy to
look at implementing it.
> As for using up BRAM, a gzipped dtb image is smaller than 2k and it
> can be reclaimed for other uses after the kernel has booted. That may
> not help your situation, but for my use cases the tradeoff works.
>
If I recall the minimum increment for BRAM is 16K.
I am not trying to claim it is not an answer for anyone, or even
most people.
Though I still have an issue. One of the problem with 98% solutions,
is that they result in a chain of work arrounds for
the other 2%. Instead of one case or two cases, you end up with a
dozen cases each handling increasingly tiny slivers.
And it becomes increasingly easy to claim that the problem is with
the slivers not the broad solution.
>> Regardless it still makes my point. The problem with devicetrees as
>> they are is that they handle probably 98% of all cases well.
>> The remaining 2% are a mess.
>>
>
> No it isn't. It is expected that firmware will fixup the device tree
> data with board specific values. This is intentional. The device
> tree is simply the bearer. It makes no determination about where the
> data comes from.
>
I am not sure what you are saying here ?
By firmware do you mean bootloader ? Or do you mean bitfiles ?
In large systems like Sun or Apple desktop the OF Device tree need
not be static.
There is software that may well be larger than the linux kernel.
Our "firmware" bootloader is actually stored in the bitfile - 16K of
BRAM basically becomes the boot ROM - except that it is bitfile
initialized RAM.
I guess this is much like you dtb in BRAM.
We are not increasing the size of the BRAM, while most of our
systems have NOR flash, or ... or ...., the all don't.
Even the amount of DRAM varies, and our code is written not to use
DRAM until we have verified that it works.
>
>> lots of .dtb files lying arround is only a better solution than
>> simpleimage.
>> I will guarantee that unless they are welded together the wrong
>> device tree will get used with the wrong bit file.
>>
>
> I agree.
>
>
>> Inevitably I will make that mistake myself occasionally and waste
>> hours or possibly days trying to debug it.
>> And if I will do it rarely clients will do it frequently.
>>
>> In my expereince if you create a situation where confusion can exist
>> it will.
>>
>> It is also my expereince that time spend coding a solution to a
>> common client problem is well spent.
>> If it takes me a week to work out dynamically creating a device
>> tree, that ill likely save many weeks of
>> support headaches.
>>
>
> Again, it doesn't sound like you want dynamic *creation* of device
> trees. It sounds like you want a reliable way to make sure the
> bitstream is welded together with the correct dtb, preferably within
> the Xilinx toolchain.
>
Welding the bit file to the dtb might solve 75% of my issues,
And it probably would get me to the point where I could move
forward and live with
the remaining issues untile I was inspired to solve them.
but it does not solve everything. It is increasingly clear to me
that I am going to have to
manipulate device trees.
>
>> Even if I do not end up creating the device tree dynamically, I am
>> likely to end up at a minimum doing some validation on it.
>> i.e. once the bitfile is loaded scanning the device tree and probing
>> to ascertain that the hardware that I am supposed to expect
>> it really present.
>>
>
> If you like.
>
>
>> ultimately devicetrees are supposed to be a database not a black box.
>>
>
> I don't understand what you mean by this statement.
>
Right now for embedded systems device trees are a black box not a
database.
You get them from EDK and pass them on to linux.
Only Linux and the EDK understand and manipulate them.
I gather u-boot is twiddling at the fringes.
>
>> Anyway, all I was looking for was a leg up on figuring out how to do
>> what I want with them. Rather than starting from scratch.
>> I am not looking to be convinced that I am approaching this all wrong.
>> If you are happy with what you have - great. I am not.
>> While I was not looking to restart a great debate over device trees
>> - I do not actually think they are a bad idea.
>>
>
> I still don't understand what you're worried about starting an arguing
> about. Pretty much any of the PowerPC maintainers can point at warts
> and problems in the current handling of device trees. I'm not
> particularly happy with simpleImage (and I wrote it), but it takes
> time and effort to write something more capable.
>
I was not trying to start an argument, my initial question was
"Is there an example somewhere that shows building a device tree on the fly ?"
The responses have questioned why I want to do that rather than how can I do that.
I was not actually seeking a debate over the merit of device trees, or u-boot, or libdft, or ....
--
Dave Lynch DLA Systems
Software Development: Embedded Linux
717.627.3770 dhlii@dlasys.net http://www.dlasys.net
fax: 1.253.369.9244 Cell: 1.717.587.7774
Over 25 years' experience in platforms, languages, and technologies too numerous to list.
"Any intelligent fool can make things bigger and more complex... It takes a touch of genius - and a lot of courage to move in the opposite direction."
Albert Einstein
^ permalink raw reply
* Re: [PATCH] ps3: remove driver_data direct access of struct device
From: Geoff Levand @ 2009-05-11 21:07 UTC (permalink / raw)
To: Roel Kluin; +Cc: Geert.Uytterhoeven, linuxppc-dev, cbe-oss-dev, lkml
In-Reply-To: <4A087DB9.50305@gmail.com>
Hi Roel,
On 05/11/2009 12:34 PM, Roel Kluin wrote:
> To avoid direct access to the driver_data pointer in struct device, the
> functions dev_get_drvdata() and dev_set_drvdata() should be used.
>
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> Please review. especially note that I removed a
> kfree(dev->sbd.core.driver_data);
> Is that correct?
Comment below.
> arch/powerpc/include/asm/ps3.h | 4 ++--
> drivers/char/ps3flash.c | 11 +++++------
> drivers/scsi/ps3rom.c | 10 +++++-----
> drivers/video/ps3fb.c | 6 +++---
> 4 files changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ps3.h b/arch/powerpc/include/asm/ps3.h
> index cdb6fd8..a55717e 100644
> --- a/arch/powerpc/include/asm/ps3.h
> +++ b/arch/powerpc/include/asm/ps3.h
> @@ -421,12 +421,12 @@ static inline struct ps3_system_bus_driver *
> static inline void ps3_system_bus_set_driver_data(
> struct ps3_system_bus_device *dev, void *data)
> {
> - dev->core.driver_data = data;
> + dev_set_drvdata(&dev->core, data);
> }
> static inline void *ps3_system_bus_get_driver_data(
> struct ps3_system_bus_device *dev)
> {
> - return dev->core.driver_data;
> + return dev_get_drvdata(&dev->core);
> }
>
> /* These two need global scope for get_dma_ops(). */
> diff --git a/drivers/char/ps3flash.c b/drivers/char/ps3flash.c
> index afbe456..6083032 100644
> --- a/drivers/char/ps3flash.c
> +++ b/drivers/char/ps3flash.c
> @@ -108,7 +108,7 @@ static ssize_t ps3flash_read(struct file *file, char __user *buf, size_t count,
> loff_t *pos)
> {
> struct ps3_storage_device *dev = ps3flash_dev;
> - struct ps3flash_private *priv = dev->sbd.core.driver_data;
> + struct ps3flash_private *priv = dev_get_drvdata(&dev->sbd.core);
These should all be using ps3_system_bus_get_driver_data() and
ps3_system_bus_set_driver_data():
struct ps3flash_private *priv = ps3_system_bus_get_driver_data(&dev->sbd)
> @@ -404,8 +404,7 @@ static int ps3flash_remove(struct ps3_system_bus_device *_dev)
>
> misc_deregister(&ps3flash_misc);
> ps3stor_teardown(dev);
> - kfree(dev->sbd.core.driver_data);
> - dev->sbd.core.driver_data = NULL;
> + dev_set_drvdata(&dev->sbd.core, NULL);
It seems that will result in a memory leak. Why did you
think the kfree() should be removed?
Won't this work?
kfree(ps3_system_bus_get_driver_data(&dev->sbd));
ps3_system_bus_set_driver_data(&dev->sbd, NULL);
> ps3flash_dev = NULL;
> return 0;
> }
-Geoff
^ permalink raw reply
* [RFC] Hardware Breakpoint interfaces implementation for PPC64
From: K.Prasad @ 2009-05-11 20:03 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Benjamin Herrenschmidt, paulus
Hi PPC Dev folks,
Please find a patch below that implements the proposed Hardware
Breakpoint interfaces for PPC64 architecture.
As a brief introduction, the proposed Hardware Breakpoint
infrastructure provides generic in-kernel interfaces to which users
from kernel- and user-space can request for breakpoint registers. An
ftrace plugin that can trace accesses to data variables is also part
of the generic HW Breakpoint interface patchset. The latest submission
for this patchset along with an x86 implementation can be found here:
http://lkml.org/lkml/2009/5/11/159.
The following are the salient features of the PPC64 patch.
- Arch-specific definitions for kernel and user-space requests are
defined in arch/powerpc/kernel/hw_breakpoint.c
- Ptrace is converted to use the HW Breakpoint interfaces
- The ftrace plugin called ksym_tracer is tested to work fine. For
instance when tracing pid_max kernel variable, the following was
obtained as output
# cat trace
# tracer: ksym_tracer
#
# TASK-PID CPU# Symbol Type Function
# | | | | |
bash 4502 3 pid_max RW .do_proc_dointvec_minmax_conv+0x78/0x10c
bash 4502 3 pid_max RW .do_proc_dointvec_minmax_conv+0xa0/0x10c
bash 4502 3 pid_max RW .alloc_pid+0x8c/0x4a4
There are however a few limitations/caveats of the patch as identified
below:
- The patch is currently implemented only for PPC64 architecture. Other
architectures (especially Book-E implementations are expected to
happen in due course).
- HW Breakpoints over data addresses through Xmon (using "bd" command)
and the proposed HW Breakpoint interfaces can now operate in a
mutually exclusive manner. Xmon's integration is pending and is
dependant on successful triggering of breakpoints through "bd<ops>".
(Note: On a Power5 machine running 2.6.29, Xmon could not trigger HW
Breakpoints when tested).
Kindly let me know your comments.
Thanks,
K.Prasad
Signed-off-by: K.Prasad <prasad@linux.vnet.ibm.com>
---
arch/powerpc/Kconfig | 1
arch/powerpc/include/asm/hw_breakpoint.h | 52 +++++
arch/powerpc/include/asm/processor.h | 1
arch/powerpc/include/asm/reg.h | 2
arch/powerpc/include/asm/thread_info.h | 2
arch/powerpc/kernel/Makefile | 2
arch/powerpc/kernel/hw_breakpoint.c | 271 +++++++++++++++++++++++++++++++
arch/powerpc/kernel/process.c | 18 ++
arch/powerpc/kernel/ptrace.c | 48 +++++
arch/powerpc/mm/fault.c | 14 -
samples/hw_breakpoint/data_breakpoint.c | 4
12 files changed, 423 insertions(+), 9 deletions(-)
Index: linux-2.6-tip.hbkpt/arch/powerpc/Kconfig
===================================================================
--- linux-2.6-tip.hbkpt.orig/arch/powerpc/Kconfig
+++ linux-2.6-tip.hbkpt/arch/powerpc/Kconfig
@@ -125,6 +125,7 @@ config PPC
select USE_GENERIC_SMP_HELPERS if SMP
select HAVE_OPROFILE
select HAVE_SYSCALL_WRAPPERS if PPC64
+ select HAVE_HW_BREAKPOINT if PPC64
config EARLY_PRINTK
bool
Index: linux-2.6-tip.hbkpt/arch/powerpc/kernel/Makefile
===================================================================
--- linux-2.6-tip.hbkpt.orig/arch/powerpc/kernel/Makefile
+++ linux-2.6-tip.hbkpt/arch/powerpc/kernel/Makefile
@@ -33,7 +33,7 @@ obj-$(CONFIG_PPC64) += setup_64.o sys_p
signal_64.o ptrace32.o \
paca.o cpu_setup_ppc970.o \
cpu_setup_pa6t.o \
- firmware.o nvram_64.o
+ firmware.o nvram_64.o hw_breakpoint.o
obj64-$(CONFIG_RELOCATABLE) += reloc_64.o
obj-$(CONFIG_PPC64) += vdso64/
obj-$(CONFIG_ALTIVEC) += vecemu.o vector.o
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,271 @@
+/*
+ * 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
+ */
+
+/*
+ * HW_breakpoint: a unified kernel/user-space hardware breakpoint facility,
+ * using the CPU's debug registers.
+ */
+
+#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);
+
+void arch_update_kernel_hw_breakpoint(void *unused)
+{
+ struct hw_breakpoint *bp;
+
+ /* Check if there is nothing to update */
+ if (hbp_kernel_pos == HB_NUM)
+ return;
+ 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);
+}
+
+/*
+ * Install the debug register values for just the kernel, no thread.
+ */
+void arch_uninstall_thread_hw_breakpoint()
+{
+ set_dabr(0);
+}
+
+/*
+ * Check for virtual address in user space.
+ */
+int arch_check_va_in_userspace(unsigned long va, u8 hbp_len)
+{
+ return (va <= TASK_SIZE - HW_BREAKPOINT_LEN);
+}
+
+/*
+ * Check for virtual address in kernel space.
+ */
+int arch_check_va_in_kernelspace(unsigned long va, u8 hbp_len)
+{
+ return (va >= TASK_SIZE) && ((va + HW_BREAKPOINT_LEN - 1) >= TASK_SIZE);
+}
+
+/*
+ * Store a breakpoint's encoded address, length, and type.
+ */
+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.
+ */
+ 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;
+ default:
+ return ret;
+ }
+
+ if (bp->triggered)
+ ret = arch_store_info(bp);
+
+ /* Check for double word alignment - 8 bytes */
+ if (bp->info.address & HW_BREAKPOINT_ALIGN)
+ 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 | 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;
+ 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;
+
+ if (is_kernel)
+ bp = hbp_kernel[0];
+ else {
+ bp = current->thread.hbp[0];
+ /* Lazy debug register switching */
+ if (!bp)
+ return rc;
+ 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;
+}
+
+/*
+ * 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);
+ }
+
+ set_dabr(this_dabr_data);
+ per_cpu(dabr_data, cpu) = 0;
+ ret = NOTIFY_STOP;
+ put_cpu_no_resched();
+
+out:
+ 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;
+}
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,52 @@
+#ifndef _PPC64_HW_BREAKPOINT_H
+#define _PPC64_HW_BREAKPOINT_H
+
+#ifdef __KERNEL__
+#define __ARCH_HW_BREAKPOINT_H
+
+struct arch_hw_breakpoint {
+ char *name; /* Contains name of the symbol to set bkpt */
+ unsigned long address;
+ u8 type;
+};
+
+#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_RW
+
+#define HW_BREAKPOINT_ALIGN 0x7
+#define HW_BREAKPOINT_LEN 4
+
+extern struct hw_breakpoint *hbp_kernel[HB_NUM];
+extern unsigned int hbp_user_refcount[HB_NUM];
+
+/*
+ * Ptrace support: breakpoint trigger routine.
+ */
+extern int __modify_user_hw_breakpoint(int pos, struct task_struct *tsk,
+ struct hw_breakpoint *bp);
+
+extern void arch_install_thread_hw_breakpoint(struct task_struct *tsk);
+extern void arch_uninstall_thread_hw_breakpoint(void);
+extern int arch_check_va_in_userspace(unsigned long va, u8 hbp_len);
+extern int arch_check_va_in_kernelspace(unsigned long va, u8 hbp_len);
+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 switch_to_thread_hw_breakpoint(struct task_struct *tsk);
+
+#endif /* __KERNEL__ */
+#endif /* _PPC64_HW_BREAKPOINT_H */
+
Index: linux-2.6-tip.hbkpt/arch/powerpc/include/asm/processor.h
===================================================================
--- linux-2.6-tip.hbkpt.orig/arch/powerpc/include/asm/processor.h
+++ linux-2.6-tip.hbkpt/arch/powerpc/include/asm/processor.h
@@ -177,6 +177,7 @@ struct thread_struct {
#ifdef CONFIG_PPC64
unsigned long start_tb; /* Start purr when proc switched in */
unsigned long accum_tb; /* Total accumilated purr for process */
+ struct hw_breakpoint *hbp[HB_NUM];
#endif
unsigned long dabr; /* Data address breakpoint register */
#ifdef CONFIG_ALTIVEC
Index: linux-2.6-tip.hbkpt/arch/powerpc/kernel/ptrace.c
===================================================================
--- 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
/*
* does not yet catch signals sent when the child dies.
@@ -735,9 +738,22 @@ void user_disable_single_step(struct tas
clear_tsk_thread_flag(task, TIF_SINGLESTEP);
}
+static void ptrace_triggered(struct hw_breakpoint *bp, struct pt_regs *regs)
+{
+ /*
+ * The SIGTRAP signal is generated automatically for us in do_dabr().
+ * We don't have to do anything here
+ */
+}
+
int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
unsigned long data)
{
+#ifdef CONFIG_PPC64
+ struct thread_struct *thread = &(task->thread);
+ struct hw_breakpoint *bp;
+ int ret;
+#endif
/* For ppc64 we support one DABR and no IABR's at the moment (ppc64).
* For embedded processors we support one DAC and no IAC's at the
* moment.
@@ -767,6 +783,38 @@ int ptrace_set_debugreg(struct task_stru
if (data && !(data & DABR_TRANSLATION))
return -EIO;
+#ifdef CONFIG_PPC64
+ bp = thread->hbp[0];
+ if ((data & ~0x7UL) == 0) {
+ if (bp) {
+ unregister_user_hw_breakpoint(task, bp);
+ kfree(bp);
+ thread->hbp[0] = NULL;
+ }
+ return 0;
+ }
+
+ if (bp) {
+ bp->info.type = data & 0x3UL;
+ task->thread.dabr = bp->info.address =
+ (data & ~HW_BREAKPOINT_ALIGN);
+ return __modify_user_hw_breakpoint(0, task, bp);
+ }
+ bp = kzalloc(sizeof(struct hw_breakpoint), GFP_KERNEL);
+ if (!bp)
+ return -ENOMEM;
+
+ /* Store the type of breakpoint */
+ bp->info.type = data & 0x3UL;
+ bp->triggered = ptrace_triggered;
+ task->thread.dabr = bp->info.address = (data & ~HW_BREAKPOINT_ALIGN);
+
+ ret = register_user_hw_breakpoint(task, bp);
+ if (ret)
+ return ret;
+ set_tsk_thread_flag(task, TIF_DEBUG);
+#endif /* CONFIG_PPC64 */
+
/* Move contents to the DABR register */
task->thread.dabr = data;
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
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;
+#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
Index: linux-2.6-tip.hbkpt/samples/hw_breakpoint/data_breakpoint.c
===================================================================
--- linux-2.6-tip.hbkpt.orig/samples/hw_breakpoint/data_breakpoint.c
+++ linux-2.6-tip.hbkpt/samples/hw_breakpoint/data_breakpoint.c
@@ -54,6 +54,10 @@ static int __init hw_break_module_init(v
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;
Index: linux-2.6-tip.hbkpt/arch/powerpc/include/asm/thread_info.h
===================================================================
--- linux-2.6-tip.hbkpt.orig/arch/powerpc/include/asm/thread_info.h
+++ linux-2.6-tip.hbkpt/arch/powerpc/include/asm/thread_info.h
@@ -114,6 +114,7 @@ static inline struct thread_info *curren
#define TIF_FREEZE 14 /* Freezing for suspend */
#define TIF_RUNLATCH 15 /* Is the runlatch enabled? */
#define TIF_ABI_PENDING 16 /* 32/64 bit switch needed */
+#define TIF_DEBUG 17 /* uses debug registers */
/* as above, but as bit values */
#define _TIF_SYSCALL_TRACE (1<<TIF_SYSCALL_TRACE)
@@ -132,6 +133,7 @@ static inline struct thread_info *curren
#define _TIF_FREEZE (1<<TIF_FREEZE)
#define _TIF_RUNLATCH (1<<TIF_RUNLATCH)
#define _TIF_ABI_PENDING (1<<TIF_ABI_PENDING)
+#define _TIF_DEBUG (1<<TIF_DEBUG)
#define _TIF_SYSCALL_T_OR_A (_TIF_SYSCALL_TRACE|_TIF_SYSCALL_AUDIT|_TIF_SECCOMP)
#define _TIF_USER_WORK_MASK (_TIF_SIGPENDING | _TIF_NEED_RESCHED | \
Index: linux-2.6-tip.hbkpt/arch/powerpc/include/asm/reg.h
===================================================================
--- linux-2.6-tip.hbkpt.orig/arch/powerpc/include/asm/reg.h
+++ linux-2.6-tip.hbkpt/arch/powerpc/include/asm/reg.h
@@ -184,9 +184,11 @@
#define CTRL_TE 0x00c00000 /* thread enable */
#define CTRL_RUNLATCH 0x1
#define SPRN_DABR 0x3F5 /* Data Address Breakpoint Register */
+#define HB_NUM 1 /* Number of physical HW breakpoint registers */
#define DABR_TRANSLATION (1UL << 2)
#define DABR_DATA_WRITE (1UL << 1)
#define DABR_DATA_READ (1UL << 0)
+#define DABR_DATA_RW (3UL << 0)
#define SPRN_DABR2 0x13D /* e300 */
#define SPRN_DABRX 0x3F7 /* Data Address Breakpoint Register Extension */
#define DABRX_USER (1UL << 0)
Index: linux-2.6-tip.hbkpt/arch/powerpc/mm/fault.c
===================================================================
--- linux-2.6-tip.hbkpt.orig/arch/powerpc/mm/fault.c
+++ linux-2.6-tip.hbkpt/arch/powerpc/mm/fault.c
@@ -137,6 +137,12 @@ int __kprobes do_page_fault(struct pt_re
error_code &= 0x48200000;
else
is_write = error_code & DSISR_ISSTORE;
+
+ if (error_code & DSISR_DABRMATCH) {
+ /* DABR match */
+ do_dabr(regs, address, error_code);
+ return 0;
+ }
#else
is_write = error_code & ESR_DST;
#endif /* CONFIG_4xx || CONFIG_BOOKE */
@@ -151,14 +157,6 @@ int __kprobes do_page_fault(struct pt_re
if (!user_mode(regs) && (address >= TASK_SIZE))
return SIGSEGV;
-#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE))
- if (error_code & DSISR_DABRMATCH) {
- /* DABR match */
- do_dabr(regs, address, error_code);
- return 0;
- }
-#endif /* !(CONFIG_4xx || CONFIG_BOOKE)*/
-
if (in_atomic() || mm == NULL) {
if (!user_mode(regs))
return SIGSEGV;
^ permalink raw reply
* [PATCH] aoa: remove driver_data direct access of struct device
From: Roel Kluin @ 2009-05-11 19:54 UTC (permalink / raw)
To: johannes; +Cc: linuxppc-dev, alsa-devel, lkml
To avoid direct access to the driver_data pointer in struct device, the
functions dev_get_drvdata() and dev_set_drvdata() should be used.
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
diff --git a/sound/aoa/fabrics/layout.c b/sound/aoa/fabrics/layout.c
index fbf5c93..586965f 100644
--- a/sound/aoa/fabrics/layout.c
+++ b/sound/aoa/fabrics/layout.c
@@ -1037,7 +1037,7 @@ static int aoa_fabric_layout_probe(struct soundbus_dev *sdev)
}
ldev->selfptr_headphone.ptr = ldev;
ldev->selfptr_lineout.ptr = ldev;
- sdev->ofdev.dev.driver_data = ldev;
+ dev_set_drvdata(&sdev->ofdev.dev, ldev);
list_add(&ldev->list, &layouts_list);
layouts_list_items++;
@@ -1081,7 +1081,7 @@ static int aoa_fabric_layout_probe(struct soundbus_dev *sdev)
static int aoa_fabric_layout_remove(struct soundbus_dev *sdev)
{
- struct layout_dev *ldev = sdev->ofdev.dev.driver_data;
+ struct layout_dev *ldev = dev_get_drvdata(&sdev->ofdev.dev);
int i;
for (i=0; i<MAX_CODECS_PER_BUS; i++) {
@@ -1114,7 +1114,7 @@ static int aoa_fabric_layout_remove(struct soundbus_dev *sdev)
#ifdef CONFIG_PM
static int aoa_fabric_layout_suspend(struct soundbus_dev *sdev, pm_message_t state)
{
- struct layout_dev *ldev = sdev->ofdev.dev.driver_data;
+ struct layout_dev *ldev = dev_get_drvdata(&sdev->ofdev.dev);
if (ldev->gpio.methods && ldev->gpio.methods->all_amps_off)
ldev->gpio.methods->all_amps_off(&ldev->gpio);
@@ -1124,7 +1124,7 @@ static int aoa_fabric_layout_suspend(struct soundbus_dev *sdev, pm_message_t sta
static int aoa_fabric_layout_resume(struct soundbus_dev *sdev)
{
- struct layout_dev *ldev = sdev->ofdev.dev.driver_data;
+ struct layout_dev *ldev = dev_get_drvdata(&sdev->ofdev.dev);
if (ldev->gpio.methods && ldev->gpio.methods->all_amps_off)
ldev->gpio.methods->all_amps_restore(&ldev->gpio);
diff --git a/sound/aoa/soundbus/i2sbus/core.c b/sound/aoa/soundbus/i2sbus/core.c
index 418c84c..4e3b819 100644
--- a/sound/aoa/soundbus/i2sbus/core.c
+++ b/sound/aoa/soundbus/i2sbus/core.c
@@ -358,14 +358,14 @@ static int i2sbus_probe(struct macio_dev* dev, const struct of_device_id *match)
return -ENODEV;
}
- dev->ofdev.dev.driver_data = control;
+ dev_set_drvdata(&dev->ofdev.dev, control);
return 0;
}
static int i2sbus_remove(struct macio_dev* dev)
{
- struct i2sbus_control *control = dev->ofdev.dev.driver_data;
+ struct i2sbus_control *control = dev_get_drvdata(&dev->ofdev.dev);
struct i2sbus_dev *i2sdev, *tmp;
list_for_each_entry_safe(i2sdev, tmp, &control->list, item)
@@ -377,7 +377,7 @@ static int i2sbus_remove(struct macio_dev* dev)
#ifdef CONFIG_PM
static int i2sbus_suspend(struct macio_dev* dev, pm_message_t state)
{
- struct i2sbus_control *control = dev->ofdev.dev.driver_data;
+ struct i2sbus_control *control = dev_get_drvdata(&dev->ofdev.dev);
struct codec_info_item *cii;
struct i2sbus_dev* i2sdev;
int err, ret = 0;
@@ -407,7 +407,7 @@ static int i2sbus_suspend(struct macio_dev* dev, pm_message_t state)
static int i2sbus_resume(struct macio_dev* dev)
{
- struct i2sbus_control *control = dev->ofdev.dev.driver_data;
+ struct i2sbus_control *control = dev_get_drvdata(&dev->ofdev.dev);
struct codec_info_item *cii;
struct i2sbus_dev* i2sdev;
int err, ret = 0;
^ permalink raw reply related
* Re: [PATCH] fix sata_sil compilation on non-DMI platforms
From: Alan Cox @ 2009-05-11 18:44 UTC (permalink / raw)
To: Jeff Garzik
Cc: linux-ide, Konstantinos Margaritis, linuxppc-dev,
Lennert Buytenhek, Lennart Sorensen, rmk
In-Reply-To: <4A086A76.3090008@garzik.org>
> Ideally the DMI subsystem should be provided wrappers for platforms
> without DMI, rendering patches like this unnecessary.
Interesting question - is the PPC OpenFirmware machine information
mappable onto the DMI space ? or the various static bits of name
information in the various ARM and the like machine descriptions.
If not which bits are similar enough we could replace dmi at the high
level with an abstract interface for system/vendor/... that was ?
(
^ permalink raw reply
* [PATCH] ps3: remove driver_data direct access of struct device
From: Roel Kluin @ 2009-05-11 19:34 UTC (permalink / raw)
To: geoffrey.levand, Geert.Uytterhoeven; +Cc: linuxppc-dev, cbe-oss-dev, lkml
To avoid direct access to the driver_data pointer in struct device, the
functions dev_get_drvdata() and dev_set_drvdata() should be used.
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
Please review. especially note that I removed a
kfree(dev->sbd.core.driver_data);
Is that correct?
arch/powerpc/include/asm/ps3.h | 4 ++--
drivers/char/ps3flash.c | 11 +++++------
drivers/scsi/ps3rom.c | 10 +++++-----
drivers/video/ps3fb.c | 6 +++---
4 files changed, 15 insertions(+), 16 deletions(-)
diff --git a/arch/powerpc/include/asm/ps3.h b/arch/powerpc/include/asm/ps3.h
index cdb6fd8..a55717e 100644
--- a/arch/powerpc/include/asm/ps3.h
+++ b/arch/powerpc/include/asm/ps3.h
@@ -421,12 +421,12 @@ static inline struct ps3_system_bus_driver *
static inline void ps3_system_bus_set_driver_data(
struct ps3_system_bus_device *dev, void *data)
{
- dev->core.driver_data = data;
+ dev_set_drvdata(&dev->core, data);
}
static inline void *ps3_system_bus_get_driver_data(
struct ps3_system_bus_device *dev)
{
- return dev->core.driver_data;
+ return dev_get_drvdata(&dev->core);
}
/* These two need global scope for get_dma_ops(). */
diff --git a/drivers/char/ps3flash.c b/drivers/char/ps3flash.c
index afbe456..6083032 100644
--- a/drivers/char/ps3flash.c
+++ b/drivers/char/ps3flash.c
@@ -108,7 +108,7 @@ static ssize_t ps3flash_read(struct file *file, char __user *buf, size_t count,
loff_t *pos)
{
struct ps3_storage_device *dev = ps3flash_dev;
- struct ps3flash_private *priv = dev->sbd.core.driver_data;
+ struct ps3flash_private *priv = dev_get_drvdata(&dev->sbd.core);
u64 size, start_sector, end_sector, offset;
ssize_t sectors_read;
size_t remaining, n;
@@ -173,7 +173,7 @@ static ssize_t ps3flash_write(struct file *file, const char __user *buf,
size_t count, loff_t *pos)
{
struct ps3_storage_device *dev = ps3flash_dev;
- struct ps3flash_private *priv = dev->sbd.core.driver_data;
+ struct ps3flash_private *priv = dev_get_drvdata(&dev->sbd.core);
u64 size, chunk_sectors, start_write_sector, end_write_sector,
end_read_sector, start_read_sector, head, tail, offset;
ssize_t res;
@@ -366,7 +366,7 @@ static int __devinit ps3flash_probe(struct ps3_system_bus_device *_dev)
goto fail;
}
- dev->sbd.core.driver_data = priv;
+ dev_set_drvdata(&dev->sbd.core, priv);
mutex_init(&priv->mutex);
dev->bounce_size = ps3flash_bounce_buffer.size;
@@ -392,7 +392,7 @@ fail_teardown:
ps3stor_teardown(dev);
fail_free_priv:
kfree(priv);
- dev->sbd.core.driver_data = NULL;
+ dev_set_drvdata(&dev->sbd.core, NULL);
fail:
ps3flash_dev = NULL;
return error;
@@ -404,8 +404,7 @@ static int ps3flash_remove(struct ps3_system_bus_device *_dev)
misc_deregister(&ps3flash_misc);
ps3stor_teardown(dev);
- kfree(dev->sbd.core.driver_data);
- dev->sbd.core.driver_data = NULL;
+ dev_set_drvdata(&dev->sbd.core, NULL);
ps3flash_dev = NULL;
return 0;
}
diff --git a/drivers/scsi/ps3rom.c b/drivers/scsi/ps3rom.c
index ca0dd33..f2f840a 100644
--- a/drivers/scsi/ps3rom.c
+++ b/drivers/scsi/ps3rom.c
@@ -299,7 +299,7 @@ static irqreturn_t ps3rom_interrupt(int irq, void *data)
return IRQ_HANDLED;
}
- host = dev->sbd.core.driver_data;
+ host = dev_get_drvdata(&dev->sbd.core);
priv = shost_priv(host);
cmd = priv->curr_cmd;
@@ -387,7 +387,7 @@ static int __devinit ps3rom_probe(struct ps3_system_bus_device *_dev)
}
priv = shost_priv(host);
- dev->sbd.core.driver_data = host;
+ dev_set_drvdata(&dev->sbd.core, host);
priv->dev = dev;
/* One device/LUN per SCSI bus */
@@ -407,7 +407,7 @@ static int __devinit ps3rom_probe(struct ps3_system_bus_device *_dev)
fail_host_put:
scsi_host_put(host);
- dev->sbd.core.driver_data = NULL;
+ dev_set_drvdata(&dev->sbd.core, NULL);
fail_teardown:
ps3stor_teardown(dev);
fail_free_bounce:
@@ -418,12 +418,12 @@ fail_free_bounce:
static int ps3rom_remove(struct ps3_system_bus_device *_dev)
{
struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
- struct Scsi_Host *host = dev->sbd.core.driver_data;
+ struct Scsi_Host *host = dev_get_drvdata(&dev->sbd.core);
scsi_remove_host(host);
ps3stor_teardown(dev);
scsi_host_put(host);
- dev->sbd.core.driver_data = NULL;
+ dev_set_drvdata(&dev->sbd.core, NULL);
kfree(dev->bounce_buf);
return 0;
}
diff --git a/drivers/video/ps3fb.c b/drivers/video/ps3fb.c
index e00c1df..55f4250 100644
--- a/drivers/video/ps3fb.c
+++ b/drivers/video/ps3fb.c
@@ -1210,7 +1210,7 @@ static int __devinit ps3fb_probe(struct ps3_system_bus_device *dev)
if (retval < 0)
goto err_fb_dealloc;
- dev->core.driver_data = info;
+ dev_set_drvdata(&dev->core, info);
dev_info(info->device, "%s %s, using %u KiB of video memory\n",
dev_driver_string(info->dev), dev_name(info->dev),
@@ -1248,7 +1248,7 @@ err:
static int ps3fb_shutdown(struct ps3_system_bus_device *dev)
{
int status;
- struct fb_info *info = dev->core.driver_data;
+ struct fb_info *info = dev_get_drvdata(&dev->core);
dev_dbg(&dev->core, " -> %s:%d\n", __func__, __LINE__);
@@ -1268,7 +1268,7 @@ static int ps3fb_shutdown(struct ps3_system_bus_device *dev)
unregister_framebuffer(info);
fb_dealloc_cmap(&info->cmap);
framebuffer_release(info);
- info = dev->core.driver_data = NULL;
+ info = dev_set_drvdata(&dev->core, NULL);
}
iounmap((u8 __force __iomem *)ps3fb.dinfo);
^ permalink raw reply related
* Re: question about softirqs
From: Chris Friesen @ 2009-05-11 18:25 UTC (permalink / raw)
To: David Miller; +Cc: linuxppc-dev, paulus
In-Reply-To: <20090508.234815.127227651.davem@davemloft.net>
David Miller wrote:
> You know, for networking over loopback (one of the only real cases
> that even matters, if we get a hard interrupt then the return from
> that would process any softints), we probably make out just fine
> anyways. As long as we hit a local_bh_enable() (and in the return
> path from device transmit that's exceedingly likely as all of the
> networking locking is BH safe) we'll run the softints from that and
> thus long before we get to syscall return.
What about the issue I raised earlier? (I don't think you were copied
at that point.)
Suppose I have a SCHED_FIFO task spinning on recvmsg() with MSG_DONTWAIT
set (and maybe doing other stuff if there are no messages). In this
case, schedule() would re-run the spinning task rather than running
ksoftirqd. This could prevent any incoming packets from actually being
sent up the stack until we get a real hardware interrupt--which could be
a whole jiffy if interrupt mitigation is enabled in the net device.
(And maybe longer if NOHZ is enabled.)
Chris
^ permalink raw reply
* Re: [PATCH 1/3 v3] Added support for Designware SATA controller driver
From: Jeff Garzik @ 2009-05-11 18:23 UTC (permalink / raw)
To: Feng Kan; +Cc: Mark Miesfeld, linuxppc-dev, linux-ide
In-Reply-To: <1241632174-2570-1-git-send-email-fkan@amcc.com>
Feng Kan wrote:
> Signed-off-by: Feng Kan <fkan@amcc.com>
> Signed-off-by: Mark Miesfeld <miesfeld@gmail.com>
> ---
> drivers/ata/Kconfig | 10 +
> drivers/ata/Makefile | 1 +
> drivers/ata/sata_dwc.c | 2053 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 2064 insertions(+), 0 deletions(-)
> create mode 100644 drivers/ata/sata_dwc.c
>
> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
> index 0bcf264..c3d0b24 100644
> --- a/drivers/ata/Kconfig
> +++ b/drivers/ata/Kconfig
> @@ -72,6 +72,16 @@ config SATA_FSL
>
> If unsure, say N.
>
> +config SATA_DWC
> + tristate "DesignWare Cores SATA support"
> + depends on 460EX
> + help
> + This option enables support for the Synopsys DesignWare Cores SATA
> + controller.
> + It can be found on the AMCC 460EX.
> +
> + If unsure, say N.
> +
> config ATA_SFF
> bool "ATA SFF support"
> default y
> diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
> index 7f1ecf9..3d41fc7 100644
> --- a/drivers/ata/Makefile
> +++ b/drivers/ata/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_SATA_MV) += sata_mv.o
> obj-$(CONFIG_SATA_INIC162X) += sata_inic162x.o
> obj-$(CONFIG_PDC_ADMA) += pdc_adma.o
> obj-$(CONFIG_SATA_FSL) += sata_fsl.o
> +obj-$(CONFIG_SATA_DWC) += sata_dwc.o
>
> obj-$(CONFIG_PATA_ALI) += pata_ali.o
> obj-$(CONFIG_PATA_AMD) += pata_amd.o
> diff --git a/drivers/ata/sata_dwc.c b/drivers/ata/sata_dwc.c
> new file mode 100644
> index 0000000..672f91f
> --- /dev/null
> +++ b/drivers/ata/sata_dwc.c
> @@ -0,0 +1,2053 @@
> +/*
> + * drivers/ata/sata_dwc.c
> + *
> + * Synopsys DesignWare Cores (DWC) SATA host driver
> + *
> + * Author: Mark Miesfeld <mmiesfeld@amcc.com>
> + *
> + * Ported from 2.6.19.2 to 2.6.25/26 by Stefan Roese <sr@denx.de>
> + * Copyright 2008 DENX Software Engineering
> + *
> + * Based on versions provided by AMCC and Synopsys which are:
> + * Copyright 2006 Applied Micro Circuits Corporation
> + * COPYRIGHT (C) 2005 SYNOPSYS, INC. ALL RIGHTS RESERVED
> + *
> + * 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.
> + *
> + */
> +#ifdef CONFIG_SATA_DWC_DEBUG
> +#define DEBUG
> +#endif
> +
> +#ifdef CONFIG_SATA_DWC_VDEBUG
> +#define VERBOSE_DEBUG
> +#define DEBUG_NCQ
> +#endif
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/device.h>
> +#include <linux/platform_device.h>
> +#include <linux/libata.h>
> +
> +#include <scsi/scsi_host.h>
> +#include <scsi/scsi_cmnd.h>
> +
> +#define DRV_NAME "sata-dwc"
> +#define DRV_VERSION "1.0"
> +
> +/* SATA DMA driver Globals */
> +#define DMA_NUM_CHANS 1
> +#define DMA_NUM_CHAN_REGS 8
> +
> +/* SATA DMA Register definitions */
> +#define AHB_DMA_BRST_DFLT 64 /* 16 data items burst length */
> +
> +struct dmareg {
> + u32 low; /* Low bits 0-31 */
> + u32 high; /* High bits 32-63 */
> +};
> +
> +/* DMA Per Channel registers */
> +
> +struct dma_chan_regs {
> + struct dmareg sar; /* Source Address */
> + struct dmareg dar; /* Destination address */
> + struct dmareg llp; /* Linked List Pointer */
> + struct dmareg ctl; /* Control */
> + struct dmareg sstat; /* Source Status not implemented in core */
> + struct dmareg dstat; /* Destination Status not implemented in core */
> + struct dmareg sstatar; /* Source Status Address not impl in core */
> + struct dmareg dstatar; /* Destination Status Address not implemented */
> + struct dmareg cfg; /* Config */
> + struct dmareg sgr; /* Source Gather */
> + struct dmareg dsr; /* Destination Scatter */
> +};
> +
> +/* Generic Interrupt Registers */
> +struct dma_interrupt_regs {
> + struct dmareg tfr; /* Transfer Interrupt */
> + struct dmareg block; /* Block Interrupt */
> + struct dmareg srctran; /* Source Transfer Interrupt */
> + struct dmareg dsttran; /* Dest Transfer Interrupt */
> + struct dmareg error; /* Error */
> +};
> +
> +struct ahb_dma_regs {
> + struct dma_chan_regs chan_regs[DMA_NUM_CHAN_REGS];
> + struct dma_interrupt_regs interrupt_raw; /* Raw Interrupt */
> + struct dma_interrupt_regs interrupt_status; /* Interrupt Status */
> + struct dma_interrupt_regs interrupt_mask; /* Interrupt Mask */
> + struct dma_interrupt_regs interrupt_clear; /* Interrupt Clear */
> + struct dmareg statusInt; /* Interrupt combined */
> + struct dmareg rq_srcreg; /* Src Trans Req */
> + struct dmareg rq_dstreg; /* Dst Trans Req */
> + struct dmareg rq_sgl_srcreg; /* Sngl Src Trans Req */
> + struct dmareg rq_sgl_dstreg; /* Sngl Dst Trans Req */
> + struct dmareg rq_lst_srcreg; /* Last Src Trans Req */
> + struct dmareg rq_lst_dstreg; /* Last Dst Trans Req */
> + struct dmareg dma_cfg; /* DMA Config */
> + struct dmareg dma_chan_en; /* DMA Channel Enable */
> + struct dmareg dma_id; /* DMA ID */
> + struct dmareg dma_test; /* DMA Test */
> + struct dmareg res1; /* reserved */
> + struct dmareg res2; /* reserved */
> +
> + /* DMA Comp Params
> + * Param 6 = dma_param[0], Param 5 = dma_param[1],
> + * Param 4 = dma_param[2] ...
> + */
> + struct dmareg dma_params[6];
> +};
> +
> +/* Data structure for linked list item */
> +struct lli {
> + u32 sar; /* Source Address */
> + u32 dar; /* Destination address */
> + u32 llp; /* Linked List Pointer */
> + struct dmareg ctl; /* Control */
> + struct dmareg dstat; /* Destination Status */
> +};
> +
> +#define SATA_DWC_DMAC_LLI_SZ (sizeof(struct lli))
> +#define SATA_DWC_DMAC_LLI_NUM 256
> +#define SATA_DWC_DMAC_TWIDTH_BYTES 4
> +#define SATA_DWC_DMAC_LLI_TBL_SZ \
> + (SATA_DWC_DMAC_LLI_SZ * SATA_DWC_DMAC_LLI_NUM)
> +#define SATA_DWC_DMAC_CTRL_TSIZE_MAX \
> + (0x00000800 * SATA_DWC_DMAC_TWIDTH_BYTES)
> +
> +/* DMA Register Operation Bits */
> +#define DMA_EN 0x00000001 /* Enable AHB DMA */
> +#define DMA_CHANNEL(ch) (0x00000001 << (ch)) /* Select channel */
> +#define DMA_ENABLE_CHAN(ch) ((0x00000001 << (ch)) | \
> + ((0x000000001 << (ch)) << 8))
> +#define DMA_DISABLE_CHAN(ch) (0x00000000 | ((0x000000001 << (ch)) << 8))
> +
> +/* Channel Control Register */
> +#define DMA_CTL_BLK_TS(size) ((size) & 0x000000FFF) /* Blk Transfer size */
> +#define DMA_CTL_LLP_SRCEN 0x10000000 /* Blk chain enable Src */
> +#define DMA_CTL_LLP_DSTEN 0x08000000 /* Blk chain enable Dst */
> +/*
> + * This define is used to set block chaining disabled in the control low
> + * register. It is already in little endian format so it can be &'d dirctly.
> + * It is essentially: cpu_to_le32(~(DMA_CTL_LLP_SRCEN | DMA_CTL_LLP_DSTEN))
> + */
> +#define DMA_CTL_LLP_DISABLE_LE32 0xffffffe7
> +#define DMA_CTL_SMS(num) ((num & 0x3) << 25) /*Src Master Select*/
> +#define DMA_CTL_DMS(num) ((num & 0x3) << 23) /*Dst Master Select*/
> +#define DMA_CTL_TTFC(type) ((type & 0x7) << 20) /*Type&Flow cntr*/
> +#define DMA_CTL_TTFC_P2M_DMAC 0x00000002 /*Per mem,DMAC cntr*/
> +#define DMA_CTL_TTFC_M2P_PER 0x00000003 /*Mem per,peri cntr*/
> +#define DMA_CTL_SRC_MSIZE(size) ((size & 0x7) << 14) /*Src Burst Len*/
> +#define DMA_CTL_DST_MSIZE(size) ((size & 0x7) << 11) /*Dst Burst Len*/
> +#define DMA_CTL_SINC_INC 0x00000000 /*Src addr incr*/
> +#define DMA_CTL_SINC_DEC 0x00000200
> +#define DMA_CTL_SINC_NOCHANGE 0x00000400
> +#define DMA_CTL_DINC_INC 0x00000000 /*Dst addr incr*/
> +#define DMA_CTL_DINC_DEC 0x00000080
> +#define DMA_CTL_DINC_NOCHANGE 0x00000100
> +#define DMA_CTL_SRC_TRWID(size) ((size & 0x7) << 4) /*Src Trnsfr Width*/
> +#define DMA_CTL_DST_TRWID(size) ((size & 0x7) << 1) /*Dst Trnsfr Width*/
> +#define DMA_CTL_INT_EN 0x00000001 /*Interrupt Enable*/
> +
> +/* Channel Configuration Register high bits */
> +#define DMA_CFG_FCMOD_REQ 0x00000001 /*Flow cntrl req*/
> +#define DMA_CFG_PROTCTL (0x00000003 << 2) /*Protection cntrl*/
> +
> +/* Channel Configuration Register low bits */
> +#define DMA_CFG_RELD_DST 0x80000000 /*Reload Dst/Src Addr*/
> +#define DMA_CFG_RELD_SRC 0x40000000
> +#define DMA_CFG_HS_SELSRC 0x00000800 /*SW hndshk Src/Dst*/
> +#define DMA_CFG_HS_SELDST 0x00000400
> +#define DMA_CFG_FIFOEMPTY (0x00000001 << 9) /*FIFO Empty bit*/
> +
> +/* Assign hardware handshaking interface (x) to dst / sre peripheral */
> +#define DMA_CFG_HW_HS_DEST(int_num) ((int_num & 0xF) << 11)
> +#define DMA_CFG_HW_HS_SRC(int_num) ((int_num & 0xF) << 7)
> +
> +/* Channel Linked List Pointer Register */
> +#define DMA_LLP_LMS(addr, master) (((addr) & 0xfffffffc) | (master))
> +#define DMA_LLP_AHBMASTER1 0 /* List Master Select */
> +#define DMA_LLP_AHBMASTER2 1
> +
> +#define SATA_DWC_MAX_PORTS 1
> +
> +#define SATA_DWC_SCR_OFFSET 0x24
> +#define SATA_DWC_REG_OFFSET 0x64
Use 'enum' rather than pre-processor #defines, for creating constants,
as we do in the rest of libata. This gives more debugger and compiler
symbol visibility, and makes pre-processed source much easier to read.
> +/* DWC SATA Registers */
> +struct sata_dwc_regs {
> + u32 fptagr; /* 1st party DMA tag */
> + u32 fpbor; /* 1st party DMA buffer offset */
> + u32 fptcr; /* 1st party DMA Xfr count */
> + u32 dmacr; /* DMA Control */
> + u32 dbtsr; /* DMA Burst Transac size */
> + u32 intpr; /* Interrupt Pending */
> + u32 intmr; /* Interrupt Mask */
> + u32 errmr; /* Error Mask */
> + u32 llcr; /* Link Layer Control */
> + u32 phycr; /* PHY Control */
> + u32 physr; /* PHY Status */
> + u32 rxbistpd; /* Recvd BIST pattern def register */
> + u32 rxbistpd1; /* Recvd BIST data dword1 */
> + u32 rxbistpd2; /* Recvd BIST pattern data dword2 */
> + u32 txbistpd; /* Trans BIST pattern def register */
> + u32 txbistpd1; /* Trans BIST data dword1 */
> + u32 txbistpd2; /* Trans BIST data dword2 */
> + u32 bistcr; /* BIST Control Register */
> + u32 bistfctr; /* BIST FIS Count Register */
> + u32 bistsr; /* BIST Status Register */
> + u32 bistdecr; /* BIST Dword Error count register */
> + u32 res[15]; /* Reserved locations */
> + u32 testr; /* Test Register */
> + u32 versionr; /* Version Register */
> + u32 idr; /* ID Register */
> + u32 unimpl[192]; /* Unimplemented */
> + u32 dmadr[256]; /* FIFO Locations in DMA Mode */
> +};
> +
> +#define SCR_SCONTROL_DET_ENABLE 0x00000001
> +#define SCR_SSTATUS_DET_PRESENT 0x00000001
> +#define SCR_SERROR_DIAG_X 0x04000000
> +
> +/* DWC SATA Register Operations */
> +#define SATA_DWC_TXFIFO_DEPTH 0x01FF
> +#define SATA_DWC_RXFIFO_DEPTH 0x01FF
> +
> +#define SATA_DWC_DMACR_TMOD_TXCHEN 0x00000004
> +#define SATA_DWC_DMACR_TXCHEN (0x00000001 | \
> + SATA_DWC_DMACR_TMOD_TXCHEN)
> +#define SATA_DWC_DMACR_RXCHEN (0x00000002 | \
> + SATA_DWC_DMACR_TMOD_TXCHEN)
> +#define SATA_DWC_DMACR_TX_CLEAR(v) (((v) & ~SATA_DWC_DMACR_TXCHEN) | \
> + SATA_DWC_DMACR_TMOD_TXCHEN)
> +#define SATA_DWC_DMACR_RX_CLEAR(v) (((v) & ~SATA_DWC_DMACR_RXCHEN) | \
> + SATA_DWC_DMACR_TMOD_TXCHEN)
> +#define SATA_DWC_DMACR_TXRXCH_CLEAR SATA_DWC_DMACR_TMOD_TXCHEN
> +
> +#define SATA_DWC_DBTSR_MWR(size) ((size/4) & \
> + SATA_DWC_TXFIFO_DEPTH)
> +#define SATA_DWC_DBTSR_MRD(size) (((size/4) & \
> + SATA_DWC_RXFIFO_DEPTH) << 16)
> +#define SATA_DWC_INTPR_DMAT 0x00000001
> +#define SATA_DWC_INTPR_NEWFP 0x00000002
> +#define SATA_DWC_INTPR_PMABRT 0x00000004
> +#define SATA_DWC_INTPR_ERR 0x00000008
> +#define SATA_DWC_INTPR_NEWBIST 0x00000010
> +#define SATA_DWC_INTPR_IPF 0x10000000
> +#define SATA_DWC_INTMR_DMATM 0x00000001
> +#define SATA_DWC_INTMR_NEWFPM 0x00000002
> +#define SATA_DWC_INTMR_PMABRTM 0x00000004
> +#define SATA_DWC_INTMR_ERRM 0x00000008
> +#define SATA_DWC_INTMR_NEWBISTM 0x00000010
> +#define SATA_DWC_LLCR_SCRAMEN 0x00000001
> +#define SATA_DWC_LLCR_DESCRAMEN 0x00000002
> +#define SATA_DWC_LLCR_RPDEN 0x00000004
ditto
> +struct sata_dwc_device {
> + struct device *dev; /* generic device struct */
> + struct ata_probe_ent *pe; /* ptr to probe-ent */
> + struct ata_host *host;
> + u8 *reg_base;
> + struct sata_dwc_regs *sata_dwc_regs; /* DW Synopsys SATA specific */
> + int irq_dma;
> +};
I think this is missing __iomem notations and 'sparse' checks
(Documentation/sparse.txt)
> + */
> +static u32 sata_dwc_sactive_issued; /* issued queued ops */
> +static u32 sata_dwc_sactive_queued; /* queued ops */
> +static struct ahb_dma_regs *sata_dma_regs;
> +static u32 dma_interrupt_count;
> +static void *scr_addr_sstatus;
> +static struct device *dwc_dev;
create a host-private struct, do not use global variables.
Global variables are discouraged because they create a barrier to
supporting multiple controller instances in a single driver -- and also
because people look at existing driver code for examples, and this
creates a poor example
> +static void sata_dwc_bmdma_start_by_tag(struct ata_queued_cmd *qc, u8 tag);
> +static int sata_dwc_qc_complete(struct ata_port *ap, struct ata_queued_cmd *qc,
> + u32 check_status);
> +static void sata_dwc_dma_xfer_complete(struct ata_port *ap, u32 check_status);
> +static void sata_dwc_port_stop(struct ata_port *ap);
> +static void sata_dwc_clear_dmacr(struct sata_dwc_device_port *hsdevp, u8 tag);
> +
> +static int dma_dwc_init(struct sata_dwc_device *hsdev, struct resource *res,
> + int irq);
> +static void dma_dwc_exit(struct sata_dwc_device *hsdev);
> +static int dma_dwc_xfer_setup(struct scatterlist *sg, int num_elems,
> + struct lli *lli, dma_addr_t dma_lli,
> + void __iomem *addr, int dir);
> +static void dma_dwc_xfer_start(int dma_ch);
> +static void dma_dwc_terminate_dma(int dma_ch);
> +
> +static const char *dir_2_txt(enum dma_data_direction dir)
> +{
> + switch (dir) {
> + case DMA_BIDIRECTIONAL:
> + return "bi";
> + case DMA_FROM_DEVICE:
> + return "from";
> + case DMA_TO_DEVICE:
> + return "to";
> + case DMA_NONE:
> + return "none";
> + default:
> + return "err";
> + }
> +}
> +
> +static const char *prot_2_txt(enum ata_tf_protocols protocol)
> +{
> + switch (protocol) {
> + case ATA_PROT_UNKNOWN:
> + return "unknown";
> + case ATA_PROT_NODATA:
> + return "nodata";
> + case ATA_PROT_PIO:
> + return "pio";
> + case ATA_PROT_DMA:
> + return "dma";
> + case ATA_PROT_NCQ:
> + return "ncq";
> + case ATAPI_PROT_PIO:
> + return "atapi pio";
> + case ATAPI_PROT_NODATA:
> + return "atapi nodata";
> + case ATAPI_PROT_DMA:
> + return "atapi dma";
> + default:
> + return "err";
> + }
> +}
> +
> +inline const char *ata_cmd_2_txt(const struct ata_taskfile *tf)
> +{
> + switch (tf->command) {
> + case ATA_CMD_CHK_POWER:
> + return "ATA_CMD_CHK_POWER";
> + case ATA_CMD_EDD:
> + return "ATA_CMD_EDD";
> + case ATA_CMD_FLUSH:
> + return "ATA_CMD_FLUSH";
> + case ATA_CMD_FLUSH_EXT:
> + return "ATA_CMD_FLUSH_EXT";
> + case ATA_CMD_ID_ATA:
> + return "ATA_CMD_ID_ATA";
> + case ATA_CMD_ID_ATAPI:
> + return "ATA_CMD_ID_ATAPI";
> + case ATA_CMD_FPDMA_READ:
> + return "ATA_CMD_FPDMA_READ";
> + case ATA_CMD_FPDMA_WRITE:
> + return "ATA_CMD_FPDMA_WRITE";
> + case ATA_CMD_READ:
> + return "ATA_CMD_READ";
> + case ATA_CMD_READ_EXT:
> + return "ATA_CMD_READ_EXT";
> + case ATA_CMD_WRITE:
> + return "ATA_CMD_WRITE";
> + case ATA_CMD_WRITE_EXT:
> + return "ATA_CMD_WRITE_EXT";
> + case ATA_CMD_PIO_READ:
> + return "ATA_CMD_PIO_READ";
> + case ATA_CMD_PIO_READ_EXT:
> + return "ATA_CMD_PIO_READ_EXT";
> + case ATA_CMD_PIO_WRITE:
> + return "ATA_CMD_PIO_WRITE";
> + case ATA_CMD_PIO_WRITE_EXT:
> + return "ATA_CMD_PIO_WRITE_EXT";
> + case ATA_CMD_SET_FEATURES:
> + return "ATA_CMD_SET_FEATURES";
> + case ATA_CMD_PACKET:
> + return "ATA_CMD_PACKET";
> + default:
> + return "ATA_CMD_???";
libata already has debugging facilties. we don't need this generic dump
stuff in a specific driver.
> +static void sata_dwc_tf_dump(struct ata_taskfile *tf)
> +{
> + dev_vdbg(dwc_dev, "taskfile cmd: 0x%02x protocol: %s flags: 0x%lx"
> + "device: %x\n", tf->command, prot_2_txt(tf->protocol),
> + tf->flags, tf->device);
> + dev_vdbg(dwc_dev, "feature: 0x%02x nsect: 0x%x lbal: 0x%x lbam:"
> + "0x%x lbah: 0x%x\n", tf->feature, tf->nsect, tf->lbal,
> + tf->lbam, tf->lbah);
> + dev_vdbg(dwc_dev, "hob_feature: 0x%02x hob_nsect: 0x%x hob_lbal: 0x%x "
> + "hob_lbam: 0x%x hob_lbah: 0x%x\n", tf->hob_feature,
> + tf->hob_nsect, tf->hob_lbal, tf->hob_lbam,
> + tf->hob_lbah);
ditto
> + * Function: get_burst_length_encode
> + * arguments: datalength: length in bytes of data
> + * returns value to be programmed in register corrresponding to data length
> + * This value is effectively the log(base 2) of the length
> + */
> +static inline int get_burst_length_encode(int datalength)
> +{
> + int items = datalength >> 2; /* div by 4 to get lword count */
> +
> + if (items >= 64)
> + return 5;
> +
> + if (items >= 32)
> + return 4;
> +
> + if (items >= 16)
> + return 3;
> +
> + if (items >= 8)
> + return 2;
> +
> + if (items >= 4)
> + return 1;
> +
> + return 0;
> +}
> +
> +static inline void clear_chan_interrupts(int c)
> +{
> + out_le32(&(sata_dma_regs->interrupt_clear.tfr.low), DMA_CHANNEL(c));
> + out_le32(&(sata_dma_regs->interrupt_clear.block.low), DMA_CHANNEL(c));
> + out_le32(&(sata_dma_regs->interrupt_clear.srctran.low), DMA_CHANNEL(c));
> + out_le32(&(sata_dma_regs->interrupt_clear.dsttran.low), DMA_CHANNEL(c));
> + out_le32(&(sata_dma_regs->interrupt_clear.error.low), DMA_CHANNEL(c));
> +}
> +
> +/*
> + * Function: dma_request_channel
> + * arguments: None
> + * returns channel number if available else -1
> + * This function assigns the next available DMA channel from the list to the
> + * requester
> + */
> +static int dma_request_channel(void)
> +{
> + int i;
> + struct ahb_dma_regs *pdma;
> +
> + pdma = sata_dma_regs;
> + for (i = 0; i < DMA_NUM_CHANS; i++) {
> + if (!(in_le32(&(pdma->dma_chan_en.low)) & DMA_CHANNEL(i)))
> + return i;
> + }
> +
> + dev_err(dwc_dev, "%s NO channel chan_en: 0x%08x\n", __func__,
> + in_le32(&(pdma->dma_chan_en.low)));
> +
> + return -1;
> +}
> +
> +/*
> + * Function: dma_dwc_interrupt
> + * arguments: irq, dev_id, pt_regs
> + * returns channel number if available else -1
> + * Interrupt Handler for DW AHB SATA DMA
> + */
> +static irqreturn_t dma_dwc_interrupt(int irq, void *hsdev_instance)
> +{
> + int chan;
> + u32 tfr_reg, err_reg;
> + unsigned long flags;
> + struct sata_dwc_device *hsdev =
> + (struct sata_dwc_device *)hsdev_instance;
> + struct ata_host *host = (struct ata_host *)hsdev->host;
> + struct ata_port *ap;
> + struct sata_dwc_device_port *hsdevp;
> + u8 tag = 0;
> + unsigned int port = 0;
> +
> + spin_lock_irqsave(&host->lock, flags);
> +
> + ap = host->ports[port];
> + hsdevp = HSDEVP_FROM_AP(ap);
> + tag = ap->link.active_tag;
> +
> + tfr_reg = in_le32(&(sata_dma_regs->interrupt_status.tfr.low));
> + err_reg = in_le32(&(sata_dma_regs->interrupt_status.error.low));
> +
> + dev_dbg(ap->dev, "eot=0x%08x err=0x%08x pending=%d active port=%d\n",
> + tfr_reg, err_reg, hsdevp->dma_pending[tag], port);
> +
> + for (chan = 0; chan < DMA_NUM_CHANS; chan++) {
> + /* Check for end-of-transfer interrupt. */
> + if (tfr_reg & DMA_CHANNEL(chan)) {
> + /*
> + *Each DMA command produces 2 interrupts. Only
> + * complete the command after both interrupts have been
> + * seen. (See sata_dwc_isr())
> + */
> + dma_interrupt_count++;
> +
> + sata_dwc_clear_dmacr(hsdevp, tag);
> +
> + if (hsdevp->dma_pending[tag] == 0) {
> + dev_err(ap->dev, "DMA not pending eot=0x%08x "
> + "err=0x%08x tag=0x%02x pending=%d\n",
> + tfr_reg, err_reg, tag,
> + hsdevp->dma_pending[tag]);
> + }
> +
> + if ((dma_interrupt_count % 2) == 0)
> + sata_dwc_dma_xfer_complete(ap, 1);
> +
> + /* Clear the interrupt */
> + out_le32(&(sata_dma_regs->interrupt_clear.tfr.low),
> + DMA_CHANNEL(chan));
> + }
> +
> + /* Check for error interrupt. */
> + if (err_reg & DMA_CHANNEL(chan)) {
> + /* TODO Need error handler ! */
> + dev_err(ap->dev, "error interrupt err_reg=0x%08x\n",
> + err_reg);
> +
> + /* Clear the interrupt. */
> + out_le32(&(sata_dma_regs->interrupt_clear.error.low),
> + DMA_CHANNEL(chan));
> + }
> + }
> +
> + spin_unlock_irqrestore(&host->lock, flags);
spin_lock_irqsave() is most likely expensive overkill. Use spin_lock()
unless you have concrete justification for something else.
> +/*
> + * Function: dma_request_interrupts
> + * arguments: hsdev
> + * returns status
> + * This function registers ISR for a particular DMA channel interrupt
> + */
> +static int dma_request_interrupts(struct sata_dwc_device *hsdev, int irq)
> +{
> + int retval = 0;
> + int chan;
> +
> + for (chan = 0; chan < DMA_NUM_CHANS; chan++) {
> + /* Unmask error interrupt */
> + out_le32(&sata_dma_regs->interrupt_mask.error.low,
> + DMA_ENABLE_CHAN(chan));
> +
> + /* Unmask end-of-transfer interrupt */
> + out_le32(&sata_dma_regs->interrupt_mask.tfr.low,
> + DMA_ENABLE_CHAN(chan));
> + }
> +
> + retval = request_irq(irq, dma_dwc_interrupt, 0, "SATA DMA", hsdev);
> + if (retval) {
> + dev_err(dwc_dev, "%s: could not get IRQ %d\n", __func__, irq);
> + return -ENODEV;
> + }
> +
> + /* Mark this interrupt as requested */
> + hsdev->irq_dma = irq;
> +
> + return 0;
> +}
> +
> +/*
> + * Function: map_sg_to_lli
> + * arguments: sg: scatter/gather list(sg)
> + * num_elems: no of elements in sg list
> + * dma_lli: LLI table
> + * dest: destination address
> + * read: whether the transfer is read or write
> + * returns array of AHB DMA Linked List Items
> + * This function creates a list of LLIs for DMA Xfr and returns the number
> + * of elements in the DMA linked list.
> + *
> + * Note that the Synopsis driver has a comment proposing that better performance
> + * is possible by only enabling interrupts on the last item in the linked list.
> + * However, it seems that could be a problem if an error happened on one of the
> + * first items. The transfer would halt, but no error interrupt would occur.
> + *
> + * Currently this function sets interrupts enabled for each linked list item:
> + * DMA_CTL_INT_EN.
> + */
> +static int map_sg_to_lli(struct scatterlist *sg, int num_elems, struct lli *lli,
> + dma_addr_t dma_lli, void __iomem *dmadr_addr, int dir)
> +{
> + int i, idx = 0;
> + int fis_len = 0;
> + dma_addr_t next_llp;
> + int bl;
> + unsigned int dma_ts = 0;
> +
> + dev_dbg(dwc_dev, "%s: sg=%p nelem=%d lli=%p dma_lli=0x%08x "
> + "dmadr=0x%08x\n", __func__, sg, num_elems, lli, (u32)dma_lli,
> + (u32)dmadr_addr);
> +
> + bl = get_burst_length_encode(AHB_DMA_BRST_DFLT);
> +
> + for (i = 0; i < num_elems; i++, sg++) {
> + u32 addr, offset;
> + u32 sg_len, len;
> +
> + addr = (u32) sg_dma_address(sg);
> + sg_len = sg_dma_len(sg);
> +
> + dev_dbg(dwc_dev, "%s: elem=%d sg_addr=0x%x sg_len=%d\n",
> + __func__, i, addr, sg_len);
> +
> + while (sg_len) {
> +
> + if (idx >= SATA_DWC_DMAC_LLI_NUM) {
> + /* The LLI table is not large enough. */
> + dev_err(dwc_dev, "LLI table overrun (idx=%d)\n",
> + idx);
> + break;
> + }
> + len = (sg_len > SATA_DWC_DMAC_CTRL_TSIZE_MAX) ?
> + SATA_DWC_DMAC_CTRL_TSIZE_MAX : sg_len;
> +
> + offset = addr & 0xffff;
> + if ((offset + sg_len) > 0x10000)
> + len = 0x10000 - offset;
> +
> + /*
> + * Make sure a LLI block is not created that will span a
> + * 8K max FIS boundary. If the block spans such a FIS
> + * boundary, there is a chance that a DMA burst will
> + * cross that boundary -- this results in an error in
> + * the host controller.
> + */
> + if (fis_len + len > 8192) {
> + dev_dbg(dwc_dev, "SPLITTING: fis_len=%d(0x%x) "
> + "len=%d(0x%x)\n", fis_len, fis_len,
> + len, len);
> + len = 8192 - fis_len;
> + fis_len = 0;
> + } else {
> + fis_len += len;
> + }
> + if (fis_len == 8192)
> + fis_len = 0;
> +
> + /*
> + * Set DMA addresses and lower half of control register
> + * based on direction.
> + */
> + if (dir == DMA_FROM_DEVICE) {
> + lli[idx].dar = cpu_to_le32(addr);
> + lli[idx].sar = cpu_to_le32((u32)dmadr_addr);
> +
> + lli[idx].ctl.low = cpu_to_le32(
> + DMA_CTL_TTFC(DMA_CTL_TTFC_P2M_DMAC) |
> + DMA_CTL_SMS(0) |
> + DMA_CTL_DMS(1) |
> + DMA_CTL_SRC_MSIZE(bl) |
> + DMA_CTL_DST_MSIZE(bl) |
> + DMA_CTL_SINC_NOCHANGE |
> + DMA_CTL_SRC_TRWID(2) |
> + DMA_CTL_DST_TRWID(2) |
> + DMA_CTL_INT_EN |
> + DMA_CTL_LLP_SRCEN |
> + DMA_CTL_LLP_DSTEN);
> + } else { /* DMA_TO_DEVICE */
> + lli[idx].sar = cpu_to_le32(addr);
> + lli[idx].dar = cpu_to_le32((u32)dmadr_addr);
> +
> + lli[idx].ctl.low = cpu_to_le32(
> + DMA_CTL_TTFC(DMA_CTL_TTFC_M2P_PER) |
> + DMA_CTL_SMS(1) |
> + DMA_CTL_DMS(0) |
> + DMA_CTL_SRC_MSIZE(bl) |
> + DMA_CTL_DST_MSIZE(bl) |
> + DMA_CTL_DINC_NOCHANGE |
> + DMA_CTL_SRC_TRWID(2) |
> + DMA_CTL_DST_TRWID(2) |
> + DMA_CTL_INT_EN |
> + DMA_CTL_LLP_SRCEN |
> + DMA_CTL_LLP_DSTEN);
> + }
> +
> + dev_dbg(dwc_dev, "%s setting ctl.high len: 0x%08x val: "
> + "0x%08x\n", __func__, len,
> + DMA_CTL_BLK_TS(len / 4));
> +
> + /* Program the LLI CTL high register */
> + dma_ts = DMA_CTL_BLK_TS(len / 4);
> + lli[idx].ctl.high = cpu_to_le32(dma_ts);
> +
> + /*
> + *Program the next pointer. The next pointer must be
> + * the physical address, not the virtual address.
> + */
> + next_llp = (dma_lli + ((idx + 1) * sizeof(struct lli)));
> +
> + /* The last 2 bits encode the list master select. */
> + next_llp = DMA_LLP_LMS(next_llp, DMA_LLP_AHBMASTER2);
> +
> + lli[idx].llp = cpu_to_le32(next_llp);
> +
> + idx++;
> + sg_len -= len;
> + addr += len;
> + }
> + }
> +
> + /*
> + * The last next ptr has to be zero and the last control low register
> + * has to have LLP_SRC_EN and LLP_DST_EN (linked list pointer source
> + * and destination enable) set back to 0 (disabled.) This is what tells
> + * the core that this is the last item in the linked list.
> + */
> + if (idx) {
> + lli[idx-1].llp = 0x00000000;
> + lli[idx-1].ctl.low &= DMA_CTL_LLP_DISABLE_LE32;
> +
> + /* Flush cache to memory */
> + dma_cache_sync(NULL, lli, (sizeof(struct lli) * idx),
> + DMA_BIDIRECTIONAL);
> + }
> +
> + return idx;
> +}
> +
> +/*
> + * Function: dma_dwc_xfer_start
> + * arguments: Channel number
> + * Return : None
> + * Enables the DMA channel
> + */
> +static void dma_dwc_xfer_start(int dma_ch)
> +{
> + /* Enable the DMA channel */
> + out_le32(&(sata_dma_regs->dma_chan_en.low),
> + in_le32(&(sata_dma_regs->dma_chan_en.low)) |
> + DMA_ENABLE_CHAN(dma_ch));
> +}
> +
> +static int dma_dwc_channel_enabled(int ch)
> +{
> + if (in_le32(&(sata_dma_regs->dma_chan_en.low)) & DMA_CHANNEL(ch))
> + return 1;
> +
> + return 0;
> +}
> +
> +static void dma_dwc_terminate_dma(int dma_ch)
> +{
> + int enabled = dma_dwc_channel_enabled(dma_ch);
> +
> + dev_info(dwc_dev, "%s terminate DMA on channel=%d enabled=%d\n",
> + __func__, dma_ch, enabled);
> +
> + if (enabled) {
> + out_le32(&(sata_dma_regs->dma_chan_en.low),
> + DMA_DISABLE_CHAN(dma_ch));
> +
> + do {
> + enabled = dma_dwc_channel_enabled(dma_ch);
> + } while (enabled);
> + }
> +}
> +
> +static int dma_dwc_xfer_setup(struct scatterlist *sg, int num_elems,
> + struct lli *lli, dma_addr_t dma_lli,
> + void __iomem *addr, int dir)
> +{
> + int dma_ch;
> + int num_lli;
> +
> + /* Acquire DMA channel */
> + dma_ch = dma_request_channel();
> + if (dma_ch == -1) {
> + dev_err(dwc_dev, "%s: dma channel unavailable\n", __func__);
> + return -EAGAIN;
> + }
> +
> + /* Convert SG list to linked list of items (LLIs) for AHB DMA */
> + num_lli = map_sg_to_lli(sg, num_elems, lli, dma_lli, addr, dir);
> +
> + dev_dbg(dwc_dev, "%s sg: 0x%p, count: %d lli: %p dma_lli: 0x%0xlx addr:"
> + " %p lli count: %d\n", __func__, sg, num_elems, lli,
> + (u32)dma_lli, addr, num_lli);
> +
> + /* Clear channel interrupts */
> + clear_chan_interrupts(dma_ch);
> +
> + /* Program the CFG register. */
> + out_le32(&(sata_dma_regs->chan_regs[dma_ch].cfg.high),
> + DMA_CFG_PROTCTL | DMA_CFG_FCMOD_REQ);
> + out_le32(&(sata_dma_regs->chan_regs[dma_ch].cfg.low), 0);
> +
> + /* Program the address of the linked list */
> + out_le32(&(sata_dma_regs->chan_regs[dma_ch].llp.low),
> + DMA_LLP_LMS(dma_lli, DMA_LLP_AHBMASTER2));
> +
> + /* Program the CTL register with src enable / dst enable */
> + out_le32(&(sata_dma_regs->chan_regs[dma_ch].ctl.low),
> + DMA_CTL_LLP_SRCEN | DMA_CTL_LLP_DSTEN);
> +
> + return 0;
> +}
> +
add this is inside a spinlock, at all times, I hope?
> +static void dma_dwc_exit(struct sata_dwc_device *hsdev)
> +{
> + dev_dbg(dwc_dev, "%s:\n", __func__);
> + if (sata_dma_regs)
> + iounmap(sata_dma_regs);
> +
> + if (hsdev->irq_dma)
> + free_irq(hsdev->irq_dma, hsdev);
> +}
> +
> +/*
> + * Function: dma_dwc_init
> + * arguments: hsdev
> + * returns status
> + * This function initializes the SATA DMA driver
> + */
> +static int dma_dwc_init(struct sata_dwc_device *hsdev, struct resource *res,
> + int irq)
> +{
> + int err;
> +
> + sata_dma_regs = ioremap(res->start, res->end - res->start + 1);
> + if (!sata_dma_regs) {
> + dev_err(dwc_dev, "%s: ioremap failed\n", __func__);
> + err = -ENODEV;
> + goto error_out;
> + }
> +
> + err = dma_request_interrupts(hsdev, irq);
> + if (err) {
> + dev_err(dwc_dev, "%s: dma_request_interrupts returns %d\n",
> + __func__, err);
> + goto error_out;
> + }
> +
> + /* Enabe DMA */
> + out_le32(&(sata_dma_regs->dma_cfg.low), DMA_EN);
> +
> + dev_notice(dwc_dev, "DMA initialized\n");
> + dev_dbg(dwc_dev, "SATA DMA registers=0x%p\n", sata_dma_regs);
> +
> + return 0;
> +
> +error_out:
> + dma_dwc_exit(hsdev);
> +
> + return err;
> +}
> +
> +static int sata_dwc_scr_read(struct ata_link *link, unsigned int scr, u32 *val)
> +{
> + if (scr > SCR_NOTIFICATION) {
> + dev_err(link->ap->dev, "%s: Incorrect SCR offset 0x%02x\n",
> + __func__, scr);
> + return -EINVAL;
> + }
> +
> + *val = in_le32((void *)link->ap->ioaddr.scr_addr + (scr * 4));
> + dev_dbg(link->ap->dev, "%s: id=%d reg=%d val=val=0x%08x\n",
> + __func__, link->ap->print_id, scr, *val);
> +
> + return 0;
> +}
> +
> +static int sata_dwc_scr_write(struct ata_link *link, unsigned int scr, u32 val)
> +{
> + dev_dbg(link->ap->dev, "%s: id=%d reg=%d val=val=0x%08x\n",
> + __func__, link->ap->print_id, scr, val);
> + if (scr > SCR_NOTIFICATION) {
> + dev_err(link->ap->dev, "%s: Incorrect SCR offset 0x%02x\n",
> + __func__, scr);
> + return -EINVAL;
> + }
> + out_le32((void *)link->ap->ioaddr.scr_addr + (scr * 4), val);
> +
> + return 0;
> +}
> +
> +static inline u32 core_scr_read(unsigned int scr)
> +{
> + return in_le32((void __iomem *)scr_addr_sstatus + (scr * 4));
> +}
> +
> +static inline void core_scr_write(unsigned int scr, u32 val)
> +{
> + out_le32((void __iomem *)scr_addr_sstatus + (scr * 4), val);
> +}
> +
> +static inline void clear_serror(void)
> +{
> + out_le32((void __iomem *)scr_addr_sstatus + 4,
> + in_le32((void __iomem *)scr_addr_sstatus + 4));
> +}
adding __iomem casts indicates you are _missing_ iomem casts elsewhere
> +static inline void clear_intpr(struct sata_dwc_device *hsdev)
> +{
> + out_le32(&hsdev->sata_dwc_regs->intpr,
> + in_le32(&hsdev->sata_dwc_regs->intpr));
> +}
> +
> +static inline void clear_interrupt_bit(struct sata_dwc_device *hsdev, u32 bit)
> +{
> + out_le32(&hsdev->sata_dwc_regs->intpr,
> + in_le32(&hsdev->sata_dwc_regs->intpr));
> +}
> +
> +static inline void disable_err_irq(struct sata_dwc_device *hsdev)
> +{
> + out_le32(&hsdev->sata_dwc_regs->intmr,
> + in_le32(&hsdev->sata_dwc_regs->intmr) & ~SATA_DWC_INTMR_ERRM);
> + out_le32(&hsdev->sata_dwc_regs->errmr, ~SATA_DWC_SERROR_ERR_BITS);
> +}
> +
> +static inline void enable_err_irq(struct sata_dwc_device *hsdev)
> +{
> + out_le32(&hsdev->sata_dwc_regs->intmr,
> + in_le32(&hsdev->sata_dwc_regs->intmr) | SATA_DWC_INTMR_ERRM);
> + out_le32(&hsdev->sata_dwc_regs->errmr, SATA_DWC_SERROR_ERR_BITS);
> +}
> +
> +static inline u32 qcmd_tag_to_mask(u8 tag)
> +{
> + return 0x00000001 << (tag & 0x1f);
> +}
> +
> +/* See ahci.c */
> +static void sata_dwc_error_intr(struct ata_port *ap,
> + struct sata_dwc_device *hsdev, uint intpr)
> +{
> + struct sata_dwc_device_port *hsdevp = HSDEVP_FROM_AP(ap);
> + struct ata_eh_info *ehi = &ap->link.eh_info;
> + unsigned int err_mask = 0, action = 0;
> + struct ata_queued_cmd *qc;
> + u32 serror;
> + u8 status, tag;
> + u32 err_reg;
> +
> + ata_ehi_clear_desc(ehi);
> +
> + serror = core_scr_read(SCR_ERROR);
> + status = ap->ops->sff_check_status(ap);
> +
> + err_reg = in_le32(&(sata_dma_regs->interrupt_status.error.low));
> + tag = ap->link.active_tag;
> +
> + dev_err(ap->dev, "%s SCR_ERROR=0x%08x intpr=0x%08x status=0x%08x "
> + "dma_intp=%d pending=%d issued=%d dma_err_status=0x%08x\n",
> + __func__, serror, intpr, status, dma_interrupt_count,
> + hsdevp->dma_pending[tag], hsdevp->cmd_issued[tag], err_reg);
> +
> + /* Clear error register and interrupt bit */
> + clear_serror();
> + clear_interrupt_bit(hsdev, SATA_DWC_INTPR_ERR);
> +
> + /* This is the only error happening now. */
> + /* TODO check for exact error */
> + err_mask |= AC_ERR_HOST_BUS;
> + action |= ATA_EH_RESET;
> +
> + /* Pass this on to EH */
> + ehi->serror |= serror;
> + ehi->action |= action;
> +
> + qc = ata_qc_from_tag(ap, tag);
> + if (qc)
> + qc->err_mask |= err_mask;
> + else
> + ehi->err_mask |= err_mask;
> +
> + ata_port_abort(ap);
> +
> + /*
> + if (irq_stat & PORT_IRQ_FREEZE)
> + ata_port_freeze(ap);
> + else
> + ata_port_abort(ap);
> + */
> +}
> +
> +/*
> + * Function : sata_dwc_isr
> + * arguments : irq, void *dev_instance, struct pt_regs *regs
> + * Return value : irqreturn_t - status of IRQ
> + * This Interrupt handler called via port ops registered function.
> + * .irq_handler = sata_dwc_isr
> + */
> +static irqreturn_t sata_dwc_isr(int irq, void *dev_instance)
> +{
> + struct ata_host *host = (struct ata_host *)dev_instance;
> + struct sata_dwc_device *hsdev = HSDEV_FROM_HOST(host);
> + struct ata_port *ap;
> + struct ata_queued_cmd *qc;
> + unsigned long flags;
> + u8 status, tag;
> + int handled, num_processed, port = 0;
> + uint intpr, sactive, sactive2, tag_mask;
> + struct sata_dwc_device_port *hsdevp;
> +
> + spin_lock_irqsave(&host->lock, flags);
> +
> + /* Read the interrupt register */
> + intpr = in_le32(&hsdev->sata_dwc_regs->intpr);
> +
> + ap = host->ports[port];
> + hsdevp = HSDEVP_FROM_AP(ap);
> +
> + dev_dbg(ap->dev, "%s intpr=0x%08x active_tag=%d\n", __func__, intpr,
> + ap->link.active_tag);
> +
> + /* Check for error interrupt */
> + if (intpr & SATA_DWC_INTPR_ERR) {
> + sata_dwc_error_intr(ap, hsdev, intpr);
> + handled = 1;
> + goto done_irqrestore;
> + }
> +
> + /* Check for DMA SETUP FIS (FP DMA) interrupt */
> + if (intpr & SATA_DWC_INTPR_NEWFP) {
> + clear_interrupt_bit(hsdev, SATA_DWC_INTPR_NEWFP);
> +
> + tag = (u8)(in_le32(&hsdev->sata_dwc_regs->fptagr));
> + dev_dbg(ap->dev, "%s: NEWFP tag=%d\n", __func__, tag);
> + if (hsdevp->cmd_issued[tag] != SATA_DWC_CMD_ISSUED_PEND)
> + dev_warn(ap->dev, "CMD tag=%d not pending?\n", tag);
> +
> + sata_dwc_sactive_issued |= qcmd_tag_to_mask(tag);
> +
> + qc = ata_qc_from_tag(ap, tag);
> + /*
> + * Start FP DMA for NCQ command. At this point the tag is the
> + * active tag. It is the tag that matches the command about to
> + * be completed.
> + */
> + qc->ap->link.active_tag = tag;
> + sata_dwc_bmdma_start_by_tag(qc, tag);
> +
> + handled = 1;
> + goto done_irqrestore;
> + }
> +
> + sactive = core_scr_read(SCR_ACTIVE);
> + tag_mask = (sata_dwc_sactive_issued | sactive) ^ sactive;
> +
> + /* If no sactive issued and tag_mask is zero then this is not NCQ */
> + if (sata_dwc_sactive_issued == 0 && tag_mask == 0) {
> + tag = 0;
> + qc = ata_qc_from_tag(ap, tag);
> +
> + /* DEV interrupt w/ no active qc? */
> + if (unlikely(!qc || (qc->tf.flags & ATA_TFLAG_POLLING))) {
> + dev_err(ap->dev, "%s intr with no active qc qc=%p\n",
> + __func__, qc);
> + ata_sff_check_status(ap);
> + handled = 1;
> + goto done_irqrestore;
> + }
> +
> + status = ap->ops->sff_check_status(ap);
> +
> + qc->ap->link.active_tag = tag;
> + hsdevp->cmd_issued[tag] = SATA_DWC_CMD_ISSUED_NOT;
> +
> + if (status & ATA_ERR) {
> + dev_dbg(ap->dev, "interrupt ATA_ERR (0x%x)\n", status);
> + sata_dwc_qc_complete(ap, qc, 1);
> + handled = 1;
> + goto done_irqrestore;
> + }
> +
> + dev_dbg(ap->dev, "%s non-NCQ cmd interrupt, protocol: %s\n",
> + __func__, prot_2_txt(qc->tf.protocol));
> +drv_still_busy:
> + if (ata_is_dma(qc->tf.protocol)) {
> + int dma_flag = hsdevp->dma_pending[tag];
> + /*
> + * Each DMA transaction produces 2 interrupts. The DMAC
> + * transfer complete interrupt and the SATA controller
> + * operation done interrupt. The command should be
> + * completed only after both interrupts are seen.
> + */
> + dma_interrupt_count++;
> + if (dma_flag == SATA_DWC_DMA_PEND_NONE) {
> + dev_err(ap->dev, "%s: DMA not pending "
> + "intpr=0x%08x status=0x%08x pend=%d\n",
> + __func__, intpr, status, dma_flag);
> + }
> +
> + if ((dma_interrupt_count % 2) == 0)
> + sata_dwc_dma_xfer_complete(ap, 1);
> + } else {
> + if (unlikely(sata_dwc_qc_complete(ap, qc, 1)))
> + goto drv_still_busy;
> + }
> +
> + handled = 1;
> + goto done_irqrestore;
> + }
> +
> + /*
> + * This is a NCQ command. At this point we need to figure out for which
> + * tags we have gotten a completion interrupt. One interrupt may serve
> + * as completion for more than one operation when commands are queued
> + * (NCQ). We need to process each completed command.
> + */
> +
> +process_cmd: /* process completed commands */
> + sactive = core_scr_read(SCR_ACTIVE);
> + tag_mask = (sata_dwc_sactive_issued | sactive) ^ sactive;
> +
> + if (sactive != 0 || sata_dwc_sactive_issued > 1 || tag_mask > 1) {
> + dev_dbg(ap->dev, "%s NCQ: sactive=0x%08x sactive_issued=0x%08x"
> + " tag_mask=0x%08x\n", __func__, sactive,
> + sata_dwc_sactive_issued, tag_mask);
> + }
> +
> + if ((tag_mask | sata_dwc_sactive_issued) != sata_dwc_sactive_issued) {
> + dev_warn(ap->dev, "Bad tag mask? sactive=0x%08x "
> + "sata_dwc_sactive_issued=0x%08x tag_mask=0x%08x\n",
> + sactive, sata_dwc_sactive_issued, tag_mask);
> + }
> +
> + /* read just to clear ... not bad if currently still busy */
> + status = ap->ops->sff_check_status(ap);
> + dev_dbg(ap->dev, "%s ATA status register=0x%x\n", __func__, status);
> +
> + tag = 0;
> + num_processed = 0;
> + while (tag_mask) {
> + num_processed++;
> + while (!(tag_mask & 0x00000001)) {
> + tag++;
> + tag_mask <<= 1;
> + }
> + tag_mask &= (~0x00000001);
> + qc = ata_qc_from_tag(ap, tag);
> +
> + /* To be picked up by completion functions */
> + qc->ap->link.active_tag = tag;
> + hsdevp->cmd_issued[tag] = SATA_DWC_CMD_ISSUED_NOT;
> +
> + /* Let libata/scsi layers handle error */
> + if (status & ATA_ERR) {
> + dev_dbg(ap->dev, "%s ATA_ERR (0x%x)\n",
> + __func__, status);
> +
> + sata_dwc_qc_complete(ap, qc, 1);
> + handled = 1;
> + goto done_irqrestore;
> + }
> +
> + /* Process completed command */
> + dev_dbg(ap->dev, "%s NCQ command, protocol: %s\n", __func__,
> + prot_2_txt(qc->tf.protocol));
> + if (ata_is_dma(qc->tf.protocol)) {
> + dma_interrupt_count++;
> + if (hsdevp->dma_pending[tag] == SATA_DWC_DMA_PEND_NONE)
> + dev_warn(ap->dev,
> + "%s: DMA not pending?\n", __func__);
> + if ((dma_interrupt_count % 2) == 0)
> + sata_dwc_dma_xfer_complete(ap, 1);
> + } else {
> + if (unlikely(sata_dwc_qc_complete(ap, qc, 1)))
> + goto still_busy;
> + }
> + continue;
> +
> +still_busy:
> + ap->stats.idle_irq++;
> + dev_warn(ap->dev, "STILL BUSY IRQ ata%d: irq trap\n",
> + ap->print_id);
> + } /* while tag_mask */
> +
> + /*
> + * Check to see if any commands completed while we were processing our
> + * initial set of completed commands (reading of status clears
> + * interrupts, so we might miss a completed command interrupt if one
> + * came in while we were processing:
> + * we read status as part of processing a completed command).
> + */
> + sactive2 = core_scr_read(SCR_ACTIVE);
> + if (sactive2 != sactive) {
> + dev_dbg(ap->dev, "More finished - sactive=0x%x sactive2=0x%x\n",
> + sactive, sactive2);
> + goto process_cmd;
> + }
> + handled = 1;
> +
> +done_irqrestore:
> + spin_unlock_irqrestore(&host->lock, flags);
> + return IRQ_RETVAL(handled);
1) it is completely silly to have a function this large. break it up
into multiple, smaller functions.
2) same comment as above -- spin_lock_irqsave() is both expensive and
probably wrong, for this irq handler
> +static void sata_dwc_clear_dmacr(struct sata_dwc_device_port *hsdevp, u8 tag)
> +{
> + struct sata_dwc_device *hsdev = HSDEV_FROM_HSDEVP(hsdevp);
> +
> + if (hsdevp->dma_pending[tag] == SATA_DWC_DMA_PEND_RX) {
> + out_le32(&(hsdev->sata_dwc_regs->dmacr),
> + SATA_DWC_DMACR_RX_CLEAR(
> + in_le32(&(hsdev->sata_dwc_regs->dmacr))));
> + } else if (hsdevp->dma_pending[tag] == SATA_DWC_DMA_PEND_TX) {
> + out_le32(&(hsdev->sata_dwc_regs->dmacr),
> + SATA_DWC_DMACR_TX_CLEAR(
> + in_le32(&(hsdev->sata_dwc_regs->dmacr))));
> + } else {
> + /*
> + * This should not happen, it indicates the driver is out of
> + * sync. If it does happen, clear dmacr anyway.
> + */
> + dev_err(dwc_dev, "%s DMA protocol RX and TX DMA not pending "
> + "tag=0x%02x pending=%d dmacr: 0x%08x\n",
> + __func__, tag, hsdevp->dma_pending[tag],
> + in_le32(&(hsdev->sata_dwc_regs->dmacr)));
> + out_le32(&(hsdev->sata_dwc_regs->dmacr),
> + SATA_DWC_DMACR_TXRXCH_CLEAR);
> + }
> +}
> +
> +static void sata_dwc_dma_xfer_complete(struct ata_port *ap, u32 check_status)
> +{
> + struct ata_queued_cmd *qc;
> + struct sata_dwc_device_port *hsdevp = HSDEVP_FROM_AP(ap);
> + struct sata_dwc_device *hsdev = HSDEV_FROM_AP(ap);
> + u8 tag = 0;
> +
> + tag = ap->link.active_tag;
> + qc = ata_qc_from_tag(ap, tag);
> +
> +#ifdef DEBUG_NCQ
> + if (tag > 0) {
> + dev_info(ap->dev, "%s tag=%u cmd=0x%02x dma dir=%s proto=%s "
> + "dmacr=0x%08x\n", __func__, qc->tag, qc->tf.command,
> + dir_2_txt(qc->dma_dir), prot_2_txt(qc->tf.protocol),
> + in_le32(&(hsdev->sata_dwc_regs->dmacr)));
> + }
> +#endif
> +
> + if (ata_is_dma(qc->tf.protocol)) {
> + if (hsdevp->dma_pending[tag] == SATA_DWC_DMA_PEND_NONE) {
> + dev_err(ap->dev, "%s DMA protocol RX and TX DMA not "
> + "pending dmacr: 0x%08x\n", __func__,
> + in_le32(&(hsdev->sata_dwc_regs->dmacr)));
> + }
> +
> + hsdevp->dma_pending[tag] = SATA_DWC_DMA_PEND_NONE;
> + sata_dwc_qc_complete(ap, qc, check_status);
> + ap->link.active_tag = ATA_TAG_POISON;
> + } else {
> + sata_dwc_qc_complete(ap, qc, check_status);
> + }
> +}
> +
> +static int sata_dwc_qc_complete(struct ata_port *ap, struct ata_queued_cmd *qc,
> + u32 check_status)
> +{
> + u8 status = 0;
> + int i = 0;
> + u32 mask = 0x0;
> + u8 tag = qc->tag;
> + struct sata_dwc_device_port *hsdevp = HSDEVP_FROM_AP(ap);
> + u32 serror;
> +
> + dev_dbg(ap->dev, "%s checkstatus? %x\n", __func__, check_status);
> +
> + if (hsdevp->dma_pending[tag] == SATA_DWC_DMA_PEND_TX)
> + dev_err(ap->dev, "TX DMA PENDING\n");
> + else if (hsdevp->dma_pending[tag] == SATA_DWC_DMA_PEND_RX)
> + dev_err(ap->dev, "RX DMA PENDING\n");
> +
> + if (check_status) {
> + i = 0;
> + do {
> + /* check main status, clearing INTRQ */
> + status = ap->ops->sff_check_status(ap);
> + if (status & ATA_BUSY) {
> + dev_dbg(ap->dev, "STATUS BUSY (0x%02x) [%d]\n",
> + status, i);
> + }
> + if (++i > 10)
> + break;
> + } while (status & ATA_BUSY);
> +
> + status = ap->ops->sff_check_status(ap);
> + if (unlikely(status & ATA_BUSY))
> + dev_err(ap->dev, "QC complete cmd=0x%02x STATUS BUSY "
> + "(0x%02x) [%d]\n", qc->tf.command, status, i);
> + serror = core_scr_read(SCR_ERROR);
> + if (serror & SATA_DWC_SERROR_ERR_BITS)
> + dev_err(ap->dev, "****** SERROR=0x%08x ******\n",
> + serror);
> + }
it is both silly and expensive to call a hook from your own driver.
just call the function directly!
> + dev_dbg(ap->dev, "QC complete cmd=0x%02x status=0x%02x ata%u: "
> + "protocol=%d\n", qc->tf.command, status, ap->print_id,
> + qc->tf.protocol);
> +
> + /* clear active bit */
> + mask = (~(qcmd_tag_to_mask(tag)));
> + sata_dwc_sactive_queued = sata_dwc_sactive_queued & mask;
> + sata_dwc_sactive_issued = sata_dwc_sactive_issued & mask;
> +
> + /* Complete taskfile transaction (does not read SCR registers) */
> + ata_qc_complete(qc);
> +
> + return 0;
> +}
> +
> +static void sata_dwc_enable_interrupts(struct sata_dwc_device *hsdev)
> +{
> + /* Enable selective interrupts by setting the interrupt mask register */
> + out_le32(&hsdev->sata_dwc_regs->intmr,
> + SATA_DWC_INTMR_ERRM |
> + SATA_DWC_INTMR_NEWFPM |
> + SATA_DWC_INTMR_PMABRTM |
> + SATA_DWC_INTMR_DMATM);
> +
> + /*
> + * Unmask the error bits that should trigger an error interrupt by
> + * setting the error mask register.
> + */
> + out_le32(&hsdev->sata_dwc_regs->errmr, SATA_DWC_SERROR_ERR_BITS);
> +
> + dev_dbg(dwc_dev, "%s: INTMR = 0x%08x, ERRMR = 0x%08x\n", __func__,
> + in_le32(&hsdev->sata_dwc_regs->intmr),
> + in_le32(&hsdev->sata_dwc_regs->errmr));
> +}
> +
> +static void sata_dwc_setup_port(struct ata_ioports *port, unsigned long base)
> +{
> + port->cmd_addr = (void *)base + 0x00;
> + port->data_addr = (void *)base + 0x00;
> +
> + port->error_addr = (void *)base + 0x04;
> + port->feature_addr = (void *)base + 0x04;
> +
> + port->nsect_addr = (void *)base + 0x08;
> +
> + port->lbal_addr = (void *)base + 0x0c;
> + port->lbam_addr = (void *)base + 0x10;
> + port->lbah_addr = (void *)base + 0x14;
> +
> + port->device_addr = (void *)base + 0x18;
> + port->command_addr = (void *)base + 0x1c;
> + port->status_addr = (void *)base + 0x1c;
> +
> + port->altstatus_addr = (void *)base + 0x20;
> + port->ctl_addr = (void *)base + 0x20;
casts indicate a C typing bug
^ 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