LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: State of the MPC5200 PSC AC97 driver
From: Sylvain Munaut @ 2008-04-11  7:29 UTC (permalink / raw)
  To: Marian Balakowicz; +Cc: spitzauer_77, linuxppc-dev
In-Reply-To: <47FDEF9B.5000506@semihalf.com>


> Last year you have posted a MPC5200 PSC AC97 driver patch
> "[PATCH 9/9] sound: Add support for Freescale MPC5200 AC97 interface."
> with the following comment:
>
>   
>> Not quite a clean driver, but it get things done (well, mostly).
>> Only included to be able to test functionalityi/usage of
>> the BestComm driver.
>>     
Yes ... and it still applies.
>
> There are various FIXMEs and commented out code here and there.
> Could you elaborate a bit on the overall state of the driver's
> functionality,
> which areas need improvement and attention?
>   
It's a minimum boiler plate.
I filled the function at the minimum to get some sound output and being
able to hear music coming out of it :)
I also completely skipped the 5200 (not B) support ...

At first there was no DMA at all (full software copy). I added some
simple DMA later just to prove it could work and how to use the API.

I just wrote it to get some sound, prove the interface could work under
linux and to show how to use DMA. I had hoped someone else would finish
it ... (yeah, I know ... big mistake).
> Seems that you mainly tested BestComm with this driver, what was the
> overall
> BestComm performance, any issues here? Did you use any specific test setup
> involving some dedicated application, etc.?
>   
My test was listening to Gorrillaz "Feel Good inc" using mplayer ...

> Did anyone else tried it and/or has a updated version or can share
> experience?
>   
At the time several other people tried it and it worked ... unless you
did lots of harddrive IO and then it crumbled ...



Sylvain

^ permalink raw reply

* Re: [PATCH 1/5] phylib: don't create a phydev for ID-less PHYs.
From: Joakim Tjernlund @ 2008-04-11  7:06 UTC (permalink / raw)
  To: Paul Gortmaker; +Cc: scottwood, linuxppc-dev, linux-net
In-Reply-To: <4764055962e0a42ce083783bbb897a62090ad90b.1207846542.git.paul.gortmaker@windriver.com>


On Thu, 2008-04-10 at 13:51 -0400, Paul Gortmaker wrote:
> I've tested on 8360, 8540 and 8641D and in all cases, the PHY
> ID returned for bus addr 0x1f is all zeros, and not all 0xf.
> This means we've been allocating a phydev for this "ghost".
> 
> In addition to marking 0x0 as an invalid PHY ID, I've also
> changed the existing somewhat useless printk to actually
> list the bus IDs where it found a PHY so we get a basic
> bus summary.
 
PHY ID 0x0 isn't an invalid id, I got a Broadcom PHY that has
PHY ID=0. Maybe I am misunderstanding something?

 Jocke

^ permalink raw reply

* Re: State of the MPC5200 PSC AC97 driver
From: Robert Schwebel @ 2008-04-11  6:51 UTC (permalink / raw)
  To: Matt Sealey; +Cc: linuxppc-dev, Sylvain Munaut, spitzauer_77, Marian Balakowicz
In-Reply-To: <47FE3160.2060907@genesi-usa.com>

Hi Matt,

On Thu, Apr 10, 2008 at 04:25:20PM +0100, Matt Sealey wrote:
> I have a copy of the driver I hacked to work with the new PSC
> framework but it doesn't "work" anymore - I have no idea why.
>
> Mail me privately if you want it, I have no time to make patches or
> look for a good reason for the breakages, but it compiles at least.
> The Efika hack should be gone now (irrelevant since we released a
> firmware that obseletes it) and the other fixmes should be by the
> wayside considering the fscking thing doesn't play sound anymore...

I'd be interested as well; do I read your mail right that you have
already managed to hear sound coming out of the MPC5200B PSC AC97
interface? You would be the first one I've heared of, so it would
definitely be interesting to see your code. 

Robert
-- 
 Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry
   Handelsregister:  Amtsgericht Hildesheim, HRA 2686
     Hannoversche Str. 2, 31134 Hildesheim, Germany
   Phone: +49-5121-206917-0 |  Fax: +49-5121-206917-9

^ permalink raw reply

* Re: [PATCH] [v5] Add idle wait support for 44x platforms
From: Olof Johansson @ 2008-04-11  5:31 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: kvm-ppc-devel, linuxppc-dev
In-Reply-To: <200804110218.23912.arnd@arndb.de>

On Fri, Apr 11, 2008 at 02:18:22AM +0200, Arnd Bergmann wrote:
> On Thursday 10 April 2008, Jerone Young wrote:
> > Well it could be this simple. But the current code leaves a lot more
> > room to add different type waits or spins if need be (if they are ever
> > needed ... though none off the top of my head at the moment)...but it
> > does allow you to create another wait state for whatever reason a lot
> > easier.
> > 
> > So I really don't think this needs to change. Unless everyone really
> > feels that it just has to be.
> 
> No, it doesn't need to change, the current patch is entirely correct,
> just a little bit more complicated than it needs to be, and I try
> not to have code in anticipation of something getting more complicated
> in the future, you can always add the complexity at the point where you
> need it.

That piece of code came across from the platforms/pasemi version,
where we for a while had more than two kinds of idle loops
(doze/nap/sleep/rvwinkle). Turns out doze does well enough that the
others weren't really needed so I never submitted support for the deeper
sleep modes.


-Olof

^ permalink raw reply

* Re: [PATCH 0/5] dynamic detection of gianfar TPIPA
From: Paul Gortmaker @ 2008-04-11  3:34 UTC (permalink / raw)
  To: Andy Fleming; +Cc: scottwood, linuxppc-dev, netdev
In-Reply-To: <3F8109F5-F512-48CD-AD93-2C6B4164E5AA@freescale.com>

In message: Re: [PATCH 0/5] dynamic detection of gianfar TPIPA
on 10/04/2008 Andy Fleming wrote:

>
> I may be missing something, but I don't think this quite right.
>
> If you have a PHY at 0x1f, this patchset will cause no PHY device to be 
> allocated for that address, and you'll actually end up assigning TBIPA to 
> be 0x1f again, since there's no PHY there.  Right?  Were you able to use 
> this code with a PHY at 0x1f?

I tested on several "normal" boards and on a board with the PHY @ 0x1f,
and it did what I expected it to do.  It was when I was testing on the
normal boards (8540MDS, 8360MDS, HPCN) that I observed we were showing
a PHY ID of 0x0 at 0x1f during the routine PHY scan, because the
autodetect code was skipping 0x1f even on those boards.  I backed out
all my patches and the situation was the same, hence why I decided to
skip IDs of either 0xffff or 0x0.


> I like the idea of passing around priv->mii_bus instead of regs, but I  
> think it won't work without becoming unnecessarily unwieldy.  The  
> problem is that the TBI PHY is not necessarily accessed through the same 
> bus as the PHY.  Each controller has its own TBI PHY, and that PHY can 
> only be accessed from *that* controller's MDIO bus.  So if you want to 
> configure TSEC2's TBI PHY, you use TSEC2's MDIO regs.  That's what 
> gfar_local_mdio_* allowed; they write the *local* controller's MDIO regs. 
>  It looks like this code sets up priv->mii_bus to point at the bus which 
> holds the PHY, but only TSEC0's bus (on most SoCs) is connected to actual 
> PHYs.  So you will only ever be able to configure the TBI PHY on TSEC0, 
> which will not allow any of the other TSECs to use an SGMII PHY.  Were 
> you able to use other TSECs to connect to an SGMII PHY?

Okay -- that explanation helps me understand the role of the *_local_*
variants -- it wasn't obvious to me that they were being used to jump
the device --> bus association and go right at MDIO bus of tsec0.  I
think this can still be handled sanely though -- we'd have to simply
say that if you wanted the bus of the TBI of the controller, you would
go at dev->priv->mii_bus, and if you wanted the bus of the PHY of the
controller, you'd go at dev->priv->phydev->bus.  I'd have to think a bit
to see if that would afford the same or similar cleanups, but the
distinction at least seems clearer to me now.

> We could still pass around an mii_bus reference, but this would require 
> creating an mii_bus instance for every single TSEC, which is a little 
> heavyweight when we just want to configure the TBI PHY once on startup.

Yep.  Is there any boards out there with more than 4 tsec?  I'd have
to go look at the size of mii_bus to see what the per bus cost is.

>
> After some thinking, I went ahead and implemented a patch which isn't  
> ideal, but should solve the problems your patches set out to solve.   
> I've sent it in a separate message.  If you have some systems with SGMII 
> and/or a PHY at 0x1f, please test this patch on them.  I don't currently 
> have either.

I'll go have a look.  I've only got the SBC8641D with the PHY @ 0x1f to
be the oddball guniea pig.

Paul.

>
> Andy
>

^ permalink raw reply

* Re: [PATCH 6/13] devres: implement managed iomap interface
From: Tejun Heo @ 2008-04-11  2:08 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev, gregkh, linux-kernel, linux-ide, jgarzik, alan
In-Reply-To: <A567023C-17FC-402D-9394-29DFFA816891@kernel.crashing.org>

Kumar Gala wrote:
>>> but there is no reason not to make it work properly.  For example I  
>>> believe libata uses devm_* and the fsl SATA driver (non-PCI) will 
>>> need  to work in cases similar to the 44x.
>>
>>   Well, as for sata_fsl, it calls of_iomap() which does The Right Thing.
> 
> Fair, but I don't see why we should introduce new APIs that are already 
> "broken".  We went through a lot of effort to clean up and introduce 
> resource_t (and clearly still have some bugs) for the >32-bit physical 
> address problem.

Yes, please go ahead.  In case it wasn't clear, I wasn't objecting to 
the fix at all.  I was just curious what could actually happen on x86.

-- 
tejun

^ permalink raw reply

* Re: [PATCH] powerpc TLF_RESTORE_SIGMASK
From: Andrew Morton @ 2008-04-11  1:50 UTC (permalink / raw)
  To: Roland McGrath; +Cc: linuxppc-dev, Paul Mackerras, linux-kernel
In-Reply-To: <20080411013635.1F4B126F992@magilla.localdomain>

On Thu, 10 Apr 2008 18:36:35 -0700 (PDT) Roland McGrath <roland@redhat.com> wrote:

> > This crashes my powerpc mac g5.
> 
> It worked fine on mine.
> 
> > It happens after the boot, during the first login-over-ssh.
> 
> Mine did lots of stuff fine.
> 
> > This is with most of the rest of the -mm poopile applied.  i386 and x86_64
> > seem OK.
> 
> I had only tested with Linus's tree plus the small handful of post-2.6.25
> cleanup patches I've posted in the last few weeks.  To be precise it was
> 9597362d354f8655ece324b01d0c640a0e99c077 plus several of my cleanup patches
> (that are probably all in -mm, but I'm not sure off hand).
> 
> I'd rebased my tree today to 783e391b7b5b273cd20856d8f6f4878da8ec31b3
> anyway.  I just tried the new kernel with the sigmask cleanups and only 
> a few other patches, and have no problems.
> 
> The details of your crash make it look pretty unrelated to this code.
> Off hand I would guess that it's some other bug from other -mm patches
> that just happens only to bite you on powerpc.  If the crash is not
> intermittent and you bisected it to this one change, then I am at a
> loss to see what might be happening.  I'd have to leave it to Paul et
> al to figure out if there is some strange powerpc juju going on.

It's 100% repeatable and I bisected it to this change.

I expect you could repeat it by applying it to
http://userweb.kernel.org/~akpm/mmotm/ (or to -rc6-mm2, if I ever manage to
get it to boot on something) and using
http://userweb.kernel.org/~akpm/config-g5.txt

^ permalink raw reply

* Re: [PATCH 1/3] powerpc: Add support for Wind River SBC8641D board
From: Dale Farnsworth @ 2008-04-11  1:37 UTC (permalink / raw)
  To: Paul Gortmaker; +Cc: linuxppc-dev
In-Reply-To: <0006b107cbf99a968352d9369a8e26304d4bf13a.1207868891.git.paul.gortmaker@windriver.com>

On Thu, Apr 10, 2008 at 07:22:17PM -0400, Paul Gortmaker wrote:
> This adds support for the Wind River SBC8641D board, based
> largely on the mpc86xx_hpcn support.  The biggest difference is
> the lack of the Uli and the i8259 cascade, which simplifies things.

Hi Paul,

A couple issues.  (These should be fixed in the mpc86xx_hpcn.c as well.)

> +void __init
> +sbc8641_init_irq(void)

Make this function static.

> +void
> +sbc8641_show_cpuinfo(struct seq_file *m)

This one too.

> +static int __init sbc8641_probe(void)
> +{
> +	unsigned long root = of_get_flat_dt_root();
> +
> +	if (of_flat_dt_is_compatible(root, "mpc86xx"))

"mpc86xx" is too generic.  You should make the compatible field
in your devtree something like "WIND,sbc8641d", to differentiate
it from the mpc86xx_hpcn.

> +long __init
> +mpc86xx_time_init(void)

Another static function.

Regards,
-Dale

^ permalink raw reply

* Re: [PATCH] powerpc TLF_RESTORE_SIGMASK
From: Roland McGrath @ 2008-04-11  1:36 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linuxppc-dev, Paul Mackerras, linux-kernel
In-Reply-To: <20080409235156.33ee33f2.akpm@linux-foundation.org>

> This crashes my powerpc mac g5.

It worked fine on mine.

> It happens after the boot, during the first login-over-ssh.

Mine did lots of stuff fine.

> This is with most of the rest of the -mm poopile applied.  i386 and x86_64
> seem OK.

I had only tested with Linus's tree plus the small handful of post-2.6.25
cleanup patches I've posted in the last few weeks.  To be precise it was
9597362d354f8655ece324b01d0c640a0e99c077 plus several of my cleanup patches
(that are probably all in -mm, but I'm not sure off hand).

I'd rebased my tree today to 783e391b7b5b273cd20856d8f6f4878da8ec31b3
anyway.  I just tried the new kernel with the sigmask cleanups and only 
a few other patches, and have no problems.

The details of your crash make it look pretty unrelated to this code.
Off hand I would guess that it's some other bug from other -mm patches
that just happens only to bite you on powerpc.  If the crash is not
intermittent and you bisected it to this one change, then I am at a
loss to see what might be happening.  I'd have to leave it to Paul et
al to figure out if there is some strange powerpc juju going on.


Thanks,
Roland

^ permalink raw reply

* Re: ppc64 build issue
From: Stephen Rothwell @ 2008-04-11  1:15 UTC (permalink / raw)
  To: Kumar Gala; +Cc: ppc-dev list
In-Reply-To: <221074DB-2297-4263-8325-BAB964DCCEF1@kernel.crashing.org>

[-- Attachment #1: Type: text/plain, Size: 357 bytes --]

Hi Kumar,

On Thu, 10 Apr 2008 10:01:27 -0500 Kumar Gala <galak@kernel.crashing.org> wrote:
>
> anyone looking into this?
> 
> http://kisskb.ellerman.id.au/kisskb/buildresult/23299/
> 
> Its really a Kconfig issue.

Just posted a patch.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* [PATCH] [POWERPC] Fix MAX_ORDER config problem
From: Stephen Rothwell @ 2008-04-11  1:11 UTC (permalink / raw)
  To: paulus; +Cc: ppc-dev

The allyesconfig (among others) build was giving this:

In file included from include/linux/gfp.h:4,
                 from include/linux/slab.h:14,
                 from include/linux/percpu.h:5,
                 from include2/asm/time.h:18,
                 from include2/asm/cputime.h:26,
                 from include/linux/sched.h:67,
                 from
arch/powerpc/kernel/asm-offsets.c:17:
include/linux/mmzone.h:791:2: error: #error Allocator MAX_ORDER exceeds SECTION_SIZE

Kconfig options are order depenendent, so move the setting of
FORCE_MAX_ZONEORDER to after the setting of PPC_64K_PAGES. Also add an
explicit !PPC_64K_PAGES.

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 arch/powerpc/Kconfig |   40 ++++++++++++++++++++--------------------
 1 files changed, 20 insertions(+), 20 deletions(-)

Paul, this is a bug in the powerpc next tree.

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 625342e..1d4d19f 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -211,26 +211,6 @@ source kernel/Kconfig.hz
 source kernel/Kconfig.preempt
 source "fs/Kconfig.binfmt"
 
-config FORCE_MAX_ZONEORDER
-	int "Maximum zone order"
-	default "9" if PPC_64K_PAGES
-	default "13" if PPC64
-	default "11"
-	help
-	  The kernel memory allocator divides physically contiguous memory
-	  blocks into "zones", where each zone is a power of two number of
-	  pages.  This option selects the largest power of two that the kernel
-	  keeps in the memory allocator.  If you need to allocate very large
-	  blocks of physically contiguous memory, then you may need to
-	  increase this value.
-
-	  This config option is actually maximum order plus one. For example,
-	  a value of 11 means that the largest free memory block is 2^10 pages.
-
-	  The page size is not necessarily 4KB.  For example, on 64-bit
-	  systems, 64KB pages can be enabled via CONFIG_PPC_64K_PAGES.  Keep
-	  this in mind when choosing a value for this option.
-
 config HUGETLB_PAGE_SIZE_VARIABLE
 	bool
 	depends on HUGETLB_PAGE
@@ -403,6 +383,26 @@ config PPC_64K_PAGES
 	  while on hardware with such support, it will be used to map
 	  normal application pages.
 
+config FORCE_MAX_ZONEORDER
+	int "Maximum zone order"
+	default "9" if PPC_64K_PAGES
+	default "13" if PPC64 && !PPC_64K_PAGES
+	default "11"
+	help
+	  The kernel memory allocator divides physically contiguous memory
+	  blocks into "zones", where each zone is a power of two number of
+	  pages.  This option selects the largest power of two that the kernel
+	  keeps in the memory allocator.  If you need to allocate very large
+	  blocks of physically contiguous memory, then you may need to
+	  increase this value.
+
+	  This config option is actually maximum order plus one. For example,
+	  a value of 11 means that the largest free memory block is 2^10 pages.
+
+	  The page size is not necessarily 4KB.  For example, on 64-bit
+	  systems, 64KB pages can be enabled via CONFIG_PPC_64K_PAGES.  Keep
+	  this in mind when choosing a value for this option.
+
 config PPC_SUBPAGE_PROT
 	bool "Support setting protections for 4k subpages"
 	depends on PPC_64K_PAGES
-- 
1.5.4.5

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

^ permalink raw reply related

* Re: [PATCH] [v5] Add idle wait support for 44x platforms
From: Josh Boyer @ 2008-04-11  1:02 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: kvm-ppc-devel, linuxppc-dev
In-Reply-To: <200804110218.23912.arnd@arndb.de>

On Fri, 2008-04-11 at 02:18 +0200, Arnd Bergmann wrote:
> > > static int __init idle_param(char *p)
> > > {
> > >       if (!strcmp(modes[i].name, "spin"))
> > >               ppc_md.power_save = NULL;
> > > }
> > > early_param("idle", idle_param);
> > > 
> > > if you statically initialize the ppc_md.power_save function to ppc44x_idle
> > > in the platform setup files?
> > 
> > The idea is to not statically initialize ppc_md.power_save to
> > ppc44x_idle in each platform setup file.
> > 
> 
> Why not? Unlike the platform_initcall, it wouldn't cost anything and your
> current code has the same effect in the end, but in a less obvious way.

This is likely something that has evolved from the original patch which
added a machine_late_initcall(<machine>, ppc44x_idle_init) to every
platform.c file.  It made the original patch overly complicated as
opposed to adding a single arch_initcall in a common file.  That was
also before we decided to make "wait" the default, which made things
much easier to deal with.

I'll take the blame for the way the minorly suboptimal state the patch
is, as Jerone had to jump through several hoops already at my request.
I agree it's cleaner to do as you suggest in the long run, and may well
make some changes along those lines myself.

josh

^ permalink raw reply

* Re: [PATCH 1/3] powerpc: Add support for Wind River SBC8641D board
From: Michael Ellerman @ 2008-04-11  0:38 UTC (permalink / raw)
  To: Stephen Rothwell; +Cc: Paul Gortmaker, linuxppc-dev
In-Reply-To: <20080411100541.e45b92f4.sfr@canb.auug.org.au>

[-- Attachment #1: Type: text/plain, Size: 853 bytes --]

On Fri, 2008-04-11 at 10:05 +1000, Stephen Rothwell wrote:
> Hi Paul,
> 
> Just a couple of comments.
> 
> On Thu, 10 Apr 2008 19:22:17 -0400 Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
> >
> > +++ b/arch/powerpc/platforms/86xx/sbc8641d.c
> > +#undef DEBUG
> > +
> > +#ifdef DEBUG
> > +#define DBG(fmt...) do { printk(KERN_ERR fmt); } while(0)
>                                    ^^^^^^^^
> Not KERN_DEBUG?  Also, if you include linux/kernel.h you can use pr_debug() instead.

s/Also, if you include/Please include/

We don't need more hand-rolled debug macros :)

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [PATCH] [v5] Add idle wait support for 44x platforms
From: Arnd Bergmann @ 2008-04-11  0:18 UTC (permalink / raw)
  To: jyoung5; +Cc: kvm-ppc-devel, linuxppc-dev
In-Reply-To: <1207858131.26428.4.camel@thinkpadL>

On Thursday 10 April 2008, Jerone Young wrote:
> Well it could be this simple. But the current code leaves a lot more
> room to add different type waits or spins if need be (if they are ever
> needed ... though none off the top of my head at the moment)...but it
> does allow you to create another wait state for whatever reason a lot
> easier.
>=20
> So I really don't think this needs to change. Unless everyone really
> feels that it just has to be.

No, it doesn't need to change, the current patch is entirely correct,
just a little bit more complicated than it needs to be, and I try
not to have code in anticipation of something getting more complicated
in the future, you can always add the complexity at the point where you
need it.

> >=20
> > static int __init idle_param(char *p)
> > {
> > =A0=A0=A0=A0=A0=A0if (!strcmp(modes[i].name, "spin"))
> > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0ppc_md.power_save =3D NULL;
> > }
> > early_param("idle", idle_param);
> >=20
> > if you statically initialize the ppc_md.power_save function to ppc44x_i=
dle
> > in the platform setup files?
>=20
> The idea is to not statically initialize ppc_md.power_save to
> ppc44x_idle in each platform setup file.
>=20

Why not? Unlike the platform_initcall, it wouldn't cost anything and your
current code has the same effect in the end, but in a less obvious way.

	Arnd <><

^ permalink raw reply

* Re: [PATCH 1/3] powerpc: Add support for Wind River SBC8641D board
From: Stephen Rothwell @ 2008-04-11  0:05 UTC (permalink / raw)
  To: Paul Gortmaker; +Cc: linuxppc-dev
In-Reply-To: <0006b107cbf99a968352d9369a8e26304d4bf13a.1207868891.git.paul.gortmaker@windriver.com>

[-- Attachment #1: Type: text/plain, Size: 1138 bytes --]

Hi Paul,

Just a couple of comments.

On Thu, 10 Apr 2008 19:22:17 -0400 Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
>
> +++ b/arch/powerpc/platforms/86xx/sbc8641d.c
> +#undef DEBUG
> +
> +#ifdef DEBUG
> +#define DBG(fmt...) do { printk(KERN_ERR fmt); } while(0)
                                   ^^^^^^^^
Not KERN_DEBUG?  Also, if you include linux/kernel.h you can use pr_debug() instead.

> +sbc8641_init_irq(void)
> +{
> +	struct mpic *mpic1;
> +	struct device_node *np;
> +	struct resource res;
> +
> +	/* Determine PIC address. */
> +	np = of_find_node_by_type(NULL, "open-pic");
> +	if (np == NULL)
> +		return;
> +	of_address_to_resource(np, 0, &res);
> +
> +	/* Alloc mpic structure and per isu has 16 INT entries. */
> +	mpic1 = mpic_alloc(np, res.start,
> +			MPIC_PRIMARY | MPIC_WANTS_RESET | MPIC_BIG_ENDIAN,
> +			0, 256, " MPIC     ");
> +	BUG_ON(mpic1 == NULL);
> +
> +	mpic_init(mpic1);

You need an of_node_put(np) in here somewhere (probably after the mpic_alloc).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [PATCH] CPM: Always use new binding.
From: Kumar Gala @ 2008-04-10 23:44 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <20080410204502.GB20197@loki.buserror.net>


On Apr 10, 2008, at 3:45 PM, Scott Wood wrote:
> The kconfig entry can go away once arch/ppc and references to the  
> config in
> drivers are removed.

just to be clear.  arch/powerpc will always just use the new binding  
and arch/ppc uses the old (or has the option)?

- k

^ permalink raw reply

* [PATCH 0/5] dynamic detection of gianfar TPIPA
From: Paul Gortmaker @ 2008-04-10 17:51 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: scottwood, linux-net


This patch series consists of some minor cleanups that eventually
allow us to implement a dynamic assignment of the gianfar TBIPA.
This was the implementation that Andy and Scott indicated was
the most desireable solution at the bottom of this discussion:

http://patchwork.ozlabs.org/linuxppc-embedded/patch?id=12902

The lead up patches are as follows:

 -dont allocate a phydev for the "ghost" phy that seems to appear
  on ID 0x1f of all the MDIO bus scans.

 -add in the missing assignment to priv->mii_bus when we assign a
  phy to the device.

 -cleanup of unnecessary externs that the above change allows us.

 -make the mii_bus->priv point to struct gianfar_mdio_data

At this point, we can then actually use the phy_map of the mii_bus
to tell us which MDIO ID we can use for the TBIPA, instead of just
hard coding 0x1f.

Paul.

^ permalink raw reply

* [PATCH 2/2] [POWERPC] Fix kernel stack allocation alignment
From: Benjamin Herrenschmidt @ 2008-04-10 23:36 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linux-Arch, linuxppc-dev, takata, linux-m32r, Paul Mackerras

The powerpc kernel stacks need to be naturally aligned, as they
contain the thread info at the bottom, which is obtained by
clearing the low bits of the stack pointer.

However, when using 64K pages (the stack is smaller than a page),
we use kmalloc to allocate it, which doesn't provide that guarantee.

It appeared to work so far... until one enables SLUB debugging
which then returns unaligned pointers. Ooops...

This patch fixes it by using a slab cache with enforced alignment
for those. It replies on my previous patch that adds a
thread_info_cache_init() callback.

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

---
Note that m32r seems to also have this problem.

 arch/powerpc/kernel/process.c     |   31 +++++++++++++++++++++++++++++++
 include/asm-powerpc/thread_info.h |   10 ++++------
 2 files changed, 35 insertions(+), 6 deletions(-)

--- linux-work.orig/arch/powerpc/kernel/process.c	2008-04-10 13:11:05.000000000 +1000
+++ linux-work/arch/powerpc/kernel/process.c	2008-04-10 13:14:29.000000000 +1000
@@ -1052,3 +1052,34 @@ void ppc64_runlatch_off(void)
 	}
 }
 #endif
+
+#if THREAD_SHIFT < PAGE_SHIFT
+
+static struct kmem_cache *thread_info_cache;
+
+struct thread_info *alloc_thread_info(struct task_struct *tsk)
+{
+	struct thread_info *ti;
+
+	ti = kmem_cache_alloc(thread_info_cache, GFP_KERNEL);
+	if (unlikely(ti == NULL))
+		return NULL;
+#ifdef CONFIG_DEBUG_STACK_USAGE
+	memset(ti, 0, THREAD_SIZE);
+#endif
+	return ti;
+}
+
+void free_thread_info(struct thread_info *ti)
+{
+	kmem_cache_free(thread_info_cache, ti);
+}
+
+void thread_info_cache_init(void)
+{
+	thread_info_cache = kmem_cache_create("thread_info", THREAD_SIZE,
+					      THREAD_SIZE, 0, NULL);
+	BUG_ON(thread_info_cache == NULL);
+}
+
+#endif /* THREAD_SHIFT < PAGE_SHIFT */
Index: linux-work/include/asm-powerpc/thread_info.h
===================================================================
--- linux-work.orig/include/asm-powerpc/thread_info.h	2008-04-10 13:11:05.000000000 +1000
+++ linux-work/include/asm-powerpc/thread_info.h	2008-04-10 13:14:47.000000000 +1000
@@ -80,12 +80,10 @@ struct thread_info {
 
 #else /* THREAD_SHIFT < PAGE_SHIFT */
 
-#ifdef CONFIG_DEBUG_STACK_USAGE
-#define alloc_thread_info(tsk)	kzalloc(THREAD_SIZE, GFP_KERNEL)
-#else
-#define alloc_thread_info(tsk)	kmalloc(THREAD_SIZE, GFP_KERNEL)
-#endif
-#define free_thread_info(ti)	kfree(ti)
+extern struct thread_info *alloc_thread_info(struct task_struct *tsk);
+extern void free_thread_info(struct thread_info *ti);
+extern void thread_info_cache_init(void);
+#define thread_info_cache_init thread_info_cache_init
 
 #endif /* THREAD_SHIFT < PAGE_SHIFT */
 

^ permalink raw reply

* [PATCH 1/2] Add thread_info_cache_init() to all archs
From: Benjamin Herrenschmidt @ 2008-04-10 23:36 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linux-Arch, linuxppc-dev, takata, linux-m32r, Paul Mackerras

Some architecture need to maintain a kmem cache for thread info
structures. (next patch adds that to powerpc to fix an alignment
problem).

There is no good arch callback to use to initialize that cache
that I can find, so this adds a new one and adds an empty macro
for when it's not implemented.

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

So we have the choice here between:

 - the ifdef on the func name that I did, consistent with what
I did before for iomap, which iirc Linus liked

 - add some more ARCH_HAS_* or HAVE_* (yuck)

 - add an empty definition to all archs .h (pain in the neck but I
can do it, though it will be an annoying patch to keep around)

 - do a weak function (will slightly bloat everybody for no good reason)

So unless there is strong complaints, I'd like to stick to my
current approach.

(This one fixes a stupid typo, missing () in the macro def.)

 include/linux/sched.h |    4 ++++
 init/main.c           |    1 +
 2 files changed, 5 insertions(+)

--- linux-work.orig/init/main.c	2008-04-10 13:11:06.000000000 +1000
+++ linux-work/init/main.c	2008-04-10 13:11:19.000000000 +1000
@@ -623,6 +623,7 @@ asmlinkage void __init start_kernel(void
 	if (efi_enabled)
 		efi_enter_virtual_mode();
 #endif
+	thread_info_cache_init();
 	fork_init(num_physpages);
 	proc_caches_init();
 	buffer_init();
Index: linux-work/include/linux/sched.h
===================================================================
--- linux-work.orig/include/linux/sched.h	2008-04-10 13:11:44.000000000 +1000
+++ linux-work/include/linux/sched.h	2008-04-11 09:35:58.000000000 +1000
@@ -1893,6 +1893,10 @@ static inline unsigned long *end_of_stac
 
 #endif
 
+#ifndef thread_info_cache_init
+#define thread_info_cache_init()	do { } while(0)
+#endif
+
 /* set thread flags in other task's structures
  * - see asm/thread_info.h for TIF_xxxx flags available
  */

^ permalink raw reply

* [PATCH v2.6.26] gianfar: Determine TBIPA value dynamically
From: Andy Fleming @ 2008-04-10 23:34 UTC (permalink / raw)
  To: netdev; +Cc: paul.gortmaker, linuxppc-dev

TBIPA needs to be set to a value (on connected MDIO buses) that doesn't
conflict with PHYs on the bus.  By hardcoding it to 0x1f, we were preventing
boards with PHYs at 0x1f from working properly.  Instead, scan the bus when
it comes up, and find an address that doesn't have a PHY on it.  The TBI PHY
configuration code then trusts that the value in TBIPA is either safe, or
doesn't matter (ie - it's not an active bus with other PHYs).

Signed-off-by: Andy Fleming <afleming@freescale.com>
---

I think this should go in, but I'd like to see some testing first.  I don't
have hardware which is affected by this.  I've only confirmed that it doesn't
break current hardware.

 drivers/net/gianfar.c     |   25 +++++++++++++----------
 drivers/net/gianfar_mii.c |   47 ++++++++++++++++++++++++++++++++++++++++----
 drivers/net/gianfar_mii.h |    3 ++
 3 files changed, 59 insertions(+), 16 deletions(-)

diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index c8c3df7..6661015 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -476,24 +476,30 @@ static int init_phy(struct net_device *dev)
 	return 0;
 }
 
+/*
+ * Initialize TBI PHY interface for communicating with the
+ * SERDES lynx PHY on the chip.  We communicate with this PHY
+ * through the MDIO bus on each controller, treating it as a
+ * "normal" PHY at the address found in the TBIPA register.  We assume
+ * that the TBIPA register is valid.  Either the MDIO bus code will set
+ * it to a value that doesn't conflict with other PHYs on the bus, or the
+ * value doesn't matter, as there are no other PHYs on the bus.
+ */
 static void gfar_configure_serdes(struct net_device *dev)
 {
 	struct gfar_private *priv = netdev_priv(dev);
 	struct gfar_mii __iomem *regs =
 			(void __iomem *)&priv->regs->gfar_mii_regs;
+	int tbipa = gfar_read(&priv->regs->tbipa);
 
-	/* Initialise TBI i/f to communicate with serdes (lynx phy) */
+	/* Single clk mode, mii mode off(for serdes communication) */
+	gfar_local_mdio_write(regs, tbipa, MII_TBICON, TBICON_CLK_SELECT);
 
-	/* Single clk mode, mii mode off(for aerdes communication) */
-	gfar_local_mdio_write(regs, TBIPA_VALUE, MII_TBICON, TBICON_CLK_SELECT);
-
-	/* Supported pause and full-duplex, no half-duplex */
-	gfar_local_mdio_write(regs, TBIPA_VALUE, MII_ADVERTISE,
+	gfar_local_mdio_write(regs, tbipa, MII_ADVERTISE,
 			ADVERTISE_1000XFULL | ADVERTISE_1000XPAUSE |
 			ADVERTISE_1000XPSE_ASYM);
 
-	/* ANEG enable, restart ANEG, full duplex mode, speed[1] set */
-	gfar_local_mdio_write(regs, TBIPA_VALUE, MII_BMCR, BMCR_ANENABLE |
+	gfar_local_mdio_write(regs, tbipa, MII_BMCR, BMCR_ANENABLE |
 			BMCR_ANRESTART | BMCR_FULLDPLX | BMCR_SPEED1000);
 }
 
@@ -540,9 +546,6 @@ static void init_registers(struct net_device *dev)
 
 	/* Initialize the Minimum Frame Length Register */
 	gfar_write(&priv->regs->minflr, MINFLR_INIT_SETTINGS);
-
-	/* Assign the TBI an address which won't conflict with the PHYs */
-	gfar_write(&priv->regs->tbipa, TBIPA_VALUE);
 }
 
 
diff --git a/drivers/net/gianfar_mii.c b/drivers/net/gianfar_mii.c
index b889892..e061738 100644
--- a/drivers/net/gianfar_mii.c
+++ b/drivers/net/gianfar_mii.c
@@ -78,7 +78,6 @@ int gfar_local_mdio_write(struct gfar_mii __iomem *regs, int mii_id,
  * same as system mdio bus, used for controlling the external PHYs, for eg.
  */
 int gfar_local_mdio_read(struct gfar_mii __iomem *regs, int mii_id, int regnum)
-
 {
 	u16 value;
 
@@ -122,7 +121,7 @@ int gfar_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
 }
 
 /* Reset the MIIM registers, and wait for the bus to free */
-int gfar_mdio_reset(struct mii_bus *bus)
+static int gfar_mdio_reset(struct mii_bus *bus)
 {
 	struct gfar_mii __iomem *regs = (void __iomem *)bus->priv;
 	unsigned int timeout = PHY_INIT_TIMEOUT;
@@ -152,14 +151,15 @@ int gfar_mdio_reset(struct mii_bus *bus)
 }
 
 
-int gfar_mdio_probe(struct device *dev)
+static int gfar_mdio_probe(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct gianfar_mdio_data *pdata;
 	struct gfar_mii __iomem *regs;
+	struct gfar __iomem *enet_regs;
 	struct mii_bus *new_bus;
 	struct resource *r;
-	int err = 0;
+	int i, err = 0;
 
 	if (NULL == dev)
 		return -EINVAL;
@@ -199,6 +199,43 @@ int gfar_mdio_probe(struct device *dev)
 	new_bus->dev = dev;
 	dev_set_drvdata(dev, new_bus);
 
+	/*
+	 * This is mildly evil, but so is our hardware for doing this.
+	 * Also, we have to cast back to struct gfar_mii because of
+	 * definition weirdness done in gianfar.h.
+	 */
+	enet_regs = (struct gfar __iomem *)
+		((char *)regs - offsetof(struct gfar, gfar_mii_regs));
+
+	/* Scan the bus, looking for an empty spot for TBIPA */
+	gfar_write(&enet_regs->tbipa, 0);
+	for (i = PHY_MAX_ADDR; i > 0; i--) {
+		int physid1;
+		int physid2;
+		u32 phy_id;
+
+		physid1 = gfar_mdio_read(new_bus, i, MII_PHYSID1);
+
+		if (physid1 < 0)
+			return physid1;
+
+		physid2 = gfar_mdio_read(new_bus, i, MII_PHYSID2);
+
+		if (physid2 < 0)
+			return physid2;
+
+		phy_id = ((physid1 & 0xffff) << 16) | (physid2 & 0xffff);
+
+		if (phy_id == 0xffffffff)
+			break;
+	}
+
+	/* The bus is full.  We don't support using 31 PHYs, sorry */
+	if (i < 0)
+		return -EBUSY;
+
+	gfar_write(&enet_regs->tbipa, i);
+
 	err = mdiobus_register(new_bus);
 
 	if (0 != err) {
@@ -218,7 +255,7 @@ reg_map_fail:
 }
 
 
-int gfar_mdio_remove(struct device *dev)
+static int gfar_mdio_remove(struct device *dev)
 {
 	struct mii_bus *bus = dev_get_drvdata(dev);
 
diff --git a/drivers/net/gianfar_mii.h b/drivers/net/gianfar_mii.h
index b373091..2af28b1 100644
--- a/drivers/net/gianfar_mii.h
+++ b/drivers/net/gianfar_mii.h
@@ -41,6 +41,9 @@ struct gfar_mii {
 
 int gfar_mdio_read(struct mii_bus *bus, int mii_id, int regnum);
 int gfar_mdio_write(struct mii_bus *bus, int mii_id, int regnum, u16 value);
+int gfar_local_mdio_write(struct gfar_mii __iomem *regs, int mii_id,
+			  int regnum, u16 value);
+int gfar_local_mdio_read(struct gfar_mii __iomem *regs, int mii_id, int regnum);
 int __init gfar_mdio_init(void);
 void gfar_mdio_exit(void);
 #endif /* GIANFAR_PHY_H */
-- 
1.5.4.GIT

^ permalink raw reply related

* Re: [PATCH 0/5] dynamic detection of gianfar TPIPA
From: Andy Fleming @ 2008-04-10 23:30 UTC (permalink / raw)
  To: Paul Gortmaker; +Cc: scottwood, linuxppc-dev, linux-net
In-Reply-To: <1207849922-2436-1-git-send-email-paul.gortmaker@windriver.com>


On Apr 10, 2008, at 12:51, Paul Gortmaker wrote:

>
> This patch series consists of some minor cleanups that eventually
> allow us to implement a dynamic assignment of the gianfar TBIPA.
> This was the implementation that Andy and Scott indicated was
> the most desireable solution at the bottom of this discussion:
>
> http://patchwork.ozlabs.org/linuxppc-embedded/patch?id=12902
>
> The lead up patches are as follows:
>
> -dont allocate a phydev for the "ghost" phy that seems to appear
>  on ID 0x1f of all the MDIO bus scans.
>
> -add in the missing assignment to priv->mii_bus when we assign a
>  phy to the device.
>
> -cleanup of unnecessary externs that the above change allows us.
>
> -make the mii_bus->priv point to struct gianfar_mdio_data
>
> At this point, we can then actually use the phy_map of the mii_bus
> to tell us which MDIO ID we can use for the TBIPA, instead of just
> hard coding 0x1f.


I may be missing something, but I don't think this quite right.

If you have a PHY at 0x1f, this patchset will cause no PHY device to  
be allocated for that address, and you'll actually end up assigning  
TBIPA to be 0x1f again, since there's no PHY there.  Right?  Were you  
able to use this code with a PHY at 0x1f?

I like the idea of passing around priv->mii_bus instead of regs, but I  
think it won't work without becoming unnecessarily unwieldy.  The  
problem is that the TBI PHY is not necessarily accessed through the  
same bus as the PHY.  Each controller has its own TBI PHY, and that  
PHY can only be accessed from *that* controller's MDIO bus.  So if you  
want to configure TSEC2's TBI PHY, you use TSEC2's MDIO regs.  That's  
what gfar_local_mdio_* allowed; they write the *local* controller's  
MDIO regs.  It looks like this code sets up priv->mii_bus to point at  
the bus which holds the PHY, but only TSEC0's bus (on most SoCs) is  
connected to actual PHYs.  So you will only ever be able to configure  
the TBI PHY on TSEC0, which will not allow any of the other TSECs to  
use an SGMII PHY.  Were you able to use other TSECs to connect to an  
SGMII PHY?

We could still pass around an mii_bus reference, but this would  
require creating an mii_bus instance for every single TSEC, which is a  
little heavyweight when we just want to configure the TBI PHY once on  
startup.

After some thinking, I went ahead and implemented a patch which isn't  
ideal, but should solve the problems your patches set out to solve.   
I've sent it in a separate message.  If you have some systems with  
SGMII and/or a PHY at 0x1f, please test this patch on them.  I don't  
currently have either.

Andy

^ permalink raw reply

* [PATCH 5/5] gianfar: don't hard code the TBIPA MDIO address
From: Paul Gortmaker @ 2008-04-10 17:52 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: scottwood, linux-net, Paul Gortmaker
In-Reply-To: <4764055962e0a42ce083783bbb897a62090ad90b.1207846542.git.paul.gortmaker@windriver.com>

Currently the MDIO address for the gianfar Ten Bit
Interface is hard coded to be 0x1f, which can conflit
with some boards if they happen to put a PHY there.

Previous discussions indicated that the proper approach
here would be to dynamically allocate it, based on
picking the highest MDIO address that is not in use
by a PHY.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/net/gianfar.c       |   20 ++++++++++++++++----
 drivers/net/gianfar.h       |    1 -
 drivers/net/gianfar_mii.c   |   11 ++++++++++-
 include/linux/fsl_devices.h |    1 +
 4 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 9173784..684c97b 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -123,6 +123,7 @@ static irqreturn_t gfar_transmit(int irq, void *dev_id);
 static irqreturn_t gfar_interrupt(int irq, void *dev_id);
 static void adjust_link(struct net_device *dev);
 static void init_registers(struct net_device *dev);
+static void gfar_set_tbipa(struct net_device *dev);
 static int init_phy(struct net_device *dev);
 static int gfar_probe(struct platform_device *pdev);
 static int gfar_remove(struct platform_device *pdev);
@@ -469,6 +470,8 @@ static int init_phy(struct net_device *dev)
 	priv->mii_bus = phydev->bus;
 	priv->phydev = phydev;
 
+	gfar_set_tbipa(dev);
+
 	if (interface == PHY_INTERFACE_MODE_SGMII)
 		gfar_configure_serdes(dev);
 
@@ -479,19 +482,20 @@ static void gfar_configure_serdes(struct net_device *dev)
 {
 	struct gfar_private *priv = netdev_priv(dev);
 	struct mii_bus *bus = priv->mii_bus;
+	struct gianfar_mdio_data *mdata = (struct gianfar_mdio_data *)bus->priv;
 
 	/* Initialise TBI i/f to communicate with serdes (lynx phy) */
 
 	/* Single clk mode, mii mode off(for aerdes communication) */
-	gfar_mdio_write(bus, TBIPA_VALUE, MII_TBICON, TBICON_CLK_SELECT);
+	gfar_mdio_write(bus, mdata->tbi_pa, MII_TBICON, TBICON_CLK_SELECT);
 
 	/* Supported pause and full-duplex, no half-duplex */
-	gfar_mdio_write(bus, TBIPA_VALUE, MII_ADVERTISE,
+	gfar_mdio_write(bus, mdata->tbi_pa, MII_ADVERTISE,
 			ADVERTISE_1000XFULL | ADVERTISE_1000XPAUSE |
 			ADVERTISE_1000XPSE_ASYM);
 
 	/* ANEG enable, restart ANEG, full duplex mode, speed[1] set */
-	gfar_mdio_write(bus, TBIPA_VALUE, MII_BMCR, BMCR_ANENABLE |
+	gfar_mdio_write(bus, mdata->tbi_pa, MII_BMCR, BMCR_ANENABLE |
 			BMCR_ANRESTART | BMCR_FULLDPLX | BMCR_SPEED1000);
 }
 
@@ -539,8 +543,16 @@ static void init_registers(struct net_device *dev)
 	/* Initialize the Minimum Frame Length Register */
 	gfar_write(&priv->regs->minflr, MINFLR_INIT_SETTINGS);
 
+}
+
+static void gfar_set_tbipa(struct net_device *dev)
+{
+	struct gfar_private *priv = netdev_priv(dev);
+	struct mii_bus *bus = priv->mii_bus;
+	struct gianfar_mdio_data *mdata = (struct gianfar_mdio_data *)bus->priv;
+
 	/* Assign the TBI an address which won't conflict with the PHYs */
-	gfar_write(&priv->regs->tbipa, TBIPA_VALUE);
+	gfar_write(&priv->regs->tbipa, mdata->tbi_pa);
 }
 
 
diff --git a/drivers/net/gianfar.h b/drivers/net/gianfar.h
index 46cd773..771aa5e 100644
--- a/drivers/net/gianfar.h
+++ b/drivers/net/gianfar.h
@@ -130,7 +130,6 @@ extern const char gfar_driver_version[];
 #define DEFAULT_RXCOUNT	16
 #define DEFAULT_RXTIME	4
 
-#define TBIPA_VALUE		0x1f
 #define MIIMCFG_INIT_VALUE	0x00000007
 #define MIIMCFG_RESET           0x80000000
 #define MIIMIND_BUSY            0x00000001
diff --git a/drivers/net/gianfar_mii.c b/drivers/net/gianfar_mii.c
index 806df3f..41ff8c4 100644
--- a/drivers/net/gianfar_mii.c
+++ b/drivers/net/gianfar_mii.c
@@ -160,7 +160,7 @@ int gfar_mdio_probe(struct device *dev)
 	struct gfar_mii __iomem *regs;
 	struct mii_bus *new_bus;
 	struct resource *r;
-	int err = 0;
+	int i, err = 0;
 
 	if (NULL == dev)
 		return -EINVAL;
@@ -209,6 +209,15 @@ int gfar_mdio_probe(struct device *dev)
 		goto bus_register_fail;
 	}
 
+	/* mdiobus_register has populated the phy_map, so we now know
+ 	   which doghouses have wild dogs living in them.  Search
+	   backwards for the 1st vacant one for the Ten Bit Interface */
+	for (i = PHY_MAX_ADDR - 1; i >= 0; i--)
+		if (new_bus->phy_map[i] == NULL) break;
+
+	printk(KERN_INFO "Gianfar MDIO: using ID 0x%x for TBIPA\n", i);
+	pdata->tbi_pa = i;
+
 	return 0;
 
 bus_register_fail:
diff --git a/include/linux/fsl_devices.h b/include/linux/fsl_devices.h
index 4b304b6..285bd80 100644
--- a/include/linux/fsl_devices.h
+++ b/include/linux/fsl_devices.h
@@ -60,6 +60,7 @@ struct gianfar_mdio_data {
 	/* board specific information */
 	struct	gfar_mii __iomem *regs;
 	int	irq[32];
+	int	tbi_pa;
 };
 
 /* Flags related to gianfar device features */
-- 
1.5.4.3

^ permalink raw reply related

* [PATCH 3/5] gianfar: limit scope of gfar_local_mdio functions
From: Paul Gortmaker @ 2008-04-10 17:52 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: scottwood, linux-net, Paul Gortmaker
In-Reply-To: <4764055962e0a42ce083783bbb897a62090ad90b.1207846542.git.paul.gortmaker@windriver.com>

The gfar_local_mdio_read/write functions were brought in via
extern since they go right at the regs vs. going via the normal
gfar_mdio_read/write functions.  With the priv->mii_bus properly
set, we can get rid of the externs, the casts etc. and use the
normal gfar_mdio_read/write. We just need to move the call to the
gfar_configure_serdes down slightly to after where priv->mii_bus
is set to a sane value.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/net/gianfar.c     |   17 +++++++----------
 drivers/net/gianfar_mii.c |    4 ++--
 2 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 0ad74c1..9173784 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -130,8 +130,6 @@ static void free_skb_resources(struct gfar_private *priv);
 static void gfar_set_multi(struct net_device *dev);
 static void gfar_set_hash_for_addr(struct net_device *dev, u8 *addr);
 static void gfar_configure_serdes(struct net_device *dev);
-extern int gfar_local_mdio_write(struct gfar_mii __iomem *regs, int mii_id, int regnum, u16 value);
-extern int gfar_local_mdio_read(struct gfar_mii __iomem *regs, int mii_id, int regnum);
 #ifdef CONFIG_GFAR_NAPI
 static int gfar_poll(struct napi_struct *napi, int budget);
 #endif
@@ -459,9 +457,6 @@ static int init_phy(struct net_device *dev)
 
 	phydev = phy_connect(dev, phy_id, &adjust_link, 0, interface);
 
-	if (interface == PHY_INTERFACE_MODE_SGMII)
-		gfar_configure_serdes(dev);
-
 	if (IS_ERR(phydev)) {
 		printk(KERN_ERR "%s: Could not attach to PHY\n", dev->name);
 		return PTR_ERR(phydev);
@@ -474,27 +469,29 @@ static int init_phy(struct net_device *dev)
 	priv->mii_bus = phydev->bus;
 	priv->phydev = phydev;
 
+	if (interface == PHY_INTERFACE_MODE_SGMII)
+		gfar_configure_serdes(dev);
+
 	return 0;
 }
 
 static void gfar_configure_serdes(struct net_device *dev)
 {
 	struct gfar_private *priv = netdev_priv(dev);
-	struct gfar_mii __iomem *regs =
-			(void __iomem *)&priv->regs->gfar_mii_regs;
+	struct mii_bus *bus = priv->mii_bus;
 
 	/* Initialise TBI i/f to communicate with serdes (lynx phy) */
 
 	/* Single clk mode, mii mode off(for aerdes communication) */
-	gfar_local_mdio_write(regs, TBIPA_VALUE, MII_TBICON, TBICON_CLK_SELECT);
+	gfar_mdio_write(bus, TBIPA_VALUE, MII_TBICON, TBICON_CLK_SELECT);
 
 	/* Supported pause and full-duplex, no half-duplex */
-	gfar_local_mdio_write(regs, TBIPA_VALUE, MII_ADVERTISE,
+	gfar_mdio_write(bus, TBIPA_VALUE, MII_ADVERTISE,
 			ADVERTISE_1000XFULL | ADVERTISE_1000XPAUSE |
 			ADVERTISE_1000XPSE_ASYM);
 
 	/* ANEG enable, restart ANEG, full duplex mode, speed[1] set */
-	gfar_local_mdio_write(regs, TBIPA_VALUE, MII_BMCR, BMCR_ANENABLE |
+	gfar_mdio_write(bus, TBIPA_VALUE, MII_BMCR, BMCR_ANENABLE |
 			BMCR_ANRESTART | BMCR_FULLDPLX | BMCR_SPEED1000);
 }
 
diff --git a/drivers/net/gianfar_mii.c b/drivers/net/gianfar_mii.c
index 2432762..d5efa9c 100644
--- a/drivers/net/gianfar_mii.c
+++ b/drivers/net/gianfar_mii.c
@@ -51,7 +51,7 @@
  * the local mdio pins, which may not be the same as system mdio bus, used for
  * controlling the external PHYs, for example.
  */
-int gfar_local_mdio_write(struct gfar_mii __iomem *regs, int mii_id,
+static int gfar_local_mdio_write(struct gfar_mii __iomem *regs, int mii_id,
 			  int regnum, u16 value)
 {
 	/* Set the PHY address and the register address we want to write */
@@ -77,7 +77,7 @@ int gfar_local_mdio_write(struct gfar_mii __iomem *regs, int mii_id,
  * and are always tied to the local mdio pins, which may not be the
  * same as system mdio bus, used for controlling the external PHYs, for eg.
  */
-int gfar_local_mdio_read(struct gfar_mii __iomem *regs, int mii_id, int regnum)
+static int gfar_local_mdio_read(struct gfar_mii __iomem *regs, int mii_id, int regnum)
 
 {
 	u16 value;
-- 
1.5.4.3

^ permalink raw reply related

* [PATCH 4/5] gianfar: dont hog the mii_bus->priv with just the regs.
From: Paul Gortmaker @ 2008-04-10 17:52 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: scottwood, linux-net, Paul Gortmaker
In-Reply-To: <4764055962e0a42ce083783bbb897a62090ad90b.1207846542.git.paul.gortmaker@windriver.com>

The gfar was previously using the mii_bus->priv to directly
store the gfar_mii regs, which leaves no room/flexibility to
store anything else there.  I believe it makes more sense to
have mii_bus->priv point at a struct gianfar_mdio_data and
then have the regs stored as a field within that.  It makes
the code easier to read too.

This isn't in a hot path, so there should be no performance
penalty associated with this.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/net/gianfar_mii.c   |   17 ++++++++++-------
 include/linux/fsl_devices.h |    1 +
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/net/gianfar_mii.c b/drivers/net/gianfar_mii.c
index d5efa9c..806df3f 100644
--- a/drivers/net/gianfar_mii.c
+++ b/drivers/net/gianfar_mii.c
@@ -104,10 +104,10 @@ static int gfar_local_mdio_read(struct gfar_mii __iomem *regs, int mii_id, int r
  * All PHY configuration is done through the TSEC1 MIIM regs */
 int gfar_mdio_write(struct mii_bus *bus, int mii_id, int regnum, u16 value)
 {
-	struct gfar_mii __iomem *regs = (void __iomem *)bus->priv;
+	struct gianfar_mdio_data *mdata = (struct gianfar_mdio_data *)bus->priv;
 
 	/* Write to the local MII regs */
-	return(gfar_local_mdio_write(regs, mii_id, regnum, value));
+	return(gfar_local_mdio_write(mdata->regs, mii_id, regnum, value));
 }
 
 /* Read the bus for PHY at addr mii_id, register regnum, and
@@ -115,16 +115,17 @@ int gfar_mdio_write(struct mii_bus *bus, int mii_id, int regnum, u16 value)
  * configuration has to be done through the TSEC1 MIIM regs */
 int gfar_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
 {
-	struct gfar_mii __iomem *regs = (void __iomem *)bus->priv;
+	struct gianfar_mdio_data *mdata = (struct gianfar_mdio_data *)bus->priv;
 
 	/* Read the local MII regs */
-	return(gfar_local_mdio_read(regs, mii_id, regnum));
+	return(gfar_local_mdio_read(mdata->regs, mii_id, regnum));
 }
 
 /* Reset the MIIM registers, and wait for the bus to free */
 int gfar_mdio_reset(struct mii_bus *bus)
 {
-	struct gfar_mii __iomem *regs = (void __iomem *)bus->priv;
+	struct gianfar_mdio_data *mdata = (struct gianfar_mdio_data *)bus->priv;
+	struct gfar_mii __iomem *regs = mdata->regs;
 	unsigned int timeout = PHY_INIT_TIMEOUT;
 
 	mutex_lock(&bus->mdio_lock);
@@ -192,7 +193,8 @@ int gfar_mdio_probe(struct device *dev)
 		goto reg_map_fail;
 	}
 
-	new_bus->priv = (void __force *)regs;
+	new_bus->priv = pdata;
+	pdata->regs = (void __force *)regs;
 
 	new_bus->irq = pdata->irq;
 
@@ -221,12 +223,13 @@ reg_map_fail:
 int gfar_mdio_remove(struct device *dev)
 {
 	struct mii_bus *bus = dev_get_drvdata(dev);
+	struct gianfar_mdio_data *mdata = (struct gianfar_mdio_data *)bus->priv;
 
 	mdiobus_unregister(bus);
 
 	dev_set_drvdata(dev, NULL);
 
-	iounmap((void __iomem *)bus->priv);
+	iounmap(mdata->regs);
 	bus->priv = NULL;
 	kfree(bus);
 
diff --git a/include/linux/fsl_devices.h b/include/linux/fsl_devices.h
index 1831b19..4b304b6 100644
--- a/include/linux/fsl_devices.h
+++ b/include/linux/fsl_devices.h
@@ -58,6 +58,7 @@ struct gianfar_platform_data {
 
 struct gianfar_mdio_data {
 	/* board specific information */
+	struct	gfar_mii __iomem *regs;
 	int	irq[32];
 };
 
-- 
1.5.4.3

^ permalink raw reply related

* [PATCH 1/5] phylib: don't create a phydev for ID-less PHYs.
From: Paul Gortmaker @ 2008-04-10 17:51 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: scottwood, linux-net, Paul Gortmaker
In-Reply-To: <1207849922-2436-1-git-send-email-paul.gortmaker@windriver.com>

I've tested on 8360, 8540 and 8641D and in all cases, the PHY
ID returned for bus addr 0x1f is all zeros, and not all 0xf.
This means we've been allocating a phydev for this "ghost".

In addition to marking 0x0 as an invalid PHY ID, I've also
changed the existing somewhat useless printk to actually
list the bus IDs where it found a PHY so we get a basic
bus summary.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/net/phy/mdio_bus.c   |    4 +++-
 drivers/net/phy/phy_device.c |    5 +++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 963630c..e33a119 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -59,6 +59,7 @@ int mdiobus_register(struct mii_bus *bus)
 	if (bus->reset)
 		bus->reset(bus);
 
+	pr_info("%s: PHY(s) at:", bus->name);
 	for (i = 0; i < PHY_MAX_ADDR; i++) {
 		struct phy_device *phydev;
 
@@ -97,12 +98,13 @@ int mdiobus_register(struct mii_bus *bus)
 				phy_device_free(phydev);
 				phydev = NULL;
 			}
+			printk(" 0x%x", i);
 		}
 
 		bus->phy_map[i] = phydev;
 	}
 
-	pr_info("%s: probed\n", bus->name);
+	printk("\n");
 
 	return err;
 }
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index f4c4fd8..740dd2e 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -116,8 +116,9 @@ struct phy_device * get_phy_device(struct mii_bus *bus, int addr)
 
 	phy_id |= (phy_reg & 0xffff);
 
-	/* If the phy_id is all Fs, there is no device there */
-	if (0xffffffff == phy_id)
+	/* If the phy_id is all Fs, there is no device there. Similarly
+ 	   it seems common to get an ID of zero at addr 0x1f */
+	if (0xffffffff == phy_id || 0 == phy_id)
 		return NULL;
 
 	dev = phy_device_create(bus, addr, phy_id);
-- 
1.5.4.3

^ permalink raw reply related


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