From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 5A0131DA0E1 for ; Mon, 23 Mar 2026 16:21:54 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774282914; cv=none; b=awvnyt+55+rJJ/hgAsHL9fKQqTzSZPfozbXWHA8EA3IxtNlr2wyhrkQpn4PTxyghc8x0ml9O96SDrhzhKItbteukaY2T4OMrPDN/GVPoaBOGrLJc0HdsB9gfWfxGdUdxy6o38QVpy+777eiVcF6SZqKVsiZ6DoGUsTmppMQzd9c= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774282914; c=relaxed/simple; bh=kPibRPfRtf+WPXyCOIj3JopY6yYxH1OZQfOUsXlyzhU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=beyvhXe0NteAnKMO+rLPdkREMD+qDi/62l+1pu25UtcUy6zt83qzWoYez5qvB9vF6khKZk9FM7u6NiuNEQNLVw5LN/zmtZHN0cotXyiMza4IJIlDYXdjC5D9OkBu7pqHlUWC6u8LTxgiYGOBh1rk8Lx0xUOrb1QajhvI2wQviTs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=iIxZcnAr; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="iIxZcnAr" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 01241C4CEF7; Mon, 23 Mar 2026 16:21:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1774282914; bh=kPibRPfRtf+WPXyCOIj3JopY6yYxH1OZQfOUsXlyzhU=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=iIxZcnArS3m374yqzzIKX5Yfml22zdGH8K5sve47t2n3pMbgXUT8aqhKhn1TX1E9m +LwM12Tj7fhWVOw+1Hbj78mct/Xv+M7vQ4hbYkWCIWhIZybHgtEE0Lvarkz9WKQe8C b8DUFUHpv1eTzwtx1GEsRg1nbC49bgO4f8BgsbqgyF9lM7w1dTVGOQHlNRpDsdSEDW Oiv6b3GCBZNdn8v+2BUh6GRMDlUg6+9AWkrh0fc3mNxJU01sIwJ1lnJMxkBkLBXgAY z974vzn6uvL9zMV01O9TdsIvaIml2PY+TYwCT58WuEsSItSuBWZS/0a/g2JjYd1/S2 EkU3eQ85o/4vA== Received: from johan by xi.lan with local (Exim 4.98.2) (envelope-from ) id 1w4i2J-00000003dxJ-3ekh; Mon, 23 Mar 2026 17:21:51 +0100 Date: Mon, 23 Mar 2026 17:21:51 +0100 From: Johan Hovold To: Bartosz Golaszewski Cc: Srinivas Kandagatla , Bartosz Golaszewski , linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 2/7] nvmem: return -EOPNOTSUPP to in-kernel users on missing callbacks Message-ID: References: <20260223-nvmem-unbind-v2-0-0df33a933dca@oss.qualcomm.com> <20260223-nvmem-unbind-v2-2-0df33a933dca@oss.qualcomm.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: <20260223-nvmem-unbind-v2-2-0df33a933dca@oss.qualcomm.com> On Mon, Feb 23, 2026 at 11:57:03AM +0100, Bartosz Golaszewski wrote: > __nvmem_reg_read/write() currently return -EINVAL if the relevant > callback is not present. User-space helpers again check the presence > of the callbacks to see if they should return -EPERM. > > Ahead of adding SRCU synchronization: change the error code returned to > in-kernel users to -EOPNOTSUPP which is more indicative of the actual > reason for the failure as well as allows us to translate it easily to > -EPERM in sysfs callbacks without having to dereference the callback > pointers. This will allow us to limit the number of SRCU critical > sections in the follow-up commits. > > Signed-off-by: Bartosz Golaszewski > --- > drivers/nvmem/core.c | 37 +++++++++++++++++++------------------ > 1 file changed, 19 insertions(+), 18 deletions(-) > > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c > index 311cb2e5a5c02d2c6979d7c9bbb7f94abdfbdad1..14f583e466c8d95690539bd886fd0c2fdd440998 100644 > --- a/drivers/nvmem/core.c > +++ b/drivers/nvmem/core.c > @@ -55,10 +55,10 @@ static BLOCKING_NOTIFIER_HEAD(nvmem_notifier); > static int __nvmem_reg_read(struct nvmem_device *nvmem, unsigned int offset, > void *val, size_t bytes) > { > - if (nvmem->reg_read) > - return nvmem->reg_read(nvmem->priv, offset, val, bytes); > + if (!nvmem->reg_read) > + return -EOPNOTSUPP; > > - return -EINVAL; > + return nvmem->reg_read(nvmem->priv, offset, val, bytes); > } > > static int __nvmem_reg_write(struct nvmem_device *nvmem, unsigned int offset, > @@ -66,14 +66,14 @@ static int __nvmem_reg_write(struct nvmem_device *nvmem, unsigned int offset, > { > int ret; > > - if (nvmem->reg_write) { > - gpiod_set_value_cansleep(nvmem->wp_gpio, 0); > - ret = nvmem->reg_write(nvmem->priv, offset, val, bytes); > - gpiod_set_value_cansleep(nvmem->wp_gpio, 1); > - return ret; > - } > + if (!nvmem->reg_write) > + return -EOPNOTSUPP; > + > + gpiod_set_value_cansleep(nvmem->wp_gpio, 0); > + ret = nvmem->reg_write(nvmem->priv, offset, val, bytes); > + gpiod_set_value_cansleep(nvmem->wp_gpio, 1); > > - return -EINVAL; > + return ret; > } The above looks like good clean ups in their own right. But shouldn't the check be moved to to nvmem_reg_read/write() to avoid calling into nvmem_access_with_keepouts()? > static int nvmem_access_with_keepouts(struct nvmem_device *nvmem, > @@ -231,13 +231,12 @@ static ssize_t bin_attr_nvmem_read(struct file *filp, struct kobject *kobj, > > count = round_down(count, nvmem->word_size); > > - if (!nvmem->reg_read) > - return -EPERM; > - > rc = nvmem_reg_read(nvmem, pos, buf, count); > - > - if (rc) > + if (rc) { > + if (rc == -EOPNOTSUPP) > + rc = -EPERM; > return rc; > + } > > return count; > } > @@ -264,13 +263,15 @@ static ssize_t bin_attr_nvmem_write(struct file *filp, struct kobject *kobj, > > count = round_down(count, nvmem->word_size); > > - if (!nvmem->reg_write || nvmem->read_only) > + if (nvmem->read_only) > return -EPERM; > > rc = nvmem_reg_write(nvmem, pos, buf, count); > - > - if (rc) > + if (rc) { > + if (rc == -EOPNOTSUPP) > + rc = -EPERM; > return rc; > + } > > return count; > } And these attributes are not even visible when the device does not support reading or writing so checking later is not an issue. Did you make sure that there are no in-kernel callers that will get confused by the -EINVAL => -EOPNOTSUPP change? Johan