linux-kernel - Re: [PATCH 16/41] clocksource: pxa: 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <87h9pk9bfe.fsf@belgarion.home>
Date:	Sat, 04 Jul 2015 17:42:29 +0200
From:	Robert Jarzmik <robert.jarzmik@...e.fr>
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,
	Russell King <linux@....linux.org.uk>
Subject: Re: [PATCH 16/41] clocksource: pxa: Migrate to new 'set-state' interface

Viresh Kumar <viresh.kumar@...aro.org> 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 <robert.jarzmik@...e.fr>

Cheers.

-- 
Robert
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/