tsc2007 driver problem with workqueue "Unable to handle kernel paging request at virtual address"

Hi Ali,

I forwarded your problem to an export on Linux, and asked for help. The following is his analysis:

"

I am looking at my kernel that I use for OMAP development (2.6.33) ...    In a cursory check, the panic trace suggests that the i2c_transfer function is 232 bytes long, where mine is 240 bytes long, a discrepancy of 4 bytes.  Right now I am trying 2.6.32 to see if it might match.  Right now I am working out a build on that version... so while I do this, I'll critique what I see here...

First, with
static unsigned int short normal_i2c[] = { 0x48, I2C_CLIENT_END };
...
static DECLARE_WORK(work, (void (*)(void *))tsc2007_wq_function, &normal_i2c);
static void tsc2007_wq_function(void *arg)

What I see here is that this person is creating a task to deal with the data (&normal_i2c) which is then being passed to the work queue function, and then being cast...

static void tsc2007_wq_function(void *arg)
{
 struct tsc2007_data *data = (struct tsc2007_data *)arg;

... to a tsc2007_data.   It's an array of unsigned short ints.  I have no idea (I searched for it, didn't find it) what type the tsc2007_data is; or more to the point, what this structure contains.  I am convinced that it is not as small as the array being passed to the function, observing that within that structure there is an i2c_client structure which we take the address of here:

 struct i2c_client *client = &data->client;

This is declared in include/linux/i2c.h...

   188  struct i2c_client {
   189          unsigned short flags;           /* div., see below              */
   190          unsigned short addr;            /* chip address - NOTE: 7bit    */
   191                                          /* addresses are stored in the  */
   192                                          /* _LOWER_ 7 bits               */
   193          char name[I2C_NAME_SIZE];
   194          struct i2c_adapter *adapter;    /* the adapter we sit on        */
   195          struct i2c_driver *driver;      /* and our access routines      */
   196          struct device dev;              /* the device structure         */
   197          int irq;                        /* irq issued by device         */
   198          struct list_head detected;
   199  };

So, supposing that the client structure is the first element in the tsc2007_data structure, immediately the address that we're going to use is I2C_CLIENT_END (assuming we don't have any padding between elements) which is not right.

Now that the build is complete, I've found that the i2c_transfer function blew up on line 1084  (assuming that I am using the right kernel version, which I'm not... this one's i2c_transfer function is 220 bytes long).  This does look correct to me, considering the type of oops/panic that we're seeing and the code, even though the kernel that I've built is different in this function:

In drivers/i2c/i2c-core.c:
  1062  int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
  1063  {
  1064          unsigned long orig_jiffies;
  1065          int ret, try;
  1066
  1067          /* REVISIT the fault reporting model here is weak:
...
  1082           */
  1083
*1084          if (adap->algo->master_xfer) {

What we see going on here in 1084 is a check to see whether adap->algo->master_xfer is zero.  The field adap is taken straight out of i2c_client, deeper into the structure than the pointer to (two unsigned short ints) normal_i2c will support.

I'd like to see the way this function is being called in the timer function... it's possible that it's being called correctly there, but I can guarantee you it's not being called correctly in this instance.   The data that needs to be passed to DECLARE_WORK should be a pointer to a properly-populated tsc2007_data structure, but right now, it's a pointer to an unsigned int array of two elements.

"

Hi,

I wonder if this problem has been resolved. I experience something similar on beagleboard clone board with tsc2007 on i2c bus 2 using Linux2.6.29-omap1. The driver works fine except for reading tsc2007 data via i2c WHILE inside timer function. I reads tsc2007 OK inside probe() but not when inside timer function where it causes kernel dump. Timer function looks like

static void tsc2007_timer( unsigned long data ){

       unsigned long flags;

       struct tsc2007 *ts = (struct tsc2007 *)data;

       if (unlikely(!ts->get_pendown_state() && ts->pendown)) 

       {

              struct input_dev *input = ts->input;

              dev_dbg(&ts->client->dev, "UP\n");

              input_report_key(input, BTN_TOUCH, 0);

              input_report_abs(input, ABS_PRESSURE, 0);

              input_sync(input);

              ts->pendown = 0;

              enable_irq( ts->irq );

              ts->irq_disabled = 0;

       } else {

              // pen is still down, continue with the measurement 

              dev_dbg(&ts->client->dev, "DOWN\n");

              tsc2007_read_values( ts ); // this makes kernel crash  

              tsc2007_send_event(ts);

       }

}   

Here is the kernel dump caused by tsc2007_read_values( ts ) call

[ 2750.183441] ------------[ cut here ]------------

[ 2750.188110] WARNING: at kernel/mutex.c:351 mutex_trylock+0x68/0x13c()

[ 2750.194580] Modules linked in: tsc2007 g_zero [last unloaded: tsc2007]

[ 2750.201171] [<c035da80>] (dump_stack+0x0/0x14) from [<c005bfa8>] (warn_slowpath+0x68/0x9c)

[ 2750.209533] [<c005bf40>] (warn_slowpath+0x0/0x9c) from [<c035ed74>] (mutex_trylock+0x68/0x13c)

[ 2750.218231]  r3:c04e636c r2:00000000

[ 2750.221862]  r7:00000001 r6:cfb2ee38 r5:00000049 r4:cfb2ee58

[ 2750.227600] [<c035ed0c>] (mutex_trylock+0x0/0x13c) from [<c022a7a0>] (i2c_transfer+0xec/0x164)

[ 2750.236297]  r6:cfb2ee38 r5:00000049 r4:00000001

[ 2750.240966] [<c022a6b4>] (i2c_transfer+0x0/0x164) from [<c022b540>] (i2c_master_send+0x44/0x54     )

[ 2750.249755] [<c022b4fc>] (i2c_master_send+0x0/0x54) from [<bf011078>] (tsc2007_read_values+0x2     c/0x418 [tsc2007])

[ 2750.260040]  r4:cfbb31c0

[ 2750.262573] [<bf01104c>] (tsc2007_read_values+0x0/0x418 [tsc2007]) from [<bf011880>] (tsc2007_     timer+0xe0/0x2a4 [tsc2007])

[ 2750.273651]  r5:cfbb31c0 r4:00000103

[ 2750.277282] [<bf0117a0>] (tsc2007_timer+0x0/0x2a4 [tsc2007]) from [<c00660dc>] (run_timer_soft     irq+0x1ac/0x250)

[ 2750.287384] [<c0065f30>] (run_timer_softirq+0x0/0x250) from [<c0061ac0>] (__do_softirq+0x6c/0x     10c)

[ 2750.296417] [<c0061a54>] (__do_softirq+0x0/0x10c) from [<c0061ba8>] (irq_exit+0x48/0x94)

[ 2750.304595] [<c0061b60>] (irq_exit+0x0/0x94) from [<c003705c>] (__exception_text_start+0x5c/0x     70)

[ 2750.313568] [<c0037000>] (__exception_text_start+0x0/0x70) from [<c0037a84>] (__irq_svc+0x44/0     xa4)

[ 2750.322631] Exception stack(0xc0473f20 to 0xc0473f68)

[ 2750.327697] 3f20: 2b4e76c4 a0000013 0051f4c5 00000000 c0043ad0 c0472000 c047603c c0038f1c

[ 2750.336120] 3f40: c04a9bc8 411fc082 0000001f c0473f8c c0473f90 c0473f68 c0038edc c0043adc

[ 2750.344512] 3f60: 60000013 ffffffff

[ 2750.352905]  r5:d8200000 r4:ffffffff

[ 2750.356506] [<c0043ad0>] (omap3_pm_idle+0x0/0x130) from [<c0038edc>] (cpu_idle+0x50/0x90)

[ 2750.364776] [<c0038e8c>] (cpu_idle+0x0/0x90) from [<c0359924>] (rest_init+0x6c/0x80)

[ 2750.372619]  r8:8002a3c4 r7:c0475ed0 r6:c002bea4 r5:c04a9764 r4:c04e34d4

[ 2750.379394] [<c03598b8>] (rest_init+0x0/0x80) from [<c0008968>] (start_kernel+0x25c/0x2b8)

[ 2750.387756] [<c000870c>] (start_kernel+0x0/0x2b8) from [<80008034>] (0x80008034)

[ 2750.395233] ---[ end trace 91dd314bb23dd928 ]---