From mboxrd@z Thu Jan  1 00:00:00 1970
From: Peng Fan <peng.fan@nxp.com>
Date: Sun, 3 May 2020 13:14:20 +0000
Subject: [PATCH V2 1/9] uclass: cpu: Add new API to get udevice for
 current CPU
In-Reply-To: <CAPnjgZ22KAgZEQk3i6+bWd2rAAq-NtiFgS2W=0Zxm2JCRrdQrg@mail.gmail.com>
References: <20200501134418.7319-1-peng.fan@nxp.com>
 <CAPnjgZ22KAgZEQk3i6+bWd2rAAq-NtiFgS2W=0Zxm2JCRrdQrg@mail.gmail.com>
Message-ID: <DB6PR0402MB2760BAA84EB8640EFB45B5D288A90@DB6PR0402MB2760.eurprd04.prod.outlook.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Simon,

> Subject: Re: [PATCH V2 1/9] uclass: cpu: Add new API to get udevice for
> current CPU
> 
> Hi Peng,
> 
> On Fri, 1 May 2020 at 07:22, Peng Fan <peng.fan@nxp.com> wrote:
> >
> > When running on SoC with multiple clusters, the boot CPU may not be
> > fixed, saying booting from cluster A or cluster B.
> > Add a API that can return the udevice for current boot CPU.
> > Cpu driver needs to implement is_current_cpu interface for this
> > feature, otherwise the API only returns the first udevice in cpu
> > uclass.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > Signed-off-by: Ye Li <ye.li@nxp.com>
> > ---
> >
> > V2:
> >  Per Simon's comment,
> >   - Add cpu_is_current
> >   - use uclass_foreach_dev_probe
> >   - Update code comment
> >
> >  drivers/cpu/cpu-uclass.c | 34 ++++++++++++++++++++++++++++++++++
> >  include/cpu.h            | 23 +++++++++++++++++++++++
> >  2 files changed, 57 insertions(+)
> >
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> 
> > diff --git a/drivers/cpu/cpu-uclass.c b/drivers/cpu/cpu-uclass.c index
> > 457f77b7c8..33d38a0fde 100644
> > --- a/drivers/cpu/cpu-uclass.c
> > +++ b/drivers/cpu/cpu-uclass.c
> > @@ -10,6 +10,7 @@
> >  #include <errno.h>
> >  #include <dm/lists.h>
> >  #include <dm/root.h>
> > +#include <linux/err.h>
> >
> >  int cpu_probe_all(void)
> >  {
> > @@ -34,6 +35,39 @@ int cpu_probe_all(void)
> >         return 0;
> >  }
> >
> > +int cpu_is_current(struct udevice *cpu) {
> > +       struct cpu_ops *ops = cpu_get_ops(cpu);
> > +
> > +       if (ops && ops->is_current) {
> > +               if (ops->is_current(cpu))
> > +                       return 1;
> 
> return 0 here I think

I prefer to use 1 here, since is_current return 0 seems
werid to show it is the cpu that uboot is running from.

> 
> Also you should not check 'ops'.

I'll drop it.

Thanks,
Peng.

> 
> 
> > +       }
> > +
> > +       return -ENOSYS;
> > +}
> > +
> > +struct udevice *cpu_get_current_dev(void) {
> > +       struct udevice *cpu;
> > +       int ret;
> > +
> > +       uclass_foreach_dev_probe(UCLASS_CPU, cpu) {
> > +               if (cpu_is_current(cpu) > 0)
> > +                       return cpu;
> > +       }
> > +
> > +       /* If can't find current cpu device, use the first dev instead */
> > +       ret = uclass_first_device_err(UCLASS_CPU, &cpu);
> > +       if (ret) {
> > +               debug("%s: Could not get CPU device (err = %d)\n",
> > +                     __func__, ret);
> > +               return NULL;
> > +       }
> > +
> > +       return cpu;
> > +}
> > +
> >  int cpu_get_desc(struct udevice *dev, char *buf, int size)  {
> >         struct cpu_ops *ops = cpu_get_ops(dev); diff --git
> > a/include/cpu.h b/include/cpu.h index 6b1b6b37b3..2f283fe244 100644
> > --- a/include/cpu.h
> > +++ b/include/cpu.h
> > @@ -89,6 +89,15 @@ struct cpu_ops {
> >          * @return 0 if OK, -ENOSPC if buffer is too small, other -ve on
> error
> >          */
> >         int (*get_vendor)(struct udevice *dev, char *buf, int size);
> > +
> > +       /**
> > +        * is_current() - Check if the CPU that U-Boot is currently running
> from
> > +        *
> > +        * @dev:        Device to check (UCLASS_CPU)
> > +        * @return 1 if the CPU that U-Boot is currently running from, 0
> > +        *         if not.
> > +        */
> > +       int (*is_current)(struct udevice *dev);
> >  };
> >
> >  #define cpu_get_ops(dev)        ((struct cpu_ops *)(dev)->driver->ops)
> > @@ -137,4 +146,18 @@ int cpu_get_vendor(struct udevice *dev, char *buf,
> int size);
> >   */
> >  int cpu_probe_all(void);
> >
> > +/**
> > + * cpu_is_current() - Check if the CPU that U-Boot is currently
> > +running from
> > + *
> > + * Return: 1 if yes, - 0 if not
> > + */
> > +int cpu_is_current(struct udevice *cpu);
> > +
> > +/**
> > + * cpu_get_current_dev() - Get CPU udevice for current CPU
> > + *
> > + * Return: udevice if OK, - NULL on error  */ struct udevice
> > +*cpu_get_current_dev(void);
> > +
> >  #endif
> > --
> > 2.16.4
> >