Hi Kishon,

Thanks for the comments!

On 01/21/2015 11:11 AM, Kishon Vijay Abraham I wrote:

Hi,

On Friday 12 December 2014 10:43 PM, Stanimir Varbanov wrote:

quoted

Add a PCIe PHY driver used by PCIe host controller driver
on Qualcomm SoCs like Snapdragon 805.

Signed-off-by: Stanimir Varbanov <redacted>
---
 drivers/phy/Kconfig         |    7 +
 drivers/phy/Makefile        |    1 +
 drivers/phy/phy-qcom-pcie.c |  311 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 319 insertions(+), 0 deletions(-)
 create mode 100644 drivers/phy/phy-qcom-pcie.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 2a436e6..135bdcc 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig

@@ -218,6 +218,13 @@ config PHY_QCOM_IPQ806X_SATA
 	depends on OF
 	select GENERIC_PHY
 
+config PHY_QCOM_PCIE
+	tristate "Qualcomm PCIe SerDes/PHY driver"
+	depends on ARCH_QCOM
+	depends on HAS_IOMEM
+	depends on OF
+	select GENERIC_PHY

Please add a small description about the driver here.

Sure I will.

<snip>

quoted

+static const struct phy_regs pcie_phy_regs[] = {
+	{ PCIE_PHY_POWER_DOWN_CONTROL,			0x03 },
+	{ QSERDES_COM_SYSCLK_EN_SEL,			0x08 },
+	{ QSERDES_COM_DEC_START1,			0x82 },
+	{ QSERDES_COM_DEC_START2,			0x03 },
+	{ QSERDES_COM_DIV_FRAC_START1,			0xd5 },
+	{ QSERDES_COM_DIV_FRAC_START2,			0xaa },
+	{ QSERDES_COM_DIV_FRAC_START3,			0x13 },
+	{ QSERDES_COM_PLLLOCK_CMP_EN,			0x01 },
+	{ QSERDES_COM_PLLLOCK_CMP1,			0x2b },
+	{ QSERDES_COM_PLLLOCK_CMP2,			0x68 },
+	{ QSERDES_COM_PLL_CRCTRL,			0xff },
+	{ QSERDES_COM_PLL_CP_SETI,			0x3f },
+	{ QSERDES_COM_PLL_IP_SETP,			0x07 },
+	{ QSERDES_COM_PLL_CP_SETP,			0x03 },
+	{ QSERDES_RX_CDR_CONTROL,			0xf3 },
+	{ QSERDES_RX_CDR_CONTROL2,			0x6b },
+	{ QSERDES_COM_RESETSM_CNTRL,			0x10 },
+	{ QSERDES_RX_RX_TERM_HIGHZ_CM_AC_COUPLE,	0x87 },
+	{ QSERDES_RX_RX_EQ_GAIN12,			0x54 },
+	{ PCIE_PHY_POWER_STATE_CONFIG1,			0xa3 },
+	{ PCIE_PHY_POWER_STATE_CONFIG2,			0xcb },
+	{ QSERDES_COM_PLL_RXTXEPCLK_EN,			0x10 },
+	{ PCIE_PHY_ENDPOINT_REFCLK_DRIVE,		0x10 },
+	{ PCIE_PHY_SW_RESET,				0x00 },
+	{ PCIE_PHY_START,				0x03 },

No magic values for register writes.

Unfortunately these register values are taken as they are in CAF
downstream kernel and there are no bit/fields description for them.

quoted

+};
+
+static void qcom_pcie_phy_init(struct qcom_pcie_phy *pcie)
+{
+	const struct phy_regs *regs = pcie_phy_regs;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(pcie_phy_regs); i++)
+		writel(regs[i].val, pcie->base + regs[i].reg_offset);
+}
+
+static bool qcom_pcie_phy_is_ready(struct qcom_pcie_phy *pcie)
+{
+	u32 val = readl(pcie->base + PCIE_PHY_PCS_STATUS);
+
+	return val & BIT(6) ? false : true;
+}
+
+static int qcom_pcie_phy_power_on(struct phy *phy)
+{
+	struct qcom_pcie_phy *pcie = phy_get_drvdata(phy);
+	struct device *dev = pcie->dev;
+	int ret, retries;
+
+	ret = regulator_enable(pcie->vdda_pll);
+	if (ret) {
+		dev_err(dev, "cannot enable vdda_pll regulator\n");
+		return ret;
+	}
+
+	ret = regulator_enable(pcie->vdda);
+	if (ret) {
+		dev_err(dev, "cannot enable vdda regulator\n");
+		goto fail_vdda_pll;
+	}
+
+	ret = reset_control_deassert(pcie->res_phy);
+	if (ret) {
+		dev_err(dev, "cannot deassert phy reset\n");
+		goto fail_vdda;
+	}
+
+	qcom_pcie_phy_init(pcie);
+
+	usleep_range(PHY_DELAY_MIN_US, PHY_DELAY_MAX_US);

add a comment on why this delay is required.

Actually this delay is not required anymore and I will remove it in next
version. The delay which is important here is the delay between
clk_set_rate and clk_prepare_enable.

quoted

+
+	ret = clk_set_rate(pcie->clk, ~0);

What is the actual clock rate?

According to clk freq table in clock driver it could be 125Mhz or 250Mhz.

quoted

+	if (ret) {
+		dev_err(dev, "cannot set pipe clk rate\n");
+		goto fail_res;
+	}
+
+	/*
+	 * setting pipe rate takes time, try arbitrary delay before enabling
+	 * the clock
+	 */
+	retries = PIPE_CLK_RETRIES_COUNT;
+	do {
+		usleep_range(PIPE_CLK_DELAY_MIN_US, PIPE_CLK_DELAY_MAX_US);
+
+		ret = clk_prepare_enable(pcie->clk);
+		if (!ret)
+			break;
+	} while (retries--);
+
+	if (retries < 0) {
+		dev_err(dev, "cannot enable phy clock\n");
+		goto fail_res;
+	}
+
+	retries = PHY_RETRIES_COUNT;
+	do {
+		ret = qcom_pcie_phy_is_ready(pcie);
+		if (ret)
+			break;
+		usleep_range(PHY_DELAY_MIN_US, PHY_DELAY_MAX_US);
+	} while (retries--);
+
+	if (retries < 0) {
+		dev_err(dev, "phy failed to come up\n");
+		ret = -ETIMEDOUT;
+		goto fail;
+	}
+
+	return 0;
+
+fail:
+	clk_disable_unprepare(pcie->clk);
+fail_res:
+	reset_control_assert(pcie->res_phy);
+fail_vdda:
+	regulator_disable(pcie->vdda);
+fail_vdda_pll:
+	regulator_disable(pcie->vdda_pll);
+
+	return ret;
+}
+

<snip>

quoted

+
+	phy = devm_phy_create(dev, NULL, &qcom_pcie_phy_ops, NULL);

Please rebase it to the latest kernel.

Already done.

-- 
regards,
Stan