Re: [PATCH v3 2/3] hvc_init(): Enforce one-time initialization.
On Tue, 2011-11-08 at 13:45 -0800, Miche Baker-Harvey wrote:
> hvc_init() must only be called once, and no thread should continue with
> hvc_alloc()
> until after initialization is complete. The original code does not enforce
> either
> of these requirements. A new mutex limits entry to hvc_init() to a single
> thread,
> and blocks all later comers until it has completed.
>
> This patch fixes multiple crash symptoms.
Hi Miche,
A few nit-picky comments below ..
> @@ -84,6 +85,10 @@ static LIST_HEAD(hvc_structs);
> * list traversal.
> */
> static DEFINE_SPINLOCK(hvc_structs_lock);
> +/*
> + * only one task does allocation at a time.
> + */
> +static DEFINE_MUTEX(hvc_ports_mutex);
The comment is wrong, isn't it? Only one task does _init_ at a time.
Once the driver is initialised allocs can run concurrently.
So shouldn't it be called hvc_init_mutex ?
> @@ -825,11 +830,15 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
> int i;
>
> /* We wait until a driver actually comes along */
> + mutex_lock(&hvc_ports_mutex);
> if (!hvc_driver) {
> int err = hvc_init();
> - if (err)
> + if (err) {
> + mutex_unlock(&hvc_ports_mutex);
> return ERR_PTR(err);
> + }
> }
> + mutex_unlock(&hvc_ports_mutex);
>
> hp = kzalloc(ALIGN(sizeof(*hp), sizeof(long)) + outbuf_size,
> GFP_KERNEL);
It'd be cleaner I think to do all the locking in hvc_init(). That's safe
as long as you recheck !hvc_driver in hvc_init() with the lock held.
cheers
signature.asc
Description: This is a digitally signed message part
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Export PIR data through sysfs
On Tue, 2011-11-08 at 10:59 -0600, Scott Wood wrote: > On 11/08/2011 12:58 AM, Ananth N Mavinakayanahalli wrote: > > On Mon, Nov 07, 2011 at 11:18:32AM -0600, Scott Wood wrote: > >> What use does userspace have for this? If you want to return the > >> currently executing CPU (which unless you're pinned could change as soon > >> as the value is read...), why not just return smp_processor_id() or > >> hard_smp_processor_id()? > > > > Its not just the current cpu. Decoding PIR can tell you the core id, > > thread id in case of SMT, and this information can be used by userspace > > apps to set affinities, etc. > > Wouldn't it make more sense to expose the thread to core mappings in a > general way, not tied to hardware or what thread we're currently running on? AFAIK that is already available in /sys/devices/system/cpu/cpuX/topology cheers signature.asc Description: This is a digitally signed message part ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 1/5] [ppc] Process dynamic relocations for kernel
On Tue, 08 Nov 2011 10:19:05 -0600 Josh Poimboeuf wrote: > On Tue, 2011-11-08 at 12:41 +0530, Suzuki Poulose wrote: > > What I was suggesting is, instead of flushing the cache in > > relocate(), lets do it like: > > > > for e.g, on 440x, (in head_44x.S :) > > > > #ifdef CONFIG_RELOCATABLE > > ... > > bl relocate > > > > #Flush the d-cache and invalidate the i-cache here > > #endif > > > > > > This would let the different platforms do the the cache > > invalidation in their own way. > > > > Btw, I didn't find an instruction to flush the entire d-cache in > > PPC440 manual. We have instructions to flush only a block > > corresponding to an address. > > > > However, we have 'iccci' which would invalidate the entire i-cache > > which, which I think is better than 80,000 i-cache invalidates. > > In misc_32.S there are already some platform-independent cache > management functions. If we use those, then relocate() could simply > call them. Then the different platforms calling relocate() wouldn't > have to worry about flushing/invalidating caches. > > For example, there's a clean_dcache_range() function. Given any range > twice the size of the d-cache, it should flush the entire d-cache. > But the only drawback is that it would require the caller to know the > size of the d-cache. > > Instead, I think it would be preferable to create a new clean_dcache() > (or clean_dcache_all()?) function in misc_32.S, which could call > clean_dcache_range() with the appropriate args for flushing the entire > d-cache. relocate() could then call the platform-independent > clean_dcache(). > How about using clean_dcache_range() to flush the range runtime address range [ _stext, _end ] ? That would flush the entire instructions. > For i-cache invalidation there's already the (incorrectly named?) > flush_instruction_cache(). It uses the appropriate platform-specific > methods (e.g. iccci for 44x) to invalidate the entire i-cache. Agreed. The only thing that worries me is the use of KERNELBASE in the flush_instruction_cache() for CONFIG_4xx. Can we safely assume all 4xx implementations ignore the arguments passed to iccci ? > > Suzuki, if you agree with this direction, I could work up a new patch > if needed. > I have the following (untested) patch which uses clean_dcache_range() and flush_instruction_cache(): I will send the next version soon with those changes and the DYNAMIC_MEMSTART config for oldstyle relocatoin, if every one agrees to this. diff --git a/arch/powerpc/kernel/reloc_32.S b/arch/powerpc/kernel/reloc_32.S index 045d61e..cce0510 100644 --- a/arch/powerpc/kernel/reloc_32.S +++ b/arch/powerpc/kernel/reloc_32.S @@ -33,10 +33,9 @@ R_PPC_RELATIVE = 22 _GLOBAL(relocate) - mflrr0 + mflrr14 /* Save our LR */ bl 0f /* Find our current runtime address */ 0: mflrr12 /* Make it accessible */ - mtlrr0 lwz r11, (p_dyn - 0b)(r12) add r11, r11, r12 /* runtime address of .dynamic section */ @@ -87,18 +86,19 @@ eodyn: /* End of Dyn Table scan */ * Work out the current offset from the link time address of .rela * section. * cur_offset[r7] = rela.run[r9] - rela.link [r7] -* _stext.link[r10] = _stext.run[r10] - cur_offset[r7] -* final_offset[r3] = _stext.final[r3] - _stext.link[r10] +* _stext.link[r12] = _stext.run[r10] - cur_offset[r7] +* final_offset[r3] = _stext.final[r3] - _stext.link[r12] */ subfr7, r7, r9 /* cur_offset */ - subfr10, r7, r10 - subfr3, r10, r3 /* final_offset */ + subfr12, r7, r10 + subfr3, r12, r3 /* final_offset */ subfr8, r6, r8 /* relaz -= relaent */ /* * Scan through the .rela table and process each entry * r9 - points to the current .rela table entry * r13 - points to the symbol table +* r10 - holds the runtime address of _stext */ /* @@ -180,12 +180,23 @@ store_half: nxtrela: cmpwi r8, 0 /* relasz = 0 ? */ - ble done + ble flush add r9, r9, r6 /* move to next entry in the .rela table */ subf r8, r6, r8 /* relasz -= relaent */ b applyrela -done: blr + /* Flush the d-cache'd instructions */ +flush: + mr r3, r10 + lis r4, (_end - _stext)@h + ori r4, r4, (_end - _stext)@l + add r4, r3, r4 + bl clean_dcache_range + /* Invalidate the i-cache */ + bl flush_instruction_cache +done: + mtlrr14 + blr p_dyn: .long __dynamic_start - 0b Thanks Suzuki ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Export PIR data through sysfs
On Tue, Nov 08, 2011 at 10:59:46AM -0600, Scott Wood wrote: > On 11/08/2011 12:58 AM, Ananth N Mavinakayanahalli wrote: > > On Mon, Nov 07, 2011 at 11:18:32AM -0600, Scott Wood wrote: > >> What use does userspace have for this? If you want to return the > >> currently executing CPU (which unless you're pinned could change as soon > >> as the value is read...), why not just return smp_processor_id() or > >> hard_smp_processor_id()? > > > > Its not just the current cpu. Decoding PIR can tell you the core id, > > thread id in case of SMT, and this information can be used by userspace > > apps to set affinities, etc. > > Wouldn't it make more sense to expose the thread to core mappings in a > general way, not tied to hardware or what thread we're currently running on? AFAIK, the information encoding in PIR is platform dependent. There is no general way to expose this information unless you want have a per-platform ifdef. Even then, I am not sure if that information will generally be available or provided. > What's the use case for knowing this information only about the current > thread (or rather the state the current thread was in a few moments ago)? Its not information about the thread but about the cpu. Unless you have a shared LPAR environment, the data will be consistent and can be used by applications with knowledge of the platform. > > +#if defined(CONFIG_SMP) && defined(SPRN_PIR) > > +SYSFS_PMCSETUP(pir, SPRN_PIR); > > +static SYSDEV_ATTR(pir, 0400, show_pir, NULL); > > +#endif > > This only helps on architectures such as 8xx where you can't build as > SMP -- and I don't think #ifdef SPRN_PIR excludes any builds. > > It doesn't help on chips like 750 or e300 where you can run a normal 6xx > SMP build, you just won't have multiple CPUs, and thus won't run things > like the secondary entry code. Ugh! Booke builds seem to be fun :-) I think this calls for a CPU_FTR_PIR. What do you suggest? Ananth ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/2] PS3 fixes for 3.2
On Tue, 2011-11-08 at 17:51 -0800, Geoff Levand wrote: > git://git.kernel.org/pub/scm/linux/kernel/git/geoff/ps3-linux.git Thanks. Will do after he pulls my current batch. Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 2/2] powerpc/ps3: Fix SMP lockdep boot warning
Move the PS3 IPI message setup from ps3_smp_setup_cpu() to ps3_smp_probe().
Fixes startup warnings like these:
[ cut here ]
WARNING: at kernel/lockdep.c:2649
Modules linked in:
...
---[ end trace 31fd0ba7d8756001 ]---
Signed-off-by: Geoff Levand
---
arch/powerpc/platforms/ps3/smp.c | 64 +++---
1 files changed, 32 insertions(+), 32 deletions(-)
diff --git a/arch/powerpc/platforms/ps3/smp.c b/arch/powerpc/platforms/ps3/smp.c
index f609345..efc1cd8 100644
--- a/arch/powerpc/platforms/ps3/smp.c
+++ b/arch/powerpc/platforms/ps3/smp.c
@@ -59,48 +59,49 @@ static void ps3_smp_message_pass(int cpu, int msg)
static int ps3_smp_probe(void)
{
- return 2;
-}
+ int cpu;
-static void __init ps3_smp_setup_cpu(int cpu)
-{
- int result;
- unsigned int *virqs = per_cpu(ps3_ipi_virqs, cpu);
- int i;
+ for (cpu = 0; cpu < 2; cpu++) {
+ int result;
+ unsigned int *virqs = per_cpu(ps3_ipi_virqs, cpu);
+ int i;
- DBG(" -> %s:%d: (%d)\n", __func__, __LINE__, cpu);
+ DBG(" -> %s:%d: (%d)\n", __func__, __LINE__, cpu);
- /*
-* Check assumptions on ps3_ipi_virqs[] indexing. If this
-* check fails, then a different mapping of PPC_MSG_
-* to index needs to be setup.
-*/
+ /*
+ * Check assumptions on ps3_ipi_virqs[] indexing. If this
+ * check fails, then a different mapping of PPC_MSG_
+ * to index needs to be setup.
+ */
- BUILD_BUG_ON(PPC_MSG_CALL_FUNCTION!= 0);
- BUILD_BUG_ON(PPC_MSG_RESCHEDULE != 1);
- BUILD_BUG_ON(PPC_MSG_CALL_FUNC_SINGLE != 2);
- BUILD_BUG_ON(PPC_MSG_DEBUGGER_BREAK != 3);
+ BUILD_BUG_ON(PPC_MSG_CALL_FUNCTION!= 0);
+ BUILD_BUG_ON(PPC_MSG_RESCHEDULE != 1);
+ BUILD_BUG_ON(PPC_MSG_CALL_FUNC_SINGLE != 2);
+ BUILD_BUG_ON(PPC_MSG_DEBUGGER_BREAK != 3);
- for (i = 0; i < MSG_COUNT; i++) {
- result = ps3_event_receive_port_setup(cpu, &virqs[i]);
+ for (i = 0; i < MSG_COUNT; i++) {
+ result = ps3_event_receive_port_setup(cpu, &virqs[i]);
- if (result)
- continue;
+ if (result)
+ continue;
- DBG("%s:%d: (%d, %d) => virq %u\n",
- __func__, __LINE__, cpu, i, virqs[i]);
+ DBG("%s:%d: (%d, %d) => virq %u\n",
+ __func__, __LINE__, cpu, i, virqs[i]);
- result = smp_request_message_ipi(virqs[i], i);
+ result = smp_request_message_ipi(virqs[i], i);
- if (result)
- virqs[i] = NO_IRQ;
- else
- ps3_register_ipi_irq(cpu, virqs[i]);
- }
+ if (result)
+ virqs[i] = NO_IRQ;
+ else
+ ps3_register_ipi_irq(cpu, virqs[i]);
+ }
- ps3_register_ipi_debug_brk(cpu, virqs[PPC_MSG_DEBUGGER_BREAK]);
+ ps3_register_ipi_debug_brk(cpu, virqs[PPC_MSG_DEBUGGER_BREAK]);
- DBG(" <- %s:%d: (%d)\n", __func__, __LINE__, cpu);
+ DBG(" <- %s:%d: (%d)\n", __func__, __LINE__, cpu);
+ }
+
+ return 2;
}
void ps3_smp_cleanup_cpu(int cpu)
@@ -123,7 +124,6 @@ static struct smp_ops_t ps3_smp_ops = {
.probe = ps3_smp_probe,
.message_pass = ps3_smp_message_pass,
.kick_cpu = smp_generic_kick_cpu,
- .setup_cpu = ps3_smp_setup_cpu,
};
void smp_init_ps3(void)
--
1.7.0.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/2] powerpc/ps3: Fix lost SMP IPIs
Fixes the PS3 bootup hang introduced in 3.0-rc1 by:
commit 317f394160e9beb97d19a84c39b7e5eb3d7815a
sched: Move the second half of ttwu() to the remote cpu
Move the PS3's LV1 EOI call lv1_end_of_interrupt_ext() from ps3_chip_eoi()
to ps3_get_irq() for IPI messages.
If lv1_send_event_locally() is called between a previous call to
lv1_send_event_locally() and the coresponding call to
lv1_end_of_interrupt_ext() the second event will not be delivered to the
target cpu.
The PS3's SMP IPIs are implemented using lv1_send_event_locally(), so if two
IPI messages of the same type are sent to the same target in a relatively
short period of time the second IPI event can become lost when
lv1_end_of_interrupt_ext() is called from ps3_chip_eoi().
CC: sta...@kernel.org
Signed-off-by: Geoff Levand
---
arch/powerpc/platforms/ps3/interrupt.c | 23 ++-
arch/powerpc/platforms/ps3/platform.h |1 +
arch/powerpc/platforms/ps3/smp.c |2 ++
3 files changed, 25 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/platforms/ps3/interrupt.c
b/arch/powerpc/platforms/ps3/interrupt.c
index 404bc52..1d6f4f4 100644
--- a/arch/powerpc/platforms/ps3/interrupt.c
+++ b/arch/powerpc/platforms/ps3/interrupt.c
@@ -88,6 +88,7 @@ struct ps3_private {
struct ps3_bmp bmp __attribute__ ((aligned (PS3_BMP_MINALIGN)));
u64 ppe_id;
u64 thread_id;
+ unsigned long ipi_mask;
};
static DEFINE_PER_CPU(struct ps3_private, ps3_private);
@@ -144,7 +145,11 @@ static void ps3_chip_unmask(struct irq_data *d)
static void ps3_chip_eoi(struct irq_data *d)
{
const struct ps3_private *pd = irq_data_get_irq_chip_data(d);
- lv1_end_of_interrupt_ext(pd->ppe_id, pd->thread_id, d->irq);
+
+ /* non-IPIs are EOIed here. */
+
+ if (!test_bit(63 - d->irq, &pd->ipi_mask))
+ lv1_end_of_interrupt_ext(pd->ppe_id, pd->thread_id, d->irq);
}
/**
@@ -691,6 +696,16 @@ void __init ps3_register_ipi_debug_brk(unsigned int cpu,
unsigned int virq)
cpu, virq, pd->bmp.ipi_debug_brk_mask);
}
+void __init ps3_register_ipi_irq(unsigned int cpu, unsigned int virq)
+{
+ struct ps3_private *pd = &per_cpu(ps3_private, cpu);
+
+ set_bit(63 - virq, &pd->ipi_mask);
+
+ DBG("%s:%d: cpu %u, virq %u, ipi_mask %lxh\n", __func__, __LINE__,
+ cpu, virq, pd->ipi_mask);
+}
+
static unsigned int ps3_get_irq(void)
{
struct ps3_private *pd = &__get_cpu_var(ps3_private);
@@ -720,6 +735,12 @@ static unsigned int ps3_get_irq(void)
BUG();
}
#endif
+
+ /* IPIs are EOIed here. */
+
+ if (test_bit(63 - plug, &pd->ipi_mask))
+ lv1_end_of_interrupt_ext(pd->ppe_id, pd->thread_id, plug);
+
return plug;
}
diff --git a/arch/powerpc/platforms/ps3/platform.h
b/arch/powerpc/platforms/ps3/platform.h
index 9a196a8..1a633ed 100644
--- a/arch/powerpc/platforms/ps3/platform.h
+++ b/arch/powerpc/platforms/ps3/platform.h
@@ -43,6 +43,7 @@ void ps3_mm_shutdown(void);
void ps3_init_IRQ(void);
void ps3_shutdown_IRQ(int cpu);
void __init ps3_register_ipi_debug_brk(unsigned int cpu, unsigned int virq);
+void __init ps3_register_ipi_irq(unsigned int cpu, unsigned int virq);
/* smp */
diff --git a/arch/powerpc/platforms/ps3/smp.c b/arch/powerpc/platforms/ps3/smp.c
index 4c44794..f609345 100644
--- a/arch/powerpc/platforms/ps3/smp.c
+++ b/arch/powerpc/platforms/ps3/smp.c
@@ -94,6 +94,8 @@ static void __init ps3_smp_setup_cpu(int cpu)
if (result)
virqs[i] = NO_IRQ;
+ else
+ ps3_register_ipi_irq(cpu, virqs[i]);
}
ps3_register_ipi_debug_brk(cpu, virqs[PPC_MSG_DEBUGGER_BREAK]);
--
1.7.0.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 0/2] PS3 fixes for 3.2
Ben, Here are two fixes for PS3. Please apply for 3.2. -Geoff The following changes are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/geoff/ps3-linux.git for-powerpc Geoff Levand (2): powerpc/ps3: Fix lost SMP IPIs powerpc/ps3: Fix SMP lockdep boot warning arch/powerpc/platforms/ps3/interrupt.c | 23 +++- arch/powerpc/platforms/ps3/platform.h |1 + arch/powerpc/platforms/ps3/smp.c | 62 --- 3 files changed, 55 insertions(+), 31 deletions(-) ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v3 3/3] Use separate struct console structure for each hvc_console.
It is possible to make any virtio_console port be a console
by sending VIRITO_CONSOLE_CONSOLE_PORT. But hvc_alloc was
using a single struct console hvc_console, which contains
both an index and flags which are per-port.
This adds a separate struct console for each virtio_console
that is CONSOLE_PORT.
Signed-off-by: Miche Baker-Harvey
---
drivers/tty/hvc/hvc_console.c | 20
drivers/tty/hvc/hvc_console.h |1 +
2 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index 09a6159..24a84d6 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -247,6 +247,7 @@ static void destroy_hvc_struct(struct kref *kref)
spin_unlock(&hvc_structs_lock);
+ kfree(hp->hvc_console);
kfree(hp);
}
@@ -827,6 +828,7 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
int outbuf_size)
{
struct hvc_struct *hp;
+ struct console *cp;
int i;
/* We wait until a driver actually comes along */
@@ -854,6 +856,17 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
kref_init(&hp->kref);
INIT_WORK(&hp->tty_resize, hvc_set_winsz);
+ /*
+* Make each console its own struct console.
+*/
+ cp = kmemdup(&hvc_console, sizeof(*cp), GFP_KERNEL);
+ if (!cp) {
+ kfree(hp);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ hp->hvc_console = cp;
+
spin_lock_init(&hp->lock);
spin_lock(&hvc_structs_lock);
@@ -872,8 +885,13 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
hp->index = i;
+ cp->index = i;
+ vtermnos[i] = vtermno;
+ cons_ops[i] = ops;
+
list_add_tail(&(hp->next), &hvc_structs);
spin_unlock(&hvc_structs_lock);
+ register_console(cp);
return hp;
}
@@ -884,6 +902,8 @@ int hvc_remove(struct hvc_struct *hp)
unsigned long flags;
struct tty_struct *tty;
+ BUG_ON(!hp->hvc_console);
+ unregister_console(hp->hvc_console);
spin_lock_irqsave(&hp->lock, flags);
tty = tty_kref_get(hp->tty);
diff --git a/drivers/tty/hvc/hvc_console.h b/drivers/tty/hvc/hvc_console.h
index c335a14..2d20ab7 100644
--- a/drivers/tty/hvc/hvc_console.h
+++ b/drivers/tty/hvc/hvc_console.h
@@ -58,6 +58,7 @@ struct hvc_struct {
const struct hv_ops *ops;
int irq_requested;
int data;
+ struct console *hvc_console;
struct winsize ws;
struct work_struct tty_resize;
struct list_head next;
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH RFC v3 0/3] Support multiple VirtioConsoles.
(Amit pointed out that the patches never went out. This was a resend of the series meant to go out on 11/2/2011; Now it's a resend of the mail this morning, with everyone copied on the same mail. So sorry for the spam! This is v3.) This patchset applies to linux-next/next-2002. This series implements support for multiple virtio_consoles using KVM. This patchset addresses several issues associated with trying to establish multiple virtio consoles. I'm trying to start a guest via KVM that supports multiple virtual consoles, with getty's on each, and with some being console devices. These patches let me establish more than one VirtioConsole (I'm running eight at the moment), and enable console output appearing on one of them. It still doesn't successfully generate console output on multiple VirtioConsoles. Let me apologise for my last patch having gotten into Linus' tree, and leaving other people to deal with crashes. I had meant to be asking for guidance, but I didn't mark it as "RFC". This series reflects the input from Konrad Rzeszutek, Amit Shah, Stephen Boyd, and Rusty Russell. I think we do have to limit hvc_alloc() to one thread. I would appreciate any comments or feedback, or accept if appropriate. Thanks, Miche Baker-Harvey --- Miche Baker-Harvey (3): virtio_console: Fix locking of vtermno. hvc_init(): Enforce one-time initialization. Use separate struct console structure for each hvc_console. drivers/char/virtio_console.c |9 ++--- drivers/tty/hvc/hvc_console.c | 33 +++-- drivers/tty/hvc/hvc_console.h |1 + 3 files changed, 38 insertions(+), 5 deletions(-) -- ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v3 1/3] virtio_console: Fix locking of vtermno.
Some modifications of vtermno were not done under the spinlock.
Moved assignment from vtermno and increment of vtermno together,
putting both under the spinlock. Revert vtermno on failure.
Signed-off-by: Miche Baker-Harvey
---
drivers/char/virtio_console.c |9 ++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 8e3c46d..9722e76 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -987,18 +987,21 @@ int init_port_console(struct port *port)
* pointers. The final argument is the output buffer size: we
* can do any size, so we put PAGE_SIZE here.
*/
- port->cons.vtermno = pdrvdata.next_vtermno;
+ spin_lock_irq(&pdrvdata_lock);
+ port->cons.vtermno = pdrvdata.next_vtermno++;
+ spin_unlock_irq(&pdrvdata_lock);
port->cons.hvc = hvc_alloc(port->cons.vtermno, 0, &hv_ops, PAGE_SIZE);
+ spin_lock_irq(&pdrvdata_lock);
if (IS_ERR(port->cons.hvc)) {
ret = PTR_ERR(port->cons.hvc);
dev_err(port->dev,
"error %d allocating hvc for port\n", ret);
port->cons.hvc = NULL;
+ port->cons.vtermno = pdrvdata.next_vtermno--;
+ spin_unlock_irq(&pdrvdata_lock);
return ret;
}
- spin_lock_irq(&pdrvdata_lock);
- pdrvdata.next_vtermno++;
list_add_tail(&port->cons.list, &pdrvdata.consoles);
spin_unlock_irq(&pdrvdata_lock);
port->guest_connected = true;
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v3 2/3] hvc_init(): Enforce one-time initialization.
hvc_init() must only be called once, and no thread should continue with
hvc_alloc()
until after initialization is complete. The original code does not enforce
either
of these requirements. A new mutex limits entry to hvc_init() to a single
thread,
and blocks all later comers until it has completed.
This patch fixes multiple crash symptoms.
Signed-off-by: Miche Baker-Harvey
---
drivers/tty/hvc/hvc_console.c | 13 +++--
1 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index b6b2d18..09a6159 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -29,8 +29,9 @@
#include
#include
#include
-#include
#include
+#include
+#include
#include
#include
#include
@@ -84,6 +85,10 @@ static LIST_HEAD(hvc_structs);
* list traversal.
*/
static DEFINE_SPINLOCK(hvc_structs_lock);
+/*
+ * only one task does allocation at a time.
+ */
+static DEFINE_MUTEX(hvc_ports_mutex);
/*
* This value is used to assign a tty->index value to a hvc_struct based
@@ -825,11 +830,15 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
int i;
/* We wait until a driver actually comes along */
+ mutex_lock(&hvc_ports_mutex);
if (!hvc_driver) {
int err = hvc_init();
- if (err)
+ if (err) {
+ mutex_unlock(&hvc_ports_mutex);
return ERR_PTR(err);
+ }
}
+ mutex_unlock(&hvc_ports_mutex);
hp = kzalloc(ALIGN(sizeof(*hp), sizeof(long)) + outbuf_size,
GFP_KERNEL);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/7] powerpc/85xx: add HOTPLUG_CPU support
On 11/08/2011 04:05 AM, Li Yang-R58472 wrote:
>> Subject: Re: [PATCH 2/7] powerpc/85xx: add HOTPLUG_CPU support
>>
>> On 11/04/2011 07:31 AM, Zhao Chenhui wrote:
>>> +static int is_corenet;
>>> +static void __cpuinit smp_85xx_setup_cpu(int cpu_nr);
>>> +
>>> +#if defined(CONFIG_HOTPLUG_CPU) && defined(CONFIG_PPC32)
>>
>> Why PPC32?
>
> Not sure if it is the same for e5500. E5500 support will be verified later.
It's better to have 64-bit silently do nothing here?
>>> + flush_disable_L1();
>>
>> You'll also need to take down L2 on e500mc.
>
> Right. E500mc support is beyond this patch series. We will work on it after
> the e500v2 support is finalized.
I saw some support with "is_corenet"... If we don't support e500mc,
make sure the code doesn't try to run on e500mc.
This isn't an e500v2-only BSP you're putting the code into. :-)
>>> + tmp = 0;
>>> + if (cpu_has_feature(CPU_FTR_CAN_NAP))
>>> + tmp = HID0_NAP;
>>> + else if (cpu_has_feature(CPU_FTR_CAN_DOZE))
>>> + tmp = HID0_DOZE;
>>
>> Those FTR bits are for what we can do in idle, and can be cleared if the
>> user sets CONFIG_BDI_SWITCH.
>
> It is powersave_nap variable shows what we can do in idle.
No, that shows what the user wants to do in idle.
> I think it's correct to use the CPU_FTR_CAN_* macro as CPU capability.
This is 85xx-specific code. We always can, and want to, nap here
(though the way we enter nap will be different on e500mc and up) -- even
if it's broken to use it for idle (such as on p4080, which would miss
doorbells as wake events).
>>> +static void __cpuinit smp_85xx_reset_core(int nr)
>>> +{
>>> + __iomem u32 *vaddr, *pir_vaddr;
>>> + u32 val, cpu_mask;
>>> +
>>> + /* If CoreNet platform, use BRR as release register. */
>>> + if (is_corenet) {
>>> + cpu_mask = 1 << nr;
>>> + vaddr = ioremap(get_immrbase() + MPC85xx_BRR_OFF, 4);
>>> + } else {
>>> + cpu_mask = 1 << (24 + nr);
>>> + vaddr = ioremap(get_immrbase() + MPC85xx_ECM_EEBPCR_OFF, 4);
>>> + }
>>
>> Please use the device tree node, not get_immrbase().
>
> The get_immrbase() implementation uses the device tree node.
I mean the guts node. get_immrbase() should be avoided where possible.
Also, some people might care about latency to enter/exit deep sleep.
Searching through the device tree at this point (rather than on init)
slows that down.
>>> +static int __cpuinit smp_85xx_map_bootpg(u32 page)
>>> +{
>>> + __iomem u32 *bootpg_ptr;
>>> + u32 bptr;
>>> +
>>> + /* Get the BPTR */
>>> + bootpg_ptr = ioremap(get_immrbase() + MPC85xx_BPTR_OFF, 4);
>>> +
>>> + /* Set the BPTR to the secondary boot page */
>>> + bptr = MPC85xx_BPTR_EN | (page & MPC85xx_BPTR_BOOT_PAGE_MASK);
>>> + out_be32(bootpg_ptr, bptr);
>>> +
>>> + iounmap(bootpg_ptr);
>>> + return 0;
>>> +}
>>
>> Shouldn't the boot page already be set by U-Boot?
>
> The register should be lost after a deep sleep. Better to do it again.
How can it be lost after a deep sleep if we're relying on it to hold our
wakeup code?
It's not "better to do it again" if we're making a bad assumption about
the code and the table being in the same page.
>>> local_irq_save(flags);
>>>
>>> - out_be32(bptr_vaddr + BOOT_ENTRY_PIR, nr);
>>> + out_be32(&epapr->pir, hw_cpu);
>>> #ifdef CONFIG_PPC32
>>> - out_be32(bptr_vaddr + BOOT_ENTRY_ADDR_LOWER, __pa(__early_start));
>>> +#ifdef CONFIG_HOTPLUG_CPU
>>> + if (system_state == SYSTEM_RUNNING) {
>>> + out_be32(&epapr->addr_l, 0);
>>> + smp_85xx_map_bootpg((u32)(*cpu_rel_addr >> PAGE_SHIFT));
>>
>> Why is this inside PPC32?
>
> Not tested on 64-bit yet. Might require a different implementation on PPC64.
Please make a reasonable effort to do things in a way that works on
both. It shouldn't be a big deal to test that it doesn't break booting
on p5020.
>>> if (!ioremappable)
>>> - flush_dcache_range((ulong)bptr_vaddr,
>>> - (ulong)(bptr_vaddr + SIZE_BOOT_ENTRY));
>>> + flush_dcache_range((ulong)epapr,
>>> + (ulong)epapr + sizeof(struct epapr_entry));
>>
>> We don't wait for the core to come up on 64-bit?
>
> Not sure about it. But at least the normal boot up should be tested on
> P5020, right?
Well, that's a special case in that it only has one secondary. :-)
Or we could be getting lucky with timing. It's not a new issue with
this patch, it just looks odd.
-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 5/7] fsl_pmc: update device bindings
On 11/08/2011 04:56 AM, Li Yang-R58472 wrote: >>> diff --git a/Documentation/devicetree/bindings/powerpc/fsl/pmc.txt >> b/Documentation/devicetree/bindings/powerpc/fsl/pmc.txt >>> index 07256b7..d84b4f8 100644 >>> --- a/Documentation/devicetree/bindings/powerpc/fsl/pmc.txt >>> +++ b/Documentation/devicetree/bindings/powerpc/fsl/pmc.txt >>> @@ -9,22 +9,27 @@ Properties: >>> >>>"fsl,mpc8548-pmc" should be listed for any chip whose PMC is >>>compatible. "fsl,mpc8536-pmc" should also be listed for any chip >>> - whose PMC is compatible, and implies deep-sleep capability. >>> + whose PMC is compatible, and implies deep-sleep capability and >>> + wake on user defined packet(wakeup on ARP). >> >> Why does the PMC care? This is an ethernet controller feature, the PMC >> is just keeping the wakeup-relevant parts of the ethernet controller >> alive (whatever they happen to be). >> >> Do we have any chips that have ethernet controller support for wake on >> user-defined packet, but a sleep mode that doesn't let it be used? > > I think it is more a PMC feature cause Ethernet controller on many > SoCs have the filer feature, but only very limited SoCs can support > using it as a wake-up source. The differences should be mostly in > the PM controller block. Have you tried waking from sleep with it on those other SoCs? > Also the wake on user-defined packet feature was never mentioned in the > Ethernet controller part of UM. AFAICT all this "feature" is, is programming the Ethernet controller to filter out some packets that we don't want to wake us up, and then refraining from entering magic packet mode. What PMC registers are programmed any differently for this? It's in the PM part of the manual because that's where they generally describe issues related to PM. They describe magic packet there too. The set of devices which are still active during sleep is a different issue from the conditions on which a device will request a wake. I also don't trust our PM documentation very much. It's ambiguous just what gets shut down in ordinary sleep mode. E.g. the mpc8544 manual talks about "external interrupts", but in one place it looks like it means external to the SoC: > In sleep mode, all clocks internal to the e500 core are turned off, including > the timer facilities clock. All > I/O interfaces in the device logic are also shut down. Only the clocks to the > MPC8544E PIC are still > running so that an external interrupt can wake up the device. But the note immediately below that seems to imply they're talking about external to the core: > Only external interrupts can wake the MPC8544E from sleep mode. Internal > interrupt sources like the core interval timer or watchdog timer depend on > an active clock for their operation and these are disabled in sleep mode. >> Normally that shouldn't matter, but we already an unused binding for >> this. :-) >> >> Please provide rationale for doing it this way. Ideally it should >> probably use whatever http://devicetree.org/ClockBindings ends up being. > > I have looked into that binding. The binding was primarily defined for the > Linux clock API which is not same as what we are doing here(set wakeup > source). > If in the future the clock API also covers wakeup related features, we can > change to use it. While Linux APIs can serve as an inspiration, they're not the basis of device tree bindings. The device tree is not Linux-specific, and should not change just because Linux changes, but rather should describe the hardware. We want to get this right the first time. What we are describing here is how a device's clock is connected to the PMC. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2 3/3] Use separate struct console structure for each hvc_console.
It is possible to make any virtio_console port be a console
by sending VIRITO_CONSOLE_CONSOLE_PORT. But hvc_alloc was
using a single struct console hvc_console, which contains
both an index and flags which are per-port.
This adds a separate struct console for each virtio_console
that is CONSOLE_PORT.
Signed-off-by: Miche Baker-Harvey
---
drivers/tty/hvc/hvc_console.c | 20
drivers/tty/hvc/hvc_console.h |1 +
2 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index 09a6159..24a84d6 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -247,6 +247,7 @@ static void destroy_hvc_struct(struct kref *kref)
spin_unlock(&hvc_structs_lock);
+ kfree(hp->hvc_console);
kfree(hp);
}
@@ -827,6 +828,7 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
int outbuf_size)
{
struct hvc_struct *hp;
+ struct console *cp;
int i;
/* We wait until a driver actually comes along */
@@ -854,6 +856,17 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
kref_init(&hp->kref);
INIT_WORK(&hp->tty_resize, hvc_set_winsz);
+ /*
+* Make each console its own struct console.
+*/
+ cp = kmemdup(&hvc_console, sizeof(*cp), GFP_KERNEL);
+ if (!cp) {
+ kfree(hp);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ hp->hvc_console = cp;
+
spin_lock_init(&hp->lock);
spin_lock(&hvc_structs_lock);
@@ -872,8 +885,13 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
hp->index = i;
+ cp->index = i;
+ vtermnos[i] = vtermno;
+ cons_ops[i] = ops;
+
list_add_tail(&(hp->next), &hvc_structs);
spin_unlock(&hvc_structs_lock);
+ register_console(cp);
return hp;
}
@@ -884,6 +902,8 @@ int hvc_remove(struct hvc_struct *hp)
unsigned long flags;
struct tty_struct *tty;
+ BUG_ON(!hp->hvc_console);
+ unregister_console(hp->hvc_console);
spin_lock_irqsave(&hp->lock, flags);
tty = tty_kref_get(hp->tty);
diff --git a/drivers/tty/hvc/hvc_console.h b/drivers/tty/hvc/hvc_console.h
index c335a14..2d20ab7 100644
--- a/drivers/tty/hvc/hvc_console.h
+++ b/drivers/tty/hvc/hvc_console.h
@@ -58,6 +58,7 @@ struct hvc_struct {
const struct hv_ops *ops;
int irq_requested;
int data;
+ struct console *hvc_console;
struct winsize ws;
struct work_struct tty_resize;
struct list_head next;
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2 2/3] hvc_init(): Enforce one-time initialization.
hvc_init() must only be called once, and no thread should continue with
hvc_alloc()
until after initialization is complete. The original code does not enforce
either
of these requirements. A new mutex limits entry to hvc_init() to a single
thread,
and blocks all later comers until it has completed.
This patch fixes multiple crash symptoms.
Signed-off-by: Miche Baker-Harvey
---
drivers/tty/hvc/hvc_console.c | 13 +++--
1 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index b6b2d18..09a6159 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -29,8 +29,9 @@
#include
#include
#include
-#include
#include
+#include
+#include
#include
#include
#include
@@ -84,6 +85,10 @@ static LIST_HEAD(hvc_structs);
* list traversal.
*/
static DEFINE_SPINLOCK(hvc_structs_lock);
+/*
+ * only one task does allocation at a time.
+ */
+static DEFINE_MUTEX(hvc_ports_mutex);
/*
* This value is used to assign a tty->index value to a hvc_struct based
@@ -825,11 +830,15 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
int i;
/* We wait until a driver actually comes along */
+ mutex_lock(&hvc_ports_mutex);
if (!hvc_driver) {
int err = hvc_init();
- if (err)
+ if (err) {
+ mutex_unlock(&hvc_ports_mutex);
return ERR_PTR(err);
+ }
}
+ mutex_unlock(&hvc_ports_mutex);
hp = kzalloc(ALIGN(sizeof(*hp), sizeof(long)) + outbuf_size,
GFP_KERNEL);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2 1/3] virtio_console: Fix locking of vtermno.
Some modifications of vtermno were not done under the spinlock.
Moved assignment from vtermno and increment of vtermno together,
putting both under the spinlock. Revert vtermno on failure.
Signed-off-by: Miche Baker-Harvey
---
drivers/char/virtio_console.c |9 ++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 8e3c46d..9722e76 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -987,18 +987,21 @@ int init_port_console(struct port *port)
* pointers. The final argument is the output buffer size: we
* can do any size, so we put PAGE_SIZE here.
*/
- port->cons.vtermno = pdrvdata.next_vtermno;
+ spin_lock_irq(&pdrvdata_lock);
+ port->cons.vtermno = pdrvdata.next_vtermno++;
+ spin_unlock_irq(&pdrvdata_lock);
port->cons.hvc = hvc_alloc(port->cons.vtermno, 0, &hv_ops, PAGE_SIZE);
+ spin_lock_irq(&pdrvdata_lock);
if (IS_ERR(port->cons.hvc)) {
ret = PTR_ERR(port->cons.hvc);
dev_err(port->dev,
"error %d allocating hvc for port\n", ret);
port->cons.hvc = NULL;
+ port->cons.vtermno = pdrvdata.next_vtermno--;
+ spin_unlock_irq(&pdrvdata_lock);
return ret;
}
- spin_lock_irq(&pdrvdata_lock);
- pdrvdata.next_vtermno++;
list_add_tail(&port->cons.list, &pdrvdata.consoles);
spin_unlock_irq(&pdrvdata_lock);
port->guest_connected = true;
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH RFC v2 0/3] Support multiple VirtioConsoles.
(Amit pointed out that the patches never went out. This is a resend of the series meant to go out on 11/2/2011; I've marked it "v2".) This patchset applies to linux-next/next-2002. This series implements support for multiple virtio_consoles using KVM. This patchset addresses several issues associated with trying to establish multiple virtio consoles. I'm trying to start a guest via KVM that supports multiple virtual consoles, with getty's on each, and with some being console devices. These patches let me establish more than one VirtioConsole (I'm running eight at the moment), and enable console output appearing on one of them. It still doesn't successfully generate console output on multiple VirtioConsoles. Let me apologise for my last patch having gotten into Linus' tree, and leaving other people to deal with crashes. I had meant to be asking for guidance, but I didn't mark it as "RFC". This series reflects the input from Konrad Rzeszutek, Amit Shah, Stephen Boyd, and Rusty Russell. I think we do have to limit hvc_alloc() to one thread. I would appreciate any comments or feedback, or accept if appropriate. Thanks, Miche Baker-Harvey --- Miche Baker-Harvey (3): virtio_console: Fix locking of vtermno. hvc_init(): Enforce one-time initialization. Use separate struct console structure for each hvc_console. drivers/char/virtio_console.c |9 ++--- drivers/tty/hvc/hvc_console.c | 33 +++-- drivers/tty/hvc/hvc_console.h |1 + 3 files changed, 38 insertions(+), 5 deletions(-) -- ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/7] powerpc/85xx: add sleep and deep sleep support
On 11/08/2011 04:32 AM, Li Yang-R58472 wrote:
>> Subject: Re: [PATCH 3/7] powerpc/85xx: add sleep and deep sleep support
>>
>> On 11/04/2011 07:33 AM, Zhao Chenhui wrote:
>>> static int pmc_suspend_enter(suspend_state_t state)
>>> {
>>> int ret;
>>> + u32 powmgtreq = 0x0050;
>>
>> Where does this 0x0050 come from? Please symbolically define
>> individual bits.
>>
>> The comment in the asm code says it should be 0x0010, BTW.
>
> I think we should get rid of the powmgtreq argument, as we don't support
> multiple modes for mpc85xx_enter_deep_sleep() now.
Doesn't that happen later in the patch series?
-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/7] powerpc/85xx: re-enable timebase sync disabled by KEXEC patch
On 11/08/2011 03:06 AM, Li Yang-R58472 wrote:
>
>
>> -Original Message-
>> From: linuxppc-dev-bounces+leoli=freescale@lists.ozlabs.org
>> [mailto:linuxppc-dev-bounces+leoli=freescale@lists.ozlabs.org] On
>> Behalf Of Scott Wood
>> Sent: Saturday, November 05, 2011 1:34 AM
>> To: Zhao Chenhui-B35336
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Subject: Re: [PATCH 1/7] powerpc/85xx: re-enable timebase sync disabled by
>> KEXEC patch
>>
>> On 11/04/2011 07:29 AM, Zhao Chenhui wrote:
>>> From: Li Yang
>>>
>>> The timebase sync is not only necessary when using KEXEC. It should also
>>> be used by normal boot up and cpu hotplug. Remove the ifdef added by
>>> the KEXEC patch.
>>
>> The KEXEC patch didn't just add the ifdef, it also added the initializers:
>
> Yes. But the code suggests that the timebase synchronization is only
> necessary for KEXEC, but it turns out that sleep/wakeup also need it. Maybe
> the description of the patch need to be changed as KEXEC is not to be blamed.
It is needed when you hard reset a core. This was something we never
did on SMP before kexec. Now you're adding a second thing that does it,
so it'll need the sync as well, but that doesn't mean we should do it on
normal boot.
>>> @@ -105,8 +107,64 @@ smp_85xx_setup_cpu(int cpu_nr)
>>>
>>> struct smp_ops_t smp_85xx_ops = {
>>> .kick_cpu = smp_85xx_kick_cpu,
>>> +#ifdef CONFIG_KEXEC
>>> + .give_timebase = smp_generic_give_timebase,
>>> + .take_timebase = smp_generic_take_timebase,
>>> +#endif
>>> };
>>
>> U-Boot synchronizes the timebase on 85xx. With what chip and U-Boot
>> version are you seeing this not happen?
>
> I'm curious why don't we make it happen in kernel as we are against
> adding dependency to the bootloader?
We are against adding gratuitous dependencies on the bootloader, but
some things are just a lot easier to do in that context. Nobody
complains about Linux expecting RAM to be working on entry. :-)
While it's certainly possible to do this in Linux (and should be done
the way U-Boot does instead of the software sync, in the cases where we
need to), it's easier to do in U-Boot, before the cores are running.
It would be impossible for Linux to do this (or any other tb
modifications) when running on top of a hypervisor.
In http://lists.ozlabs.org/pipermail/linuxppc-dev/2011-June/091321.html,
Ben Herrenschmidt said, "smp-tbsync.c is and has always been a
'workaround' for broken HW."
> Other architectures don't have this dependency,
Which "other architectures" are you referring to?
On PPC server this is handled with a firmware call to freeze the
timebase. On x86 this is handled by the BIOS by the time the OS starts.
-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] KVM: PPC: protect use of kvmppc_h_pr
kvmppc_h_pr is only available if CONFIG_KVM_BOOK3S_64_PR.
Signed-off-by: Andreas Schwab
---
arch/powerpc/kvm/book3s_pr.c |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index bc4d50d..05473b5 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -660,10 +660,12 @@ program_interrupt:
ulong cmd = kvmppc_get_gpr(vcpu, 3);
int i;
+#ifdef CONFIG_KVM_BOOK3S_64_PR
if (kvmppc_h_pr(vcpu, cmd) == EMULATE_DONE) {
r = RESUME_GUEST;
break;
}
+#endif
run->papr_hcall.nr = cmd;
for (i = 0; i < 9; ++i) {
--
1.7.7.2
--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] KVM: PPC: move compute_tlbie_rb to book3s_64 common header
compute_tlbie_rb is only used on ppc64 and cannot be compiled on ppc32.
Signed-off-by: Andreas Schwab
---
arch/powerpc/include/asm/kvm_book3s.h| 33 --
arch/powerpc/include/asm/kvm_book3s_64.h | 33 ++
2 files changed, 33 insertions(+), 33 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_book3s.h
b/arch/powerpc/include/asm/kvm_book3s.h
index a384ffd..db73fa3 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -383,39 +383,6 @@ static inline bool kvmppc_critical_section(struct kvm_vcpu
*vcpu)
}
#endif
-static inline unsigned long compute_tlbie_rb(unsigned long v, unsigned long r,
-unsigned long pte_index)
-{
- unsigned long rb, va_low;
-
- rb = (v & ~0x7fUL) << 16; /* AVA field */
- va_low = pte_index >> 3;
- if (v & HPTE_V_SECONDARY)
- va_low = ~va_low;
- /* xor vsid from AVA */
- if (!(v & HPTE_V_1TB_SEG))
- va_low ^= v >> 12;
- else
- va_low ^= v >> 24;
- va_low &= 0x7ff;
- if (v & HPTE_V_LARGE) {
- rb |= 1;/* L field */
- if (cpu_has_feature(CPU_FTR_ARCH_206) &&
- (r & 0xff000)) {
- /* non-16MB large page, must be 64k */
- /* (masks depend on page size) */
- rb |= 0x1000; /* page encoding in LP field */
- rb |= (va_low & 0x7f) << 16; /* 7b of VA in AVA/LP
field */
- rb |= (va_low & 0xfe); /* AVAL field (P7 doesn't seem
to care) */
- }
- } else {
- /* 4kB page */
- rb |= (va_low & 0x7ff) << 12; /* remaining 11b of VA */
- }
- rb |= (v >> 54) & 0x300;/* B field */
- return rb;
-}
-
/* Magic register values loaded into r3 and r4 before the 'sc' assembly
* instruction for the OSI hypercalls */
#define OSI_SC_MAGIC_R30x113724FA
diff --git a/arch/powerpc/include/asm/kvm_book3s_64.h
b/arch/powerpc/include/asm/kvm_book3s_64.h
index e43fe42..d0ac94f 100644
--- a/arch/powerpc/include/asm/kvm_book3s_64.h
+++ b/arch/powerpc/include/asm/kvm_book3s_64.h
@@ -29,4 +29,37 @@ static inline struct kvmppc_book3s_shadow_vcpu
*to_svcpu(struct kvm_vcpu *vcpu)
#define SPAPR_TCE_SHIFT12
+static inline unsigned long compute_tlbie_rb(unsigned long v, unsigned long r,
+unsigned long pte_index)
+{
+ unsigned long rb, va_low;
+
+ rb = (v & ~0x7fUL) << 16; /* AVA field */
+ va_low = pte_index >> 3;
+ if (v & HPTE_V_SECONDARY)
+ va_low = ~va_low;
+ /* xor vsid from AVA */
+ if (!(v & HPTE_V_1TB_SEG))
+ va_low ^= v >> 12;
+ else
+ va_low ^= v >> 24;
+ va_low &= 0x7ff;
+ if (v & HPTE_V_LARGE) {
+ rb |= 1;/* L field */
+ if (cpu_has_feature(CPU_FTR_ARCH_206) &&
+ (r & 0xff000)) {
+ /* non-16MB large page, must be 64k */
+ /* (masks depend on page size) */
+ rb |= 0x1000; /* page encoding in LP field */
+ rb |= (va_low & 0x7f) << 16; /* 7b of VA in AVA/LP
field */
+ rb |= (va_low & 0xfe); /* AVAL field (P7 doesn't seem
to care) */
+ }
+ } else {
+ /* 4kB page */
+ rb |= (va_low & 0x7ff) << 12; /* remaining 11b of VA */
+ }
+ rb |= (v >> 54) & 0x300;/* B field */
+ return rb;
+}
+
#endif /* __ASM_KVM_BOOK3S_64_H__ */
--
1.7.7.2
--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Export PIR data through sysfs
On 11/08/2011 12:58 AM, Ananth N Mavinakayanahalli wrote: > On Mon, Nov 07, 2011 at 11:18:32AM -0600, Scott Wood wrote: >> What use does userspace have for this? If you want to return the >> currently executing CPU (which unless you're pinned could change as soon >> as the value is read...), why not just return smp_processor_id() or >> hard_smp_processor_id()? > > Its not just the current cpu. Decoding PIR can tell you the core id, > thread id in case of SMT, and this information can be used by userspace > apps to set affinities, etc. Wouldn't it make more sense to expose the thread to core mappings in a general way, not tied to hardware or what thread we're currently running on? What's the use case for knowing this information only about the current thread (or rather the state the current thread was in a few moments ago)? > +#if defined(CONFIG_SMP) && defined(SPRN_PIR) > +SYSFS_PMCSETUP(pir, SPRN_PIR); > +static SYSDEV_ATTR(pir, 0400, show_pir, NULL); > +#endif This only helps on architectures such as 8xx where you can't build as SMP -- and I don't think #ifdef SPRN_PIR excludes any builds. It doesn't help on chips like 750 or e300 where you can run a normal 6xx SMP build, you just won't have multiple CPUs, and thus won't run things like the secondary entry code. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/p1023: set IRQ[4:6, 11] to high level sensitive for PCIe
On 11/07/2011 11:51 PM, Zang Roy-R61911 wrote: > > >> -Original Message- >> From: Wood Scott-B07421 >> Sent: Tuesday, November 08, 2011 2:44 AM >> To: Zang Roy-R61911 >> Cc: linuxppc-dev@lists.ozlabs.org >> Subject: Re: [PATCH] powerpc/p1023: set IRQ[4:6, 11] to high level sensitive >> for >> PCIe >> >> On 11/07/2011 02:32 AM, Roy Zang wrote: >>> P1023 external IRQ[4:6, 11] do not pin out, but the interrupts are >>> shared with PCIe controller. >>> The silicon internally ties the interrupts to L, so change the >>> IRQ[4:6,11] to high level sensitive for PCIe. >> >> Some extra commentary on why this works would be nice. > I do not know what kind of extra commentary you request. Just a note that there's magic to allow the PCIe block to output these interrupts as either active-high or active-low, depending on how they're configured at the mpic. > IRQ 4,5,6, 11 are internally tie to low by silicon. To use these interrupts > for PCIe, they need to set high level sensitive. > It is clear enough for this patch. It's odd enough that I felt the need to go reading through the docs to see why such a thing would work at all. >> The manual says: >> >>> If a PCI Express INTx interrupt is being used, then the PIC must be >>> configured >> so that external interrupts >>> are level-sensitive (EIVPRn[S] = 1). > That is true for all FSL powerpc silicon with PCIe controller beside P1023. Sure, my point was more that it didn't say anything there about how to configure EIVPRn[P]. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 1/5] [ppc] Process dynamic relocations for kernel
On Tue, 2011-11-08 at 12:41 +0530, Suzuki Poulose wrote: > What I was suggesting is, instead of flushing the cache in relocate(), lets > do it > like: > > for e.g, on 440x, (in head_44x.S :) > > #ifdef CONFIG_RELOCATABLE > ... > bl relocate > > #Flush the d-cache and invalidate the i-cache here > #endif > > > This would let the different platforms do the the cache invalidation in their > own way. > > Btw, I didn't find an instruction to flush the entire d-cache in PPC440 > manual. > We have instructions to flush only a block corresponding to an address. > > However, we have 'iccci' which would invalidate the entire i-cache which, > which > I think is better than 80,000 i-cache invalidates. In misc_32.S there are already some platform-independent cache management functions. If we use those, then relocate() could simply call them. Then the different platforms calling relocate() wouldn't have to worry about flushing/invalidating caches. For example, there's a clean_dcache_range() function. Given any range twice the size of the d-cache, it should flush the entire d-cache. But the only drawback is that it would require the caller to know the size of the d-cache. Instead, I think it would be preferable to create a new clean_dcache() (or clean_dcache_all()?) function in misc_32.S, which could call clean_dcache_range() with the appropriate args for flushing the entire d-cache. relocate() could then call the platform-independent clean_dcache(). For i-cache invalidation there's already the (incorrectly named?) flush_instruction_cache(). It uses the appropriate platform-specific methods (e.g. iccci for 44x) to invalidate the entire i-cache. Suzuki, if you agree with this direction, I could work up a new patch if needed. Josh ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] powerpc: signal32: fix sigset_t conversion when copying to user
On PPC64, put_sigset_t converts a sigset_t to a compat_sigset_t
before copying it to userspace. There is a typo in the case that
we have 4 words to copy, meaning that we corrupt the compat_sigset_t.
It appears that _NSIG_WORDS can't be greater than 2 at the moment
so this code is probably always optimised away anyway.
Signed-off-by: Will Deacon
---
arch/powerpc/kernel/signal_32.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 78b76dc..836a5a1 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -97,7 +97,7 @@ static inline int put_sigset_t(compat_sigset_t __user *uset,
sigset_t *set)
compat_sigset_t cset;
switch (_NSIG_WORDS) {
- case 4: cset.sig[5] = set->sig[3] & 0xull;
+ case 4: cset.sig[6] = set->sig[3] & 0xull;
cset.sig[7] = set->sig[3] >> 32;
case 3: cset.sig[4] = set->sig[2] & 0xull;
cset.sig[5] = set->sig[2] >> 32;
--
1.7.4.1
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 7/7] gianfar: add support for wake-on-packet
On Nov 8, 2011, at 5:20 AM, Li Yang-R58472 wrote: > > >> -Original Message- >> From: linuxppc-dev-bounces+leoli=freescale@lists.ozlabs.org >> [mailto:linuxppc-dev-bounces+leoli=freescale@lists.ozlabs.org] On >> Behalf Of Scott Wood >> Sent: Saturday, November 05, 2011 5:12 AM >> To: Zhao Chenhui-B35336 >> Cc: net...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; Fleming Andy- >> AFLEMING >> Subject: Re: [PATCH 7/7] gianfar: add support for wake-on-packet >> >> On 11/04/2011 07:40 AM, Zhao Chenhui wrote: >>> diff --git a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt >> b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt >>> index 2c6be03..543e36c 100644 >>> --- a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt >>> +++ b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt >>> @@ -56,6 +56,9 @@ Properties: >>> hardware. >>> - fsl,magic-packet : If present, indicates that the hardware supports >>> waking up via magic packet. >>> + - fsl,wake-on-filer : If present, indicates that the hardware >> supports >>> +waking up via arp request to local ip address or unicast packet to >>> +local mac address. >> >> Is there any way to determine this at runtime via the device's registers? >> >> I think TSEC_ID2[TSEC_CFG] can be used. The manual describes it >> awkwardly, but it looks like 0x20 is the bit for the filer. > > That bit only defines the filer feature but not wakeup on it. Another > solution is to get the capability from the fsl_pmc driver, but will make the > driver a lot more complex. I don't believe there is a way to know this from the controller itself. We have to use device tree for it. - k ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 7/7] gianfar: add support for wake-on-packet
>-Original Message-
>From: linuxppc-dev-bounces+leoli=freescale@lists.ozlabs.org
>[mailto:linuxppc-dev-bounces+leoli=freescale@lists.ozlabs.org] On
>Behalf Of Scott Wood
>Sent: Saturday, November 05, 2011 5:12 AM
>To: Zhao Chenhui-B35336
>Cc: net...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; Fleming Andy-
>AFLEMING
>Subject: Re: [PATCH 7/7] gianfar: add support for wake-on-packet
>
>On 11/04/2011 07:40 AM, Zhao Chenhui wrote:
>> diff --git a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
>b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
>> index 2c6be03..543e36c 100644
>> --- a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
>> +++ b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
>> @@ -56,6 +56,9 @@ Properties:
>> hardware.
>>- fsl,magic-packet : If present, indicates that the hardware supports
>> waking up via magic packet.
>> + - fsl,wake-on-filer : If present, indicates that the hardware
>supports
>> +waking up via arp request to local ip address or unicast packet to
>> +local mac address.
>
>Is there any way to determine this at runtime via the device's registers?
>
>I think TSEC_ID2[TSEC_CFG] can be used. The manual describes it
>awkwardly, but it looks like 0x20 is the bit for the filer.
That bit only defines the filer feature but not wakeup on it. Another solution
is to get the capability from the fsl_pmc driver, but will make the driver a
lot more complex.
>
>> @@ -751,7 +764,6 @@ static int gfar_of_init(struct platform_device
>*ofdev, struct net_device **pdev)
>> FSL_GIANFAR_DEV_HAS_PADDING |
>> FSL_GIANFAR_DEV_HAS_CSUM |
>> FSL_GIANFAR_DEV_HAS_VLAN |
>> -FSL_GIANFAR_DEV_HAS_MAGIC_PACKET |
>> FSL_GIANFAR_DEV_HAS_EXTENDED_HASH |
>> FSL_GIANFAR_DEV_HAS_TIMER;
>
>This is an unrelated change. Are there any eTSECs that don't support
>magic packet?
I'm not sure. But as we have the property for it in device tree, we use it.
>
>> +static int gfar_get_ip(struct net_device *dev)
>> +{
>> +struct gfar_private *priv = netdev_priv(dev);
>> +struct in_device *in_dev = (struct in_device *)dev->ip_ptr;
>> +struct in_ifaddr *ifa;
>> +
>> +if (in_dev != NULL) {
>> +ifa = (struct in_ifaddr *)in_dev->ifa_list;
>> +if (ifa != NULL) {
>> +memcpy(priv->ip_addr, &ifa->ifa_address, 4);
>> +return 0;
>> +}
>> +}
>> +return -ENOENT;
>> +}
>
>Unnecessary cast, ifa_list is already struct in_ifaddr *.
>
>Better, use for_primary_ifa(), and document that you won't wake on ARP
>packets for secondary IP addresses.
>
>> static int gfar_suspend(struct device *dev)
>> {
>> @@ -1268,9 +1443,17 @@ static int gfar_suspend(struct device *dev)
>> struct gfar __iomem *regs = priv->gfargrp[0].regs;
>> unsigned long flags;
>> u32 tempval;
>> -
>> int magic_packet = priv->wol_en &&
>> -(priv->device_flags & FSL_GIANFAR_DEV_HAS_MAGIC_PACKET);
>> +(priv->wol_opts & GIANFAR_WOL_MAGIC);
>> +int arp_packet = priv->wol_en &&
>> +(priv->wol_opts & GIANFAR_WOL_ARP);
>> +
>> +if (arp_packet) {
>> +pmc_enable_wake(priv->ofdev, PM_SUSPEND_MEM, 1);
>> +pmc_enable_lossless(1);
>> +gfar_arp_suspend(ndev);
>> +return 0;
>> +}
>
>How do we know this isn't standby?
Maybe we can remove the PM state parameter from the API.
- Leo
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 6/7] fsl_pmc: Add API to enable device as wakeup event source
>-Original Message-
>From: linuxppc-dev-bounces+leoli=freescale@lists.ozlabs.org
>[mailto:linuxppc-dev-bounces+leoli=freescale@lists.ozlabs.org] On
>Behalf Of Scott Wood
>Sent: Saturday, November 05, 2011 5:14 AM
>To: Zhao Chenhui-B35336
>Cc: net...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
>Subject: Re: [PATCH 6/7] fsl_pmc: Add API to enable device as wakeup event
>source
>
>On 11/04/2011 07:39 AM, Zhao Chenhui wrote:
>> @@ -45,6 +46,72 @@ static int has_lossless;
>> * code can be compatible with both 32-bit & 36-bit */ extern void
>> mpc85xx_enter_deep_sleep(u64 ccsrbar, u32 powmgtreq);
>>
>> +#ifdef CONFIG_FSL_PMC
>> +/**
>> + * pmc_enable_wake - enable OF device as wakeup event source
>> + * @pdev: platform device affected
>> + * @state: PM state from which device will issue wakeup events
>> + * @enable: True to enable event generation; false to disable
>> + *
>> + * This enables the device as a wakeup event source, or disables it.
>> + *
>> + * RETURN VALUE:
>> + * 0 is returned on success
>> + * -EINVAL is returned if device is not supposed to wake up the
>> +system
>> + * Error code depending on the platform is returned if both the
>> +platform and
>> + * the native mechanism fail to enable the generation of wake-up
>> +events */ int pmc_enable_wake(struct platform_device *pdev,
>> +suspend_state_t state, bool enable)
>
>"pmc" is too generic for a global function. If this can be either enable
>or disable, perhaps it should be something like mpc85xx_pmc_set_wake().
>
>> +{
>> +int ret = 0;
>> +struct device_node *clk_np;
>> +u32 *pmcdr_mask;
>> +
>> +if (!pmc_regs) {
>> +printk(KERN_WARNING "PMC is unavailable\n");
>> +return -ENOMEM;
>> +}
>
>-ENOMEM is not appropriate here, maybe -ENODEV?
>
>Should print __func__ so the user knows what's complaining.
>
>> +if (enable && !device_may_wakeup(&pdev->dev))
>> +return -EINVAL;
>> +
>> +clk_np = of_parse_phandle(pdev->dev.of_node, "clk-handle", 0);
>> +if (!clk_np)
>> +return -EINVAL;
>> +
>> +pmcdr_mask = (u32 *)of_get_property(clk_np, "fsl,pmcdr-mask", NULL);
>> +if (!pmcdr_mask) {
>> +ret = -EINVAL;
>> +goto out;
>> +}
>> +
>> +/* clear to enable clock in low power mode */
>> +if (enable)
>> +clrbits32(&pmc_regs->pmcdr, *pmcdr_mask);
>> +else
>> +setbits32(&pmc_regs->pmcdr, *pmcdr_mask);
>
>We should probably initialize PMCDR to all bits set (or at least all ones
>we know are valid) -- the default should be "not a wakeup source".
Ideally I agree with you. But currently we only have the UI of changing
wake-up source for Ethernet device. Will do when we can change all of the
devices.
>
>> +/**
>> + * pmc_enable_lossless - enable lossless ethernet in low power mode
>> + * @enable: True to enable event generation; false to disable */
>> +void pmc_enable_lossless(int enable) {
>> +if (enable && has_lossless)
>> +setbits32(&pmc_regs->pmcsr, PMCSR_LOSSLESS);
>> +else
>> +clrbits32(&pmc_regs->pmcsr, PMCSR_LOSSLESS); }
>> +EXPORT_SYMBOL_GPL(pmc_enable_lossless);
>> +#endif
>
>Won't we overwrite this later?
You are right. Will remove the code that overwrite this.
- Leo
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 7/7] gianfar: add support for wake-on-packet
>-Original Message-
>From: linuxppc-dev-bounces+leoli=freescale@lists.ozlabs.org
>[mailto:linuxppc-dev-bounces+leoli=freescale@lists.ozlabs.org] On
>Behalf Of Scott Wood
>Sent: Saturday, November 05, 2011 5:14 AM
>To: Zhao Chenhui-B35336
>Cc: net...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; Fleming Andy-
>AFLEMING
>Subject: Re: [PATCH 7/7] gianfar: add support for wake-on-packet
>
>On 11/04/2011 04:11 PM, Scott Wood wrote:
>> On 11/04/2011 07:40 AM, Zhao Chenhui wrote:
>>> static int gfar_suspend(struct device *dev) { @@ -1268,9 +1443,17
>>> @@ static int gfar_suspend(struct device *dev)
>>> struct gfar __iomem *regs = priv->gfargrp[0].regs;
>>> unsigned long flags;
>>> u32 tempval;
>>> -
>>> int magic_packet = priv->wol_en &&
>>> - (priv->device_flags & FSL_GIANFAR_DEV_HAS_MAGIC_PACKET);
>>> + (priv->wol_opts & GIANFAR_WOL_MAGIC);
>>> + int arp_packet = priv->wol_en &&
>>> + (priv->wol_opts & GIANFAR_WOL_ARP);
>>> +
>>> + if (arp_packet) {
>>> + pmc_enable_wake(priv->ofdev, PM_SUSPEND_MEM, 1);
>>> + pmc_enable_lossless(1);
>>> + gfar_arp_suspend(ndev);
>>> + return 0;
>>> + }
>>
>> How do we know this isn't standby?
>
>Or suspend to disk, for that matter?
There is nothing we can do for hibernation. The whole system is literally off.
- Leo
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH RFC 0/3] Support multiple VirtioConsoles.
On (Wed) 02 Nov 2011 [15:19:06], Miche Baker-Harvey wrote: > This patchset applies to linux-next/next-2002. Did you forget to send out the patches? I don't see anything on any of the lists nor my inbox. Amit ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 5/7] fsl_pmc: update device bindings
>-Original Message- >From: linuxppc-dev-bounces+leoli=freescale@lists.ozlabs.org >[mailto:linuxppc-dev-bounces+leoli=freescale@lists.ozlabs.org] On >Behalf Of Scott Wood >Sent: Saturday, November 05, 2011 4:05 AM >To: Zhao Chenhui-B35336 >Cc: linuxppc-dev@lists.ozlabs.org >Subject: Re: [PATCH 5/7] fsl_pmc: update device bindings > >On 11/04/2011 07:36 AM, Zhao Chenhui wrote: >> From: Li Yang >> >> Signed-off-by: Li Yang >> --- >> .../devicetree/bindings/powerpc/fsl/pmc.txt| 63 +++-- >-- >> 1 files changed, 36 insertions(+), 27 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/powerpc/fsl/pmc.txt >b/Documentation/devicetree/bindings/powerpc/fsl/pmc.txt >> index 07256b7..d84b4f8 100644 >> --- a/Documentation/devicetree/bindings/powerpc/fsl/pmc.txt >> +++ b/Documentation/devicetree/bindings/powerpc/fsl/pmc.txt >> @@ -9,22 +9,27 @@ Properties: >> >>"fsl,mpc8548-pmc" should be listed for any chip whose PMC is >>compatible. "fsl,mpc8536-pmc" should also be listed for any chip >> - whose PMC is compatible, and implies deep-sleep capability. >> + whose PMC is compatible, and implies deep-sleep capability and >> + wake on user defined packet(wakeup on ARP). > >Why does the PMC care? This is an ethernet controller feature, the PMC >is just keeping the wakeup-relevant parts of the ethernet controller >alive (whatever they happen to be). > >Do we have any chips that have ethernet controller support for wake on >user-defined packet, but a sleep mode that doesn't let it be used? I think it is more a PMC feature cause Ethernet controller on many SoCs have the filer feature, but only very limited SoCs can support using it as a wake-up source. The differences should be mostly in the PM controller block. Also the wake on user-defined packet feature was never mentioned in the Ethernet controller part of UM. > >BTW, please remove fsl,mpc8536-pmc from the p1023rds device tree -- it >was wrong before (no deep sleep, though it does appear to have jog >mode...), and is even more wrong with this provision (it has a different >ethernet controller). > >> + "fsl,p1022-pmc" should be listed for any chip whose PMC is >> + compatible, and implies lossless Ethernet capability during sleep. >> >>"fsl,mpc8641d-pmc" should be listed for any chip whose PMC is >>compatible; all statements below that apply to "fsl,mpc8548-pmc" also >>apply to "fsl,mpc8641d-pmc". >> >>Compatibility does not include bit assignments in SCCR/PMCDR/DEVDISR; >these >> - bit assignments are indicated via the sleep specifier in each >device's >> - sleep property. >> + bit assignments are indicated via the clock nodes. Device which has >a >> + controllable clock source should have a "clk-handle" property >pointing >> + to the clock node. > >Do we have any code to use this? Yes, in another patch in the series. > >Normally that shouldn't matter, but we already an unused binding for >this. :-) > >Please provide rationale for doing it this way. Ideally it should >probably use whatever http://devicetree.org/ClockBindings ends up being. I have looked into that binding. The binding was primarily defined for the Linux clock API which is not same as what we are doing here(set wakeup source). If in the future the clock API also covers wakeup related features, we can change to use it. - Leo ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 3/7] powerpc/85xx: add sleep and deep sleep support
>Subject: Re: [PATCH 3/7] powerpc/85xx: add sleep and deep sleep support
>
>On 11/04/2011 07:33 AM, Zhao Chenhui wrote:
>> +/* Cast the ccsrbar to 64-bit parameter so that the assembly
>> + * code can be compatible with both 32-bit & 36-bit */
>> +extern void mpc85xx_enter_deep_sleep(u64 ccsrbar, u32 powmgtreq);
>
>/*
> * Please use proper
> * Linux multi-line comment format.
> */
>
>> static int pmc_suspend_enter(suspend_state_t state)
>> {
>> int ret;
>> +u32 powmgtreq = 0x0050;
>
>Where does this 0x0050 come from? Please symbolically define
>individual bits.
>
>The comment in the asm code says it should be 0x0010, BTW.
I think we should get rid of the powmgtreq argument, as we don't support
multiple modes for mpc85xx_enter_deep_sleep() now.
>
>> +
>> +switch (state) {
>> +case PM_SUSPEND_MEM:
>> +#ifdef CONFIG_SPE
>> +enable_kernel_spe();
>> +#endif
>
>Should comment that currently only e500v2 hardware supports deep sleep
>-- else we'd need to save normal FP here.
>
>> +pr_debug("Entering deep sleep\n");
>> +
>> +local_irq_disable();
>> +mpc85xx_enter_deep_sleep(get_immrbase(),
>> +powmgtreq);
>> +pr_debug("Resumed from deep sleep\n");
>> +
>> +return 0;
>> +
>> +/* else fall-through */
>> +case PM_SUSPEND_STANDBY:
>
>What fall-through? You just returned.
>
>> +}
>>
>> -/* Upon resume, wait for SLP bit to be clear. */
>> -ret = spin_event_timeout((in_be32(&pmc_regs->pmcsr) & PMCSR_SLP) ==
>0,
>> - 1, 10) ? 0 : -ETIMEDOUT;
>> -if (ret)
>> -dev_err(pmc_dev, "tired waiting for SLP bit to clear\n");
>> -return ret;
>> }
>
>Remove that blank line as well.
>
>> @@ -58,13 +101,23 @@ static const struct platform_suspend_ops
>pmc_suspend_ops = {
>> .enter = pmc_suspend_enter,
>> };
>>
>> -static int pmc_probe(struct platform_device *ofdev)
>> +static int pmc_probe(struct platform_device *pdev)
>> {
>> -pmc_regs = of_iomap(ofdev->dev.of_node, 0);
>> +struct device_node *np = pdev->dev.of_node;
>> +
>> +pmc_regs = of_iomap(pdev->dev.of_node, 0);
>> if (!pmc_regs)
>> return -ENOMEM;
>>
>> -pmc_dev = &ofdev->dev;
>> +has_deep_sleep = 0;
>> +if (of_device_is_compatible(np, "fsl,mpc8536-pmc"))
>> +has_deep_sleep = 1;
>> +
>> +has_lossless = 0;
>> +if (of_device_is_compatible(np, "fsl,p1022-pmc"))
>> +has_lossless = 1;
>> +
>
>You never use has_lossless.
It will be used in the later patch in the series.
- Leo
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 3/7] powerpc/85xx: add sleep and deep sleep support
>To: Zhao Chenhui-B35336; linuxppc-dev@lists.ozlabs.org >Subject: Re: [PATCH 3/7] powerpc/85xx: add sleep and deep sleep support > >Hi Zhao, > >From: Li Yang > >Some Freescale chips like MPC8536 and P1022 has deep sleep PM mode in >addtion to the sleep PM mode. > >In sleep PM mode, the clocks of e500 core and unused IP blocks is turned >off. IP blocks which are allowed to wake up the processor are still >running > >While in deep sleep PM mode, additionally, the power supply is removed >from e500 core and most IP blocks. Only the blocks needed to wake up the >chip out of deep sleep are ON. > >This patch supports 32-bit and 36-bit address space. > >The deep sleep mode is equal to the Suspend-to-RAM state of Linux Power >Management. > >Command to enter deep sleep mode. > echo mem > /sys/power/state > > >Thanks a lot for bringing this code to mainline. I was recently involved >in enabling deep sleep on a custom P1022 board, and would like to make >some remarks based on this experience. > >1. I think 85xx deep sleep code would be more complete if you also port >FSL > BSP code that saves eLBC configuration before entering deep sleep and > restores it afterwards. Otherwise all eLBC customization done by u- >boot is lost. Thanks for the comment. That work is also being considered for upstream, but not in this series. > >2. You should implement fsl_deep_sleep() routine for 85xx. The default >implementation > in include/linux/fsl_devices.h always returns 0. The routine is used >by FSL USB host > driver, drivers/usb/host/ehci-fsl.c to restore USB hardware state >after deep sleep. > With default implementation USB is dead on 85xx after deep sleep if >USB PHY is > powered down completely. Added to the to-do list. Thanks. - Leo ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/7] powerpc/85xx: add HOTPLUG_CPU support
>Subject: Re: [PATCH 2/7] powerpc/85xx: add HOTPLUG_CPU support
>
>On 11/04/2011 07:31 AM, Zhao Chenhui wrote:
>> From: Li Yang
>>
>> Add support to disable and re-enable individual cores at runtime
>> on MPC85xx/QorIQ SMP machines. Currently support e500 core.
>>
>> MPC85xx machines use ePAPR spin-table in boot page for CPU kick-off.
>> This patch uses the boot page from bootloader to boot core at runtime.
>> It supports 32-bit and 36-bit physical address.
>
>Note that there is no guarantee that the bootloader can handle you
>resetting a core. In ePAPR the spin table is a one-time release
>mechanism, not a core reset mechanism. If this has a U-Boot dependency,
>document that.
Actually we suggested to add a spin table in kernel so that we won't have the
dependency on u-boot. But there seems to be some opposite opinion in the
internal discussion. I personally prefer to remove the u-boot dependency and
add the mechanism in kernel.
>
>> #ifdef CONFIG_SMP
>> /* When we get here, r24 needs to hold the CPU # */
>> .globl __secondary_start
>> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
>> index 7bf2187..12a54f0 100644
>> --- a/arch/powerpc/kernel/smp.c
>> +++ b/arch/powerpc/kernel/smp.c
>> @@ -381,8 +381,14 @@ void generic_cpu_die(unsigned int cpu)
>>
>> for (i = 0; i < 100; i++) {
>> smp_rmb();
>> -if (per_cpu(cpu_state, cpu) == CPU_DEAD)
>> +if (per_cpu(cpu_state, cpu) == CPU_DEAD) {
>> +/*
>> + * After another core sets cpu_state to CPU_DEAD,
>> + * it needs some time to die.
>> + */
>> +msleep(10);
>> return;
>> +}
>> msleep(100);
>
>It would be better to do this as a call into platform-specific code than
>can check registers to determine whether the core has checked out (in
>our case, whether it has entered nap) -- or to do a suitable delay for
>that platform if this isn't possible.
It will be easier if we can add a platform specific cpu_die instead of using
the generic one?
>
>> diff --git a/arch/powerpc/platforms/85xx/smp.c
>b/arch/powerpc/platforms/85xx/smp.c
>> index 9b0de9c..5a54fc1 100644
>> --- a/arch/powerpc/platforms/85xx/smp.c
>> +++ b/arch/powerpc/platforms/85xx/smp.c
>> @@ -17,6 +17,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include
>> #include
>> @@ -30,26 +31,141 @@
>>
>> extern void __early_start(void);
>>
>> -#define BOOT_ENTRY_ADDR_UPPER 0
>> -#define BOOT_ENTRY_ADDR_LOWER 1
>> -#define BOOT_ENTRY_R3_UPPER 2
>> -#define BOOT_ENTRY_R3_LOWER 3
>> -#define BOOT_ENTRY_RESV 4
>> -#define BOOT_ENTRY_PIR 5
>> -#define BOOT_ENTRY_R6_UPPER 6
>> -#define BOOT_ENTRY_R6_LOWER 7
>> -#define NUM_BOOT_ENTRY 8
>> -#define SIZE_BOOT_ENTRY (NUM_BOOT_ENTRY * sizeof(u32))
>> -
>> -static int __init
>> -smp_85xx_kick_cpu(int nr)
>> +#define MPC85xx_BPTR_OFF0x00020
>> +#define MPC85xx_BPTR_EN 0x8000
>> +#define MPC85xx_BPTR_BOOT_PAGE_MASK 0x00ff
>> +#define MPC85xx_BRR_OFF 0xe0e4
>> +#define MPC85xx_ECM_EEBPCR_OFF 0x01010
>> +#define MPC85xx_PIC_PIR_OFF 0x41090
>> +
>> +struct epapr_entry {
>
>ePAPR is more than just the spin table. Call it something like
>epapr_spin_table.
>
>> +u32 addr_h;
>> +u32 addr_l;
>> +u32 r3_h;
>> +u32 r3_l;
>> +u32 reserved;
>> +u32 pir;
>> +u32 r6_h;
>> +u32 r6_l;
>> +};
>
>Get rid of r6, it is not part of the ePAPR spin table.
Agree.
>
>> +static int is_corenet;
>> +static void __cpuinit smp_85xx_setup_cpu(int cpu_nr);
>> +
>> +#if defined(CONFIG_HOTPLUG_CPU) && defined(CONFIG_PPC32)
>
>Why PPC32?
Not sure if it is the same for e5500. E5500 support will be verified later.
>
>> +extern void flush_disable_L1(void);
>
>If this isn't already in a header file, put it in one.
>
>> +static void __cpuinit smp_85xx_mach_cpu_die(void)
>> +{
>> +unsigned int cpu = smp_processor_id();
>> +register u32 tmp;
>> +
>> +local_irq_disable();
>> +idle_task_exit();
>> +generic_set_cpu_dead(cpu);
>> +smp_wmb();
>> +
>> +mtspr(SPRN_TSR, TSR_ENW | TSR_WIS | TSR_DIS | TSR_FIS);
>> +mtspr(SPRN_TCR, 0);
>
>If clearing TSR matters at all (I'm not sure that it does), first clear
>TCR, then TSR.
>
>> +flush_disable_L1();
>
>You'll also need to take down L2 on e500mc.
Right. E500mc support is beyond this patch series. We will work on it after
the e500v2 support is finalized.
>
>> +tmp = 0;
>> +if (cpu_has_feature(CPU_FTR_CAN_NAP))
>> +tmp = HID0_NAP;
>> +else if (cpu_has_feature(CPU_FTR_CAN_DOZE))
>> +tmp = HID0_DOZE;
>
>Those FTR bits are for what we can do in idle, and can be cleared if the
>user sets CONFIG_BDI_SWITCH.
It is powersave_nap variable shows what we can do
RE: [PATCH 1/7] powerpc/85xx: re-enable timebase sync disabled by KEXEC patch
>-Original Message-
>From: linuxppc-dev-bounces+leoli=freescale@lists.ozlabs.org
>[mailto:linuxppc-dev-bounces+leoli=freescale@lists.ozlabs.org] On
>Behalf Of Scott Wood
>Sent: Saturday, November 05, 2011 1:34 AM
>To: Zhao Chenhui-B35336
>Cc: linuxppc-dev@lists.ozlabs.org
>Subject: Re: [PATCH 1/7] powerpc/85xx: re-enable timebase sync disabled by
>KEXEC patch
>
>On 11/04/2011 07:29 AM, Zhao Chenhui wrote:
>> From: Li Yang
>>
>> The timebase sync is not only necessary when using KEXEC. It should also
>> be used by normal boot up and cpu hotplug. Remove the ifdef added by
>> the KEXEC patch.
>
>The KEXEC patch didn't just add the ifdef, it also added the initializers:
Yes. But the code suggests that the timebase synchronization is only necessary
for KEXEC, but it turns out that sleep/wakeup also need it. Maybe the
description of the patch need to be changed as KEXEC is not to be blamed.
>
>> @@ -105,8 +107,64 @@ smp_85xx_setup_cpu(int cpu_nr)
>>
>> struct smp_ops_t smp_85xx_ops = {
>> .kick_cpu = smp_85xx_kick_cpu,
>> +#ifdef CONFIG_KEXEC
>> + .give_timebase = smp_generic_give_timebase,
>> + .take_timebase = smp_generic_take_timebase,
>> +#endif
>> };
>
>U-Boot synchronizes the timebase on 85xx. With what chip and U-Boot
>version are you seeing this not happen?
I'm curious why don't we make it happen in kernel as we are against adding
dependency to the bootloader? Other architectures don't have this dependency,
it will be better if we don't add this dependency either IMO.
>
>If you are seeing only a small (around one tick) difference, make sure
>you're running a U-Boot that has this commit:
>
>> commit 7afc45ad7d9493208d89072cbb78a5bfc8034b59
>> Author: Kumar Gala
>> Date: Sun Mar 13 10:55:53 2011 -0500
>>
>> powerpc/85xx: Fix synchronization of timebase on MP boot
>>
>> There is a small ordering issue in the master core in that we need
>to
>> make sure the disabling of the timebase in the SoC is visible before
>we
>> set the value to 0. We can simply just read back the value to
>> synchronizatize the write, before we set TB to 0.
>>
>> Reported-by: Dan Hettena
>> Tested-by: Dan Hettena
>> Signed-off-by: Kumar Gala
>
>
>-Scott
>
>___
>Linuxppc-dev mailing list
>Linuxppc-dev@lists.ozlabs.org
>https://lists.ozlabs.org/listinfo/linuxppc-dev
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
