* [PATCH v3 0/3] mainline ti tsc2046 adc driver
@ 2021-03-19 14:45 Oleksij Rempel
2021-03-19 14:45 ` [PATCH v3 1/3] dt-bindings:iio:adc: add generic settling-time-us and oversampling-ratio channel properties Oleksij Rempel
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Oleksij Rempel @ 2021-03-19 14:45 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 v3:
- different spell fixes
- add some notes about driver structure
- rename the trigger to point on the touchscreen nature of it
- rename DT binding to oversampling-ratio
- make sure we have some defaults in case no DT property is set
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
oversampling-ratio 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 | 728 ++++++++++++++++++
6 files changed, 873 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] 14+ messages in thread* [PATCH v3 1/3] dt-bindings:iio:adc: add generic settling-time-us and oversampling-ratio channel properties 2021-03-19 14:45 [PATCH v3 0/3] mainline ti tsc2046 adc driver Oleksij Rempel @ 2021-03-19 14:45 ` Oleksij Rempel 2021-03-19 14:45 ` [PATCH v3 2/3] dt-bindings:iio:adc: add documentation for TI TSC2046 controller Oleksij Rempel 2021-03-19 14:45 ` [PATCH v3 3/3] iio: adc: add ADC driver for the " Oleksij Rempel 2 siblings, 0 replies; 14+ messages in thread From: Oleksij Rempel @ 2021-03-19 14:45 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 oversampling-ratio 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..66fd4b45f097 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. + + oversampling-ratio: + $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] 14+ messages in thread
* [PATCH v3 2/3] dt-bindings:iio:adc: add documentation for TI TSC2046 controller 2021-03-19 14:45 [PATCH v3 0/3] mainline ti tsc2046 adc driver Oleksij Rempel 2021-03-19 14:45 ` [PATCH v3 1/3] dt-bindings:iio:adc: add generic settling-time-us and oversampling-ratio channel properties Oleksij Rempel @ 2021-03-19 14:45 ` Oleksij Rempel 2021-03-19 14:45 ` [PATCH v3 3/3] iio: adc: add ADC driver for the " Oleksij Rempel 2 siblings, 0 replies; 14+ messages in thread From: Oleksij Rempel @ 2021-03-19 14:45 UTC (permalink / raw) To: Rob Herring, Jonathan Cameron Cc: Oleksij Rempel, Rob Herring, 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> Reviewed-by: Rob Herring <robh@kernel.org> --- .../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..601d69971d84 --- /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 + oversampling-ratio: 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>; + oversampling-ratio = <5>; + }; + channel@2 { + reg = <2>; + }; + channel@3 { + reg = <3>; + settling-time-us = <700>; + oversampling-ratio = <5>; + }; + channel@4 { + reg = <4>; + settling-time-us = <700>; + oversampling-ratio = <5>; + }; + channel@5 { + reg = <5>; + settling-time-us = <700>; + oversampling-ratio = <5>; + }; + channel@6 { + reg = <6>; + }; + channel@7 { + reg = <7>; + }; + }; + }; +... -- 2.29.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 3/3] iio: adc: add ADC driver for the TI TSC2046 controller 2021-03-19 14:45 [PATCH v3 0/3] mainline ti tsc2046 adc driver Oleksij Rempel 2021-03-19 14:45 ` [PATCH v3 1/3] dt-bindings:iio:adc: add generic settling-time-us and oversampling-ratio channel properties Oleksij Rempel 2021-03-19 14:45 ` [PATCH v3 2/3] dt-bindings:iio:adc: add documentation for TI TSC2046 controller Oleksij Rempel @ 2021-03-19 14:45 ` Oleksij Rempel 2021-03-19 16:35 ` kernel test robot ` (2 more replies) 2 siblings, 3 replies; 14+ messages in thread From: Oleksij Rempel @ 2021-03-19 14:45 UTC (permalink / raw) To: Rob Herring, Jonathan Cameron Cc: Oleksij Rempel, Andy Shevchenko, 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 an IIO ADC device, we can make use of resistive-adc-touch and iio-hwmon drivers. So far, this driver was tested with a custom version of resistive-adc-touch driver, since it needs 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> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> --- MAINTAINERS | 8 + drivers/iio/adc/Kconfig | 12 + drivers/iio/adc/Makefile | 1 + drivers/iio/adc/ti-tsc2046.c | 728 +++++++++++++++++++++++++++++++++++ 4 files changed, 749 insertions(+) create mode 100644 drivers/iio/adc/ti-tsc2046.c diff --git a/MAINTAINERS b/MAINTAINERS index 0455cfd5c76c..2ea85a5bb4dd 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -18002,6 +18002,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 bf7d22fa4be2..a2898108f418 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..c8c0dd9087c5 --- /dev/null +++ b/drivers/iio/adc/ti-tsc2046.c @@ -0,0 +1,728 @@ +// 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> + +/* + * The PENIRQ of TSC2046 controller is implemented as level shifter attached to + * the X+ line. If voltage of the X+ line reaches a specific level the IRQ will + * be activated or deactivated. + * To make this kind of IRQ reusable as trigger following additions were + * implemented: + * - rate limiting: + * For typical touchscreen use case, we need to trigger about each 10ms. + * - hrtimer: + * Continue triggering at least once after the IRQ was deactivated. Then + * deactivate this trigger to stop sampling in order to reduce power + * consumption. + */ + +#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 \ + (sizeof(struct tsc2046_adc_atom) * BITS_PER_BYTE) +#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 oversampling_ratio; +}; + +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, \ + .datasheet_name = "#name", \ + .scan_index = index, \ + .scan_type = { \ + .sign = 'u', \ + .realbits = bits, \ + .storagebits = 16, \ + .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); + + /* + * We aren't using spi_write_then_read() because we 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; + + 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->oversampling_ratio; + + 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++) + 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 consumer is kfifo, we may get a EBUSY here - ignore it. */ + if (ret < 0 && ret != -EBUSY) { + dev_err_ratelimited(dev, "Failed 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; + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(priv->ch_cfg); i++) { + priv->ch_cfg[i].settling_time_us = 1; + priv->ch_cfg[i].oversampling_ratio = 1; + } + + device_for_each_child_node(dev, child) { + u32 stl, overs, reg; + int ret; + + ret = fwnode_property_read_u32(child, "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: %zu.\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, "oversampling-ratio", + &overs); + if (!ret) + priv->ch_cfg[reg].oversampling_ratio = overs; + } +} + +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, "touchscreen-%s", indio_dev->name); + 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] 14+ messages in thread
* Re: [PATCH v3 3/3] iio: adc: add ADC driver for the TI TSC2046 controller 2021-03-19 14:45 ` [PATCH v3 3/3] iio: adc: add ADC driver for the " Oleksij Rempel @ 2021-03-19 16:35 ` kernel test robot 2021-03-19 17:42 ` Andy Shevchenko 2021-03-20 15:46 ` Jonathan Cameron 2 siblings, 0 replies; 14+ messages in thread From: kernel test robot @ 2021-03-19 16:35 UTC (permalink / raw) To: Oleksij Rempel, Rob Herring, Jonathan Cameron Cc: kbuild-all, Oleksij Rempel, Andy Shevchenko, devicetree, linux-kernel, Pengutronix Kernel Team, David Jander, Robin van der Gracht, linux-iio [-- Attachment #1: Type: text/plain, Size: 6672 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-rc3 next-20210319] [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/20210319-224746 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git a74e6a014c9d4d4161061f770c9b4f98372ac778 config: ia64-allmodconfig (attached as .config) compiler: ia64-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/63e96b25ae609f5659a28132f77d353cc7ecbd84 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Oleksij-Rempel/mainline-ti-tsc2046-adc-driver/20210319-224746 git checkout 63e96b25ae609f5659a28132f77d353cc7ecbd84 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=ia64 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/spi/spi.h:10, from drivers/iio/adc/ti-tsc2046.c:11: drivers/iio/adc/ti-tsc2046.c: In function 'tsc2046_adc_probe': >> drivers/iio/adc/ti-tsc2046.c:621:16: warning: format '%d' expects argument of type 'int', but argument 4 has type 'long unsigned int' [-Wformat=] 621 | dev_err(dev, "SPI max_speed_hz is too high: %d Hz. Max supported freq is %d Hz\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:621:3: note: in expansion of macro 'dev_err' 621 | dev_err(dev, "SPI max_speed_hz is too high: %d Hz. Max supported freq is %d Hz\n", | ^~~~~~~ drivers/iio/adc/ti-tsc2046.c:621:77: note: format string is defined here 621 | dev_err(dev, "SPI max_speed_hz is too high: %d Hz. Max supported freq is %d Hz\n", | ~^ | | | int | %ld Kconfig warnings: (for reference only) WARNING: unmet direct dependencies detected for FRAME_POINTER Depends on DEBUG_KERNEL && (M68K || UML || SUPERH) || ARCH_WANT_FRAME_POINTERS Selected by - FAULT_INJECTION_STACKTRACE_FILTER && FAULT_INJECTION_DEBUG_FS && STACKTRACE_SUPPORT && !X86_64 && !MIPS && !PPC && !S390 && !MICROBLAZE && !ARM && !ARC && !X86 vim +621 drivers/iio/adc/ti-tsc2046.c 609 610 static int tsc2046_adc_probe(struct spi_device *spi) 611 { 612 const struct tsc2046_adc_dcfg *dcfg; 613 struct device *dev = &spi->dev; 614 struct tsc2046_adc_priv *priv; 615 struct iio_dev *indio_dev; 616 struct iio_trigger *trig; 617 const char *name; 618 int ret; 619 620 if (spi->max_speed_hz > TI_TSC2046_MAX_CLK_FREQ) { > 621 dev_err(dev, "SPI max_speed_hz is too high: %d Hz. Max supported freq is %d Hz\n", 622 spi->max_speed_hz, TI_TSC2046_MAX_CLK_FREQ); 623 return -EINVAL; 624 } 625 626 dcfg = device_get_match_data(dev); 627 if (!dcfg) 628 return -EINVAL; 629 630 spi->bits_per_word = 8; 631 spi->mode &= ~SPI_MODE_X_MASK; 632 spi->mode |= SPI_MODE_0; 633 ret = spi_setup(spi); 634 if (ret < 0) 635 return dev_err_probe(dev, ret, "Error in spi setup\n"); 636 637 indio_dev = devm_iio_device_alloc(dev, sizeof(*priv)); 638 if (!indio_dev) 639 return -ENOMEM; 640 641 priv = iio_priv(indio_dev); 642 priv->dcfg = dcfg; 643 644 spi_set_drvdata(spi, indio_dev); 645 646 priv->spi = spi; 647 648 name = devm_kasprintf(dev, GFP_KERNEL, "%s-%s", 649 TI_TSC2046_NAME, dev_name(dev)); 650 651 indio_dev->name = name; 652 indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_TRIGGERED; 653 indio_dev->channels = dcfg->channels; 654 indio_dev->num_channels = dcfg->num_channels; 655 indio_dev->info = &tsc2046_adc_info; 656 657 tsc2046_adc_parse_fwnode(priv); 658 659 ret = tsc2046_adc_setup_spi_msg(priv); 660 if (ret) 661 return ret; 662 663 mutex_init(&priv->slock); 664 665 /* TODO: remove IRQ_NOAUTOEN after needed patches are mainline */ 666 irq_set_status_flags(spi->irq, IRQ_NOAUTOEN); 667 ret = devm_request_irq(dev, spi->irq, &tsc2046_adc_irq, 668 0, name, indio_dev); 669 if (ret) 670 return ret; 671 672 trig = devm_iio_trigger_alloc(dev, "touchscreen-%s", indio_dev->name); 673 if (!trig) 674 return -ENOMEM; 675 676 priv->trig = trig; 677 trig->dev.parent = indio_dev->dev.parent; 678 iio_trigger_set_drvdata(trig, indio_dev); 679 trig->ops = &tsc2046_adc_trigger_ops; 680 681 spin_lock_init(&priv->trig_lock); 682 hrtimer_init(&priv->trig_timer, CLOCK_MONOTONIC, 683 HRTIMER_MODE_REL_SOFT); 684 priv->trig_timer.function = tsc2046_adc_trig_more; 685 686 ret = devm_iio_trigger_register(dev, trig); 687 if (ret) { 688 dev_err(dev, "failed to register trigger\n"); 689 return ret; 690 } 691 692 ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL, 693 &tsc2046_adc_trigger_handler, NULL); 694 if (ret) { 695 dev_err(dev, "Failed to setup triggered buffer\n"); 696 return ret; 697 } 698 699 /* set default trigger */ 700 indio_dev->trig = iio_trigger_get(priv->trig); 701 702 ret = devm_iio_device_register(dev, indio_dev); 703 if (ret) { 704 dev_err(dev, "Failed to register iio device\n"); 705 return ret; 706 } 707 708 return 0; 709 } 710 --- 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: 63789 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 3/3] iio: adc: add ADC driver for the TI TSC2046 controller 2021-03-19 14:45 ` [PATCH v3 3/3] iio: adc: add ADC driver for the " Oleksij Rempel 2021-03-19 16:35 ` kernel test robot @ 2021-03-19 17:42 ` Andy Shevchenko 2021-03-22 10:30 ` Oleksij Rempel 2021-03-20 15:46 ` Jonathan Cameron 2 siblings, 1 reply; 14+ messages in thread From: Andy Shevchenko @ 2021-03-19 17:42 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 19, 2021 at 4:45 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 an IIO ADC device, we can > make use of resistive-adc-touch and iio-hwmon drivers. > > So far, this driver was tested with a custom version of resistive-adc-touch driver, > since it needs to be extended to make use of Z1 and Z2 channels. The X/Y > are working without additional changes. Since kbuild bot found some issues and it will be v4, some additional comments from me below. ... > +#define TI_TSC2046_SAMPLE_BITS \ > + (sizeof(struct tsc2046_adc_atom) * BITS_PER_BYTE) Isn't it something like BITS_PER_TYPE(struct ...) ? ... > +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; filed -> field in both cases above. ... > + /* > + * Lock to protect the layout and the spi transfer buffer. SPI > + * 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. > + */ ... > +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); Does it survive 32-bit builds? > + 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; > +} ... > + /* > + * 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; Can be ternary on one line, but it's up to you. ... > +static u16 tsc2046_adc_get_value(struct tsc2046_adc_atom *buf) > +{ > + /* Last 3 bits on the wire are empty */ Last?! You meant Least significant? Also, don't we lose precision if a new compatible chip appears that does fill those bits? Perhaps define the constant and put a comment why it's like this. > + return get_unaligned_be16(&buf->data) >> 3; > +} ... > +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; > + } I guess you may easily refactor this by supplying a pointer to the current layout + current size. > + cur = &priv->l[group]; Also, can you move it down closer to the (single?) caller. > +} ... > +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)); One line? > + return ret; > + } > + ret = iio_push_to_buffers_with_timestamp(indio_dev, &priv->scan_buf, > + iio_get_time_ns(indio_dev)); > + /* If consumer is kfifo, we may get a EBUSY here - ignore it. */ the consumer > + if (ret < 0 && ret != -EBUSY) { > + dev_err_ratelimited(dev, "Failed to push scan buffer %pe\n", > + ERR_PTR(ret)); > + > + return ret; > + } > + > + return 0; > +} ... > +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); You already have a spin lock, do you need to use the atomic API? > + iio_trigger_poll(priv->trig); > + > + spin_unlock_irqrestore(&priv->trig_lock, flags); > + > + return HRTIMER_NORESTART; > +} ... > + size_t size = 0; Move the assignment closer to the actual use of the variable. ... > + /* > + * In case SPI controller do not report effective_speed_hz, use > + * configure value and hope it will match Missed period. > + */ > + if (!priv->effective_speed_hz) > + priv->effective_speed_hz = priv->spi->max_speed_hz; Also can be ternary on one line, but it's up to you. ... > + name = devm_kasprintf(dev, GFP_KERNEL, "%s-%s", > + TI_TSC2046_NAME, dev_name(dev)); No NULL check? Should be added or justified. ... > + trig->dev.parent = indio_dev->dev.parent; Don't we have this done by core (some recent patches in upstream)? ... > + ret = devm_iio_device_register(dev, indio_dev); > + if (ret) { > + dev_err(dev, "Failed to register iio device\n"); > + return ret; > + } > + > + return 0; return ret; or even return devm_iio_device_register(dev, indio_dev); -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 3/3] iio: adc: add ADC driver for the TI TSC2046 controller 2021-03-19 17:42 ` Andy Shevchenko @ 2021-03-22 10:30 ` Oleksij Rempel 2021-03-22 13:41 ` Andy Shevchenko 0 siblings, 1 reply; 14+ messages in thread From: Oleksij Rempel @ 2021-03-22 10:30 UTC (permalink / raw) To: Andy Shevchenko 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 19, 2021 at 07:42:41PM +0200, Andy Shevchenko wrote: > On Fri, Mar 19, 2021 at 4:45 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 an IIO ADC device, we can > > make use of resistive-adc-touch and iio-hwmon drivers. > > > > So far, this driver was tested with a custom version of resistive-adc-touch driver, > > since it needs to be extended to make use of Z1 and Z2 channels. The X/Y > > are working without additional changes. > > Since kbuild bot found some issues and it will be v4, some additional > comments from me below. > ... > > > +#define TI_TSC2046_SAMPLE_BITS \ > > + (sizeof(struct tsc2046_adc_atom) * BITS_PER_BYTE) > > Isn't it something like BITS_PER_TYPE(struct ...) ? sounds good. > ... > > > +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; > > filed -> field in both cases above. done > ... > > > + /* > > + * Lock to protect the layout and the spi transfer buffer. > > SPI > > > + * 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. > > + */ > > ... > > > +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); > > Does it survive 32-bit builds? ACK. I develop and test it on 32bit ARM > > + 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; > > +} > > ... > > > + /* > > + * 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; > > Can be ternary on one line, but it's up to you. ACK. I'll keep it > ... > > > +static u16 tsc2046_adc_get_value(struct tsc2046_adc_atom *buf) > > +{ > > + /* Last 3 bits on the wire are empty */ > > Last?! You meant Least significant? ACK. LSB > Also, don't we lose precision if a new compatible chip appears that > does fill those bits? ACK. All of controllers supported by this driver: drivers/input/touchscreen/ads7846.c: - ti,tsc2046 - ti,ads7843 - ti,ads7845 - ti,ads7846 - ti,ads7873 (hm, there is no ti,ads7873, is it actually analog devices AD7873?) support 8- or 12-bit resolution. Only 12 bit mode is supported by this driver. It is possible that some one can produce a resistive touchscreen controller based on X > 12bit ADC, but this will probably not increase precision of this construction (there is a lot of noise any ways...). With other words, it is possible, but not probably that some one will really do it. > Perhaps define the constant and put a comment why it's like this. > > > + return get_unaligned_be16(&buf->data) >> 3; > > +} > > ... > > > +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; > > + } > > I guess you may easily refactor this by supplying a pointer to the > current layout + current size. Sure, but this will not make code more readable and it will not affect the performance. Are there any other reason to do it? Just to make one line instead of two? > > > + cur = &priv->l[group]; > > Also, can you move it down closer to the (single?) caller. > > > +} > > ... > > > +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)); > > One line? it will exceed the 80 char rule > > + return ret; > > + } > > > + ret = iio_push_to_buffers_with_timestamp(indio_dev, &priv->scan_buf, > > + iio_get_time_ns(indio_dev)); > > + /* If consumer is kfifo, we may get a EBUSY here - ignore it. */ > > the consumer > > > + if (ret < 0 && ret != -EBUSY) { > > + dev_err_ratelimited(dev, "Failed to push scan buffer %pe\n", > > + ERR_PTR(ret)); > > + > > + return ret; > > + } > > + > > + return 0; > > +} > > > ... > > > +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); > > You already have a spin lock, do you need to use the atomic API? I can only pass review comment from my other driver: Memory locations that are concurrently accessed needs to be marked as such, otherwise the compiler is allowed to funky stuff: https://lore.kernel.org/lkml/CAGzjT4ez+gWr3BFQsEr-wma+vs6UZNJ+mRARx_BWoAKEJSsN=w@mail.gmail.com/ And here is one more link: https://lwn.net/Articles/793253/#How%20Real%20Is%20All%20This? Starting with commit 62e8a3258bda atomic API is using READ/WRITE_ONCE, so I assume, I do nothing wrong by using it. Correct? > > + iio_trigger_poll(priv->trig); > > + > > + spin_unlock_irqrestore(&priv->trig_lock, flags); > > + > > + return HRTIMER_NORESTART; > > +} > > ... > > > + size_t size = 0; > > Move the assignment closer to the actual use of the variable. > > ... > > > + /* > > + * In case SPI controller do not report effective_speed_hz, use > > + * configure value and hope it will match > > Missed period. > > > + */ > > + if (!priv->effective_speed_hz) > > + priv->effective_speed_hz = priv->spi->max_speed_hz; > > Also can be ternary on one line, but it's up to you. it is better readable for me. > ... > > > + name = devm_kasprintf(dev, GFP_KERNEL, "%s-%s", > > + TI_TSC2046_NAME, dev_name(dev)); > > No NULL check? > Should be added or justified. name is set not optionally by the spi_add_device()->spi_dev_set_name() > ... > > > + trig->dev.parent = indio_dev->dev.parent; > > Don't we have this done by core (some recent patches in upstream)? can you please point to the code which is doing it? > ... > > > + ret = devm_iio_device_register(dev, indio_dev); > > + if (ret) { > > + dev_err(dev, "Failed to register iio device\n"); > > > + return ret; > > + } > > + > > + return 0; > > return ret; > or even > return devm_iio_device_register(dev, indio_dev); good point, regards, Oleksij -- 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] 14+ messages in thread
* Re: [PATCH v3 3/3] iio: adc: add ADC driver for the TI TSC2046 controller 2021-03-22 10:30 ` Oleksij Rempel @ 2021-03-22 13:41 ` Andy Shevchenko 2021-03-22 14:08 ` Oleksij Rempel 0 siblings, 1 reply; 14+ messages in thread From: Andy Shevchenko @ 2021-03-22 13:41 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 Mon, Mar 22, 2021 at 12:30 PM Oleksij Rempel <o.rempel@pengutronix.de> wrote: > On Fri, Mar 19, 2021 at 07:42:41PM +0200, Andy Shevchenko wrote: > > On Fri, Mar 19, 2021 at 4:45 PM Oleksij Rempel <o.rempel@pengutronix.de> wrote: ... > > > +static u16 tsc2046_adc_get_value(struct tsc2046_adc_atom *buf) > > > +{ > > > + /* Last 3 bits on the wire are empty */ > > > > Last?! You meant Least significant? > > ACK. LSB > > > Also, don't we lose precision if a new compatible chip appears that > > does fill those bits? > > ACK. All of controllers supported by this driver: > drivers/input/touchscreen/ads7846.c: > - ti,tsc2046 > - ti,ads7843 > - ti,ads7845 > - ti,ads7846 > - ti,ads7873 (hm, there is no ti,ads7873, is it actually analog devices AD7873?) > > support 8- or 12-bit resolution. Only 12 bit mode is supported by this > driver. It is possible that some one can produce a resistive touchscreen > controller based on X > 12bit ADC, but this will probably not increase precision > of this construction (there is a lot of noise any ways...). With other > words, it is possible, but not probably that some one will really do it. > > > Perhaps define the constant and put a comment why it's like this. Okay, and what happens here is something like cutting LSBs, but it sounds strange to me. If you get 16 bit values, the MSBs should not be used? So, a good comment is required to explain the logic behind. > > > + return get_unaligned_be16(&buf->data) >> 3; > > > +} ... > > > +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; > > > + } > > > > I guess you may easily refactor this by supplying a pointer to the > > current layout + current size. > > Sure, but this will not make code more readable and it will not affect > the performance. Are there any other reason to do it? Just to make one > line instead of two? It's still N - 1 unneeded checks and code is slightly harder to read. > > > + cur = &priv->l[group]; > > > > Also, can you move it down closer to the (single?) caller. > > > > > +} ... > > > + dev_err_ratelimited(dev, "SPI transfer filed: %pe\n", > > > + ERR_PTR(ret)); > > > > One line? > > it will exceed the 80 char rule It's fine here. ... > > > + spin_lock_irqsave(&priv->trig_lock, flags); > > > + > > > + disable_irq_nosync(priv->spi->irq); > > > > > + atomic_inc(&priv->trig_more_count); > > > > You already have a spin lock, do you need to use the atomic API? > > I can only pass review comment from my other driver: > Memory locations that are concurrently accessed needs to be > marked as such, otherwise the compiler is allowed to funky stuff: > https://lore.kernel.org/lkml/CAGzjT4ez+gWr3BFQsEr-wma+vs6UZNJ+mRARx_BWoAKEJSsN=w@mail.gmail.com/ > > And here is one more link: > https://lwn.net/Articles/793253/#How%20Real%20Is%20All%20This? > > Starting with commit 62e8a3258bda atomic API is using READ/WRITE_ONCE, > so I assume, I do nothing wrong by using it. Correct? Hmm... What I don't understand here is why you need a second level of atomicity. spin lock already makes this access atomic (at least I have checked couple of places and in both the variable is being accessed under spin lock). > > > + iio_trigger_poll(priv->trig); > > > + > > > + spin_unlock_irqrestore(&priv->trig_lock, flags); ... > > > + name = devm_kasprintf(dev, GFP_KERNEL, "%s-%s", > > > + TI_TSC2046_NAME, dev_name(dev)); > > > > No NULL check? > > Should be added or justified. > > name is set not optionally by the spi_add_device()->spi_dev_set_name() I didn't get it. You allocate memory and haven't checked against NULL. Why? If the name field is optional and having it's NULL is okay (non-fatal error), put a comment. ... > > > + trig->dev.parent = indio_dev->dev.parent; > > > > Don't we have this done by core (some recent patches in upstream)? > > can you please point to the code which is doing it? I believe it's this one: commit 970108185a0be29d1dbb25202d8c12d798e1c3a5 Author: Gwendal Grignou <gwendal@chromium.org> Date: Tue Mar 9 11:36:13 2021 -0800 iio: set default trig->dev.parent -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 3/3] iio: adc: add ADC driver for the TI TSC2046 controller 2021-03-22 13:41 ` Andy Shevchenko @ 2021-03-22 14:08 ` Oleksij Rempel 0 siblings, 0 replies; 14+ messages in thread From: Oleksij Rempel @ 2021-03-22 14:08 UTC (permalink / raw) To: Andy Shevchenko Cc: devicetree, Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio, Robin van der Gracht, Dmitry Torokhov, Linux Kernel Mailing List, Rob Herring, Pengutronix Kernel Team, David Jander, Jonathan Cameron On Mon, Mar 22, 2021 at 03:41:22PM +0200, Andy Shevchenko wrote: > On Mon, Mar 22, 2021 at 12:30 PM Oleksij Rempel <o.rempel@pengutronix.de> wrote: > > On Fri, Mar 19, 2021 at 07:42:41PM +0200, Andy Shevchenko wrote: > > > On Fri, Mar 19, 2021 at 4:45 PM Oleksij Rempel <o.rempel@pengutronix.de> wrote: > > ... > > > > > +static u16 tsc2046_adc_get_value(struct tsc2046_adc_atom *buf) > > > > +{ > > > > + /* Last 3 bits on the wire are empty */ > > > > > > Last?! You meant Least significant? > > > > ACK. LSB > > > > > Also, don't we lose precision if a new compatible chip appears that > > > does fill those bits? > > > > ACK. All of controllers supported by this driver: > > drivers/input/touchscreen/ads7846.c: > > - ti,tsc2046 > > - ti,ads7843 > > - ti,ads7845 > > - ti,ads7846 > > - ti,ads7873 (hm, there is no ti,ads7873, is it actually analog devices AD7873?) > > > > support 8- or 12-bit resolution. Only 12 bit mode is supported by this > > driver. It is possible that some one can produce a resistive touchscreen > > controller based on X > 12bit ADC, but this will probably not increase precision > > of this construction (there is a lot of noise any ways...). With other > > words, it is possible, but not probably that some one will really do it. > > > > > Perhaps define the constant and put a comment why it's like this. > > Okay, and what happens here is something like cutting LSBs, but it > sounds strange to me. If you get 16 bit values, the MSBs should not be > used? > > So, a good comment is required to explain the logic behind. > > > > > + return get_unaligned_be16(&buf->data) >> 3; > > > > +} > > ... > > > > > +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; > > > > + } > > > > > > I guess you may easily refactor this by supplying a pointer to the > > > current layout + current size. > > > > Sure, but this will not make code more readable and it will not affect > > the performance. Are there any other reason to do it? Just to make one > > line instead of two? > > It's still N - 1 unneeded checks and code is slightly harder to read. fixed > > > > + cur = &priv->l[group]; > > > > > > Also, can you move it down closer to the (single?) caller. > > > > > > > +} > > ... > > > > > + dev_err_ratelimited(dev, "SPI transfer filed: %pe\n", > > > > + ERR_PTR(ret)); > > > > > > One line? > > > > it will exceed the 80 char rule > > It's fine here. fixed > ... > > > > > + spin_lock_irqsave(&priv->trig_lock, flags); > > > > + > > > > + disable_irq_nosync(priv->spi->irq); > > > > > > > + atomic_inc(&priv->trig_more_count); > > > > > > You already have a spin lock, do you need to use the atomic API? > > > > I can only pass review comment from my other driver: > > Memory locations that are concurrently accessed needs to be > > marked as such, otherwise the compiler is allowed to funky stuff: > > https://lore.kernel.org/lkml/CAGzjT4ez+gWr3BFQsEr-wma+vs6UZNJ+mRARx_BWoAKEJSsN=w@mail.gmail.com/ > > > > And here is one more link: > > https://lwn.net/Articles/793253/#How%20Real%20Is%20All%20This? > > > > Starting with commit 62e8a3258bda atomic API is using READ/WRITE_ONCE, > > so I assume, I do nothing wrong by using it. Correct? > > Hmm... What I don't understand here is why you need a second level of > atomicity. spin lock already makes this access atomic (at least I have > checked couple of places and in both the variable is being accessed > under spin lock). fixed > > > > + iio_trigger_poll(priv->trig); > > > > + > > > > + spin_unlock_irqrestore(&priv->trig_lock, flags); > > ... > > > > > + name = devm_kasprintf(dev, GFP_KERNEL, "%s-%s", > > > > + TI_TSC2046_NAME, dev_name(dev)); > > > > > > No NULL check? > > > Should be added or justified. > > > > name is set not optionally by the spi_add_device()->spi_dev_set_name() > > I didn't get it. > You allocate memory and haven't checked against NULL. Why? > If the name field is optional and having it's NULL is okay (non-fatal > error), put a comment. ah... I missed the point. You was talking about name == NULL, i was thinking about dev_name(dev) == NULL. fixed > ... > > > > > + trig->dev.parent = indio_dev->dev.parent; > > > > > > Don't we have this done by core (some recent patches in upstream)? > > > > can you please point to the code which is doing it? > > I believe it's this one: > > commit 970108185a0be29d1dbb25202d8c12d798e1c3a5 > Author: Gwendal Grignou <gwendal@chromium.org> > Date: Tue Mar 9 11:36:13 2021 -0800 > > iio: set default trig->dev.parent ok, found it on iio/testing and rebased against it.. fixed Thank you, Oleksij -- 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] 14+ messages in thread
* Re: [PATCH v3 3/3] iio: adc: add ADC driver for the TI TSC2046 controller 2021-03-19 14:45 ` [PATCH v3 3/3] iio: adc: add ADC driver for the " Oleksij Rempel 2021-03-19 16:35 ` kernel test robot 2021-03-19 17:42 ` Andy Shevchenko @ 2021-03-20 15:46 ` Jonathan Cameron 2021-03-22 11:56 ` Oleksij Rempel 2 siblings, 1 reply; 14+ messages in thread From: Jonathan Cameron @ 2021-03-20 15:46 UTC (permalink / raw) To: Oleksij Rempel Cc: Rob Herring, Andy Shevchenko, devicetree, linux-kernel, Pengutronix Kernel Team, David Jander, Robin van der Gracht, linux-iio, Lars-Peter Clausen, Peter Meerwald-Stadler, Dmitry Torokhov On Fri, 19 Mar 2021 15:45:09 +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 an IIO ADC device, we can > make use of resistive-adc-touch and iio-hwmon drivers. > > So far, this driver was tested with a custom version of resistive-adc-touch driver, > since it needs 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> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> Not a lot to add to Andy's review. A few minor things inline. Biggest one is I think we should call out that we expect to add single channel polled reading + scales etc in future to enable the iio-hwmon usecase. Jonathan > --- > MAINTAINERS | 8 + > drivers/iio/adc/Kconfig | 12 + > drivers/iio/adc/Makefile | 1 + > drivers/iio/adc/ti-tsc2046.c | 728 +++++++++++++++++++++++++++++++++++ > 4 files changed, 749 insertions(+) > create mode 100644 drivers/iio/adc/ti-tsc2046.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 0455cfd5c76c..2ea85a5bb4dd 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -18002,6 +18002,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/ti-tsc2046.c b/drivers/iio/adc/ti-tsc2046.c > new file mode 100644 > index 000000000000..c8c0dd9087c5 > --- /dev/null > +++ b/drivers/iio/adc/ti-tsc2046.c > @@ -0,0 +1,728 @@ > +/* > + * The PENIRQ of TSC2046 controller is implemented as level shifter attached to > + * the X+ line. If voltage of the X+ line reaches a specific level the IRQ will > + * be activated or deactivated. > + * To make this kind of IRQ reusable as trigger following additions were > + * implemented: > + * - rate limiting: > + * For typical touchscreen use case, we need to trigger about each 10ms. > + * - hrtimer: > + * Continue triggering at least once after the IRQ was deactivated. Then > + * deactivate this trigger to stop sampling in order to reduce power > + * consumption. > + */ Good description :) ... > + > +struct tsc2046_adc_scan_buf { > + /* Scan data for each channel */ > + u16 data[TI_TSC2046_MAX_CHAN]; > + /* Timestamp */ > + s64 ts __aligned(8); > +}; ... > +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; Given the type tsc2046_adc_scan_buf is never used for anything else you could make things more compact by using struct { } 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, \ > + .datasheet_name = "#name", \ > + .scan_index = index, \ > + .scan_type = { \ > + .sign = 'u', \ > + .realbits = bits, \ > + .storagebits = 16, \ > + .endianness = IIO_CPU, \ > + }, \ > +} I'd not noticed this before but you are exposing nothing to the normal IIO interfaces. That means for usecases like iio-hwmon there is no access to polled readings, or information like channel scaling. I suppose that could be added later, but perhaps you want to call this out by mentioning it in the patch description. > + > +#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), > +}; > + Hmm. Flexibility that isn't yet used. Normally I'm pretty resistant to this going in, unless I'm reassured that there is support for additional devices already in the pipeline. Is that true here? Otherwise this is basically unneeded complexity. ... Jonathan ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 3/3] iio: adc: add ADC driver for the TI TSC2046 controller 2021-03-20 15:46 ` Jonathan Cameron @ 2021-03-22 11:56 ` Oleksij Rempel 2021-03-22 14:27 ` Jonathan Cameron 0 siblings, 1 reply; 14+ messages in thread From: Oleksij Rempel @ 2021-03-22 11:56 UTC (permalink / raw) To: Jonathan Cameron Cc: devicetree, Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio, Robin van der Gracht, Dmitry Torokhov, linux-kernel, Andy Shevchenko, Rob Herring, Pengutronix Kernel Team, David Jander Hi, On Sat, Mar 20, 2021 at 03:46:01PM +0000, Jonathan Cameron wrote: > On Fri, 19 Mar 2021 15:45:09 +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 an IIO ADC device, we can > > make use of resistive-adc-touch and iio-hwmon drivers. > > > > So far, this driver was tested with a custom version of resistive-adc-touch driver, > > since it needs 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> > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > Not a lot to add to Andy's review. A few minor things inline. > Biggest one is I think we should call out that we expect to add single > channel polled reading + scales etc in future to enable the iio-hwmon > usecase. > > Jonathan > > > --- > > MAINTAINERS | 8 + > > drivers/iio/adc/Kconfig | 12 + > > drivers/iio/adc/Makefile | 1 + > > drivers/iio/adc/ti-tsc2046.c | 728 +++++++++++++++++++++++++++++++++++ > > 4 files changed, 749 insertions(+) > > create mode 100644 drivers/iio/adc/ti-tsc2046.c > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 0455cfd5c76c..2ea85a5bb4dd 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -18002,6 +18002,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/ti-tsc2046.c b/drivers/iio/adc/ti-tsc2046.c > > new file mode 100644 > > index 000000000000..c8c0dd9087c5 > > --- /dev/null > > +++ b/drivers/iio/adc/ti-tsc2046.c > > @@ -0,0 +1,728 @@ > > +/* > > + * The PENIRQ of TSC2046 controller is implemented as level shifter attached to > > + * the X+ line. If voltage of the X+ line reaches a specific level the IRQ will > > + * be activated or deactivated. > > + * To make this kind of IRQ reusable as trigger following additions were > > + * implemented: > > + * - rate limiting: > > + * For typical touchscreen use case, we need to trigger about each 10ms. > > + * - hrtimer: > > + * Continue triggering at least once after the IRQ was deactivated. Then > > + * deactivate this trigger to stop sampling in order to reduce power > > + * consumption. > > + */ > > Good description :) > > ... > > > > + > > +struct tsc2046_adc_scan_buf { > > + /* Scan data for each channel */ > > + u16 data[TI_TSC2046_MAX_CHAN]; > > + /* Timestamp */ > > + s64 ts __aligned(8); > > +}; > > ... > > > +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; > > Given the type tsc2046_adc_scan_buf is never used for anything else > you could make things more compact by using > struct { > } 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, \ > > + .datasheet_name = "#name", \ > > + .scan_index = index, \ > > + .scan_type = { \ > > + .sign = 'u', \ > > + .realbits = bits, \ > > + .storagebits = 16, \ > > + .endianness = IIO_CPU, \ > > + }, \ > > +} > > I'd not noticed this before but you are exposing nothing to the > normal IIO interfaces. > > That means for usecases like iio-hwmon there is no access > to polled readings, or information like channel scaling. > > I suppose that could be added later, but perhaps you want to call this > out by mentioning it in the patch description. If it is ok for you, then I'll add some words about it in to the patch description. > > + > > +#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), > > +}; > > + > > Hmm. Flexibility that isn't yet used. Normally I'm pretty resistant > to this going in, unless I'm reassured that there is support for additional > devices already in the pipeline. Is that true here? Otherwise > this is basically unneeded complexity. In the long term this driver should replace drivers/input/touchscreen/ads7846.c This driver supports ti,ads7843, ti,ads7845, ti,ads7846.. at least with following number of supported channels: ti,ads7843 - 4 channels: x, y, aux0, aux1 ti,ads7845 - 3 channels: x, y, aux0 ti,ads7846 - 8 channels... Currently I don't have this HW for testing and there a subtle differences that have to be taken care of and tested. -- 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] 14+ messages in thread
* Re: [PATCH v3 3/3] iio: adc: add ADC driver for the TI TSC2046 controller 2021-03-22 11:56 ` Oleksij Rempel @ 2021-03-22 14:27 ` Jonathan Cameron 2021-03-29 7:38 ` Oleksij Rempel 0 siblings, 1 reply; 14+ messages in thread From: Jonathan Cameron @ 2021-03-22 14:27 UTC (permalink / raw) To: Oleksij Rempel Cc: Jonathan Cameron, devicetree, Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio, Robin van der Gracht, Dmitry Torokhov, linux-kernel, Andy Shevchenko, Rob Herring, Pengutronix Kernel Team, David Jander > > > > > + /* > > > + * 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, \ > > > + .datasheet_name = "#name", \ > > > + .scan_index = index, \ > > > + .scan_type = { \ > > > + .sign = 'u', \ > > > + .realbits = bits, \ > > > + .storagebits = 16, \ > > > + .endianness = IIO_CPU, \ > > > + }, \ > > > +} > > > > I'd not noticed this before but you are exposing nothing to the > > normal IIO interfaces. > > > > That means for usecases like iio-hwmon there is no access > > to polled readings, or information like channel scaling. > > > > I suppose that could be added later, but perhaps you want to call this > > out by mentioning it in the patch description. > > If it is ok for you, then I'll add some words about it in to the patch > description. Sure > > > > + > > > +#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), > > > +}; > > > + > > > > Hmm. Flexibility that isn't yet used. Normally I'm pretty resistant > > to this going in, unless I'm reassured that there is support for additional > > devices already in the pipeline. Is that true here? Otherwise > > this is basically unneeded complexity. > > In the long term this driver should replace > drivers/input/touchscreen/ads7846.c > > This driver supports ti,ads7843, ti,ads7845, ti,ads7846.. at least with > following number of supported channels: > ti,ads7843 - 4 channels: x, y, aux0, aux1 > ti,ads7845 - 3 channels: x, y, aux0 > ti,ads7846 - 8 channels... > > Currently I don't have this HW for testing and there a subtle > differences that have to be taken care of and tested. > Note that I'm only going to merge this driver with an explicit statement from Dmitry as input maintainer that he is fine with this approach. Jonathan ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 3/3] iio: adc: add ADC driver for the TI TSC2046 controller 2021-03-22 14:27 ` Jonathan Cameron @ 2021-03-29 7:38 ` Oleksij Rempel 2021-03-29 10:23 ` Jonathan Cameron 0 siblings, 1 reply; 14+ messages in thread From: Oleksij Rempel @ 2021-03-29 7:38 UTC (permalink / raw) To: Jonathan Cameron Cc: Jonathan Cameron, devicetree, Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio, Robin van der Gracht, Dmitry Torokhov, linux-kernel, Andy Shevchenko, Rob Herring, Pengutronix Kernel Team, David Jander On Mon, Mar 22, 2021 at 02:27:22PM +0000, Jonathan Cameron wrote: > > > > +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), > > > > +}; > > > > + > > > > > > Hmm. Flexibility that isn't yet used. Normally I'm pretty resistant > > > to this going in, unless I'm reassured that there is support for additional > > > devices already in the pipeline. Is that true here? Otherwise > > > this is basically unneeded complexity. > > > > In the long term this driver should replace > > drivers/input/touchscreen/ads7846.c > > > > This driver supports ti,ads7843, ti,ads7845, ti,ads7846.. at least with > > following number of supported channels: > > ti,ads7843 - 4 channels: x, y, aux0, aux1 > > ti,ads7845 - 3 channels: x, y, aux0 > > ti,ads7846 - 8 channels... > > > > Currently I don't have this HW for testing and there a subtle > > differences that have to be taken care of and tested. > > > > Note that I'm only going to merge this driver with an explicit statement > from Dmitry as input maintainer that he is fine with this approach. Since there is still no Dmitry's feedback please take a look to the ti,ads7843 datasheet: https://www.ti.com/lit/ds/symlink/ads7843.pdf On the page 1 you can see, that this device has two general purpose ADC inputs IN3, IN4. If some one will decide to mainline support for this pins, will end with IIO ADC driver any way :) Regards, Oleksij -- 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] 14+ messages in thread
* Re: [PATCH v3 3/3] iio: adc: add ADC driver for the TI TSC2046 controller 2021-03-29 7:38 ` Oleksij Rempel @ 2021-03-29 10:23 ` Jonathan Cameron 0 siblings, 0 replies; 14+ messages in thread From: Jonathan Cameron @ 2021-03-29 10:23 UTC (permalink / raw) To: Oleksij Rempel Cc: Jonathan Cameron, devicetree, Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio, Robin van der Gracht, Dmitry Torokhov, linux-kernel, Andy Shevchenko, Rob Herring, Pengutronix Kernel Team, David Jander On Mon, 29 Mar 2021 09:38:38 +0200 Oleksij Rempel <o.rempel@pengutronix.de> wrote: > On Mon, Mar 22, 2021 at 02:27:22PM +0000, Jonathan Cameron wrote: > > > > > +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), > > > > > +}; > > > > > + > > > > > > > > Hmm. Flexibility that isn't yet used. Normally I'm pretty resistant > > > > to this going in, unless I'm reassured that there is support for additional > > > > devices already in the pipeline. Is that true here? Otherwise > > > > this is basically unneeded complexity. > > > > > > In the long term this driver should replace > > > drivers/input/touchscreen/ads7846.c > > > > > > This driver supports ti,ads7843, ti,ads7845, ti,ads7846.. at least with > > > following number of supported channels: > > > ti,ads7843 - 4 channels: x, y, aux0, aux1 > > > ti,ads7845 - 3 channels: x, y, aux0 > > > ti,ads7846 - 8 channels... > > > > > > Currently I don't have this HW for testing and there a subtle > > > differences that have to be taken care of and tested. > > > > > > > Note that I'm only going to merge this driver with an explicit statement > > from Dmitry as input maintainer that he is fine with this approach. > > Since there is still no Dmitry's feedback please take a look to the > ti,ads7843 datasheet: > https://www.ti.com/lit/ds/symlink/ads7843.pdf > > On the page 1 you can see, that this device has two general purpose ADC > inputs IN3, IN4. If some one will decide to mainline support for this > pins, will end with IIO ADC driver any way :) Understood, but I'm still going to want Dmitry to give an opinion on this. That might take a little while though as he may well be busy! Jonathan > > Regards, > Oleksij ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-03-29 10:24 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-19 14:45 [PATCH v3 0/3] mainline ti tsc2046 adc driver Oleksij Rempel 2021-03-19 14:45 ` [PATCH v3 1/3] dt-bindings:iio:adc: add generic settling-time-us and oversampling-ratio channel properties Oleksij Rempel 2021-03-19 14:45 ` [PATCH v3 2/3] dt-bindings:iio:adc: add documentation for TI TSC2046 controller Oleksij Rempel 2021-03-19 14:45 ` [PATCH v3 3/3] iio: adc: add ADC driver for the " Oleksij Rempel 2021-03-19 16:35 ` kernel test robot 2021-03-19 17:42 ` Andy Shevchenko 2021-03-22 10:30 ` Oleksij Rempel 2021-03-22 13:41 ` Andy Shevchenko 2021-03-22 14:08 ` Oleksij Rempel 2021-03-20 15:46 ` Jonathan Cameron 2021-03-22 11:56 ` Oleksij Rempel 2021-03-22 14:27 ` Jonathan Cameron 2021-03-29 7:38 ` Oleksij Rempel 2021-03-29 10:23 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).