pxa/hx4700: Add hx4700 LED support

Add hx4700 LED support. LED hx4700:amber blinks while the battery is charging. LED hx4700:green lights when the battery is full. LED hx4700:blue is a placeholder for absent bluetooth/wlan support. They are enabled via CONFIG_LEDS_ASIC3, and can be tested via /sys/class/leds.

Signed-off-by Paul Parsons <[email protected]>

---
diff -uprN clean-2.6.39-rc4/arch/arm/mach-pxa/hx4700.c linux-2.6.39-rc4/arch/arm/mach-pxa/hx4700.c

arch/arm/mach-pxa/hx4700.c +1 -0

@@ -23,6 +23,7 @@
 #include <linux/gpio_keys.h>
 #include <linux/input.h>
 #include <linux/lcd.h>
+#include <linux/leds.h>
 #include <linux/mfd/htc-egpio.h>
 #include <linux/mfd/asic3.h>
 #include <linux/mtd/physmap.h>

arch/arm/mach-pxa/hx4700.c +23 -0

+	},
+	.led1 = {
+		.name = "hx4700:green",
+		.default_trigger = "ds2760-battery.0-full",
+		.cdev = NULL,
+		.clock = ASIC3_CLOCK_LED1,
+		.base = ASIC3_LED_1_Base,
+		.timebase = (LED_EN|0x0),
+		.periodtime = 16,
+		.dutytime = 16,
+		.autostopcount = 0,
+	},
+	.led2 = {
+		.name = "hx4700:blue",
+		.default_trigger = "hx4700-radio",
+		.cdev = NULL,
+		.clock = ASIC3_CLOCK_LED2,
+		.base = ASIC3_LED_2_Base,
+		.timebase = (LED_EN|0x0),
+		.periodtime = 16,
+		.dutytime = 16,
+		.autostopcount = 0,
+	},

arch/arm/mach-pxa/hx4700.c +0 -0

 	&pcmcia,
 };
 

diff -uprN clean-2.6.39-rc4/drivers/leds/Kconfig linux-2.6.39-rc4/drivers/leds/Kconfig

drivers/leds/Kconfig +7 -0

@@ -379,6 +379,13 @@ 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.
+
 config LEDS_TRIGGERS
 	bool "LED Trigger support"
 	depends on LEDS_CLASS

diff -uprN clean-2.6.39-rc4/drivers/leds/Makefile linux-2.6.39-rc4/drivers/leds/Makefile

drivers/leds/Makefile +1 -0

@@ -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-rc4/drivers/leds/leds-asic3.c linux-2.6.39-rc4/drivers/leds/leds-asic3.c

drivers/leds/leds-asic3.c +8 -0

@@ -0,0 +1,151 @@
+/*
+ *  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.
+ */
+

drivers/leds/leds-asic3.c +133 -0

+ *	Hence we turn the LEDs on/off via the TimeBase register.
+ */
+
+static void asic3_led_set(struct led_classdev *led_cdev, enum led_brightness value)
+{
+	struct platform_device *pdev = to_platform_device(led_cdev->dev->parent);
+	struct asic3 *asic = dev_get_drvdata(pdev->dev.parent);
+	struct asic3_led_platform_data *pdata = pdev->dev.platform_data;
+	struct asic3_led *led;
+	unsigned timebase;
+
+	if (led_cdev == pdata->led0.cdev)
+		led = &pdata->led0;
+	else if (led_cdev == pdata->led1.cdev)
+		led = &pdata->led1;
+	else if (led_cdev == pdata->led2.cdev)
+		led = &pdata->led2;
+	else
+		return;		/* NOTREACHED */
+
+	timebase = (value == LED_OFF) ? 0 : led->timebase;
+	asic3_write_register(asic, (led->base+ASIC3_LED_TimeBase), timebase);
+}
+
+static int __devinit led_init(
+	struct platform_device *pdev,
+	struct asic3 *asic,
+	struct asic3_led *led)
+{
+	int ret;
+
+	led->cdev = kzalloc(sizeof(struct led_classdev), GFP_KERNEL);
+	if (! led->cdev)
+		return (-ENOMEM);
+
+	led->cdev->name = led->name;
+	led->cdev->default_trigger = led->default_trigger;
+	led->cdev->brightness_set = asic3_led_set;
+
+	ret = led_classdev_register(&pdev->dev, led->cdev);
+	if (ret < 0) {
+		kfree(led->cdev);
+		return (ret);
+	}
+
+	asic3_clk_enable(asic, led->clock);
+
+	asic3_write_register(asic, (led->base+ASIC3_LED_PeriodTime), led->periodtime);
+	asic3_write_register(asic, (led->base+ASIC3_LED_DutyTime), led->dutytime);
+	asic3_write_register(asic, (led->base+ASIC3_LED_AutoStopCount), led->autostopcount);
+	asic3_write_register(asic, (led->base+ASIC3_LED_TimeBase), 0);	/* disable */
+
+	return (0);
+}
+
+static void led_term(struct asic3 *asic, struct asic3_led *led)
+{
+	asic3_write_register(asic, (led->base+ASIC3_LED_TimeBase), 0);	/* disable */
+
+	asic3_clk_disable(asic, led->clock);
+
+	led_classdev_unregister(led->cdev);
+
+	kfree(led->cdev);
+}
+
+static int __devinit asic3_led_probe(struct platform_device *pdev)
+{
+	struct asic3 *asic = dev_get_drvdata(pdev->dev.parent);
+	struct asic3_led_platform_data *pdata = pdev->dev.platform_data;
+	int ret;
+
+	ret = led_init(pdev, asic, &pdata->led0);
+	if (ret < 0)
+		goto out0;
+
+	ret = led_init(pdev, asic, &pdata->led1);
+	if (ret < 0)
+		goto out1;
+
+	ret = led_init(pdev, asic, &pdata->led2);
+	if (ret < 0)
+		goto out2;
+
+	return (ret);
+
+out2:
+	led_term(asic, &pdata->led1);
+out1:
+	led_term(asic, &pdata->led0);
+out0:
+	return (ret);
+}
+
+static int __devexit asic3_led_remove(struct platform_device *pdev)
+{
+	struct asic3 *asic = dev_get_drvdata(pdev->dev.parent);
+	struct asic3_led_platform_data *pdata = pdev->dev.platform_data;
+
+	led_term(asic, &pdata->led2);
+	led_term(asic, &pdata->led1);
+	led_term(asic, &pdata->led0);
+
+	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");

diff -uprN clean-2.6.39-rc4/drivers/mfd/asic3.c linux-2.6.39-rc4/drivers/mfd/asic3.c

drivers/mfd/asic3.c +0 -15

@@ -30,21 +30,6 @@
 #include <linux/mfd/ds1wm.h>
 #include <linux/mfd/tmio.h>
 
-enum {
-	ASIC3_CLOCK_SPI,
-	ASIC3_CLOCK_OWM,
-	ASIC3_CLOCK_PWM0,
-	ASIC3_CLOCK_PWM1,
-	ASIC3_CLOCK_LED0,
-	ASIC3_CLOCK_LED1,
-	ASIC3_CLOCK_LED2,
-	ASIC3_CLOCK_SD_HOST,
-	ASIC3_CLOCK_SD_BUS,
-	ASIC3_CLOCK_SMBUS,
-	ASIC3_CLOCK_EX0,
-	ASIC3_CLOCK_EX1,
-};
-
 struct asic3_clk {
 	int enabled;
 	unsigned int cdex;

@@ -88,19 +73,19 @@ struct asic3 {
 

drivers/mfd/asic3.c +7 -3

 {
+	struct asic3_clk *clk;
 	unsigned long flags;
 	u32 cdex;
 
+	clk = &asic->clocks[clock];
 	spin_lock_irqsave(&asic->lock, flags);
 	if (clk->enabled++ == 0) {
 		cdex = asic3_read_register(asic, ASIC3_OFFSET(CLOCK, CDEX));

@@ -596,15 +583,16 @@ static int asic3_clk_enable(struct asic3
 		asic3_write_register(asic, ASIC3_OFFSET(CLOCK, CDEX), cdex);
 	}
 	spin_unlock_irqrestore(&asic->lock, flags);
-
-	return 0;
 }
+EXPORT_SYMBOL_GPL(asic3_clk_enable);
 
-static void asic3_clk_disable(struct asic3 *asic, struct asic3_clk *clk)
+void asic3_clk_disable(struct asic3 *asic, enum asic3_clock clock)
 {
+	struct asic3_clk *clk;
 	unsigned long flags;
 	u32 cdex;
 
+	clk = &asic->clocks[clock];
 	WARN_ON(clk->enabled == 0);
 
 	spin_lock_irqsave(&asic->lock, flags);

@@ -615,6 +603,7 @@ static void asic3_clk_disable(struct asi
 	}
 	spin_unlock_irqrestore(&asic->lock, flags);
 }
+EXPORT_SYMBOL_GPL(asic3_clk_disable);
 
 /* MFD cells (SPI, PWM, LED, DS1WM, MMC) */
 static struct ds1wm_driver_data ds1wm_pdata = {

drivers/mfd/asic3.c +0 -0

 	msleep(1);
 
 	asic3_set_register(asic, ASIC3_OFFSET(EXTCF, SELECT),

@@ -766,10 +755,10 @@ static int asic3_mmc_disable(struct plat
 			   ASIC3_SDHWCTRL_SUSPEND, 1);

drivers/mfd/asic3.c +0 -0

 	asic->gpio.base = pdata->gpio_base;
 	asic->gpio.ngpio = ASIC3_NUM_GPIOS;
 	asic->gpio.get = asic3_gpio_get;

diff -uprN clean-2.6.39-rc4/include/linux/mfd/asic3.h linux-2.6.39-rc4/include/linux/mfd/asic3.h

include/linux/mfd/asic3.h +34 -0

 #define ASIC3_GPIOC3_SPI_RXD		ASIC3_CONFIG_GPIO(35, 1, 0, 0)
 #define ASIC3_GPIOC4_CF_nCD		ASIC3_CONFIG_GPIO(36, 1, 0, 0)
 #define ASIC3_GPIOC4_SPI_TXD		ASIC3_CONFIG_GPIO(36, 1, 1, 0)

@@ -171,6 +171,40 @@ struct asic3_platform_data {
 #define LED_AUTOSTOP	(1 << 5) /* LED ON/OFF auto stop 0:disable, 1:enable */
 #define LED_ALWAYS	(1 << 6) /* LED Interrupt Mask 0:No mask, 1:mask */
 
+struct led_classdev;
+struct asic3_led {
+	const char	*name;
+	const char	*default_trigger;
+	struct led_classdev *cdev;
+	int		clock;          /* ASIC3_CLOCK_LED? */
+	unsigned	base;           /* ASIC3_LED_?_Base */
+	unsigned 	timebase;       /* ASIC3_LED_TimeBase */
+	unsigned	periodtime;     /* ASIC3_LED_PeriodTime */
+	unsigned	dutytime;       /* ASIC3_LED_DutyTime */
+	unsigned	autostopcount;  /* ASIC3_LED_AutoStopCount */
+};
+
+struct asic3_led_platform_data {
+	struct asic3_led led0;          /* LED0 */
+	struct asic3_led led1;          /* LED1 */
+	struct asic3_led led2;          /* LED2 */
+};
+
+enum asic3_clock {
+	ASIC3_CLOCK_SPI,
+	ASIC3_CLOCK_OWM,
+	ASIC3_CLOCK_PWM0,
+	ASIC3_CLOCK_PWM1,
+	ASIC3_CLOCK_LED0,
+	ASIC3_CLOCK_LED1,
+	ASIC3_CLOCK_LED2,
+	ASIC3_CLOCK_SD_HOST,
+	ASIC3_CLOCK_SD_BUS,
+	ASIC3_CLOCK_SMBUS,
+	ASIC3_CLOCK_EX0,
+	ASIC3_CLOCK_EX1,
+};
+
 #define ASIC3_CLOCK_BASE	   0x0A00
 #define ASIC3_CLOCK_CDEX           0x00
 #define ASIC3_CLOCK_SEL            0x04

@@ -293,4 +327,12 @@ struct asic3_platform_data {
 #define ASIC3_MAP_SIZE_32BIT	0x2000
 #define ASIC3_MAP_SIZE_16BIT	0x1000

thanks for working on this! I have to offer my opinion on the overall
structure of the patch and hx4700 LED support:

The patch should be split in three parts, to be sent to the respective
subsystem maintainers - the necessary modifications to the asic3 driver
(mfd), the new leds-asic3 driver (led subsystem) and finally the hookup
in hx4700.c (pxa).

I think exporting the asic3_read/write_register() and asic3_clk_enable/disable() should be avoided, eventually. You can get rid of the asic3_read/write_register() exports by

implementing the leds-asic3 driver as a mfd_cell, symmetrical to how
ds1wm and tmio_mmc are hooked up to asic3.
The asic3_clk_enable/disable() exports we have to live with until people
agree on a clock framework with a common struct clk implementation, but
we should look forward to get rid of those, too.

I would like to keep #ifdefs to a minimum in hx4700.c. It shouldn't be
necessary to compile this conditionally.

That one's not needed.

This would be the same for all platforms using ASIC3 and thus should be
set up inside asic3.c.

Please do not hard-code the timebase, periodtime, dutytime and
autostopcount parameters in platform data. This should be hooked up to
led_blink_set(), really.

asic3_led_info should be hooked into asic3_platform_data, and leds-asic3
created from asic3.c as a mfd cell...
... and then this wouldn't be needed.

Are they?

I'd like to avoid this.

And this.

And this was meant to mimic clk_enable(struct clk *clk). I guess this is
fine for now. Let's keep in mind that eventually this is going return to
a static function taking a struct clk * once the common struct clk is in
place.

This will have to be reverted once there is a common struct clk.
And this.
And this.
And this.
And this.
And this.

Is this relevant to the led support?

Where did you get this information? I don't have WinCE+HaRET installed
right now, but I'm pretty sure that my old notes say that these were
configured as low outputs by default, and I believe the SDG Systems code
even triggered them as if to power the LEDs, although I'm not sure of
that right now.

Quoted 5 lines

Please do not hard-code the timebase, periodtime, dutytime and
autostopcount parameters in platform data. This should be hooked up to
led_blink_set(), really.

I just kept things simple. Will look into it.

Quoted 1 line

Is this relevant to the led support?

No, but it helps to confirm the mapping between platform and asic3 GPIOs. Other mfd drivers define the label so why not asic3? If you're saying that this should be submitted as a separate patch then fair enough.

Quoted 9 lines

Where did you get this information? I don't have WinCE+HaRET installed
right now, but I'm pretty sure that my old notes say that these were
configured as low outputs by default, and I believe the SDG Systems code
even triggered them as if to power the LEDs, although I'm not sure of
that right now.

I got it from observation. Writing the 3 LED GPIOs had no effect despite the options I tried. Reading them was a different story however; it always returned the current on/off state. For example:

# echo 162 > /sys/class/gpio/export    # platform #162 = asic3 #34
# cat /sys/class/gpio/gpio162/value
0
# echo 1 > /sys/class/gpio/gpio162/value    # has no effect
# cat /sys/class/gpio/gpio162/value
0
# echo 1 > /sys/class/leds/hx4700:blue/brightness    # turn on
# cat /sys/class/gpio/gpio162/value
1
# echo 0 > /sys/class/gpio/gpio162/value    # has no effect
# cat /sys/class/gpio/gpio162/value
1
# 
A more compelling illustration that the 3 LED GPIOs are configured as inputs comes from the amber LED, which blinks on / off for about one second each:
# echo 160 > /sys/class/gpio/export    # platform #160 = asic3 #32
# echo 1 > /sys/class/leds/hx4700:amber/brightness    # start blinking
# while true; do cat /sys/class/gpio/gpio160/value; sleep 1; done
1
0

1
0
1
0
1
0
etc...
If anyone can demonstrate how the 3 LED GPIOs can be used as outputs then I'll be happy to revisit this.

Add hx4700 LED support. LED hx4700:amber blinks while the battery is charging. LED hx4700:green lights when the battery is full. LED hx4700:blue is a placeholder for absent bluetooth/wlan support. They are enabled via CONFIG_LEDS_ASIC3, and can be tested via /sys/class/leds.