LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] PowerPC 440EPx/GRx fix memory size calculation
From: Valentine @ 2009-03-11  1:40 UTC (permalink / raw)
  To: Mikhail Zolotaryov; +Cc: linuxppc-dev
In-Reply-To: <49B6D427.5050600@lebon.org.ua>

Mikhail Zolotaryov wrote:
> Valentine Barshak wrote:
>> According to the AMCC 440EPX/GRX user manual,
>> the Chip Select width is always fixed at 1 bit no matter
>> what is actually read from register DDR_10.
>
> Well, from my point of view original kernel code is correct in this part.
>
> Adding one bit into memory address means multiplying memory size by 2 
> i.e. cs=2. The question is: is Chip Select bit used in memory address. 
> ChipSelect input of memory chip enables or disabled it, so if we have 
> only one BankSel installed/connected (DDR0_10[22:23] is 01 or 10) 
> there's no need to use Chip Select bit in an address. On the contrary, 
> if both BankSel lines are connected (DDR0_10[22:23] is 11), to let 
> memory controller know which memory rank to use, Chip Select bit is 
> added into memory address. (and yes, if DDR0_10[22:23] is 00 - no 
> ranks installed, memory size is 0, cs=0)
Yes, you could phrase it that way. According to the PPC440EPx manual, 
the total memory size is calculated based on the following formula:
memsize = cs * (1 << (col+row)) * bank * dpath;
So, if both chipselects are used, we add an extra bit to the memory 
address to distinguish between these chipselects.
There's nothing wrong with this part of the code.
The problem is that the controller is hardwired to use only one 
chipselect, even if both are enabled in the DDR0_10 on PPC440EPx/GRx 
processors.
So, the patch provides a workaround to always use single cs for 
440EPx/GRx (use predefined value instead of reading DDR0_10).
>
> Original kernel code use exactly the same logic as I described above. 
> Please suggest if it's wrong.
Thanks,
Valentine.

^ permalink raw reply

* [patch 16/18] powerpc: convert obsolete hw_interrupt_type to struct irq_chip
From: Thomas Gleixner @ 2009-03-11  0:46 UTC (permalink / raw)
  To: LKML; +Cc: Andrew Morton, Ingo Molnar, linuxppc-dev
In-Reply-To: <20090311003204.592133594@linutronix.de>

Impact: cleanup

Convert the last remaining users to struct irq_chip.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: linuxppc-dev@ozlabs.org
---
 arch/powerpc/include/asm/hw_irq.h     |    2 +-
 arch/powerpc/platforms/powermac/pic.h |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6/arch/powerpc/include/asm/hw_irq.h
===================================================================
--- linux-2.6.orig/arch/powerpc/include/asm/hw_irq.h
+++ linux-2.6/arch/powerpc/include/asm/hw_irq.h
@@ -129,7 +129,7 @@ static inline int irqs_disabled_flags(un
  * interrupt-retrigger: should we handle this via lost interrupts and IPIs
  * or should we not care like we do now ? --BenH.
  */
-struct hw_interrupt_type;
+struct irq_chip;
 
 #endif	/* __KERNEL__ */
 #endif	/* _ASM_POWERPC_HW_IRQ_H */
Index: linux-2.6/arch/powerpc/platforms/powermac/pic.h
===================================================================
--- linux-2.6.orig/arch/powerpc/platforms/powermac/pic.h
+++ linux-2.6/arch/powerpc/platforms/powermac/pic.h
@@ -3,7 +3,7 @@
 
 #include <linux/irq.h>
 
-extern struct hw_interrupt_type pmac_pic;
+extern struct irq_chip pmac_pic;
 
 extern void pmac_pic_init(void);
 extern int pmac_get_irq(void);

^ permalink raw reply

* [patch 08/18] powerpc: convert obsolete irq_desc_t to struct irq_desc
From: Thomas Gleixner @ 2009-03-11  0:45 UTC (permalink / raw)
  To: LKML; +Cc: Andrew Morton, Ingo Molnar, linuxppc-dev
In-Reply-To: <20090311003204.592133594@linutronix.de>

Impact: cleanup

Convert the last remaining users.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: linuxppc-dev@ozlabs.org
---
 arch/powerpc/kernel/irq.c            |    4 ++--
 arch/powerpc/platforms/iseries/irq.c |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

Index: linux-2.6/arch/powerpc/kernel/irq.c
===================================================================
--- linux-2.6.orig/arch/powerpc/kernel/irq.c
+++ linux-2.6/arch/powerpc/kernel/irq.c
@@ -171,7 +171,7 @@ int show_interrupts(struct seq_file *p, 
 {
 	int i = *(loff_t *)v, j;
 	struct irqaction *action;
-	irq_desc_t *desc;
+	struct irq_desc *desc;
 	unsigned long flags;
 
 	if (i == 0) {
@@ -1038,7 +1038,7 @@ arch_initcall(irq_late_init);
 static int virq_debug_show(struct seq_file *m, void *private)
 {
 	unsigned long flags;
-	irq_desc_t *desc;
+	struct irq_desc *desc;
 	const char *p;
 	char none[] = "none";
 	int i;
Index: linux-2.6/arch/powerpc/platforms/iseries/irq.c
===================================================================
--- linux-2.6.orig/arch/powerpc/platforms/iseries/irq.c
+++ linux-2.6/arch/powerpc/platforms/iseries/irq.c
@@ -214,7 +214,7 @@ void __init iSeries_activate_IRQs()
 	unsigned long flags;
 
 	for_each_irq (irq) {
-		irq_desc_t *desc = get_irq_desc(irq);
+		struct irq_desc *desc = get_irq_desc(irq);
 
 		if (desc && desc->chip && desc->chip->startup) {
 			spin_lock_irqsave(&desc->lock, flags);

^ permalink raw reply

* Re: [PATCH v5] introduce macro spin_event_timeout()
From: Josh Boyer @ 2009-03-11  0:44 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Scott Wood, linuxppc-dev, Timur Tabi
In-Reply-To: <1236729551.7086.26.camel@pasglop>

On Wed, Mar 11, 2009 at 10:59:11AM +1100, Benjamin Herrenschmidt wrote:
>On Tue, 2009-03-10 at 18:37 -0400, Josh Boyer wrote:
>> On Tue, Mar 10, 2009 at 05:33:08PM -0500, Scott Wood wrote:
>> > Timur Tabi wrote:
>> >> The macro spin_event_timeout() takes a condition and timeout value
>> >> (in microseconds) as parameters.  It spins until either the condition is true
>> >> or the timeout expires.  It returns zero if the timeout expires first, non-zero
>> >> otherwise.
>> >>
>> >> This primary purpose of this macro is to poll on a hardware register until a
>> >> status bit changes.  The timeout ensures that the loop still terminates if the
>> >> bit doesn't change as expected.  This macro makes it easier for driver
>> >> developers to perform this kind of operation properly.
>> >>
>> >> Signed-off-by: Timur Tabi <timur@freescale.com>
>> >> ---
>> >>
>> >> v5: ported to arch/powerpc, made it powerpc-specific, eliminated udelay
>> >
>> > Why make it powerpc-specific?  This would be nice to have in  
>> > arch-independent code.
>> 
>> That's just mean.  He already posted it to lkml and was told to make it
>> powerpc specific by Alan.
>
>And ? We can disagree with Alan...

Did I say Alan was right?  I'm just explaining why Timur probably posted it
as arch-specific.

josh

^ permalink raw reply

* Re: [PATCH v5] introduce macro spin_event_timeout()
From: Josh Boyer @ 2009-03-11  0:32 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, Timur Tabi
In-Reply-To: <49B6F0B2.70102@freescale.com>

On Tue, Mar 10, 2009 at 05:58:58PM -0500, Scott Wood wrote:
> Josh Boyer wrote:
>> On Tue, Mar 10, 2009 at 05:33:08PM -0500, Scott Wood wrote:
>>> Timur Tabi wrote:
>>>> The macro spin_event_timeout() takes a condition and timeout value
>>>> (in microseconds) as parameters.  It spins until either the condition is true
>>>> or the timeout expires.  It returns zero if the timeout expires first, non-zero
>>>> otherwise.
>>>>
>>>> This primary purpose of this macro is to poll on a hardware register until a
>>>> status bit changes.  The timeout ensures that the loop still terminates if the
>>>> bit doesn't change as expected.  This macro makes it easier for driver
>>>> developers to perform this kind of operation properly.
>>>>
>>>> Signed-off-by: Timur Tabi <timur@freescale.com>
>>>> ---
>>>>
>>>> v5: ported to arch/powerpc, made it powerpc-specific, eliminated udelay
>>> Why make it powerpc-specific?  This would be nice to have in   
>>> arch-independent code.
>>
>> That's just mean.  He already posted it to lkml and was told to make it
>> powerpc specific by Alan.
>
> Well, that's what happens when a discussion hops mailing lists with no  
> backreference. :-P
>
> I don't see anywhere where he says it should be architecture dependent,  
> but rather a general "I don't like this, get off my lawn!" response.
>
> I cannot agree with the "we shouldn't be encouraging this" sentiment;  
> people don't generally do spin loops because they're lazy[1], but rather  
> because the hardware demands it -- and it's hardly only on powerpc (much  
> less just "some Freescale drivers") that I've encountered hardware that  
> demands it, typiclally during reset/initialization or similarly non-hot  
> paths.  Why not provide something less likely to have bugs (the timeout  
> case is unlikely to be well tested), more easily seen when reviewing a  
> patch, and more likely to result in spin loops *with* a timeout rather  
> than without?

Excellent questions.  Did you send them to lkml and Alan?

josh

^ permalink raw reply

* Re: [PATCH v5] introduce macro spin_event_timeout()
From: Timur Tabi @ 2009-03-11  0:22 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Scott Wood, linuxppc-dev
In-Reply-To: <1236729551.7086.26.camel@pasglop>

On Tue, Mar 10, 2009 at 6:59 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:

> And ? We can disagree with Alan...

If you guys want to argue with Alan on lkml, please go ahead.  I could
use the support.

Alan did have one valid point though.  Determining how long to loop
for is architecture-specific.  Using jiffies is bad, because even one
jiffy is too long.  Adding a udelay() inside the loop means that it
only checks he condition every microsecond.  So the real solution is
to use keep looping until a certain amount of time has passed.  This
means using an architecture-specific timebase register.

Now we can create a generic version of the function that uses jiffies,
and then arch-specific versions where possible.  But Alan still needs
to be convinced.  I already posted a length rebuttal to his email, but
I haven't gotten a reply yet.

-- 
Timur Tabi
Linux kernel developer at Freescale

^ permalink raw reply

* Re: [PATCH v5] introduce macro spin_event_timeout()
From: Benjamin Herrenschmidt @ 2009-03-11  0:24 UTC (permalink / raw)
  To: Timur Tabi; +Cc: Scott Wood, linuxppc-dev
In-Reply-To: <ed82fe3e0903101722i610638e8le1f2e925095c8ba6@mail.gmail.com>

On Tue, 2009-03-10 at 19:22 -0500, Timur Tabi wrote:
> 
> Alan did have one valid point though.  Determining how long to loop
> for is architecture-specific.  Using jiffies is bad, because even one
> jiffy is too long.  Adding a udelay() inside the loop means that it
> only checks he condition every microsecond.  So the real solution is
> to use keep looping until a certain amount of time has passed.  This
> means using an architecture-specific timebase register.

> Now we can create a generic version of the function that uses jiffies,
> and then arch-specific versions where possible.  But Alan still needs
> to be convinced.  I already posted a length rebuttal to his email, but
> I haven't gotten a reply yet.
> 
There are several aspects here:

 - The amount of time to wait should be specified by the caller since
it's generally going to come from HW specs

 - The amount of time between the polls ... that could also be an
argument to the macro, not sure there

 - The precision of the actual wait calls... I vote for microseconds for
everything and udelay. The arch will do its best.

Cheers,
Ben.

^ permalink raw reply

* Re: RX problem in ibm_newemac driver
From: Benjamin Herrenschmidt @ 2009-03-11  0:21 UTC (permalink / raw)
  To: Felix Radensky; +Cc: netdev, linuxppc-dev list
In-Reply-To: <49B6FA3B.8030509@embedded-sol.com>

On Wed, 2009-03-11 at 01:39 +0200, Felix Radensky wrote:
> Benjamin Herrenschmidt wrote:
> > On Wed, 2009-03-11 at 00:14 +0200, Felix Radensky wrote:
> >   
> >> Yes, seems logical. U-boot has code to enable and disable loopback clock
> >> for 440SPE, 440EPX,440GRX,405EX, 460EX and 460GT.
> >>
> >> I can test patches on my board. Alternatively, I can try something myself
> >> if you can provide some guidance. I guess you are referring to the code 
> >> using
> >> EMAC_FTR_440GX_PHY_CLK_FIX and EMAC_FTR_440EP_PHY_CLK_FIX.
> >>     
> >
> > It would be nice if you could try something as I don't have anything to
> > test here.
> >
> > And yes, it's probably one of those 2 fixes that need to be extended.
> >
> > I'll have a look later today if I can find the 405EXr user manual and
> > give you more precise guidance.

>From the doc, it looks like it needs the 440 type workaround (and the
405EX as well). Can you try this patch:

emac: Fix clock control for 405EX and 405EXr chips

The EMAC variant in the 405EX and 405EXr chips needs the "440EP" type clock
control workaround to avoid lockups of the Rx side during reset.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

Index: linux-work/drivers/net/ibm_newemac/core.c
===================================================================
--- linux-work.orig/drivers/net/ibm_newemac/core.c	2009-03-11 11:13:37.000000000 +1100
+++ linux-work/drivers/net/ibm_newemac/core.c	2009-03-11 11:14:00.000000000 +1100
@@ -2594,6 +2594,9 @@ static int __devinit emac_init_config(st
 		if (of_device_is_compatible(np, "ibm,emac-460ex") ||
 		    of_device_is_compatible(np, "ibm,emac-460gt"))
 			dev->features |= EMAC_FTR_460EX_PHY_CLK_FIX;
+		if (of_device_is_compatible(np, "ibm,emac-405ex") ||
+		    of_device_is_compatible(np, "ibm,emac-405exr"))
+			dev->features |= EMAC_FTR_440EP_PHY_CLK_FIX;
 	} else if (of_device_is_compatible(np, "ibm,emac4")) {
 		dev->features |= EMAC_FTR_EMAC4;
 		if (of_device_is_compatible(np, "ibm,emac-440gx"))

^ permalink raw reply

* Re: [PATCH v5] introduce macro spin_event_timeout()
From: Benjamin Herrenschmidt @ 2009-03-10 23:59 UTC (permalink / raw)
  To: Josh Boyer; +Cc: Scott Wood, linuxppc-dev, Timur Tabi
In-Reply-To: <20090310223753.GB26415@zod.rchland.ibm.com>

On Tue, 2009-03-10 at 18:37 -0400, Josh Boyer wrote:
> On Tue, Mar 10, 2009 at 05:33:08PM -0500, Scott Wood wrote:
> > Timur Tabi wrote:
> >> The macro spin_event_timeout() takes a condition and timeout value
> >> (in microseconds) as parameters.  It spins until either the condition is true
> >> or the timeout expires.  It returns zero if the timeout expires first, non-zero
> >> otherwise.
> >>
> >> This primary purpose of this macro is to poll on a hardware register until a
> >> status bit changes.  The timeout ensures that the loop still terminates if the
> >> bit doesn't change as expected.  This macro makes it easier for driver
> >> developers to perform this kind of operation properly.
> >>
> >> Signed-off-by: Timur Tabi <timur@freescale.com>
> >> ---
> >>
> >> v5: ported to arch/powerpc, made it powerpc-specific, eliminated udelay
> >
> > Why make it powerpc-specific?  This would be nice to have in  
> > arch-independent code.
> 
> That's just mean.  He already posted it to lkml and was told to make it
> powerpc specific by Alan.

And ? We can disagree with Alan...

Ben.

^ permalink raw reply

* Re: [PATCH v5] introduce macro spin_event_timeout()
From: Benjamin Herrenschmidt @ 2009-03-10 23:58 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev
In-Reply-To: <1236723118-3577-1-git-send-email-timur@freescale.com>

On Tue, 2009-03-10 at 17:11 -0500, Timur Tabi wrote:
> The macro spin_event_timeout() takes a condition and timeout value
> (in microseconds) as parameters.  It spins until either the condition is true
> or the timeout expires.  It returns zero if the timeout expires first, non-zero
> otherwise.
> 
> This primary purpose of this macro is to poll on a hardware register until a
> status bit changes.  The timeout ensures that the loop still terminates if the
> bit doesn't change as expected.  This macro makes it easier for driver
> developers to perform this kind of operation properly.

I've missed the history here but why is that arch code ? it could be
used by more drivers... though there's always the idea that spinning is
bad :-)

Cheers,
Ben.

> Signed-off-by: Timur Tabi <timur@freescale.com>
> ---
> 
> v5: ported to arch/powerpc, made it powerpc-specific, eliminated udelay
> 
> v4: removed cpu_relax (redundant), changed timeout to unsigned long
> 
> v3: eliminated secondary evaluation of condition, replaced jiffies with udelay
> 
> v2: added cpu_relax and time_before
> 
>  arch/powerpc/include/asm/delay.h |   32 ++++++++++++++++++++++++++++++++
>  1 files changed, 32 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/delay.h b/arch/powerpc/include/asm/delay.h
> index f9200a6..aadec70 100644
> --- a/arch/powerpc/include/asm/delay.h
> +++ b/arch/powerpc/include/asm/delay.h
> @@ -2,6 +2,8 @@
>  #define _ASM_POWERPC_DELAY_H
>  #ifdef __KERNEL__
>  
> +#include <asm/time.h>
> +
>  /*
>   * Copyright 1996, Paul Mackerras.
>   *
> @@ -30,5 +32,35 @@ extern void udelay(unsigned long usecs);
>  #define mdelay(n)	udelay((n) * 1000)
>  #endif
>  
> +/**
> + * spin_event_timeout - spin until a condition gets true or a timeout elapses
> + * @condition: a C expression to evalate
> + * @timeout: timeout, in microseconds
> + *
> + * The process spins until the @condition evaluates to true (non-zero) or
> + * the @timeout elapses.
> + *
> + * This primary purpose of this macro is to poll on a hardware register
> + * until a status bit changes.  The timeout ensures that the loop still
> + * terminates if the bit never changes.
> + *
> + * The return value is non-zero if the condition evaluates to true first, or
> + * zero if the timeout elapses first.
> + */
> +#define spin_event_timeout(condition, timeout)			\
> +({								\
> +	unsigned long __start = get_tbl();			\
> +	unsigned long __loops = tb_ticks_per_usec * timeout;	\
> +	int __ret = 1;						\
> +	while (!(condition)) {					\
> +		if (tb_ticks_since(__start) > __loops) {	\
> +			__ret = 0;				\
> +			break;					\
> +		}						\
> +		cpu_relax();					\
> +	}							\
> +	__ret;							\
> +})
> +
>  #endif /* __KERNEL__ */
>  #endif /* _ASM_POWERPC_DELAY_H */

^ permalink raw reply

* [git pull] Please pull powerpc.git merge branch
From: Benjamin Herrenschmidt @ 2009-03-10 23:57 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linuxppc-dev list, Andrew Morton, Linux Kernel list

Hi Linus !

Here are some late fixes for 2.6.29. I've included a radeonfb/aty128fb commit
as it only affects a powerpc specific code path and solves a reported regression.
There's also an hvc_console commit that only affects powerpc pseries backends,
and also solves a regression. The rest are defconfig fixes and removing some
obsolete and non-building code from a pair of embedded platforms.
 
Cheers,
Ben.

The following changes since commit 99adcd9d67aaf04e28f5ae96df280f236bde4b66:
  Linus Torvalds (1):
        Merge branch 'fixes' of git://git.kernel.org/.../davej/cpufreq

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git merge

Benjamin Herrenschmidt (3):
      Merge commit 'gcl/merge' into merge
      hvc_console: Remove tty->low_latency on pseries backends
      radeonfb/aty128fb: Disable broken early resume hook for PowerBooks

Grant Likely (1):
      powerpc/4xx: Enable SERIAL_OF support by default for Virtex platforms

Guennadi Liakhovetski (1):
      powerpc: fix linkstation and storcenter compilation breakage

 arch/powerpc/configs/40x/virtex_defconfig        |    2 +-
 arch/powerpc/configs/44x/virtex5_defconfig       |    2 +-
 arch/powerpc/configs/linkstation_defconfig       |   36 ++++++++++++--------
 arch/powerpc/configs/storcenter_defconfig        |   35 ++++++++++++-------
 arch/powerpc/platforms/embedded6xx/linkstation.c |   38 ----------------------
 arch/powerpc/platforms/embedded6xx/storcenter.c  |   32 ------------------
 drivers/char/hvcs.c                              |    9 -----
 drivers/char/hvsi.c                              |    1 -
 drivers/video/aty/aty128fb.c                     |   10 +++++-
 drivers/video/aty/radeon_pm.c                    |   10 +++++-
 10 files changed, 64 insertions(+), 111 deletions(-)

^ permalink raw reply

* Re: TSEC/MDIO bug in recent kernels
From: Grant Likely @ 2009-03-10 23:57 UTC (permalink / raw)
  To: Johns Daniel; +Cc: linuxppc-dev, afleming
In-Reply-To: <ba5d9360903101555o3d6f5932k9292a006328c7161@mail.gmail.com>

On Tue, Mar 10, 2009 at 4:55 PM, Johns Daniel <johns.daniel@gmail.com> wrot=
e:
> For those of you who are running into this error:
> =A0 =A0 =A0 =A024520:01 not found
> =A0 =A0 =A0 =A0eth0: Could not attach to PHY
> =A0 =A0 =A0 =A0IP-Config: Failed to open eth0
> =A0 =A0 =A0 =A0IP-Config: Device `eth0' not found.
>
> There is a bug in recent kernels. I found it in 2.6.28.7:
> =A0 =A0 =A0 =A0linux/arch/powerpc/sysdev/fsl_soc.c:
> for_each_compatible_node(np, "mdio", "gianfar");
> should be:
> =A0 =A0 =A0 =A0linux/arch/powerpc/sysdev/fsl_soc.c:
> for_each_compatible_node(np, "mdio", "gianfar")

This code is gone in what will become 2.6.29, but the fix can be
backported into the stable tree.  Do you want to generate and send the
patch (and get your name in the commit message)?  Send the patch to
afleming@freescale.com, linuxppc-dev@ozlabs.org and to
stable@kernel.org.  Make sure you specify that this is applicable for
2.6.28 only.

g.


--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: C67x00 reset problems
From: Grant Likely @ 2009-03-10 23:42 UTC (permalink / raw)
  To: A. Nolson; +Cc: linuxppc-dev
In-Reply-To: <49B6F513.2090304@gmail.com>

Unfortunately, the 2.6.24-rc3 stuff in my git tree is really old.
I've been getting all of my recent work into mainline.  The CF driver
is in much better shape there.  The c67x00 driver is merged into
mainline, but I haven't tested it at all in the last year so I don't
know how well it will work.

Cheers,
g.

On Tue, Mar 10, 2009 at 5:17 PM, A. Nolson <alohanono@gmail.com> wrote:
> Hi,
>
> =A0I am using 2.6.24-rc3 ( secretlabs git) in an ML403 where I want to
> use the USB c67x00 based host with Peter Kosgaard driver. I am using it
> together with the sysace device for CF access ( in the ML403 those two
> share lines, but I managed to insert some logic to multiplex both
> devices). I have been able to use the c67300 without OS, but when I try
> to boot the kernel I am getting driver reset problems (the whole log is
> at the end)
>
> [ =A0 =A06.882540] c67x00 c67x00.0: Device reset
> failed
> [ =A0 =A06.889278] c67x00: probe of c67x00.0 failed with error 65531
>
> And of course the device cannot get probed. I think this has happened to
> other people before but I can't find the solution in the archive. My
> virtex_devices.c initialization looks like this:
>
> #define XPAR_C67x00_USB(num) { \
> =A0.name =3D "c67x00", \
> =A0.id =3D num, \
> =A0.num_resources =3D 2, \
> =A0.resource =3D (struct resource[]) { \
> =A0 =A0 =A0 =A0 { \
> =A0 =A0 =A0 =A0 .start =3D XPAR_C67X00_USB_PRH##num##_BASEADDR, \
> =A0 =A0 =A0 =A0 .end =3D XPAR_C67X00_USB_PRH##num##_BASEADDR + 0xf, \
> =A0 =A0 =A0 =A0 .flags =3D IORESOURCE_MEM, \
> =A0 =A0 =A0 =A0 }, \
> =A0 =A0 =A0 =A0 { \
> =A0 =A0 =A0 =A0 .start =A0=3D XPAR_OPB_INTC_0_SYSTEM_USB_HPI_INT_INTR, \
> =A0 =A0 =A0 =A0 .end =A0 =A0=3D XPAR_OPB_INTC_0_SYSTEM_USB_HPI_INT_INTR, =
\
> =A0 =A0 =A0 =A0 .flags =A0=3D IORESOURCE_IRQ, \
> =A0 =A0 =A0 =A0 =A0}, \
> =A0}, \
> =A0.dev.platform_data =3D &(struct c67x00_platform_data) { \
> =A0.sie_config =3D C67X00_SIE1_HOST | C67X00_SIE2_PERIPHERAL_A, \
> =A0.hpi_regstep =3D 0x02, /* A0 not connected on 16bit bus */ \
> =A0}, \
> }
>
> =A0/* Cypress USB C67x00 instances */
> #if defined(XPAR_C67X00_USB_PRH0_BASEADDR)
> =A0XPAR_C67x00_USB(0),
> #endif
>
>
> and my xparameters_ml403.h like this:
>
> /* Definitions for peripheral C67X00_USB */
> #define XPAR_C67X00_USB_PRH0_BASEADDR 0xA5000000
> #define XPAR_C67X00_USB_PRH0_HIGHADDR 0xA500FFFF
> #define XPAR_C67X00_USB_PRH1_BASEADDR 0xA5010000
> #define XPAR_C67X00_USB_PRH1_HIGHADDR 0xA501FFFF
> #define XPAR_C67X00_USB_PRH2_BASEADDR 0xA5020000
> #define XPAR_C67X00_USB_PRH2_HIGHADDR 0xA502FFFF
> #define XPAR_C67X00_USB_PRH3_BASEADDR 0xA5030000
> #define XPAR_C67X00_USB_PRH3_HIGHADDR 0xA503FFFF
>
> #define XPAR_OPB_INTC_0_SYSTEM_USB_HPI_INT_INTR 0
> #define XPAR_SYSTEM_USB_HPI_INT_MASK 0X000001
>
> I don't know what could be wrong here. Any clues?
>
> /Albert
>
>
>
> loaded at: =A0 =A0 00400000
> 0056419C
> board data at: 00562120
> 0056219C
> relocated to: =A00040406C
> 004040E8
> zimage at: =A0 =A0 00404E58
> 00561F12
> avail ram: =A0 =A0 00565000
> 04000000
>
>
> Linux/PPC load: console=3DttyUL0,57600 root=3D/dev/xsa2 rw
> init=3D/sbin/init
> Uncompressing
> Linux...done.
> Now booting the
> kernel
> [ =A0 =A00.000000] Linux version 2.6.24-rc3-dirty (xxx@xxx) (gcc vers
> n 4.2.2) #77 PREEMPT Mon Mar 9 18:31:59 CET
> 2009
> [ =A0 =A00.000000] Xilinx ML403 Reference System (Virtex-4
> FX)
> [ =A0 =A00.000000] Zone PFN
> ranges:
> [ =A0 =A00.000000] =A0 DMA =A0 =A0 =A0 =A0 =A0 =A0 0 ->
> 16384
> [ =A0 =A00.000000] =A0 Normal =A0 =A0 =A016384 ->
> 16384
> [ =A0 =A00.000000] =A0 HighMem =A0 =A0 16384 ->
> 16384
> [ =A0 =A00.000000] Movable zone start PFN for each
> node
> [ =A0 =A00.000000] early_node_map[1] active PFN
> ranges
> [ =A0 =A00.000000] =A0 =A0 0: =A0 =A0 =A0 =A00 ->
> 16384
> [ =A0 =A00.000000] Built 1 zonelists in Zone order, mobility grouping on.
> Total p
> es:
> 16256
> [ =A0 =A00.000000] Kernel command line: console=3DttyUL0,57600 root=3D/de=
v/xsa2
> rw ini
> /sbin/init
>
> [ =A0 =A00.000000] Xilinx INTC #0 at 0x41200000 mapped to
> 0xFDFFF000
> [ =A0 =A00.000000] PID hash table entries: 256 (order: 8, 1024
> bytes)
> [ =A0 =A00.000160] Console: colour dummy device
> 80x25
> [ =A0 =A00.000569] Dentry cache hash table entries: 8192 (order: 3, 32768
> bytes)
> [ =A0 =A00.001326] Inode-cache hash table entries: 4096 (order: 2, 16384
> bytes)
> [ =A0 =A00.013549] Memory: 61628k available (2352k kernel code, 796k data=
,
> 112k in
> , 0k
> highmem)
> [ =A0 =A00.013800] SLUB: Genslabs=3D11, HWalign=3D32, Order=3D0-1, MinObj=
ects=3D4,
> CPUs=3D1,
> odes=3D1
>
> [ =A0 =A00.035282] Mount-cache hash table entries:
> 512
> [ =A0 =A00.039567] net_namespace: 64
> bytes
> [ =A0 =A00.044545] NET: Registered protocol family
> 16
> [ =A0 =A00.051781] Registering
> spi1_board_info
> [ =A0 =A00.074960] usbcore: registered new interface driver
> usbfs
> [ =A0 =A00.075863] usbcore: registered new interface driver
> hub
> [ =A0 =A00.076911] usbcore: registered new device driver
> usb
> [ =A0 =A00.090812] NET: Registered protocol family
> 2
> [ =A0 =A00.100648] IP route cache hash table entries: 1024 (order: 0, 409=
6
> bytes)
> [ =A0 =A00.103337] TCP established hash table entries: 2048 (order: 2, 16=
384
> bytes
> [ =A0 =A00.103614] TCP bind hash table entries: 2048 (order: 1, 8192
> bytes)
> [ =A0 =A00.103770] TCP: Hash tables configured (established 2048 bind
> 2048)
> [ =A0 =A00.103798] TCP reno
> registered
> [ =A0 =A00.107443] sysctl table check failed: /kernel/l2cr .1.31 Missing
> strategy
> [ =A0 =A00.107511] Call
> Trace:
> [ =A0 =A00.107530] [c3c11de0] [c0008338] show_stack+0x50/0x188
> (unreliable)
> [ =A0 =A00.107606] [c3c11e30] [c003cf88]
> set_fail+0x50/0x68
> [ =A0 =A00.107675] [c3c11e50] [c003d5f8]
> sysctl_check_table+0x658/0x6ac
> [ =A0 =A00.107730] [c3c11ec0] [c003d60c]
> sysctl_check_table+0x66c/0x6ac
> [ =A0 =A00.107785] [c3c11f30] [c002907c]
> register_sysctl_table+0x64/0xe0
> [ =A0 =A00.107851] [c3c11f50] [c02ee750]
> register_ppc_htab_sysctl+0x18/0x2c
> [ =A0 =A00.107919] [c3c11f60] [c02e8230]
> kernel_init+0x10c/0x2a4
> [ =A0 =A00.107962] [c3c11ff0] [c0004c38]
> kernel_thread+0x44/0x60
> [ =A0 =A00.165612] Installing knfsd (copyright (C) 1996
> okir@monad.swb.de).
> [ =A0 =A00.168476] io scheduler noop
> registered
> [ =A0 =A00.168515] io scheduler anticipatory
> registered
> [ =A0 =A00.168538] io scheduler deadline
> registered
> [ =A0 =A00.168885] io scheduler cfq registered
> (default)
> [ =A0 =A00.763801] Generic RTC Driver
> v1.07
> [ =A0 =A00.764606] Serial: 8250/16550 driver $Revision: 1.90 $ 4 ports, I=
RQ
> sharin
> disabled
>
> [ =A0 =A00.773092] uartlite.0: ttyUL0 at MMIO 0x40600003 (irq =3D 5) is a
> uartlite
> [ =A0 =A00.773152] console [ttyUL0]
> enabled
> [ =A0 =A01.334309] RAMDISK driver initialized: 16 RAM disks of 4096K size
> 1024 blo
> size
>
> [ =A0 =A01.353807] loop: module
> loaded
> [ =A0 =A01.358480] xsysace xsysace.0: Xilinx SystemACE revision
> 1.0.12
> [ =A0 =A01.370084] xsysace xsysace.0: capacity: 7831152
> sectors
> [ =A0 =A01.380238] =A0xsa: xsa1 xsa2
> xsa3
> [ =A0 =A01.389686] Xilinx SystemACE device driver,
> major=3D254
> [ =A0 =A01.398224] nbd: registered device at major
> 43
> [ =A0 =A01.424199] XTemac: using sgDMA
> mode.
> [ =A0 =A01.429022] XTemac: using TxDRE
> mode
> [ =A0 =A01.436090] XTemac: using RxDRE
> mode
> [ =A0 =A01.443173] XTemac: buffer descriptor size: 32768
> (0x8000)
> [ =A0 =A01.454594] XTemac: (buffer_descriptor_init) phy: 0x3d20000, virt:
> 0xff1000
> , size:
> 0x8000
> [ =A0 =A01.475707] eth%d: XTemac: PHY detected at address
> 0.
> [ =A0 =A01.483329] eth0: Dropping NETIF_F_SG since no checksum
> feature.
> [ =A0 =A01.498175] eth0: Xilinx TEMAC #0 at 0x81200000 mapped to 0xC50200=
00,
> irq=3D2
> [ =A0 =A01.509565] eth0: XTemac id 1.0f, block id 5, type
> 8
> [ =A0 =A01.522258] xilinx_spi xilinx_spi.0: at 0x40618000 mapped to
> 0xC5004000, ir
> 4
>
> [ =A0 =A06.538578] ------------[ cut here
> ]------------
> [ =A0 =A06.545188] Badness at
> drivers/usb/c67x00/c67x00-ll-hpi.c:241
> [ =A0 =A06.556588] NIP: c01a4ec0 LR: c01a4eb4 CTR:
> c00186a4
> [ =A0 =A06.566441] REGS: c3c11d80 TRAP: 0700 =A0 Not tainted
> (2.6.24-rc3-robotplatf
> m)
>
> [ =A0 =A06.580945] MSR: 00029030 <EE,ME,IR,DR> =A0CR: 35000093 =A0XER:
> e0000065
> [ =A0 =A06.593565] TASK =3D c3c0d000[1] 'swapper' THREAD:
> c3c10000
> [ =A0 =A06.603925] GPR00: 00000001 c3c11e30 c3c0d000 00000000 c02c8528
> 00000001 85
> 5751
> 00000000
> [ =A0 =A06.620512] GPR08: 00000000 00000000 c3d12384 c3c10000 35000093
> ffffffff c0
> 4040
> c0284050
> [ =A0 =A06.637100] GPR16: c0284074 c028407c c028408c c02840b8 c02840cc
> 00000000 ff
> ffff
> ffffffff
> [ =A0 =A06.653687] GPR24: c0300000 c0284038 c02c9a20 00000000 c3d12368
> 00000000 c3
> 2360
> c02c9a04
> [ =A0 =A06.670620] NIP [c01a4ec0]
> c67x00_ll_reset+0x48/0x88
> [ =A0 =A06.680468] LR [c01a4eb4]
> c67x00_ll_reset+0x3c/0x88
> [ =A0 =A06.690143] Call
> Trace:
> [ =A0 =A06.694986] [c3c11e30] [c01a4eb4] c67x00_ll_reset+0x3c/0x88
> (unreliable)
> [ =A0 =A06.708290] [c3c11e50] [c01a3b0c]
> c67x00_drv_probe+0x16c/0x2e0
> [ =A0 =A06.719866] [c3c11e80] [c017af4c]
> platform_drv_probe+0x20/0x30
> [ =A0 =A06.731442] [c3c11e90] [c0179328]
> driver_probe_device+0xec/0x194
> [ =A0 =A06.743364] [c3c11eb0] [c0179580]
> __driver_attach+0x88/0xf4
> [ =A0 =A06.754422] [c3c11ed0] [c017855c]
> bus_for_each_dev+0x54/0x94
> [ =A0 =A06.765654] [c3c11f00] [c017912c]
> driver_attach+0x24/0x34
> [ =A0 =A06.776366] [c3c11f10] [c0178998]
> bus_add_driver+0x78/0x1d4
> [ =A0 =A06.787424] [c3c11f30] [c01798b4]
> driver_register+0x88/0x9c
> [ =A0 =A06.798482] [c3c11f40] [c017b2c8]
> platform_driver_register+0x9c/0xac
> [ =A0 =A06.811095] [c3c11f50] [c02fae74]
> c67x00_init+0x18/0x28
> [ =A0 =A06.821462] [c3c11f60] [c02e8230]
> kernel_init+0x10c/0x2a4
> [ =A0 =A06.832175] [c3c11ff0] [c0004c38]
> kernel_thread+0x44/0x60
> [ =A0 =A06.842886] Instruction
> dump:
> [ =A0 =A06.848760] 90010024 480a4969 38800000 7fa3eb78 6084fa50 4bffefbd
> 387d0020
> 801388
>
> [ =A0 =A06.864136] 480a3edd 707dffff 7c000026 54001ffe <0f000000> 3c00fff=
f
> 7fbd00d
> 60000005
>
> [ =A0 =A06.882540] c67x00 c67x00.0: Device reset
> failed
> [ =A0 =A06.889278] c67x00: probe of c67x00.0 failed with error
> 65531
> [ =A0 =A06.901621] usbcore: registered new interface driver
> libusual
> [ =A0 =A06.912837] usbcore: registered new interface driver
> cypress_cy7c63
> [ =A0 =A06.925443] i8042.c: No controller
> found.
> [ =A0 =A06.934533] mice: PS/2 mouse device common for all
> mice
> [ =A0 =A06.945746] i2c /dev entries
> driver
> [ =A0 =A06.953016] i2c-parport-light: adapter type
> unspecified
> [ =A0 =A06.964041] usbcore: registered new interface driver
> usbhid
> [ =A0 =A06.972725] drivers/hid/usbhid/hid-core.c: v2.6:USB HID core
> driver
> [ =A0 =A06.985267] TCP cubic
> registered
> [ =A0 =A06.991691] NET: Registered protocol family
> 1
> [ =A0 =A07.000260] NET: Registered protocol family
> 17
> [ =A0 =A07.011149] RPC: Registered udp transport
> module.
> [ =A0 =A07.018303] RPC: Registered tcp transport
> module.
> [ =A0 20.920617] kjournald starting. =A0Commit interval 5
> seconds
> [ =A0 20.975785] EXT3 FS on xsa2, internal
> journal
> [ =A0 20.981905] EXT3-fs: recovery
> complete.
> [ =A0 21.287791] EXT3-fs: mounted filesystem with ordered data
> mode.
> [ =A0 21.297194] VFS: Mounted root (ext3
> filesystem).
>
>
>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>



--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* C67x00 reset problems
From: A. Nolson @ 2009-03-10 23:17 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <1236379543.13132.10.camel@md40100b.ft>

Hi,

  I am using 2.6.24-rc3 ( secretlabs git) in an ML403 where I want to
use the USB c67x00 based host with Peter Kosgaard driver. I am using it
together with the sysace device for CF access ( in the ML403 those two
share lines, but I managed to insert some logic to multiplex both
devices). I have been able to use the c67300 without OS, but when I try
to boot the kernel I am getting driver reset problems (the whole log is
at the end)

[    6.882540] c67x00 c67x00.0: Device reset
failed                           
[    6.889278] c67x00: probe of c67x00.0 failed with error 65531  

And of course the device cannot get probed. I think this has happened to
other people before but I can't find the solution in the archive. My
virtex_devices.c initialization looks like this:

#define XPAR_C67x00_USB(num) { \
 .name = "c67x00", \
 .id = num, \
 .num_resources = 2, \
 .resource = (struct resource[]) { \
         { \
         .start = XPAR_C67X00_USB_PRH##num##_BASEADDR, \
         .end = XPAR_C67X00_USB_PRH##num##_BASEADDR + 0xf, \
         .flags = IORESOURCE_MEM, \
         }, \
         { \
         .start  = XPAR_OPB_INTC_0_SYSTEM_USB_HPI_INT_INTR, \
         .end    = XPAR_OPB_INTC_0_SYSTEM_USB_HPI_INT_INTR, \
         .flags  = IORESOURCE_IRQ, \
          }, \
  }, \
 .dev.platform_data = &(struct c67x00_platform_data) { \
 .sie_config = C67X00_SIE1_HOST | C67X00_SIE2_PERIPHERAL_A, \
 .hpi_regstep = 0x02, /* A0 not connected on 16bit bus */ \
 }, \
}

 /* Cypress USB C67x00 instances */       
#if defined(XPAR_C67X00_USB_PRH0_BASEADDR)
 XPAR_C67x00_USB(0),
#endif


and my xparameters_ml403.h like this:

/* Definitions for peripheral C67X00_USB */
#define XPAR_C67X00_USB_PRH0_BASEADDR 0xA5000000
#define XPAR_C67X00_USB_PRH0_HIGHADDR 0xA500FFFF
#define XPAR_C67X00_USB_PRH1_BASEADDR 0xA5010000
#define XPAR_C67X00_USB_PRH1_HIGHADDR 0xA501FFFF
#define XPAR_C67X00_USB_PRH2_BASEADDR 0xA5020000
#define XPAR_C67X00_USB_PRH2_HIGHADDR 0xA502FFFF
#define XPAR_C67X00_USB_PRH3_BASEADDR 0xA5030000
#define XPAR_C67X00_USB_PRH3_HIGHADDR 0xA503FFFF

#define XPAR_OPB_INTC_0_SYSTEM_USB_HPI_INT_INTR 0
#define XPAR_SYSTEM_USB_HPI_INT_MASK 0X000001

I don't know what could be wrong here. Any clues?

/Albert



loaded at:     00400000
0056419C                                              
board data at: 00562120
0056219C                                              
relocated to:  0040406C
004040E8                                              
zimage at:     00404E58
00561F12                                              
avail ram:     00565000
04000000                                              
                                                                            
 
Linux/PPC load: console=ttyUL0,57600 root=/dev/xsa2 rw
init=/sbin/init        
Uncompressing
Linux...done.                                                   
Now booting the
kernel                                                        
[    0.000000] Linux version 2.6.24-rc3-dirty (xxx@xxx) (gcc vers
n 4.2.2) #77 PREEMPT Mon Mar 9 18:31:59 CET
2009                              
[    0.000000] Xilinx ML403 Reference System (Virtex-4
FX)                    
[    0.000000] Zone PFN
ranges:                                               
[    0.000000]   DMA             0 ->   
16384                                
[    0.000000]   Normal      16384 ->   
16384                                
[    0.000000]   HighMem     16384 ->   
16384                                
[    0.000000] Movable zone start PFN for each
node                           
[    0.000000] early_node_map[1] active PFN
ranges                            
[    0.000000]     0:        0 ->   
16384                                    
[    0.000000] Built 1 zonelists in Zone order, mobility grouping on. 
Total p
es:
16256                                                                     
[    0.000000] Kernel command line: console=ttyUL0,57600 root=/dev/xsa2
rw ini
/sbin/init                                                                  
 
[    0.000000] Xilinx INTC #0 at 0x41200000 mapped to
0xFDFFF000              
[    0.000000] PID hash table entries: 256 (order: 8, 1024
bytes)             
[    0.000160] Console: colour dummy device
80x25                             
[    0.000569] Dentry cache hash table entries: 8192 (order: 3, 32768
bytes)  
[    0.001326] Inode-cache hash table entries: 4096 (order: 2, 16384
bytes)   
[    0.013549] Memory: 61628k available (2352k kernel code, 796k data,
112k in
, 0k
highmem)                                                                 
[    0.013800] SLUB: Genslabs=11, HWalign=32, Order=0-1, MinObjects=4,
CPUs=1,
odes=1                                                                      
 
[    0.035282] Mount-cache hash table entries:
512                            
[    0.039567] net_namespace: 64
bytes                                        
[    0.044545] NET: Registered protocol family
16                             
[    0.051781] Registering
spi1_board_info                                    
[    0.074960] usbcore: registered new interface driver
usbfs                 
[    0.075863] usbcore: registered new interface driver
hub                   
[    0.076911] usbcore: registered new device driver
usb                      
[    0.090812] NET: Registered protocol family
2                              
[    0.100648] IP route cache hash table entries: 1024 (order: 0, 4096
bytes)
[    0.103337] TCP established hash table entries: 2048 (order: 2, 16384
bytes
[    0.103614] TCP bind hash table entries: 2048 (order: 1, 8192
bytes)       
[    0.103770] TCP: Hash tables configured (established 2048 bind
2048)       
[    0.103798] TCP reno
registered                                            
[    0.107443] sysctl table check failed: /kernel/l2cr .1.31 Missing
strategy
[    0.107511] Call
Trace:                                                    
[    0.107530] [c3c11de0] [c0008338] show_stack+0x50/0x188
(unreliable)       
[    0.107606] [c3c11e30] [c003cf88]
set_fail+0x50/0x68                       
[    0.107675] [c3c11e50] [c003d5f8]
sysctl_check_table+0x658/0x6ac           
[    0.107730] [c3c11ec0] [c003d60c]
sysctl_check_table+0x66c/0x6ac           
[    0.107785] [c3c11f30] [c002907c]
register_sysctl_table+0x64/0xe0          
[    0.107851] [c3c11f50] [c02ee750]
register_ppc_htab_sysctl+0x18/0x2c       
[    0.107919] [c3c11f60] [c02e8230]
kernel_init+0x10c/0x2a4                  
[    0.107962] [c3c11ff0] [c0004c38]
kernel_thread+0x44/0x60                  
[    0.165612] Installing knfsd (copyright (C) 1996
okir@monad.swb.de).       
[    0.168476] io scheduler noop
registered                                   
[    0.168515] io scheduler anticipatory
registered                           
[    0.168538] io scheduler deadline
registered                               
[    0.168885] io scheduler cfq registered
(default)                          
[    0.763801] Generic RTC Driver
v1.07                                       
[    0.764606] Serial: 8250/16550 driver $Revision: 1.90 $ 4 ports, IRQ
sharin
disabled                                                                    
 
[    0.773092] uartlite.0: ttyUL0 at MMIO 0x40600003 (irq = 5) is a
uartlite  
[    0.773152] console [ttyUL0]
enabled                                       
[    1.334309] RAMDISK driver initialized: 16 RAM disks of 4096K size
1024 blo
size                                                                        
 
[    1.353807] loop: module
loaded                                            
[    1.358480] xsysace xsysace.0: Xilinx SystemACE revision
1.0.12            
[    1.370084] xsysace xsysace.0: capacity: 7831152
sectors                   
[    1.380238]  xsa: xsa1 xsa2
xsa3                                           
[    1.389686] Xilinx SystemACE device driver,
major=254                      
[    1.398224] nbd: registered device at major
43                             
[    1.424199] XTemac: using sgDMA
mode.                                      
[    1.429022] XTemac: using TxDRE
mode                                       
[    1.436090] XTemac: using RxDRE
mode                                       
[    1.443173] XTemac: buffer descriptor size: 32768
(0x8000)                 
[    1.454594] XTemac: (buffer_descriptor_init) phy: 0x3d20000, virt:
0xff1000
, size:
0x8000                                                                
[    1.475707] eth%d: XTemac: PHY detected at address
0.                      
[    1.483329] eth0: Dropping NETIF_F_SG since no checksum
feature.           
[    1.498175] eth0: Xilinx TEMAC #0 at 0x81200000 mapped to 0xC5020000,
irq=2
[    1.509565] eth0: XTemac id 1.0f, block id 5, type
8                       
[    1.522258] xilinx_spi xilinx_spi.0: at 0x40618000 mapped to
0xC5004000, ir
4                                                                           
 
[    6.538578] ------------[ cut here
]------------                           
[    6.545188] Badness at
drivers/usb/c67x00/c67x00-ll-hpi.c:241              
[    6.556588] NIP: c01a4ec0 LR: c01a4eb4 CTR:
c00186a4                       
[    6.566441] REGS: c3c11d80 TRAP: 0700   Not tainted 
(2.6.24-rc3-robotplatf
m)                                                                          
 
[    6.580945] MSR: 00029030 <EE,ME,IR,DR>  CR: 35000093  XER:
e0000065       
[    6.593565] TASK = c3c0d000[1] 'swapper' THREAD:
c3c10000                  
[    6.603925] GPR00: 00000001 c3c11e30 c3c0d000 00000000 c02c8528
00000001 85
5751
00000000                                                                 
[    6.620512] GPR08: 00000000 00000000 c3d12384 c3c10000 35000093
ffffffff c0
4040
c0284050                                                                 
[    6.637100] GPR16: c0284074 c028407c c028408c c02840b8 c02840cc
00000000 ff
ffff
ffffffff                                                                 
[    6.653687] GPR24: c0300000 c0284038 c02c9a20 00000000 c3d12368
00000000 c3
2360
c02c9a04                                                                 
[    6.670620] NIP [c01a4ec0]
c67x00_ll_reset+0x48/0x88                       
[    6.680468] LR [c01a4eb4]
c67x00_ll_reset+0x3c/0x88                        
[    6.690143] Call
Trace:                                                    
[    6.694986] [c3c11e30] [c01a4eb4] c67x00_ll_reset+0x3c/0x88
(unreliable)   
[    6.708290] [c3c11e50] [c01a3b0c]
c67x00_drv_probe+0x16c/0x2e0             
[    6.719866] [c3c11e80] [c017af4c]
platform_drv_probe+0x20/0x30             
[    6.731442] [c3c11e90] [c0179328]
driver_probe_device+0xec/0x194           
[    6.743364] [c3c11eb0] [c0179580]
__driver_attach+0x88/0xf4                
[    6.754422] [c3c11ed0] [c017855c]
bus_for_each_dev+0x54/0x94               
[    6.765654] [c3c11f00] [c017912c]
driver_attach+0x24/0x34                  
[    6.776366] [c3c11f10] [c0178998]
bus_add_driver+0x78/0x1d4                
[    6.787424] [c3c11f30] [c01798b4]
driver_register+0x88/0x9c                
[    6.798482] [c3c11f40] [c017b2c8]
platform_driver_register+0x9c/0xac       
[    6.811095] [c3c11f50] [c02fae74]
c67x00_init+0x18/0x28                    
[    6.821462] [c3c11f60] [c02e8230]
kernel_init+0x10c/0x2a4                  
[    6.832175] [c3c11ff0] [c0004c38]
kernel_thread+0x44/0x60                  
[    6.842886] Instruction
dump:                                              
[    6.848760] 90010024 480a4969 38800000 7fa3eb78 6084fa50 4bffefbd
387d0020
801388                                                                      
 
[    6.864136] 480a3edd 707dffff 7c000026 54001ffe <0f000000> 3c00ffff
7fbd00d
60000005                                                                    
 
[    6.882540] c67x00 c67x00.0: Device reset
failed                           
[    6.889278] c67x00: probe of c67x00.0 failed with error
65531              
[    6.901621] usbcore: registered new interface driver
libusual              
[    6.912837] usbcore: registered new interface driver
cypress_cy7c63        
[    6.925443] i8042.c: No controller
found.                                  
[    6.934533] mice: PS/2 mouse device common for all
mice                    
[    6.945746] i2c /dev entries
driver                                        
[    6.953016] i2c-parport-light: adapter type
unspecified                    
[    6.964041] usbcore: registered new interface driver
usbhid                
[    6.972725] drivers/hid/usbhid/hid-core.c: v2.6:USB HID core
driver        
[    6.985267] TCP cubic
registered                                           
[    6.991691] NET: Registered protocol family
1                              
[    7.000260] NET: Registered protocol family
17                             
[    7.011149] RPC: Registered udp transport
module.                          
[    7.018303] RPC: Registered tcp transport
module.                          
[   20.920617] kjournald starting.  Commit interval 5
seconds                 
[   20.975785] EXT3 FS on xsa2, internal
journal                              
[   20.981905] EXT3-fs: recovery
complete.                                    
[   21.287791] EXT3-fs: mounted filesystem with ordered data
mode.            
[   21.297194] VFS: Mounted root (ext3
filesystem).                           

^ permalink raw reply

* Re: TSEC/MDIO bug in recent kernels
From: Johns Daniel @ 2009-03-10 22:55 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <ba5d9360903101553h3cf7c33ex639999803655c440@mail.gmail.com>

(I did not get to complete the message!)

For those of you who are running into this error:
        24520:01 not found
        eth0: Could not attach to PHY
        IP-Config: Failed to open eth0
        IP-Config: Device `eth0' not found.

There is a bug in recent kernels. I found it in 2.6.28.7:
        linux/arch/powerpc/sysdev/fsl_soc.c:
for_each_compatible_node(np, "mdio", "gianfar");
should be:
        linux/arch/powerpc/sysdev/fsl_soc.c:
for_each_compatible_node(np, "mdio", "gianfar")

Note that the semicolon should not be there!

-- Johns

^ permalink raw reply

* Re: [PATCH v5] introduce macro spin_event_timeout()
From: Scott Wood @ 2009-03-10 22:58 UTC (permalink / raw)
  To: Josh Boyer; +Cc: linuxppc-dev, Timur Tabi
In-Reply-To: <20090310223753.GB26415@zod.rchland.ibm.com>

Josh Boyer wrote:
> On Tue, Mar 10, 2009 at 05:33:08PM -0500, Scott Wood wrote:
>> Timur Tabi wrote:
>>> The macro spin_event_timeout() takes a condition and timeout value
>>> (in microseconds) as parameters.  It spins until either the condition is true
>>> or the timeout expires.  It returns zero if the timeout expires first, non-zero
>>> otherwise.
>>>
>>> This primary purpose of this macro is to poll on a hardware register until a
>>> status bit changes.  The timeout ensures that the loop still terminates if the
>>> bit doesn't change as expected.  This macro makes it easier for driver
>>> developers to perform this kind of operation properly.
>>>
>>> Signed-off-by: Timur Tabi <timur@freescale.com>
>>> ---
>>>
>>> v5: ported to arch/powerpc, made it powerpc-specific, eliminated udelay
>> Why make it powerpc-specific?  This would be nice to have in  
>> arch-independent code.
> 
> That's just mean.  He already posted it to lkml and was told to make it
> powerpc specific by Alan.

Well, that's what happens when a discussion hops mailing lists with no 
backreference. :-P

I don't see anywhere where he says it should be architecture dependent, 
but rather a general "I don't like this, get off my lawn!" response.

I cannot agree with the "we shouldn't be encouraging this" sentiment; 
people don't generally do spin loops because they're lazy[1], but rather 
because the hardware demands it -- and it's hardly only on powerpc (much 
less just "some Freescale drivers") that I've encountered hardware that 
demands it, typiclally during reset/initialization or similarly non-hot 
paths.  Why not provide something less likely to have bugs (the timeout 
case is unlikely to be well tested), more easily seen when reviewing a 
patch, and more likely to result in spin loops *with* a timeout rather 
than without?

-Scott

[1] Or rather, those that do should be smacked down during patch review.

^ permalink raw reply

* TSEC/MDIO bug in recent kernels
From: Johns Daniel @ 2009-03-10 22:53 UTC (permalink / raw)
  To: linuxppc-dev

For those of you who are running into this error:
         24520:01 not found
         eth0: Could not attach to PHY
         IP-Config: Failed to open eth0
         IP-Config: Device `eth0' not found.

There is a bug in recent kernels. I found it in 2.6.28.7:
linux/arch/powerpc/sysdev/fsl_soc.c.~1~:
for_each_compatible_node(np, "mdio", "gianfar");

^ permalink raw reply

* Re: [PATCH v5] introduce macro spin_event_timeout()
From: Josh Boyer @ 2009-03-10 22:37 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, Timur Tabi
In-Reply-To: <49B6EAA4.9000803@freescale.com>

On Tue, Mar 10, 2009 at 05:33:08PM -0500, Scott Wood wrote:
> Timur Tabi wrote:
>> The macro spin_event_timeout() takes a condition and timeout value
>> (in microseconds) as parameters.  It spins until either the condition is true
>> or the timeout expires.  It returns zero if the timeout expires first, non-zero
>> otherwise.
>>
>> This primary purpose of this macro is to poll on a hardware register until a
>> status bit changes.  The timeout ensures that the loop still terminates if the
>> bit doesn't change as expected.  This macro makes it easier for driver
>> developers to perform this kind of operation properly.
>>
>> Signed-off-by: Timur Tabi <timur@freescale.com>
>> ---
>>
>> v5: ported to arch/powerpc, made it powerpc-specific, eliminated udelay
>
> Why make it powerpc-specific?  This would be nice to have in  
> arch-independent code.

That's just mean.  He already posted it to lkml and was told to make it
powerpc specific by Alan.

josh

^ permalink raw reply

* Re: [PATCH v5] introduce macro spin_event_timeout()
From: Scott Wood @ 2009-03-10 22:33 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev
In-Reply-To: <1236723118-3577-1-git-send-email-timur@freescale.com>

Timur Tabi wrote:
> The macro spin_event_timeout() takes a condition and timeout value
> (in microseconds) as parameters.  It spins until either the condition is true
> or the timeout expires.  It returns zero if the timeout expires first, non-zero
> otherwise.
> 
> This primary purpose of this macro is to poll on a hardware register until a
> status bit changes.  The timeout ensures that the loop still terminates if the
> bit doesn't change as expected.  This macro makes it easier for driver
> developers to perform this kind of operation properly.
> 
> Signed-off-by: Timur Tabi <timur@freescale.com>
> ---
> 
> v5: ported to arch/powerpc, made it powerpc-specific, eliminated udelay

Why make it powerpc-specific?  This would be nice to have in 
arch-independent code.

-Scott

^ permalink raw reply

* [PATCH v5] introduce macro spin_event_timeout()
From: Timur Tabi @ 2009-03-10 22:11 UTC (permalink / raw)
  To: linuxppc-dev

The macro spin_event_timeout() takes a condition and timeout value
(in microseconds) as parameters.  It spins until either the condition is true
or the timeout expires.  It returns zero if the timeout expires first, non-zero
otherwise.

This primary purpose of this macro is to poll on a hardware register until a
status bit changes.  The timeout ensures that the loop still terminates if the
bit doesn't change as expected.  This macro makes it easier for driver
developers to perform this kind of operation properly.

Signed-off-by: Timur Tabi <timur@freescale.com>
---

v5: ported to arch/powerpc, made it powerpc-specific, eliminated udelay

v4: removed cpu_relax (redundant), changed timeout to unsigned long

v3: eliminated secondary evaluation of condition, replaced jiffies with udelay

v2: added cpu_relax and time_before

 arch/powerpc/include/asm/delay.h |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/delay.h b/arch/powerpc/include/asm/delay.h
index f9200a6..aadec70 100644
--- a/arch/powerpc/include/asm/delay.h
+++ b/arch/powerpc/include/asm/delay.h
@@ -2,6 +2,8 @@
 #define _ASM_POWERPC_DELAY_H
 #ifdef __KERNEL__
 
+#include <asm/time.h>
+
 /*
  * Copyright 1996, Paul Mackerras.
  *
@@ -30,5 +32,35 @@ extern void udelay(unsigned long usecs);
 #define mdelay(n)	udelay((n) * 1000)
 #endif
 
+/**
+ * spin_event_timeout - spin until a condition gets true or a timeout elapses
+ * @condition: a C expression to evalate
+ * @timeout: timeout, in microseconds
+ *
+ * The process spins until the @condition evaluates to true (non-zero) or
+ * the @timeout elapses.
+ *
+ * This primary purpose of this macro is to poll on a hardware register
+ * until a status bit changes.  The timeout ensures that the loop still
+ * terminates if the bit never changes.
+ *
+ * The return value is non-zero if the condition evaluates to true first, or
+ * zero if the timeout elapses first.
+ */
+#define spin_event_timeout(condition, timeout)			\
+({								\
+	unsigned long __start = get_tbl();			\
+	unsigned long __loops = tb_ticks_per_usec * timeout;	\
+	int __ret = 1;						\
+	while (!(condition)) {					\
+		if (tb_ticks_since(__start) > __loops) {	\
+			__ret = 0;				\
+			break;					\
+		}						\
+		cpu_relax();					\
+	}							\
+	__ret;							\
+})
+
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_DELAY_H */
-- 
1.6.1.3

^ permalink raw reply related

* Re: [PATCH] PowerPC 440EPx/GRx fix memory size calculation
From: Mikhail Zolotaryov @ 2009-03-10 20:57 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <20090310195013.GA27835@ru.mvista.com>

Valentine Barshak wrote:
> According to the AMCC 440EPX/GRX user manual,
> the Chip Select width is always fixed at 1 bit no matter
> what is actually read from register DDR_10.

Well, from my point of view original kernel code is correct in this part.

Adding one bit into memory address means multiplying memory size by 2 
i.e. cs=2. The question is: is Chip Select bit used in memory address. 
ChipSelect input of memory chip enables or disabled it, so if we have 
only one BankSel installed/connected (DDR0_10[22:23] is 01 or 10) 
there's no need to use Chip Select bit in an address. On the contrary, 
if both BankSel lines are connected (DDR0_10[22:23] is 11), to let 
memory controller know which memory rank to use, Chip Select bit is 
added into memory address. (and yes, if DDR0_10[22:23] is 00 - no ranks 
installed, memory size is 0, cs=0)

Original kernel code use exactly the same logic as I described above. 
Please suggest if it's wrong.

^ permalink raw reply

* [PATCH] PowerPC 440EPx/GRx fix memory size calculation
From: Valentine Barshak @ 2009-03-10 19:50 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: lebon
In-Reply-To: <49B58779.9040905@lebon.org.ua>

I was just going to submit a patch for that too.
Indeed, the denali_fixup_memsize() miscalculated a couple of address
field widths. We were lucky to eventually get the right result,
because the effect of the first error was killed by the other one.
According to the AMCC 440EPX/GRX user manual,
the Chip Select width is always fixed at 1 bit no matter
what is actually read from register DDR_10.
The workaround is to use a predefined chipselect value for 440EPx/GRx.
Also, setting the REDUC bit (REDUC = 1) enables 32-bit data path.
If REDUC = 0, full data path of 64 bits is used.

Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com>
Signed-off-by: Mikhail Zolotaryov <lebon@lebon.org.ua>


--- a/arch/powerpc/boot/4xx.c	2008-04-26 02:18:34.000000000 +0400
+++ b/arch/powerpc/boot/4xx.c	2008-10-26 01:40:27.000000000 +0400
@@ -173,15 +173,20 @@ void ibm4xx_denali_fixup_memsize(void)
 	max_col = DDR_GET_VAL(val, DDR_MAX_COL_REG, DDR_MAX_COL_REG_SHIFT);
 	max_row = DDR_GET_VAL(val, DDR_MAX_ROW_REG, DDR_MAX_ROW_REG_SHIFT);
 
-	/* get CS value */
-	val = SDRAM0_READ(DDR0_10);
-
-	val = DDR_GET_VAL(val, DDR_CS_MAP, DDR_CS_MAP_SHIFT);
-	cs = 0;
-	while (val) {
-		if (val & 0x1)
-			cs++;
-		val = val >> 1;
+	/* 440EPx/GRx chipselect always fixed at 1 bit */
+	if ((mfpvr() & 0xf0000ff0) == 0x200008D0)
+		cs = 1;
+	else {
+		/* get CS value */
+		val = SDRAM0_READ(DDR0_10);
+		val = DDR_GET_VAL(val, DDR_CS_MAP, DDR_CS_MAP_SHIFT);
+
+		cs = 0;
+		while (val) {
+			if (val & 0x1)
+				cs++;
+			val = val >> 1;
+		}
 	}
 
 	if (!cs)
@@ -192,7 +197,7 @@ void ibm4xx_denali_fixup_memsize(void)
 	/* get data path bytes */
 	val = SDRAM0_READ(DDR0_14);
 
-	if (DDR_GET_VAL(val, DDR_REDUC, DDR_REDUC_SHIFT))
+	if (!DDR_GET_VAL(val, DDR_REDUC, DDR_REDUC_SHIFT))
 		dpath = 8; /* 64 bits */
 	else
 		dpath = 4; /* 32 bits */

^ permalink raw reply

* Re: [PATCH 5/5] net: make mpc5200 fec driver use of_mdio infrastructure
From: Anton Vorontsov @ 2009-03-10 20:29 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, jgarzik, afleming, linux-kernel, netdev
In-Reply-To: <fa686aa40903101248n33ee35c2o32c4b24b349b14fc@mail.gmail.com>

On Tue, Mar 10, 2009 at 01:48:26PM -0600, Grant Likely wrote:
[...]
> >> eliminates the assumption that the PHY for the FEC is always
> >> attached to the FEC's own MDIO bus. With this patch, the FEC can
> >> use a PHY attached to any MDIO bus if it is described in the device
> >> tree.
> >
> > AFAIK, Gianfar and UCC Geth drivers can do this too, so I'm assuming
> > that this isn't the cause for these major changes.
> 
> Certainly the mpc5200-fec driver's original phy code certainly wasn't
> as robust as the ucc_geth and gianfar phy handling.
> 
> ucc_geth open codes a solution to decode the phy_device name from
> several nodes in the device tree and doesn't handle the case where the
> ucc_geth is initialized before the phy_device is registered.  gianfar
> open codes the same thing.  This solution uses common code to locate
> the phy_device, and it works regardless of what order devices are
> registered in.
> 
> That being said, the 5200 driver originally using probe() time to
> connect to the phy.  If I change it to be connected at open time, then
> does the registration order issue become irrelevant?

Yup. `ifconfig ethX up' calls ->open(). If it fails (and prints
nice error), you can try `ifconfig ethX up' again later.

> Regardless, I
> think all the drivers should be using common code for obtaining the
> phy_device from the device tree.

Not necessary `struct phy_device'. All we need is some common
routine for translating PHY's "mdio_node->full_name + phy id" to
phy_bus_id.

That is, you can just factor out this code from the gianfar driver,

void gfar_mdio_bus_name(char *name, struct device_node *np)
{
        const u32 *reg;

        reg = of_get_property(np, "reg", NULL);

        snprintf(name, MII_BUS_ID_SIZE, "%s@%x", np->name, reg ? *reg : 0);
}

...
        gfar_mdio_bus_name(bus_name, mdio);
        snprintf(priv->phy_bus_id, sizeof(priv->phy_bus_id), "%s:%02x",
                 bus_name, *id);
...

And make sure FEC MDIO driver does mdio_bus->parent = &ofdev->dev;

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

^ permalink raw reply

* Re: [PATCH 5/5] net: make mpc5200 fec driver use of_mdio infrastructure
From: Grant Likely @ 2009-03-10 19:48 UTC (permalink / raw)
  To: avorontsov; +Cc: linuxppc-dev, jgarzik, afleming, linux-kernel, netdev
In-Reply-To: <20090310191638.GA8539@oksana.dev.rtsoft.ru>

On Tue, Mar 10, 2009 at 1:16 PM, Anton Vorontsov
<avorontsov@ru.mvista.com> wrote:
> On Tue, Mar 10, 2009 at 09:22:24AM -0600, Grant Likely wrote:
>> From: Grant Likely <grant.likely@secretlab.ca>
> [...]
>> +static int mpc52xx_fec_notifier_phy_add(struct notifier_block *nb,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 unsigned long event, void *_dev)
>> +{
> [...]
>> + =A0 =A0 rc =3D phy_connect_direct(priv->ndev, priv->phydev,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mpc52xx_fec_ad=
just_link, 0, 0);
>> + =A0 =A0 if (rc) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 dev_err(dev, "phy_connect_direct() failed\n");
>> + =A0 =A0 =A0 =A0 =A0 =A0 return 0;
>> + =A0 =A0 }
>> +
>> + =A0 =A0 rc =3D register_netdev(priv->ndev);
>> + =A0 =A0 if (rc) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 phy_disconnect(priv->phydev);
>> + =A0 =A0 =A0 =A0 =A0 =A0 dev_err(dev, "register_netdev() failed\n");
>> + =A0 =A0 }
>> +
>> + =A0 =A0 return 0;
>> +}
> [...]
>> =A0static int __devinit
>> =A0mpc52xx_fec_probe(struct of_device *op, const struct of_device_id *ma=
tch)
>> @@ -896,7 +874,6 @@ mpc52xx_fec_probe(struct of_device *op, const struct=
 of_device_id *match)
> [...]
>> + =A0 =A0 /* Register the new network device immediately if we don't nee=
d
>> + =A0 =A0 =A0* to wait for a phy_device first. */
>> + =A0 =A0 if (!priv->phy_node) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 if (priv->seven_wire_mode)
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_info(&ndev->dev, "using 7-=
wire PHY mode\n");
>> + =A0 =A0 =A0 =A0 =A0 =A0 else
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_info(&ndev->dev, "Fixed sp=
eed MII link: %i%cD\n",
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0priv->speed=
, priv->duplex ? 'F' : 'H');
>> + =A0 =A0 =A0 =A0 =A0 =A0 rv =3D register_netdev(ndev);
>> + =A0 =A0 =A0 =A0 =A0 =A0 if (rv < 0)
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto probe_error;
>> =A0 =A0 =A0 }
> [...]
>
> Two registration points for the netdev... That's ugly. :-/
>
> What problem are you trying to solve w/ these patches, btw?
>
> `ifconfig ethX up` is safe even w/o PHY attached.
>
> All the (user-visible) changes is that we no longer have "ethX"
> until PHY is registered, and I can't say that this is good either.

Fair enough.  If it is okay to register the PHY after registering the
netdev, then I have no problem with it.  I'll change the patch.

> I can't say that the probing code is much prettier or easier to
> understand... But maybe there are some other problems that you're
> solving, which I don't see so far?

Primary problem is that this driver currently does not work for a PHY
on a different MDIO bus.  Secondary is that current code depends on
phys being registered before the FEC.

> That is, can you explain why the changes are needed? Did you
> consider other solutions?

Yes, I considered doing some kind of platform function to call and get
the name of the PHY, but any such thing turned out to be fragile and
rather platform specific.  The bus notifier approach seemed to be the
simplest way to defer part of initialization while waiting for the PHY
to become available.  I want to be using common code here as I've got
another Ethernet driver (ll_temac; not posted yet) that needs to do
the same thing.

>> eliminates the assumption that the PHY for the FEC is always
>> attached to the FEC's own MDIO bus. With this patch, the FEC can
>> use a PHY attached to any MDIO bus if it is described in the device
>> tree.
>
> AFAIK, Gianfar and UCC Geth drivers can do this too, so I'm assuming
> that this isn't the cause for these major changes.

Certainly the mpc5200-fec driver's original phy code certainly wasn't
as robust as the ucc_geth and gianfar phy handling.

ucc_geth open codes a solution to decode the phy_device name from
several nodes in the device tree and doesn't handle the case where the
ucc_geth is initialized before the phy_device is registered.  gianfar
open codes the same thing.  This solution uses common code to locate
the phy_device, and it works regardless of what order devices are
registered in.

That being said, the 5200 driver originally using probe() time to
connect to the phy.  If I change it to be connected at open time, then
does the registration order issue become irrelevant?  Regardless, I
think all the drivers should be using common code for obtaining the
phy_device from the device tree.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH 5/5] net: make mpc5200 fec driver use of_mdio infrastructure
From: Anton Vorontsov @ 2009-03-10 19:16 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, jgarzik, afleming, linux-kernel, netdev
In-Reply-To: <20090310152224.12455.99348.stgit@localhost.localdomain>

On Tue, Mar 10, 2009 at 09:22:24AM -0600, Grant Likely wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
[...]
> +static int mpc52xx_fec_notifier_phy_add(struct notifier_block *nb,
> +					unsigned long event, void *_dev)
> +{
[...]
> +	rc = phy_connect_direct(priv->ndev, priv->phydev,
> +				mpc52xx_fec_adjust_link, 0, 0);
> +	if (rc) {
> +		dev_err(dev, "phy_connect_direct() failed\n");
> +		return 0;
> +	}
> +
> +	rc = register_netdev(priv->ndev);
> +	if (rc) {
> +		phy_disconnect(priv->phydev);
> +		dev_err(dev, "register_netdev() failed\n");
> +	}
> +
> +	return 0;
> +}
[...]
>  static int __devinit
>  mpc52xx_fec_probe(struct of_device *op, const struct of_device_id *match)
> @@ -896,7 +874,6 @@ mpc52xx_fec_probe(struct of_device *op, const struct of_device_id *match)
[...]
> +	/* Register the new network device immediately if we don't need
> +	 * to wait for a phy_device first. */
> +	if (!priv->phy_node) {
> +		if (priv->seven_wire_mode)
> +			dev_info(&ndev->dev, "using 7-wire PHY mode\n");
> +		else
> +			dev_info(&ndev->dev, "Fixed speed MII link: %i%cD\n",
> +				 priv->speed, priv->duplex ? 'F' : 'H');
> +		rv = register_netdev(ndev);
> +		if (rv < 0)
> +			goto probe_error;
>  	}
[...]

Two registration points for the netdev... That's ugly. :-/

What problem are you trying to solve w/ these patches, btw?

`ifconfig ethX up` is safe even w/o PHY attached.

All the (user-visible) changes is that we no longer have "ethX"
until PHY is registered, and I can't say that this is good either.

Previously you'd have ethX all the time, and `ifconfig ethX up`
would report user-friendly "PHY not attached" error. Now we have
to guess why ethX isn't there.

I can't say that the probing code is much prettier or easier to
understand... But maybe there are some other problems that you're
solving, which I don't see so far?

That is, can you explain why the changes are needed? Did you
consider other solutions?


Thanks!

p.s.
> eliminates the assumption that the PHY for the FEC is always
> attached to the FEC's own MDIO bus. With this patch, the FEC can
> use a PHY attached to any MDIO bus if it is described in the device
> tree.

AFAIK, Gianfar and UCC Geth drivers can do this too, so I'm assuming
that this isn't the cause for these major changes.

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox