Skip to contentSkip to conversation

usb: typec: tps6598x: Add support for polling interrupts status

PATCH In review Roger Quadros 3 participants 6 messages Mar 24, 2023 – Mar 31, 2023

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 usb: typec: tps6598x: Add support for polling interrupts status

From: Aswath Govindraju <[email protected]>

Some development boards don't have the interrupt line connected.

In such cases we can resort to polling the interrupt status.

---
 drivers/usb/typec/tipd/core.c | 41 ++++++++++++++++++++++++++++++-----
 1 file changed, 36 insertions(+), 5 deletions(-)
drivers/usb/typec/tipd/core.c +36 -5
index 485b90c13078..d28ffa10a122 100644
@@ -16,6 +16,7 @@
 #include <linux/usb/typec.h>
 #include <linux/usb/typec_altmode.h>
 #include <linux/usb/role.h>
+#include <linux/workqueue.h>
 
 #include "tps6598x.h"
 #include "trace.h"
@@ -97,6 +98,8 @@ struct tps6598x {
 
 	int wakeup;
 	u16 pwr_status;
+	struct delayed_work	wq_poll;
+	irq_handler_t irq_handler;
 };
 
 static enum power_supply_property tps6598x_psy_props[] = {
@@ -568,6 +571,18 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
 	return IRQ_NONE;
 }
 
+/* Time interval for Polling */
+#define POLL_INTERVAL	500 /* msecs */
+static void tps6598x_poll_work(struct work_struct *work)
+{
+	struct tps6598x *tps = container_of(to_delayed_work(work),
+					    struct tps6598x, wq_poll);
+
+	tps->irq_handler(0, tps);
+	queue_delayed_work(system_power_efficient_wq,
+			   &tps->wq_poll, msecs_to_jiffies(POLL_INTERVAL));
+}
+
 static int tps6598x_check_mode(struct tps6598x *tps)
 {
 	char mode[5] = { };
@@ -746,6 +761,7 @@ static int tps6598x_probe(struct i2c_client *client)
 			TPS_REG_INT_PLUG_EVENT;
 	}
 
+	tps->irq_handler = irq_handler;
 	/* Make sure the controller has application firmware running */
 	ret = tps6598x_check_mode(tps);
 	if (ret)
@@ -837,10 +853,18 @@ static int tps6598x_probe(struct i2c_client *client)
 			dev_err(&client->dev, "failed to register partner\n");
 	}
 
-	ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
-					irq_handler,
-					IRQF_SHARED | IRQF_ONESHOT,
-					dev_name(&client->dev), tps);
+	if (client->irq) {
+		ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
+						irq_handler,
+						IRQF_SHARED | IRQF_ONESHOT,
+						dev_name(&client->dev), tps);
+	} else {
+		dev_warn(tps->dev, "Unable to find the interrupt, switching to polling\n");
+		INIT_DELAYED_WORK(&tps->wq_poll, tps6598x_poll_work);
+		queue_delayed_work(system_power_efficient_wq, &tps->wq_poll,
+				   msecs_to_jiffies(POLL_INTERVAL));
+	}
+
 	if (ret)
 		goto err_disconnect;
 
@@ -848,7 +872,7 @@ static int tps6598x_probe(struct i2c_client *client)
 	fwnode_handle_put(fwnode);
 
 	tps->wakeup = device_property_read_bool(tps->dev, "wakeup-source");
-	if (tps->wakeup) {
+	if (tps->wakeup && client->irq) {
 		device_init_wakeup(&client->dev, true);
 		enable_irq_wake(client->irq);
 	}
@@ -887,6 +911,9 @@ static int __maybe_unused tps6598x_suspend(struct device *dev)
 		enable_irq_wake(client->irq);
 	}
 
+	if (!client->irq)
+		cancel_delayed_work_sync(&tps->wq_poll);
+
 	return 0;
 }
 
@@ -900,6 +927,10 @@ static int __maybe_unused tps6598x_resume(struct device *dev)
 		enable_irq(client->irq);
 	}
 
+	if (client->irq)
+		queue_delayed_work(system_power_efficient_wq, &tps->wq_poll,
+				   msecs_to_jiffies(POLL_INTERVAL));
+
 	return 0;
 }
 
Signature · 1 line
2.34.1
#1 lore raw

PATCH 1/1 dt-bindings: usb: tps6598x: make interrupts optional

The driver can poll for interrupt status so interrupts can be optional. It is still recommended to use the interrupt line. Polling should only be used for debug and prototyping.

---
 Documentation/devicetree/bindings/usb/ti,tps6598x.yaml | 2 --
 1 file changed, 2 deletions(-)
Documentation/devicetree/bindings/usb/ti,tps6598x.yaml +0 -2
index 348a715d61f4..8c2db282735a 100644
@@ -35,8 +35,6 @@ properties:
 required:
   - compatible
   - reg
-  - interrupts
-  - interrupt-names
 
 additionalProperties: true
 
Signature · 1 line
2.34.1
#2 lore raw

Re: [PATCH] dt-bindings: usb: tps6598x: make interrupts optional

Hi Heikki & Rob,

Quoted 15 lines · Roger Quadros

The driver can poll for interrupt status so interrupts can be optional. It is still recommended to use the interrupt line. Polling should only be used for debug and prototyping.

---
 Documentation/devicetree/bindings/usb/ti,tps6598x.yaml | 2 --
 1 file changed, 2 deletions(-)
Documentation/devicetree/bindings/usb/ti,tps6598x.yaml +0 -0
index 348a715d61f4..8c2db282735a 100644

Documentation/devicetree/bindings/usb/ti,tps6598x.yaml properties: · line 40

Quoted 8 lines
Documentation/devicetree/bindings/usb/ti,tps6598x.yaml +0 -2
@@ -35,8 +35,6 @@ properties:
 required:
   - compatible
   - reg
-  - interrupts
-  - interrupt-names
 
 additionalProperties: true
 

We need this patch as well along with the driver changes [1] Could you please Ack. Thanks!

[1] - https://lore.kernel.org/r/[email protected]

cheers,
-roger

maintainer Acked-by
#3 lore raw

Re: [PATCH] dt-bindings: usb: tps6598x: make interrupts optional

Quoted 24 lines · Roger Quadros

Hi Heikki & Rob,

Quoted 21 lines · Roger Quadros

The driver can poll for interrupt status so interrupts can be optional. It is still recommended to use the interrupt line. Polling should only be used for debug and prototyping.

---
 Documentation/devicetree/bindings/usb/ti,tps6598x.yaml | 2 --
 1 file changed, 2 deletions(-)
Documentation/devicetree/bindings/usb/ti,tps6598x.yaml +0 -2
index 348a715d61f4..8c2db282735a 100644
@@ -35,8 +35,6 @@ properties:
 required:
   - compatible
   - reg
-  - interrupts
-  - interrupt-names
 
Quoted 5 lines

Documentation/devicetree/bindings/usb/ti,tps6598x.yaml properties: · line 40

Quoted 2 lines
Documentation/devicetree/bindings/usb/ti,tps6598x.yaml +0 -0
 additionalProperties: true
 

We need this patch as well along with the driver changes [1] Could you please Ack. Thanks!

I can give my ack FWIW, but we should still wait for Rob.

thanks,

Signature · 1 line
heikki
maintainer Acked-by
#4 lore raw

Re: [PATCH] dt-bindings: usb: tps6598x: make interrupts optional

In reply to Roger Quadros
Quoted 24 lines · Roger Quadros

Hi Heikki & Rob,

Quoted 21 lines · Roger Quadros

The driver can poll for interrupt status so interrupts can be optional. It is still recommended to use the interrupt line. Polling should only be used for debug and prototyping.

---
 Documentation/devicetree/bindings/usb/ti,tps6598x.yaml | 2 --
 1 file changed, 2 deletions(-)
Documentation/devicetree/bindings/usb/ti,tps6598x.yaml +0 -2
index 348a715d61f4..8c2db282735a 100644
@@ -35,8 +35,6 @@ properties:
 required:
   - compatible
   - reg
-  - interrupts
-  - interrupt-names
 
Quoted 5 lines

Documentation/devicetree/bindings/usb/ti,tps6598x.yaml properties: · line 40

Quoted 2 lines
Documentation/devicetree/bindings/usb/ti,tps6598x.yaml +0 -0
 additionalProperties: true
 

We need this patch as well along with the driver changes [1] Could you please Ack. Thanks!

If get_maintainers.pl had been properly used, then it probably would have been acked by Krzysztof already. Otherwise, it's in my queue if the DT list is copied.

Rob

maintainer Reviewed-by on v1 1/1
#5 lore raw
In reply to Roger Quadros
Quoted 8 lines · Roger Quadros

From: Aswath Govindraju <[email protected]>

Some development boards don't have the interrupt line connected.

In such cases we can resort to polling the interrupt status.

Quoted 107 lines
---
 drivers/usb/typec/tipd/core.c | 41 ++++++++++++++++++++++++++++++-----
 1 file changed, 36 insertions(+), 5 deletions(-)
drivers/usb/typec/tipd/core.c +36 -5
index 485b90c13078..d28ffa10a122 100644
@@ -16,6 +16,7 @@
 #include <linux/usb/typec.h>
 #include <linux/usb/typec_altmode.h>
 #include <linux/usb/role.h>
+#include <linux/workqueue.h>
 
 #include "tps6598x.h"
 #include "trace.h"
@@ -97,6 +98,8 @@ struct tps6598x {
 
 	int wakeup;
 	u16 pwr_status;
+	struct delayed_work	wq_poll;
+	irq_handler_t irq_handler;
 };
 
 static enum power_supply_property tps6598x_psy_props[] = {
@@ -568,6 +571,18 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
 	return IRQ_NONE;
 }
 
+/* Time interval for Polling */
+#define POLL_INTERVAL	500 /* msecs */
+static void tps6598x_poll_work(struct work_struct *work)
+{
+	struct tps6598x *tps = container_of(to_delayed_work(work),
+					    struct tps6598x, wq_poll);
+
+	tps->irq_handler(0, tps);
+	queue_delayed_work(system_power_efficient_wq,
+			   &tps->wq_poll, msecs_to_jiffies(POLL_INTERVAL));
+}
+
 static int tps6598x_check_mode(struct tps6598x *tps)
 {
 	char mode[5] = { };
@@ -746,6 +761,7 @@ static int tps6598x_probe(struct i2c_client *client)
 			TPS_REG_INT_PLUG_EVENT;
 	}
 
+	tps->irq_handler = irq_handler;
 	/* Make sure the controller has application firmware running */
 	ret = tps6598x_check_mode(tps);
 	if (ret)
@@ -837,10 +853,18 @@ static int tps6598x_probe(struct i2c_client *client)
 			dev_err(&client->dev, "failed to register partner\n");
 	}
 
-	ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
-					irq_handler,
-					IRQF_SHARED | IRQF_ONESHOT,
-					dev_name(&client->dev), tps);
+	if (client->irq) {
+		ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
+						irq_handler,
+						IRQF_SHARED | IRQF_ONESHOT,
+						dev_name(&client->dev), tps);
+	} else {
+		dev_warn(tps->dev, "Unable to find the interrupt, switching to polling\n");
+		INIT_DELAYED_WORK(&tps->wq_poll, tps6598x_poll_work);
+		queue_delayed_work(system_power_efficient_wq, &tps->wq_poll,
+				   msecs_to_jiffies(POLL_INTERVAL));
+	}
+
 	if (ret)
 		goto err_disconnect;
 
@@ -848,7 +872,7 @@ static int tps6598x_probe(struct i2c_client *client)
 	fwnode_handle_put(fwnode);
 
 	tps->wakeup = device_property_read_bool(tps->dev, "wakeup-source");
-	if (tps->wakeup) {
+	if (tps->wakeup && client->irq) {
 		device_init_wakeup(&client->dev, true);
 		enable_irq_wake(client->irq);
 	}
@@ -887,6 +911,9 @@ static int __maybe_unused tps6598x_suspend(struct device *dev)
 		enable_irq_wake(client->irq);
 	}
 
+	if (!client->irq)
+		cancel_delayed_work_sync(&tps->wq_poll);
+
 	return 0;
 }
 
@@ -900,6 +927,10 @@ static int __maybe_unused tps6598x_resume(struct device *dev)
 		enable_irq(client->irq);
 	}
 
+	if (client->irq)
+		queue_delayed_work(system_power_efficient_wq, &tps->wq_poll,
+				   msecs_to_jiffies(POLL_INTERVAL));
+
 	return 0;
 }
 
Signature · 1 line
2.34.1
Signature · 1 line
heikki