From patchwork Thu Dec 22 18:50:42 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Cristian Marussi X-Patchwork-Id: 13080227 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 6AEA2C4332F for ; Thu, 22 Dec 2022 18:52:48 +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:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version: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=ugyzqjt3Nb8NGwh8finn18hAKHdA9urPxgS1siydDOc=; b=tHs5wudlyT9Zik Vl+xMMER7AlrFTyCPxplRc7mM4jj1LMzCx8kf9FO/He4V6fKAEtIqpzUZooly4ioqoAEHZRoGjkUV 0NfqFeQvKWLmAyKNXU4jvNzAjimAKK0IkgNwGuYvcY0vhyhS/M5sRcu5l60m4jQNkAOz5f2fTBgmA 0Bz0JstK/spYL4lAO09FkepJNh8UCF8sA9iblPMxwfkU6NfWlK2Y7WKISIjzUH6i9w3k+kUltd5n8 v+LQkPIlq0R+eQPGWf1OaIA+oJybQvKbpzUhMFzJYeIz1BUNl/qgZ5qqYo0C0DpjXFm1xPXxYGnk7 5V1/iJ0uOzTdvxFaQI8g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1p8QfQ-00FdV8-U4; Thu, 22 Dec 2022 18:51:45 +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 1p8Qes-00FdBE-B6 for linux-arm-kernel@lists.infradead.org; Thu, 22 Dec 2022 18:51:13 +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 94FEDAD7; Thu, 22 Dec 2022 10:51:49 -0800 (PST) Received: from e120937-lin.. (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 23A133FA32; Thu, 22 Dec 2022 10:51:07 -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, f.fainelli@gmail.com, etienne.carriere@linaro.org, vincent.guittot@linaro.org, Ludvig.Parsson@axis.com, cristian.marussi@arm.com Subject: [PATCH 2/9] firmware: arm_scmi: Use dedicated devices to initialize channels Date: Thu, 22 Dec 2022 18:50:42 +0000 Message-Id: <20221222185049.737625-3-cristian.marussi@arm.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20221222185049.737625-1-cristian.marussi@arm.com> References: <20221222185049.737625-1-cristian.marussi@arm.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221222_105111_267105_1EC988D0 X-CRM114-Status: GOOD ( 22.18 ) 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 Refactor channels initialization to use dedicated transport devices instead of using devices borrowed from the SCMI drivers. Initialize all channels, as described in the DT, upfront during SCMI core stack probe phase and free all of them, including the underlying devices, when the SCMI core is removed. Signed-off-by: Cristian Marussi --- drivers/firmware/arm_scmi/common.h | 1 - drivers/firmware/arm_scmi/driver.c | 158 ++++++++++++++++++++-------- drivers/firmware/arm_scmi/mailbox.c | 2 - drivers/firmware/arm_scmi/optee.c | 2 - drivers/firmware/arm_scmi/smc.c | 2 - drivers/firmware/arm_scmi/virtio.c | 2 - 6 files changed, 116 insertions(+), 51 deletions(-) diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h index 16db1e177123..136bfd30f99c 100644 --- a/drivers/firmware/arm_scmi/common.h +++ b/drivers/firmware/arm_scmi/common.h @@ -229,7 +229,6 @@ extern const struct scmi_desc scmi_optee_desc; #endif void scmi_rx_callback(struct scmi_chan_info *cinfo, u32 msg_hdr, void *priv); -void scmi_free_channel(struct scmi_chan_info *cinfo, struct idr *idr, int id); /* shmem related declarations */ struct scmi_shared_mem; diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index f1ebadffdfe4..f85b34e69d01 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -1987,23 +1987,20 @@ static int scmi_xfer_info_init(struct scmi_info *sinfo) return ret; } -static int scmi_chan_setup(struct scmi_info *info, struct device *dev, +static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node, int prot_id, bool tx) { int ret, idx; + char name[32]; struct scmi_chan_info *cinfo; struct idr *idr; + struct scmi_device *tdev = NULL; /* Transmit channel is first entry i.e. index 0 */ idx = tx ? 0 : 1; idr = tx ? &info->tx_idr : &info->rx_idr; - /* check if already allocated, used for multiple device per protocol */ - cinfo = idr_find(idr, prot_id); - if (cinfo) - return 0; - - if (!info->desc->ops->chan_available(dev->of_node, idx)) { + if (!info->desc->ops->chan_available(of_node, idx)) { cinfo = idr_find(idr, SCMI_PROTOCOL_BASE); if (unlikely(!cinfo)) /* Possible only if platform has no Rx */ return -EINVAL; @@ -2014,27 +2011,49 @@ static int scmi_chan_setup(struct scmi_info *info, struct device *dev, if (!cinfo) return -ENOMEM; - cinfo->dev = dev; cinfo->rx_timeout_ms = info->desc->max_rx_timeout_ms; + /* Create a unique name for this transport device */ + snprintf(name, 32, "__scmi_transport_device_%s_%02X", + idx ? "rx" : "tx", prot_id); + /* Create a uniquely named, dedicated transport device for this chan */ + tdev = scmi_device_create(of_node, info->dev, prot_id, name); + if (!tdev) { + devm_kfree(info->dev, cinfo); + return -EINVAL; + } + of_node_get(of_node); + + cinfo->dev = &tdev->dev; ret = info->desc->ops->chan_setup(cinfo, info->dev, tx); - if (ret) + if (ret) { + of_node_put(of_node); + scmi_device_destroy(tdev); + devm_kfree(info->dev, cinfo); return ret; + } if (tx && is_polling_required(cinfo, info)) { if (is_transport_polling_capable(info)) - dev_info(dev, + dev_info(&tdev->dev, "Enabled polling mode TX channel - prot_id:%d\n", prot_id); else - dev_warn(dev, + dev_warn(&tdev->dev, "Polling mode NOT supported by transport.\n"); } idr_alloc: ret = idr_alloc(idr, cinfo, prot_id, prot_id + 1, GFP_KERNEL); if (ret != prot_id) { - dev_err(dev, "unable to allocate SCMI idr slot err %d\n", ret); + dev_err(info->dev, + "unable to allocate SCMI idr slot err %d\n", ret); + /* Destroy channel and device only if created by this call. */ + if (tdev) { + of_node_put(of_node); + scmi_device_destroy(tdev); + devm_kfree(info->dev, cinfo); + } return ret; } @@ -2043,13 +2062,14 @@ static int scmi_chan_setup(struct scmi_info *info, struct device *dev, } static inline int -scmi_txrx_setup(struct scmi_info *info, struct device *dev, int prot_id) +scmi_txrx_setup(struct scmi_info *info, struct device_node *of_node, + int prot_id) { - int ret = scmi_chan_setup(info, dev, prot_id, true); + int ret = scmi_chan_setup(info, of_node, prot_id, true); if (!ret) { /* Rx is optional, report only memory errors */ - ret = scmi_chan_setup(info, dev, prot_id, false); + ret = scmi_chan_setup(info, of_node, prot_id, false); if (ret && ret != -ENOMEM) ret = 0; } @@ -2057,6 +2077,54 @@ scmi_txrx_setup(struct scmi_info *info, struct device *dev, int prot_id) return ret; } +/** + * scmi_channels_setup - Helper to initialize all required channels + * + * @info: The SCMI instance descriptor. + * + * Initialize all the channels found described in the DT against the underlying + * configured transport using custom defined dedicated devices instead of + * borrowing devices from the SCMI drivers; this way channels are initialized + * upfront during core SCMI stack probing and are no more coupled with SCMI + * devices used by SCMI drivers. + * + * Note that, even though a pair of TX/RX channels is associated to each + * protocol defined in the DT, a distinct freshly initialized channel is + * created only if the DT node for the protocol at hand describes a dedicated + * channel: in all the other cases the common BASE protocol channel is reused. + * + * Return: 0 on Success + */ +static int scmi_channels_setup(struct scmi_info *info) +{ + int ret; + struct device_node *child, *top_np = info->dev->of_node; + + /* Initialize a common generic channel at first */ + ret = scmi_txrx_setup(info, top_np, SCMI_PROTOCOL_BASE); + if (ret) + return ret; + + for_each_available_child_of_node(top_np, child) { + u32 prot_id; + + if (of_property_read_u32(child, "reg", &prot_id)) + continue; + + if (!FIELD_FIT(MSG_PROTOCOL_ID_MASK, prot_id)) + dev_err(info->dev, + "Out of range protocol %d\n", prot_id); + + ret = scmi_txrx_setup(info, child, prot_id); + if (ret) { + of_node_put(child); + return ret; + } + } + + return 0; +} + /** * scmi_get_protocol_device - Helper to get/create an SCMI device. * @@ -2106,14 +2174,6 @@ scmi_get_protocol_device(struct device_node *np, struct scmi_info *info, return NULL; } - if (scmi_txrx_setup(info, &sdev->dev, prot_id)) { - dev_err(&sdev->dev, "failed to setup transport\n"); - scmi_device_destroy(sdev); - mutex_unlock(&scmi_syspower_mtx); - - return NULL; - } - if (prot_id == SCMI_PROTOCOL_SYSTEM) scmi_syspower_registered = true; @@ -2347,19 +2407,39 @@ void scmi_protocol_device_unrequest(const struct scmi_device_id *id_table) mutex_unlock(&scmi_requested_devices_mtx); } -static int scmi_cleanup_txrx_channels(struct scmi_info *info) +static int scmi_chan_destroy(int id, void *p, void *idr) { - int ret; - struct idr *idr = &info->tx_idr; + struct scmi_chan_info *cinfo = p; + + if (cinfo->dev) { + struct scmi_device *sdev = to_scmi_dev(cinfo->dev); - ret = idr_for_each(idr, info->desc->ops->chan_free, idr); - idr_destroy(&info->tx_idr); + of_node_put(cinfo->dev->of_node); + scmi_device_destroy(sdev); + cinfo->dev = NULL; + } - idr = &info->rx_idr; - ret = idr_for_each(idr, info->desc->ops->chan_free, idr); - idr_destroy(&info->rx_idr); + idr_remove(idr, id); - return ret; + return 0; +} + +static void scmi_cleanup_channels(struct scmi_info *info, struct idr *idr) +{ + /* At first free all channels at the transport layer ... */ + idr_for_each(idr, info->desc->ops->chan_free, idr); + + /* ...then destroy all underlying devices */ + idr_for_each(idr, scmi_chan_destroy, idr); + + idr_destroy(idr); +} + +static void scmi_cleanup_txrx_channels(struct scmi_info *info) +{ + scmi_cleanup_channels(info, &info->tx_idr); + + scmi_cleanup_channels(info, &info->rx_idr); } static int scmi_probe(struct platform_device *pdev) @@ -2411,7 +2491,8 @@ static int scmi_probe(struct platform_device *pdev) return ret; } - ret = scmi_txrx_setup(info, dev, SCMI_PROTOCOL_BASE); + /* Setup all channels described in the DT at first */ + ret = scmi_channels_setup(info); if (ret) return ret; @@ -2481,14 +2562,9 @@ static int scmi_probe(struct platform_device *pdev) return ret; } -void scmi_free_channel(struct scmi_chan_info *cinfo, struct idr *idr, int id) -{ - idr_remove(idr, id); -} - static int scmi_remove(struct platform_device *pdev) { - int ret, id; + int id; struct scmi_info *info = platform_get_drvdata(pdev); struct device_node *child; @@ -2510,9 +2586,7 @@ static int scmi_remove(struct platform_device *pdev) idr_destroy(&info->active_protocols); /* Safe to free channels since no more users */ - ret = scmi_cleanup_txrx_channels(info); - if (ret) - dev_warn(&pdev->dev, "Failed to cleanup SCMI channels.\n"); + scmi_cleanup_txrx_channels(info); return 0; } diff --git a/drivers/firmware/arm_scmi/mailbox.c b/drivers/firmware/arm_scmi/mailbox.c index c33dcadafea8..0d9c9538b7f4 100644 --- a/drivers/firmware/arm_scmi/mailbox.c +++ b/drivers/firmware/arm_scmi/mailbox.c @@ -120,8 +120,6 @@ static int mailbox_chan_free(int id, void *p, void *data) smbox->cinfo = NULL; } - scmi_free_channel(cinfo, data, id); - return 0; } diff --git a/drivers/firmware/arm_scmi/optee.c b/drivers/firmware/arm_scmi/optee.c index 5a11091c72da..929720387102 100644 --- a/drivers/firmware/arm_scmi/optee.c +++ b/drivers/firmware/arm_scmi/optee.c @@ -481,8 +481,6 @@ static int scmi_optee_chan_free(int id, void *p, void *data) cinfo->transport_info = NULL; channel->cinfo = NULL; - scmi_free_channel(cinfo, data, id); - return 0; } diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/smc.c index 122128d56d2f..93272e4bbd12 100644 --- a/drivers/firmware/arm_scmi/smc.c +++ b/drivers/firmware/arm_scmi/smc.c @@ -171,8 +171,6 @@ static int smc_chan_free(int id, void *p, void *data) cinfo->transport_info = NULL; scmi_info->cinfo = NULL; - scmi_free_channel(cinfo, data, id); - return 0; } diff --git a/drivers/firmware/arm_scmi/virtio.c b/drivers/firmware/arm_scmi/virtio.c index a917eca7d902..d68c01cb7aa0 100644 --- a/drivers/firmware/arm_scmi/virtio.c +++ b/drivers/firmware/arm_scmi/virtio.c @@ -489,8 +489,6 @@ static int virtio_chan_free(int id, void *p, void *data) virtio_break_device(vioch->vqueue->vdev); scmi_vio_channel_cleanup_sync(vioch); - scmi_free_channel(cinfo, data, id); - return 0; }