On 02/08/2023 09:49, Dan Carpenter wrote: > On Wed, Aug 02, 2023 at 09:27:17AM +0200, Hans Verkuil wrote: >> Hmm, this is old code and when digging into this I noticed inconsistencies >> in how it is used and how it is documented. >> >> It's used by the v4l2_subdev interrupt_service_routine callback. The header >> include/media/v4l2-subdev.h claims that it is called from IRQ context, but this >> isn't true in most cases. It's used primarily to pass on interrupts meant for >> i2c video receivers/transmitters that arrive at the bridge chip to the actual >> i2c device. And that happens in work handlers that allow the use of locking. >> >> I found one exception where it is also used for an IR (infrared) receiver and >> there it is actually called from IRQ context. It doesn't use i2c or sleeps/locking >> in that case, so it is safe. >> >> But what confuses me is that you state 'This looks at functions which are passed to >> request_irq()', but adv7511_isr isn't passed to request_irq. So why is this warning >> raised? >> > > Actually you're right. I was thrown off a bit by the name. > > Was the exception cx23885_irq()? Because that's what Smatch is > concerned about. That calls "v4l2_subdev_call(dev->sd_ir, core, > interrupt_service_routine," and Smatch thinks interrupt_service_routine > can point to here. Indeed, that's the exception. I wasn't even aware of this myself until I started grepping the code. I wonder if there is better kernel support for things like this, i.e. the irq from one device is hooked up to another device. Typical for PCI devices where the i2c devices on the PCI card route their interrupt lines to the PCI bridge device. An alternative is to change the sd_ir handling so that it doesn't use the interrupt_service_routine callback and then change the documentation for the interrupt_service_routine callback to allow sleeping. It's rather ugly the way these two modes are mixed today. Regards, Hans > > Thanks for looking at this. It's a new check I'm working on and it > still has a bunch of issues. > > regards, > dan carpenter >
