Re: [PATCH] PCI: mediatek-gen3: Avoid PCIe resetting for Airoha EN7581 SoC

AngeloGioacchino Del Regno
2024-09-23T09:42:56Z

Il 20/09/24 10:26, Lorenzo Bianconi ha scritto:
> The PCIe controller available on the EN7581 SoC does not support reset
> via the following lines:
> - PCIE_MAC_RSTB
> - PCIE_PHY_RSTB
> - PCIE_BRG_RSTB
> - PCIE_PE_RSTB
>
> Introduce the reset callback in order to avoid resetting the PCIe port
> for Airoha EN7581 SoC.
>

EN7581 doesn't support pulling up/down PERST#?!
That looks definitely odd, as that signal is part of the PCI-Express CEM spec.

Besides, there's another PERST# assertion at mtk_pcie_suspend_noirq()...

Cheers,
Angelo

> Tested-by: Hui Ma <hui.ma@airoha.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> drivers/pci/controller/pcie-mediatek-gen3.c | 44 ++++++++++++++++++-----------
> 1 file changed, 28 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
> index 5c19abac74e8..9cea67e92d98 100644
> --- a/drivers/pci/controller/pcie-mediatek-gen3.c
> +++ b/drivers/pci/controller/pcie-mediatek-gen3.c
> @@ -128,10 +128,12 @@ struct mtk_gen3_pcie;
> /**
> * struct mtk_gen3_pcie_pdata - differentiate between host generations
> * @power_up: pcie power_up callback
> + * @reset: pcie reset callback
> * @phy_resets: phy reset lines SoC data.
> */
> struct mtk_gen3_pcie_pdata {
> int (*power_up)(struct mtk_gen3_pcie *pcie);
> + void (*reset)(struct mtk_gen3_pcie *pcie);
> struct {
> const char *id[MAX_NUM_PHY_RESETS];
> int num_resets;
> @@ -373,6 +375,28 @@ static void mtk_pcie_enable_msi(struct mtk_gen3_pcie *pcie)
> writel_relaxed(val, pcie->base + PCIE_INT_ENABLE_REG);
> }
>
> +static void mtk_pcie_reset(struct mtk_gen3_pcie *pcie)
> +{
> + u32 val;
> +
> + /* Assert all reset signals */
> + val = readl_relaxed(pcie->base + PCIE_RST_CTRL_REG);
> + val |= PCIE_MAC_RSTB | PCIE_PHY_RSTB | PCIE_BRG_RSTB | PCIE_PE_RSTB;
> + writel_relaxed(val, pcie->base + PCIE_RST_CTRL_REG);
> +
> + /*
> + * Described in PCIe CEM specification sections 2.2 (PERST# Signal)
> + * and 2.2.1 (Initial Power-Up (G3 to S0)).
> + * The deassertion of PERST# should be delayed 100ms (TPVPERL)
> + * for the power and clock to become stable.
> + */
> + msleep(100);
> +
> + /* De-assert reset signals */
> + val &= ~(PCIE_MAC_RSTB | PCIE_PHY_RSTB | PCIE_BRG_RSTB | PCIE_PE_RSTB);
> + writel_relaxed(val, pcie->base + PCIE_RST_CTRL_REG);
> +}
> +
> static int mtk_pcie_startup_port(struct mtk_gen3_pcie *pcie)
> {
> struct resource_entry *entry;
> @@ -402,22 +426,9 @@ static int mtk_pcie_startup_port(struct mtk_gen3_pcie *pcie)
> val |= PCIE_DISABLE_DVFSRC_VLT_REQ;
> writel_relaxed(val, pcie->base + PCIE_MISC_CTRL_REG);
>
> - /* Assert all reset signals */
> - val = readl_relaxed(pcie->base + PCIE_RST_CTRL_REG);
> - val |= PCIE_MAC_RSTB | PCIE_PHY_RSTB | PCIE_BRG_RSTB | PCIE_PE_RSTB;
> - writel_relaxed(val, pcie->base + PCIE_RST_CTRL_REG);
> -
> - /*
> - * Described in PCIe CEM specification sections 2.2 (PERST# Signal)
> - * and 2.2.1 (Initial Power-Up (G3 to S0)).
> - * The deassertion of PERST# should be delayed 100ms (TPVPERL)
> - * for the power and clock to become stable.
> - */
> - msleep(100);
> -
> - /* De-assert reset signals */
> - val &= ~(PCIE_MAC_RSTB | PCIE_PHY_RSTB | PCIE_BRG_RSTB | PCIE_PE_RSTB);
> - writel_relaxed(val, pcie->base + PCIE_RST_CTRL_REG);
> + /* Reset the PCIe port if requested by the hw */
> + if (pcie->soc->reset)
> + pcie->soc->reset(pcie);
>
> /* Check if the link is up or not */
> err = readl_poll_timeout(pcie->base + PCIE_LINK_STATUS_REG, val,
> @@ -1207,6 +1218,7 @@ static const struct dev_pm_ops mtk_pcie_pm_ops = {
>
> static const struct mtk_gen3_pcie_pdata mtk_pcie_soc_mt8192 = {
> .power_up = mtk_pcie_power_up,
> + .reset = mtk_pcie_reset,
> .phy_resets = {
> .id[0] = "phy",
> .num_resets = 1,
>
> ---
> base-commit: f2024903cb387971abdbc6398a430e735a9b394c
> change-id: 20240920-pcie-en7581-rst-fix-8161658c13c4
>
> Best regards,