Re: [PATCH v2 2/7] nvmem: return -EOPNOTSUPP to in-kernel users on missing callbacks

[Johan Hovold]

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 <bartosz.golaszewski@xxxxxxxxxxxxxxxx>
> ---
>  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