Messages in this thread | | | Date | Wed, 25 Jan 2023 15:36:58 -0800 | Subject | Re: [PATCH 2/2] drm/msm/dp: Return IRQ_NONE for unhandled interrupts | From | Kuogee Hsieh <> |
| |
On 1/25/2023 10:21 AM, Doug Anderson wrote: > Hi, > > On Wed, Jan 25, 2023 at 9:22 AM Kuogee Hsieh <quic_khsieh@quicinc.com> wrote: >>> -void dp_ctrl_isr(struct dp_ctrl *dp_ctrl) >>> +irqreturn_t dp_ctrl_isr(struct dp_ctrl *dp_ctrl) >>> { >>> struct dp_ctrl_private *ctrl; >>> u32 isr; >>> + irqreturn_t ret = IRQ_NONE; >>> >>> if (!dp_ctrl) >>> - return; >>> + return IRQ_NONE; >>> >>> ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); >>> >>> isr = dp_catalog_ctrl_get_interrupt(ctrl->catalog); >> can you add (!isr) check and return IRQ_NONE here to be consistent with >> dp_aux_isr()? > I could, though it doesn't really buy us a whole lot in this case and > just adds an extra test that's not needed. Here it should be easy for > someone reading the function to see that if "isr == 0" that neither of > the two "if" statements below will fire and we'll return "IRQ_NONE" > anyway. > > ...that actually made me go back and wonder whether we still needed > the "if" test in dp_aux_isr() or if it too was also redundant. It > turns out that it's not! The previous patch made dp_aux_irq() detect > unexpected interrupts. Thus the "if (!isr)" test earlier is important > because otherwise we'd end up WARNing "Unexpected interrupt: > 0x00000000" which would be confusing. > > So unless you or others feel strongly that I should add the redundant > test here, I'd rather keep it off. Let me know. > > -Doug ack
| |