From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755797AbbGEIpR (ORCPT ); Sun, 5 Jul 2015 04:45:17 -0400 Received: from smtp05.smtpout.orange.fr ([80.12.242.127]:19875 "EHLO smtp.smtpout.orange.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753334AbbGEIpI (ORCPT ); Sun, 5 Jul 2015 04:45:08 -0400 X-ME-Helo: belgarion X-ME-Auth: amFyem1pay5yb2JlcnRAb3JhbmdlLmZy X-ME-Date: Sat, 04 Jul 2015 17:45:02 +0200 X-ME-IP: 86.199.196.87 From: Robert Jarzmik To: Viresh Kumar Cc: Thomas Gleixner , Daniel Lezcano , linaro-kernel@lists.linaro.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Russell King Subject: Re: [PATCH 16/41] clocksource: pxa: Migrate to new 'set-state' interface References: <3aa5c1a740911258698769cbbb8f60ee02178848.1434622147.git.viresh.kumar@linaro.org> X-URL: http://belgarath.falguerolles.org/ Date: Sat, 04 Jul 2015 17:42:29 +0200 Message-ID: <87h9pk9bfe.fsf@belgarion.home> User-Agent: Gnus/5.130008 (Ma Gnus v0.8) Emacs/24.4 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Viresh Kumar writes: > @@ -88,26 +88,12 @@ pxa_osmr0_set_next_event(unsigned long delta, struct clock_event_device *dev) > return (signed)(next - oscr) <= MIN_OSCR_DELTA ? -ETIME : 0; > } > > -static void > -pxa_osmr0_set_mode(enum clock_event_mode mode, struct clock_event_device *dev) > +static int pxa_osmr0_shutdown(struct clock_event_device *evt) > { > - switch (mode) { > - case CLOCK_EVT_MODE_ONESHOT: > - timer_writel(timer_readl(OIER) & ~OIER_E0, OIER); > - timer_writel(OSSR_M0, OSSR); > - break; > - > - case CLOCK_EVT_MODE_UNUSED: > - case CLOCK_EVT_MODE_SHUTDOWN: > - /* initializing, released, or preparing for suspend */ > - timer_writel(timer_readl(OIER) & ~OIER_E0, OIER); > - timer_writel(OSSR_M0, OSSR); > - break; > - > - case CLOCK_EVT_MODE_RESUME: > - case CLOCK_EVT_MODE_PERIODIC: > - break; > - } > + /* initializing, released, or preparing for suspend */ > + timer_writel(timer_readl(OIER) & ~OIER_E0, OIER); > + timer_writel(OSSR_M0, OSSR); > + return 0; For consistency, please leave an empty line before that return statement. > @@ -147,13 +133,14 @@ static void pxa_timer_resume(struct clock_event_device *cedev) > #endif > > static struct clock_event_device ckevt_pxa_osmr0 = { > - .name = "osmr0", > - .features = CLOCK_EVT_FEAT_ONESHOT, > - .rating = 200, > - .set_next_event = pxa_osmr0_set_next_event, > - .set_mode = pxa_osmr0_set_mode, > - .suspend = pxa_timer_suspend, > - .resume = pxa_timer_resume, > + .name = "osmr0", > + .features = CLOCK_EVT_FEAT_ONESHOT, > + .rating = 200, > + .set_next_event = pxa_osmr0_set_next_event, > + .set_state_shutdown = pxa_osmr0_shutdown, > + .set_state_oneshot = pxa_osmr0_shutdown, A bit weird to have a "set_state_oneshot" function to point to a function called "X_shutdown". As I don't have a clear idea on what's this new interface for, I'll just hope it's the intended purpose. The code does look equivalent to me anyway. Apart from the cosmetic comment, once it is fixed : Acked-by: Robert Jarzmik Cheers. -- Robert From mboxrd@z Thu Jan 1 00:00:00 1970 From: robert.jarzmik@free.fr (Robert Jarzmik) Date: Sat, 04 Jul 2015 17:42:29 +0200 Subject: [PATCH 16/41] clocksource: pxa: Migrate to new 'set-state' interface References: <3aa5c1a740911258698769cbbb8f60ee02178848.1434622147.git.viresh.kumar@linaro.org> Message-ID: <87h9pk9bfe.fsf@belgarion.home> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Viresh Kumar writes: > @@ -88,26 +88,12 @@ pxa_osmr0_set_next_event(unsigned long delta, struct clock_event_device *dev) > return (signed)(next - oscr) <= MIN_OSCR_DELTA ? -ETIME : 0; > } > > -static void > -pxa_osmr0_set_mode(enum clock_event_mode mode, struct clock_event_device *dev) > +static int pxa_osmr0_shutdown(struct clock_event_device *evt) > { > - switch (mode) { > - case CLOCK_EVT_MODE_ONESHOT: > - timer_writel(timer_readl(OIER) & ~OIER_E0, OIER); > - timer_writel(OSSR_M0, OSSR); > - break; > - > - case CLOCK_EVT_MODE_UNUSED: > - case CLOCK_EVT_MODE_SHUTDOWN: > - /* initializing, released, or preparing for suspend */ > - timer_writel(timer_readl(OIER) & ~OIER_E0, OIER); > - timer_writel(OSSR_M0, OSSR); > - break; > - > - case CLOCK_EVT_MODE_RESUME: > - case CLOCK_EVT_MODE_PERIODIC: > - break; > - } > + /* initializing, released, or preparing for suspend */ > + timer_writel(timer_readl(OIER) & ~OIER_E0, OIER); > + timer_writel(OSSR_M0, OSSR); > + return 0; For consistency, please leave an empty line before that return statement. > @@ -147,13 +133,14 @@ static void pxa_timer_resume(struct clock_event_device *cedev) > #endif > > static struct clock_event_device ckevt_pxa_osmr0 = { > - .name = "osmr0", > - .features = CLOCK_EVT_FEAT_ONESHOT, > - .rating = 200, > - .set_next_event = pxa_osmr0_set_next_event, > - .set_mode = pxa_osmr0_set_mode, > - .suspend = pxa_timer_suspend, > - .resume = pxa_timer_resume, > + .name = "osmr0", > + .features = CLOCK_EVT_FEAT_ONESHOT, > + .rating = 200, > + .set_next_event = pxa_osmr0_set_next_event, > + .set_state_shutdown = pxa_osmr0_shutdown, > + .set_state_oneshot = pxa_osmr0_shutdown, A bit weird to have a "set_state_oneshot" function to point to a function called "X_shutdown". As I don't have a clear idea on what's this new interface for, I'll just hope it's the intended purpose. The code does look equivalent to me anyway. Apart from the cosmetic comment, once it is fixed : Acked-by: Robert Jarzmik Cheers. -- Robert