diff mbox series

[07/12] pcie: qcom: add tx term offset support

Message ID 20200320183455.21311-7-ansuelsmth@gmail.com (mailing list archive)
State New, archived
Headers show
Series [01/12] pcie: qcom: add missing ipq806x clocks in pcie driver | expand

Commit Message

Christian Marangi March 20, 2020, 6:34 p.m. UTC 
From: Sham Muthayyan <smuthayy@codeaurora.org>

Add tx term offset support to pcie qcom driver
need in some revision of the ipq806x soc

Signed-off-by: Sham Muthayyan <smuthayy@codeaurora.org>
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 61 ++++++++++++++++++++++----
 1 file changed, 52 insertions(+), 9 deletions(-)

Comments

Bjorn Helgaas March 20, 2020, 7:22 p.m. UTC | #1 
On Fri, Mar 20, 2020 at 07:34:49PM +0100, Ansuel Smith wrote:
> From: Sham Muthayyan <smuthayy@codeaurora.org>
> 
> Add tx term offset support to pcie qcom driver
> need in some revision of the ipq806x soc

The usual (s/pcie/PCIe/, s/soc/SoC/, wrap to fill 75 columns, add
period at end).  Apply to all patches.

> +#define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK	(0x1f << 16)
> +#define PHY_CTRL_PHY_TX0_TERM_OFFSET(x)		(x << 16)

Since the rest of the file uses BIT(), I think it would make sense to
use GENMASK() here.  And below, of course.

> +static inline void
> +writel_masked(void __iomem *addr, u32 clear_mask, u32 set_mask)

Follow coding style of rest of file, i.e.,

  static inline void writel_masked(...)

The name "writel_masked" suggests that we're writing "val & mask" to a
register, but that's not what's happening.  This is really a "clear
some bits and set others" operation.

The names are wordy, but we do have pci_clear_and_set_dword(),
pcie_capability_clear_and_set_word(), etc., functions that work
similarly.

> +{
> +	u32 val = readl(addr);
> +
> +	val &= ~clear_mask;
> +	val |= set_mask;
> +	writel(val, addr);
> +}

> +	/* Set Tx termination offset */

Capitalize consistently with other comments in the file.  Below also.
Hmm, I see the file is already a bit of a mess in that regard.  So
just do what you can.
Bjorn Andersson April 1, 2020, 8:40 p.m. UTC | #2 
On Fri 20 Mar 11:34 PDT 2020, Ansuel Smith wrote:

> From: Sham Muthayyan <smuthayy@codeaurora.org>
> 
> Add tx term offset support to pcie qcom driver
> need in some revision of the ipq806x soc
> 
> Signed-off-by: Sham Muthayyan <smuthayy@codeaurora.org>
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 61 ++++++++++++++++++++++----
>  1 file changed, 52 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index ecc22fd27ea6..8009e3117765 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -45,7 +45,13 @@
>  #define PCIE_CAP_CPL_TIMEOUT_DISABLE		0x10
>  
>  #define PCIE20_PARF_PHY_CTRL			0x40
> +#define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK	(0x1f << 16)
> +#define PHY_CTRL_PHY_TX0_TERM_OFFSET(x)		(x << 16)
> +
>  #define PCIE20_PARF_PHY_REFCLK			0x4C
> +#define REF_SSP_EN				BIT(16)
> +#define REF_USE_PAD				BIT(12)
> +
>  #define PCIE20_PARF_DBI_BASE_ADDR		0x168
>  #define PCIE20_PARF_SLV_ADDR_SPACE_SIZE		0x16C
>  #define PCIE20_PARF_MHI_CLOCK_RESET_CTRL	0x174
> @@ -77,6 +83,18 @@
>  #define DBI_RO_WR_EN				1
>  
>  #define PERST_DELAY_US				1000
> +/* PARF registers */
> +#define PCIE20_PARF_PCS_DEEMPH			0x34
> +#define PCS_DEEMPH_TX_DEEMPH_GEN1(x)		(x << 16)
> +#define PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(x)	(x << 8)
> +#define PCS_DEEMPH_TX_DEEMPH_GEN2_6DB(x)	(x << 0)
> +
> +#define PCIE20_PARF_PCS_SWING			0x38
> +#define PCS_SWING_TX_SWING_FULL(x)		(x << 8)
> +#define PCS_SWING_TX_SWING_LOW(x)		(x << 0)
> +
> +#define PCIE20_PARF_CONFIG_BITS			0x50
> +#define PHY_RX0_EQ(x)				(x << 24)
>  
>  #define PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE	0x358
>  #define SLV_ADDR_SPACE_SZ			0x10000000
> @@ -97,6 +115,7 @@ struct qcom_pcie_resources_2_1_0 {
>  	struct reset_control *phy_reset;
>  	struct reset_control *ext_reset;
>  	struct regulator_bulk_data supplies[QCOM_PCIE_2_1_0_MAX_SUPPLY];
> +	uint8_t phy_tx0_term_offset;
>  };
>  
>  struct qcom_pcie_resources_1_0_0 {
> @@ -184,6 +203,16 @@ struct qcom_pcie {
>  
>  #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
>  
> +static inline void
> +writel_masked(void __iomem *addr, u32 clear_mask, u32 set_mask)
> +{
> +	u32 val = readl(addr);
> +
> +	val &= ~clear_mask;
> +	val |= set_mask;
> +	writel(val, addr);
> +}
> +
>  static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
>  {
>  	gpiod_set_value_cansleep(pcie->reset, 1);
> @@ -277,6 +306,10 @@ static int qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie)
>  	if (IS_ERR(res->ext_reset))
>  		return PTR_ERR(res->ext_reset);
>  
> +	if (of_property_read_u8(dev->of_node, "phy-tx0-term-offset",
> +				&res->phy_tx0_term_offset))
> +		res->phy_tx0_term_offset = 0;

The appropriate way is to encode differences in hardware is to use
different compatibles for the two different versions of the hardware.

Regards,
Bjorn

> +
>  	res->phy_reset = devm_reset_control_get_exclusive(dev, "phy");
>  	return PTR_ERR_OR_ZERO(res->phy_reset);
>  }
> @@ -304,7 +337,6 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
>  	struct qcom_pcie_resources_2_1_0 *res = &pcie->res.v2_1_0;
>  	struct dw_pcie *pci = pcie->pci;
>  	struct device *dev = pci->dev;
> -	u32 val;
>  	int ret;
>  
>  	ret = reset_control_assert(res->ahb_reset);
> @@ -355,15 +387,26 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
>  		goto err_deassert_ahb;
>  	}
>  
> -	/* enable PCIe clocks and resets */
> -	val = readl(pcie->parf + PCIE20_PARF_PHY_CTRL);
> -	val &= ~BIT(0);
> -	writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
> +	writel_masked(pcie->parf + PCIE20_PARF_PHY_CTRL, BIT(0), 0);
> +
> +	/* Set Tx termination offset */
> +	writel_masked(pcie->parf + PCIE20_PARF_PHY_CTRL,
> +		      PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK,
> +		      PHY_CTRL_PHY_TX0_TERM_OFFSET(res->phy_tx0_term_offset));
> +
> +	/* PARF programming */
> +	writel(PCS_DEEMPH_TX_DEEMPH_GEN1(0x18) |
> +	       PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(0x18) |
> +	       PCS_DEEMPH_TX_DEEMPH_GEN2_6DB(0x22),
> +	       pcie->parf + PCIE20_PARF_PCS_DEEMPH);
> +	writel(PCS_SWING_TX_SWING_FULL(0x78) |
> +	       PCS_SWING_TX_SWING_LOW(0x78),
> +	       pcie->parf + PCIE20_PARF_PCS_SWING);
> +	writel(PHY_RX0_EQ(0x4), pcie->parf + PCIE20_PARF_CONFIG_BITS);
>  
> -	/* enable external reference clock */
> -	val = readl(pcie->parf + PCIE20_PARF_PHY_REFCLK);
> -	val |= BIT(16);
> -	writel(val, pcie->parf + PCIE20_PARF_PHY_REFCLK);
> +	/* Enable reference clock */
> +	writel_masked(pcie->parf + PCIE20_PARF_PHY_REFCLK,
> +		      REF_USE_PAD, REF_SSP_EN);
>  
>  	ret = reset_control_deassert(res->phy_reset);
>  	if (ret) {
> -- 
> 2.25.1
>
Christian Marangi April 1, 2020, 9:55 p.m. UTC | #3 
> On Fri 20 Mar 11:34 PDT 2020, Ansuel Smith wrote:
> 
> > From: Sham Muthayyan <smuthayy@codeaurora.org>
> >
> > Add tx term offset support to pcie qcom driver
> > need in some revision of the ipq806x soc
> >
> > Signed-off-by: Sham Muthayyan <smuthayy@codeaurora.org>
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > ---
> >  drivers/pci/controller/dwc/pcie-qcom.c | 61
> ++++++++++++++++++++++----
> >  1 file changed, 52 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
> b/drivers/pci/controller/dwc/pcie-qcom.c
> > index ecc22fd27ea6..8009e3117765 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -45,7 +45,13 @@
> >  #define PCIE_CAP_CPL_TIMEOUT_DISABLE		0x10
> >
> >  #define PCIE20_PARF_PHY_CTRL			0x40
> > +#define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK	(0x1f << 16)
> > +#define PHY_CTRL_PHY_TX0_TERM_OFFSET(x)		(x << 16)
> > +
> >  #define PCIE20_PARF_PHY_REFCLK			0x4C
> > +#define REF_SSP_EN				BIT(16)
> > +#define REF_USE_PAD				BIT(12)
> > +
> >  #define PCIE20_PARF_DBI_BASE_ADDR		0x168
> >  #define PCIE20_PARF_SLV_ADDR_SPACE_SIZE		0x16C
> >  #define PCIE20_PARF_MHI_CLOCK_RESET_CTRL	0x174
> > @@ -77,6 +83,18 @@
> >  #define DBI_RO_WR_EN				1
> >
> >  #define PERST_DELAY_US				1000
> > +/* PARF registers */
> > +#define PCIE20_PARF_PCS_DEEMPH			0x34
> > +#define PCS_DEEMPH_TX_DEEMPH_GEN1(x)		(x << 16)
> > +#define PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(x)	(x << 8)
> > +#define PCS_DEEMPH_TX_DEEMPH_GEN2_6DB(x)	(x << 0)
> > +
> > +#define PCIE20_PARF_PCS_SWING			0x38
> > +#define PCS_SWING_TX_SWING_FULL(x)		(x << 8)
> > +#define PCS_SWING_TX_SWING_LOW(x)		(x << 0)
> > +
> > +#define PCIE20_PARF_CONFIG_BITS			0x50
> > +#define PHY_RX0_EQ(x)				(x << 24)
> >
> >  #define PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE	0x358
> >  #define SLV_ADDR_SPACE_SZ			0x10000000
> > @@ -97,6 +115,7 @@ struct qcom_pcie_resources_2_1_0 {
> >  	struct reset_control *phy_reset;
> >  	struct reset_control *ext_reset;
> >  	struct regulator_bulk_data
> supplies[QCOM_PCIE_2_1_0_MAX_SUPPLY];
> > +	uint8_t phy_tx0_term_offset;
> >  };
> >
> >  struct qcom_pcie_resources_1_0_0 {
> > @@ -184,6 +203,16 @@ struct qcom_pcie {
> >
> >  #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
> >
> > +static inline void
> > +writel_masked(void __iomem *addr, u32 clear_mask, u32 set_mask)
> > +{
> > +	u32 val = readl(addr);
> > +
> > +	val &= ~clear_mask;
> > +	val |= set_mask;
> > +	writel(val, addr);
> > +}
> > +
> >  static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
> >  {
> >  	gpiod_set_value_cansleep(pcie->reset, 1);
> > @@ -277,6 +306,10 @@ static int
> qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie)
> >  	if (IS_ERR(res->ext_reset))
> >  		return PTR_ERR(res->ext_reset);
> >
> > +	if (of_property_read_u8(dev->of_node, "phy-tx0-term-offset",
> > +				&res->phy_tx0_term_offset))
> > +		res->phy_tx0_term_offset = 0;
> 
> The appropriate way is to encode differences in hardware is to use
> different compatibles for the two different versions of the hardware.
> 
> Regards,
> Bjorn
> 

So a better way to handle this would be to check the SoC compatible?
AFAIK a different offset is only needed on ipq8064 revision 2 and ipq8065
but
it looks bad to add a special code just for that 2 SoC. 
I would prefer to handle this with the offset definition but If you think
this would be
the right way, I will follow that. Waiting for your response about this.

> > +
> >  	res->phy_reset = devm_reset_control_get_exclusive(dev, "phy");
> >  	return PTR_ERR_OR_ZERO(res->phy_reset);
> >  }
> > @@ -304,7 +337,6 @@ static int qcom_pcie_init_2_1_0(struct
> qcom_pcie *pcie)
> >  	struct qcom_pcie_resources_2_1_0 *res = &pcie->res.v2_1_0;
> >  	struct dw_pcie *pci = pcie->pci;
> >  	struct device *dev = pci->dev;
> > -	u32 val;
> >  	int ret;
> >
> >  	ret = reset_control_assert(res->ahb_reset);
> > @@ -355,15 +387,26 @@ static int qcom_pcie_init_2_1_0(struct
> qcom_pcie *pcie)
> >  		goto err_deassert_ahb;
> >  	}
> >
> > -	/* enable PCIe clocks and resets */
> > -	val = readl(pcie->parf + PCIE20_PARF_PHY_CTRL);
> > -	val &= ~BIT(0);
> > -	writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
> > +	writel_masked(pcie->parf + PCIE20_PARF_PHY_CTRL, BIT(0), 0);
> > +
> > +	/* Set Tx termination offset */
> > +	writel_masked(pcie->parf + PCIE20_PARF_PHY_CTRL,
> > +		      PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK,
> > +		      PHY_CTRL_PHY_TX0_TERM_OFFSET(res-
> >phy_tx0_term_offset));
> > +
> > +	/* PARF programming */
> > +	writel(PCS_DEEMPH_TX_DEEMPH_GEN1(0x18) |
> > +	       PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(0x18) |
> > +	       PCS_DEEMPH_TX_DEEMPH_GEN2_6DB(0x22),
> > +	       pcie->parf + PCIE20_PARF_PCS_DEEMPH);
> > +	writel(PCS_SWING_TX_SWING_FULL(0x78) |
> > +	       PCS_SWING_TX_SWING_LOW(0x78),
> > +	       pcie->parf + PCIE20_PARF_PCS_SWING);
> > +	writel(PHY_RX0_EQ(0x4), pcie->parf + PCIE20_PARF_CONFIG_BITS);
> >
> > -	/* enable external reference clock */
> > -	val = readl(pcie->parf + PCIE20_PARF_PHY_REFCLK);
> > -	val |= BIT(16);
> > -	writel(val, pcie->parf + PCIE20_PARF_PHY_REFCLK);
> > +	/* Enable reference clock */
> > +	writel_masked(pcie->parf + PCIE20_PARF_PHY_REFCLK,
> > +		      REF_USE_PAD, REF_SSP_EN);
> >
> >  	ret = reset_control_deassert(res->phy_reset);
> >  	if (ret) {
> > --
> > 2.25.1
> >
Bjorn Andersson April 1, 2020, 11:52 p.m. UTC | #4 
On Wed 01 Apr 14:55 PDT 2020, ansuelsmth@gmail.com wrote:

> > On Fri 20 Mar 11:34 PDT 2020, Ansuel Smith wrote:
> > 
> > > From: Sham Muthayyan <smuthayy@codeaurora.org>
> > >
> > > Add tx term offset support to pcie qcom driver
> > > need in some revision of the ipq806x soc
> > >
> > > Signed-off-by: Sham Muthayyan <smuthayy@codeaurora.org>
> > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > > ---
> > >  drivers/pci/controller/dwc/pcie-qcom.c | 61
> > ++++++++++++++++++++++----
> > >  1 file changed, 52 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
> > b/drivers/pci/controller/dwc/pcie-qcom.c
> > > index ecc22fd27ea6..8009e3117765 100644
> > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > @@ -45,7 +45,13 @@
> > >  #define PCIE_CAP_CPL_TIMEOUT_DISABLE		0x10
> > >
> > >  #define PCIE20_PARF_PHY_CTRL			0x40
> > > +#define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK	(0x1f << 16)
> > > +#define PHY_CTRL_PHY_TX0_TERM_OFFSET(x)		(x << 16)
> > > +
> > >  #define PCIE20_PARF_PHY_REFCLK			0x4C
> > > +#define REF_SSP_EN				BIT(16)
> > > +#define REF_USE_PAD				BIT(12)
> > > +
> > >  #define PCIE20_PARF_DBI_BASE_ADDR		0x168
> > >  #define PCIE20_PARF_SLV_ADDR_SPACE_SIZE		0x16C
> > >  #define PCIE20_PARF_MHI_CLOCK_RESET_CTRL	0x174
> > > @@ -77,6 +83,18 @@
> > >  #define DBI_RO_WR_EN				1
> > >
> > >  #define PERST_DELAY_US				1000
> > > +/* PARF registers */
> > > +#define PCIE20_PARF_PCS_DEEMPH			0x34
> > > +#define PCS_DEEMPH_TX_DEEMPH_GEN1(x)		(x << 16)
> > > +#define PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(x)	(x << 8)
> > > +#define PCS_DEEMPH_TX_DEEMPH_GEN2_6DB(x)	(x << 0)
> > > +
> > > +#define PCIE20_PARF_PCS_SWING			0x38
> > > +#define PCS_SWING_TX_SWING_FULL(x)		(x << 8)
> > > +#define PCS_SWING_TX_SWING_LOW(x)		(x << 0)
> > > +
> > > +#define PCIE20_PARF_CONFIG_BITS			0x50
> > > +#define PHY_RX0_EQ(x)				(x << 24)
> > >
> > >  #define PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE	0x358
> > >  #define SLV_ADDR_SPACE_SZ			0x10000000
> > > @@ -97,6 +115,7 @@ struct qcom_pcie_resources_2_1_0 {
> > >  	struct reset_control *phy_reset;
> > >  	struct reset_control *ext_reset;
> > >  	struct regulator_bulk_data
> > supplies[QCOM_PCIE_2_1_0_MAX_SUPPLY];
> > > +	uint8_t phy_tx0_term_offset;
> > >  };
> > >
> > >  struct qcom_pcie_resources_1_0_0 {
> > > @@ -184,6 +203,16 @@ struct qcom_pcie {
> > >
> > >  #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
> > >
> > > +static inline void
> > > +writel_masked(void __iomem *addr, u32 clear_mask, u32 set_mask)
> > > +{
> > > +	u32 val = readl(addr);
> > > +
> > > +	val &= ~clear_mask;
> > > +	val |= set_mask;
> > > +	writel(val, addr);
> > > +}
> > > +
> > >  static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
> > >  {
> > >  	gpiod_set_value_cansleep(pcie->reset, 1);
> > > @@ -277,6 +306,10 @@ static int
> > qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie)
> > >  	if (IS_ERR(res->ext_reset))
> > >  		return PTR_ERR(res->ext_reset);
> > >
> > > +	if (of_property_read_u8(dev->of_node, "phy-tx0-term-offset",
> > > +				&res->phy_tx0_term_offset))
> > > +		res->phy_tx0_term_offset = 0;
> > 
> > The appropriate way is to encode differences in hardware is to use
> > different compatibles for the two different versions of the hardware.
> > 
> > Regards,
> > Bjorn
> > 
> 
> So a better way to handle this would be to check the SoC compatible?
> AFAIK a different offset is only needed on ipq8064 revision 2 and ipq8065
> but
> it looks bad to add a special code just for that 2 SoC. 
> I would prefer to handle this with the offset definition but If you think
> this would be
> the right way, I will follow that. Waiting for your response about this.
> 

Yes, please do this by having different compatibles for the different
revisions of the hardware block.

You should be able to use the same implementation for the two
compatibles, just make the phy_tx0_term_offset depends on which was
used.

Regards,
Bjorn
diff mbox series

Patch

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index ecc22fd27ea6..8009e3117765 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -45,7 +45,13 @@ 
 #define PCIE_CAP_CPL_TIMEOUT_DISABLE		0x10
 
 #define PCIE20_PARF_PHY_CTRL			0x40
+#define PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK	(0x1f << 16)
+#define PHY_CTRL_PHY_TX0_TERM_OFFSET(x)		(x << 16)
+
 #define PCIE20_PARF_PHY_REFCLK			0x4C
+#define REF_SSP_EN				BIT(16)
+#define REF_USE_PAD				BIT(12)
+
 #define PCIE20_PARF_DBI_BASE_ADDR		0x168
 #define PCIE20_PARF_SLV_ADDR_SPACE_SIZE		0x16C
 #define PCIE20_PARF_MHI_CLOCK_RESET_CTRL	0x174
@@ -77,6 +83,18 @@ 
 #define DBI_RO_WR_EN				1
 
 #define PERST_DELAY_US				1000
+/* PARF registers */
+#define PCIE20_PARF_PCS_DEEMPH			0x34
+#define PCS_DEEMPH_TX_DEEMPH_GEN1(x)		(x << 16)
+#define PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(x)	(x << 8)
+#define PCS_DEEMPH_TX_DEEMPH_GEN2_6DB(x)	(x << 0)
+
+#define PCIE20_PARF_PCS_SWING			0x38
+#define PCS_SWING_TX_SWING_FULL(x)		(x << 8)
+#define PCS_SWING_TX_SWING_LOW(x)		(x << 0)
+
+#define PCIE20_PARF_CONFIG_BITS			0x50
+#define PHY_RX0_EQ(x)				(x << 24)
 
 #define PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE	0x358
 #define SLV_ADDR_SPACE_SZ			0x10000000
@@ -97,6 +115,7 @@  struct qcom_pcie_resources_2_1_0 {
 	struct reset_control *phy_reset;
 	struct reset_control *ext_reset;
 	struct regulator_bulk_data supplies[QCOM_PCIE_2_1_0_MAX_SUPPLY];
+	uint8_t phy_tx0_term_offset;
 };
 
 struct qcom_pcie_resources_1_0_0 {
@@ -184,6 +203,16 @@  struct qcom_pcie {
 
 #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
 
+static inline void
+writel_masked(void __iomem *addr, u32 clear_mask, u32 set_mask)
+{
+	u32 val = readl(addr);
+
+	val &= ~clear_mask;
+	val |= set_mask;
+	writel(val, addr);
+}
+
 static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
 {
 	gpiod_set_value_cansleep(pcie->reset, 1);
@@ -277,6 +306,10 @@  static int qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie)
 	if (IS_ERR(res->ext_reset))
 		return PTR_ERR(res->ext_reset);
 
+	if (of_property_read_u8(dev->of_node, "phy-tx0-term-offset",
+				&res->phy_tx0_term_offset))
+		res->phy_tx0_term_offset = 0;
+
 	res->phy_reset = devm_reset_control_get_exclusive(dev, "phy");
 	return PTR_ERR_OR_ZERO(res->phy_reset);
 }
@@ -304,7 +337,6 @@  static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
 	struct qcom_pcie_resources_2_1_0 *res = &pcie->res.v2_1_0;
 	struct dw_pcie *pci = pcie->pci;
 	struct device *dev = pci->dev;
-	u32 val;
 	int ret;
 
 	ret = reset_control_assert(res->ahb_reset);
@@ -355,15 +387,26 @@  static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
 		goto err_deassert_ahb;
 	}
 
-	/* enable PCIe clocks and resets */
-	val = readl(pcie->parf + PCIE20_PARF_PHY_CTRL);
-	val &= ~BIT(0);
-	writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
+	writel_masked(pcie->parf + PCIE20_PARF_PHY_CTRL, BIT(0), 0);
+
+	/* Set Tx termination offset */
+	writel_masked(pcie->parf + PCIE20_PARF_PHY_CTRL,
+		      PHY_CTRL_PHY_TX0_TERM_OFFSET_MASK,
+		      PHY_CTRL_PHY_TX0_TERM_OFFSET(res->phy_tx0_term_offset));
+
+	/* PARF programming */
+	writel(PCS_DEEMPH_TX_DEEMPH_GEN1(0x18) |
+	       PCS_DEEMPH_TX_DEEMPH_GEN2_3_5DB(0x18) |
+	       PCS_DEEMPH_TX_DEEMPH_GEN2_6DB(0x22),
+	       pcie->parf + PCIE20_PARF_PCS_DEEMPH);
+	writel(PCS_SWING_TX_SWING_FULL(0x78) |
+	       PCS_SWING_TX_SWING_LOW(0x78),
+	       pcie->parf + PCIE20_PARF_PCS_SWING);
+	writel(PHY_RX0_EQ(0x4), pcie->parf + PCIE20_PARF_CONFIG_BITS);
 
-	/* enable external reference clock */
-	val = readl(pcie->parf + PCIE20_PARF_PHY_REFCLK);
-	val |= BIT(16);
-	writel(val, pcie->parf + PCIE20_PARF_PHY_REFCLK);
+	/* Enable reference clock */
+	writel_masked(pcie->parf + PCIE20_PARF_PHY_REFCLK,
+		      REF_USE_PAD, REF_SSP_EN);
 
 	ret = reset_control_deassert(res->phy_reset);
 	if (ret) {