[PATCH v2] PCI: mediatek-gen3: Avoid PCIe resetting via PCIE_RSTB for Airoha EN7581 SoC
lorenzo at kernel.org
lorenzo at kernel.org
Wed Nov 6 01:00:21 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?
Hi Jianjun Wang,
according to my understanding from Hui Ma's description, EN7581 has a hw issue
with PCIE_PE_RSTB and it can't rely on it.
Regards,
Lorenzo
>
> 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>
> > > >
> > > >
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-mediatek/attachments/20241106/21de1662/attachment.sig>
More information about the Linux-mediatek
mailing list