[U-Boot] [PATCH v2 1/5] spi_sf: Skip the erasing of protected sectors
Many SPI flashes have protection bits (BP2, BP1 and BP0) in the status register that can protect selected regions of the SPI NOR.
Take these bits into account when performing erase operations, making sure that the protected areas are skipped.
Introduce the CONFIG_SPI_NOR_PROTECTION_STM option that can be selected by systems that want to protect selected regions of SPI NOR flash using the same programming model as in the ST Micro SPI NOR flashes, like for example M25P32.
Cc: Jagan Teki jteki@openedev.com Signed-off-by: Otavio Salvador otavio@ossystems.com.br Tested-by: Fabio Estevam fabio.estevam@freescale.com --- Changes since v1: - Use CONFIG_SPI_NOR_PROTECTION_STM to make clear that the bit protection bits follow the same programming model as in the ST Micro SPI NOR flashes like M25P32. - Improve the README entry addressing the questions made by Jagan
README | 14 +++++++ drivers/mtd/spi/sf_ops.c | 91 ++++++++++++++++++++++++++++++++++++++++++++-- drivers/mtd/spi/sf_probe.c | 2 + 3 files changed, 104 insertions(+), 3 deletions(-)
diff --git a/README b/README index 1acc355..a45dfcd 100644 --- a/README +++ b/README @@ -2751,6 +2751,20 @@ CBFS (Coreboot Filesystem) support Timeout for waiting until spi transfer completed. default: (CONFIG_SYS_HZ/100) /* 10 ms */
+ CONFIG_SPI_NOR_PROTECTION_STM + Enable the built-in protection mechanism provided by the + BP2, BP1 and BP0 bits from the status register present + on ST-Micro flashes such as M25P32. Please refer to the + M25P32 datasheet to understand how to program these bits + in order to protect a selected region of the SPI NOR flash. + This same bit protection programming model applies to SPI + NOR flashes from other manufacturers such as: + - Micron M25P32 + - SST SST25VF032B + In order to program the bit protection bits of the status + register the spi_flash_cmd_write_status() function can be + used. + - FPGA Support: CONFIG_FPGA
Enables FPGA subsystem. diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c index 900ec1f..e53dcdc 100644 --- a/drivers/mtd/spi/sf_ops.c +++ b/drivers/mtd/spi/sf_ops.c @@ -264,6 +264,84 @@ int spi_flash_write_common(struct spi_flash *flash, const u8 *cmd, return ret; }
+#ifdef CONFIG_SPI_NOR_PROTECTION_STM +static int is_protected(struct spi_flash *flash, u32 offset) +{ + int total_sector, ret, protected_sector; + bool *protected; + u8 sr; + + total_sector = flash->size / flash->erase_size; + protected = calloc(1, total_sector); + if (!protected) + return -ENOMEM; + + ret = spi_flash_cmd_read_status(flash, &sr); + if (ret < 0) + goto free_protected; + + /* Extract the protection bits: BP2, BP1 and BP0 */ + sr = (sr & 0x1c) >> 2; + + switch (sr) { + case 0: + protected_sector = 0; + break; + + case 1: + protected_sector = total_sector / 64; + break; + + case 2: + protected_sector = total_sector / 32; + break; + + case 3: + protected_sector = total_sector / 16; + break; + + case 4: + protected_sector = total_sector / 8; + break; + + case 5: + protected_sector = total_sector / 4; + break; + + case 6: + protected_sector = total_sector / 2; + break; + + case 7: + protected_sector = total_sector; + break; + + default: + ret = -EINVAL; + goto free_protected; + } + + while (protected_sector) { + protected[total_sector - protected_sector] = 1; + protected_sector--; + } + + if (protected[offset/flash->erase_size]) + return 1; + else + return 0; + +free_protected: + free(protected); + return ret; +} +#else +static int is_protected(struct spi_flash *flash, u32 offset) +{ + return 0; +} +#endif + int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len) { u32 erase_size, erase_addr; @@ -294,10 +372,17 @@ int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len) debug("SF: erase %2x %2x %2x %2x (%x)\n", cmd[0], cmd[1], cmd[2], cmd[3], erase_addr);
- ret = spi_flash_write_common(flash, cmd, sizeof(cmd), NULL, 0); - if (ret < 0) { - debug("SF: erase failed\n"); + if (is_protected(flash, offset) > 0) { + printf("Skipping to erase protected offset 0x%x\n", + offset); break; + } else { + ret = spi_flash_write_common(flash, cmd, sizeof(cmd), + NULL, 0); + if (ret < 0) { + debug("SF: erase failed\n"); + break; + } }
offset += erase_size; diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c index 954376d..c1d188f 100644 --- a/drivers/mtd/spi/sf_probe.c +++ b/drivers/mtd/spi/sf_probe.c @@ -256,12 +256,14 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode, } #endif
+#if !defined(CONFIG_SPI_NOR_PROTECTION_STM) /* Flash powers up read-only, so clear BP# bits */ #if defined(CONFIG_SPI_FLASH_ATMEL) || \ defined(CONFIG_SPI_FLASH_MACRONIX) || \ defined(CONFIG_SPI_FLASH_SST) spi_flash_cmd_write_status(flash, 0); #endif +#endif
return 0; }
Add a prototype for spi_flash_cmd_write_status(), so that it can be used by other files.
Cc: Jagan Teki jteki@openedev.com Signed-off-by: Otavio Salvador otavio@ossystems.com.br --- Changes since v1: - None
include/spi_flash.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/include/spi_flash.h b/include/spi_flash.h index 3b2d555..4b13926 100644 --- a/include/spi_flash.h +++ b/include/spi_flash.h @@ -232,4 +232,6 @@ static inline int spi_flash_erase(struct spi_flash *flash, u32 offset, void spi_boot(void) __noreturn; void spi_spl_load_image(uint32_t offs, unsigned int size, void *vdst);
+int spi_flash_cmd_write_status(struct spi_flash *flash, u8 ws); + #endif /* _SPI_FLASH_H_ */
Add SPI NOR support:
=> sf probe SF: Detected SST25VF032B with page size 256 Bytes, erase size 4 KiB, total 4 MiB
Signed-off-by: Otavio Salvador otavio@ossystems.com.br --- Changes since v1: - None
board/congatec/cgtqmx6eval/cgtqmx6eval.c | 23 +++++++++++++++++++++++ include/configs/cgtqmx6eval.h | 10 ++++++++++ 2 files changed, 33 insertions(+)
diff --git a/board/congatec/cgtqmx6eval/cgtqmx6eval.c b/board/congatec/cgtqmx6eval/cgtqmx6eval.c index 574891e..a9694d5 100644 --- a/board/congatec/cgtqmx6eval/cgtqmx6eval.c +++ b/board/congatec/cgtqmx6eval/cgtqmx6eval.c @@ -45,6 +45,10 @@ DECLARE_GLOBAL_DATA_PTR; PAD_CTL_DSE_40ohm | PAD_CTL_HYS | \ PAD_CTL_ODE | PAD_CTL_SRE_FAST)
+#define SPI_PAD_CTRL (PAD_CTL_HYS | \ + PAD_CTL_SPEED_MED | \ + PAD_CTL_DSE_40ohm | PAD_CTL_SRE_FAST) + #define MX6Q_QMX6_PFUZE_MUX IMX_GPIO_NR(6, 9)
@@ -152,6 +156,13 @@ static iomux_v3_cfg_t enet_pads_ar8035[] = { MX6_PAD_RGMII_RX_CTL__RGMII_RX_CTL | MUX_PAD_CTRL(ENET_PAD_CTRL), };
+static iomux_v3_cfg_t const ecspi1_pads[] = { + MX6_PAD_EIM_D16__ECSPI1_SCLK | MUX_PAD_CTRL(SPI_PAD_CTRL), + MX6_PAD_EIM_D17__ECSPI1_MISO | MUX_PAD_CTRL(SPI_PAD_CTRL), + MX6_PAD_EIM_D18__ECSPI1_MOSI | MUX_PAD_CTRL(SPI_PAD_CTRL), + MX6_PAD_EIM_D19__GPIO3_IO19 | MUX_PAD_CTRL(NO_PAD_CTRL), +}; + #define PC MUX_PAD_CTRL(I2C_PAD_CTRL) struct i2c_pads_info i2c_pad_info1 = { .scl = { @@ -382,6 +393,12 @@ static void setup_iomux_uart(void) imx_iomux_v3_setup_multiple_pads(uart2_pads, ARRAY_SIZE(uart2_pads)); }
+void setup_spinor(void) +{ + imx_iomux_v3_setup_multiple_pads(ecspi1_pads, ARRAY_SIZE(ecspi1_pads)); + gpio_direction_output(IMX_GPIO_NR(3, 19), 0); +} + #ifdef CONFIG_FSL_ESDHC static struct fsl_esdhc_cfg usdhc_cfg[] = { {USDHC2_BASE_ADDR}, @@ -647,6 +664,7 @@ int board_early_init_f(void) { setup_iomux_uart(); setup_display(); + setup_spinor();
return 0; } @@ -672,6 +690,11 @@ int checkboard(void) return 0; }
+int board_spi_cs_gpio(unsigned bus, unsigned cs) +{ + return (bus == 0 && cs == 0) ? (IMX_GPIO_NR(3, 19)) : -EINVAL; +} + #ifdef CONFIG_CMD_BMODE static const struct boot_mode board_boot_modes[] = { /* 4 bit bus width */ diff --git a/include/configs/cgtqmx6eval.h b/include/configs/cgtqmx6eval.h index 92930c8..d0d098a 100644 --- a/include/configs/cgtqmx6eval.h +++ b/include/configs/cgtqmx6eval.h @@ -29,6 +29,16 @@ /* MMC Configs */ #define CONFIG_SYS_FSL_ESDHC_ADDR 0
+/* SPI NOR */ +#define CONFIG_CMD_SF +#define CONFIG_SPI_FLASH +#define CONFIG_SPI_FLASH_STMICRO +#define CONFIG_SPI_FLASH_SST +#define CONFIG_MXC_SPI +#define CONFIG_SF_DEFAULT_BUS 0 +#define CONFIG_SF_DEFAULT_SPEED 20000000 +#define CONFIG_SF_DEFAULT_MODE (SPI_MODE_0) + /* Miscellaneous commands */ #define CONFIG_CMD_BMODE
Congatec boards boot from SPI NOR, so it makes more sense to use SPI NOR to store the environment variables.
Signed-off-by: Otavio Salvador otavio@ossystems.com.br --- Changes since v1: - None
include/configs/cgtqmx6eval.h | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/include/configs/cgtqmx6eval.h b/include/configs/cgtqmx6eval.h index d0d098a..66f81ec 100644 --- a/include/configs/cgtqmx6eval.h +++ b/include/configs/cgtqmx6eval.h @@ -232,11 +232,21 @@ (CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_INIT_SP_OFFSET)
/* Environment organization */ -#define CONFIG_ENV_SIZE (8 * 1024) - -#define CONFIG_ENV_IS_IN_MMC - +#if defined (CONFIG_ENV_IS_IN_MMC) #define CONFIG_ENV_OFFSET (6 * 64 * 1024) #define CONFIG_SYS_MMC_ENV_DEV 0 +#endif + +#define CONFIG_ENV_SIZE (8 * 1024) + +#define CONFIG_ENV_IS_IN_SPI_FLASH +#if defined(CONFIG_ENV_IS_IN_SPI_FLASH) +#define CONFIG_ENV_OFFSET (768 * 1024) +#define CONFIG_ENV_SECT_SIZE (64 * 1024) +#define CONFIG_ENV_SPI_BUS CONFIG_SF_DEFAULT_BUS +#define CONFIG_ENV_SPI_CS CONFIG_SF_DEFAULT_CS +#define CONFIG_ENV_SPI_MODE CONFIG_SF_DEFAULT_MODE +#define CONFIG_ENV_SPI_MAX_HZ CONFIG_SF_DEFAULT_SPEED +#endif
#endif /* __CONFIG_CGTQMX6EVAL_H */
The last 16 KiB of the SPI NOR contain some manufacturing information, which should not be erased/overwritten.
Configure the protection bits BP2, BP1 and BP0 so that this region is protected.
Signed-off-by: Otavio Salvador otavio@ossystems.com.br --- Changes since v1: - Use CONFIG_SPI_NOR_PROTECTION_STM
board/congatec/cgtqmx6eval/cgtqmx6eval.c | 25 +++++++++++++++++++++++++ include/configs/cgtqmx6eval.h | 2 ++ 2 files changed, 27 insertions(+)
diff --git a/board/congatec/cgtqmx6eval/cgtqmx6eval.c b/board/congatec/cgtqmx6eval/cgtqmx6eval.c index a9694d5..9aff08d 100644 --- a/board/congatec/cgtqmx6eval/cgtqmx6eval.c +++ b/board/congatec/cgtqmx6eval/cgtqmx6eval.c @@ -31,6 +31,8 @@ #include <miiphy.h> #include <netdev.h> #include <micrel.h> +#include <spi_flash.h> +#include <spi.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -399,6 +401,22 @@ void setup_spinor(void) gpio_direction_output(IMX_GPIO_NR(3, 19), 0); }
+static void spinor_protect(void) +{ + struct spi_flash *spi; + + spi = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS, + CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE); + /* + * Set BP2 BP1 BP0 to 001, so that the last 64 sectors + * can be protected (0x3f0000 to 0x3fffff). + * + * This area stores some manufacturing information that + * should not be erased. + */ + spi_flash_cmd_write_status(spi, 1 << 2); +} + #ifdef CONFIG_FSL_ESDHC static struct fsl_esdhc_cfg usdhc_cfg[] = { {USDHC2_BASE_ADDR}, @@ -711,3 +729,10 @@ int misc_init_r(void) #endif return 0; } + +int board_late_init(void) +{ + spinor_protect(); + + return 0; +} diff --git a/include/configs/cgtqmx6eval.h b/include/configs/cgtqmx6eval.h index 66f81ec..8110605 100644 --- a/include/configs/cgtqmx6eval.h +++ b/include/configs/cgtqmx6eval.h @@ -21,6 +21,7 @@ #define CONFIG_SYS_MALLOC_LEN (10 * 1024 * 1024)
#define CONFIG_BOARD_EARLY_INIT_F +#define CONFIG_BOARD_LATE_INIT #define CONFIG_MISC_INIT_R
#define CONFIG_MXC_UART @@ -34,6 +35,7 @@ #define CONFIG_SPI_FLASH #define CONFIG_SPI_FLASH_STMICRO #define CONFIG_SPI_FLASH_SST +#define CONFIG_SPI_NOR_PROTECTION_STM #define CONFIG_MXC_SPI #define CONFIG_SF_DEFAULT_BUS 0 #define CONFIG_SF_DEFAULT_SPEED 20000000
On 12 September 2015 at 19:27, Otavio Salvador otavio@ossystems.com.br wrote:
The last 16 KiB of the SPI NOR contain some manufacturing information, which should not be erased/overwritten.
Configure the protection bits BP2, BP1 and BP0 so that this region is protected.
Signed-off-by: Otavio Salvador otavio@ossystems.com.br
Changes since v1:
- Use CONFIG_SPI_NOR_PROTECTION_STM
board/congatec/cgtqmx6eval/cgtqmx6eval.c | 25 +++++++++++++++++++++++++ include/configs/cgtqmx6eval.h | 2 ++ 2 files changed, 27 insertions(+)
diff --git a/board/congatec/cgtqmx6eval/cgtqmx6eval.c b/board/congatec/cgtqmx6eval/cgtqmx6eval.c index a9694d5..9aff08d 100644 --- a/board/congatec/cgtqmx6eval/cgtqmx6eval.c +++ b/board/congatec/cgtqmx6eval/cgtqmx6eval.c @@ -31,6 +31,8 @@ #include <miiphy.h> #include <netdev.h> #include <micrel.h> +#include <spi_flash.h> +#include <spi.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -399,6 +401,22 @@ void setup_spinor(void) gpio_direction_output(IMX_GPIO_NR(3, 19), 0); }
+static void spinor_protect(void) +{
struct spi_flash *spi;spi = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE);/** Set BP2 BP1 BP0 to 001, so that the last 64 sectors* can be protected (0x3f0000 to 0x3fffff).
The last sector (64*1024) area is being protected here, why? does this specific to your board? And also your taking an example of (0x3f0000 to 0x3fffff) that means the flash your using is 4MB is it? then this again becomes your board specific? is it?
** This area stores some manufacturing information that* should not be erased.*/spi_flash_cmd_write_status(spi, 1 << 2);+}
#ifdef CONFIG_FSL_ESDHC static struct fsl_esdhc_cfg usdhc_cfg[] = { {USDHC2_BASE_ADDR}, @@ -711,3 +729,10 @@ int misc_init_r(void) #endif return 0; }
+int board_late_init(void) +{
spinor_protect();return 0;+} diff --git a/include/configs/cgtqmx6eval.h b/include/configs/cgtqmx6eval.h index 66f81ec..8110605 100644 --- a/include/configs/cgtqmx6eval.h +++ b/include/configs/cgtqmx6eval.h @@ -21,6 +21,7 @@ #define CONFIG_SYS_MALLOC_LEN (10 * 1024 * 1024)
#define CONFIG_BOARD_EARLY_INIT_F +#define CONFIG_BOARD_LATE_INIT #define CONFIG_MISC_INIT_R
#define CONFIG_MXC_UART @@ -34,6 +35,7 @@ #define CONFIG_SPI_FLASH #define CONFIG_SPI_FLASH_STMICRO #define CONFIG_SPI_FLASH_SST +#define CONFIG_SPI_NOR_PROTECTION_STM #define CONFIG_MXC_SPI #define CONFIG_SF_DEFAULT_BUS 0
#define CONFIG_SF_DEFAULT_SPEED 20000000
1.9.1
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On Tue, Sep 22, 2015 at 4:11 PM, Jagan Teki jteki@openedev.com wrote:
On 12 September 2015 at 19:27, Otavio Salvador otavio@ossystems.com.br wrote:
+static void spinor_protect(void) +{
struct spi_flash *spi;spi = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE);/** Set BP2 BP1 BP0 to 001, so that the last 64 sectors* can be protected (0x3f0000 to 0x3fffff).The last sector (64*1024) area is being protected here, why? does this specific to your board? And also your taking an example of (0x3f0000 to 0x3fffff) that means the flash your using is 4MB is it? then this again becomes your board specific? is it?
Yes, the protected range is board specific. Congatec put some data in this area which are valuable and should not be erased. So we need to protect it.
On 23 September 2015 at 01:27, Otavio Salvador otavio.salvador@ossystems.com.br wrote:
On Tue, Sep 22, 2015 at 4:11 PM, Jagan Teki jteki@openedev.com wrote:
On 12 September 2015 at 19:27, Otavio Salvador otavio@ossystems.com.br wrote:
+static void spinor_protect(void) +{
struct spi_flash *spi;spi = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE);/** Set BP2 BP1 BP0 to 001, so that the last 64 sectors* can be protected (0x3f0000 to 0x3fffff).The last sector (64*1024) area is being protected here, why? does this specific to your board? And also your taking an example of (0x3f0000 to 0x3fffff) that means the flash your using is 4MB is it? then this again becomes your board specific? is it?
Yes, the protected range is board specific. Congatec put some data in this area which are valuable and should not be erased. So we need to protect it.
Sorry, I didn't understand why protection range or bits are specific to board, is it different in Congetc because the flash area being protected by means of sector range which is obviously the flash offset.
m25p32
BP2/BP1/BP0=protected sectors 000 = sector (none) 001 = sector (63) 010 = sector (63, 62) .... 111 = sector (all)
I understand your concept of protecting sectors once you find the BP2, BP1 and BP0 bits on board and then you locked the particular sectors, is it? it's totally reverse way that you're trying to do is it?
thanks!
On Wed, Sep 23, 2015 at 5:15 AM, Jagan Teki jteki@openedev.com wrote:
Sorry, I didn't understand why protection range or bits are specific to board, is it different in Congetc because the flash area being protected by means of sector range which is obviously the flash offset.
The protection range is board specific. For example: certain boards don't need any region to be protected at all, other boards may want the entire SPI flash to be protected. In Congatec's boards we only need to protect the last 16KiB sector because it contains data that are pre-programmed in the factory and we don't want the user to erase it.
That's why we set in the congatec board code the BP bits to 001.
m25p32
BP2/BP1/BP0=protected sectors 000 = sector (none) 001 = sector (63) 010 = sector (63, 62) .... 111 = sector (all)
I understand your concept of protecting sectors once you find the BP2, BP1 and BP0 bits on board and then you locked the particular sectors,
Yes, this is the idea.
is it? it's totally reverse way that you're trying to do is it?
No, the logic is correct. In Congatec's board we only want the last sector to be protected, hence BP2 BP1 BP0 is set to 001.
On 23 September 2015 at 19:06, Otavio Salvador otavio.salvador@ossystems.com.br wrote:
On Wed, Sep 23, 2015 at 5:15 AM, Jagan Teki jteki@openedev.com wrote:
Sorry, I didn't understand why protection range or bits are specific to board, is it different in Congetc because the flash area being protected by means of sector range which is obviously the flash offset.
The protection range is board specific. For example: certain boards don't need any region to be protected at all, other boards may want the entire SPI flash to be protected. In Congatec's boards we only need to protect the last 16KiB sector because it contains data that are pre-programmed in the factory and we don't want the user to erase it.
That's why we set in the congatec board code the BP bits to 001.
m25p32
BP2/BP1/BP0=protected sectors 000 = sector (none) 001 = sector (63) 010 = sector (63, 62) .... 111 = sector (all)
I understand your concept of protecting sectors once you find the BP2, BP1 and BP0 bits on board and then you locked the particular sectors,
Yes, this is the idea.
is it? it's totally reverse way that you're trying to do is it?
No, the logic is correct. In Congatec's board we only want the last sector to be protected, hence BP2 BP1 BP0 is set to 001.
All looks fine as per your patches, but probing flash from board files isn't a good approach if one more board add similar approach.
I have an idea similar to "cfi_flash" approach.
"sf protect on off len" then based on the offset and len write the protected bits and skips the sectors which are protected by showing warning say "protected sectors will not be erased!" [1]
Use the Linux approach[2] for more information, let me know for any more inputs.
[1] http://www.denx.de/wiki/view/DULG/UBootCmdGroupFlash#Section_5.9.3.4. [2] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/driver...
thanks!
Hello Jagan,
On Wed, Sep 23, 2015 at 12:56 PM, Jagan Teki jteki@openedev.com wrote:
All looks fine as per your patches, but probing flash from board files isn't a good approach if one more board add similar approach.
I have an idea similar to "cfi_flash" approach.
"sf protect on off len" then based on the offset and len write the protected bits and skips the sectors which are protected by showing warning say "protected sectors will not be erased!" [1]
Use the Linux approach[2] for more information, let me know for any more inputs.
[1] http://www.denx.de/wiki/view/DULG/UBootCmdGroupFlash#Section_5.9.3.4. [2] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/driver...
I think this is a good option however, can we include this one for this release and we can improve it for next?
On 23 September 2015 at 22:21, Otavio Salvador otavio.salvador@ossystems.com.br wrote:
Hello Jagan,
On Wed, Sep 23, 2015 at 12:56 PM, Jagan Teki jteki@openedev.com wrote:
All looks fine as per your patches, but probing flash from board files isn't a good approach if one more board add similar approach.
I have an idea similar to "cfi_flash" approach.
"sf protect on off len" then based on the offset and len write the protected bits and skips the sectors which are protected by showing warning say "protected sectors will not be erased!" [1]
Use the Linux approach[2] for more information, let me know for any more inputs.
[1] http://www.denx.de/wiki/view/DULG/UBootCmdGroupFlash#Section_5.9.3.4. [2] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/driver...
I think this is a good option however, can we include this one for this release and we can improve it for next?
Do you have any defined schedule on coming release about this feature, because right now sf has lot of pending items to tune - I'm unable add again this on TODO list that become big task in future.
If you have any time, please start the suggested approach I would help at any moment.
thanks!
On Wed, Sep 23, 2015 at 2:15 PM, Jagan Teki jteki@openedev.com wrote:
On 23 September 2015 at 22:21, Otavio Salvador otavio.salvador@ossystems.com.br wrote:
Hello Jagan,
On Wed, Sep 23, 2015 at 12:56 PM, Jagan Teki jteki@openedev.com wrote:
All looks fine as per your patches, but probing flash from board files isn't a good approach if one more board add similar approach.
I have an idea similar to "cfi_flash" approach.
"sf protect on off len" then based on the offset and len write the protected bits and skips the sectors which are protected by showing warning say "protected sectors will not be erased!" [1]
Use the Linux approach[2] for more information, let me know for any more inputs.
[1] http://www.denx.de/wiki/view/DULG/UBootCmdGroupFlash#Section_5.9.3.4. [2] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/driver...
I think this is a good option however, can we include this one for this release and we can improve it for next?
Do you have any defined schedule on coming release about this feature, because right now sf has lot of pending items to tune - I'm unable add again this on TODO list that become big task in future.
If you have any time, please start the suggested approach I would help at any moment.
We are adding support for a number of different SoM part numbers from Congatec and the SPI protection is required for we be able to merge the SPL and 2015.10 to be usable for them.
I can commit to work in this feature for 2016.01.
On 23 September 2015 at 22:51, Otavio Salvador otavio.salvador@ossystems.com.br wrote:
On Wed, Sep 23, 2015 at 2:15 PM, Jagan Teki jteki@openedev.com wrote:
On 23 September 2015 at 22:21, Otavio Salvador otavio.salvador@ossystems.com.br wrote:
Hello Jagan,
On Wed, Sep 23, 2015 at 12:56 PM, Jagan Teki jteki@openedev.com wrote:
All looks fine as per your patches, but probing flash from board files isn't a good approach if one more board add similar approach.
I have an idea similar to "cfi_flash" approach.
"sf protect on off len" then based on the offset and len write the protected bits and skips the sectors which are protected by showing warning say "protected sectors will not be erased!" [1]
Use the Linux approach[2] for more information, let me know for any more inputs.
[1] http://www.denx.de/wiki/view/DULG/UBootCmdGroupFlash#Section_5.9.3.4. [2] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/driver...
I think this is a good option however, can we include this one for this release and we can improve it for next?
Do you have any defined schedule on coming release about this feature, because right now sf has lot of pending items to tune - I'm unable add again this on TODO list that become big task in future.
If you have any time, please start the suggested approach I would help at any moment.
We are adding support for a number of different SoM part numbers from Congatec and the SPI protection is required for we be able to merge the SPL and 2015.10 to be usable for them.
I can commit to work in this feature for 2016.01.
Sorry, I understand your concern - but it's very difficult for me to maintain the drop_code (which should again removed later). Why can't you just add code as per my suggestion.. just a basic support as you aware probably will move the same in coming release if all set, because extending functionality is better approach rather than add it remove the same.
Thanks for your commitment, let me know if you need any more help.
thanks!
Hello Jagan,
On Thu, Sep 24, 2015 at 5:03 PM, Jagan Teki jteki@openedev.com wrote: ...
"sf protect on off len" then based on the offset and len write the protected bits and skips the sectors which are protected by showing warning say "protected sectors will not be erased!" [1]"
What about creating commands doing like this instead?
"sf protect 000" ---> Write 000 to BP2 BP1 BP0 (do not protect any sector) "sf protect 001" ---> Write 001 to BP2 BP1 BP0 (protect the last 1/64 sectors) ... "sf protect 111" ---> Write 111 to BP2 BP1 BP0" (protect all sectors)
Would this method be acceptable? It much simpler.
I don't think that the proposed "sf protect on off len" would apply to the SPI NOR protection layout.
Please advise.
On 25 September 2015 at 22:33, Otavio Salvador otavio.salvador@ossystems.com.br wrote:
Hello Jagan,
On Thu, Sep 24, 2015 at 5:03 PM, Jagan Teki jteki@openedev.com wrote: ...
"sf protect on off len" then based on the offset and len write the protected bits and skips the sectors which are protected by showing warning say "protected sectors will not be erased!" [1]"
What about creating commands doing like this instead?
"sf protect 000" ---> Write 000 to BP2 BP1 BP0 (do not protect any sector) "sf protect 001" ---> Write 001 to BP2 BP1 BP0 (protect the last 1/64 sectors) ... "sf protect 111" ---> Write 111 to BP2 BP1 BP0" (protect all sectors)
This would rather un-obvious implementation, how can use controls register bits, from user point-of-view flash can be accessed in-terms of offset, size.
Would this method be acceptable? It much simpler.
I don't think that the proposed "sf protect on off len" would apply to the SPI NOR protection layout.
No, it would apply any flash-layout not only for SPI-NOR, in fact it is much known and acceptable way of implementation - see cfi flash for example(both in Linux, U-Boot) and same case with SPI-NOR on Linux[1]
Please advise.
[1] https://patchwork.ozlabs.org/patch/513041/
-- Jagan.
Hi Jagan,
On Sat, Sep 26, 2015 at 12:47 AM, Jagan Teki jteki@openedev.com wrote:
Thanks for your suggestion.
I have implemented the SPI NOR protection support based on Brian's patch for the kernel as you suggested.
Please let me know your thoughts.
Thanks
participants (4)
-
Fabio Estevam -
Jagan Teki -
Otavio Salvador -
Otavio Salvador