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 51E62CA5FE3 for ; Fri, 2 Oct 2026 23:26:14 +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: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:References: List-Owner; bh=Vf+uPb5Ka6V5L11YRJYprfpyKFQk3KpeYeFlGC0uHZM=; b=bGWHgBqoNfLCA7 eR7dtxygohwHWtRNVBodCaEtIlQqHOi5K7NftymVRVCOMXMx+Obmbc9eJ9Z7fHNOd4H4YsVadfDDa mGdtS1qsp7w5eLiknRx5emLjCiuAwuXjJWv2H40QIVJAQ2KqO78CzixjDjRTHzqlFiNpMMOlSZA3w AMGKAC8ho7piH1pefpkbO+3nr/NFceLszFLjQWtWlRNEAaKEjchAT4h0aFRBqPpm/XQFK+XPTdxH9 jscg8m8CqLRfMbnrqlFr0PDaz2XVK/k2pwRbSgyVxLP3Zz/EXjqHDUA8ivO9VKWTBbFnJxozzJ3OG dT56958C1J8PImOYOlyw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.99.1 #2 (Red Hat Linux)) id 1xCmdj-0000000Cgm0-0qq0; Fri, 02 Oct 2026 23:26:07 +0000 Received: from tor.source.kernel.org ([172.105.4.254]) by bombadil.infradead.org with esmtps (Exim 4.99.1 #2 (Red Hat Linux)) id 1xCmdi-0000000Cgll-2SI4; Fri, 02 Oct 2026 23:26:06 +0000 Received: from smtp.kernel.org (quasi.space.kernel.org [100.103.45.18]) by tor.source.kernel.org (Postfix) with ESMTP id 84D7E60214; Fri, 2 Oct 2026 23:26:05 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0CFA21F00893; Fri, 2 Oct 2026 23:26:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1790983565; bh=qu9dEtP9YAh4yNiAM94k1GvC1NXBsCRK7l37Wy+2FEU=; h=Date:From:To:Cc:Subject:In-Reply-To; b=Jv2UEINNPTvxl6/MPEAJB5TLj2CtWVtUaWt1PnB2eSs3XfpFRY4Cy+Vne+ttmP/H6 Q32lJBAGdwM+eOBEvMbJtdGp8UcrqoQ+5yYTGTvOUqotijZ8xB6/JjbMoD2Nyor79L 3B/O0w29eVwrMC0fZFqD16EuRbN7DWW+pxmivi0L5vZ7H7qWIXkaKSOXhPnXX0pLoc TGOHMazC+qltWb0nL1TC+OM3xfxQ26gQlWl1b6VV4mE9jGTte8E7DXxqk5QPF8rfeE ctgWXgYJCN+534DL1CWDqSitYUxiliZdgxGc344IGBH4yX8u1WoDsR6x+o+RkzLXwj d7qPkvu1XC18Q== Date: Fri, 2 Oct 2026 18:26:03 -0500 From: Bjorn Helgaas To: Shuvam Pandey Cc: Jingoo Han , Manivannan Sadhasivam , Lorenzo Pieralisi , Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= , Bjorn Helgaas , Yue Wang , Neil Armstrong , Rob Herring , Kevin Hilman , Jerome Brunet , Martin Blumenstingl , Fan Ni , Shradha Todi , Hanjie Lin , linux-pci@vger.kernel.org, linux-amlogic@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/2] PCI: meson: Add missing remove callback Message-ID: <20261002232603.GA397281@bhelgaas> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1a0c86ab264cdc1c79c917e984b90991af51d827.1779123847.git.shuvampandey1@gmail.com> X-BeenThere: linux-amlogic@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-amlogic" Errors-To: linux-amlogic-bounces+linux-amlogic=archiver.kernel.org@lists.infradead.org On Mon, May 18, 2026 at 10:44:18PM +0545, Shuvam Pandey wrote: > meson_pcie_probe() powers on the PHY and registers the DesignWare host > bridge with dw_pcie_host_init(), but the driver has no remove callback. > On driver unbind or module unload, the driver core therefore proceeds to > devres cleanup without first unregistering the host bridge or powering off > the PHY. > > Add a remove callback that deinitializes the DesignWare host bridge and > powers off the PHY while device-managed resources are still valid. > > Fixes: 9c0ef6d34fdb ("PCI: amlogic: Add the Amlogic Meson PCIe controller driver") > Signed-off-by: Shuvam Pandey > --- > drivers/pci/controller/dwc/pci-meson.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c > index 0694084f6..c96e2244a 100644 > --- a/drivers/pci/controller/dwc/pci-meson.c > +++ b/drivers/pci/controller/dwc/pci-meson.c > @@ -451,6 +451,14 @@ static int meson_pcie_probe(struct platform_device *pdev) > return ret; > } > > +static void meson_pcie_remove(struct platform_device *pdev) > +{ > + struct meson_pcie *mp = platform_get_drvdata(pdev); > + > + dw_pcie_host_deinit(&mp->pci.pp); > + meson_pcie_power_off(mp); FYI, from internal Sashiko review while backporting this commit (4b0dc84b2939 ("PCI: meson: Add missing remove callback")): [Severity: Medium] Will this manual call to meson_pcie_power_off() execute too early, before devres tears down the clocks? During probe, the initialization order powers on the PHY before enabling theclocks. LIFO order requires disabling the clocks before powering off the PHY.Because meson_pcie_probe_clocks() uses devm_add_action_or_reset() to manage the clocks, they will be disabled after meson_pcie_remove() returns. Could this cause a hardware hang or undefined behavior during driver unbind due to the clock and power sequencing violation? [Severity: High] This is a pre-existing issue, but does this remove function also need to call reset_control_assert() to avoid leaking the shared reset deassert count? During probe, meson_pcie_get_resets() obtains a shared reset and manually increments its deassert count: mrst->apb = meson_pcie_get_reset(mp, "apb", PCIE_SHARED_RESET); if (IS_ERR(mrst->apb)) return PTR_ERR(mrst->apb); reset_control_deassert(mrst->apb); Because the devm reset getters used here do not automatically assert on cleanup, omitting reset_control_assert() in meson_pcie_remove() means the apb shared reset will permanently have a non-zero deassert_count. This prevents it from ever being asserted again, even if all consumers unbind. > +} > + > static const struct of_device_id meson_pcie_of_match[] = { > { > .compatible = "amlogic,axg-pcie", > @@ -464,6 +472,7 @@ MODULE_DEVICE_TABLE(of, meson_pcie_of_match); > > static struct platform_driver meson_pcie_driver = { > .probe = meson_pcie_probe, > + .remove = meson_pcie_remove, > .driver = { > .name = "meson-pcie", > .of_match_table = meson_pcie_of_match, > -- > 2.50.1 (Apple Git-155) > > > _______________________________________________ > linux-amlogic mailing list > linux-amlogic@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-amlogic _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic