[U-Boot] [PATCH] arm: socfpga: control reboot from SRAM via env callback
This moves the code that enables the Boot ROM to just jump to SRAM instead of loading SPL from the original boot source on warm reboot.
Instead of always enabling this, an environment callback for the env var "socfpga_reboot_from_sram" is used. This way, the behaviour can be enabled at runtime and via saved environment.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com ---
arch/arm/mach-socfpga/misc_gen5.c | 49 +++++++++++++++++++++++++------ include/configs/socfpga_common.h | 5 ++++ 2 files changed, 45 insertions(+), 9 deletions(-)
diff --git a/arch/arm/mach-socfpga/misc_gen5.c b/arch/arm/mach-socfpga/misc_gen5.c index 9865f5b5b1..db662bb22a 100644 --- a/arch/arm/mach-socfpga/misc_gen5.c +++ b/arch/arm/mach-socfpga/misc_gen5.c @@ -6,6 +6,7 @@ #include <common.h> #include <asm/io.h> #include <errno.h> +#include <environment.h> #include <fdtdec.h> #include <linux/libfdt.h> #include <altera.h> @@ -182,15 +183,6 @@ int arch_early_init_r(void) { int i;
- /* - * Write magic value into magic register to unlock support for - * issuing warm reset. The ancient kernel code expects this - * value to be written into the register by the bootloader, so - * to support that old code, we write it here instead of in the - * reset_cpu() function just before resetting the CPU. - */ - writel(0xae9efebc, &sysmgr_regs->romcodegrp_warmramgrp_enable); - for (i = 0; i < 8; i++) /* Cache initial SW setting regs */ iswgrp_handoff[i] = readl(&sysmgr_regs->iswgrp_handoff[i]);
@@ -255,4 +247,43 @@ void do_bridge_reset(int enable) writel(1, &nic301_regs->remap); } } + +/* + * This function controls the reboot behaviour via an environment callback to + * the variable "socfpga_reboot_from_sram". + * + * Setting this variable to 1 writes a magic value into a magic register that + * makes the Boot ROM jump to SRAM on a warm reset. Note that this doesn't + * happen on cold reset, and that the SPL is not CRC protected, so if it gets + * overwritten before boot, something bad will happen. + * + * Given these limitations, this env callback only exists for backwards + * compatibility. + */ +static int on_socfpga_reboot_from_sram(const char *name, const char *value, + enum env_op op, int flags) +{ + int val; + u32 reg; + + val = simple_strtoul(value, NULL, 10); + if (val) { + /* + * Write magic value into magic register to unlock support for + * issuing warm reset. The ancient kernel code expects this + * value to be written into the register by the bootloader, so + * to support that old code, we write it here instead of in the + * reset_cpu() function just before resetting the CPU. + */ + reg = 0xae9efebc; + } else { + reg = 0; + } + writel(reg, &sysmgr_regs->romcodegrp_warmramgrp_enable); + + return 0; +} + +U_BOOT_ENV_CALLBACK(socfpga_reboot_from_sram, on_socfpga_reboot_from_sram); + #endif diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h index d1034ac280..aae4daf5d2 100644 --- a/include/configs/socfpga_common.h +++ b/include/configs/socfpga_common.h @@ -273,4 +273,9 @@ unsigned int cm_get_qspi_controller_clk_hz(void); #endif #endif
+#ifndef CONFIG_ENV_CALLBACK_LIST_STATIC +#define CONFIG_ENV_CALLBACK_LIST_STATIC \ + "socfpga_reboot_from_sram:socfpga_reboot_from_sram" +#endif + #endif /* __CONFIG_SOCFPGA_COMMON_H__ */
On 5/3/19 10:08 PM, Simon Goldschmidt wrote:
This moves the code that enables the Boot ROM to just jump to SRAM instead of loading SPL from the original boot source on warm reboot.
Instead of always enabling this, an environment callback for the env var "socfpga_reboot_from_sram" is used. This way, the behaviour can be enabled at runtime and via saved environment.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
Would that be like a default "reset" command action ? This probably shouldn't be socfpga specific then.
arch/arm/mach-socfpga/misc_gen5.c | 49 +++++++++++++++++++++++++------ include/configs/socfpga_common.h | 5 ++++ 2 files changed, 45 insertions(+), 9 deletions(-)
diff --git a/arch/arm/mach-socfpga/misc_gen5.c b/arch/arm/mach-socfpga/misc_gen5.c index 9865f5b5b1..db662bb22a 100644 --- a/arch/arm/mach-socfpga/misc_gen5.c +++ b/arch/arm/mach-socfpga/misc_gen5.c @@ -6,6 +6,7 @@ #include <common.h> #include <asm/io.h> #include <errno.h> +#include <environment.h> #include <fdtdec.h> #include <linux/libfdt.h> #include <altera.h> @@ -182,15 +183,6 @@ int arch_early_init_r(void) { int i;
- /*
* Write magic value into magic register to unlock support for* issuing warm reset. The ancient kernel code expects this* value to be written into the register by the bootloader, so* to support that old code, we write it here instead of in the* reset_cpu() function just before resetting the CPU.*/- writel(0xae9efebc, &sysmgr_regs->romcodegrp_warmramgrp_enable);
- for (i = 0; i < 8; i++) /* Cache initial SW setting regs */ iswgrp_handoff[i] = readl(&sysmgr_regs->iswgrp_handoff[i]);
@@ -255,4 +247,43 @@ void do_bridge_reset(int enable) writel(1, &nic301_regs->remap); } }
+/*
- This function controls the reboot behaviour via an environment callback to
- the variable "socfpga_reboot_from_sram".
- Setting this variable to 1 writes a magic value into a magic register that
- makes the Boot ROM jump to SRAM on a warm reset. Note that this doesn't
- happen on cold reset, and that the SPL is not CRC protected, so if it gets
- overwritten before boot, something bad will happen.
- Given these limitations, this env callback only exists for backwards
- compatibility.
- */
+static int on_socfpga_reboot_from_sram(const char *name, const char *value,
enum env_op op, int flags)+{
- int val;
- u32 reg;
- val = simple_strtoul(value, NULL, 10);
- if (val) {
/** Write magic value into magic register to unlock support for* issuing warm reset. The ancient kernel code expects this* value to be written into the register by the bootloader, so* to support that old code, we write it here instead of in the* reset_cpu() function just before resetting the CPU.*/reg = 0xae9efebc;- } else {
reg = 0;- }
- writel(reg, &sysmgr_regs->romcodegrp_warmramgrp_enable);
- return 0;
+}
+U_BOOT_ENV_CALLBACK(socfpga_reboot_from_sram, on_socfpga_reboot_from_sram);
#endif diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h index d1034ac280..aae4daf5d2 100644 --- a/include/configs/socfpga_common.h +++ b/include/configs/socfpga_common.h @@ -273,4 +273,9 @@ unsigned int cm_get_qspi_controller_clk_hz(void); #endif #endif
+#ifndef CONFIG_ENV_CALLBACK_LIST_STATIC +#define CONFIG_ENV_CALLBACK_LIST_STATIC \
- "socfpga_reboot_from_sram:socfpga_reboot_from_sram"
+#endif
#endif /* __CONFIG_SOCFPGA_COMMON_H__ */
On 03.05.19 22:28, Marek Vasut wrote:
On 5/3/19 10:08 PM, Simon Goldschmidt wrote:
This moves the code that enables the Boot ROM to just jump to SRAM instead of loading SPL from the original boot source on warm reboot.
Instead of always enabling this, an environment callback for the env var "socfpga_reboot_from_sram" is used. This way, the behaviour can be enabled at runtime and via saved environment.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
Would that be like a default "reset" command action ? This probably shouldn't be socfpga specific then.
No, it's a thing that lives on and influences even the soft reset issued by linux "reboot" command. This is something *very* socfpga specific.
Regards, Simon
arch/arm/mach-socfpga/misc_gen5.c | 49 +++++++++++++++++++++++++------ include/configs/socfpga_common.h | 5 ++++ 2 files changed, 45 insertions(+), 9 deletions(-)
diff --git a/arch/arm/mach-socfpga/misc_gen5.c b/arch/arm/mach-socfpga/misc_gen5.c index 9865f5b5b1..db662bb22a 100644 --- a/arch/arm/mach-socfpga/misc_gen5.c +++ b/arch/arm/mach-socfpga/misc_gen5.c @@ -6,6 +6,7 @@ #include <common.h> #include <asm/io.h> #include <errno.h> +#include <environment.h> #include <fdtdec.h> #include <linux/libfdt.h> #include <altera.h> @@ -182,15 +183,6 @@ int arch_early_init_r(void) { int i;
- /*
* Write magic value into magic register to unlock support for* issuing warm reset. The ancient kernel code expects this* value to be written into the register by the bootloader, so* to support that old code, we write it here instead of in the* reset_cpu() function just before resetting the CPU.*/- writel(0xae9efebc, &sysmgr_regs->romcodegrp_warmramgrp_enable);
- for (i = 0; i < 8; i++) /* Cache initial SW setting regs */ iswgrp_handoff[i] = readl(&sysmgr_regs->iswgrp_handoff[i]);
@@ -255,4 +247,43 @@ void do_bridge_reset(int enable) writel(1, &nic301_regs->remap); } }
+/*
- This function controls the reboot behaviour via an environment callback to
- the variable "socfpga_reboot_from_sram".
- Setting this variable to 1 writes a magic value into a magic register that
- makes the Boot ROM jump to SRAM on a warm reset. Note that this doesn't
- happen on cold reset, and that the SPL is not CRC protected, so if it gets
- overwritten before boot, something bad will happen.
- Given these limitations, this env callback only exists for backwards
- compatibility.
- */
+static int on_socfpga_reboot_from_sram(const char *name, const char *value,
enum env_op op, int flags)+{
- int val;
- u32 reg;
- val = simple_strtoul(value, NULL, 10);
- if (val) {
/** Write magic value into magic register to unlock support for* issuing warm reset. The ancient kernel code expects this* value to be written into the register by the bootloader, so* to support that old code, we write it here instead of in the* reset_cpu() function just before resetting the CPU.*/reg = 0xae9efebc;- } else {
reg = 0;- }
- writel(reg, &sysmgr_regs->romcodegrp_warmramgrp_enable);
- return 0;
+}
+U_BOOT_ENV_CALLBACK(socfpga_reboot_from_sram, on_socfpga_reboot_from_sram);
- #endif
diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h index d1034ac280..aae4daf5d2 100644 --- a/include/configs/socfpga_common.h +++ b/include/configs/socfpga_common.h @@ -273,4 +273,9 @@ unsigned int cm_get_qspi_controller_clk_hz(void); #endif #endif
+#ifndef CONFIG_ENV_CALLBACK_LIST_STATIC +#define CONFIG_ENV_CALLBACK_LIST_STATIC \
- "socfpga_reboot_from_sram:socfpga_reboot_from_sram"
+#endif
- #endif /* __CONFIG_SOCFPGA_COMMON_H__ */
On 5/3/19 10:30 PM, Simon Goldschmidt wrote:
On 03.05.19 22:28, Marek Vasut wrote:
On 5/3/19 10:08 PM, Simon Goldschmidt wrote:
This moves the code that enables the Boot ROM to just jump to SRAM instead of loading SPL from the original boot source on warm reboot.
Instead of always enabling this, an environment callback for the env var "socfpga_reboot_from_sram" is used. This way, the behaviour can be enabled at runtime and via saved environment.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
Would that be like a default "reset" command action ? This probably shouldn't be socfpga specific then.
No, it's a thing that lives on and influences even the soft reset issued by linux "reboot" command. This is something *very* socfpga specific.
Hmmm, so isn't this a policy to be configured on the Linux end ?
On 03.05.19 22:35, Marek Vasut wrote:
On 5/3/19 10:30 PM, Simon Goldschmidt wrote:
On 03.05.19 22:28, Marek Vasut wrote:
On 5/3/19 10:08 PM, Simon Goldschmidt wrote:
This moves the code that enables the Boot ROM to just jump to SRAM instead of loading SPL from the original boot source on warm reboot.
Instead of always enabling this, an environment callback for the env var "socfpga_reboot_from_sram" is used. This way, the behaviour can be enabled at runtime and via saved environment.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
Would that be like a default "reset" command action ? This probably shouldn't be socfpga specific then.
No, it's a thing that lives on and influences even the soft reset issued by linux "reboot" command. This is something *very* socfpga specific.
Hmmm, so isn't this a policy to be configured on the Linux end ?
Might be, but it affects U-Boot's 'reset' command as well. And I guess it's set up in U-Boot this early to ensure it always works.
If it were for me, we could drop writing this magic altogether. I just figured some boards might require it to be written somewhere, and came up with a patch that might save those boards with the way it was before.
Regards, Simon
On 5/3/19 10:39 PM, Simon Goldschmidt wrote:
On 03.05.19 22:35, Marek Vasut wrote:
On 5/3/19 10:30 PM, Simon Goldschmidt wrote:
On 03.05.19 22:28, Marek Vasut wrote:
On 5/3/19 10:08 PM, Simon Goldschmidt wrote:
This moves the code that enables the Boot ROM to just jump to SRAM instead of loading SPL from the original boot source on warm reboot.
Instead of always enabling this, an environment callback for the env var "socfpga_reboot_from_sram" is used. This way, the behaviour can be enabled at runtime and via saved environment.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
Would that be like a default "reset" command action ? This probably shouldn't be socfpga specific then.
No, it's a thing that lives on and influences even the soft reset issued by linux "reboot" command. This is something *very* socfpga specific.
Hmmm, so isn't this a policy to be configured on the Linux end ?
Might be, but it affects U-Boot's 'reset' command as well. And I guess it's set up in U-Boot this early to ensure it always works.
Drat, that's right. So there has to be some way to agree on how the reset works between the kernel and U-Boot ?
If it were for me, we could drop writing this magic altogether. I just figured some boards might require it to be written somewhere, and came up with a patch that might save those boards with the way it was before.
Isn't this magic actually used by bootrom ?
Marek Vasut marex@denx.de schrieb am Fr., 3. Mai 2019, 22:42:
On 5/3/19 10:39 PM, Simon Goldschmidt wrote:
On 03.05.19 22:35, Marek Vasut wrote:
On 5/3/19 10:30 PM, Simon Goldschmidt wrote:
On 03.05.19 22:28, Marek Vasut wrote:
On 5/3/19 10:08 PM, Simon Goldschmidt wrote:
This moves the code that enables the Boot ROM to just jump to SRAM instead of loading SPL from the original boot source on warm reboot.
Instead of always enabling this, an environment callback for the env var "socfpga_reboot_from_sram" is used. This way, the behaviour can be enabled at runtime and via saved environment.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
Would that be like a default "reset" command action ? This probably shouldn't be socfpga specific then.
No, it's a thing that lives on and influences even the soft reset
issued
by linux "reboot" command. This is something *very* socfpga specific.
Hmmm, so isn't this a policy to be configured on the Linux end ?
Might be, but it affects U-Boot's 'reset' command as well. And I guess it's set up in U-Boot this early to ensure it always works.
Drat, that's right. So there has to be some way to agree on how the reset works between the kernel and U-Boot ?
If it were for me, we could drop writing this magic altogether. I just figured some boards might require it to be written somewhere, and came up with a patch that might save those boards with the way it was before.
Isn't this magic actually used by bootrom ?
Right. It tells the boot rom to jump to ocram on next reboot instead of loading spl from qspi or mmc. But if that's required or not a good idea at all depends on many factors. Some of them board related, some U-Boot related and some Linux related (depending on the hardware and drivers used).
Regards, Simon
On 5/3/19 10:53 PM, Simon Goldschmidt wrote:
Marek Vasut <marex@denx.de mailto:marex@denx.de> schrieb am Fr., 3. Mai 2019, 22:42:
On 5/3/19 10:39 PM, Simon Goldschmidt wrote: > > > On 03.05.19 22:35, Marek Vasut wrote: >> On 5/3/19 10:30 PM, Simon Goldschmidt wrote: >>> >>> >>> On 03.05.19 22:28, Marek Vasut wrote: >>>> On 5/3/19 10:08 PM, Simon Goldschmidt wrote: >>>>> This moves the code that enables the Boot ROM to just jump to SRAM >>>>> instead >>>>> of loading SPL from the original boot source on warm reboot. >>>>> >>>>> Instead of always enabling this, an environment callback for the >>>>> env var >>>>> "socfpga_reboot_from_sram" is used. This way, the behaviour can be >>>>> enabled >>>>> at runtime and via saved environment. >>>>> >>>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>> >>>> >>>> Would that be like a default "reset" command action ? >>>> This probably shouldn't be socfpga specific then. >>> >>> No, it's a thing that lives on and influences even the soft reset issued >>> by linux "reboot" command. This is something *very* socfpga specific. >> >> Hmmm, so isn't this a policy to be configured on the Linux end ? > > Might be, but it affects U-Boot's 'reset' command as well. And I guess > it's set up in U-Boot this early to ensure it always works. Drat, that's right. So there has to be some way to agree on how the reset works between the kernel and U-Boot ? > If it were for me, we could drop writing this magic altogether. I just > figured some boards might require it to be written somewhere, and came > up with a patch that might save those boards with the way it was before. Isn't this magic actually used by bootrom ?Right. It tells the boot rom to jump to ocram on next reboot instead of loading spl from qspi or mmc. But if that's required or not a good idea at all depends on many factors. Some of them board related, some U-Boot related and some Linux related (depending on the hardware and drivers used).
Should that be runtime configurable then ?
Am 04.05.2019 um 20:43 schrieb Marek Vasut:
On 5/3/19 10:53 PM, Simon Goldschmidt wrote:
Marek Vasut <marex@denx.de mailto:marex@denx.de> schrieb am Fr., 3. Mai 2019, 22:42:
On 5/3/19 10:39 PM, Simon Goldschmidt wrote: > > > On 03.05.19 22:35, Marek Vasut wrote: >> On 5/3/19 10:30 PM, Simon Goldschmidt wrote: >>> >>> >>> On 03.05.19 22:28, Marek Vasut wrote: >>>> On 5/3/19 10:08 PM, Simon Goldschmidt wrote: >>>>> This moves the code that enables the Boot ROM to just jump to SRAM >>>>> instead >>>>> of loading SPL from the original boot source on warm reboot. >>>>> >>>>> Instead of always enabling this, an environment callback for the >>>>> env var >>>>> "socfpga_reboot_from_sram" is used. This way, the behaviour can be >>>>> enabled >>>>> at runtime and via saved environment. >>>>> >>>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>> >>>> >>>> Would that be like a default "reset" command action ? >>>> This probably shouldn't be socfpga specific then. >>> >>> No, it's a thing that lives on and influences even the soft reset issued >>> by linux "reboot" command. This is something *very* socfpga specific. >> >> Hmmm, so isn't this a policy to be configured on the Linux end ? > > Might be, but it affects U-Boot's 'reset' command as well. And I guess > it's set up in U-Boot this early to ensure it always works. Drat, that's right. So there has to be some way to agree on how the reset works between the kernel and U-Boot ? > If it were for me, we could drop writing this magic altogether. I just > figured some boards might require it to be written somewhere, and came > up with a patch that might save those boards with the way it was before. Isn't this magic actually used by bootrom ?Right. It tells the boot rom to jump to ocram on next reboot instead of loading spl from qspi or mmc. But if that's required or not a good idea at all depends on many factors. Some of them board related, some U-Boot related and some Linux related (depending on the hardware and drivers used).
Should that be runtime configurable then ?
Since it might depend on Linux putting the qspi chip into a state where it's not accessible by the boot ROM. That might change without rebuilding U-Boot.
On the other hand, this is probably more of a U-Boot build time config. I could make it a Kconfig option as well...
Regards, Simon
On 5/4/19 9:10 PM, Simon Goldschmidt wrote:
Am 04.05.2019 um 20:43 schrieb Marek Vasut:
On 5/3/19 10:53 PM, Simon Goldschmidt wrote:
Marek Vasut <marex@denx.de mailto:marex@denx.de> schrieb am Fr., 3. Mai 2019, 22:42:
On 5/3/19 10:39 PM, Simon Goldschmidt wrote: > > > On 03.05.19 22:35, Marek Vasut wrote: >> On 5/3/19 10:30 PM, Simon Goldschmidt wrote: >>> >>> >>> On 03.05.19 22:28, Marek Vasut wrote: >>>> On 5/3/19 10:08 PM, Simon Goldschmidt wrote: >>>>> This moves the code that enables the Boot ROM to just jump to SRAM >>>>> instead >>>>> of loading SPL from the original boot source on warm reboot. >>>>> >>>>> Instead of always enabling this, an environment callback for the >>>>> env var >>>>> "socfpga_reboot_from_sram" is used. This way, the behaviour can be >>>>> enabled >>>>> at runtime and via saved environment. >>>>> >>>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com mailto:simon.k.r.goldschmidt@gmail.com> >>>> >>>> Would that be like a default "reset" command action ? >>>> This probably shouldn't be socfpga specific then. >>> >>> No, it's a thing that lives on and influences even the soft reset issued >>> by linux "reboot" command. This is something *very* socfpga specific. >> >> Hmmm, so isn't this a policy to be configured on the Linux end ? > > Might be, but it affects U-Boot's 'reset' command as well. And I guess > it's set up in U-Boot this early to ensure it always works.
Drat, that's right. So there has to be some way to agree on how the reset works between the kernel and U-Boot ?
> If it were for me, we could drop writing this magic altogether. I just > figured some boards might require it to be written somewhere, and came > up with a patch that might save those boards with the way it was before.
Isn't this magic actually used by bootrom ?
Right. It tells the boot rom to jump to ocram on next reboot instead of loading spl from qspi or mmc. But if that's required or not a good idea at all depends on many factors. Some of them board related, some U-Boot related and some Linux related (depending on the hardware and drivers used).
Should that be runtime configurable then ?
Since it might depend on Linux putting the qspi chip into a state where it's not accessible by the boot ROM. That might change without rebuilding U-Boot.
If Linux switches the chip into some weird mode the bootrom cannot cope with, it's a reset routing problem. This cannot be fixed in software.
On the other hand, this is probably more of a U-Boot build time config. I could make it a Kconfig option as well...
Regards, Simon
On 05.05.19 03:42, Marek Vasut wrote:
On 5/4/19 9:10 PM, Simon Goldschmidt wrote:
Am 04.05.2019 um 20:43 schrieb Marek Vasut:
On 5/3/19 10:53 PM, Simon Goldschmidt wrote:
Marek Vasut <marex@denx.de mailto:marex@denx.de> schrieb am Fr., 3. Mai 2019, 22:42:
On 5/3/19 10:39 PM, Simon Goldschmidt wrote: > > > On 03.05.19 22:35, Marek Vasut wrote: >> On 5/3/19 10:30 PM, Simon Goldschmidt wrote: >>> >>> >>> On 03.05.19 22:28, Marek Vasut wrote: >>>> On 5/3/19 10:08 PM, Simon Goldschmidt wrote: >>>>> This moves the code that enables the Boot ROM to just jump to SRAM >>>>> instead >>>>> of loading SPL from the original boot source on warm reboot. >>>>> >>>>> Instead of always enabling this, an environment callback for the >>>>> env var >>>>> "socfpga_reboot_from_sram" is used. This way, the behaviour can be >>>>> enabled >>>>> at runtime and via saved environment. >>>>> >>>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com mailto:simon.k.r.goldschmidt@gmail.com> >>>> >>>> Would that be like a default "reset" command action ? >>>> This probably shouldn't be socfpga specific then. >>> >>> No, it's a thing that lives on and influences even the soft reset issued >>> by linux "reboot" command. This is something *very* socfpga specific. >> >> Hmmm, so isn't this a policy to be configured on the Linux end ? > > Might be, but it affects U-Boot's 'reset' command as well. And I guess > it's set up in U-Boot this early to ensure it always works.
Drat, that's right. So there has to be some way to agree on how the reset works between the kernel and U-Boot ?
> If it were for me, we could drop writing this magic altogether. I just > figured some boards might require it to be written somewhere, and came > up with a patch that might save those boards with the way it was before.
Isn't this magic actually used by bootrom ?
Right. It tells the boot rom to jump to ocram on next reboot instead of loading spl from qspi or mmc. But if that's required or not a good idea at all depends on many factors. Some of them board related, some U-Boot related and some Linux related (depending on the hardware and drivers used).
Should that be runtime configurable then ?
Since it might depend on Linux putting the qspi chip into a state where it's not accessible by the boot ROM. That might change without rebuilding U-Boot.
If Linux switches the chip into some weird mode the bootrom cannot cope with, it's a reset routing problem. This cannot be fixed in software.
No, it cannot be fixed, but currently there's a workaround for those boards and I thought it was worth to keep this workaround, even though my own boards will be fixed and not require such a workaround in the future :-)
On the other hand, this is probably more of a U-Boot build time config. I could make it a Kconfig option as well...
Regards, Simon
On 5/5/19 8:05 AM, Simon Goldschmidt wrote:
On 05.05.19 03:42, Marek Vasut wrote:
On 5/4/19 9:10 PM, Simon Goldschmidt wrote:
Am 04.05.2019 um 20:43 schrieb Marek Vasut:
On 5/3/19 10:53 PM, Simon Goldschmidt wrote:
Marek Vasut <marex@denx.de mailto:marex@denx.de> schrieb am Fr., 3. Mai 2019, 22:42:
On 5/3/19 10:39 PM, Simon Goldschmidt wrote: > > > On 03.05.19 22:35, Marek Vasut wrote: >> On 5/3/19 10:30 PM, Simon Goldschmidt wrote: >>> >>> >>> On 03.05.19 22:28, Marek Vasut wrote: >>>> On 5/3/19 10:08 PM, Simon Goldschmidt wrote: >>>>> This moves the code that enables the Boot ROM to just jump to SRAM >>>>> instead >>>>> of loading SPL from the original boot source on warm reboot. >>>>> >>>>> Instead of always enabling this, an environment callback for the >>>>> env var >>>>> "socfpga_reboot_from_sram" is used. This way, the behaviour can be >>>>> enabled >>>>> at runtime and via saved environment. >>>>> >>>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com mailto:simon.k.r.goldschmidt@gmail.com> >>>> >>>> Would that be like a default "reset" command action ? >>>> This probably shouldn't be socfpga specific then. >>> >>> No, it's a thing that lives on and influences even the soft reset issued >>> by linux "reboot" command. This is something *very* socfpga specific. >> >> Hmmm, so isn't this a policy to be configured on the Linux end ? > > Might be, but it affects U-Boot's 'reset' command as well. And I guess > it's set up in U-Boot this early to ensure it always works.
Drat, that's right. So there has to be some way to agree on how the reset works between the kernel and U-Boot ?
> If it were for me, we could drop writing this magic altogether. I just > figured some boards might require it to be written somewhere, and came > up with a patch that might save those boards with the way it was before.
Isn't this magic actually used by bootrom ?
Right. It tells the boot rom to jump to ocram on next reboot instead of loading spl from qspi or mmc. But if that's required or not a good idea at all depends on many factors. Some of them board related, some U-Boot related and some Linux related (depending on the hardware and drivers used).
Should that be runtime configurable then ?
Since it might depend on Linux putting the qspi chip into a state where it's not accessible by the boot ROM. That might change without rebuilding U-Boot.
If Linux switches the chip into some weird mode the bootrom cannot cope with, it's a reset routing problem. This cannot be fixed in software.
No, it cannot be fixed, but currently there's a workaround for those boards and I thought it was worth to keep this workaround, even though my own boards will be fixed and not require such a workaround in the future :-)
What's the workaround ?
Am 05.05.2019 um 22:17 schrieb Marek Vasut:
On 5/5/19 8:05 AM, Simon Goldschmidt wrote:
On 05.05.19 03:42, Marek Vasut wrote:
On 5/4/19 9:10 PM, Simon Goldschmidt wrote:
Am 04.05.2019 um 20:43 schrieb Marek Vasut:
On 5/3/19 10:53 PM, Simon Goldschmidt wrote:
Marek Vasut <marex@denx.de mailto:marex@denx.de> schrieb am Fr., 3. Mai 2019, 22:42:
On 5/3/19 10:39 PM, Simon Goldschmidt wrote: > > > On 03.05.19 22:35, Marek Vasut wrote: >> On 5/3/19 10:30 PM, Simon Goldschmidt wrote: >>> >>> >>> On 03.05.19 22:28, Marek Vasut wrote: >>>> On 5/3/19 10:08 PM, Simon Goldschmidt wrote: >>>>> This moves the code that enables the Boot ROM to just jump to SRAM >>>>> instead >>>>> of loading SPL from the original boot source on warm reboot. >>>>> >>>>> Instead of always enabling this, an environment callback for the >>>>> env var >>>>> "socfpga_reboot_from_sram" is used. This way, the behaviour can be >>>>> enabled >>>>> at runtime and via saved environment. >>>>> >>>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com mailto:simon.k.r.goldschmidt@gmail.com> >>>> >>>> Would that be like a default "reset" command action ? >>>> This probably shouldn't be socfpga specific then. >>> >>> No, it's a thing that lives on and influences even the soft reset issued >>> by linux "reboot" command. This is something *very* socfpga specific. >> >> Hmmm, so isn't this a policy to be configured on the Linux end ? > > Might be, but it affects U-Boot's 'reset' command as well. And I guess > it's set up in U-Boot this early to ensure it always works.
Drat, that's right. So there has to be some way to agree on how the reset works between the kernel and U-Boot ?
> If it were for me, we could drop writing this magic altogether. I just > figured some boards might require it to be written somewhere, and came > up with a patch that might save those boards with the way it was before.
Isn't this magic actually used by bootrom ?
Right. It tells the boot rom to jump to ocram on next reboot instead of loading spl from qspi or mmc. But if that's required or not a good idea at all depends on many factors. Some of them board related, some U-Boot related and some Linux related (depending on the hardware and drivers used).
Should that be runtime configurable then ?
Since it might depend on Linux putting the qspi chip into a state where it's not accessible by the boot ROM. That might change without rebuilding U-Boot.
If Linux switches the chip into some weird mode the bootrom cannot cope with, it's a reset routing problem. This cannot be fixed in software.
No, it cannot be fixed, but currently there's a workaround for those boards and I thought it was worth to keep this workaround, even though my own boards will be fixed and not require such a workaround in the future :-)
What's the workaround ?
The workaround is what this patch is about: the Boot ROM just branches off to SRAM where it expectes SPL to be still working.
SPL can then e.g. reset 4-byte mode or whatever to still communicate with the device when Boot ROM can't.
Of course the downside is that SRAM might be overwritten meanwhile, which is why it's a workaround only, not the best idea how to do things...
Regards, Simon
On 5/5/19 10:21 PM, Simon Goldschmidt wrote:
Am 05.05.2019 um 22:17 schrieb Marek Vasut:
On 5/5/19 8:05 AM, Simon Goldschmidt wrote:
On 05.05.19 03:42, Marek Vasut wrote:
On 5/4/19 9:10 PM, Simon Goldschmidt wrote:
Am 04.05.2019 um 20:43 schrieb Marek Vasut:
On 5/3/19 10:53 PM, Simon Goldschmidt wrote: > > > Marek Vasut <marex@denx.de mailto:marex@denx.de> schrieb am > Fr., 3. > Mai 2019, 22:42: > > On 5/3/19 10:39 PM, Simon Goldschmidt wrote: > > > > > > On 03.05.19 22:35, Marek Vasut wrote: > >> On 5/3/19 10:30 PM, Simon Goldschmidt wrote: > >>> > >>> > >>> On 03.05.19 22:28, Marek Vasut wrote: > >>>> On 5/3/19 10:08 PM, Simon Goldschmidt wrote: > >>>>> This moves the code that enables the Boot ROM to > just jump > to SRAM > >>>>> instead > >>>>> of loading SPL from the original boot source on warm > reboot. > >>>>> > >>>>> Instead of always enabling this, an environment > callback > for the > >>>>> env var > >>>>> "socfpga_reboot_from_sram" is used. This way, the > behaviour can be > >>>>> enabled > >>>>> at runtime and via saved environment. > >>>>> > >>>>> Signed-off-by: Simon Goldschmidt > <simon.k.r.goldschmidt@gmail.com > mailto:simon.k.r.goldschmidt@gmail.com> > >>>> > >>>> Would that be like a default "reset" command action ? > >>>> This probably shouldn't be socfpga specific then. > >>> > >>> No, it's a thing that lives on and influences even the > soft > reset issued > >>> by linux "reboot" command. This is something *very* > socfpga > specific. > >> > >> Hmmm, so isn't this a policy to be configured on the Linux > end ? > > > > Might be, but it affects U-Boot's 'reset' command as > well. And > I guess > > it's set up in U-Boot this early to ensure it always works. > > Drat, that's right. So there has to be some way to agree on > how the > reset works between the kernel and U-Boot ? > > > If it were for me, we could drop writing this magic > altogether. I just > > figured some boards might require it to be written > somewhere, > and came > > up with a patch that might save those boards with the way > it was > before. > > Isn't this magic actually used by bootrom ? > > > Right. It tells the boot rom to jump to ocram on next reboot > instead of > loading spl from qspi or mmc. But if that's required or not a good > idea > at all depends on many factors. Some of them board related, some > U-Boot > related and some Linux related (depending on the hardware and > drivers > used).
Should that be runtime configurable then ?
Since it might depend on Linux putting the qspi chip into a state where it's not accessible by the boot ROM. That might change without rebuilding U-Boot.
If Linux switches the chip into some weird mode the bootrom cannot cope with, it's a reset routing problem. This cannot be fixed in software.
No, it cannot be fixed, but currently there's a workaround for those boards and I thought it was worth to keep this workaround, even though my own boards will be fixed and not require such a workaround in the future :-)
What's the workaround ?
The workaround is what this patch is about: the Boot ROM just branches off to SRAM where it expectes SPL to be still working.
But you still cannot communicate with the SPI NOR from your SPL ?
SPL can then e.g. reset 4-byte mode or whatever to still communicate with the device when Boot ROM can't.
Unless, of course, the SPI NOR doesn't interpret the command as data and corrupts something in the flash itself.
Of course the downside is that SRAM might be overwritten meanwhile, which is why it's a workaround only, not the best idea how to do things...
Regards, Simon
Am 06.05.2019 um 00:51 schrieb Marek Vasut:
On 5/5/19 10:21 PM, Simon Goldschmidt wrote:
Am 05.05.2019 um 22:17 schrieb Marek Vasut:
On 5/5/19 8:05 AM, Simon Goldschmidt wrote:
On 05.05.19 03:42, Marek Vasut wrote:
On 5/4/19 9:10 PM, Simon Goldschmidt wrote:
Am 04.05.2019 um 20:43 schrieb Marek Vasut: > On 5/3/19 10:53 PM, Simon Goldschmidt wrote: >> >> >> Marek Vasut <marex@denx.de mailto:marex@denx.de> schrieb am >> Fr., 3. >> Mai 2019, 22:42: >> >> On 5/3/19 10:39 PM, Simon Goldschmidt wrote: >> > >> > >> > On 03.05.19 22:35, Marek Vasut wrote: >> >> On 5/3/19 10:30 PM, Simon Goldschmidt wrote: >> >>> >> >>> >> >>> On 03.05.19 22:28, Marek Vasut wrote: >> >>>> On 5/3/19 10:08 PM, Simon Goldschmidt wrote: >> >>>>> This moves the code that enables the Boot ROM to >> just jump >> to SRAM >> >>>>> instead >> >>>>> of loading SPL from the original boot source on warm >> reboot. >> >>>>> >> >>>>> Instead of always enabling this, an environment >> callback >> for the >> >>>>> env var >> >>>>> "socfpga_reboot_from_sram" is used. This way, the >> behaviour can be >> >>>>> enabled >> >>>>> at runtime and via saved environment. >> >>>>> >> >>>>> Signed-off-by: Simon Goldschmidt >> <simon.k.r.goldschmidt@gmail.com >> mailto:simon.k.r.goldschmidt@gmail.com> >> >>>> >> >>>> Would that be like a default "reset" command action ? >> >>>> This probably shouldn't be socfpga specific then. >> >>> >> >>> No, it's a thing that lives on and influences even the >> soft >> reset issued >> >>> by linux "reboot" command. This is something *very* >> socfpga >> specific. >> >> >> >> Hmmm, so isn't this a policy to be configured on the Linux >> end ? >> > >> > Might be, but it affects U-Boot's 'reset' command as >> well. And >> I guess >> > it's set up in U-Boot this early to ensure it always works. >> >> Drat, that's right. So there has to be some way to agree on >> how the >> reset works between the kernel and U-Boot ? >> >> > If it were for me, we could drop writing this magic >> altogether. I just >> > figured some boards might require it to be written >> somewhere, >> and came >> > up with a patch that might save those boards with the way >> it was >> before. >> >> Isn't this magic actually used by bootrom ? >> >> >> Right. It tells the boot rom to jump to ocram on next reboot >> instead of >> loading spl from qspi or mmc. But if that's required or not a good >> idea >> at all depends on many factors. Some of them board related, some >> U-Boot >> related and some Linux related (depending on the hardware and >> drivers >> used). > > Should that be runtime configurable then ?
Since it might depend on Linux putting the qspi chip into a state where it's not accessible by the boot ROM. That might change without rebuilding U-Boot.
If Linux switches the chip into some weird mode the bootrom cannot cope with, it's a reset routing problem. This cannot be fixed in software.
No, it cannot be fixed, but currently there's a workaround for those boards and I thought it was worth to keep this workaround, even though my own boards will be fixed and not require such a workaround in the future :-)
What's the workaround ?
The workaround is what this patch is about: the Boot ROM just branches off to SRAM where it expectes SPL to be still working.
But you still cannot communicate with the SPI NOR from your SPL ?
Well, in most "every day reboot" cases, you can. Just reset BAR or 4-byte mode.
SPL can then e.g. reset 4-byte mode or whatever to still communicate with the device when Boot ROM can't.
Unless, of course, the SPI NOR doesn't interpret the command as data and corrupts something in the flash itself.
Right, in this case, you can't.
Don't get me wrong, I'm not arguing for this to be totally right, of course I'd rahter get the boards fixed.
I'm just trying to find a way to keep this code in for people depending on it. I know we have some broken boards that depend on it. I could live with writing this magic in our private board code, but it's a bummer for other people upgrading if we removed it...
Regards, Simon
Of course the downside is that SRAM might be overwritten meanwhile, which is why it's a workaround only, not the best idea how to do things...
Regards, Simon
On 5/6/19 9:50 PM, Simon Goldschmidt wrote:
Am 06.05.2019 um 00:51 schrieb Marek Vasut:
On 5/5/19 10:21 PM, Simon Goldschmidt wrote:
Am 05.05.2019 um 22:17 schrieb Marek Vasut:
On 5/5/19 8:05 AM, Simon Goldschmidt wrote:
On 05.05.19 03:42, Marek Vasut wrote:
On 5/4/19 9:10 PM, Simon Goldschmidt wrote: > Am 04.05.2019 um 20:43 schrieb Marek Vasut: >> On 5/3/19 10:53 PM, Simon Goldschmidt wrote: >>> >>> >>> Marek Vasut <marex@denx.de mailto:marex@denx.de> schrieb am >>> Fr., 3. >>> Mai 2019, 22:42: >>> >>> On 5/3/19 10:39 PM, Simon Goldschmidt wrote: >>> > >>> > >>> > On 03.05.19 22:35, Marek Vasut wrote: >>> >> On 5/3/19 10:30 PM, Simon Goldschmidt wrote: >>> >>> >>> >>> >>> >>> On 03.05.19 22:28, Marek Vasut wrote: >>> >>>> On 5/3/19 10:08 PM, Simon Goldschmidt wrote: >>> >>>>> This moves the code that enables the Boot ROM to >>> just jump >>> to SRAM >>> >>>>> instead >>> >>>>> of loading SPL from the original boot source on warm >>> reboot. >>> >>>>> >>> >>>>> Instead of always enabling this, an environment >>> callback >>> for the >>> >>>>> env var >>> >>>>> "socfpga_reboot_from_sram" is used. This way, the >>> behaviour can be >>> >>>>> enabled >>> >>>>> at runtime and via saved environment. >>> >>>>> >>> >>>>> Signed-off-by: Simon Goldschmidt >>> <simon.k.r.goldschmidt@gmail.com >>> mailto:simon.k.r.goldschmidt@gmail.com> >>> >>>> >>> >>>> Would that be like a default "reset" command action ? >>> >>>> This probably shouldn't be socfpga specific then. >>> >>> >>> >>> No, it's a thing that lives on and influences even the >>> soft >>> reset issued >>> >>> by linux "reboot" command. This is something *very* >>> socfpga >>> specific. >>> >> >>> >> Hmmm, so isn't this a policy to be configured on the >>> Linux >>> end ? >>> > >>> > Might be, but it affects U-Boot's 'reset' command as >>> well. And >>> I guess >>> > it's set up in U-Boot this early to ensure it always >>> works. >>> >>> Drat, that's right. So there has to be some way to >>> agree on >>> how the >>> reset works between the kernel and U-Boot ? >>> >>> > If it were for me, we could drop writing this magic >>> altogether. I just >>> > figured some boards might require it to be written >>> somewhere, >>> and came >>> > up with a patch that might save those boards with the >>> way >>> it was >>> before. >>> >>> Isn't this magic actually used by bootrom ? >>> >>> >>> Right. It tells the boot rom to jump to ocram on next reboot >>> instead of >>> loading spl from qspi or mmc. But if that's required or not a good >>> idea >>> at all depends on many factors. Some of them board related, some >>> U-Boot >>> related and some Linux related (depending on the hardware and >>> drivers >>> used). >> >> Should that be runtime configurable then ? > > Since it might depend on Linux putting the qspi chip into a state > where > it's not accessible by the boot ROM. That might change without > rebuilding U-Boot.
If Linux switches the chip into some weird mode the bootrom cannot cope with, it's a reset routing problem. This cannot be fixed in software.
No, it cannot be fixed, but currently there's a workaround for those boards and I thought it was worth to keep this workaround, even though my own boards will be fixed and not require such a workaround in the future :-)
What's the workaround ?
The workaround is what this patch is about: the Boot ROM just branches off to SRAM where it expectes SPL to be still working.
But you still cannot communicate with the SPI NOR from your SPL ?
Well, in most "every day reboot" cases, you can. Just reset BAR or 4-byte mode.
"In most" reads as "it's unreliable".
SPL can then e.g. reset 4-byte mode or whatever to still communicate with the device when Boot ROM can't.
Unless, of course, the SPI NOR doesn't interpret the command as data and corrupts something in the flash itself.
Right, in this case, you can't.
Don't get me wrong, I'm not arguing for this to be totally right, of course I'd rahter get the boards fixed.
I'm just trying to find a way to keep this code in for people depending on it. I know we have some broken boards that depend on it. I could live with writing this magic in our private board code, but it's a bummer for other people upgrading if we removed it...
Put it in a board file with BIG FAT WARNING that it's wrong ?
Am 06.05.2019 um 23:12 schrieb Marek Vasut:
On 5/6/19 9:50 PM, Simon Goldschmidt wrote:
Am 06.05.2019 um 00:51 schrieb Marek Vasut:
On 5/5/19 10:21 PM, Simon Goldschmidt wrote:
Am 05.05.2019 um 22:17 schrieb Marek Vasut:
On 5/5/19 8:05 AM, Simon Goldschmidt wrote:
On 05.05.19 03:42, Marek Vasut wrote: > On 5/4/19 9:10 PM, Simon Goldschmidt wrote: >> Am 04.05.2019 um 20:43 schrieb Marek Vasut: >>> On 5/3/19 10:53 PM, Simon Goldschmidt wrote: >>>> >>>> >>>> Marek Vasut <marex@denx.de mailto:marex@denx.de> schrieb am >>>> Fr., 3. >>>> Mai 2019, 22:42: >>>> >>>> On 5/3/19 10:39 PM, Simon Goldschmidt wrote: >>>> > >>>> > >>>> > On 03.05.19 22:35, Marek Vasut wrote: >>>> >> On 5/3/19 10:30 PM, Simon Goldschmidt wrote: >>>> >>> >>>> >>> >>>> >>> On 03.05.19 22:28, Marek Vasut wrote: >>>> >>>> On 5/3/19 10:08 PM, Simon Goldschmidt wrote: >>>> >>>>> This moves the code that enables the Boot ROM to >>>> just jump >>>> to SRAM >>>> >>>>> instead >>>> >>>>> of loading SPL from the original boot source on warm >>>> reboot. >>>> >>>>> >>>> >>>>> Instead of always enabling this, an environment >>>> callback >>>> for the >>>> >>>>> env var >>>> >>>>> "socfpga_reboot_from_sram" is used. This way, the >>>> behaviour can be >>>> >>>>> enabled >>>> >>>>> at runtime and via saved environment. >>>> >>>>> >>>> >>>>> Signed-off-by: Simon Goldschmidt >>>> <simon.k.r.goldschmidt@gmail.com >>>> mailto:simon.k.r.goldschmidt@gmail.com> >>>> >>>> >>>> >>>> Would that be like a default "reset" command action ? >>>> >>>> This probably shouldn't be socfpga specific then. >>>> >>> >>>> >>> No, it's a thing that lives on and influences even the >>>> soft >>>> reset issued >>>> >>> by linux "reboot" command. This is something *very* >>>> socfpga >>>> specific. >>>> >> >>>> >> Hmmm, so isn't this a policy to be configured on the >>>> Linux >>>> end ? >>>> > >>>> > Might be, but it affects U-Boot's 'reset' command as >>>> well. And >>>> I guess >>>> > it's set up in U-Boot this early to ensure it always >>>> works. >>>> >>>> Drat, that's right. So there has to be some way to >>>> agree on >>>> how the >>>> reset works between the kernel and U-Boot ? >>>> >>>> > If it were for me, we could drop writing this magic >>>> altogether. I just >>>> > figured some boards might require it to be written >>>> somewhere, >>>> and came >>>> > up with a patch that might save those boards with the >>>> way >>>> it was >>>> before. >>>> >>>> Isn't this magic actually used by bootrom ? >>>> >>>> >>>> Right. It tells the boot rom to jump to ocram on next reboot >>>> instead of >>>> loading spl from qspi or mmc. But if that's required or not a good >>>> idea >>>> at all depends on many factors. Some of them board related, some >>>> U-Boot >>>> related and some Linux related (depending on the hardware and >>>> drivers >>>> used). >>> >>> Should that be runtime configurable then ? >> >> Since it might depend on Linux putting the qspi chip into a state >> where >> it's not accessible by the boot ROM. That might change without >> rebuilding U-Boot. > > If Linux switches the chip into some weird mode the bootrom cannot > cope > with, it's a reset routing problem. This cannot be fixed in software.
No, it cannot be fixed, but currently there's a workaround for those boards and I thought it was worth to keep this workaround, even though my own boards will be fixed and not require such a workaround in the future :-)
What's the workaround ?
The workaround is what this patch is about: the Boot ROM just branches off to SRAM where it expectes SPL to be still working.
But you still cannot communicate with the SPI NOR from your SPL ?
Well, in most "every day reboot" cases, you can. Just reset BAR or 4-byte mode.
"In most" reads as "it's unreliable".
SPL can then e.g. reset 4-byte mode or whatever to still communicate with the device when Boot ROM can't.
Unless, of course, the SPI NOR doesn't interpret the command as data and corrupts something in the flash itself.
Right, in this case, you can't.
Don't get me wrong, I'm not arguing for this to be totally right, of course I'd rahter get the boards fixed.
I'm just trying to find a way to keep this code in for people depending on it. I know we have some broken boards that depend on it. I could live with writing this magic in our private board code, but it's a bummer for other people upgrading if we removed it...
Put it in a board file with BIG FAT WARNING that it's wrong ?
Ok, sorry for taking so long to reply. I hope you can still follow the discussion nevertheless.
I'll give up this patch and post v2 that just removes the "warm reboot from SRAM" thing. Then, users of boards that now fail to reboot write that magic constant in their board files. (And if enough mainline boards should be affected, we could still add a Kconfig option "broken reset" or something.)
Regards, Simon
participants (2)
-
Marek Vasut -
Simon Goldschmidt