Re: [PATCH 1/2] INPUT/HID: add touch support for SiS touch driver

On Fri, 2015-01-16 at 18:59 +0800, 曾婷葳 (tammy_tseng) wrote:

Hey, Oliver

On Mon, Jan 12, 2015 at 7:50 PM, Oliver Neukum [off-list ref] wrote:
On Mon, 2015-01-12 at 18:53 +0800, 曾婷葳 (tammy_tseng) wrote:

quoted

quoted

(Skip the code diff...)

Again macros for endianness

And the driver has a great number of conditional compilations are they 
really needed? The driver as is has a number of issues and is hard to 
review due to the use of "//" for comments and a lot of conditional 
compilation and unnecessary variables used for constants. Could you 
fix this up and resubmit?

Thanks for the reply. I've modified the code (sis_i2c.c/sis_i2c.h/Kconfig/Makefile) to fix them.
Please help check them if anything needs to fix.
Thanks.

You still have conditional compilation needlessly. You still haven't
addressed the issues I told you about during the review of the last
version.

	Oliver

Please use the macros for endianness conversion.

   
+	}
+		
+#ifdef _DEBUG_REPORT
+	for (i = 0; i < TPInfo->fingers; i++)
+	{
+		 printk(KERN_INFO "chaoban test: i = %d, id = %d, x = %d, y = %d, pstatus = %d, width = %d, height = %d, pressure = %d, \n", i, TPInfo->pt[i].id, TPInfo->pt[i].x, TPInfo->pt[i].y, buf[pstatus], TPInfo->pt[i].Width,  TPInfo->pt[i].Height, TPInfo->pt[i].Pressure);
+	}
+#endif
+
+label_send_report:
+/* Report co-ordinates to the multi-touch stack */
+#ifdef _ANDROID_4	
+	for(i = 0; ((i < TPInfo->fingers) && (i < MAX_FINGERS)); i++)
+	{
+		if(TPInfo->pt[i].Pressure)
+		{
+			TPInfo->pt[i].Width *= AREA_UNIT;	
+			input_report_abs(ts->input_dev, ABS_MT_TOUCH_MAJOR, TPInfo->pt[i].Width);
+			TPInfo->pt[i].Height *= AREA_UNIT;
+			input_report_abs(ts->input_dev, ABS_MT_TOUCH_MINOR, TPInfo->pt[i].Height);			
+			input_report_abs(ts->input_dev, ABS_MT_PRESSURE, TPInfo->pt[i].Pressure);
+			input_report_abs(ts->input_dev, ABS_MT_POSITION_X, TPInfo->pt[i].x);
+			input_report_abs(ts->input_dev, ABS_MT_POSITION_Y, TPInfo->pt[i].y);
+			input_report_abs(ts->input_dev, ABS_MT_TRACKING_ID, TPInfo->pt[i].id);
+			input_mt_sync(ts->input_dev);
+			all_touch_up = false;
+		}
+		
+		if (i == (TPInfo->fingers -1) && all_touch_up == true)
+		{
+			input_mt_sync(ts->input_dev);
+		}
+	}
+
+	if(TPInfo->fingers == 0)
+	{
+		input_mt_sync(ts->input_dev);
+	}
+#else
+	i = 0;
+	do
+	{
+		input_report_abs(ts->input_dev, ABS_MT_TOUCH_MAJOR, TPInfo->pt[i].Pressure);
+		input_report_abs(ts->input_dev, ABS_MT_WIDTH_MAJOR, TPInfo->pt[i].Width);
+		input_report_abs(ts->input_dev, ABS_MT_WIDTH_MINOR, TPInfo->pt[i].Height);
+		input_report_abs(ts->input_dev, ABS_MT_POSITION_X, TPInfo->pt[i].x);
+		input_report_abs(ts->input_dev, ABS_MT_POSITION_Y, TPInfo->pt[i].y);
+		input_report_abs(ts->input_dev, ABS_MT_TRACKING_ID, TPInfo->pt[i].id); /* Android 2.3 */
+		input_mt_sync(ts->input_dev);
+		i++;
+	}
+	while ((i < TPInfo->fingers) && (i < MAX_FINGERS));
+#endif
+	input_sync(ts->input_dev);
+
+err_free_allocate:
+
+    if (ts->use_irq)
+    {
+/* case 1 mode */
+#ifdef _INT_MODE_1 
+	    /*TODO: After interrupt status low, read i2c bus data by polling, until interrupt status is high */
+	    ret = check_gpio_interrupt();	/* interrupt pin is still LOW, read data until interrupt pin is released. */
+	    if (!ret)
+	    {
+	        hrtimer_start(&ts->timer, ktime_set(0, TIMER_NS), HRTIMER_MODE_REL);
+	    }
+	    else
+	    {
+			if (TPInfo->pre_keybit_state)
+			{
+				ts_report_key(ts->client, 0x0);	/* clear for interrupt */
+			}
+			
+#if ( LINUX_VERSION_CODE < KERNEL_VERSION (2, 6, 39) )
+        	if ((ts->desc->status & IRQ_DISABLED) == IRQ_STATUS_DISABLED)
+#else
+        	if ((ts->desc->irq_data.state_use_accessors & IRQD_IRQ_DISABLED) == IRQ_STATUS_DISABLED)
+#endif
+        	{
+				enable_irq(ts->client->irq);
+			}
+	    }
+/* case 2 mode */
+#else
+
+#if ( LINUX_VERSION_CODE < KERNEL_VERSION (2, 6, 39) )
+		if ((ts->desc->status & IRQ_DISABLED) == IRQ_STATUS_DISABLED)
+#else
+		if ((ts->desc->irq_data.state_use_accessors & IRQD_IRQ_DISABLED) == IRQ_STATUS_DISABLED)
+#endif
+		{
+			enable_irq(ts->client->irq);
+		}
+#endif
+	}
+
+	mutex_unlock(&ts->mutex_wq);
+    return;
+}
+
+static void sis_tpinfo_clear(struct sisTP_driver_data *TPInfo, int max)
+{
+	int i = 0;
+	for(i = 0; i < max; i++)
+	{
+		TPInfo->pt[i].id = -1;
+		TPInfo->pt[i].x = 0;
+		TPInfo->pt[i].y = 0;
+		TPInfo->pt[i].Pressure = 0;
+		TPInfo->pt[i].Width = 0;
+	}
+	TPInfo->id = 0x0;
+	TPInfo->fingers = 0;
+}
+
+static enum hrtimer_restart sis_ts_timer_func(struct hrtimer *timer)
+{
+	struct sis_ts_data *ts = container_of(timer, struct sis_ts_data, timer);
+	queue_work(sis_wq, &ts->work);
+	if (!ts->use_irq)
+	{	/* For Polling mode */
+	    hrtimer_start(&ts->timer, ktime_set(0, TIMER_NS), HRTIMER_MODE_REL);
+	}
+	return HRTIMER_NORESTART;
+}
+
+static irqreturn_t sis_ts_irq_handler(int irq, void *dev_id)
+{
+	struct sis_ts_data *ts = dev_id;
+	
+#if ( LINUX_VERSION_CODE < KERNEL_VERSION (2, 6, 39) )
+	if ((ts->desc->status & IRQ_DISABLED) == IRQ_STATUS_ENABLED)
+#else
+	if ((ts->desc->irq_data.state_use_accessors & IRQD_IRQ_DISABLED) == IRQ_STATUS_ENABLED)
+#endif
+	{
+		disable_irq_nosync(ts->client->irq);
+	}
+	queue_work(sis_wq, &ts->work);
+		
+	return IRQ_HANDLED;
+}
+
+static int initial_irq(void)
+{
+	int ret = 0;
+#ifdef _I2C_INT_ENABLE
+	/* initialize gpio and interrupt pins */
+	ret = gpio_request(GPIO_IRQ, "GPIO_133");	/* ex. GPIO_133 for interrupt mode */
+	if (ret < 0)
+	{
+		/* Set Active Low. Please reference the file include/linux/interrupt.h */
+	}
+	/* setting gpio direction here OR boardinfo file*/
+
+#else
+	ret = -1;
+#endif
+	return ret;
+}
+
+uint16_t cal_crc (char* cmd, int start, int end)
+{
+	int i = 0;
+	uint16_t crc = 0;
+	
+	for (i = start; i <= end ; i++)
+	{
+		crc = (crc<<8) ^ crc16tab[((crc>>8) ^ cmd[i] )&0x00FF];
+	}
+	
+	return crc;
+}

How about making this CRC generally available?

+
+uint16_t cal_crc_with_cmd (char* data, int start, int end, uint8_t cmd)
+{
+	int i = 0;
+	uint16_t crc = 0;
+	
+	crc = (crc<<8) ^ crc16tab[((crc>>8) ^ cmd)&0x00FF];
+	for (i = start; i <= end ; i++)
+	{
+		crc = (crc<<8) ^ crc16tab[((crc>>8) ^ data[i] )&0x00FF];
+	}
+	
+	return crc;
+}
+
+void write_crc (unsigned char *buf, int start, int end)
+{
+	uint16_t crc = 0;
+	
+	crc = cal_crc (buf, start , end);
+	buf[end+1] = (crc >> 8)& 0xff;
+	buf[end+2] = crc & 0xff;

Please use the macro

+}
+
+#ifdef _STD_RW_IO
+#define BUFFER_SIZE MAX_BYTE
+static ssize_t sis_cdev_write( struct file *file, const char __user *buf, size_t count, loff_t *f_pos )
+{
+	 int ret = 0;
+	 char *kdata;
+	 char cmd;
+	 
+	 printk(KERN_INFO "sis_cdev_write.\n");
+	 
+	 if (ts_bak == 0)
+    	return -13;

You _must_ use symbolic values.

+    	
+    ret = access_ok(VERIFY_WRITE, buf, BUFFER_SIZE);
+    if (!ret) {
+        printk(KERN_ERR "cannot access user space memory\n");
+        return -11;
+    }
+
+	 kdata = kmalloc(BUFFER_SIZE, GFP_KERNEL);
+     if (kdata == 0)
+    	return -12;
+    	
+     ret = copy_from_user(kdata, buf, BUFFER_SIZE);
+     if (ret) {
+        printk(KERN_ERR "copy_from_user fail\n");
+        kfree(kdata);
+        return -14;
+     } 
+#if 0
+	PrintBuffer(0, count, kdata);
+#endif
+		
+	cmd = kdata[6];
+
+/* Write & Read */
+    ret = sis_command_for_write(ts_bak->client, count, kdata);
+    if (ret < 0) {
+        printk(KERN_ERR "i2c_transfer write error %d\n", ret);
+		kfree(kdata);
+		return -21;
+	}
+
+    if ( copy_to_user((char*) buf, kdata, BUFFER_SIZE ) )
+    {
+        printk(KERN_ERR "copy_to_user fail\n" );
+        ret = -19;
+    }
+
+    kfree( kdata );
+
+	return ret;
+}
+
+/* for get system time */
+static ssize_t sis_cdev_read( struct file *file, char __user *buf, size_t count, loff_t *f_pos )
+{
+	 int ret = 0;
+	 char *kdata;
+	 char cmd;
+	 int i;
+	 
+	 printk(KERN_INFO "sis_cdev_read.\n");
+	 
+	 if (ts_bak == 0)
+    	return -13;
+    	
+    ret = access_ok(VERIFY_WRITE, buf, BUFFER_SIZE);
+    if (!ret) {
+        printk(KERN_ERR "cannot access user space memory\n");
+        return -11;
+    }
+
+	 kdata = kmalloc(BUFFER_SIZE, GFP_KERNEL);
+     if (kdata == 0)
+    	return -12;
+    	
+     ret = copy_from_user(kdata, buf, BUFFER_SIZE);
+     if (ret) {
+        printk(KERN_ERR "copy_from_user fail\n");
+        kfree(kdata);
+        return -14;
+     }    
+#if 0
+    PrintBuffer(0, count, kdata);
+#endif
+	 cmd = kdata[6];
+	 /* for making sure AP communicates with SiS driver */
+    if(cmd == 0xa2)
+    {
+		kdata[0] = 5;
+		kdata[1] = 0;
+		kdata[3] = 'S';
+		kdata[4] = 'i';
+		kdata[5] = 'S';
+		if ( copy_to_user((char*) buf, kdata, BUFFER_SIZE ) )
+		{
+			printk(KERN_ERR "copy_to_user fail\n" );
+			kfree( kdata );
+			return -19;
+		}
+
+		kfree( kdata );
+		return 3;	
+	}
+/* Write & Read */
+    ret = sis_command_for_read(ts_bak->client, MAX_BYTE, kdata);
+    if (ret < 0) {
+        printk(KERN_ERR "i2c_transfer read error %d\n", ret);
+		kfree(kdata);
+		return -21;
+	}
+
+	ret = kdata[0] | (kdata[1] << 8);
+
+    printk(KERN_INFO "%d\n", ret);
+
+    for ( i = 0; i < ret && i < BUFFER_SIZE; i++ )
+    {
+        printk("%02x ", kdata[i]);
+    }
+
+    printk( "\n" );
+
+    if ( copy_to_user((char*) buf, kdata, BUFFER_SIZE ) )
+    {
+        printk(KERN_ERR "copy_to_user fail\n" );
+        ret = -19;
+    }
+
+    kfree( kdata );
+
+	return ret;
+}
+
+#undef BUFFER_SIZE
+
+static int sis_cdev_open(struct inode *inode, struct file *filp)
+{
+	printk(KERN_INFO "sis_cdev_open.\n");
+	if ( ts_bak == 0 )
+    	return -13;
+
+	msleep(200);
+	if (ts_bak->use_irq)
+	{
+#if ( LINUX_VERSION_CODE < KERNEL_VERSION (2, 6, 39) )
+        if ((ts_bak->desc->status & IRQ_DISABLED) == IRQ_STATUS_ENABLED)
+#else
+		if ((ts_bak->desc->irq_data.state_use_accessors & IRQD_IRQ_DISABLED) == IRQ_STATUS_ENABLED)
+#endif	
+		{
+			disable_irq(ts_bak->client->irq);
+		}
+		else
+		{
+#if ( LINUX_VERSION_CODE < KERNEL_VERSION (2, 6, 39) )
+			printk(KERN_INFO "sis_cdev_open: IRQ_STATUS: %x\n",(ts_bak->desc->status & IRQ_DISABLED));
+#else
+			printk(KERN_INFO "sis_cdev_open: IRQ_STATUS: %x\n",(ts_bak->desc->irq_data.state_use_accessors & IRQD_IRQ_DISABLED));
+#endif	
+		}
+	}
+	hrtimer_cancel(&ts_bak->timer);
+
+	/* only flush sis_wq */
+	flush_workqueue(sis_wq); 	   
+    
+    msleep(200);
+
+	return 0; /* success */
+}
+
+static int sis_cdev_release(struct inode *inode, struct file *filp)
+{
+	printk(KERN_INFO "sis_cdev_release.\n");
+	
+	msleep(200);
+	
+    if (ts_bak == 0)
+    	return -13;
+
+	if (ts_bak->use_irq)
+	{
+#if ( LINUX_VERSION_CODE < KERNEL_VERSION (2, 6, 39) )
+        if ((ts_bak->desc->status & IRQ_DISABLED) == IRQ_STATUS_DISABLED)
+#else
+		if ((ts_bak->desc->irq_data.state_use_accessors & IRQD_IRQ_DISABLED) == IRQ_STATUS_DISABLED)
+#endif	
+		{
+			enable_irq(ts_bak->client->irq);
+		}
+	}
+	else
+		hrtimer_start(&ts_bak->timer, ktime_set(1, 0), HRTIMER_MODE_REL);
+
+    return 0;
+}
+
+static const struct file_operations sis_cdev_fops = {
+	.owner	= THIS_MODULE,
+	.read	= sis_cdev_read,
+	.write	= sis_cdev_write,
+	.open	= sis_cdev_open,
+	.release= sis_cdev_release,
+};
+
+static int sis_setup_chardev(struct sis_ts_data *ts)
+{
+	
+	dev_t dev = MKDEV(sis_char_major, 0);
+	int alloc_ret = 0;
+	int cdev_err = 0;
+	int input_err = 0;
+	struct device *class_dev = NULL;
+	void *ptr_err;
+	
+	printk("sis_setup_chardev.\n");
+	
+	if (ts == NULL) 
+	{
+	  input_err = -ENOMEM;
+	  goto error;
+	} 
+	/* dynamic allocate driver handle */
+	alloc_ret = alloc_chrdev_region(&dev, 0, sis_char_devs_count, DEVICE_NAME);
+	if (alloc_ret)
+		goto error;
+		
+	sis_char_major = MAJOR(dev);
+	cdev_init(&sis_char_cdev, &sis_cdev_fops);
+	sis_char_cdev.owner = THIS_MODULE;
+	cdev_err = cdev_add(&sis_char_cdev, MKDEV(sis_char_major, 0), sis_char_devs_count);
+	
+	if (cdev_err) 
+		goto error;
+	
+	printk(KERN_INFO "%s driver(major %d) installed.\n", DEVICE_NAME, sis_char_major);
+	
+	/* register class */
+	sis_char_class = class_create(THIS_MODULE, DEVICE_NAME);
+	if(IS_ERR(ptr_err = sis_char_class)) 
+	{
+		goto err2;
+	}
+	
+	class_dev = device_create(sis_char_class, NULL, MKDEV(sis_char_major, 0), NULL, DEVICE_NAME);
+	
+	if(IS_ERR(ptr_err = class_dev)) 
+	{
+		goto err;
+	}
+	
+	return 0;
+error:
+	if (cdev_err == 0)
+		cdev_del(&sis_char_cdev);
+	if (alloc_ret == 0)
+		unregister_chrdev_region(MKDEV(sis_char_major, 0), sis_char_devs_count);
+	if(input_err != 0)
+	{
+		printk("sis_ts_bak error!\n");
+	}
+err:
+	device_destroy(sis_char_class, MKDEV(sis_char_major, 0));
+err2:
+	class_destroy(sis_char_class);
+	return -1;
+}
+#endif
+
+static int sis_ts_probe(
+	struct i2c_client *client, const struct i2c_device_id *id)
+{
+	int ret = 0;
+	struct sis_ts_data *ts = NULL;
+	struct sis_i2c_rmi_platform_data *pdata = NULL;
+
+    printk(KERN_INFO "sis_ts_probe\n");
+
+    TPInfo = kzalloc(sizeof(struct sisTP_driver_data), GFP_KERNEL);
+    if (TPInfo == NULL) 
+    {
+		ret = -ENOMEM;
+		goto err_alloc_data_failed;
+	}
+
+	ts = kzalloc(sizeof(struct sis_ts_data), GFP_KERNEL);
+	if (ts == NULL) 
+	{
+		ret = -ENOMEM;
+		goto err_alloc_data_failed;
+	}
+
+	ts_bak = ts;
+
+	mutex_init(&ts->mutex_wq);
+	
+	/* 1. Init Work queue and necessary buffers */
+	INIT_WORK(&ts->work, sis_ts_work_func);
+	ts->client = client;
+	i2c_set_clientdata(client, ts);
+    pdata = client->dev.platform_data;
+
+	if (pdata)
+		ts->power = pdata->power;
+	if (ts->power) 
+	{
+		ret = ts->power(1);
+		if (ret < 0) 
+		{
+			printk(KERN_ERR "sis_ts_probe power on failed\n");
+			goto err_power_failed;
+		}
+	}
+
+	/* 2. Allocate input device */
+	ts->input_dev = input_allocate_device();
+	if (ts->input_dev == NULL) 
+	{
+		ret = -ENOMEM;
+		printk(KERN_ERR "sis_ts_probe: Failed to allocate input device\n");
+		goto err_input_dev_alloc_failed;
+	}
+
+	/* This input device name should be the same to IDC file name. */
+	ts->input_dev->name = "sis_touch";
+
+	set_bit(EV_ABS, ts->input_dev->evbit);
+	set_bit(EV_KEY, ts->input_dev->evbit);
+    set_bit(ABS_MT_POSITION_X, ts->input_dev->absbit);
+    set_bit(ABS_MT_POSITION_Y, ts->input_dev->absbit);
+    set_bit(ABS_MT_TRACKING_ID, ts->input_dev->absbit);
+
+#ifdef _ANDROID_4
+	set_bit(INPUT_PROP_DIRECT, ts->input_dev->propbit);
+    set_bit(ABS_MT_PRESSURE, ts->input_dev->absbit);
+    set_bit(ABS_MT_TOUCH_MAJOR, ts->input_dev->absbit);
+    set_bit(ABS_MT_TOUCH_MINOR, ts->input_dev->absbit);
+    input_set_abs_params(ts->input_dev, ABS_MT_PRESSURE, 0, PRESSURE_MAX, 0, 0);
+    input_set_abs_params(ts->input_dev, ABS_MT_TOUCH_MAJOR, 0, AREA_LENGTH_LONGER, 0, 0);
+    input_set_abs_params(ts->input_dev, ABS_MT_TOUCH_MINOR, 0, AREA_LENGTH_SHORT, 0, 0);   
+#else
+    set_bit(ABS_MT_TOUCH_MAJOR, ts->input_dev->absbit);
+    set_bit(ABS_MT_WIDTH_MAJOR, ts->input_dev->absbit);
+    set_bit(ABS_MT_WIDTH_MINOR, ts->input_dev->absbit);
+    input_set_abs_params(ts->input_dev, ABS_MT_TOUCH_MAJOR, 0, PRESSURE_MAX, 0, 0);
+    input_set_abs_params(ts->input_dev, ABS_MT_WIDTH_MAJOR, 0, AREA_LENGTH_LONGER, 0, 0);
+    input_set_abs_params(ts->input_dev, ABS_MT_WIDTH_MINOR, 0, AREA_LENGTH_SHORT, 0, 0);
+#endif
+
+    input_set_abs_params(ts->input_dev, ABS_MT_POSITION_X, 0, SIS_MAX_X, 0, 0);
+    input_set_abs_params(ts->input_dev, ABS_MT_POSITION_Y, 0, SIS_MAX_Y, 0, 0);
+    input_set_abs_params(ts->input_dev, ABS_MT_TRACKING_ID, 0, 15, 0, 0);
+	
+    /* add for touch keys */
+	set_bit(KEY_COMPOSE, ts->input_dev->keybit);
+	set_bit(KEY_BACK, ts->input_dev->keybit);
+	set_bit(KEY_MENU, ts->input_dev->keybit);
+	set_bit(KEY_HOME, ts->input_dev->keybit);
+
+	/* 3. Register input device to core */
+	ret = input_register_device(ts->input_dev);
+
+	if (ret) 
+	{
+		printk(KERN_ERR "sis_ts_probe: Unable to register %s input device\n", ts->input_dev->name);
+		goto err_input_register_device_failed;
+	}
+	
+	/* 4. irq or timer setup */
+	ret = initial_irq();
+	if (ret < 0) 
+	{
+
+	}
+	else
+	{
+		client->irq = gpio_to_irq(GPIO_IRQ);
+		ret = request_irq(client->irq, sis_ts_irq_handler, IRQF_TRIGGER_FALLING, client->name, ts);
+		if (ret == 0) 
+		{
+		   ts->use_irq = 1;
+		}
+		else 
+		{
+			dev_err(&client->dev, "request_irq failed\n");
+		}
+	}
+
+	ts->desc = irq_to_desc(ts_bak->client->irq);
+	
+	hrtimer_init(&ts->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	ts->timer.function = sis_ts_timer_func;
+
+	if (!ts->use_irq) 
+	{
+		hrtimer_start(&ts->timer, ktime_set(1, 0), HRTIMER_MODE_REL);
+	}
+
+#ifdef CONFIG_HAS_EARLYSUSPEND
+	ts->early_suspend.level = EARLY_SUSPEND_LEVEL_BLANK_SCREEN + 1;
+	ts->early_suspend.suspend = sis_ts_early_suspend;
+	ts->early_suspend.resume = sis_ts_late_resume;
+	register_early_suspend(&ts->early_suspend);
+#endif
+	printk(KERN_INFO "sis_ts_probe: Start touchscreen %s in %s mode\n", ts->input_dev->name, ts->use_irq ? "interrupt" : "polling");
+	
+	if (ts->use_irq)
+	{
+#ifdef _INT_MODE_1
+		printk(KERN_INFO "sis_ts_probe: interrupt case 1 mode\n");
+#else
+		printk(KERN_INFO "sis_ts_probe: interrupt case 2 mode\n");
+#endif
+	}
+	
+#ifdef _STD_RW_IO
+	ret = sis_setup_chardev(ts);
+	if(ret)
+	{
+		printk( KERN_INFO"sis_setup_chardev fail\n");
+	}
+#endif
+
+	printk( KERN_INFO"sis SIS_SLAVE_ADDR: %d\n", SIS_SLAVE_ADDR);
+
+	return 0;
+
+err_input_register_device_failed:
+	input_free_device(ts->input_dev);
+
+err_input_dev_alloc_failed:
+err_power_failed:
+	kfree(ts);
+err_alloc_data_failed:
+	return ret;
+}
+
+static int sis_ts_remove(struct i2c_client *client)
+{
+	struct sis_ts_data *ts = i2c_get_clientdata(client);
+	
+#ifdef CONFIG_HAS_EARLYSUSPEND
+	unregister_early_suspend(&ts->early_suspend);
+#endif
+	if (ts->use_irq)
+		free_irq(client->irq, ts);
+	else
+		hrtimer_cancel(&ts->timer);
+	input_unregister_device(ts->input_dev);
+	kfree(ts);
+	
+	return 0;
+}
+
+static int sis_ts_suspend(struct i2c_client *client, pm_message_t mesg)
+{
+	int ret = 0;
+	struct sis_ts_data *ts = i2c_get_clientdata(client);
+
+	TPInfo->pre_keybit_state = 0x0;
+
+	if (ts->use_irq)
+	{
+#if ( LINUX_VERSION_CODE < KERNEL_VERSION (2, 6, 39) )
+		if ((ts->desc->status & IRQ_DISABLED) == IRQ_STATUS_ENABLED)
+#else
+		if ((ts->desc->irq_data.state_use_accessors & IRQD_IRQ_DISABLED) == IRQ_STATUS_ENABLED)
+#endif
+		{
+			disable_irq(client->irq);
+		}
+	}
+	else
+		hrtimer_cancel(&ts->timer);
+	/* only flush sis_wq */
+	flush_workqueue(sis_wq); 	   		
+
+	if (ts->power) {
+		ret = ts->power(0);
+		if (ret < 0)
+			printk(KERN_ERR "sis_ts_suspend power off failed\n");
+	}
+	
+	return 0;
+}
+
+static int sis_ts_resume(struct i2c_client *client)
+{
+	int ret = 0;
+	struct sis_ts_data *ts = i2c_get_clientdata(client);
+
+	if (ts->power)
+	{
+		ret = ts->power(1);
+		if (ret < 0)
+			printk(KERN_ERR "sis_ts_resume power on failed\n");
+	}
+
+	if (ts->use_irq)
+	{
+#if ( LINUX_VERSION_CODE < KERNEL_VERSION (2, 6, 39) )
+		if ((ts->desc->status & IRQ_DISABLED) == IRQ_STATUS_DISABLED)
+#else
+		if ((ts->desc->irq_data.state_use_accessors & IRQD_IRQ_DISABLED) == IRQ_STATUS_DISABLED)
+#endif
+		{
+			enable_irq(client->irq);
+		}
+	}
+	else
+		hrtimer_start(&ts->timer, ktime_set(1, 0), HRTIMER_MODE_REL);
+
+	return 0;
+}
+
+#ifdef CONFIG_HAS_EARLYSUSPEND
+static void sis_ts_early_suspend(struct early_suspend *h)
+{
+	struct sis_ts_data *ts;
+	
+	TPInfo->pre_keybit_state = 0x0;
+	ts = container_of(h, struct sis_ts_data, early_suspend);
+	sis_ts_suspend(ts->client, PMSG_SUSPEND);
+}
+
+static void sis_ts_late_resume(struct early_suspend *h)
+{
+	struct sis_ts_data *ts;
+	
+	ts = container_of(h, struct sis_ts_data, early_suspend);
+	sis_ts_resume(ts->client);
+}
+#endif
+
+static const struct i2c_device_id sis_ts_id[] = {
+	{ SIS_I2C_NAME, 0 },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(i2c, sis_ts_id);
+
+static struct i2c_driver sis_ts_driver = {
+	.probe		= sis_ts_probe,
+	.remove		= sis_ts_remove,
+#ifndef CONFIG_HAS_EARLYSUSPEND
+	.suspend	= sis_ts_suspend,
+	.resume		= sis_ts_resume,
+#endif
+#ifdef CONFIG_X86
+    .class      = I2C_CLASS_HWMON,
+    .detect		= sis_ts_detect,
+	.address_list	= normal_i2c,
+#endif
+	.id_table	= sis_ts_id,
+	.driver = {
+		.name	= SIS_I2C_NAME,
+	},
+};
+
+static int __init sis_ts_init(void)
+{
+	printk( KERN_INFO "sis_ts_init\n" );
+	
+	sis_wq = create_singlethread_workqueue("sis_wq");
+
+	if (!sis_wq)
+		return -ENOMEM;
+		
+	return i2c_add_driver(&sis_ts_driver);
+}
+
+#ifdef CONFIG_X86
+/* Return 0 if detection is successful, -ENODEV otherwise */
+static int sis_ts_detect(struct i2c_client *client,
+		       struct i2c_board_info *info)
+{
+	const char *type_name;
+	
+    printk(KERN_INFO "sis_ts_detect\n");
+	
+	type_name = "sis_i2c_ts";
+	strlcpy(info->type, type_name, I2C_NAME_SIZE);
+	
+	return 0;
+}
+#endif
+
+static void __exit sis_ts_exit(void)
+{
+#ifdef _STD_RW_IO
+	dev_t dev;
+#endif
+
+	printk(KERN_INFO "sis_ts_exit\n");
+	
+	i2c_del_driver(&sis_ts_driver);
+	
+	if (sis_wq)
+		destroy_workqueue(sis_wq);
+
+#ifdef _STD_RW_IO
+	dev = MKDEV(sis_char_major, 0);
+	cdev_del(&sis_char_cdev);
+	unregister_chrdev_region(dev, sis_char_devs_count);
+	device_destroy(sis_char_class, MKDEV(sis_char_major, 0));
+	class_destroy(sis_char_class);

This is a race condition. You must destroy the character devce
first, then the infrastructure it uses.

-- 
Oliver Neukum [off-list ref]

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html