-----Original Message----- From: Wajdeczko, Michal <[email protected]> Sent: Saturday, June 6, 2026 3:40 AM To: Cavitt, Jonathan <[email protected]>; [email protected]; [email protected] Cc: Gupta, Saurabhg <[email protected]>; Zuo, Alex <[email protected]>; Jadav, Raag <[email protected]> Subject: Re: [PATCH v3 5/5] drm/xe/i2c: Report i2c irq handler issue > > On 6/5/2026 11:05 PM, Jonathan Cavitt wrote: > > Error logging is expected but not included in xe_i2c_irq_handler() for > > generic_handle_irq_safe(), so add error logging there. > > > > This issue was caught by static analysis. > > hmm, what exactly was this tool complaining about?
It is complaining that xe_i2c_irq_handler() is the only place where the return value for generic_handle_irq_safe() is not checked. > > almost no other callers of the generic_handle_irq_safe() are printing any > errors > (which are more about programming mistakes than runtime failures) > > I'm asking, as if we believe that such errors are legit then maybe we > shouldn't > > "Deassert after I2C adapter clears the interrupt" Is this not also the case for every other caller of generic_handle_irq_safe()? If generic_handle_irq_safe() fails, and it fails for a legitimate reason, we really should be doing something about that other than just logging the error and moving on. Since we don't, based on historical usage, I don't think whatever error generic_handle_irq_safe() can throw is a legitimate issue that requires intervention. But you don't have to believe just me about it. Raag Jadav also approves of this implementation where we don't do anything about the error other than just log it, and he's the one that wrote xe_i2c_irq_handler(). -Jonathan Cavitt > > > > > v2: > > - Reword error message (Wajdeczko) > > > > Signed-off-by: Jonathan Cavitt <[email protected]> > > Cc: Raag Jadav <[email protected]> > > Cc: Michal Wajdeczko <[email protected]> > > --- > > drivers/gpu/drm/xe/xe_i2c.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/xe/xe_i2c.c b/drivers/gpu/drm/xe/xe_i2c.c > > index 148e82e01ae8..84171021e6ea 100644 > > --- a/drivers/gpu/drm/xe/xe_i2c.c > > +++ b/drivers/gpu/drm/xe/xe_i2c.c > > @@ -177,12 +177,15 @@ static bool xe_i2c_irq_present(struct xe_device *xe) > > void xe_i2c_irq_handler(struct xe_device *xe, u32 master_ctl) > > { > > struct xe_mmio *mmio = xe_root_tile_mmio(xe); > > + int ret; > > > > if (!(master_ctl & I2C_IRQ) || !xe_i2c_irq_present(xe)) > > return; > > > > /* Forward interrupt to I2C adapter */ > > - generic_handle_irq_safe(xe->i2c->adapter_irq); > > + ret = generic_handle_irq_safe(xe->i2c->adapter_irq); > > + if (ret) > > + xe_err_ratelimited(xe, "I2C: irq handling failure (%pe)\n", > > ERR_PTR(ret)); > > > > /* Deassert after I2C adapter clears the interrupt */ > > xe_mmio_rmw32(mmio, I2C_CONFIG_CMD, 0, PCI_COMMAND_INTX_DISABLE); > >
