[PATCH 2/2] fsl/mpic_timer: make mpic_timer to support deep sleep feature

[Dongsheng.Wang at freescale.com]

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Wednesday, April 16, 2014 5:06 AM
> To: Wang Dongsheng-B40534
> Cc: Jin Zhengxiong-R64188; Li Yang-Leo-R58472; Zhao Chenhui-B35336; linuxppc-
> dev at lists.ozlabs.org
> Subject: Re: [PATCH 2/2] fsl/mpic_timer: make mpic_timer to support deep sleep
> feature
> 
> On Mon, 2014-04-14 at 22:23 -0500, Wang Dongsheng-B40534 wrote:
> >
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: Tuesday, April 15, 2014 7:36 AM
> > > To: Wang Dongsheng-B40534
> > > Cc: Jin Zhengxiong-R64188; Li Yang-Leo-R58472; Zhao Chenhui-B35336;
> linuxppc-
> > > dev at lists.ozlabs.org
> > > Subject: Re: [PATCH 2/2] fsl/mpic_timer: make mpic_timer to support deep
> sleep
> > > feature
> > >
> > > On Mon, 2014-04-14 at 10:24 +0800, Dongsheng Wang wrote:
> > > > From: Wang Dongsheng <dongsheng.wang at freescale.com>
> > > >
> > > > At T104x platfrom the timer clock will be changed when system going to
> > > > deep sleep.
> > >
> > > Could you elaborate on what is changing and why?
> > >
> >
> > Okay.
> >
> > > > +#include <asm/mpc85xx.h>
> > > >  #include <asm/mpic_timer.h>
> > >
> > > So much for, "The driver currently is only tested on fsl chip, but it can
> > > potentially support other global timers complying to OpenPIC standard."
> > >
> > > >  #define FSL_GLOBAL_TIMER		0x1
> > > > @@ -71,8 +74,10 @@ struct timer_group_priv {
> > > >  	struct timer_regs __iomem	*regs;
> > > >  	struct mpic_timer		timer[TIMERS_PER_GROUP];
> > > >  	struct list_head		node;
> > > > +	unsigned long			idle;
> > > >  	unsigned int			timerfreq;
> > > > -	unsigned int			idle;
> > >
> > > Why?
> > >
> >
> > Um... It shouldn't be happened...i will remove this.
> >
> > > > +	unsigned int			suspended_timerfreq;
> > > > +	unsigned int			resume_timerfreq;
> > > >  	unsigned int			flags;
> > > >  	spinlock_t			lock;
> > > >  	void __iomem			*group_tcr;
> > > > @@ -88,6 +93,7 @@ static struct cascade_priv cascade_timer[] = {  };
> > > >
> > > >  static LIST_HEAD(timer_group_list);
> > > > +static int switch_freq_flag;
> > >
> > > Needs documentation, and based on "_flag" it should probably be a bool.
> > >
> >
> > Okay.
> >
> > > >  static void convert_ticks_to_time(struct timer_group_priv *priv,
> > > >  		const u64 ticks, struct timeval *time) @@ -423,6 +429,33 @@
> struct
> > > > mpic_timer *mpic_request_timer(irq_handler_t fn, void *dev,  }
> > > > EXPORT_SYMBOL(mpic_request_timer);
> > > >
> > > > +static void timer_group_get_suspended_freq(struct timer_group_priv
> > > > +*priv) {
> > > > +	struct device_node *np;
> > > > +
> > > > +	np = of_find_compatible_node(NULL, NULL, "fsl,qoriq-clockgen-2.0");
> > > > +	if (!np) {
> > > > +		pr_err("mpic timer: Missing clockgen device node.\n");
> > >
> > > Why is it an error to not have a 2.0 QorIQ clockgen?
> > >
> >
> > This will affect the accuracy of the timer. But not means the timer cannot
> work.
> > Maybe you are right, this pr_err should be a pr_warn.
> 
> What I mean is, what if the mpic timer driver is used with deep sleep on
> a different chip such as mpc8536?
> 

Only T104x has this feature, other platform will not be effect.
I will remove this pr_err.

> > > > +		return;
> > > > +	}
> > > > +
> > > > +	of_property_read_u32(np, "clock-frequency", &priv-
> >suspended_timerfreq);
> > > > +	of_node_put(np);
> > >
> > > Shouldn't this go through the clock API?
> > >
> >
> > Sorry, I'm not clear about clock API, you mean fsl_get_sys_freq()? Or ?
> 
> No, I mean drivers/clk/
> 
> Though I suppose manually reading clock-frequency is OK, as the
> clock-frequency on the clockgen node predated the introduction of clock
> bindings to the device tree.
> 
> Don't use fsl_get_sys_freq().
> 

No, we cannot use drivers/clk/. Because clk-ppc-corenet.c only support corenet platform.

> > > > +static int need_to_switch_freq(void)
> > > > +{
> > > > +	u32 svr;
> > > > +
> > > > +	svr = mfspr(SPRN_SVR);
> > > > +	if (SVR_SOC_VER(svr) == SVR_T1040 ||
> > > > +			SVR_SOC_VER(svr) == SVR_T1042)
> > > > +		return 1;
> > >
> > > Explain why this is specific to T104x.
> > >
> >
> > Mpic timer freq will be change when system going to deep sleep. So we need to
> recalculate the time.
> 
> Do any other chips with deep sleep have this problem?
> 

Only has on T104x right now.

Regards,
-Dongsheng

> -Scott
>