Hi Simon,
Subject: Re: [PATCH V2 1/9] uclass: cpu: Add new API to get udevice for current CPU
Hi Peng,
On Sun, 3 May 2020 at 07:14, Peng Fan peng.fan@nxp.com wrote:
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
Sorry that as very unclear. I mean that you should have something like:
if (ops->is_current) { if (ops->is_current(cpu)) return 1; return 0; } return -ENOSYS;since if the driver is not current you should return so, and not -ENOSYS.
Also, why not just return what the driver returns? E.g. if the driver returns an error you should return it. The normal pattern used is:
struct cpu_ops *ops = cpu_get_ops(cpu);
if (!ops->is_current) return -ENOSYS;
ret = ops->is_current(cpu); if (ret) return log_ret(ret);
return 0;
Since the patch has been applied by Stefano, I'll create a follow up patch to address the comments you mentioned.
Thanks, Peng.
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.
Regards, Simon