* [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver @ 2017-11-15 19:42 ` Dan Murphy 0 siblings, 0 replies; 42+ messages in thread From: Dan Murphy @ 2017-11-15 19:42 UTC (permalink / raw) To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8, rpurdie-Fm38FmjxZ/leoWH0uzbU5w, jacek.anaszewski-Re5JQEeQqe8AvxtiuMwx3w, pavel-+ZI9xUNit7I Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-leds-u79uwXL29TY76Z2rM5mHXA, Dan Murphy This adds the devicetree bindings for the LM3692x I2C LED string driver. Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org> Signed-off-by: Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org> --- v3 - No changes v2 - No changes - https://patchwork.kernel.org/patch/10056677/ .../devicetree/bindings/leds/leds-lm3692x.txt | 28 ++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3692x.txt diff --git a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt new file mode 100644 index 000000000000..cfef67bd4100 --- /dev/null +++ b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt @@ -0,0 +1,28 @@ +* Texas Instruments - LM3692x Highly Efficient Dual-String White LED Driver + +The LM36922 is an ultra-compact, highly efficient, +two string white-LED driver designed for LCD display +backlighting. + +Required properties: + - compatible: + "ti,lm3692x" + - reg - I2C slave address + +Optional properties: + - label - Used for naming LEDs + - enable-gpio - gpio pin to enable/disable the device. + - supply - "vled" - LED supply + +Example: + +leds: leds@6 { + compatible = "ti,lm3692x"; + reg = <0x36>; + label = "backlight_cluster"; + enable-gpio = <&gpio1 28 GPIO_ACTIVE_HIGH>; + vled-supply = <&vbatt>; +} + +For more product information please see the link below: +http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf -- 2.15.0.124.g7668cbc60 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver @ 2017-11-15 19:42 ` Dan Murphy 0 siblings, 0 replies; 42+ messages in thread From: Dan Murphy @ 2017-11-15 19:42 UTC (permalink / raw) To: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, pavel Cc: devicetree, linux-kernel, linux-leds, Dan Murphy This adds the devicetree bindings for the LM3692x I2C LED string driver. Acked-by: Pavel Machek <pavel@ucw.cz> Signed-off-by: Dan Murphy <dmurphy@ti.com> --- v3 - No changes v2 - No changes - https://patchwork.kernel.org/patch/10056677/ .../devicetree/bindings/leds/leds-lm3692x.txt | 28 ++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3692x.txt diff --git a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt new file mode 100644 index 000000000000..cfef67bd4100 --- /dev/null +++ b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt @@ -0,0 +1,28 @@ +* Texas Instruments - LM3692x Highly Efficient Dual-String White LED Driver + +The LM36922 is an ultra-compact, highly efficient, +two string white-LED driver designed for LCD display +backlighting. + +Required properties: + - compatible: + "ti,lm3692x" + - reg - I2C slave address + +Optional properties: + - label - Used for naming LEDs + - enable-gpio - gpio pin to enable/disable the device. + - supply - "vled" - LED supply + +Example: + +leds: leds@6 { + compatible = "ti,lm3692x"; + reg = <0x36>; + label = "backlight_cluster"; + enable-gpio = <&gpio1 28 GPIO_ACTIVE_HIGH>; + vled-supply = <&vbatt>; +} + +For more product information please see the link below: +http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf -- 2.15.0.124.g7668cbc60 ^ permalink raw reply related [flat|nested] 42+ messages in thread
[parent not found: <20171115194203.13572-1-dmurphy-l0cyMroinI0@public.gmane.org>]
* [PATCH v3 2/2] leds: lm3692x: Introduce LM3692x dual string driver 2017-11-15 19:42 ` Dan Murphy @ 2017-11-15 19:42 ` Dan Murphy -1 siblings, 0 replies; 42+ messages in thread From: Dan Murphy @ 2017-11-15 19:42 UTC (permalink / raw) To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8, rpurdie-Fm38FmjxZ/leoWH0uzbU5w, jacek.anaszewski-Re5JQEeQqe8AvxtiuMwx3w, pavel-+ZI9xUNit7I Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-leds-u79uwXL29TY76Z2rM5mHXA, Dan Murphy Introducing the LM3692x Dual-String white LED driver. Data sheet is located http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf Signed-off-by: Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org> --- v3 - Add missing Makefile and Kconfig from v1 and v2 - https://patchwork.kernel.org/patch/10060075/ v2 - Added data sheet link, fixed linuxdoc format, returned on failure in init routine, return on fault_check failure, updated brightness calculation and fixed capitalization issue - https://patchwork.kernel.org/patch/10056675/ drivers/leds/Kconfig | 7 + drivers/leds/Makefile | 1 + drivers/leds/leds-lm3692x.c | 390 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 398 insertions(+) create mode 100644 drivers/leds/leds-lm3692x.c diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index 318a28fd58fe..1363e77a7d5f 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -137,6 +137,13 @@ config LEDS_LM3642 converter plus 1.5A constant current driver for a high-current white LED. +config LEDS_LM3692X + tristate "LED support for LM3692x Chips" + depends on LEDS_CLASS && I2C + select REGMAP_I2C + help + This option enables support for the TI LM3692x family + of LED drivers. config LEDS_LOCOMO tristate "LED Support for Locomo device" diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index a2a6b5a4f86d..987884a5b9a5 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -74,6 +74,7 @@ obj-$(CONFIG_LEDS_PM8058) += leds-pm8058.o obj-$(CONFIG_LEDS_MLXCPLD) += leds-mlxcpld.o obj-$(CONFIG_LEDS_NIC78BX) += leds-nic78bx.o obj-$(CONFIG_LEDS_MT6323) += leds-mt6323.o +obj-$(CONFIG_LEDS_LM3692X) += leds-lm3692x.o # LED SPI Drivers obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c new file mode 100644 index 000000000000..9fa7f5a20466 --- /dev/null +++ b/drivers/leds/leds-lm3692x.c @@ -0,0 +1,390 @@ +/* + * TI lm3692x LED Driver + * + * Copyright (C) 2017 Texas Instruments + * + * Author: Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org> + * + * 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. + * + * Data sheet is located + * http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf + */ + +#include <linux/i2c.h> +#include <linux/init.h> +#include <linux/leds.h> +#include <linux/regmap.h> +#include <linux/regulator/consumer.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/of.h> +#include <linux/of_gpio.h> +#include <linux/gpio/consumer.h> +#include <linux/slab.h> + +#define LM3692X_LED_NAME "lm3692x_led" + +#define LM3692X_REV 0x0 +#define LM3692X_RESET 0x1 +#define LM3692X_EN 0x10 +#define LM3692X_BRT_CTRL 0x11 +#define LM3692X_PWM_CTRL 0x12 +#define LM3692X_BOOST_CTRL 0x13 +#define LM3692X_AUTO_FREQ_HI 0x15 +#define LM3692X_AUTO_FREQ_LO 0x16 +#define LM3692X_BL_ADJ_THRESH 0x17 +#define LM3692X_BRT_LSB 0x18 +#define LM3692X_BRT_MSB 0x19 +#define LM3692X_FAULT_CTRL 0x1e +#define LM3692X_FAULT_FLAGS 0x1f + +#define LM3692X_SW_RESET BIT(0) +#define LM3692X_DEVICE_EN BIT(0) +#define LM3692X_LED1_EN BIT(1) +#define LM3692X_LED2_EN BIT(2) + +/* Brightness Control Bits */ +#define LM3692X_BL_ADJ_POL BIT(0) +#define LM3692X_RAMP_RATE_125us 0x00 +#define LM3692X_RAMP_RATE_250us BIT(1) +#define LM3692X_RAMP_RATE_500us BIT(2) +#define LM3692X_RAMP_RATE_1ms (BIT(1) | BIT(2)) +#define LM3692X_RAMP_RATE_2ms BIT(3) +#define LM3692X_RAMP_RATE_4ms (BIT(3) | BIT(1)) +#define LM3692X_RAMP_RATE_8ms (BIT(2) | BIT(3)) +#define LM3692X_RAMP_RATE_16ms (BIT(1) | BIT(2) | BIT(3)) +#define LM3692X_RAMP_EN BIT(4) +#define LM3692X_BRHT_MODE_REG 0x00 +#define LM3692X_BRHT_MODE_PWM BIT(5) +#define LM3692X_BRHT_MODE_MULTI_RAMP BIT(6) +#define LM3692X_BRHT_MODE_RAMP_MULTI (BIT(5) | BIT(6)) +#define LM3692X_MAP_MODE_EXP BIT(7) + +/* PWM Register Bits */ +#define LM3692X_PWM_FILTER_100 BIT(0) +#define LM3692X_PWM_FILTER_150 BIT(1) +#define LM3692X_PWM_FILTER_200 (BIT(0) | BIT(1)) +#define LM3692X_PWM_HYSTER_1LSB BIT(2) +#define LM3692X_PWM_HYSTER_2LSB BIT(3) +#define LM3692X_PWM_HYSTER_3LSB (BIT(3) | BIT(2)) +#define LM3692X_PWM_HYSTER_4LSB BIT(4) +#define LM3692X_PWM_HYSTER_5LSB (BIT(4) | BIT(2)) +#define LM3692X_PWM_HYSTER_6LSB (BIT(4) | BIT(3)) +#define LM3692X_PWM_POLARITY BIT(5) +#define LM3692X_PWM_SAMP_4MHZ BIT(6) +#define LM3692X_PWM_SAMP_24MHZ BIT(7) + +/* Boost Control Bits */ +#define LM3692X_OCP_PROT_1A BIT(0) +#define LM3692X_OCP_PROT_1_25A BIT(1) +#define LM3692X_OCP_PROT_1_5A (BIT(0) | BIT(1)) +#define LM3692X_OVP_21V BIT(2) +#define LM3692X_OVP_25V BIT(3) +#define LM3692X_OVP_29V (BIT(2) | BIT(3)) +#define LM3692X_MIN_IND_22UH BIT(4) +#define LM3692X_BOOST_SW_1MHZ BIT(5) +#define LM3692X_BOOST_SW_NO_SHIFT BIT(6) + +/* Fault Control Bits */ +#define LM3692X_FAULT_CTRL_OVP BIT(0) +#define LM3692X_FAULT_CTRL_OCP BIT(1) +#define LM3692X_FAULT_CTRL_TSD BIT(2) +#define LM3692X_FAULT_CTRL_OPEN BIT(3) + +/* Fault Flag Bits */ +#define LM3692X_FAULT_FLAG_OVP BIT(0) +#define LM3692X_FAULT_FLAG_OCP BIT(1) +#define LM3692X_FAULT_FLAG_TSD BIT(2) +#define LM3692X_FAULT_FLAG_SHRT BIT(3) +#define LM3692X_FAULT_FLAG_OPEN BIT(4) + +/** + * struct lm3692x_led - + * @lock - Lock for reading/writing the device + * @client - Pointer to the I2C client + * @led_dev - led class device pointer + * @regmap - Devices register map + * @enable_gpio - VDDIO/EN gpio to enable communication interface + * @regulator - LED supply regulator pointer + * @label - LED label + */ +struct lm3692x_led { + struct mutex lock; + struct i2c_client *client; + struct led_classdev led_dev; + struct regmap *regmap; + struct gpio_desc *enable_gpio; + struct regulator *regulator; + const char *label; +}; + +static const struct reg_default lm3692x_reg_defs[] = { + {LM3692X_EN, 0xf}, + {LM3692X_BRT_CTRL, 0x61}, + {LM3692X_PWM_CTRL, 0x73}, + {LM3692X_BOOST_CTRL, 0x6f}, + {LM3692X_AUTO_FREQ_HI, 0x0}, + {LM3692X_AUTO_FREQ_LO, 0x0}, + {LM3692X_BL_ADJ_THRESH, 0x0}, + {LM3692X_BRT_LSB, 0x7}, + {LM3692X_BRT_MSB, 0xff}, + {LM3692X_FAULT_CTRL, 0x7}, +}; + +static const struct regmap_config lm3692x_regmap_config = { + .reg_bits = 8, + .val_bits = 8, + + .max_register = LM3692X_FAULT_FLAGS, + .reg_defaults = lm3692x_reg_defs, + .num_reg_defaults = ARRAY_SIZE(lm3692x_reg_defs), + .cache_type = REGCACHE_RBTREE, +}; + +static int lm3692x_fault_check(struct lm3692x_led *led) +{ + int ret, fault; + unsigned int read_buf; + + ret = regmap_read(led->regmap, LM3692X_FAULT_FLAGS, &read_buf); + if (ret) + return ret; + + fault = read_buf; + + if (fault) + dev_err(&led->client->dev, "Detected a fault 0x%X\n", + fault); + + return ret; +} + +static int lm3692x_brightness_set(struct led_classdev *led_cdev, + enum led_brightness brt_val) +{ + struct lm3692x_led *led = + container_of(led_cdev, struct lm3692x_led, led_dev); + int ret; + int led_brightness_lsb = (brt_val >> 5); + + mutex_lock(&led->lock); + + ret = lm3692x_fault_check(led); + if (ret) { + dev_err(&led->client->dev, "Cannot read/clear faults\n"); + goto out; + } + + ret = regmap_write(led->regmap, LM3692X_BRT_MSB, brt_val); + if (ret) { + dev_err(&led->client->dev, "Cannot write MSB\n"); + goto out; + } + + ret = regmap_write(led->regmap, LM3692X_BRT_LSB, led_brightness_lsb); + if (ret) { + dev_err(&led->client->dev, "Cannot write LSB\n"); + goto out; + } +out: + mutex_unlock(&led->lock); + return ret; +} + +static int lm3692x_init(struct lm3692x_led *led) +{ + int ret; + + if (led->regulator) { + ret = regulator_enable(led->regulator); + if (ret) + dev_err(&led->client->dev, + "Failed to enable regulator\n"); + goto out; + } + + if (led->enable_gpio) + gpiod_direction_output(led->enable_gpio, 1); + + ret = lm3692x_fault_check(led); + if (ret) { + dev_err(&led->client->dev, "Cannot read/clear faults\n"); + goto out; + } + + ret = regmap_write(led->regmap, LM3692X_BRT_CTRL, 0x00); + if (ret) { + dev_err(&led->client->dev, "Fail writing BRT CTRL\n"); + goto out; + } + + /* + * For glitch free operation, the following data should + * only be written while device enable bit is 0 + * per Section 7.5.14 of the data sheet + */ + ret = regmap_write(led->regmap, LM3692X_PWM_CTRL, + LM3692X_PWM_FILTER_100 | LM3692X_PWM_SAMP_24MHZ); + if (ret) { + dev_err(&led->client->dev, "Failed programming PWM CTRL\n"); + goto out; + } + + ret = regmap_write(led->regmap, LM3692X_BOOST_CTRL, + LM3692X_BRHT_MODE_RAMP_MULTI | + LM3692X_BL_ADJ_POL | + LM3692X_RAMP_RATE_250us); + if (ret) { + dev_err(&led->client->dev, "Failed programming BOOST CTRL\n"); + goto out; + } + + ret = regmap_write(led->regmap, LM3692X_AUTO_FREQ_HI, 0x00); + if (ret) { + dev_err(&led->client->dev, "Failed programming AUTO HI FREQ\n"); + goto out; + } + + ret = regmap_write(led->regmap, LM3692X_AUTO_FREQ_LO, 0x00); + if (ret) { + dev_err(&led->client->dev, "Failed programming AUTO LO FREQ\n"); + goto out; + } + + ret = regmap_write(led->regmap, LM3692X_BL_ADJ_THRESH, 0x00); + if (ret) { + dev_err(&led->client->dev, "Failed programming BL Threshold\n"); + goto out; + } + + ret = regmap_write(led->regmap, LM3692X_BRT_CTRL, + LM3692X_BL_ADJ_POL | LM3692X_PWM_HYSTER_4LSB); + if (ret) + dev_err(&led->client->dev, "Failed programming BRT CTRL\n"); + + return ret; +out: + + 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"); + } + + return ret; +} + +static int lm3692x_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + int ret; + struct lm3692x_led *led; + struct device_node *np = client->dev.of_node; + + led = devm_kzalloc(&client->dev, sizeof(*led), GFP_KERNEL); + if (!led) + return -ENOMEM; + + if (client->dev.of_node) { + ret = of_property_read_string(np, "label", &led->label); + if (ret) + led->label = LM3692X_LED_NAME; + } + + led->enable_gpio = devm_gpiod_get_optional(&client->dev, + "enable", GPIOD_OUT_LOW); + if (IS_ERR(led->enable_gpio)) { + ret = PTR_ERR(led->enable_gpio); + dev_err(&client->dev, "Failed to get enable gpio: %d\n", ret); + return ret; + } + + led->regulator = devm_regulator_get(&client->dev, "vled"); + if (IS_ERR(led->regulator)) + led->regulator = NULL; + + led->client = client; + led->led_dev.name = led->label; + led->led_dev.max_brightness = LED_FULL; + led->led_dev.brightness_set_blocking = lm3692x_brightness_set; + + mutex_init(&led->lock); + + i2c_set_clientdata(client, led); + + led->regmap = devm_regmap_init_i2c(client, &lm3692x_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 = lm3692x_init(led); + if (ret) + return ret; + + ret = led_classdev_register(&client->dev, &led->led_dev); + if (ret) { + dev_err(&client->dev, "led register err: %d\n", ret); + return ret; + } + + return 0; +} + +static int lm3692x_remove(struct i2c_client *client) +{ + struct lm3692x_led *led = i2c_get_clientdata(client); + int ret; + + led_classdev_unregister(&led->led_dev); + + 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"); + } + + return 0; +} + +static const struct i2c_device_id lm3692x_id[] = { + { "lm3692x", 0 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, lm3692x_id); + +#ifdef CONFIG_OF +static const struct of_device_id of_lm3692x_leds_match[] = { + { .compatible = "ti,lm3692x", }, + {}, +}; +MODULE_DEVICE_TABLE(of, of_lm3692x_leds_match); +#endif + +static struct i2c_driver lm3692x_driver = { + .driver = { + .name = "lm3692x", + .of_match_table = of_match_ptr(of_lm3692x_leds_match), + }, + .probe = lm3692x_probe, + .remove = lm3692x_remove, + .id_table = lm3692x_id, +}; +module_i2c_driver(lm3692x_driver); + +MODULE_DESCRIPTION("Texas Instruments LM3692X LED driver"); +MODULE_AUTHOR("Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org>"); +MODULE_LICENSE("GPL"); -- 2.15.0.124.g7668cbc60 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v3 2/2] leds: lm3692x: Introduce LM3692x dual string driver @ 2017-11-15 19:42 ` Dan Murphy 0 siblings, 0 replies; 42+ messages in thread From: Dan Murphy @ 2017-11-15 19:42 UTC (permalink / raw) To: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, pavel Cc: devicetree, linux-kernel, linux-leds, Dan Murphy Introducing the LM3692x Dual-String white LED driver. Data sheet is located http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf Signed-off-by: Dan Murphy <dmurphy@ti.com> --- v3 - Add missing Makefile and Kconfig from v1 and v2 - https://patchwork.kernel.org/patch/10060075/ v2 - Added data sheet link, fixed linuxdoc format, returned on failure in init routine, return on fault_check failure, updated brightness calculation and fixed capitalization issue - https://patchwork.kernel.org/patch/10056675/ drivers/leds/Kconfig | 7 + drivers/leds/Makefile | 1 + drivers/leds/leds-lm3692x.c | 390 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 398 insertions(+) create mode 100644 drivers/leds/leds-lm3692x.c diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index 318a28fd58fe..1363e77a7d5f 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -137,6 +137,13 @@ config LEDS_LM3642 converter plus 1.5A constant current driver for a high-current white LED. +config LEDS_LM3692X + tristate "LED support for LM3692x Chips" + depends on LEDS_CLASS && I2C + select REGMAP_I2C + help + This option enables support for the TI LM3692x family + of LED drivers. config LEDS_LOCOMO tristate "LED Support for Locomo device" diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index a2a6b5a4f86d..987884a5b9a5 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -74,6 +74,7 @@ obj-$(CONFIG_LEDS_PM8058) += leds-pm8058.o obj-$(CONFIG_LEDS_MLXCPLD) += leds-mlxcpld.o obj-$(CONFIG_LEDS_NIC78BX) += leds-nic78bx.o obj-$(CONFIG_LEDS_MT6323) += leds-mt6323.o +obj-$(CONFIG_LEDS_LM3692X) += leds-lm3692x.o # LED SPI Drivers obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c new file mode 100644 index 000000000000..9fa7f5a20466 --- /dev/null +++ b/drivers/leds/leds-lm3692x.c @@ -0,0 +1,390 @@ +/* + * TI lm3692x LED Driver + * + * Copyright (C) 2017 Texas Instruments + * + * Author: Dan Murphy <dmurphy@ti.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. + * + * Data sheet is located + * http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf + */ + +#include <linux/i2c.h> +#include <linux/init.h> +#include <linux/leds.h> +#include <linux/regmap.h> +#include <linux/regulator/consumer.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/of.h> +#include <linux/of_gpio.h> +#include <linux/gpio/consumer.h> +#include <linux/slab.h> + +#define LM3692X_LED_NAME "lm3692x_led" + +#define LM3692X_REV 0x0 +#define LM3692X_RESET 0x1 +#define LM3692X_EN 0x10 +#define LM3692X_BRT_CTRL 0x11 +#define LM3692X_PWM_CTRL 0x12 +#define LM3692X_BOOST_CTRL 0x13 +#define LM3692X_AUTO_FREQ_HI 0x15 +#define LM3692X_AUTO_FREQ_LO 0x16 +#define LM3692X_BL_ADJ_THRESH 0x17 +#define LM3692X_BRT_LSB 0x18 +#define LM3692X_BRT_MSB 0x19 +#define LM3692X_FAULT_CTRL 0x1e +#define LM3692X_FAULT_FLAGS 0x1f + +#define LM3692X_SW_RESET BIT(0) +#define LM3692X_DEVICE_EN BIT(0) +#define LM3692X_LED1_EN BIT(1) +#define LM3692X_LED2_EN BIT(2) + +/* Brightness Control Bits */ +#define LM3692X_BL_ADJ_POL BIT(0) +#define LM3692X_RAMP_RATE_125us 0x00 +#define LM3692X_RAMP_RATE_250us BIT(1) +#define LM3692X_RAMP_RATE_500us BIT(2) +#define LM3692X_RAMP_RATE_1ms (BIT(1) | BIT(2)) +#define LM3692X_RAMP_RATE_2ms BIT(3) +#define LM3692X_RAMP_RATE_4ms (BIT(3) | BIT(1)) +#define LM3692X_RAMP_RATE_8ms (BIT(2) | BIT(3)) +#define LM3692X_RAMP_RATE_16ms (BIT(1) | BIT(2) | BIT(3)) +#define LM3692X_RAMP_EN BIT(4) +#define LM3692X_BRHT_MODE_REG 0x00 +#define LM3692X_BRHT_MODE_PWM BIT(5) +#define LM3692X_BRHT_MODE_MULTI_RAMP BIT(6) +#define LM3692X_BRHT_MODE_RAMP_MULTI (BIT(5) | BIT(6)) +#define LM3692X_MAP_MODE_EXP BIT(7) + +/* PWM Register Bits */ +#define LM3692X_PWM_FILTER_100 BIT(0) +#define LM3692X_PWM_FILTER_150 BIT(1) +#define LM3692X_PWM_FILTER_200 (BIT(0) | BIT(1)) +#define LM3692X_PWM_HYSTER_1LSB BIT(2) +#define LM3692X_PWM_HYSTER_2LSB BIT(3) +#define LM3692X_PWM_HYSTER_3LSB (BIT(3) | BIT(2)) +#define LM3692X_PWM_HYSTER_4LSB BIT(4) +#define LM3692X_PWM_HYSTER_5LSB (BIT(4) | BIT(2)) +#define LM3692X_PWM_HYSTER_6LSB (BIT(4) | BIT(3)) +#define LM3692X_PWM_POLARITY BIT(5) +#define LM3692X_PWM_SAMP_4MHZ BIT(6) +#define LM3692X_PWM_SAMP_24MHZ BIT(7) + +/* Boost Control Bits */ +#define LM3692X_OCP_PROT_1A BIT(0) +#define LM3692X_OCP_PROT_1_25A BIT(1) +#define LM3692X_OCP_PROT_1_5A (BIT(0) | BIT(1)) +#define LM3692X_OVP_21V BIT(2) +#define LM3692X_OVP_25V BIT(3) +#define LM3692X_OVP_29V (BIT(2) | BIT(3)) +#define LM3692X_MIN_IND_22UH BIT(4) +#define LM3692X_BOOST_SW_1MHZ BIT(5) +#define LM3692X_BOOST_SW_NO_SHIFT BIT(6) + +/* Fault Control Bits */ +#define LM3692X_FAULT_CTRL_OVP BIT(0) +#define LM3692X_FAULT_CTRL_OCP BIT(1) +#define LM3692X_FAULT_CTRL_TSD BIT(2) +#define LM3692X_FAULT_CTRL_OPEN BIT(3) + +/* Fault Flag Bits */ +#define LM3692X_FAULT_FLAG_OVP BIT(0) +#define LM3692X_FAULT_FLAG_OCP BIT(1) +#define LM3692X_FAULT_FLAG_TSD BIT(2) +#define LM3692X_FAULT_FLAG_SHRT BIT(3) +#define LM3692X_FAULT_FLAG_OPEN BIT(4) + +/** + * struct lm3692x_led - + * @lock - Lock for reading/writing the device + * @client - Pointer to the I2C client + * @led_dev - led class device pointer + * @regmap - Devices register map + * @enable_gpio - VDDIO/EN gpio to enable communication interface + * @regulator - LED supply regulator pointer + * @label - LED label + */ +struct lm3692x_led { + struct mutex lock; + struct i2c_client *client; + struct led_classdev led_dev; + struct regmap *regmap; + struct gpio_desc *enable_gpio; + struct regulator *regulator; + const char *label; +}; + +static const struct reg_default lm3692x_reg_defs[] = { + {LM3692X_EN, 0xf}, + {LM3692X_BRT_CTRL, 0x61}, + {LM3692X_PWM_CTRL, 0x73}, + {LM3692X_BOOST_CTRL, 0x6f}, + {LM3692X_AUTO_FREQ_HI, 0x0}, + {LM3692X_AUTO_FREQ_LO, 0x0}, + {LM3692X_BL_ADJ_THRESH, 0x0}, + {LM3692X_BRT_LSB, 0x7}, + {LM3692X_BRT_MSB, 0xff}, + {LM3692X_FAULT_CTRL, 0x7}, +}; + +static const struct regmap_config lm3692x_regmap_config = { + .reg_bits = 8, + .val_bits = 8, + + .max_register = LM3692X_FAULT_FLAGS, + .reg_defaults = lm3692x_reg_defs, + .num_reg_defaults = ARRAY_SIZE(lm3692x_reg_defs), + .cache_type = REGCACHE_RBTREE, +}; + +static int lm3692x_fault_check(struct lm3692x_led *led) +{ + int ret, fault; + unsigned int read_buf; + + ret = regmap_read(led->regmap, LM3692X_FAULT_FLAGS, &read_buf); + if (ret) + return ret; + + fault = read_buf; + + if (fault) + dev_err(&led->client->dev, "Detected a fault 0x%X\n", + fault); + + return ret; +} + +static int lm3692x_brightness_set(struct led_classdev *led_cdev, + enum led_brightness brt_val) +{ + struct lm3692x_led *led = + container_of(led_cdev, struct lm3692x_led, led_dev); + int ret; + int led_brightness_lsb = (brt_val >> 5); + + mutex_lock(&led->lock); + + ret = lm3692x_fault_check(led); + if (ret) { + dev_err(&led->client->dev, "Cannot read/clear faults\n"); + goto out; + } + + ret = regmap_write(led->regmap, LM3692X_BRT_MSB, brt_val); + if (ret) { + dev_err(&led->client->dev, "Cannot write MSB\n"); + goto out; + } + + ret = regmap_write(led->regmap, LM3692X_BRT_LSB, led_brightness_lsb); + if (ret) { + dev_err(&led->client->dev, "Cannot write LSB\n"); + goto out; + } +out: + mutex_unlock(&led->lock); + return ret; +} + +static int lm3692x_init(struct lm3692x_led *led) +{ + int ret; + + if (led->regulator) { + ret = regulator_enable(led->regulator); + if (ret) + dev_err(&led->client->dev, + "Failed to enable regulator\n"); + goto out; + } + + if (led->enable_gpio) + gpiod_direction_output(led->enable_gpio, 1); + + ret = lm3692x_fault_check(led); + if (ret) { + dev_err(&led->client->dev, "Cannot read/clear faults\n"); + goto out; + } + + ret = regmap_write(led->regmap, LM3692X_BRT_CTRL, 0x00); + if (ret) { + dev_err(&led->client->dev, "Fail writing BRT CTRL\n"); + goto out; + } + + /* + * For glitch free operation, the following data should + * only be written while device enable bit is 0 + * per Section 7.5.14 of the data sheet + */ + ret = regmap_write(led->regmap, LM3692X_PWM_CTRL, + LM3692X_PWM_FILTER_100 | LM3692X_PWM_SAMP_24MHZ); + if (ret) { + dev_err(&led->client->dev, "Failed programming PWM CTRL\n"); + goto out; + } + + ret = regmap_write(led->regmap, LM3692X_BOOST_CTRL, + LM3692X_BRHT_MODE_RAMP_MULTI | + LM3692X_BL_ADJ_POL | + LM3692X_RAMP_RATE_250us); + if (ret) { + dev_err(&led->client->dev, "Failed programming BOOST CTRL\n"); + goto out; + } + + ret = regmap_write(led->regmap, LM3692X_AUTO_FREQ_HI, 0x00); + if (ret) { + dev_err(&led->client->dev, "Failed programming AUTO HI FREQ\n"); + goto out; + } + + ret = regmap_write(led->regmap, LM3692X_AUTO_FREQ_LO, 0x00); + if (ret) { + dev_err(&led->client->dev, "Failed programming AUTO LO FREQ\n"); + goto out; + } + + ret = regmap_write(led->regmap, LM3692X_BL_ADJ_THRESH, 0x00); + if (ret) { + dev_err(&led->client->dev, "Failed programming BL Threshold\n"); + goto out; + } + + ret = regmap_write(led->regmap, LM3692X_BRT_CTRL, + LM3692X_BL_ADJ_POL | LM3692X_PWM_HYSTER_4LSB); + if (ret) + dev_err(&led->client->dev, "Failed programming BRT CTRL\n"); + + return ret; +out: + + 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"); + } + + return ret; +} + +static int lm3692x_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + int ret; + struct lm3692x_led *led; + struct device_node *np = client->dev.of_node; + + led = devm_kzalloc(&client->dev, sizeof(*led), GFP_KERNEL); + if (!led) + return -ENOMEM; + + if (client->dev.of_node) { + ret = of_property_read_string(np, "label", &led->label); + if (ret) + led->label = LM3692X_LED_NAME; + } + + led->enable_gpio = devm_gpiod_get_optional(&client->dev, + "enable", GPIOD_OUT_LOW); + if (IS_ERR(led->enable_gpio)) { + ret = PTR_ERR(led->enable_gpio); + dev_err(&client->dev, "Failed to get enable gpio: %d\n", ret); + return ret; + } + + led->regulator = devm_regulator_get(&client->dev, "vled"); + if (IS_ERR(led->regulator)) + led->regulator = NULL; + + led->client = client; + led->led_dev.name = led->label; + led->led_dev.max_brightness = LED_FULL; + led->led_dev.brightness_set_blocking = lm3692x_brightness_set; + + mutex_init(&led->lock); + + i2c_set_clientdata(client, led); + + led->regmap = devm_regmap_init_i2c(client, &lm3692x_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 = lm3692x_init(led); + if (ret) + return ret; + + ret = led_classdev_register(&client->dev, &led->led_dev); + if (ret) { + dev_err(&client->dev, "led register err: %d\n", ret); + return ret; + } + + return 0; +} + +static int lm3692x_remove(struct i2c_client *client) +{ + struct lm3692x_led *led = i2c_get_clientdata(client); + int ret; + + led_classdev_unregister(&led->led_dev); + + 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"); + } + + return 0; +} + +static const struct i2c_device_id lm3692x_id[] = { + { "lm3692x", 0 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, lm3692x_id); + +#ifdef CONFIG_OF +static const struct of_device_id of_lm3692x_leds_match[] = { + { .compatible = "ti,lm3692x", }, + {}, +}; +MODULE_DEVICE_TABLE(of, of_lm3692x_leds_match); +#endif + +static struct i2c_driver lm3692x_driver = { + .driver = { + .name = "lm3692x", + .of_match_table = of_match_ptr(of_lm3692x_leds_match), + }, + .probe = lm3692x_probe, + .remove = lm3692x_remove, + .id_table = lm3692x_id, +}; +module_i2c_driver(lm3692x_driver); + +MODULE_DESCRIPTION("Texas Instruments LM3692X LED driver"); +MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com>"); +MODULE_LICENSE("GPL"); -- 2.15.0.124.g7668cbc60 ^ permalink raw reply related [flat|nested] 42+ messages in thread
[parent not found: <20171115194203.13572-2-dmurphy-l0cyMroinI0@public.gmane.org>]
* Re: [PATCH v3 2/2] leds: lm3692x: Introduce LM3692x dual string driver 2017-11-15 19:42 ` Dan Murphy @ 2017-11-15 21:12 ` Pavel Machek -1 siblings, 0 replies; 42+ messages in thread From: Pavel Machek @ 2017-11-15 21:12 UTC (permalink / raw) To: Dan Murphy Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8, rpurdie-Fm38FmjxZ/leoWH0uzbU5w, jacek.anaszewski-Re5JQEeQqe8AvxtiuMwx3w, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-leds-u79uwXL29TY76Z2rM5mHXA [-- Attachment #1: Type: text/plain, Size: 2060 bytes --] Hi! > Introducing the LM3692x Dual-String white LED driver. > > Data sheet is located "located at"? (Twice.) > http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf > > Signed-off-by: Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org> > +config LEDS_LM3692X > + tristate "LED support for LM3692x Chips" > + depends on LEDS_CLASS && I2C > + select REGMAP_I2C > + help > + This option enables support for the TI LM3692x family > + of LED drivers. "If unsure ..., module will be named..." Might want to say this is for backlight LEDs here. > +static int lm3692x_fault_check(struct lm3692x_led *led) > +{ > + int ret, fault; > + unsigned int read_buf; > + > + ret = regmap_read(led->regmap, LM3692X_FAULT_FLAGS, &read_buf); > + if (ret) > + return ret; > + > + fault = read_buf; > + > + if (fault) > + dev_err(&led->client->dev, "Detected a fault 0x%X\n", > + fault); > + > + return ret; > +} Get rid of "fault" variable? Does fault need to be propagated to the caller? > +static int lm3692x_init(struct lm3692x_led *led) > +{ > + int ret; > + > + if (led->regulator) { > + ret = regulator_enable(led->regulator); > + if (ret) > + dev_err(&led->client->dev, > + "Failed to enable regulator\n"); > + goto out; > + } > + > + if (led->enable_gpio) > + gpiod_direction_output(led->enable_gpio, 1); > + > + ret = lm3692x_fault_check(led); > + if (ret) { > + dev_err(&led->client->dev, "Cannot read/clear faults\n"); > + goto out; > + } > + > + ret = regmap_write(led->regmap, LM3692X_BRT_CTRL, 0x00); > + if (ret) { > + dev_err(&led->client->dev, "Fail writing BRT CTRL\n"); > + goto out; > + } How often are those fails reached? Maybe regmap wrapper that would print "reading/writing register XY failed" would be enough? Otherwise looks good, Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org> Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 2/2] leds: lm3692x: Introduce LM3692x dual string driver @ 2017-11-15 21:12 ` Pavel Machek 0 siblings, 0 replies; 42+ messages in thread From: Pavel Machek @ 2017-11-15 21:12 UTC (permalink / raw) To: Dan Murphy Cc: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, devicetree, linux-kernel, linux-leds [-- Attachment #1: Type: text/plain, Size: 2016 bytes --] Hi! > Introducing the LM3692x Dual-String white LED driver. > > Data sheet is located "located at"? (Twice.) > http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf > > Signed-off-by: Dan Murphy <dmurphy@ti.com> > +config LEDS_LM3692X > + tristate "LED support for LM3692x Chips" > + depends on LEDS_CLASS && I2C > + select REGMAP_I2C > + help > + This option enables support for the TI LM3692x family > + of LED drivers. "If unsure ..., module will be named..." Might want to say this is for backlight LEDs here. > +static int lm3692x_fault_check(struct lm3692x_led *led) > +{ > + int ret, fault; > + unsigned int read_buf; > + > + ret = regmap_read(led->regmap, LM3692X_FAULT_FLAGS, &read_buf); > + if (ret) > + return ret; > + > + fault = read_buf; > + > + if (fault) > + dev_err(&led->client->dev, "Detected a fault 0x%X\n", > + fault); > + > + return ret; > +} Get rid of "fault" variable? Does fault need to be propagated to the caller? > +static int lm3692x_init(struct lm3692x_led *led) > +{ > + int ret; > + > + if (led->regulator) { > + ret = regulator_enable(led->regulator); > + if (ret) > + dev_err(&led->client->dev, > + "Failed to enable regulator\n"); > + goto out; > + } > + > + if (led->enable_gpio) > + gpiod_direction_output(led->enable_gpio, 1); > + > + ret = lm3692x_fault_check(led); > + if (ret) { > + dev_err(&led->client->dev, "Cannot read/clear faults\n"); > + goto out; > + } > + > + ret = regmap_write(led->regmap, LM3692X_BRT_CTRL, 0x00); > + if (ret) { > + dev_err(&led->client->dev, "Fail writing BRT CTRL\n"); > + goto out; > + } How often are those fails reached? Maybe regmap wrapper that would print "reading/writing register XY failed" would be enough? Otherwise looks good, Acked-by: Pavel Machek <pavel@ucw.cz> Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 2/2] leds: lm3692x: Introduce LM3692x dual string driver 2017-11-15 21:12 ` Pavel Machek @ 2017-11-15 21:25 ` Dan Murphy -1 siblings, 0 replies; 42+ messages in thread From: Dan Murphy @ 2017-11-15 21:25 UTC (permalink / raw) To: Pavel Machek Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8, rpurdie-Fm38FmjxZ/leoWH0uzbU5w, jacek.anaszewski-Re5JQEeQqe8AvxtiuMwx3w, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-leds-u79uwXL29TY76Z2rM5mHXA Pavel On 11/15/2017 03:12 PM, Pavel Machek wrote: > Hi! > >> Introducing the LM3692x Dual-String white LED driver. >> >> Data sheet is located > > "located at"? (Twice.) Actually it is 3x since I call it out in the dt binding as well. So what to eliminate? > >> http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf >> >> Signed-off-by: Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org> > >> +config LEDS_LM3692X >> + tristate "LED support for LM3692x Chips" >> + depends on LEDS_CLASS && I2C >> + select REGMAP_I2C >> + help >> + This option enables support for the TI LM3692x family >> + of LED drivers. > > "If unsure ..., module will be named..." > > Might want to say this is for backlight LEDs here. AcK > >> +static int lm3692x_fault_check(struct lm3692x_led *led) >> +{ >> + int ret, fault; >> + unsigned int read_buf; >> + >> + ret = regmap_read(led->regmap, LM3692X_FAULT_FLAGS, &read_buf); >> + if (ret) >> + return ret; >> + >> + fault = read_buf; >> + >> + if (fault) >> + dev_err(&led->client->dev, "Detected a fault 0x%X\n", >> + fault); >> + >> + return ret; >> +} > > Get rid of "fault" variable? > > Does fault need to be propagated to the caller? I should probably set ret to fail if I see a fault or as you suggest propagate the fault > > >> +static int lm3692x_init(struct lm3692x_led *led) >> +{ >> + int ret; >> + >> + if (led->regulator) { >> + ret = regulator_enable(led->regulator); >> + if (ret) >> + dev_err(&led->client->dev, >> + "Failed to enable regulator\n"); >> + goto out; >> + } >> + >> + if (led->enable_gpio) >> + gpiod_direction_output(led->enable_gpio, 1); >> + >> + ret = lm3692x_fault_check(led); >> + if (ret) { >> + dev_err(&led->client->dev, "Cannot read/clear faults\n"); >> + goto out; >> + } >> + >> + ret = regmap_write(led->regmap, LM3692X_BRT_CTRL, 0x00); >> + if (ret) { >> + dev_err(&led->client->dev, "Fail writing BRT CTRL\n"); >> + goto out; >> + } > > How often are those fails reached? Maybe regmap wrapper that would > print "reading/writing register XY failed" would be enough? Or maybe in the out label I can just dev_err once saying initializing the device failed. These are really only called at probe time. Its the only time init is called. Dan > > Otherwise looks good, > > Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org> > Pavel > -- ------------------ Dan Murphy -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 2/2] leds: lm3692x: Introduce LM3692x dual string driver @ 2017-11-15 21:25 ` Dan Murphy 0 siblings, 0 replies; 42+ messages in thread From: Dan Murphy @ 2017-11-15 21:25 UTC (permalink / raw) To: Pavel Machek Cc: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, devicetree, linux-kernel, linux-leds Pavel On 11/15/2017 03:12 PM, Pavel Machek wrote: > Hi! > >> Introducing the LM3692x Dual-String white LED driver. >> >> Data sheet is located > > "located at"? (Twice.) Actually it is 3x since I call it out in the dt binding as well. So what to eliminate? > >> http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf >> >> Signed-off-by: Dan Murphy <dmurphy@ti.com> > >> +config LEDS_LM3692X >> + tristate "LED support for LM3692x Chips" >> + depends on LEDS_CLASS && I2C >> + select REGMAP_I2C >> + help >> + This option enables support for the TI LM3692x family >> + of LED drivers. > > "If unsure ..., module will be named..." > > Might want to say this is for backlight LEDs here. AcK > >> +static int lm3692x_fault_check(struct lm3692x_led *led) >> +{ >> + int ret, fault; >> + unsigned int read_buf; >> + >> + ret = regmap_read(led->regmap, LM3692X_FAULT_FLAGS, &read_buf); >> + if (ret) >> + return ret; >> + >> + fault = read_buf; >> + >> + if (fault) >> + dev_err(&led->client->dev, "Detected a fault 0x%X\n", >> + fault); >> + >> + return ret; >> +} > > Get rid of "fault" variable? > > Does fault need to be propagated to the caller? I should probably set ret to fail if I see a fault or as you suggest propagate the fault > > >> +static int lm3692x_init(struct lm3692x_led *led) >> +{ >> + int ret; >> + >> + if (led->regulator) { >> + ret = regulator_enable(led->regulator); >> + if (ret) >> + dev_err(&led->client->dev, >> + "Failed to enable regulator\n"); >> + goto out; >> + } >> + >> + if (led->enable_gpio) >> + gpiod_direction_output(led->enable_gpio, 1); >> + >> + ret = lm3692x_fault_check(led); >> + if (ret) { >> + dev_err(&led->client->dev, "Cannot read/clear faults\n"); >> + goto out; >> + } >> + >> + ret = regmap_write(led->regmap, LM3692X_BRT_CTRL, 0x00); >> + if (ret) { >> + dev_err(&led->client->dev, "Fail writing BRT CTRL\n"); >> + goto out; >> + } > > How often are those fails reached? Maybe regmap wrapper that would > print "reading/writing register XY failed" would be enough? Or maybe in the out label I can just dev_err once saying initializing the device failed. These are really only called at probe time. Its the only time init is called. Dan > > Otherwise looks good, > > Acked-by: Pavel Machek <pavel@ucw.cz> > Pavel > -- ------------------ Dan Murphy ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 2/2] leds: lm3692x: Introduce LM3692x dual string driver 2017-11-15 19:42 ` Dan Murphy @ 2017-11-18 14:19 ` Jacek Anaszewski -1 siblings, 0 replies; 42+ messages in thread From: Jacek Anaszewski @ 2017-11-18 14:19 UTC (permalink / raw) To: Dan Murphy, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8, rpurdie-Fm38FmjxZ/leoWH0uzbU5w, pavel-+ZI9xUNit7I Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-leds-u79uwXL29TY76Z2rM5mHXA Hi Dan, Please find my comments below. On 11/15/2017 08:42 PM, Dan Murphy wrote: > Introducing the LM3692x Dual-String white LED driver. > > Data sheet is located > http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf > > Signed-off-by: Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org> > --- > > v3 - Add missing Makefile and Kconfig from v1 and v2 - https://patchwork.kernel.org/patch/10060075/ > > v2 - Added data sheet link, fixed linuxdoc format, returned on failure in init > routine, return on fault_check failure, updated brightness calculation and > fixed capitalization issue - https://patchwork.kernel.org/patch/10056675/ > > drivers/leds/Kconfig | 7 + > drivers/leds/Makefile | 1 + > drivers/leds/leds-lm3692x.c | 390 ++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 398 insertions(+) > create mode 100644 drivers/leds/leds-lm3692x.c > > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > index 318a28fd58fe..1363e77a7d5f 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -137,6 +137,13 @@ config LEDS_LM3642 > converter plus 1.5A constant current driver for a high-current > white LED. > > +config LEDS_LM3692X > + tristate "LED support for LM3692x Chips" > + depends on LEDS_CLASS && I2C > + select REGMAP_I2C > + help > + This option enables support for the TI LM3692x family > + of LED drivers. > > config LEDS_LOCOMO > tristate "LED Support for Locomo device" > diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile > index a2a6b5a4f86d..987884a5b9a5 100644 > --- a/drivers/leds/Makefile > +++ b/drivers/leds/Makefile > @@ -74,6 +74,7 @@ obj-$(CONFIG_LEDS_PM8058) += leds-pm8058.o > obj-$(CONFIG_LEDS_MLXCPLD) += leds-mlxcpld.o > obj-$(CONFIG_LEDS_NIC78BX) += leds-nic78bx.o > obj-$(CONFIG_LEDS_MT6323) += leds-mt6323.o > +obj-$(CONFIG_LEDS_LM3692X) += leds-lm3692x.o > > # LED SPI Drivers > obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o > diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c > new file mode 100644 > index 000000000000..9fa7f5a20466 > --- /dev/null > +++ b/drivers/leds/leds-lm3692x.c > @@ -0,0 +1,390 @@ > +/* > + * TI lm3692x LED Driver > + * > + * Copyright (C) 2017 Texas Instruments > + * > + * Author: Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org> > + * > + * 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. > + * > + * Data sheet is located > + * http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf > + */ > + > +#include <linux/i2c.h> > +#include <linux/init.h> > +#include <linux/leds.h> > +#include <linux/regmap.h> > +#include <linux/regulator/consumer.h> > +#include <linux/module.h> > +#include <linux/mutex.h> > +#include <linux/of.h> > +#include <linux/of_gpio.h> > +#include <linux/gpio/consumer.h> > +#include <linux/slab.h> > + > +#define LM3692X_LED_NAME "lm3692x_led" > + > +#define LM3692X_REV 0x0 > +#define LM3692X_RESET 0x1 > +#define LM3692X_EN 0x10 > +#define LM3692X_BRT_CTRL 0x11 > +#define LM3692X_PWM_CTRL 0x12 > +#define LM3692X_BOOST_CTRL 0x13 > +#define LM3692X_AUTO_FREQ_HI 0x15 > +#define LM3692X_AUTO_FREQ_LO 0x16 > +#define LM3692X_BL_ADJ_THRESH 0x17 > +#define LM3692X_BRT_LSB 0x18 > +#define LM3692X_BRT_MSB 0x19 > +#define LM3692X_FAULT_CTRL 0x1e > +#define LM3692X_FAULT_FLAGS 0x1f > + > +#define LM3692X_SW_RESET BIT(0) > +#define LM3692X_DEVICE_EN BIT(0) > +#define LM3692X_LED1_EN BIT(1) > +#define LM3692X_LED2_EN BIT(2) > + > +/* Brightness Control Bits */ > +#define LM3692X_BL_ADJ_POL BIT(0) > +#define LM3692X_RAMP_RATE_125us 0x00 > +#define LM3692X_RAMP_RATE_250us BIT(1) > +#define LM3692X_RAMP_RATE_500us BIT(2) > +#define LM3692X_RAMP_RATE_1ms (BIT(1) | BIT(2)) > +#define LM3692X_RAMP_RATE_2ms BIT(3) > +#define LM3692X_RAMP_RATE_4ms (BIT(3) | BIT(1)) > +#define LM3692X_RAMP_RATE_8ms (BIT(2) | BIT(3)) > +#define LM3692X_RAMP_RATE_16ms (BIT(1) | BIT(2) | BIT(3)) > +#define LM3692X_RAMP_EN BIT(4) > +#define LM3692X_BRHT_MODE_REG 0x00 > +#define LM3692X_BRHT_MODE_PWM BIT(5) > +#define LM3692X_BRHT_MODE_MULTI_RAMP BIT(6) > +#define LM3692X_BRHT_MODE_RAMP_MULTI (BIT(5) | BIT(6)) > +#define LM3692X_MAP_MODE_EXP BIT(7) > + > +/* PWM Register Bits */ > +#define LM3692X_PWM_FILTER_100 BIT(0) > +#define LM3692X_PWM_FILTER_150 BIT(1) > +#define LM3692X_PWM_FILTER_200 (BIT(0) | BIT(1)) > +#define LM3692X_PWM_HYSTER_1LSB BIT(2) > +#define LM3692X_PWM_HYSTER_2LSB BIT(3) > +#define LM3692X_PWM_HYSTER_3LSB (BIT(3) | BIT(2)) > +#define LM3692X_PWM_HYSTER_4LSB BIT(4) > +#define LM3692X_PWM_HYSTER_5LSB (BIT(4) | BIT(2)) > +#define LM3692X_PWM_HYSTER_6LSB (BIT(4) | BIT(3)) > +#define LM3692X_PWM_POLARITY BIT(5) > +#define LM3692X_PWM_SAMP_4MHZ BIT(6) > +#define LM3692X_PWM_SAMP_24MHZ BIT(7) > + > +/* Boost Control Bits */ > +#define LM3692X_OCP_PROT_1A BIT(0) > +#define LM3692X_OCP_PROT_1_25A BIT(1) > +#define LM3692X_OCP_PROT_1_5A (BIT(0) | BIT(1)) > +#define LM3692X_OVP_21V BIT(2) > +#define LM3692X_OVP_25V BIT(3) > +#define LM3692X_OVP_29V (BIT(2) | BIT(3)) > +#define LM3692X_MIN_IND_22UH BIT(4) > +#define LM3692X_BOOST_SW_1MHZ BIT(5) > +#define LM3692X_BOOST_SW_NO_SHIFT BIT(6) > + > +/* Fault Control Bits */ > +#define LM3692X_FAULT_CTRL_OVP BIT(0) > +#define LM3692X_FAULT_CTRL_OCP BIT(1) > +#define LM3692X_FAULT_CTRL_TSD BIT(2) > +#define LM3692X_FAULT_CTRL_OPEN BIT(3) > + > +/* Fault Flag Bits */ > +#define LM3692X_FAULT_FLAG_OVP BIT(0) > +#define LM3692X_FAULT_FLAG_OCP BIT(1) > +#define LM3692X_FAULT_FLAG_TSD BIT(2) > +#define LM3692X_FAULT_FLAG_SHRT BIT(3) > +#define LM3692X_FAULT_FLAG_OPEN BIT(4) > + > +/** > + * struct lm3692x_led - > + * @lock - Lock for reading/writing the device > + * @client - Pointer to the I2C client > + * @led_dev - led class device pointer > + * @regmap - Devices register map > + * @enable_gpio - VDDIO/EN gpio to enable communication interface > + * @regulator - LED supply regulator pointer > + * @label - LED label > + */ > +struct lm3692x_led { > + struct mutex lock; > + struct i2c_client *client; > + struct led_classdev led_dev; > + struct regmap *regmap; > + struct gpio_desc *enable_gpio; > + struct regulator *regulator; > + const char *label; > +}; > + > +static const struct reg_default lm3692x_reg_defs[] = { > + {LM3692X_EN, 0xf}, > + {LM3692X_BRT_CTRL, 0x61}, > + {LM3692X_PWM_CTRL, 0x73}, > + {LM3692X_BOOST_CTRL, 0x6f}, > + {LM3692X_AUTO_FREQ_HI, 0x0}, > + {LM3692X_AUTO_FREQ_LO, 0x0}, > + {LM3692X_BL_ADJ_THRESH, 0x0}, > + {LM3692X_BRT_LSB, 0x7}, > + {LM3692X_BRT_MSB, 0xff}, > + {LM3692X_FAULT_CTRL, 0x7}, > +}; > + > +static const struct regmap_config lm3692x_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > + > + .max_register = LM3692X_FAULT_FLAGS, > + .reg_defaults = lm3692x_reg_defs, > + .num_reg_defaults = ARRAY_SIZE(lm3692x_reg_defs), > + .cache_type = REGCACHE_RBTREE, > +}; > + > +static int lm3692x_fault_check(struct lm3692x_led *led) > +{ > + int ret, fault; > + unsigned int read_buf; > + > + ret = regmap_read(led->regmap, LM3692X_FAULT_FLAGS, &read_buf); > + if (ret) > + return ret; > + > + fault = read_buf; > + > + if (fault) > + dev_err(&led->client->dev, "Detected a fault 0x%X\n", > + fault); > + > + return ret; > +} > + > +static int lm3692x_brightness_set(struct led_classdev *led_cdev, > + enum led_brightness brt_val) > +{ > + struct lm3692x_led *led = > + container_of(led_cdev, struct lm3692x_led, led_dev); > + int ret; > + int led_brightness_lsb = (brt_val >> 5); > + > + mutex_lock(&led->lock); > + > + ret = lm3692x_fault_check(led); > + if (ret) { > + dev_err(&led->client->dev, "Cannot read/clear faults\n"); > + goto out; > + } > + > + ret = regmap_write(led->regmap, LM3692X_BRT_MSB, brt_val); > + if (ret) { > + dev_err(&led->client->dev, "Cannot write MSB\n"); > + goto out; > + } > + > + ret = regmap_write(led->regmap, LM3692X_BRT_LSB, led_brightness_lsb); > + if (ret) { > + dev_err(&led->client->dev, "Cannot write LSB\n"); > + goto out; > + } > +out: > + mutex_unlock(&led->lock); > + return ret; > +} > + > +static int lm3692x_init(struct lm3692x_led *led) > +{ > + int ret; > + > + if (led->regulator) { > + ret = regulator_enable(led->regulator); > + if (ret) > + dev_err(&led->client->dev, > + "Failed to enable regulator\n"); > + goto out; > + } > + > + if (led->enable_gpio) > + gpiod_direction_output(led->enable_gpio, 1); > + > + ret = lm3692x_fault_check(led); > + if (ret) { > + dev_err(&led->client->dev, "Cannot read/clear faults\n"); > + goto out; > + } > + > + ret = regmap_write(led->regmap, LM3692X_BRT_CTRL, 0x00); > + if (ret) { > + dev_err(&led->client->dev, "Fail writing BRT CTRL\n"); > + goto out; > + } > + > + /* > + * For glitch free operation, the following data should > + * only be written while device enable bit is 0 > + * per Section 7.5.14 of the data sheet > + */ > + ret = regmap_write(led->regmap, LM3692X_PWM_CTRL, > + LM3692X_PWM_FILTER_100 | LM3692X_PWM_SAMP_24MHZ); > + if (ret) { > + dev_err(&led->client->dev, "Failed programming PWM CTRL\n"); > + goto out; > + } > + > + ret = regmap_write(led->regmap, LM3692X_BOOST_CTRL, > + LM3692X_BRHT_MODE_RAMP_MULTI | > + LM3692X_BL_ADJ_POL | > + LM3692X_RAMP_RATE_250us); > + if (ret) { > + dev_err(&led->client->dev, "Failed programming BOOST CTRL\n"); > + goto out; > + } > + > + ret = regmap_write(led->regmap, LM3692X_AUTO_FREQ_HI, 0x00); > + if (ret) { > + dev_err(&led->client->dev, "Failed programming AUTO HI FREQ\n"); > + goto out; > + } > + > + ret = regmap_write(led->regmap, LM3692X_AUTO_FREQ_LO, 0x00); > + if (ret) { > + dev_err(&led->client->dev, "Failed programming AUTO LO FREQ\n"); > + goto out; > + } > + > + ret = regmap_write(led->regmap, LM3692X_BL_ADJ_THRESH, 0x00); > + if (ret) { > + dev_err(&led->client->dev, "Failed programming BL Threshold\n"); > + goto out; > + } > + > + ret = regmap_write(led->regmap, LM3692X_BRT_CTRL, > + LM3692X_BL_ADJ_POL | LM3692X_PWM_HYSTER_4LSB); > + if (ret) > + dev_err(&led->client->dev, "Failed programming BRT CTRL\n"); > + > + return ret; > +out: > + > + 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"); > + } > + > + return ret; > +} > + > +static int lm3692x_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + int ret; > + struct lm3692x_led *led; > + struct device_node *np = client->dev.of_node; > + > + led = devm_kzalloc(&client->dev, sizeof(*led), GFP_KERNEL); > + if (!led) > + return -ENOMEM; > + > + if (client->dev.of_node) { We should fail in the opposite case. > + ret = of_property_read_string(np, "label", &led->label); > + if (ret) > + led->label = LM3692X_LED_NAME; LED label should be taken from the child node name if label property is not present. See Documentation/devicetree/bindings/leds/common.txt. We have some mess in this area, but I'm going to sort it out soon (added you on CC to the related discussion). Generally LED class device name should be composed by the LED class driver by taking label and prefixing it with "[devicename]:" string. > + } > + > + led->enable_gpio = devm_gpiod_get_optional(&client->dev, > + "enable", GPIOD_OUT_LOW); > + if (IS_ERR(led->enable_gpio)) { > + ret = PTR_ERR(led->enable_gpio); > + dev_err(&client->dev, "Failed to get enable gpio: %d\n", ret); > + return ret; > + } > + > + led->regulator = devm_regulator_get(&client->dev, "vled"); > + if (IS_ERR(led->regulator)) > + led->regulator = NULL; > + > + led->client = client; > + led->led_dev.name = led->label; > + led->led_dev.max_brightness = LED_FULL; You can skip this line, it will be initialized to LED_FULL in of_led_classdev_register in this case. > + led->led_dev.brightness_set_blocking = lm3692x_brightness_set; > + > + mutex_init(&led->lock); > + > + i2c_set_clientdata(client, led); > + > + led->regmap = devm_regmap_init_i2c(client, &lm3692x_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 = lm3692x_init(led); > + if (ret) > + return ret; > + > + ret = led_classdev_register(&client->dev, &led->led_dev); Please use devm_* prefixed version. > + if (ret) { > + dev_err(&client->dev, "led register err: %d\n", ret); > + return ret; > + } > + > + return 0; > +} > + > +static int lm3692x_remove(struct i2c_client *client) > +{ > + struct lm3692x_led *led = i2c_get_clientdata(client); > + int ret; > + > + led_classdev_unregister(&led->led_dev); > + > + 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 lm3692x_id[] = { > + { "lm3692x", 0 }, > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, lm3692x_id); > + > +#ifdef CONFIG_OF > +static const struct of_device_id of_lm3692x_leds_match[] = { > + { .compatible = "ti,lm3692x", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, of_lm3692x_leds_match); > +#endif > + > +static struct i2c_driver lm3692x_driver = { > + .driver = { > + .name = "lm3692x", > + .of_match_table = of_match_ptr(of_lm3692x_leds_match), > + }, > + .probe = lm3692x_probe, > + .remove = lm3692x_remove, > + .id_table = lm3692x_id, > +}; > +module_i2c_driver(lm3692x_driver); > + > +MODULE_DESCRIPTION("Texas Instruments LM3692X LED driver"); > +MODULE_AUTHOR("Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org>"); > +MODULE_LICENSE("GPL"); According to your license info it should be "GPL v2" -- Best regards, Jacek Anaszewski -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 2/2] leds: lm3692x: Introduce LM3692x dual string driver @ 2017-11-18 14:19 ` Jacek Anaszewski 0 siblings, 0 replies; 42+ messages in thread From: Jacek Anaszewski @ 2017-11-18 14:19 UTC (permalink / raw) To: Dan Murphy, robh+dt, mark.rutland, rpurdie, pavel Cc: devicetree, linux-kernel, linux-leds Hi Dan, Please find my comments below. On 11/15/2017 08:42 PM, Dan Murphy wrote: > Introducing the LM3692x Dual-String white LED driver. > > Data sheet is located > http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf > > Signed-off-by: Dan Murphy <dmurphy@ti.com> > --- > > v3 - Add missing Makefile and Kconfig from v1 and v2 - https://patchwork.kernel.org/patch/10060075/ > > v2 - Added data sheet link, fixed linuxdoc format, returned on failure in init > routine, return on fault_check failure, updated brightness calculation and > fixed capitalization issue - https://patchwork.kernel.org/patch/10056675/ > > drivers/leds/Kconfig | 7 + > drivers/leds/Makefile | 1 + > drivers/leds/leds-lm3692x.c | 390 ++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 398 insertions(+) > create mode 100644 drivers/leds/leds-lm3692x.c > > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > index 318a28fd58fe..1363e77a7d5f 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -137,6 +137,13 @@ config LEDS_LM3642 > converter plus 1.5A constant current driver for a high-current > white LED. > > +config LEDS_LM3692X > + tristate "LED support for LM3692x Chips" > + depends on LEDS_CLASS && I2C > + select REGMAP_I2C > + help > + This option enables support for the TI LM3692x family > + of LED drivers. > > config LEDS_LOCOMO > tristate "LED Support for Locomo device" > diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile > index a2a6b5a4f86d..987884a5b9a5 100644 > --- a/drivers/leds/Makefile > +++ b/drivers/leds/Makefile > @@ -74,6 +74,7 @@ obj-$(CONFIG_LEDS_PM8058) += leds-pm8058.o > obj-$(CONFIG_LEDS_MLXCPLD) += leds-mlxcpld.o > obj-$(CONFIG_LEDS_NIC78BX) += leds-nic78bx.o > obj-$(CONFIG_LEDS_MT6323) += leds-mt6323.o > +obj-$(CONFIG_LEDS_LM3692X) += leds-lm3692x.o > > # LED SPI Drivers > obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o > diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c > new file mode 100644 > index 000000000000..9fa7f5a20466 > --- /dev/null > +++ b/drivers/leds/leds-lm3692x.c > @@ -0,0 +1,390 @@ > +/* > + * TI lm3692x LED Driver > + * > + * Copyright (C) 2017 Texas Instruments > + * > + * Author: Dan Murphy <dmurphy@ti.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. > + * > + * Data sheet is located > + * http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf > + */ > + > +#include <linux/i2c.h> > +#include <linux/init.h> > +#include <linux/leds.h> > +#include <linux/regmap.h> > +#include <linux/regulator/consumer.h> > +#include <linux/module.h> > +#include <linux/mutex.h> > +#include <linux/of.h> > +#include <linux/of_gpio.h> > +#include <linux/gpio/consumer.h> > +#include <linux/slab.h> > + > +#define LM3692X_LED_NAME "lm3692x_led" > + > +#define LM3692X_REV 0x0 > +#define LM3692X_RESET 0x1 > +#define LM3692X_EN 0x10 > +#define LM3692X_BRT_CTRL 0x11 > +#define LM3692X_PWM_CTRL 0x12 > +#define LM3692X_BOOST_CTRL 0x13 > +#define LM3692X_AUTO_FREQ_HI 0x15 > +#define LM3692X_AUTO_FREQ_LO 0x16 > +#define LM3692X_BL_ADJ_THRESH 0x17 > +#define LM3692X_BRT_LSB 0x18 > +#define LM3692X_BRT_MSB 0x19 > +#define LM3692X_FAULT_CTRL 0x1e > +#define LM3692X_FAULT_FLAGS 0x1f > + > +#define LM3692X_SW_RESET BIT(0) > +#define LM3692X_DEVICE_EN BIT(0) > +#define LM3692X_LED1_EN BIT(1) > +#define LM3692X_LED2_EN BIT(2) > + > +/* Brightness Control Bits */ > +#define LM3692X_BL_ADJ_POL BIT(0) > +#define LM3692X_RAMP_RATE_125us 0x00 > +#define LM3692X_RAMP_RATE_250us BIT(1) > +#define LM3692X_RAMP_RATE_500us BIT(2) > +#define LM3692X_RAMP_RATE_1ms (BIT(1) | BIT(2)) > +#define LM3692X_RAMP_RATE_2ms BIT(3) > +#define LM3692X_RAMP_RATE_4ms (BIT(3) | BIT(1)) > +#define LM3692X_RAMP_RATE_8ms (BIT(2) | BIT(3)) > +#define LM3692X_RAMP_RATE_16ms (BIT(1) | BIT(2) | BIT(3)) > +#define LM3692X_RAMP_EN BIT(4) > +#define LM3692X_BRHT_MODE_REG 0x00 > +#define LM3692X_BRHT_MODE_PWM BIT(5) > +#define LM3692X_BRHT_MODE_MULTI_RAMP BIT(6) > +#define LM3692X_BRHT_MODE_RAMP_MULTI (BIT(5) | BIT(6)) > +#define LM3692X_MAP_MODE_EXP BIT(7) > + > +/* PWM Register Bits */ > +#define LM3692X_PWM_FILTER_100 BIT(0) > +#define LM3692X_PWM_FILTER_150 BIT(1) > +#define LM3692X_PWM_FILTER_200 (BIT(0) | BIT(1)) > +#define LM3692X_PWM_HYSTER_1LSB BIT(2) > +#define LM3692X_PWM_HYSTER_2LSB BIT(3) > +#define LM3692X_PWM_HYSTER_3LSB (BIT(3) | BIT(2)) > +#define LM3692X_PWM_HYSTER_4LSB BIT(4) > +#define LM3692X_PWM_HYSTER_5LSB (BIT(4) | BIT(2)) > +#define LM3692X_PWM_HYSTER_6LSB (BIT(4) | BIT(3)) > +#define LM3692X_PWM_POLARITY BIT(5) > +#define LM3692X_PWM_SAMP_4MHZ BIT(6) > +#define LM3692X_PWM_SAMP_24MHZ BIT(7) > + > +/* Boost Control Bits */ > +#define LM3692X_OCP_PROT_1A BIT(0) > +#define LM3692X_OCP_PROT_1_25A BIT(1) > +#define LM3692X_OCP_PROT_1_5A (BIT(0) | BIT(1)) > +#define LM3692X_OVP_21V BIT(2) > +#define LM3692X_OVP_25V BIT(3) > +#define LM3692X_OVP_29V (BIT(2) | BIT(3)) > +#define LM3692X_MIN_IND_22UH BIT(4) > +#define LM3692X_BOOST_SW_1MHZ BIT(5) > +#define LM3692X_BOOST_SW_NO_SHIFT BIT(6) > + > +/* Fault Control Bits */ > +#define LM3692X_FAULT_CTRL_OVP BIT(0) > +#define LM3692X_FAULT_CTRL_OCP BIT(1) > +#define LM3692X_FAULT_CTRL_TSD BIT(2) > +#define LM3692X_FAULT_CTRL_OPEN BIT(3) > + > +/* Fault Flag Bits */ > +#define LM3692X_FAULT_FLAG_OVP BIT(0) > +#define LM3692X_FAULT_FLAG_OCP BIT(1) > +#define LM3692X_FAULT_FLAG_TSD BIT(2) > +#define LM3692X_FAULT_FLAG_SHRT BIT(3) > +#define LM3692X_FAULT_FLAG_OPEN BIT(4) > + > +/** > + * struct lm3692x_led - > + * @lock - Lock for reading/writing the device > + * @client - Pointer to the I2C client > + * @led_dev - led class device pointer > + * @regmap - Devices register map > + * @enable_gpio - VDDIO/EN gpio to enable communication interface > + * @regulator - LED supply regulator pointer > + * @label - LED label > + */ > +struct lm3692x_led { > + struct mutex lock; > + struct i2c_client *client; > + struct led_classdev led_dev; > + struct regmap *regmap; > + struct gpio_desc *enable_gpio; > + struct regulator *regulator; > + const char *label; > +}; > + > +static const struct reg_default lm3692x_reg_defs[] = { > + {LM3692X_EN, 0xf}, > + {LM3692X_BRT_CTRL, 0x61}, > + {LM3692X_PWM_CTRL, 0x73}, > + {LM3692X_BOOST_CTRL, 0x6f}, > + {LM3692X_AUTO_FREQ_HI, 0x0}, > + {LM3692X_AUTO_FREQ_LO, 0x0}, > + {LM3692X_BL_ADJ_THRESH, 0x0}, > + {LM3692X_BRT_LSB, 0x7}, > + {LM3692X_BRT_MSB, 0xff}, > + {LM3692X_FAULT_CTRL, 0x7}, > +}; > + > +static const struct regmap_config lm3692x_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > + > + .max_register = LM3692X_FAULT_FLAGS, > + .reg_defaults = lm3692x_reg_defs, > + .num_reg_defaults = ARRAY_SIZE(lm3692x_reg_defs), > + .cache_type = REGCACHE_RBTREE, > +}; > + > +static int lm3692x_fault_check(struct lm3692x_led *led) > +{ > + int ret, fault; > + unsigned int read_buf; > + > + ret = regmap_read(led->regmap, LM3692X_FAULT_FLAGS, &read_buf); > + if (ret) > + return ret; > + > + fault = read_buf; > + > + if (fault) > + dev_err(&led->client->dev, "Detected a fault 0x%X\n", > + fault); > + > + return ret; > +} > + > +static int lm3692x_brightness_set(struct led_classdev *led_cdev, > + enum led_brightness brt_val) > +{ > + struct lm3692x_led *led = > + container_of(led_cdev, struct lm3692x_led, led_dev); > + int ret; > + int led_brightness_lsb = (brt_val >> 5); > + > + mutex_lock(&led->lock); > + > + ret = lm3692x_fault_check(led); > + if (ret) { > + dev_err(&led->client->dev, "Cannot read/clear faults\n"); > + goto out; > + } > + > + ret = regmap_write(led->regmap, LM3692X_BRT_MSB, brt_val); > + if (ret) { > + dev_err(&led->client->dev, "Cannot write MSB\n"); > + goto out; > + } > + > + ret = regmap_write(led->regmap, LM3692X_BRT_LSB, led_brightness_lsb); > + if (ret) { > + dev_err(&led->client->dev, "Cannot write LSB\n"); > + goto out; > + } > +out: > + mutex_unlock(&led->lock); > + return ret; > +} > + > +static int lm3692x_init(struct lm3692x_led *led) > +{ > + int ret; > + > + if (led->regulator) { > + ret = regulator_enable(led->regulator); > + if (ret) > + dev_err(&led->client->dev, > + "Failed to enable regulator\n"); > + goto out; > + } > + > + if (led->enable_gpio) > + gpiod_direction_output(led->enable_gpio, 1); > + > + ret = lm3692x_fault_check(led); > + if (ret) { > + dev_err(&led->client->dev, "Cannot read/clear faults\n"); > + goto out; > + } > + > + ret = regmap_write(led->regmap, LM3692X_BRT_CTRL, 0x00); > + if (ret) { > + dev_err(&led->client->dev, "Fail writing BRT CTRL\n"); > + goto out; > + } > + > + /* > + * For glitch free operation, the following data should > + * only be written while device enable bit is 0 > + * per Section 7.5.14 of the data sheet > + */ > + ret = regmap_write(led->regmap, LM3692X_PWM_CTRL, > + LM3692X_PWM_FILTER_100 | LM3692X_PWM_SAMP_24MHZ); > + if (ret) { > + dev_err(&led->client->dev, "Failed programming PWM CTRL\n"); > + goto out; > + } > + > + ret = regmap_write(led->regmap, LM3692X_BOOST_CTRL, > + LM3692X_BRHT_MODE_RAMP_MULTI | > + LM3692X_BL_ADJ_POL | > + LM3692X_RAMP_RATE_250us); > + if (ret) { > + dev_err(&led->client->dev, "Failed programming BOOST CTRL\n"); > + goto out; > + } > + > + ret = regmap_write(led->regmap, LM3692X_AUTO_FREQ_HI, 0x00); > + if (ret) { > + dev_err(&led->client->dev, "Failed programming AUTO HI FREQ\n"); > + goto out; > + } > + > + ret = regmap_write(led->regmap, LM3692X_AUTO_FREQ_LO, 0x00); > + if (ret) { > + dev_err(&led->client->dev, "Failed programming AUTO LO FREQ\n"); > + goto out; > + } > + > + ret = regmap_write(led->regmap, LM3692X_BL_ADJ_THRESH, 0x00); > + if (ret) { > + dev_err(&led->client->dev, "Failed programming BL Threshold\n"); > + goto out; > + } > + > + ret = regmap_write(led->regmap, LM3692X_BRT_CTRL, > + LM3692X_BL_ADJ_POL | LM3692X_PWM_HYSTER_4LSB); > + if (ret) > + dev_err(&led->client->dev, "Failed programming BRT CTRL\n"); > + > + return ret; > +out: > + > + 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"); > + } > + > + return ret; > +} > + > +static int lm3692x_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + int ret; > + struct lm3692x_led *led; > + struct device_node *np = client->dev.of_node; > + > + led = devm_kzalloc(&client->dev, sizeof(*led), GFP_KERNEL); > + if (!led) > + return -ENOMEM; > + > + if (client->dev.of_node) { We should fail in the opposite case. > + ret = of_property_read_string(np, "label", &led->label); > + if (ret) > + led->label = LM3692X_LED_NAME; LED label should be taken from the child node name if label property is not present. See Documentation/devicetree/bindings/leds/common.txt. We have some mess in this area, but I'm going to sort it out soon (added you on CC to the related discussion). Generally LED class device name should be composed by the LED class driver by taking label and prefixing it with "[devicename]:" string. > + } > + > + led->enable_gpio = devm_gpiod_get_optional(&client->dev, > + "enable", GPIOD_OUT_LOW); > + if (IS_ERR(led->enable_gpio)) { > + ret = PTR_ERR(led->enable_gpio); > + dev_err(&client->dev, "Failed to get enable gpio: %d\n", ret); > + return ret; > + } > + > + led->regulator = devm_regulator_get(&client->dev, "vled"); > + if (IS_ERR(led->regulator)) > + led->regulator = NULL; > + > + led->client = client; > + led->led_dev.name = led->label; > + led->led_dev.max_brightness = LED_FULL; You can skip this line, it will be initialized to LED_FULL in of_led_classdev_register in this case. > + led->led_dev.brightness_set_blocking = lm3692x_brightness_set; > + > + mutex_init(&led->lock); > + > + i2c_set_clientdata(client, led); > + > + led->regmap = devm_regmap_init_i2c(client, &lm3692x_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 = lm3692x_init(led); > + if (ret) > + return ret; > + > + ret = led_classdev_register(&client->dev, &led->led_dev); Please use devm_* prefixed version. > + if (ret) { > + dev_err(&client->dev, "led register err: %d\n", ret); > + return ret; > + } > + > + return 0; > +} > + > +static int lm3692x_remove(struct i2c_client *client) > +{ > + struct lm3692x_led *led = i2c_get_clientdata(client); > + int ret; > + > + led_classdev_unregister(&led->led_dev); > + > + 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 lm3692x_id[] = { > + { "lm3692x", 0 }, > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, lm3692x_id); > + > +#ifdef CONFIG_OF > +static const struct of_device_id of_lm3692x_leds_match[] = { > + { .compatible = "ti,lm3692x", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, of_lm3692x_leds_match); > +#endif > + > +static struct i2c_driver lm3692x_driver = { > + .driver = { > + .name = "lm3692x", > + .of_match_table = of_match_ptr(of_lm3692x_leds_match), > + }, > + .probe = lm3692x_probe, > + .remove = lm3692x_remove, > + .id_table = lm3692x_id, > +}; > +module_i2c_driver(lm3692x_driver); > + > +MODULE_DESCRIPTION("Texas Instruments LM3692X LED driver"); > +MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com>"); > +MODULE_LICENSE("GPL"); According to your license info it should be "GPL v2" -- Best regards, Jacek Anaszewski ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 2/2] leds: lm3692x: Introduce LM3692x dual string driver 2017-11-18 14:19 ` Jacek Anaszewski @ 2017-11-28 12:53 ` Dan Murphy -1 siblings, 0 replies; 42+ messages in thread From: Dan Murphy @ 2017-11-28 12:53 UTC (permalink / raw) To: Jacek Anaszewski, robh+dt, mark.rutland, rpurdie, pavel Cc: devicetree, linux-kernel, linux-leds Jacek Thanks On 11/18/2017 08:19 AM, Jacek Anaszewski wrote: > Hi Dan, > > Please find my comments below. > > On 11/15/2017 08:42 PM, Dan Murphy wrote: >> Introducing the LM3692x Dual-String white LED driver. >> >> Data sheet is located >> http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf >> >> Signed-off-by: Dan Murphy <dmurphy@ti.com> >> --- >> >> v3 - Add missing Makefile and Kconfig from v1 and v2 - https://patchwork.kernel.org/patch/10060075/ >> >> v2 - Added data sheet link, fixed linuxdoc format, returned on failure in init >> routine, return on fault_check failure, updated brightness calculation and >> fixed capitalization issue - https://patchwork.kernel.org/patch/10056675/ >> >> drivers/leds/Kconfig | 7 + >> drivers/leds/Makefile | 1 + >> drivers/leds/leds-lm3692x.c | 390 ++++++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 398 insertions(+) >> create mode 100644 drivers/leds/leds-lm3692x.c >> >> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig >> index 318a28fd58fe..1363e77a7d5f 100644 >> --- a/drivers/leds/Kconfig >> +++ b/drivers/leds/Kconfig >> @@ -137,6 +137,13 @@ config LEDS_LM3642 >> converter plus 1.5A constant current driver for a high-current >> white LED. >> >> +config LEDS_LM3692X >> + tristate "LED support for LM3692x Chips" >> + depends on LEDS_CLASS && I2C >> + select REGMAP_I2C >> + help >> + This option enables support for the TI LM3692x family >> + of LED drivers. >> >> config LEDS_LOCOMO >> tristate "LED Support for Locomo device" >> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile >> index a2a6b5a4f86d..987884a5b9a5 100644 >> --- a/drivers/leds/Makefile >> +++ b/drivers/leds/Makefile >> @@ -74,6 +74,7 @@ obj-$(CONFIG_LEDS_PM8058) += leds-pm8058.o >> obj-$(CONFIG_LEDS_MLXCPLD) += leds-mlxcpld.o >> obj-$(CONFIG_LEDS_NIC78BX) += leds-nic78bx.o >> obj-$(CONFIG_LEDS_MT6323) += leds-mt6323.o >> +obj-$(CONFIG_LEDS_LM3692X) += leds-lm3692x.o >> >> # LED SPI Drivers >> obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o >> diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c >> new file mode 100644 >> index 000000000000..9fa7f5a20466 >> --- /dev/null >> +++ b/drivers/leds/leds-lm3692x.c >> @@ -0,0 +1,390 @@ >> +/* >> + * TI lm3692x LED Driver >> + * >> + * Copyright (C) 2017 Texas Instruments >> + * >> + * Author: Dan Murphy <dmurphy@ti.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. >> + * >> + * Data sheet is located >> + * http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf >> + */ >> + >> +#include <linux/i2c.h> >> +#include <linux/init.h> >> +#include <linux/leds.h> >> +#include <linux/regmap.h> >> +#include <linux/regulator/consumer.h> >> +#include <linux/module.h> >> +#include <linux/mutex.h> >> +#include <linux/of.h> >> +#include <linux/of_gpio.h> >> +#include <linux/gpio/consumer.h> >> +#include <linux/slab.h> >> + >> +#define LM3692X_LED_NAME "lm3692x_led" >> + >> +#define LM3692X_REV 0x0 >> +#define LM3692X_RESET 0x1 >> +#define LM3692X_EN 0x10 >> +#define LM3692X_BRT_CTRL 0x11 >> +#define LM3692X_PWM_CTRL 0x12 >> +#define LM3692X_BOOST_CTRL 0x13 >> +#define LM3692X_AUTO_FREQ_HI 0x15 >> +#define LM3692X_AUTO_FREQ_LO 0x16 >> +#define LM3692X_BL_ADJ_THRESH 0x17 >> +#define LM3692X_BRT_LSB 0x18 >> +#define LM3692X_BRT_MSB 0x19 >> +#define LM3692X_FAULT_CTRL 0x1e >> +#define LM3692X_FAULT_FLAGS 0x1f >> + >> +#define LM3692X_SW_RESET BIT(0) >> +#define LM3692X_DEVICE_EN BIT(0) >> +#define LM3692X_LED1_EN BIT(1) >> +#define LM3692X_LED2_EN BIT(2) >> + >> +/* Brightness Control Bits */ >> +#define LM3692X_BL_ADJ_POL BIT(0) >> +#define LM3692X_RAMP_RATE_125us 0x00 >> +#define LM3692X_RAMP_RATE_250us BIT(1) >> +#define LM3692X_RAMP_RATE_500us BIT(2) >> +#define LM3692X_RAMP_RATE_1ms (BIT(1) | BIT(2)) >> +#define LM3692X_RAMP_RATE_2ms BIT(3) >> +#define LM3692X_RAMP_RATE_4ms (BIT(3) | BIT(1)) >> +#define LM3692X_RAMP_RATE_8ms (BIT(2) | BIT(3)) >> +#define LM3692X_RAMP_RATE_16ms (BIT(1) | BIT(2) | BIT(3)) >> +#define LM3692X_RAMP_EN BIT(4) >> +#define LM3692X_BRHT_MODE_REG 0x00 >> +#define LM3692X_BRHT_MODE_PWM BIT(5) >> +#define LM3692X_BRHT_MODE_MULTI_RAMP BIT(6) >> +#define LM3692X_BRHT_MODE_RAMP_MULTI (BIT(5) | BIT(6)) >> +#define LM3692X_MAP_MODE_EXP BIT(7) >> + >> +/* PWM Register Bits */ >> +#define LM3692X_PWM_FILTER_100 BIT(0) >> +#define LM3692X_PWM_FILTER_150 BIT(1) >> +#define LM3692X_PWM_FILTER_200 (BIT(0) | BIT(1)) >> +#define LM3692X_PWM_HYSTER_1LSB BIT(2) >> +#define LM3692X_PWM_HYSTER_2LSB BIT(3) >> +#define LM3692X_PWM_HYSTER_3LSB (BIT(3) | BIT(2)) >> +#define LM3692X_PWM_HYSTER_4LSB BIT(4) >> +#define LM3692X_PWM_HYSTER_5LSB (BIT(4) | BIT(2)) >> +#define LM3692X_PWM_HYSTER_6LSB (BIT(4) | BIT(3)) >> +#define LM3692X_PWM_POLARITY BIT(5) >> +#define LM3692X_PWM_SAMP_4MHZ BIT(6) >> +#define LM3692X_PWM_SAMP_24MHZ BIT(7) >> + >> +/* Boost Control Bits */ >> +#define LM3692X_OCP_PROT_1A BIT(0) >> +#define LM3692X_OCP_PROT_1_25A BIT(1) >> +#define LM3692X_OCP_PROT_1_5A (BIT(0) | BIT(1)) >> +#define LM3692X_OVP_21V BIT(2) >> +#define LM3692X_OVP_25V BIT(3) >> +#define LM3692X_OVP_29V (BIT(2) | BIT(3)) >> +#define LM3692X_MIN_IND_22UH BIT(4) >> +#define LM3692X_BOOST_SW_1MHZ BIT(5) >> +#define LM3692X_BOOST_SW_NO_SHIFT BIT(6) >> + >> +/* Fault Control Bits */ >> +#define LM3692X_FAULT_CTRL_OVP BIT(0) >> +#define LM3692X_FAULT_CTRL_OCP BIT(1) >> +#define LM3692X_FAULT_CTRL_TSD BIT(2) >> +#define LM3692X_FAULT_CTRL_OPEN BIT(3) >> + >> +/* Fault Flag Bits */ >> +#define LM3692X_FAULT_FLAG_OVP BIT(0) >> +#define LM3692X_FAULT_FLAG_OCP BIT(1) >> +#define LM3692X_FAULT_FLAG_TSD BIT(2) >> +#define LM3692X_FAULT_FLAG_SHRT BIT(3) >> +#define LM3692X_FAULT_FLAG_OPEN BIT(4) >> + >> +/** >> + * struct lm3692x_led - >> + * @lock - Lock for reading/writing the device >> + * @client - Pointer to the I2C client >> + * @led_dev - led class device pointer >> + * @regmap - Devices register map >> + * @enable_gpio - VDDIO/EN gpio to enable communication interface >> + * @regulator - LED supply regulator pointer >> + * @label - LED label >> + */ >> +struct lm3692x_led { >> + struct mutex lock; >> + struct i2c_client *client; >> + struct led_classdev led_dev; >> + struct regmap *regmap; >> + struct gpio_desc *enable_gpio; >> + struct regulator *regulator; >> + const char *label; >> +}; >> + >> +static const struct reg_default lm3692x_reg_defs[] = { >> + {LM3692X_EN, 0xf}, >> + {LM3692X_BRT_CTRL, 0x61}, >> + {LM3692X_PWM_CTRL, 0x73}, >> + {LM3692X_BOOST_CTRL, 0x6f}, >> + {LM3692X_AUTO_FREQ_HI, 0x0}, >> + {LM3692X_AUTO_FREQ_LO, 0x0}, >> + {LM3692X_BL_ADJ_THRESH, 0x0}, >> + {LM3692X_BRT_LSB, 0x7}, >> + {LM3692X_BRT_MSB, 0xff}, >> + {LM3692X_FAULT_CTRL, 0x7}, >> +}; >> + >> +static const struct regmap_config lm3692x_regmap_config = { >> + .reg_bits = 8, >> + .val_bits = 8, >> + >> + .max_register = LM3692X_FAULT_FLAGS, >> + .reg_defaults = lm3692x_reg_defs, >> + .num_reg_defaults = ARRAY_SIZE(lm3692x_reg_defs), >> + .cache_type = REGCACHE_RBTREE, >> +}; >> + >> +static int lm3692x_fault_check(struct lm3692x_led *led) >> +{ >> + int ret, fault; >> + unsigned int read_buf; >> + >> + ret = regmap_read(led->regmap, LM3692X_FAULT_FLAGS, &read_buf); >> + if (ret) >> + return ret; >> + >> + fault = read_buf; >> + >> + if (fault) >> + dev_err(&led->client->dev, "Detected a fault 0x%X\n", >> + fault); >> + >> + return ret; >> +} >> + >> +static int lm3692x_brightness_set(struct led_classdev *led_cdev, >> + enum led_brightness brt_val) >> +{ >> + struct lm3692x_led *led = >> + container_of(led_cdev, struct lm3692x_led, led_dev); >> + int ret; >> + int led_brightness_lsb = (brt_val >> 5); >> + >> + mutex_lock(&led->lock); >> + >> + ret = lm3692x_fault_check(led); >> + if (ret) { >> + dev_err(&led->client->dev, "Cannot read/clear faults\n"); >> + goto out; >> + } >> + >> + ret = regmap_write(led->regmap, LM3692X_BRT_MSB, brt_val); >> + if (ret) { >> + dev_err(&led->client->dev, "Cannot write MSB\n"); >> + goto out; >> + } >> + >> + ret = regmap_write(led->regmap, LM3692X_BRT_LSB, led_brightness_lsb); >> + if (ret) { >> + dev_err(&led->client->dev, "Cannot write LSB\n"); >> + goto out; >> + } >> +out: >> + mutex_unlock(&led->lock); >> + return ret; >> +} >> + >> +static int lm3692x_init(struct lm3692x_led *led) >> +{ >> + int ret; >> + >> + if (led->regulator) { >> + ret = regulator_enable(led->regulator); >> + if (ret) >> + dev_err(&led->client->dev, >> + "Failed to enable regulator\n"); >> + goto out; >> + } >> + >> + if (led->enable_gpio) >> + gpiod_direction_output(led->enable_gpio, 1); >> + >> + ret = lm3692x_fault_check(led); >> + if (ret) { >> + dev_err(&led->client->dev, "Cannot read/clear faults\n"); >> + goto out; >> + } >> + >> + ret = regmap_write(led->regmap, LM3692X_BRT_CTRL, 0x00); >> + if (ret) { >> + dev_err(&led->client->dev, "Fail writing BRT CTRL\n"); >> + goto out; >> + } >> + >> + /* >> + * For glitch free operation, the following data should >> + * only be written while device enable bit is 0 >> + * per Section 7.5.14 of the data sheet >> + */ >> + ret = regmap_write(led->regmap, LM3692X_PWM_CTRL, >> + LM3692X_PWM_FILTER_100 | LM3692X_PWM_SAMP_24MHZ); >> + if (ret) { >> + dev_err(&led->client->dev, "Failed programming PWM CTRL\n"); >> + goto out; >> + } >> + >> + ret = regmap_write(led->regmap, LM3692X_BOOST_CTRL, >> + LM3692X_BRHT_MODE_RAMP_MULTI | >> + LM3692X_BL_ADJ_POL | >> + LM3692X_RAMP_RATE_250us); >> + if (ret) { >> + dev_err(&led->client->dev, "Failed programming BOOST CTRL\n"); >> + goto out; >> + } >> + >> + ret = regmap_write(led->regmap, LM3692X_AUTO_FREQ_HI, 0x00); >> + if (ret) { >> + dev_err(&led->client->dev, "Failed programming AUTO HI FREQ\n"); >> + goto out; >> + } >> + >> + ret = regmap_write(led->regmap, LM3692X_AUTO_FREQ_LO, 0x00); >> + if (ret) { >> + dev_err(&led->client->dev, "Failed programming AUTO LO FREQ\n"); >> + goto out; >> + } >> + >> + ret = regmap_write(led->regmap, LM3692X_BL_ADJ_THRESH, 0x00); >> + if (ret) { >> + dev_err(&led->client->dev, "Failed programming BL Threshold\n"); >> + goto out; >> + } >> + >> + ret = regmap_write(led->regmap, LM3692X_BRT_CTRL, >> + LM3692X_BL_ADJ_POL | LM3692X_PWM_HYSTER_4LSB); >> + if (ret) >> + dev_err(&led->client->dev, "Failed programming BRT CTRL\n"); >> + >> + return ret; >> +out: >> + >> + 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"); >> + } >> + >> + return ret; >> +} >> + >> +static int lm3692x_probe(struct i2c_client *client, >> + const struct i2c_device_id *id) >> +{ >> + int ret; >> + struct lm3692x_led *led; >> + struct device_node *np = client->dev.of_node; >> + >> + led = devm_kzalloc(&client->dev, sizeof(*led), GFP_KERNEL); >> + if (!led) >> + return -ENOMEM; >> + >> + if (client->dev.of_node) { > > We should fail in the opposite case. OK > >> + ret = of_property_read_string(np, "label", &led->label); >> + if (ret) >> + led->label = LM3692X_LED_NAME; > > LED label should be taken from the child node name if label property is > not present. See Documentation/devicetree/bindings/leds/common.txt. > > We have some mess in this area, but I'm going to sort it out soon > (added you on CC to the related discussion). > > Generally LED class device name should be composed by the LED class > driver by taking label and prefixing it with "[devicename]:" string. OK > >> + } >> + >> + led->enable_gpio = devm_gpiod_get_optional(&client->dev, >> + "enable", GPIOD_OUT_LOW); >> + if (IS_ERR(led->enable_gpio)) { >> + ret = PTR_ERR(led->enable_gpio); >> + dev_err(&client->dev, "Failed to get enable gpio: %d\n", ret); >> + return ret; >> + } >> + >> + led->regulator = devm_regulator_get(&client->dev, "vled"); >> + if (IS_ERR(led->regulator)) >> + led->regulator = NULL; >> + >> + led->client = client; >> + led->led_dev.name = led->label; >> + led->led_dev.max_brightness = LED_FULL; > > You can skip this line, it will be initialized to LED_FULL > in of_led_classdev_register in this case. OK > > >> + led->led_dev.brightness_set_blocking = lm3692x_brightness_set; >> + >> + mutex_init(&led->lock); >> + >> + i2c_set_clientdata(client, led); >> + >> + led->regmap = devm_regmap_init_i2c(client, &lm3692x_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 = lm3692x_init(led); >> + if (ret) >> + return ret; >> + >> + ret = led_classdev_register(&client->dev, &led->led_dev); > > Please use devm_* prefixed version. OK > >> + if (ret) { >> + dev_err(&client->dev, "led register err: %d\n", ret); >> + return ret; >> + } >> + >> + return 0; >> +} >> + >> +static int lm3692x_remove(struct i2c_client *client) >> +{ >> + struct lm3692x_led *led = i2c_get_clientdata(client); >> + int ret; >> + >> + led_classdev_unregister(&led->led_dev); >> + >> + 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) OK > >> + >> + return 0; >> +} >> + >> +static const struct i2c_device_id lm3692x_id[] = { >> + { "lm3692x", 0 }, >> + { } >> +}; >> +MODULE_DEVICE_TABLE(i2c, lm3692x_id); >> + >> +#ifdef CONFIG_OF >> +static const struct of_device_id of_lm3692x_leds_match[] = { >> + { .compatible = "ti,lm3692x", }, >> + {}, >> +}; >> +MODULE_DEVICE_TABLE(of, of_lm3692x_leds_match); >> +#endif >> + >> +static struct i2c_driver lm3692x_driver = { >> + .driver = { >> + .name = "lm3692x", >> + .of_match_table = of_match_ptr(of_lm3692x_leds_match), >> + }, >> + .probe = lm3692x_probe, >> + .remove = lm3692x_remove, >> + .id_table = lm3692x_id, >> +}; >> +module_i2c_driver(lm3692x_driver); >> + >> +MODULE_DESCRIPTION("Texas Instruments LM3692X LED driver"); >> +MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com>"); >> +MODULE_LICENSE("GPL"); > > According to your license info it should be "GPL v2" > OK Dan -- ------------------ Dan Murphy ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 2/2] leds: lm3692x: Introduce LM3692x dual string driver @ 2017-11-28 12:53 ` Dan Murphy 0 siblings, 0 replies; 42+ messages in thread From: Dan Murphy @ 2017-11-28 12:53 UTC (permalink / raw) To: Jacek Anaszewski, robh+dt, mark.rutland, rpurdie, pavel Cc: devicetree, linux-kernel, linux-leds Jacek Thanks On 11/18/2017 08:19 AM, Jacek Anaszewski wrote: > Hi Dan, > > Please find my comments below. > > On 11/15/2017 08:42 PM, Dan Murphy wrote: >> Introducing the LM3692x Dual-String white LED driver. >> >> Data sheet is located >> http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf >> >> Signed-off-by: Dan Murphy <dmurphy@ti.com> >> --- >> >> v3 - Add missing Makefile and Kconfig from v1 and v2 - https://patchwork.kernel.org/patch/10060075/ >> >> v2 - Added data sheet link, fixed linuxdoc format, returned on failure in init >> routine, return on fault_check failure, updated brightness calculation and >> fixed capitalization issue - https://patchwork.kernel.org/patch/10056675/ >> >> drivers/leds/Kconfig | 7 + >> drivers/leds/Makefile | 1 + >> drivers/leds/leds-lm3692x.c | 390 ++++++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 398 insertions(+) >> create mode 100644 drivers/leds/leds-lm3692x.c >> >> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig >> index 318a28fd58fe..1363e77a7d5f 100644 >> --- a/drivers/leds/Kconfig >> +++ b/drivers/leds/Kconfig >> @@ -137,6 +137,13 @@ config LEDS_LM3642 >> converter plus 1.5A constant current driver for a high-current >> white LED. >> >> +config LEDS_LM3692X >> + tristate "LED support for LM3692x Chips" >> + depends on LEDS_CLASS && I2C >> + select REGMAP_I2C >> + help >> + This option enables support for the TI LM3692x family >> + of LED drivers. >> >> config LEDS_LOCOMO >> tristate "LED Support for Locomo device" >> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile >> index a2a6b5a4f86d..987884a5b9a5 100644 >> --- a/drivers/leds/Makefile >> +++ b/drivers/leds/Makefile >> @@ -74,6 +74,7 @@ obj-$(CONFIG_LEDS_PM8058) += leds-pm8058.o >> obj-$(CONFIG_LEDS_MLXCPLD) += leds-mlxcpld.o >> obj-$(CONFIG_LEDS_NIC78BX) += leds-nic78bx.o >> obj-$(CONFIG_LEDS_MT6323) += leds-mt6323.o >> +obj-$(CONFIG_LEDS_LM3692X) += leds-lm3692x.o >> >> # LED SPI Drivers >> obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o >> diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c >> new file mode 100644 >> index 000000000000..9fa7f5a20466 >> --- /dev/null >> +++ b/drivers/leds/leds-lm3692x.c >> @@ -0,0 +1,390 @@ >> +/* >> + * TI lm3692x LED Driver >> + * >> + * Copyright (C) 2017 Texas Instruments >> + * >> + * Author: Dan Murphy <dmurphy@ti.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. >> + * >> + * Data sheet is located >> + * http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf >> + */ >> + >> +#include <linux/i2c.h> >> +#include <linux/init.h> >> +#include <linux/leds.h> >> +#include <linux/regmap.h> >> +#include <linux/regulator/consumer.h> >> +#include <linux/module.h> >> +#include <linux/mutex.h> >> +#include <linux/of.h> >> +#include <linux/of_gpio.h> >> +#include <linux/gpio/consumer.h> >> +#include <linux/slab.h> >> + >> +#define LM3692X_LED_NAME "lm3692x_led" >> + >> +#define LM3692X_REV 0x0 >> +#define LM3692X_RESET 0x1 >> +#define LM3692X_EN 0x10 >> +#define LM3692X_BRT_CTRL 0x11 >> +#define LM3692X_PWM_CTRL 0x12 >> +#define LM3692X_BOOST_CTRL 0x13 >> +#define LM3692X_AUTO_FREQ_HI 0x15 >> +#define LM3692X_AUTO_FREQ_LO 0x16 >> +#define LM3692X_BL_ADJ_THRESH 0x17 >> +#define LM3692X_BRT_LSB 0x18 >> +#define LM3692X_BRT_MSB 0x19 >> +#define LM3692X_FAULT_CTRL 0x1e >> +#define LM3692X_FAULT_FLAGS 0x1f >> + >> +#define LM3692X_SW_RESET BIT(0) >> +#define LM3692X_DEVICE_EN BIT(0) >> +#define LM3692X_LED1_EN BIT(1) >> +#define LM3692X_LED2_EN BIT(2) >> + >> +/* Brightness Control Bits */ >> +#define LM3692X_BL_ADJ_POL BIT(0) >> +#define LM3692X_RAMP_RATE_125us 0x00 >> +#define LM3692X_RAMP_RATE_250us BIT(1) >> +#define LM3692X_RAMP_RATE_500us BIT(2) >> +#define LM3692X_RAMP_RATE_1ms (BIT(1) | BIT(2)) >> +#define LM3692X_RAMP_RATE_2ms BIT(3) >> +#define LM3692X_RAMP_RATE_4ms (BIT(3) | BIT(1)) >> +#define LM3692X_RAMP_RATE_8ms (BIT(2) | BIT(3)) >> +#define LM3692X_RAMP_RATE_16ms (BIT(1) | BIT(2) | BIT(3)) >> +#define LM3692X_RAMP_EN BIT(4) >> +#define LM3692X_BRHT_MODE_REG 0x00 >> +#define LM3692X_BRHT_MODE_PWM BIT(5) >> +#define LM3692X_BRHT_MODE_MULTI_RAMP BIT(6) >> +#define LM3692X_BRHT_MODE_RAMP_MULTI (BIT(5) | BIT(6)) >> +#define LM3692X_MAP_MODE_EXP BIT(7) >> + >> +/* PWM Register Bits */ >> +#define LM3692X_PWM_FILTER_100 BIT(0) >> +#define LM3692X_PWM_FILTER_150 BIT(1) >> +#define LM3692X_PWM_FILTER_200 (BIT(0) | BIT(1)) >> +#define LM3692X_PWM_HYSTER_1LSB BIT(2) >> +#define LM3692X_PWM_HYSTER_2LSB BIT(3) >> +#define LM3692X_PWM_HYSTER_3LSB (BIT(3) | BIT(2)) >> +#define LM3692X_PWM_HYSTER_4LSB BIT(4) >> +#define LM3692X_PWM_HYSTER_5LSB (BIT(4) | BIT(2)) >> +#define LM3692X_PWM_HYSTER_6LSB (BIT(4) | BIT(3)) >> +#define LM3692X_PWM_POLARITY BIT(5) >> +#define LM3692X_PWM_SAMP_4MHZ BIT(6) >> +#define LM3692X_PWM_SAMP_24MHZ BIT(7) >> + >> +/* Boost Control Bits */ >> +#define LM3692X_OCP_PROT_1A BIT(0) >> +#define LM3692X_OCP_PROT_1_25A BIT(1) >> +#define LM3692X_OCP_PROT_1_5A (BIT(0) | BIT(1)) >> +#define LM3692X_OVP_21V BIT(2) >> +#define LM3692X_OVP_25V BIT(3) >> +#define LM3692X_OVP_29V (BIT(2) | BIT(3)) >> +#define LM3692X_MIN_IND_22UH BIT(4) >> +#define LM3692X_BOOST_SW_1MHZ BIT(5) >> +#define LM3692X_BOOST_SW_NO_SHIFT BIT(6) >> + >> +/* Fault Control Bits */ >> +#define LM3692X_FAULT_CTRL_OVP BIT(0) >> +#define LM3692X_FAULT_CTRL_OCP BIT(1) >> +#define LM3692X_FAULT_CTRL_TSD BIT(2) >> +#define LM3692X_FAULT_CTRL_OPEN BIT(3) >> + >> +/* Fault Flag Bits */ >> +#define LM3692X_FAULT_FLAG_OVP BIT(0) >> +#define LM3692X_FAULT_FLAG_OCP BIT(1) >> +#define LM3692X_FAULT_FLAG_TSD BIT(2) >> +#define LM3692X_FAULT_FLAG_SHRT BIT(3) >> +#define LM3692X_FAULT_FLAG_OPEN BIT(4) >> + >> +/** >> + * struct lm3692x_led - >> + * @lock - Lock for reading/writing the device >> + * @client - Pointer to the I2C client >> + * @led_dev - led class device pointer >> + * @regmap - Devices register map >> + * @enable_gpio - VDDIO/EN gpio to enable communication interface >> + * @regulator - LED supply regulator pointer >> + * @label - LED label >> + */ >> +struct lm3692x_led { >> + struct mutex lock; >> + struct i2c_client *client; >> + struct led_classdev led_dev; >> + struct regmap *regmap; >> + struct gpio_desc *enable_gpio; >> + struct regulator *regulator; >> + const char *label; >> +}; >> + >> +static const struct reg_default lm3692x_reg_defs[] = { >> + {LM3692X_EN, 0xf}, >> + {LM3692X_BRT_CTRL, 0x61}, >> + {LM3692X_PWM_CTRL, 0x73}, >> + {LM3692X_BOOST_CTRL, 0x6f}, >> + {LM3692X_AUTO_FREQ_HI, 0x0}, >> + {LM3692X_AUTO_FREQ_LO, 0x0}, >> + {LM3692X_BL_ADJ_THRESH, 0x0}, >> + {LM3692X_BRT_LSB, 0x7}, >> + {LM3692X_BRT_MSB, 0xff}, >> + {LM3692X_FAULT_CTRL, 0x7}, >> +}; >> + >> +static const struct regmap_config lm3692x_regmap_config = { >> + .reg_bits = 8, >> + .val_bits = 8, >> + >> + .max_register = LM3692X_FAULT_FLAGS, >> + .reg_defaults = lm3692x_reg_defs, >> + .num_reg_defaults = ARRAY_SIZE(lm3692x_reg_defs), >> + .cache_type = REGCACHE_RBTREE, >> +}; >> + >> +static int lm3692x_fault_check(struct lm3692x_led *led) >> +{ >> + int ret, fault; >> + unsigned int read_buf; >> + >> + ret = regmap_read(led->regmap, LM3692X_FAULT_FLAGS, &read_buf); >> + if (ret) >> + return ret; >> + >> + fault = read_buf; >> + >> + if (fault) >> + dev_err(&led->client->dev, "Detected a fault 0x%X\n", >> + fault); >> + >> + return ret; >> +} >> + >> +static int lm3692x_brightness_set(struct led_classdev *led_cdev, >> + enum led_brightness brt_val) >> +{ >> + struct lm3692x_led *led = >> + container_of(led_cdev, struct lm3692x_led, led_dev); >> + int ret; >> + int led_brightness_lsb = (brt_val >> 5); >> + >> + mutex_lock(&led->lock); >> + >> + ret = lm3692x_fault_check(led); >> + if (ret) { >> + dev_err(&led->client->dev, "Cannot read/clear faults\n"); >> + goto out; >> + } >> + >> + ret = regmap_write(led->regmap, LM3692X_BRT_MSB, brt_val); >> + if (ret) { >> + dev_err(&led->client->dev, "Cannot write MSB\n"); >> + goto out; >> + } >> + >> + ret = regmap_write(led->regmap, LM3692X_BRT_LSB, led_brightness_lsb); >> + if (ret) { >> + dev_err(&led->client->dev, "Cannot write LSB\n"); >> + goto out; >> + } >> +out: >> + mutex_unlock(&led->lock); >> + return ret; >> +} >> + >> +static int lm3692x_init(struct lm3692x_led *led) >> +{ >> + int ret; >> + >> + if (led->regulator) { >> + ret = regulator_enable(led->regulator); >> + if (ret) >> + dev_err(&led->client->dev, >> + "Failed to enable regulator\n"); >> + goto out; >> + } >> + >> + if (led->enable_gpio) >> + gpiod_direction_output(led->enable_gpio, 1); >> + >> + ret = lm3692x_fault_check(led); >> + if (ret) { >> + dev_err(&led->client->dev, "Cannot read/clear faults\n"); >> + goto out; >> + } >> + >> + ret = regmap_write(led->regmap, LM3692X_BRT_CTRL, 0x00); >> + if (ret) { >> + dev_err(&led->client->dev, "Fail writing BRT CTRL\n"); >> + goto out; >> + } >> + >> + /* >> + * For glitch free operation, the following data should >> + * only be written while device enable bit is 0 >> + * per Section 7.5.14 of the data sheet >> + */ >> + ret = regmap_write(led->regmap, LM3692X_PWM_CTRL, >> + LM3692X_PWM_FILTER_100 | LM3692X_PWM_SAMP_24MHZ); >> + if (ret) { >> + dev_err(&led->client->dev, "Failed programming PWM CTRL\n"); >> + goto out; >> + } >> + >> + ret = regmap_write(led->regmap, LM3692X_BOOST_CTRL, >> + LM3692X_BRHT_MODE_RAMP_MULTI | >> + LM3692X_BL_ADJ_POL | >> + LM3692X_RAMP_RATE_250us); >> + if (ret) { >> + dev_err(&led->client->dev, "Failed programming BOOST CTRL\n"); >> + goto out; >> + } >> + >> + ret = regmap_write(led->regmap, LM3692X_AUTO_FREQ_HI, 0x00); >> + if (ret) { >> + dev_err(&led->client->dev, "Failed programming AUTO HI FREQ\n"); >> + goto out; >> + } >> + >> + ret = regmap_write(led->regmap, LM3692X_AUTO_FREQ_LO, 0x00); >> + if (ret) { >> + dev_err(&led->client->dev, "Failed programming AUTO LO FREQ\n"); >> + goto out; >> + } >> + >> + ret = regmap_write(led->regmap, LM3692X_BL_ADJ_THRESH, 0x00); >> + if (ret) { >> + dev_err(&led->client->dev, "Failed programming BL Threshold\n"); >> + goto out; >> + } >> + >> + ret = regmap_write(led->regmap, LM3692X_BRT_CTRL, >> + LM3692X_BL_ADJ_POL | LM3692X_PWM_HYSTER_4LSB); >> + if (ret) >> + dev_err(&led->client->dev, "Failed programming BRT CTRL\n"); >> + >> + return ret; >> +out: >> + >> + 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"); >> + } >> + >> + return ret; >> +} >> + >> +static int lm3692x_probe(struct i2c_client *client, >> + const struct i2c_device_id *id) >> +{ >> + int ret; >> + struct lm3692x_led *led; >> + struct device_node *np = client->dev.of_node; >> + >> + led = devm_kzalloc(&client->dev, sizeof(*led), GFP_KERNEL); >> + if (!led) >> + return -ENOMEM; >> + >> + if (client->dev.of_node) { > > We should fail in the opposite case. OK > >> + ret = of_property_read_string(np, "label", &led->label); >> + if (ret) >> + led->label = LM3692X_LED_NAME; > > LED label should be taken from the child node name if label property is > not present. See Documentation/devicetree/bindings/leds/common.txt. > > We have some mess in this area, but I'm going to sort it out soon > (added you on CC to the related discussion). > > Generally LED class device name should be composed by the LED class > driver by taking label and prefixing it with "[devicename]:" string. OK > >> + } >> + >> + led->enable_gpio = devm_gpiod_get_optional(&client->dev, >> + "enable", GPIOD_OUT_LOW); >> + if (IS_ERR(led->enable_gpio)) { >> + ret = PTR_ERR(led->enable_gpio); >> + dev_err(&client->dev, "Failed to get enable gpio: %d\n", ret); >> + return ret; >> + } >> + >> + led->regulator = devm_regulator_get(&client->dev, "vled"); >> + if (IS_ERR(led->regulator)) >> + led->regulator = NULL; >> + >> + led->client = client; >> + led->led_dev.name = led->label; >> + led->led_dev.max_brightness = LED_FULL; > > You can skip this line, it will be initialized to LED_FULL > in of_led_classdev_register in this case. OK > > >> + led->led_dev.brightness_set_blocking = lm3692x_brightness_set; >> + >> + mutex_init(&led->lock); >> + >> + i2c_set_clientdata(client, led); >> + >> + led->regmap = devm_regmap_init_i2c(client, &lm3692x_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 = lm3692x_init(led); >> + if (ret) >> + return ret; >> + >> + ret = led_classdev_register(&client->dev, &led->led_dev); > > Please use devm_* prefixed version. OK > >> + if (ret) { >> + dev_err(&client->dev, "led register err: %d\n", ret); >> + return ret; >> + } >> + >> + return 0; >> +} >> + >> +static int lm3692x_remove(struct i2c_client *client) >> +{ >> + struct lm3692x_led *led = i2c_get_clientdata(client); >> + int ret; >> + >> + led_classdev_unregister(&led->led_dev); >> + >> + 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) OK > >> + >> + return 0; >> +} >> + >> +static const struct i2c_device_id lm3692x_id[] = { >> + { "lm3692x", 0 }, >> + { } >> +}; >> +MODULE_DEVICE_TABLE(i2c, lm3692x_id); >> + >> +#ifdef CONFIG_OF >> +static const struct of_device_id of_lm3692x_leds_match[] = { >> + { .compatible = "ti,lm3692x", }, >> + {}, >> +}; >> +MODULE_DEVICE_TABLE(of, of_lm3692x_leds_match); >> +#endif >> + >> +static struct i2c_driver lm3692x_driver = { >> + .driver = { >> + .name = "lm3692x", >> + .of_match_table = of_match_ptr(of_lm3692x_leds_match), >> + }, >> + .probe = lm3692x_probe, >> + .remove = lm3692x_remove, >> + .id_table = lm3692x_id, >> +}; >> +module_i2c_driver(lm3692x_driver); >> + >> +MODULE_DESCRIPTION("Texas Instruments LM3692X LED driver"); >> +MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com>"); >> +MODULE_LICENSE("GPL"); > > According to your license info it should be "GPL v2" > OK Dan -- ------------------ Dan Murphy ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver 2017-11-15 19:42 ` Dan Murphy @ 2017-11-15 20:15 ` Jacek Anaszewski -1 siblings, 0 replies; 42+ messages in thread From: Jacek Anaszewski @ 2017-11-15 20:15 UTC (permalink / raw) To: Dan Murphy Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8, rpurdie-Fm38FmjxZ/leoWH0uzbU5w, pavel-+ZI9xUNit7I, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-leds-u79uwXL29TY76Z2rM5mHXA, Lee Jones, Daniel Thompson, Jingoo Han Hi Dan, Thanks for the patch. On 11/15/2017 08:42 PM, Dan Murphy wrote: > This adds the devicetree bindings for the LM3692x > I2C LED string driver. > > Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org> > Signed-off-by: Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org> > --- > > v3 - No changes > v2 - No changes - https://patchwork.kernel.org/patch/10056677/ > > .../devicetree/bindings/leds/leds-lm3692x.txt | 28 ++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3692x.txt > > diff --git a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt > new file mode 100644 > index 000000000000..cfef67bd4100 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt > @@ -0,0 +1,28 @@ > +* Texas Instruments - LM3692x Highly Efficient Dual-String White LED Driver > + > +The LM36922 is an ultra-compact, highly efficient, > +two string white-LED driver designed for LCD display > +backlighting. Shouldn't the driver be targeted for backlight subsystem then? Adding backlight maintainers. Best regards, Jacek Anaszewski > +Required properties: > + - compatible: > + "ti,lm3692x" > + - reg - I2C slave address > + > +Optional properties: > + - label - Used for naming LEDs > + - enable-gpio - gpio pin to enable/disable the device. > + - supply - "vled" - LED supply > + > +Example: > + > +leds: leds@6 { > + compatible = "ti,lm3692x"; > + reg = <0x36>; > + label = "backlight_cluster"; > + enable-gpio = <&gpio1 28 GPIO_ACTIVE_HIGH>; > + vled-supply = <&vbatt>; > +} > + > +For more product information please see the link below: > +http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver @ 2017-11-15 20:15 ` Jacek Anaszewski 0 siblings, 0 replies; 42+ messages in thread From: Jacek Anaszewski @ 2017-11-15 20:15 UTC (permalink / raw) To: Dan Murphy Cc: robh+dt, mark.rutland, rpurdie, pavel, devicetree, linux-kernel, linux-leds, Lee Jones, Daniel Thompson, Jingoo Han Hi Dan, Thanks for the patch. On 11/15/2017 08:42 PM, Dan Murphy wrote: > This adds the devicetree bindings for the LM3692x > I2C LED string driver. > > Acked-by: Pavel Machek <pavel@ucw.cz> > Signed-off-by: Dan Murphy <dmurphy@ti.com> > --- > > v3 - No changes > v2 - No changes - https://patchwork.kernel.org/patch/10056677/ > > .../devicetree/bindings/leds/leds-lm3692x.txt | 28 ++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3692x.txt > > diff --git a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt > new file mode 100644 > index 000000000000..cfef67bd4100 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt > @@ -0,0 +1,28 @@ > +* Texas Instruments - LM3692x Highly Efficient Dual-String White LED Driver > + > +The LM36922 is an ultra-compact, highly efficient, > +two string white-LED driver designed for LCD display > +backlighting. Shouldn't the driver be targeted for backlight subsystem then? Adding backlight maintainers. Best regards, Jacek Anaszewski > +Required properties: > + - compatible: > + "ti,lm3692x" > + - reg - I2C slave address > + > +Optional properties: > + - label - Used for naming LEDs > + - enable-gpio - gpio pin to enable/disable the device. > + - supply - "vled" - LED supply > + > +Example: > + > +leds: leds@6 { > + compatible = "ti,lm3692x"; > + reg = <0x36>; > + label = "backlight_cluster"; > + enable-gpio = <&gpio1 28 GPIO_ACTIVE_HIGH>; > + vled-supply = <&vbatt>; > +} > + > +For more product information please see the link below: > +http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf > ^ permalink raw reply [flat|nested] 42+ messages in thread
[parent not found: <cb5c5a65-d5a0-86a3-36b4-cf06baae9ec4-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver 2017-11-15 20:15 ` Jacek Anaszewski @ 2017-11-15 20:31 ` Dan Murphy -1 siblings, 0 replies; 42+ messages in thread From: Dan Murphy @ 2017-11-15 20:31 UTC (permalink / raw) To: Jacek Anaszewski Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8, rpurdie-Fm38FmjxZ/leoWH0uzbU5w, pavel-+ZI9xUNit7I, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-leds-u79uwXL29TY76Z2rM5mHXA, Lee Jones, Daniel Thompson, Jingoo Han Jacek On 11/15/2017 02:15 PM, Jacek Anaszewski wrote: > Hi Dan, > > Thanks for the patch. > > On 11/15/2017 08:42 PM, Dan Murphy wrote: >> This adds the devicetree bindings for the LM3692x >> I2C LED string driver. >> >> Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org> >> Signed-off-by: Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org> >> --- >> >> v3 - No changes >> v2 - No changes - https://patchwork.kernel.org/patch/10056677/ >> >> .../devicetree/bindings/leds/leds-lm3692x.txt | 28 ++++++++++++++++++++++ >> 1 file changed, 28 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3692x.txt >> >> diff --git a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt >> new file mode 100644 >> index 000000000000..cfef67bd4100 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt >> @@ -0,0 +1,28 @@ >> +* Texas Instruments - LM3692x Highly Efficient Dual-String White LED Driver >> + >> +The LM36922 is an ultra-compact, highly efficient, >> +two string white-LED driver designed for LCD display >> +backlighting. > > Shouldn't the driver be targeted for backlight subsystem then? > > Adding backlight maintainers. > Great point! I was not aware of the backlight subsystem. Looks like I need to create a back light version as well. Like the lp8788 did since this can be used as a LED driver beyond display back lighting. Unless the maintainers don't want it. Dan -- ------------------ Dan Murphy -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver @ 2017-11-15 20:31 ` Dan Murphy 0 siblings, 0 replies; 42+ messages in thread From: Dan Murphy @ 2017-11-15 20:31 UTC (permalink / raw) To: Jacek Anaszewski Cc: robh+dt, mark.rutland, rpurdie, pavel, devicetree, linux-kernel, linux-leds, Lee Jones, Daniel Thompson, Jingoo Han Jacek On 11/15/2017 02:15 PM, Jacek Anaszewski wrote: > Hi Dan, > > Thanks for the patch. > > On 11/15/2017 08:42 PM, Dan Murphy wrote: >> This adds the devicetree bindings for the LM3692x >> I2C LED string driver. >> >> Acked-by: Pavel Machek <pavel@ucw.cz> >> Signed-off-by: Dan Murphy <dmurphy@ti.com> >> --- >> >> v3 - No changes >> v2 - No changes - https://patchwork.kernel.org/patch/10056677/ >> >> .../devicetree/bindings/leds/leds-lm3692x.txt | 28 ++++++++++++++++++++++ >> 1 file changed, 28 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3692x.txt >> >> diff --git a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt >> new file mode 100644 >> index 000000000000..cfef67bd4100 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt >> @@ -0,0 +1,28 @@ >> +* Texas Instruments - LM3692x Highly Efficient Dual-String White LED Driver >> + >> +The LM36922 is an ultra-compact, highly efficient, >> +two string white-LED driver designed for LCD display >> +backlighting. > > Shouldn't the driver be targeted for backlight subsystem then? > > Adding backlight maintainers. > Great point! I was not aware of the backlight subsystem. Looks like I need to create a back light version as well. Like the lp8788 did since this can be used as a LED driver beyond display back lighting. Unless the maintainers don't want it. Dan -- ------------------ Dan Murphy ^ permalink raw reply [flat|nested] 42+ messages in thread
[parent not found: <0a78c883-f074-cc19-3c0c-fc05607400e9-l0cyMroinI0@public.gmane.org>]
* Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver 2017-11-15 20:31 ` Dan Murphy @ 2017-11-15 22:23 ` Pavel Machek -1 siblings, 0 replies; 42+ messages in thread From: Pavel Machek @ 2017-11-15 22:23 UTC (permalink / raw) To: Dan Murphy Cc: Jacek Anaszewski, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8, rpurdie-Fm38FmjxZ/leoWH0uzbU5w, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-leds-u79uwXL29TY76Z2rM5mHXA, Lee Jones, Daniel Thompson, Jingoo Han [-- Attachment #1: Type: text/plain, Size: 879 bytes --] Hi! > > Shouldn't the driver be targeted for backlight subsystem then? > > > > Adding backlight maintainers. > > > > Great point! I was not aware of the backlight subsystem. > > Looks like I need to create a back light version as well. > > Like the lp8788 did since this can be used as a LED driver beyond > display back lighting. No, definitely not two drivers for lp8788 hardware. If that does not yet exist... you want to create glue layer to be able to use LED as a display backlight. (It may already exist, no idea). ... Actually or maybe a LED trigger. Just set LED's trigger to "this is display backlight". Other solutions are acceptable, but not two hardware drivers. Thanks, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver @ 2017-11-15 22:23 ` Pavel Machek 0 siblings, 0 replies; 42+ messages in thread From: Pavel Machek @ 2017-11-15 22:23 UTC (permalink / raw) To: Dan Murphy Cc: Jacek Anaszewski, robh+dt, mark.rutland, rpurdie, devicetree, linux-kernel, linux-leds, Lee Jones, Daniel Thompson, Jingoo Han [-- Attachment #1: Type: text/plain, Size: 879 bytes --] Hi! > > Shouldn't the driver be targeted for backlight subsystem then? > > > > Adding backlight maintainers. > > > > Great point! I was not aware of the backlight subsystem. > > Looks like I need to create a back light version as well. > > Like the lp8788 did since this can be used as a LED driver beyond > display back lighting. No, definitely not two drivers for lp8788 hardware. If that does not yet exist... you want to create glue layer to be able to use LED as a display backlight. (It may already exist, no idea). ... Actually or maybe a LED trigger. Just set LED's trigger to "this is display backlight". Other solutions are acceptable, but not two hardware drivers. Thanks, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver 2017-11-15 22:23 ` Pavel Machek (?) @ 2017-11-16 20:14 ` Jacek Anaszewski [not found] ` <82336eb7-2b89-d37b-d688-4e4302766346-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> -1 siblings, 1 reply; 42+ messages in thread From: Jacek Anaszewski @ 2017-11-16 20:14 UTC (permalink / raw) To: Pavel Machek, Dan Murphy Cc: robh+dt, mark.rutland, rpurdie, devicetree, linux-kernel, linux-leds, Lee Jones, Daniel Thompson, Jingoo Han Hi Dan and Pavel, On 11/15/2017 11:23 PM, Pavel Machek wrote: > Hi! > >>> Shouldn't the driver be targeted for backlight subsystem then? >>> >>> Adding backlight maintainers. >>> >> >> Great point! I was not aware of the backlight subsystem. >> >> Looks like I need to create a back light version as well. >> >> Like the lp8788 did since this can be used as a LED driver beyond >> display back lighting. > > No, definitely not two drivers for lp8788 hardware. I agree. > If that does not yet exist... you want to create glue layer to be able > to use LED as a display backlight. (It may already exist, no idea). > > ... > > Actually or maybe a LED trigger. Just set LED's trigger to "this is > display backlight". There is one already: drivers/leds/trigger/ledtrig-backlight.c It adds a LED class device to the fb_notifier_list (drivers/video/fbdev/core/fb_notify.c) using fb_register_client(). The same is used in backlight_device_register (drivers/video/backlight/backlight.c). Actually why do you want to have this driver in the LED subsystem, if it is advertised as "designed for LCD display backlighting"? As a side note I can say that I've been always wondering why the two subsystems for similar type of hardware. -- Best regards, Jacek Anaszewski ^ permalink raw reply [flat|nested] 42+ messages in thread
[parent not found: <82336eb7-2b89-d37b-d688-4e4302766346-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver 2017-11-16 20:14 ` Jacek Anaszewski @ 2017-11-16 21:42 ` Dan Murphy 0 siblings, 0 replies; 42+ messages in thread From: Dan Murphy @ 2017-11-16 21:42 UTC (permalink / raw) To: Jacek Anaszewski, Pavel Machek Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8, rpurdie-Fm38FmjxZ/leoWH0uzbU5w, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-leds-u79uwXL29TY76Z2rM5mHXA, Lee Jones, Daniel Thompson, Jingoo Han Jacek On 11/16/2017 02:14 PM, Jacek Anaszewski wrote: > Hi Dan and Pavel, > > On 11/15/2017 11:23 PM, Pavel Machek wrote: >> Hi! >> >>>> Shouldn't the driver be targeted for backlight subsystem then? >>>> >>>> Adding backlight maintainers. >>>> >>> >>> Great point! I was not aware of the backlight subsystem. >>> >>> Looks like I need to create a back light version as well. >>> >>> Like the lp8788 did since this can be used as a LED driver beyond >>> display back lighting. >> >> No, definitely not two drivers for lp8788 hardware. > > I agree. > >> If that does not yet exist... you want to create glue layer to be able >> to use LED as a display backlight. (It may already exist, no idea). >> >> ... >> >> Actually or maybe a LED trigger. Just set LED's trigger to "this is >> display backlight". > > There is one already: > > drivers/leds/trigger/ledtrig-backlight.c > > It adds a LED class device to the fb_notifier_list > (drivers/video/fbdev/core/fb_notify.c) > > using fb_register_client(). The same is used in > backlight_device_register (drivers/video/backlight/backlight.c). > > Actually why do you want to have this driver in the LED subsystem, > if it is advertised as "designed for LCD display backlighting"? Well this is also advertised as a driver for Smart phone and tablet devices. And having worked with the Android lighting solutions this is the preferred subsystem for Android. The Android OS manages the led brightness based on ALS values and in turn calls into the driver to control the brightness register through the vendor provided lighting HAL. I am going to look at the backlight source to figure out how to get the same functionality using the backlight subsystem. Otherwise I will plug in this driver to the backlight subsystem through the notifier. > > As a side note I can say that I've been always wondering why the two > subsystems for similar type of hardware. This is my worry too. Why do we need both subsystems to do the same thing? I don't see either having one advantage over the other. Dan > -- ------------------ Dan Murphy -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver @ 2017-11-16 21:42 ` Dan Murphy 0 siblings, 0 replies; 42+ messages in thread From: Dan Murphy @ 2017-11-16 21:42 UTC (permalink / raw) To: Jacek Anaszewski, Pavel Machek Cc: robh+dt, mark.rutland, rpurdie, devicetree, linux-kernel, linux-leds, Lee Jones, Daniel Thompson, Jingoo Han Jacek On 11/16/2017 02:14 PM, Jacek Anaszewski wrote: > Hi Dan and Pavel, > > On 11/15/2017 11:23 PM, Pavel Machek wrote: >> Hi! >> >>>> Shouldn't the driver be targeted for backlight subsystem then? >>>> >>>> Adding backlight maintainers. >>>> >>> >>> Great point! I was not aware of the backlight subsystem. >>> >>> Looks like I need to create a back light version as well. >>> >>> Like the lp8788 did since this can be used as a LED driver beyond >>> display back lighting. >> >> No, definitely not two drivers for lp8788 hardware. > > I agree. > >> If that does not yet exist... you want to create glue layer to be able >> to use LED as a display backlight. (It may already exist, no idea). >> >> ... >> >> Actually or maybe a LED trigger. Just set LED's trigger to "this is >> display backlight". > > There is one already: > > drivers/leds/trigger/ledtrig-backlight.c > > It adds a LED class device to the fb_notifier_list > (drivers/video/fbdev/core/fb_notify.c) > > using fb_register_client(). The same is used in > backlight_device_register (drivers/video/backlight/backlight.c). > > Actually why do you want to have this driver in the LED subsystem, > if it is advertised as "designed for LCD display backlighting"? Well this is also advertised as a driver for Smart phone and tablet devices. And having worked with the Android lighting solutions this is the preferred subsystem for Android. The Android OS manages the led brightness based on ALS values and in turn calls into the driver to control the brightness register through the vendor provided lighting HAL. I am going to look at the backlight source to figure out how to get the same functionality using the backlight subsystem. Otherwise I will plug in this driver to the backlight subsystem through the notifier. > > As a side note I can say that I've been always wondering why the two > subsystems for similar type of hardware. This is my worry too. Why do we need both subsystems to do the same thing? I don't see either having one advantage over the other. Dan > -- ------------------ Dan Murphy ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver 2017-11-16 21:42 ` Dan Murphy @ 2017-11-17 2:19 ` Jingoo Han -1 siblings, 0 replies; 42+ messages in thread From: Jingoo Han @ 2017-11-17 2:19 UTC (permalink / raw) To: 'Dan Murphy', 'Jacek Anaszewski', 'Pavel Machek' Cc: robh+dt, mark.rutland, rpurdie, devicetree, linux-kernel, linux-leds, 'Lee Jones', 'Daniel Thompson' On Thursday, November 16, 2017 4:42 PM, Dan Murphy wrote: > > Jacek > > On 11/16/2017 02:14 PM, Jacek Anaszewski wrote: > > Hi Dan and Pavel, > > > > On 11/15/2017 11:23 PM, Pavel Machek wrote: > >> Hi! > >> > >>>> Shouldn't the driver be targeted for backlight subsystem then? > >>>> > >>>> Adding backlight maintainers. > >>>> > >>> > >>> Great point! I was not aware of the backlight subsystem. > >>> > >>> Looks like I need to create a back light version as well. > >>> > >>> Like the lp8788 did since this can be used as a LED driver beyond > >>> display back lighting. > >> > >> No, definitely not two drivers for lp8788 hardware. > > > > I agree. > > > >> If that does not yet exist... you want to create glue layer to be able > >> to use LED as a display backlight. (It may already exist, no idea). > >> > >> ... > >> > >> Actually or maybe a LED trigger. Just set LED's trigger to "this is > >> display backlight". > > > > There is one already: > > > > drivers/leds/trigger/ledtrig-backlight.c > > > > It adds a LED class device to the fb_notifier_list > > (drivers/video/fbdev/core/fb_notify.c) > > > > using fb_register_client(). The same is used in > > backlight_device_register (drivers/video/backlight/backlight.c). > > > > Actually why do you want to have this driver in the LED subsystem, > > if it is advertised as "designed for LCD display backlighting"? > > Well this is also advertised as a driver for Smart phone and tablet > devices. And having worked with the Android lighting > solutions this is the preferred subsystem for Android. The Android OS > manages the led brightness based on ALS values and in > turn calls into the driver to control the brightness register through the > vendor provided lighting HAL. > > I am going to look at the backlight source to figure out how to get the > same functionality using the backlight subsystem. > Otherwise I will plug in this driver to the backlight subsystem through > the notifier. I also developed Android devices for a long time. I think that modifying HAL will not be difficult. Also, backlight subsystem will be similar with LED subsystem. You can grasp it easily. Best regards, Jingoo Han > > > > > As a side note I can say that I've been always wondering why the two > > subsystems for similar type of hardware. > > This is my worry too. Why do we need both subsystems to do the same thing? > > I don't see either having one advantage over the other. > > > Dan > > > > > -- > ------------------ > Dan Murphy ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver @ 2017-11-17 2:19 ` Jingoo Han 0 siblings, 0 replies; 42+ messages in thread From: Jingoo Han @ 2017-11-17 2:19 UTC (permalink / raw) To: 'Dan Murphy', 'Jacek Anaszewski', 'Pavel Machek' Cc: robh+dt, mark.rutland, rpurdie, devicetree, linux-kernel, linux-leds, 'Lee Jones', 'Daniel Thompson' On Thursday, November 16, 2017 4:42 PM, Dan Murphy wrote: > > Jacek > > On 11/16/2017 02:14 PM, Jacek Anaszewski wrote: > > Hi Dan and Pavel, > > > > On 11/15/2017 11:23 PM, Pavel Machek wrote: > >> Hi! > >> > >>>> Shouldn't the driver be targeted for backlight subsystem then? > >>>> > >>>> Adding backlight maintainers. > >>>> > >>> > >>> Great point! I was not aware of the backlight subsystem. > >>> > >>> Looks like I need to create a back light version as well. > >>> > >>> Like the lp8788 did since this can be used as a LED driver beyond > >>> display back lighting. > >> > >> No, definitely not two drivers for lp8788 hardware. > > > > I agree. > > > >> If that does not yet exist... you want to create glue layer to be able > >> to use LED as a display backlight. (It may already exist, no idea). > >> > >> ... > >> > >> Actually or maybe a LED trigger. Just set LED's trigger to "this is > >> display backlight". > > > > There is one already: > > > > drivers/leds/trigger/ledtrig-backlight.c > > > > It adds a LED class device to the fb_notifier_list > > (drivers/video/fbdev/core/fb_notify.c) > > > > using fb_register_client(). The same is used in > > backlight_device_register (drivers/video/backlight/backlight.c). > > > > Actually why do you want to have this driver in the LED subsystem, > > if it is advertised as "designed for LCD display backlighting"? > > Well this is also advertised as a driver for Smart phone and tablet > devices. And having worked with the Android lighting > solutions this is the preferred subsystem for Android. The Android OS > manages the led brightness based on ALS values and in > turn calls into the driver to control the brightness register through the > vendor provided lighting HAL. > > I am going to look at the backlight source to figure out how to get the > same functionality using the backlight subsystem. > Otherwise I will plug in this driver to the backlight subsystem through > the notifier. I also developed Android devices for a long time. I think that modifying HAL will not be difficult. Also, backlight subsystem will be similar with LED subsystem. You can grasp it easily. Best regards, Jingoo Han > > > > > As a side note I can say that I've been always wondering why the two > > subsystems for similar type of hardware. > > This is my worry too. Why do we need both subsystems to do the same thing? > > I don't see either having one advantage over the other. > > > Dan > > > > > -- > ------------------ > Dan Murphy ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver 2017-11-16 21:42 ` Dan Murphy (?) (?) @ 2017-11-17 11:20 ` Pavel Machek 2017-11-17 16:30 ` Jingoo Han -1 siblings, 1 reply; 42+ messages in thread From: Pavel Machek @ 2017-11-17 11:20 UTC (permalink / raw) To: Dan Murphy Cc: Jacek Anaszewski, robh+dt, mark.rutland, rpurdie, devicetree, linux-kernel, linux-leds, Lee Jones, Daniel Thompson, Jingoo Han [-- Attachment #1: Type: text/plain, Size: 1651 bytes --] Hi! > >> If that does not yet exist... you want to create glue layer to be able > >> to use LED as a display backlight. (It may already exist, no idea). > >> > >> ... > >> > >> Actually or maybe a LED trigger. Just set LED's trigger to "this is > >> display backlight". > > > > There is one already: > > > > drivers/leds/trigger/ledtrig-backlight.c > > > > It adds a LED class device to the fb_notifier_list > > (drivers/video/fbdev/core/fb_notify.c) > > > > using fb_register_client(). The same is used in > > backlight_device_register (drivers/video/backlight/backlight.c). > > > > Actually why do you want to have this driver in the LED subsystem, > > if it is advertised as "designed for LCD display backlighting"? > > Well this is also advertised as a driver for Smart phone and tablet devices. And having worked with the Android lighting > solutions this is the preferred subsystem for Android. The Android OS manages the led brightness based on ALS values and in > turn calls into the driver to control the brightness register through the vendor provided lighting HAL. > Well.. if it can control other LEDs than just backlight, I believe it can stay in the LED subsystem. > I am going to look at the backlight source to figure out how to get the same functionality using the backlight subsystem. > Otherwise I will plug in this driver to the backlight subsystem through the notifier. > The backlight trigger should be ok. Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver 2017-11-17 11:20 ` Pavel Machek @ 2017-11-17 16:30 ` Jingoo Han 0 siblings, 0 replies; 42+ messages in thread From: Jingoo Han @ 2017-11-17 16:30 UTC (permalink / raw) To: 'Pavel Machek', 'Dan Murphy' Cc: 'Jacek Anaszewski', robh+dt, mark.rutland, rpurdie, devicetree, linux-kernel, linux-leds, 'Lee Jones', 'Daniel Thompson' On Friday, November 17, 2017 6:21 AM, Pavel Machek wrote: > > Hi! > > > >> If that does not yet exist... you want to create glue layer to be > able > > >> to use LED as a display backlight. (It may already exist, no idea). > > >> > > >> ... > > >> > > >> Actually or maybe a LED trigger. Just set LED's trigger to "this is > > >> display backlight". > > > > > > There is one already: > > > > > > drivers/leds/trigger/ledtrig-backlight.c > > > > > > It adds a LED class device to the fb_notifier_list > > > (drivers/video/fbdev/core/fb_notify.c) > > > > > > using fb_register_client(). The same is used in > > > backlight_device_register (drivers/video/backlight/backlight.c). > > > > > > Actually why do you want to have this driver in the LED subsystem, > > > if it is advertised as "designed for LCD display backlighting"? > > > > Well this is also advertised as a driver for Smart phone and tablet > devices. And having worked with the Android lighting > > solutions this is the preferred subsystem for Android. The Android OS > manages the led brightness based on ALS values and in > > turn calls into the driver to control the brightness register through > the vendor provided lighting HAL. > > > > Well.. if it can control other LEDs than just backlight, I believe it > can stay in the LED subsystem. I also agree with your opinion. Best regards, Jingoo Han > > > I am going to look at the backlight source to figure out how to get the > same functionality using the backlight subsystem. > > Otherwise I will plug in this driver to the backlight subsystem through > the notifier. > > > > The backlight trigger should be ok. > > Best regards, > Pavel > -- > (english) http://www.livejournal.com/~pavelmachek > (cesky, pictures) > http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver @ 2017-11-17 16:30 ` Jingoo Han 0 siblings, 0 replies; 42+ messages in thread From: Jingoo Han @ 2017-11-17 16:30 UTC (permalink / raw) To: 'Pavel Machek', 'Dan Murphy' Cc: 'Jacek Anaszewski', robh+dt, mark.rutland, rpurdie, devicetree, linux-kernel, linux-leds, 'Lee Jones', 'Daniel Thompson' On Friday, November 17, 2017 6:21 AM, Pavel Machek wrote: > > Hi! > > > >> If that does not yet exist... you want to create glue layer to be > able > > >> to use LED as a display backlight. (It may already exist, no idea). > > >> > > >> ... > > >> > > >> Actually or maybe a LED trigger. Just set LED's trigger to "this is > > >> display backlight". > > > > > > There is one already: > > > > > > drivers/leds/trigger/ledtrig-backlight.c > > > > > > It adds a LED class device to the fb_notifier_list > > > (drivers/video/fbdev/core/fb_notify.c) > > > > > > using fb_register_client(). The same is used in > > > backlight_device_register (drivers/video/backlight/backlight.c). > > > > > > Actually why do you want to have this driver in the LED subsystem, > > > if it is advertised as "designed for LCD display backlighting"? > > > > Well this is also advertised as a driver for Smart phone and tablet > devices. And having worked with the Android lighting > > solutions this is the preferred subsystem for Android. The Android OS > manages the led brightness based on ALS values and in > > turn calls into the driver to control the brightness register through > the vendor provided lighting HAL. > > > > Well.. if it can control other LEDs than just backlight, I believe it > can stay in the LED subsystem. I also agree with your opinion. Best regards, Jingoo Han > > > I am going to look at the backlight source to figure out how to get the > same functionality using the backlight subsystem. > > Otherwise I will plug in this driver to the backlight subsystem through > the notifier. > > > > The backlight trigger should be ok. > > Best regards, > Pavel > -- > (english) http://www.livejournal.com/~pavelmachek > (cesky, pictures) > http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver 2017-11-17 16:30 ` Jingoo Han @ 2017-11-17 18:02 ` Dan Murphy -1 siblings, 0 replies; 42+ messages in thread From: Dan Murphy @ 2017-11-17 18:02 UTC (permalink / raw) To: Jingoo Han, 'Pavel Machek' Cc: 'Jacek Anaszewski', robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8, rpurdie-Fm38FmjxZ/leoWH0uzbU5w, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-leds-u79uwXL29TY76Z2rM5mHXA, 'Lee Jones', 'Daniel Thompson' Pavel and Jacek On 11/17/2017 10:30 AM, Jingoo Han wrote: > On Friday, November 17, 2017 6:21 AM, Pavel Machek wrote: >> >> Hi! >> >>>>> If that does not yet exist... you want to create glue layer to be >> able >>>>> to use LED as a display backlight. (It may already exist, no idea). >>>>> >>>>> ... >>>>> >>>>> Actually or maybe a LED trigger. Just set LED's trigger to "this is >>>>> display backlight". >>>> >>>> There is one already: >>>> >>>> drivers/leds/trigger/ledtrig-backlight.c >>>> >>>> It adds a LED class device to the fb_notifier_list >>>> (drivers/video/fbdev/core/fb_notify.c) >>>> >>>> using fb_register_client(). The same is used in >>>> backlight_device_register (drivers/video/backlight/backlight.c). >>>> >>>> Actually why do you want to have this driver in the LED subsystem, >>>> if it is advertised as "designed for LCD display backlighting"? >>> >>> Well this is also advertised as a driver for Smart phone and tablet >> devices. And having worked with the Android lighting >>> solutions this is the preferred subsystem for Android. The Android OS >> manages the led brightness based on ALS values and in >>> turn calls into the driver to control the brightness register through >> the vendor provided lighting HAL. >>> >> >> Well.. if it can control other LEDs than just backlight, I believe it >> can stay in the LED subsystem. > > I also agree with your opinion. I will make the necessary changes for v4. Just a heads up I won't be posting v4 until after the US holiday so you don't think I abandoned everything. Dan > > Best regards, > Jingoo Han > >> >>> I am going to look at the backlight source to figure out how to get the >> same functionality using the backlight subsystem. >>> Otherwise I will plug in this driver to the backlight subsystem through >> the notifier. >>> >> >> The backlight trigger should be ok. >> >> Best regards, >> > Pavel >> -- >> (english) http://www.livejournal.com/~pavelmachek >> (cesky, pictures) >> http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html > -- ------------------ Dan Murphy -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver @ 2017-11-17 18:02 ` Dan Murphy 0 siblings, 0 replies; 42+ messages in thread From: Dan Murphy @ 2017-11-17 18:02 UTC (permalink / raw) To: Jingoo Han, 'Pavel Machek' Cc: 'Jacek Anaszewski', robh+dt, mark.rutland, rpurdie, devicetree, linux-kernel, linux-leds, 'Lee Jones', 'Daniel Thompson' Pavel and Jacek On 11/17/2017 10:30 AM, Jingoo Han wrote: > On Friday, November 17, 2017 6:21 AM, Pavel Machek wrote: >> >> Hi! >> >>>>> If that does not yet exist... you want to create glue layer to be >> able >>>>> to use LED as a display backlight. (It may already exist, no idea). >>>>> >>>>> ... >>>>> >>>>> Actually or maybe a LED trigger. Just set LED's trigger to "this is >>>>> display backlight". >>>> >>>> There is one already: >>>> >>>> drivers/leds/trigger/ledtrig-backlight.c >>>> >>>> It adds a LED class device to the fb_notifier_list >>>> (drivers/video/fbdev/core/fb_notify.c) >>>> >>>> using fb_register_client(). The same is used in >>>> backlight_device_register (drivers/video/backlight/backlight.c). >>>> >>>> Actually why do you want to have this driver in the LED subsystem, >>>> if it is advertised as "designed for LCD display backlighting"? >>> >>> Well this is also advertised as a driver for Smart phone and tablet >> devices. And having worked with the Android lighting >>> solutions this is the preferred subsystem for Android. The Android OS >> manages the led brightness based on ALS values and in >>> turn calls into the driver to control the brightness register through >> the vendor provided lighting HAL. >>> >> >> Well.. if it can control other LEDs than just backlight, I believe it >> can stay in the LED subsystem. > > I also agree with your opinion. I will make the necessary changes for v4. Just a heads up I won't be posting v4 until after the US holiday so you don't think I abandoned everything. Dan > > Best regards, > Jingoo Han > >> >>> I am going to look at the backlight source to figure out how to get the >> same functionality using the backlight subsystem. >>> Otherwise I will plug in this driver to the backlight subsystem through >> the notifier. >>> >> >> The backlight trigger should be ok. >> >> Best regards, >> > Pavel >> -- >> (english) http://www.livejournal.com/~pavelmachek >> (cesky, pictures) >> http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html > -- ------------------ Dan Murphy ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver 2017-11-17 18:02 ` Dan Murphy (?) @ 2017-11-17 23:58 ` Pavel Machek 2017-11-28 17:27 ` Dan Murphy -1 siblings, 1 reply; 42+ messages in thread From: Pavel Machek @ 2017-11-17 23:58 UTC (permalink / raw) To: Dan Murphy Cc: Jingoo Han, 'Jacek Anaszewski', robh+dt, mark.rutland, rpurdie, devicetree, linux-kernel, linux-leds, 'Lee Jones', 'Daniel Thompson' [-- Attachment #1: Type: text/plain, Size: 600 bytes --] Hi! > >> Well.. if it can control other LEDs than just backlight, I believe it > >> can stay in the LED subsystem. > > > > I also agree with your opinion. > > I will make the necessary changes for v4. I'm not sure if you need to make any changes. Just add default trigger to the dts and you should be done. > Just a heads up I won't be posting v4 until after the US holiday so you > don't think I abandoned everything. :-) Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver 2017-11-17 23:58 ` Pavel Machek @ 2017-11-28 17:27 ` Dan Murphy 0 siblings, 0 replies; 42+ messages in thread From: Dan Murphy @ 2017-11-28 17:27 UTC (permalink / raw) To: Pavel Machek Cc: Jingoo Han, 'Jacek Anaszewski', robh+dt, mark.rutland, rpurdie, devicetree, linux-kernel, linux-leds, 'Lee Jones', 'Daniel Thompson' Pavel On 11/17/2017 05:58 PM, Pavel Machek wrote: > Hi! > >>>> Well.. if it can control other LEDs than just backlight, I believe it >>>> can stay in the LED subsystem. >>> >>> I also agree with your opinion. >> >> I will make the necessary changes for v4. > > I'm not sure if you need to make any changes. Just add default trigger > to the dts and you should be done. > Sorry what I meant was changes for the additional v3 comments. Dan >> Just a heads up I won't be posting v4 until after the US holiday so you >> don't think I abandoned everything. > > :-) > Pavel > -- ------------------ Dan Murphy ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver @ 2017-11-28 17:27 ` Dan Murphy 0 siblings, 0 replies; 42+ messages in thread From: Dan Murphy @ 2017-11-28 17:27 UTC (permalink / raw) To: Pavel Machek Cc: Jingoo Han, 'Jacek Anaszewski', robh+dt, mark.rutland, rpurdie, devicetree, linux-kernel, linux-leds, 'Lee Jones', 'Daniel Thompson' Pavel On 11/17/2017 05:58 PM, Pavel Machek wrote: > Hi! > >>>> Well.. if it can control other LEDs than just backlight, I believe it >>>> can stay in the LED subsystem. >>> >>> I also agree with your opinion. >> >> I will make the necessary changes for v4. > > I'm not sure if you need to make any changes. Just add default trigger > to the dts and you should be done. > Sorry what I meant was changes for the additional v3 comments. Dan >> Just a heads up I won't be posting v4 until after the US holiday so you >> don't think I abandoned everything. > > :-) > Pavel > -- ------------------ Dan Murphy ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver 2017-11-15 19:42 ` Dan Murphy @ 2017-11-16 15:41 ` Rob Herring -1 siblings, 0 replies; 42+ messages in thread From: Rob Herring @ 2017-11-16 15:41 UTC (permalink / raw) To: Dan Murphy Cc: mark.rutland-5wv7dgnIgG8, rpurdie-Fm38FmjxZ/leoWH0uzbU5w, jacek.anaszewski-Re5JQEeQqe8AvxtiuMwx3w, pavel-+ZI9xUNit7I, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-leds-u79uwXL29TY76Z2rM5mHXA On Wed, Nov 15, 2017 at 01:42:02PM -0600, Dan Murphy wrote: > This adds the devicetree bindings for the LM3692x > I2C LED string driver. > > Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org> > Signed-off-by: Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org> > --- > > v3 - No changes > v2 - No changes - https://patchwork.kernel.org/patch/10056677/ > > .../devicetree/bindings/leds/leds-lm3692x.txt | 28 ++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3692x.txt > > diff --git a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt > new file mode 100644 > index 000000000000..cfef67bd4100 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt > @@ -0,0 +1,28 @@ > +* Texas Instruments - LM3692x Highly Efficient Dual-String White LED Driver > + > +The LM36922 is an ultra-compact, highly efficient, > +two string white-LED driver designed for LCD display > +backlighting. > + > +Required properties: > + - compatible: > + "ti,lm3692x" Don't use wildcards in compatible strings. > + - reg - I2C slave address > + > +Optional properties: > + - label - Used for naming LEDs > + - enable-gpio - gpio pin to enable/disable the device. enable-gpios is the preferred form. > + - supply - "vled" - LED supply vled-supply - ... > + > +Example: > + > +leds: leds@6 { > + compatible = "ti,lm3692x"; > + reg = <0x36>; > + label = "backlight_cluster"; > + enable-gpio = <&gpio1 28 GPIO_ACTIVE_HIGH>; > + vled-supply = <&vbatt>; > +} > + > +For more product information please see the link below: > +http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf > -- > 2.15.0.124.g7668cbc60 > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver @ 2017-11-16 15:41 ` Rob Herring 0 siblings, 0 replies; 42+ messages in thread From: Rob Herring @ 2017-11-16 15:41 UTC (permalink / raw) To: Dan Murphy Cc: mark.rutland, rpurdie, jacek.anaszewski, pavel, devicetree, linux-kernel, linux-leds On Wed, Nov 15, 2017 at 01:42:02PM -0600, Dan Murphy wrote: > This adds the devicetree bindings for the LM3692x > I2C LED string driver. > > Acked-by: Pavel Machek <pavel@ucw.cz> > Signed-off-by: Dan Murphy <dmurphy@ti.com> > --- > > v3 - No changes > v2 - No changes - https://patchwork.kernel.org/patch/10056677/ > > .../devicetree/bindings/leds/leds-lm3692x.txt | 28 ++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3692x.txt > > diff --git a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt > new file mode 100644 > index 000000000000..cfef67bd4100 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt > @@ -0,0 +1,28 @@ > +* Texas Instruments - LM3692x Highly Efficient Dual-String White LED Driver > + > +The LM36922 is an ultra-compact, highly efficient, > +two string white-LED driver designed for LCD display > +backlighting. > + > +Required properties: > + - compatible: > + "ti,lm3692x" Don't use wildcards in compatible strings. > + - reg - I2C slave address > + > +Optional properties: > + - label - Used for naming LEDs > + - enable-gpio - gpio pin to enable/disable the device. enable-gpios is the preferred form. > + - supply - "vled" - LED supply vled-supply - ... > + > +Example: > + > +leds: leds@6 { > + compatible = "ti,lm3692x"; > + reg = <0x36>; > + label = "backlight_cluster"; > + enable-gpio = <&gpio1 28 GPIO_ACTIVE_HIGH>; > + vled-supply = <&vbatt>; > +} > + > +For more product information please see the link below: > +http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf > -- > 2.15.0.124.g7668cbc60 > ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver 2017-11-16 15:41 ` Rob Herring @ 2017-11-16 15:45 ` Dan Murphy -1 siblings, 0 replies; 42+ messages in thread From: Dan Murphy @ 2017-11-16 15:45 UTC (permalink / raw) To: Rob Herring Cc: mark.rutland, rpurdie, jacek.anaszewski, pavel, devicetree, linux-kernel, linux-leds Rob Thanks for the review On 11/16/2017 09:41 AM, Rob Herring wrote: > On Wed, Nov 15, 2017 at 01:42:02PM -0600, Dan Murphy wrote: >> This adds the devicetree bindings for the LM3692x >> I2C LED string driver. >> >> Acked-by: Pavel Machek <pavel@ucw.cz> >> Signed-off-by: Dan Murphy <dmurphy@ti.com> >> --- >> >> v3 - No changes >> v2 - No changes - https://patchwork.kernel.org/patch/10056677/ >> >> .../devicetree/bindings/leds/leds-lm3692x.txt | 28 ++++++++++++++++++++++ >> 1 file changed, 28 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3692x.txt >> >> diff --git a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt >> new file mode 100644 >> index 000000000000..cfef67bd4100 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt >> @@ -0,0 +1,28 @@ >> +* Texas Instruments - LM3692x Highly Efficient Dual-String White LED Driver >> + >> +The LM36922 is an ultra-compact, highly efficient, >> +two string white-LED driver designed for LCD display >> +backlighting. >> + >> +Required properties: >> + - compatible: >> + "ti,lm3692x" > > Don't use wildcards in compatible strings. Do you mean to remove the x? How do we denote a family of parts then? > >> + - reg - I2C slave address >> + >> +Optional properties: >> + - label - Used for naming LEDs >> + - enable-gpio - gpio pin to enable/disable the device. > > enable-gpios is the preferred form. ack > >> + - supply - "vled" - LED supply > > vled-supply - ... > ack >> + >> +Example: >> + >> +leds: leds@6 { >> + compatible = "ti,lm3692x"; >> + reg = <0x36>; >> + label = "backlight_cluster"; >> + enable-gpio = <&gpio1 28 GPIO_ACTIVE_HIGH>; >> + vled-supply = <&vbatt>; >> +} >> + >> +For more product information please see the link below: >> +http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf >> -- >> 2.15.0.124.g7668cbc60 >> -- ------------------ Dan Murphy ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver @ 2017-11-16 15:45 ` Dan Murphy 0 siblings, 0 replies; 42+ messages in thread From: Dan Murphy @ 2017-11-16 15:45 UTC (permalink / raw) To: Rob Herring Cc: mark.rutland, rpurdie, jacek.anaszewski, pavel, devicetree, linux-kernel, linux-leds Rob Thanks for the review On 11/16/2017 09:41 AM, Rob Herring wrote: > On Wed, Nov 15, 2017 at 01:42:02PM -0600, Dan Murphy wrote: >> This adds the devicetree bindings for the LM3692x >> I2C LED string driver. >> >> Acked-by: Pavel Machek <pavel@ucw.cz> >> Signed-off-by: Dan Murphy <dmurphy@ti.com> >> --- >> >> v3 - No changes >> v2 - No changes - https://patchwork.kernel.org/patch/10056677/ >> >> .../devicetree/bindings/leds/leds-lm3692x.txt | 28 ++++++++++++++++++++++ >> 1 file changed, 28 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3692x.txt >> >> diff --git a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt >> new file mode 100644 >> index 000000000000..cfef67bd4100 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt >> @@ -0,0 +1,28 @@ >> +* Texas Instruments - LM3692x Highly Efficient Dual-String White LED Driver >> + >> +The LM36922 is an ultra-compact, highly efficient, >> +two string white-LED driver designed for LCD display >> +backlighting. >> + >> +Required properties: >> + - compatible: >> + "ti,lm3692x" > > Don't use wildcards in compatible strings. Do you mean to remove the x? How do we denote a family of parts then? > >> + - reg - I2C slave address >> + >> +Optional properties: >> + - label - Used for naming LEDs >> + - enable-gpio - gpio pin to enable/disable the device. > > enable-gpios is the preferred form. ack > >> + - supply - "vled" - LED supply > > vled-supply - ... > ack >> + >> +Example: >> + >> +leds: leds@6 { >> + compatible = "ti,lm3692x"; >> + reg = <0x36>; >> + label = "backlight_cluster"; >> + enable-gpio = <&gpio1 28 GPIO_ACTIVE_HIGH>; >> + vled-supply = <&vbatt>; >> +} >> + >> +For more product information please see the link below: >> +http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf >> -- >> 2.15.0.124.g7668cbc60 >> -- ------------------ Dan Murphy ^ permalink raw reply [flat|nested] 42+ messages in thread
[parent not found: <d3483e49-650d-1d68-bec6-eeb84d8ac48a-l0cyMroinI0@public.gmane.org>]
* Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver 2017-11-16 15:45 ` Dan Murphy @ 2017-11-16 15:58 ` Rob Herring -1 siblings, 0 replies; 42+ messages in thread From: Rob Herring @ 2017-11-16 15:58 UTC (permalink / raw) To: Dan Murphy Cc: Mark Rutland, Richard Purdie, Jacek Anaszewski, Pavel Machek, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Linux LED Subsystem On Thu, Nov 16, 2017 at 9:45 AM, Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org> wrote: > Rob > > Thanks for the review > > On 11/16/2017 09:41 AM, Rob Herring wrote: >> On Wed, Nov 15, 2017 at 01:42:02PM -0600, Dan Murphy wrote: >>> This adds the devicetree bindings for the LM3692x >>> I2C LED string driver. >>> >>> Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org> >>> Signed-off-by: Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org> >>> --- >>> >>> v3 - No changes >>> v2 - No changes - https://patchwork.kernel.org/patch/10056677/ >>> >>> .../devicetree/bindings/leds/leds-lm3692x.txt | 28 ++++++++++++++++++++++ >>> 1 file changed, 28 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3692x.txt >>> >>> diff --git a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt >>> new file mode 100644 >>> index 000000000000..cfef67bd4100 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt >>> @@ -0,0 +1,28 @@ >>> +* Texas Instruments - LM3692x Highly Efficient Dual-String White LED Driver >>> + >>> +The LM36922 is an ultra-compact, highly efficient, >>> +two string white-LED driver designed for LCD display >>> +backlighting. >>> + >>> +Required properties: >>> + - compatible: >>> + "ti,lm3692x" >> >> Don't use wildcards in compatible strings. > > Do you mean to remove the x? How do we denote a family of parts then? Yes. With a compatible string for each one. You can have multiple compatible strings if some are a superset of others. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver @ 2017-11-16 15:58 ` Rob Herring 0 siblings, 0 replies; 42+ messages in thread From: Rob Herring @ 2017-11-16 15:58 UTC (permalink / raw) To: Dan Murphy Cc: Mark Rutland, Richard Purdie, Jacek Anaszewski, Pavel Machek, devicetree, linux-kernel, Linux LED Subsystem On Thu, Nov 16, 2017 at 9:45 AM, Dan Murphy <dmurphy@ti.com> wrote: > Rob > > Thanks for the review > > On 11/16/2017 09:41 AM, Rob Herring wrote: >> On Wed, Nov 15, 2017 at 01:42:02PM -0600, Dan Murphy wrote: >>> This adds the devicetree bindings for the LM3692x >>> I2C LED string driver. >>> >>> Acked-by: Pavel Machek <pavel@ucw.cz> >>> Signed-off-by: Dan Murphy <dmurphy@ti.com> >>> --- >>> >>> v3 - No changes >>> v2 - No changes - https://patchwork.kernel.org/patch/10056677/ >>> >>> .../devicetree/bindings/leds/leds-lm3692x.txt | 28 ++++++++++++++++++++++ >>> 1 file changed, 28 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3692x.txt >>> >>> diff --git a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt >>> new file mode 100644 >>> index 000000000000..cfef67bd4100 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt >>> @@ -0,0 +1,28 @@ >>> +* Texas Instruments - LM3692x Highly Efficient Dual-String White LED Driver >>> + >>> +The LM36922 is an ultra-compact, highly efficient, >>> +two string white-LED driver designed for LCD display >>> +backlighting. >>> + >>> +Required properties: >>> + - compatible: >>> + "ti,lm3692x" >> >> Don't use wildcards in compatible strings. > > Do you mean to remove the x? How do we denote a family of parts then? Yes. With a compatible string for each one. You can have multiple compatible strings if some are a superset of others. Rob ^ permalink raw reply [flat|nested] 42+ messages in thread
* dts: fun with chip names Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver 2017-11-16 15:45 ` Dan Murphy (?) (?) @ 2017-11-16 20:11 ` Pavel Machek 2017-11-16 20:36 ` Rob Herring -1 siblings, 1 reply; 42+ messages in thread From: Pavel Machek @ 2017-11-16 20:11 UTC (permalink / raw) To: Dan Murphy Cc: Rob Herring, mark.rutland, rpurdie, jacek.anaszewski, devicetree, linux-kernel, linux-leds [-- Attachment #1: Type: text/plain, Size: 817 bytes --] Hi! > >> +Required properties: > >> + - compatible: > >> + "ti,lm3692x" > > > > Don't use wildcards in compatible strings. > > Do you mean to remove the x? How do we denote a family of parts > then? I guess you should specify the exact chip. Which will present interesting problem for me on Nokia N9/N950; in one case, compatible chip is produced by two companies, and it looks like some machines have one and some have the other; but we'd like to share the dts as user has no chance telling them apart (and it is not important, anyway). In second case, chip is refered as APDS990X and I don't know where to get more exact data. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: dts: fun with chip names Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver 2017-11-16 20:11 ` dts: fun with chip names " Pavel Machek @ 2017-11-16 20:36 ` Rob Herring [not found] ` <CAL_Jsq+aysVR9U2Wquwwv7cjjO7wO8EXBg1Ja82u6s5t1yeJUw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 42+ messages in thread From: Rob Herring @ 2017-11-16 20:36 UTC (permalink / raw) To: Pavel Machek Cc: Dan Murphy, Mark Rutland, Richard Purdie, Jacek Anaszewski, devicetree, linux-kernel, Linux LED Subsystem On Thu, Nov 16, 2017 at 2:11 PM, Pavel Machek <pavel@ucw.cz> wrote: > Hi! > >> >> +Required properties: >> >> + - compatible: >> >> + "ti,lm3692x" >> > >> > Don't use wildcards in compatible strings. >> >> Do you mean to remove the x? How do we denote a family of parts >> then? > > I guess you should specify the exact chip. > > Which will present interesting problem for me on Nokia N9/N950; in one > case, compatible chip is produced by two companies, and it looks like > some machines have one and some have the other; but we'd like to share > the dts as user has no chance telling them apart (and it is not > important, anyway). > > In second case, chip is refered as APDS990X and I don't know where to > get more exact data. There's always exceptions to rules. Just make the case for it. IIRC, there was the same case for the BT chip. Rob ^ permalink raw reply [flat|nested] 42+ messages in thread
[parent not found: <CAL_Jsq+aysVR9U2Wquwwv7cjjO7wO8EXBg1Ja82u6s5t1yeJUw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: dts: fun with chip names Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver 2017-11-16 20:36 ` Rob Herring @ 2017-11-16 21:40 ` Dan Murphy 0 siblings, 0 replies; 42+ messages in thread From: Dan Murphy @ 2017-11-16 21:40 UTC (permalink / raw) To: Rob Herring, Pavel Machek Cc: Mark Rutland, Richard Purdie, Jacek Anaszewski, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Linux LED Subsystem On 11/16/2017 02:36 PM, Rob Herring wrote: > On Thu, Nov 16, 2017 at 2:11 PM, Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org> wrote: >> Hi! >> >>>>> +Required properties: >>>>> + - compatible: >>>>> + "ti,lm3692x" >>>> >>>> Don't use wildcards in compatible strings. >>> >>> Do you mean to remove the x? How do we denote a family of parts >>> then? >> >> I guess you should specify the exact chip. >> >> Which will present interesting problem for me on Nokia N9/N950; in one >> case, compatible chip is produced by two companies, and it looks like >> some machines have one and some have the other; but we'd like to share >> the dts as user has no chance telling them apart (and it is not >> important, anyway). >> >> In second case, chip is refered as APDS990X and I don't know where to >> get more exact data. > > There's always exceptions to rules. Just make the case for it. IIRC, > there was the same case for the BT chip. > Not sure how I got looped into the dts fun. But I am going to call out the specific chips out in the compatible strings. There are currently only 2 devices supported by this driver. The delta between the chips is one supports a third string of LEDs. 1 bit in 1 register. Dan > Rob > -- ------------------ Dan Murphy -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: dts: fun with chip names Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver @ 2017-11-16 21:40 ` Dan Murphy 0 siblings, 0 replies; 42+ messages in thread From: Dan Murphy @ 2017-11-16 21:40 UTC (permalink / raw) To: Rob Herring, Pavel Machek Cc: Mark Rutland, Richard Purdie, Jacek Anaszewski, devicetree, linux-kernel, Linux LED Subsystem On 11/16/2017 02:36 PM, Rob Herring wrote: > On Thu, Nov 16, 2017 at 2:11 PM, Pavel Machek <pavel@ucw.cz> wrote: >> Hi! >> >>>>> +Required properties: >>>>> + - compatible: >>>>> + "ti,lm3692x" >>>> >>>> Don't use wildcards in compatible strings. >>> >>> Do you mean to remove the x? How do we denote a family of parts >>> then? >> >> I guess you should specify the exact chip. >> >> Which will present interesting problem for me on Nokia N9/N950; in one >> case, compatible chip is produced by two companies, and it looks like >> some machines have one and some have the other; but we'd like to share >> the dts as user has no chance telling them apart (and it is not >> important, anyway). >> >> In second case, chip is refered as APDS990X and I don't know where to >> get more exact data. > > There's always exceptions to rules. Just make the case for it. IIRC, > there was the same case for the BT chip. > Not sure how I got looped into the dts fun. But I am going to call out the specific chips out in the compatible strings. There are currently only 2 devices supported by this driver. The delta between the chips is one supports a third string of LEDs. 1 bit in 1 register. Dan > Rob > -- ------------------ Dan Murphy ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver 2017-11-15 19:42 ` Dan Murphy (?) (?) @ 2017-11-18 14:19 ` Jacek Anaszewski -1 siblings, 0 replies; 42+ messages in thread From: Jacek Anaszewski @ 2017-11-18 14:19 UTC (permalink / raw) To: Dan Murphy, robh+dt, mark.rutland, rpurdie, pavel Cc: devicetree, linux-kernel, linux-leds Hi Dan, On 11/15/2017 08:42 PM, Dan Murphy wrote: > This adds the devicetree bindings for the LM3692x > I2C LED string driver. > > Acked-by: Pavel Machek <pavel@ucw.cz> > Signed-off-by: Dan Murphy <dmurphy@ti.com> > --- > > v3 - No changes > v2 - No changes - https://patchwork.kernel.org/patch/10056677/ > > .../devicetree/bindings/leds/leds-lm3692x.txt | 28 ++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3692x.txt > > diff --git a/Documentation/devicetree/bindings/leds/leds-lm3692x.txt b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt > new file mode 100644 > index 000000000000..cfef67bd4100 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/leds-lm3692x.txt > @@ -0,0 +1,28 @@ > +* Texas Instruments - LM3692x Highly Efficient Dual-String White LED Driver > + > +The LM36922 is an ultra-compact, highly efficient, > +two string white-LED driver designed for LCD display > +backlighting. > + > +Required properties: > + - compatible: > + "ti,lm3692x" > + - reg - I2C slave address > + > +Optional properties: > + - label - Used for naming LEDs > + - enable-gpio - gpio pin to enable/disable the device. > + - supply - "vled" - LED supply > + > +Example: > + > +leds: leds@6 { > + compatible = "ti,lm3692x"; > + reg = <0x36>; > + label = "backlight_cluster"; Documentation/devicetree/bindings/leds/common.txt states that each LED component has to be represented as a child node (for consistency it should apply also to the arrangements with one LED). > + enable-gpio = <&gpio1 28 GPIO_ACTIVE_HIGH>; > + vled-supply = <&vbatt>; > +} > + > +For more product information please see the link below: > +http://www.ti.com/lit/ds/snvsa29/snvsa29.pdf > -- Best regards, Jacek Anaszewski ^ permalink raw reply [flat|nested] 42+ messages in thread
end of thread, other threads:[~2017-11-28 17:27 UTC | newest]
Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-15 19:42 [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver Dan Murphy
2017-11-15 19:42 ` Dan Murphy
[not found] ` <20171115194203.13572-1-dmurphy-l0cyMroinI0@public.gmane.org>
2017-11-15 19:42 ` [PATCH v3 2/2] leds: lm3692x: Introduce LM3692x dual string driver Dan Murphy
2017-11-15 19:42 ` Dan Murphy
[not found] ` <20171115194203.13572-2-dmurphy-l0cyMroinI0@public.gmane.org>
2017-11-15 21:12 ` Pavel Machek
2017-11-15 21:12 ` Pavel Machek
2017-11-15 21:25 ` Dan Murphy
2017-11-15 21:25 ` Dan Murphy
2017-11-18 14:19 ` Jacek Anaszewski
2017-11-18 14:19 ` Jacek Anaszewski
2017-11-28 12:53 ` Dan Murphy
2017-11-28 12:53 ` Dan Murphy
2017-11-15 20:15 ` [PATCH v3 1/2] dt: bindings: lm3692x: Add bindings for lm3692x LED driver Jacek Anaszewski
2017-11-15 20:15 ` Jacek Anaszewski
[not found] ` <cb5c5a65-d5a0-86a3-36b4-cf06baae9ec4-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-11-15 20:31 ` Dan Murphy
2017-11-15 20:31 ` Dan Murphy
[not found] ` <0a78c883-f074-cc19-3c0c-fc05607400e9-l0cyMroinI0@public.gmane.org>
2017-11-15 22:23 ` Pavel Machek
2017-11-15 22:23 ` Pavel Machek
2017-11-16 20:14 ` Jacek Anaszewski
[not found] ` <82336eb7-2b89-d37b-d688-4e4302766346-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-11-16 21:42 ` Dan Murphy
2017-11-16 21:42 ` Dan Murphy
2017-11-17 2:19 ` Jingoo Han
2017-11-17 2:19 ` Jingoo Han
2017-11-17 11:20 ` Pavel Machek
2017-11-17 16:30 ` Jingoo Han
2017-11-17 16:30 ` Jingoo Han
2017-11-17 18:02 ` Dan Murphy
2017-11-17 18:02 ` Dan Murphy
2017-11-17 23:58 ` Pavel Machek
2017-11-28 17:27 ` Dan Murphy
2017-11-28 17:27 ` Dan Murphy
2017-11-16 15:41 ` Rob Herring
2017-11-16 15:41 ` Rob Herring
2017-11-16 15:45 ` Dan Murphy
2017-11-16 15:45 ` Dan Murphy
[not found] ` <d3483e49-650d-1d68-bec6-eeb84d8ac48a-l0cyMroinI0@public.gmane.org>
2017-11-16 15:58 ` Rob Herring
2017-11-16 15:58 ` Rob Herring
2017-11-16 20:11 ` dts: fun with chip names " Pavel Machek
2017-11-16 20:36 ` Rob Herring
[not found] ` <CAL_Jsq+aysVR9U2Wquwwv7cjjO7wO8EXBg1Ja82u6s5t1yeJUw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-11-16 21:40 ` Dan Murphy
2017-11-16 21:40 ` Dan Murphy
2017-11-18 14:19 ` Jacek Anaszewski
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.