[PATCH v3 0/3] rockchip: fix efuse/otp misc_read return values

[PATCH v3 0/3] rockchip: fix efuse/otp misc_read return values

[John Keeping]

Fix the return value for misc_read() in the two Rockchip nvmem drivers
so that they return the number of bytes read.

In v3 there is a new initial patch to fix the one place that uses this
return value to correctly check for negative error returns.  The other
two patches are unchanged from v2.

John Keeping (3):
  rockchip: misc: fix misc_read() return check
  rockchip: efuse: fix misc_read() return values
  rockchip: otp: fix misc_read() return values

 arch/arm/mach-rockchip/misc.c |  2 +-
 drivers/misc/rockchip-efuse.c | 12 ++++++++----
 drivers/misc/rockchip-otp.c   | 12 ++++++++----
 3 files changed, 17 insertions(+), 9 deletions(-)

-- 
2.40.0

[PATCH v3 1/3] rockchip: misc: fix misc_read() return check

[John Keeping]

misc_read() is documented to return the number of bytes read or a
negative error value.  The Rockchip drivers currently do not implement
this correctly and instead return zero on success or a negative error
value.

In preparation for fixing the drivers, fix the condition here to only
error on negative values.

Suggested-by: Jonas Karlman <jonas@kwiboo.se>
Signed-off-by: John Keeping <john@metanate.com>
---
v3: new patch

 arch/arm/mach-rockchip/misc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-rockchip/misc.c b/arch/arm/mach-rockchip/misc.c
index 849014d2fb..7d03f0c2b6 100644
--- a/arch/arm/mach-rockchip/misc.c
+++ b/arch/arm/mach-rockchip/misc.c
@@ -83,7 +83,7 @@ int rockchip_cpuid_from_efuse(const u32 cpuid_offset,
 
 	/* read the cpu_id range from the efuses */
 	ret = misc_read(dev, cpuid_offset, cpuid, cpuid_length);
-	if (ret) {
+	if (ret < 0) {
 		debug("%s: reading cpuid from the efuses failed\n",
 		      __func__);
 		return -1;
-- 
2.40.0

[PATCH v3 2/3] rockchip: efuse: fix misc_read() return values

[John Keeping]

The documentation for misc_read() says:

    Return: number of bytes read if OK (may be 0 if EOF), -ve on error

The Rockchip efuse driver implements this so it should return the number
of bytes read rather than zero on success.  Fix this so that the driver
follows the usual contract for read operations.

Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Jonas Karlman <jonas@kwiboo.se>
Signed-off-by: John Keeping <john@metanate.com>
---
v3: unchanged
v2:
- Fix when block_size > 1 by moving the return value to the main
  rockchip_efuse_read() wrapper and leaving the individual
  implementations alone (Jonas)

 drivers/misc/rockchip-efuse.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/rockchip-efuse.c b/drivers/misc/rockchip-efuse.c
index 60931a5131..2f96b79ea4 100644
--- a/drivers/misc/rockchip-efuse.c
+++ b/drivers/misc/rockchip-efuse.c
@@ -73,7 +73,7 @@ static int dump_efuse(struct cmd_tbl *cmdtp, int flag,
 
 	for (i = 0; true; i += sizeof(data)) {
 		ret = misc_read(dev, i, &data, sizeof(data));
-		if (ret < 0)
+		if (ret <= 0)
 			return 0;
 
 		print_buffer(i, data, 1, sizeof(data), sizeof(data));
@@ -238,8 +238,10 @@ static int rockchip_efuse_read(struct udevice *dev, int offset,
 
 	offset += data->offset;
 
-	if (data->block_size <= 1)
-		return data->read(dev, offset, buf, size);
+	if (data->block_size <= 1) {
+		ret = data->read(dev, offset, buf, size);
+		goto done;
+	}
 
 	block_start = offset / data->block_size;
 	block_offset = offset % data->block_size;
@@ -255,7 +257,9 @@ static int rockchip_efuse_read(struct udevice *dev, int offset,
 		memcpy(buf, buffer + block_offset, size);
 
 	free(buffer);
-	return ret;
+
+done:
+	return ret < 0 ? ret : size;
 }
 
 static const struct misc_ops rockchip_efuse_ops = {
-- 
2.40.0

[PATCH v3 3/3] rockchip: otp: fix misc_read() return values

[John Keeping]

The documentation for misc_read() says:

    Return: number of bytes read if OK (may be 0 if EOF), -ve on error

The Rockchip efuse driver implements this so it should return the number
of bytes read rather than zero on success.  Fix this so that the driver
follows the usual contract for read operations.

Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Jonas Karlman <jonas@kwiboo.se>
Signed-off-by: John Keeping <john@metanate.com>
---
v3: unchanged
v2: new patch

 drivers/misc/rockchip-otp.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/rockchip-otp.c b/drivers/misc/rockchip-otp.c
index c19cd5ce62..4814e0e501 100644
--- a/drivers/misc/rockchip-otp.c
+++ b/drivers/misc/rockchip-otp.c
@@ -89,7 +89,7 @@ static int dump_otp(struct cmd_tbl *cmdtp, int flag,
 
 	for (i = 0; true; i += sizeof(data)) {
 		ret = misc_read(dev, i, &data, sizeof(data));
-		if (ret < 0)
+		if (ret <= 0)
 			return 0;
 
 		print_buffer(i, data, 1, sizeof(data), sizeof(data));
@@ -249,8 +249,10 @@ static int rockchip_otp_read(struct udevice *dev, int offset,
 
 	offset += data->offset;
 
-	if (data->block_size <= 1)
-		return data->read(dev, offset, buf, size);
+	if (data->block_size <= 1) {
+		ret = data->read(dev, offset, buf, size);
+		goto done;
+	}
 
 	block_start = offset / data->block_size;
 	block_offset = offset % data->block_size;
@@ -266,7 +268,9 @@ static int rockchip_otp_read(struct udevice *dev, int offset,
 		memcpy(buf, buffer + block_offset, size);
 
 	free(buffer);
-	return ret;
+
+done:
+	return ret < 0 ? ret : size;
 }
 
 static const struct misc_ops rockchip_otp_ops = {
-- 
2.40.0

Re: [PATCH v3 1/3] rockchip: misc: fix misc_read() return check

[Simon Glass]

On Tue, 28 Mar 2023 at 00:01, John Keeping <john@metanate.com> wrote:
>
> misc_read() is documented to return the number of bytes read or a
> negative error value.  The Rockchip drivers currently do not implement
> this correctly and instead return zero on success or a negative error
> value.
>
> In preparation for fixing the drivers, fix the condition here to only
> error on negative values.
>
> Suggested-by: Jonas Karlman <jonas@kwiboo.se>
> Signed-off-by: John Keeping <john@metanate.com>
> ---
> v3: new patch
>
>  arch/arm/mach-rockchip/misc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v3 1/3] rockchip: misc: fix misc_read() return check

[Kever Yang]

On 2023/3/27 19:01, John Keeping wrote:
> misc_read() is documented to return the number of bytes read or a
> negative error value.  The Rockchip drivers currently do not implement
> this correctly and instead return zero on success or a negative error
> value.
>
> In preparation for fixing the drivers, fix the condition here to only
> error on negative values.
>
> Suggested-by: Jonas Karlman <jonas@kwiboo.se>
> Signed-off-by: John Keeping <john@metanate.com>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
> v3: new patch
>
>   arch/arm/mach-rockchip/misc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-rockchip/misc.c b/arch/arm/mach-rockchip/misc.c
> index 849014d2fb..7d03f0c2b6 100644
> --- a/arch/arm/mach-rockchip/misc.c
> +++ b/arch/arm/mach-rockchip/misc.c
> @@ -83,7 +83,7 @@ int rockchip_cpuid_from_efuse(const u32 cpuid_offset,
>   
>   	/* read the cpu_id range from the efuses */
>   	ret = misc_read(dev, cpuid_offset, cpuid, cpuid_length);
> -	if (ret) {
> +	if (ret < 0) {
>   		debug("%s: reading cpuid from the efuses failed\n",
>   		      __func__);
>   		return -1;

Re: [PATCH v3 2/3] rockchip: efuse: fix misc_read() return values

[Kever Yang]

On 2023/3/27 19:01, John Keeping wrote:
> The documentation for misc_read() says:
>
>      Return: number of bytes read if OK (may be 0 if EOF), -ve on error
>
> The Rockchip efuse driver implements this so it should return the number
> of bytes read rather than zero on success.  Fix this so that the driver
> follows the usual contract for read operations.
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Jonas Karlman <jonas@kwiboo.se>
> Signed-off-by: John Keeping <john@metanate.com>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
> v3: unchanged
> v2:
> - Fix when block_size > 1 by moving the return value to the main
>    rockchip_efuse_read() wrapper and leaving the individual
>    implementations alone (Jonas)
>
>   drivers/misc/rockchip-efuse.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/misc/rockchip-efuse.c b/drivers/misc/rockchip-efuse.c
> index 60931a5131..2f96b79ea4 100644
> --- a/drivers/misc/rockchip-efuse.c
> +++ b/drivers/misc/rockchip-efuse.c
> @@ -73,7 +73,7 @@ static int dump_efuse(struct cmd_tbl *cmdtp, int flag,
>   
>   	for (i = 0; true; i += sizeof(data)) {
>   		ret = misc_read(dev, i, &data, sizeof(data));
> -		if (ret < 0)
> +		if (ret <= 0)
>   			return 0;
>   
>   		print_buffer(i, data, 1, sizeof(data), sizeof(data));
> @@ -238,8 +238,10 @@ static int rockchip_efuse_read(struct udevice *dev, int offset,
>   
>   	offset += data->offset;
>   
> -	if (data->block_size <= 1)
> -		return data->read(dev, offset, buf, size);
> +	if (data->block_size <= 1) {
> +		ret = data->read(dev, offset, buf, size);
> +		goto done;
> +	}
>   
>   	block_start = offset / data->block_size;
>   	block_offset = offset % data->block_size;
> @@ -255,7 +257,9 @@ static int rockchip_efuse_read(struct udevice *dev, int offset,
>   		memcpy(buf, buffer + block_offset, size);
>   
>   	free(buffer);
> -	return ret;
> +
> +done:
> +	return ret < 0 ? ret : size;
>   }
>   
>   static const struct misc_ops rockchip_efuse_ops = {

Re: [PATCH v3 3/3] rockchip: otp: fix misc_read() return values

[Kever Yang]

On 2023/3/27 19:01, John Keeping wrote:
> The documentation for misc_read() says:
>
>      Return: number of bytes read if OK (may be 0 if EOF), -ve on error
>
> The Rockchip efuse driver implements this so it should return the number
> of bytes read rather than zero on success.  Fix this so that the driver
> follows the usual contract for read operations.
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Jonas Karlman <jonas@kwiboo.se>
> Signed-off-by: John Keeping <john@metanate.com>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
> v3: unchanged
> v2: new patch
>
>   drivers/misc/rockchip-otp.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/misc/rockchip-otp.c b/drivers/misc/rockchip-otp.c
> index c19cd5ce62..4814e0e501 100644
> --- a/drivers/misc/rockchip-otp.c
> +++ b/drivers/misc/rockchip-otp.c
> @@ -89,7 +89,7 @@ static int dump_otp(struct cmd_tbl *cmdtp, int flag,
>   
>   	for (i = 0; true; i += sizeof(data)) {
>   		ret = misc_read(dev, i, &data, sizeof(data));
> -		if (ret < 0)
> +		if (ret <= 0)
>   			return 0;
>   
>   		print_buffer(i, data, 1, sizeof(data), sizeof(data));
> @@ -249,8 +249,10 @@ static int rockchip_otp_read(struct udevice *dev, int offset,
>   
>   	offset += data->offset;
>   
> -	if (data->block_size <= 1)
> -		return data->read(dev, offset, buf, size);
> +	if (data->block_size <= 1) {
> +		ret = data->read(dev, offset, buf, size);
> +		goto done;
> +	}
>   
>   	block_start = offset / data->block_size;
>   	block_offset = offset % data->block_size;
> @@ -266,7 +268,9 @@ static int rockchip_otp_read(struct udevice *dev, int offset,
>   		memcpy(buf, buffer + block_offset, size);
>   
>   	free(buffer);
> -	return ret;
> +
> +done:
> +	return ret < 0 ? ret : size;
>   }
>   
>   static const struct misc_ops rockchip_otp_ops = {