From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tony Makkiel Subject: Re: [PATCH 1/1] leds: LED driver for TI LP3952 6-Channel Color LED Date: Tue, 31 May 2016 16:31:01 +0100 Message-ID: <574DAE35.1050002@daqri.com> References: <1464258368-20021-1-git-send-email-tony.makkiel@daqri.com> <574C374E.3070205@samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Received: from mail-wm0-f44.google.com ([74.125.82.44]:37919 "EHLO mail-wm0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754528AbcEaPbH (ORCPT ); Tue, 31 May 2016 11:31:07 -0400 Received: by mail-wm0-f44.google.com with SMTP id n129so113332762wmn.1 for ; Tue, 31 May 2016 08:31:05 -0700 (PDT) In-Reply-To: <574C374E.3070205@samsung.com> Sender: linux-leds-owner@vger.kernel.org List-Id: linux-leds@vger.kernel.org To: Jacek Anaszewski Cc: Linux LED Subsystem Thank you for the comments Jacek. I have done all the changes suggested= ,=20 except some on which, needed some clarification please. On 30/05/16 13:51, Jacek Anaszewski wrote: > Hi Tony, >=20 > Thanks for the patch. Please refer to my comments in the code. >=20 > Please address also all issues filed by scripts/checkpatch.pl --stric= t. >=20 > On 05/26/2016 12:26 PM, Tony Makkiel wrote: >> Datasheet: http://www.ti.com/lit/gpn/lp3952 >=20 > Please put this line at the end of the commit message. >=20 >> >> The chip can drive 2 sets of RGB leds. Controller can >> be controlled via PWM, I2C and audio sychnronisation. >=20 > s/sychnronisation/synchronisation/ >=20 >> This driver use I2C to communicate with chip. >=20 > s/use/uses/ > s/chip/the chip/ >=20 >> >> Signed-off-by: Tony Makkiel >> --- >> drivers/leds/Kconfig | 12 ++ >> drivers/leds/Makefile | 1 + >> drivers/leds/leds-lp3952.c | 411 ++++++++++++++++++++++++++++++++= ++++++++++++ >> include/linux/leds-lp3952.h | 89 ++++++++++ >> 4 files changed, 513 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_L= P8501 >> 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) +=3D leds-pca953= 2.o >> obj-$(CONFIG_LEDS_GPIO_REGISTER) +=3D leds-gpio-register.o >> obj-$(CONFIG_LEDS_GPIO) +=3D leds-gpio.o >> obj-$(CONFIG_LEDS_LP3944) +=3D leds-lp3944.o >> +obj-$(CONFIG_LEDS_LP3952) +=3D leds-lp3952.o >> obj-$(CONFIG_LEDS_LP55XX_COMMON) +=3D leds-lp55xx-common.o >> obj-$(CONFIG_LEDS_LP5521) +=3D leds-lp5521.o >> obj-$(CONFIG_LEDS_LP5523) +=3D leds-lp5523.o >> diff --git a/drivers/leds/leds-lp3952.c b/drivers/leds/leds-lp3952.c >> new file mode 100644 >> index 0000000..8229d5a >> --- /dev/null >> +++ b/drivers/leds/leds-lp3952.c >> @@ -0,0 +1,411 @@ >> +/* >> + * Copyright (c) 2016, DAQRI, LLC. >> + * >> + * License Terms: GNU General Public License v2 >> + * >> + * 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 395= 2 >> + * 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. >> + * >> + */ >> + >> +/*#define DEBUG 1*/ >=20 > This seems to be stray debug definition. Let's drop it. >=20 >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >=20 > Please arrange include directives in the alphabetical order. >=20 Have arranged them in alphabetical order. But have left=20 at the end as it is specific to this file. Is that ok? Or should that also follow the order? >> + >> +#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 >> + >> +struct lp3952_ctrl_hdl { >> + struct led_classdev cdev; >> + char name[10]; >> + 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; >> + }; >> +} __attribute__ ((packed)); >> + >> +struct lp3952_led_array { >> + u8 ndev; >> + u32 enable_gpio; >> + struct regmap *regmap; >> + struct i2c_client *client; >> + struct ptrn_gen_cmd pgm[8]; >=20 > Why 8? Please add a macro definition for this constant. >=20 >> + struct lp3952_ctrl_hdl *leds; >> +}; >> + >> +int lp3952_register_write(struct i2c_client *client, u8 reg, u8 val= ) >> +{ >> + int ret; >> + struct lp3952_led_array *priv =3D i2c_get_clientdata(client); >> + >> + ret =3D 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; >> +} >> + >> +int lp3952_register_read(struct i2c_client *client, u8 reg, u8 len, >> + u8 *valarray) >> +{ >> + struct lp3952_led_array *priv =3D i2c_get_clientdata(client); >> + int ret =3D regmap_bulk_read(priv->regmap, reg, valarray, len); >> + >> + if (ret < 0) >> + dev_err(&client->dev, "%s: reg 0x%x, val 0x%x, err %d\n", >> + __func__, reg, valarray[0], ret); >> + return ret; >> +} >=20 > This function seems to be unused. Removed the function. >=20 >> + >> +static void lp3952_on_off(struct lp3952_led_array *priv, enum lp395= 2_leds led, >> + int on) >> +{ >> + int val =3D 1 << led; >> + >> + dev_dbg(&priv->client->dev, "%s LED %d to %d\n", __func__, led,= on); >=20 > Please add empty line here. >=20 >> + if (LP3952_LED_ALL =3D=3D led) >> + val =3D 0x3f; >> + >> + regmap_update_bits(priv->regmap, LP3952_REG_LED_CTRL, val, >> + on ? val : 0); >=20 > Please check return value. >=20 >> +} >> + >> +/* >> + * 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 lp3952_brightness b) >> +{ >=20 > I'd prefer something more meaningful than 'b'. brightness or value? >=20 >> + unsigned int reg, val, shiftVal; >> + struct lp3952_ctrl_hdl *led =3D container_of(cdev, >> + struct lp3952_ctrl_hdl, >> + cdev); >> + struct lp3952_led_array *priv =3D (struct lp3952_led_array *)le= d->priv; >> + >> + dev_dbg(cdev->dev, "Brightness request: %d on %d\n", b, led->ch= annel); >> + >> + if (LP3952_OFF =3D=3D b) { >> + lp3952_on_off(priv, led->channel, LP3952_OFF); >> + return 0; >> + } >> + >> + val =3D b - 1; >> + reg =3D LP3952_REG_RGB1_MAX_I_CTRL; >> + shiftVal =3D (led->channel - LP3952_BLUE_1) * 2; >> + >> + if (led->channel > LP3952_RED_1) { >> + dev_err(cdev->dev, " %s Invalid LED requested", __func__); >> + return -EINVAL; >> + } else if (led->channel < LP3952_BLUE_1) { >> + shiftVal =3D led->channel * 2; >> + reg =3D LP3952_REG_RGB2_MAX_I_CTRL; >> + } >=20 > Empty line here >=20 >> + /* Enable the LED in case it is not enabled already */ >> + lp3952_on_off(priv, led->channel, 1); >=20 > and here would improve readability. >=20 >> + return (regmap_update_bits(priv->regmap, reg, 3 << shiftVal, >> + val << shiftVal)); >> +} >> + >> +static void lp3952_brightness_ctrl(struct led_classdev *cdev, >> + enum led_brightness b) >=20 > brightness_set_blocking op returns int. >=20 >> +{ >> + int range =3D LED_HALF / 2; >> + enum lp3952_brightness val =3D LP3952_FULL_BRIGHT; >> + >> + dev_dbg(cdev->dev, "Requested Brightness %d\n", b); >> + >> + if (LED_OFF =3D=3D b) >> + val =3D LP3952_OFF; >> + else if (b < (LED_HALF - range)) >> + val =3D LP3952_QUARTER_BRIGHT; >> + else if (b <=3D LED_HALF) >> + val =3D LP3952_HALF_BRIGHT; >> + else if (b <=3D LED_HALF + range) >> + val =3D LP3952_THREE_QRTR_BRIGHT; >=20 > You should operate directly on brightness levels, i.e. write the > brightness level passed from user space directly to the device. > enum lp3952_brightness is redundant IMO. The chip supports 5 brightness values, hence lp3952_brightness. The brightness register takes values between 0-3 (2 bits). This=20 function should convert user requested legacy values 0-255 to a=20 value within the register range. >=20 >> + >> + lp3952_set_brightness(cdev, val); >> +} >> + >> +static int lp3952_register_led_classdev(struct lp3952_led_array *pr= iv) >> +{ >> + int ret, i; >> + const char *led_name[] =3D { >> + "blue2", >> + "green2", >> + "red2", >> + "blue1", >> + "green1", >> + "red1" >> + }; >=20 > According to Documentation/leds/leds-class.txt LED class device name > should match following pattern: devicename:colour:function. Colour > can be omitted if not relevant. >=20 >> + for (i =3D 0; i < priv->ndev; i++) { >> + sprintf(priv->leds[i].name, "%s", led_name[i]); >> + priv->leds[i].cdev.name =3D priv->leds[i].name; >> + priv->leds[i].cdev.brightness =3D LED_OFF; >> + priv->leds[i].cdev.max_brightness =3D LED_FULL; >=20 > max_brightness should reflect the maximum brightness level allowed fo= r > given LED. LED_FULL is a legacy enum, that max_brightness property > overrides. >=20 lp3952_brightness_ctrl should convert the legacy value to value understood by the chip. The function ( + lp3952_set_brightness)=20 should convert users max request 255 to chips max supported value 3. >> + priv->leds[i].cdev.brightness_set_blocking =3D >> + lp3952_brightness_ctrl; >> + priv->leds[i].channel =3D i; >> + priv->leds[i].priv =3D priv; >> + >> + ret =3D led_classdev_register(&priv->client->dev, >> + &priv->leds[i].cdev); >=20 > Please use devm prefixed version. >=20 >> + if (ret < 0) { >> + dev_err(&priv->client->dev, >> + "couldn't register LED %s\n", >> + priv->leds[i].cdev.name); >> + goto error; >> + } >> + } >> + return 0; >> + >> +error: >> + for (; i >=3D 0; i--) >> + led_classdev_unregister(&priv->leds[i].cdev); >> + return ret; >> +} >> + >> +static void lp3952_unregister_led_classdev(struct lp3952_led_array = *priv) >> +{ >> + int i; >> + >> + for (i =3D 0; i < priv->ndev; i++) >> + led_classdev_unregister(&priv->leds[i].cdev); >> +} >> + >> +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) >> +{ >> + struct ptrn_gen_cmd line =3D { >> + .r =3D r, >> + .g =3D g, >> + .b =3D b, >> + .cet =3D cet, >> + .tt =3D tt >> + }; >> + if (cmd_index >=3D 8) >> + return -EINVAL; >> + >> + priv->pgm[cmd_index] =3D line; >> + lp3952_register_write(priv->client, LP3952_REG_CMD_0 + cmd_inde= x * 2, >> + line.bytes.msb); >=20 > Please check return value. >=20 >> + 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 =3D lp3952_register_write(priv->client, LP3952_REG_LED_CTRL= , 0); >> + if (ret) >> + return ret; >=20 > Empty line here please. >=20 >> + /* enable rgb patter, loop */ >> + lp3952_register_write(priv->client, LP3952_REG_PAT_GEN_CTRL, 0x= 06); >=20 > Please check return value and add empty line after that. >=20 >> + /* Update Bit 6 (Active mode),1 and 0 for pattern Gen */ >> + ret =3D regmap_update_bits(priv->regmap, LP3952_REG_ENABLES, 0x= 43, 0xfc); >=20 > Please add bit definitions for all the register bit fields. >=20 >> + if (ret) >> + return ret; >=20 > Empty line here please. >=20 >> + /* 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 =3D { >> + .reg_bits =3D 8, >> + .val_bits =3D 8, >> + .max_register =3D REG_MAX, >> +}; >> + >> +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; >> + struct lp3952_platform_data *pdata =3D dev_get_platdata(&client= ->dev); >> + >> + dev_info(&client->dev, "lp3952 probe\n"); >=20 > I believe this is stray debug log. Please remove it. >=20 >> + >> + if (!pdata) { >> + dev_err(&client->dev, >> + "lp3952: failed to obtain platform_data\n"); >> + return -EINVAL; >> + } >> + priv =3D kzalloc(sizeof(struct lp3952_led_array), GFP_KERNEL); >=20 > Please use devm prefixed version. >=20 >> + if (!priv) >> + return -ENOMEM; >> + >> + priv->ndev =3D 6; >> + >> + leds =3D kcalloc(priv->ndev, sizeof(*leds), GFP_KERNEL); >=20 > Please use devm prefixed version. >=20 >> + if (!leds) { >> + kfree(priv); >> + return -ENOMEM; >> + } >> + priv->leds =3D leds; >> + priv->client =3D client; >> + priv->enable_gpio =3D pdata->enable_gpio; >> + priv->regmap =3D devm_regmap_init_i2c(client, &lp3952_regmap); >> + if (IS_ERR(priv->regmap)) { >> + int err =3D PTR_ERR(priv->regmap); >> + dev_err(&client->dev, "Failed to allocate register map: %d\= n", >> + err); >> + return err; >> + } >=20 > Empty line here please. >=20 >> + status =3D gpio_request(priv->enable_gpio, "lp3952_gpio"); >> + if (status) >> + dev_warn(&client->dev, "Unable to disable reset gpio: %d\n"= , >> + status); >=20 > Empty line here please. >=20 >> + status =3D gpio_direction_output(priv->enable_gpio, 1); >> + if (status) >> + dev_warn(&client->dev, "Unable to disable reset gpio: %d\n"= , >> + status); >=20 > Empty line here please. >=20 >> + i2c_set_clientdata(client, priv); >> + >> + status =3D lp3952_configure(priv); >> + if (status) { >> + dev_err(&client->dev, "Probe failed. Device not found (%d)\= n", >> + status); >> + kfree(leds); >> + kfree(priv); >> + return status; >> + } >=20 > Empty line here please. >=20 >> + lp3952_register_led_classdev(priv); >=20 > Please check return value. >=20 >> + >> + lp3952_on_off(priv, LP3952_LED_ALL, 1); >=20 > If it is possible then device should remain in power down > mode if brightness equals 0. And this should be secured before > the LED class device is registered. Preferrably to be moved to the > lp3952_configure(). >=20 >> + return 0; >> +} >> + >> +static int lp3952_remove(struct i2c_client *client) >> +{ >> + struct lp3952_led_array *priv; >> + >> + priv =3D i2c_get_clientdata(client); >> + lp3952_on_off(priv, LP3952_LED_ALL, 0); >> + >> + lp3952_unregister_led_classdev(priv); >> + >> + kfree(priv->leds); >> + gpio_direction_input(priv->enable_gpio); >> + gpio_free(priv->enable_gpio); >> + kfree(priv); >> + return 0; >> +} >> + >> +#ifdef CONFIG_PM >> +static int lp3952_suspend(struct device *dev) >> +{ >> + struct i2c_client *client =3D to_i2c_client(dev); >> + struct lp3952_led_array *priv =3D i2c_get_clientdata(client); >> + >> + lp3952_on_off(priv, LP3952_LED_ALL, 0); >> + gpio_direction_input(priv->enable_gpio); >> + return 0; >> +} >> + >> +static int lp3952_resume(struct device *dev) >> +{ >> + struct i2c_client *client =3D to_i2c_client(dev); >> + struct lp3952_led_array *priv =3D i2c_get_clientdata(client); >> + >> + gpio_direction_output(priv->enable_gpio, 1); >> + return 0; >> +} >=20 > Power management is already handled in led-class.c. Please > refer to led_suspend(). It sets brightness to 0, and thus the LED > class drivers are expected to turn the devices they control into > power down mode in case all LEDs controlled by them are set to LED_OF= =46. >=20 > Therefore you should control the "enable_gpio" state in the > brightness_set_blocking() op. >=20 Removed the suspend/resume routine within the driver. Power management in led-class.c should be enough. >> + >> +static SIMPLE_DEV_PM_OPS(lp3952_pm, lp3952_suspend, lp3952_resume); >> +#define LP3952_PM (&lp3952_pm) >> +#else /* CONFIG_PM */ >> +#define LP3952_PM NULL >> +#endif >> + >> +#ifdef CONFIG_ACPI >> +static const struct acpi_device_id lp3952_acpi_match[] =3D { >> + {LP3952_NAME, 0}, >> + {} >> +}; >> >> + >> +MODULE_DEVICE_TABLE(acpi, lp3952_acpi_match); >> +#endif >> + >> +static const struct i2c_device_id lp3952_id[] =3D { >> + {LP3952_NAME, 0}, >> + {} >> +}; >> + >> +static struct i2c_driver lp3952_i2c_driver =3D { >> + .driver =3D { >> + .name =3D LP3952_NAME, >> + .owner =3D THIS_MODULE, >> + .pm =3D LP3952_PM, >> +#ifdef CONFIG_ACPI >> + .acpi_match_table =3D ACPI_PTR(lp3952_acpi_match), >> +#endif >=20 > Did you test booting using ACPI? Would it be possible to switch > to using Device Tree? This device is controlled via I2C so this > would be a more suitable way. >=20 My host platform runs intel chipset which prefers ACPI( dont have firmw= are code).=20 I couldn't test it as ACPI device. But have followed instructions in=20 Documentation/acpi/enumeration.txt. I tested the led controller registering it, from host board=20 init file. #include static struct lp3952_platform_data lp3952_data =3D { .enable_gpio =3D LP3952_NRST_GPIO }; static struct i2c_board_info __initdata lp3952_i2c_board_info =3D { I2C_BOARD_INFO(LP3952_NAME, LP3952_DEV_ADDR), .platform_data =3D &lp3952_data, }; then i2c_new_device(adap, &lp3952_i2c_board_info); from host board initialisation. Should I remove the ACPI registration?=20 >> + }, >> + .probe =3D lp3952_probe, >> + .remove =3D __exit_p(lp3952_remove) >=20 > If not building the driver as a module I am getting the following: >=20 > drivers/leds/leds-lp3952.c:337:12: warning: =91lp3952_remove=92 defin= ed but not used >=20 > Is there some specific reason for which you don't want to have > the "remove" op initialized when driver is built into the kernel? >=20 > What config are you using for building this driver? >=20 I have been building it as module. I assumed, if it was built in, wont = be using remove op. I have removed the __exit_p so that it is built in all the = time.=20 I have tested the module as built in. The warning is gone, with the cha= nge. > , >> + .id_table =3D 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= =2Eh >> new file mode 100644 >> index 0000000..1b1e7c3 >> --- /dev/null >> +++ b/include/linux/leds-lp3952.h >> @@ -0,0 +1,89 @@ >> +/* >> + * Copyright (C) 2016, DAQRI LLC >> + * >> + * License Terms: GPL v2 >> + * >> + * TI lp3952 Controller driver >> + * >> + * Author: Tony Makkiel >> + * >> + */ >> + >> +#ifndef LEDS_LP3952_H_ >> +#define LEDS_LP3952_H_ >> + >> +#define LP3952_NAME "lp3952" >> +#define LP3952_DEV_ADDR 0x54 >> +/* Following 2 MACRO are specific to Minnowboard Max >> + * Use the appropriate host specific values when >> + * instantiating device >> + */ >> +#define LP3952_BUS_ADDR 7 >> +#define LP3952_NRST_GPIO 464 >> + >> +/* 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_brightness { >> + LP3952_OFF =3D 0, >> + LP3952_QUARTER_BRIGHT, >> + LP3952_HALF_BRIGHT, >> + LP3952_THREE_QRTR_BRIGHT, >> + LP3952_FULL_BRIGHT >> +}; >> + >> +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_platform_data { >> + u32 enable_gpio; >> +}; >> + >> +#endif /* LEDS_LP3952_H_ */ >> >=20