* [PATCH 1/2] alsa/soc: Fix typo in MPC5200 PSC AC97 driver Kconfig
From: Grant Likely @ 2009-06-28 7:41 UTC (permalink / raw)
To: jonsmirl, linuxppc-dev, alsa-devel, broonie, timur
From: Grant Likely <grant.likely@secretlab.ca>
ALSA SoC drivers should be specify SND_SOC_AC97_BUS instead, not AC97_BUS.
Without SND_SOC_AC97_BUS defined, an AC97 device will not get correctly
registered on the AC97 bus, which prevents thinks like the WM9712
touchscreen driver from getting probed.
Tested against 2.6.31-rc1.
Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---
sound/soc/fsl/Kconfig | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
index 5dbebf8..5661876 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -33,7 +33,7 @@ config SND_SOC_MPC5200_I2S
config SND_SOC_MPC5200_AC97
tristate "Freescale MPC5200 PSC in AC97 mode driver"
depends on PPC_MPC52xx && PPC_BESTCOMM
- select AC97_BUS
+ select SND_SOC_AC97_BUS
select SND_MPC52xx_DMA
select PPC_BESTCOMM_GEN_BD
help
^ permalink raw reply related
* Re: Inline Assembly queries
From: kernel mailz @ 2009-06-28 4:57 UTC (permalink / raw)
To: Ian Lance Taylor; +Cc: gcc-help, linuxppc-dev
In-Reply-To: <m3ab3t4623.fsf@google.com>
Thanks Ian,
For the "memory" clobber
I tried with the a function in linux kernel
--
/*
* Atomic exchange
*
* Changes the memory location '*ptr' to be val and returns
* the previous value stored there.
*/
static inline unsigned long
__xchg_u32(volatile void *p, unsigned long val)
{
unsigned long prev;
__asm__ __volatile__(
"1: lwarx %0,0,%2 \n"
" stwcx. %3,0,%2 \n\
bne- 1b"
: "=3D&r" (prev), "+m" (*(volatile unsigned int *)p)
: "r" (p), "r" (val)
// :"memory","cc");
return prev;
}
#define ADDR 0x1000
int main()
{
__xchg_u32((void*)ADDR, 0x2000);
__xchg_u32((void*)ADDR, 0x3000);
return 0;
}
Got the same asm, when compiled with O1 , with / without "memory" clobber
100003fc <main>:
100003fc: 39 20 10 00 li r9,4096
10000400: 38 00 20 00 li r0,8192
10000404: 7d 60 48 28 lwarx r11,0,r9
10000408: 7c 00 49 2d stwcx. r0,0,r9
1000040c: 40 a2 ff f8 bne- 10000404 <main+0x8>
10000410: 38 00 30 00 li r0,12288
10000414: 7d 60 48 28 lwarx r11,0,r9
10000418: 7c 00 49 2d stwcx. r0,0,r9
1000041c: 40 a2 ff f8 bne- 10000414 <main+0x18>
10000420: 38 60 00 00 li r3,0
10000424: 4e 80 00 20 blr
No diff ?
am I choosing the right example ?
-TZ
On Sun, Jun 28, 2009 at 4:50 AM, Ian Lance Taylor<iant@google.com> wrote:
> kernel mailz <kernelmailz@googlemail.com> writes:
>
>> I've been fiddling my luck with gcc 4.3.2 inline assembly on powerpc
>> There are a few queries
>>
>> 1. asm volatile or simply asm produce the same assembly code.
>> Tried with a few examples but didnt find any difference by adding
>> volatile with asm
>>
>> 2. Use of "memory" and clobbered registers.
>>
>> "memory" -
>> a. announce to the compiler that the memory has been modified
>> b. this instruction writes to some memory (other than a listed output)
>> and GCC shouldn=92t cache memory values in registers across this asm.
>>
>> I tried with stw and stwcx instruction, adding "memory" has no effect.
>>
>> Is there any example scenerio where gcc would generate different
>> assembly by adding / removing "memory" ?
>
> Please never send a message to both gcc@gcc.gnu.org and
> gcc-help@gcc.gnu.org. =A0This message is appropriate for
> gcc-help@gcc.gnu.org, not for gcc@gcc.gnu.org. =A0Thanks.
>
> An asm with no outputs is always considered to be volatile. =A0To see the
> affect of volatile, just try something like
> =A0 =A0asm ("# modify %0" : "=3Dr" (i) : /* no inputs */ : /* no clobbers=
*/);
> Try it with and without optimization.
>
> As the documentation says, the effect of adding a "memory" clobber is
> that gcc does not cache values in registers across the asm. =A0So the
> effect will be shown in something like
> =A0int i =3D *p;
> =A0asm volatile ("# read %0" : : "r" (i));
> =A0return *p;
> The memory clobber will only make a different when optimizing.
>
> Ian
>
^ permalink raw reply
* Re: sym scsi driver problem with 2.6.26 or newer debian kernel on p610 (fwd)
From: Benjamin Herrenschmidt @ 2009-06-27 22:54 UTC (permalink / raw)
To: blackluck
Cc: Olof Johansson, debian-powerpc, Guennadi Liakhovetski,
linuxppc-dev
In-Reply-To: <4A465AF5.9000606@ktk.bme.hu>
On Sat, 2009-06-27 at 19:46 +0200, Laszlo Fekete wrote:
> Hello!
>
> Thank you very much, this patch works me too.
>
> Maybe this patch will be in the debian kernel someday?
The patch isn't actually correct just yet :-) Michael will
be posting a proper one next week. It should be possible to
request its inclusion into debian separately, we'll probably
send it to stable@kernel.org as well.
Cheers,
Ben.
> Thank you: blackluck
>
> Michael Ellerman wrote:
> > On Wed, 2009-06-24 at 15:56 +1000, Benjamin Herrenschmidt wrote:
> >
> > > On Wed, 2009-06-24 at 15:53 +1000, Michael Ellerman wrote:
> > >
> > > > Doesn't fix my machine :/
> > > >
> > > >
> > > That doesn't make sense ... What if you remove the bit inside the ifdef
> > > CONFIG_MPIC_BROKEN_REGREAD in _mpic_read() ?
> > >
> > > If that makes a difference, then it would be interesting to add a printk
> > > in there that prints what the original value "val" is and what we have
> > > in the shadow...
> > >
> >
> > With this patch it boots:
> >
> > diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> > index 2353adc..fc17289 100644
> > --- a/arch/powerpc/sysdev/mpic.c
> > +++ b/arch/powerpc/sysdev/mpic.c
> > @@ -231,13 +231,16 @@ static inline u32 _mpic_irq_read(struct mpic *mpic, unsign
> > unsigned int isu = src_no >> mpic->isu_shift;
> > unsigned int idx = src_no & mpic->isu_mask;
> > unsigned int val;
> > + unsigned int shadow;
> >
> > val = _mpic_read(mpic->reg_type, &mpic->isus[isu],
> > reg + (idx * MPIC_INFO(IRQ_STRIDE)));
> > #ifdef CONFIG_MPIC_BROKEN_REGREAD
> > - if (reg == 0)
> > - val = (val & (MPIC_VECPRI_MASK | MPIC_VECPRI_ACTIVITY)) |
> > + if (reg == 0) {
> > + shadow = (val & (MPIC_VECPRI_MASK | MPIC_VECPRI_ACTIVITY)) |
> > mpic->isu_reg0_shadow[idx];
> > + printk("%s: val 0x%x shadow 0x%x\n", __func__, val, shadow);
> > + }
> > #endif
> > return val;
> > }
> >
> >
> > And I see:
> >
> > sym53c8xx 0000:00:0c.0: enabling device (0140 -> 0143)
> > sym0: <896> rev 0x7 at pci 0000:00:0c.0 irq 17
> > sym0: No NVRAM, ID 7, Fast-40, SE, parity checking
> > _mpic_irq_read: val 0x80480004 shadow 0x80080014
> > _mpic_irq_read: val 0x480004 shadow 0x480004
> >
> >
> >
> > cheers
> >
^ permalink raw reply
* Re: Trouble "Transferring control to Linux (at address 00000000)"
From: Frank Svendsbøe @ 2009-06-27 21:01 UTC (permalink / raw)
To: Mikhail Zaturenskiy; +Cc: Scott Wood, linuxppc-dev
In-Reply-To: <97dd5fd20906261012h34a85b33j9183eb9e41ed1181@mail.gmail.com>
On Fri, Jun 26, 2009 at 7:12 PM, Mikhail
Zaturenskiy<mzaturenskiy.st@gmail.com> wrote:
> Hi Scott,
>
>> This isn't the denx list;
> I've noticed :) but I'm still learning about this whole process so I
> though I could get some general suggestions.
>
>> what kernel version is that, and with what
>> modifications from mainline?
> Kernel is v2.6.30, I'm not yet familiar enough with it to know what's
> been modified from mainline, just following instructions.
>
>> Note that ep88xc.dts in mainline is intended for use with PlanetCore, wh=
ich
>> is what ships on that board. =A0You may need to make modifications for i=
t to
>> work with u-boot (at the least, the IMMR base is probably different).
> Hmm, this hasn't occurred to me, thank you for pointing it out.
>
I don't have access to this board, but have experience with a similar
one (Adder 875), and
I might be able to help out. Scott's right. According to U-Boots
include/configs/EP88x.h,
CONFIG_SYS_IMMR is 0xf0000000, but the Linux the ep88xc.dts is
referring to an IMMR set
to 0xfa200000. Replace every instance of 0xfa20 with 0xf000, and it may wor=
k.
>> Also, make sure u-boot is properly updating the memory size in the devic=
e
>> tree. =A0Can you dump the post-fixup device tree in u-boot?
> Not sure, but I'll try to find out if that's possible. It'd certainly
> answer a lot of questions...
>
> Thanks,
> Mikhail Zaturenskiy
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
^ permalink raw reply
* Inline Assembly queries
From: kernel mailz @ 2009-06-27 19:46 UTC (permalink / raw)
To: gcc-help, gcc-help-help, linuxppc-dev
Hello All the gurus,
I've been fiddling my luck with gcc 4.3.2 inline assembly on powerpc
There are a few queries
1. asm volatile or simply asm produce the same assembly code.
Tried with a few examples but didnt find any difference by adding
volatile with asm
2. Use of "memory" and clobbered registers.
"memory" -
a. announce to the compiler that the memory has been modified
b. this instruction writes to some memory (other than a listed output)
and GCC shouldn=92t cache memory values in registers across this asm.
I tried with stw and stwcx instruction, adding "memory" has no effect.
Is there any example scenerio where gcc would generate different
assembly by adding / removing "memory" ?
-TZ
^ permalink raw reply
* Re: sym scsi driver problem with 2.6.26 or newer debian kernel on p610 (fwd)
From: Laszlo Fekete @ 2009-06-27 17:46 UTC (permalink / raw)
To: michael; +Cc: Olof Johansson, debian-powerpc, Guennadi Liakhovetski,
linuxppc-dev
In-Reply-To: <1245824392.9237.85.camel@concordia>
[-- Attachment #1.1: Type: text/plain, Size: 2032 bytes --]
Hello!
Thank you very much, this patch works me too.
Maybe this patch will be in the debian kernel someday?
Thank you: blackluck
Michael Ellerman wrote:
> On Wed, 2009-06-24 at 15:56 +1000, Benjamin Herrenschmidt wrote:
>
>> On Wed, 2009-06-24 at 15:53 +1000, Michael Ellerman wrote:
>>
>>> Doesn't fix my machine :/
>>>
>>>
>> That doesn't make sense ... What if you remove the bit inside the ifdef
>> CONFIG_MPIC_BROKEN_REGREAD in _mpic_read() ?
>>
>> If that makes a difference, then it would be interesting to add a printk
>> in there that prints what the original value "val" is and what we have
>> in the shadow...
>>
>
> With this patch it boots:
>
> diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> index 2353adc..fc17289 100644
> --- a/arch/powerpc/sysdev/mpic.c
> +++ b/arch/powerpc/sysdev/mpic.c
> @@ -231,13 +231,16 @@ static inline u32 _mpic_irq_read(struct mpic *mpic, unsign
> unsigned int isu = src_no >> mpic->isu_shift;
> unsigned int idx = src_no & mpic->isu_mask;
> unsigned int val;
> + unsigned int shadow;
>
> val = _mpic_read(mpic->reg_type, &mpic->isus[isu],
> reg + (idx * MPIC_INFO(IRQ_STRIDE)));
> #ifdef CONFIG_MPIC_BROKEN_REGREAD
> - if (reg == 0)
> - val = (val & (MPIC_VECPRI_MASK | MPIC_VECPRI_ACTIVITY)) |
> + if (reg == 0) {
> + shadow = (val & (MPIC_VECPRI_MASK | MPIC_VECPRI_ACTIVITY)) |
> mpic->isu_reg0_shadow[idx];
> + printk("%s: val 0x%x shadow 0x%x\n", __func__, val, shadow);
> + }
> #endif
> return val;
> }
>
>
> And I see:
>
> sym53c8xx 0000:00:0c.0: enabling device (0140 -> 0143)
> sym0: <896> rev 0x7 at pci 0000:00:0c.0 irq 17
> sym0: No NVRAM, ID 7, Fast-40, SE, parity checking
> _mpic_irq_read: val 0x80480004 shadow 0x80080014
> _mpic_irq_read: val 0x480004 shadow 0x480004
>
>
>
> cheers
>
[-- Attachment #1.2: Type: text/html, Size: 2541 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 260 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] perf_counter: powerpc: Add callchain support
From: Ingo Molnar @ 2009-06-27 16:58 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: Paul Mackerras, linux-kernel, linuxppc-dev
In-Reply-To: <1246091684.31755.210.camel@twins>
* Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> On Sat, 2009-06-27 at 15:31 +1000, Paul Mackerras wrote:
> > + if (regs) {
> > + if (current_is_64bit())
> > + perf_callchain_user_64(regs, entry);
> > + else
> > + perf_callchain_user_32(regs, entry);
> > + }
>
> Ingo do we need 32 on 64 stuff like that too?
hm, indeed.
Ingo
^ permalink raw reply
* Re: [PATCH 2/2] perf_counter: powerpc: Add callchain support
From: Peter Zijlstra @ 2009-06-27 8:34 UTC (permalink / raw)
To: Paul Mackerras; +Cc: Ingo Molnar, linux-kernel, linuxppc-dev
In-Reply-To: <19013.44722.893263.275594@cargo.ozlabs.ibm.com>
On Sat, 2009-06-27 at 15:31 +1000, Paul Mackerras wrote:
> + if (regs) {
> + if (current_is_64bit())
> + perf_callchain_user_64(regs, entry);
> + else
> + perf_callchain_user_32(regs, entry);
> + }
Ingo do we need 32 on 64 stuff like that too?
^ permalink raw reply
* [PATCH 2/2] perf_counter: powerpc: Add callchain support
From: Paul Mackerras @ 2009-06-27 5:31 UTC (permalink / raw)
To: Ingo Molnar, benh; +Cc: linuxppc-dev, Peter Zijlstra, linux-kernel
In-Reply-To: <19013.44646.549261.100582@cargo.ozlabs.ibm.com>
This adds support for tracing callchains for powerpc, both 32-bit
and 64-bit, and both in the kernel and userspace, from PMU interrupt
context.
The first three entries stored for each callchain are the NIP (next
instruction pointer), LR (link register), and the contents of the LR
save area in the second stack frame (the first is ignored because the
ABI convention on powerpc is that functions save their return address
in their caller's stack frame). Because functions don't have to save
their return address (LR value) and don't have to establish a stack
frame, it's possible for either or both of LR and the second stack
frame's LR save area to have valid return addresses in them. This
is basically impossible to disambiguate without either reading the
code or looking at auxiliary information such as CFI tables. Since
we don't want to do that at interrupt time, we store both LR and the
second stack frame's LR save area.
Once we get past the second stack frame, there is no ambiguity; all
return addresses we get are reliable.
For kernel traces, we check whether they are valid kernel instruction
addresses and store zero instead if they are not (rather than
omitting them, which would make it impossible for userspace to know
which was which). We also store zero instead of the second stack
frame's LR save area value if it is the same as LR.
For kernel traces, we check for interrupt frames, and for user traces,
we check for signal frames. In each case, since we're starting a new
trace, we store a PERF_CONTEXT_KERNEL/USER marker so that userspace
knows that the next three entries are NIP, LR and the second stack fram=
e
for the interrupted context.
We read user memory with __get_user_inatomic. On 64-bit, we set a flag=
to indicate that the data storage exception handler shouldn't call
hash_page on a MMU hashtable miss. Instead we get a -EFAULT from
__get_user_inatomic and then read the Linux PTE and access the page
via the kernel linear mapping. Since 64-bit doesn't use (or need)
highmem there is no need to do kmap_atomic.
Signed-off-by: Paul Mackerras <paulus@samba.org>
---
arch/powerpc/kernel/Makefile | 2 +-
arch/powerpc/kernel/perf_callchain.c | 544 ++++++++++++++++++++++++++=
++++++++
2 files changed, 545 insertions(+), 1 deletions(-)
create mode 100644 arch/powerpc/kernel/perf_callchain.c
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefil=
e
index b73396b..9619285 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -97,7 +97,7 @@ obj64-$(CONFIG_AUDIT)=09=09+=3D compat_audit.o
=20
obj-$(CONFIG_DYNAMIC_FTRACE)=09+=3D ftrace.o
obj-$(CONFIG_FUNCTION_GRAPH_TRACER)=09+=3D ftrace.o
-obj-$(CONFIG_PPC_PERF_CTRS)=09+=3D perf_counter.o
+obj-$(CONFIG_PPC_PERF_CTRS)=09+=3D perf_counter.o perf_callchain.o
obj64-$(CONFIG_PPC_PERF_CTRS)=09+=3D power4-pmu.o ppc970-pmu.o power5-=
pmu.o \
=09=09=09=09 power5+-pmu.o power6-pmu.o power7-pmu.o
obj32-$(CONFIG_PPC_PERF_CTRS)=09+=3D mpc7450-pmu.o
diff --git a/arch/powerpc/kernel/perf_callchain.c b/arch/powerpc/kernel=
/perf_callchain.c
new file mode 100644
index 0000000..3cc1487
--- /dev/null
+++ b/arch/powerpc/kernel/perf_callchain.c
@@ -0,0 +1,544 @@
+/*
+ * Performance counter callchain support - powerpc architecture code
+ *
+ * Copyright =A9 2009 Paul Mackerras, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/perf_counter.h>
+#include <linux/percpu.h>
+#include <linux/uaccess.h>
+#include <linux/mm.h>
+#include <asm/ptrace.h>
+#include <asm/pgtable.h>
+#include <asm/sigcontext.h>
+#include <asm/ucontext.h>
+#include <asm/vdso.h>
+#ifdef CONFIG_PPC64
+#include "ppc32.h"
+#endif
+
+/*
+ * Store another value in a callchain_entry.
+ */
+static inline void callchain_store(struct perf_callchain_entry *entry,=
u64 ip)
+{
+=09unsigned int nr =3D entry->nr;
+
+=09if (nr < PERF_MAX_STACK_DEPTH) {
+=09=09entry->ip[nr] =3D ip;
+=09=09entry->nr =3D nr + 1;
+=09}
+}
+
+/*
+ * Is sp valid as the address of the next kernel stack frame after pre=
v_sp?
+ * The next frame may be in a different stack area but should not go
+ * back down in the same stack area.
+ */
+static int valid_next_sp(unsigned long sp, unsigned long prev_sp)
+{
+=09if (sp & 0xf)
+=09=09return 0;=09=09/* must be 16-byte aligned */
+=09if (!validate_sp(sp, current, STACK_FRAME_OVERHEAD))
+=09=09return 0;
+=09if (sp >=3D prev_sp + STACK_FRAME_OVERHEAD)
+=09=09return 1;
+=09/*
+=09 * sp could decrease when we jump off an interrupt stack
+=09 * back to the regular process stack.
+=09 */
+=09if ((sp & ~(THREAD_SIZE - 1)) !=3D (prev_sp & ~(THREAD_SIZE - 1)))
+=09=09return 1;
+=09return 0;
+}
+
+static void perf_callchain_kernel(struct pt_regs *regs,
+=09=09=09=09 struct perf_callchain_entry *entry)
+{
+=09unsigned long sp, next_sp;
+=09unsigned long next_ip;
+=09unsigned long lr;
+=09long level =3D 0;
+=09unsigned long *fp;
+
+=09lr =3D regs->link;
+=09sp =3D regs->gpr[1];
+=09callchain_store(entry, PERF_CONTEXT_KERNEL);
+=09callchain_store(entry, regs->nip);
+
+=09if (!validate_sp(sp, current, STACK_FRAME_OVERHEAD))
+=09=09return;
+
+=09for (;;) {
+=09=09fp =3D (unsigned long *) sp;
+=09=09next_sp =3D fp[0];
+
+=09=09if (next_sp =3D=3D sp + STACK_INT_FRAME_SIZE &&
+=09=09 fp[STACK_FRAME_MARKER] =3D=3D STACK_FRAME_REGS_MARKER) {
+=09=09=09/*
+=09=09=09 * This looks like an interrupt frame for an
+=09=09=09 * interrupt that occurred in the kernel
+=09=09=09 */
+=09=09=09regs =3D (struct pt_regs *)(sp + STACK_FRAME_OVERHEAD);
+=09=09=09next_ip =3D regs->nip;
+=09=09=09lr =3D regs->link;
+=09=09=09level =3D 0;
+=09=09=09callchain_store(entry, PERF_CONTEXT_KERNEL);
+
+=09=09} else {
+=09=09=09if (level =3D=3D 0)
+=09=09=09=09next_ip =3D lr;
+=09=09=09else
+=09=09=09=09next_ip =3D fp[STACK_FRAME_LR_SAVE];
+
+=09=09=09/*
+=09=09=09 * We can't tell which of the first two addresses
+=09=09=09 * we get are valid, but we can filter out the
+=09=09=09 * obviously bogus ones here. We replace them
+=09=09=09 * with 0 rather than removing them entirely so
+=09=09=09 * that userspace can tell which is which.
+=09=09=09 */
+=09=09=09if ((level =3D=3D 1 && next_ip =3D=3D lr) ||
+=09=09=09 (level <=3D 1 && !kernel_text_address(next_ip)))
+=09=09=09=09next_ip =3D 0;
+
+=09=09=09++level;
+=09=09}
+
+=09=09callchain_store(entry, next_ip);
+=09=09if (!valid_next_sp(next_sp, sp))
+=09=09=09return;
+=09=09sp =3D next_sp;
+=09}
+}
+
+#ifdef CONFIG_PPC64
+/*
+ * On 64-bit we don't want to invoke hash_page on user addresses from
+ * interrupt context, so if the access faults, we read the page tables=
+ * to find which page (if any) is mapped and access it directly.
+ */
+static int read_user_stack_slow(void __user *ptr, void *ret, int nb)
+{
+=09pgd_t *pgdir;
+=09pte_t *ptep, pte;
+=09int pagesize;
+=09unsigned long addr =3D (unsigned long) ptr;
+=09unsigned long offset;
+=09unsigned long pfn;
+=09void *kaddr;
+
+=09pgdir =3D current->mm->pgd;
+=09if (!pgdir)
+=09=09return -EFAULT;
+
+=09pagesize =3D get_slice_psize(current->mm, addr);
+
+=09/* align address to page boundary */
+=09offset =3D addr & ((1ul << mmu_psize_defs[pagesize].shift) - 1);
+=09addr -=3D offset;
+
+=09if (HPAGE_SHIFT && mmu_huge_psizes[pagesize])
+=09=09ptep =3D huge_pte_offset(current->mm, addr);
+=09else
+=09=09ptep =3D find_linux_pte(pgdir, addr);
+
+=09if (ptep =3D=3D NULL)
+=09=09return -EFAULT;
+=09pte =3D *ptep;
+=09if (!pte_present(pte) || !(pte_val(pte) & _PAGE_USER))
+=09=09return -EFAULT;
+=09pfn =3D pte_pfn(pte);
+=09if (!page_is_ram(pfn))
+=09=09return -EFAULT;
+
+=09/* no highmem to worry about here */
+=09kaddr =3D pfn_to_kaddr(pfn);
+=09memcpy(ret, kaddr + offset, nb);
+=09return 0;
+}
+
+static int read_user_stack_64(unsigned long __user *ptr, unsigned long=
*ret)
+{
+=09int err;
+
+=09if ((unsigned long)ptr > TASK_SIZE - sizeof(unsigned long) ||
+=09 ((unsigned long)ptr & 7))
+=09=09return -EFAULT;
+
+=09/*
+=09 * On 64-bit, tell the DSI handler not to call hash_page
+=09 * if this access causes a hashtable miss fault.
+=09 */
+=09get_paca()->in_pmu_nmi =3D 1;
+=09barrier();
+=09err =3D __get_user_inatomic(*ret, ptr);
+=09barrier();
+=09get_paca()->in_pmu_nmi =3D 0;
+
+=09if (!err)
+=09=09return 0;
+
+=09return read_user_stack_slow(ptr, ret, 8);
+}
+
+static int read_user_stack_32(unsigned int __user *ptr, unsigned int *=
ret)
+{
+=09int err;
+
+=09if ((unsigned long)ptr > TASK_SIZE - sizeof(unsigned int) ||
+=09 ((unsigned long)ptr & 3))
+=09=09return -EFAULT;
+
+=09/*
+=09 * On 64-bit, tell the DSI handler not to call hash_page
+=09 * if this access causes a hashtable miss fault.
+=09 */
+=09get_paca()->in_pmu_nmi =3D 1;
+=09barrier();
+=09err =3D __get_user_inatomic(*ret, ptr);
+=09barrier();
+=09get_paca()->in_pmu_nmi =3D 0;
+
+=09if (!err)
+=09=09return 0;
+
+=09return read_user_stack_slow(ptr, ret, 4);
+}
+
+static inline int valid_user_sp(unsigned long sp, int is_64)
+{
+=09if (!sp || (sp & 7) || sp > (is_64 ? TASK_SIZE : 0x100000000UL) - 3=
2)
+=09=09return 0;
+=09return 1;
+}
+
+/*
+ * 64-bit user processes use the same stack frame for RT and non-RT si=
gnals.
+ */
+struct signal_frame_64 {
+=09char=09=09dummy[__SIGNAL_FRAMESIZE];
+=09struct ucontext=09uc;
+=09unsigned long=09unused[2];
+=09unsigned int=09tramp[6];
+=09struct siginfo=09*pinfo;
+=09void=09=09*puc;
+=09struct siginfo=09info;
+=09char=09=09abigap[288];
+};
+
+static int is_sigreturn_64_address(unsigned long nip, unsigned long fp=
)
+{
+=09if (nip =3D=3D fp + offsetof(struct signal_frame_64, tramp))
+=09=09return 1;
+=09if (vdso64_rt_sigtramp && current->mm->context.vdso_base &&
+=09 nip =3D=3D current->mm->context.vdso_base + vdso64_rt_sigtramp)=
+=09=09return 1;
+=09return 0;
+}
+
+/*
+ * Do some sanity checking on the signal frame pointed to by sp.
+ * We check the pinfo and puc pointers in the frame.
+ */
+static int sane_signal_64_frame(unsigned long sp)
+{
+=09struct signal_frame_64 __user *sf;
+=09unsigned long pinfo, puc;
+
+=09sf =3D (struct signal_frame_64 __user *) sp;
+=09if (read_user_stack_64((unsigned long __user *) &sf->pinfo, &pinfo)=
||
+=09 read_user_stack_64((unsigned long __user *) &sf->puc, &puc))
+=09=09return 0;
+=09return pinfo =3D=3D (unsigned long) &sf->info &&
+=09=09puc =3D=3D (unsigned long) &sf->uc;
+}
+
+static void perf_callchain_user_64(struct pt_regs *regs,
+=09=09=09=09 struct perf_callchain_entry *entry)
+{
+=09unsigned long sp, next_sp;
+=09unsigned long next_ip;
+=09unsigned long lr;
+=09long level =3D 0;
+=09struct signal_frame_64 __user *sigframe;
+=09unsigned long __user *fp, *uregs;
+
+=09next_ip =3D regs->nip;
+=09lr =3D regs->link;
+=09sp =3D regs->gpr[1];
+=09callchain_store(entry, PERF_CONTEXT_USER);
+=09callchain_store(entry, next_ip);
+
+=09for (;;) {
+=09=09fp =3D (unsigned long __user *) sp;
+=09=09if (!valid_user_sp(sp, 1) || read_user_stack_64(fp, &next_sp))
+=09=09=09return;
+=09=09if (level > 0 && read_user_stack_64(&fp[2], &next_ip))
+=09=09=09return;
+
+=09=09/*
+=09=09 * Note: the next_sp - sp >=3D signal frame size check
+=09=09 * is true when next_sp < sp, which can happen when
+=09=09 * transitioning from an alternate signal stack to the
+=09=09 * normal stack.
+=09=09 */
+=09=09if (next_sp - sp >=3D sizeof(struct signal_frame_64) &&
+=09=09 (is_sigreturn_64_address(next_ip, sp) ||
+=09=09 (level <=3D 1 && is_sigreturn_64_address(lr, sp))) &&
+=09=09 sane_signal_64_frame(sp)) {
+=09=09=09/*
+=09=09=09 * This looks like an signal frame
+=09=09=09 */
+=09=09=09sigframe =3D (struct signal_frame_64 __user *) sp;
+=09=09=09uregs =3D sigframe->uc.uc_mcontext.gp_regs;
+=09=09=09if (read_user_stack_64(&uregs[PT_NIP], &next_ip) ||
+=09=09=09 read_user_stack_64(&uregs[PT_LNK], &lr) ||
+=09=09=09 read_user_stack_64(&uregs[PT_R1], &sp))
+=09=09=09=09return;
+=09=09=09level =3D 0;
+=09=09=09callchain_store(entry, PERF_CONTEXT_USER);
+=09=09=09callchain_store(entry, next_ip);
+=09=09=09continue;
+=09=09}
+
+=09=09if (level =3D=3D 0)
+=09=09=09next_ip =3D lr;
+=09=09callchain_store(entry, next_ip);
+=09=09++level;
+=09=09sp =3D next_sp;
+=09}
+}
+
+static inline int current_is_64bit(void)
+{
+=09/*
+=09 * We can't use test_thread_flag() here because we may be on an
+=09 * interrupt stack, and the thread flags don't get copied over
+=09 * from the thread_info on the main stack to the interrupt stack.
+=09 */
+=09return !test_ti_thread_flag(task_thread_info(current), TIF_32BIT);
+}
+
+#else /* CONFIG_PPC64 */
+/*
+ * On 32-bit we just access the address and let hash_page create a
+ * HPTE if necessary, so there is no need to fall back to reading
+ * the page tables. Since this is called at interrupt level,
+ * do_page_fault() won't treat a DSI as a page fault.
+ */
+static int read_user_stack_32(unsigned int __user *ptr, unsigned int *=
ret)
+{
+=09if ((unsigned long)ptr > TASK_SIZE - sizeof(unsigned int) ||
+=09 ((unsigned long)ptr & 3))
+=09=09return -EFAULT;
+
+=09return __get_user_inatomic(*ret, ptr);
+}
+
+static inline void perf_callchain_user_64(struct pt_regs *regs,
+=09=09=09=09=09 struct perf_callchain_entry *entry)
+{
+}
+
+static inline int current_is_64bit(void)
+{
+=09return 0;
+}
+
+static inline int valid_user_sp(unsigned long sp, int is_64)
+{
+=09if (!sp || (sp & 7) || sp > TASK_SIZE - 32)
+=09=09return 0;
+=09return 1;
+}
+
+#define __SIGNAL_FRAMESIZE32=09__SIGNAL_FRAMESIZE
+#define sigcontext32=09=09sigcontext
+#define mcontext32=09=09mcontext
+#define ucontext32=09=09ucontext
+#define compat_siginfo_t=09struct siginfo
+
+#endif /* CONFIG_PPC64 */
+
+/*
+ * Layout for non-RT signal frames
+ */
+struct signal_frame_32 {
+=09char=09=09=09dummy[__SIGNAL_FRAMESIZE32];
+=09struct sigcontext32=09sctx;
+=09struct mcontext32=09mctx;
+=09int=09=09=09abigap[56];
+};
+
+/*
+ * Layout for RT signal frames
+ */
+struct rt_signal_frame_32 {
+=09char=09=09=09dummy[__SIGNAL_FRAMESIZE32 + 16];
+=09compat_siginfo_t=09info;
+=09struct ucontext32=09uc;
+=09int=09=09=09abigap[56];
+};
+
+static int is_sigreturn_32_address(unsigned int nip, unsigned int fp)
+{
+=09if (nip =3D=3D fp + offsetof(struct signal_frame_32, mctx.mc_pad))
+=09=09return 1;
+=09if (vdso32_sigtramp && current->mm->context.vdso_base &&
+=09 nip =3D=3D current->mm->context.vdso_base + vdso32_sigtramp)
+=09=09return 1;
+=09return 0;
+}
+
+static int is_rt_sigreturn_32_address(unsigned int nip, unsigned int f=
p)
+{
+=09if (nip =3D=3D fp + offsetof(struct rt_signal_frame_32,
+=09=09=09=09 uc.uc_mcontext.mc_pad))
+=09=09return 1;
+=09if (vdso32_rt_sigtramp && current->mm->context.vdso_base &&
+=09 nip =3D=3D current->mm->context.vdso_base + vdso32_rt_sigtramp)=
+=09=09return 1;
+=09return 0;
+}
+
+static int sane_signal_32_frame(unsigned int sp)
+{
+=09struct signal_frame_32 __user *sf;
+=09unsigned int regs;
+
+=09sf =3D (struct signal_frame_32 __user *) (unsigned long) sp;
+=09if (read_user_stack_32((unsigned int __user *) &sf->sctx.regs, ®=
s))
+=09=09return 0;
+=09return regs =3D=3D (unsigned long) &sf->mctx;
+}
+
+static int sane_rt_signal_32_frame(unsigned int sp)
+{
+=09struct rt_signal_frame_32 __user *sf;
+=09unsigned int regs;
+
+=09sf =3D (struct rt_signal_frame_32 __user *) (unsigned long) sp;
+=09if (read_user_stack_32((unsigned int __user *) &sf->uc.uc_regs, &re=
gs))
+=09=09return 0;
+=09return regs =3D=3D (unsigned long) &sf->uc.uc_mcontext;
+}
+
+static unsigned int __user *signal_frame_32_regs(unsigned int sp,
+=09=09=09=09unsigned int next_sp, unsigned int next_ip)
+{
+=09struct mcontext32 __user *mctx =3D NULL;
+=09struct signal_frame_32 __user *sf;
+=09struct rt_signal_frame_32 __user *rt_sf;
+
+=09/*
+=09 * Note: the next_sp - sp >=3D signal frame size check
+=09 * is true when next_sp < sp, for example, when
+=09 * transitioning from an alternate signal stack to the
+=09 * normal stack.
+=09 */
+=09if (next_sp - sp >=3D sizeof(struct signal_frame_32) &&
+=09 is_sigreturn_32_address(next_ip, sp) &&
+=09 sane_signal_32_frame(sp)) {
+=09=09sf =3D (struct signal_frame_32 __user *) (unsigned long) sp;
+=09=09mctx =3D &sf->mctx;
+=09}
+
+=09if (!mctx && next_sp - sp >=3D sizeof(struct rt_signal_frame_32) &&=
+=09 is_rt_sigreturn_32_address(next_ip, sp) &&
+=09 sane_rt_signal_32_frame(sp)) {
+=09=09rt_sf =3D (struct rt_signal_frame_32 __user *) (unsigned long) s=
p;
+=09=09mctx =3D &rt_sf->uc.uc_mcontext;
+=09}
+
+=09if (!mctx)
+=09=09return NULL;
+=09return mctx->mc_gregs;
+}
+
+static void perf_callchain_user_32(struct pt_regs *regs,
+=09=09=09=09 struct perf_callchain_entry *entry)
+{
+=09unsigned int sp, next_sp;
+=09unsigned int next_ip;
+=09unsigned int lr;
+=09long level =3D 0;
+=09unsigned int __user *fp, *uregs;
+
+=09next_ip =3D regs->nip;
+=09lr =3D regs->link;
+=09sp =3D regs->gpr[1];
+=09callchain_store(entry, PERF_CONTEXT_USER);
+=09callchain_store(entry, next_ip);
+
+=09while (entry->nr < PERF_MAX_STACK_DEPTH) {
+=09=09fp =3D (unsigned int __user *) (unsigned long) sp;
+=09=09if (!valid_user_sp(sp, 0) || read_user_stack_32(fp, &next_sp))
+=09=09=09return;
+=09=09if (level > 0 && read_user_stack_32(&fp[1], &next_ip))
+=09=09=09return;
+
+=09=09uregs =3D signal_frame_32_regs(sp, next_sp, next_ip);
+=09=09if (!uregs && level <=3D 1)
+=09=09=09uregs =3D signal_frame_32_regs(sp, next_sp, lr);
+=09=09if (uregs) {
+=09=09=09/*
+=09=09=09 * This looks like an signal frame, so restart
+=09=09=09 * the stack trace with the values in it.
+=09=09=09 */
+=09=09=09if (read_user_stack_32(&uregs[PT_NIP], &next_ip) ||
+=09=09=09 read_user_stack_32(&uregs[PT_LNK], &lr) ||
+=09=09=09 read_user_stack_32(&uregs[PT_R1], &sp))
+=09=09=09=09return;
+=09=09=09level =3D 0;
+=09=09=09callchain_store(entry, PERF_CONTEXT_USER);
+=09=09=09callchain_store(entry, next_ip);
+=09=09=09continue;
+=09=09}
+
+=09=09if (level =3D=3D 0)
+=09=09=09next_ip =3D lr;
+=09=09callchain_store(entry, next_ip);
+=09=09++level;
+=09=09sp =3D next_sp;
+=09}
+}
+
+/*
+ * Since we can't get PMU interrupts inside a PMU interrupt handler,
+ * we don't need separate irq and nmi entries here.
+ */
+static DEFINE_PER_CPU(struct perf_callchain_entry, callchain);
+
+struct perf_callchain_entry *perf_callchain(struct pt_regs *regs)
+{
+=09struct perf_callchain_entry *entry =3D &__get_cpu_var(callchain);
+
+=09entry->nr =3D 0;
+
+=09if (current->pid =3D=3D 0)=09=09/* idle task? */
+=09=09return entry;
+
+=09if (!user_mode(regs)) {
+=09=09perf_callchain_kernel(regs, entry);
+=09=09if (current->mm)
+=09=09=09regs =3D task_pt_regs(current);
+=09=09else
+=09=09=09regs =3D NULL;
+=09}
+
+=09if (regs) {
+=09=09if (current_is_64bit())
+=09=09=09perf_callchain_user_64(regs, entry);
+=09=09else
+=09=09=09perf_callchain_user_32(regs, entry);
+=09}
+
+=09return entry;
+}
--=20
1.6.0.4
^ permalink raw reply related
* [PATCH 1/2] powerpc: Allow perf_counters to access user memory at interrupt time
From: Paul Mackerras @ 2009-06-27 5:30 UTC (permalink / raw)
To: benh; +Cc: linuxppc-dev, Ingo Molnar, Peter Zijlstra, linux-kernel
This provides a mechanism to allow the perf_counters code to access
user memory in a PMU interrupt routine on a 64-bit kernel. Such an
access can cause a SLB miss interrupt and/or a MMU hash table miss
interrupt.
An SLB miss interrupt on a user address will update the slb_cache and
slb_cache_ptr fields in the paca. This is OK except in the case where
a PMU interrupt occurs in switch_slb, which also accesses those fields.
To prevent this, we hard-disable interrupts in switch_slb. Interrupts
are already soft-disabled in switch-slb, and will get hard-enabled
when they get soft-enabled later.
If a MMU hashtable miss interrupt occurs, normally we would call
hash_page to look up the Linux PTE for the address and create a HPTE.
However, hash_page is fairly complex and takes some locks, so to
avoid the possibility of deadlock, we add a flag to the paca to
indicate to the MMU hashtable miss handler that it should not call
hash_page but instead treat it like a bad access that will get
reported up through the exception table mechanism. The PMU interrupt
code should then set this flag (get_paca()->in_pmu_nmi) and use
__get_user_inatomic to read user memory. If there is no HPTE for
the address, __get_user_inatomic will return -EFAULT.
On a 32-bit processor, there is no SLB. The 32-bit hash_page appears
to be safe to call in an interrupt handler since we don't do soft
disabling of interrupts on 32-bit, and the only lock that hash_page
takes is the mmu_hash_lock, which is always taken with interrupts
hard-disabled.
Signed-off-by: Paul Mackerras <paulus@samba.org>
---
arch/powerpc/include/asm/paca.h | 3 ++-
arch/powerpc/kernel/asm-offsets.c | 2 ++
arch/powerpc/kernel/exceptions-64s.S | 21 +++++++++++++++++++++
arch/powerpc/mm/slb.c | 10 +++++++++-
4 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
index c8a3cbf..17b29b1 100644
--- a/arch/powerpc/include/asm/paca.h
+++ b/arch/powerpc/include/asm/paca.h
@@ -105,7 +105,8 @@ struct paca_struct {
u8 soft_enabled; /* irq soft-enable flag */
u8 hard_enabled; /* set if irqs are enabled in MSR */
u8 io_sync; /* writel() needs spin_unlock sync */
- u8 perf_counter_pending; /* PM interrupt while soft-disabled */
+ u8 perf_counter_pending; /* perf_counter stuff needs wakeup */
+ u8 in_pmu_nmi; /* PM interrupt while soft-disabled */
/* Stuff for accurate time accounting */
u64 user_time; /* accumulated usermode TB ticks */
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index 561b646..5347780 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -130,6 +130,7 @@ int main(void)
DEFINE(PACASOFTIRQEN, offsetof(struct paca_struct, soft_enabled));
DEFINE(PACAHARDIRQEN, offsetof(struct paca_struct, hard_enabled));
DEFINE(PACAPERFPEND, offsetof(struct paca_struct, perf_counter_pending));
+ DEFINE(PACAPERFNMI, offsetof(struct paca_struct, in_pmu_nmi));
DEFINE(PACACONTEXTID, offsetof(struct paca_struct, context.id));
#ifdef CONFIG_PPC_MM_SLICES
DEFINE(PACALOWSLICESPSIZE, offsetof(struct paca_struct,
@@ -398,6 +399,7 @@ int main(void)
DEFINE(VCPU_TIMING_LAST_ENTER_TBL, offsetof(struct kvm_vcpu,
arch.timing_last_enter.tv32.tbl));
#endif
+ DEFINE(SIGSEGV, SIGSEGV);
return 0;
}
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index eb89811..02d96b0 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -722,6 +722,12 @@ _STATIC(do_hash_page)
std r3,_DAR(r1)
std r4,_DSISR(r1)
+#ifdef CONFIG_PERF_COUNTERS
+ lbz r0,PACAPERFNMI(r13)
+ cmpwi r0,0
+ bne 77f /* if we don't want to hash_page now */
+#endif /* CONFIG_PERF_COUNTERS */
+
andis. r0,r4,0xa450 /* weird error? */
bne- handle_page_fault /* if not, try to insert a HPTE */
BEGIN_FTR_SECTION
@@ -833,6 +839,21 @@ handle_page_fault:
bl .low_hash_fault
b .ret_from_except
+#ifdef CONFIG_PERF_COUNTERS
+/*
+ * We come here as a result of a DSI when accessing memory (possibly
+ * user memory) inside a PMU interrupt that occurred while interrupts
+ * were soft-disabled, so we just want to invoke the exception handler
+ * for the access, or panic if there isn't a handler.
+ */
+77: bl .save_nvgprs
+ mr r4,r3
+ addi r3,r1,STACK_FRAME_OVERHEAD
+ li r5,SIGSEGV
+ bl .bad_page_fault
+ b .ret_from_except
+#endif
+
/* here we have a segment miss */
do_ste_alloc:
bl .ste_allocate /* try to insert stab entry */
diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c
index 3b52c80..ebfb8df 100644
--- a/arch/powerpc/mm/slb.c
+++ b/arch/powerpc/mm/slb.c
@@ -187,12 +187,20 @@ static inline int esids_match(unsigned long addr1, unsigned long addr2)
/* Flush all user entries from the segment table of the current processor. */
void switch_slb(struct task_struct *tsk, struct mm_struct *mm)
{
- unsigned long offset = get_paca()->slb_cache_ptr;
+ unsigned long offset;
unsigned long slbie_data = 0;
unsigned long pc = KSTK_EIP(tsk);
unsigned long stack = KSTK_ESP(tsk);
unsigned long unmapped_base;
+ /*
+ * We need interrupts hard-disabled here, not just soft-disabled,
+ * so that a PMU interrupt can't occur, which might try to access
+ * user memory (to get a stack trace) and possible cause an SLB miss
+ * which would update the slb_cache/slb_cache_ptr fields in the PACA.
+ */
+ hard_irq_disable();
+ offset = get_paca()->slb_cache_ptr;
if (!cpu_has_feature(CPU_FTR_NO_SLBIE_B) &&
offset <= SLB_CACHE_ENTRIES) {
int i;
--
1.6.0.4
^ permalink raw reply related
* Re: [Bugme-new] [Bug 13304] New: ehci_hcd module causing problems in using usb head phone
From: Alan Stern @ 2009-06-27 1:33 UTC (permalink / raw)
To: Geoff Levand; +Cc: linuxppc-dev, USB list, abhishekkumar, bugme-daemon
In-Reply-To: <4A454616.7040501@am.sony.com>
On Fri, 26 Jun 2009, Geoff Levand wrote:
> > Where is the information about the hardware errata you mentioned?
>
> Sorry, I should have mentioned it. Unfortunately, that info is not
> in the public as far as I know. I think only someone with access
> to those will be able to work on this fix.
Okay, then I guess there's nothing more I can do regarding Bug #13304.
Can you take it over?
Alan Stern
^ permalink raw reply
* Re: [PATCH 1/4] of/mdio: Add fixed link support
From: Anton Vorontsov @ 2009-06-27 0:11 UTC (permalink / raw)
To: Grant Likely; +Cc: netdev, Li Yang, Andy Fleming, David Miller, linuxppc-dev
In-Reply-To: <fa686aa40906261633y502f0ed2jebc58f9160c683ce@mail.gmail.com>
On Fri, Jun 26, 2009 at 05:33:26PM -0600, Grant Likely wrote:
> On Fri, Jun 26, 2009 at 4:29 PM, Anton
> Vorontsov<avorontsov@ru.mvista.com> wrote:
> > Currently the fixed link support is broken for all OF ethernet drivers,
> > an "OF MDIO rework" removed most of the support. Instead of re-adding
> > fixed-link stuff to the drivers, add the support to a framework, so we
> > won't duplicate any code.
> >
> > With this patch, if a node pointer is NULL, then of_phy_connect() will
> > try to find ethernet device's node, then will look for fixed-link
> > property, and if specified, it connects PHY as usual, via bus_id (fixed
> > link PHYs do not have any device tree nodes associated with them).
> >
> > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
>
> Ugh. I do not like this approach. I did not intend to break fixed
> links, but I do not think that this approach is the right fix. There
> are several problems.
>
> I don't like the fixed.c approach of creating a dummy phy to begin
> with. I think it is an abuse of the device model to register a dummy
> mii_bus and have dummy devices registered on it. It is a lot of code
> for what should be a very simple thing. In particular, if a PHY is
> not specified, then the driver should use a static link configuration.
> This is trivial to implement in an Ethernet driver and I do not think
> the dummy phy adds anything.
Dummy PHYs add more than you think, for example you'll have to
refactor or duplicate the code that is responsible for MAC settings
for a given mode (10/100/100, duplex, pause), and you'll have to
do netif_carrier_* handling yourself.
Not a problem per se, but you'll have to address this, and you'll
have two paths in the drivers.
> It hooks into the initialization path of *all* OF enabled net drivers,
> whether it wants it or not. ie. The MPC5200 FEC driver does not want
> it because the fixed-link property is not part of the mpc5200-fec
> binding; it uses a current-speed property instead. 'fixed-link' has
> not been agreed upon to be applicable to all Ethernet bindings, and
> I'm not convinced that the format of it won't need to be changed for
> future Ethernet bindings. A function for parsing fixed-link should be
> a library function that a driver can choose to call out to. It should
> not be welded into the init path.
>
> I also think parsing the device tree at device open time (when
> of_phy_connect is usually called) is best to be avoided. fixed-link
> parsing should really happen at probe time and the values cached IMHO.
> It's probably not significant, but I'd like to keep device tree reads
> constrained in the cold path (driver probe time) as opposed to the hot
> (or slightly less cool) device open path.
open() time isn't a hot path at all.
> Instead, I think that each driver should be more graceful about
> missing phy pointers and the init path should call out to a fixed-link
> parser function that sets the initial link settings. Probably less
> than 5 lines of code per driver.
>
> I'm sorry about breaking it. It was my fault, and I'd be happy to fix
> it if you'd like me to,
Please do.
> but I don't think that this patch is the right
> approach.
OK, fine by me if you think you can do this stuff better. :-)
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: [PATCH 0/4 for 2.6.31] NET: Revive fixed link support
From: Grant Likely @ 2009-06-26 23:35 UTC (permalink / raw)
To: avorontsov; +Cc: netdev, Li Yang, Andy Fleming, David Miller, linuxppc-dev
In-Reply-To: <20090626222900.GA14594@oksana.dev.rtsoft.ru>
On Fri, Jun 26, 2009 at 4:29 PM, Anton
Vorontsov<avorontsov@ru.mvista.com> wrote:
> Hi all,
>
> The fixed link support is broken since The Big OF MDIO Rework,
> the rework simply removed most of the code that was needed for
> fixed links.
>
> Too bad I didn't notice this earlier, I saw a bunch of patches
> on the ml, but unfortunately I didn't look very close presuming
> that Grant knew what he was doing. :-) And obviously I didn't
> test linux-next on anything that requires a fixed link.
Apparently I didn't. sorry.
> Anyway, here are four patches. The first one adds the fixed link
> support to a framework, otherwise we'd duplicate the code across
> the drivers (as we did previously), and I tried to keep drivers'
> changes minimal.
As I described in my previous email, I don't think this is a good
approach and I don't think it should be merged.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH 1/4] of/mdio: Add fixed link support
From: Grant Likely @ 2009-06-26 23:33 UTC (permalink / raw)
To: Anton Vorontsov; +Cc: netdev, Li Yang, Andy Fleming, David Miller, linuxppc-dev
In-Reply-To: <20090626222945.GA32487@oksana.dev.rtsoft.ru>
On Fri, Jun 26, 2009 at 4:29 PM, Anton
Vorontsov<avorontsov@ru.mvista.com> wrote:
> Currently the fixed link support is broken for all OF ethernet drivers,
> an "OF MDIO rework" removed most of the support. Instead of re-adding
> fixed-link stuff to the drivers, add the support to a framework, so we
> won't duplicate any code.
>
> With this patch, if a node pointer is NULL, then of_phy_connect() will
> try to find ethernet device's node, then will look for fixed-link
> property, and if specified, it connects PHY as usual, via bus_id (fixed
> link PHYs do not have any device tree nodes associated with them).
>
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
Ugh. I do not like this approach. I did not intend to break fixed
links, but I do not think that this approach is the right fix. There
are several problems.
I don't like the fixed.c approach of creating a dummy phy to begin
with. I think it is an abuse of the device model to register a dummy
mii_bus and have dummy devices registered on it. It is a lot of code
for what should be a very simple thing. In particular, if a PHY is
not specified, then the driver should use a static link configuration.
This is trivial to implement in an Ethernet driver and I do not think
the dummy phy adds anything.
It hooks into the initialization path of *all* OF enabled net drivers,
whether it wants it or not. ie. The MPC5200 FEC driver does not want
it because the fixed-link property is not part of the mpc5200-fec
binding; it uses a current-speed property instead. 'fixed-link' has
not been agreed upon to be applicable to all Ethernet bindings, and
I'm not convinced that the format of it won't need to be changed for
future Ethernet bindings. A function for parsing fixed-link should be
a library function that a driver can choose to call out to. It should
not be welded into the init path.
I also think parsing the device tree at device open time (when
of_phy_connect is usually called) is best to be avoided. fixed-link
parsing should really happen at probe time and the values cached IMHO.
It's probably not significant, but I'd like to keep device tree reads
constrained in the cold path (driver probe time) as opposed to the hot
(or slightly less cool) device open path.
Instead, I think that each driver should be more graceful about
missing phy pointers and the init path should call out to a fixed-link
parser function that sets the initial link settings. Probably less
than 5 lines of code per driver.
I'm sorry about breaking it. It was my fault, and I'd be happy to fix
it if you'd like me to, but I don't think that this patch is the right
approach.
g.
> ---
> =A0drivers/of/of_mdio.c | =A0 45 ++++++++++++++++++++++++++++++++++++++++=
+----
> =A01 files changed, 41 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index aee967d..cfd876a 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -9,6 +9,10 @@
> =A0* out of the OpenFirmware device tree and using it to populate an mii_=
bus.
> =A0*/
>
> +#include <linux/kernel.h>
> +#include <linux/device.h>
> +#include <linux/netdevice.h>
> +#include <linux/err.h>
> =A0#include <linux/phy.h>
> =A0#include <linux/of.h>
> =A0#include <linux/of_mdio.h>
> @@ -129,11 +133,44 @@ struct phy_device *of_phy_connect(struct net_device=
*dev,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0void (=
*hndlr)(struct net_device *), u32 flags,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0phy_in=
terface_t iface)
> =A0{
> - =A0 =A0 =A0 struct phy_device *phy =3D of_phy_find_device(phy_np);
> + =A0 =A0 =A0 struct phy_device *phy =3D NULL;
> +
> + =A0 =A0 =A0 if (phy_np) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 int ret;
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 phy =3D of_phy_find_device(phy_np);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!phy)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return NULL;
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 ret =3D phy_connect_direct(dev, phy, hndlr,=
flags, iface);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (ret)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return NULL;
> + =A0 =A0 =A0 } else if (dev->dev.parent) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct device_node *net_np;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 const u32 *phy_id;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 char *bus_id;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 int sz;
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 net_np =3D dev_archdata_get_node(&dev->dev.=
parent->archdata);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!net_np)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return NULL;
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 phy_id =3D of_get_property(net_np, "fixed-l=
ink", &sz);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!phy_id || sz < sizeof(*phy_id))
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return NULL;
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 bus_id =3D kasprintf(GFP_KERNEL, PHY_ID_FMT=
, "0", phy_id[0]);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!bus_id) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_err(&dev->dev, "could n=
ot allocate memory\n");
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return NULL;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
>
> - =A0 =A0 =A0 if (!phy)
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 return NULL;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 phy =3D phy_connect(dev, bus_id, hndlr, 0, =
iface);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 kfree(bus_id);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (IS_ERR(phy))
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return NULL;
> + =A0 =A0 =A0 }
>
> - =A0 =A0 =A0 return phy_connect_direct(dev, phy, hndlr, flags, iface) ? =
NULL : phy;
> + =A0 =A0 =A0 return phy;
> =A0}
> =A0EXPORT_SYMBOL(of_phy_connect);
> --
> 1.6.3.1
>
>
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* [PATCH 4/4] fs_enet: Revive fixed link support
From: Anton Vorontsov @ 2009-06-26 22:29 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Li Yang, Andy Fleming, linuxppc-dev
In-Reply-To: <20090626222900.GA14594@oksana.dev.rtsoft.ru>
Since commit aa73832c5a80d6c52c69b18af858d88fa595dd3c ("Rework
fs_enet driver to use of_mdio infrastructure") the fixed-link support
is broken in the fs_enet driver.
This patch fixes the support by removing a check for phy_node, today
of_phy_connect() tries to find fixed PHY if phy_node appears to be
NULL.
Also set netdev parent device via SET_NETDEV_DEV() call, this is needed
so that OF MDIO core could find a node pointer for a device.
Plus, fix "if (IS_ERR(phydev))" check, in case of errors,
of_phy_connect() returns NULL, not ERR_PTR as phy_connect().
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
drivers/net/fs_enet/fs_enet-main.c | 14 +++++---------
1 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c
index b892c3a..1fea39e 100644
--- a/drivers/net/fs_enet/fs_enet-main.c
+++ b/drivers/net/fs_enet/fs_enet-main.c
@@ -754,15 +754,10 @@ static int fs_init_phy(struct net_device *dev)
fep->oldlink = 0;
fep->oldspeed = 0;
fep->oldduplex = -1;
- if(fep->fpi->phy_node)
- phydev = of_phy_connect(dev, fep->fpi->phy_node,
- &fs_adjust_link, 0,
- PHY_INTERFACE_MODE_MII);
- else {
- printk("No phy bus ID specified in BSP code\n");
- return -EINVAL;
- }
- if (IS_ERR(phydev)) {
+
+ phydev = of_phy_connect(dev, fep->fpi->phy_node, &fs_adjust_link, 0,
+ PHY_INTERFACE_MODE_MII);
+ if (!phydev) {
printk(KERN_ERR "%s: Could not attach to PHY\n", dev->name);
return PTR_ERR(phydev);
}
@@ -1005,6 +1000,7 @@ static int __devinit fs_enet_probe(struct of_device *ofdev,
goto out_free_fpi;
}
+ SET_NETDEV_DEV(ndev, &ofdev->dev);
dev_set_drvdata(&ofdev->dev, ndev);
fep = netdev_priv(ndev);
--
1.6.3.1
^ permalink raw reply related
* [PATCH 3/4] ucc_geth: Revive fixed link support
From: Anton Vorontsov @ 2009-06-26 22:29 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Li Yang, Andy Fleming, linuxppc-dev
In-Reply-To: <20090626222900.GA14594@oksana.dev.rtsoft.ru>
Since commit 0b9da337dca972e7a4144e298ec3adb8f244d4a4 ("Rework
ucc_geth driver to use of_mdio infrastructure") the fixed-link
support is broken.
This patch fixes the support by removing !ug_info->phy_node check,
today the of_phy_connect() will try to find a fixed PHY if the
phy_node appears to be NULL.
Also, remove an old fixed-link code that we don't use any longer.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
drivers/net/ucc_geth.c | 18 +++---------------
1 files changed, 3 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index 40c6eba..2dc83b1 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -1590,9 +1590,6 @@ static int init_phy(struct net_device *dev)
priv->oldspeed = 0;
priv->oldduplex = -1;
- if (!ug_info->phy_node)
- return 0;
-
phydev = of_phy_connect(dev, ug_info->phy_node, &adjust_link, 0,
priv->phy_interface);
if (!phydev) {
@@ -3608,9 +3605,7 @@ static int ucc_geth_probe(struct of_device* ofdev, const struct of_device_id *ma
struct ucc_geth_private *ugeth = NULL;
struct ucc_geth_info *ug_info;
struct resource res;
- struct device_node *phy;
int err, ucc_num, max_speed = 0;
- const u32 *fixed_link;
const unsigned int *prop;
const char *sprop;
const void *mac_addr;
@@ -3708,15 +3703,8 @@ static int ucc_geth_probe(struct of_device* ofdev, const struct of_device_id *ma
ug_info->uf_info.regs = res.start;
ug_info->uf_info.irq = irq_of_parse_and_map(np, 0);
- fixed_link = of_get_property(np, "fixed-link", NULL);
- if (fixed_link) {
- phy = NULL;
- } else {
- phy = of_parse_phandle(np, "phy-handle", 0);
- if (phy == NULL)
- return -ENODEV;
- }
- ug_info->phy_node = phy;
+
+ ug_info->phy_node = of_parse_phandle(np, "phy-handle", 0);
/* Find the TBI PHY node. If it's not there, we don't support SGMII */
ug_info->tbi_node = of_parse_phandle(np, "tbi-handle", 0);
@@ -3725,7 +3713,7 @@ static int ucc_geth_probe(struct of_device* ofdev, const struct of_device_id *ma
prop = of_get_property(np, "phy-connection-type", NULL);
if (!prop) {
/* handle interface property present in old trees */
- prop = of_get_property(phy, "interface", NULL);
+ prop = of_get_property(ug_info->phy_node, "interface", NULL);
if (prop != NULL) {
phy_interface = enet_to_phy_interface[*prop];
max_speed = enet_to_speed[*prop];
--
1.6.3.1
^ permalink raw reply related
* [PATCH 2/4] gianfar: Revive fixed link support
From: Anton Vorontsov @ 2009-06-26 22:29 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Li Yang, Andy Fleming, linuxppc-dev
In-Reply-To: <20090626222900.GA14594@oksana.dev.rtsoft.ru>
Since commit fe192a49118f5b1272317d60c7930ece4e13ae49 ("Rework gianfar
driver to use of_mdio infrastructure") the fixed-link support is
broken, the driver oopses at init_phy():
Unable to handle kernel paging request for data at address 0x000000e4
Faulting instruction address: 0xc01cf298
Oops: Kernel access of bad area, sig: 11 [#1]
[...]
NIP [c01cf298] init_phy+0x80/0xdc
LR [c01cf250] init_phy+0x38/0xdc
Call Trace:
[cf81fe80] [c01d1cf8] gfar_enet_open+0x6c/0x19c
[cf81fea0] [c024494c] dev_open+0xfc/0x134
[cf81fec0] [c0242edc] dev_change_flags+0x84/0x1ac
[cf81fee0] [c0399ee0] ic_open_devs+0x168/0x2d8
[cf81ff20] [c039b2e8] ip_auto_config+0x90/0x2a4
[cf81ff60] [c0003884] do_one_initcall+0x34/0x1a8
This patch fixes the oops, and removes phy_node checks (of_phy_connect
call now tries to find fixed PHYs if the phy_node is NULL).
Also, remove an old fixed-link code that we don't use any longer.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
drivers/net/gianfar.c | 21 +++++----------------
1 files changed, 5 insertions(+), 16 deletions(-)
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 4ae1d25..41b2107 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -262,15 +262,6 @@ static int gfar_of_init(struct net_device *dev)
priv->device_flags |= FSL_GIANFAR_DEV_HAS_MAGIC_PACKET;
priv->phy_node = of_parse_phandle(np, "phy-handle", 0);
- if (!priv->phy_node) {
- u32 *fixed_link;
-
- fixed_link = (u32 *)of_get_property(np, "fixed-link", NULL);
- if (!fixed_link) {
- err = -ENODEV;
- goto err_out;
- }
- }
/* Find the TBI PHY. If it's not there, we don't support SGMII */
priv->tbi_node = of_parse_phandle(np, "tbi-handle", 0);
@@ -657,13 +648,11 @@ static int init_phy(struct net_device *dev)
interface = gfar_get_interface(dev);
- if (priv->phy_node) {
- priv->phydev = of_phy_connect(dev, priv->phy_node, &adjust_link,
- 0, interface);
- if (!priv->phydev) {
- dev_err(&dev->dev, "error: Could not attach to PHY\n");
- return -ENODEV;
- }
+ priv->phydev = of_phy_connect(dev, priv->phy_node, &adjust_link, 0,
+ interface);
+ if (!priv->phydev) {
+ dev_err(&dev->dev, "could not attach to PHY\n");
+ return -ENODEV;
}
if (interface == PHY_INTERFACE_MODE_SGMII)
--
1.6.3.1
^ permalink raw reply related
* [PATCH 1/4] of/mdio: Add fixed link support
From: Anton Vorontsov @ 2009-06-26 22:29 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Li Yang, Andy Fleming, linuxppc-dev
In-Reply-To: <20090626222900.GA14594@oksana.dev.rtsoft.ru>
Currently the fixed link support is broken for all OF ethernet drivers,
an "OF MDIO rework" removed most of the support. Instead of re-adding
fixed-link stuff to the drivers, add the support to a framework, so we
won't duplicate any code.
With this patch, if a node pointer is NULL, then of_phy_connect() will
try to find ethernet device's node, then will look for fixed-link
property, and if specified, it connects PHY as usual, via bus_id (fixed
link PHYs do not have any device tree nodes associated with them).
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
drivers/of/of_mdio.c | 45 +++++++++++++++++++++++++++++++++++++++++----
1 files changed, 41 insertions(+), 4 deletions(-)
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index aee967d..cfd876a 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -9,6 +9,10 @@
* out of the OpenFirmware device tree and using it to populate an mii_bus.
*/
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/netdevice.h>
+#include <linux/err.h>
#include <linux/phy.h>
#include <linux/of.h>
#include <linux/of_mdio.h>
@@ -129,11 +133,44 @@ struct phy_device *of_phy_connect(struct net_device *dev,
void (*hndlr)(struct net_device *), u32 flags,
phy_interface_t iface)
{
- struct phy_device *phy = of_phy_find_device(phy_np);
+ struct phy_device *phy = NULL;
+
+ if (phy_np) {
+ int ret;
+
+ phy = of_phy_find_device(phy_np);
+ if (!phy)
+ return NULL;
+
+ ret = phy_connect_direct(dev, phy, hndlr, flags, iface);
+ if (ret)
+ return NULL;
+ } else if (dev->dev.parent) {
+ struct device_node *net_np;
+ const u32 *phy_id;
+ char *bus_id;
+ int sz;
+
+ net_np = dev_archdata_get_node(&dev->dev.parent->archdata);
+ if (!net_np)
+ return NULL;
+
+ phy_id = of_get_property(net_np, "fixed-link", &sz);
+ if (!phy_id || sz < sizeof(*phy_id))
+ return NULL;
+
+ bus_id = kasprintf(GFP_KERNEL, PHY_ID_FMT, "0", phy_id[0]);
+ if (!bus_id) {
+ dev_err(&dev->dev, "could not allocate memory\n");
+ return NULL;
+ }
- if (!phy)
- return NULL;
+ phy = phy_connect(dev, bus_id, hndlr, 0, iface);
+ kfree(bus_id);
+ if (IS_ERR(phy))
+ return NULL;
+ }
- return phy_connect_direct(dev, phy, hndlr, flags, iface) ? NULL : phy;
+ return phy;
}
EXPORT_SYMBOL(of_phy_connect);
--
1.6.3.1
^ permalink raw reply related
* [PATCH 0/4 for 2.6.31] NET: Revive fixed link support
From: Anton Vorontsov @ 2009-06-26 22:29 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Li Yang, Andy Fleming, linuxppc-dev
Hi all,
The fixed link support is broken since The Big OF MDIO Rework,
the rework simply removed most of the code that was needed for
fixed links.
Too bad I didn't notice this earlier, I saw a bunch of patches
on the ml, but unfortunately I didn't look very close presuming
that Grant knew what he was doing. :-) And obviously I didn't
test linux-next on anything that requires a fixed link.
Anyway, here are four patches. The first one adds the fixed link
support to a framework, otherwise we'd duplicate the code across
the drivers (as we did previously), and I tried to keep drivers'
changes minimal.
Patches 2 and 3 are for the drivers that I tested in run-time,
with normal PHYs, and fixed links.
Patch #4 was build-tested, but I believe it should work in
run-time too.
Thanks,
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: [Bugme-new] [Bug 13304] New: ehci_hcd module causing problems in using usb head phone
From: Geoff Levand @ 2009-06-26 22:05 UTC (permalink / raw)
To: Alan Stern; +Cc: linuxppc-dev, USB list, abhishekkumar, bugme-daemon
In-Reply-To: <Pine.LNX.4.44L0.0906261638370.4155-100000@iolanthe.rowland.org>
On 06/26/2009 01:42 PM, Alan Stern wrote:
> On Fri, 26 Jun 2009, Geoff Levand wrote:
>
>> There is a known but yet unfixed bug for the PS3's EHCI Async Periodic
>> (typically audio recording) device handling. There are a few related
>> hardware errata that I have not yet implemented driver fixes for that
>> I think are causing it.
>>
>> I guess it is the same problem as reported by Abhishek since Andrew's
>> dmesg (http://www.osl.iu.edu/~afriedle/dmesg.txt) shows similar
>> results.
>
> Yes, it definitely looks the same.
>
>> More info is here:
>>
>> http://ozlabs.org/pipermail/cbe-oss-dev/2009-February/006365.html
>>
>> I have bought a ART Tube USB device but have not had time to fix
>> this bug. It is on my todo list. Please feel free to make an
>> attempt.
>
> Where is the information about the hardware errata you mentioned?
Sorry, I should have mentioned it. Unfortunately, that info is not
in the public as far as I know. I think only someone with access
to those will be able to work on this fix.
-Geoff
^ permalink raw reply
* Re: [Bugme-new] [Bug 13304] New: ehci_hcd module causing problems in using usb head phone
From: Alan Stern @ 2009-06-26 20:42 UTC (permalink / raw)
To: Geoff Levand; +Cc: linuxppc-dev, USB list, abhishekkumar, bugme-daemon
In-Reply-To: <4A450207.5000502@am.sony.com>
On Fri, 26 Jun 2009, Geoff Levand wrote:
> There is a known but yet unfixed bug for the PS3's EHCI Async Periodic
> (typically audio recording) device handling. There are a few related
> hardware errata that I have not yet implemented driver fixes for that
> I think are causing it.
>
> I guess it is the same problem as reported by Abhishek since Andrew's
> dmesg (http://www.osl.iu.edu/~afriedle/dmesg.txt) shows similar
> results.
Yes, it definitely looks the same.
> More info is here:
>
> http://ozlabs.org/pipermail/cbe-oss-dev/2009-February/006365.html
>
> I have bought a ART Tube USB device but have not had time to fix
> this bug. It is on my todo list. Please feel free to make an
> attempt.
Where is the information about the hardware errata you mentioned?
Alan Stern
^ permalink raw reply
* Re: [Bugme-new] [Bug 13304] New: ehci_hcd module causing problems in using usb head phone
From: Geoff Levand @ 2009-06-26 17:14 UTC (permalink / raw)
To: Alan Stern; +Cc: linuxppc-dev, USB list, abhishekkumar, bugme-daemon
In-Reply-To: <Pine.LNX.4.44L0.0906261140540.2965-100000@iolanthe.rowland.org>
Hi,
On 06/26/2009 08:54 AM, Alan Stern wrote:
> On Fri, 26 Jun 2009, abhishekkumar wrote:
>>
>> bus ps3_system_bus, device sb_05 (driver 10 Dec 2004)
>> PS3 EHCI Host Controller
>> EHCI ff.ff, hcd state 1
>> structural params 0xffffffff
>> capability params 0xffffffff
>> status ffffffff Async Periodic Recl Halt IAA FATAL FLR PCD ERR INT
>> command ffffffff park=3 ithresh=63 LReset IAAD Async Periodic period=?? Reset R
>> intrenable ffffffff IAA FATAL FLR PCD ERR INT
>> uframe ffffffff
>> port 1 status ffffffff POWER OWNER sig=? RESET SUSPEND RESUME OCC OC PEC PE CSC
>> port 2 status ffffffff POWER OWNER sig=? RESET SUSPEND RESUME OCC OC PEC PE CSC
>> irq normal 30832 err 30 reclaim 84 (lost 1)
>> complete 31221 unlink 10
>
> This is very bad. It indicates that the CPU was unable to communicate
> with the EHCI controller at all! All the memory-mapped I/O reads
> returned 0xffffffff. No wonder the keyboard and mouse stopped working.
>
> I have no idea what could have caused this to happen. Even if the
> controller had suffered a fatal error, you wouldn't see this. It looks
> like the bus's connection to the controller was turned off.
>
> I'm CC-ing the PS3 maintainer and mailing list. Maybe people there can
> help.
>
> Alan Stern
There is a known but yet unfixed bug for the PS3's EHCI Async Periodic
(typically audio recording) device handling. There are a few related
hardware errata that I have not yet implemented driver fixes for that
I think are causing it.
I guess it is the same problem as reported by Abhishek since Andrew's
dmesg (http://www.osl.iu.edu/~afriedle/dmesg.txt) shows similar
results.
More info is here:
http://ozlabs.org/pipermail/cbe-oss-dev/2009-February/006365.html
I have bought a ART Tube USB device but have not had time to fix
this bug. It is on my todo list. Please feel free to make an
attempt.
Alan, thanks for your effort on this so far, sorry you didn't know
about the previous report.
-Geoff
^ permalink raw reply
* Re: Trouble "Transferring control to Linux (at address 00000000)"
From: Mikhail Zaturenskiy @ 2009-06-26 17:12 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <4A44FD90.2040607@freescale.com>
Hi Scott,
> This isn't the denx list;
I've noticed :) but I'm still learning about this whole process so I
though I could get some general suggestions.
> what kernel version is that, and with what
> modifications from mainline?
Kernel is v2.6.30, I'm not yet familiar enough with it to know what's
been modified from mainline, just following instructions.
> Note that ep88xc.dts in mainline is intended for use with PlanetCore, whi=
ch
> is what ships on that board. =A0You may need to make modifications for it=
to
> work with u-boot (at the least, the IMMR base is probably different).
Hmm, this hasn't occurred to me, thank you for pointing it out.
> Also, make sure u-boot is properly updating the memory size in the device
> tree. =A0Can you dump the post-fixup device tree in u-boot?
Not sure, but I'll try to find out if that's possible. It'd certainly
answer a lot of questions...
Thanks,
Mikhail Zaturenskiy
^ permalink raw reply
* Re: Trouble "Transferring control to Linux (at address 00000000)"
From: Scott Wood @ 2009-06-26 16:55 UTC (permalink / raw)
To: Mikhail Zaturenskiy; +Cc: linuxppc-dev
In-Reply-To: <97dd5fd20906260913x351593c9raa429fd657eb42f0@mail.gmail.com>
Mikhail Zaturenskiy wrote:
> Hello,
>
> I'm trying to load Linux from U-Boot. I am using ELDK 4.2 with U-Boot
> 2009.03 on an EP88xC rev1.1 board (MPC885 cpu, 64MB RAM, 32MB FLASH).
> The linux kernel sources were obtained from instructions at
> "http://www.denx.de/wiki/view/DULG/LinuxConfiguration#Section_6.1."
> and the FDT blob (dtb) was generated from
> "linux-2.6-denx/arch/powerpc/boot/dts/ep88xc.dts". Below is my output
> from boot to hang:
This isn't the denx list; what kernel version is that, and with what
modifications from mainline?
Note that ep88xc.dts in mainline is intended for use with PlanetCore, which is
what ships on that board. You may need to make modifications for it to work
with u-boot (at the least, the IMMR base is probably different). This is why
u-boot should be maintaining its own repository of trees that it passes...
Also, make sure u-boot is properly updating the memory size in the device tree.
Can you dump the post-fixup device tree in u-boot?
-Scott
^ permalink raw reply
* Re: [PATCH] sky2: Fix checksum endianness
From: David Miller @ 2009-06-26 16:28 UTC (permalink / raw)
To: avorontsov; +Cc: netdev, shemminger, linux-kernel, linuxppc-dev
In-Reply-To: <20090626145159.GA2242@oksana.dev.rtsoft.ru>
From: Anton Vorontsov <avorontsov@ru.mvista.com>
Date: Fri, 26 Jun 2009 18:51:59 +0400
> sky2 driver on PowerPC targets floods kernel log with following errors:
>
> eth1: hw csum failure.
> Call Trace:
> [ef84b8a0] [c00075e4] show_stack+0x50/0x160 (unreliable)
> [ef84b8d0] [c02fa178] netdev_rx_csum_fault+0x3c/0x5c
> [ef84b8f0] [c02f6920] __skb_checksum_complete_head+0x7c/0x84
> [ef84b900] [c02f693c] __skb_checksum_complete+0x14/0x24
> [ef84b910] [c0337e08] tcp_v4_rcv+0x4c8/0x6f8
> [ef84b940] [c031a9c8] ip_local_deliver+0x98/0x210
> [ef84b960] [c031a788] ip_rcv+0x38c/0x534
> [ef84b990] [c0300338] netif_receive_skb+0x260/0x36c
> [ef84b9c0] [c025de00] sky2_poll+0x5dc/0xcf8
> [ef84ba20] [c02fb7fc] net_rx_action+0xc0/0x144
>
> The NIC is Yukon-2 EC chip revision 1.
>
> Converting checksum field from le16 to CPU byte order fixes the issue.
>
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
Applied, thank you!
^ 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