[PATCH 01/16] arm: stm32mp: update dependency for STM32_ETZPC

[PATCH 01/16] arm: stm32mp: update dependency for STM32_ETZPC

[Patrick Delaunay]

Correct the dependency for STM32 ETZPC protection, linked to SOC
STM32MP identified by CONFIG_STM32MP15x and not linked to
CONFIG_TARGET_STM32MP1 (no more existing).

This patch fix an issue introduced by commit 846254888e2e ("stm32mp1:
split board and SOC support for STM32MP15x family").

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
Acked-by: Patrice Chotard <patrice.chotard@st.com>
---

 arch/arm/mach-stm32mp/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-stm32mp/Kconfig b/arch/arm/mach-stm32mp/Kconfig
index e4d621dee8..96153693a7 100644
--- a/arch/arm/mach-stm32mp/Kconfig
+++ b/arch/arm/mach-stm32mp/Kconfig
@@ -119,7 +119,7 @@ config SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION_MMC2
 
 config STM32_ETZPC
 	bool "STM32 Extended TrustZone Protection"
-	depends on TARGET_STM32MP1
+	depends on STM32MP15x
 	default y
 	help
 	  Say y to enable STM32 Extended TrustZone Protection
-- 
2.17.1

[PATCH 02/16] arm: stm32mp: remove dependency for STM32KEY

[Patrick Delaunay]

This command is not more depending on fuse command, but have
direct access to BSEC misc driver, so the dependency wuth
can be removed CMD_FUSE

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

 arch/arm/mach-stm32mp/Kconfig | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm/mach-stm32mp/Kconfig b/arch/arm/mach-stm32mp/Kconfig
index 96153693a7..032facff31 100644
--- a/arch/arm/mach-stm32mp/Kconfig
+++ b/arch/arm/mach-stm32mp/Kconfig
@@ -127,12 +127,10 @@ config STM32_ETZPC
 config CMD_STM32KEY
 	bool "command stm32key to fuse public key hash"
 	default y
-	depends on CMD_FUSE
 	help
 		fuse public key hash in corresponding fuse used to authenticate
 		binary.
 
-
 config PRE_CON_BUF_ADDR
 	default 0xC02FF000
 
-- 
2.17.1

[PATCH 02/16] arm: stm32mp: remove dependency for STM32KEY

[Patrice CHOTARD]

HI Patrick

On 3/31/20 6:04 PM, Patrick Delaunay wrote:
> This command is not more depending on fuse command, but have
> direct access to BSEC misc driver, so the dependency wuth
> can be removed CMD_FUSE

s/wuth/with

rephrase "the dependency wuth can be removed CMD_FUSE" by " "the dependency with CMD_FUSE can be removed"

>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  arch/arm/mach-stm32mp/Kconfig | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/arch/arm/mach-stm32mp/Kconfig b/arch/arm/mach-stm32mp/Kconfig
> index 96153693a7..032facff31 100644
> --- a/arch/arm/mach-stm32mp/Kconfig
> +++ b/arch/arm/mach-stm32mp/Kconfig
> @@ -127,12 +127,10 @@ config STM32_ETZPC
>  config CMD_STM32KEY
>  	bool "command stm32key to fuse public key hash"
>  	default y
> -	depends on CMD_FUSE
>  	help
>  		fuse public key hash in corresponding fuse used to authenticate
>  		binary.
>  
> -
>  config PRE_CON_BUF_ADDR
>  	default 0xC02FF000
>  

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

Thanks

[PATCH 03/16] arm: stm32mp: reset to default environment when serial# change

[Patrick Delaunay]

Serial number is first checked and, in case of mismatch, all
environment variables are reset to their default value.

This patch allows to detect that environment is saved in a removable
device, as a SD card, and reused on a other board, potentially with
incompatible variables.

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

 arch/arm/mach-stm32mp/cpu.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c
index 9aa5794334..365c2aa4f7 100644
--- a/arch/arm/mach-stm32mp/cpu.c
+++ b/arch/arm/mach-stm32mp/cpu.c
@@ -511,8 +511,9 @@ __weak int setup_mac_address(void)
 		return -EINVAL;
 	}
 	pr_debug("OTP MAC address = %pM\n", enetaddr);
-	ret = !eth_env_set_enetaddr("ethaddr", enetaddr);
-	if (!ret)
+
+	ret = eth_env_set_enetaddr("ethaddr", enetaddr);
+	if (ret)
 		pr_err("Failed to set mac address %pM from OTP: %d\n",
 		       enetaddr, ret);
 #endif
@@ -522,13 +523,13 @@ __weak int setup_mac_address(void)
 
 static int setup_serial_number(void)
 {
+	char *serial_env;
 	char serial_string[25];
 	u32 otp[3] = {0, 0, 0 };
 	struct udevice *dev;
 	int ret;
 
-	if (env_get("serial#"))
-		return 0;
+	serial_env = env_get("serial#");
 
 	ret = uclass_get_device_by_driver(UCLASS_MISC,
 					  DM_GET_DRIVER(stm32mp_bsec),
@@ -542,6 +543,15 @@ static int setup_serial_number(void)
 		return ret;
 
 	sprintf(serial_string, "%08X%08X%08X", otp[0], otp[1], otp[2]);
+
+	if (serial_env) {
+		if (!strcmp(serial_string, serial_env))
+			return 0;
+		/* For invalid enviromnent (serial# change), reset to default */
+		env_set_default("serial number mismatch", 0);
+	}
+
+	/* save serial number */
 	env_set("serial#", serial_string);
 
 	return 0;
@@ -549,9 +559,9 @@ static int setup_serial_number(void)
 
 int arch_misc_init(void)
 {
+	setup_serial_number();
 	setup_boot_mode();
 	setup_mac_address();
-	setup_serial_number();
 
 	return 0;
 }
-- 
2.17.1

[PATCH 03/16] arm: stm32mp: reset to default environment when serial# change

[Patrice CHOTARD]

Hi Patrick???

On 3/31/20 6:04 PM, Patrick Delaunay wrote:
> Serial number is first checked and, in case of mismatch, all
> environment variables are reset to their default value.
>
> This patch allows to detect that environment is saved in a removable
> device, as a SD card, and reused on a other board, potentially with
> incompatible variables.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  arch/arm/mach-stm32mp/cpu.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c
> index 9aa5794334..365c2aa4f7 100644
> --- a/arch/arm/mach-stm32mp/cpu.c
> +++ b/arch/arm/mach-stm32mp/cpu.c
> @@ -511,8 +511,9 @@ __weak int setup_mac_address(void)
>  		return -EINVAL;
>  	}
>  	pr_debug("OTP MAC address = %pM\n", enetaddr);
> -	ret = !eth_env_set_enetaddr("ethaddr", enetaddr);
> -	if (!ret)
> +
> +	ret = eth_env_set_enetaddr("ethaddr", enetaddr);
> +	if (ret)
>  		pr_err("Failed to set mac address %pM from OTP: %d\n",
>  		       enetaddr, ret);
>  #endif
> @@ -522,13 +523,13 @@ __weak int setup_mac_address(void)
>  
>  static int setup_serial_number(void)
>  {
> +	char *serial_env;
>  	char serial_string[25];
>  	u32 otp[3] = {0, 0, 0 };
>  	struct udevice *dev;
>  	int ret;
>  
> -	if (env_get("serial#"))
> -		return 0;
> +	serial_env = env_get("serial#");
>  
>  	ret = uclass_get_device_by_driver(UCLASS_MISC,
>  					  DM_GET_DRIVER(stm32mp_bsec),
> @@ -542,6 +543,15 @@ static int setup_serial_number(void)
>  		return ret;
>  
>  	sprintf(serial_string, "%08X%08X%08X", otp[0], otp[1], otp[2]);
> +
> +	if (serial_env) {
> +		if (!strcmp(serial_string, serial_env))
> +			return 0;
> +		/* For invalid enviromnent (serial# change), reset to default */
> +		env_set_default("serial number mismatch", 0);
> +	}
> +
> +	/* save serial number */
>  	env_set("serial#", serial_string);
>  
>  	return 0;
> @@ -549,9 +559,9 @@ static int setup_serial_number(void)
>  
>  int arch_misc_init(void)
>  {
> +	setup_serial_number();
>  	setup_boot_mode();
>  	setup_mac_address();
> -	setup_serial_number();
>  
>  	return 0;
>  }

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

Thanks

[PATCH 03/16] arm: stm32mp: reset to default environment when serial# change

[Wolfgang Denk]

Dear Patrick,

In message <20200331180330.3.I8f6df6d28ce5b4b601ced711af3699d95e1576fb@changeid> you wrote:
> Serial number is first checked and, in case of mismatch, all
> environment variables are reset to their default value.
>
> This patch allows to detect that environment is saved in a removable
> device, as a SD card, and reused on a other board, potentially with
> incompatible variables.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  arch/arm/mach-stm32mp/cpu.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c
> index 9aa5794334..365c2aa4f7 100644
> --- a/arch/arm/mach-stm32mp/cpu.c
> +++ b/arch/arm/mach-stm32mp/cpu.c
> @@ -511,8 +511,9 @@ __weak int setup_mac_address(void)
>  		return -EINVAL;
>  	}
>  	pr_debug("OTP MAC address = %pM\n", enetaddr);
> -	ret = !eth_env_set_enetaddr("ethaddr", enetaddr);
> -	if (!ret)
> +
> +	ret = eth_env_set_enetaddr("ethaddr", enetaddr);
> +	if (ret)
>  		pr_err("Failed to set mac address %pM from OTP: %d\n",
>  		       enetaddr, ret);

This is an unrelated and totally undocumented change.  Please split
into separate commit.


> +
> +	if (serial_env) {
> +		if (!strcmp(serial_string, serial_env))
> +			return 0;
> +		/* For invalid enviromnent (serial# change), reset to default */
> +		env_set_default("serial number mismatch", 0);
> +	}

Resetting the enviropnment to the defaults means you drop all
setting sa user may have made.  This is a very bad move - as a user
I find such things completely unacceptable.  If I make any changes,
they must never ever be killed without my explicit confirmation.

I strongly advice against such a method. Please drop that.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"You ain't experienced..." "Well, nor are you." "That's true. But the
point is ... the point is ... the point is we've been not experienced
for a lot longer than you. We've got  a  lot  of  experience  of  not
having any experience."           - Terry Pratchett, _Witches Abroad_

[PATCH 03/16] arm: stm32mp: reset to default environment when serial# change

[Patrick DELAUNAY]

Dear Wolfgang,

> From: Wolfgang Denk <wd@denx.de>
> Sent: mercredi 1 avril 2020 13:19
> 
> Dear Patrick,
> 
> In message
> <20200331180330.3.I8f6df6d28ce5b4b601ced711af3699d95e1576fb@changeid>
> you wrote:
> > Serial number is first checked and, in case of mismatch, all
> > environment variables are reset to their default value.
> >
> > This patch allows to detect that environment is saved in a removable
> > device, as a SD card, and reused on a other board, potentially with
> > incompatible variables.
> >
> > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> > ---
> >
> >  arch/arm/mach-stm32mp/cpu.c | 20 +++++++++++++++-----
> >  1 file changed, 15 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c
> > index 9aa5794334..365c2aa4f7 100644
> > --- a/arch/arm/mach-stm32mp/cpu.c
> > +++ b/arch/arm/mach-stm32mp/cpu.c
> > @@ -511,8 +511,9 @@ __weak int setup_mac_address(void)
> >  		return -EINVAL;
> >  	}
> >  	pr_debug("OTP MAC address = %pM\n", enetaddr);
> > -	ret = !eth_env_set_enetaddr("ethaddr", enetaddr);
> > -	if (!ret)
> > +
> > +	ret = eth_env_set_enetaddr("ethaddr", enetaddr);
> > +	if (ret)
> >  		pr_err("Failed to set mac address %pM from OTP: %d\n",
> >  		       enetaddr, ret);
> 
> This is an unrelated and totally undocumented change.  Please split into separate
> commit.

Yes, sorry.

Done in http://patchwork.ozlabs.org/project/uboot/list/?series=169021
" arm: stm32mp: cleanup test on eth_env_set_enetaddr result"

> 
> > +
> > +	if (serial_env) {
> > +		if (!strcmp(serial_string, serial_env))
> > +			return 0;
> > +		/* For invalid enviromnent (serial# change), reset to default */
> > +		env_set_default("serial number mismatch", 0);
> > +	}
> 
> Resetting the enviropnment to the defaults means you drop all setting sa user
> may have made.  This is a very bad move - as a user I find such things
> completely unacceptable.  If I make any changes, they must never ever be killed
> without my explicit confirmation.
> 
> I strongly advice against such a method. Please drop that.

Understood, I drops this patch....

I introduce this behavior after a internal request and to avoid issues during tests:

Some users use the same SD card (same rootfs) on several boards (EV1 and DK2 for example)
and the U-Boot environment is saved on this SD card. 

When an user updates U-Boot binary to use this SD card on other board but not erase the
environment file (save in ext4 file in bootfs partition), the boot can fail because the
environment is not compatible between the 2 boards.

This patch try to avoid this issue when I detect that the removable device (as SD card) is used on a
a different board (it is detected when saved serial number with different the OTP).

But I admit that can be strange/unacceptable for end-user, particularly it is only change of board
(DK2#1 => DK2#2) : the environment tuned one board #2 can't be used on board#2.

I make too many assumption on user usage and this patch can't be acceptable in arch 
(common for all board based on STM32MP15x).
 
> Best regards,
> 
> Wolfgang Denk
> 
> --
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de "You
> ain't experienced..." "Well, nor are you." "That's true. But the point is ... the point is
> ... the point is we've been not experienced for a lot longer than you. We've got  a
> lot  of  experience  of  not
> having any experience."           - Terry Pratchett, _Witches Abroad_

[PATCH 03/16] arm: stm32mp: reset to default environment when serial# change

[Wolfgang Denk]

Dear Patrick,

In message <60273317e5704581bef81c4beb28ad75@SFHDAG6NODE3.st.com> you wrote:
> 
> > I strongly advice against such a method. Please drop that.
> 
> Understood, I drops this patch....

Thanks.

> I introduce this behavior after a internal request and to avoid issues during tests:
> 
> Some users use the same SD card (same rootfs) on several boards (EV1 and DK2 for example)
> and the U-Boot environment is saved on this SD card. 

OK.

> When an user updates U-Boot binary to use this SD card on other board but not erase the
> environment file (save in ext4 file in bootfs partition), the boot can fail because the
> environment is not compatible between the 2 boards.

Well, why would that fail on another board but not on the one that
is currently in use?  Where is the U-Boot image you are booting
from?   Not on the SDCard, too?

Well, then this is a design problem (or you may even call it a
design bug).  It has always been a bad idea to use a fixed structure
binary format if there are chances that provider (the env storage)
and consumer (U-Boot) may get out of sync.

The binary blob environment format (checksum, eventually redundancy
flag, date with a fixed total size) is inherently only compatible
with other U-Boot versions as long as redundancy and size settings
are kept the same.

If you cannot garantee this, you should use a different storage
format - for example as plain text file.  Of course you pay for
added compatibility across U-Boot configurations with the price of
reduced security (lack of checksum).

But then, normally you do not change redundancy or environment size
between U-Boot versions, so all this should be a theroretical
problem only?


> This patch try to avoid this issue when I detect that the removable device (as SD card) is used on a
> a different board (it is detected when saved serial number with different the OTP).

You see this only as a risk - try to see it as a chance, too.
Imagine a user is trying to copy environment between systems.  Why
would you want to stop him?

Any incompatibility issues can reliably be avoided when using binary
blob format, as then you will get a checksum error, and the
incorrect copy is ignored.

> I make too many assumption on user usage and this patch can't be acceptable in arch 
> (common for all board based on STM32MP15x).

See signature below :-)

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"UNIX was not designed to stop you from doing stupid things,  because
that would also stop you from doing clever things."       - Doug Gwyn

[PATCH 04/16] arm: stm32mp: detect U-Boot version used to save environment

[Patrick Delaunay]

Imply CONFIG_VERSION_VARIABLE for stm32mp1 target
and test U-Boot version ($env_ver) when the environment was
saved for the last time and to display warning trace.

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

 arch/arm/mach-stm32mp/Kconfig |  1 +
 include/configs/stm32mp1.h    | 11 ++++++++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-stm32mp/Kconfig b/arch/arm/mach-stm32mp/Kconfig
index 032facff31..a86288cb76 100644
--- a/arch/arm/mach-stm32mp/Kconfig
+++ b/arch/arm/mach-stm32mp/Kconfig
@@ -67,6 +67,7 @@ config TARGET_ST_STM32MP15x
 	imply DISABLE_CONSOLE
 	imply PRE_CONSOLE_BUFFER
 	imply SILENT_CONSOLE
+	imply VERSION_VARIABLE
 	help
 		target the STMicroelectronics board with SOC STM32MP15x
 		managed by board/st/stm32mp1:
diff --git a/include/configs/stm32mp1.h b/include/configs/stm32mp1.h
index 42717c167e..ae060fbc4b 100644
--- a/include/configs/stm32mp1.h
+++ b/include/configs/stm32mp1.h
@@ -222,9 +222,14 @@
 	"splashimage=0xc4300000\0"  \
 	"ramdisk_addr_r=0xc4400000\0" \
 	"altbootcmd=run bootcmd\0" \
-	"env_default=1\0" \
-	"env_check=if test $env_default -eq 1;"\
-		" then env set env_default 0;env save;fi\0" \
+	"env_check=" \
+		"env exists env_ver || env set env_ver ${ver};" \
+		"if env info -p -d -q; then env save; fi;" \
+		"if test \"$env_ver\" != \"$ver\"; then" \
+		" echo \"*** Warning: old environment ${env_ver}\";" \
+		" echo '* set default: env default -a; env save; reset';" \
+		" echo '* update current: env set env_ver ${ver}; env save';" \
+		"fi;\0" \
 	STM32MP_BOOTCMD \
 	STM32MP_MTDPARTS \
 	STM32MP_DFU_ALT_RAM \
-- 
2.17.1

[PATCH 04/16] arm: stm32mp: detect U-Boot version used to save environment

[Patrice CHOTARD]

Hi Patrick

On 3/31/20 6:04 PM, Patrick Delaunay wrote:
> Imply CONFIG_VERSION_VARIABLE for stm32mp1 target
> and test U-Boot version ($env_ver) when the environment was
> saved for the last time and to display warning trace.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  arch/arm/mach-stm32mp/Kconfig |  1 +
>  include/configs/stm32mp1.h    | 11 ++++++++---
>  2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-stm32mp/Kconfig b/arch/arm/mach-stm32mp/Kconfig
> index 032facff31..a86288cb76 100644
> --- a/arch/arm/mach-stm32mp/Kconfig
> +++ b/arch/arm/mach-stm32mp/Kconfig
> @@ -67,6 +67,7 @@ config TARGET_ST_STM32MP15x
>  	imply DISABLE_CONSOLE
>  	imply PRE_CONSOLE_BUFFER
>  	imply SILENT_CONSOLE
> +	imply VERSION_VARIABLE
>  	help
>  		target the STMicroelectronics board with SOC STM32MP15x
>  		managed by board/st/stm32mp1:
> diff --git a/include/configs/stm32mp1.h b/include/configs/stm32mp1.h
> index 42717c167e..ae060fbc4b 100644
> --- a/include/configs/stm32mp1.h
> +++ b/include/configs/stm32mp1.h
> @@ -222,9 +222,14 @@
>  	"splashimage=0xc4300000\0"  \
>  	"ramdisk_addr_r=0xc4400000\0" \
>  	"altbootcmd=run bootcmd\0" \
> -	"env_default=1\0" \
> -	"env_check=if test $env_default -eq 1;"\
> -		" then env set env_default 0;env save;fi\0" \
> +	"env_check=" \
> +		"env exists env_ver || env set env_ver ${ver};" \
> +		"if env info -p -d -q; then env save; fi;" \

Is option "-q" exist ? i can't find anything about it into source code


> +		"if test \"$env_ver\" != \"$ver\"; then" \
> +		" echo \"*** Warning: old environment ${env_ver}\";" \
> +		" echo '* set default: env default -a; env save; reset';" \
> +		" echo '* update current: env set env_ver ${ver}; env save';" \
> +		"fi;\0" \
>  	STM32MP_BOOTCMD \
>  	STM32MP_MTDPARTS \
>  	STM32MP_DFU_ALT_RAM \

[PATCH 04/16] arm: stm32mp: detect U-Boot version used to save environment

[Patrick DELAUNAY]

Hi Patrice

> From: Patrice CHOTARD <patrice.chotard@st.com>
> Sent: mercredi 1 avril 2020 09:34
> 
> Hi Patrick
> 
> On 3/31/20 6:04 PM, Patrick Delaunay wrote:
> > Imply CONFIG_VERSION_VARIABLE for stm32mp1 target and test U-Boot
> > version ($env_ver) when the environment was saved for the last time
> > and to display warning trace.
> >
> > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> > ---

[....]

> > diff --git a/include/configs/stm32mp1.h b/include/configs/stm32mp1.h
> > index 42717c167e..ae060fbc4b 100644
> > --- a/include/configs/stm32mp1.h
> > +++ b/include/configs/stm32mp1.h
> > @@ -222,9 +222,14 @@
> >  	"splashimage=0xc4300000\0"  \
> >  	"ramdisk_addr_r=0xc4400000\0" \
> >  	"altbootcmd=run bootcmd\0" \
> > -	"env_default=1\0" \
> > -	"env_check=if test $env_default -eq 1;"\
> > -		" then env set env_default 0;env save;fi\0" \
> > +	"env_check=" \
> > +		"env exists env_ver || env set env_ver ${ver};" \
> > +		"if env info -p -d -q; then env save; fi;" \
> 
> Is option "-q" exist ? i can't find anything about it into source code
> 
> 

Introduced in 
[v3,1/7] cmd: env: add option for quiet output on env info

http://patchwork.ozlabs.org/patch/1236816/

I forget to indicate dependency.

PAtrick

[PATCH 04/16] arm: stm32mp: detect U-Boot version used to save environment

[Wolfgang Denk]

Dear Patrick Delaunay,

In message <20200331160456.26254-1-patrick.delaunay@st.com> you wrote:
> Imply CONFIG_VERSION_VARIABLE for stm32mp1 target
> and test U-Boot version ($env_ver) when the environment was
> saved for the last time and to display warning trace.

What is env_ver?  Are you by chance reinventing the wheel?

The U-Boot version is stored in the environment variable "ver";
there should be no need for something similar.


Also. where is $env_ver coming from? It does not exist in mainline,
nor in any of the 3 patches that preceed this patch # 4/16 ?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Computers are not intelligent.  They only think they are.

[PATCH 04/16] arm: stm32mp: detect U-Boot version used to save environment

[Patrick DELAUNAY]

Dear Wolfgang,

> From: Wolfgang Denk <wd@denx.de>
> Sent: mercredi 1 avril 2020 13:26
> 
> Dear Patrick Delaunay,
> 
> In message <20200331160456.26254-1-patrick.delaunay@st.com> you wrote:
> > Imply CONFIG_VERSION_VARIABLE for stm32mp1 target and test U-Boot
> > version ($env_ver) when the environment was saved for the last time
> > and to display warning trace.
> 
> What is env_ver?  Are you by chance reinventing the wheel?

The script env_check is ;
 
	env exists env_ver || env set env_ver ${ver};

	if env info -p -d -q; then env save; fi;

	if test \"$env_ver\" != \"$ver\"; then
		echo \"*** Warning: old environment ${env_ver}\";
		echo '* set default: env default -a; env save; reset';
		echo '* update current: env set env_ver ${ver}; env save';
	fi;

In the first line of the script: "env exists env_ver || env set env_ver ${ver}", so

$env_ver = $ver, before the first env_save during the first boot (second line of the script)
 
> The U-Boot version is stored in the environment variable "ver"; there should be no
> need for something similar.
> 
> 
> Also. where is $env_ver coming from? It does not exist in mainline, nor in any of
> the 3 patches that preceed this patch # 4/16 ?

env_ver is only defined and used in this script to detect that current U-Boot version ($ver) and 
the version of U-Boot for last env save ($env_var) are not aligned.

I introduce this warning after debug of many issue around this kind of error, but perhaps more
a debug feature.

So if you found that it is a bad idea for upstream, I will drop this part and just to the new quiet option
To simplify the test:

env_check = " if env info -p -d -q; then env save; fi;"

> Best regards,
> 
> Wolfgang Denk

Regards
Patrick

[PATCH 04/16] arm: stm32mp: detect U-Boot version used to save environment

[Wolfgang Denk]

Dear Patrick,

In message <8607d1778bcd4035807908e4a3a90381@SFHDAG6NODE3.st.com> you wrote:
> 
> To simplify the test:
> 
> env_check = " if env info -p -d -q; then env save; fi;"

All such automatical "env save" actions somewhere in the code give
me the creeps.  I've seen too often that they did things I nver
intended to do or would have accepted if I had a chance to decide.

Use extremely careful, please.

From a user point of view, it's me who owns the environment, and
nobody should mess with my data without me confirming it.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Well I don't see why I have to make one man  miserable  when  I  can
make so many men happy."              - Ellyn Mustard, about marriage

[Uboot-stm32] [PATCH 04/16] arm: stm32mp: detect U-Boot version used to save environment

[Patrick DELAUNAY]

Dear Wolfgang,

> From: Uboot-stm32 <uboot-stm32-bounces@st-md-mailman.stormreply.com> On
> Behalf Of Wolfgang Denk
> 
> Dear Patrick,
> 
> In message <8607d1778bcd4035807908e4a3a90381@SFHDAG6NODE3.st.com>
> you wrote:
> >
> > To simplify the test:
> >
> > env_check = " if env info -p -d -q; then env save; fi;"
> 
> All such automatical "env save" actions somewhere in the code give me the
> creeps.  I've seen too often that they did things I nver intended to do or would
> have accepted if I had a chance to decide.
> 
> Use extremely careful, please.

Sure,

In this case, the command "env info -d" tests if the default environment is currently used,
So the user have never updated and saved the environment.

In this case and if the persistent storage is available (option -p), the script "env_check" save the environment.

PS: I take the initial idea from ./include/configs/opos6uldev.h and ./include/configs/apf27.h
 
> From a user point of view, it's me who owns the environment, and nobody should
> mess with my data without me confirming it.

As the save action is performed only when default environment is used, it is done before 
any user modification so I don't think that it is annoying for user.

I also kept the call this feature only in the ST specific bootcmd_stm32mp to allow customization for users or other boards.
(I prefer to don't add it in board_late_init() as it is done in board/intel/edison/edison.c)

The purpose of the "env save" is just to avoid a "Warning" during the boot, 
until the first user action and "env save" command:

Loading Environment from MMC... *** Warning - bad CRC, using default environment

The content of environment before and after the save is identical: it is the default one.
but if it is too aggressive I can kept it for downstream.

Marek do you have a opinion: it is acceptable for DH SOM using STM32MP15x SOC? 


> Best regards,
> 
> Wolfgang Denk
> 

Best Regards

Patrick Delaunay

[PATCH 05/16] arm: stm32mp: spl: add bsec driver in SPL

[Patrick Delaunay]

Add the bsec driver in SPL, as it is needed by SOC part number detection.

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

 arch/arm/dts/stm32mp15-u-boot.dtsi | 2 +-
 arch/arm/mach-stm32mp/Makefile     | 2 +-
 arch/arm/mach-stm32mp/bsec.c       | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/dts/stm32mp15-u-boot.dtsi b/arch/arm/dts/stm32mp15-u-boot.dtsi
index 8f9535a4db..e0b1223de8 100644
--- a/arch/arm/dts/stm32mp15-u-boot.dtsi
+++ b/arch/arm/dts/stm32mp15-u-boot.dtsi
@@ -40,7 +40,7 @@
 };
 
 &bsec {
-	u-boot,dm-pre-proper;
+	u-boot,dm-pre-reloc;
 };
 
 &clk_csi {
diff --git a/arch/arm/mach-stm32mp/Makefile b/arch/arm/mach-stm32mp/Makefile
index eee39c27c3..f29d6f795f 100644
--- a/arch/arm/mach-stm32mp/Makefile
+++ b/arch/arm/mach-stm32mp/Makefile
@@ -6,11 +6,11 @@
 obj-y += cpu.o
 obj-y += dram_init.o
 obj-y += syscon.o
+obj-y += bsec.o
 
 ifdef CONFIG_SPL_BUILD
 obj-y += spl.o
 else
-obj-y += bsec.o
 obj-$(CONFIG_CMD_STM32KEY) += cmd_stm32key.o
 obj-$(CONFIG_ARMV7_PSCI) += psci.o
 endif
diff --git a/arch/arm/mach-stm32mp/bsec.c b/arch/arm/mach-stm32mp/bsec.c
index 3b923f088e..1bd287e8bf 100644
--- a/arch/arm/mach-stm32mp/bsec.c
+++ b/arch/arm/mach-stm32mp/bsec.c
@@ -473,7 +473,7 @@ static int stm32mp_bsec_ofdata_to_platdata(struct udevice *dev)
 	return 0;
 }
 
-#ifndef CONFIG_STM32MP1_TRUSTED
+#if !defined(CONFIG_STM32MP1_TRUSTED) && !defined(CONFIG_SPL_BUILD)
 static int stm32mp_bsec_probe(struct udevice *dev)
 {
 	int otp;
@@ -500,7 +500,7 @@ U_BOOT_DRIVER(stm32mp_bsec) = {
 	.ofdata_to_platdata = stm32mp_bsec_ofdata_to_platdata,
 	.platdata_auto_alloc_size = sizeof(struct stm32mp_bsec_platdata),
 	.ops = &stm32mp_bsec_ops,
-#ifndef CONFIG_STM32MP1_TRUSTED
+#if !defined(CONFIG_STM32MP1_TRUSTED) && !defined(CONFIG_SPL_BUILD)
 	.probe = stm32mp_bsec_probe,
 #endif
 };
-- 
2.17.1

[PATCH 05/16] arm: stm32mp: spl: add bsec driver in SPL

[Patrice CHOTARD]

Hi Patrick

On 3/31/20 6:04 PM, Patrick Delaunay wrote:
> Add the bsec driver in SPL, as it is needed by SOC part number detection.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  arch/arm/dts/stm32mp15-u-boot.dtsi | 2 +-
>  arch/arm/mach-stm32mp/Makefile     | 2 +-
>  arch/arm/mach-stm32mp/bsec.c       | 4 ++--
>  3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/dts/stm32mp15-u-boot.dtsi b/arch/arm/dts/stm32mp15-u-boot.dtsi
> index 8f9535a4db..e0b1223de8 100644
> --- a/arch/arm/dts/stm32mp15-u-boot.dtsi
> +++ b/arch/arm/dts/stm32mp15-u-boot.dtsi
> @@ -40,7 +40,7 @@
>  };
>  
>  &bsec {
> -	u-boot,dm-pre-proper;
> +	u-boot,dm-pre-reloc;
>  };
>  
>  &clk_csi {
> diff --git a/arch/arm/mach-stm32mp/Makefile b/arch/arm/mach-stm32mp/Makefile
> index eee39c27c3..f29d6f795f 100644
> --- a/arch/arm/mach-stm32mp/Makefile
> +++ b/arch/arm/mach-stm32mp/Makefile
> @@ -6,11 +6,11 @@
>  obj-y += cpu.o
>  obj-y += dram_init.o
>  obj-y += syscon.o
> +obj-y += bsec.o
>  
>  ifdef CONFIG_SPL_BUILD
>  obj-y += spl.o
>  else
> -obj-y += bsec.o
>  obj-$(CONFIG_CMD_STM32KEY) += cmd_stm32key.o
>  obj-$(CONFIG_ARMV7_PSCI) += psci.o
>  endif
> diff --git a/arch/arm/mach-stm32mp/bsec.c b/arch/arm/mach-stm32mp/bsec.c
> index 3b923f088e..1bd287e8bf 100644
> --- a/arch/arm/mach-stm32mp/bsec.c
> +++ b/arch/arm/mach-stm32mp/bsec.c
> @@ -473,7 +473,7 @@ static int stm32mp_bsec_ofdata_to_platdata(struct udevice *dev)
>  	return 0;
>  }
>  
> -#ifndef CONFIG_STM32MP1_TRUSTED
> +#if !defined(CONFIG_STM32MP1_TRUSTED) && !defined(CONFIG_SPL_BUILD)
>  static int stm32mp_bsec_probe(struct udevice *dev)
>  {
>  	int otp;
> @@ -500,7 +500,7 @@ U_BOOT_DRIVER(stm32mp_bsec) = {
>  	.ofdata_to_platdata = stm32mp_bsec_ofdata_to_platdata,
>  	.platdata_auto_alloc_size = sizeof(struct stm32mp_bsec_platdata),
>  	.ops = &stm32mp_bsec_ops,
> -#ifndef CONFIG_STM32MP1_TRUSTED
> +#if !defined(CONFIG_STM32MP1_TRUSTED) && !defined(CONFIG_SPL_BUILD)
>  	.probe = stm32mp_bsec_probe,
>  #endif
>  };

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

Thanks

[PATCH 05/16] arm: stm32mp: spl: add bsec driver in SPL

[Wolfgang Denk]

Dear Patrick Delaunay,

In message <20200331180330.5.I7a042a9ffbb5c2668034eddf5ace91271bb53c5f@changeid> you wrote:
> Add the bsec driver in SPL, as it is needed by SOC part number detection.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  arch/arm/dts/stm32mp15-u-boot.dtsi | 2 +-
>  arch/arm/mach-stm32mp/Makefile     | 2 +-
>  arch/arm/mach-stm32mp/bsec.c       | 4 ++--
>  3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/dts/stm32mp15-u-boot.dtsi b/arch/arm/dts/stm32mp15-u-boot.dtsi
> index 8f9535a4db..e0b1223de8 100644
> --- a/arch/arm/dts/stm32mp15-u-boot.dtsi
> +++ b/arch/arm/dts/stm32mp15-u-boot.dtsi
> @@ -40,7 +40,7 @@
>  };
>  
>  &bsec {
> -	u-boot,dm-pre-proper;
> +	u-boot,dm-pre-reloc;
>  };

This seems to be an unrelated change?  You should at least explain
why this is needed, and if it's unrelated, spilt into a separate
commit.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Good manners are the settled  medium  of  social,  as  specie  is  of
commercial, life; returns are equally expected for both.
           - Lord Chesterfield _Letters to his Son_, 25 December 1753

[PATCH 05/16] arm: stm32mp: spl: add bsec driver in SPL

[Patrick DELAUNAY]

Dear Wolfgang,

> From: Wolfgang Denk <wd@denx.de>
> Sent: mercredi 1 avril 2020 13:27
> 
> Dear Patrick Delaunay,
> 
> In message
> <20200331180330.5.I7a042a9ffbb5c2668034eddf5ace91271bb53c5f@changeid>
> you wrote:
> > Add the bsec driver in SPL, as it is needed by SOC part number detection.
> >
> > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> > ---

[...]

> 
> This seems to be an unrelated change?  You should at least explain why this is
> needed, and if it's unrelated, spilt into a separate commit.

Yes, unrelated and it is missing the patch for it (linked to 800MHz support)

I will pushed a separate serie for this point.

> Best regards,
> 
> Wolfgang Denk

Regards
Patrick

[PATCH 06/16] arm: stm32mp: spl: display error in board_init_f

[Patrick Delaunay]

Update board_init_f and try to display error message
when console is available.

This patch adds trace to debug a spl boot issue when DEBUG
and DEBUG_UART is not activated, after uart probe.

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

 arch/arm/mach-stm32mp/spl.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/arch/arm/mach-stm32mp/spl.c b/arch/arm/mach-stm32mp/spl.c
index ca4231cd0d..dfdb5bb7e9 100644
--- a/arch/arm/mach-stm32mp/spl.c
+++ b/arch/arm/mach-stm32mp/spl.c
@@ -79,37 +79,36 @@ void spl_display_print(void)
 void board_init_f(ulong dummy)
 {
 	struct udevice *dev;
-	int ret;
+	int ret, clk, reset, pinctrl;
 
 	arch_cpu_init();
 
 	ret = spl_early_init();
 	if (ret) {
-		debug("spl_early_init() failed: %d\n", ret);
+		debug("%s: spl_early_init() failed: %d\n", __func__, ret);
 		hang();
 	}
 
-	ret = uclass_get_device(UCLASS_CLK, 0, &dev);
-	if (ret) {
-		debug("Clock init failed: %d\n", ret);
-		return;
-	}
+	clk = uclass_get_device(UCLASS_CLK, 0, &dev);
+	if (clk)
+		debug("%s: Clock init failed: %d\n", __func__, clk);
 
-	ret = uclass_get_device(UCLASS_RESET, 0, &dev);
-	if (ret) {
-		debug("Reset init failed: %d\n", ret);
-		return;
-	}
+	reset = uclass_get_device(UCLASS_RESET, 0, &dev);
+	if (reset)
+		debug("%s: Reset init failed: %d\n", __func__, reset);
 
-	ret = uclass_get_device(UCLASS_PINCTRL, 0, &dev);
-	if (ret) {
-		debug("%s: Cannot find pinctrl device\n", __func__);
-		return;
-	}
+	pinctrl = uclass_get_device(UCLASS_PINCTRL, 0, &dev);
+	if (pinctrl)
+		debug("%s: Cannot find pinctrl device: %d\n",
+		      __func__, pinctrl);
 
 	/* enable console uart printing */
 	preloader_console_init();
 
+	if (clk || reset || pinctrl)
+		printf("%s: probe failed clk=%d reset=%d pinctrl=%d\n",
+		       __func__, clk, reset, pinctrl);
+
 	ret = uclass_get_device(UCLASS_RAM, 0, &dev);
 	if (ret) {
 		printf("DRAM init failed: %d\n", ret);
-- 
2.17.1

[PATCH 06/16] arm: stm32mp: spl: display error in board_init_f

[Patrice CHOTARD]

Hi Patrick

On 3/31/20 6:04 PM, Patrick Delaunay wrote:
> Update board_init_f and try to display error message
> when console is available.
>
> This patch adds trace to debug a spl boot issue when DEBUG
> and DEBUG_UART is not activated, after uart probe.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  arch/arm/mach-stm32mp/spl.c | 33 ++++++++++++++++-----------------
>  1 file changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/arch/arm/mach-stm32mp/spl.c b/arch/arm/mach-stm32mp/spl.c
> index ca4231cd0d..dfdb5bb7e9 100644
> --- a/arch/arm/mach-stm32mp/spl.c
> +++ b/arch/arm/mach-stm32mp/spl.c
> @@ -79,37 +79,36 @@ void spl_display_print(void)
>  void board_init_f(ulong dummy)
>  {
>  	struct udevice *dev;
> -	int ret;
> +	int ret, clk, reset, pinctrl;
>  
>  	arch_cpu_init();
>  
>  	ret = spl_early_init();
>  	if (ret) {
> -		debug("spl_early_init() failed: %d\n", ret);
> +		debug("%s: spl_early_init() failed: %d\n", __func__, ret);
>  		hang();
>  	}
>  
> -	ret = uclass_get_device(UCLASS_CLK, 0, &dev);
> -	if (ret) {
> -		debug("Clock init failed: %d\n", ret);
> -		return;
> -	}
> +	clk = uclass_get_device(UCLASS_CLK, 0, &dev);
> +	if (clk)
> +		debug("%s: Clock init failed: %d\n", __func__, clk);
>  
> -	ret = uclass_get_device(UCLASS_RESET, 0, &dev);
> -	if (ret) {
> -		debug("Reset init failed: %d\n", ret);
> -		return;
> -	}
> +	reset = uclass_get_device(UCLASS_RESET, 0, &dev);
> +	if (reset)
> +		debug("%s: Reset init failed: %d\n", __func__, reset);
>  
> -	ret = uclass_get_device(UCLASS_PINCTRL, 0, &dev);
> -	if (ret) {
> -		debug("%s: Cannot find pinctrl device\n", __func__);
> -		return;
> -	}
> +	pinctrl = uclass_get_device(UCLASS_PINCTRL, 0, &dev);
> +	if (pinctrl)
> +		debug("%s: Cannot find pinctrl device: %d\n",
> +		      __func__, pinctrl);
>  
>  	/* enable console uart printing */
>  	preloader_console_init();
>  
> +	if (clk || reset || pinctrl)
> +		printf("%s: probe failed clk=%d reset=%d pinctrl=%d\n",
> +		       __func__, clk, reset, pinctrl);
> +
>  	ret = uclass_get_device(UCLASS_RAM, 0, &dev);
>  	if (ret) {
>  		printf("DRAM init failed: %d\n", ret);

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

Thanks

[PATCH 06/16] arm: stm32mp: spl: display error in board_init_f

[Wolfgang Denk]

Dear Patrick Delaunay,

In message <20200331180330.6.I41a641a07fd12da45b392920fc3407e608926396@changeid> you wrote:
> Update board_init_f and try to display error message
> when console is available.
>
> This patch adds trace to debug a spl boot issue when DEBUG
> and DEBUG_UART is not activated, after uart probe.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  arch/arm/mach-stm32mp/spl.c | 33 ++++++++++++++++-----------------
>  1 file changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/arch/arm/mach-stm32mp/spl.c b/arch/arm/mach-stm32mp/spl.c
> index ca4231cd0d..dfdb5bb7e9 100644
> --- a/arch/arm/mach-stm32mp/spl.c
> +++ b/arch/arm/mach-stm32mp/spl.c
> @@ -79,37 +79,36 @@ void spl_display_print(void)
>  void board_init_f(ulong dummy)
>  {
>  	struct udevice *dev;
> -	int ret;
> +	int ret, clk, reset, pinctrl;
>  
>  	arch_cpu_init();
>  
>  	ret = spl_early_init();
>  	if (ret) {
> -		debug("spl_early_init() failed: %d\n", ret);
> +		debug("%s: spl_early_init() failed: %d\n", __func__, ret);
>  		hang();
>  	}
>  
> -	ret = uclass_get_device(UCLASS_CLK, 0, &dev);
> -	if (ret) {
> -		debug("Clock init failed: %d\n", ret);
> -		return;
> -	}
> +	clk = uclass_get_device(UCLASS_CLK, 0, &dev);
> +	if (clk)
> +		debug("%s: Clock init failed: %d\n", __func__, clk);
>  
> -	ret = uclass_get_device(UCLASS_RESET, 0, &dev);
> -	if (ret) {
> -		debug("Reset init failed: %d\n", ret);
> -		return;
> -	}
> +	reset = uclass_get_device(UCLASS_RESET, 0, &dev);
> +	if (reset)
> +		debug("%s: Reset init failed: %d\n", __func__, reset);
>  
> -	ret = uclass_get_device(UCLASS_PINCTRL, 0, &dev);
> -	if (ret) {
> -		debug("%s: Cannot find pinctrl device\n", __func__);
> -		return;
> -	}
> +	pinctrl = uclass_get_device(UCLASS_PINCTRL, 0, &dev);
> +	if (pinctrl)
> +		debug("%s: Cannot find pinctrl device: %d\n",
> +		      __func__, pinctrl);
>  
>  	/* enable console uart printing */
>  	preloader_console_init();
>  
> +	if (clk || reset || pinctrl)
> +		printf("%s: probe failed clk=%d reset=%d pinctrl=%d\n",
> +		       __func__, clk, reset, pinctrl);
> +

This change makes little sense to me/  If you want error messages,
then just turn above debug() into printf(), and be done with it.
As an additional benefit so see at once which step failed.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A Chairman was as necessary to a Board planet  as  the  zero  was  in
mathematics, but being a zero had big disadvantages...
                         - Terry Pratchett, _The Dark Side of the Sun_

[PATCH 06/16] arm: stm32mp: spl: display error in board_init_f

[Patrick DELAUNAY]

Dear Wolfgang,

> From: Wolfgang Denk <wd@denx.de>
> Sent: mercredi 1 avril 2020 13:30
> 
> Dear Patrick Delaunay,
> 
> In message
> <20200331180330.6.I41a641a07fd12da45b392920fc3407e608926396@changeid>
> you wrote:
> > Update board_init_f and try to display error message when console is
> > available.
> >
> > This patch adds trace to debug a spl boot issue when DEBUG and
> > DEBUG_UART is not activated, after uart probe.
> >
> > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> > ---
> >
> >  arch/arm/mach-stm32mp/spl.c | 33 ++++++++++++++++-----------------
> >  1 file changed, 16 insertions(+), 17 deletions(-)
> >
> > diff --git a/arch/arm/mach-stm32mp/spl.c b/arch/arm/mach-stm32mp/spl.c
> > index ca4231cd0d..dfdb5bb7e9 100644
> > --- a/arch/arm/mach-stm32mp/spl.c
> > +++ b/arch/arm/mach-stm32mp/spl.c
> > @@ -79,37 +79,36 @@ void spl_display_print(void)  void
> > board_init_f(ulong dummy)  {
> >  	struct udevice *dev;
> > -	int ret;
> > +	int ret, clk, reset, pinctrl;
> >
> >  	arch_cpu_init();
> >
> >  	ret = spl_early_init();
> >  	if (ret) {
> > -		debug("spl_early_init() failed: %d\n", ret);
> > +		debug("%s: spl_early_init() failed: %d\n", __func__, ret);
> >  		hang();
> >  	}
> >
> > -	ret = uclass_get_device(UCLASS_CLK, 0, &dev);
> > -	if (ret) {
> > -		debug("Clock init failed: %d\n", ret);
> > -		return;
> > -	}
> > +	clk = uclass_get_device(UCLASS_CLK, 0, &dev);
> > +	if (clk)
> > +		debug("%s: Clock init failed: %d\n", __func__, clk);
> >
> > -	ret = uclass_get_device(UCLASS_RESET, 0, &dev);
> > -	if (ret) {
> > -		debug("Reset init failed: %d\n", ret);
> > -		return;
> > -	}
> > +	reset = uclass_get_device(UCLASS_RESET, 0, &dev);
> > +	if (reset)
> > +		debug("%s: Reset init failed: %d\n", __func__, reset);
> >
> > -	ret = uclass_get_device(UCLASS_PINCTRL, 0, &dev);
> > -	if (ret) {
> > -		debug("%s: Cannot find pinctrl device\n", __func__);
> > -		return;
> > -	}
> > +	pinctrl = uclass_get_device(UCLASS_PINCTRL, 0, &dev);
> > +	if (pinctrl)
> > +		debug("%s: Cannot find pinctrl device: %d\n",
> > +		      __func__, pinctrl);
> >
> >  	/* enable console uart printing */
> >  	preloader_console_init();
> >
> > +	if (clk || reset || pinctrl)
> > +		printf("%s: probe failed clk=%d reset=%d pinctrl=%d\n",
> > +		       __func__, clk, reset, pinctrl);
> > +
> 
> This change makes little sense to me/  If you want error messages, then just turn
> above debug() into printf(), and be done with it.
> As an additional benefit so see at once which step failed.

In this patch, I try to display error as soon as console is available
(after preloader_console_init) , if after one driver initialization
(clk, reset, pincontrol) failing.

Change debug to printf only works only if CONFIG_DEBUG_UART 
is activated (not by default) and board_debug_uart_init() exist to configure
the needed UART TX pins.

At least I need to remove the return and change them to hang() to interrupt SPL
execution if one probe failed to detect issue

I spent some time for this kind of issue: clock probe failed without any trace.
 

> Best regards,
> 
> Wolfgang Denk
> 
> --

Regards
Patrick

[PATCH 06/16] arm: stm32mp: spl: display error in board_init_f

[Wolfgang Denk]

Dear Patrick,

In message <8970fb86c1374d1999ff656c2a3272da@SFHDAG6NODE3.st.com> you wrote:
> 
> > > -	ret = uclass_get_device(UCLASS_CLK, 0, &dev);
> > > -	if (ret) {
> > > -		debug("Clock init failed: %d\n", ret);
> > > -		return;
> > > -	}
> > > +	clk = uclass_get_device(UCLASS_CLK, 0, &dev);
> > > +	if (clk)
> > > +		debug("%s: Clock init failed: %d\n", __func__, clk);
> > >
> > > -	ret = uclass_get_device(UCLASS_RESET, 0, &dev);
> > > -	if (ret) {
> > > -		debug("Reset init failed: %d\n", ret);
> > > -		return;
> > > -	}
> > > +	reset = uclass_get_device(UCLASS_RESET, 0, &dev);
> > > +	if (reset)
> > > +		debug("%s: Reset init failed: %d\n", __func__, reset);
> > >
> > > -	ret = uclass_get_device(UCLASS_PINCTRL, 0, &dev);
> > > -	if (ret) {
> > > -		debug("%s: Cannot find pinctrl device\n", __func__);
> > > -		return;
> > > -	}
> > > +	pinctrl = uclass_get_device(UCLASS_PINCTRL, 0, &dev);
> > > +	if (pinctrl)
> > > +		debug("%s: Cannot find pinctrl device: %d\n",
> > > +		      __func__, pinctrl);
> > >
> > >  	/* enable console uart printing */
> > >  	preloader_console_init();
> > >
> > > +	if (clk || reset || pinctrl)
> > > +		printf("%s: probe failed clk=%d reset=%d pinctrl=%d\n",
> > > +		       __func__, clk, reset, pinctrl);
> > > +
> > 
> > This change makes little sense to me/  If you want error messages, then just turn
> > above debug() into printf(), and be done with it.
> > As an additional benefit so see at once which step failed.
> 
> In this patch, I try to display error as soon as console is available
> (after preloader_console_init) , if after one driver initialization
> (clk, reset, pincontrol) failing.
> 
> Change debug to printf only works only if CONFIG_DEBUG_UART 
> is activated (not by default) and board_debug_uart_init() exist to configure
> the needed UART TX pins.

Maybe you can remember an error code so you can tell the user which
of the steps failed? That would be more useful, then.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I know engineers. They love to change things.             - Dr. McCoy

[PATCH 06/16] arm: stm32mp: spl: display error in board_init_f

[Patrick DELAUNAY]

Dear Wolfgang,

> From: Wolfgang Denk <wd@denx.de>
> Sent: jeudi 23 avril 2020 22:40
> 
> Dear Patrick,
> 
> In message <8970fb86c1374d1999ff656c2a3272da@SFHDAG6NODE3.st.com>
> you wrote:
> >

[...]

> > > >  	/* enable console uart printing */
> > > >  	preloader_console_init();
> > > >
> > > > +	if (clk || reset || pinctrl)
> > > > +		printf("%s: probe failed clk=%d reset=%d pinctrl=%d\n",
> > > > +		       __func__, clk, reset, pinctrl);
> > > > +
> > >
> > > This change makes little sense to me/  If you want error messages,
> > > then just turn above debug() into printf(), and be done with it.
> > > As an additional benefit so see at once which step failed.
> >
> > In this patch, I try to display error as soon as console is available
> > (after preloader_console_init) , if after one driver initialization
> > (clk, reset, pincontrol) failing.
> >
> > Change debug to printf only works only if CONFIG_DEBUG_UART is
> > activated (not by default) and board_debug_uart_init() exist to
> > configure the needed UART TX pins.
> 
> Maybe you can remember an error code so you can tell the user which of the steps
> failed? That would be more useful, then.

After check, I agree with you first comment.

Because console over serial can work only if the driver required for uart driver (clk, reset, pincontrol) can probe.

So CONFIG_DEBUG_UART is mandatory to identify the  failing step and debug() is enough in this case.

In fact my initial issue was only the simple return (and SPL boot continue) 
when a error was detected: I replace them by hang() calls.

That simplify the implementation in V2:
 
[PATCH v2 02/12] arm: stm32mp: spl: update error management in board_init_f
http://patchwork.ozlabs.org/project/uboot/patch/20200422142834.v2.2.I703cbd885066981e3bab374021d5578dce7cb035 at changeid/

Regards
 
 
> Best regards,
> 
> Wolfgang Denk
> 
> --
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> I know engineers. They love to change things.             - Dr. McCoy

Patrick

[PATCH 07/16] board: stm32mp1: Keep error led ON in case of low power detection

[Patrick Delaunay]

From: Patrice Chotard <patrice.chotard@st.com>

Since commit commit dd2810851eb1 ("stm32mp1: board: support of
error led on ed1/ev1 board") the attended behavior was no more
respected in case of low power source detection on DK2.

The expected behavior is either the error LED keeps blinking for ever,
or blinks 2 or 3 times and must stay ON.

Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

 board/st/stm32mp1/stm32mp1.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
index 07f5344ec9..8ed09ae24a 100644
--- a/board/st/stm32mp1/stm32mp1.c
+++ b/board/st/stm32mp1/stm32mp1.c
@@ -316,6 +316,7 @@ static void __maybe_unused led_error_blink(u32 nb_blink)
 			mdelay(125);
 			WATCHDOG_RESET();
 		}
+		led_set_state(led, LEDST_ON);
 	}
 #endif
 
-- 
2.17.1

[PATCH 07/16] board: stm32mp1: Keep error led ON in case of low power detection

[Patrice CHOTARD]

HI Patrick

On 3/31/20 6:04 PM, Patrick Delaunay wrote:
> From: Patrice Chotard <patrice.chotard@st.com>
>
> Since commit commit dd2810851eb1 ("stm32mp1: board: support of
> error led on ed1/ev1 board") the attended behavior was no more
> respected in case of low power source detection on DK2.
>
> The expected behavior is either the error LED keeps blinking for ever,
> or blinks 2 or 3 times and must stay ON.
>
> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  board/st/stm32mp1/stm32mp1.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
> index 07f5344ec9..8ed09ae24a 100644
> --- a/board/st/stm32mp1/stm32mp1.c
> +++ b/board/st/stm32mp1/stm32mp1.c
> @@ -316,6 +316,7 @@ static void __maybe_unused led_error_blink(u32 nb_blink)
>  			mdelay(125);
>  			WATCHDOG_RESET();
>  		}
> +		led_set_state(led, LEDST_ON);
>  	}
>  #endif
>  

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

Thanks

[PATCH 08/16] board: stm32mp1: update management of boot-led

[Patrick Delaunay]

Force boot-led ON and no more rely on default-state.
This patch avoid device-tree modification for U-Boot.

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

 arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi |  4 ----
 arch/arm/dts/stm32mp157c-ed1-u-boot.dtsi |  4 ----
 board/st/stm32mp1/stm32mp1.c             | 10 ++++++++--
 3 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi b/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi
index 5844d41c53..c52abeb1e7 100644
--- a/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi
+++ b/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi
@@ -27,10 +27,6 @@
 			default-state = "off";
 			status = "okay";
 		};
-
-		blue {
-			default-state = "on";
-		};
 	};
 };
 
diff --git a/arch/arm/dts/stm32mp157c-ed1-u-boot.dtsi b/arch/arm/dts/stm32mp157c-ed1-u-boot.dtsi
index ed2f024be9..84af7fa47b 100644
--- a/arch/arm/dts/stm32mp157c-ed1-u-boot.dtsi
+++ b/arch/arm/dts/stm32mp157c-ed1-u-boot.dtsi
@@ -28,10 +28,6 @@
 			default-state = "off";
 			status = "okay";
 		};
-
-		blue {
-			default-state = "on";
-		};
 	};
 };
 
diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
index 8ed09ae24a..6ca47509b3 100644
--- a/board/st/stm32mp1/stm32mp1.c
+++ b/board/st/stm32mp1/stm32mp1.c
@@ -280,9 +280,11 @@ static int get_led(struct udevice **dev, char *led_string)
 
 	return 0;
 }
+#endif
 
 static int setup_led(enum led_state_t cmd)
 {
+#ifdef CONFIG_LED
 	struct udevice *dev;
 	int ret;
 
@@ -292,8 +294,10 @@ static int setup_led(enum led_state_t cmd)
 
 	ret = led_set_state(dev, cmd);
 	return ret;
-}
+#else
+	return 0;
 #endif
+}
 
 static void __maybe_unused led_error_blink(u32 nb_blink)
 {
@@ -648,8 +652,10 @@ int board_init(void)
 
 	sysconf_init();
 
-	if (CONFIG_IS_ENABLED(CONFIG_LED))
+	if (CONFIG_IS_ENABLED(CONFIG_LED)) {
 		led_default_state();
+		setup_led(LEDST_ON);
+	}
 
 	return 0;
 }
-- 
2.17.1

[Uboot-stm32] [PATCH 08/16] board: stm32mp1: update management of boot-led

[Patrice CHOTARD]

Hi Patrick

On 3/31/20 6:04 PM, Patrick Delaunay wrote:
> Force boot-led ON and no more rely on default-state.
> This patch avoid device-tree modification for U-Boot.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi |  4 ----
>  arch/arm/dts/stm32mp157c-ed1-u-boot.dtsi |  4 ----
>  board/st/stm32mp1/stm32mp1.c             | 10 ++++++++--
>  3 files changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi b/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi
> index 5844d41c53..c52abeb1e7 100644
> --- a/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi
> +++ b/arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi
> @@ -27,10 +27,6 @@
>  			default-state = "off";
>  			status = "okay";
>  		};
> -
> -		blue {
> -			default-state = "on";
> -		};
>  	};
>  };
>  
> diff --git a/arch/arm/dts/stm32mp157c-ed1-u-boot.dtsi b/arch/arm/dts/stm32mp157c-ed1-u-boot.dtsi
> index ed2f024be9..84af7fa47b 100644
> --- a/arch/arm/dts/stm32mp157c-ed1-u-boot.dtsi
> +++ b/arch/arm/dts/stm32mp157c-ed1-u-boot.dtsi
> @@ -28,10 +28,6 @@
>  			default-state = "off";
>  			status = "okay";
>  		};
> -
> -		blue {
> -			default-state = "on";
> -		};
>  	};
>  };
>  
> diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
> index 8ed09ae24a..6ca47509b3 100644
> --- a/board/st/stm32mp1/stm32mp1.c
> +++ b/board/st/stm32mp1/stm32mp1.c
> @@ -280,9 +280,11 @@ static int get_led(struct udevice **dev, char *led_string)
>  
>  	return 0;
>  }
> +#endif
>  
>  static int setup_led(enum led_state_t cmd)
>  {
> +#ifdef CONFIG_LED
>  	struct udevice *dev;
>  	int ret;
>  
> @@ -292,8 +294,10 @@ static int setup_led(enum led_state_t cmd)
>  
>  	ret = led_set_state(dev, cmd);
>  	return ret;
> -}
> +#else
> +	return 0;
>  #endif
> +}
>  
>  static void __maybe_unused led_error_blink(u32 nb_blink)
>  {
> @@ -648,8 +652,10 @@ int board_init(void)
>  
>  	sysconf_init();
>  
> -	if (CONFIG_IS_ENABLED(CONFIG_LED))
> +	if (CONFIG_IS_ENABLED(CONFIG_LED)) {
>  		led_default_state();
> +		setup_led(LEDST_ON);
> +	}
>  
>  	return 0;
>  }

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

Thanks

[PATCH 08/16] board: stm32mp1: update management of boot-led

[Wolfgang Denk]

Dear Patrick Delaunay,

In message <20200331180330.8.I15cb0a6245fb4cd5d23371683c2697f794adf306@changeid> you wrote:
> Force boot-led ON and no more rely on default-state.
> This patch avoid device-tree modification for U-Boot.
...

> +#ifdef CONFIG_LED
>  	struct udevice *dev;
>  	int ret;
>  
> @@ -292,8 +294,10 @@ static int setup_led(enum led_state_t cmd)
>  
>  	ret = led_set_state(dev, cmd);
>  	return ret;
> -}
> +#else
> +	return 0;
>  #endif
> +}
>  
>  static void __maybe_unused led_error_blink(u32 nb_blink)
>  {
> @@ -648,8 +652,10 @@ int board_init(void)
>  
>  	sysconf_init();
>  
> -	if (CONFIG_IS_ENABLED(CONFIG_LED))
> +	if (CONFIG_IS_ENABLED(CONFIG_LED)) {
>  		led_default_state();
> +		setup_led(LEDST_ON);
> +	}

This is inconsistent, please use CONFIG_IS_ENABLED() everywhere
instead of #ifdef's.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Q:  Do you know what the death rate around here is?
A:  One per person.

[PATCH 08/16] board: stm32mp1: update management of boot-led

[Anatolij Gustschin]

Hi Patrick,

On Tue, 31 Mar 2020 18:04:25 +0200
Patrick Delaunay patrick.delaunay at st.com wrote:
...
> @@ -648,8 +652,10 @@ int board_init(void)
>  
>  	sysconf_init();
>  
> -	if (CONFIG_IS_ENABLED(CONFIG_LED))
> +	if (CONFIG_IS_ENABLED(CONFIG_LED)) {
>  		led_default_state();

Did you verify that this works like expected? We either use
	if (CONFIG_IS_ENABLED(LED))
or
	if (IS_ENABLED(CONFIG_LED))

Please check.

--
Anatolij

[PATCH 08/16] board: stm32mp1: update management of boot-led

[Patrick DELAUNAY]

Dear Anatolij

> From: Anatolij Gustschin <agust@denx.de>
> Sent: mercredi 1 avril 2020 13:43
> 
> Hi Patrick,
> 
> On Tue, 31 Mar 2020 18:04:25 +0200
> Patrick Delaunay patrick.delaunay at st.com wrote:
> ...
> > @@ -648,8 +652,10 @@ int board_init(void)
> >
> >  	sysconf_init();
> >
> > -	if (CONFIG_IS_ENABLED(CONFIG_LED))
> > +	if (CONFIG_IS_ENABLED(CONFIG_LED)) {
> >  		led_default_state();
> 
> Did you verify that this works like expected? We either use
> 	if (CONFIG_IS_ENABLED(LED))
> or
> 	if (IS_ENABLED(CONFIG_LED))
> 
> Please check.

You are right: it is not working.

I had already make this error,
I will solve the issue a in separate patch (also impacting dh_stm32mp1).

Thanks for review.

> --
> Anatolij

Patrick

[PATCH 09/16] board: stm32mp1: gt9147 IRQ before reset on EV1

[Patrick Delaunay]

Software workaround for I2C issue on EV1 board,
configure the IRQ line for touchscreen before LCD reset
to fix the used I2C address.

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

 board/st/stm32mp1/stm32mp1.c | 41 ++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
index 6ca47509b3..52881adef7 100644
--- a/board/st/stm32mp1/stm32mp1.c
+++ b/board/st/stm32mp1/stm32mp1.c
@@ -626,6 +626,44 @@ static bool board_is_dk2(void)
 }
 #endif
 
+static bool board_is_ev1(void)
+{
+	if (CONFIG_IS_ENABLED(TARGET_ST_STM32MP15x) &&
+	    (of_machine_is_compatible("st,stm32mp157a-ev1") ||
+	     of_machine_is_compatible("st,stm32mp157c-ev1") ||
+	     of_machine_is_compatible("st,stm32mp157d-ev1") ||
+	     of_machine_is_compatible("st,stm32mp157f-ev1")))
+		return true;
+
+	return false;
+}
+
+/* touchscreen driver: only used for pincontrol configuration */
+static const struct udevice_id goodix_ids[] = {
+	{ .compatible = "goodix,gt9147", },
+	{ }
+};
+
+U_BOOT_DRIVER(goodix) = {
+	.name		= "goodix",
+	.id		= UCLASS_NOP,
+	.of_match	= goodix_ids,
+};
+
+static int board_ev1_init(void)
+{
+	struct udevice *dev;
+	int ret;
+
+	/* configure IRQ line on EV1 for touchscreen before LCD reset */
+	ret = uclass_get_device_by_driver(UCLASS_NOP, DM_GET_DRIVER(goodix),
+					  &dev);
+	if (ret)
+		debug("goodix init failed: %d\n", ret);
+
+	return ret;
+}
+
 /* board dependent setup after realloc */
 int board_init(void)
 {
@@ -643,6 +681,9 @@ int board_init(void)
 
 	board_key_check();
 
+	if (board_is_ev1())
+		board_ev1_init();
+
 #ifdef CONFIG_DM_REGULATOR
 	if (board_is_dk2())
 		dk2_i2c1_fix();
-- 
2.17.1

[Uboot-stm32] [PATCH 09/16] board: stm32mp1: gt9147 IRQ before reset on EV1

[Patrice CHOTARD]

HI Patrick

On 3/31/20 6:04 PM, Patrick Delaunay wrote:
> Software workaround for I2C issue on EV1 board,
> configure the IRQ line for touchscreen before LCD reset
> to fix the used I2C address.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  board/st/stm32mp1/stm32mp1.c | 41 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>
> diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
> index 6ca47509b3..52881adef7 100644
> --- a/board/st/stm32mp1/stm32mp1.c
> +++ b/board/st/stm32mp1/stm32mp1.c
> @@ -626,6 +626,44 @@ static bool board_is_dk2(void)
>  }
>  #endif
>  
> +static bool board_is_ev1(void)
> +{
> +	if (CONFIG_IS_ENABLED(TARGET_ST_STM32MP15x) &&
> +	    (of_machine_is_compatible("st,stm32mp157a-ev1") ||
> +	     of_machine_is_compatible("st,stm32mp157c-ev1") ||
> +	     of_machine_is_compatible("st,stm32mp157d-ev1") ||
> +	     of_machine_is_compatible("st,stm32mp157f-ev1")))
> +		return true;
> +
> +	return false;
> +}
> +
> +/* touchscreen driver: only used for pincontrol configuration */
> +static const struct udevice_id goodix_ids[] = {
> +	{ .compatible = "goodix,gt9147", },
> +	{ }
> +};
> +
> +U_BOOT_DRIVER(goodix) = {
> +	.name		= "goodix",
> +	.id		= UCLASS_NOP,
> +	.of_match	= goodix_ids,
> +};
> +
> +static int board_ev1_init(void)
> +{
> +	struct udevice *dev;
> +	int ret;
> +
> +	/* configure IRQ line on EV1 for touchscreen before LCD reset */
> +	ret = uclass_get_device_by_driver(UCLASS_NOP, DM_GET_DRIVER(goodix),
> +					  &dev);
> +	if (ret)
> +		debug("goodix init failed: %d\n", ret);
> +
> +	return ret;
> +}
> +
>  /* board dependent setup after realloc */
>  int board_init(void)
>  {
> @@ -643,6 +681,9 @@ int board_init(void)
>  
>  	board_key_check();
>  
> +	if (board_is_ev1())
> +		board_ev1_init();
> +
>  #ifdef CONFIG_DM_REGULATOR
>  	if (board_is_dk2())
>  		dk2_i2c1_fix();

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

Thanks

[PATCH 09/16] board: stm32mp1: gt9147 IRQ before reset on EV1

[Wolfgang Denk]

Dear Patrick Delaunay,

In message <20200331180330.9.I5d296f8fd82b60a592b51029e7e420672d0e855b@changeid> you wrote:
> Software workaround for I2C issue on EV1 board,
> configure the IRQ line for touchscreen before LCD reset
> to fix the used I2C address.
...

> +	ret = uclass_get_device_by_driver(UCLASS_NOP, DM_GET_DRIVER(goodix),
> +					  &dev);
> +	if (ret)
> +		debug("goodix init failed: %d\n", ret);

If this is an error condition, you should use printf(), and not
paper over it with a debug().

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Don't have a battle of wits with an unarmed opponent.

[PATCH 10/16] board: stm32mp1: set environment variable fdtfile

[Patrick Delaunay]

For booting Linux in the generic distro mechanism and support of
FDTDIR in extlinux.conf , cmd/pxe.c retrieves the FDT file name
from "fdtfile" environment variable.

Dynamically build this variable with compatible of STMicroelectronics
boards.

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

 board/st/stm32mp1/stm32mp1.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
index 52881adef7..89a088cd28 100644
--- a/board/st/stm32mp1/stm32mp1.c
+++ b/board/st/stm32mp1/stm32mp1.c
@@ -715,10 +715,19 @@ int board_late_init(void)
 	fdt_compat = fdt_getprop(gd->fdt_blob, 0, "compatible",
 				 &fdt_compat_len);
 	if (fdt_compat && fdt_compat_len) {
-		if (strncmp(fdt_compat, "st,", 3) != 0)
+		if (strncmp(fdt_compat, "st,", 3) != 0) {
 			env_set("board_name", fdt_compat);
-		else
+		} else {
+			char dtb_name[256];
+			int buf_len = sizeof(dtb_name);
+
 			env_set("board_name", fdt_compat + 3);
+
+			strncpy(dtb_name, fdt_compat + 3, buf_len);
+			buf_len -= strlen(fdt_compat + 3);
+			strncat(dtb_name, ".dtb", buf_len);
+			env_set("fdtfile", dtb_name);
+		}
 	}
 	ret = uclass_get_device_by_driver(UCLASS_MISC,
 					  DM_GET_DRIVER(stm32mp_bsec),
-- 
2.17.1

[Uboot-stm32] [PATCH 10/16] board: stm32mp1: set environment variable fdtfile

[Patrice CHOTARD]

Hi PAtrick

On 3/31/20 6:04 PM, Patrick Delaunay wrote:
> For booting Linux in the generic distro mechanism and support of
> FDTDIR in extlinux.conf , cmd/pxe.c retrieves the FDT file name
> from "fdtfile" environment variable.
>
> Dynamically build this variable with compatible of STMicroelectronics
> boards.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  board/st/stm32mp1/stm32mp1.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
> index 52881adef7..89a088cd28 100644
> --- a/board/st/stm32mp1/stm32mp1.c
> +++ b/board/st/stm32mp1/stm32mp1.c
> @@ -715,10 +715,19 @@ int board_late_init(void)
>  	fdt_compat = fdt_getprop(gd->fdt_blob, 0, "compatible",
>  				 &fdt_compat_len);
>  	if (fdt_compat && fdt_compat_len) {
> -		if (strncmp(fdt_compat, "st,", 3) != 0)
> +		if (strncmp(fdt_compat, "st,", 3) != 0) {
>  			env_set("board_name", fdt_compat);
> -		else
> +		} else {
> +			char dtb_name[256];
> +			int buf_len = sizeof(dtb_name);
> +
>  			env_set("board_name", fdt_compat + 3);
> +
> +			strncpy(dtb_name, fdt_compat + 3, buf_len);
> +			buf_len -= strlen(fdt_compat + 3);
> +			strncat(dtb_name, ".dtb", buf_len);
> +			env_set("fdtfile", dtb_name);
> +		}
>  	}
>  	ret = uclass_get_device_by_driver(UCLASS_MISC,
>  					  DM_GET_DRIVER(stm32mp_bsec),

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

Thanks

[PATCH 11/16] board: stm32mp1: check env_get result in board_late_init

[Patrick Delaunay]

This patch avoids crash in strcmp when the boot_device is not
present in environment (this case should be never occur)

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

 board/st/stm32mp1/stm32mp1.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
index 89a088cd28..fff4cef2c2 100644
--- a/board/st/stm32mp1/stm32mp1.c
+++ b/board/st/stm32mp1/stm32mp1.c
@@ -753,7 +753,8 @@ int board_late_init(void)
 
 	/* Check the boot-source to disable bootdelay */
 	boot_device = env_get("boot_device");
-	if (!strcmp(boot_device, "serial") || !strcmp(boot_device, "usb"))
+	if (boot_device &&
+	    (!strcmp(boot_device, "serial") || !strcmp(boot_device, "usb")))
 		env_set("bootdelay", "0");
 
 	return 0;
-- 
2.17.1

[Uboot-stm32] [PATCH 11/16] board: stm32mp1: check env_get result in board_late_init

[Patrice CHOTARD]

Hi Patrick

On 3/31/20 6:04 PM, Patrick Delaunay wrote:
> This patch avoids crash in strcmp when the boot_device is not
> present in environment (this case should be never occur)
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  board/st/stm32mp1/stm32mp1.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
> index 89a088cd28..fff4cef2c2 100644
> --- a/board/st/stm32mp1/stm32mp1.c
> +++ b/board/st/stm32mp1/stm32mp1.c
> @@ -753,7 +753,8 @@ int board_late_init(void)
>  
>  	/* Check the boot-source to disable bootdelay */
>  	boot_device = env_get("boot_device");
> -	if (!strcmp(boot_device, "serial") || !strcmp(boot_device, "usb"))
> +	if (boot_device &&
> +	    (!strcmp(boot_device, "serial") || !strcmp(boot_device, "usb")))
>  		env_set("bootdelay", "0");
>  
>  	return 0;

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

Thanks

[PATCH 11/16] board: stm32mp1: check env_get result in board_late_init

[Wolfgang Denk]

Dear Patrick Delaunay,

In message <20200331180330.11.Ic051e25812481db408f2431c7962da1db1f198fb@changeid> you wrote:
> This patch avoids crash in strcmp when the boot_device is not
> present in environment (this case should be never occur)
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  board/st/stm32mp1/stm32mp1.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
> index 89a088cd28..fff4cef2c2 100644
> --- a/board/st/stm32mp1/stm32mp1.c
> +++ b/board/st/stm32mp1/stm32mp1.c
> @@ -753,7 +753,8 @@ int board_late_init(void)
>  
>  	/* Check the boot-source to disable bootdelay */
>  	boot_device = env_get("boot_device");
> -	if (!strcmp(boot_device, "serial") || !strcmp(boot_device, "usb"))
> +	if (boot_device &&
> +	    (!strcmp(boot_device, "serial") || !strcmp(boot_device, "usb")))
>  		env_set("bootdelay", "0");

I think this is generally a bad idea.  You should have more respect
to the intentions of your users.  If a user defines a specific
bootdelay setting in his environment, this must be respected.

I really hate vendors who believe they know better than me what I
want or what is good to me.  Please get rid of such stuff.

"bootdelay" is an environment variable that is intentionally user
definable. You must never mess with those.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Everything should be made as simple as possible, but not simpler."
                                                    - Albert Einstein

[PATCH 12/16] board: stm32mp1: add timeout for I/O compensation ready

[Patrick Delaunay]

This patch avoids infinite loop when I/O compensation failed,
it adds a 1s timeout to detect error.

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

 board/st/stm32mp1/stm32mp1.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
index fff4cef2c2..75aac6d66c 100644
--- a/board/st/stm32mp1/stm32mp1.c
+++ b/board/st/stm32mp1/stm32mp1.c
@@ -35,6 +35,7 @@
 #include <asm/arch/sys_proto.h>
 #include <jffs2/load_kernel.h>
 #include <linux/err.h>
+#include <linux/iopoll.h>
 #include <power/regulator.h>
 #include <usb/dwc2_udc.h>
 
@@ -473,10 +474,10 @@ static void sysconf_init(void)
 	struct udevice *pwr_dev;
 	struct udevice *pwr_reg;
 	struct udevice *dev;
-	int ret;
 	u32 otp = 0;
 #endif
-	u32 bootr;
+	int ret;
+	u32 bootr, val;
 
 	syscfg = (u8 *)syscon_get_first_range(STM32MP_SYSCON_SYSCFG);
 
@@ -553,8 +554,15 @@ static void sysconf_init(void)
 	 */
 	writel(SYSCFG_CMPENSETR_MPU_EN, syscfg + SYSCFG_CMPENSETR);
 
-	while (!(readl(syscfg + SYSCFG_CMPCR) & SYSCFG_CMPCR_READY))
-		;
+	/* poll until ready (1s timeout) */
+	ret = readl_poll_timeout(syscfg + SYSCFG_CMPCR, val,
+				 val & SYSCFG_CMPCR_READY,
+				 1000000);
+	if (ret) {
+		pr_err("SYSCFG: I/O compensation failed, timeout.\n");
+		led_error_blink(10);
+	}
+
 	clrbits_le32(syscfg + SYSCFG_CMPCR, SYSCFG_CMPCR_SW_CTRL);
 #endif
 }
-- 
2.17.1

[Uboot-stm32] [PATCH 12/16] board: stm32mp1: add timeout for I/O compensation ready

[Patrice CHOTARD]

Hi Patrick

On 3/31/20 6:04 PM, Patrick Delaunay wrote:
> This patch avoids infinite loop when I/O compensation failed,
> it adds a 1s timeout to detect error.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  board/st/stm32mp1/stm32mp1.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
> index fff4cef2c2..75aac6d66c 100644
> --- a/board/st/stm32mp1/stm32mp1.c
> +++ b/board/st/stm32mp1/stm32mp1.c
> @@ -35,6 +35,7 @@
>  #include <asm/arch/sys_proto.h>
>  #include <jffs2/load_kernel.h>
>  #include <linux/err.h>
> +#include <linux/iopoll.h>
>  #include <power/regulator.h>
>  #include <usb/dwc2_udc.h>
>  
> @@ -473,10 +474,10 @@ static void sysconf_init(void)
>  	struct udevice *pwr_dev;
>  	struct udevice *pwr_reg;
>  	struct udevice *dev;
> -	int ret;
>  	u32 otp = 0;
>  #endif
> -	u32 bootr;
> +	int ret;
> +	u32 bootr, val;
>  
>  	syscfg = (u8 *)syscon_get_first_range(STM32MP_SYSCON_SYSCFG);
>  
> @@ -553,8 +554,15 @@ static void sysconf_init(void)
>  	 */
>  	writel(SYSCFG_CMPENSETR_MPU_EN, syscfg + SYSCFG_CMPENSETR);
>  
> -	while (!(readl(syscfg + SYSCFG_CMPCR) & SYSCFG_CMPCR_READY))
> -		;
> +	/* poll until ready (1s timeout) */
> +	ret = readl_poll_timeout(syscfg + SYSCFG_CMPCR, val,
> +				 val & SYSCFG_CMPCR_READY,
> +				 1000000);
> +	if (ret) {
> +		pr_err("SYSCFG: I/O compensation failed, timeout.\n");
> +		led_error_blink(10);
> +	}
> +
>  	clrbits_le32(syscfg + SYSCFG_CMPCR, SYSCFG_CMPCR_SW_CTRL);
>  #endif
>  }

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

Thanks

[PATCH 13/16] gpio: stm32: support gpio ops in SPL

[Patrick Delaunay]

The GPIO support is needed in SPL to managed the SD cart detect
used on stm32mp157c-ev1 and dk2 board.
So this patch activates the associated code in stm32_gpio.c.

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

 drivers/gpio/stm32_gpio.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpio/stm32_gpio.c b/drivers/gpio/stm32_gpio.c
index f55f834e7d..37a8cfa47a 100644
--- a/drivers/gpio/stm32_gpio.c
+++ b/drivers/gpio/stm32_gpio.c
@@ -20,7 +20,6 @@
 #define MODE_BITS_MASK			3
 #define BSRR_BIT(gpio_pin, value)	BIT(gpio_pin + (value ? 0 : 16))
 
-#ifndef CONFIG_SPL_BUILD
 /*
  * convert gpio offset to gpio index taking into account gpio holes
  * into gpio bank
@@ -147,7 +146,6 @@ static const struct dm_gpio_ops gpio_stm32_ops = {
 	.set_value		= stm32_gpio_set_value,
 	.get_function		= stm32_gpio_get_function,
 };
-#endif
 
 static int gpio_stm32_probe(struct udevice *dev)
 {
@@ -162,7 +160,6 @@ static int gpio_stm32_probe(struct udevice *dev)
 
 	priv->regs = (struct stm32_gpio_regs *)addr;
 
-#ifndef CONFIG_SPL_BUILD
 	struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
 	struct ofnode_phandle_args args;
 	const char *name;
@@ -195,7 +192,7 @@ static int gpio_stm32_probe(struct udevice *dev)
 	dev_dbg(dev, "addr = 0x%p bank_name = %s gpio_count = %d gpio_range = 0x%x\n",
 		(u32 *)priv->regs, uc_priv->bank_name, uc_priv->gpio_count,
 		priv->gpio_range);
-#endif
+
 	ret = clk_get_by_index(dev, 0, &clk);
 	if (ret < 0)
 		return ret;
@@ -215,9 +212,7 @@ U_BOOT_DRIVER(gpio_stm32) = {
 	.name	= "gpio_stm32",
 	.id	= UCLASS_GPIO,
 	.probe	= gpio_stm32_probe,
-#ifndef CONFIG_SPL_BUILD
 	.ops	= &gpio_stm32_ops,
-#endif
 	.flags	= DM_UC_FLAG_SEQ_ALIAS,
 	.priv_auto_alloc_size	= sizeof(struct stm32_gpio_priv),
 };
-- 
2.17.1

[PATCH 13/16] gpio: stm32: support gpio ops in SPL

[Patrice CHOTARD]

Hi Patrick

On 3/31/20 6:04 PM, Patrick Delaunay wrote:
> The GPIO support is needed in SPL to managed the SD cart detect
> used on stm32mp157c-ev1 and dk2 board.
> So this patch activates the associated code in stm32_gpio.c.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  drivers/gpio/stm32_gpio.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/gpio/stm32_gpio.c b/drivers/gpio/stm32_gpio.c
> index f55f834e7d..37a8cfa47a 100644
> --- a/drivers/gpio/stm32_gpio.c
> +++ b/drivers/gpio/stm32_gpio.c
> @@ -20,7 +20,6 @@
>  #define MODE_BITS_MASK			3
>  #define BSRR_BIT(gpio_pin, value)	BIT(gpio_pin + (value ? 0 : 16))
>  
> -#ifndef CONFIG_SPL_BUILD
>  /*
>   * convert gpio offset to gpio index taking into account gpio holes
>   * into gpio bank
> @@ -147,7 +146,6 @@ static const struct dm_gpio_ops gpio_stm32_ops = {
>  	.set_value		= stm32_gpio_set_value,
>  	.get_function		= stm32_gpio_get_function,
>  };
> -#endif
>  
>  static int gpio_stm32_probe(struct udevice *dev)
>  {
> @@ -162,7 +160,6 @@ static int gpio_stm32_probe(struct udevice *dev)
>  
>  	priv->regs = (struct stm32_gpio_regs *)addr;
>  
> -#ifndef CONFIG_SPL_BUILD
>  	struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
>  	struct ofnode_phandle_args args;
>  	const char *name;
> @@ -195,7 +192,7 @@ static int gpio_stm32_probe(struct udevice *dev)
>  	dev_dbg(dev, "addr = 0x%p bank_name = %s gpio_count = %d gpio_range = 0x%x\n",
>  		(u32 *)priv->regs, uc_priv->bank_name, uc_priv->gpio_count,
>  		priv->gpio_range);
> -#endif
> +
>  	ret = clk_get_by_index(dev, 0, &clk);
>  	if (ret < 0)
>  		return ret;
> @@ -215,9 +212,7 @@ U_BOOT_DRIVER(gpio_stm32) = {
>  	.name	= "gpio_stm32",
>  	.id	= UCLASS_GPIO,
>  	.probe	= gpio_stm32_probe,
> -#ifndef CONFIG_SPL_BUILD
>  	.ops	= &gpio_stm32_ops,
> -#endif
>  	.flags	= DM_UC_FLAG_SEQ_ALIAS,
>  	.priv_auto_alloc_size	= sizeof(struct stm32_gpio_priv),
>  };

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

Thanks

[PATCH 14/16] ARM: dts: stm32mp15: use DDR3 files generated by STM32CubeMX

[Patrick Delaunay]

Use the DDR3 dtsi files generated by STM32CubeMX 5.6.0
Speed Bin Grade = using DDR3-1066G / 8-8-8 and all others
parameters at default value.

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

 .../dts/stm32mp15-ddr3-1x4Gb-1066-binG.dtsi   | 49 +++++++++----------
 .../dts/stm32mp15-ddr3-2x4Gb-1066-binG.dtsi   | 49 +++++++++----------
 2 files changed, 48 insertions(+), 50 deletions(-)

diff --git a/arch/arm/dts/stm32mp15-ddr3-1x4Gb-1066-binG.dtsi b/arch/arm/dts/stm32mp15-ddr3-1x4Gb-1066-binG.dtsi
index 11e8f2bef6..c0fc1f772e 100644
--- a/arch/arm/dts/stm32mp15-ddr3-1x4Gb-1066-binG.dtsi
+++ b/arch/arm/dts/stm32mp15-ddr3-1x4Gb-1066-binG.dtsi
@@ -1,24 +1,23 @@
 // SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause
 /*
  * Copyright (C) 2018, STMicroelectronics - All Rights Reserved
+ */
+
+/*
+ * File generated by STMicroelectronics STM32CubeMX DDR Tool for MPUs
+ * DDR type: DDR3 / DDR3L
+ * DDR width: 16bits
+ * DDR density: 4Gb
+ * System frequency: 533000Khz
+ * Relaxed Timing Mode: false
+ * Address mapping type: RBC
  *
- * STM32MP157C DK1/DK2 BOARD configuration
- * 1x DDR3L 4Gb, 16-bit, 533MHz.
- * Reference used NT5CC256M16DP-DI from NANYA
- *
- * DDR type / Platform	DDR3/3L
- * freq		533MHz
- * width	16
- * datasheet	0  = MT41J256M16-187 / DDR3-1066 bin G
- * DDR density	4
- * timing mode	optimized
- * Scheduling/QoS options : type = 2
- * address mapping : RBC
- * Tc > + 85C : N
+ * Save Date: 2020.02.20, save Time: 18:45:20
  */
-#define DDR_MEM_NAME "DDR3-1066/888 bin G 1x4Gb 533MHz v1.45"
-#define DDR_MEM_SPEED 533000
-#define DDR_MEM_SIZE 0x20000000
+
+#define DDR_MEM_NAME	"DDR3-DDR3L 16bits 533000Khz"
+#define DDR_MEM_SPEED	533000
+#define DDR_MEM_SIZE	0x20000000
 
 #define DDR_MSTR 0x00041401
 #define DDR_MRCTRL0 0x00000010
@@ -50,15 +49,6 @@
 #define DDR_DFIUPD1 0x00000000
 #define DDR_DFIUPD2 0x00000000
 #define DDR_DFIPHYMSTR 0x00000000
-#define DDR_ADDRMAP1 0x00070707
-#define DDR_ADDRMAP2 0x00000000
-#define DDR_ADDRMAP3 0x1F000000
-#define DDR_ADDRMAP4 0x00001F1F
-#define DDR_ADDRMAP5 0x06060606
-#define DDR_ADDRMAP6 0x0F060606
-#define DDR_ADDRMAP9 0x00000000
-#define DDR_ADDRMAP10 0x00000000
-#define DDR_ADDRMAP11 0x00000000
 #define DDR_ODTCFG 0x06000600
 #define DDR_ODTMAP 0x00000001
 #define DDR_SCHED 0x00000C01
@@ -83,6 +73,15 @@
 #define DDR_PCFGQOS1_1 0x00800040
 #define DDR_PCFGWQOS0_1 0x01100C03
 #define DDR_PCFGWQOS1_1 0x01000200
+#define DDR_ADDRMAP1 0x00070707
+#define DDR_ADDRMAP2 0x00000000
+#define DDR_ADDRMAP3 0x1F000000
+#define DDR_ADDRMAP4 0x00001F1F
+#define DDR_ADDRMAP5 0x06060606
+#define DDR_ADDRMAP6 0x0F060606
+#define DDR_ADDRMAP9 0x00000000
+#define DDR_ADDRMAP10 0x00000000
+#define DDR_ADDRMAP11 0x00000000
 #define DDR_PGCR 0x01442E02
 #define DDR_PTR0 0x0022AA5B
 #define DDR_PTR1 0x04841104
diff --git a/arch/arm/dts/stm32mp15-ddr3-2x4Gb-1066-binG.dtsi b/arch/arm/dts/stm32mp15-ddr3-2x4Gb-1066-binG.dtsi
index 4b70b60554..fc226d2544 100644
--- a/arch/arm/dts/stm32mp15-ddr3-2x4Gb-1066-binG.dtsi
+++ b/arch/arm/dts/stm32mp15-ddr3-2x4Gb-1066-binG.dtsi
@@ -1,24 +1,23 @@
 // SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause
 /*
  * Copyright (C) 2018, STMicroelectronics - All Rights Reserved
+ */
+
+/*
+ * File generated by STMicroelectronics STM32CubeMX DDR Tool for MPUs
+ * DDR type: DDR3 / DDR3L
+ * DDR width: 32bits
+ * DDR density: 8Gb
+ * System frequency: 533000Khz
+ * Relaxed Timing Mode: false
+ * Address mapping type: RBC
  *
- * STM32MP157C ED1 BOARD configuration
- * 2x DDR3L 4Gb each, 16-bit, 533MHz, Single Die Package in flyby topology.
- * Reference used NT5CC256M16DP-DI from NANYA
- *
- * DDR type / Platform	DDR3/3L
- * freq		533MHz
- * width	32
- * datasheet	0  = MT41J256M16-187 / DDR3-1066 bin G
- * DDR density	8
- * timing mode	optimized
- * Scheduling/QoS options : type = 2
- * address mapping : RBC
- * Tc > + 85C : N
+ * Save Date: 2020.02.20, save Time: 18:49:33
  */
-#define DDR_MEM_NAME "DDR3-1066/888 bin G 2x4Gb 533MHz v1.45"
-#define DDR_MEM_SPEED 533000
-#define DDR_MEM_SIZE 0x40000000
+
+#define DDR_MEM_NAME	"DDR3-DDR3L 32bits 533000Khz"
+#define DDR_MEM_SPEED	533000
+#define DDR_MEM_SIZE	0x40000000
 
 #define DDR_MSTR 0x00040401
 #define DDR_MRCTRL0 0x00000010
@@ -50,15 +49,6 @@
 #define DDR_DFIUPD1 0x00000000
 #define DDR_DFIUPD2 0x00000000
 #define DDR_DFIPHYMSTR 0x00000000
-#define DDR_ADDRMAP1 0x00080808
-#define DDR_ADDRMAP2 0x00000000
-#define DDR_ADDRMAP3 0x00000000
-#define DDR_ADDRMAP4 0x00001F1F
-#define DDR_ADDRMAP5 0x07070707
-#define DDR_ADDRMAP6 0x0F070707
-#define DDR_ADDRMAP9 0x00000000
-#define DDR_ADDRMAP10 0x00000000
-#define DDR_ADDRMAP11 0x00000000
 #define DDR_ODTCFG 0x06000600
 #define DDR_ODTMAP 0x00000001
 #define DDR_SCHED 0x00000C01
@@ -83,6 +73,15 @@
 #define DDR_PCFGQOS1_1 0x00800040
 #define DDR_PCFGWQOS0_1 0x01100C03
 #define DDR_PCFGWQOS1_1 0x01000200
+#define DDR_ADDRMAP1 0x00080808
+#define DDR_ADDRMAP2 0x00000000
+#define DDR_ADDRMAP3 0x00000000
+#define DDR_ADDRMAP4 0x00001F1F
+#define DDR_ADDRMAP5 0x07070707
+#define DDR_ADDRMAP6 0x0F070707
+#define DDR_ADDRMAP9 0x00000000
+#define DDR_ADDRMAP10 0x00000000
+#define DDR_ADDRMAP11 0x00000000
 #define DDR_PGCR 0x01442E02
 #define DDR_PTR0 0x0022AA5B
 #define DDR_PTR1 0x04841104
-- 
2.17.1

[Uboot-stm32] [PATCH 14/16] ARM: dts: stm32mp15: use DDR3 files generated by STM32CubeMX

[Patrice CHOTARD]

Hi Patrick

On 3/31/20 6:04 PM, Patrick Delaunay wrote:
> Use the DDR3 dtsi files generated by STM32CubeMX 5.6.0
> Speed Bin Grade = using DDR3-1066G / 8-8-8 and all others
> parameters at default value.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  .../dts/stm32mp15-ddr3-1x4Gb-1066-binG.dtsi   | 49 +++++++++----------
>  .../dts/stm32mp15-ddr3-2x4Gb-1066-binG.dtsi   | 49 +++++++++----------
>  2 files changed, 48 insertions(+), 50 deletions(-)
>
> diff --git a/arch/arm/dts/stm32mp15-ddr3-1x4Gb-1066-binG.dtsi b/arch/arm/dts/stm32mp15-ddr3-1x4Gb-1066-binG.dtsi
> index 11e8f2bef6..c0fc1f772e 100644
> --- a/arch/arm/dts/stm32mp15-ddr3-1x4Gb-1066-binG.dtsi
> +++ b/arch/arm/dts/stm32mp15-ddr3-1x4Gb-1066-binG.dtsi
> @@ -1,24 +1,23 @@
>  // SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause
>  /*
>   * Copyright (C) 2018, STMicroelectronics - All Rights Reserved
> + */
> +
> +/*
> + * File generated by STMicroelectronics STM32CubeMX DDR Tool for MPUs
> + * DDR type: DDR3 / DDR3L
> + * DDR width: 16bits
> + * DDR density: 4Gb
> + * System frequency: 533000Khz
> + * Relaxed Timing Mode: false
> + * Address mapping type: RBC
>   *
> - * STM32MP157C DK1/DK2 BOARD configuration
> - * 1x DDR3L 4Gb, 16-bit, 533MHz.
> - * Reference used NT5CC256M16DP-DI from NANYA
> - *
> - * DDR type / Platform	DDR3/3L
> - * freq		533MHz
> - * width	16
> - * datasheet	0  = MT41J256M16-187 / DDR3-1066 bin G
> - * DDR density	4
> - * timing mode	optimized
> - * Scheduling/QoS options : type = 2
> - * address mapping : RBC
> - * Tc > + 85C : N
> + * Save Date: 2020.02.20, save Time: 18:45:20
>   */
> -#define DDR_MEM_NAME "DDR3-1066/888 bin G 1x4Gb 533MHz v1.45"
> -#define DDR_MEM_SPEED 533000
> -#define DDR_MEM_SIZE 0x20000000
> +
> +#define DDR_MEM_NAME	"DDR3-DDR3L 16bits 533000Khz"
> +#define DDR_MEM_SPEED	533000
> +#define DDR_MEM_SIZE	0x20000000
>  
>  #define DDR_MSTR 0x00041401
>  #define DDR_MRCTRL0 0x00000010
> @@ -50,15 +49,6 @@
>  #define DDR_DFIUPD1 0x00000000
>  #define DDR_DFIUPD2 0x00000000
>  #define DDR_DFIPHYMSTR 0x00000000
> -#define DDR_ADDRMAP1 0x00070707
> -#define DDR_ADDRMAP2 0x00000000
> -#define DDR_ADDRMAP3 0x1F000000
> -#define DDR_ADDRMAP4 0x00001F1F
> -#define DDR_ADDRMAP5 0x06060606
> -#define DDR_ADDRMAP6 0x0F060606
> -#define DDR_ADDRMAP9 0x00000000
> -#define DDR_ADDRMAP10 0x00000000
> -#define DDR_ADDRMAP11 0x00000000
>  #define DDR_ODTCFG 0x06000600
>  #define DDR_ODTMAP 0x00000001
>  #define DDR_SCHED 0x00000C01
> @@ -83,6 +73,15 @@
>  #define DDR_PCFGQOS1_1 0x00800040
>  #define DDR_PCFGWQOS0_1 0x01100C03
>  #define DDR_PCFGWQOS1_1 0x01000200
> +#define DDR_ADDRMAP1 0x00070707
> +#define DDR_ADDRMAP2 0x00000000
> +#define DDR_ADDRMAP3 0x1F000000
> +#define DDR_ADDRMAP4 0x00001F1F
> +#define DDR_ADDRMAP5 0x06060606
> +#define DDR_ADDRMAP6 0x0F060606
> +#define DDR_ADDRMAP9 0x00000000
> +#define DDR_ADDRMAP10 0x00000000
> +#define DDR_ADDRMAP11 0x00000000
>  #define DDR_PGCR 0x01442E02
>  #define DDR_PTR0 0x0022AA5B
>  #define DDR_PTR1 0x04841104
> diff --git a/arch/arm/dts/stm32mp15-ddr3-2x4Gb-1066-binG.dtsi b/arch/arm/dts/stm32mp15-ddr3-2x4Gb-1066-binG.dtsi
> index 4b70b60554..fc226d2544 100644
> --- a/arch/arm/dts/stm32mp15-ddr3-2x4Gb-1066-binG.dtsi
> +++ b/arch/arm/dts/stm32mp15-ddr3-2x4Gb-1066-binG.dtsi
> @@ -1,24 +1,23 @@
>  // SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause
>  /*
>   * Copyright (C) 2018, STMicroelectronics - All Rights Reserved
> + */
> +
> +/*
> + * File generated by STMicroelectronics STM32CubeMX DDR Tool for MPUs
> + * DDR type: DDR3 / DDR3L
> + * DDR width: 32bits
> + * DDR density: 8Gb
> + * System frequency: 533000Khz
> + * Relaxed Timing Mode: false
> + * Address mapping type: RBC
>   *
> - * STM32MP157C ED1 BOARD configuration
> - * 2x DDR3L 4Gb each, 16-bit, 533MHz, Single Die Package in flyby topology.
> - * Reference used NT5CC256M16DP-DI from NANYA
> - *
> - * DDR type / Platform	DDR3/3L
> - * freq		533MHz
> - * width	32
> - * datasheet	0  = MT41J256M16-187 / DDR3-1066 bin G
> - * DDR density	8
> - * timing mode	optimized
> - * Scheduling/QoS options : type = 2
> - * address mapping : RBC
> - * Tc > + 85C : N
> + * Save Date: 2020.02.20, save Time: 18:49:33
>   */
> -#define DDR_MEM_NAME "DDR3-1066/888 bin G 2x4Gb 533MHz v1.45"
> -#define DDR_MEM_SPEED 533000
> -#define DDR_MEM_SIZE 0x40000000
> +
> +#define DDR_MEM_NAME	"DDR3-DDR3L 32bits 533000Khz"
> +#define DDR_MEM_SPEED	533000
> +#define DDR_MEM_SIZE	0x40000000
>  
>  #define DDR_MSTR 0x00040401
>  #define DDR_MRCTRL0 0x00000010
> @@ -50,15 +49,6 @@
>  #define DDR_DFIUPD1 0x00000000
>  #define DDR_DFIUPD2 0x00000000
>  #define DDR_DFIPHYMSTR 0x00000000
> -#define DDR_ADDRMAP1 0x00080808
> -#define DDR_ADDRMAP2 0x00000000
> -#define DDR_ADDRMAP3 0x00000000
> -#define DDR_ADDRMAP4 0x00001F1F
> -#define DDR_ADDRMAP5 0x07070707
> -#define DDR_ADDRMAP6 0x0F070707
> -#define DDR_ADDRMAP9 0x00000000
> -#define DDR_ADDRMAP10 0x00000000
> -#define DDR_ADDRMAP11 0x00000000
>  #define DDR_ODTCFG 0x06000600
>  #define DDR_ODTMAP 0x00000001
>  #define DDR_SCHED 0x00000C01
> @@ -83,6 +73,15 @@
>  #define DDR_PCFGQOS1_1 0x00800040
>  #define DDR_PCFGWQOS0_1 0x01100C03
>  #define DDR_PCFGWQOS1_1 0x01000200
> +#define DDR_ADDRMAP1 0x00080808
> +#define DDR_ADDRMAP2 0x00000000
> +#define DDR_ADDRMAP3 0x00000000
> +#define DDR_ADDRMAP4 0x00001F1F
> +#define DDR_ADDRMAP5 0x07070707
> +#define DDR_ADDRMAP6 0x0F070707
> +#define DDR_ADDRMAP9 0x00000000
> +#define DDR_ADDRMAP10 0x00000000
> +#define DDR_ADDRMAP11 0x00000000
>  #define DDR_PGCR 0x01442E02
>  #define DDR_PTR0 0x0022AA5B
>  #define DDR_PTR1 0x04841104

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

Thanks

[PATCH 15/16] configs: stm32mp1: activate Ethernet PHY Realtek

[Patrick Delaunay]

From: Christophe Roullier <christophe.roullier@st.com>

Need Realtek driver to manage in RTL8211F the configuration of the LED.
Initialize LCR (LED Control Register) to configure green LED for Link,
yellow LED for Active

Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

 configs/stm32mp15_basic_defconfig   | 1 +
 configs/stm32mp15_trusted_defconfig | 1 +
 2 files changed, 2 insertions(+)

diff --git a/configs/stm32mp15_basic_defconfig b/configs/stm32mp15_basic_defconfig
index 6d82365348..30cfb8d8e1 100644
--- a/configs/stm32mp15_basic_defconfig
+++ b/configs/stm32mp15_basic_defconfig
@@ -101,6 +101,7 @@ CONFIG_SPI_FLASH_WINBOND=y
 # CONFIG_SPI_FLASH_USE_4K_SECTORS is not set
 CONFIG_SPI_FLASH_MTD=y
 CONFIG_SPL_SPI_FLASH_MTD=y
+CONFIG_PHY_REALTEK=y
 CONFIG_DM_ETH=y
 CONFIG_DWC_ETH_QOS=y
 CONFIG_PHY=y
diff --git a/configs/stm32mp15_trusted_defconfig b/configs/stm32mp15_trusted_defconfig
index 6928e9a65c..aefad2b109 100644
--- a/configs/stm32mp15_trusted_defconfig
+++ b/configs/stm32mp15_trusted_defconfig
@@ -86,6 +86,7 @@ CONFIG_SPI_FLASH_STMICRO=y
 CONFIG_SPI_FLASH_WINBOND=y
 # CONFIG_SPI_FLASH_USE_4K_SECTORS is not set
 CONFIG_SPI_FLASH_MTD=y
+CONFIG_PHY_REALTEK=y
 CONFIG_DM_ETH=y
 CONFIG_DWC_ETH_QOS=y
 CONFIG_PHY=y
-- 
2.17.1

[Uboot-stm32] [PATCH 15/16] configs: stm32mp1: activate Ethernet PHY Realtek

[Patrice CHOTARD]

HI Patrick

On 3/31/20 6:04 PM, Patrick Delaunay wrote:
> From: Christophe Roullier <christophe.roullier@st.com>
>
> Need Realtek driver to manage in RTL8211F the configuration of the LED.
> Initialize LCR (LED Control Register) to configure green LED for Link,
> yellow LED for Active
>
> Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  configs/stm32mp15_basic_defconfig   | 1 +
>  configs/stm32mp15_trusted_defconfig | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/configs/stm32mp15_basic_defconfig b/configs/stm32mp15_basic_defconfig
> index 6d82365348..30cfb8d8e1 100644
> --- a/configs/stm32mp15_basic_defconfig
> +++ b/configs/stm32mp15_basic_defconfig
> @@ -101,6 +101,7 @@ CONFIG_SPI_FLASH_WINBOND=y
>  # CONFIG_SPI_FLASH_USE_4K_SECTORS is not set
>  CONFIG_SPI_FLASH_MTD=y
>  CONFIG_SPL_SPI_FLASH_MTD=y
> +CONFIG_PHY_REALTEK=y
>  CONFIG_DM_ETH=y
>  CONFIG_DWC_ETH_QOS=y
>  CONFIG_PHY=y
> diff --git a/configs/stm32mp15_trusted_defconfig b/configs/stm32mp15_trusted_defconfig
> index 6928e9a65c..aefad2b109 100644
> --- a/configs/stm32mp15_trusted_defconfig
> +++ b/configs/stm32mp15_trusted_defconfig
> @@ -86,6 +86,7 @@ CONFIG_SPI_FLASH_STMICRO=y
>  CONFIG_SPI_FLASH_WINBOND=y
>  # CONFIG_SPI_FLASH_USE_4K_SECTORS is not set
>  CONFIG_SPI_FLASH_MTD=y
> +CONFIG_PHY_REALTEK=y
>  CONFIG_DM_ETH=y
>  CONFIG_DWC_ETH_QOS=y
>  CONFIG_PHY=y

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

Thanks

[PATCH 16/16] configs: stm32mp1: activate CONFIG_ERRNO_STR

[Patrick Delaunay]

Add support of errno_str, used in command pmic and regulator.

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

 configs/stm32mp15_basic_defconfig   | 1 +
 configs/stm32mp15_trusted_defconfig | 1 +
 2 files changed, 2 insertions(+)

diff --git a/configs/stm32mp15_basic_defconfig b/configs/stm32mp15_basic_defconfig
index 30cfb8d8e1..8a6201bc48 100644
--- a/configs/stm32mp15_basic_defconfig
+++ b/configs/stm32mp15_basic_defconfig
@@ -144,4 +144,5 @@ CONFIG_VIDEO_STM32=y
 CONFIG_VIDEO_STM32_DSI=y
 CONFIG_VIDEO_STM32_MAX_XRES=1280
 CONFIG_VIDEO_STM32_MAX_YRES=800
+CONFIG_ERRNO_STR=y
 CONFIG_FDT_FIXUP_PARTITIONS=y
diff --git a/configs/stm32mp15_trusted_defconfig b/configs/stm32mp15_trusted_defconfig
index aefad2b109..394df39d0a 100644
--- a/configs/stm32mp15_trusted_defconfig
+++ b/configs/stm32mp15_trusted_defconfig
@@ -127,4 +127,5 @@ CONFIG_VIDEO_STM32=y
 CONFIG_VIDEO_STM32_DSI=y
 CONFIG_VIDEO_STM32_MAX_XRES=1280
 CONFIG_VIDEO_STM32_MAX_YRES=800
+CONFIG_ERRNO_STR=y
 CONFIG_FDT_FIXUP_PARTITIONS=y
-- 
2.17.1

[Uboot-stm32] [PATCH 16/16] configs: stm32mp1: activate CONFIG_ERRNO_STR

[Patrice CHOTARD]

Hi Patrick

On 3/31/20 6:04 PM, Patrick Delaunay wrote:
> Add support of errno_str, used in command pmic and regulator.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  configs/stm32mp15_basic_defconfig   | 1 +
>  configs/stm32mp15_trusted_defconfig | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/configs/stm32mp15_basic_defconfig b/configs/stm32mp15_basic_defconfig
> index 30cfb8d8e1..8a6201bc48 100644
> --- a/configs/stm32mp15_basic_defconfig
> +++ b/configs/stm32mp15_basic_defconfig
> @@ -144,4 +144,5 @@ CONFIG_VIDEO_STM32=y
>  CONFIG_VIDEO_STM32_DSI=y
>  CONFIG_VIDEO_STM32_MAX_XRES=1280
>  CONFIG_VIDEO_STM32_MAX_YRES=800
> +CONFIG_ERRNO_STR=y
>  CONFIG_FDT_FIXUP_PARTITIONS=y
> diff --git a/configs/stm32mp15_trusted_defconfig b/configs/stm32mp15_trusted_defconfig
> index aefad2b109..394df39d0a 100644
> --- a/configs/stm32mp15_trusted_defconfig
> +++ b/configs/stm32mp15_trusted_defconfig
> @@ -127,4 +127,5 @@ CONFIG_VIDEO_STM32=y
>  CONFIG_VIDEO_STM32_DSI=y
>  CONFIG_VIDEO_STM32_MAX_XRES=1280
>  CONFIG_VIDEO_STM32_MAX_YRES=800
> +CONFIG_ERRNO_STR=y
>  CONFIG_FDT_FIXUP_PARTITIONS=y

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

Thanks

[PATCH 01/16] arm: stm32mp: update dependency for STM32_ETZPC

[Patrice CHOTARD]

Hi Patrick

On 3/31/20 6:04 PM, Patrick Delaunay wrote:
> Correct the dependency for STM32 ETZPC protection, linked to SOC
> STM32MP identified by CONFIG_STM32MP15x and not linked to
> CONFIG_TARGET_STM32MP1 (no more existing).
>
> This patch fix an issue introduced by commit 846254888e2e ("stm32mp1:
> split board and SOC support for STM32MP15x family").
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> Acked-by: Patrice Chotard <patrice.chotard@st.com>
> ---
>
>  arch/arm/mach-stm32mp/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-stm32mp/Kconfig b/arch/arm/mach-stm32mp/Kconfig
> index e4d621dee8..96153693a7 100644
> --- a/arch/arm/mach-stm32mp/Kconfig
> +++ b/arch/arm/mach-stm32mp/Kconfig
> @@ -119,7 +119,7 @@ config SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION_MMC2
>  
>  config STM32_ETZPC
>  	bool "STM32 Extended TrustZone Protection"
> -	depends on TARGET_STM32MP1
> +	depends on STM32MP15x
>  	default y
>  	help
>  	  Say y to enable STM32 Extended TrustZone Protection

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

Thanks