From patchwork Wed Oct 9 19:26:37 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Justin Chen X-Patchwork-Id: 13829263 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 9222DCEE350 for ; Wed, 9 Oct 2024 21:16:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: MIME-Version:Message-Id:Date:Subject:Cc:To:From:Reply-To:Content-Type: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References:List-Owner; bh=aFnRjeTVGGQvd1QanxRfvZGU3hZHGwK30eJuLrfs1Ro=; b=V/EFiIu8i4nvkvqNga13Nxhrx2 NMk1Wwy5EXr9I/bvqjLdDZ4TYLFlNZRWWpaPl0o0wyW7WGsDYnSltjchjOdI8c13VKaZ6upmG5ELx 4ejX/B+7r7gUBdYmjiCXOfVaabR3VB7c+7K/+xPz2SMNeYQHDsrtARWrP5gHdC7btwKQxQjrFEYmG 2EJ6uU+t2sOeaISAWfyZwHIHpNhjGQSP2WHK4q4ND6cqoH1rNxoSH0ki44Qqx0Tpm//MHJl718osE d0irX4YEmeNVAE5Sk6hxujacQ+gqMfHWo3J753nTSDwFe4NXN7UjRmbiMcuD2xT7TMwmNwpvOU7db VWaM5Mww==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1sye2L-0000000AkN4-3cIf; Wed, 09 Oct 2024 21:16:01 +0000 Received: from mail-qv1-xf2f.google.com ([2607:f8b0:4864:20::f2f]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1sycKk-0000000AVH1-1K9v for linux-arm-kernel@lists.infradead.org; Wed, 09 Oct 2024 19:26:55 +0000 Received: by mail-qv1-xf2f.google.com with SMTP id 6a1803df08f44-6cbcc2bd80fso1442356d6.1 for ; Wed, 09 Oct 2024 12:26:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google; t=1728502012; x=1729106812; darn=lists.infradead.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=aFnRjeTVGGQvd1QanxRfvZGU3hZHGwK30eJuLrfs1Ro=; b=IVE7e1uW6h7uusxr2f45hDKOQb4Ee9pyYdXZXLYy4dbVLv88htQ6THLrjrHavTUZUs gIWQcvphWHaDdVL9xFwlweKq7AJ7xAUO1lQtL15MNfkPc11p2nKMcAVPdiHnSMZm9Gui YkKFBmETOslPcRgYmSg7Z+2YmUlEfkYzzAVMU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728502012; x=1729106812; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=aFnRjeTVGGQvd1QanxRfvZGU3hZHGwK30eJuLrfs1Ro=; b=gwXZBlEto9NLvZpYBunI9ZE/KS+RqmHOMEfC2LNGhDSOfExqZAz0aeKsdBYehxe2k7 A7WgJk8YUj6zze2IRglhSyLAcuvbgPpxyh3prJ/ApynVoWb3PXuRQp2mOP2nIay4mzXH c3M5MSMHgoffU7VKYOdnaRTtw8sliOkO8Qgre62l/l+PWsF+P3KVigNoFbLk3f4cijK5 ZJYT8KtHUkwVODPRFEfHqXEHrZg1FpEjsF88i7RsN8lCiWmYgU4BEZAtYwU7aJzS4oCz 07ZQ03zIrZOSCixmKZlv8VrN0H9laqsiQMbvfJuqwtqy+jDvGLa8nfGbGhQnzQDsy/UG I7CA== X-Gm-Message-State: AOJu0YwgxNGlHpcOfCfMDN2oRrAyJ0XRBXO/O5rjT0t2Sr2q52zAK/dh WWVjgDyc+J6j1HQtd850AhO++Du4m+40EvIODQBkuT1J63awbHssbQvIa85ujg== X-Google-Smtp-Source: AGHT+IHV0WAEt5thVKS88Lh79SPoGz5dND4w2rxvfSsWAa37Kvxr15vakcbkP62eBOqPs4ChkOCKiA== X-Received: by 2002:a05:6214:4517:b0:6cb:e517:d1ca with SMTP id 6a1803df08f44-6cbe517d790mr8090676d6.9.1728502012446; Wed, 09 Oct 2024 12:26:52 -0700 (PDT) Received: from stbirv-lnx-1.igp.broadcom.net ([192.19.223.252]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-6cba46dfd5fsm48504036d6.52.2024.10.09.12.26.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 09 Oct 2024 12:26:51 -0700 (PDT) From: Justin Chen To: arm-scmi@vger.kernel.org, cristian.marussi@arm.com, sudeep.holla@arm.com Cc: linux-arm-kernel@lists.infradead.org, peng.fan@nxp.com, bcm-kernel-feedback-list@broadcom.com, florian.fainelli@broadcom.com, Justin Chen Subject: [PATCH v2] firmware: arm_scmi: Queue in scmi layer for mailbox implementation Date: Wed, 9 Oct 2024 12:26:37 -0700 Message-Id: <20241009192637.1090238-1-justin.chen@broadcom.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20241009_122654_381571_B5BCF9AA X-CRM114-Status: GOOD ( 17.20 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org send_message() does not block in the MBOX implementation. This is because the mailbox layer has its own queue. However, this confuses the per xfer timeouts as they all start their timeout ticks in parallel. Consider a case where the xfer timeout is 30ms and a SCMI transaction takes 25ms. 0ms: Message #0 is queued in mailbox layer and sent out, then sits at scmi_wait_for_message_response() with a timeout of 30ms 1ms: Message #1 is queued in mailbox layer but not sent out yet. Since send_message() doesn't block, it also sits at scmi_wait_for_message_response() with a timeout of 30ms ... 25ms: Message #0 is completed, txdone is called and Message #1 is sent out 31ms: Message #1 times out since the count started at 1ms. Even though it has only been inflight for 6ms. Fixes: b53515fa177c ("firmware: arm_scmi: Make MBOX transport a standalone driver") Signed-off-by: Justin Chen Reviewed-by: Cristian Marussi Tested-by: Cristian Marussi --- Changes in v2: - Added Fixes tag - Improved commit message to better capture the issue .../firmware/arm_scmi/transports/mailbox.c | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/drivers/firmware/arm_scmi/transports/mailbox.c b/drivers/firmware/arm_scmi/transports/mailbox.c index 1a754dee24f7..30bc2865582f 100644 --- a/drivers/firmware/arm_scmi/transports/mailbox.c +++ b/drivers/firmware/arm_scmi/transports/mailbox.c @@ -33,6 +33,7 @@ struct scmi_mailbox { struct mbox_chan *chan_platform_receiver; struct scmi_chan_info *cinfo; struct scmi_shared_mem __iomem *shmem; + struct mutex chan_lock; }; #define client_to_scmi_mailbox(c) container_of(c, struct scmi_mailbox, cl) @@ -205,6 +206,7 @@ static int mailbox_chan_setup(struct scmi_chan_info *cinfo, struct device *dev, cl->rx_callback = rx_callback; cl->tx_block = false; cl->knows_txdone = tx; + mutex_init(&smbox->chan_lock); smbox->chan = mbox_request_channel(cl, tx ? 0 : p2a_chan); if (IS_ERR(smbox->chan)) { @@ -267,11 +269,21 @@ static int mailbox_send_message(struct scmi_chan_info *cinfo, struct scmi_mailbox *smbox = cinfo->transport_info; int ret; + /* + * The mailbox layer has it's 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. + */ + mutex_lock(&smbox->chan_lock); + ret = mbox_send_message(smbox->chan, xfer); /* mbox_send_message returns non-negative value on success, so reset */ if (ret > 0) ret = 0; + else + mutex_unlock(&smbox->chan_lock); return ret; } @@ -281,13 +293,10 @@ static void mailbox_mark_txdone(struct scmi_chan_info *cinfo, int ret, { struct scmi_mailbox *smbox = cinfo->transport_info; - /* - * NOTE: we might prefer not to need the mailbox ticker to manage the - * transfer queueing since the protocol layer queues things by itself. - * Unfortunately, we have to kick the mailbox framework after we have - * received our message. - */ mbox_client_txdone(smbox->chan, ret); + + /* Release channel */ + mutex_unlock(&smbox->chan_lock); } static void mailbox_fetch_response(struct scmi_chan_info *cinfo,