LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH -v2 0/7] powerpc: use asm-generic/dma-mapping-common.h
From: Benjamin Herrenschmidt @ 2009-08-27  3:12 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: FUJITA Tomonori, linuxppc-dev, linux-kernel, galak
In-Reply-To: <20090813085505.GA10671@elte.hu>

On Thu, 2009-08-13 at 10:55 +0200, Ingo Molnar wrote:

> Ok! We could also stage it a bit (one or two weeks) in a separate 
> branch and allow a rebase, should you find any bugs during testing?

Allright so after various delays and sidetracking on my side, the
patches have been in my -test branch for long enough, I'm happy for them
to go in either way now.

I'm going to take them out of powerpc test for now and not put them in
my -next right away until I'm sure I have the right source to pull, at
which point I can just put from tip iommu.

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH] powerpc: Change archdata dma_data type to dma_addr_t
From: Michael Ellerman @ 2009-08-27  0:24 UTC (permalink / raw)
  To: Becky Bruce; +Cc: Christoph Hellwig, linuxppc-dev
In-Reply-To: <E3F20841-FB04-44EF-8DAA-91F51873C0A6@kernel.crashing.org>

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

On Wed, 2009-08-26 at 15:20 -0500, Becky Bruce wrote:
> On Aug 26, 2009, at 9:08 AM, Michael Ellerman wrote:
> 
> > On Wed, 2009-08-26 at 22:29 +1000, Benjamin Herrenschmidt wrote:
> >> On Mon, 2009-08-24 at 21:48 +0200, Christoph Hellwig wrote:
> >>> On Mon, Aug 24, 2009 at 11:17:14AM -0500, Becky Bruce wrote:
> >>>> Previously, this was specified as a void *, but that's not
> >>>> large enough on 32-bit systems with 36-bit physical
> >>>> addressing support.  Change the type to dma_addr_t so it
> >>>> will scale based on the size of a dma address.
> >>>
> >>> This looks extreml ugly to me.  It seems like the typical use is to
> >>> store a pointer to a structure.  So what about making the direct
> >>> dma case follow that general scheme instead?
> >>>
> >>> E.g. declare a
> >>>
> >>> struct direct_dma_data {
> >>> 	dma_addr_t	direct_dma_offset;
> >>> };
> >>>
> >>> and have one normal instace of it, and one per weird cell device.
> >>
> >> Right, but we want to avoid a structure for the classic case of 32- 
> >> bit
> >> systems with no iommu...
> >>
> >> I wouldn't mind doing a union here.
> >
> > That might be best, the patch as it stands is a horrible mess of  
> > casts.
> 
> Let's be fair - the code before was a horrible mess of casts, I've  
> just moved them :)

Yeah true. Though I think we end up with more casts because there were
more call sites using it as a pointer originally. But yeah it's not
pretty either way.

> > Stashing a dma_addr_t into a void * is sort of gross, but storing a
> > pointer to some struct (a void *) in a dma_addr_t is _really_ gross :)
> 
> Both are revolting (and storing a dma_addr_t into a void * is really  
> gross when the void * is smaller than the dma_addr_t!!).  A union  
> might not be a bad idea, though.  I'll look at doing that instead.

Cool. That is how we're using it, sometimes it points to something
sometimes it's a dma_addr_t, so I think a union will work.

cheers

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

^ permalink raw reply

* Re: PPC driver - generic bus width
From: Stephen Rothwell @ 2009-08-27  0:04 UTC (permalink / raw)
  To: michael; +Cc: linuxppc-dev, Alemao, linux-kernel
In-Reply-To: <1251296565.7492.7.camel@concordia>

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

On Thu, 27 Aug 2009 00:22:45 +1000 Michael Ellerman <michael@ellerman.id.au> wrote:
>
> > struct device_info_t {
> > 	void (*read)();
> > 	void (*write)();

These are not prototypes because there is no parameter list (use "void" for empty).

> > };
> > 
> > static int __devinit device_probe()

This isn't a prototype either.

> > cc1: warnings being treated as errors
> > drivers/dev_test.c:37: warning: function declaration isn't a prototype
> > drivers/dev_test.c:38: warning: function declaration isn't a prototype
> > drivers/dev_test.c: In function 'device_probe':
> > drivers/dev_test.c:113: warning: assignment from incompatible pointer
> > type
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

^ permalink raw reply

* Re: [PATCH] powerpc: Change archdata dma_data type to dma_addr_t
From: Becky Bruce @ 2009-08-26 20:20 UTC (permalink / raw)
  To: michael; +Cc: Christoph Hellwig, linuxppc-dev
In-Reply-To: <1251295725.7492.5.camel@concordia>


On Aug 26, 2009, at 9:08 AM, Michael Ellerman wrote:

> On Wed, 2009-08-26 at 22:29 +1000, Benjamin Herrenschmidt wrote:
>> On Mon, 2009-08-24 at 21:48 +0200, Christoph Hellwig wrote:
>>> On Mon, Aug 24, 2009 at 11:17:14AM -0500, Becky Bruce wrote:
>>>> Previously, this was specified as a void *, but that's not
>>>> large enough on 32-bit systems with 36-bit physical
>>>> addressing support.  Change the type to dma_addr_t so it
>>>> will scale based on the size of a dma address.
>>>
>>> This looks extreml ugly to me.  It seems like the typical use is to
>>> store a pointer to a structure.  So what about making the direct
>>> dma case follow that general scheme instead?
>>>
>>> E.g. declare a
>>>
>>> struct direct_dma_data {
>>> 	dma_addr_t	direct_dma_offset;
>>> };
>>>
>>> and have one normal instace of it, and one per weird cell device.
>>
>> Right, but we want to avoid a structure for the classic case of 32- 
>> bit
>> systems with no iommu...
>>
>> I wouldn't mind doing a union here.
>
> That might be best, the patch as it stands is a horrible mess of  
> casts.

Let's be fair - the code before was a horrible mess of casts, I've  
just moved them :)

>
> Stashing a dma_addr_t into a void * is sort of gross, but storing a
> pointer to some struct (a void *) in a dma_addr_t is _really_ gross :)

Both are revolting (and storing a dma_addr_t into a void * is really  
gross when the void * is smaller than the dma_addr_t!!).  A union  
might not be a bad idea, though.  I'll look at doing that instead.

-Becky

^ permalink raw reply

* Re: Can't write value into memory ?(E500 V2)
From: Scott Wood @ 2009-08-26 19:20 UTC (permalink / raw)
  To: wilbur.chan; +Cc: linuxppc-dev
In-Reply-To: <e997b7420908260739u49fb4b36j9eb148782829547b@mail.gmail.com>

On Wed, Aug 26, 2009 at 10:39:24PM +0800, wilbur.chan wrote:
> In  an assemblely code ,   I invalided all the TLB entries except for
> the entry  we are executed in.
> 
> After that , I setuped a 1:1 TLB entry mapping of 1GB .

What is it you're trying to do?

> At last , I wrote value 30 into the physical address 0x0400,0000 (also
> the virtual address because of my 1:1 mapping).
> 
> However, it seemed failed to store the value '30' at address
> 0x400,0000. The following is my code:
> 
> //code start
> 
> // setup a 1:1 mapping of 1GB
> ...

"..." is not code. :-)

> //store '30' into address 0x0400,0000
> 
> li  r23, 30
> lis r22, 0x400
> ori r22,r22,0x0
> stb  r23,0(r22)
> 
> 
> //check if we successfully store value at 0x400,0000
> 
> lis r22, 0x400
> ori r22,r22,0x0
> lwz r23,0(r22)
> cmpw r23, 30

The values should not be equal, since you wrote a byte and read back a
word.

Furthermore, you are storing the constant 30, but are comparing r23 with
the register r30 (I never liked that aspect of ppc asm syntax -- too
error prone).  If you want to compare with the constant 30, use "cmpwi".

> beq  print_equal
> 
> 1: b 1b
> 
> print_equal:
>  ...
> //code end
> 
> I found that, print_equal was not called ,the whole code seemed  to
> enter an  infinite loop.

That's because you have an infinite loop in your code, right after "beq
print_equal".

-Scott

^ permalink raw reply

* Re: [PATCH V2 3/3] powerpc/pci: Merge ppc32 and ppc64 versions of phb_scan()
From: Grant Likely @ 2009-08-26 17:52 UTC (permalink / raw)
  To: Kumar Gala; +Cc: linuxppc-dev, sfr
In-Reply-To: <E575F71A-166A-4933-8612-1F0E88A426C5@kernel.crashing.org>

On Wed, Aug 26, 2009 at 7:29 AM, Kumar Gala<galak@kernel.crashing.org> wrot=
e:
>
> On Aug 26, 2009, at 1:07 AM, Grant Likely wrote:
>
>> +/**
>> + * pci_scan_phb - Given a pci_controller, setup and scan the PCI bus
>> + * @hose: Pointer to the PCI host controller instance structure
>> + * @data: value to use for sysdata pointer. =A0ppc32 and ppc64 differ h=
ere
>> + *
>> + * Note: the 'data' pointer is a temporary measure. =A0As 32 and 64 bit
>> + * pci code gets merged, this parameter should become unnecessary becau=
se
>> + * both will use the same value.
>> + */
>> +void __devinit pcibios_scan_phb(struct pci_controller *hose, void *data=
)
>> +{
>> +
>
> Just a nit, but why not data -> sysdata since that's what we using it as.

okay.

g.

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

^ permalink raw reply

* Re: Extending virtio_console to support multiple ports
From: Amit Shah @ 2009-08-26 15:45 UTC (permalink / raw)
  To: qemu-devel, kvm, virtualization
  Cc: borntraeger, linux-kernel, miltonm, linuxppc-dev, brueckner, alan
In-Reply-To: <20090826112718.GA11117@amit-x200.redhat.com>

[cc'ing some people who have made some commits in hvc_console.c]

On (Wed) Aug 26 2009 [16:57:18], Amit Shah wrote:
> On (Tue) Aug 25 2009 [11:47:20], Amit Shah wrote:
> > 
> > Hello all,
> > 
> > Here is a new iteration of the patch series that implements a
> > transport for guest and host communications.
> > 
> > The code has been updated to reuse the virtio-console device instead
> > of creating a new virtio-serial device.
> 
> And the problem now is that hvc calls the put_chars function with
> spinlocks held and we now allocate pages in send_buf(), called from
> put_chars.
> 
> A few solutions:

[snip]

> - Convert hvc's usage of spinlocks to mutexes. I've no idea how this
>   will play out; I'm no expert here. But I did try doing this and so far
>   it all looks OK. No lockups, lockdep warnings, nothing. I have full
>   debugging enabled. But this doesn't mean it's right.

So just to test this further I added the capability to have more than
one hvc console spawn from virtio_console, created two consoles and did
a 'cat' of a file in each of the virtio-consoles. It's been running for
half an hour now without any badness. No spew in debug logs too.

I also checked the code in hvc_console.c that takes the spin_locks.
Nothing there that runs from (or needs to run from) interrupt context.
So the change to mutexes does seem reasonable. Also, the spinlock code
was added really long back -- git blame shows Linus' first git commit
introduced them in the git history, so it's pure legacy baggage.

Also found a bug: hvc_resize() wants to be called with a lock held
(hp->lock) but virtio_console just calls it directly.

Anyway I'm wondering whether all those locks are needed.

		Amit


diff --git a/drivers/char/hvc_console.c b/drivers/char/hvc_console.c
index d97779e..51078a3 100644
--- a/drivers/char/hvc_console.c
+++ b/drivers/char/hvc_console.c
@@ -35,7 +35,7 @@
 #include <linux/tty.h>
 #include <linux/tty_flip.h>
 #include <linux/sched.h>
-#include <linux/spinlock.h>
+#include <linux/mutex.h>
 #include <linux/delay.h>
 #include <linux/freezer.h>
 
@@ -81,7 +81,7 @@ static LIST_HEAD(hvc_structs);
  * Protect the list of hvc_struct instances from inserts and removals during
  * list traversal.
  */
-static DEFINE_SPINLOCK(hvc_structs_lock);
+static DEFINE_MUTEX(hvc_structs_lock);
 
 /*
  * This value is used to assign a tty->index value to a hvc_struct based
@@ -98,23 +98,22 @@ static int last_hvc = -1;
 static struct hvc_struct *hvc_get_by_index(int index)
 {
 	struct hvc_struct *hp;
-	unsigned long flags;
 
-	spin_lock(&hvc_structs_lock);
+	mutex_lock(&hvc_structs_lock);
 
 	list_for_each_entry(hp, &hvc_structs, next) {
-		spin_lock_irqsave(&hp->lock, flags);
+		mutex_lock(&hp->lock);
 		if (hp->index == index) {
 			kref_get(&hp->kref);
-			spin_unlock_irqrestore(&hp->lock, flags);
-			spin_unlock(&hvc_structs_lock);
+			mutex_unlock(&hp->lock);
+			mutex_unlock(&hvc_structs_lock);
 			return hp;
 		}
-		spin_unlock_irqrestore(&hp->lock, flags);
+		mutex_unlock(&hp->lock);
 	}
 	hp = NULL;
 
-	spin_unlock(&hvc_structs_lock);
+	mutex_unlock(&hvc_structs_lock);
 	return hp;
 }
 
@@ -228,15 +227,14 @@ console_initcall(hvc_console_init);
 static void destroy_hvc_struct(struct kref *kref)
 {
 	struct hvc_struct *hp = container_of(kref, struct hvc_struct, kref);
-	unsigned long flags;
 
-	spin_lock(&hvc_structs_lock);
+	mutex_lock(&hvc_structs_lock);
 
-	spin_lock_irqsave(&hp->lock, flags);
+	mutex_lock(&hp->lock);
 	list_del(&(hp->next));
-	spin_unlock_irqrestore(&hp->lock, flags);
+	mutex_unlock(&hp->lock);
 
-	spin_unlock(&hvc_structs_lock);
+	mutex_unlock(&hvc_structs_lock);
 
 	kfree(hp);
 }
@@ -302,17 +300,16 @@ static void hvc_unthrottle(struct tty_struct *tty)
 static int hvc_open(struct tty_struct *tty, struct file * filp)
 {
 	struct hvc_struct *hp;
-	unsigned long flags;
 	int rc = 0;
 
 	/* Auto increments kref reference if found. */
 	if (!(hp = hvc_get_by_index(tty->index)))
 		return -ENODEV;
 
-	spin_lock_irqsave(&hp->lock, flags);
+	mutex_lock(&hp->lock);
 	/* Check and then increment for fast path open. */
 	if (hp->count++ > 0) {
-		spin_unlock_irqrestore(&hp->lock, flags);
+		mutex_unlock(&hp->lock);
 		hvc_kick();
 		return 0;
 	} /* else count == 0 */
@@ -321,7 +318,7 @@ static int hvc_open(struct tty_struct *tty, struct file * filp)
 
 	hp->tty = tty;
 
-	spin_unlock_irqrestore(&hp->lock, flags);
+	mutex_unlock(&hp->lock);
 
 	if (hp->ops->notifier_add)
 		rc = hp->ops->notifier_add(hp, hp->data);
@@ -333,9 +330,9 @@ static int hvc_open(struct tty_struct *tty, struct file * filp)
 	 * tty fields and return the kref reference.
 	 */
 	if (rc) {
-		spin_lock_irqsave(&hp->lock, flags);
+		mutex_lock(&hp->lock);
 		hp->tty = NULL;
-		spin_unlock_irqrestore(&hp->lock, flags);
+		mutex_unlock(&hp->lock);
 		tty->driver_data = NULL;
 		kref_put(&hp->kref, destroy_hvc_struct);
 		printk(KERN_ERR "hvc_open: request_irq failed with rc %d.\n", rc);
@@ -349,7 +346,6 @@ static int hvc_open(struct tty_struct *tty, struct file * filp)
 static void hvc_close(struct tty_struct *tty, struct file * filp)
 {
 	struct hvc_struct *hp;
-	unsigned long flags;
 
 	if (tty_hung_up_p(filp))
 		return;
@@ -363,12 +359,12 @@ static void hvc_close(struct tty_struct *tty, struct file * filp)
 		return;
 
 	hp = tty->driver_data;
-	spin_lock_irqsave(&hp->lock, flags);
+	mutex_lock(&hp->lock);
 
 	if (--hp->count == 0) {
 		/* We are done with the tty pointer now. */
 		hp->tty = NULL;
-		spin_unlock_irqrestore(&hp->lock, flags);
+		mutex_unlock(&hp->lock);
 
 		if (hp->ops->notifier_del)
 			hp->ops->notifier_del(hp, hp->data);
@@ -386,7 +382,7 @@ static void hvc_close(struct tty_struct *tty, struct file * filp)
 		if (hp->count < 0)
 			printk(KERN_ERR "hvc_close %X: oops, count is %d\n",
 				hp->vtermno, hp->count);
-		spin_unlock_irqrestore(&hp->lock, flags);
+		mutex_unlock(&hp->lock);
 	}
 
 	kref_put(&hp->kref, destroy_hvc_struct);
@@ -395,7 +391,6 @@ static void hvc_close(struct tty_struct *tty, struct file * filp)
 static void hvc_hangup(struct tty_struct *tty)
 {
 	struct hvc_struct *hp = tty->driver_data;
-	unsigned long flags;
 	int temp_open_count;
 
 	if (!hp)
@@ -404,7 +399,7 @@ static void hvc_hangup(struct tty_struct *tty)
 	/* cancel pending tty resize work */
 	cancel_work_sync(&hp->tty_resize);
 
-	spin_lock_irqsave(&hp->lock, flags);
+	mutex_lock(&hp->lock);
 
 	/*
 	 * The N_TTY line discipline has problems such that in a close vs
@@ -412,7 +407,7 @@ static void hvc_hangup(struct tty_struct *tty)
 	 * that from happening for now.
 	 */
 	if (hp->count <= 0) {
-		spin_unlock_irqrestore(&hp->lock, flags);
+		mutex_unlock(&hp->lock);
 		return;
 	}
 
@@ -421,7 +416,7 @@ static void hvc_hangup(struct tty_struct *tty)
 	hp->n_outbuf = 0;
 	hp->tty = NULL;
 
-	spin_unlock_irqrestore(&hp->lock, flags);
+	mutex_unlock(&hp->lock);
 
 	if (hp->ops->notifier_hangup)
 			hp->ops->notifier_hangup(hp, hp->data);
@@ -462,7 +457,6 @@ static int hvc_push(struct hvc_struct *hp)
 static int hvc_write(struct tty_struct *tty, const unsigned char *buf, int count)
 {
 	struct hvc_struct *hp = tty->driver_data;
-	unsigned long flags;
 	int rsize, written = 0;
 
 	/* This write was probably executed during a tty close. */
@@ -472,7 +466,7 @@ static int hvc_write(struct tty_struct *tty, const unsigned char *buf, int count
 	if (hp->count <= 0)
 		return -EIO;
 
-	spin_lock_irqsave(&hp->lock, flags);
+	mutex_lock(&hp->lock);
 
 	/* Push pending writes */
 	if (hp->n_outbuf > 0)
@@ -488,7 +482,7 @@ static int hvc_write(struct tty_struct *tty, const unsigned char *buf, int count
 		written += rsize;
 		hvc_push(hp);
 	}
-	spin_unlock_irqrestore(&hp->lock, flags);
+	mutex_unlock(&hp->lock);
 
 	/*
 	 * Racy, but harmless, kick thread if there is still pending data.
@@ -511,7 +505,6 @@ static int hvc_write(struct tty_struct *tty, const unsigned char *buf, int count
 static void hvc_set_winsz(struct work_struct *work)
 {
 	struct hvc_struct *hp;
-	unsigned long hvc_flags;
 	struct tty_struct *tty;
 	struct winsize ws;
 
@@ -519,14 +512,14 @@ static void hvc_set_winsz(struct work_struct *work)
 	if (!hp)
 		return;
 
-	spin_lock_irqsave(&hp->lock, hvc_flags);
+	mutex_lock(&hp->lock);
 	if (!hp->tty) {
-		spin_unlock_irqrestore(&hp->lock, hvc_flags);
+		mutex_unlock(&hp->lock);
 		return;
 	}
 	ws  = hp->ws;
 	tty = tty_kref_get(hp->tty);
-	spin_unlock_irqrestore(&hp->lock, hvc_flags);
+	mutex_unlock(&hp->lock);
 
 	tty_do_resize(tty, &ws);
 	tty_kref_put(tty);
@@ -576,11 +569,10 @@ int hvc_poll(struct hvc_struct *hp)
 	struct tty_struct *tty;
 	int i, n, poll_mask = 0;
 	char buf[N_INBUF] __ALIGNED__;
-	unsigned long flags;
 	int read_total = 0;
 	int written_total = 0;
 
-	spin_lock_irqsave(&hp->lock, flags);
+	mutex_lock(&hp->lock);
 
 	/* Push pending writes */
 	if (hp->n_outbuf > 0)
@@ -622,9 +614,9 @@ int hvc_poll(struct hvc_struct *hp)
 		if (n <= 0) {
 			/* Hangup the tty when disconnected from host */
 			if (n == -EPIPE) {
-				spin_unlock_irqrestore(&hp->lock, flags);
+				mutex_unlock(&hp->lock);
 				tty_hangup(tty);
-				spin_lock_irqsave(&hp->lock, flags);
+				mutex_lock(&hp->lock);
 			} else if ( n == -EAGAIN ) {
 				/*
 				 * Some back-ends can only ensure a certain min
@@ -665,7 +657,7 @@ int hvc_poll(struct hvc_struct *hp)
 		tty_wakeup(tty);
 	}
  bail:
-	spin_unlock_irqrestore(&hp->lock, flags);
+	mutex_unlock(&hp->lock);
 
 	if (read_total) {
 		/* Activity is occurring, so reset the polling backoff value to
@@ -714,11 +706,11 @@ static int khvcd(void *unused)
 		try_to_freeze();
 		wmb();
 		if (!cpus_are_in_xmon()) {
-			spin_lock(&hvc_structs_lock);
+			mutex_lock(&hvc_structs_lock);
 			list_for_each_entry(hp, &hvc_structs, next) {
 				poll_mask |= hvc_poll(hp);
 			}
-			spin_unlock(&hvc_structs_lock);
+			mutex_unlock(&hvc_structs_lock);
 		} else
 			poll_mask |= HVC_POLL_READ;
 		if (hvc_kicked)
@@ -777,8 +769,8 @@ struct hvc_struct __devinit *hvc_alloc(uint32_t vtermno, int data,
 	kref_init(&hp->kref);
 
 	INIT_WORK(&hp->tty_resize, hvc_set_winsz);
-	spin_lock_init(&hp->lock);
-	spin_lock(&hvc_structs_lock);
+	mutex_init(&hp->lock);
+	mutex_lock(&hvc_structs_lock);
 
 	/*
 	 * find index to use:
@@ -796,7 +788,7 @@ struct hvc_struct __devinit *hvc_alloc(uint32_t vtermno, int data,
 	hp->index = i;
 
 	list_add_tail(&(hp->next), &hvc_structs);
-	spin_unlock(&hvc_structs_lock);
+	mutex_unlock(&hvc_structs_lock);
 
 	return hp;
 }
@@ -804,10 +796,9 @@ EXPORT_SYMBOL_GPL(hvc_alloc);
 
 int hvc_remove(struct hvc_struct *hp)
 {
-	unsigned long flags;
 	struct tty_struct *tty;
 
-	spin_lock_irqsave(&hp->lock, flags);
+	mutex_lock(&hp->lock);
 	tty = hp->tty;
 
 	if (hp->index < MAX_NR_HVC_CONSOLES)
@@ -815,7 +806,7 @@ int hvc_remove(struct hvc_struct *hp)
 
 	/* Don't whack hp->irq because tty_hangup() will need to free the irq. */
 
-	spin_unlock_irqrestore(&hp->lock, flags);
+	mutex_unlock(&hp->lock);
 
 	/*
 	 * We 'put' the instance that was grabbed when the kref instance
diff --git a/drivers/char/hvc_console.h b/drivers/char/hvc_console.h
index 3c85d78..3c086f8 100644
--- a/drivers/char/hvc_console.h
+++ b/drivers/char/hvc_console.h
@@ -45,7 +45,7 @@
 #define HVC_ALLOC_TTY_ADAPTERS	8
 
 struct hvc_struct {
-	spinlock_t lock;
+	struct mutex lock;
 	int index;
 	struct tty_struct *tty;
 	int count;

^ permalink raw reply related

* Re: [ewg] [PATCH] IB/ehca: Construct MAD redirect replies from request MAD
From: Hal Rosenstock @ 2009-08-26 15:15 UTC (permalink / raw)
  To: Joachim Fenkes
  Cc: Hal Rosenstock, LKML, OF-EWG, Jason Gunthorpe, LinuxPPC-Dev,
	Christoph Raisch, OF-General, Stefan Roscher
In-Reply-To: <200908261337.56128.fenkes@de.ibm.com>

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

On 8/26/09, Joachim Fenkes <fenkes@de.ibm.com> wrote:
>
> The old code used a lot of hardcoded values, which might not be valid in
> all
> environments (especially routed fabrics or partitioned subnets). Copy as
> much information as possible from the incoming request to prevent that.
>
> Signed-off-by: Joachim Fenkes <fenkes@de.ibm.com>
> ---
>
> Hal, Jason -- here's the change I promised. Looks okay to you?
> Roland -- if Hal and Jason don't object, please queue this up for the next
> kernel. Thanks!


Thanks for doing this. It looks sane to me. The only issue I recall that
appears to be remaining is a better setting of ClassPortInfo:RespTimeValue
rather than hardcoding. Perhaps using the value from PortInfo is the way to
go (ideally it would be that value from the port to which the the requester
is being redirected to but that might not be so easy to get from this port
(I guess that could be SA Get PortInfoRecord for that port but that is a
larger change and it likely to be same as local port issuing the redirect
response).

-- Hal

Regards,
> Joachim
>
> drivers/infiniband/hw/ehca/ehca_sqp.c |   47
> ++++++++++++++++++++++++++++----
> 1 files changed, 41 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/infiniband/hw/ehca/ehca_sqp.c
> b/drivers/infiniband/hw/ehca/ehca_sqp.c
> index c568b28..8c1213f 100644
> --- a/drivers/infiniband/hw/ehca/ehca_sqp.c
> +++ b/drivers/infiniband/hw/ehca/ehca_sqp.c
> @@ -125,14 +125,30 @@ struct ib_perf {
>        u8 data[192];
> } __attribute__ ((packed));
>
> +/* TC/SL/FL packed into 32 bits, as in ClassPortInfo */
> +struct tcslfl {
> +       u32 tc:8;
> +       u32 sl:4;
> +       u32 fl:20;
> +} __attribute__ ((packed));
> +
> +/* IP Version/TC/FL packed into 32 bits, as in GRH */
> +struct vertcfl {
> +       u32 ver:4;
> +       u32 tc:8;
> +       u32 fl:20;
> +} __attribute__ ((packed));
>
> static int ehca_process_perf(struct ib_device *ibdev, u8 port_num,
> +                            struct ib_wc *in_wc, struct ib_grh *in_grh,
>                             struct ib_mad *in_mad, struct ib_mad *out_mad)
> {
>        struct ib_perf *in_perf = (struct ib_perf *)in_mad;
>        struct ib_perf *out_perf = (struct ib_perf *)out_mad;
>        struct ib_class_port_info *poi =
>                (struct ib_class_port_info *)out_perf->data;
> +       struct tcslfl *tcslfl =
> +               (struct tcslfl *)&poi->redirect_tcslfl;
>        struct ehca_shca *shca =
>                container_of(ibdev, struct ehca_shca, ib_device);
>        struct ehca_sport *sport = &shca->sport[port_num - 1];
> @@ -158,10 +174,29 @@ static int ehca_process_perf(struct ib_device *ibdev,
> u8 port_num,
>                poi->base_version = 1;
>                poi->class_version = 1;
>                poi->resp_time_value = 18;
> -               poi->redirect_lid = sport->saved_attr.lid;
> -               poi->redirect_qp = sport->pma_qp_nr;
> +
> +               /* copy local routing information from WC where applicable
> */
> +               tcslfl->sl         = in_wc->sl;
> +               poi->redirect_lid  =
> +                       sport->saved_attr.lid | in_wc->dlid_path_bits;
> +               poi->redirect_qp   = sport->pma_qp_nr;
>                poi->redirect_qkey = IB_QP1_QKEY;
> -               poi->redirect_pkey = IB_DEFAULT_PKEY_FULL;
> +
> +               ehca_query_pkey(ibdev, port_num, in_wc->pkey_index,
> +                               &poi->redirect_pkey);
> +
> +               /* if request was globally routed, copy route info */
> +               if (in_grh) {
> +                       struct vertcfl *vertcfl =
> +                               (struct vertcfl
> *)&in_grh->version_tclass_flow;
> +                       memcpy(poi->redirect_gid, in_grh->dgid.raw,
> +                              sizeof(poi->redirect_gid));
> +                       tcslfl->tc        = vertcfl->tc;
> +                       tcslfl->fl        = vertcfl->fl;
> +               } else
> +                       /* else only fill in default GID */
> +                       ehca_query_gid(ibdev, port_num, 0,
> +                                      (union ib_gid *)&poi->redirect_gid);
>
>                ehca_dbg(ibdev, "ehca_pma_lid=%x ehca_pma_qp=%x",
>                         sport->saved_attr.lid, sport->pma_qp_nr);
> @@ -183,8 +218,7 @@ perf_reply:
>
> int ehca_process_mad(struct ib_device *ibdev, int mad_flags, u8 port_num,
>                     struct ib_wc *in_wc, struct ib_grh *in_grh,
> -                    struct ib_mad *in_mad,
> -                    struct ib_mad *out_mad)
> +                    struct ib_mad *in_mad, struct ib_mad *out_mad)
> {
>        int ret;
>
> @@ -196,7 +230,8 @@ int ehca_process_mad(struct ib_device *ibdev, int
> mad_flags, u8 port_num,
>                return IB_MAD_RESULT_SUCCESS;
>
>        ehca_dbg(ibdev, "port_num=%x src_qp=%x", port_num, in_wc->src_qp);
> -       ret = ehca_process_perf(ibdev, port_num, in_mad, out_mad);
> +       ret = ehca_process_perf(ibdev, port_num, in_wc, in_grh,
> +                               in_mad, out_mad);
>
>        return ret;
> }
> --
> 1.6.0.4
>
>
> _______________________________________________
> ewg mailing list
> ewg@lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ewg
>

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

^ permalink raw reply

* Can't write value into memory ?(E500 V2)
From: wilbur.chan @ 2009-08-26 14:39 UTC (permalink / raw)
  To: linuxppc-dev

In  an assemblely code ,   I invalided all the TLB entries except for
the entry  we are executed in.

After that , I setuped a 1:1 TLB entry mapping of 1GB .

At last , I wrote value 30 into the physical address 0x0400,0000 (also
the virtual address because of my 1:1 mapping).

However, it seemed failed to store the value '30' at address
0x400,0000. The following is my code:

//code start

// setup a 1:1 mapping of 1GB
...
//store '30' into address 0x0400,0000

li  r23, 30
lis r22, 0x400
ori r22,r22,0x0
stb  r23,0(r22)


//check if we successfully store value at 0x400,0000

lis r22, 0x400
ori r22,r22,0x0
lwz r23,0(r22)
cmpw r23, 30
beq  print_equal

1: b 1b

print_equal:
 ...
//code end

I found that, print_equal was not called ,the whole code seemed  to
enter an  infinite loop.

Can anyone plz tell me why this happended?


Thanks in advance.


regards,

wilbur

^ permalink raw reply

* Re: PPC driver - generic bus width
From: Michael Ellerman @ 2009-08-26 14:22 UTC (permalink / raw)
  To: Alemao; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <d970ff420908260625q2d34bc10p62241218f022b5c2@mail.gmail.com>

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

On Wed, 2009-08-26 at 10:25 -0300, Alemao wrote:
> Hi all,
> 
> Im trying to develop a driver for my device, and its data bus can be 8, 16
> or 32 bits. This information is passed through the device tree source.
> My code is like this:
> 
> struct device_info_t {
> 	void (*read)();
> 	void (*write)();
> };
> 
> static int __devinit device_probe()
> {
> 	prop = of_get_property(ofdev->node, "bus-width", &size);
> 
> 	device->bus_width = *prop;
> 
> 	switch (device->bus_width) {
> 	case 8:
> 		device->read  = in_8;
> 		device->write = out_8;
> 		break;
> 	case 16:
> 		device->read  = in_be16;
> 		device->write = out_be16;
> 		break;
> 	case 32:
> 		device->read  = in_be32;
> 		device->write = out_be32;
> 		break;
> 	default:
> 		break;
> 	}
> }
> 
> Can someone point me some driver that is doing something similar to the
> ideia shown above?
> 
> Or what Im doing wrong in my code? Cause Im getting erros (warnings) like:
> 
> cc1: warnings being treated as errors
> drivers/dev_test.c:37: warning: function declaration isn't a prototype
> drivers/dev_test.c:38: warning: function declaration isn't a prototype
> drivers/dev_test.c: In function 'device_probe':
> drivers/dev_test.c:113: warning: assignment from incompatible pointer
> type

I'm not sure about the first two, you don't seem to have pasted all the
source, and you haven't given the line numbers.

The 3rd is probably because the prototype for in_8 etc. doesn't match
your structure.

cheers

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

^ permalink raw reply

* Re: [PATCH] powerpc: Change archdata dma_data type to dma_addr_t
From: Michael Ellerman @ 2009-08-26 14:08 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Christoph Hellwig
In-Reply-To: <1251289790.1379.71.camel@pasglop>

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

On Wed, 2009-08-26 at 22:29 +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2009-08-24 at 21:48 +0200, Christoph Hellwig wrote:
> > On Mon, Aug 24, 2009 at 11:17:14AM -0500, Becky Bruce wrote:
> > > Previously, this was specified as a void *, but that's not
> > > large enough on 32-bit systems with 36-bit physical
> > > addressing support.  Change the type to dma_addr_t so it
> > > will scale based on the size of a dma address.
> > 
> > This looks extreml ugly to me.  It seems like the typical use is to
> > store a pointer to a structure.  So what about making the direct
> > dma case follow that general scheme instead?
> > 
> > E.g. declare a
> > 
> > struct direct_dma_data {
> > 	dma_addr_t	direct_dma_offset;
> > };
> > 
> > and have one normal instace of it, and one per weird cell device.
> 
> Right, but we want to avoid a structure for the classic case of 32-bit
> systems with no iommu... 
> 
> I wouldn't mind doing a union here. 

That might be best, the patch as it stands is a horrible mess of casts.

Stashing a dma_addr_t into a void * is sort of gross, but storing a
pointer to some struct (a void *) in a dma_addr_t is _really_ gross :)

cheers

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

^ permalink raw reply

* PPC driver - generic bus width
From: Alemao @ 2009-08-26 13:25 UTC (permalink / raw)
  To: linux-kernel; +Cc: linuxppc-dev, linuxppc-embedded

Hi all,

Im trying to develop a driver for my device, and its data bus can be 8, 16
or 32 bits. This information is passed through the device tree source.
My code is like this:

struct device_info_t {
	void (*read)();
	void (*write)();
};

static int __devinit device_probe()
{
	prop = of_get_property(ofdev->node, "bus-width", &size);

	device->bus_width = *prop;

	switch (device->bus_width) {
	case 8:
		device->read  = in_8;
		device->write = out_8;
		break;
	case 16:
		device->read  = in_be16;
		device->write = out_be16;
		break;
	case 32:
		device->read  = in_be32;
		device->write = out_be32;
		break;
	default:
		break;
	}
}

Can someone point me some driver that is doing something similar to the
ideia shown above?

Or what Im doing wrong in my code? Cause Im getting erros (warnings) like:

cc1: warnings being treated as errors
drivers/dev_test.c:37: warning: function declaration isn't a prototype
drivers/dev_test.c:38: warning: function declaration isn't a prototype
drivers/dev_test.c: In function 'device_probe':
drivers/dev_test.c:113: warning: assignment from incompatible pointer
type

PS: Im using a MPC8349, linux kernel 2.6.26

Cheers,

--
Alemao

^ permalink raw reply

* Re: [PATCH V2 3/3] powerpc/pci: Merge ppc32 and ppc64 versions of phb_scan()
From: Kumar Gala @ 2009-08-26 13:29 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, sfr
In-Reply-To: <20090826060716.30936.40578.stgit@localhost.localdomain>


On Aug 26, 2009, at 1:07 AM, Grant Likely wrote:

> +/**
> + * pci_scan_phb - Given a pci_controller, setup and scan the PCI bus
> + * @hose: Pointer to the PCI host controller instance structure
> + * @data: value to use for sysdata pointer.  ppc32 and ppc64 differ  
> here
> + *
> + * Note: the 'data' pointer is a temporary measure.  As 32 and 64 bit
> + * pci code gets merged, this parameter should become unnecessary  
> because
> + * both will use the same value.
> + */
> +void __devinit pcibios_scan_phb(struct pci_controller *hose, void  
> *data)
> +{
> +

Just a nit, but why not data -> sysdata since that's what we using it  
as.

- k

^ permalink raw reply

* Re: [PATCH V2 2/3] powerpc/pci: move pci_64.c device tree scanning code into pci-common.c
From: Kumar Gala @ 2009-08-26 13:24 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, sfr
In-Reply-To: <20090826060708.30936.54239.stgit@localhost.localdomain>


On Aug 26, 2009, at 1:07 AM, Grant Likely wrote:

> From: Grant Likely <grant.likely@secretlab.ca>
>
> The PCI device tree scanning code in pci_64.c is some useful  
> functionality.
> It allows PCI devices to be described in the device tree instead of  
> being
> probed for, which in turn allows pci devices to use all of the  
> device tree
> facilities to describe complex PCI bus architectures like GPIO and IRQ
> routing (perhaps not a common situation for desktop or server systems,
> but useful for embedded systems with on-board PCI devices).
>
> This patch moves the device tree scanning into pci-common.c so it is
> available for 32-bit powerpc machines too.
>
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
>
> arch/powerpc/include/asm/pci-bridge.h |    5
> arch/powerpc/include/asm/pci.h        |    5
> arch/powerpc/kernel/Makefile          |    2
> arch/powerpc/kernel/pci-common.c      |    1
> arch/powerpc/kernel/pci_64.c          |  289  
> ---------------------------
> arch/powerpc/kernel/pci_of_scan.c     |  358 ++++++++++++++++++++++++ 
> +++++++++
> 6 files changed, 364 insertions(+), 296 deletions(-)
> create mode 100644 arch/powerpc/kernel/pci_of_scan.c
>

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

- k

^ permalink raw reply

* Re: [PATCH] powerpc: Change archdata dma_data type to dma_addr_t
From: Benjamin Herrenschmidt @ 2009-08-26 12:29 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linuxppc-dev
In-Reply-To: <20090824194800.GA13503@lst.de>

On Mon, 2009-08-24 at 21:48 +0200, Christoph Hellwig wrote:
> On Mon, Aug 24, 2009 at 11:17:14AM -0500, Becky Bruce wrote:
> > Previously, this was specified as a void *, but that's not
> > large enough on 32-bit systems with 36-bit physical
> > addressing support.  Change the type to dma_addr_t so it
> > will scale based on the size of a dma address.
> 
> This looks extreml ugly to me.  It seems like the typical use is to
> store a pointer to a structure.  So what about making the direct
> dma case follow that general scheme instead?
> 
> E.g. declare a
> 
> struct direct_dma_data {
> 	dma_addr_t	direct_dma_offset;
> };
> 
> and have one normal instace of it, and one per weird cell device.

Right, but we want to avoid a structure for the classic case of 32-bit
systems with no iommu... 

I wouldn't mind doing a union here. 

The other option is to have a global somewhere that we make that point
to or something like that but it's probably even more ugly.

Cheers,
Ben.

^ permalink raw reply

* Re: [v2 PATCH 2/2]: pseries: Implement Pseries Processor Idle idle module.
From: Peter Zijlstra @ 2009-08-26 11:37 UTC (permalink / raw)
  To: arun
  Cc: Gautham R Shenoy, Pallipadi, Venkatesh, linux-kernel,
	Paul Mackerras, Ingo Molnar, linuxppc-dev, Balbir Singh
In-Reply-To: <20090826113217.GA14758@linux.vnet.ibm.com>

On Wed, 2009-08-26 at 17:02 +0530, Arun R Bharadwaj wrote:
> * Peter Zijlstra <a.p.zijlstra@chello.nl> [2009-08-26 13:27:18]:
> 
> > On Wed, 2009-08-26 at 16:40 +0530, Arun R Bharadwaj wrote:
> > > +void (*pm_idle)(void);
> > > +EXPORT_SYMBOL_GPL(pm_idle);
> > 
> > Seriously.. this caused plenty problems over on x86 and you're doing the
> > exact same dumb thing?
> > 
> 
> Hi Peter,
> 
> Cpuidle assumes pm_idle to be the default idle power management
> function. So i should either do this, or change the stuff in cpuidle.c
> so that it is more abstract.

I would much prefer the latter, I've been telling the x86 power folks to
fix this like forever, but they never seem to get around to it.

They even tried adding a second such unmanaged function pointer for
play-dead, instead of integrating everything into a single management
interface.

^ permalink raw reply

* [PATCH] IB/ehca: Construct MAD redirect replies from request MAD
From: Joachim Fenkes @ 2009-08-26 11:37 UTC (permalink / raw)
  To: LinuxPPC-Dev, LKML, OF-General, Roland Dreier, OF-EWG,
	Jason Gunthorpe, Hal Rosenstock
  Cc: Stefan Roscher, Christoph Raisch

The old code used a lot of hardcoded values, which might not be valid in all
environments (especially routed fabrics or partitioned subnets). Copy as
much information as possible from the incoming request to prevent that.

Signed-off-by: Joachim Fenkes <fenkes@de.ibm.com>
---

Hal, Jason -- here's the change I promised. Looks okay to you?
Roland -- if Hal and Jason don't object, please queue this up for the next
kernel. Thanks!

Regards,
  Joachim

 drivers/infiniband/hw/ehca/ehca_sqp.c |   47 ++++++++++++++++++++++++++++----
 1 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/drivers/infiniband/hw/ehca/ehca_sqp.c b/drivers/infiniband/hw/ehca/ehca_sqp.c
index c568b28..8c1213f 100644
--- a/drivers/infiniband/hw/ehca/ehca_sqp.c
+++ b/drivers/infiniband/hw/ehca/ehca_sqp.c
@@ -125,14 +125,30 @@ struct ib_perf {
 	u8 data[192];
 } __attribute__ ((packed));
 
+/* TC/SL/FL packed into 32 bits, as in ClassPortInfo */
+struct tcslfl {
+	u32 tc:8;
+	u32 sl:4;
+	u32 fl:20;
+} __attribute__ ((packed));
+
+/* IP Version/TC/FL packed into 32 bits, as in GRH */
+struct vertcfl {
+	u32 ver:4;
+	u32 tc:8;
+	u32 fl:20;
+} __attribute__ ((packed));
 
 static int ehca_process_perf(struct ib_device *ibdev, u8 port_num,
+			     struct ib_wc *in_wc, struct ib_grh *in_grh,
 			     struct ib_mad *in_mad, struct ib_mad *out_mad)
 {
 	struct ib_perf *in_perf = (struct ib_perf *)in_mad;
 	struct ib_perf *out_perf = (struct ib_perf *)out_mad;
 	struct ib_class_port_info *poi =
 		(struct ib_class_port_info *)out_perf->data;
+	struct tcslfl *tcslfl =
+		(struct tcslfl *)&poi->redirect_tcslfl;
 	struct ehca_shca *shca =
 		container_of(ibdev, struct ehca_shca, ib_device);
 	struct ehca_sport *sport = &shca->sport[port_num - 1];
@@ -158,10 +174,29 @@ static int ehca_process_perf(struct ib_device *ibdev, u8 port_num,
 		poi->base_version = 1;
 		poi->class_version = 1;
 		poi->resp_time_value = 18;
-		poi->redirect_lid = sport->saved_attr.lid;
-		poi->redirect_qp = sport->pma_qp_nr;
+
+		/* copy local routing information from WC where applicable */
+		tcslfl->sl         = in_wc->sl;
+		poi->redirect_lid  =
+			sport->saved_attr.lid | in_wc->dlid_path_bits;
+		poi->redirect_qp   = sport->pma_qp_nr;
 		poi->redirect_qkey = IB_QP1_QKEY;
-		poi->redirect_pkey = IB_DEFAULT_PKEY_FULL;
+
+		ehca_query_pkey(ibdev, port_num, in_wc->pkey_index,
+				&poi->redirect_pkey);
+
+		/* if request was globally routed, copy route info */
+		if (in_grh) {
+			struct vertcfl *vertcfl =
+				(struct vertcfl *)&in_grh->version_tclass_flow;
+			memcpy(poi->redirect_gid, in_grh->dgid.raw,
+			       sizeof(poi->redirect_gid));
+			tcslfl->tc        = vertcfl->tc;
+			tcslfl->fl        = vertcfl->fl;
+		} else
+			/* else only fill in default GID */
+			ehca_query_gid(ibdev, port_num, 0,
+				       (union ib_gid *)&poi->redirect_gid);
 
 		ehca_dbg(ibdev, "ehca_pma_lid=%x ehca_pma_qp=%x",
 			 sport->saved_attr.lid, sport->pma_qp_nr);
@@ -183,8 +218,7 @@ perf_reply:
 
 int ehca_process_mad(struct ib_device *ibdev, int mad_flags, u8 port_num,
 		     struct ib_wc *in_wc, struct ib_grh *in_grh,
-		     struct ib_mad *in_mad,
-		     struct ib_mad *out_mad)
+		     struct ib_mad *in_mad, struct ib_mad *out_mad)
 {
 	int ret;
 
@@ -196,7 +230,8 @@ int ehca_process_mad(struct ib_device *ibdev, int mad_flags, u8 port_num,
 		return IB_MAD_RESULT_SUCCESS;
 
 	ehca_dbg(ibdev, "port_num=%x src_qp=%x", port_num, in_wc->src_qp);
-	ret = ehca_process_perf(ibdev, port_num, in_mad, out_mad);
+	ret = ehca_process_perf(ibdev, port_num, in_wc, in_grh,
+				in_mad, out_mad);
 
 	return ret;
 }
-- 
1.6.0.4

^ permalink raw reply related

* Re: [v2 PATCH 2/2]: pseries: Implement Pseries Processor Idle idle module.
From: Peter Zijlstra @ 2009-08-26 11:27 UTC (permalink / raw)
  To: arun
  Cc: Gautham R Shenoy, linux-kernel, Paul Mackerras, Ingo Molnar,
	linuxppc-dev, Balbir Singh
In-Reply-To: <20090826111003.GC29759@linux.vnet.ibm.com>

On Wed, 2009-08-26 at 16:40 +0530, Arun R Bharadwaj wrote:
> +void (*pm_idle)(void);
> +EXPORT_SYMBOL_GPL(pm_idle);

Seriously.. this caused plenty problems over on x86 and you're doing the
exact same dumb thing?

^ permalink raw reply

* Re: [v2 PATCH 2/2]: pseries: Implement Pseries Processor Idle idle module.
From: Arun R Bharadwaj @ 2009-08-26 11:32 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Gautham R Shenoy, linux-kernel, Paul Mackerras, Arun Bharadwaj,
	Ingo Molnar, linuxppc-dev, Balbir Singh
In-Reply-To: <1251286038.1329.1.camel@twins>

* Peter Zijlstra <a.p.zijlstra@chello.nl> [2009-08-26 13:27:18]:

> On Wed, 2009-08-26 at 16:40 +0530, Arun R Bharadwaj wrote:
> > +void (*pm_idle)(void);
> > +EXPORT_SYMBOL_GPL(pm_idle);
> 
> Seriously.. this caused plenty problems over on x86 and you're doing the
> exact same dumb thing?
> 

Hi Peter,

Cpuidle assumes pm_idle to be the default idle power management
function. So i should either do this, or change the stuff in cpuidle.c
so that it is more abstract.

--arun

^ permalink raw reply

* [v2 PATCH 2/2]: pseries: Implement Pseries Processor Idle idle module.
From: Arun R Bharadwaj @ 2009-08-26 11:10 UTC (permalink / raw)
  To: Joel Schopp, Benjamin Herrenschmidt, Paul Mackerras,
	Peter Zijlstra, Ingo Molnar, Vaidyanathan Srinivasan,
	Dipankar Sarma, Balbir Singh, Gautham R Shenoy, Arun Bharadwaj
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20090826110721.GA29759@linux.vnet.ibm.com>

* Arun R Bharadwaj <arun@linux.vnet.ibm.com> [2009-08-26 16:37:21]:

This patch creates arch/powerpc/platforms/pseries/processor_idle.c,
which implements the cpuidle infrastructure for pseries.
It implements a pseries_cpuidle_loop() which would be the main idle loop
called from cpu_idle(). It makes decision of entering either snooze or nap
state based on the decision taken by the cpuidle governor.

Signed-off-by: Arun R Bharadwaj <arun@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/Makefile         |    1 
 arch/powerpc/platforms/pseries/processor_idle.c |  181 ++++++++++++++++++++++++
 arch/powerpc/platforms/pseries/pseries.h        |   14 +
 arch/powerpc/platforms/pseries/setup.c          |    3 
 4 files changed, 196 insertions(+), 3 deletions(-)

Index: linux.trees.git/arch/powerpc/platforms/pseries/Makefile
===================================================================
--- linux.trees.git.orig/arch/powerpc/platforms/pseries/Makefile
+++ linux.trees.git/arch/powerpc/platforms/pseries/Makefile
@@ -26,3 +26,4 @@ obj-$(CONFIG_HCALL_STATS)	+= hvCall_inst
 obj-$(CONFIG_PHYP_DUMP)	+= phyp_dump.o
 obj-$(CONFIG_CMM)		+= cmm.o
 obj-$(CONFIG_DTL)		+= dtl.o
+obj-$(CONFIG_PSERIES_PROCESSOR_IDLE)	+= processor_idle.o
Index: linux.trees.git/arch/powerpc/platforms/pseries/pseries.h
===================================================================
--- linux.trees.git.orig/arch/powerpc/platforms/pseries/pseries.h
+++ linux.trees.git/arch/powerpc/platforms/pseries/pseries.h
@@ -10,6 +10,8 @@
 #ifndef _PSERIES_PSERIES_H
 #define _PSERIES_PSERIES_H
 
+#include <linux/cpuidle.h>
+
 extern void __init fw_feature_init(const char *hypertas, unsigned long len);
 
 struct pt_regs;
@@ -40,4 +42,16 @@ extern unsigned long rtas_poweron_auto;
 
 extern void find_udbg_vterm(void);
 
+DECLARE_PER_CPU(unsigned long, smt_snooze_delay);
+
+#ifdef CONFIG_PSERIES_PROCESSOR_IDLE
+struct pseries_processor_power {
+	struct cpuidle_device dev;
+	int count;
+	int id;
+};
+
+extern struct cpuidle_driver pseries_idle_driver;
+#endif
+
 #endif /* _PSERIES_PSERIES_H */
Index: linux.trees.git/arch/powerpc/platforms/pseries/processor_idle.c
===================================================================
--- /dev/null
+++ linux.trees.git/arch/powerpc/platforms/pseries/processor_idle.c
@@ -0,0 +1,181 @@
+/*
+ *  processor_idle - idle state cpuidle driver.
+ *  Adapted from drivers/acpi/processor_idle.c
+ *
+ *  Arun R Bharadwaj <arun@linux.vnet.ibm.com>
+ *
+ *  Copyright (C) 2009 IBM Corporation.
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or (at
+ *  your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful, but
+ *  WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License along
+ *  with this program; if not, write to the Free Software Foundation, Inc.,
+ *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/moduleparam.h>
+#include <linux/cpuidle.h>
+
+#include <asm/paca.h>
+#include <asm/reg.h>
+#include <asm/machdep.h>
+
+#include "plpar_wrappers.h"
+#include "pseries.h"
+
+MODULE_AUTHOR("Arun R Bharadwaj");
+MODULE_DESCRIPTION("pSeries Idle State Driver");
+MODULE_LICENSE("GPL");
+
+struct cpuidle_driver pseries_idle_driver = {
+	.name =		"pseries_idle",
+	.owner =	THIS_MODULE,
+};
+
+void (*pm_idle)(void);
+EXPORT_SYMBOL_GPL(pm_idle);
+
+DEFINE_PER_CPU(struct pseries_processor_power, power);
+
+#define IDLE_STATE_COUNT	2
+
+static int pseries_idle_init(struct pseries_processor_power *power)
+{
+	return cpuidle_register_device(&power->dev);
+}
+
+static void snooze(void)
+{
+	local_irq_enable();
+	set_thread_flag(TIF_POLLING_NRFLAG);
+	while (!need_resched()) {
+		ppc64_runlatch_off();
+		HMT_low();
+		HMT_very_low();
+	}
+	HMT_medium();
+	clear_thread_flag(TIF_POLLING_NRFLAG);
+	smp_mb();
+	local_irq_disable();
+}
+
+static void nap(void)
+{
+	ppc64_runlatch_off();
+	HMT_medium();
+	cede_processor();
+}
+
+static int pseries_cpuidle_loop(struct cpuidle_device *dev,
+				struct cpuidle_state *st)
+{
+	ktime_t t1, t2;
+	s64 diff;
+	int ret;
+	unsigned long in_purr, out_purr;
+
+	get_lppaca()->idle = 1;
+	get_lppaca()->donate_dedicated_cpu = 1;
+	in_purr = mfspr(SPRN_PURR);
+
+	t1 = ktime_get();
+
+	if (strcmp(st->desc, "snooze") == 0)
+		snooze();
+	else
+		nap();
+
+	t2 = ktime_get();
+	diff = ktime_to_us(ktime_sub(t2, t1));
+	if (diff > INT_MAX)
+		diff = INT_MAX;
+
+	ret = (int) diff;
+
+	out_purr = mfspr(SPRN_PURR);
+	get_lppaca()->wait_state_cycles += out_purr - in_purr;
+	get_lppaca()->donate_dedicated_cpu = 0;
+	get_lppaca()->idle = 0;
+
+	return ret;
+}
+
+static int pseries_setup_cpuidle(struct pseries_processor_power *power)
+{
+	int i;
+	struct cpuidle_state *state;
+	struct cpuidle_device *dev = &power->dev;
+
+	dev->cpu = power->id;
+
+	dev->enabled = 0;
+	for (i = 0; i < IDLE_STATE_COUNT; i++) {
+		state = &dev->states[i];
+
+		snprintf(state->name, CPUIDLE_NAME_LEN, "IDLE%d", i);
+		state->enter = pseries_cpuidle_loop;
+
+		switch (i) {
+		case 0:
+			strncpy(state->desc, "snooze", CPUIDLE_DESC_LEN);
+			state->exit_latency = 0;
+			state->target_residency = 0;
+			break;
+
+		case 1:
+			strncpy(state->desc, "nap", CPUIDLE_DESC_LEN);
+			state->exit_latency = 1;
+			state->target_residency =
+					__get_cpu_var(smt_snooze_delay);
+			break;
+		}
+	}
+
+	power->dev.state_count = i;
+	return 0;
+}
+
+static int pseries_get_power_info(struct pseries_processor_power *power,
+				int cpu)
+{
+	power->id = cpu;
+	power->count = IDLE_STATE_COUNT;
+	return 0;
+}
+
+static int __init pseries_processor_idle_init(void)
+{
+	int cpu;
+	int result = cpuidle_register_driver(&pseries_idle_driver);
+
+	if (result < 0)
+		return result;
+
+	printk(KERN_DEBUG "pSeries idle driver registered\n");
+
+	for_each_online_cpu(cpu) {
+		pseries_get_power_info(&per_cpu(power, cpu), cpu);
+		pseries_setup_cpuidle(&per_cpu(power, cpu));
+		pseries_idle_init(&per_cpu(power, cpu));
+	}
+
+	printk(KERN_DEBUG "Using cpuidle idle loop\n");
+	ppc_md.power_save = pm_idle;
+	return 0;
+}
+
+late_initcall(pseries_processor_idle_init);
Index: linux.trees.git/arch/powerpc/platforms/pseries/setup.c
===================================================================
--- linux.trees.git.orig/arch/powerpc/platforms/pseries/setup.c
+++ linux.trees.git/arch/powerpc/platforms/pseries/setup.c
@@ -521,9 +521,6 @@ static int __init pSeries_probe(void)
 	return 1;
 }
 
-
-DECLARE_PER_CPU(unsigned long, smt_snooze_delay);
-
 static void pseries_dedicated_idle_sleep(void)
 { 
 	unsigned int cpu = smp_processor_id();

^ permalink raw reply

* [v2 PATCH 1/2]: pseries: Enable cpuidle for pSeries.
From: Arun R Bharadwaj @ 2009-08-26 11:08 UTC (permalink / raw)
  To: Joel Schopp, Benjamin Herrenschmidt, Paul Mackerras,
	Peter Zijlstra, Ingo Molnar, Vaidyanathan Srinivasan,
	Dipankar Sarma, Balbir Singh, Gautham R Shenoy, Arun Bharadwaj
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20090826110721.GA29759@linux.vnet.ibm.com>

* Arun R Bharadwaj <arun@linux.vnet.ibm.com> [2009-08-26 16:37:21]:

This patch enables the cpuidle option in Kconfig for pSeries.

Currently cpuidle infrastructure is enabled only for x86.
This code is almost completely borrowed from x86 to enable
cpuidle for pSeries.

Signed-off-by: Arun R Bharadwaj <arun@linux.vnet.ibm.com>
---
 arch/powerpc/Kconfig                   |   17 +++++++++++++++++
 arch/powerpc/include/asm/system.h      |    2 ++
 arch/powerpc/platforms/pseries/setup.c |   21 +++++++++++++++++++++
 3 files changed, 40 insertions(+)

Index: linux.trees.git/arch/powerpc/Kconfig
===================================================================
--- linux.trees.git.orig/arch/powerpc/Kconfig
+++ linux.trees.git/arch/powerpc/Kconfig
@@ -88,6 +88,9 @@ config ARCH_HAS_ILOG2_U64
 	bool
 	default y if 64BIT
 
+config ARCH_HAS_CPU_IDLE_WAIT
+	def_bool y
+
 config GENERIC_HWEIGHT
 	bool
 	default y
@@ -243,6 +246,20 @@ source "kernel/Kconfig.freezer"
 source "arch/powerpc/sysdev/Kconfig"
 source "arch/powerpc/platforms/Kconfig"
 
+menu "Power management options"
+
+source "drivers/cpuidle/Kconfig"
+
+config PSERIES_PROCESSOR_IDLE
+	bool "Idle Power Management Support for pSeries"
+	depends on PPC_PSERIES && CPU_IDLE
+	default y
+	help
+	  Idle Power Management Support for pSeries. This hooks onto cpuidle
+	  infrastructure to help in idle cpu power management.
+
+endmenu
+
 menu "Kernel options"
 
 config HIGHMEM
Index: linux.trees.git/arch/powerpc/platforms/pseries/setup.c
===================================================================
--- linux.trees.git.orig/arch/powerpc/platforms/pseries/setup.c
+++ linux.trees.git/arch/powerpc/platforms/pseries/setup.c
@@ -278,6 +278,27 @@ static struct notifier_block pci_dn_reco
 	.notifier_call = pci_dn_reconfig_notifier,
 };
 
+static void do_nothing(void *unused)
+{
+}
+
+/*
+ * cpu_idle_wait - Used to ensure that all the CPUs discard old value of
+ * pm_idle and update to new pm_idle value. Required while changing pm_idle
+ * handler on SMP systems.
+ *
+ * Caller must have changed pm_idle to the new value before the call. Old
+ * pm_idle value will not be used by any CPU after the return of this function.
+ */
+void cpu_idle_wait(void)
+{
+	/* Ensure that new value of pm_idle is set before proceeding */
+	smp_mb();
+	/* kick all the CPUs so that they exit out of pm_idle */
+	smp_call_function(do_nothing, NULL, 1);
+}
+EXPORT_SYMBOL_GPL(cpu_idle_wait);
+
 static void __init pSeries_setup_arch(void)
 {
 	/* Discover PIC type and setup ppc_md accordingly */
Index: linux.trees.git/arch/powerpc/include/asm/system.h
===================================================================
--- linux.trees.git.orig/arch/powerpc/include/asm/system.h
+++ linux.trees.git/arch/powerpc/include/asm/system.h
@@ -546,5 +546,7 @@ extern void account_system_vtime(struct 
 
 extern struct dentry *powerpc_debugfs_root;
 
+void cpu_idle_wait(void);
+
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_SYSTEM_H */

^ permalink raw reply

* [v2 PATCH 0/2]: cpuidle: Introducing cpuidle infrastructure to POWER
From: Arun R Bharadwaj @ 2009-08-26 11:07 UTC (permalink / raw)
  To: Joel Schopp, Benjamin Herrenschmidt, Paul Mackerras,
	Peter Zijlstra, Ingo Molnar, Vaidyanathan Srinivasan,
	Dipankar Sarma, Balbir Singh, Gautham R Shenoy, Arun R Bharadwaj
  Cc: linuxppc-dev, linux-kernel

Hi,



"Cpuidle" is a CPU Power Management infrastrusture which helps manage
idle CPUs in a clean and efficient manner. The architecture can register
its driver (in this case, pseries_idle driver) so that it subscribes for
cpuidle feature. Cpuidle has a set of governors (ladder and menu),
which will decide the best idle state to be chosen for the current situation,
based on heuristics, and calculates the expected residency time
for the current idle state. So based on this, the cpu is put into
the right idle state.

Currently, cpuidle infrasture is exploited by ACPI to choose between
the available ACPI C-states. This patch-set is aimed at enabling
cpuidle for powerpc and provides a sample implementation for pseries.

Currently, in the pseries_dedicated_idle_sleep(), the processor would
poll for a time period, which is called the snooze, and only then it
is ceded, which would put the processor in nap state. Cpuidle aims at
separating this into 2 different idle states. Based on the expected
residency time predicted by the cpuidle governor, the idle state is
chosen directly. So, choosing to enter the nap state directly based on
the decision made by cpuidle would avoid unnecessary snoozing before
entering nap.

This patch-set tries to achieve the above objective by introducing a
pseries processor idle driver called pseries_idle_driver in
arch/powerpc/platform/pseries/processor_idle.c, which implements the
idle loop which would replace the pseries_dedicated_idle_sleep()
when cpuidle is enabled.

Experiment conducted:
----------------------

The following experiment was conducted on a completely idle JS22 blade,
to prove that using cpuidle infrastructure, the amount of nap time increases.

Nap and snooze times were sampled for all the cpus.
For a window of 1000 samples, When cpuidle was enabled,
the total nap time was of the order of a few seconds (5-10s), whereas
the total snooze time was of the order of a few milliseconds(10-30 ms).

When cpuidle infrastructure was disabled and the regular
pseries_dedicated_idle_sleep() idle loop was used, the snooze time itself
was of the order of hundreds of milliseconds. (100 - 500 ms).
This is clearly due to unnecessary snoozing before napping even on a
completely idle system.


The previous post in this area can be found at
http://lkml.org/lkml/2009/8/19/150

Changes from the previous version:
----------------------------------

Earlier I used the the name TPMD (Thermal and Power Management Devices)
to refer to this module which hooks on to the cpuidle infrastructure.
I have renamed this to Pseries Processor Idle, in order to avoid confusion.


Patches included in this set:
------------------------------
PATCH 1/2 - Enable cpuidle for pSeries.
PATCH 2/2 - Implement Pseries Processor Idle idle module


Any feedback on the overall design and idea is immensely valuable.

--arun

^ permalink raw reply

* [PATCH V2 0/3] Make 64-bit PCI device tree scanning code common
From: Grant Likely @ 2009-08-26  6:09 UTC (permalink / raw)
  To: benh, galak, sfr, linuxppc-dev
In-Reply-To: <20090825162314.10936.11028.stgit@localhost.localdomain>

Hi guys

Here's v2 of the PCI device tree scanning code. =A0This is tested and
working on 32bit. =A0It needs to be tested on 64 bit, but there shouldn't
be any behavioural changes. =A0It's mostly just a code move with a little
bit of fixups to merge code. =A0I'd like to see this go into .32 when the
merge window opens.

Changes in v2:
- Move scanning code into its own file: pci_of_scan.c

On Thu, Aug 20, 2009 at 11:30 PM, Grant Likely<grant.likely@secretlab.ca> w=
rote:
> Ben and Kumar,
>
> Compile tested only. =A0I haven't even tried to boot this on real
> hardware, but I'm posting so that you guys can see what I'm up to.
> Basically, I want access to the device tree scanning in ppc32 land,
> and these patches start to get me there. =A0Please take a look and
> comment. =A0Tomorrow I'll actually try running this stuff and debugging
> the details.
>

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



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

^ permalink raw reply

* [PATCH V2 0/3] Make 64-bit PCI device tree scanning code common
From: Grant Likely @ 2009-08-26  6:07 UTC (permalink / raw)
  To: benh, galak, sfr, linuxppc-dev
In-Reply-To: <20090825162314.10936.11028.stgit@localhost.localdomain>

Hi guys

Here's v2 of the PCI device tree scanning code. =A0This is tested and
working on 32bit. =A0It needs to be tested on 64 bit, but there shouldn't
be any behavioural changes. =A0It's mostly just a code move with a little
bit of fixups to merge code. =A0I'd like to see this go into .32 when the
merge window opens.

Changes in v2:
- Move scanning code into its own file: pci_of_scan.c

On Thu, Aug 20, 2009 at 11:30 PM, Grant Likely<grant.likely@secretlab.ca> w=
rote:
> Ben and Kumar,
>
> Compile tested only. =A0I haven't even tried to boot this on real
> hardware, but I'm posting so that you guys can see what I'm up to.
> Basically, I want access to the device tree scanning in ppc32 land,
> and these patches start to get me there. =A0Please take a look and
> comment. =A0Tomorrow I'll actually try running this stuff and debugging
> the details.
>

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



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

^ permalink raw reply

* [PATCH V2 3/3] powerpc/pci: Merge ppc32 and ppc64 versions of phb_scan()
From: Grant Likely @ 2009-08-26  6:07 UTC (permalink / raw)
  To: benh, galak, sfr, linuxppc-dev
In-Reply-To: <20090826060659.30936.75706.stgit@localhost.localdomain>

From: Grant Likely <grant.likely@secretlab.ca>

The two versions are doing almost exactly the same thing.  No need to
maintain them as separate files.  This patch also has the side effect
of making the PCI device tree scanning code available to 32 bit powerpc
machines, but no board ports actually make use of this feature at this
point.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---

 arch/powerpc/include/asm/pci.h   |    2 ++
 arch/powerpc/kernel/pci-common.c |   48 ++++++++++++++++++++++++++++++++++++++
 arch/powerpc/kernel/pci_32.c     |   25 ++------------------
 arch/powerpc/kernel/pci_64.c     |   46 +++++-------------------------------
 4 files changed, 58 insertions(+), 63 deletions(-)


diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
index 9ae2e3e..feebfed 100644
--- a/arch/powerpc/include/asm/pci.h
+++ b/arch/powerpc/include/asm/pci.h
@@ -233,6 +233,8 @@ extern void pci_resource_to_user(const struct pci_dev *dev, int bar,
 
 extern void pcibios_setup_bus_devices(struct pci_bus *bus);
 extern void pcibios_setup_bus_self(struct pci_bus *bus);
+extern void pcibios_setup_phb_io_space(struct pci_controller *hose);
+extern void pcibios_scan_phb(struct pci_controller *hose, void *data);
 
 #endif	/* __KERNEL__ */
 #endif /* __ASM_POWERPC_PCI_H */
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 44497a8..8a16dbe 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1617,3 +1617,51 @@ void __devinit pcibios_setup_phb_resources(struct pci_controller *hose)
 		 (unsigned long)hose->io_base_virt - _IO_BASE);
 
 }
+
+/**
+ * pci_scan_phb - Given a pci_controller, setup and scan the PCI bus
+ * @hose: Pointer to the PCI host controller instance structure
+ * @data: value to use for sysdata pointer.  ppc32 and ppc64 differ here
+ *
+ * Note: the 'data' pointer is a temporary measure.  As 32 and 64 bit
+ * pci code gets merged, this parameter should become unnecessary because
+ * both will use the same value.
+ */
+void __devinit pcibios_scan_phb(struct pci_controller *hose, void *data)
+{
+	struct pci_bus *bus;
+	struct device_node *node = hose->dn;
+	int mode;
+
+	pr_debug("PCI: Scanning PHB %s\n",
+		 node ? node->full_name : "<NO NAME>");
+
+	/* Create an empty bus for the toplevel */
+	bus = pci_create_bus(hose->parent, hose->first_busno, hose->ops, data);
+	if (bus == NULL) {
+		pr_err("Failed to create bus for PCI domain %04x\n",
+			hose->global_number);
+		return;
+	}
+	bus->secondary = hose->first_busno;
+	hose->bus = bus;
+
+	/* Get some IO space for the new PHB */
+	pcibios_setup_phb_io_space(hose);
+
+	/* Wire up PHB bus resources */
+	pcibios_setup_phb_resources(hose);
+
+	/* Get probe mode and perform scan */
+	mode = PCI_PROBE_NORMAL;
+	if (node && ppc_md.pci_probe_mode)
+		mode = ppc_md.pci_probe_mode(bus);
+	pr_debug("    probe mode: %d\n", mode);
+	if (mode == PCI_PROBE_DEVTREE) {
+		bus->subordinate = hose->last_busno;
+		of_scan_bus(node, bus);
+	}
+
+	if (mode == PCI_PROBE_NORMAL)
+		hose->last_busno = bus->subordinate = pci_scan_child_bus(bus);
+}
diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
index 1e807fe..4e415e1 100644
--- a/arch/powerpc/kernel/pci_32.c
+++ b/arch/powerpc/kernel/pci_32.c
@@ -354,36 +354,15 @@ pci_create_OF_bus_map(void)
 	}
 }
 
-static void __devinit pcibios_scan_phb(struct pci_controller *hose)
+void __devinit pcibios_setup_phb_io_space(struct pci_controller *hose)
 {
-	struct pci_bus *bus;
-	struct device_node *node = hose->dn;
 	unsigned long io_offset;
 	struct resource *res = &hose->io_resource;
 
-	pr_debug("PCI: Scanning PHB %s\n",
-		 node ? node->full_name : "<NO NAME>");
-
-	/* Create an empty bus for the toplevel */
-	bus = pci_create_bus(hose->parent, hose->first_busno, hose->ops, hose);
-	if (bus == NULL) {
-		printk(KERN_ERR "Failed to create bus for PCI domain %04x\n",
-		       hose->global_number);
-		return;
-	}
-	bus->secondary = hose->first_busno;
-	hose->bus = bus;
-
 	/* Fixup IO space offset */
 	io_offset = (unsigned long)hose->io_base_virt - isa_io_base;
 	res->start = (res->start + io_offset) & 0xffffffffu;
 	res->end = (res->end + io_offset) & 0xffffffffu;
-
-	/* Wire up PHB bus resources */
-	pcibios_setup_phb_resources(hose);
-
-	/* Scan children */
-	hose->last_busno = bus->subordinate = pci_scan_child_bus(bus);
 }
 
 static int __init pcibios_init(void)
@@ -401,7 +380,7 @@ static int __init pcibios_init(void)
 		if (pci_assign_all_buses)
 			hose->first_busno = next_busno;
 		hose->last_busno = 0xff;
-		pcibios_scan_phb(hose);
+		pcibios_scan_phb(hose, hose);
 		pci_bus_add_devices(hose->bus);
 		if (pci_assign_all_buses || next_busno <= hose->last_busno)
 			next_busno = hose->last_busno + pcibios_assign_bus_offset;
diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
index 4d5b4ce..ba949a2 100644
--- a/arch/powerpc/kernel/pci_64.c
+++ b/arch/powerpc/kernel/pci_64.c
@@ -43,45 +43,6 @@ unsigned long pci_probe_only = 1;
 unsigned long pci_io_base = ISA_IO_BASE;
 EXPORT_SYMBOL(pci_io_base);
 
-void __devinit scan_phb(struct pci_controller *hose)
-{
-	struct pci_bus *bus;
-	struct device_node *node = hose->dn;
-	int mode;
-
-	pr_debug("PCI: Scanning PHB %s\n",
-		 node ? node->full_name : "<NO NAME>");
-
-	/* Create an empty bus for the toplevel */
-	bus = pci_create_bus(hose->parent, hose->first_busno, hose->ops, node);
-	if (bus == NULL) {
-		printk(KERN_ERR "Failed to create bus for PCI domain %04x\n",
-		       hose->global_number);
-		return;
-	}
-	bus->secondary = hose->first_busno;
-	hose->bus = bus;
-
-	/* Get some IO space for the new PHB */
-	pcibios_map_io_space(bus);
-
-	/* Wire up PHB bus resources */
-	pcibios_setup_phb_resources(hose);
-
-	/* Get probe mode and perform scan */
-	mode = PCI_PROBE_NORMAL;
-	if (node && ppc_md.pci_probe_mode)
-		mode = ppc_md.pci_probe_mode(bus);
-	pr_debug("    probe mode: %d\n", mode);
-	if (mode == PCI_PROBE_DEVTREE) {
-		bus->subordinate = hose->last_busno;
-		of_scan_bus(node, bus);
-	}
-
-	if (mode == PCI_PROBE_NORMAL)
-		hose->last_busno = bus->subordinate = pci_scan_child_bus(bus);
-}
-
 static int __init pcibios_init(void)
 {
 	struct pci_controller *hose, *tmp;
@@ -103,7 +64,7 @@ static int __init pcibios_init(void)
 
 	/* Scan all of the recorded PCI controllers.  */
 	list_for_each_entry_safe(hose, tmp, &hose_list, list_node) {
-		scan_phb(hose);
+		pcibios_scan_phb(hose, hose->dn);
 		pci_bus_add_devices(hose->bus);
 	}
 
@@ -237,6 +198,11 @@ int __devinit pcibios_map_io_space(struct pci_bus *bus)
 }
 EXPORT_SYMBOL_GPL(pcibios_map_io_space);
 
+void __devinit pcibios_setup_phb_io_space(struct pci_controller *hose)
+{
+	pcibios_map_io_space(hose->bus);
+}
+
 #define IOBASE_BRIDGE_NUMBER	0
 #define IOBASE_MEMORY		1
 #define IOBASE_IO		2

^ 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