[PATCH V1 3/4] qcom: spmi-wled: Add support for OVP interrupt handling

Subsystems: arm/qualcomm mailing list, backlight class/subsystem, framebuffer layer, led subsystem, open firmware and flattened device tree bindings, the rest

STALE3036d

3 messages, 2 authors, 2017-12-11 · open the first message on its own page

[PATCH V1 3/4] qcom: spmi-wled: Add support for OVP interrupt handling

From: Kiran Gunda <hidden>
Date: 2017-11-16 12:20:12

WLED peripheral has over voltage protection(OVP) circuitry and the OVP
fault is notified through an interrupt. Though this fault condition rising
is due to an incorrect hardware configuration is mitigated in the hardware,
it still needs to be detected and handled. Add support for it.

When WLED module is enabled, keep OVP fault interrupt disabled for 10 ms to
account for soft start delay.

Signed-off-by: Kiran Gunda <redacted>
---
 .../bindings/leds/backlight/qcom-spmi-wled.txt     |  7 +-
 drivers/video/backlight/qcom-spmi-wled.c           | 83 ++++++++++++++++++++++
 2 files changed, 87 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt b/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt
index 768608c..d39ee93 100644
--- a/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt
+++ b/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt
@@ -92,7 +92,7 @@ The PMIC is connected to the host processor via SPMI bus.
 	Usage:      optional
 	Value type: <string>
 	Definition: Interrupt names associated with the interrupts.
-		    Must be "sc-irq".
+		    Currently supported interrupts are "sc-irq" and "ovp-irq".
 
 Example:
 
@@ -102,8 +102,9 @@ qcom-wled@d800 {
 	reg-names = "qcom-wled-ctrl-base", "qcom-wled-sink-base";
 	label = "backlight";
 
-	interrupts = <0x3 0xd8 0x2 IRQ_TYPE_EDGE_RISING>;
-	interrupt-names = "sc-irq";
+	interrupts = <0x3 0xd8 0x2 IRQ_TYPE_EDGE_RISING>,
+			<0x3 0xd8 0x1 IRQ_TYPE_EDGE_RISING>;
+	interrupt-names = "sc-irq", "ovp-irq";
 	qcom,fs-current-limit = <25000>;
 	qcom,current-boost-limit = <970>;
 	qcom,switching-freq = <800>;
diff --git a/drivers/video/backlight/qcom-spmi-wled.c b/drivers/video/backlight/qcom-spmi-wled.c
index 7dbaaa7..8b2a77a 100644
--- a/drivers/video/backlight/qcom-spmi-wled.c
+++ b/drivers/video/backlight/qcom-spmi-wled.c
@@ -29,9 +29,15 @@
 #define QCOM_WLED_SC_DLY_MS			20
 #define QCOM_WLED_SC_CNT_MAX			5
 #define QCOM_WLED_SC_RESET_CNT_DLY_US		1000000
+#define QCOM_WLED_SOFT_START_DLY_US		10000
 
 /* WLED control registers */
 #define QCOM_WLED_CTRL_FAULT_STATUS		0x08
+#define  QCOM_WLED_CTRL_ILIM_FAULT_BIT		BIT(0)
+#define  QCOM_WLED_CTRL_OVP_FAULT_BIT		BIT(1)
+#define  QCOM_WLED_CTRL_SC_FAULT_BIT		BIT(2)
+
+#define QCOM_WLED_CTRL_INT_RT_STS		0x10
 
 #define QCOM_WLED_CTRL_MOD_ENABLE		0x46
 #define  QCOM_WLED_CTRL_MOD_EN_MASK		BIT(7)
@@ -90,6 +96,7 @@ struct qcom_wled_config {
 	u32 fs_current;
 	u32 string_cfg;
 	int sc_irq;
+	int ovp_irq;
 	bool en_cabc;
 	bool ext_pfet_sc_pro_en;
 };
@@ -106,6 +113,7 @@ struct qcom_wled {
 	u32 brightness;
 	u32 sc_count;
 	bool prev_state;
+	bool ovp_irq_disabled;
 };
 
 static int qcom_wled_module_enable(struct qcom_wled *wled, int val)
@@ -115,6 +123,28 @@ static int qcom_wled_module_enable(struct qcom_wled *wled, int val)
 	rc = regmap_update_bits(wled->regmap, wled->ctrl_addr +
 			QCOM_WLED_CTRL_MOD_ENABLE, QCOM_WLED_CTRL_MOD_EN_MASK,
 			val << QCOM_WLED_CTRL_MODULE_EN_SHIFT);
+	if (rc < 0)
+		return rc;
+	/*
+	 * Wait for at least 10ms before enabling OVP fault interrupt after
+	 * enabling the module so that soft start is completed. Keep the OVP
+	 * interrupt disabled when the module is disabled.
+	 */
+	if (val) {
+		usleep_range(QCOM_WLED_SOFT_START_DLY_US,
+				QCOM_WLED_SOFT_START_DLY_US + 1000);
+
+		if (wled->cfg.ovp_irq > 0 && wled->ovp_irq_disabled) {
+			enable_irq(wled->cfg.ovp_irq);
+			wled->ovp_irq_disabled = false;
+		}
+	} else {
+		if (wled->cfg.ovp_irq > 0 && !wled->ovp_irq_disabled) {
+			disable_irq(wled->cfg.ovp_irq);
+			wled->ovp_irq_disabled = true;
+		}
+	}
+
 	return rc;
 }
 
@@ -264,12 +294,42 @@ static irqreturn_t qcom_wled_sc_irq_handler(int irq, void *_wled)
 	return IRQ_HANDLED;
 }
 
+static irqreturn_t qcom_wled_ovp_irq_handler(int irq, void *_wled)
+{
+	struct qcom_wled *wled = _wled;
+	int rc;
+	u32 int_sts, fault_sts;
+
+	rc = regmap_read(wled->regmap,
+			wled->ctrl_addr + QCOM_WLED_CTRL_INT_RT_STS, &int_sts);
+	if (rc < 0) {
+		pr_err("Error in reading WLED_INT_RT_STS rc=%d\n", rc);
+		return IRQ_HANDLED;
+	}
+
+	rc = regmap_read(wled->regmap, wled->ctrl_addr +
+			QCOM_WLED_CTRL_FAULT_STATUS, &fault_sts);
+	if (rc < 0) {
+		pr_err("Error in reading WLED_FAULT_STATUS rc=%d\n", rc);
+		return IRQ_HANDLED;
+	}
+
+	if (fault_sts &
+		(QCOM_WLED_CTRL_OVP_FAULT_BIT | QCOM_WLED_CTRL_ILIM_FAULT_BIT))
+		pr_err("WLED OVP fault detected, int_sts=%x fault_sts= %x\n",
+			int_sts, fault_sts);
+
+	return IRQ_HANDLED;
+}
+
 static int qcom_wled_setup(struct qcom_wled *wled)
 {
 	int rc, temp, i;
 	u8 sink_en = 0;
+	u32 val;
 	u8 string_cfg = wled->cfg.string_cfg;
 	int sc_irq = wled->cfg.sc_irq;
+	int ovp_irq = wled->cfg.ovp_irq;
 
 	rc = regmap_update_bits(wled->regmap,
 			wled->ctrl_addr + QCOM_WLED_CTRL_OVP,
@@ -367,6 +427,25 @@ static int qcom_wled_setup(struct qcom_wled *wled)
 		}
 	}
 
+	if (ovp_irq >= 0) {
+		rc = devm_request_threaded_irq(&wled->pdev->dev, ovp_irq,
+				NULL, qcom_wled_ovp_irq_handler, IRQF_ONESHOT,
+				"qcom_wled_ovp_irq", wled);
+		if (rc < 0) {
+			dev_err(&wled->pdev->dev, "Unable to request ovp(%d) IRQ(err:%d)\n",
+				ovp_irq, rc);
+			return rc;
+		}
+
+		rc = regmap_read(wled->regmap, wled->ctrl_addr +
+				QCOM_WLED_CTRL_MOD_ENABLE, &val);
+		/* disable the OVP irq only if the module is not enabled */
+		if (!rc && !(val & QCOM_WLED_CTRL_MOD_EN_MASK)) {
+			disable_irq(ovp_irq);
+			wled->ovp_irq_disabled = true;
+		}
+	}
+
 	return 0;
 }
 
@@ -539,6 +618,10 @@ static int qcom_wled_configure(struct qcom_wled *wled, struct device *dev)
 	if (wled->cfg.sc_irq < 0)
 		dev_dbg(&wled->pdev->dev, "sc irq is not used\n");
 
+	wled->cfg.ovp_irq = platform_get_irq_byname(wled->pdev, "ovp-irq");
+	if (wled->cfg.ovp_irq < 0)
+		dev_dbg(&wled->pdev->dev, "ovp irq is not used\n");
+
 	return 0;
 }
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH V1 3/4] qcom: spmi-wled: Add support for OVP interrupt handling

From: Bjorn Andersson <hidden>
Date: 2017-12-05 04:45:37

On Thu 16 Nov 04:18 PST 2017, Kiran Gunda wrote:
quoted hunk
WLED peripheral has over voltage protection(OVP) circuitry and the OVP
fault is notified through an interrupt. Though this fault condition rising
is due to an incorrect hardware configuration is mitigated in the hardware,
it still needs to be detected and handled. Add support for it.

When WLED module is enabled, keep OVP fault interrupt disabled for 10 ms to
account for soft start delay.

Signed-off-by: Kiran Gunda <redacted>
---
 .../bindings/leds/backlight/qcom-spmi-wled.txt     |  7 +-
 drivers/video/backlight/qcom-spmi-wled.c           | 83 ++++++++++++++++++++++
 2 files changed, 87 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt b/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt
index 768608c..d39ee93 100644
--- a/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt
+++ b/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt
@@ -92,7 +92,7 @@ The PMIC is connected to the host processor via SPMI bus.
 	Usage:      optional
 	Value type: <string>
 	Definition: Interrupt names associated with the interrupts.
-		    Must be "sc-irq".
+		    Currently supported interrupts are "sc-irq" and "ovp-irq".
 
As before, we know this is an IRQ, so omit the -irq from the name.

[..]
quoted hunk
diff --git a/drivers/video/backlight/qcom-spmi-wled.c b/drivers/video/backlight/qcom-spmi-wled.c
[..]
quoted hunk
@@ -115,6 +123,28 @@ static int qcom_wled_module_enable(struct qcom_wled *wled, int val)
 	rc = regmap_update_bits(wled->regmap, wled->ctrl_addr +
 			QCOM_WLED_CTRL_MOD_ENABLE, QCOM_WLED_CTRL_MOD_EN_MASK,
 			val << QCOM_WLED_CTRL_MODULE_EN_SHIFT);
+	if (rc < 0)
+		return rc;
+	/*
+	 * Wait for at least 10ms before enabling OVP fault interrupt after
+	 * enabling the module so that soft start is completed. Keep the OVP
+	 * interrupt disabled when the module is disabled.
+	 */
+	if (val) {
+		usleep_range(QCOM_WLED_SOFT_START_DLY_US,
+				QCOM_WLED_SOFT_START_DLY_US + 1000);
This is sleeping in the brightness/enable code path, can you
schedule_delayed_work() instead to not block this code path
unnecessarily?
quoted hunk
+
+		if (wled->cfg.ovp_irq > 0 && wled->ovp_irq_disabled) {
+			enable_irq(wled->cfg.ovp_irq);
+			wled->ovp_irq_disabled = false;
+		}
+	} else {
+		if (wled->cfg.ovp_irq > 0 && !wled->ovp_irq_disabled) {
+			disable_irq(wled->cfg.ovp_irq);
+			wled->ovp_irq_disabled = true;
+		}
+	}
+
 	return rc;
 }
 
@@ -264,12 +294,42 @@ static irqreturn_t qcom_wled_sc_irq_handler(int irq, void *_wled)
 	return IRQ_HANDLED;
 }
 
+static irqreturn_t qcom_wled_ovp_irq_handler(int irq, void *_wled)
+{
+	struct qcom_wled *wled = _wled;
+	int rc;
+	u32 int_sts, fault_sts;
+
+	rc = regmap_read(wled->regmap,
+			wled->ctrl_addr + QCOM_WLED_CTRL_INT_RT_STS, &int_sts);
+	if (rc < 0) {
+		pr_err("Error in reading WLED_INT_RT_STS rc=%d\n", rc);
+		return IRQ_HANDLED;
+	}
+
+	rc = regmap_read(wled->regmap, wled->ctrl_addr +
+			QCOM_WLED_CTRL_FAULT_STATUS, &fault_sts);
+	if (rc < 0) {
+		pr_err("Error in reading WLED_FAULT_STATUS rc=%d\n", rc);
+		return IRQ_HANDLED;
+	}
+
+	if (fault_sts &
+		(QCOM_WLED_CTRL_OVP_FAULT_BIT | QCOM_WLED_CTRL_ILIM_FAULT_BIT))
+		pr_err("WLED OVP fault detected, int_sts=%x fault_sts= %x\n",
+			int_sts, fault_sts);
All this function does is print things to the log. When is this
information consumed and by whom? dev_dbg() instead?
quoted hunk
+
+	return IRQ_HANDLED;
+}
+
 static int qcom_wled_setup(struct qcom_wled *wled)
 {
 	int rc, temp, i;
 	u8 sink_en = 0;
+	u32 val;
 	u8 string_cfg = wled->cfg.string_cfg;
 	int sc_irq = wled->cfg.sc_irq;
+	int ovp_irq = wled->cfg.ovp_irq;
 
 	rc = regmap_update_bits(wled->regmap,
 			wled->ctrl_addr + QCOM_WLED_CTRL_OVP,
@@ -367,6 +427,25 @@ static int qcom_wled_setup(struct qcom_wled *wled)
 		}
 	}
 
+	if (ovp_irq >= 0) {
As with the previous patch.

[..]
quoted hunk
@@ -539,6 +618,10 @@ static int qcom_wled_configure(struct qcom_wled *wled, struct device *dev)
[..]
+	wled->cfg.ovp_irq = platform_get_irq_byname(wled->pdev, "ovp-irq");
+	if (wled->cfg.ovp_irq < 0)
+		dev_dbg(&wled->pdev->dev, "ovp irq is not used\n");
+
Regards,
Bjorn

Re: [PATCH V1 3/4] qcom: spmi-wled: Add support for OVP interrupt handling

From: <hidden>
Date: 2017-12-11 09:31:45

On 2017-12-05 10:15, Bjorn Andersson wrote:
On Thu 16 Nov 04:18 PST 2017, Kiran Gunda wrote:
quoted
WLED peripheral has over voltage protection(OVP) circuitry and the OVP
fault is notified through an interrupt. Though this fault condition 
rising
is due to an incorrect hardware configuration is mitigated in the 
hardware,
it still needs to be detected and handled. Add support for it.

When WLED module is enabled, keep OVP fault interrupt disabled for 10 
ms to
account for soft start delay.

Signed-off-by: Kiran Gunda <redacted>
---
 .../bindings/leds/backlight/qcom-spmi-wled.txt     |  7 +-
 drivers/video/backlight/qcom-spmi-wled.c           | 83 
++++++++++++++++++++++
 2 files changed, 87 insertions(+), 3 deletions(-)
diff --git 
a/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt 
b/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt
index 768608c..d39ee93 100644
--- 
a/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt
+++ 
b/Documentation/devicetree/bindings/leds/backlight/qcom-spmi-wled.txt
@@ -92,7 +92,7 @@ The PMIC is connected to the host processor via SPMI 
bus.
 	Usage:      optional
 	Value type: <string>
 	Definition: Interrupt names associated with the interrupts.
-		    Must be "sc-irq".
+		    Currently supported interrupts are "sc-irq" and "ovp-irq".
As before, we know this is an IRQ, so omit the -irq from the name.

[..]
Sure. Will change it in the next series.
quoted
diff --git a/drivers/video/backlight/qcom-spmi-wled.c 
b/drivers/video/backlight/qcom-spmi-wled.c
[..]
quoted
@@ -115,6 +123,28 @@ static int qcom_wled_module_enable(struct 
qcom_wled *wled, int val)
 	rc = regmap_update_bits(wled->regmap, wled->ctrl_addr +
 			QCOM_WLED_CTRL_MOD_ENABLE, QCOM_WLED_CTRL_MOD_EN_MASK,
 			val << QCOM_WLED_CTRL_MODULE_EN_SHIFT);
+	if (rc < 0)
+		return rc;
+	/*
+	 * Wait for at least 10ms before enabling OVP fault interrupt after
+	 * enabling the module so that soft start is completed. Keep the OVP
+	 * interrupt disabled when the module is disabled.
+	 */
+	if (val) {
+		usleep_range(QCOM_WLED_SOFT_START_DLY_US,
+				QCOM_WLED_SOFT_START_DLY_US + 1000);
This is sleeping in the brightness/enable code path, can you
schedule_delayed_work() instead to not block this code path
unnecessarily?
Sure. Will change it in the next series.
quoted
+
+		if (wled->cfg.ovp_irq > 0 && wled->ovp_irq_disabled) {
+			enable_irq(wled->cfg.ovp_irq);
+			wled->ovp_irq_disabled = false;
+		}
+	} else {
+		if (wled->cfg.ovp_irq > 0 && !wled->ovp_irq_disabled) {
+			disable_irq(wled->cfg.ovp_irq);
+			wled->ovp_irq_disabled = true;
+		}
+	}
+
 	return rc;
 }
@@ -264,12 +294,42 @@ static irqreturn_t qcom_wled_sc_irq_handler(int 
irq, void *_wled)
 	return IRQ_HANDLED;
 }

+static irqreturn_t qcom_wled_ovp_irq_handler(int irq, void *_wled)
+{
+	struct qcom_wled *wled = _wled;
+	int rc;
+	u32 int_sts, fault_sts;
+
+	rc = regmap_read(wled->regmap,
+			wled->ctrl_addr + QCOM_WLED_CTRL_INT_RT_STS, &int_sts);
+	if (rc < 0) {
+		pr_err("Error in reading WLED_INT_RT_STS rc=%d\n", rc);
+		return IRQ_HANDLED;
+	}
+
+	rc = regmap_read(wled->regmap, wled->ctrl_addr +
+			QCOM_WLED_CTRL_FAULT_STATUS, &fault_sts);
+	if (rc < 0) {
+		pr_err("Error in reading WLED_FAULT_STATUS rc=%d\n", rc);
+		return IRQ_HANDLED;
+	}
+
+	if (fault_sts &
+		(QCOM_WLED_CTRL_OVP_FAULT_BIT | QCOM_WLED_CTRL_ILIM_FAULT_BIT))
+		pr_err("WLED OVP fault detected, int_sts=%x fault_sts= %x\n",
+			int_sts, fault_sts);
All this function does is print things to the log. When is this
information consumed and by whom? dev_dbg() instead?
Sure. Will change it in the next series.
quoted
+
+	return IRQ_HANDLED;
+}
+
 static int qcom_wled_setup(struct qcom_wled *wled)
 {
 	int rc, temp, i;
 	u8 sink_en = 0;
+	u32 val;
 	u8 string_cfg = wled->cfg.string_cfg;
 	int sc_irq = wled->cfg.sc_irq;
+	int ovp_irq = wled->cfg.ovp_irq;

 	rc = regmap_update_bits(wled->regmap,
 			wled->ctrl_addr + QCOM_WLED_CTRL_OVP,
@@ -367,6 +427,25 @@ static int qcom_wled_setup(struct qcom_wled 
*wled)
 		}
 	}

+	if (ovp_irq >= 0) {
As with the previous patch.

[..]
Sure. Will change it in the next series.
quoted
@@ -539,6 +618,10 @@ static int qcom_wled_configure(struct qcom_wled 
*wled, struct device *dev)
[..]
quoted
+	wled->cfg.ovp_irq = platform_get_irq_byname(wled->pdev, "ovp-irq");
+	if (wled->cfg.ovp_irq < 0)
+		dev_dbg(&wled->pdev->dev, "ovp irq is not used\n");
+
Regards,
Bjorn
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help