[PATCH 0/2] leds: lp55xx: support disabling the charge pump

[PATCH 0/2] leds: lp55xx: support disabling the charge pump

[Maarten Zanders]

A new option in the devicetree "ti,disable-charge-pump" disables the internal
charge pump. This can be used to avoid current flowing into the Vout pin when
an external supply has been connected to drive the LEDs.

Maarten Zanders (2):
  dt-bindings: leds-lp55xx: add ti,disable-charge-pump
  leds: lp55xx: add option to disable charge pump

 .../devicetree/bindings/leds/leds-lp55xx.yaml     |  5 +++++
 drivers/leds/leds-lp5521.c                        |  6 ++++--
 drivers/leds/leds-lp5523.c                        | 15 +++++++++++----
 drivers/leds/leds-lp55xx-common.c                 |  3 +++
 drivers/leds/leds-lp8501.c                        |  5 ++++-
 include/linux/platform_data/leds-lp55xx.h         |  3 +++
 6 files changed, 30 insertions(+), 7 deletions(-)


base-commit: 5f52a8ba7e91215c3d046d298fb328d1b9f7897d
-- 
2.37.3

[PATCH 1/2] dt-bindings: leds-lp55xx: add ti,disable-charge-pump

[Maarten Zanders]

Add a binding to disable the internal charge pump for lp55xx.

Signed-off-by: Maarten Zanders <maarten.zanders@mind.be>
---
 Documentation/devicetree/bindings/leds/leds-lp55xx.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml b/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml
index 7ec676e53851..9eb8d1017402 100644
--- a/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml
+++ b/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml
@@ -64,6 +64,11 @@ properties:
   '#size-cells':
     const: 0
 
+  ti,disable-charge-pump:
+    description: |
+      Disable the internal charge pump.
+    type: boolean
+
 patternProperties:
   "(^led@[0-9a-f]$|led)":
     type: object
-- 
2.37.3

[PATCH 2/2] leds: lp55xx: add option to disable charge pump

[Maarten Zanders]

The LP55xx range of devices have an internal charge pump which
can (automatically) increase the output voltage towards the
LED's, boosting the output voltage to 4.5V.

While undocumented, it is possible to connect an external power
rail to the Vout pin on the device to save power. However, when
doing this, the internal charge pump needs to be disabled. If
it is enabled in this configuration, there is leakage from the
externally supplied Vout back to Vin (Vdd).

This commit adds an option in the devicetree to disable the
charge pump. When the option is not present, the device config
is exactly as before (automatic mode).

Tested on LP55231. Datasheet analysis shows that LP5521, LP5523
and LP8501 are identical in topology and have been modified in
the same way.

Signed-off-by: Maarten Zanders <maarten.zanders@mind.be>
---
 drivers/leds/leds-lp5521.c                |  6 ++++--
 drivers/leds/leds-lp5523.c                | 15 +++++++++++----
 drivers/leds/leds-lp55xx-common.c         |  3 +++
 drivers/leds/leds-lp8501.c                |  5 ++++-
 include/linux/platform_data/leds-lp55xx.h |  3 +++
 5 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/drivers/leds/leds-lp5521.c b/drivers/leds/leds-lp5521.c
index 7ff20c260504..80f3b50c9502 100644
--- a/drivers/leds/leds-lp5521.c
+++ b/drivers/leds/leds-lp5521.c
@@ -64,8 +64,7 @@
 #define LP5521_CP_MODE_AUTO		0x18	/* Automatic mode selection */
 #define LP5521_R_TO_BATT		0x04	/* R out: 0 = CP, 1 = Vbat */
 #define LP5521_CLK_INT			0x01	/* Internal clock */
-#define LP5521_DEFAULT_CFG		\
-	(LP5521_PWM_HF | LP5521_PWRSAVE_EN | LP5521_CP_MODE_AUTO)
+#define LP5521_DEFAULT_CFG		(LP5521_PWM_HF | LP5521_PWRSAVE_EN)
 
 /* Status */
 #define LP5521_EXT_CLK_USED		0x08
@@ -310,6 +309,9 @@ static int lp5521_post_init_device(struct lp55xx_chip *chip)
 	if (!lp55xx_is_extclk_used(chip))
 		val |= LP5521_CLK_INT;
 
+	if (!chip->pdata->cp_disable)
+		val |= LP5521_CP_MODE_AUTO;
+
 	ret = lp55xx_write(chip, LP5521_REG_CONFIG, val);
 	if (ret)
 		return ret;
diff --git a/drivers/leds/leds-lp5523.c b/drivers/leds/leds-lp5523.c
index 369d40b0b65b..06d81a520168 100644
--- a/drivers/leds/leds-lp5523.c
+++ b/drivers/leds/leds-lp5523.c
@@ -59,6 +59,9 @@
 #define LP5523_PWM_PWR_SAVE		0x04
 #define LP5523_CP_AUTO			0x18
 #define LP5523_AUTO_CLK			0x02
+#define LP5523_DEFAULT_CONFIG	\
+	(LP5523_AUTO_INC | LP5523_PWR_SAVE |\
+	 LP5523_AUTO_CLK | LP5523_PWM_PWR_SAVE)
 
 #define LP5523_EN_LEDTEST		0x80
 #define LP5523_LEDTEST_DONE		0x80
@@ -125,6 +128,7 @@ static void lp5523_set_led_current(struct lp55xx_led *led, u8 led_current)
 static int lp5523_post_init_device(struct lp55xx_chip *chip)
 {
 	int ret;
+	int val;
 
 	ret = lp55xx_write(chip, LP5523_REG_ENABLE, LP5523_ENABLE);
 	if (ret)
@@ -133,10 +137,13 @@ static int lp5523_post_init_device(struct lp55xx_chip *chip)
 	/* Chip startup time is 500 us, 1 - 2 ms gives some margin */
 	usleep_range(1000, 2000);
 
-	ret = lp55xx_write(chip, LP5523_REG_CONFIG,
-			    LP5523_AUTO_INC | LP5523_PWR_SAVE |
-			    LP5523_CP_AUTO | LP5523_AUTO_CLK |
-			    LP5523_PWM_PWR_SAVE);
+	val = LP5523_DEFAULT_CONFIG;
+
+	if (!chip->pdata->cp_disable)
+		val |= LP5523_CP_AUTO;
+
+	ret = lp55xx_write(chip, LP5523_REG_CONFIG, val);
+
 	if (ret)
 		return ret;
 
diff --git a/drivers/leds/leds-lp55xx-common.c b/drivers/leds/leds-lp55xx-common.c
index ca2e28fb843f..aadaca26fb23 100644
--- a/drivers/leds/leds-lp55xx-common.c
+++ b/drivers/leds/leds-lp55xx-common.c
@@ -691,6 +691,9 @@ struct lp55xx_platform_data *lp55xx_of_populate_pdata(struct device *dev,
 		i++;
 	}
 
+	if (of_find_property(np, "ti,disable-charge-pump", NULL))
+		pdata->cp_disable = 1;
+
 	of_property_read_string(np, "label", &pdata->label);
 	of_property_read_u8(np, "clock-mode", &pdata->clock_mode);
 
diff --git a/drivers/leds/leds-lp8501.c b/drivers/leds/leds-lp8501.c
index ae11a02c0ab2..6370abb2c7ae 100644
--- a/drivers/leds/leds-lp8501.c
+++ b/drivers/leds/leds-lp8501.c
@@ -56,7 +56,7 @@
 #define LP8501_CP_AUTO			0x18
 #define LP8501_INT_CLK			BIT(0)
 #define LP8501_DEFAULT_CFG	\
-	(LP8501_PWM_PSAVE | LP8501_AUTO_INC | LP8501_PWR_SAVE | LP8501_CP_AUTO)
+	(LP8501_PWM_PSAVE | LP8501_AUTO_INC | LP8501_PWR_SAVE)
 
 #define LP8501_REG_RESET		0x3D
 #define LP8501_RESET			0xFF
@@ -102,6 +102,9 @@ static int lp8501_post_init_device(struct lp55xx_chip *chip)
 	if (chip->pdata->clock_mode != LP55XX_CLOCK_EXT)
 		val |= LP8501_INT_CLK;
 
+	if (!chip->pdata->cp_disable)
+		val |= LP8501_CP_AUTO;
+
 	ret = lp55xx_write(chip, LP8501_REG_CONFIG, val);
 	if (ret)
 		return ret;
diff --git a/include/linux/platform_data/leds-lp55xx.h b/include/linux/platform_data/leds-lp55xx.h
index 3441064713a3..8e2ba5e2a7ee 100644
--- a/include/linux/platform_data/leds-lp55xx.h
+++ b/include/linux/platform_data/leds-lp55xx.h
@@ -73,6 +73,9 @@ struct lp55xx_platform_data {
 	/* Clock configuration */
 	u8 clock_mode;
 
+	/* charge pump disable */
+	u8 cp_disable;
+
 	/* optional enable GPIO */
 	struct gpio_desc *enable_gpiod;
 
-- 
2.37.3

Re: [PATCH 1/2] dt-bindings: leds-lp55xx: add ti,disable-charge-pump

[Krzysztof Kozlowski]

On 28/10/2022 09:34, Maarten Zanders wrote:
> Add a binding to disable the internal charge pump for lp55xx.
> 
> Signed-off-by: Maarten Zanders <maarten.zanders@mind.be>
> ---
>  Documentation/devicetree/bindings/leds/leds-lp55xx.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml b/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml
> index 7ec676e53851..9eb8d1017402 100644
> --- a/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml
> +++ b/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml
> @@ -64,6 +64,11 @@ properties:
>    '#size-cells':
>      const: 0
>  
> +  ti,disable-charge-pump:
> +    description: |

Drop |

> +      Disable the internal charge pump.

Does it mean that it is enabled by default?

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: leds-lp55xx: add ti,disable-charge-pump

[Maarten Zanders]

On 10/29/22 00:07, Krzysztof Kozlowski wrote:
> Drop | 
OK.
>> +      Disable the internal charge pump.
> Does it mean that it is enabled by default?

The device default after reset is "off".
The current implementation sets it to "auto".
Other possible modes are "bypass" and "boost".

Should I change to an optional "ti,charge_pump_mode" as string and set 
default to "auto"?

Thanks!

Re: [PATCH 1/2] dt-bindings: leds-lp55xx: add ti,disable-charge-pump

[Krzysztof Kozlowski]

On 29/10/2022 03:48, Maarten Zanders wrote:
> 
> On 10/29/22 00:07, Krzysztof Kozlowski wrote:
>> Drop | 
> OK.
>>> +      Disable the internal charge pump.
>> Does it mean that it is enabled by default?
> 
> The device default after reset is "off".
> The current implementation sets it to "auto".
> Other possible modes are "bypass" and "boost".
> 
> Should I change to an optional "ti,charge_pump_mode" as string and set 
> default to "auto"?

enum makes more sense in that case. Just don't user underscores in
property names.

Best regards,
Krzysztof