From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 272892F4A15; Thu, 19 Feb 2026 23:59:50 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1771545591; cv=none; b=bP2YXFVTb3v67wT1dQWd3BC5kRmHSEihoAsF6JQiQvdA4Jp46qh1g2wQ8excGaMNHOHdMPmOL4GRYQhvF0NtqVMCUJdRobwtDdLUIS1lH8MtAS8mTxv0n4c0T3iqn3EvupG6QlHJBJpk32cYhso8XTVxjht74e390cH/sR0irZk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1771545591; c=relaxed/simple; bh=ZMzYxL94z8q0TJyvNhpiuIgpQ1hKazr0tfXAbomaJd8=; h=Mime-Version:Content-Type:Date:Message-Id:Subject:Cc:To:From: References:In-Reply-To; b=PGgbC9p3CfsS1sX5SyEx0K6C3iooqDZbhDSUCmtmWv/c0sH3unF2sg6RGGf/IhCzhU2WuFl6f1LiHydjQO1G7gFuV0eJaF6/o8/PdEW8nF3mAdVwqwA+bd7JY1fEpTj0fF5ixA6lz6Eq4LJ6VjN/gk5umMxANjFZVoCVnM2DLa0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=oU3BrviT; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="oU3BrviT" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3F3BBC4CEF7; Thu, 19 Feb 2026 23:59:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1771545590; bh=ZMzYxL94z8q0TJyvNhpiuIgpQ1hKazr0tfXAbomaJd8=; h=Date:Subject:Cc:To:From:References:In-Reply-To:From; b=oU3BrviTxQgpucmePP3fA6Qy04sAxAocBTXaGBNbp12HervoLWm+VmBJ5hNq4fzdp dFTDstOoHfXpG9EcQsOBajZN5jYCs56hg535NMq1Z+l95v9+9v5DzsxKn+13hSpW8R 7H8I/BKIMsOYmm/8TskUgBgsAxKAvetR9Y0rFWli9JXE9tdVsEJuQ56zNauG3qbA/1 TvThSUrlro3U/abDVryk99nkNQZFBvaM918dHud/DPKT/jgcwiDOAJB1NaPpRWVNp4 u9Eri/MFKhkKHYx724FyDD5F3d/KvYwlgPU2E8oQSABPQ/iCkI5q1ZN0vOdIJAjgGb wrB7Ulm2y9aBg== Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Fri, 20 Feb 2026 00:59:45 +0100 Message-Id: Subject: Re: [PATCH V2] driver core: auxiliary bus: Fix sysfs creation on bind Cc: "Greg Kroah-Hartman" , "Rafael J. Wysocki" , "Saeed Mahameed" , "Leon Romanovsky" , "Mark Bloch" , "Andrew Lunn" , "David S. Miller" , "Eric Dumazet" , "Jakub Kicinski" , "Paolo Abeni" , , , , , "Gal Pressman" , "Moshe Shemesh" , "Amir Tzin" , "Leon Romanovsky" To: "Tariq Toukan" From: "Danilo Krummrich" References: <20260219210435.1769394-1-tariqt@nvidia.com> In-Reply-To: <20260219210435.1769394-1-tariqt@nvidia.com> On Thu Feb 19, 2026 at 10:04 PM CET, Tariq Toukan wrote: > +/** > + * auxiliary_device_sysfs_irq_dir_init - initialize the IRQ sysfs direct= ory > + * @auxdev: auxiliary bus device to initialize the sysfs directory. > + * > + * This function should be called by drivers to initialize the IRQ direc= tory > + * before adding any IRQ sysfs entries. The driver is responsible for en= suring > + * this function is called only once and for handling any concurrency co= ntrol > + * if needed. > + * > + * Drivers must call auxiliary_device_sysfs_irq_dir_destroy() to clean u= p when > + * done. > + * > + * Return: zero on success or an error code on failure. > + */ > +int auxiliary_device_sysfs_irq_dir_init(struct auxiliary_device *auxdev) > { > - int ret =3D 0; > - > - guard(mutex)(&auxdev->sysfs.lock); > - if (auxdev->sysfs.irq_dir_exists) > - return 0; > + int ret; > =20 > - ret =3D devm_device_add_group(&auxdev->dev, &auxiliary_irqs_group); > + ret =3D sysfs_create_group(&auxdev->dev.kobj, &auxiliary_irqs_group); > if (ret) > return ret; > =20 > - auxdev->sysfs.irq_dir_exists =3D true; > xa_init(&auxdev->sysfs.irqs); > return 0; > } > +EXPORT_SYMBOL_GPL(auxiliary_device_sysfs_irq_dir_init); > + > +/** > + * auxiliary_device_sysfs_irq_dir_destroy - destroy the IRQ sysfs direct= ory > + * @auxdev: auxiliary bus device to destroy the sysfs directory. > + * > + * This function should be called by drivers to clean up the IRQ directo= ry > + * after all IRQ sysfs entries have been removed. The driver is responsi= ble > + * for ensuring all IRQs are removed before calling this function. > + */ > +void auxiliary_device_sysfs_irq_dir_destroy(struct auxiliary_device *aux= dev) > +{ > + xa_destroy(&auxdev->sysfs.irqs); > + sysfs_remove_group(&auxdev->dev.kobj, &auxiliary_irqs_group); > +} > +EXPORT_SYMBOL_GPL(auxiliary_device_sysfs_irq_dir_destroy); > =20 > /** > * auxiliary_device_sysfs_irq_add - add a sysfs entry for the given IRQ > @@ -45,7 +70,8 @@ static int auxiliary_irq_dir_prepare(struct auxiliary_d= evice *auxdev) > * @irq: The associated interrupt number. > * > * This function should be called after auxiliary device have successful= ly > - * received the irq. > + * received the irq. The driver must call auxiliary_device_sysfs_irq_dir= _init() > + * before calling this function for the first time. I'm not convinced by this approach. This adds two new sources of bugs for drivers. 1. Drivers can now forget to call auxiliary_device_sysfs_irq_dir_init() *before* auxiliary_device_sysfs_irq_add(). 2. Drivers can forget to call auxiliary_device_sysfs_irq_dir_destroy(). Instead, I suggest to keep the current approach and just replace devm_device_add_group() with devm_auxiliary_device_add_group(), which in it= s devres callback additionally clears auxdev->sysfs.irq_dir_exists. In terms of the auxdev->sysfs.lock, I think this can still be removed, as i= t wasn't needed in the first place. auxiliary_device_sysfs_irq_add() must only be called from a scope where the auxiliary device is guaranteed to be bound, so there can't be a concurrent unbind. There may only be multiple concurrent calls to auxiliary_device_sysfs_irq_a= dd() itself, and in this case irq_dir_exists can just be an atomic. Yes, we're still stuck with an atomic for irq_dir_exists, but the driver AP= I remains much simpler and less error prone. - Danilo