Hi!
> A LED is usually powered by a voltage/current regulator. Let the LED core
> know about it. This allows the LED core to turn on or off the power supply
> as needed.
>
> Because turning ON/OFF a regulator might block, it is not done
> synchronously but done in a workqueue. Turning ON the regulator is
> always
How will this interact with LEDs that can be used from atomic context?
> +static ssize_t regulator_auto_off_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t size)
> +{
> + struct led_classdev *led_cdev = dev_get_drvdata(dev);
> + ssize_t ret = size;
> + bool auto_off;
> +
> + if (strncmp(buf, "enable\n", size) == 0)
> + auto_off = true;
> + else if (strncmp(buf, "disable\n", size) == 0)
> + auto_off = false;
> + else
> + return -EINVAL;
Sounds like device power management to me. Is it compatible with that?
> @@ -135,6 +203,8 @@ static void set_brightness_delayed(struct work_struct *ws)
> (led_cdev->flags & LED_HW_PLUGGABLE)))
> dev_err(led_cdev->dev,
> "Setting an LED's brightness failed (%d)\n", ret);
> +
> + led_handle_regulator(led_cdev);
> }
>
You only modify set_brigthness_delays, so this will not work at all
for non-blocking LEDs, right?
> static void led_set_software_blink(struct led_classdev *led_cdev,
> @@ -189,6 +259,7 @@ static void led_blink_setup(struct led_classdev *led_cdev,
> void led_init_core(struct led_classdev *led_cdev)
> {
> INIT_WORK(&led_cdev->set_brightness_work, set_brightness_delayed);
> + INIT_DELAYED_WORK(&led_cdev->reg_off_work, turn_off_regulator_delayed);
>
Could this re-use the workqueue? Many systems will not need
regulators, so this is overhead...
> + /*
> + * the regulator must be turned off. This cannot be
Use "The", and fix double spaces between must and be.
> + } else if (regulator_on && old == REG_R_OFF_U_OFF) {
> + /*
> + * the regulator must be enabled. This cannot be here
"The"
> + /*
> + * small optimization. Cancel the work that had been started
"Small."
> +#include <linux/regulator/consumer.h>
> +
> +/*
> + * The regulator state tracks 2 boolean variables:
> + * - the state of regulator (or more precisely the state required by
> + * led core layer, as many users can interact with the same regulator).
> + * It is tracked by bit 0.
> + * - the state last asked-for by the LED user. It is tracked by bit 1.
> + */
> +#define REG_R_ON BIT(0)
> +#define REG_U_ON BIT(1)
> +
> +enum { REG_R_OFF_U_OFF = 0,
> + REG_R_ON_U_OFF = REG_R_ON,
> + REG_R_OFF_U_ON = REG_U_ON,
> + REG_R_ON_U_ON = REG_R_ON | REG_U_ON
> +};
That's quite weird use of enum.
> +++ b/include/linux/leds.h
> @@ -149,6 +149,15 @@ struct led_classdev {
>
> /* Ensures consistent access to the LED Flash Class device */
> struct mutex led_access;
> +
> + /* regulator */
"Regulator".
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Attachment:
signature.asc
Description: PGP signature
