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

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 4469 bytes --]

> On Fri, Sep 20, 2024 at 10:26:28AM +0200, Lorenzo Bianconi wrote:
> > 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.
> > 
> > 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);
> 
> I see you're just moving this, but it's a good chance to use
> PCIE_T_PVPERL_MS.

ack, I will add it.

> 
> > +
> > +	/* 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 */
> 
> I don't see any real "request" from the hardware.  IIUC, this is more
> like "assert reset if this hardware supports it".

ack, %s/requested/supported. I will fix it.

Regards,
Lorenzo

> 
> > +	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,
> > -- 
> > Lorenzo Bianconi <lorenzo@kernel.org>
> > 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]