linux-kernel - Re: [PATCH] clockevents: Add (missing) default case for switch blocks

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: <CAKohpokmCof2P8Un+i5eOS3BQhH_pMNATH4xkbbhw5TqJFEy9Q@mail.gmail.com>
Date:	Fri, 20 Feb 2015 17:26:49 +0530
From:	Viresh Kumar <viresh.kumar@...aro.org>
To:	Ingo Molnar <mingo@...nel.org>
Cc:	Peter Zijlstra <peterz@...radead.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	Linaro Kernel Mailman List <linaro-kernel@...ts.linaro.org>,
	Kevin Hilman <khilman@...aro.org>,
	Preeti U Murthy <preeti@...ux.vnet.ibm.com>,
	Daniel Lezcano <daniel.lezcano@...aro.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Linaro Networking <linaro-networking@...aro.org>,
	Steven Miao <realmz6@...il.com>,
	Mark Salter <msalter@...hat.com>,
	Michal Simek <monstr@...str.eu>,
	Ralf Baechle <ralf@...ux-mips.org>,
	Ley Foon Tan <lftan@...era.com>,
	Jonas Bonn <jonas@...thpole.se>,
	"David S. Miller" <davem@...emloft.net>,
	Jeff Dike <jdike@...toit.com>,
	Guan Xuetao <gxt@...c.pku.edu.cn>
Subject: Re: [PATCH] clockevents: Add (missing) default case for switch blocks

On 20 February 2015 at 17:11, Ingo Molnar <mingo@...nel.org> wrote:
>
> * Peter Zijlstra <peterz@...radead.org> wrote:

>> Maybe we should break that enum into two; one for devices
>> and one for the core interface and avoid the problem that
>> way.
>
> Yeah, that would do the trick.

Thanks for your suggestions. Just to confirm (before I spam lists with
patches), is
this somewhat similar to what you are looking for ?

diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h
index 59af26b54d15..80b669cb836d 100644
--- a/include/linux/clockchips.h
+++ b/include/linux/clockchips.h
@@ -32,17 +32,24 @@ enum clock_event_nofitiers {
 struct clock_event_device;
 struct module;

-/* Clock event mode commands */
+/* Clock event mode commands for legacy ->set_mode(): OBSOLETE */
 enum clock_event_mode {
        CLOCK_EVT_MODE_UNUSED = 0,
        CLOCK_EVT_MODE_SHUTDOWN,
        CLOCK_EVT_MODE_PERIODIC,
        CLOCK_EVT_MODE_ONESHOT,
        CLOCK_EVT_MODE_RESUME,
-
-       /* Legacy ->set_mode() callback doesn't support below modes */
 };

+/* Clock event modes, only for core's internal use */
+enum clock_event_dev_mode {
+       CLOCK_EVT_DEV_MODE_UNUSED = 0,
+       CLOCK_EVT_DEV_MODE_SHUTDOWN,
+       CLOCK_EVT_DEV_MODE_PERIODIC,
+       CLOCK_EVT_DEV_MODE_ONESHOT,
+       CLOCK_EVT_DEV_MODE_RESUME,
+       CLOCK_EVT_DEV_MODE_ONESHOT_STOPPED,  /* This would be the new
mode which I will add later */
+};
+
 /*
  * Clock event features
  */
diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
index 489642b08d64..16555d3db94d 100644
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -95,14 +95,18 @@ u64 clockevent_delta2ns(unsigned long latch,
struct clock_event_device *evt)
 EXPORT_SYMBOL_GPL(clockevent_delta2ns);

 static int __clockevents_set_mode(struct clock_event_device *dev,
-                                 enum clock_event_mode mode)
+                                 enum clock_event_dev_mode mode)
 {
        /* Transition with legacy set_mode() callback */
        if (dev->set_mode) {
                /* Legacy callback doesn't support new modes */
-               if (mode > CLOCK_EVT_MODE_RESUME)
+               if (mode > CLOCK_EVT_DEV_MODE_RESUME)
                        return -ENOSYS;
-               dev->set_mode(mode, dev);
+               /*
+                * 'clock_event_dev_mode' and 'clock_event_mode' have 1-to-1
+                * mapping until *_RESUME, and so a simple cast will work.
+                */
+               dev->set_mode((enum clock_event_mode)mode, dev);
                return 0;
        }

@@ -111,29 +115,29 @@ static int __clockevents_set_mode(struct
clock_event_device *dev,

        /* Transition with new mode-specific callbacks */
        switch (mode) {
-       case CLOCK_EVT_MODE_UNUSED:
+       case CLOCK_EVT_DEV_MODE_UNUSED:
                /*
                 * This is an internal state, which is guaranteed to go from
                 * SHUTDOWN to UNUSED. No driver interaction required.
                 */
                return 0;

-       case CLOCK_EVT_MODE_SHUTDOWN:
+       case CLOCK_EVT_DEV_MODE_SHUTDOWN:
                return dev->set_mode_shutdown(dev);

-       case CLOCK_EVT_MODE_PERIODIC:
+       case CLOCK_EVT_DEV_MODE_PERIODIC:
                /* Core internal bug */
                if (!(dev->features & CLOCK_EVT_FEAT_PERIODIC))
                        return -ENOSYS;
                return dev->set_mode_periodic(dev);

-       case CLOCK_EVT_MODE_ONESHOT:
+       case CLOCK_EVT_DEV_MODE_ONESHOT:
                /* Core internal bug */
                if (!(dev->features & CLOCK_EVT_FEAT_ONESHOT))
                        return -ENOSYS;
                return dev->set_mode_oneshot(dev);

-       case CLOCK_EVT_MODE_RESUME:
+       case CLOCK_EVT_DEV_MODE_RESUME:
                /* Optional callback */
                if (dev->set_mode_resume)
                        return dev->set_mode_resume(dev);



Ofcourse, we also need to replace 'clock_event_mode' with 'clock_event_dev_mode'
and 'CLOCK_EVT_MODE_*' with 'CLOCK_EVT_DEV_MODE_*' in all core code..

--
viresh
--
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/