2011-04-30 17:59:38

by Paul Parsons

[permalink] [raw]
Subject: [PATCH 2/2] leds: Add ASIC3 LED support

Add LED support for the HTC ASIC3. Underlying support is provided by the mfd/asic3 and leds/leds-asic3 drivers. An example configuration is provided by the pxa/hx4700 platform.

Signed-off-by: Paul Parsons <[email protected]>
---
diff -uprN clean-2.6.39-rc5/drivers/leds/Kconfig linux-2.6.39-rc5/drivers/leds/Kconfig
--- clean-2.6.39-rc5/drivers/leds/Kconfig 2011-04-28 11:18:14.818578777 +0100
+++ linux-2.6.39-rc5/drivers/leds/Kconfig 2011-04-30 16:39:48.434871549 +0100
@@ -379,6 +379,16 @@ config LEDS_NETXBIG
and 5Big Network v2 boards. The LEDs are wired to a CPLD and are
controlled through a GPIO extension bus.

+config LEDS_ASIC3
+ bool "LED support for the HTC ASIC3"
+ depends on MFD_ASIC3
+ default y
+ help
+ This option enables support for the LEDs on the HTC ASIC3. The HTC
+ ASIC3 LED GPIOs are inputs, not outputs, thus the leds-gpio driver
+ cannot be used. This driver supports hardware blinking with an on+off
+ period from 62ms to 125s. Say Y to enable LEDs on the HP iPAQ hx4700.
+
config LEDS_TRIGGERS
bool "LED Trigger support"
depends on LEDS_CLASS
diff -uprN clean-2.6.39-rc5/drivers/leds/Makefile linux-2.6.39-rc5/drivers/leds/Makefile
--- clean-2.6.39-rc5/drivers/leds/Makefile 2011-04-28 11:18:14.818578777 +0100
+++ linux-2.6.39-rc5/drivers/leds/Makefile 2011-04-30 15:44:30.288890098 +0100
@@ -42,6 +42,7 @@ obj-$(CONFIG_LEDS_DELL_NETBOOKS) += dell
obj-$(CONFIG_LEDS_MC13783) += leds-mc13783.o
obj-$(CONFIG_LEDS_NS2) += leds-ns2.o
obj-$(CONFIG_LEDS_NETXBIG) += leds-netxbig.o
+obj-$(CONFIG_LEDS_ASIC3) += leds-asic3.o

# LED SPI Drivers
obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o
diff -uprN clean-2.6.39-rc5/drivers/leds/leds-asic3.c linux-2.6.39-rc5/drivers/leds/leds-asic3.c
--- clean-2.6.39-rc5/drivers/leds/leds-asic3.c 1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.39-rc5/drivers/leds/leds-asic3.c 2011-04-30 16:39:41.066821912 +0100
@@ -0,0 +1,167 @@
+/*
+ * Copyright (C) 2011 Paul Parsons <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/leds.h>
+#include <linux/slab.h>
+
+#include <linux/mfd/asic3.h>
+#include <linux/mfd/core.h>
+
+/*
+ * The HTC ASIC3 LED GPIOs are inputs, not outputs.
+ * Hence we turn the LEDs on/off via the TimeBase register.
+ */
+
+/*
+ * When TimeBase is 4 the clock resolution is about 32Hz.
+ * This driver supports hardware blinking with an on+off
+ * period from 62ms (2 clocks) to 125s (4000 clocks).
+ */
+#define MS_TO_CLK(ms) ((((ms)*1024)+16000)/32000) /* Round to nearest */
+#define CLK_TO_MS(clk) (((clk)*32000)/1024)
+#define MAX_CLK 4000 /* Fits into 12-bit Time registers */
+#define MAX_MS CLK_TO_MS(MAX_CLK)
+
+static const unsigned int led_n_base[ASIC3_NUM_LEDS] = {
+ [0] = ASIC3_LED_0_Base,
+ [1] = ASIC3_LED_1_Base,
+ [2] = ASIC3_LED_2_Base,
+};
+
+static void brightness_set(struct led_classdev *cdev,
+ enum led_brightness value)
+{
+ struct platform_device *pdev = to_platform_device(cdev->dev->parent);
+ const struct mfd_cell *cell = mfd_get_cell(pdev);
+ struct asic3 *asic = dev_get_drvdata(pdev->dev.parent);
+ u32 timebase;
+ unsigned int base;
+
+ timebase = (value == LED_OFF) ? 0 : (LED_EN|0x4);
+
+ base = led_n_base[cell->id];
+ asic3_write_register(asic, (base + ASIC3_LED_PeriodTime), 32);
+ asic3_write_register(asic, (base + ASIC3_LED_DutyTime), 32);
+ asic3_write_register(asic, (base + ASIC3_LED_AutoStopCount), 0);
+ asic3_write_register(asic, (base + ASIC3_LED_TimeBase), timebase);
+}
+
+static int blink_set(struct led_classdev *cdev,
+ unsigned long *delay_on,
+ unsigned long *delay_off)
+{
+ struct platform_device *pdev = to_platform_device(cdev->dev->parent);
+ const struct mfd_cell *cell = mfd_get_cell(pdev);
+ struct asic3 *asic = dev_get_drvdata(pdev->dev.parent);
+ u32 on;
+ u32 off;
+ unsigned int base;
+
+ if (*delay_on > MAX_MS || *delay_off > MAX_MS)
+ return -EINVAL;
+
+ if (*delay_on == 0 && *delay_off == 0) {
+ /* If both are zero then a sensible default should be chosen */
+ on = MS_TO_CLK(500);
+ off = MS_TO_CLK(500);
+ } else {
+ on = MS_TO_CLK(*delay_on);
+ off = MS_TO_CLK(*delay_off);
+ if ((on + off) > MAX_CLK)
+ return -EINVAL;
+ }
+
+ base = led_n_base[cell->id];
+ asic3_write_register(asic, (base + ASIC3_LED_PeriodTime), (on + off));
+ asic3_write_register(asic, (base + ASIC3_LED_DutyTime), on);
+ asic3_write_register(asic, (base + ASIC3_LED_AutoStopCount), 0);
+ asic3_write_register(asic, (base + ASIC3_LED_TimeBase), (LED_EN|0x4));
+
+ *delay_on = CLK_TO_MS(on);
+ *delay_off = CLK_TO_MS(off);
+
+ return 0;
+}
+
+static int __devinit asic3_led_probe(struct platform_device *pdev)
+{
+ struct asic3_led *led = mfd_get_data(pdev);
+ int ret;
+
+ ret = mfd_cell_enable(pdev);
+ if (ret < 0)
+ goto ret0;
+
+ led->cdev = kzalloc(sizeof(struct led_classdev), GFP_KERNEL);
+ if (!led->cdev) {
+ ret = -ENOMEM;
+ goto ret1;
+ }
+
+ led->cdev->name = led->name;
+ led->cdev->default_trigger = led->default_trigger;
+ led->cdev->brightness_set = brightness_set;
+ led->cdev->blink_set = blink_set;
+
+ ret = led_classdev_register(&pdev->dev, led->cdev);
+ if (ret < 0)
+ goto ret2;
+
+ return 0;
+
+ret2:
+ kfree(led->cdev);
+ret1:
+ (void) mfd_cell_disable(pdev);
+ret0:
+ return ret;
+}
+
+static int __devexit asic3_led_remove(struct platform_device *pdev)
+{
+ struct asic3_led *led = mfd_get_data(pdev);
+
+ led_classdev_unregister(led->cdev);
+
+ kfree(led->cdev);
+
+ (void) mfd_cell_disable(pdev);
+
+ return 0;
+}
+
+static struct platform_driver asic3_led_driver = {
+ .probe = asic3_led_probe,
+ .remove = __devexit_p(asic3_led_remove),
+ .driver = {
+ .name = "leds-asic3",
+ .owner = THIS_MODULE,
+ },
+};
+
+MODULE_ALIAS("platform:leds-asic3");
+
+static int __init asic3_led_init(void)
+{
+ return platform_driver_register(&asic3_led_driver);
+}
+
+static void __exit asic3_led_exit(void)
+{
+ platform_driver_unregister(&asic3_led_driver);
+}
+
+module_init(asic3_led_init);
+module_exit(asic3_led_exit);
+
+MODULE_AUTHOR("Paul Parsons <[email protected]>");
+MODULE_DESCRIPTION("HTC ASIC3 LED driver");
+MODULE_LICENSE("GPL");


2011-05-03 11:19:33

by Philipp Zabel

[permalink] [raw]
Subject: Re: [PATCH 2/2] leds: Add ASIC3 LED support

Hi Paul,

Am Samstag, den 30.04.2011, 17:59 +0000 schrieb Paul Parsons:
> Add LED support for the HTC ASIC3. Underlying support is provided by the mfd/asic3 and leds/leds-asic3 drivers. An example configuration is provided by the pxa/hx4700 platform.
>
> Signed-off-by: Paul Parsons <[email protected]>
> ---
> diff -uprN clean-2.6.39-rc5/drivers/leds/Kconfig linux-2.6.39-rc5/drivers/leds/Kconfig
> --- clean-2.6.39-rc5/drivers/leds/Kconfig 2011-04-28 11:18:14.818578777 +0100
> +++ linux-2.6.39-rc5/drivers/leds/Kconfig 2011-04-30 16:39:48.434871549 +0100
> @@ -379,6 +379,16 @@ config LEDS_NETXBIG
> and 5Big Network v2 boards. The LEDs are wired to a CPLD and are
> controlled through a GPIO extension bus.
>
> +config LEDS_ASIC3
> + bool "LED support for the HTC ASIC3"
> + depends on MFD_ASIC3
> + default y
> + help
> + This option enables support for the LEDs on the HTC ASIC3. The HTC
> + ASIC3 LED GPIOs are inputs, not outputs, thus the leds-gpio driver
> + cannot be used. This driver supports hardware blinking with an on+off
> + period from 62ms to 125s. Say Y to enable LEDs on the HP iPAQ hx4700.
> +
> config LEDS_TRIGGERS
> bool "LED Trigger support"
> depends on LEDS_CLASS
> diff -uprN clean-2.6.39-rc5/drivers/leds/Makefile linux-2.6.39-rc5/drivers/leds/Makefile
> --- clean-2.6.39-rc5/drivers/leds/Makefile 2011-04-28 11:18:14.818578777 +0100
> +++ linux-2.6.39-rc5/drivers/leds/Makefile 2011-04-30 15:44:30.288890098 +0100
> @@ -42,6 +42,7 @@ obj-$(CONFIG_LEDS_DELL_NETBOOKS) += dell
> obj-$(CONFIG_LEDS_MC13783) += leds-mc13783.o
> obj-$(CONFIG_LEDS_NS2) += leds-ns2.o
> obj-$(CONFIG_LEDS_NETXBIG) += leds-netxbig.o
> +obj-$(CONFIG_LEDS_ASIC3) += leds-asic3.o
>
> # LED SPI Drivers
> obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o
> diff -uprN clean-2.6.39-rc5/drivers/leds/leds-asic3.c linux-2.6.39-rc5/drivers/leds/leds-asic3.c
> --- clean-2.6.39-rc5/drivers/leds/leds-asic3.c 1970-01-01 01:00:00.000000000 +0100
> +++ linux-2.6.39-rc5/drivers/leds/leds-asic3.c 2011-04-30 16:39:41.066821912 +0100
> @@ -0,0 +1,167 @@
> +/*
> + * Copyright (C) 2011 Paul Parsons <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/leds.h>
> +#include <linux/slab.h>
> +
> +#include <linux/mfd/asic3.h>
> +#include <linux/mfd/core.h>
> +
> +/*
> + * The HTC ASIC3 LED GPIOs are inputs, not outputs.
> + * Hence we turn the LEDs on/off via the TimeBase register.
> + */

About this, see the other patch I sent. This should work with the GPIOs
configured as outputs.

> +
> +/*
> + * When TimeBase is 4 the clock resolution is about 32Hz.
> + * This driver supports hardware blinking with an on+off
> + * period from 62ms (2 clocks) to 125s (4000 clocks).
> + */
> +#define MS_TO_CLK(ms) ((((ms)*1024)+16000)/32000) /* Round to nearest */

I'd just use DIV_ROUND_CLOSEST((ms)*1024, 32000) here.

> +#define CLK_TO_MS(clk) (((clk)*32000)/1024)
> +#define MAX_CLK 4000 /* Fits into 12-bit Time registers */
> +#define MAX_MS CLK_TO_MS(MAX_CLK)
> +
> +static const unsigned int led_n_base[ASIC3_NUM_LEDS] = {
> + [0] = ASIC3_LED_0_Base,
> + [1] = ASIC3_LED_1_Base,
> + [2] = ASIC3_LED_2_Base,
> +};

Could be moved into struct resource definitions in asic3.c.
Get to them with platform_get_resource() in asic3_led_probe().
After calculating the bus shift from the resource size, you have
everything you need to get rid of the asic3_write_register() export.

> +static void brightness_set(struct led_classdev *cdev,
> + enum led_brightness value)
> +{
> + struct platform_device *pdev = to_platform_device(cdev->dev->parent);
> + const struct mfd_cell *cell = mfd_get_cell(pdev);
> + struct asic3 *asic = dev_get_drvdata(pdev->dev.parent);
> + u32 timebase;
> + unsigned int base;
> +
> + timebase = (value == LED_OFF) ? 0 : (LED_EN|0x4);
> +
> + base = led_n_base[cell->id];
> + asic3_write_register(asic, (base + ASIC3_LED_PeriodTime), 32);
> + asic3_write_register(asic, (base + ASIC3_LED_DutyTime), 32);
> + asic3_write_register(asic, (base + ASIC3_LED_AutoStopCount), 0);
> + asic3_write_register(asic, (base + ASIC3_LED_TimeBase), timebase);
> +}
> +
> +static int blink_set(struct led_classdev *cdev,
> + unsigned long *delay_on,
> + unsigned long *delay_off)
> +{
> + struct platform_device *pdev = to_platform_device(cdev->dev->parent);
> + const struct mfd_cell *cell = mfd_get_cell(pdev);
> + struct asic3 *asic = dev_get_drvdata(pdev->dev.parent);
> + u32 on;
> + u32 off;
> + unsigned int base;
> +
> + if (*delay_on > MAX_MS || *delay_off > MAX_MS)
> + return -EINVAL;
> +
> + if (*delay_on == 0 && *delay_off == 0) {
> + /* If both are zero then a sensible default should be chosen */
> + on = MS_TO_CLK(500);
> + off = MS_TO_CLK(500);
> + } else {
> + on = MS_TO_CLK(*delay_on);
> + off = MS_TO_CLK(*delay_off);
> + if ((on + off) > MAX_CLK)
> + return -EINVAL;
> + }
> +
> + base = led_n_base[cell->id];
> + asic3_write_register(asic, (base + ASIC3_LED_PeriodTime), (on + off));
> + asic3_write_register(asic, (base + ASIC3_LED_DutyTime), on);
> + asic3_write_register(asic, (base + ASIC3_LED_AutoStopCount), 0);
> + asic3_write_register(asic, (base + ASIC3_LED_TimeBase), (LED_EN|0x4));
> +
> + *delay_on = CLK_TO_MS(on);
> + *delay_off = CLK_TO_MS(off);
> +
> + return 0;
> +}
> +
> +static int __devinit asic3_led_probe(struct platform_device *pdev)
> +{
> + struct asic3_led *led = mfd_get_data(pdev);
> + int ret;
> +
> + ret = mfd_cell_enable(pdev);
> + if (ret < 0)
> + goto ret0;
> +
> + led->cdev = kzalloc(sizeof(struct led_classdev), GFP_KERNEL);
> + if (!led->cdev) {
> + ret = -ENOMEM;
> + goto ret1;
> + }
> +
> + led->cdev->name = led->name;
> + led->cdev->default_trigger = led->default_trigger;
> + led->cdev->brightness_set = brightness_set;
> + led->cdev->blink_set = blink_set;
> +
> + ret = led_classdev_register(&pdev->dev, led->cdev);
> + if (ret < 0)
> + goto ret2;
> +
> + return 0;
> +
> +ret2:
> + kfree(led->cdev);
> +ret1:
> + (void) mfd_cell_disable(pdev);
> +ret0:
> + return ret;
> +}
> +
> +static int __devexit asic3_led_remove(struct platform_device *pdev)
> +{
> + struct asic3_led *led = mfd_get_data(pdev);
> +
> + led_classdev_unregister(led->cdev);
> +
> + kfree(led->cdev);
> +
> + (void) mfd_cell_disable(pdev);
> +
> + return 0;

Why not just return mfd_cell_disable(pdev);?

> +}
> +
> +static struct platform_driver asic3_led_driver = {
> + .probe = asic3_led_probe,
> + .remove = __devexit_p(asic3_led_remove),
> + .driver = {
> + .name = "leds-asic3",
> + .owner = THIS_MODULE,
> + },
> +};
> +
> +MODULE_ALIAS("platform:leds-asic3");
> +
> +static int __init asic3_led_init(void)
> +{
> + return platform_driver_register(&asic3_led_driver);
> +}
> +
> +static void __exit asic3_led_exit(void)
> +{
> + platform_driver_unregister(&asic3_led_driver);
> +}
> +
> +module_init(asic3_led_init);
> +module_exit(asic3_led_exit);
> +
> +MODULE_AUTHOR("Paul Parsons <[email protected]>");
> +MODULE_DESCRIPTION("HTC ASIC3 LED driver");
> +MODULE_LICENSE("GPL");
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

regards
Philipp

2011-05-03 20:25:22

by Paul Parsons

[permalink] [raw]
Subject: Re: [PATCH 2/2] leds: Add ASIC3 LED support

Hi Philipp,

--- On Tue, 3/5/11, Philipp Zabel <[email protected]> wrote:

> About this, see the other patch I sent. This should work
> with the GPIOs
> configured as outputs.
See my response on the other thread.

> I'd just use DIV_ROUND_CLOSEST((ms)*1024, 32000) here.
Will change.

> Could be moved into struct resource definitions in
> asic3.c.
> Get to them with platform_get_resource() in
> asic3_led_probe().
> After calculating the bus shift from the resource size, you
> have
> everything you need to get rid of the
> asic3_write_register() export.
I understand, but my feeling is that implementing a more generic mechanism will add complexity for little obvious gain. If the LED driver ever needs to be more generic then this can be revisited.

> Why not just return mfd_cell_disable(pdev);?
It wasn't always at the end of the function as it is now! Will change.

Regards,
Paul

2011-05-04 07:48:14

by Philipp Zabel

[permalink] [raw]
Subject: Re: [PATCH 2/2] leds: Add ASIC3 LED support

On Tue, May 3, 2011 at 10:25 PM, Paul Parsons <[email protected]> wrote:
> Hi Philipp,
>
> --- On Tue, 3/5/11, Philipp Zabel <[email protected]> wrote:
>
>> About this, see the other patch I sent. This should work
>> with the GPIOs
>> configured as outputs.
> See my response on the other thread.

Will do.

>> Could be moved into struct resource definitions in
>> asic3.c.
>> Get to them with platform_get_resource() in
>> asic3_led_probe().
>> After calculating the bus shift from the resource size, you
>> have
>> everything you need to get rid of the
>> asic3_write_register() export.
> I understand, but my feeling is that implementing a more generic mechanism will add complexity for little obvious gain. If the LED driver ever needs to be more generic then this can be revisited.

Fair enough. Looks like your feeling is right - de-inlining and
exporting asic3_read/write_register costs only ~300 bytes. iomem
resources + ioremap + bus shift calculation in leds-asic3.o are
probably bigger than that.

regards
Philipp