| Message ID | 20250121-imx-lmm-cpu-v1-2-0eab7e073e4e@nxp.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | firmware: scmi/imx: Add i.MX95 LMM/CPU Protocol | expand |
On Tue, Jan 21, 2025 at 11:08:12PM +0800, Peng Fan (OSS) wrote: > From: Peng Fan <peng.fan@nxp.com> > > This protocol allows an agent to start, stop a CPU or set reset vector. It > is used to manage auxiliary CPUs in an LM (e.g. additional cores in an AP > cluster). > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > drivers/firmware/arm_scmi/vendors/imx/Kconfig | 13 +- > drivers/firmware/arm_scmi/vendors/imx/Makefile | 1 + > drivers/firmware/arm_scmi/vendors/imx/imx-sm-cpu.c | 283 +++++++++++++++++++++ > include/linux/scmi_imx_protocol.h | 10 + > 4 files changed, 306 insertions(+), 1 deletion(-) > > diff --git a/drivers/firmware/arm_scmi/vendors/imx/Kconfig b/drivers/firmware/arm_scmi/vendors/imx/Kconfig > index 1a936fc87d2350e2a21bccd45dfbeebfa3b90286..9070522510e4d3f3d7276a7581f8676006d20f90 100644 > --- a/drivers/firmware/arm_scmi/vendors/imx/Kconfig > +++ b/drivers/firmware/arm_scmi/vendors/imx/Kconfig > @@ -12,6 +12,17 @@ config IMX_SCMI_BBM_EXT > To compile this driver as a module, choose M here: the > module will be called imx-sm-bbm. > > +config IMX_SCMI_CPU_EXT > + tristate "i.MX SCMI CPU EXTENSION" > + depends on ARM_SCMI_PROTOCOL || (COMPILE_TEST && OF) > + default y if ARCH_MXC > + help > + This enables i.MX System CPU Protocol to manage cpu > + start, stop and etc. > + > + To compile this driver as a module, choose M here: the > + module will be called imx-sm-cpu. > + > config IMX_SCMI_LMM_EXT > tristate "i.MX SCMI LMM EXTENSION" > depends on ARM_SCMI_PROTOCOL || (COMPILE_TEST && OF) > @@ -21,7 +32,7 @@ config IMX_SCMI_LMM_EXT > manage Logical Machines boot, shutdown and etc. > > To compile this driver as a module, choose M here: the > - module will be called imx-sm-lmm. > + module will be called imx-sm-cpu. > It's supposed to be called imx-sm-lmm. > config IMX_SCMI_MISC_EXT > tristate "i.MX SCMI MISC EXTENSION" > diff --git a/drivers/firmware/arm_scmi/vendors/imx/Makefile b/drivers/firmware/arm_scmi/vendors/imx/Makefile > index f39a99ccaf9af757475e8b112d224669444d7ddc..e3a5ea46345c89da1afae25e55698044672b7c28 100644 > --- a/drivers/firmware/arm_scmi/vendors/imx/Makefile > +++ b/drivers/firmware/arm_scmi/vendors/imx/Makefile > @@ -1,4 +1,5 @@ > # SPDX-License-Identifier: GPL-2.0-only > obj-$(CONFIG_IMX_SCMI_BBM_EXT) += imx-sm-bbm.o > +obj-$(CONFIG_IMX_SCMI_CPU_EXT) += imx-sm-cpu.o > obj-$(CONFIG_IMX_SCMI_LMM_EXT) += imx-sm-lmm.o > obj-$(CONFIG_IMX_SCMI_MISC_EXT) += imx-sm-misc.o > diff --git a/drivers/firmware/arm_scmi/vendors/imx/imx-sm-cpu.c b/drivers/firmware/arm_scmi/vendors/imx/imx-sm-cpu.c > new file mode 100644 > index 0000000000000000000000000000000000000000..e3f294c2cb69a5b5a916d55984f4a63539937d02 > --- /dev/null > +++ b/drivers/firmware/arm_scmi/vendors/imx/imx-sm-cpu.c > @@ -0,0 +1,283 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * System control and Management Interface (SCMI) NXP CPU Protocol > + * > + * Copyright 2025 NXP > + */ > + > +#include <linux/bits.h> > +#include <linux/io.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > +#include <linux/scmi_protocol.h> > +#include <linux/scmi_imx_protocol.h> > + > +#include "../../protocols.h" > +#include "../../notify.h" > + > +#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x10000 > + > +enum scmi_imx_cpu_protocol_cmd { > + SCMI_IMX_CPU_ATTRIBUTES = 0x3, > + SCMI_IMX_CPU_START = 0x4, > + SCMI_IMX_CPU_STOP = 0x5, > + SCMI_IMX_CPU_RESET_VECTOR_SET = 0x6, > + SCMI_IMX_CPU_INFO_GET = 0xC, > +}; > + > +struct scmi_imx_cpu_info { > + u32 nr_cpu; > +}; > + > +#define SCMI_IMX_CPU_PROTO_ATTR_NUM_CPUS(x) ((x) & 0xFFFF) > +struct scmi_msg_imx_cpu_protocol_attributes { > + __le32 attributes; > +}; > + > +struct scmi_msg_imx_cpu_attributes_out { > + __le32 attributes; > +#define CPU_MAX_NAME 16 > + u8 name[CPU_MAX_NAME]; char is always unsigned in the kernel these days but strings should still always be char. Same thing in patch 1, there were a couple u8 names. > +}; > + > +struct scmi_imx_cpu_reset_vector_set_in { > + __le32 cpuid; > +#define CPU_VEC_FLAGS_RESUME BIT(31) > +#define CPU_VEC_FLAGS_START BIT(30) > +#define CPU_VEC_FLAGS_BOOT BIT(29) > + __le32 flags; > + __le32 resetvectorlow; > + __le32 resetvectorhigh; > +}; > + > +struct scmi_imx_cpu_info_get_out { > +#define CPU_RUN_MODE_START 0 > +#define CPU_RUN_MODE_HOLD 1 > +#define CPU_RUN_MODE_STOP 2 > +#define CPU_RUN_MODE_SLEEP 3 > + __le32 runmode; > + __le32 sleepmode; > + __le32 resetvectorlow; > + __le32 resetvectorhigh; > +}; > + > +static int scmi_imx_cpu_validate_cpuid(const struct scmi_protocol_handle *ph, > + u32 cpuid) > +{ > + struct scmi_imx_cpu_info *info = ph->get_priv(ph); > + > + if (cpuid >= info->nr_cpu) > + return -EINVAL; > + > + return 0; > +} > + > +static int scmi_imx_cpu_start(const struct scmi_protocol_handle *ph, u32 cpuid) > +{ > + struct scmi_xfer *t; > + int ret; > + > + ret = scmi_imx_cpu_validate_cpuid(ph, cpuid); > + if (ret) > + return ret; > + > + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_CPU_START, sizeof(u32), > + 0, &t); > + if (ret) > + return ret; > + > + put_unaligned_le32(cpuid, t->tx.buf); > + ret = ph->xops->do_xfer(ph, t); > + > + ph->xops->xfer_put(ph, t); > + > + return ret; > +} > + > +static int scmi_imx_cpu_stop(const struct scmi_protocol_handle *ph, u32 cpuid) > +{ > + struct scmi_xfer *t; > + int ret; > + > + ret = scmi_imx_cpu_validate_cpuid(ph, cpuid); > + if (ret) > + return ret; > + > + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_CPU_STOP, sizeof(u32), > + 0, &t); > + if (ret) > + return ret; > + > + put_unaligned_le32(cpuid, t->tx.buf); > + ret = ph->xops->do_xfer(ph, t); > + > + ph->xops->xfer_put(ph, t); > + > + return ret; > +} > + > +static int scmi_imx_cpu_reset_vector_set(const struct scmi_protocol_handle *ph, > + u32 cpuid, u64 vector, bool start, > + bool boot, bool resume) > +{ > + struct scmi_imx_cpu_reset_vector_set_in *in; > + struct scmi_xfer *t; > + int ret; > + > + ret = scmi_imx_cpu_validate_cpuid(ph, cpuid); > + if (ret) > + return ret; > + > + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_CPU_RESET_VECTOR_SET, sizeof(*in), > + 0, &t); > + if (ret) > + return ret; > + > + in = t->tx.buf; > + in->cpuid = cpu_to_le32(cpuid); > + in->flags = start ? CPU_VEC_FLAGS_START : 0; > + in->flags |= boot ? CPU_VEC_FLAGS_BOOT : 0; > + in->flags |= resume ? CPU_VEC_FLAGS_BOOT : 0; s/CPU_VEC_FLAGS_BOOT/CPU_VEC_FLAGS_RESUME/ > + in->resetvectorlow = cpu_to_le32(lower_32_bits(vector)); > + in->resetvectorhigh = cpu_to_le32(upper_32_bits(vector)); > + ret = ph->xops->do_xfer(ph, t); > + > + ph->xops->xfer_put(ph, t); > + > + return ret; > +} > + > +static int scmi_imx_cpu_started(const struct scmi_protocol_handle *ph, u32 cpuid, > + bool *started) > +{ > + struct scmi_imx_cpu_info_get_out *out; > + struct scmi_xfer *t; > + int ret; > + > + *started = false; > + ret = scmi_imx_cpu_validate_cpuid(ph, cpuid); > + if (ret) > + return ret; > + > + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_CPU_INFO_GET, sizeof(u32), > + 0, &t); > + if (ret) > + return ret; > + > + put_unaligned_le32(cpuid, t->tx.buf); > + ret = ph->xops->do_xfer(ph, t); > + if (!ret) { > + out = t->rx.buf; > + if ((out->runmode == CPU_RUN_MODE_START) || > + (out->runmode == CPU_RUN_MODE_SLEEP)) > + *started = true; > + } > + > + ph->xops->xfer_put(ph, t); > + > + return ret; > +} > + > +static const struct scmi_imx_cpu_proto_ops scmi_imx_cpu_proto_ops = { > + .cpu_reset_vector_set = scmi_imx_cpu_reset_vector_set, > + .cpu_start = scmi_imx_cpu_start, > + .cpu_started = scmi_imx_cpu_started, > + .cpu_stop = scmi_imx_cpu_stop, > +}; > + > +static int scmi_imx_cpu_protocol_attributes_get(const struct scmi_protocol_handle *ph, > + struct scmi_imx_cpu_info *info) > +{ > + struct scmi_msg_imx_cpu_protocol_attributes *attr; > + struct scmi_xfer *t; > + int ret; > + > + ret = ph->xops->xfer_get_init(ph, PROTOCOL_ATTRIBUTES, 0, > + sizeof(*attr), &t); > + if (ret) > + return ret; > + > + attr = t->rx.buf; > + > + ret = ph->xops->do_xfer(ph, t); > + if (!ret) { > + info->nr_cpu = SCMI_IMX_CPU_PROTO_ATTR_NUM_CPUS(attr->attributes); > + dev_info(ph->dev, "i.MX SM CPU: %d cpus\n", > + info->nr_cpu); This can fit in 80 characters: dev_info(ph->dev, "i.MX SM CPU: %d cpus\n", info->nr_cpu); > + } > + > + ph->xops->xfer_put(ph, t); > + > + return ret; > +} > + > +static int scmi_imx_cpu_attributes_get(const struct scmi_protocol_handle *ph, > + u32 cpuid, struct scmi_imx_cpu_info *info) No need to pass the "info" pointer. > +{ > + struct scmi_msg_imx_cpu_attributes_out *out; > + struct scmi_xfer *t; > + int ret; > + > + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_CPU_ATTRIBUTES, sizeof(u32), 0, &t); > + if (ret) > + return ret; > + > + put_unaligned_le32(cpuid, t->tx.buf); > + ret = ph->xops->do_xfer(ph, t); > + if (!ret) { > + out = t->rx.buf; > + dev_info(ph->dev, "i.MX CPU: name: %s\n", out->name); > + } else { > + dev_err(ph->dev, "i.MX cpu: Failed to get info of cpu(%u)\n", cpuid); > + } > + > + ph->xops->xfer_put(ph, t); > + > + return ret; > +} > + > + Multiple blank lines (checkpatch.pl --strict). > +static int scmi_imx_cpu_protocol_init(const struct scmi_protocol_handle *ph) > +{ regards, dan carpenter
On Wed, Jan 22, 2025 at 11:48:52AM +0300, Dan Carpenter wrote: > On Tue, Jan 21, 2025 at 11:08:12PM +0800, Peng Fan (OSS) wrote: > > From: Peng Fan <peng.fan@nxp.com> > > > > This protocol allows an agent to start, stop a CPU or set reset vector. It > > is used to manage auxiliary CPUs in an LM (e.g. additional cores in an AP > > cluster). > > Hi, just a quick one down below. > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > drivers/firmware/arm_scmi/vendors/imx/Kconfig | 13 +- > > drivers/firmware/arm_scmi/vendors/imx/Makefile | 1 + > > drivers/firmware/arm_scmi/vendors/imx/imx-sm-cpu.c | 283 +++++++++++++++++++++ > > include/linux/scmi_imx_protocol.h | 10 + > > 4 files changed, 306 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/firmware/arm_scmi/vendors/imx/Kconfig b/drivers/firmware/arm_scmi/vendors/imx/Kconfig > > index 1a936fc87d2350e2a21bccd45dfbeebfa3b90286..9070522510e4d3f3d7276a7581f8676006d20f90 100644 > > --- a/drivers/firmware/arm_scmi/vendors/imx/Kconfig > > +++ b/drivers/firmware/arm_scmi/vendors/imx/Kconfig > > @@ -12,6 +12,17 @@ config IMX_SCMI_BBM_EXT > > To compile this driver as a module, choose M here: the > > module will be called imx-sm-bbm. > > > > +config IMX_SCMI_CPU_EXT > > + tristate "i.MX SCMI CPU EXTENSION" > > + depends on ARM_SCMI_PROTOCOL || (COMPILE_TEST && OF) > > + default y if ARCH_MXC > > + help > > + This enables i.MX System CPU Protocol to manage cpu > > + start, stop and etc. > > + > > + To compile this driver as a module, choose M here: the > > + module will be called imx-sm-cpu. > > + > > config IMX_SCMI_LMM_EXT > > tristate "i.MX SCMI LMM EXTENSION" > > depends on ARM_SCMI_PROTOCOL || (COMPILE_TEST && OF) > > @@ -21,7 +32,7 @@ config IMX_SCMI_LMM_EXT > > manage Logical Machines boot, shutdown and etc. > > > > To compile this driver as a module, choose M here: the > > - module will be called imx-sm-lmm. > > + module will be called imx-sm-cpu. > > > > It's supposed to be called imx-sm-lmm. > > > config IMX_SCMI_MISC_EXT > > tristate "i.MX SCMI MISC EXTENSION" > > diff --git a/drivers/firmware/arm_scmi/vendors/imx/Makefile b/drivers/firmware/arm_scmi/vendors/imx/Makefile > > index f39a99ccaf9af757475e8b112d224669444d7ddc..e3a5ea46345c89da1afae25e55698044672b7c28 100644 > > --- a/drivers/firmware/arm_scmi/vendors/imx/Makefile > > +++ b/drivers/firmware/arm_scmi/vendors/imx/Makefile > > @@ -1,4 +1,5 @@ > > # SPDX-License-Identifier: GPL-2.0-only > > obj-$(CONFIG_IMX_SCMI_BBM_EXT) += imx-sm-bbm.o > > +obj-$(CONFIG_IMX_SCMI_CPU_EXT) += imx-sm-cpu.o > > obj-$(CONFIG_IMX_SCMI_LMM_EXT) += imx-sm-lmm.o > > obj-$(CONFIG_IMX_SCMI_MISC_EXT) += imx-sm-misc.o > > diff --git a/drivers/firmware/arm_scmi/vendors/imx/imx-sm-cpu.c b/drivers/firmware/arm_scmi/vendors/imx/imx-sm-cpu.c > > new file mode 100644 > > index 0000000000000000000000000000000000000000..e3f294c2cb69a5b5a916d55984f4a63539937d02 > > --- /dev/null > > +++ b/drivers/firmware/arm_scmi/vendors/imx/imx-sm-cpu.c > > @@ -0,0 +1,283 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * System control and Management Interface (SCMI) NXP CPU Protocol > > + * > > + * Copyright 2025 NXP > > + */ > > + > > +#include <linux/bits.h> > > +#include <linux/io.h> > > +#include <linux/module.h> > > +#include <linux/of.h> > > +#include <linux/platform_device.h> > > +#include <linux/scmi_protocol.h> > > +#include <linux/scmi_imx_protocol.h> > > + > > +#include "../../protocols.h" > > +#include "../../notify.h" > > + > > +#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x10000 > > + > > +enum scmi_imx_cpu_protocol_cmd { > > + SCMI_IMX_CPU_ATTRIBUTES = 0x3, > > + SCMI_IMX_CPU_START = 0x4, > > + SCMI_IMX_CPU_STOP = 0x5, > > + SCMI_IMX_CPU_RESET_VECTOR_SET = 0x6, > > + SCMI_IMX_CPU_INFO_GET = 0xC, > > +}; > > + > > +struct scmi_imx_cpu_info { > > + u32 nr_cpu; > > +}; > > + > > +#define SCMI_IMX_CPU_PROTO_ATTR_NUM_CPUS(x) ((x) & 0xFFFF) > > +struct scmi_msg_imx_cpu_protocol_attributes { > > + __le32 attributes; > > +}; > > + > > +struct scmi_msg_imx_cpu_attributes_out { > > + __le32 attributes; > > +#define CPU_MAX_NAME 16 > > + u8 name[CPU_MAX_NAME]; > > char is always unsigned in the kernel these days but strings should > still always be char. Same thing in patch 1, there were a couple u8 > names. > While it is certainly true that char is the way to go for strings and, as such, it is used elsewhere to hold the resource names across all SCMI protocols, in this context it is a field of structure representing exactly the layout of message reply coming from the server, and defined in the SCMI spec as a uint8 array, so, we have generally preferred to used u8 to represent such fixed size array all across the SCMI stack protocols implementation.... .... not saying that it is necessarily completelt right, but that is the reason we are guilty :D Thanks, Cristian
On Wed, Jan 22, 2025 at 12:22:18PM +0000, Cristian Marussi wrote: > > > +struct scmi_msg_imx_cpu_attributes_out { > > > + __le32 attributes; > > > +#define CPU_MAX_NAME 16 > > > + u8 name[CPU_MAX_NAME]; > > > > char is always unsigned in the kernel these days but strings should > > still always be char. Same thing in patch 1, there were a couple u8 > > names. > > > > While it is certainly true that char is the way to go for strings and, as > such, it is used elsewhere to hold the resource names across all SCMI > protocols, in this context it is a field of structure representing > exactly the layout of message reply coming from the server, and defined > in the SCMI spec as a uint8 array, so, we have generally preferred to > used u8 to represent such fixed size array all across the SCMI stack > protocols implementation.... > > .... not saying that it is necessarily completelt right, but that is the > reason we are guilty :D Fine. I don't have intense emotions about this. It does slightly bother me when we assume that the SCMI server NUL terminates these when we do things like: dev_info(ph->dev, "i.MX CPU: name: %s\n", out->name); But from a practical perspective we have to trust the SCMI server. regards, dan carpenter
On Wed, Jan 22, 2025 at 03:41:41PM +0300, Dan Carpenter wrote: > On Wed, Jan 22, 2025 at 12:22:18PM +0000, Cristian Marussi wrote: > > > > +struct scmi_msg_imx_cpu_attributes_out { > > > > + __le32 attributes; > > > > +#define CPU_MAX_NAME 16 > > > > + u8 name[CPU_MAX_NAME]; > > > > > > char is always unsigned in the kernel these days but strings should > > > still always be char. Same thing in patch 1, there were a couple u8 > > > names. > > > > > Hi Dan, > > While it is certainly true that char is the way to go for strings and, as > > such, it is used elsewhere to hold the resource names across all SCMI > > protocols, in this context it is a field of structure representing > > exactly the layout of message reply coming from the server, and defined > > in the SCMI spec as a uint8 array, so, we have generally preferred to > > used u8 to represent such fixed size array all across the SCMI stack > > protocols implementation.... > > > > .... not saying that it is necessarily completelt right, but that is the > > reason we are guilty :D > > Fine. I don't have intense emotions about this. > > It does slightly bother me when we assume that the SCMI server NUL > terminates these when we do things like: > > dev_info(ph->dev, "i.MX CPU: name: %s\n", out->name); > Hang on...I have not really done a proper review still on this series... ...and this printout above straight out of the message payload seems very wrong to me too.. > But from a practical perspective we have to trust the SCMI server. > ....nope we should NEVER trust the server...and instead assume it can kill us (kernel) all the time :P ...despite what the spec says, we tend to assume tha the server can be maliciously wrong (or just crappy), so in other protocols where we do used an u8[] to describe the resource name field in a message, we have also always (hopefully :D) taken care to use it ONLY after having processed that field like... strscpy(dom_info->name, attr->name, SCMI_SHORT_NAME_MAX_SIZE); ...to remove any possible bad outcome from a misbehaving SCMI fw server. Thanks, Cristian
On Wed, Jan 22, 2025 at 03:41:41PM +0300, Dan Carpenter wrote: > On Wed, Jan 22, 2025 at 12:22:18PM +0000, Cristian Marussi wrote: > > > > +struct scmi_msg_imx_cpu_attributes_out { > > > > + __le32 attributes; > > > > +#define CPU_MAX_NAME 16 > > > > + u8 name[CPU_MAX_NAME]; > > > > > > char is always unsigned in the kernel these days but strings should > > > still always be char. Same thing in patch 1, there were a couple u8 > > > names. > > > > > > > While it is certainly true that char is the way to go for strings and, as > > such, it is used elsewhere to hold the resource names across all SCMI > > protocols, in this context it is a field of structure representing > > exactly the layout of message reply coming from the server, and defined > > in the SCMI spec as a uint8 array, so, we have generally preferred to > > used u8 to represent such fixed size array all across the SCMI stack > > protocols implementation.... > > > > .... not saying that it is necessarily completelt right, but that is the > > reason we are guilty :D > > Fine. I don't have intense emotions about this. > > It does slightly bother me when we assume that the SCMI server NUL > terminates these when we do things like: > > dev_info(ph->dev, "i.MX CPU: name: %s\n", out->name); > I was about to reply earlier and got distracted. This makes it even better. I agree with Cristian that we have defined it as u8 when referring to the payload from the SCMI platform/server. But it should be used only to extract information from the response payload and copied to OSPM's own buffer before accessing it like the above dev_info example. In short, I agree with you Dan that it shouldn't be used in OS like above. I haven't looked at the implementation patches(just looked at the doc 3/5), but if they need such logging, they need to copy it to some buffer in the kernel. -- Regards, Sudeep
Hi Peng, kernel test robot noticed the following build warnings: [auto build test WARNING on 0907e7fb35756464aa34c35d6abb02998418164b] url: https://github.com/intel-lab-lkp/linux/commits/Peng-Fan-OSS/firmware-arm_scmi-imx-Add-i-MX95-LMM-protocol/20250121-231254 base: 0907e7fb35756464aa34c35d6abb02998418164b patch link: https://lore.kernel.org/r/20250121-imx-lmm-cpu-v1-2-0eab7e073e4e%40nxp.com patch subject: [PATCH 2/5] firmware: arm_scmi: imx: Add i.MX95 CPU Protocol config: arm64-randconfig-r122-20250124 (https://download.01.org/0day-ci/archive/20250124/202501241148.0CI9mBP5-lkp@intel.com/config) compiler: aarch64-linux-gcc (GCC) 14.2.0 reproduce: (https://download.01.org/0day-ci/archive/20250124/202501241148.0CI9mBP5-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202501241148.0CI9mBP5-lkp@intel.com/ sparse warnings: (new ones prefixed by >>) >> drivers/firmware/arm_scmi/vendors/imx/imx-sm-cpu.c:139:19: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __le32 [usertype] flags @@ got unsigned long @@ drivers/firmware/arm_scmi/vendors/imx/imx-sm-cpu.c:139:19: sparse: expected restricted __le32 [usertype] flags drivers/firmware/arm_scmi/vendors/imx/imx-sm-cpu.c:139:19: sparse: got unsigned long >> drivers/firmware/arm_scmi/vendors/imx/imx-sm-cpu.c:140:19: sparse: sparse: invalid assignment: |= drivers/firmware/arm_scmi/vendors/imx/imx-sm-cpu.c:140:19: sparse: left side has type restricted __le32 drivers/firmware/arm_scmi/vendors/imx/imx-sm-cpu.c:140:19: sparse: right side has type unsigned long drivers/firmware/arm_scmi/vendors/imx/imx-sm-cpu.c:141:19: sparse: sparse: invalid assignment: |= drivers/firmware/arm_scmi/vendors/imx/imx-sm-cpu.c:141:19: sparse: left side has type restricted __le32 drivers/firmware/arm_scmi/vendors/imx/imx-sm-cpu.c:141:19: sparse: right side has type unsigned long >> drivers/firmware/arm_scmi/vendors/imx/imx-sm-cpu.c:173:25: sparse: sparse: restricted __le32 degrades to integer drivers/firmware/arm_scmi/vendors/imx/imx-sm-cpu.c:205:32: sparse: sparse: restricted __le32 degrades to integer vim +139 drivers/firmware/arm_scmi/vendors/imx/imx-sm-cpu.c 119 120 static int scmi_imx_cpu_reset_vector_set(const struct scmi_protocol_handle *ph, 121 u32 cpuid, u64 vector, bool start, 122 bool boot, bool resume) 123 { 124 struct scmi_imx_cpu_reset_vector_set_in *in; 125 struct scmi_xfer *t; 126 int ret; 127 128 ret = scmi_imx_cpu_validate_cpuid(ph, cpuid); 129 if (ret) 130 return ret; 131 132 ret = ph->xops->xfer_get_init(ph, SCMI_IMX_CPU_RESET_VECTOR_SET, sizeof(*in), 133 0, &t); 134 if (ret) 135 return ret; 136 137 in = t->tx.buf; 138 in->cpuid = cpu_to_le32(cpuid); > 139 in->flags = start ? CPU_VEC_FLAGS_START : 0; > 140 in->flags |= boot ? CPU_VEC_FLAGS_BOOT : 0; 141 in->flags |= resume ? CPU_VEC_FLAGS_BOOT : 0; 142 in->resetvectorlow = cpu_to_le32(lower_32_bits(vector)); 143 in->resetvectorhigh = cpu_to_le32(upper_32_bits(vector)); 144 ret = ph->xops->do_xfer(ph, t); 145 146 ph->xops->xfer_put(ph, t); 147 148 return ret; 149 } 150 151 static int scmi_imx_cpu_started(const struct scmi_protocol_handle *ph, u32 cpuid, 152 bool *started) 153 { 154 struct scmi_imx_cpu_info_get_out *out; 155 struct scmi_xfer *t; 156 int ret; 157 158 *started = false; 159 ret = scmi_imx_cpu_validate_cpuid(ph, cpuid); 160 if (ret) 161 return ret; 162 163 ret = ph->xops->xfer_get_init(ph, SCMI_IMX_CPU_INFO_GET, sizeof(u32), 164 0, &t); 165 if (ret) 166 return ret; 167 168 put_unaligned_le32(cpuid, t->tx.buf); 169 ret = ph->xops->do_xfer(ph, t); 170 if (!ret) { 171 out = t->rx.buf; 172 if ((out->runmode == CPU_RUN_MODE_START) || > 173 (out->runmode == CPU_RUN_MODE_SLEEP)) 174 *started = true; 175 } 176 177 ph->xops->xfer_put(ph, t); 178 179 return ret; 180 } 181
diff --git a/drivers/firmware/arm_scmi/vendors/imx/Kconfig b/drivers/firmware/arm_scmi/vendors/imx/Kconfig index 1a936fc87d2350e2a21bccd45dfbeebfa3b90286..9070522510e4d3f3d7276a7581f8676006d20f90 100644 --- a/drivers/firmware/arm_scmi/vendors/imx/Kconfig +++ b/drivers/firmware/arm_scmi/vendors/imx/Kconfig @@ -12,6 +12,17 @@ config IMX_SCMI_BBM_EXT To compile this driver as a module, choose M here: the module will be called imx-sm-bbm. +config IMX_SCMI_CPU_EXT + tristate "i.MX SCMI CPU EXTENSION" + depends on ARM_SCMI_PROTOCOL || (COMPILE_TEST && OF) + default y if ARCH_MXC + help + This enables i.MX System CPU Protocol to manage cpu + start, stop and etc. + + To compile this driver as a module, choose M here: the + module will be called imx-sm-cpu. + config IMX_SCMI_LMM_EXT tristate "i.MX SCMI LMM EXTENSION" depends on ARM_SCMI_PROTOCOL || (COMPILE_TEST && OF) @@ -21,7 +32,7 @@ config IMX_SCMI_LMM_EXT manage Logical Machines boot, shutdown and etc. To compile this driver as a module, choose M here: the - module will be called imx-sm-lmm. + module will be called imx-sm-cpu. config IMX_SCMI_MISC_EXT tristate "i.MX SCMI MISC EXTENSION" diff --git a/drivers/firmware/arm_scmi/vendors/imx/Makefile b/drivers/firmware/arm_scmi/vendors/imx/Makefile index f39a99ccaf9af757475e8b112d224669444d7ddc..e3a5ea46345c89da1afae25e55698044672b7c28 100644 --- a/drivers/firmware/arm_scmi/vendors/imx/Makefile +++ b/drivers/firmware/arm_scmi/vendors/imx/Makefile @@ -1,4 +1,5 @@ # SPDX-License-Identifier: GPL-2.0-only obj-$(CONFIG_IMX_SCMI_BBM_EXT) += imx-sm-bbm.o +obj-$(CONFIG_IMX_SCMI_CPU_EXT) += imx-sm-cpu.o obj-$(CONFIG_IMX_SCMI_LMM_EXT) += imx-sm-lmm.o obj-$(CONFIG_IMX_SCMI_MISC_EXT) += imx-sm-misc.o diff --git a/drivers/firmware/arm_scmi/vendors/imx/imx-sm-cpu.c b/drivers/firmware/arm_scmi/vendors/imx/imx-sm-cpu.c new file mode 100644 index 0000000000000000000000000000000000000000..e3f294c2cb69a5b5a916d55984f4a63539937d02 --- /dev/null +++ b/drivers/firmware/arm_scmi/vendors/imx/imx-sm-cpu.c @@ -0,0 +1,283 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * System control and Management Interface (SCMI) NXP CPU Protocol + * + * Copyright 2025 NXP + */ + +#include <linux/bits.h> +#include <linux/io.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/scmi_protocol.h> +#include <linux/scmi_imx_protocol.h> + +#include "../../protocols.h" +#include "../../notify.h" + +#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x10000 + +enum scmi_imx_cpu_protocol_cmd { + SCMI_IMX_CPU_ATTRIBUTES = 0x3, + SCMI_IMX_CPU_START = 0x4, + SCMI_IMX_CPU_STOP = 0x5, + SCMI_IMX_CPU_RESET_VECTOR_SET = 0x6, + SCMI_IMX_CPU_INFO_GET = 0xC, +}; + +struct scmi_imx_cpu_info { + u32 nr_cpu; +}; + +#define SCMI_IMX_CPU_PROTO_ATTR_NUM_CPUS(x) ((x) & 0xFFFF) +struct scmi_msg_imx_cpu_protocol_attributes { + __le32 attributes; +}; + +struct scmi_msg_imx_cpu_attributes_out { + __le32 attributes; +#define CPU_MAX_NAME 16 + u8 name[CPU_MAX_NAME]; +}; + +struct scmi_imx_cpu_reset_vector_set_in { + __le32 cpuid; +#define CPU_VEC_FLAGS_RESUME BIT(31) +#define CPU_VEC_FLAGS_START BIT(30) +#define CPU_VEC_FLAGS_BOOT BIT(29) + __le32 flags; + __le32 resetvectorlow; + __le32 resetvectorhigh; +}; + +struct scmi_imx_cpu_info_get_out { +#define CPU_RUN_MODE_START 0 +#define CPU_RUN_MODE_HOLD 1 +#define CPU_RUN_MODE_STOP 2 +#define CPU_RUN_MODE_SLEEP 3 + __le32 runmode; + __le32 sleepmode; + __le32 resetvectorlow; + __le32 resetvectorhigh; +}; + +static int scmi_imx_cpu_validate_cpuid(const struct scmi_protocol_handle *ph, + u32 cpuid) +{ + struct scmi_imx_cpu_info *info = ph->get_priv(ph); + + if (cpuid >= info->nr_cpu) + return -EINVAL; + + return 0; +} + +static int scmi_imx_cpu_start(const struct scmi_protocol_handle *ph, u32 cpuid) +{ + struct scmi_xfer *t; + int ret; + + ret = scmi_imx_cpu_validate_cpuid(ph, cpuid); + if (ret) + return ret; + + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_CPU_START, sizeof(u32), + 0, &t); + if (ret) + return ret; + + put_unaligned_le32(cpuid, t->tx.buf); + ret = ph->xops->do_xfer(ph, t); + + ph->xops->xfer_put(ph, t); + + return ret; +} + +static int scmi_imx_cpu_stop(const struct scmi_protocol_handle *ph, u32 cpuid) +{ + struct scmi_xfer *t; + int ret; + + ret = scmi_imx_cpu_validate_cpuid(ph, cpuid); + if (ret) + return ret; + + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_CPU_STOP, sizeof(u32), + 0, &t); + if (ret) + return ret; + + put_unaligned_le32(cpuid, t->tx.buf); + ret = ph->xops->do_xfer(ph, t); + + ph->xops->xfer_put(ph, t); + + return ret; +} + +static int scmi_imx_cpu_reset_vector_set(const struct scmi_protocol_handle *ph, + u32 cpuid, u64 vector, bool start, + bool boot, bool resume) +{ + struct scmi_imx_cpu_reset_vector_set_in *in; + struct scmi_xfer *t; + int ret; + + ret = scmi_imx_cpu_validate_cpuid(ph, cpuid); + if (ret) + return ret; + + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_CPU_RESET_VECTOR_SET, sizeof(*in), + 0, &t); + if (ret) + return ret; + + in = t->tx.buf; + in->cpuid = cpu_to_le32(cpuid); + in->flags = start ? CPU_VEC_FLAGS_START : 0; + in->flags |= boot ? CPU_VEC_FLAGS_BOOT : 0; + in->flags |= resume ? CPU_VEC_FLAGS_BOOT : 0; + in->resetvectorlow = cpu_to_le32(lower_32_bits(vector)); + in->resetvectorhigh = cpu_to_le32(upper_32_bits(vector)); + ret = ph->xops->do_xfer(ph, t); + + ph->xops->xfer_put(ph, t); + + return ret; +} + +static int scmi_imx_cpu_started(const struct scmi_protocol_handle *ph, u32 cpuid, + bool *started) +{ + struct scmi_imx_cpu_info_get_out *out; + struct scmi_xfer *t; + int ret; + + *started = false; + ret = scmi_imx_cpu_validate_cpuid(ph, cpuid); + if (ret) + return ret; + + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_CPU_INFO_GET, sizeof(u32), + 0, &t); + if (ret) + return ret; + + put_unaligned_le32(cpuid, t->tx.buf); + ret = ph->xops->do_xfer(ph, t); + if (!ret) { + out = t->rx.buf; + if ((out->runmode == CPU_RUN_MODE_START) || + (out->runmode == CPU_RUN_MODE_SLEEP)) + *started = true; + } + + ph->xops->xfer_put(ph, t); + + return ret; +} + +static const struct scmi_imx_cpu_proto_ops scmi_imx_cpu_proto_ops = { + .cpu_reset_vector_set = scmi_imx_cpu_reset_vector_set, + .cpu_start = scmi_imx_cpu_start, + .cpu_started = scmi_imx_cpu_started, + .cpu_stop = scmi_imx_cpu_stop, +}; + +static int scmi_imx_cpu_protocol_attributes_get(const struct scmi_protocol_handle *ph, + struct scmi_imx_cpu_info *info) +{ + struct scmi_msg_imx_cpu_protocol_attributes *attr; + struct scmi_xfer *t; + int ret; + + ret = ph->xops->xfer_get_init(ph, PROTOCOL_ATTRIBUTES, 0, + sizeof(*attr), &t); + if (ret) + return ret; + + attr = t->rx.buf; + + ret = ph->xops->do_xfer(ph, t); + if (!ret) { + info->nr_cpu = SCMI_IMX_CPU_PROTO_ATTR_NUM_CPUS(attr->attributes); + dev_info(ph->dev, "i.MX SM CPU: %d cpus\n", + info->nr_cpu); + } + + ph->xops->xfer_put(ph, t); + + return ret; +} + +static int scmi_imx_cpu_attributes_get(const struct scmi_protocol_handle *ph, + u32 cpuid, struct scmi_imx_cpu_info *info) +{ + struct scmi_msg_imx_cpu_attributes_out *out; + struct scmi_xfer *t; + int ret; + + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_CPU_ATTRIBUTES, sizeof(u32), 0, &t); + if (ret) + return ret; + + put_unaligned_le32(cpuid, t->tx.buf); + ret = ph->xops->do_xfer(ph, t); + if (!ret) { + out = t->rx.buf; + dev_info(ph->dev, "i.MX CPU: name: %s\n", out->name); + } else { + dev_err(ph->dev, "i.MX cpu: Failed to get info of cpu(%u)\n", cpuid); + } + + ph->xops->xfer_put(ph, t); + + return ret; +} + + +static int scmi_imx_cpu_protocol_init(const struct scmi_protocol_handle *ph) +{ + struct scmi_imx_cpu_info *info; + u32 version; + int ret, i; + + ret = ph->xops->version_get(ph, &version); + if (ret) + return ret; + + dev_info(ph->dev, "NXP SM CPU Protocol Version %d.%d\n", + PROTOCOL_REV_MAJOR(version), PROTOCOL_REV_MINOR(version)); + + info = devm_kzalloc(ph->dev, sizeof(*info), GFP_KERNEL); + if (!info) + return -ENOMEM; + + ret = scmi_imx_cpu_protocol_attributes_get(ph, info); + if (ret) + return ret; + + for (i = 0; i < info->nr_cpu; i++) { + ret = scmi_imx_cpu_attributes_get(ph, i, info); + if (ret) + return ret; + } + + return ph->set_priv(ph, info, version); +} + +static const struct scmi_protocol scmi_imx_cpu = { + .id = SCMI_PROTOCOL_IMX_CPU, + .owner = THIS_MODULE, + .instance_init = &scmi_imx_cpu_protocol_init, + .ops = &scmi_imx_cpu_proto_ops, + .supported_version = SCMI_PROTOCOL_SUPPORTED_VERSION, + .vendor_id = SCMI_IMX_VENDOR, + .sub_vendor_id = SCMI_IMX_SUBVENDOR, +}; +module_scmi_protocol(scmi_imx_cpu); + +MODULE_DESCRIPTION("i.MX SCMI CPU driver"); +MODULE_LICENSE("GPL"); diff --git a/include/linux/scmi_imx_protocol.h b/include/linux/scmi_imx_protocol.h index 456559f021696ae51f40ed11e6f85089d430ce71..6521199cdab67c7e751e850def221d89898cd0f2 100644 --- a/include/linux/scmi_imx_protocol.h +++ b/include/linux/scmi_imx_protocol.h @@ -16,6 +16,7 @@ #define SCMI_PROTOCOL_IMX_LMM 0x80 #define SCMI_PROTOCOL_IMX_BBM 0x81 +#define SCMI_PROTOCOL_IMX_CPU 0x82 #define SCMI_PROTOCOL_IMX_MISC 0x84 #define SCMI_IMX_VENDOR "NXP" @@ -86,4 +87,13 @@ struct scmi_imx_lmm_proto_ops { u32 flags); }; +struct scmi_imx_cpu_proto_ops { + int (*cpu_reset_vector_set)(const struct scmi_protocol_handle *ph, + u32 cpuid, u64 vector, bool start, + bool boot, bool resume); + int (*cpu_start)(const struct scmi_protocol_handle *ph, u32 cpuid); + int (*cpu_started)(const struct scmi_protocol_handle *ph, u32 cpuid, + bool *started); + int (*cpu_stop)(const struct scmi_protocol_handle *ph, u32 cpuid); +}; #endif