From: Jeremy Kerr <jeremy.kerr@canonical.com>

BugLink: http://bugs.launchpad.net/bugs/531696

Setting the LED brightness on mc13892 boards uses spi_sync, which may
sleep. The LED class infrastructure requires that the brightness_set
operation does not sleep, as it may be called from atomic contexts.

This results in a 'scheduling while atomic BUG' when doing the
following:

echo mmc0 > /sys/class/leds/xxx/trigger

This change modifies the driver to use a workqueue to do the SPI
operation, so that we can sleep. The led_classdev->brightness_set
callback just updates the brightness value and schedules work, making it
suitable to call with irqs disabled.

Because we've split the set operation into two parts, we need to define
some context (struct mc13892_led) to pass between the led classdev code
and the work function.

Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
Signed-off-by: Bryan Wu <bryan.wu@canonical.com>
---
drivers/leds/leds-mc13892.c | 120 ++++++++++++++++++++++++-------------------
1 files changed, 68 insertions(+), 52 deletions(-)

diff --git a/drivers/leds/leds-mc13892.c b/drivers/leds/leds-mc13892.c
index 9edd204..464bec9 100644
--- a/drivers/leds/leds-mc13892.c
+++ b/drivers/leds/leds-mc13892.c
@@ -16,89 +16,105 @@
#include <linux/platform_device.h>
#include <linux/leds.h>
#include <linux/pmic_light.h>
+#include <linux/workqueue.h>

-static void mc13892_led_set(struct led_classdev *led_cdev,
- enum led_brightness value)
-{
- struct platform_device *dev = to_platform_device(led_cdev->dev->parent);
- int led_ch;
+#define LED_NAME_LEN 16

- switch (dev->id) {
- case 'r':
- led_ch = LIT_RED;
- break;
- case 'g':
- led_ch = LIT_GREEN;
- break;
- case 'b':
- led_ch = LIT_BLUE;
- break;
- default:
- return;
- }
+struct mc13892_led {
+ char name[LED_NAME_LEN];
+ enum lit_channel channel;
+ int brightness;
+ struct work_struct work;
+ struct led_classdev cdev;
+};
+
+static void mc13892_led_work(struct work_struct *work)
+{
+ struct mc13892_led *led = container_of(work, struct mc13892_led, work);

/* set current with medium value, in case current is too large */
- mc13892_bklit_set_current(led_ch, LIT_CURR_12);
+ mc13892_bklit_set_current(led->channel, LIT_CURR_12);
/* max duty cycle is 63, brightness needs to be divided by 4 */
- mc13892_bklit_set_dutycycle(led_ch, value / 4);
+ mc13892_bklit_set_dutycycle(led->channel, led->brightness / 4);
+}
+

+static void mc13892_led_set(struct led_classdev *led_cdev,
+ enum led_brightness value)
+{
+ struct mc13892_led *led = container_of(led_cdev,
+ struct mc13892_led, cdev);
+ led->brightness = value;
+ schedule_work(&led->work);
}

static int mc13892_led_remove(struct platform_device *dev)
{
- struct led_classdev *led_cdev = platform_get_drvdata(dev);
+ struct mc13892_led *led = platform_get_drvdata(dev);

- led_classdev_unregister(led_cdev);
- kfree(led_cdev->name);
- kfree(led_cdev);
+ led_classdev_unregister(&led->cdev);
+ flush_work(&led->work);
+ kfree(led);

return 0;
}

-#define LED_NAME_LEN 16
+static enum lit_channel mc13892_led_channel(int id)
+{
+ switch (id) {
+ case 'r':
+ return LIT_RED;
+ case 'g':
+ return LIT_GREEN;
+ case 'b':
+ return LIT_BLUE;
+ default:
+ return -1;
+ }
+}

static int mc13892_led_probe(struct platform_device *dev)
{
+ struct mc13892_led *led;
+ enum lit_channel chan;
int ret;
- struct led_classdev *led_cdev;
- char *name;

- led_cdev = kzalloc(sizeof(struct led_classdev), GFP_KERNEL);
- if (led_cdev == NULL) {
- dev_err(&dev->dev, "No memory for device
");
- return -ENOMEM;
+ /* ensure we have space for the channel name and a NUL */
+ if (strlen(dev->name) > LED_NAME_LEN - 2) {
+ dev_err(&dev->dev, "led name is too long
");
+ return -EINVAL;
}
- name = kzalloc(LED_NAME_LEN, GFP_KERNEL);
- if (name == NULL) {
- dev_err(&dev->dev, "No memory for device
");
- ret = -ENOMEM;
- goto exit_err;
+
+ chan = mc13892_led_channel(dev->id);
+ if (chan == -1) {
+ dev_err(&dev->dev, "invalid LED id '%d'
", dev->id);
+ return -EINVAL;
}

- strcpy(name, dev->name);
- ret = strlen(dev->name);
- if (ret > LED_NAME_LEN - 2) {
- dev_err(&dev->dev, "led name is too long
");
- goto exit_err1;
+ led = kzalloc(sizeof(*led), GFP_KERNEL);
+ if (!led) {
+ dev_err(&dev->dev, "No memory for device
");
+ return -ENOMEM;
}
- name[ret] = dev->id;
- name[ret + 1] = '';
- led_cdev->name = name;
- led_cdev->brightness_set = mc13892_led_set;

- ret = led_classdev_register(&dev->dev, led_cdev);
+ led->channel = chan;
+ led->cdev.name = led->name;
+ led->cdev.brightness_set = mc13892_led_set;
+ INIT_WORK(&led->work, mc13892_led_work);
+ snprintf(led->name, sizeof(led->name), "%s%c",
+ dev->name, (char)dev->id);
+
+ ret = led_classdev_register(&dev->dev, &led->cdev);
if (ret < 0) {
dev_err(&dev->dev, "led_classdev_register failed
");
- goto exit_err1;
+ goto err_free;
}

- platform_set_drvdata(dev, led_cdev);
+ platform_set_drvdata(dev, led);

return 0;
- exit_err1:
- kfree(led_cdev->name);
- exit_err:
- kfree(led_cdev);
+err_free:
+ kfree(led);
return ret;
}

--
1.7.0


--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team