msleep() in interrupt handler?

[victorascroft at gmail.com]

On 15-08-20 21:44:14, Woody Wu wrote:
> On Thursday, August 20, 2015, <victorascroft at gmail.com> wrote:
> 
> > On 15-08-20 19:02:50, Woody Wu wrote:
> > > On Thursday, August 20, 2015, John de la Garza <john at jjdev.com
> > <javascript:;>> wrote:
> > >
> > > > On Thu, Aug 20, 2015 at 01:45:34PM +0800, Woody Wu wrote:
> > > > > I did not see the message.  Actually my interrupt handler is calling
> > > > > i2c_transfer which in turn used msleep() somewhere in its code.  Is
> > this
> > > > > normal or dangerous?
> > > >
> > > > Can you have the interrupt handler put the work on a workqueue
> > > > and quickly return?
> > > >
> > >
> > > Yes, that is an option.  But I firstly need to know the old code is
> > really
> > > bad. The interrupt is triggered by an i2c touchscreen, and  the interrupt
> > > handler use the i2c core code to start the i2c transferring.  I see in
> > the
> > > i2c adapter code a msleep() was invoked at beginning of transfer.  I
> > doubt
> > > that this is a potential problem.  But you know the i2c touchscreen
> > driver
> > > code is also part of the mainline, so I am not sure my option.  You guys
> > > can check the code of atmel_mXT224_ts.c, the i2c adapter code is
> > i2c_s3c.c
> >
> > I checked the code. The kernel release I am checking in is 4.1.5. From what
> > I can see there is only atmel_mxt_ts.c and not atmel_mXT224_ts.c in
> > drivers/
> > input/touchscreen. In this code, it is requesting a threaded irq with the
> > top handler being specified as null and the bottom handler specified.
> >
> > Since the bottom handler is being used where i2c_transfer is called and
> > as such though on a quick check I do not see the msleep() call, even if
> > the msleep were called while in the bottom handler context it would be
> > fine.
> >
> > I do not know which code you are referring to but in hard interrupt context
> > atleast you can never ever call any function which can sleep. It is just
> > gonna blow in some way.
> >
> > - Sanchayan.
> >
> 
> The file name you said is right.  The kernel version I am using is 3.1.x,
> but I guess it does no much matter to the question. The interrupt handler
> of the atmel_mxt_ts called i2c_transfer() which indeed called the actual
> i2c adapter's transfer method. In my platform, the i2c adapter is a s3c i2c
> controller, so I was checking the code in i2c/busses/i2c_s3c.c, from this
> file I saw the msleep() was called in  i2c_doxfer()->i2c_set_master() call
> sequence. I think you can find he similar things in 4.1.5.

Yes right atmel_mxt_ts does call i2c_transfer. I did not check myself but even
if the call chain results in msleep() getting called somewhere this would not
be in the top irq handler. So mlseep() is ok. Had this been in top irq handler
(which it will never be in the kernel) then it will just not work at all as
the kernel will crash.

Check all drivers in touchscreen. All of them do not use the top irq handler
and use a bottom handler specified with threaded irq request. So it is fine
if msleep() is getting called somewhere down that line.

Also as far as I know none of the touchscreen drivers in drivers/input/touchscreen
use the irq handler plus workqueue approach anymore. Also if one were to try
and submit such a one to the mainline you will be just asked to convert to
a threaded irq approach. Just some info since I saw a recommendation on going
with irq + workqueue approach. However I dont know if threaded irqs existed in
3.1.x.

- Sanchayan.