On Mon 09 Jul 03:22 PDT 2018, Kiran Gunda wrote:

diff --git a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt

[..]

• qcom,num-strings Usage: optional Value type: <u32> Definition: #; number of led strings attached;

•       value from 1 to 3. default: 2
  

•       This property is supported only for PM8941.
  

•       value: For PM8941 from 1 to 3.
  

• 	     For PMI8998 from 1 to 4.
  

[..]

+- qcom,enabled-strings

• Usage: optional
• Value tyoe: <u32 array>
• Definition: Array of the WLED strings numbered from 0 to 3. Each

•       string of leds are operated individually. Specify the
  

•       list of strings used by the device. Any combination of
  

•       led strings can be used.
  

[..]

Example:

@@ -99,4 +146,5 @@ pm8941-wled@d800 { qcom,switching-freq = <1600>; qcom,ovp = <29>; qcom,num-strings = <2>;

• qcom,enabled-strings = <0x00 0x01>;

Nit. I would assume that specifying qcom,num-strings = <2> implies that the first 2 strings are used, so one would not also specify qcom,enabled-strings.

(And you should use decimal for natural numbers)

Reviewed-by: Bjorn Andersson bjorn.andersson@linaro.org

Regards, Bjorn

On Wed 15 Aug 22:23 PDT 2018, kgunda@codeaurora.org wrote:

On 2018-08-07 10:53, Bjorn Andersson wrote:

...

On Mon 09 Jul 03:22 PDT 2018, Kiran Gunda wrote:

...

diff --git a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt

[..]

...

• qcom,num-strings Usage: optional Value type: <u32> Definition: #; number of led strings attached;

•       value from 1 to 3. default: 2
  

•       This property is supported only for PM8941.
  

•       value: For PM8941 from 1 to 3.
  

• 	     For PMI8998 from 1 to 4.
  

[..]

...

+- qcom,enabled-strings

• Usage: optional
• Value tyoe: <u32 array>
• Definition: Array of the WLED strings numbered from 0 to 3. Each

•       string of leds are operated individually. Specify the
  

•       list of strings used by the device. Any combination of
  

•       led strings can be used.
  

[..]

...

Example:

@@ -99,4 +146,5 @@ pm8941-wled@d800 { qcom,switching-freq = <1600>; qcom,ovp = <29>; qcom,num-strings = <2>;

• qcom,enabled-strings = <0x00 0x01>;

Nit. I would assume that specifying qcom,num-strings = <2> implies that the first 2 strings are used, so one would not also specify qcom,enabled-strings.

Thanks Bjorn for reviewing the series !

"qcom,enabled-strings" need be specified along with the "qcom,num-strings". Because the enabled-strings can be <0, 2> or <0, 3 > also. The driver picks the string configuration from the enabled-strings array and enable only those current-sinks.

The original binding described qcom,num-strings to mean "the first N strings", requiring qcom,enabled-strings now would break backwards compatibility with this binding.

In the case that qcom,enabled-strings is specified we can easily derive num-strings from the listed entires. So I would suggest that you look for enabled-strings and if not found fall back to checking for num-strings.

Regards, Bjorn

On 2018-07-20 18:53, Daniel Thompson wrote:

On 09/07/18 11:22, Kiran Gunda wrote:

...

Rename the PM8941* references as WLED3 to make the driver generic and have WLED support for other PMICs.

Signed-off-by: Kiran Gunda kgunda@codeaurora.org

Changes from V3:

• Changed the MODULE_DESCRIPTION

drivers/video/backlight/qcom-wled.c | 248 ++++++++++++++++++------------------ 1 file changed, 125 insertions(+), 123 deletions(-)

diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c index 0b6d219..3cd6e75 100644 --- a/drivers/video/backlight/qcom-wled.c +++ b/drivers/video/backlight/qcom-wled.c @@ -18,77 +18,79 @@ #include <linux/regmap.h> /* From DT binding */ -#define PM8941_WLED_DEFAULT_BRIGHTNESS 2048 +#define WLED_DEFAULT_BRIGHTNESS 2048 -#define PM8941_WLED_REG_VAL_BASE 0x40 -#define PM8941_WLED_REG_VAL_MAX 0xFFF +#define WLED3_SINK_REG_BRIGHT_MAX 0xFFF +#define WLED3_CTRL_REG_VAL_BASE 0x40 -#define PM8941_WLED_REG_MOD_EN 0x46 -#define PM8941_WLED_REG_MOD_EN_BIT BIT(7) -#define PM8941_WLED_REG_MOD_EN_MASK BIT(7) +/* WLED3 control registers */ +#define WLED3_CTRL_REG_MOD_EN 0x46 +#define WLED3_CTRL_REG_MOD_EN_BIT BIT(7) +#define WLED3_CTRL_REG_MOD_EN_MASK BIT(7) -#define PM8941_WLED_REG_SYNC 0x47 -#define PM8941_WLED_REG_SYNC_MASK 0x07 -#define PM8941_WLED_REG_SYNC_LED1 BIT(0) -#define PM8941_WLED_REG_SYNC_LED2 BIT(1) -#define PM8941_WLED_REG_SYNC_LED3 BIT(2) -#define PM8941_WLED_REG_SYNC_ALL 0x07 -#define PM8941_WLED_REG_SYNC_CLEAR 0x00 +#define WLED3_CTRL_REG_FREQ 0x4c +#define WLED3_CTRL_REG_FREQ_MASK 0x0f -#define PM8941_WLED_REG_FREQ 0x4c -#define PM8941_WLED_REG_FREQ_MASK 0x0f +#define WLED3_CTRL_REG_OVP 0x4d +#define WLED3_CTRL_REG_OVP_MASK 0x03 -#define PM8941_WLED_REG_OVP 0x4d -#define PM8941_WLED_REG_OVP_MASK 0x03 +#define WLED3_CTRL_REG_ILIMIT 0x4e +#define WLED3_CTRL_REG_ILIMIT_MASK 0x07 -#define PM8941_WLED_REG_BOOST 0x4e -#define PM8941_WLED_REG_BOOST_MASK 0x07 +/* WLED3 sink registers */ +#define WLED3_SINK_REG_SYNC 0x47 +#define WLED3_SINK_REG_SYNC_MASK 0x07 +#define WLED3_SINK_REG_SYNC_LED1 BIT(0) +#define WLED3_SINK_REG_SYNC_LED2 BIT(1) +#define WLED3_SINK_REG_SYNC_LED3 BIT(2) +#define WLED3_SINK_REG_SYNC_ALL 0x07 +#define WLED3_SINK_REG_SYNC_CLEAR 0x00 -#define PM8941_WLED_REG_SINK 0x4f -#define PM8941_WLED_REG_SINK_MASK 0xe0 -#define PM8941_WLED_REG_SINK_SHFT 0x05 +#define WLED3_SINK_REG_CURR_SINK 0x4f +#define WLED3_SINK_REG_CURR_SINK_MASK 0xe0 +#define WLED3_SINK_REG_CURR_SINK_SHFT 0x05 -/* Per-'string' registers below */ -#define PM8941_WLED_REG_STR_OFFSET 0x10 +/* WLED3 per-'string' registers below */ +#define WLED3_SINK_REG_STR_OFFSET 0x10 -#define PM8941_WLED_REG_STR_MOD_EN_BASE 0x60 -#define PM8941_WLED_REG_STR_MOD_MASK BIT(7) -#define PM8941_WLED_REG_STR_MOD_EN BIT(7) +#define WLED3_SINK_REG_STR_MOD_EN_BASE 0x60 +#define WLED3_SINK_REG_STR_MOD_MASK BIT(7) +#define WLED3_SINK_REG_STR_MOD_EN BIT(7) -#define PM8941_WLED_REG_STR_SCALE_BASE 0x62 -#define PM8941_WLED_REG_STR_SCALE_MASK 0x1f +#define WLED3_SINK_REG_STR_FULL_SCALE_CURR 0x62 +#define WLED3_SINK_REG_STR_FULL_SCALE_CURR_MASK 0x1f -#define PM8941_WLED_REG_STR_MOD_SRC_BASE 0x63 -#define PM8941_WLED_REG_STR_MOD_SRC_MASK 0x01 -#define PM8941_WLED_REG_STR_MOD_SRC_INT 0x00 -#define PM8941_WLED_REG_STR_MOD_SRC_EXT 0x01 +#define WLED3_SINK_REG_STR_MOD_SRC_BASE 0x63 +#define WLED3_SINK_REG_STR_MOD_SRC_MASK 0x01 +#define WLED3_SINK_REG_STR_MOD_SRC_INT 0x00 +#define WLED3_SINK_REG_STR_MOD_SRC_EXT 0x01 -#define PM8941_WLED_REG_STR_CABC_BASE 0x66 -#define PM8941_WLED_REG_STR_CABC_MASK BIT(7) -#define PM8941_WLED_REG_STR_CABC_EN BIT(7) +#define WLED3_SINK_REG_STR_CABC_BASE 0x66 +#define WLED3_SINK_REG_STR_CABC_MASK BIT(7) +#define WLED3_SINK_REG_STR_CABC_EN BIT(7) -struct pm8941_wled_config {

• u32 i_boost_limit;

+struct wled_config {

• u32 boost_i_limit; u32 ovp; u32 switch_freq; u32 num_strings;

• u32 i_limit;

• u32 string_i_limit;

This patch implements a grand renaming and I have no objections to including change for boost_i_limit and string_i_limit in it. I'd prefer it to be mentioned in the patch header though.

Once that is done:

Reviewed-by: Daniel Thompson daniel.thompson@linaro.org

Thanks Daneil. I will include it in the patch header in next series.

On 2018-08-07 11:11, Bjorn Andersson wrote:

On Mon 09 Jul 03:22 PDT 2018, Kiran Gunda wrote:

...

Rename the PM8941* references as WLED3 to make the driver generic and have WLED support for other PMICs.

Signed-off-by: Kiran Gunda kgunda@codeaurora.org

I agree with Daniel, regarding the mentioning of the variable rename.

Yes. As Daniel asked I will update the commit text.

Apart from that:

Reviewed-by: Bjorn Andersson bjorn.andersson@linaro.org

Regards, Bjorn

...

---

Changes from V3:

• Changed the MODULE_DESCRIPTION

drivers/video/backlight/qcom-wled.c | 248 ++++++++++++++++++------------------ 1 file changed, 125 insertions(+), 123 deletions(-)

diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c index 0b6d219..3cd6e75 100644 --- a/drivers/video/backlight/qcom-wled.c +++ b/drivers/video/backlight/qcom-wled.c @@ -18,77 +18,79 @@ #include <linux/regmap.h>

/* From DT binding */ -#define PM8941_WLED_DEFAULT_BRIGHTNESS 2048 +#define WLED_DEFAULT_BRIGHTNESS 2048

-#define PM8941_WLED_REG_VAL_BASE 0x40 -#define PM8941_WLED_REG_VAL_MAX 0xFFF +#define WLED3_SINK_REG_BRIGHT_MAX 0xFFF +#define WLED3_CTRL_REG_VAL_BASE 0x40

-#define PM8941_WLED_REG_MOD_EN 0x46 -#define PM8941_WLED_REG_MOD_EN_BIT BIT(7) -#define PM8941_WLED_REG_MOD_EN_MASK BIT(7) +/* WLED3 control registers */ +#define WLED3_CTRL_REG_MOD_EN 0x46 +#define WLED3_CTRL_REG_MOD_EN_BIT BIT(7) +#define WLED3_CTRL_REG_MOD_EN_MASK BIT(7)

-#define PM8941_WLED_REG_SYNC 0x47 -#define PM8941_WLED_REG_SYNC_MASK 0x07 -#define PM8941_WLED_REG_SYNC_LED1 BIT(0) -#define PM8941_WLED_REG_SYNC_LED2 BIT(1) -#define PM8941_WLED_REG_SYNC_LED3 BIT(2) -#define PM8941_WLED_REG_SYNC_ALL 0x07 -#define PM8941_WLED_REG_SYNC_CLEAR 0x00 +#define WLED3_CTRL_REG_FREQ 0x4c +#define WLED3_CTRL_REG_FREQ_MASK 0x0f

-#define PM8941_WLED_REG_FREQ 0x4c -#define PM8941_WLED_REG_FREQ_MASK 0x0f +#define WLED3_CTRL_REG_OVP 0x4d +#define WLED3_CTRL_REG_OVP_MASK 0x03

-#define PM8941_WLED_REG_OVP 0x4d -#define PM8941_WLED_REG_OVP_MASK 0x03 +#define WLED3_CTRL_REG_ILIMIT 0x4e +#define WLED3_CTRL_REG_ILIMIT_MASK 0x07

-#define PM8941_WLED_REG_BOOST 0x4e -#define PM8941_WLED_REG_BOOST_MASK 0x07 +/* WLED3 sink registers */ +#define WLED3_SINK_REG_SYNC 0x47 +#define WLED3_SINK_REG_SYNC_MASK 0x07 +#define WLED3_SINK_REG_SYNC_LED1 BIT(0) +#define WLED3_SINK_REG_SYNC_LED2 BIT(1) +#define WLED3_SINK_REG_SYNC_LED3 BIT(2) +#define WLED3_SINK_REG_SYNC_ALL 0x07 +#define WLED3_SINK_REG_SYNC_CLEAR 0x00

-#define PM8941_WLED_REG_SINK 0x4f -#define PM8941_WLED_REG_SINK_MASK 0xe0 -#define PM8941_WLED_REG_SINK_SHFT 0x05 +#define WLED3_SINK_REG_CURR_SINK 0x4f +#define WLED3_SINK_REG_CURR_SINK_MASK 0xe0 +#define WLED3_SINK_REG_CURR_SINK_SHFT 0x05

-/* Per-'string' registers below */ -#define PM8941_WLED_REG_STR_OFFSET 0x10 +/* WLED3 per-'string' registers below */ +#define WLED3_SINK_REG_STR_OFFSET 0x10

-#define PM8941_WLED_REG_STR_MOD_EN_BASE 0x60 -#define PM8941_WLED_REG_STR_MOD_MASK BIT(7) -#define PM8941_WLED_REG_STR_MOD_EN BIT(7) +#define WLED3_SINK_REG_STR_MOD_EN_BASE 0x60 +#define WLED3_SINK_REG_STR_MOD_MASK BIT(7) +#define WLED3_SINK_REG_STR_MOD_EN BIT(7)

-#define PM8941_WLED_REG_STR_SCALE_BASE 0x62 -#define PM8941_WLED_REG_STR_SCALE_MASK 0x1f +#define WLED3_SINK_REG_STR_FULL_SCALE_CURR 0x62 +#define WLED3_SINK_REG_STR_FULL_SCALE_CURR_MASK 0x1f

-#define PM8941_WLED_REG_STR_MOD_SRC_BASE 0x63 -#define PM8941_WLED_REG_STR_MOD_SRC_MASK 0x01 -#define PM8941_WLED_REG_STR_MOD_SRC_INT 0x00 -#define PM8941_WLED_REG_STR_MOD_SRC_EXT 0x01 +#define WLED3_SINK_REG_STR_MOD_SRC_BASE 0x63 +#define WLED3_SINK_REG_STR_MOD_SRC_MASK 0x01 +#define WLED3_SINK_REG_STR_MOD_SRC_INT 0x00 +#define WLED3_SINK_REG_STR_MOD_SRC_EXT 0x01

-#define PM8941_WLED_REG_STR_CABC_BASE 0x66 -#define PM8941_WLED_REG_STR_CABC_MASK BIT(7) -#define PM8941_WLED_REG_STR_CABC_EN BIT(7) +#define WLED3_SINK_REG_STR_CABC_BASE 0x66 +#define WLED3_SINK_REG_STR_CABC_MASK BIT(7) +#define WLED3_SINK_REG_STR_CABC_EN BIT(7)

-struct pm8941_wled_config {

• u32 i_boost_limit;

+struct wled_config {

• u32 boost_i_limit; u32 ovp; u32 switch_freq; u32 num_strings;

• u32 i_limit;

• u32 string_i_limit; bool cs_out_en; bool ext_gen; bool cabc_en;

};

-struct pm8941_wled { +struct wled { const char *name; struct regmap *regmap; u16 addr;

• struct pm8941_wled_config cfg;

• struct wled_config cfg;

};

-static int pm8941_wled_update_status(struct backlight_device *bl) +static int wled_update_status(struct backlight_device *bl) {

• struct pm8941_wled *wled = bl_get_data(bl);

• struct wled *wled = bl_get_data(bl); u16 val = bl->props.brightness; u8 ctrl = 0; int rc;

@@ -100,11 +102,11 @@ static int pm8941_wled_update_status(struct backlight_device *bl) val = 0;

if (val != 0)

• ctrl = PM8941_WLED_REG_MOD_EN_BIT;
  

• ctrl = WLED3_CTRL_REG_MOD_EN_BIT;
  
  

  rc = regmap_update_bits(wled->regmap,

• 	wled->addr + PM8941_WLED_REG_MOD_EN,
  

• 	PM8941_WLED_REG_MOD_EN_MASK, ctrl);
  

• 	wled->addr + WLED3_CTRL_REG_MOD_EN,
  

• 	WLED3_CTRL_REG_MOD_EN_MASK, ctrl);
  

  if (rc) return rc;

@@ -112,89 +114,89 @@ static int pm8941_wled_update_status(struct backlight_device *bl) u8 v[2] = { val & 0xff, (val >> 8) & 0xf };

rc = regmap_bulk_write(wled->regmap,

• 		wled->addr + PM8941_WLED_REG_VAL_BASE + 2 * i,
  

• 		wled->addr + WLED3_CTRL_REG_VAL_BASE + 2 * i,
  	v, 2);
  

  if (rc) return rc; }

  rc = regmap_update_bits(wled->regmap,

• 	wled->addr + PM8941_WLED_REG_SYNC,
  

• 	PM8941_WLED_REG_SYNC_MASK, PM8941_WLED_REG_SYNC_ALL);
  

• 	wled->addr + WLED3_SINK_REG_SYNC,
  

• 	WLED3_SINK_REG_SYNC_MASK, WLED3_SINK_REG_SYNC_ALL);
  

  if (rc) return rc;

  rc = regmap_update_bits(wled->regmap,

• 	wled->addr + PM8941_WLED_REG_SYNC,
  

• 	PM8941_WLED_REG_SYNC_MASK, PM8941_WLED_REG_SYNC_CLEAR);
  

• 	wled->addr + WLED3_SINK_REG_SYNC,
  

• 	WLED3_SINK_REG_SYNC_MASK, WLED3_SINK_REG_SYNC_CLEAR);
  

  return rc;

}

-static int pm8941_wled_setup(struct pm8941_wled *wled) +static int wled_setup(struct wled *wled) { int rc; int i;

rc = regmap_update_bits(wled->regmap,

• 	wled->addr + PM8941_WLED_REG_OVP,
  

• 	PM8941_WLED_REG_OVP_MASK, wled->cfg.ovp);
  

• 	wled->addr + WLED3_CTRL_REG_OVP,
  

• 	WLED3_CTRL_REG_OVP_MASK, wled->cfg.ovp);
  

  if (rc) return rc;

  rc = regmap_update_bits(wled->regmap,

• 	wled->addr + PM8941_WLED_REG_BOOST,
  

• 	PM8941_WLED_REG_BOOST_MASK, wled->cfg.i_boost_limit);
  

• 	wled->addr + WLED3_CTRL_REG_ILIMIT,
  

• 	WLED3_CTRL_REG_ILIMIT_MASK, wled->cfg.boost_i_limit);
  

  if (rc) return rc;

  rc = regmap_update_bits(wled->regmap,

• 	wled->addr + PM8941_WLED_REG_FREQ,
  

• 	PM8941_WLED_REG_FREQ_MASK, wled->cfg.switch_freq);
  

• 	wled->addr + WLED3_CTRL_REG_FREQ,
  

• 	WLED3_CTRL_REG_FREQ_MASK, wled->cfg.switch_freq);
  

  if (rc) return rc;

  if (wled->cfg.cs_out_en) { u8 all = (BIT(wled->cfg.num_strings) - 1)

• 		<< PM8941_WLED_REG_SINK_SHFT;
  

• 		<< WLED3_SINK_REG_CURR_SINK_SHFT;
  
  

  rc = regmap_update_bits(wled->regmap,

• 		wled->addr + PM8941_WLED_REG_SINK,
  

• 		PM8941_WLED_REG_SINK_MASK, all);
  

• 		wled->addr + WLED3_SINK_REG_CURR_SINK,
  

• 		WLED3_SINK_REG_CURR_SINK_MASK, all);
  

  if (rc) return rc; }

  for (i = 0; i < wled->cfg.num_strings; ++i) {

• u16 addr = wled->addr + PM8941_WLED_REG_STR_OFFSET * i;
  

• u16 addr = wled->addr + WLED3_SINK_REG_STR_OFFSET * i;
  
  

  rc = regmap_update_bits(wled->regmap,

• 		addr + PM8941_WLED_REG_STR_MOD_EN_BASE,
  

• 		PM8941_WLED_REG_STR_MOD_MASK,
  

• 		PM8941_WLED_REG_STR_MOD_EN);
  

• 		addr + WLED3_SINK_REG_STR_MOD_EN_BASE,
  

• 		WLED3_SINK_REG_STR_MOD_MASK,
  

• 		WLED3_SINK_REG_STR_MOD_EN);
  

  if (rc) return rc;

  if (wled->cfg.ext_gen) { rc = regmap_update_bits(wled->regmap,

• 			addr + PM8941_WLED_REG_STR_MOD_SRC_BASE,
  

• 			PM8941_WLED_REG_STR_MOD_SRC_MASK,
  

• 			PM8941_WLED_REG_STR_MOD_SRC_EXT);
  

• 			addr + WLED3_SINK_REG_STR_MOD_SRC_BASE,
  

• 			WLED3_SINK_REG_STR_MOD_SRC_MASK,
  

• 			WLED3_SINK_REG_STR_MOD_SRC_EXT);
  if (rc)
  	return rc;
  

  }

  rc = regmap_update_bits(wled->regmap,

• 		addr + PM8941_WLED_REG_STR_SCALE_BASE,
  

• 		PM8941_WLED_REG_STR_SCALE_MASK,
  

• 		wled->cfg.i_limit);
  

• 		addr + WLED3_SINK_REG_STR_FULL_SCALE_CURR,
  

• 		WLED3_SINK_REG_STR_FULL_SCALE_CURR_MASK,
  

• 		wled->cfg.string_i_limit);
  

  if (rc) return rc;

  rc = regmap_update_bits(wled->regmap,

• 		addr + PM8941_WLED_REG_STR_CABC_BASE,
  

• 		PM8941_WLED_REG_STR_CABC_MASK,
  

• 		addr + WLED3_SINK_REG_STR_CABC_BASE,
  

• 		WLED3_SINK_REG_STR_CABC_MASK,
  	wled->cfg.cabc_en ?
  

• 			PM8941_WLED_REG_STR_CABC_EN : 0);
  

• 			WLED3_SINK_REG_STR_CABC_EN : 0);
  

  if (rc) return rc; }

@@ -202,9 +204,9 @@ static int pm8941_wled_setup(struct pm8941_wled *wled) return 0; }

-static const struct pm8941_wled_config pm8941_wled_config_defaults = {

• .i_boost_limit = 3,
• .i_limit = 20,

+static const struct wled_config wled3_config_defaults = {

• .boost_i_limit = 3,
• .string_i_limit = 20, .ovp = 2, .switch_freq = 5, .num_strings = 0,

@@ -213,55 +215,55 @@ static int pm8941_wled_setup(struct pm8941_wled *wled) .cabc_en = false, };

-struct pm8941_wled_var_cfg { +struct wled_var_cfg { const u32 *values; u32 (*fn)(u32); int size; };

-static const u32 pm8941_wled_i_boost_limit_values[] = { +static const u32 wled3_boost_i_limit_values[] = { 105, 385, 525, 805, 980, 1260, 1400, 1680, };

-static const struct pm8941_wled_var_cfg pm8941_wled_i_boost_limit_cfg = {

• .values = pm8941_wled_i_boost_limit_values,
• .size = ARRAY_SIZE(pm8941_wled_i_boost_limit_values),

+static const struct wled_var_cfg wled3_boost_i_limit_cfg = {

• .values = wled3_boost_i_limit_values,
• .size = ARRAY_SIZE(wled3_boost_i_limit_values),

};

-static const u32 pm8941_wled_ovp_values[] = { +static const u32 wled3_ovp_values[] = { 35, 32, 29, 27, };

-static const struct pm8941_wled_var_cfg pm8941_wled_ovp_cfg = {

• .values = pm8941_wled_ovp_values,
• .size = ARRAY_SIZE(pm8941_wled_ovp_values),

+static const struct wled_var_cfg wled3_ovp_cfg = {

• .values = wled3_ovp_values,
• .size = ARRAY_SIZE(wled3_ovp_values),

};

-static u32 pm8941_wled_num_strings_values_fn(u32 idx) +static u32 wled3_num_strings_values_fn(u32 idx) { return idx + 1; }

-static const struct pm8941_wled_var_cfg pm8941_wled_num_strings_cfg = {

• .fn = pm8941_wled_num_strings_values_fn,

+static const struct wled_var_cfg wled3_num_strings_cfg = {

• .fn = wled3_num_strings_values_fn, .size = 3,

};

-static u32 pm8941_wled_switch_freq_values_fn(u32 idx) +static u32 wled3_switch_freq_values_fn(u32 idx) { return 19200 / (2 * (1 + idx)); }

-static const struct pm8941_wled_var_cfg pm8941_wled_switch_freq_cfg = {

• .fn = pm8941_wled_switch_freq_values_fn,

+static const struct wled_var_cfg wled3_switch_freq_cfg = {

• .fn = wled3_switch_freq_values_fn, .size = 16,

};

-static const struct pm8941_wled_var_cfg pm8941_wled_i_limit_cfg = { +static const struct wled_var_cfg wled3_string_i_limit_cfg = { .size = 26, };

-static u32 pm8941_wled_values(const struct pm8941_wled_var_cfg *cfg, u32 idx) +static u32 wled3_values(const struct wled_var_cfg *cfg, u32 idx) { if (idx >= cfg->size) return UINT_MAX; @@ -272,9 +274,9 @@ static u32 pm8941_wled_values(const struct pm8941_wled_var_cfg *cfg, u32 idx) return idx; }

-static int pm8941_wled_configure(struct pm8941_wled *wled, struct device *dev) +static int wled_configure(struct wled *wled, struct device *dev) {

• struct pm8941_wled_config *cfg = &wled->cfg;

• struct wled_config *cfg = &wled->cfg; u32 val; int rc; u32 c;

@@ -284,32 +286,32 @@ static int pm8941_wled_configure(struct pm8941_wled *wled, struct device *dev) const struct { const char *name; u32 *val_ptr;

• const struct pm8941_wled_var_cfg *cfg;
  

• const struct wled_var_cfg *cfg;
  

  } u32_opts[] = { { "qcom,current-boost-limit",

• 	&cfg->i_boost_limit,
  

• 	.cfg = &pm8941_wled_i_boost_limit_cfg,
  

• 	&cfg->boost_i_limit,
  

• 	.cfg = &wled3_boost_i_limit_cfg,
  

  }, { "qcom,current-limit",

• 	&cfg->i_limit,
  

• 	.cfg = &pm8941_wled_i_limit_cfg,
  

• 	&cfg->string_i_limit,
  

• 	.cfg = &wled3_string_i_limit_cfg,
  

  }, { "qcom,ovp", &cfg->ovp,

• 	.cfg = &pm8941_wled_ovp_cfg,
  

• 	.cfg = &wled3_ovp_cfg,
  

  }, { "qcom,switching-freq", &cfg->switch_freq,

• 	.cfg = &pm8941_wled_switch_freq_cfg,
  

• 	.cfg = &wled3_switch_freq_cfg,
  

  }, { "qcom,num-strings", &cfg->num_strings,

• 	.cfg = &pm8941_wled_num_strings_cfg,
  

• 	.cfg = &wled3_num_strings_cfg,
  

  }, }; const struct {

@@ -332,7 +334,7 @@ static int pm8941_wled_configure(struct pm8941_wled *wled, struct device *dev) if (rc) wled->name = dev->of_node->name;

• *cfg = pm8941_wled_config_defaults;

• *cfg = wled3_config_defaults; for (i = 0; i < ARRAY_SIZE(u32_opts); ++i) { rc = of_property_read_u32(dev->of_node, u32_opts[i].name, &val); if (rc == -EINVAL) {

@@ -344,7 +346,7 @@ static int pm8941_wled_configure(struct pm8941_wled *wled, struct device *dev)

c = UINT_MAX;
for (j = 0; c != val; j++) {

• 	c = pm8941_wled_values(u32_opts[i].cfg, j);
  

• 	c = wled3_values(u32_opts[i].cfg, j);
  if (c == UINT_MAX) {
  	dev_err(dev, "invalid value for '%s'\n",
  		u32_opts[i].name);
  

@@ -366,15 +368,15 @@ static int pm8941_wled_configure(struct pm8941_wled *wled, struct device *dev) return 0; }

-static const struct backlight_ops pm8941_wled_ops = {

• .update_status = pm8941_wled_update_status,

+static const struct backlight_ops wled_ops = {

• .update_status = wled_update_status,

};

-static int pm8941_wled_probe(struct platform_device *pdev) +static int wled_probe(struct platform_device *pdev) { struct backlight_properties props; struct backlight_device *bl;

• struct pm8941_wled *wled;

• struct wled *wled; struct regmap *regmap; u32 val; int rc;

@@ -391,42 +393,42 @@ static int pm8941_wled_probe(struct platform_device *pdev)

wled->regmap = regmap;

• rc = pm8941_wled_configure(wled, &pdev->dev);

• rc = wled_configure(wled, &pdev->dev); if (rc) return rc;

• rc = pm8941_wled_setup(wled);

• rc = wled_setup(wled); if (rc) return rc;

• val = PM8941_WLED_DEFAULT_BRIGHTNESS;

• val = WLED_DEFAULT_BRIGHTNESS; of_property_read_u32(pdev->dev.of_node, "default-brightness", &val);

  memset(&props, 0, sizeof(struct backlight_properties)); props.type = BACKLIGHT_RAW; props.brightness = val;

• props.max_brightness = PM8941_WLED_REG_VAL_MAX;

• props.max_brightness = WLED3_SINK_REG_BRIGHT_MAX; bl = devm_backlight_device_register(&pdev->dev, wled->name, &pdev->dev, wled,

• 			    &pm8941_wled_ops, &props);
  

• 			    &wled_ops, &props);
  

  return PTR_ERR_OR_ZERO(bl);

};

-static const struct of_device_id pm8941_wled_match_table[] = { +static const struct of_device_id wled_match_table[] = { { .compatible = "qcom,pm8941-wled" }, {} }; -MODULE_DEVICE_TABLE(of, pm8941_wled_match_table); +MODULE_DEVICE_TABLE(of, wled_match_table);

-static struct platform_driver pm8941_wled_driver = {

• .probe = pm8941_wled_probe,

+static struct platform_driver wled_driver = {

• .probe = wled_probe, .driver = {

• .name = "pm8941-wled",
  

• .of_match_table	= pm8941_wled_match_table,
  

• .name = "qcom,wled",
  

• .of_match_table	= wled_match_table,
  

  },

};

-module_platform_driver(pm8941_wled_driver); +module_platform_driver(wled_driver);

-MODULE_DESCRIPTION("pm8941 wled driver"); +MODULE_DESCRIPTION("Qualcomm WLED driver"); MODULE_LICENSE("GPL v2"); -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project

-- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

On 09/07/18 11:22, Kiran Gunda wrote:

Restructure the driver to add the support for new WLED peripherals.

Signed-off-by: Kiran Gunda kgunda@codeaurora.org

Acked-by: Daniel Thompson daniel.thompson@linaro.org

---

Changes from V3:

• This is the new patch after splitting the "backlight: qcom-wled: Add support for WLED4 peripheral" patch to seperate the WLED3 specific restructure.

drivers/video/backlight/qcom-wled.c | 396 ++++++++++++++++++++++-------------- 1 file changed, 246 insertions(+), 150 deletions(-)

diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c index 3cd6e75..87fc1d0 100644 --- a/drivers/video/backlight/qcom-wled.c +++ b/drivers/video/backlight/qcom-wled.c @@ -15,59 +15,71 @@ #include <linux/module.h> #include <linux/of.h> #include <linux/of_device.h> +#include <linux/of_address.h> #include <linux/regmap.h>

/* From DT binding */ +#define WLED_MAX_STRINGS 4

• #define WLED_DEFAULT_BRIGHTNESS 2048

-#define WLED3_SINK_REG_BRIGHT_MAX 0xFFF -#define WLED3_CTRL_REG_VAL_BASE 0x40 +#define WLED_SINK_REG_BRIGHT_MAX 0xFFF

-/* WLED3 control registers */ -#define WLED3_CTRL_REG_MOD_EN 0x46 -#define WLED3_CTRL_REG_MOD_EN_BIT BIT(7) -#define WLED3_CTRL_REG_MOD_EN_MASK BIT(7) +/* WLED control registers */ +#define WLED_CTRL_REG_MOD_EN 0x46 +#define WLED_CTRL_REG_MOD_EN_MASK BIT(7) +#define WLED_CTRL_REG_MOD_EN_SHIFT 7

-#define WLED3_CTRL_REG_FREQ 0x4c -#define WLED3_CTRL_REG_FREQ_MASK 0x0f +#define WLED_CTRL_REG_FREQ 0x4c +#define WLED_CTRL_REG_FREQ_MASK GENMASK(3, 0)

-#define WLED3_CTRL_REG_OVP 0x4d -#define WLED3_CTRL_REG_OVP_MASK 0x03 +#define WLED_CTRL_REG_OVP 0x4d +#define WLED_CTRL_REG_OVP_MASK GENMASK(1, 0)

-#define WLED3_CTRL_REG_ILIMIT 0x4e -#define WLED3_CTRL_REG_ILIMIT_MASK 0x07 +#define WLED_CTRL_REG_ILIMIT 0x4e +#define WLED_CTRL_REG_ILIMIT_MASK GENMASK(2, 0)

-/* WLED3 sink registers */ -#define WLED3_SINK_REG_SYNC 0x47 -#define WLED3_SINK_REG_SYNC_MASK 0x07 -#define WLED3_SINK_REG_SYNC_LED1 BIT(0) -#define WLED3_SINK_REG_SYNC_LED2 BIT(1) -#define WLED3_SINK_REG_SYNC_LED3 BIT(2) -#define WLED3_SINK_REG_SYNC_ALL 0x07 -#define WLED3_SINK_REG_SYNC_CLEAR 0x00 +/* WLED sink registers */ +#define WLED_SINK_REG_SYNC 0x47 +#define WLED_SINK_REG_SYNC_CLEAR 0x00

#define WLED3_SINK_REG_CURR_SINK 0x4f -#define WLED3_SINK_REG_CURR_SINK_MASK 0xe0 -#define WLED3_SINK_REG_CURR_SINK_SHFT 0x05 +#define WLED3_SINK_REG_CURR_SINK_MASK GENMASK(7, 5) +#define WLED3_SINK_REG_CURR_SINK_SHFT 5

-/* WLED3 per-'string' registers below */ -#define WLED3_SINK_REG_STR_OFFSET 0x10 +/* WLED3 specific per-'string' registers below */ +#define WLED3_SINK_REG_BRIGHT(n) (0x40 + n)

-#define WLED3_SINK_REG_STR_MOD_EN_BASE 0x60 +#define WLED3_SINK_REG_STR_MOD_EN(n) (0x60 + (n * 0x10)) #define WLED3_SINK_REG_STR_MOD_MASK BIT(7) -#define WLED3_SINK_REG_STR_MOD_EN BIT(7)

-#define WLED3_SINK_REG_STR_FULL_SCALE_CURR 0x62 -#define WLED3_SINK_REG_STR_FULL_SCALE_CURR_MASK 0x1f +#define WLED3_SINK_REG_STR_FULL_SCALE_CURR(n) (0x62 + (n * 0x10)) +#define WLED3_SINK_REG_STR_FULL_SCALE_CURR_MASK GENMASK(4, 0)

-#define WLED3_SINK_REG_STR_MOD_SRC_BASE 0x63 -#define WLED3_SINK_REG_STR_MOD_SRC_MASK 0x01 +#define WLED3_SINK_REG_STR_MOD_SRC(n) (0x63 + (n * 0x10)) +#define WLED3_SINK_REG_STR_MOD_SRC_MASK BIT(0) #define WLED3_SINK_REG_STR_MOD_SRC_INT 0x00 #define WLED3_SINK_REG_STR_MOD_SRC_EXT 0x01

-#define WLED3_SINK_REG_STR_CABC_BASE 0x66 +#define WLED3_SINK_REG_STR_CABC(n) (0x66 + (n * 0x10)) #define WLED3_SINK_REG_STR_CABC_MASK BIT(7) -#define WLED3_SINK_REG_STR_CABC_EN BIT(7)

+struct wled_var_cfg {

• const u32 *values;
• u32 (*fn)(u32);
• int size;

+};

+struct wled_u32_opts {

• const char *name;
• u32 *val_ptr;
• const struct wled_var_cfg *cfg;

+};

+struct wled_bool_opts {

• const char *name;
• bool *val_ptr;

+};

struct wled_config { u32 boost_i_limit; @@ -75,132 +87,179 @@ struct wled_config { u32 switch_freq; u32 num_strings; u32 string_i_limit;

• u32 enabled_strings[WLED_MAX_STRINGS]; bool cs_out_en; bool ext_gen;

• bool cabc_en;

• bool cabc; };

  struct wled { const char *name;

• struct device *dev; struct regmap *regmap;

• u16 addr;

• u16 ctrl_addr;

• u16 max_string_count;

• u32 brightness;

• u32 max_brightness;

  struct wled_config cfg;

• int (*wled_set_brightness)(struct wled *wled, u16 brightness); };

+static int wled3_set_brightness(struct wled *wled, u16 brightness) +{

• int rc, i;
• u8 v[2];
• v[0] = brightness & 0xff;
• v[1] = (brightness >> 8) & 0xf;
• for (i = 0; i < wled->cfg.num_strings; ++i) {

• rc = regmap_bulk_write(wled->regmap, wled->ctrl_addr +
  

• 		       WLED3_SINK_REG_BRIGHT(i), v, 2);
  

• if (rc < 0)
  

• 	return rc;
  

• }
• return 0;

+}

+static int wled_module_enable(struct wled *wled, int val) +{

• int rc;
• rc = regmap_update_bits(wled->regmap, wled->ctrl_addr +

• 		WLED_CTRL_REG_MOD_EN,
  

• 		WLED_CTRL_REG_MOD_EN_MASK,
  

• 		val << WLED_CTRL_REG_MOD_EN_SHIFT);
  

• return rc;

+}

+static int wled_sync_toggle(struct wled *wled) +{

• int rc;
• unsigned int mask = GENMASK(wled->max_string_count - 1, 0);
• rc = regmap_update_bits(wled->regmap,

• 		wled->ctrl_addr + WLED_SINK_REG_SYNC,
  

• 		mask, mask);
  

• if (rc < 0)

• return rc;
  

• rc = regmap_update_bits(wled->regmap,

• 		wled->ctrl_addr + WLED_SINK_REG_SYNC,
  

• 		mask, WLED_SINK_REG_SYNC_CLEAR);
  

• return rc;

+}

• static int wled_update_status(struct backlight_device *bl) { struct wled *wled = bl_get_data(bl);

• u16 val = bl->props.brightness;
• u8 ctrl = 0;
• int rc;
• int i;

• u16 brightness = bl->props.brightness;

• int rc = 0;

  if (bl->props.power != FB_BLANK_UNBLANK || bl->props.fb_blank != FB_BLANK_UNBLANK || bl->props.state & BL_CORE_FBBLANK)

• val = 0;
  

• if (val != 0)

• ctrl = WLED3_CTRL_REG_MOD_EN_BIT;
  

• brightness = 0;
  
  

• rc = regmap_update_bits(wled->regmap,

• 	wled->addr + WLED3_CTRL_REG_MOD_EN,
  

• 	WLED3_CTRL_REG_MOD_EN_MASK, ctrl);
  

• if (rc)

• return rc;
  

• if (brightness) {

• rc = wled->wled_set_brightness(wled, brightness);
  

• if (rc < 0) {
  

• 	dev_err(wled->dev, "wled failed to set brightness rc:%d\n",
  

• 		rc);
  

• 	return rc;
  

• }
  
  

• for (i = 0; i < wled->cfg.num_strings; ++i) {

• u8 v[2] = { val & 0xff, (val >> 8) & 0xf };
  

• rc = wled_sync_toggle(wled);
  

• if (rc < 0) {
  

• 	dev_err(wled->dev, "wled sync failed rc:%d\n", rc);
  

• 	return rc;
  

• }
  

• }

• rc = regmap_bulk_write(wled->regmap,
  

• 		wled->addr + WLED3_CTRL_REG_VAL_BASE + 2 * i,
  

• 		v, 2);
  

• if (rc)
  

• if (!!brightness != !!wled->brightness) {

• rc = wled_module_enable(wled, !!brightness);
  

• if (rc < 0) {
  

• 	dev_err(wled->dev, "wled enable failed rc:%d\n", rc);
  return rc;
  

• }
  

  }

• rc = regmap_update_bits(wled->regmap,

• 	wled->addr + WLED3_SINK_REG_SYNC,
  

• 	WLED3_SINK_REG_SYNC_MASK, WLED3_SINK_REG_SYNC_ALL);
  

• if (rc)

• return rc;
  

• wled->brightness = brightness;

• rc = regmap_update_bits(wled->regmap,

• 	wled->addr + WLED3_SINK_REG_SYNC,
  

• 	WLED3_SINK_REG_SYNC_MASK, WLED3_SINK_REG_SYNC_CLEAR);
  

  return rc; }

-static int wled_setup(struct wled *wled) +static int wled3_setup(struct wled *wled) {

• int rc;
• int i;

• u16 addr;

• u8 sink_en = 0;

• int rc, i, j;

  rc = regmap_update_bits(wled->regmap,

• 	wled->addr + WLED3_CTRL_REG_OVP,
  

• 	WLED3_CTRL_REG_OVP_MASK, wled->cfg.ovp);
  

• 		wled->ctrl_addr + WLED_CTRL_REG_OVP,
  

• 		WLED_CTRL_REG_OVP_MASK, wled->cfg.ovp);
  

  if (rc) return rc;

  rc = regmap_update_bits(wled->regmap,

• 	wled->addr + WLED3_CTRL_REG_ILIMIT,
  

• 	WLED3_CTRL_REG_ILIMIT_MASK, wled->cfg.boost_i_limit);
  

• 		wled->ctrl_addr + WLED_CTRL_REG_ILIMIT,
  

• 		WLED_CTRL_REG_ILIMIT_MASK,
  

• 		wled->cfg.boost_i_limit);
  

  if (rc) return rc;

  rc = regmap_update_bits(wled->regmap,

• 	wled->addr + WLED3_CTRL_REG_FREQ,
  

• 	WLED3_CTRL_REG_FREQ_MASK, wled->cfg.switch_freq);
  

• 		wled->ctrl_addr + WLED_CTRL_REG_FREQ,
  

• 		WLED_CTRL_REG_FREQ_MASK,
  

• 		wled->cfg.switch_freq);
  

  if (rc) return rc;

• if (wled->cfg.cs_out_en) {

• u8 all = (BIT(wled->cfg.num_strings) - 1)
  

• 		<< WLED3_SINK_REG_CURR_SINK_SHFT;
  

• rc = regmap_update_bits(wled->regmap,
  

• 		wled->addr + WLED3_SINK_REG_CURR_SINK,
  

• 		WLED3_SINK_REG_CURR_SINK_MASK, all);
  

• if (rc)
  

• 	return rc;
  

• }
• for (i = 0; i < wled->cfg.num_strings; ++i) {

• u16 addr = wled->addr + WLED3_SINK_REG_STR_OFFSET * i;
  

• rc = regmap_update_bits(wled->regmap,
  

• 		addr + WLED3_SINK_REG_STR_MOD_EN_BASE,
  

• 		WLED3_SINK_REG_STR_MOD_MASK,
  

• 		WLED3_SINK_REG_STR_MOD_EN);
  

• j = wled->cfg.enabled_strings[i];
  

• addr = wled->ctrl_addr + WLED3_SINK_REG_STR_MOD_EN(j);
  

• rc = regmap_update_bits(wled->regmap, addr,
  

• 			WLED3_SINK_REG_STR_MOD_MASK,
  

• 			WLED3_SINK_REG_STR_MOD_MASK);
  

  if (rc) return rc;

  if (wled->cfg.ext_gen) {

• 	rc = regmap_update_bits(wled->regmap,
  

• 			addr + WLED3_SINK_REG_STR_MOD_SRC_BASE,
  

• 			WLED3_SINK_REG_STR_MOD_SRC_MASK,
  

• 			WLED3_SINK_REG_STR_MOD_SRC_EXT);
  

• 	addr = wled->ctrl_addr + WLED3_SINK_REG_STR_MOD_SRC(j);
  

• 	rc = regmap_update_bits(wled->regmap, addr,
  

• 				WLED3_SINK_REG_STR_MOD_SRC_MASK,
  

• 				WLED3_SINK_REG_STR_MOD_SRC_EXT);
  if (rc)
  	return rc;
  

  }

• rc = regmap_update_bits(wled->regmap,
  

• 		addr + WLED3_SINK_REG_STR_FULL_SCALE_CURR,
  

• 		WLED3_SINK_REG_STR_FULL_SCALE_CURR_MASK,
  

• 		wled->cfg.string_i_limit);
  

• addr = wled->ctrl_addr + WLED3_SINK_REG_STR_FULL_SCALE_CURR(j);
  

• rc = regmap_update_bits(wled->regmap, addr,
  

• 			WLED3_SINK_REG_STR_FULL_SCALE_CURR_MASK,
  

• 			wled->cfg.string_i_limit);
  

  if (rc) return rc;

• rc = regmap_update_bits(wled->regmap,
  

• 		addr + WLED3_SINK_REG_STR_CABC_BASE,
  

• 		WLED3_SINK_REG_STR_CABC_MASK,
  

• 		wled->cfg.cabc_en ?
  

• 			WLED3_SINK_REG_STR_CABC_EN : 0);
  

• addr = wled->ctrl_addr + WLED3_SINK_REG_STR_CABC(j);
  

• rc = regmap_update_bits(wled->regmap, addr,
  

• 			WLED3_SINK_REG_STR_CABC_MASK,
  

• 			wled->cfg.cabc ?
  

• 			WLED3_SINK_REG_STR_CABC_MASK : 0);
  

  if (rc) return rc;

• sink_en |= BIT(j + WLED3_SINK_REG_CURR_SINK_SHFT);
  

  }

• rc = regmap_update_bits(wled->regmap,

• 		wled->ctrl_addr + WLED3_SINK_REG_CURR_SINK,
  

• 		WLED3_SINK_REG_CURR_SINK_MASK, sink_en);
  

• if (rc)

• return rc;
  

• return 0; }

@@ -208,17 +267,11 @@ static int wled_setup(struct wled *wled) .boost_i_limit = 3, .string_i_limit = 20, .ovp = 2,

• .num_strings = 3, .switch_freq = 5,

• .num_strings = 0, .cs_out_en = false, .ext_gen = false,
• .cabc_en = false,

-};

-struct wled_var_cfg {

• const u32 *values;
• u32 (*fn)(u32);
• int size;

• .cabc = false, };

  static const u32 wled3_boost_i_limit_values[] = {

@@ -263,7 +316,11 @@ static u32 wled3_switch_freq_values_fn(u32 idx) .size = 26, };

-static u32 wled3_values(const struct wled_var_cfg *cfg, u32 idx) +static const struct wled_var_cfg wled3_string_cfg = {

• .size = 8,

+};

+static u32 wled_values(const struct wled_var_cfg *cfg, u32 idx) { if (idx >= cfg->size) return UINT_MAX; @@ -274,68 +331,77 @@ static u32 wled3_values(const struct wled_var_cfg *cfg, u32 idx) return idx; }

-static int wled_configure(struct wled *wled, struct device *dev) +#define WLED3 3

+static int wled_configure(struct wled *wled, int version) { struct wled_config *cfg = &wled->cfg;

• u32 val;
• int rc;
• u32 c;
• int i;
• int j;
• const struct {

• const char *name;
  

• u32 *val_ptr;
  

• const struct wled_var_cfg *cfg;
  

• } u32_opts[] = {

• struct device *dev = wled->dev;
• const __be32 *prop_addr;
• u32 size, val, c, string_len;
• int rc, i, j;
• const struct wled_u32_opts *u32_opts = NULL;
• const struct wled_u32_opts wled3_opts[] = { {

• 	"qcom,current-boost-limit",
  

• 	&cfg->boost_i_limit,
  

• 	.name = "qcom,current-boost-limit",
  

• 	.val_ptr = &cfg->boost_i_limit,
  .cfg = &wled3_boost_i_limit_cfg,
  

  }, {

• 	"qcom,current-limit",
  

• 	&cfg->string_i_limit,
  

• 	.name = "qcom,current-limit",
  

• 	.val_ptr = &cfg->string_i_limit,
  .cfg = &wled3_string_i_limit_cfg,
  

  }, {

• 	"qcom,ovp",
  

• 	&cfg->ovp,
  

• 	.name = "qcom,ovp",
  

• 	.val_ptr = &cfg->ovp,
  .cfg = &wled3_ovp_cfg,
  

  }, {

• 	"qcom,switching-freq",
  

• 	&cfg->switch_freq,
  

• 	.name = "qcom,switching-freq",
  

• 	.val_ptr = &cfg->switch_freq,
  .cfg = &wled3_switch_freq_cfg,
  

  }, {

• 	"qcom,num-strings",
  

• 	&cfg->num_strings,
  

• 	.name = "qcom,num-strings",
  

• 	.val_ptr = &cfg->num_strings,
  .cfg = &wled3_num_strings_cfg,
  

  }, };

• const struct {

• const char *name;
  

• bool *val_ptr;
  

• } bool_opts[] = {

• const struct wled_bool_opts bool_opts[] = { { "qcom,cs-out", &cfg->cs_out_en, }, { "qcom,ext-gen", &cfg->ext_gen, },

• { "qcom,cabc", &cfg->cabc_en, },
  

• { "qcom,cabc", &cfg->cabc, },
  

  };

• rc = of_property_read_u32(dev->of_node, "reg", &val);
• if (rc || val > 0xffff) {

• dev_err(dev, "invalid IO resources\n");
  

• return rc ? rc : -EINVAL;
  

• prop_addr = of_get_address(dev->of_node, 0, NULL, NULL);
• if (!prop_addr) {

• dev_err(wled->dev, "invalid IO resources\n");
  

• return -EINVAL;
  

  }

• wled->addr = val;

• wled->ctrl_addr = be32_to_cpu(*prop_addr);

  rc = of_property_read_string(dev->of_node, "label", &wled->name); if (rc) wled->name = dev->of_node->name;

• *cfg = wled3_config_defaults;
• for (i = 0; i < ARRAY_SIZE(u32_opts); ++i) {

• switch (version) {
• case WLED3:

• u32_opts = wled3_opts;
  

• size = ARRAY_SIZE(wled3_opts);
  

• *cfg = wled3_config_defaults;
  

• wled->wled_set_brightness = wled3_set_brightness;
  

• wled->max_string_count = 3;
  

• break;
  

• default:

• dev_err(wled->dev, "Invalid WLED version\n");
  

• break;
  

• }
• for (i = 0; i < size; ++i) { rc = of_property_read_u32(dev->of_node, u32_opts[i].name, &val); if (rc == -EINVAL) { continue;

@@ -346,12 +412,15 @@ static int wled_configure(struct wled *wled, struct device *dev)

c = UINT_MAX;
for (j = 0; c != val; j++) {

• 	c = wled3_values(u32_opts[i].cfg, j);
  

• 	c = wled_values(u32_opts[i].cfg, j);
  if (c == UINT_MAX) {
  	dev_err(dev, "invalid value for '%s'\n",
  		u32_opts[i].name);
  	return -EINVAL;
  }
  

• 	if (c == val)
  

• 		break;
  

  }

  dev_dbg(dev, "'%s' = %u\n", u32_opts[i].name, c);

@@ -365,6 +434,15 @@ static int wled_configure(struct wled *wled, struct device *dev)

cfg->num_strings = cfg->num_strings + 1;

• string_len = of_property_count_elems_of_size(dev->of_node,

• 				     "qcom,enabled-strings",
  

• 				     sizeof(u32));
  

• if (string_len > 0)

• rc = of_property_read_u32_array(dev->of_node,
  

• 				"qcom,enabled-strings",
  

• 				wled->cfg.enabled_strings,
  

• 				sizeof(u32));
  

• return 0; }

@@ -378,6 +456,7 @@ static int wled_probe(struct platform_device *pdev) struct backlight_device *bl; struct wled *wled; struct regmap *regmap;

• int version; u32 val; int rc;

@@ -392,22 +471,39 @@ static int wled_probe(struct platform_device *pdev) return -ENOMEM;

wled->regmap = regmap;

• wled->dev = &pdev->dev;

• rc = wled_configure(wled, &pdev->dev);
• if (rc)

• return rc;
  

• version = (uintptr_t)of_device_get_match_data(&pdev->dev);
• if (!version) {

• dev_err(&pdev->dev, "Unknown device version\n");
  

• return -ENODEV;
  

• }

• rc = wled_setup(wled);

• rc = wled_configure(wled, version); if (rc) return rc;

• switch (version) {

• case WLED3:

• rc = wled3_setup(wled);
  

• if (rc) {
  

• 	dev_err(&pdev->dev, "wled3_setup failed\n");
  

• 	return rc;
  

• }
  

• break;
  

• default:

• dev_err(wled->dev, "Invalid WLED version\n");
  

• break;
  

• }

• val = WLED_DEFAULT_BRIGHTNESS; of_property_read_u32(pdev->dev.of_node, "default-brightness", &val);

  memset(&props, 0, sizeof(struct backlight_properties)); props.type = BACKLIGHT_RAW; props.brightness = val;

• props.max_brightness = WLED3_SINK_REG_BRIGHT_MAX;

• props.max_brightness = WLED_SINK_REG_BRIGHT_MAX; bl = devm_backlight_device_register(&pdev->dev, wled->name, &pdev->dev, wled, &wled_ops, &props);

@@ -415,7 +511,7 @@ static int wled_probe(struct platform_device *pdev) };

static const struct of_device_id wled_match_table[] = {

• { .compatible = "qcom,pm8941-wled" },

• { .compatible = "qcom,pm8941-wled", .data = (void *)3 }, {} }; MODULE_DEVICE_TABLE(of, wled_match_table);

On 2018-08-07 11:07, Bjorn Andersson wrote:

On Mon 09 Jul 03:22 PDT 2018, Kiran Gunda wrote:

...

diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c

[..]

...

@@ -365,6 +434,15 @@ static int wled_configure(struct wled *wled, struct device *dev)

cfg->num_strings = cfg->num_strings + 1;

• string_len = of_property_count_elems_of_size(dev->of_node,

• 				     "qcom,enabled-strings",
  

• 				     sizeof(u32));
  

• if (string_len > 0)

• rc = of_property_read_u32_array(dev->of_node,
  

• 				"qcom,enabled-strings",
  

• 				wled->cfg.enabled_strings,
  

qcom,enabled-strings is listed as optional, but without it we will end up with qcom,num-strings zeros in an array. Initialize wled->cfg.enabled_strings to 0,1,2,3 and the driver will be backwards compatible.

Correct. I will initialize the array in the next series.

I also think that if you do qcom,enabled-strings = <0, 1, 2>; there's no need to also specify qcom,num-strings = <3>; and we can just use string_len for num_strings.

Correct. But I have kept this property to have the backward compatibility with the old driver (WLED3).

...

• 				sizeof(u32));
  

• return 0;

}

Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

On 09/07/18 11:22, Kiran Gunda wrote:

WLED4 peripheral is present on some PMICs like pmi8998 and pm660l. It has a different register map and configurations are also different. Add support for it.

Signed-off-by: Kiran Gunda kgunda@codeaurora.org

Changes from V3:

• The WLED3 specific changes are splitted out
• Merged the wled3_sync_toggle and wled4_syn_toggle functions
• Modified the compatible .data
• Moved from if/else to switch/case to select the version specific data
• Addressed few more minor comments from Bjorn

drivers/video/backlight/qcom-wled.c | 264 +++++++++++++++++++++++++++++++++++- 1 file changed, 258 insertions(+), 6 deletions(-)

diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c index 87fc1d0..362d254 100644 --- a/drivers/video/backlight/qcom-wled.c +++ b/drivers/video/backlight/qcom-wled.c @@ -64,6 +64,28 @@ #define WLED3_SINK_REG_STR_CABC(n) (0x66 + (n * 0x10)) #define WLED3_SINK_REG_STR_CABC_MASK BIT(7)

+/* WLED4 specific sink registers */ +#define WLED4_SINK_REG_CURR_SINK 0x46 +#define WLED4_SINK_REG_CURR_SINK_MASK GENMASK(7, 4) +#define WLED4_SINK_REG_CURR_SINK_SHFT 4

+/* WLED4 specific per-'string' registers below */ +#define WLED4_SINK_REG_STR_MOD_EN(n) (0x50 + (n * 0x10)) +#define WLED4_SINK_REG_STR_MOD_MASK BIT(7)

+#define WLED4_SINK_REG_STR_FULL_SCALE_CURR(n) (0x52 + (n * 0x10)) +#define WLED4_SINK_REG_STR_FULL_SCALE_CURR_MASK GENMASK(3, 0)

+#define WLED4_SINK_REG_STR_MOD_SRC(n) (0x53 + (n * 0x10)) +#define WLED4_SINK_REG_STR_MOD_SRC_MASK BIT(0) +#define WLED4_SINK_REG_STR_MOD_SRC_INT 0x00 +#define WLED4_SINK_REG_STR_MOD_SRC_EXT 0x01

+#define WLED4_SINK_REG_STR_CABC(n) (0x56 + (n * 0x10)) +#define WLED4_SINK_REG_STR_CABC_MASK BIT(7)

+#define WLED4_SINK_REG_BRIGHT(n) (0x57 + (n * 0x10))

• struct wled_var_cfg { const u32 *values; u32 (*fn)(u32);

@@ -98,6 +120,7 @@ struct wled { struct device *dev; struct regmap *regmap; u16 ctrl_addr;

• u16 sink_addr; u16 max_string_count; u32 brightness; u32 max_brightness;

@@ -124,6 +147,29 @@ static int wled3_set_brightness(struct wled *wled, u16 brightness) return 0; }

+static int wled4_set_brightness(struct wled *wled, u16 brightness) +{

• int rc, i;
• u16 low_limit = wled->max_brightness * 4 / 1000;
• u8 v[2];
• /* WLED4's lower limit of operation is 0.4% */
• if (brightness > 0 && brightness < low_limit)

• brightness = low_limit;
  

• v[0] = brightness & 0xff;
• v[1] = (brightness >> 8) & 0xf;
• for (i = 0; i < wled->cfg.num_strings; ++i) {

• rc = regmap_bulk_write(wled->regmap, wled->sink_addr +
  

• 		       WLED4_SINK_REG_BRIGHT(i), v, 2);
  

• if (rc < 0)
  

• 	return rc;
  

• }
• return 0;

+}

• static int wled_module_enable(struct wled *wled, int val) { int rc;

@@ -141,13 +187,13 @@ static int wled_sync_toggle(struct wled *wled) unsigned int mask = GENMASK(wled->max_string_count - 1, 0);

rc = regmap_update_bits(wled->regmap,

• 		wled->ctrl_addr + WLED_SINK_REG_SYNC,
  

• 		wled->sink_addr + WLED_SINK_REG_SYNC,
  	mask, mask);
  

  if (rc < 0) return rc;

  rc = regmap_update_bits(wled->regmap,

• 		wled->ctrl_addr + WLED_SINK_REG_SYNC,
  

• 		wled->sink_addr + WLED_SINK_REG_SYNC,
  	mask, WLED_SINK_REG_SYNC_CLEAR);
  
  

  return rc;

@@ -274,6 +320,120 @@ static int wled3_setup(struct wled *wled) .cabc = false, };

+static int wled4_setup(struct wled *wled) +{

• int rc, temp, i, j;
• u16 addr;
• u8 sink_en = 0;
• u32 sink_cfg = 0;
• rc = regmap_update_bits(wled->regmap,

• 		wled->ctrl_addr + WLED_CTRL_REG_OVP,
  

• 		WLED_CTRL_REG_OVP_MASK, wled->cfg.ovp);
  

• if (rc < 0)

• return rc;
  

• rc = regmap_update_bits(wled->regmap,

• 		wled->ctrl_addr + WLED_CTRL_REG_ILIMIT,
  

• 		WLED_CTRL_REG_ILIMIT_MASK,
  

• 		wled->cfg.boost_i_limit);
  

• if (rc < 0)

• return rc;
  

• rc = regmap_update_bits(wled->regmap,

• 		wled->ctrl_addr + WLED_CTRL_REG_FREQ,
  

• 		WLED_CTRL_REG_FREQ_MASK,
  

• 		wled->cfg.switch_freq);
  

• if (rc < 0)

• return rc;
  

• rc = regmap_read(wled->regmap, wled->sink_addr +

• 	 WLED4_SINK_REG_CURR_SINK, &sink_cfg);
  

• if (rc < 0)

• return rc;
  

• for (i = 0; i < wled->cfg.num_strings; i++) {

• j = wled->cfg.enabled_strings[i];
  

• temp = j + WLED4_SINK_REG_CURR_SINK_SHFT;
  

• sink_en |= 1 << temp;
  

• }
• if (sink_cfg == sink_en)

• return 0;
  

• rc = regmap_update_bits(wled->regmap,

• 		wled->sink_addr + WLED4_SINK_REG_CURR_SINK,
  

• 		WLED4_SINK_REG_CURR_SINK_MASK, 0);
  

• if (rc < 0)

• return rc;
  

• rc = regmap_update_bits(wled->regmap, wled->ctrl_addr +

• 		WLED_CTRL_REG_MOD_EN,
  

• 		WLED_CTRL_REG_MOD_EN_MASK, 0);
  

• if (rc < 0)

• return rc;
  

• /* Per sink/string configuration */
• for (i = 0; i < wled->cfg.num_strings; i++) {

• j = wled->cfg.enabled_strings[i];
  

• addr = wled->sink_addr +
  

• 		WLED4_SINK_REG_STR_MOD_EN(j);
  

• rc = regmap_update_bits(wled->regmap, addr,
  

• 			WLED4_SINK_REG_STR_MOD_MASK,
  

• 			WLED4_SINK_REG_STR_MOD_MASK);
  

• if (rc < 0)
  

• 	return rc;
  

• addr = wled->sink_addr +
  

• 		WLED4_SINK_REG_STR_FULL_SCALE_CURR(j);
  

• rc = regmap_update_bits(wled->regmap, addr,
  

• 			WLED4_SINK_REG_STR_FULL_SCALE_CURR_MASK,
  

• 			wled->cfg.string_i_limit);
  

• if (rc < 0)
  

• 	return rc;
  

• addr = wled->sink_addr +
  

• 		WLED4_SINK_REG_STR_CABC(j);
  

• rc = regmap_update_bits(wled->regmap, addr,
  

• 			WLED4_SINK_REG_STR_CABC_MASK,
  

• 			wled->cfg.cabc ?
  

• 			WLED4_SINK_REG_STR_CABC_MASK : 0);
  

• if (rc < 0)
  

• 	return rc;
  

• }
• rc = regmap_update_bits(wled->regmap, wled->ctrl_addr +

• 		WLED_CTRL_REG_MOD_EN,
  

• 		WLED_CTRL_REG_MOD_EN_MASK,
  

• 		WLED_CTRL_REG_MOD_EN_MASK);
  

• if (rc < 0)

• return rc;
  

• rc = regmap_update_bits(wled->regmap,

• 		wled->sink_addr + WLED4_SINK_REG_CURR_SINK,
  

• 		WLED4_SINK_REG_CURR_SINK_MASK, sink_en);
  

• if (rc < 0)

• return rc;
  

• rc = wled_sync_toggle(wled);
• if (rc < 0) {

• dev_err(wled->dev, "Failed to toggle sync reg rc:%d\n", rc);
  

• return rc;
  

• }
• return 0;

+}

+static const struct wled_config wled4_config_defaults = {

• .boost_i_limit = 4,
• .string_i_limit = 10,
• .ovp = 1,
• .num_strings = 4,
• .switch_freq = 11,
• .cabc = false,

+};

• static const u32 wled3_boost_i_limit_values[] = { 105, 385, 525, 805, 980, 1260, 1400, 1680, };

@@ -283,6 +443,15 @@ static int wled3_setup(struct wled *wled) .size = ARRAY_SIZE(wled3_boost_i_limit_values), };

+static const u32 wled4_boost_i_limit_values[] = {

• 105, 280, 450, 620, 970, 1150, 1300, 1500,

+};

+static const struct wled_var_cfg wled4_boost_i_limit_cfg = {

• .values = wled4_boost_i_limit_values,
• .size = ARRAY_SIZE(wled4_boost_i_limit_values),

+};

• static const u32 wled3_ovp_values[] = { 35, 32, 29, 27, };

@@ -292,6 +461,15 @@ static int wled3_setup(struct wled *wled) .size = ARRAY_SIZE(wled3_ovp_values), };

+static const u32 wled4_ovp_values[] = {

• 31100, 29600, 19600, 18100,

+};

+static const struct wled_var_cfg wled4_ovp_cfg = {

• .values = wled4_ovp_values,
• .size = ARRAY_SIZE(wled4_ovp_values),

+};

• static u32 wled3_num_strings_values_fn(u32 idx) { return idx + 1;

@@ -302,6 +480,11 @@ static u32 wled3_num_strings_values_fn(u32 idx) .size = 3, };

+static const struct wled_var_cfg wled4_num_strings_cfg = {

• .fn = wled3_num_strings_values_fn,
• .size = 4,

+};

• static u32 wled3_switch_freq_values_fn(u32 idx) { return 19200 / (2 * (1 + idx));

@@ -316,10 +499,24 @@ static u32 wled3_switch_freq_values_fn(u32 idx) .size = 26, };

+static const u32 wled4_string_i_limit_values[] = {

• 0, 2500, 5000, 7500, 10000, 12500, 15000, 17500, 20000,
• 22500, 25000, 27500, 30000,

+};

+static const struct wled_var_cfg wled4_string_i_limit_cfg = {

• .values = wled4_string_i_limit_values,
• .size = ARRAY_SIZE(wled4_string_i_limit_values),

+};

• static const struct wled_var_cfg wled3_string_cfg = { .size = 8, };

+static const struct wled_var_cfg wled4_string_cfg = {

• .size = 16,

+};

• static u32 wled_values(const struct wled_var_cfg *cfg, u32 idx) { if (idx >= cfg->size)

@@ -332,6 +529,7 @@ static u32 wled_values(const struct wled_var_cfg *cfg, u32 idx) }

#define WLED3 3 +#define WLED4 4

Are these macros always going to define 3 to be 3 and 4 to be 4. If so we probably don't need them (and they should be removed from patch 5/8 too).

There is a degree of nitpicking here however. The rest looks OK to me.

Daniel.

On 2018-07-21 02:51, Pavel Machek wrote:

Hi!

...

...

@@ -332,6 +529,7 @@ static u32 wled_values(const struct wled_var_cfg *cfg, u32 idx) } #define WLED3 3 +#define WLED4 4

Are these macros always going to define 3 to be 3 and 4 to be 4. If so we probably don't need them (and they should be removed from patch 5/8 too).

There is a degree of nitpicking here however. The rest looks OK to me.

Really nitpicking, and I don't care much either way, but as natural numbering would be 0-based, I'd keep the defines. Pavel

Thanks Pavale/Daniel! I will remove the macros and use the number 3 and 4 instead.

On 2018-07-20 19:37, Daniel Thompson wrote:

On 09/07/18 11:22, Kiran Gunda wrote:

...

Handle the short circuit interrupt and check if the short circuit interrupt is valid. Re-enable the module to check if it goes away. Disable the module altogether if the short circuit event persists.

Signed-off-by: Kiran Gunda kgunda@codeaurora.org Reviewed-by: Bjorn Andersson bjorn.andersson@linaro.org

---

Changes from V3:

• Added Reviewed by tag.
• Addressed minor comments from Vinod

drivers/video/backlight/qcom-wled.c | 132 ++++++++++++++++++++++++++++++++++-- 1 file changed, 128 insertions(+), 4 deletions(-)

diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c index 362d254..a14f1a6 100644 --- a/drivers/video/backlight/qcom-wled.c +++ b/drivers/video/backlight/qcom-wled.c @@ -10,6 +10,9 @@

• GNU General Public License for more details.

*/ +#include <linux/delay.h> +#include <linux/interrupt.h> +#include <linux/ktime.h> #include <linux/kernel.h> #include <linux/backlight.h> #include <linux/module.h> @@ -64,6 +67,16 @@ #define WLED3_SINK_REG_STR_CABC(n) (0x66 + (n * 0x10)) #define WLED3_SINK_REG_STR_CABC_MASK BIT(7) +/* WLED4 specific control registers */ +#define WLED4_CTRL_REG_SHORT_PROTECT 0x5e +#define WLED4_CTRL_REG_SHORT_EN_MASK BIT(7)

+#define WLED4_CTRL_REG_SEC_ACCESS 0xd0 +#define WLED4_CTRL_REG_SEC_UNLOCK 0xa5

+#define WLED4_CTRL_REG_TEST1 0xe2 +#define WLED4_CTRL_REG_TEST1_EXT_FET_DTEST2 0x09

• /* WLED4 specific sink registers */ #define WLED4_SINK_REG_CURR_SINK 0x46 #define WLED4_SINK_REG_CURR_SINK_MASK GENMASK(7, 4)

@@ -113,17 +126,23 @@ struct wled_config { bool cs_out_en; bool ext_gen; bool cabc;

• bool external_pfet; }; struct wled { const char *name; struct device *dev; struct regmap *regmap;
• struct mutex lock; /* Lock to avoid race from thread irq handler */
• ktime_t last_short_event; u16 ctrl_addr; u16 sink_addr; u16 max_string_count; u32 brightness; u32 max_brightness;
• u32 short_count;
• bool disabled_by_short;
• bool has_short_detect; struct wled_config cfg; int (*wled_set_brightness)(struct wled *wled, u16 brightness);

@@ -174,6 +193,9 @@ static int wled_module_enable(struct wled *wled, int val) { int rc;

• if (wled->disabled_by_short)

• return -EFAULT;
  

EFAULT means bad address (memory fault). Perhaps EIO or maybe even something more exotic like ENXIO might be more appropriate?

...

• rc = regmap_update_bits(wled->regmap, wled->ctrl_addr + WLED_CTRL_REG_MOD_EN, WLED_CTRL_REG_MOD_EN_MASK,

@@ -210,18 +232,19 @@ static int wled_update_status(struct backlight_device *bl) bl->props.state & BL_CORE_FBBLANK) brightness = 0;

• mutex_lock(&wled->lock);

if (brightness) { rc = wled->wled_set_brightness(wled, brightness); if (rc < 0) { dev_err(wled->dev, "wled failed to set brightness rc:%d\n", rc);

• 	return rc;
  

• 	goto unlock_mutex;
  

  } rc = wled_sync_toggle(wled); if (rc < 0) { dev_err(wled->dev, "wled sync failed rc:%d\n", rc);

• 	return rc;
  

• 	goto unlock_mutex;
  

  } } @@ -229,15 +252,61 @@ static int wled_update_status(struct

backlight_device *bl) rc = wled_module_enable(wled, !!brightness); if (rc < 0) { dev_err(wled->dev, "wled enable failed rc:%d\n", rc);

• 	return rc;
  

• 	goto unlock_mutex;
  

  } } wled->brightness = brightness; +unlock_mutex:
• mutex_unlock(&wled->lock);
• return rc; } +#define WLED_SHORT_DLY_MS 20

+#define WLED_SHORT_CNT_MAX 5 +#define WLED_SHORT_RESET_CNT_DLY_US USEC_PER_SEC

+static irqreturn_t wled_short_irq_handler(int irq, void *_wled) +{

• struct wled *wled = _wled;
• int rc;
• s64 elapsed_time;
• wled->short_count++;
• mutex_lock(&wled->lock);
• rc = wled_module_enable(wled, false);
• if (rc < 0) {

• dev_err(wled->dev, "wled disable failed rc:%d\n", rc);
  

• goto unlock_mutex;
  

• }
• elapsed_time = ktime_us_delta(ktime_get(),

• 		      wled->last_short_event);
  

• if (elapsed_time > WLED_SHORT_RESET_CNT_DLY_US)

• wled->short_count = 0;
  

Shouldn't this reset to 1 (e.g. we have just seen a short).

Daniel.

Ok. I will change it in the next series.

On Mon 09 Jul 03:22 PDT 2018, Kiran Gunda wrote:

diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c

[..]

@@ -189,6 +206,15 @@ static int wled4_set_brightness(struct wled *wled, u16 brightness) return 0; }

+static void wled_ovp_work(struct work_struct *work) +{

• struct wled *wled = container_of(work,

• 			 struct wled, ovp_work.work);
  

• if (wled->ovp_irq > 0)

• enable_irq(wled->ovp_irq);
  

+}

static int wled_module_enable(struct wled *wled, int val) { int rc; @@ -200,7 +226,18 @@ static int wled_module_enable(struct wled *wled, int val) WLED_CTRL_REG_MOD_EN, WLED_CTRL_REG_MOD_EN_MASK, val << WLED_CTRL_REG_MOD_EN_SHIFT);

• return rc;

• if (rc < 0)

• return rc;
  

• if (val) {

• schedule_delayed_work(&wled->ovp_work, WLED_SOFT_START_DLY_US);
  

schedule_delayed_work() takes a delay in jiffies, not micro seconds.

• } else {

• cancel_delayed_work_sync(&wled->ovp_work);
  

If we get here within WLED_SOFT_START_DLY_US of a module enable we have yet to execute the enable_irq() in the delayed worker, which should mean that the irq is unbalanced from then on.

I believe you can check the return value of cancel_delayed_work_sync() being true to know if wled_ovp_work() was yet to execute.

• if (wled->ovp_irq > 0)
  

• 	disable_irq(wled->ovp_irq);
  

• }
• return 0;

}

Regards, Bjorn