linux-kernel - Re: [PATCH 23/41] clocksource: sun4i: Migrate to new 'set-state' interface

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20150619103008.GB11732@lukather>
Date:	Fri, 19 Jun 2015 12:30:08 +0200
From:	Maxime Ripard <maxime.ripard@...e-electrons.com>
To:	Viresh Kumar <viresh.kumar@...aro.org>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	Daniel Lezcano <daniel.lezcano@...aro.org>,
	linaro-kernel@...ts.linaro.org, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 23/41] clocksource: sun4i: Migrate to new 'set-state'
 interface

On Thu, Jun 18, 2015 at 05:53:36PM +0530, Viresh Kumar wrote:
> On 18-06-15, 14:01, Maxime Ripard wrote:
> > On Thu, Jun 18, 2015 at 04:24:37PM +0530, Viresh Kumar wrote:
> > > +static int sun4i_clkevt_shutdown(struct clock_event_device *evt)
> > >  {
> > > -	switch (mode) {
> > > -	case CLOCK_EVT_MODE_PERIODIC:
> > > -		sun4i_clkevt_time_stop(0);
> > > -		sun4i_clkevt_time_setup(0, ticks_per_jiffy);
> > > -		sun4i_clkevt_time_start(0, true);
> > > -		break;
> > > -	case CLOCK_EVT_MODE_ONESHOT:
> > > -		sun4i_clkevt_time_stop(0);
> > > -		sun4i_clkevt_time_start(0, false);
> > > -		break;
> > > -	case CLOCK_EVT_MODE_UNUSED:
> > > -	case CLOCK_EVT_MODE_SHUTDOWN:
> > > -	default:
> > > -		sun4i_clkevt_time_stop(0);
> > > -		break;
> 
> Because sun4i_clkevt_time_stop() is getting called in default case, it
> is getting called for tick-resume mode already with the old set_mode
> interface.
> 
> > > +	.tick_resume = sun4i_clkevt_shutdown,
> > 
> > I'm not exactly sure of the context here, but I wouldn't expect a
> > callback called tick_resume to stop a timer. Is this expected?
> 
> And so this patch carried the same logic here.
> 
> At suspend: clockevents core calls ->set_state_shutdown() and at
> resume it calls ->tick_resume() followed by setting to the proper
> mode, i.e. periodic or oneshot.
> 
> Many driver authors didn't knew about these details and so did
> shutdown in resume path as well.
> 
> For me, you might not even need a tick_resume() at all, as your driver
> would have already shutted down on suspend and is just required to be
> put to the right mode again.

Ok, thanks for the explanation.

It looks good for both this patch and the sun5i one. You can add my
Acked-by.

> But, I didn't wanted to change the way things behaved until now. I can
> add another patch to get things fixed separately if you want me to.

If you have some spare time, it would be great :)

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)