Linux-Amlogic Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] PCI: dwc: meson: Fix reset GPIO initial state
@ 2026-06-16  7:07 Ronald Claveau via B4 Relay
  2026-06-16  7:10 ` Neil Armstrong
  2026-06-16  7:19 ` sashiko-bot
  0 siblings, 2 replies; 3+ messages in thread
From: Ronald Claveau via B4 Relay @ 2026-06-16  7:07 UTC (permalink / raw)
  To: Yue Wang, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Neil Armstrong,
	Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Remi Pommarel
  Cc: linux-pci, linux-amlogic, linux-arm-kernel, linux-kernel,
	Ronald Claveau

From: Ronald Claveau <linux-kernel-dev@aliel.fr>

Commit 4d3186a525b3 ("PCI: amlogic: Fix reset assertion via gpio
descriptor") inverted the reset assertion logic to use proper gpio
descriptor semantics, and moved the polarity configuration to the
device tree as GPIO_ACTIVE_LOW. However, the initial GPIO state
"GPIOD_OUT_LOW" was not updated accordingly.

Change GPIOD_OUT_LOW to GPIOD_OUT_HIGH to get the right behaviour.

With GPIOD_OUT_LOW:

    ahci 0000:01:00.0: enabling device (0000 -> 0002)
    ahci 0000:01:00.0: SSS flag set, parallel bus scan disabled
    ahci 0000:01:00.0: Controller reset failed (0xffffffff)
    ahci 0000:01:00.0: probe with driver ahci failed with error -5

With this fix:

    ahci 0000:01:00.0: enabling device (0000 -> 0002)
    ahci 0000:01:00.0: AHCI vers 0001.0300, 32 command slots, 6 Gbps,
                       SATA mode
    ahci 0000:01:00.0: 1/1 ports implemented (port mask 0x1)
    ahci 0000:01:00.0: flags: 64bit ncq led clo only pio ccc

Fixes: 4d3186a525b3 ("PCI: amlogic: Fix reset assertion via gpio descriptor")
Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
---
 drivers/pci/controller/dwc/pci-meson.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
index 0694084f612b7..15ed59b8764fb 100644
--- a/drivers/pci/controller/dwc/pci-meson.c
+++ b/drivers/pci/controller/dwc/pci-meson.c
@@ -400,7 +400,7 @@ static int meson_pcie_probe(struct platform_device *pdev)
 		return PTR_ERR(mp->phy);
 	}
 
-	mp->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+	mp->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
 	if (IS_ERR(mp->reset_gpio)) {
 		dev_err(dev, "get reset gpio failed\n");
 		return PTR_ERR(mp->reset_gpio);

---
base-commit: abe651837cb394f76d738a7a747322fca3bf17ba
change-id: 20260616-fix-meson-pcie-reset-gpio-2b9e188668c5

Best regards,
-- 
Ronald Claveau <linux-kernel-dev@aliel.fr>



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

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] PCI: dwc: meson: Fix reset GPIO initial state
  2026-06-16  7:07 [PATCH] PCI: dwc: meson: Fix reset GPIO initial state Ronald Claveau via B4 Relay
@ 2026-06-16  7:10 ` Neil Armstrong
  2026-06-16  7:19 ` sashiko-bot
  1 sibling, 0 replies; 3+ messages in thread
From: Neil Armstrong @ 2026-06-16  7:10 UTC (permalink / raw)
  To: linux-kernel-dev, Yue Wang, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring,
	Bjorn Helgaas, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
	Remi Pommarel
  Cc: linux-pci, linux-amlogic, linux-arm-kernel, linux-kernel

On 6/16/26 09:07, Ronald Claveau via B4 Relay wrote:
> From: Ronald Claveau <linux-kernel-dev@aliel.fr>
> 
> Commit 4d3186a525b3 ("PCI: amlogic: Fix reset assertion via gpio
> descriptor") inverted the reset assertion logic to use proper gpio
> descriptor semantics, and moved the polarity configuration to the
> device tree as GPIO_ACTIVE_LOW. However, the initial GPIO state
> "GPIOD_OUT_LOW" was not updated accordingly.
> 
> Change GPIOD_OUT_LOW to GPIOD_OUT_HIGH to get the right behaviour.
> 
> With GPIOD_OUT_LOW:
> 
>      ahci 0000:01:00.0: enabling device (0000 -> 0002)
>      ahci 0000:01:00.0: SSS flag set, parallel bus scan disabled
>      ahci 0000:01:00.0: Controller reset failed (0xffffffff)
>      ahci 0000:01:00.0: probe with driver ahci failed with error -5
> 
> With this fix:
> 
>      ahci 0000:01:00.0: enabling device (0000 -> 0002)
>      ahci 0000:01:00.0: AHCI vers 0001.0300, 32 command slots, 6 Gbps,
>                         SATA mode
>      ahci 0000:01:00.0: 1/1 ports implemented (port mask 0x1)
>      ahci 0000:01:00.0: flags: 64bit ncq led clo only pio ccc
> 
> Fixes: 4d3186a525b3 ("PCI: amlogic: Fix reset assertion via gpio descriptor")
> Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
> ---
>   drivers/pci/controller/dwc/pci-meson.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
> index 0694084f612b7..15ed59b8764fb 100644
> --- a/drivers/pci/controller/dwc/pci-meson.c
> +++ b/drivers/pci/controller/dwc/pci-meson.c
> @@ -400,7 +400,7 @@ static int meson_pcie_probe(struct platform_device *pdev)
>   		return PTR_ERR(mp->phy);
>   	}
>   
> -	mp->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> +	mp->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
>   	if (IS_ERR(mp->reset_gpio)) {
>   		dev_err(dev, "get reset gpio failed\n");
>   		return PTR_ERR(mp->reset_gpio);
> 
> ---
> base-commit: abe651837cb394f76d738a7a747322fca3bf17ba
> change-id: 20260616-fix-meson-pcie-reset-gpio-2b9e188668c5
> 
> Best regards,

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Thanks,
Neil

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] PCI: dwc: meson: Fix reset GPIO initial state
  2026-06-16  7:07 [PATCH] PCI: dwc: meson: Fix reset GPIO initial state Ronald Claveau via B4 Relay
  2026-06-16  7:10 ` Neil Armstrong
@ 2026-06-16  7:19 ` sashiko-bot
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-06-16  7:19 UTC (permalink / raw)
  To: Ronald Claveau via B4 Relay; +Cc: neil.armstrong, linux-pci, linux-amlogic

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing .remove callback leads to Use-After-Free and resource leak on unbind/module unload.
--

commit 6f34b371980942ee76ca5383825dd8ecaff2cbfb
Author: Ronald Claveau <linux-kernel-dev@aliel.fr>

PCI: dwc: meson: Fix reset GPIO initial state

This commit changes the initial state of the reset GPIO from GPIOD_OUT_LOW
to GPIOD_OUT_HIGH. This fixes the reset assertion logic that was inverted
by a previous commit.

> diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
> index 0694084f612b7..15ed59b8764fb 100644
> --- a/drivers/pci/controller/dwc/pci-meson.c
> +++ b/drivers/pci/controller/dwc/pci-meson.c
> @@ -400,7 +400,7 @@ static int meson_pcie_probe(struct platform_device *pdev)
>  		return PTR_ERR(mp->phy);
>  	}
>  
> -	mp->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> +	mp->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);

[Severity: High]
This is a pre-existing issue, but does this driver lack a .remove callback
to clean up the PCIe host?

Looking at meson_pcie_probe(), it initializes the DWC PCIe host and
registers the PCI root bus:

drivers/pci/controller/dwc/pci-meson.c:meson_pcie_probe() {
    ...
    ret = dw_pcie_host_init(&pci->pp);
    if (ret < 0) {
    ...
}

However, meson_pcie_driver does not implement a .remove callback to call
dw_pcie_host_deinit(), nor does it set .suppress_bind_attrs = true.

When the driver is unbound via sysfs or unloaded, devres automatically frees
the memory for struct meson_pcie (and its embedded struct dw_pcie) and
disables clocks and resets.

Because dw_pcie_host_deinit() is never called, the PCIe root bus and its child
devices appear to remain registered in the PCI core.

Could this lead to a use-after-free and panic if subsequent interactions
with these leaked PCI devices occur (for example, via sysfs or lspci) after
the memory has been freed?

>  	if (IS_ERR(mp->reset_gpio)) {
>  		dev_err(dev, "get reset gpio failed\n");
>  		return PTR_ERR(mp->reset_gpio);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-fix-meson-pcie-reset-gpio-v1-1-fca404b4c8be@aliel.fr?part=1

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-06-16  7:19 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-16  7:07 [PATCH] PCI: dwc: meson: Fix reset GPIO initial state Ronald Claveau via B4 Relay
2026-06-16  7:10 ` Neil Armstrong
2026-06-16  7:19 ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox