RE: [PATCH 2/2] ARM: at91/pm: add ultra Low-power mode 1(ULP1) support

2015-10-22 Thread Yang, Wenyou 
Hi Alexandre,

Sorry for late answer.

> -Original Message-
> From: Alexandre Belloni [mailto:alexandre.bell...@free-electrons.com]
> Sent: 2015年10月15日 21:22
> To: Yang, Wenyou
> Cc: Ferre, Nicolas; Jean-Christophe Plagniol-Villard; Russell King; Stephen 
> Boyd;
> Michael Turquette; linux-arm-ker...@lists.infradead.org; linux-
> ker...@vger.kernel.org; linux-...@vger.kernel.org
> Subject: Re: [PATCH 2/2] ARM: at91/pm: add ultra Low-power mode 1(ULP1)
> support
> 
> Hello Wenyou,
> 
> On 15/10/2015 at 11:41:07 +0800, Wenyou Yang wrote :
> > +#define ULP0_MODE  0x00
> > +#define ULP1_MODE  0x11
> 
> Those are not used.
> 
> > +static void at91_config_ulp1_wkup_source(void)
> > +{
> > +   if (at91_pmc_read(AT91_PMC_VERSION) >= SAMA5D2_PMC_VERSION)
> {
> > +   at91_pmc_write(AT91_PMC_FSMR, AT91_PMC_RTCAL |
> > + AT91_PMC_FSTT9 |
> > + AT91_PMC_FSTT8 |
> > + AT91_PMC_FSTT7 |
> > + AT91_PMC_FSTT6 |
> > + AT91_PMC_FSTT5 |
> > + AT91_PMC_FSTT4 |
> > + AT91_PMC_FSTT3 |
> > + AT91_PMC_FSTT2 |
> > + AT91_PMC_FSTT0);
> > +
> > +   at91_pmc_write(AT91_PMC_FSPR, 0);
> 
> Shouldn't those be configured from irq_set_wake() in the aic driver instead of
> activating all the wakeup sources?
> I think you need to get the PMC syscon from the aic5 driver and implement a 
> new
> irq_set_wake function when you can find the sam5d2 pmc.
I understand these code should not be here,  but I don't think those should be 
configured from irq_set_wake().
As said in datasheet, those configuration is to trigger a fast restart signal 
to the PMC. I think they should not be related to irq. 
 
> 
> The PMC syscon is introduced with that series:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-October/376493.html
> 
> > @@ -140,6 +165,10 @@ static void at91_pm_suspend(suspend_state_t state)
> > pm_data |= (state == PM_SUSPEND_MEM) ?
> > AT91_PM_MODE(AT91_PM_SLOW_CLOCK) : 0;
> >
> > +   pm_data |= ((state == PM_SUSPEND_MEM) &&
> > +   (at91_pmc_read(AT91_PMC_VERSION) >=
> SAMA5D2_PMC_VERSION)) ?
> > +   AT91_PM_ULP(AT91_PM_ULP1_MODE) : 0;
> > +
> 
> I would prefer not relying on the AT91_PMC_VERSION. Also, you probably
> shouldn't read the register each time you go to suspend.
> Finally, this could be better written by testing state only once.
Yes, it is not good.
> 
> 
> > flush_cache_all();
> > outer_disable();
> >
> > diff --git a/arch/arm/mach-at91/pm.h b/arch/arm/mach-at91/pm.h index
> > 3fcf881..2e76745 100644
> > --- a/arch/arm/mach-at91/pm.h
> > +++ b/arch/arm/mach-at91/pm.h
> > @@ -39,4 +39,11 @@ extern void __iomem *at91_ramc_base[];
> >
> >  #defineAT91_PM_SLOW_CLOCK  0x01
> >
> > +#define AT91_PM_ULP_OFFSET 5
> > +#define AT91_PM_ULP_MASK   0x03
> > +#define AT91_PM_ULP(x) (((x) & AT91_PM_ULP_MASK) <<
> AT91_PM_ULP_OFFSET)
> > +
> > +#define AT91_PM_ULP0_MODE  0x00
> > +#define AT91_PM_ULP1_MODE  0x01
> > +
> >  #endif
> > diff --git a/arch/arm/mach-at91/pm_suspend.S
> > b/arch/arm/mach-at91/pm_suspend.S index 825347b..543c430 100644
> > --- a/arch/arm/mach-at91/pm_suspend.S
> > +++ b/arch/arm/mach-at91/pm_suspend.S
> > @@ -41,6 +41,15 @@ tmp2 .reqr5
> > .endm
> >
> >  /*
> > + * Wait for main oscillator selection is done  */
> > +   .macro wait_moscsels
> > +1: ldr tmp1, [pmc, #AT91_PMC_SR]
> > +   tst tmp1, #AT91_PMC_MOSCSELS
> > +   beq 1b
> > +   .endm
> > +
> > +/*
> >   * Wait until PLLA has locked.
> >   */
> > .macro wait_pllalock
> > @@ -99,6 +108,10 @@ ENTRY(at91_pm_suspend_in_sram)
> > and r0, r0, #AT91_PM_MODE_MASK
> > str r0, .pm_mode
> >
> > +   lsr r0, r3, #AT91_PM_ULP_OFFSET
> > +   and r0, r0, #AT91_PM_ULP_MASK
> > +   str r0, .ulp_mode
> > +
> > /* Active the self-refresh mode */
> > mov r0, #SRAMC_SELF_FRESH_ACTIVE
> > bl  at91_sramc_self_refresh
> > @@ -107,6 +120,14 @@ ENTRY(at91_pm_suspend_in_sram)
> > tst r0, #AT91_PM_SLOW_CLOCK
> > beq standby_mode
&g

RE: [PATCH 2/2] ARM: at91/pm: add ultra Low-power mode 1(ULP1) support

2015-10-22 Thread Yang, Wenyou 
Hi Alexandre,

Sorry for late answer.

> -Original Message-
> From: Alexandre Belloni [mailto:alexandre.bell...@free-electrons.com]
> Sent: 2015年10月15日 21:22
> To: Yang, Wenyou
> Cc: Ferre, Nicolas; Jean-Christophe Plagniol-Villard; Russell King; Stephen 
> Boyd;
> Michael Turquette; linux-arm-ker...@lists.infradead.org; linux-
> ker...@vger.kernel.org; linux-...@vger.kernel.org
> Subject: Re: [PATCH 2/2] ARM: at91/pm: add ultra Low-power mode 1(ULP1)
> support
> 
> Hello Wenyou,
> 
> On 15/10/2015 at 11:41:07 +0800, Wenyou Yang wrote :
> > +#define ULP0_MODE  0x00
> > +#define ULP1_MODE  0x11
> 
> Those are not used.
> 
> > +static void at91_config_ulp1_wkup_source(void)
> > +{
> > +   if (at91_pmc_read(AT91_PMC_VERSION) >= SAMA5D2_PMC_VERSION)
> {
> > +   at91_pmc_write(AT91_PMC_FSMR, AT91_PMC_RTCAL |
> > + AT91_PMC_FSTT9 |
> > + AT91_PMC_FSTT8 |
> > + AT91_PMC_FSTT7 |
> > + AT91_PMC_FSTT6 |
> > + AT91_PMC_FSTT5 |
> > + AT91_PMC_FSTT4 |
> > + AT91_PMC_FSTT3 |
> > + AT91_PMC_FSTT2 |
> > + AT91_PMC_FSTT0);
> > +
> > +   at91_pmc_write(AT91_PMC_FSPR, 0);
> 
> Shouldn't those be configured from irq_set_wake() in the aic driver instead of
> activating all the wakeup sources?
> I think you need to get the PMC syscon from the aic5 driver and implement a 
> new
> irq_set_wake function when you can find the sam5d2 pmc.
I understand these code should not be here,  but I don't think those should be 
configured from irq_set_wake().
As said in datasheet, those configuration is to trigger a fast restart signal 
to the PMC. I think they should not be related to irq. 
 
> 
> The PMC syscon is introduced with that series:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-October/376493.html
> 
> > @@ -140,6 +165,10 @@ static void at91_pm_suspend(suspend_state_t state)
> > pm_data |= (state == PM_SUSPEND_MEM) ?
> > AT91_PM_MODE(AT91_PM_SLOW_CLOCK) : 0;
> >
> > +   pm_data |= ((state == PM_SUSPEND_MEM) &&
> > +   (at91_pmc_read(AT91_PMC_VERSION) >=
> SAMA5D2_PMC_VERSION)) ?
> > +   AT91_PM_ULP(AT91_PM_ULP1_MODE) : 0;
> > +
> 
> I would prefer not relying on the AT91_PMC_VERSION. Also, you probably
> shouldn't read the register each time you go to suspend.
> Finally, this could be better written by testing state only once.
Yes, it is not good.
> 
> 
> > flush_cache_all();
> > outer_disable();
> >
> > diff --git a/arch/arm/mach-at91/pm.h b/arch/arm/mach-at91/pm.h index
> > 3fcf881..2e76745 100644
> > --- a/arch/arm/mach-at91/pm.h
> > +++ b/arch/arm/mach-at91/pm.h
> > @@ -39,4 +39,11 @@ extern void __iomem *at91_ramc_base[];
> >
> >  #defineAT91_PM_SLOW_CLOCK  0x01
> >
> > +#define AT91_PM_ULP_OFFSET 5
> > +#define AT91_PM_ULP_MASK   0x03
> > +#define AT91_PM_ULP(x) (((x) & AT91_PM_ULP_MASK) <<
> AT91_PM_ULP_OFFSET)
> > +
> > +#define AT91_PM_ULP0_MODE  0x00
> > +#define AT91_PM_ULP1_MODE  0x01
> > +
> >  #endif
> > diff --git a/arch/arm/mach-at91/pm_suspend.S
> > b/arch/arm/mach-at91/pm_suspend.S index 825347b..543c430 100644
> > --- a/arch/arm/mach-at91/pm_suspend.S
> > +++ b/arch/arm/mach-at91/pm_suspend.S
> > @@ -41,6 +41,15 @@ tmp2 .reqr5
> > .endm
> >
> >  /*
> > + * Wait for main oscillator selection is done  */
> > +   .macro wait_moscsels
> > +1: ldr tmp1, [pmc, #AT91_PMC_SR]
> > +   tst tmp1, #AT91_PMC_MOSCSELS
> > +   beq 1b
> > +   .endm
> > +
> > +/*
> >   * Wait until PLLA has locked.
> >   */
> > .macro wait_pllalock
> > @@ -99,6 +108,10 @@ ENTRY(at91_pm_suspend_in_sram)
> > and r0, r0, #AT91_PM_MODE_MASK
> > str r0, .pm_mode
> >
> > +   lsr r0, r3, #AT91_PM_ULP_OFFSET
> > +   and r0, r0, #AT91_PM_ULP_MASK
> > +   str r0, .ulp_mode
> > +
> > /* Active the self-refresh mode */
> > mov r0, #SRAMC_SELF_FRESH_ACTIVE
> > bl  at91_sramc_self_refresh
> > @@ -107,6 +120,14 @@ ENTRY(at91_pm_suspend_in_sram)
> > tst r0, #AT91_PM_SLOW_CLOCK
> > beq standby_mode
&g

Re: [PATCH 2/2] ARM: at91/pm: add ultra Low-power mode 1(ULP1) support

2015-10-15 Thread Alexandre Belloni 
Hello Wenyou,

On 15/10/2015 at 11:41:07 +0800, Wenyou Yang wrote :
> +#define ULP0_MODE  0x00
> +#define ULP1_MODE  0x11

Those are not used.

> +static void at91_config_ulp1_wkup_source(void)
> +{
> + if (at91_pmc_read(AT91_PMC_VERSION) >= SAMA5D2_PMC_VERSION) {
> + at91_pmc_write(AT91_PMC_FSMR, AT91_PMC_RTCAL |
> +   AT91_PMC_FSTT9 |
> +   AT91_PMC_FSTT8 |
> +   AT91_PMC_FSTT7 |
> +   AT91_PMC_FSTT6 |
> +   AT91_PMC_FSTT5 |
> +   AT91_PMC_FSTT4 |
> +   AT91_PMC_FSTT3 |
> +   AT91_PMC_FSTT2 |
> +   AT91_PMC_FSTT0);
> +
> + at91_pmc_write(AT91_PMC_FSPR, 0);

Shouldn't those be configured from irq_set_wake() in the aic driver
instead of activating all the wakeup sources?
I think you need to get the PMC syscon from the aic5 driver and
implement a new irq_set_wake function when you can find the sam5d2 pmc.

The PMC syscon is introduced with that series:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-October/376493.html

> @@ -140,6 +165,10 @@ static void at91_pm_suspend(suspend_state_t state)
>   pm_data |= (state == PM_SUSPEND_MEM) ?
>   AT91_PM_MODE(AT91_PM_SLOW_CLOCK) : 0;
>  
> + pm_data |= ((state == PM_SUSPEND_MEM) &&
> + (at91_pmc_read(AT91_PMC_VERSION) >= SAMA5D2_PMC_VERSION)) ?
> + AT91_PM_ULP(AT91_PM_ULP1_MODE) : 0;
> +

I would prefer not relying on the AT91_PMC_VERSION. Also, you probably
shouldn't read the register each time you go to suspend.
Finally, this could be better written by testing state only once.


>   flush_cache_all();
>   outer_disable();
>  
> diff --git a/arch/arm/mach-at91/pm.h b/arch/arm/mach-at91/pm.h
> index 3fcf881..2e76745 100644
> --- a/arch/arm/mach-at91/pm.h
> +++ b/arch/arm/mach-at91/pm.h
> @@ -39,4 +39,11 @@ extern void __iomem *at91_ramc_base[];
>  
>  #define  AT91_PM_SLOW_CLOCK  0x01
>  
> +#define AT91_PM_ULP_OFFSET   5
> +#define AT91_PM_ULP_MASK 0x03
> +#define AT91_PM_ULP(x)   (((x) & AT91_PM_ULP_MASK) << 
> AT91_PM_ULP_OFFSET)
> +
> +#define AT91_PM_ULP0_MODE0x00
> +#define AT91_PM_ULP1_MODE0x01
> +
>  #endif
> diff --git a/arch/arm/mach-at91/pm_suspend.S b/arch/arm/mach-at91/pm_suspend.S
> index 825347b..543c430 100644
> --- a/arch/arm/mach-at91/pm_suspend.S
> +++ b/arch/arm/mach-at91/pm_suspend.S
> @@ -41,6 +41,15 @@ tmp2   .reqr5
>   .endm
>  
>  /*
> + * Wait for main oscillator selection is done
> + */
> + .macro wait_moscsels
> +1:   ldr tmp1, [pmc, #AT91_PMC_SR]
> + tst tmp1, #AT91_PMC_MOSCSELS
> + beq 1b
> + .endm
> +
> +/*
>   * Wait until PLLA has locked.
>   */
>   .macro wait_pllalock
> @@ -99,6 +108,10 @@ ENTRY(at91_pm_suspend_in_sram)
>   and r0, r0, #AT91_PM_MODE_MASK
>   str r0, .pm_mode
>  
> + lsr r0, r3, #AT91_PM_ULP_OFFSET
> + and r0, r0, #AT91_PM_ULP_MASK
> + str r0, .ulp_mode
> +
>   /* Active the self-refresh mode */
>   mov r0, #SRAMC_SELF_FRESH_ACTIVE
>   bl  at91_sramc_self_refresh
> @@ -107,6 +120,14 @@ ENTRY(at91_pm_suspend_in_sram)
>   tst r0, #AT91_PM_SLOW_CLOCK
>   beq standby_mode
>  
> + ldr r0, .ulp_mode
> + tst r0, #AT91_PM_ULP1_MODE
> + beq ulp0_mode
> +
> +ulp1_mode:
> + bl  at91_pm_ulp1_mode
> + b   pm_exit
> +
>  ulp0_mode:
>   bl  at91_pm_ulp0_mode
>   b   pm_exit
> @@ -313,6 +334,94 @@ ENTRY(at91_pm_ulp0_mode)
>   mov pc, lr
>  ENDPROC(at91_pm_ulp0_mode)
>  
> +/*
> + * void at91_pm_ulp1_mode(void)
> + *
> + */
> +ENTRY(at91_pm_ulp1_mode)
> + ldr pmc, .pmc_base
> +
> + /* Save PMC_MCKR config */
> + ldr tmp1, [pmc, #AT91_PMC_MCKR]
> + str tmp1, .saved_mckr
> +
> + /* Switch the master clock source to main clock */
> + bic tmp1, tmp1, #AT91_PMC_CSS
> + orr tmp1, tmp1, #AT91_PMC_CSS_MAIN
> + str tmp1, [pmc, #AT91_PMC_MCKR]
> +
> + wait_mckrdy
> +
> + /* Save PLLA config, then and disable PLLA */
> + ldr tmp1, [pmc, #AT91_CKGR_PLLAR]
> + str tmp1, .saved_pllar
> +
> + bic tmp1, tmp1, #AT91_PMC3_MUL
> + str tmp1, [pmc, #AT91_CKGR_PLLAR]
> +
> + /* Switch main clock to 12-MHz RC oscillator */
> + ldr tmp1, [pmc, #AT91_CKGR_MOR]
> + bic tmp1, tmp1, #AT91_PMC_MOSCSEL
> + bic tmp1, tmp1, #AT91_PMC_KEY_MASK
> + orr tmp1, tmp1, #AT91_PMC_KEY
> + bic tmp1, tmp1, #(7 << 4)
> + str tmp1, [pmc, #AT91_CKGR_MOR]
> +
> + wait_moscsels
> +
> + /* Disable the main oscillator */
> + ldr

Re: [PATCH 2/2] ARM: at91/pm: add ultra Low-power mode 1(ULP1) support

2015-10-15 Thread Michael Turquette 
Quoting Wenyou Yang (2015-10-14 20:41:07)
> The Ultra Low-power mode 1(ULP1) is introduced by SAMA5D2.
> 
> In the ULP1 mode, all the clocks are shut off, inclusive the embedded
> 12MHz RC oscillator, so as to achieve the lowest power consumption
> with the system in retention mode and able to resume on the wake up
> events. As soon as the wake up event is asserted, the embedded 12MHz
> RC oscillator restarts automatically.
> 
> The number of wake up sources for the ULP1 mode is limited, the wake
> up sources should be configured via the PMC_FSMR and PMC_FSPR
> registers.
> 
> In this patch, the following wake up sources are enabled,
>  - WKUP0 pin
>  - WKUP1 pin to WKUP8 pin (shared with PIOBU0 to PIOBU7)
>  - RTC alarm
> 
> Signed-off-by: Wenyou Yang 

For the changes to the clk header:

Acked-by: Michael Turquette 

> ---
> 
>  arch/arm/mach-at91/pm.c |   29 ++
>  arch/arm/mach-at91/pm.h |7 +++
>  arch/arm/mach-at91/pm_suspend.S |  111 
> +++
>  include/linux/clk/at91_pmc.h|   36 +
>  4 files changed, 183 insertions(+)
> 
> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> index 80e277c..49443d9 100644
> --- a/arch/arm/mach-at91/pm.c
> +++ b/arch/arm/mach-at91/pm.c
> @@ -35,6 +35,11 @@
>  #include "generic.h"
>  #include "pm.h"
>  
> +#define ULP0_MODE  0x00
> +#define ULP1_MODE  0x11
> +
> +#define SAMA5D2_PMC_VERSION0x20540
> +
>  /*
>   * FIXME: this is needed to communicate between the pinctrl driver and
>   * the PM implementation in the machine. Possibly part of the PM
> @@ -64,6 +69,23 @@ static int at91_pm_valid_state(suspend_state_t state)
> }
>  }
>  
> +static void at91_config_ulp1_wkup_source(void)
> +{
> +   if (at91_pmc_read(AT91_PMC_VERSION) >= SAMA5D2_PMC_VERSION) {
> +   at91_pmc_write(AT91_PMC_FSMR, AT91_PMC_RTCAL |
> + AT91_PMC_FSTT9 |
> + AT91_PMC_FSTT8 |
> + AT91_PMC_FSTT7 |
> + AT91_PMC_FSTT6 |
> + AT91_PMC_FSTT5 |
> + AT91_PMC_FSTT4 |
> + AT91_PMC_FSTT3 |
> + AT91_PMC_FSTT2 |
> + AT91_PMC_FSTT0);
> +
> +   at91_pmc_write(AT91_PMC_FSPR, 0);
> +   }
> +}
>  
>  static suspend_state_t target_state;
>  
> @@ -73,6 +95,9 @@ static suspend_state_t target_state;
>  static int at91_pm_begin(suspend_state_t state)
>  {
> target_state = state;
> +
> +   at91_config_ulp1_wkup_source();
> +
> return 0;
>  }
>  
> @@ -140,6 +165,10 @@ static void at91_pm_suspend(suspend_state_t state)
> pm_data |= (state == PM_SUSPEND_MEM) ?
> AT91_PM_MODE(AT91_PM_SLOW_CLOCK) : 0;
>  
> +   pm_data |= ((state == PM_SUSPEND_MEM) &&
> +   (at91_pmc_read(AT91_PMC_VERSION) >= SAMA5D2_PMC_VERSION)) 
> ?
> +   AT91_PM_ULP(AT91_PM_ULP1_MODE) : 0;
> +
> flush_cache_all();
> outer_disable();
>  
> diff --git a/arch/arm/mach-at91/pm.h b/arch/arm/mach-at91/pm.h
> index 3fcf881..2e76745 100644
> --- a/arch/arm/mach-at91/pm.h
> +++ b/arch/arm/mach-at91/pm.h
> @@ -39,4 +39,11 @@ extern void __iomem *at91_ramc_base[];
>  
>  #defineAT91_PM_SLOW_CLOCK  0x01
>  
> +#define AT91_PM_ULP_OFFSET 5
> +#define AT91_PM_ULP_MASK   0x03
> +#define AT91_PM_ULP(x) (((x) & AT91_PM_ULP_MASK) << 
> AT91_PM_ULP_OFFSET)
> +
> +#define AT91_PM_ULP0_MODE  0x00
> +#define AT91_PM_ULP1_MODE  0x01
> +
>  #endif
> diff --git a/arch/arm/mach-at91/pm_suspend.S b/arch/arm/mach-at91/pm_suspend.S
> index 825347b..543c430 100644
> --- a/arch/arm/mach-at91/pm_suspend.S
> +++ b/arch/arm/mach-at91/pm_suspend.S
> @@ -41,6 +41,15 @@ tmp2 .reqr5
> .endm
>  
>  /*
> + * Wait for main oscillator selection is done
> + */
> +   .macro wait_moscsels
> +1: ldr tmp1, [pmc, #AT91_PMC_SR]
> +   tst tmp1, #AT91_PMC_MOSCSELS
> +   beq 1b
> +   .endm
> +
> +/*
>   * Wait until PLLA has locked.
>   */
> .macro wait_pllalock
> @@ -99,6 +108,10 @@ ENTRY(at91_pm_suspend_in_sram)
> and r0, r0, #AT91_PM_MODE_MASK
> str r0, .pm_mode
>  
> +   lsr r0, r3, #AT91_PM_ULP_OFFSET
> +   and r0, r0, #AT91_PM_ULP_MASK
> +   str r0, .ulp_mode
> +
> /* Active the self-refresh mode */
> mov r0, #SRAMC_SELF_FRESH_ACTIVE
> bl  at91_sramc_self_refresh
> @@ -107,6 +120,14 @@ ENTRY(at91_pm_suspend_in_sram)
> tst r0, #AT91_PM_SLOW_CLOCK
> beq standby_mode
>  
> +   ldr r0, .ulp_mode
> +   tst r0, #AT91_PM_ULP1_MODE
> +   beq ulp0_mode
> +
>

Re: [PATCH 2/2] ARM: at91/pm: add ultra Low-power mode 1(ULP1) support

2015-10-15 Thread Michael Turquette 
Quoting Wenyou Yang (2015-10-14 20:41:07)
> The Ultra Low-power mode 1(ULP1) is introduced by SAMA5D2.
> 
> In the ULP1 mode, all the clocks are shut off, inclusive the embedded
> 12MHz RC oscillator, so as to achieve the lowest power consumption
> with the system in retention mode and able to resume on the wake up
> events. As soon as the wake up event is asserted, the embedded 12MHz
> RC oscillator restarts automatically.
> 
> The number of wake up sources for the ULP1 mode is limited, the wake
> up sources should be configured via the PMC_FSMR and PMC_FSPR
> registers.
> 
> In this patch, the following wake up sources are enabled,
>  - WKUP0 pin
>  - WKUP1 pin to WKUP8 pin (shared with PIOBU0 to PIOBU7)
>  - RTC alarm
> 
> Signed-off-by: Wenyou Yang 

For the changes to the clk header:

Acked-by: Michael Turquette 

> ---
> 
>  arch/arm/mach-at91/pm.c |   29 ++
>  arch/arm/mach-at91/pm.h |7 +++
>  arch/arm/mach-at91/pm_suspend.S |  111 
> +++
>  include/linux/clk/at91_pmc.h|   36 +
>  4 files changed, 183 insertions(+)
> 
> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> index 80e277c..49443d9 100644
> --- a/arch/arm/mach-at91/pm.c
> +++ b/arch/arm/mach-at91/pm.c
> @@ -35,6 +35,11 @@
>  #include "generic.h"
>  #include "pm.h"
>  
> +#define ULP0_MODE  0x00
> +#define ULP1_MODE  0x11
> +
> +#define SAMA5D2_PMC_VERSION0x20540
> +
>  /*
>   * FIXME: this is needed to communicate between the pinctrl driver and
>   * the PM implementation in the machine. Possibly part of the PM
> @@ -64,6 +69,23 @@ static int at91_pm_valid_state(suspend_state_t state)
> }
>  }
>  
> +static void at91_config_ulp1_wkup_source(void)
> +{
> +   if (at91_pmc_read(AT91_PMC_VERSION) >= SAMA5D2_PMC_VERSION) {
> +   at91_pmc_write(AT91_PMC_FSMR, AT91_PMC_RTCAL |
> + AT91_PMC_FSTT9 |
> + AT91_PMC_FSTT8 |
> + AT91_PMC_FSTT7 |
> + AT91_PMC_FSTT6 |
> + AT91_PMC_FSTT5 |
> + AT91_PMC_FSTT4 |
> + AT91_PMC_FSTT3 |
> + AT91_PMC_FSTT2 |
> + AT91_PMC_FSTT0);
> +
> +   at91_pmc_write(AT91_PMC_FSPR, 0);
> +   }
> +}
>  
>  static suspend_state_t target_state;
>  
> @@ -73,6 +95,9 @@ static suspend_state_t target_state;
>  static int at91_pm_begin(suspend_state_t state)
>  {
> target_state = state;
> +
> +   at91_config_ulp1_wkup_source();
> +
> return 0;
>  }
>  
> @@ -140,6 +165,10 @@ static void at91_pm_suspend(suspend_state_t state)
> pm_data |= (state == PM_SUSPEND_MEM) ?
> AT91_PM_MODE(AT91_PM_SLOW_CLOCK) : 0;
>  
> +   pm_data |= ((state == PM_SUSPEND_MEM) &&
> +   (at91_pmc_read(AT91_PMC_VERSION) >= SAMA5D2_PMC_VERSION)) 
> ?
> +   AT91_PM_ULP(AT91_PM_ULP1_MODE) : 0;
> +
> flush_cache_all();
> outer_disable();
>  
> diff --git a/arch/arm/mach-at91/pm.h b/arch/arm/mach-at91/pm.h
> index 3fcf881..2e76745 100644
> --- a/arch/arm/mach-at91/pm.h
> +++ b/arch/arm/mach-at91/pm.h
> @@ -39,4 +39,11 @@ extern void __iomem *at91_ramc_base[];
>  
>  #defineAT91_PM_SLOW_CLOCK  0x01
>  
> +#define AT91_PM_ULP_OFFSET 5
> +#define AT91_PM_ULP_MASK   0x03
> +#define AT91_PM_ULP(x) (((x) & AT91_PM_ULP_MASK) << 
> AT91_PM_ULP_OFFSET)
> +
> +#define AT91_PM_ULP0_MODE  0x00
> +#define AT91_PM_ULP1_MODE  0x01
> +
>  #endif
> diff --git a/arch/arm/mach-at91/pm_suspend.S b/arch/arm/mach-at91/pm_suspend.S
> index 825347b..543c430 100644
> --- a/arch/arm/mach-at91/pm_suspend.S
> +++ b/arch/arm/mach-at91/pm_suspend.S
> @@ -41,6 +41,15 @@ tmp2 .reqr5
> .endm
>  
>  /*
> + * Wait for main oscillator selection is done
> + */
> +   .macro wait_moscsels
> +1: ldr tmp1, [pmc, #AT91_PMC_SR]
> +   tst tmp1, #AT91_PMC_MOSCSELS
> +   beq 1b
> +   .endm
> +
> +/*
>   * Wait until PLLA has locked.
>   */
> .macro wait_pllalock
> @@ -99,6 +108,10 @@ ENTRY(at91_pm_suspend_in_sram)
> and r0, r0, #AT91_PM_MODE_MASK
> str r0, .pm_mode
>  
> +   lsr r0, r3, #AT91_PM_ULP_OFFSET
> +   and r0, r0, #AT91_PM_ULP_MASK
> +   str r0, .ulp_mode
> +
> /* Active the self-refresh mode */
> mov r0, #SRAMC_SELF_FRESH_ACTIVE
> bl  at91_sramc_self_refresh
> @@ -107,6 +120,14 @@ ENTRY(at91_pm_suspend_in_sram)
> tst r0, #AT91_PM_SLOW_CLOCK
> beq standby_mode
>  
> +   ldr r0, .ulp_mode
> +   tst r0,

Re: [PATCH 2/2] ARM: at91/pm: add ultra Low-power mode 1(ULP1) support

2015-10-15 Thread Alexandre Belloni 
Hello Wenyou,

On 15/10/2015 at 11:41:07 +0800, Wenyou Yang wrote :
> +#define ULP0_MODE  0x00
> +#define ULP1_MODE  0x11

Those are not used.

> +static void at91_config_ulp1_wkup_source(void)
> +{
> + if (at91_pmc_read(AT91_PMC_VERSION) >= SAMA5D2_PMC_VERSION) {
> + at91_pmc_write(AT91_PMC_FSMR, AT91_PMC_RTCAL |
> +   AT91_PMC_FSTT9 |
> +   AT91_PMC_FSTT8 |
> +   AT91_PMC_FSTT7 |
> +   AT91_PMC_FSTT6 |
> +   AT91_PMC_FSTT5 |
> +   AT91_PMC_FSTT4 |
> +   AT91_PMC_FSTT3 |
> +   AT91_PMC_FSTT2 |
> +   AT91_PMC_FSTT0);
> +
> + at91_pmc_write(AT91_PMC_FSPR, 0);

Shouldn't those be configured from irq_set_wake() in the aic driver
instead of activating all the wakeup sources?
I think you need to get the PMC syscon from the aic5 driver and
implement a new irq_set_wake function when you can find the sam5d2 pmc.

The PMC syscon is introduced with that series:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-October/376493.html

> @@ -140,6 +165,10 @@ static void at91_pm_suspend(suspend_state_t state)
>   pm_data |= (state == PM_SUSPEND_MEM) ?
>   AT91_PM_MODE(AT91_PM_SLOW_CLOCK) : 0;
>  
> + pm_data |= ((state == PM_SUSPEND_MEM) &&
> + (at91_pmc_read(AT91_PMC_VERSION) >= SAMA5D2_PMC_VERSION)) ?
> + AT91_PM_ULP(AT91_PM_ULP1_MODE) : 0;
> +

I would prefer not relying on the AT91_PMC_VERSION. Also, you probably
shouldn't read the register each time you go to suspend.
Finally, this could be better written by testing state only once.


>   flush_cache_all();
>   outer_disable();
>  
> diff --git a/arch/arm/mach-at91/pm.h b/arch/arm/mach-at91/pm.h
> index 3fcf881..2e76745 100644
> --- a/arch/arm/mach-at91/pm.h
> +++ b/arch/arm/mach-at91/pm.h
> @@ -39,4 +39,11 @@ extern void __iomem *at91_ramc_base[];
>  
>  #define  AT91_PM_SLOW_CLOCK  0x01
>  
> +#define AT91_PM_ULP_OFFSET   5
> +#define AT91_PM_ULP_MASK 0x03
> +#define AT91_PM_ULP(x)   (((x) & AT91_PM_ULP_MASK) << 
> AT91_PM_ULP_OFFSET)
> +
> +#define AT91_PM_ULP0_MODE0x00
> +#define AT91_PM_ULP1_MODE0x01
> +
>  #endif
> diff --git a/arch/arm/mach-at91/pm_suspend.S b/arch/arm/mach-at91/pm_suspend.S
> index 825347b..543c430 100644
> --- a/arch/arm/mach-at91/pm_suspend.S
> +++ b/arch/arm/mach-at91/pm_suspend.S
> @@ -41,6 +41,15 @@ tmp2   .reqr5
>   .endm
>  
>  /*
> + * Wait for main oscillator selection is done
> + */
> + .macro wait_moscsels
> +1:   ldr tmp1, [pmc, #AT91_PMC_SR]
> + tst tmp1, #AT91_PMC_MOSCSELS
> + beq 1b
> + .endm
> +
> +/*
>   * Wait until PLLA has locked.
>   */
>   .macro wait_pllalock
> @@ -99,6 +108,10 @@ ENTRY(at91_pm_suspend_in_sram)
>   and r0, r0, #AT91_PM_MODE_MASK
>   str r0, .pm_mode
>  
> + lsr r0, r3, #AT91_PM_ULP_OFFSET
> + and r0, r0, #AT91_PM_ULP_MASK
> + str r0, .ulp_mode
> +
>   /* Active the self-refresh mode */
>   mov r0, #SRAMC_SELF_FRESH_ACTIVE
>   bl  at91_sramc_self_refresh
> @@ -107,6 +120,14 @@ ENTRY(at91_pm_suspend_in_sram)
>   tst r0, #AT91_PM_SLOW_CLOCK
>   beq standby_mode
>  
> + ldr r0, .ulp_mode
> + tst r0, #AT91_PM_ULP1_MODE
> + beq ulp0_mode
> +
> +ulp1_mode:
> + bl  at91_pm_ulp1_mode
> + b   pm_exit
> +
>  ulp0_mode:
>   bl  at91_pm_ulp0_mode
>   b   pm_exit
> @@ -313,6 +334,94 @@ ENTRY(at91_pm_ulp0_mode)
>   mov pc, lr
>  ENDPROC(at91_pm_ulp0_mode)
>  
> +/*
> + * void at91_pm_ulp1_mode(void)
> + *
> + */
> +ENTRY(at91_pm_ulp1_mode)
> + ldr pmc, .pmc_base
> +
> + /* Save PMC_MCKR config */
> + ldr tmp1, [pmc, #AT91_PMC_MCKR]
> + str tmp1, .saved_mckr
> +
> + /* Switch the master clock source to main clock */
> + bic tmp1, tmp1, #AT91_PMC_CSS
> + orr tmp1, tmp1, #AT91_PMC_CSS_MAIN
> + str tmp1, [pmc, #AT91_PMC_MCKR]
> +
> + wait_mckrdy
> +
> + /* Save PLLA config, then and disable PLLA */
> + ldr tmp1, [pmc, #AT91_CKGR_PLLAR]
> + str tmp1, .saved_pllar
> +
> + bic tmp1, tmp1, #AT91_PMC3_MUL
> + str tmp1, [pmc, #AT91_CKGR_PLLAR]
> +
> + /* Switch main clock to 12-MHz RC oscillator */
> + ldr tmp1, [pmc, #AT91_CKGR_MOR]
> + bic tmp1, tmp1, #AT91_PMC_MOSCSEL
> + bic tmp1, tmp1, #AT91_PMC_KEY_MASK
> + orr tmp1, tmp1, #AT91_PMC_KEY
> + bic tmp1, tmp1, #(7 << 4)
> + str tmp1, [pmc, #AT91_CKGR_MOR]
> +
> + wait_moscsels
> +
> + /* Disable the main oscillator */
> + ldr