2023-03-24 13:20:04

by Roger Quadros

[permalink] [raw]
Subject: [PATCH] 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.

Signed-off-by: Aswath Govindraju <[email protected]>
Signed-off-by: Roger Quadros <[email protected]>
---
drivers/usb/typec/tipd/core.c | 41 ++++++++++++++++++++++++++++++-----
1 file changed, 36 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index 485b90c13078..d28ffa10a122 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -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;
}

--
2.34.1


2023-03-24 13:43:16

by Roger Quadros

[permalink] [raw]
Subject: [PATCH] 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.

Signed-off-by: Roger Quadros <[email protected]>
---
Documentation/devicetree/bindings/usb/ti,tps6598x.yaml | 2 --
1 file changed, 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
index 348a715d61f4..8c2db282735a 100644
--- a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
+++ b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
@@ -35,8 +35,6 @@ properties:
required:
- compatible
- reg
- - interrupts
- - interrupt-names

additionalProperties: true

--
2.34.1

2023-03-28 13:52:50

by Heikki Krogerus

[permalink] [raw]
Subject: Re: [PATCH] usb: typec: tps6598x: Add support for polling interrupts status

On Fri, Mar 24, 2023 at 03:18:53PM +0200, Roger Quadros wrote:
> 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.
>
> Signed-off-by: Aswath Govindraju <[email protected]>
> Signed-off-by: Roger Quadros <[email protected]>

Reviewed-by: Heikki Krogerus <[email protected]>

> ---
> drivers/usb/typec/tipd/core.c | 41 ++++++++++++++++++++++++++++++-----
> 1 file changed, 36 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index 485b90c13078..d28ffa10a122 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -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;
> }
>
> --
> 2.34.1

--
heikki

2023-03-29 07:08:17

by Roger Quadros

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

Hi Heikki & Rob,

On 24/03/2023 15:37, Roger Quadros wrote:
> 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.
>
> Signed-off-by: Roger Quadros <[email protected]>
> ---
> Documentation/devicetree/bindings/usb/ti,tps6598x.yaml | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
> index 348a715d61f4..8c2db282735a 100644
> --- a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
> +++ b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
> @@ -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

2023-03-29 10:44:53

by Heikki Krogerus

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

On Wed, Mar 29, 2023 at 10:05:33AM +0300, Roger Quadros wrote:
> Hi Heikki & Rob,
>
> On 24/03/2023 15:37, Roger Quadros wrote:
> > 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.
> >
> > Signed-off-by: Roger Quadros <[email protected]>
> > ---
> > Documentation/devicetree/bindings/usb/ti,tps6598x.yaml | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
> > index 348a715d61f4..8c2db282735a 100644
> > --- a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
> > +++ b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
> > @@ -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!

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

Acked-by: Heikki Krogerus <[email protected]>

thanks,

--
heikki

2023-03-31 19:01:23

by Rob Herring (Arm)

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

On Wed, Mar 29, 2023 at 10:05:33AM +0300, Roger Quadros wrote:
> Hi Heikki & Rob,
>
> On 24/03/2023 15:37, Roger Quadros wrote:
> > 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.
> >
> > Signed-off-by: Roger Quadros <[email protected]>
> > ---
> > Documentation/devicetree/bindings/usb/ti,tps6598x.yaml | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
> > index 348a715d61f4..8c2db282735a 100644
> > --- a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
> > +++ b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
> > @@ -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!

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.

Acked-by: Rob Herring <[email protected]>

Rob