From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754743AbbFSKfM (ORCPT ); Fri, 19 Jun 2015 06:35:12 -0400 Received: from down.free-electrons.com ([37.187.137.238]:33035 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750868AbbFSKfE (ORCPT ); Fri, 19 Jun 2015 06:35:04 -0400 Date: Fri, 19 Jun 2015 12:30:08 +0200 From: Maxime Ripard To: Viresh Kumar Cc: Thomas Gleixner , Daniel Lezcano , linaro-kernel@lists.linaro.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 23/41] clocksource: sun4i: Migrate to new 'set-state' interface Message-ID: <20150619103008.GB11732@lukather> References: <20150618120105.GM11732@lukather> <20150618122336.GB1982@linux> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="2IUIi54rL+LZyXu+" Content-Disposition: inline In-Reply-To: <20150618122336.GB1982@linux> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --2IUIi54rL+LZyXu+ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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; >=20 > 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. >=20 > > > + .tick_resume =3D sun4i_clkevt_shutdown, > >=20 > > 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? >=20 > And so this patch carried the same logic here. >=20 > 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. >=20 > Many driver authors didn't knew about these details and so did > shutdown in resume path as well. >=20 > 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 --=20 Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com --2IUIi54rL+LZyXu+ Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJVg+8wAAoJEBx+YmzsjxAgdKoQALiIVjUcfV5bSQDpBtvXrCo7 miRpvhVX1Si4fAHe7umVgrHykAzIXRkxkffe+Y+PviAbfkrLw7LT2bpe67Hd8MxL zt05wyv8RpxVi/Vc40BAC1tcdxWQNWxBc0RmAaYf7V9zE5UghTRudBIA7FUydP86 XhztghdwSrsCEPuL80JPPxkKEcbMYlm2xwXRs6encUbSnKQ7SzRg3IPASA+GF67c 7U68TU554cCqcEVMc2pLBeoHDhfBO4r7BORLYeOWe6qvvzD6vcllD6VAlyzSQh0K 3Nzz2k5FAFsFkp0O8Zii/vdHPo2UxS0pMe4ur8sZA0afXDgLOHksDwjRG8OxdqDl RU0/Kfzl1YSsgstYtlMeQDPwK1Nra49mHyX7j0KG+PzK0p1ZX7TxqgAumvunVxiD N88K+IHDfidIni0xd0uId0WTAnqR51PcdVtHaLUDhEbt1TipacXe+uS4QKfn3PmD Y1T2f+OHuFc7VQC82UD4XuWiD+xiaNAlDKfglHzceIEzkCQtPmFDAPxPI857yMMM Ci2jBUKa7PLerdtUpIaise6YvNI+tEkjEp7wx5OFDy80wYak4yI463jm7E/QWaZE 6RIDJzJZG1V/0cu8VG46MVd8WFyx0Dwz3427sUyc+NVqUWfzHEMMY9ZYIBWtF9U1 Vpblhl2e6oRdsvuTSC6z =dFYz -----END PGP SIGNATURE----- --2IUIi54rL+LZyXu+-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: maxime.ripard@free-electrons.com (Maxime Ripard) Date: Fri, 19 Jun 2015 12:30:08 +0200 Subject: [PATCH 23/41] clocksource: sun4i: Migrate to new 'set-state' interface In-Reply-To: <20150618122336.GB1982@linux> References: <20150618120105.GM11732@lukather> <20150618122336.GB1982@linux> Message-ID: <20150619103008.GB11732@lukather> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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 -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: