From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-oo1-f42.google.com (mail-oo1-f42.google.com [209.85.161.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7F41B3B27F7 for ; Mon, 23 Mar 2026 14:39:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.161.42 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774276789; cv=none; b=H/K7j6qn8s5l8ScA0up3uCawz9m5PIoURNMC7pYKA9v5S2T36N1RFDlJjMfRzg54tW93mAOPhfe8fpB/haihfLmgbZqu07boeNErDhvLWTRqOtuWgmQpsAvycib1iS2QwB4nMD3+P2gIXMSyNFJjR+y+JLaj3qEFhOwKqBTShu8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774276789; c=relaxed/simple; bh=/Vc04mp+YvGzxnIgYBmQ5LjwPSfRG5KdQa8ysQAw3eE=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=nxRL4mAnpaXKk1t2VsXCWE+pR3lwq09FO46M2bj0uyqWhaNHYM1lw56Jpx2X4WNGJtQCWyDShrn+W0HcJWItbWQXh0Pg57lLxJURfpbF4ak1K6r4J8KJflTcatJDUiiG+/bpDULfANcMn+FGxII58Smwt1Pb3loQDnk3WJYdmXY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=baylibre.com; spf=pass smtp.mailfrom=baylibre.com; dkim=pass (2048-bit key) header.d=baylibre-com.20230601.gappssmtp.com header.i=@baylibre-com.20230601.gappssmtp.com header.b=muhWkSEE; arc=none smtp.client-ip=209.85.161.42 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=baylibre.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=baylibre.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=baylibre-com.20230601.gappssmtp.com header.i=@baylibre-com.20230601.gappssmtp.com header.b="muhWkSEE" Received: by mail-oo1-f42.google.com with SMTP id 006d021491bc7-67c2b70124cso1575001eaf.1 for ; Mon, 23 Mar 2026 07:39:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20230601.gappssmtp.com; s=20230601; t=1774276786; x=1774881586; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=9JeALHOS5PAPURwj6AYWeAG7dI7olikWsM+DOIkL5mk=; b=muhWkSEEOIr0r1HABPGLoDEiNYbOGYBkiqu3R9H0q/8y9dIbXK5R0ru+2mkMPr5oGS Ctl11GOTGM+0aq7Wk4IrDrqQOO0Q5b8CaKWT99ziIzWJeBw6MNkQNtEeXc465JkYlNdK WVatuNlhS6lGwLigm4APXJRe1Q9FxLiBWVZl/Ac29CykT7MOBBcNxUCaZzTt9XgF6HW2 c2Ys7ntNSFyFl1BqiTNIx251F+5P7h81ajm19LVQkxKGiWXP7b8xkM7Wx8Y5EClOym8V kvu+wEkOKbXH5d0YnL+tteJzRwtoZotR2TSu+bgVrFVUCMnmLjf9s5PlM1fysG8/RruH /18Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1774276786; x=1774881586; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=9JeALHOS5PAPURwj6AYWeAG7dI7olikWsM+DOIkL5mk=; b=MGGct23KIc6yAG0FvZuNopKpZtsb7VNExI2IuSwFH2Ee83rBpW5bJOLMyU0FNHkSG1 /gq5VCG0CD+l0FyclJKx5VcKJ1hfnx8hZbTsrHYECTE/mKAOxIVFEIt+ZiBaNyWaogjQ 3Gw22f28MAOA+dKQpNoPGrkSMq0tujwdtq6vavZYy1FuSKrvXmQqh3VeuAaxvtxNo6J4 eUYBbak4ytiszeo69/EfFQRXsscNjF5wpuTppaunKh96ycklnKeB68V9uqzE8O982xZI lDDI2ZH6TeaG/lxCbJrVa/7XhtaOYtqUFJ2/gCM3pP0tt7ir7JjtfF2qW0T5Bucr/lr6 /UYA== X-Forwarded-Encrypted: i=1; AJvYcCUMDSX5Rx9ux8EZOLh5CsYraQYAmHRLZNqb4gtp/E0CcPvXJNbSbF99RP6u/cD7fvuV20kkWv+HNTuG5Q4=@vger.kernel.org X-Gm-Message-State: AOJu0Ywsn7yCiF21tW5gg5IGJh9oI7yvNd0uKb8eSJ/9KWH4GUjLabuK vg48wUZl9xd35sPK8tfWtdj56aO7O6X0AZLkBwpr+V2Ky2F7Nz3J5dXt3Crr5ePmEIcryTVpkb2 Gah7S X-Gm-Gg: ATEYQzygFd3vMhP5zZiWD0armAxpMXimJSYXMy+zt7Snr0Bv3dIDvU7LIACAwKzH9C6 yIdYvUVq+DtPGM7oEkvYugpewXRdUT6MhG7ruXQjRdfTxte+OEbSSvXrhlVtQUryC0MIalQOxFQ Z3Oe9fVqxVxCxq1PW3uZIjK1NuB1A8SrrQ0Vee9Z1kkFPKJfG0f/jgWyTQjNA3+NlGAZKctLik6 5x8CYuaj4w/V+CcohxQqyUPpJaUDvldbVH/pbFUsWTQ0r/fafmKJGEwCnNG25BZZFWhuOk46wiX Tu9nlaBMrs05bDiyiEGXLEeqWWt1FXcOsbfzciPeLsInHHp6n9cQi3gC0ioabIn9mg99unSe3ef gsy/3MrHiEirqgPmBVDk5KQcHKjdnTGKT8LOuIKcVQGOyK7+i5ZZRtkNf4Cjlp87KES+Hrh2vj9 FHim2eru8a81lk6Mr15d7NrvUO0VJ7D4IRBx6CnohpX0Bgf1EVZ1CiN1iW+cgTTVNO6jaxZW0= X-Received: by 2002:a05:6820:8ca:b0:67b:aca0:3d96 with SMTP id 006d021491bc7-67c22fc295bmr8670885eaf.65.1774276785962; Mon, 23 Mar 2026 07:39:45 -0700 (PDT) Received: from ?IPV6:2600:8803:e7e4:500:964:f712:dbc7:4119? ([2600:8803:e7e4:500:964:f712:dbc7:4119]) by smtp.gmail.com with ESMTPSA id 006d021491bc7-67c252c892dsm6680581eaf.4.2026.03.23.07.39.45 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 23 Mar 2026 07:39:45 -0700 (PDT) Message-ID: Date: Mon, 23 Mar 2026 09:39:44 -0500 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v5 3/4] iio: adc: ad799x: cache regulator voltages during probe To: Archit Anant , Jonathan Cameron Cc: lars@metafoo.de, Michael.Hennerich@analog.com, nuno.sa@analog.com, andy@kernel.org, linux-iio@vger.kernel.org, linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org References: <20260318092715.42538-1-architanant5@gmail.com> <20260318092715.42538-4-architanant5@gmail.com> <20260321182710.1621d1aa@jic23-huawei> Content-Language: en-US From: David Lechner In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 3/23/26 7:22 AM, Archit Anant wrote: > On Sat, Mar 21, 2026 at 11:57 PM Jonathan Cameron wrote: >> >> On Wed, 18 Mar 2026 14:57:14 +0530 >> Archit Anant wrote: >> >>> Reading the regulator voltage via regulator_get_voltage() can be a slow >>> operation. >> >> Whilst that might be true, it isn't a reason for this change. >> Sysfs reads that would cause it to be read are never a particularly >> fast path anyway. So drop this first sentence. >> >>> Since the reference voltages for this ADC are not expected to >>> change at runtime, it is inefficient to query the regulator API every >>> time userspace reads the IIO_CHAN_INFO_SCALE attribute. >>> >>> Determine the active reference voltage (either VREF or VCC) during >>> probe() and cache it in the state structure. This improves the >>> performance of ad799x_read_raw() and removes the dependency on the >>> regulator pointers during fast-path reads. >>> >>> Suggested-by: Jonathan Cameron >>> Suggested-by: David Lechner >>> Signed-off-by: Archit Anant >> >> A suggested alternative approach inline. >> >> Thanks, >> >> Jonathan >> >>> --- >>> drivers/iio/adc/ad799x.c | 24 ++++++++++++++++-------- >>> 1 file changed, 16 insertions(+), 8 deletions(-) >>> >>> diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c >>> index 7504bcf627da..ae2ad4bd37cc 100644 >>> --- a/drivers/iio/adc/ad799x.c >>> +++ b/drivers/iio/adc/ad799x.c >>> @@ -30,6 +30,7 @@ >>> #include >>> #include >>> #include >>> +#include >>> >>> #include >>> #include >>> @@ -135,6 +136,9 @@ struct ad799x_state { >>> u16 config; >>> >>> unsigned int transfer_size; >>> + >>> + int vref_uV; >>> + >>> IIO_DECLARE_BUFFER_WITH_TS(__be16, rx_buf, AD799X_MAX_CHANNELS); >>> }; >>> >>> @@ -302,14 +306,7 @@ static int ad799x_read_raw(struct iio_dev *indio_dev, >>> GENMASK(chan->scan_type.realbits - 1, 0); >>> return IIO_VAL_INT; >>> case IIO_CHAN_INFO_SCALE: >>> - if (st->vref) >>> - ret = regulator_get_voltage(st->vref); >>> - else >>> - ret = regulator_get_voltage(st->reg); >>> - >>> - if (ret < 0) >>> - return ret; >>> - *val = ret / 1000; >>> + *val = st->vref_uV / MILLI; >>> *val2 = chan->scan_type.realbits; >>> return IIO_VAL_FRACTIONAL_LOG2; >>> } >>> @@ -828,9 +825,20 @@ static int ad799x_probe(struct i2c_client *client) >>> ret = regulator_enable(st->vref); >>> if (ret) >>> goto error_disable_reg; >>> + ret = regulator_get_voltage(st->vref); >> >> For vref I don't think we need to keep the regulator around, so you should >> be able to use devm_regulator_get_enable_read_voltage() with checking >> for -ENODEV to identify it simply isn't there. >> >> It would need a tiny bit of reordering though or a custom >> devm_add_action_or_reset() registered callback to ensure that regulator >> disable for vcc happens in reverse sequence of what happens on setup. >> >> Anyone think there are actually ordering constraints on these regulators? >> Would be fairly unusual for this sort of device, but not impossible. >> If not, cleanest option might be; > ... >> Then no need to undo anything by hand in remove() and no need to keep >> a pointer to any regulators around for later. > > I completely agree that devm_regulator_get_enable_read_voltage() is > the cleanest approach. > > However, as I noted briefly in the v5 changelog (which I should have > highlighted better), the driver currently relies on those regulator > pointers (st->reg and st->vref) in the ad799x_suspend() and > ad799x_resume() callbacks. > > If we drop the pointers from the state structure, we lose the ability > to disable the regulators during system sleep. > > If keeping power management active during suspend is still desired for > this driver, I believe we are forced to keep the pointers and use the > devm_add_action_or_reset() pattern. Yes, this is the best we can do with current regulator APIs. > > If you prefer, I can drop the manual regulator control from the PM > callbacks entirely (or drop the PM callbacks altogether), which would > allow us to use the cleaner devm_regulator_get_enable_read_voltage() > helper. We can't do this unless we can be 100% sure we don't break existing users who might be depending on power management working as-is. > > Let me know which path you prefer for v6! >