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 E35E4C25B75 for ; Wed, 15 May 2024 09:58:34 +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:In-Reply-To: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=tsS2aFq4LZg429RCVYQUseA22TYd901rc03R+vM8mro=; b=1s+v1uuMxs/5cy ae0humYtC4k1W7nM96OgWDQ50CuC/rR5ERaDHWXVA95lA+cN7mQ9jXNjzreGQH3GjziUa5ZNnZPOq NXDmHvXvHkMP2C3Jt3AIpWfPOLQQZcicqvJnwvtKpc/hd56zRiBdXUtFl62DZNxBXtF+yMtiR6hjq 2Uo0SmGfqcn+rT/F5MjEc+0iWfHzPxbFwVheHpkG6qPzSGy4TVO0JJphFMvKKuK9iDIL9YGgMxiUq VxreNdKABEcGuT2oCv1XfydheCO0C1Vj1uPqdm7vPPNNCunZbaiTUpFI7GO3Dc3oN/pcjNv8tVq9Y rM9xUuBiWKOsfbb0BD7Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1s7BOy-000000019t9-2ZDk; Wed, 15 May 2024 09:58:24 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1s7BOv-000000019se-2W1J for linux-arm-kernel@lists.infradead.org; Wed, 15 May 2024 09:58:23 +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 CCAF01007; Wed, 15 May 2024 02:58:44 -0700 (PDT) Received: from bogus (e103737-lin.cambridge.arm.com [10.1.197.49]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 693D63F762; Wed, 15 May 2024 02:58:19 -0700 (PDT) Date: Wed, 15 May 2024 10:58:16 +0100 From: Sudeep Holla To: Sebastian Ene Cc: linux-arm-kernel@lists.infradead.org, Sudeep Holla , Lorenzo Pieralisi , Jens Wiklander Subject: Re: [PATCH 2/2] firmware: arm_ffa: Split bus and driver into distinct modules Message-ID: References: <20240510183811.3802285-1-sudeep.holla@arm.com> <20240510183811.3802285-2-sudeep.holla@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240515_025821_938639_6E4D56FC X-CRM114-Status: GOOD ( 34.95 ) 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: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, May 14, 2024 at 02:54:46PM +0000, Sebastian Ene wrote: > On Fri, May 10, 2024 at 07:38:11PM +0100, Sudeep Holla wrote: > > Make the FF-A bus on its own as a distinct module initialized at > > subsys_initcall level when builtin. > > > > Keep the FF-A driver core stack, together with any configured transport, > > in a different module initialized as module_init level. > > > > FF-A drivers initialization is now changed to module_init level. > > > > Signed-off-by: Sudeep Holla > > --- > > drivers/firmware/arm_ffa/Makefile | 6 ++++-- > > drivers/firmware/arm_ffa/bus.c | 22 ++++++++++++++++------ > > drivers/firmware/arm_ffa/common.h | 2 -- > > drivers/firmware/arm_ffa/driver.c | 12 ++---------- > > 4 files changed, 22 insertions(+), 20 deletions(-) > > > > Hi Sebastian, > > > > I reworked the patch I shared with you in private to include handling of > > the FF-A v1.0 workaround I had removed as a quick hack when I shared the > > patch. Please check if these couple of patches help you in the initcall > > order issue you were facing. > > > > Regards, > > Sudeep > > Hello Sudeep, > > > Thank for sharing the patch. I gave it a spin with pKVM and OPTee under > Qemu and it works well. Having the bus driver initialize separately is a > good ideea. > Thanks for the review and testing. > > > > diff --git a/drivers/firmware/arm_ffa/Makefile b/drivers/firmware/arm_ffa/Makefile > > index 9d9f37523200..168990a7e792 100644 > > --- a/drivers/firmware/arm_ffa/Makefile > > +++ b/drivers/firmware/arm_ffa/Makefile > > @@ -2,5 +2,7 @@ > > ffa-bus-y = bus.o > > ffa-driver-y = driver.o > > ffa-transport-$(CONFIG_ARM_FFA_SMCCC) += smccc.o > > -ffa-module-objs := $(ffa-bus-y) $(ffa-driver-y) $(ffa-transport-y) > > -obj-$(CONFIG_ARM_FFA_TRANSPORT) = ffa-module.o > > +ffa-core-objs := $(ffa-bus-y) > > +ffa-module-objs := $(ffa-driver-y) $(ffa-transport-y) > > +obj-$(CONFIG_ARM_FFA_TRANSPORT) = ffa-core.o > > +obj-$(CONFIG_ARM_FFA_TRANSPORT) += ffa-module.o > > diff --git a/drivers/firmware/arm_ffa/bus.c b/drivers/firmware/arm_ffa/bus.c > > index 2f557e90f2eb..0c83931485f6 100644 > > --- a/drivers/firmware/arm_ffa/bus.c > > +++ b/drivers/firmware/arm_ffa/bus.c > > @@ -30,12 +30,11 @@ static int ffa_device_match(struct device *dev, struct device_driver *drv) > > while (!uuid_is_null(&id_table->uuid)) { > > /* > > * FF-A v1.0 doesn't provide discovery of UUIDs, just the > > - * partition IDs, so fetch the partitions IDs for this > > - * id_table UUID and assign the UUID to the device if the > > - * partition ID matches > > + * partition IDs, so match it unconditionally here and handle > > + * it via the installed bus notifier during driver binding. > > */ > > if (uuid_is_null(&ffa_dev->uuid)) > > - ffa_device_match_uuid(ffa_dev, &id_table->uuid); > > + return 1; > > > > if (uuid_equal(&ffa_dev->uuid, &id_table->uuid)) > > return 1; > > @@ -50,6 +49,10 @@ static int ffa_device_probe(struct device *dev) > > struct ffa_driver *ffa_drv = to_ffa_driver(dev->driver); > > struct ffa_device *ffa_dev = to_ffa_dev(dev); > > > > + /* UUID can be still NULL with FF-A v1.0, so just skip probing them */ > > + if (uuid_is_null(&ffa_dev->uuid)) > > + return -ENODEV; > > + > > return ffa_drv->probe(ffa_dev); > > } > > > > @@ -232,14 +235,21 @@ void ffa_device_unregister(struct ffa_device *ffa_dev) > > } > > EXPORT_SYMBOL_GPL(ffa_device_unregister); > > > > -int arm_ffa_bus_init(void) > > +static int __init arm_ffa_bus_init(void) > > { > > return bus_register(&ffa_bus_type); > > } > > +subsys_initcall(arm_ffa_bus_init); > > > > -void arm_ffa_bus_exit(void) > > +static void __exit arm_ffa_bus_exit(void) > > { > > ffa_devices_unregister(); > > bus_unregister(&ffa_bus_type); > > ida_destroy(&ffa_bus_id); > > } > > +module_exit(arm_ffa_bus_exit); > > + > > +MODULE_ALIAS("ffa-core"); > > +MODULE_AUTHOR("Sudeep Holla "); > > +MODULE_DESCRIPTION("ARM FF-A bus"); > > +MODULE_LICENSE("GPL"); > > diff --git a/drivers/firmware/arm_ffa/common.h b/drivers/firmware/arm_ffa/common.h > > index d6eccf1fd3f6..9c6425a81d0d 100644 > > --- a/drivers/firmware/arm_ffa/common.h > > +++ b/drivers/firmware/arm_ffa/common.h > > @@ -14,8 +14,6 @@ typedef struct arm_smccc_1_2_regs ffa_value_t; > > > > typedef void (ffa_fn)(ffa_value_t, ffa_value_t *); > > > > -int arm_ffa_bus_init(void); > > -void arm_ffa_bus_exit(void); > > bool ffa_device_is_valid(struct ffa_device *ffa_dev); > > void ffa_device_match_uuid(struct ffa_device *ffa_dev, const uuid_t *uuid); > > > > diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c > > index 159f2106a541..7cdd425c5fc4 100644 > > --- a/drivers/firmware/arm_ffa/driver.c > > +++ b/drivers/firmware/arm_ffa/driver.c > > @@ -1603,14 +1603,9 @@ static int __init ffa_init(void) > > if (ret) > > return ret; > > > > - ret = arm_ffa_bus_init(); > > - if (ret) > > - return ret; > > - > > drv_info = kzalloc(sizeof(*drv_info), GFP_KERNEL); > > if (!drv_info) { > > - ret = -ENOMEM; > > - goto ffa_bus_exit; > > + return -ENOMEM; > > } > > > > ret = ffa_version_check(&drv_info->version); > > @@ -1671,11 +1666,9 @@ static int __init ffa_init(void) > > free_pages_exact(drv_info->rx_buffer, RXTX_BUFFER_SIZE); > > free_drv_info: > > kfree(drv_info); > > -ffa_bus_exit: > > - arm_ffa_bus_exit(); > > return ret; > > } > > -subsys_initcall(ffa_init); > > +module_init(ffa_init); > > > > static void __exit ffa_exit(void) > > { > > @@ -1685,7 +1678,6 @@ static void __exit ffa_exit(void) > > free_pages_exact(drv_info->tx_buffer, RXTX_BUFFER_SIZE); > > free_pages_exact(drv_info->rx_buffer, RXTX_BUFFER_SIZE); > > kfree(drv_info); > > - arm_ffa_bus_exit(); > > } > > module_exit(ffa_exit); > > > > -- > > 2.43.2 > > > > Ack, Sebastian I wasn't sure if that meant Acked-by: Sebastian Ene So, I didn't add it intentionally in v2. Please ack/review v2 if you are happy with that. -- Regards, Sudeep _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel