[RFC PATCH 0/4] cmd: fuse: Introduce fuse writebuff sub-system and clean up

[RFC PATCH 0/4] cmd: fuse: Introduce fuse writebuff sub-system and clean up

[Harsha Vardhan V M]

This patch series introduces the fuse writebuff sub-system command and
makes improvements to the existing fuse implementation by removing the
custom string functions. The patches are required to be applied in
sequence.

The series consists of the following changes:
Patch 1 removes custom string functions and replaces them with standard
string functions.
Patch 2 introduces the fuse writebuff sub-system command, allowing to
write a structured buffer in memory to fuses, and implementing the
necessary function calls.
Patch 3 enables the fuse sub-system in the K3 platform.
Patch 4 updates the documentation to include details about the new fuse
writebuff command.

These changes aim to improve the fuse sub-system by the removal of
custom string functions and the addition of the fuse writebuff
command improves fuse programming workflows by allowing to write a
structured buffer in memory to efuses.

Harsha Vardhan V M (4):
  cmd: fuse: Remove custom string functions
  cmd: fuse: Add fuse writebuff sub-system command
  drivers: k3_fuse: Add fuse sub-system func calls
  docs: fuse: Add fuse writebuff cmd docs

 cmd/Kconfig            |  8 +++++
 cmd/fuse.c             | 49 ++++++++++++++------------
 doc/README.fuse        | 14 ++++++++
 drivers/misc/Kconfig   |  7 ++++
 drivers/misc/Makefile  |  1 +
 drivers/misc/k3_fuse.c | 78 ++++++++++++++++++++++++++++++++++++++++++
 include/fuse.h         |  9 +++++
 7 files changed, 145 insertions(+), 21 deletions(-)
 create mode 100644 drivers/misc/k3_fuse.c

-- 
2.34.1

[RFC PATCH 1/4] cmd: fuse: Remove custom string functions

[Harsha Vardhan V M]

Remove custom string functions and replace them with normal string
functions. Remove the custom strtou32 and replace it with str2long.

Signed-off-by: Harsha Vardhan V M <h-vm@ti.com>
---
 cmd/fuse.c | 27 ++++++++-------------------
 1 file changed, 8 insertions(+), 19 deletions(-)

diff --git a/cmd/fuse.c b/cmd/fuse.c
index 598ef496a43..9f489570634 100644
--- a/cmd/fuse.c
+++ b/cmd/fuse.c
@@ -15,17 +15,6 @@
 #include <vsprintf.h>
 #include <linux/errno.h>
 
-static int strtou32(const char *str, unsigned int base, u32 *result)
-{
-	char *ep;
-
-	*result = simple_strtoul(str, &ep, base);
-	if (ep == str || *ep != '\0')
-		return -EINVAL;
-
-	return 0;
-}
-
 static int confirm_prog(void)
 {
 	puts("Warning: Programming fuses is an irreversible operation!\n"
@@ -54,14 +43,14 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
 	argc -= 2 + confirmed;
 	argv += 2 + confirmed;
 
-	if (argc < 2 || strtou32(argv[0], 0, &bank) ||
-			strtou32(argv[1], 0, &word))
+	if (argc < 2 || !(str2long(argv[0], (ulong *)&bank)) ||
+			!(str2long(argv[1], (ulong *)&word)))
 		return CMD_RET_USAGE;
 
 	if (!strcmp(op, "read")) {
 		if (argc == 2)
 			cnt = 1;
-		else if (argc != 3 || strtou32(argv[2], 0, &cnt))
+		else if (argc != 3 || !(str2long(argv[2], (ulong *)&cnt)))
 			return CMD_RET_USAGE;
 
 		printf("Reading bank %u:\n", bank);
@@ -79,7 +68,7 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
 	} else if (!strcmp(op, "readm")) {
 		if (argc == 3)
 			cnt = 1;
-		else if (argc != 4 || strtou32(argv[3], 0, &cnt))
+		else if (argc != 4 || !(str2long(argv[3], (ulong *)&cnt)))
 			return CMD_RET_USAGE;
 
 		addr = simple_strtoul(argv[2], NULL, 16);
@@ -99,7 +88,7 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
 
 		unmap_sysmem(start);
 	} else if (!strcmp(op, "cmp")) {
-		if (argc != 3 || strtou32(argv[2], 0, &cmp))
+		if (argc != 3 || !(str2long(argv[2], (ulong *)&cmp)))
 			return CMD_RET_USAGE;
 
 		printf("Comparing bank %u:\n", bank);
@@ -119,7 +108,7 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
 	} else if (!strcmp(op, "sense")) {
 		if (argc == 2)
 			cnt = 1;
-		else if (argc != 3 || strtou32(argv[2], 0, &cnt))
+		else if (argc != 3 || !(str2long(argv[2], (ulong *)&cnt)))
 			return CMD_RET_USAGE;
 
 		printf("Sensing bank %u:\n", bank);
@@ -139,7 +128,7 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
 			return CMD_RET_USAGE;
 
 		for (i = 2; i < argc; i++, word++) {
-			if (strtou32(argv[i], 16, &val))
+			if (!(str2long(argv[i], (ulong *)&val)))
 				return CMD_RET_USAGE;
 
 			printf("Programming bank %u word 0x%.8x to 0x%.8x...\n",
@@ -155,7 +144,7 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
 			return CMD_RET_USAGE;
 
 		for (i = 2; i < argc; i++, word++) {
-			if (strtou32(argv[i], 16, &val))
+			if (!(str2long(argv[i], (ulong *)&val)))
 				return CMD_RET_USAGE;
 
 			printf("Overriding bank %u word 0x%.8x with "
-- 
2.34.1

[RFC PATCH 2/4] cmd: fuse: Add fuse writebuff sub-system command

[Harsha Vardhan V M]

Add CMD_FUSE_WRITEBUFF config option to add and enable fuse writebuff
sub-system command. Add fuse_writebuff function to be invoked on
writebuff command.

Signed-off-by: Harsha Vardhan V M <h-vm@ti.com>
---
 cmd/Kconfig    |  8 ++++++++
 cmd/fuse.c     | 26 ++++++++++++++++++++++----
 include/fuse.h |  9 +++++++++
 3 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index cd391d422ae..81c59becc2d 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1236,6 +1236,14 @@ config CMD_FUSE
 	  which control the behaviour of the device. The command uses the
 	  fuse_...() API.
 
+config CMD_FUSE_WRITEBUFF
+	bool "Support for the fuse writebuff"
+	depends on CMD_FUSE
+	help
+	  This allows programming fuses, which control the behaviour of
+	  the device, using a structured buffer in memory. The command
+	  uses the fuse_writebuff() API.
+
 config CMD_GPIO
 	bool "gpio"
 	help
diff --git a/cmd/fuse.c b/cmd/fuse.c
index 9f489570634..2d53e6bf2c4 100644
--- a/cmd/fuse.c
+++ b/cmd/fuse.c
@@ -43,9 +43,14 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
 	argc -= 2 + confirmed;
 	argv += 2 + confirmed;
 
-	if (argc < 2 || !(str2long(argv[0], (ulong *)&bank)) ||
-			!(str2long(argv[1], (ulong *)&word)))
-		return CMD_RET_USAGE;
+	if (IS_ENABLED(CONFIG_CMD_FUSE_WRITEBUFF) && !strcmp(op, "writebuff")) {
+		if (argc != 1 || !(str2long(argv[0], &addr)))
+			return CMD_RET_USAGE;
+	} else {
+		if (argc < 2 || !(str2long(argv[0], (ulong *)&bank)) ||
+				!(str2long(argv[1], (ulong *)&word)))
+			return CMD_RET_USAGE;
+	}
 
 	if (!strcmp(op, "read")) {
 		if (argc == 2)
@@ -153,6 +158,15 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
 			if (ret)
 				goto err;
 		}
+	} else if (IS_ENABLED(CONFIG_CMD_FUSE_WRITEBUFF) && !strcmp(op, "writebuff")) {
+		printf("Programming fuses using a structured buffer in memory "
+				"starting at addr 0x%lx\n", addr);
+		if (!confirmed && !confirm_prog())
+			return CMD_RET_FAILURE;
+
+		ret = fuse_writebuff(addr);
+		if (ret)
+			goto err;
 	} else {
 		return CMD_RET_USAGE;
 	}
@@ -178,5 +192,9 @@ U_BOOT_CMD(
 	"fuse prog [-y] <bank> <word> <hexval> [<hexval>...] - program 1 or\n"
 	"    several fuse words, starting at 'word' (PERMANENT)\n"
 	"fuse override <bank> <word> <hexval> [<hexval>...] - override 1 or\n"
-	"    several fuse words, starting at 'word'"
+	"    several fuse words, starting at 'word'\n"
+#ifdef CONFIG_CMD_FUSE_WRITEBUFF
+	"fuse writebuff [-y] <addr> - program fuse data\n"
+	"    using a structured buffer in memory starting at 'addr'\n"
+#endif /* CONFIG_CMD_FUSE_WRITEBUFF */
 );
diff --git a/include/fuse.h b/include/fuse.h
index 4519821af7e..902b5f56a74 100644
--- a/include/fuse.h
+++ b/include/fuse.h
@@ -12,6 +12,7 @@
 #define _FUSE_H_
 
 #include <linux/types.h>
+#include <linux/errno.h>
 
 /*
  * Read/Sense/Program/Override interface:
@@ -25,5 +26,13 @@ int fuse_read(u32 bank, u32 word, u32 *val);
 int fuse_sense(u32 bank, u32 word, u32 *val);
 int fuse_prog(u32 bank, u32 word, u32 val);
 int fuse_override(u32 bank, u32 word, u32 val);
+#ifdef CONFIG_CMD_FUSE_WRITEBUFF
+int fuse_writebuff(ulong addr);
+#else
+static inline int fuse_writebuff(ulong addr)
+{
+	return -EPERM;
+}
+#endif	/* CONFIG_CMD_FUSE_WRITEBUFF */
 
 #endif	/* _FUSE_H_ */
-- 
2.34.1

[RFC PATCH 3/4] drivers: k3_fuse: Add fuse sub-system func calls

[Harsha Vardhan V M]

Add K3_FUSE config option to add and enable fuse sub-system
implementation function calls.

Signed-off-by: Harsha Vardhan V M <h-vm@ti.com>
---
 drivers/misc/Kconfig   |  7 ++++
 drivers/misc/Makefile  |  1 +
 drivers/misc/k3_fuse.c | 78 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 86 insertions(+)
 create mode 100644 drivers/misc/k3_fuse.c

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index da84b35e804..834e0285097 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -468,6 +468,13 @@ config STM32MP_FUSE
 	  for STM32MP architecture.
 	  This API is needed for CMD_FUSE.
 
+config K3_FUSE
+	bool "Enable TI K3 fuse wrapper providing the fuse API"
+	depends on MISC && CMD_FUSE && CMD_FUSE_WRITEBUFF
+	help
+	  If you say Y here, you will get support for the fuse API (OTP)
+	  for TI K3 architecture.
+
 config STM32_RCC
 	bool "Enable RCC driver for the STM32 SoC's family"
 	depends on (ARCH_STM32 || ARCH_STM32MP) && MISC
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index dac805e4cdd..0b81ba2604f 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_$(XPL_)I2C_EEPROM) += i2c_eeprom.o
 obj-$(CONFIG_IHS_FPGA) += ihs_fpga.o
 obj-$(CONFIG_IMX8) += imx8/
 obj-$(CONFIG_IMX_ELE) += imx_ele/
+obj-$(CONFIG_K3_FUSE) += k3_fuse.o
 obj-$(CONFIG_LED_STATUS) += status_led.o
 obj-$(CONFIG_LED_STATUS_GPIO) += gpio_led.o
 obj-$(CONFIG_MPC83XX_SERDES) += mpc83xx_serdes.o
diff --git a/drivers/misc/k3_fuse.c b/drivers/misc/k3_fuse.c
new file mode 100644
index 00000000000..4a8ff1f2523
--- /dev/null
+++ b/drivers/misc/k3_fuse.c
@@ -0,0 +1,78 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2025 Texas Instruments Incorporated, <www.ti.com>
+ */
+
+#include <errno.h>
+#include <stdio.h>
+#include <fuse.h>
+#include <linux/arm-smccc.h>
+#include <string.h>
+
+#define K3_SIP_OTP_WRITEBUFF 0xC2000000
+#define K3_SIP_OTP_WRITE 0xC2000001
+#define K3_SIP_OTP_READ 0xC2000002
+
+int fuse_read(u32 bank, u32 word, u32 *val)
+{
+	struct arm_smccc_res res;
+
+	if (bank != 0U) {
+		printf("Invalid bank argument, ONLY bank 0 is supported\n");
+		return -EINVAL;
+	}
+
+	/* Make SiP SMC call and send the word in the parameter register */
+	arm_smccc_smc(K3_SIP_OTP_READ, word,
+		      0, 0, 0, 0, 0, 0, &res);
+
+	*val = res.a1;
+	if (res.a0 != 0)
+		printf("SMC call failed: Error code %lu\n", res.a0);
+
+	return res.a0;
+}
+
+int fuse_sense(u32 bank, u32 word, u32 *val)
+{
+	return -EPERM;
+}
+
+int fuse_prog(u32 bank, u32 word, u32 val)
+{
+	struct arm_smccc_res res;
+	u32 mask = val;
+
+	if (bank != 0U) {
+		printf("Invalid bank argument, ONLY bank 0 is supported\n");
+		return -EINVAL;
+	}
+
+	/* Make SiP SMC call and send the word, val and mask in the parameter register */
+	arm_smccc_smc(K3_SIP_OTP_WRITE, word,
+		      val, mask, 0, 0, 0, 0, &res);
+
+	if (res.a0 != 0)
+		printf("SMC call failed: Error code %lu\n", res.a0);
+
+	return res.a0;
+}
+
+int fuse_override(u32 bank, u32 word, u32 val)
+{
+	return -EPERM;
+}
+
+int fuse_writebuff(ulong addr)
+{
+	struct arm_smccc_res res;
+
+	/* Make SiP SMC call and send the addr in the parameter register */
+	arm_smccc_smc(K3_SIP_OTP_WRITEBUFF, (unsigned long)addr,
+		      0, 0, 0, 0, 0, 0, &res);
+
+	if (res.a0 != 0)
+		printf("SMC call failed: Error code %lu\n", res.a0);
+
+	return res.a0;
+}
-- 
2.34.1

[RFC PATCH 4/4] docs: fuse: Add fuse writebuff cmd docs

[Harsha Vardhan V M]

Add fuse writebuff sub-system command documentation

Signed-off-by: Harsha Vardhan V M <h-vm@ti.com>
---
 doc/README.fuse | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/doc/README.fuse b/doc/README.fuse
index 1bc91c44a6a..b150f39cc16 100644
--- a/doc/README.fuse
+++ b/doc/README.fuse
@@ -61,7 +61,21 @@ Functions / commands:
       fuses have already been programmed or are locked (if the SoC allows to
       override a locked fuse).
 
+   int fuse_writebuff(ulong addr);
+   fuse writebuff [-y] <addr>
+      Program fuse data using a structured buffer in memory starting at 'addr'.
+      This operation directly affects the fusebox and is irreversible.
+
+      The structure of the buffer should contain all necessary details for
+      programming fuses, such as the values to be written to the fuse, optional
+      metadata for validation or programming constraints and any configuration
+      data required for the operation. Define CONFIG_CMD_FUSE_WRITEBUFF to
+      enable the fuse writebuff command.
+
 Configuration:
 
    CONFIG_CMD_FUSE
       Define this to enable the fuse commands.
+
+   CONFIG_CMD_FUSE_WRITEBUFF
+      Define this to enable the fuse writebuff command.
-- 
2.34.1

Re: [RFC PATCH 1/4] cmd: fuse: Remove custom string functions

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1457 bytes --]

On Thu, Mar 13, 2025 at 05:25:14PM +0530, Harsha Vardhan V M wrote:

> Remove custom string functions and replace them with normal string
> functions. Remove the custom strtou32 and replace it with str2long.
> 
> Signed-off-by: Harsha Vardhan V M <h-vm@ti.com>

Thanks for doing this.

> ---
>  cmd/fuse.c | 27 ++++++++-------------------
>  1 file changed, 8 insertions(+), 19 deletions(-)
> 
> diff --git a/cmd/fuse.c b/cmd/fuse.c
> index 598ef496a43..9f489570634 100644
> --- a/cmd/fuse.c
> +++ b/cmd/fuse.c
> @@ -15,17 +15,6 @@
>  #include <vsprintf.h>
>  #include <linux/errno.h>
>  
> -static int strtou32(const char *str, unsigned int base, u32 *result)
> -{
> -	char *ep;
> -
> -	*result = simple_strtoul(str, &ep, base);
> -	if (ep == str || *ep != '\0')
> -		return -EINVAL;
> -
> -	return 0;
> -}
> -
>  static int confirm_prog(void)
>  {
>  	puts("Warning: Programming fuses is an irreversible operation!\n"
> @@ -54,14 +43,14 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
>  	argc -= 2 + confirmed;
>  	argv += 2 + confirmed;
>  
> -	if (argc < 2 || strtou32(argv[0], 0, &bank) ||
> -			strtou32(argv[1], 0, &word))
> +	if (argc < 2 || !(str2long(argv[0], (ulong *)&bank)) ||
> +			!(str2long(argv[1], (ulong *)&word)))

I didn't know we had "str2long" which is a differently rarely used
function. Why not just simple_strtoul inline? Am I missing something?
Thanks.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [RFC PATCH 2/4] cmd: fuse: Add fuse writebuff sub-system command

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 396 bytes --]

On Thu, Mar 13, 2025 at 05:25:15PM +0530, Harsha Vardhan V M wrote:

> Add CMD_FUSE_WRITEBUFF config option to add and enable fuse writebuff
> sub-system command. Add fuse_writebuff function to be invoked on
> writebuff command.
> 
> Signed-off-by: Harsha Vardhan V M <h-vm@ti.com>

Pending updates for what I asked in part one,

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [RFC PATCH 3/4] drivers: k3_fuse: Add fuse sub-system func calls

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 571 bytes --]

On Thu, Mar 13, 2025 at 05:25:16PM +0530, Harsha Vardhan V M wrote:

> Add K3_FUSE config option to add and enable fuse sub-system
> implementation function calls.
> 
> Signed-off-by: Harsha Vardhan V M <h-vm@ti.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

And can you please file an issue on
https://source.denx.de/u-boot/u-boot/-/issues/ to further clean-up the
fuse related Kconfig symbols and logic? What we have today is a bit odd,
but it's I think easier for me to go and shuffle things than explain how
I think it should be. Thanks.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [RFC PATCH 4/4] docs: fuse: Add fuse writebuff cmd docs

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 416 bytes --]

On Thu, Mar 13, 2025 at 05:25:17PM +0530, Harsha Vardhan V M wrote:

> Add fuse writebuff sub-system command documentation
> 
> Signed-off-by: Harsha Vardhan V M <h-vm@ti.com>
> ---
>  doc/README.fuse | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)

Please convert this to doc/cmd/usage/fuse.rst first (as say patch #2)
and then update with the new command in the final patch. Thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [EXTERNAL] Re: [RFC PATCH 1/4] cmd: fuse: Remove custom string functions

[Harsha Vardhan V M]

On 13/03/25 21:45, Tom Rini wrote:
> On Thu, Mar 13, 2025 at 05:25:14PM +0530, Harsha Vardhan V M wrote:
> 
>> Remove custom string functions and replace them with normal string
>> functions. Remove the custom strtou32 and replace it with str2long.
>>
>> Signed-off-by: Harsha Vardhan V M <h-vm@ti.com>
> 
> Thanks for doing this.
> 
>> ---
>>   cmd/fuse.c | 27 ++++++++-------------------
>>   1 file changed, 8 insertions(+), 19 deletions(-)
>>
>> diff --git a/cmd/fuse.c b/cmd/fuse.c
>> index 598ef496a43..9f489570634 100644
>> --- a/cmd/fuse.c
>> +++ b/cmd/fuse.c
>> @@ -15,17 +15,6 @@
>>   #include <vsprintf.h>
>>   #include <linux/errno.h>
>>   
>> -static int strtou32(const char *str, unsigned int base, u32 *result)
>> -{
>> -	char *ep;
>> -
>> -	*result = simple_strtoul(str, &ep, base);
>> -	if (ep == str || *ep != '\0')
>> -		return -EINVAL;
>> -
>> -	return 0;
>> -}
>> -
>>   static int confirm_prog(void)
>>   {
>>   	puts("Warning: Programming fuses is an irreversible operation!\n"
>> @@ -54,14 +43,14 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
>>   	argc -= 2 + confirmed;
>>   	argv += 2 + confirmed;
>>   
>> -	if (argc < 2 || strtou32(argv[0], 0, &bank) ||
>> -			strtou32(argv[1], 0, &word))
>> +	if (argc < 2 || !(str2long(argv[0], (ulong *)&bank)) ||
>> +			!(str2long(argv[1], (ulong *)&word)))
> 
> I didn't know we had "str2long" which is a differently rarely used
> function. Why not just simple_strtoul inline? Am I missing something?
> Thanks.
> 

We cannot use simple_strtoul inline directly because we need proper 
error checking to ensure the simple_strtoul conversion was successful. 
The str2long function is a wrapper around simple_strtoul and the 
necessary error checks. Hence, using str2long here.
Thanks.

Re: [EXTERNAL] Re: [RFC PATCH 2/4] cmd: fuse: Add fuse writebuff sub-system command

[Harsha Vardhan V M]

On 13/03/25 21:48, Tom Rini wrote:
> On Thu, Mar 13, 2025 at 05:25:15PM +0530, Harsha Vardhan V M wrote:
> 
>> Add CMD_FUSE_WRITEBUFF config option to add and enable fuse writebuff
>> sub-system command. Add fuse_writebuff function to be invoked on
>> writebuff command.
>>
>> Signed-off-by: Harsha Vardhan V M <h-vm@ti.com>
> 
> Pending updates for what I asked in part one,
> 
> Reviewed-by: Tom Rini <trini@konsulko.com>
> 

I assume that by ‘part one,’ you are referring to the question about why 
we don’t use simple_strtoul inline. I’ve addressed that in the other 
thread. Let me know if further clarification is needed.
Thanks.

Re: [EXTERNAL] Re: [RFC PATCH 1/4] cmd: fuse: Remove custom string functions

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2290 bytes --]

On Fri, Mar 14, 2025 at 07:13:19PM +0530, Harsha Vardhan V M wrote:
> 
> 
> On 13/03/25 21:45, Tom Rini wrote:
> > On Thu, Mar 13, 2025 at 05:25:14PM +0530, Harsha Vardhan V M wrote:
> > 
> > > Remove custom string functions and replace them with normal string
> > > functions. Remove the custom strtou32 and replace it with str2long.
> > > 
> > > Signed-off-by: Harsha Vardhan V M <h-vm@ti.com>
> > 
> > Thanks for doing this.
> > 
> > > ---
> > >   cmd/fuse.c | 27 ++++++++-------------------
> > >   1 file changed, 8 insertions(+), 19 deletions(-)
> > > 
> > > diff --git a/cmd/fuse.c b/cmd/fuse.c
> > > index 598ef496a43..9f489570634 100644
> > > --- a/cmd/fuse.c
> > > +++ b/cmd/fuse.c
> > > @@ -15,17 +15,6 @@
> > >   #include <vsprintf.h>
> > >   #include <linux/errno.h>
> > > -static int strtou32(const char *str, unsigned int base, u32 *result)
> > > -{
> > > -	char *ep;
> > > -
> > > -	*result = simple_strtoul(str, &ep, base);
> > > -	if (ep == str || *ep != '\0')
> > > -		return -EINVAL;
> > > -
> > > -	return 0;
> > > -}
> > > -
> > >   static int confirm_prog(void)
> > >   {
> > >   	puts("Warning: Programming fuses is an irreversible operation!\n"
> > > @@ -54,14 +43,14 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
> > >   	argc -= 2 + confirmed;
> > >   	argv += 2 + confirmed;
> > > -	if (argc < 2 || strtou32(argv[0], 0, &bank) ||
> > > -			strtou32(argv[1], 0, &word))
> > > +	if (argc < 2 || !(str2long(argv[0], (ulong *)&bank)) ||
> > > +			!(str2long(argv[1], (ulong *)&word)))
> > 
> > I didn't know we had "str2long" which is a differently rarely used
> > function. Why not just simple_strtoul inline? Am I missing something?
> > Thanks.
> 
> We cannot use simple_strtoul inline directly because we need proper error
> checking to ensure the simple_strtoul conversion was successful. The
> str2long function is a wrapper around simple_strtoul and the necessary error
> checks. Hence, using str2long here.
> Thanks.

I'm sorry I'm still not getting it. We virtually never call str2long. I
was taking a quick look at why if anything the 4 callers in the entire
tree today have more special error checking requirements than every
other command which parses user input.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [RFC PATCH 1/4] cmd: fuse: Remove custom string functions

[Harsha Vardhan V M]

On 14/03/25 19:39, Tom Rini wrote:
> On Fri, Mar 14, 2025 at 07:13:19PM +0530, Harsha Vardhan V M wrote:
>>
>>
>> On 13/03/25 21:45, Tom Rini wrote:
>>> On Thu, Mar 13, 2025 at 05:25:14PM +0530, Harsha Vardhan V M wrote:
>>>
>>>> Remove custom string functions and replace them with normal string
>>>> functions. Remove the custom strtou32 and replace it with str2long.
>>>>
>>>> Signed-off-by: Harsha Vardhan V M <h-vm@ti.com>
>>>
>>> Thanks for doing this.
>>>
>>>> ---
>>>>    cmd/fuse.c | 27 ++++++++-------------------
>>>>    1 file changed, 8 insertions(+), 19 deletions(-)
>>>>
>>>> diff --git a/cmd/fuse.c b/cmd/fuse.c
>>>> index 598ef496a43..9f489570634 100644
>>>> --- a/cmd/fuse.c
>>>> +++ b/cmd/fuse.c
>>>> @@ -15,17 +15,6 @@
>>>>    #include <vsprintf.h>
>>>>    #include <linux/errno.h>
>>>> -static int strtou32(const char *str, unsigned int base, u32 *result)
>>>> -{
>>>> -	char *ep;
>>>> -
>>>> -	*result = simple_strtoul(str, &ep, base);
>>>> -	if (ep == str || *ep != '\0')
>>>> -		return -EINVAL;
>>>> -
>>>> -	return 0;
>>>> -}
>>>> -
>>>>    static int confirm_prog(void)
>>>>    {
>>>>    	puts("Warning: Programming fuses is an irreversible operation!\n"
>>>> @@ -54,14 +43,14 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
>>>>    	argc -= 2 + confirmed;
>>>>    	argv += 2 + confirmed;
>>>> -	if (argc < 2 || strtou32(argv[0], 0, &bank) ||
>>>> -			strtou32(argv[1], 0, &word))
>>>> +	if (argc < 2 || !(str2long(argv[0], (ulong *)&bank)) ||
>>>> +			!(str2long(argv[1], (ulong *)&word)))
>>>
>>> I didn't know we had "str2long" which is a differently rarely used
>>> function. Why not just simple_strtoul inline? Am I missing something?
>>> Thanks.
>>
>> We cannot use simple_strtoul inline directly because we need proper error
>> checking to ensure the simple_strtoul conversion was successful. The
>> str2long function is a wrapper around simple_strtoul and the necessary error
>> checks. Hence, using str2long here.
>> Thanks.
> 
> I'm sorry I'm still not getting it. We virtually never call str2long. I
> was taking a quick look at why if anything the 4 callers in the entire
> tree today have more special error checking requirements than every
> other command which parses user input.
> 

Hi Tom,
I tried to retain the error checking done by the custom strtou32 
function, hence replaced the custom strtou32 function with str2long. I 
did notice that in some other files in the cmd/ directory, 
simple_strtoul is used inline with NULL as the endp parameter, like 
this: simple_strtoul(argv[2], NULL, 16).

Do I proceed with replacing the custom strtou32 function with 
simple_strtoul inline by passing NULL as the endp parameter, similar to 
the example above? Please let me know if you have an alternative suggestion.

Thanks,
Harsha

Re: [RFC PATCH 3/4] drivers: k3_fuse: Add fuse sub-system func calls

[Harsha Vardhan V M]

On 13/03/25 21:49, Tom Rini wrote:
> On Thu, Mar 13, 2025 at 05:25:16PM +0530, Harsha Vardhan V M wrote:
> 
>> Add K3_FUSE config option to add and enable fuse sub-system
>> implementation function calls.
>>
>> Signed-off-by: Harsha Vardhan V M <h-vm@ti.com>
> 
> Reviewed-by: Tom Rini <trini@konsulko.com>
> 
> And can you please file an issue on
> https://source.denx.de/u-boot/u-boot/-/issues/ to further clean-up the
> fuse related Kconfig symbols and logic? What we have today is a bit odd,
> but it's I think easier for me to go and shuffle things than explain how
> I think it should be. Thanks.
> 

Filed an issue here,
https://source.denx.de/u-boot/u-boot/-/issues/48

Thanks,
Harsha

Re: [RFC PATCH 1/4] cmd: fuse: Remove custom string functions

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3193 bytes --]

On Mon, Mar 17, 2025 at 02:29:39PM +0530, Harsha Vardhan V M wrote:
> 
> 
> On 14/03/25 19:39, Tom Rini wrote:
> > On Fri, Mar 14, 2025 at 07:13:19PM +0530, Harsha Vardhan V M wrote:
> > > 
> > > 
> > > On 13/03/25 21:45, Tom Rini wrote:
> > > > On Thu, Mar 13, 2025 at 05:25:14PM +0530, Harsha Vardhan V M wrote:
> > > > 
> > > > > Remove custom string functions and replace them with normal string
> > > > > functions. Remove the custom strtou32 and replace it with str2long.
> > > > > 
> > > > > Signed-off-by: Harsha Vardhan V M <h-vm@ti.com>
> > > > 
> > > > Thanks for doing this.
> > > > 
> > > > > ---
> > > > >    cmd/fuse.c | 27 ++++++++-------------------
> > > > >    1 file changed, 8 insertions(+), 19 deletions(-)
> > > > > 
> > > > > diff --git a/cmd/fuse.c b/cmd/fuse.c
> > > > > index 598ef496a43..9f489570634 100644
> > > > > --- a/cmd/fuse.c
> > > > > +++ b/cmd/fuse.c
> > > > > @@ -15,17 +15,6 @@
> > > > >    #include <vsprintf.h>
> > > > >    #include <linux/errno.h>
> > > > > -static int strtou32(const char *str, unsigned int base, u32 *result)
> > > > > -{
> > > > > -	char *ep;
> > > > > -
> > > > > -	*result = simple_strtoul(str, &ep, base);
> > > > > -	if (ep == str || *ep != '\0')
> > > > > -		return -EINVAL;
> > > > > -
> > > > > -	return 0;
> > > > > -}
> > > > > -
> > > > >    static int confirm_prog(void)
> > > > >    {
> > > > >    	puts("Warning: Programming fuses is an irreversible operation!\n"
> > > > > @@ -54,14 +43,14 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
> > > > >    	argc -= 2 + confirmed;
> > > > >    	argv += 2 + confirmed;
> > > > > -	if (argc < 2 || strtou32(argv[0], 0, &bank) ||
> > > > > -			strtou32(argv[1], 0, &word))
> > > > > +	if (argc < 2 || !(str2long(argv[0], (ulong *)&bank)) ||
> > > > > +			!(str2long(argv[1], (ulong *)&word)))
> > > > 
> > > > I didn't know we had "str2long" which is a differently rarely used
> > > > function. Why not just simple_strtoul inline? Am I missing something?
> > > > Thanks.
> > > 
> > > We cannot use simple_strtoul inline directly because we need proper error
> > > checking to ensure the simple_strtoul conversion was successful. The
> > > str2long function is a wrapper around simple_strtoul and the necessary error
> > > checks. Hence, using str2long here.
> > > Thanks.
> > 
> > I'm sorry I'm still not getting it. We virtually never call str2long. I
> > was taking a quick look at why if anything the 4 callers in the entire
> > tree today have more special error checking requirements than every
> > other command which parses user input.
> > 
> 
> Hi Tom,
> I tried to retain the error checking done by the custom strtou32 function,
> hence replaced the custom strtou32 function with str2long. I did notice that
> in some other files in the cmd/ directory, simple_strtoul is used inline
> with NULL as the endp parameter, like this: simple_strtoul(argv[2], NULL,
> 16).
> 
> Do I proceed with replacing the custom strtou32 function with simple_strtoul
> inline by passing NULL as the endp parameter, similar to the example above?

Yes, that would be good, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]