[PATCH v2] PCI: mediatek-gen3: Avoid PCIe resetting via PCIE_RSTB for Airoha EN7581 SoC
Jianjun Wang (王建军)
Jianjun.Wang at mediatek.com
Wed Nov 6 00:25:59 PST 2024
On Tue, 2024-11-05 at 10:30 +0100, lorenzo at kernel.org wrote:
> > On Mon, 2024-11-04 at 23:00 +0100, Lorenzo Bianconi wrote:
> > > External email : Please do not click links or open attachments
> > > until
> > > you have verified the sender or the content.
> > >
> > >
> > > Airoha EN7581 has a hw bug asserting/releasing PCIE_PE_RSTB
> > > signal
> > > causing occasional PCIe link down issues. In order to overcome
> > > the
> > > problem, PCIE_RSTB signals are not asserted/released during
> > > device
> > > probe or
> > > suspend/resume phase and the PCIe block is reset using
> > > REG_PCI_CONTROL
> > > (0x88) and REG_RESET_CONTROL (0x834) registers available via the
> > > clock
> > > module.
> > > Introduce flags field in the mtk_gen3_pcie_pdata struct in order
> > > to
> > > specify per-SoC capabilities.
> > >
> > > Tested-by: Hui Ma <hui.ma at airoha.com>
> > > Signed-off-by: Lorenzo Bianconi <lorenzo at kernel.org>
> > > ---
> > > Changes in v2:
> > > - introduce flags field in mtk_gen3_pcie_flags struct instead of
> > > adding
> > > reset callback
> > > - fix the leftover case in mtk_pcie_suspend_noirq routine
> > > - add more comments
> > > - Link to v1:
> > >
https://lore.kernel.org/r/20240920-pcie-en7581-rst-fix-v1-1-1043fb63ffc9@kernel.org
> > > ---
> > > drivers/pci/controller/pcie-mediatek-gen3.c | 59
> > > ++++++++++++++++++++---------
> > > 1 file changed, 41 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c
> > > b/drivers/pci/controller/pcie-mediatek-gen3.c
> > > index
> > > 66ce4b5d309bb6d64618c70ac5e0a529e0910511..8e4704ff3509867fc0ff799
> > > e9fb
> > > 99e71e46756cd 100644
> > > --- a/drivers/pci/controller/pcie-mediatek-gen3.c
> > > +++ b/drivers/pci/controller/pcie-mediatek-gen3.c
> > > @@ -125,10 +125,18 @@
> > >
> > > struct mtk_gen3_pcie;
> > >
> > > +enum mtk_gen3_pcie_flags {
> > > + SKIP_PCIE_RSTB = BIT(0), /* skip PCIE_RSTB signals
> > > configuration
> > > + * during device probing or
> > > suspend/resume
> > > + * phase in order to avoid hw
> > > bugs/issues.
> > > + */
> > > +};
> > > +
> > > /**
> > > * struct mtk_gen3_pcie_pdata - differentiate between host
> > > generations
> > > * @power_up: pcie power_up callback
> > > * @phy_resets: phy reset lines SoC data.
> > > + * @flags: pcie device flags.
> > > */
> > > struct mtk_gen3_pcie_pdata {
> > > int (*power_up)(struct mtk_gen3_pcie *pcie);
> > > @@ -136,6 +144,7 @@ struct mtk_gen3_pcie_pdata {
> > > const char *id[MAX_NUM_PHY_RESETS];
> > > int num_resets;
> > > } phy_resets;
> > > + u32 flags;
> > > };
> > >
> > > /**
> > > @@ -402,22 +411,33 @@ 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.
> > > + * Airoha EN7581 has a hw bug asserting/releasing
> > > PCIE_PE_RSTB signal
> > > + * causing occasional PCIe link down. In order to
> > > overcome
> > > the issue,
> > > + * PCIE_RSTB signals are not asserted/released at this
> > > stage
> > > and the
> > > + * PCIe block is reset using REG_PCI_CONTROL (0x88) and
> > > + * REG_RESET_CONTROL (0x834) registers available via the
> > > clock module.
> > > */
> > > - 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);
> >
> > What will happen if the EN7581 use this reset flow? Will it still
> > work
> > after this link down?
>
> Hi Jianjun Wang,
>
> This has been described here by Hui Ma:
>
https://lore.kernel.org/r/20240920-pcie-en7581-rst-fix-v1-1-1043fb63ffc9@kernel.org
>
> Setting PCIE_PE_RSTB bit on EN7581 SoC during reset triggers
> occasional PCIe link
> down issues caused by a hw problem.
Hi Lorenzo,
I'm wondering if we can ignore the previous reset and take this one as
the initial reset?
Thanks.
>
> Regards,
> Lorenzo
>
> >
> > > + if (!(pcie->soc->flags & SKIP_PCIE_RSTB)) {
> > > + /* 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(PCIE_T_PVPERL_MS);
> > > +
> > > + /* 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);
> > > + }
> > >
> > > /* Check if the link is up or not */
> > > err = readl_poll_timeout(pcie->base +
> > > PCIE_LINK_STATUS_REG,
> > > val,
> > > @@ -1160,10 +1180,12 @@ static int mtk_pcie_suspend_noirq(struct
> > > device *dev)
> > > return err;
> > > }
> > >
> > > - /* Pull down the PERST# pin */
> > > - val = readl_relaxed(pcie->base + PCIE_RST_CTRL_REG);
> > > - val |= PCIE_PE_RSTB;
> > > - writel_relaxed(val, pcie->base + PCIE_RST_CTRL_REG);
> > > + if (!(pcie->soc->flags & SKIP_PCIE_RSTB)) {
> > > + /* Pull down the PERST# pin */
> > > + val = readl_relaxed(pcie->base +
> > > PCIE_RST_CTRL_REG);
> > > + val |= PCIE_PE_RSTB;
> > > + writel_relaxed(val, pcie->base +
> > > PCIE_RST_CTRL_REG);
> > > + }
> > >
> > > dev_dbg(pcie->dev, "entered L2 states successfully");
> > >
> > > @@ -1214,6 +1236,7 @@ static const struct mtk_gen3_pcie_pdata
> > > mtk_pcie_soc_en7581 = {
> > > .id[2] = "phy-lane2",
> > > .num_resets = 3,
> > > },
> > > + .flags = SKIP_PCIE_RSTB,
> > > };
> > >
> > > static const struct of_device_id mtk_pcie_of_match[] = {
> > >
> > > ---
> > > base-commit: 3102ce10f3111e4c3b8fb233dc93f29e220adaf7
> > > change-id: 20240920-pcie-en7581-rst-fix-8161658c13c4
> > >
> > > Best regards,
> > > --
> > > Lorenzo Bianconi <lorenzo at kernel.org>
> > >
> > >
More information about the Linux-mediatek
mailing list