[PATCH] rockchip: efuse: fix misc_read() return values

[Jonas Karlman]

Hi John,
On 2023-03-20 17:38, 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.
> 
> Signed-off-by: John Keeping <john at metanate.com>
> ---
>  drivers/misc/rockchip-efuse.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/misc/rockchip-efuse.c b/drivers/misc/rockchip-efuse.c
> index 60931a5131..d021a20b1d 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));
> @@ -99,7 +99,7 @@ static int rockchip_rk3036_efuse_read(struct udevice *dev, int offset,
>  	writel(EFUSE_LOAD, efuse->base + EFUSE_CTRL);
>  	udelay(2);
>  
> -	while (size--) {
> +	for (int i = 0; i < size; i++) {
>  		clrsetbits_le32(efuse->base + EFUSE_CTRL, RK3036_A_MASK,
>  				RK3036_ADDR(offset++));
>  		udelay(2);
> @@ -113,7 +113,7 @@ static int rockchip_rk3036_efuse_read(struct udevice *dev, int offset,
>  	/* Switch to inactive mode */
>  	writel(0x0, efuse->base + EFUSE_CTRL);
>  
> -	return 0;
> +	return size;
>  }
>  
>  static int rockchip_rk3128_efuse_read(struct udevice *dev, int offset,
> @@ -126,7 +126,7 @@ static int rockchip_rk3128_efuse_read(struct udevice *dev, int offset,
>  	writel(EFUSE_LOAD, efuse->base + EFUSE_CTRL);
>  	udelay(2);
>  
> -	while (size--) {
> +	for (int i = 0; i < size; i++) {
>  		clrsetbits_le32(efuse->base + EFUSE_CTRL, RK3128_A_MASK,
>  				RK3128_ADDR(offset++));
>  		udelay(2);
> @@ -140,7 +140,7 @@ static int rockchip_rk3128_efuse_read(struct udevice *dev, int offset,
>  	/* Switch to inactive mode */
>  	writel(0x0, efuse->base + EFUSE_CTRL);
>  
> -	return 0;
> +	return size;
>  }
>  
>  static int rockchip_rk3288_efuse_read(struct udevice *dev, int offset,
> @@ -154,7 +154,7 @@ static int rockchip_rk3288_efuse_read(struct udevice *dev, int offset,
>  	writel(EFUSE_LOAD | EFUSE_PGENB, efuse->base + EFUSE_CTRL);
>  	udelay(2);
>  
> -	while (size--) {
> +	for (int i = 0; i < size; i++) {
>  		clrsetbits_le32(efuse->base + EFUSE_CTRL, RK3288_A_MASK,
>  				RK3288_ADDR(offset++));
>  		udelay(2);
> @@ -168,7 +168,7 @@ static int rockchip_rk3288_efuse_read(struct udevice *dev, int offset,
>  	/* Switch to standby mode */
>  	writel(EFUSE_CSB | EFUSE_PGENB, efuse->base + EFUSE_CTRL);
>  
> -	return 0;
> +	return size;
>  }
>  
>  static int rockchip_rk3328_efuse_read(struct udevice *dev, int offset,
> @@ -178,7 +178,7 @@ static int rockchip_rk3328_efuse_read(struct udevice *dev, int offset,
>  	u32 status, *buffer = buf;
>  	int ret;
>  
> -	while (size--) {
> +	for (int i = 0; i < size; i++) {
>  		writel(RK3328_AUTO_RD | RK3328_AUTO_ENB | RK3399_ADDR(offset++),
>  		       efuse->base + RK3328_AUTO_CTRL);
>  		udelay(1);
> @@ -192,7 +192,7 @@ static int rockchip_rk3328_efuse_read(struct udevice *dev, int offset,
>  		writel(RK3328_INT_FINISH, efuse->base + RK3328_INT_STATUS);
>  	}
>  
> -	return 0;
> +	return size;
>  }
>  
>  static int rockchip_rk3399_efuse_read(struct udevice *dev, int offset,
> @@ -206,7 +206,7 @@ static int rockchip_rk3399_efuse_read(struct udevice *dev, int offset,
>  	       efuse->base + EFUSE_CTRL);
>  	udelay(1);
>  
> -	while (size--) {
> +	for (int i = 0; i < size; i++) {
>  		setbits_le32(efuse->base + EFUSE_CTRL,
>  			     EFUSE_STROBE | RK3399_ADDR(offset++));
>  		udelay(1);
> @@ -218,7 +218,7 @@ static int rockchip_rk3399_efuse_read(struct udevice *dev, int offset,
>  	/* Switch to power-down mode */
>  	writel(RK3399_PD | EFUSE_CSB, efuse->base + EFUSE_CTRL);
>  
> -	return 0;
> +	return size;

The size here is in blocks of 4 bytes, same for the rk3328 function above.

I suggest you just modify the return value of rockchip_efuse_read and not
the driver internal read functions.

>  }
>  
>  static int rockchip_efuse_read(struct udevice *dev, int offset,
> @@ -251,7 +251,7 @@ static int rockchip_efuse_read(struct udevice *dev, int offset,
>  		return -ENOMEM;
>  
>  	ret = data->read(dev, block_start, buffer, blocks);
> -	if (!ret)
> +	if (ret > 0)
>  		memcpy(buf, buffer + block_offset, size);

Maybe something like this could work:

if (!ret) {
	memcpy(buf, buffer + block_offset, size);
	ret = size;
}

Would be perfect if you also could update the rockchip otp driver
at the same time, both work pretty much the same and will be good to
continue keeping them in sync.

Regards,
Jonas

>  
>  	free(buffer);