RE: [PATCH] powerpc/smp: Fix Non-boot cpus cannot be bring up.
> -Original Message- > From: Michael Ellerman [mailto:m...@ellerman.id.au] > Sent: Tuesday, December 23, 2014 9:01 AM > To: Wang Dongsheng-B40534 > Cc: b...@kernel.crashing.org; Wood Scott-B07421; an...@samba.org; linuxppc- > d...@lists.ozlabs.org > Subject: Re: [PATCH] powerpc/smp: Fix Non-boot cpus cannot be bring up. > > On Mon, 2014-12-22 at 14:38 +0800, Dongsheng Wang wrote: > > From: Wang Dongsheng > > > > Kernel cannot bring up Non-boot cpus always get "Processor xx is stuck". > > this issue bring by http://patchwork.ozlabs.org/patch/418912/ (powerpc: > > Secondary CPUs must set cpu_callin_map after setting active and > > online) We need to take timebase after bootup cpu give the timebase firstly. > > > > When start_secondary, non-boot cpus set cpu_callin_map for boot cpu > > after that boot cpu will give the timebase for non-boot cpu. Otherwise > > non-boot cpus will fall in dead loop to waiting bootup cpu to give > > imebase. > > Right. > > However, doesn't this introduce the possibility that the secondary cpu is up > and > marked online but has an unsynchronised clock? > Yes, right. But Freescale platform boot-cpu will freeze the TB until secondary cpu take the time base, so the clock is synchronized. For generic PowerPC maybe has this issue. So for safe I think we need to set cpu online after synchronized clock. I will update my patch if you agree this way. + if (smp_ops->take_timebase) + smp_ops->take_timebase(); + secondary_cpu_time_init(); + Move set_cpu_online to here. + set_cpu_online(cpu, true); Regards, -Dongsheng ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] powerpc/smp: Fix Non-boot cpus cannot be bring up.
> -Original Message- > From: Michael Ellerman [mailto:m...@ellerman.id.au] > Sent: Tuesday, December 23, 2014 1:46 PM > To: Wang Dongsheng-B40534 > Cc: b...@kernel.crashing.org; Wood Scott-B07421; an...@samba.org; linuxppc- > d...@lists.ozlabs.org > Subject: Re: [PATCH] powerpc/smp: Fix Non-boot cpus cannot be bring up. > > On Tue, 2014-12-23 at 02:41 +, dongsheng.w...@freescale.com wrote: > > > -Original Message- > > > From: Michael Ellerman [mailto:m...@ellerman.id.au] > > > Sent: Tuesday, December 23, 2014 9:01 AM > > > To: Wang Dongsheng-B40534 > > > Cc: b...@kernel.crashing.org; Wood Scott-B07421; an...@samba.org; > > > linuxppc- d...@lists.ozlabs.org > > > Subject: Re: [PATCH] powerpc/smp: Fix Non-boot cpus cannot be bring up. > > > > > > On Mon, 2014-12-22 at 14:38 +0800, Dongsheng Wang wrote: > > > > From: Wang Dongsheng > > > > > > > > Kernel cannot bring up Non-boot cpus always get "Processor xx is stuck". > > > > this issue bring by http://patchwork.ozlabs.org/patch/418912/ (powerpc: > > > > Secondary CPUs must set cpu_callin_map after setting active and > > > > online) We need to take timebase after bootup cpu give the timebase > firstly. > > > > > > > > When start_secondary, non-boot cpus set cpu_callin_map for boot > > > > cpu after that boot cpu will give the timebase for non-boot cpu. > > > > Otherwise non-boot cpus will fall in dead loop to waiting bootup > > > > cpu to give imebase. > > > > > > Right. > > > > > > However, doesn't this introduce the possibility that the secondary > > > cpu is up and marked online but has an unsynchronised clock? > > > Yes, right. But Freescale platform boot-cpu will freeze the TB until > > secondary cpu take the time base, so the clock is synchronized. > > It does the freeze in give_timebase() doesn't it? > > So there's still a window there where the secondary is up & online but hasn't > had it's timebase synchronised, and the primary hasn't frozen the timebase > yet. > So that makes me nervous. > > > For generic PowerPC maybe has this issue. So for safe I think we need > > to set cpu online after synchronized clock. > > > > I will update my patch if you agree this way. > > + if (smp_ops->take_timebase) > > + smp_ops->take_timebase(); > > + secondary_cpu_time_init(); > > + > > Move set_cpu_online to here. > > + set_cpu_online(cpu, true); > > But that reverses the effect of the original patch, which was that we have to > set online *before* we set the callin map. > > > Looking harder at Anton's patch I'm not sure it's right anyway. > > The issue he was trying to fix was that the cpu was online but not active, > which > confused the scheduler. > > I think Anton missed that we have a loop that waits for online at the bottom > of > __cpu_up(): > > /* Wait until cpu puts itself in the online map */ > while (!cpu_online(cpu)) > cpu_relax(); > > > He must have seen a case where that popped due to the cpu being online, but > the > cpu wasn't yet active. > > His patch fixed the problem by ensuring the previous loop that waits for > cpu_callin_map doesn't finish until active & online are set, making the while > loop above a nop. > > So I think we should probably revert Anton's patch and instead change that > while > loop to: > > /* Wait until cpu is online AND active */ > while (!cpu_online(cpu) || !cpu_active(cpu)) > cpu_relax(); > Umm.. Sorry about that...I forgot Anton's patch. But set_cpu_online also set cpu_active_bits, I think this judgment cannot fix Anton's issue. It is actually the effects of the original is the same. Regrads, -Dongsheng ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] powerpc/smp: Fix Non-boot cpus cannot be bring up.
> -Original Message- > From: Wang Dongsheng-B40534 > Sent: Tuesday, December 23, 2014 2:53 PM > To: 'Michael Ellerman' > Cc: b...@kernel.crashing.org; Wood Scott-B07421; an...@samba.org; linuxppc- > d...@lists.ozlabs.org > Subject: RE: [PATCH] powerpc/smp: Fix Non-boot cpus cannot be bring up. > > > > > -Original Message- > > From: Michael Ellerman [mailto:m...@ellerman.id.au] > > Sent: Tuesday, December 23, 2014 1:46 PM > > To: Wang Dongsheng-B40534 > > Cc: b...@kernel.crashing.org; Wood Scott-B07421; an...@samba.org; > > linuxppc- d...@lists.ozlabs.org > > Subject: Re: [PATCH] powerpc/smp: Fix Non-boot cpus cannot be bring up. > > > > On Tue, 2014-12-23 at 02:41 +, dongsheng.w...@freescale.com wrote: > > > > -Original Message- > > > > From: Michael Ellerman [mailto:m...@ellerman.id.au] > > > > Sent: Tuesday, December 23, 2014 9:01 AM > > > > To: Wang Dongsheng-B40534 > > > > Cc: b...@kernel.crashing.org; Wood Scott-B07421; an...@samba.org; > > > > linuxppc- d...@lists.ozlabs.org > > > > Subject: Re: [PATCH] powerpc/smp: Fix Non-boot cpus cannot be bring up. > > > > > > > > On Mon, 2014-12-22 at 14:38 +0800, Dongsheng Wang wrote: > > > > > From: Wang Dongsheng > > > > > > > > > > Kernel cannot bring up Non-boot cpus always get "Processor xx is > > > > > stuck". > > > > > this issue bring by http://patchwork.ozlabs.org/patch/418912/ > > > > > (powerpc: > > > > > Secondary CPUs must set cpu_callin_map after setting active and > > > > > online) We need to take timebase after bootup cpu give the > > > > > timebase > > firstly. > > > > > > > > > > When start_secondary, non-boot cpus set cpu_callin_map for boot > > > > > cpu after that boot cpu will give the timebase for non-boot cpu. > > > > > Otherwise non-boot cpus will fall in dead loop to waiting bootup > > > > > cpu to give imebase. > > > > > > > > Right. > > > > > > > > However, doesn't this introduce the possibility that the secondary > > > > cpu is up and marked online but has an unsynchronised clock? > > > > > Yes, right. But Freescale platform boot-cpu will freeze the TB until > > > secondary cpu take the time base, so the clock is synchronized. > > > > It does the freeze in give_timebase() doesn't it? > > > > So there's still a window there where the secondary is up & online but > > hasn't had it's timebase synchronised, and the primary hasn't frozen the > timebase yet. > > So that makes me nervous. > > > > > For generic PowerPC maybe has this issue. So for safe I think we > > > need to set cpu online after synchronized clock. > > > > > > I will update my patch if you agree this way. > > > + if (smp_ops->take_timebase) > > > + smp_ops->take_timebase(); > > > + secondary_cpu_time_init(); > > > + > > > Move set_cpu_online to here. > > > + set_cpu_online(cpu, true); > > > > But that reverses the effect of the original patch, which was that we > > have to set online *before* we set the callin map. > > > > > > Looking harder at Anton's patch I'm not sure it's right anyway. > > > > The issue he was trying to fix was that the cpu was online but not > > active, which confused the scheduler. > > > > I think Anton missed that we have a loop that waits for online at the > > bottom of > > __cpu_up(): > > > > /* Wait until cpu puts itself in the online map */ > > while (!cpu_online(cpu)) > > cpu_relax(); > > > > > > He must have seen a case where that popped due to the cpu being > > online, but the cpu wasn't yet active. > > > > His patch fixed the problem by ensuring the previous loop that waits > > for cpu_callin_map doesn't finish until active & online are set, > > making the while loop above a nop. > > > > So I think we should probably revert Anton's patch and instead change > > that while loop to: > > > > /* Wait until cpu is online AND active */ > > while (!cpu_online(cpu) || !cpu_active(cpu)) > > cpu_relax(); > > Base on Anton's patch, we should probably change __cup_up. Please comment the changes. --- a/arch/powerpc/kernel/smp.c +++ b
RE: [PATCH v2 1/2] fsl/corenet_generic: add a particular initialization for platform
> -Original Message- > From: Wood Scott-B07421 > Sent: Thursday, April 17, 2014 3:36 AM > To: Wang Dongsheng-B40534 > Cc: Jin Zhengxiong-R64188; haoke...@gmail.com; Kushwaha Prabhakar-B32579; > linuxppc-dev@lists.ozlabs.org > Subject: Re: [PATCH v2 1/2] fsl/corenet_generic: add a particular > initialization > for platform > > On Tue, 2014-04-15 at 21:58 -0500, Wang Dongsheng-B40534 wrote: > > > > > -Original Message- > > > From: Wood Scott-B07421 > > > Sent: Wednesday, April 16, 2014 3:39 AM > > > To: Wang Dongsheng-B40534 > > > Cc: Jin Zhengxiong-R64188; haoke...@gmail.com; Kushwaha > > > Prabhakar-B32579; linuxppc-dev@lists.ozlabs.org > > > Subject: Re: [PATCH v2 1/2] fsl/corenet_generic: add a particular > > > initialization for platform > > > > > > On Tue, 2014-04-15 at 13:53 +0800, Dongsheng Wang wrote: > > > > From: Wang Dongsheng > > > > > > > > Corenet_generic is a generic platform initialization. Those based > > > > on the corenet_generic board maybe need a particular initialize to > > > > enable/set some IP-Blocks. So add "Fix Generic Initialization" to > > > > solve this kind of special cases. > > > > > > I still don't understand what you mean by "fix". What are you > > > fixing, or what is fixed? > > > > > > There is no need for adding an infrastructure layer here. Just add > > > a new piece of code for t104x diu, and have it be called by an > > > appropriate initfunc. > > > > > > > "fix" is means to handle some boards those based on corenet_generic > > config file, But those boards may need some special handle. Perhaps > > these used to handle special feature codes not have an appropriate > > initfunc we cannot *just find* an appropriate place, > > I'm not asking you to "just find" anything. I'm asking you to add an initfunc > in a standalone file. > > > if more and more boards need to do this, at that time maybe *initfunc* > > looks very complicated. > > They would each have their own initfunc. There is no reason to tie this in > with > anything else. > Sorry, if those platforms are using corenet_generic, I don’t see any standalone file for initfunc of platform. That's why I'm adding "fix" layer. BTW, if I missed something about "corenet_generic standalone file" please let me know. > > > > --- a/arch/powerpc/platforms/85xx/Kconfig > > > > +++ b/arch/powerpc/platforms/85xx/Kconfig > > > > @@ -269,6 +269,17 @@ config CORENET_GENERIC > > > > The following boards are supported for both 32bit and 64bit > kernel: > > > > P5020 DS and P5040 DS > > > > > > > > +config FIX_GENERIC_PLATFORM_INIT > > > > + bool "Fix Generic Initialization" > > > > + depends on CORENET_GENERIC > > > > > > Why does this depend on CORENET_GENERIC? > > > > > > > Because CORENET_GENERIC is a multiboards file, This is designed to handle > > this > situation. > > This DIU code is going to be just as applicable to a custom T104x board which > may or may not use CORENET_GENERIC. > "fix" is a middle layer, it's not only for T104xrdb-DIU. > > > > + default y > > > > > > No. > > > > > > > Why not? This will not increase any redundant operations if there is not any > boards need fix. > > You can see my fix.c code. > > default y should not be used for hardware specific code. > fix.c and hardware no relationship at all. :), It's just a software layer. Regards, -Dongsheng > -Scott > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH v5 4/4] powerpc/85xx: add sysfs for pw20 state and altivec idle
> -Original Message-
> From: Bhushan Bharat-R65777
> Sent: Monday, November 11, 2013 12:11 PM
> To: Wang Dongsheng-B40534; Wood Scott-B07421
> Cc: linuxppc-dev@lists.ozlabs.org
> Subject: RE: [PATCH v5 4/4] powerpc/85xx: add sysfs for pw20 state and altivec
> idle
>
> > > Those codes just for discuss with Bharat. He want to make one flow at
> > > "show_pw20_wait_time"/" show_altivec_idle_wait_time" function. If we
> > > do that, we need to initialize pw20_wt/altivec_idle_wt.
> > >
> > I will keep this stuff at
> > "show_pw20_wait_time"/"show_altivec_idle_wait_time"
> > and add a comment before our discussion.
> >
> > /*
> > * If the "value" less than 10, this will overflow.
> > * From benchmark test, the default wait bit will not be set less than
> > 10bit.
> > * Because 10 bit corresponds to the wait entry time is
> > 439375573401999609(ns),
> > * for wait-entry-idle time this value looks too long, and we cannot use
> > those
> > * "long" time as a default wait-entry time. So overflow could not have
> happened
> > * and we use this calculation method to get wait-entry-idle time.
> > */
>
> I think now we will use same calculation code for default value and user set
> value, so adding the comment is not sufficient, we should error out from the
> code if value is less than 10. As default value is not less than 10 so this
> will
> always work with default value but if user tries to set less than 10 then
> error
> out and ask user to try more than 9.
>
Again, once the user has set up a time, the code will go to another branch.
else {
time = pw20_wt;
}
We do so much for this a little function processing is not worth it. If we can't
agree on this sys interface. I will change this sys interface to
show_pw20_wait_bit. :)
code e.g:
static ssize_t show_pw20_wait_bit(struct device *dev,
struct device_attribute *attr, char *buf)
{
u32 entry_bit;
unsigned int cpu = dev->id;
smp_call_function_single(cpu, do_show_pwrmgtcr0, &entry_bit, 1);
entry_bit = (entry_bit & PWRMGTCR0_PW20_ENT) >>
PWRMGTCR0_PW20_ENT_SHIFT;
return sprintf(buf, "%u\n", entry_bit);
}
-dongsheng
> -Bharat
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] powerpc/mpic_timer: fix the time calculation is not accurate
> -Original Message- > From: Wood Scott-B07421 > Sent: Saturday, December 28, 2013 8:00 AM > To: Wang Dongsheng-B40534 > Cc: ga...@kernel.crashing.org; linuxppc-dev@lists.ozlabs.org > Subject: Re: [PATCH] powerpc/mpic_timer: fix the time calculation is not > accurate > > On Mon, 2013-12-23 at 10:33 +0800, Dongsheng Wang wrote: > > From: Wang Dongsheng > > > > When the timer GTCCR toggle bit is inverted, we calculated the rest of > > the time is not accurate. So we need to ignore this bit. > > > > Signed-off-by: Wang Dongsheng > > > > diff --git a/arch/powerpc/sysdev/mpic_timer.c > > b/arch/powerpc/sysdev/mpic_timer.c > > index 22d7d57..0fb70c9 100644 > > --- a/arch/powerpc/sysdev/mpic_timer.c > > +++ b/arch/powerpc/sysdev/mpic_timer.c > > @@ -41,6 +41,7 @@ > > #define MPIC_TIMER_TCR_ROVR_OFFSET 24 > > > > #define TIMER_STOP 0x8000 > > +#define GTCCR_TOG 0x8000 > > #define TIMERS_PER_GROUP 4 > > #define MAX_TICKS (~0U >> 1) > > #define MAX_TICKS_CASCADE (~0U) > > @@ -96,8 +97,15 @@ static void convert_ticks_to_time(struct timer_group_priv > *priv, > > time->tv_sec = (__kernel_time_t)div_u64(ticks, priv->timerfreq); > > tmp_sec = (u64)time->tv_sec * (u64)priv->timerfreq; > > > > - time->tv_usec = (__kernel_suseconds_t) > > - div_u64((ticks - tmp_sec) * 100, priv->timerfreq); > > + time->tv_usec = 0; > > + > > + /* > > +* In some cases tmp_sec may be greater than ticks, because in the > > +* process of calculation ticks and tmp_sec will be rounded. > > +*/ > > + if (tmp_sec <= ticks) > > + time->tv_usec = (__kernel_suseconds_t) > > + div_u64((ticks - tmp_sec) * 100, priv->timerfreq); > > I don't see how this part of the patch relates to the patch description. > Thanks, I miss this description, :(. I should split the patch. -Dongsheng > -Scott > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH v6 1/4] powerpc/fsl: add E6500 PVR and SPRN_PWRMGTCR0 define
Hi Scott, Could you apply these patches? [v6,1/4] powerpc/fsl: add E6500 PVR and SPRN_PWRMGTCR0 define, http://patchwork.ozlabs.org/patch/302045/ [v6,2/4] powerpc/85xx: add hardware automatically enter altivec idle state, http://patchwork.ozlabs.org/patch/302046/ [v6,3/4] powerpc/85xx: add hardware automatically enter pw20 state, http://patchwork.ozlabs.org/patch/302047/ [v6,4/4] powerpc/85xx: add sysfs for pw20 state and altivec idle, http://patchwork.ozlabs.org/patch/302048/ -Dongsheng > -Original Message- > From: Dongsheng Wang [mailto:dongsheng.w...@freescale.com] > Sent: Tuesday, December 17, 2013 4:17 PM > To: Wood Scott-B07421; Bhushan Bharat-R65777 > Cc: linuxppc-dev@lists.ozlabs.org; Wang Dongsheng-B40534 > Subject: [PATCH v6 1/4] powerpc/fsl: add E6500 PVR and SPRN_PWRMGTCR0 define > > From: Wang Dongsheng > > E6500 PVR and SPRN_PWRMGTCR0 will be used in subsequent pw20/altivec > idle patches. > > Signed-off-by: Wang Dongsheng > --- > *v3: > Add bit definitions for PWRMGTCR0. > > arch/powerpc/include/asm/reg.h | 2 ++ > arch/powerpc/include/asm/reg_booke.h | 9 + > 2 files changed, 11 insertions(+) > > diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h > index 64264bf..d4160ca 100644 > --- a/arch/powerpc/include/asm/reg.h > +++ b/arch/powerpc/include/asm/reg.h > @@ -1053,6 +1053,8 @@ > #define PVR_8560 0x8020 > #define PVR_VER_E500V1 0x8020 > #define PVR_VER_E500V2 0x8021 > +#define PVR_VER_E65000x8040 > + > /* > * For the 8xx processors, all of them report the same PVR family for > * the PowerPC core. The various versions of these processors must be > diff --git a/arch/powerpc/include/asm/reg_booke.h > b/arch/powerpc/include/asm/reg_booke.h > index ed8f836..4a6457e 100644 > --- a/arch/powerpc/include/asm/reg_booke.h > +++ b/arch/powerpc/include/asm/reg_booke.h > @@ -170,6 +170,7 @@ > #define SPRN_L2CSR1 0x3FA /* L2 Data Cache Control and Status Register 1 > */ > #define SPRN_DCCR0x3FA /* Data Cache Cacheability Register */ > #define SPRN_ICCR0x3FB /* Instruction Cache Cacheability Register */ > +#define SPRN_PWRMGTCR0 0x3FB /* Power management control register 0 > */ > #define SPRN_SVR 0x3FF /* System Version Register */ > > /* > @@ -216,6 +217,14 @@ > #define CCR1_DPC0x0100 /* Disable L1 I-Cache/D-Cache parity > checking */ > #define CCR1_TCS0x0080 /* Timer Clock Select */ > > +/* Bit definitions for PWRMGTCR0. */ > +#define PWRMGTCR0_PW20_WAIT (1 << 14) /* PW20 state enable bit */ > +#define PWRMGTCR0_PW20_ENT_SHIFT 8 > +#define PWRMGTCR0_PW20_ENT 0x3F00 > +#define PWRMGTCR0_AV_IDLE_PD_EN (1 << 22) /* Altivec idle > enable */ > +#define PWRMGTCR0_AV_IDLE_CNT_SHIFT 16 > +#define PWRMGTCR0_AV_IDLE_CNT0x3F > + > /* Bit definitions for the MCSR. */ > #define MCSR_MCS 0x8000 /* Machine Check Summary */ > #define MCSR_IB 0x4000 /* Instruction PLB Error */ > -- > 1.8.0 > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/2] powerpc/p1022ds: fix rtc compatible string
Hi Scott,
Could you apply these patches?
[1/2] powerpc/p1022ds: fix rtc compatible string,
http://patchwork.ozlabs.org/patch/302741/
[2/2] powerpc/p1022ds: add a interrupt for rtc node,
http://patchwork.ozlabs.org/patch/302742/
-Dongsheng
> -Original Message-
> From: Dongsheng Wang [mailto:dongsheng.w...@freescale.com]
> Sent: Wednesday, December 18, 2013 4:39 PM
> To: Wood Scott-B07421; Huang Changming-R66093; Zang Roy-R61911
> Cc: linuxppc-dev@lists.ozlabs.org; Wang Dongsheng-B40534
> Subject: [PATCH 1/2] powerpc/p1022ds: fix rtc compatible string
>
> From: Wang Dongsheng
>
> RTC Hardware(ds3232) and rtc compatible string does not match.
> Change "dallas,ds1339" to "dallas,ds3232".
>
> Signed-off-by: Wang Dongsheng
>
> diff --git a/arch/powerpc/boot/dts/p1022ds.dtsi
> b/arch/powerpc/boot/dts/p1022ds.dtsi
> index 873da35..5725058 100644
> --- a/arch/powerpc/boot/dts/p1022ds.dtsi
> +++ b/arch/powerpc/boot/dts/p1022ds.dtsi
> @@ -146,7 +146,7 @@
>*/
> };
> rtc@68 {
> - compatible = "dallas,ds1339";
> + compatible = "dallas,ds3232";
> reg = <0x68>;
> };
> adt7461@4c {
> --
> 1.8.5
>
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] powerpc/fsl-booke: Use SPRN_SPRGn rather than mfsprg/mtsprg
Looks good. I will test it as soon as possible. BTW, there is only SPRG3 need to save. 32bit: SPRG0-SPRG1, SPRG2-SPRG7, SPRG9 be use to deal with exception, those register not need to save.(SPRG8 not be used) Only SPRG3 be used to save current thread_info pointer. -Dongsheng > -Original Message- > From: Wood Scott-B07421 > Sent: Friday, January 03, 2014 6:38 AM > To: Benjamin Herrenschmidt > Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; Wang Dongsheng-B40534; > Anton Vorontsov > Subject: [PATCH] powerpc/fsl-booke: Use SPRN_SPRGn rather than mfsprg/mtsprg > > This fixes a build break that was probably introduced with the removal > of -Wa,-me500 (commit f49596a4cf4753d13951608f24f939a59fdcc653), where > the assembler refuses to recognize SPRG4-7 with a generic PPC target. > > Signed-off-by: Scott Wood > Cc: Dongsheng Wang > Cc: Anton Vorontsov > --- > Dongsheng, please test. > --- > arch/powerpc/kernel/swsusp_booke.S | 32 > 1 file changed, 16 insertions(+), 16 deletions(-) > > diff --git a/arch/powerpc/kernel/swsusp_booke.S > b/arch/powerpc/kernel/swsusp_booke.S > index 0f20405..553c140 100644 > --- a/arch/powerpc/kernel/swsusp_booke.S > +++ b/arch/powerpc/kernel/swsusp_booke.S > @@ -74,21 +74,21 @@ _GLOBAL(swsusp_arch_suspend) > bne 1b > > /* Save SPRGs */ > - mfsprg r4,0 > + mfspr r4,SPRN_SPRG0 > stw r4,SL_SPRG0(r11) > - mfsprg r4,1 > + mfspr r4,SPRN_SPRG1 > stw r4,SL_SPRG1(r11) > - mfsprg r4,2 > + mfspr r4,SPRN_SPRG2 > stw r4,SL_SPRG2(r11) > - mfsprg r4,3 > + mfspr r4,SPRN_SPRG3 > stw r4,SL_SPRG3(r11) > - mfsprg r4,4 > + mfspr r4,SPRN_SPRG4 > stw r4,SL_SPRG4(r11) > - mfsprg r4,5 > + mfspr r4,SPRN_SPRG5 > stw r4,SL_SPRG5(r11) > - mfsprg r4,6 > + mfspr r4,SPRN_SPRG6 > stw r4,SL_SPRG6(r11) > - mfsprg r4,7 > + mfspr r4,SPRN_SPRG7 > stw r4,SL_SPRG7(r11) > > /* Call the low level suspend stuff (we should probably have made > @@ -150,21 +150,21 @@ _GLOBAL(swsusp_arch_resume) > bl _tlbil_all > > lwz r4,SL_SPRG0(r11) > - mtsprg 0,r4 > + mtspr SPRN_SPRG0,r4 > lwz r4,SL_SPRG1(r11) > - mtsprg 1,r4 > + mtspr SPRN_SPRG1,r4 > lwz r4,SL_SPRG2(r11) > - mtsprg 2,r4 > + mtspr SPRN_SPRG2,r4 > lwz r4,SL_SPRG3(r11) > - mtsprg 3,r4 > + mtspr SPRN_SPRG3,r4 > lwz r4,SL_SPRG4(r11) > - mtsprg 4,r4 > + mtspr SPRN_SPRG4,r4 > lwz r4,SL_SPRG5(r11) > - mtsprg 5,r4 > + mtspr SPRN_SPRG5,r4 > lwz r4,SL_SPRG6(r11) > - mtsprg 6,r4 > + mtspr SPRN_SPRG6,r4 > lwz r4,SL_SPRG7(r11) > - mtsprg 7,r4 > + mtspr SPRN_SPRG7,r4 > > /* restore the MSR */ > lwz r3,SL_MSR(r11) > -- > 1.8.3.2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] powerpc/fsl-booke: Use SPRN_SPRGn rather than mfsprg/mtsprg
Reviewed-by: Wang Dongsheng Tested-by: Wang Dongsheng Works well. :) -Dongsheng > -Original Message- > From: Linuxppc-dev [mailto:linuxppc-dev- > bounces+b40534=freescale@lists.ozlabs.org] On Behalf Of > dongsheng.w...@freescale.com > Sent: Friday, January 03, 2014 6:33 PM > To: Wood Scott-B07421; Benjamin Herrenschmidt > Cc: Anton Vorontsov; linuxppc-dev@lists.ozlabs.org > Subject: RE: [PATCH] powerpc/fsl-booke: Use SPRN_SPRGn rather than > mfsprg/mtsprg > > Looks good. I will test it as soon as possible. > > BTW, there is only SPRG3 need to save. > 32bit: SPRG0-SPRG1, SPRG2-SPRG7, SPRG9 be use to deal with exception, > those register not need to save.(SPRG8 not be used) Only SPRG3 be used > to save current thread_info pointer. > > -Dongsheng > > > -Original Message- > > From: Wood Scott-B07421 > > Sent: Friday, January 03, 2014 6:38 AM > > To: Benjamin Herrenschmidt > > Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; Wang Dongsheng-B40534; > > Anton Vorontsov > > Subject: [PATCH] powerpc/fsl-booke: Use SPRN_SPRGn rather than mfsprg/mtsprg > > > > This fixes a build break that was probably introduced with the removal > > of -Wa,-me500 (commit f49596a4cf4753d13951608f24f939a59fdcc653), where > > the assembler refuses to recognize SPRG4-7 with a generic PPC target. > > > > Signed-off-by: Scott Wood > > Cc: Dongsheng Wang > > Cc: Anton Vorontsov > > --- > > Dongsheng, please test. > > --- > > arch/powerpc/kernel/swsusp_booke.S | 32 > > 1 file changed, 16 insertions(+), 16 deletions(-) > > > > diff --git a/arch/powerpc/kernel/swsusp_booke.S > > b/arch/powerpc/kernel/swsusp_booke.S > > index 0f20405..553c140 100644 > > --- a/arch/powerpc/kernel/swsusp_booke.S > > +++ b/arch/powerpc/kernel/swsusp_booke.S > > @@ -74,21 +74,21 @@ _GLOBAL(swsusp_arch_suspend) > > bne 1b > > > > /* Save SPRGs */ > > - mfsprg r4,0 > > + mfspr r4,SPRN_SPRG0 > > stw r4,SL_SPRG0(r11) > > - mfsprg r4,1 > > + mfspr r4,SPRN_SPRG1 > > stw r4,SL_SPRG1(r11) > > - mfsprg r4,2 > > + mfspr r4,SPRN_SPRG2 > > stw r4,SL_SPRG2(r11) > > - mfsprg r4,3 > > + mfspr r4,SPRN_SPRG3 > > stw r4,SL_SPRG3(r11) > > - mfsprg r4,4 > > + mfspr r4,SPRN_SPRG4 > > stw r4,SL_SPRG4(r11) > > - mfsprg r4,5 > > + mfspr r4,SPRN_SPRG5 > > stw r4,SL_SPRG5(r11) > > - mfsprg r4,6 > > + mfspr r4,SPRN_SPRG6 > > stw r4,SL_SPRG6(r11) > > - mfsprg r4,7 > > + mfspr r4,SPRN_SPRG7 > > stw r4,SL_SPRG7(r11) > > > > /* Call the low level suspend stuff (we should probably have made > > @@ -150,21 +150,21 @@ _GLOBAL(swsusp_arch_resume) > > bl _tlbil_all > > > > lwz r4,SL_SPRG0(r11) > > - mtsprg 0,r4 > > + mtspr SPRN_SPRG0,r4 > > lwz r4,SL_SPRG1(r11) > > - mtsprg 1,r4 > > + mtspr SPRN_SPRG1,r4 > > lwz r4,SL_SPRG2(r11) > > - mtsprg 2,r4 > > + mtspr SPRN_SPRG2,r4 > > lwz r4,SL_SPRG3(r11) > > - mtsprg 3,r4 > > + mtspr SPRN_SPRG3,r4 > > lwz r4,SL_SPRG4(r11) > > - mtsprg 4,r4 > > + mtspr SPRN_SPRG4,r4 > > lwz r4,SL_SPRG5(r11) > > - mtsprg 5,r4 > > + mtspr SPRN_SPRG5,r4 > > lwz r4,SL_SPRG6(r11) > > - mtsprg 6,r4 > > + mtspr SPRN_SPRG6,r4 > > lwz r4,SL_SPRG7(r11) > > - mtsprg 7,r4 > > + mtspr SPRN_SPRG7,r4 > > > > /* restore the MSR */ > > lwz r3,SL_MSR(r11) > > -- > > 1.8.3.2 > > ___ > Linuxppc-dev mailing list > Linuxppc-dev@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/linuxppc-dev > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [RFC] linux/pci: move pci_platform_pm_ops to linux/pci.h
> -Original Message- > From: Rafael J. Wysocki [mailto:r...@rjwysocki.net] > Sent: Monday, January 06, 2014 8:13 PM > To: Bjorn Helgaas > Cc: Wang Dongsheng-B40534; Zang Roy-R61911; Wood Scott-B07421; Kumar Gala; > Linux > PM list; linux-...@vger.kernel.org; linuxppc-dev > Subject: Re: [RFC] linux/pci: move pci_platform_pm_ops to linux/pci.h > > On Friday, December 20, 2013 09:42:59 AM Bjorn Helgaas wrote: > > On Fri, Dec 20, 2013 at 3:03 AM, Dongsheng Wang > > wrote: > > > From: Wang Dongsheng > > > > > > make Freescale platform use pci_platform_pm_ops struct. > > > > This changelog doesn't say anything about what the patch does. > > > > I infer that you want to use pci_platform_pm_ops from some Freescale > > code. This patch should be posted along with the patches that add > > that Freescale code, so we can see how you intend to use it. > > > > The existing use is in drivers/pci/pci-acpi.c, so it's possible that > > your new use should be added in the same way, in drivers/pci, so we > > don't have to make pci_platform_pm_ops part of the public PCI > > interface in include/linux/pci.h. > > > > That said, if Raphael thinks this makes sense, it's OK with me. > > Well, I'd like to know why exactly the change is needed in the first place. > Thanks for review, I think the idea is not suitable for freescale platform implementation of the right now. I will drop this RFC patch. -Dongsheng > Thanks! > > -- > I speak only for myself. > Rafael J. Wysocki, Intel Open Source Technology Center. > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/2] pci: Fix root port bus->self is NULL
> >> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index > >> 38e403d..7f2d1ab 100644 > >> --- a/drivers/pci/probe.c > >> +++ b/drivers/pci/probe.c > >> @@ -1472,6 +1472,9 @@ int pci_scan_slot(struct pci_bus *bus, int devfn) > >>if (!dev->is_added) > >>nr++; > >> > >> + if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) > >> + bus->self = dev; > > > > In this case, bus is the pci root bus I think, so why set bus->self = root > port ? > > "bus->self" should pointer to the pci device that bridge out this bus. > Yes, this patch seems wrong. If dev is root port, bus should be root bus, so > we > shouldn't set root_bus->self = pci_dev_of_root_port. > Why the root bus->self pointer to the bridge? If child bus create from root bus, the child->self will get the bridge(root port) pci device. > Actually PCI core has correctly setup pci_bus->self for secondary bus of PCIe > root port. Yes, right. But the root-bus->self is NULL. I think the root bus should get root port pci device for itself. If there is no bridge at board how to get the root port device? -Dongsheng > > Thanks! > Gerry > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] powerpc/mpic: supply a .disable callback
> -Original Message- > From: Wood Scott-B07421 > Sent: Tuesday, January 07, 2014 2:39 PM > To: Wang Dongsheng-B40534 > Cc: b...@kernel.crashing.org; linuxppc-dev@lists.ozlabs.org > Subject: Re: [PATCH] powerpc/mpic: supply a .disable callback > > On Tue, 2014-01-07 at 13:38 +0800, Dongsheng Wang wrote: > > From: Wang Dongsheng > > Why did you change the author field? > :(. I forgot that, I added this code, not use git. Sorry about that, I'll change back after our discussion -Dongsheng > -Scott > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/2] powerpc/85xx: handle the eLBC error interrupt if it exist in dts
> -Original Message- > From: Wood Scott-B07421 > Sent: Tuesday, January 07, 2014 2:58 PM > To: Wang Dongsheng-B40534 > Cc: linuxppc-dev@lists.ozlabs.org; Xie Shaohui-B21989; Kumar Gala > Subject: Re: [PATCH 2/2] powerpc/85xx: handle the eLBC error interrupt if it > exist in dts > > On Tue, 2014-01-07 at 14:27 +0800, Dongsheng Wang wrote: > > From: Wang Dongsheng > > AFAICT this patch was originally written by Shaohui Xie. > > > On P3041, P1020, P1021, P1022, P1023 eLBC event interrupts are routed > > to Int9(P3041) & Int3(P102x) while ELBC error interrupts are routed to > > Int0, we need to call request_irq for each. > > For p3041 I thought that was only on early silicon revs that we don't > support anymore. > > As for p102x, have you tested that this is actually what happens? How > would we distinguish eLBC errors from other error sources, given that > there's no EISR0? Do we just hope that no other error interrupts > happen? Yes, I tested. The interrupt is shard eLBC interrupt handler could check the error. This patch is fix "nobody cared" the error interrupt. After sleep resume the lbc will get a chip select error. -Dongsheng > > -Scott > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] powerpc/mpic: supply a .disable callback
> -Original Message- > From: Benjamin Herrenschmidt [mailto:b...@kernel.crashing.org] > Sent: Tuesday, January 07, 2014 1:50 PM > To: Wang Dongsheng-B40534 > Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org > Subject: Re: [PATCH] powerpc/mpic: supply a .disable callback > > On Tue, 2014-01-07 at 13:38 +0800, Dongsheng Wang wrote: > > From: Wang Dongsheng > > > > Currently MPIC provides .mask, but not .disable. This means that > > effectively disable_irq() soft-disables the interrupt, and you get > > a .mask call if an interrupt actually occurs. > > > > I'm not sure if this was intended as a performance benefit (it seems common > > to omit .disable on powerpc interrupt controllers, but nowhere else), but it > > interacts badly with threaded/workqueue interrupts (including KVM > > reflection). In such cases, where the real interrupt handler does a > > disable_irq_nosync(), schedules defered handling, and returns, we get two > > interrupts for every real interrupt. The second interrupt does nothing > > but see that IRQ_DISABLED is set, and decide that it would be a good > > idea to actually call .mask. > > We probably don't want to do that for edge, only level interrupts. > Sorry Ben, I am not understand your comments. This issue is the kernel api irq_disable() only use chip->irq_disable(), but mpic not have this interface so we don't real disable the interrupt. -Dongsheng > Cheers, > Ben. > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/2] pci: Fix root port bus->self is NULL
> -Original Message- > From: Jiang Liu [mailto:jiang@linux.intel.com] > Sent: Wednesday, January 08, 2014 10:33 AM > To: Wang Dongsheng-B40534; Yijing Wang; bhelg...@google.com; > r...@rjwysocki.net > Cc: Wood Scott-B07421; ga...@codeaurora.org; Zang Roy-R61911; linux- > p...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org > Subject: Re: [PATCH 1/2] pci: Fix root port bus->self is NULL > > > > On 2014/1/7 17:51, dongsheng.w...@freescale.com wrote: > >>>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index > >>>> 38e403d..7f2d1ab 100644 > >>>> --- a/drivers/pci/probe.c > >>>> +++ b/drivers/pci/probe.c > >>>> @@ -1472,6 +1472,9 @@ int pci_scan_slot(struct pci_bus *bus, int devfn) > >>>> if (!dev->is_added) > >>>> nr++; > >>>> > >>>> +if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) > >>>> +bus->self = dev; > >>> > >>> In this case, bus is the pci root bus I think, so why set bus->self > >>> = root > >> port ? > >>> "bus->self" should pointer to the pci device that bridge out this bus. > >> Yes, this patch seems wrong. If dev is root port, bus should be root > >> bus, so we shouldn't set root_bus->self = pci_dev_of_root_port. > >> > > Why the root bus->self pointer to the bridge? > > If child bus create from root bus, the child->self will get the bridge(root > port) pci device. > > > >> Actually PCI core has correctly setup pci_bus->self for secondary bus > >> of PCIe root port. > > Yes, right. But the root-bus->self is NULL. I think the root bus > > should get root port pci device for itself. If there is no bridge at board > > how > to get the root port device? > Hi Dongsheng, > PCI root bus represents PCI host bridge, which has no corresponding PCI > device. > Yes, agree. If more than one bridge on the root bus, this patch is wrong. Thanks for your review. :) -Dongsheng > > > > -Dongsheng > > > >> > >> Thanks! > >> Gerry > >> > > > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/2] fsl/pci: The new pci suspend/resume implementation
> -Original Message- > From: Rafael J. Wysocki [mailto:r...@rjwysocki.net] > Sent: Wednesday, January 08, 2014 4:42 AM > To: Wang Dongsheng-B40534 > Cc: bhelg...@google.com; Wood Scott-B07421; ga...@codeaurora.org; Zang Roy- > R61911; linux-...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org > Subject: Re: [PATCH 2/2] fsl/pci: The new pci suspend/resume implementation > > On Tuesday, January 07, 2014 04:04:08 PM Dongsheng Wang wrote: > > From: Wang Dongsheng > > > > The new suspend/resume implementation, send pme turnoff message > > in suspend, and send pme exit message in resume. > > > > Add a PME handler, to response PME & message interrupt. > > > > Change platform_driver->suspend/resume to syscore->suspend/resume. > > Can you please first describe the problem you're trying to address? > If we do nothing in suspend/resume, some platform PCIe ip-block can't guarantee the link back to L0 state from sleep, then, when we read the EP device will hang. Only we send pme turnoff message in pci controller suspend, and send pme exit message in resume, the link state will be normal. When we send pme turnoff message in pci controller suspend, the links will into l2/l3 ready, then, host cannot communicate with ep device, but pci-driver will call back EP device to save them state. So we need to change platform_driver->suspend/resume to syscore->suspend/resume. Thanks, -Dongsheng ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/2] powerpc/85xx: handle the eLBC error interrupt if it exist in dts
> -Original Message- > From: Wood Scott-B07421 > Sent: Wednesday, January 08, 2014 4:45 AM > To: Wang Dongsheng-B40534 > Cc: linuxppc-dev@lists.ozlabs.org; Xie Shaohui-B21989; Kumar Gala > Subject: Re: [PATCH 2/2] powerpc/85xx: handle the eLBC error interrupt if it > exist in dts > > On Tue, 2014-01-07 at 04:01 -0600, Wang Dongsheng-B40534 wrote: > > > > > -Original Message- > > > From: Wood Scott-B07421 > > > Sent: Tuesday, January 07, 2014 2:58 PM > > > To: Wang Dongsheng-B40534 > > > Cc: linuxppc-dev@lists.ozlabs.org; Xie Shaohui-B21989; Kumar Gala > > > Subject: Re: [PATCH 2/2] powerpc/85xx: handle the eLBC error interrupt if > > > it > > > exist in dts > > > > > > On Tue, 2014-01-07 at 14:27 +0800, Dongsheng Wang wrote: > > > > From: Wang Dongsheng > > > > > > AFAICT this patch was originally written by Shaohui Xie. > > > > > > > On P3041, P1020, P1021, P1022, P1023 eLBC event interrupts are routed > > > > to Int9(P3041) & Int3(P102x) while ELBC error interrupts are routed to > > > > Int0, we need to call request_irq for each. > > > > > > For p3041 I thought that was only on early silicon revs that we don't > > > support anymore. > > > > > > As for p102x, have you tested that this is actually what happens? How > > > would we distinguish eLBC errors from other error sources, given that > > > there's no EISR0? Do we just hope that no other error interrupts > > > happen? > > Yes, I tested. The interrupt is shard eLBC interrupt handler could check the > error. > > This patch is fix "nobody cared" the error interrupt. After sleep resume the > lbc > > will get a chip select error. > > s/no other error interrupts happen/no other error interrupts for which > we don't have a handler registered or which don't even have an > associated status register happen/ > If the ip-block does not handle their error interrupt is a ip-block issue that. Since the use of this shared interrupt must maintain their status, once there is no deal shared interrupt, it will affect the other ip-block. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [v6,4/4] powerpc/85xx: add sysfs for pw20 state and altivec idle
> /home/scott/fsl/git/linux/upstream/arch/powerpc/kernel/sysfs.c:326:19: error: > 'PWRMGTCR0_AV_IDLE_CNT' undeclared (first use in this function) > /home/scott/fsl/git/linux/upstream/arch/powerpc/kernel/sysfs.c:329:36: error: > 'PWRMGTCR0_AV_IDLE_CNT_SHIFT' undeclared (first use in this function) > make[2]: *** [arch/powerpc/kernel/sysfs.o] Error 1 > make[1]: *** [arch/powerpc/kernel] Error 2 > make[1]: *** Waiting for unfinished jobs > make: *** [sub-make] Error 2 > > I'll fix when applying with this: > :), Thanks. -Dongsheng ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 3/3] powerpc/fsl: Use the new interface to save or restore registers
> -Original Message- > From: Wood Scott-B07421 > Sent: Wednesday, January 15, 2014 7:30 AM > To: Wang Dongsheng-B40534 > Cc: b...@kernel.crashing.org; Zhao Chenhui-B35336; an...@enomsg.org; linuxppc- > d...@lists.ozlabs.org > Subject: Re: [PATCH 3/3] powerpc/fsl: Use the new interface to save or restore > registers > > On Tue, 2014-01-14 at 15:59 +0800, Dongsheng Wang wrote: > > From: Wang Dongsheng > > > > Use fsl_cpu_state_save/fsl_cpu_state_restore to save/restore registers. > > Use the functions to save/restore registers, so we don't need to > > maintain the code. > > > > Signed-off-by: Wang Dongsheng > > Is there any functional change with this patchset (e.g. suspend > supported on chips where it wasn't before), or is it just cleanup? A > cover letter would be useful to describe the purpose of the overall > patchset when it isn't obvious. > Yes, just cleanup.. > > + > > + /* Restore base register */ > > + li r4, 0 > > + bl fsl_cpu_state_restore > > Why are you calling anything with "fsl" in the name from code that is > supposed to be for all booke? > E200, E300 not support. Support E500, E500v2, E500MC, E5500, E6500. Do you have any suggestions about this? Thanks, -Dongsheng ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers
> -Original Message- > From: Wood Scott-B07421 > Sent: Wednesday, January 15, 2014 7:51 AM > To: Wang Dongsheng-B40534 > Cc: b...@kernel.crashing.org; Zhao Chenhui-B35336; an...@enomsg.org; linuxppc- > d...@lists.ozlabs.org > Subject: Re: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore > the > core registers > > On Tue, 2014-01-14 at 15:59 +0800, Dongsheng Wang wrote: > > From: Wang Dongsheng > > > > Add fsl_cpu_state_save/fsl_cpu_state_restore functions, used for deep > > sleep and hibernation to save/restore core registers. We abstract out > > save/restore code for use in various modules, to make them don't need > > to maintain. > > > > Currently supported processors type are E6500, E5500, E500MC, E500v2 > > and E500v1. > > > > Signed-off-by: Wang Dongsheng > > What is there that is specfic to a particular core type that can't be handled > from C code? > In the context of the calling, maybe not in C environment.(Deep sleep without C environment when calling those interfaces) > > + /* > > +* Need to save float-point registers if MSR[FP] = 1. > > +*/ > > + mfmsr r12 > > + andi. r12, r12, MSR_FP > > + beq 1f > > + do_sr_fpr_regs(save) > > C code should have already ensured that MSR[FP] is not 1 (and thus the FP > context has been saved). > Yes, right. But I mean if the FP still use in core save flow, we need to save it. In this process, i don't care what other code do, we need to focus on not losing valuable data. > > +/* > > + * r3 = the virtual address of buffer > > + * r4 = suspend type, 0-BASE_SAVE, 1-ALL_SAVE > > #define these magic numbers, and define what is meant by "base save" > versus "all save". Ok, thanks. -Dongsheng ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 3/3] powerpc/fsl: Use the new interface to save or restore registers
> > > > Use fsl_cpu_state_save/fsl_cpu_state_restore to save/restore registers. > > > > Use the functions to save/restore registers, so we don't need to > > > > maintain the code. > > > > > > > > Signed-off-by: Wang Dongsheng > > > > > > Is there any functional change with this patchset (e.g. suspend > > > supported on chips where it wasn't before), or is it just cleanup? A > > > cover letter would be useful to describe the purpose of the overall > > > patchset when it isn't obvious. > > > > > > > Yes, just cleanup.. > > It seems to be introducing complexity rather than removing it. Is this > cleanup needed to prepare for adding new functionality? > > Plus, I'm skeptical that this is functionally equivalent. It looks like > the new code saves a lot more than the old code does. Why? > Actually, I want to take a practical example to push the save/restore patches. And this is also reasonable for 32bit-hibernation, the code is more clean. :) I think I need to change the description of the patch. > > > > + > > > > + /* Restore base register */ > > > > + li r4, 0 > > > > + bl fsl_cpu_state_restore > > > > > > Why are you calling anything with "fsl" in the name from code that is > > > supposed to be for all booke? > > > > > E200, E300 not support. > > Support E500, E500v2, E500MC, E5500, E6500. > > > > Do you have any suggestions about this? > > What about non-FSL booke such as 44x? > > Or if this file never supported 44x, rename it appropriately. > Currently does not support. ok change the name first, if later support, and then again to modify the name of this function. How about 85xx_cpu_state_restore? Thanks, -Dongsheng ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers
> > > What is there that is specfic to a particular core type that can't be > handled > > > from C code? > > > > > > > In the context of the calling, maybe not in C environment.(Deep sleep > > without > > C environment when calling those interfaces) > > Could you provide a concrete example? > :) Deep sleep, the patches will comes out soon. > > > > + /* > > > > +* Need to save float-point registers if MSR[FP] = 1. > > > > +*/ > > > > + mfmsr r12 > > > > + andi. r12, r12, MSR_FP > > > > + beq 1f > > > > + do_sr_fpr_regs(save) > > > > > > C code should have already ensured that MSR[FP] is not 1 (and thus the FP > > > context has been saved). > > > > > > > Yes, right. But I mean if the FP still use in core save flow, we need to > > save > it. > > In this process, i don't care what other code do, we need to focus on not > losing > > valuable data. > > It is not allowed to use FP at that point. > If MSR[FP] not active, that is FP not allowed to use. But here is a normal judgment, if MSR[FP] is active, this means that the floating point module is being used. I offer is a function of the interface, we don't know where is the function will be called. Just because we call this function in the context of uncertainty, we need this judgment to ensure that no data is lost. Thanks, -Dongsheng ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers
> -Original Message- > From: Wood Scott-B07421 > Sent: Tuesday, January 21, 2014 9:06 AM > To: Wang Dongsheng-B40534 > Cc: b...@kernel.crashing.org; Zhao Chenhui-B35336; an...@enomsg.org; linuxppc- > d...@lists.ozlabs.org > Subject: Re: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore > the > core registers > > On Mon, 2014-01-20 at 00:03 -0600, Wang Dongsheng-B40534 wrote: > > > > > > + /* > > > > > > +* Need to save float-point registers if MSR[FP] = 1. > > > > > > +*/ > > > > > > + mfmsr r12 > > > > > > + andi. r12, r12, MSR_FP > > > > > > + beq 1f > > > > > > + do_sr_fpr_regs(save) > > > > > > > > > > C code should have already ensured that MSR[FP] is not 1 (and thus the > FP > > > > > context has been saved). > > > > > > > > > > > > > Yes, right. But I mean if the FP still use in core save flow, we need to > save > > > it. > > > > In this process, i don't care what other code do, we need to focus on > > > > not > > > losing > > > > valuable data. > > > > > > It is not allowed to use FP at that point. > > > > > If MSR[FP] not active, that is FP not allowed to use. > > But here is a normal judgment, if MSR[FP] is active, this means that the > floating > > point module is being used. I offer is a function of the interface, we don't > know > > where is the function will be called. Just because we call this function in > the > > context of uncertainty, we need this judgment to ensure that no data is > > lost. > > The whole point of calling enable_kernel_fp() in C code before > suspending is to ensure that the FP state gets saved. If FP is used > after that point it is a bug. If you're worried about such bugs, then > clear MSR[FP] after calling enable_kernel_fp(), rather than adding > redundant state saving. > enable_kernel_fp() calling in MEM suspend flow. Hibernation is different with MEM suspend, and I'm not sure where will call this interface, so we need to ensure the integrity of the core saving. I don't think this code is *redundant*. I trust that the kernel can keep the FP related operations, that's why a judgment is here. :) Thanks, -Dongsheng ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/2] fsl/pci: The new pci suspend/resume implementation
Any more comments? :) Thanks, -Dongsheng > -Original Message- > From: Linuxppc-dev [mailto:linuxppc-dev- > bounces+b40534=freescale@lists.ozlabs.org] On Behalf Of > dongsheng.w...@freescale.com > Sent: Wednesday, January 08, 2014 3:13 PM > To: Rafael J. Wysocki > Cc: linuxppc-dev@lists.ozlabs.org; ga...@codeaurora.org; Wood Scott-B07421; > linux-...@vger.kernel.org; bhelg...@google.com > Subject: RE: [PATCH 2/2] fsl/pci: The new pci suspend/resume implementation > > > > > -Original Message- > > From: Rafael J. Wysocki [mailto:r...@rjwysocki.net] > > Sent: Wednesday, January 08, 2014 4:42 AM > > To: Wang Dongsheng-B40534 > > Cc: bhelg...@google.com; Wood Scott-B07421; ga...@codeaurora.org; Zang Roy- > > R61911; linux-...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org > > Subject: Re: [PATCH 2/2] fsl/pci: The new pci suspend/resume implementation > > > > On Tuesday, January 07, 2014 04:04:08 PM Dongsheng Wang wrote: > > > From: Wang Dongsheng > > > > > > The new suspend/resume implementation, send pme turnoff message > > > in suspend, and send pme exit message in resume. > > > > > > Add a PME handler, to response PME & message interrupt. > > > > > > Change platform_driver->suspend/resume to syscore->suspend/resume. > > > > Can you please first describe the problem you're trying to address? > > > If we do nothing in suspend/resume, some platform PCIe ip-block can't > guarantee > the link back to L0 state from sleep, then, when we read the EP device will > hang. > Only we send pme turnoff message in pci controller suspend, and send pme exit > message in resume, the link state will be normal. > > When we send pme turnoff message in pci controller suspend, the links will > into > l2/l3 > ready, then, host cannot communicate with ep device, but pci-driver will call > back EP > device to save them state. So we need to change > platform_driver->suspend/resume > to > syscore->suspend/resume. > > Thanks, > -Dongsheng > ___ > Linuxppc-dev mailing list > Linuxppc-dev@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/linuxppc-dev > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/3] powerpc/85xx: Provide two functions to save/restore the core registers
> > > > > > The whole point of calling enable_kernel_fp() in C code before > > > suspending is to ensure that the FP state gets saved. If FP is used > > > after that point it is a bug. If you're worried about such bugs, then > > > clear MSR[FP] after calling enable_kernel_fp(), rather than adding > > > redundant state saving. > > > > > > > enable_kernel_fp() calling in MEM suspend flow. > > Hibernation is different with MEM suspend, and I'm not sure where will call > this > > interface, so we need to ensure the integrity of the core saving. I don't > think > > this code is *redundant*. I trust that the kernel can keep the FP related > > operations, that's why a judgment is here. :) > > For hibernation, save_processor_state() is called first, which does > flush_fp_to_thread() which has a similar effect (though I wonder if it's > being called on the correct task for non-SMP). > Yes, thanks, I miss this code.:) But I still think we need to keep this judgment, because i provide an API. If you still insist on I can remove *FP*, but I don't want to do this..:) ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 3/3] powerpc/fsl: Use the new interface to save or restore registers
> > Currently does not support. ok change the name first, if later support, and > > then again to modify the name of this function. > > > > How about 85xx_cpu_state_restore? > > Symbols can't begin with numbers. booke_cpu_state_restore would be > better (it would still provide a place for 44x to be added if somebody > actually cared about doing so). > :). Thanks. -Dongsheng ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [rtc-linux] [PATCH] rtc/ds3232: Enable ds3232 to work as wakeup source
> -Original Message-
> From: Andrew Morton [mailto:a...@linux-foundation.org]
> Sent: Wednesday, February 26, 2014 6:07 AM
> To: rtc-li...@googlegroups.com
> Cc: Wang Dongsheng-B40534; a.zu...@towertech.it; Zhao Chenhui-B35336;
> linuxppc-
> d...@lists.ozlabs.org
> Subject: Re: [rtc-linux] [PATCH] rtc/ds3232: Enable ds3232 to work as wakeup
> source
>
> On Tue, 21 Jan 2014 13:24:51 +0800 Dongsheng Wang
>
> wrote:
>
> > From: Wang Dongsheng
> >
> > Add suspend/resume and device_init_wakeup to enable ds3232 as
> > wakeup source, /sys/class/rtc/rtcX/wakealarm for set wakeup alarm.
> >
> > ...
> >
> > @@ -411,23 +424,21 @@ static int ds3232_probe(struct i2c_client *client,
> > if (ret)
> > return ret;
> >
> > - ds3232->rtc = devm_rtc_device_register(&client->dev, client->name,
> > - &ds3232_rtc_ops, THIS_MODULE);
> > - if (IS_ERR(ds3232->rtc)) {
> > - dev_err(&client->dev, "unable to register the class device\n");
> > - return PTR_ERR(ds3232->rtc);
> > - }
> > -
> > - if (client->irq >= 0) {
> > + if (client->irq != NO_IRQ) {
>
> x86_64 allmodconfig:
>
> drivers/rtc/rtc-ds3232.c: In function 'ds3232_probe':
> drivers/rtc/rtc-ds3232.c:427: error: 'NO_IRQ' undeclared (first use in this
> function)
> drivers/rtc/rtc-ds3232.c:427: error: (Each undeclared identifier is reported
> only once
> drivers/rtc/rtc-ds3232.c:427: error: for each function it appears in.)
>
> Not all architectures implement NO_IRQ.
>
> I think this should be
>
> if (client->irq > 0) {
>
> but I'm not sure - iirc, x86 (at least) treats zero as "not an IRQ".
> But I think some architectures permit IRQ 0. There was discussion many
> years ago but I don't think anything got resolved.
>
I think this is why NO_IRQ is defined in kernel, that should be resolved this
issue.
Sorry, I don't know why some architectures didn't define this macro?
Hi Ben,
Did you have some suggestion?
Thanks,
-Dongsheng
>
> Help! I think some ppc people will know what to do here?
>
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [rtc-linux] [PATCH] rtc/ds3232: Enable ds3232 to work as wakeup source
> -Original Message-
> From: Wood Scott-B07421
> Sent: Wednesday, February 26, 2014 11:21 AM
> To: Wang Dongsheng-B40534
> Cc: Andrew Morton; rtc-li...@googlegroups.com; b...@kernel.crashing.org;
> a.zu...@towertech.it; Zhao Chenhui-B35336; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [rtc-linux] [PATCH] rtc/ds3232: Enable ds3232 to work as wakeup
> source
>
> On Tue, 2014-02-25 at 21:09 -0600, Wang Dongsheng-B40534 wrote:
> >
> > > -Original Message-
> > > From: Andrew Morton [mailto:a...@linux-foundation.org]
> > > Sent: Wednesday, February 26, 2014 6:07 AM
> > > To: rtc-li...@googlegroups.com
> > > Cc: Wang Dongsheng-B40534; a.zu...@towertech.it; Zhao Chenhui-B35336;
> linuxppc-
> > > d...@lists.ozlabs.org
> > > Subject: Re: [rtc-linux] [PATCH] rtc/ds3232: Enable ds3232 to work as
> > > wakeup
> > > source
> > >
> > > On Tue, 21 Jan 2014 13:24:51 +0800 Dongsheng Wang
>
> > > wrote:
> > >
> > > > + if (client->irq != NO_IRQ) {
> > >
> > > x86_64 allmodconfig:
> > >
> > > drivers/rtc/rtc-ds3232.c: In function 'ds3232_probe':
> > > drivers/rtc/rtc-ds3232.c:427: error: 'NO_IRQ' undeclared (first use in
> > > this
> > > function)
> > > drivers/rtc/rtc-ds3232.c:427: error: (Each undeclared identifier is
> > > reported
> > > only once
> > > drivers/rtc/rtc-ds3232.c:427: error: for each function it appears in.)
> > >
> > > Not all architectures implement NO_IRQ.
> > >
> > > I think this should be
> > >
> > > if (client->irq > 0) {
> > >
> > > but I'm not sure - iirc, x86 (at least) treats zero as "not an IRQ".
> > > But I think some architectures permit IRQ 0. There was discussion many
> > > years ago but I don't think anything got resolved.
> > >
> > I think this is why NO_IRQ is defined in kernel, that should be resolved
> > this
> issue.
> >
> > Sorry, I don't know why some architectures didn't define this macro?
>
> NO_IRQ is deprecated (see "git log -SNO_IRQ" for the trend of removing
> uses of it, as well as situations where it gives the wrong results).
> "if (client->irq > 0)" is correct.
>
Thanks.
-Dongsheng
> -Scott
>
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [2/2] fsl/pci: The new pci suspend/resume implementation
Hi Scott,
I will send v2 patch to fix your comment. Thanks for your review. :)
> -Original Message-
> From: Wood Scott-B07421
> Sent: Thursday, March 20, 2014 5:01 AM
> To: Wang Dongsheng-B40534
> Cc: bhelg...@google.com; r...@rjwysocki.net; roy.z...@freescale.com;
> ga...@codeaurora.org; linux-...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [2/2] fsl/pci: The new pci suspend/resume implementation
>
> On Tue, Jan 07, 2014 at 04:04:08PM +0800, Dongsheng Wang wrote:
> > From: Wang Dongsheng
> >
> > The new suspend/resume implementation, send pme turnoff message in
> > suspend, and send pme exit message in resume.
> >
> > Add a PME handler, to response PME & message interrupt.
> >
> > Change platform_driver->suspend/resume to syscore->suspend/resume.
> > pci-driver will call back EP device, to save EP state in
> > pci_pm_suspend_noirq, so we need to keep the link, until
> > pci_pm_suspend_noirq finish.
> >
> > Signed-off-by: Wang Dongsheng
>
> Is this patch OK to go in without patch 1/2? It's not clear whether that was
> deemed incorrect (as in new patch coming) or unnecessary.
>
Yes, I will abandon 1/2. And send this as a independent patch.
> It would also be good if you submit with the explanation from
> http://www.spinics.net/lists/linux-pci/msg27844.html in the commit message.
>
Thanks.
> > -static int fsl_pci_probe(struct platform_device *pdev)
> > +#ifdef CONFIG_PM
> > +static irqreturn_t fsl_pci_pme_handle(int irq, void *dev_id)
> > {
> > - int ret;
> > - struct device_node *node;
> > + struct pci_controller *hose = dev_id;
> > + struct ccsr_pci __iomem *pci = hose->private_data;
> > + u32 dr;
> >
> > - node = pdev->dev.of_node;
> > - ret = fsl_add_bridge(pdev, fsl_pci_primary == node);
> > + dr = in_be32(&pci->pex_pme_mes_dr);
> > + if (dr)
> > + out_be32(&pci->pex_pme_mes_dr, dr);
> > + else
> > + return IRQ_NONE;
> >
> > - mpc85xx_pci_err_probe(pdev);
> > + return IRQ_HANDLED;
> > +}
>
> Why do you put some of the HANDLED path in the if statement, and some outside?
>
> Just do:
>
> if (!dr)
> return IRQ_NONE;
>
> out_be32(...);
> return IRQ_HANDLED;
>
Right. :)
> > +static int fsl_pci_pme_probe(struct pci_controller *hose) {
> > + struct ccsr_pci __iomem *pci;
> > + struct pci_dev *dev = hose->bus->self;
> > + u16 pms;
> > + int pme_irq;
> > + int res;
> > +
> > + /* PME Disable */
> > + pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pms);
> > + pms &= ~PCI_PM_CTRL_PME_ENABLE;
> > + pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, pms);
> > +
> > + pme_irq = irq_of_parse_and_map(hose->dn, 0);
> > + if (!pme_irq) {
> > + pr_warn("Failed to map PME interrupt.\n");
>
> dev_err()
>
> > +
> > + return -ENXIO;
> > + }
> > +
> > + res = devm_request_irq(hose->parent, pme_irq,
> > + fsl_pci_pme_handle,
> > + IRQF_DISABLED | IRQF_SHARED,
> > + "[PCI] PME", hose);
>
> IRQF_DISABLED is a deprecated no-op.
>
> > + if (res < 0) {
> > + pr_warn("Unable to requiest irq %d for PME\n", pme_irq);
>
> dev_err() etc.
>
Ok, I will use it.
Regards,
-Dongsheng
> -Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] powerpc/irq: Remove HAVE_IRQ_EXIT_ON_IRQ_STACK feature at powerpc platform
Thanks Kevin. Your patch works normal. :) I still have some confused. I think when __do_softirq always get a interrupt, the hard stack will be run out, isn't it? Regards, -Dongsheng > -Original Message- > From: Kevin Hao [mailto:haoke...@gmail.com] > Sent: Friday, March 28, 2014 4:18 PM > To: Wang Dongsheng-B40534 > Cc: fweis...@gmail.com; James Hogan; Andrew Morton; David S. Miller; Peter > Zijlstra; Helge Deller; H. Peter Anvin; Heiko Carstens; linux- > ker...@vger.kernel.org; Paul Mackerras; James E.J. Bottomley; Linus Torvalds; > Jin Zhengxiong-R64188; Wood Scott-B07421; Thomas Gleixner; linuxppc- > d...@lists.ozlabs.org; Ingo Molnar; Martin Schwidefsky > Subject: Re: [PATCH] powerpc/irq: Remove HAVE_IRQ_EXIT_ON_IRQ_STACK feature at > powerpc platform > > On Fri, Mar 28, 2014 at 03:38:32PM +0800, Dongsheng Wang wrote: > > From: Wang Dongsheng > > > > If softirq use hardirq stack, we will get kernel painc when a hard irq > > coming again during __do_softirq enable local irq to deal with softirq > > action. So we need to switch satck into softirq stack when invoke soft irq. > > > > Task---> > > | Task stack > > | > > Interrput->EXCEPTION->do_IRQ-> > > ^| Hard irq stack > > || > > |irq_exit->__do_softirq->local_irq_enable-- > > -- > >local_irq_disable > > | > > | Hard irq > stack > > | > > | > > | > > Interrupt > coming again > > | There will get a Interrupt nesting > > | > > > > -- > > -- > > > > Trace 1: Trap 900 > > > > Kernel stack overflow in process e8152f40, r1=e8e05ec0 > > CPU: 0 PID: 2399 Comm: image_compress/ Not tainted > > 3.13.0-rc3-03475-g2e3f85b #432 > > task: e8152f40 ti: c080a000 task.ti: ef176000 > > NIP: c05bec04 LR: c0305590 CTR: 0010 > > REGS: e8e05e10 TRAP: 0901 Not tainted (3.13.0-rc3-03475-g2e3f85b) > > Could you double check if you got the following patch applied? > > commit 1a18a66446f3f289b05b634f18012424d82aa63a > Author: Kevin Hao > Date: Fri Jan 17 12:25:28 2014 +0800 > > powerpc: Set the correct ksp_limit on ppc32 when switching to irq stack > > Guenter Roeck has got the following call trace on a p2020 board: > Kernel stack overflow in process eb3e5a00, r1=eb79df90 > CPU: 0 PID: 2838 Comm: ssh Not tainted 3.13.0-rc8-juniper-00146-g19eca00 > #4 > task: eb3e5a00 ti: c0616000 task.ti: ef44 > NIP: c003a420 LR: c003a410 CTR: c0017518 > REGS: eb79dee0 TRAP: 0901 Not tainted > (3.13.0-rc8-juniper-00146-g19eca00) > MSR: 00029000 CR: 24008444 XER: > GPR00: c003a410 eb79df90 eb3e5a00 eb05d900 0001 65d87646 > > GPR08: 020b8000 44008442 > NIP [c003a420] __do_softirq+0x94/0x1ec > LR [c003a410] __do_softirq+0x84/0x1ec > Call Trace: > [eb79df90] [c003a410] __do_softirq+0x84/0x1ec (unreliable) > [eb79dfe0] [c003a970] irq_exit+0xbc/0xc8 > [eb79dff0] [c000cc1c] call_do_irq+0x24/0x3c > [ef441f20] [c00046a8] do_IRQ+0x8c/0xf8 > [ef441f40] [c000e7f4] ret_from_except+0x0/0x18 > --- Exception: 501 at 0xfcda524 > LR = 0x10024900 > Instruction dump: > 7c781b78 3b4a 3a73b040 543c0024 3a80 3b3913a0 7ef5bb78 48201bf9 > 5463103a 7d3b182e 7e89b92e 7c008146 <3ba0> 7e7e9b78 4814 > 57fff87f > Kernel panic - not syncing: kernel stack overflow > CPU: 0 PID: 2838 Comm: ssh Not tainted 3.13.0-rc8-juniper-00146-g19eca00 > #4 > Call Trace: > > The reason is that we have used the wrong register to calculate the > ksp_limit in commit cbc9565ee826 (powerpc: Remove ksp_limit on ppc64). > Just fix it. > > As suggested by Benjamin Herrenschmidt, also add the C prototype of the > function in the comment in order to avoid such kind of errors in the > future. > > Cc: sta...@vger.kernel.org # 3.12 > Reported-by: Guenter Roeck > Tested-by: Guenter Roeck > Signed-off-by: Kevin Hao > Signed-off-by: Benjamin Herrenschmidt > > Thanks, > Kevin ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/2] Make the diu driver work without board level initilization
> -Original Message-
> From: Linuxppc-dev [mailto:linuxppc-dev-
> bounces+b40534=freescale@lists.ozlabs.org] On Behalf Of Jason Jin
> Sent: Thursday, March 27, 2014 7:38 PM
> To: Wood Scott-B07421; ti...@tabi.org
> Cc: linux-fb...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; Li Yang-Leo-
> R58472; Jin Zhengxiong-R64188
> Subject: [PATCH 2/2] Make the diu driver work without board level
> initilization
>
> So far the DIU driver does not have a mechanism to do the
> board specific initialization. So on some platforms,
> such as P1022, 8610 and 5121, The board specific initialization
> is implmented in the platform file such p10222_ds.
>
> Actually, the DIU is already intialized in the u-boot, the pin sharing
> and the signal routing are also set in u-boot. So we can leverage that
> in kernel driver to avoid board sepecific initialization, especially
> for the corenet platform, which is the abstraction for serveral
> platfroms.
>
> The potential problem is that when the system wakeup from the deep
> sleep, some platform settings may need to be re-initialized. The CPLD
> and FPGA settings will be kept, but the pixel clock register which
> usually locate at the global utility space need to be reinitialized.
>
> Generally, the pixel clock setting was implemented in the platform
> file, But the pixel clock register itself should be part of the DIU
> module, And for P1022,8610 and T1040, the pixel clock register have the
> same structure, So we can consider to move the pixel clock setting
> from the platform to the diu driver. This patch provide the options
> set the pixel clock in the diu driver. But the original platform pixel
> clock setting stil can be used for P1022,8610 and 512x without any
> update. To implement the pixel clock setting in the diu driver. the
> following update in the diu dts node was needed.
> display:display@18 {
> compatible = "fsl,t1040-diu", "fsl,diu";
> - reg = <0x18 1000>;
> + reg = <0x18 1000 0xfc028 4>;
> + pixclk = <0 255 0>;
> interrupts = <74 2 0 0>;
> }
> The 0xfc028 is the offset for pixel clock register. the 3 segment of
> the pixclk stand for the PXCKDLYDIR, the max of PXCK and the PXCKDLY
> which will be used by the pixel clock register setting.
>
> This was tested on T1040 platform. For other platform, the original
> node together with the platform settings still can work.
>
> Signed-off-by: Jason Jin
> ---
> V2: Remove the pixel clock register saving for suspend.
> add the pixel clock setting in driver.
>
> drivers/video/fsl-diu-fb.c | 61
> --
> 1 file changed, 59 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
> index 4bc4730..792038f 100644
> --- a/drivers/video/fsl-diu-fb.c
> +++ b/drivers/video/fsl-diu-fb.c
> @@ -50,6 +50,7 @@
> #define INT_PARERR 0x08/* Display parameters error interrupt */
> #define INT_LS_BF_VS 0x10/* Lines before vsync. interrupt */
>
> +#define PIXCLKCR_PXCKEN 0x8000
> /*
> * List of supported video modes
> *
> @@ -372,6 +373,8 @@ struct fsl_diu_data {
> unsigned int irq;
> enum fsl_diu_monitor_port monitor_port;
> struct diu __iomem *diu_reg;
> + void __iomem *pixelclk_reg;
> + u32 pixclkcr[3];
> spinlock_t reg_lock;
> u8 dummy_aoi[4 * 4 * 4];
> struct diu_ad dummy_ad __aligned(8);
> @@ -479,7 +482,10 @@ static enum fsl_diu_monitor_port
> fsl_diu_name_to_port(const
> char *s)
> port = FSL_DIU_PORT_DLVDS;
> }
>
> - return diu_ops.valid_monitor_port(port);
> + if (diu_ops.valid_monitor_port)
> + return diu_ops.valid_monitor_port(port);
> + else
Remove this "else", otherwise looks good.
Regards,
-Dongsheng
> + return port;
> }
>
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH bisect 2/2] ASoC: fsl_sai: Separately enable interrupts for Tx and Rx streams
> -Original Message-
> From: Linuxppc-dev [mailto:linuxppc-dev-
> bounces+b40534=freescale@lists.ozlabs.org] On Behalf Of Nicolin Chen
> Sent: Tuesday, April 01, 2014 11:17 AM
> To: broo...@kernel.org
> Cc: alsa-de...@alsa-project.org; Xiubo Li-B47053;
> linuxppc-dev@lists.ozlabs.org;
> linux-ker...@vger.kernel.org; ti...@tabi.org
> Subject: [PATCH bisect 2/2] ASoC: fsl_sai: Separately enable interrupts for Tx
> and Rx streams
>
> We only enable one side interrupt for each stream since over/underrun
> on the opposite stream would be resulted from what we previously did,
> enabling TERE but remaining FRDE disabled, even though the xrun on the
> opposite direction will not break the current stream.
>
> Signed-off-by: Nicolin Chen
> Acked-by: Xiubo Li
> ---
> sound/soc/fsl/fsl_sai.c | 8 ++--
> sound/soc/fsl/fsl_sai.h | 1 +
> 2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> index bdfd497..d64c33f 100644
> --- a/sound/soc/fsl/fsl_sai.c
> +++ b/sound/soc/fsl/fsl_sai.c
> @@ -397,4 +397,6 @@ static int fsl_sai_trigger(struct snd_pcm_substream
> *substream, int cmd,
>
> regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> +FSL_SAI_CSR_xIE_MASK, FSL_SAI_FLAGS);
> + regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> FSL_SAI_CSR_FRDE, FSL_SAI_CSR_FRDE);
> break;
> @@ -404,4 +406,6 @@ static int fsl_sai_trigger(struct snd_pcm_substream
> *substream, int cmd,
> regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> FSL_SAI_CSR_FRDE, 0);
> + regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> +FSL_SAI_CSR_xIE_MASK, 0);
>
> if (!(tcsr & FSL_SAI_CSR_FRDE || rcsr & FSL_SAI_CSR_FRDE)) {
> @@ -464,6 +468,6 @@ static int fsl_sai_dai_probe(struct snd_soc_dai *cpu_dai)
> struct fsl_sai *sai = dev_get_drvdata(cpu_dai->dev);
>
> - regmap_update_bits(sai->regmap, FSL_SAI_TCSR, 0x,
> FSL_SAI_FLAGS);
> - regmap_update_bits(sai->regmap, FSL_SAI_RCSR, 0x,
> FSL_SAI_FLAGS);
> + regmap_update_bits(sai->regmap, FSL_SAI_TCSR, 0x, 0x0);
> + regmap_update_bits(sai->regmap, FSL_SAI_RCSR, 0x, 0x0);
Why are you remove this macro? Don't use magic number.
Regards,
-Dongsheng
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH bisect 2/2] ASoC: fsl_sai: Separately enable interrupts for Tx and Rx streams
> -Original Message-
> From: Nicolin Chen [mailto:guangyu.c...@freescale.com]
> Sent: Tuesday, April 01, 2014 11:14 AM
> To: Wang Dongsheng-B40534
> Cc: broo...@kernel.org; alsa-de...@alsa-project.org; Xiubo Li-B47053;
> linuxppc-
> d...@lists.ozlabs.org; linux-ker...@vger.kernel.org; ti...@tabi.org
> Subject: Re: [PATCH bisect 2/2] ASoC: fsl_sai: Separately enable interrupts
> for
> Tx and Rx streams
>
> On Tue, Apr 01, 2014 at 11:25:02AM +0800, Wang Dongsheng-B40534 wrote:
> > > Subject: [PATCH bisect 2/2] ASoC: fsl_sai: Separately enable interrupts
> > > for
> Tx
> > > and Rx streams
> > >
> > > We only enable one side interrupt for each stream since over/underrun
> > > on the opposite stream would be resulted from what we previously did,
> > > enabling TERE but remaining FRDE disabled, even though the xrun on the
> > > opposite direction will not break the current stream.
> > >
> > > Signed-off-by: Nicolin Chen
> > > Acked-by: Xiubo Li
> > > ---
> > > sound/soc/fsl/fsl_sai.c | 8 ++--
> > > sound/soc/fsl/fsl_sai.h | 1 +
> > > 2 files changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> > > index bdfd497..d64c33f 100644
> > > --- a/sound/soc/fsl/fsl_sai.c
> > > +++ b/sound/soc/fsl/fsl_sai.c
> > > @@ -397,4 +397,6 @@ static int fsl_sai_trigger(struct snd_pcm_substream
> > > *substream, int cmd,
> > >
> > > regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> > > +FSL_SAI_CSR_xIE_MASK, FSL_SAI_FLAGS);
> > > + regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> > > FSL_SAI_CSR_FRDE, FSL_SAI_CSR_FRDE);
> > > break;
> > > @@ -404,4 +406,6 @@ static int fsl_sai_trigger(struct snd_pcm_substream
> > > *substream, int cmd,
> > > regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> > > FSL_SAI_CSR_FRDE, 0);
> > > + regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> > > +FSL_SAI_CSR_xIE_MASK, 0);
> > >
> > > if (!(tcsr & FSL_SAI_CSR_FRDE || rcsr & FSL_SAI_CSR_FRDE)) {
> > > @@ -464,6 +468,6 @@ static int fsl_sai_dai_probe(struct snd_soc_dai
> > > *cpu_dai)
> > > struct fsl_sai *sai = dev_get_drvdata(cpu_dai->dev);
> > >
> > > - regmap_update_bits(sai->regmap, FSL_SAI_TCSR, 0x,
> > > FSL_SAI_FLAGS);
> > > - regmap_update_bits(sai->regmap, FSL_SAI_RCSR, 0x,
> > > FSL_SAI_FLAGS);
> > > + regmap_update_bits(sai->regmap, FSL_SAI_TCSR, 0x, 0x0);
> > > + regmap_update_bits(sai->regmap, FSL_SAI_RCSR, 0x, 0x0);
> >
> > Why are you remove this macro? Don't use magic number.
>
> It's pretty clear that the so-called magic number is to clear the settings
> in the registers for driver init as what this driver did at the first place
> -- no offense but I don't think you would ask this if you check the git-log
> of the driver.
>
~FSL_SAI_MASK is better than 0x0. And you also replace 0x.
Regards,
-Dongsheng
> Thank you,
> Nicolin Chen
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH bisect 2/2] ASoC: fsl_sai: Separately enable interrupts for Tx and Rx streams
> -Original Message-
> From: Nicolin Chen [mailto:guangyu.c...@freescale.com]
> Sent: Tuesday, April 01, 2014 11:38 AM
> To: Wang Dongsheng-B40534
> Cc: broo...@kernel.org; alsa-de...@alsa-project.org; Xiubo Li-B47053;
> linuxppc-
> d...@lists.ozlabs.org; linux-ker...@vger.kernel.org; ti...@tabi.org
> Subject: Re: [PATCH bisect 2/2] ASoC: fsl_sai: Separately enable interrupts
> for
> Tx and Rx streams
>
> On Tue, Apr 01, 2014 at 11:48:16AM +0800, Wang Dongsheng-B40534 wrote:
> >
> >
> > > -Original Message-
> > > From: Nicolin Chen [mailto:guangyu.c...@freescale.com]
> > > Sent: Tuesday, April 01, 2014 11:14 AM
> > > To: Wang Dongsheng-B40534
> > > Cc: broo...@kernel.org; alsa-de...@alsa-project.org; Xiubo Li-B47053;
> linuxppc-
> > > d...@lists.ozlabs.org; linux-ker...@vger.kernel.org; ti...@tabi.org
> > > Subject: Re: [PATCH bisect 2/2] ASoC: fsl_sai: Separately enable
> > > interrupts
> for
> > > Tx and Rx streams
> > >
> > > On Tue, Apr 01, 2014 at 11:25:02AM +0800, Wang Dongsheng-B40534 wrote:
> > > > > Subject: [PATCH bisect 2/2] ASoC: fsl_sai: Separately enable
> > > > > interrupts
> for
> > > Tx
> > > > > and Rx streams
> > > > >
> > > > > We only enable one side interrupt for each stream since over/underrun
> > > > > on the opposite stream would be resulted from what we previously did,
> > > > > enabling TERE but remaining FRDE disabled, even though the xrun on the
> > > > > opposite direction will not break the current stream.
> > > > >
> > > > > Signed-off-by: Nicolin Chen
> > > > > Acked-by: Xiubo Li
> > > > > ---
> > > > > sound/soc/fsl/fsl_sai.c | 8 ++--
> > > > > sound/soc/fsl/fsl_sai.h | 1 +
> > > > > 2 files changed, 7 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> > > > > index bdfd497..d64c33f 100644
> > > > > --- a/sound/soc/fsl/fsl_sai.c
> > > > > +++ b/sound/soc/fsl/fsl_sai.c
> > > > > @@ -397,4 +397,6 @@ static int fsl_sai_trigger(struct
> > > > > snd_pcm_substream
> > > > > *substream, int cmd,
> > > > >
> > > > > regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> > > > > +FSL_SAI_CSR_xIE_MASK, FSL_SAI_FLAGS);
> > > > > + regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> > > > > FSL_SAI_CSR_FRDE, FSL_SAI_CSR_FRDE);
> > > > > break;
> > > > > @@ -404,4 +406,6 @@ static int fsl_sai_trigger(struct
> > > > > snd_pcm_substream
> > > > > *substream, int cmd,
> > > > > regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> > > > > FSL_SAI_CSR_FRDE, 0);
> > > > > + regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx),
> > > > > +FSL_SAI_CSR_xIE_MASK, 0);
> > > > >
> > > > > if (!(tcsr & FSL_SAI_CSR_FRDE || rcsr &
> > > > > FSL_SAI_CSR_FRDE)) {
> > > > > @@ -464,6 +468,6 @@ static int fsl_sai_dai_probe(struct snd_soc_dai
> *cpu_dai)
> > > > > struct fsl_sai *sai = dev_get_drvdata(cpu_dai->dev);
> > > > >
> > > > > - regmap_update_bits(sai->regmap, FSL_SAI_TCSR, 0x,
> FSL_SAI_FLAGS);
> > > > > - regmap_update_bits(sai->regmap, FSL_SAI_RCSR, 0x,
> FSL_SAI_FLAGS);
> > > > > + regmap_update_bits(sai->regmap, FSL_SAI_TCSR, 0x, 0x0);
> > > > > + regmap_update_bits(sai->regmap, FSL_SAI_RCSR, 0x, 0x0);
> > > >
> > > > Why are you remove this macro? Don't use magic number.
> > >
> > > It's pretty clear that the so-called magic number is to clear the settings
> > > in the registers for driver init as what this driver did at the first
> > > place
> > > -- no offense but I don't think you would ask this if you check the
> > > git-log
> > > of the driver.
> > >
> > ~FSL_SAI_MASK is better than 0x0. And you also replace 0x.
>
> I would later send a patch to reset SAI for a true init instead of these lines
> but not within this patch as it's focusing on the interrupts enabling.
>
> So please don't grasp the mask here. Just let me continue.
>
:), fine.
Regards,
-Dongsheng
> Thank you,
> Nicolin Chen
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] cpuidle: add freescale e500 family porcessors idle support
Hi Daniel,
Thanks for your review. I will fix your comments.
BTW, fix Rafael's email. :)
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include
> > +#include
> > +#include
> > +
> > +static unsigned int max_idle_state;
> > +static struct cpuidle_state *cpuidle_state_table;
> > +
> > +struct cpuidle_driver e500_idle_driver = {
> > + .name = "e500_idle",
> > + .owner = THIS_MODULE,
> > +};
> > +
> > +static void e500_cpuidle(void)
> > +{
> > + if (cpuidle_idle_call())
> > + cpuidle_wait();
> > +}
>
> Nope, that has been changed. No more call to cpuidle_idle_call in a driver.
>
Thanks.
> > +
> > +static int pw10_enter(struct cpuidle_device *dev,
> > + struct cpuidle_driver *drv, int index)
> > +{
> > + cpuidle_wait();
> > + return index;
> > +}
> > +
> > +#define MAX_BIT63
> > +#define MIN_BIT1
> > +extern u32 cpuidle_entry_bit;
> > +static int pw20_enter(struct cpuidle_device *dev,
> > + struct cpuidle_driver *drv, int index)
> > +{
> > + u32 pw20_idle;
> > + u32 entry_bit;
> > + pw20_idle = mfspr(SPRN_PWRMGTCR0);
> > + if ((pw20_idle & PWRMGTCR0_PW20_ENT) != PWRMGTCR0_PW20_ENT) {
> > + pw20_idle &= ~PWRMGTCR0_PW20_ENT;
> > + entry_bit = MAX_BIT - cpuidle_entry_bit;
> > + pw20_idle |= (entry_bit << PWRMGTCR0_PW20_ENT_SHIFT);
> > + mtspr(SPRN_PWRMGTCR0, pw20_idle);
> > + }
> > +
> > + cpuidle_wait();
> > +
> > + pw20_idle &= ~PWRMGTCR0_PW20_ENT;
> > + pw20_idle |= (MIN_BIT << PWRMGTCR0_PW20_ENT_SHIFT);
> > + mtspr(SPRN_PWRMGTCR0, pw20_idle);
> > +
> > + return index;
> > +}
>
> Is it possible to give some comments and encapsulate the code with
> explicit function names to be implemented in an arch specific directory
> file (eg. pm.c) and export these functions in a linux/ header ? We try
> to prevent to include asm if possible.
>
Yep, Looks better. Thanks.
> > +
> > +static struct cpuidle_state pw_idle_states[] = {
> > + {
> > + .name = "pw10",
> > + .desc = "pw10",
> > + .flags = CPUIDLE_FLAG_TIME_VALID,
> > + .exit_latency = 0,
> > + .target_residency = 0,
> > + .enter = &pw10_enter
> > + },
> > +
> > + {
> > + .name = "pw20",
> > + .desc = "pw20-core-idle",
> > + .flags = CPUIDLE_FLAG_TIME_VALID,
> > + .exit_latency = 1,
> > + .target_residency = 50,
> > + .enter = &pw20_enter
> > + },
> > +};
>
> No need to define this intermediate structure here, you can directly
> initialize the cpuidle_driver:
>
Thanks. :)
> > +static int cpu_hotplug_notify(struct notifier_block *n,
> > + unsigned long action, void *hcpu)
> > +{
> > + unsigned long hotcpu = (unsigned long)hcpu;
> > + struct cpuidle_device *dev =
> > + per_cpu_ptr(cpuidle_devices, hotcpu);
> > +
> > + if (dev && cpuidle_get_driver()) {
> > + switch (action) {
> > + case CPU_ONLINE:
> > + case CPU_ONLINE_FROZEN:
> > + cpuidle_pause_and_lock();
> > + cpuidle_enable_device(dev);
> > + cpuidle_resume_and_unlock();
> > + break;
> > +
> > + case CPU_DEAD:
> > + case CPU_DEAD_FROZEN:
> > + cpuidle_pause_and_lock();
> > + cpuidle_disable_device(dev);
> > + cpuidle_resume_and_unlock();
> > + break;
> > +
> > + default:
> > + return NOTIFY_DONE;
> > + }
> > + }
> > +
> > + return NOTIFY_OK;
> > +}
> > +
> > +static struct notifier_block cpu_hotplug_notifier = {
> > + .notifier_call = cpu_hotplug_notify,
> > +};
>
> Can you explain why this is needed ?
>
If a cpu will be plugged out/in, We should be let Cpuidle know to remove
corresponding sys interface and disable/enable cpudile-governor for current cpu.
> > + err = register_cpu_notifier(&cpu_hotplug_notifier);
> > + if (err)
> > + pr_warn("Cpuidle driver: register cpu notifier failed.\n");
> > +
> > + /* Replace the original way of idle after cpuidle registered. */
> > + ppc_md.power_save = e500_cpuidle;
> > + on_each_cpu(replace_orig_idle, NULL, 1);
>
> Why ?
>
I checked again, if we put cpuidle_idle_call in asm, this is not need.
Regards,
-Dongsheng
> > + pr_info("e500_idle_driver registered.\n");
> > +
> > + return 0;
> > +}
> > +late_initcall(e500_idle_init);
> >
>
> Thanks
>
>-- Daniel
>
>
> --
> Facebook |
> Twitter |
>
RE: [PATCH] cpuidle: add freescale e500 family porcessors idle support
> -Original Message-
> From: Daniel Lezcano [mailto:daniel.lezc...@linaro.org]
> Sent: Thursday, April 03, 2014 2:29 PM
> To: Wang Dongsheng-B40534; Wood Scott-B07421
> Cc: r...@rjwysocki.net; Li Yang-Leo-R58472; Jin Zhengxiong-R64188; Zhao
> Chenhui-
> B35336; linux...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH] cpuidle: add freescale e500 family porcessors idle
> support
>
> On 04/03/2014 05:20 AM, dongsheng.w...@freescale.com wrote:
> > Hi Daniel,
> >
> > Thanks for your review. I will fix your comments.
> >
> > BTW, fix Rafael's email. :)
> >
> >>> +#include
> >>> +#include
> >>> +#include
> >>> +#include
> >>> +#include
> >>> +
> >>> +#include
> >>> +#include
> >>> +#include
> >>> +
> >>> +static unsigned int max_idle_state; static struct cpuidle_state
> >>> +*cpuidle_state_table;
> >>> +
> >>> +struct cpuidle_driver e500_idle_driver = {
> >>> + .name = "e500_idle",
> >>> + .owner = THIS_MODULE,
> >>> +};
> >>> +
> >>> +static void e500_cpuidle(void)
> >>> +{
> >>> + if (cpuidle_idle_call())
> >>> + cpuidle_wait();
> >>> +}
> >>
> >> Nope, that has been changed. No more call to cpuidle_idle_call in a driver.
> >>
> >
> > Thanks.
> >
> >>> +
> >>> +static int pw10_enter(struct cpuidle_device *dev,
> >>> + struct cpuidle_driver *drv, int index) {
> >>> + cpuidle_wait();
> >>> + return index;
> >>> +}
> >>> +
> >>> +#define MAX_BIT 63
> >>> +#define MIN_BIT 1
> >>> +extern u32 cpuidle_entry_bit;
> >>> +static int pw20_enter(struct cpuidle_device *dev,
> >>> + struct cpuidle_driver *drv, int index) {
> >>> + u32 pw20_idle;
> >>> + u32 entry_bit;
> >>> + pw20_idle = mfspr(SPRN_PWRMGTCR0);
> >>> + if ((pw20_idle & PWRMGTCR0_PW20_ENT) != PWRMGTCR0_PW20_ENT) {
> >>> + pw20_idle &= ~PWRMGTCR0_PW20_ENT;
> >>> + entry_bit = MAX_BIT - cpuidle_entry_bit;
> >>> + pw20_idle |= (entry_bit << PWRMGTCR0_PW20_ENT_SHIFT);
> >>> + mtspr(SPRN_PWRMGTCR0, pw20_idle);
> >>> + }
> >>> +
> >>> + cpuidle_wait();
> >>> +
> >>> + pw20_idle &= ~PWRMGTCR0_PW20_ENT;
> >>> + pw20_idle |= (MIN_BIT << PWRMGTCR0_PW20_ENT_SHIFT);
> >>> + mtspr(SPRN_PWRMGTCR0, pw20_idle);
> >>> +
> >>> + return index;
> >>> +}
> >>
> >> Is it possible to give some comments and encapsulate the code with
> >> explicit function names to be implemented in an arch specific
> >> directory file (eg. pm.c) and export these functions in a linux/
> >> header ? We try to prevent to include asm if possible.
> >>
> >
> > Yep, Looks better. Thanks.
> >
> >>> +
> >>> +static struct cpuidle_state pw_idle_states[] = {
> >>> + {
> >>> + .name = "pw10",
> >>> + .desc = "pw10",
> >>> + .flags = CPUIDLE_FLAG_TIME_VALID,
> >>> + .exit_latency = 0,
> >>> + .target_residency = 0,
> >>> + .enter = &pw10_enter
> >>> + },
> >>> +
> >>> + {
> >>> + .name = "pw20",
> >>> + .desc = "pw20-core-idle",
> >>> + .flags = CPUIDLE_FLAG_TIME_VALID,
> >>> + .exit_latency = 1,
> >>> + .target_residency = 50,
> >>> + .enter = &pw20_enter
> >>> + },
> >>> +};
> >>
> >> No need to define this intermediate structure here, you can directly
> >> initialize the cpuidle_driver:
> >>
> >
> > Thanks. :)
> >
> >>> +static int cpu_hotplug_notify(struct notifier_block *n,
> >>> + unsigned long action, void *hcpu) {
> >>> + unsigned long hotcpu = (unsigned long)hcpu;
> >>> + struct cpuidle_device *dev =
> >>> + per_cpu_ptr(cpuidle_devices, hotcpu);
> >>> +
> >>> + if (dev && cpuidle_get_driver()) {
> >>> + switch (action) {
> >>> + case CPU_ONLINE:
> >>
RE: [PATCH 1/2] powerpc/mpc85xx: add two functions to get suspend state which is standby or mem
> -Original Message-
> From: Wood Scott-B07421
> Sent: Tuesday, April 15, 2014 7:27 AM
> To: Wang Dongsheng-B40534
> Cc: Jin Zhengxiong-R64188; Li Yang-Leo-R58472; Zhao Chenhui-B35336; linuxppc-
> d...@lists.ozlabs.org
> Subject: Re: [PATCH 1/2] powerpc/mpc85xx: add two functions to get suspend
> state
> which is standby or mem
>
> On Mon, 2014-04-14 at 10:24 +0800, Dongsheng Wang wrote:
> > From: Wang Dongsheng
> >
> > Add set_pm_suspend_state & pm_suspend_state functions to set/get suspend
> > state.
> > When system going to sleep, devices can get the system suspend
> > state(STANDBY/MEM) through pm_suspend_state function and handle different
> situations.
> >
> > Signed-off-by: Wang Dongsheng
> >
> > diff --git a/arch/powerpc/platforms/85xx/common.c
> > b/arch/powerpc/platforms/85xx/common.c
> > index b564b5e..3853d43 100644
> > --- a/arch/powerpc/platforms/85xx/common.c
> > +++ b/arch/powerpc/platforms/85xx/common.c
> > @@ -8,6 +8,7 @@
> >
> > #include
> > #include
> > +#include
> >
> > #include
> > #include
> > @@ -47,6 +48,19 @@ int __init mpc85xx_common_publish_devices(void)
> > {
> > return of_platform_bus_probe(NULL, mpc85xx_common_ids, NULL); }
> > +
> > +static suspend_state_t pm_state;
> > +
> > +void set_pm_suspend_state(suspend_state_t state) {
> > + pm_state = state;
> > +}
> > +
> > +suspend_state_t pm_suspend_state(void) {
> > + return pm_state;
> > +}
>
> These need to be namespaced to indicate that they apply only to mpc85xx.
> Where do you plan on using these from, that mpc85xx can be safely assumed?
>
Mpic timer and PCIe driver.
> This seems like a feature that should be implemented in generic code instead.
>
OK, I will move this to a generic path.
Regards,
-Dongsheng
> -Scott
>
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/2] fsl/mpic_timer: make mpic_timer to support deep sleep feature
> -Original Message-
> From: Wood Scott-B07421
> Sent: Tuesday, April 15, 2014 7:36 AM
> To: Wang Dongsheng-B40534
> Cc: Jin Zhengxiong-R64188; Li Yang-Leo-R58472; Zhao Chenhui-B35336; linuxppc-
> d...@lists.ozlabs.org
> Subject: Re: [PATCH 2/2] fsl/mpic_timer: make mpic_timer to support deep sleep
> feature
>
> On Mon, 2014-04-14 at 10:24 +0800, Dongsheng Wang wrote:
> > From: Wang Dongsheng
> >
> > At T104x platfrom the timer clock will be changed when system going to
> > deep sleep.
>
> Could you elaborate on what is changing and why?
>
Okay.
> > +#include
> > #include
>
> So much for, "The driver currently is only tested on fsl chip, but it can
> potentially support other global timers complying to OpenPIC standard."
>
> > #define FSL_GLOBAL_TIMER 0x1
> > @@ -71,8 +74,10 @@ struct timer_group_priv {
> > struct timer_regs __iomem *regs;
> > struct mpic_timer timer[TIMERS_PER_GROUP];
> > struct list_headnode;
> > + unsigned long idle;
> > unsigned inttimerfreq;
> > - unsigned intidle;
>
> Why?
>
Um... It shouldn't be happened...i will remove this.
> > + unsigned intsuspended_timerfreq;
> > + unsigned intresume_timerfreq;
> > unsigned intflags;
> > spinlock_t lock;
> > void __iomem*group_tcr;
> > @@ -88,6 +93,7 @@ static struct cascade_priv cascade_timer[] = { };
> >
> > static LIST_HEAD(timer_group_list);
> > +static int switch_freq_flag;
>
> Needs documentation, and based on "_flag" it should probably be a bool.
>
Okay.
> > static void convert_ticks_to_time(struct timer_group_priv *priv,
> > const u64 ticks, struct timeval *time) @@ -423,6 +429,33 @@
> > struct
> > mpic_timer *mpic_request_timer(irq_handler_t fn, void *dev, }
> > EXPORT_SYMBOL(mpic_request_timer);
> >
> > +static void timer_group_get_suspended_freq(struct timer_group_priv
> > +*priv) {
> > + struct device_node *np;
> > +
> > + np = of_find_compatible_node(NULL, NULL, "fsl,qoriq-clockgen-2.0");
> > + if (!np) {
> > + pr_err("mpic timer: Missing clockgen device node.\n");
>
> Why is it an error to not have a 2.0 QorIQ clockgen?
>
This will affect the accuracy of the timer. But not means the timer cannot work.
Maybe you are right, this pr_err should be a pr_warn.
> > + return;
> > + }
> > +
> > + of_property_read_u32(np, "clock-frequency", &priv->suspended_timerfreq);
> > + of_node_put(np);
>
> Shouldn't this go through the clock API?
>
Sorry, I'm not clear about clock API, you mean fsl_get_sys_freq()? Or ?
The timer operates on sys_ref_clk frequency during deep sleep. And The timer
runs on
platform clock/2 during normal operation.
fsl_get_sys_freq() can get platform clock, but cannot get sys_ref_clk.
> > +}
> > +
> > +static int need_to_switch_freq(void)
> > +{
> > + u32 svr;
> > +
> > + svr = mfspr(SPRN_SVR);
> > + if (SVR_SOC_VER(svr) == SVR_T1040 ||
> > + SVR_SOC_VER(svr) == SVR_T1042)
> > + return 1;
>
> Explain why this is specific to T104x.
>
Mpic timer freq will be change when system going to deep sleep. So we need to
recalculate the time.
> -Scott
>
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/2] fsl/pci: fix EP device sometimes hangup when system resume from sleep
Hi all,
Please ignore this patch. :(
I will resend it. Because I found when e1000 card plug in P1020 PCIe slot 2, we
need more delay time
to let EP device return back.
Regards,
-Dongsheng
> -Original Message-
> From: Dongsheng Wang [mailto:dongsheng.w...@freescale.com]
> Sent: Tuesday, April 15, 2014 3:43 PM
> To: Wood Scott-B07421
> Cc: Zang Roy-R61911; Jin Zhengxiong-R64188; linuxppc-dev@lists.ozlabs.org;
> Wang
> Dongsheng-B40534
> Subject: [PATCH 2/2] fsl/pci: fix EP device sometimes hangup when system
> resume
> from sleep
>
> From: Wang Dongsheng
>
> Root cause is pcie power management state transition need a delay.
> The delay time define in "PCI Bus Power Management Interface Specification".
>
> D0, D1 or D2 --> D3 need to delay 10ms.
> D3 --> D0 need to delay 10ms.
>
> Signed-off-by: Wang Dongsheng
>
> diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
> index 4bd091a..33950ad 100644
> --- a/arch/powerpc/sysdev/fsl_pci.c
> +++ b/arch/powerpc/sysdev/fsl_pci.c
> @@ -1175,15 +1175,24 @@ static void send_pme_turnoff_message(struct
> pci_controller *hose)
> setbits32(&pci->pex_pmcr, PEX_PMCR_PTOMR);
>
> /* Wait trun off done */
> - for (i = 0; i < 150; i++) {
> + /* RC will get this detect quickly */
> + for (i = 0; i < 50; i++) {
> dr = in_be32(&pci->pex_pme_mes_dr);
> - if (dr) {
> + if (dr & ENL23_DETECT_BIT) {
> out_be32(&pci->pex_pme_mes_dr, dr);
> break;
> }
>
> udelay(1000);
> }
> +
> + /*
> + * "PCI Bus Power Management Interface Specification" define
> + * Minimum System Software Guaranteed Delays
> + *
> + * D0, D1 or D2 --> D3, need delay 10ms.
> + */
> + mdelay(10);
> }
>
> static void fsl_pci_syscore_do_suspend(struct pci_controller *hose)
> @@ -1211,9 +1220,10 @@ static void fsl_pci_syscore_do_resume(struct
> pci_controller *hose)
> setbits32(&pci->pex_pmcr, PEX_PMCR_EXL2S);
>
> /* Wait exit done */
> - for (i = 0; i < 150; i++) {
> + /* RC will get this detect quickly */
> + for (i = 0; i < 50; i++) {
> dr = in_be32(&pci->pex_pme_mes_dr);
> - if (dr) {
> + if (dr & EXL23_DETECT_BIT) {
> out_be32(&pci->pex_pme_mes_dr, dr);
> break;
> }
> @@ -1221,6 +1231,14 @@ static void fsl_pci_syscore_do_resume(struct
> pci_controller *hose)
> udelay(1000);
> }
>
> + /*
> + * "PCI Bus Power Management Interface Specification" define
> + * Minimum System Software Guaranteed Delays
> + *
> + * D3 hot --> D0, need delay 10ms.
> + */
> + mdelay(10);
> +
> setup_pci_atmu(hose);
> }
>
> diff --git a/arch/powerpc/sysdev/fsl_pci.h b/arch/powerpc/sysdev/fsl_pci.h
> index c1cec77..37fc644 100644
> --- a/arch/powerpc/sysdev/fsl_pci.h
> +++ b/arch/powerpc/sysdev/fsl_pci.h
> @@ -39,6 +39,9 @@ struct platform_device;
> #define PME_DISR_EN_ENL23D 0x2000
> #define PME_DISR_EN_EXL23D 0x1000
>
> +#define ENL23_DETECT_BIT0x2000
> +#define EXL23_DETECT_BIT0x1000
> +
> /* PCI/PCI Express outbound window reg */
> struct pci_outbound_window_regs {
> __be32 potar; /* 0x.0 - Outbound translation address register */
> --
> 1.8.5
>
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH v2 1/2] fsl/corenet_generic: add a particular initialization for platform
> -Original Message- > From: Wood Scott-B07421 > Sent: Wednesday, April 16, 2014 3:39 AM > To: Wang Dongsheng-B40534 > Cc: Jin Zhengxiong-R64188; haoke...@gmail.com; Kushwaha Prabhakar-B32579; > linuxppc-dev@lists.ozlabs.org > Subject: Re: [PATCH v2 1/2] fsl/corenet_generic: add a particular > initialization > for platform > > On Tue, 2014-04-15 at 13:53 +0800, Dongsheng Wang wrote: > > From: Wang Dongsheng > > > > Corenet_generic is a generic platform initialization. Those based on > > the corenet_generic board maybe need a particular initialize to > > enable/set some IP-Blocks. So add "Fix Generic Initialization" to solve > > this kind of special cases. > > I still don't understand what you mean by "fix". What are you fixing, > or what is fixed? > > There is no need for adding an infrastructure layer here. Just add a > new piece of code for t104x diu, and have it be called by an appropriate > initfunc. > "fix" is means to handle some boards those based on corenet_generic config file, But those boards may need some special handle. Perhaps these used to handle special feature codes not have an appropriate initfunc we cannot *just find* an appropriate place, if more and more boards need to do this, at that time maybe *initfunc* looks very complicated. So we need this "fix" layer to deal them. We need a plan to fix those boards, not just find an initfunc. I thinks "fix" is the best initfunc to handle those special things. If "fix" is not clear, did you have a good idear? :) > > Signed-off-by: Wang Dongsheng > > --- > > *v2* > > 1/ Split DIU code. > > 2/ make fix.c as a independent driver. > > diff --git a/arch/powerpc/platforms/85xx/Kconfig > b/arch/powerpc/platforms/85xx/Kconfig > > index c17aae8..fce2341 100644 > > --- a/arch/powerpc/platforms/85xx/Kconfig > > +++ b/arch/powerpc/platforms/85xx/Kconfig > > @@ -269,6 +269,17 @@ config CORENET_GENERIC > > The following boards are supported for both 32bit and 64bit kernel: > > P5020 DS and P5040 DS > > > > +config FIX_GENERIC_PLATFORM_INIT > > + bool "Fix Generic Initialization" > > + depends on CORENET_GENERIC > > Why does this depend on CORENET_GENERIC? > Because CORENET_GENERIC is a multiboards file, This is designed to handle this situation. > > + default y > > No. > Why not? This will not increase any redundant operations if there is not any boards need fix. You can see my fix.c code. Regards, -Dongsheng > -Scott > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 2/2] fsl/mpic_timer: make mpic_timer to support deep sleep feature
> -Original Message-
> From: Wood Scott-B07421
> Sent: Wednesday, April 16, 2014 5:06 AM
> To: Wang Dongsheng-B40534
> Cc: Jin Zhengxiong-R64188; Li Yang-Leo-R58472; Zhao Chenhui-B35336; linuxppc-
> d...@lists.ozlabs.org
> Subject: Re: [PATCH 2/2] fsl/mpic_timer: make mpic_timer to support deep sleep
> feature
>
> On Mon, 2014-04-14 at 22:23 -0500, Wang Dongsheng-B40534 wrote:
> >
> > > -Original Message-
> > > From: Wood Scott-B07421
> > > Sent: Tuesday, April 15, 2014 7:36 AM
> > > To: Wang Dongsheng-B40534
> > > Cc: Jin Zhengxiong-R64188; Li Yang-Leo-R58472; Zhao Chenhui-B35336;
> linuxppc-
> > > d...@lists.ozlabs.org
> > > Subject: Re: [PATCH 2/2] fsl/mpic_timer: make mpic_timer to support deep
> sleep
> > > feature
> > >
> > > On Mon, 2014-04-14 at 10:24 +0800, Dongsheng Wang wrote:
> > > > From: Wang Dongsheng
> > > >
> > > > At T104x platfrom the timer clock will be changed when system going to
> > > > deep sleep.
> > >
> > > Could you elaborate on what is changing and why?
> > >
> >
> > Okay.
> >
> > > > +#include
> > > > #include
> > >
> > > So much for, "The driver currently is only tested on fsl chip, but it can
> > > potentially support other global timers complying to OpenPIC standard."
> > >
> > > > #define FSL_GLOBAL_TIMER 0x1
> > > > @@ -71,8 +74,10 @@ struct timer_group_priv {
> > > > struct timer_regs __iomem *regs;
> > > > struct mpic_timer timer[TIMERS_PER_GROUP];
> > > > struct list_headnode;
> > > > + unsigned long idle;
> > > > unsigned inttimerfreq;
> > > > - unsigned intidle;
> > >
> > > Why?
> > >
> >
> > Um... It shouldn't be happened...i will remove this.
> >
> > > > + unsigned intsuspended_timerfreq;
> > > > + unsigned intresume_timerfreq;
> > > > unsigned intflags;
> > > > spinlock_t lock;
> > > > void __iomem*group_tcr;
> > > > @@ -88,6 +93,7 @@ static struct cascade_priv cascade_timer[] = { };
> > > >
> > > > static LIST_HEAD(timer_group_list);
> > > > +static int switch_freq_flag;
> > >
> > > Needs documentation, and based on "_flag" it should probably be a bool.
> > >
> >
> > Okay.
> >
> > > > static void convert_ticks_to_time(struct timer_group_priv *priv,
> > > > const u64 ticks, struct timeval *time) @@ -423,6
> > > > +429,33 @@
> struct
> > > > mpic_timer *mpic_request_timer(irq_handler_t fn, void *dev, }
> > > > EXPORT_SYMBOL(mpic_request_timer);
> > > >
> > > > +static void timer_group_get_suspended_freq(struct timer_group_priv
> > > > +*priv) {
> > > > + struct device_node *np;
> > > > +
> > > > + np = of_find_compatible_node(NULL, NULL,
> > > > "fsl,qoriq-clockgen-2.0");
> > > > + if (!np) {
> > > > + pr_err("mpic timer: Missing clockgen device node.\n");
> > >
> > > Why is it an error to not have a 2.0 QorIQ clockgen?
> > >
> >
> > This will affect the accuracy of the timer. But not means the timer cannot
> work.
> > Maybe you are right, this pr_err should be a pr_warn.
>
> What I mean is, what if the mpic timer driver is used with deep sleep on
> a different chip such as mpc8536?
>
Only T104x has this feature, other platform will not be effect.
I will remove this pr_err.
> > > > + return;
> > > > + }
> > > > +
> > > > + of_property_read_u32(np, "clock-frequency", &priv-
> >suspended_timerfreq);
> > > > + of_node_put(np);
> > >
> > > Shouldn't this go through the clock API?
> > >
> >
> > Sorry, I'm not clear about clock API, you mean fsl_get_sys_freq()? Or ?
>
> No, I mean drivers/clk/
>
> Though I suppose manually reading clock-frequency is OK, as the
> clock-frequency on the clockgen node predated the introduction of clock
> bindings to the device tree.
>
> Don't use fsl_get_sys_freq().
>
No, we cannot use drivers/clk/. Because clk-ppc-corenet.c only support corenet
platform.
> > > > +static int need_to_switch_freq(void)
> > > > +{
> > > > + u32 svr;
> > > > +
> > > > + svr = mfspr(SPRN_SVR);
> > > > + if (SVR_SOC_VER(svr) == SVR_T1040 ||
> > > > + SVR_SOC_VER(svr) == SVR_T1042)
> > > > + return 1;
> > >
> > > Explain why this is specific to T104x.
> > >
> >
> > Mpic timer freq will be change when system going to deep sleep. So we need
> > to
> recalculate the time.
>
> Do any other chips with deep sleep have this problem?
>
Only has on T104x right now.
Regards,
-Dongsheng
> -Scott
>
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Powerpc: e1000e cannot work normally after system resume from sleep(standby)
Hi all,
Now I'm developing Freescale PCIe power management feature. The following is my
PCIe
suspend/resume code.
when I test system wake up from sleep(STANDBY), I got below calltrace. Looks
like e1000e
cannot transfer data, maybe watchdog has some issue. Or maybe some of the other
causes.
I think this is not root cause. Because If changed mdelay(10) to 20ms. E1000
will work normal.
Maybe e1000e need more time to recover? Or ?
Here are my test results:
E1000e plug in PCIe slot 1, we need to delay 10ms.
E1000e plug in PCIe slot 2, we need to delay 20ms.
E1000e plug in PCIe slot 3, we need to delay 60ms.
Anyone have any idea about this?
*Suspend flow*:
if (fsl_pcie_check_link(hose))
return;
/* Send PME_Turn_Off Message Request */
setbits32(&pci->pex_pmcr, PEX_PMCR_PTOMR);
/* Wait trun off done */
/* RC will get this detect quickly */
for (i = 0; i < 50; i++) {
dr = in_be32(&pci->pex_pme_mes_dr);
if (dr & ENL23_DETECT_BIT) {
out_be32(&pci->pex_pme_mes_dr, dr);
break;
}
udelay(1000);
}
/*
* "PCI Bus Power Management Interface Specification" define
* Minimum System Software Guaranteed Delays
*
* D0, D1 or D2 --> D3, need delay 10ms.
* But we need to delay more time when EP plug in p1022 slot2.
*/
mdelay(10);
*Resume flow*:
if (fsl_pcie_check_link(hose))
return;
/* Send Exit L2 State Message */
setbits32(&pci->pex_pmcr, PEX_PMCR_EXL2S);
/* Wait exit done */
/* RC will get this detect quickly */
for (i = 0; i < 50; i++) {
dr = in_be32(&pci->pex_pme_mes_dr);
if (dr & EXL23_DETECT_BIT) {
out_be32(&pci->pex_pme_mes_dr, dr);
break;
}
udelay(1000);
}
/*
* "PCI Bus Power Management Interface Specification" define
* Minimum System Software Guaranteed Delays
*
* D3 hot --> D0, need delay 10ms.
* But we need to delay more time when EP plug in p1022 slot2.
*/
mdelay(10);
CALL TRACE
[root@p1022 /root]# udhcpc -i eth0
udhcpc (v1.11.2) started
IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready
Sending discover...
e1000e: eth0 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: Rx/Tx
IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
Sending discover...
Sending discover...
NETDEV WATCHDOG: eth0 (e1000e): transmit queue 0 timed out
[ cut here ]
WARNING: at net/sched/sch_generic.c:264
Modules linked in:
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 3.14.0-173871-g9964256-dirty #42
task: ee0534e0 ti: ee07 task.ti: ee07
NIP: c036267c LR: c036267c CTR: c028b834
REGS: ee071d00 TRAP: 0700 Not tainted (3.14.0-173871-g9964256-dirty)
MSR: 00029000 CR: 4222 XER: 2000
GPR00: c036267c ee071db0 ee0534e0 003a c16393b0 c16398e8 01079000 c05c03a8
GPR08: 0001 0001 01079000 0132 0132 0004 c05ed040
GPR16: 0001 0100 0004 fffef539 c05c10c0 0038
GPR24: 0001 ee07 0004 c05e c05f ee3c
NIP [c036267c] dev_watchdog+0x2c8/0x2d8
LR [c036267c] dev_watchdog+0x2c8/0x2d8
Call Trace:
[ee071db0] [c036267c] dev_watchdog+0x2c8/0x2d8 (unreliable)
[ee071de0] [c004778c] call_timer_fn.isra.24+0x28/0x84
[ee071e00] [c00479b8] run_timer_softirq+0x1d0/0x248
[ee071e40] [c004090c] __do_softirq+0x104/0x20c
[ee071ea0] [c0040cd8] irq_exit+0xa4/0xc8
[ee071eb0] [c0009ef4] timer_interrupt+0xb8/0xdc
[ee071ed0] [c000f57c] ret_from_except+0x0/0x18
--- Exception: 901 at arch_cpu_idle+0x24/0x5c
LR = arch_cpu_idle+0x24/0x5c
[ee071f90] [c0089534] rcu_idle_enter+0xa4/0xd0 (unreliable)
[ee071fa0] [c0076010] cpu_startup_entry+0x120/0x17c
[ee071fd0] [c0010bbc] start_secondary+0x214/0x224
[ee071ff0] [c0002154] __secondary_start+0x7c/0xc8
Instruction dump:
4e800421 80fe0224 4b4c 7fc3f378 4bfe5401 7fc4f378 7c651b78 3c60c055
7fe6fb78 3863e0f4 4cc63182 480e9a05 <0fe0> 3921 993c6f38 4bb4
---[ end trace 2d4f2cedfa86e32f ]---
e1000e 0002:05:00.0: Net device Info
e1000e: Device Name statetrans_start last_rx
e1000e: eth00003 FFFEF040
e1000e 0002:05:00.0: Register Dump
e1000e: Register Name Value
e1000e: CTRL58100248
e1000e: STATUS 00080783
e1000e: CTRL_EXT1858
e1000e: ICR
e1000e: RCTL04008002
e1000e: RDLEN 1000
e1000e: RDH 001d
e1000e: RDT 00f0
e1000e: RDTR0020
e1000e: RXDCTL[0-1] 01040420 01040420
e1000e: ERT
e1000e: RDBAL 2d39
e1000e: RDBAH
e1000e: RDFH020a
e1000e: RDFT020a
e1000e: RDFHS 020a
e1000e: RDFTS
RE: [PATCH] fsl/smp: add low power boot support to replace spin boot
Hi all,
U-boot patch link:
http://patchwork.ozlabs.org/patch/429265/
Regards,
-Dongsheng
> -Original Message-
> From: Dongsheng Wang [mailto:dongsheng.w...@freescale.com]
> Sent: Thursday, January 15, 2015 2:06 PM
> To: Wood Scott-B07421; Sun York-R58495; Li Yang-Leo-R58472
> Cc: linuxppc-dev@lists.ozlabs.org; Wang Dongsheng-B40534
> Subject: [PATCH] fsl/smp: add low power boot support to replace spin boot
>
> From: Wang Dongsheng
>
> U-boot put non-boot cpus into an low power state(PW10/PW20 or DOZE) when cpu
> powered up. To exit low power state kernel will send DOORBELL or MPIC-IPI
> signal to all those CPUs.
>
> e500/e500v2 use mpic to send IPI signal.
> e500mc and later use doorbell to send IPI signal.
>
> This feature tested on:
> POWER UP TEST:
> P1022DS(e500v2),96k times.
> P4080(e500mc), 110k times.
> T1024(e5500), 83k times.
> T4240(e6500), 150k times.
>
> CPU HOTPLUG TEST:
> P1022DS(e500v2),1.4 million times.
> P4080(e500mc), 1.8 million times.
> T1024(e5500), 1.3 million times.
> T4240(e6500), 1.1 million times.
>
> Signed-off-by: Wang Dongsheng
>
> diff --git a/arch/powerpc/include/asm/mpic.h b/arch/powerpc/include/asm/mpic.h
> index 754f93d..8af6a25 100644
> --- a/arch/powerpc/include/asm/mpic.h
> +++ b/arch/powerpc/include/asm/mpic.h
> @@ -474,6 +474,15 @@ extern int mpic_cpu_get_priority(void);
> /* Set the current cpu priority for this cpu */
> extern void mpic_cpu_set_priority(int prio);
>
> +/* Set cpu priority */
> +void mpic_set_cpu_priority(int nr, int prio);
> +
> +/* Set cpu EOI */
> +void mpic_cpu_eoi_write(int cpu);
> +
> +/* CPU ACK interrupt */
> +void mpic_cpu_ack(int cpu);
> +
> /* Request IPIs on primary mpic */
> extern void mpic_request_ipis(void);
>
> diff --git a/arch/powerpc/platforms/85xx/smp.c
> b/arch/powerpc/platforms/85xx/smp.c
> index d7c1e69..6c54632 100644
> --- a/arch/powerpc/platforms/85xx/smp.c
> +++ b/arch/powerpc/platforms/85xx/smp.c
> @@ -193,6 +193,30 @@ static int smp_85xx_kick_cpu(int nr)
> const u64 *cpu_rel_addr;
> __iomem struct epapr_spin_table *spin_table;
> struct device_node *np;
> +
> + /*
> + * DOORBELL:
> + * When kernel kick one of cpus, all cpus will be wakenup. To make
> + * sure that only the target cpu is effected, other cpus (by checking
> + * spin_table->addr_l) should go back to low power state.
> + *
> + * U-boot has renumber the cpu PIR Why we need to set all of PIR to
> + * the same value?
> + * A: Before kernel kicking cpu, the doorbell message was not configured
> + * for target cpu(cpu_messages->data). If we try to send a
> + * non-configured message to target cpu, it cannot correctly receive
> + * doorbell interrput. So SET ALL OF CPU'S PIR to the same value to
> + * let all cpus catch the interrupt.
> + *
> + * Why set PIR to zero?
> + * A: U-boot cannot know how many cpus will be kicked up(Kernel allow us
> + * to configure NR_CPUS) and IPI is a per_cpu variable, u-boot cannot
> + * set a appropriate PIR for every cpu, but the boot cpu(CPU0) always be
> + * there. U-boot set PIR to zero as a default PIR ID for each CPU, so
> + * initialize the kick_cpus to 0.
> + */
> + u32 kick_cpus = 0;
> +
> int hw_cpu = get_hard_smp_processor_id(nr);
> int ioremappable;
> int ret = 0;
> @@ -251,8 +275,7 @@ static int smp_85xx_kick_cpu(int nr)
> spin_table = phys_to_virt(*cpu_rel_addr);
>
> local_irq_save(flags);
> -#ifdef CONFIG_PPC32
> -#ifdef CONFIG_HOTPLUG_CPU
> +#if defined(CONFIG_PPC32) && defined(CONFIG_HOTPLUG_CPU)
> /* Corresponding to generic_set_cpu_dead() */
> generic_set_cpu_up(nr);
>
> @@ -292,11 +315,58 @@ static int smp_85xx_kick_cpu(int nr)
> __secondary_hold_acknowledge = -1;
> }
> #endif
> +
> flush_spin_table(spin_table);
> - out_be32(&spin_table->pir, hw_cpu);
> + /*
> + * U-boot will wait kernel send eoi to MPIC, after EOI has send
> + * kernel will set PIR for uboot, let uboot know EOI has send.
> + */
> + out_be32(&spin_table->pir, 0);
> +
> +#ifdef CONFIG_PPC32
> out_be32(&spin_table->addr_l, __pa(__early_start));
> +#else
> + out_be64((u64 *)(&spin_table->addr_h),
> + __pa(ppc_function_entry(generic_secondary_smp_init)));
> +#endif
> flush_spin_table(spin_table);
>
> + /*
> + * e500, e500v2 need to use MPIC to send IPI signal, so we need to
> + * open IPI firstly.
> + */
> + if (!cpu_has_feature(CPU_FTR_DBELL)) {
> + mpic_set_cpu_priority(nr, 0);
> + kick_cpus = nr;
> + }
> +
> + /* Let cpu exit low power state, and from u-boot jump to kernel */
> + arch_send_call_function_single_ipi(kick_cpus);
> +
> + /*
> + * Let we ACK interrput and Send EOI signal to finish INT server
> + * U-boot has read EPR to ACK interrput when MPIC work in external
> + *
RE: [PATCH] fsl/smp: add low power boot support to replace spin boot
Thanks for your review. > -Original Message- > From: Sun York-R58495 > Sent: Friday, January 16, 2015 1:09 AM > To: Wang Dongsheng-B40534; Wood Scott-B07421; Li Yang-Leo-R58472 > Cc: linuxppc-dev@lists.ozlabs.org > Subject: Re: [PATCH] fsl/smp: add low power boot support to replace spin boot > > > > On 01/14/2015 10:05 PM, Dongsheng Wang wrote: > > From: Wang Dongsheng > > > > U-boot put non-boot cpus into an low power state(PW10/PW20 or DOZE) > > when cpu powered up. To exit low power state kernel will send DOORBELL > > or MPIC-IPI signal to all those CPUs. > > > > e500/e500v2 use mpic to send IPI signal. > > e500mc and later use doorbell to send IPI signal. > > > > This feature tested on: > > POWER UP TEST: > > P1022DS(e500v2),96k times. > > P4080(e500mc), 110k times. > > T1024(e5500), 83k times. > > T4240(e6500), 150k times. > > > > CPU HOTPLUG TEST: > > P1022DS(e500v2),1.4 million times. > > P4080(e500mc), 1.8 million times. > > T1024(e5500), 1.3 million times. > > T4240(e6500), 1.1 million times. > > > > Signed-off-by: Wang Dongsheng > > > > This is a very good move. Can you explain what has been tested for 100K times, > or over a million times? > Thanks, I tested linux boot process and cpu hotplug. For Linux boot process test: Each CPU can normally be started and random run an application, the application works well. For cpu hotplug test: Just plug out and plug in for each cpu. Cpu hotplug test works well. If I missed some case please let me know. > Have you verified on older e500v2 platforms, such as MPC8572? > I haven't tested MPC8572 and Just test P1022DS. Because is e500 and e500v2 use the same Low power boot codes. I will test this feature on MPC8572 platform. Regards, -Dongsheng ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] powerpc/fsl: add power_off support for fsl platform
> -Original Message-
> From: Wood Scott-B07421
> Sent: Wednesday, February 25, 2015 10:30 AM
> To: Wang Dongsheng-B40534
> Cc: Jin Zhengxiong-R64188; Li Yang-Leo-R58472; Jia Hongtao-B38951; linuxppc-
> d...@lists.ozlabs.org
> Subject: Re: [PATCH] powerpc/fsl: add power_off support for fsl platform
>
> On Wed, 2015-02-04 at 14:47 +0800, Dongsheng Wang wrote:
> > +void ppc_md_fixup(void)
> > +{
>
> This name is way too generic (though it's moot since you shouldn't use ppc_md
> for this).
>
Agree.
> > + struct device_node *np;
> > +
> > + np = of_find_compatible_node(NULL, NULL, "fsl,fpga-qixis");
> > + if (!np)
> > + return;
> > +
> > + of_node_put(np);
> > +
> > + pm_power_off = fsl_power_off;
> > + ppc_md.halt = fsl_power_off;
> > +}
>
> Please implement this as a drivers/power/reset driver, and consider basing on
> top of http://lists.infradead.org/pipermail/linux-arm-kernel/2014-
> October/293089.html
>
Thanks.
Regards,
-Dongsheng
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
