From patchwork Mon Nov 22 23:06:35 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Cristian Marussi X-Patchwork-Id: 12693396 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id EAEADC433F5 for ; Mon, 22 Nov 2021 23:24:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:MIME-Version:List-Subscribe:List-Help: List-Post:List-Archive:List-Unsubscribe:List-Id:References:In-Reply-To: Message-Id:Date:Subject:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Gy01LWP8DkXBIT+iZ0TBG9DIQPUofeyR/4eM94z/HCg=; b=R0UH041fg/B+5d XIAOnzNf1ZKJWWfWlE0oP5gejFTPMmqTJtzG3jv/Y+2+HolC7mXOaadJqjj2WeMq7sHE3zbp1/pnn J7bBGpkN3+FmauQdjq3eWmjwyzrp+5eecun4zLJro3uIrBvxVS6eJDj5NOmcIY+0dxE1VpWh9jZkA 8bVr0uFun0d4hfd/woI97d0vd+tbvmJD4Ma5NMTqCFA8fbF+GUelzYrbB8m9R5MBVX3JmiAaSRU+l vqytciOjaa9/BZi7iGjIP907Q0SvRrlnhc7Xypq4alrBe5C9mCk57G2TrGcMTNJeNtMbJ+kXdhOZ9 sxbHDBkAuFWpoLjZt7VA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mpIeM-000Ojz-0m; Mon, 22 Nov 2021 23:23:02 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mpIPD-000JbE-GD for linux-arm-kernel@lists.infradead.org; Mon, 22 Nov 2021 23:07:25 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 2BB9D152B; Mon, 22 Nov 2021 15:07:23 -0800 (PST) Received: from e120937-lin.home (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A6EC03F5A1; Mon, 22 Nov 2021 15:07:21 -0800 (PST) From: Cristian Marussi To: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Cc: sudeep.holla@arm.com, james.quinlan@broadcom.com, Jonathan.Cameron@Huawei.com, f.fainelli@gmail.com, vincent.guittot@linaro.org, souvik.chakravarty@arm.com, cristian.marussi@arm.com Subject: [PATCH v6 11/16] firmware: arm_scmi: Add support for atomic transports Date: Mon, 22 Nov 2021 23:06:35 +0000 Message-Id: <20211122230640.1345-12-cristian.marussi@arm.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20211122230640.1345-1-cristian.marussi@arm.com> References: <20211122230640.1345-1-cristian.marussi@arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211122_150723_693907_1D8425BB X-CRM114-Status: GOOD ( 24.22 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org An SCMI transport can be configured as .atomic_enabled in order to signal to the SCMI core that all its TX path is executed in atomic context. When a specific platform configuration had properly configured such a transport as .atomic_enabled, the SCMI core will also take care not to sleep in the corresponding RX path while waiting for a response. Asynchronous commands should not be used in an atomic context and as such a warning is emitted if polling was requested for an asynchronous command. Add also a method to check if, from the SCMI drivers, the underlying SCMI transport is configured to support atomic transaction of SCMI commands. Signed-off-by: Cristian Marussi --- v5 --> v6 - removed atomic_capable - fully relying on transport polling capabilities - removed polling/atomic support for delayed_reponse and WARN - merged with is_transport_atomic() patch - is_transport_atomic() now considers polling_capable and atomic_enabled flags v4 --> v5 - added .atomic_enabled flag to decide wheter to enable atomic mode or not for atomic_capable transports - reviewed commit message --- drivers/firmware/arm_scmi/common.h | 4 +++ drivers/firmware/arm_scmi/driver.c | 49 ++++++++++++++++++++++++++++-- include/linux/scmi_protocol.h | 8 +++++ 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h index bf25f0e89c78..97a65d5fbb1d 100644 --- a/drivers/firmware/arm_scmi/common.h +++ b/drivers/firmware/arm_scmi/common.h @@ -419,6 +419,9 @@ struct scmi_device *scmi_child_dev_find(struct device *parent, * Used by core internally only when polling is * selected as a waiting for reply method: i.e. * if a completion irq was found use that anyway. + * @atomic_enabled: Flag to indicate that this transport, which is assured not + * to sleep anywhere on the TX path, can be used in atomic mode + * when requested. */ struct scmi_desc { int (*transport_init)(void); @@ -429,6 +432,7 @@ struct scmi_desc { int max_msg_size; const bool force_polling; const bool sync_cmds_atomic_replies; + const bool atomic_enabled; }; #ifdef CONFIG_ARM_SCMI_TRANSPORT_MAILBOX diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 8c04632f3ba3..7d6e97d7a077 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -907,6 +907,20 @@ static void reset_rx_to_maxsz(const struct scmi_protocol_handle *ph, * @ph: Pointer to SCMI protocol handle * @xfer: Transfer to initiate and wait for response * + * Using asynchronous commands in atomic/polling mode should be avoided since + * it could cause long busy-waiting here, so WARN if polling mode is detected + * for this command transaction since upper layers should refrain from issuing + * such kind of requests. + * + * The only other option would have been to refrain from using any asynchronous + * command even if made available, when an atomic transport is detected, and + * instead forcibly use the synchronous version (thing that can be easily + * attained at the protocol layer), but this would also have led to longer + * stalls of the channel for synchronous commands and possibly timeouts. + * (in other words there is usually a good reason if a platform provides an + * asynchronous version of a command and we should prefer to use it...just not + * in atomic/polling mode) + * * Return: -ETIMEDOUT in case of no delayed response, if transmit error, * return corresponding error, else if all goes well, return 0. */ @@ -918,12 +932,23 @@ static int do_xfer_with_response(const struct scmi_protocol_handle *ph, xfer->async_done = &async_response; + /* + * Delayed responses are not pollable, an async command should + * not have been used when requiring an atomic/poll context. + * (and Async + IgnoreDelayedResponses are sent via do_xfer) + */ + WARN_ON(xfer->hdr.poll_completion); + ret = do_xfer(ph, xfer); if (!ret) { - if (!wait_for_completion_timeout(xfer->async_done, timeout)) + if (!wait_for_completion_timeout(xfer->async_done, timeout)) { + dev_err(ph->dev, + "timed out in delayed resp(caller: %pS)\n", + (void *)_RET_IP_); ret = -ETIMEDOUT; - else if (xfer->hdr.status) + } else if (xfer->hdr.status) { ret = scmi_to_linux_errno(xfer->hdr.status); + } } xfer->async_done = NULL; @@ -1357,6 +1382,21 @@ static void scmi_devm_protocol_put(struct scmi_device *sdev, u8 protocol_id) WARN_ON(ret); } +/** + * scmi_is_transport_atomic - Method to check if underlying transport for an + * SCMI instance is configured as atomic. + * + * @handle: A reference to the SCMI platform instance. + * + * Return: True if transport is configured as atomic + */ +static bool scmi_is_transport_atomic(const struct scmi_handle *handle) +{ + struct scmi_info *info = handle_to_scmi_info(handle); + + return info->desc->atomic_enabled && info->polling_capable; +} + static inline struct scmi_handle *scmi_handle_get_from_info_unlocked(struct scmi_info *info) { @@ -1903,6 +1943,7 @@ static int scmi_probe(struct platform_device *pdev) handle->version = &info->version; handle->devm_protocol_get = scmi_devm_protocol_get; handle->devm_protocol_put = scmi_devm_protocol_put; + handle->is_transport_atomic = scmi_is_transport_atomic; if (desc->ops->link_supplier) { ret = desc->ops->link_supplier(dev); @@ -1921,6 +1962,10 @@ static int scmi_probe(struct platform_device *pdev) if (scmi_notification_init(handle)) dev_err(dev, "SCMI Notifications NOT available.\n"); + if (info->desc->atomic_enabled && !info->polling_capable) + dev_err(dev, + "Transport is not polling capable. Atomic mode not supported.\n"); + /* * Trigger SCMI Base protocol initialization. * It's mandatory and won't be ever released/deinit until the diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h index 80e781c51ddc..9f895cb81818 100644 --- a/include/linux/scmi_protocol.h +++ b/include/linux/scmi_protocol.h @@ -612,6 +612,13 @@ struct scmi_notify_ops { * @devm_protocol_get: devres managed method to acquire a protocol and get specific * operations and a dedicated protocol handler * @devm_protocol_put: devres managed method to release a protocol + * @is_transport_atomic: method to check if the underlying transport for this + * instance handle is configured to support atomic + * transactions for commands. + * Some users of the SCMI stack in the upper layers could + * be interested to know if they can assume SCMI + * command transactions associated to this handle will + * never sleep and act accordingly. * @notify_ops: pointer to set of notifications related operations */ struct scmi_handle { @@ -622,6 +629,7 @@ struct scmi_handle { (*devm_protocol_get)(struct scmi_device *sdev, u8 proto, struct scmi_protocol_handle **ph); void (*devm_protocol_put)(struct scmi_device *sdev, u8 proto); + bool (*is_transport_atomic)(const struct scmi_handle *handle); const struct scmi_notify_ops *notify_ops; };