From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Rafael J. Wysocki" Subject: Re: [PATCH v7 1/1] leds: LED driver for TI LP3952 6-Channel Color LED Date: Wed, 29 Jun 2016 02:45:24 +0200 Message-ID: <3435169.nmqvdouPPq@vostro.rjw.lan> References: <1467129919-27641-1-git-send-email-tony.makkiel@daqri.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7Bit Return-path: In-Reply-To: <1467129919-27641-1-git-send-email-tony.makkiel@daqri.com> Sender: linux-acpi-owner@vger.kernel.org To: Tony Makkiel Cc: linux-leds@vger.kernel.org, j.anaszewski@samsung.com, rpurdie@rpsys.net, lenb@kernel.org, mika.westerberg@linux.intel.com, linux-acpi@vger.kernel.org List-Id: linux-leds@vger.kernel.org On Tuesday, June 28, 2016 05:05:19 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 > --- > drivers/leds/Kconfig | 13 ++ > drivers/leds/Makefile | 1 + > drivers/leds/leds-lp3952.c | 289 ++++++++++++++++++++++++++++++++++++++++++++ > include/linux/leds-lp3952.h | 128 ++++++++++++++++++++ > 4 files changed, 431 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..0f321f4 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -228,6 +228,19 @@ 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 > + 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..cec7374 > --- /dev/null > +++ b/drivers/leds/leds-lp3952.c > @@ -0,0 +1,289 @@ > +/* > + * LED driver for TI lp3952 controller > + * > + * Copyright (C) 2016, DAQRI, LLC. > + * Author: Tony Makkiel > + * > + * 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 > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +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, int 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, LED_OFF); > + 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, 1); > + > + return regmap_update_bits(priv->regmap, reg, 3 << shift_val, > + --value << shift_val); > +} > + > +static int lp3952_register_led_classdev(struct lp3952_led_array *priv) > +{ > + int i, ret = -ENODEV; > + static const char *led_name[LP3952_LED_ALL] = { > + "lp3952:blue2", > + "lp3952:green2", > + "lp3952:red2", > + "lp3952:blue1", > + "lp3952:green1", > + "lp3952:red1" > + }; > + > + for (i = 0; i < LP3952_LED_ALL; i++) { > + sprintf(priv->leds[i].name, "%s", led_name[i]); > + 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, NULL, 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, 0); > + 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[] = { > + {LP3952_ACPI_NAME, 0}, No, you can't use "PRP0001" in this list. > + {} > +}; > + > +MODULE_DEVICE_TABLE(acpi, lp3952_acpi_match); And you don't need this for the "PRP0001" thing to work. The core will take care of it for you then. > +#endif So the entire ACPI block can be dropped for now. And the driver doesn't have to depend on CONFIG_ACPI any more, does it? > + > +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 "); > +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..9808ab7 > --- /dev/null > +++ b/include/linux/leds-lp3952.h > @@ -0,0 +1,128 @@ > +/* > + * LED driver for TI lp3952 controller > + * > + * Copyright (C) 2016, DAQRI, LLC. > + * Author: Tony Makkiel > + * > + * 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_ > + > +/* ACPI iasl compiler wont take name LP3952, > + * "ASL_MSG_HID_LENGTH". Hence the name TLP3952 > + */ > +#define LP3952_NAME "lp3952" > +#define LP3952_ACPI_NAME "PRP0001" This is not necessary. > +#define LP3952_CMD_REG_COUNT 8 > +#define LP3952_BRIGHT_MAX 4 > + > +#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[15]; > + 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_ */ Thanks, Rafael