Re: [U-Boot] [PATCH 4/4] x86: Switch to use DM sysreset driver
Hi Andy,
On Tue, Jul 3, 2018 at 6:42 PM, Andy Shevchenko
wrote:
> On Tue, 2018-07-03 at 02:48 -0700, Bin Meng wrote:
>> This converts all x86 boards over to DM sysreset.
>
>> -int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const
>> argv[])
>> -{
>> - printf("resetting ...\n");
>> -
>> - /* wait 50 ms */
>> - udelay(5);
>> - disable_interrupts();
>> - reset_cpu(0);
>
>> -}
>
>> --- a/arch/x86/cpu/tangier/tangier.c
>> +++ b/arch/x86/cpu/tangier/tangier.c
>> @@ -25,7 +25,15 @@ int print_cpuinfo(void)
>> return default_print_cpuinfo();
>> }
>>
>> +/* TODO: convert to DM sysreset */
>> void reset_cpu(ulong addr)
>> {
>> scu_ipc_simple_command(IPCMSG_COLD_RESET, 0);
>> }
>> +
>> +int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const
>> argv[])
>> +{
>> + reset_cpu(0);
>> +
>> + return 0;
>
> This is not equivalent to the above.
>
> First of all, in some cases would be good to have at least a debug
> message that we got into do_reset().
>
> Second, I didn't test if udelay() + disable_interrupts() make any
> difference. So, I would leave them for now.
>
OK, will leave them in v2.
>> +}
>
>> diff --git a/arch/x86/dts/edison.dts b/arch/x86/dts/edison.dts
>> index 9033532..a1d3c90 100644
>> --- a/arch/x86/dts/edison.dts
>> +++ b/arch/x86/dts/edison.dts
>> @@ -9,6 +9,7 @@
>> #include
>>
>> /include/ "skeleton.dtsi"
>
>> +/include/ "reset.dtsi"
>
> If i read this right we are not using generic reset sequence.
> Why do we include this here?
>
This is a mistake. Will fix in v2.
BTW: do you have time to convert the tangier SoC reset driver to DM?
>> /include/ "rtc.dtsi"
>> /include/ "tsc_timer.dtsi"
>
>> --- a/configs/edison_defconfig
>> +++ b/configs/edison_defconfig
>
>> +# CONFIG_SYSRESET is not set
>
> --
Regards,
Bin
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 4/4] x86: Switch to use DM sysreset driver
Dear Andy,
In message <18255ae55e4faa5733fb8e93dc8ebbc1559a3694.ca...@linux.intel.com> you
wrote:
...
> > This converts all x86 boards over to DM sysreset.
>
> > -int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> > argv[])
> > -{
> > - printf("resetting ...\n");
> > -
> > - /* wait 50 ms */
> > - udelay(5);
I am willing to bet that the delay here is a dirty surrogate for
flushing/closing the standard output channel, i. e. we wait until
the characters have actually been set over the serial console.
Don't we have a way to close() the device in DM (remove ?) ?
> First of all, in some cases would be good to have at least a debug
> message that we got into do_reset().
This should not only be a debug message, but a standard message;
actually even something that goes to STDERR.
> Second, I didn't test if udelay() + disable_interrupts() make any
> difference. So, I would leave them for now.
See above... I think the delay should be replaced by the proper way
to close the device in DM (remove?).
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: w...@denx.de
But the only way of discovering the limits of the possible is to
venture a little way past them into the impossible.
- _Profiles of the Future_ (1962; rev. 1973)
``Hazards of Prophecy: The Failure of Imagination''
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 4/4] x86: Switch to use DM sysreset driver
On Tue, 2018-07-03 at 02:48 -0700, Bin Meng wrote:
> This converts all x86 boards over to DM sysreset.
> -int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> argv[])
> -{
> - printf("resetting ...\n");
> -
> - /* wait 50 ms */
> - udelay(5);
> - disable_interrupts();
> - reset_cpu(0);
> -}
> --- a/arch/x86/cpu/tangier/tangier.c
> +++ b/arch/x86/cpu/tangier/tangier.c
> @@ -25,7 +25,15 @@ int print_cpuinfo(void)
> return default_print_cpuinfo();
> }
>
> +/* TODO: convert to DM sysreset */
> void reset_cpu(ulong addr)
> {
> scu_ipc_simple_command(IPCMSG_COLD_RESET, 0);
> }
> +
> +int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> argv[])
> +{
> + reset_cpu(0);
> +
> + return 0;
This is not equivalent to the above.
First of all, in some cases would be good to have at least a debug
message that we got into do_reset().
Second, I didn't test if udelay() + disable_interrupts() make any
difference. So, I would leave them for now.
> +}
> diff --git a/arch/x86/dts/edison.dts b/arch/x86/dts/edison.dts
> index 9033532..a1d3c90 100644
> --- a/arch/x86/dts/edison.dts
> +++ b/arch/x86/dts/edison.dts
> @@ -9,6 +9,7 @@
> #include
>
> /include/ "skeleton.dtsi"
> +/include/ "reset.dtsi"
If i read this right we are not using generic reset sequence.
Why do we include this here?
> /include/ "rtc.dtsi"
> /include/ "tsc_timer.dtsi"
> --- a/configs/edison_defconfig
> +++ b/configs/edison_defconfig
> +# CONFIG_SYSRESET is not set
--
Andy Shevchenko
Intel Finland Oy
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 4/4] x86: Switch to use DM sysreset driver
This converts all x86 boards over to DM sysreset.
Signed-off-by: Bin Meng
---
arch/Kconfig | 2 ++
arch/x86/cpu/baytrail/valleyview.c| 6 --
arch/x86/cpu/braswell/braswell.c | 6 --
arch/x86/cpu/cpu.c| 26 --
arch/x86/cpu/ivybridge/early_me.c | 7 ---
arch/x86/cpu/ivybridge/sdram.c| 3 ++-
arch/x86/cpu/qemu/qemu.c | 6 --
arch/x86/cpu/quark/quark.c| 6 --
arch/x86/cpu/tangier/tangier.c| 8
arch/x86/dts/bayleybay.dts| 1 +
arch/x86/dts/baytrail_som-db5800-som-6867.dts | 1 +
arch/x86/dts/broadwell_som-6896.dts | 1 +
arch/x86/dts/cherryhill.dts | 1 +
arch/x86/dts/chromebook_link.dts | 1 +
arch/x86/dts/chromebook_samus.dts | 1 +
arch/x86/dts/chromebox_panther.dts| 1 +
arch/x86/dts/conga-qeval20-qa3-e3845.dts | 1 +
arch/x86/dts/cougarcanyon2.dts| 1 +
arch/x86/dts/crownbay.dts | 1 +
arch/x86/dts/dfi-bt700.dtsi | 1 +
arch/x86/dts/edison.dts | 1 +
arch/x86/dts/efi-x86_payload.dts | 1 +
arch/x86/dts/galileo.dts | 1 +
arch/x86/dts/minnowmax.dts| 1 +
arch/x86/dts/qemu-x86_i440fx.dts | 1 +
arch/x86/dts/qemu-x86_q35.dts | 1 +
arch/x86/dts/reset.dtsi | 6 ++
arch/x86/include/asm/processor.h | 5 -
arch/x86/include/asm/u-boot-x86.h | 1 -
configs/chromebook_link64_defconfig | 1 +
configs/edison_defconfig | 1 +
configs/efi-x86_app_defconfig | 1 +
32 files changed, 42 insertions(+), 60 deletions(-)
create mode 100644 arch/x86/dts/reset.dtsi
diff --git a/arch/Kconfig b/arch/Kconfig
index dd5a887..cbeb9f6 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -118,6 +118,8 @@ config X86
imply DM_SPI_FLASH
imply DM_USB
imply DM_VIDEO
+ imply SYSRESET
+ imply SYSRESET_X86
imply CMD_FPGA_LOADMK
imply CMD_GETTIME
imply CMD_IO
diff --git a/arch/x86/cpu/baytrail/valleyview.c
b/arch/x86/cpu/baytrail/valleyview.c
index b7d481a..8882a76 100644
--- a/arch/x86/cpu/baytrail/valleyview.c
+++ b/arch/x86/cpu/baytrail/valleyview.c
@@ -55,9 +55,3 @@ int arch_misc_init(void)
return 0;
}
-
-void reset_cpu(ulong addr)
-{
- /* cold reset */
- x86_full_reset();
-}
diff --git a/arch/x86/cpu/braswell/braswell.c b/arch/x86/cpu/braswell/braswell.c
index 32a6a5e..7a83b06 100644
--- a/arch/x86/cpu/braswell/braswell.c
+++ b/arch/x86/cpu/braswell/braswell.c
@@ -27,9 +27,3 @@ int arch_misc_init(void)
return 0;
}
-
-void reset_cpu(ulong addr)
-{
- /* cold reset */
- x86_full_reset();
-}
diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
index 3a45677..395f845 100644
--- a/arch/x86/cpu/cpu.c
+++ b/arch/x86/cpu/cpu.c
@@ -76,37 +76,11 @@ int x86_init_cache(void)
}
int init_cache(void) __attribute__((weak, alias("x86_init_cache")));
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
-{
- printf("resetting ...\n");
-
- /* wait 50 ms */
- udelay(5);
- disable_interrupts();
- reset_cpu(0);
-
- /*NOTREACHED*/
- return 0;
-}
-
void flush_cache(unsigned long dummy1, unsigned long dummy2)
{
asm("wbinvd\n");
}
-__weak void reset_cpu(ulong addr)
-{
- /* Do a hard reset through the chipset's reset control register */
- outb(SYS_RST | RST_CPU, IO_PORT_RESET);
- for (;;)
- cpu_hlt();
-}
-
-void x86_full_reset(void)
-{
- outb(FULL_RST | SYS_RST | RST_CPU, IO_PORT_RESET);
-}
-
/* Define these functions to allow ehch-hcd to function */
void flush_dcache_range(unsigned long start, unsigned long stop)
{
diff --git a/arch/x86/cpu/ivybridge/early_me.c
b/arch/x86/cpu/ivybridge/early_me.c
index 1a15229..219d5be 100644
--- a/arch/x86/cpu/ivybridge/early_me.c
+++ b/arch/x86/cpu/ivybridge/early_me.c
@@ -8,6 +8,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -138,17 +139,17 @@ int intel_early_me_init_done(struct udevice *dev, struct
udevice *me_dev,
case ME_HFS_ACK_RESET:
/* Non-power cycle reset */
set_global_reset(dev, 0);
- reset_cpu(0);
+ sysreset_walk_halt(SYSRESET_COLD);
break;
case ME_HFS_ACK_PWR_CYCLE:
/* Power cycle reset */
set_global_reset(dev, 0);
- x86_full_reset();
+ sysreset_walk_halt(SYSRESET_COLD);
break;
case ME_HFS_ACK_GBL_RESET:
/* Global reset */
set_global_reset(dev, 1);
-
