Skip to contentSkip to conversation

clockevent: let set_mode() setup delta information

PATCH In review Magnus Damm 1 participant 2 messages Dec 1, 2008 – Dec 10, 2008

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 2/10 clockevent: let set_mode() setup delta information

From: Magnus Damm <[email protected]>

Allow the set_mode() clockevent callback to decide and fill in delta details such as shift, mult, max_delta_ns and min_delta_ns.

With this change the clockevent can be registered without delta details which allows us to keep the parent clock disabled until the clockevent gets setup using set_mode().

Letting set_mode() fill in or update delta details allows us to save power by disabling the parent clock while the clockevent is unused. This may however make the parent clock rate change, so next time the clockevent gets enabled we need let set_mode() to update the detla details accordingly. Doing it at registration time is not enough.

Furthermore, the delta details seem unused in the case of periodic-only clockevent drivers, so this change also allows registration of such drivers without the delta details filled in.

---

 kernel/time/clockevents.c |   20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)
kernel/time/clockevents.c +11 -9
@@ -68,6 +68,17 @@ void clockevents_set_mode(struct clock_e
 	if (dev->mode != mode) {
 		dev->set_mode(mode, dev);
 		dev->mode = mode;
+
+		/*
+		 * A nsec2cyc multiplicator of 0 is invalid and we'd crash
+		 * on it, so fix it up and emit a warning:
+		 */
+		if (mode = CLOCK_EVT_MODE_ONESHOT) {
+			if (unlikely(!dev->mult)) {
+				dev->mult = 1;
+				WARN_ON(1);
+			}
+		}
 	}
 }
 
@@ -166,15 +177,6 @@ static void clockevents_notify_released(
 void clockevents_register_device(struct clock_event_device *dev)
 {
 	BUG_ON(dev->mode != CLOCK_EVT_MODE_UNUSED);
-	/*
-	 * A nsec2cyc multiplicator of 0 is invalid and we'd crash
-	 * on it, so fix it up and emit a warning:
-	 */
-	if (unlikely(!dev->mult)) {
-		dev->mult = 1;
-		WARN_ON(1);
-	}
-
 	spin_lock(&clockevents_lock);
 
 	list_add(&dev->list, &clockevent_devices);
#1 lore raw

Hi guys,

Any comments on this patch? It allow us to use the frequency from the clock framework.
Please let me know what you think. Thanks for your help.

Cheers,

/ magnus

Quoted 62 lines · Magnus Damm

From: Magnus Damm <[email protected]>

Allow the set_mode() clockevent callback to decide and fill in delta details such as shift, mult, max_delta_ns and min_delta_ns.

With this change the clockevent can be registered without delta details which allows us to keep the parent clock disabled until the clockevent gets setup using set_mode().

Letting set_mode() fill in or update delta details allows us to save power by disabling the parent clock while the clockevent is unused. This may however make the parent clock rate change, so next time the clockevent gets enabled we need let set_mode() to update the detla details accordingly. Doing it at registration time is not enough.

Furthermore, the delta details seem unused in the case of periodic-only clockevent drivers, so this change also allows registration of such drivers without the delta details filled in.

---

 kernel/time/clockevents.c |   20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)
kernel/time/clockevents.c 2008-11-28 15:51:31.000000000 +0900 renamed from kernel/time/clockevents.c +11 -9
@@ -68,6 +68,17 @@ void clockevents_set_mode(struct clock_e
       if (dev->mode != mode) {
               dev->set_mode(mode, dev);
               dev->mode = mode;
+
+               /*
+                * A nsec2cyc multiplicator of 0 is invalid and we'd crash
+                * on it, so fix it up and emit a warning:
+                */
+               if (mode = CLOCK_EVT_MODE_ONESHOT) {
+                       if (unlikely(!dev->mult)) {
+                               dev->mult = 1;
+                               WARN_ON(1);
+                       }
+               }
       }
 }
 
@@ -166,15 +177,6 @@ static void clockevents_notify_released(
 void clockevents_register_device(struct clock_event_device *dev)
 {
       BUG_ON(dev->mode != CLOCK_EVT_MODE_UNUSED);
-       /*
-        * A nsec2cyc multiplicator of 0 is invalid and we'd crash
-        * on it, so fix it up and emit a warning:
-        */
-       if (unlikely(!dev->mult)) {
-               dev->mult = 1;
-               WARN_ON(1);
-       }
-
       spin_lock(&clockevents_lock);
 
       list_add(&dev->list, &clockevent_devices);