LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Sa, Nuno" <Nuno.Sa@analog.com>
To: Miquel Raynal <miquel.raynal@bootlin.com>,
	Jonathan Cameron <jic23@kernel.org>,
	Lars-Peter Clausen <lars@metafoo.de>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	"linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH 15/16] iio: adc: max1027: Support software triggers
Date: Fri, 20 Aug 2021 07:58:25 +0000	[thread overview]
Message-ID: <SJ0PR03MB63594E162F7246874CD1A72599C19@SJ0PR03MB6359.namprd03.prod.outlook.com> (raw)
In-Reply-To: <20210818111139.330636-16-miquel.raynal@bootlin.com>



> -----Original Message-----
> From: Miquel Raynal <miquel.raynal@bootlin.com>
> Sent: Wednesday, August 18, 2021 1:12 PM
> To: Jonathan Cameron <jic23@kernel.org>; Lars-Peter Clausen
> <lars@metafoo.de>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>; linux-
> iio@vger.kernel.org; linux-kernel@vger.kernel.org; Miquel Raynal
> <miquel.raynal@bootlin.com>
> Subject: [PATCH 15/16] iio: adc: max1027: Support software triggers
> 
> [External]
> 
> Now that max1027_trigger_handler() has been freed from handling
> hardware
> triggers EOC situations, we can use it for what it has been designed in
> the first place: trigger software originated conversions. In other
> words, when userspace initiates a conversion with a sysfs trigger or a
> hrtimer trigger, we must do all configuration steps, ie:
> 1- Configuring the trigger
> 2- Configuring the channels to scan
> 3- Starting the conversion (actually done automatically by step 2 in
>    this case)
> 4- Waiting for the conversion to end
> 5- Retrieving the data from the ADC
> 6- Push the data to the IIO core and notify it
> 
> Add the missing steps to this helper and drop the trigger verification
> hook otherwise software triggers would simply not be accepted at all.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/iio/adc/max1027.c | 26 ++++++++++++++------------
>  1 file changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/iio/adc/max1027.c b/drivers/iio/adc/max1027.c
> index 8c5995ae59f2..bb437e43adaf 100644
> --- a/drivers/iio/adc/max1027.c
> +++ b/drivers/iio/adc/max1027.c
> @@ -413,17 +413,6 @@ static int max1027_debugfs_reg_access(struct
> iio_dev *indio_dev,
>  	return spi_write(st->spi, val, 1);
>  }
> 
> -static int max1027_validate_trigger(struct iio_dev *indio_dev,
> -				    struct iio_trigger *trig)
> -{
> -	struct max1027_state *st = iio_priv(indio_dev);
> -
> -	if (st->trig != trig)
> -		return -EINVAL;
> -
> -	return 0;
> -}
> -
>  static int max1027_set_cnvst_trigger_state(struct iio_trigger *trig,
> bool state)
>  {
>  	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
> @@ -512,7 +501,21 @@ static irqreturn_t max1027_trigger_handler(int
> irq, void *private)
> 
>  	pr_debug("%s(irq=%d, private=0x%p)\n", __func__, irq,
> private);
> 
> +	ret = max1027_configure_trigger(indio_dev);
> +	if (ret)
> +		goto out;
> +
> +	ret = max1027_configure_chans_to_scan(indio_dev);
> +	if (ret)
> +		goto out;
> +
> +	ret = max1027_wait_eoc(indio_dev);
> +	if (ret)
> +		goto out;
> +
>  	ret = max1027_read_scan(indio_dev);

There's something that I'm not getting... How are we checking that
we have software triggers? This API is called only if the device
allocates it's own trigger which will happen if there's a spi IRQ.

I'm probably missing something as this series is fairly big but the way
I would do it is (in the probe):

- always call 'devm_iio_triggered_buffer_setup()' function and properly use
buffer ops [1] (for example, you can use 'validate_scan_mask()' to setup the
channels to read);  
- only allocate a trigger if an IRQ is present in which case, we assume HW 
triggering is supported.

Does this makes sense?

- Nuno Sá



  reply	other threads:[~2021-08-20  7:58 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-18 11:11 [PATCH 00/16] Bring software triggers support to MAX1027-like ADCs Miquel Raynal
2021-08-18 11:11 ` [PATCH 01/16] iio: adc: max1027: Fix wrong shift with 12-bit devices Miquel Raynal
2021-08-20  7:02   ` Sa, Nuno
2021-08-30  9:57     ` Jonathan Cameron
2021-08-18 11:11 ` [PATCH 02/16] iio: adc: max1027: Fix the number of max1X31 channels Miquel Raynal
2021-08-20  7:03   ` Sa, Nuno
2021-08-30 10:00     ` Jonathan Cameron
2021-08-18 11:11 ` [PATCH 03/16] iio: adc: max1027: Push only the requested samples Miquel Raynal
2021-08-20  7:10   ` Sa, Nuno
2021-08-30 10:07     ` Jonathan Cameron
2021-08-30 10:49       ` Sa, Nuno
2021-08-30 14:29         ` Jonathan Cameron
2021-08-30 15:02           ` Sa, Nuno
2021-09-01  8:12             ` Miquel Raynal
2021-09-04 14:06               ` Jonathan Cameron
2021-09-06  8:59               ` Sa, Nuno
2021-09-06 16:56                 ` Jonathan Cameron
2021-09-06 17:34                   ` Miquel Raynal
2021-08-30 10:06   ` Jonathan Cameron
2021-08-18 11:11 ` [PATCH 04/16] iio: adc: max1027: Lower conversion time Miquel Raynal
2021-08-20  7:12   ` Sa, Nuno
2021-08-30 10:10   ` Jonathan Cameron
2021-08-18 11:11 ` [PATCH 05/16] iio: adc: max1027: Drop extra warning message Miquel Raynal
2021-08-20  7:12   ` Sa, Nuno
2021-08-30 10:12   ` Jonathan Cameron
2021-08-18 11:11 ` [PATCH 06/16] iio: adc: max1027: Rename a helper Miquel Raynal
2021-08-20  7:13   ` Sa, Nuno
2021-08-18 11:11 ` [PATCH 07/16] iio: adc: max1027: Create a helper to configure the trigger Miquel Raynal
2021-08-20  7:16   ` Sa, Nuno
2021-08-30 10:16   ` Jonathan Cameron
2021-08-18 11:11 ` [PATCH 08/16] iio: adc: max1027: Explain better how the trigger state gets changed Miquel Raynal
2021-08-20  7:17   ` Sa, Nuno
2021-08-18 11:11 ` [PATCH 09/16] iio: adc: max1027: Create a helper to configure the channels to scan Miquel Raynal
2021-08-20  7:18   ` Sa, Nuno
2021-08-18 11:11 ` [PATCH 10/16] iio: adc: max1027: Prevent single channel accesses during buffer reads Miquel Raynal
2021-08-20  7:20   ` Sa, Nuno
2021-08-20  7:30     ` Sa, Nuno
2021-08-30 10:20       ` Jonathan Cameron
2021-09-02  8:56         ` Miquel Raynal
2021-08-18 11:11 ` [PATCH 11/16] iio: adc: max1027: Separate the IRQ handler from the read logic Miquel Raynal
2021-08-20  7:23   ` Sa, Nuno
2021-09-02  8:55     ` Miquel Raynal
2021-09-04 14:08       ` Jonathan Cameron
2021-08-18 11:11 ` [PATCH 12/16] iio: adc: max1027: Introduce an end of conversion helper Miquel Raynal
2021-08-20  7:28   ` Sa, Nuno
2021-09-02  9:26     ` Miquel Raynal
2021-08-30 10:34   ` Jonathan Cameron
2021-08-18 11:11 ` [PATCH 13/16] iio: adc: max1027: Prepare re-using the EOC interrupt Miquel Raynal
2021-08-20  7:31   ` Sa, Nuno
2021-08-30 10:30   ` Jonathan Cameron
2021-08-30 10:47   ` Jonathan Cameron
2021-08-18 11:11 ` [PATCH 14/16] iio: adc: max1027: Consolidate the end of conversion helper Miquel Raynal
2021-08-20  7:45   ` Sa, Nuno
2021-08-30 10:37   ` Jonathan Cameron
2021-08-30 12:44     ` Sa, Nuno
2021-08-30 14:32       ` Jonathan Cameron
2021-09-02 15:12       ` Miquel Raynal
2021-09-03 14:28         ` Sa, Nuno
2021-09-03 14:46           ` Miquel Raynal
2021-09-05  9:41             ` Jonathan Cameron
2021-09-06  9:12               ` Sa, Nuno
2021-09-06  9:30                 ` Sa, Nuno
2021-08-18 11:11 ` [PATCH 15/16] iio: adc: max1027: Support software triggers Miquel Raynal
2021-08-20  7:58   ` Sa, Nuno [this message]
2021-09-02 12:25     ` Miquel Raynal
2021-09-03 14:20       ` Sa, Nuno
2021-08-30 10:50   ` Jonathan Cameron
2021-09-02 15:21     ` Miquel Raynal
2021-09-05  9:43       ` Jonathan Cameron
2021-08-18 11:11 ` [PATCH 16/16] iio: adc: max1027: Enable software triggers to be used without IRQ Miquel Raynal
2021-08-30 10:54   ` Jonathan Cameron

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=SJ0PR03MB63594E162F7246874CD1A72599C19@SJ0PR03MB6359.namprd03.prod.outlook.com \
    --to=nuno.sa@analog.com \
    --cc=jic23@kernel.org \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miquel.raynal@bootlin.com \
    --cc=thomas.petazzoni@bootlin.com \
    --subject='RE: [PATCH 15/16] iio: adc: max1027: Support software triggers' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).