| Message ID | 20250212-imx-lmm-cpu-v2-3-3aee005968c1@nxp.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | firmware: scmi/imx: Add i.MX95 LMM/CPU Protocol | expand |
On Wed, Feb 12, 2025 at 03:40:25PM +0800, Peng Fan (OSS) wrote: > From: Peng Fan <peng.fan@nxp.com> > > Add Logical Machine Management(LMM) protocol which is intended for boot, > shutdown, and reset of other logical machines (LM). It is usually used to > allow one LM to manager another used as an offload or accelerator engine. > Hi, > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > drivers/firmware/arm_scmi/vendors/imx/Kconfig | 11 + > drivers/firmware/arm_scmi/vendors/imx/Makefile | 1 + > drivers/firmware/arm_scmi/vendors/imx/imx-sm-lmm.c | 270 +++++++++++++++++++++ > include/linux/scmi_imx_protocol.h | 31 +++ > 4 files changed, 313 insertions(+) > > diff --git a/drivers/firmware/arm_scmi/vendors/imx/Kconfig b/drivers/firmware/arm_scmi/vendors/imx/Kconfig > index a01bf5e47301d2f93c9bfc7eebc77e083ea4ed75..1a936fc87d2350e2a21bccd45dfbeebfa3b90286 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_LMM_EXT > + tristate "i.MX SCMI LMM EXTENSION" > + depends on ARM_SCMI_PROTOCOL || (COMPILE_TEST && OF) > + default y if ARCH_MXC > + help > + This enables i.MX System Logical Machine Protocol to > + 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. > + > config IMX_SCMI_MISC_EXT > tristate "i.MX SCMI MISC EXTENSION" > depends on ARM_SCMI_PROTOCOL || (COMPILE_TEST && OF) > diff --git a/drivers/firmware/arm_scmi/vendors/imx/Makefile b/drivers/firmware/arm_scmi/vendors/imx/Makefile > index d3ee6d5449244a4f5cdf6abcf1845f312c512325..f39a99ccaf9af757475e8b112d224669444d7ddc 100644 > --- a/drivers/firmware/arm_scmi/vendors/imx/Makefile > +++ b/drivers/firmware/arm_scmi/vendors/imx/Makefile > @@ -1,3 +1,4 @@ > # SPDX-License-Identifier: GPL-2.0-only > obj-$(CONFIG_IMX_SCMI_BBM_EXT) += imx-sm-bbm.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-lmm.c b/drivers/firmware/arm_scmi/vendors/imx/imx-sm-lmm.c > new file mode 100644 > index 0000000000000000000000000000000000000000..4b9211df2329623fae0400039db91cb2b98cded1 > --- /dev/null > +++ b/drivers/firmware/arm_scmi/vendors/imx/imx-sm-lmm.c > @@ -0,0 +1,270 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * System control and Management Interface (SCMI) NXP LMM 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_lmm_protocol_cmd { > + SCMI_IMX_LMM_ATTRIBUTES = 0x3, > + SCMI_IMX_LMM_BOOT = 0x4, > + SCMI_IMX_LMM_RESET = 0x5, > + SCMI_IMX_LMM_SHUTDOWN = 0x6, > + SCMI_IMX_LMM_WAKE = 0x7, > + SCMI_IMX_LMM_SUSPEND = 0x8, > + SCMI_IMX_LMM_NOTIFY = 0x9, > + SCMI_IMX_LMM_RESET_REASON = 0xA, > + SCMI_IMX_LMM_POWER_ON = 0xB, > + SCMI_IMX_LMM_RESET_VECTOR_SET = 0xC, > +}; > + > +struct scmi_imx_lmm_priv { > + u32 nr_lmm; > +}; > + > +#define SCMI_IMX_LMM_PROTO_ATTR_NUM_LM(x) (((x) & 0xFFU)) > +struct scmi_msg_imx_lmm_protocol_attributes { > + __le32 attributes; > +}; > + > +struct scmi_msg_imx_lmm_attributes_out { > + __le32 lmid; > + __le32 attributes; > + __le32 state; > + __le32 errstatus; > + u8 name[LMM_MAX_NAME]; > +}; > + > +struct scmi_imx_lmm_reset_vector_set_in { > + __le32 lmid; > + __le32 cpuid; > +#define SCMI_LMM_VEC_FLAGS_TABLE BIT(0) > + __le32 flags; > + __le32 resetvectorlow; > + __le32 resetvectorhigh; > +}; > + > +struct scmi_imx_lmm_shutdown_in { > + __le32 lmid; > + __le32 flags; > +}; > + > +static int scmi_imx_lmm_validate_lmid(const struct scmi_protocol_handle *ph, u32 lmid) > +{ > + struct scmi_imx_lmm_priv *priv = ph->get_priv(ph); > + > + if (lmid >= priv->nr_lmm) > + return -EINVAL; > + > + return 0; > +} > + > +static int scmi_imx_lmm_attributes(const struct scmi_protocol_handle *ph, > + u32 lmid, struct scmi_imx_lmm_info *info) > +{ > + struct scmi_msg_imx_lmm_attributes_out *out; > + struct scmi_xfer *t; > + int ret; > + > + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_LMM_ATTRIBUTES, sizeof(u32), 0, &t); > + if (ret) > + return ret; > + > + put_unaligned_le32(lmid, t->tx.buf); > + ret = ph->xops->do_xfer(ph, t); > + if (!ret) { > + out = t->rx.buf; > + info->lmid = le32_to_cpu(out->lmid); > + info->state = le32_to_cpu(out->state); > + info->errstatus = le32_to_cpu(out->errstatus); > + strscpy(info->name, out->name); > + dev_dbg(ph->dev, "i.MX LMM: Logical Machine(%d), name: %s\n", > + info->lmid, out->name); > + } else { > + dev_err(ph->dev, "i.MX LMM: Failed to get info of Logical Machine(%u)\n", lmid); > + } > + > + ph->xops->xfer_put(ph, t); > + > + return ret; > +} > + > +static int scmi_imx_lmm_boot(const struct scmi_protocol_handle *ph, u32 lmid) > +{ > + struct scmi_xfer *t; > + int ret; > + > + ret = scmi_imx_lmm_validate_lmid(ph, lmid); > + if (ret) > + return ret; > + > + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_LMM_BOOT, sizeof(u32), > + 0, &t); > + if (ret) > + return ret; > + > + put_unaligned_le32(lmid, t->tx.buf); > + ret = ph->xops->do_xfer(ph, t); > + > + ph->xops->xfer_put(ph, t); > + > + return ret; > +} > + ..this.... > +static int scmi_imx_lmm_power_on(const struct scmi_protocol_handle *ph, u32 lmid) > +{ > + struct scmi_xfer *t; > + int ret; > + > + ret = scmi_imx_lmm_validate_lmid(ph, lmid); > + if (ret) > + return ret; > + > + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_LMM_POWER_ON, sizeof(u32), > + 0, &t); > + if (ret) > + return ret; > + > + put_unaligned_le32(lmid, t->tx.buf); > + ret = ph->xops->do_xfer(ph, t); > + > + ph->xops->xfer_put(ph, t); > + > + return ret; > +} ...can be refatcored to use one common workhorse fcuntion sued by both ops... > + > +static int scmi_imx_lmm_reset_vector_set(const struct scmi_protocol_handle *ph, > + u32 lmid, u32 cpuid, u32 flags, u64 vector) > +{ > + struct scmi_imx_lmm_reset_vector_set_in *in; > + struct scmi_xfer *t; > + int ret; > + > + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_LMM_RESET_VECTOR_SET, sizeof(*in), > + 0, &t); > + if (ret) > + return ret; > + > + in = t->tx.buf; > + in->lmid = cpu_to_le32(lmid); > + in->cpuid = cpu_to_le32(cpuid); > + in->flags = cpu_to_le32(flags); ...bitfields in a flag field must not be enianity converted...only eventually subfields representing numbers (liek you did above)... ..I feel FIELD_PREP should be fine...or even BIT(0) really given what these flags represents... ...again check issues with smatch.... > + 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_lmm_shutdown(const struct scmi_protocol_handle *ph, u32 lmid, > + u32 flags) > +{ > + struct scmi_imx_lmm_shutdown_in *in; > + struct scmi_xfer *t; > + int ret; > + > + ret = scmi_imx_lmm_validate_lmid(ph, lmid); > + if (ret) > + return ret; > + > + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_LMM_SHUTDOWN, sizeof(*in), > + 0, &t); > + if (ret) > + return ret; > + > + in = t->tx.buf; > + in->lmid = cpu_to_le32(lmid); > + in->flags = cpu_to_le32(flags); Same here, smatch would complain if you remove straight away this cpu_to_le32(), BUT this is a bitfield and does NOT contain any longer-than-2 number value that needs to be endianess manipulated...you just have to be able to set the required BIT 0 and 1, whitouth pissing off smatch :P > + ret = ph->xops->do_xfer(ph, t); > + > + ph->xops->xfer_put(ph, t); > + > + return ret; > +} > + > +static const struct scmi_imx_lmm_proto_ops scmi_imx_lmm_proto_ops = { > + .lmm_boot = scmi_imx_lmm_boot, > + .lmm_info = scmi_imx_lmm_attributes, > + .lmm_power_on = scmi_imx_lmm_power_on, > + .lmm_reset_vector_set = scmi_imx_lmm_reset_vector_set, > + .lmm_shutdown = scmi_imx_lmm_shutdown, > +}; > + > +static int scmi_imx_lmm_protocol_attributes_get(const struct scmi_protocol_handle *ph, > + struct scmi_imx_lmm_priv *priv) > +{ > + struct scmi_msg_imx_lmm_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) { > + priv->nr_lmm = SCMI_IMX_LMM_PROTO_ATTR_NUM_LM(le32_to_cpu(attr->attributes)); This seems wrong to me...you have an 8bit field to extract from an attribute field...so I would at first cut the byte out and then convert to LE, NOT the other way around like you are doing (i.e.: le32_to_cpu((attr->attributes & 0xFF))). Even BETTER to use: le32_get_bits((x), GENMASK(7, 0)) ...the thing is, being a single byte you really SHOULD NOT need to address any endianity concern (i.e. FIELD_GET(GENMASK(7, 0), (x))), BUT I fear that if you dont use some of the le32_ accessors smatch/sparse will complain since the message field is, correctly, declared as __le32. So le32_get_bits is the way to go (bit check with the static analyzer :P) > + dev_info(ph->dev, "i.MX LMM: %d Logical Machines\n", > + priv->nr_lmm); > + } > + > + ph->xops->xfer_put(ph, t); > + > + return ret; > +} > + > +static int scmi_imx_lmm_protocol_init(const struct scmi_protocol_handle *ph) > +{ > + struct scmi_imx_lmm_priv *info; > + u32 version; > + int ret; > + > + ret = ph->xops->version_get(ph, &version); > + if (ret) > + return ret; > + > + dev_info(ph->dev, "NXP SM LMM 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_lmm_protocol_attributes_get(ph, info); > + if (ret) > + return ret; > + > + return ph->set_priv(ph, info, version); > +} > + > +static const struct scmi_protocol scmi_imx_lmm = { > + .id = SCMI_PROTOCOL_IMX_LMM, > + .owner = THIS_MODULE, > + .instance_init = &scmi_imx_lmm_protocol_init, > + .ops = &scmi_imx_lmm_proto_ops, > + .supported_version = SCMI_PROTOCOL_SUPPORTED_VERSION, > + .vendor_id = SCMI_IMX_VENDOR, > + .sub_vendor_id = SCMI_IMX_SUBVENDOR, > +}; > +module_scmi_protocol(scmi_imx_lmm); > + Should this also be added: MODULE_ALIAS("scmi-protocol-" __stringify(SCMI_PROTOCOL_IMX_LMM) "-" SCMI_IMX_VENDOR); ...after this went in: commit d4cc8912cbff4990940b33cc61a9b09ddaee9704 Author: Cristian Marussi <cristian.marussi@arm.com> Date: Mon Dec 9 16:49:56 2024 +0000 firmware: arm_scmi: Add module aliases to i.MX vendor protocol ...I think is in v6.14-rc already to rebae on... > +MODULE_DESCRIPTION("i.MX SCMI LMM driver"); > +MODULE_LICENSE("GPL"); > diff --git a/include/linux/scmi_imx_protocol.h b/include/linux/scmi_imx_protocol.h > index 53b356a26414279f4aaaa8287c32209ed1ad57b4..07779c36ef9a126907e26e304a8feca16fd60ab2 100644 > --- a/include/linux/scmi_imx_protocol.h > +++ b/include/linux/scmi_imx_protocol.h > @@ -11,8 +11,10 @@ > #include <linux/bitfield.h> > #include <linux/device.h> > #include <linux/notifier.h> > +#include <linux/scmi_protocol.h> > #include <linux/types.h> > > +#define SCMI_PROTOCOL_IMX_LMM 0x80 > #define SCMI_PROTOCOL_IMX_BBM 0x81 > #define SCMI_PROTOCOL_IMX_MISC 0x84 > > @@ -57,4 +59,33 @@ struct scmi_imx_misc_proto_ops { > int (*misc_ctrl_req_notify)(const struct scmi_protocol_handle *ph, > u32 ctrl_id, u32 evt_id, u32 flags); > }; > + > +#define LMM_ID_DISCOVER 0xFFFFFFFFU What is this ? Documented anywhere ? > +#define LMM_MAX_NAME 16 > + > +enum scmi_imx_lmm_state { > + LMM_STATE_LM_OFF, > + LMM_STATE_LM_ON, > + LMM_STATE_LM_SUSPEND, > + LMM_STATE_LM_POWERED, > +}; > + > +struct scmi_imx_lmm_info { > + u32 lmid; > + enum scmi_imx_lmm_state state; > + u32 errstatus; > + u8 name[LMM_MAX_NAME]; > +}; > + > +struct scmi_imx_lmm_proto_ops { > + int (*lmm_boot)(const struct scmi_protocol_handle *ph, u32 lmid); > + int (*lmm_info)(const struct scmi_protocol_handle *ph, u32 lmid, > + struct scmi_imx_lmm_info *info); > + int (*lmm_power_on)(const struct scmi_protocol_handle *ph, u32 lmid); > + int (*lmm_reset_vector_set)(const struct scmi_protocol_handle *ph, > + u32 lmid, u32 cpuid, u32 flags, u64 vector); > + int (*lmm_shutdown)(const struct scmi_protocol_handle *ph, u32 lmid, > + u32 flags); > +}; > + > #endif > Thanks, Cristian
On Mon, Feb 24, 2025 at 03:54:19PM +0000, Cristian Marussi wrote: >On Wed, Feb 12, 2025 at 03:40:25PM +0800, Peng Fan (OSS) wrote: >> From: Peng Fan <peng.fan@nxp.com> >> >> Add Logical Machine Management(LMM) protocol which is intended for boot, >> shutdown, and reset of other logical machines (LM). It is usually used to >> allow one LM to manager another used as an offload or accelerator engine. >> > >Hi, > >> Signed-off-by: Peng Fan <peng.fan@nxp.com> >> --- >> drivers/firmware/arm_scmi/vendors/imx/Kconfig | 11 + >> drivers/firmware/arm_scmi/vendors/imx/Makefile | 1 + >> drivers/firmware/arm_scmi/vendors/imx/imx-sm-lmm.c | 270 +++++++++++++++++++++ >> include/linux/scmi_imx_protocol.h | 31 +++ >> 4 files changed, 313 insertions(+) >> >> diff --git a/drivers/firmware/arm_scmi/vendors/imx/Kconfig b/drivers/firmware/arm_scmi/vendors/imx/Kconfig >> index a01bf5e47301d2f93c9bfc7eebc77e083ea4ed75..1a936fc87d2350e2a21bccd45dfbeebfa3b90286 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_LMM_EXT >> + tristate "i.MX SCMI LMM EXTENSION" >> + depends on ARM_SCMI_PROTOCOL || (COMPILE_TEST && OF) >> + default y if ARCH_MXC >> + help >> + This enables i.MX System Logical Machine Protocol to >> + 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. >> + >> config IMX_SCMI_MISC_EXT >> tristate "i.MX SCMI MISC EXTENSION" >> depends on ARM_SCMI_PROTOCOL || (COMPILE_TEST && OF) >> diff --git a/drivers/firmware/arm_scmi/vendors/imx/Makefile b/drivers/firmware/arm_scmi/vendors/imx/Makefile >> index d3ee6d5449244a4f5cdf6abcf1845f312c512325..f39a99ccaf9af757475e8b112d224669444d7ddc 100644 >> --- a/drivers/firmware/arm_scmi/vendors/imx/Makefile >> +++ b/drivers/firmware/arm_scmi/vendors/imx/Makefile >> @@ -1,3 +1,4 @@ >> # SPDX-License-Identifier: GPL-2.0-only >> obj-$(CONFIG_IMX_SCMI_BBM_EXT) += imx-sm-bbm.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-lmm.c b/drivers/firmware/arm_scmi/vendors/imx/imx-sm-lmm.c >> new file mode 100644 >> index 0000000000000000000000000000000000000000..4b9211df2329623fae0400039db91cb2b98cded1 >> --- /dev/null >> +++ b/drivers/firmware/arm_scmi/vendors/imx/imx-sm-lmm.c >> @@ -0,0 +1,270 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * System control and Management Interface (SCMI) NXP LMM 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_lmm_protocol_cmd { >> + SCMI_IMX_LMM_ATTRIBUTES = 0x3, >> + SCMI_IMX_LMM_BOOT = 0x4, >> + SCMI_IMX_LMM_RESET = 0x5, >> + SCMI_IMX_LMM_SHUTDOWN = 0x6, >> + SCMI_IMX_LMM_WAKE = 0x7, >> + SCMI_IMX_LMM_SUSPEND = 0x8, >> + SCMI_IMX_LMM_NOTIFY = 0x9, >> + SCMI_IMX_LMM_RESET_REASON = 0xA, >> + SCMI_IMX_LMM_POWER_ON = 0xB, >> + SCMI_IMX_LMM_RESET_VECTOR_SET = 0xC, >> +}; >> + >> +struct scmi_imx_lmm_priv { >> + u32 nr_lmm; >> +}; >> + >> +#define SCMI_IMX_LMM_PROTO_ATTR_NUM_LM(x) (((x) & 0xFFU)) >> +struct scmi_msg_imx_lmm_protocol_attributes { >> + __le32 attributes; >> +}; >> + >> +struct scmi_msg_imx_lmm_attributes_out { >> + __le32 lmid; >> + __le32 attributes; >> + __le32 state; >> + __le32 errstatus; >> + u8 name[LMM_MAX_NAME]; >> +}; >> + >> +struct scmi_imx_lmm_reset_vector_set_in { >> + __le32 lmid; >> + __le32 cpuid; >> +#define SCMI_LMM_VEC_FLAGS_TABLE BIT(0) >> + __le32 flags; >> + __le32 resetvectorlow; >> + __le32 resetvectorhigh; >> +}; >> + >> +struct scmi_imx_lmm_shutdown_in { >> + __le32 lmid; >> + __le32 flags; >> +}; >> + >> +static int scmi_imx_lmm_validate_lmid(const struct scmi_protocol_handle *ph, u32 lmid) >> +{ >> + struct scmi_imx_lmm_priv *priv = ph->get_priv(ph); >> + >> + if (lmid >= priv->nr_lmm) >> + return -EINVAL; >> + >> + return 0; >> +} >> + >> +static int scmi_imx_lmm_attributes(const struct scmi_protocol_handle *ph, >> + u32 lmid, struct scmi_imx_lmm_info *info) >> +{ >> + struct scmi_msg_imx_lmm_attributes_out *out; >> + struct scmi_xfer *t; >> + int ret; >> + >> + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_LMM_ATTRIBUTES, sizeof(u32), 0, &t); >> + if (ret) >> + return ret; >> + >> + put_unaligned_le32(lmid, t->tx.buf); >> + ret = ph->xops->do_xfer(ph, t); >> + if (!ret) { >> + out = t->rx.buf; >> + info->lmid = le32_to_cpu(out->lmid); >> + info->state = le32_to_cpu(out->state); >> + info->errstatus = le32_to_cpu(out->errstatus); >> + strscpy(info->name, out->name); >> + dev_dbg(ph->dev, "i.MX LMM: Logical Machine(%d), name: %s\n", >> + info->lmid, out->name); >> + } else { >> + dev_err(ph->dev, "i.MX LMM: Failed to get info of Logical Machine(%u)\n", lmid); >> + } >> + >> + ph->xops->xfer_put(ph, t); >> + >> + return ret; >> +} >> + >> +static int scmi_imx_lmm_boot(const struct scmi_protocol_handle *ph, u32 lmid) >> +{ >> + struct scmi_xfer *t; ... >> + > >..this.... > >> +static int scmi_imx_lmm_power_on(const struct scmi_protocol_handle *ph, u32 lmid) >> +{ >> + ... >> + return ret; >> +} > >...can be refatcored to use one common workhorse fcuntion sued by both >ops... ok. I will merge power on and boot into one function. > >> + >> +static int scmi_imx_lmm_reset_vector_set(const struct scmi_protocol_handle *ph, >> + u32 lmid, u32 cpuid, u32 flags, u64 vector) >> +{ >> + struct scmi_imx_lmm_reset_vector_set_in *in; >> + struct scmi_xfer *t; >> + int ret; >> + >> + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_LMM_RESET_VECTOR_SET, sizeof(*in), >> + 0, &t); >> + if (ret) >> + return ret; >> + >> + in = t->tx.buf; >> + in->lmid = cpu_to_le32(lmid); >> + in->cpuid = cpu_to_le32(cpuid); >> + in->flags = cpu_to_le32(flags); > >...bitfields in a flag field must not be enianity converted...only >eventually subfields representing numbers (liek you did above)... > >..I feel FIELD_PREP should be fine...or even BIT(0) really given what >these flags represents... I will mark flags as reserved as updated doc, because this flag is not used as of now. > >...again check issues with smatch.... yes, I will try this. > >> + 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_lmm_shutdown(const struct scmi_protocol_handle *ph, u32 lmid, >> + u32 flags) >> +{ >> + struct scmi_imx_lmm_shutdown_in *in; >> + struct scmi_xfer *t; >> + int ret; >> + >> + ret = scmi_imx_lmm_validate_lmid(ph, lmid); >> + if (ret) >> + return ret; >> + >> + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_LMM_SHUTDOWN, sizeof(*in), >> + 0, &t); >> + if (ret) >> + return ret; >> + >> + in = t->tx.buf; >> + in->lmid = cpu_to_le32(lmid); >> + in->flags = cpu_to_le32(flags); > >Same here, smatch would complain if you remove straight away this >cpu_to_le32(), BUT this is a bitfield and does NOT contain any >longer-than-2 number value that needs to be endianess manipulated...you >just have to be able to set the required BIT 0 and 1, whitouth pissing >off smatch :P Only BIT0 used as of now. I will update as you suggest. > >> + ret = ph->xops->do_xfer(ph, t); >> + >> + ph->xops->xfer_put(ph, t); >> + >> + return ret; >> +} >> + >> +static const struct scmi_imx_lmm_proto_ops scmi_imx_lmm_proto_ops = { >> + .lmm_boot = scmi_imx_lmm_boot, >> + .lmm_info = scmi_imx_lmm_attributes, >> + .lmm_power_on = scmi_imx_lmm_power_on, >> + .lmm_reset_vector_set = scmi_imx_lmm_reset_vector_set, >> + .lmm_shutdown = scmi_imx_lmm_shutdown, >> +}; >> + >> +static int scmi_imx_lmm_protocol_attributes_get(const struct scmi_protocol_handle *ph, >> + struct scmi_imx_lmm_priv *priv) >> +{ >> + struct scmi_msg_imx_lmm_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) { >> + priv->nr_lmm = SCMI_IMX_LMM_PROTO_ATTR_NUM_LM(le32_to_cpu(attr->attributes)); > >This seems wrong to me...you have an 8bit field to extract from an >attribute field...so I would at first cut the byte out and then convert >to LE, NOT the other way around like you are doing >(i.e.: le32_to_cpu((attr->attributes & 0xFF))). > >Even BETTER to use: > > le32_get_bits((x), GENMASK(7, 0)) > >...the thing is, being a single byte you really SHOULD NOT need to address >any endianity concern (i.e. FIELD_GET(GENMASK(7, 0), (x))), BUT I fear that >if you dont use some of the le32_ accessors smatch/sparse will complain since >the message field is, correctly, declared as __le32. > >So le32_get_bits is the way to go (bit check with the static analyzer :P) Yup, fix in next version. > >> + dev_info(ph->dev, "i.MX LMM: %d Logical Machines\n", >> + priv->nr_lmm); >> + } >> + ... >> +}; >> +module_scmi_protocol(scmi_imx_lmm); >> + > > >Should this also be added: > >MODULE_ALIAS("scmi-protocol-" __stringify(SCMI_PROTOCOL_IMX_LMM) "-" SCMI_IMX_VENDOR); > >...after this went in: fix in next version. > >> }; .... >> + >> +#define LMM_ID_DISCOVER 0xFFFFFFFFU > >What is this ? Documented anywhere ? LMM_ATTRIBUTES if ID is 0xFFFFFFFF, it will return current lmid. I will update in doc patch. >
diff --git a/drivers/firmware/arm_scmi/vendors/imx/Kconfig b/drivers/firmware/arm_scmi/vendors/imx/Kconfig index a01bf5e47301d2f93c9bfc7eebc77e083ea4ed75..1a936fc87d2350e2a21bccd45dfbeebfa3b90286 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_LMM_EXT + tristate "i.MX SCMI LMM EXTENSION" + depends on ARM_SCMI_PROTOCOL || (COMPILE_TEST && OF) + default y if ARCH_MXC + help + This enables i.MX System Logical Machine Protocol to + 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. + config IMX_SCMI_MISC_EXT tristate "i.MX SCMI MISC EXTENSION" depends on ARM_SCMI_PROTOCOL || (COMPILE_TEST && OF) diff --git a/drivers/firmware/arm_scmi/vendors/imx/Makefile b/drivers/firmware/arm_scmi/vendors/imx/Makefile index d3ee6d5449244a4f5cdf6abcf1845f312c512325..f39a99ccaf9af757475e8b112d224669444d7ddc 100644 --- a/drivers/firmware/arm_scmi/vendors/imx/Makefile +++ b/drivers/firmware/arm_scmi/vendors/imx/Makefile @@ -1,3 +1,4 @@ # SPDX-License-Identifier: GPL-2.0-only obj-$(CONFIG_IMX_SCMI_BBM_EXT) += imx-sm-bbm.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-lmm.c b/drivers/firmware/arm_scmi/vendors/imx/imx-sm-lmm.c new file mode 100644 index 0000000000000000000000000000000000000000..4b9211df2329623fae0400039db91cb2b98cded1 --- /dev/null +++ b/drivers/firmware/arm_scmi/vendors/imx/imx-sm-lmm.c @@ -0,0 +1,270 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * System control and Management Interface (SCMI) NXP LMM 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_lmm_protocol_cmd { + SCMI_IMX_LMM_ATTRIBUTES = 0x3, + SCMI_IMX_LMM_BOOT = 0x4, + SCMI_IMX_LMM_RESET = 0x5, + SCMI_IMX_LMM_SHUTDOWN = 0x6, + SCMI_IMX_LMM_WAKE = 0x7, + SCMI_IMX_LMM_SUSPEND = 0x8, + SCMI_IMX_LMM_NOTIFY = 0x9, + SCMI_IMX_LMM_RESET_REASON = 0xA, + SCMI_IMX_LMM_POWER_ON = 0xB, + SCMI_IMX_LMM_RESET_VECTOR_SET = 0xC, +}; + +struct scmi_imx_lmm_priv { + u32 nr_lmm; +}; + +#define SCMI_IMX_LMM_PROTO_ATTR_NUM_LM(x) (((x) & 0xFFU)) +struct scmi_msg_imx_lmm_protocol_attributes { + __le32 attributes; +}; + +struct scmi_msg_imx_lmm_attributes_out { + __le32 lmid; + __le32 attributes; + __le32 state; + __le32 errstatus; + u8 name[LMM_MAX_NAME]; +}; + +struct scmi_imx_lmm_reset_vector_set_in { + __le32 lmid; + __le32 cpuid; +#define SCMI_LMM_VEC_FLAGS_TABLE BIT(0) + __le32 flags; + __le32 resetvectorlow; + __le32 resetvectorhigh; +}; + +struct scmi_imx_lmm_shutdown_in { + __le32 lmid; + __le32 flags; +}; + +static int scmi_imx_lmm_validate_lmid(const struct scmi_protocol_handle *ph, u32 lmid) +{ + struct scmi_imx_lmm_priv *priv = ph->get_priv(ph); + + if (lmid >= priv->nr_lmm) + return -EINVAL; + + return 0; +} + +static int scmi_imx_lmm_attributes(const struct scmi_protocol_handle *ph, + u32 lmid, struct scmi_imx_lmm_info *info) +{ + struct scmi_msg_imx_lmm_attributes_out *out; + struct scmi_xfer *t; + int ret; + + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_LMM_ATTRIBUTES, sizeof(u32), 0, &t); + if (ret) + return ret; + + put_unaligned_le32(lmid, t->tx.buf); + ret = ph->xops->do_xfer(ph, t); + if (!ret) { + out = t->rx.buf; + info->lmid = le32_to_cpu(out->lmid); + info->state = le32_to_cpu(out->state); + info->errstatus = le32_to_cpu(out->errstatus); + strscpy(info->name, out->name); + dev_dbg(ph->dev, "i.MX LMM: Logical Machine(%d), name: %s\n", + info->lmid, out->name); + } else { + dev_err(ph->dev, "i.MX LMM: Failed to get info of Logical Machine(%u)\n", lmid); + } + + ph->xops->xfer_put(ph, t); + + return ret; +} + +static int scmi_imx_lmm_boot(const struct scmi_protocol_handle *ph, u32 lmid) +{ + struct scmi_xfer *t; + int ret; + + ret = scmi_imx_lmm_validate_lmid(ph, lmid); + if (ret) + return ret; + + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_LMM_BOOT, sizeof(u32), + 0, &t); + if (ret) + return ret; + + put_unaligned_le32(lmid, t->tx.buf); + ret = ph->xops->do_xfer(ph, t); + + ph->xops->xfer_put(ph, t); + + return ret; +} + +static int scmi_imx_lmm_power_on(const struct scmi_protocol_handle *ph, u32 lmid) +{ + struct scmi_xfer *t; + int ret; + + ret = scmi_imx_lmm_validate_lmid(ph, lmid); + if (ret) + return ret; + + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_LMM_POWER_ON, sizeof(u32), + 0, &t); + if (ret) + return ret; + + put_unaligned_le32(lmid, t->tx.buf); + ret = ph->xops->do_xfer(ph, t); + + ph->xops->xfer_put(ph, t); + + return ret; +} + +static int scmi_imx_lmm_reset_vector_set(const struct scmi_protocol_handle *ph, + u32 lmid, u32 cpuid, u32 flags, u64 vector) +{ + struct scmi_imx_lmm_reset_vector_set_in *in; + struct scmi_xfer *t; + int ret; + + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_LMM_RESET_VECTOR_SET, sizeof(*in), + 0, &t); + if (ret) + return ret; + + in = t->tx.buf; + in->lmid = cpu_to_le32(lmid); + in->cpuid = cpu_to_le32(cpuid); + in->flags = cpu_to_le32(flags); + 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_lmm_shutdown(const struct scmi_protocol_handle *ph, u32 lmid, + u32 flags) +{ + struct scmi_imx_lmm_shutdown_in *in; + struct scmi_xfer *t; + int ret; + + ret = scmi_imx_lmm_validate_lmid(ph, lmid); + if (ret) + return ret; + + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_LMM_SHUTDOWN, sizeof(*in), + 0, &t); + if (ret) + return ret; + + in = t->tx.buf; + in->lmid = cpu_to_le32(lmid); + in->flags = cpu_to_le32(flags); + ret = ph->xops->do_xfer(ph, t); + + ph->xops->xfer_put(ph, t); + + return ret; +} + +static const struct scmi_imx_lmm_proto_ops scmi_imx_lmm_proto_ops = { + .lmm_boot = scmi_imx_lmm_boot, + .lmm_info = scmi_imx_lmm_attributes, + .lmm_power_on = scmi_imx_lmm_power_on, + .lmm_reset_vector_set = scmi_imx_lmm_reset_vector_set, + .lmm_shutdown = scmi_imx_lmm_shutdown, +}; + +static int scmi_imx_lmm_protocol_attributes_get(const struct scmi_protocol_handle *ph, + struct scmi_imx_lmm_priv *priv) +{ + struct scmi_msg_imx_lmm_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) { + priv->nr_lmm = SCMI_IMX_LMM_PROTO_ATTR_NUM_LM(le32_to_cpu(attr->attributes)); + dev_info(ph->dev, "i.MX LMM: %d Logical Machines\n", + priv->nr_lmm); + } + + ph->xops->xfer_put(ph, t); + + return ret; +} + +static int scmi_imx_lmm_protocol_init(const struct scmi_protocol_handle *ph) +{ + struct scmi_imx_lmm_priv *info; + u32 version; + int ret; + + ret = ph->xops->version_get(ph, &version); + if (ret) + return ret; + + dev_info(ph->dev, "NXP SM LMM 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_lmm_protocol_attributes_get(ph, info); + if (ret) + return ret; + + return ph->set_priv(ph, info, version); +} + +static const struct scmi_protocol scmi_imx_lmm = { + .id = SCMI_PROTOCOL_IMX_LMM, + .owner = THIS_MODULE, + .instance_init = &scmi_imx_lmm_protocol_init, + .ops = &scmi_imx_lmm_proto_ops, + .supported_version = SCMI_PROTOCOL_SUPPORTED_VERSION, + .vendor_id = SCMI_IMX_VENDOR, + .sub_vendor_id = SCMI_IMX_SUBVENDOR, +}; +module_scmi_protocol(scmi_imx_lmm); + +MODULE_DESCRIPTION("i.MX SCMI LMM driver"); +MODULE_LICENSE("GPL"); diff --git a/include/linux/scmi_imx_protocol.h b/include/linux/scmi_imx_protocol.h index 53b356a26414279f4aaaa8287c32209ed1ad57b4..07779c36ef9a126907e26e304a8feca16fd60ab2 100644 --- a/include/linux/scmi_imx_protocol.h +++ b/include/linux/scmi_imx_protocol.h @@ -11,8 +11,10 @@ #include <linux/bitfield.h> #include <linux/device.h> #include <linux/notifier.h> +#include <linux/scmi_protocol.h> #include <linux/types.h> +#define SCMI_PROTOCOL_IMX_LMM 0x80 #define SCMI_PROTOCOL_IMX_BBM 0x81 #define SCMI_PROTOCOL_IMX_MISC 0x84 @@ -57,4 +59,33 @@ struct scmi_imx_misc_proto_ops { int (*misc_ctrl_req_notify)(const struct scmi_protocol_handle *ph, u32 ctrl_id, u32 evt_id, u32 flags); }; + +#define LMM_ID_DISCOVER 0xFFFFFFFFU +#define LMM_MAX_NAME 16 + +enum scmi_imx_lmm_state { + LMM_STATE_LM_OFF, + LMM_STATE_LM_ON, + LMM_STATE_LM_SUSPEND, + LMM_STATE_LM_POWERED, +}; + +struct scmi_imx_lmm_info { + u32 lmid; + enum scmi_imx_lmm_state state; + u32 errstatus; + u8 name[LMM_MAX_NAME]; +}; + +struct scmi_imx_lmm_proto_ops { + int (*lmm_boot)(const struct scmi_protocol_handle *ph, u32 lmid); + int (*lmm_info)(const struct scmi_protocol_handle *ph, u32 lmid, + struct scmi_imx_lmm_info *info); + int (*lmm_power_on)(const struct scmi_protocol_handle *ph, u32 lmid); + int (*lmm_reset_vector_set)(const struct scmi_protocol_handle *ph, + u32 lmid, u32 cpuid, u32 flags, u64 vector); + int (*lmm_shutdown)(const struct scmi_protocol_handle *ph, u32 lmid, + u32 flags); +}; + #endif