[U-Boot] [PATCH 3/5] Add fuse API and commands

["Benoît Thébaudeau" <benoit.thebaudeau@advansee.com>]

Hi Stefano,

> > This can be useful for fuse-like hardware, OTP SoC options, etc.
> > 
> > Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>
> > Cc: Wolfgang Denk <wd@denx.de>
> > Cc: Stefano Babic <sbabic@denx.de>
> > ---
> 
> CC to Anatolji, he knows very well the MPC5121 that has currently
> support of fuses.
> 
> >  {u-boot-4d3c95f.orig => u-boot-4d3c95f}/README     |    1 +
> >  .../common/Makefile                                |    1 +
> >  /dev/null => u-boot-4d3c95f/common/cmd_fuse.c      |  182
> >  ++++++++++++++++++++
> >  .../include/config_cmd_all.h                       |    1 +
> >  /dev/null => u-boot-4d3c95f/include/fuse.h         |   49 ++++++
> >  5 files changed, 234 insertions(+)
> >  create mode 100644 u-boot-4d3c95f/common/cmd_fuse.c
> >  create mode 100644 u-boot-4d3c95f/include/fuse.h
> > 
> > diff --git u-boot-4d3c95f.orig/README u-boot-4d3c95f/README
> > index fb9d904..c40fd34 100644
> > --- u-boot-4d3c95f.orig/README
> > +++ u-boot-4d3c95f/README
> > @@ -780,6 +780,7 @@ The following options need to be configured:
> >  		CONFIG_CMD_FDOS		* Dos diskette Support
> >  		CONFIG_CMD_FLASH	  flinfo, erase, protect
> >  		CONFIG_CMD_FPGA		  FPGA device initialization support
> > +		CONFIG_CMD_FUSE		  Device fuse support
> >  		CONFIG_CMD_GO		* the 'go' command (exec code)
> >  		CONFIG_CMD_GREPENV	* search environment
> >  		CONFIG_CMD_HWFLOW	* RTS/CTS hw flow control
> 
> Agree in this split: we have a general fuse command and each SOC /
> SOC
> family can add its own implementation.
> 
> > +static int do_fuse(cmd_tbl_t *cmdtp, int flag, int argc, char
> > *const argv[])
> > +{
> > +	u32 bank, row, bit, cnt, val;
> > +	int ret, i;
> > +
> > +	if (argc < 4 || strtou32(argv[2], 0, &bank) ||
> > +			strtou32(argv[3], 0, &row))
> > +		return CMD_RET_USAGE;
> > +
> > +	if (!strcmp(argv[1], "read.bit")) {
> > +		if (argc != 5 || strtou32(argv[4], 0, &bit))
> > +			return CMD_RET_USAGE;
> > +
> > +		printf("Reading bank %u row 0x%.8x bit %u: ", bank, row, bit);
> > +		ret = fuse_read_bit(bank, row, bit, &val);
> > +		if (ret)
> > +			goto err;
> > +
> > +		printf("%u\n", val);
> > +	} else if (!strcmp(argv[1], "read.row")) {
> > +		if (argc == 4)
> > +			cnt = 1;
> > +		else if (argc != 5 || strtou32(argv[4], 0, &cnt))
> > +			return CMD_RET_USAGE;
> > +
> > +		printf("Reading bank %u:\n", bank);
> > +		for (i = 0; i < cnt; i++, row++) {
> > +			if (!(i % 4))
> > +				printf("\nRow 0x%.8x:", row);
> > +
> > +			ret = fuse_read_row(bank, row, &val);
> > +			if (ret)
> > +				goto err;
> > +
> > +			printf(" %.8x", val);
> > +		}
> > +		putc('\n');
> > +	} else if (!strcmp(argv[1], "sense.bit")) {
> > +		if (argc != 5 || strtou32(argv[4], 0, &bit))
> > +			return CMD_RET_USAGE;
> > +
> > +		printf("Sensing bank %u row 0x%.8x bit %u: ", bank, row, bit);
> 
> Each command sends this output to the console. I am thinking about if
> instead of printf() we shoud use debug()

It's only a preamble to the result string. It's useful as feedback to the user
to confirm addresses and values (hex or not, etc.). There is also this kind of
indication for some existing commands, like md that confirms addresses. IMHO,
it's better as printf() than as debug(), all the more fuse stuff is sensible.

> > +U_BOOT_CMD(
> > +	fuse, CONFIG_SYS_MAXARGS, 0, do_fuse,
> > +	"Fuse sub-system",
> > +	     "read.bit <bank> <row> <bit> - read a fuse bit\n"
> > +	"fuse read.row <bank> <row> [<cnt>] - read 1 or 'cnt' fuse
> > rows,\n"
> > +	"    starting at 'row'\n"
> > +	"fuse sense.bit <bank> <row> <bit> - sense a fuse bit\n"
> > +	"fuse sense.row <bank> <row> [<cnt>] - sense 1 or 'cnt' fuse
> > rows,\n"
> > +	"    starting at 'row'\n"
> > +	"fuse prog.bit <bank> <row> <bit> - program a fuse bit
> > (PERMANENT)\n"
> > +	"fuse prog.row <bank> <row> <hexval> [<hexval>...] - program 1
> > or\n"
> > +	"    several fuse rows, starting at 'row' (PERMANENT)\n"
> > +	"fuse ovride.bit <bank> <row> <bit> <val> - override a fuse
> > bit\n"
> > +	"fuse ovride.row <bank> <row> <hexval> [<hexval>...] - override 1
> > or\n"
> > +	"    several fuse rows, starting at 'row'"
> > +);
> 
> General question: why do we need the "bit" interface ? I have thought
> it
> is enough the read row / prog row interface (even if there is a bit
> programming).

For prog, it corresponds to the hardware API (at least on FSL IIM). It is also a
matter of safety. Fuse operations are sensible, irreversible and operate on
single bits, so it is less error prone for users to concentrate on the bit that
they want to program.

For read/sense/override, it is mostly for API consistency with prog.

For all these commands, this is also useful to easily automate accesses to fuse
bits from command line, e.g. for end-of-line programming of boards.

If some hardware implementations are more bit- than byte-oriented or the
opposite, it also makes the API more flexible.

Best regards,
Beno?t