| Message ID | 20240730073003.28892-11-Zhiqiang.Hou@nxp.com |
|---|---|
| State | Superseded |
| Delegated to: | Fabio Estevam |
| Headers | show |
| Series | Add a subcommand 'release' to cmd/cpu.c | expand |
Hi Zhiqiang, On Tue, 30 Jul 2024 at 01:09, Zhiqiang Hou <Zhiqiang.Hou@nxp.com> wrote: > > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > > Add a new subcommand 'release' to bring up a core to run baremetal > and RTOS applications. > > For example on i.MX8M Plus EVK, release the LAST core to run a RTOS > application, passing the sequence number of the CPU core to release, > here it is 3: > u-boot=> cpu list > 0: cpu@0 NXP i.MX8MP Rev1.1 A53 at 1200 MHz at 31C > 1: cpu@1 NXP i.MX8MP Rev1.1 A53 at 1200 MHz at 30C > 2: cpu@2 NXP i.MX8MP Rev1.1 A53 at 1200 MHz at 31C > 3: cpu@3 NXP i.MX8MP Rev1.1 A53 at 1200 MHz at 31C > > u-boot=> load mmc 1:2 c0000000 /hello_world.bin > 66008 bytes read in 5 ms (12.6 MiB/s) > u-boot=> dcache flush; icache flush > u-boot=> cpu release 3 c0000000 > Released CPU core (mpidr: 0x3) to address 0xc0000000 > > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > Reviewed-by: Simon Glass <sjg@chromium.org> > --- > V4: > - Updated the change logs. > > cmd/cpu.c | 44 ++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 42 insertions(+), 2 deletions(-) > > diff --git a/cmd/cpu.c b/cmd/cpu.c > index 9e323069b9..2755250756 100644 > --- a/cmd/cpu.c > +++ b/cmd/cpu.c > @@ -3,6 +3,7 @@ > * Copyright (c) 2015 Google, Inc > * Written by Simon Glass <sjg@chromium.org> > * Copyright (c) 2017 Álvaro Fernández Rojas <noltari@gmail.com> > + * Copyright 2024 NXP > */ > > #include <command.h> > @@ -18,6 +19,19 @@ static const char *cpu_feature_name[CPU_FEAT_COUNT] = { > "Device ID", > }; > > +static struct udevice *cpu_find_device(unsigned long cpu_id) > +{ > + struct udevice *dev; > + > + for (uclass_first_device(UCLASS_CPU, &dev); dev; > + uclass_next_device(&dev)) { > + if (cpu_id == dev_seq(dev)) > + return dev; > + } > + > + return NULL; > +} > + > static int print_cpu_list(bool detail) > { > struct udevice *dev; > @@ -82,10 +96,36 @@ static int do_cpu_detail(struct cmd_tbl *cmdtp, int flag, int argc, > return 0; > } > > +static int do_cpu_release(struct cmd_tbl *cmdtp, int flag, int argc, > + char *const argv[]) > +{ > + struct udevice *dev; > + unsigned long cpu_id; > + unsigned long long boot_addr; > + > + if (argc != 3) > + return CMD_RET_USAGE; > + > + cpu_id = dectoul(argv[1], NULL); I just noticed this...normally we use hex for parameters. > + dev = cpu_find_device(cpu_id); > + if (!dev) > + return CMD_RET_FAILURE; > + > + boot_addr = simple_strtoull(argv[2], NULL, 16); > + > + if (cpu_release_core(dev, boot_addr)) > + return CMD_RET_FAILURE; > + > + return 0; > +} > + > U_BOOT_LONGHELP(cpu, > "list - list available CPUs\n" > - "cpu detail - show CPU detail"); > + "cpu detail - show CPU detail\n" > + "cpu release <core ID> <addr> - Release CPU <core ID> at <addr>\n" > + " <core ID>: the sequence number in list subcommand outputs"); > > U_BOOT_CMD_WITH_SUBCMDS(cpu, "display information about CPUs", cpu_help_text, > U_BOOT_SUBCMD_MKENT(list, 1, 1, do_cpu_list), > - U_BOOT_SUBCMD_MKENT(detail, 1, 0, do_cpu_detail)); > + U_BOOT_SUBCMD_MKENT(detail, 1, 0, do_cpu_detail), > + U_BOOT_SUBCMD_MKENT(release, 3, 0, do_cpu_release)); > -- > 2.17.1 > Regards, Simon
Hi Simon, > -----Original Message----- > From: Simon Glass <sjg@chromium.org> > Sent: Wednesday, July 31, 2024 10:39 PM > To: Z.Q. Hou <zhiqiang.hou@nxp.com> > Cc: u-boot@lists.denx.de; trini@konsulko.com; Peng Fan > <peng.fan@nxp.com>; festevam@gmail.com; marex@denx.de; > lukma@denx.de; seanga2@gmail.com; xypron.glpk@gmx.de > Subject: Re: [PATCHv4 10/14] cmd: cpu: add release subcommand > > Hi Zhiqiang, > > On Tue, 30 Jul 2024 at 01:09, Zhiqiang Hou <Zhiqiang.Hou@nxp.com> wrote: > > > > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > > > > Add a new subcommand 'release' to bring up a core to run baremetal and > > RTOS applications. > > > > For example on i.MX8M Plus EVK, release the LAST core to run a RTOS > > application, passing the sequence number of the CPU core to release, > > here it is 3: > > u-boot=> cpu list > > 0: cpu@0 NXP i.MX8MP Rev1.1 A53 at 1200 MHz at 31C > > 1: cpu@1 NXP i.MX8MP Rev1.1 A53 at 1200 MHz at 30C > > 2: cpu@2 NXP i.MX8MP Rev1.1 A53 at 1200 MHz at 31C > > 3: cpu@3 NXP i.MX8MP Rev1.1 A53 at 1200 MHz at 31C > > > > u-boot=> load mmc 1:2 c0000000 /hello_world.bin > > 66008 bytes read in 5 ms (12.6 MiB/s) > > u-boot=> dcache flush; icache flush > > u-boot=> cpu release 3 c0000000 > > Released CPU core (mpidr: 0x3) to address 0xc0000000 > > > > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > > Reviewed-by: Simon Glass <sjg@chromium.org> > > --- > > V4: > > - Updated the change logs. > > > > cmd/cpu.c | 44 ++++++++++++++++++++++++++++++++++++++++++-- > > 1 file changed, 42 insertions(+), 2 deletions(-) > > > > diff --git a/cmd/cpu.c b/cmd/cpu.c > > index 9e323069b9..2755250756 100644 > > --- a/cmd/cpu.c > > +++ b/cmd/cpu.c > > @@ -3,6 +3,7 @@ > > * Copyright (c) 2015 Google, Inc > > * Written by Simon Glass <sjg@chromium.org> > > * Copyright (c) 2017 Álvaro Fernández Rojas <noltari@gmail.com> > > + * Copyright 2024 NXP > > */ > > > > #include <command.h> > > @@ -18,6 +19,19 @@ static const char > *cpu_feature_name[CPU_FEAT_COUNT] = { > > "Device ID", > > }; > > > > +static struct udevice *cpu_find_device(unsigned long cpu_id) { > > + struct udevice *dev; > > + > > + for (uclass_first_device(UCLASS_CPU, &dev); dev; > > + uclass_next_device(&dev)) { > > + if (cpu_id == dev_seq(dev)) > > + return dev; > > + } > > + > > + return NULL; > > +} > > + > > static int print_cpu_list(bool detail) { > > struct udevice *dev; > > @@ -82,10 +96,36 @@ static int do_cpu_detail(struct cmd_tbl *cmdtp, int > flag, int argc, > > return 0; > > } > > > > +static int do_cpu_release(struct cmd_tbl *cmdtp, int flag, int argc, > > + char *const argv[]) { > > + struct udevice *dev; > > + unsigned long cpu_id; > > + unsigned long long boot_addr; > > + > > + if (argc != 3) > > + return CMD_RET_USAGE; > > + > > + cpu_id = dectoul(argv[1], NULL); > > I just noticed this...normally we use hex for parameters. Yes, but the <core ID> is from the CPU device sequence number listed by the 'cpu list', which is printed in dec. So I think use dec parameter here is reasonable. Thanks, Zhiqiang
Hi Zhiqiang, On Wed, 31 Jul 2024 at 20:37, Z.Q. Hou <zhiqiang.hou@nxp.com> wrote: > > Hi Simon, > > > -----Original Message----- > > From: Simon Glass <sjg@chromium.org> > > Sent: Wednesday, July 31, 2024 10:39 PM > > To: Z.Q. Hou <zhiqiang.hou@nxp.com> > > Cc: u-boot@lists.denx.de; trini@konsulko.com; Peng Fan > > <peng.fan@nxp.com>; festevam@gmail.com; marex@denx.de; > > lukma@denx.de; seanga2@gmail.com; xypron.glpk@gmx.de > > Subject: Re: [PATCHv4 10/14] cmd: cpu: add release subcommand > > > > Hi Zhiqiang, > > > > On Tue, 30 Jul 2024 at 01:09, Zhiqiang Hou <Zhiqiang.Hou@nxp.com> wrote: > > > > > > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > > > > > > Add a new subcommand 'release' to bring up a core to run baremetal and > > > RTOS applications. > > > > > > For example on i.MX8M Plus EVK, release the LAST core to run a RTOS > > > application, passing the sequence number of the CPU core to release, > > > here it is 3: > > > u-boot=> cpu list > > > 0: cpu@0 NXP i.MX8MP Rev1.1 A53 at 1200 MHz at 31C > > > 1: cpu@1 NXP i.MX8MP Rev1.1 A53 at 1200 MHz at 30C > > > 2: cpu@2 NXP i.MX8MP Rev1.1 A53 at 1200 MHz at 31C > > > 3: cpu@3 NXP i.MX8MP Rev1.1 A53 at 1200 MHz at 31C > > > > > > u-boot=> load mmc 1:2 c0000000 /hello_world.bin > > > 66008 bytes read in 5 ms (12.6 MiB/s) > > > u-boot=> dcache flush; icache flush > > > u-boot=> cpu release 3 c0000000 > > > Released CPU core (mpidr: 0x3) to address 0xc0000000 > > > > > > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > --- > > > V4: > > > - Updated the change logs. > > > > > > cmd/cpu.c | 44 ++++++++++++++++++++++++++++++++++++++++++-- > > > 1 file changed, 42 insertions(+), 2 deletions(-) > > > > > > diff --git a/cmd/cpu.c b/cmd/cpu.c > > > index 9e323069b9..2755250756 100644 > > > --- a/cmd/cpu.c > > > +++ b/cmd/cpu.c > > > @@ -3,6 +3,7 @@ > > > * Copyright (c) 2015 Google, Inc > > > * Written by Simon Glass <sjg@chromium.org> > > > * Copyright (c) 2017 Álvaro Fernández Rojas <noltari@gmail.com> > > > + * Copyright 2024 NXP > > > */ > > > > > > #include <command.h> > > > @@ -18,6 +19,19 @@ static const char > > *cpu_feature_name[CPU_FEAT_COUNT] = { > > > "Device ID", > > > }; > > > > > > +static struct udevice *cpu_find_device(unsigned long cpu_id) { > > > + struct udevice *dev; > > > + > > > + for (uclass_first_device(UCLASS_CPU, &dev); dev; > > > + uclass_next_device(&dev)) { > > > + if (cpu_id == dev_seq(dev)) > > > + return dev; > > > + } > > > + > > > + return NULL; > > > +} > > > + > > > static int print_cpu_list(bool detail) { > > > struct udevice *dev; > > > @@ -82,10 +96,36 @@ static int do_cpu_detail(struct cmd_tbl *cmdtp, int > > flag, int argc, > > > return 0; > > > } > > > > > > +static int do_cpu_release(struct cmd_tbl *cmdtp, int flag, int argc, > > > + char *const argv[]) { > > > + struct udevice *dev; > > > + unsigned long cpu_id; > > > + unsigned long long boot_addr; > > > + > > > + if (argc != 3) > > > + return CMD_RET_USAGE; > > > + > > > + cpu_id = dectoul(argv[1], NULL); > > > > I just noticed this...normally we use hex for parameters. > > Yes, but the <core ID> is from the CPU device sequence number listed by the 'cpu list', which is printed in dec. So I think use dec parameter here is reasonable. We will have devices with hundreds of CPUs so decimal is probably going to bite us. U-Boot uses hex for most values. Anyway, let's worry about it later. What you have is fine. Regards, Simon
Hi Simon, > -----Original Message----- > From: Simon Glass <sjg@chromium.org> > Sent: Thursday, August 1, 2024 10:42 PM > To: Z.Q. Hou <zhiqiang.hou@nxp.com> > Cc: u-boot@lists.denx.de; trini@konsulko.com; Peng Fan > <peng.fan@nxp.com>; festevam@gmail.com; marex@denx.de; > lukma@denx.de; seanga2@gmail.com; xypron.glpk@gmx.de > Subject: Re: [PATCHv4 10/14] cmd: cpu: add release subcommand > > Hi Zhiqiang, > > On Wed, 31 Jul 2024 at 20:37, Z.Q. Hou <zhiqiang.hou@nxp.com> wrote: > > > > Hi Simon, > > > > > -----Original Message----- > > > From: Simon Glass <sjg@chromium.org> > > > Sent: Wednesday, July 31, 2024 10:39 PM > > > To: Z.Q. Hou <zhiqiang.hou@nxp.com> > > > Cc: u-boot@lists.denx.de; trini@konsulko.com; Peng Fan > > > <peng.fan@nxp.com>; festevam@gmail.com; marex@denx.de; > > > lukma@denx.de; seanga2@gmail.com; xypron.glpk@gmx.de > > > Subject: Re: [PATCHv4 10/14] cmd: cpu: add release subcommand > > > > > > Hi Zhiqiang, > > > > > > On Tue, 30 Jul 2024 at 01:09, Zhiqiang Hou <Zhiqiang.Hou@nxp.com> > wrote: > > > > > > > > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > > > > > > > > Add a new subcommand 'release' to bring up a core to run baremetal > > > > and RTOS applications. > > > > > > > > For example on i.MX8M Plus EVK, release the LAST core to run a > > > > RTOS application, passing the sequence number of the CPU core to > > > > release, here it is 3: > > > > u-boot=> cpu list > > > > 0: cpu@0 NXP i.MX8MP Rev1.1 A53 at 1200 MHz at 31C > > > > 1: cpu@1 NXP i.MX8MP Rev1.1 A53 at 1200 MHz at 30C > > > > 2: cpu@2 NXP i.MX8MP Rev1.1 A53 at 1200 MHz at 31C > > > > 3: cpu@3 NXP i.MX8MP Rev1.1 A53 at 1200 MHz at 31C > > > > > > > > u-boot=> load mmc 1:2 c0000000 /hello_world.bin > > > > 66008 bytes read in 5 ms (12.6 MiB/s) > > > > u-boot=> dcache flush; icache flush > > > > u-boot=> cpu release 3 c0000000 > > > > Released CPU core (mpidr: 0x3) to address 0xc0000000 > > > > > > > > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com> > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > > --- > > > > V4: > > > > - Updated the change logs. > > > > > > > > cmd/cpu.c | 44 ++++++++++++++++++++++++++++++++++++++++++-- > > > > 1 file changed, 42 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/cmd/cpu.c b/cmd/cpu.c index 9e323069b9..2755250756 > > > > 100644 > > > > --- a/cmd/cpu.c > > > > +++ b/cmd/cpu.c > > > > @@ -3,6 +3,7 @@ > > > > * Copyright (c) 2015 Google, Inc > > > > * Written by Simon Glass <sjg@chromium.org> > > > > * Copyright (c) 2017 Álvaro Fernández Rojas <noltari@gmail.com> > > > > + * Copyright 2024 NXP > > > > */ > > > > > > > > #include <command.h> > > > > @@ -18,6 +19,19 @@ static const char > > > *cpu_feature_name[CPU_FEAT_COUNT] = { > > > > "Device ID", > > > > }; > > > > > > > > +static struct udevice *cpu_find_device(unsigned long cpu_id) { > > > > + struct udevice *dev; > > > > + > > > > + for (uclass_first_device(UCLASS_CPU, &dev); dev; > > > > + uclass_next_device(&dev)) { > > > > + if (cpu_id == dev_seq(dev)) > > > > + return dev; > > > > + } > > > > + > > > > + return NULL; > > > > +} > > > > + > > > > static int print_cpu_list(bool detail) { > > > > struct udevice *dev; > > > > @@ -82,10 +96,36 @@ static int do_cpu_detail(struct cmd_tbl > > > > *cmdtp, int > > > flag, int argc, > > > > return 0; > > > > } > > > > > > > > +static int do_cpu_release(struct cmd_tbl *cmdtp, int flag, int argc, > > > > + char *const argv[]) { > > > > + struct udevice *dev; > > > > + unsigned long cpu_id; > > > > + unsigned long long boot_addr; > > > > + > > > > + if (argc != 3) > > > > + return CMD_RET_USAGE; > > > > + > > > > + cpu_id = dectoul(argv[1], NULL); > > > > > > I just noticed this...normally we use hex for parameters. > > > > Yes, but the <core ID> is from the CPU device sequence number listed by the > 'cpu list', which is printed in dec. So I think use dec parameter here is > reasonable. > > We will have devices with hundreds of CPUs so decimal is probably going to > bite us. U-Boot uses hex for most values. > > Anyway, let's worry about it later. What you have is fine. Okay, I think the key point is to keep the CPU core sequence number in 'list' subcommand and the input <core ID> using the same format. Thanks, Zhiqiang
diff --git a/cmd/cpu.c b/cmd/cpu.c index 9e323069b9..2755250756 100644 --- a/cmd/cpu.c +++ b/cmd/cpu.c @@ -3,6 +3,7 @@ * Copyright (c) 2015 Google, Inc * Written by Simon Glass <sjg@chromium.org> * Copyright (c) 2017 Álvaro Fernández Rojas <noltari@gmail.com> + * Copyright 2024 NXP */ #include <command.h> @@ -18,6 +19,19 @@ static const char *cpu_feature_name[CPU_FEAT_COUNT] = { "Device ID", }; +static struct udevice *cpu_find_device(unsigned long cpu_id) +{ + struct udevice *dev; + + for (uclass_first_device(UCLASS_CPU, &dev); dev; + uclass_next_device(&dev)) { + if (cpu_id == dev_seq(dev)) + return dev; + } + + return NULL; +} + static int print_cpu_list(bool detail) { struct udevice *dev; @@ -82,10 +96,36 @@ static int do_cpu_detail(struct cmd_tbl *cmdtp, int flag, int argc, return 0; } +static int do_cpu_release(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) +{ + struct udevice *dev; + unsigned long cpu_id; + unsigned long long boot_addr; + + if (argc != 3) + return CMD_RET_USAGE; + + cpu_id = dectoul(argv[1], NULL); + dev = cpu_find_device(cpu_id); + if (!dev) + return CMD_RET_FAILURE; + + boot_addr = simple_strtoull(argv[2], NULL, 16); + + if (cpu_release_core(dev, boot_addr)) + return CMD_RET_FAILURE; + + return 0; +} + U_BOOT_LONGHELP(cpu, "list - list available CPUs\n" - "cpu detail - show CPU detail"); + "cpu detail - show CPU detail\n" + "cpu release <core ID> <addr> - Release CPU <core ID> at <addr>\n" + " <core ID>: the sequence number in list subcommand outputs"); U_BOOT_CMD_WITH_SUBCMDS(cpu, "display information about CPUs", cpu_help_text, U_BOOT_SUBCMD_MKENT(list, 1, 1, do_cpu_list), - U_BOOT_SUBCMD_MKENT(detail, 1, 0, do_cpu_detail)); + U_BOOT_SUBCMD_MKENT(detail, 1, 0, do_cpu_detail), + U_BOOT_SUBCMD_MKENT(release, 3, 0, do_cpu_release));