[PATCH v3 0/3] rockchip: fix efuse/otp misc_read return values
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(-)
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;
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
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;
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 = {
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 errorThe 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 = {
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 = {
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 errorThe 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 = {
participants (3)
-
John Keeping -
Kever Yang -
Simon Glass