Re: [PATCH] usb: typec: tcpci: fix NULL pointer issue on shared irq case

[Xu Yang <xu.yang_2@nxp.com>]

On Wed, Dec 11, 2024 at 10:07:55PM +0800, Xu Yang wrote:
> On Wed, Dec 11, 2024 at 12:51:06PM +0100, Francesco Dolcini wrote:
> > On Wed, Dec 11, 2024 at 07:40:51PM +0800, Xu Yang wrote:
> > > On Wed, Dec 11, 2024 at 12:09:28PM +0100, Francesco Dolcini wrote:
> > > > Hello,
> > > > 
> > > > On Wed, Dec 11, 2024 at 06:59:53PM +0800, Xu Yang wrote:
> > > > > The tcpci_irq() may meet below NULL pointer dereference issue:
> > > > > 
> > > > > [    2.641851] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000010
> > > > > [    2.641951] status 0x1, 0x37f
> > > > > [    2.650659] Mem abort info:
> > > > > [    2.656490]   ESR = 0x0000000096000004
> > > > > [    2.660230]   EC = 0x25: DABT (current EL), IL = 32 bits
> 
> [...]
> 
> > > > 
> > > > I do not think this is the correct fix, what about using
> > > > IRQF_NO_AUTOEN ? Would it solve the issue? You need to adjust also the
> > > > disable/enable irq accordingly.
> > > 
> > > Not work. Probe failed directly.
> > > 
> > > [    2.646391] tcpci 2-0050: probe with driver tcpci failed with error -22
> > > [    2.680086] tcpci 2-0051: probe with driver tcpci failed with error -22
> > 
> > Ok, clear. The reason is the shared IRQ.
> > 
> > I think your change will break the support for edge IRQ, because we have
> > interrupt generated calling tcpci_register_port and they will just be lost if
> > the interrupt is not already requested.
> 
> Do you really meet issue where interrupts got lost?
> 
> I can't find the case because ALERT_MASK is set to 0 firstly, and
> ALERT_MASK is assigned some value in tcpci_init() which should be
> the last step of tcpm_register_port(). So the interrupts are masked
> before tcpci_register_port() completed. That's say, typec chip can't
> pull down ALERT line during this stage even though you don't call
> disable_irq(). This behavior is suit for both level and edge type
> interrupt.

Well, above assumption is based on commit 77e85107a771 ("usb: typec:
tcpci: support edge irq"). I think the irq may be missed with this
patch. A better way may be to lookup ALERT and handle event before
probe() finishes.

Thanks,
Xu Yang