* [PATCH v10 1/1] leds: LED driver for TI LP3952 6-Channel Color LED
@ 2016-07-11 10:48 Tony Makkiel
2016-07-11 11:25 ` Jacek Anaszewski
0 siblings, 1 reply; 6+ messages in thread
From: Tony Makkiel @ 2016-07-11 10:48 UTC (permalink / raw)
To: linux-leds
Cc: Tony Makkiel, linux-acpi, j.anaszewski, rpurdie, rjw, lenb,
mika.westerberg
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
+ 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_ */
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v10 1/1] leds: LED driver for TI LP3952 6-Channel Color LED
2016-07-11 10:48 [PATCH v10 1/1] leds: LED driver for TI LP3952 6-Channel Color LED Tony Makkiel
@ 2016-07-11 11:25 ` Jacek Anaszewski
2016-07-11 11:39 ` Tony
0 siblings, 1 reply; 6+ messages in thread
From: Jacek Anaszewski @ 2016-07-11 11:25 UTC (permalink / raw)
To: Tony Makkiel; +Cc: linux-leds, linux-acpi, rpurdie, rjw, lenb, mika.westerberg
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v10 1/1] leds: LED driver for TI LP3952 6-Channel Color LED
2016-07-11 11:25 ` Jacek Anaszewski
@ 2016-07-11 11:39 ` Tony
2016-07-11 12:08 ` Jacek Anaszewski
0 siblings, 1 reply; 6+ messages in thread
From: Tony @ 2016-07-11 11:39 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: linux-leds, linux-acpi, rpurdie, rjw, lenb, mika.westerberg
On 11/07/16 12:25, Jacek Anaszewski wrote:
> 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 causes "recursive dependency error".
drivers/gpio/Kconfig:34:error: recursive dependency detected!
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/gpio/Kconfig:34: symbol GPIOLIB is selected by LEDS_LP3952
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/leds/Kconfig:231: symbol LEDS_LP3952 depends on NEW_LEDS
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/leds/Kconfig:8: symbol NEW_LEDS is selected by BCMA_DRIVER_GPIO
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/bcma/Kconfig:97: symbol BCMA_DRIVER_GPIO depends on GPIOLIB
>
>> + 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_ */
>>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v10 1/1] leds: LED driver for TI LP3952 6-Channel Color LED
2016-07-11 11:39 ` Tony
@ 2016-07-11 12:08 ` Jacek Anaszewski
2016-07-11 12:20 ` Tony
0 siblings, 1 reply; 6+ messages in thread
From: Jacek Anaszewski @ 2016-07-11 12:08 UTC (permalink / raw)
To: Tony; +Cc: linux-leds, linux-acpi, rpurdie, rjw, lenb, mika.westerberg
On 07/11/2016 01:39 PM, Tony wrote:
>
>
> On 11/07/16 12:25, Jacek Anaszewski wrote:
>> 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 causes "recursive dependency error".
>
>
> drivers/gpio/Kconfig:34:error: recursive dependency detected!
> For a resolution refer to Documentation/kbuild/kconfig-language.txt
> subsection "Kconfig recursive dependency limitations"
> drivers/gpio/Kconfig:34: symbol GPIOLIB is selected by LEDS_LP3952
> For a resolution refer to Documentation/kbuild/kconfig-language.txt
> subsection "Kconfig recursive dependency limitations"
> drivers/leds/Kconfig:231: symbol LEDS_LP3952 depends on NEW_LEDS
> For a resolution refer to Documentation/kbuild/kconfig-language.txt
> subsection "Kconfig recursive dependency limitations"
> drivers/leds/Kconfig:8: symbol NEW_LEDS is selected by BCMA_DRIVER_GPIO
> For a resolution refer to Documentation/kbuild/kconfig-language.txt
> subsection "Kconfig recursive dependency limitations"
> drivers/bcma/Kconfig:97: symbol BCMA_DRIVER_GPIO depends on GPIOLIB
I didn't get this with my configuration, but it seems that it is
likely to happen. Let's abide by "depends on" then.
Besides, some redundant brackets are left, see below.
>>
>>> + 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));
These brackets are redundant.
>>> +}
>>> +
>>> +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));
Ditto.
If you agree I can remove them and take the patch.
>>> +}
>>> +
>>> +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_ */
>>>
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-leds" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
--
Best regards,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v10 1/1] leds: LED driver for TI LP3952 6-Channel Color LED
2016-07-11 12:08 ` Jacek Anaszewski
@ 2016-07-11 12:20 ` Tony
2016-07-11 13:10 ` Jacek Anaszewski
0 siblings, 1 reply; 6+ messages in thread
From: Tony @ 2016-07-11 12:20 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: linux-leds, linux-acpi, rpurdie, rjw, lenb, mika.westerberg
Thank you for the comments Jacek.
On 11/07/16 13:08, Jacek Anaszewski wrote:
>>>> @@ -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 causes "recursive dependency error".
>>
>>
>> drivers/gpio/Kconfig:34:error: recursive dependency detected!
>> For a resolution refer to Documentation/kbuild/kconfig-language.txt
>> subsection "Kconfig recursive dependency limitations"
>> drivers/gpio/Kconfig:34: symbol GPIOLIB is selected by LEDS_LP3952
>> For a resolution refer to Documentation/kbuild/kconfig-language.txt
>> subsection "Kconfig recursive dependency limitations"
>> drivers/leds/Kconfig:231: symbol LEDS_LP3952 depends on NEW_LEDS
>> For a resolution refer to Documentation/kbuild/kconfig-language.txt
>> subsection "Kconfig recursive dependency limitations"
>> drivers/leds/Kconfig:8: symbol NEW_LEDS is selected by
>> BCMA_DRIVER_GPIO
>> For a resolution refer to Documentation/kbuild/kconfig-language.txt
>> subsection "Kconfig recursive dependency limitations"
>> drivers/bcma/Kconfig:97: symbol BCMA_DRIVER_GPIO depends on GPIOLIB
>
> I didn't get this with my configuration, but it seems that it is
> likely to happen. Let's abide by "depends on" then.
>
> Besides, some redundant brackets are left, see below.
>
>>>
>>>> + 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));
>
> These brackets are redundant.
>
>>>> +}
>>>> +
>>>> +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));
>
> Ditto.
>
> If you agree I can remove them and take the patch.
Yes please :). Thank you for your help.
>
>>>> +}
>>>> +
>>>> +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_ */
>>>>
>>>
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-leds" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v10 1/1] leds: LED driver for TI LP3952 6-Channel Color LED
2016-07-11 12:20 ` Tony
@ 2016-07-11 13:10 ` Jacek Anaszewski
0 siblings, 0 replies; 6+ messages in thread
From: Jacek Anaszewski @ 2016-07-11 13:10 UTC (permalink / raw)
To: Tony; +Cc: linux-leds, linux-acpi, rpurdie, rjw, lenb, mika.westerberg
On 07/11/2016 02:20 PM, Tony wrote:
>
> Thank you for the comments Jacek.
[...]
>>>>> +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));
>>
>> These brackets are redundant.
>>
>>>>> +}
>>>>> +
>>>>> +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));
>>
>> Ditto.
>>
>> If you agree I can remove them and take the patch.
>
> Yes please :). Thank you for your help.
Patch applied to the for-next branch of linux-leds.git.
Thank you for your effort and determination in submitting
subsequent versions.
--
Best regards,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-07-11 13:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-11 10:48 [PATCH v10 1/1] leds: LED driver for TI LP3952 6-Channel Color LED Tony Makkiel
2016-07-11 11:25 ` Jacek Anaszewski
2016-07-11 11:39 ` Tony
2016-07-11 12:08 ` Jacek Anaszewski
2016-07-11 12:20 ` Tony
2016-07-11 13:10 ` Jacek Anaszewski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).