Re: [PATCH 2/2] iio: exynos-adc: add experimental touchscreen support

[Arnd Bergmann <arnd@arndb.de>]

On Monday 21 July 2014 12:23:58 Arnd Bergmann wrote:
> 
> Thanks a lot for the review! I'll send out the new version after some build testing.

Here are the changes I did to the adc driver based on the review comments.
I'll start a new thread for the updated version that includes the changes.

diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
index 3d2caea05977..823d7ebc7f52 100644
--- a/drivers/iio/adc/exynos_adc.c
+++ b/drivers/iio/adc/exynos_adc.c
@@ -67,6 +67,9 @@
 
 #define ADC_S3C2410_CON_SELMUX(x) (((x)&0x7)<<3)
 
+/* touch screen always uses channel 0 */
+#define ADC_S3C2410_MUX_TS	0
+
 /* ADCTSC Register Bits */
 #define ADC_S3C2443_TSC_UD_SEN		(1u << 8)
 #define ADC_S3C2410_TSC_YM_SEN		(1u << 7)
@@ -106,6 +109,7 @@
 #define ADC_CON_EN_START	(1u << 0)
 #define ADC_DATX_PRESSED	(1u << 15)
 #define ADC_DATX_MASK		0xFFF
+#define ADC_DATY_MASK		0xFFF
 
 #define EXYNOS_ADC_TIMEOUT	(msecs_to_jiffies(100))
 
@@ -123,10 +127,12 @@ struct exynos_adc {
 
 	struct completion	completion;
 
-	bool			read_ts;
 	u32			value;
-	u32			value2;
 	unsigned int            version;
+
+	bool			read_ts;
+	u32			ts_x;
+	u32			ts_y;
 };
 
 struct exynos_adc_data {
@@ -396,21 +402,14 @@ static int exynos_read_raw(struct iio_dev *indio_dev,
 	return ret;
 }
 
-static int exynos_read_s3c64xx_ts(struct iio_dev *indio_dev,
-				struct iio_chan_spec const *chan,
-				int *val,
-				int *val2,
-				long mask)
+static int exynos_read_s3c64xx_ts(struct iio_dev *indio_dev, int *x, int *y)
 {
 	struct exynos_adc *info = iio_priv(indio_dev);
 	unsigned long timeout;
 	int ret;
 
-	if (mask != IIO_CHAN_INFO_RAW)
-		return -EINVAL;
-
 	mutex_lock(&indio_dev->mlock);
-	info->read_ts = 1;
+	info->read_ts = true;
 
 	reinit_completion(&info->completion);
 
@@ -418,7 +417,7 @@ static int exynos_read_s3c64xx_ts(struct iio_dev *indio_dev,
 	       ADC_V1_TSC(info->regs));
 
 	/* Select the ts channel to be used and Trigger conversion */
-	info->data->start_conv(info, 0);
+	info->data->start_conv(info, ADC_S3C2410_MUX_TS);
 
 	timeout = wait_for_completion_timeout
 			(&info->completion, EXYNOS_ADC_TIMEOUT);
@@ -428,12 +427,12 @@ static int exynos_read_s3c64xx_ts(struct iio_dev *indio_dev,
 			info->data->init_hw(info);
 		ret = -ETIMEDOUT;
 	} else {
-		*val = info->value;
-		*val2 = info->value2;
-		ret = IIO_VAL_INT;
+		*x = info->ts_x;
+		*y = info->ts_y;
+		ret = 0;
 	}
 
-	info->read_ts = 0;
+	info->read_ts = false;
 	mutex_unlock(&indio_dev->mlock);
 
 	return ret;
@@ -445,8 +444,8 @@ static irqreturn_t exynos_adc_isr(int irq, void *dev_id)
 
 	/* Read value */
 	if (info->read_ts) {
-		info->value = readl(ADC_V1_DATX(info->regs)) & ADC_DATX_MASK;
-		info->value2 = readl(ADC_V1_DATY(info->regs)) & ADC_DATX_MASK;
+		info->ts_x = readl(ADC_V1_DATX(info->regs));
+		info->ts_y = readl(ADC_V1_DATY(info->regs));
 		writel(ADC_TSC_WAIT4INT | ADC_S3C2443_TSC_UD_SEN, ADC_V1_TSC(info->regs));
 	} else {
 		info->value = readl(ADC_V1_DATX(info->regs)) & ADC_DATX_MASK;
@@ -477,7 +476,7 @@ static irqreturn_t exynos_ts_isr(int irq, void *dev_id)
 	int ret;
 
 	do {
-		ret =exynos_read_s3c64xx_ts(dev, NULL, &x, &y, IIO_CHAN_INFO_RAW);
+		ret = exynos_read_s3c64xx_ts(dev, &x, &y);
 		if (ret == -ETIMEDOUT)
 			break;
 
@@ -486,11 +485,15 @@ static irqreturn_t exynos_ts_isr(int irq, void *dev_id)
 			break;
 
 		input_report_abs(info->input, ABS_X, x & ADC_DATX_MASK);
-		input_report_abs(info->input, ABS_Y, y & ADC_DATX_MASK);
+		input_report_abs(info->input, ABS_Y, y & ADC_DATY_MASK);
 		input_report_key(info->input, BTN_TOUCH, 1);
 		input_sync(info->input);
 
 		msleep(1);
+
+		/* device may have been closed while touched */
+		if (!info->input->users)
+			return IRQ_HANDLED;
 	} while (1);
 
 	input_report_key(info->input, BTN_TOUCH, 0);
@@ -552,10 +555,28 @@ static int exynos_adc_remove_devices(struct device *dev, void *c)
 	return 0;
 }
 
+static int exynos_adc_ts_open(struct input_dev *dev)
+{
+	struct exynos_adc *info = input_get_drvdata(dev);
+
+	return request_threaded_irq(info->tsirq, NULL, exynos_ts_isr,
+				    0, "touchscreen", info);
+}
+
+static void exynos_adc_ts_close(struct input_dev *dev)
+{
+	struct exynos_adc *info = input_get_drvdata(dev);
+
+	free_irq(info->tsirq, info);
+}
+
 static int exynos_adc_ts_init(struct exynos_adc *info)
 {
 	int ret;
 
+	if (info->tsirq <= 0)
+		return -ENODEV;
+
 	info->input = input_allocate_device();
 	if (!info->input)
 		return -ENOMEM;
@@ -566,33 +587,17 @@ static int exynos_adc_ts_init(struct exynos_adc *info)
 	input_set_abs_params(info->input, ABS_X, 0, 0x3FF, 0, 0);
 	input_set_abs_params(info->input, ABS_Y, 0, 0x3FF, 0, 0);
 
-	/* data from s3c2410_ts driver */
 	info->input->name = "S3C24xx TouchScreen";
 	info->input->id.bustype = BUS_HOST;
-	info->input->id.vendor = 0xDEAD;
-	info->input->id.product = 0xBEEF;
-	info->input->id.version = 0x0200;
+	info->input->open = exynos_adc_ts_open;
+	info->input->close = exynos_adc_ts_close;
+	
+	input_set_drvdata(info->input, info);
 
 	ret = input_register_device(info->input);
-	if (ret) {
+	if (ret)
 		input_free_device(info->input);
-		goto err;
-	}
-
-	if (info->tsirq > 0)
-		ret = request_threaded_irq(info->irq, NULL, exynos_ts_isr,
-					0, "touchscreen", info);
-	if (ret < 0) {
-		dev_err(info->dev, "failed requesting touchsccreen irq, irq = %d\n",
-							info->irq);
-		goto err_input;
-	}
-
-	return 0;
 
-err_input:
-	input_unregister_device(info->input);
-err:
 	return ret;
 }
 
@@ -602,7 +607,7 @@ static int exynos_adc_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	struct iio_dev *indio_dev = NULL;
 	struct resource	*mem;
-	bool has_ts;
+	bool has_ts = false;
 	int ret = -ENODEV;
 	int irq;
 
@@ -711,7 +716,10 @@ static int exynos_adc_probe(struct platform_device *pdev)
 	if (info->data->init_hw)
 		info->data->init_hw(info);
 
-	has_ts = of_property_read_bool(pdev->dev.of_node, "has-touchscreen");
+	/* leave out any TS related code if unreachable */
+	if (IS_BUILTIN(CONFIG_INPUT) ||
+	    (IS_MODULE(CONFIG_INPUT) && config_enabled(MODULE)))
+		has_ts = of_property_read_bool(pdev->dev.of_node, "has-touchscreen");
 	if (has_ts)
 		ret = exynos_adc_ts_init(info);
 	if (ret)
@@ -752,7 +760,7 @@ static int exynos_adc_remove(struct platform_device *pdev)
 	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
 	struct exynos_adc *info = iio_priv(indio_dev);
 
-	input_free_device(info->input);	
+	input_unregister_device(info->input);	
 	device_for_each_child(&indio_dev->dev, NULL,
 				exynos_adc_remove_devices);
 	iio_device_unregister(indio_dev);