[U-Boot] [PATCH v2 1/5] spi_sf: Skip the erasing of protected sectors

[U-Boot] [PATCH v2 1/5] spi_sf: Skip the erasing of protected sectors

[Otavio Salvador]

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;
 }
-- 
1.9.1

[U-Boot] [PATCH v2 2/5] spi_flash: Include spi_flash_cmd_write_status() prototype

[Otavio Salvador]

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_ */
-- 
1.9.1

[U-Boot] [PATCH v2 3/5] cgtqmx6eval: Add SPI NOR flash support

[Otavio Salvador]

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
 
-- 
1.9.1

[U-Boot] [PATCH v2 4/5] cgtqmx6eval: Use SPI NOR to store the environment

[Otavio Salvador]

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 */
-- 
1.9.1

[U-Boot] [PATCH v2 5/5] cgtqmx6eval: Protect the manufacturing information in SPI NOR

[Otavio Salvador]

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
-- 
1.9.1

[U-Boot] [PATCH v2 5/5] cgtqmx6eval: Protect the manufacturing information in SPI NOR

[Jagan Teki]

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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot



-- 
Jagan | openedev.

[U-Boot] [PATCH v2 5/5] cgtqmx6eval: Protect the manufacturing information in SPI NOR

[Otavio Salvador]

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.

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

[U-Boot] [PATCH v2 5/5] cgtqmx6eval: Protect the manufacturing information in SPI NOR

[Jagan Teki]

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!
-- 
Jagan | openedev.

[U-Boot] [PATCH v2 5/5] cgtqmx6eval: Protect the manufacturing information in SPI NOR

[Otavio Salvador]

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.

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

[U-Boot] [PATCH v2 5/5] cgtqmx6eval: Protect the manufacturing information in SPI NOR

[Jagan Teki]

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/drivers/mtd/devices/m25p80.c?id=972e1b7b450a93589b2a4c709e68f68da059aa5c


thanks!
-- 
Jagan | openedev.

[U-Boot] [PATCH v2 5/5] cgtqmx6eval: Protect the manufacturing information in SPI NOR

[Otavio Salvador]

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/drivers/mtd/devices/m25p80.c?id=972e1b7b450a93589b2a4c709e68f68da059aa5c

I think this is a good option however, can we include this one for
this release and we can improve it for next?

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

[U-Boot] [PATCH v2 5/5] cgtqmx6eval: Protect the manufacturing information in SPI NOR

[Jagan Teki]

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/drivers/mtd/devices/m25p80.c?id=972e1b7b450a93589b2a4c709e68f68da059aa5c
>
> 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!
-- 
Jagan | openedev.

[U-Boot] [PATCH v2 5/5] cgtqmx6eval: Protect the manufacturing information in SPI NOR

[Otavio Salvador]

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/drivers/mtd/devices/m25p80.c?id=972e1b7b450a93589b2a4c709e68f68da059aa5c
>>
>> 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.

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

[U-Boot] [PATCH v2 5/5] cgtqmx6eval: Protect the manufacturing information in SPI NOR

[Jagan Teki]

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/drivers/mtd/devices/m25p80.c?id=972e1b7b450a93589b2a4c709e68f68da059aa5c
>>>
>>> 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!
-- 
Jagan | openedev.

[U-Boot] [PATCH v2 5/5] cgtqmx6eval: Protect the manufacturing information in SPI NOR

[Otavio Salvador]

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.
-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

[U-Boot] [PATCH v2 5/5] cgtqmx6eval: Protect the manufacturing information in SPI NOR

[Jagan Teki]

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.

[U-Boot] [PATCH v2 5/5] cgtqmx6eval: Protect the manufacturing information in SPI NOR

[Fabio Estevam]

Hi Jagan,

On Sat, Sep 26, 2015 at 12:47 AM, Jagan Teki <jteki@openedev.com> wrote:

> [1] https://patchwork.ozlabs.org/patch/513041/

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