From mboxrd@z Thu Jan 1 00:00:00 1970 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 29570CFB43F for ; Mon, 7 Oct 2024 12:47:44 +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:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=KGLGBm7wtbM0PFv1yN6YoWJ8siuj3CzysjCrvFUmrms=; b=psHjdLZswy/AhHt+r8SyiVNjI8 4fVpiAGmu3mQkMT9zBJQZcIFWDocL5EL6yu1T1msPG+/qHN1d+3bzizqtPcT+Zo6NOfnb84hdhEN0 OSN6/DWnZC4MRWRUsVYX1Gj2+Mlh7ZMKp9uwuuisDnWi40WOWvBEr+IrW4w+zJAkYOJxmpN7PxE7z +5K2c4zHG7gsq8tfT1J/XdfBCYUkEopJX6oVT1bzTre/6IsLKrVHIx4SoikHqSLsFNgMN2noHex5f XAExiqkYl5AXelxAAZB1RMVIM8vX89FNSGOp+S+T0zh+KtqThT/gslcpfp6eOhlfrCKwBNPRIR9Kt 6OCeSPtw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1sxn9B-00000002Ozo-1gcL; Mon, 07 Oct 2024 12:47:33 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1sxmx3-00000002LsX-1JN6 for linux-arm-kernel@lists.infradead.org; Mon, 07 Oct 2024 12:35:03 +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 4E683FEC; Mon, 7 Oct 2024 05:35:30 -0700 (PDT) Received: from pluto (usa-sjc-mx-foss1.foss.arm.com [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A59203F640; Mon, 7 Oct 2024 05:34:59 -0700 (PDT) Date: Mon, 7 Oct 2024 13:34:57 +0100 From: Cristian Marussi To: Justin Chen Cc: arm-scmi@vger.kernel.org, cristian.marussi@arm.com, sudeep.holla@arm.com, linux-arm-kernel@lists.infradead.org, peng.fan@nxp.com, bcm-kernel-feedback-list@broadcom.com, florian.fainelli@broadcom.com Subject: Re: [PATCH] firmware: arm_scmi: Queue in scmi layer for mailbox implementation Message-ID: References: <20241004221257.2888603-1-justin.chen@broadcom.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20241004221257.2888603-1-justin.chen@broadcom.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20241007_053502_067414_47E84EAD X-CRM114-Status: GOOD ( 15.39 ) 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 On Fri, Oct 04, 2024 at 03:12:57PM -0700, Justin Chen wrote: > The mailbox layer has its own queue. However this confuses the per > message timeouts since the clock starts ticking the moment the messages > get queued up. So all messages in the queue have there timeout clocks > ticking instead of only the message inflight. To fix this, lets move the > queue back into the SCMI layer. Hi Justin, thanks for reporting this, I'll try to understand better in the following since we always avoided a lock in the mailbox driver (till now) since it seemed not to be needed due to the interaction and inner workings of the mailbox subsytem and its queues....so I just want to understand better...not saying that this patch is wrong by itself for now... when you say: "this confuses the per-message timeouts since the clock starts ticking the moment the message get queued up." ...you are referring to the core mailbox subsystem queues and the code flow that derives from mbox_send_message() right ? So roughly the mailbox subsystem callchain involved would be this: mbox_send_message() add_to_rbuf() msg_submit() Am I right ? Is this the code flow you are referring to m? In this case I am not sure what you mean with "the timer starts ticking as soon as the message is queued"...which timer ? ... because: - the SCMI mailbox client is registered as "tx_block = false" so the sent messages are never waited for with wait_for_completion and on the other side - once we have 1 message in-flight already, the next attempt to send a message will result in a call to add_to_rbuf() BUT the following msg_submit() will bailout immediately at line 63 if (!chan->msg_count || chan->active_req) goto exit; after the first message is queued since there is an active_req inflight, and so no more messages will be queued till the next tx_tick(), which is invoked only by us from the SCMI stack when the reply to the inflight message has been read back. (this also means tx_prepare is NOT called until we kick from SCMI MBOX driver with mbox_client_txdone() What I am missing, here ? Thanks, Cristian