LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: asm-ppc header issues when building ARCH=powerpc
From: David Gibson @ 2007-08-23  2:49 UTC (permalink / raw)
  To: Josh Boyer; +Cc: linuxppc-dev@ozlabs.org list, Paul Mackerras
In-Reply-To: <20070822143047.6c8a0d4e@weaponx.rchland.ibm.com>

On Wed, Aug 22, 2007 at 02:30:47PM -0500, Josh Boyer wrote:
> On Wed, 22 Aug 2007 10:19:21 -0500
> Kumar Gala <galak@kernel.crashing.org> wrote:
> 
> > Guys,
> > 
> > I was wondering if I could get your help with looking at the  
> > following lists and determining if we have an issue or not related  
> > the following files:
> > 
> > Getting some classification on these would be good.  Possibly  
> > classifications, doesn't build in ARCH=powerpc, remove include, real  
> > issue, etc.
> 
> Sure.
> 
> > ./drivers/i2c/busses/i2c-ibm_iic.c:#include <asm/ocp.h>
> > ./drivers/i2c/busses/i2c-ibm_iic.c:#include <asm/ibm4xx.h>
> 
> These one depends on IBM_OCP in Kconfig.  We don't select/enable that on
> any existing arch/powerpc 4xx stuff so it won't be built anyway.

Nor will we ever enable IBM_OCP in arch/powerpc: the device tree
entirely obsoletes the OCP crap.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* [PATCH] ucc_geth: kill unused include
From: Kumar Gala @ 2007-08-23  2:51 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: netdev, Li Yang, Paul Mackerras, linuxppc-dev

The ucc_geth_mii code is based on the gianfar_mii code that use to include
ocp.h.  ucc never need this and it causes issues when we want to kill
arch/ppc includes from arch/powerpc.

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---

Jeff, if you issue with this for 2.6.23, I'd prefer to push this via
the powerpc.git trees in 2.6.24 as part of a larger cleanup.  Let me know
one way or the other.

- k

 drivers/net/ucc_geth_mii.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ucc_geth_mii.c b/drivers/net/ucc_geth_mii.c
index 6c257b8..df884f0 100644
--- a/drivers/net/ucc_geth_mii.c
+++ b/drivers/net/ucc_geth_mii.c
@@ -32,7 +32,6 @@
 #include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
-#include <asm/ocp.h>
 #include <linux/crc32.h>
 #include <linux/mii.h>
 #include <linux/phy.h>
-- 
1.5.2.4

^ permalink raw reply related

* Re: asm-ppc header issues when building ARCH=powerpc
From: David Gibson @ 2007-08-23  2:47 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev@ozlabs.org list, Paul Mackerras
In-Reply-To: <75EDD900-3B5E-4D08-9311-13B6510EDE6E@kernel.crashing.org>

On Wed, Aug 22, 2007 at 10:19:21AM -0500, Kumar Gala wrote:
> Guys,
> 
> I was wondering if I could get your help with looking at the  
> following lists and determining if we have an issue or not related  
> the following files:
> 
> Getting some classification on these would be good.  Possibly  
> classifications, doesn't build in ARCH=powerpc, remove include, real  
> issue, etc.
> 
> - k
> 
> ./include/asm-powerpc/irq.h:#include <asm/mpc83xx.h>	- protected by ! 
> CONFIG_PPC_MERGE

irq.h seems to have a big slab of CONFIG_PPC_MERGE dependent stuff.
Looks like a good candidate for splitting into asm-ppc/irq.h and
asm-powerpc/irq.h

> ./drivers/ide/ppc/mpc8xx.c:#include <asm/residual.h>

Since mpc8xx is certainly not PReP, I don't think there's any way it
ought to be including residual.h

> ./drivers/mtd/maps/tqm834x.c:#include <asm/ppcboot.h>
> ./drivers/mtd/maps/pq2fads.c:#include <asm/ppcboot.h>

Although these both have an extern of type bd_t (defined in
ppcboot.h), afaict they don't actually use it, so these should be
removable.  Longer term, all these ugly hardcoded map files should be
relegated to arch/ppc only, replaced with physmap_of and suitable
information in the device tree for arch/powerpc.  However, being able
to build them on arch/powerpc may be useful during transition.

> ./drivers/i2c/busses/i2c-ibm_iic.c:#include <asm/ibm4xx.h>

This driver will need significant reworking to port to arch/powerpc
(similar to the treatment ibm_emac has received).  Such rework will
remove the dependency on ibm4xx.h and ocp.h

> ./drivers/mtd/maps/walnut.c:#include <asm/ibm4xx.h>

Suspect it's not needed.  But in any case, this too should be replaced
by physmap_of for arch/powerpc.

> ./include/asm-powerpc/irq.h:#include <asm/ibm4xx.h>	- protected by ! 
> CONFIG_PPC_MERGE
> 
> ./drivers/mtd/maps/ebony.c:#include <asm/ibm44x.h>

Ebony flash now works with physmap_of, so this is only used on
arch/ppc.

> ./drivers/mtd/maps/ocotea.c:#include <asm/ibm44x.h>

As for drivers/mtd/maps/walnut.c.

> ./drivers/mtd/nand/ndfc.c:#include <asm/ibm44x.h>

This probably also wants rework to make a device-tree-aware
arch/powerpc version.

> ./include/asm-powerpc/irq.h:#include <asm/ibm44x.h>	- protected by ! 
> CONFIG_PPC_MERGE
> 
> ./drivers/i2c/busses/i2c-ibm_iic.c:#include <asm/ocp.h>

See above.

> ./drivers/net/ucc_geth_mii.c:#include <asm/ocp.h>	- just bogus, needs  
> removal
> ./drivers/net/ibm_emac/ibm_emac_core.c:#include <asm/ocp.h>
> ./drivers/net/ibm_emac/ibm_emac_core.h:#include <asm/ocp.h>
> ./drivers/net/ibm_emac/ibm_emac_tah.h:#include <asm/ocp.h>
> ./drivers/net/ibm_emac/ibm_emac_phy.c:#include <asm/ocp.h>
> ./drivers/net/ibm_emac/ibm_emac_mal.c:#include <asm/ocp.h>
> ./drivers/net/ibm_emac/ibm_emac_zmii.h:#include <asm/ocp.h>

ibm_emac is arch/ppc only.  BenH and my 'ibm_newemac' patch adds a
new, device-tree aware arch/powerpc port of this driver.

> 
> ./drivers/macintosh/adb-iop.c:#include <asm/bootinfo.h>

I think this is just unnecessary, maybe also for a bunch of the ones
below.

> ./drivers/char/vme_scc.c:#include <asm/bootinfo.h>
> ./drivers/char/serial167.c:#include <asm/bootinfo.h>
> ./drivers/serial/dz.c:#include <asm/bootinfo.h>
> ./drivers/mtd/devices/ms02-nv.c:#include <asm/bootinfo.h>
> ./drivers/net/macsonic.c:#include <asm/bootinfo.h>
> ./drivers/net/jazzsonic.c:#include <asm/bootinfo.h>
> ./drivers/video/pmag-aa-fb.c:#include <asm/bootinfo.h>
> ./drivers/video/maxinefb.c:#include <asm/bootinfo.h>
> ./drivers/video/logo/logo.c:#include <asm/bootinfo.h>
> ./drivers/video/valkyriefb.c:#include <asm/bootinfo.h>
> ./drivers/video/macfb.c:#include <asm/bootinfo.h>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: Section mismatch warning
From: Kumar Gala @ 2007-08-23  2:36 UTC (permalink / raw)
  To: sivaji; +Cc: linuxppc-dev
In-Reply-To: <12286518.post@talk.nabble.com>


On Aug 22, 2007, at 9:30 PM, sivaji wrote:

>
> Hi,
>         When compiling the 2.6.23-rc3 kernel,  i got following warning
> messages.
>
> WARNING: vmlinux.o(.text+0x18): Section mismatch: reference to
> .init.text:early_init (between '__start' and '__after_mmu_off')
> WARNING: vmlinux.o(.text+0x3834): Section mismatch: reference to
> .init.text:machine_init (between 'start_here' and 'set_context')
> WARNING: vmlinux.o(.text+0x383c): Section mismatch: reference to
> .init.text:MMU_init (between 'start_here' and 'set_context')
> WARNING: vmlinux.o(.text+0x3866): Section mismatch: reference to
> .init.text:start_kernel (between 'start_here' and 'set_context')
> WARNING: vmlinux.o(.text+0x386a): Section mismatch: reference to
> .init.text:start_kernel (between 'start_here' and 'set_context')
>
> Processor            : 8641D
> Give some idea how to overcome this warning.

There is a patch posted on the list for this.  I doubt will fix these  
for 2.6.23 as they are just warnings.

http://patchwork.ozlabs.org/linuxppc/patch?id=13066

- k

^ permalink raw reply

* Section mismatch warning
From: sivaji @ 2007-08-23  2:30 UTC (permalink / raw)
  To: linuxppc-dev


Hi,
        When compiling the 2.6.23-rc3 kernel,  i got following warning
messages.

WARNING: vmlinux.o(.text+0x18): Section mismatch: reference to
.init.text:early_init (between '__start' and '__after_mmu_off')
WARNING: vmlinux.o(.text+0x3834): Section mismatch: reference to
.init.text:machine_init (between 'start_here' and 'set_context')
WARNING: vmlinux.o(.text+0x383c): Section mismatch: reference to
.init.text:MMU_init (between 'start_here' and 'set_context')
WARNING: vmlinux.o(.text+0x3866): Section mismatch: reference to
.init.text:start_kernel (between 'start_here' and 'set_context')
WARNING: vmlinux.o(.text+0x386a): Section mismatch: reference to
.init.text:start_kernel (between 'start_here' and 'set_context')

Processor            : 8641D
Give some idea how to overcome this warning. 

                
-- 
View this message in context: http://www.nabble.com/Section-mismatch-warning-tf4315101.html#a12286518
Sent from the linuxppc-dev mailing list archive at Nabble.com.

^ permalink raw reply

* Re: wmb vs mmiowb
From: Nick Piggin @ 2007-08-23  2:20 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: linux-ia64, Linus Torvalds, linuxppc-dev
In-Reply-To: <200708221202.12403.jesse.barnes@intel.com>

On Wed, Aug 22, 2007 at 12:02:11PM -0700, Jesse Barnes wrote:
> On Wednesday, August 22, 2007 11:07 am Linus Torvalds wrote:
> > > It really seems like it is some completely different concept from a
> > > barrier. And it shows, on the platform where it really matters
> > > (sn2), where the thing actually spins.
> >
> > I agree that it probably isn't a "write barrier" per se. Think of it
> > as a "tie two subsystems together" thing.
> 
> Right, maybe it's not the best name, but as long as you separate your 
> memory access types, you can think of it as a real write barrier, just 
> for mmio accesses (well uncached access really).

If we have the following situation (all vars start at 0)
CPU0			CPU1			CPU2
spin_lock(&lock);				~
A = 1;						~
wmb();						~
B = 2;						~
spin_unlock(&lock);				X = B;
			spin_lock(&lock);	rmb();
			A = 10;			Y = A;
			wmb();			~
			B = 11;			~
			spin_unlock(&lock);	~

(I use the '~' just to show CPU2 is not specifically temporally
related to CPU0 or CPU1).

Then CPU2 could have X==11 and Y==1, according to the Linux abstract
memory consistency model, couldn't it? I think so, and I think this
is what your mmiowb is trying to protect. In the above situation,
CPU2 would just use the spinlock -- I don't think we have a simple
primitive that CPU0 and 1 can call to prevent this reordering at
CPU2. An IO device obviously can't use a spinlock :).


> > (And it doesn't just matter on sn2. It also matters on powerpc64,
> > although I think they just set a flag and do the *real* sync in the
> > spin_unlock() path).
> 
> Yeah, they keep threatening to use this instead, but I'm not sure how 
> easy it would be.  Also they may have more devices/drivers to worry 
> about than sn2, so maybe changing over would mean too much driver 
> debugging (well auditing really since it's not that hard to know where 
> to put them).  Irix actually had an io_unlock() routine that did this 
> implicitly, but iirc that was shot down for Linux...

Why was it shot down? Seems like a pretty good idea to me ;)

I'm clueless when it comes to drivers, but I see a lot of mmiowb()
that are not paired with spin_unlock. How are these obvious? (ie.
what is the pattern?) It looks like some might be lockless FIFOs (or
maybe I'm just not aware of where the locks are). Can you just quickly
illustrate the problem being solved?

Thanks,
Nick

^ permalink raw reply

* Re: [PATCH 05/20] bootwrapper: flatdevtree fixes
From: David Gibson @ 2007-08-23  2:01 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, paulus
In-Reply-To: <20070822172456.GA22932@ld0162-tx32.am.freescale.net>

On Wed, Aug 22, 2007 at 12:24:56PM -0500, Scott Wood wrote:
> On Wed, Aug 22, 2007 at 11:09:07AM +1000, David Gibson wrote:
> > On Tue, Aug 21, 2007 at 11:09:58AM -0500, Scott Wood wrote:
> > > The point of #2 was as part of the fix to #1 -- otherwise, the same 
> > > check for NULL would have to be moved into ft_create_node to 
> > > conditionally call ft_find_device or ft_find_device_rel.
> > 
> > Um... oh, ok, I hadn't spotted that (1) made ft_create() use
> > find_device_rel().  That sounds doubly wrong: you have the internal
> > offset pointer, you should be able to create a phandle using the
> > phandle allocation stuff, rather than having to refind the node you've
> > just created from the parent.
> 
> Yeah, that'd make more sense.
> 
> > > >  (3) I really dislike;  I just don't see the point.
> > > 
> > > It's needed by dt_get_path().
> > 
> > No, it isn't.  dt_get_path() needs *some* way of getting the name of a
> > node, but it could be a separate function, which I think would be
> > preferable rather than folding it into getprop - you don't need to
> > search for the name, so a getname() function would have quite a
> > different structure to getprop().
> 
> I'd rather not add a new entry in ops just for that; it's more of an
> attribute of the dtb format that name is handled specially.  IIUC, on
> real OF you'd use the same code for both.

Actually, no - sorry, that's the other problem with this, which I
forgot to mention.  On real OF, the "name" property contains the
node's name *without the unit address*; that is, only the portion
before the '@'.  So your getprop change does not match real OF
behaviour - and real OF behaviour will not do what you want for
dt_get_path().

Actually, in any case, I don't think we want to implement get_path()
this way for real OF.  Better to have get_path() itself as a callback:
on real OF I believe we can directly ask for the full path to a given
phandle, the get name based implementation can then be made specific
to the flat device tree.

Or actually, I think we might be able to come up with a get_path()
implementation for flat tree that's less hideous than repeatedly
calling get_parent() which is an ugly, ugly operation on the flat tree
(and will get worse with libfdt).

> Plus, something might come along that needs to dynamically look for
> either name or something else.  It's more flexible this way.

Hrm... "something might come along" just seems contrived to me.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: wmb vs mmiowb
From: Nick Piggin @ 2007-08-23  1:59 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-ia64, Jesse Barnes, linuxppc-dev
In-Reply-To: <alpine.LFD.0.999.0708221049560.30176@woody.linux-foundation.org>

On Wed, Aug 22, 2007 at 11:07:32AM -0700, Linus Torvalds wrote:
> 
> 
> On Wed, 22 Aug 2007, Nick Piggin wrote:
> > 
> > It took me more than a glance to see what the difference is supposed to be
> > between wmb() and mmiowb(). I think especially because mmiowb isn't really
> > like a write barrier.
> 
> Well, it is, but it isn't. Not on its own - but together with a "normal" 
> barrier it is.
 
But it is stronger (or different) to write barrier semantics, because it
enforces the order in which a 3rd party (the IO device) sees writes from
multiple CPUs. The rest of our barrier concept is based purely on the
POV of the single entity executing the barrier.

Now it's needed because the IO device is not participating in the same
synchronisation logic that the CPUs are, which is why I say it is more
like a synchronisation primitive than a barrier primitive.


> > wmb is supposed to order all writes coming out of a single CPU, so that's
> > pretty simple.
> 
> No. wmb orders all *normal* writes coming out of a single CPU.

I'm pretty sure wmb() should order *all* writes, and smp_wmb() is what
you're thinking of for ordering regular writes to cacheable memory.
 

> It may not do anything at all for "uncached" IO writes that aren't part of 
> the cache coherency, and that are handled using totally different queues 
> (both inside and outside of the CPU)!
> 
> Now, on x86, the CPU actually tends to order IO writes *more* than it 
> orders any other writes (they are mostly entirely synchronous, unless the 
> area has been marked as write merging), but at least on PPC, it's the 
> other way around: without the cache as a serialization entry, you end up 
> having a totally separate queueu to serialize, and a regular-memory write 
> barrier does nothing at all to the IO queue.

Well PPC AFAIKS doesn't need the special synchronisation semantics of
this mmiowb primitive -- the reason it is not a noop is because the API
seems to also imply a wmb() (which is fine, and you'd normally want that
eg.  uncacheable stores must be ordered with the spin_unlock store).

It is just implemented with the PPC sync instruction, which just orders
all stores coming out of _this_ CPU. Their IO fabric must prevent IOs
from being reordered between CPUs if they're executed in a known order
(which is what Altix does not prevent).

 
> So think of the IO write queue as something totally asynchronous that has 
> zero connection to the normal write ordering - and then think of mmiowb() 
> as a way to *insert* a synchronization point.

If wmb (the non _smp one) orders all stores including IO stores, then it
should be sufficient to prevent IO writes from leaking out of a critical
section. The problem is that the "reader" (the IO device) itself is not
coherent. So _synchronisation_ point is right; it is not really a barrier.
Basically it says all IO writes issued by this CPU at this point will be
seen before any other IO writes issued by any other CPUs subsequently.

make_mmio_coherent()? queue_mmio_writes()? (I'd still prefer some kind of
acquire/release API that shows why CPU/CPU order matters too, and how it
is taken care of).


> > It really seems like it is some completely different concept from a
> > barrier. And it shows, on the platform where it really matters (sn2), where
> > the thing actually spins.
> 
> I agree that it probably isn't a "write barrier" per se. Think of it as a 
> "tie two subsystems together" thing.

Yes, in a way it is more like that. Which does fit with my suggestions
for a name.


> (And it doesn't just matter on sn2. It also matters on powerpc64, although 
> I think they just set a flag and do the *real* sync in the spin_unlock() 
> path).
> 
> Side note: the thing that makes "mmiowb()" even more exciting is that it's 
> not just the CPU, it's the fabric outside the CPU that matters too. That's 
> why the sn2 needs this - but the powerpc example shows a case where the 
> ordering requirement actually comes from the CPU itself.

Well I think sn2 is the *only* reason it matters.  When the ordering
requirement is coming from the CPU itself, that *is* just a traditional
write barrier (one which orders normal and io writes).

The funny things powerpc are doing in spin_unlock/etc. are a different
issue. Basically they are just helping along device drivers who get this
wrong and assume spinlocks order IOs; our lack of an acquire/release API
for IOs... they're just trying to get through this sorry state of affairs
without going insane ;) Powerpc is special here because their ordering
instructions distinguish between normal and IO, wheras most others don't
(including ia64, alpha, etc), so _most_ others do get their IOs ordered by
critical sections. This is a different issue to the mmiowb one (but still
shows that our APIs could be improved).

Why don't we get a nice easy spin_lock_io/spin_unlock_io, which takes
care of all the mmiowb and iowrite vs spin unlock problems? (individual
IOs within the lock would still need to be ordered as approprite).

Then we could also have a serialize_io()/unserialize_io() that takes
care of the same things but can be used when we have something other
than a spinlock for ordering CPUs (serialize_io may be a noop, but it
is good to ensure people are thinking about how they're excluding
other CPUs here -- if other CPUs are not excluded, then any code calling
mmiowb is buggy, right?).

^ permalink raw reply

* Re: [Kgdb-bugreport] 2.6.23-rc3-mm1: kgdb build failure on powerpc
From: Jason Wessel @ 2007-08-22 22:44 UTC (permalink / raw)
  To: Andrew Morton
  Cc: kgdb-bugreport, amitkale, linux-kernel, linuxppc-dev,
	Mariusz Kozlowski, Paul Mackerras
In-Reply-To: <20070822124743.fc316963.akpm@linux-foundation.org>

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

Andrew Morton wrote:
> On Wed, 22 Aug 2007 21:04:28 +0200
> Mariusz Kozlowski <m.kozlowski@tuxland.pl> wrote:
>
>   
>> Hello,
>>
>> 	Got that on imac g3.
>>
>>   CC      kernel/kgdb.o
>> kernel/kgdb.c: In function 'kgdb_handle_exception':
>> kernel/kgdb.c:940: error: invalid lvalue in unary '&'
>> kernel/kgdb.c:940: warning: type defaults to 'int' in declaration of '_o_'
>> kernel/kgdb.c:940: error: invalid lvalue in unary '&'
>> kernel/kgdb.c:940: warning: type defaults to 'int' in declaration of '_n_'
>> kernel/kgdb.c:940: error: invalid lvalue in unary '&'
>> kernel/kgdb.c:940: error: invalid lvalue in unary '&'
>> kernel/kgdb.c:940: error: invalid lvalue in unary '&'
>> kernel/kgdb.c:940: warning: type defaults to 'int' in declaration of 'type name'
>> make[1]: *** [kernel/kgdb.o] Blad 1
>> make: *** [kernel] Blad 2
>>
>>     
>
>   
Against the tip of the kernel + kgdb patches this config builds.  I 
wonder if is the compiler or the macros for atomic_read or cmpxchg have 
changed for in the -mm tree.  Perhaps it is not relevant though if you 
read on.
> I'm not surprised.
>
> 	while (cmpxchg(&atomic_read(&debugger_active), 0, (procid + 1)) != 0) {
>
> a) cmpxchg isn't available on all architectures
>
>   
It was available for all the archs that the kgdb had been implemented on 
at the time.
> b) we can't just go and take the address of atomic_read()'s return value!
>
>   
Perhaps yes, perhaps no I guess it depends on what actually gets 
generated...  In the past the intent of this was to guard for the race 
to be the master processor and looked like some attempt to do it 
atomically.  This code had been in use for a number of years at this point.
> c) that's pretty ugly-looking stuff anyway.
>
>   

Perhaps there is a cleaner way to do the same thing and avoid the 
cmpxchg all together.  I used the attached patch to eliminate the 
cmpxchg operation.


Jason.

[-- Attachment #2: kgdb_enter_atomic.patch --]
[-- Type: text/plain, Size: 2028 bytes --]

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>

---
 kernel/kgdb.c |   18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

--- a/kernel/kgdb.c
+++ b/kernel/kgdb.c
@@ -121,6 +121,7 @@ struct task_struct *kgdb_usethread, *kgd
 
 int debugger_step;
 atomic_t debugger_active;
+static atomic_t kgdb_sync = ATOMIC_INIT(-1);
 
 /* Our I/O buffers. */
 static char remcom_in_buffer[BUFMAX];
@@ -638,8 +639,14 @@ static void kgdb_wait(struct pt_regs *re
 	kgdb_info[processor].task = current;
 	atomic_set(&procindebug[processor], 1);
 
+	/* The master processor must be active to enter here, but this is
+	 * gaurd in case the master processor had not been selected if
+	 * this was an entry via nmi.
+	 */
+	while (!atomic_read(&debugger_active));
+
 	/* Wait till master processor goes completely into the debugger.
-	 * FIXME: this looks racy */
+	 */
 	while (!atomic_read(&procindebug[atomic_read(&debugger_active) - 1])) {
 		int i = 10;	/* an arbitrary number */
 
@@ -973,8 +980,13 @@ int kgdb_handle_exception(int ex_vector,
 	/* Hold debugger_active */
 	procid = raw_smp_processor_id();
 
-	while (cmpxchg(&atomic_read(&debugger_active), 0, (procid + 1)) != 0) {
+	while (1) {
 		int i = 25;	/* an arbitrary number */
+		if (atomic_read(&kgdb_sync) < 0 &&
+			atomic_inc_and_test(&kgdb_sync)) {
+			atomic_set(&debugger_active, procid + 1);
+			break;
+		}
 
 		while (--i)
 			cpu_relax();
@@ -991,6 +1003,7 @@ int kgdb_handle_exception(int ex_vector,
 	if (atomic_read(&cpu_doing_single_step) != -1 &&
 	    atomic_read(&cpu_doing_single_step) != procid) {
 		atomic_set(&debugger_active, 0);
+		atomic_set(&kgdb_sync, -1);
 		clocksource_touch_watchdog();
 		kgdb_softlock_skip[procid] = 1;
 		local_irq_restore(flags);
@@ -1557,6 +1570,7 @@ int kgdb_handle_exception(int ex_vector,
  kgdb_restore:
 	/* Free debugger_active */
 	atomic_set(&debugger_active, 0);
+	atomic_set(&kgdb_sync, -1);
 	clocksource_touch_watchdog();
 	kgdb_softlock_skip[processor] = 1;
 	local_irq_restore(flags);

^ permalink raw reply

* [POWERPC] add clrsetbits macros
From: Timur Tabi @ 2007-08-23  1:07 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Timur Tabi

This patch adds the clrsetbits_xxx() macros, which are used to set and clear
multiple bits in a single read-modify-write operation.  Specify the bits to
clear in the 'clear' parameter and the bits to set in the 'set' parameter.
These macros can also be used to set a multiple-bit bit pattern using a mask,
by specifying the mask in the 'clear' parameter and the new bit pattern in the
'set' parameter.  There are big-endian and little-endian versions for 8, 16,
32, and 64 bits.

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

put back the original functions and just based them off clrsetbits().

 include/asm-powerpc/io.h |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/include/asm-powerpc/io.h b/include/asm-powerpc/io.h
index 4c0b550..38f03bc 100644
--- a/include/asm-powerpc/io.h
+++ b/include/asm-powerpc/io.h
@@ -737,6 +737,29 @@ static inline void * bus_to_virt(unsigned long address)
 #define setbits8(_addr, _v) out_8((_addr), in_8(_addr) |  (_v))
 #define clrbits8(_addr, _v) out_8((_addr), in_8(_addr) & ~(_v))
 
+/* Clear and set bits in one shot.  These macros can be used to clear and
+ * set multiple bits in a register using a single read-modify-write.  These
+ * macros can also be used to set a multiple-bit bit pattern using a mask,
+ * by specifying the mask in the 'clear' parameter and the new bit pattern
+ * in the 'set' parameter.
+ */
+
+#define clrsetbits(type, addr, clear, set) \
+	out_##type((addr), (in_##type(addr) & ~(clear)) | (set))
+
+#ifdef __powerpc64__
+#define clrsetbits_be64(addr, clear, set) clrsetbits(be64, addr, clear, set)
+#define clrsetbits_le64(addr, clear, set) clrsetbits(le64, addr, clear, set)
+#endif
+
+#define clrsetbits_be32(addr, clear, set) clrsetbits(be32, addr, clear, set)
+#define clrsetbits_le32(addr, clear, set) clrsetbits(le32, addr, clear, set)
+
+#define clrsetbits_be16(addr, clear, set) clrsetbits(be16, addr, clear, set)
+#define clrsetbits_le16(addr, clear, set) clrsetbits(le32, addr, clear, set)
+
+#define clrsetbits_8(addr, clear, set) clrsetbits(8, addr, clear, set)
+
 #endif /* __KERNEL__ */
 
 #endif /* _ASM_POWERPC_IO_H */
-- 
1.5.2.4

^ permalink raw reply related

* Re: [PATCH] [02/10] pasemi_mac: Stop using the pci config space accessors for register read/writes
From: Stephen Rothwell @ 2007-08-23  0:31 UTC (permalink / raw)
  To: Olof Johansson; +Cc: netdev, jgarzik, linuxppc-dev
In-Reply-To: <20070822141248.GC16830@lixom.net>

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

Hi Olof,

On Wed, 22 Aug 2007 09:12:48 -0500 Olof Johansson <olof@lixom.net> wrote:
>
> -static unsigned int read_iob_reg(struct pasemi_mac *mac, unsigned int reg)
> +static inline unsigned int read_iob_reg(struct pasemi_mac *mac, unsigned int reg)
          ^^^^^^
For static functions in C files, we tend not to bother marking them
inline any more as the compiler does a pretty good job theses days.

>  {
>  	unsigned int val;
>  
> -	pci_read_config_dword(mac->iob_pdev, reg, &val);
> +	val = in_le32(mac->iob_regs+reg);
> +
>  	return val;

Why not just "return in_le32(mac->iob_regs+reg);" ?
And similarly below?

> +static inline void __iomem * __devinit map_onedev(struct pci_dev *p, int index)
          ^^^^^^                ^^^^^^^^^
Mixing inline and __*init is just plain silly :-)

> +fallback:
> +	/* This is hardcoded and ugly, but we have some firmware versions
> +	 * who don't provide the register space in the device tree. Luckily
           ^^^
"that"

-- 
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] Move serial_dev_init to device_initcall()
From: Olof Johansson @ 2007-08-23  0:26 UTC (permalink / raw)
  To: paulus; +Cc: linuxppc-dev

With the I/O space rewrite by BenH, the legacy_serial serial_dev_init()
initcall is now called before I/O space is setup, but it's dependent on
it being available.

Since there's no way to make dependencies between initcalls, we'll just
have to move it to device_initcall(). Yes, it's suboptimal but I'm not
aware of any better solution at this time.


Signed-off-by: Olof Johansson <olof@lixom.net>

---

I think I might have one of the few hardware platforms with UART on PIO,
at least if noone else has already hit this. It was introduced by BenH's
rewrite of the I/O space allocation earlier, and the problem is that by
the time serial_dev_init() runs, the hose io_base_virt isn't available.

I still see the platform devices register before the PCI ones, so it
looks like init order is undisturbed by this change. Still, it'd be good
to hear test feedback from other platforms as needed. It's uncertain what
platforms the comments above serial_dev_init() about needing overrides
applies for, so I don't know which ones might be sensitive to call order
changes.


Index: mainline/arch/powerpc/kernel/legacy_serial.c
===================================================================
--- mainline.orig/arch/powerpc/kernel/legacy_serial.c
+++ mainline/arch/powerpc/kernel/legacy_serial.c
@@ -493,7 +493,7 @@ static int __init serial_dev_init(void)
 
 	return platform_device_register(&serial_device);
 }
-arch_initcall(serial_dev_init);
+device_initcall(serial_dev_init);
 
 
 /*

^ permalink raw reply

* Re: [Kgdb-bugreport] 2.6.23-rc3-mm1: kgdb build failure on powerpc
From: Andrew Morton @ 2007-08-22 23:53 UTC (permalink / raw)
  To: Jason Wessel
  Cc: kgdb-bugreport, amitkale, linux-kernel, linuxppc-dev,
	Mariusz Kozlowski, Paul Mackerras
In-Reply-To: <46CCBC3C.7010307@windriver.com>

On Wed, 22 Aug 2007 17:44:12 -0500
Jason Wessel <jason.wessel@windriver.com> wrote:

> Perhaps there is a cleaner way to do the same thing and avoid the 
> cmpxchg all together.  I used the attached patch to eliminate the 
> cmpxchg operation.
> 
> 
> Jason.
> 
> 
> [kgdb_enter_atomic.patch  text/plain (2.0KB)]
> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
> 
> ---
>  kernel/kgdb.c |   18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> --- a/kernel/kgdb.c
> +++ b/kernel/kgdb.c
> @@ -121,6 +121,7 @@ struct task_struct *kgdb_usethread, *kgd
>  
>  int debugger_step;
>  atomic_t debugger_active;
> +static atomic_t kgdb_sync = ATOMIC_INIT(-1);
>  
>  /* Our I/O buffers. */
>  static char remcom_in_buffer[BUFMAX];
> @@ -638,8 +639,14 @@ static void kgdb_wait(struct pt_regs *re
>  	kgdb_info[processor].task = current;
>  	atomic_set(&procindebug[processor], 1);
>  
> +	/* The master processor must be active to enter here, but this is
> +	 * gaurd in case the master processor had not been selected if
> +	 * this was an entry via nmi.
> +	 */
> +	while (!atomic_read(&debugger_active));

eek.  We're in the process of hunting down and eliminating exactly this
construct.  There have been cases where the compiler cached the
atomic_read() result in a register, turning the above into an infinite
loop.

Plus we should never add power-burners like that into the kernel anyway. 
That loop should have a cpu_relax() in it.  Which will also fix the
compiler problem described above.

Thirdly, please always add a newline when coding statements like that:

	while (expr())
		;

^ permalink raw reply

* Re: Patches added to powerpc.git for-2.6.24 branch
From: Kumar Gala @ 2007-08-22 23:28 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <200708230039.07061.arnd@arndb.de>


On Aug 22, 2007, at 5:39 PM, Arnd Bergmann wrote:

> On Wednesday 22 August 2007, Kumar Gala wrote:
>> I think we can all agree that the amiga*.h, zorro.h and traps.h can
>> be ignored since they have to do with amiga/apus support.
>
> arch/powerpc/kernel/setup_32.c unconditionally #includes <asm/ 
> amigappc.h>,
> the rest looks unused indeed.

Yeah, that's fixed up in my patches.

- k

^ permalink raw reply

* Re: Patches added to powerpc.git for-2.6.24 branch
From: Arnd Bergmann @ 2007-08-22 22:39 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Paul Mackerras
In-Reply-To: <694575D6-C4BC-4FFB-91C1-B692181CEF6F@kernel.crashing.org>

On Wednesday 22 August 2007, Kumar Gala wrote:
> I think we can all agree that the amiga*.h, zorro.h and traps.h can =A0
> be ignored since they have to do with amiga/apus support.

arch/powerpc/kernel/setup_32.c unconditionally #includes <asm/amigappc.h>,
the rest looks unused indeed.

	Arnd <><

^ permalink raw reply

* Getting lite5200 debian X11 up and running with CoralP
From: Mark Gibson @ 2007-08-22 22:41 UTC (permalink / raw)
  To: linuxppc-embedded

Hi,
I've been working on getting the lite5200 board running with the Denx
supplied coralP X11 drivers. I have the Fujitsu PCI card and the console
is working on it.  The denx site has tons of info and really got me up
and running in record time.  The X11 drivers and video are exactly what
I need to get going, but I have now started to drown in my lack of
experience.=20

I followed the denx app note to get debian up:
http://www.denx.de/wiki/DULG/AN2004_08_DebianOnPowerpcInstallationHowto

I used the 2.4.25 Kernel from denx (modified a bit to support USB HID
and to change the PCI address for my CoralPA card).



I have had two problems - both related to X11 / X / XF86.

1 - I have used apt to retrieve X and can run it using the generic
framebuffer. I get the cross-hatch background and the 'X' cursor only.
The only window manager APT will let me get is olvm, which I can't seem
to get to start up. I have a feeling that using debian-woody has left me
with a too-limited set of X packages (or the supporting libs/etc) to get
past this.
    =3D=3D Has anyone had debian-woody and the zvideo app running??

2 - When I use the denx drivers from the denx site I have this issue:
  (II) Module mb86290: vendor=3D"The XFree86 Project"
        compiled for 4.3.0.1, module version =3D 0.1.0
        Module class: XFree86 Video Driver
        ABI class: XFree86 Video Driver, version 0.6
  (EE) module ABI minor version (6) is newer than the server's version
(4)
  (II) UnloadModule: "mb86290"
  (II) Unloading /usr/X11R6/lib/modules/drivers/mb86290_drv.o
  (EE) Failed to load module "mb86290" (module requirement mismatch, 0)
  =20

Any insight would be appreciated.

Thanks,
Mark

^ permalink raw reply

* RFC: SPE misalignment handling framework
From: Kumar Gala @ 2007-08-22 21:32 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Paul Mackerras

This is just to see if the modification to fix_alignment is ok with
people. Still need to fill in the actual implementation of emulate_spe().

diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
index 4c47f9c..a9fcf25 100644
--- a/arch/powerpc/kernel/align.c
+++ b/arch/powerpc/kernel/align.c
@@ -189,6 +189,76 @@ static struct aligninfo aligninfo[128] = {
 	INVALID,		/* 11 1 1111 */
 };

+#ifdef CONFIG_SPE
+static struct aligninfo spe_aligninfo[64] = {
+	{ 8, LD },		/* 0 00 00 0: evlddx */
+	{ 8, LD },		/* 0 00 00 1: evldd */
+	{ 8, LD },		/* 0 00 01 0: evldwx */
+	{ 8, LD },		/* 0 00 01 1: evldw */
+	{ 8, LD },		/* 0 00 10 0: evldhx */
+	{ 8, LD },		/* 0 00 10 1: evldh */
+	INVALID,		/* 0 00 11 0 */
+	INVALID,		/* 0 00 11 1 */
+	{ 2, LD },		/* 0 01 00 0: evlhhesplatx */
+	{ 2, LD },		/* 0 01 00 1: evlhhesplat */
+	INVALID,		/* 0 01 01 0 */
+	INVALID,		/* 0 01 01 1 */
+	{ 2, LD },		/* 0 01 10 0: evlhhousplatx */
+	{ 2, LD },		/* 0 01 10 1: evlhhousplat */
+	{ 2, LD },		/* 0 01 11 0: evlhhossplatx */
+	{ 2, LD },		/* 0 01 11 1: evlhhossplat */
+	{ 4, LD },		/* 0 10 00 0: evlwhex */
+	{ 4, LD },		/* 0 10 00 1: evlwhe */
+	INVALID,		/* 0 10 01 0 */
+	INVALID,		/* 0 10 01 1 */
+	{ 4, LD },		/* 0 10 10 0: evlwhoux */
+	{ 4, LD },		/* 0 10 10 1: evlwhou */
+	{ 4, LD },		/* 0 10 11 0: evlwhosx */
+	{ 4, LD },		/* 0 10 11 1: evlwhos */
+	{ 4, LD },		/* 0 11 00 0: evlwwsplatx */
+	{ 4, LD },		/* 0 11 00 1: evlwwsplat */
+	INVALID,		/* 0 11 01 0 */
+	INVALID,		/* 0 11 01 1 */
+	{ 4, LD },		/* 0 11 10 0: evlwhsplatx */
+	{ 4, LD },		/* 0 11 10 1: evlwhsplat */
+	INVALID,		/* 0 11 11 0 */
+	INVALID,		/* 0 11 11 1 */
+
+	{ 8, ST },		/* 1 00 00 0: evstddx */
+	{ 8, ST },		/* 1 00 00 1: evstdd */
+	{ 8, ST },		/* 1 00 01 0: evstdwx */
+	{ 8, ST },		/* 1 00 01 1: evstdw */
+	{ 8, ST },		/* 1 00 10 0: evstdhx */
+	{ 8, ST },		/* 1 00 10 1: evstdh */
+	INVALID,		/* 1 00 11 0 */
+	INVALID,		/* 1 00 11 1 */
+	INVALID,		/* 1 01 00 0 */
+	INVALID,		/* 1 01 00 1 */
+	INVALID,		/* 1 01 01 0 */
+	INVALID,		/* 1 01 01 1 */
+	INVALID,		/* 1 01 10 0 */
+	INVALID,		/* 1 01 10 1 */
+	INVALID,		/* 1 01 11 0 */
+	INVALID,		/* 1 01 11 1 */
+	{ 4, ST },		/* 1 10 00 0: evstwhex */
+	{ 4, ST },		/* 1 10 00 1: evstwhe */
+	INVALID,		/* 1 10 01 0 */
+	INVALID,		/* 1 10 01 1 */
+	{ 4, ST },		/* 1 10 10 0: evstwhoux */
+	{ 4, ST },		/* 1 10 10 1: evstwhou */
+	INVALID,		/* 1 10 11 0 */
+	INVALID,		/* 1 10 11 1 */
+	{ 4, ST },		/* 1 11 00 0: evstwwex */
+	{ 4, ST },		/* 1 11 00 1: evstwwe */
+	INVALID,		/* 1 11 01 0 */
+	INVALID,		/* 1 11 01 1 */
+	{ 4, ST },		/* 1 11 10 0: evstwwox */
+	{ 4, ST },		/* 1 11 10 1: evstwwo */
+	INVALID,		/* 1 11 11 0 */
+	INVALID,		/* 1 11 11 1 */
+};
+#endif
+
 /*
  * Create a DSISR value from the instruction
  */
@@ -392,6 +462,103 @@ static int emulate_fp_pair(struct pt_regs *regs, unsigned char __user *addr,
 	return 1;	/* exception handled and fixed up */
 }

+#ifdef CONFIG_SPE
+/*
+ * Emulate SPE loads and stores.
+ * Only Book-E has these instructions, and it does true little-endian,
+ * so we don't need the address swizzling.
+ */
+static int emulate_spe(struct pt_regs *regs, unsigned char __user *addr,
+		       unsigned int reg, unsigned int nb,
+		       unsigned int flags, unsigned int instr)
+{
+	int i, ret;
+
+	/* userland only */
+	if (unlikely(!user_mode(regs)))
+		return 0;
+
+	flush_spe_to_thread(current);
+
+
+	return 1;
+}
+#endif /* CONFIG_SPE */

 /*
  * Called on alignment exception. Attempts to fixup
@@ -408,7 +575,7 @@ int fix_alignment(struct pt_regs *regs)
 	unsigned int dsisr;
 	unsigned char __user *addr;
 	unsigned long p, swiz;
-	int ret, t;
+	int ret, t, is_spe = 0;
 	union {
 		u64 ll;
 		double dd;
@@ -445,17 +612,31 @@ int fix_alignment(struct pt_regs *regs)
 		if (cpu_has_feature(CPU_FTR_REAL_LE) && (regs->msr & MSR_LE))
 			instr = cpu_to_le32(instr);
 		dsisr = make_dsisr(instr);
+#ifdef CONFIG_SPE
+		if ((instr >> 26) == 0x4)
+			is_spe = 1;
+#endif
 	}

 	/* extract the operation and registers from the dsisr */
 	reg = (dsisr >> 5) & 0x1f;	/* source/dest register */
 	areg = dsisr & 0x1f;		/* register to update */
-	instr = (dsisr >> 10) & 0x7f;
-	instr |= (dsisr >> 13) & 0x60;
+	if (!is_spe) {
+		instr = (dsisr >> 10) & 0x7f;
+		instr |= (dsisr >> 13) & 0x60;

-	/* Lookup the operation in our table */
-	nb = aligninfo[instr].len;
-	flags = aligninfo[instr].flags;
+		/* Lookup the operation in our table */
+		nb = aligninfo[instr].len;
+		flags = aligninfo[instr].flags;
+	}
+#ifdef CONFIG_SPE
+	else {
+		instr &= 0x3f;
+		/* Lookup the operation in our table */
+		nb = spe_aligninfo[instr].len;
+		flags = spe_aligninfo[instr].flags;
+	}
+#endif

 	/* Byteswap little endian loads and stores */
 	swiz = 0;
@@ -507,6 +688,14 @@ int fix_alignment(struct pt_regs *regs)
 		flush_fp_to_thread(current);
 	}

+#ifdef CONFIG_SPE
+	/* Force the fprs into the save area so we can reference them */
+	if (is_spe) {
+		return emulate_spe(regs, addr, reg, nb, flags, instr);
+
+	}
+#endif
+
 	/* Special case for 16-byte FP loads and stores */
 	if (nb == 16)
 		return emulate_fp_pair(regs, addr, reg, flags);

^ permalink raw reply related

* Re: [PATCH 4/7] fs_enet: mac-fcc: Eliminate __fcc-* macros.
From: Scott Wood @ 2007-08-22 21:17 UTC (permalink / raw)
  To: Vitaly Bordug; +Cc: netdev, jgarzik, linuxppc-dev
In-Reply-To: <20070823010826.6ae3452d@localhost.localdomain>

Vitaly Bordug wrote:
>>-#define W8(_p, _m, _v)	__fcc_out8(&(_p)->_m, (_v))
>>-#define R8(_p, _m)	__fcc_in8(&(_p)->_m)
>>+#define W8(_p, _m, _v)	out_8(&(_p)->_m, (_v))
>>+#define R8(_p, _m)	in_8(&(_p)->_m)
>> #define S8(_p, _m, _v)	W8(_p, _m, R8(_p, _m) | (_v))
>> #define C8(_p, _m, _v)	W8(_p, _m, R8(_p, _m) & ~(_v))
>> 
>>@@ -290,7 +281,7 @@ static void restart(struct net_device *dev)
>> 
>> 	/* clear everything (slow & steady does it) */
>> 	for (i = 0; i < sizeof(*ep); i++)
>>-		__fcc_out8((char *)ep + i, 0);
>>+		out_8((char *)ep + i, 0);
>> 
> 
> Perhaps W8() here, to keep consistency?

W8 expects a struct pointer and member, which we don't have here.

-Scott

^ permalink raw reply

* Re: [PATCH 6/7 v2] fs_enet: Be an of_platform device when CONFIG_PPC_CPM_NEW_BINDING is set.
From: Scott Wood @ 2007-08-22 21:16 UTC (permalink / raw)
  To: Vitaly Bordug; +Cc: netdev, jgarzik, linuxppc-dev
In-Reply-To: <20070823010057.74e1355a@localhost.localdomain>

Vitaly Bordug wrote:
> yes, wrong snippet what about
> 
> 
>> #ifdef CONFIG_CPM2 +	r = fs_enet_mdio_bb_init(); +	if (r != 0) +
>> goto out_mdio_bb; +#endif +#ifdef CONFIG_8xx +	r =
>> fs_enet_mdio_fec_init(); +	if (r != 0) +		goto out_mdio_fec; 
>> +#endif
> 
> 
> We had to pray and hope that 8xx would only have fec, and cpm2 has
> some bitbanged stuff. now we can inquire dts and know for sure, at
> least it seems so.

Yeah, that sucks.  We should add kconfig options for each mii type, and
let them have their own init functions.

That only affects the initcalls (and kernel size), though; it still uses 
the phy-handle to decide what mdio controller to actually talk to.

>> How is that different from the old code, where you're hosed without
>>  fep->fpi->bus_id?
>> 
> 
> 
> I wasn't defending old code, and consider "old code is POS, new one
> is just great" game meaningless. I am just stating the problem, that
> we'll have to address later. On 8xx even reference boards may be 
> without phy at all.

OK -- would it suffice to just never call any phylib functions and
always assume the link is up if the phy-handle property is undefined?

> ok, agreed, size is most serious judge here. we'll definitely have to
> revisit pin problem later too (because custom designs sometimes
> switch contradictory devices on-the-fly, disable soc parts for
> alternative function, etc.)

If it's really on-the-fly (and not jumpered/once-per-boot), then board 
code should expose some kind of API to switch the functions, acting like 
a hotplug bus.  Associating it with open/close of the device won't work 
if one of the devices is something like USB which needs to start working 
without any internal open()-like action.

-Scott

^ permalink raw reply

* Re: [PATCH 4/7] fs_enet: mac-fcc: Eliminate __fcc-* macros.
From: Vitaly Bordug @ 2007-08-22 21:08 UTC (permalink / raw)
  To: Scott Wood; +Cc: netdev, jgarzik, linuxppc-dev
In-Reply-To: <20070817175402.GD9218@ld0162-tx32.am.freescale.net>

On Fri, 17 Aug 2007 12:54:02 -0500
Scott Wood wrote:

> These macros accomplish nothing other than defeating type checking.
> 
> This patch also fixes one instance of the wrong register size being
> used that was revealed by enabling type checking.
> 
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> ---
>  drivers/net/fs_enet/mac-fcc.c |   25 ++++++++-----------------
>  1 files changed, 8 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/net/fs_enet/mac-fcc.c
> b/drivers/net/fs_enet/mac-fcc.c index ad3c5fa..8b30361 100644
> --- a/drivers/net/fs_enet/mac-fcc.c
> +++ b/drivers/net/fs_enet/mac-fcc.c
> @@ -48,28 +48,19 @@
>  
>  /* FCC access macros */
>  
> -#define __fcc_out32(addr, x)	out_be32((unsigned *)addr, x)
> -#define __fcc_out16(addr, x)	out_be16((unsigned short *)addr,
> x) -#define __fcc_out8(addr, x)	out_8((unsigned char *)addr, x)
> -#define __fcc_in32(addr)	in_be32((unsigned *)addr)
> -#define __fcc_in16(addr)	in_be16((unsigned short *)addr)
> -#define __fcc_in8(addr)		in_8((unsigned char *)addr)
> -
> -/* parameter space */
> -
>  /* write, read, set bits, clear bits */
> -#define W32(_p, _m, _v)	__fcc_out32(&(_p)->_m, (_v))
> -#define R32(_p, _m)	__fcc_in32(&(_p)->_m)
> +#define W32(_p, _m, _v)	out_be32(&(_p)->_m, (_v))
> +#define R32(_p, _m)	in_be32(&(_p)->_m)
>  #define S32(_p, _m, _v)	W32(_p, _m, R32(_p, _m) | (_v))
>  #define C32(_p, _m, _v)	W32(_p, _m, R32(_p, _m) & ~(_v))
>  
> -#define W16(_p, _m, _v)	__fcc_out16(&(_p)->_m, (_v))
> -#define R16(_p, _m)	__fcc_in16(&(_p)->_m)
> +#define W16(_p, _m, _v)	out_be16(&(_p)->_m, (_v))
> +#define R16(_p, _m)	in_be16(&(_p)->_m)
>  #define S16(_p, _m, _v)	W16(_p, _m, R16(_p, _m) | (_v))
>  #define C16(_p, _m, _v)	W16(_p, _m, R16(_p, _m) & ~(_v))
>  
> -#define W8(_p, _m, _v)	__fcc_out8(&(_p)->_m, (_v))
> -#define R8(_p, _m)	__fcc_in8(&(_p)->_m)
> +#define W8(_p, _m, _v)	out_8(&(_p)->_m, (_v))
> +#define R8(_p, _m)	in_8(&(_p)->_m)
>  #define S8(_p, _m, _v)	W8(_p, _m, R8(_p, _m) | (_v))
>  #define C8(_p, _m, _v)	W8(_p, _m, R8(_p, _m) & ~(_v))
>  
> @@ -290,7 +281,7 @@ static void restart(struct net_device *dev)
>  
>  	/* clear everything (slow & steady does it) */
>  	for (i = 0; i < sizeof(*ep); i++)
> -		__fcc_out8((char *)ep + i, 0);
> +		out_8((char *)ep + i, 0);
>  
Perhaps W8() here, to keep consistency?


>  	/* get physical address */
>  	rx_bd_base_phys = fep->ring_mem_addr;
> @@ -495,7 +486,7 @@ static void tx_kickstart(struct net_device *dev)
>  	struct fs_enet_private *fep = netdev_priv(dev);
>  	fcc_t *fccp = fep->fcc.fccp;
>  
> -	S32(fccp, fcc_ftodr, 0x80);
> +	S16(fccp, fcc_ftodr, 0x8000);
>  }
>  
>  static u32 get_int_events(struct net_device *dev)


-- 
Sincerely, Vitaly

^ permalink raw reply

* Re: [PATCH 2/7] fs_enet: Whitespace cleanup.
From: Vitaly Bordug @ 2007-08-22 21:04 UTC (permalink / raw)
  To: Scott Wood; +Cc: netdev, jgarzik, linuxppc-dev
In-Reply-To: <20070817175359.GB9218@ld0162-tx32.am.freescale.net>

On Fri, 17 Aug 2007 12:53:59 -0500
Scott Wood wrote:

> Signed-off-by: Scott Wood <scottwood@freescale.com>

Acked-by: Vitaly Bordug <vitb@kernel.crashing.org>


> ---
>  drivers/net/fs_enet/fs_enet-main.c |   85
> ++++++++++++++++-------------------
> drivers/net/fs_enet/fs_enet.h      |    4 +-
> drivers/net/fs_enet/mac-fcc.c      |    1 -
> drivers/net/fs_enet/mii-fec.c      |    1 - 4 files changed, 41
> insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/net/fs_enet/fs_enet-main.c
> b/drivers/net/fs_enet/fs_enet-main.c index a4a2a0e..f261b90 100644
> --- a/drivers/net/fs_enet/fs_enet-main.c
> +++ b/drivers/net/fs_enet/fs_enet-main.c
> @@ -353,7 +353,6 @@ static void fs_enet_tx(struct net_device *dev)
>  
>  	do_wake = do_restart = 0;
>  	while (((sc = CBDR_SC(bdp)) & BD_ENET_TX_READY) == 0) {
> -
>  		dirtyidx = bdp - fep->tx_bd_base;
>  
>  		if (fep->tx_free == fep->tx_ring)
> @@ -454,7 +453,6 @@ fs_enet_interrupt(int irq, void *dev_id)
>  
>  	nr = 0;
>  	while ((int_events = (*fep->ops->get_int_events)(dev)) != 0)
> { -
>  		nr++;
>  
>  		int_clr_events = int_events;
> @@ -710,45 +708,43 @@ static void fs_timeout(struct net_device *dev)
>   *-----------------------------------------------------------------------------*/
>  static void generic_adjust_link(struct  net_device *dev)
>  {
> -       struct fs_enet_private *fep = netdev_priv(dev);
> -       struct phy_device *phydev = fep->phydev;
> -       int new_state = 0;
> -
> -       if (phydev->link) {
> -
> -               /* adjust to duplex mode */
> -               if (phydev->duplex != fep->oldduplex){
> -                       new_state = 1;
> -                       fep->oldduplex = phydev->duplex;
> -               }
> -
> -               if (phydev->speed != fep->oldspeed) {
> -                       new_state = 1;
> -                       fep->oldspeed = phydev->speed;
> -               }
> -
> -               if (!fep->oldlink) {
> -                       new_state = 1;
> -                       fep->oldlink = 1;
> -                       netif_schedule(dev);
> -                       netif_carrier_on(dev);
> -                       netif_start_queue(dev);
> -               }
> -
> -               if (new_state)
> -                       fep->ops->restart(dev);
> -
> -       } else if (fep->oldlink) {
> -               new_state = 1;
> -               fep->oldlink = 0;
> -               fep->oldspeed = 0;
> -               fep->oldduplex = -1;
> -               netif_carrier_off(dev);
> -               netif_stop_queue(dev);
> -       }
> -
> -       if (new_state && netif_msg_link(fep))
> -               phy_print_status(phydev);
> +	struct fs_enet_private *fep = netdev_priv(dev);
> +	struct phy_device *phydev = fep->phydev;
> +	int new_state = 0;
> +
> +	if (phydev->link) {
> +		/* adjust to duplex mode */
> +		if (phydev->duplex != fep->oldduplex) {
> +			new_state = 1;
> +			fep->oldduplex = phydev->duplex;
> +		}
> +
> +		if (phydev->speed != fep->oldspeed) {
> +			new_state = 1;
> +			fep->oldspeed = phydev->speed;
> +		}
> +
> +		if (!fep->oldlink) {
> +			new_state = 1;
> +			fep->oldlink = 1;
> +			netif_schedule(dev);
> +			netif_carrier_on(dev);
> +			netif_start_queue(dev);
> +		}
> +
> +		if (new_state)
> +			fep->ops->restart(dev);
> +	} else if (fep->oldlink) {
> +		new_state = 1;
> +		fep->oldlink = 0;
> +		fep->oldspeed = 0;
> +		fep->oldduplex = -1;
> +		netif_carrier_off(dev);
> +		netif_stop_queue(dev);
> +	}
> +
> +	if (new_state && netif_msg_link(fep))
> +		phy_print_status(phydev);
>  }
>  
>  
> @@ -792,7 +788,6 @@ static int fs_init_phy(struct net_device *dev)
>  	return 0;
>  }
>  
> -
>  static int fs_enet_open(struct net_device *dev)
>  {
>  	struct fs_enet_private *fep = netdev_priv(dev);
> @@ -978,7 +973,7 @@ static struct net_device *fs_init_instance(struct
> device *dev, #endif
>  
>  #ifdef CONFIG_FS_ENET_HAS_SCC
> -	if (fs_get_scc_index(fpi->fs_no) >=0 )
> +	if (fs_get_scc_index(fpi->fs_no) >= 0)
>  		fep->ops = &fs_scc_ops;
>  #endif
>  
> @@ -1069,9 +1064,8 @@ static struct net_device
> *fs_init_instance(struct device *dev, 
>  	return ndev;
>  
> -      err:
> +err:
>  	if (ndev != NULL) {
> -
>  		if (registered)
>  			unregister_netdev(ndev);
>  
> @@ -1262,7 +1256,6 @@ static int __init fs_init(void)
>  err:
>  	cleanup_immap();
>  	return r;
> -	
>  }
>  
>  static void __exit fs_cleanup(void)
> diff --git a/drivers/net/fs_enet/fs_enet.h
> b/drivers/net/fs_enet/fs_enet.h index 569be22..72a61e9 100644
> --- a/drivers/net/fs_enet/fs_enet.h
> +++ b/drivers/net/fs_enet/fs_enet.h
> @@ -15,8 +15,8 @@
>  #include <asm/commproc.h>
>  
>  struct fec_info {
> -        fec_t*  fecp;
> -	u32     mii_speed;
> +	fec_t *fecp;
> +	u32 mii_speed;
>  };
>  #endif
>  
> diff --git a/drivers/net/fs_enet/mac-fcc.c
> b/drivers/net/fs_enet/mac-fcc.c index 5603121..ad3c5fa 100644
> --- a/drivers/net/fs_enet/mac-fcc.c
> +++ b/drivers/net/fs_enet/mac-fcc.c
> @@ -86,7 +86,6 @@
>  static inline int fcc_cr_cmd(struct fs_enet_private *fep, u32 mcn,
> u32 op) {
>  	const struct fs_platform_info *fpi = fep->fpi;
> -
>  	cpm2_map_t *immap = fs_enet_immap;
>  	cpm_cpm2_t *cpmp = &immap->im_cpm;
>  	u32 v;
> diff --git a/drivers/net/fs_enet/mii-fec.c
> b/drivers/net/fs_enet/mii-fec.c index 0a563a8..53db696 100644
> --- a/drivers/net/fs_enet/mii-fec.c
> +++ b/drivers/net/fs_enet/mii-fec.c
> @@ -113,7 +113,6 @@ static int fs_enet_fec_mii_read(struct mii_bus
> *bus , int phy_id, int location) }
>  
>  	return ret;
> -
>  }
>  
>  static int fs_enet_fec_mii_write(struct mii_bus *bus, int phy_id,
> int location, u16 val)


-- 
Sincerely, Vitaly

^ permalink raw reply

* Re: [PATCH 6/7 v2] fs_enet: Be an of_platform device when CONFIG_PPC_CPM_NEW_BINDING is set.
From: Vitaly Bordug @ 2007-08-22 21:00 UTC (permalink / raw)
  To: Scott Wood; +Cc: netdev, jgarzik, linuxppc-dev
In-Reply-To: <46CB172D.9010807@freescale.com>

On Tue, 21 Aug 2007 11:47:41 -0500
Scott Wood <scottwood@freescale.com> wrote:

> Vitaly Bordug wrote:
> > On Fri, 17 Aug 2007 13:17:18 -0500
> > Scott Wood wrote:
> > 
> > 
> >>The existing OF glue code was crufty and broken.  Rather than fix
> >>it, it will be removed, and the ethernet driver now talks to the
> >>device tree directly.
> >>
> > 
> > A bit short description, I'd rather expect some specific
> > improvements list, that are now up and running using device tree.
> > Or if it is just move to new infrastucture, let's state that, too.
> 
> Some of specific binding changes (there are too many to exhaustively 
> list) are enumerated in the "new CPM binding" patch, which I'll send 
> after Kumar's include/asm-ppc patch goes in (since it modifies one of 
> those files).
> 
ok
> >>+#ifdef CONFIG_PPC_CPM_NEW_BINDING
> >>+static int __devinit find_phy(struct device_node *np,
> >>+                              struct fs_platform_info *fpi)
> >>+{
> >>+	struct device_node *phynode, *mdionode;
> >>+	struct resource res;
> >>+	int ret = 0, len;
> >>+
> >>+	const u32 *data = of_get_property(np, "phy-handle", &len);
> >>+	if (!data || len != 4)
> >>+		return -EINVAL;
> >>+
> >>+	phynode = of_find_node_by_phandle(*data);
> >>+	if (!phynode)
> >>+		return -EINVAL;
> >>+
> >>+	mdionode = of_get_parent(phynode);
> >>+	if (!phynode)
> >>+		goto out_put_phy;
> >>+
> >>+	ret = of_address_to_resource(mdionode, 0, &res);
> >>+	if (ret)
> >>+		goto out_put_mdio;
> >>+
> >>+	data = of_get_property(phynode, "reg", &len);
> >>+	if (!data || len != 4)
> >>+		goto out_put_mdio;
> >>+
> >>+	snprintf(fpi->bus_id, 16, PHY_ID_FMT, res.start, *data);
> >>+
> >>+out_put_mdio:
> >>+	of_node_put(mdionode);
> >>+out_put_phy:
> >>+	of_node_put(phynode);
> >>+	return ret;
> >>+}
> > 
> > And without phy node? 
> 
> It returns -EINVAL. :-)
> 
> >>+#ifdef CONFIG_FS_ENET_HAS_FEC
> >>+#define IS_FEC(match) ((match)->data == &fs_fec_ops)
> >>+#else
> >>+#define IS_FEC(match) 0
> >>+#endif
> >>+
> > 
> > Since we're talking directly with device tree, why bother with
> > CONFIG_ stuff? We are able to figure it out from dts..
> 
> We are figuring it out from the DTS (that's what match->data is).  I 
> just didn't want boards without a FEC to have to build in support for
> it and waste memory.

yes, wrong snippet
what about 

> #ifdef CONFIG_CPM2
> +	r = fs_enet_mdio_bb_init();
> +	if (r != 0)
> +		goto out_mdio_bb;
> +#endif
> +#ifdef CONFIG_8xx
> +	r = fs_enet_mdio_fec_init();
> +	if (r != 0)
> +		goto out_mdio_fec;
> +#endif

We had to pray and hope that 8xx would only have fec, and cpm2 has some bitbanged stuff. now we can inquire dts and know for sure, at least it seems so.



> 
> >>+	fpi->rx_ring = 32;
> >>+	fpi->tx_ring = 32;
> >>+	fpi->rx_copybreak = 240;
> >>+	fpi->use_napi = 0;
> >>+	fpi->napi_weight = 17;
> >>+
> > 
> > 
> > move params over to  dts?
> 
> No.  These aren't attributes of the hardware, they're choices the
> driver makes about how much memory to use and how to interact with
> the rest of the kernel.
> 
> >>+	ret = find_phy(ofdev->node, fpi);
> >>+	if (ret)
> >>+		goto out_free_fpi;
> >>+
> > 
> > so we're hosed without phy node.
> 
> How is that different from the old code, where you're hosed without 
> fep->fpi->bus_id?
> 

I wasn't defending old code, and consider "old code is POS, new one is just great" game meaningless.
I am just stating the problem, that we'll have to address later. On 8xx even reference boards may be 
without phy at all.

> >>+static struct of_device_id fs_enet_match[] = {
> >>+#ifdef CONFIG_FS_ENET_HAS_SCC
> > 
> > 
> > same nagging. Are we able to get rid of Kconfig arcane defining
> > which SoC currently plays the game for fs_enet?
> 
> No, it's still needed for mpc885ads to determine pin setup and 
> conflicting device tree node removal (though that could go away if
> the device tree is changed to reflect the jumper settings).
> 
> It's also useful for excluding unwanted code.  I don't like using 
> 8xx/CPM2 as the decision point for that -- why should I build in 
> mac-scc.c if I have no intention of using an SCC ethernet (either 
> because my board doesn't have one, or because it's slow and conflicts 
> with other devices)?

ok, agreed, size is most serious judge here. we'll definitely have to revisit pin problem later too
(because custom designs sometimes switch contradictory devices on-the-fly, disable soc parts for alternative function, etc.) QE-like pin encoding may be an option for this or not- I'm inclined to look at
most resource-safe approach.

> 
> -Scott

-- 
Sincerely, Vitaly

^ permalink raw reply

* Re: ppc Floating Point support status
From: Kumar Gala @ 2007-08-22 20:52 UTC (permalink / raw)
  To: Ben Buchli; +Cc: linuxppc-dev
In-Reply-To: <200708221337.21729.bbuchli@xes-inc.com>


On Aug 22, 2007, at 1:37 PM, Ben Buchli wrote:

> Hello everybody,
> I was wondering what the status was on supporting floating-point  
> instructions
> for the mpc8548.  I found the suggested patch:
> http://patchwork.ozlabs.org/linuxppc/patch? 
> order=patch&filter=none&id=9455,
> but I'm not sure for what kernel version this is and when it will be
> officially supported.
>
> Thanks a lot for your help!
>
> Please CC to me (bbuchli at xes-inc.com) as I haven't subscribed to  
> this list.

I thing these are still up in the air based on having some sense of  
testing related to them.

- k

^ permalink raw reply

* Re: [POWERPC] add clrsetbits macros
From: Kumar Gala @ 2007-08-22 20:49 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev
In-Reply-To: <11878048362390-git-send-email-timur@freescale.com>


On Aug 22, 2007, at 12:47 PM, Timur Tabi wrote:

> This patch adds the clrsetbits() macro, which is used to set and clear
> multiple bits in a single read-modify-write operation.  Specify the  
> bits to
> clear in the 'clear' parameter and the bits to set in the 'set'  
> parameter.
> These macros can also be used to set a multiple-bit bit pattern  
> using a mask,
> by specifying the mask in the 'clear' parameter and the new bit  
> pattern in the
> 'set' parameter.  The 'type' parameter is used to specify the size  
> and endian.
>
> Signed-off-by: Timur Tabi <timur@freescale.com>
> ---
>
> Replace multiple macros with a single one that uses macro  
> concatenation to
> handle the endian/size.
>
>  include/asm-powerpc/io.h |   13 +++++++++++++
>  1 files changed, 13 insertions(+), 0 deletions(-)
>
> diff --git a/include/asm-powerpc/io.h b/include/asm-powerpc/io.h
> index 4c0b550..030ed4e 100644
> --- a/include/asm-powerpc/io.h
> +++ b/include/asm-powerpc/io.h
> @@ -737,6 +737,19 @@ static inline void * bus_to_virt(unsigned long  
> address)
>  #define setbits8(_addr, _v) out_8((_addr), in_8(_addr) |  (_v))
>  #define clrbits8(_addr, _v) out_8((_addr), in_8(_addr) & ~(_v))
>
> +/* Clear and set bits in one shot.  This macrocan be used to clear  
> and set
> + * multiple bits in a register using a single read-modify-write.   
> It can
> + * also be used to set a multiple-bit bit pattern using a mask, by
> + * specifying the mask in the 'clear' parameter and the new bit  
> pattern in
> + * the 'set' parameter.
> + *
> + * For 'type', specify 8, le16, be16, le32, be32, le64, or be64.   
> The latter
> + * two only work on 64-bit PowerPC (__powerpc64__ is defined).
> + */
> +
> +#define clrsetbits(type, addr, clear, set) \
> +	out_##type((addr), (in_##type(addr) & ~(clear)) | (set))
> +

I think we should define the *_le{32,64}, *_be{32,64}, and _8  
versions using this.

- k

^ permalink raw reply

* Need the bdi cfg file for taihu PPCEP405 Eval board
From: ravi.rao @ 2007-08-22 20:39 UTC (permalink / raw)
  To: wolfgang.reissnegger; +Cc: linuxppc-embedded
In-Reply-To: <20070822153626.C415873007B@mail29-sin.bigfish.com>

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

Hi All,
   Can one of you please email me the bdi config file for Taihu Eval 
board. I need to attach bdi200 and do some debugging on that..
Thanks,
Ravishankar Govindarao
RFL Electronics Inc.
E-mail : Ravi.Rao@rflelect.com
Voice: 973.334.3100 Ext. 233
Fax: 973.334.3863
 
CONFIDENTIALITY NOTE
This e-mail, including any attachments, may contain confidential and/or 
legally privileged information.  The Information is intended only for the 
use of the individual or entity named on this e-mail .  If you are not the 
intended recipient, you are hereby notified that any disclosure, copying, 
distribution, or the taking of any action in reliance on the contents of 
this transmitted Information is strictly prohibited.  Further, if you are 
not the intended recipient, please notify us by return e-mail and delete 
the Information promptly.
 
 
 

[-- Attachment #2: Type: text/html, Size: 1593 bytes --]

^ 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