On Sun, Oct 31, 2021 at 07:12:33PM +0100, Marek Behún wrote:
> From: Pali Rohár <pali@xxxxxxxxxx>
>
> When unbinding driver, assert PERST# signal which prepares PCIe card for
> power down. Then disable link training and PHY.
This reads as three actions. If we carry them out as a single patch we
have to explain why they are related and what problem they are solving
as a _single_ commit.
Otherwise we have to split this patch into three and explain each of
them as a separate fix.
I understand it is tempting to coalesce missing code in one single
change but every commit must implement a single logical change.
Thanks,
Lorenzo
> Fixes: 526a76991b7b ("PCI: aardvark: Implement driver 'remove' function and allow to build it as module")
> Signed-off-by: Pali Rohár <pali@xxxxxxxxxx>
> Signed-off-by: Marek Behún <kabel@xxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> ---
> drivers/pci/controller/pci-aardvark.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> index b3d89cb449b6..2a82c4652c28 100644
> --- a/drivers/pci/controller/pci-aardvark.c
> +++ b/drivers/pci/controller/pci-aardvark.c
> @@ -1737,10 +1737,22 @@ static int advk_pcie_remove(struct platform_device *pdev)
> /* Free config space for emulated root bridge */
> pci_bridge_emul_cleanup(&pcie->bridge);
>
> + /* Assert PERST# signal which prepares PCIe card for power down */
> + if (pcie->reset_gpio)
> + gpiod_set_value_cansleep(pcie->reset_gpio, 1);
> +
> + /* Disable link training */
> + val = advk_readl(pcie, PCIE_CORE_CTRL0_REG);
> + val &= ~LINK_TRAINING_EN;
> + advk_writel(pcie, val, PCIE_CORE_CTRL0_REG);
> +
> /* Disable outbound address windows mapping */
> for (i = 0; i < OB_WIN_COUNT; i++)
> advk_pcie_disable_ob_win(pcie, i);
>
> + /* Disable phy */
> + advk_pcie_disable_phy(pcie);
> +
> return 0;
> }
>
> --
> 2.32.0
>
