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 033CDCEDD8F for ; Wed, 9 Oct 2024 12:39:19 +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-Transfer-Encoding:Content-Type:MIME-Version:References:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=g/ZeGqBLM2fiX7mXh93r2ET7MjM8w7cTuZzyp9PfL28=; b=OnZD4bN9YwV9CyNWLhlZ++bqWr GeFkaF5NjPyyCUFIu1wFVprxMQYnr7z8+bepiiOazYlF9/W1QiJPcHUlb8eEwehFY70Fe2WkmG8k3 vZ/kv+fc6V8JO1KTJfnmfjkm8NyX3ZFZubQEQonYW7jWbcavdAlerFc+U68pPG+7LFO4UAIkoAxC6 9y/XRKvdF+Y/z+vxgmBuhG2dAjfM6IVE/cnerNtIs//jqwD6DpIe3Ypmu/5xUVzkwQuTSpF9X579y yNMtkaZ0ZXb1qvHZtYh4TKlOWR5lY3H0c9tWOaRv7LX6EPEBRLGqWmzhExQhuFlCf8Nf9tXkwqaYw X/Rt9/VA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1syVy7-00000009El6-42AA; Wed, 09 Oct 2024 12:39:07 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1syVwm-00000009EUv-2tYb for linux-arm-kernel@lists.infradead.org; Wed, 09 Oct 2024 12:37:46 +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 1C684FEC; Wed, 9 Oct 2024 05:38:13 -0700 (PDT) Received: from bogus (e133711.arm.com [10.1.196.77]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 3E49A3F58B; Wed, 9 Oct 2024 05:37:41 -0700 (PDT) Date: Wed, 9 Oct 2024 13:37:38 +0100 From: Sudeep Holla To: Florian Fainelli Cc: Cristian Marussi , linux-arm-kernel@lists.infread.org, Rob Herring , Krzysztof Kozlowski , Sudeep Holla , Conor Dooley , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , open list , "open list:SYSTEM CONTROL & POWER/MANAGEMENT INTERFACE" , "moderated list:SYSTEM CONTROL & POWER/MANAGEMENT INTERFACE" , justin.chen@broadcom.com, opendmb@gmail.com, kapil.hali@broadcom.com, bcm-kernel-feedback-list@broadcom.com, Arnd Bergmann Subject: Re: [PATCH] firmware: arm_scmi: Give SMC transport precedence over mailbox Message-ID: References: <20241006043317.3867421-1-florian.fainelli@broadcom.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20241009_053744_839896_0DC51FFA X-CRM114-Status: GOOD ( 37.81 ) 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 Tue, Oct 08, 2024 at 10:49:01AM -0700, Florian Fainelli wrote: > On 10/8/24 06:06, Sudeep Holla wrote: [...] > > IIUC, you need to support old kernel with SMC mailbox driver and new SMC > > transport within the SCMI. Is that right understanding ? > > That is correct. > Thanks. > > IIUC, the DTB has mailbox nodes that are available but fail only in the setup > > stage ? Or is it marked unavailable and we are missing some checks either > > in SCMI or mailbox ? > > We fail at scmi_chan_setup -> idr_find() returning -EINVAL. IIRC, the original intention of code under if(!info->desc->ops->chan_available() in scmi_chan_setup(), is to just handle Rx case and valid Tx missing case for non BASE protocols. I wonder if we can add additional check at the start of this if block: if (tx && prot_id == SCMI_PROTOCOL_BASE) return -ENODEV or -ENOENT; just to better handle your scenario. But IIUC it may not fix your issue as we still return success from the platform_device probing with the new restructuring. It is only the probe of the device we create from this one can be made to fail. I think I know understand the issue much better than before. > I did check that > returning -ENODEV, which arguably might be a somewhat more accurate return > code (-ENOENT being one, too) does not help us here. Cristian suggested > device_release_driver() which sounded like a good idea, but will deadlock. > Cristian mentioned in private he will explore other options just in case we need solid alternative to address your issue. > The reason why we fail there is because mailbox_chan_available() returns > false. With fw_devlink=on Linux will parse the Device Tree, find the > 'mboxes' property pointing to the brcm_scmi_mailbox Device Tree node and > puts it on a list of providers that it is waiting for. > > Because we are using the ARM SMC transport however, the brcm_scmi_mailbox > node is never backed by any driver in Linux and this causes the system to > fail booting since we do not have any SCMI provider. At the time, because we > were under pressure to get a GKI kernel we decided to "break" our older > downstream kernels by doing this property rename and put in a patch in those > kernel to treat "brcm,mboxes" the same as "mboxes" where relevant, which was > mostly in SCMI. > > Now, assuming that we revert that DT property rename, that still does not > really solve anything anyway, the channel is not available regardless of how > we shake it. > Understood. Thanks for detailed explanation and time. > > > > IOW, have you already explored that this -EINVAL is correct return value > > here and can't be changed to -ENODEV ? I might be not following the failure > > path correctly here, but I assume it is > > scmi_chan_setup() > > info->desc->ops->chan_setup() > > mailbox_chan_setup() > > mbox_request_channel() [...] > > OK this sounds like you have already explored returning -ENODEV is not > > an option ? It is fair enough, but just want to understand correctly. > > I still think I am missing something. > > Yes, that was my first start. > Now I know why that won't work or its not so trivial as we have some kind of redirection to address various transport dependencies and the mechanism we have implemented to avoid probe deferral with additional platform devices and associated probing make it difficult to propagate the error of DD model. We need that to handle the dependency better than relying on probe deferral which comes with its problems(like initcall level adjustments when using some transports like OPTEE/FFA/virtio/...). Your issue is not an issue in normal case 😄. Anyways, I will queue this as it is not affecting anyone else. Hopefully no one has any objections. -- Regards, Sudeep