On 10/06/16 15:36, Jacek Anaszewski wrote: > On 06/10/2016 01:39 PM, Tony Makkiel wrote: >> >> >> On 10/06/16 09:26, Jacek Anaszewski wrote: >>> Hi Tony, >>> >>> On 06/09/2016 05:20 PM, Tony Makkiel wrote: >>>> >>>> >>>> On 09/06/16 13:11, Jacek Anaszewski wrote: >>>>> Hi Tony, >>>>> >>>>> Thanks for the updated patch. I have some comments in the code below. >>>>> >>>>> On 06/09/2016 12:46 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@xxxxxxxxx> >>>>>> --- >>>>>> drivers/leds/Kconfig | 12 ++ >>>>>> drivers/leds/Makefile | 1 + >>>>>> drivers/leds/leds-lp3952.c | 359 ++++++++++++++++++++++++++++++++++++++++++++ >>>>>> include/linux/leds-lp3952.h | 75 +++++++++ >>>>>> 4 files changed, 447 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..305faf9 100644 >>>>>> --- a/drivers/leds/Kconfig >>>>>> +++ b/drivers/leds/Kconfig >>>>>> @@ -228,6 +228,18 @@ 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 >>>>>> + 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..a621bda >>>>>> --- /dev/null >>>>>> +++ b/drivers/leds/leds-lp3952.c >>>>>> @@ -0,0 +1,359 @@ >>>>>> +/* >>>>>> + * Copyright (c) 2016, DAQRI, LLC. >>>>>> + * >>>>>> + * License Terms: GNU General Public License v2 >>>>> >>>>> You didn't address my remarks regarding GPL license >>>>> boilerplate. Is it intentional? >>>>> >>>> >>>> Sorry, I couldn't find the comment on license >>>> boiler plate. Can you please remind me what I missed? >>> >>> Ah, sorry I intended to ask for that but eventually forgot. >>> >>> Please use following boilerplate: >>> >>> 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. >>> >> Thank you, updated. >>>>>> + * >>>>>> + * leds-lp3952 - LED class driver for TI lp3952 controller >>>>>> + * >>>>>> + * Based on: >>>>>> + * leds-tlc9516 - LED class driver for TLC95XX series >>>>>> + * of I2C driven LED controllers from NXP >>>>>> + * >>>>>> + * Derived from leds-atmel-pwm, leds-bd2802 and initial PWM led 3952 >>>>>> + * driver written by Alex Feinman >>>>>> + * >>>>>> + * This program is distributed in the hope that it will be useful, >>>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >>>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>>>>> + * GNU General Public License for more details. >>>>>> + * >>>>>> + */ >>> >>> And drop this, because here you seem to mention GPL, and above >>> GPL v2. >>> >>>>>> +#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> >>>>>> + >>>>>> +#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_ACTIVE_MODE BIT(6) >>>>>> +#define LP3952_RGB_SEL_MASK (BIT(0) | BIT(1)) >>>>> >>>>> #define LP3952_RGB_SEL_MASK 0x03 >>>>> >>>>>> +#define LP3952_LED_MASK_ALL 0x3f >>>>> >>>>> Please put all the macro values in the same column. >>>> Will do >>>>> >>>>> Even though you don't use all features of the device, it is a good >>>>> practice to provide definitions for all bits and in all registers. >>>>> >>>>> Also, since you are providing header file, please move all macro >>>>> definitions and structures there. >>>> will do. >>>>> >>>>>> + >>>>>> +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 { >>>>>> + u8 ndev; >>>>>> + struct regmap *regmap; >>>>>> + struct i2c_client *client; >>>>>> + struct gpio_desc *enable_gpio; >>>>>> + struct ptrn_gen_cmd pgm[LP3952_CMD_REG_COUNT]; >>>>>> + struct lp3952_ctrl_hdl *leds; >>>>>> +}; >>>>>> + >>>>>> +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; >>>>>> + const char *led_name[] = { >>>>>> + "lp3952:blue2", >>>>>> + "lp3952:green2", >>>>>> + "lp3952:red2", >>>>>> + "lp3952:blue1", >>>>>> + "lp3952:green1", >>>>>> + "lp3952:red1" >>>>>> + }; >>>>>> + >>>>>> + for (i = 0; i < priv->ndev; 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; >>>>>> + >>>>>> + priv->pgm[cmd_index] = line; >>>>> >>>>> Why actually do you need pgm array? It is accessed only here during >>>>> assignment. >>>> >>>> The chip can support upto 8 commands. We are using only 1 command (Chip maintains >>>> settings of last command). But if somebody in the future wants to utilize the >>>> whole 8 command sets (say to, have custom lighting effects), they >>>> can program so using the pgm array(with this function). But for normal operation >>>> just 1 array/command would do. >>> >>> But currently it is completely useless - you're only assigning the value >>> here, and it seems not to be accessed in any other place. >> >> Yes, at present it is only called from 'lp3952_configure' to configure command 1. >> But it can be useful for any one who wants to update the other 7 commands(say >> using device_attribute). >> >> But if you prefer to remove the arrays please let me know. > > From what you're saying the function itself seems to be useful, > contrarily to the array. I have been missing the point all along, Sorry. The array was left for the show function in device_attribute. But as, it is not there yet, will remove the array, in next patch version. > >>> >>>>> >>>>>> + 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 = regmap_update_bits(priv->regmap, LP3952_REG_ENABLES, >>>>>> + LP3952_ACTIVE_MODE | LP3952_RGB_SEL_MASK, >>>>>> + LP3952_ACTIVE_MODE); >>>>> >>>>> Why LP3952_ACTIVE_MODE | LP3952_RGB_SEL_MASK? Mask argument should >>>>> match the bit field to be overwritten with the new value. >>>> >>>> We have to make sure 3 bits have specific values. >>>> >>>> Bit 6(LP3952_ACTIVE_MODE) needs to be high. >>>> Bit [1:0] decides which leds needs to be controlled. It needs to be set >>>> 0 for both LED sets to follow Pattern gen. >>> >>> regmap_update_bits() is useful if we have caching enabled, so as not to >>> be forced to remember the current register state, and update only >>> selected bits. In this case regmap_write() seems to be more suitable. >>> >> As you suggested in previous comment, I have enabled REGCACHE_RBTREE, >> for regmap now. > > But still the regmap_update_bits() is of help only when updating > register state and not when initializing it. Please change it to > regmap_write(). > Will do > >>>>>> + 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)); >>>>>> +} >>>>> >>>>> Could you explain please, why the pattern has to be set while only >>>>> fixed brightness setting is supported by the driver? >>>>> >>>> Note we are only setting Command 1 as mentioned in my comment above. >>>> Pattern gen loops on this command. This command sets the intensity >>>> and transition times. Without setting this register, the LEDs do >>>> not turn on. >>> >>> What role does the transition time play? Why is it required to be set >>> while we are interested in continuous LED glowing? >> >> I am not sure. I am assuming it is the on/off transition times. Without >> setting at least one of the commands, the chip does not respond to any >> led controls. > > ack. > >>> >>>>> Do you plan to add support for setting blinking patterns in the future? >>>> >>>> Not sure, The driver works as it is for normal led operations. >>>> >>>> This can be easily added using device_attribute with help of >>>> "lp3952_set_pattern_gen_cmd". >>>> >>>>> >>>>>> +static const struct regmap_config lp3952_regmap = { >>>>>> + .reg_bits = 8, >>>>>> + .val_bits = 8, >>>>>> + .max_register = REG_MAX, >>>>>> +}; >>>>> >>>>> You're not enabling regmap cache so regmap_update_bits(), will perform >>>>> I2C readout each time. Is it intentional? >>>>> >>>> Thank you for the pointer. I did not look into it. Did not expect >>>> lot of traffic unless there is software blink. >>>> I will find out more about caching mechanism and add an >>>> appropriate cache type. >>>> >>>>>> +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; >>>>>> + >>>>>> + dev_dbg(&client->dev, "%s\n", __func__); >>>>>> + >>>>>> + priv = devm_kzalloc(&client->dev, sizeof(*priv), GFP_KERNEL); >>>>>> + if (!priv) >>>>>> + return -ENOMEM; >>>>>> + >>>>>> + priv->ndev = LP3952_LED_ALL; >>>>>> + >>>>>> + leds = devm_kcalloc(&client->dev, priv->ndev, 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); >>>>> >>>>> How the driver knows which GPIO should it acquire? You're passing NULL >>>>> as the second argument. >>>> >>>> In the ACPI asl file, there is only one GPIO associated for lp3952 node. >>>> >>>> GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, >>>> IoRestrictionOutputOnly, "\\_SB.GPO2", >>>> 0x00, ResourceConsumer, , ) >>> >>> Doesn't devm_gpiod_get need to be passed some identifier if only one >>> GPIO is provided in the ACPI asl file? How would it look like in case >>> there was more then one GPIO provided in the file? >>> >> >> It works without name, for 1 gpio. >> >> If there were more, one of the 2 methods in >> Documentation/acpi/gpio-properties.txt >> should be used. > > Thanks for the reference. > >>>>> >>>>>> + 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_NAME, 0}, >>>>>> + {} >>>>>> +}; >>>>> >>>>> Could you please share how did you apply ACPI overlay? >>>> >>>> I am using the initrd ACPI method of Octavian's patch. >>>> >>>> https://lkml.org/lkml/2016/3/31/333 >>> >>> Thanks. Would it be possible to define entries per each LED >>> connected to the LED controller, similarly as it is in case >>> of Device Tree bindings?: >>> >>> Documentation/devicetree/bindings/leds/common.txt >>> >> >> I am not sure. I did a quick skim through ACPI Spec 5.0. >> But couldn't find anything. >> >>>>>> + >>>>>> +MODULE_DEVICE_TABLE(acpi, lp3952_acpi_match); >>>>>> +#endif >>>>>> + >>>>>> +static struct i2c_driver lp3952_i2c_driver = { >>>>>> + .driver = { >>>>>> + .name = LP3952_NAME, >>>>>> + .owner = THIS_MODULE, >>>>>> +#ifdef CONFIG_ACPI >>>>>> + .acpi_match_table = ACPI_PTR(lp3952_acpi_match), >>>>>> +#endif >>>>>> + }, >>>>>> + .probe = lp3952_probe, >>>>>> + .remove = lp3952_remove, >>>>>> + .id_table = lp3952_id, >>>>>> +}; >>>>>> + >>>>>> +module_i2c_driver(lp3952_i2c_driver); >>>>>> + >>>>>> +MODULE_AUTHOR("Tony Makkiel <tony.makkiel@xxxxxxxxx>"); >>>>>> +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..3ea120a >>>>>> --- /dev/null >>>>>> +++ b/include/linux/leds-lp3952.h >>>>>> @@ -0,0 +1,75 @@ >>>>>> +/* >>>>>> + * Copyright (C) 2016, DAQRI LLC >>>>>> + * >>>>>> + * License Terms: GPL v2 >>>>>> + * >>>>>> + * TI lp3952 Controller driver >>>>>> + * >>>>>> + * Author: Tony Makkiel <tony.makkiel@xxxxxxxxx> >>>>>> + * >>>>>> + */ >>>>>> + >>>>>> +#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 "TLP3952" >>>>>> +#define LP3952_CMD_REG_COUNT 8 >>>>>> +#define LP3952_BRIGHT_MAX 4 >>>>>> + >>>>>> +/* 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 >>>>>> +}; >>>>>> + >>>>>> +#endif /* LEDS_LP3952_H_ */ >>>>>> >>>>> >>>>> >>>> >>>> >>> >>> >> >> > > -- To unsubscribe from this list: send the line "unsubscribe linux-leds" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
