[PATCH 1/2] PCI: dwc: Guard RAS DES debugfs deinit

[PATCH 1/2] PCI: dwc: Guard RAS DES debugfs deinit

[Shuvam Pandey]

dwc_pcie_rasdes_debugfs_init() returns success when the controller has
no RAS DES capability, leaving pci->debugfs->rasdes_info unset. The
common debugfs teardown path still calls
dwc_pcie_rasdes_debugfs_deinit(), which dereferences rasdes_info
unconditionally.

Return early when no RAS DES state was allocated. In that case no RAS DES
mutex was initialized, so there is nothing to destroy.

Fixes: 4fbfa17f9a07 ("PCI: dwc: Add debugfs based Silicon Debug support for DWC")
Signed-off-by: Shuvam Pandey <shuvampandey1@gmail.com>
---
 drivers/pci/controller/dwc/pcie-designware-debugfs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware-debugfs.c b/drivers/pci/controller/dwc/pcie-designware-debugfs.c
index d0884253b..c3671cb2f 100644
--- a/drivers/pci/controller/dwc/pcie-designware-debugfs.c
+++ b/drivers/pci/controller/dwc/pcie-designware-debugfs.c
@@ -557,6 +557,9 @@ static void dwc_pcie_rasdes_debugfs_deinit(struct dw_pcie *pci)
 {
 	struct dwc_pcie_rasdes_info *rinfo = pci->debugfs->rasdes_info;
 
+	if (!rinfo)
+		return;
+
 	mutex_destroy(&rinfo->reg_event_lock);
 }
 
-- 
2.50.1 (Apple Git-155)


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH 2/2] PCI: meson: Add missing remove callback

[Shuvam Pandey]

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 <shuvampandey1@gmail.com>
---
 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);
+}
+
 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

Re: (subset) [PATCH 2/2] PCI: meson: Add missing remove callback

[Manivannan Sadhasivam]

On Mon, 18 May 2026 22:44:18 +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.
> 
> [...]

Applied, thanks!

[2/2] PCI: meson: Add missing remove callback
      commit: 4b0dc84b293984f75598881809fb2d3daf54a2a8

Best regards,
-- 
Manivannan Sadhasivam <mani@kernel.org>


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 2/2] PCI: meson: Add missing remove callback

[Bjorn Helgaas]

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.

What's the user-visible effect of this?  Does it avoid an oops?
Reduce power usage?

Of the 34 instances of .probe() in drivers/pci/controller/dwc/, on 12
implement .remove(), so if this fixes a problem, I'm wondering whether
other drivers have the same problem.

> Fixes: 9c0ef6d34fdb ("PCI: amlogic: Add the Amlogic Meson PCIe controller driver")
> Signed-off-by: Shuvam Pandey <shuvampandey1@gmail.com>
> ---
>  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);
> +}
> +
>  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

Re: [PATCH 2/2] PCI: meson: Add missing remove callback

[Shuvam Pandey]

Hi Bjorn,

> What's the user-visible effect of this?  Does it avoid an oops?
> Reduce power usage?

I don't have hardware with this controller, so I have not observed an
oops from this.

The concrete effect is that driver unbind/module unload gets the
matching teardown for the successful probe path: the DesignWare host
bridge is unregistered with dw_pcie_host_deinit(), and the PHY is
powered off with meson_pcie_power_off() before devres releases the
driver's resources.

So yes, it should reduce power usage after unbind. The host bridge
teardown is also the matching cleanup for dw_pcie_host_init(), but I
don't have hardware evidence of an oops on unbind.

> Of the 34 instances of .probe() in drivers/pci/controller/dwc/, on 12
> implement .remove(), so if this fixes a problem, I'm wondering whether
> other drivers have the same problem.

Yes, that may deserve a broader driver-by-driver audit. I only checked
Meson for this patch. Some DWC drivers implement .remove(), and several
others set .suppress_bind_attrs, which affects the manual sysfs
bind/unbind path. Before this patch, the Meson driver had neither, while
meson_pcie_probe() calls meson_pcie_power_on() and registers the host
bridge with dw_pcie_host_init().

Thanks,
Shuvam

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 2/2] PCI: meson: Add missing remove callback

[Bjorn Helgaas]

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 <shuvampandey1@gmail.com>
> ---
>  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

Re: (subset) [PATCH 1/2] PCI: dwc: Guard RAS DES debugfs deinit

[Manivannan Sadhasivam]

On Mon, 18 May 2026 22:44:17 +0545, Shuvam Pandey wrote:
> dwc_pcie_rasdes_debugfs_init() returns success when the controller has
> no RAS DES capability, leaving pci->debugfs->rasdes_info unset. The
> common debugfs teardown path still calls
> dwc_pcie_rasdes_debugfs_deinit(), which dereferences rasdes_info
> unconditionally.
> 
> Return early when no RAS DES state was allocated. In that case no RAS DES
> mutex was initialized, so there is nothing to destroy.
> 
> [...]

Applied, thanks!

[1/2] PCI: dwc: Guard RAS DES debugfs deinit
      commit: a5ee214bb36f43b4d8bb2615b30d3b5c0ea80826

Best regards,
-- 
Manivannan Sadhasivam <mani@kernel.org>


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 1/2] PCI: dwc: Guard RAS DES debugfs deinit

[Bjorn Helgaas]

On Mon, May 18, 2026 at 10:44:17PM +0545, Shuvam Pandey wrote:
> dwc_pcie_rasdes_debugfs_init() returns success when the controller has
> no RAS DES capability, leaving pci->debugfs->rasdes_info unset. The
> common debugfs teardown path still calls
> dwc_pcie_rasdes_debugfs_deinit(), which dereferences rasdes_info
> unconditionally.
> 
> Return early when no RAS DES state was allocated. In that case no RAS DES
> mutex was initialized, so there is nothing to destroy.
> 
> Fixes: 4fbfa17f9a07 ("PCI: dwc: Add debugfs based Silicon Debug support for DWC")
> Signed-off-by: Shuvam Pandey <shuvampandey1@gmail.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware-debugfs.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-debugfs.c b/drivers/pci/controller/dwc/pcie-designware-debugfs.c
> index d0884253b..c3671cb2f 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-debugfs.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-debugfs.c
> @@ -557,6 +557,9 @@ static void dwc_pcie_rasdes_debugfs_deinit(struct dw_pcie *pci)
>  {
>  	struct dwc_pcie_rasdes_info *rinfo = pci->debugfs->rasdes_info;
>  
> +	if (!rinfo)
> +		return;

I guess this prevents a NULL pointer dereference, right?

>  	mutex_destroy(&rinfo->reg_event_lock);
>  }
>  
> -- 
> 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

Re: [PATCH 1/2] PCI: dwc: Guard RAS DES debugfs deinit

[Shuvam Pandey]

Hi Bjorn,

> I guess this prevents a NULL pointer dereference, right?

Yes. This is in the CONFIG_PCIE_DW_DEBUGFS teardown path when the
controller does not advertise the RAS DES capability.

In that case dwc_pcie_rasdes_debugfs_init() returns 0 before allocating
pci->debugfs->rasdes_info or initializing reg_event_lock. Later,
dwc_pcie_debugfs_deinit() still calls dwc_pcie_rasdes_debugfs_deinit()
unconditionally.

With CONFIG_DEBUG_MUTEXES, mutex_destroy() dereferences the mutex, so
calling it via &rinfo->reg_event_lock when rinfo is NULL would
dereference NULL. Without CONFIG_DEBUG_MUTEXES, mutex_destroy() is a
no-op, but there is still no RAS DES state to tear down.

So the patch makes the deinit path match the init path: if no
rasdes_info was allocated, skip the RAS DES mutex teardown.

Thanks,
Shuvam

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic