* [PATCH V7 1/6] backlight: qcom-wled: Add new properties for PMI8998
2019-10-16 10:13 [PATCH V7 0/6] backlight: qcom-wled: Support for QCOM wled driver Kiran Gunda
@ 2019-10-16 10:13 ` Kiran Gunda
2019-10-16 10:13 ` [PATCH V7 2/6] backlight: qcom-wled: Rename PM8941* to WLED3 Kiran Gunda
` (4 subsequent siblings)
5 siblings, 0 replies; 19+ messages in thread
From: Kiran Gunda @ 2019-10-16 10:13 UTC (permalink / raw)
To: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
daniel.thompson, jacek.anaszewski, pavel, robh+dt, mark.rutland,
linux-leds, devicetree, linux-kernel, Dan Murphy
Cc: linux-arm-msm, Kiran Gunda
Update the bindings with the new properties used for
PMI8998.
Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Acked-by: Daniel Thompson <daniel.thompson@linaro.org>
---
.../bindings/leds/backlight/qcom-wled.txt | 74 ++++++++++++++++++----
1 file changed, 63 insertions(+), 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
index 14f28f2..c06863b 100644
--- a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
+++ b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
@@ -20,7 +20,7 @@ platforms. The PMIC is connected to the host processor via SPMI bus.
- default-brightness
Usage: optional
Value type: <u32>
- Definition: brightness value on boot, value from: 0-4095
+ Definition: brightness value on boot, value from: 0-4095.
Default: 2048
- label
@@ -48,20 +48,24 @@ platforms. The PMIC is connected to the host processor via SPMI bus.
- qcom,current-limit
Usage: optional
Value type: <u32>
- Definition: mA; per-string current limit
- value: For pm8941: from 0 to 25 with 5 mA step
- Default 20 mA.
- For pmi8998: from 0 to 30 with 5 mA step
- Default 25 mA.
+ Definition: mA; per-string current limit; value from 0 to 25 with
+ 1 mA step. Default 20 mA.
+ This property is supported only for pm8941.
+
+- qcom,current-limit-microamp
+ Usage: optional
+ Value type: <u32>
+ Definition: uA; per-string current limit; value from 0 to 30000 with
+ 2500 uA step. Default 25 mA.
- qcom,current-boost-limit
Usage: optional
Value type: <u32>
Definition: mA; boost current limit.
For pm8941: one of: 105, 385, 525, 805, 980, 1260, 1400,
- 1680. Default: 805 mA
+ 1680. Default: 805 mA.
For pmi8998: one of: 105, 280, 450, 620, 970, 1150, 1300,
- 1500. Default: 970 mA
+ 1500. Default: 970 mA.
- qcom,switching-freq
Usage: optional
@@ -76,15 +80,62 @@ platforms. The PMIC is connected to the host processor via SPMI bus.
Usage: optional
Value type: <u32>
Definition: V; Over-voltage protection limit; one of:
- 27, 29, 32, 35. default: 29V
+ 27, 29, 32, 35. Default: 29V
This property is supported only for PM8941.
+- qcom,ovp-millivolt
+ Usage: optional
+ Value type: <u32>
+ Definition: mV; Over-voltage protection limit;
+ For pmi8998: one of 18100, 19600, 29600, 31100.
+ Default 29600 mV.
+ If this property is not specified for PM8941, it
+ falls back to "qcom,ovp" property.
+
- 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. Default: 2
+ For PMI8998 from 1 to 4.
+
+- interrupts
+ Usage: optional
+ Value type: <prop encoded array>
+ Definition: Interrupts associated with WLED. This should be
+ "short" and "ovp" interrupts. Interrupts can be
+ specified as per the encoding listed under
+ Documentation/devicetree/bindings/spmi/
+ qcom,spmi-pmic-arb.txt.
+
+- interrupt-names
+ Usage: optional
+ Value type: <string>
+ Definition: Interrupt names associated with the interrupts.
+ Must be "short" and "ovp". The short circuit detection
+ is not supported for PM8941.
+
+- 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.
+
+- qcom,external-pfet
+ Usage: optional
+ Value type: <bool>
+ Definition: Specify if external PFET control for short circuit
+ protection is used. This property is supported only
+ for PMI8998.
+
+- qcom,auto-string-detection
+ Usage: optional
+ Value type: <bool>
+ Definition: Enables auto-detection of the WLED string configuration.
+ This feature is not supported for PM8941.
+
Example:
@@ -99,4 +150,5 @@ pm8941-wled@d800 {
qcom,switching-freq = <1600>;
qcom,ovp = <29>;
qcom,num-strings = <2>;
+ qcom,enabled-strings = <0 1>;
};
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH V7 2/6] backlight: qcom-wled: Rename PM8941* to WLED3.
2019-10-16 10:13 [PATCH V7 0/6] backlight: qcom-wled: Support for QCOM wled driver Kiran Gunda
2019-10-16 10:13 ` [PATCH V7 1/6] backlight: qcom-wled: Add new properties for PMI8998 Kiran Gunda
@ 2019-10-16 10:13 ` Kiran Gunda
2019-10-16 10:13 ` [PATCH V7 3/6] backlight: qcom-wled: Restructure the driver for WLED3 Kiran Gunda
` (3 subsequent siblings)
5 siblings, 0 replies; 19+ messages in thread
From: Kiran Gunda @ 2019-10-16 10:13 UTC (permalink / raw)
To: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
daniel.thompson, jacek.anaszewski, pavel, robh+dt, mark.rutland,
linux-leds, devicetree, linux-kernel, Andy Gross, linux-arm-msm,
linux-fbdev
Cc: Kiran Gunda
Rename the PM8941* references as WLED3 to make the driver
generic and have WLED support for other PMICs. Also rename
"i_boost_limit" and "i_limit" variables to "boost_i_limit"
and "string_i_limit" respectively to resemble the corresponding
register names.
Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
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 82b8572..f191242 100644
--- a/drivers/video/backlight/qcom-wled.c
+++ b/drivers/video/backlight/qcom-wled.c
@@ -10,77 +10,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;
@@ -92,11 +94,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;
@@ -104,89 +106,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;
}
@@ -194,9 +196,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,
@@ -205,55 +207,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;
@@ -264,9 +266,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;
@@ -276,32 +278,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 {
@@ -324,7 +326,7 @@ static int pm8941_wled_configure(struct pm8941_wled *wled, struct device *dev)
if (rc)
wled->name = devm_kasprintf(dev, GFP_KERNEL, "%pOFn", dev->of_node);
- *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) {
@@ -336,7 +338,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);
@@ -358,15 +360,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;
@@ -383,42 +385,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
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH V7 3/6] backlight: qcom-wled: Restructure the driver for WLED3
2019-10-16 10:13 [PATCH V7 0/6] backlight: qcom-wled: Support for QCOM wled driver Kiran Gunda
2019-10-16 10:13 ` [PATCH V7 1/6] backlight: qcom-wled: Add new properties for PMI8998 Kiran Gunda
2019-10-16 10:13 ` [PATCH V7 2/6] backlight: qcom-wled: Rename PM8941* to WLED3 Kiran Gunda
@ 2019-10-16 10:13 ` Kiran Gunda
2019-10-16 10:13 ` [PATCH V7 4/6] backlight: qcom-wled: Add support for WLED4 peripheral Kiran Gunda
` (2 subsequent siblings)
5 siblings, 0 replies; 19+ messages in thread
From: Kiran Gunda @ 2019-10-16 10:13 UTC (permalink / raw)
To: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
daniel.thompson, jacek.anaszewski, pavel, robh+dt, mark.rutland,
linux-leds, devicetree, linux-kernel, Andy Gross, linux-arm-msm,
linux-fbdev
Cc: Kiran Gunda
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>
---
drivers/video/backlight/qcom-wled.c | 373 ++++++++++++++++++++++--------------
1 file changed, 234 insertions(+), 139 deletions(-)
diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c
index f191242..45eeda4 100644
--- a/drivers/video/backlight/qcom-wled.c
+++ b/drivers/video/backlight/qcom-wled.c
@@ -7,59 +7,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
/* 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 WLED3_CTRL_REG_MOD_EN_SHIFT 7
#define WLED3_CTRL_REG_FREQ 0x4c
-#define WLED3_CTRL_REG_FREQ_MASK 0x0f
+#define WLED3_CTRL_REG_FREQ_MASK GENMASK(3, 0)
#define WLED3_CTRL_REG_OVP 0x4d
-#define WLED3_CTRL_REG_OVP_MASK 0x03
+#define WLED3_CTRL_REG_OVP_MASK GENMASK(1, 0)
#define WLED3_CTRL_REG_ILIMIT 0x4e
-#define WLED3_CTRL_REG_ILIMIT_MASK 0x07
+#define WLED3_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
#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;
@@ -67,132 +79,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 +
+ WLED3_CTRL_REG_MOD_EN,
+ WLED3_CTRL_REG_MOD_EN_MASK,
+ val << WLED3_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 + WLED3_SINK_REG_SYNC,
+ mask, mask);
+ if (rc < 0)
+ return rc;
+
+ rc = regmap_update_bits(wled->regmap,
+ wled->ctrl_addr + WLED3_SINK_REG_SYNC,
+ mask, WLED3_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 + WLED3_CTRL_REG_OVP,
+ WLED3_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 + 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 + WLED3_CTRL_REG_FREQ,
- WLED3_CTRL_REG_FREQ_MASK, wled->cfg.switch_freq);
+ wled->ctrl_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)
- << 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;
}
@@ -200,17 +259,12 @@ 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,
+ .enabled_strings = {0, 1, 2, 3},
};
static const u32 wled3_boost_i_limit_values[] = {
@@ -255,7 +309,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;
@@ -266,68 +324,75 @@ static u32 wled3_values(const struct wled_var_cfg *cfg, u32 idx)
return idx;
}
-static int wled_configure(struct wled *wled, struct device *dev)
+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 = devm_kasprintf(dev, GFP_KERNEL, "%pOFn", dev->of_node);
- *cfg = wled3_config_defaults;
- for (i = 0; i < ARRAY_SIZE(u32_opts); ++i) {
+ switch (version) {
+ case 3:
+ 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");
+ return -EINVAL;
+ }
+
+ for (i = 0; i < size; ++i) {
rc = of_property_read_u32(dev->of_node, u32_opts[i].name, &val);
if (rc == -EINVAL) {
continue;
@@ -338,12 +403,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);
@@ -357,6 +425,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)
+ of_property_read_u32_array(dev->of_node,
+ "qcom,enabled-strings",
+ wled->cfg.enabled_strings,
+ sizeof(u32));
+
return 0;
}
@@ -370,6 +447,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;
@@ -384,15 +462,32 @@ 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 3:
+ 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);
@@ -407,7 +502,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);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH V7 4/6] backlight: qcom-wled: Add support for WLED4 peripheral.
2019-10-16 10:13 [PATCH V7 0/6] backlight: qcom-wled: Support for QCOM wled driver Kiran Gunda
` (2 preceding siblings ...)
2019-10-16 10:13 ` [PATCH V7 3/6] backlight: qcom-wled: Restructure the driver for WLED3 Kiran Gunda
@ 2019-10-16 10:13 ` Kiran Gunda
2019-10-17 11:06 ` Daniel Thompson
2019-10-16 10:13 ` [PATCH V7 5/6] backlight: qcom-wled: add support for short circuit handling Kiran Gunda
2019-10-16 10:13 ` [PATCH V7 6/6] backlight: qcom-wled: Add auto string detection logic Kiran Gunda
5 siblings, 1 reply; 19+ messages in thread
From: Kiran Gunda @ 2019-10-16 10:13 UTC (permalink / raw)
To: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
daniel.thompson, jacek.anaszewski, pavel, robh+dt, mark.rutland,
linux-leds, devicetree, linux-kernel, Andy Gross, linux-arm-msm,
linux-fbdev
Cc: Kiran Gunda
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>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
drivers/video/backlight/qcom-wled.c | 263 +++++++++++++++++++++++++++++++++++-
1 file changed, 257 insertions(+), 6 deletions(-)
diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c
index 45eeda4..2807b4b 100644
--- a/drivers/video/backlight/qcom-wled.c
+++ b/drivers/video/backlight/qcom-wled.c
@@ -17,7 +17,7 @@
#define WLED3_SINK_REG_BRIGHT_MAX 0xFFF
-/* WLED3 control registers */
+/* WLED3/WLED4 control registers */
#define WLED3_CTRL_REG_MOD_EN 0x46
#define WLED3_CTRL_REG_MOD_EN_MASK BIT(7)
#define WLED3_CTRL_REG_MOD_EN_SHIFT 7
@@ -31,7 +31,7 @@
#define WLED3_CTRL_REG_ILIMIT 0x4e
#define WLED3_CTRL_REG_ILIMIT_MASK GENMASK(2, 0)
-/* WLED3 sink registers */
+/* WLED3/WLED4 sink registers */
#define WLED3_SINK_REG_SYNC 0x47
#define WLED3_SINK_REG_SYNC_CLEAR 0x00
@@ -56,6 +56,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);
@@ -90,6 +112,7 @@ struct wled {
struct device *dev;
struct regmap *regmap;
u16 ctrl_addr;
+ u16 sink_addr;
u16 max_string_count;
u32 brightness;
u32 max_brightness;
@@ -116,6 +139,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;
@@ -267,6 +313,120 @@ static int wled3_setup(struct wled *wled)
.enabled_strings = {0, 1, 2, 3},
};
+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 + WLED3_CTRL_REG_OVP,
+ WLED3_CTRL_REG_OVP_MASK, wled->cfg.ovp);
+ if (rc < 0)
+ return rc;
+
+ rc = regmap_update_bits(wled->regmap,
+ wled->ctrl_addr + WLED3_CTRL_REG_ILIMIT,
+ WLED3_CTRL_REG_ILIMIT_MASK,
+ wled->cfg.boost_i_limit);
+ if (rc < 0)
+ return rc;
+
+ rc = regmap_update_bits(wled->regmap,
+ wled->ctrl_addr + WLED3_CTRL_REG_FREQ,
+ WLED3_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 +
+ WLED3_CTRL_REG_MOD_EN,
+ WLED3_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 +
+ WLED3_CTRL_REG_MOD_EN,
+ WLED3_CTRL_REG_MOD_EN_MASK,
+ WLED3_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,
};
@@ -276,6 +436,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,
};
@@ -285,6 +454,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;
@@ -295,6 +473,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));
@@ -309,10 +492,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)
@@ -361,6 +558,34 @@ static int wled_configure(struct wled *wled, int version)
},
};
+ const struct wled_u32_opts wled4_opts[] = {
+ {
+ .name = "qcom,current-boost-limit",
+ .val_ptr = &cfg->boost_i_limit,
+ .cfg = &wled4_boost_i_limit_cfg,
+ },
+ {
+ .name = "qcom,current-limit-microamp",
+ .val_ptr = &cfg->string_i_limit,
+ .cfg = &wled4_string_i_limit_cfg,
+ },
+ {
+ .name = "qcom,ovp-millivolt",
+ .val_ptr = &cfg->ovp,
+ .cfg = &wled4_ovp_cfg,
+ },
+ {
+ .name = "qcom,switching-freq",
+ .val_ptr = &cfg->switch_freq,
+ .cfg = &wled3_switch_freq_cfg,
+ },
+ {
+ .name = "qcom,num-strings",
+ .val_ptr = &cfg->num_strings,
+ .cfg = &wled4_num_strings_cfg,
+ },
+ };
+
const struct wled_bool_opts bool_opts[] = {
{ "qcom,cs-out", &cfg->cs_out_en, },
{ "qcom,ext-gen", &cfg->ext_gen, },
@@ -374,10 +599,6 @@ static int wled_configure(struct wled *wled, int version)
}
wled->ctrl_addr = be32_to_cpu(*prop_addr);
- rc = of_property_read_string(dev->of_node, "label", &wled->name);
- if (rc)
- wled->name = devm_kasprintf(dev, GFP_KERNEL, "%pOFn", dev->of_node);
-
switch (version) {
case 3:
u32_opts = wled3_opts;
@@ -385,6 +606,22 @@ static int wled_configure(struct wled *wled, int version)
*cfg = wled3_config_defaults;
wled->wled_set_brightness = wled3_set_brightness;
wled->max_string_count = 3;
+ wled->sink_addr = wled->ctrl_addr;
+ break;
+
+ case 4:
+ u32_opts = wled4_opts;
+ size = ARRAY_SIZE(wled4_opts);
+ *cfg = wled4_config_defaults;
+ wled->wled_set_brightness = wled4_set_brightness;
+ wled->max_string_count = 4;
+
+ prop_addr = of_get_address(dev->of_node, 1, NULL, NULL);
+ if (!prop_addr) {
+ dev_err(wled->dev, "invalid IO resources\n");
+ return -EINVAL;
+ }
+ wled->sink_addr = be32_to_cpu(*prop_addr);
break;
default:
@@ -392,6 +629,10 @@ static int wled_configure(struct wled *wled, int version)
return -EINVAL;
}
+ rc = of_property_read_string(dev->of_node, "label", &wled->name);
+ if (rc)
+ wled->name = dev->of_node->name;
+
for (i = 0; i < size; ++i) {
rc = of_property_read_u32(dev->of_node, u32_opts[i].name, &val);
if (rc == -EINVAL) {
@@ -483,6 +724,14 @@ static int wled_probe(struct platform_device *pdev)
}
break;
+ case 4:
+ rc = wled4_setup(wled);
+ if (rc) {
+ dev_err(&pdev->dev, "wled4_setup failed\n");
+ return rc;
+ }
+ break;
+
default:
dev_err(wled->dev, "Invalid WLED version\n");
break;
@@ -503,6 +752,8 @@ static int wled_probe(struct platform_device *pdev)
static const struct of_device_id wled_match_table[] = {
{ .compatible = "qcom,pm8941-wled", .data = (void *)3 },
+ { .compatible = "qcom,pmi8998-wled", .data = (void *)4 },
+ { .compatible = "qcom,pm660l-wled", .data = (void *)4 },
{}
};
MODULE_DEVICE_TABLE(of, wled_match_table);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH V7 4/6] backlight: qcom-wled: Add support for WLED4 peripheral.
2019-10-16 10:13 ` [PATCH V7 4/6] backlight: qcom-wled: Add support for WLED4 peripheral Kiran Gunda
@ 2019-10-17 11:06 ` Daniel Thompson
2019-10-17 12:27 ` kgunda
0 siblings, 1 reply; 19+ messages in thread
From: Daniel Thompson @ 2019-10-17 11:06 UTC (permalink / raw)
To: Kiran Gunda
Cc: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
jacek.anaszewski, pavel, robh+dt, mark.rutland, linux-leds,
devicetree, linux-kernel, Andy Gross, linux-arm-msm, linux-fbdev
On Wed, Oct 16, 2019 at 03:43:44PM +0530, 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.
There is code buried in this patch that looks like it changes the name
that will be handed to the backlight sub-system.
It's purpose needs to be explained in the patch description (or the code
moved to a new patch).
Daniel.
>
> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> drivers/video/backlight/qcom-wled.c | 263 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 257 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c
> index 45eeda4..2807b4b 100644
> --- a/drivers/video/backlight/qcom-wled.c
> +++ b/drivers/video/backlight/qcom-wled.c
> @@ -17,7 +17,7 @@
>
> #define WLED3_SINK_REG_BRIGHT_MAX 0xFFF
>
> -/* WLED3 control registers */
> +/* WLED3/WLED4 control registers */
> #define WLED3_CTRL_REG_MOD_EN 0x46
> #define WLED3_CTRL_REG_MOD_EN_MASK BIT(7)
> #define WLED3_CTRL_REG_MOD_EN_SHIFT 7
> @@ -31,7 +31,7 @@
> #define WLED3_CTRL_REG_ILIMIT 0x4e
> #define WLED3_CTRL_REG_ILIMIT_MASK GENMASK(2, 0)
>
> -/* WLED3 sink registers */
> +/* WLED3/WLED4 sink registers */
> #define WLED3_SINK_REG_SYNC 0x47
> #define WLED3_SINK_REG_SYNC_CLEAR 0x00
>
> @@ -56,6 +56,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);
> @@ -90,6 +112,7 @@ struct wled {
> struct device *dev;
> struct regmap *regmap;
> u16 ctrl_addr;
> + u16 sink_addr;
> u16 max_string_count;
> u32 brightness;
> u32 max_brightness;
> @@ -116,6 +139,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;
> @@ -267,6 +313,120 @@ static int wled3_setup(struct wled *wled)
> .enabled_strings = {0, 1, 2, 3},
> };
>
> +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 + WLED3_CTRL_REG_OVP,
> + WLED3_CTRL_REG_OVP_MASK, wled->cfg.ovp);
> + if (rc < 0)
> + return rc;
> +
> + rc = regmap_update_bits(wled->regmap,
> + wled->ctrl_addr + WLED3_CTRL_REG_ILIMIT,
> + WLED3_CTRL_REG_ILIMIT_MASK,
> + wled->cfg.boost_i_limit);
> + if (rc < 0)
> + return rc;
> +
> + rc = regmap_update_bits(wled->regmap,
> + wled->ctrl_addr + WLED3_CTRL_REG_FREQ,
> + WLED3_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 +
> + WLED3_CTRL_REG_MOD_EN,
> + WLED3_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 +
> + WLED3_CTRL_REG_MOD_EN,
> + WLED3_CTRL_REG_MOD_EN_MASK,
> + WLED3_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,
> };
> @@ -276,6 +436,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,
> };
> @@ -285,6 +454,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;
> @@ -295,6 +473,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));
> @@ -309,10 +492,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)
> @@ -361,6 +558,34 @@ static int wled_configure(struct wled *wled, int version)
> },
> };
>
> + const struct wled_u32_opts wled4_opts[] = {
> + {
> + .name = "qcom,current-boost-limit",
> + .val_ptr = &cfg->boost_i_limit,
> + .cfg = &wled4_boost_i_limit_cfg,
> + },
> + {
> + .name = "qcom,current-limit-microamp",
> + .val_ptr = &cfg->string_i_limit,
> + .cfg = &wled4_string_i_limit_cfg,
> + },
> + {
> + .name = "qcom,ovp-millivolt",
> + .val_ptr = &cfg->ovp,
> + .cfg = &wled4_ovp_cfg,
> + },
> + {
> + .name = "qcom,switching-freq",
> + .val_ptr = &cfg->switch_freq,
> + .cfg = &wled3_switch_freq_cfg,
> + },
> + {
> + .name = "qcom,num-strings",
> + .val_ptr = &cfg->num_strings,
> + .cfg = &wled4_num_strings_cfg,
> + },
> + };
> +
> const struct wled_bool_opts bool_opts[] = {
> { "qcom,cs-out", &cfg->cs_out_en, },
> { "qcom,ext-gen", &cfg->ext_gen, },
> @@ -374,10 +599,6 @@ static int wled_configure(struct wled *wled, int version)
> }
> wled->ctrl_addr = be32_to_cpu(*prop_addr);
>
> - rc = of_property_read_string(dev->of_node, "label", &wled->name);
> - if (rc)
> - wled->name = devm_kasprintf(dev, GFP_KERNEL, "%pOFn", dev->of_node);
> -
> switch (version) {
> case 3:
> u32_opts = wled3_opts;
> @@ -385,6 +606,22 @@ static int wled_configure(struct wled *wled, int version)
> *cfg = wled3_config_defaults;
> wled->wled_set_brightness = wled3_set_brightness;
> wled->max_string_count = 3;
> + wled->sink_addr = wled->ctrl_addr;
> + break;
> +
> + case 4:
> + u32_opts = wled4_opts;
> + size = ARRAY_SIZE(wled4_opts);
> + *cfg = wled4_config_defaults;
> + wled->wled_set_brightness = wled4_set_brightness;
> + wled->max_string_count = 4;
> +
> + prop_addr = of_get_address(dev->of_node, 1, NULL, NULL);
> + if (!prop_addr) {
> + dev_err(wled->dev, "invalid IO resources\n");
> + return -EINVAL;
> + }
> + wled->sink_addr = be32_to_cpu(*prop_addr);
> break;
>
> default:
> @@ -392,6 +629,10 @@ static int wled_configure(struct wled *wled, int version)
> return -EINVAL;
> }
>
> + rc = of_property_read_string(dev->of_node, "label", &wled->name);
> + if (rc)
> + wled->name = dev->of_node->name;
> +
> for (i = 0; i < size; ++i) {
> rc = of_property_read_u32(dev->of_node, u32_opts[i].name, &val);
> if (rc == -EINVAL) {
> @@ -483,6 +724,14 @@ static int wled_probe(struct platform_device *pdev)
> }
> break;
>
> + case 4:
> + rc = wled4_setup(wled);
> + if (rc) {
> + dev_err(&pdev->dev, "wled4_setup failed\n");
> + return rc;
> + }
> + break;
> +
> default:
> dev_err(wled->dev, "Invalid WLED version\n");
> break;
> @@ -503,6 +752,8 @@ static int wled_probe(struct platform_device *pdev)
>
> static const struct of_device_id wled_match_table[] = {
> { .compatible = "qcom,pm8941-wled", .data = (void *)3 },
> + { .compatible = "qcom,pmi8998-wled", .data = (void *)4 },
> + { .compatible = "qcom,pm660l-wled", .data = (void *)4 },
> {}
> };
> MODULE_DEVICE_TABLE(of, wled_match_table);
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH V7 4/6] backlight: qcom-wled: Add support for WLED4 peripheral.
2019-10-17 11:06 ` Daniel Thompson
@ 2019-10-17 12:27 ` kgunda
0 siblings, 0 replies; 19+ messages in thread
From: kgunda @ 2019-10-17 12:27 UTC (permalink / raw)
To: Daniel Thompson
Cc: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
jacek.anaszewski, pavel, robh+dt, mark.rutland, linux-leds,
devicetree, linux-kernel, Andy Gross, linux-arm-msm, linux-fbdev
On 2019-10-17 16:36, Daniel Thompson wrote:
> On Wed, Oct 16, 2019 at 03:43:44PM +0530, 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.
>
> There is code buried in this patch that looks like it changes the name
> that will be handed to the backlight sub-system.
>
> It's purpose needs to be explained in the patch description (or the
> code
> moved to a new patch).
>
I will address it in next series.
>
> Daniel.
>
>>
>> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
>> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>> ---
>> drivers/video/backlight/qcom-wled.c | 263
>> +++++++++++++++++++++++++++++++++++-
>> 1 file changed, 257 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/video/backlight/qcom-wled.c
>> b/drivers/video/backlight/qcom-wled.c
>> index 45eeda4..2807b4b 100644
>> --- a/drivers/video/backlight/qcom-wled.c
>> +++ b/drivers/video/backlight/qcom-wled.c
>> @@ -17,7 +17,7 @@
>>
>> #define WLED3_SINK_REG_BRIGHT_MAX 0xFFF
>>
>> -/* WLED3 control registers */
>> +/* WLED3/WLED4 control registers */
>> #define WLED3_CTRL_REG_MOD_EN 0x46
>> #define WLED3_CTRL_REG_MOD_EN_MASK BIT(7)
>> #define WLED3_CTRL_REG_MOD_EN_SHIFT 7
>> @@ -31,7 +31,7 @@
>> #define WLED3_CTRL_REG_ILIMIT 0x4e
>> #define WLED3_CTRL_REG_ILIMIT_MASK GENMASK(2, 0)
>>
>> -/* WLED3 sink registers */
>> +/* WLED3/WLED4 sink registers */
>> #define WLED3_SINK_REG_SYNC 0x47
>> #define WLED3_SINK_REG_SYNC_CLEAR 0x00
>>
>> @@ -56,6 +56,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);
>> @@ -90,6 +112,7 @@ struct wled {
>> struct device *dev;
>> struct regmap *regmap;
>> u16 ctrl_addr;
>> + u16 sink_addr;
>> u16 max_string_count;
>> u32 brightness;
>> u32 max_brightness;
>> @@ -116,6 +139,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;
>> @@ -267,6 +313,120 @@ static int wled3_setup(struct wled *wled)
>> .enabled_strings = {0, 1, 2, 3},
>> };
>>
>> +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 + WLED3_CTRL_REG_OVP,
>> + WLED3_CTRL_REG_OVP_MASK, wled->cfg.ovp);
>> + if (rc < 0)
>> + return rc;
>> +
>> + rc = regmap_update_bits(wled->regmap,
>> + wled->ctrl_addr + WLED3_CTRL_REG_ILIMIT,
>> + WLED3_CTRL_REG_ILIMIT_MASK,
>> + wled->cfg.boost_i_limit);
>> + if (rc < 0)
>> + return rc;
>> +
>> + rc = regmap_update_bits(wled->regmap,
>> + wled->ctrl_addr + WLED3_CTRL_REG_FREQ,
>> + WLED3_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 +
>> + WLED3_CTRL_REG_MOD_EN,
>> + WLED3_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 +
>> + WLED3_CTRL_REG_MOD_EN,
>> + WLED3_CTRL_REG_MOD_EN_MASK,
>> + WLED3_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,
>> };
>> @@ -276,6 +436,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,
>> };
>> @@ -285,6 +454,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;
>> @@ -295,6 +473,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));
>> @@ -309,10 +492,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)
>> @@ -361,6 +558,34 @@ static int wled_configure(struct wled *wled, int
>> version)
>> },
>> };
>>
>> + const struct wled_u32_opts wled4_opts[] = {
>> + {
>> + .name = "qcom,current-boost-limit",
>> + .val_ptr = &cfg->boost_i_limit,
>> + .cfg = &wled4_boost_i_limit_cfg,
>> + },
>> + {
>> + .name = "qcom,current-limit-microamp",
>> + .val_ptr = &cfg->string_i_limit,
>> + .cfg = &wled4_string_i_limit_cfg,
>> + },
>> + {
>> + .name = "qcom,ovp-millivolt",
>> + .val_ptr = &cfg->ovp,
>> + .cfg = &wled4_ovp_cfg,
>> + },
>> + {
>> + .name = "qcom,switching-freq",
>> + .val_ptr = &cfg->switch_freq,
>> + .cfg = &wled3_switch_freq_cfg,
>> + },
>> + {
>> + .name = "qcom,num-strings",
>> + .val_ptr = &cfg->num_strings,
>> + .cfg = &wled4_num_strings_cfg,
>> + },
>> + };
>> +
>> const struct wled_bool_opts bool_opts[] = {
>> { "qcom,cs-out", &cfg->cs_out_en, },
>> { "qcom,ext-gen", &cfg->ext_gen, },
>> @@ -374,10 +599,6 @@ static int wled_configure(struct wled *wled, int
>> version)
>> }
>> wled->ctrl_addr = be32_to_cpu(*prop_addr);
>>
>> - rc = of_property_read_string(dev->of_node, "label", &wled->name);
>> - if (rc)
>> - wled->name = devm_kasprintf(dev, GFP_KERNEL, "%pOFn",
>> dev->of_node);
>> -
>> switch (version) {
>> case 3:
>> u32_opts = wled3_opts;
>> @@ -385,6 +606,22 @@ static int wled_configure(struct wled *wled, int
>> version)
>> *cfg = wled3_config_defaults;
>> wled->wled_set_brightness = wled3_set_brightness;
>> wled->max_string_count = 3;
>> + wled->sink_addr = wled->ctrl_addr;
>> + break;
>> +
>> + case 4:
>> + u32_opts = wled4_opts;
>> + size = ARRAY_SIZE(wled4_opts);
>> + *cfg = wled4_config_defaults;
>> + wled->wled_set_brightness = wled4_set_brightness;
>> + wled->max_string_count = 4;
>> +
>> + prop_addr = of_get_address(dev->of_node, 1, NULL, NULL);
>> + if (!prop_addr) {
>> + dev_err(wled->dev, "invalid IO resources\n");
>> + return -EINVAL;
>> + }
>> + wled->sink_addr = be32_to_cpu(*prop_addr);
>> break;
>>
>> default:
>> @@ -392,6 +629,10 @@ static int wled_configure(struct wled *wled, int
>> version)
>> return -EINVAL;
>> }
>>
>> + rc = of_property_read_string(dev->of_node, "label", &wled->name);
>> + if (rc)
>> + wled->name = dev->of_node->name;
>> +
>> for (i = 0; i < size; ++i) {
>> rc = of_property_read_u32(dev->of_node, u32_opts[i].name, &val);
>> if (rc == -EINVAL) {
>> @@ -483,6 +724,14 @@ static int wled_probe(struct platform_device
>> *pdev)
>> }
>> break;
>>
>> + case 4:
>> + rc = wled4_setup(wled);
>> + if (rc) {
>> + dev_err(&pdev->dev, "wled4_setup failed\n");
>> + return rc;
>> + }
>> + break;
>> +
>> default:
>> dev_err(wled->dev, "Invalid WLED version\n");
>> break;
>> @@ -503,6 +752,8 @@ static int wled_probe(struct platform_device
>> *pdev)
>>
>> static const struct of_device_id wled_match_table[] = {
>> { .compatible = "qcom,pm8941-wled", .data = (void *)3 },
>> + { .compatible = "qcom,pmi8998-wled", .data = (void *)4 },
>> + { .compatible = "qcom,pm660l-wled", .data = (void *)4 },
>> {}
>> };
>> MODULE_DEVICE_TABLE(of, wled_match_table);
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
>> Forum,
>> a Linux Foundation Collaborative Project
>>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH V7 5/6] backlight: qcom-wled: add support for short circuit handling.
2019-10-16 10:13 [PATCH V7 0/6] backlight: qcom-wled: Support for QCOM wled driver Kiran Gunda
` (3 preceding siblings ...)
2019-10-16 10:13 ` [PATCH V7 4/6] backlight: qcom-wled: Add support for WLED4 peripheral Kiran Gunda
@ 2019-10-16 10:13 ` Kiran Gunda
2019-10-17 11:09 ` Daniel Thompson
2019-10-16 10:13 ` [PATCH V7 6/6] backlight: qcom-wled: Add auto string detection logic Kiran Gunda
5 siblings, 1 reply; 19+ messages in thread
From: Kiran Gunda @ 2019-10-16 10:13 UTC (permalink / raw)
To: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
daniel.thompson, jacek.anaszewski, pavel, robh+dt, mark.rutland,
linux-leds, devicetree, linux-kernel, Andy Gross, linux-arm-msm,
linux-fbdev
Cc: Kiran Gunda
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>
---
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 2807b4b..b5b125c 100644
--- a/drivers/video/backlight/qcom-wled.c
+++ b/drivers/video/backlight/qcom-wled.c
@@ -2,6 +2,9 @@
/* Copyright (c) 2015, Sony Mobile Communications, AB.
*/
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/ktime.h>
#include <linux/kernel.h>
#include <linux/backlight.h>
#include <linux/module.h>
@@ -56,6 +59,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)
@@ -105,17 +118,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);
@@ -166,6 +185,9 @@ static int wled_module_enable(struct wled *wled, int val)
{
int rc;
+ if (wled->disabled_by_short)
+ return -ENXIO;
+
rc = regmap_update_bits(wled->regmap, wled->ctrl_addr +
WLED3_CTRL_REG_MOD_EN,
WLED3_CTRL_REG_MOD_EN_MASK,
@@ -202,18 +224,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;
}
}
@@ -221,15 +244,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 = 1;
+
+ if (wled->short_count > WLED_SHORT_CNT_MAX) {
+ dev_err(wled->dev, "Short trigged %d times, disabling WLED forever!\n",
+ wled->short_count);
+ wled->disabled_by_short = true;
+ goto unlock_mutex;
+ }
+
+ wled->last_short_event = ktime_get();
+
+ msleep(WLED_SHORT_DLY_MS);
+ rc = wled_module_enable(wled, true);
+ if (rc < 0)
+ dev_err(wled->dev, "wled enable failed rc:%d\n", rc);
+
+unlock_mutex:
+ mutex_unlock(&wled->lock);
+
+ return IRQ_HANDLED;
+}
+
static int wled3_setup(struct wled *wled)
{
u16 addr;
@@ -318,7 +387,7 @@ static int wled4_setup(struct wled *wled)
int rc, temp, i, j;
u16 addr;
u8 sink_en = 0;
- u32 sink_cfg = 0;
+ u32 sink_cfg;
rc = regmap_update_bits(wled->regmap,
wled->ctrl_addr + WLED3_CTRL_REG_OVP,
@@ -340,6 +409,21 @@ static int wled4_setup(struct wled *wled)
if (rc < 0)
return rc;
+ if (wled->cfg.external_pfet) {
+ /* Unlock the secure register access */
+ rc = regmap_write(wled->regmap, wled->ctrl_addr +
+ WLED4_CTRL_REG_SEC_ACCESS,
+ WLED4_CTRL_REG_SEC_UNLOCK);
+ if (rc < 0)
+ return rc;
+
+ rc = regmap_write(wled->regmap,
+ wled->ctrl_addr + WLED4_CTRL_REG_TEST1,
+ WLED4_CTRL_REG_TEST1_EXT_FET_DTEST2);
+ if (rc < 0)
+ return rc;
+ }
+
rc = regmap_read(wled->regmap, wled->sink_addr +
WLED4_SINK_REG_CURR_SINK, &sink_cfg);
if (rc < 0)
@@ -425,6 +509,7 @@ static int wled4_setup(struct wled *wled)
.num_strings = 4,
.switch_freq = 11,
.cabc = false,
+ .external_pfet = false,
};
static const u32 wled3_boost_i_limit_values[] = {
@@ -590,6 +675,7 @@ static int wled_configure(struct wled *wled, int version)
{ "qcom,cs-out", &cfg->cs_out_en, },
{ "qcom,ext-gen", &cfg->ext_gen, },
{ "qcom,cabc", &cfg->cabc, },
+ { "qcom,external-pfet", &cfg->external_pfet, },
};
prop_addr = of_get_address(dev->of_node, 0, NULL, NULL);
@@ -678,6 +764,38 @@ static int wled_configure(struct wled *wled, int version)
return 0;
}
+static int wled_configure_short_irq(struct wled *wled,
+ struct platform_device *pdev)
+{
+ int rc, short_irq;
+
+ if (!wled->has_short_detect)
+ return 0;
+
+ rc = regmap_update_bits(wled->regmap, wled->ctrl_addr +
+ WLED4_CTRL_REG_SHORT_PROTECT,
+ WLED4_CTRL_REG_SHORT_EN_MASK,
+ WLED4_CTRL_REG_SHORT_EN_MASK);
+ if (rc < 0)
+ return rc;
+
+ short_irq = platform_get_irq_byname(pdev, "short");
+ if (short_irq < 0) {
+ dev_dbg(&pdev->dev, "short irq is not used\n");
+ return 0;
+ }
+
+ rc = devm_request_threaded_irq(wled->dev, short_irq,
+ NULL, wled_short_irq_handler,
+ IRQF_ONESHOT,
+ "wled_short_irq", wled);
+ if (rc < 0)
+ dev_err(wled->dev, "Unable to request short_irq (err:%d)\n",
+ rc);
+
+ return rc;
+}
+
static const struct backlight_ops wled_ops = {
.update_status = wled_update_status,
};
@@ -711,6 +829,7 @@ static int wled_probe(struct platform_device *pdev)
return -ENODEV;
}
+ mutex_init(&wled->lock);
rc = wled_configure(wled, version);
if (rc)
return rc;
@@ -725,6 +844,7 @@ static int wled_probe(struct platform_device *pdev)
break;
case 4:
+ wled->has_short_detect = true;
rc = wled4_setup(wled);
if (rc) {
dev_err(&pdev->dev, "wled4_setup failed\n");
@@ -737,6 +857,10 @@ static int wled_probe(struct platform_device *pdev)
break;
}
+ rc = wled_configure_short_irq(wled, pdev);
+ if (rc < 0)
+ return rc;
+
val = WLED_DEFAULT_BRIGHTNESS;
of_property_read_u32(pdev->dev.of_node, "default-brightness", &val);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH V7 5/6] backlight: qcom-wled: add support for short circuit handling.
2019-10-16 10:13 ` [PATCH V7 5/6] backlight: qcom-wled: add support for short circuit handling Kiran Gunda
@ 2019-10-17 11:09 ` Daniel Thompson
2019-10-17 12:30 ` kgunda
0 siblings, 1 reply; 19+ messages in thread
From: Daniel Thompson @ 2019-10-17 11:09 UTC (permalink / raw)
To: Kiran Gunda
Cc: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
jacek.anaszewski, pavel, robh+dt, mark.rutland, linux-leds,
devicetree, linux-kernel, Andy Gross, linux-arm-msm, linux-fbdev
On Wed, Oct 16, 2019 at 03:43:45PM +0530, 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>
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
> ---
> 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 2807b4b..b5b125c 100644
> --- a/drivers/video/backlight/qcom-wled.c
> +++ b/drivers/video/backlight/qcom-wled.c
> @@ -2,6 +2,9 @@
> /* Copyright (c) 2015, Sony Mobile Communications, AB.
> */
>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/ktime.h>
> #include <linux/kernel.h>
> #include <linux/backlight.h>
> #include <linux/module.h>
> @@ -56,6 +59,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)
> @@ -105,17 +118,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);
> @@ -166,6 +185,9 @@ static int wled_module_enable(struct wled *wled, int val)
> {
> int rc;
>
> + if (wled->disabled_by_short)
> + return -ENXIO;
> +
> rc = regmap_update_bits(wled->regmap, wled->ctrl_addr +
> WLED3_CTRL_REG_MOD_EN,
> WLED3_CTRL_REG_MOD_EN_MASK,
> @@ -202,18 +224,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;
> }
> }
>
> @@ -221,15 +244,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 = 1;
> +
> + if (wled->short_count > WLED_SHORT_CNT_MAX) {
> + dev_err(wled->dev, "Short trigged %d times, disabling WLED forever!\n",
> + wled->short_count);
> + wled->disabled_by_short = true;
> + goto unlock_mutex;
> + }
> +
> + wled->last_short_event = ktime_get();
> +
> + msleep(WLED_SHORT_DLY_MS);
> + rc = wled_module_enable(wled, true);
> + if (rc < 0)
> + dev_err(wled->dev, "wled enable failed rc:%d\n", rc);
> +
> +unlock_mutex:
> + mutex_unlock(&wled->lock);
> +
> + return IRQ_HANDLED;
> +}
> +
> static int wled3_setup(struct wled *wled)
> {
> u16 addr;
> @@ -318,7 +387,7 @@ static int wled4_setup(struct wled *wled)
> int rc, temp, i, j;
> u16 addr;
> u8 sink_en = 0;
> - u32 sink_cfg = 0;
> + u32 sink_cfg;
>
> rc = regmap_update_bits(wled->regmap,
> wled->ctrl_addr + WLED3_CTRL_REG_OVP,
> @@ -340,6 +409,21 @@ static int wled4_setup(struct wled *wled)
> if (rc < 0)
> return rc;
>
> + if (wled->cfg.external_pfet) {
> + /* Unlock the secure register access */
> + rc = regmap_write(wled->regmap, wled->ctrl_addr +
> + WLED4_CTRL_REG_SEC_ACCESS,
> + WLED4_CTRL_REG_SEC_UNLOCK);
> + if (rc < 0)
> + return rc;
> +
> + rc = regmap_write(wled->regmap,
> + wled->ctrl_addr + WLED4_CTRL_REG_TEST1,
> + WLED4_CTRL_REG_TEST1_EXT_FET_DTEST2);
> + if (rc < 0)
> + return rc;
> + }
> +
> rc = regmap_read(wled->regmap, wled->sink_addr +
> WLED4_SINK_REG_CURR_SINK, &sink_cfg);
> if (rc < 0)
> @@ -425,6 +509,7 @@ static int wled4_setup(struct wled *wled)
> .num_strings = 4,
> .switch_freq = 11,
> .cabc = false,
> + .external_pfet = false,
> };
>
> static const u32 wled3_boost_i_limit_values[] = {
> @@ -590,6 +675,7 @@ static int wled_configure(struct wled *wled, int version)
> { "qcom,cs-out", &cfg->cs_out_en, },
> { "qcom,ext-gen", &cfg->ext_gen, },
> { "qcom,cabc", &cfg->cabc, },
> + { "qcom,external-pfet", &cfg->external_pfet, },
> };
>
> prop_addr = of_get_address(dev->of_node, 0, NULL, NULL);
> @@ -678,6 +764,38 @@ static int wled_configure(struct wled *wled, int version)
> return 0;
> }
>
> +static int wled_configure_short_irq(struct wled *wled,
> + struct platform_device *pdev)
> +{
> + int rc, short_irq;
> +
> + if (!wled->has_short_detect)
> + return 0;
> +
> + rc = regmap_update_bits(wled->regmap, wled->ctrl_addr +
> + WLED4_CTRL_REG_SHORT_PROTECT,
> + WLED4_CTRL_REG_SHORT_EN_MASK,
> + WLED4_CTRL_REG_SHORT_EN_MASK);
> + if (rc < 0)
> + return rc;
> +
> + short_irq = platform_get_irq_byname(pdev, "short");
> + if (short_irq < 0) {
> + dev_dbg(&pdev->dev, "short irq is not used\n");
> + return 0;
> + }
> +
> + rc = devm_request_threaded_irq(wled->dev, short_irq,
> + NULL, wled_short_irq_handler,
> + IRQF_ONESHOT,
> + "wled_short_irq", wled);
> + if (rc < 0)
> + dev_err(wled->dev, "Unable to request short_irq (err:%d)\n",
> + rc);
> +
> + return rc;
> +}
> +
> static const struct backlight_ops wled_ops = {
> .update_status = wled_update_status,
> };
> @@ -711,6 +829,7 @@ static int wled_probe(struct platform_device *pdev)
> return -ENODEV;
> }
>
> + mutex_init(&wled->lock);
> rc = wled_configure(wled, version);
> if (rc)
> return rc;
> @@ -725,6 +844,7 @@ static int wled_probe(struct platform_device *pdev)
> break;
>
> case 4:
> + wled->has_short_detect = true;
> rc = wled4_setup(wled);
> if (rc) {
> dev_err(&pdev->dev, "wled4_setup failed\n");
> @@ -737,6 +857,10 @@ static int wled_probe(struct platform_device *pdev)
> break;
> }
>
> + rc = wled_configure_short_irq(wled, pdev);
> + if (rc < 0)
> + return rc;
> +
> val = WLED_DEFAULT_BRIGHTNESS;
> of_property_read_u32(pdev->dev.of_node, "default-brightness", &val);
>
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH V7 5/6] backlight: qcom-wled: add support for short circuit handling.
2019-10-17 11:09 ` Daniel Thompson
@ 2019-10-17 12:30 ` kgunda
0 siblings, 0 replies; 19+ messages in thread
From: kgunda @ 2019-10-17 12:30 UTC (permalink / raw)
To: Daniel Thompson
Cc: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
jacek.anaszewski, pavel, robh+dt, mark.rutland, linux-leds,
devicetree, linux-kernel, Andy Gross, linux-arm-msm, linux-fbdev
On 2019-10-17 16:39, Daniel Thompson wrote:
> On Wed, Oct 16, 2019 at 03:43:45PM +0530, 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>
>
> Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
>
Thanks for that !
>> ---
>> 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 2807b4b..b5b125c 100644
>> --- a/drivers/video/backlight/qcom-wled.c
>> +++ b/drivers/video/backlight/qcom-wled.c
>> @@ -2,6 +2,9 @@
>> /* Copyright (c) 2015, Sony Mobile Communications, AB.
>> */
>>
>> +#include <linux/delay.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/ktime.h>
>> #include <linux/kernel.h>
>> #include <linux/backlight.h>
>> #include <linux/module.h>
>> @@ -56,6 +59,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)
>> @@ -105,17 +118,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);
>> @@ -166,6 +185,9 @@ static int wled_module_enable(struct wled *wled,
>> int val)
>> {
>> int rc;
>>
>> + if (wled->disabled_by_short)
>> + return -ENXIO;
>> +
>> rc = regmap_update_bits(wled->regmap, wled->ctrl_addr +
>> WLED3_CTRL_REG_MOD_EN,
>> WLED3_CTRL_REG_MOD_EN_MASK,
>> @@ -202,18 +224,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;
>> }
>> }
>>
>> @@ -221,15 +244,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 = 1;
>> +
>> + if (wled->short_count > WLED_SHORT_CNT_MAX) {
>> + dev_err(wled->dev, "Short trigged %d times, disabling WLED
>> forever!\n",
>> + wled->short_count);
>> + wled->disabled_by_short = true;
>> + goto unlock_mutex;
>> + }
>> +
>> + wled->last_short_event = ktime_get();
>> +
>> + msleep(WLED_SHORT_DLY_MS);
>> + rc = wled_module_enable(wled, true);
>> + if (rc < 0)
>> + dev_err(wled->dev, "wled enable failed rc:%d\n", rc);
>> +
>> +unlock_mutex:
>> + mutex_unlock(&wled->lock);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> static int wled3_setup(struct wled *wled)
>> {
>> u16 addr;
>> @@ -318,7 +387,7 @@ static int wled4_setup(struct wled *wled)
>> int rc, temp, i, j;
>> u16 addr;
>> u8 sink_en = 0;
>> - u32 sink_cfg = 0;
>> + u32 sink_cfg;
>>
>> rc = regmap_update_bits(wled->regmap,
>> wled->ctrl_addr + WLED3_CTRL_REG_OVP,
>> @@ -340,6 +409,21 @@ static int wled4_setup(struct wled *wled)
>> if (rc < 0)
>> return rc;
>>
>> + if (wled->cfg.external_pfet) {
>> + /* Unlock the secure register access */
>> + rc = regmap_write(wled->regmap, wled->ctrl_addr +
>> + WLED4_CTRL_REG_SEC_ACCESS,
>> + WLED4_CTRL_REG_SEC_UNLOCK);
>> + if (rc < 0)
>> + return rc;
>> +
>> + rc = regmap_write(wled->regmap,
>> + wled->ctrl_addr + WLED4_CTRL_REG_TEST1,
>> + WLED4_CTRL_REG_TEST1_EXT_FET_DTEST2);
>> + if (rc < 0)
>> + return rc;
>> + }
>> +
>> rc = regmap_read(wled->regmap, wled->sink_addr +
>> WLED4_SINK_REG_CURR_SINK, &sink_cfg);
>> if (rc < 0)
>> @@ -425,6 +509,7 @@ static int wled4_setup(struct wled *wled)
>> .num_strings = 4,
>> .switch_freq = 11,
>> .cabc = false,
>> + .external_pfet = false,
>> };
>>
>> static const u32 wled3_boost_i_limit_values[] = {
>> @@ -590,6 +675,7 @@ static int wled_configure(struct wled *wled, int
>> version)
>> { "qcom,cs-out", &cfg->cs_out_en, },
>> { "qcom,ext-gen", &cfg->ext_gen, },
>> { "qcom,cabc", &cfg->cabc, },
>> + { "qcom,external-pfet", &cfg->external_pfet, },
>> };
>>
>> prop_addr = of_get_address(dev->of_node, 0, NULL, NULL);
>> @@ -678,6 +764,38 @@ static int wled_configure(struct wled *wled, int
>> version)
>> return 0;
>> }
>>
>> +static int wled_configure_short_irq(struct wled *wled,
>> + struct platform_device *pdev)
>> +{
>> + int rc, short_irq;
>> +
>> + if (!wled->has_short_detect)
>> + return 0;
>> +
>> + rc = regmap_update_bits(wled->regmap, wled->ctrl_addr +
>> + WLED4_CTRL_REG_SHORT_PROTECT,
>> + WLED4_CTRL_REG_SHORT_EN_MASK,
>> + WLED4_CTRL_REG_SHORT_EN_MASK);
>> + if (rc < 0)
>> + return rc;
>> +
>> + short_irq = platform_get_irq_byname(pdev, "short");
>> + if (short_irq < 0) {
>> + dev_dbg(&pdev->dev, "short irq is not used\n");
>> + return 0;
>> + }
>> +
>> + rc = devm_request_threaded_irq(wled->dev, short_irq,
>> + NULL, wled_short_irq_handler,
>> + IRQF_ONESHOT,
>> + "wled_short_irq", wled);
>> + if (rc < 0)
>> + dev_err(wled->dev, "Unable to request short_irq (err:%d)\n",
>> + rc);
>> +
>> + return rc;
>> +}
>> +
>> static const struct backlight_ops wled_ops = {
>> .update_status = wled_update_status,
>> };
>> @@ -711,6 +829,7 @@ static int wled_probe(struct platform_device
>> *pdev)
>> return -ENODEV;
>> }
>>
>> + mutex_init(&wled->lock);
>> rc = wled_configure(wled, version);
>> if (rc)
>> return rc;
>> @@ -725,6 +844,7 @@ static int wled_probe(struct platform_device
>> *pdev)
>> break;
>>
>> case 4:
>> + wled->has_short_detect = true;
>> rc = wled4_setup(wled);
>> if (rc) {
>> dev_err(&pdev->dev, "wled4_setup failed\n");
>> @@ -737,6 +857,10 @@ static int wled_probe(struct platform_device
>> *pdev)
>> break;
>> }
>>
>> + rc = wled_configure_short_irq(wled, pdev);
>> + if (rc < 0)
>> + return rc;
>> +
>> val = WLED_DEFAULT_BRIGHTNESS;
>> of_property_read_u32(pdev->dev.of_node, "default-brightness", &val);
>>
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
>> Forum,
>> a Linux Foundation Collaborative Project
>>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH V7 6/6] backlight: qcom-wled: Add auto string detection logic
2019-10-16 10:13 [PATCH V7 0/6] backlight: qcom-wled: Support for QCOM wled driver Kiran Gunda
` (4 preceding siblings ...)
2019-10-16 10:13 ` [PATCH V7 5/6] backlight: qcom-wled: add support for short circuit handling Kiran Gunda
@ 2019-10-16 10:13 ` Kiran Gunda
2019-10-17 11:29 ` Daniel Thompson
2019-10-17 12:26 ` Lee Jones
5 siblings, 2 replies; 19+ messages in thread
From: Kiran Gunda @ 2019-10-16 10:13 UTC (permalink / raw)
To: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
daniel.thompson, jacek.anaszewski, pavel, robh+dt, mark.rutland,
linux-leds, devicetree, linux-kernel, Andy Gross, linux-arm-msm,
linux-fbdev
Cc: Kiran Gunda
The auto string detection algorithm checks if the current WLED
sink configuration is valid. It tries enabling every sink and
checks if the OVP fault is observed. Based on this information
it detects and enables the valid sink configuration.
Auto calibration will be triggered when the OVP fault interrupts
are seen frequently thereby it tries to fix the sink configuration.
The auto-detection also kicks in when the connected LED string
of the display-backlight malfunctions (because of damage) and
requires the damaged string to be turned off to prevent the
complete panel and/or board from being damaged.
Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
---
drivers/video/backlight/qcom-wled.c | 410 +++++++++++++++++++++++++++++++++++-
1 file changed, 404 insertions(+), 6 deletions(-)
diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c
index b5b125c..ff7c409 100644
--- a/drivers/video/backlight/qcom-wled.c
+++ b/drivers/video/backlight/qcom-wled.c
@@ -17,19 +17,29 @@
#define WLED_MAX_STRINGS 4
#define WLED_DEFAULT_BRIGHTNESS 2048
-
+#define WLED_SOFT_START_DLY_US 10000
#define WLED3_SINK_REG_BRIGHT_MAX 0xFFF
/* WLED3/WLED4 control registers */
+#define WLED3_CTRL_REG_FAULT_STATUS 0x08
+#define WLED3_CTRL_REG_ILIM_FAULT_BIT BIT(0)
+#define WLED3_CTRL_REG_OVP_FAULT_BIT BIT(1)
+#define WLED4_CTRL_REG_SC_FAULT_BIT BIT(2)
+
+#define WLED3_CTRL_REG_INT_RT_STS 0x10
+#define WLED3_CTRL_REG_OVP_FAULT_STATUS BIT(1)
+
#define WLED3_CTRL_REG_MOD_EN 0x46
#define WLED3_CTRL_REG_MOD_EN_MASK BIT(7)
#define WLED3_CTRL_REG_MOD_EN_SHIFT 7
+#define WLED3_CTRL_REG_FEEDBACK_CONTROL 0x48
+
#define WLED3_CTRL_REG_FREQ 0x4c
#define WLED3_CTRL_REG_FREQ_MASK GENMASK(3, 0)
#define WLED3_CTRL_REG_OVP 0x4d
-#define WLED3_CTRL_REG_OVP_MASK GENMASK(1, 0)
+#define WLED3_CTRL_REG_OVP_MASK GENMASK(1, 0)
#define WLED3_CTRL_REG_ILIMIT 0x4e
#define WLED3_CTRL_REG_ILIMIT_MASK GENMASK(2, 0)
@@ -119,6 +129,7 @@ struct wled_config {
bool ext_gen;
bool cabc;
bool external_pfet;
+ bool auto_detection_enabled;
};
struct wled {
@@ -127,16 +138,21 @@ struct wled {
struct regmap *regmap;
struct mutex lock; /* Lock to avoid race from thread irq handler */
ktime_t last_short_event;
+ ktime_t start_ovp_fault_time;
u16 ctrl_addr;
u16 sink_addr;
u16 max_string_count;
+ u16 auto_detection_ovp_count;
u32 brightness;
u32 max_brightness;
u32 short_count;
+ u32 auto_detect_count;
bool disabled_by_short;
bool has_short_detect;
+ int ovp_irq;
struct wled_config cfg;
+ struct delayed_work ovp_work;
int (*wled_set_brightness)(struct wled *wled, u16 brightness);
};
@@ -181,6 +197,13 @@ 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);
+ enable_irq(wled->ovp_irq);
+}
+
static int wled_module_enable(struct wled *wled, int val)
{
int rc;
@@ -192,7 +215,19 @@ static int wled_module_enable(struct wled *wled, int val)
WLED3_CTRL_REG_MOD_EN,
WLED3_CTRL_REG_MOD_EN_MASK,
val << WLED3_CTRL_REG_MOD_EN_SHIFT);
- return rc;
+ if (rc < 0)
+ return rc;
+
+ if (wled->ovp_irq > 0) {
+ if (val) {
+ schedule_delayed_work(&wled->ovp_work, HZ / 100);
+ } else {
+ if (!cancel_delayed_work_sync(&wled->ovp_work))
+ disable_irq(wled->ovp_irq);
+ }
+ }
+
+ return 0;
}
static int wled_sync_toggle(struct wled *wled)
@@ -299,6 +334,311 @@ static irqreturn_t wled_short_irq_handler(int irq, void *_wled)
return IRQ_HANDLED;
}
+#define AUTO_DETECT_BRIGHTNESS 200
+
+static void wled_auto_string_detection(struct wled *wled)
+{
+ int rc = 0, i;
+ u32 sink_config = 0, int_sts;
+ u8 sink_test = 0, sink_valid = 0, val;
+
+ /* read configured sink configuration */
+ rc = regmap_read(wled->regmap, wled->sink_addr +
+ WLED4_SINK_REG_CURR_SINK, &sink_config);
+ if (rc < 0) {
+ dev_err(wled->dev, "Failed to read SINK configuration rc=%d\n",
+ rc);
+ goto failed_detect;
+ }
+
+ /* disable the module before starting detection */
+ rc = regmap_update_bits(wled->regmap,
+ wled->ctrl_addr + WLED3_CTRL_REG_MOD_EN,
+ WLED3_CTRL_REG_MOD_EN_MASK, 0);
+ if (rc < 0) {
+ dev_err(wled->dev, "Failed to disable WLED module rc=%d\n", rc);
+ goto failed_detect;
+ }
+
+ /* set low brightness across all sinks */
+ rc = wled4_set_brightness(wled, AUTO_DETECT_BRIGHTNESS);
+ if (rc < 0) {
+ dev_err(wled->dev, "Failed to set brightness for auto detection rc=%d\n",
+ rc);
+ goto failed_detect;
+ }
+
+ if (wled->cfg.cabc) {
+ for (i = 0; i < wled->cfg.num_strings; i++) {
+ rc = regmap_update_bits(wled->regmap, wled->sink_addr +
+ WLED4_SINK_REG_STR_CABC(i),
+ WLED4_SINK_REG_STR_CABC_MASK,
+ 0);
+ if (rc < 0)
+ goto failed_detect;
+ }
+ }
+
+ /* disable all sinks */
+ rc = regmap_write(wled->regmap,
+ wled->sink_addr + WLED4_SINK_REG_CURR_SINK, 0);
+ if (rc < 0) {
+ dev_err(wled->dev, "Failed to disable all sinks rc=%d\n", rc);
+ goto failed_detect;
+ }
+
+ /* iterate through the strings one by one */
+ for (i = 0; i < wled->cfg.num_strings; i++) {
+ sink_test = BIT((WLED4_SINK_REG_CURR_SINK_SHFT + i));
+
+ /* Enable feedback control */
+ rc = regmap_write(wled->regmap, wled->ctrl_addr +
+ WLED3_CTRL_REG_FEEDBACK_CONTROL, i + 1);
+ if (rc < 0) {
+ dev_err(wled->dev, "Failed to enable feedback for SINK %d rc = %d\n",
+ i + 1, rc);
+ goto failed_detect;
+ }
+
+ /* enable the sink */
+ rc = regmap_write(wled->regmap, wled->sink_addr +
+ WLED4_SINK_REG_CURR_SINK, sink_test);
+ if (rc < 0) {
+ dev_err(wled->dev, "Failed to configure SINK %d rc=%d\n",
+ i + 1, rc);
+ goto failed_detect;
+ }
+
+ /* Enable the module */
+ rc = regmap_update_bits(wled->regmap, wled->ctrl_addr +
+ WLED3_CTRL_REG_MOD_EN,
+ WLED3_CTRL_REG_MOD_EN_MASK,
+ WLED3_CTRL_REG_MOD_EN_MASK);
+ if (rc < 0) {
+ dev_err(wled->dev, "Failed to enable WLED module rc=%d\n",
+ rc);
+ goto failed_detect;
+ }
+
+ usleep_range(WLED_SOFT_START_DLY_US,
+ WLED_SOFT_START_DLY_US + 1000);
+
+ rc = regmap_read(wled->regmap, wled->ctrl_addr +
+ WLED3_CTRL_REG_INT_RT_STS, &int_sts);
+ if (rc < 0) {
+ dev_err(wled->dev, "Error in reading WLED3_CTRL_INT_RT_STS rc=%d\n",
+ rc);
+ goto failed_detect;
+ }
+
+ if (int_sts & WLED3_CTRL_REG_OVP_FAULT_STATUS)
+ dev_dbg(wled->dev, "WLED OVP fault detected with SINK %d\n",
+ i + 1);
+ else
+ sink_valid |= sink_test;
+
+ /* Disable the module */
+ rc = regmap_update_bits(wled->regmap,
+ wled->ctrl_addr + WLED3_CTRL_REG_MOD_EN,
+ WLED3_CTRL_REG_MOD_EN_MASK, 0);
+ if (rc < 0) {
+ dev_err(wled->dev, "Failed to disable WLED module rc=%d\n",
+ rc);
+ goto failed_detect;
+ }
+ }
+
+ if (!sink_valid) {
+ dev_err(wled->dev, "No valid WLED sinks found\n");
+ wled->disabled_by_short = true;
+ goto failed_detect;
+ }
+
+ if (sink_valid == sink_config) {
+ dev_dbg(wled->dev, "WLED auto-detection complete, sink-config=%x OK!\n",
+ sink_config);
+ } else {
+ dev_warn(wled->dev, "New WLED string configuration found %x\n",
+ sink_valid);
+ sink_config = sink_valid;
+ }
+
+ /* write the new sink configuration */
+ rc = regmap_write(wled->regmap,
+ wled->sink_addr + WLED4_SINK_REG_CURR_SINK,
+ sink_config);
+ if (rc < 0) {
+ dev_err(wled->dev, "Failed to reconfigure the default sink rc=%d\n",
+ rc);
+ goto failed_detect;
+ }
+
+ /* Enable valid sinks */
+ for (i = 0; i < wled->cfg.num_strings; i++) {
+ if (wled->cfg.cabc) {
+ rc = regmap_update_bits(wled->regmap, wled->sink_addr +
+ WLED4_SINK_REG_STR_CABC(i),
+ WLED4_SINK_REG_STR_CABC_MASK,
+ WLED4_SINK_REG_STR_CABC_MASK);
+ if (rc < 0)
+ goto failed_detect;
+ }
+
+ if (sink_config & BIT(WLED4_SINK_REG_CURR_SINK_SHFT + i))
+ val = WLED4_SINK_REG_STR_MOD_MASK;
+ else
+ val = 0x0; /* disable modulator_en for unused sink */
+
+ rc = regmap_write(wled->regmap, wled->sink_addr +
+ WLED4_SINK_REG_STR_MOD_EN(i), val);
+ if (rc < 0) {
+ dev_err(wled->dev, "Failed to configure MODULATOR_EN rc=%d\n",
+ rc);
+ goto failed_detect;
+ }
+ }
+
+ /* restore the feedback setting */
+ rc = regmap_write(wled->regmap,
+ wled->ctrl_addr + WLED3_CTRL_REG_FEEDBACK_CONTROL, 0);
+ if (rc < 0) {
+ dev_err(wled->dev, "Failed to restore feedback setting rc=%d\n",
+ rc);
+ goto failed_detect;
+ }
+
+ /* restore brightness */
+ rc = wled4_set_brightness(wled, wled->brightness);
+ if (rc < 0) {
+ dev_err(wled->dev, "Failed to set brightness after auto detection rc=%d\n",
+ rc);
+ goto failed_detect;
+ }
+
+ rc = regmap_update_bits(wled->regmap,
+ wled->ctrl_addr + WLED3_CTRL_REG_MOD_EN,
+ WLED3_CTRL_REG_MOD_EN_MASK,
+ WLED3_CTRL_REG_MOD_EN_MASK);
+ if (rc < 0) {
+ dev_err(wled->dev, "Failed to enable WLED module rc=%d\n", rc);
+ goto failed_detect;
+ }
+
+failed_detect:
+ return;
+}
+
+#define WLED_AUTO_DETECT_OVP_COUNT 5
+#define WLED_AUTO_DETECT_CNT_DLY_US USEC_PER_SEC
+static bool wled_auto_detection_required(struct wled *wled)
+{
+ s64 elapsed_time_us;
+
+ if (!wled->cfg.auto_detection_enabled)
+ return false;
+
+ /*
+ * Check if the OVP fault was an occasional one
+ * or if it's firing continuously, the latter qualifies
+ * for an auto-detection check.
+ */
+ if (!wled->auto_detection_ovp_count) {
+ wled->start_ovp_fault_time = ktime_get();
+ wled->auto_detection_ovp_count++;
+ } else {
+ elapsed_time_us = ktime_us_delta(ktime_get(),
+ wled->start_ovp_fault_time);
+ if (elapsed_time_us > WLED_AUTO_DETECT_CNT_DLY_US)
+ wled->auto_detection_ovp_count = 0;
+ else
+ wled->auto_detection_ovp_count++;
+
+ if (wled->auto_detection_ovp_count >=
+ WLED_AUTO_DETECT_OVP_COUNT) {
+ wled->auto_detection_ovp_count = 0;
+ return true;
+ }
+ }
+
+ return false;
+}
+
+static int wled_auto_detection_at_init(struct wled *wled)
+{
+ int rc;
+ u32 fault_status, rt_status;
+
+ if (!wled->cfg.auto_detection_enabled)
+ return 0;
+
+ rc = regmap_read(wled->regmap,
+ wled->ctrl_addr + WLED3_CTRL_REG_INT_RT_STS,
+ &rt_status);
+ if (rc < 0) {
+ dev_err(wled->dev, "Failed to read RT status rc=%d\n", rc);
+ return rc;
+ }
+
+ rc = regmap_read(wled->regmap,
+ wled->ctrl_addr + WLED3_CTRL_REG_FAULT_STATUS,
+ &fault_status);
+ if (rc < 0) {
+ dev_err(wled->dev, "Failed to read fault status rc=%d\n", rc);
+ return rc;
+ }
+
+ if ((rt_status & WLED3_CTRL_REG_OVP_FAULT_STATUS) ||
+ (fault_status & WLED3_CTRL_REG_OVP_FAULT_BIT)) {
+ mutex_lock(&wled->lock);
+ wled_auto_string_detection(wled);
+ mutex_unlock(&wled->lock);
+ }
+
+ return rc;
+}
+
+static irqreturn_t wled_ovp_irq_handler(int irq, void *_wled)
+{
+ struct wled *wled = _wled;
+ int rc;
+ u32 int_sts, fault_sts;
+
+ rc = regmap_read(wled->regmap,
+ wled->ctrl_addr + WLED3_CTRL_REG_INT_RT_STS, &int_sts);
+ if (rc < 0) {
+ dev_err(wled->dev, "Error in reading WLED3_INT_RT_STS rc=%d\n",
+ rc);
+ return IRQ_HANDLED;
+ }
+
+ rc = regmap_read(wled->regmap, wled->ctrl_addr +
+ WLED3_CTRL_REG_FAULT_STATUS, &fault_sts);
+ if (rc < 0) {
+ dev_err(wled->dev, "Error in reading WLED_FAULT_STATUS rc=%d\n",
+ rc);
+ return IRQ_HANDLED;
+ }
+
+ if (fault_sts &
+ (WLED3_CTRL_REG_OVP_FAULT_BIT | WLED3_CTRL_REG_ILIM_FAULT_BIT))
+ dev_dbg(wled->dev, "WLED OVP fault detected, int_sts=%x fault_sts= %x\n",
+ int_sts, fault_sts);
+
+ if (fault_sts & WLED3_CTRL_REG_OVP_FAULT_BIT) {
+ mutex_lock(&wled->lock);
+ disable_irq_nosync(wled->ovp_irq);
+
+ if (wled_auto_detection_required(wled))
+ wled_auto_string_detection(wled);
+
+ enable_irq(wled->ovp_irq);
+
+ mutex_unlock(&wled->lock);
+ }
+
+ return IRQ_HANDLED;
+}
+
static int wled3_setup(struct wled *wled)
{
u16 addr;
@@ -435,8 +775,10 @@ static int wled4_setup(struct wled *wled)
sink_en |= 1 << temp;
}
- if (sink_cfg == sink_en)
- return 0;
+ if (sink_cfg == sink_en) {
+ rc = wled_auto_detection_at_init(wled);
+ return rc;
+ }
rc = regmap_update_bits(wled->regmap,
wled->sink_addr + WLED4_SINK_REG_CURR_SINK,
@@ -499,7 +841,9 @@ static int wled4_setup(struct wled *wled)
return rc;
}
- return 0;
+ rc = wled_auto_detection_at_init(wled);
+
+ return rc;
}
static const struct wled_config wled4_config_defaults = {
@@ -510,6 +854,7 @@ static int wled4_setup(struct wled *wled)
.switch_freq = 11,
.cabc = false,
.external_pfet = false,
+ .auto_detection_enabled = false,
};
static const u32 wled3_boost_i_limit_values[] = {
@@ -676,6 +1021,7 @@ static int wled_configure(struct wled *wled, int version)
{ "qcom,ext-gen", &cfg->ext_gen, },
{ "qcom,cabc", &cfg->cabc, },
{ "qcom,external-pfet", &cfg->external_pfet, },
+ { "qcom,auto-string-detection", &cfg->auto_detection_enabled, },
};
prop_addr = of_get_address(dev->of_node, 0, NULL, NULL);
@@ -796,6 +1142,40 @@ static int wled_configure_short_irq(struct wled *wled,
return rc;
}
+static int wled_configure_ovp_irq(struct wled *wled,
+ struct platform_device *pdev)
+{
+ int rc;
+ u32 val;
+
+ wled->ovp_irq = platform_get_irq_byname(pdev, "ovp");
+ if (wled->ovp_irq < 0) {
+ dev_dbg(&pdev->dev, "ovp irq is not used\n");
+ return 0;
+ }
+
+ rc = devm_request_threaded_irq(wled->dev, wled->ovp_irq, NULL,
+ wled_ovp_irq_handler, IRQF_ONESHOT,
+ "wled_ovp_irq", wled);
+ if (rc < 0) {
+ dev_err(wled->dev, "Unable to request ovp_irq (err:%d)\n",
+ rc);
+ wled->ovp_irq = 0;
+ return 0;
+ }
+
+ rc = regmap_read(wled->regmap, wled->ctrl_addr +
+ WLED3_CTRL_REG_MOD_EN, &val);
+ if (rc < 0)
+ return rc;
+
+ /* Keep OVP irq disabled until module is enabled */
+ if (!(val & WLED3_CTRL_REG_MOD_EN_MASK))
+ disable_irq(wled->ovp_irq);
+
+ return 0;
+}
+
static const struct backlight_ops wled_ops = {
.update_status = wled_update_status,
};
@@ -836,6 +1216,7 @@ static int wled_probe(struct platform_device *pdev)
switch (version) {
case 3:
+ wled->cfg.auto_detection_enabled = false;
rc = wled3_setup(wled);
if (rc) {
dev_err(&pdev->dev, "wled3_setup failed\n");
@@ -857,10 +1238,16 @@ static int wled_probe(struct platform_device *pdev)
break;
}
+ INIT_DELAYED_WORK(&wled->ovp_work, wled_ovp_work);
+
rc = wled_configure_short_irq(wled, pdev);
if (rc < 0)
return rc;
+ rc = wled_configure_ovp_irq(wled, pdev);
+ if (rc < 0)
+ return rc;
+
val = WLED_DEFAULT_BRIGHTNESS;
of_property_read_u32(pdev->dev.of_node, "default-brightness", &val);
@@ -874,6 +1261,16 @@ static int wled_probe(struct platform_device *pdev)
return PTR_ERR_OR_ZERO(bl);
};
+static int wled_remove(struct platform_device *pdev)
+{
+ struct wled *wled = dev_get_drvdata(&pdev->dev);
+
+ cancel_delayed_work_sync(&wled->ovp_work);
+ mutex_destroy(&wled->lock);
+
+ return 0;
+}
+
static const struct of_device_id wled_match_table[] = {
{ .compatible = "qcom,pm8941-wled", .data = (void *)3 },
{ .compatible = "qcom,pmi8998-wled", .data = (void *)4 },
@@ -884,6 +1281,7 @@ static int wled_probe(struct platform_device *pdev)
static struct platform_driver wled_driver = {
.probe = wled_probe,
+ .remove = wled_remove,
.driver = {
.name = "qcom,wled",
.of_match_table = wled_match_table,
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH V7 6/6] backlight: qcom-wled: Add auto string detection logic
2019-10-16 10:13 ` [PATCH V7 6/6] backlight: qcom-wled: Add auto string detection logic Kiran Gunda
@ 2019-10-17 11:29 ` Daniel Thompson
2019-10-17 12:17 ` kgunda
2019-10-17 12:26 ` Lee Jones
1 sibling, 1 reply; 19+ messages in thread
From: Daniel Thompson @ 2019-10-17 11:29 UTC (permalink / raw)
To: Kiran Gunda
Cc: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
jacek.anaszewski, pavel, robh+dt, mark.rutland, linux-leds,
devicetree, linux-kernel, Andy Gross, linux-arm-msm, linux-fbdev
On Wed, Oct 16, 2019 at 03:43:46PM +0530, Kiran Gunda wrote:
> The auto string detection algorithm checks if the current WLED
> sink configuration is valid. It tries enabling every sink and
> checks if the OVP fault is observed. Based on this information
> it detects and enables the valid sink configuration.
> Auto calibration will be triggered when the OVP fault interrupts
> are seen frequently thereby it tries to fix the sink configuration.
>
> The auto-detection also kicks in when the connected LED string
> of the display-backlight malfunctions (because of damage) and
> requires the damaged string to be turned off to prevent the
> complete panel and/or board from being damaged.
>
> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
It's a complex bit of code but I'm OK with it in principle. Everything
below is about small details and/or nitpicking.
> +static void wled_ovp_work(struct work_struct *work)
> +{
> + struct wled *wled = container_of(work,
> + struct wled, ovp_work.work);
> + enable_irq(wled->ovp_irq);
> +}
> +
A bit of commenting about why we have to wait 10ms before enabling the
OVP interrupt would be appreciated.
> +static irqreturn_t wled_ovp_irq_handler(int irq, void *_wled)
> +{
> + struct wled *wled = _wled;
> + int rc;
> + u32 int_sts, fault_sts;
> +
> + rc = regmap_read(wled->regmap,
> + wled->ctrl_addr + WLED3_CTRL_REG_INT_RT_STS, &int_sts);
> + if (rc < 0) {
> + dev_err(wled->dev, "Error in reading WLED3_INT_RT_STS rc=%d\n",
> + rc);
> + return IRQ_HANDLED;
> + }
> +
> + rc = regmap_read(wled->regmap, wled->ctrl_addr +
> + WLED3_CTRL_REG_FAULT_STATUS, &fault_sts);
> + if (rc < 0) {
> + dev_err(wled->dev, "Error in reading WLED_FAULT_STATUS rc=%d\n",
> + rc);
> + return IRQ_HANDLED;
> + }
> +
> + if (fault_sts &
> + (WLED3_CTRL_REG_OVP_FAULT_BIT | WLED3_CTRL_REG_ILIM_FAULT_BIT))
> + dev_dbg(wled->dev, "WLED OVP fault detected, int_sts=%x fault_sts= %x\n",
> + int_sts, fault_sts);
> +
> + if (fault_sts & WLED3_CTRL_REG_OVP_FAULT_BIT) {
> + mutex_lock(&wled->lock);
> + disable_irq_nosync(wled->ovp_irq);
We're currently running the threaded ISR for this irq. Do we really need
to disable it?
> +
> + if (wled_auto_detection_required(wled))
> + wled_auto_string_detection(wled);
> +
> + enable_irq(wled->ovp_irq);
> +
> + mutex_unlock(&wled->lock);
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
Snip.
> +static int wled_remove(struct platform_device *pdev)
> +{
> + struct wled *wled = dev_get_drvdata(&pdev->dev);
> +
> + cancel_delayed_work_sync(&wled->ovp_work);
> + mutex_destroy(&wled->lock);
Have the irq handlers been disabled at this point?
Also, if you want to destroy the mutex shouldn't that code be
introduced in the same patch that introduces the mutex?
> +
> + return 0;
> +}
Daniel.
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH V7 6/6] backlight: qcom-wled: Add auto string detection logic
2019-10-17 11:29 ` Daniel Thompson
@ 2019-10-17 12:17 ` kgunda
2019-10-17 13:39 ` Daniel Thompson
0 siblings, 1 reply; 19+ messages in thread
From: kgunda @ 2019-10-17 12:17 UTC (permalink / raw)
To: Daniel Thompson
Cc: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
jacek.anaszewski, pavel, robh+dt, mark.rutland, linux-leds,
devicetree, linux-kernel, Andy Gross, linux-arm-msm, linux-fbdev
On 2019-10-17 16:59, Daniel Thompson wrote:
> On Wed, Oct 16, 2019 at 03:43:46PM +0530, Kiran Gunda wrote:
>> The auto string detection algorithm checks if the current WLED
>> sink configuration is valid. It tries enabling every sink and
>> checks if the OVP fault is observed. Based on this information
>> it detects and enables the valid sink configuration.
>> Auto calibration will be triggered when the OVP fault interrupts
>> are seen frequently thereby it tries to fix the sink configuration.
>>
>> The auto-detection also kicks in when the connected LED string
>> of the display-backlight malfunctions (because of damage) and
>> requires the damaged string to be turned off to prevent the
>> complete panel and/or board from being damaged.
>>
>> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
>
> It's a complex bit of code but I'm OK with it in principle. Everything
> below is about small details and/or nitpicking.
>
>
>> +static void wled_ovp_work(struct work_struct *work)
>> +{
>> + struct wled *wled = container_of(work,
>> + struct wled, ovp_work.work);
>> + enable_irq(wled->ovp_irq);
>> +}
>> +
>
> A bit of commenting about why we have to wait 10ms before enabling the
> OVP interrupt would be appreciated.
>
>
Sure. Will add the comment in the next series.
>> +static irqreturn_t wled_ovp_irq_handler(int irq, void *_wled)
>> +{
>> + struct wled *wled = _wled;
>> + int rc;
>> + u32 int_sts, fault_sts;
>> +
>> + rc = regmap_read(wled->regmap,
>> + wled->ctrl_addr + WLED3_CTRL_REG_INT_RT_STS, &int_sts);
>> + if (rc < 0) {
>> + dev_err(wled->dev, "Error in reading WLED3_INT_RT_STS rc=%d\n",
>> + rc);
>> + return IRQ_HANDLED;
>> + }
>> +
>> + rc = regmap_read(wled->regmap, wled->ctrl_addr +
>> + WLED3_CTRL_REG_FAULT_STATUS, &fault_sts);
>> + if (rc < 0) {
>> + dev_err(wled->dev, "Error in reading WLED_FAULT_STATUS rc=%d\n",
>> + rc);
>> + return IRQ_HANDLED;
>> + }
>> +
>> + if (fault_sts &
>> + (WLED3_CTRL_REG_OVP_FAULT_BIT | WLED3_CTRL_REG_ILIM_FAULT_BIT))
>> + dev_dbg(wled->dev, "WLED OVP fault detected, int_sts=%x fault_sts=
>> %x\n",
>> + int_sts, fault_sts);
>> +
>> + if (fault_sts & WLED3_CTRL_REG_OVP_FAULT_BIT) {
>> + mutex_lock(&wled->lock);
>> + disable_irq_nosync(wled->ovp_irq);
>
> We're currently running the threaded ISR for this irq. Do we really
> need
> to disable it?
>
We need to disable this IRQ, during the auto string detection logic.
Because
in the auto string detection we configure the current sinks one by one
and check the
status register for the OVPs and set the right string configuration. We
enable it later after
the auto string detection is completed.
>> +
>> + if (wled_auto_detection_required(wled))
>> + wled_auto_string_detection(wled);
>> +
>> + enable_irq(wled->ovp_irq);
>> +
>> + mutex_unlock(&wled->lock);
>> + }
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>
> Snip.
>
>
>> +static int wled_remove(struct platform_device *pdev)
>> +{
>> + struct wled *wled = dev_get_drvdata(&pdev->dev);
>> +
>> + cancel_delayed_work_sync(&wled->ovp_work);
>> + mutex_destroy(&wled->lock);
>
> Have the irq handlers been disabled at this point?
>
Ok.. may not be. I will disable the irq's here in next series.
> Also, if you want to destroy the mutex shouldn't that code be
> introduced in the same patch that introduces the mutex?
Ok.. I will move it to the same patch where the mutex introduced in next
series.
>> +
>> + return 0;
>> +}
>
>
> Daniel.
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH V7 6/6] backlight: qcom-wled: Add auto string detection logic
2019-10-17 12:17 ` kgunda
@ 2019-10-17 13:39 ` Daniel Thompson
2019-10-18 6:42 ` kgunda
0 siblings, 1 reply; 19+ messages in thread
From: Daniel Thompson @ 2019-10-17 13:39 UTC (permalink / raw)
To: kgunda
Cc: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
jacek.anaszewski, pavel, robh+dt, mark.rutland, linux-leds,
devicetree, linux-kernel, Andy Gross, linux-arm-msm, linux-fbdev
On Thu, Oct 17, 2019 at 05:47:47PM +0530, kgunda@codeaurora.org wrote:
> On 2019-10-17 16:59, Daniel Thompson wrote:
> > On Wed, Oct 16, 2019 at 03:43:46PM +0530, Kiran Gunda wrote:
> > > The auto string detection algorithm checks if the current WLED
> > > sink configuration is valid. It tries enabling every sink and
> > > checks if the OVP fault is observed. Based on this information
> > > it detects and enables the valid sink configuration.
> > > Auto calibration will be triggered when the OVP fault interrupts
> > > are seen frequently thereby it tries to fix the sink configuration.
> > >
> > > The auto-detection also kicks in when the connected LED string
> > > of the display-backlight malfunctions (because of damage) and
> > > requires the damaged string to be turned off to prevent the
> > > complete panel and/or board from being damaged.
> > >
> > > Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
> >
> > It's a complex bit of code but I'm OK with it in principle. Everything
> > below is about small details and/or nitpicking.
> >
> >
> > > +static void wled_ovp_work(struct work_struct *work)
> > > +{
> > > + struct wled *wled = container_of(work,
> > > + struct wled, ovp_work.work);
> > > + enable_irq(wled->ovp_irq);
> > > +}
> > > +
> >
> > A bit of commenting about why we have to wait 10ms before enabling the
> > OVP interrupt would be appreciated.
> >
> >
> Sure. Will add the comment in the next series.
> > > +static irqreturn_t wled_ovp_irq_handler(int irq, void *_wled)
> > > +{
> > > + struct wled *wled = _wled;
> > > + int rc;
> > > + u32 int_sts, fault_sts;
> > > +
> > > + rc = regmap_read(wled->regmap,
> > > + wled->ctrl_addr + WLED3_CTRL_REG_INT_RT_STS, &int_sts);
> > > + if (rc < 0) {
> > > + dev_err(wled->dev, "Error in reading WLED3_INT_RT_STS rc=%d\n",
> > > + rc);
> > > + return IRQ_HANDLED;
> > > + }
> > > +
> > > + rc = regmap_read(wled->regmap, wled->ctrl_addr +
> > > + WLED3_CTRL_REG_FAULT_STATUS, &fault_sts);
> > > + if (rc < 0) {
> > > + dev_err(wled->dev, "Error in reading WLED_FAULT_STATUS rc=%d\n",
> > > + rc);
> > > + return IRQ_HANDLED;
> > > + }
> > > +
> > > + if (fault_sts &
> > > + (WLED3_CTRL_REG_OVP_FAULT_BIT | WLED3_CTRL_REG_ILIM_FAULT_BIT))
> > > + dev_dbg(wled->dev, "WLED OVP fault detected, int_sts=%x
> > > fault_sts= %x\n",
> > > + int_sts, fault_sts);
> > > +
> > > + if (fault_sts & WLED3_CTRL_REG_OVP_FAULT_BIT) {
> > > + mutex_lock(&wled->lock);
> > > + disable_irq_nosync(wled->ovp_irq);
> >
> > We're currently running the threaded ISR for this irq. Do we really need
> > to disable it?
> >
> We need to disable this IRQ, during the auto string detection logic. Because
> in the auto string detection we configure the current sinks one by one and
> check the
> status register for the OVPs and set the right string configuration. We
> enable it later after
> the auto string detection is completed.
This is a threaded oneshot interrupt handler. Why isn't the framework
masking sufficient for you here?
Daniel.
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH V7 6/6] backlight: qcom-wled: Add auto string detection logic
2019-10-17 13:39 ` Daniel Thompson
@ 2019-10-18 6:42 ` kgunda
0 siblings, 0 replies; 19+ messages in thread
From: kgunda @ 2019-10-18 6:42 UTC (permalink / raw)
To: Daniel Thompson
Cc: bjorn.andersson, jingoohan1, lee.jones, b.zolnierkie, dri-devel,
jacek.anaszewski, pavel, robh+dt, mark.rutland, linux-leds,
devicetree, linux-kernel, Andy Gross, linux-arm-msm, linux-fbdev,
linux-arm-msm-owner
On 2019-10-17 19:09, Daniel Thompson wrote:
> On Thu, Oct 17, 2019 at 05:47:47PM +0530, kgunda@codeaurora.org wrote:
>> On 2019-10-17 16:59, Daniel Thompson wrote:
>> > On Wed, Oct 16, 2019 at 03:43:46PM +0530, Kiran Gunda wrote:
>> > > The auto string detection algorithm checks if the current WLED
>> > > sink configuration is valid. It tries enabling every sink and
>> > > checks if the OVP fault is observed. Based on this information
>> > > it detects and enables the valid sink configuration.
>> > > Auto calibration will be triggered when the OVP fault interrupts
>> > > are seen frequently thereby it tries to fix the sink configuration.
>> > >
>> > > The auto-detection also kicks in when the connected LED string
>> > > of the display-backlight malfunctions (because of damage) and
>> > > requires the damaged string to be turned off to prevent the
>> > > complete panel and/or board from being damaged.
>> > >
>> > > Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
>> >
>> > It's a complex bit of code but I'm OK with it in principle. Everything
>> > below is about small details and/or nitpicking.
>> >
>> >
>> > > +static void wled_ovp_work(struct work_struct *work)
>> > > +{
>> > > + struct wled *wled = container_of(work,
>> > > + struct wled, ovp_work.work);
>> > > + enable_irq(wled->ovp_irq);
>> > > +}
>> > > +
>> >
>> > A bit of commenting about why we have to wait 10ms before enabling the
>> > OVP interrupt would be appreciated.
>> >
>> >
>> Sure. Will add the comment in the next series.
>> > > +static irqreturn_t wled_ovp_irq_handler(int irq, void *_wled)
>> > > +{
>> > > + struct wled *wled = _wled;
>> > > + int rc;
>> > > + u32 int_sts, fault_sts;
>> > > +
>> > > + rc = regmap_read(wled->regmap,
>> > > + wled->ctrl_addr + WLED3_CTRL_REG_INT_RT_STS, &int_sts);
>> > > + if (rc < 0) {
>> > > + dev_err(wled->dev, "Error in reading WLED3_INT_RT_STS rc=%d\n",
>> > > + rc);
>> > > + return IRQ_HANDLED;
>> > > + }
>> > > +
>> > > + rc = regmap_read(wled->regmap, wled->ctrl_addr +
>> > > + WLED3_CTRL_REG_FAULT_STATUS, &fault_sts);
>> > > + if (rc < 0) {
>> > > + dev_err(wled->dev, "Error in reading WLED_FAULT_STATUS rc=%d\n",
>> > > + rc);
>> > > + return IRQ_HANDLED;
>> > > + }
>> > > +
>> > > + if (fault_sts &
>> > > + (WLED3_CTRL_REG_OVP_FAULT_BIT | WLED3_CTRL_REG_ILIM_FAULT_BIT))
>> > > + dev_dbg(wled->dev, "WLED OVP fault detected, int_sts=%x
>> > > fault_sts= %x\n",
>> > > + int_sts, fault_sts);
>> > > +
>> > > + if (fault_sts & WLED3_CTRL_REG_OVP_FAULT_BIT) {
>> > > + mutex_lock(&wled->lock);
>> > > + disable_irq_nosync(wled->ovp_irq);
>> >
>> > We're currently running the threaded ISR for this irq. Do we really need
>> > to disable it?
>> >
>> We need to disable this IRQ, during the auto string detection logic.
>> Because
>> in the auto string detection we configure the current sinks one by one
>> and
>> check the
>> status register for the OVPs and set the right string configuration.
>> We
>> enable it later after
>> the auto string detection is completed.
>
> This is a threaded oneshot interrupt handler. Why isn't the framework
> masking sufficient for you here?
>
>
> Daniel.
Right .. I overlooked that it is a oneshot interrupt earlier.
I will address it in the next series.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V7 6/6] backlight: qcom-wled: Add auto string detection logic
2019-10-16 10:13 ` [PATCH V7 6/6] backlight: qcom-wled: Add auto string detection logic Kiran Gunda
2019-10-17 11:29 ` Daniel Thompson
@ 2019-10-17 12:26 ` Lee Jones
2019-10-17 13:09 ` kgunda
1 sibling, 1 reply; 19+ messages in thread
From: Lee Jones @ 2019-10-17 12:26 UTC (permalink / raw)
To: Kiran Gunda
Cc: bjorn.andersson, jingoohan1, b.zolnierkie, dri-devel,
daniel.thompson, jacek.anaszewski, pavel, robh+dt, mark.rutland,
linux-leds, devicetree, linux-kernel, Andy Gross, linux-arm-msm,
linux-fbdev
On Wed, 16 Oct 2019, Kiran Gunda wrote:
> The auto string detection algorithm checks if the current WLED
> sink configuration is valid. It tries enabling every sink and
> checks if the OVP fault is observed. Based on this information
> it detects and enables the valid sink configuration.
> Auto calibration will be triggered when the OVP fault interrupts
> are seen frequently thereby it tries to fix the sink configuration.
>
> The auto-detection also kicks in when the connected LED string
> of the display-backlight malfunctions (because of damage) and
> requires the damaged string to be turned off to prevent the
> complete panel and/or board from being damaged.
>
> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
> ---
> drivers/video/backlight/qcom-wled.c | 410 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 404 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c
> index b5b125c..ff7c409 100644
> --- a/drivers/video/backlight/qcom-wled.c
> +++ b/drivers/video/backlight/qcom-wled.c
[...]
> + /* iterate through the strings one by one */
Please use proper English in comments (less a full stop).
In this case, just s/iterate/Iterate/.
> + for (i = 0; i < wled->cfg.num_strings; i++) {
> + sink_test = BIT((WLED4_SINK_REG_CURR_SINK_SHFT + i));
> +
> + /* Enable feedback control */
> + rc = regmap_write(wled->regmap, wled->ctrl_addr +
> + WLED3_CTRL_REG_FEEDBACK_CONTROL, i + 1);
> + if (rc < 0) {
> + dev_err(wled->dev, "Failed to enable feedback for SINK %d rc = %d\n",
> + i + 1, rc);
> + goto failed_detect;
> + }
> +
> + /* enable the sink */
Here too. And everywhere else.
> + rc = regmap_write(wled->regmap, wled->sink_addr +
> + WLED4_SINK_REG_CURR_SINK, sink_test);
> + if (rc < 0) {
> + dev_err(wled->dev, "Failed to configure SINK %d rc=%d\n",
> + i + 1, rc);
> + goto failed_detect;
> + }
> +
> + /* Enable the module */
> + rc = regmap_update_bits(wled->regmap, wled->ctrl_addr +
> + WLED3_CTRL_REG_MOD_EN,
> + WLED3_CTRL_REG_MOD_EN_MASK,
> + WLED3_CTRL_REG_MOD_EN_MASK);
> + if (rc < 0) {
> + dev_err(wled->dev, "Failed to enable WLED module rc=%d\n",
> + rc);
> + goto failed_detect;
> + }
> +
> + usleep_range(WLED_SOFT_START_DLY_US,
> + WLED_SOFT_START_DLY_US + 1000);
> +
> + rc = regmap_read(wled->regmap, wled->ctrl_addr +
> + WLED3_CTRL_REG_INT_RT_STS, &int_sts);
> + if (rc < 0) {
> + dev_err(wled->dev, "Error in reading WLED3_CTRL_INT_RT_STS rc=%d\n",
> + rc);
> + goto failed_detect;
> + }
> +
> + if (int_sts & WLED3_CTRL_REG_OVP_FAULT_STATUS)
> + dev_dbg(wled->dev, "WLED OVP fault detected with SINK %d\n",
> + i + 1);
I haven't reviewed the whole patch, but this caught my eye.
I think this should be upgraded to dev_warn().
> + else
> + sink_valid |= sink_test;
> +
> + /* Disable the module */
> + rc = regmap_update_bits(wled->regmap,
> + wled->ctrl_addr + WLED3_CTRL_REG_MOD_EN,
> + WLED3_CTRL_REG_MOD_EN_MASK, 0);
> + if (rc < 0) {
> + dev_err(wled->dev, "Failed to disable WLED module rc=%d\n",
> + rc);
> + goto failed_detect;
> + }
> + }
> +
> + if (!sink_valid) {
> + dev_err(wled->dev, "No valid WLED sinks found\n");
> + wled->disabled_by_short = true;
> + goto failed_detect;
> + }
> +
> + if (sink_valid == sink_config) {
> + dev_dbg(wled->dev, "WLED auto-detection complete, sink-config=%x OK!\n",
> + sink_config);
Does this really need to be placed in the kernel log?
> + } else {
> + dev_warn(wled->dev, "New WLED string configuration found %x\n",
> + sink_valid);
Why would the user care about this? Is it not normal?
> + sink_config = sink_valid;
> + }
[...]
> +static irqreturn_t wled_ovp_irq_handler(int irq, void *_wled)
> +{
> + struct wled *wled = _wled;
> + int rc;
> + u32 int_sts, fault_sts;
> +
> + rc = regmap_read(wled->regmap,
> + wled->ctrl_addr + WLED3_CTRL_REG_INT_RT_STS, &int_sts);
> + if (rc < 0) {
> + dev_err(wled->dev, "Error in reading WLED3_INT_RT_STS rc=%d\n",
> + rc);
> + return IRQ_HANDLED;
> + }
> +
> + rc = regmap_read(wled->regmap, wled->ctrl_addr +
> + WLED3_CTRL_REG_FAULT_STATUS, &fault_sts);
> + if (rc < 0) {
> + dev_err(wled->dev, "Error in reading WLED_FAULT_STATUS rc=%d\n",
> + rc);
> + return IRQ_HANDLED;
> + }
> +
> + if (fault_sts &
> + (WLED3_CTRL_REG_OVP_FAULT_BIT | WLED3_CTRL_REG_ILIM_FAULT_BIT))
Break the line at the '|'.
> + dev_dbg(wled->dev, "WLED OVP fault detected, int_sts=%x fault_sts= %x\n",
> + int_sts, fault_sts);
dev_warn().
> + if (fault_sts & WLED3_CTRL_REG_OVP_FAULT_BIT) {
> + mutex_lock(&wled->lock);
> + disable_irq_nosync(wled->ovp_irq);
> +
> + if (wled_auto_detection_required(wled))
> + wled_auto_string_detection(wled);
> +
> + enable_irq(wled->ovp_irq);
> +
> + mutex_unlock(&wled->lock);
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> static int wled3_setup(struct wled *wled)
> {
> u16 addr;
> @@ -435,8 +775,10 @@ static int wled4_setup(struct wled *wled)
> sink_en |= 1 << temp;
> }
>
> - if (sink_cfg == sink_en)
> - return 0;
> + if (sink_cfg == sink_en) {
> + rc = wled_auto_detection_at_init(wled);
> + return rc;
> + }
>
> rc = regmap_update_bits(wled->regmap,
> wled->sink_addr + WLED4_SINK_REG_CURR_SINK,
> @@ -499,7 +841,9 @@ static int wled4_setup(struct wled *wled)
> return rc;
> }
>
> - return 0;
> + rc = wled_auto_detection_at_init(wled);
> +
> + return rc;
> }
>
> static const struct wled_config wled4_config_defaults = {
> @@ -510,6 +854,7 @@ static int wled4_setup(struct wled *wled)
> .switch_freq = 11,
> .cabc = false,
> .external_pfet = false,
> + .auto_detection_enabled = false,
> };
>
> static const u32 wled3_boost_i_limit_values[] = {
> @@ -676,6 +1021,7 @@ static int wled_configure(struct wled *wled, int version)
> { "qcom,ext-gen", &cfg->ext_gen, },
> { "qcom,cabc", &cfg->cabc, },
> { "qcom,external-pfet", &cfg->external_pfet, },
> + { "qcom,auto-string-detection", &cfg->auto_detection_enabled, },
> };
>
> prop_addr = of_get_address(dev->of_node, 0, NULL, NULL);
> @@ -796,6 +1142,40 @@ static int wled_configure_short_irq(struct wled *wled,
> return rc;
> }
>
> +static int wled_configure_ovp_irq(struct wled *wled,
> + struct platform_device *pdev)
> +{
> + int rc;
> + u32 val;
> +
> + wled->ovp_irq = platform_get_irq_byname(pdev, "ovp");
> + if (wled->ovp_irq < 0) {
> + dev_dbg(&pdev->dev, "ovp irq is not used\n");
I assume this is optional. What happens if no IRQ is provided?
If, for instance, polling mode is enabled, maybe something like this
would be better?
dev_warn(&pdev->dev, "No IRQ found, falling back to polling mode\n");
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH V7 6/6] backlight: qcom-wled: Add auto string detection logic
2019-10-17 12:26 ` Lee Jones
@ 2019-10-17 13:09 ` kgunda
2019-10-17 13:30 ` Lee Jones
0 siblings, 1 reply; 19+ messages in thread
From: kgunda @ 2019-10-17 13:09 UTC (permalink / raw)
To: Lee Jones
Cc: bjorn.andersson, jingoohan1, b.zolnierkie, dri-devel,
daniel.thompson, jacek.anaszewski, pavel, robh+dt, mark.rutland,
linux-leds, devicetree, linux-kernel, Andy Gross, linux-arm-msm,
linux-fbdev
On 2019-10-17 17:56, Lee Jones wrote:
> On Wed, 16 Oct 2019, Kiran Gunda wrote:
>
>> The auto string detection algorithm checks if the current WLED
>> sink configuration is valid. It tries enabling every sink and
>> checks if the OVP fault is observed. Based on this information
>> it detects and enables the valid sink configuration.
>> Auto calibration will be triggered when the OVP fault interrupts
>> are seen frequently thereby it tries to fix the sink configuration.
>>
>> The auto-detection also kicks in when the connected LED string
>> of the display-backlight malfunctions (because of damage) and
>> requires the damaged string to be turned off to prevent the
>> complete panel and/or board from being damaged.
>>
>> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
>> ---
>> drivers/video/backlight/qcom-wled.c | 410
>> +++++++++++++++++++++++++++++++++++-
>> 1 file changed, 404 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/video/backlight/qcom-wled.c
>> b/drivers/video/backlight/qcom-wled.c
>> index b5b125c..ff7c409 100644
>> --- a/drivers/video/backlight/qcom-wled.c
>> +++ b/drivers/video/backlight/qcom-wled.c
>
>
> [...]
>
>> + /* iterate through the strings one by one */
>
> Please use proper English in comments (less a full stop).
>
> In this case, just s/iterate/Iterate/.
>
Sorry for that ! will fix it in next series.
>> + for (i = 0; i < wled->cfg.num_strings; i++) {
>> + sink_test = BIT((WLED4_SINK_REG_CURR_SINK_SHFT + i));
>> +
>> + /* Enable feedback control */
>> + rc = regmap_write(wled->regmap, wled->ctrl_addr +
>> + WLED3_CTRL_REG_FEEDBACK_CONTROL, i + 1);
>> + if (rc < 0) {
>> + dev_err(wled->dev, "Failed to enable feedback for SINK %d rc =
>> %d\n",
>> + i + 1, rc);
>> + goto failed_detect;
>> + }
>> +
>> + /* enable the sink */
>
> Here too. And everywhere else.
>
Will fix it in next series.
>> + rc = regmap_write(wled->regmap, wled->sink_addr +
>> + WLED4_SINK_REG_CURR_SINK, sink_test);
>> + if (rc < 0) {
>> + dev_err(wled->dev, "Failed to configure SINK %d rc=%d\n",
>> + i + 1, rc);
>> + goto failed_detect;
>> + }
>> +
>> + /* Enable the module */
>> + rc = regmap_update_bits(wled->regmap, wled->ctrl_addr +
>> + WLED3_CTRL_REG_MOD_EN,
>> + WLED3_CTRL_REG_MOD_EN_MASK,
>> + WLED3_CTRL_REG_MOD_EN_MASK);
>> + if (rc < 0) {
>> + dev_err(wled->dev, "Failed to enable WLED module rc=%d\n",
>> + rc);
>> + goto failed_detect;
>> + }
>> +
>> + usleep_range(WLED_SOFT_START_DLY_US,
>> + WLED_SOFT_START_DLY_US + 1000);
>> +
>> + rc = regmap_read(wled->regmap, wled->ctrl_addr +
>> + WLED3_CTRL_REG_INT_RT_STS, &int_sts);
>> + if (rc < 0) {
>> + dev_err(wled->dev, "Error in reading WLED3_CTRL_INT_RT_STS
>> rc=%d\n",
>> + rc);
>> + goto failed_detect;
>> + }
>> +
>> + if (int_sts & WLED3_CTRL_REG_OVP_FAULT_STATUS)
>> + dev_dbg(wled->dev, "WLED OVP fault detected with SINK %d\n",
>> + i + 1);
>
> I haven't reviewed the whole patch, but this caught my eye.
>
> I think this should be upgraded to dev_warn().
>
Thought of keeping these messages silent, Because the string
configuration will be corrected in this
and informing it at end of the auto string detection.
>> + else
>> + sink_valid |= sink_test;
>> +
>> + /* Disable the module */
>> + rc = regmap_update_bits(wled->regmap,
>> + wled->ctrl_addr + WLED3_CTRL_REG_MOD_EN,
>> + WLED3_CTRL_REG_MOD_EN_MASK, 0);
>> + if (rc < 0) {
>> + dev_err(wled->dev, "Failed to disable WLED module rc=%d\n",
>> + rc);
>> + goto failed_detect;
>> + }
>> + }
>> +
>> + if (!sink_valid) {
>> + dev_err(wled->dev, "No valid WLED sinks found\n");
>> + wled->disabled_by_short = true;
>> + goto failed_detect;
>> + }
>> +
>> + if (sink_valid == sink_config) {
>> + dev_dbg(wled->dev, "WLED auto-detection complete, sink-config=%x
>> OK!\n",
>> + sink_config);
>
> Does this really need to be placed in the kernel log?
>
Ok. This can be removed. I will remove it in next series.
>> + } else {
>> + dev_warn(wled->dev, "New WLED string configuration found %x\n",
>> + sink_valid);
>
> Why would the user care about this? Is it not normal?
>
Actually, it comes here if the user provided string configuration in the
device tree is in-correct.
That's why just informing the user about the right string configuration,
after the auto string detection.
>> + sink_config = sink_valid;
>> + }
>
> [...]
>
>> +static irqreturn_t wled_ovp_irq_handler(int irq, void *_wled)
>> +{
>> + struct wled *wled = _wled;
>> + int rc;
>> + u32 int_sts, fault_sts;
>> +
>> + rc = regmap_read(wled->regmap,
>> + wled->ctrl_addr + WLED3_CTRL_REG_INT_RT_STS, &int_sts);
>> + if (rc < 0) {
>> + dev_err(wled->dev, "Error in reading WLED3_INT_RT_STS rc=%d\n",
>> + rc);
>> + return IRQ_HANDLED;
>> + }
>> +
>> + rc = regmap_read(wled->regmap, wled->ctrl_addr +
>> + WLED3_CTRL_REG_FAULT_STATUS, &fault_sts);
>> + if (rc < 0) {
>> + dev_err(wled->dev, "Error in reading WLED_FAULT_STATUS rc=%d\n",
>> + rc);
>> + return IRQ_HANDLED;
>> + }
>> +
>> + if (fault_sts &
>> + (WLED3_CTRL_REG_OVP_FAULT_BIT | WLED3_CTRL_REG_ILIM_FAULT_BIT))
>
> Break the line at the '|'.
>
Will do it next series.
>> + dev_dbg(wled->dev, "WLED OVP fault detected, int_sts=%x fault_sts=
>> %x\n",
>> + int_sts, fault_sts);
>
> dev_warn().
>
As said above, wanted to keep these messages silent and inform the right
string
configuration at the end of string detection.
>> + if (fault_sts & WLED3_CTRL_REG_OVP_FAULT_BIT) {
>> + mutex_lock(&wled->lock);
>> + disable_irq_nosync(wled->ovp_irq);
>> +
>> + if (wled_auto_detection_required(wled))
>> + wled_auto_string_detection(wled);
>> +
>> + enable_irq(wled->ovp_irq);
>> +
>> + mutex_unlock(&wled->lock);
>> + }
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> static int wled3_setup(struct wled *wled)
>> {
>> u16 addr;
>> @@ -435,8 +775,10 @@ static int wled4_setup(struct wled *wled)
>> sink_en |= 1 << temp;
>> }
>>
>> - if (sink_cfg == sink_en)
>> - return 0;
>> + if (sink_cfg == sink_en) {
>> + rc = wled_auto_detection_at_init(wled);
>> + return rc;
>> + }
>>
>> rc = regmap_update_bits(wled->regmap,
>> wled->sink_addr + WLED4_SINK_REG_CURR_SINK,
>> @@ -499,7 +841,9 @@ static int wled4_setup(struct wled *wled)
>> return rc;
>> }
>>
>> - return 0;
>> + rc = wled_auto_detection_at_init(wled);
>> +
>> + return rc;
>> }
>>
>> static const struct wled_config wled4_config_defaults = {
>> @@ -510,6 +854,7 @@ static int wled4_setup(struct wled *wled)
>> .switch_freq = 11,
>> .cabc = false,
>> .external_pfet = false,
>> + .auto_detection_enabled = false,
>> };
>>
>> static const u32 wled3_boost_i_limit_values[] = {
>> @@ -676,6 +1021,7 @@ static int wled_configure(struct wled *wled, int
>> version)
>> { "qcom,ext-gen", &cfg->ext_gen, },
>> { "qcom,cabc", &cfg->cabc, },
>> { "qcom,external-pfet", &cfg->external_pfet, },
>> + { "qcom,auto-string-detection", &cfg->auto_detection_enabled, },
>> };
>>
>> prop_addr = of_get_address(dev->of_node, 0, NULL, NULL);
>> @@ -796,6 +1142,40 @@ static int wled_configure_short_irq(struct wled
>> *wled,
>> return rc;
>> }
>>
>> +static int wled_configure_ovp_irq(struct wled *wled,
>> + struct platform_device *pdev)
>> +{
>> + int rc;
>> + u32 val;
>> +
>> + wled->ovp_irq = platform_get_irq_byname(pdev, "ovp");
>> + if (wled->ovp_irq < 0) {
>> + dev_dbg(&pdev->dev, "ovp irq is not used\n");
>
> I assume this is optional. What happens if no IRQ is provided?
>
Here OVP IRQ is used to detect the wrong string detection. If it is not
provided the auto string detection logic won't work.
> If, for instance, polling mode is enabled, maybe something like this
> would be better?
>
> dev_warn(&pdev->dev, "No IRQ found, falling back to polling
> mode\n");
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH V7 6/6] backlight: qcom-wled: Add auto string detection logic
2019-10-17 13:09 ` kgunda
@ 2019-10-17 13:30 ` Lee Jones
2019-10-18 5:03 ` kgunda
0 siblings, 1 reply; 19+ messages in thread
From: Lee Jones @ 2019-10-17 13:30 UTC (permalink / raw)
To: kgunda
Cc: bjorn.andersson, jingoohan1, b.zolnierkie, dri-devel,
daniel.thompson, jacek.anaszewski, pavel, robh+dt, mark.rutland,
linux-leds, devicetree, linux-kernel, Andy Gross, linux-arm-msm,
linux-fbdev
On Thu, 17 Oct 2019, kgunda@codeaurora.org wrote:
> On 2019-10-17 17:56, Lee Jones wrote:
> > On Wed, 16 Oct 2019, Kiran Gunda wrote:
> >
> > > The auto string detection algorithm checks if the current WLED
> > > sink configuration is valid. It tries enabling every sink and
> > > checks if the OVP fault is observed. Based on this information
> > > it detects and enables the valid sink configuration.
> > > Auto calibration will be triggered when the OVP fault interrupts
> > > are seen frequently thereby it tries to fix the sink configuration.
> > >
> > > The auto-detection also kicks in when the connected LED string
> > > of the display-backlight malfunctions (because of damage) and
> > > requires the damaged string to be turned off to prevent the
> > > complete panel and/or board from being damaged.
> > >
> > > Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
> > > ---
> > > drivers/video/backlight/qcom-wled.c | 410
> > > +++++++++++++++++++++++++++++++++++-
> > > 1 file changed, 404 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/video/backlight/qcom-wled.c
> > > b/drivers/video/backlight/qcom-wled.c
> > > index b5b125c..ff7c409 100644
> > > --- a/drivers/video/backlight/qcom-wled.c
> > > +++ b/drivers/video/backlight/qcom-wled.c
[...]
> > > + if (int_sts & WLED3_CTRL_REG_OVP_FAULT_STATUS)
> > > + dev_dbg(wled->dev, "WLED OVP fault detected with SINK %d\n",
> > > + i + 1);
> >
> > I haven't reviewed the whole patch, but this caught my eye.
> >
> > I think this should be upgraded to dev_warn().
> >
> Thought of keeping these messages silent, Because the string configuration
> will be corrected in this
> and informing it at end of the auto string detection.
[...]
> > > + } else {
> > > + dev_warn(wled->dev, "New WLED string configuration found %x\n",
> > > + sink_valid);
> >
> > Why would the user care about this? Is it not normal?
> >
> Actually, it comes here if the user provided string configuration in the
> device tree is in-correct.
> That's why just informing the user about the right string configuration,
> after the auto string detection.
Then I think we need to be more forthcoming. Tell the user the
configuration is incorrect and what you've done to rectify it.
"XXX is not a valid configuration - using YYY instead"
[...]
> > > +static int wled_configure_ovp_irq(struct wled *wled,
> > > + struct platform_device *pdev)
> > > +{
> > > + int rc;
> > > + u32 val;
> > > +
> > > + wled->ovp_irq = platform_get_irq_byname(pdev, "ovp");
> > > + if (wled->ovp_irq < 0) {
> > > + dev_dbg(&pdev->dev, "ovp irq is not used\n");
> >
> > I assume this is optional. What happens if no IRQ is provided?
> >
> Here OVP IRQ is used to detect the wrong string detection. If it is not
> provided the auto string detection logic won't work.
"OVP IRQ not found - disabling automatic string detection"
> > If, for instance, polling mode is enabled, maybe something like this
> > would be better?
> >
> > dev_warn(&pdev->dev, "No IRQ found, falling back to polling
> > mode\n");
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH V7 6/6] backlight: qcom-wled: Add auto string detection logic
2019-10-17 13:30 ` Lee Jones
@ 2019-10-18 5:03 ` kgunda
0 siblings, 0 replies; 19+ messages in thread
From: kgunda @ 2019-10-18 5:03 UTC (permalink / raw)
To: Lee Jones
Cc: bjorn.andersson, jingoohan1, b.zolnierkie, dri-devel,
daniel.thompson, jacek.anaszewski, pavel, robh+dt, mark.rutland,
linux-leds, devicetree, linux-kernel, Andy Gross, linux-arm-msm,
linux-fbdev, linux-arm-msm-owner
On 2019-10-17 19:00, Lee Jones wrote:
> On Thu, 17 Oct 2019, kgunda@codeaurora.org wrote:
>
>> On 2019-10-17 17:56, Lee Jones wrote:
>> > On Wed, 16 Oct 2019, Kiran Gunda wrote:
>> >
>> > > The auto string detection algorithm checks if the current WLED
>> > > sink configuration is valid. It tries enabling every sink and
>> > > checks if the OVP fault is observed. Based on this information
>> > > it detects and enables the valid sink configuration.
>> > > Auto calibration will be triggered when the OVP fault interrupts
>> > > are seen frequently thereby it tries to fix the sink configuration.
>> > >
>> > > The auto-detection also kicks in when the connected LED string
>> > > of the display-backlight malfunctions (because of damage) and
>> > > requires the damaged string to be turned off to prevent the
>> > > complete panel and/or board from being damaged.
>> > >
>> > > Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
>> > > ---
>> > > drivers/video/backlight/qcom-wled.c | 410
>> > > +++++++++++++++++++++++++++++++++++-
>> > > 1 file changed, 404 insertions(+), 6 deletions(-)
>> > >
>> > > diff --git a/drivers/video/backlight/qcom-wled.c
>> > > b/drivers/video/backlight/qcom-wled.c
>> > > index b5b125c..ff7c409 100644
>> > > --- a/drivers/video/backlight/qcom-wled.c
>> > > +++ b/drivers/video/backlight/qcom-wled.c
>
> [...]
>
>> > > + if (int_sts & WLED3_CTRL_REG_OVP_FAULT_STATUS)
>> > > + dev_dbg(wled->dev, "WLED OVP fault detected with SINK %d\n",
>> > > + i + 1);
>> >
>> > I haven't reviewed the whole patch, but this caught my eye.
>> >
>> > I think this should be upgraded to dev_warn().
>> >
>> Thought of keeping these messages silent, Because the string
>> configuration
>> will be corrected in this
>> and informing it at end of the auto string detection.
>
> [...]
>
>> > > + } else {
>> > > + dev_warn(wled->dev, "New WLED string configuration found %x\n",
>> > > + sink_valid);
>> >
>> > Why would the user care about this? Is it not normal?
>> >
>> Actually, it comes here if the user provided string configuration in
>> the
>> device tree is in-correct.
>> That's why just informing the user about the right string
>> configuration,
>> after the auto string detection.
>
> Then I think we need to be more forthcoming. Tell the user the
> configuration is incorrect and what you've done to rectify it.
>
> "XXX is not a valid configuration - using YYY instead"
>
Sure. Will update it in the next series.
> [...]
>
>> > > +static int wled_configure_ovp_irq(struct wled *wled,
>> > > + struct platform_device *pdev)
>> > > +{
>> > > + int rc;
>> > > + u32 val;
>> > > +
>> > > + wled->ovp_irq = platform_get_irq_byname(pdev, "ovp");
>> > > + if (wled->ovp_irq < 0) {
>> > > + dev_dbg(&pdev->dev, "ovp irq is not used\n");
>> >
>> > I assume this is optional. What happens if no IRQ is provided?
>> >
>> Here OVP IRQ is used to detect the wrong string detection. If it is
>> not
>> provided the auto string detection logic won't work.
>
> "OVP IRQ not found - disabling automatic string detection"
>
Sure. Will update it in the next series.
>> > If, for instance, polling mode is enabled, maybe something like this
>> > would be better?
>> >
>> > dev_warn(&pdev->dev, "No IRQ found, falling back to polling
>> > mode\n");
^ permalink raw reply [flat|nested] 19+ messages in thread