* [v3,07/12] staging: typec: tcpci: register port before request irq
@ 2018-03-13 9:34 Jun Li
0 siblings, 0 replies; 3+ messages in thread
From: Jun Li @ 2018-03-13 9:34 UTC (permalink / raw)
To: robh+dt, mark.rutland, gregkh, heikki.krogerus
Cc: a.hajda, jun.li, linux, yueyao, shufan_lee, o_leveque, linux-usb,
linux-imx
With that we can clear any pending events and the port is registered
so driver can be ready to handle typec events once we request irq.
Signed-off-by: Peter Chen <peter.chen@nxp.com>
Signed-off-by: Li Jun <jun.li@nxp.com>
---
drivers/staging/typec/tcpci.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
index ba4627f..f5a3bf5 100644
--- a/drivers/staging/typec/tcpci.c
+++ b/drivers/staging/typec/tcpci.c
@@ -600,25 +600,26 @@ static int tcpci_probe(struct i2c_client *client,
if (IS_ERR(chip->data.regmap))
return PTR_ERR(chip->data.regmap);
+ i2c_set_clientdata(client, chip);
+
/* Disable chip interrupts before requesting irq */
err = regmap_raw_write(chip->data.regmap, TCPC_ALERT_MASK, &val,
sizeof(u16));
if (err < 0)
return err;
+ chip->tcpci = tcpci_register_port(&client->dev, &chip->data);
+ if (PTR_ERR_OR_ZERO(chip->tcpci))
+ return PTR_ERR(chip->tcpci);
+
err = devm_request_threaded_irq(&client->dev, client->irq, NULL,
_tcpci_irq,
IRQF_ONESHOT | IRQF_TRIGGER_LOW,
dev_name(&client->dev), chip);
if (err < 0)
- return err;
+ tcpci_unregister_port(chip->tcpci);
- chip->tcpci = tcpci_register_port(&client->dev, &chip->data);
- if (PTR_ERR_OR_ZERO(chip->tcpci))
- return PTR_ERR(chip->tcpci);
-
- i2c_set_clientdata(client, chip);
- return 0;
+ return err;
}
static int tcpci_remove(struct i2c_client *client)
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [v3,07/12] staging: typec: tcpci: register port before request irq
@ 2018-03-15 4:51 Guenter Roeck
0 siblings, 0 replies; 3+ messages in thread
From: Guenter Roeck @ 2018-03-15 4:51 UTC (permalink / raw)
To: Li Jun
Cc: robh+dt, mark.rutland, gregkh, heikki.krogerus, a.hajda, yueyao,
shufan_lee, o_leveque, linux-usb, linux-imx
On Tue, Mar 13, 2018 at 05:34:33PM +0800, Li Jun wrote:
> With that we can clear any pending events and the port is registered
> so driver can be ready to handle typec events once we request irq.
>
> Signed-off-by: Peter Chen <peter.chen@nxp.com>
> Signed-off-by: Li Jun <jun.li@nxp.com>
> ---
> drivers/staging/typec/tcpci.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
> index ba4627f..f5a3bf5 100644
> --- a/drivers/staging/typec/tcpci.c
> +++ b/drivers/staging/typec/tcpci.c
> @@ -600,25 +600,26 @@ static int tcpci_probe(struct i2c_client *client,
> if (IS_ERR(chip->data.regmap))
> return PTR_ERR(chip->data.regmap);
>
> + i2c_set_clientdata(client, chip);
> +
> /* Disable chip interrupts before requesting irq */
> err = regmap_raw_write(chip->data.regmap, TCPC_ALERT_MASK, &val,
> sizeof(u16));
> if (err < 0)
> return err;
>
> + chip->tcpci = tcpci_register_port(&client->dev, &chip->data);
> + if (PTR_ERR_OR_ZERO(chip->tcpci))
> + return PTR_ERR(chip->tcpci);
> +
I am concerned that the regitration above might already trigger commands,
and that we might miss interrupts if that is the case.
Would it make sense to check for chip->tcpci in the interrupt handler instead ?
> err = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> _tcpci_irq,
> IRQF_ONESHOT | IRQF_TRIGGER_LOW,
> dev_name(&client->dev), chip);
> if (err < 0)
> - return err;
> + tcpci_unregister_port(chip->tcpci);
>
> - chip->tcpci = tcpci_register_port(&client->dev, &chip->data);
> - if (PTR_ERR_OR_ZERO(chip->tcpci))
> - return PTR_ERR(chip->tcpci);
> -
> - i2c_set_clientdata(client, chip);
> - return 0;
> + return err;
> }
>
> static int tcpci_remove(struct i2c_client *client)
> --
> 2.7.4
>
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 3+ messages in thread
* [v3,07/12] staging: typec: tcpci: register port before request irq
@ 2018-03-20 1:24 Jun Li
0 siblings, 0 replies; 3+ messages in thread
From: Jun Li @ 2018-03-20 1:24 UTC (permalink / raw)
To: Guenter Roeck
Cc: robh+dt@kernel.org, mark.rutland@arm.com,
gregkh@linuxfoundation.org, heikki.krogerus@linux.intel.com,
a.hajda@samsung.com, yueyao@google.com, shufan_lee@richtek.com,
o_leveque@orange.fr, linux-usb@vger.kernel.org, dl-linux-imx
Hi
> -----Original Message-----
> From: Guenter Roeck [mailto:groeck7@gmail.com] On Behalf Of Guenter
> Roeck
> Sent: 2018年3月15日 12:51
> To: Jun Li <jun.li@nxp.com>
> Cc: robh+dt@kernel.org; mark.rutland@arm.com;
> gregkh@linuxfoundation.org; heikki.krogerus@linux.intel.com;
> a.hajda@samsung.com; yueyao@google.com; shufan_lee@richtek.com;
> o_leveque@orange.fr; linux-usb@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>
> Subject: Re: [PATCH v3 07/12] staging: typec: tcpci: register port before
> request irq
>
> On Tue, Mar 13, 2018 at 05:34:33PM +0800, Li Jun wrote:
> > With that we can clear any pending events and the port is registered
> > so driver can be ready to handle typec events once we request irq.
> >
> > Signed-off-by: Peter Chen <peter.chen@nxp.com>
> > Signed-off-by: Li Jun <jun.li@nxp.com>
> > ---
> > drivers/staging/typec/tcpci.c | 15 ++++++++-------
> > 1 file changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/staging/typec/tcpci.c
> > b/drivers/staging/typec/tcpci.c index ba4627f..f5a3bf5 100644
> > --- a/drivers/staging/typec/tcpci.c
> > +++ b/drivers/staging/typec/tcpci.c
> > @@ -600,25 +600,26 @@ static int tcpci_probe(struct i2c_client *client,
> > if (IS_ERR(chip->data.regmap))
> > return PTR_ERR(chip->data.regmap);
> >
> > + i2c_set_clientdata(client, chip);
> > +
> > /* Disable chip interrupts before requesting irq */
> > err = regmap_raw_write(chip->data.regmap, TCPC_ALERT_MASK, &val,
> > sizeof(u16));
> > if (err < 0)
> > return err;
> >
> > + chip->tcpci = tcpci_register_port(&client->dev, &chip->data);
> > + if (PTR_ERR_OR_ZERO(chip->tcpci))
> > + return PTR_ERR(chip->tcpci);
> > +
>
> I am concerned that the regitration above might already trigger commands,
> and that we might miss interrupts if that is the case.
>
> Would it make sense to check for chip->tcpci in the interrupt handler instead ?
Tcpci_irq()
{
if(!chip->tcpci)
return IRQ_HANDLED;
/* Read alert and clear events */
...
}
With this way, is it possible we repeat enter interrupt handler without chance to
clear it?
Thanks
Jun
>
> > err = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> > _tcpci_irq,
> > IRQF_ONESHOT | IRQF_TRIGGER_LOW,
> > dev_name(&client->dev), chip);
> > if (err < 0)
> > - return err;
> > + tcpci_unregister_port(chip->tcpci);
> >
> > - chip->tcpci = tcpci_register_port(&client->dev, &chip->data);
> > - if (PTR_ERR_OR_ZERO(chip->tcpci))
> > - return PTR_ERR(chip->tcpci);
> > -
> > - i2c_set_clientdata(client, chip);
> > - return 0;
> > + return err;
> > }
> >
> > static int tcpci_remove(struct i2c_client *client)
> > --
> > 2.7.4
> >
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-03-20 1:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-15 4:51 [v3,07/12] staging: typec: tcpci: register port before request irq Guenter Roeck
-- strict thread matches above, loose matches on Subject: below --
2018-03-20 1:24 Jun Li
2018-03-13 9:34 Jun Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).