> Il 23/09/24 12:06, Hui Ma (马慧) ha scritto: > > Hi Angelo， > > > > EN7581 doesn't support pulling up/down PERST by bit3 of PCIe MAC register 0x148(PCIE_RST_CTRL_REG). > > > > EN7581 toggle PERST in clk_bulk_enable function called by mtk_pcie_en7581_power_up function. > > > > Hello Hui, > please don't top post. > Anyway, are those bits unexistant on EN7581, or are those used for different functions? > If those do not exist, and setting those bits will not produce adverse effects, it'd be possible to avoid creating a different codepath and just add a comment saying that the setting has no effect on Airoha EN7581. > Regards, > Angelo > > Hi Angelo, Sorry for reply late reply. Bit3 of the register doesn't work on EN7581 because of the hardware issue. We have already completed PCIe RC/EP reset action before setting PCIe MAC register 0x148. At this moment，the device is in release status，if we toggle PCIe MAC register 0x148，PCIe will link down occasionally. In order to avoid creating a different codepath, we are trying another solution. Regards, Hui > > > > > > > > > > > -----邮件原件----- > 发件人: AngeloGioacchino Del Regno > <angelogioacchino.delregno@xxxxxxxxxxxxx> > 发送时间: 2024年9月23日 17:42 > 收件人: Lorenzo Bianconi <lorenzo@xxxxxxxxxx>; Ryder Lee > <Ryder.Lee@xxxxxxxxxxxx>; Jianjun Wang (王建军) > <Jianjun.Wang@xxxxxxxxxxxx>; Lorenzo Pieralisi > <lpieralisi@xxxxxxxxxx>; Krzysztof Wilczyński <kw@xxxxxxxxx>; Rob > Herring <robh@xxxxxxxxxx>; Bjorn Helgaas <bhelgaas@xxxxxxxxxx>; > Matthias Brugger <matthias.bgg@xxxxxxxxx> > 抄送: Christian Marangi <ansuelsmth@xxxxxxxxx>; > linux-pci@xxxxxxxxxxxxxxx; linux-mediatek@xxxxxxxxxxxxxxxxxxx; > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; upstream <upstream@xxxxxxxxxx>; > Hui Ma (马慧) <Hui.Ma@xxxxxxxxxx> > 主题: Re: [PATCH] PCI: mediatek-gen3: Avoid PCIe resetting for Airoha > EN7581 SoC > > > > 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@xxxxxxxxxx<mailto:hui.ma@xxxxxxxxxx>> > >> Signed-off-by: Lorenzo Bianconi >> <lorenzo@xxxxxxxxxx<mailto:lorenzo@xxxxxxxxxx>> > >> --- > >> 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, > >
