RE: [PATCH v3 07/12] staging: typec: tcpci: register port before request irq

[Jun Li]

Hi
> -----Original Message-----
> From: Guenter Roeck [mailto:groe...@gmail.com] On Behalf Of Guenter
> Roeck
> Sent: 2018年3月15日 12:51
> To: Jun Li <jun...@nxp.com>
> Cc: robh...@kernel.org; mark.rutl...@arm.com;
> gre...@linuxfoundation.org; heikki.kroge...@linux.intel.com;
> a.ha...@samsung.com; yue...@google.com; shufan_...@richtek.com;
> o_leve...@orange.fr; linux-usb@vger.kernel.org; dl-linux-imx
> <linux-...@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.c...@nxp.com>
> > Signed-off-by: Li Jun <jun...@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
> >