LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup event source
From: Scott Wood @ 2012-06-04 23:02 UTC (permalink / raw)
  To: Zhao Chenhui; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20120604113609.GC20676@localhost.localdomain>

On 06/04/2012 06:36 AM, Zhao Chenhui wrote:
> On Fri, Jun 01, 2012 at 05:08:52PM -0500, Scott Wood wrote:
>> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
>>> Add APIs for setting wakeup source and lossless Ethernet in low power modes.
>>> These APIs can be used by wake-on-packet feature.
>>>
>>> Signed-off-by: Dave Liu <daveliu@freescale.com>
>>> Signed-off-by: Li Yang <leoli@freescale.com>
>>> Signed-off-by: Jin Qing <b24347@freescale.com>
>>> Signed-off-by: Zhao Chenhui <chenhui.zhao@freescale.com>
>>> ---
>>>  arch/powerpc/sysdev/fsl_pmc.c |   71 ++++++++++++++++++++++++++++++++++++++++-
>>>  arch/powerpc/sysdev/fsl_soc.h |    9 +++++
>>>  2 files changed, 79 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/powerpc/sysdev/fsl_pmc.c b/arch/powerpc/sysdev/fsl_pmc.c
>>> index 1dc6e9e..c1170f7 100644
>>> --- a/arch/powerpc/sysdev/fsl_pmc.c
>>> +++ b/arch/powerpc/sysdev/fsl_pmc.c
>>> @@ -34,6 +34,7 @@ struct pmc_regs {
>>>  	__be32 powmgtcsr;
>>>  #define POWMGTCSR_SLP		0x00020000
>>>  #define POWMGTCSR_DPSLP		0x00100000
>>> +#define POWMGTCSR_LOSSLESS	0x00400000
>>>  	__be32 res3[2];
>>>  	__be32 pmcdr;
>>>  };
>>> @@ -43,6 +44,74 @@ static unsigned int pmc_flag;
>>>  
>>>  #define PMC_SLEEP	0x1
>>>  #define PMC_DEEP_SLEEP	0x2
>>> +#define PMC_LOSSLESS	0x4
>>> +
>>> +/**
>>> + * mpc85xx_pmc_set_wake - enable devices as wakeup event source
>>> + * @pdev: platform device affected
>>> + * @enable: True to enable event generation; false to disable
>>> + *
>>> + * This enables the device as a wakeup event source, or disables it.
>>> + *
>>> + * RETURN VALUE:
>>> + * 0 is returned on success
>>> + * -EINVAL is returned if device is not supposed to wake up the system
>>> + * Error code depending on the platform is returned if both the platform and
>>> + * the native mechanism fail to enable the generation of wake-up events
>>> + */
>>> +int mpc85xx_pmc_set_wake(struct platform_device *pdev, bool enable)
>>
>> Why does it have to be a platform_device?  Would a bare device_node work
>> here?  If it's for stuff like device_may_wakeup() that could be in a
>> platform_device wrapper function.
> 
> It does not have to be a platform_device. I think it can be a struct device.

Why does it even need that?  The low level mechanism for influencing
PMCDR should only need a device node, not a Linux device struct.

>> Where does this get called from?  I don't see an example user in this
>> patchset.
> 
> It will be used by a gianfar related patch. I plan to submit that patch
> after these patches accepted.

It would be nice to see how this is used when reviewing this.

>>> +{
>>> +	int ret = 0;
>>> +	struct device_node *clk_np;
>>> +	u32 *prop;
>>> +	u32 pmcdr_mask;
>>> +
>>> +	if (!pmc_regs) {
>>> +		pr_err("%s: PMC is unavailable\n", __func__);
>>> +		return -ENODEV;
>>> +	}
>>> +
>>> +	if (enable && !device_may_wakeup(&pdev->dev))
>>> +		return -EINVAL;
>>
>> Who is setting can_wakeup for these devices?
> 
> The device driver is responsible to set can_wakeup.

How would the device driver know how to set it?  Wouldn't this depend on
the particular SoC and low power mode?

-Scott

^ permalink raw reply

* [PATCH] powerpc: Optimise the 64bit optimised __clear_user
From: Anton Blanchard @ 2012-06-05  2:02 UTC (permalink / raw)
  To: benh, paulus, michael, mikey, galak, olof; +Cc: linuxppc-dev
In-Reply-To: <20120604175858.38dac554@kryten>


I blame Mikey for this. He elevated my slightly dubious testcase:

# dd if=/dev/zero of=/dev/null bs=1M count=10000

to benchmark status. And naturally we need to be number 1 at creating
zeros. So lets improve __clear_user some more.

As Paul suggests we can use dcbz for large lengths. This patch gets
the destination cacheline aligned then uses dcbz on whole cachelines.

Before:
10485760000 bytes (10 GB) copied, 0.414744 s, 25.3 GB/s

After:
10485760000 bytes (10 GB) copied, 0.268597 s, 39.0 GB/s

39 GB/s, a new record.

Signed-off-by: Anton Blanchard <anton@samba.org>
---

v2: handle any cacheline size

Index: linux-build/arch/powerpc/lib/string_64.S
===================================================================
--- linux-build.orig/arch/powerpc/lib/string_64.S	2012-06-04 16:18:56.351604302 +1000
+++ linux-build/arch/powerpc/lib/string_64.S	2012-06-05 11:42:10.044654851 +1000
@@ -19,6 +19,12 @@
  */
 
 #include <asm/ppc_asm.h>
+#include <asm/asm-offsets.h>
+
+	.section	".toc","aw"
+PPC64_CACHES:
+	.tc		ppc64_caches[TC],ppc64_caches
+	.section	".text"
 
 /**
  * __clear_user: - Zero a block of memory in user space, with less checking.
@@ -94,9 +100,14 @@ err1;	stw	r0,0(r3)
 	addi	r3,r3,4
 
 3:	sub	r4,r4,r6
-	srdi	r6,r4,5
+
 	cmpdi	r4,32
+	cmpdi	cr1,r4,512
 	blt	.Lshort_clear
+	bgt	cr1,.Llong_clear
+
+.Lmedium_clear:
+	srdi	r6,r4,5
 	mtctr	r6
 
 	/* Do 32 byte chunks */
@@ -139,3 +150,53 @@ err1;	stb	r0,0(r3)
 
 10:	li	r3,0
 	blr
+
+.Llong_clear:
+	ld	r5,PPC64_CACHES@toc(r2)
+
+	bf	cr7*4+0,11f
+err2;	std	r0,0(r3)
+	addi	r3,r3,8
+	addi	r4,r4,-8
+
+	/* Destination is 16 byte aligned, need to get it cacheline aligned */
+11:	lwz	r7,DCACHEL1LOGLINESIZE(r5)
+	lwz	r9,DCACHEL1LINESIZE(r5)
+
+	/*
+	 * With worst case alignment the long clear loop takes a minimum
+	 * of 1 byte less than 2 cachelines.
+	 */
+	sldi	r10,r9,2
+	cmpd	r4,r10
+	blt	.Lmedium_clear
+
+	neg	r6,r3
+	addi	r10,r9,-1
+	and.	r5,r6,r10
+	beq	13f
+
+	srdi	r6,r5,4
+	mtctr	r6
+	mr	r8,r3
+12:
+err1;	std	r0,0(r3)
+err1;	std	r0,8(r3)
+	addi	r3,r3,16
+	bdnz	12b
+
+	sub	r4,r4,r5
+
+13:	srd	r6,r4,r7
+	mtctr	r6
+	mr	r8,r3
+14:
+err1;	dcbz	r0,r3
+	add	r3,r3,r9
+	bdnz	14b
+
+	and	r4,r4,r10
+
+	cmpdi	r4,32
+	blt	.Lshort_clear
+	b	.Lmedium_clear

^ permalink raw reply

* [PATCH] powerpc/ftrace: Do not trace restore_interrupts()
From: Steven Rostedt @ 2012-06-05  2:27 UTC (permalink / raw)
  To: LKML, linuxppc-dev; +Cc: stable

As I was adding code that affects all archs, I started testing function
tracer against PPC64 and found that it currently locks up with 3.4
kernel. I figured it was due to tracing a function that shouldn't be, so
I went through the following process to bisect to find the culprit:

 cat /debug/tracing/available_filter_functions > t
 num=`wc -l t`
 sed -ne "1,${num}p" t > t1
 let num=num+1
 sed -ne "${num},$p" t > t2
 cat t1 > /debug/tracing/set_ftrace_filter
 echo function /debug/tracing/current_tracer
 <failed? bisect t1, if not bisect t2>

It finally came down to this function: restore_interrupts()

I'm not sure why this locks up the system. It just seems to prevent
scheduling from occurring. Interrupts seem to still work, as I can ping
the box. But all user processes freeze.

When restore_interrupts() is not traced, function tracing works fine.

Cc: stable@kernel.org
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index 641da9e..64eec59 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -277,7 +277,7 @@ EXPORT_SYMBOL(arch_local_irq_restore);
  * NOTE: This is called with interrupts hard disabled but not marked
  * as such in paca->irq_happened, so we need to resync this.
  */
-void restore_interrupts(void)
+void notrace restore_interrupts(void)
 {
 	if (irqs_disabled()) {
 		local_paca->irq_happened |= PACA_IRQ_HARD_DIS;

^ permalink raw reply related

* RE: Cannot boot an kernel version other than 2.6.35 on P2020RDB-PCA
From: Tang Yuantian-B29983 @ 2012-06-05  2:28 UTC (permalink / raw)
  To: MAUSSIRE Cedric, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <2A5271086BF5EA4A891F3804135DEA820795A3B5@TLSEVS01.ts.eads.lcl>

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

Hi  Cedric MAUSSIRE,

P2020RDB-PCA Board gets supported since kernel 3.3 in mainline code.
Are you sure the board works on kernel 2.6.35?

Regards,
Yuantian

From: linuxppc-dev-bounces+b29983=freescale.com@lists.ozlabs.org [mailto:linuxppc-dev-bounces+b29983=freescale.com@lists.ozlabs.org] On Behalf Of MAUSSIRE Cedric
Sent: 2012年6月4日 23:06
To: linuxppc-dev@lists.ozlabs.org
Subject: Cannot boot an kernel version other than 2.6.35 on P2020RDB-PCA


Hello all,

I am currently working on the Freescale P2020RDB-PCA Board, which has been delivred with the linux-2.6.35 kernel. I succeed in re-generating my own uImage in 2.6.35 thanks to ELDK, but when I want to move on to a newer or older version of linux, the system does not respond any longer just after the boot whith the nfsboot command, the logs are :

## Booting kernel from Legacy Image at 01000000 ...
   Image Name:   Linux-2.6.39
   Created:      2012-05-29   8:24:54 UTC
   Image Type:   PowerPC Linux Kernel Image (gzip compressed)
   Data Size:    3514882 Bytes = 3.4 MiB
   Load Address: 00000000
   Entry Point:  00000000
   Verifying Checksum ... OK
## Flattened Device Tree blob at 00c00000
   Booting using the fdt blob at 0xc00000
   Uncompressing Kernel Image ... OK
   Loading Device Tree to 00ffa000, end 00fffd4a ... OK

NB : nfsboot=setenv bootargs root=/dev/nfs rw nfsroot=192.168.1.1:$rootpath ip=192.168.1.2:192.168.1.1:192.168.1.1:255.255.255.0:P2020RDB-PC:eth0:off console=ttyS0,115200

I have unsuccessfully tried to rebuild the device tree blob and use the default kernel configuration "mpc85xx_defconfig" that comes with the mainline code.

My systems seems to be linux-2.6.35 dependent, I do not known how it can be possible, because I can use the kernel.org source of the 2.6.35 whith no problems but when I use the 3.0, for example, with the same configuration but it does not work.


Best regards,


Cedric MAUSSIRE

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

^ permalink raw reply

* Re: [PATCH] powerpc/ftrace: Do not trace restore_interrupts()
From: Steven Rostedt @ 2012-06-05  2:33 UTC (permalink / raw)
  To: LKML; +Cc: linuxppc-dev, stable
In-Reply-To: <1338863274.13348.530.camel@gandalf.stny.rr.com>

On Mon, 2012-06-04 at 22:27 -0400, Steven Rostedt wrote:
> As I was adding code that affects all archs, I started testing function
> tracer against PPC64 and found that it currently locks up with 3.4
> kernel. I figured it was due to tracing a function that shouldn't be, so
> I went through the following process to bisect to find the culprit:
> 
>  cat /debug/tracing/available_filter_functions > t
>  num=`wc -l t`
>  sed -ne "1,${num}p" t > t1
>  let num=num+1
>  sed -ne "${num},$p" t > t2
>  cat t1 > /debug/tracing/set_ftrace_filter
>  echo function /debug/tracing/current_tracer
>  <failed? bisect t1, if not bisect t2>
> 
> It finally came down to this function: restore_interrupts()
> 
> I'm not sure why this locks up the system. It just seems to prevent
> scheduling from occurring. Interrupts seem to still work, as I can ping
> the box. But all user processes freeze.
> 
> When restore_interrupts() is not traced, function tracing works fine.
> 
> Cc: stable@kernel.org

This is what I get for cut-and-pasting from a random git commit. That
should be:

Cc: stable@vger.kernel.org

-- Steve

> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> 
> diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
> index 641da9e..64eec59 100644
> --- a/arch/powerpc/kernel/irq.c
> +++ b/arch/powerpc/kernel/irq.c
> @@ -277,7 +277,7 @@ EXPORT_SYMBOL(arch_local_irq_restore);
>   * NOTE: This is called with interrupts hard disabled but not marked
>   * as such in paca->irq_happened, so we need to resync this.
>   */
> -void restore_interrupts(void)
> +void notrace restore_interrupts(void)
>  {
>  	if (irqs_disabled()) {
>  		local_paca->irq_happened |= PACA_IRQ_HARD_DIS;
> 

^ permalink raw reply

* Re: [PATCH] powerpc: pseries: Round up MSI-X requests
From: Anton Blanchard @ 2012-06-05  2:47 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Michael Ellerman; +Cc: linuxppc-dev, paulus
In-Reply-To: <1338792897.7150.68.camel@pasglop>


Hi,

> On Mon, 2012-06-04 at 16:43 +1000, Michael Ellerman wrote:
> > There is some chance this will result in breakage because the driver
> > asks for N - and assumes that is what was allocated - and the
> > device is configured for > N.
> 
> We can fix that. We can whack the configuration back with N, just know
> that we have "allocated"  > N.

I agree we don't want to be giving back a larger value than requested.
There's only one place that can happen in theory and since firmware only
returns power of two values I dont think it will happen in practise.

Even so do we want to do something like this (as yet untested)? If the
rounded up request fails we retry with the original request.

The pseries msi free code just sets our vectors to 0 so it doesn't need
to know how many were originally allocated.
 
Anton
--

[PATCH] powerpc: pseries: Round up MSI-X requests

The pseries firmware currently refuses any non power of two MSI-X
request. Unfortunately most network drivers end up asking for that
because they want a power of two for RX queues and one or two extra
for everything else.

This patch rounds up the firmware request to the next power of two
if the quota allows it. If this fails we fall back to using the
original request size.

Signed-off-by: Anton Blanchard <anton@samba.org>
---        

Index: linux-build/arch/powerpc/platforms/pseries/msi.c
===================================================================
--- linux-build.orig/arch/powerpc/platforms/pseries/msi.c	2012-06-04 15:58:48.095833249 +1000
+++ linux-build/arch/powerpc/platforms/pseries/msi.c	2012-06-05 12:15:58.711074499 +1000
@@ -387,12 +387,13 @@ static int check_msix_entries(struct pci
 	return 0;
 }
 
-static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
+static int rtas_setup_msi_irqs(struct pci_dev *pdev, int nvec_in, int type)
 {
 	struct pci_dn *pdn;
 	int hwirq, virq, i, rc;
 	struct msi_desc *entry;
 	struct msi_msg msg;
+	int nvec = nvec_in;
 
 	pdn = get_pdn(pdev);
 	if (!pdn)
@@ -402,10 +403,23 @@ static int rtas_setup_msi_irqs(struct pc
 		return -EINVAL;
 
 	/*
+	 * Firmware currently refuse any non power of two allocation
+	 * so we round up if the quota will allow it.
+	 */
+	if (type == PCI_CAP_ID_MSIX) {
+		int m = roundup_pow_of_two(nvec);
+		int quota = msi_quota_for_device(pdev, m);
+
+		if (quota >= m)
+			nvec = m;
+	}
+
+	/*
 	 * Try the new more explicit firmware interface, if that fails fall
 	 * back to the old interface. The old interface is known to never
 	 * return MSI-Xs.
 	 */
+again:
 	if (type == PCI_CAP_ID_MSI) {
 		rc = rtas_change_msi(pdn, RTAS_CHANGE_MSI_FN, nvec);
 
@@ -417,6 +431,10 @@ static int rtas_setup_msi_irqs(struct pc
 		rc = rtas_change_msi(pdn, RTAS_CHANGE_MSIX_FN, nvec);
 
 	if (rc != nvec) {
+		if (nvec != nvec_in) {
+			nvec = nvec_in;
+			goto again;
+		}
 		pr_debug("rtas_msi: rtas_change_msi() failed\n");
 		return rc;
 	}

^ permalink raw reply

* RE: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup event source
From: Li Yang-R58472 @ 2012-06-05  4:08 UTC (permalink / raw)
  To: Wood Scott-B07421, Zhao Chenhui-B35336
  Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
In-Reply-To: <4FCD3E8B.30805@freescale.com>



> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, June 05, 2012 7:03 AM
> To: Zhao Chenhui-B35336
> Cc: linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org;
> galak@kernel.crashing.org; Li Yang-R58472
> Subject: Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup
> event source
>=20
> On 06/04/2012 06:36 AM, Zhao Chenhui wrote:
> > On Fri, Jun 01, 2012 at 05:08:52PM -0500, Scott Wood wrote:
> >> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
> >>> Add APIs for setting wakeup source and lossless Ethernet in low power
> modes.
> >>> These APIs can be used by wake-on-packet feature.
> >>>
> >>> Signed-off-by: Dave Liu <daveliu@freescale.com>
> >>> Signed-off-by: Li Yang <leoli@freescale.com>
> >>> Signed-off-by: Jin Qing <b24347@freescale.com>
> >>> Signed-off-by: Zhao Chenhui <chenhui.zhao@freescale.com>
> >>> ---
> >>>  arch/powerpc/sysdev/fsl_pmc.c |   71
> ++++++++++++++++++++++++++++++++++++++++-
> >>>  arch/powerpc/sysdev/fsl_soc.h |    9 +++++
> >>>  2 files changed, 79 insertions(+), 1 deletions(-)
> >>>
> >>> diff --git a/arch/powerpc/sysdev/fsl_pmc.c
> b/arch/powerpc/sysdev/fsl_pmc.c
> >>> index 1dc6e9e..c1170f7 100644
> >>> --- a/arch/powerpc/sysdev/fsl_pmc.c
> >>> +++ b/arch/powerpc/sysdev/fsl_pmc.c
> >>> @@ -34,6 +34,7 @@ struct pmc_regs {
> >>>  	__be32 powmgtcsr;
> >>>  #define POWMGTCSR_SLP		0x00020000
> >>>  #define POWMGTCSR_DPSLP		0x00100000
> >>> +#define POWMGTCSR_LOSSLESS	0x00400000
> >>>  	__be32 res3[2];
> >>>  	__be32 pmcdr;
> >>>  };
> >>> @@ -43,6 +44,74 @@ static unsigned int pmc_flag;
> >>>
> >>>  #define PMC_SLEEP	0x1
> >>>  #define PMC_DEEP_SLEEP	0x2
> >>> +#define PMC_LOSSLESS	0x4
> >>> +
> >>> +/**
> >>> + * mpc85xx_pmc_set_wake - enable devices as wakeup event source
> >>> + * @pdev: platform device affected
> >>> + * @enable: True to enable event generation; false to disable
> >>> + *
> >>> + * This enables the device as a wakeup event source, or disables it.
> >>> + *
> >>> + * RETURN VALUE:
> >>> + * 0 is returned on success
> >>> + * -EINVAL is returned if device is not supposed to wake up the
> system
> >>> + * Error code depending on the platform is returned if both the
> platform and
> >>> + * the native mechanism fail to enable the generation of wake-up
> events
> >>> + */
> >>> +int mpc85xx_pmc_set_wake(struct platform_device *pdev, bool enable)
> >>
> >> Why does it have to be a platform_device?  Would a bare device_node
> work
> >> here?  If it's for stuff like device_may_wakeup() that could be in a
> >> platform_device wrapper function.
> >
> > It does not have to be a platform_device. I think it can be a struct
> device.
>=20
> Why does it even need that?  The low level mechanism for influencing
> PMCDR should only need a device node, not a Linux device struct.

It does no harm to pass the device structure and makes more sense for objec=
t oriented interface design.=20

>=20
> >> Where does this get called from?  I don't see an example user in this
> >> patchset.
> >
> > It will be used by a gianfar related patch. I plan to submit that patch
> > after these patches accepted.
>=20
> It would be nice to see how this is used when reviewing this.
>=20
> >>> +{
> >>> +	int ret =3D 0;
> >>> +	struct device_node *clk_np;
> >>> +	u32 *prop;
> >>> +	u32 pmcdr_mask;
> >>> +
> >>> +	if (!pmc_regs) {
> >>> +		pr_err("%s: PMC is unavailable\n", __func__);
> >>> +		return -ENODEV;
> >>> +	}
> >>> +
> >>> +	if (enable && !device_may_wakeup(&pdev->dev))
> >>> +		return -EINVAL;
> >>
> >> Who is setting can_wakeup for these devices?
> >
> > The device driver is responsible to set can_wakeup.
>=20
> How would the device driver know how to set it?  Wouldn't this depend on
> the particular SoC and low power mode?

It is based on the "fsl,magic-packet" and "fsl,wake-on-filer" device tree p=
roperties.

Leo

^ permalink raw reply

* Re: [PATCH v5 1/5] powerpc/85xx: implement hardware timebase sync
From: Zhao Chenhui @ 2012-06-05  9:08 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <4FC8E250.9090000@freescale.com>

On Fri, Jun 01, 2012 at 10:40:00AM -0500, Scott Wood wrote:
> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
> >  #ifdef CONFIG_KEXEC
> > +static struct ccsr_guts __iomem *guts;
> > +static u64 timebase;
> > +static int tb_req;
> > +static int tb_valid;
> > +
> > +static void mpc85xx_timebase_freeze(int freeze)
> 
> Why is this under CONFIG_KEXEC?  It'll also be needed for CPU hotplug.

Yes, the timebase sync is also needed for CPU hotplug, but this patch is unrelated to CPU hotplug.
I added CONFIG_HOTPLUG_CPU in the next patch.

> 
> > +{
> > +	unsigned int mask;
> > +
> > +	if (!guts)
> > +		return;
> > +
> > +	mask = CCSR_GUTS_DEVDISR_TB0 | CCSR_GUTS_DEVDISR_TB1;
> > +	if (freeze)
> > +		setbits32(&guts->devdisr, mask);
> > +	else
> > +		clrbits32(&guts->devdisr, mask);
> > +
> > +	in_be32(&guts->devdisr);
> > +}
> > +
> > +static void mpc85xx_give_timebase(void)
> > +{
> > +	unsigned long flags;
> > +
> > +	local_irq_save(flags);
> > +
> > +	while (!tb_req)
> > +		barrier();
> > +	tb_req = 0;
> > +
> > +	mpc85xx_timebase_freeze(1);
> > +	timebase = get_tb();
> > +	mb();
> > +	tb_valid = 1;
> > +
> > +	while (tb_valid)
> > +		barrier();
> > +
> > +	mpc85xx_timebase_freeze(0);
> > +
> > +	local_irq_restore(flags);
> > +}
> > +
> > +static void mpc85xx_take_timebase(void)
> > +{
> > +	unsigned long flags;
> > +
> > +	local_irq_save(flags);
> > +
> > +	tb_req = 1;
> > +	while (!tb_valid)
> > +		barrier();
> > +
> > +	set_tb(timebase >> 32, timebase & 0xffffffff);
> > +	mb();
> > +	tb_valid = 0;
> > +
> > +	local_irq_restore(flags);
> > +}
> 
> I know you say this is for dual-core chips only, but it would be nice if
> you'd write this in a way that doesn't assume that (even if the
> corenet-specific timebase freezing comes later).

At this point, I have not thought about how to implement the cornet-specific timebase freezing.

> 
> Do we need an isync after setting the timebase, to ensure it's happened
> before we enable the timebase?  Likewise, do we need a readback after
> disabling the timebase to ensure it's disabled before we read the
> timebase in give_timebase?

I checked the e500 core manual (Chapter 2.16 Synchronization Requirements for SPRs).
Only some SPR registers need an isync. The timebase registers do not.

I did a readback in mpc85xx_timebase_freeze().

> 
> >  atomic_t kexec_down_cpus = ATOMIC_INIT(0);
> >  
> >  void mpc85xx_smp_kexec_cpu_down(int crash_shutdown, int secondary)
> > @@ -228,6 +286,20 @@ smp_85xx_setup_cpu(int cpu_nr)
> >  		doorbell_setup_this_cpu();
> >  }
> >  
> > +#ifdef CONFIG_KEXEC
> > +static const struct of_device_id guts_ids[] = {
> > +	{ .compatible = "fsl,mpc8572-guts", },
> > +	{ .compatible = "fsl,mpc8560-guts", },
> > +	{ .compatible = "fsl,mpc8536-guts", },
> > +	{ .compatible = "fsl,p1020-guts", },
> > +	{ .compatible = "fsl,p1021-guts", },
> > +	{ .compatible = "fsl,p1022-guts", },
> > +	{ .compatible = "fsl,p1023-guts", },
> > +	{ .compatible = "fsl,p2020-guts", },
> > +	{},
> > +};
> > +#endif
> 
> MPC8560 and MPC8536 are single-core...

Thanks. I will remove them.

> 
> Also please use a more specific name, such as e500v2_smp_guts_ids or
> mpc85xx_smp_guts_ids -- when corenet support is added it will likely be
> in the same file.
> 
> >  void __init mpc85xx_smp_init(void)
> >  {
> >  	struct device_node *np;
> > @@ -249,6 +321,19 @@ void __init mpc85xx_smp_init(void)
> >  		smp_85xx_ops.cause_ipi = doorbell_cause_ipi;
> >  	}
> >  
> > +#ifdef CONFIG_KEXEC
> > +	np = of_find_matching_node(NULL, guts_ids);
> > +	if (np) {
> > +		guts = of_iomap(np, 0);
> > +		smp_85xx_ops.give_timebase = mpc85xx_give_timebase;
> > +		smp_85xx_ops.take_timebase = mpc85xx_take_timebase;
> > +		of_node_put(np);
> > +	} else {
> > +		smp_85xx_ops.give_timebase = smp_generic_give_timebase;
> > +		smp_85xx_ops.take_timebase = smp_generic_take_timebase;
> > +	}
> 
> Do not use smp_generic_give/take_timebase, ever.  If you don't have the
> guts node, then just assume the timebase is already synced.
> 
> -Scott

smp_generic_give/take_timebase is the default in KEXEC before.
If do not set them, it may make KEXEC fail on other platforms.

-Chenhui

^ permalink raw reply

* RE: Cannot boot an kernel version other than 2.6.35 on P2020RDB-PCA
From: MAUSSIRE Cedric @ 2012-06-05  9:16 UTC (permalink / raw)
  To: Tang Yuantian-B29983, linuxppc-dev
In-Reply-To: <D07C73A334FF604B95B3CBD2A545D07B079A63F1@039-SN2MPN1-012.039d.mgd.msft.net>

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

Hi Yuantian

Thanks for the tip, it's working fine on kernel 3.4.

And yes the board works on linux kernel 2.6.35. I have downloaded the mainline code on kernel.org and use the "qoriq_sdk_defconfig" provided with system builder of Freescale, but it is only working with this defconfig file and not with the default one "mpc85xx_defconfig"....

Best regards,

Cedric


-----Original Message-----
From: Tang Yuantian-B29983 [mailto:B29983@freescale.com]
Sent: Tue 6/5/2012 4:28 AM
To: MAUSSIRE Cedric; linuxppc-dev@lists.ozlabs.org
Subject: RE: Cannot boot an kernel version other than 2.6.35 on P2020RDB-PCA
 
Hi  Cedric MAUSSIRE,

P2020RDB-PCA Board gets supported since kernel 3.3 in mainline code.
Are you sure the board works on kernel 2.6.35?

Regards,
Yuantian

From: linuxppc-dev-bounces+b29983=freescale.com@lists.ozlabs.org [mailto:linuxppc-dev-bounces+b29983=freescale.com@lists.ozlabs.org] On Behalf Of MAUSSIRE Cedric
Sent: 2012?6?4? 23:06
To: linuxppc-dev@lists.ozlabs.org
Subject: Cannot boot an kernel version other than 2.6.35 on P2020RDB-PCA


Hello all,

I am currently working on the Freescale P2020RDB-PCA Board, which has been delivred with the linux-2.6.35 kernel. I succeed in re-generating my own uImage in 2.6.35 thanks to ELDK, but when I want to move on to a newer or older version of linux, the system does not respond any longer just after the boot whith the nfsboot command, the logs are :

## Booting kernel from Legacy Image at 01000000 ...
   Image Name:   Linux-2.6.39
   Created:      2012-05-29   8:24:54 UTC
   Image Type:   PowerPC Linux Kernel Image (gzip compressed)
   Data Size:    3514882 Bytes = 3.4 MiB
   Load Address: 00000000
   Entry Point:  00000000
   Verifying Checksum ... OK
## Flattened Device Tree blob at 00c00000
   Booting using the fdt blob at 0xc00000
   Uncompressing Kernel Image ... OK
   Loading Device Tree to 00ffa000, end 00fffd4a ... OK

NB : nfsboot=setenv bootargs root=/dev/nfs rw nfsroot=192.168.1.1:$rootpath ip=192.168.1.2:192.168.1.1:192.168.1.1:255.255.255.0:P2020RDB-PC:eth0:off console=ttyS0,115200

I have unsuccessfully tried to rebuild the device tree blob and use the default kernel configuration "mpc85xx_defconfig" that comes with the mainline code.

My systems seems to be linux-2.6.35 dependent, I do not known how it can be possible, because I can use the kernel.org source of the 2.6.35 whith no problems but when I use the 3.0, for example, with the same configuration but it does not work.


Best regards,


Cedric MAUSSIRE


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

^ permalink raw reply

* Re: kernel panic during kernel module load (powerpc specific part)
From: Gabriel Paubert @ 2012-06-05 10:44 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Wrobel Heinz-R39252, Michael Ellerman, Steffen Rumler,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1338847242.7150.83.camel@pasglop>

On Tue, Jun 05, 2012 at 08:00:42AM +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2012-06-04 at 13:03 +0200, Gabriel Paubert wrote:
> > There is no conflict to the ABI. These functions are supposed to be
> > directly reachable from whatever code
> > section may need them.
> > 
> > Now I have a question: how did you get the need for this?
> > 
> > None of my kernels uses them:
> > - if I compile with -O2, the compiler simply expands epilogue and
> > prologue to series of lwz and stw
> > - if I compile with -Os, the compiler generates lmw/stmw which give
> > the smallest possible cache footprint
> > 
> > Neither did I find a single reference to these functions in several
> > systems that I grepped for.
> 
> Newer gcc's ... even worse, with -Os, it does it for a single register
> spill afaik. At least that's how I hit it with grub2 using whatever gcc
> is in fc17.

Ok, it's beyond stupid, and at this point must be considered a gcc bug.

	Gabriel

^ permalink raw reply

* Re: [PATCH v5 5/5] powerpc/85xx: add support to JOG feature using cpufreq interface
From: Zhao Chenhui @ 2012-06-05 10:59 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <4FC950AF.90007@freescale.com>

On Fri, Jun 01, 2012 at 06:30:55PM -0500, Scott Wood wrote:
> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
> > Some 85xx silicons like MPC8536 and P1022 have a JOG feature, which provides
> > a dynamic mechanism to lower or raise the CPU core clock at runtime.
> 
> Is there a reason P1023 isn't supported?

P1023 support is deferred.

> 
> > This patch adds the support to change CPU frequency using the standard
> > cpufreq interface. The ratio CORE to CCB can be 1:1(except MPC8536), 3:2,
> > 2:1, 5:2, 3:1, 7:2 and 4:1.
> > 
> > Two CPU cores on P1022 must not in the low power state during the frequency
> > transition. The driver uses a atomic counter to meet the requirement.
> > 
> > The jog mode frequency transition process on the MPC8536 is similar to
> > the deep sleep process. The driver need save the CPU state and restore
> > it after CPU warm reset.
> > 
> > Note:
> >  * The I/O peripherals such as PCIe and eTSEC may lose packets during
> >    the jog mode frequency transition.
> 
> That might be acceptable for eTSEC, but it is not acceptable to lose
> anything on PCIe.  Especially not if you're going to make this "default y".

It is a hardware limitation. Peripherals in the platform will not be operating
during the jog mode frequency transition process.

I can make it "default n".

> 
> 
> > +static int mpc85xx_job_probe(struct platform_device *ofdev)
> 
> Job?

Sorry, It should be "jog".

> 
> > +{
> > +	struct device_node *np = ofdev->dev.of_node;
> > +	unsigned int svr;
> > +
> > +	if (of_device_is_compatible(np, "fsl,mpc8536-guts")) {
> > +		svr = mfspr(SPRN_SVR);
> > +		if ((svr & 0x7fff) == 0x10) {
> > +			pr_err("MPC8536 Rev 1.0 do not support JOG.\n");
> > +			return -ENODEV;
> > +		}
> 
> s/do not support JOG/does not support cpufreq/
> 
> > +		mpc85xx_freqs = mpc8536_freqs_table;
> > +		set_pll = mpc8536_set_pll;
> > +	} else if (of_device_is_compatible(np, "fsl,p1022-guts")) {
> > +		mpc85xx_freqs = p1022_freqs_table;
> > +		set_pll = p1022_set_pll;
> > +	} else {
> > +		return -ENODEV;
> > +	}
> > +
> > +	sysfreq = fsl_get_sys_freq();
> > +
> > +	guts = of_iomap(np, 0);
> > +	if (!guts)
> > +		return -ENODEV;
> > +
> > +	max_pll[0] = get_pll(0);
> > +	if (mpc85xx_freqs == p1022_freqs_table)
> > +		max_pll[1] = get_pll(1);
> > +
> > +	pr_info("Freescale MPC85xx CPU frequency switching(JOG) driver\n");
> > +
> > +	return cpufreq_register_driver(&mpc85xx_cpufreq_driver);
> > +}
> > +
> > +static int mpc85xx_jog_remove(struct platform_device *ofdev)
> > +{
> > +	iounmap(guts);
> > +	cpufreq_unregister_driver(&mpc85xx_cpufreq_driver);
> > +
> > +	return 0;
> > +}
> > +
> > +static struct of_device_id mpc85xx_jog_ids[] = {
> > +	{ .compatible = "fsl,mpc8536-guts", },
> > +	{ .compatible = "fsl,p1022-guts", },
> > +	{}
> > +};
> > +
> > +static struct platform_driver mpc85xx_jog_driver = {
> > +	.driver = {
> > +		.name = "mpc85xx_cpufreq_jog",
> > +		.owner = THIS_MODULE,
> > +		.of_match_table = mpc85xx_jog_ids,
> > +	},
> > +	.probe = mpc85xx_job_probe,
> > +	.remove = mpc85xx_jog_remove,
> > +};
> 
> Why is this a separate driver from fsl_pmc.c?
> 
> Only one driver can bind to a node through normal mechanisms -- you
> don't get to take the entire guts node for this.

You are right. I will not bind this to the guts node.

> 
> > +static int __init mpc85xx_jog_init(void)
> > +{
> > +	return platform_driver_register(&mpc85xx_jog_driver);
> > +}
> > +
> > +static void __exit mpc85xx_jog_exit(void)
> > +{
> > +	platform_driver_unregister(&mpc85xx_jog_driver);
> > +}
> > +
> > +module_init(mpc85xx_jog_init);
> > +module_exit(mpc85xx_jog_exit);
> > +
> > +MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Dave Liu <daveliu@freescale.com>");
> > diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
> > index a35ca44..445bedd 100644
> > --- a/arch/powerpc/platforms/Kconfig
> > +++ b/arch/powerpc/platforms/Kconfig
> > @@ -204,6 +204,17 @@ config CPU_FREQ_PMAC64
> >  	  This adds support for frequency switching on Apple iMac G5,
> >  	  and some of the more recent desktop G5 machines as well.
> >  
> > +config MPC85xx_CPUFREQ
> > +	bool "Support for Freescale MPC85xx CPU freq"
> > +	depends on PPC_85xx && PPC32 && !PPC_E500MC
> 
> PPC32 is redundant given the !PPC_E500MC.

Yes.

> 
> > index 8976534..401cac0 100644
> > --- a/arch/powerpc/sysdev/fsl_soc.h
> > +++ b/arch/powerpc/sysdev/fsl_soc.h
> > @@ -62,5 +62,10 @@ void fsl_hv_halt(void);
> >   * code can be compatible with both 32-bit & 36-bit.
> >   */
> >  extern void mpc85xx_enter_deep_sleep(u64 ccsrbar, u32 powmgtreq);
> > +
> > +static inline void mpc85xx_enter_jog(u64 ccsrbar, u32 powmgtreq)
> > +{
> > +	mpc85xx_enter_deep_sleep(ccsrbar, powmgtreq);
> > +}
> 
> What value is this function adding over mpc85xx_enter_deep_sleep()?

Just an alias name. If this is improper, I could use mpc85xx_enter_deep_sleep() directly.

-Chenhui

^ permalink raw reply

* Re: [PATCH v5 2/5] powerpc/85xx: add HOTPLUG_CPU support
From: Zhao Chenhui @ 2012-06-05 11:18 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <4FCCE32F.1060301@freescale.com>

On Mon, Jun 04, 2012 at 11:32:47AM -0500, Scott Wood wrote:
> On 06/04/2012 06:04 AM, Zhao Chenhui wrote:
> > On Fri, Jun 01, 2012 at 04:27:27PM -0500, Scott Wood wrote:
> >> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
> >>> -#ifdef CONFIG_KEXEC
> >>> +#if defined(CONFIG_KEXEC) || defined(CONFIG_HOTPLUG_CPU)
> >>
> >> Let's not grow lists like this.  Is there any harm in building it
> >> unconditionally?
> >>
> >> -Scott
> > 
> > We need this ifdef. We only set give_timebase/take_timebase
> > when CONFIG_KEXEC or CONFIG_HOTPLUG_CPU is defined.
> 
> If we really need this to be a compile-time decision, make a new symbol
> for it, but I really think this should be decided at runtime.  Just
> because we have kexec or hotplug support enabled doesn't mean that's
> actually what we're doing at the moment.
> 
> -Scott

If user does not enable kexec or hotplug, these codes are redundant.
So use CONFIG_KEXEC and CONFIG_HOTPLUG_CPU to gard them.

-Chenhui

^ permalink raw reply

* Re: kernel panic during kernel module load (powerpc specific part)
From: Gabriel Paubert @ 2012-06-05 11:32 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Wrobel Heinz-R39252, Michael Ellerman, Steffen Rumler,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1338847242.7150.83.camel@pasglop>

On Tue, Jun 05, 2012 at 08:00:42AM +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2012-06-04 at 13:03 +0200, Gabriel Paubert wrote:
> > There is no conflict to the ABI. These functions are supposed to be
> > directly reachable from whatever code
> > section may need them.
> > 
> > Now I have a question: how did you get the need for this?
> > 
> > None of my kernels uses them:
> > - if I compile with -O2, the compiler simply expands epilogue and
> > prologue to series of lwz and stw
> > - if I compile with -Os, the compiler generates lmw/stmw which give
> > the smallest possible cache footprint
> > 
> > Neither did I find a single reference to these functions in several
> > systems that I grepped for.
> 
> Newer gcc's ... even worse, with -Os, it does it for a single register
> spill afaik. At least that's how I hit it with grub2 using whatever gcc
> is in fc17.

Well, I've not yet been able to make it call the save/restore routines,
but here on a machine with Debian testing and several gcc installed:

- gcc-4.4 never generates lmw/stmw, it always uses individual
 lwz/stw whatever options are set (-Os -mmultiple).

- gcc-4.6 and gcc-4.7 behave identically, if -Os is set, they
  generate by default lmw/stmw. But if I combine -Os with 
  -mno-multiple, they call the helper functions.

In other words, on this system, gcc-4.4 is broken but should not
cause any harm. gcc-4.6 and gcc-4.7 look correct, but are there
any processors on which -mno-multiple is worth setting?

	Regards,
	Gabriel

^ permalink raw reply

* Re: [PATCH v5 3/5] powerpc/85xx: add sleep and deep sleep support
From: Zhao Chenhui @ 2012-06-05 11:35 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <4FCD3D9E.9010509@freescale.com>

On Mon, Jun 04, 2012 at 05:58:38PM -0500, Scott Wood wrote:
> On 06/04/2012 06:12 AM, Zhao Chenhui wrote:
> > On Fri, Jun 01, 2012 at 04:54:35PM -0500, Scott Wood wrote:
> >> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
> >>> diff --git a/arch/powerpc/include/asm/cacheflush.h b/arch/powerpc/include/asm/cacheflush.h
> >>> index 94ec20a..baa000c 100644
> >>> --- a/arch/powerpc/include/asm/cacheflush.h
> >>> +++ b/arch/powerpc/include/asm/cacheflush.h
> >>> @@ -33,6 +33,11 @@ extern void flush_dcache_page(struct page *page);
> >>>  #if defined(CONFIG_FSL_BOOKE) || defined(CONFIG_6xx)
> >>>  extern void __flush_disable_L1(void);
> >>>  #endif
> >>> +#if defined(CONFIG_FSL_BOOKE)
> >>> +extern void flush_dcache_L1(void);
> >>> +#else
> >>> +#define flush_dcache_L1()	do { } while (0)
> >>> +#endif
> >>
> >> It doesn't seem right to no-op this on other platforms.
> > 
> > The pmc_suspend_enter() in fsl_pmc.c used by mpc85xx and mpc86xx,
> > but flush_dcache_L1() have no definition in mpc86xx platform.
> > I will write flush_dcache_L1() for mpc86xx platform.
> 
> How about only calling the function when it's needed?  If we didn't need
> an L1 flush here on 86xx before, why do we need it now?

How about using CONFIG_PPC_85xx to gard it, like this.

	case PM_SUSPEND_STANDBY:
		local_irq_disable();
#ifdef CONFIG_PPC_85xx
		flush_dcache_L1();
#endif
		setbits32(&pmc_regs->powmgtcsr, POWMGTCSR_SLP);

> 
> >>>  extern void __flush_icache_range(unsigned long, unsigned long);
> >>>  static inline void flush_icache_range(unsigned long start, unsigned long stop)
> >>> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> >>> index f5808a3..cb70dba 100644
> >>> --- a/arch/powerpc/kernel/Makefile
> >>> +++ b/arch/powerpc/kernel/Makefile
> >>> @@ -64,6 +64,9 @@ obj-$(CONFIG_FA_DUMP)		+= fadump.o
> >>>  ifeq ($(CONFIG_PPC32),y)
> >>>  obj-$(CONFIG_E500)		+= idle_e500.o
> >>>  endif
> >>> +ifneq ($(CONFIG_PPC_E500MC),y)
> >>> +obj-$(CONFIG_PPC_85xx)		+= l2cache_85xx.o
> >>> +endif
> >>
> >> Can we introduce a symbol that specifically means pre-e500mc e500,
> >> rather than using negative logic?
> >>
> >> I think something like CONFIG_PPC_E500_V1_V2 has been proposed before.
> > 
> > Agree. But CONFIG_PPC_E500_V1_V2 haven't been merged.
> 
> Has the concept been NACKed, or just forgotten?  If the latter, you
> could include it in this patchset.
> 
> -Scott

In patchwork, it's state is "Superseded".
http://patchwork.ozlabs.org/patch/124284/

-Chenhui

^ permalink raw reply

* [PATCH] powerpc: Fix assmption of end_of_DRAM() returns end address
From: Bharat Bhushan @ 2012-06-05 13:55 UTC (permalink / raw)
  To: linuxppc-dev, linux-kernel, galak, benh; +Cc: Bharat Bhushan

memblock_end_of_DRAM() returns end_address + 1, not end address.
While some code assumes that it returns end address.

Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
---
This patch is based on next branch of https://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git

 arch/powerpc/platforms/44x/currituck.c     |    2 +-
 arch/powerpc/platforms/85xx/corenet_ds.c   |    2 +-
 arch/powerpc/platforms/85xx/ge_imp3a.c     |    2 +-
 arch/powerpc/platforms/85xx/mpc8536_ds.c   |    2 +-
 arch/powerpc/platforms/85xx/mpc85xx_ds.c   |    2 +-
 arch/powerpc/platforms/85xx/mpc85xx_mds.c  |    2 +-
 arch/powerpc/platforms/85xx/p1022_ds.c     |    2 +-
 arch/powerpc/platforms/86xx/mpc86xx_hpcn.c |    2 +-
 8 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/platforms/44x/currituck.c b/arch/powerpc/platforms/44x/currituck.c
index 583e67f..9f6c33d 100644
--- a/arch/powerpc/platforms/44x/currituck.c
+++ b/arch/powerpc/platforms/44x/currituck.c
@@ -160,7 +160,7 @@ static void __init ppc47x_setup_arch(void)
 	/* No need to check the DMA config as we /know/ our windows are all of
  	 * RAM.  Lets hope that doesn't change */
 #ifdef CONFIG_SWIOTLB
-	if (memblock_end_of_DRAM() > 0xffffffff) {
+	if ((memblock_end_of_DRAM() - 1) > 0xffffffff) {
 		ppc_swiotlb_enable = 1;
 		set_pci_dma_ops(&swiotlb_dma_ops);
 		ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb;
diff --git a/arch/powerpc/platforms/85xx/corenet_ds.c b/arch/powerpc/platforms/85xx/corenet_ds.c
index dd3617c..925b028 100644
--- a/arch/powerpc/platforms/85xx/corenet_ds.c
+++ b/arch/powerpc/platforms/85xx/corenet_ds.c
@@ -77,7 +77,7 @@ void __init corenet_ds_setup_arch(void)
 #endif
 
 #ifdef CONFIG_SWIOTLB
-	if (memblock_end_of_DRAM() > max) {
+	if ((memblock_end_of_DRAM() - 1) > max) {
 		ppc_swiotlb_enable = 1;
 		set_pci_dma_ops(&swiotlb_dma_ops);
 		ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb;
diff --git a/arch/powerpc/platforms/85xx/ge_imp3a.c b/arch/powerpc/platforms/85xx/ge_imp3a.c
index 1801462..b6a728b 100644
--- a/arch/powerpc/platforms/85xx/ge_imp3a.c
+++ b/arch/powerpc/platforms/85xx/ge_imp3a.c
@@ -125,7 +125,7 @@ static void __init ge_imp3a_setup_arch(void)
 	mpc85xx_smp_init();
 
 #ifdef CONFIG_SWIOTLB
-	if (memblock_end_of_DRAM() > max) {
+	if ((memblock_end_of_DRAM() - 1) > max) {
 		ppc_swiotlb_enable = 1;
 		set_pci_dma_ops(&swiotlb_dma_ops);
 		ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb;
diff --git a/arch/powerpc/platforms/85xx/mpc8536_ds.c b/arch/powerpc/platforms/85xx/mpc8536_ds.c
index 585bd22..767c7cf 100644
--- a/arch/powerpc/platforms/85xx/mpc8536_ds.c
+++ b/arch/powerpc/platforms/85xx/mpc8536_ds.c
@@ -75,7 +75,7 @@ static void __init mpc8536_ds_setup_arch(void)
 #endif
 
 #ifdef CONFIG_SWIOTLB
-	if (memblock_end_of_DRAM() > max) {
+	if ((memblock_end_of_DRAM() - 1) > max) {
 		ppc_swiotlb_enable = 1;
 		set_pci_dma_ops(&swiotlb_dma_ops);
 		ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb;
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ds.c b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
index 1fd91e9..d30f6c4 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_ds.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_ds.c
@@ -173,7 +173,7 @@ static void __init mpc85xx_ds_setup_arch(void)
 	mpc85xx_smp_init();
 
 #ifdef CONFIG_SWIOTLB
-	if (memblock_end_of_DRAM() > max) {
+	if ((memblock_end_of_DRAM() - 1) > max) {
 		ppc_swiotlb_enable = 1;
 		set_pci_dma_ops(&swiotlb_dma_ops);
 		ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb;
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
index d208ebc..8e4b094 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
@@ -359,7 +359,7 @@ static void __init mpc85xx_mds_setup_arch(void)
 	mpc85xx_mds_qe_init();
 
 #ifdef CONFIG_SWIOTLB
-	if (memblock_end_of_DRAM() > max) {
+	if ((memblock_end_of_DRAM() - 1) > max) {
 		ppc_swiotlb_enable = 1;
 		set_pci_dma_ops(&swiotlb_dma_ops);
 		ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb;
diff --git a/arch/powerpc/platforms/85xx/p1022_ds.c b/arch/powerpc/platforms/85xx/p1022_ds.c
index f700c81..74e310b 100644
--- a/arch/powerpc/platforms/85xx/p1022_ds.c
+++ b/arch/powerpc/platforms/85xx/p1022_ds.c
@@ -450,7 +450,7 @@ static void __init p1022_ds_setup_arch(void)
 	mpc85xx_smp_init();
 
 #ifdef CONFIG_SWIOTLB
-	if (memblock_end_of_DRAM() > max) {
+	if ((memblock_end_of_DRAM() - 1) > max) {
 		ppc_swiotlb_enable = 1;
 		set_pci_dma_ops(&swiotlb_dma_ops);
 		ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb;
diff --git a/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c b/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c
index 3755e61..817245b 100644
--- a/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c
+++ b/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c
@@ -102,7 +102,7 @@ mpc86xx_hpcn_setup_arch(void)
 #endif
 
 #ifdef CONFIG_SWIOTLB
-	if (memblock_end_of_DRAM() > max) {
+	if ((memblock_end_of_DRAM() - 1) > max) {
 		ppc_swiotlb_enable = 1;
 		set_pci_dma_ops(&swiotlb_dma_ops);
 		ppc_md.pci_dma_dev_setup = pci_dma_dev_setup_swiotlb;
-- 
1.7.0.4

^ permalink raw reply related

* Re: [PATCH v5 5/5] powerpc/85xx: add support to JOG feature using cpufreq interface
From: Scott Wood @ 2012-06-05 15:58 UTC (permalink / raw)
  To: Zhao Chenhui; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20120605105929.GA22427@localhost.localdomain>

On 06/05/2012 05:59 AM, Zhao Chenhui wrote:
> On Fri, Jun 01, 2012 at 06:30:55PM -0500, Scott Wood wrote:
>> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
>>> The jog mode frequency transition process on the MPC8536 is similar to
>>> the deep sleep process. The driver need save the CPU state and restore
>>> it after CPU warm reset.
>>>
>>> Note:
>>>  * The I/O peripherals such as PCIe and eTSEC may lose packets during
>>>    the jog mode frequency transition.
>>
>> That might be acceptable for eTSEC, but it is not acceptable to lose
>> anything on PCIe.  Especially not if you're going to make this "default y".
> 
> It is a hardware limitation.

Then make sure jog isn't used if PCIe is used.

Maybe you could do something with the suspend infrastructure, but this
is sufficiently heavyweight that transitions should be manually
requested, not triggered by the automatic cpufreq governor.

Does this apply to p1022, or just mpc8536?

> Peripherals in the platform will not be operating
> during the jog mode frequency transition process.

What ensures this?

-Scott

^ permalink raw reply

* Re: [PATCH v5 1/5] powerpc/85xx: implement hardware timebase sync
From: Scott Wood @ 2012-06-05 16:07 UTC (permalink / raw)
  To: Zhao Chenhui; +Cc: Matthew McClintock, linuxppc-dev, linux-kernel
In-Reply-To: <20120605090831.GA21929@localhost.localdomain>

On 06/05/2012 04:08 AM, Zhao Chenhui wrote:
> On Fri, Jun 01, 2012 at 10:40:00AM -0500, Scott Wood wrote:
>> I know you say this is for dual-core chips only, but it would be nice if
>> you'd write this in a way that doesn't assume that (even if the
>> corenet-specific timebase freezing comes later).
> 
> At this point, I have not thought about how to implement the cornet-specific timebase freezing.

I wasn't asking you to.  I was asking you to not have logic that breaks
with more than 2 CPUs.

>> Do we need an isync after setting the timebase, to ensure it's happened
>> before we enable the timebase?  Likewise, do we need a readback after
>> disabling the timebase to ensure it's disabled before we read the
>> timebase in give_timebase?
> 
> I checked the e500 core manual (Chapter 2.16 Synchronization Requirements for SPRs).
> Only some SPR registers need an isync. The timebase registers do not.

I don't trust that, and the consequences of having the sync be imperfect
are too unpleasant to chance it.

> I did a readback in mpc85xx_timebase_freeze().

Sorry, missed that somehow.

>>> +#ifdef CONFIG_KEXEC
>>> +	np = of_find_matching_node(NULL, guts_ids);
>>> +	if (np) {
>>> +		guts = of_iomap(np, 0);
>>> +		smp_85xx_ops.give_timebase = mpc85xx_give_timebase;
>>> +		smp_85xx_ops.take_timebase = mpc85xx_take_timebase;
>>> +		of_node_put(np);
>>> +	} else {
>>> +		smp_85xx_ops.give_timebase = smp_generic_give_timebase;
>>> +		smp_85xx_ops.take_timebase = smp_generic_take_timebase;
>>> +	}
>>
>> Do not use smp_generic_give/take_timebase, ever.  If you don't have the
>> guts node, then just assume the timebase is already synced.
>>
>> -Scott
> 
> smp_generic_give/take_timebase is the default in KEXEC before.

That was a mistake.

> If do not set them, it may make KEXEC fail on other platforms.

What platforms?

-Scott

^ permalink raw reply

* Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup event source
From: Scott Wood @ 2012-06-05 16:11 UTC (permalink / raw)
  To: Li Yang-R58472
  Cc: Wood Scott-B07421, linuxppc-dev@lists.ozlabs.org,
	linux-kernel@vger.kernel.org, Zhao Chenhui-B35336
In-Reply-To: <94F013E7935FF44C83EBE7784D62AD3F09335D27@039-SN2MPN1-022.039d.mgd.msft.net>

On 06/04/2012 11:08 PM, Li Yang-R58472 wrote:
> 
> 
>> -----Original Message-----
>> From: Wood Scott-B07421
>> Sent: Tuesday, June 05, 2012 7:03 AM
>> To: Zhao Chenhui-B35336
>> Cc: linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org;
>> galak@kernel.crashing.org; Li Yang-R58472
>> Subject: Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup
>> event source
>>
>> On 06/04/2012 06:36 AM, Zhao Chenhui wrote:
>>> On Fri, Jun 01, 2012 at 05:08:52PM -0500, Scott Wood wrote:
>>>> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
>>>>> +int mpc85xx_pmc_set_wake(struct platform_device *pdev, bool enable)
>>>>
>>>> Why does it have to be a platform_device?  Would a bare device_node
>> work
>>>> here?  If it's for stuff like device_may_wakeup() that could be in a
>>>> platform_device wrapper function.
>>>
>>> It does not have to be a platform_device. I think it can be a struct
>> device.
>>
>> Why does it even need that?  The low level mechanism for influencing
>> PMCDR should only need a device node, not a Linux device struct.
> 
> It does no harm to pass the device structure and makes more sense for object oriented interface design. 

It does do harm if you don't have a device structure to pass, if for
some reason you found the device by directly looking for it rather than
going through the device model.

>>>> Who is setting can_wakeup for these devices?
>>>
>>> The device driver is responsible to set can_wakeup.
>>
>> How would the device driver know how to set it?  Wouldn't this depend on
>> the particular SoC and low power mode?
> 
> It is based on the "fsl,magic-packet" and "fsl,wake-on-filer" device tree properties.

fsl,magic-packet was a mistake.  It is equivalent to checking the
compatible for etsec.  It does not convey any information about whether
the eTSEC is still active in a given low power mode.

How is fsl,wake-os-filer relevant to this decision?  When will it be set
but not fsl,magic-packet?

What about devices other than ethernet?  What about differences between
ordinary sleep and deep sleep?

-Scott

^ permalink raw reply

* Re: [PATCH v5 3/5] powerpc/85xx: add sleep and deep sleep support
From: Scott Wood @ 2012-06-05 16:13 UTC (permalink / raw)
  To: Zhao Chenhui; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20120605113530.GC22427@localhost.localdomain>

On 06/05/2012 06:35 AM, Zhao Chenhui wrote:
> On Mon, Jun 04, 2012 at 05:58:38PM -0500, Scott Wood wrote:
>> On 06/04/2012 06:12 AM, Zhao Chenhui wrote:
>>> On Fri, Jun 01, 2012 at 04:54:35PM -0500, Scott Wood wrote:
>>>> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
>>>>> diff --git a/arch/powerpc/include/asm/cacheflush.h b/arch/powerpc/include/asm/cacheflush.h
>>>>> index 94ec20a..baa000c 100644
>>>>> --- a/arch/powerpc/include/asm/cacheflush.h
>>>>> +++ b/arch/powerpc/include/asm/cacheflush.h
>>>>> @@ -33,6 +33,11 @@ extern void flush_dcache_page(struct page *page);
>>>>>  #if defined(CONFIG_FSL_BOOKE) || defined(CONFIG_6xx)
>>>>>  extern void __flush_disable_L1(void);
>>>>>  #endif
>>>>> +#if defined(CONFIG_FSL_BOOKE)
>>>>> +extern void flush_dcache_L1(void);
>>>>> +#else
>>>>> +#define flush_dcache_L1()	do { } while (0)
>>>>> +#endif
>>>>
>>>> It doesn't seem right to no-op this on other platforms.
>>>
>>> The pmc_suspend_enter() in fsl_pmc.c used by mpc85xx and mpc86xx,
>>> but flush_dcache_L1() have no definition in mpc86xx platform.
>>> I will write flush_dcache_L1() for mpc86xx platform.
>>
>> How about only calling the function when it's needed?  If we didn't need
>> an L1 flush here on 86xx before, why do we need it now?
> 
> How about using CONFIG_PPC_85xx to gard it, like this.
> 
> 	case PM_SUSPEND_STANDBY:
> 		local_irq_disable();
> #ifdef CONFIG_PPC_85xx
> 		flush_dcache_L1();
> #endif
> 		setbits32(&pmc_regs->powmgtcsr, POWMGTCSR_SLP);

We don't support building 85xx/86xx in the same kernel and likely never
will, so this is OK.

>>>> Can we introduce a symbol that specifically means pre-e500mc e500,
>>>> rather than using negative logic?
>>>>
>>>> I think something like CONFIG_PPC_E500_V1_V2 has been proposed before.
>>>
>>> Agree. But CONFIG_PPC_E500_V1_V2 haven't been merged.
>>
>> Has the concept been NACKed, or just forgotten?  If the latter, you
>> could include it in this patchset.
>>
>> -Scott
> 
> In patchwork, it's state is "Superseded".
> http://patchwork.ozlabs.org/patch/124284/

I still think there's value in adding such a symbol.

-Scott

^ permalink raw reply

* Re: [PATCH v5 2/5] powerpc/85xx: add HOTPLUG_CPU support
From: Scott Wood @ 2012-06-05 16:15 UTC (permalink / raw)
  To: Zhao Chenhui; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20120605111823.GB22427@localhost.localdomain>

On 06/05/2012 06:18 AM, Zhao Chenhui wrote:
> On Mon, Jun 04, 2012 at 11:32:47AM -0500, Scott Wood wrote:
>> On 06/04/2012 06:04 AM, Zhao Chenhui wrote:
>>> On Fri, Jun 01, 2012 at 04:27:27PM -0500, Scott Wood wrote:
>>>> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
>>>>> -#ifdef CONFIG_KEXEC
>>>>> +#if defined(CONFIG_KEXEC) || defined(CONFIG_HOTPLUG_CPU)
>>>>
>>>> Let's not grow lists like this.  Is there any harm in building it
>>>> unconditionally?
>>>>
>>>> -Scott
>>>
>>> We need this ifdef. We only set give_timebase/take_timebase
>>> when CONFIG_KEXEC or CONFIG_HOTPLUG_CPU is defined.
>>
>> If we really need this to be a compile-time decision, make a new symbol
>> for it, but I really think this should be decided at runtime.  Just
>> because we have kexec or hotplug support enabled doesn't mean that's
>> actually what we're doing at the moment.
>>
>> -Scott
> 
> If user does not enable kexec or hotplug, these codes are redundant.
> So use CONFIG_KEXEC and CONFIG_HOTPLUG_CPU to gard them.

My point is that these lists tend to grow and be a maintenance pain.
For small things it's often better to not worry about saving a few
bytes.  For larger things that need to be conditional, define a new
symbol rather than growing ORed lists like this.

-Scott

^ permalink raw reply

* RE: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup event source
From: Li Yang-R58472 @ 2012-06-05 16:49 UTC (permalink / raw)
  To: Wood Scott-B07421
  Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	Zhao Chenhui-B35336
In-Reply-To: <4FCE2FB8.9080307@freescale.com>



> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Wednesday, June 06, 2012 12:12 AM
> To: Li Yang-R58472
> Cc: Wood Scott-B07421; Zhao Chenhui-B35336; linuxppc-dev@lists.ozlabs.org=
;
> linux-kernel@vger.kernel.org; galak@kernel.crashing.org
> Subject: Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup
> event source
>=20
> On 06/04/2012 11:08 PM, Li Yang-R58472 wrote:
> >
> >
> >> -----Original Message-----
> >> From: Wood Scott-B07421
> >> Sent: Tuesday, June 05, 2012 7:03 AM
> >> To: Zhao Chenhui-B35336
> >> Cc: linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org;
> >> galak@kernel.crashing.org; Li Yang-R58472
> >> Subject: Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as
> >> wakeup event source
> >>
> >> On 06/04/2012 06:36 AM, Zhao Chenhui wrote:
> >>> On Fri, Jun 01, 2012 at 05:08:52PM -0500, Scott Wood wrote:
> >>>> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
> >>>>> +int mpc85xx_pmc_set_wake(struct platform_device *pdev, bool
> >>>>> +enable)
> >>>>
> >>>> Why does it have to be a platform_device?  Would a bare device_node
> >> work
> >>>> here?  If it's for stuff like device_may_wakeup() that could be in
> >>>> a platform_device wrapper function.
> >>>
> >>> It does not have to be a platform_device. I think it can be a struct
> >> device.
> >>
> >> Why does it even need that?  The low level mechanism for influencing
> >> PMCDR should only need a device node, not a Linux device struct.
> >
> > It does no harm to pass the device structure and makes more sense for
> object oriented interface design.
>=20
> It does do harm if you don't have a device structure to pass, if for some
> reason you found the device by directly looking for it rather than going
> through the device model.

Whether or not a device is a wakeup source not only depends on the SoC spec=
ification but also the configuration and current state for the device.  I o=
nly expect the device driver to have this knowledge and call this function =
rather than some standalone platform code.  Therefore I don't think your co=
ncern matters.
=20
>=20
> >>>> Who is setting can_wakeup for these devices?
> >>>
> >>> The device driver is responsible to set can_wakeup.
> >>
> >> How would the device driver know how to set it?  Wouldn't this depend
> >> on the particular SoC and low power mode?
> >
> > It is based on the "fsl,magic-packet" and "fsl,wake-on-filer" device
> tree properties.
>=20
> fsl,magic-packet was a mistake.  It is equivalent to checking the
> compatible for etsec.  It does not convey any information about whether

It can be described either by explicit feature property or by the compatibl=
e.  I don't think it is a problem that we choose one against another.

> the eTSEC is still active in a given low power mode.

Whether or not the eTSEC is still active in both sleep and deep sleep is on=
ly depending on if we set it to be a wakeup source.  If it behaves differen=
tly for low power modes in the future, we could address that by adding addi=
tional property.

>=20
> How is fsl,wake-os-filer relevant to this decision?  When will it be set
> but not fsl,magic-packet?

I mean either fsl,magic-packet or fsl,wake-on-filer shows it can be a wakeu=
p source.  Currently we don't have an SoC to have wake-on-filer while not w=
ake-on-magic.  But I think it's better to consider both as they are indepen=
dent features.

>=20
> What about devices other than ethernet?  What about differences between
> ordinary sleep and deep sleep?

There is no difference for sleep and deep sleep for all wakeup sources curr=
ently.  We can address the problem if it is not the case in the future.

Leo

^ permalink raw reply

* Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup event source
From: Scott Wood @ 2012-06-05 18:05 UTC (permalink / raw)
  To: Li Yang-R58472
  Cc: Wood Scott-B07421, linuxppc-dev@lists.ozlabs.org,
	linux-kernel@vger.kernel.org, Zhao Chenhui-B35336
In-Reply-To: <94F013E7935FF44C83EBE7784D62AD3F093360C1@039-SN2MPN1-022.039d.mgd.msft.net>

On 06/05/2012 11:49 AM, Li Yang-R58472 wrote:
> 
> 
>> -----Original Message-----
>> From: Wood Scott-B07421
>> Sent: Wednesday, June 06, 2012 12:12 AM
>> To: Li Yang-R58472
>> Cc: Wood Scott-B07421; Zhao Chenhui-B35336; linuxppc-dev@lists.ozlabs.org;
>> linux-kernel@vger.kernel.org; galak@kernel.crashing.org
>> Subject: Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup
>> event source
>>
>> On 06/04/2012 11:08 PM, Li Yang-R58472 wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Wood Scott-B07421
>>>> Sent: Tuesday, June 05, 2012 7:03 AM
>>>> To: Zhao Chenhui-B35336
>>>> Cc: linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org;
>>>> galak@kernel.crashing.org; Li Yang-R58472
>>>> Subject: Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as
>>>> wakeup event source
>>>>
>>>> On 06/04/2012 06:36 AM, Zhao Chenhui wrote:
>>>>> On Fri, Jun 01, 2012 at 05:08:52PM -0500, Scott Wood wrote:
>>>>>> On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
>>>>>>> +int mpc85xx_pmc_set_wake(struct platform_device *pdev, bool
>>>>>>> +enable)
>>>>>>
>>>>>> Why does it have to be a platform_device?  Would a bare device_node
>>>> work
>>>>>> here?  If it's for stuff like device_may_wakeup() that could be in
>>>>>> a platform_device wrapper function.
>>>>>
>>>>> It does not have to be a platform_device. I think it can be a struct
>>>> device.
>>>>
>>>> Why does it even need that?  The low level mechanism for influencing
>>>> PMCDR should only need a device node, not a Linux device struct.
>>>
>>> It does no harm to pass the device structure and makes more sense for
>> object oriented interface design.
>>
>> It does do harm if you don't have a device structure to pass, if for some
>> reason you found the device by directly looking for it rather than going
>> through the device model.
> 
> Whether or not a device is a wakeup source not only depends on the
> SoC specification but also the configuration and current state for
> the device.  I only expect the device driver to have this knowledge
> and call this function rather than some standalone platform code.
> Therefore I don't think your concern matters.

First, I think it's bad API to force the passing of a higher level
object when a lower level object would suffice (and there are no
legitimate future-proofing or abstraction reasons for hiding the lower
level object).

But regardless, the entity you call a "device driver" may or may not use
the standard driver model (e.g. look at PCI root complexes), and your
assumption about what platform code knows may or may not be correct.  I
could just as well say that only platform code knows about the SoC
clock/power routing during a given low power state.

>>>>>> Who is setting can_wakeup for these devices?
>>>>>
>>>>> The device driver is responsible to set can_wakeup.
>>>>
>>>> How would the device driver know how to set it?  Wouldn't this depend
>>>> on the particular SoC and low power mode?
>>>
>>> It is based on the "fsl,magic-packet" and "fsl,wake-on-filer" device
>> tree properties.
>>
>> fsl,magic-packet was a mistake.  It is equivalent to checking the
>> compatible for etsec.  It does not convey any information about whether
> 
> It can be described either by explicit feature property or by the
> compatible.  I don't think it is a problem that we choose one against
> another.

I do think it's a problem, because it's unnecessarily complicated, and
more error prone (we probably didn't have fsl,magic-packet on all the
SoCs that support it before the .dtsi refactoring).  But my point was
that it says nothing about whether the eTSEC will still be functioning
during a low power state.

>> the eTSEC is still active in a given low power mode.
> 
> Whether or not the eTSEC is still active in both sleep and deep sleep
> is only depending on if we set it to be a wakeup source.

Only because we happen to support eTSEC as a wakeup source on all SoCs.

> If it behaves differently for low power modes in the future, we could
> address that by adding additional property.
> 
>>
>> How is fsl,wake-os-filer relevant to this decision?  When will it be set
>> but not fsl,magic-packet?
> 
> I mean either fsl,magic-packet or fsl,wake-on-filer shows it can be a
> wakeup source.  Currently we don't have an SoC to have wake-on-filer
> while not wake-on-magic.  But I think it's better to consider both as
> they are independent features.

You're not willing to consider an SoC where waking on eTSEC is
unsupported, but you're willing to consider an eTSEC that has
wake-on-filer but magic packet support has for some reason been dropped?

>> What about devices other than ethernet?  What about differences between
>> ordinary sleep and deep sleep?
> 
> There is no difference for sleep and deep sleep for all wakeup sources currently.

I recall being able to wake an mpc85xx chip (maybe mpc8544?) from
ordinary sleep using the DUART (even though the manual suggests that
only external interrupts can be used -- not even eTSEC).  You can't do
that in deep sleep.

You ignored "what about devices other than ethernet".

-Scott

^ permalink raw reply

* Access to http://ozlabs.org/people/dgibson/dldwd/
From: R.P. Burrasca @ 2012-06-05 13:27 UTC (permalink / raw)
  To: linuxppc-dev


[-- Attachment #1.1: Type: text/plain, Size: 849 bytes --]



 

Would it be possible to access the above referenced webpage in order to see
what's available as the Orinoco & Prism 2 Wireless Driver?

 

I have wireless internet access using Ubuntu 10.10 for powerpc on my G3
iBook 500 Mhz dual usb machine while I'm at home but I don't have any
scanning capability and can't tell when I'm near a hotspot or whether are
wireless networks around me when I'm somewhere else.  Unlike the case at
home, it doesn't automatically pickup and use the wireless signal coming
from some other hotspot.

 

I would very much like to use my iBook when I travel, but so far have been
unsuccessful in getting it to pickup a wireless signal anywhere other than
my home network.

 

Any access to the above site that you'd be willing to provide would be
greatly appreciated.

 

Thank you.

 

Sincerely,

 

Ray Burrasca

 


[-- Attachment #1.2: Type: text/html, Size: 6758 bytes --]

[-- Attachment #2: image001.gif --]
[-- Type: image/gif, Size: 2051 bytes --]

^ permalink raw reply

* [PATCH] perf: Don't use SIAR for user addresses
From: Sukadev Bhattiprolu @ 2012-06-05 20:32 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: mpjohn, Anton Blanchard

From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Date: Fri, 1 Jun 2012 20:56:02 -0400
Subject: [PATCH] perf: Don't use SIAR for user-space addresses

With the pipelining on Power7, the SIAR can be off by several instructions
leading to incorrect callgraphs. For user space code at least we can be more
accurate by just using the next instruction pointer.

Based on code/input from Anton Blanchard.

Before this fon Power7, we see callgraphs like:

     1.90%  sprintft  sprintft           [.] do_my_sprintf
               |
               --- 00000011.plt_call.strlen@@GLIBC_2.3+0
                   do_my_sprintf
                   main
                   generic_start_main
                   __libc_start_main
                   (nil)
        ...snip...

              |
               --- __random
                   rand
                  |
                  |--63.16%-- do_my_sprintf
                  |          main
                  |          generic_start_main
                  |          __libc_start_main
                  |          (nil)
                  |
                   --36.84%-- rand
                             do_my_sprintf
                             main
                             generic_start_main
                             __libc_start_main
                             (nil)

which seems to indicate the libc __random() calls do_my_sprintf(), instead
of the other way around.

After the fix, the same trace looks like this:

     4.08%  sprintft  sprintft           [.] do_my_sprintf
            |
            --- do_my_sprintf
                do_my_sprintf
                main
                generic_start_main
                __libc_start_main
                (nil)

Cc: Anton Blanchard <anton@samba.org>
Reported-by: Maynard Johnson <mpjohn@us.ibm.com>
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 arch/powerpc/perf/core-book3s.c |   39 ++++++++++++++++++++++++++++++---------
 1 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 8f84bcb..846bd68 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -116,6 +116,26 @@ static inline void perf_get_data_addr(struct pt_regs *regs, u64 *addrp)
 		*addrp = mfspr(SPRN_SDAR);
 }
 
+static bool mmcra_sihv(unsigned long mmcra)
+{
+	unsigned long sihv = MMCRA_SIHV;
+
+	if (ppmu->flags & PPMU_ALT_SIPR)
+		sihv = POWER6_MMCRA_SIHV;
+
+	return !!(mmcra & sihv);
+}
+
+static bool mmcra_sipr(unsigned long mmcra)
+{
+	unsigned long sipr = MMCRA_SIPR;
+
+	if (ppmu->flags & PPMU_ALT_SIPR)
+		sipr = POWER6_MMCRA_SIPR;
+
+	return !!(mmcra & sipr);
+}
+
 static inline u32 perf_flags_from_msr(struct pt_regs *regs)
 {
 	if (regs->msr & MSR_PR)
@@ -128,8 +148,6 @@ static inline u32 perf_flags_from_msr(struct pt_regs *regs)
 static inline u32 perf_get_misc_flags(struct pt_regs *regs)
 {
 	unsigned long mmcra = regs->dsisr;
-	unsigned long sihv = MMCRA_SIHV;
-	unsigned long sipr = MMCRA_SIPR;
 
 	/* Not a PMU interrupt: Make up flags from regs->msr */
 	if (TRAP(regs) != 0xf00)
@@ -156,15 +174,10 @@ static inline u32 perf_get_misc_flags(struct pt_regs *regs)
 		return PERF_RECORD_MISC_USER;
 	}
 
-	if (ppmu->flags & PPMU_ALT_SIPR) {
-		sihv = POWER6_MMCRA_SIHV;
-		sipr = POWER6_MMCRA_SIPR;
-	}
-
 	/* PR has priority over HV, so order below is important */
-	if (mmcra & sipr)
+	if (mmcra_sipr(mmcra))
 		return PERF_RECORD_MISC_USER;
-	if ((mmcra & sihv) && (freeze_events_kernel != MMCR0_FCHV))
+	if (mmcra_sihv(mmcra) && (freeze_events_kernel != MMCR0_FCHV))
 		return PERF_RECORD_MISC_HYPERVISOR;
 	return PERF_RECORD_MISC_KERNEL;
 }
@@ -1340,6 +1353,14 @@ unsigned long perf_instruction_pointer(struct pt_regs *regs)
 	    !(mmcra & MMCRA_SAMPLE_ENABLE))
 		return regs->nip;
 
+	/*
+	 * With pipelining on P7, addresses in the SIAR can be off by several
+	 * instructions. For user space use the next instruction pointer as
+	 * that will be more accurate.
+	 */
+	if (mmcra_sipr(mmcra))
+		return regs->nip;
+
 	return mfspr(SPRN_SIAR) + perf_ip_adjust(regs);
 }
 
-- 
1.7.1

^ permalink raw reply related

* Re: kernel panic during kernel module load (powerpc specific part)
From: Benjamin Herrenschmidt @ 2012-06-05 22:14 UTC (permalink / raw)
  To: Gabriel Paubert
  Cc: Wrobel Heinz-R39252, Michael Ellerman, Steffen Rumler,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20120605113225.GA11215@visitor2.iram.es>

On Tue, 2012-06-05 at 13:32 +0200, Gabriel Paubert wrote:
> - gcc-4.6 and gcc-4.7 behave identically, if -Os is set, they
>   generate by default lmw/stmw. But if I combine -Os with 
>   -mno-multiple, they call the helper functions.
> 
> In other words, on this system, gcc-4.4 is broken but should not
> cause any harm. gcc-4.6 and gcc-4.7 look correct, but are there
> any processors on which -mno-multiple is worth setting?

It could be an artifact of the -mtune we use, dunno. And yes, I'm pretty
sure some embedded processors don't like multiple load/store.

Cheers,
Ben.

^ 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