Re: [PATCH v7 14/34] i2c: tegra: Clean up probe function

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1493 bytes --]

On Thu, Sep 17, 2020 at 04:46:45PM +0300, Andy Shevchenko wrote:
> On Thu, Sep 17, 2020 at 3:37 PM Thierry Reding <thierry.reding@gmail.com> wrote:
> > On Wed, Sep 09, 2020 at 01:39:46AM +0300, Dmitry Osipenko wrote:
> 
> ...
> 
> > > +     ret = devm_request_irq(&pdev->dev, i2c_dev->irq, tegra_i2c_isr,
> > > +                            IRQF_NO_SUSPEND, dev_name(&pdev->dev),
> > > +                            i2c_dev);
> > > +     if (ret)
> > > +             return ret;
> >
> > Is it safe to install the interrupt handler at this point? What if,
> > perhaps because some bootloader didn't properly quiesce the I2C
> > controller, an interrupt triggers immediately after this?
> 
> It\s easy to check with debug shared IRQ, but here IRQ is not shared...
> So, with a hack into the code and enabled debug it will be achievable to test.
> 
> And you probably meant that IRQ triggers *before* the handler is in place?

It shouldn't be possible for the interrupt to trigger before the handler
is in place, because requesting the IRQ here is what will unmask the IRQ
at the parent.

I'm more concerned that the hardware may be in some state where it
already has a pending interrupt and therefore unmasking (as part of the
request here) is going to immediately trigger an interrupt. If we
haven't set up i2c_dev completely at this point this may cause issues
because the interrupt handler will now have to deal with a partially
initialized structure.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]