Skip to contentSkip to conversation

TI LMU rework

PATCH Superseded Dan Murphy 4 participants 24 messages Mar 25, 2019 – Apr 15, 2019

Subsystems (3)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Dan Murphy

author
#0 lore raw

cover TI LMU rework

All

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

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

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

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

Dan

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

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

PATCH 1/5 dt-bindings: mfd: Update the ramp up/down property

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

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

Re: [PATCH 1/5] dt-bindings: mfd: Update the ramp up/down property

Quoted 2 lines · Dan Murphy

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

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

Quoted 62 lines
---
 .../devicetree/bindings/mfd/ti-lmu.txt        | 20 ++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)
Documentation/devicetree/bindings/mfd/ti-lmu.txt +13 -7
index 980394d701a7..d9d529de6dc1 100644
@@ -25,6 +25,12 @@ Required properties:
 
 Optional property:
   - enable-gpios: A GPIO specifier for hardware enable pin.
+  - ramp-up-ms: Current ramping from one brightness level to
+		the a higher brightness level.
+		Range from 2048 us - 117.44 s
+  - ramp-down-ms: Current ramping from one brightness level to
+		  the a lower brightness level.
+		  Range from 2048 us - 117.44 s
 
 Required node:
   - backlight: All LMU devices have backlight child nodes.
@@ -90,7 +96,7 @@ lm3631@29 {
 
 		lcd_bl {
 			led-sources = <0 1>;
-			ramp-up-msec = <300>;
+			ramp-up-ms = <300>;
 		};
 	};
 };
@@ -152,15 +158,15 @@ lm3633@36 {
 		main {
 			label = "main_lcd";
 			led-sources = <1 2>;
-			ramp-up-msec = <500>;
-			ramp-down-msec = <500>;
+			ramp-up-ms = <500>;
+			ramp-down-ms = <500>;
 		};
 
 		front {
 			label = "front_lcd";
 			led-sources = <0>;
-			ramp-up-msec = <1000>;
-			ramp-down-msec = <0>;
+			ramp-up-ms = <1000>;
+			ramp-down-ms = <0>;
 		};
 	};
 
@@ -212,8 +218,8 @@ lm3697@36 {
 
 		lcd {
 			led-sources = <0 1 2>;
-			ramp-up-msec = <200>;
-			ramp-down-msec = <200>;
+			ramp-up-ms = <200>;
+			ramp-down-ms = <200>;
 		};
 	};
 
Signature · 4 lines
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
author
#3 lore raw

Re: [PATCH 1/5] dt-bindings: mfd: Update the ramp up/down property

Lee

Thanks for the review

Quoted 7 lines · Lee Jones
Quoted 2 lines · Dan Murphy

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

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

I will add the why in v2.

Dan
<snip>

Signature · 2 lines
------------------
Dan Murphy
author
#4 lore raw

PATCH 2/5 leds: TI LMU: Add common code for TI LMU devices

In reply to Dan Murphy

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

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

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

---
 drivers/leds/Kconfig              |   8 ++
 drivers/leds/Makefile             |   1 +
 drivers/leds/ti-lmu-led-common.c  | 131 ++++++++++++++++++++++++++++++
 include/linux/ti-lmu-led-common.h |  44 ++++++++++
 4 files changed, 184 insertions(+)
 create mode 100644 drivers/leds/ti-lmu-led-common.c
 create mode 100644 include/linux/ti-lmu-led-common.h
drivers/leds/Kconfig +8 -0
index da00b9ed5a5c..3ae24eaf4cde 100644
@@ -776,6 +776,14 @@ config LEDS_NIC78BX
 	  To compile this driver as a module, choose M here: the module
 	  will be called leds-nic78bx.
 
+config LEDS_TI_LMU_COMMON
+	tristate "LED driver for TI LMU"
+	depends on REGMAP
+	help
+          Say Y to enable the LED driver for TI LMU devices.
+          This supports common features between the TI LM3532, LM3631, LM3632,
+	  LM3633, LM3695 and LM3697.
+
 comment "LED Triggers"
 source "drivers/leds/trigger/Kconfig"
 
drivers/leds/Makefile +1 -0
index 7a8b1f55d459..85fd449c002d 100644
@@ -80,6 +80,7 @@ obj-$(CONFIG_LEDS_MT6323)		+= leds-mt6323.o
 obj-$(CONFIG_LEDS_LM3692X)		+= leds-lm3692x.o
 obj-$(CONFIG_LEDS_SC27XX_BLTC)		+= leds-sc27xx-bltc.o
 obj-$(CONFIG_LEDS_LM3601X)		+= leds-lm3601x.o
+obj-$(CONFIG_LEDS_TI_LMU_COMMON)	+= ti-lmu-led-common.o
 
 # LED SPI Drivers
 obj-$(CONFIG_LEDS_CR0014114)		+= leds-cr0014114.o
drivers/leds/ti-lmu-led-common.c new file +131 -0

131 lines not on this page · raw message

include/linux/ti-lmu-led-common.h new file +44 -0

44 lines not on this page · raw message

Signature · 1 line
2.19.0
Acked-by on v1 2/5
#5 lore raw

Re: [PATCH 2/5] leds: TI LMU: Add common code for TI LMU devices

Quoted 9 lines · Dan Murphy

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

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

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

author
#6 lore raw

PATCH 3/5 dt-bindings: ti-lmu: Modify dt bindings for the LM3697

In reply to Dan Murphy

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

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

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

Re: [PATCH 3/5] dt-bindings: ti-lmu: Modify dt bindings for the LM3697

Hi Dan,

Thank you for the patch.

You need Lee Jones on CC for this series.

One more comment below.

Quoted 47 lines · Dan Murphy

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

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

---
  .../devicetree/bindings/leds/leds-lm3697.txt  | 77 +++++++++++++++++++
  .../devicetree/bindings/mfd/ti-lmu.txt        | 26 +------
  2 files changed, 78 insertions(+), 25 deletions(-)

create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3697.txt

Documentation/devicetree/bindings/leds/leds-lm3697.txt new file +24 -0
new file mode 100644
index 000000000000..a780f11acd38
@@ -0,0 +1,77 @@
+* Texas Instruments - LM3697 Highly Efficient White LED Driver
+
+The LM3697 11-bit LED driver provides high-
+performance backlight dimming for 1, 2, or 3 series
+LED strings while delivering up to 90% efficiency.
+
+This device is suitable for display and keypad Lighting
+
+Required properties:
+	- compatible:
+		"ti,lm3697"
+	- reg :  I2C slave address
+	- #address-cells : 1
+	- #size-cells : 0
+
+Optional properties:
+	- enable-gpios : GPIO pin to enable/disable the device
+	- vled-supply : LED supply
+
+Required child properties:
+	- reg : 0 - LED is Controlled by bank A
+		1 - LED is Controlled by bank B
+	- led-sources : Indicates which HVLED string is associated to which
+			control bank.  This is a zero based property so

Documentation/devicetree/bindings/leds/leds-lm3697.txt line 34

Quoted 10 lines
Documentation/devicetree/bindings/leds/leds-lm3697.txt new file +10 -0
+			HVLED1 = 0, HVLED2 = 1, HVLED3 = 2.
+			Additional information is contained
+			in Documentation/devicetree/bindings/leds/common.txt
+
+Optional child properties:
+	- max_brightness - This determines whether to use 8 bit brightness mode
+			   or 11 bit brightness mode.  If this value is not
+			   set the device is defaulted to the preferred 8bit
+			   brightness mode per 7.3.4.1 of the data sheet.
+			   The values are 255 (8bit) or 2047 (11bit).

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

Signature · 2 lines
Best regards,
Jacek Anaszewski
author
#8 lore raw

Re: [PATCH 3/5] dt-bindings: ti-lmu: Modify dt bindings for the LM3697

Jacek

Quoted 6 lines · Jacek Anaszewski

Hi Dan,

Thank you for the patch.

You need Lee Jones on CC for this series.

Yes I saw I missed Lee.

Quoted 54 lines

One more comment below.

Quoted 51 lines · Dan Murphy

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

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

---
  .../devicetree/bindings/leds/leds-lm3697.txt  | 77 +++++++++++++++++++ .../devicetree/bindings/mfd/ti-lmu.txt        | 26 +------

  2 files changed, 78 insertions(+), 25 deletions(-)

  create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3697.txt

Documentation/devicetree/bindings/leds/leds-lm3697.txt new file +28 -0
new file mode 100644
index 000000000000..a780f11acd38
@@ -0,0 +1,77 @@
+* Texas Instruments - LM3697 Highly Efficient White LED Driver
+
+The LM3697 11-bit LED driver provides high-
+performance backlight dimming for 1, 2, or 3 series
+LED strings while delivering up to 90% efficiency.
+
+This device is suitable for display and keypad Lighting
+
+Required properties:
+    - compatible:
+        "ti,lm3697"
+    - reg :  I2C slave address
+    - #address-cells : 1
+    - #size-cells : 0
+
+Optional properties:
+    - enable-gpios : GPIO pin to enable/disable the device
+    - vled-supply : LED supply
+
+Required child properties:
+    - reg : 0 - LED is Controlled by bank A
+        1 - LED is Controlled by bank B
+    - led-sources : Indicates which HVLED string is associated to which
+            control bank.  This is a zero based property so
+            HVLED1 = 0, HVLED2 = 1, HVLED3 = 2.
+            Additional information is contained
+            in Documentation/devicetree/bindings/leds/common.txt
+
Quoted 9 lines

Documentation/devicetree/bindings/leds/leds-lm3697.txt line 34

Quoted 6 lines
Documentation/devicetree/bindings/leds/leds-lm3697.txt new file +6 -0
+Optional child properties:
+    - max_brightness - This determines whether to use 8 bit brightness mode
+               or 11 bit brightness mode.  If this value is not
+               set the device is defaulted to the preferred 8bit
+               brightness mode per 7.3.4.1 of the data sheet.
+               The values are 255 (8bit) or 2047 (11bit).

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

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

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

Dan

Signature · 2 lines
------------------
Dan Murphy
#9 lore raw

Re: [PATCH 3/5] dt-bindings: ti-lmu: Modify dt bindings for the LM3697

Dan,

Quoted 76 lines · Dan Murphy

Jacek

Quoted 6 lines · Jacek Anaszewski

Hi Dan,

Thank you for the patch.

You need Lee Jones on CC for this series.

Yes I saw I missed Lee.

Quoted 54 lines

One more comment below.

Quoted 51 lines · Dan Murphy

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

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

---
   .../devicetree/bindings/leds/leds-lm3697.txt  | 77 +++++++++++++++++++
   .../devicetree/bindings/mfd/ti-lmu.txt        | 26 +------
   2 files changed, 78 insertions(+), 25 deletions(-)

  create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3697.txt

Documentation/devicetree/bindings/leds/leds-lm3697.txt new file +28 -0
new file mode 100644
index 000000000000..a780f11acd38
@@ -0,0 +1,77 @@
+* Texas Instruments - LM3697 Highly Efficient White LED Driver
+
+The LM3697 11-bit LED driver provides high-
+performance backlight dimming for 1, 2, or 3 series
+LED strings while delivering up to 90% efficiency.
+
+This device is suitable for display and keypad Lighting
+
+Required properties:
+    - compatible:
+        "ti,lm3697"
+    - reg :  I2C slave address
+    - #address-cells : 1
+    - #size-cells : 0
+
+Optional properties:
+    - enable-gpios : GPIO pin to enable/disable the device
+    - vled-supply : LED supply
+
+Required child properties:
+    - reg : 0 - LED is Controlled by bank A
+        1 - LED is Controlled by bank B
+    - led-sources : Indicates which HVLED string is associated to which
+            control bank.  This is a zero based property so
+            HVLED1 = 0, HVLED2 = 1, HVLED3 = 2.
+            Additional information is contained
+            in Documentation/devicetree/bindings/leds/common.txt
+
Quoted 9 lines

Documentation/devicetree/bindings/leds/leds-lm3697.txt line 34

Quoted 6 lines
Documentation/devicetree/bindings/leds/leds-lm3697.txt new file +6 -0
+Optional child properties:
+    - max_brightness - This determines whether to use 8 bit brightness mode
+               or 11 bit brightness mode.  If this value is not
+               set the device is defaulted to the preferred 8bit
+               brightness mode per 7.3.4.1 of the data sheet.
+               The values are 255 (8bit) or 2047 (11bit).

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

Quoted 2 lines

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

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

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

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

Quoted 4 lines

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

Dan

Signature · 2 lines
Best regards,
Jacek Anaszewski
author
#10 lore raw

Re: [PATCH 3/5] dt-bindings: ti-lmu: Modify dt bindings for the LM3697

Quoted 89 lines · Jacek Anaszewski

Dan,

Quoted 76 lines · Dan Murphy

Jacek

Quoted 6 lines · Jacek Anaszewski

Hi Dan,

Thank you for the patch.

You need Lee Jones on CC for this series.

Yes I saw I missed Lee.

Quoted 54 lines

One more comment below.

Quoted 51 lines · Dan Murphy

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

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

---
   .../devicetree/bindings/leds/leds-lm3697.txt  | 77 +++++++++++++++++++ .../devicetree/bindings/mfd/ti-lmu.txt        | 26 +------

   2 files changed, 78 insertions(+), 25 deletions(-)

   create mode 100644 Documentation/devicetree/bindings/leds/leds-lm3697.txt

Documentation/devicetree/bindings/leds/leds-lm3697.txt new file +28 -0
new file mode 100644
index 000000000000..a780f11acd38
@@ -0,0 +1,77 @@
+* Texas Instruments - LM3697 Highly Efficient White LED Driver
+
+The LM3697 11-bit LED driver provides high-
+performance backlight dimming for 1, 2, or 3 series
+LED strings while delivering up to 90% efficiency.
+
+This device is suitable for display and keypad Lighting
+
+Required properties:
+    - compatible:
+        "ti,lm3697"
+    - reg :  I2C slave address
+    - #address-cells : 1
+    - #size-cells : 0
+
+Optional properties:
+    - enable-gpios : GPIO pin to enable/disable the device
+    - vled-supply : LED supply
+
+Required child properties:
+    - reg : 0 - LED is Controlled by bank A
+        1 - LED is Controlled by bank B
+    - led-sources : Indicates which HVLED string is associated to which
+            control bank.  This is a zero based property so
+            HVLED1 = 0, HVLED2 = 1, HVLED3 = 2.
+            Additional information is contained
+            in Documentation/devicetree/bindings/leds/common.txt
+
Quoted 9 lines

Documentation/devicetree/bindings/leds/leds-lm3697.txt line 34

Quoted 6 lines
Documentation/devicetree/bindings/leds/leds-lm3697.txt new file +6 -0
+Optional child properties:
+    - max_brightness - This determines whether to use 8 bit brightness mode
+               or 11 bit brightness mode.  If this value is not
+               set the device is defaulted to the preferred 8bit
+               brightness mode per 7.3.4.1 of the data sheet.
+               The values are 255 (8bit) or 2047 (11bit).

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

Quoted 2 lines

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

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

Quoted 7 lines

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

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

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

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

Dan

Quoted 6 lines
Quoted 5 lines

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

Dan

Signature · 2 lines
------------------
Dan Murphy
#11 lore raw

Re: [PATCH 3/5] dt-bindings: ti-lmu: Modify dt bindings for the LM3697

On 4/4/19 9:27 PM, Dan Murphy wrote:
[...]

Quoted 5 lines

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

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

ti,brightness-resolution sounds good to me.

Rob has the last word, though.

Signature · 2 lines
Best regards,
Jacek Anaszewski
author
#12 lore raw

Re: [PATCH 3/5] dt-bindings: ti-lmu: Modify dt bindings for the LM3697

In reply to Dan Murphy

Jacek

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

Quoted 12 lines
Quoted 12 lines
Quoted 4 lines
Quoted 1 line

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

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

Quoted 7 lines
Quoted 7 lines

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

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

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

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

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

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

Dan

<snip>

Signature · 2 lines
------------------
Dan Murphy
author
#13 lore raw

PATCH 4/5 mfd: ti-lmu: Remove support for LM3697

In reply to Dan Murphy

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

---
 drivers/leds/Kconfig                |  2 +-
 drivers/mfd/Kconfig                 |  2 +-
 drivers/mfd/ti-lmu.c                | 17 -----------
 include/linux/mfd/ti-lmu-register.h | 44 -----------------------------
 include/linux/mfd/ti-lmu.h          |  1 -
 5 files changed, 2 insertions(+), 64 deletions(-)
drivers/leds/Kconfig +1 -1
index 3ae24eaf4cde..735009e73414 100644
@@ -782,7 +782,7 @@ config LEDS_TI_LMU_COMMON
 	help
           Say Y to enable the LED driver for TI LMU devices.
           This supports common features between the TI LM3532, LM3631, LM3632,
-	  LM3633, LM3695 and LM3697.
+	  LM3633, and LM3695.
 
 comment "LED Triggers"
 source "drivers/leds/trigger/Kconfig"
drivers/mfd/Kconfig +1 -1
index 0ce2d8dfc5f1..fa70ea209ec6 100644
@@ -1312,7 +1312,7 @@ config MFD_TI_LMU
 	help
 	  Say yes here to enable support for TI LMU chips.
 
-	  TI LMU MFD supports LM3532, LM3631, LM3632, LM3633, LM3695 and LM3697.
+	  TI LMU MFD supports LM3532, LM3631, LM3632, LM3633, and LM3695.
 	  It consists of backlight, LED and regulator driver.
 	  It provides consistent device controls for lighting functions.
 
drivers/mfd/ti-lmu.c +0 -17
index b06cb908d1aa..89b1c5b584af 100644
@@ -111,20 +111,6 @@ static const struct mfd_cell lm3695_devices[] = {
 	},
 };
 
-static const struct mfd_cell lm3697_devices[] = {
-	{
-		.name          = "ti-lmu-backlight",
-		.id            = LM3697,
-		.of_compatible = "ti,lm3697-backlight",
-	},
-	/* Monitoring driver for open/short circuit detection */
-	{
-		.name          = "ti-lmu-fault-monitor",
-		.id            = LM3697,
-		.of_compatible = "ti,lm3697-fault-monitor",
-	},
-};
-
 #define TI_LMU_DATA(chip, max_reg)		\
 static const struct ti_lmu_data chip##_data =	\
 {						\
@@ -137,7 +123,6 @@ TI_LMU_DATA(lm3631, LM3631_MAX_REG);
 TI_LMU_DATA(lm3632, LM3632_MAX_REG);
 TI_LMU_DATA(lm3633, LM3633_MAX_REG);
 TI_LMU_DATA(lm3695, LM3695_MAX_REG);
-TI_LMU_DATA(lm3697, LM3697_MAX_REG);
 
 static int ti_lmu_probe(struct i2c_client *cl, const struct i2c_device_id *id)
 {
@@ -206,7 +191,6 @@ static const struct of_device_id ti_lmu_of_match[] = {
 	{ .compatible = "ti,lm3632", .data = &lm3632_data },
 	{ .compatible = "ti,lm3633", .data = &lm3633_data },
 	{ .compatible = "ti,lm3695", .data = &lm3695_data },
-	{ .compatible = "ti,lm3697", .data = &lm3697_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ti_lmu_of_match);
@@ -216,7 +200,6 @@ static const struct i2c_device_id ti_lmu_ids[] = {
 	{ "lm3632", LM3632 },
 	{ "lm3633", LM3633 },
 	{ "lm3695", LM3695 },
-	{ "lm3697", LM3697 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, ti_lmu_ids);
include/linux/mfd/ti-lmu-register.h +0 -44

48 lines not on this page · raw message

include/linux/mfd/ti-lmu.h +0 -1
index 7762c1bce55d..54e9d272e81c 100644
@@ -26,7 +26,6 @@ enum ti_lmu_id {
 	LM3632,
 	LM3633,
 	LM3695,
-	LM3697,
 	LMU_MAX_ID,
 };
 
Signature · 1 line
2.19.0
#14 lore raw

Re: [PATCH 4/5] mfd: ti-lmu: Remove support for LM3697

Quoted 1 line · Dan Murphy

Remove support for the LM3697 from the ti-lmu

Bit of an odd place to insert a line feed.

Quoted 10 lines

driver in favor of a dedicated LED driver.

---
 drivers/leds/Kconfig                |  2 +-
 drivers/mfd/Kconfig                 |  2 +-
 drivers/mfd/ti-lmu.c                | 17 -----------
 include/linux/mfd/ti-lmu-register.h | 44 -----------------------------
 include/linux/mfd/ti-lmu.h          |  1 -
 5 files changed, 2 insertions(+), 64 deletions(-)

I'm fine with the patch.

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

Signature · 4 lines
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#15 lore raw

Re: [PATCH 4/5] mfd: ti-lmu: Remove support for LM3697

Quoted 17 lines · Lee Jones
Quoted 1 line · Dan Murphy

Remove support for the LM3697 from the ti-lmu

Bit of an odd place to insert a line feed.

Quoted 10 lines

driver in favor of a dedicated LED driver.

---
 drivers/leds/Kconfig                |  2 +-
 drivers/mfd/Kconfig                 |  2 +-
 drivers/mfd/ti-lmu.c                | 17 -----------
 include/linux/mfd/ti-lmu-register.h | 44 -----------------------------
 include/linux/mfd/ti-lmu.h          |  1 -
 5 files changed, 2 insertions(+), 64 deletions(-)
Quoted 5 lines

I'm fine with the patch.

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

BTW, please add my:

For my own reference:

Signature · 4 lines
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
author
#16 lore raw

Re: [PATCH 4/5] mfd: ti-lmu: Remove support for LM3697

In reply to Lee Jones

Lee

Thanks for the review

Quoted 6 lines · Lee Jones
Quoted 1 line · Dan Murphy

Remove support for the LM3697 from the ti-lmu

Bit of an odd place to insert a line feed.

I will fix in v2.

Quoted 11 lines
Quoted 10 lines

driver in favor of a dedicated LED driver.

---
 drivers/leds/Kconfig                |  2 +-
 drivers/mfd/Kconfig                 |  2 +-
 drivers/mfd/ti-lmu.c                | 17 -----------
 include/linux/mfd/ti-lmu-register.h | 44 -----------------------------
 include/linux/mfd/ti-lmu.h          |  1 -
 5 files changed, 2 insertions(+), 64 deletions(-)
Quoted 6 lines

I'm fine with the patch.

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

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

Dan

Signature · 2 lines
------------------
Dan Murphy
author
#17 lore raw

PATCH 5/5 leds: lm3697: Introduce the lm3697 driver

In reply to Dan Murphy

Introduce the lm3697 LED driver for
backlighting and display.

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

---
 drivers/leds/Kconfig       |   8 +-
 drivers/leds/Makefile      |   1 +
 drivers/leds/leds-lm3697.c | 401 +++++++++++++++++++++++++++++++++++++
 3 files changed, 409 insertions(+), 1 deletion(-)
 create mode 100644 drivers/leds/leds-lm3697.c
drivers/leds/Kconfig +7 -1
index 735009e73414..688bb9a6f275 100644
@@ -776,9 +776,15 @@ config LEDS_NIC78BX
 	  To compile this driver as a module, choose M here: the module
 	  will be called leds-nic78bx.
 
+config LEDS_LM3697
+	tristate "LED driver for LM3697"
+	depends on LEDS_TI_LMU_COMMON
+	help
+	  Say Y to enable the LM3697 LED driver for TI LMU devices.
+	  This supports the LED device LM3697.
+
 config LEDS_TI_LMU_COMMON
 	tristate "LED driver for TI LMU"
-	depends on REGMAP
 	help
           Say Y to enable the LED driver for TI LMU devices.
           This supports common features between the TI LM3532, LM3631, LM3632,
drivers/leds/Makefile +1 -0
index 85fd449c002d..2eb0225d8dc6 100644
@@ -80,6 +80,7 @@ obj-$(CONFIG_LEDS_MT6323)		+= leds-mt6323.o
 obj-$(CONFIG_LEDS_LM3692X)		+= leds-lm3692x.o
 obj-$(CONFIG_LEDS_SC27XX_BLTC)		+= leds-sc27xx-bltc.o
 obj-$(CONFIG_LEDS_LM3601X)		+= leds-lm3601x.o
+obj-$(CONFIG_LEDS_LM3697)		+= leds-lm3697.o
 obj-$(CONFIG_LEDS_TI_LMU_COMMON)	+= ti-lmu-led-common.o
 
 # LED SPI Drivers
drivers/leds/leds-lm3697.c new file +401 -0

401 lines not on this page · raw message

Signature · 1 line
2.19.0
#18 lore raw

Re: [PATCH 5/5] leds: lm3697: Introduce the lm3697 driver

Quoted 25 lines · Dan Murphy

Introduce the lm3697 LED driver for
backlighting and display.

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

---
 drivers/leds/Kconfig       |   8 +-
 drivers/leds/Makefile      |   1 +
 drivers/leds/leds-lm3697.c | 401 +++++++++++++++++++++++++++++++++++++
 3 files changed, 409 insertions(+), 1 deletion(-)
 create mode 100644 drivers/leds/leds-lm3697.c
drivers/leds/Kconfig +3 -0
index 735009e73414..688bb9a6f275 100644
@@ -776,9 +776,15 @@ config LEDS_NIC78BX
 	  To compile this driver as a module, choose M here: the module
 	  will be called leds-nic78bx.
 
+config LEDS_LM3697
+	tristate "LED driver for LM3697"
+	depends on LEDS_TI_LMU_COMMON

drivers/leds/Kconfig config LEDS_NIC78BX · line 790

Quoted 10 lines
drivers/leds/Kconfig +4 -1
+	help
+	  Say Y to enable the LM3697 LED driver for TI LMU devices.
+	  This supports the LED device LM3697.
+
 config LEDS_TI_LMU_COMMON
 	tristate "LED driver for TI LMU"
-	depends on REGMAP
 	help
           Say Y to enable the LED driver for TI LMU devices.
           This supports common features between the TI LM3532, LM3631, LM3632,

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

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

Quoted 14 lines
drivers/leds/Kconfig +14 -0
+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;
+		}
+	}

drivers/leds/Kconfig

Quoted 10 lines
drivers/leds/Kconfig +10 -0
+
+	ret = regmap_write(priv->regmap, LM3697_CTRL_ENABLE, 0x0);
+	if (ret) {
+		dev_err(&priv->client->dev, "Cannot write ctrl enable\n");
+		goto out;
+	}
+
+	ret = regmap_write(priv->regmap, LM3697_OUTPUT_CONFIG, priv->bank_cfg);
+	if (ret)
+		dev_err(&priv->client->dev, "Cannot write OUTPUT config\n");

Missing goto out?

drivers/leds/Kconfig

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

Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#19 lore raw

Re: [PATCH 5/5] leds: lm3697: Introduce the lm3697 driver

Pavel

Quoted 23 lines · Pavel Machek
Quoted 22 lines · Dan Murphy

Introduce the lm3697 LED driver for
backlighting and display.

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

---
 drivers/leds/Kconfig       |   8 +-
 drivers/leds/Makefile      |   1 +
 drivers/leds/leds-lm3697.c | 401 +++++++++++++++++++++++++++++++++++++
 3 files changed, 409 insertions(+), 1 deletion(-)
 create mode 100644 drivers/leds/leds-lm3697.c
drivers/leds/Kconfig +0 -0
index 735009e73414..688bb9a6f275 100644
@@ -776,9 +776,15 @@ config LEDS_NIC78BX
 	  To compile this driver as a module, choose M here: the module
 	  will be called leds-nic78bx.
 
Quoted 16 lines

drivers/leds/Kconfig config LEDS_NIC78BX · line 790

Quoted 13 lines
drivers/leds/Kconfig +7 -1
+config LEDS_LM3697
+	tristate "LED driver for LM3697"
+	depends on LEDS_TI_LMU_COMMON
+	help
+	  Say Y to enable the LM3697 LED driver for TI LMU devices.
+	  This supports the LED device LM3697.
+
 config LEDS_TI_LMU_COMMON
 	tristate "LED driver for TI LMU"
-	depends on REGMAP
 	help
           Say Y to enable the LED driver for TI LMU devices.
           This supports common features between the TI LM3532, LM3631, LM3632,

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

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

Quoted 4 lines

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

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

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

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

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

Quoted 18 lines
Quoted 18 lines
drivers/leds/Kconfig +18 -0
+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");
Quoted 8 lines

drivers/leds/Kconfig

Quoted 6 lines
drivers/leds/Kconfig +6 -0
+		goto out;
+	}
+
+	ret = regmap_write(priv->regmap, LM3697_OUTPUT_CONFIG, priv->bank_cfg);
+	if (ret)
+		dev_err(&priv->client->dev, "Cannot write OUTPUT config\n");

Missing goto out?

Ack

Quoted 12 lines

drivers/leds/Kconfig

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

Pavel

#20 lore raw

Re: [PATCH 5/5] leds: lm3697: Introduce the lm3697 driver

Quoted 22 lines
Quoted 22 lines
Quoted 22 lines
drivers/leds/Kconfig +7 -1
index 735009e73414..688bb9a6f275 100644
@@ -776,9 +776,15 @@ config LEDS_NIC78BX
 	  To compile this driver as a module, choose M here: the module
 	  will be called leds-nic78bx.
 
+config LEDS_LM3697
+	tristate "LED driver for LM3697"
+	depends on LEDS_TI_LMU_COMMON
+	help
+	  Say Y to enable the LM3697 LED driver for TI LMU devices.
+	  This supports the LED device LM3697.
+
 config LEDS_TI_LMU_COMMON
 	tristate "LED driver for TI LMU"
-	depends on REGMAP
 	help
           Say Y to enable the LED driver for TI LMU devices.
           This supports common features between the TI LM3532, LM3631, LM3632,

...

Quoted 4 lines
Quoted 2 lines

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

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

Oh, no, that is not what I meant.

You have

config B
depends on A

config A

above. We really want

config A

config B
depends on A

Quoted 4 lines

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

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

I did not review it in great detail, so no acked-by's, yet :-).
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#21 lore raw

Re: [PATCH 5/5] leds: lm3697: Introduce the lm3697 driver

Quoted 35 lines · Pavel Machek
Quoted 22 lines
Quoted 22 lines
Quoted 22 lines
drivers/leds/Kconfig +7 -1
index 735009e73414..688bb9a6f275 100644
@@ -776,9 +776,15 @@ config LEDS_NIC78BX
 	  To compile this driver as a module, choose M here: the module
 	  will be called leds-nic78bx.
 
+config LEDS_LM3697
+	tristate "LED driver for LM3697"
+	depends on LEDS_TI_LMU_COMMON
+	help
+	  Say Y to enable the LM3697 LED driver for TI LMU devices.
+	  This supports the LED device LM3697.
+
 config LEDS_TI_LMU_COMMON
 	tristate "LED driver for TI LMU"
-	depends on REGMAP
 	help
           Say Y to enable the LED driver for TI LMU devices.
           This supports common features between the TI LM3532, LM3631, LM3632,

...

Quoted 4 lines
Quoted 2 lines

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

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

Oh, no, that is not what I meant.

You have

config B
depends on A

Quoted 9 lines

config A

above. We really want

config A

config B
depends on A

Got it I will rearrange that in v2.

Quoted 6 lines
Quoted 4 lines

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

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

I did not review it in great detail, so no acked-by's, yet :-).

Thanks
Dan

Quoted 2 lines

Pavel

author
#22 lore raw
In reply to Dan Murphy

Hello

Quoted 9 lines · Dan Murphy

All

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

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

Quoted 7 lines

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

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

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

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

Please let me know how to proceed.

Dan

Quoted 26 lines

Dan

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

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

Hi Dan,

Quoted 20 lines · Dan Murphy

Hello

Quoted 9 lines · Dan Murphy

All

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

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

Quoted 7 lines

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

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

Quoted 8 lines

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

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

Please let me know how to proceed.

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

Signature · 2 lines
Best regards,
Jacek Anaszewski