Re: [PATCH 3/4] powerpc: pm: add EPU FSM configuration for deep sleep

[Chenhui Zhao]

On Sat, Aug 1, 2015 at 8:41 AM, Scott Wood <scottwood@xxxxxxxxxxxxx> 
wrote:
> On Fri, 2015-07-31 at 20:53 +0800, Chenhui Zhao wrote:
> >  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: Chenhui Zhao <chenhui.zhao@xxxxxxxxxxxxx>
> >  ---
> >   arch/powerpc/platforms/85xx/Makefile    |   2 +-
> >   arch/powerpc/platforms/85xx/sleep_fsm.c | 256
> >  ++++++++++++++++++++++++++++++++
> >   arch/powerpc/platforms/85xx/sleep_fsm.h | 104 +++++++++++++
> >   3 files changed, 361 insertions(+), 1 deletion(-)
> >   create mode 100644 arch/powerpc/platforms/85xx/sleep_fsm.c
> >   create mode 100644 arch/powerpc/platforms/85xx/sleep_fsm.h
>
> When I asked why this was in drivers/platform[1], you said it was to 
> share
> with LS1, and that the values used were the same -- so why did you 
> move it to
> arch/powerpc?

There are some changes. LS1 will use PSCI (Power State Coordination 
Interface) to implement deep sleep. So these code just used by PowerPC.

> [1] Note that other proposed patches create a drivers/soc/fsl instead 
> of
> drivers/platform/fsl...  We need one of them, not both.
>
> >  +void fsl_fsm_setup(void __iomem *base, struct fsm_reg_vals *val)
> >  +{
> >  +     struct fsm_reg_vals *data = val;
> >  +
> >  +     BUG_ON(!base || !data);
>
> This BUG_ON is useless.  If one of those is NULL you'll get an oops 
> anyway.
>
>
> >  diff --git a/arch/powerpc/platforms/85xx/sleep_fsm.h
> >  b/arch/powerpc/platforms/85xx/sleep_fsm.h
> >  new file mode 100644
> >  index 0000000..2c60b40
> >  --- /dev/null
> >  +++ b/arch/powerpc/platforms/85xx/sleep_fsm.h
> >  @@ -0,0 +1,104 @@
> >  +/*
> >  + * Freescale deep sleep FSM (finite-state machine) configuration
> >  + *
> >  + * Copyright 2015 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 as 
> > published by the
> >  + * Free Software Foundation;  either version 2 of the  License, or 
> > (at your
> >  + * option) any later version.
> >  + */
> >  +#ifndef _FSL_SLEEP_FSM_H
> >  +#define _FSL_SLEEP_FSM_H
> >  +
> >  +#define FSL_STRIDE_4B        4
> >  +#define FSL_STRIDE_8B        8
>
> Why not just use 4/8 directly?
>
> >  +/* Block offsets */
> >  +#define RCPM_BLOCK_OFFSET    0x00022000
> >  +#define EPU_BLOCK_OFFSET     0x00000000
> >  +#define NPC_BLOCK_OFFSET     0x00001000
>
> I thought you said OK to not putting these offsets in the kernel 
> source...
>
> -Scott

OK. Will change them.

-Chenhui

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