| Message ID | 20220929002008.9177-1-christian@kohlschutter.com |
|---|---|
| State | Rejected |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | common: board_f: Fix crash in print_cpuinfo | expand |
Hi Christian, On Wed, 28 Sept 2022 at 18:20, Christian Kohlschütter <christian@kohlschutter.com> wrote: > > With CONFIG_DISPLAY_CPUINFO=y and CONFIG_CPU=y, the initcall sequence > may fail (and therefore hang the boot process) with an -ENODEV (err=-19) > error code. > > This is caused by either cpu_get_current_dev/cpu_get_desc failing to > return CPU information. > > If no CPU information can be obtained, fall-back to the non-Driver Model > implementation of print_cpuinfo. > > Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com> > --- > common/board_f.c | 14 +++++++++----- > include/init.h | 3 +-- > 2 files changed, 10 insertions(+), 7 deletions(-) No, we don't want to do this. If you have CPU enabled then the device must return the info. The non-DM code will go away one day. It is not intended as a fallback. Regards, Simon
> On 29. Sep 2022, at 04:36, Simon Glass <sjg@chromium.org> wrote: > > Hi Christian, > > On Wed, 28 Sept 2022 at 18:20, Christian Kohlschütter > <christian@kohlschutter.com> wrote: >> >> With CONFIG_DISPLAY_CPUINFO=y and CONFIG_CPU=y, the initcall sequence >> may fail (and therefore hang the boot process) with an -ENODEV (err=-19) >> error code. >> >> This is caused by either cpu_get_current_dev/cpu_get_desc failing to >> return CPU information. >> >> If no CPU information can be obtained, fall-back to the non-Driver Model >> implementation of print_cpuinfo. >> >> Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com> >> --- >> common/board_f.c | 14 +++++++++----- >> include/init.h | 3 +-- >> 2 files changed, 10 insertions(+), 7 deletions(-) > > No, we don't want to do this. If you have CPU enabled then the device > must return the info. The non-DM code will go away one day. It is not > intended as a fallback. > > Regards, > Simon Thanks for the clarification, Simon. That's what I thought (it's not really documented with CONFIG_CPU but I get the idea). It looks like the new CONFIG_CPU feature isn't really supported for ARM boards, and the existing print_cpuinfo shows additional information that may not be captured with the new setup, such as "reset cause", etc. What are the plans/timeline for implementing the new feature? I'm specifically asking because the new feature may help improve the coverage of smbios information available downstream.
Hi Christian, On Thu, 29 Sept 2022 at 08:24, Christian Kohlschütter <christian@kohlschutter.com> wrote: > > > On 29. Sep 2022, at 04:36, Simon Glass <sjg@chromium.org> wrote: > > > > Hi Christian, > > > > On Wed, 28 Sept 2022 at 18:20, Christian Kohlschütter > > <christian@kohlschutter.com> wrote: > >> > >> With CONFIG_DISPLAY_CPUINFO=y and CONFIG_CPU=y, the initcall sequence > >> may fail (and therefore hang the boot process) with an -ENODEV (err=-19) > >> error code. > >> > >> This is caused by either cpu_get_current_dev/cpu_get_desc failing to > >> return CPU information. > >> > >> If no CPU information can be obtained, fall-back to the non-Driver Model > >> implementation of print_cpuinfo. > >> > >> Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com> > >> --- > >> common/board_f.c | 14 +++++++++----- > >> include/init.h | 3 +-- > >> 2 files changed, 10 insertions(+), 7 deletions(-) > > > > No, we don't want to do this. If you have CPU enabled then the device > > must return the info. The non-DM code will go away one day. It is not > > intended as a fallback. > > > > Regards, > > Simon > > Thanks for the clarification, Simon. That's what I thought (it's not really documented with CONFIG_CPU but I get the idea). > > It looks like the new CONFIG_CPU feature isn't really supported for ARM boards, and the existing print_cpuinfo shows additional information that may not be captured with the new setup, such as "reset cause", etc. What are the plans/timeline for implementing the new feature? It really depends on when people send patches for it. You could send something to help here. > > I'm specifically asking because the new feature may help improve the coverage of smbios information available downstream. > OK Regards, Simon
diff --git a/common/board_f.c b/common/board_f.c index 18e2246733..0656845e24 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -182,9 +182,12 @@ static int print_resetinfo(void) } #endif -#if defined(CONFIG_DISPLAY_CPUINFO) && CONFIG_IS_ENABLED(CPU) -static int print_cpuinfo(void) +#if defined(CONFIG_DISPLAY_CPUINFO) +static int print_cpuinfo_0(void) { +#if !CONFIG_IS_ENABLED(CPU) + return print_cpuinfo(); +#else struct udevice *dev; char desc[512]; int ret; @@ -193,19 +196,20 @@ static int print_cpuinfo(void) if (!dev) { debug("%s: Could not get CPU device\n", __func__); - return -ENODEV; + return print_cpuinfo(); // fallback } ret = cpu_get_desc(dev, desc, sizeof(desc)); if (ret) { debug("%s: Could not get CPU description (err = %d)\n", dev->name, ret); - return ret; + return print_cpuinfo(); // fallback } printf("CPU: %s\n", desc); return 0; +#endif } #endif @@ -866,7 +870,7 @@ static const init_fnc_t init_sequence_f[] = { print_resetinfo, #endif #if defined(CONFIG_DISPLAY_CPUINFO) - print_cpuinfo, /* display cpu info (and speed) */ + print_cpuinfo_0, /* display cpu info (and speed) */ #endif #if defined(CONFIG_DTB_RESELECT) embedded_dtb_select, diff --git a/include/init.h b/include/init.h index 7b8f62c121..9837c22b75 100644 --- a/include/init.h +++ b/include/init.h @@ -210,14 +210,13 @@ int pci_init(void); */ int init_cache_f_r(void); -#if !CONFIG_IS_ENABLED(CPU) /** * print_cpuinfo() - Display information about the CPU * * Return: 0 if OK, -ve on error */ int print_cpuinfo(void); -#endif + int timer_init(void); #if defined(CONFIG_DTB_RESELECT)
With CONFIG_DISPLAY_CPUINFO=y and CONFIG_CPU=y, the initcall sequence may fail (and therefore hang the boot process) with an -ENODEV (err=-19) error code. This is caused by either cpu_get_current_dev/cpu_get_desc failing to return CPU information. If no CPU information can be obtained, fall-back to the non-Driver Model implementation of print_cpuinfo. Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com> --- common/board_f.c | 14 +++++++++----- include/init.h | 3 +-- 2 files changed, 10 insertions(+), 7 deletions(-)