Hi Marek, > Add two subcommands, rdreg and wrreg, to read and write SPI NOR > registers respectively. The registers are always accessed using > register specific access opcode and arbitrary amount of data are > read from SPI NOR register and stored to memory or loaded from > memory and written to SPI NOR register. This is useful in case > the SPI NOR non-volatile registers have to be configured in some > non-standard way, or when debugging the SPI NOR configuration. > > Example usage: > > Read 6 Bytes of SPI NOR ID (Read ID opcode is 0x9e): > " > => sf rdreg 0x9e $loadaddr 6 > => md.b $loadaddr 6 > 00000000: 20 ba 20 10 44 00 > " > > Read Micron Nonvolatile Configuration Register (opcode 0xb5): > " > => sf rdreg 0xb5 $loadaddr 2 > => md.b $loadaddr 2 > 00000000: ff ff > " > > Write 0xfffe (LSByte first) into Micron Nonvolatile Configuration > Register (opcode 0xb1): > " > => mw.b 0x00000000 0xfe > => mw.b 0x00000001 0xff > => sf wrreg 0xb1 $loadaddr 2 > " > > Signed-off-by: Marek Vasut <[email protected]>
I understand register access from CLI is useful for developing and debugging. However, if we want to make this a generic debugging feature, we need to consider many vendor/chip-specific variations. For example, non-volatile register write require polling the status until completion (which is currently missing from your implementation). Some registers require address cycles for access. Some registers require latency cycles for reads, and so on. Unless we can support these known variations, I think this feature should remain a temporary local modification. Thanks, Takahiro > --- > Cc: Johan Jonker <[email protected]> > Cc: Jungseung Lee <[email protected]> > Cc: Takahiro Kuwano <[email protected]> > Cc: Tom Rini <[email protected]> > Cc: Tudor Ambarus <[email protected]> > Cc: Vignesh R <[email protected]> > Cc: [email protected] > --- > cmd/Kconfig | 7 ++++ > cmd/sf.c | 63 ++++++++++++++++++++++++++++++++++ > drivers/mtd/spi/spi-nor-core.c | 8 ++--- > drivers/mtd/spi/spi-nor-tiny.c | 8 ++--- > include/spi_flash.h | 5 +++ > 5 files changed, 83 insertions(+), 8 deletions(-) > > diff --git a/cmd/Kconfig b/cmd/Kconfig > index ff90a87024c..562f85bead0 100644 > --- a/cmd/Kconfig > +++ b/cmd/Kconfig > @@ -1783,6 +1783,13 @@ config CMD_SF > help > SPI Flash support > > +config CMD_SF_REG > + bool "sf reg - SPI NOR register access" > + depends on CMD_SF > + help > + Provides a way to access SPI flash registers using register access > + specific opcodes. > + > config CMD_SF_TEST > bool "sf test - Allow testing of SPI flash" > depends on CMD_SF > diff --git a/cmd/sf.c b/cmd/sf.c > index b5b7f671fb9..4426440045d 100644 > --- a/cmd/sf.c > +++ b/cmd/sf.c > @@ -541,6 +541,59 @@ static int spi_flash_test(struct spi_flash *flash, > uint8_t *buf, ulong len, > return 0; > } > > +static int do_spi_flash_reg(int argc, char *const argv[], bool rdreg) > +{ > + unsigned long opcode, addr, len; > + struct spi_nor *nor = flash; > + uint8_t *buf; > + char *endp; > + int ret; > + > + if (argc < 4) > + return -1; > + opcode = hextoul(argv[1], &endp); > + if (*argv[1] == 0 || *endp != 0) > + return -1; > + addr = hextoul(argv[2], &endp); > + if (*argv[2] == 0 || *endp != 0) > + return -1; > + len = hextoul(argv[3], &endp); > + if (*argv[3] == 0 || *endp != 0) > + return -1; > + > + buf = map_sysmem(addr, 0); > + if (rdreg) { > + ret = spi_nor_read_reg(nor, opcode, buf, len); > + if (ret) { > + printf("Read command 0x%02lx failed (%d)\n", opcode, > ret); > + goto exit; > + } > + } else { > + ret = write_enable(nor); > + if (ret) { > + printf("Write enable for command 0x%02lx failed > (%d)\n", opcode, ret); > + goto exit; > + } > + > + ret = spi_nor_write_reg(nor, opcode, buf, len); > + if (ret) { > + printf("Write command 0x%02lx failed (%d)\n", opcode, > ret); > + goto exit; > + } > + > + ret = write_disable(nor); > + if (ret) { > + printf("Write disable for command 0x%02lx failed > (%d)\n", opcode, ret); > + goto exit; > + } > + } > + > +exit: > + unmap_sysmem(buf); > + > + return ret ? CMD_RET_FAILURE : CMD_RET_SUCCESS; > +} > + > static int do_spi_flash_test(int argc, char *const argv[]) > { > unsigned long offset; > @@ -614,6 +667,10 @@ static int do_spi_flash(struct cmd_tbl *cmdtp, int flag, > int argc, > ret = do_spi_flash_erase(argc, argv); > else if (IS_ENABLED(CONFIG_SPI_FLASH_LOCK) && strcmp(cmd, "protect") > == 0) > ret = do_spi_protect(argc, argv); > + else if (IS_ENABLED(CONFIG_CMD_SF_REG) && !strcmp(cmd, "rdreg")) > + ret = do_spi_flash_reg(argc, argv, true); > + else if (IS_ENABLED(CONFIG_CMD_SF_REG) && !strcmp(cmd, "wrreg")) > + ret = do_spi_flash_reg(argc, argv, false); > else if (IS_ENABLED(CONFIG_CMD_SF_TEST) && !strcmp(cmd, "test")) > ret = do_spi_flash_test(argc, argv); > else > @@ -641,6 +698,12 @@ U_BOOT_LONGHELP(sf, > "sf protect lock/unlock sector len - protect/unprotect 'len' > bytes starting\n" > " at address 'sector'" > #endif > +#ifdef CONFIG_CMD_SF_REG > + "\nsf rdreg cmd addr len - read SPI NOR > register using 'cmd' opcode\n" > + " and store 'len' > read data at 'addr'\n" > + "sf wrreg cmd addr len - write SPI NOR register > using 'cmd' opcode\n" > + " and load 'len' > write data from 'addr'" > +#endif > #ifdef CONFIG_CMD_SF_TEST > "\nsf test offset len - run a very basic destructive test" > #endif > diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c > index c73f2c7c3f8..eb022e577a2 100644 > --- a/drivers/mtd/spi/spi-nor-core.c > +++ b/drivers/mtd/spi/spi-nor-core.c > @@ -298,7 +298,7 @@ static int spi_nor_read_write_reg(struct spi_nor *nor, > struct spi_mem_op > return spi_mem_exec_op(nor->spi, op); > } > > -static int spi_nor_read_reg(struct spi_nor *nor, u8 code, u8 *val, int len) > +int spi_nor_read_reg(struct spi_nor *nor, u8 code, u8 *val, int len) > { > struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(code, 0), > SPI_MEM_OP_NO_ADDR, > @@ -315,7 +315,7 @@ static int spi_nor_read_reg(struct spi_nor *nor, u8 code, > u8 *val, int len) > return ret; > } > > -static int spi_nor_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int > len) > +int spi_nor_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len) > { > struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(opcode, 0), > SPI_MEM_OP_NO_ADDR, > @@ -637,7 +637,7 @@ static int write_sr3(struct spi_nor *nor, u8 val) > * Set write enable latch with Write Enable command. > * Returns negative if error occurred. > */ > -static int write_enable(struct spi_nor *nor) > +int write_enable(struct spi_nor *nor) > { > return nor->write_reg(nor, SPINOR_OP_WREN, NULL, 0); > } > @@ -645,7 +645,7 @@ static int write_enable(struct spi_nor *nor) > /* > * Send write disable instruction to the chip. > */ > -static int write_disable(struct spi_nor *nor) > +int write_disable(struct spi_nor *nor) > { > return nor->write_reg(nor, SPINOR_OP_WRDI, NULL, 0); > } > diff --git a/drivers/mtd/spi/spi-nor-tiny.c b/drivers/mtd/spi/spi-nor-tiny.c > index cf00473ee83..7b6b7432af8 100644 > --- a/drivers/mtd/spi/spi-nor-tiny.c > +++ b/drivers/mtd/spi/spi-nor-tiny.c > @@ -46,7 +46,7 @@ static int spi_nor_read_write_reg(struct spi_nor *nor, > struct spi_mem_op > return spi_mem_exec_op(nor->spi, op); > } > > -static int spi_nor_read_reg(struct spi_nor *nor, u8 code, u8 *val, int len) > +int spi_nor_read_reg(struct spi_nor *nor, u8 code, u8 *val, int len) > { > struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(code, 1), > SPI_MEM_OP_NO_ADDR, > @@ -72,7 +72,7 @@ static int spi_nor_read_reg(struct spi_nor *nor, u8 code, > u8 *val, int len) > return ret; > } > > -static int spi_nor_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int > len) > +int spi_nor_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len) > { > struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(opcode, 1), > SPI_MEM_OP_NO_ADDR, > @@ -155,7 +155,7 @@ static inline int write_sr(struct spi_nor *nor, u8 val) > * Set write enable latch with Write Enable command. > * Returns negative if error occurred. > */ > -static inline int write_enable(struct spi_nor *nor) > +int write_enable(struct spi_nor *nor) > { > return spi_nor_write_reg(nor, SPINOR_OP_WREN, NULL, 0); > } > @@ -163,7 +163,7 @@ static inline int write_enable(struct spi_nor *nor) > /* > * Send write disable instruction to the chip. > */ > -static inline int write_disable(struct spi_nor *nor) > +int write_disable(struct spi_nor *nor) > { > return spi_nor_write_reg(nor, SPINOR_OP_WRDI, NULL, 0); > } > diff --git a/include/spi_flash.h b/include/spi_flash.h > index 2e703e85250..b7622bce48c 100644 > --- a/include/spi_flash.h > +++ b/include/spi_flash.h > @@ -236,4 +236,9 @@ static inline int spi_flash_protect(struct spi_flash > *flash, u32 ofs, u32 len, > return flash->flash_unlock(flash, ofs, len); > } > > +int write_enable(struct spi_nor *nor); > +int write_disable(struct spi_nor *nor); > +int spi_nor_read_reg(struct spi_nor *nor, u8 code, u8 *val, int len); > +int spi_nor_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len); > + > #endif /* _SPI_FLASH_H_ */ > -- > 2.53.0
