Skip to contentSkip to conversation

led/ledtrig-heartbeat: Convert timers to use

PATCH In review Kees Cook 2 participants 3 messages Oct 5, 2017 – Oct 6, 2017 LED

lore
Share & tools
lore

Kees Cook

author
#0 lore raw

PATCH 1/1 led/ledtrig-heartbeat: Convert timers to use

Instead of using .data directly, convert to from_timer. Since the trigger_data is allocated separately, the led_cdev must be explicitly tracked for the callback.

Cc Richard Purdie <[email protected]>
Cc Jacek Anaszewski <[email protected]>
Cc Pavel Machek <[email protected]>
Cc Zhang Bo <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Thomas Gleixner <[email protected]>

---
This requires commit 686fef928bba ("timer: Prepare to change timer callback argument type") in v4.14-rc3, but should be otherwise stand-alone.

---
 drivers/leds/trigger/ledtrig-heartbeat.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)
drivers/leds/trigger/ledtrig-heartbeat.c +10 -6
index e95ea65380c8..f0896de410b8 100644
@@ -25,19 +25,23 @@
 static int panic_heartbeats;
 
 struct heartbeat_trig_data {
+	struct led_classdev *led_cdev;
 	unsigned int phase;
 	unsigned int period;
 	struct timer_list timer;
 	unsigned int invert;
 };
 
-static void led_heartbeat_function(unsigned long data)
+static void led_heartbeat_function(struct timer_list *t)
 {
-	struct led_classdev *led_cdev = (struct led_classdev *) data;
-	struct heartbeat_trig_data *heartbeat_data = led_cdev->trigger_data;
+	struct heartbeat_trig_data *heartbeat_data =
+		from_timer(heartbeat_data, t, timer);
+	struct led_classdev *led_cdev;
 	unsigned long brightness = LED_OFF;
 	unsigned long delay = 0;
 
+	led_cdev = heartbeat_data->led_cdev;
+
 	if (unlikely(panic_heartbeats)) {
 		led_set_brightness_nosleep(led_cdev, LED_OFF);
 		return;
@@ -127,18 +131,18 @@ static void heartbeat_trig_activate(struct led_classdev *led_cdev)
 		return;
 
 	led_cdev->trigger_data = heartbeat_data;
+	heartbeat_data->led_cdev = led_cdev;
 	rc = device_create_file(led_cdev->dev, &dev_attr_invert);
 	if (rc) {
 		kfree(led_cdev->trigger_data);
 		return;
 	}
 
-	setup_timer(&heartbeat_data->timer,
-		    led_heartbeat_function, (unsigned long) led_cdev);
+	timer_setup(&heartbeat_data->timer, led_heartbeat_function, 0);
 	heartbeat_data->phase = 0;
 	if (!led_cdev->blink_brightness)
 		led_cdev->blink_brightness = led_cdev->max_brightness;
-	led_heartbeat_function(heartbeat_data->timer.data);
+	led_heartbeat_function(&heartbeat_data->timer);
 	set_bit(LED_BLINK_SW, &led_cdev->work_flags);
 	led_cdev->activated = true;
 }
Signature · 6 lines
2.7.4


-- 
Kees Cook
Pixel Security
#1 lore raw

Hi Kees,

Thanks for the patch.

Quoted 65 lines · Kees Cook

Instead of using .data directly, convert to from_timer. Since the trigger_data is allocated separately, the led_cdev must be explicitly tracked for the callback.

Cc Richard Purdie <[email protected]>
Cc Jacek Anaszewski <[email protected]>
Cc Pavel Machek <[email protected]>
Cc Zhang Bo <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Thomas Gleixner <[email protected]>

---
This requires commit 686fef928bba ("timer: Prepare to change timer callback argument type") in v4.14-rc3, but should be otherwise stand-alone.

---
 drivers/leds/trigger/ledtrig-heartbeat.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)
drivers/leds/trigger/ledtrig-heartbeat.c +8 -4
index e95ea65380c8..f0896de410b8 100644
@@ -25,19 +25,23 @@
 static int panic_heartbeats;
 
 struct heartbeat_trig_data {
+	struct led_classdev *led_cdev;
 	unsigned int phase;
 	unsigned int period;
 	struct timer_list timer;
 	unsigned int invert;
 };
 
-static void led_heartbeat_function(unsigned long data)
+static void led_heartbeat_function(struct timer_list *t)
 {
-	struct led_classdev *led_cdev = (struct led_classdev *) data;
-	struct heartbeat_trig_data *heartbeat_data = led_cdev->trigger_data;
+	struct heartbeat_trig_data *heartbeat_data =
+		from_timer(heartbeat_data, t, timer);
+	struct led_classdev *led_cdev;
 	unsigned long brightness = LED_OFF;
 	unsigned long delay = 0;
 
+	led_cdev = heartbeat_data->led_cdev;
+
 	if (unlikely(panic_heartbeats)) {
 		led_set_brightness_nosleep(led_cdev, LED_OFF);
 		return;
@@ -127,18 +131,18 @@ static void heartbeat_trig_activate(struct led_classdev *led_cdev)
 		return;
 
 	led_cdev->trigger_data = heartbeat_data;
+	heartbeat_data->led_cdev = led_cdev;
 	rc = device_create_file(led_cdev->dev, &dev_attr_invert);
 	if (rc) {
 		kfree(led_cdev->trigger_data);
 		return;
 	}
 
-	setup_timer(&heartbeat_data->timer,

drivers/leds/trigger/ledtrig-heartbeat.c static void heartbeat_trig_activate(struct led_classdev *led_cdev) · line 148

Quoted 10 lines
drivers/leds/trigger/ledtrig-heartbeat.c +2 -2
-		    led_heartbeat_function, (unsigned long) led_cdev);
+	timer_setup(&heartbeat_data->timer, led_heartbeat_function, 0);
 	heartbeat_data->phase = 0;
 	if (!led_cdev->blink_brightness)
 		led_cdev->blink_brightness = led_cdev->max_brightness;
-	led_heartbeat_function(heartbeat_data->timer.data);
+	led_heartbeat_function(&heartbeat_data->timer);
 	set_bit(LED_BLINK_SW, &led_cdev->work_flags);
 	led_cdev->activated = true;
 }

It seems that the patch title is somewhat incomplete, so I amended it as follows:

s/to use/to use timer_setup()/

and applied to the for-next branch of linux-leds.git after rebasing it onto 4.14-rc3.

Signature · 2 lines
Best regards,
Jacek Anaszewski
author
#2 lore raw
Quoted 84 lines · Jacek Anaszewski

Hi Kees,

Thanks for the patch.

Quoted 65 lines · Kees Cook

Instead of using .data directly, convert to from_timer. Since the trigger_data is allocated separately, the led_cdev must be explicitly tracked for the callback.

Cc Richard Purdie <[email protected]>
Cc Jacek Anaszewski <[email protected]>
Cc Pavel Machek <[email protected]>
Cc Zhang Bo <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Thomas Gleixner <[email protected]>

---
This requires commit 686fef928bba ("timer: Prepare to change timer callback argument type") in v4.14-rc3, but should be otherwise stand-alone.

---
 drivers/leds/trigger/ledtrig-heartbeat.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)
drivers/leds/trigger/ledtrig-heartbeat.c +8 -4
index e95ea65380c8..f0896de410b8 100644
@@ -25,19 +25,23 @@
 static int panic_heartbeats;
 
 struct heartbeat_trig_data {
+     struct led_classdev *led_cdev;
      unsigned int phase;
      unsigned int period;
      struct timer_list timer;
      unsigned int invert;
 };
 
-static void led_heartbeat_function(unsigned long data)
+static void led_heartbeat_function(struct timer_list *t)
 {
-     struct led_classdev *led_cdev = (struct led_classdev *) data;
-     struct heartbeat_trig_data *heartbeat_data = led_cdev->trigger_data;
+     struct heartbeat_trig_data *heartbeat_data =
+             from_timer(heartbeat_data, t, timer);
+     struct led_classdev *led_cdev;
      unsigned long brightness = LED_OFF;
      unsigned long delay = 0;
 
+     led_cdev = heartbeat_data->led_cdev;
+
      if (unlikely(panic_heartbeats)) {
              led_set_brightness_nosleep(led_cdev, LED_OFF);
              return;
@@ -127,18 +131,18 @@ static void heartbeat_trig_activate(struct led_classdev *led_cdev)
              return;
 
      led_cdev->trigger_data = heartbeat_data;
+     heartbeat_data->led_cdev = led_cdev;
      rc = device_create_file(led_cdev->dev, &dev_attr_invert);
      if (rc) {
              kfree(led_cdev->trigger_data);
              return;
      }
 
-     setup_timer(&heartbeat_data->timer,

drivers/leds/trigger/ledtrig-heartbeat.c static void heartbeat_trig_activate(struct led_classdev *led_cdev) · line 148

Quoted 10 lines
drivers/leds/trigger/ledtrig-heartbeat.c +2 -2
-                 led_heartbeat_function, (unsigned long) led_cdev);
+     timer_setup(&heartbeat_data->timer, led_heartbeat_function, 0);
      heartbeat_data->phase = 0;
      if (!led_cdev->blink_brightness)
              led_cdev->blink_brightness = led_cdev->max_brightness;
-     led_heartbeat_function(heartbeat_data->timer.data);
+     led_heartbeat_function(&heartbeat_data->timer);
      set_bit(LED_BLINK_SW, &led_cdev->work_flags);
      led_cdev->activated = true;
 }

It seems that the patch title is somewhat incomplete, so I amended it as follows:

Quoted 1 line

s/to use/to use timer_setup()/

Yeah, thanks for fixing that. This got pointed out elsewhere and I've since hunted down and fixed my scripts to not accidentally truncate subjects. *sigh* :)

Quoted 2 lines

and applied to the for-next branch of linux-leds.git after rebasing it onto 4.14-rc3.

Thanks!

-Kees

Signature · 2 lines
Kees Cook
Pixel Security