tsc210x cleanup

This is mostly cleanup of the tsc210x patch, but some bugs were fixed so now
it works on tsc2101 too.  Also, a few issues are now noted in the code:

 Tool-reported:
  - Address checkpatch.pl issues in the original patch
  - And also "sparse" issues

 Previously wrong:
  - Cope with CONFIG_SOUND_MODULE
  - Register accessor routines will now return error codes
  - ... many callers now abort cleanly after errors
  - Don't depend on seeing only a rev #1 tsc2102 chip!
  - Add missing EXPORT_SYMBOL declarations

 Style issues:
  - BUG_ON() is strongly to be avoided
  - So are macros that capture variables
  - And needless casting from void pointers
  - And type punning
  - Use dev_*() for messaging where practical
  - Use u16 not uint16_t, etc
  - Various other whitespace issues
  - Avoid __FUNCTION__
  - Single-lines for file-top comments; no whole paths

 Object code size and Other cleanup:
  - Compile most strings out unless -DDEBUG is configured (saving space)
  - Move some code into exit sections (then it may well vanish, saving space)
  - Add some header comments

 Open issues:
  - Hwmon needs to know VREF and the chip type, and to scale its values
  - Upstream push should exclude audio until some version is working
  - Audio will needs to export symbols to modules, too
  - Lots of the I/O calls (especially audio) still don't handle errors
  - How could board init code get board-specific temp calibration data?

Also the Kconfig is more open-ended; tsc210x might in the future include the
tsc2100 and tsc2111, it doesn't need to be limited to tsc2101 and tsc2102.
Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>

drivers/hwmon/Kconfig

@@ -685,11 +685,11 @@ config SENSORS_APPLESMC
 	  the awesome power of applesmc.
 
 config SENSORS_TSC210X
-	tristate "TI TSC2101/2102 battery & temperature sensors"
+	tristate "TI TSC210x battery & temperature sensors"
 	depends on HWMON && SPI_MASTER
 	select SPI_TSC210X
 	help
-	  Say Y if your board has a TSC210X chip and you want to
+	  Say Y if your board has a TSC210x chip and you want to
 	  have its battery state, auxiliary input and/or temperature
 	  sensors exported through hwmon.
 

drivers/hwmon/tsc210x_sensors.c

 /*
- * drivers/hwmon/tsc210x_sensors.c
- *
- * hwmon interface for TSC210X sensors
+ * tsc210x_sensors.c - hwmon interface to TI TSC210x sensors
  *
  * Copyright (c) 2005-2007 Andrzej Zaborowski  <balrog@zabor.org>
  *
@@ -32,54 +30,92 @@
 
 #include <linux/spi/tsc210x.h>
 
+
+/*
+ * TI TSC210x chips include an ADC that's shared between various
+ * sensors (temperature, battery, vAUX, etc) and the touchscreen.
+ * This driver packages access to the non-touchscreen sensors
+ * available on a given board.
+ */
+
 struct tsc210x_hwmon {
 	int bat[2], aux[2], temp[2];
 
 	struct class_device *dev;
 	struct tsc210x_config *pdata;
 #ifdef CONFIG_APM
+	/* prevent APM from colliding with normal hwmon accessors */
 	spinlock_t apm_lock;
 #endif
 };
 
 #ifdef CONFIG_APM
-# define apm_lock()	spin_lock(&hwmon->apm_lock)
-# define apm_unlock()	spin_unlock(&hwmon->apm_lock)
+# define apm_lock(h)	spin_lock(&(h)->apm_lock)
+# define apm_unlock(h)	spin_unlock(&(h)->apm_lock)
 #else
-# define apm_lock()
-# define apm_unlock()
+# define apm_lock(h)	do { } while (0)
+# define apm_unlock(h)	do { } while (0)
 #endif
 
-static void tsc210x_ports(struct tsc210x_hwmon *hwmon, int bat[], int aux[])
+static void tsc210x_ports(void *context, int bat[], int aux[])
 {
-	apm_lock();
+	struct tsc210x_hwmon	*hwmon = context;
+
+	apm_lock(hwmon);
+
+	/* FIXME for tsc2101 and tsc2111, battery voltage is:
+	 *	VBAT = (5 * VREF * (bat[x])) / (2 ^ bits)
+	 * For tsc2100 and tsc2102, use "6" not "5"; that formula ignores
+	 * an external 100-300 Ohm resistor making the right value be just
+	 * a bit over 5 (or 6).
+	 *
+	 * FIXME the vAUX measurements need scaling too, but in that case
+	 * there's no *internal* voltage divider so just scale to VREF.
+	 *
+	 *  --> This code needs to know VREF, the VBAT multiplier, and
+	 *	the precision.  For now, assume VREF 1.25V and 12 bits.
+	 *	When an external reference is used, it normally won't
+	 *	match the 1.25V (or 2.5V) values supported internally...
+	 *
+	 *  --> Output units should become milliVolts; currently they are
+	 *	dimensionless...
+	 */
 	hwmon->bat[0] = bat[0];
 	hwmon->bat[1] = bat[1];
+
 	hwmon->aux[0] = aux[0];
 	hwmon->aux[1] = aux[1];
-	apm_unlock();
+
+	apm_unlock(hwmon);
 }
 
-static void tsc210x_temp1(struct tsc210x_hwmon *hwmon, int temp)
+/* FIXME temp sensors also need scaling so values are milliVolts...
+ * temperature (given calibration data) should be millidegrees C.
+ */
+
+static void tsc210x_temp1(void *context, int temp)
 {
-	apm_lock();
+	struct tsc210x_hwmon	*hwmon = context;
+
+	apm_lock(hwmon);
 	hwmon->temp[0] = temp;
-	apm_unlock();
+	apm_unlock(hwmon);
 }
 
-static void tsc210x_temp2(struct tsc210x_hwmon *hwmon, int temp)
+static void tsc210x_temp2(void *context, int temp)
 {
-	apm_lock();
+	struct tsc210x_hwmon	*hwmon = context;
+
+	apm_lock(hwmon);
 	hwmon->temp[1] = temp;
-	apm_unlock();
+	apm_unlock(hwmon);
 }
 
 #define TSC210X_INPUT(devname, field)	\
 static ssize_t tsc_show_ ## devname(struct device *dev,	\
 		struct device_attribute *devattr, char *buf)	\
 {	\
-	struct tsc210x_hwmon *hwmon = (struct tsc210x_hwmon *)	\
-		platform_get_drvdata(to_platform_device(dev));	\
+	struct tsc210x_hwmon *hwmon = dev_get_drvdata(dev);	\
 	return sprintf(buf, "%i\n", hwmon->field);	\
 }	\
 static DEVICE_ATTR(devname ## _input, S_IRUGO, tsc_show_ ## devname, NULL);
@@ -94,13 +130,11 @@ TSC210X_INPUT(in5, temp[1])
 static ssize_t tsc_show_temp1(struct device *dev,
 		struct device_attribute *devattr, char *buf)
 {
-	struct tsc210x_hwmon *hwmon = (struct tsc210x_hwmon *)
-		platform_get_drvdata(to_platform_device(dev));
-	int t1, t2;
-	int diff, value;
-
-	t1 = hwmon->temp[0];
-	t2 = hwmon->temp[1];
+	struct tsc210x_hwmon *hwmon = dev_get_drvdata(dev);
+	int t1 = hwmon->temp[0];
+	int t2 = hwmon->temp[1];
+	int diff;
+	int value;
 
 	/*
 	 * Use method #2 (differential) to calculate current temperature.
@@ -114,7 +148,6 @@ static ssize_t tsc_show_temp1(struct device *dev,
 	 * 273150 is zero degrees Celcius.
 	 */
 	diff = hwmon->pdata->temp_at25c[1] - hwmon->pdata->temp_at25c[0];
-	BUG_ON(diff == 0);
 	value = (t2 - t1) * 298150 / diff;	/* This is in Kelvins now */
 
 	value -= 273150;			/* Celcius millidegree */
@@ -128,9 +161,10 @@ static struct tsc210x_hwmon *apm_hwmon;
 static void tsc210x_get_power_status(struct apm_power_info *info)
 {
 	struct tsc210x_hwmon *hwmon = apm_hwmon;
-	apm_lock();
+
+	apm_lock(hwmon);
 	hwmon->pdata->apm_report(info, hwmon->bat);
-	apm_unlock();
+	apm_unlock(hwmon);
 }
 #endif
 
@@ -143,15 +177,14 @@ static int tsc210x_hwmon_probe(struct platform_device *pdev)
 	hwmon = (struct tsc210x_hwmon *)
 		kzalloc(sizeof(struct tsc210x_hwmon), GFP_KERNEL);
 	if (!hwmon) {
-		printk(KERN_ERR "%s: allocation failed\n", __FUNCTION__);
+		dev_dbg(&pdev->dev, "allocation failed\n");
 		return -ENOMEM;
 	}
 
 	hwmon->dev = hwmon_device_register(&pdev->dev);
 	if (IS_ERR(hwmon->dev)) {
 		kfree(hwmon);
-		printk(KERN_ERR "%s: Class registration failed\n",
-				__FUNCTION__);
+		dev_dbg(&pdev->dev, "registration failed\n");
 		return PTR_ERR(hwmon->dev);
 	}
 
@@ -170,19 +203,19 @@ static int tsc210x_hwmon_probe(struct platform_device *pdev)
 
 	if (pdata->monitor & (TSC_BAT1 | TSC_BAT2 | TSC_AUX1 | TSC_AUX2))
 		status |= tsc210x_ports_cb(pdev->dev.parent,
-				(tsc210x_ports_t) tsc210x_ports, hwmon);
+				tsc210x_ports, hwmon);
 	if (pdata->monitor & TSC_TEMP) {
 		status |= tsc210x_temp1_cb(pdev->dev.parent,
-				(tsc210x_temp_t) tsc210x_temp1, hwmon);
+				tsc210x_temp1, hwmon);
 		status |= tsc210x_temp2_cb(pdev->dev.parent,
-				(tsc210x_temp_t) tsc210x_temp2, hwmon);
+				tsc210x_temp2, hwmon);
 	}
 
 	if (status) {
-		tsc210x_ports_cb(pdev->dev.parent, 0, 0);
-		tsc210x_temp1_cb(pdev->dev.parent, 0, 0);
-		tsc210x_temp2_cb(pdev->dev.parent, 0, 0);
-		platform_set_drvdata(pdev, 0);
+		tsc210x_ports_cb(pdev->dev.parent, NULL, NULL);
+		tsc210x_temp1_cb(pdev->dev.parent, NULL, NULL);
+		tsc210x_temp2_cb(pdev->dev.parent, NULL, NULL);
+		platform_set_drvdata(pdev, NULL);
 #ifdef CONFIG_APM
 		if (pdata->apm_report)
 			apm_get_power_status = 0;
@@ -203,23 +236,28 @@ static int tsc210x_hwmon_probe(struct platform_device *pdev)
 	if (pdata->monitor & TSC_TEMP) {
 		status |= device_create_file(&pdev->dev, &dev_attr_in4_input);
 		status |= device_create_file(&pdev->dev, &dev_attr_in5_input);
-		status |= device_create_file(&pdev->dev, &dev_attr_temp1_input);
+
+		if ((pdata->temp_at25c[1] - pdata->temp_at25c[0]) == 0)
+			dev_warn(&pdev->dev, "No temp calibration data.\n");
+		else
+			status |= device_create_file(&pdev->dev,
+						&dev_attr_temp1_input);
 	}
 	if (status)	/* Not fatal */
-		printk(KERN_ERR "%s: Creating one or more "
-				"attribute files failed\n", __FUNCTION__);
+		dev_dbg(&pdev->dev, "Creating one or more "
+				"attribute files failed\n");
 
 	return 0;
 }
 
-static int tsc210x_hwmon_remove(struct platform_device *pdev)
+static int __exit tsc210x_hwmon_remove(struct platform_device *pdev)
 {
 	struct tsc210x_hwmon *dev = platform_get_drvdata(pdev);
 
-	tsc210x_ports_cb(pdev->dev.parent, 0, 0);
-	tsc210x_temp1_cb(pdev->dev.parent, 0, 0);
-	tsc210x_temp2_cb(pdev->dev.parent, 0, 0);
-	platform_set_drvdata(pdev, 0);
+	tsc210x_ports_cb(pdev->dev.parent, NULL, NULL);
+	tsc210x_temp1_cb(pdev->dev.parent, NULL, NULL);
+	tsc210x_temp2_cb(pdev->dev.parent, NULL, NULL);
+	platform_set_drvdata(pdev, NULL);
 #ifdef CONFIG_APM
 	if (dev->pdata->apm_report)
 		apm_get_power_status = 0;
@@ -231,7 +269,7 @@ static int tsc210x_hwmon_remove(struct platform_device *pdev)
 
 static struct platform_driver tsc210x_hwmon_driver = {
 	.probe		= tsc210x_hwmon_probe,
-	.remove 	= tsc210x_hwmon_remove,
+	.remove		= __exit_p(tsc210x_hwmon_remove),
 	/* Nothing to do on suspend/resume */
 	.driver		= {
 		.name	= "tsc210x-hwmon",
@@ -240,15 +278,17 @@ static struct platform_driver tsc210x_hwmon_driver = {
 
 static int __init tsc210x_hwmon_init(void)
 {
+	/* can't use driver_probe() here since the parent device
+	 * gets registered "late"
+	 */
 	return platform_driver_register(&tsc210x_hwmon_driver);
 }
+module_init(tsc210x_hwmon_init);
 
 static void __exit tsc210x_hwmon_exit(void)
 {
 	platform_driver_unregister(&tsc210x_hwmon_driver);
 }
-
-module_init(tsc210x_hwmon_init);
 module_exit(tsc210x_hwmon_exit);
 
 MODULE_AUTHOR("Andrzej Zaborowski");

drivers/input/touchscreen/Kconfig

@@ -193,12 +193,12 @@ config TOUCHSCREEN_TSC2102
 	  module will be called tsc2102_ts.
 
 config TOUCHSCREEN_TSC210X
-	tristate "TSC 2101/2102 based touchscreens"
+	tristate "TI TSC210x based touchscreens"
 	depends on SPI_MASTER
 	select SPI_TSC210X
 	help
 	  Say Y here if you have a touchscreen interface using a
-	  TI TSC 210x controller, and your board-specific initialisation
+	  TI TSC210x controller, and your board-specific initialisation
 	  code includes that in its table of SPI devices.
 
 	  If unsure, say N (but it's safe to say "Y").

drivers/input/touchscreen/tsc210x_ts.c

 /*
- * input/touchscreen/tsc210x_ts.c
- *
- * Touchscreen input device driver for the TSC 2101/2102 chips.
+ * tsc210x_ts.c - touchscreen input device for TI TSC210x chips
  *
  * Copyright (c) 2006-2007 Andrzej Zaborowski  <balrog@zabor.org>
  *
@@ -29,8 +27,23 @@
 
 #include <linux/spi/tsc210x.h>
 
-static void tsc210x_touch(struct input_dev *dev, int touching)
+
+/*
+ * The sensor ADC on tsc210x chips is most often used with the smart
+ * touchscreen controller.   Those controllers can be made to improve
+ * sample quality directly by multi-sampling and by taking the mean or
+ * median of various numbers of samples.  They also take X, Y, and
+ * pressure measurements automatically, so this driver has relatively
+ * little to do.
+ *
+ * There are a few chips in this family that don't have quite the same
+ * touchscreen interface, e.g. no "median" mode.
+ */
+
+static void tsc210x_touch(void *context, int touching)
 {
+	struct input_dev *dev = context;
+
 	if (!touching) {
 		input_report_abs(dev, ABS_X, 0);
 		input_report_abs(dev, ABS_Y, 0);
@@ -41,8 +54,9 @@ static void tsc210x_touch(struct input_dev *dev, int touching)
 	input_report_key(dev, BTN_TOUCH, touching);
 }
 
-static void tsc210x_coords(struct input_dev *dev, int x, int y, int z1, int z2)
+static void tsc210x_coords(void *context, int x, int y, int z1, int z2)
 {
+	struct input_dev *dev = context;
 	int p;
 
 	/* Calculate the touch resistance a la equation #1 */
@@ -66,17 +80,15 @@ static int tsc210x_ts_probe(struct platform_device *pdev)
 	if (!dev)
 		return -ENOMEM;
 
-	status = tsc210x_touch_cb(pdev->dev.parent,
-			(tsc210x_touch_t) tsc210x_touch, dev);
+	status = tsc210x_touch_cb(pdev->dev.parent, tsc210x_touch, dev);
 	if (status) {
 		input_free_device(dev);
 		return status;
 	}
 
-	status = tsc210x_coords_cb(pdev->dev.parent,
-			(tsc210x_coords_t) tsc210x_coords, dev);
+	status = tsc210x_coords_cb(pdev->dev.parent, tsc210x_coords, dev);
 	if (status) {
-		tsc210x_touch_cb(pdev->dev.parent, 0, 0);
+		tsc210x_touch_cb(pdev->dev.parent, NULL, NULL);
 		input_free_device(dev);
 		return status;
 	}
@@ -94,8 +106,8 @@ static int tsc210x_ts_probe(struct platform_device *pdev)
 
 	status = input_register_device(dev);
 	if (status) {
-		tsc210x_coords_cb(pdev->dev.parent, 0, 0);
-		tsc210x_touch_cb(pdev->dev.parent, 0, 0);
+		tsc210x_coords_cb(pdev->dev.parent, NULL, NULL);
+		tsc210x_touch_cb(pdev->dev.parent, NULL, NULL);
 		input_free_device(dev);
 		return status;
 	}
@@ -105,14 +117,13 @@ static int tsc210x_ts_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int tsc210x_ts_remove(struct platform_device *pdev)
+static int __exit tsc210x_ts_remove(struct platform_device *pdev)
 {
-	struct input_dev *dev = (struct input_dev *)
-		platform_get_drvdata(pdev);
+	struct input_dev *dev = platform_get_drvdata(pdev);
 
-	tsc210x_touch_cb(pdev->dev.parent, 0, 0);
-	tsc210x_coords_cb(pdev->dev.parent, 0, 0);
-	platform_set_drvdata(pdev, 0);
+	tsc210x_touch_cb(pdev->dev.parent, NULL, NULL);
+	tsc210x_coords_cb(pdev->dev.parent, NULL, NULL);
+	platform_set_drvdata(pdev, NULL);
 	input_unregister_device(dev);
 	input_free_device(dev);
 
@@ -121,7 +132,7 @@ static int tsc210x_ts_remove(struct platform_device *pdev)
 
 static struct platform_driver tsc210x_ts_driver = {
 	.probe		= tsc210x_ts_probe,
-	.remove 	= tsc210x_ts_remove,
+	.remove		= __exit_p(tsc210x_ts_remove),
 	/* Nothing to do on suspend/resume */
 	.driver		= {
 		.name	= "tsc210x-ts",
@@ -131,21 +142,17 @@ static struct platform_driver tsc210x_ts_driver = {
 
 static int __init tsc210x_ts_init(void)
 {
-	int ret;
-
-	ret = platform_driver_register(&tsc210x_ts_driver);
-	if (ret)
-		return -ENODEV;
-
-	return 0;
+	/* can't use driver_probe() here since the parent device
+	 * gets registered "late"
+	 */
+	return platform_driver_register(&tsc210x_ts_driver);
 }
+module_init(tsc210x_ts_init);
 
 static void __exit tsc210x_ts_exit(void)
 {
 	platform_driver_unregister(&tsc210x_ts_driver);
 }
-
-module_init(tsc210x_ts_init);
 module_exit(tsc210x_ts_exit);
 
 MODULE_AUTHOR("Andrzej Zaborowski");

drivers/spi/Kconfig

@@ -229,10 +229,17 @@ config SPI_TSC2102
 
 config SPI_TSC210X
 	depends on SPI_MASTER && EXPERIMENTAL
-	tristate "TSC2101/TSC2102 chips support"
+	tristate "TI TSC210x (TSC2101/TSC2102) support"
 	help
-	  Say Y here if you want support for the TSC210x chips.  It
-	  will be needed for the touchscreen driver on some boards.
+	  Say Y here if you want support for the TSC210x chips.  Some
+	  boards use these for touchscreen and audio support.
+
+	  These are members of a family of highly integrated PDA analog
+	  interface circuit.  They include a 12-bit ADC used for battery,
+	  temperature, touchscreen, and other sensors.  They also have
+	  an audio DAC and amplifier, and in some models an audio ADC.
+	  The audio support is highly chip-specific, but most of the
+	  sensor support works the same.
 
 	  Note that the device has to be present in the board's SPI
 	  devices table for this driver to load.  This driver doesn't

include/linux/spi/tsc210x.h

@@ -24,15 +24,16 @@
 #define __LINUX_SPI_TSC210X_H
 
 struct apm_power_info;
+
 struct tsc210x_config {
 	int use_internal;	/* Use internal reference voltage */
-	uint32_t monitor;	/* What inputs are relevant */
+	u32 monitor;		/* What inputs are wired on this board */
 	int temp_at25c[2];	/* Thermometer calibration data */
 	void (*apm_report)(struct apm_power_info *info, int battery[]);
 				/* Report status to APM based on battery[] */
 	void *alsa_config;	/* .platform_data for the ALSA device */
-	const char *mclk;	/* Optional: bclk name */
-	const char *bclk;	/* Optional: mclk name */
+	const char *mclk;	/* Optional: mclk name */
+	const char *bclk;	/* Optional: bclk name */
 };
 
 #define TSC_BAT1	(1 << 0)
@@ -45,16 +46,25 @@ struct tsc210x_config {
 #define TSC_VBAT	TSC_BAT1
 
 struct tsc210x_dev;
-extern u16 tsc210x_read_sync(struct tsc210x_dev *dev, int page, u8 address);
-extern void tsc210x_reads_sync(struct tsc210x_dev *dev, int page,
+
+/* Drivers for tsc210x components like touchscreen, sensor, and audio
+ * are packaged as platform drivers which can issue synchronous register
+ * acceses, and may also register a callback to process their particular
+ * type of data when that data is automatically sampled.  The platform
+ * device is a child of the TSC spi device.
+ */
+
+extern int tsc210x_read_sync(struct tsc210x_dev *dev, int page, u8 address);
+extern int tsc210x_reads_sync(struct tsc210x_dev *dev, int page,
 		u8 startaddress, u16 *data, int numregs);
-extern void tsc210x_write_sync(struct tsc210x_dev *dev, int page,
+extern int tsc210x_write_sync(struct tsc210x_dev *dev, int page,
 		u8 address, u16 data);
 
 typedef void (*tsc210x_touch_t)(void *context, int touching);
 typedef void (*tsc210x_coords_t)(void *context, int x, int y, int z1, int z2);
 typedef void (*tsc210x_ports_t)(void *context, int bat[], int aux[]);
 typedef void (*tsc210x_temp_t)(void *context, int temp);
+
 extern int tsc210x_touch_cb(struct device *dev,
 		tsc210x_touch_t handler, void *context);
 extern int tsc210x_coords_cb(struct device *dev,
@@ -66,13 +76,10 @@ extern int tsc210x_temp1_cb(struct device *dev,
 extern int tsc210x_temp2_cb(struct device *dev,
 		tsc210x_temp_t handler, void *context);
 
-#ifdef CONFIG_SOUND
-extern void tsc210x_set_dac_volume(struct device *dev,
-		uint8_t left_ch, uint8_t right_ch);
-extern void tsc210x_set_dac_mute(struct device *dev,
-		int left_ch, int right_ch);
-extern void tsc210x_get_dac_mute(struct device *dev,
-		int *left_ch, int *right_ch);
+#if defined(CONFIG_SOUND) || defined(CONFIG_SOUND_MODULE)
+extern void tsc210x_set_dac_volume(struct device *dev, u8 left, u8 right);
+extern void tsc210x_set_dac_mute(struct device *dev, int left, int right);
+extern void tsc210x_get_dac_mute(struct device *dev, int *left, int *right);
 extern void tsc210x_dac_power(struct device *dev, int on);
 extern int tsc210x_set_rate(struct device *dev, int rate);
 extern void tsc210x_set_i2s_master(struct device *dev, int state);