diff mbox

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

Message ID 1038235395.2398352.1344948755663.JavaMail.root@advansee.com
State Superseded
Delegated to: Stefano Babic
Headers show

Commit Message

Benoît Thébaudeau Aug. 14, 2012, 12:52 p.m. UTC
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>
---
 {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

Comments

Stefano Babic Aug. 21, 2012, 8:11 a.m. UTC | #1
On 14/08/2012 14:52, Benoît Thébaudeau wrote:
> 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()

> +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).

Best regards,
Stefano Babic
Benoît Thébaudeau Aug. 21, 2012, 10:14 a.m. UTC | #2
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
Behme Dirk (CM/ESO2) Aug. 22, 2012, 10:43 a.m. UTC | #3
On 14.08.2012 14:52, Benoît Thébaudeau wrote:
> This can be useful for fuse-like hardware, OTP SoC options, etc.

For i.MX6, I have a port of the OTP support from Freescale's U-Boot to 
our mainline U-Boot in the queue [1].

As I don't have the overview over the various i.MXxx SoCs and don't 
understand much of this patch below: Should this implement the same 
functionality like my patch [1] for i.MX6?

Or shall I send my patch [1] to this mailing list for official review 
because the functionality here and there is orthogonal?

Thanks

Dirk

[1] 
https://github.com/dirkbehme/u-boot-imx6/commit/da718b338a79af160f7b7e542fe97b24edbfc36a

> Signed-off-by: Benoît Thébaudeau <benoit.thebaudeau@advansee.com>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Stefano Babic <sbabic@denx.de>
> ---
>  {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
> diff --git u-boot-4d3c95f.orig/common/Makefile u-boot-4d3c95f/common/Makefile
> index 3d62775..44ef757 100644
> --- u-boot-4d3c95f.orig/common/Makefile
> +++ u-boot-4d3c95f/common/Makefile
> @@ -96,6 +96,7 @@ COBJS-$(CONFIG_CMD_FLASH) += cmd_flash.o
>  ifdef CONFIG_FPGA
>  COBJS-$(CONFIG_CMD_FPGA) += cmd_fpga.o
>  endif
> +COBJS-$(CONFIG_CMD_FUSE) += cmd_fuse.o
>  COBJS-$(CONFIG_CMD_GPIO) += cmd_gpio.o
>  COBJS-$(CONFIG_CMD_I2C) += cmd_i2c.o
>  COBJS-$(CONFIG_CMD_IDE) += cmd_ide.o
> diff --git u-boot-4d3c95f/common/cmd_fuse.c u-boot-4d3c95f/common/cmd_fuse.c
> new file mode 100644
> index 0000000..fd54d40
> --- /dev/null
> +++ u-boot-4d3c95f/common/cmd_fuse.c
> @@ -0,0 +1,182 @@
> +/*
> + * (C) Copyright 2009-2012 ADVANSEE
> + * Benoît Thébaudeau <benoit.thebaudeau@advansee.com>
> + *
> + * Based on the mpc512x iim code:
> + * Copyright 2008 Silicon Turnkey Express, Inc.
> + * Martha Marx <mmarx@silicontkx.com>
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#include <common.h>
> +#include <command.h>
> +#include <fuse.h>
> +#include <asm/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 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);
> +               ret = fuse_sense_bit(bank, row, bit, &val);
> +               if (ret)
> +                       goto err;
> +
> +               printf("%u\n", val);
> +       } else if (!strcmp(argv[1], "sense.row")) {
> +               if (argc == 4)
> +                       cnt = 1;
> +               else if (argc != 5 || strtou32(argv[4], 0, &cnt))
> +                       return CMD_RET_USAGE;
> +
> +               printf("Sensing bank %u:\n", bank);
> +               for (i = 0; i < cnt; i++, row++) {
> +                       if (!(i % 4))
> +                               printf("\nRow 0x%.8x:", row);
> +
> +                       ret = fuse_sense_row(bank, row, &val);
> +                       if (ret)
> +                               goto err;
> +
> +                       printf(" %.8x", val);
> +               }
> +               putc('\n');
> +       } else if (!strcmp(argv[1], "prog.bit")) {
> +               if (argc != 5 || strtou32(argv[4], 0, &bit))
> +                       return CMD_RET_USAGE;
> +
> +               printf("Programming bank %u row 0x%.8x bit %u...\n",
> +                               bank, row, bit);
> +               ret = fuse_prog_bit(bank, row, bit);
> +               if (ret)
> +                       goto err;
> +       } else if (!strcmp(argv[1], "prog.row")) {
> +               if (argc < 5)
> +                       return CMD_RET_USAGE;
> +
> +               for (i = 4; i < argc; i++, row++) {
> +                       if (strtou32(argv[i], 16, &val))
> +                               return CMD_RET_USAGE;
> +
> +                       printf("Programming bank %u row 0x%.8x to 0x%.8x...\n",
> +                                       bank, row, val);
> +                       ret = fuse_prog_row(bank, row, val);
> +                       if (ret)
> +                               goto err;
> +               }
> +       } else if (!strcmp(argv[1], "ovride.bit")) {
> +               if (argc != 6 || strtou32(argv[4], 0, &bit) ||
> +                               strtou32(argv[5], 0, &val) || val > 1)
> +                       return CMD_RET_USAGE;
> +
> +               printf("Overriding bank %u row 0x%.8x bit %u with %u...\n",
> +                               bank, row, bit, val);
> +               ret = fuse_override_bit(bank, row, bit, val);
> +               if (ret)
> +                       goto err;
> +       } else if (!strcmp(argv[1], "ovride.row")) {
> +               if (argc < 5)
> +                       return CMD_RET_USAGE;
> +
> +               for (i = 4; i < argc; i++, row++) {
> +                       if (strtou32(argv[i], 16, &val))
> +                               return CMD_RET_USAGE;
> +
> +                       printf("Overriding bank %u row 0x%.8x with 0x%.8x...\n",
> +                                       bank, row, val);
> +                       ret = fuse_override_row(bank, row, val);
> +                       if (ret)
> +                               goto err;
> +               }
> +       } else {
> +               return CMD_RET_USAGE;
> +       }
> +
> +       return 0;
> +
> +err:
> +       puts("ERROR\n");
> +       return ret;
> +}
> +
> +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'"
> +);
> diff --git u-boot-4d3c95f.orig/include/config_cmd_all.h u-boot-4d3c95f/include/config_cmd_all.h
> index f434cd0..8f7d9ae 100644
> --- u-boot-4d3c95f.orig/include/config_cmd_all.h
> +++ u-boot-4d3c95f/include/config_cmd_all.h
> @@ -40,6 +40,7 @@
>  #define CONFIG_CMD_FDOS                /* Floppy DOS support           */
>  #define CONFIG_CMD_FLASH       /* flinfo, erase, protect       */
>  #define CONFIG_CMD_FPGA                /* FPGA configuration Support   */
> +#define CONFIG_CMD_FUSE                /* Device fuse support          */
>  #define CONFIG_CMD_HWFLOW      /* RTS/CTS hw flow control      */
>  #define CONFIG_CMD_I2C         /* I2C serial bus support       */
>  #define CONFIG_CMD_IDE         /* IDE harddisk support         */
> diff --git u-boot-4d3c95f/include/fuse.h u-boot-4d3c95f/include/fuse.h
> new file mode 100644
> index 0000000..baefefe
> --- /dev/null
> +++ u-boot-4d3c95f/include/fuse.h
> @@ -0,0 +1,49 @@
> +/*
> + * (C) Copyright 2009-2012 ADVANSEE
> + * Benoît Thébaudeau <benoit.thebaudeau@advansee.com>
> + *
> + * Based on the mpc512x iim code:
> + * Copyright 2008 Silicon Turnkey Express, Inc.
> + * Martha Marx <mmarx@silicontkx.com>
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#ifndef _FUSE_H_
> +#define _FUSE_H_
> +
> +/*
> + * Read/Sense/Program/Override interface:
> + *   bank:    Fuse bank
> + *   row:     Fuse row within the bank
> + *   bit:     Fuse bit within the row
> + *   val:     Value to read/write
> + *
> + *   Returns: 0 on success, not 0 on failure
> + */
> +int fuse_read_bit(u32 bank, u32 row, u32 bit, u32 *val);
> +int fuse_read_row(u32 bank, u32 row, u32 *val);
> +int fuse_sense_bit(u32 bank, u32 row, u32 bit, u32 *val);
> +int fuse_sense_row(u32 bank, u32 row, u32 *val);
> +int fuse_prog_bit(u32 bank, u32 row, u32 bit);
> +int fuse_prog_row(u32 bank, u32 row, u32 val);
> +int fuse_override_bit(u32 bank, u32 row, u32 bit, u32 val);
> +int fuse_override_row(u32 bank, u32 row, u32 val);
> +
> +#endif /* _FUSE_H_ */
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
Benoît Thébaudeau Aug. 22, 2012, 11:11 a.m. UTC | #4
Hi Dirk,

On Wednesday, August 22, 2012 12:43:05 PM, Dirk Behme wrote:
> On 14.08.2012 14:52, Benoît Thébaudeau wrote:
> > This can be useful for fuse-like hardware, OTP SoC options, etc.
> 
> For i.MX6, I have a port of the OTP support from Freescale's U-Boot
> to
> our mainline U-Boot in the queue [1].
> 
> As I don't have the overview over the various i.MXxx SoCs and don't
> understand much of this patch below: Should this implement the same
> functionality like my patch [1] for i.MX6?
> 
> Or shall I send my patch [1] to this mailing list for official review
> because the functionality here and there is orthogonal?
> 
> Thanks
> 
> Dirk
> 
> [1]
> https://github.com/dirkbehme/u-boot-imx6/commit/da718b338a79af160f7b7e542fe97b24edbfc36a

This OTP IP is different from the IIM IP of other i.MXs, so having a different
driver for it is fine.

What you can do is implement in your driver the fuse API that I've defined in
this series. In that way, you could drop your common/cmd_imxotp.c and use the
commands from this series. Then, you can send the resulting patches for review.

Best regards,
Benoît
Dirk Behme Aug. 22, 2012, 4:25 p.m. UTC | #5
On 22.08.2012 13:11, Benoît Thébaudeau wrote:
> Hi Dirk,
>
> On Wednesday, August 22, 2012 12:43:05 PM, Dirk Behme wrote:
>> On 14.08.2012 14:52, Benoît Thébaudeau wrote:
>>> This can be useful for fuse-like hardware, OTP SoC options, etc.
>>
>> For i.MX6, I have a port of the OTP support from Freescale's U-Boot
>> to
>> our mainline U-Boot in the queue [1].
>>
>> As I don't have the overview over the various i.MXxx SoCs and don't
>> understand much of this patch below: Should this implement the same
>> functionality like my patch [1] for i.MX6?
>>
>> Or shall I send my patch [1] to this mailing list for official review
>> because the functionality here and there is orthogonal?
>>
>> Thanks
>>
>> Dirk
>>
>> [1]
>> https://github.com/dirkbehme/u-boot-imx6/commit/da718b338a79af160f7b7e542fe97b24edbfc36a
>
> This OTP IP is different from the IIM IP of other i.MXs, so having a different
> driver for it is fine.
>
> What you can do is implement in your driver the fuse API

You mean

http://lists.denx.de/pipermail/u-boot/2012-August/130904.html

here, correct?

>that I've defined in
> this series. In that way, you could drop your common/cmd_imxotp.c and use the
> commands from this series.

Let's see how this fits to i.MX6:

My understanding is that the i.MX6 has 128 fuse 'registers' (#define 
IMX_OTP_ADDR_MAX 0x7F), each containing 32 fuses/bits. These 
fuses/bits can be written from 0 -> 1 once.

Looking at your API [2] we could set 'bank' to 0 and interpret 'row' 
as the 'register' number. That might fit. Doing this, 
fuse_read_bit/row() and fuse_prog_bit/row() could be used for i.MX6.

 From i.MX6 point of view, as I don't know your use case, I'm not sure 
what fuse_sense_bit/row() and fuse_override_bit/row() might be good for?

Anybody else: Does this make sense? Or should we keep the i.MX6 
specific common/cmd_imxotp.c (see [1] above)?.

Best regards

Dirk

[2]

+ * Read/Sense/Program/Override interface:
+ *   bank:    Fuse bank
+ *   row:     Fuse row within the bank
+ *   bit:     Fuse bit within the row
+ *   val:     Value to read/write
+ *
+ *   Returns: 0 on success, not 0 on failure
+ */
+int fuse_read_bit(u32 bank, u32 row, u32 bit, u32 *val);
+int fuse_read_row(u32 bank, u32 row, u32 *val);
+int fuse_sense_bit(u32 bank, u32 row, u32 bit, u32 *val);
+int fuse_sense_row(u32 bank, u32 row, u32 *val);
+int fuse_prog_bit(u32 bank, u32 row, u32 bit);
+int fuse_prog_row(u32 bank, u32 row, u32 val);
+int fuse_override_bit(u32 bank, u32 row, u32 bit, u32 val);
+int fuse_override_row(u32 bank, u32 row, u32 val);
Stefano Babic Aug. 23, 2012, 10:31 a.m. UTC | #6
On 22/08/2012 12:43, Dirk Behme wrote:
> On 14.08.2012 14:52, Benoît Thébaudeau wrote:
>> This can be useful for fuse-like hardware, OTP SoC options, etc.
> 
> For i.MX6, I have a port of the OTP support from Freescale's U-Boot to
> our mainline U-Boot in the queue [1].
> 
> As I don't have the overview over the various i.MXxx SoCs and don't
> understand much of this patch below: Should this implement the same
> functionality like my patch [1] for i.MX6?

I have not checked the details. but seeing the code it looks that the
procedure to read / write are different. In this case, a further driver
is ok.

Anyway, you should take a look if your patches can be used on a mxs
(MX28) device, because they should be closer. And then I will not like
to have a driver for each SOC.

Generally, I think we should use the approach of the common command and
a specific fuse implementation. Then this API should be used by your
patches as well.

Best regards,
Stefano
Eric Nelson Aug. 23, 2012, 1:23 p.m. UTC | #7
On 08/23/2012 03:31 AM, Stefano Babic wrote:
> On 22/08/2012 12:43, Dirk Behme wrote:
>> On 14.08.2012 14:52, Benoît Thébaudeau wrote:
>>> This can be useful for fuse-like hardware, OTP SoC options, etc.
>>
>> For i.MX6, I have a port of the OTP support from Freescale's U-Boot to
>> our mainline U-Boot in the queue [1].
>>
>> As I don't have the overview over the various i.MXxx SoCs and don't
>> understand much of this patch below: Should this implement the same
>> functionality like my patch [1] for i.MX6?
>
> I have not checked the details. but seeing the code it looks that the
> procedure to read / write are different. In this case, a further driver
> is ok.
>
> Anyway, you should take a look if your patches can be used on a mxs
> (MX28) device, because they should be closer. And then I will not like
> to have a driver for each SOC.
>
> Generally, I think we should use the approach of the common command and
> a specific fuse implementation. Then this API should be used by your
> patches as well.
>
I agree.

The use of the fuse API will likely result in more code than the
imxotp implementation, and more importantly, it will make the usage
more confusing by introducing terms bank and row.

Reading and writing fuses is probably not an area that we want
confusion.

Regards,


Eric
Benoît Thébaudeau Nov. 26, 2012, 4:03 p.m. UTC | #8
Hi Eric, all,

On Thursday, August 23, 2012 3:23:20 PM, Eric Nelson wrote:
> On 08/23/2012 03:31 AM, Stefano Babic wrote:
> > On 22/08/2012 12:43, Dirk Behme wrote:
> >> On 14.08.2012 14:52, Benoît Thébaudeau wrote:
> >>> This can be useful for fuse-like hardware, OTP SoC options, etc.
> >>
> >> For i.MX6, I have a port of the OTP support from Freescale's
> >> U-Boot to
> >> our mainline U-Boot in the queue [1].
> >>
> >> As I don't have the overview over the various i.MXxx SoCs and
> >> don't
> >> understand much of this patch below: Should this implement the
> >> same
> >> functionality like my patch [1] for i.MX6?
> >
> > I have not checked the details. but seeing the code it looks that
> > the
> > procedure to read / write are different. In this case, a further
> > driver
> > is ok.
> >
> > Anyway, you should take a look if your patches can be used on a mxs
> > (MX28) device, because they should be closer. And then I will not
> > like
> > to have a driver for each SOC.
> >
> > Generally, I think we should use the approach of the common command
> > and
> > a specific fuse implementation. Then this API should be used by
> > your
> > patches as well.
> >
> I agree.
> 
> The use of the fuse API will likely result in more code than the
> imxotp implementation, and more importantly, it will make the usage
> more confusing by introducing terms bank and row.
> 
> Reading and writing fuses is probably not an area that we want
> confusion.

I am looking again into this question now that I have the i.MX6 reference
manual.

I don't see any difference between IIM and OCOTP features:
 - There are still banks: They exist as seen from the controller, even if they
   don't exist physically in the efusebox. See 46.2.1 and OCOTP memory map
   ("Value of OTP Bankx Wordy" shadow registers).
 - Rows are named words.
 - The read operations are read accesses to the shadow registers.
 - The sense operations are direct fuse reads (shadow reload + read), as
   explained by the steps in 46.2.1.2.
 - The prog operations are the programming of the fuses, as explained by the
   steps in 46.2.1.3.
 - The override operations are simple write accesses to the shadow registers, as
   explained in 46.2.1.3.

As to the vocabulary used, the differences are:
 - "row" -> "word".
 - "sense" -> "direct read".

Hence, the fuse API applies very well in this case too.

Best regards,
Benoît
Behme Dirk (CM/ESO2) Nov. 27, 2012, 7:19 a.m. UTC | #9
On 26.11.2012 17:03, Benoît Thébaudeau wrote:
> Hi Eric, all,
> 
> On Thursday, August 23, 2012 3:23:20 PM, Eric Nelson wrote:
>> On 08/23/2012 03:31 AM, Stefano Babic wrote:
>>> On 22/08/2012 12:43, Dirk Behme wrote:
>>>> On 14.08.2012 14:52, Benoît Thébaudeau wrote:
>>>>> This can be useful for fuse-like hardware, OTP SoC options, etc.
>>>> For i.MX6, I have a port of the OTP support from Freescale's
>>>> U-Boot to
>>>> our mainline U-Boot in the queue [1].
>>>>
>>>> As I don't have the overview over the various i.MXxx SoCs and
>>>> don't
>>>> understand much of this patch below: Should this implement the
>>>> same
>>>> functionality like my patch [1] for i.MX6?
>>> I have not checked the details. but seeing the code it looks that
>>> the
>>> procedure to read / write are different. In this case, a further
>>> driver
>>> is ok.
>>>
>>> Anyway, you should take a look if your patches can be used on a mxs
>>> (MX28) device, because they should be closer. And then I will not
>>> like
>>> to have a driver for each SOC.
>>>
>>> Generally, I think we should use the approach of the common command
>>> and
>>> a specific fuse implementation. Then this API should be used by
>>> your
>>> patches as well.
>>>
>> I agree.
>>
>> The use of the fuse API will likely result in more code than the
>> imxotp implementation, and more importantly, it will make the usage
>> more confusing by introducing terms bank and row.
>>
>> Reading and writing fuses is probably not an area that we want
>> confusion.
> 
> I am looking again into this question now that I have the i.MX6 reference
> manual.
> 
> I don't see any difference between IIM and OCOTP features:
>  - There are still banks: They exist as seen from the controller, even if they
>    don't exist physically in the efusebox. See 46.2.1 and OCOTP memory map
>    ("Value of OTP Bankx Wordy" shadow registers).
>  - Rows are named words.
>  - The read operations are read accesses to the shadow registers.
>  - The sense operations are direct fuse reads (shadow reload + read), as
>    explained by the steps in 46.2.1.2.
>  - The prog operations are the programming of the fuses, as explained by the
>    steps in 46.2.1.3.
>  - The override operations are simple write accesses to the shadow registers, as
>    explained in 46.2.1.3.
> 
> As to the vocabulary used, the differences are:
>  - "row" -> "word".
>  - "sense" -> "direct read".
> 
> Hence, the fuse API applies very well in this case too.

Do you like to update/rebase your patches to the latest U-Boot and 
re-send? Maybe it makes sense to discuss your patches again, then?

Eric: What do you think?

Thanks

Dirk
Eric Nelson Nov. 27, 2012, 4:58 p.m. UTC | #10
On 11/27/2012 12:19 AM, Dirk Behme wrote:
> On 26.11.2012 17:03, Benoît Thébaudeau wrote:
>> Hi Eric, all,
>>
>> On Thursday, August 23, 2012 3:23:20 PM, Eric Nelson wrote:
>>> On 08/23/2012 03:31 AM, Stefano Babic wrote:
>>>> On 22/08/2012 12:43, Dirk Behme wrote:
>>>>> On 14.08.2012 14:52, Benoît Thébaudeau wrote:
>>>>>> This can be useful for fuse-like hardware, OTP SoC options, etc.
>>>>> For i.MX6, I have a port of the OTP support from Freescale's
>>>>> U-Boot to
>>>>> our mainline U-Boot in the queue [1].
>>>>>
>>>>> As I don't have the overview over the various i.MXxx SoCs and
>>>>> don't
>>>>> understand much of this patch below: Should this implement the
>>>>> same
>>>>> functionality like my patch [1] for i.MX6?
>>>> I have not checked the details. but seeing the code it looks that
>>>> the
>>>> procedure to read / write are different. In this case, a further
>>>> driver
>>>> is ok.
>>>>
>>>> Anyway, you should take a look if your patches can be used on a mxs
>>>> (MX28) device, because they should be closer. And then I will not
>>>> like
>>>> to have a driver for each SOC.
>>>>
>>>> Generally, I think we should use the approach of the common command
>>>> and
>>>> a specific fuse implementation. Then this API should be used by
>>>> your
>>>> patches as well.
>>>>
>>> I agree.
>>>
>>> The use of the fuse API will likely result in more code than the
>>> imxotp implementation, and more importantly, it will make the usage
>>> more confusing by introducing terms bank and row.
>>>
>>> Reading and writing fuses is probably not an area that we want
>>> confusion.
>>
>> I am looking again into this question now that I have the i.MX6 reference
>> manual.
>>
>> I don't see any difference between IIM and OCOTP features:
>>  - There are still banks: They exist as seen from the controller, even
>> if they
>>    don't exist physically in the efusebox. See 46.2.1 and OCOTP memory
>> map
>>    ("Value of OTP Bankx Wordy" shadow registers).
>>  - Rows are named words.
>>  - The read operations are read accesses to the shadow registers.
>>  - The sense operations are direct fuse reads (shadow reload + read), as
>>    explained by the steps in 46.2.1.2.
>>  - The prog operations are the programming of the fuses, as explained
>> by the
>>    steps in 46.2.1.3.
>>  - The override operations are simple write accesses to the shadow
>> registers, as
>>    explained in 46.2.1.3.
>>
>> As to the vocabulary used, the differences are:
>>  - "row" -> "word".
>>  - "sense" -> "direct read".
>>
>> Hence, the fuse API applies very well in this case too.
>
> Do you like to update/rebase your patches to the latest U-Boot and
> re-send? Maybe it makes sense to discuss your patches again, then?
>
> Eric: What do you think?
>

Hi Benoit and Dirk,

I don't have strong feelings one way or the other. If left to
me, I'd probably stick with the imxotp command, but Benoît has
clearly walked through the details and we're not talking about
a lot of code either way.

Either way, this needs a general 'readme' for the fuse command
conventions and some per-arch documentation about how to translate
between the reference manual and the command.

It also seems appropriate to have documentation somewhere about
the conventions used for things like mac address storage. AFAIK,
the only place to find the mapping of OTP locations used for this
is in the various bits of code that implement it.

Regards,


Eric
Benoît Thébaudeau Nov. 27, 2012, 6:27 p.m. UTC | #11
Hi Eric, Dirk,

On Tuesday, November 27, 2012 5:58:19 PM, Eric Nelson wrote:
> I don't have strong feelings one way or the other. If left to
> me, I'd probably stick with the imxotp command, but Benoît has
> clearly walked through the details and we're not talking about
> a lot of code either way.

OK. The more code is in common, the easier it is to maintain.

> Either way, this needs a general 'readme' for the fuse command
> conventions

Sure. I'll add a doc/README.fuse to v3.

> and some per-arch documentation about how to translate
> between the reference manual and the command.

Yes. Should this documentation be located in the README.fuse, or in separate
README files, or in the drivers themselves, e.g. fsl_iim.c?

> It also seems appropriate to have documentation somewhere about
> the conventions used for things like mac address storage. AFAIK,
> the only place to find the mapping of OTP locations used for this
> is in the various bits of code that implement it.

Yes. Currently, it can be found in the various imx-regs.h, in the
fuse_bankx_regs structures. This information could be added to README.imx*.

There is also an imx_get_mac_from_fuse() function implemented for each i.MX SoC
to load the MAC address from the fuses for the FEC. Perhaps a similar function
with a default implementation could be created by each applicable SoC and used
by a new command in cmd_fuse.c to handle MAC addresses read/write operations.
But this would perhaps be too specific for cmd_fuse.c. It might be better to
either have only the README.imx* documentation, or to have SoC-specific commands
for the MAC addresses, like for the clock commands. What do you think?

Best regards,
Benoît
Eric Nelson Nov. 27, 2012, 6:36 p.m. UTC | #12
Hi Benoît,

On 11/27/2012 11:27 AM, Benoît Thébaudeau wrote:
> Hi Eric, Dirk,
>
> On Tuesday, November 27, 2012 5:58:19 PM, Eric Nelson wrote:
>> I don't have strong feelings one way or the other. If left to
>> me, I'd probably stick with the imxotp command, but Benoît has
>> clearly walked through the details and we're not talking about
>> a lot of code either way.
>
> OK. The more code is in common, the easier it is to maintain.
>
>> Either way, this needs a general 'readme' for the fuse command
>> conventions
>
> Sure. I'll add a doc/README.fuse to v3.
>
>> and some per-arch documentation about how to translate
>> between the reference manual and the command.
>
> Yes. Should this documentation be located in the README.fuse, or in separate
> README files, or in the drivers themselves, e.g. fsl_iim.c?
>

I think doc/readme.imx* is the right place.

Oops. It looks like we need one of these for i.MX6.

>> It also seems appropriate to have documentation somewhere about
>> the conventions used for things like mac address storage. AFAIK,
>> the only place to find the mapping of OTP locations used for this
>> is in the various bits of code that implement it.
>
> Yes. Currently, it can be found in the various imx-regs.h, in the
> fuse_bankx_regs structures. This information could be added to README.imx*.
>
> There is also an imx_get_mac_from_fuse() function implemented for each i.MX SoC
> to load the MAC address from the fuses for the FEC. Perhaps a similar function
> with a default implementation could be created by each applicable SoC and used
> by a new command in cmd_fuse.c to handle MAC addresses read/write operations.
> But this would perhaps be too specific for cmd_fuse.c. It might be better to
> either have only the README.imx* documentation, or to have SoC-specific commands
> for the MAC addresses, like for the clock commands. What do you think?
>

Since this really only hits us board vendors, and is mostly used
in the production process, it's probably overkill to implement a command
to make it easier.

> Best regards,
> Benoît
>

Regards,


Eric
Benoît Thébaudeau Nov. 27, 2012, 7:23 p.m. UTC | #13
Hi Eric,

On Tuesday, November 27, 2012 7:36:41 PM, Eric Nelson wrote:
> Hi Benoît,
> 
> On 11/27/2012 11:27 AM, Benoît Thébaudeau wrote:
> > Hi Eric, Dirk,
> >
> >> and some per-arch documentation about how to translate
> >> between the reference manual and the command.
> >
> > Yes. Should this documentation be located in the README.fuse, or in
> > separate
> > README files, or in the drivers themselves, e.g. fsl_iim.c?
> >
> 
> I think doc/readme.imx* is the right place.

MPC/i.MX25/35/51/53 would have the same documentation here since they all use
the IIM. On the other side, there would be the i.MX6 with the OCOTP. So this
part of the documentation is rather IP-specific than SoC- or platform-specific.
Hence, I think that the right place would be either README.fuse, or
README.fsl_iim/README.fsl_ocotp[/...], or fsl_iim.c/fsl_ocotp.c[/...].

Best regards,
Benoît
Eric Nelson Nov. 27, 2012, 7:54 p.m. UTC | #14
On 11/27/2012 12:23 PM, Benoît Thébaudeau wrote:
> Hi Eric,
>
> On Tuesday, November 27, 2012 7:36:41 PM, Eric Nelson wrote:
>> Hi Benoît,
>>
>> On 11/27/2012 11:27 AM, Benoît Thébaudeau wrote:
>>> Hi Eric, Dirk,
>>>
>>>> and some per-arch documentation about how to translate
>>>> between the reference manual and the command.
>>>
>>> Yes. Should this documentation be located in the README.fuse, or in
>>> separate
>>> README files, or in the drivers themselves, e.g. fsl_iim.c?
>>>
>>
>> I think doc/readme.imx* is the right place.
>
> MPC/i.MX25/35/51/53 would have the same documentation here since they all use
> the IIM. On the other side, there would be the i.MX6 with the OCOTP. So this
> part of the documentation is rather IP-specific than SoC- or platform-specific.
> Hence, I think that the right place would be either README.fuse, or
> README.fsl_iim/README.fsl_ocotp[/...], or fsl_iim.c/fsl_ocotp.c[/...].
>

Sounds good to me.
Stefano Babic Dec. 3, 2012, 9:03 a.m. UTC | #15
On 14/08/2012 14:52, Benoît Thébaudeau wrote:
> 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>
> ---

Hi Benoît,

I agree with you that same code can be used then for i.MX6. There is no
need of a parallel development.

I am thinking about if we need some kind of protection to avoid to
destroy very easy the SOC. Running a fuse command can be much more
dangerous as scrubbing NAND or erasing flash.

What about to add at least a "-y" option when we write the fuses, in the
same way we have for the "nand scrub command" ? This does not forbid to
use the command in scripts, but it can maybe avoid to destroy the SOC
when the command is wrong tipped on the console.

What do you think ?

Best regards,
Stefano Babic
Benoît Thébaudeau Dec. 3, 2012, 11:25 a.m. UTC | #16
Hi Stefano,

On Monday, December 3, 2012 10:03:22 AM, Stefano Babic wrote:
> I am thinking about if we need some kind of protection to avoid to
> destroy very easy the SOC. Running a fuse command can be much more
> dangerous as scrubbing NAND or erasing flash.
> 
> What about to add at least a "-y" option when we write the fuses, in
> the
> same way we have for the "nand scrub command" ? This does not forbid
> to
> use the command in scripts, but it can maybe avoid to destroy the SOC
> when the command is wrong tipped on the console.
> 
> What do you think ?

That makes sense. There could be the same kind of warning confirmation
defaulting to no that would repeat which fuse is about to be programmed.

As to the value passed for fuse row programming or overriding, do you think that
it's fine to force it to be hex, or that choice should be given with '0x'? I
don't really like implicit base selection, but it's clear in the help message,
and it is handy for writing MAC addresses without repeating the '0x' for each
byte.

Best regards,
Benoît
Stefano Babic Dec. 3, 2012, 11:41 a.m. UTC | #17
On 03/12/2012 12:25, Benoît Thébaudeau wrote:
> Hi Stefano,
> 
> On Monday, December 3, 2012 10:03:22 AM, Stefano Babic wrote:
>> I am thinking about if we need some kind of protection to avoid to
>> destroy very easy the SOC. Running a fuse command can be much more
>> dangerous as scrubbing NAND or erasing flash.
>>
>> What about to add at least a "-y" option when we write the fuses, in
>> the
>> same way we have for the "nand scrub command" ? This does not forbid
>> to
>> use the command in scripts, but it can maybe avoid to destroy the SOC
>> when the command is wrong tipped on the console.
>>
>> What do you think ?
> 
> That makes sense. There could be the same kind of warning confirmation
> defaulting to no that would repeat which fuse is about to be programmed.
> 
> As to the value passed for fuse row programming or overriding, do you think that
> it's fine to force it to be hex, or that choice should be given with '0x'? I
> don't really like implicit base selection, but it's clear in the help message,
> and it is handy for writing MAC addresses without repeating the '0x' for each
> byte.

U-boot uses as default hex as base for numbers, not decimals, and if you
use hex as default you are consistent with the rest of code.

Best regards,
Stefano
diff mbox

Patch

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
diff --git u-boot-4d3c95f.orig/common/Makefile u-boot-4d3c95f/common/Makefile
index 3d62775..44ef757 100644
--- u-boot-4d3c95f.orig/common/Makefile
+++ u-boot-4d3c95f/common/Makefile
@@ -96,6 +96,7 @@  COBJS-$(CONFIG_CMD_FLASH) += cmd_flash.o
 ifdef CONFIG_FPGA
 COBJS-$(CONFIG_CMD_FPGA) += cmd_fpga.o
 endif
+COBJS-$(CONFIG_CMD_FUSE) += cmd_fuse.o
 COBJS-$(CONFIG_CMD_GPIO) += cmd_gpio.o
 COBJS-$(CONFIG_CMD_I2C) += cmd_i2c.o
 COBJS-$(CONFIG_CMD_IDE) += cmd_ide.o
diff --git u-boot-4d3c95f/common/cmd_fuse.c u-boot-4d3c95f/common/cmd_fuse.c
new file mode 100644
index 0000000..fd54d40
--- /dev/null
+++ u-boot-4d3c95f/common/cmd_fuse.c
@@ -0,0 +1,182 @@ 
+/*
+ * (C) Copyright 2009-2012 ADVANSEE
+ * Benoît Thébaudeau <benoit.thebaudeau@advansee.com>
+ *
+ * Based on the mpc512x iim code:
+ * Copyright 2008 Silicon Turnkey Express, Inc.
+ * Martha Marx <mmarx@silicontkx.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <command.h>
+#include <fuse.h>
+#include <asm/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 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);
+		ret = fuse_sense_bit(bank, row, bit, &val);
+		if (ret)
+			goto err;
+
+		printf("%u\n", val);
+	} else if (!strcmp(argv[1], "sense.row")) {
+		if (argc == 4)
+			cnt = 1;
+		else if (argc != 5 || strtou32(argv[4], 0, &cnt))
+			return CMD_RET_USAGE;
+
+		printf("Sensing bank %u:\n", bank);
+		for (i = 0; i < cnt; i++, row++) {
+			if (!(i % 4))
+				printf("\nRow 0x%.8x:", row);
+
+			ret = fuse_sense_row(bank, row, &val);
+			if (ret)
+				goto err;
+
+			printf(" %.8x", val);
+		}
+		putc('\n');
+	} else if (!strcmp(argv[1], "prog.bit")) {
+		if (argc != 5 || strtou32(argv[4], 0, &bit))
+			return CMD_RET_USAGE;
+
+		printf("Programming bank %u row 0x%.8x bit %u...\n",
+				bank, row, bit);
+		ret = fuse_prog_bit(bank, row, bit);
+		if (ret)
+			goto err;
+	} else if (!strcmp(argv[1], "prog.row")) {
+		if (argc < 5)
+			return CMD_RET_USAGE;
+
+		for (i = 4; i < argc; i++, row++) {
+			if (strtou32(argv[i], 16, &val))
+				return CMD_RET_USAGE;
+
+			printf("Programming bank %u row 0x%.8x to 0x%.8x...\n",
+					bank, row, val);
+			ret = fuse_prog_row(bank, row, val);
+			if (ret)
+				goto err;
+		}
+	} else if (!strcmp(argv[1], "ovride.bit")) {
+		if (argc != 6 || strtou32(argv[4], 0, &bit) ||
+				strtou32(argv[5], 0, &val) || val > 1)
+			return CMD_RET_USAGE;
+
+		printf("Overriding bank %u row 0x%.8x bit %u with %u...\n",
+				bank, row, bit, val);
+		ret = fuse_override_bit(bank, row, bit, val);
+		if (ret)
+			goto err;
+	} else if (!strcmp(argv[1], "ovride.row")) {
+		if (argc < 5)
+			return CMD_RET_USAGE;
+
+		for (i = 4; i < argc; i++, row++) {
+			if (strtou32(argv[i], 16, &val))
+				return CMD_RET_USAGE;
+
+			printf("Overriding bank %u row 0x%.8x with 0x%.8x...\n",
+					bank, row, val);
+			ret = fuse_override_row(bank, row, val);
+			if (ret)
+				goto err;
+		}
+	} else {
+		return CMD_RET_USAGE;
+	}
+
+	return 0;
+
+err:
+	puts("ERROR\n");
+	return ret;
+}
+
+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'"
+);
diff --git u-boot-4d3c95f.orig/include/config_cmd_all.h u-boot-4d3c95f/include/config_cmd_all.h
index f434cd0..8f7d9ae 100644
--- u-boot-4d3c95f.orig/include/config_cmd_all.h
+++ u-boot-4d3c95f/include/config_cmd_all.h
@@ -40,6 +40,7 @@ 
 #define CONFIG_CMD_FDOS		/* Floppy DOS support		*/
 #define CONFIG_CMD_FLASH	/* flinfo, erase, protect	*/
 #define CONFIG_CMD_FPGA		/* FPGA configuration Support	*/
+#define CONFIG_CMD_FUSE		/* Device fuse support		*/
 #define CONFIG_CMD_HWFLOW	/* RTS/CTS hw flow control	*/
 #define CONFIG_CMD_I2C		/* I2C serial bus support	*/
 #define CONFIG_CMD_IDE		/* IDE harddisk support		*/
diff --git u-boot-4d3c95f/include/fuse.h u-boot-4d3c95f/include/fuse.h
new file mode 100644
index 0000000..baefefe
--- /dev/null
+++ u-boot-4d3c95f/include/fuse.h
@@ -0,0 +1,49 @@ 
+/*
+ * (C) Copyright 2009-2012 ADVANSEE
+ * Benoît Thébaudeau <benoit.thebaudeau@advansee.com>
+ *
+ * Based on the mpc512x iim code:
+ * Copyright 2008 Silicon Turnkey Express, Inc.
+ * Martha Marx <mmarx@silicontkx.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef _FUSE_H_
+#define _FUSE_H_
+
+/*
+ * Read/Sense/Program/Override interface:
+ *   bank:    Fuse bank
+ *   row:     Fuse row within the bank
+ *   bit:     Fuse bit within the row
+ *   val:     Value to read/write
+ *
+ *   Returns: 0 on success, not 0 on failure
+ */
+int fuse_read_bit(u32 bank, u32 row, u32 bit, u32 *val);
+int fuse_read_row(u32 bank, u32 row, u32 *val);
+int fuse_sense_bit(u32 bank, u32 row, u32 bit, u32 *val);
+int fuse_sense_row(u32 bank, u32 row, u32 *val);
+int fuse_prog_bit(u32 bank, u32 row, u32 bit);
+int fuse_prog_row(u32 bank, u32 row, u32 val);
+int fuse_override_bit(u32 bank, u32 row, u32 bit, u32 val);
+int fuse_override_row(u32 bank, u32 row, u32 val);
+
+#endif	/* _FUSE_H_ */