Add read_raw() support to make use of iio_hwmon and other iio clients.
Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/iio/adc/ti-tsc2046.c | 114 ++++++++++++++++++++++++++++++++---
1 file changed, 106 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/adc/ti-tsc2046.c b/drivers/iio/adc/ti-tsc2046.c
index 8126084616e6..55787d18e2cd 100644
--- a/drivers/iio/adc/ti-tsc2046.c
+++ b/drivers/iio/adc/ti-tsc2046.c
@@ -82,6 +82,7 @@
#define TI_TSC2046_DATA_12BIT GENMASK(14, 3)
#define TI_TSC2046_MAX_CHAN 8
+#define TI_TSC2046_INT_VREF 2500
/* Represents a HW sample */
struct tsc2046_adc_atom {
@@ -178,6 +179,11 @@ struct tsc2046_adc_priv {
.type = IIO_VOLTAGE, \
.indexed = 1, \
.channel = index, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) | \
+ BIT(IIO_CHAN_INFO_DEBOUNCE_COUNT) | \
+ BIT(IIO_CHAN_INFO_DEBOUNCE_TIME), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
.datasheet_name = "#name", \
.scan_index = index, \
.scan_type = { \
@@ -241,6 +247,14 @@ static u8 tsc2046_adc_get_cmd(struct tsc2046_adc_priv *priv, int ch_idx,
else
pd = 0;
+ switch (ch_idx) {
+ case TI_TSC2046_ADDR_TEMP1:
+ case TI_TSC2046_ADDR_AUX:
+ case TI_TSC2046_ADDR_VBAT:
+ case TI_TSC2046_ADDR_TEMP0:
+ pd |= TI_TSC2046_SER | TI_TSC2046_PD1_VREF_ON;
+ }
+
return TI_TSC2046_START | FIELD_PREP(TI_TSC2046_ADDR, ch_idx) | pd;
}
@@ -252,16 +266,47 @@ static u16 tsc2046_adc_get_value(struct tsc2046_adc_atom *buf)
static int tsc2046_adc_read_one(struct tsc2046_adc_priv *priv, int ch_idx,
u32 *effective_speed_hz)
{
+ struct tsc2046_adc_ch_cfg *ch = &priv->ch_cfg[ch_idx];
+ struct tsc2046_adc_atom *rx_buf, *tx_buf;
+ unsigned int val, val_normalized = 0;
+ int ret, i, count_skip = 0, max_count;
struct spi_transfer xfer;
struct spi_message msg;
- int ret;
+ u8 cmd;
+
+ if (!effective_speed_hz) {
+ count_skip = tsc2046_adc_time_to_count(priv, ch->settling_time_us);
+ max_count = count_skip + ch->oversampling_ratio;
+ } else {
+ max_count = 1;
+ }
+
+ tx_buf = kcalloc(max_count, sizeof(*tx_buf), GFP_KERNEL);
+ if (!tx_buf)
+ return -ENOMEM;
+
+ rx_buf = kcalloc(max_count, sizeof(*rx_buf), GFP_KERNEL);
+ if (!rx_buf) {
+ ret = -ENOMEM;
+ goto free_tx;
+ }
+
+ /*
+ * 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 < max_count - 1; i++)
+ tx_buf[i].cmd = cmd;
+
+ /* automatically power down on last sample */
+ tx_buf[i].cmd = tsc2046_adc_get_cmd(priv, ch_idx, false);
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);
+ xfer.tx_buf = tx_buf;
+ xfer.rx_buf = rx_buf;
+ xfer.len = sizeof(*tx_buf) * max_count;
spi_message_init_with_transfers(&msg, &xfer, 1);
/*
@@ -272,13 +317,25 @@ static int tsc2046_adc_read_one(struct tsc2046_adc_priv *priv, int ch_idx,
if (ret) {
dev_err_ratelimited(&priv->spi->dev, "SPI transfer failed %pe\n",
ERR_PTR(ret));
- return ret;
+ goto free_bufs;
}
if (effective_speed_hz)
*effective_speed_hz = xfer.effective_speed_hz;
- return tsc2046_adc_get_value(priv->rx_one);
+ for (i = 0; i < max_count - count_skip; i++) {
+ val = tsc2046_adc_get_value(&rx_buf[count_skip + i]);
+ val_normalized += val;
+ }
+
+ ret = DIV_ROUND_UP(val_normalized, max_count - count_skip);
+
+free_bufs:
+ kfree(rx_buf);
+free_tx:
+ kfree(tx_buf);
+
+ return ret;
}
static size_t tsc2046_adc_group_set_layout(struct tsc2046_adc_priv *priv,
@@ -385,6 +442,46 @@ static irqreturn_t tsc2046_adc_trigger_handler(int irq, void *p)
return IRQ_HANDLED;
}
+static int tsc2046_adc_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long m)
+{
+ struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
+ int ret;
+
+ switch (m) {
+ case IIO_CHAN_INFO_RAW:
+ ret = tsc2046_adc_read_one(priv, chan->channel, NULL);
+ if (ret < 0)
+ return ret;
+
+ *val = ret;
+
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ /*
+ * Note: the TSC2046 has internal voltage divider on the VBAT
+ * line. This divider can be influenced by external divider.
+ * So, it is better to use external voltage-divider.
+ */
+ *val = TI_TSC2046_INT_VREF;
+ *val2 = chan->scan_type.realbits;
+ return IIO_VAL_FRACTIONAL_LOG2;
+ case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+ *val = priv->ch_cfg[chan->channel].oversampling_ratio;
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_DEBOUNCE_COUNT:
+ *val = tsc2046_adc_time_to_count(priv,
+ priv->ch_cfg[chan->channel].settling_time_us);
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_DEBOUNCE_TIME:
+ *val = priv->ch_cfg[chan->channel].settling_time_us;
+ return IIO_VAL_INT;
+ }
+
+ return -EINVAL;
+}
+
static int tsc2046_adc_update_scan_mode(struct iio_dev *indio_dev,
const unsigned long *active_scan_mask)
{
@@ -415,6 +512,7 @@ static int tsc2046_adc_update_scan_mode(struct iio_dev *indio_dev,
}
static const struct iio_info tsc2046_adc_info = {
+ .read_raw = tsc2046_adc_read_raw,
.update_scan_mode = tsc2046_adc_update_scan_mode,
};
--
2.30.2
On Fri, 7 Jan 2022 10:35:27 +0100
Oleksij Rempel <[email protected]> wrote:
> Add read_raw() support to make use of iio_hwmon and other iio clients.
>
> Signed-off-by: Oleksij Rempel <[email protected]>
Hi Oleksij
Main questions in here are around settling time and the interface used for that.
> ---
> drivers/iio/adc/ti-tsc2046.c | 114 ++++++++++++++++++++++++++++++++---
> 1 file changed, 106 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iio/adc/ti-tsc2046.c b/drivers/iio/adc/ti-tsc2046.c
> index 8126084616e6..55787d18e2cd 100644
> --- a/drivers/iio/adc/ti-tsc2046.c
> +++ b/drivers/iio/adc/ti-tsc2046.c
> @@ -82,6 +82,7 @@
> #define TI_TSC2046_DATA_12BIT GENMASK(14, 3)
>
> #define TI_TSC2046_MAX_CHAN 8
> +#define TI_TSC2046_INT_VREF 2500
>
> /* Represents a HW sample */
> struct tsc2046_adc_atom {
> @@ -178,6 +179,11 @@ struct tsc2046_adc_priv {
> .type = IIO_VOLTAGE, \
> .indexed = 1, \
> .channel = index, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) | \
> + BIT(IIO_CHAN_INFO_DEBOUNCE_COUNT) | \
> + BIT(IIO_CHAN_INFO_DEBOUNCE_TIME), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> .datasheet_name = "#name", \
> .scan_index = index, \
> .scan_type = { \
> @@ -241,6 +247,14 @@ static u8 tsc2046_adc_get_cmd(struct tsc2046_adc_priv *priv, int ch_idx,
> else
> pd = 0;
>
> + switch (ch_idx) {
> + case TI_TSC2046_ADDR_TEMP1:
> + case TI_TSC2046_ADDR_AUX:
> + case TI_TSC2046_ADDR_VBAT:
> + case TI_TSC2046_ADDR_TEMP0:
> + pd |= TI_TSC2046_SER | TI_TSC2046_PD1_VREF_ON;
> + }
> +
> return TI_TSC2046_START | FIELD_PREP(TI_TSC2046_ADDR, ch_idx) | pd;
> }
>
> @@ -252,16 +266,47 @@ static u16 tsc2046_adc_get_value(struct tsc2046_adc_atom *buf)
> static int tsc2046_adc_read_one(struct tsc2046_adc_priv *priv, int ch_idx,
> u32 *effective_speed_hz)
> {
> + struct tsc2046_adc_ch_cfg *ch = &priv->ch_cfg[ch_idx];
> + struct tsc2046_adc_atom *rx_buf, *tx_buf;
> + unsigned int val, val_normalized = 0;
> + int ret, i, count_skip = 0, max_count;
> struct spi_transfer xfer;
> struct spi_message msg;
> - int ret;
> + u8 cmd;
> +
> + if (!effective_speed_hz) {
> + count_skip = tsc2046_adc_time_to_count(priv, ch->settling_time_us);
> + max_count = count_skip + ch->oversampling_ratio;
> + } else {
> + max_count = 1;
> + }
> +
> + tx_buf = kcalloc(max_count, sizeof(*tx_buf), GFP_KERNEL);
> + if (!tx_buf)
> + return -ENOMEM;
> +
> + rx_buf = kcalloc(max_count, sizeof(*rx_buf), GFP_KERNEL);
> + if (!rx_buf) {
> + ret = -ENOMEM;
> + goto free_tx;
> + }
I guess these are fine to do everytime because you expect this to be used in
paths which aren't called at a particularly high frequency?
These buffers could get rather large so maybe you need a cap on settling time?
> +
> + /*
> + * 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 < max_count - 1; i++)
> + tx_buf[i].cmd = cmd;
> +
> + /* automatically power down on last sample */
> + tx_buf[i].cmd = tsc2046_adc_get_cmd(priv, ch_idx, false);
>
> 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;
Are these used for anything else? If not probably need to drop them and
their allocation.
> - xfer.len = sizeof(*priv->tx_one);
> + xfer.tx_buf = tx_buf;
> + xfer.rx_buf = rx_buf;
> + xfer.len = sizeof(*tx_buf) * max_count;
This could be very big and more than possible some spi controllers will fail
it (or does the SPI core handle splitting very large transfers?) Maybe a loop
is needed with smaller fixed size transfers?
> spi_message_init_with_transfers(&msg, &xfer, 1);
>
> /*
> @@ -272,13 +317,25 @@ static int tsc2046_adc_read_one(struct tsc2046_adc_priv *priv, int ch_idx,
> if (ret) {
> dev_err_ratelimited(&priv->spi->dev, "SPI transfer failed %pe\n",
> ERR_PTR(ret));
> - return ret;
> + goto free_bufs;
> }
>
> if (effective_speed_hz)
> *effective_speed_hz = xfer.effective_speed_hz;
>
> - return tsc2046_adc_get_value(priv->rx_one);
> + for (i = 0; i < max_count - count_skip; i++) {
> + val = tsc2046_adc_get_value(&rx_buf[count_skip + i]);
> + val_normalized += val;
> + }
> +
> + ret = DIV_ROUND_UP(val_normalized, max_count - count_skip);
> +
> +free_bufs:
> + kfree(rx_buf);
> +free_tx:
> + kfree(tx_buf);
> +
> + return ret;
> }
>
> static size_t tsc2046_adc_group_set_layout(struct tsc2046_adc_priv *priv,
> @@ -385,6 +442,46 @@ static irqreturn_t tsc2046_adc_trigger_handler(int irq, void *p)
> return IRQ_HANDLED;
> }
>
> +static int tsc2046_adc_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long m)
> +{
> + struct tsc2046_adc_priv *priv = iio_priv(indio_dev);
> + int ret;
> +
> + switch (m) {
> + case IIO_CHAN_INFO_RAW:
> + ret = tsc2046_adc_read_one(priv, chan->channel, NULL);
> + if (ret < 0)
> + return ret;
> +
> + *val = ret;
> +
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
> + /*
> + * Note: the TSC2046 has internal voltage divider on the VBAT
> + * line. This divider can be influenced by external divider.
> + * So, it is better to use external voltage-divider.
> + */
> + *val = TI_TSC2046_INT_VREF;
> + *val2 = chan->scan_type.realbits;
> + return IIO_VAL_FRACTIONAL_LOG2;
> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> + *val = priv->ch_cfg[chan->channel].oversampling_ratio;
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_DEBOUNCE_COUNT:
These are unusual. I think they've only been used for the more literal bounce suppression
of a human step counting algorithm.
I'd probably not expect to see the both even if we decide this is applicable.
> + *val = tsc2046_adc_time_to_count(priv,
> + priv->ch_cfg[chan->channel].settling_time_us);
Setting time is often about external circuitry so it's a bit unusual to expose
it to userspace rather than making it a device tree property and just making
sure the driver doesn't provide a reading until appropriate debounce has passed.
Here is coming from DT anyway, so what benefit do these two read only channel
properties provide?
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_DEBOUNCE_TIME:
> + *val = priv->ch_cfg[chan->channel].settling_time_us;
> + return IIO_VAL_INT;
> + }
> +
> + return -EINVAL;
> +}
> +
> static int tsc2046_adc_update_scan_mode(struct iio_dev *indio_dev,
> const unsigned long *active_scan_mask)
> {
> @@ -415,6 +512,7 @@ static int tsc2046_adc_update_scan_mode(struct iio_dev *indio_dev,
> }
>
> static const struct iio_info tsc2046_adc_info = {
> + .read_raw = tsc2046_adc_read_raw,
> .update_scan_mode = tsc2046_adc_update_scan_mode,
> };
>
Hi Jonathan,
On Sun, Jan 09, 2022 at 04:00:09PM +0000, Jonathan Cameron wrote:
> On Fri, 7 Jan 2022 10:35:27 +0100
> Oleksij Rempel <[email protected]> wrote:
>
> > Add read_raw() support to make use of iio_hwmon and other iio clients.
> >
> > Signed-off-by: Oleksij Rempel <[email protected]>
> Hi Oleksij
>
> Main questions in here are around settling time and the interface used for that.
>
> > ---
> > drivers/iio/adc/ti-tsc2046.c | 114 ++++++++++++++++++++++++++++++++---
> > 1 file changed, 106 insertions(+), 8 deletions(-)
> >
> > @@ -252,16 +266,47 @@ static u16 tsc2046_adc_get_value(struct tsc2046_adc_atom *buf)
> > static int tsc2046_adc_read_one(struct tsc2046_adc_priv *priv, int ch_idx,
> > u32 *effective_speed_hz)
> > {
> > + struct tsc2046_adc_ch_cfg *ch = &priv->ch_cfg[ch_idx];
> > + struct tsc2046_adc_atom *rx_buf, *tx_buf;
> > + unsigned int val, val_normalized = 0;
> > + int ret, i, count_skip = 0, max_count;
> > struct spi_transfer xfer;
> > struct spi_message msg;
> > - int ret;
> > + u8 cmd;
> > +
> > + if (!effective_speed_hz) {
> > + count_skip = tsc2046_adc_time_to_count(priv, ch->settling_time_us);
> > + max_count = count_skip + ch->oversampling_ratio;
> > + } else {
> > + max_count = 1;
> > + }
> > +
> > + tx_buf = kcalloc(max_count, sizeof(*tx_buf), GFP_KERNEL);
> > + if (!tx_buf)
> > + return -ENOMEM;
> > +
> > + rx_buf = kcalloc(max_count, sizeof(*rx_buf), GFP_KERNEL);
> > + if (!rx_buf) {
> > + ret = -ENOMEM;
> > + goto free_tx;
> > + }
>
> I guess these are fine to do everytime because you expect this to be used in
> paths which aren't called at a particularly high frequency?
Yes, this was my assumption as well. Instead of preallocating buffer of
max size, I hope it is less ugly.
> These buffers could get rather large so maybe you need a cap on settling time?
What do you mean by "cap on settling"?
>
> > +
> > + /*
> > + * 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 < max_count - 1; i++)
> > + tx_buf[i].cmd = cmd;
> > +
> > + /* automatically power down on last sample */
> > + tx_buf[i].cmd = tsc2046_adc_get_cmd(priv, ch_idx, false);
> >
> > 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;
>
> Are these used for anything else? If not probably need to drop them and
> their allocation.
done
> > - xfer.len = sizeof(*priv->tx_one);
> > + xfer.tx_buf = tx_buf;
> > + xfer.rx_buf = rx_buf;
> > + xfer.len = sizeof(*tx_buf) * max_count;
>
> This could be very big and more than possible some spi controllers will fail
> it (or does the SPI core handle splitting very large transfers?) Maybe a loop
> is needed with smaller fixed size transfers?
I can't exclude possible issue with some of SPI drivers. But SPI level
optimizations should be done on SPI driver or framework level.
> > spi_message_init_with_transfers(&msg, &xfer, 1);
> >
> > /*
> > @@ -272,13 +317,25 @@ static int tsc2046_adc_read_one(struct tsc2046_adc_priv *priv, int ch_idx,
> > if (ret) {
> > dev_err_ratelimited(&priv->spi->dev, "SPI transfer failed %pe\n",
> > ERR_PTR(ret));
> > + *val2 = chan->scan_type.realbits;
> > + return IIO_VAL_FRACTIONAL_LOG2;
> > + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> > + *val = priv->ch_cfg[chan->channel].oversampling_ratio;
> > + return IIO_VAL_INT;
> > + case IIO_CHAN_INFO_DEBOUNCE_COUNT:
>
> These are unusual. I think they've only been used for the more literal bounce suppression
> of a human step counting algorithm.
>
> I'd probably not expect to see the both even if we decide this is applicable.
Ok, i do not need this information so far. I'll remove it
> > + *val = tsc2046_adc_time_to_count(priv,
> > + priv->ch_cfg[chan->channel].settling_time_us);
>
> Setting time is often about external circuitry so it's a bit unusual to expose
> it to userspace rather than making it a device tree property and just making
> sure the driver doesn't provide a reading until appropriate debounce has passed.
> Here is coming from DT anyway, so what benefit do these two read only channel
> properties provide?
No benefit. Will remove it.
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 |
On Tue, 11 Jan 2022 14:18:48 +0100
Oleksij Rempel <[email protected]> wrote:
> Hi Jonathan,
>
> On Sun, Jan 09, 2022 at 04:00:09PM +0000, Jonathan Cameron wrote:
> > On Fri, 7 Jan 2022 10:35:27 +0100
> > Oleksij Rempel <[email protected]> wrote:
> >
> > > Add read_raw() support to make use of iio_hwmon and other iio clients.
> > >
> > > Signed-off-by: Oleksij Rempel <[email protected]>
> > Hi Oleksij
> >
> > Main questions in here are around settling time and the interface used for that.
> >
> > > ---
> > > drivers/iio/adc/ti-tsc2046.c | 114 ++++++++++++++++++++++++++++++++---
> > > 1 file changed, 106 insertions(+), 8 deletions(-)
> > >
> > > @@ -252,16 +266,47 @@ static u16 tsc2046_adc_get_value(struct tsc2046_adc_atom *buf)
> > > static int tsc2046_adc_read_one(struct tsc2046_adc_priv *priv, int ch_idx,
> > > u32 *effective_speed_hz)
> > > {
> > > + struct tsc2046_adc_ch_cfg *ch = &priv->ch_cfg[ch_idx];
> > > + struct tsc2046_adc_atom *rx_buf, *tx_buf;
> > > + unsigned int val, val_normalized = 0;
> > > + int ret, i, count_skip = 0, max_count;
> > > struct spi_transfer xfer;
> > > struct spi_message msg;
> > > - int ret;
> > > + u8 cmd;
> > > +
> > > + if (!effective_speed_hz) {
> > > + count_skip = tsc2046_adc_time_to_count(priv, ch->settling_time_us);
> > > + max_count = count_skip + ch->oversampling_ratio;
> > > + } else {
> > > + max_count = 1;
> > > + }
> > > +
> > > + tx_buf = kcalloc(max_count, sizeof(*tx_buf), GFP_KERNEL);
> > > + if (!tx_buf)
> > > + return -ENOMEM;
> > > +
> > > + rx_buf = kcalloc(max_count, sizeof(*rx_buf), GFP_KERNEL);
> > > + if (!rx_buf) {
> > > + ret = -ENOMEM;
> > > + goto free_tx;
> > > + }
> >
> > I guess these are fine to do everytime because you expect this to be used in
> > paths which aren't called at a particularly high frequency?
>
> Yes, this was my assumption as well. Instead of preallocating buffer of
> max size, I hope it is less ugly.
>
> > These buffers could get rather large so maybe you need a cap on settling time?
>
> What do you mean by "cap on settling"?
In theory the buffer needed could get very large, so perhap set a maximum reasonable
size (1 page perhaps) and report an error if the settling time is too large to fit
in that space.
>
> >
> > > +
> > > + /*
> > > + * 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 < max_count - 1; i++)
> > > + tx_buf[i].cmd = cmd;
> > > +
> > > + /* automatically power down on last sample */
> > > + tx_buf[i].cmd = tsc2046_adc_get_cmd(priv, ch_idx, false);
> > >
> > > 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;
> >
> > Are these used for anything else? If not probably need to drop them and
> > their allocation.
>
> done
>
> > > - xfer.len = sizeof(*priv->tx_one);
> > > + xfer.tx_buf = tx_buf;
> > > + xfer.rx_buf = rx_buf;
> > > + xfer.len = sizeof(*tx_buf) * max_count;
> >
> > This could be very big and more than possible some spi controllers will fail
> > it (or does the SPI core handle splitting very large transfers?) Maybe a loop
> > is needed with smaller fixed size transfers?
>
> I can't exclude possible issue with some of SPI drivers. But SPI level
> optimizations should be done on SPI driver or framework level.
As above, I think you want to set a reasonable limit otherwise it will fail
on an awful lot of hardware if someone sets a silly value...
>
> > > spi_message_init_with_transfers(&msg, &xfer, 1);
> > >
> > > /*
> > > @@ -272,13 +317,25 @@ static int tsc2046_adc_read_one(struct tsc2046_adc_priv *priv, int ch_idx,
> > > if (ret) {
> > > dev_err_ratelimited(&priv->spi->dev, "SPI transfer failed %pe\n",
> > > ERR_PTR(ret));
> > > + *val2 = chan->scan_type.realbits;
> > > + return IIO_VAL_FRACTIONAL_LOG2;
> > > + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> > > + *val = priv->ch_cfg[chan->channel].oversampling_ratio;
> > > + return IIO_VAL_INT;
> > > + case IIO_CHAN_INFO_DEBOUNCE_COUNT:
> >
> > These are unusual. I think they've only been used for the more literal bounce suppression
> > of a human step counting algorithm.
> >
> > I'd probably not expect to see the both even if we decide this is applicable.
>
> Ok, i do not need this information so far. I'll remove it
>
> > > + *val = tsc2046_adc_time_to_count(priv,
> > > + priv->ch_cfg[chan->channel].settling_time_us);
> >
> > Setting time is often about external circuitry so it's a bit unusual to expose
> > it to userspace rather than making it a device tree property and just making
> > sure the driver doesn't provide a reading until appropriate debounce has passed.
> > Here is coming from DT anyway, so what benefit do these two read only channel
> > properties provide?
>
> No benefit. Will remove it.
>
> Regards,
> Oleksij