From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-183.mta1.migadu.com (out-183.mta1.migadu.com [95.215.58.183]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 634883680AA for ; Tue, 26 Aug 2025 19:11:23 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=95.215.58.183 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1756235485; cv=none; b=BwY6ZDcV0iSKj/9Hu3c27f5H+JDLSyRjdY0qkQ8aI+3TfxBeIG+tKy4Q2roSbuVmRjpkgL+bHpn9b3jPl3AvNkMRPzCyNK9R1PNtJFn1T4zZl35reKr6r5ee25fDdmZQd9kpNKsTm+IZ+/4dgl1IwJ9LgXuGhfoibvBXLAqZyfs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1756235485; c=relaxed/simple; bh=Uhdzc8EEUYuWN+cZjWFk5mNDLydwMtF2JLNcSZ5LYgA=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=a5Em4HZNmbVErD/wIL6jmRMvQrbRLD6GiB8QfYuOznzpiedS1QWSoERgyavAcRRlQc7HWhUKonlrVzIyGTgGLzZI1Cu8eLGDb35OK2YOnXeF7qm8f3ud8bh8pXSoFm2Fn6LEWnPprmgo0M9uFezwgqLZEGS5zpy/kn/jUruZOAw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=kernel.org; spf=pass smtp.mailfrom=linux.dev; arc=none smtp.client-ip=95.215.58.183 Authentication-Results: smtp.subspace.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=kernel.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Date: Tue, 26 Aug 2025 15:11:14 -0400 X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Ben Collins To: David Lechner Cc: Jonathan Cameron , Nuno =?utf-8?B?U8Oh?= , Andy Shevchenko , Antoniu Miclaus , Lars-Peter Clausen , Michael Hennerich , linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v7 5/5] iio: ad4080: Rework filter_type "none" logic Message-ID: <2025082615-striped-crocodile-9cb63f@boujee-and-buff> Mail-Followup-To: David Lechner , Jonathan Cameron , Nuno =?utf-8?B?U8Oh?= , Andy Shevchenko , Antoniu Miclaus , Lars-Peter Clausen , Michael Hennerich , linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org References: <20250825-mcp9600-iir-v7-0-2ba676a52589@kernel.org> <20250825-mcp9600-iir-v7-5-2ba676a52589@kernel.org> <0f515c35-b2ac-4d23-b8bb-77151ed2eb9f@baylibre.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0f515c35-b2ac-4d23-b8bb-77151ed2eb9f@baylibre.com> X-Migadu-Flow: FLOW_OUT On Tue, Aug 26, 2025 at 11:51:56AM -0500, David Lechner wrote: > On 8/25/25 7:10 PM, Ben Collins wrote: > > The filter_type logic for "none" needed to be reworked to be more > > general. > > > > As documented, return IIO_VAL_EMPTY for sampling rates in "none" type > > and EINVAL when there's an attempt to write a rate for "none" type. > > This patch breaks usespace, which is something we always must avoid. I was under the impression there was a need to make the use of filter_type "none" more consistent. I don't disagree with not breaking userspace, but it does create ambiguity for other implementations. > > > > Signed-off-by: Ben Collins > > --- > > drivers/iio/adc/ad4080.c | 23 ++++++++++------------- > > 1 file changed, 10 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/iio/adc/ad4080.c b/drivers/iio/adc/ad4080.c > > index 6e61787ed3213fe4332bd92b938a7a717dada99f..c7408b9703731ee5d4229a85ffa91ea64b233cd9 100644 > > --- a/drivers/iio/adc/ad4080.c > > +++ b/drivers/iio/adc/ad4080.c > > @@ -154,8 +154,6 @@ static const int ad4080_dec_rate_avail[] = { > > 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, > > }; > > > > -static const int ad4080_dec_rate_none[] = { 1 }; > > - > > static const char * const ad4080_power_supplies[] = { > > "vdd33", "vdd11", "vddldo", "iovdd", "vrefin", > > }; > > @@ -268,13 +266,13 @@ static int ad4080_read_raw(struct iio_dev *indio_dev, > > *val = st->clk_rate; > > return IIO_VAL_INT; > > case IIO_CHAN_INFO_OVERSAMPLING_RATIO: > > - if (st->filter_type == FILTER_NONE) { > > - *val = 1; > > - } else { > > - *val = ad4080_get_dec_rate(indio_dev, chan); > > - if (*val < 0) > > - return *val; > > - } > > + if (st->filter_type == FILTER_NONE) > > + return IIO_VAL_EMPTY; > > + > > + *val = ad4080_get_dec_rate(indio_dev, chan); > > + if (*val < 0) > > + return *val; > > + > > return IIO_VAL_INT; > > default: > > return -EINVAL; > > @@ -289,7 +287,7 @@ static int ad4080_write_raw(struct iio_dev *indio_dev, > > > > switch (mask) { > > case IIO_CHAN_INFO_OVERSAMPLING_RATIO: > > - if (st->filter_type == FILTER_NONE && val > 1) > > + if (st->filter_type == FILTER_NONE) > > return -EINVAL; > > > > return ad4080_set_dec_rate(indio_dev, chan, val); > > @@ -376,17 +374,16 @@ static int ad4080_read_avail(struct iio_dev *indio_dev, > > case IIO_CHAN_INFO_OVERSAMPLING_RATIO: > > switch (st->filter_type) { > > case FILTER_NONE: > > - *vals = ad4080_dec_rate_none; > > - *length = ARRAY_SIZE(ad4080_dec_rate_none); > > + *type = IIO_VAL_EMPTY; > > break; > > default: > > *vals = ad4080_dec_rate_avail; > > *length = st->filter_type >= SINC_5 ? > > (ARRAY_SIZE(ad4080_dec_rate_avail) - 2) : > > ARRAY_SIZE(ad4080_dec_rate_avail); > > + *type = IIO_VAL_INT; > > break; > > } > > - *type = IIO_VAL_INT; > > return IIO_AVAIL_LIST; > > default: > > return -EINVAL; > > > > Returning a value of 1 for the oversampling ratio when there is no > oversampling going on is perfectly reasonable and mathematically correct. > So I don't consider this change an improvement. -- Ben Collins https://libjwt.io https://github.com/benmcollins -- 3EC9 7598 1672 961A 1139 173A 5D5A 57C7 242B 22CF