[PATCH 1/4] ram: stm32mp1: protect minimum value in get_bufsize

[PATCH 1/4] ram: stm32mp1: protect minimum value in get_bufsize

[Patrick Delaunay]

Add protection on minimum value for result of get_bufsize
and check the alignment of buffer size: only multiple min_size
is allowed; only 4 bytes alignment was checked previously
(value & 0x3).

For example the "Random" test raises an issue when size is not 8 bytes
aligned because address for buffer = address + size / 2 is not word
aligned.

This patch avoid test error for unsupported size value.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

 drivers/ram/stm32mp1/stm32mp1_tests.c | 37 ++++++++++++++-------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/ram/stm32mp1/stm32mp1_tests.c b/drivers/ram/stm32mp1/stm32mp1_tests.c
index bacdd74705..90e82acda7 100644
--- a/drivers/ram/stm32mp1/stm32mp1_tests.c
+++ b/drivers/ram/stm32mp1/stm32mp1_tests.c
@@ -17,7 +17,7 @@
 DECLARE_GLOBAL_DATA_PTR;
 
 static int get_bufsize(char *string, int argc, char *argv[], int arg_nb,
-		       size_t *bufsize, size_t default_size)
+		       size_t *bufsize, size_t default_size, size_t min_size)
 {
 	unsigned long value;
 
@@ -27,13 +27,14 @@ static int get_bufsize(char *string, int argc, char *argv[], int arg_nb,
 				arg_nb, argv[arg_nb]);
 			return -1;
 		}
-		if (value > STM32_DDR_SIZE || value == 0) {
-			sprintf(string, "invalid size %s", argv[arg_nb]);
+		if (value > STM32_DDR_SIZE || value < min_size) {
+			sprintf(string, "invalid size %s (min=%d)",
+				argv[arg_nb], min_size);
 			return -1;
 		}
-		if (value & 0x3) {
-			sprintf(string, "unaligned size %s",
-				argv[arg_nb]);
+		if (value & (min_size - 1)) {
+			sprintf(string, "unaligned size %s (min=%d)",
+				argv[arg_nb], min_size);
 			return -1;
 		}
 		*bufsize = value;
@@ -441,7 +442,7 @@ static enum test_result test_addressbus(struct stm32mp1_ddrctl *ctl,
 	u32 bufsize;
 	u32 error;
 
-	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024))
+	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 4))
 		return TEST_ERROR;
 	if (!is_power_of_2(bufsize)) {
 		sprintf(string, "size 0x%x is not a power of 2",
@@ -470,7 +471,7 @@ static enum test_result test_memdevice(struct stm32mp1_ddrctl *ctl,
 	size_t bufsize;
 	u32 error;
 
-	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024))
+	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 4))
 		return TEST_ERROR;
 	if (get_addr(string, argc, argv, 1, &addr))
 		return TEST_ERROR;
@@ -512,7 +513,7 @@ static enum test_result test_sso(struct stm32mp1_ddrctl *ctl,
 	u32 error = 0;
 	u32 data;
 
-	if (get_bufsize(string, argc, argv, 0, &bufsize, 4))
+	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 4))
 		return TEST_ERROR;
 	if (get_addr(string, argc, argv, 1, &addr))
 		return TEST_ERROR;
@@ -584,7 +585,7 @@ static enum test_result test_random(struct stm32mp1_ddrctl *ctl,
 	u32 error = 0;
 	unsigned int seed;
 
-	if (get_bufsize(string, argc, argv, 0, &bufsize, 8 * 1024))
+	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 8))
 		return TEST_ERROR;
 	if (get_nb_loop(string, argc, argv, 1, &nb_loop, 1))
 		return TEST_ERROR;
@@ -744,7 +745,7 @@ static enum test_result test_noise_burst(struct stm32mp1_ddrctl *ctl,
 	int i;
 	enum test_result res = TEST_PASSED;
 
-	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024))
+	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 128))
 		return TEST_ERROR;
 	if (get_pattern(string, argc, argv, 1, &pattern, 0xFFFFFFFF))
 		return TEST_ERROR;
@@ -920,7 +921,7 @@ static enum test_result test_freq_pattern(struct stm32mp1_ddrctl *ctl,
 	const u32 **patterns;
 	u32 bufsize;
 
-	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024))
+	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 128))
 		return TEST_ERROR;
 
 	switch (readl(&ctl->mstr) & DDRCTRL_MSTR_DATA_BUS_WIDTH_MASK) {
@@ -1007,7 +1008,7 @@ static enum test_result test_checkboard(struct stm32mp1_ddrctl *ctl,
 
 	u32 checkboard[2] = {0x55555555, 0xAAAAAAAA};
 
-	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024))
+	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 8))
 		return TEST_ERROR;
 	if (get_nb_loop(string, argc, argv, 1, &nb_loop, 1))
 		return TEST_ERROR;
@@ -1042,7 +1043,7 @@ static enum test_result test_blockseq(struct stm32mp1_ddrctl *ctl,
 	u32 bufsize, nb_loop, loop = 0, addr, value;
 	int i;
 
-	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024))
+	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 4))
 		return TEST_ERROR;
 	if (get_nb_loop(string, argc, argv, 1, &nb_loop, 1))
 		return TEST_ERROR;
@@ -1076,7 +1077,7 @@ static enum test_result test_walkbit0(struct stm32mp1_ddrctl *ctl,
 	u32 bufsize, nb_loop, loop = 0, addr, value;
 	int i;
 
-	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024))
+	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 4))
 		return TEST_ERROR;
 	if (get_nb_loop(string, argc, argv, 1, &nb_loop, 1))
 		return TEST_ERROR;
@@ -1114,7 +1115,7 @@ static enum test_result test_walkbit1(struct stm32mp1_ddrctl *ctl,
 	u32 bufsize, nb_loop, loop = 0, addr, value;
 	int i;
 
-	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024))
+	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 4))
 		return TEST_ERROR;
 	if (get_nb_loop(string, argc, argv, 1, &nb_loop, 1))
 		return TEST_ERROR;
@@ -1156,7 +1157,7 @@ static enum test_result test_bitspread(struct stm32mp1_ddrctl *ctl,
 	u32 bufsize, nb_loop, loop = 0, addr, bitspread[4];
 	int i, j;
 
-	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024))
+	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 32))
 		return TEST_ERROR;
 	if (get_nb_loop(string, argc, argv, 1, &nb_loop, 1))
 		return TEST_ERROR;
@@ -1203,7 +1204,7 @@ static enum test_result test_bitflip(struct stm32mp1_ddrctl *ctl,
 
 	u32 bitflip[4];
 
-	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024))
+	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 32))
 		return TEST_ERROR;
 	if (get_nb_loop(string, argc, argv, 1, &nb_loop, 1))
 		return TEST_ERROR;
-- 
2.17.1

[PATCH 2/4] ram: stm32mp1: add parameter addr in test FrequencySelectivePattern

[Patrick Delaunay]

Add a parameter addr in test FrequencySelectivePattern to select
the base address used to execute the tests.

Default value (when the parameter is absent) is STM32_DDR_BASE,
selected in get_addr() function.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

 drivers/ram/stm32mp1/stm32mp1_tests.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/ram/stm32mp1/stm32mp1_tests.c b/drivers/ram/stm32mp1/stm32mp1_tests.c
index 90e82acda7..fec9fd010e 100644
--- a/drivers/ram/stm32mp1/stm32mp1_tests.c
+++ b/drivers/ram/stm32mp1/stm32mp1_tests.c
@@ -919,10 +919,12 @@ static enum test_result test_freq_pattern(struct stm32mp1_ddrctl *ctl,
 	enum test_result res = TEST_PASSED, pattern_res;
 	int i, bus_width;
 	const u32 **patterns;
-	u32 bufsize;
+	u32 bufsize, addr;
 
 	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 128))
 		return TEST_ERROR;
+	if (get_addr(string, argc, argv, 1, &addr))
+		return TEST_ERROR;
 
 	switch (readl(&ctl->mstr) & DDRCTRL_MSTR_DATA_BUS_WIDTH_MASK) {
 	case DDRCTRL_MSTR_DATA_BUS_WIDTH_HALF:
@@ -935,15 +937,14 @@ static enum test_result test_freq_pattern(struct stm32mp1_ddrctl *ctl,
 	}
 
 	printf("running test pattern at 0x%08x length 0x%x width = %d\n",
-	       STM32_DDR_BASE, bufsize, bus_width);
+	       addr, bufsize, bus_width);
 
 	patterns =
 		(const u32 **)(bus_width == 16 ? patterns_x16 : patterns_x32);
 
 	for (i = 0; i < NB_PATTERN; i++) {
 		printf("test data pattern %s:", patterns_comments[i]);
-		pattern_res = test_loop(patterns[i], (u32 *)STM32_DDR_BASE,
-					bufsize);
+		pattern_res = test_loop(patterns[i], (u32 *)addr, bufsize);
 		if (pattern_res != TEST_PASSED)	{
 			printf("Failed\n");
 			return pattern_res;
@@ -1419,9 +1420,9 @@ const struct test_desc test[] = {
 	 "Verifies r/w and memcopy(burst for pseudo random value.",
 	 3
 	},
-	{test_freq_pattern, "FrequencySelectivePattern", "[size]",
+	{test_freq_pattern, "FrequencySelectivePattern", "[size] [addr]",
 	 "write & test patterns: Mostly Zero, Mostly One and F/n",
-	 1
+	 2
 	},
 	{test_blockseq, "BlockSequential", "[size] [loop] [addr]",
 	 "test incremental pattern",
-- 
2.17.1

[PATCH 2/4] ram: stm32mp1: add parameter addr in test FrequencySelectivePattern

[Patrice CHOTARD]

Hi Patrick

On 6/12/20 10:34 AM, Patrick Delaunay wrote:
> Add a parameter addr in test FrequencySelectivePattern to select
> the base address used to execute the tests.
>
> Default value (when the parameter is absent) is STM32_DDR_BASE,
> selected in get_addr() function.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  drivers/ram/stm32mp1/stm32mp1_tests.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/ram/stm32mp1/stm32mp1_tests.c b/drivers/ram/stm32mp1/stm32mp1_tests.c
> index 90e82acda7..fec9fd010e 100644
> --- a/drivers/ram/stm32mp1/stm32mp1_tests.c
> +++ b/drivers/ram/stm32mp1/stm32mp1_tests.c
> @@ -919,10 +919,12 @@ static enum test_result test_freq_pattern(struct stm32mp1_ddrctl *ctl,
>  	enum test_result res = TEST_PASSED, pattern_res;
>  	int i, bus_width;
>  	const u32 **patterns;
> -	u32 bufsize;
> +	u32 bufsize, addr;
>  
>  	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 128))
>  		return TEST_ERROR;
> +	if (get_addr(string, argc, argv, 1, &addr))
> +		return TEST_ERROR;
>  
>  	switch (readl(&ctl->mstr) & DDRCTRL_MSTR_DATA_BUS_WIDTH_MASK) {
>  	case DDRCTRL_MSTR_DATA_BUS_WIDTH_HALF:
> @@ -935,15 +937,14 @@ static enum test_result test_freq_pattern(struct stm32mp1_ddrctl *ctl,
>  	}
>  
>  	printf("running test pattern at 0x%08x length 0x%x width = %d\n",
> -	       STM32_DDR_BASE, bufsize, bus_width);
> +	       addr, bufsize, bus_width);
>  
>  	patterns =
>  		(const u32 **)(bus_width == 16 ? patterns_x16 : patterns_x32);
>  
>  	for (i = 0; i < NB_PATTERN; i++) {
>  		printf("test data pattern %s:", patterns_comments[i]);
> -		pattern_res = test_loop(patterns[i], (u32 *)STM32_DDR_BASE,
> -					bufsize);
> +		pattern_res = test_loop(patterns[i], (u32 *)addr, bufsize);
>  		if (pattern_res != TEST_PASSED)	{
>  			printf("Failed\n");
>  			return pattern_res;
> @@ -1419,9 +1420,9 @@ const struct test_desc test[] = {
>  	 "Verifies r/w and memcopy(burst for pseudo random value.",
>  	 3
>  	},
> -	{test_freq_pattern, "FrequencySelectivePattern", "[size]",
> +	{test_freq_pattern, "FrequencySelectivePattern", "[size] [addr]",
>  	 "write & test patterns: Mostly Zero, Mostly One and F/n",
> -	 1
> +	 2
>  	},
>  	{test_blockseq, "BlockSequential", "[size] [loop] [addr]",
>  	 "test incremental pattern",

Reviewed-by: Patrice Chotard <patrice.chotard@st.com>

Thanks

[PATCH 3/4] ram: stm32mp1: use the DDR size by default in the test addressBus

[Patrick Delaunay]

The test 4 = "AddressBus [size] [addr]" without parameter
detects alias for any address bit only when:
- size = real size of DDR
- addr = start of DDR = 0xC0000000

These value must by the default value when parameters are absent.

This patch sets bufsize to STM32_DDR_SIZE and get_bufsize() selects
the correct value for bufsize when this parameter is absent =
full size of the DDDR

On EV1 board :
DDR> test 4
running at 0xC0000000 length 0x40000000

On DK2 board
DDR> test 4
running at 0xC0000000 length 0x20000000

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

 drivers/ram/stm32mp1/stm32mp1_tests.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/ram/stm32mp1/stm32mp1_tests.c b/drivers/ram/stm32mp1/stm32mp1_tests.c
index fec9fd010e..a2a69da9a3 100644
--- a/drivers/ram/stm32mp1/stm32mp1_tests.c
+++ b/drivers/ram/stm32mp1/stm32mp1_tests.c
@@ -442,7 +442,7 @@ static enum test_result test_addressbus(struct stm32mp1_ddrctl *ctl,
 	u32 bufsize;
 	u32 error;
 
-	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 4))
+	if (get_bufsize(string, argc, argv, 0, &bufsize, STM32_DDR_SIZE, 4))
 		return TEST_ERROR;
 	if (!is_power_of_2(bufsize)) {
 		sprintf(string, "size 0x%x is not a power of 2",
@@ -452,6 +452,7 @@ static enum test_result test_addressbus(struct stm32mp1_ddrctl *ctl,
 	if (get_addr(string, argc, argv, 1, &addr))
 		return TEST_ERROR;
 
+	printf("running at 0x%08x length 0x%x\n", addr, bufsize);
 	error = (u32)addressbus((u32 *)addr, bufsize);
 	if (error) {
 		sprintf(string, "0x%x: error for address 0x%x",
-- 
2.17.1

[PATCH 3/4] ram: stm32mp1: use the DDR size by default in the test addressBus

[Patrice CHOTARD]

Hi Patrick

one typo below

On 6/12/20 10:34 AM, Patrick Delaunay wrote:
> The test 4 = "AddressBus [size] [addr]" without parameter
> detects alias for any address bit only when:
> - size = real size of DDR
> - addr = start of DDR = 0xC0000000
>
> These value must by the default value when parameters are absent.
s/by the /be the
>
> This patch sets bufsize to STM32_DDR_SIZE and get_bufsize() selects
> the correct value for bufsize when this parameter is absent =
> full size of the DDDR
>
> On EV1 board :
> DDR> test 4
> running at 0xC0000000 length 0x40000000
>
> On DK2 board
> DDR> test 4
> running at 0xC0000000 length 0x20000000
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  drivers/ram/stm32mp1/stm32mp1_tests.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/ram/stm32mp1/stm32mp1_tests.c b/drivers/ram/stm32mp1/stm32mp1_tests.c
> index fec9fd010e..a2a69da9a3 100644
> --- a/drivers/ram/stm32mp1/stm32mp1_tests.c
> +++ b/drivers/ram/stm32mp1/stm32mp1_tests.c
> @@ -442,7 +442,7 @@ static enum test_result test_addressbus(struct stm32mp1_ddrctl *ctl,
>  	u32 bufsize;
>  	u32 error;
>  
> -	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 4))
> +	if (get_bufsize(string, argc, argv, 0, &bufsize, STM32_DDR_SIZE, 4))
>  		return TEST_ERROR;
>  	if (!is_power_of_2(bufsize)) {
>  		sprintf(string, "size 0x%x is not a power of 2",
> @@ -452,6 +452,7 @@ static enum test_result test_addressbus(struct stm32mp1_ddrctl *ctl,
>  	if (get_addr(string, argc, argv, 1, &addr))
>  		return TEST_ERROR;
>  
> +	printf("running at 0x%08x length 0x%x\n", addr, bufsize);
>  	error = (u32)addressbus((u32 *)addr, bufsize);
>  	if (error) {
>  		sprintf(string, "0x%x: error for address 0x%x",

You can add my reviewed-by when typo will be fixed

Thanks

[PATCH 4/4] ram: stm32mp1: add size and addr parameter to test all

[Patrick Delaunay]

Add size and addr parameter to test "All" to override the default
value (4kB and STM32_DDR_BASE) used in tests with these optional
parameters: [size] or [addr].

When other optional parameters are present before [addr],
they are replaced by default value:
- [loop] = "1"
- [pattern] = "-" (new: force default pattern)

Example to use:

DDR>test 0 1 0x20000

DDR>test 0 1 0x1000000 0xD0000000

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

 drivers/ram/stm32mp1/stm32mp1_tests.c | 47 +++++++++++++++++++++++++--
 1 file changed, 44 insertions(+), 3 deletions(-)

diff --git a/drivers/ram/stm32mp1/stm32mp1_tests.c b/drivers/ram/stm32mp1/stm32mp1_tests.c
index a2a69da9a3..952006aa14 100644
--- a/drivers/ram/stm32mp1/stm32mp1_tests.c
+++ b/drivers/ram/stm32mp1/stm32mp1_tests.c
@@ -14,6 +14,8 @@
 
 #define ADDR_INVALID	0xFFFFFFFF
 
+#define PATTERN_DEFAULT	"-"
+
 DECLARE_GLOBAL_DATA_PTR;
 
 static int get_bufsize(char *string, int argc, char *argv[], int arg_nb,
@@ -103,6 +105,10 @@ static int get_pattern(char *string, int argc, char *argv[], int arg_nb,
 	unsigned long value;
 
 	if (argc > arg_nb) {
+		if (!strcmp(argv[arg_nb], PATTERN_DEFAULT)) {
+			*pattern = default_pattern;
+			return 0;
+		}
 		if (strict_strtoul(argv[arg_nb], 16, &value) < 0) {
 			sprintf(string, "invalid %d parameter %s",
 				arg_nb, argv[arg_nb]);
@@ -1343,17 +1349,52 @@ static enum test_result test_all(struct stm32mp1_ddrctl *ctl,
 				 char *string, int argc, char *argv[])
 {
 	enum test_result res = TEST_PASSED, result;
-	int i, nb_error = 0;
+	int i, j, nb_error = 0, len;
 	u32 loop = 0, nb_loop;
+	int argc_test;
+	char *argv_test[4];
+	char loop_string[] = "1";
+	char pattern_string[] = PATTERN_DEFAULT;
+	u32 *addr;
 
 	if (get_nb_loop(string, argc, argv, 0, &nb_loop, 1))
 		return TEST_ERROR;
 
+	if (get_addr(string, argc, argv, 2, (u32 *)&addr))
+		return TEST_ERROR;
+
 	while (!nb_error) {
 		/* execute all the test except the lasts which are infinite */
 		for (i = 1; i < test_nb - NB_TEST_INFINITE; i++) {
+			argc_test = 0;
+			j = 0;
+			len = strlen(test[i].usage);
+			if (argc > 1 && j < len &&
+			    !strncmp("[size]", &test[i].usage[j], 6)) {
+				argv_test[argc_test++] = argv[1];
+				j += 7;
+			}
+			if (argc > 2) {
+				if (j < len &&
+				    !strncmp("[loop]", &test[i].usage[j], 6)) {
+					argv_test[argc_test++] = loop_string;
+					j += 7;
+				}
+				if (j < len &&
+				    !strncmp("[pattern]", &test[i].usage[j],
+					     9)) {
+					argv_test[argc_test++] = pattern_string;
+					j += 10;
+				}
+				if (j < len &&
+				    !strncmp("[addr]", &test[i].usage[j], 6)) {
+					argv_test[argc_test++] = argv[2];
+					j += 7;
+				}
+			}
 			printf("execute %d:%s\n", (int)i, test[i].name);
-			result = test[i].fct(ctl, phy, string, 0, NULL);
+			result = test[i].fct(ctl, phy, string,
+					     argc_test, argv_test);
 			printf("result %d:%s = ", (int)i, test[i].name);
 			if (result != TEST_PASSED) {
 				nb_error++;
@@ -1384,7 +1425,7 @@ static enum test_result test_all(struct stm32mp1_ddrctl *ctl,
  ****************************************************************/
 
 const struct test_desc test[] = {
-	{test_all, "All", "[loop]", "Execute all tests", 1 },
+	{test_all, "All", "[loop] [size] [addr]", "Execute all tests", 3 },
 	{test_databus, "Simple DataBus", "[addr]",
 	 "Verifies each data line by walking 1 on fixed address",
 	 1
-- 
2.17.1

[PATCH 4/4] ram: stm32mp1: add size and addr parameter to test all

[Patrice CHOTARD]

Hi Patrick

On 6/12/20 10:34 AM, Patrick Delaunay wrote:
> Add size and addr parameter to test "All" to override the default
> value (4kB and STM32_DDR_BASE) used in tests with these optional
> parameters: [size] or [addr].
>
> When other optional parameters are present before [addr],
> they are replaced by default value:
> - [loop] = "1"
> - [pattern] = "-" (new: force default pattern)
>
> Example to use:
>
> DDR>test 0 1 0x20000
>
> DDR>test 0 1 0x1000000 0xD0000000
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  drivers/ram/stm32mp1/stm32mp1_tests.c | 47 +++++++++++++++++++++++++--
>  1 file changed, 44 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/ram/stm32mp1/stm32mp1_tests.c b/drivers/ram/stm32mp1/stm32mp1_tests.c
> index a2a69da9a3..952006aa14 100644
> --- a/drivers/ram/stm32mp1/stm32mp1_tests.c
> +++ b/drivers/ram/stm32mp1/stm32mp1_tests.c
> @@ -14,6 +14,8 @@
>  
>  #define ADDR_INVALID	0xFFFFFFFF
>  
> +#define PATTERN_DEFAULT	"-"
> +
>  DECLARE_GLOBAL_DATA_PTR;
>  
>  static int get_bufsize(char *string, int argc, char *argv[], int arg_nb,
> @@ -103,6 +105,10 @@ static int get_pattern(char *string, int argc, char *argv[], int arg_nb,
>  	unsigned long value;
>  
>  	if (argc > arg_nb) {
> +		if (!strcmp(argv[arg_nb], PATTERN_DEFAULT)) {
> +			*pattern = default_pattern;
> +			return 0;
> +		}
>  		if (strict_strtoul(argv[arg_nb], 16, &value) < 0) {
>  			sprintf(string, "invalid %d parameter %s",
>  				arg_nb, argv[arg_nb]);
> @@ -1343,17 +1349,52 @@ static enum test_result test_all(struct stm32mp1_ddrctl *ctl,
>  				 char *string, int argc, char *argv[])
>  {
>  	enum test_result res = TEST_PASSED, result;
> -	int i, nb_error = 0;
> +	int i, j, nb_error = 0, len;
>  	u32 loop = 0, nb_loop;
> +	int argc_test;
> +	char *argv_test[4];
> +	char loop_string[] = "1";
> +	char pattern_string[] = PATTERN_DEFAULT;
> +	u32 *addr;
>  
>  	if (get_nb_loop(string, argc, argv, 0, &nb_loop, 1))
>  		return TEST_ERROR;
>  
> +	if (get_addr(string, argc, argv, 2, (u32 *)&addr))
> +		return TEST_ERROR;
> +
>  	while (!nb_error) {
>  		/* execute all the test except the lasts which are infinite */
>  		for (i = 1; i < test_nb - NB_TEST_INFINITE; i++) {
> +			argc_test = 0;
> +			j = 0;
> +			len = strlen(test[i].usage);
> +			if (argc > 1 && j < len &&
> +			    !strncmp("[size]", &test[i].usage[j], 6)) {
> +				argv_test[argc_test++] = argv[1];
> +				j += 7;
> +			}
> +			if (argc > 2) {
> +				if (j < len &&
> +				    !strncmp("[loop]", &test[i].usage[j], 6)) {
> +					argv_test[argc_test++] = loop_string;
> +					j += 7;
> +				}
> +				if (j < len &&
> +				    !strncmp("[pattern]", &test[i].usage[j],
> +					     9)) {
> +					argv_test[argc_test++] = pattern_string;
> +					j += 10;
> +				}
> +				if (j < len &&
> +				    !strncmp("[addr]", &test[i].usage[j], 6)) {
> +					argv_test[argc_test++] = argv[2];
> +					j += 7;
> +				}
> +			}
>  			printf("execute %d:%s\n", (int)i, test[i].name);
> -			result = test[i].fct(ctl, phy, string, 0, NULL);
> +			result = test[i].fct(ctl, phy, string,
> +					     argc_test, argv_test);
>  			printf("result %d:%s = ", (int)i, test[i].name);
>  			if (result != TEST_PASSED) {
>  				nb_error++;
> @@ -1384,7 +1425,7 @@ static enum test_result test_all(struct stm32mp1_ddrctl *ctl,
>   ****************************************************************/
>  
>  const struct test_desc test[] = {
> -	{test_all, "All", "[loop]", "Execute all tests", 1 },
> +	{test_all, "All", "[loop] [size] [addr]", "Execute all tests", 3 },
>  	{test_databus, "Simple DataBus", "[addr]",
>  	 "Verifies each data line by walking 1 on fixed address",
>  	 1

Reviewed-by: Patrice Chotard <patrice.chotard@st.com>

Thanks

[PATCH 1/4] ram: stm32mp1: protect minimum value in get_bufsize

[Patrice CHOTARD]

Hi Patrick

On 6/12/20 10:34 AM, Patrick Delaunay wrote:
> Add protection on minimum value for result of get_bufsize
> and check the alignment of buffer size: only multiple min_size
> is allowed; only 4 bytes alignment was checked previously
> (value & 0x3).
>
> For example the "Random" test raises an issue when size is not 8 bytes
> aligned because address for buffer = address + size / 2 is not word
> aligned.
>
> This patch avoid test error for unsupported size value.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  drivers/ram/stm32mp1/stm32mp1_tests.c | 37 ++++++++++++++-------------
>  1 file changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/ram/stm32mp1/stm32mp1_tests.c b/drivers/ram/stm32mp1/stm32mp1_tests.c
> index bacdd74705..90e82acda7 100644
> --- a/drivers/ram/stm32mp1/stm32mp1_tests.c
> +++ b/drivers/ram/stm32mp1/stm32mp1_tests.c
> @@ -17,7 +17,7 @@
>  DECLARE_GLOBAL_DATA_PTR;
>  
>  static int get_bufsize(char *string, int argc, char *argv[], int arg_nb,
> -		       size_t *bufsize, size_t default_size)
> +		       size_t *bufsize, size_t default_size, size_t min_size)
>  {
>  	unsigned long value;
>  
> @@ -27,13 +27,14 @@ static int get_bufsize(char *string, int argc, char *argv[], int arg_nb,
>  				arg_nb, argv[arg_nb]);
>  			return -1;
>  		}
> -		if (value > STM32_DDR_SIZE || value == 0) {
> -			sprintf(string, "invalid size %s", argv[arg_nb]);
> +		if (value > STM32_DDR_SIZE || value < min_size) {
> +			sprintf(string, "invalid size %s (min=%d)",
> +				argv[arg_nb], min_size);
>  			return -1;
>  		}
> -		if (value & 0x3) {
> -			sprintf(string, "unaligned size %s",
> -				argv[arg_nb]);
> +		if (value & (min_size - 1)) {
> +			sprintf(string, "unaligned size %s (min=%d)",
> +				argv[arg_nb], min_size);
>  			return -1;
>  		}
>  		*bufsize = value;
> @@ -441,7 +442,7 @@ static enum test_result test_addressbus(struct stm32mp1_ddrctl *ctl,
>  	u32 bufsize;
>  	u32 error;
>  
> -	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024))
> +	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 4))
>  		return TEST_ERROR;
>  	if (!is_power_of_2(bufsize)) {
>  		sprintf(string, "size 0x%x is not a power of 2",
> @@ -470,7 +471,7 @@ static enum test_result test_memdevice(struct stm32mp1_ddrctl *ctl,
>  	size_t bufsize;
>  	u32 error;
>  
> -	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024))
> +	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 4))
>  		return TEST_ERROR;
>  	if (get_addr(string, argc, argv, 1, &addr))
>  		return TEST_ERROR;
> @@ -512,7 +513,7 @@ static enum test_result test_sso(struct stm32mp1_ddrctl *ctl,
>  	u32 error = 0;
>  	u32 data;
>  
> -	if (get_bufsize(string, argc, argv, 0, &bufsize, 4))
> +	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 4))
>  		return TEST_ERROR;
>  	if (get_addr(string, argc, argv, 1, &addr))
>  		return TEST_ERROR;
> @@ -584,7 +585,7 @@ static enum test_result test_random(struct stm32mp1_ddrctl *ctl,
>  	u32 error = 0;
>  	unsigned int seed;
>  
> -	if (get_bufsize(string, argc, argv, 0, &bufsize, 8 * 1024))
> +	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 8))
>  		return TEST_ERROR;
>  	if (get_nb_loop(string, argc, argv, 1, &nb_loop, 1))
>  		return TEST_ERROR;
> @@ -744,7 +745,7 @@ static enum test_result test_noise_burst(struct stm32mp1_ddrctl *ctl,
>  	int i;
>  	enum test_result res = TEST_PASSED;
>  
> -	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024))
> +	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 128))
>  		return TEST_ERROR;
>  	if (get_pattern(string, argc, argv, 1, &pattern, 0xFFFFFFFF))
>  		return TEST_ERROR;
> @@ -920,7 +921,7 @@ static enum test_result test_freq_pattern(struct stm32mp1_ddrctl *ctl,
>  	const u32 **patterns;
>  	u32 bufsize;
>  
> -	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024))
> +	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 128))
>  		return TEST_ERROR;
>  
>  	switch (readl(&ctl->mstr) & DDRCTRL_MSTR_DATA_BUS_WIDTH_MASK) {
> @@ -1007,7 +1008,7 @@ static enum test_result test_checkboard(struct stm32mp1_ddrctl *ctl,
>  
>  	u32 checkboard[2] = {0x55555555, 0xAAAAAAAA};
>  
> -	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024))
> +	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 8))
>  		return TEST_ERROR;
>  	if (get_nb_loop(string, argc, argv, 1, &nb_loop, 1))
>  		return TEST_ERROR;
> @@ -1042,7 +1043,7 @@ static enum test_result test_blockseq(struct stm32mp1_ddrctl *ctl,
>  	u32 bufsize, nb_loop, loop = 0, addr, value;
>  	int i;
>  
> -	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024))
> +	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 4))
>  		return TEST_ERROR;
>  	if (get_nb_loop(string, argc, argv, 1, &nb_loop, 1))
>  		return TEST_ERROR;
> @@ -1076,7 +1077,7 @@ static enum test_result test_walkbit0(struct stm32mp1_ddrctl *ctl,
>  	u32 bufsize, nb_loop, loop = 0, addr, value;
>  	int i;
>  
> -	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024))
> +	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 4))
>  		return TEST_ERROR;
>  	if (get_nb_loop(string, argc, argv, 1, &nb_loop, 1))
>  		return TEST_ERROR;
> @@ -1114,7 +1115,7 @@ static enum test_result test_walkbit1(struct stm32mp1_ddrctl *ctl,
>  	u32 bufsize, nb_loop, loop = 0, addr, value;
>  	int i;
>  
> -	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024))
> +	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 4))
>  		return TEST_ERROR;
>  	if (get_nb_loop(string, argc, argv, 1, &nb_loop, 1))
>  		return TEST_ERROR;
> @@ -1156,7 +1157,7 @@ static enum test_result test_bitspread(struct stm32mp1_ddrctl *ctl,
>  	u32 bufsize, nb_loop, loop = 0, addr, bitspread[4];
>  	int i, j;
>  
> -	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024))
> +	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 32))
>  		return TEST_ERROR;
>  	if (get_nb_loop(string, argc, argv, 1, &nb_loop, 1))
>  		return TEST_ERROR;
> @@ -1203,7 +1204,7 @@ static enum test_result test_bitflip(struct stm32mp1_ddrctl *ctl,
>  
>  	u32 bitflip[4];
>  
> -	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024))
> +	if (get_bufsize(string, argc, argv, 0, &bufsize, 4 * 1024, 32))
>  		return TEST_ERROR;
>  	if (get_nb_loop(string, argc, argv, 1, &nb_loop, 1))
>  		return TEST_ERROR;

Reviewed-by: Patrice Chotard <patrice.chotard@st.com>

Thanks