RE: [PATCH v2 net] ptp: netc: fix potential interrupt storm caused by incorrect unbind order

> > diff --git a/drivers/ptp/ptp_netc.c b/drivers/ptp/ptp_netc.c
> > --- a/drivers/ptp/ptp_netc.c
> > +++ b/drivers/ptp/ptp_netc.c
> > @@ -769,6 +769,7 @@ static void netc_timer_init(struct netc_timer *priv)
> >  		   TMR_CTRL_TE | TMR_CTRL_FS;
> >  	netc_timer_wr(priv, NETC_TMR_CTRL, tmr_ctrl);
> >  	netc_timer_wr(priv, NETC_TMR_PRSC, priv->oclk_prsc);
> > +	netc_timer_wr(priv, NETC_TMR_TEMASK, 0);
> 
> [Severity: High]
> This is a pre-existing issue, but does this initialization also need to clear
> the event register (NETC_TMR_TEVENT)?
> 
> If the hardware holds pending events from a previous unbind or an unclean
> shutdown, clearing the mask here won't clear the pending bits themselves,
> leaving stale interrupt status in the hardware.
> 

This is a false positive, TMR_TEMASK provides control over which possible
interrupt events in TMR_TEVENT are permitted to participate in generating
hardware interrupts to the interrupt controller, so no need to clear the bits
are set in TMR_TEVENT but not in TMR_TEMASK. And the driver initializes
TMR_TEMASK to 0 before calling enable_irq().

> [ ... ]
> 
> > @@ -901,6 +902,11 @@ static irqreturn_t netc_timer_isr(int irq, void *data)
> >  	/* Clear interrupts status */
> >  	netc_timer_wr(priv, NETC_TMR_TEVENT, tmr_event);
> >
> > +	if (!tmr_event) {
> > +		spin_unlock(&priv->lock);
> > +		return IRQ_NONE;
> > +	}
> > +
> 
> [Severity: High]
> This is a pre-existing issue, but does writing back the masked tmr_event
> fail to clear pending but masked hardware events?
> 
> Looking earlier in netc_timer_isr(), tmr_event is masked before being written
> back to the register:
> 
>     tmr_event = netc_timer_rd(priv, NETC_TMR_TEVENT);
>     tmr_event &= priv->tmr_emask;
>     /* Clear interrupts status */
>     netc_timer_wr(priv, NETC_TMR_TEVENT, tmr_event);
> 
> If NETC_TMR_TEVENT is a Write-1-to-Clear register, writing back 0 for
> any masked events means they are never cleared.
> 
> If priv->tmr_emask is 0, the ISR will write 0 to NETC_TMR_TEVENT and then
> return IRQ_NONE without clearing the actual hardware state. Could this
> cause the unacknowledged hardware interrupt to re-trigger indefinitely,
> forcing the kernel's spurious interrupt detector to permanently disable the
> IRQ line?
> 

This is a false positive, priv->tmr_emask is equal to the value of TMR_TEMASK
, if the interrupt is not enabled in TMR_TEMASK, the hardware will not generate
the interrupt when the corresponding bit is set in TMR_TEVENT.