[PATCH v2 8/9] firmware: arm_scmi: transport: Add ACPI PCC transport
Adam Young
admiyo at amperemail.onmicrosoft.com
Thu May 28 10:29:52 PDT 2026
On 5/25/26 16:42, Sudeep Holla wrote:
> Introduce a new SCMI transport that uses ACPI PCCT (PCC) subspaces via
> the Linux PCC mailbox layer. The driver parses ACPI _DSD data to map
> protocols to PCC subspace UIDs, supports shared TX/RX channels, and
> optionally sets up a P2A channel for notifications.
>
> Key points:
> - new CONFIG_ARM_SCMI_TRANSPORT_PCC option
> - integration with SCMI core via scmi_desc and transport ops
> - response/notification fetch from PCC shared memory header/payload
> - ACPI device matching and registration via the ACPI transport macro
>
> This enables SCMI to be exercised over PCC on ACPI platforms.
>
> Signed-off-by: Sudeep Holla <sudeep.holla at kernel.org>
> ---
> drivers/firmware/arm_scmi/common.h | 11 +
> drivers/firmware/arm_scmi/transports/Kconfig | 12 +
> drivers/firmware/arm_scmi/transports/Makefile | 2 +
> drivers/firmware/arm_scmi/transports/pcc.c | 593 ++++++++++++++++++++++++++
> include/linux/scmi_protocol.h | 1 +
> 5 files changed, 619 insertions(+)
>
> diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
> index 347e0da0d9cc..e4b2e1d3e7e7 100644
> --- a/drivers/firmware/arm_scmi/common.h
> +++ b/drivers/firmware/arm_scmi/common.h
> @@ -465,6 +465,17 @@ struct scmi_transport_core_operations {
> const struct scmi_message_operations *msg;
> };
>
> +struct scmi_dsd_info {
> + u32 protocol_id;
> + const char *const property_name;
> +};
> +
> +static struct scmi_dsd_info scmi_dsd_info_list[] = {
> + { SCMI_PROTOCOL_BASE, "arm-arml0001-transport-pcc"},
> + { SCMI_PROTOCOL_POWERCAP, "arm-arml0001-protocol-pcap"},
> + { SCMI_PROTOCOL_TELEMETRY, "arm-arml0001-protocol-telemetry"},
> +};
> +
> /**
> * struct scmi_transport - A structure representing a configured transport
> *
> diff --git a/drivers/firmware/arm_scmi/transports/Kconfig b/drivers/firmware/arm_scmi/transports/Kconfig
> index 57eccf316e26..4936078f279f 100644
> --- a/drivers/firmware/arm_scmi/transports/Kconfig
> +++ b/drivers/firmware/arm_scmi/transports/Kconfig
> @@ -77,6 +77,18 @@ config ARM_SCMI_TRANSPORT_OPTEE
> This driver can also be built as a module. If so, the module
> will be called scmi_transport_optee.
>
> +config ARM_SCMI_TRANSPORT_PCC
> + tristate "SCMI transport based on ACPI PCC"
> + depends on PCC
> + select ARM_SCMI_HAVE_TRANSPORT
> + help
> + Enable ACPI PCC mailbox based transport for SCMI.
> +
> + If you want the ARM SCMI PROTOCOL stack to include support for a
> + transport based on mailboxes, answer Y.
> + This driver can also be built as a module. If so, the module
> + will be called scmi_transport_pcc.
> +
> config ARM_SCMI_TRANSPORT_VIRTIO
> tristate "SCMI transport based on VirtIO"
> depends on VIRTIO
> diff --git a/drivers/firmware/arm_scmi/transports/Makefile b/drivers/firmware/arm_scmi/transports/Makefile
> index 3ba3d3bee151..e7c4b8de6251 100644
> --- a/drivers/firmware/arm_scmi/transports/Makefile
> +++ b/drivers/firmware/arm_scmi/transports/Makefile
> @@ -7,6 +7,8 @@ scmi_transport_mailbox-objs := mailbox.o
> obj-$(CONFIG_ARM_SCMI_TRANSPORT_MAILBOX) += scmi_transport_mailbox.o
> scmi_transport_optee-objs := optee.o
> obj-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += scmi_transport_optee.o
> +scmi_transport_pcc-objs := pcc.o
> +obj-$(CONFIG_ARM_SCMI_TRANSPORT_PCC) += scmi_transport_pcc.o
> scmi_transport_virtio-objs := virtio.o
> obj-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += scmi_transport_virtio.o
>
> diff --git a/drivers/firmware/arm_scmi/transports/pcc.c b/drivers/firmware/arm_scmi/transports/pcc.c
> new file mode 100644
> index 000000000000..aaebd1530370
> --- /dev/null
> +++ b/drivers/firmware/arm_scmi/transports/pcc.c
> @@ -0,0 +1,593 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * System Control and Management Interface (SCMI) Message ACPI PCC
> + * Transport Driver
> + *
> + * This transport uses ACPI PCC (PCCT Type 3/4) subspaces via the Linux
> + * PCC mailbox controller to exchange SCMI messages over the standard
> + * SCMI Shared Memory Transport (SMT) layout.
> + *
> + * PCC subspace selection is conveyed via ACPI SCMI namespace device.
> + *
> + * Copyright (C) 2025
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/err.h>
> +#include <linux/device.h>
> +#include <linux/hashtable.h>
> +#include <linux/io.h>
> +#include <linux/limits.h>
> +#include <linux/mailbox_client.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +#include <acpi/pcc.h>
> +
> +#include "../common.h"
> +
> +#define SCMI_SHMEM_FLAG_INTR_ENABLED BIT(0)
> +#define SCMI_TRANSPORT_PACKAGE_MAX_VERSION (1)
> +#define SCMI_PROTOCOL_PACKAGE_MAX_VERSION (1)
> +#define SCMI_TRANSPORT_SHARED_CHANNEL BIT_ULL(0)
> +#define SCMI_TRANSPORT_P2A_CHANNEL BIT_ULL(1)
> +#define SCMI_TRANSPORT_FLAGS_MASK (SCMI_TRANSPORT_SHARED_CHANNEL | \
> + SCMI_TRANSPORT_P2A_CHANNEL)
> +
> +/*
> + * SCMI specification requires all parameters, message headers, return
> + * arguments or any protocol data to be expressed in little endian
> + * format only.
> + */
> +struct pcc_shared_mem {
> + struct acpi_pcct_ext_pcc_shared_memory header;
> + u8 msg_payload[];
> +};
> +
> +/**
> + * struct scmi_pcc - Structure representing a SCMI mailbox transport
> + *
> + * @cl: Mailbox Client
> + * @pchan: Transmit/Receive PCC/mailbox channel
> + * @cinfo: SCMI channel info
> + * @shmem: Transmit/Receive shared memory area
> + */
> +struct scmi_pcc {
> + struct mbox_client cl;
> + struct pcc_mbox_chan *pchan;
> + struct scmi_chan_info *cinfo;
> +};
> +
> +struct pcc_transport {
> + u32 uid;
> + u32 pcc_ss_id;
> + u32 protocol_id;
> + u32 flags;
> + struct hlist_node hnode;
> +};
> +
> +/* Not all MAX_PCC_SUBSPACES will be used for SCMI, keeping it at 16 for now */
> +static DECLARE_HASHTABLE(pcc_id_hash, ilog2(MAX_PCC_SUBSPACES / 8));
> +
> +#define client_to_scmi_pcc(c) container_of(c, struct scmi_pcc, cl)
> +
> +static struct scmi_transport_core_operations *core;
> +
> +static void acpi_scmi_clear_transport_hash(void)
> +{
> + struct pcc_transport *p;
> + struct hlist_node *tmp;
> + int idx;
> +
> + hash_for_each_safe(pcc_id_hash, idx, tmp, p, hnode) {
> + hash_del(&p->hnode);
> + kfree(p);
> + }
> +}
> +
> +static int acpi_scmi_dsd_parse_transport_package(const union acpi_object *obj)
> +{
> + unsigned int revision, pkg_cnt;
> + unsigned int common_a2p = 0, common_p2a = 0;
> + int idx;
> +
> + if (obj->type != ACPI_TYPE_PACKAGE || obj->package.count < 2)
> + return -EINVAL;
> + if (obj->package.elements[0].type != ACPI_TYPE_INTEGER ||
> + obj->package.elements[1].type != ACPI_TYPE_INTEGER)
> + return -EINVAL;
> +
> + revision = obj->package.elements[0].integer.value;
> + pkg_cnt = obj->package.elements[1].integer.value;
> +
> + if (revision != SCMI_TRANSPORT_PACKAGE_MAX_VERSION)
> + return -EINVAL;
> + if (obj->package.count != pkg_cnt + 2)
> + return -EINVAL;
> +
> + for (idx = 0; idx < pkg_cnt; idx++) {
> + union acpi_object *pack = &obj->package.elements[idx + 2];
> + struct pcc_transport *p, *tmp;
> + u64 flags;
> + u32 uid;
> +
> + if (pack->type != ACPI_TYPE_PACKAGE || pack->package.count != 3) {
> + pr_info("Invalid transport properties pkg %d\n", idx);
> + return -EINVAL;
> + }
> + if (pack->package.elements[0].type != ACPI_TYPE_INTEGER ||
> + pack->package.elements[1].type != ACPI_TYPE_INTEGER ||
> + pack->package.elements[2].type != ACPI_TYPE_INTEGER)
> + return -EINVAL;
> + if (pack->package.elements[0].integer.value > U32_MAX ||
> + pack->package.elements[1].integer.value > U32_MAX)
> + return -EINVAL;
> +
> + flags = pack->package.elements[2].integer.value;
> + if (flags & ~SCMI_TRANSPORT_FLAGS_MASK)
> + return -EINVAL;
> +
> + uid = pack->package.elements[1].integer.value;
> + hash_for_each_possible(pcc_id_hash, tmp, hnode, uid) {
> + if (tmp->uid == uid) {
> + pr_info("Duplicate UID %d\n", uid);
> + return -EEXIST;
> + }
> + }
> +
> + p = kzalloc(sizeof(*p), GFP_KERNEL);
> + if (!p)
> + return -ENOMEM;
> +
> + p->uid = uid;
> + p->pcc_ss_id = pack->package.elements[0].integer.value;
> + p->flags = flags;
> + if (p->flags & SCMI_TRANSPORT_SHARED_CHANNEL) {
> + p->protocol_id = SCMI_PROTOCOL_BASE;
> + if (p->flags & SCMI_TRANSPORT_P2A_CHANNEL)
> + common_p2a++;
> + else
> + common_a2p++;
> + }
> +
> + hash_add(pcc_id_hash, &p->hnode, uid);
> + }
> +
> + if (common_a2p != 1 || common_p2a > 1)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int acpi_scmi_dsd_parse_protocol_subpackage(const union acpi_object *obj,
> + int prot_id)
> +{
> + bool found, tx_found = false, rx_found = false;
> + u32 uid;
> + int idx, ret = 0;
> + struct pcc_transport *p;
> + unsigned int pkg_cnt = obj->package.count;
> +
> + if (pkg_cnt > 2) {
> + pr_warn("Only 2 channels: one Tx and one Rx needed\n");
> + return -EINVAL;
> + }
> +
> + for (idx = 0; idx < pkg_cnt; idx++) {
> + union acpi_object *pack = &obj->package.elements[idx];
> + u64 flags;
> +
> + if (pack->type != ACPI_TYPE_PACKAGE || pack->package.count != 2)
> + return -EINVAL;
> + if (pack->package.elements[0].type != ACPI_TYPE_INTEGER ||
> + pack->package.elements[1].type != ACPI_TYPE_INTEGER)
> + return -EINVAL;
> + if (pack->package.elements[0].integer.value > U32_MAX)
> + return -EINVAL;
> +
> + flags = pack->package.elements[1].integer.value;
> + if (flags)
> + return -EINVAL;
> +
> + uid = pack->package.elements[0].integer.value;
> + found = false;
> + hash_for_each_possible(pcc_id_hash, p, hnode, uid) {
> + if (p->uid != uid)
> + continue;
> +
> + found = true;
> + if (p->flags & SCMI_TRANSPORT_SHARED_CHANNEL) {
> + pr_info("Invalid! %d channel is shared\n",
> + p->pcc_ss_id);
> + ret = -EINVAL;
> + break;
> + }
> + if (p->flags & SCMI_TRANSPORT_P2A_CHANNEL) {
> + if (rx_found)
> + return -EINVAL;
> + rx_found = true;
> + } else {
> + if (tx_found)
> + return -EINVAL;
> + tx_found = true;
> + }
> + p->protocol_id = prot_id;
> + break;
> + }
> +
> + if (ret)
> + return ret;
> + if (!found)
> + return -ENOENT;
> + }
> +
> + return ret;
> +}
> +
> +static int
> +acpi_scmi_dsd_parse_protocol_package(const union acpi_object *obj, int prot_id)
> +{
> + unsigned int revision;
> + union acpi_object *pack;
> + int ret;
> +
> + if (obj->type != ACPI_TYPE_PACKAGE || obj->package.count != 3)
> + return -EINVAL;
> + if (obj->package.elements[0].type != ACPI_TYPE_INTEGER)
> + return -EINVAL;
> +
> + revision = obj->package.elements[0].integer.value;
> + pack = &obj->package.elements[1];
> +
> + if (revision != SCMI_PROTOCOL_PACKAGE_MAX_VERSION)
> + return -EINVAL;
> +
> + if (pack->type != ACPI_TYPE_PACKAGE) {
> + pr_info("Invalid protocol transport package\n");
> + return -EINVAL;
> + }
> +
> + /* Empty protocol specific transport package allowed */
> + if (pack->package.count != 0) {
> + ret = acpi_scmi_dsd_parse_protocol_subpackage(pack, prot_id);
> + if (ret)
> + return ret;
> + }
> +
> + pack = &obj->package.elements[2];
> + if (pack->type != ACPI_TYPE_PACKAGE) {
> + pr_info("Invalid protocol transport association package\n");
> + return -EINVAL;
> + }
> +
> + if (pack->package.count != 0) {
> + pr_info("Non-empty association package not supported\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +/* ACPI SCMI _DSD UUID: "84a2d1c6-86b6-4199-8dac-9c17449d5e03" */
> +static const guid_t acpi_scmi_uuid = GUID_INIT(0x84a2d1c6, 0x86b6, 0x4199,
> + 0x8d, 0xac, 0x9c, 0x17,
> + 0x44, 0x9d, 0x5e, 0x03);
> +
> +static int acpi_scmi_lookup_protocol_id(const char *name)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(scmi_dsd_info_list); i++) {
> + if (!strcmp(name, scmi_dsd_info_list[i].property_name))
> + return scmi_dsd_info_list[i].protocol_id;
> + }
> + return -ENOENT;
> +}
> +
> +static int acpi_scmi_parse_properties(const union acpi_object *properties)
> +{
> + int i;
> +
> + if (properties->type != ACPI_TYPE_PACKAGE)
> + return -EINVAL;
> +
> + for (i = 0; i < properties->package.count; i++) {
> + const union acpi_object *property;
> + const union acpi_object *k, *v;
> + int prot_id, ret;
> +
> + property = &properties->package.elements[i];
> + if (property->type != ACPI_TYPE_PACKAGE ||
> + property->package.count != 2)
> + return -EINVAL;
> +
> + k = &property->package.elements[0];
> + v = &property->package.elements[1];
> +
> + if (k->type != ACPI_TYPE_STRING || !k->string.pointer)
> + return -EINVAL;
> + if (v->type != ACPI_TYPE_PACKAGE)
> + return -EINVAL;
> +
> + prot_id = acpi_scmi_lookup_protocol_id(k->string.pointer);
> + if (prot_id < 0)
> + return -EINVAL;
> +
> + /*
> + * We validate structure inside the dedicated parsing functions.
> + * They return proper errors and avoid double-validation.
> + */
> + if (prot_id == SCMI_PROTOCOL_BASE)
> + ret = acpi_scmi_dsd_parse_transport_package(v);
> + else
> + ret = acpi_scmi_dsd_parse_protocol_package(v, prot_id);
> +
> + if (ret)
> + return ret;
> + }
> + return 0;
> +}
> +
> +static int acpi_scmi_parse_dsd(struct acpi_device *adev)
> +{
> + struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
> + union acpi_object *desc;
> + acpi_status status;
> + int i, ret = -ENOENT;
> +
> + if (!adev->handle)
> + return -EINVAL;
> +
> + status = acpi_evaluate_object_typed(adev->handle, "_DSD", NULL, &buf,
> + ACPI_TYPE_PACKAGE);
> + if (ACPI_FAILURE(status))
> + return -EINVAL;
> +
> + desc = buf.pointer;
> + if (desc->package.count % 2)
> + goto out_free_inval;
> +
> + /* Look for the device properties GUID. */
> + for (i = 0; i < desc->package.count; i += 2) {
> + const union acpi_object *guid;
> + const union acpi_object *properties;
> +
> + guid = &desc->package.elements[i];
> + properties = &desc->package.elements[i + 1];
> +
> + /*
> + * The first element must be a GUID and the second one must be
> + * a package.
> + */
> + if (guid->type != ACPI_TYPE_BUFFER ||
> + guid->buffer.length != UUID_SIZE ||
> + properties->type != ACPI_TYPE_PACKAGE)
> + continue;
> +
> + if (!guid_equal((guid_t *)guid->buffer.pointer,
> + &acpi_scmi_uuid))
> + continue;
> +
> + ret = acpi_scmi_parse_properties(properties);
> + goto out_free;
> + }
> +
> +out_free:
> + ACPI_FREE(buf.pointer);
> + return ret;
> +out_free_inval:
> + ret = -EINVAL;
> + goto out_free;
> +}
> +
> +static int acpi_scmi_namespace_device_parse(struct fwnode_handle *fwnode)
> +{
> + struct acpi_device *adev = to_acpi_device_node(fwnode);
> + int ret;
> +
> + acpi_scmi_clear_transport_hash();
> + hash_init(pcc_id_hash);
> +
> + ret = acpi_scmi_parse_dsd(adev);
> + if (ret)
> + acpi_scmi_clear_transport_hash();
> +
> + return ret;
> +}
> +
> +static int pcc_get_ss_id(u32 prot_id, bool tx)
> +{
> + struct pcc_transport *p;
> + int idx;
> +
> + hash_for_each(pcc_id_hash, idx, p, hnode) {
> + if (p->protocol_id != prot_id)
> + continue;
> +
> + if ((!tx && (p->flags & SCMI_TRANSPORT_P2A_CHANNEL)) ||
> + (tx && !(p->flags & SCMI_TRANSPORT_P2A_CHANNEL)))
> + return p->pcc_ss_id;
> + }
> +
> + return -ENOENT;
> +}
> +
> +static bool
> +pcc_chan_available(struct fwnode_handle *fwnode, int prot_id, int idx)
> +{
> + /*
> + * Parse the ACPI SCMI namespace device on each Tx-channel lookup and
> + * reuse the resulting mapping for the matching Rx lookup. Defer full
> + * validation until pcc_chan_setup() for simplicity.
> + */
> + if (!idx && acpi_scmi_namespace_device_parse(fwnode))
> + return false;
> +
> + if (pcc_get_ss_id(prot_id, idx ? false : true) < 0)
> + return false;
> +
> + return true;
> +}
> +
> +static void tx_prepare(struct mbox_client *cl, void *m)
> +{
> + struct scmi_pcc *smbox = client_to_scmi_pcc(cl);
> + struct pcc_shared_mem __iomem *shmem = smbox->pchan->shmem;
> + struct scmi_xfer *xfer = m;
> +
> + /*
> + * PCC take cares not to call tx_prepare until last transmit is done
> + * Mark channel busy + clear error, request platform IRQ if available
> + */
> + if (smbox->pchan->mchan->mbox->txdone_irq)
> + iowrite32(SCMI_SHMEM_FLAG_INTR_ENABLED, &shmem->header.flags);
> + iowrite32(sizeof(shmem->header.command) + xfer->tx.len,
> + &shmem->header.length);
> + iowrite32(pack_scmi_header(&xfer->hdr), &shmem->header.command);
> + if (xfer->tx.buf)
> + memcpy_toio(shmem->msg_payload, xfer->tx.buf, xfer->tx.len);
> +}
> +
> +static void rx_callback(struct mbox_client *cl, void *m)
> +{
> + struct scmi_pcc *smbox = client_to_scmi_pcc(cl);
> + struct pcc_shared_mem __iomem *shmem = smbox->pchan->shmem;
> +
> + core->rx_callback(smbox->cinfo, ioread32(&shmem->header.command), NULL);
> +}
> +
> +static int pcc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
> + bool tx)
> +{
> + const char *desc = tx ? "Tx" : "Rx";
> + struct device *cdev = cinfo->dev;
> + struct scmi_pcc *smbox;
> + int ret, ss_id;
> + struct mbox_client *cl;
> +
> + smbox = devm_kzalloc(dev, sizeof(*smbox), GFP_KERNEL);
> + if (!smbox)
> + return -ENOMEM;
> +
> + cl = &smbox->cl;
> + cl->dev = cdev;
> + cl->tx_prepare = tx ? tx_prepare : NULL;
> + cl->rx_callback = rx_callback;
> + cl->tx_block = false;
> +
> + ss_id = pcc_get_ss_id(cinfo->id, tx);
> + if (ss_id < 0)
> + return ss_id;
> +
> + smbox->pchan = pcc_mbox_request_channel(cl, ss_id);
> + if (IS_ERR(smbox->pchan)) {
> + ret = PTR_ERR(smbox->pchan);
> + if (ret != -EPROBE_DEFER)
> + dev_err(cdev,
> + "failed to request SCMI %s mailbox\n", desc);
> + return ret;
> + }
> +
> + cinfo->transport_info = smbox;
> + smbox->cinfo = cinfo;
> +
> + return 0;
> +}
> +
> +static int pcc_chan_free(int id, void *p, void *data)
> +{
> + struct scmi_chan_info *cinfo = p;
> + struct scmi_pcc *smbox = cinfo->transport_info;
> +
> + if (smbox && !IS_ERR(smbox->pchan)) {
> + pcc_mbox_free_channel(smbox->pchan);
> + cinfo->transport_info = NULL;
> + smbox->pchan = NULL;
> + smbox->cinfo = NULL;
> + }
> +
> + return 0;
> +}
> +
> +static int
> +pcc_send_message(struct scmi_chan_info *cinfo, struct scmi_xfer *xfer)
> +{
> + struct scmi_pcc *smbox = cinfo->transport_info;
> + int ret;
> +
> + /*
> + * The mailbox layer has its own queue. However the mailbox queue
> + * confuses the per message SCMI timeouts since the clock starts when
> + * the message is submitted into the mailbox queue. So when multiple
> + * messages are queued up the clock starts on all messages instead of
> + * only the one inflight.
> + */
> + ret = mbox_send_message(smbox->pchan->mchan, xfer);
From a Codex based code review:
High: the transport reintroduces the mailbox-queue timeout bug that the
existing SCMI mailbox transport already had to work around. The new code
copies the warning comment about queued mailbox messages confusing SCMI
timeouts, but still calls `mbox_send_message()` directly with no channel
mutex, no `cl->knows_txdone`, and no `mark_txdone` hook
([pcc.c](/root/linux.a6c6b6fcb1c8/drivers/firmware/arm_scmi/transports/pcc.c#L472),
[pcc.c](/root/linux.a6c6b6fcb1c8/drivers/firmware/arm_scmi/transports/pcc.c#L518),
[pcc.c](/root/linux.a6c6b6fcb1c8/drivers/firmware/arm_scmi/transports/pcc.c#L525)).
Generic mailbox queues non-blocking sends immediately
([mailbox.c](/root/linux.a6c6b6fcb1c8/drivers/mailbox/mailbox.c#L243),
[mailbox.c](/root/linux.a6c6b6fcb1c8/drivers/mailbox/mailbox.c#L279)),
while SCMI starts its per-message timeout as soon as `.send_message()`
returns
([driver.c](/root/linux.a6c6b6fcb1c8/drivers/firmware/arm_scmi/driver.c#L1446),
[driver.c](/root/linux.a6c6b6fcb1c8/drivers/firmware/arm_scmi/driver.c#L1458)).
The existing mailbox transport fixes exactly this by serializing with
`chan_lock`, setting `knows_txdone`, and only releasing the mailbox slot
from `mark_txdone()` after SCMI has finished with the response
([mailbox.c](/root/linux.a6c6b6fcb1c8/drivers/firmware/arm_scmi/transports/mailbox.c#L217),
[mailbox.c](/root/linux.a6c6b6fcb1c8/drivers/firmware/arm_scmi/transports/mailbox.c#L251),
[mailbox.c](/root/linux.a6c6b6fcb1c8/drivers/firmware/arm_scmi/transports/mailbox.c#L275),
[mailbox.c](/root/linux.a6c6b6fcb1c8/drivers/firmware/arm_scmi/transports/mailbox.c#L300)).
Under concurrent SCMI traffic on a shared PCC channel, later requests
can expire while they are still just sitting in the mailbox framework queue.
> + /* mbox_send_message returns non-negative value on success */
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
> +static void pcc_fetch_response(struct scmi_chan_info *cinfo,
> + struct scmi_xfer *xfer)
> +{
> + struct scmi_pcc *smbox = cinfo->transport_info;
> + struct pcc_shared_mem __iomem *shmem = smbox->pchan->shmem;
> + size_t len = ioread32(&shmem->header.length);
> +
> + xfer->hdr.status = ioread32(shmem->msg_payload);
> + /* Skip the length of header and status in shmem area i.e 8 bytes */
> + xfer->rx.len = min_t(size_t, xfer->rx.len, len > 8 ? len - 8 : 0);
> +
> + /* Take a copy to the rx buffer.. */
> + memcpy_fromio(xfer->rx.buf, shmem->msg_payload + 4, xfer->rx.len);
> +}
> +
> +static void pcc_fetch_notification(struct scmi_chan_info *cinfo, size_t max_len,
> + struct scmi_xfer *xfer)
> +{
> + struct scmi_pcc *smbox = cinfo->transport_info;
> + struct pcc_shared_mem __iomem *shmem = smbox->pchan->shmem;
> + size_t len = ioread32(&shmem->header.length);
> +
> + /* Skip only the length of header in shmem area i.e 4 bytes */
> + xfer->rx.len = min_t(size_t, max_len, len > 4 ? len - 4 : 0);
> +
> + /* Take a copy to the rx buffer.. */
> + memcpy_fromio(xfer->rx.buf, shmem->msg_payload, xfer->rx.len);
> +}
> +
> +static const struct scmi_transport_ops scmi_pcc_ops = {
> + .chan_available = pcc_chan_available,
> + .chan_setup = pcc_chan_setup,
> + .chan_free = pcc_chan_free,
> + .send_message = pcc_send_message,
> + .fetch_response = pcc_fetch_response,
> + .fetch_notification = pcc_fetch_notification,
> +};
I think this is OK< but we should make it explicit that this is not
going to work for Polled devices:
"- High: the new transport does not work on valid PCC configurations
that rely on mailbox polling instead of an interrupt. The PCC mailbox
controller explicitly falls back to `txdone_poll` when
`ACPI_PCCT_DOORBELL` is absent
([pcc.c](/root/linux.a6c6b6fcb1c8/drivers/mailbox/pcc.c#L800),
[pcc.c](/root/linux.a6c6b6fcb1c8/drivers/mailbox/pcc.c#L806)), but the
SCMI PCC transport never sets `no_completion_irq`, never provides
`poll_done`, and does not advertise `sync_cmds_completed_on_ret`
([pcc.c](/root/linux.a6c6b6fcb1c8/drivers/firmware/arm_scmi/transports/pcc.c#L562),
[pcc.c](/root/linux.a6c6b6fcb1c8/drivers/firmware/arm_scmi/transports/pcc.c#L575)).
SCMI core therefore waits on `xfer->done` in interrupt mode
([driver.c](/root/linux.a6c6b6fcb1c8/drivers/firmware/arm_scmi/driver.c#L1317)),
but mailbox polling only advances the TX state machine via `tx_tick()`
and never invokes the transport `rx_callback` that would complete the
transfer
([mailbox.c](/root/linux.a6c6b6fcb1c8/drivers/mailbox/mailbox.c#L105),
[mailbox.c](/root/linux.a6c6b6fcb1c8/drivers/mailbox/mailbox.c#L118)).
The result is deterministic request timeout on non-IRQ PCC subspaces."
I think those values can all be deduced from the PCCT entries. It might
be worth a "BUG-ON" if someone does provide a broken configuration.
> +
> +static struct scmi_desc scmi_pcc_desc = {
> + .ops = &scmi_pcc_ops,
> + .max_rx_timeout_ms = 30, /* We may increase this if required */
> + .max_msg = 20, /* Limited by MBOX_TX_QUEUE_LEN */
> + .max_msg_size = SCMI_SHMEM_MAX_PAYLOAD_SIZE,
> +};
> +
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id scmi_acpi_ids[] = {
> + { "ARML0001", 0 },
> + { }
> +};
> +
> +MODULE_DEVICE_TABLE(acpi, scmi_acpi_ids);
> +#endif
> +
> +DEFINE_SCMI_ACPI_TRANSPORT_DRIVER(scmi_pcc, scmi_pcc_driver,
> + scmi_pcc_desc, scmi_acpi_ids, core);
> +module_platform_driver(scmi_pcc_driver);
> +
> +MODULE_AUTHOR("Sudeep Holla <sudeep.holla at kernel.org>");
> +MODULE_DESCRIPTION("SCMI ACPI PCC Transport driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
> index aafaac1496b0..d4ab8d918ee9 100644
> --- a/include/linux/scmi_protocol.h
> +++ b/include/linux/scmi_protocol.h
> @@ -926,6 +926,7 @@ enum scmi_std_protocol {
> SCMI_PROTOCOL_VOLTAGE = 0x17,
> SCMI_PROTOCOL_POWERCAP = 0x18,
> SCMI_PROTOCOL_PINCTRL = 0x19,
> + SCMI_PROTOCOL_TELEMETRY = 0x1B,
> };
>
> enum scmi_system_events {
>
More information about the linux-arm-kernel
mailing list