All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] mainline ti tsc2046 adc driver
@ 2021-03-12 10:55 Oleksij Rempel
  2021-03-12 10:55 ` [PATCH v2 1/3] dt-bindings:iio:adc: add generic settling-time-us and average-samples channel properties Oleksij Rempel
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Oleksij Rempel @ 2021-03-12 10:55 UTC (permalink / raw)
  To: Rob Herring, Jonathan Cameron
  Cc: Oleksij Rempel, devicetree, linux-kernel,
	Pengutronix Kernel Team, David Jander, Robin van der Gracht,
	linux-iio, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Dmitry Torokhov

changes v2:
- rework and extend DT binding properties
- remove touchscreen related code from the IIO ADC driver
- make trigger be active longer then IRQ is requesting. This is needed
  to get "inactive" samples
- make oversampling and settle time configurable

TI TSC2046 is a touchscreen controller based on 8 channel ADC. Since most of
this ADC based touchscreen controller share same set of challenges, it
is better keep then as simple IIO ADC devices attached to a generic
resistive-adc-touch driver.

This driver can replace drivers/input/touchscreen/ads7846.c and has
following advantages over it:
- less code to maintain
- shared code paths (resistive-adc-touch, iio-hwmon, etc)
- can be used as plain IIO ADC to investigate signaling issues or test
  real capacity of the plates and attached low-pass filters
  (or use the touchscreen as a microphone if you like ;) )

Oleksij Rempel (3):
  dt-bindings:iio:adc: add generic settling-time-us and average-samples
    channel properties
  dt-bindings:iio:adc: add documentation for TI TSC2046 controller
  iio: adc: add ADC driver for the TI TSC2046 controller

 .../devicetree/bindings/iio/adc/adc.yaml      |   9 +
 .../bindings/iio/adc/ti,tsc2046.yaml          | 115 +++
 MAINTAINERS                                   |   8 +
 drivers/iio/adc/Kconfig                       |  12 +
 drivers/iio/adc/Makefile                      |   1 +
 drivers/iio/adc/ti-tsc2046.c                  | 713 ++++++++++++++++++
 6 files changed, 858 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,tsc2046.yaml
 create mode 100644 drivers/iio/adc/ti-tsc2046.c

-- 
2.29.2


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/3] dt-bindings:iio:adc: add generic settling-time-us and average-samples channel properties
  2021-03-12 10:55 [PATCH v2 0/3] mainline ti tsc2046 adc driver Oleksij Rempel
@ 2021-03-12 10:55 ` Oleksij Rempel
  2021-03-13 15:15   ` Jonathan Cameron
  2021-03-12 10:55 ` [PATCH v2 2/3] dt-bindings:iio:adc: add documentation for TI TSC2046 controller Oleksij Rempel
  2021-03-12 10:55 ` [PATCH v2 3/3] iio: adc: add ADC driver for the " Oleksij Rempel
  2 siblings, 1 reply; 11+ messages in thread
From: Oleksij Rempel @ 2021-03-12 10:55 UTC (permalink / raw)
  To: Rob Herring, Jonathan Cameron
  Cc: Oleksij Rempel, devicetree, linux-kernel,
	Pengutronix Kernel Team, David Jander, Robin van der Gracht,
	linux-iio, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Dmitry Torokhov

Settling time and over sampling is a typical challenge for different IIO ADC
devices. So, introduce channel specific settling-time-us and average-samples
properties to cover this use case.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 Documentation/devicetree/bindings/iio/adc/adc.yaml | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/adc.yaml b/Documentation/devicetree/bindings/iio/adc/adc.yaml
index 912a7635edc4..c748f6573027 100644
--- a/Documentation/devicetree/bindings/iio/adc/adc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/adc.yaml
@@ -39,4 +39,13 @@ properties:
       The first value specifies the positive input pin, the second
       specifies the negative input pin.
 
+  settling-time-us:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Time between enabling the channel and firs stable readings.
+
+  average-samples:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Number of data samples which are averaged for each read.
+
 additionalProperties: true
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/3] dt-bindings:iio:adc: add documentation for TI TSC2046 controller
  2021-03-12 10:55 [PATCH v2 0/3] mainline ti tsc2046 adc driver Oleksij Rempel
  2021-03-12 10:55 ` [PATCH v2 1/3] dt-bindings:iio:adc: add generic settling-time-us and average-samples channel properties Oleksij Rempel
@ 2021-03-12 10:55 ` Oleksij Rempel
  2021-03-15 16:20   ` Rob Herring
  2021-03-12 10:55 ` [PATCH v2 3/3] iio: adc: add ADC driver for the " Oleksij Rempel
  2 siblings, 1 reply; 11+ messages in thread
From: Oleksij Rempel @ 2021-03-12 10:55 UTC (permalink / raw)
  To: Rob Herring, Jonathan Cameron
  Cc: Oleksij Rempel, devicetree, linux-kernel,
	Pengutronix Kernel Team, David Jander, Robin van der Gracht,
	linux-iio, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Dmitry Torokhov

Add a binding documentation for the TI TSC2046 touchscreen controllers
ADC functionality.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 .../bindings/iio/adc/ti,tsc2046.yaml          | 115 ++++++++++++++++++
 1 file changed, 115 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,tsc2046.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/ti,tsc2046.yaml b/Documentation/devicetree/bindings/iio/adc/ti,tsc2046.yaml
new file mode 100644
index 000000000000..4f705dc0f639
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ti,tsc2046.yaml
@@ -0,0 +1,115 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/ti,tsc2046.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments TSC2046 touch screen controller.
+
+maintainers:
+  - Oleksij Rempel <o.rempel@pengutronix.de>
+
+description: |
+  TSC2046 is a touch screen controller with 8 channels ADC.
+
+properties:
+  compatible:
+    enum:
+      - ti,tsc2046e-adc
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  spi-max-frequency: true
+
+  "#io-channel-cells":
+    const: 1
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+required:
+  - compatible
+  - reg
+
+patternProperties:
+  "^channel@[0-7]$":
+    $ref: "adc.yaml"
+    type: object
+
+    properties:
+      reg:
+        description: |
+          The channel number. It can have up to 8 channels
+        items:
+          minimum: 0
+          maximum: 7
+
+      settling-time-us: true
+      average-samples: true
+
+    required:
+      - reg
+
+    additionalProperties: false
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@0 {
+            compatible = "ti,tsc2046e-adc";
+            reg = <0>;
+            spi-max-frequency = <1000000>;
+            interrupts-extended = <&gpio3 20 IRQ_TYPE_LEVEL_LOW>;
+            #io-channel-cells = <1>;
+
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            channel@0 {
+              reg = <0>;
+            };
+            channel@1 {
+              reg = <1>;
+              settling-time-us = <700>;
+              average-samples = <5>;
+            };
+            channel@2 {
+              reg = <2>;
+            };
+            channel@3 {
+              reg = <3>;
+              settling-time-us = <700>;
+              average-samples = <5>;
+            };
+            channel@4 {
+              reg = <4>;
+              settling-time-us = <700>;
+              average-samples = <5>;
+            };
+            channel@5 {
+              reg = <5>;
+              settling-time-us = <700>;
+              average-samples = <5>;
+            };
+            channel@6 {
+              reg = <6>;
+            };
+            channel@7 {
+              reg = <7>;
+            };
+        };
+    };
+...
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 3/3] iio: adc: add ADC driver for the TI TSC2046 controller
  2021-03-12 10:55 [PATCH v2 0/3] mainline ti tsc2046 adc driver Oleksij Rempel
  2021-03-12 10:55 ` [PATCH v2 1/3] dt-bindings:iio:adc: add generic settling-time-us and average-samples channel properties Oleksij Rempel
  2021-03-12 10:55 ` [PATCH v2 2/3] dt-bindings:iio:adc: add documentation for TI TSC2046 controller Oleksij Rempel
@ 2021-03-12 10:55 ` Oleksij Rempel
  2021-03-12 15:32   ` Andy Shevchenko
                     ` (2 more replies)
  2 siblings, 3 replies; 11+ messages in thread
From: Oleksij Rempel @ 2021-03-12 10:55 UTC (permalink / raw)
  To: Rob Herring, Jonathan Cameron
  Cc: Oleksij Rempel, devicetree, linux-kernel,
	Pengutronix Kernel Team, David Jander, Robin van der Gracht,
	linux-iio, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Dmitry Torokhov

Basically the TI TSC2046 touchscreen controller is 8 channel ADC optimized for
the touchscreen use case. By implementing it as IIO ADC device, we can
make use of resistive-adc-touch and iio-hwmon drivers.

So far, this driver was tested with custom version of resistive-adc-touch driver,
since it need to be extended to make use of Z1 and Z2 channels. The X/Y
are working without additional changes.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 MAINTAINERS                  |   8 +
 drivers/iio/adc/Kconfig      |  12 +
 drivers/iio/adc/Makefile     |   1 +
 drivers/iio/adc/ti-tsc2046.c | 713 +++++++++++++++++++++++++++++++++++
 4 files changed, 734 insertions(+)
 create mode 100644 drivers/iio/adc/ti-tsc2046.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 3fea1a934b32..2d33c6442a55 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17852,6 +17852,14 @@ S:	Supported
 F:	Documentation/devicetree/bindings/net/nfc/trf7970a.txt
 F:	drivers/nfc/trf7970a.c
 
+TI TSC2046 ADC DRIVER
+M:	Oleksij Rempel <o.rempel@pengutronix.de>
+R:	kernel@pengutronix.de
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/adc/ti,tsc2046.yaml
+F:	drivers/iio/adc/ti-tsc2046.c
+
 TI TWL4030 SERIES SOC CODEC DRIVER
 M:	Peter Ujfalusi <peter.ujfalusi@gmail.com>
 L:	alsa-devel@alsa-project.org (moderated for non-subscribers)
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 15587a1bc80d..6ad6f04dfd20 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1175,6 +1175,18 @@ config TI_TLC4541
 	  This driver can also be built as a module. If so, the module will be
 	  called ti-tlc4541.
 
+config TI_TSC2046
+	tristate "Texas Instruments TSC2046 ADC driver"
+	depends on SPI
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+	help
+	  Say yes here to build support for ADC functionality of Texas
+	  Instruments TSC2046 touch screen controller.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called ti-tsc2046.
+
 config TWL4030_MADC
 	tristate "TWL4030 MADC (Monitoring A/D Converter)"
 	depends on TWL4030_CORE
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 5fca90ada0ec..440e18ac6780 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -105,6 +105,7 @@ obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o
 obj-$(CONFIG_TI_ADS124S08) += ti-ads124s08.o
 obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
 obj-$(CONFIG_TI_TLC4541) += ti-tlc4541.o
+obj-$(CONFIG_TI_TSC2046) += ti-tsc2046.o
 obj-$(CONFIG_TWL4030_MADC) += twl4030-madc.o
 obj-$(CONFIG_TWL6030_GPADC) += twl6030-gpadc.o
 obj-$(CONFIG_VF610_ADC) += vf610_adc.o
diff --git a/drivers/iio/adc/ti-tsc2046.c b/drivers/iio/adc/ti-tsc2046.c
new file mode 100644
index 000000000000..7c3ae9181164
--- /dev/null
+++ b/drivers/iio/adc/ti-tsc2046.c
@@ -0,0 +1,713 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Texas Instruments TSC2046 SPI ADC driver
+ *
+ * Copyright (c) 2021 Oleksij Rempel <kernel@pengutronix.de>, Pengutronix
+ */
+
+#include <linux/bitfield.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/spi/spi.h>
+
+#include <asm/unaligned.h>
+
+#include <linux/iio/buffer.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger.h>
+
+#define TI_TSC2046_NAME				"tsc2046"
+
+/* This driver doesn't aim at the peak continuous sample rate */
+#define	TI_TSC2046_MAX_SAMPLE_RATE		125000
+#define	TI_TSC2046_SAMPLE_BITS			(8 /*cmd*/ + 16 /*sample*/)
+#define	TI_TSC2046_MAX_CLK_FREQ \
+	(TI_TSC2046_MAX_SAMPLE_RATE * TI_TSC2046_SAMPLE_BITS)
+
+#define TI_TSC2046_SAMPLE_INTERVAL_US		10000
+
+#define TI_TSC2046_START			BIT(7)
+#define TI_TSC2046_ADDR				GENMASK(6, 4)
+#define TI_TSC2046_ADDR_TEMP1			7
+#define TI_TSC2046_ADDR_AUX			6
+#define TI_TSC2046_ADDR_X			5
+#define TI_TSC2046_ADDR_Z2			4
+#define TI_TSC2046_ADDR_Z1			3
+#define TI_TSC2046_ADDR_VBAT			2
+#define TI_TSC2046_ADDR_Y			1
+#define TI_TSC2046_ADDR_TEMP0			0
+
+/*
+ * The mode bit sets the resolution of the ADC. With this bit low, the next
+ * conversion has 12-bit resolution, whereas with this bit high, the next
+ * conversion has 8-bit resolution. This driver is optimized for 12-bit mode.
+ * So, for this driver, this bit should stay zero.
+ */
+#define TI_TSC2046_8BIT_MODE			BIT(3)
+
+/*
+ * SER/DFR - The SER/DFR bit controls the reference mode, either single-ended
+ * (high) or differential (low).
+ */
+#define TI_TSC2046_SER				BIT(2)
+
+/*
+ * If VREF_ON and ADC_ON are both zero, then the chip operates in
+ * auto-wake/suspend mode. In most case this bits should stay zero.
+ */
+#define TI_TSC2046_PD1_VREF_ON			BIT(1)
+#define TI_TSC2046_PD0_ADC_ON			BIT(0)
+
+#define TI_TSC2046_MAX_CHAN			8
+
+/* Represents a HW sample */
+struct tsc2046_adc_atom {
+	/*
+	 * Command transmitted to the controller. This filed is empty on the RX
+	 * buffer.
+	 */
+	u8 cmd;
+	/*
+	 * Data received from the controller. This filed is empty for the TX
+	 * buffer
+	 */
+	__be16 data;
+} __packed;
+
+struct tsc2046_adc_scan_buf {
+	/* Scan data for each channel */
+	u16 data[TI_TSC2046_MAX_CHAN];
+	/* Timestamp */
+	s64 ts __aligned(8);
+};
+
+/* Layout of atomic buffers within big buffer */
+struct tsc2046_adc_group_layout {
+	/* Group offset within the SPI RX buffer */
+	unsigned int offset;
+	/*
+	 * Amount of tsc2046_adc_atom structs within the same command gathered
+	 * within same group.
+	 */
+	unsigned int count;
+	/*
+	 * Settling samples (tsc2046_adc_atom structs) which should be skipped
+	 * before good samples will start.
+	 */
+	unsigned int skip;
+};
+
+struct tsc2046_adc_dcfg {
+	const struct iio_chan_spec *channels;
+	unsigned int num_channels;
+};
+
+struct tsc2046_adc_ch_cfg {
+	unsigned int settling_time_us;
+	unsigned int average_samples;
+};
+
+struct tsc2046_adc_priv {
+	struct spi_device *spi;
+	const struct tsc2046_adc_dcfg *dcfg;
+
+	struct iio_trigger *trig;
+	struct hrtimer trig_timer;
+	spinlock_t trig_lock;
+	atomic_t trig_more_count;
+
+	struct spi_transfer xfer;
+	struct spi_message msg;
+
+	struct tsc2046_adc_scan_buf scan_buf;
+	/*
+	 * Lock to protect the layout and the spi transfer buffer.
+	 * tsc2046_adc_group_layout can be changed within update_scan_mode(),
+	 * in this case the l[] and tx/rx buffer will be out of sync to each
+	 * other.
+	 */
+	struct mutex slock;
+	struct tsc2046_adc_group_layout l[TI_TSC2046_MAX_CHAN];
+	struct tsc2046_adc_atom *rx;
+	struct tsc2046_adc_atom *tx;
+
+	struct tsc2046_adc_atom *rx_one;
+	struct tsc2046_adc_atom *tx_one;
+
+	unsigned int count;
+	unsigned int groups;
+	u32 effective_speed_hz;
+	u32 scan_interval_us;
+	u32 time_per_scan_us;
+	u32 time_per_bit_ns;
+
+	struct tsc2046_adc_ch_cfg ch_cfg[TI_TSC2046_MAX_CHAN];
+};
+
+#define TI_TSC2046_V_CHAN(index, bits, name)			\
+{								\
+	.type = IIO_VOLTAGE,					\
+	.indexed = 1,						\
+	.channel = index,					\
+	.address = index,					\
+	.datasheet_name = "#name",				\
+	.scan_index = index,					\
+	.scan_type = {						\
+		.sign = 'u',					\
+		.realbits = bits,				\
+		.storagebits = 16,				\
+		.shift = 0,					\
+		.endianness = IIO_CPU,				\
+	},							\
+}
+
+#define DECLARE_TI_TSC2046_8_CHANNELS(name, bits) \
+const struct iio_chan_spec name ## _channels[] = { \
+	TI_TSC2046_V_CHAN(0, bits, TEMP0), \
+	TI_TSC2046_V_CHAN(1, bits, Y), \
+	TI_TSC2046_V_CHAN(2, bits, VBAT), \
+	TI_TSC2046_V_CHAN(3, bits, Z1), \
+	TI_TSC2046_V_CHAN(4, bits, Z2), \
+	TI_TSC2046_V_CHAN(5, bits, X), \
+	TI_TSC2046_V_CHAN(6, bits, AUX), \
+	TI_TSC2046_V_CHAN(7, bits, TEMP1), \
+	IIO_CHAN_SOFT_TIMESTAMP(8), \
+}
+
+static DECLARE_TI_TSC2046_8_CHANNELS(tsc2046_adc, 12);
+
+static const struct tsc2046_adc_dcfg tsc2046_adc_dcfg_tsc2046e = {
+	.channels = tsc2046_adc_channels,
+	.num_channels = ARRAY_SIZE(tsc2046_adc_channels),
+};
+
+/*
+ * Convert time to a number of samples which can be transferred within this
+ * time.
+ */
+static unsigned int tsc2046_adc_time_to_count(struct tsc2046_adc_priv *priv,
+					      unsigned long time)
+{
+	unsigned int bit_count, sample_count;
+
+	bit_count = DIV_ROUND_UP(time * NSEC_PER_USEC, priv->time_per_bit_ns);
+	sample_count = DIV_ROUND_UP(bit_count, TI_TSC2046_SAMPLE_BITS);
+
+	dev_dbg(&priv->spi->dev, "Effective speed %u, time per bit: %u, count bits: %u, count samples: %u\n",
+		priv->effective_speed_hz, priv->time_per_bit_ns,
+		bit_count, sample_count);
+
+	return sample_count;
+}
+
+static u8 tsc2046_adc_get_cmd(struct tsc2046_adc_priv *priv, int ch_idx,
+			      bool keep_power)
+{
+	u32 pd;
+
+	/*
+	 * if PD bits are 0, controller will automatically disable ADC, VREF and
+	 * enable IRQ.
+	 */
+	if (keep_power)
+		pd = TI_TSC2046_PD0_ADC_ON;
+	else
+		pd = 0;
+
+	return TI_TSC2046_START | FIELD_PREP(TI_TSC2046_ADDR, ch_idx) | pd;
+}
+
+static u16 tsc2046_adc_get_value(struct tsc2046_adc_atom *buf)
+{
+	/* Last 3 bits on the wire are empty */
+	return get_unaligned_be16(&buf->data) >> 3;
+}
+
+static int tsc2046_adc_read_one(struct tsc2046_adc_priv *priv, int ch_idx,
+				u32 *effective_speed_hz)
+{
+	struct spi_transfer xfer;
+	struct spi_message msg;
+	int ret;
+
+	memset(&xfer, 0, sizeof(xfer));
+	priv->tx_one->cmd = tsc2046_adc_get_cmd(priv, ch_idx, false);
+	priv->tx_one->data = 0;
+	xfer.tx_buf = priv->tx_one;
+	xfer.rx_buf = priv->rx_one;
+	xfer.len = sizeof(*priv->tx_one);
+	spi_message_init(&msg);
+	spi_message_add_tail(&xfer, &msg);
+
+	ret = spi_sync(priv->spi, &msg);
+	if (ret) {
+		dev_err_ratelimited(&priv->spi->dev, "SPI transfer filed %pe\n",
+				    ERR_PTR(ret));
+		return ret;
+	}
+
+	if (effective_speed_hz)
+		*effective_speed_hz = xfer.effective_speed_hz;
+
+	return tsc2046_adc_get_value(priv->rx_one);
+}
+
+static size_t tsc2046_adc_group_set_layout(struct tsc2046_adc_priv *priv,
+					   unsigned int group,
+					   unsigned int ch_idx)
+{
+	struct tsc2046_adc_ch_cfg *ch = &priv->ch_cfg[ch_idx];
+	struct tsc2046_adc_group_layout *prev, *cur;
+	unsigned int max_count, count_skip;
+	unsigned int offset = 0;
+
+
+	if (group) {
+		prev = &priv->l[group - 1];
+		offset = prev->offset + prev->count;
+	}
+
+	cur = &priv->l[group];
+
+	count_skip = tsc2046_adc_time_to_count(priv, ch->settling_time_us);
+	max_count = count_skip + ch->average_samples;
+
+	cur->offset = offset;
+	cur->count = max_count;
+	cur->skip = count_skip;
+
+	return sizeof(*priv->tx) * max_count;
+}
+
+static void tsc2046_adc_group_set_cmd(struct tsc2046_adc_priv *priv,
+				      unsigned int group, int ch_idx)
+{
+	struct tsc2046_adc_group_layout *l = &priv->l[group];
+	unsigned int i;
+	u8 cmd;
+
+	/*
+	 * Do not enable automatic power down on working samples. Otherwise the
+	 * plates will never be completely charged.
+	 */
+	cmd = tsc2046_adc_get_cmd(priv, ch_idx, true);
+
+	for (i = 0; i < l->count - 1; i++)
+		priv->tx[l->offset + i].cmd = cmd;
+
+	/* automatically power down on last sample */
+	priv->tx[l->offset + i].cmd = tsc2046_adc_get_cmd(priv, ch_idx, false);
+}
+
+static u16 tsc2046_adc_get_val(struct tsc2046_adc_priv *priv, int group)
+{
+	struct tsc2046_adc_group_layout *l;
+	unsigned int val, val_normalized = 0;
+	int valid_count, i;
+
+	l = &priv->l[group];
+	valid_count = l->count - l->skip;
+
+	for (i = 0; i < valid_count; i++) {
+		val = tsc2046_adc_get_value(&priv->rx[l->offset + l->skip + i]);
+		val_normalized += val;
+	}
+
+	return DIV_ROUND_UP(val_normalized, valid_count);
+}
+
+static int tsc2046_adc_scan(struct iio_dev *indio_dev)
+{
+	struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
+	struct device *dev = &priv->spi->dev;
+	int group;
+	int ret;
+
+	ret = spi_sync(priv->spi, &priv->msg);
+	if (ret < 0) {
+		dev_err_ratelimited(dev, "SPI transfer filed: %pe\n",
+				    ERR_PTR(ret));
+		return ret;
+	}
+
+	for (group = 0; group < priv->groups; group++) {
+		u16 val = tsc2046_adc_get_val(priv, group);
+
+		priv->scan_buf.data[group] = val;
+	}
+
+	ret = iio_push_to_buffers_with_timestamp(indio_dev, &priv->scan_buf,
+						 iio_get_time_ns(indio_dev));
+	/*
+	 * If there's no consumer (or consumer is kfifo), may get a EBUSY here
+	 * - ignore it.
+	 */
+	if (ret < 0 && ret != -EBUSY) {
+		dev_err_ratelimited(dev, "Filed to push scan buffer %pe\n",
+				    ERR_PTR(ret));
+
+		return ret;
+	}
+
+	return 0;
+}
+
+static irqreturn_t tsc2046_adc_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
+
+	mutex_lock(&priv->slock);
+	tsc2046_adc_scan(indio_dev);
+	mutex_unlock(&priv->slock);
+
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
+static int tsc2046_adc_update_scan_mode(struct iio_dev *indio_dev,
+					const unsigned long *active_scan_mask)
+{
+	struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
+	unsigned int ch_idx, group = 0;
+	size_t size = 0;
+
+	mutex_lock(&priv->slock);
+
+	for_each_set_bit(ch_idx, active_scan_mask, indio_dev->num_channels) {
+		size += tsc2046_adc_group_set_layout(priv, group, ch_idx);
+		tsc2046_adc_group_set_cmd(priv, group, ch_idx);
+		group++;
+	}
+
+	priv->groups = group;
+	priv->xfer.len = size;
+	priv->time_per_scan_us = size * 8 * priv->time_per_bit_ns / NSEC_PER_USEC;
+
+	if ((priv->scan_interval_us - priv->time_per_scan_us) < 0)
+		dev_warn(&priv->spi->dev, "The scan interval (%d) is less then calculated scan time (%d)\n",
+			 priv->scan_interval_us, priv->time_per_scan_us);
+
+	mutex_unlock(&priv->slock);
+
+	return 0;
+}
+
+static const struct iio_info tsc2046_adc_info = {
+	.update_scan_mode = tsc2046_adc_update_scan_mode,
+};
+
+static enum hrtimer_restart tsc2046_adc_trig_more(struct hrtimer *hrtimer)
+{
+	struct tsc2046_adc_priv *priv = container_of(hrtimer,
+						     struct tsc2046_adc_priv,
+						     trig_timer);
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->trig_lock, flags);
+
+	disable_irq_nosync(priv->spi->irq);
+
+	atomic_inc(&priv->trig_more_count);
+	iio_trigger_poll(priv->trig);
+
+	spin_unlock_irqrestore(&priv->trig_lock, flags);
+
+	return HRTIMER_NORESTART;
+}
+
+static irqreturn_t tsc2046_adc_irq(int irq, void *dev_id)
+{
+	struct iio_dev *indio_dev = dev_id;
+	struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
+
+
+	spin_lock(&priv->trig_lock);
+
+	hrtimer_try_to_cancel(&priv->trig_timer);
+
+	atomic_set(&priv->trig_more_count, 0);
+	disable_irq_nosync(priv->spi->irq);
+	iio_trigger_poll(priv->trig);
+
+	spin_unlock(&priv->trig_lock);
+
+	return IRQ_HANDLED;
+}
+
+static void tsc2046_adc_reenable_trigger(struct iio_trigger *trig)
+{
+	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
+	struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
+	unsigned long flags;
+	int delta;
+
+	/*
+	 * We can sample it as fast as we can, but usually we do not need so
+	 * many samples. Reduce the sample rate for default (touchscreen) use
+	 * case.
+	 * Currently we do not need a highly precise sample rate. It is enough
+	 * to have calculated numbers.
+	 */
+	delta = priv->scan_interval_us - priv->time_per_scan_us;
+	if (delta > 0)
+		fsleep(delta);
+
+	spin_lock_irqsave(&priv->trig_lock, flags);
+
+	/*
+	 * We need to trigger at least one extra sample to detect state
+	 * difference on ADC side.
+	 */
+	if (atomic_read(&priv->trig_more_count) == 0) {
+		int timeout_ms = DIV_ROUND_UP(priv->scan_interval_us,
+					      USEC_PER_MSEC);
+
+		hrtimer_start(&priv->trig_timer, ms_to_ktime(timeout_ms),
+			      HRTIMER_MODE_REL_SOFT);
+	}
+
+	enable_irq(priv->spi->irq);
+
+	spin_unlock_irqrestore(&priv->trig_lock, flags);
+}
+
+static int tsc2046_adc_set_trigger_state(struct iio_trigger *trig, bool enable)
+{
+	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
+	struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
+
+	if (enable) {
+		enable_irq(priv->spi->irq);
+	} else {
+		disable_irq(priv->spi->irq);
+		hrtimer_try_to_cancel(&priv->trig_timer);
+	}
+
+	return 0;
+}
+
+static const struct iio_trigger_ops tsc2046_adc_trigger_ops = {
+	.set_trigger_state = tsc2046_adc_set_trigger_state,
+	.reenable = tsc2046_adc_reenable_trigger,
+};
+
+static int tsc2046_adc_setup_spi_msg(struct tsc2046_adc_priv *priv)
+{
+	unsigned int ch_idx;
+	size_t size = 0;
+	int ret;
+
+	priv->tx_one = devm_kzalloc(&priv->spi->dev, sizeof(*priv->tx_one),
+				    GFP_KERNEL);
+	if (!priv->tx_one)
+		return -ENOMEM;
+
+	priv->rx_one = devm_kzalloc(&priv->spi->dev, sizeof(*priv->rx_one),
+				    GFP_KERNEL);
+	if (!priv->rx_one)
+		return -ENOMEM;
+
+	/*
+	 * Make dummy read to set initial power state and get real SPI clock
+	 * freq. It seems to be not important which channel is used for this
+	 * case.
+	 */
+	ret = tsc2046_adc_read_one(priv, TI_TSC2046_ADDR_TEMP0,
+				   &priv->effective_speed_hz);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * In case SPI controller do not report effective_speed_hz, use
+	 * configure value and hope it will match
+	 */
+	if (!priv->effective_speed_hz)
+		priv->effective_speed_hz = priv->spi->max_speed_hz;
+
+
+	priv->scan_interval_us = TI_TSC2046_SAMPLE_INTERVAL_US;
+	priv->time_per_bit_ns = DIV_ROUND_UP(NSEC_PER_SEC,
+					     priv->effective_speed_hz);
+
+	/*
+	 * Calculate and allocate maximal size buffer if all channels are
+	 * enabled.
+	 */
+	for (ch_idx = 0; ch_idx < priv->dcfg->num_channels; ch_idx++)
+		size += tsc2046_adc_group_set_layout(priv, ch_idx, ch_idx);
+
+	priv->tx = devm_kzalloc(&priv->spi->dev, size, GFP_KERNEL);
+	if (!priv->tx)
+		return -ENOMEM;
+
+	priv->rx = devm_kzalloc(&priv->spi->dev, size, GFP_KERNEL);
+	if (!priv->rx)
+		return -ENOMEM;
+
+	spi_message_init(&priv->msg);
+	priv->msg.context = priv;
+
+	priv->xfer.tx_buf = priv->tx;
+	priv->xfer.rx_buf = priv->rx;
+	priv->xfer.len = size;
+	spi_message_add_tail(&priv->xfer, &priv->msg);
+
+	return 0;
+}
+
+static void tsc2046_adc_parse_fwnode(struct tsc2046_adc_priv *priv)
+{
+	struct fwnode_handle *child;
+	struct device *dev = &priv->spi->dev;
+
+	device_for_each_child_node(dev, child) {
+		u32 stl, aver, reg;
+		int ret;
+
+		ret = fwnode_property_read_u32(child, "reg", &reg);
+		if (ret) {
+			dev_err(dev, "invalid reg on %pfw, err: %pe\n", child,
+				ERR_PTR(ret));
+			continue;
+		}
+
+		if (reg >= ARRAY_SIZE(priv->ch_cfg)) {
+			dev_err(dev, "%pfw: Unsupported reg value: %i, max supported is: %i.\n",
+				child, reg, ARRAY_SIZE(priv->ch_cfg));
+			continue;
+		}
+
+		ret = fwnode_property_read_u32(child, "settling-time-us", &stl);
+		if (!ret)
+			priv->ch_cfg[reg].settling_time_us = stl;
+
+		ret = fwnode_property_read_u32(child, "average-samples", &aver);
+		if (!ret)
+			priv->ch_cfg[reg].average_samples = aver;
+	}
+}
+
+static int tsc2046_adc_probe(struct spi_device *spi)
+{
+	const struct tsc2046_adc_dcfg *dcfg;
+	struct device *dev = &spi->dev;
+	struct tsc2046_adc_priv *priv;
+	struct iio_dev *indio_dev;
+	struct iio_trigger *trig;
+	const char *name;
+	int ret;
+
+	if (spi->max_speed_hz > TI_TSC2046_MAX_CLK_FREQ) {
+		dev_err(dev, "SPI max_speed_hz is too high: %d Hz. Max supported freq is %d Hz\n",
+			spi->max_speed_hz, TI_TSC2046_MAX_CLK_FREQ);
+		return -EINVAL;
+	}
+
+	dcfg = device_get_match_data(dev);
+	if (!dcfg)
+		return -EINVAL;
+
+	spi->bits_per_word = 8;
+	spi->mode &= ~SPI_MODE_X_MASK;
+	spi->mode |= SPI_MODE_0;
+	ret = spi_setup(spi);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Error in spi setup\n");
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*priv));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	priv = iio_priv(indio_dev);
+	priv->dcfg = dcfg;
+
+	spi_set_drvdata(spi, indio_dev);
+
+	priv->spi = spi;
+
+	name = devm_kasprintf(dev, GFP_KERNEL, "%s-%s",
+			      TI_TSC2046_NAME, dev_name(dev));
+
+	indio_dev->name = name;
+	indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_TRIGGERED;
+	indio_dev->channels = dcfg->channels;
+	indio_dev->num_channels = dcfg->num_channels;
+	indio_dev->info = &tsc2046_adc_info;
+
+	tsc2046_adc_parse_fwnode(priv);
+
+	ret = tsc2046_adc_setup_spi_msg(priv);
+	if (ret)
+		return ret;
+
+	mutex_init(&priv->slock);
+
+	/* TODO: remove IRQ_NOAUTOEN after needed patches are mainline */
+	irq_set_status_flags(spi->irq, IRQ_NOAUTOEN);
+	ret = devm_request_irq(dev, spi->irq, &tsc2046_adc_irq,
+			       0, name, indio_dev);
+	if (ret)
+		return ret;
+
+	trig = devm_iio_trigger_alloc(dev, "%s-dev%d", indio_dev->name,
+				      indio_dev->id);
+	if (!trig)
+		return -ENOMEM;
+
+	priv->trig = trig;
+	trig->dev.parent = indio_dev->dev.parent;
+	iio_trigger_set_drvdata(trig, indio_dev);
+	trig->ops = &tsc2046_adc_trigger_ops;
+
+	spin_lock_init(&priv->trig_lock);
+	hrtimer_init(&priv->trig_timer, CLOCK_MONOTONIC,
+		     HRTIMER_MODE_REL_SOFT);
+	priv->trig_timer.function = tsc2046_adc_trig_more;
+
+	ret = devm_iio_trigger_register(dev, trig);
+	if (ret) {
+		dev_err(dev, "failed to register trigger\n");
+		return ret;
+	}
+
+	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
+					      &tsc2046_adc_trigger_handler, NULL);
+	if (ret) {
+		dev_err(dev, "Failed to setup triggered buffer\n");
+		return ret;
+	}
+
+	/* set default trigger */
+	indio_dev->trig = iio_trigger_get(priv->trig);
+
+	ret = devm_iio_device_register(dev, indio_dev);
+	if (ret) {
+		dev_err(dev, "Failed to register iio device\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id ads7950_of_table[] = {
+	{ .compatible = "ti,tsc2046e-adc", .data = &tsc2046_adc_dcfg_tsc2046e },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ads7950_of_table);
+
+static struct spi_driver tsc2046_adc_driver = {
+	.driver = {
+		.name	= "tsc2046",
+		.of_match_table = ads7950_of_table,
+	},
+	.probe		= tsc2046_adc_probe,
+};
+module_spi_driver(tsc2046_adc_driver);
+
+MODULE_AUTHOR("Oleksij Rempel <kernel@pengutronix.de>");
+MODULE_DESCRIPTION("TI TSC2046 ADC");
+MODULE_LICENSE("GPL v2");
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] iio: adc: add ADC driver for the TI TSC2046 controller
  2021-03-12 10:55 ` [PATCH v2 3/3] iio: adc: add ADC driver for the " Oleksij Rempel
@ 2021-03-12 15:32   ` Andy Shevchenko
  2021-03-12 17:49     ` kernel test robot
  2021-03-13 16:11   ` Jonathan Cameron
  2 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2021-03-12 15:32 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Rob Herring, Jonathan Cameron, devicetree,
	Linux Kernel Mailing List, Pengutronix Kernel Team, David Jander,
	Robin van der Gracht, linux-iio, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Dmitry Torokhov

On Fri, Mar 12, 2021 at 12:57 PM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
>
> Basically the TI TSC2046 touchscreen controller is 8 channel ADC optimized for
> the touchscreen use case. By implementing it as IIO ADC device, we can

as an IIO

> make use of resistive-adc-touch and iio-hwmon drivers.
>
> So far, this driver was tested with custom version of resistive-adc-touch driver,

with a custom

> since it need to be extended to make use of Z1 and Z2 channels. The X/Y

needs

> are working without additional changes.

With above typos fixed, although there is still room for bikeshedding
and small improvements
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Thanks!

> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  MAINTAINERS                  |   8 +
>  drivers/iio/adc/Kconfig      |  12 +
>  drivers/iio/adc/Makefile     |   1 +
>  drivers/iio/adc/ti-tsc2046.c | 713 +++++++++++++++++++++++++++++++++++
>  4 files changed, 734 insertions(+)
>  create mode 100644 drivers/iio/adc/ti-tsc2046.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3fea1a934b32..2d33c6442a55 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -17852,6 +17852,14 @@ S:     Supported
>  F:     Documentation/devicetree/bindings/net/nfc/trf7970a.txt
>  F:     drivers/nfc/trf7970a.c
>
> +TI TSC2046 ADC DRIVER
> +M:     Oleksij Rempel <o.rempel@pengutronix.de>
> +R:     kernel@pengutronix.de
> +L:     linux-iio@vger.kernel.org
> +S:     Maintained
> +F:     Documentation/devicetree/bindings/iio/adc/ti,tsc2046.yaml
> +F:     drivers/iio/adc/ti-tsc2046.c
> +
>  TI TWL4030 SERIES SOC CODEC DRIVER
>  M:     Peter Ujfalusi <peter.ujfalusi@gmail.com>
>  L:     alsa-devel@alsa-project.org (moderated for non-subscribers)
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 15587a1bc80d..6ad6f04dfd20 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -1175,6 +1175,18 @@ config TI_TLC4541
>           This driver can also be built as a module. If so, the module will be
>           called ti-tlc4541.
>
> +config TI_TSC2046
> +       tristate "Texas Instruments TSC2046 ADC driver"
> +       depends on SPI
> +       select IIO_BUFFER
> +       select IIO_TRIGGERED_BUFFER
> +       help
> +         Say yes here to build support for ADC functionality of Texas
> +         Instruments TSC2046 touch screen controller.
> +
> +         This driver can also be built as a module. If so, the module will be
> +         called ti-tsc2046.
> +
>  config TWL4030_MADC
>         tristate "TWL4030 MADC (Monitoring A/D Converter)"
>         depends on TWL4030_CORE
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 5fca90ada0ec..440e18ac6780 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -105,6 +105,7 @@ obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o
>  obj-$(CONFIG_TI_ADS124S08) += ti-ads124s08.o
>  obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
>  obj-$(CONFIG_TI_TLC4541) += ti-tlc4541.o
> +obj-$(CONFIG_TI_TSC2046) += ti-tsc2046.o
>  obj-$(CONFIG_TWL4030_MADC) += twl4030-madc.o
>  obj-$(CONFIG_TWL6030_GPADC) += twl6030-gpadc.o
>  obj-$(CONFIG_VF610_ADC) += vf610_adc.o
> diff --git a/drivers/iio/adc/ti-tsc2046.c b/drivers/iio/adc/ti-tsc2046.c
> new file mode 100644
> index 000000000000..7c3ae9181164
> --- /dev/null
> +++ b/drivers/iio/adc/ti-tsc2046.c
> @@ -0,0 +1,713 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Texas Instruments TSC2046 SPI ADC driver
> + *
> + * Copyright (c) 2021 Oleksij Rempel <kernel@pengutronix.de>, Pengutronix
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +
> +#include <asm/unaligned.h>
> +
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger.h>
> +
> +#define TI_TSC2046_NAME                                "tsc2046"
> +
> +/* This driver doesn't aim at the peak continuous sample rate */
> +#define        TI_TSC2046_MAX_SAMPLE_RATE              125000
> +#define        TI_TSC2046_SAMPLE_BITS                  (8 /*cmd*/ + 16 /*sample*/)
> +#define        TI_TSC2046_MAX_CLK_FREQ \
> +       (TI_TSC2046_MAX_SAMPLE_RATE * TI_TSC2046_SAMPLE_BITS)
> +
> +#define TI_TSC2046_SAMPLE_INTERVAL_US          10000
> +
> +#define TI_TSC2046_START                       BIT(7)
> +#define TI_TSC2046_ADDR                                GENMASK(6, 4)
> +#define TI_TSC2046_ADDR_TEMP1                  7
> +#define TI_TSC2046_ADDR_AUX                    6
> +#define TI_TSC2046_ADDR_X                      5
> +#define TI_TSC2046_ADDR_Z2                     4
> +#define TI_TSC2046_ADDR_Z1                     3
> +#define TI_TSC2046_ADDR_VBAT                   2
> +#define TI_TSC2046_ADDR_Y                      1
> +#define TI_TSC2046_ADDR_TEMP0                  0
> +
> +/*
> + * The mode bit sets the resolution of the ADC. With this bit low, the next
> + * conversion has 12-bit resolution, whereas with this bit high, the next
> + * conversion has 8-bit resolution. This driver is optimized for 12-bit mode.
> + * So, for this driver, this bit should stay zero.
> + */
> +#define TI_TSC2046_8BIT_MODE                   BIT(3)
> +
> +/*
> + * SER/DFR - The SER/DFR bit controls the reference mode, either single-ended
> + * (high) or differential (low).
> + */
> +#define TI_TSC2046_SER                         BIT(2)
> +
> +/*
> + * If VREF_ON and ADC_ON are both zero, then the chip operates in
> + * auto-wake/suspend mode. In most case this bits should stay zero.
> + */
> +#define TI_TSC2046_PD1_VREF_ON                 BIT(1)
> +#define TI_TSC2046_PD0_ADC_ON                  BIT(0)
> +
> +#define TI_TSC2046_MAX_CHAN                    8
> +
> +/* Represents a HW sample */
> +struct tsc2046_adc_atom {
> +       /*
> +        * Command transmitted to the controller. This filed is empty on the RX
> +        * buffer.
> +        */
> +       u8 cmd;
> +       /*
> +        * Data received from the controller. This filed is empty for the TX
> +        * buffer
> +        */
> +       __be16 data;
> +} __packed;
> +
> +struct tsc2046_adc_scan_buf {
> +       /* Scan data for each channel */
> +       u16 data[TI_TSC2046_MAX_CHAN];
> +       /* Timestamp */
> +       s64 ts __aligned(8);
> +};
> +
> +/* Layout of atomic buffers within big buffer */
> +struct tsc2046_adc_group_layout {
> +       /* Group offset within the SPI RX buffer */
> +       unsigned int offset;
> +       /*
> +        * Amount of tsc2046_adc_atom structs within the same command gathered
> +        * within same group.
> +        */
> +       unsigned int count;
> +       /*
> +        * Settling samples (tsc2046_adc_atom structs) which should be skipped
> +        * before good samples will start.
> +        */
> +       unsigned int skip;
> +};
> +
> +struct tsc2046_adc_dcfg {
> +       const struct iio_chan_spec *channels;
> +       unsigned int num_channels;
> +};
> +
> +struct tsc2046_adc_ch_cfg {
> +       unsigned int settling_time_us;
> +       unsigned int average_samples;
> +};
> +
> +struct tsc2046_adc_priv {
> +       struct spi_device *spi;
> +       const struct tsc2046_adc_dcfg *dcfg;
> +
> +       struct iio_trigger *trig;
> +       struct hrtimer trig_timer;
> +       spinlock_t trig_lock;
> +       atomic_t trig_more_count;
> +
> +       struct spi_transfer xfer;
> +       struct spi_message msg;
> +
> +       struct tsc2046_adc_scan_buf scan_buf;
> +       /*
> +        * Lock to protect the layout and the spi transfer buffer.
> +        * tsc2046_adc_group_layout can be changed within update_scan_mode(),
> +        * in this case the l[] and tx/rx buffer will be out of sync to each
> +        * other.
> +        */
> +       struct mutex slock;
> +       struct tsc2046_adc_group_layout l[TI_TSC2046_MAX_CHAN];
> +       struct tsc2046_adc_atom *rx;
> +       struct tsc2046_adc_atom *tx;
> +
> +       struct tsc2046_adc_atom *rx_one;
> +       struct tsc2046_adc_atom *tx_one;
> +
> +       unsigned int count;
> +       unsigned int groups;
> +       u32 effective_speed_hz;
> +       u32 scan_interval_us;
> +       u32 time_per_scan_us;
> +       u32 time_per_bit_ns;
> +
> +       struct tsc2046_adc_ch_cfg ch_cfg[TI_TSC2046_MAX_CHAN];
> +};
> +
> +#define TI_TSC2046_V_CHAN(index, bits, name)                   \
> +{                                                              \
> +       .type = IIO_VOLTAGE,                                    \
> +       .indexed = 1,                                           \
> +       .channel = index,                                       \
> +       .address = index,                                       \
> +       .datasheet_name = "#name",                              \
> +       .scan_index = index,                                    \
> +       .scan_type = {                                          \
> +               .sign = 'u',                                    \
> +               .realbits = bits,                               \
> +               .storagebits = 16,                              \
> +               .shift = 0,                                     \
> +               .endianness = IIO_CPU,                          \
> +       },                                                      \
> +}
> +
> +#define DECLARE_TI_TSC2046_8_CHANNELS(name, bits) \
> +const struct iio_chan_spec name ## _channels[] = { \
> +       TI_TSC2046_V_CHAN(0, bits, TEMP0), \
> +       TI_TSC2046_V_CHAN(1, bits, Y), \
> +       TI_TSC2046_V_CHAN(2, bits, VBAT), \
> +       TI_TSC2046_V_CHAN(3, bits, Z1), \
> +       TI_TSC2046_V_CHAN(4, bits, Z2), \
> +       TI_TSC2046_V_CHAN(5, bits, X), \
> +       TI_TSC2046_V_CHAN(6, bits, AUX), \
> +       TI_TSC2046_V_CHAN(7, bits, TEMP1), \
> +       IIO_CHAN_SOFT_TIMESTAMP(8), \
> +}
> +
> +static DECLARE_TI_TSC2046_8_CHANNELS(tsc2046_adc, 12);
> +
> +static const struct tsc2046_adc_dcfg tsc2046_adc_dcfg_tsc2046e = {
> +       .channels = tsc2046_adc_channels,
> +       .num_channels = ARRAY_SIZE(tsc2046_adc_channels),
> +};
> +
> +/*
> + * Convert time to a number of samples which can be transferred within this
> + * time.
> + */
> +static unsigned int tsc2046_adc_time_to_count(struct tsc2046_adc_priv *priv,
> +                                             unsigned long time)
> +{
> +       unsigned int bit_count, sample_count;
> +
> +       bit_count = DIV_ROUND_UP(time * NSEC_PER_USEC, priv->time_per_bit_ns);
> +       sample_count = DIV_ROUND_UP(bit_count, TI_TSC2046_SAMPLE_BITS);
> +
> +       dev_dbg(&priv->spi->dev, "Effective speed %u, time per bit: %u, count bits: %u, count samples: %u\n",
> +               priv->effective_speed_hz, priv->time_per_bit_ns,
> +               bit_count, sample_count);
> +
> +       return sample_count;
> +}
> +
> +static u8 tsc2046_adc_get_cmd(struct tsc2046_adc_priv *priv, int ch_idx,
> +                             bool keep_power)
> +{
> +       u32 pd;
> +
> +       /*
> +        * if PD bits are 0, controller will automatically disable ADC, VREF and
> +        * enable IRQ.
> +        */
> +       if (keep_power)
> +               pd = TI_TSC2046_PD0_ADC_ON;
> +       else
> +               pd = 0;
> +
> +       return TI_TSC2046_START | FIELD_PREP(TI_TSC2046_ADDR, ch_idx) | pd;
> +}
> +
> +static u16 tsc2046_adc_get_value(struct tsc2046_adc_atom *buf)
> +{
> +       /* Last 3 bits on the wire are empty */
> +       return get_unaligned_be16(&buf->data) >> 3;
> +}
> +
> +static int tsc2046_adc_read_one(struct tsc2046_adc_priv *priv, int ch_idx,
> +                               u32 *effective_speed_hz)
> +{
> +       struct spi_transfer xfer;
> +       struct spi_message msg;
> +       int ret;
> +
> +       memset(&xfer, 0, sizeof(xfer));
> +       priv->tx_one->cmd = tsc2046_adc_get_cmd(priv, ch_idx, false);
> +       priv->tx_one->data = 0;
> +       xfer.tx_buf = priv->tx_one;
> +       xfer.rx_buf = priv->rx_one;
> +       xfer.len = sizeof(*priv->tx_one);
> +       spi_message_init(&msg);
> +       spi_message_add_tail(&xfer, &msg);
> +
> +       ret = spi_sync(priv->spi, &msg);
> +       if (ret) {
> +               dev_err_ratelimited(&priv->spi->dev, "SPI transfer filed %pe\n",
> +                                   ERR_PTR(ret));
> +               return ret;
> +       }
> +
> +       if (effective_speed_hz)
> +               *effective_speed_hz = xfer.effective_speed_hz;
> +
> +       return tsc2046_adc_get_value(priv->rx_one);
> +}
> +
> +static size_t tsc2046_adc_group_set_layout(struct tsc2046_adc_priv *priv,
> +                                          unsigned int group,
> +                                          unsigned int ch_idx)
> +{
> +       struct tsc2046_adc_ch_cfg *ch = &priv->ch_cfg[ch_idx];
> +       struct tsc2046_adc_group_layout *prev, *cur;
> +       unsigned int max_count, count_skip;
> +       unsigned int offset = 0;
> +
> +
> +       if (group) {
> +               prev = &priv->l[group - 1];
> +               offset = prev->offset + prev->count;
> +       }
> +
> +       cur = &priv->l[group];
> +
> +       count_skip = tsc2046_adc_time_to_count(priv, ch->settling_time_us);
> +       max_count = count_skip + ch->average_samples;
> +
> +       cur->offset = offset;
> +       cur->count = max_count;
> +       cur->skip = count_skip;
> +
> +       return sizeof(*priv->tx) * max_count;
> +}
> +
> +static void tsc2046_adc_group_set_cmd(struct tsc2046_adc_priv *priv,
> +                                     unsigned int group, int ch_idx)
> +{
> +       struct tsc2046_adc_group_layout *l = &priv->l[group];
> +       unsigned int i;
> +       u8 cmd;
> +
> +       /*
> +        * Do not enable automatic power down on working samples. Otherwise the
> +        * plates will never be completely charged.
> +        */
> +       cmd = tsc2046_adc_get_cmd(priv, ch_idx, true);
> +
> +       for (i = 0; i < l->count - 1; i++)
> +               priv->tx[l->offset + i].cmd = cmd;
> +
> +       /* automatically power down on last sample */
> +       priv->tx[l->offset + i].cmd = tsc2046_adc_get_cmd(priv, ch_idx, false);
> +}
> +
> +static u16 tsc2046_adc_get_val(struct tsc2046_adc_priv *priv, int group)
> +{
> +       struct tsc2046_adc_group_layout *l;
> +       unsigned int val, val_normalized = 0;
> +       int valid_count, i;
> +
> +       l = &priv->l[group];
> +       valid_count = l->count - l->skip;
> +
> +       for (i = 0; i < valid_count; i++) {
> +               val = tsc2046_adc_get_value(&priv->rx[l->offset + l->skip + i]);
> +               val_normalized += val;
> +       }
> +
> +       return DIV_ROUND_UP(val_normalized, valid_count);
> +}
> +
> +static int tsc2046_adc_scan(struct iio_dev *indio_dev)
> +{
> +       struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
> +       struct device *dev = &priv->spi->dev;
> +       int group;
> +       int ret;
> +
> +       ret = spi_sync(priv->spi, &priv->msg);
> +       if (ret < 0) {
> +               dev_err_ratelimited(dev, "SPI transfer filed: %pe\n",
> +                                   ERR_PTR(ret));
> +               return ret;
> +       }
> +
> +       for (group = 0; group < priv->groups; group++) {
> +               u16 val = tsc2046_adc_get_val(priv, group);
> +
> +               priv->scan_buf.data[group] = val;
> +       }
> +
> +       ret = iio_push_to_buffers_with_timestamp(indio_dev, &priv->scan_buf,
> +                                                iio_get_time_ns(indio_dev));
> +       /*
> +        * If there's no consumer (or consumer is kfifo), may get a EBUSY here
> +        * - ignore it.
> +        */
> +       if (ret < 0 && ret != -EBUSY) {
> +               dev_err_ratelimited(dev, "Filed to push scan buffer %pe\n",
> +                                   ERR_PTR(ret));
> +
> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +static irqreturn_t tsc2046_adc_trigger_handler(int irq, void *p)
> +{
> +       struct iio_poll_func *pf = p;
> +       struct iio_dev *indio_dev = pf->indio_dev;
> +       struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
> +
> +       mutex_lock(&priv->slock);
> +       tsc2046_adc_scan(indio_dev);
> +       mutex_unlock(&priv->slock);
> +
> +       iio_trigger_notify_done(indio_dev->trig);
> +
> +       return IRQ_HANDLED;
> +}
> +
> +static int tsc2046_adc_update_scan_mode(struct iio_dev *indio_dev,
> +                                       const unsigned long *active_scan_mask)
> +{
> +       struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
> +       unsigned int ch_idx, group = 0;
> +       size_t size = 0;
> +
> +       mutex_lock(&priv->slock);
> +
> +       for_each_set_bit(ch_idx, active_scan_mask, indio_dev->num_channels) {
> +               size += tsc2046_adc_group_set_layout(priv, group, ch_idx);
> +               tsc2046_adc_group_set_cmd(priv, group, ch_idx);
> +               group++;
> +       }
> +
> +       priv->groups = group;
> +       priv->xfer.len = size;
> +       priv->time_per_scan_us = size * 8 * priv->time_per_bit_ns / NSEC_PER_USEC;
> +
> +       if ((priv->scan_interval_us - priv->time_per_scan_us) < 0)
> +               dev_warn(&priv->spi->dev, "The scan interval (%d) is less then calculated scan time (%d)\n",
> +                        priv->scan_interval_us, priv->time_per_scan_us);
> +
> +       mutex_unlock(&priv->slock);
> +
> +       return 0;
> +}
> +
> +static const struct iio_info tsc2046_adc_info = {
> +       .update_scan_mode = tsc2046_adc_update_scan_mode,
> +};
> +
> +static enum hrtimer_restart tsc2046_adc_trig_more(struct hrtimer *hrtimer)
> +{
> +       struct tsc2046_adc_priv *priv = container_of(hrtimer,
> +                                                    struct tsc2046_adc_priv,
> +                                                    trig_timer);
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(&priv->trig_lock, flags);
> +
> +       disable_irq_nosync(priv->spi->irq);
> +
> +       atomic_inc(&priv->trig_more_count);
> +       iio_trigger_poll(priv->trig);
> +
> +       spin_unlock_irqrestore(&priv->trig_lock, flags);
> +
> +       return HRTIMER_NORESTART;
> +}
> +
> +static irqreturn_t tsc2046_adc_irq(int irq, void *dev_id)
> +{
> +       struct iio_dev *indio_dev = dev_id;
> +       struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
> +
> +
> +       spin_lock(&priv->trig_lock);
> +
> +       hrtimer_try_to_cancel(&priv->trig_timer);
> +
> +       atomic_set(&priv->trig_more_count, 0);
> +       disable_irq_nosync(priv->spi->irq);
> +       iio_trigger_poll(priv->trig);
> +
> +       spin_unlock(&priv->trig_lock);
> +
> +       return IRQ_HANDLED;
> +}
> +
> +static void tsc2046_adc_reenable_trigger(struct iio_trigger *trig)
> +{
> +       struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
> +       struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
> +       unsigned long flags;
> +       int delta;
> +
> +       /*
> +        * We can sample it as fast as we can, but usually we do not need so
> +        * many samples. Reduce the sample rate for default (touchscreen) use
> +        * case.
> +        * Currently we do not need a highly precise sample rate. It is enough
> +        * to have calculated numbers.
> +        */
> +       delta = priv->scan_interval_us - priv->time_per_scan_us;
> +       if (delta > 0)
> +               fsleep(delta);
> +
> +       spin_lock_irqsave(&priv->trig_lock, flags);
> +
> +       /*
> +        * We need to trigger at least one extra sample to detect state
> +        * difference on ADC side.
> +        */
> +       if (atomic_read(&priv->trig_more_count) == 0) {
> +               int timeout_ms = DIV_ROUND_UP(priv->scan_interval_us,
> +                                             USEC_PER_MSEC);
> +
> +               hrtimer_start(&priv->trig_timer, ms_to_ktime(timeout_ms),
> +                             HRTIMER_MODE_REL_SOFT);
> +       }
> +
> +       enable_irq(priv->spi->irq);
> +
> +       spin_unlock_irqrestore(&priv->trig_lock, flags);
> +}
> +
> +static int tsc2046_adc_set_trigger_state(struct iio_trigger *trig, bool enable)
> +{
> +       struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
> +       struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
> +
> +       if (enable) {
> +               enable_irq(priv->spi->irq);
> +       } else {
> +               disable_irq(priv->spi->irq);
> +               hrtimer_try_to_cancel(&priv->trig_timer);
> +       }
> +
> +       return 0;
> +}
> +
> +static const struct iio_trigger_ops tsc2046_adc_trigger_ops = {
> +       .set_trigger_state = tsc2046_adc_set_trigger_state,
> +       .reenable = tsc2046_adc_reenable_trigger,
> +};
> +
> +static int tsc2046_adc_setup_spi_msg(struct tsc2046_adc_priv *priv)
> +{
> +       unsigned int ch_idx;
> +       size_t size = 0;
> +       int ret;
> +
> +       priv->tx_one = devm_kzalloc(&priv->spi->dev, sizeof(*priv->tx_one),
> +                                   GFP_KERNEL);
> +       if (!priv->tx_one)
> +               return -ENOMEM;
> +
> +       priv->rx_one = devm_kzalloc(&priv->spi->dev, sizeof(*priv->rx_one),
> +                                   GFP_KERNEL);
> +       if (!priv->rx_one)
> +               return -ENOMEM;
> +
> +       /*
> +        * Make dummy read to set initial power state and get real SPI clock
> +        * freq. It seems to be not important which channel is used for this
> +        * case.
> +        */
> +       ret = tsc2046_adc_read_one(priv, TI_TSC2046_ADDR_TEMP0,
> +                                  &priv->effective_speed_hz);
> +       if (ret < 0)
> +               return ret;
> +
> +       /*
> +        * In case SPI controller do not report effective_speed_hz, use
> +        * configure value and hope it will match
> +        */
> +       if (!priv->effective_speed_hz)
> +               priv->effective_speed_hz = priv->spi->max_speed_hz;
> +
> +
> +       priv->scan_interval_us = TI_TSC2046_SAMPLE_INTERVAL_US;
> +       priv->time_per_bit_ns = DIV_ROUND_UP(NSEC_PER_SEC,
> +                                            priv->effective_speed_hz);
> +
> +       /*
> +        * Calculate and allocate maximal size buffer if all channels are
> +        * enabled.
> +        */
> +       for (ch_idx = 0; ch_idx < priv->dcfg->num_channels; ch_idx++)
> +               size += tsc2046_adc_group_set_layout(priv, ch_idx, ch_idx);
> +
> +       priv->tx = devm_kzalloc(&priv->spi->dev, size, GFP_KERNEL);
> +       if (!priv->tx)
> +               return -ENOMEM;
> +
> +       priv->rx = devm_kzalloc(&priv->spi->dev, size, GFP_KERNEL);
> +       if (!priv->rx)
> +               return -ENOMEM;
> +
> +       spi_message_init(&priv->msg);
> +       priv->msg.context = priv;
> +
> +       priv->xfer.tx_buf = priv->tx;
> +       priv->xfer.rx_buf = priv->rx;
> +       priv->xfer.len = size;
> +       spi_message_add_tail(&priv->xfer, &priv->msg);
> +
> +       return 0;
> +}
> +
> +static void tsc2046_adc_parse_fwnode(struct tsc2046_adc_priv *priv)
> +{
> +       struct fwnode_handle *child;
> +       struct device *dev = &priv->spi->dev;
> +
> +       device_for_each_child_node(dev, child) {
> +               u32 stl, aver, reg;
> +               int ret;
> +
> +               ret = fwnode_property_read_u32(child, "reg", &reg);
> +               if (ret) {
> +                       dev_err(dev, "invalid reg on %pfw, err: %pe\n", child,
> +                               ERR_PTR(ret));
> +                       continue;
> +               }
> +
> +               if (reg >= ARRAY_SIZE(priv->ch_cfg)) {
> +                       dev_err(dev, "%pfw: Unsupported reg value: %i, max supported is: %i.\n",
> +                               child, reg, ARRAY_SIZE(priv->ch_cfg));
> +                       continue;
> +               }
> +
> +               ret = fwnode_property_read_u32(child, "settling-time-us", &stl);
> +               if (!ret)
> +                       priv->ch_cfg[reg].settling_time_us = stl;
> +
> +               ret = fwnode_property_read_u32(child, "average-samples", &aver);
> +               if (!ret)
> +                       priv->ch_cfg[reg].average_samples = aver;
> +       }
> +}
> +
> +static int tsc2046_adc_probe(struct spi_device *spi)
> +{
> +       const struct tsc2046_adc_dcfg *dcfg;
> +       struct device *dev = &spi->dev;
> +       struct tsc2046_adc_priv *priv;
> +       struct iio_dev *indio_dev;
> +       struct iio_trigger *trig;
> +       const char *name;
> +       int ret;
> +
> +       if (spi->max_speed_hz > TI_TSC2046_MAX_CLK_FREQ) {
> +               dev_err(dev, "SPI max_speed_hz is too high: %d Hz. Max supported freq is %d Hz\n",
> +                       spi->max_speed_hz, TI_TSC2046_MAX_CLK_FREQ);
> +               return -EINVAL;
> +       }
> +
> +       dcfg = device_get_match_data(dev);
> +       if (!dcfg)
> +               return -EINVAL;
> +
> +       spi->bits_per_word = 8;
> +       spi->mode &= ~SPI_MODE_X_MASK;
> +       spi->mode |= SPI_MODE_0;
> +       ret = spi_setup(spi);
> +       if (ret < 0)
> +               return dev_err_probe(dev, ret, "Error in spi setup\n");
> +
> +       indio_dev = devm_iio_device_alloc(dev, sizeof(*priv));
> +       if (!indio_dev)
> +               return -ENOMEM;
> +
> +       priv = iio_priv(indio_dev);
> +       priv->dcfg = dcfg;
> +
> +       spi_set_drvdata(spi, indio_dev);
> +
> +       priv->spi = spi;
> +
> +       name = devm_kasprintf(dev, GFP_KERNEL, "%s-%s",
> +                             TI_TSC2046_NAME, dev_name(dev));
> +
> +       indio_dev->name = name;
> +       indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_TRIGGERED;
> +       indio_dev->channels = dcfg->channels;
> +       indio_dev->num_channels = dcfg->num_channels;
> +       indio_dev->info = &tsc2046_adc_info;
> +
> +       tsc2046_adc_parse_fwnode(priv);
> +
> +       ret = tsc2046_adc_setup_spi_msg(priv);
> +       if (ret)
> +               return ret;
> +
> +       mutex_init(&priv->slock);
> +
> +       /* TODO: remove IRQ_NOAUTOEN after needed patches are mainline */
> +       irq_set_status_flags(spi->irq, IRQ_NOAUTOEN);
> +       ret = devm_request_irq(dev, spi->irq, &tsc2046_adc_irq,
> +                              0, name, indio_dev);
> +       if (ret)
> +               return ret;
> +
> +       trig = devm_iio_trigger_alloc(dev, "%s-dev%d", indio_dev->name,
> +                                     indio_dev->id);
> +       if (!trig)
> +               return -ENOMEM;
> +
> +       priv->trig = trig;
> +       trig->dev.parent = indio_dev->dev.parent;
> +       iio_trigger_set_drvdata(trig, indio_dev);
> +       trig->ops = &tsc2046_adc_trigger_ops;
> +
> +       spin_lock_init(&priv->trig_lock);
> +       hrtimer_init(&priv->trig_timer, CLOCK_MONOTONIC,
> +                    HRTIMER_MODE_REL_SOFT);
> +       priv->trig_timer.function = tsc2046_adc_trig_more;
> +
> +       ret = devm_iio_trigger_register(dev, trig);
> +       if (ret) {
> +               dev_err(dev, "failed to register trigger\n");
> +               return ret;
> +       }
> +
> +       ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
> +                                             &tsc2046_adc_trigger_handler, NULL);
> +       if (ret) {
> +               dev_err(dev, "Failed to setup triggered buffer\n");
> +               return ret;
> +       }
> +
> +       /* set default trigger */
> +       indio_dev->trig = iio_trigger_get(priv->trig);
> +
> +       ret = devm_iio_device_register(dev, indio_dev);
> +       if (ret) {
> +               dev_err(dev, "Failed to register iio device\n");
> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id ads7950_of_table[] = {
> +       { .compatible = "ti,tsc2046e-adc", .data = &tsc2046_adc_dcfg_tsc2046e },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(of, ads7950_of_table);
> +
> +static struct spi_driver tsc2046_adc_driver = {
> +       .driver = {
> +               .name   = "tsc2046",
> +               .of_match_table = ads7950_of_table,
> +       },
> +       .probe          = tsc2046_adc_probe,
> +};
> +module_spi_driver(tsc2046_adc_driver);
> +
> +MODULE_AUTHOR("Oleksij Rempel <kernel@pengutronix.de>");
> +MODULE_DESCRIPTION("TI TSC2046 ADC");
> +MODULE_LICENSE("GPL v2");
> --
> 2.29.2
>


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] iio: adc: add ADC driver for the TI TSC2046 controller
  2021-03-12 10:55 ` [PATCH v2 3/3] iio: adc: add ADC driver for the " Oleksij Rempel
@ 2021-03-12 17:49     ` kernel test robot
  2021-03-12 17:49     ` kernel test robot
  2021-03-13 16:11   ` Jonathan Cameron
  2 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2021-03-12 17:49 UTC (permalink / raw)
  To: Oleksij Rempel, Rob Herring, Jonathan Cameron
  Cc: kbuild-all, Oleksij Rempel, devicetree, linux-kernel,
	Pengutronix Kernel Team, David Jander, Robin van der Gracht,
	linux-iio, Lars-Peter Clausen

[-- Attachment #1: Type: text/plain, Size: 4383 bytes --]

Hi Oleksij,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linux/master]
[also build test WARNING on linus/master v5.12-rc2]
[cannot apply to iio/togreg]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Oleksij-Rempel/mainline-ti-tsc2046-adc-driver/20210312-185657
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git a74e6a014c9d4d4161061f770c9b4f98372ac778
config: riscv-allyesconfig (attached as .config)
compiler: riscv64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/7c34f2cd7f0176071b53152c6065f6d5a078463b
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Oleksij-Rempel/mainline-ti-tsc2046-adc-driver/20210312-185657
        git checkout 7c34f2cd7f0176071b53152c6065f6d5a078463b
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=riscv 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from include/linux/device.h:15,
                    from include/linux/node.h:18,
                    from include/linux/cpu.h:17,
                    from include/linux/cacheinfo.h:6,
                    from arch/riscv/include/asm/cacheinfo.h:9,
                    from arch/riscv/include/asm/elf.h:14,
                    from include/linux/elf.h:6,
                    from include/linux/module.h:18,
                    from drivers/iio/adc/ti-tsc2046.c:10:
   drivers/iio/adc/ti-tsc2046.c: In function 'tsc2046_adc_parse_fwnode':
>> drivers/iio/adc/ti-tsc2046.c:579:17: warning: format '%i' expects argument of type 'int', but argument 5 has type 'long unsigned int' [-Wformat=]
     579 |    dev_err(dev, "%pfw: Unsupported reg value: %i, max supported is: %i.\n",
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:19:22: note: in definition of macro 'dev_fmt'
      19 | #define dev_fmt(fmt) fmt
         |                      ^~~
   drivers/iio/adc/ti-tsc2046.c:579:4: note: in expansion of macro 'dev_err'
     579 |    dev_err(dev, "%pfw: Unsupported reg value: %i, max supported is: %i.\n",
         |    ^~~~~~~
   drivers/iio/adc/ti-tsc2046.c:579:70: note: format string is defined here
     579 |    dev_err(dev, "%pfw: Unsupported reg value: %i, max supported is: %i.\n",
         |                                                                     ~^
         |                                                                      |
         |                                                                      int
         |                                                                     %li


vim +579 drivers/iio/adc/ti-tsc2046.c

   561	
   562	static void tsc2046_adc_parse_fwnode(struct tsc2046_adc_priv *priv)
   563	{
   564		struct fwnode_handle *child;
   565		struct device *dev = &priv->spi->dev;
   566	
   567		device_for_each_child_node(dev, child) {
   568			u32 stl, aver, reg;
   569			int ret;
   570	
   571			ret = fwnode_property_read_u32(child, "reg", &reg);
   572			if (ret) {
   573				dev_err(dev, "invalid reg on %pfw, err: %pe\n", child,
   574					ERR_PTR(ret));
   575				continue;
   576			}
   577	
   578			if (reg >= ARRAY_SIZE(priv->ch_cfg)) {
 > 579				dev_err(dev, "%pfw: Unsupported reg value: %i, max supported is: %i.\n",
   580					child, reg, ARRAY_SIZE(priv->ch_cfg));
   581				continue;
   582			}
   583	
   584			ret = fwnode_property_read_u32(child, "settling-time-us", &stl);
   585			if (!ret)
   586				priv->ch_cfg[reg].settling_time_us = stl;
   587	
   588			ret = fwnode_property_read_u32(child, "average-samples", &aver);
   589			if (!ret)
   590				priv->ch_cfg[reg].average_samples = aver;
   591		}
   592	}
   593	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 68974 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] iio: adc: add ADC driver for the TI TSC2046 controller
@ 2021-03-12 17:49     ` kernel test robot
  0 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2021-03-12 17:49 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 4481 bytes --]

Hi Oleksij,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linux/master]
[also build test WARNING on linus/master v5.12-rc2]
[cannot apply to iio/togreg]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Oleksij-Rempel/mainline-ti-tsc2046-adc-driver/20210312-185657
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git a74e6a014c9d4d4161061f770c9b4f98372ac778
config: riscv-allyesconfig (attached as .config)
compiler: riscv64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/7c34f2cd7f0176071b53152c6065f6d5a078463b
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Oleksij-Rempel/mainline-ti-tsc2046-adc-driver/20210312-185657
        git checkout 7c34f2cd7f0176071b53152c6065f6d5a078463b
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=riscv 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from include/linux/device.h:15,
                    from include/linux/node.h:18,
                    from include/linux/cpu.h:17,
                    from include/linux/cacheinfo.h:6,
                    from arch/riscv/include/asm/cacheinfo.h:9,
                    from arch/riscv/include/asm/elf.h:14,
                    from include/linux/elf.h:6,
                    from include/linux/module.h:18,
                    from drivers/iio/adc/ti-tsc2046.c:10:
   drivers/iio/adc/ti-tsc2046.c: In function 'tsc2046_adc_parse_fwnode':
>> drivers/iio/adc/ti-tsc2046.c:579:17: warning: format '%i' expects argument of type 'int', but argument 5 has type 'long unsigned int' [-Wformat=]
     579 |    dev_err(dev, "%pfw: Unsupported reg value: %i, max supported is: %i.\n",
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:19:22: note: in definition of macro 'dev_fmt'
      19 | #define dev_fmt(fmt) fmt
         |                      ^~~
   drivers/iio/adc/ti-tsc2046.c:579:4: note: in expansion of macro 'dev_err'
     579 |    dev_err(dev, "%pfw: Unsupported reg value: %i, max supported is: %i.\n",
         |    ^~~~~~~
   drivers/iio/adc/ti-tsc2046.c:579:70: note: format string is defined here
     579 |    dev_err(dev, "%pfw: Unsupported reg value: %i, max supported is: %i.\n",
         |                                                                     ~^
         |                                                                      |
         |                                                                      int
         |                                                                     %li


vim +579 drivers/iio/adc/ti-tsc2046.c

   561	
   562	static void tsc2046_adc_parse_fwnode(struct tsc2046_adc_priv *priv)
   563	{
   564		struct fwnode_handle *child;
   565		struct device *dev = &priv->spi->dev;
   566	
   567		device_for_each_child_node(dev, child) {
   568			u32 stl, aver, reg;
   569			int ret;
   570	
   571			ret = fwnode_property_read_u32(child, "reg", &reg);
   572			if (ret) {
   573				dev_err(dev, "invalid reg on %pfw, err: %pe\n", child,
   574					ERR_PTR(ret));
   575				continue;
   576			}
   577	
   578			if (reg >= ARRAY_SIZE(priv->ch_cfg)) {
 > 579				dev_err(dev, "%pfw: Unsupported reg value: %i, max supported is: %i.\n",
   580					child, reg, ARRAY_SIZE(priv->ch_cfg));
   581				continue;
   582			}
   583	
   584			ret = fwnode_property_read_u32(child, "settling-time-us", &stl);
   585			if (!ret)
   586				priv->ch_cfg[reg].settling_time_us = stl;
   587	
   588			ret = fwnode_property_read_u32(child, "average-samples", &aver);
   589			if (!ret)
   590				priv->ch_cfg[reg].average_samples = aver;
   591		}
   592	}
   593	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 68974 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/3] dt-bindings:iio:adc: add generic settling-time-us and average-samples channel properties
  2021-03-12 10:55 ` [PATCH v2 1/3] dt-bindings:iio:adc: add generic settling-time-us and average-samples channel properties Oleksij Rempel
@ 2021-03-13 15:15   ` Jonathan Cameron
  0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2021-03-13 15:15 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Rob Herring, devicetree, linux-kernel, Pengutronix Kernel Team,
	David Jander, Robin van der Gracht, linux-iio,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Dmitry Torokhov

On Fri, 12 Mar 2021 11:55:13 +0100
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> Settling time and over sampling is a typical challenge for different IIO ADC
> devices. So, introduce channel specific settling-time-us and average-samples
> properties to cover this use case.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/iio/adc/adc.yaml | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adc.yaml b/Documentation/devicetree/bindings/iio/adc/adc.yaml
> index 912a7635edc4..c748f6573027 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adc.yaml
> @@ -39,4 +39,13 @@ properties:
>        The first value specifies the positive input pin, the second
>        specifies the negative input pin.
>  
> +  settling-time-us:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Time between enabling the channel and firs stable readings.

first

> +
> +  average-samples:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Number of data samples which are averaged for each read.

So in IIO at least, we tend to refer to this as oversampling.  Perhaps we
should use that term here as well?  It would also be good to give some hint
as to why this might be a DT property rather than a userspace control.

Thanks,

Jonathan

> +
>  additionalProperties: true


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] iio: adc: add ADC driver for the TI TSC2046 controller
  2021-03-12 10:55 ` [PATCH v2 3/3] iio: adc: add ADC driver for the " Oleksij Rempel
  2021-03-12 15:32   ` Andy Shevchenko
  2021-03-12 17:49     ` kernel test robot
@ 2021-03-13 16:11   ` Jonathan Cameron
  2021-03-16  5:57     ` Oleksij Rempel
  2 siblings, 1 reply; 11+ messages in thread
From: Jonathan Cameron @ 2021-03-13 16:11 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Rob Herring, devicetree, linux-kernel, Pengutronix Kernel Team,
	David Jander, Robin van der Gracht, linux-iio,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Dmitry Torokhov

On Fri, 12 Mar 2021 11:55:15 +0100
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> Basically the TI TSC2046 touchscreen controller is 8 channel ADC optimized for
> the touchscreen use case. By implementing it as IIO ADC device, we can
> make use of resistive-adc-touch and iio-hwmon drivers.
> 
> So far, this driver was tested with custom version of resistive-adc-touch driver,
> since it need to be extended to make use of Z1 and Z2 channels. The X/Y
> are working without additional changes.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

So the flow in here is still rather non obvious and touchscreen specific.
Please add some documentation on what the trigger actually is. It seems
to be some combination of an hrtimer and a interrupt driven trigger.
I 'think' the idea is to ensure you get one 'no touch' measurement once
touch is removed?

I guess we don't need the performance but I was a bit surprised that I didn't
see this doing overlapping of the previous read out with the config for the
next sample.

Anyhow, overall this looks pretty good to me.  Add that a bit of documentation
for the trigger to the comments at the top of the file and tidy up the last
few things inline.

It's a bit unusual as ADC drivers go, but not so strange that it worries
me that much.

Thanks,

Jonathan

> ---
>  MAINTAINERS                  |   8 +
>  drivers/iio/adc/Kconfig      |  12 +
>  drivers/iio/adc/Makefile     |   1 +
>  drivers/iio/adc/ti-tsc2046.c | 713 +++++++++++++++++++++++++++++++++++
>  4 files changed, 734 insertions(+)
>  create mode 100644 drivers/iio/adc/ti-tsc2046.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3fea1a934b32..2d33c6442a55 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -17852,6 +17852,14 @@ S:	Supported
>  F:	Documentation/devicetree/bindings/net/nfc/trf7970a.txt
>  F:	drivers/nfc/trf7970a.c
>  
> +TI TSC2046 ADC DRIVER
> +M:	Oleksij Rempel <o.rempel@pengutronix.de>
> +R:	kernel@pengutronix.de
> +L:	linux-iio@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/iio/adc/ti,tsc2046.yaml
> +F:	drivers/iio/adc/ti-tsc2046.c
> +
>  TI TWL4030 SERIES SOC CODEC DRIVER
>  M:	Peter Ujfalusi <peter.ujfalusi@gmail.com>
>  L:	alsa-devel@alsa-project.org (moderated for non-subscribers)
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 15587a1bc80d..6ad6f04dfd20 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -1175,6 +1175,18 @@ config TI_TLC4541
>  	  This driver can also be built as a module. If so, the module will be
>  	  called ti-tlc4541.
>  
> +config TI_TSC2046
> +	tristate "Texas Instruments TSC2046 ADC driver"
> +	depends on SPI
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
> +	help
> +	  Say yes here to build support for ADC functionality of Texas
> +	  Instruments TSC2046 touch screen controller.
> +
> +	  This driver can also be built as a module. If so, the module will be
> +	  called ti-tsc2046.
> +
>  config TWL4030_MADC
>  	tristate "TWL4030 MADC (Monitoring A/D Converter)"
>  	depends on TWL4030_CORE
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 5fca90ada0ec..440e18ac6780 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -105,6 +105,7 @@ obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o
>  obj-$(CONFIG_TI_ADS124S08) += ti-ads124s08.o
>  obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
>  obj-$(CONFIG_TI_TLC4541) += ti-tlc4541.o
> +obj-$(CONFIG_TI_TSC2046) += ti-tsc2046.o
>  obj-$(CONFIG_TWL4030_MADC) += twl4030-madc.o
>  obj-$(CONFIG_TWL6030_GPADC) += twl6030-gpadc.o
>  obj-$(CONFIG_VF610_ADC) += vf610_adc.o
> diff --git a/drivers/iio/adc/ti-tsc2046.c b/drivers/iio/adc/ti-tsc2046.c
> new file mode 100644
> index 000000000000..7c3ae9181164
> --- /dev/null
> +++ b/drivers/iio/adc/ti-tsc2046.c
> @@ -0,0 +1,713 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Texas Instruments TSC2046 SPI ADC driver
> + *
> + * Copyright (c) 2021 Oleksij Rempel <kernel@pengutronix.de>, Pengutronix
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +
> +#include <asm/unaligned.h>
> +
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger.h>
> +
> +#define TI_TSC2046_NAME				"tsc2046"
> +
> +/* This driver doesn't aim at the peak continuous sample rate */
> +#define	TI_TSC2046_MAX_SAMPLE_RATE		125000
> +#define	TI_TSC2046_SAMPLE_BITS			(8 /*cmd*/ + 16 /*sample*/)
> +#define	TI_TSC2046_MAX_CLK_FREQ \
> +	(TI_TSC2046_MAX_SAMPLE_RATE * TI_TSC2046_SAMPLE_BITS)
> +
> +#define TI_TSC2046_SAMPLE_INTERVAL_US		10000
> +
> +#define TI_TSC2046_START			BIT(7)
> +#define TI_TSC2046_ADDR				GENMASK(6, 4)
> +#define TI_TSC2046_ADDR_TEMP1			7
> +#define TI_TSC2046_ADDR_AUX			6
> +#define TI_TSC2046_ADDR_X			5
> +#define TI_TSC2046_ADDR_Z2			4
> +#define TI_TSC2046_ADDR_Z1			3
> +#define TI_TSC2046_ADDR_VBAT			2
> +#define TI_TSC2046_ADDR_Y			1
> +#define TI_TSC2046_ADDR_TEMP0			0
> +
> +/*
> + * The mode bit sets the resolution of the ADC. With this bit low, the next
> + * conversion has 12-bit resolution, whereas with this bit high, the next
> + * conversion has 8-bit resolution. This driver is optimized for 12-bit mode.
> + * So, for this driver, this bit should stay zero.
> + */
> +#define TI_TSC2046_8BIT_MODE			BIT(3)
> +
> +/*
> + * SER/DFR - The SER/DFR bit controls the reference mode, either single-ended
> + * (high) or differential (low).
> + */
> +#define TI_TSC2046_SER				BIT(2)
> +
> +/*
> + * If VREF_ON and ADC_ON are both zero, then the chip operates in
> + * auto-wake/suspend mode. In most case this bits should stay zero.
> + */
> +#define TI_TSC2046_PD1_VREF_ON			BIT(1)
> +#define TI_TSC2046_PD0_ADC_ON			BIT(0)
> +
> +#define TI_TSC2046_MAX_CHAN			8
> +
> +/* Represents a HW sample */
> +struct tsc2046_adc_atom {
> +	/*
> +	 * Command transmitted to the controller. This filed is empty on the RX
> +	 * buffer.
> +	 */
> +	u8 cmd;
> +	/*
> +	 * Data received from the controller. This filed is empty for the TX
> +	 * buffer
> +	 */
> +	__be16 data;
> +} __packed;
> +
> +struct tsc2046_adc_scan_buf {
> +	/* Scan data for each channel */
> +	u16 data[TI_TSC2046_MAX_CHAN];
> +	/* Timestamp */
> +	s64 ts __aligned(8);
> +};
> +
> +/* Layout of atomic buffers within big buffer */
> +struct tsc2046_adc_group_layout {
> +	/* Group offset within the SPI RX buffer */
> +	unsigned int offset;
> +	/*
> +	 * Amount of tsc2046_adc_atom structs within the same command gathered
> +	 * within same group.
> +	 */
> +	unsigned int count;
> +	/*
> +	 * Settling samples (tsc2046_adc_atom structs) which should be skipped
> +	 * before good samples will start.
> +	 */
> +	unsigned int skip;
> +};
> +
> +struct tsc2046_adc_dcfg {
> +	const struct iio_chan_spec *channels;
> +	unsigned int num_channels;
> +};
> +
> +struct tsc2046_adc_ch_cfg {
> +	unsigned int settling_time_us;
> +	unsigned int average_samples;
> +};
> +
> +struct tsc2046_adc_priv {
> +	struct spi_device *spi;
> +	const struct tsc2046_adc_dcfg *dcfg;
> +
> +	struct iio_trigger *trig;
> +	struct hrtimer trig_timer;
> +	spinlock_t trig_lock;
> +	atomic_t trig_more_count;
> +
> +	struct spi_transfer xfer;
> +	struct spi_message msg;
> +
> +	struct tsc2046_adc_scan_buf scan_buf;
> +	/*
> +	 * Lock to protect the layout and the spi transfer buffer.
> +	 * tsc2046_adc_group_layout can be changed within update_scan_mode(),
> +	 * in this case the l[] and tx/rx buffer will be out of sync to each
> +	 * other.
> +	 */
> +	struct mutex slock;
> +	struct tsc2046_adc_group_layout l[TI_TSC2046_MAX_CHAN];
> +	struct tsc2046_adc_atom *rx;
> +	struct tsc2046_adc_atom *tx;
> +
> +	struct tsc2046_adc_atom *rx_one;
> +	struct tsc2046_adc_atom *tx_one;
> +
> +	unsigned int count;
> +	unsigned int groups;
> +	u32 effective_speed_hz;
> +	u32 scan_interval_us;
> +	u32 time_per_scan_us;
> +	u32 time_per_bit_ns;
> +
> +	struct tsc2046_adc_ch_cfg ch_cfg[TI_TSC2046_MAX_CHAN];
> +};
> +
> +#define TI_TSC2046_V_CHAN(index, bits, name)			\
> +{								\
> +	.type = IIO_VOLTAGE,					\
> +	.indexed = 1,						\
> +	.channel = index,					\
> +	.address = index,					\

Don't think you ever use address, so don't set it.

> +	.datasheet_name = "#name",				\
> +	.scan_index = index,					\
> +	.scan_type = {						\
> +		.sign = 'u',					\
> +		.realbits = bits,				\
> +		.storagebits = 16,				\
> +		.shift = 0,					\

Shift of 0 assumed default, so no need to specify that here (it'll end
up 0 anyway)

> +		.endianness = IIO_CPU,				\
> +	},							\
> +}
> +
> +#define DECLARE_TI_TSC2046_8_CHANNELS(name, bits) \
> +const struct iio_chan_spec name ## _channels[] = { \
> +	TI_TSC2046_V_CHAN(0, bits, TEMP0), \
> +	TI_TSC2046_V_CHAN(1, bits, Y), \
> +	TI_TSC2046_V_CHAN(2, bits, VBAT), \
> +	TI_TSC2046_V_CHAN(3, bits, Z1), \
> +	TI_TSC2046_V_CHAN(4, bits, Z2), \
> +	TI_TSC2046_V_CHAN(5, bits, X), \
> +	TI_TSC2046_V_CHAN(6, bits, AUX), \
> +	TI_TSC2046_V_CHAN(7, bits, TEMP1), \
> +	IIO_CHAN_SOFT_TIMESTAMP(8), \
> +}
> +
> +static DECLARE_TI_TSC2046_8_CHANNELS(tsc2046_adc, 12);
> +
> +static const struct tsc2046_adc_dcfg tsc2046_adc_dcfg_tsc2046e = {
> +	.channels = tsc2046_adc_channels,
> +	.num_channels = ARRAY_SIZE(tsc2046_adc_channels),
> +};
> +
> +/*
> + * Convert time to a number of samples which can be transferred within this
> + * time.
> + */
> +static unsigned int tsc2046_adc_time_to_count(struct tsc2046_adc_priv *priv,
> +					      unsigned long time)
> +{
> +	unsigned int bit_count, sample_count;
> +
> +	bit_count = DIV_ROUND_UP(time * NSEC_PER_USEC, priv->time_per_bit_ns);
> +	sample_count = DIV_ROUND_UP(bit_count, TI_TSC2046_SAMPLE_BITS);
> +
> +	dev_dbg(&priv->spi->dev, "Effective speed %u, time per bit: %u, count bits: %u, count samples: %u\n",
> +		priv->effective_speed_hz, priv->time_per_bit_ns,
> +		bit_count, sample_count);
> +
> +	return sample_count;
> +}
> +
> +static u8 tsc2046_adc_get_cmd(struct tsc2046_adc_priv *priv, int ch_idx,
> +			      bool keep_power)
> +{
> +	u32 pd;
> +
> +	/*
> +	 * if PD bits are 0, controller will automatically disable ADC, VREF and
> +	 * enable IRQ.
> +	 */
> +	if (keep_power)
> +		pd = TI_TSC2046_PD0_ADC_ON;
> +	else
> +		pd = 0;
> +
> +	return TI_TSC2046_START | FIELD_PREP(TI_TSC2046_ADDR, ch_idx) | pd;
> +}
> +
> +static u16 tsc2046_adc_get_value(struct tsc2046_adc_atom *buf)
> +{
> +	/* Last 3 bits on the wire are empty */
> +	return get_unaligned_be16(&buf->data) >> 3;
> +}
> +
> +static int tsc2046_adc_read_one(struct tsc2046_adc_priv *priv, int ch_idx,
> +				u32 *effective_speed_hz)
> +{
> +	struct spi_transfer xfer;
> +	struct spi_message msg;
> +	int ret;
> +
> +	memset(&xfer, 0, sizeof(xfer));
> +	priv->tx_one->cmd = tsc2046_adc_get_cmd(priv, ch_idx, false);
> +	priv->tx_one->data = 0;
> +	xfer.tx_buf = priv->tx_one;
> +	xfer.rx_buf = priv->rx_one;
> +	xfer.len = sizeof(*priv->tx_one);
> +	spi_message_init(&msg);
> +	spi_message_add_tail(&xfer, &msg);

Perhaps add a comment that you aren't using spi_write_then_read() because
you need to be able to get hold of the effective_speed_hz from the
xfer.

> +
> +	ret = spi_sync(priv->spi, &msg);
> +	if (ret) {
> +		dev_err_ratelimited(&priv->spi->dev, "SPI transfer filed %pe\n",
> +				    ERR_PTR(ret));
> +		return ret;
> +	}
> +
> +	if (effective_speed_hz)
> +		*effective_speed_hz = xfer.effective_speed_hz;
> +
> +	return tsc2046_adc_get_value(priv->rx_one);
> +}
> +
> +static size_t tsc2046_adc_group_set_layout(struct tsc2046_adc_priv *priv,
> +					   unsigned int group,
> +					   unsigned int ch_idx)
> +{
> +	struct tsc2046_adc_ch_cfg *ch = &priv->ch_cfg[ch_idx];
> +	struct tsc2046_adc_group_layout *prev, *cur;
> +	unsigned int max_count, count_skip;
> +	unsigned int offset = 0;
> +

One blank line almost always enough :)

> +
> +	if (group) {
> +		prev = &priv->l[group - 1];
> +		offset = prev->offset + prev->count;
> +	}
> +
> +	cur = &priv->l[group];
> +
> +	count_skip = tsc2046_adc_time_to_count(priv, ch->settling_time_us);
> +	max_count = count_skip + ch->average_samples;
> +
> +	cur->offset = offset;
> +	cur->count = max_count;
> +	cur->skip = count_skip;
> +
> +	return sizeof(*priv->tx) * max_count;
> +}
> +
> +static void tsc2046_adc_group_set_cmd(struct tsc2046_adc_priv *priv,
> +				      unsigned int group, int ch_idx)
> +{
> +	struct tsc2046_adc_group_layout *l = &priv->l[group];
> +	unsigned int i;
> +	u8 cmd;
> +
> +	/*
> +	 * Do not enable automatic power down on working samples. Otherwise the
> +	 * plates will never be completely charged.
> +	 */
> +	cmd = tsc2046_adc_get_cmd(priv, ch_idx, true);
> +
> +	for (i = 0; i < l->count - 1; i++)
> +		priv->tx[l->offset + i].cmd = cmd;
> +
> +	/* automatically power down on last sample */
> +	priv->tx[l->offset + i].cmd = tsc2046_adc_get_cmd(priv, ch_idx, false);
> +}
> +
> +static u16 tsc2046_adc_get_val(struct tsc2046_adc_priv *priv, int group)
> +{
> +	struct tsc2046_adc_group_layout *l;
> +	unsigned int val, val_normalized = 0;
> +	int valid_count, i;
> +
> +	l = &priv->l[group];
> +	valid_count = l->count - l->skip;
> +
> +	for (i = 0; i < valid_count; i++) {
> +		val = tsc2046_adc_get_value(&priv->rx[l->offset + l->skip + i]);
> +		val_normalized += val;
> +	}
> +
> +	return DIV_ROUND_UP(val_normalized, valid_count);
> +}
> +
> +static int tsc2046_adc_scan(struct iio_dev *indio_dev)
> +{
> +	struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
> +	struct device *dev = &priv->spi->dev;
> +	int group;
> +	int ret;
> +
> +	ret = spi_sync(priv->spi, &priv->msg);
> +	if (ret < 0) {
> +		dev_err_ratelimited(dev, "SPI transfer filed: %pe\n",
> +				    ERR_PTR(ret));
> +		return ret;
> +	}
> +
> +	for (group = 0; group < priv->groups; group++) {
> +		u16 val = tsc2046_adc_get_val(priv, group);
> +
> +		priv->scan_buf.data[group] = val;

nitpick, but why not the more compact:
		priv->scan_buf.data[group] = tsc2046_adc_get_val(priv, group);
?


> +	}
> +
> +	ret = iio_push_to_buffers_with_timestamp(indio_dev, &priv->scan_buf,
> +						 iio_get_time_ns(indio_dev));
> +	/*
> +	 * If there's no consumer (or consumer is kfifo), may get a EBUSY here
> +	 * - ignore it.

The no consumer case should be impossible (I hope!), but fair enough of kfifo side of
things.

> +	 */
> +	if (ret < 0 && ret != -EBUSY) {
> +		dev_err_ratelimited(dev, "Filed to push scan buffer %pe\n",
> +				    ERR_PTR(ret));

Failed

> +
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static irqreturn_t tsc2046_adc_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
> +
> +	mutex_lock(&priv->slock);
> +	tsc2046_adc_scan(indio_dev);
> +	mutex_unlock(&priv->slock);
> +
> +	iio_trigger_notify_done(indio_dev->trig);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int tsc2046_adc_update_scan_mode(struct iio_dev *indio_dev,
> +					const unsigned long *active_scan_mask)
> +{
> +	struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
> +	unsigned int ch_idx, group = 0;
> +	size_t size = 0;
> +
> +	mutex_lock(&priv->slock);
> +
> +	for_each_set_bit(ch_idx, active_scan_mask, indio_dev->num_channels) {
> +		size += tsc2046_adc_group_set_layout(priv, group, ch_idx);
> +		tsc2046_adc_group_set_cmd(priv, group, ch_idx);
> +		group++;
> +	}
> +
> +	priv->groups = group;
> +	priv->xfer.len = size;
> +	priv->time_per_scan_us = size * 8 * priv->time_per_bit_ns / NSEC_PER_USEC;
> +
> +	if ((priv->scan_interval_us - priv->time_per_scan_us) < 0)
> +		dev_warn(&priv->spi->dev, "The scan interval (%d) is less then calculated scan time (%d)\n",
> +			 priv->scan_interval_us, priv->time_per_scan_us);
> +
> +	mutex_unlock(&priv->slock);
> +
> +	return 0;
> +}
> +
> +static const struct iio_info tsc2046_adc_info = {
> +	.update_scan_mode = tsc2046_adc_update_scan_mode,
> +};
> +
> +static enum hrtimer_restart tsc2046_adc_trig_more(struct hrtimer *hrtimer)
> +{
> +	struct tsc2046_adc_priv *priv = container_of(hrtimer,
> +						     struct tsc2046_adc_priv,
> +						     trig_timer);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&priv->trig_lock, flags);
> +
> +	disable_irq_nosync(priv->spi->irq);
> +
> +	atomic_inc(&priv->trig_more_count);
> +	iio_trigger_poll(priv->trig);
> +
> +	spin_unlock_irqrestore(&priv->trig_lock, flags);
> +
> +	return HRTIMER_NORESTART;
> +}
> +
> +static irqreturn_t tsc2046_adc_irq(int irq, void *dev_id)
> +{
> +	struct iio_dev *indio_dev = dev_id;
> +	struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
> +

One blank line is enough.

> +
> +	spin_lock(&priv->trig_lock);
> +
> +	hrtimer_try_to_cancel(&priv->trig_timer);
> +
> +	atomic_set(&priv->trig_more_count, 0);
> +	disable_irq_nosync(priv->spi->irq);
> +	iio_trigger_poll(priv->trig);
> +
> +	spin_unlock(&priv->trig_lock);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static void tsc2046_adc_reenable_trigger(struct iio_trigger *trig)
> +{
> +	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
> +	struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
> +	unsigned long flags;
> +	int delta;
> +
> +	/*
> +	 * We can sample it as fast as we can, but usually we do not need so
> +	 * many samples. Reduce the sample rate for default (touchscreen) use
> +	 * case.
> +	 * Currently we do not need a highly precise sample rate. It is enough
> +	 * to have calculated numbers.
> +	 */
> +	delta = priv->scan_interval_us - priv->time_per_scan_us;
> +	if (delta > 0)
> +		fsleep(delta);
> +
> +	spin_lock_irqsave(&priv->trig_lock, flags);
> +
> +	/*
> +	 * We need to trigger at least one extra sample to detect state
> +	 * difference on ADC side.
> +	 */
> +	if (atomic_read(&priv->trig_more_count) == 0) {
> +		int timeout_ms = DIV_ROUND_UP(priv->scan_interval_us,
> +					      USEC_PER_MSEC);
> +
> +		hrtimer_start(&priv->trig_timer, ms_to_ktime(timeout_ms),
> +			      HRTIMER_MODE_REL_SOFT);
> +	}
> +
> +	enable_irq(priv->spi->irq);
> +
> +	spin_unlock_irqrestore(&priv->trig_lock, flags);
> +}
> +
> +static int tsc2046_adc_set_trigger_state(struct iio_trigger *trig, bool enable)
> +{
> +	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
> +	struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
> +
> +	if (enable) {
> +		enable_irq(priv->spi->irq);
> +	} else {
> +		disable_irq(priv->spi->irq);
> +		hrtimer_try_to_cancel(&priv->trig_timer);
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct iio_trigger_ops tsc2046_adc_trigger_ops = {
> +	.set_trigger_state = tsc2046_adc_set_trigger_state,
> +	.reenable = tsc2046_adc_reenable_trigger,
> +};
> +
> +static int tsc2046_adc_setup_spi_msg(struct tsc2046_adc_priv *priv)
> +{
> +	unsigned int ch_idx;
> +	size_t size = 0;
> +	int ret;
> +
> +	priv->tx_one = devm_kzalloc(&priv->spi->dev, sizeof(*priv->tx_one),
> +				    GFP_KERNEL);
> +	if (!priv->tx_one)
> +		return -ENOMEM;
> +
> +	priv->rx_one = devm_kzalloc(&priv->spi->dev, sizeof(*priv->rx_one),
> +				    GFP_KERNEL);
> +	if (!priv->rx_one)
> +		return -ENOMEM;
> +
> +	/*
> +	 * Make dummy read to set initial power state and get real SPI clock
> +	 * freq. It seems to be not important which channel is used for this
> +	 * case.
> +	 */
> +	ret = tsc2046_adc_read_one(priv, TI_TSC2046_ADDR_TEMP0,
> +				   &priv->effective_speed_hz);
> +	if (ret < 0)
> +		return ret;
> +
> +	/*
> +	 * In case SPI controller do not report effective_speed_hz, use
> +	 * configure value and hope it will match
> +	 */
> +	if (!priv->effective_speed_hz)
> +		priv->effective_speed_hz = priv->spi->max_speed_hz;
> +
> +
> +	priv->scan_interval_us = TI_TSC2046_SAMPLE_INTERVAL_US;
> +	priv->time_per_bit_ns = DIV_ROUND_UP(NSEC_PER_SEC,
> +					     priv->effective_speed_hz);
> +
> +	/*
> +	 * Calculate and allocate maximal size buffer if all channels are
> +	 * enabled.
> +	 */
> +	for (ch_idx = 0; ch_idx < priv->dcfg->num_channels; ch_idx++)
> +		size += tsc2046_adc_group_set_layout(priv, ch_idx, ch_idx);
> +
> +	priv->tx = devm_kzalloc(&priv->spi->dev, size, GFP_KERNEL);
> +	if (!priv->tx)
> +		return -ENOMEM;
> +
> +	priv->rx = devm_kzalloc(&priv->spi->dev, size, GFP_KERNEL);
> +	if (!priv->rx)
> +		return -ENOMEM;
> +
> +	spi_message_init(&priv->msg);
> +	priv->msg.context = priv;
> +
> +	priv->xfer.tx_buf = priv->tx;
> +	priv->xfer.rx_buf = priv->rx;
> +	priv->xfer.len = size;
> +	spi_message_add_tail(&priv->xfer, &priv->msg);
> +
> +	return 0;
> +}
> +
> +static void tsc2046_adc_parse_fwnode(struct tsc2046_adc_priv *priv)
> +{
> +	struct fwnode_handle *child;
> +	struct device *dev = &priv->spi->dev;
> +
> +	device_for_each_child_node(dev, child) {
> +		u32 stl, aver, reg;
> +		int ret;
> +
> +		ret = fwnode_property_read_u32(child, "reg", &reg);
> +		if (ret) {
> +			dev_err(dev, "invalid reg on %pfw, err: %pe\n", child,
> +				ERR_PTR(ret));
> +			continue;
> +		}
> +
> +		if (reg >= ARRAY_SIZE(priv->ch_cfg)) {
> +			dev_err(dev, "%pfw: Unsupported reg value: %i, max supported is: %i.\n",
> +				child, reg, ARRAY_SIZE(priv->ch_cfg));
> +			continue;
> +		}
> +
> +		ret = fwnode_property_read_u32(child, "settling-time-us", &stl);
> +		if (!ret)
> +			priv->ch_cfg[reg].settling_time_us = stl;
> +
> +		ret = fwnode_property_read_u32(child, "average-samples", &aver);
> +		if (!ret)
> +			priv->ch_cfg[reg].average_samples = aver;
> +	}
> +}
> +
> +static int tsc2046_adc_probe(struct spi_device *spi)
> +{
> +	const struct tsc2046_adc_dcfg *dcfg;
> +	struct device *dev = &spi->dev;
> +	struct tsc2046_adc_priv *priv;
> +	struct iio_dev *indio_dev;
> +	struct iio_trigger *trig;
> +	const char *name;
> +	int ret;
> +
> +	if (spi->max_speed_hz > TI_TSC2046_MAX_CLK_FREQ) {
> +		dev_err(dev, "SPI max_speed_hz is too high: %d Hz. Max supported freq is %d Hz\n",
> +			spi->max_speed_hz, TI_TSC2046_MAX_CLK_FREQ);
> +		return -EINVAL;
> +	}
> +
> +	dcfg = device_get_match_data(dev);
> +	if (!dcfg)
> +		return -EINVAL;
> +
> +	spi->bits_per_word = 8;
> +	spi->mode &= ~SPI_MODE_X_MASK;
> +	spi->mode |= SPI_MODE_0;
> +	ret = spi_setup(spi);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Error in spi setup\n");
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*priv));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	priv = iio_priv(indio_dev);
> +	priv->dcfg = dcfg;
> +
> +	spi_set_drvdata(spi, indio_dev);
> +
> +	priv->spi = spi;
> +
> +	name = devm_kasprintf(dev, GFP_KERNEL, "%s-%s",
> +			      TI_TSC2046_NAME, dev_name(dev));
> +
> +	indio_dev->name = name;
> +	indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_TRIGGERED;
> +	indio_dev->channels = dcfg->channels;
> +	indio_dev->num_channels = dcfg->num_channels;
> +	indio_dev->info = &tsc2046_adc_info;
> +
> +	tsc2046_adc_parse_fwnode(priv);
> +
> +	ret = tsc2046_adc_setup_spi_msg(priv);
> +	if (ret)
> +		return ret;
> +
> +	mutex_init(&priv->slock);
> +
> +	/* TODO: remove IRQ_NOAUTOEN after needed patches are mainline */
> +	irq_set_status_flags(spi->irq, IRQ_NOAUTOEN);
> +	ret = devm_request_irq(dev, spi->irq, &tsc2046_adc_irq,
> +			       0, name, indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	trig = devm_iio_trigger_alloc(dev, "%s-dev%d", indio_dev->name,
> +				      indio_dev->id);

Given the trigger a more descriptive name to reflect that it is 'touch'
based.

> +	if (!trig)
> +		return -ENOMEM;
> +
> +	priv->trig = trig;
> +	trig->dev.parent = indio_dev->dev.parent;
> +	iio_trigger_set_drvdata(trig, indio_dev);
> +	trig->ops = &tsc2046_adc_trigger_ops;
> +
> +	spin_lock_init(&priv->trig_lock);
> +	hrtimer_init(&priv->trig_timer, CLOCK_MONOTONIC,
> +		     HRTIMER_MODE_REL_SOFT);
> +	priv->trig_timer.function = tsc2046_adc_trig_more;
> +
> +	ret = devm_iio_trigger_register(dev, trig);
> +	if (ret) {
> +		dev_err(dev, "failed to register trigger\n");
> +		return ret;
> +	}
> +
> +	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
> +					      &tsc2046_adc_trigger_handler, NULL);
> +	if (ret) {
> +		dev_err(dev, "Failed to setup triggered buffer\n");
> +		return ret;
> +	}
> +
> +	/* set default trigger */
> +	indio_dev->trig = iio_trigger_get(priv->trig);
> +
> +	ret = devm_iio_device_register(dev, indio_dev);
> +	if (ret) {
> +		dev_err(dev, "Failed to register iio device\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id ads7950_of_table[] = {
> +	{ .compatible = "ti,tsc2046e-adc", .data = &tsc2046_adc_dcfg_tsc2046e },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ads7950_of_table);
> +
> +static struct spi_driver tsc2046_adc_driver = {
> +	.driver = {
> +		.name	= "tsc2046",
> +		.of_match_table = ads7950_of_table,
> +	},
> +	.probe		= tsc2046_adc_probe,
> +};
> +module_spi_driver(tsc2046_adc_driver);
> +
> +MODULE_AUTHOR("Oleksij Rempel <kernel@pengutronix.de>");
> +MODULE_DESCRIPTION("TI TSC2046 ADC");
> +MODULE_LICENSE("GPL v2");


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/3] dt-bindings:iio:adc: add documentation for TI TSC2046 controller
  2021-03-12 10:55 ` [PATCH v2 2/3] dt-bindings:iio:adc: add documentation for TI TSC2046 controller Oleksij Rempel
@ 2021-03-15 16:20   ` Rob Herring
  0 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2021-03-15 16:20 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Robin van der Gracht, Peter Meerwald-Stadler, Jonathan Cameron,
	Dmitry Torokhov, Pengutronix Kernel Team, linux-kernel,
	David Jander, devicetree, linux-iio, Rob Herring,
	Lars-Peter Clausen

On Fri, 12 Mar 2021 11:55:14 +0100, Oleksij Rempel wrote:
> Add a binding documentation for the TI TSC2046 touchscreen controllers
> ADC functionality.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  .../bindings/iio/adc/ti,tsc2046.yaml          | 115 ++++++++++++++++++
>  1 file changed, 115 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,tsc2046.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] iio: adc: add ADC driver for the TI TSC2046 controller
  2021-03-13 16:11   ` Jonathan Cameron
@ 2021-03-16  5:57     ` Oleksij Rempel
  0 siblings, 0 replies; 11+ messages in thread
From: Oleksij Rempel @ 2021-03-16  5:57 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: devicetree, Lars-Peter Clausen, Peter Meerwald-Stadler,
	linux-iio, Robin van der Gracht, Dmitry Torokhov, linux-kernel,
	Rob Herring, Pengutronix Kernel Team, David Jander

Hi,

On Sat, Mar 13, 2021 at 04:11:19PM +0000, Jonathan Cameron wrote:
> On Fri, 12 Mar 2021 11:55:15 +0100
> Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> 
> > Basically the TI TSC2046 touchscreen controller is 8 channel ADC optimized for
> > the touchscreen use case. By implementing it as IIO ADC device, we can
> > make use of resistive-adc-touch and iio-hwmon drivers.
> > 
> > So far, this driver was tested with custom version of resistive-adc-touch driver,
> > since it need to be extended to make use of Z1 and Z2 channels. The X/Y
> > are working without additional changes.
> > 
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> 
> So the flow in here is still rather non obvious and touchscreen specific.
> Please add some documentation on what the trigger actually is. It seems
> to be some combination of an hrtimer and a interrupt driven trigger.
> I 'think' the idea is to ensure you get one 'no touch' measurement once
> touch is removed?


Yes, this IRQ line is just a voltage level converter integrated in to
controller. As soon voltage level reaches some threshold the IRQ level
is changed. It means, the IRQ is inactive as soon as nothing is pressing
on the screen, or the channel is changed. Since we need more sample and not so
frequent, I made this construction of rate limited IRQ with extra triggering on
the end.

Potentially we can use hrtimer trigger, especially if the IRQ line
is not attached or can't be used for some reason. But for most use cases
this trigger will provide better power efficiency.

I can imagine, that in case of field board diagnostic, we may need some extra
functionality. So the workflow will look like:
- unbind the touchscreen driver
- disable oversampling
- disable settling time
- attach sysfs trigger
- enable one of channels
- start grabbing data over IIO char dev

But this functionality will need some more work and currently has lowest
prio.

> I guess we don't need the performance but I was a bit surprised that I didn't
> see this doing overlapping of the previous read out with the config for the
> next sample.

I can't follow here. Can you please explain.

> Anyhow, overall this looks pretty good to me.  Add that a bit of documentation
> for the trigger to the comments at the top of the file and tidy up the last
> few things inline.

ok

> It's a bit unusual as ADC drivers go, but not so strange that it worries
> me that much.

:)

> Thanks,
> 
> Jonathan
> 
> > ---
> >  MAINTAINERS                  |   8 +
> >  drivers/iio/adc/Kconfig      |  12 +
> >  drivers/iio/adc/Makefile     |   1 +
> >  drivers/iio/adc/ti-tsc2046.c | 713 +++++++++++++++++++++++++++++++++++
> >  4 files changed, 734 insertions(+)
> >  create mode 100644 drivers/iio/adc/ti-tsc2046.c
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 3fea1a934b32..2d33c6442a55 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -17852,6 +17852,14 @@ S:	Supported
> >  F:	Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> >  F:	drivers/nfc/trf7970a.c
> >  
> > +TI TSC2046 ADC DRIVER
> > +M:	Oleksij Rempel <o.rempel@pengutronix.de>
> > +R:	kernel@pengutronix.de
> > +L:	linux-iio@vger.kernel.org
> > +S:	Maintained
> > +F:	Documentation/devicetree/bindings/iio/adc/ti,tsc2046.yaml
> > +F:	drivers/iio/adc/ti-tsc2046.c
> > +
> >  TI TWL4030 SERIES SOC CODEC DRIVER
> >  M:	Peter Ujfalusi <peter.ujfalusi@gmail.com>
> >  L:	alsa-devel@alsa-project.org (moderated for non-subscribers)
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index 15587a1bc80d..6ad6f04dfd20 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -1175,6 +1175,18 @@ config TI_TLC4541
> >  	  This driver can also be built as a module. If so, the module will be
> >  	  called ti-tlc4541.
> >  
> > +config TI_TSC2046
> > +	tristate "Texas Instruments TSC2046 ADC driver"
> > +	depends on SPI
> > +	select IIO_BUFFER
> > +	select IIO_TRIGGERED_BUFFER
> > +	help
> > +	  Say yes here to build support for ADC functionality of Texas
> > +	  Instruments TSC2046 touch screen controller.
> > +
> > +	  This driver can also be built as a module. If so, the module will be
> > +	  called ti-tsc2046.
> > +
> >  config TWL4030_MADC
> >  	tristate "TWL4030 MADC (Monitoring A/D Converter)"
> >  	depends on TWL4030_CORE
> > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> > index 5fca90ada0ec..440e18ac6780 100644
> > --- a/drivers/iio/adc/Makefile
> > +++ b/drivers/iio/adc/Makefile
> > @@ -105,6 +105,7 @@ obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o
> >  obj-$(CONFIG_TI_ADS124S08) += ti-ads124s08.o
> >  obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
> >  obj-$(CONFIG_TI_TLC4541) += ti-tlc4541.o
> > +obj-$(CONFIG_TI_TSC2046) += ti-tsc2046.o
> >  obj-$(CONFIG_TWL4030_MADC) += twl4030-madc.o
> >  obj-$(CONFIG_TWL6030_GPADC) += twl6030-gpadc.o
> >  obj-$(CONFIG_VF610_ADC) += vf610_adc.o
> > diff --git a/drivers/iio/adc/ti-tsc2046.c b/drivers/iio/adc/ti-tsc2046.c
> > new file mode 100644
> > index 000000000000..7c3ae9181164
> > --- /dev/null
> > +++ b/drivers/iio/adc/ti-tsc2046.c
> > @@ -0,0 +1,713 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Texas Instruments TSC2046 SPI ADC driver
> > + *
> > + * Copyright (c) 2021 Oleksij Rempel <kernel@pengutronix.de>, Pengutronix
> > + */
> > +
> > +#include <linux/bitfield.h>
> > +#include <linux/delay.h>
> > +#include <linux/module.h>
> > +#include <linux/spi/spi.h>
> > +
> > +#include <asm/unaligned.h>
> > +
> > +#include <linux/iio/buffer.h>
> > +#include <linux/iio/trigger_consumer.h>
> > +#include <linux/iio/triggered_buffer.h>
> > +#include <linux/iio/trigger.h>
> > +
> > +#define TI_TSC2046_NAME				"tsc2046"
> > +
> > +/* This driver doesn't aim at the peak continuous sample rate */
> > +#define	TI_TSC2046_MAX_SAMPLE_RATE		125000
> > +#define	TI_TSC2046_SAMPLE_BITS			(8 /*cmd*/ + 16 /*sample*/)
> > +#define	TI_TSC2046_MAX_CLK_FREQ \
> > +	(TI_TSC2046_MAX_SAMPLE_RATE * TI_TSC2046_SAMPLE_BITS)
> > +
> > +#define TI_TSC2046_SAMPLE_INTERVAL_US		10000
> > +
> > +#define TI_TSC2046_START			BIT(7)
> > +#define TI_TSC2046_ADDR				GENMASK(6, 4)
> > +#define TI_TSC2046_ADDR_TEMP1			7
> > +#define TI_TSC2046_ADDR_AUX			6
> > +#define TI_TSC2046_ADDR_X			5
> > +#define TI_TSC2046_ADDR_Z2			4
> > +#define TI_TSC2046_ADDR_Z1			3
> > +#define TI_TSC2046_ADDR_VBAT			2
> > +#define TI_TSC2046_ADDR_Y			1
> > +#define TI_TSC2046_ADDR_TEMP0			0
> > +
> > +/*
> > + * The mode bit sets the resolution of the ADC. With this bit low, the next
> > + * conversion has 12-bit resolution, whereas with this bit high, the next
> > + * conversion has 8-bit resolution. This driver is optimized for 12-bit mode.
> > + * So, for this driver, this bit should stay zero.
> > + */
> > +#define TI_TSC2046_8BIT_MODE			BIT(3)
> > +
> > +/*
> > + * SER/DFR - The SER/DFR bit controls the reference mode, either single-ended
> > + * (high) or differential (low).
> > + */
> > +#define TI_TSC2046_SER				BIT(2)
> > +
> > +/*
> > + * If VREF_ON and ADC_ON are both zero, then the chip operates in
> > + * auto-wake/suspend mode. In most case this bits should stay zero.
> > + */
> > +#define TI_TSC2046_PD1_VREF_ON			BIT(1)
> > +#define TI_TSC2046_PD0_ADC_ON			BIT(0)
> > +
> > +#define TI_TSC2046_MAX_CHAN			8
> > +
> > +/* Represents a HW sample */
> > +struct tsc2046_adc_atom {
> > +	/*
> > +	 * Command transmitted to the controller. This filed is empty on the RX
> > +	 * buffer.
> > +	 */
> > +	u8 cmd;
> > +	/*
> > +	 * Data received from the controller. This filed is empty for the TX
> > +	 * buffer
> > +	 */
> > +	__be16 data;
> > +} __packed;
> > +
> > +struct tsc2046_adc_scan_buf {
> > +	/* Scan data for each channel */
> > +	u16 data[TI_TSC2046_MAX_CHAN];
> > +	/* Timestamp */
> > +	s64 ts __aligned(8);
> > +};
> > +
> > +/* Layout of atomic buffers within big buffer */
> > +struct tsc2046_adc_group_layout {
> > +	/* Group offset within the SPI RX buffer */
> > +	unsigned int offset;
> > +	/*
> > +	 * Amount of tsc2046_adc_atom structs within the same command gathered
> > +	 * within same group.
> > +	 */
> > +	unsigned int count;
> > +	/*
> > +	 * Settling samples (tsc2046_adc_atom structs) which should be skipped
> > +	 * before good samples will start.
> > +	 */
> > +	unsigned int skip;
> > +};
> > +
> > +struct tsc2046_adc_dcfg {
> > +	const struct iio_chan_spec *channels;
> > +	unsigned int num_channels;
> > +};
> > +
> > +struct tsc2046_adc_ch_cfg {
> > +	unsigned int settling_time_us;
> > +	unsigned int average_samples;
> > +};
> > +
> > +struct tsc2046_adc_priv {
> > +	struct spi_device *spi;
> > +	const struct tsc2046_adc_dcfg *dcfg;
> > +
> > +	struct iio_trigger *trig;
> > +	struct hrtimer trig_timer;
> > +	spinlock_t trig_lock;
> > +	atomic_t trig_more_count;
> > +
> > +	struct spi_transfer xfer;
> > +	struct spi_message msg;
> > +
> > +	struct tsc2046_adc_scan_buf scan_buf;
> > +	/*
> > +	 * Lock to protect the layout and the spi transfer buffer.
> > +	 * tsc2046_adc_group_layout can be changed within update_scan_mode(),
> > +	 * in this case the l[] and tx/rx buffer will be out of sync to each
> > +	 * other.
> > +	 */
> > +	struct mutex slock;
> > +	struct tsc2046_adc_group_layout l[TI_TSC2046_MAX_CHAN];
> > +	struct tsc2046_adc_atom *rx;
> > +	struct tsc2046_adc_atom *tx;
> > +
> > +	struct tsc2046_adc_atom *rx_one;
> > +	struct tsc2046_adc_atom *tx_one;
> > +
> > +	unsigned int count;
> > +	unsigned int groups;
> > +	u32 effective_speed_hz;
> > +	u32 scan_interval_us;
> > +	u32 time_per_scan_us;
> > +	u32 time_per_bit_ns;
> > +
> > +	struct tsc2046_adc_ch_cfg ch_cfg[TI_TSC2046_MAX_CHAN];
> > +};
> > +
> > +#define TI_TSC2046_V_CHAN(index, bits, name)			\
> > +{								\
> > +	.type = IIO_VOLTAGE,					\
> > +	.indexed = 1,						\
> > +	.channel = index,					\
> > +	.address = index,					\
> 
> Don't think you ever use address, so don't set it.
> 
> > +	.datasheet_name = "#name",				\
> > +	.scan_index = index,					\
> > +	.scan_type = {						\
> > +		.sign = 'u',					\
> > +		.realbits = bits,				\
> > +		.storagebits = 16,				\
> > +		.shift = 0,					\
> 
> Shift of 0 assumed default, so no need to specify that here (it'll end
> up 0 anyway)
> 
> > +		.endianness = IIO_CPU,				\
> > +	},							\
> > +}
> > +
> > +#define DECLARE_TI_TSC2046_8_CHANNELS(name, bits) \
> > +const struct iio_chan_spec name ## _channels[] = { \
> > +	TI_TSC2046_V_CHAN(0, bits, TEMP0), \
> > +	TI_TSC2046_V_CHAN(1, bits, Y), \
> > +	TI_TSC2046_V_CHAN(2, bits, VBAT), \
> > +	TI_TSC2046_V_CHAN(3, bits, Z1), \
> > +	TI_TSC2046_V_CHAN(4, bits, Z2), \
> > +	TI_TSC2046_V_CHAN(5, bits, X), \
> > +	TI_TSC2046_V_CHAN(6, bits, AUX), \
> > +	TI_TSC2046_V_CHAN(7, bits, TEMP1), \
> > +	IIO_CHAN_SOFT_TIMESTAMP(8), \
> > +}
> > +
> > +static DECLARE_TI_TSC2046_8_CHANNELS(tsc2046_adc, 12);
> > +
> > +static const struct tsc2046_adc_dcfg tsc2046_adc_dcfg_tsc2046e = {
> > +	.channels = tsc2046_adc_channels,
> > +	.num_channels = ARRAY_SIZE(tsc2046_adc_channels),
> > +};
> > +
> > +/*
> > + * Convert time to a number of samples which can be transferred within this
> > + * time.
> > + */
> > +static unsigned int tsc2046_adc_time_to_count(struct tsc2046_adc_priv *priv,
> > +					      unsigned long time)
> > +{
> > +	unsigned int bit_count, sample_count;
> > +
> > +	bit_count = DIV_ROUND_UP(time * NSEC_PER_USEC, priv->time_per_bit_ns);
> > +	sample_count = DIV_ROUND_UP(bit_count, TI_TSC2046_SAMPLE_BITS);
> > +
> > +	dev_dbg(&priv->spi->dev, "Effective speed %u, time per bit: %u, count bits: %u, count samples: %u\n",
> > +		priv->effective_speed_hz, priv->time_per_bit_ns,
> > +		bit_count, sample_count);
> > +
> > +	return sample_count;
> > +}
> > +
> > +static u8 tsc2046_adc_get_cmd(struct tsc2046_adc_priv *priv, int ch_idx,
> > +			      bool keep_power)
> > +{
> > +	u32 pd;
> > +
> > +	/*
> > +	 * if PD bits are 0, controller will automatically disable ADC, VREF and
> > +	 * enable IRQ.
> > +	 */
> > +	if (keep_power)
> > +		pd = TI_TSC2046_PD0_ADC_ON;
> > +	else
> > +		pd = 0;
> > +
> > +	return TI_TSC2046_START | FIELD_PREP(TI_TSC2046_ADDR, ch_idx) | pd;
> > +}
> > +
> > +static u16 tsc2046_adc_get_value(struct tsc2046_adc_atom *buf)
> > +{
> > +	/* Last 3 bits on the wire are empty */
> > +	return get_unaligned_be16(&buf->data) >> 3;
> > +}
> > +
> > +static int tsc2046_adc_read_one(struct tsc2046_adc_priv *priv, int ch_idx,
> > +				u32 *effective_speed_hz)
> > +{
> > +	struct spi_transfer xfer;
> > +	struct spi_message msg;
> > +	int ret;
> > +
> > +	memset(&xfer, 0, sizeof(xfer));
> > +	priv->tx_one->cmd = tsc2046_adc_get_cmd(priv, ch_idx, false);
> > +	priv->tx_one->data = 0;
> > +	xfer.tx_buf = priv->tx_one;
> > +	xfer.rx_buf = priv->rx_one;
> > +	xfer.len = sizeof(*priv->tx_one);
> > +	spi_message_init(&msg);
> > +	spi_message_add_tail(&xfer, &msg);
> 
> Perhaps add a comment that you aren't using spi_write_then_read() because
> you need to be able to get hold of the effective_speed_hz from the
> xfer.
> 
> > +
> > +	ret = spi_sync(priv->spi, &msg);
> > +	if (ret) {
> > +		dev_err_ratelimited(&priv->spi->dev, "SPI transfer filed %pe\n",
> > +				    ERR_PTR(ret));
> > +		return ret;
> > +	}
> > +
> > +	if (effective_speed_hz)
> > +		*effective_speed_hz = xfer.effective_speed_hz;
> > +
> > +	return tsc2046_adc_get_value(priv->rx_one);
> > +}
> > +
> > +static size_t tsc2046_adc_group_set_layout(struct tsc2046_adc_priv *priv,
> > +					   unsigned int group,
> > +					   unsigned int ch_idx)
> > +{
> > +	struct tsc2046_adc_ch_cfg *ch = &priv->ch_cfg[ch_idx];
> > +	struct tsc2046_adc_group_layout *prev, *cur;
> > +	unsigned int max_count, count_skip;
> > +	unsigned int offset = 0;
> > +
> 
> One blank line almost always enough :)
> 
> > +
> > +	if (group) {
> > +		prev = &priv->l[group - 1];
> > +		offset = prev->offset + prev->count;
> > +	}
> > +
> > +	cur = &priv->l[group];
> > +
> > +	count_skip = tsc2046_adc_time_to_count(priv, ch->settling_time_us);
> > +	max_count = count_skip + ch->average_samples;
> > +
> > +	cur->offset = offset;
> > +	cur->count = max_count;
> > +	cur->skip = count_skip;
> > +
> > +	return sizeof(*priv->tx) * max_count;
> > +}
> > +
> > +static void tsc2046_adc_group_set_cmd(struct tsc2046_adc_priv *priv,
> > +				      unsigned int group, int ch_idx)
> > +{
> > +	struct tsc2046_adc_group_layout *l = &priv->l[group];
> > +	unsigned int i;
> > +	u8 cmd;
> > +
> > +	/*
> > +	 * Do not enable automatic power down on working samples. Otherwise the
> > +	 * plates will never be completely charged.
> > +	 */
> > +	cmd = tsc2046_adc_get_cmd(priv, ch_idx, true);
> > +
> > +	for (i = 0; i < l->count - 1; i++)
> > +		priv->tx[l->offset + i].cmd = cmd;
> > +
> > +	/* automatically power down on last sample */
> > +	priv->tx[l->offset + i].cmd = tsc2046_adc_get_cmd(priv, ch_idx, false);
> > +}
> > +
> > +static u16 tsc2046_adc_get_val(struct tsc2046_adc_priv *priv, int group)
> > +{
> > +	struct tsc2046_adc_group_layout *l;
> > +	unsigned int val, val_normalized = 0;
> > +	int valid_count, i;
> > +
> > +	l = &priv->l[group];
> > +	valid_count = l->count - l->skip;
> > +
> > +	for (i = 0; i < valid_count; i++) {
> > +		val = tsc2046_adc_get_value(&priv->rx[l->offset + l->skip + i]);
> > +		val_normalized += val;
> > +	}
> > +
> > +	return DIV_ROUND_UP(val_normalized, valid_count);
> > +}
> > +
> > +static int tsc2046_adc_scan(struct iio_dev *indio_dev)
> > +{
> > +	struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
> > +	struct device *dev = &priv->spi->dev;
> > +	int group;
> > +	int ret;
> > +
> > +	ret = spi_sync(priv->spi, &priv->msg);
> > +	if (ret < 0) {
> > +		dev_err_ratelimited(dev, "SPI transfer filed: %pe\n",
> > +				    ERR_PTR(ret));
> > +		return ret;
> > +	}
> > +
> > +	for (group = 0; group < priv->groups; group++) {
> > +		u16 val = tsc2046_adc_get_val(priv, group);
> > +
> > +		priv->scan_buf.data[group] = val;
> 
> nitpick, but why not the more compact:
> 		priv->scan_buf.data[group] = tsc2046_adc_get_val(priv, group);
> ?
> 
> 
> > +	}
> > +
> > +	ret = iio_push_to_buffers_with_timestamp(indio_dev, &priv->scan_buf,
> > +						 iio_get_time_ns(indio_dev));
> > +	/*
> > +	 * If there's no consumer (or consumer is kfifo), may get a EBUSY here
> > +	 * - ignore it.
> 
> The no consumer case should be impossible (I hope!), but fair enough of kfifo side of
> things.
> 
> > +	 */
> > +	if (ret < 0 && ret != -EBUSY) {
> > +		dev_err_ratelimited(dev, "Filed to push scan buffer %pe\n",
> > +				    ERR_PTR(ret));
> 
> Failed
> 
> > +
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static irqreturn_t tsc2046_adc_trigger_handler(int irq, void *p)
> > +{
> > +	struct iio_poll_func *pf = p;
> > +	struct iio_dev *indio_dev = pf->indio_dev;
> > +	struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
> > +
> > +	mutex_lock(&priv->slock);
> > +	tsc2046_adc_scan(indio_dev);
> > +	mutex_unlock(&priv->slock);
> > +
> > +	iio_trigger_notify_done(indio_dev->trig);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static int tsc2046_adc_update_scan_mode(struct iio_dev *indio_dev,
> > +					const unsigned long *active_scan_mask)
> > +{
> > +	struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
> > +	unsigned int ch_idx, group = 0;
> > +	size_t size = 0;
> > +
> > +	mutex_lock(&priv->slock);
> > +
> > +	for_each_set_bit(ch_idx, active_scan_mask, indio_dev->num_channels) {
> > +		size += tsc2046_adc_group_set_layout(priv, group, ch_idx);
> > +		tsc2046_adc_group_set_cmd(priv, group, ch_idx);
> > +		group++;
> > +	}
> > +
> > +	priv->groups = group;
> > +	priv->xfer.len = size;
> > +	priv->time_per_scan_us = size * 8 * priv->time_per_bit_ns / NSEC_PER_USEC;
> > +
> > +	if ((priv->scan_interval_us - priv->time_per_scan_us) < 0)
> > +		dev_warn(&priv->spi->dev, "The scan interval (%d) is less then calculated scan time (%d)\n",
> > +			 priv->scan_interval_us, priv->time_per_scan_us);
> > +
> > +	mutex_unlock(&priv->slock);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct iio_info tsc2046_adc_info = {
> > +	.update_scan_mode = tsc2046_adc_update_scan_mode,
> > +};
> > +
> > +static enum hrtimer_restart tsc2046_adc_trig_more(struct hrtimer *hrtimer)
> > +{
> > +	struct tsc2046_adc_priv *priv = container_of(hrtimer,
> > +						     struct tsc2046_adc_priv,
> > +						     trig_timer);
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&priv->trig_lock, flags);
> > +
> > +	disable_irq_nosync(priv->spi->irq);
> > +
> > +	atomic_inc(&priv->trig_more_count);
> > +	iio_trigger_poll(priv->trig);
> > +
> > +	spin_unlock_irqrestore(&priv->trig_lock, flags);
> > +
> > +	return HRTIMER_NORESTART;
> > +}
> > +
> > +static irqreturn_t tsc2046_adc_irq(int irq, void *dev_id)
> > +{
> > +	struct iio_dev *indio_dev = dev_id;
> > +	struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
> > +
> 
> One blank line is enough.
> 
> > +
> > +	spin_lock(&priv->trig_lock);
> > +
> > +	hrtimer_try_to_cancel(&priv->trig_timer);
> > +
> > +	atomic_set(&priv->trig_more_count, 0);
> > +	disable_irq_nosync(priv->spi->irq);
> > +	iio_trigger_poll(priv->trig);
> > +
> > +	spin_unlock(&priv->trig_lock);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static void tsc2046_adc_reenable_trigger(struct iio_trigger *trig)
> > +{
> > +	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
> > +	struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
> > +	unsigned long flags;
> > +	int delta;
> > +
> > +	/*
> > +	 * We can sample it as fast as we can, but usually we do not need so
> > +	 * many samples. Reduce the sample rate for default (touchscreen) use
> > +	 * case.
> > +	 * Currently we do not need a highly precise sample rate. It is enough
> > +	 * to have calculated numbers.
> > +	 */
> > +	delta = priv->scan_interval_us - priv->time_per_scan_us;
> > +	if (delta > 0)
> > +		fsleep(delta);
> > +
> > +	spin_lock_irqsave(&priv->trig_lock, flags);
> > +
> > +	/*
> > +	 * We need to trigger at least one extra sample to detect state
> > +	 * difference on ADC side.
> > +	 */
> > +	if (atomic_read(&priv->trig_more_count) == 0) {
> > +		int timeout_ms = DIV_ROUND_UP(priv->scan_interval_us,
> > +					      USEC_PER_MSEC);
> > +
> > +		hrtimer_start(&priv->trig_timer, ms_to_ktime(timeout_ms),
> > +			      HRTIMER_MODE_REL_SOFT);
> > +	}
> > +
> > +	enable_irq(priv->spi->irq);
> > +
> > +	spin_unlock_irqrestore(&priv->trig_lock, flags);
> > +}
> > +
> > +static int tsc2046_adc_set_trigger_state(struct iio_trigger *trig, bool enable)
> > +{
> > +	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
> > +	struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
> > +
> > +	if (enable) {
> > +		enable_irq(priv->spi->irq);
> > +	} else {
> > +		disable_irq(priv->spi->irq);
> > +		hrtimer_try_to_cancel(&priv->trig_timer);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct iio_trigger_ops tsc2046_adc_trigger_ops = {
> > +	.set_trigger_state = tsc2046_adc_set_trigger_state,
> > +	.reenable = tsc2046_adc_reenable_trigger,
> > +};
> > +
> > +static int tsc2046_adc_setup_spi_msg(struct tsc2046_adc_priv *priv)
> > +{
> > +	unsigned int ch_idx;
> > +	size_t size = 0;
> > +	int ret;
> > +
> > +	priv->tx_one = devm_kzalloc(&priv->spi->dev, sizeof(*priv->tx_one),
> > +				    GFP_KERNEL);
> > +	if (!priv->tx_one)
> > +		return -ENOMEM;
> > +
> > +	priv->rx_one = devm_kzalloc(&priv->spi->dev, sizeof(*priv->rx_one),
> > +				    GFP_KERNEL);
> > +	if (!priv->rx_one)
> > +		return -ENOMEM;
> > +
> > +	/*
> > +	 * Make dummy read to set initial power state and get real SPI clock
> > +	 * freq. It seems to be not important which channel is used for this
> > +	 * case.
> > +	 */
> > +	ret = tsc2046_adc_read_one(priv, TI_TSC2046_ADDR_TEMP0,
> > +				   &priv->effective_speed_hz);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	/*
> > +	 * In case SPI controller do not report effective_speed_hz, use
> > +	 * configure value and hope it will match
> > +	 */
> > +	if (!priv->effective_speed_hz)
> > +		priv->effective_speed_hz = priv->spi->max_speed_hz;
> > +
> > +
> > +	priv->scan_interval_us = TI_TSC2046_SAMPLE_INTERVAL_US;
> > +	priv->time_per_bit_ns = DIV_ROUND_UP(NSEC_PER_SEC,
> > +					     priv->effective_speed_hz);
> > +
> > +	/*
> > +	 * Calculate and allocate maximal size buffer if all channels are
> > +	 * enabled.
> > +	 */
> > +	for (ch_idx = 0; ch_idx < priv->dcfg->num_channels; ch_idx++)
> > +		size += tsc2046_adc_group_set_layout(priv, ch_idx, ch_idx);
> > +
> > +	priv->tx = devm_kzalloc(&priv->spi->dev, size, GFP_KERNEL);
> > +	if (!priv->tx)
> > +		return -ENOMEM;
> > +
> > +	priv->rx = devm_kzalloc(&priv->spi->dev, size, GFP_KERNEL);
> > +	if (!priv->rx)
> > +		return -ENOMEM;
> > +
> > +	spi_message_init(&priv->msg);
> > +	priv->msg.context = priv;
> > +
> > +	priv->xfer.tx_buf = priv->tx;
> > +	priv->xfer.rx_buf = priv->rx;
> > +	priv->xfer.len = size;
> > +	spi_message_add_tail(&priv->xfer, &priv->msg);
> > +
> > +	return 0;
> > +}
> > +
> > +static void tsc2046_adc_parse_fwnode(struct tsc2046_adc_priv *priv)
> > +{
> > +	struct fwnode_handle *child;
> > +	struct device *dev = &priv->spi->dev;
> > +
> > +	device_for_each_child_node(dev, child) {
> > +		u32 stl, aver, reg;
> > +		int ret;
> > +
> > +		ret = fwnode_property_read_u32(child, "reg", &reg);
> > +		if (ret) {
> > +			dev_err(dev, "invalid reg on %pfw, err: %pe\n", child,
> > +				ERR_PTR(ret));
> > +			continue;
> > +		}
> > +
> > +		if (reg >= ARRAY_SIZE(priv->ch_cfg)) {
> > +			dev_err(dev, "%pfw: Unsupported reg value: %i, max supported is: %i.\n",
> > +				child, reg, ARRAY_SIZE(priv->ch_cfg));
> > +			continue;
> > +		}
> > +
> > +		ret = fwnode_property_read_u32(child, "settling-time-us", &stl);
> > +		if (!ret)
> > +			priv->ch_cfg[reg].settling_time_us = stl;
> > +
> > +		ret = fwnode_property_read_u32(child, "average-samples", &aver);
> > +		if (!ret)
> > +			priv->ch_cfg[reg].average_samples = aver;
> > +	}
> > +}
> > +
> > +static int tsc2046_adc_probe(struct spi_device *spi)
> > +{
> > +	const struct tsc2046_adc_dcfg *dcfg;
> > +	struct device *dev = &spi->dev;
> > +	struct tsc2046_adc_priv *priv;
> > +	struct iio_dev *indio_dev;
> > +	struct iio_trigger *trig;
> > +	const char *name;
> > +	int ret;
> > +
> > +	if (spi->max_speed_hz > TI_TSC2046_MAX_CLK_FREQ) {
> > +		dev_err(dev, "SPI max_speed_hz is too high: %d Hz. Max supported freq is %d Hz\n",
> > +			spi->max_speed_hz, TI_TSC2046_MAX_CLK_FREQ);
> > +		return -EINVAL;
> > +	}
> > +
> > +	dcfg = device_get_match_data(dev);
> > +	if (!dcfg)
> > +		return -EINVAL;
> > +
> > +	spi->bits_per_word = 8;
> > +	spi->mode &= ~SPI_MODE_X_MASK;
> > +	spi->mode |= SPI_MODE_0;
> > +	ret = spi_setup(spi);
> > +	if (ret < 0)
> > +		return dev_err_probe(dev, ret, "Error in spi setup\n");
> > +
> > +	indio_dev = devm_iio_device_alloc(dev, sizeof(*priv));
> > +	if (!indio_dev)
> > +		return -ENOMEM;
> > +
> > +	priv = iio_priv(indio_dev);
> > +	priv->dcfg = dcfg;
> > +
> > +	spi_set_drvdata(spi, indio_dev);
> > +
> > +	priv->spi = spi;
> > +
> > +	name = devm_kasprintf(dev, GFP_KERNEL, "%s-%s",
> > +			      TI_TSC2046_NAME, dev_name(dev));
> > +
> > +	indio_dev->name = name;
> > +	indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_TRIGGERED;
> > +	indio_dev->channels = dcfg->channels;
> > +	indio_dev->num_channels = dcfg->num_channels;
> > +	indio_dev->info = &tsc2046_adc_info;
> > +
> > +	tsc2046_adc_parse_fwnode(priv);
> > +
> > +	ret = tsc2046_adc_setup_spi_msg(priv);
> > +	if (ret)
> > +		return ret;
> > +
> > +	mutex_init(&priv->slock);
> > +
> > +	/* TODO: remove IRQ_NOAUTOEN after needed patches are mainline */
> > +	irq_set_status_flags(spi->irq, IRQ_NOAUTOEN);
> > +	ret = devm_request_irq(dev, spi->irq, &tsc2046_adc_irq,
> > +			       0, name, indio_dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	trig = devm_iio_trigger_alloc(dev, "%s-dev%d", indio_dev->name,
> > +				      indio_dev->id);
> 
> Given the trigger a more descriptive name to reflect that it is 'touch'
> based.
> 
> > +	if (!trig)
> > +		return -ENOMEM;
> > +
> > +	priv->trig = trig;
> > +	trig->dev.parent = indio_dev->dev.parent;
> > +	iio_trigger_set_drvdata(trig, indio_dev);
> > +	trig->ops = &tsc2046_adc_trigger_ops;
> > +
> > +	spin_lock_init(&priv->trig_lock);
> > +	hrtimer_init(&priv->trig_timer, CLOCK_MONOTONIC,
> > +		     HRTIMER_MODE_REL_SOFT);
> > +	priv->trig_timer.function = tsc2046_adc_trig_more;
> > +
> > +	ret = devm_iio_trigger_register(dev, trig);
> > +	if (ret) {
> > +		dev_err(dev, "failed to register trigger\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
> > +					      &tsc2046_adc_trigger_handler, NULL);
> > +	if (ret) {
> > +		dev_err(dev, "Failed to setup triggered buffer\n");
> > +		return ret;
> > +	}
> > +
> > +	/* set default trigger */
> > +	indio_dev->trig = iio_trigger_get(priv->trig);
> > +
> > +	ret = devm_iio_device_register(dev, indio_dev);
> > +	if (ret) {
> > +		dev_err(dev, "Failed to register iio device\n");
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id ads7950_of_table[] = {
> > +	{ .compatible = "ti,tsc2046e-adc", .data = &tsc2046_adc_dcfg_tsc2046e },
> > +	{ }
> > +};
> > +MODULE_DEVICE_TABLE(of, ads7950_of_table);
> > +
> > +static struct spi_driver tsc2046_adc_driver = {
> > +	.driver = {
> > +		.name	= "tsc2046",
> > +		.of_match_table = ads7950_of_table,
> > +	},
> > +	.probe		= tsc2046_adc_probe,
> > +};
> > +module_spi_driver(tsc2046_adc_driver);
> > +
> > +MODULE_AUTHOR("Oleksij Rempel <kernel@pengutronix.de>");
> > +MODULE_DESCRIPTION("TI TSC2046 ADC");
> > +MODULE_LICENSE("GPL v2");
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-03-16  5:58 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-12 10:55 [PATCH v2 0/3] mainline ti tsc2046 adc driver Oleksij Rempel
2021-03-12 10:55 ` [PATCH v2 1/3] dt-bindings:iio:adc: add generic settling-time-us and average-samples channel properties Oleksij Rempel
2021-03-13 15:15   ` Jonathan Cameron
2021-03-12 10:55 ` [PATCH v2 2/3] dt-bindings:iio:adc: add documentation for TI TSC2046 controller Oleksij Rempel
2021-03-15 16:20   ` Rob Herring
2021-03-12 10:55 ` [PATCH v2 3/3] iio: adc: add ADC driver for the " Oleksij Rempel
2021-03-12 15:32   ` Andy Shevchenko
2021-03-12 17:49   ` kernel test robot
2021-03-12 17:49     ` kernel test robot
2021-03-13 16:11   ` Jonathan Cameron
2021-03-16  5:57     ` Oleksij Rempel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.