LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v3 1/4] powerpc: Removing support for 'protected-sources'
From: Benjamin Herrenschmidt @ 2011-02-07  1:37 UTC (permalink / raw)
  To: Meador Inge; +Cc: Hollis Blanchard, devicetree-discuss, linuxppc-dev
In-Reply-To: <4D4F4BAD.4000909@mentor.com>

On Sun, 2011-02-06 at 19:32 -0600, Meador Inge wrote:
> So barring the removal of protected sources, does the inclusion of the
> "pic-no-reset" property seem reasonable? 

Sure.

Ben.

^ permalink raw reply

* Re: [PATCH v3 1/4] powerpc: Removing support for 'protected-sources'
From: Meador Inge @ 2011-02-07  1:32 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Hollis Blanchard, devicetree-discuss, linuxppc-dev
In-Reply-To: <1297035336.14982.14.camel@pasglop>

On 02/06/2011 05:35 PM, Benjamin Herrenschmidt wrote:
> On Fri, 2011-02-04 at 17:25 -0600, Meador Inge wrote:
>> In a recent thread [1,2,3] concerning device trees for AMP systems, the
>> question of whether we really need 'protected-sources' arose.  The general
>> consensus was that a new boolean property 'pic-no-reset' (described in more
>> detail in a following patch) could be expanded to cover the use cases that
>> 'protected-sources' was covering.
>>
>> One concern that was raised was for legacy systems which already use the
>> 'protected-sources' property [4].  For legacy use cases, 'protected-sources'
>> will be treated as an alias of 'pic-no-reset'.  The sources
>> encoded in the 'protected-sources' cells, however, will not be processed.  This
>> legacy check is added in a later patch in the series.
>
> I'm a bit annoyed here. Why do we need to do that ? Those Cell machines

Apologies, that certainly was not the intent.

> are going to be real bastards to find and test with, and I don't really
> see the point...

The idea came up when submitting a patch for documenting an Open PIC 
binding.  The following two properties were documented in that binding: 
(1) "protected-sources" and (2) "pic-no-reset".  "pic-no-reset" is a 
newly proposed property with the intent of specifying from a device tree 
that the PIC should not be reset.  The question of whether the one 
property, "pic-no-reset", would suffice for both purposes came up.

It seems reasonable that "pic-no-reset" could satisfy the use case that 
"protected-sources" is covering (since all of the sources that a 
particular machine is actually using are already explicitly mentioned in 
the device tree) and the use case of marking from a device tree that the 
PIC should not be reset.  So it is not so much as a need as it is a 
potential simplification.  It sounds like as a practical concern 
(several systems using "protected-sources" are already in the wild and 
testing considerations) that this may not be possible, which is fine.

> The reason for not resetting the MPIC wasn't -only- about the protected
> sources, but also because, from memory, some MPIC implementations had
> issues when toggling the reset lines (pseries I think is one).
>
> That's actually why the MPIC_WANTS_RESET flag is working the other way
> around, only platforms that actually want the reset set it.
>
> This is orthogonal to the need to touch or not touch the interrupt
> sources as set by firmware. Yes, having protected sources probably
> implies no-reset but the reverse isn't necessarily true.

So barring the removal of protected sources, does the inclusion of the 
"pic-no-reset" property seem reasonable?

> Ben.

-- 
Meador Inge     | meador_inge AT mentor.com
Mentor Embedded | http://www.mentor.com/embedded-software

^ permalink raw reply

* Re: [PATCH] watchdog: booke_wdt: clean up status messages
From: Benjamin Herrenschmidt @ 2011-02-07  1:06 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev, wim, linux-watchdog
In-Reply-To: <1295299780-27338-1-git-send-email-timur@freescale.com>

On Mon, 2011-01-17 at 15:29 -0600, Timur Tabi wrote:
> Improve the status messages that are displayed during some operations of the
> PowerPC watchdog timer driver.  When the watchdog is enabled, the timeout is
> displayed as a number of seconds, instead of an obscure "period".  The "period"
> is the position of a bit in a 64-bit timer register.  The higher the value,
> the quicker the watchdog timeout occurs.  Some people chose a high "period"
> value for the timer and get confused as to why the board resets within a
> few seconds.
> 
> Messages displayed during open and close are now debug messages, so that they
> don't clutter the console by default.  Finally, printk() is replaced with the
> pr_xxx() equivalent.

Minor nit bu
>  
> -	printk(KERN_INFO "PowerPC Book-E Watchdog Timer Loaded\n");
> +	pr_info("PowerPC Book-E Watchdog Timer Loaded\n");
>  	ident.firmware_version = cur_cpu_spec->pvr_value;
>  
>  	ret = misc_register(&booke_wdt_miscdev);
>  	if (ret) {
> -		printk(KERN_CRIT "Cannot register miscdev on minor=%d: %d\n",
> -				WATCHDOG_MINOR, ret);
> +		pr_err("booke_wdt: cannot register device (minor=%u, ret=%i)\n",
> +		       WATCHDOG_MINOR, ret);
>  		return ret;
>  	}
>  
>  	spin_lock(&booke_wdt_lock);
>  	if (booke_wdt_enabled == 1) {
> -		printk(KERN_INFO
> -		      "PowerPC Book-E Watchdog Timer Enabled (wdt_period=%d)\n",
> -				booke_wdt_period);
> +		pr_info("booke_wdt: watchdog enabled (timeout = %llu sec)\n",
> +			period_to_sec(booke_wdt_period));
>  		on_each_cpu(__booke_wdt_enable, NULL, 0);
>  	}

If you're going to cleanup the messages, then I think displaying:

PowerPC Book-E Watchdog Timer Loaded
booke_wdt: watchdog enabled (timeout = xxx sec)

Isn't very nice.

Lacks consistency ... you can prefix both lines the same way, or
maybe better print only one line with all the necessary info.

Ben.

^ permalink raw reply

* Re: [PATCH 1/2] video, sm501: add OF binding to support SM501
From: Benjamin Herrenschmidt @ 2011-02-06 23:45 UTC (permalink / raw)
  To: Heiko Schocher; +Cc: linux-fbdev, devicetree-discuss, linuxppc-dev
In-Reply-To: <1291451028-22532-1-git-send-email-hs@denx.de>

On Sat, 2010-12-04 at 09:23 +0100, Heiko Schocher wrote:
> - add binding to OF, compatible name "smi,sm501"
> 
> - add read/write functions for using this driver
>   also on powerpc plattforms

 .../...

Who plans to merge that patch series ? I'm happy for whoever is doing
that to take the powerpc patch at the end (that adds the entry to
the .dts file) with my

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

Cheers,
Ben.

^ permalink raw reply

* RE: [PATCH] e500: Erratum cpu a005 workaround
From: Benjamin Herrenschmidt @ 2011-02-06 23:49 UTC (permalink / raw)
  To: Liu Yu-B13201; +Cc: Gala Kumar-B11780, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <FBDE6FBB4662C043AC9EECB95F62CDDE114038@039-SN1MPN1-005.039d.mgd.msft.net>


> > 
> > This isn't the way to do this.  We normally add entries in 
> > cputable.c an add a new cpu_feature_bit for the errata.
> > 
> > Than above we'd do:
> > 
> > if (cur_cpu_spec->cpu_features & CPU_FTR_E500_A005_ERRATUM)
> > 
> > 
> 
> IMHO, a cpu erratum is not a cpu feature.
> See there're only 32 bits can be used for all PowerPC platform to represent cpu feature, 
> then is it worth consuming one of them to represent one e500 erratum?

This is an interesting debate :-)

We have used cpu_features for errata in the past. However, we are
getting a bit short and I'd rather keep CPU features for things
that are spread out in multiple places and/or hitting hot code path.

If the workaround is very limited to a single non-critical code path,
testing the PVR might actually be a nicer way to do it.

Now, this specific patch is ... hrm ... hard to decide. I don't like
that much the global variable. On the other hand, it's static and allows
the whole business of this errata to be completely local to the
math_efp.c file which is a good thing.

So I'm tempted to say go with the patch as it-is, but I'll let Kumar
ultimately decide.

Cheers,
Ben.

^ permalink raw reply

* [PATCH 21/23] hvc_console: Fix race between hvc_close and hvc_remove
From: Willy Tarreau @ 2011-02-06 23:23 UTC (permalink / raw)
  To: linux-kernel, stable, stable-review
  Cc: Greg Kroah-Hartman, Rusty Russell, linuxppc-dev, Amit Shah,
	Willy Tarreau, Alan Cox
In-Reply-To: <4beed4da27f06efb2c13d6ed48850634@local>

2.6.27.58-stable review patch.  If anyone has any objections, please let us know.

------------------

From: Amit Shah <amit.shah@redhat.com>

commit e74d098c66543d0731de62eb747ccd5b636a6f4c upstream.

Alan pointed out a race in the code where hvc_remove is invoked. The
recent virtio_console work is the first user of hvc_remove().

Alan describes it thus:

The hvc_console assumes that a close and remove call can't occur at the
same time.

In addition tty_hangup(tty) is problematic as tty_hangup is asynchronous
itself....

So this can happen

        hvc_close                               hvc_remove
        hung up ? - no
                                                lock
                                                tty = hp->tty
                                                unlock
        lock
        hp->tty = NULL
        unlock
        notify del
        kref_put the hvc struct
        close completes
        tty is destroyed
                                                tty_hangup dead tty
                                                tty->ops will be NULL
                                                NULL->...

This patch adds some tty krefs and also converts to using tty_vhangup().

Reported-by: Alan Cox <alan@lxorguk.ukuu.org.uk>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
CC: Alan Cox <alan@lxorguk.ukuu.org.uk>
CC: linuxppc-dev@ozlabs.org
CC: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Signed-off-by: Willy Tarreau <w@1wt.eu>

---
 drivers/char/hvc_console.c |   31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

Index: longterm-2.6.27/drivers/char/hvc_console.c
===================================================================
--- longterm-2.6.27.orig/drivers/char/hvc_console.c	2011-01-23 10:52:15.000000000 +0100
+++ longterm-2.6.27/drivers/char/hvc_console.c	2011-01-29 15:33:05.169065818 +0100
@@ -312,6 +312,7 @@
 	spin_lock_irqsave(&hp->lock, flags);
 	/* Check and then increment for fast path open. */
 	if (hp->count++ > 0) {
+		tty_kref_get(tty);
 		spin_unlock_irqrestore(&hp->lock, flags);
 		hvc_kick();
 		return 0;
@@ -320,7 +321,7 @@
 	tty->driver_data = hp;
 	tty->low_latency = 1; /* Makes flushes to ldisc synchronous. */
 
-	hp->tty = tty;
+	hp->tty = tty_kref_get(tty);
 
 	spin_unlock_irqrestore(&hp->lock, flags);
 
@@ -337,6 +338,7 @@
 		spin_lock_irqsave(&hp->lock, flags);
 		hp->tty = NULL;
 		spin_unlock_irqrestore(&hp->lock, flags);
+		tty_kref_put(tty);
 		tty->driver_data = NULL;
 		kref_put(&hp->kref, destroy_hvc_struct);
 		printk(KERN_ERR "hvc_open: request_irq failed with rc %d.\n", rc);
@@ -364,13 +366,18 @@
 		return;
 
 	hp = tty->driver_data;
+
 	spin_lock_irqsave(&hp->lock, flags);
+	tty_kref_get(tty);
 
 	if (--hp->count == 0) {
 		/* We are done with the tty pointer now. */
 		hp->tty = NULL;
 		spin_unlock_irqrestore(&hp->lock, flags);
 
+		/* Put the ref obtained in hvc_open() */
+		tty_kref_put(tty);
+
 		if (hp->ops->notifier_del)
 			hp->ops->notifier_del(hp, hp->data);
 
@@ -387,6 +394,7 @@
 		spin_unlock_irqrestore(&hp->lock, flags);
 	}
 
+	tty_kref_put(tty);
 	kref_put(&hp->kref, destroy_hvc_struct);
 }
 
@@ -423,6 +431,7 @@
 
 	while(temp_open_count) {
 		--temp_open_count;
+		tty_kref_put(tty);
 		kref_put(&hp->kref, destroy_hvc_struct);
 	}
 }
@@ -550,7 +559,7 @@
 		poll_mask |= HVC_POLL_WRITE;
 
 	/* No tty attached, just skip */
-	tty = hp->tty;
+	tty = tty_kref_get(hp->tty);
 	if (tty == NULL)
 		goto bail;
 
@@ -627,6 +636,8 @@
 
 		tty_flip_buffer_push(tty);
 	}
+	if (tty)
+		tty_kref_put(tty);
 
 	return poll_mask;
 }
@@ -749,7 +760,7 @@
 	struct tty_struct *tty;
 
 	spin_lock_irqsave(&hp->lock, flags);
-	tty = hp->tty;
+	tty = tty_kref_get(hp->tty);
 
 	if (hp->index < MAX_NR_HVC_CONSOLES)
 		vtermnos[hp->index] = -1;
@@ -761,18 +772,18 @@
 	/*
 	 * We 'put' the instance that was grabbed when the kref instance
 	 * was initialized using kref_init().  Let the last holder of this
-	 * kref cause it to be removed, which will probably be the tty_hangup
+	 * kref cause it to be removed, which will probably be the tty_vhangup
 	 * below.
 	 */
 	kref_put(&hp->kref, destroy_hvc_struct);
 
 	/*
-	 * This function call will auto chain call hvc_hangup.  The tty should
-	 * always be valid at this time unless a simultaneous tty close already
-	 * cleaned up the hvc_struct.
+	 * This function call will auto chain call hvc_hangup.
 	 */
-	if (tty)
-		tty_hangup(tty);
+	if (tty) {
+		tty_vhangup(tty);
+		tty_kref_put(tty);
+	}
 	return 0;
 }
 

^ permalink raw reply

* Re: [PATCH 3/4]Define memory_block_size_bytes for powerpc/pseries
From: Benjamin Herrenschmidt @ 2011-02-06 23:39 UTC (permalink / raw)
  To: Nathan Fontenot
  Cc: Greg KH, linux-kernel, linux-mm, Robin Holt, linuxppc-dev,
	KAMEZAWA Hiroyuki
In-Reply-To: <4D3866A0.6010803@austin.ibm.com>

On Thu, 2011-01-20 at 10:45 -0600, Nathan Fontenot wrote:
> Define a version of memory_block_size_bytes() for powerpc/pseries such that
> a memory block spans an entire lmb.
> 
> Signed-off-by: Nathan Fontenot <nfont@austin.ibm.com>
> Reviewed-by: Robin Holt <holt@sgi.com>

Hi Nathan !

Is somebody from -mm picking the rest of the series ? This patch as well
or shall I wait for the first two to go in and then pick that one in
-powerpc ?

Cheers,
Ben.

> ---
>  arch/powerpc/platforms/pseries/hotplug-memory.c |   66 +++++++++++++++++++-----
>  1 file changed, 53 insertions(+), 13 deletions(-)
> 
> Index: linux-2.6/arch/powerpc/platforms/pseries/hotplug-memory.c
> ===================================================================
> --- linux-2.6.orig/arch/powerpc/platforms/pseries/hotplug-memory.c	2011-01-20 08:18:21.000000000 -0600
> +++ linux-2.6/arch/powerpc/platforms/pseries/hotplug-memory.c	2011-01-20 08:21:07.000000000 -0600
> @@ -17,6 +17,54 @@
>  #include <asm/pSeries_reconfig.h>
>  #include <asm/sparsemem.h>
>  
> +static unsigned long get_memblock_size(void)
> +{
> +	struct device_node *np;
> +	unsigned int memblock_size = 0;
> +
> +	np = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
> +	if (np) {
> +		const unsigned long *size;
> +
> +		size = of_get_property(np, "ibm,lmb-size", NULL);
> +		memblock_size = size ? *size : 0;
> +
> +		of_node_put(np);
> +	} else {
> +		unsigned int memzero_size = 0;
> +		const unsigned int *regs;
> +
> +		np = of_find_node_by_path("/memory@0");
> +		if (np) {
> +			regs = of_get_property(np, "reg", NULL);
> +			memzero_size = regs ? regs[3] : 0;
> +			of_node_put(np);
> +		}
> +
> +		if (memzero_size) {
> +			/* We now know the size of memory@0, use this to find
> +			 * the first memoryblock and get its size.
> +			 */
> +			char buf[64];
> +
> +			sprintf(buf, "/memory@%x", memzero_size);
> +			np = of_find_node_by_path(buf);
> +			if (np) {
> +				regs = of_get_property(np, "reg", NULL);
> +				memblock_size = regs ? regs[3] : 0;
> +				of_node_put(np);
> +			}
> +		}
> +	}
> +
> +	return memblock_size;
> +}
> +
> +unsigned long memory_block_size_bytes(void)
> +{
> +	return get_memblock_size();
> +}
> +
>  static int pseries_remove_memblock(unsigned long base, unsigned int memblock_size)
>  {
>  	unsigned long start, start_pfn;
> @@ -127,30 +175,22 @@ static int pseries_add_memory(struct dev
>  
>  static int pseries_drconf_memory(unsigned long *base, unsigned int action)
>  {
> -	struct device_node *np;
> -	const unsigned long *lmb_size;
> +	unsigned long memblock_size;
>  	int rc;
>  
> -	np = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
> -	if (!np)
> +	memblock_size = get_memblock_size();
> +	if (!memblock_size)
>  		return -EINVAL;
>  
> -	lmb_size = of_get_property(np, "ibm,lmb-size", NULL);
> -	if (!lmb_size) {
> -		of_node_put(np);
> -		return -EINVAL;
> -	}
> -
>  	if (action == PSERIES_DRCONF_MEM_ADD) {
> -		rc = memblock_add(*base, *lmb_size);
> +		rc = memblock_add(*base, memblock_size);
>  		rc = (rc < 0) ? -EINVAL : 0;
>  	} else if (action == PSERIES_DRCONF_MEM_REMOVE) {
> -		rc = pseries_remove_memblock(*base, *lmb_size);
> +		rc = pseries_remove_memblock(*base, memblock_size);
>  	} else {
>  		rc = -EINVAL;
>  	}
>  
> -	of_node_put(np);
>  	return rc;
>  }
>  
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: [PATCH v3 1/4] powerpc: Removing support for 'protected-sources'
From: Benjamin Herrenschmidt @ 2011-02-06 23:35 UTC (permalink / raw)
  To: Meador Inge; +Cc: Hollis Blanchard, devicetree-discuss, linuxppc-dev
In-Reply-To: <1296861941-3370-2-git-send-email-meador_inge@mentor.com>

On Fri, 2011-02-04 at 17:25 -0600, Meador Inge wrote:
> In a recent thread [1,2,3] concerning device trees for AMP systems, the
> question of whether we really need 'protected-sources' arose.  The general
> consensus was that a new boolean property 'pic-no-reset' (described in more
> detail in a following patch) could be expanded to cover the use cases that
> 'protected-sources' was covering.
> 
> One concern that was raised was for legacy systems which already use the
> 'protected-sources' property [4].  For legacy use cases, 'protected-sources'
> will be treated as an alias of 'pic-no-reset'.  The sources
> encoded in the 'protected-sources' cells, however, will not be processed.  This
> legacy check is added in a later patch in the series.

I'm a bit annoyed here. Why do we need to do that ? Those Cell machines
are going to be real bastards to find and test with, and I don't really
see the point...

The reason for not resetting the MPIC wasn't -only- about the protected
sources, but also because, from memory, some MPIC implementations had
issues when toggling the reset lines (pseries I think is one).

That's actually why the MPIC_WANTS_RESET flag is working the other way
around, only platforms that actually want the reset set it.

This is orthogonal to the need to touch or not touch the interrupt
sources as set by firmware. Yes, having protected sources probably
implies no-reset but the reverse isn't necessarily true.

Ben.
 
> [1] http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-January/004038.html
> [2] http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-January/003991.html
> [3] http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-January/004043.html
> [4] http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-February/004254.html
> 
> Signed-off-by: Meador Inge <meador_inge@mentor.com>
> Cc: Hollis Blanchard <hollis_blanchard@mentor.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>  arch/powerpc/include/asm/mpic.h |    3 ---
>  arch/powerpc/sysdev/mpic.c      |   38 --------------------------------------
>  2 files changed, 0 insertions(+), 41 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/mpic.h b/arch/powerpc/include/asm/mpic.h
> index e000cce..9b94f18 100644
> --- a/arch/powerpc/include/asm/mpic.h
> +++ b/arch/powerpc/include/asm/mpic.h
> @@ -301,9 +301,6 @@ struct mpic
>  	struct mpic_reg_bank	cpuregs[MPIC_MAX_CPUS];
>  	struct mpic_reg_bank	isus[MPIC_MAX_ISU];
>  
> -	/* Protected sources */
> -	unsigned long		*protected;
> -
>  #ifdef CONFIG_MPIC_WEIRD
>  	/* Pointer to HW info array */
>  	u32			*hw_set;
> diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> index 7c13426..a98f41d 100644
> --- a/arch/powerpc/sysdev/mpic.c
> +++ b/arch/powerpc/sysdev/mpic.c
> @@ -947,8 +947,6 @@ static int mpic_host_map(struct irq_host *h, unsigned int virq,
>  
>  	if (hw == mpic->spurious_vec)
>  		return -EINVAL;
> -	if (mpic->protected && test_bit(hw, mpic->protected))
> -		return -EINVAL;
>  
>  #ifdef CONFIG_SMP
>  	else if (hw >= mpic->ipi_vecs[0]) {
> @@ -1095,26 +1093,6 @@ struct mpic * __init mpic_alloc(struct device_node *node,
>  	if (node && of_get_property(node, "big-endian", NULL) != NULL)
>  		mpic->flags |= MPIC_BIG_ENDIAN;
>  
> -	/* Look for protected sources */
> -	if (node) {
> -		int psize;
> -		unsigned int bits, mapsize;
> -		const u32 *psrc =
> -			of_get_property(node, "protected-sources", &psize);
> -		if (psrc) {
> -			psize /= 4;
> -			bits = intvec_top + 1;
> -			mapsize = BITS_TO_LONGS(bits) * sizeof(unsigned long);
> -			mpic->protected = kzalloc(mapsize, GFP_KERNEL);
> -			BUG_ON(mpic->protected == NULL);
> -			for (i = 0; i < psize; i++) {
> -				if (psrc[i] > intvec_top)
> -					continue;
> -				__set_bit(psrc[i], mpic->protected);
> -			}
> -		}
> -	}
> -
>  #ifdef CONFIG_MPIC_WEIRD
>  	mpic->hw_set = mpic_infos[MPIC_GET_REGSET(flags)];
>  #endif
> @@ -1321,9 +1299,6 @@ void __init mpic_init(struct mpic *mpic)
>  		u32 vecpri = MPIC_VECPRI_MASK | i |
>  			(8 << MPIC_VECPRI_PRIORITY_SHIFT);
>  		
> -		/* check if protected */
> -		if (mpic->protected && test_bit(i, mpic->protected))
> -			continue;
>  		/* init hw */
>  		mpic_irq_write(i, MPIC_INFO(IRQ_VECTOR_PRI), vecpri);
>  		mpic_irq_write(i, MPIC_INFO(IRQ_DESTINATION), 1 << cpu);
> @@ -1492,13 +1467,6 @@ static unsigned int _mpic_get_one_irq(struct mpic *mpic, int reg)
>  			mpic_eoi(mpic);
>  		return NO_IRQ;
>  	}
> -	if (unlikely(mpic->protected && test_bit(src, mpic->protected))) {
> -		if (printk_ratelimit())
> -			printk(KERN_WARNING "%s: Got protected source %d !\n",
> -			       mpic->name, (int)src);
> -		mpic_eoi(mpic);
> -		return NO_IRQ;
> -	}
>  
>  	return irq_linear_revmap(mpic->irqhost, src);
>  }
> @@ -1532,12 +1500,6 @@ unsigned int mpic_get_coreint_irq(void)
>  			mpic_eoi(mpic);
>  		return NO_IRQ;
>  	}
> -	if (unlikely(mpic->protected && test_bit(src, mpic->protected))) {
> -		if (printk_ratelimit())
> -			printk(KERN_WARNING "%s: Got protected source %d !\n",
> -			       mpic->name, (int)src);
> -		return NO_IRQ;
> -	}
>  
>  	return irq_linear_revmap(mpic->irqhost, src);
>  #else

^ permalink raw reply

* Re: [PATCH] sched: provide scheduler_ipi() callback in response to smp_send_reschedule()
From: Benjamin Herrenschmidt @ 2011-02-06 23:26 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: linux-m32r-ja, linux-mips, linux-ia64, linux-sh, H. Peter Anvin,
	Heiko Carstens, Paul Mackerras, Helge Deller, sparclinux,
	Linux-Arch, linux-s390, Jesper Nilsson, Jeremy Fitzhardinge,
	Russell King, Hirokazu Takata, x86, James E.J. Bottomley,
	virtualization, Ingo Molnar, Matt Turner, Fenghua Yu,
	Mike Frysinger, user-mode-linux-devel, Konrad Rzeszutek Wilk,
	Jeff Dike, Chris Metcalf, xen-devel, Mikael Starvik, linux-m32r,
	Ivan Kokshaysky, user-mode-linux-user, uclinux-dist-devel,
	Thomas Gleixner, linux-arm-kernel, Richard Henderson, Tony Luck,
	linux-parisc, linux-cris-kernel, linux-am33-list, linux-kernel,
	Ralf Baechle, Kyle McMartin, Paul Mundt, linux-alpha,
	Martin Schwidefsky, linux390, Koichi Yasutake, linuxppc-dev,
	David S. Miller
In-Reply-To: <1295262433.30950.53.camel@laptop>

On Mon, 2011-01-17 at 12:07 +0100, Peter Zijlstra wrote:
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 68034bb..7ee0fc3 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -128,6 +128,7 @@ static irqreturn_t call_function_action(int irq, void *data)
>  static irqreturn_t reschedule_action(int irq, void *data)
>  {
>         /* we just need the return path side effect of checking need_resched */
> +       scheduler_ipi();
>         return IRQ_HANDLED;
>  }
>   

You missed:

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 9813605..467d122 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -98,6 +98,7 @@ void smp_message_recv(int msg)
                break;
        case PPC_MSG_RESCHEDULE:
                /* we notice need_resched on exit */
+               scheduler_ipi();
                break;
        case PPC_MSG_CALL_FUNC_SINGLE:
                generic_smp_call_function_single_interrupt();

Fold that in and add:

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

(We have two variants of the IPIs)

Cheers,
Ben.

^ permalink raw reply related

* Re: [PATCH -mm 2/6] powerpc: convert little-endian bitops macros to static inline functions
From: Benjamin Herrenschmidt @ 2011-02-06 23:14 UTC (permalink / raw)
  To: Akinobu Mita; +Cc: linux-arch, linuxppc-dev, akpm, Paul Mackerras, linux-kernel
In-Reply-To: <1296136583-13815-3-git-send-email-akinobu.mita@gmail.com>

On Thu, 2011-01-27 at 22:56 +0900, Akinobu Mita wrote:
> (This patch is intended to be folded into the patch in -mm:
> powerpc-introduce-little-endian-bitops.patch)
> 
> The little-endian bitops on powerpc are written as preprocessor
> macros with the cast to "unsigned long *".
> This means that even non-pointers will be accepted without an error, and
> that is a Very Bad Thing.
> 
> This converts the little-endian bitops macros to static inline functions
> with proper prototypes.

No objection to the powerpc variant of the patches. What is the status
with the wholes series tho ? Does it looks like its going to be
accepted ? Do you expect my Ack and will merge the whole thing at once ?
Does it break bisection unless it's merged as one single giant patch ?

Cheers,
Ben.

^ permalink raw reply

* [PATCH] powerpc: Fix call to flush_ptrace_hw_breakpoint()
From: Benjamin Herrenschmidt @ 2011-02-06 22:54 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: K.Prasad

A typo in the #ifdef statement makes us never call it
in flush_thread()

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

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 7a1d5cb..c4e4eab 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -670,11 +670,11 @@ void flush_thread(void)
 {
 	discard_lazy_cpu_state();
 
-#ifdef CONFIG_HAVE_HW_BREAKPOINTS
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
 	flush_ptrace_hw_breakpoint(current);
-#else /* CONFIG_HAVE_HW_BREAKPOINTS */
+#else /* CONFIG_HAVE_HW_BREAKPOINT */
 	set_debug_reg_defaults(&current->thread);
-#endif /* CONFIG_HAVE_HW_BREAKPOINTS */
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
 }
 
 void

^ permalink raw reply related

* Re: [PATCH] powerpc/pci: Make both ppc32 and ppc64 use sysdata for pci_controller
From: Grant Likely @ 2011-02-06  4:44 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <AANLkTinrn0rCRD2=wW59oVBPgDjzX2Qe8rJZm6MURMSZ@mail.gmail.com>

Bah, my android phone sent this with html parts and lkml rejected it.
resending...

On Feb 5, 2011 4:43 PM, "Benjamin Herrenschmidt"
<benh@kernel.crashing.org> wrote:
>
> On Fri, 2011-02-04 at 23:22 -0700, Grant Likely wrote:
> > Currently, ppc32 uses sysdata for the pci_controller pointer, and
> > ppc64 uses it to hold the device_node pointer. =A0This patch moves the
> > of_node pointer into (struct pci_bus*)->dev.of_node and
> > (struct pci_dev*)->dev.of_node so that sysdata can be converted to alwa=
ys
> > use the pci_controller pointer instead. =A0It also fixes up the
> > allocating of pci devices so that the of_node pointer gets assigned
> > consistently and increments the ref count.
> >
> > Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> > ---
> >
> > Hi Ben,
> >
> > Please take a look at this. =A0It is based on Sebastian's patch
> > to move some of the OF pci code into driver/of, so it probably doesn't
> > apply cleanly, but it is in my devicetree/next branch if you want to
> > give it a spin. =A0With this patch I'll be able to clean up a lot of th=
e
> > differences between ppc32 and ppc64 pci support.
>
> So the ppc64 "aux" data (EEH stuff etc...) is still referenced from the
> device-node ? That's a bit of a problem as I will need to eventually
> support that stuff even for devices that don't have a node in the
> device-tree.

As far as this patch goes it is, but it doesn't have to stay that way.
 Now that sysdata is used consistently, it would be quite easy to
switch it over to a new structure and redirect the pci_controller
reference.

>
> I was thinking about using the sysdata to point to such an aux structure
> to represent what we call "partitionable endpoints" in ppc64 land, which
> isn't exactly PHBs (but sub divisions of a PHB based on filtering on
> bus/dev/fn, for iommu etc...). Unless we can have another platform
> specific pointer in pci_dev I can use for that purpose ?

I of course cannot answer the additional fields in pci_dev question,
but I have no problem with either approach.

g.

>
> Ben.
>
> > g.
> >
> >
> > =A0arch/microblaze/pci/pci_32.c =A0 =A0 =A0 =A0 =A0 =A0 =A0 | =A0 =A01 =
+
> > =A0arch/powerpc/include/asm/pci-bridge.h =A0 =A0 =A0| =A0 27 +++++-----=
-----------------
> > =A0arch/powerpc/include/asm/pci.h =A0 =A0 =A0 =A0 =A0 =A0 | =A0 =A02 +-
> > =A0arch/powerpc/kernel/of_platform.c =A0 =A0 =A0 =A0 =A0| =A0 =A02 +-
> > =A0arch/powerpc/kernel/pci-common.c =A0 =A0 =A0 =A0 =A0 | =A0 11 +++---=
-----
> > =A0arch/powerpc/kernel/pci_32.c =A0 =A0 =A0 =A0 =A0 =A0 =A0 | =A0 =A02 =
+-
> > =A0arch/powerpc/kernel/pci_64.c =A0 =A0 =A0 =A0 =A0 =A0 =A0 | =A0 =A06 =
+++---
> > =A0arch/powerpc/kernel/pci_dn.c =A0 =A0 =A0 =A0 =A0 =A0 =A0 | =A0 =A09 =
++++++---
> > =A0arch/powerpc/kernel/pci_of_scan.c =A0 =A0 =A0 =A0 =A0| =A0 =A04 ++--
> > =A0arch/powerpc/platforms/pseries/pci_dlpar.c | =A0 =A02 +-
> > =A010 files changed, 24 insertions(+), 42 deletions(-)
> >
> > diff --git a/arch/microblaze/pci/pci_32.c b/arch/microblaze/pci/pci_32.=
c
> > index 3c3d808..92728a6 100644
> > --- a/arch/microblaze/pci/pci_32.c
> > +++ b/arch/microblaze/pci/pci_32.c
> > @@ -332,6 +332,7 @@ static void __devinit pcibios_scan_phb(struct pci_c=
ontroller *hose)
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0hose->global_number);
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 return;
> > =A0 =A0 =A0 }
> > + =A0 =A0 bus.dev->of_node =3D of_node_get(node);
> > =A0 =A0 =A0 bus->secondary =3D hose->first_busno;
> > =A0 =A0 =A0 hose->bus =3D bus;
> >
> > diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/inclu=
de/asm/pci-bridge.h
> > index edeb80f..5e156e0 100644
> > --- a/arch/powerpc/include/asm/pci-bridge.h
> > +++ b/arch/powerpc/include/asm/pci-bridge.h
> > @@ -164,13 +164,13 @@ extern void setup_indirect_pci(struct pci_control=
ler* hose,
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0resource_siz=
e_t cfg_addr,
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0resource_siz=
e_t cfg_data, u32 flags);
> >
> > -#ifndef CONFIG_PPC64
> > -
> > =A0static inline struct pci_controller *pci_bus_to_host(const struct pc=
i_bus *bus)
> > =A0{
> > =A0 =A0 =A0 return bus->sysdata;
> > =A0}
> >
> > +#ifndef CONFIG_PPC64
> > +
> > =A0static inline struct device_node *pci_bus_to_OF_node(struct pci_bus =
*bus)
> > =A0{
> > =A0 =A0 =A0 struct pci_controller *host;
> > @@ -228,19 +228,10 @@ extern void * update_dn_pci_info(struct device_no=
de *dn, void *data);
> >
> > =A0/* Get a device_node from a pci_dev. =A0This code must be fast excep=
t
> > =A0 * in the case where the sysdata is incorrect and needs to be fixed
> > - * up (this will only happen once).
> > - * In this case the sysdata will have been inherited from a PCI host
> > - * bridge or a PCI-PCI bridge further up the tree, so it will point
> > - * to a valid struct pci_dn, just not the one we want.
> > - */
> > + * up (this will only happen once). */
> > =A0static inline struct device_node *pci_device_to_OF_node(struct pci_d=
ev *dev)
> > =A0{
> > - =A0 =A0 struct device_node *dn =3D dev->sysdata;
> > - =A0 =A0 struct pci_dn *pdn =3D dn->data;
> > -
> > - =A0 =A0 if (pdn && pdn->devfn =3D=3D dev->devfn && pdn->busno =3D=3D =
dev->bus->number)
> > - =A0 =A0 =A0 =A0 =A0 =A0 return dn; =A0 =A0 =A0/* fast path. =A0sysdat=
a is good */
> > - =A0 =A0 return fetch_dev_dn(dev);
> > + =A0 =A0 return dev->dev.of_node ? dev->dev.of_node : fetch_dev_dn(dev=
);
> > =A0}
> >
> > =A0static inline int pci_device_from_OF_node(struct device_node *np,
> > @@ -258,7 +249,7 @@ static inline struct device_node *pci_bus_to_OF_nod=
e(struct pci_bus *bus)
> > =A0 =A0 =A0 if (bus->self)
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 return pci_device_to_OF_node(bus->self);
> > =A0 =A0 =A0 else
> > - =A0 =A0 =A0 =A0 =A0 =A0 return bus->sysdata; /* Must be root bus (PHB=
) */
> > + =A0 =A0 =A0 =A0 =A0 =A0 return bus->dev.of_node; /* Must be root bus =
(PHB) */
> > =A0}
> >
> > =A0/** Find the bus corresponding to the indicated device node */
> > @@ -270,14 +261,6 @@ extern void pcibios_remove_pci_devices(struct pci_=
bus *bus);
> > =A0/** Discover new pci devices under this bus, and add them */
> > =A0extern void pcibios_add_pci_devices(struct pci_bus *bus);
> >
> > -static inline struct pci_controller *pci_bus_to_host(const struct pci_=
bus *bus)
> > -{
> > - =A0 =A0 struct device_node *busdn =3D bus->sysdata;
> > -
> > - =A0 =A0 BUG_ON(busdn =3D=3D NULL);
> > - =A0 =A0 return PCI_DN(busdn)->phb;
> > -}
> > -
> >
> > =A0extern void isa_bridge_find_early(struct pci_controller *hose);
> >
> > diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/=
pci.h
> > index a20a9ad..7d77909 100644
> > --- a/arch/powerpc/include/asm/pci.h
> > +++ b/arch/powerpc/include/asm/pci.h
> > @@ -201,7 +201,7 @@ extern void pci_resource_to_user(const struct pci_d=
ev *dev, int bar,
> > =A0extern void pcibios_setup_bus_devices(struct pci_bus *bus);
> > =A0extern void pcibios_setup_bus_self(struct pci_bus *bus);
> > =A0extern void pcibios_setup_phb_io_space(struct pci_controller *hose);
> > -extern void pcibios_scan_phb(struct pci_controller *hose, void *sysdat=
a);
> > +extern void pcibios_scan_phb(struct pci_controller *hose);
> >
> > =A0#endif =A0 =A0 =A0 /* __KERNEL__ */
> > =A0#endif /* __ASM_POWERPC_PCI_H */
> > diff --git a/arch/powerpc/kernel/of_platform.c b/arch/powerpc/kernel/of=
_platform.c
> > index b2c363e..9bd951c 100644
> > --- a/arch/powerpc/kernel/of_platform.c
> > +++ b/arch/powerpc/kernel/of_platform.c
> > @@ -74,7 +74,7 @@ static int __devinit of_pci_phb_probe(struct platform=
_device *dev,
> > =A0#endif /* CONFIG_EEH */
> >
> > =A0 =A0 =A0 /* Scan the bus */
> > - =A0 =A0 pcibios_scan_phb(phb, dev->dev.of_node);
> > + =A0 =A0 pcibios_scan_phb(phb);
> > =A0 =A0 =A0 if (phb->bus =3D=3D NULL)
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -ENXIO;
> >
> > diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci=
-common.c
> > index eb341be..3cd85fa 100644
> > --- a/arch/powerpc/kernel/pci-common.c
> > +++ b/arch/powerpc/kernel/pci-common.c
> > @@ -1688,13 +1688,8 @@ int early_find_capability(struct pci_controller =
*hose, int bus, int devfn,
> > =A0/**
> > =A0 * pci_scan_phb - Given a pci_controller, setup and scan the PCI bus
> > =A0 * @hose: Pointer to the PCI host controller instance structure
> > - * @sysdata: value to use for sysdata pointer. =A0ppc32 and ppc64 diff=
er here
> > - *
> > - * Note: the 'data' pointer is a temporary measure. =A0As 32 and 64 bi=
t
> > - * pci code gets merged, this parameter should become unnecessary beca=
use
> > - * both will use the same value.
> > =A0 */
> > -void __devinit pcibios_scan_phb(struct pci_controller *hose, void *sys=
data)
> > +void __devinit pcibios_scan_phb(struct pci_controller *hose)
> > =A0{
> > =A0 =A0 =A0 struct pci_bus *bus;
> > =A0 =A0 =A0 struct device_node *node =3D hose->dn;
> > @@ -1704,13 +1699,13 @@ void __devinit pcibios_scan_phb(struct pci_cont=
roller *hose, void *sysdata)
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0node ? node->full_name : "<NO NAME>");
> >
> > =A0 =A0 =A0 /* Create an empty bus for the toplevel */
> > - =A0 =A0 bus =3D pci_create_bus(hose->parent, hose->first_busno, hose-=
>ops,
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0sysdata);
> > + =A0 =A0 bus =3D pci_create_bus(hose->parent, hose->first_busno, hose-=
>ops, hose);
> > =A0 =A0 =A0 if (bus =3D=3D NULL) {
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 pr_err("Failed to create bus for PCI domain=
 %04x\n",
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 hose->global_number);
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 return;
> > =A0 =A0 =A0 }
> > + =A0 =A0 bus->dev.of_node =3D of_node_get(node);
> > =A0 =A0 =A0 bus->secondary =3D hose->first_busno;
> > =A0 =A0 =A0 hose->bus =3D bus;
> >
> > diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.=
c
> > index e7db5b4..bedb370 100644
> > --- a/arch/powerpc/kernel/pci_32.c
> > +++ b/arch/powerpc/kernel/pci_32.c
> > @@ -381,7 +381,7 @@ static int __init pcibios_init(void)
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (pci_assign_all_buses)
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 hose->first_busno =3D next_=
busno;
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 hose->last_busno =3D 0xff;
> > - =A0 =A0 =A0 =A0 =A0 =A0 pcibios_scan_phb(hose, hose);
> > + =A0 =A0 =A0 =A0 =A0 =A0 pcibios_scan_phb(hose);
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 pci_bus_add_devices(hose->bus);
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (pci_assign_all_buses || next_busno <=3D=
 hose->last_busno)
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 next_busno =3D hose->last_b=
usno + pcibios_assign_bus_offset;
> > diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.=
c
> > index 8515776..fc6452b 100644
> > --- a/arch/powerpc/kernel/pci_64.c
> > +++ b/arch/powerpc/kernel/pci_64.c
> > @@ -64,7 +64,7 @@ static int __init pcibios_init(void)
> >
> > =A0 =A0 =A0 /* Scan all of the recorded PCI controllers. =A0*/
> > =A0 =A0 =A0 list_for_each_entry_safe(hose, tmp, &hose_list, list_node) =
{
> > - =A0 =A0 =A0 =A0 =A0 =A0 pcibios_scan_phb(hose, hose->dn);
> > + =A0 =A0 =A0 =A0 =A0 =A0 pcibios_scan_phb(hose);
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 pci_bus_add_devices(hose->bus);
> > =A0 =A0 =A0 }
> >
> > @@ -242,10 +242,10 @@ long sys_pciconfig_iobase(long which, unsigned lo=
ng in_bus,
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 bus =3D NULL;
> > =A0 =A0 =A0 }
> > - =A0 =A0 if (bus =3D=3D NULL || bus->sysdata =3D=3D NULL)
> > + =A0 =A0 if (bus =3D=3D NULL || bus->dev.of_node =3D=3D NULL)
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -ENODEV;
> >
> > - =A0 =A0 hose_node =3D (struct device_node *)bus->sysdata;
> > + =A0 =A0 hose_node =3D bus->dev.of_node;
> > =A0 =A0 =A0 hose =3D PCI_DN(hose_node)->phb;
> >
> > =A0 =A0 =A0 switch (which) {
> > diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.=
c
> > index d56b35e..2985268 100644
> > --- a/arch/powerpc/kernel/pci_dn.c
> > +++ b/arch/powerpc/kernel/pci_dn.c
> > @@ -161,7 +161,7 @@ static void *is_devfn_node(struct device_node *dn, =
void *data)
> > =A0/*
> > =A0 * This is the "slow" path for looking up a device_node from a
> > =A0 * pci_dev. =A0It will hunt for the device under its parent's
> > - * phb and then update sysdata for a future fastpath.
> > + * phb and then update of_node pointer.
> > =A0 *
> > =A0 * It may also do fixups on the actual device since this happens
> > =A0 * on the first read/write.
> > @@ -170,16 +170,19 @@ static void *is_devfn_node(struct device_node *dn=
, void *data)
> > =A0 * In this case it may probe for real hardware ("just in case")
> > =A0 * and add a device_node to the device tree if necessary.
> > =A0 *
> > + * Is this function necessary anymore now that dev->dev.of_node is
> > + * used to store the node pointer?
> > + *
> > =A0 */
> > =A0struct device_node *fetch_dev_dn(struct pci_dev *dev)
> > =A0{
> > - =A0 =A0 struct device_node *orig_dn =3D dev->sysdata;
> > + =A0 =A0 struct device_node *orig_dn =3D dev->dev.of_node;
> > =A0 =A0 =A0 struct device_node *dn;
> > =A0 =A0 =A0 unsigned long searchval =3D (dev->bus->number << 8) | dev->=
devfn;
> >
> > =A0 =A0 =A0 dn =3D traverse_pci_devices(orig_dn, is_devfn_node, (void *=
)searchval);
> > =A0 =A0 =A0 if (dn)
> > - =A0 =A0 =A0 =A0 =A0 =A0 dev->sysdata =3D dn;
> > + =A0 =A0 =A0 =A0 =A0 =A0 dev->dev.of_node =3D dn;
> > =A0 =A0 =A0 return dn;
> > =A0}
> > =A0EXPORT_SYMBOL(fetch_dev_dn);
> > diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pc=
i_of_scan.c
> > index e751506..1e89a72 100644
> > --- a/arch/powerpc/kernel/pci_of_scan.c
> > +++ b/arch/powerpc/kernel/pci_of_scan.c
> > @@ -135,7 +135,7 @@ struct pci_dev *of_create_pci_dev(struct device_nod=
e *node,
> > =A0 =A0 =A0 pr_debug(" =A0 =A0create device, devfn: %x, type: %s\n", de=
vfn, type);
> >
> > =A0 =A0 =A0 dev->bus =3D bus;
> > - =A0 =A0 dev->sysdata =3D node;
> > + =A0 =A0 dev->dev.of_node =3D of_node_get(node);
> > =A0 =A0 =A0 dev->dev.parent =3D bus->bridge;
> > =A0 =A0 =A0 dev->dev.bus =3D &pci_bus_type;
> > =A0 =A0 =A0 dev->devfn =3D devfn;
> > @@ -238,7 +238,7 @@ void __devinit of_scan_pci_bridge(struct device_nod=
e *node,
> > =A0 =A0 =A0 bus->primary =3D dev->bus->number;
> > =A0 =A0 =A0 bus->subordinate =3D busrange[1];
> > =A0 =A0 =A0 bus->bridge_ctl =3D 0;
> > - =A0 =A0 bus->sysdata =3D node;
> > + =A0 =A0 bus->dev.of_node =3D of_node_get(node);
> >
> > =A0 =A0 =A0 /* parse ranges property */
> > =A0 =A0 =A0 /* PCI #address-cells =3D=3D 3 and #size-cells =3D=3D 2 alw=
ays */
> > diff --git a/arch/powerpc/platforms/pseries/pci_dlpar.c b/arch/powerpc/=
platforms/pseries/pci_dlpar.c
> > index 5fcc92a..3bf4488 100644
> > --- a/arch/powerpc/platforms/pseries/pci_dlpar.c
> > +++ b/arch/powerpc/platforms/pseries/pci_dlpar.c
> > @@ -149,7 +149,7 @@ struct pci_controller * __devinit init_phb_dynamic(=
struct device_node *dn)
> > =A0 =A0 =A0 if (dn->child)
> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 eeh_add_device_tree_early(dn);
> >
> > - =A0 =A0 pcibios_scan_phb(phb, dn);
> > + =A0 =A0 pcibios_scan_phb(phb);
> > =A0 =A0 =A0 pcibios_finish_adding_to_bus(phb->bus);
> >
> > =A0 =A0 =A0 return phb;
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel"=
 in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at =A0http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at =A0http://www.tux.org/lkml/
>
>



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

^ permalink raw reply

* Re: [PATCH] powerpc/pci: Make both ppc32 and ppc64 use sysdata for pci_controller
From: Grant Likely @ 2011-02-06  2:09 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1296949393.2349.842.camel@pasglop>

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

On Feb 5, 2011 4:43 PM, "Benjamin Herrenschmidt" <benh@kernel.crashing.org>
wrote:
>
> On Fri, 2011-02-04 at 23:22 -0700, Grant Likely wrote:
> > Currently, ppc32 uses sysdata for the pci_controller pointer, and
> > ppc64 uses it to hold the device_node pointer.  This patch moves the
> > of_node pointer into (struct pci_bus*)->dev.of_node and
> > (struct pci_dev*)->dev.of_node so that sysdata can be converted to
always
> > use the pci_controller pointer instead.  It also fixes up the
> > allocating of pci devices so that the of_node pointer gets assigned
> > consistently and increments the ref count.
> >
> > Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> > ---
> >
> > Hi Ben,
> >
> > Please take a look at this.  It is based on Sebastian's patch
> > to move some of the OF pci code into driver/of, so it probably doesn't
> > apply cleanly, but it is in my devicetree/next branch if you want to
> > give it a spin.  With this patch I'll be able to clean up a lot of the
> > differences between ppc32 and ppc64 pci support.
>
> So the ppc64 "aux" data (EEH stuff etc...) is still referenced from the
> device-node ? That's a bit of a problem as I will need to eventually
> support that stuff even for devices that don't have a node in the
> device-tree.

As far as this patch goes it is, but it doesn't have to stay that way.  Now
that sysdata is used consistently, it would be quite easy to switch it over
to a new structure and redirect the pci_controller reference.

>
> I was thinking about using the sysdata to point to such an aux structure
> to represent what we call "partitionable endpoints" in ppc64 land, which
> isn't exactly PHBs (but sub divisions of a PHB based on filtering on
> bus/dev/fn, for iommu etc...). Unless we can have another platform
> specific pointer in pci_dev I can use for that purpose ?

I of course cannot answer the additional fields in pci_dev question, but I
have no problem with either approach.

g.

>
> Ben.
>
> > g.
> >
> >
> >  arch/microblaze/pci/pci_32.c               |    1 +
> >  arch/powerpc/include/asm/pci-bridge.h      |   27
+++++----------------------
> >  arch/powerpc/include/asm/pci.h             |    2 +-
> >  arch/powerpc/kernel/of_platform.c          |    2 +-
> >  arch/powerpc/kernel/pci-common.c           |   11 +++--------
> >  arch/powerpc/kernel/pci_32.c               |    2 +-
> >  arch/powerpc/kernel/pci_64.c               |    6 +++---
> >  arch/powerpc/kernel/pci_dn.c               |    9 ++++++---
> >  arch/powerpc/kernel/pci_of_scan.c          |    4 ++--
> >  arch/powerpc/platforms/pseries/pci_dlpar.c |    2 +-
> >  10 files changed, 24 insertions(+), 42 deletions(-)
> >
> > diff --git a/arch/microblaze/pci/pci_32.c b/arch/microblaze/pci/pci_32.c
> > index 3c3d808..92728a6 100644
> > --- a/arch/microblaze/pci/pci_32.c
> > +++ b/arch/microblaze/pci/pci_32.c
> > @@ -332,6 +332,7 @@ static void __devinit pcibios_scan_phb(struct
pci_controller *hose)
> >                      hose->global_number);
> >               return;
> >       }
> > +     bus.dev->of_node = of_node_get(node);
> >       bus->secondary = hose->first_busno;
> >       hose->bus = bus;
> >
> > diff --git a/arch/powerpc/include/asm/pci-bridge.h
b/arch/powerpc/include/asm/pci-bridge.h
> > index edeb80f..5e156e0 100644
> > --- a/arch/powerpc/include/asm/pci-bridge.h
> > +++ b/arch/powerpc/include/asm/pci-bridge.h
> > @@ -164,13 +164,13 @@ extern void setup_indirect_pci(struct
pci_controller* hose,
> >                              resource_size_t cfg_addr,
> >                              resource_size_t cfg_data, u32 flags);
> >
> > -#ifndef CONFIG_PPC64
> > -
> >  static inline struct pci_controller *pci_bus_to_host(const struct
pci_bus *bus)
> >  {
> >       return bus->sysdata;
> >  }
> >
> > +#ifndef CONFIG_PPC64
> > +
> >  static inline struct device_node *pci_bus_to_OF_node(struct pci_bus
*bus)
> >  {
> >       struct pci_controller *host;
> > @@ -228,19 +228,10 @@ extern void * update_dn_pci_info(struct
device_node *dn, void *data);
> >
> >  /* Get a device_node from a pci_dev.  This code must be fast except
> >   * in the case where the sysdata is incorrect and needs to be fixed
> > - * up (this will only happen once).
> > - * In this case the sysdata will have been inherited from a PCI host
> > - * bridge or a PCI-PCI bridge further up the tree, so it will point
> > - * to a valid struct pci_dn, just not the one we want.
> > - */
> > + * up (this will only happen once). */
> >  static inline struct device_node *pci_device_to_OF_node(struct pci_dev
*dev)
> >  {
> > -     struct device_node *dn = dev->sysdata;
> > -     struct pci_dn *pdn = dn->data;
> > -
> > -     if (pdn && pdn->devfn == dev->devfn && pdn->busno ==
dev->bus->number)
> > -             return dn;      /* fast path.  sysdata is good */
> > -     return fetch_dev_dn(dev);
> > +     return dev->dev.of_node ? dev->dev.of_node : fetch_dev_dn(dev);
> >  }
> >
> >  static inline int pci_device_from_OF_node(struct device_node *np,
> > @@ -258,7 +249,7 @@ static inline struct device_node
*pci_bus_to_OF_node(struct pci_bus *bus)
> >       if (bus->self)
> >               return pci_device_to_OF_node(bus->self);
> >       else
> > -             return bus->sysdata; /* Must be root bus (PHB) */
> > +             return bus->dev.of_node; /* Must be root bus (PHB) */
> >  }
> >
> >  /** Find the bus corresponding to the indicated device node */
> > @@ -270,14 +261,6 @@ extern void pcibios_remove_pci_devices(struct
pci_bus *bus);
> >  /** Discover new pci devices under this bus, and add them */
> >  extern void pcibios_add_pci_devices(struct pci_bus *bus);
> >
> > -static inline struct pci_controller *pci_bus_to_host(const struct
pci_bus *bus)
> > -{
> > -     struct device_node *busdn = bus->sysdata;
> > -
> > -     BUG_ON(busdn == NULL);
> > -     return PCI_DN(busdn)->phb;
> > -}
> > -
> >
> >  extern void isa_bridge_find_early(struct pci_controller *hose);
> >
> > diff --git a/arch/powerpc/include/asm/pci.h
b/arch/powerpc/include/asm/pci.h
> > index a20a9ad..7d77909 100644
> > --- a/arch/powerpc/include/asm/pci.h
> > +++ b/arch/powerpc/include/asm/pci.h
> > @@ -201,7 +201,7 @@ 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
*sysdata);
> > +extern void pcibios_scan_phb(struct pci_controller *hose);
> >
> >  #endif       /* __KERNEL__ */
> >  #endif /* __ASM_POWERPC_PCI_H */
> > diff --git a/arch/powerpc/kernel/of_platform.c
b/arch/powerpc/kernel/of_platform.c
> > index b2c363e..9bd951c 100644
> > --- a/arch/powerpc/kernel/of_platform.c
> > +++ b/arch/powerpc/kernel/of_platform.c
> > @@ -74,7 +74,7 @@ static int __devinit of_pci_phb_probe(struct
platform_device *dev,
> >  #endif /* CONFIG_EEH */
> >
> >       /* Scan the bus */
> > -     pcibios_scan_phb(phb, dev->dev.of_node);
> > +     pcibios_scan_phb(phb);
> >       if (phb->bus == NULL)
> >               return -ENXIO;
> >
> > diff --git a/arch/powerpc/kernel/pci-common.c
b/arch/powerpc/kernel/pci-common.c
> > index eb341be..3cd85fa 100644
> > --- a/arch/powerpc/kernel/pci-common.c
> > +++ b/arch/powerpc/kernel/pci-common.c
> > @@ -1688,13 +1688,8 @@ int early_find_capability(struct pci_controller
*hose, int bus, int devfn,
> >  /**
> >   * pci_scan_phb - Given a pci_controller, setup and scan the PCI bus
> >   * @hose: Pointer to the PCI host controller instance structure
> > - * @sysdata: 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
*sysdata)
> > +void __devinit pcibios_scan_phb(struct pci_controller *hose)
> >  {
> >       struct pci_bus *bus;
> >       struct device_node *node = hose->dn;
> > @@ -1704,13 +1699,13 @@ void __devinit pcibios_scan_phb(struct
pci_controller *hose, void *sysdata)
> >                node ? node->full_name : "<NO NAME>");
> >
> >       /* Create an empty bus for the toplevel */
> > -     bus = pci_create_bus(hose->parent, hose->first_busno, hose->ops,
> > -                          sysdata);
> > +     bus = pci_create_bus(hose->parent, hose->first_busno, hose->ops,
hose);
> >       if (bus == NULL) {
> >               pr_err("Failed to create bus for PCI domain %04x\n",
> >                       hose->global_number);
> >               return;
> >       }
> > +     bus->dev.of_node = of_node_get(node);
> >       bus->secondary = hose->first_busno;
> >       hose->bus = bus;
> >
> > diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
> > index e7db5b4..bedb370 100644
> > --- a/arch/powerpc/kernel/pci_32.c
> > +++ b/arch/powerpc/kernel/pci_32.c
> > @@ -381,7 +381,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, hose);
> > +             pcibios_scan_phb(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 8515776..fc6452b 100644
> > --- a/arch/powerpc/kernel/pci_64.c
> > +++ b/arch/powerpc/kernel/pci_64.c
> > @@ -64,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) {
> > -             pcibios_scan_phb(hose, hose->dn);
> > +             pcibios_scan_phb(hose);
> >               pci_bus_add_devices(hose->bus);
> >       }
> >
> > @@ -242,10 +242,10 @@ long sys_pciconfig_iobase(long which, unsigned
long in_bus,
> >                       break;
> >               bus = NULL;
> >       }
> > -     if (bus == NULL || bus->sysdata == NULL)
> > +     if (bus == NULL || bus->dev.of_node == NULL)
> >               return -ENODEV;
> >
> > -     hose_node = (struct device_node *)bus->sysdata;
> > +     hose_node = bus->dev.of_node;
> >       hose = PCI_DN(hose_node)->phb;
> >
> >       switch (which) {
> > diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
> > index d56b35e..2985268 100644
> > --- a/arch/powerpc/kernel/pci_dn.c
> > +++ b/arch/powerpc/kernel/pci_dn.c
> > @@ -161,7 +161,7 @@ static void *is_devfn_node(struct device_node *dn,
void *data)
> >  /*
> >   * This is the "slow" path for looking up a device_node from a
> >   * pci_dev.  It will hunt for the device under its parent's
> > - * phb and then update sysdata for a future fastpath.
> > + * phb and then update of_node pointer.
> >   *
> >   * It may also do fixups on the actual device since this happens
> >   * on the first read/write.
> > @@ -170,16 +170,19 @@ static void *is_devfn_node(struct device_node *dn,
void *data)
> >   * In this case it may probe for real hardware ("just in case")
> >   * and add a device_node to the device tree if necessary.
> >   *
> > + * Is this function necessary anymore now that dev->dev.of_node is
> > + * used to store the node pointer?
> > + *
> >   */
> >  struct device_node *fetch_dev_dn(struct pci_dev *dev)
> >  {
> > -     struct device_node *orig_dn = dev->sysdata;
> > +     struct device_node *orig_dn = dev->dev.of_node;
> >       struct device_node *dn;
> >       unsigned long searchval = (dev->bus->number << 8) | dev->devfn;
> >
> >       dn = traverse_pci_devices(orig_dn, is_devfn_node, (void
*)searchval);
> >       if (dn)
> > -             dev->sysdata = dn;
> > +             dev->dev.of_node = dn;
> >       return dn;
> >  }
> >  EXPORT_SYMBOL(fetch_dev_dn);
> > diff --git a/arch/powerpc/kernel/pci_of_scan.c
b/arch/powerpc/kernel/pci_of_scan.c
> > index e751506..1e89a72 100644
> > --- a/arch/powerpc/kernel/pci_of_scan.c
> > +++ b/arch/powerpc/kernel/pci_of_scan.c
> > @@ -135,7 +135,7 @@ struct pci_dev *of_create_pci_dev(struct device_node
*node,
> >       pr_debug("    create device, devfn: %x, type: %s\n", devfn, type);
> >
> >       dev->bus = bus;
> > -     dev->sysdata = node;
> > +     dev->dev.of_node = of_node_get(node);
> >       dev->dev.parent = bus->bridge;
> >       dev->dev.bus = &pci_bus_type;
> >       dev->devfn = devfn;
> > @@ -238,7 +238,7 @@ void __devinit of_scan_pci_bridge(struct device_node
*node,
> >       bus->primary = dev->bus->number;
> >       bus->subordinate = busrange[1];
> >       bus->bridge_ctl = 0;
> > -     bus->sysdata = node;
> > +     bus->dev.of_node = of_node_get(node);
> >
> >       /* parse ranges property */
> >       /* PCI #address-cells == 3 and #size-cells == 2 always */
> > diff --git a/arch/powerpc/platforms/pseries/pci_dlpar.c
b/arch/powerpc/platforms/pseries/pci_dlpar.c
> > index 5fcc92a..3bf4488 100644
> > --- a/arch/powerpc/platforms/pseries/pci_dlpar.c
> > +++ b/arch/powerpc/platforms/pseries/pci_dlpar.c
> > @@ -149,7 +149,7 @@ struct pci_controller * __devinit
init_phb_dynamic(struct device_node *dn)
> >       if (dn->child)
> >               eeh_add_device_tree_early(dn);
> >
> > -     pcibios_scan_phb(phb, dn);
> > +     pcibios_scan_phb(phb);
> >       pcibios_finish_adding_to_bus(phb->bus);
> >
> >       return phb;
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel"
in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
>
>

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

^ permalink raw reply

* Re: [PATCH] powerpc/pci: Make both ppc32 and ppc64 use sysdata for pci_controller
From: Benjamin Herrenschmidt @ 2011-02-05 23:43 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20110205062002.32654.18765.stgit@localhost6.localdomain6>

On Fri, 2011-02-04 at 23:22 -0700, Grant Likely wrote:
> Currently, ppc32 uses sysdata for the pci_controller pointer, and
> ppc64 uses it to hold the device_node pointer.  This patch moves the
> of_node pointer into (struct pci_bus*)->dev.of_node and
> (struct pci_dev*)->dev.of_node so that sysdata can be converted to always
> use the pci_controller pointer instead.  It also fixes up the
> allocating of pci devices so that the of_node pointer gets assigned
> consistently and increments the ref count.
> 
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
> 
> Hi Ben,
> 
> Please take a look at this.  It is based on Sebastian's patch
> to move some of the OF pci code into driver/of, so it probably doesn't
> apply cleanly, but it is in my devicetree/next branch if you want to
> give it a spin.  With this patch I'll be able to clean up a lot of the
> differences between ppc32 and ppc64 pci support.

So the ppc64 "aux" data (EEH stuff etc...) is still referenced from the
device-node ? That's a bit of a problem as I will need to eventually
support that stuff even for devices that don't have a node in the
device-tree.

I was thinking about using the sysdata to point to such an aux structure
to represent what we call "partitionable endpoints" in ppc64 land, which
isn't exactly PHBs (but sub divisions of a PHB based on filtering on
bus/dev/fn, for iommu etc...). Unless we can have another platform
specific pointer in pci_dev I can use for that purpose ?

Ben.

> g.
> 
> 
>  arch/microblaze/pci/pci_32.c               |    1 +
>  arch/powerpc/include/asm/pci-bridge.h      |   27 +++++----------------------
>  arch/powerpc/include/asm/pci.h             |    2 +-
>  arch/powerpc/kernel/of_platform.c          |    2 +-
>  arch/powerpc/kernel/pci-common.c           |   11 +++--------
>  arch/powerpc/kernel/pci_32.c               |    2 +-
>  arch/powerpc/kernel/pci_64.c               |    6 +++---
>  arch/powerpc/kernel/pci_dn.c               |    9 ++++++---
>  arch/powerpc/kernel/pci_of_scan.c          |    4 ++--
>  arch/powerpc/platforms/pseries/pci_dlpar.c |    2 +-
>  10 files changed, 24 insertions(+), 42 deletions(-)
> 
> diff --git a/arch/microblaze/pci/pci_32.c b/arch/microblaze/pci/pci_32.c
> index 3c3d808..92728a6 100644
> --- a/arch/microblaze/pci/pci_32.c
> +++ b/arch/microblaze/pci/pci_32.c
> @@ -332,6 +332,7 @@ static void __devinit pcibios_scan_phb(struct pci_controller *hose)
>  		       hose->global_number);
>  		return;
>  	}
> +	bus.dev->of_node = of_node_get(node);
>  	bus->secondary = hose->first_busno;
>  	hose->bus = bus;
>  
> diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
> index edeb80f..5e156e0 100644
> --- a/arch/powerpc/include/asm/pci-bridge.h
> +++ b/arch/powerpc/include/asm/pci-bridge.h
> @@ -164,13 +164,13 @@ extern void setup_indirect_pci(struct pci_controller* hose,
>  			       resource_size_t cfg_addr,
>  			       resource_size_t cfg_data, u32 flags);
>  
> -#ifndef CONFIG_PPC64
> -
>  static inline struct pci_controller *pci_bus_to_host(const struct pci_bus *bus)
>  {
>  	return bus->sysdata;
>  }
>  
> +#ifndef CONFIG_PPC64
> +
>  static inline struct device_node *pci_bus_to_OF_node(struct pci_bus *bus)
>  {
>  	struct pci_controller *host;
> @@ -228,19 +228,10 @@ extern void * update_dn_pci_info(struct device_node *dn, void *data);
>  
>  /* Get a device_node from a pci_dev.  This code must be fast except
>   * in the case where the sysdata is incorrect and needs to be fixed
> - * up (this will only happen once).
> - * In this case the sysdata will have been inherited from a PCI host
> - * bridge or a PCI-PCI bridge further up the tree, so it will point
> - * to a valid struct pci_dn, just not the one we want.
> - */
> + * up (this will only happen once). */
>  static inline struct device_node *pci_device_to_OF_node(struct pci_dev *dev)
>  {
> -	struct device_node *dn = dev->sysdata;
> -	struct pci_dn *pdn = dn->data;
> -
> -	if (pdn && pdn->devfn == dev->devfn && pdn->busno == dev->bus->number)
> -		return dn;	/* fast path.  sysdata is good */
> -	return fetch_dev_dn(dev);
> +	return dev->dev.of_node ? dev->dev.of_node : fetch_dev_dn(dev);
>  }
>  
>  static inline int pci_device_from_OF_node(struct device_node *np,
> @@ -258,7 +249,7 @@ static inline struct device_node *pci_bus_to_OF_node(struct pci_bus *bus)
>  	if (bus->self)
>  		return pci_device_to_OF_node(bus->self);
>  	else
> -		return bus->sysdata; /* Must be root bus (PHB) */
> +		return bus->dev.of_node; /* Must be root bus (PHB) */
>  }
>  
>  /** Find the bus corresponding to the indicated device node */
> @@ -270,14 +261,6 @@ extern void pcibios_remove_pci_devices(struct pci_bus *bus);
>  /** Discover new pci devices under this bus, and add them */
>  extern void pcibios_add_pci_devices(struct pci_bus *bus);
>  
> -static inline struct pci_controller *pci_bus_to_host(const struct pci_bus *bus)
> -{
> -	struct device_node *busdn = bus->sysdata;
> -
> -	BUG_ON(busdn == NULL);
> -	return PCI_DN(busdn)->phb;
> -}
> -
>  
>  extern void isa_bridge_find_early(struct pci_controller *hose);
>  
> diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
> index a20a9ad..7d77909 100644
> --- a/arch/powerpc/include/asm/pci.h
> +++ b/arch/powerpc/include/asm/pci.h
> @@ -201,7 +201,7 @@ 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 *sysdata);
> +extern void pcibios_scan_phb(struct pci_controller *hose);
>  
>  #endif	/* __KERNEL__ */
>  #endif /* __ASM_POWERPC_PCI_H */
> diff --git a/arch/powerpc/kernel/of_platform.c b/arch/powerpc/kernel/of_platform.c
> index b2c363e..9bd951c 100644
> --- a/arch/powerpc/kernel/of_platform.c
> +++ b/arch/powerpc/kernel/of_platform.c
> @@ -74,7 +74,7 @@ static int __devinit of_pci_phb_probe(struct platform_device *dev,
>  #endif /* CONFIG_EEH */
>  
>  	/* Scan the bus */
> -	pcibios_scan_phb(phb, dev->dev.of_node);
> +	pcibios_scan_phb(phb);
>  	if (phb->bus == NULL)
>  		return -ENXIO;
>  
> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index eb341be..3cd85fa 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -1688,13 +1688,8 @@ int early_find_capability(struct pci_controller *hose, int bus, int devfn,
>  /**
>   * pci_scan_phb - Given a pci_controller, setup and scan the PCI bus
>   * @hose: Pointer to the PCI host controller instance structure
> - * @sysdata: 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 *sysdata)
> +void __devinit pcibios_scan_phb(struct pci_controller *hose)
>  {
>  	struct pci_bus *bus;
>  	struct device_node *node = hose->dn;
> @@ -1704,13 +1699,13 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose, void *sysdata)
>  		 node ? node->full_name : "<NO NAME>");
>  
>  	/* Create an empty bus for the toplevel */
> -	bus = pci_create_bus(hose->parent, hose->first_busno, hose->ops,
> -			     sysdata);
> +	bus = pci_create_bus(hose->parent, hose->first_busno, hose->ops, hose);
>  	if (bus == NULL) {
>  		pr_err("Failed to create bus for PCI domain %04x\n",
>  			hose->global_number);
>  		return;
>  	}
> +	bus->dev.of_node = of_node_get(node);
>  	bus->secondary = hose->first_busno;
>  	hose->bus = bus;
>  
> diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
> index e7db5b4..bedb370 100644
> --- a/arch/powerpc/kernel/pci_32.c
> +++ b/arch/powerpc/kernel/pci_32.c
> @@ -381,7 +381,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, hose);
> +		pcibios_scan_phb(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 8515776..fc6452b 100644
> --- a/arch/powerpc/kernel/pci_64.c
> +++ b/arch/powerpc/kernel/pci_64.c
> @@ -64,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) {
> -		pcibios_scan_phb(hose, hose->dn);
> +		pcibios_scan_phb(hose);
>  		pci_bus_add_devices(hose->bus);
>  	}
>  
> @@ -242,10 +242,10 @@ long sys_pciconfig_iobase(long which, unsigned long in_bus,
>  			break;
>  		bus = NULL;
>  	}
> -	if (bus == NULL || bus->sysdata == NULL)
> +	if (bus == NULL || bus->dev.of_node == NULL)
>  		return -ENODEV;
>  
> -	hose_node = (struct device_node *)bus->sysdata;
> +	hose_node = bus->dev.of_node;
>  	hose = PCI_DN(hose_node)->phb;
>  
>  	switch (which) {
> diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
> index d56b35e..2985268 100644
> --- a/arch/powerpc/kernel/pci_dn.c
> +++ b/arch/powerpc/kernel/pci_dn.c
> @@ -161,7 +161,7 @@ static void *is_devfn_node(struct device_node *dn, void *data)
>  /*
>   * This is the "slow" path for looking up a device_node from a
>   * pci_dev.  It will hunt for the device under its parent's
> - * phb and then update sysdata for a future fastpath.
> + * phb and then update of_node pointer.
>   *
>   * It may also do fixups on the actual device since this happens
>   * on the first read/write.
> @@ -170,16 +170,19 @@ static void *is_devfn_node(struct device_node *dn, void *data)
>   * In this case it may probe for real hardware ("just in case")
>   * and add a device_node to the device tree if necessary.
>   *
> + * Is this function necessary anymore now that dev->dev.of_node is
> + * used to store the node pointer?
> + *
>   */
>  struct device_node *fetch_dev_dn(struct pci_dev *dev)
>  {
> -	struct device_node *orig_dn = dev->sysdata;
> +	struct device_node *orig_dn = dev->dev.of_node;
>  	struct device_node *dn;
>  	unsigned long searchval = (dev->bus->number << 8) | dev->devfn;
>  
>  	dn = traverse_pci_devices(orig_dn, is_devfn_node, (void *)searchval);
>  	if (dn)
> -		dev->sysdata = dn;
> +		dev->dev.of_node = dn;
>  	return dn;
>  }
>  EXPORT_SYMBOL(fetch_dev_dn);
> diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c
> index e751506..1e89a72 100644
> --- a/arch/powerpc/kernel/pci_of_scan.c
> +++ b/arch/powerpc/kernel/pci_of_scan.c
> @@ -135,7 +135,7 @@ struct pci_dev *of_create_pci_dev(struct device_node *node,
>  	pr_debug("    create device, devfn: %x, type: %s\n", devfn, type);
>  
>  	dev->bus = bus;
> -	dev->sysdata = node;
> +	dev->dev.of_node = of_node_get(node);
>  	dev->dev.parent = bus->bridge;
>  	dev->dev.bus = &pci_bus_type;
>  	dev->devfn = devfn;
> @@ -238,7 +238,7 @@ void __devinit of_scan_pci_bridge(struct device_node *node,
>  	bus->primary = dev->bus->number;
>  	bus->subordinate = busrange[1];
>  	bus->bridge_ctl = 0;
> -	bus->sysdata = node;
> +	bus->dev.of_node = of_node_get(node);
>  
>  	/* parse ranges property */
>  	/* PCI #address-cells == 3 and #size-cells == 2 always */
> diff --git a/arch/powerpc/platforms/pseries/pci_dlpar.c b/arch/powerpc/platforms/pseries/pci_dlpar.c
> index 5fcc92a..3bf4488 100644
> --- a/arch/powerpc/platforms/pseries/pci_dlpar.c
> +++ b/arch/powerpc/platforms/pseries/pci_dlpar.c
> @@ -149,7 +149,7 @@ struct pci_controller * __devinit init_phb_dynamic(struct device_node *dn)
>  	if (dn->child)
>  		eeh_add_device_tree_early(dn);
>  
> -	pcibios_scan_phb(phb, dn);
> +	pcibios_scan_phb(phb);
>  	pcibios_finish_adding_to_bus(phb->bus);
>  
>  	return phb;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* [PATCH] powerpc/pci: Make both ppc32 and ppc64 use sysdata for pci_controller
From: Grant Likely @ 2011-02-05  6:22 UTC (permalink / raw)
  To: benh, linuxppc-dev, linux-kernel

Currently, ppc32 uses sysdata for the pci_controller pointer, and
ppc64 uses it to hold the device_node pointer.  This patch moves the
of_node pointer into (struct pci_bus*)->dev.of_node and
(struct pci_dev*)->dev.of_node so that sysdata can be converted to always
use the pci_controller pointer instead.  It also fixes up the
allocating of pci devices so that the of_node pointer gets assigned
consistently and increments the ref count.

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

Hi Ben,

Please take a look at this.  It is based on Sebastian's patch
to move some of the OF pci code into driver/of, so it probably doesn't
apply cleanly, but it is in my devicetree/next branch if you want to
give it a spin.  With this patch I'll be able to clean up a lot of the
differences between ppc32 and ppc64 pci support.

g.


 arch/microblaze/pci/pci_32.c               |    1 +
 arch/powerpc/include/asm/pci-bridge.h      |   27 +++++----------------------
 arch/powerpc/include/asm/pci.h             |    2 +-
 arch/powerpc/kernel/of_platform.c          |    2 +-
 arch/powerpc/kernel/pci-common.c           |   11 +++--------
 arch/powerpc/kernel/pci_32.c               |    2 +-
 arch/powerpc/kernel/pci_64.c               |    6 +++---
 arch/powerpc/kernel/pci_dn.c               |    9 ++++++---
 arch/powerpc/kernel/pci_of_scan.c          |    4 ++--
 arch/powerpc/platforms/pseries/pci_dlpar.c |    2 +-
 10 files changed, 24 insertions(+), 42 deletions(-)

diff --git a/arch/microblaze/pci/pci_32.c b/arch/microblaze/pci/pci_32.c
index 3c3d808..92728a6 100644
--- a/arch/microblaze/pci/pci_32.c
+++ b/arch/microblaze/pci/pci_32.c
@@ -332,6 +332,7 @@ static void __devinit pcibios_scan_phb(struct pci_controller *hose)
 		       hose->global_number);
 		return;
 	}
+	bus.dev->of_node = of_node_get(node);
 	bus->secondary = hose->first_busno;
 	hose->bus = bus;
 
diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
index edeb80f..5e156e0 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -164,13 +164,13 @@ extern void setup_indirect_pci(struct pci_controller* hose,
 			       resource_size_t cfg_addr,
 			       resource_size_t cfg_data, u32 flags);
 
-#ifndef CONFIG_PPC64
-
 static inline struct pci_controller *pci_bus_to_host(const struct pci_bus *bus)
 {
 	return bus->sysdata;
 }
 
+#ifndef CONFIG_PPC64
+
 static inline struct device_node *pci_bus_to_OF_node(struct pci_bus *bus)
 {
 	struct pci_controller *host;
@@ -228,19 +228,10 @@ extern void * update_dn_pci_info(struct device_node *dn, void *data);
 
 /* Get a device_node from a pci_dev.  This code must be fast except
  * in the case where the sysdata is incorrect and needs to be fixed
- * up (this will only happen once).
- * In this case the sysdata will have been inherited from a PCI host
- * bridge or a PCI-PCI bridge further up the tree, so it will point
- * to a valid struct pci_dn, just not the one we want.
- */
+ * up (this will only happen once). */
 static inline struct device_node *pci_device_to_OF_node(struct pci_dev *dev)
 {
-	struct device_node *dn = dev->sysdata;
-	struct pci_dn *pdn = dn->data;
-
-	if (pdn && pdn->devfn == dev->devfn && pdn->busno == dev->bus->number)
-		return dn;	/* fast path.  sysdata is good */
-	return fetch_dev_dn(dev);
+	return dev->dev.of_node ? dev->dev.of_node : fetch_dev_dn(dev);
 }
 
 static inline int pci_device_from_OF_node(struct device_node *np,
@@ -258,7 +249,7 @@ static inline struct device_node *pci_bus_to_OF_node(struct pci_bus *bus)
 	if (bus->self)
 		return pci_device_to_OF_node(bus->self);
 	else
-		return bus->sysdata; /* Must be root bus (PHB) */
+		return bus->dev.of_node; /* Must be root bus (PHB) */
 }
 
 /** Find the bus corresponding to the indicated device node */
@@ -270,14 +261,6 @@ extern void pcibios_remove_pci_devices(struct pci_bus *bus);
 /** Discover new pci devices under this bus, and add them */
 extern void pcibios_add_pci_devices(struct pci_bus *bus);
 
-static inline struct pci_controller *pci_bus_to_host(const struct pci_bus *bus)
-{
-	struct device_node *busdn = bus->sysdata;
-
-	BUG_ON(busdn == NULL);
-	return PCI_DN(busdn)->phb;
-}
-
 
 extern void isa_bridge_find_early(struct pci_controller *hose);
 
diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
index a20a9ad..7d77909 100644
--- a/arch/powerpc/include/asm/pci.h
+++ b/arch/powerpc/include/asm/pci.h
@@ -201,7 +201,7 @@ 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 *sysdata);
+extern void pcibios_scan_phb(struct pci_controller *hose);
 
 #endif	/* __KERNEL__ */
 #endif /* __ASM_POWERPC_PCI_H */
diff --git a/arch/powerpc/kernel/of_platform.c b/arch/powerpc/kernel/of_platform.c
index b2c363e..9bd951c 100644
--- a/arch/powerpc/kernel/of_platform.c
+++ b/arch/powerpc/kernel/of_platform.c
@@ -74,7 +74,7 @@ static int __devinit of_pci_phb_probe(struct platform_device *dev,
 #endif /* CONFIG_EEH */
 
 	/* Scan the bus */
-	pcibios_scan_phb(phb, dev->dev.of_node);
+	pcibios_scan_phb(phb);
 	if (phb->bus == NULL)
 		return -ENXIO;
 
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index eb341be..3cd85fa 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1688,13 +1688,8 @@ int early_find_capability(struct pci_controller *hose, int bus, int devfn,
 /**
  * pci_scan_phb - Given a pci_controller, setup and scan the PCI bus
  * @hose: Pointer to the PCI host controller instance structure
- * @sysdata: 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 *sysdata)
+void __devinit pcibios_scan_phb(struct pci_controller *hose)
 {
 	struct pci_bus *bus;
 	struct device_node *node = hose->dn;
@@ -1704,13 +1699,13 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose, void *sysdata)
 		 node ? node->full_name : "<NO NAME>");
 
 	/* Create an empty bus for the toplevel */
-	bus = pci_create_bus(hose->parent, hose->first_busno, hose->ops,
-			     sysdata);
+	bus = pci_create_bus(hose->parent, hose->first_busno, hose->ops, hose);
 	if (bus == NULL) {
 		pr_err("Failed to create bus for PCI domain %04x\n",
 			hose->global_number);
 		return;
 	}
+	bus->dev.of_node = of_node_get(node);
 	bus->secondary = hose->first_busno;
 	hose->bus = bus;
 
diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
index e7db5b4..bedb370 100644
--- a/arch/powerpc/kernel/pci_32.c
+++ b/arch/powerpc/kernel/pci_32.c
@@ -381,7 +381,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, hose);
+		pcibios_scan_phb(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 8515776..fc6452b 100644
--- a/arch/powerpc/kernel/pci_64.c
+++ b/arch/powerpc/kernel/pci_64.c
@@ -64,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) {
-		pcibios_scan_phb(hose, hose->dn);
+		pcibios_scan_phb(hose);
 		pci_bus_add_devices(hose->bus);
 	}
 
@@ -242,10 +242,10 @@ long sys_pciconfig_iobase(long which, unsigned long in_bus,
 			break;
 		bus = NULL;
 	}
-	if (bus == NULL || bus->sysdata == NULL)
+	if (bus == NULL || bus->dev.of_node == NULL)
 		return -ENODEV;
 
-	hose_node = (struct device_node *)bus->sysdata;
+	hose_node = bus->dev.of_node;
 	hose = PCI_DN(hose_node)->phb;
 
 	switch (which) {
diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index d56b35e..2985268 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -161,7 +161,7 @@ static void *is_devfn_node(struct device_node *dn, void *data)
 /*
  * This is the "slow" path for looking up a device_node from a
  * pci_dev.  It will hunt for the device under its parent's
- * phb and then update sysdata for a future fastpath.
+ * phb and then update of_node pointer.
  *
  * It may also do fixups on the actual device since this happens
  * on the first read/write.
@@ -170,16 +170,19 @@ static void *is_devfn_node(struct device_node *dn, void *data)
  * In this case it may probe for real hardware ("just in case")
  * and add a device_node to the device tree if necessary.
  *
+ * Is this function necessary anymore now that dev->dev.of_node is
+ * used to store the node pointer?
+ *
  */
 struct device_node *fetch_dev_dn(struct pci_dev *dev)
 {
-	struct device_node *orig_dn = dev->sysdata;
+	struct device_node *orig_dn = dev->dev.of_node;
 	struct device_node *dn;
 	unsigned long searchval = (dev->bus->number << 8) | dev->devfn;
 
 	dn = traverse_pci_devices(orig_dn, is_devfn_node, (void *)searchval);
 	if (dn)
-		dev->sysdata = dn;
+		dev->dev.of_node = dn;
 	return dn;
 }
 EXPORT_SYMBOL(fetch_dev_dn);
diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c
index e751506..1e89a72 100644
--- a/arch/powerpc/kernel/pci_of_scan.c
+++ b/arch/powerpc/kernel/pci_of_scan.c
@@ -135,7 +135,7 @@ struct pci_dev *of_create_pci_dev(struct device_node *node,
 	pr_debug("    create device, devfn: %x, type: %s\n", devfn, type);
 
 	dev->bus = bus;
-	dev->sysdata = node;
+	dev->dev.of_node = of_node_get(node);
 	dev->dev.parent = bus->bridge;
 	dev->dev.bus = &pci_bus_type;
 	dev->devfn = devfn;
@@ -238,7 +238,7 @@ void __devinit of_scan_pci_bridge(struct device_node *node,
 	bus->primary = dev->bus->number;
 	bus->subordinate = busrange[1];
 	bus->bridge_ctl = 0;
-	bus->sysdata = node;
+	bus->dev.of_node = of_node_get(node);
 
 	/* parse ranges property */
 	/* PCI #address-cells == 3 and #size-cells == 2 always */
diff --git a/arch/powerpc/platforms/pseries/pci_dlpar.c b/arch/powerpc/platforms/pseries/pci_dlpar.c
index 5fcc92a..3bf4488 100644
--- a/arch/powerpc/platforms/pseries/pci_dlpar.c
+++ b/arch/powerpc/platforms/pseries/pci_dlpar.c
@@ -149,7 +149,7 @@ struct pci_controller * __devinit init_phb_dynamic(struct device_node *dn)
 	if (dn->child)
 		eeh_add_device_tree_early(dn);
 
-	pcibios_scan_phb(phb, dn);
+	pcibios_scan_phb(phb);
 	pcibios_finish_adding_to_bus(phb->bus);
 
 	return phb;

^ permalink raw reply related

* Re: minimum guaranteed alignment of dma_alloc_coherent?
From: Dan Malek @ 2011-02-05  2:56 UTC (permalink / raw)
  To: Tabi Timur-B04825; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <4D4CB038.9020602@freescale.com>


On Feb 4, 2011, at 6:04 PM, Tabi Timur-B04825 wrote:

> I guess I'm not clear.  I was wondering why an API called  
> "dma_alloc_coherent" (that has the word "dma" in it) needs to be  
> told to allocate DMA-safe memory.

I understood your question, and I indicated this is used in a
platform specific manner.

>  When would it make sense to call dma_alloc_cohernet without  
> GFP_DMA?  If you don't want DMA-able memory, then you shouldn't be  
> calling dma_alloc_anything.

So, I did a little research.  It appears in the case of PowerPC,
the GFP_DMA can change the way the allocation operates.
Since the coherent allocator works with pages of memory,
in the case of a system with highmem, not using GFP_DMA
could hand you a physical page out of the highmem pool.
If your device is capable of doing a DMA to that physical
location, all is good.  If your device has some addressing
restriction to low memory buffers, you should specify GFP_DMA.
I think for all architectures, GFP_DMA will guarantee a DMA
buffer for any device in a system.  If your device can relax that
constraint, you can elect to not use this flag and conserve
the DMA pool for those devices that require the restrictions.

This behavior is modified if you specify a restricted DMA
mask for the device.  In this case, dma_alloc_coherent
will force GFP_DMA on your behalf (on PowerPC).

Thanks.

	-- Dan

^ permalink raw reply

* Re: minimum guaranteed alignment of dma_alloc_coherent?
From: Tabi Timur-B04825 @ 2011-02-05  2:04 UTC (permalink / raw)
  To: Dan Malek; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <2A5A3DDA-94B3-48E3-9E49-6A5260BE1484@digitaldans.com>

Dan Malek wrote:
>
>
>> On a side note, do I really need to pass GFP_DMA ....
>
> The GFP_DMA is architecture dependent.  Are you writing
> a driver to be used across multiple architectures?

It's conceivable that the driver could work on PowerPC and ARM.

> If it's
> necessary, I'd document why you are using it (an ISA device
> on x86 for example) and then let other architectures
> determine if it's necessary for them.

I guess I'm not clear.  I was wondering why an API called "dma_alloc_cohere=
nt" (that has the word "dma" in it) needs to be told to allocate DMA-safe m=
emory.  When would it make sense to call dma_alloc_cohernet without GFP_DMA=
?  If you don't want DMA-able memory, then you shouldn't be calling dma_all=
oc_anything.

--=20
Timur Tabi
Linux kernel developer=

^ permalink raw reply

* Re: minimum guaranteed alignment of dma_alloc_coherent?
From: Dan Malek @ 2011-02-05  1:09 UTC (permalink / raw)
  To: Timur Tabi; +Cc: linuxppc-dev
In-Reply-To: <AANLkTi=1VkJM6hJH44jeUmYwoEtJj22d6WyKVDitg=KY@mail.gmail.com>


On Feb 4, 2011, at 4:14 PM, Timur Tabi wrote:

> Is there any official statement on what the minimum alignment is for
> memory returned by dma_alloc_coherent?

This is dependent upon the particular implementation.
There have been several over the history of this API,
and some would work out of a DMA pool that would
only provide word alignment.

> Would anyone have a complaint if I did this:
>
> vaddr = dma_alloc_coherent(dev, ssize, &paddr, GFP_DMA | __GFP_ZERO);
> BUG_ON(paddr & (alignment - 1));

Well, the usual design for anything that requires alignment
is that you never assume you get it unless it can be specified,
so over allocate as you are doing currently, and then adjust
the base address.  Or, you could allocate exactly what you
want, if it aligns properly you are good to go, and if not release,
over allocate, and adjust.

The problem with the suggested implementation is the results
aren't predictable.  Even if the API doesn't guarantee your
alignment, you may get it most of the time, but once in a
while this may fail.  You could test and all looks good,
ship something and it fails.

Today, this may work fine, but it would be nice to ensure
it continues to work in the future. :-)

> On a side note, do I really need to pass GFP_DMA ....

The GFP_DMA is architecture dependent.  Are you writing
a driver to be used across multiple architectures?  If it's
necessary, I'd document why you are using it (an ISA device
on x86 for example) and then let other architectures
determine if it's necessary for them.

Thanks.

	-- Dan

^ permalink raw reply

* minimum guaranteed alignment of dma_alloc_coherent?
From: Timur Tabi @ 2011-02-05  0:14 UTC (permalink / raw)
  To: linuxppc-dev

Is there any official statement on what the minimum alignment is for
memory returned by dma_alloc_coherent?  I know that since it uses a
page allocator to do the actual allocation, that the memory is page
aligned (at least on PowerPC).  Is this something I can rely on?

Would anyone have a complaint if I did this:

vaddr = dma_alloc_coherent(dev, ssize, &paddr, GFP_DMA | __GFP_ZERO);
BUG_ON(paddr & (alignment - 1));

'alignment' will always be 32 or less.

The reason I ask is that I currently have code that ensures an
alignment, but it does so by allocating extra memory, and that seems
wasteful.

On a side note, do I really need to pass GFP_DMA when calling
dma_alloc_coherent?  That seems redundant, but I don't see the code
force it to be set.  Should it be forced on?

-- 
Timur Tabi
Linux kernel developer at Freescale

^ permalink raw reply

* Re: Problem with Busybox shell
From: Gorelik, Jacob (335F) @ 2011-02-04 23:14 UTC (permalink / raw)
  To: linuxppc-dev@lists.ozlabs.org; +Cc: mohanreddykv@gmail.com

Hi Mohan,

Were you able to fix your problem? I am having the same problem with my boa=
rd. It is PPC750FX with Marvell MV64460.

Jacob=

^ permalink raw reply

* [PATCH v3 4/4] powerpc: Replacing "protected-sources" with "pic-no-reset" in DTS files
From: Meador Inge @ 2011-02-04 23:25 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: devicetree-discuss, Hollis Blanchard
In-Reply-To: <1296861941-3370-1-git-send-email-meador_inge@mentor.com>

The "protected-sources" property was being used in the AMP configured
MPC8572DS and P2020RDB DTS files.  This changeset modifies those files
to use "pic-no-reset" instead.

Signed-off-by: Meador Inge <meador_inge@mentor.com>
Cc: Hollis Blanchard <hollis_blanchard@mentor.com>
Cc: Kumar Gala <galak@kernel.crashing.org>
---
 arch/powerpc/boot/dts/mpc8572ds_camp_core0.dts |    6 +-----
 arch/powerpc/boot/dts/mpc8572ds_camp_core1.dts |   11 +----------
 arch/powerpc/boot/dts/p2020rdb_camp_core0.dts  |    7 +------
 arch/powerpc/boot/dts/p2020rdb_camp_core1.dts  |    7 +------
 4 files changed, 4 insertions(+), 27 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8572ds_camp_core0.dts b/arch/powerpc/boot/dts/mpc8572ds_camp_core0.dts
index 3375c2a..4f7ee6d 100644
--- a/arch/powerpc/boot/dts/mpc8572ds_camp_core0.dts
+++ b/arch/powerpc/boot/dts/mpc8572ds_camp_core0.dts
@@ -252,11 +252,7 @@
 			reg = <0x40000 0x40000>;
 			compatible = "chrp,open-pic";
 			device_type = "open-pic";
-			protected-sources = <
-			31 32 33 37 38 39       /* enet2 enet3 */
-			76 77 78 79 26 42	/* dma2 pci2 serial*/
-			0xe4 0xe5 0xe6 0xe7	/* msi */
-			>;
+			pic-no-reset;
 		};
 	};
 
diff --git a/arch/powerpc/boot/dts/mpc8572ds_camp_core1.dts b/arch/powerpc/boot/dts/mpc8572ds_camp_core1.dts
index e7b477f..62fd64f 100644
--- a/arch/powerpc/boot/dts/mpc8572ds_camp_core1.dts
+++ b/arch/powerpc/boot/dts/mpc8572ds_camp_core1.dts
@@ -178,16 +178,7 @@
 			reg = <0x40000 0x40000>;
 			compatible = "chrp,open-pic";
 			device_type = "open-pic";
-			protected-sources = <
-			18 16 10 42 45 58	/* MEM L2 mdio serial crypto */
-			29 30 34 35 36 40	/* enet0 enet1 */
-			24 25 20 21 22 23	/* pci0 pci1 dma1 */
-			43			/* i2c */
-			0x1 0x2 0x3 0x4         /* pci slot */
-			0x9 0xa 0xb 0xc         /* usb */
-			0x6 0x7 0xe 0x5         /* Audio elgacy SATA */
-			0xe0 0xe1 0xe2 0xe3	/* msi */
-			>;
+			pic-no-reset;
 		};
 	};
 
diff --git a/arch/powerpc/boot/dts/p2020rdb_camp_core0.dts b/arch/powerpc/boot/dts/p2020rdb_camp_core0.dts
index 0fe93d0..f749cbb 100644
--- a/arch/powerpc/boot/dts/p2020rdb_camp_core0.dts
+++ b/arch/powerpc/boot/dts/p2020rdb_camp_core0.dts
@@ -318,12 +318,7 @@
 			reg = <0x40000 0x40000>;
 			compatible = "chrp,open-pic";
 			device_type = "open-pic";
-			protected-sources = <
-			42 76 77 78 79 /* serial1 , dma2 */
-			29 30 34 26 /* enet0, pci1 */
-			0xe0 0xe1 0xe2 0xe3 /* msi */
-			0xe4 0xe5 0xe6 0xe7
-			>;
+			pic-no-reset;
 		};
 
 		global-utilities@e0000 {
diff --git a/arch/powerpc/boot/dts/p2020rdb_camp_core1.dts b/arch/powerpc/boot/dts/p2020rdb_camp_core1.dts
index e95a512..6ef2e64 100644
--- a/arch/powerpc/boot/dts/p2020rdb_camp_core1.dts
+++ b/arch/powerpc/boot/dts/p2020rdb_camp_core1.dts
@@ -129,12 +129,7 @@
 			reg = <0x40000 0x40000>;
 			compatible = "chrp,open-pic";
 			device_type = "open-pic";
-			protected-sources = <
-			17 18 43 42 59 47 /*ecm, mem, i2c, serial0, spi,gpio */
-			16 20 21 22 23 28 	/* L2, dma1, USB */
-			03 35 36 40 31 32 33 	/* mdio, enet1, enet2 */
-			72 45 58 25 		/* sdhci, crypto , pci */
-			>;
+			pic-no-reset;
 		};
 
 		msi@41600 {
-- 
1.6.3.3

^ permalink raw reply related

* [PATCH v3 3/4] powerpc: make MPIC honor the "pic-no-reset" device tree property
From: Meador Inge @ 2011-02-04 23:25 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: devicetree-discuss, Hollis Blanchard
In-Reply-To: <1296861941-3370-1-git-send-email-meador_inge@mentor.com>

This property, defined in the Open PIC binding, tells the kernel not to use the
reset bit in the global configuration register.  Additionally, its presence
mandates that only sources which are actually used (i.e. appear in the device
tree) should have their VECPRI bits initialized.

The presence of "protected-sources" is checked for the backwards
compatibility of systems which are already using "protected-sources"
in firmware and the firmware can not be updated.  Any sources encoded in the
"protected-sources" cells are not processed.  "protected-sources" is
effectively a synonym for "pic-no-reset".

Signed-off-by: Meador Inge <meador_inge@mentor.com>
Cc: Hollis Blanchard <hollis_blanchard@mentor.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 arch/powerpc/include/asm/mpic.h |    4 ++-
 arch/powerpc/sysdev/mpic.c      |   64 +++++++++++++++++++++++++++++++-------
 2 files changed, 55 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/include/asm/mpic.h b/arch/powerpc/include/asm/mpic.h
index 9b94f18..688e3e0 100644
--- a/arch/powerpc/include/asm/mpic.h
+++ b/arch/powerpc/include/asm/mpic.h
@@ -322,6 +322,8 @@ struct mpic
 #ifdef CONFIG_PM
 	struct mpic_irq_save	*save_data;
 #endif
+
+	int cpu;
 };
 
 /*
@@ -333,7 +335,7 @@ struct mpic
  */
 
 /* This is the primary controller, only that one has IPIs and
- * has afinity control. A non-primary MPIC always uses CPU0
+ * has affinity control. A non-primary MPIC always uses CPU0
  * registers only
  */
 #define MPIC_PRIMARY			0x00000001
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index a98f41d..8de47ca 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -308,6 +308,15 @@ static inline void mpic_map(struct mpic *mpic, struct device_node *node,
 #define mpic_map(m,n,p,b,o,s)	_mpic_map_mmio(m,p,b,o,s)
 #endif /* !CONFIG_PPC_DCR */
 
+static inline void mpic_init_vector(struct mpic *mpic, int source)
+{
+	/* start with vector = source number, and masked */
+	u32 vecpri = MPIC_VECPRI_MASK | source | (8 << MPIC_VECPRI_PRIORITY_SHIFT);
+		
+	/* init hw */
+	mpic_irq_write(source, MPIC_INFO(IRQ_VECTOR_PRI), vecpri);
+	mpic_irq_write(source, MPIC_INFO(IRQ_DESTINATION), 1 << mpic->cpu);
+}
 
 
 /* Check if we have one of those nice broken MPICs with a flipped endian on
@@ -622,6 +631,14 @@ static unsigned int mpic_is_ipi(struct mpic *mpic, unsigned int irq)
 	return (src >= mpic->ipi_vecs[0] && src <= mpic->ipi_vecs[3]);
 }
 
+/* Determine if the linux irq is a timer interrupt */
+static unsigned int mpic_is_timer_interrupt(struct mpic *mpic, unsigned int irq)
+{
+	unsigned int src = mpic_irq_to_hw(irq);
+
+	return (src >= mpic->timer_vecs[0] && src <= mpic->timer_vecs[3]);
+}
+
 
 /* Convert a cpu mask from logical to physical cpu numbers. */
 static inline u32 mpic_physmask(u32 cpumask)
@@ -963,6 +980,15 @@ static int mpic_host_map(struct irq_host *h, unsigned int virq,
 	if (hw >= mpic->irq_count)
 		return -EINVAL;
 
+	/* If the MPIC was reset, then all vectors have already been
+	 * initialized.  Otherwise, the appropriate vector needs to be
+	 * initialized here to ensure that only used sources are setup with
+	 * a vector.
+	 */
+	if (!(mpic->flags & MPIC_WANTS_RESET))
+		if (!(mpic_is_ipi(mpic, hw) || mpic_is_timer_interrupt(mpic, hw)))
+			mpic_init_vector(mpic, hw);
+
 	mpic_msi_reserve_hwirq(mpic, hw);
 
 	/* Default chip */
@@ -1029,6 +1055,16 @@ static struct irq_host_ops mpic_host_ops = {
 	.xlate = mpic_host_xlate,
 };
 
+static int mpic_reset_prohibited(struct device_node *node)
+{
+	/* The presence of "protected-sources" is checked for the backwards
+	 * compatibility of systems which are already using "protected-sources"
+	 * in firmware and the firmware can not be updated.
+	 */
+	return node && (of_get_property(node, "pic-no-reset", NULL)
+		|| of_get_property(node, "protected-sources", NULL));
+}
+
 /*
  * Exported functions
  */
@@ -1129,7 +1165,16 @@ struct mpic * __init mpic_alloc(struct device_node *node,
 	mpic_map(mpic, node, paddr, &mpic->tmregs, MPIC_INFO(TIMER_BASE), 0x1000);
 
 	/* Reset */
-	if (flags & MPIC_WANTS_RESET) {
+
+	/* When using a device-node, reset requests are only honored if the MPIC
+	 * is allowed to reset.
+	 */
+	if (mpic_reset_prohibited(node)) {
+		mpic->flags &= ~MPIC_WANTS_RESET;
+	}
+
+	if (mpic->flags & MPIC_WANTS_RESET) {
+		printk(KERN_DEBUG "mpic: Resetting\n");
 		mpic_write(mpic->gregs, MPIC_INFO(GREG_GLOBAL_CONF_0),
 			   mpic_read(mpic->gregs, MPIC_INFO(GREG_GLOBAL_CONF_0))
 			   | MPIC_GREG_GCONF_RESET);
@@ -1246,7 +1291,6 @@ void __init mpic_set_default_senses(struct mpic *mpic, u8 *senses, int count)
 void __init mpic_init(struct mpic *mpic)
 {
 	int i;
-	int cpu;
 
 	BUG_ON(mpic->num_sources == 0);
 
@@ -1290,18 +1334,14 @@ void __init mpic_init(struct mpic *mpic)
 	mpic_pasemi_msi_init(mpic);
 
 	if (mpic->flags & MPIC_PRIMARY)
-		cpu = hard_smp_processor_id();
+		mpic->cpu = hard_smp_processor_id();
 	else
-		cpu = 0;
+		mpic->cpu = 0;
 
-	for (i = 0; i < mpic->num_sources; i++) {
-		/* start with vector = source number, and masked */
-		u32 vecpri = MPIC_VECPRI_MASK | i |
-			(8 << MPIC_VECPRI_PRIORITY_SHIFT);
-		
-		/* init hw */
-		mpic_irq_write(i, MPIC_INFO(IRQ_VECTOR_PRI), vecpri);
-		mpic_irq_write(i, MPIC_INFO(IRQ_DESTINATION), 1 << cpu);
+	if (mpic->flags & MPIC_WANTS_RESET) {
+		for (i = 0; i < mpic->num_sources; i++) {
+			mpic_init_vector(mpic, i);
+		}
 	}
 	
 	/* Init spurious vector */
-- 
1.6.3.3

^ permalink raw reply related

* [PATCH v3 2/4] powerpc: document the Open PIC device tree binding
From: Meador Inge @ 2011-02-04 23:25 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: devicetree-discuss, Hollis Blanchard, Stuart Yoder
In-Reply-To: <1296861941-3370-1-git-send-email-meador_inge@mentor.com>

This binding documents several properties that have been in use for quite
some time, and adds one new property 'pic-no-reset', which controls the runtime
initialization behavior of the PIC.  More specifically, the presence of
'pic-no-reset' mandates that the PIC shall not be reset during runtime
initialization and that any initialization related to interrupt sources
shall be limited to sources explicitly referenced in the device tree.  This
functionality is useful in AMP systems where multiple OSes are sharing the
PIC and the reinitialization of the PIC can interfere with OSes that are
already up and running.

The interrupt specifier definition is based off of Stuart Yoder's FSL MPIC
binding.

Signed-off-by: Meador Inge <meador_inge@mentor.com>
Cc: Hollis Blanchard <hollis_blanchard@mentor.com>
Cc: Stuart Yoder <stuart.yoder@freescale.com>
---
 Documentation/powerpc/dts-bindings/open-pic.txt |   98 +++++++++++++++++++++++
 1 files changed, 98 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/powerpc/dts-bindings/open-pic.txt

diff --git a/Documentation/powerpc/dts-bindings/open-pic.txt b/Documentation/powerpc/dts-bindings/open-pic.txt
new file mode 100644
index 0000000..909a902
--- /dev/null
+++ b/Documentation/powerpc/dts-bindings/open-pic.txt
@@ -0,0 +1,98 @@
+* Open PIC Binding
+
+This binding specifies what properties must be available in the device tree
+representation of an Open PIC compliant interrupt controller.  This binding is
+based on the binding defined for Open PIC in [1] and is a superset of that
+binding.
+
+Required properties:
+
+  NOTE: Many of these descriptions were paraphrased here from [1] to aid
+        readability.
+
+    - compatible: Specifies the compatibility list for the PIC.  The type
+      shall be <string> and the value shall include "open-pic".
+
+    - reg: Specifies the base physical address(s) and size(s) of this
+      PIC's addressable register space.  The type shall be <prop-encoded-array>.
+
+    - interrupt-controller: The presence of this property identifies the node
+      as an Open PIC.  No property value shall be defined.
+
+    - #interrupt-cells: Specifies the number of cells needed to encode an
+      interrupt source.  The type shall be a <u32> and the value shall be 2.
+
+    - #address-cells: Specifies the number of cells needed to encode an
+      address.  The type shall be <u32> and the value shall be 0.  As such,
+      'interrupt-map' nodes do not have to specify a parent unit address.
+
+Optional properties:
+
+    - pic-no-reset: The presence of this property indicates that the PIC
+      shall not be reset during runtime initialization.  No property value shall
+      be defined.  The presence of this property also mandates that any
+      initialization related to interrupt sources shall be limited to sources
+      explicitly referenced in the device tree.
+
+* Interrupt Specifier Definition
+
+  Interrupt specifiers consists of 2 cells encoded as
+  follows:
+
+    - <1st-cell>: The interrupt-number that identifies the interrupt source.
+
+    - <2nd-cell>: The level-sense information, encoded as follows:
+                    0 = low-to-high edge triggered
+                    1 = active low level-sensitive
+                    2 = active high level-sensitive
+                    3 = high-to-low edge triggered
+
+* Examples
+
+Example 1:
+
+	/*
+	 * An Open PIC interrupt controller
+	 */
+	mpic: pic@40000 {
+		// This is an interrupt controller node.
+		interrupt-controller;
+
+		// No address cells so that 'interrupt-map' nodes which reference
+		// this Open PIC node do not need a parent address specifier.
+		#address-cells = <0>;
+
+		// Two cells to encode interrupt sources.
+		#interrupt-cells = <2>;
+
+		// Offset address of 0x40000 and size of 0x40000.
+		reg = <0x40000 0x40000>;
+
+		// Compatible with Open PIC.
+		compatible = "open-pic";
+
+		// The PIC shall not be reset.
+		pic-no-reset;
+	};
+
+Example 2:
+
+	/*
+	 * An interrupt generating device that is wired to an Open PIC.
+	 */
+	serial0: serial@4500 {
+		// Interrupt source '42' that is active high level-sensitive.
+		// Note that there are only two cells as specified in the interrupt
+		// parent's '#interrupt-cells' property.
+		interrupts = <42 2>;
+
+		// The interrupt controller that this device is wired to.
+		interrupt-parent = <&mpic>;
+	};
+
+* References
+
+[1] Power.org (TM) Standard for Embedded Power Architecture (TM) Platform
+    Requirements (ePAPR), Version 1.0, July 2008.
+    (http://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.0.pdf)
+
-- 
1.6.3.3

^ permalink raw reply related

* [PATCH v3 1/4] powerpc: Removing support for 'protected-sources'
From: Meador Inge @ 2011-02-04 23:25 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: devicetree-discuss, Hollis Blanchard
In-Reply-To: <1296861941-3370-1-git-send-email-meador_inge@mentor.com>

In a recent thread [1,2,3] concerning device trees for AMP systems, the
question of whether we really need 'protected-sources' arose.  The general
consensus was that a new boolean property 'pic-no-reset' (described in more
detail in a following patch) could be expanded to cover the use cases that
'protected-sources' was covering.

One concern that was raised was for legacy systems which already use the
'protected-sources' property [4].  For legacy use cases, 'protected-sources'
will be treated as an alias of 'pic-no-reset'.  The sources
encoded in the 'protected-sources' cells, however, will not be processed.  This
legacy check is added in a later patch in the series.

[1] http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-January/004038.html
[2] http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-January/003991.html
[3] http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-January/004043.html
[4] http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-February/004254.html

Signed-off-by: Meador Inge <meador_inge@mentor.com>
Cc: Hollis Blanchard <hollis_blanchard@mentor.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 arch/powerpc/include/asm/mpic.h |    3 ---
 arch/powerpc/sysdev/mpic.c      |   38 --------------------------------------
 2 files changed, 0 insertions(+), 41 deletions(-)

diff --git a/arch/powerpc/include/asm/mpic.h b/arch/powerpc/include/asm/mpic.h
index e000cce..9b94f18 100644
--- a/arch/powerpc/include/asm/mpic.h
+++ b/arch/powerpc/include/asm/mpic.h
@@ -301,9 +301,6 @@ struct mpic
 	struct mpic_reg_bank	cpuregs[MPIC_MAX_CPUS];
 	struct mpic_reg_bank	isus[MPIC_MAX_ISU];
 
-	/* Protected sources */
-	unsigned long		*protected;
-
 #ifdef CONFIG_MPIC_WEIRD
 	/* Pointer to HW info array */
 	u32			*hw_set;
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index 7c13426..a98f41d 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -947,8 +947,6 @@ static int mpic_host_map(struct irq_host *h, unsigned int virq,
 
 	if (hw == mpic->spurious_vec)
 		return -EINVAL;
-	if (mpic->protected && test_bit(hw, mpic->protected))
-		return -EINVAL;
 
 #ifdef CONFIG_SMP
 	else if (hw >= mpic->ipi_vecs[0]) {
@@ -1095,26 +1093,6 @@ struct mpic * __init mpic_alloc(struct device_node *node,
 	if (node && of_get_property(node, "big-endian", NULL) != NULL)
 		mpic->flags |= MPIC_BIG_ENDIAN;
 
-	/* Look for protected sources */
-	if (node) {
-		int psize;
-		unsigned int bits, mapsize;
-		const u32 *psrc =
-			of_get_property(node, "protected-sources", &psize);
-		if (psrc) {
-			psize /= 4;
-			bits = intvec_top + 1;
-			mapsize = BITS_TO_LONGS(bits) * sizeof(unsigned long);
-			mpic->protected = kzalloc(mapsize, GFP_KERNEL);
-			BUG_ON(mpic->protected == NULL);
-			for (i = 0; i < psize; i++) {
-				if (psrc[i] > intvec_top)
-					continue;
-				__set_bit(psrc[i], mpic->protected);
-			}
-		}
-	}
-
 #ifdef CONFIG_MPIC_WEIRD
 	mpic->hw_set = mpic_infos[MPIC_GET_REGSET(flags)];
 #endif
@@ -1321,9 +1299,6 @@ void __init mpic_init(struct mpic *mpic)
 		u32 vecpri = MPIC_VECPRI_MASK | i |
 			(8 << MPIC_VECPRI_PRIORITY_SHIFT);
 		
-		/* check if protected */
-		if (mpic->protected && test_bit(i, mpic->protected))
-			continue;
 		/* init hw */
 		mpic_irq_write(i, MPIC_INFO(IRQ_VECTOR_PRI), vecpri);
 		mpic_irq_write(i, MPIC_INFO(IRQ_DESTINATION), 1 << cpu);
@@ -1492,13 +1467,6 @@ static unsigned int _mpic_get_one_irq(struct mpic *mpic, int reg)
 			mpic_eoi(mpic);
 		return NO_IRQ;
 	}
-	if (unlikely(mpic->protected && test_bit(src, mpic->protected))) {
-		if (printk_ratelimit())
-			printk(KERN_WARNING "%s: Got protected source %d !\n",
-			       mpic->name, (int)src);
-		mpic_eoi(mpic);
-		return NO_IRQ;
-	}
 
 	return irq_linear_revmap(mpic->irqhost, src);
 }
@@ -1532,12 +1500,6 @@ unsigned int mpic_get_coreint_irq(void)
 			mpic_eoi(mpic);
 		return NO_IRQ;
 	}
-	if (unlikely(mpic->protected && test_bit(src, mpic->protected))) {
-		if (printk_ratelimit())
-			printk(KERN_WARNING "%s: Got protected source %d !\n",
-			       mpic->name, (int)src);
-		return NO_IRQ;
-	}
 
 	return irq_linear_revmap(mpic->irqhost, src);
 #else
-- 
1.6.3.3

^ permalink raw reply related

* [PATCH v3 0/4] powerpc: Open PIC binding and "pic-no-reset"
From: Meador Inge @ 2011-02-04 23:25 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: devicetree-discuss, Hollis Blanchard

This patch set provides a binding for Open PIC and implements support for
a new property, specified by that binding, called "pic-no-reset".  With
"pic-no-reset" in place the "protected-sources" property is no longer needed
and its full implementation was removed.  "protected-sources" is still checked
for, however, for legacy purposes.

For v3 of this patch the Open PIC binding was changed to be more consistent
with existing bindings, several DTS files were cleaned up, "no-reset" was
changed to "pic-no-reset", and a check to treat "protected-sources" as a
synonym for "pic-no-reset" was added.

Signed-off-by: Meador Inge <meador_inge@mentor.com>
Cc: Hollis Blanchard <hollis_blanchard@mentor.com>

Meador Inge (4):
  powerpc: Removing support for 'protected-sources'
  powerpc: document the Open PIC device tree binding
  powerpc: make MPIC honor the "pic-no-reset" device tree property
  powerpc: Replacing "protected-sources" with "pic-no-reset" in DTS
    files

 Documentation/powerpc/dts-bindings/open-pic.txt |   98 ++++++++++++++++++++++
 arch/powerpc/boot/dts/mpc8572ds_camp_core0.dts  |    6 +-
 arch/powerpc/boot/dts/mpc8572ds_camp_core1.dts  |   11 +--
 arch/powerpc/boot/dts/p2020rdb_camp_core0.dts   |    7 +--
 arch/powerpc/boot/dts/p2020rdb_camp_core1.dts   |    7 +--
 arch/powerpc/include/asm/mpic.h                 |    7 +-
 arch/powerpc/sysdev/mpic.c                      |  102 ++++++++++++-----------
 7 files changed, 157 insertions(+), 81 deletions(-)
 create mode 100644 Documentation/powerpc/dts-bindings/open-pic.txt

^ 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