Re: [PATCH] PCI: keystone: deinitialize endpoint on remove

[Marek Vasut]

On 9/16/26 10:00 AM, Guangshuo Li wrote:
> ks_pcie_probe() initializes the DesignWare PCIe endpoint with
> dw_pcie_ep_init(). The probe failure path calls dw_pcie_ep_deinit()
> when endpoint register initialization fails, but the remove path does
> not perform the corresponding endpoint teardown after a successful
> probe.
>
> The successful endpoint initialization also calls pci_epc_init_notify().
> Without the matching teardown on removal, the EPC initialization state
> and resources allocated by the DesignWare endpoint core are left
> active after the driver is removed.
>
> Notify the endpoint framework about deinitialization and call
> dw_pcie_ep_deinit() before disabling runtime PM and the PHYs.
>
> This issue was found by manual code inspection.
>
> Fixes: 23284ad677a9 ("PCI: keystone: Add support for PCIe EP in AM654x Platforms")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Guangshuo Li <lgs201920130244@xxxxxxxxx>
> ---
>   drivers/pci/controller/dwc/pci-keystone.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> index 602516239a57..35787e727c5c 100644
> --- a/drivers/pci/controller/dwc/pci-keystone.c
> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> @@ -1349,9 +1349,17 @@ static void ks_pcie_remove(struct platform_device *pdev)
>   {
>   	struct keystone_pcie *ks_pcie = platform_get_drvdata(pdev);
>   	struct device_link **link = ks_pcie->link;
> +	struct dw_pcie *pci = ks_pcie->pci;
> +	const struct ks_pcie_of_data *data;
>   	int num_lanes = ks_pcie->num_lanes;
>   	struct device *dev = &pdev->dev;
>   
> +	data = of_device_get_match_data(dev);

Could you maybe cache the mode in struct keystone_pcie {} instead ?

> +	if (data->mode == DW_PCIE_EP_TYPE) {
> +		pci_epc_deinit_notify(pci->ep.epc);
> +		dw_pcie_ep_deinit(&pci->ep);

Would it make sense to make dw_pcie_ep_deinit() call 
pci_epc_deinit_notify() , to avoid duplication in controller drivers ?

> +	}
> +
>   	pm_runtime_put(dev);
>   	pm_runtime_disable(dev);
>   	ks_pcie_disable_phy(ks_pcie);

--
Best regards,
Marek Vasut