Re: [PATCH 2/2] leds: lp5024: Add the LP5024/18 RGB LED driver

From: Dan Murphy
Date: Tue Jan 08 2019 - 16:18:05 EST

Next message: Tim Chen: "Re: [PATCH] x86/speculation: Add document to describe Spectre and its mitigations"
Previous message: Alex Deucher: "Re: [PATCH v2 0/3] drm/amdgpu: Fix suspend/resume issues with MST"
In reply to: Jacek Anaszewski: "Re: [PATCH 2/2] leds: lp5024: Add the LP5024/18 RGB LED driver"
Next in thread: Vesa JÃÃskelÃinen: "Re: [PATCH 2/2] leds: lp5024: Add the LP5024/18 RGB LED driver"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Jacek

Thanks for the review. v2 will contain the LP5030/36 as well.

On 1/8/19 3:10 PM, Jacek Anaszewski wrote:
> Dan,
>
> On 12/19/18 5:26 PM, Dan Murphy wrote:
>> Introduce the LP5024 and LP5018 RGB LED driver.
>> The difference in these 2 parts are only in the number of
>> LED outputs where the LP5024 can control 24 LEDs the LP5018
>> can only control 18.
>>
>> The device has the ability to group LED output into control banks
>> so that multiple LED banks can be controlled with the same mixing and
>> brightness.Â Inversely the LEDs can also be controlled independently.
>>
>> Signed-off-by: Dan Murphy <dmurphy@xxxxxx>
>> ---
>> Â drivers/leds/KconfigÂÂÂÂÂÂ |ÂÂ 7 +
>> Â drivers/leds/MakefileÂÂÂÂÂ |ÂÂ 1 +
>> Â drivers/leds/leds-lp5024.c | 610 +++++++++++++++++++++++++++++++++++++
>> Â 3 files changed, 618 insertions(+)
>> Â create mode 100644 drivers/leds/leds-lp5024.c
>>
>> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
>> index a72f97fca57b..d306bedb00b7 100644
>> --- a/drivers/leds/Kconfig
>> +++ b/drivers/leds/Kconfig
>> @@ -326,6 +326,13 @@ config LEDS_LP3952
>> ÂÂÂÂÂÂÂ To compile this driver as a module, choose M here: the
>> ÂÂÂÂÂÂÂ module will be called leds-lp3952.
>> Â +config LEDS_LP5024
>> +ÂÂÂ tristate "LED Support for TI LP5024/18 LED driver chip"
>> +ÂÂÂ depends on LEDS_CLASS && REGMAP_I2C
>> +ÂÂÂ help
>> +ÂÂÂÂÂ If you say yes here you get support for the Texas Instruments
>> +ÂÂÂÂÂ LP5024 and LP5018 LED driver.
>> +
>> Â 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 4c1b0054f379..60b4e4ddd3ee 100644
>> --- a/drivers/leds/Makefile
>> +++ b/drivers/leds/Makefile
>> @@ -32,6 +32,7 @@ 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_LP5024)ÂÂÂÂÂÂÂ += leds-lp5024.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-lp5024.c b/drivers/leds/leds-lp5024.c
>> new file mode 100644
>> index 000000000000..90e8dca15609
>> --- /dev/null
>> +++ b/drivers/leds/leds-lp5024.c
>> @@ -0,0 +1,610 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* TI LP50XX LED chip family driver
>> + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
>> + */
>> +
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/i2c.h>
>> +#include <linux/init.h>
>> +#include <linux/leds.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/of.h>
>> +#include <linux/of_gpio.h>
>> +#include <linux/regmap.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/slab.h>
>> +#include <uapi/linux/uleds.h>
>> +
>> +#define LP5024_DEV_CFG0ÂÂÂÂÂÂÂ 0x00
>> +#define LP5024_DEV_CFG1ÂÂÂÂÂÂÂ 0x01
>> +#define LP5024_LED_CFG0ÂÂÂÂÂÂÂ 0x02
>> +#define LP5024_BNK_BRTÂÂÂÂÂÂÂ 0x03
>> +#define LP5024_BNKA_CLRÂÂÂÂÂÂÂ 0x04
>> +#define LP5024_BNKB_CLRÂÂÂÂÂÂÂ 0x05
>> +#define LP5024_BNKC_CLRÂÂÂÂÂÂÂ 0x06
>> +#define LP5024_LED0_BRTÂÂÂÂÂÂÂ 0x07
>> +#define LP5024_LED1_BRTÂÂÂÂÂÂÂ 0x08
>> +#define LP5024_LED2_BRTÂÂÂÂÂÂÂ 0x09
>> +#define LP5024_LED3_BRTÂÂÂÂÂÂÂ 0x0a
>> +#define LP5024_LED4_BRTÂÂÂÂÂÂÂ 0x0b
>> +#define LP5024_LED5_BRTÂÂÂÂÂÂÂ 0x0c
>> +#define LP5024_LED6_BRTÂÂÂÂÂÂÂ 0x0d
>> +#define LP5024_LED7_BRTÂÂÂÂÂÂÂ 0x0e
>> +
>> +#define LP5024_OUT0_CLRÂÂÂÂÂÂÂ 0x0f
>> +#define LP5024_OUT1_CLRÂÂÂÂÂÂÂ 0x10
>> +#define LP5024_OUT2_CLRÂÂÂÂÂÂÂ 0x11
>> +#define LP5024_OUT3_CLRÂÂÂÂÂÂÂ 0x12
>> +#define LP5024_OUT4_CLRÂÂÂÂÂÂÂ 0x13
>> +#define LP5024_OUT5_CLRÂÂÂÂÂÂÂ 0x14
>> +#define LP5024_OUT6_CLRÂÂÂÂÂÂÂ 0x15
>> +#define LP5024_OUT7_CLRÂÂÂÂÂÂÂ 0x16
>> +#define LP5024_OUT8_CLRÂÂÂÂÂÂÂ 0x17
>> +#define LP5024_OUT9_CLRÂÂÂÂÂÂÂ 0x18
>> +#define LP5024_OUT10_CLRÂÂÂ 0x19
>> +#define LP5024_OUT11_CLRÂÂÂ 0x1a
>> +#define LP5024_OUT12_CLRÂÂÂ 0x1b
>> +#define LP5024_OUT13_CLRÂÂÂ 0x1c
>> +#define LP5024_OUT14_CLRÂÂÂ 0x1d
>> +#define LP5024_OUT15_CLRÂÂÂ 0x1e
>> +#define LP5024_OUT16_CLRÂÂÂ 0x1f
>> +#define LP5024_OUT17_CLRÂÂÂ 0x20
>> +#define LP5024_OUT18_CLRÂÂÂ 0x21
>> +#define LP5024_OUT19_CLRÂÂÂ 0x22
>> +#define LP5024_OUT20_CLRÂÂÂ 0x23
>> +#define LP5024_OUT21_CLRÂÂÂ 0x24
>> +#define LP5024_OUT22_CLRÂÂÂ 0x25
>> +#define LP5024_OUT23_CLRÂÂÂ 0x26
>> +
>> +#define LP5024_RESETÂÂÂÂÂÂÂ 0x27
>> +#define LP5024_SW_RESETÂÂÂÂÂÂÂ 0xff
>> +
>> +#define LP5024_CHIP_ENÂÂÂÂÂÂÂ BIT(6)
>> +
>> +#define LP5024_CONTROL_AÂÂÂÂÂÂÂ 0
>> +#define LP5024_CONTROL_BÂÂÂÂÂÂÂ 1
>> +#define LP5024_CONTROL_CÂÂÂÂÂÂÂ 2
>> +#define LP5024_MAX_CONTROL_BANKSÂÂÂ 3
>> +
>> +#define LP5018_MAX_LED_STRINGSÂÂÂ 6
>> +#define LP5024_MAX_LED_STRINGSÂÂÂ 8
>> +
>> +enum lp5024_model {
>> +ÂÂÂ LP5018,
>> +ÂÂÂ LP5024,
>> +};
>> +
>> +struct lp5024_led {
>> +ÂÂÂ u32 led_strings[LP5024_MAX_LED_STRINGS];
>> +ÂÂÂ char label[LED_MAX_NAME_SIZE];
>> +ÂÂÂ struct led_classdev led_dev;
>> +ÂÂÂ struct lp5024 *priv;
>> +ÂÂÂ int led_number;
>> +ÂÂÂ u8 ctrl_bank_enabled;
>> +};
>> +
>> +/**
>> + * struct lp5024 -
>> + * @enable_gpio: Hardware enable gpio
>> + * @regulator: LED supply regulator pointer
>> + * @client: Pointer to the I2C client
>> + * @regmap: Devices register map
>> + * @dev: Pointer to the devices device struct
>> + * @lock: Lock for reading/writing the device
>> + * @model_id: ID of the device
>> + * @leds: Array of LED strings
>> + */
>> +struct lp5024 {
>> +ÂÂÂ struct gpio_desc *enable_gpio;
>> +ÂÂÂ struct regulator *regulator;
>> +ÂÂÂ struct i2c_client *client;
>> +ÂÂÂ struct regmap *regmap;
>> +ÂÂÂ struct device *dev;
>> +ÂÂÂ struct mutex lock;
>> +ÂÂÂ int model_id;
>> +ÂÂÂ int max_leds;
>> +ÂÂÂ int num_of_leds;
>> +
>> +ÂÂÂ /* This needs to be at the end of the struct */
>> +ÂÂÂ struct lp5024_led leds[];
>> +};
>> +
>> +static const struct reg_default lp5024_reg_defs[] = {
>> +ÂÂÂ {LP5024_DEV_CFG0, 0x0},
>> +ÂÂÂ {LP5024_DEV_CFG1, 0x3c},
>> +ÂÂÂ {LP5024_BNK_BRT, 0xff},
>> +ÂÂÂ {LP5024_BNKA_CLR, 0x0f},
>> +ÂÂÂ {LP5024_BNKB_CLR, 0x0f},
>> +ÂÂÂ {LP5024_BNKC_CLR, 0x0f},
>> +ÂÂÂ {LP5024_LED0_BRT, 0x0f},
>> +ÂÂÂ {LP5024_LED1_BRT, 0xff},
>> +ÂÂÂ {LP5024_LED2_BRT, 0xff},
>> +ÂÂÂ {LP5024_LED3_BRT, 0xff},
>> +ÂÂÂ {LP5024_LED4_BRT, 0xff},
>> +ÂÂÂ {LP5024_LED5_BRT, 0xff},
>> +ÂÂÂ {LP5024_LED6_BRT, 0xff},
>> +ÂÂÂ {LP5024_LED7_BRT, 0xff},
>> +ÂÂÂ {LP5024_OUT0_CLR, 0x0f},
>> +ÂÂÂ {LP5024_OUT1_CLR, 0x00},
>> +ÂÂÂ {LP5024_OUT2_CLR, 0x00},
>> +ÂÂÂ {LP5024_OUT3_CLR, 0x00},
>> +ÂÂÂ {LP5024_OUT4_CLR, 0x00},
>> +ÂÂÂ {LP5024_OUT5_CLR, 0x00},
>> +ÂÂÂ {LP5024_OUT6_CLR, 0x00},
>> +ÂÂÂ {LP5024_OUT7_CLR, 0x00},
>> +ÂÂÂ {LP5024_OUT8_CLR, 0x00},
>> +ÂÂÂ {LP5024_OUT9_CLR, 0x00},
>> +ÂÂÂ {LP5024_OUT10_CLR, 0x00},
>> +ÂÂÂ {LP5024_OUT11_CLR, 0x00},
>> +ÂÂÂ {LP5024_OUT12_CLR, 0x00},
>> +ÂÂÂ {LP5024_OUT13_CLR, 0x00},
>> +ÂÂÂ {LP5024_OUT14_CLR, 0x00},
>> +ÂÂÂ {LP5024_OUT15_CLR, 0x00},
>> +ÂÂÂ {LP5024_OUT16_CLR, 0x00},
>> +ÂÂÂ {LP5024_OUT17_CLR, 0x00},
>> +ÂÂÂ {LP5024_OUT18_CLR, 0x00},
>> +ÂÂÂ {LP5024_OUT19_CLR, 0x00},
>> +ÂÂÂ {LP5024_OUT20_CLR, 0x00},
>> +ÂÂÂ {LP5024_OUT21_CLR, 0x00},
>> +ÂÂÂ {LP5024_OUT22_CLR, 0x00},
>> +ÂÂÂ {LP5024_OUT23_CLR, 0x00},
>> +ÂÂÂ {LP5024_RESET, 0x00}
>> +};
>> +
>> +static const struct regmap_config lp5024_regmap_config = {
>> +ÂÂÂ .reg_bits = 8,
>> +ÂÂÂ .val_bits = 8,
>> +
>> +ÂÂÂ .max_register = LP5024_RESET,
>> +ÂÂÂ .reg_defaults = lp5024_reg_defs,
>> +ÂÂÂ .num_reg_defaults = ARRAY_SIZE(lp5024_reg_defs),
>> +ÂÂÂ .cache_type = REGCACHE_RBTREE,
>> +};
>> +
>> +static int lp5024_set_color_mix(struct lp5024_led *led, u8 color_reg,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ u8 color_val)
>> +{
>> +ÂÂÂ return regmap_write(led->priv->regmap, color_reg, color_val);
>> +}
>> +
>> +
>> +static ssize_t ctrl_bank_a_mix_store(struct device *dev,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct device_attribute *attr,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ const char *buf, size_t size)
>> +{
>> +ÂÂÂ struct led_classdev *led_cdev = dev_get_drvdata(dev);
>> +ÂÂÂ struct lp5024_led *led = container_of(led_cdev, struct lp5024_led,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ led_dev);
>> +ÂÂÂ u8 mix_value;
>> +ÂÂÂ int ret;
>> +
>> +ÂÂÂ ret = kstrtou8(buf, 0, &mix_value);
>> +ÂÂÂ if (ret)
>> +ÂÂÂÂÂÂÂ return ret;
>> +
>> +ÂÂÂ lp5024_set_color_mix(led, LP5024_BNKA_CLR, mix_value);
>> +
>> +ÂÂÂ return size;
>> +}
>> +static ssize_t ctrl_bank_b_mix_store(struct device *dev,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct device_attribute *attr,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ const char *buf, size_t size)
>> +{
>> +ÂÂÂ struct led_classdev *led_cdev = dev_get_drvdata(dev);
>> +ÂÂÂ struct lp5024_led *led = container_of(led_cdev, struct lp5024_led,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ led_dev);
>> +ÂÂÂ u8 mix_value;
>> +ÂÂÂ int ret;
>> +
>> +ÂÂÂ ret = kstrtou8(buf, 0, &mix_value);
>> +ÂÂÂ if (ret)
>> +ÂÂÂÂÂÂÂ return ret;
>> +
>> +ÂÂÂ lp5024_set_color_mix(led, LP5024_BNKB_CLR, mix_value);
>> +
>> +ÂÂÂ return size;
>> +}
>> +static ssize_t ctrl_bank_c_mix_store(struct device *dev,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct device_attribute *attr,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ const char *buf, size_t size)
>> +{
>> +ÂÂÂ struct led_classdev *led_cdev = dev_get_drvdata(dev);
>> +ÂÂÂ struct lp5024_led *led = container_of(led_cdev, struct lp5024_led,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ led_dev);
>> +ÂÂÂ u8 mix_value;
>> +ÂÂÂ int ret;
>> +
>> +ÂÂÂ ret = kstrtou8(buf, 0, &mix_value);
>> +ÂÂÂ if (ret)
>> +ÂÂÂÂÂÂÂ return ret;
>> +
>> +ÂÂÂ lp5024_set_color_mix(led, LP5024_BNKC_CLR, mix_value);
>> +
>> +ÂÂÂ return size;
>> +}
>> +
>> +static DEVICE_ATTR_WO(ctrl_bank_a_mix);
>> +static DEVICE_ATTR_WO(ctrl_bank_b_mix);
>> +static DEVICE_ATTR_WO(ctrl_bank_c_mix);
>
> Why WO?

I did not feel like there was anything to read but I can create the show to read out the register

Of course I am thinking of eliminating these files and combining them with the led_mix files.
And renaming the files to something more generic.
I was thinking of renaming the file to "saturation" and passing in a 32 bit number that will be
parsed as RGB values, well at least the first 24 bytes.

At least this way if the HSV framework comes in then the ABI will be available and only the implemenation
will change.

>
>> +
>> +static struct attribute *lp5024_ctrl_bank_attrs[] = {
>> +ÂÂÂ &dev_attr_ctrl_bank_a_mix.attr,
>> +ÂÂÂ &dev_attr_ctrl_bank_b_mix.attr,
>> +ÂÂÂ &dev_attr_ctrl_bank_c_mix.attr,
>> +ÂÂÂ NULL
>> +};
>> +ATTRIBUTE_GROUPS(lp5024_ctrl_bank);
>> +
>> +static ssize_t led3_mix_store(struct device *dev,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct device_attribute *attr,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ const char *buf, size_t size)
>> +{
>> +ÂÂÂ struct led_classdev *led_cdev = dev_get_drvdata(dev);
>> +ÂÂÂ struct lp5024_led *led = container_of(led_cdev, struct lp5024_led,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ led_dev);
>> +ÂÂÂ u8 mix_value;
>> +ÂÂÂ u8 reg_value; > +ÂÂÂ int ret;
>> +
>> +ÂÂÂ ret = kstrtou8(buf, 0, &mix_value);
>> +ÂÂÂ if (ret)
>> +ÂÂÂÂÂÂÂ return ret;
>> +
>> +ÂÂÂ reg_value = (led->led_number * 3) + LP5024_OUT2_CLR;
>
> It is more natural if constant goes first. Like BASE_ADDR + offset.

ACK and repeat

>
>> +
>> +ÂÂÂ lp5024_set_color_mix(led, reg_value, mix_value);
>> +
>> +ÂÂÂ return size;
>> +}
>> +
>> +static ssize_t led2_mix_store(struct device *dev,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct device_attribute *attr,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ const char *buf, size_t size)
>> +{
>> +ÂÂÂ struct led_classdev *led_cdev = dev_get_drvdata(dev);
>> +ÂÂÂ struct lp5024_led *led = container_of(led_cdev, struct lp5024_led,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ led_dev);
>> +ÂÂÂ u8 mix_value;
>> +ÂÂÂ u8 reg_value;
>> +ÂÂÂ int ret;
>> +
>> +ÂÂÂ ret = kstrtou8(buf, 0, &mix_value);
>> +ÂÂÂ if (ret)
>> +ÂÂÂÂÂÂÂ return ret;
>> +
>> +ÂÂÂ reg_value = (led->led_number * 3) + LP5024_OUT1_CLR;
>> +
>> +ÂÂÂ lp5024_set_color_mix(led, reg_value, mix_value);
>> +
>> +ÂÂÂ return size;
>> +}
>> +
>> +static ssize_t led1_mix_store(struct device *dev,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct device_attribute *attr,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ const char *buf, size_t size)
>> +{
>> +ÂÂÂ struct led_classdev *led_cdev = dev_get_drvdata(dev);
>> +ÂÂÂ struct lp5024_led *led = container_of(led_cdev, struct lp5024_led,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ led_dev);
>> +ÂÂÂ u8 mix_value;
>> +ÂÂÂ u8 reg_value;
>> +ÂÂÂ int ret;
>> +
>> +ÂÂÂ ret = kstrtou8(buf, 0, &mix_value);
>> +ÂÂÂ if (ret)
>> +ÂÂÂÂÂÂÂ return ret;
>> +
>> +ÂÂÂ reg_value = (led->led_number * 3) + LP5024_OUT0_CLR;
>> +
>> +ÂÂÂ lp5024_set_color_mix(led, reg_value, mix_value);
>> +
>> +ÂÂÂ return size;
>> +}
>> +
>> +static DEVICE_ATTR_WO(led1_mix);
>> +static DEVICE_ATTR_WO(led2_mix);
>> +static DEVICE_ATTR_WO(led3_mix);
>
> Why WO?

See above

>
>> +static struct attribute *lp5024_led_independent_attrs[] = {
>> +ÂÂÂ &dev_attr_led1_mix.attr,
>> +ÂÂÂ &dev_attr_led2_mix.attr,
>> +ÂÂÂ &dev_attr_led3_mix.attr,
>> +ÂÂÂ NULL
>> +};
>> +ATTRIBUTE_GROUPS(lp5024_led_independent);
>> +
>> +static int lp5024_brightness_set(struct led_classdev *led_cdev,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ enum led_brightness brt_val)
>> +{
>> +ÂÂÂ struct lp5024_led *led = container_of(led_cdev, struct lp5024_led,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ led_dev);
>> +ÂÂÂ int ret = 0;
>> +ÂÂÂ u8 reg_val;
>> +
>> +ÂÂÂ mutex_lock(&led->priv->lock);
>> +
>> +ÂÂÂ if (led->ctrl_bank_enabled)
>> +ÂÂÂÂÂÂÂ reg_val = LP5024_BNK_BRT;
>> +ÂÂÂ else
>> +ÂÂÂÂÂÂÂ reg_val = led->led_number + LP5024_LED0_BRT;
>> +
>> +ÂÂÂ ret = regmap_write(led->priv->regmap, reg_val, brt_val);
>> +
>> +ÂÂÂ mutex_unlock(&led->priv->lock);
>> +
>> +ÂÂÂ return ret;
>> +}
>> +
>> +static enum led_brightness lp5024_brightness_get(struct led_classdev *led_cdev)
>> +{
>> +ÂÂÂ struct lp5024_led *led = container_of(led_cdev, struct lp5024_led,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ led_dev);
>> +ÂÂÂ unsigned int brt_val;
>> +ÂÂÂ u8 reg_val;
>> +ÂÂÂ int ret;
>> +
>> +ÂÂÂ mutex_lock(&led->priv->lock);
>> +
>> +ÂÂÂ if (led->ctrl_bank_enabled)
>> +ÂÂÂÂÂÂÂ reg_val = LP5024_BNK_BRT;
>> +ÂÂÂ else
>> +ÂÂÂÂÂÂÂ reg_val = led->led_number + LP5024_LED0_BRT;
>> +
>> +ÂÂÂ ret = regmap_read(led->priv->regmap, reg_val, &brt_val);
>> +
>> +ÂÂÂ mutex_unlock(&led->priv->lock);
>> +
>> +ÂÂÂ return brt_val;
>> +}
>> +
>> +static int lp5024_set_led_values(struct lp5024 *priv)
>> +{
>> +ÂÂÂ struct lp5024_led *led;
>> +ÂÂÂ int i, j;
>> +ÂÂÂ u8 led_ctrl_enable = 0;
>> +
>> +ÂÂÂ for (i = 0; i <= priv->num_of_leds; i++) {
>> +ÂÂÂÂÂÂÂ led = &priv->leds[i];
>> +ÂÂÂÂÂÂÂ if (led->ctrl_bank_enabled) {
>> +ÂÂÂÂÂÂÂÂÂÂÂ for (j = 0; j <= LP5024_MAX_LED_STRINGS - 1; j++)
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ led_ctrl_enable |= (1 << led->led_strings[j]);
>> +ÂÂÂÂÂÂÂ }
>> +ÂÂÂ }
>> +
>> +ÂÂÂ regmap_write(priv->regmap, LP5024_LED_CFG0, led_ctrl_enable);
>> +
>> +ÂÂÂ return 0;
>> +}
>> +
>> +static int lp5024_init(struct lp5024 *priv)
>> +{
>> +ÂÂÂ int ret;
>> +
>> +ÂÂÂ if (priv->enable_gpio) {
>> +ÂÂÂÂÂÂÂ gpiod_direction_output(priv->enable_gpio, 1);
>> +ÂÂÂ } else {
>> +ÂÂÂÂÂÂÂ ret = regmap_write(priv->regmap, LP5024_RESET, LP5024_SW_RESET);
>> +ÂÂÂÂÂÂÂ if (ret) {
>> +ÂÂÂÂÂÂÂÂÂÂÂ dev_err(&priv->client->dev,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ "Cannot reset the device\n");
>> +ÂÂÂÂÂÂÂÂÂÂÂ goto out;
>> +ÂÂÂÂÂÂÂ }
>> +ÂÂÂ }
>> +
>> +ÂÂÂ ret = lp5024_set_led_values(priv);
>> +ÂÂÂ if (ret)
>> +ÂÂÂÂÂÂÂ dev_err(&priv->client->dev, "Setting the CRTL bank failed\n");
>> +
>> +ÂÂÂ ret = regmap_write(priv->regmap, LP5024_DEV_CFG0, LP5024_CHIP_EN);
>> +ÂÂÂ if (ret) {
>> +ÂÂÂÂÂÂÂ dev_err(&priv->client->dev, "Cannot write ctrl enable\n");
>> +ÂÂÂÂÂÂÂ goto out;
>> +ÂÂÂ }
>> +out:
>> +ÂÂÂ return ret;
>> +}
>> +
>> +static int lp5024_probe_dt(struct lp5024 *priv)
>> +{
>> +ÂÂÂ struct fwnode_handle *child = NULL;
>> +ÂÂÂ struct lp5024_led *led;
>> +ÂÂÂ const char *name;
>> +ÂÂÂ int led_number;
>> +ÂÂÂ size_t i = 0;
>> +ÂÂÂ int ret;
>> +
>> +ÂÂÂ priv->enable_gpio = devm_gpiod_get_optional(&priv->client->dev,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ "enable", GPIOD_OUT_LOW);
>> +ÂÂÂ if (IS_ERR(priv->enable_gpio)) {
>> +ÂÂÂÂÂÂÂ ret = PTR_ERR(priv->enable_gpio);
>> +ÂÂÂÂÂÂÂ dev_err(&priv->client->dev, "Failed to get enable gpio: %d\n",
>> +ÂÂÂÂÂÂÂÂÂÂÂ ret);
>> +ÂÂÂÂÂÂÂ return ret;
>> +ÂÂÂ }
>> +
>> +ÂÂÂ priv->regulator = devm_regulator_get(&priv->client->dev, "vled");
>> +ÂÂÂ if (IS_ERR(priv->regulator))
>> +ÂÂÂÂÂÂÂ priv->regulator = NULL;
>> +
>> +ÂÂÂ if (priv->model_id == LP5018)
>> +ÂÂÂÂÂÂÂ priv->max_leds = LP5018_MAX_LED_STRINGS;
>> +ÂÂÂ else
>> +ÂÂÂÂÂÂÂ priv->max_leds = LP5024_MAX_LED_STRINGS;
>> +
>> +ÂÂÂ device_for_each_child_node(&priv->client->dev, child) {
>> +ÂÂÂÂÂÂÂ led = &priv->leds[i];
>> +
>> +ÂÂÂÂÂÂÂ if (fwnode_property_present(child, "ti,control-bank"))
>> +ÂÂÂÂÂÂÂÂÂÂÂ led->ctrl_bank_enabled = 1;
>> +ÂÂÂÂÂÂÂ else
>> +ÂÂÂÂÂÂÂÂÂÂÂ led->ctrl_bank_enabled = 0;
>> +
>> +ÂÂÂÂÂÂÂ if (led->ctrl_bank_enabled) {
>> +ÂÂÂÂÂÂÂÂÂÂÂ ret = fwnode_property_read_u32_array(child,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ "led-sources",
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ NULL, 0);
>> +ÂÂÂÂÂÂÂÂÂÂÂ ret = fwnode_property_read_u32_array(child,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ "led-sources",
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ led->led_strings,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ ret);
>> +
>> +ÂÂÂÂÂÂÂÂÂÂÂ led->led_number = led->led_strings[0];
>> +
>> +ÂÂÂÂÂÂÂ } else {
>> +ÂÂÂÂÂÂÂÂÂÂÂ ret = fwnode_property_read_u32(child, "led-sources",
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ &led_number);
>> +
>> +ÂÂÂÂÂÂÂÂÂÂÂ led->led_number = led_number;
>> +ÂÂÂÂÂÂÂ }
>> +ÂÂÂÂÂÂÂ if (ret) {
>> +ÂÂÂÂÂÂÂÂÂÂÂ dev_err(&priv->client->dev,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ "led-sources property missing\n");
>> +ÂÂÂÂÂÂÂÂÂÂÂ fwnode_handle_put(child);
>> +ÂÂÂÂÂÂÂÂÂÂÂ goto child_out;
>> +ÂÂÂÂÂÂÂ }
>> +
>> +ÂÂÂÂÂÂÂ if (led_number > priv->max_leds) {
>> +ÂÂÂÂÂÂÂÂÂÂÂ dev_err(&priv->client->dev,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ "led-sources property is invalid\n");
>> +ÂÂÂÂÂÂÂÂÂÂÂ ret = -EINVAL;
>> +ÂÂÂÂÂÂÂÂÂÂÂ fwnode_handle_put(child);
>> +ÂÂÂÂÂÂÂÂÂÂÂ goto child_out;
>> +ÂÂÂÂÂÂÂ }
>> +
>> +ÂÂÂÂÂÂÂ ret = fwnode_property_read_string(child, "label", &name);
>> +ÂÂÂÂÂÂÂ if (ret)
>> +ÂÂÂÂÂÂÂÂÂÂÂ snprintf(led->label, sizeof(led->label),
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ "%s::", priv->client->name);
>> +ÂÂÂÂÂÂÂ else
>> +ÂÂÂÂÂÂÂÂÂÂÂ snprintf(led->label, sizeof(led->label),
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ "%s:%s", priv->client->name, name);
>> +
>> +ÂÂÂÂÂÂÂ fwnode_property_read_string(child, "linux,default-trigger",
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ &led->led_dev.default_trigger);
>> +
>> +ÂÂÂÂÂÂÂ led->priv = priv;
>> +ÂÂÂÂÂÂÂ led->led_dev.name = led->label;
>> +ÂÂÂÂÂÂÂ led->led_dev.max_brightness = 255;
>> +ÂÂÂÂÂÂÂ led->led_dev.brightness_set_blocking = lp5024_brightness_set;
>> +ÂÂÂÂÂÂÂ led->led_dev.brightness_get = lp5024_brightness_get;
>> +
>> +ÂÂÂÂÂÂÂ if (led->ctrl_bank_enabled)
>> +ÂÂÂÂÂÂÂÂÂÂÂ led->led_dev.groups = lp5024_ctrl_bank_groups;
>> +ÂÂÂÂÂÂÂ else
>> +ÂÂÂÂÂÂÂÂÂÂÂ led->led_dev.groups = lp5024_led_independent_groups;
>> +
>> +ÂÂÂÂÂÂÂ ret = devm_led_classdev_register(&priv->client->dev,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ &led->led_dev);
>> +ÂÂÂÂÂÂÂ if (ret) {
>> +ÂÂÂÂÂÂÂÂÂÂÂ dev_err(&priv->client->dev, "led register err: %d\n",
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ ret);
>> +ÂÂÂÂÂÂÂÂÂÂÂ fwnode_handle_put(child);
>> +ÂÂÂÂÂÂÂÂÂÂÂ goto child_out;
>> +ÂÂÂÂÂÂÂ }
>> +ÂÂÂÂÂÂÂ i++;
>> +ÂÂÂ }
>> +ÂÂÂ priv->num_of_leds = i;
>> +
>> +child_out:
>> +ÂÂÂ return ret;
>> +}
>> +
>> +static int lp5024_probe(struct i2c_client *client,
>> +ÂÂÂÂÂÂÂÂÂÂÂ const struct i2c_device_id *id)
>> +{
>> +ÂÂÂ struct lp5024 *led;
>> +ÂÂÂ int count;
>> +ÂÂÂ int ret;
>> +
>> +ÂÂÂ count = device_get_child_node_count(&client->dev);
>> +ÂÂÂ if (!count) {
>> +ÂÂÂÂÂÂÂ dev_err(&client->dev, "LEDs are not defined in device tree!");
>> +ÂÂÂÂÂÂÂ return -ENODEV;
>> +ÂÂÂ }
>> +
>> +ÂÂÂ led = devm_kzalloc(&client->dev, struct_size(led, leds, count),
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ GFP_KERNEL);
>> +ÂÂÂ if (!led)
>> +ÂÂÂÂÂÂÂ return -ENOMEM;
>> +
>> +ÂÂÂ mutex_init(&led->lock);
>> +ÂÂÂ led->client = client;
>> +ÂÂÂ led->dev = &client->dev;
>> +ÂÂÂ led->model_id = id->driver_data;
>> +ÂÂÂ i2c_set_clientdata(client, led);
>> +
>> +ÂÂÂ led->regmap = devm_regmap_init_i2c(client, &lp5024_regmap_config);
>> +ÂÂÂ if (IS_ERR(led->regmap)) {
>> +ÂÂÂÂÂÂÂ ret = PTR_ERR(led->regmap);
>> +ÂÂÂÂÂÂÂ dev_err(&client->dev, "Failed to allocate register map: %d\n",
>> +ÂÂÂÂÂÂÂÂÂÂÂ ret);
>> +ÂÂÂÂÂÂÂ return ret;
>> +ÂÂÂ }
>> +
>> +ÂÂÂ ret = lp5024_probe_dt(led);
>> +ÂÂÂ if (ret)
>> +ÂÂÂÂÂÂÂ return ret;
>> +
>> +ÂÂÂ ret = lp5024_init(led);
>> +ÂÂÂ if (ret)
>> +ÂÂÂÂÂÂÂ return ret;
>> +
>> +ÂÂÂ return 0;
>> +}
>> +
>> +static int lp5024_remove(struct i2c_client *client)
>> +{
>> +ÂÂÂ struct lp5024 *led = i2c_get_clientdata(client);
>> +ÂÂÂ int ret;
>> +
>> +ÂÂÂ ret = regmap_update_bits(led->regmap, LP5024_DEV_CFG0,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ LP5024_CHIP_EN, 0);
>> +ÂÂÂ if (ret) {
>> +ÂÂÂÂÂÂÂ dev_err(&led->client->dev, "Failed to disable regulator\n");
>> +ÂÂÂÂÂÂÂ return ret;
>> +ÂÂÂ }
>> +
>> +ÂÂÂ if (led->enable_gpio)
>> +ÂÂÂÂÂÂÂ gpiod_direction_output(led->enable_gpio, 0);
>> +
>> +ÂÂÂ if (led->regulator) {
>> +ÂÂÂÂÂÂÂ ret = regulator_disable(led->regulator);
>> +ÂÂÂÂÂÂÂ if (ret)
>> +ÂÂÂÂÂÂÂÂÂÂÂ dev_err(&led->client->dev,
>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ "Failed to disable regulator\n");
>> +ÂÂÂ }
>> +
>> +ÂÂÂ mutex_destroy(&led->lock);
>> +
>> +ÂÂÂ return 0;
>> +}
>> +
>> +static const struct i2c_device_id lp5024_id[] = {
>> +ÂÂÂ { "lp5018", LP5018 },
>> +ÂÂÂ { "lp5024", LP5024 },
>> +ÂÂÂ { }
>> +};
>> +MODULE_DEVICE_TABLE(i2c, lp5024_id);
>> +
>> +static const struct of_device_id of_lp5024_leds_match[] = {
>> +ÂÂÂ { .compatible = "ti,lp5018", },
>> +ÂÂÂ { .compatible = "ti,lp5024", },
>> +ÂÂÂ {},
>> +};
>> +MODULE_DEVICE_TABLE(of, of_lp5024_leds_match);
>> +
>> +static struct i2c_driver lp5024_driver = {
>> +ÂÂÂ .driver = {
>> +ÂÂÂÂÂÂÂ .nameÂÂÂ = "lp5024",
>> +ÂÂÂÂÂÂÂ .of_match_table = of_lp5024_leds_match,
>> +ÂÂÂ },
>> +ÂÂÂ .probeÂÂÂÂÂÂÂ = lp5024_probe,
>> +ÂÂÂ .removeÂÂÂÂÂÂÂ = lp5024_remove,
>> +ÂÂÂ .id_tableÂÂÂ = lp5024_id,
>> +};
>> +module_i2c_driver(lp5024_driver);
>> +
>> +MODULE_DESCRIPTION("Texas Instruments LP5024 LED driver");
>> +MODULE_AUTHOR("Dan Murphy <dmurphy@xxxxxx>");
>> +MODULE_LICENSE("GPL v2");
>>
>

--
------------------
Dan Murphy

Next message: Tim Chen: "Re: [PATCH] x86/speculation: Add document to describe Spectre and its mitigations"
Previous message: Alex Deucher: "Re: [PATCH v2 0/3] drm/amdgpu: Fix suspend/resume issues with MST"
In reply to: Jacek Anaszewski: "Re: [PATCH 2/2] leds: lp5024: Add the LP5024/18 RGB LED driver"
Next in thread: Vesa JÃÃskelÃinen: "Re: [PATCH 2/2] leds: lp5024: Add the LP5024/18 RGB LED driver"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]