linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/2] leds: lp55xx: configure internal charge pump
@ 2023-02-13 10:20 Maarten Zanders
  2023-02-13 10:20 ` [PATCH v5 1/2] dt-bindings: leds-lp55xx: add ti,charge-pump-mode Maarten Zanders
  2023-02-13 10:20 ` [PATCH v5 2/2] leds: lp55xx: configure internal charge pump Maarten Zanders
  0 siblings, 2 replies; 7+ messages in thread
From: Maarten Zanders @ 2023-02-13 10:20 UTC (permalink / raw)
  To: Pavel Machek, Lee Jones, Rob Herring, Krzysztof Kozlowski,
	Jacek Anaszewski
  Cc: Maarten Zanders, linux-leds, devicetree, linux-kernel

A new option in the devicetree "ti,charge-pump-mode" allows the user to
configure the charge pump in a certain mode. The previous implementation
was "auto" mode, which remains the default.

v1 of the patch implemented a bool to disable the charge pump and had some
issues in the yaml binding.

v2 implemented all options of the charge pump as a string which was too
complex to parse & check.

v3 replaces the string by constants.

v4 resend with changelog (notes) in each patch

v5 dual license in dt header, change property type to u32

Maarten Zanders (2):
  dt-bindings: leds-lp55xx: add ti,charge-pump-mode
  leds: lp55xx: configure internal charge pump

 .../devicetree/bindings/leds/leds-lp55xx.yaml  |  8 ++++++++
 drivers/leds/leds-lp5521.c                     | 12 ++++++------
 drivers/leds/leds-lp5523.c                     | 18 +++++++++++++-----
 drivers/leds/leds-lp55xx-common.c              | 14 ++++++++++++++
 drivers/leds/leds-lp8501.c                     |  8 ++++++--
 include/dt-bindings/leds/leds-lp55xx.h         | 10 ++++++++++
 include/linux/platform_data/leds-lp55xx.h      |  3 +++
 7 files changed, 60 insertions(+), 13 deletions(-)
 create mode 100644 include/dt-bindings/leds/leds-lp55xx.h

-- 
2.37.3


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

* [PATCH v5 1/2] dt-bindings: leds-lp55xx: add ti,charge-pump-mode
  2023-02-13 10:20 [PATCH v5 0/2] leds: lp55xx: configure internal charge pump Maarten Zanders
@ 2023-02-13 10:20 ` Maarten Zanders
  2023-02-13 10:22   ` Krzysztof Kozlowski
  2023-02-13 10:20 ` [PATCH v5 2/2] leds: lp55xx: configure internal charge pump Maarten Zanders
  1 sibling, 1 reply; 7+ messages in thread
From: Maarten Zanders @ 2023-02-13 10:20 UTC (permalink / raw)
  To: Pavel Machek, Lee Jones, Rob Herring, Krzysztof Kozlowski,
	Jacek Anaszewski
  Cc: Maarten Zanders, linux-leds, devicetree, linux-kernel

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

Signed-off-by: Maarten Zanders <maarten.zanders@mind.be>
---

Notes:
    v1: implement as bool to disable charge pump
    v2: rewrite to use string configuration, supporting all modes
    v3: simplification by replacing string option by u8 constant,
        removing previous Reviewed-by tags as it's a complete
        rewrite of the patch.
    v4: added notes
    v5: dual license, change property type to u32

 .../devicetree/bindings/leds/leds-lp55xx.yaml          |  8 ++++++++
 include/dt-bindings/leds/leds-lp55xx.h                 | 10 ++++++++++
 2 files changed, 18 insertions(+)
 create mode 100644 include/dt-bindings/leds/leds-lp55xx.h

diff --git a/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml b/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml
index ae607911f1db..ede9cb9ca175 100644
--- a/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml
+++ b/Documentation/devicetree/bindings/leds/leds-lp55xx.yaml
@@ -66,6 +66,12 @@ properties:
   '#size-cells':
     const: 0
 
+  ti,charge-pump-mode:
+    description:
+      Set the operating mode of the internal charge pump as defined in
+      <dt-bindings/leds/leds-lp55xx.h>. Defaults to auto.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
 patternProperties:
   '^multi-led@[0-8]$':
     type: object
@@ -152,6 +158,7 @@ additionalProperties: false
 examples:
   - |
     #include <dt-bindings/leds/common.h>
+    #include <dt-bindings/leds/leds-lp55xx.h>
 
     i2c {
         #address-cells = <1>;
@@ -164,6 +171,7 @@ examples:
             reg = <0x32>;
             clock-mode = /bits/ 8 <2>;
             pwr-sel = /bits/ 8 <3>;	/* D1~9 connected to VOUT */
+            ti,charge-pump-mode = <LP55XX_CP_BYPASS>;
 
             led@0 {
                 reg = <0>;
diff --git a/include/dt-bindings/leds/leds-lp55xx.h b/include/dt-bindings/leds/leds-lp55xx.h
new file mode 100644
index 000000000000..e2256312714b
--- /dev/null
+++ b/include/dt-bindings/leds/leds-lp55xx.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
+#ifndef _DT_BINDINGS_LEDS_LP55XX_H
+#define _DT_BINDINGS_LEDS_LP55XX_H
+
+#define LP55XX_CP_OFF		0
+#define LP55XX_CP_BYPASS	1
+#define LP55XX_CP_BOOST		2
+#define LP55XX_CP_AUTO		3
+
+#endif /* _DT_BINDINGS_LEDS_LP55XX_H */
-- 
2.37.3


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

* [PATCH v5 2/2] leds: lp55xx: configure internal charge pump
  2023-02-13 10:20 [PATCH v5 0/2] leds: lp55xx: configure internal charge pump Maarten Zanders
  2023-02-13 10:20 ` [PATCH v5 1/2] dt-bindings: leds-lp55xx: add ti,charge-pump-mode Maarten Zanders
@ 2023-02-13 10:20 ` Maarten Zanders
  2023-03-03  9:58   ` Lee Jones
  1 sibling, 1 reply; 7+ messages in thread
From: Maarten Zanders @ 2023-02-13 10:20 UTC (permalink / raw)
  To: Pavel Machek, Lee Jones, Rob Herring, Krzysztof Kozlowski,
	Jacek Anaszewski
  Cc: Maarten Zanders, linux-leds, devicetree, linux-kernel

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.

Implement this option from the devicetree. When the setting
is not present it will operate in automatic mode as before.

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

Signed-off-by: Maarten Zanders <maarten.zanders@mind.be>
---

Notes:
    v1: implement as bool to disable charge pump
    v2: rewrite to use string configuration, supporting all modes
    v3: simplification by replacing string from DTS by constant
    v4: added notes
    v5: property type to u32

 drivers/leds/leds-lp5521.c                | 12 ++++++------
 drivers/leds/leds-lp5523.c                | 18 +++++++++++++-----
 drivers/leds/leds-lp55xx-common.c         | 14 ++++++++++++++
 drivers/leds/leds-lp8501.c                |  8 ++++++--
 include/linux/platform_data/leds-lp55xx.h |  3 +++
 5 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/drivers/leds/leds-lp5521.c b/drivers/leds/leds-lp5521.c
index 19478d9c19a7..76c6b81afb38 100644
--- a/drivers/leds/leds-lp5521.c
+++ b/drivers/leds/leds-lp5521.c
@@ -58,14 +58,11 @@
 /* CONFIG register */
 #define LP5521_PWM_HF			0x40	/* PWM: 0 = 256Hz, 1 = 558Hz */
 #define LP5521_PWRSAVE_EN		0x20	/* 1 = Power save mode */
-#define LP5521_CP_MODE_OFF		0	/* Charge pump (CP) off */
-#define LP5521_CP_MODE_BYPASS		8	/* CP forced to bypass mode */
-#define LP5521_CP_MODE_1X5		0x10	/* CP forced to 1.5x mode */
-#define LP5521_CP_MODE_AUTO		0x18	/* Automatic mode selection */
+#define LP5521_CP_MODE_MASK		0x18	/* Charge pump mode */
+#define LP5521_CP_MODE_SHIFT		3
 #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 +307,9 @@ static int lp5521_post_init_device(struct lp55xx_chip *chip)
 	if (!lp55xx_is_extclk_used(chip))
 		val |= LP5521_CLK_INT;
 
+	val |= (chip->pdata->charge_pump_mode << LP5521_CP_MODE_SHIFT) &
+		LP5521_CP_MODE_MASK;
+
 	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 e08e3de1428d..b5d10d4252e6 100644
--- a/drivers/leds/leds-lp5523.c
+++ b/drivers/leds/leds-lp5523.c
@@ -57,8 +57,12 @@
 #define LP5523_AUTO_INC			0x40
 #define LP5523_PWR_SAVE			0x20
 #define LP5523_PWM_PWR_SAVE		0x04
-#define LP5523_CP_AUTO			0x18
+#define LP5523_CP_MODE_MASK		0x18
+#define LP5523_CP_MODE_SHIFT		3
 #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 +129,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 +138,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;
+
+	val |= (chip->pdata->charge_pump_mode << LP5523_CP_MODE_SHIFT) &
+	       LP5523_CP_MODE_MASK;
+
+	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 c1940964067a..5a02c4a4ec98 100644
--- a/drivers/leds/leds-lp55xx-common.c
+++ b/drivers/leds/leds-lp55xx-common.c
@@ -19,6 +19,8 @@
 #include <linux/slab.h>
 #include <linux/gpio/consumer.h>
 
+#include <dt-bindings/leds/leds-lp55xx.h>
+
 #include "leds-lp55xx-common.h"
 
 /* External clock rate */
@@ -691,6 +693,18 @@ struct lp55xx_platform_data *lp55xx_of_populate_pdata(struct device *dev,
 		i++;
 	}
 
+	ret = of_property_read_u32(np, "ti,charge-pump-mode",
+				   &pdata->charge_pump_mode);
+	if (ret) {
+		pdata->charge_pump_mode = LP55XX_CP_AUTO;
+	} else {
+		if (pdata->charge_pump_mode > LP55XX_CP_AUTO) {
+			dev_err(dev, "invalid charge pump mode %d\n",
+				pdata->charge_pump_mode);
+			return ERR_PTR(-EINVAL);
+		}
+	}
+
 	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..f0e70e116919 100644
--- a/drivers/leds/leds-lp8501.c
+++ b/drivers/leds/leds-lp8501.c
@@ -53,10 +53,11 @@
 #define LP8501_PWM_PSAVE		BIT(7)
 #define LP8501_AUTO_INC			BIT(6)
 #define LP8501_PWR_SAVE			BIT(5)
-#define LP8501_CP_AUTO			0x18
+#define LP8501_CP_MODE_MASK		0x18
+#define LP8501_CP_MODE_SHIFT		3
 #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 +103,9 @@ static int lp8501_post_init_device(struct lp55xx_chip *chip)
 	if (chip->pdata->clock_mode != LP55XX_CLOCK_EXT)
 		val |= LP8501_INT_CLK;
 
+	val |= (chip->pdata->charge_pump_mode << LP8501_CP_MODE_SHIFT) &
+	       LP8501_CP_MODE_MASK;
+
 	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..3cc8db0b12b5 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 mode */
+	u32 charge_pump_mode;
+
 	/* optional enable GPIO */
 	struct gpio_desc *enable_gpiod;
 
-- 
2.37.3


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

* Re: [PATCH v5 1/2] dt-bindings: leds-lp55xx: add ti,charge-pump-mode
  2023-02-13 10:20 ` [PATCH v5 1/2] dt-bindings: leds-lp55xx: add ti,charge-pump-mode Maarten Zanders
@ 2023-02-13 10:22   ` Krzysztof Kozlowski
  2023-02-13 10:32     ` Maarten Zanders
  0 siblings, 1 reply; 7+ messages in thread
From: Krzysztof Kozlowski @ 2023-02-13 10:22 UTC (permalink / raw)
  To: Maarten Zanders, Pavel Machek, Lee Jones, Rob Herring,
	Krzysztof Kozlowski, Jacek Anaszewski
  Cc: linux-leds, devicetree, linux-kernel

On 13/02/2023 11:20, Maarten Zanders wrote:
> Add a binding to configure the internal charge pump for lp55xx.
> 
> Signed-off-by: Maarten Zanders <maarten.zanders@mind.be>
> ---
> 
> Notes:
>     v1: implement as bool to disable charge pump
>     v2: rewrite to use string configuration, supporting all modes
>     v3: simplification by replacing string option by u8 constant,
>         removing previous Reviewed-by tags as it's a complete
>         rewrite of the patch.
>     v4: added notes
>     v5: dual license, change property type to u32

Thank you for your patch. There is something to discuss/improve.


> diff --git a/include/dt-bindings/leds/leds-lp55xx.h b/include/dt-bindings/leds/leds-lp55xx.h
> new file mode 100644
> index 000000000000..e2256312714b
> --- /dev/null
> +++ b/include/dt-bindings/leds/leds-lp55xx.h
> @@ -0,0 +1,10 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */

While technically it is almost equal to BSD-2, any particular reason for
using different second license than the binding and generic
recommendation (checkpatch)?

Best regards,
Krzysztof


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

* Re: [PATCH v5 1/2] dt-bindings: leds-lp55xx: add ti,charge-pump-mode
  2023-02-13 10:22   ` Krzysztof Kozlowski
@ 2023-02-13 10:32     ` Maarten Zanders
  2023-02-13 10:46       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 7+ messages in thread
From: Maarten Zanders @ 2023-02-13 10:32 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Pavel Machek, Lee Jones, Rob Herring,
	Krzysztof Kozlowski, Jacek Anaszewski
  Cc: linux-leds, devicetree, linux-kernel


On 2/13/23 11:22, Krzysztof Kozlowski wrote:
>
> While technically it is almost equal to BSD-2, any particular reason for
> using different second license than the binding and generic
> recommendation (checkpatch)?
>
>
No, no specific reason. I'm happy to adapt.

Br,
Maarten



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

* Re: [PATCH v5 1/2] dt-bindings: leds-lp55xx: add ti,charge-pump-mode
  2023-02-13 10:32     ` Maarten Zanders
@ 2023-02-13 10:46       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 7+ messages in thread
From: Krzysztof Kozlowski @ 2023-02-13 10:46 UTC (permalink / raw)
  To: Maarten Zanders, Pavel Machek, Lee Jones, Rob Herring,
	Krzysztof Kozlowski, Jacek Anaszewski
  Cc: linux-leds, devicetree, linux-kernel

On 13/02/2023 11:32, Maarten Zanders wrote:
> 
> On 2/13/23 11:22, Krzysztof Kozlowski wrote:
>>
>> While technically it is almost equal to BSD-2, any particular reason for
>> using different second license than the binding and generic
>> recommendation (checkpatch)?
>>
>>
> No, no specific reason. I'm happy to adapt.

Then please switch to recommended ones. With that:


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof


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

* Re: [PATCH v5 2/2] leds: lp55xx: configure internal charge pump
  2023-02-13 10:20 ` [PATCH v5 2/2] leds: lp55xx: configure internal charge pump Maarten Zanders
@ 2023-03-03  9:58   ` Lee Jones
  0 siblings, 0 replies; 7+ messages in thread
From: Lee Jones @ 2023-03-03  9:58 UTC (permalink / raw)
  To: Maarten Zanders
  Cc: Pavel Machek, Rob Herring, Krzysztof Kozlowski, Jacek Anaszewski,
	linux-leds, devicetree, linux-kernel

On Mon, 13 Feb 2023, Maarten Zanders wrote:

> 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.
> 
> Implement this option from the devicetree. When the setting
> is not present it will operate in automatic mode as before.
> 
> Tested on LP55231. Datasheet analysis shows that LP5521, LP5523
> and LP8501 are identical in topology and are modified in the
> same way.
> 
> Signed-off-by: Maarten Zanders <maarten.zanders@mind.be>
> ---
> 
> Notes:
>     v1: implement as bool to disable charge pump
>     v2: rewrite to use string configuration, supporting all modes
>     v3: simplification by replacing string from DTS by constant
>     v4: added notes
>     v5: property type to u32
> 
>  drivers/leds/leds-lp5521.c                | 12 ++++++------
>  drivers/leds/leds-lp5523.c                | 18 +++++++++++++-----
>  drivers/leds/leds-lp55xx-common.c         | 14 ++++++++++++++
>  drivers/leds/leds-lp8501.c                |  8 ++++++--
>  include/linux/platform_data/leds-lp55xx.h |  3 +++
>  5 files changed, 42 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/leds/leds-lp5521.c b/drivers/leds/leds-lp5521.c
> index 19478d9c19a7..76c6b81afb38 100644
> --- a/drivers/leds/leds-lp5521.c
> +++ b/drivers/leds/leds-lp5521.c
> @@ -58,14 +58,11 @@
>  /* CONFIG register */
>  #define LP5521_PWM_HF			0x40	/* PWM: 0 = 256Hz, 1 = 558Hz */
>  #define LP5521_PWRSAVE_EN		0x20	/* 1 = Power save mode */
> -#define LP5521_CP_MODE_OFF		0	/* Charge pump (CP) off */
> -#define LP5521_CP_MODE_BYPASS		8	/* CP forced to bypass mode */
> -#define LP5521_CP_MODE_1X5		0x10	/* CP forced to 1.5x mode */
> -#define LP5521_CP_MODE_AUTO		0x18	/* Automatic mode selection */
> +#define LP5521_CP_MODE_MASK		0x18	/* Charge pump mode */
> +#define LP5521_CP_MODE_SHIFT		3
>  #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 +307,9 @@ static int lp5521_post_init_device(struct lp55xx_chip *chip)
>  	if (!lp55xx_is_extclk_used(chip))
>  		val |= LP5521_CLK_INT;
>  
> +	val |= (chip->pdata->charge_pump_mode << LP5521_CP_MODE_SHIFT) &
> +		LP5521_CP_MODE_MASK;
> +
>  	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 e08e3de1428d..b5d10d4252e6 100644
> --- a/drivers/leds/leds-lp5523.c
> +++ b/drivers/leds/leds-lp5523.c
> @@ -57,8 +57,12 @@
>  #define LP5523_AUTO_INC			0x40
>  #define LP5523_PWR_SAVE			0x20
>  #define LP5523_PWM_PWR_SAVE		0x04
> -#define LP5523_CP_AUTO			0x18
> +#define LP5523_CP_MODE_MASK		0x18
> +#define LP5523_CP_MODE_SHIFT		3
>  #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 +129,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 +138,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;
> +
> +	val |= (chip->pdata->charge_pump_mode << LP5523_CP_MODE_SHIFT) &
> +	       LP5523_CP_MODE_MASK;
> +
> +	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 c1940964067a..5a02c4a4ec98 100644
> --- a/drivers/leds/leds-lp55xx-common.c
> +++ b/drivers/leds/leds-lp55xx-common.c
> @@ -19,6 +19,8 @@
>  #include <linux/slab.h>
>  #include <linux/gpio/consumer.h>
>  
> +#include <dt-bindings/leds/leds-lp55xx.h>
> +
>  #include "leds-lp55xx-common.h"
>  
>  /* External clock rate */
> @@ -691,6 +693,18 @@ struct lp55xx_platform_data *lp55xx_of_populate_pdata(struct device *dev,
>  		i++;
>  	}
>  
> +	ret = of_property_read_u32(np, "ti,charge-pump-mode",
> +				   &pdata->charge_pump_mode);
> +	if (ret) {
> +		pdata->charge_pump_mode = LP55XX_CP_AUTO;
> +	} else {

You can clean this up by dropping the else.

> +		if (pdata->charge_pump_mode > LP55XX_CP_AUTO) {
> +			dev_err(dev, "invalid charge pump mode %d\n",
> +				pdata->charge_pump_mode);

Then this can be single line.

> +			return ERR_PTR(-EINVAL);
> +		}
> +	}
> +
>  	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..f0e70e116919 100644
> --- a/drivers/leds/leds-lp8501.c
> +++ b/drivers/leds/leds-lp8501.c
> @@ -53,10 +53,11 @@
>  #define LP8501_PWM_PSAVE		BIT(7)
>  #define LP8501_AUTO_INC			BIT(6)
>  #define LP8501_PWR_SAVE			BIT(5)
> -#define LP8501_CP_AUTO			0x18
> +#define LP8501_CP_MODE_MASK		0x18
> +#define LP8501_CP_MODE_SHIFT		3
>  #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 +103,9 @@ static int lp8501_post_init_device(struct lp55xx_chip *chip)
>  	if (chip->pdata->clock_mode != LP55XX_CLOCK_EXT)
>  		val |= LP8501_INT_CLK;
>  
> +	val |= (chip->pdata->charge_pump_mode << LP8501_CP_MODE_SHIFT) &
> +	       LP8501_CP_MODE_MASK;
> +
>  	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..3cc8db0b12b5 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 mode */
> +	u32 charge_pump_mode;
> +
>  	/* optional enable GPIO */
>  	struct gpio_desc *enable_gpiod;
>  
> -- 
> 2.37.3
> 

-- 
Lee Jones [李琼斯]

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

end of thread, other threads:[~2023-03-03  9:59 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-13 10:20 [PATCH v5 0/2] leds: lp55xx: configure internal charge pump Maarten Zanders
2023-02-13 10:20 ` [PATCH v5 1/2] dt-bindings: leds-lp55xx: add ti,charge-pump-mode Maarten Zanders
2023-02-13 10:22   ` Krzysztof Kozlowski
2023-02-13 10:32     ` Maarten Zanders
2023-02-13 10:46       ` Krzysztof Kozlowski
2023-02-13 10:20 ` [PATCH v5 2/2] leds: lp55xx: configure internal charge pump Maarten Zanders
2023-03-03  9:58   ` Lee Jones

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).