diff mbox series

usb: typec: tcpci: Request IRQ with IRQF_SHARED

Message ID 20221208071648.2379254-1-xu.yang_2@nxp.com
State New
Headers show
Series usb: typec: tcpci: Request IRQ with IRQF_SHARED | expand

Commit Message

Xu Yang Dec. 8, 2022, 7:16 a.m. UTC
Under resource constraints, this interrupt may use other interrupt line
or this interrupt line may be shared with other devices as long as they
meet the sharing requirements. Besides, This irq flag will not cause other
side effect if tcpci driver is the only user. So a kindly wish to add this
flag.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
 drivers/usb/typec/tcpm/tcpci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Guenter Roeck Dec. 11, 2022, 3:56 p.m. UTC | #1
On 12/7/22 23:16, Xu Yang wrote:
> Under resource constraints, this interrupt may use other interrupt line
> or this interrupt line may be shared with other devices as long as they
> meet the sharing requirements. Besides, This irq flag will not cause other
> side effect if tcpci driver is the only user. So a kindly wish to add this
> flag.

The last sentence is not appropriate for a commit description.

> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
>   drivers/usb/typec/tcpm/tcpci.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> index fe781a38dc82..223a1de4fb1d 100644
> --- a/drivers/usb/typec/tcpm/tcpci.c
> +++ b/drivers/usb/typec/tcpm/tcpci.c
> @@ -838,7 +838,7 @@ static int tcpci_probe(struct i2c_client *client)
>   
>   	err = devm_request_threaded_irq(&client->dev, client->irq, NULL,
>   					_tcpci_irq,
> -					IRQF_ONESHOT | IRQF_TRIGGER_LOW,
> +					IRQF_SHARED | IRQF_ONESHOT | IRQF_TRIGGER_LOW,
>   					dev_name(&client->dev), chip);
>   	if (err < 0) {
>   		tcpci_unregister_port(chip->tcpci);

I don't think this is sufficient. The interrupt handler always returns
IRQ_HANDLED, even if the interrupt status was 0 and the handler did not do
anything. It should return IRQ_NONE in that case.

Guenter
Xu Yang Dec. 12, 2022, 9:59 a.m. UTC | #2
Hi Guenter,

> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: Sunday, December 11, 2022 11:56 PM
> To: Xu Yang <xu.yang_2@nxp.com>; heikki.krogerus@linux.intel.com
> Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Jun Li <jun.li@nxp.com>
> Subject: [EXT] Re: [PATCH] usb: typec: tcpci: Request IRQ with IRQF_SHARED
> 
> Caution: EXT Email
> 
> On 12/7/22 23:16, Xu Yang wrote:
> > Under resource constraints, this interrupt may use other interrupt line
> > or this interrupt line may be shared with other devices as long as they
> > meet the sharing requirements. Besides, This irq flag will not cause other
> > side effect if tcpci driver is the only user. So a kindly wish to add this
> > flag.
> 
> The last sentence is not appropriate for a commit description.

Will remove it.

> 
> >
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > ---
> >   drivers/usb/typec/tcpm/tcpci.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> > index fe781a38dc82..223a1de4fb1d 100644
> > --- a/drivers/usb/typec/tcpm/tcpci.c
> > +++ b/drivers/usb/typec/tcpm/tcpci.c
> > @@ -838,7 +838,7 @@ static int tcpci_probe(struct i2c_client *client)
> >
> >       err = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> >                                       _tcpci_irq,
> > -                                     IRQF_ONESHOT | IRQF_TRIGGER_LOW,
> > +                                     IRQF_SHARED | IRQF_ONESHOT | IRQF_TRIGGER_LOW,
> >                                       dev_name(&client->dev), chip);
> >       if (err < 0) {
> >               tcpci_unregister_port(chip->tcpci);
> 
> I don't think this is sufficient. The interrupt handler always returns
> IRQ_HANDLED, even if the interrupt status was 0 and the handler did not do
> anything. It should return IRQ_NONE in that case.

Yes, you are right. Thanks for your suggestion and I will preprare v2 for this.

Thanks
Xu Yang

> 
> Guenter
diff mbox series

Patch

diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
index fe781a38dc82..223a1de4fb1d 100644
--- a/drivers/usb/typec/tcpm/tcpci.c
+++ b/drivers/usb/typec/tcpm/tcpci.c
@@ -838,7 +838,7 @@  static int tcpci_probe(struct i2c_client *client)
 
 	err = devm_request_threaded_irq(&client->dev, client->irq, NULL,
 					_tcpci_irq,
-					IRQF_ONESHOT | IRQF_TRIGGER_LOW,
+					IRQF_SHARED | IRQF_ONESHOT | IRQF_TRIGGER_LOW,
 					dev_name(&client->dev), chip);
 	if (err < 0) {
 		tcpci_unregister_port(chip->tcpci);