Hi Simon,
> -----Original Message-----
> From: Simon Glass <s...@chromium.org>
> Sent: Thursday, August 1, 2024 10:42 PM
> To: Z.Q. Hou <zhiqiang....@nxp.com>
> Cc: u-boot@lists.denx.de; tr...@konsulko.com; Peng Fan
> <peng....@nxp.com>; feste...@gmail.com; ma...@denx.de;
> lu...@denx.de; sean...@gmail.com; xypron.g...@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....@nxp.com> wrote:
> >
> > Hi Simon,
> >
> > > -----Original Message-----
> > > From: Simon Glass <s...@chromium.org>
> > > Sent: Wednesday, July 31, 2024 10:39 PM
> > > To: Z.Q. Hou <zhiqiang....@nxp.com>
> > > Cc: u-boot@lists.denx.de; tr...@konsulko.com; Peng Fan
> > > <peng....@nxp.com>; feste...@gmail.com; ma...@denx.de;
> > > lu...@denx.de; sean...@gmail.com; xypron.g...@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....@nxp.com>
> wrote:
> > > >
> > > > From: Hou Zhiqiang <zhiqiang....@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....@nxp.com>
> > > > Reviewed-by: Simon Glass <s...@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 <s...@chromium.org>
> > > > * Copyright (c) 2017 Álvaro Fernández Rojas <nolt...@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
