Re: [PATCH v2] iio: adc: add CSR SiRFSoC internal ADC driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 17/07/14 10:22, Guoying Zhang wrote:
SiRFSoC internal ADC provides touchscreen single or dual touch
channels, and provides several auxiliary channels to measure
temperature, battery and so on.

Cc: Peter Meerwald <pmeerw@xxxxxxxxxx>
Signed-off-by: Guoying Zhang <Guoying.Zhang@xxxxxxx>
Signed-off-by: Barry Song <Baohua.Song@xxxxxxx>

Hi,

The way the touchscreen side is pushed into here causes a number of issues.
You try to push multiple coordinates out through a poll interface that only
allows one.

I would suggest you effectively rip the touchscreen channels out of the IIO
elements of the driver and support them via the input subsystem (within this
single driver is fine).

I'm currious to note that there was a sirfsoc_rs touchscreen driver posted
a while back.  Why was that not pursued futher?

I'm reasonably happy with the aux channels side of things.  It's just the
touchscreen being blugeoned into an incorrect interface that is causing
trouble here.

Various other bits inline.

Jonathan
---
  -v2: fix many issues from Peter and Jonathan's feedbacks

  .../devicetree/bindings/iio/adc/sirfsoc_adc.txt    |  33 +
  drivers/iio/adc/Kconfig                            |  10 +
  drivers/iio/adc/Makefile                           |   1 +
  drivers/iio/adc/sirfsoc_adc.c                      | 755 +++++++++++++++++++++
  4 files changed, 799 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/iio/adc/sirfsoc_adc.txt
  create mode 100644 drivers/iio/adc/sirfsoc_adc.c

diff --git a/Documentation/devicetree/bindings/iio/adc/sirfsoc_adc.txt b/Documentation/devicetree/bindings/iio/adc/sirfsoc_adc.txt
new file mode 100644
index 0000000..70eaf40
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/sirfsoc_adc.txt
@@ -0,0 +1,33 @@
+Sirfsoc inner Analog to Digital Converter bindings
+
+The devicetree bindings are for the new ADC driver written for
+Prima2/Atlas6 CSR.
+
+Required properties:
+- compatible: Should be one of
+	* "sirf,prima2-adc": When use Prima2 SOC
+	* "sirf,atlas6-adc": When use Atlas6 SOC
+- reg: Offset and length of the register set for the device
+- interrupts: Should contain the interrupt for the device
+- clocks: The clock is needed by the ADC controller
+- resets: The reset for the device
+- #io-channel-cells: Should be set to <1>
+
+Example:
+	adc: adc@b0110000 {
+		compatible = "sirf,prima2-adc";
+		reg = <0xb0110000 0x10000>;
+		interrupts = <34>;
+		clocks = <&clks 16>;
+		resets = <&rstc 32>;
+		#io-channel-cells = <1>;
+	};
+
+	adc: adc@b0110000 {
+		compatible = "sirf,atlas6-adc";
+		reg = <0xb0110000 0x10000>;
+		interrupts = <34>;
+		clocks = <&clks 16>;
+		resets = <&rstc 32>;
+		#io-channel-cells = <1>;
+	};
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index a80d236..6b803d5 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -187,6 +187,16 @@ config NAU7802
  	  To compile this driver as a module, choose M here: the
  	  module will be called nau7802.

+config SIRFSOC_ADC
+	tristate "SiRFSoC ADC"
+	depends on ARCH_SIRF
+	help
+	  If you say yes here you get support for CSR SiRFSoC internal
+	  ADC.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called sirfsoc_adc.
+
  config TI_ADC081C
  	tristate "Texas Instruments ADC081C021/027"
  	depends on I2C
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 9d60f2d..2912e58 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_MCP320X) += mcp320x.o
  obj-$(CONFIG_MCP3422) += mcp3422.o
  obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
  obj-$(CONFIG_NAU7802) += nau7802.o
+obj-$(CONFIG_SIRFSOC_ADC) += sirfsoc_adc.o
  obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
  obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
  obj-$(CONFIG_TWL4030_MADC) += twl4030-madc.o
diff --git a/drivers/iio/adc/sirfsoc_adc.c b/drivers/iio/adc/sirfsoc_adc.c
new file mode 100644
index 0000000..ae11a36
--- /dev/null
+++ b/drivers/iio/adc/sirfsoc_adc.c
@@ -0,0 +1,755 @@
+/*
+* ADC Driver for CSR SiRFSoC
+*
+* Copyright (c) 2014 Cambridge Silicon Radio Limited, a CSR plc group company.
+*
+* Licensed under GPLv2.
+*/
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/of.h>
+#include <linux/pm.h>
+#include <linux/platform_device.h>
+#include <linux/of_platform.h>
+#include <linux/reset.h>
+#include <linux/rtc/sirfsoc_rtciobrg.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/machine.h>
+#include <linux/iio/driver.h>
+
+#define DRIVER_NAME "sirfsoc_adc"
+
+#define SIRFSOC_PWR_WAKEEN_TSC_SHIFT	23
+#define SIRFSOC_PWR_WAKEEN_TS_SHIFT	5
+#define SIRFSOC_PWRC_TRIGGER_EN		0x8
+#define SIRFSOC_PWRC_BASE		0x3000
+
+/* Registers offset */
+#define SIRFSOC_ADC_CONTROL1		0x00
+#define SIRFSOC_ADC_CONTROL2		0x04
+#define SIRFSOC_ADC_INTR		0x08
+#define SIRFSOC_ADC_COORD		0x0C
+#define SIRFSOC_ADC_PRESSURE		0x10
+#define SIRFSOC_ADC_AUX1		0x14
+/* Atlas6 AUX2 and AUX4 reserved*/
+#define SIRFSOC_ADC_AUX2		0x18
+#define SIRFSOC_ADC_AUX3		0x1C
+#define SIRFSOC_ADC_AUX4		0x20
+#define SIRFSOC_ADC_AUX5		0x24
+#define SIRFSOC_ADC_AUX6		0x28
+/* Read Back calibration register */
+#define SIRFSOC_ADC_CB			0x2C
+#define SIRFSOC_ADC_COORD2		0x30
+#define SIRFSOC_ADC_COORD3		0x34
+#define SIRFSOC_ADC_COORD4		0x38
+
+/* CTRL1 defines */
+#define SIRFSOC_ADC_RESET_QUANT_EN	BIT(24)
+#define SIRFSOC_ADC_RST_B		BIT(23)
+#define SIRFSOC_ADC_RESOLUTION_12	BIT(22)
+#define SIRFSOC_ADC_RBAT_DISABLE	(0x0 << 21)
+#define SIRFSOC_ADC_RBAT_ENABLE		(0x1 << 21)
+#define SIRFSOC_ADC_EXTCM(x)		(((x) & 0x3) << 19)
+#define SIRFSOC_ADC_SGAIN(x)		(((x) & 0x7) << 16)
+#define SIRFSOC_ADC_POLL		BIT(15)
+#define SIRFSOC_ADC_SEL(x)		(((x) & 0xF) << 11)
+#define SIRFSOC_ADC_FREQ_6K		(0x0 << 8)
+#define SIRFSOC_ADC_FREQ_13K		BIT(8)
+#define SIRFSOC_ADC_DEL_SET(x)		(((x) & 0xF) << 4)
+#define SIRFSOC_ADC_TP_TIME(x)		(((x) & 0x7) << 0)
+
+/* CTRL2 defines */
+/* Pen detector off, digitizer off */
+#define SIRFSOC_ADC_PRP_MODE0		(0 << 14)
+/* Pen detector on, digitizer off, digitizer wakes up on pen detect */
+#define SIRFSOC_ADC_PRP_MODE1		BIT(14)
+/* Pen detector on, digitizer off, no wake up on pen detect */
+#define SIRFSOC_ADC_PRP_MODE2		(2 << 14)
+/* Pen detector on, digitizer on */
+#define SIRFSOC_ADC_PRP_MODE3		(3 << 14)
+#define SIRFSOC_ADC_RTOUCH(x)		(((x) & 0x3) << 12)
+#define SIRFSOC_ADC_DEL_PRE(x)		(((x) & 0xF) << 4)
+#define SIRFSOC_ADC_DEL_DIS(x)		(((x) & 0xF) << 0)
+
+/* INTR register defines */
+#define SIRFSOC_ADC_PEN_INTR_EN		BIT(5)
+#define SIRFSOC_ADC_DATA_INTR_EN	BIT(4)
+#define SIRFSOC_ADC_PEN_INTR		BIT(1)
+#define SIRFSOC_ADC_DATA_INTR		BIT(0)
+
+/* DATA register defines */
+#define SIRFSOC_ADC_PEN_DOWN		BIT(31)
+#define SIRFSOC_ADC_DATA_AUXVALID	BIT(30)
+#define SIRFSOC_ADC_DATA_CB_VALID	BIT(30)
+#define SIRFSOC_ADC_ADC_DATA_MASK(x)	(0x3FFF << (x))
+#define SIRFSOC_ADC_DATA_AUXMASK	SIRFSOC_ADC_ADC_DATA_MASK(0)
+#define SIRFSOC_ADC_DATA_CBMASK		SIRFSOC_ADC_ADC_DATA_MASK(0)
+
+#define SIRFSOC_ADC_MORE_CTL1	(of_machine_is_compatible("sirf,atlas6") ?\
+				(SIRFSOC_ADC_RESET_QUANT_EN |\
+				SIRFSOC_ADC_RST_B) : (0))
+
+/* Select AD samples to read (SEL bits in ADC_CONTROL1 register) */
+#define SIRFSOC_ADC_AUX1_SEL	0x04
+#define SIRFSOC_ADC_AUX2_SEL	0x05
+#define SIRFSOC_ADC_AUX3_SEL	0x06
+#define SIRFSOC_ADC_AUX4_SEL	0x07
+#define SIRFSOC_ADC_AUX5_SEL	0x08
+#define SIRFSOC_ADC_AUX6_SEL	0x09
+#define SIRFSOC_ADC_TS_SEL	0x0A    /* Sample for single touch*/
+#define SIRFSOC_ADC_GROUND_SEL	0x0B    /* Gound for offset calibration */
+#define SIRFSOC_ADC_BANDGAP_SEL	0x0C    /* Bandgap for gain calibration */
+#define SIRFSOC_ADC_TS_SEL_DUAL	0x0F    /* Samples for dual touch */
+
+#define SIRFSOC_ADC_TS_SAMPLE_SIZE	4
+#define SIRFSOC_ADC_CTL1(sel)    (SIRFSOC_ADC_POLL | SIRFSOC_ADC_SEL(sel) |\
+		SIRFSOC_ADC_DEL_SET(6) | SIRFSOC_ADC_FREQ_6K |\
+		SIRFSOC_ADC_TP_TIME(0) | SIRFSOC_ADC_SGAIN(0) |\
+		SIRFSOC_ADC_EXTCM(0) | SIRFSOC_ADC_RBAT_DISABLE |\
+		SIRFSOC_ADC_MORE_CTL1)
+
+enum sirfsoc_adc_chips {
+	PRIMA2,
+	ATLAS6,
+};
+
+struct sirfsoc_adc_chip_info {
+	const struct iio_chan_spec *channels;
+	unsigned int num_channels;
+	unsigned int flags;
+	const struct iio_info *iio_info;
+
+	u32 (*calculate_volt)(u32, u32, u32);
I'd definitely put parameter names in here to act as some
form of minimal documentation of what is needed.
+	const u32 *channel_sel;
+};
+
+struct sirfsoc_adc_request {
+	u16 mode;
+	u16 extcm;
+	u16 reference;
+	u8 delay_bits;
+	u32 s_gain_bits;
+	u16 read_back_data;
+};
+
+struct sirfsoc_adc {
+	const struct sirfsoc_adc_chip_info	*chip_info;
+	struct clk	*clk;
+	void __iomem	*base;
+	struct sirfsoc_adc_request req;
+	struct completion	done;
+	struct mutex	adc_lock;
+};
+
+static const u32 sirfsoc_adc_ts_reg[SIRFSOC_ADC_TS_SAMPLE_SIZE] = {
+	/* Dual touch samples read registers*/
+	SIRFSOC_ADC_COORD, SIRFSOC_ADC_COORD2,
+	SIRFSOC_ADC_COORD3, SIRFSOC_ADC_COORD4
+};
+
+enum sirfsoc_adc_ts_type {
+	SINGLE_TOUCH,
+	DUAL_TOUCH,
+};
+
+static int sirfsoc_adc_get_ts_sample(
+	struct sirfsoc_adc *adc, int *sample, int touch_type)
+{
+	int adc_intr;
+	int ret = 0;
+	int i;
+
+	mutex_lock(&adc->adc_lock);
+	adc_intr = readl(adc->base + SIRFSOC_ADC_INTR);
+	if (adc_intr & SIRFSOC_ADC_PEN_INTR)
+		writel(SIRFSOC_ADC_PEN_INTR | SIRFSOC_ADC_PEN_INTR_EN |
+			SIRFSOC_ADC_DATA_INTR_EN,
+			adc->base + SIRFSOC_ADC_INTR);
+
+	/* check pen status */
+	if (!(readl(adc->base + SIRFSOC_ADC_COORD) & SIRFSOC_ADC_PEN_DOWN)) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (SINGLE_TOUCH == touch_type)
+		writel(SIRFSOC_ADC_CTL1(SIRFSOC_ADC_TS_SEL),
+			adc->base + SIRFSOC_ADC_CONTROL1);
+	else
+		writel(SIRFSOC_ADC_CTL1(SIRFSOC_ADC_TS_SEL_DUAL),
+			adc->base + SIRFSOC_ADC_CONTROL1);
+
+	if (!wait_for_completion_timeout(&adc->done,
+		msecs_to_jiffies(50))) {
+		ret = -EIO;
+		goto out;
+	}
+
+	if (SINGLE_TOUCH == touch_type)
+		*sample = readl(adc->base + SIRFSOC_ADC_COORD);
Err, silly question but what the heck is the format of this?  Please don't
send multiple channels of real data through as 'RAW'.  If you do they'll
never work with anything other than carefully contrived non standard userspace.
+	else
+		for (i = 0; i < SIRFSOC_ADC_TS_SAMPLE_SIZE; i++)
+			sample[i] = readl(adc->base + sirfsoc_adc_ts_reg[i]);
Sample is a pointer to a single element so you can't do this.
+
+out:
+	mutex_unlock(&adc->adc_lock);
+	return ret;
+}
+
+/* get touchscreen coordinates for single touch */
+static int sirfsoc_adc_single_ts_sample(struct sirfsoc_adc *adc, int *sample)
+{
sample is pointing to a single integer...
+	return sirfsoc_adc_get_ts_sample(adc, sample, SINGLE_TOUCH);
+}
+
+/* get touchscreen coordinates for dual touch */
+static int sirfsoc_adc_dual_ts_sample(struct sirfsoc_adc *adc, int *samples)
+{
+	return sirfsoc_adc_get_ts_sample(adc, samples, DUAL_TOUCH);
+}
+
+static int sirfsoc_adc_send_request(struct sirfsoc_adc_request *req)
+{
+	struct sirfsoc_adc *adc = container_of(req, struct sirfsoc_adc, req);
+	int control1, control2, intr;
+	int data, reg_offset;
+	int ret = 0;
+
+	mutex_lock(&adc->adc_lock);
+
+	/* Store registers for recover */
+	intr = readl(adc->base + SIRFSOC_ADC_INTR);
+	control1 = readl(adc->base + SIRFSOC_ADC_CONTROL1);
+	control2 = readl(adc->base + SIRFSOC_ADC_CONTROL2);
+
+	writel(intr | SIRFSOC_ADC_DATA_INTR_EN | SIRFSOC_ADC_DATA_INTR,
+		adc->base + SIRFSOC_ADC_INTR);
+	writel(SIRFSOC_ADC_PRP_MODE3 | req->reference,
+		adc->base + SIRFSOC_ADC_CONTROL2);
+	writel(SIRFSOC_ADC_POLL | SIRFSOC_ADC_MORE_CTL1 | req->mode |
+		req->extcm | req->delay_bits | SIRFSOC_ADC_RESOLUTION_12,
+		adc->base + SIRFSOC_ADC_CONTROL1);
+
+	if (!wait_for_completion_timeout(&adc->done,
+		msecs_to_jiffies(50))) {
+		ret = -EIO;
+		goto out;
+	}
+
+	switch (req->mode) {
+	case SIRFSOC_ADC_SEL(SIRFSOC_ADC_AUX1_SEL):
+	case SIRFSOC_ADC_SEL(SIRFSOC_ADC_AUX2_SEL):
+	case SIRFSOC_ADC_SEL(SIRFSOC_ADC_AUX3_SEL):
+	case SIRFSOC_ADC_SEL(SIRFSOC_ADC_AUX4_SEL):
+	case SIRFSOC_ADC_SEL(SIRFSOC_ADC_AUX5_SEL):
+	case SIRFSOC_ADC_SEL(SIRFSOC_ADC_AUX6_SEL):
+		/* Calculate aux offset from mode */
+		reg_offset = 0x14 + ((req->mode >> 11) - 0x04) * 4;
+		data = readl(adc->base + reg_offset);
+		if (data & SIRFSOC_ADC_DATA_AUXVALID)
+			req->read_back_data = data & SIRFSOC_ADC_DATA_AUXMASK;
+		break;
+	case SIRFSOC_ADC_SEL(SIRFSOC_ADC_GROUND_SEL):
+	case SIRFSOC_ADC_SEL(SIRFSOC_ADC_BANDGAP_SEL):
+		reg_offset = SIRFSOC_ADC_CB;
+		data = readl(adc->base + reg_offset);
+		if (data & SIRFSOC_ADC_DATA_CB_VALID)
+			req->read_back_data = data & SIRFSOC_ADC_DATA_CBMASK;
+		break;
+	default:
+		break;
+	}
+
+out:
+	writel(intr, adc->base + SIRFSOC_ADC_INTR);
+	writel(control1, adc->base + SIRFSOC_ADC_CONTROL1);
+	writel(control2, adc->base + SIRFSOC_ADC_CONTROL2);
+	mutex_unlock(&adc->adc_lock);
+	return ret;
+}
+
+/* Store params to calibrate */
+struct sirfsoc_adc_cali_data {
+	u32 digital_offset;
+	u32 digital_again;
+	bool is_calibration;
+};
+
+/* Offset Calibration calibrates the ADC offset error */
+static u32 sirfsoc_adc_offset_cali(struct sirfsoc_adc_request *req)
+{
+	u32 i, digital_offset = 0, count = 0, sum = 0;
+	/* To set the registers in order to get the ADC offset */
+	req->mode = SIRFSOC_ADC_SEL(SIRFSOC_ADC_GROUND_SEL);
+	req->extcm = SIRFSOC_ADC_EXTCM(0);
+	req->s_gain_bits = SIRFSOC_ADC_SGAIN(7);
+	req->delay_bits = SIRFSOC_ADC_DEL_SET(4);
+
+	for (i = 0; i < 10; i++) {
+		if (sirfsoc_adc_send_request(req))
+			break;
+		digital_offset = req->read_back_data;
+		sum += digital_offset;
+		count++;
+	}
+	if (!count)
+		digital_offset = 0;
+	else
+		digital_offset = sum / count;
+
+	return digital_offset;
+}
+
+
+/* Gain Calibration calibrates the ADC gain error */
+static u32 sirfsoc_adc_gain_cali(struct sirfsoc_adc_request *req)
+{
+	u32 i, digital_gain = 0, count = 0, sum = 0;
+	/* To set the registers in order to get the ADC gain */
+	req->mode = SIRFSOC_ADC_SEL(SIRFSOC_ADC_BANDGAP_SEL);
+	req->extcm = SIRFSOC_ADC_EXTCM(0);
+	req->s_gain_bits = SIRFSOC_ADC_SGAIN(1);
+	req->delay_bits = SIRFSOC_ADC_DEL_SET(4);
+
+	for (i = 0; i < 10; i++) {
+		if (sirfsoc_adc_send_request(req))
+			break;
+		digital_gain = req->read_back_data;
+		sum += digital_gain;
+		count++;
+	}
+	if (!count)
+		digital_gain = 0;
+	else
+		digital_gain = sum / count;
+
+	return digital_gain;
+}
+
+/* Absolute gain calibration */
+static int sirfsoc_adc_adc_cali(struct sirfsoc_adc_request *req,
+				struct sirfsoc_adc_cali_data *cali_data)
+{
+	cali_data->digital_offset = sirfsoc_adc_offset_cali(req);
+	if (!cali_data->digital_offset)
+		return -EINVAL;
+	cali_data->digital_again = sirfsoc_adc_gain_cali(req);
+	if (!cali_data->digital_again)
+		return -EINVAL;
+
+	return 0;
+}
+
+/* Get voltage after ADC conversion */
+static u32 sirfsoc_adc_get_adc_volt(struct sirfsoc_adc *adc,
+				struct sirfsoc_adc_cali_data *cali_data)
+{
+	u32 digital_out, volt;
+	struct sirfsoc_adc_request *req = &adc->req;
+
+	req->s_gain_bits = SIRFSOC_ADC_SGAIN(0);
+	req->delay_bits = SIRFSOC_ADC_DEL_SET(4);
+
+	/*
+	 * First read original data
+	 * then read calibrate errors
+	 */
+	sirfsoc_adc_send_request(req);
+	if (req->read_back_data) {
+		digital_out = req->read_back_data;
+		/* Get errors for each sample */
+		if (!cali_data->is_calibration) {
+			if (sirfsoc_adc_adc_cali(req, cali_data))
+				return 0;
+			cali_data->is_calibration = true;
+		}
+
+		volt = adc->chip_info->calculate_volt(digital_out,
+			cali_data->digital_offset,
+			cali_data->digital_again);
+	} else {
+		return 0;
+	}
+
+	return volt;
+}
+
+static u32 prima2_adc_calculate_volt(u32 digital_out,
+				u32 digital_offset, u32 digital_again)
+{
+	u32 volt, digital_ideal, digital_convert;
+
+	/*
+	 * see Equation 3.2 of SiRFPrimaII™ Internal ADC and Touch
+	 * User Guide
+	 */
+	digital_ideal = (5522 * 1200) / (7 * 333);
+	/*
+	 * see Equation 3.3 of SiRFatlasVI™ Internal ADC and Touch
+	 */
+	digital_convert = (digital_out - 11645 * digital_offset /
+		140000) * digital_ideal / (digital_again
+		- digital_offset * 11645 / 70000);
+	volt = 14 * 333 * digital_convert / 5522;
+
+	return volt;
+}
+
+static u32 atlas6_adc_calculate_volt(u32 digital_out,
+				u32 digital_offset, u32 digital_again)
+{
+	u32 volt, digital_ideal, digital_convert;
+
+	/*
+	 * see Equation 3.2 of SiRFatlasVI™ Internal ADC and Touch
+	 * User Guide
+	 */
+	digital_ideal = (3986 * 12100) / (7 * 3333);
+	/*
+	 * see Equation 3.3 of SiRFatlasVI™ Internal ADC and Touch
+	 */
+	digital_offset &= 0xfff;
+	digital_convert = abs(digital_out - 2 * digital_offset)
+		* digital_ideal / (digital_again
+		- digital_offset * 2);
+	volt = 14 * 333 * digital_convert / 3986;
+	volt = volt / 2;
+	if (volt > 1500)
+		volt = volt - (volt - 1500) / 15;
+	else
+		volt = volt + (1500 - volt) / 28;
+
+	return volt;
+}
+
+static irqreturn_t sirfsoc_adc_data_irq(int irq, void *handle)
+{
+	struct iio_dev *indio_dev = handle;
+	struct sirfsoc_adc *adc = iio_priv(indio_dev);
+	int val;
+
+	val = readl(adc->base + SIRFSOC_ADC_INTR);
+
+	writel(SIRFSOC_ADC_PEN_INTR | SIRFSOC_ADC_PEN_INTR_EN |
+		SIRFSOC_ADC_DATA_INTR | SIRFSOC_ADC_DATA_INTR_EN,
+		adc->base + SIRFSOC_ADC_INTR);
+
+	if (val & SIRFSOC_ADC_DATA_INTR)
+		complete(&adc->done);
+
+	return IRQ_HANDLED;
+}
+
+static int sirfsoc_adc_read_raw(struct iio_dev *indio_dev,
+				struct iio_chan_spec const *chan,
+				int *val,
+				int *val2,
+				long mask)
+{
+	struct sirfsoc_adc *adc = iio_priv(indio_dev);
+	struct sirfsoc_adc_cali_data cali_data;
+	int ret;
+
+	cali_data.is_calibration = false;
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		if (0 == chan->channel)
+			ret = sirfsoc_adc_single_ts_sample(adc, val);
+		else
+			ret = sirfsoc_adc_dual_ts_sample(adc, val);
+
+		if (ret)
+			return ret;
+		return IIO_VAL_INT;
This will result in the first value only being output.  More to the
point *val only points to a single integer so chances are you'll crash
the kernel. I don't think it would be appropriate to use our more recent
support for multiple element outputs.  The two axes are independent after
all.  I'd really be happier seeing this done via the input subsystem.
(note that would require these channels not to be polled as done here, but
driven by interrupt or perhaps timer).

+	case IIO_CHAN_INFO_PROCESSED:
+		adc->req.mode = SIRFSOC_ADC_SEL(
+			adc->chip_info->channel_sel[chan->channel]);
+		adc->req.extcm = SIRFSOC_ADC_EXTCM(0);
+		*val = sirfsoc_adc_get_adc_volt(adc, &cali_data);
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int sirfsoc_adc_suspend(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct sirfsoc_adc *adc = iio_priv(indio_dev);
+
+	sirfsoc_rtc_iobrg_writel(sirfsoc_rtc_iobrg_readl(
+		SIRFSOC_PWRC_BASE + SIRFSOC_PWRC_TRIGGER_EN)
+		& ~BIT(SIRFSOC_PWR_WAKEEN_TSC_SHIFT),
+		SIRFSOC_PWRC_BASE + SIRFSOC_PWRC_TRIGGER_EN);
+
+	clk_disable_unprepare(adc->clk);
+
+	return 0;
+}
+
+static int sirfsoc_adc_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct sirfsoc_adc *adc = iio_priv(indio_dev);
+	int ret;
+	int val;
+
+	clk_prepare_enable(adc->clk);
+
+	sirfsoc_rtc_iobrg_writel(sirfsoc_rtc_iobrg_readl(
+		SIRFSOC_PWRC_BASE + SIRFSOC_PWRC_TRIGGER_EN) |
+		BIT(SIRFSOC_PWR_WAKEEN_TS_SHIFT),
+		SIRFSOC_PWRC_BASE + SIRFSOC_PWRC_TRIGGER_EN);
+
+	ret = device_reset(dev);
+	if (ret) {
+		dev_err(dev, "Failed to reset\n");
+		return ret;
+	}
+
+	writel(SIRFSOC_ADC_PRP_MODE3 | SIRFSOC_ADC_RTOUCH(1) |
+		SIRFSOC_ADC_DEL_PRE(2) | SIRFSOC_ADC_DEL_DIS(5),
+		adc->base + SIRFSOC_ADC_CONTROL2);
+
+	val = readl(adc->base + SIRFSOC_ADC_INTR);
+
+	/* Clear interrupts and enable PEN interrupt */
+	writel(val | SIRFSOC_ADC_PEN_INTR | SIRFSOC_ADC_DATA_INTR |
+		SIRFSOC_ADC_PEN_INTR_EN | SIRFSOC_ADC_DATA_INTR_EN,
+		adc->base + SIRFSOC_ADC_INTR);
+
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops sirfsoc_adc_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(sirfsoc_adc_suspend, sirfsoc_adc_resume)
+};
+
+#define SIRFSOC_ADC_CHANNEL(_channel, _type, _name, _mask) {	\
+	.type = _type,					\
+	.indexed = 1,					\
+	.channel = _channel,				\
+	.datasheet_name = _name,			\
+	.info_mask_separate = BIT(_mask),		\
+}
+
+#define SIRFSOC_ADC_TS_CHANNEL(_channel, _name)		\
+	SIRFSOC_ADC_CHANNEL(_channel, IIO_VOLTAGE,	\
+	_name, IIO_CHAN_INFO_RAW)
+
+#define SIRFSOC_ADC_AUX_CHANNEL(_channel, _name)	\
+	SIRFSOC_ADC_CHANNEL(_channel, IIO_VOLTAGE,	\
+	_name, IIO_CHAN_INFO_PROCESSED)
+
+static const struct iio_chan_spec prima2_adc_iio_channels[] = {
+	/* Channels to get the touch data */
+	SIRFSOC_ADC_TS_CHANNEL(0, "touch_coord"),
Passing a pair of coordinates through a single channel doesn't
really make sense. I'd be inclined to say you want a basic
input driver in here as well to handle these channels appropriately.
See the at91_adc driver for example.

+	/* Channels to get the pin input */
+	SIRFSOC_ADC_AUX_CHANNEL(1, "auxiliary1"),
+	SIRFSOC_ADC_AUX_CHANNEL(2, "auxiliary2"),
+	SIRFSOC_ADC_AUX_CHANNEL(3, "auxiliary3"),
+	SIRFSOC_ADC_AUX_CHANNEL(4, "auxiliary4"),
+	SIRFSOC_ADC_AUX_CHANNEL(5, "auxiliary5"),
+	SIRFSOC_ADC_AUX_CHANNEL(6, "auxiliary6"),
+};
+
+static const u32 prima2_adc_channel_sel[] = {
+	0, SIRFSOC_ADC_AUX1_SEL, SIRFSOC_ADC_AUX2_SEL,
+	SIRFSOC_ADC_AUX3_SEL, SIRFSOC_ADC_AUX4_SEL,
+	SIRFSOC_ADC_AUX5_SEL, SIRFSOC_ADC_AUX6_SEL
+};
+
+static const struct iio_chan_spec atlas6_adc_iio_channels[] = {
+	/* Channels to get the touch data */
+	SIRFSOC_ADC_TS_CHANNEL(0, "touch_coord"),
+	SIRFSOC_ADC_TS_CHANNEL(1, "dual_touch_coord"),
+	/* Channels to get the pin input */
+	SIRFSOC_ADC_AUX_CHANNEL(2, "auxiliary1"),
+	/* Atlas6 has no auxiliary2 and auxiliary3 */
+	SIRFSOC_ADC_AUX_CHANNEL(3, "auxiliary4"),
+	SIRFSOC_ADC_AUX_CHANNEL(4, "auxiliary5"),
+	SIRFSOC_ADC_AUX_CHANNEL(5, "auxiliary6"),
+};
+
+static const u32 atlas6_adc_channel_sel[] = {
+	0, 0, SIRFSOC_ADC_AUX1_SEL, SIRFSOC_ADC_AUX4_SEL,
+	SIRFSOC_ADC_AUX5_SEL, SIRFSOC_ADC_AUX6_SEL
+};
+
+static const struct iio_info sirfsoc_adc_info = {
+	.read_raw = &sirfsoc_adc_read_raw,
+	.driver_module = THIS_MODULE,
+};
+
+static const struct sirfsoc_adc_chip_info sirfsoc_adc_chip_info_tbl[] = {
+	[PRIMA2] = {
+		.channels = prima2_adc_iio_channels,
+		.num_channels = ARRAY_SIZE(prima2_adc_iio_channels),
+		.iio_info = &sirfsoc_adc_info,
+		.calculate_volt = prima2_adc_calculate_volt,
+		.channel_sel = prima2_adc_channel_sel,
+	},
+	[ATLAS6] = {
+		.channels = atlas6_adc_iio_channels,
+		.num_channels = ARRAY_SIZE(atlas6_adc_iio_channels),
+		.iio_info = &sirfsoc_adc_info,
+		.calculate_volt = atlas6_adc_calculate_volt,
+		.channel_sel = atlas6_adc_channel_sel,
+	},
+};
+
+static const struct of_device_id sirfsoc_adc_of_match[] = {
+	{ .compatible = "sirf,prima2-adc",
+	  .data = &sirfsoc_adc_chip_info_tbl[PRIMA2] },
+	{ .compatible = "sirf,atlas6-adc",
+	  .data = &sirfsoc_adc_chip_info_tbl[ATLAS6] },
+	{}
+};
+MODULE_DEVICE_TABLE(of, sirfsoc_adc_of_match);
+
+static int sirfsoc_adc_probe(struct platform_device *pdev)
+{
+	struct resource	*mem_res;
+	struct sirfsoc_adc *adc;
+	struct iio_dev *indio_dev;
+	const struct of_device_id *match;
+	int irq;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&pdev->dev,
+			sizeof(struct sirfsoc_adc));
sizeof(*adc)); Shorter and slightly more obviously correct given that's what
you sent to iio_priv(indio_dev);
+	if (!indio_dev)
+		return -ENOMEM;
+
+	adc = iio_priv(indio_dev);
+
+	/* ADC specific data */
+	match = of_match_device(
+		of_match_ptr(sirfsoc_adc_of_match), &pdev->dev);
+	adc->chip_info = match->data;
+
+	indio_dev->info = adc->chip_info->iio_info;
+	indio_dev->channels = adc->chip_info->channels;
+	indio_dev->num_channels = adc->chip_info->num_channels;
+	indio_dev->name = "sirfsoc adc";
+	indio_dev->dev.parent = &pdev->dev;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	platform_set_drvdata(pdev, indio_dev);
+
+	adc->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(adc->clk)) {
+		dev_err(&pdev->dev, "Get adc clk failed\n");
+		ret = -ENOMEM;
+		goto err;
+	}
+	clk_prepare_enable(adc->clk);
Is it just me who feels that a devm_clk_prepare_enable would be nice, or
for really short code : devm_clk_get_prepare_enable :)
Also, clk_prepare_enable can return errors so should be checked.
+
+	mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!mem_res) {
+		dev_err(&pdev->dev, "Unable to get io resource\n");
+		ret = -ENODEV;
+		goto err;
+	}
+
+	adc->base = devm_request_and_ioremap(&pdev->dev, mem_res);
Quoting from a review Lars did earlier today:

devm_request_and_ioremap() is deprecated an will be removed in the next
release[1]. Use devm_ioremap_resource(), note that devm_ioremap_resource()
returns a ERR_PTR instead of NULL on error.

[1] https://lkml.org/lkml/2014/6/11/26

+	if (!adc->base) {
+		dev_err(&pdev->dev, "IO remap failed!\n");
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	init_completion(&adc->done);
+	mutex_init(&adc->adc_lock);
+
+	sirfsoc_rtc_iobrg_writel(sirfsoc_rtc_iobrg_readl(SIRFSOC_PWRC_BASE +
+		SIRFSOC_PWRC_TRIGGER_EN) | BIT(SIRFSOC_PWR_WAKEEN_TS_SHIFT),
+		SIRFSOC_PWRC_BASE + SIRFSOC_PWRC_TRIGGER_EN);
+
+	ret = device_reset(&pdev->dev);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to reset\n");
+		goto err;
+	}
+
+	writel(SIRFSOC_ADC_PRP_MODE3 | SIRFSOC_ADC_RTOUCH(1) |
+		SIRFSOC_ADC_DEL_PRE(2) | SIRFSOC_ADC_DEL_DIS(5),
+		adc->base + SIRFSOC_ADC_CONTROL2);
+
+	/* Clear interrupts and enable PEN INTR */
+	writel(readl(adc->base + SIRFSOC_ADC_INTR) | SIRFSOC_ADC_PEN_INTR |
+		SIRFSOC_ADC_DATA_INTR | SIRFSOC_ADC_PEN_INTR_EN |
+		SIRFSOC_ADC_DATA_INTR_EN,  adc->base + SIRFSOC_ADC_INTR);
+
double space above.

+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "Failed to get IRQ!\n");
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	ret = devm_request_irq(&pdev->dev, irq, sirfsoc_adc_data_irq,
+		0, DRIVER_NAME, indio_dev);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to register irq handler\n");
+		ret = -ENODEV;
+		goto err;
+	}
+
+	ret = of_platform_populate(pdev->dev.of_node, sirfsoc_adc_of_match,
+		NULL, &pdev->dev);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to add child nodes\n");
+		goto err;
+	}
+
+	ret = iio_device_register(indio_dev);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to register adc iio dev\n");
Nitpick: ADC IIO
+		goto err;
+	}
+
+	return 0;
+
+err:
+	clk_disable_unprepare(adc->clk);
+
+	return ret;
+}
+
+static int sirfsoc_adc_remove(struct platform_device *pdev)
+{
+	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+	struct sirfsoc_adc *adc = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+	clk_disable_unprepare(adc->clk);
+
+	return 0;
+}
+
+static struct platform_driver sirfsoc_adc_driver = {
+	.driver = {
+		.name	= DRIVER_NAME,
+		.of_match_table = sirfsoc_adc_of_match,
+		.pm	= &sirfsoc_adc_pm_ops,
+	},
+	.probe		= sirfsoc_adc_probe,
+	.remove		= sirfsoc_adc_remove,
+};
+
+module_platform_driver(sirfsoc_adc_driver);
+
+MODULE_AUTHOR("Guoying Zhang <Guoying.Zhang@xxxxxxx>");
+MODULE_DESCRIPTION("SiRF SoC On-chip ADC driver");
+MODULE_LICENSE("GPL v2");


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




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux