All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] TI LMU rework
@ 2019-03-25 14:23 ` Dan Murphy
  0 siblings, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-03-25 14:23 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel; +Cc: linux-kernel, linux-leds, Dan Murphy

All

I know that it has been a long time but I put some additional effort into this
code.  The TI LMU common code right now handles brightness and ramp up/down
setting for the LM3697.  This so far are the common features I could find.

The LM3697 driver has the ability to assign HVLED strings to specific control
banks as well as assigning different max brightnesses to these strings.

Fault monitoring was removed as the data sheet indicates that this is for
production tests only.

I have plans to add additional LED drivers to use the TI LMU but I figured trying
to add all of them at once would be a daunting review and probably wrought with 
problems.

Dan

Dan Murphy (5):
  dt-bindings: mfd: Update the ramp up/down property
  leds: TI LMU: Add common code for TI LMU devices
  dt-bindings: ti-lmu: Modify dt bindings for the LM3697
  mfd: ti-lmu: Remove support for LM3697
  leds: lm3697: Introduce the lm3697 driver

 .../devicetree/bindings/leds/leds-lm3697.txt  |  77 ++++
 .../devicetree/bindings/mfd/ti-lmu.txt        |  42 +-
 drivers/leds/Kconfig                          |  14 +
 drivers/leds/Makefile                         |   2 +
 drivers/leds/leds-lm3697.c                    | 401 ++++++++++++++++++
 drivers/leds/ti-lmu-led-common.c              | 131 ++++++
 drivers/mfd/Kconfig                           |   2 +-
 drivers/mfd/ti-lmu.c                          |  17 -
 include/linux/mfd/ti-lmu-register.h           |  44 --
 include/linux/mfd/ti-lmu.h                    |   1 -
 include/linux/ti-lmu-led-common.h             |  44 ++
 11 files changed, 682 insertions(+), 93 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3697.txt
 create mode 100644 drivers/leds/leds-lm3697.c
 create mode 100644 drivers/leds/ti-lmu-led-common.c
 create mode 100644 include/linux/ti-lmu-led-common.h

-- 
2.19.0

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH 0/5] TI LMU rework
@ 2019-03-25 14:23 ` Dan Murphy
  0 siblings, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-03-25 14:23 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel; +Cc: linux-kernel, linux-leds, Dan Murphy

All

I know that it has been a long time but I put some additional effort into this
code.  The TI LMU common code right now handles brightness and ramp up/down
setting for the LM3697.  This so far are the common features I could find.

The LM3697 driver has the ability to assign HVLED strings to specific control
banks as well as assigning different max brightnesses to these strings.

Fault monitoring was removed as the data sheet indicates that this is for
production tests only.

I have plans to add additional LED drivers to use the TI LMU but I figured trying
to add all of them at once would be a daunting review and probably wrought with 
problems.

Dan

Dan Murphy (5):
  dt-bindings: mfd: Update the ramp up/down property
  leds: TI LMU: Add common code for TI LMU devices
  dt-bindings: ti-lmu: Modify dt bindings for the LM3697
  mfd: ti-lmu: Remove support for LM3697
  leds: lm3697: Introduce the lm3697 driver

 .../devicetree/bindings/leds/leds-lm3697.txt  |  77 ++++
 .../devicetree/bindings/mfd/ti-lmu.txt        |  42 +-
 drivers/leds/Kconfig                          |  14 +
 drivers/leds/Makefile                         |   2 +
 drivers/leds/leds-lm3697.c                    | 401 ++++++++++++++++++
 drivers/leds/ti-lmu-led-common.c              | 131 ++++++
 drivers/mfd/Kconfig                           |   2 +-
 drivers/mfd/ti-lmu.c                          |  17 -
 include/linux/mfd/ti-lmu-register.h           |  44 --
 include/linux/mfd/ti-lmu.h                    |   1 -
 include/linux/ti-lmu-led-common.h             |  44 ++
 11 files changed, 682 insertions(+), 93 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3697.txt
 create mode 100644 drivers/leds/leds-lm3697.c
 create mode 100644 drivers/leds/ti-lmu-led-common.c
 create mode 100644 include/linux/ti-lmu-led-common.h

-- 
2.19.0


^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH 1/5] dt-bindings: mfd: Update the ramp up/down property
  2019-03-25 14:23 ` Dan Murphy
@ 2019-03-25 14:23   ` Dan Murphy
  -1 siblings, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-03-25 14:23 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel; +Cc: linux-kernel, linux-leds, Dan Murphy

Modify the ramp-up/down property and add the property description
to the binding.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 .../devicetree/bindings/mfd/ti-lmu.txt        | 20 ++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/ti-lmu.txt b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
index 980394d701a7..d9d529de6dc1 100644
--- a/Documentation/devicetree/bindings/mfd/ti-lmu.txt
+++ b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
@@ -25,6 +25,12 @@ Required properties:
 
 Optional property:
   - enable-gpios: A GPIO specifier for hardware enable pin.
+  - ramp-up-ms: Current ramping from one brightness level to
+		the a higher brightness level.
+		Range from 2048 us - 117.44 s
+  - ramp-down-ms: Current ramping from one brightness level to
+		  the a lower brightness level.
+		  Range from 2048 us - 117.44 s
 
 Required node:
   - backlight: All LMU devices have backlight child nodes.
@@ -90,7 +96,7 @@ lm3631@29 {
 
 		lcd_bl {
 			led-sources = <0 1>;
-			ramp-up-msec = <300>;
+			ramp-up-ms = <300>;
 		};
 	};
 };
@@ -152,15 +158,15 @@ lm3633@36 {
 		main {
 			label = "main_lcd";
 			led-sources = <1 2>;
-			ramp-up-msec = <500>;
-			ramp-down-msec = <500>;
+			ramp-up-ms = <500>;
+			ramp-down-ms = <500>;
 		};
 
 		front {
 			label = "front_lcd";
 			led-sources = <0>;
-			ramp-up-msec = <1000>;
-			ramp-down-msec = <0>;
+			ramp-up-ms = <1000>;
+			ramp-down-ms = <0>;
 		};
 	};
 
@@ -212,8 +218,8 @@ lm3697@36 {
 
 		lcd {
 			led-sources = <0 1 2>;
-			ramp-up-msec = <200>;
-			ramp-down-msec = <200>;
+			ramp-up-ms = <200>;
+			ramp-down-ms = <200>;
 		};
 	};
 
-- 
2.19.0

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 1/5] dt-bindings: mfd: Update the ramp up/down property
@ 2019-03-25 14:23   ` Dan Murphy
  0 siblings, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-03-25 14:23 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel; +Cc: linux-kernel, linux-leds, Dan Murphy

Modify the ramp-up/down property and add the property description
to the binding.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 .../devicetree/bindings/mfd/ti-lmu.txt        | 20 ++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/ti-lmu.txt b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
index 980394d701a7..d9d529de6dc1 100644
--- a/Documentation/devicetree/bindings/mfd/ti-lmu.txt
+++ b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
@@ -25,6 +25,12 @@ Required properties:
 
 Optional property:
   - enable-gpios: A GPIO specifier for hardware enable pin.
+  - ramp-up-ms: Current ramping from one brightness level to
+		the a higher brightness level.
+		Range from 2048 us - 117.44 s
+  - ramp-down-ms: Current ramping from one brightness level to
+		  the a lower brightness level.
+		  Range from 2048 us - 117.44 s
 
 Required node:
   - backlight: All LMU devices have backlight child nodes.
@@ -90,7 +96,7 @@ lm3631@29 {
 
 		lcd_bl {
 			led-sources = <0 1>;
-			ramp-up-msec = <300>;
+			ramp-up-ms = <300>;
 		};
 	};
 };
@@ -152,15 +158,15 @@ lm3633@36 {
 		main {
 			label = "main_lcd";
 			led-sources = <1 2>;
-			ramp-up-msec = <500>;
-			ramp-down-msec = <500>;
+			ramp-up-ms = <500>;
+			ramp-down-ms = <500>;
 		};
 
 		front {
 			label = "front_lcd";
 			led-sources = <0>;
-			ramp-up-msec = <1000>;
-			ramp-down-msec = <0>;
+			ramp-up-ms = <1000>;
+			ramp-down-ms = <0>;
 		};
 	};
 
@@ -212,8 +218,8 @@ lm3697@36 {
 
 		lcd {
 			led-sources = <0 1 2>;
-			ramp-up-msec = <200>;
-			ramp-down-msec = <200>;
+			ramp-up-ms = <200>;
+			ramp-down-ms = <200>;
 		};
 	};
 
-- 
2.19.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 2/5] leds: TI LMU: Add common code for TI LMU devices
  2019-03-25 14:23 ` Dan Murphy
@ 2019-03-25 14:24   ` Dan Murphy
  -1 siblings, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-03-25 14:24 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel; +Cc: linux-kernel, linux-leds, Dan Murphy

Create a TI LMU common framework for TI LMU devices that share
common features.

Currently the runtime ramp and brightness setting have
been identified as common features with common register settings.

This work is derived from Milo Kims TI LMU MFD code.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 drivers/leds/Kconfig              |   8 ++
 drivers/leds/Makefile             |   1 +
 drivers/leds/ti-lmu-led-common.c  | 131 ++++++++++++++++++++++++++++++
 include/linux/ti-lmu-led-common.h |  44 ++++++++++
 4 files changed, 184 insertions(+)
 create mode 100644 drivers/leds/ti-lmu-led-common.c
 create mode 100644 include/linux/ti-lmu-led-common.h

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index da00b9ed5a5c..3ae24eaf4cde 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -776,6 +776,14 @@ config LEDS_NIC78BX
 	  To compile this driver as a module, choose M here: the module
 	  will be called leds-nic78bx.
 
+config LEDS_TI_LMU_COMMON
+	tristate "LED driver for TI LMU"
+	depends on REGMAP
+	help
+          Say Y to enable the LED driver for TI LMU devices.
+          This supports common features between the TI LM3532, LM3631, LM3632,
+	  LM3633, LM3695 and LM3697.
+
 comment "LED Triggers"
 source "drivers/leds/trigger/Kconfig"
 
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 7a8b1f55d459..85fd449c002d 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -80,6 +80,7 @@ obj-$(CONFIG_LEDS_MT6323)		+= leds-mt6323.o
 obj-$(CONFIG_LEDS_LM3692X)		+= leds-lm3692x.o
 obj-$(CONFIG_LEDS_SC27XX_BLTC)		+= leds-sc27xx-bltc.o
 obj-$(CONFIG_LEDS_LM3601X)		+= leds-lm3601x.o
+obj-$(CONFIG_LEDS_TI_LMU_COMMON)	+= ti-lmu-led-common.o
 
 # LED SPI Drivers
 obj-$(CONFIG_LEDS_CR0014114)		+= leds-cr0014114.o
diff --git a/drivers/leds/ti-lmu-led-common.c b/drivers/leds/ti-lmu-led-common.c
new file mode 100644
index 000000000000..65d44bde9050
--- /dev/null
+++ b/drivers/leds/ti-lmu-led-common.c
@@ -0,0 +1,131 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright 2015 Texas Instruments
+// Copyright 2018 Sebastian Reichel
+// Copyright 2018 Pavel Machek <pavel@ucw.cz>
+// TI LMU LED common framework, based on previous work from
+// Milo Kim <milo.kim@ti.com>
+
+#include <linux/bitops.h>
+#include <linux/err.h>
+#include <linux/of_device.h>
+
+#include <linux/ti-lmu-led-common.h>
+
+const static int ramp_table[16] = { 2, 262, 524, 1049, 2090, 4194, 8389,
+				16780, 33550, 41940, 50330, 58720,
+				67110, 83880, 100660, 117440};
+
+static int ti_lmu_common_update_brightness(struct ti_lmu_bank *lmu_bank,
+					   int brightness)
+{
+	struct regmap *regmap = lmu_bank->regmap;
+	u8 reg, val;
+	int ret;
+
+	/*
+	 * Brightness register update
+	 *
+	 * 11 bit dimming: update LSB bits and write MSB byte.
+	 *		   MSB brightness should be shifted.
+	 *  8 bit dimming: write MSB byte.
+	 */
+	if (lmu_bank->max_brightness == MAX_BRIGHTNESS_11BIT) {
+		reg = lmu_bank->lsb_brightness_reg;
+		ret = regmap_update_bits(regmap, reg,
+					 LMU_11BIT_LSB_MASK,
+					 brightness);
+		if (ret)
+			return ret;
+
+		val = brightness >> LMU_11BIT_MSB_SHIFT;
+	} else {
+		val = brightness;
+	}
+
+	reg = lmu_bank->msb_brightness_reg;
+
+	return regmap_write(regmap, reg, val);
+}
+
+int ti_lmu_common_set_brightness(struct ti_lmu_bank *lmu_bank, int brightness)
+{
+	return ti_lmu_common_update_brightness(lmu_bank, brightness);
+}
+EXPORT_SYMBOL(ti_lmu_common_set_brightness);
+
+static int ti_lmu_common_convert_ramp_to_index(unsigned int msec)
+{
+	int size = ARRAY_SIZE(ramp_table);
+	int i;
+
+	if (msec <= ramp_table[0])
+		return 0;
+
+	if (msec > ramp_table[size - 1])
+		return size - 1;
+
+	for (i = 1; i < size; i++) {
+		if (msec == ramp_table[i])
+			return i;
+
+		/* Find an approximate index by looking up the table */
+		if (msec > ramp_table[i - 1] && msec < ramp_table[i]) {
+			if (msec - ramp_table[i - 1] < ramp_table[i] - msec)
+				return i - 1;
+			else
+				return i;
+		}
+	}
+
+	return -EINVAL;
+}
+
+int ti_lmu_common_set_ramp(struct ti_lmu_bank *lmu_bank)
+{
+	struct regmap *regmap = lmu_bank->regmap;
+	u8 ramp, ramp_up, ramp_down;
+
+	if (lmu_bank->ramp_up_msec == 0 && lmu_bank->ramp_down_msec == 0) {
+		ramp_up = 0;
+		ramp_down = 0;
+	} else {
+		ramp_up = ti_lmu_common_convert_ramp_to_index(lmu_bank->ramp_up_msec);
+		ramp_down = ti_lmu_common_convert_ramp_to_index(lmu_bank->ramp_down_msec);
+	}
+
+	if (ramp_up < 0 || ramp_down < 0)
+		return -EINVAL;
+
+	ramp = (ramp_up << 4) | ramp_down;
+
+	return regmap_write(regmap, lmu_bank->runtime_ramp_reg, ramp);
+
+}
+EXPORT_SYMBOL(ti_lmu_common_set_ramp);
+
+int ti_lmu_common_get_ramp_params(struct device *dev,
+				  struct fwnode_handle *child,
+				  struct ti_lmu_bank *lmu_data)
+{
+	int ret;
+
+	ret = fwnode_property_read_u32(child, "ramp-up-ms",
+				 &lmu_data->ramp_up_msec);
+	if (ret)
+		dev_warn(dev, "ramp-up-ms property missing\n");
+
+
+	ret = fwnode_property_read_u32(child, "ramp-down-ms",
+				 &lmu_data->ramp_down_msec);
+	if (ret)
+		dev_warn(dev, "ramp-down-ms property missing\n");
+
+	return 0;
+}
+EXPORT_SYMBOL(ti_lmu_common_get_ramp_params);
+
+MODULE_DESCRIPTION("TI LMU common LED framework");
+MODULE_AUTHOR("Sebastian Reichel");
+MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("ti-lmu-led-common");
diff --git a/include/linux/ti-lmu-led-common.h b/include/linux/ti-lmu-led-common.h
new file mode 100644
index 000000000000..510c07025117
--- /dev/null
+++ b/include/linux/ti-lmu-led-common.h
@@ -0,0 +1,44 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+// TI LMU Common Core
+// Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
+
+#ifndef _TI_LMU_COMMON_H_
+#define _TI_LMU_COMMON_H_
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <uapi/linux/uleds.h>
+
+#define LMU_11BIT_LSB_MASK	(BIT(0) | BIT(1) | BIT(2))
+#define LMU_11BIT_MSB_SHIFT	3
+
+#define MAX_BRIGHTNESS_8BIT	255
+#define MAX_BRIGHTNESS_11BIT	2047
+
+struct ti_lmu_bank {
+	struct regmap *regmap;
+
+	int max_brightness;
+
+	u8 lsb_brightness_reg;
+	u8 msb_brightness_reg;
+
+	u8 runtime_ramp_reg;
+	u32 ramp_up_msec;
+	u32 ramp_down_msec;
+};
+
+int ti_lmu_common_set_brightness(struct ti_lmu_bank *lmu_bank, int brightness);
+
+int ti_lmu_common_set_ramp(struct ti_lmu_bank *lmu_bank);
+
+int ti_lmu_common_get_ramp_params(struct device *dev,
+				  struct fwnode_handle *child,
+				  struct ti_lmu_bank *lmu_data);
+
+#endif /* _TI_LMU_COMMON_H_ */
-- 
2.19.0

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 2/5] leds: TI LMU: Add common code for TI LMU devices
@ 2019-03-25 14:24   ` Dan Murphy
  0 siblings, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-03-25 14:24 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel; +Cc: linux-kernel, linux-leds, Dan Murphy

Create a TI LMU common framework for TI LMU devices that share
common features.

Currently the runtime ramp and brightness setting have
been identified as common features with common register settings.

This work is derived from Milo Kims TI LMU MFD code.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 drivers/leds/Kconfig              |   8 ++
 drivers/leds/Makefile             |   1 +
 drivers/leds/ti-lmu-led-common.c  | 131 ++++++++++++++++++++++++++++++
 include/linux/ti-lmu-led-common.h |  44 ++++++++++
 4 files changed, 184 insertions(+)
 create mode 100644 drivers/leds/ti-lmu-led-common.c
 create mode 100644 include/linux/ti-lmu-led-common.h

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index da00b9ed5a5c..3ae24eaf4cde 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -776,6 +776,14 @@ config LEDS_NIC78BX
 	  To compile this driver as a module, choose M here: the module
 	  will be called leds-nic78bx.
 
+config LEDS_TI_LMU_COMMON
+	tristate "LED driver for TI LMU"
+	depends on REGMAP
+	help
+          Say Y to enable the LED driver for TI LMU devices.
+          This supports common features between the TI LM3532, LM3631, LM3632,
+	  LM3633, LM3695 and LM3697.
+
 comment "LED Triggers"
 source "drivers/leds/trigger/Kconfig"
 
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 7a8b1f55d459..85fd449c002d 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -80,6 +80,7 @@ obj-$(CONFIG_LEDS_MT6323)		+= leds-mt6323.o
 obj-$(CONFIG_LEDS_LM3692X)		+= leds-lm3692x.o
 obj-$(CONFIG_LEDS_SC27XX_BLTC)		+= leds-sc27xx-bltc.o
 obj-$(CONFIG_LEDS_LM3601X)		+= leds-lm3601x.o
+obj-$(CONFIG_LEDS_TI_LMU_COMMON)	+= ti-lmu-led-common.o
 
 # LED SPI Drivers
 obj-$(CONFIG_LEDS_CR0014114)		+= leds-cr0014114.o
diff --git a/drivers/leds/ti-lmu-led-common.c b/drivers/leds/ti-lmu-led-common.c
new file mode 100644
index 000000000000..65d44bde9050
--- /dev/null
+++ b/drivers/leds/ti-lmu-led-common.c
@@ -0,0 +1,131 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright 2015 Texas Instruments
+// Copyright 2018 Sebastian Reichel
+// Copyright 2018 Pavel Machek <pavel@ucw.cz>
+// TI LMU LED common framework, based on previous work from
+// Milo Kim <milo.kim@ti.com>
+
+#include <linux/bitops.h>
+#include <linux/err.h>
+#include <linux/of_device.h>
+
+#include <linux/ti-lmu-led-common.h>
+
+const static int ramp_table[16] = { 2, 262, 524, 1049, 2090, 4194, 8389,
+				16780, 33550, 41940, 50330, 58720,
+				67110, 83880, 100660, 117440};
+
+static int ti_lmu_common_update_brightness(struct ti_lmu_bank *lmu_bank,
+					   int brightness)
+{
+	struct regmap *regmap = lmu_bank->regmap;
+	u8 reg, val;
+	int ret;
+
+	/*
+	 * Brightness register update
+	 *
+	 * 11 bit dimming: update LSB bits and write MSB byte.
+	 *		   MSB brightness should be shifted.
+	 *  8 bit dimming: write MSB byte.
+	 */
+	if (lmu_bank->max_brightness == MAX_BRIGHTNESS_11BIT) {
+		reg = lmu_bank->lsb_brightness_reg;
+		ret = regmap_update_bits(regmap, reg,
+					 LMU_11BIT_LSB_MASK,
+					 brightness);
+		if (ret)
+			return ret;
+
+		val = brightness >> LMU_11BIT_MSB_SHIFT;
+	} else {
+		val = brightness;
+	}
+
+	reg = lmu_bank->msb_brightness_reg;
+
+	return regmap_write(regmap, reg, val);
+}
+
+int ti_lmu_common_set_brightness(struct ti_lmu_bank *lmu_bank, int brightness)
+{
+	return ti_lmu_common_update_brightness(lmu_bank, brightness);
+}
+EXPORT_SYMBOL(ti_lmu_common_set_brightness);
+
+static int ti_lmu_common_convert_ramp_to_index(unsigned int msec)
+{
+	int size = ARRAY_SIZE(ramp_table);
+	int i;
+
+	if (msec <= ramp_table[0])
+		return 0;
+
+	if (msec > ramp_table[size - 1])
+		return size - 1;
+
+	for (i = 1; i < size; i++) {
+		if (msec == ramp_table[i])
+			return i;
+
+		/* Find an approximate index by looking up the table */
+		if (msec > ramp_table[i - 1] && msec < ramp_table[i]) {
+			if (msec - ramp_table[i - 1] < ramp_table[i] - msec)
+				return i - 1;
+			else
+				return i;
+		}
+	}
+
+	return -EINVAL;
+}
+
+int ti_lmu_common_set_ramp(struct ti_lmu_bank *lmu_bank)
+{
+	struct regmap *regmap = lmu_bank->regmap;
+	u8 ramp, ramp_up, ramp_down;
+
+	if (lmu_bank->ramp_up_msec == 0 && lmu_bank->ramp_down_msec == 0) {
+		ramp_up = 0;
+		ramp_down = 0;
+	} else {
+		ramp_up = ti_lmu_common_convert_ramp_to_index(lmu_bank->ramp_up_msec);
+		ramp_down = ti_lmu_common_convert_ramp_to_index(lmu_bank->ramp_down_msec);
+	}
+
+	if (ramp_up < 0 || ramp_down < 0)
+		return -EINVAL;
+
+	ramp = (ramp_up << 4) | ramp_down;
+
+	return regmap_write(regmap, lmu_bank->runtime_ramp_reg, ramp);
+
+}
+EXPORT_SYMBOL(ti_lmu_common_set_ramp);
+
+int ti_lmu_common_get_ramp_params(struct device *dev,
+				  struct fwnode_handle *child,
+				  struct ti_lmu_bank *lmu_data)
+{
+	int ret;
+
+	ret = fwnode_property_read_u32(child, "ramp-up-ms",
+				 &lmu_data->ramp_up_msec);
+	if (ret)
+		dev_warn(dev, "ramp-up-ms property missing\n");
+
+
+	ret = fwnode_property_read_u32(child, "ramp-down-ms",
+				 &lmu_data->ramp_down_msec);
+	if (ret)
+		dev_warn(dev, "ramp-down-ms property missing\n");
+
+	return 0;
+}
+EXPORT_SYMBOL(ti_lmu_common_get_ramp_params);
+
+MODULE_DESCRIPTION("TI LMU common LED framework");
+MODULE_AUTHOR("Sebastian Reichel");
+MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("ti-lmu-led-common");
diff --git a/include/linux/ti-lmu-led-common.h b/include/linux/ti-lmu-led-common.h
new file mode 100644
index 000000000000..510c07025117
--- /dev/null
+++ b/include/linux/ti-lmu-led-common.h
@@ -0,0 +1,44 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+// TI LMU Common Core
+// Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
+
+#ifndef _TI_LMU_COMMON_H_
+#define _TI_LMU_COMMON_H_
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <uapi/linux/uleds.h>
+
+#define LMU_11BIT_LSB_MASK	(BIT(0) | BIT(1) | BIT(2))
+#define LMU_11BIT_MSB_SHIFT	3
+
+#define MAX_BRIGHTNESS_8BIT	255
+#define MAX_BRIGHTNESS_11BIT	2047
+
+struct ti_lmu_bank {
+	struct regmap *regmap;
+
+	int max_brightness;
+
+	u8 lsb_brightness_reg;
+	u8 msb_brightness_reg;
+
+	u8 runtime_ramp_reg;
+	u32 ramp_up_msec;
+	u32 ramp_down_msec;
+};
+
+int ti_lmu_common_set_brightness(struct ti_lmu_bank *lmu_bank, int brightness);
+
+int ti_lmu_common_set_ramp(struct ti_lmu_bank *lmu_bank);
+
+int ti_lmu_common_get_ramp_params(struct device *dev,
+				  struct fwnode_handle *child,
+				  struct ti_lmu_bank *lmu_data);
+
+#endif /* _TI_LMU_COMMON_H_ */
-- 
2.19.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 3/5] dt-bindings: ti-lmu: Modify dt bindings for the LM3697
  2019-03-25 14:23 ` Dan Murphy
@ 2019-03-25 14:24   ` Dan Murphy
  -1 siblings, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-03-25 14:24 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel; +Cc: linux-kernel, linux-leds, Dan Murphy

The LM3697 is a single function LED driver. The single function LED
driver needs to reside in the LED directory as a dedicated LED driver
and not as a MFD device.  The device does have common brightness and ramp
features and those can be accomodated by a TI LMU framework.

The LM3697 dt binding needs to be moved from the ti-lmu.txt and a dedicated
LED dt binding needs to be added.  The new LM3697 LED dt binding will then
reside in the Documentation/devicetree/bindings/leds directory and follow the
current LED and general bindings guidelines.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 .../devicetree/bindings/leds/leds-lm3697.txt  | 77 +++++++++++++++++++
 .../devicetree/bindings/mfd/ti-lmu.txt        | 26 +------
 2 files changed, 78 insertions(+), 25 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3697.txt

diff --git a/Documentation/devicetree/bindings/leds/leds-lm3697.txt b/Documentation/devicetree/bindings/leds/leds-lm3697.txt
new file mode 100644
index 000000000000..a780f11acd38
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-lm3697.txt
@@ -0,0 +1,77 @@
+* Texas Instruments - LM3697 Highly Efficient White LED Driver
+
+The LM3697 11-bit LED driver provides high-
+performance backlight dimming for 1, 2, or 3 series
+LED strings while delivering up to 90% efficiency.
+
+This device is suitable for display and keypad Lighting
+
+Required properties:
+	- compatible:
+		"ti,lm3697"
+	- reg :  I2C slave address
+	- #address-cells : 1
+	- #size-cells : 0
+
+Optional properties:
+	- enable-gpios : GPIO pin to enable/disable the device
+	- vled-supply : LED supply
+
+Required child properties:
+	- reg : 0 - LED is Controlled by bank A
+		1 - LED is Controlled by bank B
+	- led-sources : Indicates which HVLED string is associated to which
+			control bank.  This is a zero based property so
+			HVLED1 = 0, HVLED2 = 1, HVLED3 = 2.
+			Additional information is contained
+			in Documentation/devicetree/bindings/leds/common.txt
+
+Optional child properties:
+	- max_brightness - This determines whether to use 8 bit brightness mode
+			   or 11 bit brightness mode.  If this value is not
+			   set the device is defaulted to the preferred 8bit
+			   brightness mode per 7.3.4.1 of the data sheet.
+			   The values are 255 (8bit) or 2047 (11bit).
+	- ramp-up-ms: see Documentation/devicetree/bindings/mfd/ti-lmu.txt
+	- ramp-down-ms: see Documentation/devicetree/bindings/mfd/ti-lmu.txt
+	- label : see Documentation/devicetree/bindings/leds/common.txt
+	- linux,default-trigger :
+	   see Documentation/devicetree/bindings/leds/common.txt
+
+Example:
+
+HVLED string 1 and 3 are controlled by control bank A and HVLED 2 string is
+controlled by control bank B.
+
+led-controller@36 {
+	compatible = "ti,lm3697";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	reg = <0x36>;
+
+	enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
+	vled-supply = <&vbatt>;
+
+	led@0 {
+		reg = <0>;
+		led-sources = <0 2>;
+		max_brightness = <2047>;
+		ramp-up-ms = <5000>;
+		ramp-down-ms = <1000>;
+		label = "white:first_backlight_cluster";
+		linux,default-trigger = "backlight";
+	};
+
+	led@1 {
+		reg = <1>;
+		led-sources = <1>;
+		max_brightness = <255>;
+		ramp-up-ms = <500>;
+		ramp-down-ms = <1000>;
+		label = "white:second_backlight_cluster";
+		linux,default-trigger = "backlight";
+	};
+}
+
+For more product information please see the link below:
+http://www.ti.com/lit/ds/symlink/lm3697.pdf
diff --git a/Documentation/devicetree/bindings/mfd/ti-lmu.txt b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
index d9d529de6dc1..4b2dc0ccfe26 100644
--- a/Documentation/devicetree/bindings/mfd/ti-lmu.txt
+++ b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
@@ -8,7 +8,6 @@ TI LMU driver supports lighting devices below.
   LM3632       Backlight and regulator
   LM3633       Backlight, LED and fault monitor
   LM3695       Backlight
-  LM3697       Backlight and fault monitor
 
 Required properties:
   - compatible: Should be one of:
@@ -16,11 +15,10 @@ Required properties:
                 "ti,lm3632"
                 "ti,lm3633"
                 "ti,lm3695"
-                "ti,lm3697"
   - reg: I2C slave address.
          0x11 for LM3632
          0x29 for LM3631
-         0x36 for LM3633, LM3697
+         0x36 for LM3633
          0x63 for LM3695
 
 Optional property:
@@ -41,7 +39,6 @@ Optional nodes:
     Required properties:
       - compatible: Should be one of:
                     "ti,lm3633-fault-monitor"
-                    "ti,lm3697-fault-monitor"
   - leds: LED properties for LM3633. Please refer to [2].
   - regulators: Regulator properties for LM3631 and LM3632.
                 Please refer to [3].
@@ -206,24 +203,3 @@ lm3695@63 {
 		};
 	};
 };
-
-lm3697@36 {
-	compatible = "ti,lm3697";
-	reg = <0x36>;
-
-	enable-gpios = <&pioC 2 GPIO_ACTIVE_HIGH>;
-
-	backlight {
-		compatible = "ti,lm3697-backlight";
-
-		lcd {
-			led-sources = <0 1 2>;
-			ramp-up-ms = <200>;
-			ramp-down-ms = <200>;
-		};
-	};
-
-	fault-monitor {
-		compatible = "ti,lm3697-fault-monitor";
-	};
-};
-- 
2.19.0

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 3/5] dt-bindings: ti-lmu: Modify dt bindings for the LM3697
@ 2019-03-25 14:24   ` Dan Murphy
  0 siblings, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-03-25 14:24 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel; +Cc: linux-kernel, linux-leds, Dan Murphy

The LM3697 is a single function LED driver. The single function LED
driver needs to reside in the LED directory as a dedicated LED driver
and not as a MFD device.  The device does have common brightness and ramp
features and those can be accomodated by a TI LMU framework.

The LM3697 dt binding needs to be moved from the ti-lmu.txt and a dedicated
LED dt binding needs to be added.  The new LM3697 LED dt binding will then
reside in the Documentation/devicetree/bindings/leds directory and follow the
current LED and general bindings guidelines.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 .../devicetree/bindings/leds/leds-lm3697.txt  | 77 +++++++++++++++++++
 .../devicetree/bindings/mfd/ti-lmu.txt        | 26 +------
 2 files changed, 78 insertions(+), 25 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3697.txt

diff --git a/Documentation/devicetree/bindings/leds/leds-lm3697.txt b/Documentation/devicetree/bindings/leds/leds-lm3697.txt
new file mode 100644
index 000000000000..a780f11acd38
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-lm3697.txt
@@ -0,0 +1,77 @@
+* Texas Instruments - LM3697 Highly Efficient White LED Driver
+
+The LM3697 11-bit LED driver provides high-
+performance backlight dimming for 1, 2, or 3 series
+LED strings while delivering up to 90% efficiency.
+
+This device is suitable for display and keypad Lighting
+
+Required properties:
+	- compatible:
+		"ti,lm3697"
+	- reg :  I2C slave address
+	- #address-cells : 1
+	- #size-cells : 0
+
+Optional properties:
+	- enable-gpios : GPIO pin to enable/disable the device
+	- vled-supply : LED supply
+
+Required child properties:
+	- reg : 0 - LED is Controlled by bank A
+		1 - LED is Controlled by bank B
+	- led-sources : Indicates which HVLED string is associated to which
+			control bank.  This is a zero based property so
+			HVLED1 = 0, HVLED2 = 1, HVLED3 = 2.
+			Additional information is contained
+			in Documentation/devicetree/bindings/leds/common.txt
+
+Optional child properties:
+	- max_brightness - This determines whether to use 8 bit brightness mode
+			   or 11 bit brightness mode.  If this value is not
+			   set the device is defaulted to the preferred 8bit
+			   brightness mode per 7.3.4.1 of the data sheet.
+			   The values are 255 (8bit) or 2047 (11bit).
+	- ramp-up-ms: see Documentation/devicetree/bindings/mfd/ti-lmu.txt
+	- ramp-down-ms: see Documentation/devicetree/bindings/mfd/ti-lmu.txt
+	- label : see Documentation/devicetree/bindings/leds/common.txt
+	- linux,default-trigger :
+	   see Documentation/devicetree/bindings/leds/common.txt
+
+Example:
+
+HVLED string 1 and 3 are controlled by control bank A and HVLED 2 string is
+controlled by control bank B.
+
+led-controller@36 {
+	compatible = "ti,lm3697";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	reg = <0x36>;
+
+	enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
+	vled-supply = <&vbatt>;
+
+	led@0 {
+		reg = <0>;
+		led-sources = <0 2>;
+		max_brightness = <2047>;
+		ramp-up-ms = <5000>;
+		ramp-down-ms = <1000>;
+		label = "white:first_backlight_cluster";
+		linux,default-trigger = "backlight";
+	};
+
+	led@1 {
+		reg = <1>;
+		led-sources = <1>;
+		max_brightness = <255>;
+		ramp-up-ms = <500>;
+		ramp-down-ms = <1000>;
+		label = "white:second_backlight_cluster";
+		linux,default-trigger = "backlight";
+	};
+}
+
+For more product information please see the link below:
+http://www.ti.com/lit/ds/symlink/lm3697.pdf
diff --git a/Documentation/devicetree/bindings/mfd/ti-lmu.txt b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
index d9d529de6dc1..4b2dc0ccfe26 100644
--- a/Documentation/devicetree/bindings/mfd/ti-lmu.txt
+++ b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
@@ -8,7 +8,6 @@ TI LMU driver supports lighting devices below.
   LM3632       Backlight and regulator
   LM3633       Backlight, LED and fault monitor
   LM3695       Backlight
-  LM3697       Backlight and fault monitor
 
 Required properties:
   - compatible: Should be one of:
@@ -16,11 +15,10 @@ Required properties:
                 "ti,lm3632"
                 "ti,lm3633"
                 "ti,lm3695"
-                "ti,lm3697"
   - reg: I2C slave address.
          0x11 for LM3632
          0x29 for LM3631
-         0x36 for LM3633, LM3697
+         0x36 for LM3633
          0x63 for LM3695
 
 Optional property:
@@ -41,7 +39,6 @@ Optional nodes:
     Required properties:
       - compatible: Should be one of:
                     "ti,lm3633-fault-monitor"
-                    "ti,lm3697-fault-monitor"
   - leds: LED properties for LM3633. Please refer to [2].
   - regulators: Regulator properties for LM3631 and LM3632.
                 Please refer to [3].
@@ -206,24 +203,3 @@ lm3695@63 {
 		};
 	};
 };
-
-lm3697@36 {
-	compatible = "ti,lm3697";
-	reg = <0x36>;
-
-	enable-gpios = <&pioC 2 GPIO_ACTIVE_HIGH>;
-
-	backlight {
-		compatible = "ti,lm3697-backlight";
-
-		lcd {
-			led-sources = <0 1 2>;
-			ramp-up-ms = <200>;
-			ramp-down-ms = <200>;
-		};
-	};
-
-	fault-monitor {
-		compatible = "ti,lm3697-fault-monitor";
-	};
-};
-- 
2.19.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 4/5] mfd: ti-lmu: Remove support for LM3697
  2019-03-25 14:23 ` Dan Murphy
@ 2019-03-25 14:24   ` Dan Murphy
  -1 siblings, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-03-25 14:24 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel; +Cc: linux-kernel, linux-leds, Dan Murphy

Remove support for the LM3697 from the ti-lmu
driver in favor of a dedicated LED driver.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 drivers/leds/Kconfig                |  2 +-
 drivers/mfd/Kconfig                 |  2 +-
 drivers/mfd/ti-lmu.c                | 17 -----------
 include/linux/mfd/ti-lmu-register.h | 44 -----------------------------
 include/linux/mfd/ti-lmu.h          |  1 -
 5 files changed, 2 insertions(+), 64 deletions(-)

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 3ae24eaf4cde..735009e73414 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -782,7 +782,7 @@ config LEDS_TI_LMU_COMMON
 	help
           Say Y to enable the LED driver for TI LMU devices.
           This supports common features between the TI LM3532, LM3631, LM3632,
-	  LM3633, LM3695 and LM3697.
+	  LM3633, and LM3695.
 
 comment "LED Triggers"
 source "drivers/leds/trigger/Kconfig"
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 0ce2d8dfc5f1..fa70ea209ec6 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1312,7 +1312,7 @@ config MFD_TI_LMU
 	help
 	  Say yes here to enable support for TI LMU chips.
 
-	  TI LMU MFD supports LM3532, LM3631, LM3632, LM3633, LM3695 and LM3697.
+	  TI LMU MFD supports LM3532, LM3631, LM3632, LM3633, and LM3695.
 	  It consists of backlight, LED and regulator driver.
 	  It provides consistent device controls for lighting functions.
 
diff --git a/drivers/mfd/ti-lmu.c b/drivers/mfd/ti-lmu.c
index b06cb908d1aa..89b1c5b584af 100644
--- a/drivers/mfd/ti-lmu.c
+++ b/drivers/mfd/ti-lmu.c
@@ -111,20 +111,6 @@ static const struct mfd_cell lm3695_devices[] = {
 	},
 };
 
-static const struct mfd_cell lm3697_devices[] = {
-	{
-		.name          = "ti-lmu-backlight",
-		.id            = LM3697,
-		.of_compatible = "ti,lm3697-backlight",
-	},
-	/* Monitoring driver for open/short circuit detection */
-	{
-		.name          = "ti-lmu-fault-monitor",
-		.id            = LM3697,
-		.of_compatible = "ti,lm3697-fault-monitor",
-	},
-};
-
 #define TI_LMU_DATA(chip, max_reg)		\
 static const struct ti_lmu_data chip##_data =	\
 {						\
@@ -137,7 +123,6 @@ TI_LMU_DATA(lm3631, LM3631_MAX_REG);
 TI_LMU_DATA(lm3632, LM3632_MAX_REG);
 TI_LMU_DATA(lm3633, LM3633_MAX_REG);
 TI_LMU_DATA(lm3695, LM3695_MAX_REG);
-TI_LMU_DATA(lm3697, LM3697_MAX_REG);
 
 static int ti_lmu_probe(struct i2c_client *cl, const struct i2c_device_id *id)
 {
@@ -206,7 +191,6 @@ static const struct of_device_id ti_lmu_of_match[] = {
 	{ .compatible = "ti,lm3632", .data = &lm3632_data },
 	{ .compatible = "ti,lm3633", .data = &lm3633_data },
 	{ .compatible = "ti,lm3695", .data = &lm3695_data },
-	{ .compatible = "ti,lm3697", .data = &lm3697_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ti_lmu_of_match);
@@ -216,7 +200,6 @@ static const struct i2c_device_id ti_lmu_ids[] = {
 	{ "lm3632", LM3632 },
 	{ "lm3633", LM3633 },
 	{ "lm3695", LM3695 },
-	{ "lm3697", LM3697 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, ti_lmu_ids);
diff --git a/include/linux/mfd/ti-lmu-register.h b/include/linux/mfd/ti-lmu-register.h
index f09510561a55..76998b01764b 100644
--- a/include/linux/mfd/ti-lmu-register.h
+++ b/include/linux/mfd/ti-lmu-register.h
@@ -189,48 +189,4 @@
 #define LM3695_REG_BRT_MSB			0x14
 
 #define LM3695_MAX_REG				0x14
-
-/* LM3697 */
-#define LM3697_REG_HVLED_OUTPUT_CFG		0x10
-#define LM3697_HVLED1_CFG_MASK			BIT(0)
-#define LM3697_HVLED2_CFG_MASK			BIT(1)
-#define LM3697_HVLED3_CFG_MASK			BIT(2)
-#define LM3697_HVLED1_CFG_SHIFT			0
-#define LM3697_HVLED2_CFG_SHIFT			1
-#define LM3697_HVLED3_CFG_SHIFT			2
-
-#define LM3697_REG_BL0_RAMP			0x11
-#define LM3697_REG_BL1_RAMP			0x12
-#define LM3697_RAMPUP_MASK			0xF0
-#define LM3697_RAMPUP_SHIFT			4
-#define LM3697_RAMPDN_MASK			0x0F
-#define LM3697_RAMPDN_SHIFT			0
-
-#define LM3697_REG_RAMP_CONF			0x14
-#define LM3697_RAMP_MASK			0x0F
-#define LM3697_RAMP_EACH			0x05
-
-#define LM3697_REG_PWM_CFG			0x1C
-#define LM3697_PWM_A_MASK			BIT(0)
-#define LM3697_PWM_B_MASK			BIT(1)
-
-#define LM3697_REG_IMAX_A			0x17
-#define LM3697_REG_IMAX_B			0x18
-
-#define LM3697_REG_FEEDBACK_ENABLE		0x19
-
-#define LM3697_REG_BRT_A_LSB			0x20
-#define LM3697_REG_BRT_A_MSB			0x21
-#define LM3697_REG_BRT_B_LSB			0x22
-#define LM3697_REG_BRT_B_MSB			0x23
-
-#define LM3697_REG_ENABLE			0x24
-
-#define LM3697_REG_OPEN_FAULT_STATUS		0xB0
-
-#define LM3697_REG_SHORT_FAULT_STATUS		0xB2
-
-#define LM3697_REG_MONITOR_ENABLE		0xB4
-
-#define LM3697_MAX_REG				0xB4
 #endif
diff --git a/include/linux/mfd/ti-lmu.h b/include/linux/mfd/ti-lmu.h
index 7762c1bce55d..54e9d272e81c 100644
--- a/include/linux/mfd/ti-lmu.h
+++ b/include/linux/mfd/ti-lmu.h
@@ -26,7 +26,6 @@ enum ti_lmu_id {
 	LM3632,
 	LM3633,
 	LM3695,
-	LM3697,
 	LMU_MAX_ID,
 };
 
-- 
2.19.0

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 4/5] mfd: ti-lmu: Remove support for LM3697
@ 2019-03-25 14:24   ` Dan Murphy
  0 siblings, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-03-25 14:24 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel; +Cc: linux-kernel, linux-leds, Dan Murphy

Remove support for the LM3697 from the ti-lmu
driver in favor of a dedicated LED driver.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 drivers/leds/Kconfig                |  2 +-
 drivers/mfd/Kconfig                 |  2 +-
 drivers/mfd/ti-lmu.c                | 17 -----------
 include/linux/mfd/ti-lmu-register.h | 44 -----------------------------
 include/linux/mfd/ti-lmu.h          |  1 -
 5 files changed, 2 insertions(+), 64 deletions(-)

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 3ae24eaf4cde..735009e73414 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -782,7 +782,7 @@ config LEDS_TI_LMU_COMMON
 	help
           Say Y to enable the LED driver for TI LMU devices.
           This supports common features between the TI LM3532, LM3631, LM3632,
-	  LM3633, LM3695 and LM3697.
+	  LM3633, and LM3695.
 
 comment "LED Triggers"
 source "drivers/leds/trigger/Kconfig"
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 0ce2d8dfc5f1..fa70ea209ec6 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1312,7 +1312,7 @@ config MFD_TI_LMU
 	help
 	  Say yes here to enable support for TI LMU chips.
 
-	  TI LMU MFD supports LM3532, LM3631, LM3632, LM3633, LM3695 and LM3697.
+	  TI LMU MFD supports LM3532, LM3631, LM3632, LM3633, and LM3695.
 	  It consists of backlight, LED and regulator driver.
 	  It provides consistent device controls for lighting functions.
 
diff --git a/drivers/mfd/ti-lmu.c b/drivers/mfd/ti-lmu.c
index b06cb908d1aa..89b1c5b584af 100644
--- a/drivers/mfd/ti-lmu.c
+++ b/drivers/mfd/ti-lmu.c
@@ -111,20 +111,6 @@ static const struct mfd_cell lm3695_devices[] = {
 	},
 };
 
-static const struct mfd_cell lm3697_devices[] = {
-	{
-		.name          = "ti-lmu-backlight",
-		.id            = LM3697,
-		.of_compatible = "ti,lm3697-backlight",
-	},
-	/* Monitoring driver for open/short circuit detection */
-	{
-		.name          = "ti-lmu-fault-monitor",
-		.id            = LM3697,
-		.of_compatible = "ti,lm3697-fault-monitor",
-	},
-};
-
 #define TI_LMU_DATA(chip, max_reg)		\
 static const struct ti_lmu_data chip##_data =	\
 {						\
@@ -137,7 +123,6 @@ TI_LMU_DATA(lm3631, LM3631_MAX_REG);
 TI_LMU_DATA(lm3632, LM3632_MAX_REG);
 TI_LMU_DATA(lm3633, LM3633_MAX_REG);
 TI_LMU_DATA(lm3695, LM3695_MAX_REG);
-TI_LMU_DATA(lm3697, LM3697_MAX_REG);
 
 static int ti_lmu_probe(struct i2c_client *cl, const struct i2c_device_id *id)
 {
@@ -206,7 +191,6 @@ static const struct of_device_id ti_lmu_of_match[] = {
 	{ .compatible = "ti,lm3632", .data = &lm3632_data },
 	{ .compatible = "ti,lm3633", .data = &lm3633_data },
 	{ .compatible = "ti,lm3695", .data = &lm3695_data },
-	{ .compatible = "ti,lm3697", .data = &lm3697_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ti_lmu_of_match);
@@ -216,7 +200,6 @@ static const struct i2c_device_id ti_lmu_ids[] = {
 	{ "lm3632", LM3632 },
 	{ "lm3633", LM3633 },
 	{ "lm3695", LM3695 },
-	{ "lm3697", LM3697 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, ti_lmu_ids);
diff --git a/include/linux/mfd/ti-lmu-register.h b/include/linux/mfd/ti-lmu-register.h
index f09510561a55..76998b01764b 100644
--- a/include/linux/mfd/ti-lmu-register.h
+++ b/include/linux/mfd/ti-lmu-register.h
@@ -189,48 +189,4 @@
 #define LM3695_REG_BRT_MSB			0x14
 
 #define LM3695_MAX_REG				0x14
-
-/* LM3697 */
-#define LM3697_REG_HVLED_OUTPUT_CFG		0x10
-#define LM3697_HVLED1_CFG_MASK			BIT(0)
-#define LM3697_HVLED2_CFG_MASK			BIT(1)
-#define LM3697_HVLED3_CFG_MASK			BIT(2)
-#define LM3697_HVLED1_CFG_SHIFT			0
-#define LM3697_HVLED2_CFG_SHIFT			1
-#define LM3697_HVLED3_CFG_SHIFT			2
-
-#define LM3697_REG_BL0_RAMP			0x11
-#define LM3697_REG_BL1_RAMP			0x12
-#define LM3697_RAMPUP_MASK			0xF0
-#define LM3697_RAMPUP_SHIFT			4
-#define LM3697_RAMPDN_MASK			0x0F
-#define LM3697_RAMPDN_SHIFT			0
-
-#define LM3697_REG_RAMP_CONF			0x14
-#define LM3697_RAMP_MASK			0x0F
-#define LM3697_RAMP_EACH			0x05
-
-#define LM3697_REG_PWM_CFG			0x1C
-#define LM3697_PWM_A_MASK			BIT(0)
-#define LM3697_PWM_B_MASK			BIT(1)
-
-#define LM3697_REG_IMAX_A			0x17
-#define LM3697_REG_IMAX_B			0x18
-
-#define LM3697_REG_FEEDBACK_ENABLE		0x19
-
-#define LM3697_REG_BRT_A_LSB			0x20
-#define LM3697_REG_BRT_A_MSB			0x21
-#define LM3697_REG_BRT_B_LSB			0x22
-#define LM3697_REG_BRT_B_MSB			0x23
-
-#define LM3697_REG_ENABLE			0x24
-
-#define LM3697_REG_OPEN_FAULT_STATUS		0xB0
-
-#define LM3697_REG_SHORT_FAULT_STATUS		0xB2
-
-#define LM3697_REG_MONITOR_ENABLE		0xB4
-
-#define LM3697_MAX_REG				0xB4
 #endif
diff --git a/include/linux/mfd/ti-lmu.h b/include/linux/mfd/ti-lmu.h
index 7762c1bce55d..54e9d272e81c 100644
--- a/include/linux/mfd/ti-lmu.h
+++ b/include/linux/mfd/ti-lmu.h
@@ -26,7 +26,6 @@ enum ti_lmu_id {
 	LM3632,
 	LM3633,
 	LM3695,
-	LM3697,
 	LMU_MAX_ID,
 };
 
-- 
2.19.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 5/5] leds: lm3697: Introduce the lm3697 driver
  2019-03-25 14:23 ` Dan Murphy
@ 2019-03-25 14:24   ` Dan Murphy
  -1 siblings, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-03-25 14:24 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel; +Cc: linux-kernel, linux-leds, Dan Murphy

Introduce the lm3697 LED driver for
backlighting and display.

Datasheet location:
http://www.ti.com/lit/ds/symlink/lm3697.pdf

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 drivers/leds/Kconfig       |   8 +-
 drivers/leds/Makefile      |   1 +
 drivers/leds/leds-lm3697.c | 401 +++++++++++++++++++++++++++++++++++++
 3 files changed, 409 insertions(+), 1 deletion(-)
 create mode 100644 drivers/leds/leds-lm3697.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 735009e73414..688bb9a6f275 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -776,9 +776,15 @@ config LEDS_NIC78BX
 	  To compile this driver as a module, choose M here: the module
 	  will be called leds-nic78bx.
 
+config LEDS_LM3697
+	tristate "LED driver for LM3697"
+	depends on LEDS_TI_LMU_COMMON
+	help
+	  Say Y to enable the LM3697 LED driver for TI LMU devices.
+	  This supports the LED device LM3697.
+
 config LEDS_TI_LMU_COMMON
 	tristate "LED driver for TI LMU"
-	depends on REGMAP
 	help
           Say Y to enable the LED driver for TI LMU devices.
           This supports common features between the TI LM3532, LM3631, LM3632,
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 85fd449c002d..2eb0225d8dc6 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -80,6 +80,7 @@ obj-$(CONFIG_LEDS_MT6323)		+= leds-mt6323.o
 obj-$(CONFIG_LEDS_LM3692X)		+= leds-lm3692x.o
 obj-$(CONFIG_LEDS_SC27XX_BLTC)		+= leds-sc27xx-bltc.o
 obj-$(CONFIG_LEDS_LM3601X)		+= leds-lm3601x.o
+obj-$(CONFIG_LEDS_LM3697)		+= leds-lm3697.o
 obj-$(CONFIG_LEDS_TI_LMU_COMMON)	+= ti-lmu-led-common.o
 
 # LED SPI Drivers
diff --git a/drivers/leds/leds-lm3697.c b/drivers/leds/leds-lm3697.c
new file mode 100644
index 000000000000..0f3df7e17cbc
--- /dev/null
+++ b/drivers/leds/leds-lm3697.c
@@ -0,0 +1,401 @@
+// SPDX-License-Identifier: GPL-2.0
+// TI LM3697 LED chip family driver
+// Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
+
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/regulator/consumer.h>
+#include <linux/ti-lmu-led-common.h>
+
+#define LM3697_REV			0x0
+#define LM3697_RESET			0x1
+#define LM3697_OUTPUT_CONFIG		0x10
+#define LM3697_CTRL_A_RAMP		0x11
+#define LM3697_CTRL_B_RAMP		0x12
+#define LM3697_CTRL_A_B_RT_RAMP		0x13
+#define LM3697_CTRL_A_B_RAMP_CFG	0x14
+#define LM3697_CTRL_A_B_BRT_CFG		0x16
+#define LM3697_CTRL_A_FS_CURR_CFG	0x17
+#define LM3697_CTRL_B_FS_CURR_CFG	0x18
+#define LM3697_PWM_CFG			0x1c
+#define LM3697_CTRL_A_BRT_LSB		0x20
+#define LM3697_CTRL_A_BRT_MSB		0x21
+#define LM3697_CTRL_B_BRT_LSB		0x22
+#define LM3697_CTRL_B_BRT_MSB		0x23
+#define LM3697_CTRL_ENABLE		0x24
+
+#define LM3697_SW_RESET		BIT(0)
+
+#define LM3697_CTRL_A_EN	BIT(0)
+#define LM3697_CTRL_B_EN	BIT(1)
+#define LM3697_CTRL_A_B_EN	(LM3697_CTRL_A_EN | LM3697_CTRL_B_EN)
+
+#define LM3697_MAX_LED_STRINGS	3
+
+#define LM3697_CONTROL_A	0
+#define LM3697_CONTROL_B	1
+#define LM3697_MAX_CONTROL_BANKS 2
+
+/**
+ * struct lm3697_led -
+ * @hvled_strings: Array of LED strings associated with a control bank
+ * @label: LED label
+ * @led_dev: LED class device
+ * @priv: Pointer to the device struct
+ * @lmu_data: Register and setting values for common code
+ * @control_bank: Control bank the LED is associated to. 0 is control bank A
+ *		   1 is control bank B
+ */
+struct lm3697_led {
+	u32 hvled_strings[LM3697_MAX_LED_STRINGS];
+	char label[LED_MAX_NAME_SIZE];
+	struct led_classdev led_dev;
+	struct lm3697 *priv;
+	struct ti_lmu_bank lmu_data;
+	int control_bank;
+	int enabled;
+	int num_leds;
+};
+
+/**
+ * struct lm3697 -
+ * @enable_gpio: Hardware enable gpio
+ * @regulator: LED supply regulator pointer
+ * @client: Pointer to the I2C client
+ * @regmap: Devices register map
+ * @dev: Pointer to the devices device struct
+ * @lock: Lock for reading/writing the device
+ * @leds: Array of LED strings
+ */
+struct lm3697 {
+	struct gpio_desc *enable_gpio;
+	struct regulator *regulator;
+	struct i2c_client *client;
+	struct regmap *regmap;
+	struct device *dev;
+	struct mutex lock;
+
+	int bank_cfg;
+
+	struct lm3697_led leds[];
+};
+
+static const struct reg_default lm3697_reg_defs[] = {
+	{LM3697_OUTPUT_CONFIG, 0x6},
+	{LM3697_CTRL_A_RAMP, 0x0},
+	{LM3697_CTRL_B_RAMP, 0x0},
+	{LM3697_CTRL_A_B_RT_RAMP, 0x0},
+	{LM3697_CTRL_A_B_RAMP_CFG, 0x0},
+	{LM3697_CTRL_A_B_BRT_CFG, 0x0},
+	{LM3697_CTRL_A_FS_CURR_CFG, 0x13},
+	{LM3697_CTRL_B_FS_CURR_CFG, 0x13},
+	{LM3697_PWM_CFG, 0xc},
+	{LM3697_CTRL_A_BRT_LSB, 0x0},
+	{LM3697_CTRL_A_BRT_MSB, 0x0},
+	{LM3697_CTRL_B_BRT_LSB, 0x0},
+	{LM3697_CTRL_B_BRT_MSB, 0x0},
+	{LM3697_CTRL_ENABLE, 0x0},
+};
+
+static const struct regmap_config lm3697_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+
+	.max_register = LM3697_CTRL_ENABLE,
+	.reg_defaults = lm3697_reg_defs,
+	.num_reg_defaults = ARRAY_SIZE(lm3697_reg_defs),
+	.cache_type = REGCACHE_FLAT,
+};
+
+static int lm3697_brightness_set(struct led_classdev *led_cdev,
+				enum led_brightness brt_val)
+{
+	struct lm3697_led *led = container_of(led_cdev, struct lm3697_led,
+					      led_dev);
+	int ctrl_en_val = (1 << led->control_bank);
+	int ret;
+
+	mutex_lock(&led->priv->lock);
+
+	if (brt_val == LED_OFF) {
+		ret = regmap_update_bits(led->priv->regmap, LM3697_CTRL_ENABLE,
+					 ctrl_en_val, ~ctrl_en_val);
+		if (ret) {
+			dev_err(&led->priv->client->dev, "Cannot write ctrl register\n");
+			goto brightness_out;
+		}
+
+		led->enabled = LED_OFF;
+	} else {
+		ret = ti_lmu_common_set_brightness(&led->lmu_data, brt_val);
+		if (ret) {
+			dev_err(&led->priv->client->dev,
+				"Cannot write brightness\n");
+			goto brightness_out;
+		}
+
+		if (!led->enabled) {
+			ret = regmap_update_bits(led->priv->regmap,
+						 LM3697_CTRL_ENABLE,
+						 ctrl_en_val, ctrl_en_val);
+			if (ret) {
+				dev_err(&led->priv->client->dev,
+					"Cannot enable the device\n");
+				goto brightness_out;
+			}
+
+			led->enabled = brt_val;
+		}
+	}
+
+brightness_out:
+	mutex_unlock(&led->priv->lock);
+	return ret;
+}
+
+static int lm3697_init(struct lm3697 *priv)
+{
+	struct lm3697_led *led;
+	int i, ret;
+
+	if (priv->enable_gpio) {
+		gpiod_direction_output(priv->enable_gpio, 1);
+	} else {
+		ret = regmap_write(priv->regmap, LM3697_RESET, LM3697_SW_RESET);
+		if (ret) {
+			dev_err(&priv->client->dev, "Cannot reset the device\n");
+			goto out;
+		}
+	}
+
+	ret = regmap_write(priv->regmap, LM3697_CTRL_ENABLE, 0x0);
+	if (ret) {
+		dev_err(&priv->client->dev, "Cannot write ctrl enable\n");
+		goto out;
+	}
+
+	ret = regmap_write(priv->regmap, LM3697_OUTPUT_CONFIG, priv->bank_cfg);
+	if (ret)
+		dev_err(&priv->client->dev, "Cannot write OUTPUT config\n");
+
+	for (i = 0; i < LM3697_MAX_CONTROL_BANKS; i++) {
+		led = &priv->leds[i];
+		ret = ti_lmu_common_set_ramp(&led->lmu_data);
+		if (ret)
+			dev_err(&priv->client->dev, "Setting the ramp rate failed\n");
+	}
+out:
+	return ret;
+}
+
+static int lm3697_probe_dt(struct lm3697 *priv)
+{
+	struct fwnode_handle *child = NULL;
+	struct lm3697_led *led;
+	const char *name;
+	u32 max_brightness;
+	int control_bank;
+	size_t i = 0;
+	int ret = -EINVAL;
+	int j;
+
+	priv->enable_gpio = devm_gpiod_get_optional(&priv->client->dev,
+						   "enable", GPIOD_OUT_LOW);
+	if (IS_ERR(priv->enable_gpio)) {
+		ret = PTR_ERR(priv->enable_gpio);
+		dev_err(&priv->client->dev, "Failed to get enable gpio: %d\n",
+			ret);
+		return ret;
+	}
+
+	priv->regulator = devm_regulator_get(&priv->client->dev, "vled");
+	if (IS_ERR(priv->regulator))
+		priv->regulator = NULL;
+
+	device_for_each_child_node(priv->dev, child) {
+		ret = fwnode_property_read_u32(child, "reg", &control_bank);
+		if (ret) {
+			dev_err(&priv->client->dev, "reg property missing\n");
+			fwnode_handle_put(child);
+			goto child_out;
+		}
+
+		if (control_bank > LM3697_CONTROL_B) {
+			dev_err(&priv->client->dev, "reg property is invalid\n");
+			ret = -EINVAL;
+			fwnode_handle_put(child);
+			goto child_out;
+		}
+
+		led = &priv->leds[i];
+
+		ret = fwnode_property_read_u32(child, "max_brightness",
+					       &max_brightness);
+		if (ret)
+			max_brightness = MAX_BRIGHTNESS_8BIT;
+
+		if (max_brightness > MAX_BRIGHTNESS_11BIT)
+			max_brightness = MAX_BRIGHTNESS_11BIT;
+
+		led->control_bank = control_bank;
+		led->lmu_data.regmap = priv->regmap;
+		led->lmu_data.max_brightness = max_brightness;
+		led->lmu_data.runtime_ramp_reg = LM3697_CTRL_A_RAMP +
+						 control_bank;
+		led->lmu_data.msb_brightness_reg = LM3697_CTRL_A_BRT_MSB +
+						   led->control_bank * 2;
+		led->lmu_data.lsb_brightness_reg = LM3697_CTRL_A_BRT_LSB +
+						   led->control_bank * 2;
+
+		led->num_leds = fwnode_property_read_u32_array(child,
+						       "led-sources",
+						       NULL, 0);
+
+		if (led->num_leds > LM3697_MAX_LED_STRINGS) {
+			dev_err(&priv->client->dev, "To many LED strings defined\n");
+			continue;
+		}
+
+		ret = fwnode_property_read_u32_array(child, "led-sources",
+						    led->hvled_strings,
+						    led->num_leds);
+		if (ret) {
+			dev_err(&priv->client->dev, "led-sources property missing\n");
+			fwnode_handle_put(child);
+			goto child_out;
+		}
+
+		for (j = 0; j < led->num_leds; j++)
+			priv->bank_cfg |=
+				(led->control_bank << led->hvled_strings[j]);
+
+
+		ret = ti_lmu_common_get_ramp_params(&priv->client->dev,
+						    child, &led->lmu_data);
+		if (ret)
+			dev_warn(&priv->client->dev, "runtime-ramp properties missing\n");
+
+		fwnode_property_read_string(child, "linux,default-trigger",
+					    &led->led_dev.default_trigger);
+
+		ret = fwnode_property_read_string(child, "label", &name);
+		if (ret)
+			snprintf(led->label, sizeof(led->label),
+				"%s::", priv->client->name);
+		else
+			snprintf(led->label, sizeof(led->label),
+				 "%s:%s", priv->client->name, name);
+
+		led->priv = priv;
+		led->led_dev.name = led->label;
+		led->led_dev.max_brightness = max_brightness;
+		led->led_dev.brightness_set_blocking = lm3697_brightness_set;
+
+		ret = devm_led_classdev_register(priv->dev, &led->led_dev);
+		if (ret) {
+			dev_err(&priv->client->dev, "led register err: %d\n",
+				ret);
+			fwnode_handle_put(child);
+			goto child_out;
+		}
+
+		i++;
+	}
+
+child_out:
+	return ret;
+}
+
+static int lm3697_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct lm3697 *led;
+	int count;
+	int ret;
+
+	count = device_get_child_node_count(&client->dev);
+	if (!count) {
+		dev_err(&client->dev, "LEDs are not defined in device tree!");
+		return -ENODEV;
+	}
+
+	led = devm_kzalloc(&client->dev, struct_size(led, leds, count),
+			   GFP_KERNEL);
+	if (!led)
+		return -ENOMEM;
+
+	mutex_init(&led->lock);
+	i2c_set_clientdata(client, led);
+
+	led->client = client;
+	led->dev = &client->dev;
+	led->regmap = devm_regmap_init_i2c(client, &lm3697_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 = lm3697_probe_dt(led);
+	if (ret)
+		return ret;
+
+	return lm3697_init(led);
+}
+
+static int lm3697_remove(struct i2c_client *client)
+{
+	struct lm3697 *led = i2c_get_clientdata(client);
+	int ret;
+
+	ret = regmap_update_bits(led->regmap, LM3697_CTRL_ENABLE,
+				 LM3697_CTRL_A_B_EN, 0);
+	if (ret) {
+		dev_err(&led->client->dev, "Failed to disable the device\n");
+		return ret;
+	}
+
+	if (led->enable_gpio)
+		gpiod_direction_output(led->enable_gpio, 0);
+
+	if (led->regulator) {
+		ret = regulator_disable(led->regulator);
+		if (ret)
+			dev_err(&led->client->dev,
+				"Failed to disable regulator\n");
+	}
+
+	mutex_destroy(&led->lock);
+
+	return 0;
+}
+
+static const struct i2c_device_id lm3697_id[] = {
+	{ "lm3697", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, lm3697_id);
+
+static const struct of_device_id of_lm3697_leds_match[] = {
+	{ .compatible = "ti,lm3697", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, of_lm3697_leds_match);
+
+static struct i2c_driver lm3697_driver = {
+	.driver = {
+		.name	= "lm3697",
+		.of_match_table = of_lm3697_leds_match,
+	},
+	.probe		= lm3697_probe,
+	.remove		= lm3697_remove,
+	.id_table	= lm3697_id,
+};
+module_i2c_driver(lm3697_driver);
+
+MODULE_DESCRIPTION("Texas Instruments LM3697 LED driver");
+MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com>");
+MODULE_LICENSE("GPL v2");
-- 
2.19.0

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 5/5] leds: lm3697: Introduce the lm3697 driver
@ 2019-03-25 14:24   ` Dan Murphy
  0 siblings, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-03-25 14:24 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel; +Cc: linux-kernel, linux-leds, Dan Murphy

Introduce the lm3697 LED driver for
backlighting and display.

Datasheet location:
http://www.ti.com/lit/ds/symlink/lm3697.pdf

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 drivers/leds/Kconfig       |   8 +-
 drivers/leds/Makefile      |   1 +
 drivers/leds/leds-lm3697.c | 401 +++++++++++++++++++++++++++++++++++++
 3 files changed, 409 insertions(+), 1 deletion(-)
 create mode 100644 drivers/leds/leds-lm3697.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 735009e73414..688bb9a6f275 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -776,9 +776,15 @@ config LEDS_NIC78BX
 	  To compile this driver as a module, choose M here: the module
 	  will be called leds-nic78bx.
 
+config LEDS_LM3697
+	tristate "LED driver for LM3697"
+	depends on LEDS_TI_LMU_COMMON
+	help
+	  Say Y to enable the LM3697 LED driver for TI LMU devices.
+	  This supports the LED device LM3697.
+
 config LEDS_TI_LMU_COMMON
 	tristate "LED driver for TI LMU"
-	depends on REGMAP
 	help
           Say Y to enable the LED driver for TI LMU devices.
           This supports common features between the TI LM3532, LM3631, LM3632,
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 85fd449c002d..2eb0225d8dc6 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -80,6 +80,7 @@ obj-$(CONFIG_LEDS_MT6323)		+= leds-mt6323.o
 obj-$(CONFIG_LEDS_LM3692X)		+= leds-lm3692x.o
 obj-$(CONFIG_LEDS_SC27XX_BLTC)		+= leds-sc27xx-bltc.o
 obj-$(CONFIG_LEDS_LM3601X)		+= leds-lm3601x.o
+obj-$(CONFIG_LEDS_LM3697)		+= leds-lm3697.o
 obj-$(CONFIG_LEDS_TI_LMU_COMMON)	+= ti-lmu-led-common.o
 
 # LED SPI Drivers
diff --git a/drivers/leds/leds-lm3697.c b/drivers/leds/leds-lm3697.c
new file mode 100644
index 000000000000..0f3df7e17cbc
--- /dev/null
+++ b/drivers/leds/leds-lm3697.c
@@ -0,0 +1,401 @@
+// SPDX-License-Identifier: GPL-2.0
+// TI LM3697 LED chip family driver
+// Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
+
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/regulator/consumer.h>
+#include <linux/ti-lmu-led-common.h>
+
+#define LM3697_REV			0x0
+#define LM3697_RESET			0x1
+#define LM3697_OUTPUT_CONFIG		0x10
+#define LM3697_CTRL_A_RAMP		0x11
+#define LM3697_CTRL_B_RAMP		0x12
+#define LM3697_CTRL_A_B_RT_RAMP		0x13
+#define LM3697_CTRL_A_B_RAMP_CFG	0x14
+#define LM3697_CTRL_A_B_BRT_CFG		0x16
+#define LM3697_CTRL_A_FS_CURR_CFG	0x17
+#define LM3697_CTRL_B_FS_CURR_CFG	0x18
+#define LM3697_PWM_CFG			0x1c
+#define LM3697_CTRL_A_BRT_LSB		0x20
+#define LM3697_CTRL_A_BRT_MSB		0x21
+#define LM3697_CTRL_B_BRT_LSB		0x22
+#define LM3697_CTRL_B_BRT_MSB		0x23
+#define LM3697_CTRL_ENABLE		0x24
+
+#define LM3697_SW_RESET		BIT(0)
+
+#define LM3697_CTRL_A_EN	BIT(0)
+#define LM3697_CTRL_B_EN	BIT(1)
+#define LM3697_CTRL_A_B_EN	(LM3697_CTRL_A_EN | LM3697_CTRL_B_EN)
+
+#define LM3697_MAX_LED_STRINGS	3
+
+#define LM3697_CONTROL_A	0
+#define LM3697_CONTROL_B	1
+#define LM3697_MAX_CONTROL_BANKS 2
+
+/**
+ * struct lm3697_led -
+ * @hvled_strings: Array of LED strings associated with a control bank
+ * @label: LED label
+ * @led_dev: LED class device
+ * @priv: Pointer to the device struct
+ * @lmu_data: Register and setting values for common code
+ * @control_bank: Control bank the LED is associated to. 0 is control bank A
+ *		   1 is control bank B
+ */
+struct lm3697_led {
+	u32 hvled_strings[LM3697_MAX_LED_STRINGS];
+	char label[LED_MAX_NAME_SIZE];
+	struct led_classdev led_dev;
+	struct lm3697 *priv;
+	struct ti_lmu_bank lmu_data;
+	int control_bank;
+	int enabled;
+	int num_leds;
+};
+
+/**
+ * struct lm3697 -
+ * @enable_gpio: Hardware enable gpio
+ * @regulator: LED supply regulator pointer
+ * @client: Pointer to the I2C client
+ * @regmap: Devices register map
+ * @dev: Pointer to the devices device struct
+ * @lock: Lock for reading/writing the device
+ * @leds: Array of LED strings
+ */
+struct lm3697 {
+	struct gpio_desc *enable_gpio;
+	struct regulator *regulator;
+	struct i2c_client *client;
+	struct regmap *regmap;
+	struct device *dev;
+	struct mutex lock;
+
+	int bank_cfg;
+
+	struct lm3697_led leds[];
+};
+
+static const struct reg_default lm3697_reg_defs[] = {
+	{LM3697_OUTPUT_CONFIG, 0x6},
+	{LM3697_CTRL_A_RAMP, 0x0},
+	{LM3697_CTRL_B_RAMP, 0x0},
+	{LM3697_CTRL_A_B_RT_RAMP, 0x0},
+	{LM3697_CTRL_A_B_RAMP_CFG, 0x0},
+	{LM3697_CTRL_A_B_BRT_CFG, 0x0},
+	{LM3697_CTRL_A_FS_CURR_CFG, 0x13},
+	{LM3697_CTRL_B_FS_CURR_CFG, 0x13},
+	{LM3697_PWM_CFG, 0xc},
+	{LM3697_CTRL_A_BRT_LSB, 0x0},
+	{LM3697_CTRL_A_BRT_MSB, 0x0},
+	{LM3697_CTRL_B_BRT_LSB, 0x0},
+	{LM3697_CTRL_B_BRT_MSB, 0x0},
+	{LM3697_CTRL_ENABLE, 0x0},
+};
+
+static const struct regmap_config lm3697_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+
+	.max_register = LM3697_CTRL_ENABLE,
+	.reg_defaults = lm3697_reg_defs,
+	.num_reg_defaults = ARRAY_SIZE(lm3697_reg_defs),
+	.cache_type = REGCACHE_FLAT,
+};
+
+static int lm3697_brightness_set(struct led_classdev *led_cdev,
+				enum led_brightness brt_val)
+{
+	struct lm3697_led *led = container_of(led_cdev, struct lm3697_led,
+					      led_dev);
+	int ctrl_en_val = (1 << led->control_bank);
+	int ret;
+
+	mutex_lock(&led->priv->lock);
+
+	if (brt_val == LED_OFF) {
+		ret = regmap_update_bits(led->priv->regmap, LM3697_CTRL_ENABLE,
+					 ctrl_en_val, ~ctrl_en_val);
+		if (ret) {
+			dev_err(&led->priv->client->dev, "Cannot write ctrl register\n");
+			goto brightness_out;
+		}
+
+		led->enabled = LED_OFF;
+	} else {
+		ret = ti_lmu_common_set_brightness(&led->lmu_data, brt_val);
+		if (ret) {
+			dev_err(&led->priv->client->dev,
+				"Cannot write brightness\n");
+			goto brightness_out;
+		}
+
+		if (!led->enabled) {
+			ret = regmap_update_bits(led->priv->regmap,
+						 LM3697_CTRL_ENABLE,
+						 ctrl_en_val, ctrl_en_val);
+			if (ret) {
+				dev_err(&led->priv->client->dev,
+					"Cannot enable the device\n");
+				goto brightness_out;
+			}
+
+			led->enabled = brt_val;
+		}
+	}
+
+brightness_out:
+	mutex_unlock(&led->priv->lock);
+	return ret;
+}
+
+static int lm3697_init(struct lm3697 *priv)
+{
+	struct lm3697_led *led;
+	int i, ret;
+
+	if (priv->enable_gpio) {
+		gpiod_direction_output(priv->enable_gpio, 1);
+	} else {
+		ret = regmap_write(priv->regmap, LM3697_RESET, LM3697_SW_RESET);
+		if (ret) {
+			dev_err(&priv->client->dev, "Cannot reset the device\n");
+			goto out;
+		}
+	}
+
+	ret = regmap_write(priv->regmap, LM3697_CTRL_ENABLE, 0x0);
+	if (ret) {
+		dev_err(&priv->client->dev, "Cannot write ctrl enable\n");
+		goto out;
+	}
+
+	ret = regmap_write(priv->regmap, LM3697_OUTPUT_CONFIG, priv->bank_cfg);
+	if (ret)
+		dev_err(&priv->client->dev, "Cannot write OUTPUT config\n");
+
+	for (i = 0; i < LM3697_MAX_CONTROL_BANKS; i++) {
+		led = &priv->leds[i];
+		ret = ti_lmu_common_set_ramp(&led->lmu_data);
+		if (ret)
+			dev_err(&priv->client->dev, "Setting the ramp rate failed\n");
+	}
+out:
+	return ret;
+}
+
+static int lm3697_probe_dt(struct lm3697 *priv)
+{
+	struct fwnode_handle *child = NULL;
+	struct lm3697_led *led;
+	const char *name;
+	u32 max_brightness;
+	int control_bank;
+	size_t i = 0;
+	int ret = -EINVAL;
+	int j;
+
+	priv->enable_gpio = devm_gpiod_get_optional(&priv->client->dev,
+						   "enable", GPIOD_OUT_LOW);
+	if (IS_ERR(priv->enable_gpio)) {
+		ret = PTR_ERR(priv->enable_gpio);
+		dev_err(&priv->client->dev, "Failed to get enable gpio: %d\n",
+			ret);
+		return ret;
+	}
+
+	priv->regulator = devm_regulator_get(&priv->client->dev, "vled");
+	if (IS_ERR(priv->regulator))
+		priv->regulator = NULL;
+
+	device_for_each_child_node(priv->dev, child) {
+		ret = fwnode_property_read_u32(child, "reg", &control_bank);
+		if (ret) {
+			dev_err(&priv->client->dev, "reg property missing\n");
+			fwnode_handle_put(child);
+			goto child_out;
+		}
+
+		if (control_bank > LM3697_CONTROL_B) {
+			dev_err(&priv->client->dev, "reg property is invalid\n");
+			ret = -EINVAL;
+			fwnode_handle_put(child);
+			goto child_out;
+		}
+
+		led = &priv->leds[i];
+
+		ret = fwnode_property_read_u32(child, "max_brightness",
+					       &max_brightness);
+		if (ret)
+			max_brightness = MAX_BRIGHTNESS_8BIT;
+
+		if (max_brightness > MAX_BRIGHTNESS_11BIT)
+			max_brightness = MAX_BRIGHTNESS_11BIT;
+
+		led->control_bank = control_bank;
+		led->lmu_data.regmap = priv->regmap;
+		led->lmu_data.max_brightness = max_brightness;
+		led->lmu_data.runtime_ramp_reg = LM3697_CTRL_A_RAMP +
+						 control_bank;
+		led->lmu_data.msb_brightness_reg = LM3697_CTRL_A_BRT_MSB +
+						   led->control_bank * 2;
+		led->lmu_data.lsb_brightness_reg = LM3697_CTRL_A_BRT_LSB +
+						   led->control_bank * 2;
+
+		led->num_leds = fwnode_property_read_u32_array(child,
+						       "led-sources",
+						       NULL, 0);
+
+		if (led->num_leds > LM3697_MAX_LED_STRINGS) {
+			dev_err(&priv->client->dev, "To many LED strings defined\n");
+			continue;
+		}
+
+		ret = fwnode_property_read_u32_array(child, "led-sources",
+						    led->hvled_strings,
+						    led->num_leds);
+		if (ret) {
+			dev_err(&priv->client->dev, "led-sources property missing\n");
+			fwnode_handle_put(child);
+			goto child_out;
+		}
+
+		for (j = 0; j < led->num_leds; j++)
+			priv->bank_cfg |=
+				(led->control_bank << led->hvled_strings[j]);
+
+
+		ret = ti_lmu_common_get_ramp_params(&priv->client->dev,
+						    child, &led->lmu_data);
+		if (ret)
+			dev_warn(&priv->client->dev, "runtime-ramp properties missing\n");
+
+		fwnode_property_read_string(child, "linux,default-trigger",
+					    &led->led_dev.default_trigger);
+
+		ret = fwnode_property_read_string(child, "label", &name);
+		if (ret)
+			snprintf(led->label, sizeof(led->label),
+				"%s::", priv->client->name);
+		else
+			snprintf(led->label, sizeof(led->label),
+				 "%s:%s", priv->client->name, name);
+
+		led->priv = priv;
+		led->led_dev.name = led->label;
+		led->led_dev.max_brightness = max_brightness;
+		led->led_dev.brightness_set_blocking = lm3697_brightness_set;
+
+		ret = devm_led_classdev_register(priv->dev, &led->led_dev);
+		if (ret) {
+			dev_err(&priv->client->dev, "led register err: %d\n",
+				ret);
+			fwnode_handle_put(child);
+			goto child_out;
+		}
+
+		i++;
+	}
+
+child_out:
+	return ret;
+}
+
+static int lm3697_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct lm3697 *led;
+	int count;
+	int ret;
+
+	count = device_get_child_node_count(&client->dev);
+	if (!count) {
+		dev_err(&client->dev, "LEDs are not defined in device tree!");
+		return -ENODEV;
+	}
+
+	led = devm_kzalloc(&client->dev, struct_size(led, leds, count),
+			   GFP_KERNEL);
+	if (!led)
+		return -ENOMEM;
+
+	mutex_init(&led->lock);
+	i2c_set_clientdata(client, led);
+
+	led->client = client;
+	led->dev = &client->dev;
+	led->regmap = devm_regmap_init_i2c(client, &lm3697_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 = lm3697_probe_dt(led);
+	if (ret)
+		return ret;
+
+	return lm3697_init(led);
+}
+
+static int lm3697_remove(struct i2c_client *client)
+{
+	struct lm3697 *led = i2c_get_clientdata(client);
+	int ret;
+
+	ret = regmap_update_bits(led->regmap, LM3697_CTRL_ENABLE,
+				 LM3697_CTRL_A_B_EN, 0);
+	if (ret) {
+		dev_err(&led->client->dev, "Failed to disable the device\n");
+		return ret;
+	}
+
+	if (led->enable_gpio)
+		gpiod_direction_output(led->enable_gpio, 0);
+
+	if (led->regulator) {
+		ret = regulator_disable(led->regulator);
+		if (ret)
+			dev_err(&led->client->dev,
+				"Failed to disable regulator\n");
+	}
+
+	mutex_destroy(&led->lock);
+
+	return 0;
+}
+
+static const struct i2c_device_id lm3697_id[] = {
+	{ "lm3697", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, lm3697_id);
+
+static const struct of_device_id of_lm3697_leds_match[] = {
+	{ .compatible = "ti,lm3697", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, of_lm3697_leds_match);
+
+static struct i2c_driver lm3697_driver = {
+	.driver = {
+		.name	= "lm3697",
+		.of_match_table = of_lm3697_leds_match,
+	},
+	.probe		= lm3697_probe,
+	.remove		= lm3697_remove,
+	.id_table	= lm3697_id,
+};
+module_i2c_driver(lm3697_driver);
+
+MODULE_DESCRIPTION("Texas Instruments LM3697 LED driver");
+MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com>");
+MODULE_LICENSE("GPL v2");
-- 
2.19.0


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [PATCH 0/5] TI LMU rework
  2019-03-25 14:23 ` Dan Murphy
@ 2019-04-03 12:02   ` Dan Murphy
  -1 siblings, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-04-03 12:02 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel; +Cc: linux-kernel, linux-leds, Lee Jones

Hello

On 3/25/19 9:23 AM, Dan Murphy wrote:
> All
> 
> I know that it has been a long time but I put some additional effort into this
> code.  The TI LMU common code right now handles brightness and ramp up/down
> setting for the LM3697.  This so far are the common features I could find.
> 
> The LM3697 driver has the ability to assign HVLED strings to specific control
> banks as well as assigning different max brightnesses to these strings.
> 
> Fault monitoring was removed as the data sheet indicates that this is for
> production tests only.
> 
> I have plans to add additional LED drivers to use the TI LMU but I figured trying
> to add all of them at once would be a daunting review and probably wrought with 
> problems.
> 

I have a new device I need to add to the TI LMU code LM36274.
This will use the MFD ti-lmu code and I will need to create the backlight and regulator drivers.

I need to know how to proceed.  I can base the work off of the LM3532 patchset or use what is in mainline
or base it off this patchset.  The LM36274 would use the ti-lmu-led-common code for brightness setting but the
ramping times are different.

Please let me know how to proceed.

Dan

> Dan
> 
> Dan Murphy (5):
>   dt-bindings: mfd: Update the ramp up/down property
>   leds: TI LMU: Add common code for TI LMU devices
>   dt-bindings: ti-lmu: Modify dt bindings for the LM3697
>   mfd: ti-lmu: Remove support for LM3697
>   leds: lm3697: Introduce the lm3697 driver
> 
>  .../devicetree/bindings/leds/leds-lm3697.txt  |  77 ++++
>  .../devicetree/bindings/mfd/ti-lmu.txt        |  42 +-
>  drivers/leds/Kconfig                          |  14 +
>  drivers/leds/Makefile                         |   2 +
>  drivers/leds/leds-lm3697.c                    | 401 ++++++++++++++++++
>  drivers/leds/ti-lmu-led-common.c              | 131 ++++++
>  drivers/mfd/Kconfig                           |   2 +-
>  drivers/mfd/ti-lmu.c                          |  17 -
>  include/linux/mfd/ti-lmu-register.h           |  44 --
>  include/linux/mfd/ti-lmu.h                    |   1 -
>  include/linux/ti-lmu-led-common.h             |  44 ++
>  11 files changed, 682 insertions(+), 93 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3697.txt
>  create mode 100644 drivers/leds/leds-lm3697.c
>  create mode 100644 drivers/leds/ti-lmu-led-common.c
>  create mode 100644 include/linux/ti-lmu-led-common.h
> 


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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 0/5] TI LMU rework
@ 2019-04-03 12:02   ` Dan Murphy
  0 siblings, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-04-03 12:02 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel; +Cc: linux-kernel, linux-leds, Lee Jones

Hello

On 3/25/19 9:23 AM, Dan Murphy wrote:
> All
> 
> I know that it has been a long time but I put some additional effort into this
> code.  The TI LMU common code right now handles brightness and ramp up/down
> setting for the LM3697.  This so far are the common features I could find.
> 
> The LM3697 driver has the ability to assign HVLED strings to specific control
> banks as well as assigning different max brightnesses to these strings.
> 
> Fault monitoring was removed as the data sheet indicates that this is for
> production tests only.
> 
> I have plans to add additional LED drivers to use the TI LMU but I figured trying
> to add all of them at once would be a daunting review and probably wrought with 
> problems.
> 

I have a new device I need to add to the TI LMU code LM36274.
This will use the MFD ti-lmu code and I will need to create the backlight and regulator drivers.

I need to know how to proceed.  I can base the work off of the LM3532 patchset or use what is in mainline
or base it off this patchset.  The LM36274 would use the ti-lmu-led-common code for brightness setting but the
ramping times are different.

Please let me know how to proceed.

Dan

> Dan
> 
> Dan Murphy (5):
>   dt-bindings: mfd: Update the ramp up/down property
>   leds: TI LMU: Add common code for TI LMU devices
>   dt-bindings: ti-lmu: Modify dt bindings for the LM3697
>   mfd: ti-lmu: Remove support for LM3697
>   leds: lm3697: Introduce the lm3697 driver
> 
>  .../devicetree/bindings/leds/leds-lm3697.txt  |  77 ++++
>  .../devicetree/bindings/mfd/ti-lmu.txt        |  42 +-
>  drivers/leds/Kconfig                          |  14 +
>  drivers/leds/Makefile                         |   2 +
>  drivers/leds/leds-lm3697.c                    | 401 ++++++++++++++++++
>  drivers/leds/ti-lmu-led-common.c              | 131 ++++++
>  drivers/mfd/Kconfig                           |   2 +-
>  drivers/mfd/ti-lmu.c                          |  17 -
>  include/linux/mfd/ti-lmu-register.h           |  44 --
>  include/linux/mfd/ti-lmu.h                    |   1 -
>  include/linux/ti-lmu-led-common.h             |  44 ++
>  11 files changed, 682 insertions(+), 93 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3697.txt
>  create mode 100644 drivers/leds/leds-lm3697.c
>  create mode 100644 drivers/leds/ti-lmu-led-common.c
>  create mode 100644 include/linux/ti-lmu-led-common.h
> 


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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 3/5] dt-bindings: ti-lmu: Modify dt bindings for the LM3697
  2019-03-25 14:24   ` Dan Murphy
  (?)
@ 2019-04-03 20:10   ` Jacek Anaszewski
  2019-04-03 20:23       ` Dan Murphy
  -1 siblings, 1 reply; 38+ messages in thread
From: Jacek Anaszewski @ 2019-04-03 20:10 UTC (permalink / raw)
  To: Dan Murphy, robh+dt, pavel; +Cc: linux-kernel, linux-leds

Hi Dan,

Thank you for the patch.

You need Lee Jones on CC for this series.

One more comment below.

On 3/25/19 3:24 PM, Dan Murphy wrote:
> The LM3697 is a single function LED driver. The single function LED
> driver needs to reside in the LED directory as a dedicated LED driver
> and not as a MFD device.  The device does have common brightness and ramp
> features and those can be accomodated by a TI LMU framework.
> 
> The LM3697 dt binding needs to be moved from the ti-lmu.txt and a dedicated
> LED dt binding needs to be added.  The new LM3697 LED dt binding will then
> reside in the Documentation/devicetree/bindings/leds directory and follow the
> current LED and general bindings guidelines.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>   .../devicetree/bindings/leds/leds-lm3697.txt  | 77 +++++++++++++++++++
>   .../devicetree/bindings/mfd/ti-lmu.txt        | 26 +------
>   2 files changed, 78 insertions(+), 25 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3697.txt
> 
> diff --git a/Documentation/devicetree/bindings/leds/leds-lm3697.txt b/Documentation/devicetree/bindings/leds/leds-lm3697.txt
> new file mode 100644
> index 000000000000..a780f11acd38
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-lm3697.txt
> @@ -0,0 +1,77 @@
> +* Texas Instruments - LM3697 Highly Efficient White LED Driver
> +
> +The LM3697 11-bit LED driver provides high-
> +performance backlight dimming for 1, 2, or 3 series
> +LED strings while delivering up to 90% efficiency.
> +
> +This device is suitable for display and keypad Lighting
> +
> +Required properties:
> +	- compatible:
> +		"ti,lm3697"
> +	- reg :  I2C slave address
> +	- #address-cells : 1
> +	- #size-cells : 0
> +
> +Optional properties:
> +	- enable-gpios : GPIO pin to enable/disable the device
> +	- vled-supply : LED supply
> +
> +Required child properties:
> +	- reg : 0 - LED is Controlled by bank A
> +		1 - LED is Controlled by bank B
> +	- led-sources : Indicates which HVLED string is associated to which
> +			control bank.  This is a zero based property so
> +			HVLED1 = 0, HVLED2 = 1, HVLED3 = 2.
> +			Additional information is contained
> +			in Documentation/devicetree/bindings/leds/common.txt
> +
> +Optional child properties:
> +	- max_brightness - This determines whether to use 8 bit brightness mode
> +			   or 11 bit brightness mode.  If this value is not
> +			   set the device is defaulted to the preferred 8bit
> +			   brightness mode per 7.3.4.1 of the data sheet.
> +			   The values are 255 (8bit) or 2047 (11bit).

We should use led-max-microamp for that, if possible.

-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 0/5] TI LMU rework
  2019-04-03 12:02   ` Dan Murphy
  (?)
@ 2019-04-03 20:14   ` Jacek Anaszewski
  -1 siblings, 0 replies; 38+ messages in thread
From: Jacek Anaszewski @ 2019-04-03 20:14 UTC (permalink / raw)
  To: Dan Murphy, robh+dt, pavel; +Cc: linux-kernel, linux-leds, Lee Jones

Hi Dan,

On 4/3/19 2:02 PM, Dan Murphy wrote:
> Hello
> 
> On 3/25/19 9:23 AM, Dan Murphy wrote:
>> All
>>
>> I know that it has been a long time but I put some additional effort into this
>> code.  The TI LMU common code right now handles brightness and ramp up/down
>> setting for the LM3697.  This so far are the common features I could find.
>>
>> The LM3697 driver has the ability to assign HVLED strings to specific control
>> banks as well as assigning different max brightnesses to these strings.
>>
>> Fault monitoring was removed as the data sheet indicates that this is for
>> production tests only.
>>
>> I have plans to add additional LED drivers to use the TI LMU but I figured trying
>> to add all of them at once would be a daunting review and probably wrought with
>> problems.
>>
> 
> I have a new device I need to add to the TI LMU code LM36274.
> This will use the MFD ti-lmu code and I will need to create the backlight and regulator drivers.
> 
> I need to know how to proceed.  I can base the work off of the LM3532 patchset or use what is in mainline
> or base it off this patchset.  The LM36274 would use the ti-lmu-led-common code for brightness setting but the
> ramping times are different.
> 
> Please let me know how to proceed.

The patch set looks good to me with a single reservation I've just
posted. You can base your work off it.

-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 3/5] dt-bindings: ti-lmu: Modify dt bindings for the LM3697
  2019-04-03 20:10   ` Jacek Anaszewski
@ 2019-04-03 20:23       ` Dan Murphy
  0 siblings, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-04-03 20:23 UTC (permalink / raw)
  To: Jacek Anaszewski, robh+dt, pavel; +Cc: linux-kernel, linux-leds

Jacek

On 4/3/19 3:10 PM, Jacek Anaszewski wrote:
> Hi Dan,
> 
> Thank you for the patch.
> 
> You need Lee Jones on CC for this series.
> 

Yes I saw I missed Lee.

> One more comment below.
> 
> On 3/25/19 3:24 PM, Dan Murphy wrote:
>> The LM3697 is a single function LED driver. The single function LED
>> driver needs to reside in the LED directory as a dedicated LED driver
>> and not as a MFD device.  The device does have common brightness and ramp
>> features and those can be accomodated by a TI LMU framework.
>>
>> The LM3697 dt binding needs to be moved from the ti-lmu.txt and a dedicated
>> LED dt binding needs to be added.  The new LM3697 LED dt binding will then
>> reside in the Documentation/devicetree/bindings/leds directory and follow the
>> current LED and general bindings guidelines.
>>
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>> ---
>>   .../devicetree/bindings/leds/leds-lm3697.txt  | 77 +++++++++++++++++++
>>   .../devicetree/bindings/mfd/ti-lmu.txt        | 26 +------
>>   2 files changed, 78 insertions(+), 25 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3697.txt
>>
>> diff --git a/Documentation/devicetree/bindings/leds/leds-lm3697.txt b/Documentation/devicetree/bindings/leds/leds-lm3697.txt
>> new file mode 100644
>> index 000000000000..a780f11acd38
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/leds/leds-lm3697.txt
>> @@ -0,0 +1,77 @@
>> +* Texas Instruments - LM3697 Highly Efficient White LED Driver
>> +
>> +The LM3697 11-bit LED driver provides high-
>> +performance backlight dimming for 1, 2, or 3 series
>> +LED strings while delivering up to 90% efficiency.
>> +
>> +This device is suitable for display and keypad Lighting
>> +
>> +Required properties:
>> +    - compatible:
>> +        "ti,lm3697"
>> +    - reg :  I2C slave address
>> +    - #address-cells : 1
>> +    - #size-cells : 0
>> +
>> +Optional properties:
>> +    - enable-gpios : GPIO pin to enable/disable the device
>> +    - vled-supply : LED supply
>> +
>> +Required child properties:
>> +    - reg : 0 - LED is Controlled by bank A
>> +        1 - LED is Controlled by bank B
>> +    - led-sources : Indicates which HVLED string is associated to which
>> +            control bank.  This is a zero based property so
>> +            HVLED1 = 0, HVLED2 = 1, HVLED3 = 2.
>> +            Additional information is contained
>> +            in Documentation/devicetree/bindings/leds/common.txt
>> +
>> +Optional child properties:
>> +    - max_brightness - This determines whether to use 8 bit brightness mode
>> +               or 11 bit brightness mode.  If this value is not
>> +               set the device is defaulted to the preferred 8bit
>> +               brightness mode per 7.3.4.1 of the data sheet.
>> +               The values are 255 (8bit) or 2047 (11bit).
> 
> We should use led-max-microamp for that, if possible.
> 

Actually I was thinking this property could move to common.txt
LM3697 would use it and it is also defined in leds-pwm.txt leds-netxbig.txt 

But I could rename it to led-max-microamp and figure out an algo to convert to max brightness

Dan

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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 3/5] dt-bindings: ti-lmu: Modify dt bindings for the LM3697
@ 2019-04-03 20:23       ` Dan Murphy
  0 siblings, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-04-03 20:23 UTC (permalink / raw)
  To: Jacek Anaszewski, robh+dt, pavel; +Cc: linux-kernel, linux-leds

Jacek

On 4/3/19 3:10 PM, Jacek Anaszewski wrote:
> Hi Dan,
> 
> Thank you for the patch.
> 
> You need Lee Jones on CC for this series.
> 

Yes I saw I missed Lee.

> One more comment below.
> 
> On 3/25/19 3:24 PM, Dan Murphy wrote:
>> The LM3697 is a single function LED driver. The single function LED
>> driver needs to reside in the LED directory as a dedicated LED driver
>> and not as a MFD device.  The device does have common brightness and ramp
>> features and those can be accomodated by a TI LMU framework.
>>
>> The LM3697 dt binding needs to be moved from the ti-lmu.txt and a dedicated
>> LED dt binding needs to be added.  The new LM3697 LED dt binding will then
>> reside in the Documentation/devicetree/bindings/leds directory and follow the
>> current LED and general bindings guidelines.
>>
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>> ---
>>   .../devicetree/bindings/leds/leds-lm3697.txt  | 77 +++++++++++++++++++
>>   .../devicetree/bindings/mfd/ti-lmu.txt        | 26 +------
>>   2 files changed, 78 insertions(+), 25 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3697.txt
>>
>> diff --git a/Documentation/devicetree/bindings/leds/leds-lm3697.txt b/Documentation/devicetree/bindings/leds/leds-lm3697.txt
>> new file mode 100644
>> index 000000000000..a780f11acd38
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/leds/leds-lm3697.txt
>> @@ -0,0 +1,77 @@
>> +* Texas Instruments - LM3697 Highly Efficient White LED Driver
>> +
>> +The LM3697 11-bit LED driver provides high-
>> +performance backlight dimming for 1, 2, or 3 series
>> +LED strings while delivering up to 90% efficiency.
>> +
>> +This device is suitable for display and keypad Lighting
>> +
>> +Required properties:
>> +    - compatible:
>> +        "ti,lm3697"
>> +    - reg :  I2C slave address
>> +    - #address-cells : 1
>> +    - #size-cells : 0
>> +
>> +Optional properties:
>> +    - enable-gpios : GPIO pin to enable/disable the device
>> +    - vled-supply : LED supply
>> +
>> +Required child properties:
>> +    - reg : 0 - LED is Controlled by bank A
>> +        1 - LED is Controlled by bank B
>> +    - led-sources : Indicates which HVLED string is associated to which
>> +            control bank.  This is a zero based property so
>> +            HVLED1 = 0, HVLED2 = 1, HVLED3 = 2.
>> +            Additional information is contained
>> +            in Documentation/devicetree/bindings/leds/common.txt
>> +
>> +Optional child properties:
>> +    - max_brightness - This determines whether to use 8 bit brightness mode
>> +               or 11 bit brightness mode.  If this value is not
>> +               set the device is defaulted to the preferred 8bit
>> +               brightness mode per 7.3.4.1 of the data sheet.
>> +               The values are 255 (8bit) or 2047 (11bit).
> 
> We should use led-max-microamp for that, if possible.
> 

Actually I was thinking this property could move to common.txt
LM3697 would use it and it is also defined in leds-pwm.txt leds-netxbig.txt 

But I could rename it to led-max-microamp and figure out an algo to convert to max brightness

Dan

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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 4/5] mfd: ti-lmu: Remove support for LM3697
  2019-03-25 14:24   ` Dan Murphy
  (?)
@ 2019-04-04  8:17   ` Lee Jones
  2019-04-04  8:18     ` Lee Jones
  2019-04-04 12:09       ` Dan Murphy
  -1 siblings, 2 replies; 38+ messages in thread
From: Lee Jones @ 2019-04-04  8:17 UTC (permalink / raw)
  To: Dan Murphy; +Cc: robh+dt, jacek.anaszewski, pavel, linux-kernel, linux-leds

On Mon, 25 Mar 2019, Dan Murphy wrote:

> Remove support for the LM3697 from the ti-lmu

Bit of an odd place to insert a line feed.

> driver in favor of a dedicated LED driver.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>  drivers/leds/Kconfig                |  2 +-
>  drivers/mfd/Kconfig                 |  2 +-
>  drivers/mfd/ti-lmu.c                | 17 -----------
>  include/linux/mfd/ti-lmu-register.h | 44 -----------------------------
>  include/linux/mfd/ti-lmu.h          |  1 -
>  5 files changed, 2 insertions(+), 64 deletions(-)

I'm fine with the patch.

Can you edit drivers/leds/Kconfig as part of a separate patch please?
Keeping this in sync with this change is not critical and would add a
whole bunch of process which we can really do without.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 4/5] mfd: ti-lmu: Remove support for LM3697
  2019-04-04  8:17   ` Lee Jones
@ 2019-04-04  8:18     ` Lee Jones
  2019-04-04 12:09       ` Dan Murphy
  1 sibling, 0 replies; 38+ messages in thread
From: Lee Jones @ 2019-04-04  8:18 UTC (permalink / raw)
  To: Dan Murphy; +Cc: robh+dt, jacek.anaszewski, pavel, linux-kernel, linux-leds

On Thu, 04 Apr 2019, Lee Jones wrote:

> On Mon, 25 Mar 2019, Dan Murphy wrote:
> 
> > Remove support for the LM3697 from the ti-lmu
> 
> Bit of an odd place to insert a line feed.
> 
> > driver in favor of a dedicated LED driver.
> > 
> > Signed-off-by: Dan Murphy <dmurphy@ti.com>
> > ---
> >  drivers/leds/Kconfig                |  2 +-
> >  drivers/mfd/Kconfig                 |  2 +-
> >  drivers/mfd/ti-lmu.c                | 17 -----------
> >  include/linux/mfd/ti-lmu-register.h | 44 -----------------------------
> >  include/linux/mfd/ti-lmu.h          |  1 -
> >  5 files changed, 2 insertions(+), 64 deletions(-)
> 
> I'm fine with the patch.
> 
> Can you edit drivers/leds/Kconfig as part of a separate patch please?
> Keeping this in sync with this change is not critical and would add a
> whole bunch of process which we can really do without.

BTW, please add my:

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/5] dt-bindings: mfd: Update the ramp up/down property
  2019-03-25 14:23   ` Dan Murphy
  (?)
@ 2019-04-04  8:19   ` Lee Jones
  2019-04-04 12:07       ` Dan Murphy
  -1 siblings, 1 reply; 38+ messages in thread
From: Lee Jones @ 2019-04-04  8:19 UTC (permalink / raw)
  To: Dan Murphy; +Cc: robh+dt, jacek.anaszewski, pavel, linux-kernel, linux-leds

On Mon, 25 Mar 2019, Dan Murphy wrote:

> Modify the ramp-up/down property and add the property description
> to the binding.

This is the 'what', but where is the 'why'?

> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>  .../devicetree/bindings/mfd/ti-lmu.txt        | 20 ++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/ti-lmu.txt b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
> index 980394d701a7..d9d529de6dc1 100644
> --- a/Documentation/devicetree/bindings/mfd/ti-lmu.txt
> +++ b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
> @@ -25,6 +25,12 @@ Required properties:
>  
>  Optional property:
>    - enable-gpios: A GPIO specifier for hardware enable pin.
> +  - ramp-up-ms: Current ramping from one brightness level to
> +		the a higher brightness level.
> +		Range from 2048 us - 117.44 s
> +  - ramp-down-ms: Current ramping from one brightness level to
> +		  the a lower brightness level.
> +		  Range from 2048 us - 117.44 s
>  
>  Required node:
>    - backlight: All LMU devices have backlight child nodes.
> @@ -90,7 +96,7 @@ lm3631@29 {
>  
>  		lcd_bl {
>  			led-sources = <0 1>;
> -			ramp-up-msec = <300>;
> +			ramp-up-ms = <300>;
>  		};
>  	};
>  };
> @@ -152,15 +158,15 @@ lm3633@36 {
>  		main {
>  			label = "main_lcd";
>  			led-sources = <1 2>;
> -			ramp-up-msec = <500>;
> -			ramp-down-msec = <500>;
> +			ramp-up-ms = <500>;
> +			ramp-down-ms = <500>;
>  		};
>  
>  		front {
>  			label = "front_lcd";
>  			led-sources = <0>;
> -			ramp-up-msec = <1000>;
> -			ramp-down-msec = <0>;
> +			ramp-up-ms = <1000>;
> +			ramp-down-ms = <0>;
>  		};
>  	};
>  
> @@ -212,8 +218,8 @@ lm3697@36 {
>  
>  		lcd {
>  			led-sources = <0 1 2>;
> -			ramp-up-msec = <200>;
> -			ramp-down-msec = <200>;
> +			ramp-up-ms = <200>;
> +			ramp-down-ms = <200>;
>  		};
>  	};
>  

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/5] dt-bindings: mfd: Update the ramp up/down property
  2019-04-04  8:19   ` Lee Jones
@ 2019-04-04 12:07       ` Dan Murphy
  0 siblings, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-04-04 12:07 UTC (permalink / raw)
  To: Lee Jones; +Cc: robh+dt, jacek.anaszewski, pavel, linux-kernel, linux-leds

Lee

Thanks for the review

On 4/4/19 3:19 AM, Lee Jones wrote:
> On Mon, 25 Mar 2019, Dan Murphy wrote:
> 
>> Modify the ramp-up/down property and add the property description
>> to the binding.
> 
> This is the 'what', but where is the 'why'?
> 

I will add the why in v2.

Dan
<snip>

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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/5] dt-bindings: mfd: Update the ramp up/down property
@ 2019-04-04 12:07       ` Dan Murphy
  0 siblings, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-04-04 12:07 UTC (permalink / raw)
  To: Lee Jones; +Cc: robh+dt, jacek.anaszewski, pavel, linux-kernel, linux-leds

Lee

Thanks for the review

On 4/4/19 3:19 AM, Lee Jones wrote:
> On Mon, 25 Mar 2019, Dan Murphy wrote:
> 
>> Modify the ramp-up/down property and add the property description
>> to the binding.
> 
> This is the 'what', but where is the 'why'?
> 

I will add the why in v2.

Dan
<snip>

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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 4/5] mfd: ti-lmu: Remove support for LM3697
  2019-04-04  8:17   ` Lee Jones
@ 2019-04-04 12:09       ` Dan Murphy
  2019-04-04 12:09       ` Dan Murphy
  1 sibling, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-04-04 12:09 UTC (permalink / raw)
  To: Lee Jones; +Cc: robh+dt, jacek.anaszewski, pavel, linux-kernel, linux-leds

Lee

Thanks for the review

On 4/4/19 3:17 AM, Lee Jones wrote:
> On Mon, 25 Mar 2019, Dan Murphy wrote:
> 
>> Remove support for the LM3697 from the ti-lmu
> 
> Bit of an odd place to insert a line feed.
> 

I will fix in v2.

>> driver in favor of a dedicated LED driver.
>>
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>> ---
>>  drivers/leds/Kconfig                |  2 +-
>>  drivers/mfd/Kconfig                 |  2 +-
>>  drivers/mfd/ti-lmu.c                | 17 -----------
>>  include/linux/mfd/ti-lmu-register.h | 44 -----------------------------
>>  include/linux/mfd/ti-lmu.h          |  1 -
>>  5 files changed, 2 insertions(+), 64 deletions(-)
> 
> I'm fine with the patch.
> 
> Can you edit drivers/leds/Kconfig as part of a separate patch please?
> Keeping this in sync with this change is not critical and would add a
> whole bunch of process which we can really do without.
> 

Ack.  I will add this when I add the LM3697 config in the Kconfig

Dan

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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 4/5] mfd: ti-lmu: Remove support for LM3697
@ 2019-04-04 12:09       ` Dan Murphy
  0 siblings, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-04-04 12:09 UTC (permalink / raw)
  To: Lee Jones; +Cc: robh+dt, jacek.anaszewski, pavel, linux-kernel, linux-leds

Lee

Thanks for the review

On 4/4/19 3:17 AM, Lee Jones wrote:
> On Mon, 25 Mar 2019, Dan Murphy wrote:
> 
>> Remove support for the LM3697 from the ti-lmu
> 
> Bit of an odd place to insert a line feed.
> 

I will fix in v2.

>> driver in favor of a dedicated LED driver.
>>
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>> ---
>>  drivers/leds/Kconfig                |  2 +-
>>  drivers/mfd/Kconfig                 |  2 +-
>>  drivers/mfd/ti-lmu.c                | 17 -----------
>>  include/linux/mfd/ti-lmu-register.h | 44 -----------------------------
>>  include/linux/mfd/ti-lmu.h          |  1 -
>>  5 files changed, 2 insertions(+), 64 deletions(-)
> 
> I'm fine with the patch.
> 
> Can you edit drivers/leds/Kconfig as part of a separate patch please?
> Keeping this in sync with this change is not critical and would add a
> whole bunch of process which we can really do without.
> 

Ack.  I will add this when I add the LM3697 config in the Kconfig

Dan

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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 2/5] leds: TI LMU: Add common code for TI LMU devices
  2019-03-25 14:24   ` Dan Murphy
  (?)
@ 2019-04-04 13:22   ` Pavel Machek
  -1 siblings, 0 replies; 38+ messages in thread
From: Pavel Machek @ 2019-04-04 13:22 UTC (permalink / raw)
  To: Dan Murphy; +Cc: robh+dt, jacek.anaszewski, linux-kernel, linux-leds

On Mon 2019-03-25 09:24:00, Dan Murphy wrote:
> Create a TI LMU common framework for TI LMU devices that share
> common features.
> 
> Currently the runtime ramp and brightness setting have
> been identified as common features with common register settings.
> 
> This work is derived from Milo Kims TI LMU MFD code.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>

Acked-by: Pavel Machek <pavel@ucw.cz>

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 3/5] dt-bindings: ti-lmu: Modify dt bindings for the LM3697
  2019-04-03 20:23       ` Dan Murphy
  (?)
@ 2019-04-04 18:39       ` Jacek Anaszewski
  2019-04-04 19:27           ` Dan Murphy
  -1 siblings, 1 reply; 38+ messages in thread
From: Jacek Anaszewski @ 2019-04-04 18:39 UTC (permalink / raw)
  To: Dan Murphy, robh+dt, pavel; +Cc: linux-kernel, linux-leds

Dan,

On 4/3/19 10:23 PM, Dan Murphy wrote:
> Jacek
> 
> On 4/3/19 3:10 PM, Jacek Anaszewski wrote:
>> Hi Dan,
>>
>> Thank you for the patch.
>>
>> You need Lee Jones on CC for this series.
>>
> 
> Yes I saw I missed Lee.
> 
>> One more comment below.
>>
>> On 3/25/19 3:24 PM, Dan Murphy wrote:
>>> The LM3697 is a single function LED driver. The single function LED
>>> driver needs to reside in the LED directory as a dedicated LED driver
>>> and not as a MFD device.  The device does have common brightness and ramp
>>> features and those can be accomodated by a TI LMU framework.
>>>
>>> The LM3697 dt binding needs to be moved from the ti-lmu.txt and a dedicated
>>> LED dt binding needs to be added.  The new LM3697 LED dt binding will then
>>> reside in the Documentation/devicetree/bindings/leds directory and follow the
>>> current LED and general bindings guidelines.
>>>
>>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>>> ---
>>>    .../devicetree/bindings/leds/leds-lm3697.txt  | 77 +++++++++++++++++++
>>>    .../devicetree/bindings/mfd/ti-lmu.txt        | 26 +------
>>>    2 files changed, 78 insertions(+), 25 deletions(-)
>>>    create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3697.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/leds/leds-lm3697.txt b/Documentation/devicetree/bindings/leds/leds-lm3697.txt
>>> new file mode 100644
>>> index 000000000000..a780f11acd38
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/leds/leds-lm3697.txt
>>> @@ -0,0 +1,77 @@
>>> +* Texas Instruments - LM3697 Highly Efficient White LED Driver
>>> +
>>> +The LM3697 11-bit LED driver provides high-
>>> +performance backlight dimming for 1, 2, or 3 series
>>> +LED strings while delivering up to 90% efficiency.
>>> +
>>> +This device is suitable for display and keypad Lighting
>>> +
>>> +Required properties:
>>> +    - compatible:
>>> +        "ti,lm3697"
>>> +    - reg :  I2C slave address
>>> +    - #address-cells : 1
>>> +    - #size-cells : 0
>>> +
>>> +Optional properties:
>>> +    - enable-gpios : GPIO pin to enable/disable the device
>>> +    - vled-supply : LED supply
>>> +
>>> +Required child properties:
>>> +    - reg : 0 - LED is Controlled by bank A
>>> +        1 - LED is Controlled by bank B
>>> +    - led-sources : Indicates which HVLED string is associated to which
>>> +            control bank.  This is a zero based property so
>>> +            HVLED1 = 0, HVLED2 = 1, HVLED3 = 2.
>>> +            Additional information is contained
>>> +            in Documentation/devicetree/bindings/leds/common.txt
>>> +
>>> +Optional child properties:
>>> +    - max_brightness - This determines whether to use 8 bit brightness mode
>>> +               or 11 bit brightness mode.  If this value is not
>>> +               set the device is defaulted to the preferred 8bit
>>> +               brightness mode per 7.3.4.1 of the data sheet.
>>> +               The values are 255 (8bit) or 2047 (11bit).
>>
>> We should use led-max-microamp for that, if possible.
>>
> 
> Actually I was thinking this property could move to common.txt
> LM3697 would use it and it is also defined in leds-pwm.txt leds-netxbig.txt

It was considered back in 2014 when I was working on LED flash
class framework. It was then assessed inappropriate for describing
physical property of a device. It needs to be kept in mind that
it was in the context of flash LEDs, where currents are much
higher than for common LEDs. Since then we have been always using
led-max-microamp.

With max-brightness in Device Tree there is also another problem -
we don't know what it really means - greater allowed current or
greater resolution.

Why not allow for maximum available brightness resolution for ti-lmu
always when the amperage is not an issue?


> But I could rename it to led-max-microamp and figure out an algo to convert to max brightness
> 
> Dan
> 

-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 3/5] dt-bindings: ti-lmu: Modify dt bindings for the LM3697
  2019-04-04 18:39       ` Jacek Anaszewski
@ 2019-04-04 19:27           ` Dan Murphy
  0 siblings, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-04-04 19:27 UTC (permalink / raw)
  To: Jacek Anaszewski, robh+dt, pavel; +Cc: linux-kernel, linux-leds

On 4/4/19 1:39 PM, Jacek Anaszewski wrote:
> Dan,
> 
> On 4/3/19 10:23 PM, Dan Murphy wrote:
>> Jacek
>>
>> On 4/3/19 3:10 PM, Jacek Anaszewski wrote:
>>> Hi Dan,
>>>
>>> Thank you for the patch.
>>>
>>> You need Lee Jones on CC for this series.
>>>
>>
>> Yes I saw I missed Lee.
>>
>>> One more comment below.
>>>
>>> On 3/25/19 3:24 PM, Dan Murphy wrote:
>>>> The LM3697 is a single function LED driver. The single function LED
>>>> driver needs to reside in the LED directory as a dedicated LED driver
>>>> and not as a MFD device.  The device does have common brightness and ramp
>>>> features and those can be accomodated by a TI LMU framework.
>>>>
>>>> The LM3697 dt binding needs to be moved from the ti-lmu.txt and a dedicated
>>>> LED dt binding needs to be added.  The new LM3697 LED dt binding will then
>>>> reside in the Documentation/devicetree/bindings/leds directory and follow the
>>>> current LED and general bindings guidelines.
>>>>
>>>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>>>> ---
>>>>    .../devicetree/bindings/leds/leds-lm3697.txt  | 77 +++++++++++++++++++
>>>>    .../devicetree/bindings/mfd/ti-lmu.txt        | 26 +------
>>>>    2 files changed, 78 insertions(+), 25 deletions(-)
>>>>    create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3697.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/leds/leds-lm3697.txt b/Documentation/devicetree/bindings/leds/leds-lm3697.txt
>>>> new file mode 100644
>>>> index 000000000000..a780f11acd38
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/leds/leds-lm3697.txt
>>>> @@ -0,0 +1,77 @@
>>>> +* Texas Instruments - LM3697 Highly Efficient White LED Driver
>>>> +
>>>> +The LM3697 11-bit LED driver provides high-
>>>> +performance backlight dimming for 1, 2, or 3 series
>>>> +LED strings while delivering up to 90% efficiency.
>>>> +
>>>> +This device is suitable for display and keypad Lighting
>>>> +
>>>> +Required properties:
>>>> +    - compatible:
>>>> +        "ti,lm3697"
>>>> +    - reg :  I2C slave address
>>>> +    - #address-cells : 1
>>>> +    - #size-cells : 0
>>>> +
>>>> +Optional properties:
>>>> +    - enable-gpios : GPIO pin to enable/disable the device
>>>> +    - vled-supply : LED supply
>>>> +
>>>> +Required child properties:
>>>> +    - reg : 0 - LED is Controlled by bank A
>>>> +        1 - LED is Controlled by bank B
>>>> +    - led-sources : Indicates which HVLED string is associated to which
>>>> +            control bank.  This is a zero based property so
>>>> +            HVLED1 = 0, HVLED2 = 1, HVLED3 = 2.
>>>> +            Additional information is contained
>>>> +            in Documentation/devicetree/bindings/leds/common.txt
>>>> +
>>>> +Optional child properties:
>>>> +    - max_brightness - This determines whether to use 8 bit brightness mode
>>>> +               or 11 bit brightness mode.  If this value is not
>>>> +               set the device is defaulted to the preferred 8bit
>>>> +               brightness mode per 7.3.4.1 of the data sheet.
>>>> +               The values are 255 (8bit) or 2047 (11bit).
>>>
>>> We should use led-max-microamp for that, if possible.
>>>
>>
>> Actually I was thinking this property could move to common.txt
>> LM3697 would use it and it is also defined in leds-pwm.txt leds-netxbig.txt
> 
> It was considered back in 2014 when I was working on LED flash
> class framework. It was then assessed inappropriate for describing
> physical property of a device. It needs to be kept in mind that
> it was in the context of flash LEDs, where currents are much
> higher than for common LEDs. Since then we have been always using
> led-max-microamp.
> 
> With max-brightness in Device Tree there is also another problem -
> we don't know what it really means - greater allowed current or
> greater resolution.
> 
> Why not allow for maximum available brightness resolution for ti-lmu
> always when the amperage is not an issue?
> 

Maybe I should rename this to ti,brightness-resolution with the same values.
or ti,brightness-res for short hand.

Or I could try to figure out the max-microamp but it really does not describe the hardware or
the configuration.

Dan


> 
>> But I could rename it to led-max-microamp and figure out an algo to convert to max brightness
>>
>> Dan
>>
> 


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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 3/5] dt-bindings: ti-lmu: Modify dt bindings for the LM3697
@ 2019-04-04 19:27           ` Dan Murphy
  0 siblings, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-04-04 19:27 UTC (permalink / raw)
  To: Jacek Anaszewski, robh+dt, pavel; +Cc: linux-kernel, linux-leds

On 4/4/19 1:39 PM, Jacek Anaszewski wrote:
> Dan,
> 
> On 4/3/19 10:23 PM, Dan Murphy wrote:
>> Jacek
>>
>> On 4/3/19 3:10 PM, Jacek Anaszewski wrote:
>>> Hi Dan,
>>>
>>> Thank you for the patch.
>>>
>>> You need Lee Jones on CC for this series.
>>>
>>
>> Yes I saw I missed Lee.
>>
>>> One more comment below.
>>>
>>> On 3/25/19 3:24 PM, Dan Murphy wrote:
>>>> The LM3697 is a single function LED driver. The single function LED
>>>> driver needs to reside in the LED directory as a dedicated LED driver
>>>> and not as a MFD device.  The device does have common brightness and ramp
>>>> features and those can be accomodated by a TI LMU framework.
>>>>
>>>> The LM3697 dt binding needs to be moved from the ti-lmu.txt and a dedicated
>>>> LED dt binding needs to be added.  The new LM3697 LED dt binding will then
>>>> reside in the Documentation/devicetree/bindings/leds directory and follow the
>>>> current LED and general bindings guidelines.
>>>>
>>>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>>>> ---
>>>>    .../devicetree/bindings/leds/leds-lm3697.txt  | 77 +++++++++++++++++++
>>>>    .../devicetree/bindings/mfd/ti-lmu.txt        | 26 +------
>>>>    2 files changed, 78 insertions(+), 25 deletions(-)
>>>>    create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3697.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/leds/leds-lm3697.txt b/Documentation/devicetree/bindings/leds/leds-lm3697.txt
>>>> new file mode 100644
>>>> index 000000000000..a780f11acd38
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/leds/leds-lm3697.txt
>>>> @@ -0,0 +1,77 @@
>>>> +* Texas Instruments - LM3697 Highly Efficient White LED Driver
>>>> +
>>>> +The LM3697 11-bit LED driver provides high-
>>>> +performance backlight dimming for 1, 2, or 3 series
>>>> +LED strings while delivering up to 90% efficiency.
>>>> +
>>>> +This device is suitable for display and keypad Lighting
>>>> +
>>>> +Required properties:
>>>> +    - compatible:
>>>> +        "ti,lm3697"
>>>> +    - reg :  I2C slave address
>>>> +    - #address-cells : 1
>>>> +    - #size-cells : 0
>>>> +
>>>> +Optional properties:
>>>> +    - enable-gpios : GPIO pin to enable/disable the device
>>>> +    - vled-supply : LED supply
>>>> +
>>>> +Required child properties:
>>>> +    - reg : 0 - LED is Controlled by bank A
>>>> +        1 - LED is Controlled by bank B
>>>> +    - led-sources : Indicates which HVLED string is associated to which
>>>> +            control bank.  This is a zero based property so
>>>> +            HVLED1 = 0, HVLED2 = 1, HVLED3 = 2.
>>>> +            Additional information is contained
>>>> +            in Documentation/devicetree/bindings/leds/common.txt
>>>> +
>>>> +Optional child properties:
>>>> +    - max_brightness - This determines whether to use 8 bit brightness mode
>>>> +               or 11 bit brightness mode.  If this value is not
>>>> +               set the device is defaulted to the preferred 8bit
>>>> +               brightness mode per 7.3.4.1 of the data sheet.
>>>> +               The values are 255 (8bit) or 2047 (11bit).
>>>
>>> We should use led-max-microamp for that, if possible.
>>>
>>
>> Actually I was thinking this property could move to common.txt
>> LM3697 would use it and it is also defined in leds-pwm.txt leds-netxbig.txt
> 
> It was considered back in 2014 when I was working on LED flash
> class framework. It was then assessed inappropriate for describing
> physical property of a device. It needs to be kept in mind that
> it was in the context of flash LEDs, where currents are much
> higher than for common LEDs. Since then we have been always using
> led-max-microamp.
> 
> With max-brightness in Device Tree there is also another problem -
> we don't know what it really means - greater allowed current or
> greater resolution.
> 
> Why not allow for maximum available brightness resolution for ti-lmu
> always when the amperage is not an issue?
> 

Maybe I should rename this to ti,brightness-resolution with the same values.
or ti,brightness-res for short hand.

Or I could try to figure out the max-microamp but it really does not describe the hardware or
the configuration.

Dan


> 
>> But I could rename it to led-max-microamp and figure out an algo to convert to max brightness
>>
>> Dan
>>
> 


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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 3/5] dt-bindings: ti-lmu: Modify dt bindings for the LM3697
  2019-04-04 19:27           ` Dan Murphy
  (?)
@ 2019-04-04 19:31           ` Jacek Anaszewski
  -1 siblings, 0 replies; 38+ messages in thread
From: Jacek Anaszewski @ 2019-04-04 19:31 UTC (permalink / raw)
  To: Dan Murphy, robh+dt, pavel; +Cc: linux-kernel, linux-leds

On 4/4/19 9:27 PM, Dan Murphy wrote:
[...]
> Maybe I should rename this to ti,brightness-resolution with the same values.
> or ti,brightness-res for short hand.
> 
> Or I could try to figure out the max-microamp but it really does not describe the hardware or
> the configuration.

ti,brightness-resolution sounds good to me.

Rob has the last word, though.

-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 3/5] dt-bindings: ti-lmu: Modify dt bindings for the LM3697
  2019-04-04 19:27           ` Dan Murphy
@ 2019-04-04 19:34             ` Dan Murphy
  -1 siblings, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-04-04 19:34 UTC (permalink / raw)
  To: Jacek Anaszewski, robh+dt, pavel; +Cc: linux-kernel, linux-leds

Jacek

On 4/4/19 2:27 PM, Dan Murphy wrote:
<snip>

>>>>
>>>
>>> Actually I was thinking this property could move to common.txt
>>> LM3697 would use it and it is also defined in leds-pwm.txt leds-netxbig.txt
>>
>> It was considered back in 2014 when I was working on LED flash
>> class framework. It was then assessed inappropriate for describing
>> physical property of a device. It needs to be kept in mind that
>> it was in the context of flash LEDs, where currents are much
>> higher than for common LEDs. Since then we have been always using
>> led-max-microamp.
>>
>> With max-brightness in Device Tree there is also another problem -
>> we don't know what it really means - greater allowed current or
>> greater resolution.
>>
>> Why not allow for maximum available brightness resolution for ti-lmu
>> always when the amperage is not an issue?
>>

Missed answering this question.  Full scale ramping only works in 8-bit mode as the
LSB is used for the ramp.  This is why, if the customer does not want ramping and wants full 11 bit control
they can set this parameter for 11 bit brightness control.  Otherwise I default this to the 8 bit control as the preferred
method.

7.3.4.1 8-Bit Control (Preferred)
The preferred operating mode is to control the high-voltage LED brightness by setting the Control Bank LSB
register (3 LSB's) to zero and using only the Control Bank MSB register (8 MSB's). In this mode the LM3697
controls the 3 LSB's to ramp the high-voltage LED current using all 11-bits.

Ramping does work in 11 bit mode but the ramp time may be shorter based on the brightness value set.

Of course the code can be made smarter that if the optional ramp properties are available then set 8 bit automatically
but it cannot assume 11 bit if they are not.

Dan

<snip>

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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 3/5] dt-bindings: ti-lmu: Modify dt bindings for the LM3697
@ 2019-04-04 19:34             ` Dan Murphy
  0 siblings, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-04-04 19:34 UTC (permalink / raw)
  To: Jacek Anaszewski, robh+dt, pavel; +Cc: linux-kernel, linux-leds

Jacek

On 4/4/19 2:27 PM, Dan Murphy wrote:
<snip>

>>>>
>>>
>>> Actually I was thinking this property could move to common.txt
>>> LM3697 would use it and it is also defined in leds-pwm.txt leds-netxbig.txt
>>
>> It was considered back in 2014 when I was working on LED flash
>> class framework. It was then assessed inappropriate for describing
>> physical property of a device. It needs to be kept in mind that
>> it was in the context of flash LEDs, where currents are much
>> higher than for common LEDs. Since then we have been always using
>> led-max-microamp.
>>
>> With max-brightness in Device Tree there is also another problem -
>> we don't know what it really means - greater allowed current or
>> greater resolution.
>>
>> Why not allow for maximum available brightness resolution for ti-lmu
>> always when the amperage is not an issue?
>>

Missed answering this question.  Full scale ramping only works in 8-bit mode as the
LSB is used for the ramp.  This is why, if the customer does not want ramping and wants full 11 bit control
they can set this parameter for 11 bit brightness control.  Otherwise I default this to the 8 bit control as the preferred
method.

7.3.4.1 8-Bit Control (Preferred)
The preferred operating mode is to control the high-voltage LED brightness by setting the Control Bank LSB
register (3 LSB's) to zero and using only the Control Bank MSB register (8 MSB's). In this mode the LM3697
controls the 3 LSB's to ramp the high-voltage LED current using all 11-bits.

Ramping does work in 11 bit mode but the ramp time may be shorter based on the brightness value set.

Of course the code can be made smarter that if the optional ramp properties are available then set 8 bit automatically
but it cannot assume 11 bit if they are not.

Dan

<snip>

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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 5/5] leds: lm3697: Introduce the lm3697 driver
  2019-03-25 14:24   ` Dan Murphy
  (?)
@ 2019-04-13 20:06   ` Pavel Machek
  2019-04-15 12:49       ` Dan Murphy
  -1 siblings, 1 reply; 38+ messages in thread
From: Pavel Machek @ 2019-04-13 20:06 UTC (permalink / raw)
  To: Dan Murphy; +Cc: robh+dt, jacek.anaszewski, linux-kernel, linux-leds

[-- Attachment #1: Type: text/plain, Size: 2650 bytes --]

On Mon 2019-03-25 09:24:03, Dan Murphy wrote:
> Introduce the lm3697 LED driver for
> backlighting and display.
> 
> Datasheet location:
> http://www.ti.com/lit/ds/symlink/lm3697.pdf
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>  drivers/leds/Kconfig       |   8 +-
>  drivers/leds/Makefile      |   1 +
>  drivers/leds/leds-lm3697.c | 401 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 409 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/leds/leds-lm3697.c
> 
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 735009e73414..688bb9a6f275 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -776,9 +776,15 @@ config LEDS_NIC78BX
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called leds-nic78bx.
>  
> +config LEDS_LM3697
> +	tristate "LED driver for LM3697"
> +	depends on LEDS_TI_LMU_COMMON
> +	help
> +	  Say Y to enable the LM3697 LED driver for TI LMU devices.
> +	  This supports the LED device LM3697.
> +
>  config LEDS_TI_LMU_COMMON
>  	tristate "LED driver for TI LMU"
> -	depends on REGMAP
>  	help
>            Say Y to enable the LED driver for TI LMU devices.
>            This supports common features between the TI LM3532, LM3631, LM3632,

Is deleting "depends on REGMAP" intentional? AFAICT you are using it.

Plus we'd normally expect "COMMON" first and then specific driver. Not
sure if Kconfig can handle it out-of-order...


> +static int lm3697_init(struct lm3697 *priv)
> +{
> +	struct lm3697_led *led;
> +	int i, ret;
> +
> +	if (priv->enable_gpio) {
> +		gpiod_direction_output(priv->enable_gpio, 1);
> +	} else {
> +		ret = regmap_write(priv->regmap, LM3697_RESET, LM3697_SW_RESET);
> +		if (ret) {
> +			dev_err(&priv->client->dev, "Cannot reset the device\n");
> +			goto out;
> +		}
> +	}
> +
> +	ret = regmap_write(priv->regmap, LM3697_CTRL_ENABLE, 0x0);
> +	if (ret) {
> +		dev_err(&priv->client->dev, "Cannot write ctrl enable\n");
> +		goto out;
> +	}
> +
> +	ret = regmap_write(priv->regmap, LM3697_OUTPUT_CONFIG, priv->bank_cfg);
> +	if (ret)
> +		dev_err(&priv->client->dev, "Cannot write OUTPUT config\n");

Missing goto out?

> +	for (i = 0; i < LM3697_MAX_CONTROL_BANKS; i++) {
> +		led = &priv->leds[i];
> +		ret = ti_lmu_common_set_ramp(&led->lmu_data);
> +		if (ret)
> +			dev_err(&priv->client->dev, "Setting the ramp rate failed\n");
> +	}
> +out:
> +	return ret;
> +}
									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] 38+ messages in thread

* Re: [PATCH 5/5] leds: lm3697: Introduce the lm3697 driver
  2019-04-13 20:06   ` Pavel Machek
@ 2019-04-15 12:49       ` Dan Murphy
  0 siblings, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-04-15 12:49 UTC (permalink / raw)
  To: Pavel Machek; +Cc: robh+dt, jacek.anaszewski, linux-kernel, linux-leds

Pavel

On 4/13/19 3:06 PM, Pavel Machek wrote:
> On Mon 2019-03-25 09:24:03, Dan Murphy wrote:
>> Introduce the lm3697 LED driver for
>> backlighting and display.
>>
>> Datasheet location:
>> http://www.ti.com/lit/ds/symlink/lm3697.pdf
>>
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>> ---
>>  drivers/leds/Kconfig       |   8 +-
>>  drivers/leds/Makefile      |   1 +
>>  drivers/leds/leds-lm3697.c | 401 +++++++++++++++++++++++++++++++++++++
>>  3 files changed, 409 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/leds/leds-lm3697.c
>>
>> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
>> index 735009e73414..688bb9a6f275 100644
>> --- a/drivers/leds/Kconfig
>> +++ b/drivers/leds/Kconfig
>> @@ -776,9 +776,15 @@ config LEDS_NIC78BX
>>  	  To compile this driver as a module, choose M here: the module
>>  	  will be called leds-nic78bx.
>>  
>> +config LEDS_LM3697
>> +	tristate "LED driver for LM3697"
>> +	depends on LEDS_TI_LMU_COMMON
>> +	help
>> +	  Say Y to enable the LM3697 LED driver for TI LMU devices.
>> +	  This supports the LED device LM3697.
>> +
>>  config LEDS_TI_LMU_COMMON
>>  	tristate "LED driver for TI LMU"
>> -	depends on REGMAP
>>  	help
>>            Say Y to enable the LED driver for TI LMU devices.
>>            This supports common features between the TI LM3532, LM3631, LM3632,
> 
> Is deleting "depends on REGMAP" intentional? AFAICT you are using it.
> 

Thanks for pointing that out.  I don't know how that was in there.

> Plus we'd normally expect "COMMON" first and then specific driver. Not
> sure if Kconfig can handle it out-of-order...
> 
> 

OK.  Should I rename the ti_lmu file to leds-common-ti-lmu?

This keeps the naming convention the same in the leds directory as well.

FYI I will not add your acked-by on the LMU patch that introduced the code unless you approve.
Since you found issues with the kconfig

Refererence
https://lore.kernel.org/patchwork/patch/1054500/

>> +static int lm3697_init(struct lm3697 *priv)
>> +{
>> +	struct lm3697_led *led;
>> +	int i, ret;
>> +
>> +	if (priv->enable_gpio) {
>> +		gpiod_direction_output(priv->enable_gpio, 1);
>> +	} else {
>> +		ret = regmap_write(priv->regmap, LM3697_RESET, LM3697_SW_RESET);
>> +		if (ret) {
>> +			dev_err(&priv->client->dev, "Cannot reset the device\n");
>> +			goto out;
>> +		}
>> +	}
>> +
>> +	ret = regmap_write(priv->regmap, LM3697_CTRL_ENABLE, 0x0);
>> +	if (ret) {
>> +		dev_err(&priv->client->dev, "Cannot write ctrl enable\n");
>> +		goto out;
>> +	}
>> +
>> +	ret = regmap_write(priv->regmap, LM3697_OUTPUT_CONFIG, priv->bank_cfg);
>> +	if (ret)
>> +		dev_err(&priv->client->dev, "Cannot write OUTPUT config\n");
> 
> Missing goto out?

Ack

> 
>> +	for (i = 0; i < LM3697_MAX_CONTROL_BANKS; i++) {
>> +		led = &priv->leds[i];
>> +		ret = ti_lmu_common_set_ramp(&led->lmu_data);
>> +		if (ret)
>> +			dev_err(&priv->client->dev, "Setting the ramp rate failed\n");
>> +	}
>> +out:
>> +	return ret;
>> +}
> 									Pavel
> 

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 5/5] leds: lm3697: Introduce the lm3697 driver
@ 2019-04-15 12:49       ` Dan Murphy
  0 siblings, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-04-15 12:49 UTC (permalink / raw)
  To: Pavel Machek; +Cc: robh+dt, jacek.anaszewski, linux-kernel, linux-leds

Pavel

On 4/13/19 3:06 PM, Pavel Machek wrote:
> On Mon 2019-03-25 09:24:03, Dan Murphy wrote:
>> Introduce the lm3697 LED driver for
>> backlighting and display.
>>
>> Datasheet location:
>> http://www.ti.com/lit/ds/symlink/lm3697.pdf
>>
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>> ---
>>  drivers/leds/Kconfig       |   8 +-
>>  drivers/leds/Makefile      |   1 +
>>  drivers/leds/leds-lm3697.c | 401 +++++++++++++++++++++++++++++++++++++
>>  3 files changed, 409 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/leds/leds-lm3697.c
>>
>> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
>> index 735009e73414..688bb9a6f275 100644
>> --- a/drivers/leds/Kconfig
>> +++ b/drivers/leds/Kconfig
>> @@ -776,9 +776,15 @@ config LEDS_NIC78BX
>>  	  To compile this driver as a module, choose M here: the module
>>  	  will be called leds-nic78bx.
>>  
>> +config LEDS_LM3697
>> +	tristate "LED driver for LM3697"
>> +	depends on LEDS_TI_LMU_COMMON
>> +	help
>> +	  Say Y to enable the LM3697 LED driver for TI LMU devices.
>> +	  This supports the LED device LM3697.
>> +
>>  config LEDS_TI_LMU_COMMON
>>  	tristate "LED driver for TI LMU"
>> -	depends on REGMAP
>>  	help
>>            Say Y to enable the LED driver for TI LMU devices.
>>            This supports common features between the TI LM3532, LM3631, LM3632,
> 
> Is deleting "depends on REGMAP" intentional? AFAICT you are using it.
> 

Thanks for pointing that out.  I don't know how that was in there.

> Plus we'd normally expect "COMMON" first and then specific driver. Not
> sure if Kconfig can handle it out-of-order...
> 
> 

OK.  Should I rename the ti_lmu file to leds-common-ti-lmu?

This keeps the naming convention the same in the leds directory as well.

FYI I will not add your acked-by on the LMU patch that introduced the code unless you approve.
Since you found issues with the kconfig

Refererence
https://lore.kernel.org/patchwork/patch/1054500/

>> +static int lm3697_init(struct lm3697 *priv)
>> +{
>> +	struct lm3697_led *led;
>> +	int i, ret;
>> +
>> +	if (priv->enable_gpio) {
>> +		gpiod_direction_output(priv->enable_gpio, 1);
>> +	} else {
>> +		ret = regmap_write(priv->regmap, LM3697_RESET, LM3697_SW_RESET);
>> +		if (ret) {
>> +			dev_err(&priv->client->dev, "Cannot reset the device\n");
>> +			goto out;
>> +		}
>> +	}
>> +
>> +	ret = regmap_write(priv->regmap, LM3697_CTRL_ENABLE, 0x0);
>> +	if (ret) {
>> +		dev_err(&priv->client->dev, "Cannot write ctrl enable\n");
>> +		goto out;
>> +	}
>> +
>> +	ret = regmap_write(priv->regmap, LM3697_OUTPUT_CONFIG, priv->bank_cfg);
>> +	if (ret)
>> +		dev_err(&priv->client->dev, "Cannot write OUTPUT config\n");
> 
> Missing goto out?

Ack

> 
>> +	for (i = 0; i < LM3697_MAX_CONTROL_BANKS; i++) {
>> +		led = &priv->leds[i];
>> +		ret = ti_lmu_common_set_ramp(&led->lmu_data);
>> +		if (ret)
>> +			dev_err(&priv->client->dev, "Setting the ramp rate failed\n");
>> +	}
>> +out:
>> +	return ret;
>> +}
> 									Pavel
> 

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 5/5] leds: lm3697: Introduce the lm3697 driver
  2019-04-15 12:49       ` Dan Murphy
  (?)
@ 2019-04-15 14:03       ` Pavel Machek
  2019-04-15 19:53           ` Dan Murphy
  -1 siblings, 1 reply; 38+ messages in thread
From: Pavel Machek @ 2019-04-15 14:03 UTC (permalink / raw)
  To: Dan Murphy; +Cc: robh+dt, jacek.anaszewski, linux-kernel, linux-leds

[-- Attachment #1: Type: text/plain, Size: 1659 bytes --]


> >> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> >> index 735009e73414..688bb9a6f275 100644
> >> --- a/drivers/leds/Kconfig
> >> +++ b/drivers/leds/Kconfig
> >> @@ -776,9 +776,15 @@ config LEDS_NIC78BX
> >>  	  To compile this driver as a module, choose M here: the module
> >>  	  will be called leds-nic78bx.
> >>  
> >> +config LEDS_LM3697
> >> +	tristate "LED driver for LM3697"
> >> +	depends on LEDS_TI_LMU_COMMON
> >> +	help
> >> +	  Say Y to enable the LM3697 LED driver for TI LMU devices.
> >> +	  This supports the LED device LM3697.
> >> +
> >>  config LEDS_TI_LMU_COMMON
> >>  	tristate "LED driver for TI LMU"
> >> -	depends on REGMAP
> >>  	help
> >>            Say Y to enable the LED driver for TI LMU devices.
> >>            This supports common features between the TI LM3532, LM3631, LM3632,
...
> > Plus we'd normally expect "COMMON" first and then specific driver. Not
> > sure if Kconfig can handle it out-of-order...
> 
> OK.  Should I rename the ti_lmu file to leds-common-ti-lmu?

Oh, no, that is not what I meant.

You have

config B
       depends on A

config A

above. We really want

config A

config B
       depends on A

> This keeps the naming convention the same in the leds directory as well.
> 
> FYI I will not add your acked-by on the LMU patch that introduced the code unless you approve.
> Since you found issues with the kconfig

I did not review it in great detail, so no acked-by's, yet :-).
									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] 38+ messages in thread

* Re: [PATCH 5/5] leds: lm3697: Introduce the lm3697 driver
  2019-04-15 14:03       ` Pavel Machek
@ 2019-04-15 19:53           ` Dan Murphy
  0 siblings, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-04-15 19:53 UTC (permalink / raw)
  To: Pavel Machek; +Cc: robh+dt, jacek.anaszewski, linux-kernel, linux-leds



On 4/15/19 9:03 AM, Pavel Machek wrote:
> 
>>>> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
>>>> index 735009e73414..688bb9a6f275 100644
>>>> --- a/drivers/leds/Kconfig
>>>> +++ b/drivers/leds/Kconfig
>>>> @@ -776,9 +776,15 @@ config LEDS_NIC78BX
>>>>  	  To compile this driver as a module, choose M here: the module
>>>>  	  will be called leds-nic78bx.
>>>>  
>>>> +config LEDS_LM3697
>>>> +	tristate "LED driver for LM3697"
>>>> +	depends on LEDS_TI_LMU_COMMON
>>>> +	help
>>>> +	  Say Y to enable the LM3697 LED driver for TI LMU devices.
>>>> +	  This supports the LED device LM3697.
>>>> +
>>>>  config LEDS_TI_LMU_COMMON
>>>>  	tristate "LED driver for TI LMU"
>>>> -	depends on REGMAP
>>>>  	help
>>>>            Say Y to enable the LED driver for TI LMU devices.
>>>>            This supports common features between the TI LM3532, LM3631, LM3632,
> ...
>>> Plus we'd normally expect "COMMON" first and then specific driver. Not
>>> sure if Kconfig can handle it out-of-order...
>>
>> OK.  Should I rename the ti_lmu file to leds-common-ti-lmu?
> 
> Oh, no, that is not what I meant.
> 
> You have
> 
> config B
>        depends on A
> 
> config A
> 
> above. We really want
> 
> config A
> 
> config B
>        depends on A
> 

Got it I will rearrange that in v2.

>> This keeps the naming convention the same in the leds directory as well.
>>
>> FYI I will not add your acked-by on the LMU patch that introduced the code unless you approve.
>> Since you found issues with the kconfig
> 
> I did not review it in great detail, so no acked-by's, yet :-).

Thanks
Dan

> 									Pavel
> 

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 5/5] leds: lm3697: Introduce the lm3697 driver
@ 2019-04-15 19:53           ` Dan Murphy
  0 siblings, 0 replies; 38+ messages in thread
From: Dan Murphy @ 2019-04-15 19:53 UTC (permalink / raw)
  To: Pavel Machek; +Cc: robh+dt, jacek.anaszewski, linux-kernel, linux-leds



On 4/15/19 9:03 AM, Pavel Machek wrote:
> 
>>>> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
>>>> index 735009e73414..688bb9a6f275 100644
>>>> --- a/drivers/leds/Kconfig
>>>> +++ b/drivers/leds/Kconfig
>>>> @@ -776,9 +776,15 @@ config LEDS_NIC78BX
>>>>  	  To compile this driver as a module, choose M here: the module
>>>>  	  will be called leds-nic78bx.
>>>>  
>>>> +config LEDS_LM3697
>>>> +	tristate "LED driver for LM3697"
>>>> +	depends on LEDS_TI_LMU_COMMON
>>>> +	help
>>>> +	  Say Y to enable the LM3697 LED driver for TI LMU devices.
>>>> +	  This supports the LED device LM3697.
>>>> +
>>>>  config LEDS_TI_LMU_COMMON
>>>>  	tristate "LED driver for TI LMU"
>>>> -	depends on REGMAP
>>>>  	help
>>>>            Say Y to enable the LED driver for TI LMU devices.
>>>>            This supports common features between the TI LM3532, LM3631, LM3632,
> ...
>>> Plus we'd normally expect "COMMON" first and then specific driver. Not
>>> sure if Kconfig can handle it out-of-order...
>>
>> OK.  Should I rename the ti_lmu file to leds-common-ti-lmu?
> 
> Oh, no, that is not what I meant.
> 
> You have
> 
> config B
>        depends on A
> 
> config A
> 
> above. We really want
> 
> config A
> 
> config B
>        depends on A
> 

Got it I will rearrange that in v2.

>> This keeps the naming convention the same in the leds directory as well.
>>
>> FYI I will not add your acked-by on the LMU patch that introduced the code unless you approve.
>> Since you found issues with the kconfig
> 
> I did not review it in great detail, so no acked-by's, yet :-).

Thanks
Dan

> 									Pavel
> 

^ permalink raw reply	[flat|nested] 38+ messages in thread

end of thread, other threads:[~2019-04-15 19:53 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-25 14:23 [PATCH 0/5] TI LMU rework Dan Murphy
2019-03-25 14:23 ` Dan Murphy
2019-03-25 14:23 ` [PATCH 1/5] dt-bindings: mfd: Update the ramp up/down property Dan Murphy
2019-03-25 14:23   ` Dan Murphy
2019-04-04  8:19   ` Lee Jones
2019-04-04 12:07     ` Dan Murphy
2019-04-04 12:07       ` Dan Murphy
2019-03-25 14:24 ` [PATCH 2/5] leds: TI LMU: Add common code for TI LMU devices Dan Murphy
2019-03-25 14:24   ` Dan Murphy
2019-04-04 13:22   ` Pavel Machek
2019-03-25 14:24 ` [PATCH 3/5] dt-bindings: ti-lmu: Modify dt bindings for the LM3697 Dan Murphy
2019-03-25 14:24   ` Dan Murphy
2019-04-03 20:10   ` Jacek Anaszewski
2019-04-03 20:23     ` Dan Murphy
2019-04-03 20:23       ` Dan Murphy
2019-04-04 18:39       ` Jacek Anaszewski
2019-04-04 19:27         ` Dan Murphy
2019-04-04 19:27           ` Dan Murphy
2019-04-04 19:31           ` Jacek Anaszewski
2019-04-04 19:34           ` Dan Murphy
2019-04-04 19:34             ` Dan Murphy
2019-03-25 14:24 ` [PATCH 4/5] mfd: ti-lmu: Remove support for LM3697 Dan Murphy
2019-03-25 14:24   ` Dan Murphy
2019-04-04  8:17   ` Lee Jones
2019-04-04  8:18     ` Lee Jones
2019-04-04 12:09     ` Dan Murphy
2019-04-04 12:09       ` Dan Murphy
2019-03-25 14:24 ` [PATCH 5/5] leds: lm3697: Introduce the lm3697 driver Dan Murphy
2019-03-25 14:24   ` Dan Murphy
2019-04-13 20:06   ` Pavel Machek
2019-04-15 12:49     ` Dan Murphy
2019-04-15 12:49       ` Dan Murphy
2019-04-15 14:03       ` Pavel Machek
2019-04-15 19:53         ` Dan Murphy
2019-04-15 19:53           ` Dan Murphy
2019-04-03 12:02 ` [PATCH 0/5] TI LMU rework Dan Murphy
2019-04-03 12:02   ` Dan Murphy
2019-04-03 20:14   ` 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.