linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/9] TI LMU common Framework
@ 2018-10-11 16:51 Dan Murphy
  2018-10-11 16:51 ` [PATCH v3 1/9] leds: add TI LMU backlight driver Dan Murphy
                   ` (8 more replies)
  0 siblings, 9 replies; 28+ messages in thread
From: Dan Murphy @ 2018-10-11 16:51 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel
  Cc: lee.jones, tony, devicetree, linux-kernel, linux-leds, Dan Murphy

All

I have updated the patch set and addressed some of the comments.
Thanks to Pavel, he found the lm3639 back light driver that is similar
enough to the LM3632 device.  So I removed that device from this patch series.

I am in the process of adding the LM3632 to the LM3639 back light driver as well
as adding additional improvements to that driver.

Dan

Dan Murphy (8):
  dt-bindings: ti-lmu: Remove LM3697
  mfd: ti-lmu: Remove support for LM3697
  dt-bindings: leds: Add bindings for lm3697 driver
  leds: lm3697: Introduce the lm3697 driver
  dt-bindings: ti-lmu: Remove LM3633
  mfd: ti-lmu: Remove support for LM3633
  dt-bindings: leds: Add support for the LM3633
  leds: lm3633: Introduce the lm3633 driver

Pavel Machek (1):
  leds: add TI LMU backlight driver

 .../devicetree/bindings/leds/leds-lm3633.txt  | 102 ++++
 .../devicetree/bindings/leds/leds-lm3697.txt  |  98 +++
 .../devicetree/bindings/mfd/ti-lmu.txt        |  72 ---
 drivers/leds/Kconfig                          |  22 +
 drivers/leds/Makefile                         |   3 +
 drivers/leds/leds-lm3633.c                    | 556 ++++++++++++++++++
 drivers/leds/leds-lm3697.c                    | 381 ++++++++++++
 drivers/leds/ti-lmu-led-common.c              | 138 +++++
 drivers/mfd/Kconfig                           |   2 +-
 drivers/mfd/ti-lmu.c                          |  38 --
 include/linux/mfd/ti-lmu-register.h           |  44 --
 include/linux/mfd/ti-lmu.h                    |   1 -
 include/linux/ti-lmu-led-common.h             |  59 ++
 13 files changed, 1360 insertions(+), 156 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3633.txt
 create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3697.txt
 create mode 100644 drivers/leds/leds-lm3633.c
 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] 28+ messages in thread

* [PATCH v3 1/9] leds: add TI LMU backlight driver
  2018-10-11 16:51 [PATCH v3 0/9] TI LMU common Framework Dan Murphy
@ 2018-10-11 16:51 ` Dan Murphy
  2018-10-11 16:51 ` [PATCH v3 2/9] dt-bindings: ti-lmu: Remove LM3697 Dan Murphy
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 28+ messages in thread
From: Dan Murphy @ 2018-10-11 16:51 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel
  Cc: lee.jones, tony, devicetree, linux-kernel, linux-leds, Milo Kim,
	Sebastian Reichel

From: Pavel Machek <pavel@ucw.cz>

This adds backlight support for the following TI LMU
chips: LM3532, LM3631, LM3632, LM3633, LM3695 and LM3697.

It controls LEDs on Droid 4
smartphone, including keyboard and screen backlights.

Signed-off-by: Milo Kim <milo.kim@ti.com>
[add LED subsystem support for keyboard backlight and rework DT
binding according to Rob Herrings feedback]
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
[remove backlight subsystem support for now]
Signed-off-by: Pavel Machek <pavel@ucw.cz>
---
 drivers/leds/Kconfig              |   8 ++
 drivers/leds/Makefile             |   1 +
 drivers/leds/ti-lmu-led-common.c  | 138 ++++++++++++++++++++++++++++++
 include/linux/ti-lmu-led-common.h |  59 +++++++++++++
 4 files changed, 206 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 44097a3e0fcc..dc717b30d9d3 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -756,6 +756,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 420b5d2cfa62..e09bb27bc7ea 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -78,6 +78,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..a4435bd1d248
--- /dev/null
+++ b/drivers/leds/ti-lmu-led-common.c
@@ -0,0 +1,138 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2015 Texas Instruments
+ * Copyright 2018 Sebastian Reichel
+ * Copyright 2018 Pavel Machek <pavel@ucw.cz>
+ *
+ * TI LMU Led driver, based on previous work from
+ * Milo Kim <milo.kim@ti.com>
+ */
+
+#include <linux/bitops.h>
+#include <linux/err.h>
+#include <linux/notifier.h>
+#include <linux/of_device.h>
+#include <linux/platform_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_register(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)
+{
+	lmu_bank->current_brightness = brightness;
+
+	return ti_lmu_common_update_brightness_register(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 LED Driver");
+MODULE_AUTHOR("Sebastian Reichel");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:ti-lmu-led");
diff --git a/include/linux/ti-lmu-led-common.h b/include/linux/ti-lmu-led-common.h
new file mode 100644
index 000000000000..b0fe08f05be2
--- /dev/null
+++ b/include/linux/ti-lmu-led-common.h
@@ -0,0 +1,59 @@
+// 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/i2c.h>
+#include <linux/init.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <uapi/linux/uleds.h>
+
+#define LMU_DUAL_CHANNEL_USED	(BIT(0) | BIT(1))
+#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
+
+#define NUM_DUAL_CHANNEL	2
+
+struct ti_lmu_bank {
+	struct regmap *regmap;
+
+	int bank_id;
+	int fault_monitor_used;
+
+	u8 enable_reg;
+	unsigned long enable_usec;
+
+	int current_brightness;
+	u32 default_brightness;
+	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] 28+ messages in thread

* [PATCH v3 2/9] dt-bindings: ti-lmu: Remove LM3697
  2018-10-11 16:51 [PATCH v3 0/9] TI LMU common Framework Dan Murphy
  2018-10-11 16:51 ` [PATCH v3 1/9] leds: add TI LMU backlight driver Dan Murphy
@ 2018-10-11 16:51 ` Dan Murphy
  2018-10-11 18:27   ` Pavel Machek
  2018-10-12  9:07   ` Lee Jones
  2018-10-11 16:51 ` [PATCH v3 3/9] mfd: ti-lmu: Remove support for LM3697 Dan Murphy
                   ` (6 subsequent siblings)
  8 siblings, 2 replies; 28+ messages in thread
From: Dan Murphy @ 2018-10-11 16:51 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel
  Cc: lee.jones, tony, devicetree, linux-kernel, linux-leds, Dan Murphy

Remove support for the LM3697 LED device
from the ti-lmu.  The LM3697 will be supported
via a stand alone LED driver.

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

diff --git a/Documentation/devicetree/bindings/mfd/ti-lmu.txt b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
index c885cf89b8ce..920f910be4e9 100644
--- a/Documentation/devicetree/bindings/mfd/ti-lmu.txt
+++ b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
@@ -9,7 +9,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:
@@ -18,11 +17,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
          0x38 for LM3532
          0x63 for LM3695
 
@@ -38,7 +36,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].
@@ -220,24 +217,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-msec = <200>;
-			ramp-down-msec = <200>;
-		};
-	};
-
-	fault-monitor {
-		compatible = "ti,lm3697-fault-monitor";
-	};
-};
-- 
2.19.0


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

* [PATCH v3 3/9] mfd: ti-lmu: Remove support for LM3697
  2018-10-11 16:51 [PATCH v3 0/9] TI LMU common Framework Dan Murphy
  2018-10-11 16:51 ` [PATCH v3 1/9] leds: add TI LMU backlight driver Dan Murphy
  2018-10-11 16:51 ` [PATCH v3 2/9] dt-bindings: ti-lmu: Remove LM3697 Dan Murphy
@ 2018-10-11 16:51 ` Dan Murphy
  2018-10-11 16:51 ` [PATCH v3 4/9] dt-bindings: leds: Add bindings for lm3697 driver Dan Murphy
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 28+ messages in thread
From: Dan Murphy @ 2018-10-11 16:51 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel
  Cc: lee.jones, tony, devicetree, 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 dc717b30d9d3..853014dbb1c2 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -762,7 +762,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 11841f4b7b2b..9b04dd527c68 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1293,7 +1293,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 cfb411cde51c..b6bfa99a29dd 100644
--- a/drivers/mfd/ti-lmu.c
+++ b/drivers/mfd/ti-lmu.c
@@ -128,20 +128,6 @@ static struct mfd_cell lm3695_devices[] = {
 	},
 };
 
-static 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 =	\
 {						\
@@ -155,7 +141,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 const struct of_device_id ti_lmu_of_match[] = {
 	{ .compatible = "ti,lm3532", .data = &lm3532_data },
@@ -163,7 +148,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);
@@ -237,7 +221,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 2125c7c02818..99711ff4b809 100644
--- a/include/linux/mfd/ti-lmu-register.h
+++ b/include/linux/mfd/ti-lmu-register.h
@@ -233,48 +233,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 09d5f30384e5..bc9272f08f47 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] 28+ messages in thread

* [PATCH v3 4/9] dt-bindings: leds: Add bindings for lm3697 driver
  2018-10-11 16:51 [PATCH v3 0/9] TI LMU common Framework Dan Murphy
                   ` (2 preceding siblings ...)
  2018-10-11 16:51 ` [PATCH v3 3/9] mfd: ti-lmu: Remove support for LM3697 Dan Murphy
@ 2018-10-11 16:51 ` Dan Murphy
  2018-10-12 16:27   ` Rob Herring
  2018-10-11 16:51 ` [PATCH v3 5/9] leds: lm3697: Introduce the " Dan Murphy
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 28+ messages in thread
From: Dan Murphy @ 2018-10-11 16:51 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel
  Cc: lee.jones, tony, devicetree, linux-kernel, linux-leds, Dan Murphy

Add the device tree bindings for the lm3697
LED driver for backlighting and display.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 .../devicetree/bindings/leds/leds-lm3697.txt  | 98 +++++++++++++++++++
 1 file changed, 98 insertions(+)
 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..4bb2ed51025b
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-lm3697.txt
@@ -0,0 +1,98 @@
+* 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.  Each element in the array is associated
+			with a specific HVLED string.  Element 0 is HVLED1,
+			element 1 is HVLED2 and element 2 HVLED3.
+			Additional information is contained
+			in Documentation/devicetree/bindings/leds/common.txt
+			0 - HVLED is not active in this control bank
+			1 - HVLED string is controlled by this control bank
+
+Optional child properties:
+	- runtime-ramp-up-msec: Current ramping from one brightness level to
+				the a higher brightness level.
+				Range from 2048 us - 117.44 s
+	- runtime-ramp-down-msec: Current ramping from one brightness level to
+				  the a lower brightness level.
+				  Range from 2048 us - 117.44 s
+	- 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";
+	reg = <0x36>;
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
+	vled-supply = <&vbatt>;
+
+	led@0 {
+		reg = <0>;
+		led-sources = <1 0 1>;
+		runtime-ramp-up-msec = <5000>;
+		runtime-ramp-down-msec = <1000>;
+		label = "white:first_backlight_cluster";
+		linux,default-trigger = "backlight";
+	};
+
+	led@1 {
+		reg = <1>;
+		led-sources = <0 1 0>;
+		runtime-ramp-up-msec = <500>;
+		runtime-ramp-down-msec = <1000>;
+		label = "white:second_backlight_cluster";
+		linux,default-trigger = "backlight";
+	};
+}
+
+All HVLED strings controlled by control bank A
+
+led-controller@36 {
+	compatible = "ti,lm3697";
+	reg = <0x36>;
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
+	vled-supply = <&vbatt>;
+
+	led@0 {
+		reg = <0>;
+		led-sources = <1 1 1>;
+		runtime-ramp-up-msec = <500>;
+		runtime-ramp-down-msec = <1000>;
+		label = "white:backlight_cluster";
+		linux,default-trigger = "backlight";
+	};
+}
+
+For more product information please see the link below:
+http://www.ti.com/lit/ds/symlink/lm3697.pdf
-- 
2.19.0


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

* [PATCH v3 5/9] leds: lm3697: Introduce the lm3697 driver
  2018-10-11 16:51 [PATCH v3 0/9] TI LMU common Framework Dan Murphy
                   ` (3 preceding siblings ...)
  2018-10-11 16:51 ` [PATCH v3 4/9] dt-bindings: leds: Add bindings for lm3697 driver Dan Murphy
@ 2018-10-11 16:51 ` Dan Murphy
  2018-10-11 16:51 ` [PATCH v3 6/9] dt-bindings: ti-lmu: Remove LM3633 Dan Murphy
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 28+ messages in thread
From: Dan Murphy @ 2018-10-11 16:51 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel
  Cc: lee.jones, tony, devicetree, 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 | 381 +++++++++++++++++++++++++++++++++++++
 3 files changed, 389 insertions(+), 1 deletion(-)
 create mode 100644 drivers/leds/leds-lm3697.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 853014dbb1c2..7d9a98044be4 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -756,9 +756,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 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 e09bb27bc7ea..6fbce7cfc41c 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -78,6 +78,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..1ed724fb0c43
--- /dev/null
+++ b/drivers/leds/leds-lm3697.c
@@ -0,0 +1,381 @@
+// 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/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
+
+#define LM3697_HVLED_ASSIGNMENT	1
+
+/**
+ * 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;
+};
+
+/**
+ * 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;
+	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;
+	int ret;
+
+	mutex_lock(&led->priv->lock);
+
+	if (led->control_bank == LM3697_CONTROL_A) {
+		led->lmu_data.msb_brightness_reg = LM3697_CTRL_A_BRT_MSB;
+		led->lmu_data.lsb_brightness_reg = LM3697_CTRL_A_BRT_LSB;
+		ctrl_en_val = LM3697_CTRL_A_EN;
+	} else {
+		led->lmu_data.msb_brightness_reg = LM3697_CTRL_B_BRT_MSB;
+		led->lmu_data.lsb_brightness_reg = LM3697_CTRL_B_BRT_LSB;
+		ctrl_en_val = LM3697_CTRL_B_EN;
+	}
+
+	if (brt_val == LED_OFF)
+		ret = regmap_update_bits(led->priv->regmap, LM3697_CTRL_ENABLE,
+					 ctrl_en_val, ~ctrl_en_val);
+	else
+		ret = regmap_update_bits(led->priv->regmap, LM3697_CTRL_ENABLE,
+					 ctrl_en_val, ctrl_en_val);
+
+	ret = ti_lmu_common_set_brightness(&led->lmu_data, brt_val);
+	if (ret)
+		dev_err(&led->priv->client->dev, "Cannot write brightness\n");
+
+	mutex_unlock(&led->priv->lock);
+	return ret;
+}
+
+static int lm3697_set_control_bank(struct lm3697 *priv)
+{
+	u8 control_bank_config = 0;
+	struct lm3697_led *led;
+	int ret, i;
+
+	led = &priv->leds[0];
+	if (led->control_bank == LM3697_CONTROL_A)
+		led = &priv->leds[1];
+
+	for (i = 0; i < LM3697_MAX_LED_STRINGS; i++)
+		if (led->hvled_strings[i] == LM3697_HVLED_ASSIGNMENT)
+			control_bank_config |= 1 << i;
+
+	ret = regmap_write(priv->regmap, LM3697_OUTPUT_CONFIG,
+			   control_bank_config);
+	if (ret)
+		dev_err(&priv->client->dev, "Cannot write OUTPUT config\n");
+
+	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 = lm3697_set_control_bank(priv);
+	if (ret)
+		dev_err(&priv->client->dev, "Setting the CRTL bank failed\n");
+
+	for (i = 0; i < LM3697_MAX_CONTROL_BANKS; i++) {
+		led = &priv->leds[i];
+		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;
+	int control_bank;
+	size_t i = 0;
+	int ret;
+
+	priv->enable_gpio = devm_gpiod_get_optional(&priv->client->dev,
+						   "enable", GPIOD_OUT_LOW);
+	if (IS_ERR(priv->enable_gpio)) {
+		ret = PTR_ERR(priv->enable_gpio);
+		dev_err(&priv->client->dev, "Failed to get enable gpio: %d\n",
+			ret);
+		return ret;
+	}
+
+	priv->regulator = devm_regulator_get(&priv->client->dev, "vled");
+	if (IS_ERR(priv->regulator))
+		priv->regulator = NULL;
+
+	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];
+
+		led->control_bank = control_bank;
+		led->lmu_data.bank_id = control_bank;
+		led->lmu_data.enable_reg = LM3697_CTRL_ENABLE;
+		led->lmu_data.regmap = priv->regmap;
+		if (control_bank == LM3697_CONTROL_A)
+			led->lmu_data.runtime_ramp_reg = LM3697_CTRL_A_RAMP;
+		else
+			led->lmu_data.runtime_ramp_reg = LM3697_CTRL_B_RAMP;
+
+		ret = fwnode_property_read_u32_array(child, "led-sources",
+						    led->hvled_strings,
+						    LM3697_MAX_LED_STRINGS);
+		if (ret) {
+			dev_err(&priv->client->dev, "led-sources property missing\n");
+			fwnode_handle_put(child);
+			goto child_out;
+		}
+
+		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.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] 28+ messages in thread

* [PATCH v3 6/9] dt-bindings: ti-lmu: Remove LM3633
  2018-10-11 16:51 [PATCH v3 0/9] TI LMU common Framework Dan Murphy
                   ` (4 preceding siblings ...)
  2018-10-11 16:51 ` [PATCH v3 5/9] leds: lm3697: Introduce the " Dan Murphy
@ 2018-10-11 16:51 ` Dan Murphy
  2018-10-11 16:51 ` [PATCH v3 7/9] mfd: ti-lmu: Remove support for LM3633 Dan Murphy
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 28+ messages in thread
From: Dan Murphy @ 2018-10-11 16:51 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel
  Cc: lee.jones, tony, devicetree, linux-kernel, linux-leds, Dan Murphy

Remove support for the LM3633 LED device
from the ti-lmu.  The LM3633 will be supported
via a stand alone LED driver.

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

diff --git a/Documentation/devicetree/bindings/mfd/ti-lmu.txt b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
index 920f910be4e9..573e88578d3d 100644
--- a/Documentation/devicetree/bindings/mfd/ti-lmu.txt
+++ b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
@@ -7,7 +7,6 @@ TI LMU driver supports lighting devices below.
   LM3532       Backlight
   LM3631       Backlight and regulator
   LM3632       Backlight and regulator
-  LM3633       Backlight, LED and fault monitor
   LM3695       Backlight
 
 Required properties:
@@ -15,12 +14,10 @@ Required properties:
                 "ti,lm3532"
                 "ti,lm3631"
                 "ti,lm3632"
-                "ti,lm3633"
                 "ti,lm3695"
   - reg: I2C slave address.
          0x11 for LM3632
          0x29 for LM3631
-         0x36 for LM3633
          0x38 for LM3532
          0x63 for LM3695
 
@@ -157,51 +154,6 @@ lm3632@11 {
 	};
 };
 
-lm3633@36 {
-	compatible = "ti,lm3633";
-	reg = <0x36>;
-
-	enable-gpios = <&pioC 2 GPIO_ACTIVE_HIGH>;
-
-	backlight {
-		compatible = "ti,lm3633-backlight";
-
-		main {
-			label = "main_lcd";
-			led-sources = <1 2>;
-			ramp-up-msec = <500>;
-			ramp-down-msec = <500>;
-		};
-
-		front {
-			label = "front_lcd";
-			led-sources = <0>;
-			ramp-up-msec = <1000>;
-			ramp-down-msec = <0>;
-		};
-	};
-
-	leds {
-		compatible = "ti,lm3633-leds";
-
-		chan1 {
-			label = "status";
-			led-sources = <1>;
-			led-max-microamp = <6000>;
-		};
-
-		chan345 {
-			label = "rgb";
-			led-sources = <3 4 5>;
-			led-max-microamp = <10000>;
-		};
-	};
-
-	fault-monitor {
-		compatible = "ti,lm3633-fault-monitor";
-	};
-};
-
 lm3695@63 {
 	compatible = "ti,lm3695";
 	reg = <0x63>;
-- 
2.19.0


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

* [PATCH v3 7/9] mfd: ti-lmu: Remove support for LM3633
  2018-10-11 16:51 [PATCH v3 0/9] TI LMU common Framework Dan Murphy
                   ` (5 preceding siblings ...)
  2018-10-11 16:51 ` [PATCH v3 6/9] dt-bindings: ti-lmu: Remove LM3633 Dan Murphy
@ 2018-10-11 16:51 ` Dan Murphy
  2018-10-11 16:51 ` [PATCH v3 8/9] dt-bindings: leds: Add support for the LM3633 Dan Murphy
  2018-10-11 16:51 ` [PATCH v3 9/9] leds: lm3633: Introduce the lm3633 driver Dan Murphy
  8 siblings, 0 replies; 28+ messages in thread
From: Dan Murphy @ 2018-10-11 16:51 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel
  Cc: lee.jones, tony, devicetree, linux-kernel, linux-leds, Dan Murphy

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

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 drivers/mfd/Kconfig  |  2 +-
 drivers/mfd/ti-lmu.c | 21 ---------------------
 2 files changed, 1 insertion(+), 22 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 9b04dd527c68..225cb3be350c 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1293,7 +1293,7 @@ config MFD_TI_LMU
 	help
 	  Say yes here to enable support for TI LMU chips.
 
-	  TI LMU MFD supports LM3532, LM3631, LM3632, LM3633, and LM3695.
+	  TI LMU MFD supports LM3532, LM3631, LM3632, 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 b6bfa99a29dd..2cf8a23cba52 100644
--- a/drivers/mfd/ti-lmu.c
+++ b/drivers/mfd/ti-lmu.c
@@ -102,24 +102,6 @@ static struct mfd_cell lm3632_devices[] = {
 	},
 };
 
-static struct mfd_cell lm3633_devices[] = {
-	{
-		.name          = "ti-lmu-backlight",
-		.id            = LM3633,
-		.of_compatible = "ti,lm3633-backlight",
-	},
-	{
-		.name          = "lm3633-leds",
-		.of_compatible = "ti,lm3633-leds",
-	},
-	/* Monitoring driver for open/short circuit detection */
-	{
-		.name          = "ti-lmu-fault-monitor",
-		.id            = LM3633,
-		.of_compatible = "ti,lm3633-fault-monitor",
-	},
-};
-
 static struct mfd_cell lm3695_devices[] = {
 	{
 		.name          = "ti-lmu-backlight",
@@ -139,14 +121,12 @@ static const struct ti_lmu_data chip##_data =	\
 TI_LMU_DATA(lm3532, LM3532_MAX_REG);
 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);
 
 static const struct of_device_id ti_lmu_of_match[] = {
 	{ .compatible = "ti,lm3532", .data = &lm3532_data },
 	{ .compatible = "ti,lm3631", .data = &lm3631_data },
 	{ .compatible = "ti,lm3632", .data = &lm3632_data },
-	{ .compatible = "ti,lm3633", .data = &lm3633_data },
 	{ .compatible = "ti,lm3695", .data = &lm3695_data },
 	{ }
 };
@@ -219,7 +199,6 @@ static const struct i2c_device_id ti_lmu_ids[] = {
 	{ "lm3532", LM3532 },
 	{ "lm3631", LM3631 },
 	{ "lm3632", LM3632 },
-	{ "lm3633", LM3633 },
 	{ "lm3695", LM3695 },
 	{ }
 };
-- 
2.19.0


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

* [PATCH v3 8/9] dt-bindings: leds: Add support for the LM3633
  2018-10-11 16:51 [PATCH v3 0/9] TI LMU common Framework Dan Murphy
                   ` (6 preceding siblings ...)
  2018-10-11 16:51 ` [PATCH v3 7/9] mfd: ti-lmu: Remove support for LM3633 Dan Murphy
@ 2018-10-11 16:51 ` Dan Murphy
  2018-10-11 16:51 ` [PATCH v3 9/9] leds: lm3633: Introduce the lm3633 driver Dan Murphy
  8 siblings, 0 replies; 28+ messages in thread
From: Dan Murphy @ 2018-10-11 16:51 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel
  Cc: lee.jones, tony, devicetree, linux-kernel, linux-leds, Dan Murphy

Add support for the TI LM3633 LED driver.
This device has 9 control banks 3 high voltage
LED channels and 6 low voltage LED channels.

Data sheet:
http://www.ti.com/lit/ds/symlink/lm3633.pdf

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 .../devicetree/bindings/leds/leds-lm3633.txt  | 102 ++++++++++++++++++
 1 file changed, 102 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3633.txt

diff --git a/Documentation/devicetree/bindings/leds/leds-lm3633.txt b/Documentation/devicetree/bindings/leds/leds-lm3633.txt
new file mode 100644
index 000000000000..d791b891ea6e
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-lm3633.txt
@@ -0,0 +1,102 @@
+* Texas Instruments - LM3633 Lighting Power Solution for Smartphone Handsets
+
+The LM3633 is a complete power source for
+backlight, keypad, and indicator LEDs in smartphone
+handsets.
+
+This device is suitable for display and keypad Lighting
+
+Required properties:
+	- compatible:
+		"ti,lm3633"
+	- 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 - HVLED is Controlled by bank A
+		1 - HVLED is Controlled by bank B
+		2,3,4 - LVLED1, 2 and 3 are Controlled by bank C, D and E
+		5,6,7 - LVLED4, 5 and 6 are Controlled by bank F, G and H
+	- led-sources : Indicates which LED string is associated to which
+			control bank.
+			0 - LED is not active in this control bank
+			1 - LED string is controlled by this control bank
+
+Optional child properties:
+	- runtime-ramp-up-msec: Current ramping from one brightness level to
+				the a higher brightness level.
+				Range from 2048 us - 117.44 s
+	- runtime-ramp-down-msec: Current ramping from one brightness level to
+				  the a lower brightness level.
+				  Range from 2048 us - 117.44 s
+	- label : see Documentation/devicetree/bindings/leds/common.txt
+	- linux,default-trigger :
+	   see Documentation/devicetree/bindings/leds/common.txt
+
+Example:
+Control bank C output to 3 LVLEDs and Control F, G and H have independent
+controls of the LVLEDs.
+
+led-controller@36 {
+	compatible = "ti,lm3633";
+	reg = <0x36>;
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
+	led@0 {
+		reg = <0>;
+		led-sources = < 1 0 1 >;
+		label = "white:backlight";
+		ramp-up-ms = <100000>;
+		ramp-down-ms = <1000>;
+		linux,default-trigger = "backlight";
+	};
+
+	led@1 {
+		reg = <1>;
+		led-sources = < 0 1 0 >;
+		label = "white:light";
+		ramp-up-ms = <100000>;
+		ramp-down-ms = <1000>;
+	};
+
+	led@2 {
+		reg = <2>;
+		led-sources = <1>;
+		ramp-up-ms = <100000>;
+		ramp-down-ms = <1000>;
+		label = "white:indicator1";
+	};
+
+	led@5 {
+		reg = <5>;
+		led-sources = <1>;
+		ramp-up-ms = <100000>;
+		ramp-down-ms = <1000>;
+		label = "red:light";
+	};
+
+	led@6 {
+		reg = <6>;
+		led-sources = <1>;
+		ramp-up-ms = <41940>;
+		ramp-down-ms = <1000>;
+		label = "green:light";
+	};
+
+	led@7 {
+		reg = <7>;
+		led-sources = <1>;
+		ramp-up-ms = <100000>;
+		ramp-down-ms = <1000>;
+		label = "blue:light";
+	};
+};
+For more product information please see the link below:
+http://www.ti.com/lit/ds/symlink/lm3633.pdf
-- 
2.19.0


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

* [PATCH v3 9/9] leds: lm3633: Introduce the lm3633 driver
  2018-10-11 16:51 [PATCH v3 0/9] TI LMU common Framework Dan Murphy
                   ` (7 preceding siblings ...)
  2018-10-11 16:51 ` [PATCH v3 8/9] dt-bindings: leds: Add support for the LM3633 Dan Murphy
@ 2018-10-11 16:51 ` Dan Murphy
  8 siblings, 0 replies; 28+ messages in thread
From: Dan Murphy @ 2018-10-11 16:51 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel
  Cc: lee.jones, tony, devicetree, linux-kernel, linux-leds, Dan Murphy

Introduce the LED LM3633 driver.  This LED
driver has 9 total LED outputs with runtime
internal ramp configurations.

Data sheet:
http://www.ti.com/lit/ds/symlink/lm3633.pdf

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

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 7d9a98044be4..1f9093e5d902 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -763,8 +763,16 @@ config LEDS_LM3697
           Say Y to enable the LED driver for TI LMU devices.
           This supports the LED device LM3697.
 
+config LEDS_LM3633
+	tristate "LED driver for LM3633"
+ 	depends on LEDS_TI_LMU_COMMON
+	help
+          Say Y to enable the LED driver for TI LMU devices.
+          This supports the LED device LM3633.
+
 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 6fbce7cfc41c..6ec006892fc0 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -79,6 +79,7 @@ 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_LM3633)		+= leds-lm3633.o
 obj-$(CONFIG_LEDS_TI_LMU_COMMON)	+= ti-lmu-led-common.o
 
 # LED SPI Drivers
diff --git a/drivers/leds/leds-lm3633.c b/drivers/leds/leds-lm3633.c
new file mode 100644
index 000000000000..de8eb7b83d20
--- /dev/null
+++ b/drivers/leds/leds-lm3633.c
@@ -0,0 +1,556 @@
+// SPDX-License-Identifier: GPL-2.0
+// TI LM3633 LED chip family driver
+// Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
+
+#include <linux/gpio/consumer.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/ti-lmu-led-common.h>
+#include <linux/regulator/consumer.h>
+
+#define LM3633_REV			0x0
+#define LM3633_RESET			0x1
+#define LM3633_HVLED_OUTPUT_CONFIG	0x10
+#define LM3633_LVLED_OUTPUT_CONFIG	0x11
+
+#define LM3633_CTRL_A_RAMP		0x12
+#define LM3633_CTRL_B_RAMP		0x13
+#define LM3633_CTRL_C_RAMP		0x14
+#define LM3633_CTRL_D_RAMP		0x15
+#define LM3633_CTRL_E_RAMP		0x16
+#define LM3633_CTRL_F_RAMP		0x17
+#define LM3633_CTRL_G_RAMP		0x18
+#define LM3633_CTRL_H_RAMP		0x19
+
+#define LM3633_CTRL_A_B_RT_RAMP		0x1a
+#define LM3633_CTRL_A_B_RAMP_CFG	0x1b
+#define LM3633_CTRL_C_E_RT_RAMP		0x1c
+#define LM3633_CTRL_F_H_RT_RAMP		0x1d
+
+#define LM3633_CTRL_A_B_BRT_CFG		0x16
+#define LM3633_CTRL_A_FS_CURR_CFG	0x17
+#define LM3633_CTRL_B_FS_CURR_CFG	0x18
+#define LM3633_PWM_CFG			0x1c
+
+#define LM3633_CTRL_ENABLE		0x2b
+
+#define LM3633_CTRL_A_BRT_LSB		0x40
+#define LM3633_CTRL_A_BRT_MSB		0x41
+#define LM3633_CTRL_B_BRT_LSB		0x42
+#define LM3633_CTRL_B_BRT_MSB		0x43
+#define LM3633_CTRL_C_BRT		0x44
+#define LM3633_CTRL_D_BRT		0x45
+#define LM3633_CTRL_E_BRT		0x46
+#define LM3633_CTRL_F_BRT		0x47
+#define LM3633_CTRL_G_BRT		0x48
+#define LM3633_CTRL_H_BRT		0x49
+
+#define LM3633_SW_RESET		BIT(0)
+
+#define LM3633_CTRL_A_EN	BIT(0)
+#define LM3633_CTRL_B_EN	BIT(1)
+#define LM3633_CTRL_C_EN	BIT(2)
+#define LM3633_CTRL_D_EN	BIT(3)
+#define LM3633_CTRL_E_EN	BIT(4)
+#define LM3633_CTRL_F_EN	BIT(5)
+#define LM3633_CTRL_G_EN	BIT(6)
+#define LM3633_CTRL_H_EN	BIT(7)
+
+#define LM3633_MAX_HVLED_STRINGS	3
+#define LM3633_MAX_LVLED_STRINGS	6
+
+#define LM3633_CONTROL_A	0
+#define LM3633_CONTROL_B	1
+#define LM3633_CONTROL_C	2
+#define LM3633_CONTROL_D	3
+#define LM3633_CONTROL_E	4
+#define LM3633_CONTROL_F	5
+#define LM3633_CONTROL_G	6
+#define LM3633_CONTROL_H	7
+
+#define LM3633_MAX_CONTROL_BANKS 8
+
+#define LM3633_LED_ASSIGNMENT	1
+
+#define LM3633_CTRL_F_EN_MASK	0x07
+#define LM3633_CTRL_EN_OFFSET	2
+
+/**
+ * struct lm3633_led -
+ * @hvled_strings: Array of high voltage LED strings associated to control bank
+ * @lvled_strings: Array of low voltage LED strings associated to 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
+ */
+struct lm3633_led {
+	u32 hvled_strings[LM3633_MAX_HVLED_STRINGS];
+	u32 lvled_strings[LM3633_MAX_LVLED_STRINGS];
+	char label[LED_MAX_NAME_SIZE];
+	struct led_classdev led_dev;
+	struct lm3633 *priv;
+	struct ti_lmu_bank lmu_data;
+	int control_bank;
+};
+
+/**
+ * struct lm3633 -
+ * @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 lm3633 {
+	struct gpio_desc *enable_gpio;
+	struct regulator *regulator;
+	struct i2c_client *client;
+	struct regmap *regmap;
+	struct device *dev;
+	struct mutex lock;
+	struct lm3633_led leds[];
+};
+
+static const struct reg_default lm3633_reg_defs[] = {
+	{LM3633_HVLED_OUTPUT_CONFIG, 0x6},
+	{LM3633_LVLED_OUTPUT_CONFIG, 0x36},
+	{LM3633_CTRL_A_RAMP, 0x0},
+	{LM3633_CTRL_B_RAMP, 0x0},
+	{LM3633_CTRL_A_B_RT_RAMP, 0x0},
+	{LM3633_CTRL_A_B_RAMP_CFG, 0x0},
+	{LM3633_CTRL_A_B_BRT_CFG, 0x0},
+	{LM3633_CTRL_A_FS_CURR_CFG, 0x13},
+	{LM3633_CTRL_B_FS_CURR_CFG, 0x13},
+	{LM3633_PWM_CFG, 0xc},
+	{LM3633_CTRL_A_BRT_LSB, 0x0},
+	{LM3633_CTRL_A_BRT_MSB, 0x0},
+	{LM3633_CTRL_B_BRT_LSB, 0x0},
+	{LM3633_CTRL_B_BRT_MSB, 0x0},
+	{LM3633_CTRL_ENABLE, 0x0},
+};
+
+static const struct regmap_config lm3633_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+
+	.max_register = LM3633_CTRL_H_BRT,
+	.reg_defaults = lm3633_reg_defs,
+	.num_reg_defaults = ARRAY_SIZE(lm3633_reg_defs),
+	.cache_type = REGCACHE_FLAT,
+};
+
+static int lm3633_brightness_set(struct led_classdev *led_cdev,
+				enum led_brightness brt_val)
+{
+	struct lm3633_led *led = container_of(led_cdev, struct lm3633_led,
+					      led_dev);
+	int ctrl_en_val;
+	int ret;
+
+	mutex_lock(&led->priv->lock);
+
+	switch (led->control_bank) {
+	case LM3633_CONTROL_A:
+		ctrl_en_val = LM3633_CTRL_A_EN;
+		break;
+	case LM3633_CONTROL_B:
+		ctrl_en_val = LM3633_CTRL_B_EN;
+		break;
+	case LM3633_CONTROL_C:
+		ctrl_en_val = LM3633_CTRL_C_EN;
+		break;
+	case LM3633_CONTROL_D:
+		ctrl_en_val = LM3633_CTRL_D_EN;
+		break;
+	case LM3633_CONTROL_E:
+		ctrl_en_val = LM3633_CTRL_E_EN;
+		break;
+	case LM3633_CONTROL_F:
+		ctrl_en_val = LM3633_CTRL_F_EN;
+		break;
+	case LM3633_CONTROL_G:
+		ctrl_en_val = LM3633_CTRL_G_EN;
+		break;
+	case LM3633_CONTROL_H:
+		ctrl_en_val = LM3633_CTRL_H_EN;
+		break;
+	default:
+		dev_err(&led->priv->client->dev, "Cannot write brightness\n");
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (brt_val == LED_OFF)
+		ret = regmap_update_bits(led->priv->regmap, LM3633_CTRL_ENABLE,
+					 ctrl_en_val, ~ctrl_en_val);
+	else
+		ret = regmap_update_bits(led->priv->regmap, LM3633_CTRL_ENABLE,
+					 ctrl_en_val, ctrl_en_val);
+
+	ret = ti_lmu_common_set_brightness(&led->lmu_data, brt_val);
+	if (ret)
+		dev_err(&led->priv->client->dev, "Cannot write brightness\n");
+out:
+	mutex_unlock(&led->priv->lock);
+	return ret;
+}
+
+static void lm3633_set_control_bank_regs(struct lm3633_led *led)
+{
+	switch (led->control_bank) {
+	case LM3633_CONTROL_A:
+		led->lmu_data.msb_brightness_reg = LM3633_CTRL_A_BRT_MSB;
+		led->lmu_data.lsb_brightness_reg = LM3633_CTRL_A_BRT_LSB;
+		led->lmu_data.runtime_ramp_reg = LM3633_CTRL_A_RAMP;
+	case LM3633_CONTROL_B:
+		led->lmu_data.msb_brightness_reg = LM3633_CTRL_B_BRT_MSB;
+		led->lmu_data.lsb_brightness_reg = LM3633_CTRL_B_BRT_LSB;
+		led->lmu_data.runtime_ramp_reg = LM3633_CTRL_B_RAMP;
+		break;
+	case LM3633_CONTROL_C:
+		led->lmu_data.msb_brightness_reg = LM3633_CTRL_C_BRT;
+		led->lmu_data.runtime_ramp_reg = LM3633_CTRL_C_E_RT_RAMP;
+		break;
+	case LM3633_CONTROL_D:
+		led->lmu_data.msb_brightness_reg = LM3633_CTRL_D_BRT;
+		led->lmu_data.runtime_ramp_reg = LM3633_CTRL_C_E_RT_RAMP;
+		break;
+	case LM3633_CONTROL_E:
+		led->lmu_data.msb_brightness_reg = LM3633_CTRL_E_BRT;
+		led->lmu_data.runtime_ramp_reg = LM3633_CTRL_C_E_RT_RAMP;
+		break;
+	case LM3633_CONTROL_F:
+		led->lmu_data.msb_brightness_reg = LM3633_CTRL_F_BRT;
+		led->lmu_data.runtime_ramp_reg = LM3633_CTRL_F_H_RT_RAMP;
+		break;
+	case LM3633_CONTROL_G:
+		led->lmu_data.msb_brightness_reg = LM3633_CTRL_G_BRT;
+		led->lmu_data.runtime_ramp_reg = LM3633_CTRL_F_H_RT_RAMP;
+		break;
+	case LM3633_CONTROL_H:
+		led->lmu_data.msb_brightness_reg = LM3633_CTRL_H_BRT;
+		led->lmu_data.runtime_ramp_reg = LM3633_CTRL_F_H_RT_RAMP;
+		break;
+	default:
+		dev_err(&led->priv->client->dev,
+			"Control bank is out of bounds\n");
+	}
+}
+
+static int lm3633_set_control_bank(struct lm3633 *priv)
+{
+	u8 control_bank_config = 0;
+	struct lm3633_led *led;
+	int ret, i;
+
+	led = &priv->leds[0];
+	if (led->control_bank == LM3633_CONTROL_A) {
+		lm3633_set_control_bank_regs(led);
+		led = &priv->leds[1];
+	}
+
+	if (led->control_bank >= LM3633_CONTROL_C)
+		return 0;
+
+	lm3633_set_control_bank_regs(led);
+	for (i = 0; i < LM3633_MAX_HVLED_STRINGS; i++)
+		if (led->hvled_strings[i] == LM3633_LED_ASSIGNMENT)
+			control_bank_config |= 1 << i;
+
+	ret = regmap_write(priv->regmap, LM3633_HVLED_OUTPUT_CONFIG,
+			   control_bank_config);
+	if (ret)
+		dev_err(&priv->client->dev, "Cannot write OUTPUT config\n");
+
+	return ret;
+}
+
+static int lm3633_set_lvled_control_bank(struct lm3633 *priv)
+{
+	u8 control_bank_config = 0;
+	struct lm3633_led *led;
+	int ret, i;
+
+	for (i = 0; i <= LM3633_MAX_CONTROL_BANKS; i++) {
+		led = &priv->leds[i];
+		if (led) {
+			if (led->control_bank < LM3633_CONTROL_C)
+				continue;
+
+			if (led->lvled_strings[0]) {
+				if (led->control_bank == LM3633_CONTROL_C)
+					control_bank_config = 0x0;
+				else if (led->control_bank == LM3633_CONTROL_F)
+					control_bank_config &= LM3633_CTRL_F_EN_MASK;
+				else
+					control_bank_config |= 1 << (led->control_bank - LM3633_CTRL_EN_OFFSET);
+			}
+		} else
+			continue;
+
+		lm3633_set_control_bank_regs(led);
+	}
+
+	ret = regmap_write(priv->regmap, LM3633_LVLED_OUTPUT_CONFIG,
+			   control_bank_config);
+	if (ret)
+		dev_err(&priv->client->dev, "Cannot write OUTPUT config\n");
+
+	return ret;
+}
+
+static int lm3633_init(struct lm3633 *priv)
+{
+	struct lm3633_led *led;
+	int i, ret;
+
+	if (priv->enable_gpio) {
+		gpiod_direction_output(priv->enable_gpio, 1);
+	} else {
+		ret = regmap_write(priv->regmap, LM3633_RESET, LM3633_SW_RESET);
+		if (ret) {
+			dev_err(&priv->client->dev,
+				"Cannot reset the device\n");
+			goto out;
+		}
+	}
+
+	ret = regmap_write(priv->regmap, LM3633_CTRL_ENABLE, 0x0);
+	if (ret) {
+		dev_err(&priv->client->dev, "Cannot write ctrl enable\n");
+		goto out;
+	}
+
+	ret = lm3633_set_control_bank(priv);
+	if (ret)
+		dev_err(&priv->client->dev, "Setting the CRTL bank failed\n");
+
+	ret = lm3633_set_lvled_control_bank(priv);
+	if (ret)
+		dev_err(&priv->client->dev,
+			"Setting the lvled CRTL bank failed\n");
+
+	for (i = 0; i < LM3633_MAX_CONTROL_BANKS; i++) {
+		led = &priv->leds[i];
+		if (led->lmu_data.runtime_ramp_reg) {
+			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 lm3633_parse_hvled_sources(struct fwnode_handle *child,
+			      struct lm3633_led *led)
+{
+	struct lm3633 *priv = led->priv;
+	int ret;
+
+	ret = fwnode_property_read_u32_array(child, "led-sources",
+			    led->hvled_strings,
+			    LM3633_MAX_HVLED_STRINGS);
+
+	if (ret)
+		dev_err(&priv->client->dev, "Cannot write OUTPUT config\n");
+
+	return ret;
+}
+
+static int lm3633_parse_lvled_sources(struct fwnode_handle *child,
+			      struct lm3633_led *led)
+{
+	struct lm3633 *priv = led->priv;
+	int ret;
+
+	ret = fwnode_property_read_u32_array(child, "led-sources",
+			    led->lvled_strings, 1);
+	if (ret)
+		dev_err(&priv->client->dev, "Cannot write OUTPUT config\n");
+
+	led->lmu_data.max_brightness = MAX_BRIGHTNESS_8BIT;
+
+	return 0;
+}
+
+static int lm3633_probe_dt(struct lm3633 *priv)
+{
+	struct fwnode_handle *child = NULL;
+	struct lm3633_led *led;
+	const char *name;
+	int control_bank;
+	size_t i = 0;
+	int ret;
+
+	priv->enable_gpio = devm_gpiod_get_optional(&priv->client->dev,
+						   "enable", GPIOD_OUT_LOW);
+	if (IS_ERR(priv->enable_gpio)) {
+		ret = PTR_ERR(priv->enable_gpio);
+		dev_err(&priv->client->dev, "Failed to get enable gpio: %d\n",
+			ret);
+		return ret;
+	}
+
+	priv->regulator = devm_regulator_get(&priv->client->dev, "vled");
+	if (IS_ERR(priv->regulator))
+		priv->regulator = NULL;
+
+	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 > LM3633_MAX_CONTROL_BANKS) {
+			dev_err(&priv->client->dev,
+				"reg property is invalid\n");
+			ret = -EINVAL;
+			fwnode_handle_put(child);
+			goto child_out;
+		}
+
+		led = &priv->leds[i];
+		led->control_bank = control_bank;
+		led->lmu_data.bank_id = control_bank;
+		led->lmu_data.regmap = priv->regmap;
+		led->lmu_data.enable_reg = LM3633_CTRL_ENABLE;
+
+		if (control_bank > LM3633_CONTROL_B)
+			lm3633_parse_lvled_sources(child, led);
+		else
+			lm3633_parse_hvled_sources(child, led);
+
+		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.brightness_set_blocking = lm3633_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 lm3633_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct lm3633 *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, &lm3633_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 = lm3633_probe_dt(led);
+	if (ret)
+		return ret;
+
+	return lm3633_init(led);
+}
+
+static int lm3633_remove(struct i2c_client *client)
+{
+	struct lm3633 *led = i2c_get_clientdata(client);
+	int ret;
+
+	ret = regmap_write(led->regmap, LM3633_CTRL_ENABLE, 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 lm3633_id[] = {
+	{ "lm3633", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, lm3633_id);
+
+static const struct of_device_id of_lm3633_leds_match[] = {
+	{ .compatible = "ti,lm3633", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, of_lm3633_leds_match);
+
+static struct i2c_driver lm3633_driver = {
+	.driver = {
+		.name	= "lm3633",
+		.of_match_table = of_lm3633_leds_match,
+	},
+	.probe		= lm3633_probe,
+	.remove		= lm3633_remove,
+	.id_table	= lm3633_id,
+};
+module_i2c_driver(lm3633_driver);
+
+MODULE_DESCRIPTION("Texas Instruments LM3633 LED driver");
+MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com>");
+MODULE_LICENSE("GPL v2");
-- 
2.19.0


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

* Re: [PATCH v3 2/9] dt-bindings: ti-lmu: Remove LM3697
  2018-10-11 16:51 ` [PATCH v3 2/9] dt-bindings: ti-lmu: Remove LM3697 Dan Murphy
@ 2018-10-11 18:27   ` Pavel Machek
  2018-10-11 18:34     ` Dan Murphy
  2018-10-12  9:07   ` Lee Jones
  1 sibling, 1 reply; 28+ messages in thread
From: Pavel Machek @ 2018-10-11 18:27 UTC (permalink / raw)
  To: Dan Murphy
  Cc: robh+dt, jacek.anaszewski, lee.jones, tony, devicetree,
	linux-kernel, linux-leds

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

On Thu 2018-10-11 11:51:16, Dan Murphy wrote:
> Remove support for the LM3697 LED device
> from the ti-lmu.  The LM3697 will be supported
> via a stand alone LED driver.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>

NAK.

								Pavel
								
> index c885cf89b8ce..920f910be4e9 100644
> --- a/Documentation/devicetree/bindings/mfd/ti-lmu.txt
> +++ b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
> @@ -9,7 +9,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:
> @@ -18,11 +17,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
>           0x38 for LM3532
>           0x63 for LM3695
>  
> @@ -38,7 +36,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].
> @@ -220,24 +217,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-msec = <200>;
> -			ramp-down-msec = <200>;
> -		};
> -	};
> -
> -	fault-monitor {
> -		compatible = "ti,lm3697-fault-monitor";
> -	};
> -};

-- 
(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] 28+ messages in thread

* Re: [PATCH v3 2/9] dt-bindings: ti-lmu: Remove LM3697
  2018-10-11 18:27   ` Pavel Machek
@ 2018-10-11 18:34     ` Dan Murphy
  2018-10-11 19:38       ` Jacek Anaszewski
  0 siblings, 1 reply; 28+ messages in thread
From: Dan Murphy @ 2018-10-11 18:34 UTC (permalink / raw)
  To: Pavel Machek
  Cc: robh+dt, jacek.anaszewski, lee.jones, tony, devicetree,
	linux-kernel, linux-leds

Pavel

On 10/11/2018 01:27 PM, Pavel Machek wrote:
> On Thu 2018-10-11 11:51:16, Dan Murphy wrote:
>> Remove support for the LM3697 LED device
>> from the ti-lmu.  The LM3697 will be supported
>> via a stand alone LED driver.
>>
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> 
> NAK.

Thanks for the NAK.

This NAK was NAK'd by other maintainer in the V2 RFC patchset

https://lore.kernel.org/patchwork/patch/993171/

> 
> 								Pavel
> 								
>> index c885cf89b8ce..920f910be4e9 100644
>> --- a/Documentation/devicetree/bindings/mfd/ti-lmu.txt
>> +++ b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
>> @@ -9,7 +9,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:
>> @@ -18,11 +17,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
>>           0x38 for LM3532
>>           0x63 for LM3695
>>  
>> @@ -38,7 +36,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].
>> @@ -220,24 +217,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-msec = <200>;
>> -			ramp-down-msec = <200>;
>> -		};
>> -	};
>> -
>> -	fault-monitor {
>> -		compatible = "ti,lm3697-fault-monitor";
>> -	};
>> -};
> 


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

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

* Re: [PATCH v3 2/9] dt-bindings: ti-lmu: Remove LM3697
  2018-10-11 18:34     ` Dan Murphy
@ 2018-10-11 19:38       ` Jacek Anaszewski
  2018-10-12 16:26         ` Rob Herring
  0 siblings, 1 reply; 28+ messages in thread
From: Jacek Anaszewski @ 2018-10-11 19:38 UTC (permalink / raw)
  To: Dan Murphy, Pavel Machek
  Cc: robh+dt, lee.jones, tony, devicetree, linux-kernel, linux-leds

On 10/11/2018 08:34 PM, Dan Murphy wrote:
> Pavel
> 
> On 10/11/2018 01:27 PM, Pavel Machek wrote:
>> On Thu 2018-10-11 11:51:16, Dan Murphy wrote:
>>> Remove support for the LM3697 LED device
>>> from the ti-lmu.  The LM3697 will be supported
>>> via a stand alone LED driver.
>>>
>>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>>
>> NAK.
> 
> Thanks for the NAK.
> 
> This NAK was NAK'd by other maintainer in the V2 RFC patchset
> 
> https://lore.kernel.org/patchwork/patch/993171/

I confirm. LM3697 is a standalone device and not a cell of any
MFD device.

Waiting for DT maintainer's ack.

Best regards,
Jacek Anaszewski
								
>>> index c885cf89b8ce..920f910be4e9 100644
>>> --- a/Documentation/devicetree/bindings/mfd/ti-lmu.txt
>>> +++ b/Documentation/devicetree/bindings/mfd/ti-lmu.txt
>>> @@ -9,7 +9,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:
>>> @@ -18,11 +17,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
>>>           0x38 for LM3532
>>>           0x63 for LM3695
>>>  
>>> @@ -38,7 +36,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].
>>> @@ -220,24 +217,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-msec = <200>;
>>> -			ramp-down-msec = <200>;
>>> -		};
>>> -	};
>>> -
>>> -	fault-monitor {
>>> -		compatible = "ti,lm3697-fault-monitor";
>>> -	};
>>> -};
>>
> 
> 


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

* Re: [PATCH v3 2/9] dt-bindings: ti-lmu: Remove LM3697
  2018-10-11 16:51 ` [PATCH v3 2/9] dt-bindings: ti-lmu: Remove LM3697 Dan Murphy
  2018-10-11 18:27   ` Pavel Machek
@ 2018-10-12  9:07   ` Lee Jones
  1 sibling, 0 replies; 28+ messages in thread
From: Lee Jones @ 2018-10-12  9:07 UTC (permalink / raw)
  To: Dan Murphy
  Cc: robh+dt, jacek.anaszewski, pavel, tony, devicetree, linux-kernel,
	linux-leds

On Thu, 11 Oct 2018, Dan Murphy wrote:

> Remove support for the LM3697 LED device
> from the ti-lmu.  The LM3697 will be supported
> via a stand alone LED driver.

What's with the odd 50 char line wrapping?

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

For the code:

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] 28+ messages in thread

* Re: [PATCH v3 2/9] dt-bindings: ti-lmu: Remove LM3697
  2018-10-11 19:38       ` Jacek Anaszewski
@ 2018-10-12 16:26         ` Rob Herring
  2018-10-12 18:03           ` Pavel Machek
  0 siblings, 1 reply; 28+ messages in thread
From: Rob Herring @ 2018-10-12 16:26 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: Dan Murphy, Pavel Machek, lee.jones, tony, devicetree,
	linux-kernel, linux-leds

On Thu, Oct 11, 2018 at 09:38:53PM +0200, Jacek Anaszewski wrote:
> On 10/11/2018 08:34 PM, Dan Murphy wrote:
> > Pavel
> > 
> > On 10/11/2018 01:27 PM, Pavel Machek wrote:
> >> On Thu 2018-10-11 11:51:16, Dan Murphy wrote:
> >>> Remove support for the LM3697 LED device
> >>> from the ti-lmu.  The LM3697 will be supported
> >>> via a stand alone LED driver.
> >>>
> >>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> >>
> >> NAK.
> > 
> > Thanks for the NAK.
> > 
> > This NAK was NAK'd by other maintainer in the V2 RFC patchset
> > 
> > https://lore.kernel.org/patchwork/patch/993171/
> 
> I confirm. LM3697 is a standalone device and not a cell of any
> MFD device.
> 
> Waiting for DT maintainer's ack.

You all sort out what you want... I can't follow it all, and I'm not 
going to spend the time trying to figure out what is going on here.

As this is worded, changing the driver is a Linux problem and irrelevant 
to the binding. Now if you want to move documentation to a location that 
makes more sense, then fine. But structure patches that way and make it 
clear that from an binding ABI perspective, nothing is changing.

Rob

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

* Re: [PATCH v3 4/9] dt-bindings: leds: Add bindings for lm3697 driver
  2018-10-11 16:51 ` [PATCH v3 4/9] dt-bindings: leds: Add bindings for lm3697 driver Dan Murphy
@ 2018-10-12 16:27   ` Rob Herring
  2018-10-19 11:47     ` Dan Murphy
  0 siblings, 1 reply; 28+ messages in thread
From: Rob Herring @ 2018-10-12 16:27 UTC (permalink / raw)
  To: Dan Murphy
  Cc: jacek.anaszewski, pavel, lee.jones, tony, devicetree,
	linux-kernel, linux-leds

On Thu, Oct 11, 2018 at 11:51:18AM -0500, Dan Murphy wrote:
> Add the device tree bindings for the lm3697
> LED driver for backlighting and display.

Bindings are for h/w, not drivers...

> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>  .../devicetree/bindings/leds/leds-lm3697.txt  | 98 +++++++++++++++++++
>  1 file changed, 98 insertions(+)
>  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..4bb2ed51025b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-lm3697.txt
> @@ -0,0 +1,98 @@
> +* 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.  Each element in the array is associated
> +			with a specific HVLED string.  Element 0 is HVLED1,
> +			element 1 is HVLED2 and element 2 HVLED3.
> +			Additional information is contained
> +			in Documentation/devicetree/bindings/leds/common.txt
> +			0 - HVLED is not active in this control bank
> +			1 - HVLED string is controlled by this control bank
> +
> +Optional child properties:
> +	- runtime-ramp-up-msec: Current ramping from one brightness level to
> +				the a higher brightness level.
> +				Range from 2048 us - 117.44 s
> +	- runtime-ramp-down-msec: Current ramping from one brightness level to
> +				  the a lower brightness level.
> +				  Range from 2048 us - 117.44 s
> +	- 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";
> +	reg = <0x36>;
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
> +	vled-supply = <&vbatt>;
> +
> +	led@0 {
> +		reg = <0>;
> +		led-sources = <1 0 1>;
> +		runtime-ramp-up-msec = <5000>;
> +		runtime-ramp-down-msec = <1000>;
> +		label = "white:first_backlight_cluster";
> +		linux,default-trigger = "backlight";
> +	};
> +
> +	led@1 {
> +		reg = <1>;
> +		led-sources = <0 1 0>;
> +		runtime-ramp-up-msec = <500>;
> +		runtime-ramp-down-msec = <1000>;
> +		label = "white:second_backlight_cluster";
> +		linux,default-trigger = "backlight";
> +	};
> +}
> +
> +All HVLED strings controlled by control bank A
> +
> +led-controller@36 {
> +	compatible = "ti,lm3697";
> +	reg = <0x36>;
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
> +	vled-supply = <&vbatt>;
> +
> +	led@0 {
> +		reg = <0>;
> +		led-sources = <1 1 1>;
> +		runtime-ramp-up-msec = <500>;
> +		runtime-ramp-down-msec = <1000>;
> +		label = "white:backlight_cluster";
> +		linux,default-trigger = "backlight";
> +	};
> +}
> +
> +For more product information please see the link below:
> +http://www.ti.com/lit/ds/symlink/lm3697.pdf
> -- 
> 2.19.0
> 

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

* Re: [PATCH v3 2/9] dt-bindings: ti-lmu: Remove LM3697
  2018-10-12 16:26         ` Rob Herring
@ 2018-10-12 18:03           ` Pavel Machek
  2018-10-13 18:45             ` Jacek Anaszewski
  0 siblings, 1 reply; 28+ messages in thread
From: Pavel Machek @ 2018-10-12 18:03 UTC (permalink / raw)
  To: Rob Herring
  Cc: Jacek Anaszewski, Dan Murphy, lee.jones, tony, devicetree,
	linux-kernel, linux-leds

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

Hi!

> > >>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> > >>
> > >> NAK.
> > > 
> > > Thanks for the NAK.
> > > 
> > > This NAK was NAK'd by other maintainer in the V2 RFC patchset
> > > 
> > > https://lore.kernel.org/patchwork/patch/993171/
> > 
> > I confirm. LM3697 is a standalone device and not a cell of any
> > MFD device.
> > 
> > Waiting for DT maintainer's ack.
> 
> You all sort out what you want... I can't follow it all, and I'm not 
> going to spend the time trying to figure out what is going on here.

This is what I want:

> As this is worded, changing the driver is a Linux problem and irrelevant 
> to the binding. Now if you want to move documentation to a location that 
> makes more sense, then fine. But structure patches that way and make it 
> clear that from an binding ABI perspective, nothing is changing.

...but apparently I did not have enough authority to get it.

(I'm ok with move, and it is possible that binding does need some
fixups besides the move; still it should be done as fixup not as a new
binding).

Thanks,
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

* Re: [PATCH v3 2/9] dt-bindings: ti-lmu: Remove LM3697
  2018-10-12 18:03           ` Pavel Machek
@ 2018-10-13 18:45             ` Jacek Anaszewski
  2018-10-15  0:56               ` Rob Herring
  0 siblings, 1 reply; 28+ messages in thread
From: Jacek Anaszewski @ 2018-10-13 18:45 UTC (permalink / raw)
  To: Pavel Machek, Rob Herring
  Cc: Dan Murphy, lee.jones, tony, devicetree, linux-kernel, linux-leds

On 10/12/2018 08:03 PM, Pavel Machek wrote:
> Hi!
> 
>>>>>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>>>>>
>>>>> NAK.
>>>>
>>>> Thanks for the NAK.
>>>>
>>>> This NAK was NAK'd by other maintainer in the V2 RFC patchset
>>>>
>>>> https://lore.kernel.org/patchwork/patch/993171/
>>>
>>> I confirm. LM3697 is a standalone device and not a cell of any
>>> MFD device.
>>>
>>> Waiting for DT maintainer's ack.
>>
>> You all sort out what you want... I can't follow it all, and I'm not 
>> going to spend the time trying to figure out what is going on here.
> 
> This is what I want:
> 
>> As this is worded, changing the driver is a Linux problem and irrelevant 
>> to the binding. Now if you want to move documentation to a location that 
>> makes more sense, then fine. But structure patches that way and make it 
>> clear that from an binding ABI perspective, nothing is changing.
> 
> ...but apparently I did not have enough authority to get it.
> 
> (I'm ok with move, and it is possible that binding does need some
> fixups besides the move; still it should be done as fixup not as a new
> binding).

There is a fundamental question - should the bindings be considered
an ABI, even though there is no mainline "*.dts" implementation basing
on these bindings?

This patch fixes the issues of bindings in a way that would change
the ABI, if only it existed. But it apparently doesn't exist in
mainline. Unless a DT documentation itself constitutes an ABI.

I'd like to have it clarified at this occasion, and that's why
I kindly ask for DT maintainer's ack or NACK for this modification
of bindings.

For a reference we have a nice summary of the MFD driver and related
bindings' flaws in [0] and [1].

[0] https://lkml.org/lkml/2018/9/7/774
[1] https://lkml.org/lkml/2018/9/11/984

-- 
Best regards,
Jacek Anaszewski

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

* Re: [PATCH v3 2/9] dt-bindings: ti-lmu: Remove LM3697
  2018-10-13 18:45             ` Jacek Anaszewski
@ 2018-10-15  0:56               ` Rob Herring
  2018-10-15 19:13                 ` Jacek Anaszewski
  0 siblings, 1 reply; 28+ messages in thread
From: Rob Herring @ 2018-10-15  0:56 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: Pavel Machek, Dan Murphy, Lee Jones, Tony Lindgren, devicetree,
	linux-kernel, Linux LED Subsystem

On Sat, Oct 13, 2018 at 1:46 PM Jacek Anaszewski
<jacek.anaszewski@gmail.com> wrote:
>
> On 10/12/2018 08:03 PM, Pavel Machek wrote:
> > Hi!
> >
> >>>>>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> >>>>>
> >>>>> NAK.
> >>>>
> >>>> Thanks for the NAK.
> >>>>
> >>>> This NAK was NAK'd by other maintainer in the V2 RFC patchset
> >>>>
> >>>> https://lore.kernel.org/patchwork/patch/993171/
> >>>
> >>> I confirm. LM3697 is a standalone device and not a cell of any
> >>> MFD device.
> >>>
> >>> Waiting for DT maintainer's ack.
> >>
> >> You all sort out what you want... I can't follow it all, and I'm not
> >> going to spend the time trying to figure out what is going on here.
> >
> > This is what I want:
> >
> >> As this is worded, changing the driver is a Linux problem and irrelevant
> >> to the binding. Now if you want to move documentation to a location that
> >> makes more sense, then fine. But structure patches that way and make it
> >> clear that from an binding ABI perspective, nothing is changing.
> >
> > ...but apparently I did not have enough authority to get it.
> >
> > (I'm ok with move, and it is possible that binding does need some
> > fixups besides the move; still it should be done as fixup not as a new
> > binding).
>
> There is a fundamental question - should the bindings be considered
> an ABI, even though there is no mainline "*.dts" implementation basing
> on these bindings?

In theory there could be out of tree users. Having a dts file using it
in tree shouldn't be a requirement to maintain the ABI IMO. However, a
lack of dts is one piece of determining whether this is in use or not.

> This patch fixes the issues of bindings in a way that would change
> the ABI, if only it existed. But it apparently doesn't exist in
> mainline. Unless a DT documentation itself constitutes an ABI.
>
> I'd like to have it clarified at this occasion, and that's why
> I kindly ask for DT maintainer's ack or NACK for this modification
> of bindings.
>
> For a reference we have a nice summary of the MFD driver and related
> bindings' flaws in [0] and [1].
>
> [0] https://lkml.org/lkml/2018/9/7/774
> [1] https://lkml.org/lkml/2018/9/11/984

Given this one seems to have not really been finished, it's probably
okay to make changes in this case. Still, it would be good to see
patches structured so that it's obvious we're breaking things. But how
the patches are structured doesn't matter until there's some agreement
on the end result.

Rob

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

* Re: [PATCH v3 2/9] dt-bindings: ti-lmu: Remove LM3697
  2018-10-15  0:56               ` Rob Herring
@ 2018-10-15 19:13                 ` Jacek Anaszewski
  2018-10-15 19:14                   ` Dan Murphy
  0 siblings, 1 reply; 28+ messages in thread
From: Jacek Anaszewski @ 2018-10-15 19:13 UTC (permalink / raw)
  To: Rob Herring, Dan Murphy
  Cc: Pavel Machek, Lee Jones, Tony Lindgren, devicetree, linux-kernel,
	Linux LED Subsystem

On 10/15/2018 02:56 AM, Rob Herring wrote:
> On Sat, Oct 13, 2018 at 1:46 PM Jacek Anaszewski
> <jacek.anaszewski@gmail.com> wrote:
>>
>> On 10/12/2018 08:03 PM, Pavel Machek wrote:
>>> Hi!
>>>
>>>>>>>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>>>>>>>
>>>>>>> NAK.
>>>>>>
>>>>>> Thanks for the NAK.
>>>>>>
>>>>>> This NAK was NAK'd by other maintainer in the V2 RFC patchset
>>>>>>
>>>>>> https://lore.kernel.org/patchwork/patch/993171/
>>>>>
>>>>> I confirm. LM3697 is a standalone device and not a cell of any
>>>>> MFD device.
>>>>>
>>>>> Waiting for DT maintainer's ack.
>>>>
>>>> You all sort out what you want... I can't follow it all, and I'm not
>>>> going to spend the time trying to figure out what is going on here.
>>>
>>> This is what I want:
>>>
>>>> As this is worded, changing the driver is a Linux problem and irrelevant
>>>> to the binding. Now if you want to move documentation to a location that
>>>> makes more sense, then fine. But structure patches that way and make it
>>>> clear that from an binding ABI perspective, nothing is changing.
>>>
>>> ...but apparently I did not have enough authority to get it.
>>>
>>> (I'm ok with move, and it is possible that binding does need some
>>> fixups besides the move; still it should be done as fixup not as a new
>>> binding).
>>
>> There is a fundamental question - should the bindings be considered
>> an ABI, even though there is no mainline "*.dts" implementation basing
>> on these bindings?
> 
> In theory there could be out of tree users. Having a dts file using it
> in tree shouldn't be a requirement to maintain the ABI IMO. However, a
> lack of dts is one piece of determining whether this is in use or not.
> 
>> This patch fixes the issues of bindings in a way that would change
>> the ABI, if only it existed. But it apparently doesn't exist in
>> mainline. Unless a DT documentation itself constitutes an ABI.
>>
>> I'd like to have it clarified at this occasion, and that's why
>> I kindly ask for DT maintainer's ack or NACK for this modification
>> of bindings.
>>
>> For a reference we have a nice summary of the MFD driver and related
>> bindings' flaws in [0] and [1].
>>
>> [0] https://lkml.org/lkml/2018/9/7/774
>> [1] https://lkml.org/lkml/2018/9/11/984
> 
> Given this one seems to have not really been finished, it's probably
> okay to make changes in this case. Still, it would be good to see
> patches structured so that it's obvious we're breaking things. But how
> the patches are structured doesn't matter until there's some agreement
> on the end result.

OK, so if I'm getting it right, the correct patch structure in this case
means that changes removing bindings from MFD and moving them along
with the modifications to the LED subsystem should be placed in a single
patch.

Dan, could you please arrange the next patch set version accordingly?

-- 
Best regards,
Jacek Anaszewski

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

* Re: [PATCH v3 2/9] dt-bindings: ti-lmu: Remove LM3697
  2018-10-15 19:13                 ` Jacek Anaszewski
@ 2018-10-15 19:14                   ` Dan Murphy
  2018-10-15 21:45                     ` Pavel Machek
  0 siblings, 1 reply; 28+ messages in thread
From: Dan Murphy @ 2018-10-15 19:14 UTC (permalink / raw)
  To: Jacek Anaszewski, Rob Herring
  Cc: Pavel Machek, Lee Jones, Tony Lindgren, devicetree, linux-kernel,
	Linux LED Subsystem

Jacek

On 10/15/2018 02:13 PM, Jacek Anaszewski wrote:
> On 10/15/2018 02:56 AM, Rob Herring wrote:
>> On Sat, Oct 13, 2018 at 1:46 PM Jacek Anaszewski
>> <jacek.anaszewski@gmail.com> wrote:
>>>
>>> On 10/12/2018 08:03 PM, Pavel Machek wrote:
>>>> Hi!
>>>>
>>>>>>>>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>>>>>>>>
>>>>>>>> NAK.
>>>>>>>
>>>>>>> Thanks for the NAK.
>>>>>>>
>>>>>>> This NAK was NAK'd by other maintainer in the V2 RFC patchset
>>>>>>>
>>>>>>> https://lore.kernel.org/patchwork/patch/993171/
>>>>>>
>>>>>> I confirm. LM3697 is a standalone device and not a cell of any
>>>>>> MFD device.
>>>>>>
>>>>>> Waiting for DT maintainer's ack.
>>>>>
>>>>> You all sort out what you want... I can't follow it all, and I'm not
>>>>> going to spend the time trying to figure out what is going on here.
>>>>
>>>> This is what I want:
>>>>
>>>>> As this is worded, changing the driver is a Linux problem and irrelevant
>>>>> to the binding. Now if you want to move documentation to a location that
>>>>> makes more sense, then fine. But structure patches that way and make it
>>>>> clear that from an binding ABI perspective, nothing is changing.
>>>>
>>>> ...but apparently I did not have enough authority to get it.
>>>>
>>>> (I'm ok with move, and it is possible that binding does need some
>>>> fixups besides the move; still it should be done as fixup not as a new
>>>> binding).
>>>
>>> There is a fundamental question - should the bindings be considered
>>> an ABI, even though there is no mainline "*.dts" implementation basing
>>> on these bindings?
>>
>> In theory there could be out of tree users. Having a dts file using it
>> in tree shouldn't be a requirement to maintain the ABI IMO. However, a
>> lack of dts is one piece of determining whether this is in use or not.
>>
>>> This patch fixes the issues of bindings in a way that would change
>>> the ABI, if only it existed. But it apparently doesn't exist in
>>> mainline. Unless a DT documentation itself constitutes an ABI.
>>>
>>> I'd like to have it clarified at this occasion, and that's why
>>> I kindly ask for DT maintainer's ack or NACK for this modification
>>> of bindings.
>>>
>>> For a reference we have a nice summary of the MFD driver and related
>>> bindings' flaws in [0] and [1].
>>>
>>> [0] https://lkml.org/lkml/2018/9/7/774
>>> [1] https://lkml.org/lkml/2018/9/11/984
>>
>> Given this one seems to have not really been finished, it's probably
>> okay to make changes in this case. Still, it would be good to see
>> patches structured so that it's obvious we're breaking things. But how
>> the patches are structured doesn't matter until there's some agreement
>> on the end result.
> 
> OK, so if I'm getting it right, the correct patch structure in this case
> means that changes removing bindings from MFD and moving them along
> with the modifications to the LED subsystem should be placed in a single
> patch.
> 
> Dan, could you please arrange the next patch set version accordingly?

Yes I can squash the DT bindings patches and call it a "move and modify"

Dan

> 


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

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

* Re: [PATCH v3 2/9] dt-bindings: ti-lmu: Remove LM3697
  2018-10-15 19:14                   ` Dan Murphy
@ 2018-10-15 21:45                     ` Pavel Machek
  2018-10-16 13:25                       ` Dan Murphy
  0 siblings, 1 reply; 28+ messages in thread
From: Pavel Machek @ 2018-10-15 21:45 UTC (permalink / raw)
  To: Dan Murphy
  Cc: Jacek Anaszewski, Rob Herring, Lee Jones, Tony Lindgren,
	devicetree, linux-kernel, Linux LED Subsystem

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

Hi!

> >> Given this one seems to have not really been finished, it's probably
> >> okay to make changes in this case. Still, it would be good to see
> >> patches structured so that it's obvious we're breaking things. But how
> >> the patches are structured doesn't matter until there's some agreement
> >> on the end result.
> > 
> > OK, so if I'm getting it right, the correct patch structure in this case
> > means that changes removing bindings from MFD and moving them along
> > with the modifications to the LED subsystem should be placed in a single
> > patch.
> > 
> > Dan, could you please arrange the next patch set version accordingly?
> 
> Yes I can squash the DT bindings patches and call it a "move and modify"

You can do move without problems. But if you plan to modify them,
please try to change as little as possible, make it separate patch and
explain why new version is better than old one.

Thanks,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

* Re: [PATCH v3 2/9] dt-bindings: ti-lmu: Remove LM3697
  2018-10-15 21:45                     ` Pavel Machek
@ 2018-10-16 13:25                       ` Dan Murphy
  2018-10-18 22:10                         ` Pavel Machek
  0 siblings, 1 reply; 28+ messages in thread
From: Dan Murphy @ 2018-10-16 13:25 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Jacek Anaszewski, Rob Herring, Lee Jones, Tony Lindgren,
	devicetree, linux-kernel, Linux LED Subsystem

Thanks

On 10/15/2018 04:45 PM, Pavel Machek wrote:
> Hi!
> 
>>>> Given this one seems to have not really been finished, it's probably
>>>> okay to make changes in this case. Still, it would be good to see
>>>> patches structured so that it's obvious we're breaking things. But how
>>>> the patches are structured doesn't matter until there's some agreement
>>>> on the end result.
>>>
>>> OK, so if I'm getting it right, the correct patch structure in this case
>>> means that changes removing bindings from MFD and moving them along
>>> with the modifications to the LED subsystem should be placed in a single
>>> patch.
>>>
>>> Dan, could you please arrange the next patch set version accordingly?
>>
>> Yes I can squash the DT bindings patches and call it a "move and modify"
> 
> You can do move without problems. But if you plan to modify them,
> please try to change as little as possible, make it separate patch and
> explain why new version is better than old one.
> 

I have been thinking of how to do this.  Since the 2 devices are part of the current
binding there really is not a way to move them.  Since there are still MFD capable
devices available.

I would still need to remove the current active binding and create a new binding in the LED
bindings directory.

I would have to remove and create in the same patch.

Dan

> Thanks,
> 									Pavel
> 


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

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

* Re: [PATCH v3 2/9] dt-bindings: ti-lmu: Remove LM3697
  2018-10-16 13:25                       ` Dan Murphy
@ 2018-10-18 22:10                         ` Pavel Machek
  2018-10-19 11:42                           ` Dan Murphy
  0 siblings, 1 reply; 28+ messages in thread
From: Pavel Machek @ 2018-10-18 22:10 UTC (permalink / raw)
  To: Dan Murphy
  Cc: Jacek Anaszewski, Rob Herring, Lee Jones, Tony Lindgren,
	devicetree, linux-kernel, Linux LED Subsystem

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

Hi!

> >>>> Given this one seems to have not really been finished, it's probably
> >>>> okay to make changes in this case. Still, it would be good to see
> >>>> patches structured so that it's obvious we're breaking things. But how
> >>>> the patches are structured doesn't matter until there's some agreement
> >>>> on the end result.
> >>>
> >>> OK, so if I'm getting it right, the correct patch structure in this case
> >>> means that changes removing bindings from MFD and moving them along
> >>> with the modifications to the LED subsystem should be placed in a single
> >>> patch.
> >>>
> >>> Dan, could you please arrange the next patch set version accordingly?
> >>
> >> Yes I can squash the DT bindings patches and call it a "move and modify"
> > 
> > You can do move without problems. But if you plan to modify them,
> > please try to change as little as possible, make it separate patch and
> > explain why new version is better than old one.
> > 
> 
> I have been thinking of how to do this.  Since the 2 devices are part of the current
> binding there really is not a way to move them.  Since there are still MFD capable
> devices available.
> 
> I would still need to remove the current active binding and create a new binding in the LED
> bindings directory.
> 
> I would have to remove and create in the same patch.

Yeah, and this all is a sign that you should just keep the current
binding, and make your code use it, see?

You are free to use Sebastian's updated binding. It has "suggested by:
Rob" or something like that on it, so it should be fine.

You can add note to bindings/leds pointing to mfd binding.

Now... this is what I've suggested before. If you don't agree, you may
want to contact Tony Lindgren, IIRC he works for TI, too, and might be
willing to help.

Thank you,
									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] 28+ messages in thread

* Re: [PATCH v3 2/9] dt-bindings: ti-lmu: Remove LM3697
  2018-10-18 22:10                         ` Pavel Machek
@ 2018-10-19 11:42                           ` Dan Murphy
  2018-10-19 14:58                             ` Tony Lindgren
  0 siblings, 1 reply; 28+ messages in thread
From: Dan Murphy @ 2018-10-19 11:42 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Jacek Anaszewski, Rob Herring, Lee Jones, Tony Lindgren,
	devicetree, linux-kernel, Linux LED Subsystem

On 10/18/2018 05:10 PM, Pavel Machek wrote:
> Hi!
> 
>>>>>> Given this one seems to have not really been finished, it's probably
>>>>>> okay to make changes in this case. Still, it would be good to see
>>>>>> patches structured so that it's obvious we're breaking things. But how
>>>>>> the patches are structured doesn't matter until there's some agreement
>>>>>> on the end result.
>>>>>
>>>>> OK, so if I'm getting it right, the correct patch structure in this case
>>>>> means that changes removing bindings from MFD and moving them along
>>>>> with the modifications to the LED subsystem should be placed in a single
>>>>> patch.
>>>>>
>>>>> Dan, could you please arrange the next patch set version accordingly?
>>>>
>>>> Yes I can squash the DT bindings patches and call it a "move and modify"
>>>
>>> You can do move without problems. But if you plan to modify them,
>>> please try to change as little as possible, make it separate patch and
>>> explain why new version is better than old one.
>>>
>>
>> I have been thinking of how to do this.  Since the 2 devices are part of the current
>> binding there really is not a way to move them.  Since there are still MFD capable
>> devices available.
>>
>> I would still need to remove the current active binding and create a new binding in the LED
>> bindings directory.
>>
>> I would have to remove and create in the same patch.
> 
> Yeah, and this all is a sign that you should just keep the current
> binding, and make your code use it, see?


No I don't actually see this as a sign.  This is just operational issues
nothing to do with actually correcting the incomplete unused bindings.

And actually moving and creating the bindings in the same patch makes
sense as the change is self contained in a single patch and is easier to
track.

> 
> You are free to use Sebastian's updated binding. It has "suggested by:
> Rob" or something like that on it, so it should be fine.
> 
> You can add note to bindings/leds pointing to mfd binding.
> 
> Now... this is what I've suggested before. If you don't agree, you may
> want to contact Tony Lindgren, IIRC he works for TI, too, and might be
> willing to help.

I will ping Tony just to close the loop.  I will be posting v4 today after making the changes.
I was hoping to have some code review prior to posting v4 but have not received any comments so
v4 will just be a patch rearrangement.

Dan

> 
> Thank you,
> 									Pavel
> 


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

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

* Re: [PATCH v3 4/9] dt-bindings: leds: Add bindings for lm3697 driver
  2018-10-12 16:27   ` Rob Herring
@ 2018-10-19 11:47     ` Dan Murphy
  0 siblings, 0 replies; 28+ messages in thread
From: Dan Murphy @ 2018-10-19 11:47 UTC (permalink / raw)
  To: Rob Herring
  Cc: jacek.anaszewski, pavel, lee.jones, tony, devicetree,
	linux-kernel, linux-leds

Rob

Thanks for the review.

On 10/12/2018 11:27 AM, Rob Herring wrote:
> On Thu, Oct 11, 2018 at 11:51:18AM -0500, Dan Murphy wrote:
>> Add the device tree bindings for the lm3697
>> LED driver for backlighting and display.
> 
> Bindings are for h/w, not drivers...
> 

ACK

Dan
>>
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>> ---
>>  .../devicetree/bindings/leds/leds-lm3697.txt  | 98 +++++++++++++++++++
>>  1 file changed, 98 insertions(+)
>>  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..4bb2ed51025b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/leds/leds-lm3697.txt
>> @@ -0,0 +1,98 @@
>> +* 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.  Each element in the array is associated
>> +			with a specific HVLED string.  Element 0 is HVLED1,
>> +			element 1 is HVLED2 and element 2 HVLED3.
>> +			Additional information is contained
>> +			in Documentation/devicetree/bindings/leds/common.txt
>> +			0 - HVLED is not active in this control bank
>> +			1 - HVLED string is controlled by this control bank
>> +
>> +Optional child properties:
>> +	- runtime-ramp-up-msec: Current ramping from one brightness level to
>> +				the a higher brightness level.
>> +				Range from 2048 us - 117.44 s
>> +	- runtime-ramp-down-msec: Current ramping from one brightness level to
>> +				  the a lower brightness level.
>> +				  Range from 2048 us - 117.44 s
>> +	- 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";
>> +	reg = <0x36>;
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +
>> +	enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
>> +	vled-supply = <&vbatt>;
>> +
>> +	led@0 {
>> +		reg = <0>;
>> +		led-sources = <1 0 1>;
>> +		runtime-ramp-up-msec = <5000>;
>> +		runtime-ramp-down-msec = <1000>;
>> +		label = "white:first_backlight_cluster";
>> +		linux,default-trigger = "backlight";
>> +	};
>> +
>> +	led@1 {
>> +		reg = <1>;
>> +		led-sources = <0 1 0>;
>> +		runtime-ramp-up-msec = <500>;
>> +		runtime-ramp-down-msec = <1000>;
>> +		label = "white:second_backlight_cluster";
>> +		linux,default-trigger = "backlight";
>> +	};
>> +}
>> +
>> +All HVLED strings controlled by control bank A
>> +
>> +led-controller@36 {
>> +	compatible = "ti,lm3697";
>> +	reg = <0x36>;
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +
>> +	enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
>> +	vled-supply = <&vbatt>;
>> +
>> +	led@0 {
>> +		reg = <0>;
>> +		led-sources = <1 1 1>;
>> +		runtime-ramp-up-msec = <500>;
>> +		runtime-ramp-down-msec = <1000>;
>> +		label = "white:backlight_cluster";
>> +		linux,default-trigger = "backlight";
>> +	};
>> +}
>> +
>> +For more product information please see the link below:
>> +http://www.ti.com/lit/ds/symlink/lm3697.pdf
>> -- 
>> 2.19.0
>>


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

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

* Re: [PATCH v3 2/9] dt-bindings: ti-lmu: Remove LM3697
  2018-10-19 11:42                           ` Dan Murphy
@ 2018-10-19 14:58                             ` Tony Lindgren
  2018-10-24  8:55                               ` Pavel Machek
  0 siblings, 1 reply; 28+ messages in thread
From: Tony Lindgren @ 2018-10-19 14:58 UTC (permalink / raw)
  To: Dan Murphy
  Cc: Pavel Machek, Jacek Anaszewski, Rob Herring, Lee Jones,
	devicetree, linux-kernel, Linux LED Subsystem

* Dan Murphy <dmurphy@ti.com> [181019 11:42]:
> On 10/18/2018 05:10 PM, Pavel Machek wrote:
> > 
> > Now... this is what I've suggested before. If you don't agree, you may
> > want to contact Tony Lindgren, IIRC he works for TI, too, and might be
> > willing to help.
>
> I will ping Tony just to close the loop.  I will be posting v4 today after making the changes.
> I was hoping to have some code review prior to posting v4 but have not received any comments so
> v4 will just be a patch rearrangement.

I guess not much to ping here though as I know little about these
chips :) As long as Rob is happy with the binding changes I'll be
happy too.

Regards,

Tony

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

* Re: [PATCH v3 2/9] dt-bindings: ti-lmu: Remove LM3697
  2018-10-19 14:58                             ` Tony Lindgren
@ 2018-10-24  8:55                               ` Pavel Machek
  0 siblings, 0 replies; 28+ messages in thread
From: Pavel Machek @ 2018-10-24  8:55 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Dan Murphy, Jacek Anaszewski, Rob Herring, Lee Jones, devicetree,
	linux-kernel, Linux LED Subsystem

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

On Fri 2018-10-19 07:58:12, Tony Lindgren wrote:
> * Dan Murphy <dmurphy@ti.com> [181019 11:42]:
> > On 10/18/2018 05:10 PM, Pavel Machek wrote:
> > > 
> > > Now... this is what I've suggested before. If you don't agree, you may
> > > want to contact Tony Lindgren, IIRC he works for TI, too, and might be
> > > willing to help.
> >
> > I will ping Tony just to close the loop.  I will be posting v4 today after making the changes.
> > I was hoping to have some code review prior to posting v4 but have not received any comments so
> > v4 will just be a patch rearrangement.
> 
> I guess not much to ping here though as I know little about these
> chips :) As long as Rob is happy with the binding changes I'll be
> happy too.

Well, question is more "how to make Rob happy". I see v4 is out, let
me comment on that.

									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] 28+ messages in thread

end of thread, other threads:[~2018-10-24  8:55 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-11 16:51 [PATCH v3 0/9] TI LMU common Framework Dan Murphy
2018-10-11 16:51 ` [PATCH v3 1/9] leds: add TI LMU backlight driver Dan Murphy
2018-10-11 16:51 ` [PATCH v3 2/9] dt-bindings: ti-lmu: Remove LM3697 Dan Murphy
2018-10-11 18:27   ` Pavel Machek
2018-10-11 18:34     ` Dan Murphy
2018-10-11 19:38       ` Jacek Anaszewski
2018-10-12 16:26         ` Rob Herring
2018-10-12 18:03           ` Pavel Machek
2018-10-13 18:45             ` Jacek Anaszewski
2018-10-15  0:56               ` Rob Herring
2018-10-15 19:13                 ` Jacek Anaszewski
2018-10-15 19:14                   ` Dan Murphy
2018-10-15 21:45                     ` Pavel Machek
2018-10-16 13:25                       ` Dan Murphy
2018-10-18 22:10                         ` Pavel Machek
2018-10-19 11:42                           ` Dan Murphy
2018-10-19 14:58                             ` Tony Lindgren
2018-10-24  8:55                               ` Pavel Machek
2018-10-12  9:07   ` Lee Jones
2018-10-11 16:51 ` [PATCH v3 3/9] mfd: ti-lmu: Remove support for LM3697 Dan Murphy
2018-10-11 16:51 ` [PATCH v3 4/9] dt-bindings: leds: Add bindings for lm3697 driver Dan Murphy
2018-10-12 16:27   ` Rob Herring
2018-10-19 11:47     ` Dan Murphy
2018-10-11 16:51 ` [PATCH v3 5/9] leds: lm3697: Introduce the " Dan Murphy
2018-10-11 16:51 ` [PATCH v3 6/9] dt-bindings: ti-lmu: Remove LM3633 Dan Murphy
2018-10-11 16:51 ` [PATCH v3 7/9] mfd: ti-lmu: Remove support for LM3633 Dan Murphy
2018-10-11 16:51 ` [PATCH v3 8/9] dt-bindings: leds: Add support for the LM3633 Dan Murphy
2018-10-11 16:51 ` [PATCH v3 9/9] leds: lm3633: Introduce the lm3633 driver Dan Murphy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).