| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20081229151152.5699d59c.akpm@linux-foundation.org>
Date: Mon, 29 Dec 2008 15:11:52 -0800
From: Andrew Morton <akpm@...ux-foundation.org>
To: Guennadi Liakhovetski <lg@...x.de>
Cc: ben-linux@...ff.org, linux-kernel@...r.kernel.org,
rpurdie@...ys.net
Subject: Re: [PATCH v2] leds: add a dac124s085 SPI LED driver
On Thu, 18 Dec 2008 13:51:06 +0100 (CET)
Guennadi Liakhovetski <lg@...x.de> wrote:
> A simple LED driver using the DAC124S085 DAC from NatSemi
>
> Signed-off-by: Guennadi Liakhovetski <lg@...x.de>
> ---
>
> Changes since v1: implemented corrections by Ben Dooks: macro instead of a
> magic constant, endianness conversion, ARRAY_SIZE instead of a hard-coded
> constant.
>
> Still needs a previous patch making maximum brightness configurable:
>
> http://marc.info/?l=linux-kernel&m=122771446123369&w=2
>
> drivers/leds/Kconfig | 7 ++
> drivers/leds/Makefile | 3 +
> drivers/leds/leds-dac124s085.c | 147 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 157 insertions(+), 0 deletions(-)
> create mode 100644 drivers/leds/leds-dac124s085.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index a4a1ae2..a28635f 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -178,6 +178,13 @@ config LEDS_DA903X
> This option enables support for on-chip LED drivers found
> on Dialog Semiconductor DA9030/DA9034 PMICs.
>
> +config LEDS_DAC124S085
> + tristate "LED Support for DAC124S085 SPI DAC"
> + depends on LEDS_CLASS && SPI
> + help
> + This option enables support for DAC124S085 SPI DAC from NatSemi,
> + which can be used to control up to four LEDs.
> +
> comment "LED Triggers"
>
> config LEDS_TRIGGERS
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index bc247cb..8780152 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -26,6 +26,9 @@ obj-$(CONFIG_LEDS_DA903X) += leds-da903x.o
> obj-$(CONFIG_LEDS_HP_DISK) += leds-hp-disk.o
> obj-$(CONFIG_LEDS_WM8350) += leds-wm8350.o
>
> +# LED SPI Drivers
> +obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o
> +
> # LED Triggers
> obj-$(CONFIG_LEDS_TRIGGER_TIMER) += ledtrig-timer.o
> obj-$(CONFIG_LEDS_TRIGGER_IDE_DISK) += ledtrig-ide-disk.o
> diff --git a/drivers/leds/leds-dac124s085.c b/drivers/leds/leds-dac124s085.c
> new file mode 100644
> index 0000000..9bd25c1
> --- /dev/null
> +++ b/drivers/leds/leds-dac124s085.c
> @@ -0,0 +1,147 @@
> +/*
> + * Copyright 2008
> + * Guennadi Liakhovetski, DENX Software Engineering, <lg@...x.de>
> + *
> + * This file is subject to the terms and conditions of version 2 of
> + * the GNU General Public License. See the file COPYING in the main
> + * directory of this archive for more details.
> + *
> + * LED driver for the DAC124S085 SPI DAC
> + */
> +
> +#include <linux/leds.h>
> +#include <linux/mutex.h>
> +#include <linux/spinlock.h>
> +#include <linux/workqueue.h>
> +#include <linux/spi/spi.h>
> +
> +struct dac124s085_led {
> + struct led_classdev ldev;
> + struct spi_device *spi;
> + int id;
> + int brightness;
> + char name[sizeof("dac124s085-3")];
> +
> + struct mutex mutex;
> + struct work_struct work;
> + spinlock_t lock;
> +};
> +
> +struct dac124s085 {
> + struct dac124s085_led leds[4];
> +};
> +
> +#define REG_WRITE (0 << 12)
> +#define REG_WRITE_UPDATE (1 << 12)
> +#define ALL_WRITE_UPDATE (2 << 12)
> +#define POWER_DOWN_OUTPUT (3 << 12)
> +
> +static void dac124s085_led_work(struct work_struct *work)
> +{
> + struct dac124s085_led *led = container_of(work, struct dac124s085_led,
> + work);
> + u16 word;
> +
> + mutex_lock(&led->mutex);
> + word = cpu_to_le16(((led->id) << 14) | REG_WRITE_UPDATE |
> + (led->brightness & 0xfff));
> + spi_write(led->spi, (const u8 *)&word, sizeof(word));
This looks like it might have endianness issues?
> + mutex_unlock(&led->mutex);
> +}
> +
> +static void dac124s085_set_brightness(struct led_classdev *ldev,
> + enum led_brightness brightness)
> +{
> + struct dac124s085_led *led = container_of(ldev, struct dac124s085_led,
> + ldev);
> +
> + spin_lock(&led->lock);
> + led->brightness = brightness;
> + schedule_work(&led->work);
> + spin_unlock(&led->lock);
> +}
So.. why do we need to defer this to a workqueue rather than just
performing the operation synchronously?
What happens if there are two calls to set_brightness() in quick
succession, and the second occurs before the first has completed? I
think the schedule_work() fails and the second write gets lost?
> +static int dac124s085_probe(struct spi_device *spi)
> +{
> + struct dac124s085 *dac;
> + struct dac124s085_led *led;
> + int i, ret;
> +
> + dac = kzalloc(sizeof(*dac), GFP_KERNEL);
> + if (!dac)
> + return -ENOMEM;
> +
> + spi->bits_per_word = 16;
> +
> + for (i = 0; i < ARRAY_SIZE(dac->leds); i++) {
> + led = dac->leds + i;
> + led->id = i;
> + led->brightness = LED_OFF;
> + led->spi = spi;
> + snprintf(led->name, sizeof(led->name), "dac124s085-%d", i);
> + spin_lock_init(&led->lock);
> + INIT_WORK(&led->work, dac124s085_led_work);
> + mutex_init(&led->mutex);
> + led->ldev.name = led->name;
> + led->ldev.brightness = LED_OFF;
> + led->ldev.max_brightness = 0xfff;
> + led->ldev.brightness_set = dac124s085_set_brightness;
> + ret = led_classdev_register(&spi->dev, &led->ldev);
> + if (ret < 0)
> + goto eledcr;
> + }
> +
> + spi_set_drvdata(spi, dac);
> +
> + return 0;
> +
> +eledcr:
> + while (i--)
> + led_classdev_unregister(&dac->leds[i].ldev);
> +
> + spi_set_drvdata(spi, NULL);
> + kfree(dac);
> + return ret;
> +}
> +
> +static int dac124s085_remove(struct spi_device *spi)
> +{
> + struct dac124s085 *dac = spi_get_drvdata(spi);
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(dac->leds); i++) {
> + led_classdev_unregister(&dac->leds[i].ldev);
> + cancel_work_sync(&dac->leds[i].work);
> + }
> +
> + spi_set_drvdata(spi, NULL);
> + kfree(dac);
> +
> + return 0;
> +}
> +
> +static struct spi_driver dac124s085_driver = {
> + .probe = dac124s085_probe,
> + .remove = dac124s085_remove,
> + .driver = {
> + .name = "dac124s085",
> + .owner = THIS_MODULE,
> + },
> +};
> +
> +static int __init dac124s085_leds_init(void)
> +{
> + return spi_register_driver(&dac124s085_driver);
> +}
> +
> +static void __exit dac124s085_leds_exit(void)
> +{
> + spi_unregister_driver(&dac124s085_driver);
> +}
> +
> +module_init(dac124s085_leds_init);
> +module_exit(dac124s085_leds_exit);
> +
> +MODULE_AUTHOR("Guennadi Liakhovetski <lg@...x.de>");
> +MODULE_DESCRIPTION("DAC124S085 LED driver");
> +MODULE_LICENSE("GPL v2");
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists