From 92fe41caadfd01b6c31df12195b8a9b59ad4f4f5 Mon Sep 17 00:00:00 2001 From: Peng Fan Date: Sat, 27 Sep 2025 00:06:14 +0800 Subject: [PATCH 01/10] firmware: scmi: Typo fix Typo: 'to' -> 'too' Signed-off-by: Peng Fan --- drivers/firmware/scmi/smt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/firmware/scmi/smt.c b/drivers/firmware/scmi/smt.c index 3253f4211d6..a7721bbe54e 100644 --- a/drivers/firmware/scmi/smt.c +++ b/drivers/firmware/scmi/smt.c @@ -129,7 +129,7 @@ int scmi_read_resp_from_smt(struct udevice *dev, struct scmi_smt *smt, } if (hdr->length > msg->out_msg_sz + sizeof(hdr->msg_header)) { - dev_err(dev, "Buffer to small\n"); + dev_err(dev, "Buffer too small\n"); return -ETOOSMALL; } @@ -191,7 +191,7 @@ int scmi_msg_from_smt_msg(struct udevice *dev, struct scmi_smt *smt, struct scmi_smt_msg_header *hdr = (void *)smt->buf; if (buf_size > msg->out_msg_sz + sizeof(hdr->msg_header)) { - dev_err(dev, "Buffer to small\n"); + dev_err(dev, "Buffer too small\n"); return -ETOOSMALL; } From 8c48bae4f5783a1da95177e7d5d85c00e2ada0c3 Mon Sep 17 00:00:00 2001 From: Peng Fan Date: Sat, 27 Sep 2025 00:06:15 +0800 Subject: [PATCH 02/10] firmware: scmi: smt: Use io helpers It is not good practice to directly use "hdr->x" to read/write the hdr, because the SCMI buffer may not mapped as normal memory. Following Linux Kernel, using ioread32/iowrite32/memcpy_[from,to]io for smt header read, write. Signed-off-by: Peng Fan --- drivers/firmware/scmi/smt.c | 42 +++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/drivers/firmware/scmi/smt.c b/drivers/firmware/scmi/smt.c index a7721bbe54e..8bc721c6647 100644 --- a/drivers/firmware/scmi/smt.c +++ b/drivers/firmware/scmi/smt.c @@ -25,9 +25,9 @@ static void scmi_smt_enable_intr(struct scmi_smt *smt, bool enable) struct scmi_smt_header *hdr = (void *)smt->buf; if (enable) - hdr->flags |= SCMI_SHMEM_FLAG_INTR_ENABLED; + iowrite32(ioread32(&hdr->flags) | SCMI_SHMEM_FLAG_INTR_ENABLED, &hdr->flags); else - hdr->flags &= ~SCMI_SHMEM_FLAG_INTR_ENABLED; + iowrite32(ioread32(&hdr->flags) & ~SCMI_SHMEM_FLAG_INTR_ENABLED, &hdr->flags); } /** @@ -85,7 +85,7 @@ int scmi_write_msg_to_smt(struct udevice *dev, struct scmi_smt *smt, (!msg->out_msg && msg->out_msg_sz)) return -EINVAL; - if (!(hdr->channel_status & SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE)) { + if (!(ioread32(&hdr->channel_status) & SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE)) { dev_dbg(dev, "Channel busy\n"); return -EBUSY; } @@ -97,12 +97,13 @@ int scmi_write_msg_to_smt(struct udevice *dev, struct scmi_smt *smt, } /* Load message in shared memory */ - hdr->channel_status &= ~SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE; - hdr->length = msg->in_msg_sz + sizeof(hdr->msg_header); - hdr->msg_header = SMT_HEADER_TOKEN(0) | - SMT_HEADER_MESSAGE_TYPE(0) | - SMT_HEADER_PROTOCOL_ID(msg->protocol_id) | - SMT_HEADER_MESSAGE_ID(msg->message_id); + iowrite32(ioread32(&hdr->channel_status) & ~SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE, + &hdr->channel_status); + iowrite32(msg->in_msg_sz + sizeof(hdr->msg_header), &hdr->length); + iowrite32(SMT_HEADER_TOKEN(0) | + SMT_HEADER_MESSAGE_TYPE(0) | + SMT_HEADER_PROTOCOL_ID(msg->protocol_id) | + SMT_HEADER_MESSAGE_ID(msg->message_id), &hdr->msg_header); memcpy_toio(hdr->msg_payload, msg->in_msg, msg->in_msg_sz); @@ -118,23 +119,23 @@ int scmi_read_resp_from_smt(struct udevice *dev, struct scmi_smt *smt, { struct scmi_smt_header *hdr = (void *)smt->buf; - if (!(hdr->channel_status & SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE)) { + if (!(ioread32(&hdr->channel_status) & SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE)) { dev_err(dev, "Channel unexpectedly busy\n"); return -EBUSY; } - if (hdr->channel_status & SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR) { + if (ioread32(&hdr->channel_status) & SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR) { dev_err(dev, "Channel error reported, reset channel\n"); return -ECOMM; } - if (hdr->length > msg->out_msg_sz + sizeof(hdr->msg_header)) { + if (ioread32(&hdr->length) > msg->out_msg_sz + sizeof(hdr->msg_header)) { dev_err(dev, "Buffer too small\n"); return -ETOOSMALL; } /* Get the data */ - msg->out_msg_sz = hdr->length - sizeof(hdr->msg_header); + msg->out_msg_sz = ioread32(&hdr->length) - sizeof(hdr->msg_header); memcpy_fromio(msg->out_msg, hdr->msg_payload, msg->out_msg_sz); return 0; @@ -147,7 +148,8 @@ void scmi_clear_smt_channel(struct scmi_smt *smt) { struct scmi_smt_header *hdr = (void *)smt->buf; - hdr->channel_status &= ~SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR; + iowrite32(ioread32(&hdr->channel_status) & ~SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR, + &hdr->channel_status); } /** @@ -171,12 +173,12 @@ int scmi_msg_to_smt_msg(struct udevice *dev, struct scmi_smt *smt, *buf_size = msg->in_msg_sz + sizeof(hdr->msg_header); - hdr->msg_header = SMT_HEADER_TOKEN(0) | - SMT_HEADER_MESSAGE_TYPE(0) | - SMT_HEADER_PROTOCOL_ID(msg->protocol_id) | - SMT_HEADER_MESSAGE_ID(msg->message_id); + iowrite32(SMT_HEADER_TOKEN(0) | + SMT_HEADER_MESSAGE_TYPE(0) | + SMT_HEADER_PROTOCOL_ID(msg->protocol_id) | + SMT_HEADER_MESSAGE_ID(msg->message_id), &hdr->msg_header); - memcpy(hdr->msg_payload, msg->in_msg, msg->in_msg_sz); + memcpy_fromio(hdr->msg_payload, msg->in_msg, msg->in_msg_sz); return 0; } @@ -196,7 +198,7 @@ int scmi_msg_from_smt_msg(struct udevice *dev, struct scmi_smt *smt, } msg->out_msg_sz = buf_size - sizeof(hdr->msg_header); - memcpy(msg->out_msg, hdr->msg_payload, msg->out_msg_sz); + memcpy_toio(msg->out_msg, hdr->msg_payload, msg->out_msg_sz); return 0; } From 23e2b769220be4d3e35167492ef2cd915f647888 Mon Sep 17 00:00:00 2001 From: Peng Fan Date: Sat, 27 Sep 2025 00:06:16 +0800 Subject: [PATCH 03/10] firmware: scmi: smt: Dump more info "Buffer too small" is too vague, dump more info to make it easier to debug issues. Change dev_dbg to dev_err when buffer is too small. Signed-off-by: Peng Fan --- drivers/firmware/scmi/smt.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/firmware/scmi/smt.c b/drivers/firmware/scmi/smt.c index 8bc721c6647..5eb78387344 100644 --- a/drivers/firmware/scmi/smt.c +++ b/drivers/firmware/scmi/smt.c @@ -92,7 +92,9 @@ int scmi_write_msg_to_smt(struct udevice *dev, struct scmi_smt *smt, if (smt->size < (sizeof(*hdr) + msg->in_msg_sz) || smt->size < (sizeof(*hdr) + msg->out_msg_sz)) { - dev_dbg(dev, "Buffer too small\n"); + dev_err(dev, + "Buffer write too small: mst->size:%zu, in_msg_sz:%zu, out_msg_sz:%zu\n", + smt->size, msg->in_msg_sz, msg->out_msg_sz); return -ETOOSMALL; } @@ -130,7 +132,8 @@ int scmi_read_resp_from_smt(struct udevice *dev, struct scmi_smt *smt, } if (ioread32(&hdr->length) > msg->out_msg_sz + sizeof(hdr->msg_header)) { - dev_err(dev, "Buffer too small\n"); + dev_err(dev, "Buffer too small: hdr->length:%u, out_msg_sz:%zu\n", + ioread32(&hdr->length), msg->out_msg_sz); return -ETOOSMALL; } @@ -167,7 +170,8 @@ int scmi_msg_to_smt_msg(struct udevice *dev, struct scmi_smt *smt, if (smt->size < (sizeof(*hdr) + msg->in_msg_sz) || smt->size < (sizeof(*hdr) + msg->out_msg_sz)) { - dev_dbg(dev, "Buffer too small\n"); + dev_err(dev, "Buffer too small: mst->size:%zu, in_msg_sz:%zu, out_msg_sz:%zu\n", + smt->size, msg->in_msg_sz, msg->out_msg_sz); return -ETOOSMALL; } @@ -193,7 +197,8 @@ int scmi_msg_from_smt_msg(struct udevice *dev, struct scmi_smt *smt, struct scmi_smt_msg_header *hdr = (void *)smt->buf; if (buf_size > msg->out_msg_sz + sizeof(hdr->msg_header)) { - dev_err(dev, "Buffer too small\n"); + dev_err(dev, "Buffer too small: buf_size:%zu, out_msg_sz:%zu\n", + buf_size, msg->out_msg_sz); return -ETOOSMALL; } From eb7469eb1a32a02d024da254d34912c3fe741c7f Mon Sep 17 00:00:00 2001 From: Peng Fan Date: Sat, 27 Sep 2025 00:06:17 +0800 Subject: [PATCH 04/10] firmware: scmi: Add error code IN_USE In SCMI spec 3.2, there is an update: Add IN_USE error code for usage with Pin control protocol So add the error decoding for IN_USE. Signed-off-by: Peng Fan --- drivers/firmware/scmi/scmi_agent-uclass.c | 1 + include/scmi_protocols.h | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/firmware/scmi/scmi_agent-uclass.c b/drivers/firmware/scmi/scmi_agent-uclass.c index e7ec2c108e6..69a277e8786 100644 --- a/drivers/firmware/scmi/scmi_agent-uclass.c +++ b/drivers/firmware/scmi/scmi_agent-uclass.c @@ -35,6 +35,7 @@ static const struct error_code scmi_linux_errmap[] = { { .scmi = SCMI_GENERIC_ERROR, .errno = -EIO, }, { .scmi = SCMI_HARDWARE_ERROR, .errno = -EREMOTEIO, }, { .scmi = SCMI_PROTOCOL_ERROR, .errno = -EPROTO, }, + { .scmi = SCMI_IN_USE, .errno = -EADDRINUSE, }, }; /** diff --git a/include/scmi_protocols.h b/include/scmi_protocols.h index 762a1032c37..95e0c3cce3b 100644 --- a/include/scmi_protocols.h +++ b/include/scmi_protocols.h @@ -40,6 +40,7 @@ enum scmi_status_code { SCMI_GENERIC_ERROR = -8, SCMI_HARDWARE_ERROR = -9, SCMI_PROTOCOL_ERROR = -10, + SCMI_IN_USE = -11, }; /* From b2ae10970d40a26e955d6f763cda77948d8b4f7e Mon Sep 17 00:00:00 2001 From: Peng Fan Date: Sat, 27 Sep 2025 00:06:18 +0800 Subject: [PATCH 05/10] firmware: scmi: use PAGE_SIZE alignment for ARM64 For ARMv7, the alignment could be SECTION size. But for ARM64, use PAGE_SIZE. Signed-off-by: Peng Fan --- drivers/firmware/scmi/smt.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/firmware/scmi/smt.c b/drivers/firmware/scmi/smt.c index 5eb78387344..237871559f0 100644 --- a/drivers/firmware/scmi/smt.c +++ b/drivers/firmware/scmi/smt.c @@ -62,11 +62,17 @@ int scmi_dt_get_smt_buffer(struct udevice *dev, struct scmi_smt *smt) scmi_smt_enable_intr(smt, true); #ifdef CONFIG_ARM - if (dcache_status()) - mmu_set_region_dcache_behaviour(ALIGN_DOWN((uintptr_t)smt->buf, MMU_SECTION_SIZE), - ALIGN(smt->size, MMU_SECTION_SIZE), - DCACHE_OFF); + if (dcache_status()) { + u32 align_size; + if (IS_ENABLED(CONFIG_ARM64)) + align_size = PAGE_SIZE; + else + align_size = MMU_SECTION_SIZE; + + mmu_set_region_dcache_behaviour(ALIGN_DOWN((uintptr_t)smt->buf, align_size), + ALIGN(smt->size, align_size), DCACHE_OFF); + } #endif return 0; From f116ec5b9105ffa4792bfced12aab822e492ff5f Mon Sep 17 00:00:00 2001 From: Peng Fan Date: Sat, 27 Sep 2025 00:06:19 +0800 Subject: [PATCH 06/10] firmware: scmi: mailbox: Update timeout to 30ms Following Linux Kernel drivers/firmware/arm_scmi/transports/mailbox.c to set the default timeout to 30ms. Signed-off-by: Peng Fan --- drivers/firmware/scmi/mailbox_agent.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/firmware/scmi/mailbox_agent.c b/drivers/firmware/scmi/mailbox_agent.c index 6d4497f4b92..32b0fd9f589 100644 --- a/drivers/firmware/scmi/mailbox_agent.c +++ b/drivers/firmware/scmi/mailbox_agent.c @@ -16,7 +16,7 @@ #include "smt.h" -#define TIMEOUT_US_10MS 10000 +#define TIMEOUT_US_30MS 30000 /** * struct scmi_mbox_channel - Description of an SCMI mailbox transport @@ -87,7 +87,7 @@ static int setup_channel(struct udevice *dev, struct scmi_mbox_channel *chan) return ret; } - chan->timeout_us = TIMEOUT_US_10MS; + chan->timeout_us = TIMEOUT_US_30MS; return 0; } From 5e9fb0e5835ace9f9f7c6eceb3dd4cabc17487d4 Mon Sep 17 00:00:00 2001 From: Peng Fan Date: Sat, 27 Sep 2025 00:06:20 +0800 Subject: [PATCH 07/10] firmware: scmi: mailbox: Support arm,max_rx_timeout_ms Per devicetree bindings: arm,max-rx-timeout-ms indicates an optional time value, expressed in milliseconds, representing the transport maximum timeout value for the receive channel. The value should be a non-zero value if set. Support this property if platform set it to a non-default value. This property is a per SCMI property, so all channels share same value. Signed-off-by: Peng Fan Reviewed-by: Tom Rini --- drivers/firmware/scmi/mailbox_agent.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/firmware/scmi/mailbox_agent.c b/drivers/firmware/scmi/mailbox_agent.c index 32b0fd9f589..16a82f55ab7 100644 --- a/drivers/firmware/scmi/mailbox_agent.c +++ b/drivers/firmware/scmi/mailbox_agent.c @@ -16,7 +16,7 @@ #include "smt.h" -#define TIMEOUT_US_30MS 30000 +#define TIMEOUT_30MS 30 /** * struct scmi_mbox_channel - Description of an SCMI mailbox transport @@ -73,6 +73,7 @@ out: static int setup_channel(struct udevice *dev, struct scmi_mbox_channel *chan) { + struct scmi_mbox_channel *base_chan = dev_get_plat(dev); int ret; ret = mbox_get_by_index(dev, 0, &chan->mbox); @@ -87,7 +88,7 @@ static int setup_channel(struct udevice *dev, struct scmi_mbox_channel *chan) return ret; } - chan->timeout_us = TIMEOUT_US_30MS; + chan->timeout_us = base_chan->timeout_us; return 0; } @@ -127,6 +128,9 @@ int scmi_mbox_of_to_plat(struct udevice *dev) { struct scmi_mbox_channel *chan = dev_get_plat(dev); + chan->timeout_us = dev_read_u32_default(dev, "arm,max-rx-timeout-ms", + TIMEOUT_30MS) * 1000; + return setup_channel(dev, chan); } From 0d71bc3a1bf82ad60fa4fb1b4441acde633249f1 Mon Sep 17 00:00:00 2001 From: Peng Fan Date: Sat, 27 Sep 2025 00:06:21 +0800 Subject: [PATCH 08/10] clk: scmi: Replace log_debug with dev_dbg Use dev_dbg to dump device name, dev_dbg also a encapsulation call to log() and print(). So it is ok to use dev_dbg. Signed-off-by: Peng Fan --- drivers/clk/clk_scmi.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c index 0c9a81cabcc..a7d89f32cd7 100644 --- a/drivers/clk/clk_scmi.c +++ b/drivers/clk/clk_scmi.c @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -41,19 +42,21 @@ static int scmi_clk_get_permissions(struct udevice *dev, int clkid, u32 *perm) }; if (priv->version < CLOCK_PROTOCOL_VERSION_3_0) { - log_debug("%s: SCMI clock management protocol version is less than 3.0.\n", __func__); + dev_dbg(dev, + "%s: SCMI clock management protocol version is less than 3.0.\n", __func__); return -EINVAL; } ret = devm_scmi_process_msg(dev, &msg); if (ret) { - log_debug("%s: get SCMI clock management protocol permissions failed\n", __func__); + dev_dbg(dev, + "%s: get SCMI clock management protocol permissions failed\n", __func__); return ret; } ret = scmi_to_linux_errno(out.status); if (ret < 0) { - log_debug("%s: the status code of getting permissions: %d\n", __func__, ret); + dev_dbg(dev, "%s: the status code of getting permissions: %d\n", __func__, ret); return ret; } @@ -167,7 +170,7 @@ static int scmi_clk_enable(struct clk *clk) return scmi_clk_gate(clk, 1); /* Following Linux drivers/clk/clk-scmi.c, directly return 0 if agent has no permission. */ - log_debug("%s: SCMI CLOCK: the clock cannot be enabled by the agent.\n", __func__); + dev_dbg(clk->dev, "%s: SCMI CLOCK: the clock cannot be enabled by the agent.\n", __func__); return 0; } @@ -190,7 +193,8 @@ static int scmi_clk_disable(struct clk *clk) return scmi_clk_gate(clk, 0); /* Following Linux drivers/clk/clk-scmi.c, directly return 0 if agent has no permission. */ - log_debug("%s: SCMI CLOCK: the clock cannot be disabled by the agent.\n", __func__); + dev_dbg(clk->dev, + "%s: SCMI CLOCK: the clock cannot be disabled by the agent.\n", __func__); return 0; } @@ -260,7 +264,8 @@ static ulong scmi_clk_set_rate(struct clk *clk, ulong rate) return __scmi_clk_set_rate(clk, rate); /* Following Linux drivers/clk/clk-scmi.c, directly return 0 if agent has no permission. */ - log_debug("%s: SCMI CLOCK: the clock rate cannot be changed by the agent.\n", __func__); + dev_dbg(clk->dev, + "%s: SCMI CLOCK: the clock rate cannot be changed by the agent.\n", __func__); return 0; } @@ -291,7 +296,7 @@ static int scmi_clk_probe(struct udevice *dev) ret = scmi_generic_protocol_version(dev, SCMI_PROTOCOL_ID_CLOCK, &priv->version); if (ret) { - log_debug("%s: get SCMI clock management protocol version failed\n", __func__); + dev_dbg(dev, "%s: get SCMI clock management protocol version failed\n", __func__); return ret; } @@ -371,7 +376,8 @@ static int scmi_clk_set_parent(struct clk *clk, struct clk *parent) return __scmi_clk_set_parent(clk, parent); /* Following Linux drivers/clk/clk-scmi.c, directly return 0 if agent has no permission. */ - log_debug("%s: SCMI CLOCK: the clock's parent cannot be changed by the agent.\n", __func__); + dev_dbg(clk->dev, + "%s: SCMI CLOCK: the clock's parent cannot be changed by the agent.\n", __func__); return 0; } From 5ea37da298777ce6fe53b51bd32e3e0614c61bbc Mon Sep 17 00:00:00 2001 From: Peng Fan Date: Sat, 27 Sep 2025 00:06:22 +0800 Subject: [PATCH 09/10] cmd: scmi: Add pin control protocol name Pin control protocol name was not added into 'protocol_name' array, however Pin control was supported on i.MX95. So add it. Signed-off-by: Peng Fan --- cmd/scmi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/scmi.c b/cmd/scmi.c index cfbca63e164..d0498b816aa 100644 --- a/cmd/scmi.c +++ b/cmd/scmi.c @@ -29,6 +29,7 @@ struct { {SCMI_PROTOCOL_ID_SENSOR, "Sensor management"}, {SCMI_PROTOCOL_ID_RESET_DOMAIN, "Reset domain management"}, {SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN, "Voltage domain management"}, + {SCMI_PROTOCOL_ID_PINCTRL, "Pin control management"}, }; /** From 7828597f5430431281071b60289e46afed74ef94 Mon Sep 17 00:00:00 2001 From: Peng Fan Date: Sat, 27 Sep 2025 00:06:23 +0800 Subject: [PATCH 10/10] MAINTAINERS: Add an entry for SCMI Add an entry for SCMI and add myself as maintainer Signed-off-by: Peng Fan --- MAINTAINERS | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index b030242a2d8..b6654396218 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1618,6 +1618,13 @@ F: drivers/*/*sandbox*.c F: include/dt-bindings/*/sandbox*.h F: include/os.h +SCMI +M: Peng Fan +S: Maintained +F: drivers/firmware/scmi/ +F: include/scmi* +N: scmi + SEAMA M: Linus Walleij S: Maintained