[PATCH v2 0/2] 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 v2 the rockchip-efuse patch is change to correctly handle block_size > 1 and the rockchip-otp patch is new.
John Keeping (2): rockchip: efuse: fix misc_read() return values rockchip: otp: fix misc_read() return values
drivers/misc/rockchip-efuse.c | 12 ++++++++---- drivers/misc/rockchip-otp.c | 12 ++++++++---- 2 files changed, 16 insertions(+), 8 deletions(-)

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.
Signed-off-by: John Keeping john@metanate.com --- 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 Tue, 21 Mar 2023 at 06:18, John Keeping john@metanate.com 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.
Signed-off-by: John Keeping john@metanate.com
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(-)
Reviewed-by: Simon Glass sjg@chromium.org

Hi John,
On 2023-03-20 19:39, Simon Glass wrote:
On Tue, 21 Mar 2023 at 06:18, John Keeping john@metanate.com 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.
Signed-off-by: John Keeping john@metanate.com
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(-)
Reviewed-by: Simon Glass sjg@chromium.org
With these changes I get the following trying to boot my boards and having ROCKCHIP_EFUSE/OTP enabled.
initcall sequence 000000007ffcecc0 failed at call 0000000000202610 (err=-1) ### ERROR ### Please RESET the board ###
We need a change to ret value handling in rockchip_cpuid_from_efuse in arch/arm/mach-rockchip/misc.c to fix this, maybe something like:
/* read the cpu_id range from the efuses */ ret = misc_read(dev, cpuid_offset, cpuid, cpuid_length); - if (ret) { + if (ret < 0) {
Reviewed-by: Jonas Karlman jonas@kwiboo.se
Regards, Jonas

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.
Signed-off-by: John Keeping john@metanate.com --- 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 Tue, 21 Mar 2023 at 06:18, John Keeping john@metanate.com 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.
Signed-off-by: John Keeping john@metanate.com
v2:
- New patch
drivers/misc/rockchip-otp.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

Hi John,
On 2023-03-20 19:39, Simon Glass wrote:
On Tue, 21 Mar 2023 at 06:18, John Keeping john@metanate.com 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.
Signed-off-by: John Keeping john@metanate.com
v2:
- New patch
drivers/misc/rockchip-otp.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Reviewed-by: Jonas Karlman jonas@kwiboo.se
Regards, Jonas
participants (3)
-
John Keeping
-
Jonas Karlman
-
Simon Glass