Driver for TI tlc59116 16 Channel i2c LED driver

This patchset is a driver for the TI tlc59116 16 Channel i2c LED driver. This driver is used on the Belkin WRT1900AC access point and the C code is derived from code Belkin contributed to OpenWRT. However it has been extensively re-written, and a device tree binding added to replace platform data.

Since v2:
Remove incorrect /* Mode register ? */ comment Parenthesis around the macro arguments Converted many signed variables into unsigned Saved an initialization

Since v1:
s/uint8_t/u8/g
Remove empty line
Removed #gpio-cells
Added select REGMAP_I2C
Sorted #includes into alphabetic order
Added missing MODULE_DEVICE_TABLE(of, ...)
Check return value of regmap_write()
Simplified tlc59116_set_mode()

Andrew Lunn (2):
leds: tlc59116: Document binding for the TI 16 Channel i2c LED driver
leds: tlc59116: Driver for the TI 16 Channel i2c LED driver

 .../devicetree/bindings/leds/leds-tlc59116.txt     |  41 ++++
 drivers/leds/Kconfig                               |   7 +
 drivers/leds/Makefile                              |   1 +
 drivers/leds/leds-tlc59116.c                       | 253 +++++++++++++++++++++
 4 files changed, 302 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-tlc59116.txt
 create mode 100644 drivers/leds/leds-tlc59116.c

2.1.3

Document the binding for the TLC59116 LED driver.

---
 .../devicetree/bindings/leds/leds-tlc59116.txt     | 40 ++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-tlc59116.txt

new file mode 100644
index 000000000000..45c2123774d3
@@ -0,0 +1,40 @@
+LEDs connected to tcl59116
+
+Required properties
+- compatible: should be "ti,tlc59116"
+- #address-cells: must be 1
+- #size-cells: must be 0
+- reg: typically 0x68
+
+Each led is represented as a sub-node of the ti,,tlc59116.
+See Documentation/devicetree/bindings/leds/common.txt
+
+LED sub-node properties:
+- reg: number of LED line, 0 to 15
+- label: (optional) name of LED
+- linux,default-trigger : (optional)
+
+Examples:
+
+tlc59116 at 68 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	compatible = "ti,tlc59116";
+	reg = <0x68>;
+
+	wan at 0 {
+		label = "wrt1900ac:amber:wan";
+		reg = <0x0>;
+	};
+
+	2g at 2 {
+		label = "wrt1900ac:white:2g";
+		reg = <0x2>;
+	};
+
+	alive at 9 {
+		label = "wrt1900ac:green:alive";
+		reg = <0x9>;
+		linux,default_trigger = "heartbeat";
+	};
+};

2.1.4

The TLC59116 is an I2C bus controlled 16-channel LED driver. Each LED output has its own 8-bit fixed-frequency PWM controller to control the brightness of the LED.

This is based on a driver from Belkin, but has been extensively rewritten.

---
 drivers/leds/Kconfig         |   8 ++
 drivers/leds/Makefile        |   1 +
 drivers/leds/leds-tlc59116.c | 252 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 261 insertions(+)
 create mode 100644 drivers/leds/leds-tlc59116.c

index a6c3d2f153f3..8fc283c01673 100644
@@ -457,6 +457,14 @@ config LEDS_TCA6507
 	  LED driver chips accessed via the I2C bus.
 	  Driver support brightness control and hardware-assisted blinking.
 
+config LEDS_TLC59116
+	tristate "LED driver for TLC59116F controllers"
+	depends on LEDS_CLASS && I2C
+	select REGMAP_I2C
+	help
+	  This option enables support for Texas Instruments TLC59116F
+	  LED controller.
+
 config LEDS_MAX8997
 	tristate "LED support for MAX8997 PMIC"
 	depends on LEDS_CLASS && MFD_MAX8997

index 1c65a191d907..8e7d20acfa7b 100644
@@ -30,6 +30,7 @@ obj-$(CONFIG_LEDS_LP8501)		+= leds-lp8501.o
 obj-$(CONFIG_LEDS_LP8788)		+= leds-lp8788.o
 obj-$(CONFIG_LEDS_LP8860)		+= leds-lp8860.o
 obj-$(CONFIG_LEDS_TCA6507)		+= leds-tca6507.o
+obj-$(CONFIG_LEDS_TLC59116)		+= leds-tlc59116.o
 obj-$(CONFIG_LEDS_CLEVO_MAIL)		+= leds-clevo-mail.o
 obj-$(CONFIG_LEDS_IPAQ_MICRO)		+= leds-ipaq-micro.o
 obj-$(CONFIG_LEDS_HP6XX)		+= leds-hp6xx.o

2.1.4

Hi,

We have a TLC59108 on one of our boards, and with a quick glance it looks about the same as TLC59116, except the amount of outputs. Vignesh wrote a driver for it and was about to send it for review.

However, Vignesh implemented it as a PWM driver. We use it for LCD backlight (via pwm-backlight).

I'm not very familiar with LED and PWM drivers, but doesn't implementing this as a LED driver prevent us from using it as a backlight? Whereas it looks like PWM can be used as a led via pwm-leds (I think).

Tomi

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150116/dcd6c719/attachment.sig>

Hi Tomi

I've no idea about backlighting....

But the driver does fully implement brightness. So on my board:

echo 128 > /sys/class/leds/wrt1900ac\:white\:wan/brightness

will set that LED to 128/256 brightness. You have 255 steps of brightness.

The reason i did not model it as a PWM, is that it cannot fulfil the PWM interface. The configure interface is:

int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns);

However with this device, you have no control over the period. It is fixed, from a 97-kHz clock. All you can change is the duty as a ratio between 0 and 256. There is the group PWM, where you do have control over the period. But as the name implies, this PWM is shared for all outputs, which is not what the PWM interface expects, it wants to be able to control them individually. Also, it only has a range of 24 Hz to 10.73 s. Again, i have no idea about backlights, but i suspect if it is driven at 24Hz, i'm going to get a headache.

The LED interface can be fully implemented. So that is what i have done.

So i think modeling this as an LED driver is correct, and maybe you need to implemented an led_bl.c driver?

Andrew

I understand that period of TLC chip cannot be changed and hence cannot fully implement PWM interface. But, suppose I want to control brightness of an LCD screen, with your current design, my LCD driver can never can do something like:

	pwm_get(chip);
	pwm_update_brightness();
	pwm_remove();

I will always have to rely on sysfs entries to control brightness.

If the driver is implemented as pwm-backlight, then brightness can be controlled both via sysfs entries and pwm-core callbacks(in kernel).

But, as far as I understand, leds-pwm.c does almost similar function.

Regards
Vignesh R

O.K, so that is agreed.

Or you can use the kernel API for controlling leds. There is a hint in

Documentation/devicetree/bindings/leds/common.txt

- linux,default-trigger : This parameter, if present, is a string defining the trigger assigned to the LED. Current triggers are:
"backlight" - LED will act as a back-light, controlled by the framebuffer
system

So somebody has at least thought about this, and there is

drivers/leds/trigger/ledtrig-backlight.c

What i also find interesting is:

drivers/video/backlight/adp8860_bl.c
drivers/video/backlight/adp8870_bl.c
drivers/video/backlight/lm3639_bl.c

all implement an led_class driver. So it does seem some people think backlight LEDs can be models using the led class.

Andrew

Hi Tomi

I took a look at the TLC59108. The hardware designs have not made it easy, but it does look similar enough that my driver can be extended to cover both. These extensions It will not change the DT binding, it will just gain a new compatibility string. So i would prefer my driver is first fully reviewed and accepted, and then i will take a stab at extending it for the TLC59108. I will need help testing it, but i hope you can help there.

Andrew

Ok.

I'm not familiar with the LED/PWM frameworks, so I want to summarize our use case and how I guess this should be done:

We use the TLC59108 to provide backlight to a LCD, but in addition to that, it is used as a GPIO expander (or GPO, as it cannot do input). In your earlier patch versions there were some 'gpio' leftovers. Does your board have such GPIO use also?

So my current thinking is that TLC591xx should be a LED driver (as it sounded to me that PWM is not quite suitable for it). On top of that, we need generic 'led-backlight' and 'led-gpio' drivers, each of which uses the given LED driver to do the hardware manipulation, and they expose a backlight device and gpio device, respectively.

Another option would be an mfd device, but that doesn't sound good as I think the backlight and gpio functionality here is not related to TLC591xx as such, but to a LED device in general.

Does this sound sane?

Tomi

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150120/b78d1571/attachment.sig>

No, only LEDs.

That sounds sensible. led-backlight seems to be mostly implemented already via the led trigger code. Adding a minimal backlight_ops does not look too hard. You might also be able to do some cleanup of the other led based backlight drivers.

led-gpio looks like more work, but i don't see why it should not be possible. One thing i do need to check is that if the brightness is set to 255 the output is not set to 255/256 on and you have a regular glitch. For an LED that does not really matter, but for GPIO it would not be good.

Andrew

Right. TLC591xx has the constant output mode which should be used for GPIO. Perhaps that mode could be always used when the brightness is turned to maximum. I haven't read the spec carefully enough to know if there are some downsides.

Tomi

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150120/07a8a6d4/attachment.sig>

The original Belkin code did use that mode for full brightness. But it added complexity to the code which i did not see a need for. Now you need gpio support, i can add it back.

Andrew

is set to 255/256?

Indeed.

Gr{oetje,eeting}s,

Geert, who had a quick look at the data sheet recently

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds