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:
> > 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:
> > > > From: Zhao Chenhui <chenhui.z...@freescale.com>
> > > > 
> > > > 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 <chenhui.z...@freescale.com>
> > > > ---
> > > >  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
> > > > 
> > > > diff --git a/arch/powerpc/include/asm/booke_save_regs.h 
> > > > b/arch/powerpc/include/asm/booke_save_regs.h
> > > > index 87c357a..37c1f6c 100644
> > > > --- a/arch/powerpc/include/asm/booke_save_regs.h
> > > > +++ b/arch/powerpc/include/asm/booke_save_regs.h
> > > > @@ -88,6 +88,9 @@
> > > >  #define HIBERNATION_FLAG       1
> > > >  #define DEEPSLEEP_FLAG         2
> > > >  
> > > > +#define CPLD_FLAG      1
> > > > +#define FPGA_FLAG      2
> > > 
> > > What is this?
> > 
> > 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.
>  
> > > > diff --git a/arch/powerpc/kernel/head_fsl_booke.S 
> > > > b/arch/powerpc/kernel/head_fsl_booke.S
> > > > index 20204fe..3285752 100644
> > > > --- a/arch/powerpc/kernel/head_fsl_booke.S
> > > > +++ b/arch/powerpc/kernel/head_fsl_booke.S
> > > > @@ -162,6 +162,19 @@ _ENTRY(__early_start)
> > > >  #include "fsl_booke_entry_mapping.S"
> > > >  #undef ENTRY_MAPPING_BOOT_SETUP
> > > >  
> > > > +#if defined(CONFIG_SUSPEND) && defined(CONFIG_FSL_CORENET_RCPM)
> > > > +       /* if deep_sleep_flag != 0, jump to the deep sleep resume entry 
> > > > */
> > > > +       LOAD_REG_ADDR(r4, deep_sleep_flag)
> > > > +       lwz     r3, 0(r4)
> > > > +       cmpwi   r3, 0
> > > > +       beq     11f
> > > > +       /* clear deep_sleep_flag */
> > > > +       li      r3, 0
> > > > +       stw     r3, 0(r4)
> > > > +       b       fsl_deepsleep_resume
> > > > +11:
> > > > +#endif
> > > 
> > > 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.

> > 
> > > > +#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?
> 
> fsl_booke_deep_sleep_resume
> 
> > > > +#define FSLDELAY(count)                \
> > > > +       li      r3, (count)@l;  \
> > > > +       slwi    r3, r3, 10;     \
> > > > +       mtctr   r3;             \
> > > > +101:   nop;                    \
> > > > +       bdnz    101b;
> > > 
> > > 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).

> 
> > > 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.

-Chenhui

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to