Re: [PATCH v10 1/1] leds: LED driver for TI LP3952 6-Channel Color LED

[Jacek Anaszewski <j.anaszewski@samsung.com>]

On 07/11/2016 12:48 PM, Tony Makkiel wrote:
> The chip can drive 2 sets of RGB leds. Controller can
> be controlled via PWM, I2C and audio synchronisation.
> This driver uses I2C to communicate with the chip.
>
> Datasheet: http://www.ti.com/lit/gpn/lp3952
>
> Signed-off-by: Tony Makkiel <tony.makkiel@daqri.com>
> ---
>
> Changes from v9:
>   - Add dependancy on GPIOLIB.
>   - make on/off variable of lp3952_on_off function bool.
>
> Changes from v8:
>   - Removed macro LP3952_ACPI_NAME.
>   - Replaced acpi_dev_get_property with device_property_read_string
>   - Skip led class registration if led data couldn't be read from
>     ACPI table.
>
>
>   drivers/leds/Kconfig        |  14 ++
>   drivers/leds/Makefile       |   1 +
>   drivers/leds/leds-lp3952.c  | 306 ++++++++++++++++++++++++++++++++++++++++++++
>   include/linux/leds-lp3952.h | 125 ++++++++++++++++++
>   4 files changed, 446 insertions(+)
>   create mode 100644 drivers/leds/leds-lp3952.c
>   create mode 100644 include/linux/leds-lp3952.h
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 5ae2834..9dcc9b1 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -228,6 +228,20 @@ config LEDS_LP3944
>   	  To compile this driver as a module, choose M here: the
>   	  module will be called leds-lp3944.
>
> +config LEDS_LP3952
> +	tristate "LED Support for TI LP3952 2 channel LED driver"
> +	depends on LEDS_CLASS
> +	depends on I2C
> +	depends on ACPI
> +	depends on GPIOLIB

Any specific reason for choosing "depends" instead of "select"?

> +	select REGMAP_I2C
> +	help
> +	  This option enables support for LEDs connected to the Texas
> +	  Instruments LP3952 LED driver.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called leds-lp3952.
> +
>   config LEDS_LP55XX_COMMON
>   	tristate "Common Driver for TI/National LP5521/5523/55231/5562/8501"
>   	depends on LEDS_LP5521 || LEDS_LP5523 || LEDS_LP5562 || LEDS_LP8501
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index cb2013d..0684c86 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_LEDS_PCA9532)		+= leds-pca9532.o
>   obj-$(CONFIG_LEDS_GPIO_REGISTER)	+= leds-gpio-register.o
>   obj-$(CONFIG_LEDS_GPIO)			+= leds-gpio.o
>   obj-$(CONFIG_LEDS_LP3944)		+= leds-lp3944.o
> +obj-$(CONFIG_LEDS_LP3952)		+= leds-lp3952.o
>   obj-$(CONFIG_LEDS_LP55XX_COMMON)	+= leds-lp55xx-common.o
>   obj-$(CONFIG_LEDS_LP5521)		+= leds-lp5521.o
>   obj-$(CONFIG_LEDS_LP5523)		+= leds-lp5523.o
> diff --git a/drivers/leds/leds-lp3952.c b/drivers/leds/leds-lp3952.c
> new file mode 100644
> index 0000000..c0eb2f4
> --- /dev/null
> +++ b/drivers/leds/leds-lp3952.c
> @@ -0,0 +1,306 @@
> +/*
> + *	LED driver for TI lp3952 controller
> + *
> + *	Copyright (C) 2016, DAQRI, LLC.
> + *	Author: Tony Makkiel <tony.makkiel@daqri.com>
> + *
> + * 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/acpi.h>
> +#include <linux/delay.h>
> +#include <linux/gpio.h>
> +#include <linux/i2c.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/leds.h>
> +#include <linux/leds-lp3952.h>
> +#include <linux/module.h>
> +#include <linux/notifier.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm.h>
> +#include <linux/reboot.h>
> +#include <linux/regmap.h>
> +
> +static int lp3952_register_write(struct i2c_client *client, u8 reg, u8 val)
> +{
> +	int ret;
> +	struct lp3952_led_array *priv = i2c_get_clientdata(client);
> +
> +	ret = regmap_write(priv->regmap, reg, val);
> +
> +	if (ret)
> +		dev_err(&client->dev, "%s: reg 0x%x, val 0x%x, err %d\n",
> +			__func__, reg, val, ret);
> +	return ret;
> +}
> +
> +static void lp3952_on_off(struct lp3952_led_array *priv,
> +			  enum lp3952_leds led_id, bool on)
> +{
> +	int ret, val;
> +
> +	dev_dbg(&priv->client->dev, "%s LED %d to %d\n", __func__, led_id, on);
> +
> +	val = 1 << led_id;
> +	if (led_id == LP3952_LED_ALL)
> +		val = LP3952_LED_MASK_ALL;
> +
> +	ret = regmap_update_bits(priv->regmap, LP3952_REG_LED_CTRL, val,
> +				 on ? val : 0);
> +	if (ret)
> +		dev_err(&priv->client->dev, "%s, Error %d\n", __func__, ret);
> +}
> +
> +/*
> + * Using Imax to control brightness. There are 4 possible
> + * setting 25, 50, 75 and 100 % of Imax. Possible values are
> + * values 0-4. 0 meaning turn off.
> + */
> +static int lp3952_set_brightness(struct led_classdev *cdev,
> +				 enum led_brightness value)
> +{
> +	unsigned int reg, shift_val;
> +	struct lp3952_ctrl_hdl *led = container_of(cdev,
> +						   struct lp3952_ctrl_hdl,
> +						   cdev);
> +	struct lp3952_led_array *priv = (struct lp3952_led_array *)led->priv;
> +
> +	dev_dbg(cdev->dev, "Brightness request: %d on %d\n", value,
> +		led->channel);
> +
> +	if (value == LED_OFF) {
> +		lp3952_on_off(priv, led->channel, false);
> +		return 0;
> +	}
> +
> +	if (led->channel > LP3952_RED_1) {
> +		dev_err(cdev->dev, " %s Invalid LED requested", __func__);
> +		return -EINVAL;
> +	}
> +
> +	if (led->channel >= LP3952_BLUE_1) {
> +		reg = LP3952_REG_RGB1_MAX_I_CTRL;
> +		shift_val = (led->channel - LP3952_BLUE_1) * 2;
> +	} else {
> +		reg = LP3952_REG_RGB2_MAX_I_CTRL;
> +		shift_val = led->channel * 2;
> +	}
> +
> +	/* Enable the LED in case it is not enabled already */
> +	lp3952_on_off(priv, led->channel, true);
> +
> +	return regmap_update_bits(priv->regmap, reg, 3 << shift_val,
> +				  --value << shift_val);
> +}
> +
> +static int lp3952_get_label(struct device *dev, const char *label, char *dest)
> +{
> +	int ret;
> +	const char *str;
> +
> +	ret = device_property_read_string(dev, label, &str);
> +	if (!ret)
> +		strncpy(dest, str, LP3952_LABEL_MAX_LEN);
> +
> +	return ret;
> +}
> +
> +static int lp3952_register_led_classdev(struct lp3952_led_array *priv)
> +{
> +	int i, acpi_ret, ret = -ENODEV;
> +	static const char *led_name_hdl[LP3952_LED_ALL] = {
> +		"blue2",
> +		"green2",
> +		"red2",
> +		"blue1",
> +		"green1",
> +		"red1"
> +	};
> +
> +	for (i = 0; i < LP3952_LED_ALL; i++) {
> +		acpi_ret = lp3952_get_label(&priv->client->dev, led_name_hdl[i],
> +					    priv->leds[i].name);
> +		if (acpi_ret)
> +			continue;
> +
> +		priv->leds[i].cdev.name = priv->leds[i].name;
> +		priv->leds[i].cdev.brightness = LED_OFF;
> +		priv->leds[i].cdev.max_brightness = LP3952_BRIGHT_MAX;
> +		priv->leds[i].cdev.brightness_set_blocking =
> +				lp3952_set_brightness;
> +		priv->leds[i].channel = i;
> +		priv->leds[i].priv = priv;
> +
> +		ret = devm_led_classdev_register(&priv->client->dev,
> +						 &priv->leds[i].cdev);
> +		if (ret < 0) {
> +			dev_err(&priv->client->dev,
> +				"couldn't register LED %s\n",
> +				priv->leds[i].cdev.name);
> +			break;
> +		}
> +	}
> +	return ret;
> +}
> +
> +static int lp3952_set_pattern_gen_cmd(struct lp3952_led_array *priv,
> +				      u8 cmd_index, u8 r, u8 g, u8 b,
> +				      enum lp3952_tt tt, enum lp3952_cet cet)
> +{
> +	int ret;
> +	struct ptrn_gen_cmd line = {
> +		.r = r,
> +		.g = g,
> +		.b = b,
> +		.cet = cet,
> +		.tt = tt
> +	};
> +
> +	if (cmd_index >= LP3952_CMD_REG_COUNT)
> +		return -EINVAL;
> +
> +	ret = lp3952_register_write(priv->client,
> +				    LP3952_REG_CMD_0 + cmd_index * 2,
> +				    line.bytes.msb);
> +	if (ret)
> +		return ret;
> +
> +	return (lp3952_register_write(priv->client,
> +				      LP3952_REG_CMD_0 + cmd_index * 2 + 1,
> +				      line.bytes.lsb));
> +}
> +
> +static int lp3952_configure(struct lp3952_led_array *priv)
> +{
> +	int ret;
> +
> +	/* Disable any LEDs on from any previous conf. */
> +	ret = lp3952_register_write(priv->client, LP3952_REG_LED_CTRL, 0);
> +	if (ret)
> +		return ret;
> +
> +	/* enable rgb patter, loop */
> +	ret = lp3952_register_write(priv->client, LP3952_REG_PAT_GEN_CTRL,
> +				    LP3952_PATRN_LOOP | LP3952_PATRN_GEN_EN);
> +	if (ret)
> +		return ret;
> +
> +	/* Update Bit 6 (Active mode), Select both Led sets, Bit [1:0] */
> +	ret = lp3952_register_write(priv->client, LP3952_REG_ENABLES,
> +				    LP3952_ACTIVE_MODE | LP3952_INT_B00ST_LDR);
> +	if (ret)
> +		return ret;
> +
> +	/* Set Cmd1 for RGB intensity,cmd and transition time */
> +	return (lp3952_set_pattern_gen_cmd(priv, 0, I46, I71, I100, TT0,
> +					   CET197));
> +}
> +
> +static const struct regmap_config lp3952_regmap = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = REG_MAX,
> +	.cache_type = REGCACHE_RBTREE,
> +};
> +
> +static int lp3952_probe(struct i2c_client *client,
> +			const struct i2c_device_id *id)
> +{
> +	int status;
> +	struct lp3952_ctrl_hdl *leds;
> +	struct lp3952_led_array *priv;
> +
> +	priv = devm_kzalloc(&client->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	leds = devm_kcalloc(&client->dev, LP3952_LED_ALL, sizeof(*leds),
> +			    GFP_KERNEL);
> +	if (!leds) {
> +		devm_kfree(&client->dev, priv);
> +		return -ENOMEM;
> +	}
> +
> +	priv->leds = leds;
> +	priv->client = client;
> +
> +	priv->enable_gpio = devm_gpiod_get(&client->dev, "nrst",
> +					   GPIOD_OUT_HIGH);
> +	if (IS_ERR(priv->enable_gpio)) {
> +		status = PTR_ERR(priv->enable_gpio);
> +		dev_err(&client->dev, "Failed to enable gpio: %d\n", status);
> +		return status;
> +	}
> +
> +	priv->regmap = devm_regmap_init_i2c(client, &lp3952_regmap);
> +	if (IS_ERR(priv->regmap)) {
> +		int err = PTR_ERR(priv->regmap);
> +
> +		dev_err(&client->dev, "Failed to allocate register map: %d\n",
> +			err);
> +		return err;
> +	}
> +
> +	i2c_set_clientdata(client, priv);
> +
> +	status = lp3952_configure(priv);
> +	if (status) {
> +		dev_err(&client->dev, "Probe failed. Device not found (%d)\n",
> +			status);
> +		return status;
> +	}
> +
> +	status = lp3952_register_led_classdev(priv);
> +	if (status) {
> +		dev_err(&client->dev, "Unable to register led_classdev: %d\n",
> +			status);
> +		return status;
> +	}
> +
> +	return 0;
> +}
> +
> +static int lp3952_remove(struct i2c_client *client)
> +{
> +	struct lp3952_led_array *priv;
> +
> +	priv = i2c_get_clientdata(client);
> +	lp3952_on_off(priv, LP3952_LED_ALL, false);
> +	gpiod_set_value(priv->enable_gpio, 0);
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id lp3952_id[] = {
> +	{LP3952_NAME, 0},
> +	{}
> +};
> +
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id lp3952_acpi_match[] = {
> +	{"TXNW3952", 0},
> +	{}
> +};
> +
> +MODULE_DEVICE_TABLE(acpi, lp3952_acpi_match);
> +#endif
> +
> +static struct i2c_driver lp3952_i2c_driver = {
> +	.driver = {
> +			.name = LP3952_NAME,
> +			.acpi_match_table = ACPI_PTR(lp3952_acpi_match),
> +	},
> +	.probe = lp3952_probe,
> +	.remove = lp3952_remove,
> +	.id_table = lp3952_id,
> +};
> +
> +module_i2c_driver(lp3952_i2c_driver);
> +
> +MODULE_AUTHOR("Tony Makkiel <tony.makkiel@daqri.com>");
> +MODULE_DESCRIPTION("lp3952 I2C LED controller driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/leds-lp3952.h b/include/linux/leds-lp3952.h
> new file mode 100644
> index 0000000..edd5ed6
> --- /dev/null
> +++ b/include/linux/leds-lp3952.h
> @@ -0,0 +1,125 @@
> +/*
> + *	LED driver for TI lp3952 controller
> + *
> + *	Copyright (C) 2016, DAQRI, LLC.
> + *	Author: Tony Makkiel <tony.makkiel@daqri.com>
> + *
> + * 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.
> + *
> + */
> +
> +#ifndef LEDS_LP3952_H_
> +#define LEDS_LP3952_H_
> +
> +#define LP3952_NAME                         "lp3952"
> +#define LP3952_CMD_REG_COUNT                8
> +#define LP3952_BRIGHT_MAX                   4
> +#define LP3952_LABEL_MAX_LEN                15
> +
> +#define LP3952_REG_LED_CTRL                 0x00
> +#define LP3952_REG_R1_BLNK_TIME_CTRL        0x01
> +#define LP3952_REG_R1_BLNK_CYCLE_CTRL       0x02
> +#define LP3952_REG_G1_BLNK_TIME_CTRL        0x03
> +#define LP3952_REG_G1_BLNK_CYCLE_CTRL       0x04
> +#define LP3952_REG_B1_BLNK_TIME_CTRL        0x05
> +#define LP3952_REG_B1_BLNK_CYCLE_CTRL       0x06
> +#define LP3952_REG_ENABLES                  0x0B
> +#define LP3952_REG_PAT_GEN_CTRL             0x11
> +#define LP3952_REG_RGB1_MAX_I_CTRL          0x12
> +#define LP3952_REG_RGB2_MAX_I_CTRL          0x13
> +#define LP3952_REG_CMD_0                    0x50
> +#define LP3952_REG_RESET                    0x60
> +#define REG_MAX                             LP3952_REG_RESET
> +
> +#define LP3952_PATRN_LOOP                   BIT(1)
> +#define LP3952_PATRN_GEN_EN                 BIT(2)
> +#define LP3952_INT_B00ST_LDR                BIT(2)
> +#define LP3952_ACTIVE_MODE                  BIT(6)
> +#define LP3952_LED_MASK_ALL                 0x3f
> +
> +/* Transition Time in ms */
> +enum lp3952_tt {
> +	TT0,
> +	TT55,
> +	TT110,
> +	TT221,
> +	TT422,
> +	TT885,
> +	TT1770,
> +	TT3539
> +};
> +
> +/* Command Execution Time in ms */
> +enum lp3952_cet {
> +	CET197,
> +	CET393,
> +	CET590,
> +	CET786,
> +	CET1180,
> +	CET1376,
> +	CET1573,
> +	CET1769,
> +	CET1966,
> +	CET2163,
> +	CET2359,
> +	CET2556,
> +	CET2763,
> +	CET2949,
> +	CET3146
> +};
> +
> +/* Max Current in % */
> +enum lp3952_colour_I_log_0 {
> +	I0,
> +	I7,
> +	I14,
> +	I21,
> +	I32,
> +	I46,
> +	I71,
> +	I100
> +};
> +
> +enum lp3952_leds {
> +	LP3952_BLUE_2,
> +	LP3952_GREEN_2,
> +	LP3952_RED_2,
> +	LP3952_BLUE_1,
> +	LP3952_GREEN_1,
> +	LP3952_RED_1,
> +	LP3952_LED_ALL
> +};
> +
> +struct lp3952_ctrl_hdl {
> +	struct led_classdev cdev;
> +	char name[LP3952_LABEL_MAX_LEN];
> +	enum lp3952_leds channel;
> +	void *priv;
> +};
> +
> +struct ptrn_gen_cmd {
> +	union {
> +		struct {
> +			u16 tt:3;
> +			u16 b:3;
> +			u16 cet:4;
> +			u16 g:3;
> +			u16 r:3;
> +		};
> +		struct {
> +			u8 lsb;
> +			u8 msb;
> +		} bytes;
> +	};
> +} __packed;
> +
> +struct lp3952_led_array {
> +	struct regmap *regmap;
> +	struct i2c_client *client;
> +	struct gpio_desc *enable_gpio;
> +	struct lp3952_ctrl_hdl *leds;
> +};
> +
> +#endif /* LEDS_LP3952_H_ */
>


-- 
Best regards,
Jacek Anaszewski