linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH v2 0/9] TI LMU and Dedicated Drivers
@ 2018-09-28 18:29 Dan Murphy
  2018-09-28 18:29 ` [RFC PATCH v2 1/9] leds: add TI LMU backlight driver Dan Murphy
                   ` (8 more replies)
  0 siblings, 9 replies; 21+ messages in thread
From: Dan Murphy @ 2018-09-28 18:29 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel
  Cc: devicetree, linux-kernel, lee.jones, linux-omap, linux-leds, Dan Murphy

All

The following v2 patchset has been debugged and updated to support the
LM3697, LM3632 and LM3633.  The TI LMU common code has been used and can
be extended where need be.  If this approach is acceptable I will make code
changes based on review and submit v3 with additional bindings updates for
full review and merge.

These 3 LED drivers should be functional to provide basic lighting experiences.
Other features like PWM support will be included later and maybe part of the
common code.

I usually list all the deltas in my patch sets but due to the major rip in the
drivers I would repeat myself just saying that there are too many changes to
list.

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: leds: Add support for the LM3633
  leds: lm3633: Introduce the lm3633 driver
  dt-bindings: leds: Add the LM3632 LED dt binding
  leds: lm3632: Introduce the TI LM3632 driver

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

 .../devicetree/bindings/leds/leds-lm3632.txt  |  53 ++
 .../devicetree/bindings/leds/leds-lm3633.txt  | 102 +++
 .../devicetree/bindings/leds/leds-lm3697.txt  |  98 +++
 .../devicetree/bindings/mfd/ti-lmu.txt        |  26 +-
 drivers/leds/Kconfig                          |  30 +
 drivers/leds/Makefile                         |   4 +
 drivers/leds/leds-lm3632.c                    | 646 ++++++++++++++++++
 drivers/leds/leds-lm3633.c                    | 557 +++++++++++++++
 drivers/leds/leds-lm3697.c                    | 382 +++++++++++
 drivers/leds/ti-lmu-led-common.c              | 138 ++++
 drivers/leds/ti-lmu-led-common.h              |  54 ++
 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 -
 15 files changed, 2066 insertions(+), 88 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3632.txt
 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-lm3632.c
 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 drivers/leds/ti-lmu-led-common.h

-- 
2.19.0


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

* [RFC PATCH v2 1/9] leds: add TI LMU backlight driver
  2018-09-28 18:29 [RFC PATCH v2 0/9] TI LMU and Dedicated Drivers Dan Murphy
@ 2018-09-28 18:29 ` Dan Murphy
  2018-10-02  7:56   ` Pavel Machek
  2018-09-28 18:29 ` [RFC PATCH v2 2/9] dt-bindings: ti-lmu: Remove LM3697 Dan Murphy
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 21+ messages in thread
From: Dan Murphy @ 2018-09-28 18:29 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel
  Cc: devicetree, linux-kernel, lee.jones, linux-omap, 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 +++++++++++++++++++++++++++++++
 drivers/leds/ti-lmu-led-common.h |  54 ++++++++++++
 4 files changed, 201 insertions(+)
 create mode 100644 drivers/leds/ti-lmu-led-common.c
 create mode 100644 drivers/leds/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..2d4f22f480d1
--- /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 "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/drivers/leds/ti-lmu-led-common.h b/drivers/leds/ti-lmu-led-common.h
new file mode 100644
index 000000000000..511768dd54b6
--- /dev/null
+++ b/drivers/leds/ti-lmu-led-common.h
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-2.0
+// TI LMU Common Core
+// Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
+
+#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);
-- 
2.19.0


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

* [RFC PATCH v2 2/9] dt-bindings: ti-lmu: Remove LM3697
  2018-09-28 18:29 [RFC PATCH v2 0/9] TI LMU and Dedicated Drivers Dan Murphy
  2018-09-28 18:29 ` [RFC PATCH v2 1/9] leds: add TI LMU backlight driver Dan Murphy
@ 2018-09-28 18:29 ` Dan Murphy
  2018-10-02  7:28   ` Pavel Machek
  2018-09-28 18:29 ` [RFC PATCH v2 3/9] mfd: ti-lmu: Remove support for LM3697 Dan Murphy
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 21+ messages in thread
From: Dan Murphy @ 2018-09-28 18:29 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel
  Cc: devicetree, linux-kernel, lee.jones, linux-omap, 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] 21+ messages in thread

* [RFC PATCH v2 3/9] mfd: ti-lmu: Remove support for LM3697
  2018-09-28 18:29 [RFC PATCH v2 0/9] TI LMU and Dedicated Drivers Dan Murphy
  2018-09-28 18:29 ` [RFC PATCH v2 1/9] leds: add TI LMU backlight driver Dan Murphy
  2018-09-28 18:29 ` [RFC PATCH v2 2/9] dt-bindings: ti-lmu: Remove LM3697 Dan Murphy
@ 2018-09-28 18:29 ` Dan Murphy
  2018-09-28 18:29 ` [RFC PATCH v2 4/9] dt-bindings: leds: Add bindings for lm3697 driver Dan Murphy
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Dan Murphy @ 2018-09-28 18:29 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel
  Cc: devicetree, linux-kernel, lee.jones, linux-omap, 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/mfd/Kconfig                 |  2 +-
 drivers/mfd/ti-lmu.c                | 17 -----------
 include/linux/mfd/ti-lmu-register.h | 44 -----------------------------
 include/linux/mfd/ti-lmu.h          |  1 -
 4 files changed, 1 insertion(+), 63 deletions(-)

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

* [RFC PATCH v2 4/9] dt-bindings: leds: Add bindings for lm3697 driver
  2018-09-28 18:29 [RFC PATCH v2 0/9] TI LMU and Dedicated Drivers Dan Murphy
                   ` (2 preceding siblings ...)
  2018-09-28 18:29 ` [RFC PATCH v2 3/9] mfd: ti-lmu: Remove support for LM3697 Dan Murphy
@ 2018-09-28 18:29 ` Dan Murphy
  2018-09-28 18:29 ` [RFC PATCH v2 5/9] leds: lm3697: Introduce the " Dan Murphy
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Dan Murphy @ 2018-09-28 18:29 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel
  Cc: devicetree, linux-kernel, lee.jones, linux-omap, 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] 21+ messages in thread

* [RFC PATCH v2 5/9] leds: lm3697: Introduce the lm3697 driver
  2018-09-28 18:29 [RFC PATCH v2 0/9] TI LMU and Dedicated Drivers Dan Murphy
                   ` (3 preceding siblings ...)
  2018-09-28 18:29 ` [RFC PATCH v2 4/9] dt-bindings: leds: Add bindings for lm3697 driver Dan Murphy
@ 2018-09-28 18:29 ` Dan Murphy
  2018-09-28 18:29 ` [RFC PATCH v2 6/9] dt-bindings: leds: Add support for the LM3633 Dan Murphy
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Dan Murphy @ 2018-09-28 18:29 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel
  Cc: devicetree, linux-kernel, lee.jones, linux-omap, 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 | 382 +++++++++++++++++++++++++++++++++++++
 3 files changed, 390 insertions(+), 1 deletion(-)
 create mode 100644 drivers/leds/leds-lm3697.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index dc717b30d9d3..1de4cbb13bd2 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..eac4073fe393
--- /dev/null
+++ b/drivers/leds/leds-lm3697.c
@@ -0,0 +1,382 @@
+// 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 "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] 21+ messages in thread

* [RFC PATCH v2 6/9] dt-bindings: leds: Add support for the LM3633
  2018-09-28 18:29 [RFC PATCH v2 0/9] TI LMU and Dedicated Drivers Dan Murphy
                   ` (4 preceding siblings ...)
  2018-09-28 18:29 ` [RFC PATCH v2 5/9] leds: lm3697: Introduce the " Dan Murphy
@ 2018-09-28 18:29 ` Dan Murphy
  2018-09-28 18:29 ` [RFC PATCH v2 7/9] leds: lm3633: Introduce the lm3633 driver Dan Murphy
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Dan Murphy @ 2018-09-28 18:29 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel
  Cc: devicetree, linux-kernel, lee.jones, linux-omap, 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] 21+ messages in thread

* [RFC PATCH v2 7/9] leds: lm3633: Introduce the lm3633 driver
  2018-09-28 18:29 [RFC PATCH v2 0/9] TI LMU and Dedicated Drivers Dan Murphy
                   ` (5 preceding siblings ...)
  2018-09-28 18:29 ` [RFC PATCH v2 6/9] dt-bindings: leds: Add support for the LM3633 Dan Murphy
@ 2018-09-28 18:29 ` Dan Murphy
  2018-09-28 18:29 ` [RFC PATCH v2 8/9] dt-bindings: leds: Add the LM3632 LED dt binding Dan Murphy
  2018-09-28 18:29 ` [RFC PATCH v2 9/9] leds: lm3632: Introduce the TI LM3632 driver Dan Murphy
  8 siblings, 0 replies; 21+ messages in thread
From: Dan Murphy @ 2018-09-28 18:29 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel
  Cc: devicetree, linux-kernel, lee.jones, linux-omap, 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 | 557 +++++++++++++++++++++++++++++++++++++
 3 files changed, 566 insertions(+)
 create mode 100644 drivers/leds/leds-lm3633.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 1de4cbb13bd2..1a78588d9806 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..bf946eed271a
--- /dev/null
+++ b/drivers/leds/leds-lm3633.c
@@ -0,0 +1,557 @@
+// 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/regulator/consumer.h>
+
+#include "ti-lmu-led-common.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] 21+ messages in thread

* [RFC PATCH v2 8/9] dt-bindings: leds: Add the LM3632 LED dt binding
  2018-09-28 18:29 [RFC PATCH v2 0/9] TI LMU and Dedicated Drivers Dan Murphy
                   ` (6 preceding siblings ...)
  2018-09-28 18:29 ` [RFC PATCH v2 7/9] leds: lm3633: Introduce the lm3633 driver Dan Murphy
@ 2018-09-28 18:29 ` Dan Murphy
  2018-09-28 18:29 ` [RFC PATCH v2 9/9] leds: lm3632: Introduce the TI LM3632 driver Dan Murphy
  8 siblings, 0 replies; 21+ messages in thread
From: Dan Murphy @ 2018-09-28 18:29 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel
  Cc: devicetree, linux-kernel, lee.jones, linux-omap, linux-leds, Dan Murphy

Add the TI LM3632 LED binding.

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

diff --git a/Documentation/devicetree/bindings/leds/leds-lm3632.txt b/Documentation/devicetree/bindings/leds/leds-lm3632.txt
new file mode 100644
index 000000000000..1befd17174ac
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-lm3632.txt
@@ -0,0 +1,53 @@
+* Texas Instruments - lm3632 Single-Chip Backlight and 1.5-A Flash LED Driver
+
+The LM3632A integrates LED drivers for both
+the backlight of the LCD panel and the camera flash
+along with the bias power for the LCD panel into one
+device.
+
+Required properties:
+	- compatible : Can be one of the following
+		"ti,lm3632"
+	- reg : I2C slave address
+	- #address-cells : 1
+	- #size-cells : 0
+
+Required child properties:
+	- reg : 0 - Indicates a backlight mode
+		1 - Indicates a Torch/Strobe (white LED) mode
+
+Required properties for flash LED child nodes:
+	See Documentation/devicetree/bindings/leds/common.txt
+	- flash-max-microamp : Range from 100mA - 1.5A
+	- flash-max-timeout-us : Range from 32ms - 1024ms
+	- led-max-microamp : Range from 25mA - 375mA
+
+Optional child properties:
+	- label : see Documentation/devicetree/bindings/leds/common.txt
+	- linux,default-trigger :
+	   see Documentation/devicetree/bindings/leds/common.txt
+
+Example:
+led-controller@11 {
+	compatible = "ti,lm3632";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	reg = <0x11>;
+
+	led@0 {
+		reg = <0>;
+		label = "white:backlight";
+		linux,default-trigger = "backlight";
+	};
+
+	led@1 {
+		reg = <1>;
+		label = "white:torch";
+		led-max-microamp = <375000>;
+		flash-max-microamp = <1500000>;
+		flash-max-timeout-us = <1000000>;
+	};
+}
+
+For more product information please see the links below:
+http://www.ti.com/product/LM3632
-- 
2.19.0


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

* [RFC PATCH v2 9/9] leds: lm3632: Introduce the TI LM3632 driver
  2018-09-28 18:29 [RFC PATCH v2 0/9] TI LMU and Dedicated Drivers Dan Murphy
                   ` (7 preceding siblings ...)
  2018-09-28 18:29 ` [RFC PATCH v2 8/9] dt-bindings: leds: Add the LM3632 LED dt binding Dan Murphy
@ 2018-09-28 18:29 ` Dan Murphy
  8 siblings, 0 replies; 21+ messages in thread
From: Dan Murphy @ 2018-09-28 18:29 UTC (permalink / raw)
  To: robh+dt, jacek.anaszewski, pavel
  Cc: devicetree, linux-kernel, lee.jones, linux-omap, linux-leds, Dan Murphy

Add the dedicated TI LM3632 LED driver.  This
LED device is capable of driving a backlight display.

In addition to the backlight the device has control
of a strobe and torch output.

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

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

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 1a78588d9806..6174ab806cfb 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -764,8 +764,16 @@ config LEDS_LM3697
           This supports the LED device LM3697.
 
 config LEDS_LM3633
-	tristate "LED driver for LM3633"
+	tristate "LED driver for LM3632"
  	depends on LEDS_TI_LMU_COMMON
+	help
+          Say Y to enable the LED driver for TI LMU devices.
+          This supports the LED device LM3632.
+
+config LEDS_LM3632
+	tristate "LED driver for LM3632"
+	depends on LEDS_TI_LMU_COMMON
+	depends on LEDS_CLASS_FLASH
 	help
           Say Y to enable the LED driver for TI LMU devices.
           This supports the LED device LM3633.
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 6ec006892fc0..53d02d7f1a0b 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_LM3632)		+= leds-lm3632.o
 obj-$(CONFIG_LEDS_LM3633)		+= leds-lm3633.o
 obj-$(CONFIG_LEDS_TI_LMU_COMMON)	+= ti-lmu-led-common.o
 
diff --git a/drivers/leds/leds-lm3632.c b/drivers/leds/leds-lm3632.c
new file mode 100644
index 000000000000..ac3c21180444
--- /dev/null
+++ b/drivers/leds/leds-lm3632.c
@@ -0,0 +1,646 @@
+// SPDX-License-Identifier: GPL-2.0
+// Flash and torch driver for Texas Instruments LM3632 LED
+// Flash driver chip family
+// Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
+
+#include <linux/led-class-flash.h>
+
+#include "ti-lmu-led-common.h"
+
+#define LM3632_MODE_BL		0x0
+#define LM3632_MODE_TORCH	0x1
+
+/* Registers */
+#define LM3632_REV_REG		0x01
+#define LM3632_CFG1_REG		0x02
+#define LM3632_CFG2_REG		0x03
+#define LM3633_BL_BRT_LSB	0x04
+#define LM3633_BL_BRT_MSB	0x05
+#define LM3632_FLASH_TORCH_BRT	0x06
+#define LM3632_FLASH_CFG	0x07
+#define LM3632_IO_CTRL		0x09
+#define LM3632_ENABLE_REG	0x0a
+#define LM3632_FLAGS1_REG	0x0b
+#define LM3632_FLAGS2_REG	0x10
+
+/* Enable Mode bits */
+#define LM3632_BL_EN		BIT(0)
+#define LM3632_FLASH_OUT_EN	BIT(1)
+#define LM3632_FLASH_MODE	BIT(2)
+#define LM3632_BLED1_2_EN	BIT(3)
+#define LM3632_BLED1_EN		BIT(4)
+#define LM3632_BLED1_2_MASK	(LM3632_BL_EN | LM3632_BLED1_2_EN | LM3632_BLED1_EN)
+#define LM3632_BL_OVP_EN	BIT(6)
+#define LM3632_SW_RESET		BIT(7)
+
+/* Flags 1 Mask */
+#define LM3632_THERM_SHUTDOWN	BIT(0)
+#define LM3632_FLASH_TIME_OUT	BIT(1)
+#define LM3632_FLED_SHORT_FAULT	BIT(2)
+#define LM3632_VINM_SHORT_FAULT	BIT(4)
+#define LM3632_FOUT_SHORT_FAULT	BIT(5)
+#define LM3632_FLASH_OVP_FAULT	BIT(6)
+#define LM3632_BL_OVP_FAULT	BIT(7)
+
+/* Flags 2 Mask */
+#define LM3632_BL_OCP_FAULT	BIT(0)
+#define LM3632_FLASH_OCP_FAULT	BIT(1)
+#define LM3632_VNEG_SHORT_FAULT	BIT(2)
+#define LM3632_VPOS_SHORT_FAULT	BIT(3)
+#define LM3632_VNEG_OVP_FAULT	BIT(4)
+#define LM3632_LCM_OVP_FAULT	BIT(5)
+
+/* IO CTRL bits */
+#define LM3632_VINM_EN		BIT(0)
+#define LM3632_VINM_MODE_EN	BIT(1)
+#define LM3632_TX_EN		BIT(2)
+#define LM3632_HW_STROBE_EN	BIT(4)
+#define LM3632_PWM_EN		BIT(6)
+
+#define LM3632_TORCH_BRT_SHIFT	4
+
+#define LM3632_MAX_TORCH_I_UA	375000
+#define LM3632_MIN_TORCH_I_UA	25000
+#define LM3632_TORCH_STEP_UA	25000
+
+#define LM3632_MAX_STROBE_I_UA	1500000
+#define LM3632_MIN_STROBE_I_UA	100000
+#define LM3632_STROBE_STEP_UA	100000
+
+#define LM3632_TIMEOUT_MASK	0x1f
+#define LM3632_ENABLE_MASK	(LM3632_BL_EN | LM3632_FLASH_OUT_EN)
+
+#define LM3632_TIMEOUT_STEP_US	32000
+#define LM3632_MIN_TIMEOUT_US	32000
+#define LM3632_MAX_TIMEOUT_US	1024000
+
+#define LM3632_TORCH_BRT_MASK	0xf0
+#define LM3632_FLASH_BRT_MASK	0xf
+
+#define LM3632_NUM_OF_BL_STRINGS 2
+#define LM3632_BL_ENABLED	1
+#define LM3632_BL1_ENABLE_SRC	0
+#define LM3632_BL12_ENABLE_SRC	1
+
+/**
+ * struct lm3632_led -
+ * @fled_cdev: flash LED class device pointer
+ * @led_name: LED label
+ * @led_dev: LED class device
+ * @priv: Pointer to the lm3632 struct
+ * @lmu_data: Register and setting values for common code
+ * @led_name: LED label for the Torch or IR LED
+ * @flash_timeout: the timeout for the flash
+ * @last_flag: last known flags register value
+ * @torch_current_max: maximum current for the torch
+ * @flash_current_max: maximum current for the flash
+ * @max_flash_timeout: maximum timeout for the flash
+ */
+struct lm3632_led {
+	struct led_classdev_flash fled_cdev;
+	u32 led_strings[LM3632_NUM_OF_BL_STRINGS];
+	char led_name[LED_MAX_NAME_SIZE];
+	struct led_classdev led_dev;
+	struct ti_lmu_bank lmu_data;
+	struct lm3632 *priv;
+
+	unsigned int flash_timeout;
+	unsigned int last_flag;
+
+	u32 torch_current_max;
+	u32 flash_current_max;
+	u32 max_flash_timeout;
+};
+
+/**
+ * struct lm3632 -
+ * @client: Pointer to the I2C client
+ * @regmap: Devices register map
+ * @lock: Lock for reading/writing the device
+ * @strobe_enable_gpio: Indicates whether strobe is HW controlled
+ * @leds: Array of LED strings
+ */
+struct lm3632 {
+	struct i2c_client *client;
+	struct regmap *regmap;
+	struct device *dev;
+	struct mutex lock;
+	int strobe_enable_gpio;
+
+	struct lm3632_led leds[];
+};
+
+static const struct reg_default lm3632_regmap_defs[] = {
+	{ LM3632_CFG1_REG, 0x30 },
+	{ LM3632_CFG2_REG, 0x0d },
+	{ LM3632_FLASH_CFG, 0x2f },
+	{ LM3632_ENABLE_REG, 0x00 },
+};
+
+static bool lm3632_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case LM3632_FLAGS1_REG:
+	case LM3632_FLAGS2_REG:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static const struct regmap_config lm3632_regmap = {
+	.reg_bits = 8,
+	.val_bits = 8,
+
+	.max_register = LM3632_FLAGS2_REG,
+	.reg_defaults = lm3632_regmap_defs,
+	.num_reg_defaults = ARRAY_SIZE(lm3632_regmap_defs),
+	.cache_type = REGCACHE_RBTREE,
+	.volatile_reg = lm3632_volatile_reg,
+};
+
+static struct lm3632_led *fled_cdev_to_led(struct led_classdev_flash *fled_cdev)
+{
+	return container_of(fled_cdev, struct lm3632_led, fled_cdev);
+}
+
+static int lm3632_read_faults(struct lm3632_led *led)
+{
+	struct lm3632 *priv = led->priv;
+	int flags_val;
+	int ret;
+
+	ret = regmap_read(priv->regmap, LM3632_FLAGS1_REG, &flags_val);
+	if (ret < 0)
+		return -EIO;
+
+	led->last_flag = 0;
+
+	if (flags_val & LM3632_FLASH_OVP_FAULT)
+		led->last_flag |= LED_FAULT_OVER_VOLTAGE;
+
+	if (flags_val & LM3632_THERM_SHUTDOWN)
+		led->last_flag |= LED_FAULT_OVER_TEMPERATURE;
+
+	if (flags_val & (LM3632_FLED_SHORT_FAULT | LM3632_VINM_SHORT_FAULT |
+	    LM3632_FOUT_SHORT_FAULT))
+		led->last_flag |= LED_FAULT_SHORT_CIRCUIT;
+
+	if (flags_val & LM3632_THERM_SHUTDOWN)
+		led->last_flag |= LED_FAULT_LED_OVER_TEMPERATURE;
+
+	if (flags_val & LM3632_FLASH_TIME_OUT)
+		led->last_flag |= LED_FAULT_TIMEOUT;
+
+	ret = regmap_read(priv->regmap, LM3632_FLAGS2_REG, &flags_val);
+	if (ret < 0)
+		return -EIO;
+
+	if (flags_val & (LM3632_BL_OCP_FAULT | LM3632_FLASH_OCP_FAULT))
+		led->last_flag |= LED_FAULT_OVER_CURRENT;
+
+	if (flags_val & LM3632_FLASH_OCP_FAULT)
+		led->last_flag |= LED_FAULT_OVER_CURRENT;
+
+	return led->last_flag;
+}
+
+static int lm3632_backlight_brightness_set(struct led_classdev *cdev,
+					enum led_brightness brightness)
+{
+	struct lm3632_led *led = container_of(cdev, struct lm3632_led, led_dev);
+	struct lm3632 *priv = led->priv;
+	int ret;
+
+	mutex_lock(&priv->lock);
+
+	ret = ti_lmu_common_set_brightness(&led->lmu_data, brightness);
+	if (ret)
+		dev_err(&priv->client->dev, "Cannot write brightness\n");
+
+	mutex_unlock(&priv->lock);
+	return ret;
+}
+
+static int lm3632_torch_brightness_set(struct led_classdev *cdev,
+					enum led_brightness brightness)
+{
+	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(cdev);
+	struct lm3632_led *led = fled_cdev_to_led(fled_cdev);
+	struct lm3632 *priv = led->priv;
+	int ret, brightness_val;
+	unsigned int reg_val;
+
+	mutex_lock(&priv->lock);
+
+	ret = lm3632_read_faults(led);
+	if (ret < 0)
+		goto out;
+
+	if (brightness == LED_OFF)
+		ret = regmap_update_bits(priv->regmap, LM3632_ENABLE_REG,
+					 LM3632_FLASH_OUT_EN,
+					 ~LM3632_FLASH_OUT_EN);
+	else {
+		regmap_read(priv->regmap, LM3632_FLASH_TORCH_BRT, &reg_val);
+		brightness_val = (brightness - LM3632_TORCH_STEP_UA) / LM3632_TORCH_STEP_UA;
+		brightness_val |= (reg_val & LM3632_FLASH_BRT_MASK);
+
+		ret = regmap_write(priv->regmap, LM3632_FLASH_TORCH_BRT,
+				   brightness_val << LM3632_TORCH_BRT_SHIFT);
+		if (ret) {
+			dev_err(&priv->client->dev, "Cannot write brightness\n");
+			goto out;
+		}
+
+		ret = regmap_update_bits(priv->regmap, LM3632_ENABLE_REG,
+					 LM3632_FLASH_MODE | LM3632_FLASH_OUT_EN,
+					 LM3632_FLASH_OUT_EN | ~LM3632_FLASH_MODE);
+		if (ret)
+			dev_err(&priv->client->dev, "Cannot write enable\n");
+	}
+out:
+	mutex_unlock(&priv->lock);
+	return ret;
+}
+
+static int lm3632_strobe_set(struct led_classdev_flash *fled_cdev, bool state)
+{
+	struct lm3632_led *led = fled_cdev_to_led(fled_cdev);
+	struct lm3632 *priv = led->priv;
+	int timeout_reg_val;
+	int current_timeout;
+	int ret;
+
+	mutex_lock(&priv->lock);
+
+	ret = regmap_read(priv->regmap, LM3632_FLASH_CFG, &current_timeout);
+	if (ret < 0)
+		goto out;
+
+	if (led->flash_timeout != current_timeout) {
+		timeout_reg_val = (led->flash_timeout - LM3632_TIMEOUT_STEP_US) / LM3632_TIMEOUT_STEP_US;
+		ret = regmap_update_bits(priv->regmap, LM3632_FLASH_CFG,
+					LM3632_TIMEOUT_MASK, timeout_reg_val);
+		if (ret) {
+			dev_err(&priv->client->dev, "Cannot write timeout\n");
+			goto out;
+		}
+	}
+
+	if (state && !priv->strobe_enable_gpio) {
+		ret = regmap_update_bits(priv->regmap, LM3632_ENABLE_REG,
+					 LM3632_FLASH_MODE | LM3632_FLASH_OUT_EN,
+					 LM3632_FLASH_OUT_EN | LM3632_FLASH_MODE);
+		if (ret) {
+			dev_err(&priv->client->dev, "Cannot write flash en\n");
+			goto out;
+		}
+	}
+
+	ret = lm3632_read_faults(led);
+out:
+	mutex_unlock(&priv->lock);
+	return ret;
+}
+
+static int lm3632_flash_brightness_set(struct led_classdev_flash *fled_cdev,
+					u32 brightness)
+{
+	struct lm3632_led *led = fled_cdev_to_led(fled_cdev);
+	struct lm3632 *priv = led->priv;
+	u32 brightness_val;
+	int ret;
+	unsigned int reg_val;
+
+	mutex_lock(&priv->lock);
+	ret = lm3632_read_faults(led);
+	if (ret < 0)
+		goto out;
+
+	brightness_val = (brightness - LM3632_STROBE_STEP_UA) / LM3632_STROBE_STEP_UA;
+	regmap_read(priv->regmap, LM3632_FLASH_TORCH_BRT, &reg_val);
+
+	brightness_val |= (reg_val & LM3632_TORCH_BRT_MASK);
+	ret = regmap_write(priv->regmap, LM3632_FLASH_TORCH_BRT, brightness_val);
+	if (ret)
+		dev_err(&priv->client->dev, "Cannot write brightness\n");
+
+out:
+	mutex_unlock(&priv->lock);
+	return ret;
+}
+
+static int lm3632_flash_timeout_set(struct led_classdev_flash *fled_cdev,
+				u32 timeout)
+{
+	struct lm3632_led *led = fled_cdev_to_led(fled_cdev);
+	struct lm3632 *priv = led->priv;
+
+	mutex_lock(&priv->lock);
+
+	led->flash_timeout = timeout;
+
+	mutex_unlock(&priv->lock);
+
+	return 0;
+}
+
+static int lm3632_strobe_get(struct led_classdev_flash *fled_cdev, bool *state)
+{
+	struct lm3632_led *led = fled_cdev_to_led(fled_cdev);
+	struct lm3632 *priv = led->priv;
+	int strobe_state;
+	int ret;
+
+	mutex_lock(&priv->lock);
+
+	ret = regmap_read(priv->regmap, LM3632_ENABLE_REG, &strobe_state);
+	if (ret < 0)
+		goto out;
+
+	*state = strobe_state & LM3632_FLASH_OUT_EN;
+
+out:
+	mutex_unlock(&priv->lock);
+	return ret;
+}
+
+static int lm3632_flash_fault_get(struct led_classdev_flash *fled_cdev,
+				u32 *fault)
+{
+	struct lm3632_led *led = fled_cdev_to_led(fled_cdev);
+
+	lm3632_read_faults(led);
+
+	*fault = led->last_flag;
+
+	return 0;
+}
+
+static const struct led_flash_ops flash_ops = {
+	.flash_brightness_set	= lm3632_flash_brightness_set,
+	.strobe_set		= lm3632_strobe_set,
+	.strobe_get		= lm3632_strobe_get,
+	.timeout_set		= lm3632_flash_timeout_set,
+	.fault_get		= lm3632_flash_fault_get,
+};
+
+static int lm3632_register_strobe_leds(struct lm3632_led *led)
+{
+	struct led_classdev *led_cdev;
+	struct led_flash_setting *setting;
+
+	led->fled_cdev.ops = &flash_ops;
+
+	setting = &led->fled_cdev.timeout;
+	setting->min = LM3632_MIN_TIMEOUT_US;
+	setting->max = led->max_flash_timeout;
+	setting->step = LM3632_TIMEOUT_STEP_US;
+	setting->val = led->max_flash_timeout;
+
+	setting = &led->fled_cdev.brightness;
+	setting->min = LM3632_MIN_STROBE_I_UA;
+	setting->max = led->flash_current_max;
+	setting->step = LM3632_STROBE_STEP_UA;
+	setting->val = led->flash_current_max;
+
+	led_cdev = &led->fled_cdev.led_cdev;
+	led_cdev->name = led->led_name;
+	led_cdev->brightness_set_blocking = lm3632_torch_brightness_set;
+	led_cdev->max_brightness = led->torch_current_max;
+
+	led_cdev->flags |= LED_DEV_CAP_FLASH;
+
+	return led_classdev_flash_register(&led->priv->client->dev, &led->fled_cdev);
+}
+
+static int lm3632_strobe_init(struct lm3632_led *led)
+{
+	struct lm3632 *priv = led->priv;
+
+	if (priv->strobe_enable_gpio)
+		regmap_update_bits(priv->regmap, LM3632_IO_CTRL,
+				   LM3632_HW_STROBE_EN, LM3632_HW_STROBE_EN);
+
+	return regmap_update_bits(priv->regmap, LM3632_ENABLE_REG,
+				  LM3632_FLASH_OUT_EN, ~LM3632_FLASH_OUT_EN);
+
+}
+
+static int lm3632_backlight_init(struct lm3632_led *led)
+{
+	struct lm3632 *priv = led->priv;
+	u8 bl_enable = 0;
+	int ret;
+
+	if (led->led_strings[LM3632_BL12_ENABLE_SRC] == LM3632_BL_ENABLED)
+		bl_enable |= LM3632_BLED1_2_EN;
+	else if (led->led_strings[LM3632_BL1_ENABLE_SRC] == LM3632_BL_ENABLED)
+		bl_enable |= LM3632_BLED1_EN;
+	else
+		return -EINVAL;
+
+	bl_enable |= LM3632_BL_EN;
+
+	/* Power up default is on so lets set it to off */
+	ret = ti_lmu_common_set_brightness(&led->lmu_data, LED_OFF);
+	if (ret) {
+		dev_err(&priv->client->dev, "Cannot write brightness\n");
+		return ret;
+	}
+
+	return regmap_update_bits(priv->regmap, LM3632_ENABLE_REG,
+				  LM3632_BLED1_2_MASK, bl_enable);
+}
+
+static int lm3632_parse_node(struct lm3632 *priv)
+{
+	struct fwnode_handle *child = NULL;
+	struct lm3632_led *led;
+	int ret = -ENODEV;
+	const char *name;
+	u32 led_mode;
+	size_t i = 0;
+
+	priv->strobe_enable_gpio = device_property_present(&priv->client->dev,
+						   "hw-strobe");
+
+	device_for_each_child_node(priv->dev, child) {
+		ret = fwnode_property_read_u32(child, "reg", &led_mode);
+		if (ret) {
+			dev_err(&priv->client->dev, "reg DT property missing\n");
+			goto out_err;
+		}
+
+		if (led_mode > LM3632_MODE_TORCH ||
+		    led_mode < LM3632_MODE_BL) {
+			dev_warn(&priv->client->dev, "Invalid led mode requested\n");
+			ret = -EINVAL;
+			goto out_err;
+		}
+
+		led = &priv->leds[i];
+		led->priv = priv;
+
+		ret = fwnode_property_read_string(child, "label", &name);
+		if (ret) {
+			if (led_mode == LM3632_MODE_TORCH)
+				name = "torch";
+			else
+				name = "backlight";
+		}
+
+		snprintf(led->led_name, sizeof(led->led_name),
+			"%s:%s", priv->client->name, name);
+
+		if (led_mode == LM3632_MODE_TORCH) {
+			ret = fwnode_property_read_u32(child, "led-max-microamp",
+							&led->torch_current_max);
+			if (ret) {
+				dev_warn(&priv->client->dev,
+					"led-max-microamp DT property missing\n");
+				goto out_err;
+			}
+
+			ret = fwnode_property_read_u32(child, "flash-max-microamp",
+						&led->flash_current_max);
+			if (ret) {
+				dev_warn(&priv->client->dev,
+					 "flash-max-microamp DT property missing\n");
+				goto out_err;
+			}
+
+			ret = fwnode_property_read_u32(child, "flash-max-timeout-us",
+						&led->max_flash_timeout);
+			if (ret) {
+				dev_warn(&priv->client->dev,
+					 "flash-max-timeout-us DT property missing\n");
+				goto out_err;
+			}
+			ret = lm3632_strobe_init(led);
+			if (ret) {
+				dev_err(&priv->client->dev, "failed to init strobe\n");
+				continue;
+			}
+
+			ret = lm3632_register_strobe_leds(led);
+			if (ret) {
+				dev_warn(&priv->client->dev,
+					 "Failed to register flash LEDs\n");
+				goto out_err;
+			}
+
+		} else	if (led_mode == LM3632_MODE_BL) {
+
+			ret = fwnode_property_read_u32_array(child, "led-sources",
+							     led->led_strings,
+							     LM3632_NUM_OF_BL_STRINGS);
+			if (ret) {
+				dev_err(&priv->client->dev, "led-sources property missing\n");
+				continue;
+			}
+
+			led->led_dev.name = led->led_name;
+			led->led_dev.brightness_set_blocking = lm3632_backlight_brightness_set;
+			led->lmu_data.regmap = priv->regmap;
+			led->lmu_data.max_brightness = MAX_BRIGHTNESS_11BIT;
+			led->lmu_data.lsb_brightness_reg = LM3633_BL_BRT_LSB;
+			led->lmu_data.msb_brightness_reg = LM3633_BL_BRT_MSB;
+			led->lmu_data.enable_reg = LM3632_ENABLE_REG;
+
+			ret = lm3632_backlight_init(led);
+			if (ret) {
+				dev_err(&priv->client->dev, "failed to init backlight\n");
+				continue;
+			}
+
+			ret = devm_led_classdev_register(priv->dev, &led->led_dev);
+			if (ret) {
+				dev_err(&priv->client->dev, "failed to register backlight\n");
+				continue;
+			}
+		} else {
+			dev_warn(&priv->client->dev,
+					 "Failed to register flash LEDs\n");
+			goto out_err;
+		}
+
+		i++;
+	}
+out_err:
+	fwnode_handle_put(child);
+	return ret;
+}
+
+static int lm3632_probe(struct i2c_client *client)
+{
+	struct lm3632 *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;
+
+	led->client = client;
+	led->dev = &client->dev;
+	i2c_set_clientdata(client, led);
+
+	led->regmap = devm_regmap_init_i2c(client, &lm3632_regmap);
+	if (IS_ERR(led->regmap)) {
+		ret = PTR_ERR(led->regmap);
+		dev_err(&client->dev,
+			"Failed to allocate register map: %d\n", ret);
+		return ret;
+	}
+
+	mutex_init(&led->lock);
+
+	return lm3632_parse_node(led);
+}
+
+static int lm3632_remove(struct i2c_client *client)
+{
+	struct lm3632 *led = i2c_get_clientdata(client);
+
+/*	led_classdev_flash_unregister(&led->fled_cdev);*/
+	mutex_destroy(&led->lock);
+
+	return regmap_update_bits(led->regmap, LM3632_ENABLE_REG,
+			   LM3632_ENABLE_MASK, 0x00);
+}
+
+static const struct i2c_device_id lm3632_id[] = {
+	{ "LM3632", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, lm3632_id);
+
+static const struct of_device_id of_lm3632_leds_match[] = {
+	{ .compatible = "ti,lm3632", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, of_lm3632_leds_match);
+
+static struct i2c_driver lm3632_i2c_driver = {
+	.driver = {
+		.name = "lm3632",
+		.of_match_table = of_lm3632_leds_match,
+	},
+	.probe_new = lm3632_probe,
+	.remove = lm3632_remove,
+	.id_table = lm3632_id,
+};
+module_i2c_driver(lm3632_i2c_driver);
+
+MODULE_DESCRIPTION("Texas Instruments Flash Lighting driver for LM3632");
+MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com>");
+MODULE_LICENSE("GPL v2");
-- 
2.19.0


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

* Re: [RFC PATCH v2 2/9] dt-bindings: ti-lmu: Remove LM3697
  2018-09-28 18:29 ` [RFC PATCH v2 2/9] dt-bindings: ti-lmu: Remove LM3697 Dan Murphy
@ 2018-10-02  7:28   ` Pavel Machek
  2018-10-03 12:24     ` Dan Murphy
  0 siblings, 1 reply; 21+ messages in thread
From: Pavel Machek @ 2018-10-02  7:28 UTC (permalink / raw)
  To: Dan Murphy
  Cc: robh+dt, jacek.anaszewski, devicetree, linux-kernel, lee.jones,
	linux-omap, linux-leds

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

On Fri 2018-09-28 13:29:47, 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, for reasons I explained before. Please add it to the patch so
that it does not get applied by mistake. Ouch and AFAICT Rob was not
happy with this either.

Yes, you are creating new drivers, ok; but that does _not_ mean you
should create new binding.
									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] 21+ messages in thread

* Re: [RFC PATCH v2 1/9] leds: add TI LMU backlight driver
  2018-09-28 18:29 ` [RFC PATCH v2 1/9] leds: add TI LMU backlight driver Dan Murphy
@ 2018-10-02  7:56   ` Pavel Machek
  2018-10-02 12:32     ` Dan Murphy
  0 siblings, 1 reply; 21+ messages in thread
From: Pavel Machek @ 2018-10-02  7:56 UTC (permalink / raw)
  To: Dan Murphy
  Cc: robh+dt, jacek.anaszewski, devicetree, linux-kernel, lee.jones,
	linux-omap, linux-leds, Milo Kim, Sebastian Reichel

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

On Fri 2018-09-28 13:29:46, Dan Murphy wrote:
> 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>

So... this driver adds support for LM3532, LM3631, LM3632, LM3633,
LM3695 and LM3697 (or it did when I signed it off).

The rest of the series does not really bring any advantages (you claim
it may add advantages in future). It takes code out of common driver
and duplicates it.

Could we take this patch, get the basic support for LM3532, LM3631,
LM3632, LM3633, LM3695 and LM3697, and then split out the drivers when
we actually gain some advantage doing so (and also when the costs are
clear)?

Thanks,

								Pavel

>  drivers/leds/Kconfig             |   8 ++
>  drivers/leds/Makefile            |   1 +
>  drivers/leds/ti-lmu-led-common.c | 138 +++++++++++++++++++++++++++++++
>  drivers/leds/ti-lmu-led-common.h |  54 ++++++++++++
>  4 files changed, 201 insertions(+)
>  create mode 100644 drivers/leds/ti-lmu-led-common.c
>  create mode 100644 drivers/leds/ti-lmu-led-common.h

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

* Re: [RFC PATCH v2 1/9] leds: add TI LMU backlight driver
  2018-10-02  7:56   ` Pavel Machek
@ 2018-10-02 12:32     ` Dan Murphy
  2018-10-02 18:52       ` Jacek Anaszewski
  0 siblings, 1 reply; 21+ messages in thread
From: Dan Murphy @ 2018-10-02 12:32 UTC (permalink / raw)
  To: Pavel Machek
  Cc: robh+dt, jacek.anaszewski, devicetree, linux-kernel, lee.jones,
	linux-omap, linux-leds, Sebastian Reichel

Pavel

On 10/02/2018 02:56 AM, Pavel Machek wrote:
> On Fri 2018-09-28 13:29:46, Dan Murphy wrote:
>> 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>
> 
> So... this driver adds support for LM3532, LM3631, LM3632, LM3633,
> LM3695 and LM3697 (or it did when I signed it off).

Yes I have to pull these out of the patch.

> 
> The rest of the series does not really bring any advantages (you claim
> it may add advantages in future). It takes code out of common driver
> and duplicates it.


I disagree.  Honestly using that ideallogy all LED drivers should use the common code
as it is a wrapper around regmap and a few if statements.

The 3632 adds the proper LED flash class support coupled with proper backlight support.
The 3633 adds the proper support for LV and HV LED support.

Duplicate code that I could find is put in the common file.
This patch set adds the LED devices as all other LED devices are added in the LED directory.

> 
> Could we take this patch, get the basic support for LM3532, LM3631,
> LM3632, LM3633, LM3695 and LM3697, and then split out the drivers when
> we actually gain some advantage doing so (and also when the costs are
> clear)?

We have debated this over and over and now we have 3 different implementations
available we need to collude on which one we want to support.

Jacek I defer to you and Pavel since you are both LED maintainers.

I can support the dedicated LED drivers but I cannot support the TI LMU only implementation.

Dan

> 
> Thanks,
> 
> 								Pavel
> 
>>  drivers/leds/Kconfig             |   8 ++
>>  drivers/leds/Makefile            |   1 +
>>  drivers/leds/ti-lmu-led-common.c | 138 +++++++++++++++++++++++++++++++
>>  drivers/leds/ti-lmu-led-common.h |  54 ++++++++++++
>>  4 files changed, 201 insertions(+)
>>  create mode 100644 drivers/leds/ti-lmu-led-common.c
>>  create mode 100644 drivers/leds/ti-lmu-led-common.h
> 


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

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

* Re: [RFC PATCH v2 1/9] leds: add TI LMU backlight driver
  2018-10-02 12:32     ` Dan Murphy
@ 2018-10-02 18:52       ` Jacek Anaszewski
  2018-10-02 22:07         ` Pavel Machek
  0 siblings, 1 reply; 21+ messages in thread
From: Jacek Anaszewski @ 2018-10-02 18:52 UTC (permalink / raw)
  To: Dan Murphy, Pavel Machek
  Cc: robh+dt, devicetree, linux-kernel, lee.jones, linux-omap,
	linux-leds, Sebastian Reichel

On 10/02/2018 02:32 PM, Dan Murphy wrote:
> Pavel
> 
> On 10/02/2018 02:56 AM, Pavel Machek wrote:
>> On Fri 2018-09-28 13:29:46, Dan Murphy wrote:
>>> 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>
>>
>> So... this driver adds support for LM3532, LM3631, LM3632, LM3633,
>> LM3695 and LM3697 (or it did when I signed it off).
> 
> Yes I have to pull these out of the patch.
> 
>>
>> The rest of the series does not really bring any advantages (you claim
>> it may add advantages in future). It takes code out of common driver
>> and duplicates it.
> 
> 
> I disagree.  Honestly using that ideallogy all LED drivers should use the common code
> as it is a wrapper around regmap and a few if statements.
> 
> The 3632 adds the proper LED flash class support coupled with proper backlight support.
> The 3633 adds the proper support for LV and HV LED support.
> 
> Duplicate code that I could find is put in the common file.
> This patch set adds the LED devices as all other LED devices are added in the LED directory.
> 
>>
>> Could we take this patch, get the basic support for LM3532, LM3631,
>> LM3632, LM3633, LM3695 and LM3697, and then split out the drivers when
>> we actually gain some advantage doing so (and also when the costs are
>> clear)?
> 
> We have debated this over and over and now we have 3 different implementations
> available we need to collude on which one we want to support.
> 
> Jacek I defer to you and Pavel since you are both LED maintainers.
> 
> I can support the dedicated LED drivers but I cannot support the TI LMU only implementation.

I uphold my previous opinion - please go ahead with moving the support
for non-MFD devices from MFD subsystem to the LED subsystem. And yes -
along with the bindings. This is semantically correct, and yet we don't
have mainline users.

Pavel - you will have to engage more people for your crusade to prevail.
For now, to speed up the things, I am forced to ignore your NAK.
So NAK to your NAK. Sorry.

-- 
Best regards,
Jacek Anaszewski

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

* Re: [RFC PATCH v2 1/9] leds: add TI LMU backlight driver
  2018-10-02 18:52       ` Jacek Anaszewski
@ 2018-10-02 22:07         ` Pavel Machek
  2018-10-03 12:00           ` Dan Murphy
  0 siblings, 1 reply; 21+ messages in thread
From: Pavel Machek @ 2018-10-02 22:07 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: Dan Murphy, robh+dt, devicetree, linux-kernel, lee.jones,
	linux-omap, linux-leds, Sebastian Reichel

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

Hi!

> > We have debated this over and over and now we have 3 different implementations
> > available we need to collude on which one we want to support.
> > 
> > Jacek I defer to you and Pavel since you are both LED maintainers.
> > 
> > I can support the dedicated LED drivers but I cannot support the TI LMU only implementation.
> 
> I uphold my previous opinion - please go ahead with moving the support
> for non-MFD devices from MFD subsystem to the LED subsystem. And yes -
> along with the bindings. This is semantically correct, and yet we don't
> have mainline users.
> 
> Pavel - you will have to engage more people for your crusade to prevail.
> For now, to speed up the things, I am forced to ignore your NAK.
> So NAK to your NAK. Sorry.

No need to be sorry :-).

Lets ignore the code for now, as code can be changed easily.

Bindings are not something you or I maintain, so we don't have final
word there, and I have feeling this is not going to go past device
tree maintainers. [AFAICT: you can move binding in Documentation/ to
different place, that's not a problem; but creating a new binding when
old one exists is.]

If you and Dan feel that is okay, you are welcome to try to get the
patches past Rob, just please retain the NAK so that he remembers the
discussion, and so that it is clear that I don't like the changes.

I believe smart thing to do is to try to do that before working
further on the code, but of course, its all up to you :-).

Friends,
									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] 21+ messages in thread

* Re: [RFC PATCH v2 1/9] leds: add TI LMU backlight driver
  2018-10-02 22:07         ` Pavel Machek
@ 2018-10-03 12:00           ` Dan Murphy
  2018-10-03 12:10             ` Dan Murphy
  0 siblings, 1 reply; 21+ messages in thread
From: Dan Murphy @ 2018-10-03 12:00 UTC (permalink / raw)
  To: Pavel Machek, Jacek Anaszewski
  Cc: robh+dt, devicetree, linux-kernel, lee.jones, linux-omap,
	linux-leds, Sebastian Reichel

Hello

On 10/02/2018 05:07 PM, Pavel Machek wrote:
> Hi!
> 
>>> We have debated this over and over and now we have 3 different implementations
>>> available we need to collude on which one we want to support.
>>>
>>> Jacek I defer to you and Pavel since you are both LED maintainers.
>>>
>>> I can support the dedicated LED drivers but I cannot support the TI LMU only implementation.
>>
>> I uphold my previous opinion - please go ahead with moving the support
>> for non-MFD devices from MFD subsystem to the LED subsystem. And yes -
>> along with the bindings. This is semantically correct, and yet we don't
>> have mainline users.
>>
>> Pavel - you will have to engage more people for your crusade to prevail.
>> For now, to speed up the things, I am forced to ignore your NAK.
>> So NAK to your NAK. Sorry.
> 
> No need to be sorry :-).
> 
> Lets ignore the code for now, as code can be changed easily.
> 
> Bindings are not something you or I maintain, so we don't have final
> word there, and I have feeling this is not going to go past device
> tree maintainers. [AFAICT: you can move binding in Documentation/ to
> different place, that's not a problem; but creating a new binding when
> old one exists is.]
> 
> If you and Dan feel that is okay, you are welcome to try to get the
> patches past Rob, just please retain the NAK so that he remembers the
> discussion, and so that it is clear that I don't like the changes.
> 
> I believe smart thing to do is to try to do that before working
> further on the code, but of course, its all up to you :-).

I was looking for the review for the ti-lmu.txt binding on patchworks to see what
the comments were on the review or any explanation from reviewers to
why this implementation was done in the MFD.

Maybe there is some historical context that needs to be learned from here from
1.5 years ago.

I cannot seem to find any review posted in the patchworks archive.
I see reviews posted for the ti-lmu-backlight binding but none from
the ti-lmu.txt base binding.

Does anyone have a reference to the review?

If there is no review then I am wondering how this binding was accepted.
If there is no review and no current users then I would think that binding
 modification should be allowed.  But thats just my opinion.

Dan


> 
> Friends,
> 									Pavel
> 


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

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

* Re: [RFC PATCH v2 1/9] leds: add TI LMU backlight driver
  2018-10-03 12:00           ` Dan Murphy
@ 2018-10-03 12:10             ` Dan Murphy
  0 siblings, 0 replies; 21+ messages in thread
From: Dan Murphy @ 2018-10-03 12:10 UTC (permalink / raw)
  To: Pavel Machek, Jacek Anaszewski
  Cc: robh+dt, devicetree, linux-kernel, lee.jones, linux-omap,
	linux-leds, Sebastian Reichel, Lee Jones

Hello

On 10/03/2018 07:00 AM, Dan Murphy wrote:
> Hello
> 
> On 10/02/2018 05:07 PM, Pavel Machek wrote:
>> Hi!
>>
>>>> We have debated this over and over and now we have 3 different implementations
>>>> available we need to collude on which one we want to support.
>>>>
>>>> Jacek I defer to you and Pavel since you are both LED maintainers.
>>>>
>>>> I can support the dedicated LED drivers but I cannot support the TI LMU only implementation.
>>>
>>> I uphold my previous opinion - please go ahead with moving the support
>>> for non-MFD devices from MFD subsystem to the LED subsystem. And yes -
>>> along with the bindings. This is semantically correct, and yet we don't
>>> have mainline users.
>>>
>>> Pavel - you will have to engage more people for your crusade to prevail.
>>> For now, to speed up the things, I am forced to ignore your NAK.
>>> So NAK to your NAK. Sorry.
>>
>> No need to be sorry :-).
>>
>> Lets ignore the code for now, as code can be changed easily.
>>
>> Bindings are not something you or I maintain, so we don't have final
>> word there, and I have feeling this is not going to go past device
>> tree maintainers. [AFAICT: you can move binding in Documentation/ to
>> different place, that's not a problem; but creating a new binding when
>> old one exists is.]
>>
>> If you and Dan feel that is okay, you are welcome to try to get the
>> patches past Rob, just please retain the NAK so that he remembers the
>> discussion, and so that it is clear that I don't like the changes.
>>
>> I believe smart thing to do is to try to do that before working
>> further on the code, but of course, its all up to you :-).
> 
> I was looking for the review for the ti-lmu.txt binding on patchworks to see what
> the comments were on the review or any explanation from reviewers to
> why this implementation was done in the MFD.
> 
> Maybe there is some historical context that needs to be learned from here from
> 1.5 years ago.
> 
> I cannot seem to find any review posted in the patchworks archive.
> I see reviews posted for the ti-lmu-backlight binding but none from
> the ti-lmu.txt base binding.
> 
> Does anyone have a reference to the review?

I found the review or at least the reference for the ti-lmu.txt binding.

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

Does not appear that the binding was sent to the device tree mail list.
(Maybe that email list did not exist in Feb 2017).
Especially with the amount of change that is being submitted in the
newer patchsets.

Dan

> 
> If there is no review then I am wondering how this binding was accepted.
> If there is no review and no current users then I would think that binding
>  modification should be allowed.  But thats just my opinion.
> 
> Dan
> 
> 
>>
>> Friends,
>> 									Pavel
>>
> 
> 


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

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

* Re: [RFC PATCH v2 2/9] dt-bindings: ti-lmu: Remove LM3697
  2018-10-02  7:28   ` Pavel Machek
@ 2018-10-03 12:24     ` Dan Murphy
  2018-10-03 13:01       ` Pavel Machek
  0 siblings, 1 reply; 21+ messages in thread
From: Dan Murphy @ 2018-10-03 12:24 UTC (permalink / raw)
  To: Pavel Machek
  Cc: robh+dt, jacek.anaszewski, devicetree, linux-kernel, lee.jones,
	linux-omap, linux-leds

Hello

On 10/02/2018 02:28 AM, Pavel Machek wrote:
> On Fri 2018-09-28 13:29:47, 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, for reasons I explained before. Please add it to the patch so
> that it does not get applied by mistake. Ouch and AFAICT Rob was not
> happy with this either.
> 
> Yes, you are creating new drivers, ok; but that does _not_ mean you
> should create new binding.

I am copying my comment here on the review of this original binding for
records

I found the review or at least the reference for the ti-lmu.txt binding.

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

Does not appear that the binding was sent to the device tree mail list.
(Maybe that email list did not exist in Feb 2017).

Especially with the amount of change that is being submitted in the
newer patchsets.

<new content>
Having found this submission and no comments on the review I would think
we need to take an exception on these bindings.

Dan


> 									Pavel
> 


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

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

* Re: [RFC PATCH v2 2/9] dt-bindings: ti-lmu: Remove LM3697
  2018-10-03 12:24     ` Dan Murphy
@ 2018-10-03 13:01       ` Pavel Machek
  2018-10-03 20:46         ` Jacek Anaszewski
  0 siblings, 1 reply; 21+ messages in thread
From: Pavel Machek @ 2018-10-03 13:01 UTC (permalink / raw)
  To: Dan Murphy
  Cc: robh+dt, jacek.anaszewski, devicetree, linux-kernel, lee.jones,
	linux-omap, linux-leds

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

On Wed 2018-10-03 07:24:23, Dan Murphy wrote:
> Hello
> 
> On 10/02/2018 02:28 AM, Pavel Machek wrote:
> > On Fri 2018-09-28 13:29:47, 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, for reasons I explained before. Please add it to the patch so
> > that it does not get applied by mistake. Ouch and AFAICT Rob was not
> > happy with this either.
> > 
> > Yes, you are creating new drivers, ok; but that does _not_ mean you
> > should create new binding.
> 
> I am copying my comment here on the review of this original binding for
> records
> 
> I found the review or at least the reference for the ti-lmu.txt binding.
> 
> https://lore.kernel.org/patchwork/patch/764180/
> 
> Does not appear that the binding was sent to the device tree mail list.
> (Maybe that email list did not exist in Feb 2017).

Quick google shows:

https://lwn.net/Articles/666023/

Now can we stop this nonsense? If there is a problem with the binding,
submit patches to fix the problem.

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

* Re: [RFC PATCH v2 2/9] dt-bindings: ti-lmu: Remove LM3697
  2018-10-03 13:01       ` Pavel Machek
@ 2018-10-03 20:46         ` Jacek Anaszewski
  2018-10-04 13:26           ` Dan Murphy
  0 siblings, 1 reply; 21+ messages in thread
From: Jacek Anaszewski @ 2018-10-03 20:46 UTC (permalink / raw)
  To: Pavel Machek, Dan Murphy
  Cc: robh+dt, devicetree, linux-kernel, lee.jones, linux-omap, linux-leds

On 10/03/2018 03:01 PM, Pavel Machek wrote:
> On Wed 2018-10-03 07:24:23, Dan Murphy wrote:
>> Hello
>>
>> On 10/02/2018 02:28 AM, Pavel Machek wrote:
>>> On Fri 2018-09-28 13:29:47, 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, for reasons I explained before. Please add it to the patch so
>>> that it does not get applied by mistake. Ouch and AFAICT Rob was not
>>> happy with this either.
>>>
>>> Yes, you are creating new drivers, ok; but that does _not_ mean you
>>> should create new binding.
>>
>> I am copying my comment here on the review of this original binding for
>> records
>>
>> I found the review or at least the reference for the ti-lmu.txt binding.
>>
>> https://lore.kernel.org/patchwork/patch/764180/
>>
>> Does not appear that the binding was sent to the device tree mail list.
>> (Maybe that email list did not exist in Feb 2017).
> 
> Quick google shows:
> 
> https://lwn.net/Articles/666023/

This link refreshed my memory and allowed me to recall Milo's patch set
from the end of 2015, and the related discussion. I had an impression
that I had had some request regarding the bindings but there was no
follow-up.

I've googled that thread [0] and it proved I was right [1].

From Milo's messages we can infer that there will be next
version of the patch set and it appeared but over a year later [2],
and without the leds-lm3633 driver and related bindings, and without
drivers/video/backlight/ti-lmu-backlight-core.c.

Patch set gets merged despite dangling DT references.

Dan, I propose you to resend the patch removing the bindings from
MFD, and explain the rationale in the patch below commit message
after "---" .

> Now can we stop this nonsense? If there is a problem with the binding,
> submit patches to fix the problem.

[0]
https://lore.kernel.org/lkml/1448521025-2796-1-git-send-email-milo.kim@ti.com/
[1] https://lore.kernel.org/lkml/56656468.8020300@samsung.com/
[2]
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1341860.html

-- 
Best regards,
Jacek Anaszewski

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

* Re: [RFC PATCH v2 2/9] dt-bindings: ti-lmu: Remove LM3697
  2018-10-03 20:46         ` Jacek Anaszewski
@ 2018-10-04 13:26           ` Dan Murphy
  0 siblings, 0 replies; 21+ messages in thread
From: Dan Murphy @ 2018-10-04 13:26 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek
  Cc: robh+dt, devicetree, linux-kernel, lee.jones, linux-omap, linux-leds

Jacek

On 10/03/2018 03:46 PM, Jacek Anaszewski wrote:
> On 10/03/2018 03:01 PM, Pavel Machek wrote:
>> On Wed 2018-10-03 07:24:23, Dan Murphy wrote:
>>> Hello
>>>
>>> On 10/02/2018 02:28 AM, Pavel Machek wrote:
>>>> On Fri 2018-09-28 13:29:47, 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, for reasons I explained before. Please add it to the patch so
>>>> that it does not get applied by mistake. Ouch and AFAICT Rob was not
>>>> happy with this either.
>>>>
>>>> Yes, you are creating new drivers, ok; but that does _not_ mean you
>>>> should create new binding.
>>>
>>> I am copying my comment here on the review of this original binding for
>>> records
>>>
>>> I found the review or at least the reference for the ti-lmu.txt binding.
>>>
>>> https://lore.kernel.org/patchwork/patch/764180/
>>>
>>> Does not appear that the binding was sent to the device tree mail list.
>>> (Maybe that email list did not exist in Feb 2017).
>>
>> Quick google shows:
>>
>> https://lwn.net/Articles/666023/
> 
> This link refreshed my memory and allowed me to recall Milo's patch set
> from the end of 2015, and the related discussion. I had an impression
> that I had had some request regarding the bindings but there was no
> follow-up.
> 
> I've googled that thread [0] and it proved I was right [1].
> 
> From Milo's messages we can infer that there will be next
> version of the patch set and it appeared but over a year later [2],
> and without the leds-lm3633 driver and related bindings, and without
> drivers/video/backlight/ti-lmu-backlight-core.c.
> 
> Patch set gets merged despite dangling DT references.
> 
> Dan, I propose you to resend the patch removing the bindings from
> MFD, and explain the rationale in the patch below commit message
> after "---" .
> 
>> Now can we stop this nonsense? If there is a problem with the binding,
>> submit patches to fix the problem.
> 
> [0]
> https://lore.kernel.org/lkml/1448521025-2796-1-git-send-email-milo.kim@ti.com/
> [1] https://lore.kernel.org/lkml/56656468.8020300@samsung.com/
> [2]
> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1341860.html
> 

Sounds good I will clean this up and submit a v3 non-RFC edition

Dan

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

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

end of thread, other threads:[~2018-10-04 13:27 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-28 18:29 [RFC PATCH v2 0/9] TI LMU and Dedicated Drivers Dan Murphy
2018-09-28 18:29 ` [RFC PATCH v2 1/9] leds: add TI LMU backlight driver Dan Murphy
2018-10-02  7:56   ` Pavel Machek
2018-10-02 12:32     ` Dan Murphy
2018-10-02 18:52       ` Jacek Anaszewski
2018-10-02 22:07         ` Pavel Machek
2018-10-03 12:00           ` Dan Murphy
2018-10-03 12:10             ` Dan Murphy
2018-09-28 18:29 ` [RFC PATCH v2 2/9] dt-bindings: ti-lmu: Remove LM3697 Dan Murphy
2018-10-02  7:28   ` Pavel Machek
2018-10-03 12:24     ` Dan Murphy
2018-10-03 13:01       ` Pavel Machek
2018-10-03 20:46         ` Jacek Anaszewski
2018-10-04 13:26           ` Dan Murphy
2018-09-28 18:29 ` [RFC PATCH v2 3/9] mfd: ti-lmu: Remove support for LM3697 Dan Murphy
2018-09-28 18:29 ` [RFC PATCH v2 4/9] dt-bindings: leds: Add bindings for lm3697 driver Dan Murphy
2018-09-28 18:29 ` [RFC PATCH v2 5/9] leds: lm3697: Introduce the " Dan Murphy
2018-09-28 18:29 ` [RFC PATCH v2 6/9] dt-bindings: leds: Add support for the LM3633 Dan Murphy
2018-09-28 18:29 ` [RFC PATCH v2 7/9] leds: lm3633: Introduce the lm3633 driver Dan Murphy
2018-09-28 18:29 ` [RFC PATCH v2 8/9] dt-bindings: leds: Add the LM3632 LED dt binding Dan Murphy
2018-09-28 18:29 ` [RFC PATCH v2 9/9] leds: lm3632: Introduce the TI LM3632 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).