* Re: [PATCH v9 11/13] powerpc: select HAVE_SECCOMP_FILTER and provide seccomp_execve
From: Benjamin Herrenschmidt @ 2011-11-28 0:14 UTC (permalink / raw)
To: Will Drewry
Cc: fweisbec, scarybeasts, djm, linux-kernel, rostedt, jmorris,
torvalds, kees.cook, Paul Mackerras, mingo, tglx, linuxppc-dev,
segoon
In-Reply-To: <1314682083.2488.73.camel@pasglop>
On Tue, 2011-08-30 at 15:28 +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2011-06-23 at 19:36 -0500, Will Drewry wrote:
> > Facilitate the use of CONFIG_SECCOMP_FILTER by wrapping compatibility
> > system call numbering for execve and selecting HAVE_SECCOMP_FILTER.
> >
> > v9: rebase on to bccaeafd7c117acee36e90d37c7e05c19be9e7bf
> >
> > Signed-off-by: Will Drewry <wad@chromium.org>
>
> Seen these around for a while ... :-)
>
> I don't see a harm in the patches per-se tho I haven't reviewed the
> actual seccomp filter stuff and it's good (or bad) behaviour on ppc.
Did that stuff every got anywhere ? I don't see HAVE_SECCOMP_FILTER
upsteam ... should I just drop the powerpc patch from patchwork ?
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 01/16] pmac_zilog: fix unexpected irq
From: Benjamin Herrenschmidt @ 2011-11-28 0:30 UTC (permalink / raw)
To: Finn Thain
Cc: Geert Uytterhoeven, linux-m68k, linuxppc-dev, linux-serial,
Alan Cox
In-Reply-To: <alpine.LNX.2.00.1111250207040.6314@nippy.intranet>
> Removing ifdefs makes the changes more invasive and the suspend/resume
> code then has to be addressed, which I've avoided.
>
> The suspend/resume code path can't be tested on m68k macs and the common
> code paths I can't easily test on a powermac.
>
> This patch should not be needed because the chip reset shouldn't leave the
> tx and rx interrupts enabled. Those interrupts are explicitly enabled only
> after request_irq(), so patching the master interrupt enable behaviour
> should be redundant. But that's not the case in practice.
>
> The chip reset code is already messy. I was inclined towards ifdefs and
> reluctant to share more code after practical experience suggested possible
> differences in the SCC/ESCC devices.
>
> I guess I was hoping that the powermac maintainers might prefer ifdefs to
> increased risk of destabilising the driver on powermacs...
>
> But a more invasive patch would make for better code. I will see if I can
> borrow a suitable PCI PowerMac.
Please do the more invasive patch, I'll beat it up on powermacs.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH v9 11/13] powerpc: select HAVE_SECCOMP_FILTER and provide seccomp_execve
From: Will Drewry @ 2011-11-28 1:45 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: fweisbec, scarybeasts, djm, linux-kernel, rostedt, jmorris,
torvalds, kees.cook, Paul Mackerras, mingo, tglx, linuxppc-dev,
segoon
In-Reply-To: <1322439281.23348.34.camel@pasglop>
On Sun, Nov 27, 2011 at 6:14 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Tue, 2011-08-30 at 15:28 +1000, Benjamin Herrenschmidt wrote:
>> On Thu, 2011-06-23 at 19:36 -0500, Will Drewry wrote:
>> > Facilitate the use of CONFIG_SECCOMP_FILTER by wrapping compatibility
>> > system call numbering for execve and selecting HAVE_SECCOMP_FILTER.
>> >
>> > v9: rebase on to bccaeafd7c117acee36e90d37c7e05c19be9e7bf
>> >
>> > Signed-off-by: Will Drewry <wad@chromium.org>
>>
>> Seen these around for a while ... :-)
>>
>> I don't see a harm in the patches per-se tho I haven't reviewed the
>> actual seccomp filter stuff and it's good (or bad) behaviour on ppc.
>
> Did that stuff every got anywhere ? I don't see HAVE_SECCOMP_FILTER
> upsteam ... should I just drop the powerpc patch from patchwork ?
Thanks for following up! At present, it stalled out, so I think it'd
make sense to just drop it. I will repost the series soon-ish and see
if any progress can be made. [I've explored a number of alternative
approaches and still think something along the seccomp_filter lines is
still rational.]
cheers!
will
^ permalink raw reply
* Re: [PATCH 1/2] [hw-breakpoint] Use generic hw-breakpoint interfaces for new PPC ptrace flags
From: David Gibson @ 2011-11-28 3:11 UTC (permalink / raw)
To: K.Prasad; +Cc: linuxppc-dev, Thiago Jung Bauermann, Edjunior Barbosa Machado
In-Reply-To: <20111012173948.GA4340@in.ibm.com>
[snip]
On Wed, Oct 12, 2011 at 11:09:48PM +0530, K.Prasad wrote:
> > > + if (bp) {
> > > + attr = bp->attr;
> > > + attr.bp_addr = (unsigned long)bp_info->addr & ~HW_BREAKPOINT_ALIGN;
> > > + arch_bp_generic_fields(dabr &
> > > + (DABR_DATA_WRITE | DABR_DATA_READ),
> > > + &attr.bp_type);
> > > + attr.bp_len = len;
> >
> > If gdb is using the new breakpoint interface, surely it should just
> > use it, rather than doing this bit frobbing as in the old SET_DABR
> > call.
> >
>
> I understand that you wanted to avoid this duplication of effort in terms
> of encoding and decoding the breakpoint type from
> PPC_BREAKPOINT_TRIGGER_READ to DABR_DATA_READ to HW_BREAKPOINT_R.
>
> However HW_BREAKPOINT_R is a generic definition used across
> architectures, DABR_DATA_READ is used in the !CONFIG_HAVE_HW_BREAKPOINT
> case while PPC_BREAKPOINT_TRIGGER_READ is used in
> CONFIG_PPC_ADV_DEBUG_REGS case.
>
> While we could define PPC_BREAKPOINT_TRIGGER_READ and DABR_DATA_READ to
> the same value it may not result in any code savings (since the bit
> translation is done for !CONFIG_HAVE_HW_BREAKPOINT case anyway). So, I
> think it is best left the way it is.
That's not what I'm suggesting. What I'm saying is that ig userspace
is using the new generic interface, then it should just set the
bp_type field, and it should not use the DABR_DATA_{READ,WRITE} bits.
The DABR_DATA bits should *only* be processed in the legacy interface,
never in the generic interface.
> I'm attaching the revised patch (after incorporating your comments).
> Kindly let me know your comments.
>
> Thanks,
> K.Prasad
> ---------
>
> [hw-breakpoint] Use generic hw-breakpoint interfaces for new PPC ptrace flags
>
> PPC_PTRACE_GETHWDBGINFO, PPC_PTRACE_SETHWDEBUG and PPC_PTRACE_DELHWDEBUG are
> PowerPC specific ptrace flags that use the watchpoint register. While they are
> targeted primarily towards BookE users, user-space applications such as GDB
> have started using them for BookS too. This patch enables the use of generic
> hardware breakpoint interfaces for these new flags.
>
> Apart from the usual benefits of using generic hw-breakpoint interfaces, these
> changes allow debuggers (such as GDB) to use a common set of ptrace flags for
> their watchpoint needs and allow more precise breakpoint specification (length
> of the variable can be specified).
>
> Tested-by: Edjunior Barbosa Machado <emachado@linux.vnet.ibm.com>
> Signed-off-by: K.Prasad <prasad@linux.vnet.ibm.com>
> ---
> Documentation/powerpc/ptrace.txt | 16 +++++++
> arch/powerpc/kernel/ptrace.c | 88 +++++++++++++++++++++++++++++++++++---
> 2 files changed, 98 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/powerpc/ptrace.txt b/Documentation/powerpc/ptrace.txt
> index f4a5499..f2a7a39 100644
> --- a/Documentation/powerpc/ptrace.txt
> +++ b/Documentation/powerpc/ptrace.txt
> @@ -127,6 +127,22 @@ Some examples of using the structure to:
> p.addr2 = (uint64_t) end_range;
> p.condition_value = 0;
>
> +- set a watchpoint in server processors (BookS)
> +
> + p.version = 1;
> + p.trigger_type = PPC_BREAKPOINT_TRIGGER_RW;
> + p.addr_mode = PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE;
> + or
> + p.addr_mode = PPC_BREAKPOINT_MODE_EXACT;
> +
> + p.condition_mode = PPC_BREAKPOINT_CONDITION_NONE;
> + p.addr = (uint64_t) begin_range;
You should probably document the alignment constraint on the address
here, too.
> + /* For PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE addr2 needs to be specified, where
> + * addr2 - addr <= 8 Bytes.
> + */
> + p.addr2 = (uint64_t) end_range;
> + p.condition_value = 0;
> +
> 3. PTRACE_DELHWDEBUG
>
> Takes an integer which identifies an existing breakpoint or watchpoint
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index 05b7dd2..be5dc57 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -1339,6 +1339,12 @@ static int set_dac_range(struct task_struct *child,
> static long ppc_set_hwdebug(struct task_struct *child,
> struct ppc_hw_breakpoint *bp_info)
> {
> +#ifdef CONFIG_HAVE_HW_BREAKPOINT
> + int ret, len = 0;
> + struct thread_struct *thread = &(child->thread);
> + struct perf_event *bp;
> + struct perf_event_attr attr;
> +#endif /* CONFIG_HAVE_HW_BREAKPOINT */
> #ifndef CONFIG_PPC_ADV_DEBUG_REGS
> unsigned long dabr;
> #endif
> @@ -1382,13 +1388,9 @@ static long ppc_set_hwdebug(struct task_struct *child,
> */
> if ((bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_RW) == 0 ||
> (bp_info->trigger_type & ~PPC_BREAKPOINT_TRIGGER_RW) != 0 ||
> - bp_info->addr_mode != PPC_BREAKPOINT_MODE_EXACT ||
> bp_info->condition_mode != PPC_BREAKPOINT_CONDITION_NONE)
> return -EINVAL;
>
> - if (child->thread.dabr)
> - return -ENOSPC;
> -
> if ((unsigned long)bp_info->addr >= TASK_SIZE)
> return -EIO;
>
> @@ -1398,15 +1400,75 @@ static long ppc_set_hwdebug(struct task_struct *child,
> dabr |= DABR_DATA_READ;
> if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE)
> dabr |= DABR_DATA_WRITE;
> +#ifdef CONFIG_HAVE_HW_BREAKPOINT
> + if (ptrace_get_breakpoints(child) < 0)
> + return -ESRCH;
>
> - child->thread.dabr = dabr;
> + /*
> + * Check if the request is for 'range' breakpoints. We can
> + * support it if range < 8 bytes.
> + */
> + if (bp_info->addr_mode == PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE) {
> + len = bp_info->addr2 - bp_info->addr;
> + } else if (bp_info->addr_mode != PPC_BREAKPOINT_MODE_EXACT) {
> + ptrace_put_breakpoints(child);
> + return -EINVAL;
You are overindented here.
> + }
> + bp = thread->ptrace_bps[0];
> + if (bp) {
> + attr = bp->attr;
> + attr.bp_addr = (unsigned long)bp_info->addr & ~HW_BREAKPOINT_ALIGN;
> + arch_bp_generic_fields(dabr &
> + (DABR_DATA_WRITE | DABR_DATA_READ),
> + &attr.bp_type);
You still have this code which has no business in the generic
interface path.
> + attr.bp_len = len;
> + ret = modify_user_hw_breakpoint(bp, &attr);
> + if (ret) {
> + ptrace_put_breakpoints(child);
> + return ret;
> + }
If a bp already exists, you're modifying it. I thought the semantics
of the new interface meant that you shoul return ENOSPC in this case,
and a DEL would be necessary before adding another breakpoint.
> + thread->ptrace_bps[0] = bp;
> + ptrace_put_breakpoints(child);
> + thread->dabr = dabr;
> + return 0;
> + }
> +
> + /* Create a new breakpoint request if one doesn't exist already */
> + hw_breakpoint_init(&attr);
> + attr.bp_addr = (unsigned long)bp_info->addr & ~HW_BREAKPOINT_ALIGN;
> + attr.bp_len = len;
> + arch_bp_generic_fields(dabr & (DABR_DATA_WRITE | DABR_DATA_READ),
> + &attr.bp_type);
> +
> + thread->ptrace_bps[0] = bp = register_user_hw_breakpoint(&attr,
> + ptrace_triggered, NULL, child);
> + if (IS_ERR(bp)) {
> + thread->ptrace_bps[0] = NULL;
> + ptrace_put_breakpoints(child);
> + return PTR_ERR(bp);
> + }
>
> + ptrace_put_breakpoints(child);
> + return 1;
> +#endif /* CONFIG_HAVE_HW_BREAKPOINT */
> +
> + if (bp_info->addr_mode != PPC_BREAKPOINT_MODE_EXACT)
> + return -EINVAL;
> +
> + if (child->thread.dabr)
> + return -ENOSPC;
> +
> + child->thread.dabr = dabr;
> return 1;
> #endif /* !CONFIG_PPC_ADV_DEBUG_DVCS */
> }
>
> static long ppc_del_hwdebug(struct task_struct *child, long addr, long data)
> {
> +#ifdef CONFIG_HAVE_HW_BREAKPOINT
> + struct thread_struct *thread = &(child->thread);
> + struct perf_event *bp;
> +#endif /* CONFIG_HAVE_HW_BREAKPOINT */
> #ifdef CONFIG_PPC_ADV_DEBUG_REGS
> int rc;
>
> @@ -1426,10 +1488,24 @@ static long ppc_del_hwdebug(struct task_struct *child, long addr, long data)
> #else
> if (data != 1)
> return -EINVAL;
> +
> +#ifdef CONFIG_HAVE_HW_BREAKPOINT
> + if (ptrace_get_breakpoints(child) < 0)
> + return -ESRCH;
> +
> + bp = thread->ptrace_bps[0];
> + if (bp) {
> + unregister_hw_breakpoint(bp);
> + thread->ptrace_bps[0] = NULL;
> + }
> + ptrace_put_breakpoints(child);
> + return 0;
Shouldn't DEL return an error if there is no existing bp.
> +#else /* CONFIG_HAVE_HW_BREAKPOINT */
> if (child->thread.dabr == 0)
> return -ENOENT;
>
> child->thread.dabr = 0;
> +#endif /* CONFIG_HAVE_HW_BREAKPOINT */
>
> return 0;
> #endif
> @@ -1560,7 +1636,7 @@ long arch_ptrace(struct task_struct *child, long request,
> dbginfo.data_bp_alignment = 4;
> #endif
> dbginfo.sizeof_condition = 0;
> - dbginfo.features = 0;
> + dbginfo.features = PPC_DEBUG_FEATURE_DATA_BP_RANGE;
> #endif /* CONFIG_PPC_ADV_DEBUG_REGS */
>
> if (!access_ok(VERIFY_WRITE, datavp,
--
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 PATCH v5 2/9] fadump: Reserve the memory for firmware assisted dump.
From: Mahesh J. Salgaonkar @ 2011-11-28 6:21 UTC (permalink / raw)
To: Paul Mackerras
Cc: Anton Blanchard, Amerigo Wang, Kexec-ml, Linux Kernel,
Milton Miller, linuxppc-dev, Randy Dunlap, Eric W. Biederman,
Vivek Goyal
In-Reply-To: <20111124230213.GC19828@bloggs.ozlabs.ibm.com>
On 11/25/2011 04:32 AM, Paul Mackerras wrote:
> On Tue, Nov 15, 2011 at 08:43:43PM +0530, Mahesh J Salgaonkar wrote:
>> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>>
>> Reserve the memory during early boot to preserve CPU state data, HPTE region
>> and RMR region data in case of kernel crash. At the time of crash, powerpc
>> firmware will store CPU state data, HPTE region data and move RMR region
>> data to the reserved memory area.
>
> What is "RMR"? I don't see anywhere that you explain this acronym.
> Is it the same as the RMA (real mode area)?
>
Yes, it is the same as the RMA. I think I will replace all RMR/RMO with RMA.
>> +config FA_DUMP
>> + bool "Firmware-assisted dump"
>
> Is this new fadump infrastructure intended to supersede the existing
> phyp dump code? Does it use the same phyp interfaces as phyp dump?
> If so, you should probably remove the phyp dump code and config option
> as the final patch in your series.
>
>> +/*
>> + * The RMR region will be saved for later dumping when kernel crashes.
>> + * Set this to RMO size.
>> + */
>> +#define RMR_START 0x0
>> +#define RMR_END (ppc64_rma_size)
>
> An explanation of "RMR" here, and what the distinction (if any)
> between RMR and RMA/RMO is, would help future readers.
>
Will change this to RMA_START and RMA_END
>> + sections = of_get_flat_dt_prop(node, "ibm,configure-kernel-dump-sizes",
>> + &size);
>> +
>> + if (!sections)
>> + return 0;
>> +
>> + num_sections = size / sizeof(struct dump_section);
>> +
>> + for (i = 0; i < num_sections; i++) {
>> + switch (sections[i].dump_section) {
>> + case FADUMP_CPU_STATE_DATA:
>> + fw_dump.cpu_state_data_size = sections[i].section_size;
>> + break;
>> + case FADUMP_HPTE_REGION:
>> + fw_dump.hpte_region_size = sections[i].section_size;
>> + break;
>
> It's generally better to use of_read_number() or of_read_ulong() to
> parse OF properties, rather than using a structure like this.
>
>> + /* divide by 20 to get 5% of value */
>> + size = memblock_end_of_DRAM();
>> + do_div(size, 20);
>
> You could just say size = memblock_end_of_DRAM() / 20 here; no need to
> use do_div, since we won't be using this code on 32-bit platforms.
>
>> + if (!fw_dump.fadump_supported) {
>> + printk(KERN_ERR "Firmware-assisted dump is not supported on"
>> + " this hardware\n");
>
> This shouldn't be KERN_ERR; it's not an error to boot a kernel with
> fadump configured in on a machine that doesn't have firmware fadump
> support. I don't think we really need any message, but if we have one
> it should be KERN_INFO at most.
>
>> +/* Look for fadump= cmdline option. */
>> +static int __init early_fadump_param(char *p)
>> +{
>> + if (!p)
>> + return 1;
>> +
>> + if (p[0] == '1')
>> + fw_dump.fadump_enabled = 1;
>> + else if (p[0] == '0')
>> + fw_dump.fadump_enabled = 0;
>
> I think it's usual to allow "on" and "off" as values for this kind of
> option. There might be a handy little helper function to parse this
> sort of thing (but if there is I don't know what it is called).
Will rework on your suggestions. Thanks for the review.
Thanks,
-Mahesh.
^ permalink raw reply
* Re: [RFC PATCH v2 1/4] cpuidle: (powerpc) Add cpu_idle_wait() to allow switching of idle routines
From: Deepthi Dharwar @ 2011-11-28 11:02 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, linux-pm, linux-kernel, linux-pm
In-Reply-To: <1322434096.23348.6.camel@pasglop>
Hi Ben,
Thanks a lot for the review.
On 11/28/2011 04:18 AM, Benjamin Herrenschmidt wrote:
> On Thu, 2011-11-17 at 16:58 +0530, Deepthi Dharwar wrote:
>> This patch provides cpu_idle_wait() routine for the powerpc
>> platform which is required by the cpuidle subsystem. This
>> routine is requied to change the idle handler on SMP systems.
>> The equivalent routine for x86 is in arch/x86/kernel/process.c
>> but the powerpc implementation is different.
>>
>> Signed-off-by: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
>> Signed-off-by: Trinabh Gupta <g.trinabh@gmail.com>
>> Signed-off-by: Arun R Bharadwaj <arun.r.bharadwaj@gmail.com>
>> ---
>
> No, that patch also adds this idle boot override thing (can you pick a
> shorter name for boot_option_idle_override btw ?) which seems unrelated
> and without any explanation as to what it's supposed to be about.
Yes, we can pick a better and shorter name for this variable.
This variable is used to determine if cpuidle framework
needs to be enabled and pseries_driver to be loaded or not.
We disable cpuidle framework only when powersave_off option is set or
not enabled by the user.
>
> Additionally, I'm a bit worried (but maybe we already discussed that a
> while back, I don't know) but cpu_idle_wait() has "wait" in the name,
> which makes me think it might need to actually -wait- for all cpus to
> have come out of the function.
cpu_idle_wait is used to ensure that all the CPUs discard old idle
handler and update to new one. Required while changing idle
handler on SMP systems.
> Now your implementation doesn't provide that guarantee. It might be
> fine, I don't know, but if it is, you'd better document it well in the
> comments surrounding the code, because as it is, all you do is shoot an
> interrupt which will cause the target CPU to eventually come out of idle
> some time in the future.
I was hoping that sending an explicit reschedule to the cpus would
do the trick but sure we can add some documentation around the code.
>
> Cheers,
> Ben.
>
>> arch/powerpc/Kconfig | 4 ++++
>> arch/powerpc/include/asm/processor.h | 2 ++
>> arch/powerpc/include/asm/system.h | 1 +
>> arch/powerpc/kernel/idle.c | 26 ++++++++++++++++++++++++++
>> 4 files changed, 33 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index b177caa..87f8604 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -87,6 +87,10 @@ config ARCH_HAS_ILOG2_U64
>> bool
>> default y if 64BIT
>>
>> +config ARCH_HAS_CPU_IDLE_WAIT
>> + bool
>> + default y
>> +
>> config GENERIC_HWEIGHT
>> bool
>> default y
>> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
>> index eb11a44..811b7e7 100644
>> --- a/arch/powerpc/include/asm/processor.h
>> +++ b/arch/powerpc/include/asm/processor.h
>> @@ -382,6 +382,8 @@ static inline unsigned long get_clean_sp(struct pt_regs *regs, int is_32)
>> }
>> #endif
>>
>> +enum idle_boot_override {IDLE_NO_OVERRIDE = 0, IDLE_POWERSAVE_OFF};
>> +
>> #endif /* __KERNEL__ */
>> #endif /* __ASSEMBLY__ */
>> #endif /* _ASM_POWERPC_PROCESSOR_H */
>> diff --git a/arch/powerpc/include/asm/system.h b/arch/powerpc/include/asm/system.h
>> index e30a13d..ff66680 100644
>> --- a/arch/powerpc/include/asm/system.h
>> +++ b/arch/powerpc/include/asm/system.h
>> @@ -221,6 +221,7 @@ extern unsigned long klimit;
>> extern void *zalloc_maybe_bootmem(size_t size, gfp_t mask);
>>
>> extern int powersave_nap; /* set if nap mode can be used in idle loop */
>> +void cpu_idle_wait(void);
>>
>> /*
>> * Atomic exchange
>> diff --git a/arch/powerpc/kernel/idle.c b/arch/powerpc/kernel/idle.c
>> index 39a2baa..b478c72 100644
>> --- a/arch/powerpc/kernel/idle.c
>> +++ b/arch/powerpc/kernel/idle.c
>> @@ -39,9 +39,13 @@
>> #define cpu_should_die() 0
>> #endif
>>
>> +unsigned long boot_option_idle_override = IDLE_NO_OVERRIDE;
>> +EXPORT_SYMBOL(boot_option_idle_override);
>> +
>> static int __init powersave_off(char *arg)
>> {
>> ppc_md.power_save = NULL;
>> + boot_option_idle_override = IDLE_POWERSAVE_OFF;
>> return 0;
>> }
>> __setup("powersave=off", powersave_off);
>> @@ -102,6 +106,28 @@ void cpu_idle(void)
>> }
>> }
>>
>> +
>> +/*
>> + * cpu_idle_wait - Used to ensure that all the CPUs come out of the old
>> + * idle loop and start using the new idle loop.
>> + * Required while changing idle handler on SMP systems.
>> + * Caller must have changed idle handler to the new value before the call.
>> + */
>> +void cpu_idle_wait(void)
>> +{
>> + int cpu;
>> + smp_mb();
>> +
>> + /* kick all the CPUs so that they exit out of old idle routine */
>> + get_online_cpus();
>> + for_each_online_cpu(cpu) {
>> + if (cpu != smp_processor_id())
>> + smp_send_reschedule(cpu);
>> + }
>> + put_online_cpus();
>> +}
>> +EXPORT_SYMBOL_GPL(cpu_idle_wait);
>> +
>> int powersave_nap;
>>
>> #ifdef CONFIG_SYSCTL
>>
>> _______________________________________________
>> Linuxppc-dev mailing list
>> Linuxppc-dev@lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
>
Regards,
Deepthi
^ permalink raw reply
* Re: [RFC PATCH v2 2/4] cpuidle: (POWER) cpuidle driver for pSeries
From: Deepthi Dharwar @ 2011-11-28 11:02 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, linux-pm, linux-kernel, linux-pm
In-Reply-To: <1322435008.23348.15.camel@pasglop>
On 11/28/2011 04:33 AM, Benjamin Herrenschmidt wrote:
> On Thu, 2011-11-17 at 16:58 +0530, Deepthi Dharwar wrote:
>> This patch implements a backhand cpuidle driver for pSeries
>> based on pseries_dedicated_idle_loop and pseries_shared_idle_loop
>> routines. The driver is built only if CONFIG_CPU_IDLE is set. This
>> cpuidle driver uses global registration of idle states and
>> not per-cpu.
>
> .../...
>
>> +#define MAX_IDLE_STATE_COUNT 2
>> +
>> +static int max_cstate = MAX_IDLE_STATE_COUNT - 1;
>
> Why "cstate" ? This isn't an x86 :-)
Sure, I shall change it right away :)
>
>> +static struct cpuidle_device __percpu *pseries_idle_cpuidle_devices;
>
> Shorter name please. pseries_cpuidle_devs is fine.
I ll do so.
>
>> +static struct cpuidle_state *cpuidle_state_table;
>> +
>> +void update_smt_snooze_delay(int snooze)
>> +{
>> + struct cpuidle_driver *drv = cpuidle_get_driver();
>> + if (drv)
>> + drv->states[0].target_residency = snooze;
>> +}
>> +
>> +static inline void idle_loop_prolog(unsigned long *in_purr, ktime_t *kt_before)
>> +{
>> +
>> + *kt_before = ktime_get_real();
>> + *in_purr = mfspr(SPRN_PURR);
>> + /*
>> + * Indicate to the HV that we are idle. Now would be
>> + * a good time to find other work to dispatch.
>> + */
>> + get_lppaca()->idle = 1;
>> + get_lppaca()->donate_dedicated_cpu = 1;
>> +}
>
> I notice that you call this on shared processors as well. The old ocde
> used to not set donate_dedicated_cpu in that case. I assume that's not a
> big deal and that the HV will just ignore it in the shared processor
> case but please add a comment after you've verified it.
>
Yes, the old code does not set donate_dedicated_cpu. But yes I will
try testing it in a shared proc config but also remove this
initialization for shared idle loop.
>> +static inline s64 idle_loop_epilog(unsigned long in_purr, ktime_t kt_before)
>> +{
>> + get_lppaca()->wait_state_cycles += mfspr(SPRN_PURR) - in_purr;
>> + get_lppaca()->donate_dedicated_cpu = 0;
>> + get_lppaca()->idle = 0;
>> +
>> + return ktime_to_us(ktime_sub(ktime_get_real(), kt_before));
>> +}
>> +
>> +
>> +static int snooze_loop(struct cpuidle_device *dev,
>> + struct cpuidle_driver *drv,
>> + int index)
>> +{
>> + unsigned long in_purr;
>> + ktime_t kt_before;
>> +
>> + idle_loop_prolog(&in_purr, &kt_before);
>> +
>> + local_irq_enable();
>> + set_thread_flag(TIF_POLLING_NRFLAG);
>> + while (!need_resched()) {
>> + ppc64_runlatch_off();
>> + HMT_low();
>> + HMT_very_low();
>> + }
>> + HMT_medium();
>> + clear_thread_flag(TIF_POLLING_NRFLAG);
>> + smp_mb();
>> + local_irq_disable();
>> +
>> + dev->last_residency =
>> + (int)idle_loop_epilog(in_purr, kt_before);
>> + return index;
>> +}
>
> So your snooze loop has no timeout, is that handled by the cpuidle
> driver using some kind of timer ? That sounds a lot less efficient than
> passing a max delay to the snooze loop to handle getting into a deeper
> state after a bit of snoozing rather than interrupting etc...
My bad, snooze_loop is essential for a time out. Nope cpuidle
driver doesn't have any timer mechanism. I ll fix it.
Need to add loop for snooze time.
>> +static int dedicated_cede_loop(struct cpuidle_device *dev,
>> + struct cpuidle_driver *drv,
>> + int index)
>> +{
>> + unsigned long in_purr;
>> + ktime_t kt_before;
>> +
>> + idle_loop_prolog(&in_purr, &kt_before);
>> +
>> + ppc64_runlatch_off();
>> + HMT_medium();
>> + cede_processor();
>> +
>> + dev->last_residency =
>> + (int)idle_loop_epilog(in_purr, kt_before);
>> + return index;
>> +}
>> +
>> +static int shared_cede_loop(struct cpuidle_device *dev,
>> + struct cpuidle_driver *drv,
>> + int index)
>> +{
>> + unsigned long in_purr;
>> + ktime_t kt_before;
>> +
>> + idle_loop_prolog(&in_purr, &kt_before);
>> +
>> + /*
>> + * Yield the processor to the hypervisor. We return if
>> + * an external interrupt occurs (which are driven prior
>> + * to returning here) or if a prod occurs from another
>> + * processor. When returning here, external interrupts
>> + * are enabled.
>> + */
>> + cede_processor();
>> +
>> + dev->last_residency =
>> + (int)idle_loop_epilog(in_purr, kt_before);
>> + return index;
>> +}
>> +
>> +/*
>> + * States for dedicated partition case.
>> + */
>> +static struct cpuidle_state dedicated_states[MAX_IDLE_STATE_COUNT] = {
>> + { /* Snooze */
>> + .name = "snooze",
>> + .desc = "snooze",
>> + .flags = CPUIDLE_FLAG_TIME_VALID,
>> + .exit_latency = 0,
>> + .target_residency = 0,
>> + .enter = &snooze_loop },
>> + { /* CEDE */
>> + .name = "CEDE",
>> + .desc = "CEDE",
>> + .flags = CPUIDLE_FLAG_TIME_VALID,
>> + .exit_latency = 1,
>> + .target_residency = 10,
>> + .enter = &dedicated_cede_loop },
>> +};
>> +
>> +/*
>> + * States for shared partition case.
>> + */
>> +static struct cpuidle_state shared_states[MAX_IDLE_STATE_COUNT] = {
>> + { /* Shared Cede */
>> + .name = "Shared Cede",
>> + .desc = "Shared Cede",
>> + .flags = CPUIDLE_FLAG_TIME_VALID,
>> + .exit_latency = 0,
>> + .target_residency = 0,
>> + .enter = &shared_cede_loop },
>> +};
>> +
>> +int pseries_notify_cpuidle_add_cpu(int cpu)
>> +{
>> + struct cpuidle_device *dev =
>> + per_cpu_ptr(pseries_idle_cpuidle_devices, cpu);
>> + if (dev && cpuidle_get_driver()) {
>> + cpuidle_disable_device(dev);
>> + cpuidle_enable_device(dev);
>> + }
>> + return 0;
>> +}
>> +
>> +/*
>> + * pseries_idle_cpuidle_driver_init()
>> + */
>> +static int pseries_idle_cpuidle_driver_init(void)
>> +{
>
> Drop the "idle", those names are too long.
Sure.
>
>> + int cstate;
>
> And use something else than 'cstate', we are among civilized people
> here ;-)
Yes :)
>
>> + struct cpuidle_driver *drv = &pseries_idle_driver;
>> +
>> + drv->state_count = 0;
>> +
>> + for (cstate = 0; cstate < MAX_IDLE_STATE_COUNT; ++cstate) {
>> +
>> + if (cstate > max_cstate)
>> + break;
>> +
>> + /* is the state not enabled? */
>> + if (cpuidle_state_table[cstate].enter == NULL)
>> + continue;
>> +
>> + drv->states[drv->state_count] = /* structure copy */
>> + cpuidle_state_table[cstate];
>> +
>> + if (cpuidle_state_table == dedicated_states)
>> + drv->states[drv->state_count].target_residency =
>> + __get_cpu_var(smt_snooze_delay);
>> +
>> + drv->state_count += 1;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +/* pseries_idle_devices_uninit(void)
>> + * unregister cpuidle devices and de-allocate memory
>> + */
>> +static void pseries_idle_devices_uninit(void)
>> +{
>> + int i;
>> + struct cpuidle_device *dev;
>> +
>> + for_each_possible_cpu(i) {
>> + dev = per_cpu_ptr(pseries_idle_cpuidle_devices, i);
>> + cpuidle_unregister_device(dev);
>> + }
>> +
>> + free_percpu(pseries_idle_cpuidle_devices);
>> + return;
>> +}
>> +
>> +/* pseries_idle_devices_init()
>> + * allocate, initialize and register cpuidle device
>> + */
>> +static int pseries_idle_devices_init(void)
>> +{
>> + int i;
>> + struct cpuidle_driver *drv = &pseries_idle_driver;
>> + struct cpuidle_device *dev;
>> +
>> + pseries_idle_cpuidle_devices = alloc_percpu(struct cpuidle_device);
>> + if (pseries_idle_cpuidle_devices == NULL)
>> + return -ENOMEM;
>> +
>> + for_each_possible_cpu(i) {
>> + dev = per_cpu_ptr(pseries_idle_cpuidle_devices, i);
>> + dev->state_count = drv->state_count;
>> + dev->cpu = i;
>> + if (cpuidle_register_device(dev)) {
>> + printk(KERN_DEBUG \
>> + "cpuidle_register_device %d failed!\n", i);
>> + return -EIO;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * pseries_idle_probe()
>> + * Choose state table for shared versus dedicated partition
>> + */
>> +static int pseries_idle_probe(void)
>> +{
>> + if (max_cstate == 0) {
>> + printk(KERN_DEBUG "pseries processor idle disabled.\n");
>> + return -EPERM;
>> + }
>> +
>> + if (!firmware_has_feature(FW_FEATURE_SPLPAR)) {
>> + printk(KERN_DEBUG "Using default idle\n");
>> + return -ENODEV;
>> + }
>
> Don't even printk here, this will happen on all !pseries machines and
> the debug output isn't useful. Also move the check of max-cstate to
> after the splpar test.
I ll move the check above.
> Overall, I quite like these patches, my comments are all pretty minor,
> hopefully the next round should be the one.
Thank you.
>
> Cheers,
> Ben.
>
>
Regards,
Deepthi
^ permalink raw reply
* Re: [RFC PATCH v2 3/4] cpuidle: (POWER) Enable cpuidle and directly call cpuidle_idle_call() for pSeries
From: Deepthi Dharwar @ 2011-11-28 11:03 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, linux-pm, linux-kernel, linux-pm
In-Reply-To: <1322435131.23348.17.camel@pasglop>
On 11/28/2011 04:35 AM, Benjamin Herrenschmidt wrote:
> On Thu, 2011-11-17 at 16:58 +0530, Deepthi Dharwar wrote:
>> This patch enables cpuidle for pSeries and cpuidle_idle_call() is
>> directly called from the idle loop. As a result pseries_idle cpuidle
>> driver registered with cpuidle subsystem comes into action. This patch
>> also removes the routines pseries_shared_idle_sleep and
>> pseries_dedicated_idle_sleep as they are now implemented as part of
>> pseries_idle cpuidle driver.
>>
>> Signed-off-by: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
>> Signed-off-by: Trinabh Gupta <g.trinabh@gmail.com>
>> Signed-off-by: Arun R Bharadwaj <arun.r.bharadwaj@gmail.com>
>> ---
>> arch/powerpc/platforms/Kconfig | 6 ++
>> arch/powerpc/platforms/pseries/setup.c | 86 +-------------------------------
>> include/linux/cpuidle.h | 2 -
>> 3 files changed, 8 insertions(+), 86 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
>> index e458872..0d2a028 100644
>> --- a/arch/powerpc/platforms/Kconfig
>> +++ b/arch/powerpc/platforms/Kconfig
>> @@ -211,6 +211,12 @@ config PPC_PASEMI_CPUFREQ
>>
>> endmenu
>>
>> +menu "CPUIdle driver"
>> +
>> +source "drivers/cpuidle/Kconfig"
>> +
>> +endmenu
>> +
>> config PPC601_SYNC_FIX
>> bool "Workarounds for PPC601 bugs"
>> depends on 6xx && (PPC_PREP || PPC_PMAC)
>> diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
>> index 9c6716a..f624e74 100644
>> --- a/arch/powerpc/platforms/pseries/setup.c
>> +++ b/arch/powerpc/platforms/pseries/setup.c
>> @@ -39,6 +39,7 @@
>> #include <linux/irq.h>
>> #include <linux/seq_file.h>
>> #include <linux/root_dev.h>
>> +#include <linux/cpuidle.h>
>>
>> #include <asm/mmu.h>
>> #include <asm/processor.h>
>> @@ -74,9 +75,6 @@ EXPORT_SYMBOL(CMO_PageSize);
>>
>> int fwnmi_active; /* TRUE if an FWNMI handler is present */
>>
>> -static void pseries_shared_idle_sleep(void);
>> -static void pseries_dedicated_idle_sleep(void);
>> -
>> static struct device_node *pSeries_mpic_node;
>>
>> static void pSeries_show_cpuinfo(struct seq_file *m)
>> @@ -374,18 +372,9 @@ static void __init pSeries_setup_arch(void)
>>
>> pSeries_nvram_init();
>>
>> - /* Choose an idle loop */
>> if (firmware_has_feature(FW_FEATURE_SPLPAR)) {
>> vpa_init(boot_cpuid);
>> - if (get_lppaca()->shared_proc) {
>> - printk(KERN_DEBUG "Using shared processor idle loop\n");
>> - ppc_md.power_save = pseries_shared_idle_sleep;
>> - } else {
>> - printk(KERN_DEBUG "Using dedicated idle loop\n");
>> - ppc_md.power_save = pseries_dedicated_idle_sleep;
>> - }
>> - } else {
>> - printk(KERN_DEBUG "Using default idle loop\n");
>> + ppc_md.power_save = (void *)cpuidle_idle_call;
>> }
>
> I very very much dislike that cast. You should not have to cast a
> function pointer ... EVER.
Yes, I ll fix this.
This actually bought out a design flaw with the current pseries idle as
mentioned by you in the next patch of the series.
>
>> if (firmware_has_feature(FW_FEATURE_LPAR))
>> @@ -586,77 +575,6 @@ static int __init pSeries_probe(void)
>> return 1;
>> }
>>
>
> Cheers,
> Ben.
>
>
Regards,
Deepthi
^ permalink raw reply
* Re: [RFC PATCH v2 4/4] cpuidle: (POWER) Handle power_save=off
From: Deepthi Dharwar @ 2011-11-28 11:03 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, linux-pm, linux-kernel, linux-pm
In-Reply-To: <1322435233.23348.19.camel@pasglop>
On 11/28/2011 04:37 AM, Benjamin Herrenschmidt wrote:
> On Thu, 2011-11-17 at 16:59 +0530, Deepthi Dharwar wrote:
>> This patch makes pseries_idle_driver not to be registered when
>> power_save=off kernel boot option is specified. The
>> boot_option_idle_override variable used here is similar to
>> its usage on x86.
>
> Quick Q. With your changes, the CPU will never get into idle at all
> until cpuidle initializes and the driver loads.
>
> That means not only much later in the boot process, but potentially
> never if the distro has the driver as a module and fails to load it, or
> similar.
>
> Can't that be an issue ? Shouldn't we keep at least one of the basic
> idle functions as a fallback ?
>
On an LPAR if cpuidle is disabled, ppc_md.power_save is still set to
cpuidle_idle_call by default here. This would result in calling of
cpuidle_idle_call repeatedly, only for the call to return -ENODEV. The
default idle is never executed.
This would be a major design flaw. No fallback idle routine.
We propose to fix this by checking the return value of
ppc_md.power_save() call from void to int.
Right now return value is void, but if we change this to int, this
would solve two problems. One being removing the cast to a function
pointer in the prev patch and this design flaw stated above.
So by checking the return value of ppc_md.power_save(), we can invoke
the default idle on failure. But my only concern is about the effects of
changing the ppc_md.power_save() to return int on other powerpc
architectures. Would it be a good idea to change the return type to int
which would help us flag an error and fallback to default idle?
> Cheers,
> Ben.
>
>
Regards,
Deepthi
^ permalink raw reply
* [PATCH net-next v3 4/4] powerpc: tqm8548/tqm8xx: add and update CAN device nodes
From: Wolfgang Grandegger @ 2011-11-28 11:30 UTC (permalink / raw)
To: netdev
Cc: Stanislav Yelenskiy, devicetree-discuss, linux-can, linuxppc-dev,
IreneV, socketcan-users, Wolfgang Zarre
In-Reply-To: <1322479858-4874-1-git-send-email-wg@grandegger.com>
This patch enables or updates support for the CC770 and AN82527
CAN controller on the TQM8548 and TQM8xx boards.
CC: devicetree-discuss@lists.ozlabs.org
CC: linuxppc-dev@ozlabs.org
CC: Kumar Gala <galak@kernel.crashing.org>
Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
arch/powerpc/boot/dts/tqm8548-bigflash.dts | 19 ++++++++++++++-----
arch/powerpc/boot/dts/tqm8548.dts | 19 ++++++++++++++-----
arch/powerpc/boot/dts/tqm8xx.dts | 25 +++++++++++++++++++++++++
3 files changed, 53 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/boot/dts/tqm8548-bigflash.dts b/arch/powerpc/boot/dts/tqm8548-bigflash.dts
index 9452c3c..d918752 100644
--- a/arch/powerpc/boot/dts/tqm8548-bigflash.dts
+++ b/arch/powerpc/boot/dts/tqm8548-bigflash.dts
@@ -352,7 +352,7 @@
ranges = <
0 0x0 0xfc000000 0x04000000 // NOR FLASH bank 1
1 0x0 0xf8000000 0x08000000 // NOR FLASH bank 0
- 2 0x0 0xa3000000 0x00008000 // CAN (2 x i82527)
+ 2 0x0 0xa3000000 0x00008000 // CAN (2 x CC770)
3 0x0 0xa3010000 0x00008000 // NAND FLASH
>;
@@ -393,18 +393,27 @@
};
/* Note: CAN support needs be enabled in U-Boot */
- can0@2,0 {
- compatible = "intel,82527"; // Bosch CC770
+ can@2,0 {
+ compatible = "bosch,cc770"; // Bosch CC770
reg = <2 0x0 0x100>;
interrupts = <4 1>;
interrupt-parent = <&mpic>;
+ bosch,external-clock-frequency = <16000000>;
+ bosch,disconnect-rx1-input;
+ bosch,disconnect-tx1-output;
+ bosch,iso-low-speed-mux;
+ bosch,clock-out-frequency = <16000000>;
};
- can1@2,100 {
- compatible = "intel,82527"; // Bosch CC770
+ can@2,100 {
+ compatible = "bosch,cc770"; // Bosch CC770
reg = <2 0x100 0x100>;
interrupts = <4 1>;
interrupt-parent = <&mpic>;
+ bosch,external-clock-frequency = <16000000>;
+ bosch,disconnect-rx1-input;
+ bosch,disconnect-tx1-output;
+ bosch,iso-low-speed-mux;
};
/* Note: NAND support needs to be enabled in U-Boot */
diff --git a/arch/powerpc/boot/dts/tqm8548.dts b/arch/powerpc/boot/dts/tqm8548.dts
index 619776f..988d887 100644
--- a/arch/powerpc/boot/dts/tqm8548.dts
+++ b/arch/powerpc/boot/dts/tqm8548.dts
@@ -352,7 +352,7 @@
ranges = <
0 0x0 0xfc000000 0x04000000 // NOR FLASH bank 1
1 0x0 0xf8000000 0x08000000 // NOR FLASH bank 0
- 2 0x0 0xe3000000 0x00008000 // CAN (2 x i82527)
+ 2 0x0 0xe3000000 0x00008000 // CAN (2 x CC770)
3 0x0 0xe3010000 0x00008000 // NAND FLASH
>;
@@ -393,18 +393,27 @@
};
/* Note: CAN support needs be enabled in U-Boot */
- can0@2,0 {
- compatible = "intel,82527"; // Bosch CC770
+ can@2,0 {
+ compatible = "bosch,cc770"; // Bosch CC770
reg = <2 0x0 0x100>;
interrupts = <4 1>;
interrupt-parent = <&mpic>;
+ bosch,external-clock-frequency = <16000000>;
+ bosch,disconnect-rx1-input;
+ bosch,disconnect-tx1-output;
+ bosch,iso-low-speed-mux;
+ bosch,clock-out-frequency = <16000000>;
};
- can1@2,100 {
- compatible = "intel,82527"; // Bosch CC770
+ can@2,100 {
+ compatible = "bosch,cc770"; // Bosch CC770
reg = <2 0x100 0x100>;
interrupts = <4 1>;
interrupt-parent = <&mpic>;
+ bosch,external-clock-frequency = <16000000>;
+ bosch,disconnect-rx1-input;
+ bosch,disconnect-tx1-output;
+ bosch,iso-low-speed-mux;
};
/* Note: NAND support needs to be enabled in U-Boot */
diff --git a/arch/powerpc/boot/dts/tqm8xx.dts b/arch/powerpc/boot/dts/tqm8xx.dts
index f6da7ec..c3dba25 100644
--- a/arch/powerpc/boot/dts/tqm8xx.dts
+++ b/arch/powerpc/boot/dts/tqm8xx.dts
@@ -57,6 +57,7 @@
ranges = <
0x0 0x0 0x40000000 0x800000
+ 0x3 0x0 0xc0000000 0x200
>;
flash@0,0 {
@@ -67,6 +68,30 @@
bank-width = <4>;
device-width = <2>;
};
+
+ /* Note: CAN support needs be enabled in U-Boot */
+ can@3,0 {
+ compatible = "intc,82527";
+ reg = <3 0x0 0x80>;
+ interrupts = <8 1>;
+ interrupt-parent = <&PIC>;
+ bosch,external-clock-frequency = <16000000>;
+ bosch,disconnect-rx1-input;
+ bosch,disconnect-tx1-output;
+ bosch,iso-low-speed-mux;
+ bosch,clock-out-frequency = <16000000>;
+ };
+
+ can@3,100 {
+ compatible = "intc,82527";
+ reg = <3 0x100 0x80>;
+ interrupts = <8 1>;
+ interrupt-parent = <&PIC>;
+ bosch,external-clock-frequency = <16000000>;
+ bosch,disconnect-rx1-input;
+ bosch,disconnect-tx1-output;
+ bosch,iso-low-speed-mux;
+ };
};
soc@fff00000 {
--
1.7.4.1
^ permalink raw reply related
* [PATCH net-next v3 3/4] can: cc770: add platform bus driver for the CC770 and AN82527
From: Wolfgang Grandegger @ 2011-11-28 11:30 UTC (permalink / raw)
To: netdev
Cc: Stanislav Yelenskiy, Devicetree-discuss, linux-can, linuxppc-dev,
IreneV, socketcan-users, Wolfgang Zarre
In-Reply-To: <1322479858-4874-1-git-send-email-wg@grandegger.com>
This driver works with both, static platform data and device tree
bindings. It has been tested on a TQM855L board with two AN82527
CAN controllers on the local bus.
CC: Devicetree-discuss@lists.ozlabs.org
CC: linuxppc-dev@ozlabs.org
CC: Kumar Gala <galak@kernel.crashing.org>
Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
.../devicetree/bindings/net/can/cc770.txt | 56 ++++
drivers/net/can/cc770/Kconfig | 7 +
drivers/net/can/cc770/Makefile | 1 +
drivers/net/can/cc770/cc770_platform.c | 280 ++++++++++++++++++++
4 files changed, 344 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/can/cc770.txt
create mode 100644 drivers/net/can/cc770/cc770_platform.c
diff --git a/Documentation/devicetree/bindings/net/can/cc770.txt b/Documentation/devicetree/bindings/net/can/cc770.txt
new file mode 100644
index 0000000..01e282d
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/cc770.txt
@@ -0,0 +1,56 @@
+Memory mapped Bosch CC770 and Intel AN82527 CAN controller
+
+Note: The CC770 is a CAN controller from Bosch, which is 100%
+compatible with the old AN82527 from Intel, but with "bugs" being fixed.
+
+Required properties:
+
+- compatible : should be "bosch,cc770" for the CC770 and "intc,82527"
+ for the AN82527.
+
+- reg : should specify the chip select, address offset and size required
+ to map the registers of the controller. The size is usually 0x80.
+
+- interrupts : property with a value describing the interrupt source
+ (number and sensitivity) required for the controller.
+
+Optional properties:
+
+- bosch,external-clock-frequency : frequency of the external oscillator
+ clock in Hz. Note that the internal clock frequency used by the
+ controller is half of that value. If not specified, a default
+ value of 16000000 (16 MHz) is used.
+
+- bosch,clock-out-frequency : slock frequency in Hz on the CLKOUT pin.
+ If not specified or if the specified value is 0, the CLKOUT pin
+ will be disabled.
+
+- bosch,slew-rate : slew rate of the CLKOUT signal. If not specified,
+ a resonable value will be calculated.
+
+- bosch,disconnect-rx0-input : see data sheet.
+
+- bosch,disconnect-rx1-input : see data sheet.
+
+- bosch,disconnect-tx1-output : see data sheet.
+
+- bosch,polarity-dominant : see data sheet.
+
+- bosch,divide-memory-clock : see data sheet.
+
+- bosch,iso-low-speed-mux : see data sheet.
+
+For further information, please have a look to the CC770 or AN82527.
+
+Examples:
+
+can@3,100 {
+ compatible = "bosch,cc770";
+ reg = <3 0x100 0x80>;
+ interrupts = <2 0>;
+ interrupt-parent = <&mpic>;
+ bosch,external-clock-frequency = <16000000>;
+};
+
+
+
diff --git a/drivers/net/can/cc770/Kconfig b/drivers/net/can/cc770/Kconfig
index 28e4d48..22c07a8 100644
--- a/drivers/net/can/cc770/Kconfig
+++ b/drivers/net/can/cc770/Kconfig
@@ -11,4 +11,11 @@ config CAN_CC770_ISA
connected to the ISA bus using I/O port, memory mapped or
indirect access.
+config CAN_CC770_PLATFORM
+ tristate "Generic Platform Bus based CC770 driver"
+ ---help---
+ This driver adds support for the CC770 and AN82527 chips
+ connected to the "platform bus" (Linux abstraction for directly
+ to the processor attached devices).
+
endif
diff --git a/drivers/net/can/cc770/Makefile b/drivers/net/can/cc770/Makefile
index 872ecff..9fb8321 100644
--- a/drivers/net/can/cc770/Makefile
+++ b/drivers/net/can/cc770/Makefile
@@ -4,5 +4,6 @@
obj-$(CONFIG_CAN_CC770) += cc770.o
obj-$(CONFIG_CAN_CC770_ISA) += cc770_isa.o
+obj-$(CONFIG_CAN_CC770_PLATFORM) += cc770_platform.o
ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
diff --git a/drivers/net/can/cc770/cc770_platform.c b/drivers/net/can/cc770/cc770_platform.c
new file mode 100644
index 0000000..65e177e
--- /dev/null
+++ b/drivers/net/can/cc770/cc770_platform.c
@@ -0,0 +1,280 @@
+/*
+ * Driver for CC770 and AN82527 CAN controllers on the platform bus
+ *
+ * Copyright (C) 2009, 2011 Wolfgang Grandegger <wg@grandegger.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the version 2 of the GNU General Public License
+ * as published by the Free Software Foundation
+ *
+ * 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.
+ */
+
+/*
+ * If platform data are used you should have similar definitions
+ * in your board-specific code:
+ *
+ * static struct cc770_platform_data myboard_cc770_pdata = {
+ * .osc_freq = 16000000,
+ * .cir = 0x41,
+ * .cor = 0x20,
+ * .bcr = 0x40,
+ * };
+ *
+ * Please see include/linux/can/platform/cc770.h for description of
+ * above fields.
+ *
+ * If the device tree is used, you need a CAN node definition in your
+ * DTS file similar to:
+ *
+ * can@3,100 {
+ * compatible = "bosch,cc770";
+ * reg = <3 0x100 0x80>;
+ * interrupts = <2 0>;
+ * interrupt-parent = <&mpic>;
+ * bosch,external-clock-frequency = <16000000>;
+ * };
+ *
+ * See "Documentation/devicetree/bindings/net/can/cc770.txt" for further
+ * information.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/netdevice.h>
+#include <linux/delay.h>
+#include <linux/can.h>
+#include <linux/can/dev.h>
+#include <linux/can/platform/cc770.h>
+
+#include <linux/of_platform.h>
+
+#include "cc770.h"
+
+#define DRV_NAME "cc770_platform"
+
+MODULE_AUTHOR("Wolfgang Grandegger <wg@grandegger.com>");
+MODULE_DESCRIPTION("Socket-CAN driver for CC770 on the platform bus");
+MODULE_LICENSE("GPL v2");
+
+#define CC770_PLATFORM_CAN_CLOCK 16000000
+
+static u8 cc770_platform_read_reg(const struct cc770_priv *priv, int reg)
+{
+ return in_8(priv->reg_base + reg);
+}
+
+static void cc770_platform_write_reg(const struct cc770_priv *priv, int reg,
+ u8 val)
+{
+ out_8(priv->reg_base + reg, val);
+}
+
+static int __devinit cc770_get_of_node_data(struct platform_device *pdev,
+ struct cc770_priv *priv)
+{
+ struct device_node *np = pdev->dev.of_node;
+ const u32 *prop;
+ int prop_size;
+ u32 clkext;
+
+ prop = of_get_property(np, "bosch,external-clock-frequency",
+ &prop_size);
+ if (prop && (prop_size == sizeof(u32)))
+ clkext = *prop;
+ else
+ clkext = CC770_PLATFORM_CAN_CLOCK; /* default */
+ priv->can.clock.freq = clkext;
+
+ /* The system clock may not exceed 10 MHz */
+ if (priv->can.clock.freq > 10000000) {
+ priv->cpu_interface |= CPUIF_DSC;
+ priv->can.clock.freq /= 2;
+ }
+
+ /* The memory clock may not exceed 8 MHz */
+ if (priv->can.clock.freq > 8000000)
+ priv->cpu_interface |= CPUIF_DMC;
+
+ if (of_get_property(np, "bosch,divide-memory-clock", NULL))
+ priv->cpu_interface |= CPUIF_DMC;
+ if (of_get_property(np, "bosch,iso-low-speed-mux", NULL))
+ priv->cpu_interface |= CPUIF_MUX;
+
+ if (!of_get_property(np, "bosch,no-comperator-bypass", NULL))
+ priv->bus_config |= BUSCFG_CBY;
+ if (of_get_property(np, "bosch,disconnect-rx0-input", NULL))
+ priv->bus_config |= BUSCFG_DR0;
+ if (of_get_property(np, "bosch,disconnect-rx1-input", NULL))
+ priv->bus_config |= BUSCFG_DR1;
+ if (of_get_property(np, "bosch,disconnect-tx1-output", NULL))
+ priv->bus_config |= BUSCFG_DT1;
+ if (of_get_property(np, "bosch,polarity-dominant", NULL))
+ priv->bus_config |= BUSCFG_POL;
+
+ prop = of_get_property(np, "bosch,clock-out-frequency", &prop_size);
+ if (prop && (prop_size == sizeof(u32)) && *prop > 0) {
+ u32 cdv = clkext / *prop;
+ int slew;
+
+ if (cdv > 0 && cdv < 16) {
+ priv->cpu_interface |= CPUIF_CEN;
+ priv->clkout |= (cdv - 1) & CLKOUT_CD_MASK;
+
+ prop = of_get_property(np, "bosch,slew-rate",
+ &prop_size);
+ if (prop && (prop_size == sizeof(u32))) {
+ slew = *prop;
+ } else {
+ /* Determine default slew rate */
+ slew = (CLKOUT_SL_MASK >>
+ CLKOUT_SL_SHIFT) -
+ ((cdv * clkext - 1) / 8000000);
+ if (slew < 0)
+ slew = 0;
+ }
+ priv->clkout |= (slew << CLKOUT_SL_SHIFT) &
+ CLKOUT_SL_MASK;
+ } else {
+ dev_dbg(&pdev->dev, "invalid clock-out-frequency\n");
+ }
+ }
+
+ return 0;
+}
+
+static int __devinit cc770_get_platform_data(struct platform_device *pdev,
+ struct cc770_priv *priv)
+{
+
+ struct cc770_platform_data *pdata = pdev->dev.platform_data;
+
+ priv->can.clock.freq = pdata->osc_freq;
+ if (priv->cpu_interface | CPUIF_DSC)
+ priv->can.clock.freq /= 2;
+ priv->clkout = pdata->cor;
+ priv->bus_config = pdata->bcr;
+ priv->cpu_interface = pdata->cir;
+
+ return 0;
+}
+
+static int __devinit cc770_platform_probe(struct platform_device *pdev)
+{
+ struct net_device *dev;
+ struct cc770_priv *priv;
+ struct resource *mem;
+ resource_size_t mem_size;
+ void __iomem *base;
+ int err, irq;
+
+ mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ irq = platform_get_irq(pdev, 0);
+ if (!mem || irq <= 0)
+ return -ENODEV;
+
+ mem_size = resource_size(mem);
+ if (!request_mem_region(mem->start, mem_size, pdev->name))
+ return -EBUSY;
+
+ base = ioremap(mem->start, mem_size);
+ if (!base) {
+ err = -ENOMEM;
+ goto exit_release_mem;
+ }
+
+ dev = alloc_cc770dev(0);
+ if (!dev) {
+ err = -ENOMEM;
+ goto exit_unmap_mem;
+ }
+
+ dev->irq = irq;
+ priv = netdev_priv(dev);
+ priv->read_reg = cc770_platform_read_reg;
+ priv->write_reg = cc770_platform_write_reg;
+ priv->irq_flags = IRQF_SHARED;
+ priv->reg_base = base;
+
+ if (pdev->dev.of_node)
+ err = cc770_get_of_node_data(pdev, priv);
+ else if (pdev->dev.platform_data)
+ err = cc770_get_platform_data(pdev, priv);
+ else
+ err = -ENODEV;
+ if (err)
+ goto exit_free_cc770;
+
+ dev_dbg(&pdev->dev,
+ "reg_base=0x%p irq=%d clock=%d cpu_interface=0x%02x "
+ "bus_config=0x%02x clkout=0x%02x\n",
+ priv->reg_base, dev->irq, priv->can.clock.freq,
+ priv->cpu_interface, priv->bus_config, priv->clkout);
+
+ dev_set_drvdata(&pdev->dev, dev);
+ SET_NETDEV_DEV(dev, &pdev->dev);
+
+ err = register_cc770dev(dev);
+ if (err) {
+ dev_err(&pdev->dev,
+ "couldn't register CC700 device (err=%d)\n", err);
+ goto exit_free_cc770;
+ }
+
+ return 0;
+
+exit_free_cc770:
+ free_cc770dev(dev);
+exit_unmap_mem:
+ iounmap(base);
+exit_release_mem:
+ release_mem_region(mem->start, mem_size);
+
+ return err;
+}
+
+static int __devexit cc770_platform_remove(struct platform_device *pdev)
+{
+ struct net_device *dev = dev_get_drvdata(&pdev->dev);
+ struct cc770_priv *priv = netdev_priv(dev);
+ struct resource *mem;
+
+ unregister_cc770dev(dev);
+ iounmap(priv->reg_base);
+ free_cc770dev(dev);
+
+ mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (mem)
+ release_mem_region(mem->start, resource_size(mem));
+ else
+ dev_err(&pdev->dev, "couldn't release mem region");
+
+ return 0;
+}
+
+static struct of_device_id __devinitdata cc770_platform_table[] = {
+ {.compatible = "bosch,cc770"}, /* CC770 from Bosch */
+ {.compatible = "intc,82527"}, /* AN82527 from Intel CP */
+ {},
+};
+
+static struct platform_driver cc770_platform_driver = {
+ .driver = {
+ .name = DRV_NAME,
+ .owner = THIS_MODULE,
+ .of_match_table = cc770_platform_table,
+ },
+ .probe = cc770_platform_probe,
+ .remove = __devexit_p(cc770_platform_remove),
+};
+
+module_platform_driver(cc770_platform_driver);
+
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH 2/6] powerpc/time: Use clockevents_calc_mult_shift
From: Anton Blanchard @ 2011-11-28 11:47 UTC (permalink / raw)
To: Kumar Gala; +Cc: paulus, linuxppc-dev, miltonm, johnstul
In-Reply-To: <B695AB74-08F7-46DE-A9E9-BC7475FA1F3F@kernel.crashing.org>
Hi Kumar,
> > static void register_decrementer_clockevent(int cpu)
> > {
> > struct clock_event_device *dec = &per_cpu(decrementers,
> > cpu).event; @@ -955,7 +928,8 @@ static void __init
> > init_decrementer_cloc {
> > int cpu = smp_processor_id();
> >
> > - setup_clockevent_multiplier(ppc_tb_freq);
> > + clockevents_calc_mult_shift(&decrementer_clockevent,
> > ppc_tb_freq, 4); +
>
> Where's this magic 4 come from?
No better reason than that's what most other users do. We weren't
placing any limits on the shift/multiply in the old loop I don't think
we need to, so we could probably just use 1 instead.
Anton
^ permalink raw reply
* Re: sam460ex, sm501 incorrect device id with kernel >=linux-2.6.39
From: Josh Boyer @ 2011-11-28 12:12 UTC (permalink / raw)
To: acrux; +Cc: linuxppc-dev
In-Reply-To: <20111127173748.9f235741.acrux_it@libero.it>
On Sun, Nov 27, 2011 at 11:37 AM, acrux <acrux_it@libero.it> wrote:
>
> Acube Sam460ex has SM502 as onboard video.
> I got this with any kernel newer than 2.6.38.x thus the framebuffer is lost too:
>
> root@sam4x0:~# diff dmesg-2.6.38.8 dmesg-2.6.39.4
> 1,2c1,3
> < Using PowerPC 44x Platform machine description
> < Linux version 2.6.38.8-Sam460ex (root@sam4x0) (gcc version 4.5.3 (CRUX PPC) ) #1 Fri Nov 11 22:07:53 CET 2011
> ---
>> Using Canyonlands machine description
>> Initializing cgroup subsys cpu
You seem to have switched from using the generic PowerPC 44x platform,
to using a Canyonlands kernel and/or DTB. Not sure why that is, but
it's probably not helping your issues at all.
>> Linux version 2.6.39.4-Sam460ex (root@sam4x0) (gcc version 4.5.3 (CRUX PPC) ) #1 Fri Nov 11 19:06:02 CET 2011
> 17c18
> [cut]
> 161,179c165,167
> < sm501 0001:00:06.0: SM501 At f5480000: Version 050100c0, 64 Mb, IRQ 19
> ---
>> sm501 0001:00:06.0: incorrect device id c0000105
Something is reading the device ID in the wrong endian (and probably
everything else).
josh
^ permalink raw reply
* Re: [PATCH net-next v3 3/4] can: cc770: add platform bus driver for the CC770 and AN82527
From: Marc Kleine-Budde @ 2011-11-28 12:33 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: Stanislav Yelenskiy, netdev, Devicetree-discuss, linux-can,
linuxppc-dev, IreneV, socketcan-users, Wolfgang Zarre
In-Reply-To: <1322479858-4874-4-git-send-email-wg@grandegger.com>
[-- Attachment #1: Type: text/plain, Size: 13373 bytes --]
On 11/28/2011 12:30 PM, Wolfgang Grandegger wrote:
> This driver works with both, static platform data and device tree
> bindings. It has been tested on a TQM855L board with two AN82527
> CAN controllers on the local bus.
>
> CC: Devicetree-discuss@lists.ozlabs.org
> CC: linuxppc-dev@ozlabs.org
> CC: Kumar Gala <galak@kernel.crashing.org>
> Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
See comment in the _remove function. Otherwise good. Add my:
Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
> .../devicetree/bindings/net/can/cc770.txt | 56 ++++
> drivers/net/can/cc770/Kconfig | 7 +
> drivers/net/can/cc770/Makefile | 1 +
> drivers/net/can/cc770/cc770_platform.c | 280 ++++++++++++++++++++
> 4 files changed, 344 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/can/cc770.txt
> create mode 100644 drivers/net/can/cc770/cc770_platform.c
>
> diff --git a/Documentation/devicetree/bindings/net/can/cc770.txt b/Documentation/devicetree/bindings/net/can/cc770.txt
> new file mode 100644
> index 0000000..01e282d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/can/cc770.txt
> @@ -0,0 +1,56 @@
> +Memory mapped Bosch CC770 and Intel AN82527 CAN controller
> +
> +Note: The CC770 is a CAN controller from Bosch, which is 100%
> +compatible with the old AN82527 from Intel, but with "bugs" being fixed.
> +
> +Required properties:
> +
> +- compatible : should be "bosch,cc770" for the CC770 and "intc,82527"
> + for the AN82527.
> +
> +- reg : should specify the chip select, address offset and size required
> + to map the registers of the controller. The size is usually 0x80.
> +
> +- interrupts : property with a value describing the interrupt source
> + (number and sensitivity) required for the controller.
> +
> +Optional properties:
> +
> +- bosch,external-clock-frequency : frequency of the external oscillator
> + clock in Hz. Note that the internal clock frequency used by the
> + controller is half of that value. If not specified, a default
> + value of 16000000 (16 MHz) is used.
> +
> +- bosch,clock-out-frequency : slock frequency in Hz on the CLKOUT pin.
> + If not specified or if the specified value is 0, the CLKOUT pin
> + will be disabled.
> +
> +- bosch,slew-rate : slew rate of the CLKOUT signal. If not specified,
> + a resonable value will be calculated.
> +
> +- bosch,disconnect-rx0-input : see data sheet.
> +
> +- bosch,disconnect-rx1-input : see data sheet.
> +
> +- bosch,disconnect-tx1-output : see data sheet.
> +
> +- bosch,polarity-dominant : see data sheet.
> +
> +- bosch,divide-memory-clock : see data sheet.
> +
> +- bosch,iso-low-speed-mux : see data sheet.
> +
> +For further information, please have a look to the CC770 or AN82527.
> +
> +Examples:
> +
> +can@3,100 {
> + compatible = "bosch,cc770";
> + reg = <3 0x100 0x80>;
> + interrupts = <2 0>;
> + interrupt-parent = <&mpic>;
> + bosch,external-clock-frequency = <16000000>;
> +};
> +
> +
> +
> diff --git a/drivers/net/can/cc770/Kconfig b/drivers/net/can/cc770/Kconfig
> index 28e4d48..22c07a8 100644
> --- a/drivers/net/can/cc770/Kconfig
> +++ b/drivers/net/can/cc770/Kconfig
> @@ -11,4 +11,11 @@ config CAN_CC770_ISA
> connected to the ISA bus using I/O port, memory mapped or
> indirect access.
>
> +config CAN_CC770_PLATFORM
> + tristate "Generic Platform Bus based CC770 driver"
> + ---help---
> + This driver adds support for the CC770 and AN82527 chips
> + connected to the "platform bus" (Linux abstraction for directly
> + to the processor attached devices).
> +
> endif
> diff --git a/drivers/net/can/cc770/Makefile b/drivers/net/can/cc770/Makefile
> index 872ecff..9fb8321 100644
> --- a/drivers/net/can/cc770/Makefile
> +++ b/drivers/net/can/cc770/Makefile
> @@ -4,5 +4,6 @@
>
> obj-$(CONFIG_CAN_CC770) += cc770.o
> obj-$(CONFIG_CAN_CC770_ISA) += cc770_isa.o
> +obj-$(CONFIG_CAN_CC770_PLATFORM) += cc770_platform.o
>
> ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
> diff --git a/drivers/net/can/cc770/cc770_platform.c b/drivers/net/can/cc770/cc770_platform.c
> new file mode 100644
> index 0000000..65e177e
> --- /dev/null
> +++ b/drivers/net/can/cc770/cc770_platform.c
> @@ -0,0 +1,280 @@
> +/*
> + * Driver for CC770 and AN82527 CAN controllers on the platform bus
> + *
> + * Copyright (C) 2009, 2011 Wolfgang Grandegger <wg@grandegger.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the version 2 of the GNU General Public License
> + * as published by the Free Software Foundation
> + *
> + * 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.
please remove the address.
> + */
> +
> +/*
> + * If platform data are used you should have similar definitions
> + * in your board-specific code:
> + *
> + * static struct cc770_platform_data myboard_cc770_pdata = {
> + * .osc_freq = 16000000,
> + * .cir = 0x41,
> + * .cor = 0x20,
> + * .bcr = 0x40,
> + * };
> + *
> + * Please see include/linux/can/platform/cc770.h for description of
> + * above fields.
> + *
> + * If the device tree is used, you need a CAN node definition in your
> + * DTS file similar to:
> + *
> + * can@3,100 {
> + * compatible = "bosch,cc770";
> + * reg = <3 0x100 0x80>;
> + * interrupts = <2 0>;
> + * interrupt-parent = <&mpic>;
> + * bosch,external-clock-frequency = <16000000>;
> + * };
> + *
> + * See "Documentation/devicetree/bindings/net/can/cc770.txt" for further
> + * information.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/netdevice.h>
> +#include <linux/delay.h>
> +#include <linux/can.h>
> +#include <linux/can/dev.h>
> +#include <linux/can/platform/cc770.h>
> +
> +#include <linux/of_platform.h>
> +
> +#include "cc770.h"
> +
> +#define DRV_NAME "cc770_platform"
> +
> +MODULE_AUTHOR("Wolfgang Grandegger <wg@grandegger.com>");
> +MODULE_DESCRIPTION("Socket-CAN driver for CC770 on the platform bus");
> +MODULE_LICENSE("GPL v2");
> +
> +#define CC770_PLATFORM_CAN_CLOCK 16000000
> +
> +static u8 cc770_platform_read_reg(const struct cc770_priv *priv, int reg)
> +{
> + return in_8(priv->reg_base + reg);
> +}
> +
> +static void cc770_platform_write_reg(const struct cc770_priv *priv, int reg,
> + u8 val)
> +{
> + out_8(priv->reg_base + reg, val);
> +}
> +
> +static int __devinit cc770_get_of_node_data(struct platform_device *pdev,
> + struct cc770_priv *priv)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + const u32 *prop;
> + int prop_size;
> + u32 clkext;
> +
> + prop = of_get_property(np, "bosch,external-clock-frequency",
> + &prop_size);
> + if (prop && (prop_size == sizeof(u32)))
> + clkext = *prop;
> + else
> + clkext = CC770_PLATFORM_CAN_CLOCK; /* default */
> + priv->can.clock.freq = clkext;
> +
> + /* The system clock may not exceed 10 MHz */
> + if (priv->can.clock.freq > 10000000) {
> + priv->cpu_interface |= CPUIF_DSC;
> + priv->can.clock.freq /= 2;
> + }
> +
> + /* The memory clock may not exceed 8 MHz */
> + if (priv->can.clock.freq > 8000000)
> + priv->cpu_interface |= CPUIF_DMC;
> +
> + if (of_get_property(np, "bosch,divide-memory-clock", NULL))
> + priv->cpu_interface |= CPUIF_DMC;
> + if (of_get_property(np, "bosch,iso-low-speed-mux", NULL))
> + priv->cpu_interface |= CPUIF_MUX;
> +
> + if (!of_get_property(np, "bosch,no-comperator-bypass", NULL))
> + priv->bus_config |= BUSCFG_CBY;
> + if (of_get_property(np, "bosch,disconnect-rx0-input", NULL))
> + priv->bus_config |= BUSCFG_DR0;
> + if (of_get_property(np, "bosch,disconnect-rx1-input", NULL))
> + priv->bus_config |= BUSCFG_DR1;
> + if (of_get_property(np, "bosch,disconnect-tx1-output", NULL))
> + priv->bus_config |= BUSCFG_DT1;
> + if (of_get_property(np, "bosch,polarity-dominant", NULL))
> + priv->bus_config |= BUSCFG_POL;
> +
> + prop = of_get_property(np, "bosch,clock-out-frequency", &prop_size);
> + if (prop && (prop_size == sizeof(u32)) && *prop > 0) {
> + u32 cdv = clkext / *prop;
> + int slew;
> +
> + if (cdv > 0 && cdv < 16) {
> + priv->cpu_interface |= CPUIF_CEN;
> + priv->clkout |= (cdv - 1) & CLKOUT_CD_MASK;
> +
> + prop = of_get_property(np, "bosch,slew-rate",
> + &prop_size);
> + if (prop && (prop_size == sizeof(u32))) {
> + slew = *prop;
> + } else {
> + /* Determine default slew rate */
> + slew = (CLKOUT_SL_MASK >>
> + CLKOUT_SL_SHIFT) -
> + ((cdv * clkext - 1) / 8000000);
> + if (slew < 0)
> + slew = 0;
> + }
> + priv->clkout |= (slew << CLKOUT_SL_SHIFT) &
> + CLKOUT_SL_MASK;
> + } else {
> + dev_dbg(&pdev->dev, "invalid clock-out-frequency\n");
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int __devinit cc770_get_platform_data(struct platform_device *pdev,
> + struct cc770_priv *priv)
> +{
> +
> + struct cc770_platform_data *pdata = pdev->dev.platform_data;
> +
> + priv->can.clock.freq = pdata->osc_freq;
> + if (priv->cpu_interface | CPUIF_DSC)
> + priv->can.clock.freq /= 2;
> + priv->clkout = pdata->cor;
> + priv->bus_config = pdata->bcr;
> + priv->cpu_interface = pdata->cir;
> +
> + return 0;
> +}
> +
> +static int __devinit cc770_platform_probe(struct platform_device *pdev)
> +{
> + struct net_device *dev;
> + struct cc770_priv *priv;
> + struct resource *mem;
> + resource_size_t mem_size;
> + void __iomem *base;
> + int err, irq;
> +
> + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + irq = platform_get_irq(pdev, 0);
> + if (!mem || irq <= 0)
> + return -ENODEV;
> +
> + mem_size = resource_size(mem);
> + if (!request_mem_region(mem->start, mem_size, pdev->name))
> + return -EBUSY;
> +
> + base = ioremap(mem->start, mem_size);
> + if (!base) {
> + err = -ENOMEM;
> + goto exit_release_mem;
> + }
> +
> + dev = alloc_cc770dev(0);
> + if (!dev) {
> + err = -ENOMEM;
> + goto exit_unmap_mem;
> + }
> +
> + dev->irq = irq;
> + priv = netdev_priv(dev);
> + priv->read_reg = cc770_platform_read_reg;
> + priv->write_reg = cc770_platform_write_reg;
> + priv->irq_flags = IRQF_SHARED;
> + priv->reg_base = base;
> +
> + if (pdev->dev.of_node)
> + err = cc770_get_of_node_data(pdev, priv);
> + else if (pdev->dev.platform_data)
> + err = cc770_get_platform_data(pdev, priv);
> + else
> + err = -ENODEV;
> + if (err)
> + goto exit_free_cc770;
> +
> + dev_dbg(&pdev->dev,
> + "reg_base=0x%p irq=%d clock=%d cpu_interface=0x%02x "
> + "bus_config=0x%02x clkout=0x%02x\n",
> + priv->reg_base, dev->irq, priv->can.clock.freq,
> + priv->cpu_interface, priv->bus_config, priv->clkout);
> +
> + dev_set_drvdata(&pdev->dev, dev);
> + SET_NETDEV_DEV(dev, &pdev->dev);
> +
> + err = register_cc770dev(dev);
> + if (err) {
> + dev_err(&pdev->dev,
> + "couldn't register CC700 device (err=%d)\n", err);
> + goto exit_free_cc770;
> + }
> +
> + return 0;
> +
> +exit_free_cc770:
> + free_cc770dev(dev);
> +exit_unmap_mem:
> + iounmap(base);
> +exit_release_mem:
> + release_mem_region(mem->start, mem_size);
> +
> + return err;
> +}
> +
> +static int __devexit cc770_platform_remove(struct platform_device *pdev)
> +{
> + struct net_device *dev = dev_get_drvdata(&pdev->dev);
> + struct cc770_priv *priv = netdev_priv(dev);
> + struct resource *mem;
> +
> + unregister_cc770dev(dev);
> + iounmap(priv->reg_base);
> + free_cc770dev(dev);
> +
> + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
Can this fail?
> + if (mem)
> + release_mem_region(mem->start, resource_size(mem));
> + else
> + dev_err(&pdev->dev, "couldn't release mem region");
> +
> + return 0;
> +}
> +
> +static struct of_device_id __devinitdata cc770_platform_table[] = {
> + {.compatible = "bosch,cc770"}, /* CC770 from Bosch */
> + {.compatible = "intc,82527"}, /* AN82527 from Intel CP */
> + {},
> +};
> +
> +static struct platform_driver cc770_platform_driver = {
> + .driver = {
> + .name = DRV_NAME,
> + .owner = THIS_MODULE,
> + .of_match_table = cc770_platform_table,
> + },
> + .probe = cc770_platform_probe,
> + .remove = __devexit_p(cc770_platform_remove),
> +};
> +
> +module_platform_driver(cc770_platform_driver);
nice, I like the new module_platform_driver.
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply
* Re: [PATCH 03/13] powerpc: Fix booke hugetlb preload code for PPC_MM_SLICES and 64-bit
From: Kumar Gala @ 2011-11-28 16:01 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, david
In-Reply-To: <1322181832.32635.14.camel@pasglop>
On Nov 24, 2011, at 6:43 PM, Benjamin Herrenschmidt wrote:
> On Mon, 2011-10-10 at 15:50 -0500, Becky Bruce wrote:
>=20
> .../...
>=20
>> #ifdef CONFIG_PPC_MM_SLICES
>> - psize =3D mmu_get_tsize(get_slice_psize(mm, ea));
>> - tsize =3D mmu_get_psize(psize);
>> + psize =3D get_slice_psize(mm, ea);
>> + tsize =3D mmu_get_tsize(psize);
>> shift =3D mmu_psize_defs[psize].shift;
>> #else
>> - vma =3D find_vma(mm, ea);
>> - psize =3D vma_mmu_pagesize(vma); /* returns actual size =
in bytes */
>> - asm (PPC_CNTLZL "%0,%1" : "=3Dr" (lz) : "r" (psize));
>> - shift =3D 31 - lz;
>> - tsize =3D 21 - lz;
>> + psize =3D vma_mmu_pagesize(find_vma(mm, ea));
>> + shift =3D __ilog2(psize);
>> + tsize =3D shift - 10;
>> #endif
>=20
> Now, I know it was already there and you are just moving it around in
> this patch but come on ... find_vma() here ? Really ? And with no =
result
> checking nor boundary checking (remember it can return a vma that
> doesn't enclose the address etc....). Now I know in this specific case
> it -should- be safe but still...
>=20
> Now, the caller is just doing:
>=20
> book3e_hugetlb_preload(vma->vm_mm, address, *ptep);
>=20
> So why not just change the prototype and pass the vma down instead ?
Can we do this on top of the patchset instead of changing the existing =
patchset?
- k=
^ permalink raw reply
* Re: [PATCH] powerpc: Decode correct MSR bits in oops output
From: Kumar Gala @ 2011-11-28 16:04 UTC (permalink / raw)
To: Anton Blanchard; +Cc: paulus, linuxppc-dev
In-Reply-To: <20111125163557.5a464006@kryten>
On Nov 24, 2011, at 11:35 PM, Anton Blanchard wrote:
>=20
> On a 64bit book3s machine I have an oops from a system reset that
> claims the book3e CE bit was set:
>=20
> MSR: 8000000000021032 <ME,CE,IR,DR> CR: 24004082 XER: 00000010
>=20
> On a book3s machine system reset sets IBM bit 46 and 47 depending on
> the power saving mode. Separate the definitions by type and for
> completeness add the rest of the bits in.
>=20
> Signed-off-by: Anton Blanchard <anton@samba.org>
> ---
>=20
> Index: linux-build/arch/powerpc/kernel/process.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- linux-build.orig/arch/powerpc/kernel/process.c 2011-11-25 =
13:22:24.294919094 +1100
> +++ linux-build/arch/powerpc/kernel/process.c 2011-11-25 =
13:36:23.213834524 +1100
> @@ -584,16 +584,32 @@ static struct regbit {
> unsigned long bit;
> const char *name;
> } msr_bits[] =3D {
> +#if defined(CONFIG_PPC64) && !defined(CONFIG_BOOKE)
> + {MSR_SF, "SF"},
> + {MSR_HV, "HV"},
> +#endif
> + {MSR_VEC, "VEC"},
> + {MSR_VSX, "VSX"},
> +#ifdef CONFIG_BOOKE
> + {MSR_CE, "CE"},
> +#endif
> {MSR_EE, "EE"},
> {MSR_PR, "PR"},
> {MSR_FP, "FP"},
> - {MSR_VEC, "VEC"},
> - {MSR_VSX, "VSX"},
> {MSR_ME, "ME"},
> - {MSR_CE, "CE"},
> +#ifdef CONFIG_BOOKE
> {MSR_DE, "DE"},
> +#else
> + {MSR_SE, "SE"},
> + {MSR_BE, "BE"},
> +#endif
> {MSR_IR, "IR"},
> {MSR_DR, "DR"},
> + {MSR_PMM, "PMM"},
> +#ifndef CONFIG_BOOKE
> + {MSR_RI, "RI"},
We have 'RI' on some BOOKE so lets allow for it to be decoded
> + {MSR_LE, "LE"},
> +#endif
> {0, NULL}
> };
Since you're fixing this can you add the following for CONFIG_BOOKE:
MSR_GS, MSR_UCLE, MSR_PMM, MSR_CM
- k=
^ permalink raw reply
* Re: [PATCH] powerpc: Decode correct MSR bits in oops output
From: Josh Boyer @ 2011-11-28 16:23 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev, paulus, Anton Blanchard
In-Reply-To: <F744714B-0527-415C-92BE-A5CB678F1C29@kernel.crashing.org>
On Mon, Nov 28, 2011 at 11:04 AM, Kumar Gala <galak@kernel.crashing.org> wr=
ote:
>
> On Nov 24, 2011, at 11:35 PM, Anton Blanchard wrote:
>
>>
>> On a 64bit book3s machine I have an oops from a system reset that
>> claims the book3e CE bit was set:
>>
>> MSR: 8000000000021032 <ME,CE,IR,DR> =A0CR: 24004082 =A0XER: 00000010
>>
>> On a book3s machine system reset sets IBM bit 46 and 47 depending on
>> the power saving mode. Separate the definitions by type and for
>> completeness add the rest of the bits in.
>>
>> Signed-off-by: Anton Blanchard <anton@samba.org>
>> ---
>>
>> Index: linux-build/arch/powerpc/kernel/process.c
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>> --- linux-build.orig/arch/powerpc/kernel/process.c =A0 =A02011-11-25 13:=
22:24.294919094 +1100
>> +++ linux-build/arch/powerpc/kernel/process.c 2011-11-25 13:36:23.213834=
524 +1100
>> @@ -584,16 +584,32 @@ static struct regbit {
>> =A0 =A0 =A0 unsigned long bit;
>> =A0 =A0 =A0 const char *name;
>> } msr_bits[] =3D {
>> +#if defined(CONFIG_PPC64) && !defined(CONFIG_BOOKE)
>> + =A0 =A0 {MSR_SF, =A0 =A0 =A0 =A0"SF"},
>> + =A0 =A0 {MSR_HV, =A0 =A0 =A0 =A0"HV"},
>> +#endif
>> + =A0 =A0 {MSR_VEC, =A0 =A0 =A0 "VEC"},
>> + =A0 =A0 {MSR_VSX, =A0 =A0 =A0 "VSX"},
>> +#ifdef CONFIG_BOOKE
>> + =A0 =A0 {MSR_CE, =A0 =A0 =A0 =A0"CE"},
>> +#endif
>> =A0 =A0 =A0 {MSR_EE, =A0 =A0 =A0 =A0"EE"},
>> =A0 =A0 =A0 {MSR_PR, =A0 =A0 =A0 =A0"PR"},
>> =A0 =A0 =A0 {MSR_FP, =A0 =A0 =A0 =A0"FP"},
>> - =A0 =A0 {MSR_VEC, =A0 =A0 =A0 "VEC"},
>> - =A0 =A0 {MSR_VSX, =A0 =A0 =A0 "VSX"},
>> =A0 =A0 =A0 {MSR_ME, =A0 =A0 =A0 =A0"ME"},
>> - =A0 =A0 {MSR_CE, =A0 =A0 =A0 =A0"CE"},
>> +#ifdef CONFIG_BOOKE
>> =A0 =A0 =A0 {MSR_DE, =A0 =A0 =A0 =A0"DE"},
>> +#else
>> + =A0 =A0 {MSR_SE, =A0 =A0 =A0 =A0"SE"},
>> + =A0 =A0 {MSR_BE, =A0 =A0 =A0 =A0"BE"},
>> +#endif
>> =A0 =A0 =A0 {MSR_IR, =A0 =A0 =A0 =A0"IR"},
>> =A0 =A0 =A0 {MSR_DR, =A0 =A0 =A0 =A0"DR"},
>> + =A0 =A0 {MSR_PMM, =A0 =A0 =A0 "PMM"},
>> +#ifndef CONFIG_BOOKE
>> + =A0 =A0 {MSR_RI, =A0 =A0 =A0 =A0"RI"},
>
> We have 'RI' on some BOOKE so lets allow for it to be decoded
>
>> + =A0 =A0 {MSR_LE, =A0 =A0 =A0 =A0"LE"},
>> +#endif
>> =A0 =A0 =A0 {0, =A0 =A0 =A0 =A0 =A0 =A0 NULL}
>> };
>
> Since you're fixing this can you add the following for CONFIG_BOOKE:
>
> MSR_GS, MSR_UCLE, MSR_PMM, MSR_CM
Those don't exist on 4xx, so CONFIG_BOOKE doesn't seem appropriate.
josh
^ permalink raw reply
* Re: [PATCH][RFC] fsldma: fix performance degradation by optimizing spinlock use.
From: Ira W. Snyder @ 2011-11-28 16:38 UTC (permalink / raw)
To: Shi Xuelin-B29237
Cc: vinod.koul@intel.com, dan.j.williams@intel.com,
linuxppc-dev@lists.ozlabs.org, Li Yang-R58472,
linux-kernel@vger.kernel.org
In-Reply-To: <DBB740589CE8814680DECFE34BE197AB148CDD@039-SN1MPN1-006.039d.mgd.msft.net>
On Thu, Nov 24, 2011 at 08:12:25AM +0000, Shi Xuelin-B29237 wrote:
> Hi Ira,
>
> Thanks for your review.
>
> After second thought, I think your scenario may not occur.
> Because the cookie 20 we query must be returned by fsl_dma_tx_submit(...) in practice.
> We never query a cookie not returned by fsl_dma_tx_submit(...).
>
I agree about this part.
> When we call fsl_tx_status(20), the chan->common.cookie is definitely wrote as 20 and cpu2 could not read as 19.
>
This is what I don't agree about. However, I'm not an expert on CPU
cache vs. memory accesses in an multi-processor system. The section
titled "CACHE COHERENCY" in Documentation/memory-barriers.txt leads me
to believe that the scenario I described is possible.
What happens if CPU1's write of chan->common.cookie only goes into
CPU1's cache. It never makes it to main memory before CPU2 fetches the
old value of 19.
I don't think you should see any performance impact from the smp_mb()
operation.
Thanks,
Ira
> -----Original Message-----
> From: Ira W. Snyder [mailto:iws@ovro.caltech.edu]
> Sent: 2011年11月23日 2:59
> To: Shi Xuelin-B29237
> Cc: dan.j.williams@intel.com; Li Yang-R58472; zw@zh-kernel.org; vinod.koul@intel.com; linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH][RFC] fsldma: fix performance degradation by optimizing spinlock use.
>
> On Tue, Nov 22, 2011 at 12:55:05PM +0800, b29237@freescale.com wrote:
> > From: Forrest Shi <b29237@freescale.com>
> >
> > dma status check function fsl_tx_status is heavily called in
> > a tight loop and the desc lock in fsl_tx_status contended by
> > the dma status update function. this caused the dma performance
> > degrades much.
> >
> > this patch releases the lock in the fsl_tx_status function.
> > I believe it has no neglect impact on the following call of
> > dma_async_is_complete(...).
> >
> > we can see below three conditions will be identified as success
> > a) x < complete < use
> > b) x < complete+N < use+N
> > c) x < complete < use+N
> > here complete is the completed_cookie, use is the last_used
> > cookie, x is the querying cookie, N is MAX cookie
> >
> > when chan->completed_cookie is being read, the last_used may
> > be incresed. Anyway it has no neglect impact on the dma status
> > decision.
> >
> > Signed-off-by: Forrest Shi <xuelin.shi@freescale.com>
> > ---
> > drivers/dma/fsldma.c | 5 -----
> > 1 files changed, 0 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c index
> > 8a78154..1dca56f 100644
> > --- a/drivers/dma/fsldma.c
> > +++ b/drivers/dma/fsldma.c
> > @@ -986,15 +986,10 @@ static enum dma_status fsl_tx_status(struct dma_chan *dchan,
> > struct fsldma_chan *chan = to_fsl_chan(dchan);
> > dma_cookie_t last_complete;
> > dma_cookie_t last_used;
> > - unsigned long flags;
> > -
> > - spin_lock_irqsave(&chan->desc_lock, flags);
> >
>
> This will cause a bug. See below for a detailed explanation. You need this instead:
>
> /*
> * On an SMP system, we must ensure that this CPU has seen the
> * memory accesses performed by another CPU under the
> * chan->desc_lock spinlock.
> */
> smp_mb();
> > last_complete = chan->completed_cookie;
> > last_used = dchan->cookie;
> >
> > - spin_unlock_irqrestore(&chan->desc_lock, flags);
> > -
> > dma_set_tx_state(txstate, last_complete, last_used, 0);
> > return dma_async_is_complete(cookie, last_complete, last_used); }
>
> Facts:
> - dchan->cookie is the same member as chan->common.cookie (same memory location)
> - chan->common.cookie is the "last allocated cookie for a pending transaction"
> - chan->completed_cookie is the "last completed transaction"
>
> I have replaced "dchan->cookie" with "chan->common.cookie" in the below explanation, to keep everything referenced from the same structure.
>
> Variable usage before your change. Everything is used locked.
> - RW chan->common.cookie (fsl_dma_tx_submit)
> - R chan->common.cookie (fsl_tx_status)
> - R chan->completed_cookie (fsl_tx_status)
> - W chan->completed_cookie (dma_do_tasklet)
>
> Variable usage after your change:
> - RW chan->common.cookie LOCKED
> - R chan->common.cookie NO LOCK
> - R chan->completed_cookie NO LOCK
> - W chan->completed_cookie LOCKED
>
> What if we assume that you have a 2 CPU system (such as a P2020). After your changes, one possible sequence is:
>
> === CPU1 - allocate + submit descriptor: fsl_dma_tx_submit() === spin_lock_irqsave
> descriptor->cookie = 20 (x in your example)
> chan->common.cookie = 20 (used in your example)
> spin_unlock_irqrestore
>
> === CPU2 - immediately calls fsl_tx_status() ===
> chan->common.cookie == 19
> chan->completed_cookie == 19
> descriptor->cookie == 20
>
> Since we don't have locks anymore, CPU2 may not have seen the write to
> chan->common.cookie yet.
>
> Also assume that the DMA hardware has not started processing the transaction yet. Therefore dma_do_tasklet() has not been called, and
> chan->completed_cookie has not been updated.
>
> In this case, dma_async_is_complete() (on CPU2) returns DMA_SUCCESS, even though the DMA operation has not succeeded. The DMA operation has not even started yet!
>
> The smp_mb() fixes this, since it forces CPU2 to have seen all memory operations that happened before CPU1 released the spinlock. Spinlocks are implicit SMP memory barriers.
>
> Therefore, the above example becomes:
> smp_mb();
> chan->common.cookie == 20
> chan->completed_cookie == 19
> descriptor->cookie == 20
>
> Then dma_async_is_complete() returns DMA_IN_PROGRESS, which is correct.
>
> Thanks,
> Ira
>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* Re: [PATCH 03/13] powerpc: Fix booke hugetlb preload code for PPC_MM_SLICES and 64-bit
From: Becky Bruce @ 2011-11-28 16:54 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, david
In-Reply-To: <1322181832.32635.14.camel@pasglop>
On Nov 24, 2011, at 6:43 PM, Benjamin Herrenschmidt wrote:
> On Mon, 2011-10-10 at 15:50 -0500, Becky Bruce wrote:
>=20
> .../...
>=20
>> #ifdef CONFIG_PPC_MM_SLICES
>> - psize =3D mmu_get_tsize(get_slice_psize(mm, ea));
>> - tsize =3D mmu_get_psize(psize);
>> + psize =3D get_slice_psize(mm, ea);
>> + tsize =3D mmu_get_tsize(psize);
>> shift =3D mmu_psize_defs[psize].shift;
>> #else
>> - vma =3D find_vma(mm, ea);
>> - psize =3D vma_mmu_pagesize(vma); /* returns actual size =
in bytes */
>> - asm (PPC_CNTLZL "%0,%1" : "=3Dr" (lz) : "r" (psize));
>> - shift =3D 31 - lz;
>> - tsize =3D 21 - lz;
>> + psize =3D vma_mmu_pagesize(find_vma(mm, ea));
>> + shift =3D __ilog2(psize);
>> + tsize =3D shift - 10;
>> #endif
>=20
> Now, I know it was already there and you are just moving it around in
> this patch but come on ... find_vma() here ? Really ? And with no =
result
> checking nor boundary checking (remember it can return a vma that
> doesn't enclose the address etc....). Now I know in this specific case
> it -should- be safe but still...
>=20
> Now, the caller is just doing:
>=20
> book3e_hugetlb_preload(vma->vm_mm, address, *ptep);
>=20
> So why not just change the prototype and pass the vma down instead ?
There's no reason - I just left the prototype the way it was in the =
original code, but it makes sense to change it given the changes to the =
function. Respin coming.....
-B
^ permalink raw reply
* Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip
From: Scott Wood @ 2011-11-28 17:20 UTC (permalink / raw)
To: Li Yang-R58472
Cc: Wood Scott-B07421, Artem.Bityutskiy@nokia.com,
linuxppc-dev@lists.ozlabs.org, Liu Shuo-B35362,
linux-kernel@vger.kernel.org, linux-mtd@lists.infradead.org,
akpm@linux-foundation.org, dwmw2@infradead.org
In-Reply-To: <3F607A5180246847A760FD34122A1E052DC4CD@039-SN1MPN1-003.039d.mgd.msft.net>
On 11/24/2011 01:37 AM, Li Yang-R58472 wrote:
>> +static void io_to_buffer(struct mtd_info *mtd, int subpage, int oob)
>> +{
>> + struct nand_chip *chip = mtd->priv;
>> + struct fsl_elbc_mtd *priv = chip->priv;
>> + struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = priv->ctrl->nand;
>> + void *src, *dst;
>> + int len = (oob ? 64 : 2048);
>> +
>> + if (oob)
>> + dst = elbc_fcm_ctrl->buffer + mtd->writesize + subpage * 64;
>> + else
>> + dst = elbc_fcm_ctrl->buffer + subpage * 2048;
>> +
>> + src = elbc_fcm_ctrl->addr + (oob ? 2048 : 0);
>> + memcpy_fromio(dst, src, len);
>
> Might be safer to use _memcpy_fromio()
How so?
memcpy_fromio() is the public interface that will end up calling
_memcpy_fromio() on powerpc.
-Scott
^ permalink raw reply
* Re: [PATCH] powerpc/85xx: do not force PHYS_64BIT on the P1022DS
From: Timur Tabi @ 2011-11-28 17:41 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <C0BC3E1D-5FCF-42EC-A030-4F86EC1D76F3@freescale.com>
Kumar Gala wrote:
> If you want me to apply this please also provided a 32-bit .dts for
> p1022ds. This should be pretty trivial based on my recent .dts
> cleanups.
I think I found another bug in the 36-bit DTS. Looking at U-Boot, I see this:
#ifdef CONFIG_PHYS_64BIT
#define CONFIG_SYS_PCIE2_MEM_BUS 0xe0000000
#define CONFIG_SYS_PCIE2_MEM_PHYS 0xc20000000ull
#else
#define CONFIG_SYS_PCIE2_MEM_BUS 0xa0000000
#define CONFIG_SYS_PCIE2_MEM_PHYS 0xa0000000
#endif
But the 36-bit DTS has this:
pci0: pcie@ffe09000 {
reg = <0x0 0xffe09000 0 0x1000>;
ranges = <0x2000000 0x0 0xa0000000 0xc 0x20000000 0x0 0x20000000
0x1000000 0x0 0x00000000 0xf 0xffc10000 0x0 0x10000>;
I don't think these match. I think the first 'ranges' line should have 0xe0000000 instead of 0xa0000000.
I see the same problem with the other two PCI busses. It looks like the physical address is correct, but the BUS address is wrong (it's using the 32-bit bus address instead of the 36-bit bus address).
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH 1/6] powerpc/time: Handle wrapping of decrementer
From: Scott Wood @ 2011-11-28 17:44 UTC (permalink / raw)
To: Anton Blanchard; +Cc: paulus, linuxppc-dev, miltonm, johnstul
In-Reply-To: <20111124060847.105131188@samba.org>
On 11/24/2011 12:07 AM, Anton Blanchard wrote:
> Index: linux-build/arch/powerpc/kernel/irq.c
> ===================================================================
> --- linux-build.orig/arch/powerpc/kernel/irq.c 2011-11-17 10:04:16.551137554 +1100
> +++ linux-build/arch/powerpc/kernel/irq.c 2011-11-17 14:23:10.834514143 +1100
> @@ -164,16 +164,13 @@ notrace void arch_local_irq_restore(unsi
> */
> local_paca->hard_enabled = en;
>
> -#ifndef CONFIG_BOOKE
> - /* On server, re-trigger the decrementer if it went negative since
> - * some processors only trigger on edge transitions of the sign bit.
> - *
> - * BookE has a level sensitive decrementer (latches in TSR) so we
> - * don't need that
> + /*
> + * Trigger the decrementer if we have a pending event. Some processors
> + * only trigger on edge transitions of the sign bit. We might also
> + * have disabled interrupts long enough that the decrementer wrapped
> + * to positive.
> */
> - if ((int)mfspr(SPRN_DEC) < 0)
> - mtspr(SPRN_DEC, 1);
> -#endif /* CONFIG_BOOKE */
> + decrementer_check_overflow();
Where did the #ifndef CONFIG_BOOKE go? BookE doesn't need this; the
interrupt will continue asserting until software clears TSR[DIS].
-Scott
^ permalink raw reply
* Re: [PATCH] powerpc: Decode correct MSR bits in oops output
From: Scott Wood @ 2011-11-28 19:30 UTC (permalink / raw)
To: Josh Boyer; +Cc: paulus, linuxppc-dev, Anton Blanchard
In-Reply-To: <CA+5PVA4kkeuWqdKWAJOJSUue=XsEC=27T2zptb-1P_H+HwNymg@mail.gmail.com>
On 11/28/2011 10:23 AM, Josh Boyer wrote:
> On Mon, Nov 28, 2011 at 11:04 AM, Kumar Gala <galak@kernel.crashing.org> wrote:
>>
>> Since you're fixing this can you add the following for CONFIG_BOOKE:
>>
>> MSR_GS, MSR_UCLE, MSR_PMM, MSR_CM
>
> Those don't exist on 4xx, so CONFIG_BOOKE doesn't seem appropriate.
They're defined by book3e of ISA 2.06.
Not all bits are going to exist on all CPUs -- does 4xx use these bits
to mean something different?
-Scott
^ permalink raw reply
* Re: [PATCH] powerpc: Decode correct MSR bits in oops output
From: Josh Boyer @ 2011-11-28 19:46 UTC (permalink / raw)
To: Scott Wood; +Cc: paulus, linuxppc-dev, Anton Blanchard
In-Reply-To: <4ED3E168.5000002@freescale.com>
On Mon, Nov 28, 2011 at 2:30 PM, Scott Wood <scottwood@freescale.com> wrote:
> On 11/28/2011 10:23 AM, Josh Boyer wrote:
>> On Mon, Nov 28, 2011 at 11:04 AM, Kumar Gala <galak@kernel.crashing.org> wrote:
>>>
>>> Since you're fixing this can you add the following for CONFIG_BOOKE:
>>>
>>> MSR_GS, MSR_UCLE, MSR_PMM, MSR_CM
>>
>> Those don't exist on 4xx, so CONFIG_BOOKE doesn't seem appropriate.
>
> They're defined by book3e of ISA 2.06.
4xx existed before that, sadly (as did CONFIG_BOOKE).
> Not all bits are going to exist on all CPUs -- does 4xx use these bits
> to mean something different?
No, marked as reserved. However, given the patch shows up in human
readable output, I don't think we want reserved bits being decoded and
showing up inadvertently.
Could introduce BOOK3E_32 to cover cases like this.
josh
^ permalink raw reply
* Re: sam460ex, sm501 incorrect device id with kernel >=linux-2.6.39
From: acrux @ 2011-11-28 19:56 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev
In-Reply-To: <CA+5PVA592G1RVdj_C+NMFQ8nHx=0-eaF9Hn3NZ8VabpCTQGi1Q@mail.gmail.com>
On Mon, 28 Nov 2011 07:12:38 -0500
Josh Boyer <jwboyer@gmail.com> wrote:
> On Sun, Nov 27, 2011 at 11:37 AM, acrux <acrux_it@libero.it> wrote:
> >
> > Acube Sam460ex has SM502 as onboard video.
> > I got this with any kernel newer than 2.6.38.x thus the framebuffer
> > is lost too:
> >
> > root@sam4x0:~# diff dmesg-2.6.38.8 dmesg-2.6.39.4
> > 1,2c1,3
> > < Using PowerPC 44x Platform machine description
> > < Linux version 2.6.38.8-Sam460ex (root@sam4x0) (gcc version 4.5.3
> > (CRUX PPC) ) #1 Fri Nov 11 22:07:53 CET 2011
> > ---
> >> Using Canyonlands machine description
> >> Initializing cgroup subsys cpu
>
> You seem to have switched from using the generic PowerPC 44x platform,
> to using a Canyonlands kernel and/or DTB. Not sure why that is, but
> it's probably not helping your issues at all.
>
i think it was only a cosmetic change as it was always used
Canyonlands platform and his own proper dtb.
> >> Linux version 2.6.39.4-Sam460ex (root@sam4x0) (gcc version 4.5.3
> >> (CRUX PPC) ) #1 Fri Nov 11 19:06:02 CET 2011
> > 17c18
> > [cut]
> > 161,179c165,167
> > < sm501 0001:00:06.0: SM501 At f5480000: Version 050100c0, 64 Mb,
> > IRQ 19
>
> > ---
> >> sm501 0001:00:06.0: incorrect device id c0000105
>
> Something is reading the device ID in the wrong endian (and probably
> everything else).
>
it seems to be an endianess issue but i didn't find when it was
introduced. Really strange this kind of issue was never noticed
bumping from 2.6.38.x to 2.6.39.x .
--nico
--
GNU/Linux on Power Architecture
CRUX PPC - http://cruxppc.org/
^ 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