[v1,36/43] x86: apl: Adjust FSP-M code to avoid hard-coded address

Message ID 20200615035738.248710-17-sjg@chromium.org
State New
Headers show
Series
  • x86: Programmatic generation of ACPI tables (Part C)
Related show

Commit Message

Simon Glass June 15, 2020, 3:57 a.m.
Update this code to calculate the address to use, rather than hard-coding
it. Obtain the requested stack size from the FSP.

Signed-off-by: Simon Glass <sjg at chromium.org>
---

 arch/x86/cpu/apollolake/fsp_m.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Wolfgang Wallner June 25, 2020, 10:55 a.m. | #1
Hi Simon,

-----"Simon Glass" <sjg at chromium.org> schrieb: -----
> Betreff: [PATCH v1 36/43] x86: apl: Adjust FSP-M code to avoid hard-coded address
> 
> Update this code to calculate the address to use, rather than hard-coding
> it. Obtain the requested stack size from the FSP.
> 
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
> 
>  arch/x86/cpu/apollolake/fsp_m.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/cpu/apollolake/fsp_m.c b/arch/x86/cpu/apollolake/fsp_m.c
> index 65461d85b8..cef937573b 100644
> --- a/arch/x86/cpu/apollolake/fsp_m.c
> +++ b/arch/x86/cpu/apollolake/fsp_m.c
> @@ -24,9 +24,11 @@ int fspm_update_config(struct udevice *dev, struct fspm_upd *upd)
>  	cache_ret = prepare_mrc_cache(upd);
>  	if (cache_ret && cache_ret != -ENOENT)
>  		return log_msg_ret("mrc", cache_ret);
> -	arch->stack_base = (void *)0xfef96000;
> +	arch->stack_base = (void *)(CONFIG_SYS_CAR_ADDR + CONFIG_SYS_CAR_SIZE -
> +		 arch->stack_size);
>  	arch->boot_loader_tolum_size = 0;
> -	arch->boot_mode = FSP_BOOT_WITH_FULL_CONFIGURATION;
> +	arch->boot_mode = cache_ret ? FSP_BOOT_WITH_FULL_CONFIGURATION :
> +		FSP_BOOT_ASSUMING_NO_CONFIGURATION_CHANGES;

This change is not related to the change calculation of stack_base, and so I
think it should be a separate commit.

I see that arch-boot_mode is handled similarly in Coreboot, so I assume this
is correct, but I could not find documentation about what 
FSP_BOOT_ASSUMING_NO_CONFIGURATION_CHANGES is doing differently.
Could you point me to the relevant documentation?

>  
>  	node = dev_ofnode(dev);
>  	if (!ofnode_valid(node))
> -- 
> 2.27.0.290.gba653c62da-goog

I think both changes are correct, just that they should be separate patches.
So if you split them up both of them are:

Reviewed-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>

regards, Wolfgang

Patch

diff --git a/arch/x86/cpu/apollolake/fsp_m.c b/arch/x86/cpu/apollolake/fsp_m.c
index 65461d85b8..cef937573b 100644
--- a/arch/x86/cpu/apollolake/fsp_m.c
+++ b/arch/x86/cpu/apollolake/fsp_m.c
@@ -24,9 +24,11 @@  int fspm_update_config(struct udevice *dev, struct fspm_upd *upd)
 	cache_ret = prepare_mrc_cache(upd);
 	if (cache_ret && cache_ret != -ENOENT)
 		return log_msg_ret("mrc", cache_ret);
-	arch->stack_base = (void *)0xfef96000;
+	arch->stack_base = (void *)(CONFIG_SYS_CAR_ADDR + CONFIG_SYS_CAR_SIZE -
+		 arch->stack_size);
 	arch->boot_loader_tolum_size = 0;
-	arch->boot_mode = FSP_BOOT_WITH_FULL_CONFIGURATION;
+	arch->boot_mode = cache_ret ? FSP_BOOT_WITH_FULL_CONFIGURATION :
+		FSP_BOOT_ASSUMING_NO_CONFIGURATION_CHANGES;
 
 	node = dev_ofnode(dev);
 	if (!ofnode_valid(node))