Various e500 core have different cache architecture, so they
need different cache flush operations. Therefore, add a callback
function cpu_flush_caches to the struct cpu_spec. The cache flush
operation for the specific kind of e500 is selected at init time.
The callback function will flush all caches in the current cpu.
Signed-off-by: Chenhui Zhao <redacted>
---
arch/powerpc/include/asm/cacheflush.h | 2 -
arch/powerpc/include/asm/cputable.h | 11 +++
arch/powerpc/kernel/asm-offsets.c | 3 +
arch/powerpc/kernel/cpu_setup_fsl_booke.S | 114 ++++++++++++++++++++++++++++-
arch/powerpc/kernel/cputable.c | 4 +
arch/powerpc/kernel/head_fsl_booke.S | 74 -------------------
arch/powerpc/platforms/85xx/smp.c | 4 +-
7 files changed, 134 insertions(+), 78 deletions(-)
@@ -43,6 +43,13 @@ extern int machine_check_e500(struct pt_regs *regs);externintmachine_check_e200(structpt_regs*regs);externintmachine_check_47x(structpt_regs*regs);+#if defined(CONFIG_E500) || defined(CONFIG_PPC_E500MC)+externvoid__flush_caches_e500v2(void);+externvoid__flush_caches_e500mc(void);+externvoid__flush_caches_e5500(void);+externvoid__flush_caches_e6500(void);+#endif+/* NOTE WELL: Update identify_cpu() if fields are added or removed! */structcpu_spec{/* CPU is matched via (PVR & pvr_mask) == pvr_value */
@@ -59,6 +66,10 @@ struct cpu_spec {unsignedinticache_bsize;unsignedintdcache_bsize;+#if defined(CONFIG_E500) || defined(CONFIG_PPC_E500MC)+/* flush caches of the cpu which is running the function */+void(*cpu_flush_caches)(void);+#endif/* number of performance monitor counters */unsignedintnum_pmcs;enumpowerpc_pmc_typepmc_type;
@@ -648,8 +648,10 @@ static void __early_init_mmu(int boot_cpu)num_cams=(mfspr(SPRN_TLB1CFG)&TLBnCFG_N_ENTRY)/4;linear_map_top=map_mem_in_cams(linear_map_top,num_cams);-/* limit memory so we dont have linear faults */-memblock_enforce_memory_limit(linear_map_top);+if(boot_cpu){+/* limit memory so we dont have linear faults */+memblock_enforce_memory_limit(linear_map_top);+}if(book3e_htw_mode==PPC_HTW_NONE){patch_exception(0x1c0,exc_data_tlb_miss_bolted_book3e);
@@ -46,6 +47,17 @@ static u64 timebase;staticinttb_req;staticinttb_valid;+#ifdef CONFIG_PPC_E500MC+/* specify the cpu PM state when cpu dies, PH15/NAP is the default */+intqoriq_cpu_die_state=E500_PM_PH15;+#endif++#ifdef CONFIG_PPC_E500MC+staticvoidmpc85xx_timebase_freeze(intfreeze)+{+qoriq_pm_ops->freeze_time_base(freeze);+}+#elsestaticvoidmpc85xx_timebase_freeze(intfreeze){uint32_tmask;
@@ -208,11 +250,8 @@ 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-/* Corresponding to generic_set_cpu_dead() */-generic_set_cpu_up(nr);+#ifdef CONFIG_HOTPLUG_CPUif(system_state==SYSTEM_RUNNING){/**Tokeepitcompatiblewitholdbootprogramwhichuses
@@ -225,6 +264,12 @@ static int smp_85xx_kick_cpu(int nr)out_be32(&spin_table->addr_l,0);flush_spin_table(spin_table);+#ifdef CONFIG_PPC_E500MC+/* Due to an erratum, wake the core before reset. */+if(qoriq_pm_ops&&qoriq_pm_ops->cpu_exit_state)+qoriq_pm_ops->cpu_exit_state(nr,qoriq_cpu_die_state);+#endif+/**Wedon'tsettheBPTRregisterheresinceitalreadypoints*tothebootpageproperly.
@@ -247,13 +292,30 @@ static int smp_85xx_kick_cpu(int nr)/* clear the acknowledge status */__secondary_hold_acknowledge=-1;++#ifdef CONFIG_PPC_E500MC+if(qoriq_pm_ops->irq_unmask)+qoriq_pm_ops->irq_unmask(nr);+#endif}++/* Corresponding to generic_set_cpu_dead() */+generic_set_cpu_up(nr);#endif+flush_spin_table(spin_table);out_be32(&spin_table->pir,hw_cpu);+#ifdef CONFIG_PPC32out_be32(&spin_table->addr_l,__pa(__early_start));+#else+out_be32(&spin_table->addr_h,+__pa(*(u64*)generic_secondary_smp_init)>>32);+out_be32(&spin_table->addr_l,+__pa(*(u64*)generic_secondary_smp_init)&0xffffffff);+#endifflush_spin_table(spin_table);+#ifdef CONFIG_PPC32/* Wait a bit for the CPU to ack. */if(!spin_event_timeout(__secondary_hold_acknowledge==hw_cpu,10000,100)){
@@ -262,17 +324,10 @@ static int smp_85xx_kick_cpu(int nr)ret=-ENOENT;gotoout;}-out:#elsesmp_generic_kick_cpu(nr);--flush_spin_table(spin_table);-out_be32(&spin_table->pir,hw_cpu);-out_be64((u64*)(&spin_table->addr_h),-__pa((u64)*((unsignedlonglong*)generic_secondary_smp_init)));-flush_spin_table(spin_table);#endif-+out:local_irq_restore(flags);if(ioremappable)
In sleep mode, the clocks of e500 cores and unused IP blocks is
turned off. The IP blocks which are allowed to wake up the processor
are still running.
The sleep mode is equal to the Standby state in Linux. Use the
command to enter sleep mode:
echo standby > /sys/power/state
Signed-off-by: Chenhui Zhao <redacted>
---
arch/powerpc/Kconfig | 4 +-
arch/powerpc/platforms/85xx/Makefile | 3 +
arch/powerpc/platforms/85xx/qoriq_pm.c | 78 ++++++++++++++++++++++++++++++++
3 files changed, 83 insertions(+), 2 deletions(-)
create mode 100644 arch/powerpc/platforms/85xx/qoriq_pm.c
@@ -2,6 +2,9 @@# Makefile for the PowerPC 85xx linux kernel.#obj-$(CONFIG_SMP)+=smp.o+ifeq ($(CONFIG_FSL_CORENET_RCPM), y)+obj-$(CONFIG_SUSPEND)+=qoriq_pm.o+endifobj-y+=common.o
@@ -0,0 +1,78 @@+/*+*SupportPowerManagementfeature+*+*Copyright2014FreescaleSemiconductorInc.+*+*Author:ChenhuiZhao<chenhui.zhao@freescale.com>+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodifyit+*underthetermsoftheGNUGeneralPublicLicenseaspublishedbythe+*FreeSoftwareFoundation;eitherversion2oftheLicense,or(atyour+*option)anylaterversion.+*/++#include<linux/kernel.h>+#include<linux/suspend.h>+#include<linux/of_platform.h>++#include<sysdev/fsl_soc.h>++#define FSL_SLEEP 0x1+#define FSL_DEEP_SLEEP 0x2++/* specify the sleep state of the present platform */+intsleep_pm_state;+/* supported sleep modes by the present platform */+staticunsignedintsleep_modes;++staticintqoriq_suspend_enter(suspend_state_tstate)+{+intret=0;++switch(state){+casePM_SUSPEND_STANDBY:++if(cur_cpu_spec->cpu_flush_caches)+cur_cpu_spec->cpu_flush_caches();++ret=qoriq_pm_ops->plat_enter_state(sleep_pm_state);++break;++default:+ret=-EINVAL;++}++returnret;+}++staticintqoriq_suspend_valid(suspend_state_tstate)+{+if(state==PM_SUSPEND_STANDBY&&(sleep_modes&FSL_SLEEP))+return1;++return0;+}++staticconststructplatform_suspend_opsqoriq_suspend_ops={+.valid=qoriq_suspend_valid,+.enter=qoriq_suspend_enter,+};++staticint__initqoriq_suspend_init(void)+{+structdevice_node*np;++sleep_modes=FSL_SLEEP;+sleep_pm_state=PLAT_PM_SLEEP;++np=of_find_compatible_node(NULL,NULL,"fsl,qoriq-rcpm-2.0");+if(np)+sleep_pm_state=PLAT_PM_LPM20;++suspend_set_ops(&qoriq_suspend_ops);++return0;+}+arch_initcall(qoriq_suspend_init);
There is a RCPM (Run Control/Power Management) in Freescale QorIQ
series processors. The device performs tasks associated with device
run control and power management.
The driver implements some features: mask/unmask irq, enter/exit low
power states, freeze time base, etc.
There are two versions of register map in RCPM, which is specified by
the compatible entry in the RCPM node of device tree.
Signed-off-by: Chenhui Zhao <redacted>
---
arch/powerpc/include/asm/fsl_guts.h | 105 ++++++++
arch/powerpc/platforms/85xx/Kconfig | 1 +
arch/powerpc/platforms/85xx/corenet_generic.c | 2 +
arch/powerpc/sysdev/Kconfig | 5 +
arch/powerpc/sysdev/Makefile | 1 +
arch/powerpc/sysdev/fsl_rcpm.c | 315 +++++++++++++++++++++++++
arch/powerpc/sysdev/fsl_soc.h | 24 ++
7 files changed, 453 insertions(+), 0 deletions(-)
create mode 100644 arch/powerpc/sysdev/fsl_rcpm.c
@@ -0,0 +1,315 @@+/*+*RCPM(RunControl/PowerManagement)support+*+*Copyright2012-2014FreescaleSemiconductorInc.+*+*Author:ChenhuiZhao<chenhui.zhao@freescale.com>+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodifyit+*underthetermsoftheGNUGeneralPublicLicenseaspublishedbythe+*FreeSoftwareFoundation;eitherversion2oftheLicense,or(atyour+*option)anylaterversion.+*/+#include<linux/types.h>+#include<linux/errno.h>+#include<linux/of_address.h>+#include<linux/export.h>++#include<asm/io.h>+#include<asm/fsl_guts.h>+#include<asm/cputhreads.h>+#include<sysdev/fsl_soc.h>++conststructfsl_pm_ops*qoriq_pm_ops;++staticstructccsr_rcpm_v1__iomem*rcpm_v1_regs;+staticstructccsr_rcpm_v2__iomem*rcpm_v2_regs;++staticvoidrcpm_v1_irq_mask(intcpu)+{+inthw_cpu=get_hard_smp_processor_id(cpu);+unsignedintmask=1<<hw_cpu;++setbits32(&rcpm_v1_regs->cpmimr,mask);+setbits32(&rcpm_v1_regs->cpmcimr,mask);+setbits32(&rcpm_v1_regs->cpmmcmr,mask);+setbits32(&rcpm_v1_regs->cpmnmimr,mask);+}++staticvoidrcpm_v1_irq_unmask(intcpu)+{+inthw_cpu=get_hard_smp_processor_id(cpu);+unsignedintmask=1<<hw_cpu;++clrbits32(&rcpm_v1_regs->cpmimr,mask);+clrbits32(&rcpm_v1_regs->cpmcimr,mask);+clrbits32(&rcpm_v1_regs->cpmmcmr,mask);+clrbits32(&rcpm_v1_regs->cpmnmimr,mask);+}++staticvoidrcpm_v1_set_ip_power(intenable,u32mask)+{+if(enable)+setbits32(&rcpm_v1_regs->ippdexpcr,mask);+else+clrbits32(&rcpm_v1_regs->ippdexpcr,mask);+}++staticvoidrcpm_v1_cpu_enter_state(intcpu,intstate)+{+unsignedinthw_cpu=get_hard_smp_processor_id(cpu);+unsignedintmask=1<<hw_cpu;++switch(state){+caseE500_PM_PH10:+setbits32(&rcpm_v1_regs->cdozcr,mask);+break;+caseE500_PM_PH15:+setbits32(&rcpm_v1_regs->cnapcr,mask);+break;+default:+pr_err("Unknown cpu PM state\n");+break;+}+}++staticvoidrcpm_v1_cpu_exit_state(intcpu,intstate)+{+unsignedinthw_cpu=get_hard_smp_processor_id(cpu);+unsignedintmask=1<<hw_cpu;++switch(state){+caseE500_PM_PH10:+clrbits32(&rcpm_v1_regs->cdozcr,mask);+break;+caseE500_PM_PH15:+clrbits32(&rcpm_v1_regs->cnapcr,mask);+break;+default:+pr_err("Unknown cpu PM state\n");+break;+}+}++staticintrcpm_v1_plat_enter_state(intstate)+{+u32*pmcsr_reg=&rcpm_v1_regs->powmgtcsr;+intret=0;+intresult;++switch(state){+casePLAT_PM_SLEEP:+setbits32(pmcsr_reg,RCPM_POWMGTCSR_SLP);++/* At this point, the device is in sleep mode. */++/* Upon resume, wait for RCPM_POWMGTCSR_SLP bit to be clear. */+result=spin_event_timeout(+!(in_be32(pmcsr_reg)&RCPM_POWMGTCSR_SLP),10000,10);+if(!result){+pr_err("%s: timeout waiting for SLP bit to be cleared\n",+__func__);+ret=-ETIMEDOUT;+}+break;+default:+pr_err("Unsupported platform PM state\n");+ret=-EINVAL;+}++returnret;+}++staticvoidrcpm_v1_freeze_time_base(intfreeze)+{+u32*tben_reg=&rcpm_v1_regs->ctbenr;+staticu32mask;++if(freeze){+mask=in_be32(tben_reg);+clrbits32(tben_reg,mask);+}else{+setbits32(tben_reg,mask);+}++/* read back to push the previous write */+in_be32(tben_reg);+}++staticvoidrcpm_v2_freeze_time_base(intfreeze)+{+u32*tben_reg=&rcpm_v2_regs->pctbenr;+staticu32mask;++if(freeze){+mask=in_be32(tben_reg);+clrbits32(tben_reg,mask);+}else{+setbits32(tben_reg,mask);+}++/* read back to push the previous write */+in_be32(tben_reg);+}++staticvoidrcpm_v2_irq_mask(intcpu)+{+inthw_cpu=get_hard_smp_processor_id(cpu);+unsignedintmask=1<<hw_cpu;++setbits32(&rcpm_v2_regs->tpmimr0,mask);+setbits32(&rcpm_v2_regs->tpmcimr0,mask);+setbits32(&rcpm_v2_regs->tpmmcmr0,mask);+setbits32(&rcpm_v2_regs->tpmnmimr0,mask);+}++staticvoidrcpm_v2_irq_unmask(intcpu)+{+inthw_cpu=get_hard_smp_processor_id(cpu);+unsignedintmask=1<<hw_cpu;++clrbits32(&rcpm_v2_regs->tpmimr0,mask);+clrbits32(&rcpm_v2_regs->tpmcimr0,mask);+clrbits32(&rcpm_v2_regs->tpmmcmr0,mask);+clrbits32(&rcpm_v2_regs->tpmnmimr0,mask);+}++staticvoidrcpm_v2_set_ip_power(intenable,u32mask)+{+if(enable)+/* enable power of IP blocks in deep sleep mode */+setbits32(&rcpm_v2_regs->ippdexpcr[0],mask);+else+clrbits32(&rcpm_v2_regs->ippdexpcr[0],mask);+}++staticvoidrcpm_v2_cpu_enter_state(intcpu,intstate)+{+unsignedinthw_cpu=get_hard_smp_processor_id(cpu);+u32mask=1<<cpu_core_index_of_thread(hw_cpu);++switch(state){+caseE500_PM_PH10:+/* one bit corresponds to one thread for PH10 of 6500 */+setbits32(&rcpm_v2_regs->tph10setr0,1<<hw_cpu);+break;+caseE500_PM_PH15:+setbits32(&rcpm_v2_regs->pcph15setr,mask);+break;+caseE500_PM_PH20:+setbits32(&rcpm_v2_regs->pcph20setr,mask);+break;+caseE500_PM_PH30:+setbits32(&rcpm_v2_regs->pcph30setr,mask);+break;+default:+pr_err("Unsupported cpu PM state\n");+}+}++staticvoidrcpm_v2_cpu_exit_state(intcpu,intstate)+{+unsignedinthw_cpu=get_hard_smp_processor_id(cpu);+u32mask=1<<cpu_core_index_of_thread(hw_cpu);++switch(state){+caseE500_PM_PH10:+setbits32(&rcpm_v2_regs->tph10clrr0,1<<hw_cpu);+break;+caseE500_PM_PH15:+setbits32(&rcpm_v2_regs->pcph15clrr,mask);+break;+caseE500_PM_PH20:+setbits32(&rcpm_v2_regs->pcph20clrr,mask);+break;+caseE500_PM_PH30:+setbits32(&rcpm_v2_regs->pcph30clrr,mask);+break;+default:+pr_err("Unsupported cpu PM state\n");+}+}++staticintrcpm_v2_plat_enter_state(intstate)+{+u32*pmcsr_reg=&rcpm_v2_regs->powmgtcsr;+intret=0;+intresult;++switch(state){+casePLAT_PM_LPM20:+/* clear previous LPM20 status */+setbits32(pmcsr_reg,RCPM_POWMGTCSR_P_LPM20_ST);+/* enter LPM20 status */+setbits32(pmcsr_reg,RCPM_POWMGTCSR_LPM20_RQ);++/* At this point, the device is in LPM20 status. */++/* resume ... */+result=spin_event_timeout(+!(in_be32(pmcsr_reg)&RCPM_POWMGTCSR_LPM20_ST),10000,10);+if(!result){+pr_err("%s: timeout waiting for LPM20 bit to be cleared\n",+__func__);+ret=-ETIMEDOUT;+}+break;+default:+pr_err("Unsupported platform PM state\n");+ret=-EINVAL;+}++returnret;+}++staticconststructfsl_pm_opsqoriq_rcpm_v1_ops={+.irq_mask=rcpm_v1_irq_mask,+.irq_unmask=rcpm_v1_irq_unmask,+.cpu_enter_state=rcpm_v1_cpu_enter_state,+.cpu_exit_state=rcpm_v1_cpu_exit_state,+.plat_enter_state=rcpm_v1_plat_enter_state,+.set_ip_power=rcpm_v1_set_ip_power,+.freeze_time_base=rcpm_v1_freeze_time_base,+};++staticconststructfsl_pm_opsqoriq_rcpm_v2_ops={+.irq_mask=rcpm_v2_irq_mask,+.irq_unmask=rcpm_v2_irq_unmask,+.cpu_enter_state=rcpm_v2_cpu_enter_state,+.cpu_exit_state=rcpm_v2_cpu_exit_state,+.plat_enter_state=rcpm_v2_plat_enter_state,+.set_ip_power=rcpm_v2_set_ip_power,+.freeze_time_base=rcpm_v2_freeze_time_base,+};++intfsl_rcpm_init(void)+{+structdevice_node*np;++np=of_find_compatible_node(NULL,NULL,"fsl,qoriq-rcpm-2.0");+if(np){+rcpm_v2_regs=of_iomap(np,0);+of_node_put(np);+if(!rcpm_v2_regs)+return-ENOMEM;++qoriq_pm_ops=&qoriq_rcpm_v2_ops;++}else{+np=of_find_compatible_node(NULL,NULL,"fsl,qoriq-rcpm-1.0");+if(np){+rcpm_v1_regs=of_iomap(np,0);+of_node_put(np);+if(!rcpm_v1_regs)+return-ENOMEM;++qoriq_pm_ops=&qoriq_rcpm_v1_ops;++}else{+pr_err("%s: can't find the rcpm node.\n",__func__);+return-EINVAL;+}+}++return0;+}
From: Wang Dongsheng <redacted>
Add booke_cpu_state_save() and booke_cpu_state_restore() functions which can be
used to save/restore CPU's registers in the case of deep sleep and hibernation.
Supported processors: E6500, E5500, E500MC, E500v2 and E500v1.
Signed-off-by: Wang Dongsheng <redacted>
Signed-off-by: Chenhui Zhao <redacted>
---
arch/powerpc/include/asm/booke_save_regs.h | 96 ++++++++
arch/powerpc/kernel/Makefile | 1 +
arch/powerpc/kernel/booke_save_regs.S | 361 ++++++++++++++++++++++++++++
3 files changed, 458 insertions(+), 0 deletions(-)
create mode 100644 arch/powerpc/include/asm/booke_save_regs.h
create mode 100644 arch/powerpc/kernel/booke_save_regs.S
From: Zhao Chenhui <redacted>
T1040 supports deep sleep feature, which can switch off most parts of
the SoC when it is in deep sleep mode. This way, it becomes more
energy-efficient.
The DDR controller will also be powered off in deep sleep. Therefore,
the last stage (the latter part of fsl_dp_enter_low) will run without DDR
access. This piece of code and related TLBs will be prefetched.
Due to the different initialization code between 32-bit and 64-bit, they
have seperate resume entry and precedure.
The feature supports 32-bit and 64-bit kernel mode.
Signed-off-by: Zhao Chenhui <redacted>
---
arch/powerpc/include/asm/booke_save_regs.h | 3 +
arch/powerpc/kernel/cpu_setup_fsl_booke.S | 17 ++
arch/powerpc/kernel/head_fsl_booke.S | 30 +++
arch/powerpc/platforms/85xx/Makefile | 2 +-
arch/powerpc/platforms/85xx/deepsleep.c | 201 +++++++++++++++++++
arch/powerpc/platforms/85xx/qoriq_pm.c | 38 ++++
arch/powerpc/platforms/85xx/sleep.S | 295 ++++++++++++++++++++++++++++
arch/powerpc/sysdev/fsl_soc.h | 7 +
8 files changed, 592 insertions(+), 1 deletions(-)
create mode 100644 arch/powerpc/platforms/85xx/deepsleep.c
create mode 100644 arch/powerpc/platforms/85xx/sleep.S
@@ -0,0 +1,201 @@+/*+*Supportdeepsleepfeature+*+*Copyright2014FreescaleSemiconductorInc.+*+*Author:ChenhuiZhao<chenhui.zhao@freescale.com>+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodifyit+*underthetermsoftheGNUGeneralPublicLicenseaspublishedbythe+*FreeSoftwareFoundation;eitherversion2oftheLicense,or(atyour+*option)anylaterversion.+*/++#include<linux/kernel.h>+#include<linux/of_platform.h>+#include<linux/of_address.h>+#include<asm/machdep.h>+#include<sysdev/fsl_soc.h>+#include<asm/booke_save_regs.h>++#define SIZE_1MB 0x100000+#define SIZE_2MB 0x200000++#define CCSR_SCFG_DPSLPCR 0xfc000+#define CCSR_SCFG_DPSLPCR_WDRR_EN 0x1+#define CCSR_SCFG_SPARECR2 0xfc504+#define CCSR_SCFG_SPARECR3 0xfc508++#define CCSR_GPIO1_GPDIR 0x130000+#define CCSR_GPIO1_GPODR 0x130004+#define CCSR_GPIO1_GPDAT 0x130008+#define CCSR_GPIO1_GPDIR_29 0x4++/* 128 bytes buffer for restoring data broke by DDR training initialization */+#define DDR_BUF_SIZE 128+staticu8ddr_buff[DDR_BUF_SIZE]__aligned(64);++staticvoid*dcsr_base,*ccsr_base,*pld_base;+staticintpld_flag;++intfsl_dp_iomap(void)+{+structdevice_node*np;+constu32*prop;+intret=0;+u64ccsr_phy_addr,dcsr_phy_addr;++np=of_find_node_by_type(NULL,"soc");+if(!np){+pr_err("%s: Can't find the node of \"soc\"\n",__func__);+ret=-EINVAL;+gotoccsr_err;+}+prop=of_get_property(np,"ranges",NULL);+if(!prop){+pr_err("%s: Can't find the property of \"ranges\"\n",__func__);+of_node_put(np);+ret=-EINVAL;+gotoccsr_err;+}+ccsr_phy_addr=of_translate_address(np,prop+1);+ccsr_base=ioremap((phys_addr_t)ccsr_phy_addr,SIZE_2MB);+of_node_put(np);+if(!ccsr_base){+ret=-ENOMEM;+gotoccsr_err;+}++np=of_find_compatible_node(NULL,NULL,"fsl,dcsr");+if(!np){+pr_err("%s: Can't find the node of \"fsl,dcsr\"\n",__func__);+ret=-EINVAL;+gotodcsr_err;+}+prop=of_get_property(np,"ranges",NULL);+if(!prop){+pr_err("%s: Can't find the property of \"ranges\"\n",__func__);+of_node_put(np);+ret=-EINVAL;+gotodcsr_err;+}+dcsr_phy_addr=of_translate_address(np,prop+1);+dcsr_base=ioremap((phys_addr_t)dcsr_phy_addr,SIZE_1MB);+of_node_put(np);+if(!dcsr_base){+ret=-ENOMEM;+gotodcsr_err;+}++np=of_find_compatible_node(NULL,NULL,"fsl,fpga-qixis");+if(np){+pld_flag=FPGA_FLAG;+}else{+np=of_find_compatible_node(NULL,NULL,"fsl,p104xrdb-cpld");+if(np){+pld_flag=CPLD_FLAG;+}else{+pr_err("%s: Can't find the FPGA/CPLD node\n",+__func__);+ret=-EINVAL;+gotopld_err;+}+}+pld_base=of_iomap(np,0);+of_node_put(np);++return0;++pld_err:+iounmap(dcsr_base);+dcsr_err:+iounmap(ccsr_base);+ccsr_err:+ccsr_base=NULL;+dcsr_base=NULL;+pld_base=NULL;+returnret;+}++voidfsl_dp_iounmap(void)+{+if(dcsr_base){+iounmap(dcsr_base);+dcsr_base=NULL;+}++if(ccsr_base){+iounmap(ccsr_base);+ccsr_base=NULL;+}++if(pld_base){+iounmap(pld_base);+pld_base=NULL;+}+}++staticvoidfsl_dp_ddr_save(void*ccsr_base)+{+u32ddr_buff_addr;++/*+*DDRtraininginitializationwillbreak128bytesatthebeginning+*ofDDR,therefore,savethemsothatthebootloaderwillrestore+*them.AssumethatDDRismappedtotheaddressspacestartedwith+*CONFIG_PAGE_OFFSET.+*/+memcpy(ddr_buff,(void*)CONFIG_PAGE_OFFSET,DDR_BUF_SIZE);++/* assume ddr_buff is in the physical address space of 4GB */+ddr_buff_addr=(u32)(__pa(ddr_buff)&0xffffffff);++/*+*thebootloaderwillrestorethefirst128bytesofDDRfrom+*thelocationindicatedbytheregisterSPARECR3+*/+out_be32(ccsr_base+CCSR_SCFG_SPARECR3,ddr_buff_addr);+}++staticvoidfsl_dp_set_resume_pointer(void*ccsr_base)+{+u32resume_addr;++/* the bootloader will finally jump to this address to return kernel */+#ifdef CONFIG_PPC32+resume_addr=(u32)(__pa(__entry_deep_sleep));+#else+resume_addr=(u32)(__pa(*(u64*)__entry_deep_sleep)&0xffffffff);+#endif++/* use the register SPARECR2 to save the resume address */+out_be32(ccsr_base+CCSR_SCFG_SPARECR2,resume_addr);++}++intfsl_enter_epu_deepsleep(void)+{++fsl_dp_ddr_save(ccsr_base);++fsl_dp_set_resume_pointer(ccsr_base);++/* enable Warm Device Reset request. */+setbits32(ccsr_base+CCSR_SCFG_DPSLPCR,CCSR_SCFG_DPSLPCR_WDRR_EN);++/* set GPIO1_29 as an output pin (not open-drain), and output 0 */+clrbits32(ccsr_base+CCSR_GPIO1_GPDAT,CCSR_GPIO1_GPDIR_29);+clrbits32(ccsr_base+CCSR_GPIO1_GPODR,CCSR_GPIO1_GPDIR_29);+setbits32(ccsr_base+CCSR_GPIO1_GPDIR,CCSR_GPIO1_GPDIR_29);++fsl_dp_fsm_setup(dcsr_base);++fsl_dp_enter_low(ccsr_base,dcsr_base,pld_base,pld_flag);++/* disable Warm Device Reset request */+clrbits32(ccsr_base+CCSR_SCFG_DPSLPCR,CCSR_SCFG_DPSLPCR_WDRR_EN);++fsl_dp_fsm_clean(dcsr_base);++return0;+}
@@ -20,6 +20,8 @@#define FSL_SLEEP 0x1#define FSL_DEEP_SLEEP 0x2+int(*fsl_enter_deepsleep)(void);+/* specify the sleep state of the present platform */intsleep_pm_state;/* supported sleep modes by the present platform */
@@ -28,6 +30,7 @@ static unsigned int sleep_modes;staticintqoriq_suspend_enter(suspend_state_tstate){intret=0;+intcpu;switch(state){casePM_SUSPEND_STANDBY:
@@ -39,6 +42,17 @@ static int qoriq_suspend_enter(suspend_state_t state)break;+casePM_SUSPEND_MEM:++cpu=smp_processor_id();+qoriq_pm_ops->irq_mask(cpu);++ret=fsl_enter_deepsleep();++qoriq_pm_ops->irq_unmask(cpu);++break;+default:ret=-EINVAL;
@@ -52,12 +66,30 @@ static int qoriq_suspend_valid(suspend_state_t state)if(state==PM_SUSPEND_STANDBY&&(sleep_modes&FSL_SLEEP))return1;+if(state==PM_SUSPEND_MEM&&(sleep_modes&FSL_DEEP_SLEEP))+return1;+return0;}+staticintqoriq_suspend_begin(suspend_state_tstate)+{+if(state==PM_SUSPEND_MEM)+returnfsl_dp_iomap();++return0;+}++staticvoidqoriq_suspend_end(void)+{+fsl_dp_iounmap();+}+staticconststructplatform_suspend_opsqoriq_suspend_ops={.valid=qoriq_suspend_valid,.enter=qoriq_suspend_enter,+.begin=qoriq_suspend_begin,+.end=qoriq_suspend_end,};staticint__initqoriq_suspend_init(void)
@@ -71,6 +103,12 @@ static int __init qoriq_suspend_init(void)if(np)sleep_pm_state=PLAT_PM_LPM20;+np=of_find_compatible_node(NULL,NULL,"fsl,t1040-rcpm");+if(np){+fsl_enter_deepsleep=fsl_enter_epu_deepsleep;+sleep_modes|=FSL_DEEP_SLEEP;+}+suspend_set_ops(&qoriq_suspend_ops);return0;
From: Hongbo Zhang <redacted>
In the last stage of deep sleep, software will trigger a Finite
State Machine (FSM) to control the hardware precedure, such as
board isolation, killing PLLs, removing power, and so on.
When the system is waked up by an interrupt, the FSM controls the
hardware to complete the early resume precedure.
This patch configure the EPU FSM preparing for deep sleep.
Signed-off-by: Hongbo Zhang <redacted>
Signed-off-by: Chenhui Zhao <redacted>
---
arch/powerpc/platforms/85xx/Kconfig | 1 +
arch/powerpc/sysdev/fsl_soc.h | 3 +
drivers/platform/Kconfig | 4 +
drivers/platform/Makefile | 1 +
drivers/platform/fsl/Kconfig | 10 +
drivers/platform/fsl/Makefile | 5 +
drivers/platform/fsl/sleep_fsm.c | 415 +++++++++++++++++++++++++++++=
++++++
7 files changed, 439 insertions(+), 0 deletions(-)
create mode 100644 drivers/platform/fsl/Kconfig
create mode 100644 drivers/platform/fsl/Makefile
create mode 100644 drivers/platform/fsl/sleep_fsm.c
@@ -0,0 +1,10 @@+#+# Freescale Specific Power Management Drivers+#++configFSL_SLEEP_FSM+bool+help+ThisdriverconfiguresahardwareFSM(FiniteStateMachine)fordeep= sleep.+TheFSMisusedtofinishclean-upsatthelaststageofsystementer=
ing deep
+ sleep, and also wakes up system when a wake up event happens.
In 64-bit mode, kernel just clears the irq soft-enable flag
in struct paca_struct to disable external irqs. But, in
the case of suspend, irqs should be disabled by hardware.
Therefore, hook a function to ppc_md.suspend_disable_irqs
to really disable irqs.
Signed-off-by: Chenhui Zhao <redacted>
---
arch/powerpc/platforms/85xx/corenet_generic.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
RCPM is not board-specific. Why is this in board code?
quoted hunk
+static void rcpm_v1_cpu_enter_state(int cpu, int state)+{+ unsigned int hw_cpu = get_hard_smp_processor_id(cpu);+ unsigned int mask = 1 << hw_cpu;++ switch (state) {+ case E500_PM_PH10:+ setbits32(&rcpm_v1_regs->cdozcr, mask);+ break;+ case E500_PM_PH15:+ setbits32(&rcpm_v1_regs->cnapcr, mask);+ break;+ default:+ pr_err("Unknown cpu PM state\n");+ break;+ }+}
Put __func__ in error messages -- and for "unknown value" type messages,
print the value.
quoted hunk
+static int rcpm_v1_plat_enter_state(int state)+{+ u32 *pmcsr_reg = &rcpm_v1_regs->powmgtcsr;+ int ret = 0;+ int result;++ switch (state) {+ case PLAT_PM_SLEEP:+ setbits32(pmcsr_reg, RCPM_POWMGTCSR_SLP);++ /* At this point, the device is in sleep mode. */++ /* Upon resume, wait for RCPM_POWMGTCSR_SLP bit to be clear. */+ result = spin_event_timeout(+ !(in_be32(pmcsr_reg) & RCPM_POWMGTCSR_SLP), 10000, 10);+ if (!result) {+ pr_err("%s: timeout waiting for SLP bit to be cleared\n",+ __func__);
Why are you indenting continuation lines with only two spaces (and yet
still not aligning with anything)?
quoted hunk
+ ret = -ETIMEDOUT;+ }+ break;+ default:+ pr_err("Unsupported platform PM state\n");+ ret = -EINVAL;+ }++ return ret;+}++static void rcpm_v1_freeze_time_base(int freeze)+{+ u32 *tben_reg = &rcpm_v1_regs->ctbenr;+ static u32 mask;++ if (freeze) {+ mask = in_be32(tben_reg);+ clrbits32(tben_reg, mask);+ } else {+ setbits32(tben_reg, mask);+ }++ /* read back to push the previous write */+ in_be32(tben_reg);+}++static void rcpm_v2_freeze_time_base(int freeze)+{+ u32 *tben_reg = &rcpm_v2_regs->pctbenr;+ static u32 mask;++ if (freeze) {+ mask = in_be32(tben_reg);+ clrbits32(tben_reg, mask);+ } else {+ setbits32(tben_reg, mask);+ }++ /* read back to push the previous write */+ in_be32(tben_reg);+}
It looks like the only difference between these two functions is how you
calculate tben_reg -- factor the rest out into a single function.
@@ -46,6 +47,17 @@ static u64 timebase;staticinttb_req;staticinttb_valid;+#ifdef CONFIG_PPC_E500MC+/* specify the cpu PM state when cpu dies, PH15/NAP is the default */+intqoriq_cpu_die_state=E500_PM_PH15;+#endif
static? Is there any way to modify this other than modifying source
code?
BTW, QorIQ doesn't imply an e500mc derivative.
From: Scott Wood <hidden> Date: 2014-03-11 23:51:29
On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
quoted hunk
In 64-bit mode, kernel just clears the irq soft-enable flag
in struct paca_struct to disable external irqs. But, in
the case of suspend, irqs should be disabled by hardware.
Therefore, hook a function to ppc_md.suspend_disable_irqs
to really disable irqs.
Signed-off-by: Chenhui Zhao <redacted>
---
arch/powerpc/platforms/85xx/corenet_generic.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
Why the underscore version? Don't you want PACA_IRQ_HARD_DIS to be set?
If hard disabling is appropriate here, shouldn't we do it in
generic_suspend_disable_irqs()?
Are there any existing platforms that supply a
ppc_md.suspend_disable_irqs()? I don't see any when grepping.
-Scott
From: Scott Wood <hidden> Date: 2014-03-12 00:00:35
On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
quoted hunk
In sleep mode, the clocks of e500 cores and unused IP blocks is
turned off. The IP blocks which are allowed to wake up the processor
are still running.
The sleep mode is equal to the Standby state in Linux. Use the
command to enter sleep mode:
echo standby > /sys/power/state
Signed-off-by: Chenhui Zhao <redacted>
---
arch/powerpc/Kconfig | 4 +-
arch/powerpc/platforms/85xx/Makefile | 3 +
arch/powerpc/platforms/85xx/qoriq_pm.c | 78 ++++++++++++++++++++++++++++++++
3 files changed, 83 insertions(+), 2 deletions(-)
create mode 100644 arch/powerpc/platforms/85xx/qoriq_pm.c
@@ -2,6 +2,9 @@# Makefile for the PowerPC 85xx linux kernel.#obj-$(CONFIG_SMP)+=smp.o+ifeq ($(CONFIG_FSL_CORENET_RCPM), y)+obj-$(CONFIG_SUSPEND)+=qoriq_pm.o+endif
There should probably be a kconfig symbol for this.
++/* specify the sleep state of the present platform */+int sleep_pm_state;+/* supported sleep modes by the present platform */+static unsigned int sleep_modes;
Why is one signed and the other unsigned?
quoted hunk
++static int qoriq_suspend_enter(suspend_state_t state)+{+ int ret = 0;++ switch (state) {+ case PM_SUSPEND_STANDBY:++ if (cur_cpu_spec->cpu_flush_caches)+ cur_cpu_spec->cpu_flush_caches();++ ret = qoriq_pm_ops->plat_enter_state(sleep_pm_state);++ break;++ default:+ ret = -EINVAL;++ }++ return ret;+}++static int qoriq_suspend_valid(suspend_state_t state)+{+ if (state == PM_SUSPEND_STANDBY && (sleep_modes & FSL_SLEEP))+ return 1;++ return 0;+}++static const struct platform_suspend_ops qoriq_suspend_ops = {+ .valid = qoriq_suspend_valid,+ .enter = qoriq_suspend_enter,+};++static int __init qoriq_suspend_init(void)+{+ struct device_node *np;++ sleep_modes = FSL_SLEEP;+ sleep_pm_state = PLAT_PM_SLEEP;++ np = of_find_compatible_node(NULL, NULL, "fsl,qoriq-rcpm-2.0");+ if (np)+ sleep_pm_state = PLAT_PM_LPM20;++ suspend_set_ops(&qoriq_suspend_ops);++ return 0;+}+arch_initcall(qoriq_suspend_init);
Why is this not a platform driver? If fsl_pmc can do it...
-Scott
From: Scott Wood <hidden> Date: 2014-03-12 00:10:34
On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
From: Hongbo Zhang <redacted>
In the last stage of deep sleep, software will trigger a Finite
State Machine (FSM) to control the hardware precedure, such as
board isolation, killing PLLs, removing power, and so on.
When the system is waked up by an interrupt, the FSM controls the
hardware to complete the early resume precedure.
This patch configure the EPU FSM preparing for deep sleep.
Signed-off-by: Hongbo Zhang <redacted>
Signed-off-by: Chenhui Zhao <redacted>
@@ -0,0 +1,10 @@+#+# Freescale Specific Power Management Drivers+#++configFSL_SLEEP_FSM+bool+help+ThisdriverconfiguresahardwareFSM(FiniteStateMachine)fordeepsleep.+TheFSMisusedtofinishclean-upsatthelaststageofsystementeringdeep+sleep,andalsowakesupsystemwhenawakeupeventhappens.
From: Scott Wood <hidden> Date: 2014-03-12 00:45:24
On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
quoted hunk
From: Wang Dongsheng <redacted>
Add booke_cpu_state_save() and booke_cpu_state_restore() functions which can be
used to save/restore CPU's registers in the case of deep sleep and hibernation.
Supported processors: E6500, E5500, E500MC, E500v2 and E500v1.
Signed-off-by: Wang Dongsheng <redacted>
Signed-off-by: Chenhui Zhao <redacted>
---
arch/powerpc/include/asm/booke_save_regs.h | 96 ++++++++
arch/powerpc/kernel/Makefile | 1 +
arch/powerpc/kernel/booke_save_regs.S | 361 ++++++++++++++++++++++++++++
3 files changed, 458 insertions(+), 0 deletions(-)
create mode 100644 arch/powerpc/include/asm/booke_save_regs.h
create mode 100644 arch/powerpc/kernel/booke_save_regs.S
Filename says booke, comment says e500.
Filename and comment also fail to point out that this is specifically
for standby/suspend, not for hibernate which is implemented in
swsusp_booke.S/swsusp_asm64.S.
quoted hunk
+ *+ * Author: Wang Dongsheng <dongsheng.wang@freescale.com>+ *+ * Copyright 2014 Freescale Semiconductor Inc.+ *+ * This program is free software; you can redistribute it and/or modify+ * it under the terms of the GNU General Public License version 2 as+ * published by the Free Software Foundation.+ */++#ifndef __ASM_FSL_SLEEP_H+#define __ASM_FSL_SLEEP_H++/*+ * 8 bytes for each register, which is compatible with+ * both 32-bit and 64-bit registers+ *+ * Acronyms:+ * dw(data width) 0x08+ *+ * Map:+ * General-Purpose Registers+ * GPR1(sp) 0+ * GPR2 0x8 (dw * 1)+ * GPR13 - GPR31 0x10 ~ 0xa0 (dw * 2 ~ dw * 20)
Putting these values in a comment separate from the code that defines it
sounds like a good way to get a mismatch between the two.
Call enable_kernel_fp() or similar, rather than saving FP regs here.
Likewise with altivec. And why starting at FPR14? Volatile versus
nonvolatile is irrelevant because Linux doesn't participate in the FP
ABI. Everything is non-volatile *if* we have a user FP context
resident, and everything is volatile otherwise.
PID1/PID2 are e500v1/v2 only -- and Linux doesn't use them outside of
KVM (and you're not in KVM when you're running this code).
Are we ever going to have a non-zero PID at this point?
IAC3-4 are not implemented on e500.
Do we really need to save the debug registers? We're not going to be in
a debugged process when we do suspend. If the concern is kgdb, it
probably needs to be told to get out of the way during suspend for other
reasons.
These are very vague names to be putting in a header file.
quoted hunk
+/*+ * hibernation and deepsleep save/restore different number of registers,+ * use these flags to indicate.+ */+#define HIBERNATION_FLAG 1+#define DEEPSLEEP_FLAG 2
Again, namespacing -- but why is hibernation using this at all? What's
wrong with the existing hibernation support?
From: Scott Wood <hidden> Date: 2014-03-12 01:10:56
On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
quoted hunk
From: Zhao Chenhui <redacted>
T1040 supports deep sleep feature, which can switch off most parts of
the SoC when it is in deep sleep mode. This way, it becomes more
energy-efficient.
The DDR controller will also be powered off in deep sleep. Therefore,
the last stage (the latter part of fsl_dp_enter_low) will run without DDR
access. This piece of code and related TLBs will be prefetched.
Due to the different initialization code between 32-bit and 64-bit, they
have seperate resume entry and precedure.
The feature supports 32-bit and 64-bit kernel mode.
Signed-off-by: Zhao Chenhui <redacted>
---
arch/powerpc/include/asm/booke_save_regs.h | 3 +
arch/powerpc/kernel/cpu_setup_fsl_booke.S | 17 ++
arch/powerpc/kernel/head_fsl_booke.S | 30 +++
arch/powerpc/platforms/85xx/Makefile | 2 +-
arch/powerpc/platforms/85xx/deepsleep.c | 201 +++++++++++++++++++
arch/powerpc/platforms/85xx/qoriq_pm.c | 38 ++++
arch/powerpc/platforms/85xx/sleep.S | 295 ++++++++++++++++++++++++++++
arch/powerpc/sysdev/fsl_soc.h | 7 +
8 files changed, 592 insertions(+), 1 deletions(-)
create mode 100644 arch/powerpc/platforms/85xx/deepsleep.c
create mode 100644 arch/powerpc/platforms/85xx/sleep.S
Why do you come in via the normal kernel entry, versus specifying a
direct entry point for deep sleep resume? How does U-Boot even know
what the normal entry is when resuming?
Be careful of the "beq set_ivor" in the CONFIG_RELOCATABLE section
above. Also you probably don't want the relocation code to run again
when resuming.
quoted hunk
+#if defined(CONFIG_SUSPEND) && defined(CONFIG_FSL_CORENET_RCPM)+_ENTRY(__entry_deep_sleep)+/*+ * Bootloader will jump to here when resuming from deep sleep.+ * After executing the init code in fsl_booke_entry_mapping.S,+ * will jump to the real resume entry.+ */+ li r8, 1+ bl 12f+12: mflr r9+ addi r9, r9, (deep_sleep_flag - 12b)+ stw r8, 0(r9)+ b __early_start+deep_sleep_flag:+ .long 0+#endif
It's a bit ambiguous to say "entry_deep_sleep" when it's resuming rather
than entering...
So you do have a special entry point. Why do you go to __early_start
only to quickly test a flag and branch away?
AFAICT this supports deep sleep on T1040, not on all 85xx that has it.
quoted hunk
+ *+ * Copyright 2014 Freescale Semiconductor Inc.+ *+ * Author: Chenhui Zhao <chenhui.zhao@freescale.com>+ *+ * This program is free software; you can redistribute it and/or modify it+ * under the terms of the GNU General Public License as published by the+ * Free Software Foundation; either version 2 of the License, or (at your+ * option) any later version.+ */++#include <linux/kernel.h>+#include <linux/of_platform.h>+#include <linux/of_address.h>+#include <asm/machdep.h>+#include <sysdev/fsl_soc.h>+#include <asm/booke_save_regs.h>++#define SIZE_1MB 0x100000+#define SIZE_2MB 0x200000++#define CCSR_SCFG_DPSLPCR 0xfc000+#define CCSR_SCFG_DPSLPCR_WDRR_EN 0x1+#define CCSR_SCFG_SPARECR2 0xfc504+#define CCSR_SCFG_SPARECR3 0xfc508++#define CCSR_GPIO1_GPDIR 0x130000+#define CCSR_GPIO1_GPODR 0x130004+#define CCSR_GPIO1_GPDAT 0x130008+#define CCSR_GPIO1_GPDIR_29 0x4++/* 128 bytes buffer for restoring data broke by DDR training initialization */+#define DDR_BUF_SIZE 128+static u8 ddr_buff[DDR_BUF_SIZE] __aligned(64);++static void *dcsr_base, *ccsr_base, *pld_base;+static int pld_flag;++int fsl_dp_iomap(void)+{+ struct device_node *np;+ const u32 *prop;+ int ret = 0;+ u64 ccsr_phy_addr, dcsr_phy_addr;++ np = of_find_node_by_type(NULL, "soc");+ if (!np) {+ pr_err("%s: Can't find the node of \"soc\"\n", __func__);+ ret = -EINVAL;+ goto ccsr_err;+ }+ prop = of_get_property(np, "ranges", NULL);+ if (!prop) {+ pr_err("%s: Can't find the property of \"ranges\"\n", __func__);+ of_node_put(np);+ ret = -EINVAL;+ goto ccsr_err;+ }
Use get_immrbase(), or better use specific nodes in the device tree.
OK, so this file isn't even specific to T1040 -- it's specific to our
reference boards?
quoted hunk
+static void fsl_dp_ddr_save(void *ccsr_base)+{+ u32 ddr_buff_addr;++ /*+ * DDR training initialization will break 128 bytes at the beginning+ * of DDR, therefore, save them so that the bootloader will restore+ * them. Assume that DDR is mapped to the address space started with+ * CONFIG_PAGE_OFFSET.+ */+ memcpy(ddr_buff, (void *)CONFIG_PAGE_OFFSET, DDR_BUF_SIZE);++ /* assume ddr_buff is in the physical address space of 4GB */+ ddr_buff_addr = (u32)(__pa(ddr_buff) & 0xffffffff);
That assumption may break with a relocatable kernel.
@@ -20,6 +20,8 @@#define FSL_SLEEP 0x1#define FSL_DEEP_SLEEP 0x2+int(*fsl_enter_deepsleep)(void);+/* specify the sleep state of the present platform */intsleep_pm_state;/* supported sleep modes by the present platform */
@@ -28,6 +30,7 @@ static unsigned int sleep_modes;staticintqoriq_suspend_enter(suspend_state_tstate){intret=0;+intcpu;switch(state){casePM_SUSPEND_STANDBY:
@@ -39,6 +42,17 @@ static int qoriq_suspend_enter(suspend_state_t state)break;+casePM_SUSPEND_MEM:++cpu=smp_processor_id();+qoriq_pm_ops->irq_mask(cpu);++ret=fsl_enter_deepsleep();++qoriq_pm_ops->irq_unmask(cpu);++break;+default:ret=-EINVAL;
@@ -52,12 +66,30 @@ static int qoriq_suspend_valid(suspend_state_t state)if(state==PM_SUSPEND_STANDBY&&(sleep_modes&FSL_SLEEP))return1;+if(state==PM_SUSPEND_MEM&&(sleep_modes&FSL_DEEP_SLEEP))+return1;+return0;}+staticintqoriq_suspend_begin(suspend_state_tstate)+{+if(state==PM_SUSPEND_MEM)+returnfsl_dp_iomap();++return0;+}++staticvoidqoriq_suspend_end(void)+{+fsl_dp_iounmap();+}+staticconststructplatform_suspend_opsqoriq_suspend_ops={.valid=qoriq_suspend_valid,.enter=qoriq_suspend_enter,+.begin=qoriq_suspend_begin,+.end=qoriq_suspend_end,};staticint__initqoriq_suspend_init(void)
@@ -71,6 +103,12 @@ static int __init qoriq_suspend_init(void)if(np)sleep_pm_state=PLAT_PM_LPM20;+np=of_find_compatible_node(NULL,NULL,"fsl,t1040-rcpm");+if(np){+fsl_enter_deepsleep=fsl_enter_epu_deepsleep;+sleep_modes|=FSL_DEEP_SLEEP;+}+suspend_set_ops(&qoriq_suspend_ops);return0;
Again, this is not marked as a board-specific file. How do you expect
customers to adapt this mechanism to their boards?
quoted hunk
++ /* prefecth code to cache so that executing code after disable DDR */+1: lwz r3, 0(r8)+ addi r8, r8, 4+ cmpw r8, r9+ blt 1b+ msync
Instructions don't execute from dcache... I guess you're assuming it
will allocate in, and stay in, L2/CPC. It'd be safer to lock those
cache lines in icache, or copy the code to SRAM.
quoted hunk
+ FSL_DIS_ALL_IRQ++ /*+ * Place DDR controller in self refresh mode.+ * From here on, DDR can't be access any more.+ */+ lwz r10, 0(r13)+ oris r10, r10, CCSR_DDR_SDRAM_CFG_2_FRC_SR@h+ stw r10, 0(r13)++ /* can't call udelay() here, so use a macro to delay */+ FSLDELAY(50)
A timebase loop doesn't require accessing DDR.
You also probably want to do a "sync, readback, data dependency, isync"
sequence to make sure that the store has hit CCSR before you begin your
delay (or is a delay required at all if you do that?).
quoted hunk
+ /*+ * Enable deep sleep signals by write external CPLD/FPGA register.+ * The bootloader will disable them when wakeup from deep sleep.+ */+ lbz r10, 0(r29)+ PPC_LCMPI 0, r28, FPGA_FLAG+ beq 22f+ ori r10, r10, CPLD_MISCCSR_SLEEPEN+22:+ ori r10, r10, QIXIS_PWR_CTL2_PCTL+ stb r10, 0(r29)++ /*+ * Set GPIO1_29 to lock the signal MCKE down during deep sleep.+ * The bootloader will clear it when wakeup.+ */+ lwz r10, 0(r11)+ ori r10, r10, CCSR_GPIO1_GPDAT_29+ stw r10, 0(r11)++ FSLDELAY(10)++ /* Clear the QMan CITI Credits */+ lwz r10, 0(r16)+ oris r10, r10, CCSR_SCFG_QMCRDTRSTCR_CRDTRST@h+ stw r10, 0(r16)++ /* Enable all EPU Counters */+ li r10, 0+ oris r10, r10, DCSR_EPU_EPGCR_GCE@h+ stw r10, 0(r14)++ /* Enable SCU15 to trigger on RCPM Concentrator 0 */+ lwz r10, 0(r15)+ oris r10, r10, DCSR_EPU_EPECR15_IC0@h+ stw r10, 0(r15)++ /* put Core0 in PH15 mode, trigger EPU FSM */+ lwz r10, 0(r12)+ ori r10, r10, CCSR_RCPM_PCPH15SETR_CORE0+ stw r10, 0(r12)
Shouldn't there be a sync to ensure that the previous I/O happens before
the final store to enter PH15?
-Scott
RCPM is not board-specific. Why is this in board code?
Init the RCPM driver in the early stage before smp_init(). Because
the time base sync calls a callback function .freeze_time_base()
in the RCPM driver.
Will use early_initcall() instead.
quoted
+static void rcpm_v1_cpu_enter_state(int cpu, int state)+{+ unsigned int hw_cpu = get_hard_smp_processor_id(cpu);+ unsigned int mask = 1 << hw_cpu;++ switch (state) {+ case E500_PM_PH10:+ setbits32(&rcpm_v1_regs->cdozcr, mask);+ break;+ case E500_PM_PH15:+ setbits32(&rcpm_v1_regs->cnapcr, mask);+ break;+ default:+ pr_err("Unknown cpu PM state\n");+ break;+ }+}
Put __func__ in error messages -- and for "unknown value" type messages,
print the value.
OK.
quoted
+static int rcpm_v1_plat_enter_state(int state)+{+ u32 *pmcsr_reg = &rcpm_v1_regs->powmgtcsr;+ int ret = 0;+ int result;++ switch (state) {+ case PLAT_PM_SLEEP:+ setbits32(pmcsr_reg, RCPM_POWMGTCSR_SLP);++ /* At this point, the device is in sleep mode. */++ /* Upon resume, wait for RCPM_POWMGTCSR_SLP bit to be clear. */+ result = spin_event_timeout(+ !(in_be32(pmcsr_reg) & RCPM_POWMGTCSR_SLP), 10000, 10);+ if (!result) {+ pr_err("%s: timeout waiting for SLP bit to be cleared\n",+ __func__);
Why are you indenting continuation lines with only two spaces (and yet
still not aligning with anything)?
Will align with the previous parenthesis.
quoted
+ ret = -ETIMEDOUT;+ }+ break;+ default:+ pr_err("Unsupported platform PM state\n");+ ret = -EINVAL;+ }++ return ret;+}++static void rcpm_v1_freeze_time_base(int freeze)+{+ u32 *tben_reg = &rcpm_v1_regs->ctbenr;+ static u32 mask;++ if (freeze) {+ mask = in_be32(tben_reg);+ clrbits32(tben_reg, mask);+ } else {+ setbits32(tben_reg, mask);+ }++ /* read back to push the previous write */+ in_be32(tben_reg);+}++static void rcpm_v2_freeze_time_base(int freeze)+{+ u32 *tben_reg = &rcpm_v2_regs->pctbenr;+ static u32 mask;++ if (freeze) {+ mask = in_be32(tben_reg);+ clrbits32(tben_reg, mask);+ } else {+ setbits32(tben_reg, mask);+ }++ /* read back to push the previous write */+ in_be32(tben_reg);+}
It looks like the only difference between these two functions is how you
calculate tben_reg -- factor the rest out into a single function.
The RCPM is not a single function IP block, instead it is a collection
of device run control and power management. It would be called by other
drivers and functions. For example, the callback .freeze_time_base()
need to be called at early stage of kernel init. Therefore, it would be
better to init it at early stage.
-Chenhui
@@ -405,8 +405,12 @@ void generic_cpu_die(unsigned int cpu)for(i=0;i<100;i++){smp_rmb();-if(per_cpu(cpu_state,cpu)==CPU_DEAD)+if(per_cpu(cpu_state,cpu)==CPU_DEAD){+#ifdef CONFIG_PPC64+paca[cpu].cpu_start=0;+#endif
Why wasn't this needed by previous ppc64 machines?
If not clear, cpu can't start in the case of cpu hotpolug.
The function pseries_cpu_die() in arch/powerpc/platforms/pseries/hotplug-cpu.c
also clears the flag.
@@ -46,6 +47,17 @@ static u64 timebase;staticinttb_req;staticinttb_valid;+#ifdef CONFIG_PPC_E500MC+/* specify the cpu PM state when cpu dies, PH15/NAP is the default */+intqoriq_cpu_die_state=E500_PM_PH15;+#endif
static? Is there any way to modify this other than modifying source
code?
BTW, QorIQ doesn't imply an e500mc derivative.
Will support e500, but for now these code support e500mc derivative in
advance.
Supposed qoriq_cpu_die_state can be changed by platform init code
if the default value is not proper for a specific platform.
Why this instead of one call to hard_irq_disable() (no leading
underscores)?
-Scott
hard_irq_disable() will clear soft_enabled again. local_irq_disable()
has cleared it.
Will use hard_irq_disable() to replace these lines.
local_irq_disable();
#ifdef CONFIG_PPC64
__hard_irq_disable();
#endif
-Chenhui
From: Kevin Hao <hidden> Date: 2014-03-12 05:58:11
On Tue, Mar 11, 2014 at 08:10:24PM -0500, Scott Wood wrote:
quoted
+ FSL_DIS_ALL_IRQ++ /*+ * Place DDR controller in self refresh mode.+ * From here on, DDR can't be access any more.+ */+ lwz r10, 0(r13)+ oris r10, r10, CCSR_DDR_SDRAM_CFG_2_FRC_SR@h+ stw r10, 0(r13)++ /* can't call udelay() here, so use a macro to delay */+ FSLDELAY(50)
A timebase loop doesn't require accessing DDR.
You also probably want to do a "sync, readback, data dependency, isync"
sequence to make sure that the store has hit CCSR before you begin your
delay (or is a delay required at all if you do that?).
Shouldn't we use "readback, sync" here? The following is quoted form t4240RM:
To guarantee that the results of any sequence of writes to configuration
registers are in effect, the final configuration register write should be
immediately followed by a read of the same register, and that should be
followed by a SYNC instruction. Then accesses can safely be made to memory
regions affected by the configuration register write.
quoted
++ /* Enable SCU15 to trigger on RCPM Concentrator 0 */+ lwz r10, 0(r15)+ oris r10, r10, DCSR_EPU_EPECR15_IC0@h+ stw r10, 0(r15)++ /* put Core0 in PH15 mode, trigger EPU FSM */+ lwz r10, 0(r12)+ ori r10, r10, CCSR_RCPM_PCPH15SETR_CORE0+ stw r10, 0(r12)
Shouldn't there be a sync to ensure that the previous I/O happens before
the final store to enter PH15?
Do we really need a sync here? According to the PowerISA, the above stores
should be performed in program order.
If two Store instructions or two Load instructions
specify storage locations that are both Caching
Inhibited and Guarded, the corresponding storage
accesses are performed in program order with
respect to any processor or mechanism.
Thanks,
Kevin
On Tue, Mar 11, 2014 at 06:51:20PM -0500, Scott Wood wrote:
On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
quoted
In 64-bit mode, kernel just clears the irq soft-enable flag
in struct paca_struct to disable external irqs. But, in
the case of suspend, irqs should be disabled by hardware.
Therefore, hook a function to ppc_md.suspend_disable_irqs
to really disable irqs.
Signed-off-by: Chenhui Zhao <redacted>
---
arch/powerpc/platforms/85xx/corenet_generic.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
Why the underscore version? Don't you want PACA_IRQ_HARD_DIS to be set?
If hard disabling is appropriate here, shouldn't we do it in
generic_suspend_disable_irqs()?
Are there any existing platforms that supply a
ppc_md.suspend_disable_irqs()? I don't see any when grepping.
-Scott
Will use hard_irq_disable().
I think this is a general problem for powerpc.
Should clear MSR_EE before suspend. I agree to put it
in generic_suspend_disable_irqs().
-Chenhui
On Tue, Mar 11, 2014 at 07:00:27PM -0500, Scott Wood wrote:
On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
quoted
In sleep mode, the clocks of e500 cores and unused IP blocks is
turned off. The IP blocks which are allowed to wake up the processor
are still running.
The sleep mode is equal to the Standby state in Linux. Use the
command to enter sleep mode:
echo standby > /sys/power/state
Signed-off-by: Chenhui Zhao <redacted>
---
arch/powerpc/Kconfig | 4 +-
arch/powerpc/platforms/85xx/Makefile | 3 +
arch/powerpc/platforms/85xx/qoriq_pm.c | 78 ++++++++++++++++++++++++++++++++
3 files changed, 83 insertions(+), 2 deletions(-)
create mode 100644 arch/powerpc/platforms/85xx/qoriq_pm.c
@@ -2,6 +2,9 @@# Makefile for the PowerPC 85xx linux kernel.#obj-$(CONFIG_SMP)+=smp.o+ifeq ($(CONFIG_FSL_CORENET_RCPM), y)+obj-$(CONFIG_SUSPEND)+=qoriq_pm.o+endif
There should probably be a kconfig symbol for this.
Will be used in the last patch.
[PATCH 9/9] powerpc/pm: support deep sleep feature on T1040
quoted
++/* specify the sleep state of the present platform */+int sleep_pm_state;+/* supported sleep modes by the present platform */+static unsigned int sleep_modes;
Why is one signed and the other unsigned?
Undesigned. Will change them all to unsigned.
quoted
++static int qoriq_suspend_enter(suspend_state_t state)+{+ int ret = 0;++ switch (state) {+ case PM_SUSPEND_STANDBY:++ if (cur_cpu_spec->cpu_flush_caches)+ cur_cpu_spec->cpu_flush_caches();++ ret = qoriq_pm_ops->plat_enter_state(sleep_pm_state);++ break;++ default:+ ret = -EINVAL;++ }++ return ret;+}++static int qoriq_suspend_valid(suspend_state_t state)+{+ if (state == PM_SUSPEND_STANDBY && (sleep_modes & FSL_SLEEP))+ return 1;++ return 0;+}++static const struct platform_suspend_ops qoriq_suspend_ops = {+ .valid = qoriq_suspend_valid,+ .enter = qoriq_suspend_enter,+};++static int __init qoriq_suspend_init(void)+{+ struct device_node *np;++ sleep_modes = FSL_SLEEP;+ sleep_pm_state = PLAT_PM_SLEEP;++ np = of_find_compatible_node(NULL, NULL, "fsl,qoriq-rcpm-2.0");+ if (np)+ sleep_pm_state = PLAT_PM_LPM20;++ suspend_set_ops(&qoriq_suspend_ops);++ return 0;+}+arch_initcall(qoriq_suspend_init);
Why is this not a platform driver? If fsl_pmc can do it...
-Scott
It can be, but what advantage of being a platform driver.
-Chenhui
On Tue, Mar 11, 2014 at 07:08:43PM -0500, Scott Wood wrote:
On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
quoted
From: Hongbo Zhang <redacted>
In the last stage of deep sleep, software will trigger a Finite
State Machine (FSM) to control the hardware precedure, such as
board isolation, killing PLLs, removing power, and so on.
When the system is waked up by an interrupt, the FSM controls the
hardware to complete the early resume precedure.
This patch configure the EPU FSM preparing for deep sleep.
Signed-off-by: Hongbo Zhang <redacted>
Signed-off-by: Chenhui Zhao <redacted>
Couldn't this be part of qoriq_pm.c?
Put the code in drivers/platform/fsl/ so that LS1 can share these code.
@@ -0,0 +1,10 @@+#+# Freescale Specific Power Management Drivers+#++configFSL_SLEEP_FSM+bool+help+ThisdriverconfiguresahardwareFSM(FiniteStateMachine)fordeepsleep.+TheFSMisusedtofinishclean-upsatthelaststageofsystementeringdeep+sleep,andalsowakesupsystemwhenawakeupeventhappens.
On Tue, Mar 11, 2014 at 07:45:14PM -0500, Scott Wood wrote:
On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
quoted
From: Wang Dongsheng <redacted>
Add booke_cpu_state_save() and booke_cpu_state_restore() functions which can be
used to save/restore CPU's registers in the case of deep sleep and hibernation.
Supported processors: E6500, E5500, E500MC, E500v2 and E500v1.
Signed-off-by: Wang Dongsheng <redacted>
Signed-off-by: Chenhui Zhao <redacted>
---
arch/powerpc/include/asm/booke_save_regs.h | 96 ++++++++
arch/powerpc/kernel/Makefile | 1 +
arch/powerpc/kernel/booke_save_regs.S | 361 ++++++++++++++++++++++++++++
3 files changed, 458 insertions(+), 0 deletions(-)
create mode 100644 arch/powerpc/include/asm/booke_save_regs.h
create mode 100644 arch/powerpc/kernel/booke_save_regs.S
Filename says booke, comment says e500.
Filename and comment also fail to point out that this is specifically
for standby/suspend, not for hibernate which is implemented in
swsusp_booke.S/swsusp_asm64.S.
Sorry for inconsistency. Will changes e500 to booke.
Hibernation and suspend can share the code.
quoted
+ *+ * Author: Wang Dongsheng <dongsheng.wang@freescale.com>+ *+ * Copyright 2014 Freescale Semiconductor Inc.+ *+ * This program is free software; you can redistribute it and/or modify+ * it under the terms of the GNU General Public License version 2 as+ * published by the Free Software Foundation.+ */++#ifndef __ASM_FSL_SLEEP_H+#define __ASM_FSL_SLEEP_H++/*+ * 8 bytes for each register, which is compatible with+ * both 32-bit and 64-bit registers+ *+ * Acronyms:+ * dw(data width) 0x08+ *+ * Map:+ * General-Purpose Registers+ * GPR1(sp) 0+ * GPR2 0x8 (dw * 1)+ * GPR13 - GPR31 0x10 ~ 0xa0 (dw * 2 ~ dw * 20)
Putting these values in a comment separate from the code that defines it
sounds like a good way to get a mismatch between the two.
Call enable_kernel_fp() or similar, rather than saving FP regs here.
Likewise with altivec. And why starting at FPR14? Volatile versus
nonvolatile is irrelevant because Linux doesn't participate in the FP
ABI. Everything is non-volatile *if* we have a user FP context
resident, and everything is volatile otherwise.
PID1/PID2 are e500v1/v2 only -- and Linux doesn't use them outside of
KVM (and you're not in KVM when you're running this code).
Are we ever going to have a non-zero PID at this point?
I incline to the view that saving all registers regardless of used or
unused. The good point is that it can be compliant to the future
changes of the usage of registers.
What do you think?
IAC3-4 are not implemented on e500.
Do we really need to save the debug registers? We're not going to be in
a debugged process when we do suspend. If the concern is kgdb, it
probably needs to be told to get out of the way during suspend for other
reasons.
I think in the ideal case the suspend would not break any context. We
should try to save/restore all cpu state. Of cause, trade-off is
unavoidable in practice.
These are very vague names to be putting in a header file.
How about BOOKE_xx_OFFSET?
quoted
+/*+ * hibernation and deepsleep save/restore different number of registers,+ * use these flags to indicate.+ */+#define HIBERNATION_FLAG 1+#define DEEPSLEEP_FLAG 2
Again, namespacing -- but why is hibernation using this at all? What's
wrong with the existing hibernation support?
How about BOOKE_HIBERNATION_FLAG?
Just wish to share code between hibernation and suspend.
On Tue, Mar 11, 2014 at 08:10:24PM -0500, Scott Wood wrote:
On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
quoted
From: Zhao Chenhui <redacted>
T1040 supports deep sleep feature, which can switch off most parts of
the SoC when it is in deep sleep mode. This way, it becomes more
energy-efficient.
The DDR controller will also be powered off in deep sleep. Therefore,
the last stage (the latter part of fsl_dp_enter_low) will run without DDR
access. This piece of code and related TLBs will be prefetched.
Due to the different initialization code between 32-bit and 64-bit, they
have seperate resume entry and precedure.
The feature supports 32-bit and 64-bit kernel mode.
Signed-off-by: Zhao Chenhui <redacted>
---
arch/powerpc/include/asm/booke_save_regs.h | 3 +
arch/powerpc/kernel/cpu_setup_fsl_booke.S | 17 ++
arch/powerpc/kernel/head_fsl_booke.S | 30 +++
arch/powerpc/platforms/85xx/Makefile | 2 +-
arch/powerpc/platforms/85xx/deepsleep.c | 201 +++++++++++++++++++
arch/powerpc/platforms/85xx/qoriq_pm.c | 38 ++++
arch/powerpc/platforms/85xx/sleep.S | 295 ++++++++++++++++++++++++++++
arch/powerpc/sysdev/fsl_soc.h | 7 +
8 files changed, 592 insertions(+), 1 deletions(-)
create mode 100644 arch/powerpc/platforms/85xx/deepsleep.c
create mode 100644 arch/powerpc/platforms/85xx/sleep.S
We have two kind of boards, QDS and RDB. They have different register
map. Use the flag to indicate the current board is using which kind
of register map.
quoted
#ifndef __ASSEMBLY__
extern void booke_cpu_state_save(void *buf, int type);
extern void *booke_cpu_state_restore(void *buf, int type);
Why do you come in via the normal kernel entry, versus specifying a
direct entry point for deep sleep resume? How does U-Boot even know
what the normal entry is when resuming?
I wish to return to a specified point (like 64-bit mode), but the code in
fsl_booke_entry_mapping.S only can run in the first page. Because it
only setups a temp mapping of 4KB.
Be careful of the "beq set_ivor" in the CONFIG_RELOCATABLE section
above. Also you probably don't want the relocation code to run again
when resuming.
When resuming, will run from the point __early_start. Don't run the code
in the CONFIG_RELOCATABLE section.
quoted
+#if defined(CONFIG_SUSPEND) && defined(CONFIG_FSL_CORENET_RCPM)+_ENTRY(__entry_deep_sleep)+/*+ * Bootloader will jump to here when resuming from deep sleep.+ * After executing the init code in fsl_booke_entry_mapping.S,+ * will jump to the real resume entry.+ */+ li r8, 1+ bl 12f+12: mflr r9+ addi r9, r9, (deep_sleep_flag - 12b)+ stw r8, 0(r9)+ b __early_start+deep_sleep_flag:+ .long 0+#endif
It's a bit ambiguous to say "entry_deep_sleep" when it's resuming rather
than entering...
How about __fsl_entry_resume?
So you do have a special entry point. Why do you go to __early_start
only to quickly test a flag and branch away?
AFAICT this supports deep sleep on T1040, not on all 85xx that has it.
Yes, for now T1040 and T1042.
quoted
+ *+ * Copyright 2014 Freescale Semiconductor Inc.+ *+ * Author: Chenhui Zhao <chenhui.zhao@freescale.com>+ *+ * This program is free software; you can redistribute it and/or modify it+ * under the terms of the GNU General Public License as published by the+ * Free Software Foundation; either version 2 of the License, or (at your+ * option) any later version.+ */++#include <linux/kernel.h>+#include <linux/of_platform.h>+#include <linux/of_address.h>+#include <asm/machdep.h>+#include <sysdev/fsl_soc.h>+#include <asm/booke_save_regs.h>++#define SIZE_1MB 0x100000+#define SIZE_2MB 0x200000++#define CCSR_SCFG_DPSLPCR 0xfc000+#define CCSR_SCFG_DPSLPCR_WDRR_EN 0x1+#define CCSR_SCFG_SPARECR2 0xfc504+#define CCSR_SCFG_SPARECR3 0xfc508++#define CCSR_GPIO1_GPDIR 0x130000+#define CCSR_GPIO1_GPODR 0x130004+#define CCSR_GPIO1_GPDAT 0x130008+#define CCSR_GPIO1_GPDIR_29 0x4++/* 128 bytes buffer for restoring data broke by DDR training initialization */+#define DDR_BUF_SIZE 128+static u8 ddr_buff[DDR_BUF_SIZE] __aligned(64);++static void *dcsr_base, *ccsr_base, *pld_base;+static int pld_flag;++int fsl_dp_iomap(void)+{+ struct device_node *np;+ const u32 *prop;+ int ret = 0;+ u64 ccsr_phy_addr, dcsr_phy_addr;++ np = of_find_node_by_type(NULL, "soc");+ if (!np) {+ pr_err("%s: Can't find the node of \"soc\"\n", __func__);+ ret = -EINVAL;+ goto ccsr_err;+ }+ prop = of_get_property(np, "ranges", NULL);+ if (!prop) {+ pr_err("%s: Can't find the property of \"ranges\"\n", __func__);+ of_node_put(np);+ ret = -EINVAL;+ goto ccsr_err;+ }
Use get_immrbase(), or better use specific nodes in the device tree.
OK, so this file isn't even specific to T1040 -- it's specific to our
reference boards?
Yes. There are some FPGA/CPLD setting which needed by deep sleep.
quoted
+static void fsl_dp_ddr_save(void *ccsr_base)+{+ u32 ddr_buff_addr;++ /*+ * DDR training initialization will break 128 bytes at the beginning+ * of DDR, therefore, save them so that the bootloader will restore+ * them. Assume that DDR is mapped to the address space started with+ * CONFIG_PAGE_OFFSET.+ */+ memcpy(ddr_buff, (void *)CONFIG_PAGE_OFFSET, DDR_BUF_SIZE);++ /* assume ddr_buff is in the physical address space of 4GB */+ ddr_buff_addr = (u32)(__pa(ddr_buff) & 0xffffffff);
That assumption may break with a relocatable kernel.
@@ -20,6 +20,8 @@#define FSL_SLEEP 0x1#define FSL_DEEP_SLEEP 0x2+int(*fsl_enter_deepsleep)(void);+/* specify the sleep state of the present platform */intsleep_pm_state;/* supported sleep modes by the present platform */
@@ -28,6 +30,7 @@ static unsigned int sleep_modes;staticintqoriq_suspend_enter(suspend_state_tstate){intret=0;+intcpu;switch(state){casePM_SUSPEND_STANDBY:
@@ -39,6 +42,17 @@ static int qoriq_suspend_enter(suspend_state_t state)break;+casePM_SUSPEND_MEM:++cpu=smp_processor_id();+qoriq_pm_ops->irq_mask(cpu);++ret=fsl_enter_deepsleep();++qoriq_pm_ops->irq_unmask(cpu);++break;+default:ret=-EINVAL;
@@ -52,12 +66,30 @@ static int qoriq_suspend_valid(suspend_state_t state)if(state==PM_SUSPEND_STANDBY&&(sleep_modes&FSL_SLEEP))return1;+if(state==PM_SUSPEND_MEM&&(sleep_modes&FSL_DEEP_SLEEP))+return1;+return0;}+staticintqoriq_suspend_begin(suspend_state_tstate)+{+if(state==PM_SUSPEND_MEM)+returnfsl_dp_iomap();++return0;+}++staticvoidqoriq_suspend_end(void)+{+fsl_dp_iounmap();+}+staticconststructplatform_suspend_opsqoriq_suspend_ops={.valid=qoriq_suspend_valid,.enter=qoriq_suspend_enter,+.begin=qoriq_suspend_begin,+.end=qoriq_suspend_end,};staticint__initqoriq_suspend_init(void)
@@ -71,6 +103,12 @@ static int __init qoriq_suspend_init(void)if(np)sleep_pm_state=PLAT_PM_LPM20;+np=of_find_compatible_node(NULL,NULL,"fsl,t1040-rcpm");+if(np){+fsl_enter_deepsleep=fsl_enter_epu_deepsleep;+sleep_modes|=FSL_DEEP_SLEEP;+}+suspend_set_ops(&qoriq_suspend_ops);return0;
Again, this is not marked as a board-specific file. How do you expect
customers to adapt this mechanism to their boards?
Will add comment.
quoted
++ /* prefecth code to cache so that executing code after disable DDR */+1: lwz r3, 0(r8)+ addi r8, r8, 4+ cmpw r8, r9+ blt 1b+ msync
Instructions don't execute from dcache... I guess you're assuming it
will allocate in, and stay in, L2/CPC. It'd be safer to lock those
cache lines in icache, or copy the code to SRAM.
Yes, they should be in L2/CPC. Will try to lock them in icache.
quoted
+ FSL_DIS_ALL_IRQ++ /*+ * Place DDR controller in self refresh mode.+ * From here on, DDR can't be access any more.+ */+ lwz r10, 0(r13)+ oris r10, r10, CCSR_DDR_SDRAM_CFG_2_FRC_SR@h+ stw r10, 0(r13)++ /* can't call udelay() here, so use a macro to delay */+ FSLDELAY(50)
A timebase loop doesn't require accessing DDR.
Don't wish to jump at this time. Maybe cause problems.
You also probably want to do a "sync, readback, data dependency, isync"
sequence to make sure that the store has hit CCSR before you begin your
delay (or is a delay required at all if you do that?).
Yes. It is safer with a sync sequence.
The DDR controller need some time to signal the external DDR modules to
enter self refresh mode.
-Chenhui
From: Scott Wood <hidden> Date: 2014-03-12 17:45:58
On Wed, 2014-03-12 at 13:57 +0800, Kevin Hao wrote:
On Tue, Mar 11, 2014 at 08:10:24PM -0500, Scott Wood wrote:
quoted
quoted
+ FSL_DIS_ALL_IRQ++ /*+ * Place DDR controller in self refresh mode.+ * From here on, DDR can't be access any more.+ */+ lwz r10, 0(r13)+ oris r10, r10, CCSR_DDR_SDRAM_CFG_2_FRC_SR@h+ stw r10, 0(r13)++ /* can't call udelay() here, so use a macro to delay */+ FSLDELAY(50)
A timebase loop doesn't require accessing DDR.
You also probably want to do a "sync, readback, data dependency, isync"
sequence to make sure that the store has hit CCSR before you begin your
delay (or is a delay required at all if you do that?).
Shouldn't we use "readback, sync" here? The following is quoted form t4240RM:
To guarantee that the results of any sequence of writes to configuration
registers are in effect, the final configuration register write should be
immediately followed by a read of the same register, and that should be
followed by a SYNC instruction. Then accesses can safely be made to memory
regions affected by the configuration register write.
I agree that the sync before the readback is probably not necessary,
since transactions to the same address should already be ordered.
A sync after the readback helps if you're trying to order the readback
with subsequent memory accesses, though in that case wouldn't a sync
alone (no readback) be adequate? Though maybe not always -- see the
comment near the end of fsl_elbc_write_buf() in
drivers/mtd/nand_fsl_elbc.c. I guess the readback does more than just
make sure the device has seen the write, ensuring that the device has
finished the transaction to the point of acting on another one.
The data dependency plus isync sequence, which is done by the normal I/O
accessors used from C code, orders the readback versus all future
instructions (not just I/O). The delay loop is not I/O.
quoted
quoted
+ /* Enable SCU15 to trigger on RCPM Concentrator 0 */+ lwz r10, 0(r15)+ oris r10, r10, DCSR_EPU_EPECR15_IC0@h+ stw r10, 0(r15)++ /* put Core0 in PH15 mode, trigger EPU FSM */+ lwz r10, 0(r12)+ ori r10, r10, CCSR_RCPM_PCPH15SETR_CORE0+ stw r10, 0(r12)
Shouldn't there be a sync to ensure that the previous I/O happens before
the final store to enter PH15?
Do we really need a sync here? According to the PowerISA, the above stores
should be performed in program order.
If two Store instructions or two Load instructions
specify storage locations that are both Caching
Inhibited and Guarded, the corresponding storage
accesses are performed in program order with
respect to any processor or mechanism.
From: Kevin Hao <hidden> Date: 2014-03-13 07:46:25
On Wed, Mar 12, 2014 at 12:43:05PM -0500, Scott Wood wrote:
quoted
Shouldn't we use "readback, sync" here? The following is quoted form t4240RM:
To guarantee that the results of any sequence of writes to configuration
registers are in effect, the final configuration register write should be
immediately followed by a read of the same register, and that should be
followed by a SYNC instruction. Then accesses can safely be made to memory
regions affected by the configuration register write.
I agree that the sync before the readback is probably not necessary,
since transactions to the same address should already be ordered.
A sync after the readback helps if you're trying to order the readback
with subsequent memory accesses, though in that case wouldn't a sync
alone (no readback) be adequate?
No, we don't just want to order the subsequent memory access here.
The 'write, readback, sync' is the required sequence if we want to make
sure that the writing to CCSR register does really take effect.
Though maybe not always -- see the
comment near the end of fsl_elbc_write_buf() in
drivers/mtd/nand_fsl_elbc.c. I guess the readback does more than just
make sure the device has seen the write, ensuring that the device has
finished the transaction to the point of acting on another one.
Agree.
The data dependency plus isync sequence, which is done by the normal I/O
accessors used from C code, orders the readback versus all future
instructions (not just I/O). The delay loop is not I/O.
According to the PowerISA, the sequence 'load, date dependency, isync' only
order the load accesses. So if we want to order all the storage access as well
as execution synchronization, we should choose sync here.
Thanks,
Kevin
From: Scott Wood <hidden> Date: 2014-03-14 22:26:39
On Thu, 2014-03-13 at 15:46 +0800, Kevin Hao wrote:
On Wed, Mar 12, 2014 at 12:43:05PM -0500, Scott Wood wrote:
quoted
quoted
Shouldn't we use "readback, sync" here? The following is quoted form t4240RM:
To guarantee that the results of any sequence of writes to configuration
registers are in effect, the final configuration register write should be
immediately followed by a read of the same register, and that should be
followed by a SYNC instruction. Then accesses can safely be made to memory
regions affected by the configuration register write.
I agree that the sync before the readback is probably not necessary,
since transactions to the same address should already be ordered.
A sync after the readback helps if you're trying to order the readback
with subsequent memory accesses, though in that case wouldn't a sync
alone (no readback) be adequate?
No, we don't just want to order the subsequent memory access here.
The 'write, readback, sync' is the required sequence if we want to make
sure that the writing to CCSR register does really take effect.
quoted
Though maybe not always -- see the
comment near the end of fsl_elbc_write_buf() in
drivers/mtd/nand_fsl_elbc.c. I guess the readback does more than just
make sure the device has seen the write, ensuring that the device has
finished the transaction to the point of acting on another one.
Agree.
quoted
The data dependency plus isync sequence, which is done by the normal I/O
accessors used from C code, orders the readback versus all future
instructions (not just I/O). The delay loop is not I/O.
According to the PowerISA, the sequence 'load, date dependency, isync' only
order the load accesses.
The point is to order the delay loop after the load, not to order
storage versus storage.
This is a sequence we're already using on all of our I/O loads
(excluding accesses like in this patch that don't use the standard
accessors). I'm confident that it works even if it's not
architecturally guaranteed. I'm not sure that there exists a clear
architectural way of synchronizing non-storage instructions relative to
storage instructions.
Given that isync is documented as preventing any execution of
instructions after the isync until all previous instructions complete,
it doesn't seem to make sense for the architecture to explicitly talk
about loads (as opposed to any other instruction) following a load,
dependent conditional branch, isync sequence.
So if we want to order all the storage access as well
as execution synchronization, we should choose sync here.
Do we need execution synchronization or context synchronization?
The t4240 RM section that talks about a readback and a sync is in the
context of subsequent memory operations ("Then accesses can safely be
made to memory regions affected..."), not arbitrary instructions. There
are also a couple other places in the RM where isync is recommended
instead (when setting LAWs or CCSRBAR), even though those also only
involve memory accesses.
In any case, this is not performance critical and thus it's better to
oversynchronize than undersynchronize.
-Scott
The RCPM is not a single function IP block, instead it is a collection
of device run control and power management. It would be called by other
drivers and functions. For example, the callback .freeze_time_base()
need to be called at early stage of kernel init. Therefore, it would be
better to init it at early stage.
OK, but consider using of_find_matching_node_and_match().
-Scott
From: Scott Wood <hidden> Date: 2014-03-14 22:41:52
On Wed, 2014-03-12 at 15:46 +0800, Chenhui Zhao wrote:
On Tue, Mar 11, 2014 at 06:51:20PM -0500, Scott Wood wrote:
quoted
On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
quoted
In 64-bit mode, kernel just clears the irq soft-enable flag
in struct paca_struct to disable external irqs. But, in
the case of suspend, irqs should be disabled by hardware.
Therefore, hook a function to ppc_md.suspend_disable_irqs
to really disable irqs.
Signed-off-by: Chenhui Zhao <redacted>
---
arch/powerpc/platforms/85xx/corenet_generic.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
Why the underscore version? Don't you want PACA_IRQ_HARD_DIS to be set?
If hard disabling is appropriate here, shouldn't we do it in
generic_suspend_disable_irqs()?
Are there any existing platforms that supply a
ppc_md.suspend_disable_irqs()? I don't see any when grepping.
-Scott
Will use hard_irq_disable().
I think this is a general problem for powerpc.
Should clear MSR_EE before suspend. I agree to put it
in generic_suspend_disable_irqs().
BTW, make sure you test this patchset with CONFIG_DEBUG_PREEMPT and
similar debugging options to help ensure that the soft IRQ state is
being tracked properly.
-Scott
From: Scott Wood <hidden> Date: 2014-03-14 22:46:25
On Wed, 2014-03-12 at 16:08 +0800, Chenhui Zhao wrote:
On Tue, Mar 11, 2014 at 07:00:27PM -0500, Scott Wood wrote:
quoted
On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
quoted
In sleep mode, the clocks of e500 cores and unused IP blocks is
turned off. The IP blocks which are allowed to wake up the processor
are still running.
The sleep mode is equal to the Standby state in Linux. Use the
command to enter sleep mode:
echo standby > /sys/power/state
Signed-off-by: Chenhui Zhao <redacted>
---
arch/powerpc/Kconfig | 4 +-
arch/powerpc/platforms/85xx/Makefile | 3 +
arch/powerpc/platforms/85xx/qoriq_pm.c | 78 ++++++++++++++++++++++++++++++++
3 files changed, 83 insertions(+), 2 deletions(-)
create mode 100644 arch/powerpc/platforms/85xx/qoriq_pm.c
@@ -2,6 +2,9 @@# Makefile for the PowerPC 85xx linux kernel.#obj-$(CONFIG_SMP)+=smp.o+ifeq ($(CONFIG_FSL_CORENET_RCPM), y)+obj-$(CONFIG_SUSPEND)+=qoriq_pm.o+endif
There should probably be a kconfig symbol for this.
Why is this not a platform driver? If fsl_pmc can do it...
-Scott
It can be, but what advantage of being a platform driver.
If nothing else, compliance with the standard way of doing things. Why
not make it a platform driver? You'd be able to use dev_err, have a
place in sysfs if attributes are needed in the future, etc.
A better answer might be that there are multiple not-very-related files
driving different portions of RCPM.
-Scott
From: Scott Wood <hidden> Date: 2014-03-14 22:51:18
On Wed, 2014-03-12 at 16:34 +0800, Chenhui Zhao wrote:
On Tue, Mar 11, 2014 at 07:08:43PM -0500, Scott Wood wrote:
quoted
On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
quoted
From: Hongbo Zhang <redacted>
In the last stage of deep sleep, software will trigger a Finite
State Machine (FSM) to control the hardware precedure, such as
board isolation, killing PLLs, removing power, and so on.
When the system is waked up by an interrupt, the FSM controls the
hardware to complete the early resume precedure.
This patch configure the EPU FSM preparing for deep sleep.
Signed-off-by: Hongbo Zhang <redacted>
Signed-off-by: Chenhui Zhao <redacted>
Couldn't this be part of qoriq_pm.c?
Put the code in drivers/platform/fsl/ so that LS1 can share these code.
How can LS1 share it if it's got hardcoded T1040 values?
Where do these magic numbers come from? Which chips are they valid for?
They are for T1040. Can be found in the RCPM chapter of T1040RM.
Then put in a comment to that effect, including what part of the RCPM
chapter.
How do you plan to handle the addition of another SoC with different
values?
-Scott
From: Scott Wood <hidden> Date: 2014-03-14 23:02:10
On Wed, 2014-03-12 at 17:42 +0800, Chenhui Zhao wrote:
On Tue, Mar 11, 2014 at 07:45:14PM -0500, Scott Wood wrote:
quoted
On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
quoted
From: Wang Dongsheng <redacted>
Add booke_cpu_state_save() and booke_cpu_state_restore() functions which can be
used to save/restore CPU's registers in the case of deep sleep and hibernation.
Supported processors: E6500, E5500, E500MC, E500v2 and E500v1.
Signed-off-by: Wang Dongsheng <redacted>
Signed-off-by: Chenhui Zhao <redacted>
---
arch/powerpc/include/asm/booke_save_regs.h | 96 ++++++++
arch/powerpc/kernel/Makefile | 1 +
arch/powerpc/kernel/booke_save_regs.S | 361 ++++++++++++++++++++++++++++
3 files changed, 458 insertions(+), 0 deletions(-)
create mode 100644 arch/powerpc/include/asm/booke_save_regs.h
create mode 100644 arch/powerpc/kernel/booke_save_regs.S
Filename says booke, comment says e500.
Filename and comment also fail to point out that this is specifically
for standby/suspend, not for hibernate which is implemented in
swsusp_booke.S/swsusp_asm64.S.
Sorry for inconsistency. Will changes e500 to booke.
Hibernation and suspend can share the code.
Maybe they could, but AFAICT this patchset doesn't make that happen --
and I'm not convinced that the churn would be worthwhile. Note that
swsusp_asm64.S is not just for booke, so most of that file would not be
going away if you did make such a change.
I also don't like the way it looks like booke_save_regs.S is a booke
version of ppc_save_regs.S, even though they serve different purposes
and ppc_save_regs.S is still relevant to booke.
quoted
quoted
+ * Software-Use Registers+ * SPRG1 0x260 (dw * 76), 64-bit need to save.+ * SPRG3 0x268 (dw * 77), 32-bit need to save.
What about "CPU and NUMA node for VDSO getcpu" on 64-bit? Currently
SPRG3, but it will need to change for critical interrupt support.
PID1/PID2 are e500v1/v2 only -- and Linux doesn't use them outside of
KVM (and you're not in KVM when you're running this code).
Are we ever going to have a non-zero PID at this point?
I incline to the view that saving all registers regardless of used or
unused. The good point is that it can be compliant to the future
changes of the usage of registers.
What do you think?
I agree to a certain extent, but balance it with the complexity of
dealing with registers that don't exist on all booke chips. If they
don't really need to be saved, why go through the hassle of conditional
code?
IAC3-4 are not implemented on e500.
Do we really need to save the debug registers? We're not going to be in
a debugged process when we do suspend. If the concern is kgdb, it
probably needs to be told to get out of the way during suspend for other
reasons.
I think in the ideal case the suspend would not break any context. We
should try to save/restore all cpu state. Of cause, trade-off is
unavoidable in practice.
These are very vague names to be putting in a header file.
How about BOOKE_xx_OFFSET?
Better, but does it need to be in a public header file at all?
quoted
quoted
+/*+ * hibernation and deepsleep save/restore different number of registers,+ * use these flags to indicate.+ */+#define HIBERNATION_FLAG 1+#define DEEPSLEEP_FLAG 2
Again, namespacing -- but why is hibernation using this at all? What's
wrong with the existing hibernation support?
How about BOOKE_HIBERNATION_FLAG?
Just wish to share code between hibernation and suspend.
No need until and unless you actually implement the change for
hibernation to use this. As is, this is dead and untestable code.
quoted
quoted
+booke_cpu_append_save:+ mfspr r0, SPRN_PVR+ rlwinm r11, r0, 16, 16, 31++ lis r12, 0+ ori r12, r12, PVR_VER_E6500@l+ cmpw r11, r12+ beq e6500_append_save++ lis r12, 0+ ori r12, r12, PVR_VER_E5500@l+ cmpw r11, r12+ beq e5500_append_save++ lis r12, 0+ ori r12, r12, PVR_VER_E500MC@l+ cmpw r11, r12+ beq e500mc_append_save++ lis r12, 0+ ori r12, r12, PVR_VER_E500V2@l+ cmpw r11, r12+ beq e500v2_append_save++ lis r12, 0+ ori r12, r12, PVR_VER_E500V1@l+ cmpw r11, r12+ beq e500v1_append_save+ b 1f++e6500_append_save:+ do_e6500_sr_interrupt_regs(save)+ do_e6500_sr_debug_regs(save)+ b 1f++e5500_append_save:+ do_e5500_sr_interrupt_regs(save)+ b 1f++e500mc_append_save:+ do_e500mc_sr_interrupt_regs(save)+ b 1f++e500v2_append_save:+e500v1_append_save:+ do_e500_sr_interrupt_regs(save)+1:+ do_sr_interrupt_regs(save)+ do_sr_debug_regs(save)++ blr
What is meant by "append" here?
Means extra registers for specific e500 derivative core.
How is that related to skipping it in the hibernation case?
-Scott
From: Scott Wood <hidden> Date: 2014-03-14 23:18:37
On Wed, 2014-03-12 at 18:40 +0800, Chenhui Zhao wrote:
On Tue, Mar 11, 2014 at 08:10:24PM -0500, Scott Wood wrote:
quoted
On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
quoted
From: Zhao Chenhui <redacted>
T1040 supports deep sleep feature, which can switch off most parts of
the SoC when it is in deep sleep mode. This way, it becomes more
energy-efficient.
The DDR controller will also be powered off in deep sleep. Therefore,
the last stage (the latter part of fsl_dp_enter_low) will run without DDR
access. This piece of code and related TLBs will be prefetched.
Due to the different initialization code between 32-bit and 64-bit, they
have seperate resume entry and precedure.
The feature supports 32-bit and 64-bit kernel mode.
Signed-off-by: Zhao Chenhui <redacted>
---
arch/powerpc/include/asm/booke_save_regs.h | 3 +
arch/powerpc/kernel/cpu_setup_fsl_booke.S | 17 ++
arch/powerpc/kernel/head_fsl_booke.S | 30 +++
arch/powerpc/platforms/85xx/Makefile | 2 +-
arch/powerpc/platforms/85xx/deepsleep.c | 201 +++++++++++++++++++
arch/powerpc/platforms/85xx/qoriq_pm.c | 38 ++++
arch/powerpc/platforms/85xx/sleep.S | 295 ++++++++++++++++++++++++++++
arch/powerpc/sysdev/fsl_soc.h | 7 +
8 files changed, 592 insertions(+), 1 deletions(-)
create mode 100644 arch/powerpc/platforms/85xx/deepsleep.c
create mode 100644 arch/powerpc/platforms/85xx/sleep.S
We have two kind of boards, QDS and RDB.
They have different register map. Use the flag to indicate the current board is using which kind
of register map.
CPLD versus FPGA is not a meaningful difference. We don't care what
technology is used to implement programmable logic -- we care what
programming interface is exposed. Customers will have their own boards
that will likely not imitate either of these programming interfaces, but
what they do have will still probably be implemented in a CPLD or FPGA.
Likewise, Freescale may have future reference boards whose CPLD/FPGA is
not compatible.
So use better naming, and structure the code so it's easy to plug in
implementations for new or custom boards.
Why do you come in via the normal kernel entry, versus specifying a
direct entry point for deep sleep resume? How does U-Boot even know
what the normal entry is when resuming?
I wish to return to a specified point (like 64-bit mode), but the code in
fsl_booke_entry_mapping.S only can run in the first page. Because it
only setups a temp mapping of 4KB.
Why do you need the entry mapping on 32-bit but not 64-bit?
quoted
quoted
+#if defined(CONFIG_SUSPEND) && defined(CONFIG_FSL_CORENET_RCPM)+_ENTRY(__entry_deep_sleep)+/*+ * Bootloader will jump to here when resuming from deep sleep.+ * After executing the init code in fsl_booke_entry_mapping.S,+ * will jump to the real resume entry.+ */+ li r8, 1+ bl 12f+12: mflr r9+ addi r9, r9, (deep_sleep_flag - 12b)+ stw r8, 0(r9)+ b __early_start+deep_sleep_flag:+ .long 0+#endif
It's a bit ambiguous to say "entry_deep_sleep" when it's resuming rather
than entering...
You don't need a namespace prefix on local macros in a non-header file.
Is the timebase stopped where you're calling this from?
No. My purpose is to avoid jump in the last stage of entering deep sleep.
Jump may cause problem at that time.
"bdnz" is a jump.
What problems do you think a jump will cause?
quoted
You also probably want to do a "sync, readback, data dependency, isync"
sequence to make sure that the store has hit CCSR before you begin your
delay (or is a delay required at all if you do that?).
Yes. It is safer with a sync sequence.
The DDR controller need some time to signal the external DDR modules to
enter self refresh mode.
Is it documented how much time it requires?
-Scott
From: Kevin Hao <hidden> Date: 2014-03-16 04:58:13
On Fri, Mar 14, 2014 at 05:26:27PM -0500, Scott Wood wrote:
On Thu, 2014-03-13 at 15:46 +0800, Kevin Hao wrote:
quoted
On Wed, Mar 12, 2014 at 12:43:05PM -0500, Scott Wood wrote:
quoted
quoted
Shouldn't we use "readback, sync" here? The following is quoted form t4240RM:
To guarantee that the results of any sequence of writes to configuration
registers are in effect, the final configuration register write should be
immediately followed by a read of the same register, and that should be
followed by a SYNC instruction. Then accesses can safely be made to memory
regions affected by the configuration register write.
I agree that the sync before the readback is probably not necessary,
since transactions to the same address should already be ordered.
A sync after the readback helps if you're trying to order the readback
with subsequent memory accesses, though in that case wouldn't a sync
alone (no readback) be adequate?
No, we don't just want to order the subsequent memory access here.
The 'write, readback, sync' is the required sequence if we want to make
sure that the writing to CCSR register does really take effect.
quoted
Though maybe not always -- see the
comment near the end of fsl_elbc_write_buf() in
drivers/mtd/nand_fsl_elbc.c. I guess the readback does more than just
make sure the device has seen the write, ensuring that the device has
finished the transaction to the point of acting on another one.
Agree.
quoted
The data dependency plus isync sequence, which is done by the normal I/O
accessors used from C code, orders the readback versus all future
instructions (not just I/O). The delay loop is not I/O.
According to the PowerISA, the sequence 'load, date dependency, isync' only
order the load accesses.
The point is to order the delay loop after the load, not to order
storage versus storage.
I think the point is to make sure that the writing of the CCSR_DDR_SDRAM_CFG_2
does really take effect before we begin to delay loop. The sequence "write,
readback, sync" will guarantee this according to the manual. If we just want to
order the delay loop after the load, the following sequence should be enough:
store to CCSR_DDR_SDRAM_CFG_2
load from CCSR_DDR_SDRAM_CFG_2
isync or sync
delay loop
Why do we need the 'date dependency' here? According to the e6500 manual, the
instructions can execute out of order, but complete in order. So I am really
wondering why we need to order the load and the following delay loop if there
is no intention to order the storage access?
This is a sequence we're already using on all of our I/O loads
(excluding accesses like in this patch that don't use the standard
accessors). I'm confident that it works even if it's not
architecturally guaranteed.
This sequence is used to order the load and the followed storage access.
And this is guaranteed by the architecture. But I don't think it is suitable
for a case like this. The following is quoted from PowerISA:
Because stores cannot be performed “out-of-order”
(see Book III), if a Store instruction depends on the
value returned by a preceding Load instruction
(because the value returned by the Load is used to
compute either the effective address specified by the
Store or the value to be stored), the corresponding stor-
age accesses are performed in program order. The
same applies if whether the Store instruction is exe-
cuted depends on a conditional Branch instruction that
in turn depends on the value returned by a preceding
Load instruction.
Because an isync instruction prevents the execution of
instructions following the isync until instructions pre-
ceding the isync have completed, if an isync follows a
conditional Branch instruction that depends on the
value returned by a preceding Load instruction, the
load on which the Branch depends is performed before
any loads caused by instructions following the isync.
I think the above description would guarantee that the load will be performed
before any storage access (both load and store) following the isync in the
following scenario:
lwz r4, 0(r3)
twi 0, r4, 0
isync
I'm not sure that there exists a clear
architectural way of synchronizing non-storage instructions relative to
storage instructions.
Isn't what the execution synchronization instructions such as sync, isync, mtmsr
do?
Given that isync is documented as preventing any execution of
instructions after the isync until all previous instructions complete,
it doesn't seem to make sense for the architecture to explicitly talk
about loads (as opposed to any other instruction) following a load,
dependent conditional branch, isync sequence.
Sorry, I didn't get what you mean.
quoted
So if we want to order all the storage access as well
as execution synchronization, we should choose sync here.
Do we need execution synchronization or context synchronization?
There is no context-altering instruction here, so I think an execution
synchronizing instruction should be enough here.
The t4240 RM section that talks about a readback and a sync is in the
context of subsequent memory operations ("Then accesses can safely be
made to memory regions affected..."), not arbitrary instructions.
I assume that this sequence also guarantee that the writing does take effect.
There
are also a couple other places in the RM where isync is recommended
instead (when setting LAWs or CCSRBAR), even though those also only
involve memory accesses.
No idea why isync is used here.
In any case, this is not performance critical and thus it's better to
oversynchronize than undersynchronize.
On the contrary I think that sync is oversynchronize instead of
undersynchronize. It not only provide the execution synchronizing but also
order all the storage accesses. That is why I prefer the sync. :-)
Thanks,
Kevin
On Fri, Mar 14, 2014 at 05:41:41PM -0500, Scott Wood wrote:
On Wed, 2014-03-12 at 15:46 +0800, Chenhui Zhao wrote:
quoted
On Tue, Mar 11, 2014 at 06:51:20PM -0500, Scott Wood wrote:
quoted
On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
quoted
In 64-bit mode, kernel just clears the irq soft-enable flag
in struct paca_struct to disable external irqs. But, in
the case of suspend, irqs should be disabled by hardware.
Therefore, hook a function to ppc_md.suspend_disable_irqs
to really disable irqs.
Signed-off-by: Chenhui Zhao <redacted>
---
arch/powerpc/platforms/85xx/corenet_generic.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
Why the underscore version? Don't you want PACA_IRQ_HARD_DIS to be set?
If hard disabling is appropriate here, shouldn't we do it in
generic_suspend_disable_irqs()?
Are there any existing platforms that supply a
ppc_md.suspend_disable_irqs()? I don't see any when grepping.
-Scott
Will use hard_irq_disable().
I think this is a general problem for powerpc.
Should clear MSR_EE before suspend. I agree to put it
in generic_suspend_disable_irqs().
BTW, make sure you test this patchset with CONFIG_DEBUG_PREEMPT and
similar debugging options to help ensure that the soft IRQ state is
being tracked properly.
-Scott
On Fri, Mar 14, 2014 at 05:51:09PM -0500, Scott Wood wrote:
On Wed, 2014-03-12 at 16:34 +0800, Chenhui Zhao wrote:
quoted
On Tue, Mar 11, 2014 at 07:08:43PM -0500, Scott Wood wrote:
quoted
On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
quoted
From: Hongbo Zhang <redacted>
In the last stage of deep sleep, software will trigger a Finite
State Machine (FSM) to control the hardware precedure, such as
board isolation, killing PLLs, removing power, and so on.
When the system is waked up by an interrupt, the FSM controls the
hardware to complete the early resume precedure.
This patch configure the EPU FSM preparing for deep sleep.
Signed-off-by: Hongbo Zhang <redacted>
Signed-off-by: Chenhui Zhao <redacted>
Couldn't this be part of qoriq_pm.c?
Put the code in drivers/platform/fsl/ so that LS1 can share these code.
How can LS1 share it if it's got hardcoded T1040 values?
Where do these magic numbers come from? Which chips are they valid for?
They are for T1040. Can be found in the RCPM chapter of T1040RM.
Then put in a comment to that effect, including what part of the RCPM
chapter.
How do you plan to handle the addition of another SoC with different
values?
-Scott
Had thought that using an array to put these values (pairs of offset and value)
and passing the array to the function.
However, luckily T104x and LS1 have same values for these registers
according to the current Reference Manuals.
-Chenhui
On Fri, Mar 14, 2014 at 06:01:45PM -0500, Scott Wood wrote:
On Wed, 2014-03-12 at 17:42 +0800, Chenhui Zhao wrote:
quoted
On Tue, Mar 11, 2014 at 07:45:14PM -0500, Scott Wood wrote:
quoted
On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
quoted
From: Wang Dongsheng <redacted>
Add booke_cpu_state_save() and booke_cpu_state_restore() functions which can be
used to save/restore CPU's registers in the case of deep sleep and hibernation.
Supported processors: E6500, E5500, E500MC, E500v2 and E500v1.
Signed-off-by: Wang Dongsheng <redacted>
Signed-off-by: Chenhui Zhao <redacted>
---
arch/powerpc/include/asm/booke_save_regs.h | 96 ++++++++
arch/powerpc/kernel/Makefile | 1 +
arch/powerpc/kernel/booke_save_regs.S | 361 ++++++++++++++++++++++++++++
3 files changed, 458 insertions(+), 0 deletions(-)
create mode 100644 arch/powerpc/include/asm/booke_save_regs.h
create mode 100644 arch/powerpc/kernel/booke_save_regs.S
Filename says booke, comment says e500.
Filename and comment also fail to point out that this is specifically
for standby/suspend, not for hibernate which is implemented in
swsusp_booke.S/swsusp_asm64.S.
Sorry for inconsistency. Will changes e500 to booke.
Hibernation and suspend can share the code.
Maybe they could, but AFAICT this patchset doesn't make that happen --
and I'm not convinced that the churn would be worthwhile. Note that
swsusp_asm64.S is not just for booke, so most of that file would not be
going away if you did make such a change.
OK. Let's put Hibernation aside, and change the code just for suspend.
I also don't like the way it looks like booke_save_regs.S is a booke
version of ppc_save_regs.S, even though they serve different purposes
and ppc_save_regs.S is still relevant to booke.
quoted
quoted
quoted
+ * Software-Use Registers+ * SPRG1 0x260 (dw * 76), 64-bit need to save.+ * SPRG3 0x268 (dw * 77), 32-bit need to save.
What about "CPU and NUMA node for VDSO getcpu" on 64-bit? Currently
SPRG3, but it will need to change for critical interrupt support.
PID1/PID2 are e500v1/v2 only -- and Linux doesn't use them outside of
KVM (and you're not in KVM when you're running this code).
Are we ever going to have a non-zero PID at this point?
I incline to the view that saving all registers regardless of used or
unused. The good point is that it can be compliant to the future
changes of the usage of registers.
What do you think?
I agree to a certain extent, but balance it with the complexity of
dealing with registers that don't exist on all booke chips. If they
don't really need to be saved, why go through the hassle of conditional
code?
I agree. For these registers, I'll check if they are really needed.
-Chenhui
On Fri, Mar 14, 2014 at 06:18:27PM -0500, Scott Wood wrote:
On Wed, 2014-03-12 at 18:40 +0800, Chenhui Zhao wrote:
quoted
On Tue, Mar 11, 2014 at 08:10:24PM -0500, Scott Wood wrote:
quoted
On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
quoted
From: Zhao Chenhui <redacted>
T1040 supports deep sleep feature, which can switch off most parts of
the SoC when it is in deep sleep mode. This way, it becomes more
energy-efficient.
The DDR controller will also be powered off in deep sleep. Therefore,
the last stage (the latter part of fsl_dp_enter_low) will run without DDR
access. This piece of code and related TLBs will be prefetched.
Due to the different initialization code between 32-bit and 64-bit, they
have seperate resume entry and precedure.
The feature supports 32-bit and 64-bit kernel mode.
Signed-off-by: Zhao Chenhui <redacted>
---
arch/powerpc/include/asm/booke_save_regs.h | 3 +
arch/powerpc/kernel/cpu_setup_fsl_booke.S | 17 ++
arch/powerpc/kernel/head_fsl_booke.S | 30 +++
arch/powerpc/platforms/85xx/Makefile | 2 +-
arch/powerpc/platforms/85xx/deepsleep.c | 201 +++++++++++++++++++
arch/powerpc/platforms/85xx/qoriq_pm.c | 38 ++++
arch/powerpc/platforms/85xx/sleep.S | 295 ++++++++++++++++++++++++++++
arch/powerpc/sysdev/fsl_soc.h | 7 +
8 files changed, 592 insertions(+), 1 deletions(-)
create mode 100644 arch/powerpc/platforms/85xx/deepsleep.c
create mode 100644 arch/powerpc/platforms/85xx/sleep.S
We have two kind of boards, QDS and RDB.
They have different register map. Use the flag to indicate the current board is using which kind
of register map.
CPLD versus FPGA is not a meaningful difference. We don't care what
technology is used to implement programmable logic -- we care what
programming interface is exposed. Customers will have their own boards
that will likely not imitate either of these programming interfaces, but
what they do have will still probably be implemented in a CPLD or FPGA.
Likewise, Freescale may have future reference boards whose CPLD/FPGA is
not compatible.
Will use a better name.
So use better naming, and structure the code so it's easy to plug in
implementations for new or custom boards.
Why do you come in via the normal kernel entry, versus specifying a
direct entry point for deep sleep resume? How does U-Boot even know
what the normal entry is when resuming?
I wish to return to a specified point (like 64-bit mode), but the code in
fsl_booke_entry_mapping.S only can run in the first page. Because it
only setups a temp mapping of 4KB.
Why do you need the entry mapping on 32-bit but not 64-bit?
fsl_booke_entry_mapping.S is for 32-bit. 64-bit calls
initial_tlb_book3e() in exceptions-64e.S.
quoted
quoted
quoted
+#if defined(CONFIG_SUSPEND) && defined(CONFIG_FSL_CORENET_RCPM)+_ENTRY(__entry_deep_sleep)+/*+ * Bootloader will jump to here when resuming from deep sleep.+ * After executing the init code in fsl_booke_entry_mapping.S,+ * will jump to the real resume entry.+ */+ li r8, 1+ bl 12f+12: mflr r9+ addi r9, r9, (deep_sleep_flag - 12b)+ stw r8, 0(r9)+ b __early_start+deep_sleep_flag:+ .long 0+#endif
It's a bit ambiguous to say "entry_deep_sleep" when it's resuming rather
than entering...
You don't need a namespace prefix on local macros in a non-header file.
Is the timebase stopped where you're calling this from?
No. My purpose is to avoid jump in the last stage of entering deep sleep.
Jump may cause problem at that time.
"bdnz" is a jump.
What problems do you think a jump will cause?
I mean a far jump which can jump to an address which has not been prefetched in
advance. I wish the code is executed in a restricted environment (predictable code
and address).
quoted
quoted
You also probably want to do a "sync, readback, data dependency, isync"
sequence to make sure that the store has hit CCSR before you begin your
delay (or is a delay required at all if you do that?).
Yes. It is safer with a sync sequence.
The DDR controller need some time to signal the external DDR modules to
enter self refresh mode.
Is it documented how much time it requires?
-Scott
From: Scott Wood <hidden> Date: 2014-03-18 22:42:40
On Mon, 2014-03-17 at 19:19 +0800, Chenhui Zhao wrote:
On Fri, Mar 14, 2014 at 06:18:27PM -0500, Scott Wood wrote:
quoted
On Wed, 2014-03-12 at 18:40 +0800, Chenhui Zhao wrote:
quoted
On Tue, Mar 11, 2014 at 08:10:24PM -0500, Scott Wood wrote:
quoted
On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
quoted
From: Zhao Chenhui <redacted>
T1040 supports deep sleep feature, which can switch off most parts of
the SoC when it is in deep sleep mode. This way, it becomes more
energy-efficient.
The DDR controller will also be powered off in deep sleep. Therefore,
the last stage (the latter part of fsl_dp_enter_low) will run without DDR
access. This piece of code and related TLBs will be prefetched.
Due to the different initialization code between 32-bit and 64-bit, they
have seperate resume entry and precedure.
The feature supports 32-bit and 64-bit kernel mode.
Signed-off-by: Zhao Chenhui <redacted>
---
arch/powerpc/include/asm/booke_save_regs.h | 3 +
arch/powerpc/kernel/cpu_setup_fsl_booke.S | 17 ++
arch/powerpc/kernel/head_fsl_booke.S | 30 +++
arch/powerpc/platforms/85xx/Makefile | 2 +-
arch/powerpc/platforms/85xx/deepsleep.c | 201 +++++++++++++++++++
arch/powerpc/platforms/85xx/qoriq_pm.c | 38 ++++
arch/powerpc/platforms/85xx/sleep.S | 295 ++++++++++++++++++++++++++++
arch/powerpc/sysdev/fsl_soc.h | 7 +
8 files changed, 592 insertions(+), 1 deletions(-)
create mode 100644 arch/powerpc/platforms/85xx/deepsleep.c
create mode 100644 arch/powerpc/platforms/85xx/sleep.S
We have two kind of boards, QDS and RDB.
They have different register map. Use the flag to indicate the current board is using which kind
of register map.
CPLD versus FPGA is not a meaningful difference. We don't care what
technology is used to implement programmable logic -- we care what
programming interface is exposed. Customers will have their own boards
that will likely not imitate either of these programming interfaces, but
what they do have will still probably be implemented in a CPLD or FPGA.
Likewise, Freescale may have future reference boards whose CPLD/FPGA is
not compatible.
Will use a better name.
quoted
So use better naming, and structure the code so it's easy to plug in
implementations for new or custom boards.
Why do you come in via the normal kernel entry, versus specifying a
direct entry point for deep sleep resume? How does U-Boot even know
what the normal entry is when resuming?
I wish to return to a specified point (like 64-bit mode), but the code in
fsl_booke_entry_mapping.S only can run in the first page. Because it
only setups a temp mapping of 4KB.
Why do you need the entry mapping on 32-bit but not 64-bit?
fsl_booke_entry_mapping.S is for 32-bit. 64-bit calls
initial_tlb_book3e() in exceptions-64e.S.
The answer I was looking for is that __entry_deep_sleep calls
start_initialization_book3e which calls the code to handle it.
But why is it driven from sleep.S on 64-bit but not on 32-bit? Why
can't you make it so that the 32-bit TLB setup can be called into in a
similar manner?
You don't need a namespace prefix on local macros in a non-header file.
Is the timebase stopped where you're calling this from?
No. My purpose is to avoid jump in the last stage of entering deep sleep.
Jump may cause problem at that time.
"bdnz" is a jump.
What problems do you think a jump will cause?
I mean a far jump which can jump to an address which has not been prefetched in
advance. I wish the code is executed in a restricted environment (predictable code
and address).
Why would a timebase loop require a "far" jump?
quoted
quoted
quoted
You also probably want to do a "sync, readback, data dependency, isync"
sequence to make sure that the store has hit CCSR before you begin your
delay (or is a delay required at all if you do that?).
Yes. It is safer with a sync sequence.
The DDR controller need some time to signal the external DDR modules to
enter self refresh mode.
Is it documented how much time it requires?
-Scott
No.
How do you know the current delay is adequate in all circumstances (e.g
clock speeds), much less on future chips? Is it documented that a delay
is needed at all, or is this just something that appeared to make it
work? If the latter, what happens if you put the synchronization in,
but leave out the delay?
-Scott
From: Scott Wood <hidden> Date: 2014-03-18 23:19:21
On Sun, 2014-03-16 at 12:58 +0800, Kevin Hao wrote:
On Fri, Mar 14, 2014 at 05:26:27PM -0500, Scott Wood wrote:
quoted
On Thu, 2014-03-13 at 15:46 +0800, Kevin Hao wrote:
quoted
On Wed, Mar 12, 2014 at 12:43:05PM -0500, Scott Wood wrote:
quoted
quoted
Shouldn't we use "readback, sync" here? The following is quoted form t4240RM:
To guarantee that the results of any sequence of writes to configuration
registers are in effect, the final configuration register write should be
immediately followed by a read of the same register, and that should be
followed by a SYNC instruction. Then accesses can safely be made to memory
regions affected by the configuration register write.
I agree that the sync before the readback is probably not necessary,
since transactions to the same address should already be ordered.
A sync after the readback helps if you're trying to order the readback
with subsequent memory accesses, though in that case wouldn't a sync
alone (no readback) be adequate?
No, we don't just want to order the subsequent memory access here.
The 'write, readback, sync' is the required sequence if we want to make
sure that the writing to CCSR register does really take effect.
quoted
Though maybe not always -- see the
comment near the end of fsl_elbc_write_buf() in
drivers/mtd/nand_fsl_elbc.c. I guess the readback does more than just
make sure the device has seen the write, ensuring that the device has
finished the transaction to the point of acting on another one.
Agree.
quoted
The data dependency plus isync sequence, which is done by the normal I/O
accessors used from C code, orders the readback versus all future
instructions (not just I/O). The delay loop is not I/O.
According to the PowerISA, the sequence 'load, date dependency, isync' only
order the load accesses.
The point is to order the delay loop after the load, not to order
storage versus storage.
I think the point is to make sure that the writing of the CCSR_DDR_SDRAM_CFG_2
does really take effect before we begin to delay loop.
Yes.
The sequence "write, readback, sync" will guarantee this according to the manual.
If you're referring to the section you quoted above, the manual does not
say that. It only talks about when accesses "to memory regions affected
by the configuration register write" can be safely made.
If we just want to
order the delay loop after the load, the following sequence should be enough:
store to CCSR_DDR_SDRAM_CFG_2
load from CCSR_DDR_SDRAM_CFG_2
isync or sync
delay loop
Why do we need the 'date dependency' here? According to the e6500 manual, the
instructions can execute out of order, but complete in order. So I am really
wondering why we need to order the load and the following delay loop if there
is no intention to order the storage access?
The data (not date) dependency means that the twi will not execute until
after the load instruction returns data (thus, after the device has
responded to the load). The twi, being a flow control instruction
rather than a storage instruction, should be fully ordered by isync.
From the isync description in the ISA: "Except as described in the
preceding sentence, the isync instruction may complete before storage
accesses associated with instructions preceding the
isync instruction have been performed."
I don't know if that really applies to loads (as opposed to stores), and
it probably doesn't apply to guarded loads, but I feel safer leaving the
twi in.
As for sync instead of isync, I see nothing to indicate that it would be
adequate (though it may be that the only reason there needed to be a
delay loop in the first place was the lack of a readback/sync before
doing other I/O, in which case this is moot).
quoted
This is a sequence we're already using on all of our I/O loads
(excluding accesses like in this patch that don't use the standard
accessors). I'm confident that it works even if it's not
architecturally guaranteed.
This sequence is used to order the load and the followed storage access.
It's also used to order loads versus other things, such as enabling
MSR[EE].
And this is guaranteed by the architecture. But I don't think it is suitable
for a case like this. The following is quoted from PowerISA:
Because stores cannot be performed “out-of-order”
(see Book III), if a Store instruction depends on the
value returned by a preceding Load instruction
(because the value returned by the Load is used to
compute either the effective address specified by the
Store or the value to be stored), the corresponding stor-
age accesses are performed in program order. The
same applies if whether the Store instruction is exe-
cuted depends on a conditional Branch instruction that
in turn depends on the value returned by a preceding
Load instruction.
Because an isync instruction prevents the execution of
instructions following the isync until instructions pre-
ceding the isync have completed, if an isync follows a
conditional Branch instruction that depends on the
value returned by a preceding Load instruction, the
load on which the Branch depends is performed before
any loads caused by instructions following the isync.
I think the above description would guarantee that the load will be performed
before any storage access (both load and store) following the isync in the
following scenario:
lwz r4, 0(r3)
twi 0, r4, 0
isync
I think it's a poorly worded section, but yes, it guarantees both loads
and stores -- unnecessarily doing so in separate places with different
wording. But by the definition of isync I don't see why it does not
apply to *any* instruction after the isync, not just loads and stores.
quoted
I'm not sure that there exists a clear
architectural way of synchronizing non-storage instructions relative to
storage instructions.
Isn't what the execution synchronization instructions such as sync, isync, mtmsr
do?
No. Execution synchronizing just says that the previous instructions
"have completed to a point at which they have reported all the
exceptions they will cause" (I'm assuming this doesn't include machine
check error reports), and that the previous instructions won't be
affected by context changes after the execution synchronizing
instruction, not that the previous instructions are fully completed.
quoted
Given that isync is documented as preventing any execution of
instructions after the isync until all previous instructions complete,
it doesn't seem to make sense for the architecture to explicitly talk
about loads (as opposed to any other instruction) following a load,
dependent conditional branch, isync sequence.
Sorry, I didn't get what you mean.
I mean that I do not understand why it says, "the load on which the
Branch depends is performed before any loads caused by instructions
following the isync" rather than "the Load on which the Branch depends
is performed before any instructions following the isync".
quoted
quoted
So if we want to order all the storage access as well
as execution synchronization, we should choose sync here.
Do we need execution synchronization or context synchronization?
There is no context-altering instruction here, so I think an execution
synchronizing instruction should be enough here.
Is the ISA ever explicit about what constitutes "context"? In any case,
just because we don't need that aspect of context synchronization
doesn't mean execution synchronization is enough. The behavior we want
is described in the isync instruction specifically, not in "context
synchronization" or "execution synchronization".
quoted
The t4240 RM section that talks about a readback and a sync is in the
context of subsequent memory operations ("Then accesses can safely be
made to memory regions affected..."), not arbitrary instructions.
I assume that this sequence also guarantee that the writing does take effect.
You may assume that, but the manual doesn't say that. Sync could
(AFAIK) be implemented by emitting a barrier on the bus, or delaying
future storage accesses, rather than waiting for everything to have
finished before proceeding.
quoted
In any case, this is not performance critical and thus it's better to
oversynchronize than undersynchronize.
On the contrary I think that sync is oversynchronize instead of
undersynchronize. It not only provide the execution synchronizing but also
order all the storage accesses. That is why I prefer the sync. :-)
sync is not a superset of isync. isync is not a superset of sync. If
you want to oversynchronize to be safe, do both (though even that isn't
equivalent to a barrier that orders everything, which is partly why a
readback is needed).
-Scott
Where do these magic numbers come from? Which chips are they valid for?
They are for T1040. Can be found in the RCPM chapter of T1040RM.
Then put in a comment to that effect, including what part of the RCPM
chapter.
How do you plan to handle the addition of another SoC with different
values?
-Scott
Had thought that using an array to put these values (pairs of offset and value)
and passing the array to the function.
Arrays are better than a long sequence of function calls in any case.
However, luckily T104x and LS1 have same values for these registers
according to the current Reference Manuals.
If it's likely that the values will remain the same on all chips for the
near future, then a fancy mechanism to select the array to use can wait
-- but you should still use an array, and have a comment acknowledging
the possibility of needing to accommodate different values in the
future.
-Scott
Where do these magic numbers come from? Which chips are they valid for?
They are for T1040. Can be found in the RCPM chapter of T1040RM.
Then put in a comment to that effect, including what part of the RCPM
chapter.
How do you plan to handle the addition of another SoC with different
values?
-Scott
Had thought that using an array to put these values (pairs of offset and value)
and passing the array to the function.
Arrays are better than a long sequence of function calls in any case.
quoted
However, luckily T104x and LS1 have same values for these registers
according to the current Reference Manuals.
If it's likely that the values will remain the same on all chips for the
near future, then a fancy mechanism to select the array to use can wait
-- but you should still use an array, and have a comment acknowledging
the possibility of needing to accommodate different values in the
future.
-Scott
OK. Will use an array to pass the values.
-Chenhui
On Tue, Mar 18, 2014 at 05:42:09PM -0500, Scott Wood wrote:
On Mon, 2014-03-17 at 19:19 +0800, Chenhui Zhao wrote:
quoted
On Fri, Mar 14, 2014 at 06:18:27PM -0500, Scott Wood wrote:
quoted
On Wed, 2014-03-12 at 18:40 +0800, Chenhui Zhao wrote:
quoted
On Tue, Mar 11, 2014 at 08:10:24PM -0500, Scott Wood wrote:
quoted
On Fri, 2014-03-07 at 12:58 +0800, Chenhui Zhao wrote:
quoted
From: Zhao Chenhui <redacted>
T1040 supports deep sleep feature, which can switch off most parts of
the SoC when it is in deep sleep mode. This way, it becomes more
energy-efficient.
The DDR controller will also be powered off in deep sleep. Therefore,
the last stage (the latter part of fsl_dp_enter_low) will run without DDR
access. This piece of code and related TLBs will be prefetched.
Due to the different initialization code between 32-bit and 64-bit, they
have seperate resume entry and precedure.
The feature supports 32-bit and 64-bit kernel mode.
Signed-off-by: Zhao Chenhui <redacted>
---
arch/powerpc/include/asm/booke_save_regs.h | 3 +
arch/powerpc/kernel/cpu_setup_fsl_booke.S | 17 ++
arch/powerpc/kernel/head_fsl_booke.S | 30 +++
arch/powerpc/platforms/85xx/Makefile | 2 +-
arch/powerpc/platforms/85xx/deepsleep.c | 201 +++++++++++++++++++
arch/powerpc/platforms/85xx/qoriq_pm.c | 38 ++++
arch/powerpc/platforms/85xx/sleep.S | 295 ++++++++++++++++++++++++++++
arch/powerpc/sysdev/fsl_soc.h | 7 +
8 files changed, 592 insertions(+), 1 deletions(-)
create mode 100644 arch/powerpc/platforms/85xx/deepsleep.c
create mode 100644 arch/powerpc/platforms/85xx/sleep.S
We have two kind of boards, QDS and RDB.
They have different register map. Use the flag to indicate the current board is using which kind
of register map.
CPLD versus FPGA is not a meaningful difference. We don't care what
technology is used to implement programmable logic -- we care what
programming interface is exposed. Customers will have their own boards
that will likely not imitate either of these programming interfaces, but
what they do have will still probably be implemented in a CPLD or FPGA.
Likewise, Freescale may have future reference boards whose CPLD/FPGA is
not compatible.
Will use a better name.
quoted
So use better naming, and structure the code so it's easy to plug in
implementations for new or custom boards.
Why do you come in via the normal kernel entry, versus specifying a
direct entry point for deep sleep resume? How does U-Boot even know
what the normal entry is when resuming?
I wish to return to a specified point (like 64-bit mode), but the code in
fsl_booke_entry_mapping.S only can run in the first page. Because it
only setups a temp mapping of 4KB.
Why do you need the entry mapping on 32-bit but not 64-bit?
fsl_booke_entry_mapping.S is for 32-bit. 64-bit calls
initial_tlb_book3e() in exceptions-64e.S.
The answer I was looking for is that __entry_deep_sleep calls
start_initialization_book3e which calls the code to handle it.
But why is it driven from sleep.S on 64-bit but not on 32-bit? Why
can't you make it so that the 32-bit TLB setup can be called into in a
similar manner?
Yes. I also wish to do like this. As I mentioned, the problem in 32-bit
is that the TLB setup code in fsl_booke_entry_mapping.S only setups a temp
mapping of 4KB, so these code only can run in this 4KB address space. But the
code in sleep.S is outside of the 4KB space. So can't put the TLB setup
code in sleep.S. There are two method to solve it.
1) The current method is running the TLB setup code of fsl_booke_entry_mapping.S in the 4KB
space, then jump to the code of sleep.S.
2) extend the temp mapping space in the TLB setup code to cover kernel, say 4MB or 8MB. But
not sure if there are any side effects.
You don't need a namespace prefix on local macros in a non-header file.
Is the timebase stopped where you're calling this from?
No. My purpose is to avoid jump in the last stage of entering deep sleep.
Jump may cause problem at that time.
"bdnz" is a jump.
What problems do you think a jump will cause?
I mean a far jump which can jump to an address which has not been prefetched in
advance. I wish the code is executed in a restricted environment (predictable code
and address).
Why would a timebase loop require a "far" jump?
I mean there is far jump in udely().
Do you mean using a timebase loop in the macro FSLDELAY? If so, I agree.
quoted
quoted
quoted
quoted
You also probably want to do a "sync, readback, data dependency, isync"
sequence to make sure that the store has hit CCSR before you begin your
delay (or is a delay required at all if you do that?).
Yes. It is safer with a sync sequence.
The DDR controller need some time to signal the external DDR modules to
enter self refresh mode.
Is it documented how much time it requires?
-Scott
No.
How do you know the current delay is adequate in all circumstances (e.g
clock speeds), much less on future chips? Is it documented that a delay
is needed at all, or is this just something that appeared to make it
work? If the latter, what happens if you put the synchronization in,
but leave out the delay?
-Scott
The code controls external parts (FPGA/CPLD, DDR module) to act, and
the sequent code must wait until external parts complete. We can't get
an ack from external parts, so use delay to make sure the sequence and
insert enough time to wait.
-Chenhui
From: Kevin Hao <hidden> Date: 2014-03-20 11:47:56
On Tue, Mar 18, 2014 at 06:18:54PM -0500, Scott Wood wrote:
quoted
The sequence "write, readback, sync" will guarantee this according to the manual.
If you're referring to the section you quoted above, the manual does not
say that. It only talks about when accesses "to memory regions affected
by the configuration register write" can be safely made.
To guarantee that the results of any sequence of writes to configuration
registers are in effect, the final configuration register write should be
immediately followed by a read of the same register, and that should be
followed by a SYNC instruction. Then accesses can safely be made to memory
regions affected by the configuration register write.
According to the above description in t4240 RM manual (2.6.1 Accessing CCSR
Memory from the Local Processor), that the writing to configuration register
takes effect is a prerequisite for the memory access to the affected regions.
quoted
If we just want to
order the delay loop after the load, the following sequence should be enough:
store to CCSR_DDR_SDRAM_CFG_2
load from CCSR_DDR_SDRAM_CFG_2
isync or sync
delay loop
Why do we need the 'date dependency' here? According to the e6500 manual, the
instructions can execute out of order, but complete in order. So I am really
wondering why we need to order the load and the following delay loop if there
is no intention to order the storage access?
The data (not date) dependency means that the twi will not execute until
after the load instruction returns data (thus, after the device has
responded to the load). The twi, being a flow control instruction
rather than a storage instruction, should be fully ordered by isync.
From the isync description in the ISA: "Except as described in the
preceding sentence, the isync instruction may complete before storage
accesses associated with instructions preceding the
isync instruction have been performed."
I don't know if that really applies to loads (as opposed to stores), and
it probably doesn't apply to guarded loads, but I feel safer leaving the
twi in.
As for sync instead of isync, I see nothing to indicate that it would be
adequate (though it may be that the only reason there needed to be a
delay loop in the first place was the lack of a readback/sync before
doing other I/O, in which case this is moot).
OK, so the intention of 'twi, isync' following the load is not to order the
following storage access, but order the following delay loop instructions,
right? But according to the e6500 manual, the instructions complete in order.
The following is the definition of 'complete':
Complete—An instruction is eligible to complete after it finishes executing
and makes its results available for subsequent instructions. Instructions
must complete in order from the bottom two entries of the
completion queue (CQ). The completion unit coordinates how instructions (which
may have executed out of order) affect architected registers to ensure the
appearance of serial execution. This guarantees that the completed instruction
and all previous instructions can cause no exceptions. An instruction completes
when it is retired, that is, deleted from the CQ.
So the following delay loop instructions should never complete before the
complete of the load instruction. After the complete of load instruction,
the data should already been updated to the architecture register. So we can
guarantee that the load instruction get the data (the device has responded to
the load) before the complete of the following delay loop instructions, why do
we need the additional 'twi, isync'? Then for a case like this (which don't
need order the following storage access), I think the following sequence should
be enough:
write to configuration register
read back
delay loop
I mean that I do not understand why it says, "the load on which the
Branch depends is performed before any loads caused by instructions
following the isync" rather than "the Load on which the Branch depends
is performed before any instructions following the isync".
When we talk about the 'performed' here, we should only mean the effect of
storage access. The following is the definition of 'performed':
performed
A load or instruction fetch by a processor or mech-
anism (P1) is performed with respect to any pro-
cessor or mechanism (P2) when the value to be
returned by the load or instruction fetch can no
longer be changed by a store by P2. A store by P1
is performed with respect to P2 when a load by P2
from the location accessed by the store will return
the value stored (or a value stored subsequently).
An instruction cache block invalidation by P1 is
performed with respect to P2 when an instruction
fetch by P2 will not be satisfied from the copy of
the block that existed in its instruction cache when
the instruction causing the invalidation was exe-
cuted, and similarly for a data cache block invalida-
tion.
The preceding definitions apply regardless of
whether P1 and P2 are the same entity.
quoted
quoted
quoted
So if we want to order all the storage access as well
as execution synchronization, we should choose sync here.
Do we need execution synchronization or context synchronization?
There is no context-altering instruction here, so I think an execution
synchronizing instruction should be enough here.
Is the ISA ever explicit about what constitutes "context"?
The following is the definition of context-altering instruction:
An instruction that alters the context in which data
addresses or instruction addresses are interpreted, or
in which instructions are executed or data accesses are
performed, is called a context-altering instruction.
So the context should be:
- in which data addresses or instruction addresses are interpreted
- in which instructions are executed
- in which data accesses are performed
Thanks,
Kevin
From: Scott Wood <hidden> Date: 2014-03-20 22:08:08
On Thu, 2014-03-20 at 11:59 +0000, David Laight wrote:
I tried to work out what the 'twi, isync' instructions were for (in in_le32()).
The best I could come up with was to ensure a synchronous bus-fault.
But bus faults are probably only expected during device probing - not
normal operation, and the instructions will have a significant cost.
Additionally in_le32() and out_le32() both start with a 'sync' instruction.
In many cases that isn't needed either - an explicit iosync() can be
used after groups of instructions.
The idea is that it's better to be maximally safe by default, and let
performance critical sections be optimized using raw accessors and
explicit synchronization if needed, than to have hard-to-debug bugs due
to missing/wrong sync. A lot of I/O is slow enough that the performance
impact doesn't really matter, but the brain-time cost of getting the
sync right is still there.
-Scott
From: Scott Wood <hidden> Date: 2014-03-20 22:17:57
On Thu, 2014-03-20 at 19:47 +0800, Kevin Hao wrote:
OK, so the intention of 'twi, isync' following the load is not to order the
following storage access, but order the following delay loop instructions,
right? But according to the e6500 manual, the instructions complete in order.
The following is the definition of 'complete':
Complete—An instruction is eligible to complete after it finishes executing
and makes its results available for subsequent instructions. Instructions
must complete in order from the bottom two entries of the
completion queue (CQ). The completion unit coordinates how instructions (which
may have executed out of order) affect architected registers to ensure the
appearance of serial execution. This guarantees that the completed instruction
and all previous instructions can cause no exceptions. An instruction completes
when it is retired, that is, deleted from the CQ.
So the following delay loop instructions should never complete before the
complete of the load instruction.
We don't want the delay loop instructions to *start* until the load has
completed. Completion of the loop only matters when ordering the loop
versus post-loop actions (and again, there we'd want the loop to
complete before subsequent actions start).
quoted
quoted
quoted
quoted
So if we want to order all the storage access as well
as execution synchronization, we should choose sync here.
Do we need execution synchronization or context synchronization?
There is no context-altering instruction here, so I think an execution
synchronizing instruction should be enough here.
Is the ISA ever explicit about what constitutes "context"?
The following is the definition of context-altering instruction:
An instruction that alters the context in which data
addresses or instruction addresses are interpreted, or
in which instructions are executed or data accesses are
performed, is called a context-altering instruction.
So the context should be:
- in which data addresses or instruction addresses are interpreted
- in which instructions are executed
- in which data accesses are performed
By that definition, a store to CCSR could easily change the context in
which data accesses are performed, by changing a mapping.
But still, nothing in the above defines "context" -- or rather, it does
so circularly. While it makes intuitive sense that it would be limited
to context that lives within the core, rather than the rest of the
system, I don't think the ISA generally distinguishes between the two.
-Scott
From: Scott Wood <hidden> Date: 2014-03-20 23:33:20
On Wed, 2014-03-19 at 08:56 +0800, Chenhui Zhao wrote:
On Tue, Mar 18, 2014 at 05:42:09PM -0500, Scott Wood wrote:
quoted
On Mon, 2014-03-17 at 19:19 +0800, Chenhui Zhao wrote:
quoted
On Fri, Mar 14, 2014 at 06:18:27PM -0500, Scott Wood wrote:
quoted
Why do you need the entry mapping on 32-bit but not 64-bit?
fsl_booke_entry_mapping.S is for 32-bit. 64-bit calls
initial_tlb_book3e() in exceptions-64e.S.
The answer I was looking for is that __entry_deep_sleep calls
start_initialization_book3e which calls the code to handle it.
But why is it driven from sleep.S on 64-bit but not on 32-bit? Why
can't you make it so that the 32-bit TLB setup can be called into in a
similar manner?
Yes. I also wish to do like this. As I mentioned, the problem in 32-bit
is that the TLB setup code in fsl_booke_entry_mapping.S only setups a temp
mapping of 4KB, so these code only can run in this 4KB address space. But the
code in sleep.S is outside of the 4KB space. So can't put the TLB setup
code in sleep.S. There are two method to solve it.
1) The current method is running the TLB setup code of fsl_booke_entry_mapping.S in the 4KB
space, then jump to the code of sleep.S.
2) extend the temp mapping space in the TLB setup code to cover kernel, say 4MB or 8MB. But
not sure if there are any side effects.
fsl_booke_entry_mapping.S creates a 64M entry. The 4K entry is only
temporary while in AS1 -- it shouldn't matter if the address you return
to when leaving fsl_booke_entry_mapping.S is outside the 4K, as long as
it's within the 64M entry.
Or am I missing something?
You don't need a namespace prefix on local macros in a non-header file.
Is the timebase stopped where you're calling this from?
No. My purpose is to avoid jump in the last stage of entering deep sleep.
Jump may cause problem at that time.
"bdnz" is a jump.
What problems do you think a jump will cause?
I mean a far jump which can jump to an address which has not been prefetched in
advance. I wish the code is executed in a restricted environment (predictable code
and address).
Why would a timebase loop require a "far" jump?
I mean there is far jump in udely().
Do you mean using a timebase loop in the macro FSLDELAY? If so, I agree.
Yes, I meant a timebase loop, not udelay().
quoted
quoted
quoted
quoted
quoted
You also probably want to do a "sync, readback, data dependency, isync"
sequence to make sure that the store has hit CCSR before you begin your
delay (or is a delay required at all if you do that?).
Yes. It is safer with a sync sequence.
The DDR controller need some time to signal the external DDR modules to
enter self refresh mode.
Is it documented how much time it requires?
-Scott
No.
How do you know the current delay is adequate in all circumstances (e.g
clock speeds), much less on future chips? Is it documented that a delay
is needed at all, or is this just something that appeared to make it
work? If the latter, what happens if you put the synchronization in,
but leave out the delay?
-Scott
The code controls external parts (FPGA/CPLD, DDR module) to act, and
the sequent code must wait until external parts complete. We can't get
an ack from external parts, so use delay to make sure the sequence and
insert enough time to wait.
It would be good if you could get the hardware designers to provide an
upper bound for how long we need to wait.
-Scott
From: Scott Wood <hidden> Date: 2014-03-21 21:16:45
On Fri, 2014-03-21 at 09:21 +0000, David Laight wrote:
From: Scott Wood [mailto:scottwood@freescale.com]
quoted
On Thu, 2014-03-20 at 11:59 +0000, David Laight wrote:
quoted
I tried to work out what the 'twi, isync' instructions were for (in in_le32()).
The best I could come up with was to ensure a synchronous bus-fault.
But bus faults are probably only expected during device probing - not
normal operation, and the instructions will have a significant cost.
Additionally in_le32() and out_le32() both start with a 'sync' instruction.
In many cases that isn't needed either - an explicit iosync() can be
used after groups of instructions.
The idea is that it's better to be maximally safe by default, and let
performance critical sections be optimized using raw accessors and
explicit synchronization if needed, than to have hard-to-debug bugs due
to missing/wrong sync. A lot of I/O is slow enough that the performance
impact doesn't really matter, but the brain-time cost of getting the
sync right is still there.
Hmmm....
That might be an excuse for the 'sync', but not the twi and isync.
That might be true if I/O is always cache inhibited and guarded, in
which case I think we can rely on that to ensure that the load has
completed before we do things like wrtee or rfi. In any case, I'd want
to hear Ben's explanation.
I was setting up a dma request (for the ppc 83xx PCIe bridge) and
was doing back to back little-endian writes into memory.
I had difficulty finding and including header files containing
the definitions for byteswapped accesses I needed.
arch/powerpc/include/asm/swab.h contains some - but I couldn't
work out how to get it included (apart from giving the full path).
In any case you need to understand when synchronisation is
required - otherwise you will get it wrong.
Especially since non-byteswapped accesses are done by direct
access.
Yes, it's bad that rawness combines the lack of byteswapping with the
lack of synchronization. Ideally the raw accessors would also come in
big and little endian form, plus a native endian form if it's really
needed.
-Scott