From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.5 required=3.0 tests=BAYES_00,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_2 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7CF3BC432BE for ; Mon, 30 Aug 2021 10:47:38 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5FDFD61102 for ; Mon, 30 Aug 2021 10:47:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236425AbhH3Ksa (ORCPT ); Mon, 30 Aug 2021 06:48:30 -0400 Received: from mail.kernel.org ([198.145.29.99]:44734 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236408AbhH3Ks3 (ORCPT ); Mon, 30 Aug 2021 06:48:29 -0400 Received: from jic23-huawei (cpc108967-cmbg20-2-0-cust86.5-4.cable.virginm.net [81.101.6.87]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id ED69E610F9; Mon, 30 Aug 2021 10:47:33 +0000 (UTC) Date: Mon, 30 Aug 2021 11:50:46 +0100 From: Jonathan Cameron To: Miquel Raynal Cc: Lars-Peter Clausen , Thomas Petazzoni , linux-iio@vger.kernel.org, Subject: Re: [PATCH 15/16] iio: adc: max1027: Support software triggers Message-ID: <20210830115046.3727ccc4@jic23-huawei> In-Reply-To: <20210818111139.330636-16-miquel.raynal@bootlin.com> References: <20210818111139.330636-1-miquel.raynal@bootlin.com> <20210818111139.330636-16-miquel.raynal@bootlin.com> X-Mailer: Claws Mail 4.0.0 (GTK+ 3.24.30; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 18 Aug 2021 13:11:38 +0200 Miquel Raynal wrote: > 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. As mentioned earlier, this is not how I'd normally expect this sort of case to be handled. I'd be expecting the cnvst trigger to still be calling this function and the function to do the relevant check to ensure it knows the data is already available in that case. > 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 > --- > 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); I'd not expect to see this ever time. The configuration shouldn't change from one call of this function to the next. > + if (ret) > + goto out; > + > + ret = max1027_configure_chans_to_scan(indio_dev); This should also not change unless it is also responsible for the 'go' signal. If that's true then it is badly named. > + if (ret) > + goto out; > + > + ret = max1027_wait_eoc(indio_dev); > + if (ret) > + goto out; > + > ret = max1027_read_scan(indio_dev); > + > +out: > if (ret) > dev_err(&indio_dev->dev, > "Cannot read scanned values (%d)\n", ret); > @@ -529,7 +532,6 @@ static const struct iio_trigger_ops max1027_trigger_ops = { > > static const struct iio_info max1027_info = { > .read_raw = &max1027_read_raw, > - .validate_trigger = &max1027_validate_trigger, > .debugfs_reg_access = &max1027_debugfs_reg_access, > }; >