Skip to contentSkip to conversation

leds: blinkm: Avoid calling INIT_WORK in blinkm_led_common_set()

PATCH New Axel Lin 1 participant 1 message Oct 27, 2012

Subsystems (2)
LED Blinkm RGB LED driver
lore
Share & tools
lore

Axel Lin

author
#0 lore raw

PATCH 1/1 leds: blinkm: Avoid calling INIT_WORK in blinkm_led_common_set()

Calling INIT_WORK in blinkm_led_common_set() means we init a workqueue every time when brightness_set callback is called.
Move INIT_WORK to blinkm_probe() so we only need to init the workqueue once.

This patch also refactors the data structure of blinkm_data and blinkm_work. Embedded struct blinkm_led in struct blinkm_work, and embedded struct blinkm_work in blinkm_data. With this change, we don't need to allocate and free memory for bl_work in blinkm_led_common_set() and led_work().

Signed-off-by Axel Lin <[email protected]>

---
Hi Jan-Simon,
I don't have this hardware, can you help review and testing this patch.
Thank you,
Axel

 drivers/leds/leds-blinkm.c |   30 ++++++++++--------------------
 1 file changed, 10 insertions(+), 20 deletions(-)
drivers/leds/leds-blinkm.c +10 -20
index f7c3d7f..5e4cd9b 100644
@@ -44,7 +44,8 @@ struct blinkm_led {
 };
 
 struct blinkm_work {
-	struct blinkm_led *blinkm_led;
+	/* used for led class interface */
+	struct blinkm_led led;
 	struct work_struct work;
 };
 

@@ -54,8 +55,7 @@ struct blinkm_work {
 struct blinkm_data {
 	struct i2c_client *i2c_client;
 	struct mutex update_lock;
-	/* used for led class interface */
-	struct blinkm_led blinkm_leds[3];
+	struct blinkm_work bl_work[3];
 	/* used for "blinkm" sysfs interface */
 	u8 red;			/* color red */
 	u8 green;		/* color green */

@@ -446,12 +446,10 @@ static int blinkm_transfer_hw(struct i2c_client *client, int cmd)
 static void led_work(struct work_struct *work)
 {
 	int ret;
-	struct blinkm_led *led;
-	struct blinkm_data *data ;
 	struct blinkm_work *blm_work = work_to_blmwork(work);
+	struct blinkm_led *led = &blm_work->led;
+	struct blinkm_data *data = i2c_get_clientdata(led->i2c_client);
 
-	led = blm_work->blinkm_led;
-	data = i2c_get_clientdata(led->i2c_client);
 	ret = blinkm_transfer_hw(led->i2c_client, BLM_GO_RGB);
 	atomic_dec(&led->active);
 	dev_dbg(&led->i2c_client->dev,

@@ -459,7 +457,6 @@ static void led_work(struct work_struct *work)
 			" next_blue = %d, active = %d\n",
 			data->next_red, data->next_green,
 			data->next_blue, atomic_read(&led->active));
-	kfree(blm_work);
 }
 
 static int blinkm_led_common_set(struct led_classdev *led_cdev,

@@ -468,7 +465,7 @@ static int blinkm_led_common_set(struct led_classdev *led_cdev,
 	/* led_brightness is 0, 127 or 255 - we just use it here as-is */
 	struct blinkm_led *led = cdev_to_blmled(led_cdev);
 	struct blinkm_data *data = i2c_get_clientdata(led->i2c_client);
-	struct blinkm_work *bl_work;
+	struct blinkm_work *bl_work = &data->bl_work[color];
 
 	switch (color) {
 	case RED:

@@ -510,10 +507,6 @@ static int blinkm_led_common_set(struct led_classdev *led_cdev,
 		return -EINVAL;
 	}
 
-	bl_work = kzalloc(sizeof(*bl_work), GFP_ATOMIC);
-	if (!bl_work)
-		return -ENOMEM;
-
 	atomic_inc(&led->active);
 	dev_dbg(&led->i2c_client->dev,
 			"#TO_SCHED# next_red = %d, next_green = %d,"

@@ -521,9 +514,6 @@ static int blinkm_led_common_set(struct led_classdev *led_cdev,
 			data->next_red, data->next_green,
 			data->next_blue, atomic_read(&led->active));
 
-	/* a fresh work _item_ for each change */
-	bl_work->blinkm_led = led;
-	INIT_WORK(&bl_work->work, led_work);
 	/* queue work in own queue for easy sync on exit*/
 	schedule_work(&bl_work->work);
 

@@ -667,8 +657,9 @@ static int __devinit blinkm_probe(struct i2c_client *client,
 	}
 
 	for (i = 0; i < 3; i++) {
+		INIT_WORK(&data->bl_work[i].work, led_work);
 		/* RED = 0, GREEN = 1, BLUE = 2 */
-		led[i] = &data->blinkm_leds[i];
+		led[i] = &data->bl_work[i].led;
 		led[i]->i2c_client = client;
 		led[i]->id = i;
 		led[i]->led_cdev.max_brightness = 255;

@@ -746,13 +737,12 @@ exit:
 static int __devexit blinkm_remove(struct i2c_client *client)
 {
 	struct blinkm_data *data = i2c_get_clientdata(client);
-	int ret = 0;
-	int i;
+	int i, ret;
 
 	/* make sure no workqueue entries are pending */
 	for (i = 0; i < 3; i++) {
 		flush_scheduled_work();
-		led_classdev_unregister(&data->blinkm_leds[i].led_cdev);
+		led_classdev_unregister(&data->bl_work[i].led.led_cdev);
 	}
 
 	/* reset rgb */
Signature · 1 line
1.7.9.5