linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] dt-bindings: phy: Introduce Qualcomm eDP/DP PHY binding
@ 2021-05-11  4:19 Bjorn Andersson
  2021-05-11  4:19 ` [PATCH 2/2] phy: qcom: Introduce new eDP PHY driver Bjorn Andersson
  2021-05-11 17:24 ` [PATCH 1/2] dt-bindings: phy: Introduce Qualcomm eDP/DP PHY binding Stephen Boyd
  0 siblings, 2 replies; 12+ messages in thread
From: Bjorn Andersson @ 2021-05-11  4:19 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Kishon Vijay Abraham I, Vinod Koul,
	Rob Herring, sbillaka, Rob Clark, Dmitry Baryshkov, Stephen Boyd
  Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel

Introduce a binding for the eDP/DP PHY hardware block found in several
different Qualcomm platforms.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 .../devicetree/bindings/phy/qcom,edp-phy.yaml | 69 +++++++++++++++++++
 1 file changed, 69 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml

diff --git a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml
new file mode 100644
index 000000000000..c258e4f7e332
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/phy/qcom,edp-phy.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Qualcomm DP/eDP PHY
+
+maintainers:
+  - Bjorn Andersson <bjorn.andersson@linaro.org>
+
+description:
+  The Qualcomm DP/eDP PHY is found in a number of Qualcomm platform and
+  provides the physical interface for DisplayPort and Embedded Display Port.
+
+properties:
+  compatible:
+    enum:
+      - qcom,sc8180x-dp-phy
+      - qcom,sc8180x-edp-phy
+
+  reg:
+    items:
+      - description: PHY base register block
+      - description: tx0 register block
+      - description: tx1 register block
+      - description: PLL register block
+
+  clocks:
+    maxItems: 2
+
+  clock-names:
+    items:
+      - const: aux
+      - const: cfg_ahb
+
+  "#clock-cells":
+    const: 1
+
+  "#phy-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - "#clock-cells"
+  - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    phy@aec2a00 {
+      compatible = "qcom,sc8180x-edp-phy";
+      reg = <0x0aec2a00 0x1c0>,
+            <0x0aec2200 0xa0>,
+            <0x0aec2600 0xa0>,
+            <0x0aec2000 0x19c>;
+
+      clocks = <&dispcc 0>, <&dispcc 1>;
+      clock-names = "aux", "cfg_ahb";
+
+      #clock-cells = <1>;
+      #phy-cells = <0>;
+    };
+...
-- 
2.29.2


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/2] phy: qcom: Introduce new eDP PHY driver
  2021-05-11  4:19 [PATCH 1/2] dt-bindings: phy: Introduce Qualcomm eDP/DP PHY binding Bjorn Andersson
@ 2021-05-11  4:19 ` Bjorn Andersson
  2021-05-11  5:22   ` Stephen Boyd
                     ` (4 more replies)
  2021-05-11 17:24 ` [PATCH 1/2] dt-bindings: phy: Introduce Qualcomm eDP/DP PHY binding Stephen Boyd
  1 sibling, 5 replies; 12+ messages in thread
From: Bjorn Andersson @ 2021-05-11  4:19 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Kishon Vijay Abraham I, Vinod Koul,
	Rob Herring, sbillaka, Rob Clark, Dmitry Baryshkov, Stephen Boyd
  Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel

Many recent Qualcomm platforms comes with native DP and eDP support.
This consists of a controller int he MDSS and a QMP-like PHY.

While similar to the well known QMP block, the eDP PHY only has TX lanes
and the programming sequences are slightly different. Rather than
continuing the trend of parameterize the QMP driver to pieces, this
introduces the support as a new driver.

The registration of link and pixel clocks are borrowed from the QMP
driver. The non-DP link frequencies are omitted for now.

The eDP PHY is very similar to the dedicated (non-USB) DP PHY, but only
the prior is supported for now.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/phy/qualcomm/Kconfig        |  10 +
 drivers/phy/qualcomm/Makefile       |   1 +
 drivers/phy/qualcomm/phy-qcom-edp.c | 750 ++++++++++++++++++++++++++++
 3 files changed, 761 insertions(+)
 create mode 100644 drivers/phy/qualcomm/phy-qcom-edp.c

diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
index 7f6fcb8ec5ba..13ae6f4f3112 100644
--- a/drivers/phy/qualcomm/Kconfig
+++ b/drivers/phy/qualcomm/Kconfig
@@ -114,3 +114,13 @@ config PHY_QCOM_IPQ806X_USB
 	  This option enables support for the Synopsis PHYs present inside the
 	  Qualcomm USB3.0 DWC3 controller on ipq806x SoC. This driver supports
 	  both HS and SS PHY controllers.
+
+config PHY_QCOM_EDP
+	tristate "Qualcomm eDP PHY driver"
+	depends on ARCH_QCOM || COMPILE_TEST
+	depends on OF
+	depends on COMMON_CLK
+	select GENERIC_PHY
+	help
+	  Enable this driver to support the Qualcomm eDP PHY found in various
+	  Qualcomm chipsets.
diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
index 47acbd7daa3a..1afd0882dbcc 100644
--- a/drivers/phy/qualcomm/Makefile
+++ b/drivers/phy/qualcomm/Makefile
@@ -12,3 +12,4 @@ obj-$(CONFIG_PHY_QCOM_USB_HS_28NM)	+= phy-qcom-usb-hs-28nm.o
 obj-$(CONFIG_PHY_QCOM_USB_SS)		+= phy-qcom-usb-ss.o
 obj-$(CONFIG_PHY_QCOM_USB_SNPS_FEMTO_V2)+= phy-qcom-snps-femto-v2.o
 obj-$(CONFIG_PHY_QCOM_IPQ806X_USB)		+= phy-qcom-ipq806x-usb.o
+obj-$(CONFIG_PHY_QCOM_EDP)		+= phy-qcom-edp.o
diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c
new file mode 100644
index 000000000000..ff0508e49548
--- /dev/null
+++ b/drivers/phy/qualcomm/phy-qcom-edp.c
@@ -0,0 +1,750 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2017, 2020, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2021, Linaro Ltd.
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_address.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
+#include <linux/slab.h>
+
+#include <dt-bindings/phy/phy.h>
+
+#define DP_PHY_PD_CTL_PWRDN			0x001
+#define DP_PHY_PD_CTL_PSR_PWRDN			0x002
+#define DP_PHY_PD_CTL_AUX_PWRDN			0x004
+#define DP_PHY_PD_CTL_LANE_0_1_PWRDN		0x008
+#define DP_PHY_PD_CTL_LANE_2_3_PWRDN		0x010
+#define DP_PHY_PD_CTL_PLL_PWRDN			0x020
+#define DP_PHY_PD_CTL_DP_CLAMP_EN		0x040
+
+#define DP_PHY_CFG                              0x0010
+#define DP_PHY_CFG_1                            0x0014
+#define DP_PHY_PD_CTL                           0x001C
+#define DP_PHY_MODE                             0x0020
+
+#define DP_PHY_AUX_CFG0				0x0020
+#define DP_PHY_AUX_CFG1				0x0024
+#define DP_PHY_AUX_CFG2				0x0028
+#define DP_PHY_AUX_CFG3				0x002c
+#define DP_PHY_AUX_CFG4				0x0030
+#define DP_PHY_AUX_CFG5				0x0034
+#define DP_PHY_AUX_CFG6				0x0038
+#define DP_PHY_AUX_CFG7				0x003c
+#define DP_PHY_AUX_CFG8				0x0040
+#define DP_PHY_AUX_CFG9				0x0044
+
+#define DP_PHY_VCO_DIV                          0x0074
+#define DP_PHY_TX0_TX1_LANE_CTL                 0x007C
+#define DP_PHY_TX2_TX3_LANE_CTL                 0x00A0
+
+#define DP_PHY_SPARE0                           0x00CC
+#define DP_PHY_STATUS                           0x00E0
+
+/* Tx registers */
+#define TXn_CLKBUF_ENABLE                       0x0000
+#define TXn_TX_EMP_POST1_LVL                    0x0004
+
+#define TXn_TX_DRV_LVL                          0x0014
+#define TXn_TX_DRV_LVL_OFFSET                   0x0018
+#define TXn_RESET_TSYNC_EN                      0x001C
+//#define TXn_PRE_STALL_LDO_BOOST_EN            0x0020
+#define TXn_LDO_CONFIG                          0x0084
+#define TXn_TX_BAND                             0x0028
+#define TXn_INTERFACE_SELECT                    0x0024
+
+#define TXn_RES_CODE_LANE_OFFSET_TX0            0x0044
+#define TXn_RES_CODE_LANE_OFFSET_TX1            0x0048
+//#define TXn_RES_CODE_LANE_OFFSET_RX           0x0040
+
+#define TXn_TRANSCEIVER_BIAS_EN                 0x0054
+#define TXn_HIGHZ_DRVR_EN                       0x0058
+#define TXn_TX_POL_INV                          0x005C
+#define TXn_PARRATE_REC_DETECT_IDLE_EN          0x0060
+#define TXn_LANE_MODE_1                         0x0064
+
+#define TXn_TRAN_DRVR_EMP_EN                    0x0078
+//#define TXn_TX_INTERFACE_MODE                 0x00BC
+
+#define TXn_VMODE_CTRL1                         0x007C
+
+/* PLL register offset */
+#define QSERDES_COM_BG_TIMER                    0x000C
+#define QSERDES_COM_BIAS_EN_CLKBUFLR_EN         0x0044
+#define QSERDES_COM_CLK_ENABLE1                 0x0048
+#define QSERDES_COM_SYS_CLK_CTRL                0x004C
+#define QSERDES_COM_SYSCLK_BUF_ENABLE           0x0050
+#define QSERDES_COM_PLL_IVCO                    0x0058
+
+#define QSERDES_COM_CP_CTRL_MODE0               0x0074
+#define QSERDES_COM_PLL_RCTRL_MODE0             0x007C
+#define QSERDES_COM_PLL_CCTRL_MODE0             0x0084
+#define QSERDES_COM_SYSCLK_EN_SEL               0x0094
+#define QSERDES_COM_RESETSM_CNTRL               0x009C
+#define QSERDES_COM_LOCK_CMP_EN                 0x00A4
+#define QSERDES_COM_LOCK_CMP1_MODE0             0x00AC
+#define QSERDES_COM_LOCK_CMP2_MODE0             0x00B0
+
+#define QSERDES_COM_DEC_START_MODE0             0x00BC
+#define QSERDES_COM_DIV_FRAC_START1_MODE0       0x00CC
+#define QSERDES_COM_DIV_FRAC_START2_MODE0       0x00D0
+#define QSERDES_COM_DIV_FRAC_START3_MODE0       0x00D4
+#define QSERDES_COM_INTEGLOOP_GAIN0_MODE0       0x00EC
+#define QSERDES_COM_INTEGLOOP_GAIN1_MODE0       0x00F0
+#define QSERDES_COM_VCO_TUNE_CTRL               0x0108
+#define QSERDES_COM_VCO_TUNE_MAP                0x010C
+#define QSERDES_COM_VCO_TUNE1_MODE0             0x0110
+#define QSERDES_COM_VCO_TUNE2_MODE0             0x0114
+#define QSERDES_COM_CMN_STATUS                  0x0140
+
+#define QSERDES_COM_CLK_SEL                     0x0154
+#define QSERDES_COM_HSCLK_SEL                   0x0158
+
+#define QSERDES_COM_CORECLK_DIV_MODE0           0x0168
+
+#define QSERDES_COM_CORE_CLK_EN                 0x0174
+#define QSERDES_COM_C_READY_STATUS              0x0178
+#define QSERDES_COM_CMN_CONFIG                  0x017C
+
+#define QSERDES_COM_SVS_MODE_CLK_SEL            0x0184
+
+#define QSERDES_COM_SSC_EN_CENTER               0x0010
+#define QSERDES_COM_SSC_ADJ_PER1                0x0014
+#define QSERDES_COM_SSC_ADJ_PER2                0x0018
+#define QSERDES_COM_SSC_PER1                    0x001C
+#define QSERDES_COM_SSC_PER2                    0x0020
+#define QSERDES_COM_SSC_STEP_SIZE1_MODE0        0x0024
+#define QSERDES_COM_SSC_STEP_SIZE2_MODE0        0x0028
+
+#define DP_PHY_PLL_POLL_SLEEP_US                500
+#define DP_PHY_PLL_POLL_TIMEOUT_US              10000
+
+#define EDP_VCO_RATE_8100MHZDIV1000             8100000UL
+#define EDP_VCO_RATE_8640MHZDIV1000             8640000UL
+#define EDP_VCO_RATE_9720MHZDIV1000             9720000UL
+#define EDP_VCO_RATE_10800MHZDIV1000            10800000UL
+#define EDP_VCO_RATE_11880MHZDIV1000            11880000UL
+
+struct qcom_edp {
+	struct device *dev;
+
+	struct phy *phy;
+
+	void __iomem *edp;
+	void __iomem *tx0;
+	void __iomem *tx1;
+	void __iomem *pll;
+
+	struct clk_hw dp_link_hw;
+	struct clk_hw dp_pixel_hw;
+
+	struct phy_configure_opts_dp dp_opts;
+
+	struct clk_bulk_data clks[2];
+	struct regulator_bulk_data supplies[2];
+} ;
+
+static int qcom_edp_phy_init(struct phy *phy)
+{
+	struct qcom_edp *edp = phy_get_drvdata(phy);
+	int ret;
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(edp->supplies), edp->supplies);
+	if (ret)
+		return ret;
+
+	ret = clk_bulk_prepare_enable(ARRAY_SIZE(edp->clks), edp->clks);
+	if (ret)
+		goto out_disable_supplies;
+
+	writel(DP_PHY_PD_CTL_PWRDN | DP_PHY_PD_CTL_AUX_PWRDN |
+	       DP_PHY_PD_CTL_PLL_PWRDN | DP_PHY_PD_CTL_DP_CLAMP_EN,
+	       edp->edp + DP_PHY_PD_CTL);
+
+	writel(0x17, edp->pll + QSERDES_COM_BIAS_EN_CLKBUFLR_EN);
+
+	writel(DP_PHY_PD_CTL_PSR_PWRDN, edp->edp + DP_PHY_PD_CTL);
+	msleep(20);
+
+	writel(DP_PHY_PD_CTL_PWRDN | DP_PHY_PD_CTL_AUX_PWRDN |
+	       DP_PHY_PD_CTL_LANE_0_1_PWRDN | DP_PHY_PD_CTL_LANE_2_3_PWRDN |
+	       DP_PHY_PD_CTL_PLL_PWRDN | DP_PHY_PD_CTL_DP_CLAMP_EN,
+	       edp->edp + DP_PHY_PD_CTL);
+
+	writel(0x00, edp->edp + DP_PHY_AUX_CFG0);
+	writel(0x13, edp->edp + DP_PHY_AUX_CFG1);
+	writel(0x24, edp->edp + DP_PHY_AUX_CFG2);
+	writel(0x00, edp->edp + DP_PHY_AUX_CFG3);
+	writel(0x0a, edp->edp + DP_PHY_AUX_CFG4);
+	writel(0x26, edp->edp + DP_PHY_AUX_CFG5);
+	writel(0x0a, edp->edp + DP_PHY_AUX_CFG6);
+	writel(0x03, edp->edp + DP_PHY_AUX_CFG7);
+	writel(0x37, edp->edp + DP_PHY_AUX_CFG8);
+	writel(0x03, edp->edp + DP_PHY_AUX_CFG9);
+
+	writel(0x1f, edp->edp + 0x58);
+
+	msleep(20);
+
+	return 0;
+
+out_disable_supplies:
+	regulator_bulk_disable(ARRAY_SIZE(edp->supplies), edp->supplies);
+
+	return ret;
+}
+
+static int qcom_edp_phy_configure(struct phy *phy, union phy_configure_opts *opts)
+{
+	const struct phy_configure_opts_dp *dp_opts = &opts->dp;
+	struct qcom_edp *edp = phy_get_drvdata(phy);
+
+	memcpy(&edp->dp_opts, dp_opts, sizeof(*dp_opts));
+
+	return 0;
+}
+
+static int qcom_edp_configure_ssc(struct qcom_edp *edp)
+{
+	const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
+	u32 step1;
+	u32 step2;
+
+	switch (dp_opts->link_rate) {
+	case 1620:
+	case 2700:
+	case 8100:
+		step1 = 0x45;
+		step2 = 0x06;
+		break;
+	case 5400:
+		step1 = 0x5c;
+		step2 = 0x08;
+		break;
+	default:
+		/* Other link rates aren't supported */
+		return -EINVAL;
+	}
+
+	writel(0x01, edp->pll + QSERDES_COM_SSC_EN_CENTER);
+	writel(0x00, edp->pll + QSERDES_COM_SSC_ADJ_PER1);
+	writel(0x36, edp->pll + QSERDES_COM_SSC_PER1);
+	writel(0x01, edp->pll + QSERDES_COM_SSC_PER2);
+	writel(step1, edp->pll + QSERDES_COM_SSC_STEP_SIZE1_MODE0);
+	writel(step2, edp->pll + QSERDES_COM_SSC_STEP_SIZE2_MODE0);
+
+	return 0;
+}
+
+static int qcom_edp_configure_pll(struct qcom_edp *edp)
+{
+	const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
+	u32 div_frac_start2_mode0;
+	u32 div_frac_start3_mode0;
+	u32 dec_start_mode0;
+	u32 lock_cmp1_mode0;
+	u32 lock_cmp2_mode0;
+	u32 hsclk_sel;
+
+	switch (dp_opts->link_rate) {
+	case 1620:
+		hsclk_sel = 0x5;
+		dec_start_mode0 = 0x69;
+		div_frac_start2_mode0 = 0x80;
+		div_frac_start3_mode0 = 0x07;
+		lock_cmp1_mode0 = 0x6f;
+		lock_cmp2_mode0 = 0x08;
+		break;
+	case 2700:
+		hsclk_sel = 0x3;
+		dec_start_mode0 = 0x69;
+		div_frac_start2_mode0 = 0x80;
+		div_frac_start3_mode0 = 0x07;
+		lock_cmp1_mode0 = 0x0f;
+		lock_cmp2_mode0 = 0x0e;
+		break;
+	case 5400:
+		hsclk_sel = 0x1;
+		dec_start_mode0 = 0x8c;
+		div_frac_start2_mode0 = 0x00;
+		div_frac_start3_mode0 = 0x0a;
+		lock_cmp1_mode0 = 0x1f;
+		lock_cmp2_mode0 = 0x1c;
+		break;
+	case 8100:
+		hsclk_sel = 0x0;
+		dec_start_mode0 = 0x69;
+		div_frac_start2_mode0 = 0x80;
+		div_frac_start3_mode0 = 0x07;
+		lock_cmp1_mode0 = 0x2f;
+		lock_cmp2_mode0 = 0x2a;
+		break;
+	default:
+		/* Other link rates aren't supported */
+		return -EINVAL;
+	}
+
+	writel(0x01, edp->pll + QSERDES_COM_SVS_MODE_CLK_SEL);
+	writel(0x0b, edp->pll + QSERDES_COM_SYSCLK_EN_SEL);
+	writel(0x02, edp->pll + QSERDES_COM_SYS_CLK_CTRL);
+	writel(0x0c, edp->pll + QSERDES_COM_CLK_ENABLE1);
+	writel(0x06, edp->pll + QSERDES_COM_SYSCLK_BUF_ENABLE);
+	writel(0x30, edp->pll + QSERDES_COM_CLK_SEL);
+	writel(hsclk_sel, edp->pll + QSERDES_COM_HSCLK_SEL);
+	writel(0x0f, edp->pll + QSERDES_COM_PLL_IVCO);
+	writel(0x08, edp->pll + QSERDES_COM_LOCK_CMP_EN);
+	writel(0x36, edp->pll + QSERDES_COM_PLL_CCTRL_MODE0);
+	writel(0x16, edp->pll + QSERDES_COM_PLL_RCTRL_MODE0);
+	writel(0x06, edp->pll + QSERDES_COM_CP_CTRL_MODE0);
+	writel(dec_start_mode0, edp->pll + QSERDES_COM_DEC_START_MODE0);
+	writel(0x00, edp->pll + QSERDES_COM_DIV_FRAC_START1_MODE0);
+	writel(div_frac_start2_mode0, edp->pll + QSERDES_COM_DIV_FRAC_START2_MODE0);
+	writel(div_frac_start3_mode0, edp->pll + QSERDES_COM_DIV_FRAC_START3_MODE0);
+	writel(0x02, edp->pll + QSERDES_COM_CMN_CONFIG);
+	writel(0x3f, edp->pll + QSERDES_COM_INTEGLOOP_GAIN0_MODE0);
+	writel(0x00, edp->pll + QSERDES_COM_INTEGLOOP_GAIN1_MODE0);
+	writel(0x00, edp->pll + QSERDES_COM_VCO_TUNE_MAP);
+	writel(lock_cmp1_mode0, edp->pll + QSERDES_COM_LOCK_CMP1_MODE0);
+	writel(lock_cmp2_mode0, edp->pll + QSERDES_COM_LOCK_CMP2_MODE0);
+
+	writel(0x0a, edp->pll + QSERDES_COM_BG_TIMER);
+	writel(0x14, edp->pll + QSERDES_COM_CORECLK_DIV_MODE0);
+	writel(0x00, edp->pll + QSERDES_COM_VCO_TUNE_CTRL);
+	writel(0x17, edp->pll + QSERDES_COM_BIAS_EN_CLKBUFLR_EN);
+	writel(0x0f, edp->pll + QSERDES_COM_CORE_CLK_EN);
+	writel(0xa0, edp->pll + QSERDES_COM_VCO_TUNE1_MODE0);
+	writel(0x03, edp->pll + QSERDES_COM_VCO_TUNE2_MODE0);
+
+	return 0;
+}
+
+static int qcom_edp_set_vco_div(struct qcom_edp *edp)
+{
+	const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
+	unsigned long pixel_freq;
+	u32 vco_div;
+
+	switch (dp_opts->link_rate) {
+	case 1620:
+		vco_div = 0x1;
+		pixel_freq = 1620000000UL / 2;
+		break;
+	case 2700:
+		vco_div = 0x1;
+		pixel_freq = 2700000000UL / 2;
+		break;
+	case 5400:
+		vco_div = 0x2;
+		pixel_freq = 5400000000UL / 4;
+		break;
+	case 8100:
+		vco_div = 0x0;
+		pixel_freq = 8100000000UL / 6;
+		break;
+	default:
+		/* Other link rates aren't supported */
+		return -EINVAL;
+	}
+
+	writel(vco_div, edp->edp + DP_PHY_VCO_DIV);
+
+	clk_set_rate(edp->dp_link_hw.clk, dp_opts->link_rate * 100000);
+	clk_set_rate(edp->dp_pixel_hw.clk, pixel_freq);
+
+	return 0;
+}
+
+static int qcom_edp_phy_power_on(struct phy *phy)
+{
+	struct qcom_edp *edp = phy_get_drvdata(phy);
+	int timeout;
+	int ret;
+	u32 val;
+
+	writel(DP_PHY_PD_CTL_PWRDN | DP_PHY_PD_CTL_AUX_PWRDN |
+	       DP_PHY_PD_CTL_LANE_0_1_PWRDN | DP_PHY_PD_CTL_LANE_2_3_PWRDN |
+	       DP_PHY_PD_CTL_PLL_PWRDN | DP_PHY_PD_CTL_DP_CLAMP_EN,
+	       edp->edp + DP_PHY_PD_CTL);
+	writel(0xfc, edp->edp + DP_PHY_MODE);
+
+	timeout = readl_poll_timeout(edp->pll + QSERDES_COM_CMN_STATUS,
+				     val, val & BIT(7), 5, 200);
+	if (timeout)
+		return timeout;
+
+	writel(0x01, edp->tx0 + TXn_LDO_CONFIG);
+	writel(0x01, edp->tx1 + TXn_LDO_CONFIG);
+	writel(0x00, edp->tx0 + TXn_LANE_MODE_1);
+	writel(0x00, edp->tx1 + TXn_LANE_MODE_1);
+
+	ret = qcom_edp_configure_ssc(edp);
+	if (ret)
+		return ret;
+
+	ret = qcom_edp_configure_pll(edp);
+	if (ret)
+		return ret;
+
+	/* TX Lane configuration */
+	writel(0x05, edp->edp + DP_PHY_TX0_TX1_LANE_CTL);
+	writel(0x05, edp->edp + DP_PHY_TX2_TX3_LANE_CTL);
+
+	/* TX-0 register configuration */
+	writel(0x03, edp->tx0 + TXn_TRANSCEIVER_BIAS_EN);
+	writel(0x0f, edp->tx0 + TXn_CLKBUF_ENABLE);
+	writel(0x03, edp->tx0 + TXn_RESET_TSYNC_EN);
+	writel(0x01, edp->tx0 + TXn_TRAN_DRVR_EMP_EN);
+	writel(0x04, edp->tx0 + TXn_TX_BAND);
+
+	/* TX-1 register configuration */
+	writel(0x03, edp->tx1 + TXn_TRANSCEIVER_BIAS_EN);
+	writel(0x0f, edp->tx1 + TXn_CLKBUF_ENABLE);
+	writel(0x03, edp->tx1 + TXn_RESET_TSYNC_EN);
+	writel(0x01, edp->tx1 + TXn_TRAN_DRVR_EMP_EN);
+	writel(0x04, edp->tx1 + TXn_TX_BAND);
+
+	ret = qcom_edp_set_vco_div(edp);
+	if (ret)
+		return ret;
+
+	writel(0x01, edp->edp + DP_PHY_CFG);
+        writel(0x05, edp->edp + DP_PHY_CFG);
+        writel(0x01, edp->edp + DP_PHY_CFG);
+        writel(0x09, edp->edp + DP_PHY_CFG);
+
+        writel(0x20, edp->pll + QSERDES_COM_RESETSM_CNTRL);	
+
+	timeout = readl_poll_timeout(edp->pll + QSERDES_COM_C_READY_STATUS,
+				     val, val & BIT(0), 500, 10000);
+	if (timeout)
+		return timeout;
+
+	writel(0x19, edp->edp + DP_PHY_CFG);
+	writel(0x1f, edp->tx0 + TXn_HIGHZ_DRVR_EN);
+	writel(0x04, edp->tx0 + TXn_HIGHZ_DRVR_EN);
+	writel(0x00, edp->tx0 + TXn_TX_POL_INV);
+	writel(0x1f, edp->tx1 + TXn_HIGHZ_DRVR_EN);
+	writel(0x04, edp->tx1 + TXn_HIGHZ_DRVR_EN);
+	writel(0x00, edp->tx1 + TXn_TX_POL_INV);
+	writel(0x10, edp->tx0 + TXn_TX_DRV_LVL_OFFSET);
+	writel(0x10, edp->tx1 + TXn_TX_DRV_LVL_OFFSET);
+	writel(0x11, edp->tx0 + TXn_RES_CODE_LANE_OFFSET_TX0);
+	writel(0x11, edp->tx0 + TXn_RES_CODE_LANE_OFFSET_TX1);
+	writel(0x11, edp->tx1 + TXn_RES_CODE_LANE_OFFSET_TX0);
+	writel(0x11, edp->tx1 + TXn_RES_CODE_LANE_OFFSET_TX1);
+
+	writel(0x10, edp->tx0 + TXn_TX_EMP_POST1_LVL);
+	writel(0x10, edp->tx1 + TXn_TX_EMP_POST1_LVL);
+	writel(0x1f, edp->tx0 + TXn_TX_DRV_LVL);
+	writel(0x1f, edp->tx1 + TXn_TX_DRV_LVL);
+
+	writel(0x4, edp->tx0 + TXn_HIGHZ_DRVR_EN);
+	writel(0x3, edp->tx0 + TXn_TRANSCEIVER_BIAS_EN);
+	writel(0x4, edp->tx1 + TXn_HIGHZ_DRVR_EN);
+	writel(0x0, edp->tx1 + TXn_TRANSCEIVER_BIAS_EN);
+	writel(0x3, edp->edp + DP_PHY_CFG_1);
+
+	writel(0x18, edp->edp + DP_PHY_CFG);
+	udelay(100);
+
+	writel(0x19, edp->edp + DP_PHY_CFG);
+
+	return readl_poll_timeout(edp->edp + DP_PHY_STATUS,
+				  val, val & BIT(1), 500, 10000);
+}
+
+static int qcom_edp_phy_power_off(struct phy *phy)
+{
+	struct qcom_edp *edp = phy_get_drvdata(phy);
+
+	writel(DP_PHY_PD_CTL_PSR_PWRDN, edp->edp + DP_PHY_PD_CTL);
+
+	return 0;
+}
+
+static int qcom_edp_phy_exit(struct phy *phy)
+{
+	struct qcom_edp *edp = phy_get_drvdata(phy);
+
+	clk_bulk_disable_unprepare(ARRAY_SIZE(edp->clks), edp->clks);
+	regulator_bulk_disable(ARRAY_SIZE(edp->supplies), edp->supplies);
+
+	return 0;
+}
+
+static const struct phy_ops qcom_edp_ops = {
+	.init		= qcom_edp_phy_init,
+	.configure	= qcom_edp_phy_configure,
+	.power_on	= qcom_edp_phy_power_on,
+	.power_off	= qcom_edp_phy_power_off,
+	.exit		= qcom_edp_phy_exit,
+	.owner		= THIS_MODULE,
+};
+
+/*
+ * Display Port PLL driver block diagram for branch clocks
+ *
+ *              +------------------------------+
+ *              |         DP_VCO_CLK           |
+ *              |                              |
+ *              |    +-------------------+     |
+ *              |    |   (DP PLL/VCO)    |     |
+ *              |    +---------+---------+     |
+ *              |              v               |
+ *              |   +----------+-----------+   |
+ *              |   | hsclk_divsel_clk_src |   |
+ *              |   +----------+-----------+   |
+ *              +------------------------------+
+ *                              |
+ *          +---------<---------v------------>----------+
+ *          |                                           |
+ * +--------v----------------+                          |
+ * |    dp_phy_pll_link_clk  |                          |
+ * |     link_clk            |                          |
+ * +--------+----------------+                          |
+ *          |                                           |
+ *          |                                           |
+ *          v                                           v
+ * Input to DISPCC block                                |
+ * for link clk, crypto clk                             |
+ * and interface clock                                  |
+ *                                                      |
+ *                                                      |
+ *      +--------<------------+-----------------+---<---+
+ *      |                     |                 |
+ * +----v---------+  +--------v-----+  +--------v------+
+ * | vco_divided  |  | vco_divided  |  | vco_divided   |
+ * |    _clk_src  |  |    _clk_src  |  |    _clk_src   |
+ * |              |  |              |  |               |
+ * |divsel_six    |  |  divsel_two  |  |  divsel_four  |
+ * +-------+------+  +-----+--------+  +--------+------+
+ *         |                 |                  |
+ *         v---->----------v-------------<------v
+ *                         |
+ *              +----------+-----------------+
+ *              |   dp_phy_pll_vco_div_clk   |
+ *              +---------+------------------+
+ *                        |
+ *                        v
+ *              Input to DISPCC block
+ *              for DP pixel clock
+ *
+ */
+static int qcom_edp_dp_pixel_clk_determine_rate(struct clk_hw *hw,
+						struct clk_rate_request *req)
+{
+	switch (req->rate) {
+	case 1620000000UL / 2:
+	case 2700000000UL / 2:
+	/* 5.4 and 8.1 GHz are same link rate as 2.7GHz, i.e. div 4 and div 6 */
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static unsigned long
+qcom_edp_dp_pixel_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
+{
+	const struct qcom_edp *edp = container_of(hw, struct qcom_edp, dp_pixel_hw);
+	const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
+
+	switch (dp_opts->link_rate) {
+	case 1620:
+		return 1620000000UL / 2;
+	case 2700:
+		return 2700000000UL / 2;
+	case 5400:
+		return 5400000000UL / 4;
+	case 8100:
+		return 8100000000UL / 6;
+	default:
+		return 0;
+	}
+}
+
+static const struct clk_ops qcom_edp_dp_pixel_clk_ops = {
+	.determine_rate = qcom_edp_dp_pixel_clk_determine_rate,
+	.recalc_rate = qcom_edp_dp_pixel_clk_recalc_rate,
+};
+
+static int qcom_edp_dp_link_clk_determine_rate(struct clk_hw *hw,
+					       struct clk_rate_request *req)
+{
+	switch (req->rate) {
+	case 162000000:
+	case 270000000:
+	case 540000000:
+	case 810000000:
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static unsigned long
+qcom_edp_dp_link_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
+{
+	const struct qcom_edp *edp = container_of(hw, struct qcom_edp, dp_link_hw);
+	const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
+
+	switch (dp_opts->link_rate) {
+	case 1620:
+	case 2700:
+	case 5400:
+	case 8100:
+		return dp_opts->link_rate * 100000;
+	default:
+		return 0;
+	}
+}
+
+static const struct clk_ops qcom_edp_dp_link_clk_ops = {
+	.determine_rate = qcom_edp_dp_link_clk_determine_rate,
+	.recalc_rate = qcom_edp_dp_link_clk_recalc_rate,
+};
+
+static struct clk_hw *
+qcom_edp_dp_clks_hw_get(struct of_phandle_args *clkspec, void *data)
+{
+	unsigned int idx = clkspec->args[0];
+	struct qcom_edp *edp = data;
+
+	if (idx >= 2) {
+		pr_err("%s: invalid index %u\n", __func__, idx);
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (idx == 0)
+		return &edp->dp_link_hw;
+
+	return &edp->dp_pixel_hw;
+}
+
+static void phy_clk_release_provider(void *res)
+{
+	of_clk_del_provider(res);
+}
+
+static int qcom_edp_clks_register(struct qcom_edp *edp, struct device_node *np)
+{
+	struct clk_init_data init = { };
+	int ret;
+
+	init.ops = &qcom_edp_dp_link_clk_ops;
+	init.name = "edp_phy_pll_link_clk";
+	edp->dp_link_hw.init = &init;
+	ret = devm_clk_hw_register(edp->dev, &edp->dp_link_hw);
+	if (ret)
+		return ret;
+
+	init.ops = &qcom_edp_dp_pixel_clk_ops;
+	init.name = "edp_phy_pll_vco_div_clk";
+	edp->dp_pixel_hw.init = &init;
+	ret = devm_clk_hw_register(edp->dev, &edp->dp_pixel_hw);
+	if (ret)
+		return ret;
+
+	ret = of_clk_add_hw_provider(np, qcom_edp_dp_clks_hw_get, edp);
+	if (ret)
+		return ret;
+
+	/*
+	 * Roll a devm action because the clock provider is the child node, but
+	 * the child node is not actually a device.
+	 */
+	ret = devm_add_action(edp->dev, phy_clk_release_provider, np);
+	if (ret)
+		phy_clk_release_provider(np);
+
+	return ret;
+}
+
+static int qcom_edp_phy_probe(struct platform_device *pdev)
+{
+	struct phy_provider *phy_provider;
+	struct device *dev = &pdev->dev;
+	struct qcom_edp *edp;
+	int ret;
+
+	edp = devm_kzalloc(dev, sizeof(*edp), GFP_KERNEL);
+	if (!edp)
+		return -ENOMEM;
+
+	edp->dev = dev;
+
+	edp->edp = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(edp->edp))
+		return PTR_ERR(edp->edp);
+
+	edp->tx0 = devm_platform_ioremap_resource(pdev, 1);
+	if (IS_ERR(edp->tx0))
+		return PTR_ERR(edp->tx0);
+
+	edp->tx1 = devm_platform_ioremap_resource(pdev, 2);
+	if (IS_ERR(edp->tx1))
+		return PTR_ERR(edp->tx1);
+
+	edp->pll = devm_platform_ioremap_resource(pdev, 3);
+	if (IS_ERR(edp->pll))
+		return PTR_ERR(edp->pll);
+
+	edp->clks[0].id = "aux";
+	edp->clks[1].id = "cfg_ahb";
+	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(edp->clks), edp->clks);
+	if (ret)
+		return ret;
+
+	edp->supplies[0].supply = "vdda-phy";
+	edp->supplies[1].supply = "vdda-pll";
+	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(edp->supplies), edp->supplies);
+	if (ret)
+		return ret;
+
+	ret = qcom_edp_clks_register(edp, pdev->dev.of_node);
+	if (ret)
+		return ret;
+
+	edp->phy = devm_phy_create(dev, pdev->dev.of_node, &qcom_edp_ops);
+	if (IS_ERR(edp->phy)) {
+		dev_err(dev, "failed to register phy\n");
+		return PTR_ERR(edp->phy);
+	}
+
+	phy_set_drvdata(edp->phy, edp);
+
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static const struct of_device_id qcom_edp_phy_match_table[] = {
+	{ .compatible = "qcom,sc8180x-edp-phy" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, qcom_edp_phy_match_table);
+
+static struct platform_driver qcom_edp_phy_driver = {
+	.probe		= qcom_edp_phy_probe,
+	.driver = {
+		.name	= "qcom-edp-phy",
+		.of_match_table = qcom_edp_phy_match_table,
+	},
+};
+
+module_platform_driver(qcom_edp_phy_driver);
+
+MODULE_DESCRIPTION("Qualcomm eDP PHY driver");
+MODULE_LICENSE("GPL v2");
-- 
2.29.2


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] phy: qcom: Introduce new eDP PHY driver
  2021-05-11  4:19 ` [PATCH 2/2] phy: qcom: Introduce new eDP PHY driver Bjorn Andersson
@ 2021-05-11  5:22   ` Stephen Boyd
  2021-05-14 10:38     ` Vinod Koul
  2021-05-11 14:24   ` Jeffrey Hugo
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Stephen Boyd @ 2021-05-11  5:22 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Dmitry Baryshkov,
	Kishon Vijay Abraham I, Rob Clark, Rob Herring, Vinod Koul,
	sbillaka
  Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel

Quoting Bjorn Andersson (2021-05-10 21:19:30)
> Many recent Qualcomm platforms comes with native DP and eDP support.
> This consists of a controller int he MDSS and a QMP-like PHY.
>
> While similar to the well known QMP block, the eDP PHY only has TX lanes
> and the programming sequences are slightly different. Rather than
> continuing the trend of parameterize the QMP driver to pieces, this
> introduces the support as a new driver.

Thank you for not slamming it into the same mega driver.

>
> The registration of link and pixel clocks are borrowed from the QMP
> driver. The non-DP link frequencies are omitted for now.

Can we make some library code for "DP" stuff in qmp that the two can
call to share the logic?

>
> The eDP PHY is very similar to the dedicated (non-USB) DP PHY, but only
> the prior is supported for now.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>  drivers/phy/qualcomm/Kconfig        |  10 +
>  drivers/phy/qualcomm/Makefile       |   1 +
>  drivers/phy/qualcomm/phy-qcom-edp.c | 750 ++++++++++++++++++++++++++++
>  3 files changed, 761 insertions(+)
>  create mode 100644 drivers/phy/qualcomm/phy-qcom-edp.c
>
> diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
> index 7f6fcb8ec5ba..13ae6f4f3112 100644
> --- a/drivers/phy/qualcomm/Kconfig
> +++ b/drivers/phy/qualcomm/Kconfig
> @@ -114,3 +114,13 @@ config PHY_QCOM_IPQ806X_USB
>           This option enables support for the Synopsis PHYs present inside the
>           Qualcomm USB3.0 DWC3 controller on ipq806x SoC. This driver supports
>           both HS and SS PHY controllers.
> +
> +config PHY_QCOM_EDP

Can this come somewhere alphabet sorted?

> +       tristate "Qualcomm eDP PHY driver"
> +       depends on ARCH_QCOM || COMPILE_TEST

I wonder if this could be pulled up above for the whole file.

> +       depends on OF
> +       depends on COMMON_CLK
> +       select GENERIC_PHY
> +       help
> +         Enable this driver to support the Qualcomm eDP PHY found in various
> +         Qualcomm chipsets.
> diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
> index 47acbd7daa3a..1afd0882dbcc 100644
> --- a/drivers/phy/qualcomm/Makefile
> +++ b/drivers/phy/qualcomm/Makefile
> @@ -12,3 +12,4 @@ obj-$(CONFIG_PHY_QCOM_USB_HS_28NM)    += phy-qcom-usb-hs-28nm.o
>  obj-$(CONFIG_PHY_QCOM_USB_SS)          += phy-qcom-usb-ss.o
>  obj-$(CONFIG_PHY_QCOM_USB_SNPS_FEMTO_V2)+= phy-qcom-snps-femto-v2.o
>  obj-$(CONFIG_PHY_QCOM_IPQ806X_USB)             += phy-qcom-ipq806x-usb.o
> +obj-$(CONFIG_PHY_QCOM_EDP)             += phy-qcom-edp.o

Also alphabet sorted instead of tacked onto the end?

> diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c
> new file mode 100644
> index 000000000000..ff0508e49548
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-edp.c
> @@ -0,0 +1,750 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2017, 2020, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2021, Linaro Ltd.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_address.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/reset.h>
> +#include <linux/slab.h>
> +
> +#include <dt-bindings/phy/phy.h>
> +
> +#define DP_PHY_PD_CTL_PWRDN                    0x001
> +#define DP_PHY_PD_CTL_PSR_PWRDN                        0x002
> +#define DP_PHY_PD_CTL_AUX_PWRDN                        0x004
> +#define DP_PHY_PD_CTL_LANE_0_1_PWRDN           0x008
> +#define DP_PHY_PD_CTL_LANE_2_3_PWRDN           0x010
> +#define DP_PHY_PD_CTL_PLL_PWRDN                        0x020
> +#define DP_PHY_PD_CTL_DP_CLAMP_EN              0x040
> +
> +#define DP_PHY_CFG                              0x0010
> +#define DP_PHY_CFG_1                            0x0014
> +#define DP_PHY_PD_CTL                           0x001C
> +#define DP_PHY_MODE                             0x0020
> +
> +#define DP_PHY_AUX_CFG0                                0x0020
> +#define DP_PHY_AUX_CFG1                                0x0024
> +#define DP_PHY_AUX_CFG2                                0x0028
> +#define DP_PHY_AUX_CFG3                                0x002c
> +#define DP_PHY_AUX_CFG4                                0x0030
> +#define DP_PHY_AUX_CFG5                                0x0034
> +#define DP_PHY_AUX_CFG6                                0x0038
> +#define DP_PHY_AUX_CFG7                                0x003c
> +#define DP_PHY_AUX_CFG8                                0x0040
> +#define DP_PHY_AUX_CFG9                                0x0044
> +
> +#define DP_PHY_VCO_DIV                          0x0074
> +#define DP_PHY_TX0_TX1_LANE_CTL                 0x007C
> +#define DP_PHY_TX2_TX3_LANE_CTL                 0x00A0
> +
> +#define DP_PHY_SPARE0                           0x00CC
> +#define DP_PHY_STATUS                           0x00E0
> +
> +/* Tx registers */
> +#define TXn_CLKBUF_ENABLE                       0x0000
> +#define TXn_TX_EMP_POST1_LVL                    0x0004
> +
> +#define TXn_TX_DRV_LVL                          0x0014
> +#define TXn_TX_DRV_LVL_OFFSET                   0x0018
> +#define TXn_RESET_TSYNC_EN                      0x001C
> +//#define TXn_PRE_STALL_LDO_BOOST_EN            0x0020
> +#define TXn_LDO_CONFIG                          0x0084
> +#define TXn_TX_BAND                             0x0028
> +#define TXn_INTERFACE_SELECT                    0x0024
> +
> +#define TXn_RES_CODE_LANE_OFFSET_TX0            0x0044
> +#define TXn_RES_CODE_LANE_OFFSET_TX1            0x0048
> +//#define TXn_RES_CODE_LANE_OFFSET_RX           0x0040
> +
> +#define TXn_TRANSCEIVER_BIAS_EN                 0x0054
> +#define TXn_HIGHZ_DRVR_EN                       0x0058
> +#define TXn_TX_POL_INV                          0x005C
> +#define TXn_PARRATE_REC_DETECT_IDLE_EN          0x0060
> +#define TXn_LANE_MODE_1                         0x0064
> +
> +#define TXn_TRAN_DRVR_EMP_EN                    0x0078
> +//#define TXn_TX_INTERFACE_MODE                 0x00BC

Future work? Maybe add a TODO?

> +
> +#define TXn_VMODE_CTRL1                         0x007C
> +
> +/* PLL register offset */
> +#define QSERDES_COM_BG_TIMER                    0x000C
> +#define QSERDES_COM_BIAS_EN_CLKBUFLR_EN         0x0044
> +#define QSERDES_COM_CLK_ENABLE1                 0x0048
> +#define QSERDES_COM_SYS_CLK_CTRL                0x004C
[..]
> +static int qcom_edp_clks_register(struct qcom_edp *edp, struct device_node *np)
> +{
> +       struct clk_init_data init = { };
> +       int ret;
> +
> +       init.ops = &qcom_edp_dp_link_clk_ops;
> +       init.name = "edp_phy_pll_link_clk";
> +       edp->dp_link_hw.init = &init;
> +       ret = devm_clk_hw_register(edp->dev, &edp->dp_link_hw);
> +       if (ret)
> +               return ret;
> +
> +       init.ops = &qcom_edp_dp_pixel_clk_ops;
> +       init.name = "edp_phy_pll_vco_div_clk";
> +       edp->dp_pixel_hw.init = &init;
> +       ret = devm_clk_hw_register(edp->dev, &edp->dp_pixel_hw);
> +       if (ret)
> +               return ret;
> +
> +       ret = of_clk_add_hw_provider(np, qcom_edp_dp_clks_hw_get, edp);
> +       if (ret)
> +               return ret;
> +
> +       /*
> +        * Roll a devm action because the clock provider is the child node, but

Is it still a child node?

> +        * the child node is not actually a device.
> +        */
> +       ret = devm_add_action(edp->dev, phy_clk_release_provider, np);
> +       if (ret)
> +               phy_clk_release_provider(np);
> +
> +       return ret;
> +}
> +

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] phy: qcom: Introduce new eDP PHY driver
  2021-05-11  4:19 ` [PATCH 2/2] phy: qcom: Introduce new eDP PHY driver Bjorn Andersson
  2021-05-11  5:22   ` Stephen Boyd
@ 2021-05-11 14:24   ` Jeffrey Hugo
  2021-05-11 14:46   ` Dmitry Baryshkov
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Jeffrey Hugo @ 2021-05-11 14:24 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Andy Gross, Kishon Vijay Abraham I, Vinod Koul, Rob Herring,
	sbillaka, Rob Clark, Dmitry Baryshkov, Stephen Boyd, MSM,
	linux-phy, DTML, lkml

On Mon, May 10, 2021 at 10:21 PM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> Many recent Qualcomm platforms comes with native DP and eDP support.
> This consists of a controller int he MDSS and a QMP-like PHY.

"in the"

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] phy: qcom: Introduce new eDP PHY driver
  2021-05-11  4:19 ` [PATCH 2/2] phy: qcom: Introduce new eDP PHY driver Bjorn Andersson
  2021-05-11  5:22   ` Stephen Boyd
  2021-05-11 14:24   ` Jeffrey Hugo
@ 2021-05-11 14:46   ` Dmitry Baryshkov
  2021-05-14 10:40   ` Vinod Koul
  2021-08-10  3:15   ` sbillaka
  4 siblings, 0 replies; 12+ messages in thread
From: Dmitry Baryshkov @ 2021-05-11 14:46 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Andy Gross, Kishon Vijay Abraham I, Vinod Koul, Rob Herring,
	Sankeerth Billakanti, Rob Clark, Stephen Boyd,
	open list:DRM DRIVER FOR MSM ADRENO GPU, linux-phy,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list

On Tue, 11 May 2021 at 07:19, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> Many recent Qualcomm platforms comes with native DP and eDP support.
> This consists of a controller int he MDSS and a QMP-like PHY.
>
> While similar to the well known QMP block, the eDP PHY only has TX lanes
> and the programming sequences are slightly different. Rather than
> continuing the trend of parameterize the QMP driver to pieces, this
> introduces the support as a new driver.
>
> The registration of link and pixel clocks are borrowed from the QMP
> driver. The non-DP link frequencies are omitted for now.
>
> The eDP PHY is very similar to the dedicated (non-USB) DP PHY, but only
> the prior is supported for now.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>  drivers/phy/qualcomm/Kconfig        |  10 +
>  drivers/phy/qualcomm/Makefile       |   1 +
>  drivers/phy/qualcomm/phy-qcom-edp.c | 750 ++++++++++++++++++++++++++++
>  3 files changed, 761 insertions(+)
>  create mode 100644 drivers/phy/qualcomm/phy-qcom-edp.c
>
> diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
> index 7f6fcb8ec5ba..13ae6f4f3112 100644
> --- a/drivers/phy/qualcomm/Kconfig
> +++ b/drivers/phy/qualcomm/Kconfig
> @@ -114,3 +114,13 @@ config PHY_QCOM_IPQ806X_USB
>           This option enables support for the Synopsis PHYs present inside the
>           Qualcomm USB3.0 DWC3 controller on ipq806x SoC. This driver supports
>           both HS and SS PHY controllers.
> +

Just a generic suggestion: what about naming it
PHY_QCOM_QMP_EDP/phy-qcom-qmp-edp.c, so that it stays close to the
"big" qmp driver and also so that other qmp parts can follow this
example if we ever split the bg driver in future?

> +config PHY_QCOM_EDP
> +       tristate "Qualcomm eDP PHY driver"
> +       depends on ARCH_QCOM || COMPILE_TEST
> +       depends on OF
> +       depends on COMMON_CLK
> +       select GENERIC_PHY
> +       help
> +         Enable this driver to support the Qualcomm eDP PHY found in various
> +         Qualcomm chipsets.
> diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
> index 47acbd7daa3a..1afd0882dbcc 100644
> --- a/drivers/phy/qualcomm/Makefile
> +++ b/drivers/phy/qualcomm/Makefile
> @@ -12,3 +12,4 @@ obj-$(CONFIG_PHY_QCOM_USB_HS_28NM)    += phy-qcom-usb-hs-28nm.o
>  obj-$(CONFIG_PHY_QCOM_USB_SS)          += phy-qcom-usb-ss.o
>  obj-$(CONFIG_PHY_QCOM_USB_SNPS_FEMTO_V2)+= phy-qcom-snps-femto-v2.o
>  obj-$(CONFIG_PHY_QCOM_IPQ806X_USB)             += phy-qcom-ipq806x-usb.o
> +obj-$(CONFIG_PHY_QCOM_EDP)             += phy-qcom-edp.o
> diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c
> new file mode 100644
> index 000000000000..ff0508e49548
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-edp.c
> @@ -0,0 +1,750 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2017, 2020, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2021, Linaro Ltd.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_address.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/reset.h>
> +#include <linux/slab.h>
> +
> +#include <dt-bindings/phy/phy.h>
> +
> +#define DP_PHY_PD_CTL_PWRDN                    0x001
> +#define DP_PHY_PD_CTL_PSR_PWRDN                        0x002
> +#define DP_PHY_PD_CTL_AUX_PWRDN                        0x004
> +#define DP_PHY_PD_CTL_LANE_0_1_PWRDN           0x008
> +#define DP_PHY_PD_CTL_LANE_2_3_PWRDN           0x010
> +#define DP_PHY_PD_CTL_PLL_PWRDN                        0x020
> +#define DP_PHY_PD_CTL_DP_CLAMP_EN              0x040
> +
> +#define DP_PHY_CFG                              0x0010
> +#define DP_PHY_CFG_1                            0x0014
> +#define DP_PHY_PD_CTL                           0x001C
> +#define DP_PHY_MODE                             0x0020
> +
> +#define DP_PHY_AUX_CFG0                                0x0020
> +#define DP_PHY_AUX_CFG1                                0x0024
> +#define DP_PHY_AUX_CFG2                                0x0028
> +#define DP_PHY_AUX_CFG3                                0x002c
> +#define DP_PHY_AUX_CFG4                                0x0030
> +#define DP_PHY_AUX_CFG5                                0x0034
> +#define DP_PHY_AUX_CFG6                                0x0038
> +#define DP_PHY_AUX_CFG7                                0x003c
> +#define DP_PHY_AUX_CFG8                                0x0040
> +#define DP_PHY_AUX_CFG9                                0x0044
> +
> +#define DP_PHY_VCO_DIV                          0x0074
> +#define DP_PHY_TX0_TX1_LANE_CTL                 0x007C
> +#define DP_PHY_TX2_TX3_LANE_CTL                 0x00A0
> +
> +#define DP_PHY_SPARE0                           0x00CC
> +#define DP_PHY_STATUS                           0x00E0
> +
> +/* Tx registers */
> +#define TXn_CLKBUF_ENABLE                       0x0000
> +#define TXn_TX_EMP_POST1_LVL                    0x0004
> +
> +#define TXn_TX_DRV_LVL                          0x0014
> +#define TXn_TX_DRV_LVL_OFFSET                   0x0018
> +#define TXn_RESET_TSYNC_EN                      0x001C
> +//#define TXn_PRE_STALL_LDO_BOOST_EN            0x0020
> +#define TXn_LDO_CONFIG                          0x0084
> +#define TXn_TX_BAND                             0x0028
> +#define TXn_INTERFACE_SELECT                    0x0024
> +
> +#define TXn_RES_CODE_LANE_OFFSET_TX0            0x0044
> +#define TXn_RES_CODE_LANE_OFFSET_TX1            0x0048
> +//#define TXn_RES_CODE_LANE_OFFSET_RX           0x0040
> +
> +#define TXn_TRANSCEIVER_BIAS_EN                 0x0054
> +#define TXn_HIGHZ_DRVR_EN                       0x0058
> +#define TXn_TX_POL_INV                          0x005C
> +#define TXn_PARRATE_REC_DETECT_IDLE_EN          0x0060
> +#define TXn_LANE_MODE_1                         0x0064
> +
> +#define TXn_TRAN_DRVR_EMP_EN                    0x0078
> +//#define TXn_TX_INTERFACE_MODE                 0x00BC
> +
> +#define TXn_VMODE_CTRL1                         0x007C
> +
> +/* PLL register offset */
> +#define QSERDES_COM_BG_TIMER                    0x000C
> +#define QSERDES_COM_BIAS_EN_CLKBUFLR_EN         0x0044
> +#define QSERDES_COM_CLK_ENABLE1                 0x0048
> +#define QSERDES_COM_SYS_CLK_CTRL                0x004C
> +#define QSERDES_COM_SYSCLK_BUF_ENABLE           0x0050
> +#define QSERDES_COM_PLL_IVCO                    0x0058
> +
> +#define QSERDES_COM_CP_CTRL_MODE0               0x0074
> +#define QSERDES_COM_PLL_RCTRL_MODE0             0x007C
> +#define QSERDES_COM_PLL_CCTRL_MODE0             0x0084
> +#define QSERDES_COM_SYSCLK_EN_SEL               0x0094
> +#define QSERDES_COM_RESETSM_CNTRL               0x009C
> +#define QSERDES_COM_LOCK_CMP_EN                 0x00A4
> +#define QSERDES_COM_LOCK_CMP1_MODE0             0x00AC
> +#define QSERDES_COM_LOCK_CMP2_MODE0             0x00B0
> +
> +#define QSERDES_COM_DEC_START_MODE0             0x00BC
> +#define QSERDES_COM_DIV_FRAC_START1_MODE0       0x00CC
> +#define QSERDES_COM_DIV_FRAC_START2_MODE0       0x00D0
> +#define QSERDES_COM_DIV_FRAC_START3_MODE0       0x00D4
> +#define QSERDES_COM_INTEGLOOP_GAIN0_MODE0       0x00EC
> +#define QSERDES_COM_INTEGLOOP_GAIN1_MODE0       0x00F0
> +#define QSERDES_COM_VCO_TUNE_CTRL               0x0108
> +#define QSERDES_COM_VCO_TUNE_MAP                0x010C
> +#define QSERDES_COM_VCO_TUNE1_MODE0             0x0110
> +#define QSERDES_COM_VCO_TUNE2_MODE0             0x0114
> +#define QSERDES_COM_CMN_STATUS                  0x0140
> +
> +#define QSERDES_COM_CLK_SEL                     0x0154
> +#define QSERDES_COM_HSCLK_SEL                   0x0158
> +
> +#define QSERDES_COM_CORECLK_DIV_MODE0           0x0168
> +
> +#define QSERDES_COM_CORE_CLK_EN                 0x0174
> +#define QSERDES_COM_C_READY_STATUS              0x0178
> +#define QSERDES_COM_CMN_CONFIG                  0x017C
> +
> +#define QSERDES_COM_SVS_MODE_CLK_SEL            0x0184
> +
> +#define QSERDES_COM_SSC_EN_CENTER               0x0010
> +#define QSERDES_COM_SSC_ADJ_PER1                0x0014
> +#define QSERDES_COM_SSC_ADJ_PER2                0x0018
> +#define QSERDES_COM_SSC_PER1                    0x001C
> +#define QSERDES_COM_SSC_PER2                    0x0020
> +#define QSERDES_COM_SSC_STEP_SIZE1_MODE0        0x0024
> +#define QSERDES_COM_SSC_STEP_SIZE2_MODE0        0x0028
> +
> +#define DP_PHY_PLL_POLL_SLEEP_US                500
> +#define DP_PHY_PLL_POLL_TIMEOUT_US              10000
> +
> +#define EDP_VCO_RATE_8100MHZDIV1000             8100000UL
> +#define EDP_VCO_RATE_8640MHZDIV1000             8640000UL
> +#define EDP_VCO_RATE_9720MHZDIV1000             9720000UL
> +#define EDP_VCO_RATE_10800MHZDIV1000            10800000UL
> +#define EDP_VCO_RATE_11880MHZDIV1000            11880000UL

Can we use phy-qcom-qmp.h for register difinitions?

> +
> +struct qcom_edp {
> +       struct device *dev;
> +
> +       struct phy *phy;
> +
> +       void __iomem *edp;
> +       void __iomem *tx0;
> +       void __iomem *tx1;
> +       void __iomem *pll;
> +
> +       struct clk_hw dp_link_hw;
> +       struct clk_hw dp_pixel_hw;
> +
> +       struct phy_configure_opts_dp dp_opts;
> +
> +       struct clk_bulk_data clks[2];
> +       struct regulator_bulk_data supplies[2];
> +} ;
> +
> +static int qcom_edp_phy_init(struct phy *phy)
> +{
> +       struct qcom_edp *edp = phy_get_drvdata(phy);
> +       int ret;
> +
> +       ret = regulator_bulk_enable(ARRAY_SIZE(edp->supplies), edp->supplies);
> +       if (ret)
> +               return ret;
> +
> +       ret = clk_bulk_prepare_enable(ARRAY_SIZE(edp->clks), edp->clks);
> +       if (ret)
> +               goto out_disable_supplies;
> +
> +       writel(DP_PHY_PD_CTL_PWRDN | DP_PHY_PD_CTL_AUX_PWRDN |
> +              DP_PHY_PD_CTL_PLL_PWRDN | DP_PHY_PD_CTL_DP_CLAMP_EN,
> +              edp->edp + DP_PHY_PD_CTL);
> +
> +       writel(0x17, edp->pll + QSERDES_COM_BIAS_EN_CLKBUFLR_EN);
> +
> +       writel(DP_PHY_PD_CTL_PSR_PWRDN, edp->edp + DP_PHY_PD_CTL);
> +       msleep(20);
> +
> +       writel(DP_PHY_PD_CTL_PWRDN | DP_PHY_PD_CTL_AUX_PWRDN |
> +              DP_PHY_PD_CTL_LANE_0_1_PWRDN | DP_PHY_PD_CTL_LANE_2_3_PWRDN |
> +              DP_PHY_PD_CTL_PLL_PWRDN | DP_PHY_PD_CTL_DP_CLAMP_EN,
> +              edp->edp + DP_PHY_PD_CTL);
> +
> +       writel(0x00, edp->edp + DP_PHY_AUX_CFG0);
> +       writel(0x13, edp->edp + DP_PHY_AUX_CFG1);
> +       writel(0x24, edp->edp + DP_PHY_AUX_CFG2);
> +       writel(0x00, edp->edp + DP_PHY_AUX_CFG3);
> +       writel(0x0a, edp->edp + DP_PHY_AUX_CFG4);
> +       writel(0x26, edp->edp + DP_PHY_AUX_CFG5);
> +       writel(0x0a, edp->edp + DP_PHY_AUX_CFG6);
> +       writel(0x03, edp->edp + DP_PHY_AUX_CFG7);
> +       writel(0x37, edp->edp + DP_PHY_AUX_CFG8);
> +       writel(0x03, edp->edp + DP_PHY_AUX_CFG9);
> +
> +       writel(0x1f, edp->edp + 0x58);
> +
> +       msleep(20);
> +
> +       return 0;
> +
> +out_disable_supplies:
> +       regulator_bulk_disable(ARRAY_SIZE(edp->supplies), edp->supplies);
> +
> +       return ret;
> +}
> +
> +static int qcom_edp_phy_configure(struct phy *phy, union phy_configure_opts *opts)
> +{
> +       const struct phy_configure_opts_dp *dp_opts = &opts->dp;
> +       struct qcom_edp *edp = phy_get_drvdata(phy);
> +
> +       memcpy(&edp->dp_opts, dp_opts, sizeof(*dp_opts));
> +
> +       return 0;
> +}
> +
> +static int qcom_edp_configure_ssc(struct qcom_edp *edp)
> +{
> +       const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
> +       u32 step1;
> +       u32 step2;
> +
> +       switch (dp_opts->link_rate) {
> +       case 1620:
> +       case 2700:
> +       case 8100:
> +               step1 = 0x45;
> +               step2 = 0x06;
> +               break;
> +       case 5400:
> +               step1 = 0x5c;
> +               step2 = 0x08;
> +               break;
> +       default:
> +               /* Other link rates aren't supported */
> +               return -EINVAL;
> +       }
> +
> +       writel(0x01, edp->pll + QSERDES_COM_SSC_EN_CENTER);
> +       writel(0x00, edp->pll + QSERDES_COM_SSC_ADJ_PER1);
> +       writel(0x36, edp->pll + QSERDES_COM_SSC_PER1);
> +       writel(0x01, edp->pll + QSERDES_COM_SSC_PER2);
> +       writel(step1, edp->pll + QSERDES_COM_SSC_STEP_SIZE1_MODE0);
> +       writel(step2, edp->pll + QSERDES_COM_SSC_STEP_SIZE2_MODE0);
> +
> +       return 0;
> +}
> +
> +static int qcom_edp_configure_pll(struct qcom_edp *edp)
> +{
> +       const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
> +       u32 div_frac_start2_mode0;
> +       u32 div_frac_start3_mode0;
> +       u32 dec_start_mode0;
> +       u32 lock_cmp1_mode0;
> +       u32 lock_cmp2_mode0;
> +       u32 hsclk_sel;
> +
> +       switch (dp_opts->link_rate) {
> +       case 1620:
> +               hsclk_sel = 0x5;
> +               dec_start_mode0 = 0x69;
> +               div_frac_start2_mode0 = 0x80;
> +               div_frac_start3_mode0 = 0x07;
> +               lock_cmp1_mode0 = 0x6f;
> +               lock_cmp2_mode0 = 0x08;
> +               break;
> +       case 2700:
> +               hsclk_sel = 0x3;
> +               dec_start_mode0 = 0x69;
> +               div_frac_start2_mode0 = 0x80;
> +               div_frac_start3_mode0 = 0x07;
> +               lock_cmp1_mode0 = 0x0f;
> +               lock_cmp2_mode0 = 0x0e;
> +               break;
> +       case 5400:
> +               hsclk_sel = 0x1;
> +               dec_start_mode0 = 0x8c;
> +               div_frac_start2_mode0 = 0x00;
> +               div_frac_start3_mode0 = 0x0a;
> +               lock_cmp1_mode0 = 0x1f;
> +               lock_cmp2_mode0 = 0x1c;
> +               break;
> +       case 8100:
> +               hsclk_sel = 0x0;
> +               dec_start_mode0 = 0x69;
> +               div_frac_start2_mode0 = 0x80;
> +               div_frac_start3_mode0 = 0x07;
> +               lock_cmp1_mode0 = 0x2f;
> +               lock_cmp2_mode0 = 0x2a;
> +               break;
> +       default:
> +               /* Other link rates aren't supported */
> +               return -EINVAL;
> +       }
> +
> +       writel(0x01, edp->pll + QSERDES_COM_SVS_MODE_CLK_SEL);
> +       writel(0x0b, edp->pll + QSERDES_COM_SYSCLK_EN_SEL);
> +       writel(0x02, edp->pll + QSERDES_COM_SYS_CLK_CTRL);
> +       writel(0x0c, edp->pll + QSERDES_COM_CLK_ENABLE1);
> +       writel(0x06, edp->pll + QSERDES_COM_SYSCLK_BUF_ENABLE);
> +       writel(0x30, edp->pll + QSERDES_COM_CLK_SEL);
> +       writel(hsclk_sel, edp->pll + QSERDES_COM_HSCLK_SEL);
> +       writel(0x0f, edp->pll + QSERDES_COM_PLL_IVCO);
> +       writel(0x08, edp->pll + QSERDES_COM_LOCK_CMP_EN);
> +       writel(0x36, edp->pll + QSERDES_COM_PLL_CCTRL_MODE0);
> +       writel(0x16, edp->pll + QSERDES_COM_PLL_RCTRL_MODE0);
> +       writel(0x06, edp->pll + QSERDES_COM_CP_CTRL_MODE0);
> +       writel(dec_start_mode0, edp->pll + QSERDES_COM_DEC_START_MODE0);
> +       writel(0x00, edp->pll + QSERDES_COM_DIV_FRAC_START1_MODE0);
> +       writel(div_frac_start2_mode0, edp->pll + QSERDES_COM_DIV_FRAC_START2_MODE0);
> +       writel(div_frac_start3_mode0, edp->pll + QSERDES_COM_DIV_FRAC_START3_MODE0);
> +       writel(0x02, edp->pll + QSERDES_COM_CMN_CONFIG);
> +       writel(0x3f, edp->pll + QSERDES_COM_INTEGLOOP_GAIN0_MODE0);
> +       writel(0x00, edp->pll + QSERDES_COM_INTEGLOOP_GAIN1_MODE0);
> +       writel(0x00, edp->pll + QSERDES_COM_VCO_TUNE_MAP);
> +       writel(lock_cmp1_mode0, edp->pll + QSERDES_COM_LOCK_CMP1_MODE0);
> +       writel(lock_cmp2_mode0, edp->pll + QSERDES_COM_LOCK_CMP2_MODE0);
> +
> +       writel(0x0a, edp->pll + QSERDES_COM_BG_TIMER);
> +       writel(0x14, edp->pll + QSERDES_COM_CORECLK_DIV_MODE0);
> +       writel(0x00, edp->pll + QSERDES_COM_VCO_TUNE_CTRL);
> +       writel(0x17, edp->pll + QSERDES_COM_BIAS_EN_CLKBUFLR_EN);
> +       writel(0x0f, edp->pll + QSERDES_COM_CORE_CLK_EN);
> +       writel(0xa0, edp->pll + QSERDES_COM_VCO_TUNE1_MODE0);
> +       writel(0x03, edp->pll + QSERDES_COM_VCO_TUNE2_MODE0);
> +
> +       return 0;
> +}
> +
> +static int qcom_edp_set_vco_div(struct qcom_edp *edp)
> +{
> +       const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
> +       unsigned long pixel_freq;
> +       u32 vco_div;
> +
> +       switch (dp_opts->link_rate) {
> +       case 1620:
> +               vco_div = 0x1;
> +               pixel_freq = 1620000000UL / 2;
> +               break;
> +       case 2700:
> +               vco_div = 0x1;
> +               pixel_freq = 2700000000UL / 2;
> +               break;
> +       case 5400:
> +               vco_div = 0x2;
> +               pixel_freq = 5400000000UL / 4;
> +               break;
> +       case 8100:
> +               vco_div = 0x0;
> +               pixel_freq = 8100000000UL / 6;
> +               break;
> +       default:
> +               /* Other link rates aren't supported */
> +               return -EINVAL;
> +       }
> +
> +       writel(vco_div, edp->edp + DP_PHY_VCO_DIV);
> +
> +       clk_set_rate(edp->dp_link_hw.clk, dp_opts->link_rate * 100000);
> +       clk_set_rate(edp->dp_pixel_hw.clk, pixel_freq);
> +
> +       return 0;
> +}
> +
> +static int qcom_edp_phy_power_on(struct phy *phy)
> +{
> +       struct qcom_edp *edp = phy_get_drvdata(phy);
> +       int timeout;
> +       int ret;
> +       u32 val;
> +
> +       writel(DP_PHY_PD_CTL_PWRDN | DP_PHY_PD_CTL_AUX_PWRDN |
> +              DP_PHY_PD_CTL_LANE_0_1_PWRDN | DP_PHY_PD_CTL_LANE_2_3_PWRDN |
> +              DP_PHY_PD_CTL_PLL_PWRDN | DP_PHY_PD_CTL_DP_CLAMP_EN,
> +              edp->edp + DP_PHY_PD_CTL);
> +       writel(0xfc, edp->edp + DP_PHY_MODE);
> +
> +       timeout = readl_poll_timeout(edp->pll + QSERDES_COM_CMN_STATUS,
> +                                    val, val & BIT(7), 5, 200);
> +       if (timeout)
> +               return timeout;
> +
> +       writel(0x01, edp->tx0 + TXn_LDO_CONFIG);
> +       writel(0x01, edp->tx1 + TXn_LDO_CONFIG);
> +       writel(0x00, edp->tx0 + TXn_LANE_MODE_1);
> +       writel(0x00, edp->tx1 + TXn_LANE_MODE_1);
> +
> +       ret = qcom_edp_configure_ssc(edp);
> +       if (ret)
> +               return ret;
> +
> +       ret = qcom_edp_configure_pll(edp);
> +       if (ret)
> +               return ret;
> +
> +       /* TX Lane configuration */
> +       writel(0x05, edp->edp + DP_PHY_TX0_TX1_LANE_CTL);
> +       writel(0x05, edp->edp + DP_PHY_TX2_TX3_LANE_CTL);
> +
> +       /* TX-0 register configuration */
> +       writel(0x03, edp->tx0 + TXn_TRANSCEIVER_BIAS_EN);
> +       writel(0x0f, edp->tx0 + TXn_CLKBUF_ENABLE);
> +       writel(0x03, edp->tx0 + TXn_RESET_TSYNC_EN);
> +       writel(0x01, edp->tx0 + TXn_TRAN_DRVR_EMP_EN);
> +       writel(0x04, edp->tx0 + TXn_TX_BAND);
> +
> +       /* TX-1 register configuration */
> +       writel(0x03, edp->tx1 + TXn_TRANSCEIVER_BIAS_EN);
> +       writel(0x0f, edp->tx1 + TXn_CLKBUF_ENABLE);
> +       writel(0x03, edp->tx1 + TXn_RESET_TSYNC_EN);
> +       writel(0x01, edp->tx1 + TXn_TRAN_DRVR_EMP_EN);
> +       writel(0x04, edp->tx1 + TXn_TX_BAND);
> +
> +       ret = qcom_edp_set_vco_div(edp);
> +       if (ret)
> +               return ret;
> +
> +       writel(0x01, edp->edp + DP_PHY_CFG);
> +        writel(0x05, edp->edp + DP_PHY_CFG);
> +        writel(0x01, edp->edp + DP_PHY_CFG);
> +        writel(0x09, edp->edp + DP_PHY_CFG);
> +
> +        writel(0x20, edp->pll + QSERDES_COM_RESETSM_CNTRL);
> +
> +       timeout = readl_poll_timeout(edp->pll + QSERDES_COM_C_READY_STATUS,
> +                                    val, val & BIT(0), 500, 10000);
> +       if (timeout)
> +               return timeout;
> +
> +       writel(0x19, edp->edp + DP_PHY_CFG);
> +       writel(0x1f, edp->tx0 + TXn_HIGHZ_DRVR_EN);
> +       writel(0x04, edp->tx0 + TXn_HIGHZ_DRVR_EN);
> +       writel(0x00, edp->tx0 + TXn_TX_POL_INV);
> +       writel(0x1f, edp->tx1 + TXn_HIGHZ_DRVR_EN);
> +       writel(0x04, edp->tx1 + TXn_HIGHZ_DRVR_EN);
> +       writel(0x00, edp->tx1 + TXn_TX_POL_INV);
> +       writel(0x10, edp->tx0 + TXn_TX_DRV_LVL_OFFSET);
> +       writel(0x10, edp->tx1 + TXn_TX_DRV_LVL_OFFSET);
> +       writel(0x11, edp->tx0 + TXn_RES_CODE_LANE_OFFSET_TX0);
> +       writel(0x11, edp->tx0 + TXn_RES_CODE_LANE_OFFSET_TX1);
> +       writel(0x11, edp->tx1 + TXn_RES_CODE_LANE_OFFSET_TX0);
> +       writel(0x11, edp->tx1 + TXn_RES_CODE_LANE_OFFSET_TX1);
> +
> +       writel(0x10, edp->tx0 + TXn_TX_EMP_POST1_LVL);
> +       writel(0x10, edp->tx1 + TXn_TX_EMP_POST1_LVL);
> +       writel(0x1f, edp->tx0 + TXn_TX_DRV_LVL);
> +       writel(0x1f, edp->tx1 + TXn_TX_DRV_LVL);
> +
> +       writel(0x4, edp->tx0 + TXn_HIGHZ_DRVR_EN);
> +       writel(0x3, edp->tx0 + TXn_TRANSCEIVER_BIAS_EN);
> +       writel(0x4, edp->tx1 + TXn_HIGHZ_DRVR_EN);
> +       writel(0x0, edp->tx1 + TXn_TRANSCEIVER_BIAS_EN);
> +       writel(0x3, edp->edp + DP_PHY_CFG_1);
> +
> +       writel(0x18, edp->edp + DP_PHY_CFG);
> +       udelay(100);
> +
> +       writel(0x19, edp->edp + DP_PHY_CFG);
> +
> +       return readl_poll_timeout(edp->edp + DP_PHY_STATUS,
> +                                 val, val & BIT(1), 500, 10000);
> +}
> +
> +static int qcom_edp_phy_power_off(struct phy *phy)
> +{
> +       struct qcom_edp *edp = phy_get_drvdata(phy);
> +
> +       writel(DP_PHY_PD_CTL_PSR_PWRDN, edp->edp + DP_PHY_PD_CTL);
> +
> +       return 0;
> +}
> +
> +static int qcom_edp_phy_exit(struct phy *phy)
> +{
> +       struct qcom_edp *edp = phy_get_drvdata(phy);
> +
> +       clk_bulk_disable_unprepare(ARRAY_SIZE(edp->clks), edp->clks);
> +       regulator_bulk_disable(ARRAY_SIZE(edp->supplies), edp->supplies);
> +
> +       return 0;
> +}
> +
> +static const struct phy_ops qcom_edp_ops = {
> +       .init           = qcom_edp_phy_init,
> +       .configure      = qcom_edp_phy_configure,
> +       .power_on       = qcom_edp_phy_power_on,
> +       .power_off      = qcom_edp_phy_power_off,
> +       .exit           = qcom_edp_phy_exit,
> +       .owner          = THIS_MODULE,
> +};
> +
> +/*
> + * Display Port PLL driver block diagram for branch clocks
> + *
> + *              +------------------------------+
> + *              |         DP_VCO_CLK           |
> + *              |                              |
> + *              |    +-------------------+     |
> + *              |    |   (DP PLL/VCO)    |     |
> + *              |    +---------+---------+     |
> + *              |              v               |
> + *              |   +----------+-----------+   |
> + *              |   | hsclk_divsel_clk_src |   |
> + *              |   +----------+-----------+   |
> + *              +------------------------------+
> + *                              |
> + *          +---------<---------v------------>----------+
> + *          |                                           |
> + * +--------v----------------+                          |
> + * |    dp_phy_pll_link_clk  |                          |
> + * |     link_clk            |                          |
> + * +--------+----------------+                          |
> + *          |                                           |
> + *          |                                           |
> + *          v                                           v
> + * Input to DISPCC block                                |
> + * for link clk, crypto clk                             |
> + * and interface clock                                  |
> + *                                                      |
> + *                                                      |
> + *      +--------<------------+-----------------+---<---+
> + *      |                     |                 |
> + * +----v---------+  +--------v-----+  +--------v------+
> + * | vco_divided  |  | vco_divided  |  | vco_divided   |
> + * |    _clk_src  |  |    _clk_src  |  |    _clk_src   |
> + * |              |  |              |  |               |
> + * |divsel_six    |  |  divsel_two  |  |  divsel_four  |
> + * +-------+------+  +-----+--------+  +--------+------+
> + *         |                 |                  |
> + *         v---->----------v-------------<------v
> + *                         |
> + *              +----------+-----------------+
> + *              |   dp_phy_pll_vco_div_clk   |
> + *              +---------+------------------+
> + *                        |
> + *                        v
> + *              Input to DISPCC block
> + *              for DP pixel clock
> + *
> + */
> +static int qcom_edp_dp_pixel_clk_determine_rate(struct clk_hw *hw,
> +                                               struct clk_rate_request *req)
> +{
> +       switch (req->rate) {
> +       case 1620000000UL / 2:
> +       case 2700000000UL / 2:
> +       /* 5.4 and 8.1 GHz are same link rate as 2.7GHz, i.e. div 4 and div 6 */
> +               return 0;
> +       default:
> +               return -EINVAL;
> +       }
> +}
> +
> +static unsigned long
> +qcom_edp_dp_pixel_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
> +{
> +       const struct qcom_edp *edp = container_of(hw, struct qcom_edp, dp_pixel_hw);
> +       const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
> +
> +       switch (dp_opts->link_rate) {
> +       case 1620:
> +               return 1620000000UL / 2;
> +       case 2700:
> +               return 2700000000UL / 2;
> +       case 5400:
> +               return 5400000000UL / 4;
> +       case 8100:
> +               return 8100000000UL / 6;
> +       default:
> +               return 0;
> +       }
> +}
> +
> +static const struct clk_ops qcom_edp_dp_pixel_clk_ops = {
> +       .determine_rate = qcom_edp_dp_pixel_clk_determine_rate,
> +       .recalc_rate = qcom_edp_dp_pixel_clk_recalc_rate,
> +};
> +
> +static int qcom_edp_dp_link_clk_determine_rate(struct clk_hw *hw,
> +                                              struct clk_rate_request *req)
> +{
> +       switch (req->rate) {
> +       case 162000000:
> +       case 270000000:
> +       case 540000000:
> +       case 810000000:
> +               return 0;
> +       default:
> +               return -EINVAL;
> +       }
> +}
> +
> +static unsigned long
> +qcom_edp_dp_link_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
> +{
> +       const struct qcom_edp *edp = container_of(hw, struct qcom_edp, dp_link_hw);
> +       const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
> +
> +       switch (dp_opts->link_rate) {
> +       case 1620:
> +       case 2700:
> +       case 5400:
> +       case 8100:
> +               return dp_opts->link_rate * 100000;
> +       default:
> +               return 0;
> +       }
> +}
> +
> +static const struct clk_ops qcom_edp_dp_link_clk_ops = {
> +       .determine_rate = qcom_edp_dp_link_clk_determine_rate,
> +       .recalc_rate = qcom_edp_dp_link_clk_recalc_rate,
> +};
> +
> +static struct clk_hw *
> +qcom_edp_dp_clks_hw_get(struct of_phandle_args *clkspec, void *data)
> +{
> +       unsigned int idx = clkspec->args[0];
> +       struct qcom_edp *edp = data;
> +
> +       if (idx >= 2) {
> +               pr_err("%s: invalid index %u\n", __func__, idx);
> +               return ERR_PTR(-EINVAL);
> +       }
> +
> +       if (idx == 0)
> +               return &edp->dp_link_hw;
> +
> +       return &edp->dp_pixel_hw;
> +}
> +
> +static void phy_clk_release_provider(void *res)
> +{
> +       of_clk_del_provider(res);
> +}
> +
> +static int qcom_edp_clks_register(struct qcom_edp *edp, struct device_node *np)
> +{
> +       struct clk_init_data init = { };
> +       int ret;
> +
> +       init.ops = &qcom_edp_dp_link_clk_ops;
> +       init.name = "edp_phy_pll_link_clk";
> +       edp->dp_link_hw.init = &init;
> +       ret = devm_clk_hw_register(edp->dev, &edp->dp_link_hw);
> +       if (ret)
> +               return ret;
> +
> +       init.ops = &qcom_edp_dp_pixel_clk_ops;
> +       init.name = "edp_phy_pll_vco_div_clk";
> +       edp->dp_pixel_hw.init = &init;
> +       ret = devm_clk_hw_register(edp->dev, &edp->dp_pixel_hw);
> +       if (ret)
> +               return ret;
> +
> +       ret = of_clk_add_hw_provider(np, qcom_edp_dp_clks_hw_get, edp);

I think we can use of_clk_onecell_get here (and we should change main
qmp driver)
Also we can use devm_of_clk_add_hw_provider.

> +       if (ret)
> +               return ret;
> +
> +       /*
> +        * Roll a devm action because the clock provider is the child node, but
> +        * the child node is not actually a device.
> +        */
> +       ret = devm_add_action(edp->dev, phy_clk_release_provider, np);
> +       if (ret)
> +               phy_clk_release_provider(np);
> +
> +       return ret;
> +}
> +
> +static int qcom_edp_phy_probe(struct platform_device *pdev)
> +{
> +       struct phy_provider *phy_provider;
> +       struct device *dev = &pdev->dev;
> +       struct qcom_edp *edp;
> +       int ret;
> +
> +       edp = devm_kzalloc(dev, sizeof(*edp), GFP_KERNEL);
> +       if (!edp)
> +               return -ENOMEM;
> +
> +       edp->dev = dev;
> +
> +       edp->edp = devm_platform_ioremap_resource(pdev, 0);
> +       if (IS_ERR(edp->edp))
> +               return PTR_ERR(edp->edp);
> +
> +       edp->tx0 = devm_platform_ioremap_resource(pdev, 1);
> +       if (IS_ERR(edp->tx0))
> +               return PTR_ERR(edp->tx0);
> +
> +       edp->tx1 = devm_platform_ioremap_resource(pdev, 2);
> +       if (IS_ERR(edp->tx1))
> +               return PTR_ERR(edp->tx1);
> +
> +       edp->pll = devm_platform_ioremap_resource(pdev, 3);
> +       if (IS_ERR(edp->pll))
> +               return PTR_ERR(edp->pll);
> +
> +       edp->clks[0].id = "aux";
> +       edp->clks[1].id = "cfg_ahb";
> +       ret = devm_clk_bulk_get(dev, ARRAY_SIZE(edp->clks), edp->clks);
> +       if (ret)
> +               return ret;
> +
> +       edp->supplies[0].supply = "vdda-phy";
> +       edp->supplies[1].supply = "vdda-pll";
> +       ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(edp->supplies), edp->supplies);
> +       if (ret)
> +               return ret;
> +
> +       ret = qcom_edp_clks_register(edp, pdev->dev.of_node);
> +       if (ret)
> +               return ret;
> +
> +       edp->phy = devm_phy_create(dev, pdev->dev.of_node, &qcom_edp_ops);
> +       if (IS_ERR(edp->phy)) {
> +               dev_err(dev, "failed to register phy\n");
> +               return PTR_ERR(edp->phy);
> +       }
> +
> +       phy_set_drvdata(edp->phy, edp);
> +
> +       phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +       return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +static const struct of_device_id qcom_edp_phy_match_table[] = {
> +       { .compatible = "qcom,sc8180x-edp-phy" },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(of, qcom_edp_phy_match_table);
> +
> +static struct platform_driver qcom_edp_phy_driver = {
> +       .probe          = qcom_edp_phy_probe,
> +       .driver = {
> +               .name   = "qcom-edp-phy",
> +               .of_match_table = qcom_edp_phy_match_table,
> +       },
> +};
> +
> +module_platform_driver(qcom_edp_phy_driver);
> +
> +MODULE_DESCRIPTION("Qualcomm eDP PHY driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.29.2
>


-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] dt-bindings: phy: Introduce Qualcomm eDP/DP PHY binding
  2021-05-11  4:19 [PATCH 1/2] dt-bindings: phy: Introduce Qualcomm eDP/DP PHY binding Bjorn Andersson
  2021-05-11  4:19 ` [PATCH 2/2] phy: qcom: Introduce new eDP PHY driver Bjorn Andersson
@ 2021-05-11 17:24 ` Stephen Boyd
  2021-05-17 21:46   ` Rob Herring
  1 sibling, 1 reply; 12+ messages in thread
From: Stephen Boyd @ 2021-05-11 17:24 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Dmitry Baryshkov,
	Kishon Vijay Abraham I, Rob Clark, Rob Herring, Vinod Koul,
	sbillaka
  Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel

Quoting Bjorn Andersson (2021-05-10 21:19:29)
> diff --git a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml
> new file mode 100644
> index 000000000000..c258e4f7e332
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml
> @@ -0,0 +1,69 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/phy/qcom,edp-phy.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Qualcomm DP/eDP PHY
> +
> +maintainers:
> +  - Bjorn Andersson <bjorn.andersson@linaro.org>
> +
> +description:
> +  The Qualcomm DP/eDP PHY is found in a number of Qualcomm platform and
> +  provides the physical interface for DisplayPort and Embedded Display Port.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,sc8180x-dp-phy
> +      - qcom,sc8180x-edp-phy
> +
> +  reg:
> +    items:
> +      - description: PHY base register block
> +      - description: tx0 register block
> +      - description: tx1 register block
> +      - description: PLL register block
> +
> +  clocks:
> +    maxItems: 2

Can this be items: - const: again? I think having maxItems means you can
have one or two items, which wouldn't match the number of clock-names
then.

> +
> +  clock-names:
> +    items:
> +      - const: aux
> +      - const: cfg_ahb
> +

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] phy: qcom: Introduce new eDP PHY driver
  2021-05-11  5:22   ` Stephen Boyd
@ 2021-05-14 10:38     ` Vinod Koul
  0 siblings, 0 replies; 12+ messages in thread
From: Vinod Koul @ 2021-05-14 10:38 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Andy Gross, Bjorn Andersson, Dmitry Baryshkov,
	Kishon Vijay Abraham I, Rob Clark, Rob Herring, sbillaka,
	linux-arm-msm, linux-phy, devicetree, linux-kernel

On 10-05-21, 22:22, Stephen Boyd wrote:
> Quoting Bjorn Andersson (2021-05-10 21:19:30)
> > Many recent Qualcomm platforms comes with native DP and eDP support.
> > This consists of a controller int he MDSS and a QMP-like PHY.
> >
> > While similar to the well known QMP block, the eDP PHY only has TX lanes
> > and the programming sequences are slightly different. Rather than
> > continuing the trend of parameterize the QMP driver to pieces, this
> > introduces the support as a new driver.
> 
> Thank you for not slamming it into the same mega driver.
> 
> >
> > The registration of link and pixel clocks are borrowed from the QMP
> > driver. The non-DP link frequencies are omitted for now.
> 
> Can we make some library code for "DP" stuff in qmp that the two can
> call to share the logic?

I think we should split QMP into a library of common code which the
respective function driver DP, UFS, USB QMP drivers (re)use across...

-- 
~Vinod

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] phy: qcom: Introduce new eDP PHY driver
  2021-05-11  4:19 ` [PATCH 2/2] phy: qcom: Introduce new eDP PHY driver Bjorn Andersson
                     ` (2 preceding siblings ...)
  2021-05-11 14:46   ` Dmitry Baryshkov
@ 2021-05-14 10:40   ` Vinod Koul
  2021-08-10  3:15   ` sbillaka
  4 siblings, 0 replies; 12+ messages in thread
From: Vinod Koul @ 2021-05-14 10:40 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Andy Gross, Kishon Vijay Abraham I, Rob Herring, sbillaka,
	Rob Clark, Dmitry Baryshkov, Stephen Boyd, linux-arm-msm,
	linux-phy, devicetree, linux-kernel

On 10-05-21, 23:19, Bjorn Andersson wrote:

> +static int qcom_edp_phy_power_on(struct phy *phy)
> +{
> +	struct qcom_edp *edp = phy_get_drvdata(phy);
> +	int timeout;
> +	int ret;
> +	u32 val;
> +
> +	writel(DP_PHY_PD_CTL_PWRDN | DP_PHY_PD_CTL_AUX_PWRDN |
> +	       DP_PHY_PD_CTL_LANE_0_1_PWRDN | DP_PHY_PD_CTL_LANE_2_3_PWRDN |
> +	       DP_PHY_PD_CTL_PLL_PWRDN | DP_PHY_PD_CTL_DP_CLAMP_EN,
> +	       edp->edp + DP_PHY_PD_CTL);
> +	writel(0xfc, edp->edp + DP_PHY_MODE);
> +
> +	timeout = readl_poll_timeout(edp->pll + QSERDES_COM_CMN_STATUS,
> +				     val, val & BIT(7), 5, 200);
> +	if (timeout)
> +		return timeout;
> +
> +	writel(0x01, edp->tx0 + TXn_LDO_CONFIG);
> +	writel(0x01, edp->tx1 + TXn_LDO_CONFIG);
> +	writel(0x00, edp->tx0 + TXn_LANE_MODE_1);
> +	writel(0x00, edp->tx1 + TXn_LANE_MODE_1);
> +
> +	ret = qcom_edp_configure_ssc(edp);
> +	if (ret)
> +		return ret;
> +
> +	ret = qcom_edp_configure_pll(edp);
> +	if (ret)
> +		return ret;
> +
> +	/* TX Lane configuration */
> +	writel(0x05, edp->edp + DP_PHY_TX0_TX1_LANE_CTL);
> +	writel(0x05, edp->edp + DP_PHY_TX2_TX3_LANE_CTL);
> +
> +	/* TX-0 register configuration */
> +	writel(0x03, edp->tx0 + TXn_TRANSCEIVER_BIAS_EN);
> +	writel(0x0f, edp->tx0 + TXn_CLKBUF_ENABLE);
> +	writel(0x03, edp->tx0 + TXn_RESET_TSYNC_EN);
> +	writel(0x01, edp->tx0 + TXn_TRAN_DRVR_EMP_EN);
> +	writel(0x04, edp->tx0 + TXn_TX_BAND);
> +
> +	/* TX-1 register configuration */
> +	writel(0x03, edp->tx1 + TXn_TRANSCEIVER_BIAS_EN);
> +	writel(0x0f, edp->tx1 + TXn_CLKBUF_ENABLE);
> +	writel(0x03, edp->tx1 + TXn_RESET_TSYNC_EN);
> +	writel(0x01, edp->tx1 + TXn_TRAN_DRVR_EMP_EN);
> +	writel(0x04, edp->tx1 + TXn_TX_BAND);
> +
> +	ret = qcom_edp_set_vco_div(edp);
> +	if (ret)
> +		return ret;
> +
> +	writel(0x01, edp->edp + DP_PHY_CFG);
> +        writel(0x05, edp->edp + DP_PHY_CFG);
> +        writel(0x01, edp->edp + DP_PHY_CFG);
> +        writel(0x09, edp->edp + DP_PHY_CFG);
> +
> +        writel(0x20, edp->pll + QSERDES_COM_RESETSM_CNTRL);	
> +
> +	timeout = readl_poll_timeout(edp->pll + QSERDES_COM_C_READY_STATUS,
> +				     val, val & BIT(0), 500, 10000);
> +	if (timeout)
> +		return timeout;
> +
> +	writel(0x19, edp->edp + DP_PHY_CFG);
> +	writel(0x1f, edp->tx0 + TXn_HIGHZ_DRVR_EN);
> +	writel(0x04, edp->tx0 + TXn_HIGHZ_DRVR_EN);
> +	writel(0x00, edp->tx0 + TXn_TX_POL_INV);
> +	writel(0x1f, edp->tx1 + TXn_HIGHZ_DRVR_EN);
> +	writel(0x04, edp->tx1 + TXn_HIGHZ_DRVR_EN);
> +	writel(0x00, edp->tx1 + TXn_TX_POL_INV);
> +	writel(0x10, edp->tx0 + TXn_TX_DRV_LVL_OFFSET);
> +	writel(0x10, edp->tx1 + TXn_TX_DRV_LVL_OFFSET);
> +	writel(0x11, edp->tx0 + TXn_RES_CODE_LANE_OFFSET_TX0);
> +	writel(0x11, edp->tx0 + TXn_RES_CODE_LANE_OFFSET_TX1);
> +	writel(0x11, edp->tx1 + TXn_RES_CODE_LANE_OFFSET_TX0);
> +	writel(0x11, edp->tx1 + TXn_RES_CODE_LANE_OFFSET_TX1);
> +
> +	writel(0x10, edp->tx0 + TXn_TX_EMP_POST1_LVL);
> +	writel(0x10, edp->tx1 + TXn_TX_EMP_POST1_LVL);
> +	writel(0x1f, edp->tx0 + TXn_TX_DRV_LVL);
> +	writel(0x1f, edp->tx1 + TXn_TX_DRV_LVL);
> +
> +	writel(0x4, edp->tx0 + TXn_HIGHZ_DRVR_EN);
> +	writel(0x3, edp->tx0 + TXn_TRANSCEIVER_BIAS_EN);
> +	writel(0x4, edp->tx1 + TXn_HIGHZ_DRVR_EN);
> +	writel(0x0, edp->tx1 + TXn_TRANSCEIVER_BIAS_EN);
> +	writel(0x3, edp->edp + DP_PHY_CFG_1);
> +
> +	writel(0x18, edp->edp + DP_PHY_CFG);

Can't help but think that this should be made modular. We would come up
with next set which would have different set of sequence/values so
having a qmp style table here would be better..

-- 
~Vinod

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] dt-bindings: phy: Introduce Qualcomm eDP/DP PHY binding
  2021-05-11 17:24 ` [PATCH 1/2] dt-bindings: phy: Introduce Qualcomm eDP/DP PHY binding Stephen Boyd
@ 2021-05-17 21:46   ` Rob Herring
  0 siblings, 0 replies; 12+ messages in thread
From: Rob Herring @ 2021-05-17 21:46 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Andy Gross, Bjorn Andersson, Dmitry Baryshkov,
	Kishon Vijay Abraham I, Rob Clark, Vinod Koul, sbillaka,
	linux-arm-msm, linux-phy, devicetree, linux-kernel

On Tue, May 11, 2021 at 10:24:06AM -0700, Stephen Boyd wrote:
> Quoting Bjorn Andersson (2021-05-10 21:19:29)
> > diff --git a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml
> > new file mode 100644
> > index 000000000000..c258e4f7e332
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml
> > @@ -0,0 +1,69 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +
> > +%YAML 1.2
> > +---
> > +$id: "http://devicetree.org/schemas/phy/qcom,edp-phy.yaml#"
> > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > +
> > +title: Qualcomm DP/eDP PHY
> > +
> > +maintainers:
> > +  - Bjorn Andersson <bjorn.andersson@linaro.org>
> > +
> > +description:
> > +  The Qualcomm DP/eDP PHY is found in a number of Qualcomm platform and
> > +  provides the physical interface for DisplayPort and Embedded Display Port.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - qcom,sc8180x-dp-phy
> > +      - qcom,sc8180x-edp-phy

Is there a difference in DP and eDP?

> > +
> > +  reg:
> > +    items:
> > +      - description: PHY base register block
> > +      - description: tx0 register block
> > +      - description: tx1 register block
> > +      - description: PLL register block
> > +
> > +  clocks:
> > +    maxItems: 2
> 
> Can this be items: - const: again? I think having maxItems means you can
> have one or two items, which wouldn't match the number of clock-names
> then.

No, it's implicitly 'minItems: 2'. But you can have 'items' if a 
description of each clock is useful.

> 
> > +
> > +  clock-names:
> > +    items:
> > +      - const: aux
> > +      - const: cfg_ahb
> > +

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] phy: qcom: Introduce new eDP PHY driver
  2021-05-11  4:19 ` [PATCH 2/2] phy: qcom: Introduce new eDP PHY driver Bjorn Andersson
                     ` (3 preceding siblings ...)
  2021-05-14 10:40   ` Vinod Koul
@ 2021-08-10  3:15   ` sbillaka
  2021-08-10  3:36     ` Bjorn Andersson
  4 siblings, 1 reply; 12+ messages in thread
From: sbillaka @ 2021-08-10  3:15 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Andy Gross, Kishon Vijay Abraham I, Vinod Koul, Rob Herring,
	Rob Clark, Dmitry Baryshkov, Stephen Boyd, linux-arm-msm,
	linux-phy, devicetree, linux-kernel, kalyan_t, abhinavk

Hello Bjorn,

On 2021-05-11 09:49, Bjorn Andersson wrote:
> Many recent Qualcomm platforms comes with native DP and eDP support.
> This consists of a controller int he MDSS and a QMP-like PHY.
> 
> While similar to the well known QMP block, the eDP PHY only has TX 
> lanes
> and the programming sequences are slightly different. Rather than
> continuing the trend of parameterize the QMP driver to pieces, this
> introduces the support as a new driver.
> 
> The registration of link and pixel clocks are borrowed from the QMP
> driver. The non-DP link frequencies are omitted for now.
> 
> The eDP PHY is very similar to the dedicated (non-USB) DP PHY, but only
> the prior is supported for now.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>  drivers/phy/qualcomm/Kconfig        |  10 +
>  drivers/phy/qualcomm/Makefile       |   1 +
>  drivers/phy/qualcomm/phy-qcom-edp.c | 750 ++++++++++++++++++++++++++++
>  3 files changed, 761 insertions(+)
>  create mode 100644 drivers/phy/qualcomm/phy-qcom-edp.c
> 
> diff --git a/drivers/phy/qualcomm/Kconfig 
> b/drivers/phy/qualcomm/Kconfig
> index 7f6fcb8ec5ba..13ae6f4f3112 100644
> --- a/drivers/phy/qualcomm/Kconfig
> +++ b/drivers/phy/qualcomm/Kconfig
> @@ -114,3 +114,13 @@ config PHY_QCOM_IPQ806X_USB
>  	  This option enables support for the Synopsis PHYs present inside 
> the
>  	  Qualcomm USB3.0 DWC3 controller on ipq806x SoC. This driver 
> supports
>  	  both HS and SS PHY controllers.
> +
> +config PHY_QCOM_EDP
> +	tristate "Qualcomm eDP PHY driver"
> +	depends on ARCH_QCOM || COMPILE_TEST
> +	depends on OF
> +	depends on COMMON_CLK
> +	select GENERIC_PHY
> +	help
> +	  Enable this driver to support the Qualcomm eDP PHY found in various
> +	  Qualcomm chipsets.
> diff --git a/drivers/phy/qualcomm/Makefile 
> b/drivers/phy/qualcomm/Makefile
> index 47acbd7daa3a..1afd0882dbcc 100644
> --- a/drivers/phy/qualcomm/Makefile
> +++ b/drivers/phy/qualcomm/Makefile
> @@ -12,3 +12,4 @@ obj-$(CONFIG_PHY_QCOM_USB_HS_28NM)	+= 
> phy-qcom-usb-hs-28nm.o
>  obj-$(CONFIG_PHY_QCOM_USB_SS)		+= phy-qcom-usb-ss.o
>  obj-$(CONFIG_PHY_QCOM_USB_SNPS_FEMTO_V2)+= phy-qcom-snps-femto-v2.o
>  obj-$(CONFIG_PHY_QCOM_IPQ806X_USB)		+= phy-qcom-ipq806x-usb.o
> +obj-$(CONFIG_PHY_QCOM_EDP)		+= phy-qcom-edp.o
> diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c
> b/drivers/phy/qualcomm/phy-qcom-edp.c
> new file mode 100644
> index 000000000000..ff0508e49548
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-edp.c
> @@ -0,0 +1,750 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2017, 2020, The Linux Foundation. All rights 
> reserved.
> + * Copyright (c) 2021, Linaro Ltd.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_address.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/reset.h>
> +#include <linux/slab.h>
> +
> +#include <dt-bindings/phy/phy.h>
> +
> +#define DP_PHY_PD_CTL_PWRDN			0x001
> +#define DP_PHY_PD_CTL_PSR_PWRDN			0x002
> +#define DP_PHY_PD_CTL_AUX_PWRDN			0x004
> +#define DP_PHY_PD_CTL_LANE_0_1_PWRDN		0x008
> +#define DP_PHY_PD_CTL_LANE_2_3_PWRDN		0x010
> +#define DP_PHY_PD_CTL_PLL_PWRDN			0x020
> +#define DP_PHY_PD_CTL_DP_CLAMP_EN		0x040
> +
> +#define DP_PHY_CFG                              0x0010
> +#define DP_PHY_CFG_1                            0x0014
> +#define DP_PHY_PD_CTL                           0x001C
> +#define DP_PHY_MODE                             0x0020
> +
> +#define DP_PHY_AUX_CFG0				0x0020
> +#define DP_PHY_AUX_CFG1				0x0024
> +#define DP_PHY_AUX_CFG2				0x0028
> +#define DP_PHY_AUX_CFG3				0x002c
> +#define DP_PHY_AUX_CFG4				0x0030
> +#define DP_PHY_AUX_CFG5				0x0034
> +#define DP_PHY_AUX_CFG6				0x0038
> +#define DP_PHY_AUX_CFG7				0x003c
> +#define DP_PHY_AUX_CFG8				0x0040
> +#define DP_PHY_AUX_CFG9				0x0044

The DP_PHY_AUX_CFG0 offset for sc8180x eDP phy is 0x0024.
Some of the eDP PHY offset addresses are shifted by 4 address locations, 
compared to the DP QMP PHY offset addresses for sc8180x.
The DP_PHY_AUX_CFG* offsets for this eDP phy driver are as below:

#define DP_PHY_AUX_CFG0                         0x0024
#define DP_PHY_AUX_CFG1                         0x0028
#define DP_PHY_AUX_CFG2                         0x002c
#define DP_PHY_AUX_CFG3                         0x0030
#define DP_PHY_AUX_CFG4                         0x0034
#define DP_PHY_AUX_CFG5                         0x0038
#define DP_PHY_AUX_CFG6                         0x003c
#define DP_PHY_AUX_CFG7                         0x0040
#define DP_PHY_AUX_CFG8                         0x0044
#define DP_PHY_AUX_CFG9                         0x0048

> +
> +#define DP_PHY_VCO_DIV                          0x0074
> +#define DP_PHY_TX0_TX1_LANE_CTL                 0x007C
> +#define DP_PHY_TX2_TX3_LANE_CTL                 0x00A0
> +
> +#define DP_PHY_SPARE0                           0x00CC
> +#define DP_PHY_STATUS                           0x00E0
> +
> +/* Tx registers */
> +#define TXn_CLKBUF_ENABLE                       0x0000
> +#define TXn_TX_EMP_POST1_LVL                    0x0004
> +
> +#define TXn_TX_DRV_LVL                          0x0014
> +#define TXn_TX_DRV_LVL_OFFSET                   0x0018
> +#define TXn_RESET_TSYNC_EN                      0x001C
> +//#define TXn_PRE_STALL_LDO_BOOST_EN            0x0020
> +#define TXn_LDO_CONFIG                          0x0084
> +#define TXn_TX_BAND                             0x0028
> +#define TXn_INTERFACE_SELECT                    0x0024
> +
> +#define TXn_RES_CODE_LANE_OFFSET_TX0            0x0044
> +#define TXn_RES_CODE_LANE_OFFSET_TX1            0x0048
> +//#define TXn_RES_CODE_LANE_OFFSET_RX           0x0040
> +
> +#define TXn_TRANSCEIVER_BIAS_EN                 0x0054
> +#define TXn_HIGHZ_DRVR_EN                       0x0058
> +#define TXn_TX_POL_INV                          0x005C
> +#define TXn_PARRATE_REC_DETECT_IDLE_EN          0x0060
> +#define TXn_LANE_MODE_1                         0x0064
> +
> +#define TXn_TRAN_DRVR_EMP_EN                    0x0078
> +//#define TXn_TX_INTERFACE_MODE                 0x00BC
> +
> +#define TXn_VMODE_CTRL1                         0x007C
> +
> +/* PLL register offset */
> +#define QSERDES_COM_BG_TIMER                    0x000C
> +#define QSERDES_COM_BIAS_EN_CLKBUFLR_EN         0x0044
> +#define QSERDES_COM_CLK_ENABLE1                 0x0048
> +#define QSERDES_COM_SYS_CLK_CTRL                0x004C
> +#define QSERDES_COM_SYSCLK_BUF_ENABLE           0x0050
> +#define QSERDES_COM_PLL_IVCO                    0x0058
> +
> +#define QSERDES_COM_CP_CTRL_MODE0               0x0074
> +#define QSERDES_COM_PLL_RCTRL_MODE0             0x007C
> +#define QSERDES_COM_PLL_CCTRL_MODE0             0x0084
> +#define QSERDES_COM_SYSCLK_EN_SEL               0x0094
> +#define QSERDES_COM_RESETSM_CNTRL               0x009C
> +#define QSERDES_COM_LOCK_CMP_EN                 0x00A4
> +#define QSERDES_COM_LOCK_CMP1_MODE0             0x00AC
> +#define QSERDES_COM_LOCK_CMP2_MODE0             0x00B0
> +
> +#define QSERDES_COM_DEC_START_MODE0             0x00BC
> +#define QSERDES_COM_DIV_FRAC_START1_MODE0       0x00CC
> +#define QSERDES_COM_DIV_FRAC_START2_MODE0       0x00D0
> +#define QSERDES_COM_DIV_FRAC_START3_MODE0       0x00D4
> +#define QSERDES_COM_INTEGLOOP_GAIN0_MODE0       0x00EC
> +#define QSERDES_COM_INTEGLOOP_GAIN1_MODE0       0x00F0
> +#define QSERDES_COM_VCO_TUNE_CTRL               0x0108
> +#define QSERDES_COM_VCO_TUNE_MAP                0x010C
> +#define QSERDES_COM_VCO_TUNE1_MODE0             0x0110
> +#define QSERDES_COM_VCO_TUNE2_MODE0             0x0114
> +#define QSERDES_COM_CMN_STATUS                  0x0140
> +
> +#define QSERDES_COM_CLK_SEL                     0x0154
> +#define QSERDES_COM_HSCLK_SEL                   0x0158
> +
> +#define QSERDES_COM_CORECLK_DIV_MODE0           0x0168
> +
> +#define QSERDES_COM_CORE_CLK_EN                 0x0174
> +#define QSERDES_COM_C_READY_STATUS              0x0178
> +#define QSERDES_COM_CMN_CONFIG                  0x017C
> +
> +#define QSERDES_COM_SVS_MODE_CLK_SEL            0x0184
> +
> +#define QSERDES_COM_SSC_EN_CENTER               0x0010
> +#define QSERDES_COM_SSC_ADJ_PER1                0x0014
> +#define QSERDES_COM_SSC_ADJ_PER2                0x0018
> +#define QSERDES_COM_SSC_PER1                    0x001C
> +#define QSERDES_COM_SSC_PER2                    0x0020
> +#define QSERDES_COM_SSC_STEP_SIZE1_MODE0        0x0024
> +#define QSERDES_COM_SSC_STEP_SIZE2_MODE0        0x0028
> +
> +#define DP_PHY_PLL_POLL_SLEEP_US                500
> +#define DP_PHY_PLL_POLL_TIMEOUT_US              10000
> +
> +#define EDP_VCO_RATE_8100MHZDIV1000             8100000UL
> +#define EDP_VCO_RATE_8640MHZDIV1000             8640000UL
> +#define EDP_VCO_RATE_9720MHZDIV1000             9720000UL
> +#define EDP_VCO_RATE_10800MHZDIV1000            10800000UL
> +#define EDP_VCO_RATE_11880MHZDIV1000            11880000UL
> +
> +struct qcom_edp {
> +	struct device *dev;
> +
> +	struct phy *phy;
> +
> +	void __iomem *edp;
> +	void __iomem *tx0;
> +	void __iomem *tx1;
> +	void __iomem *pll;
> +
> +	struct clk_hw dp_link_hw;
> +	struct clk_hw dp_pixel_hw;
> +
> +	struct phy_configure_opts_dp dp_opts;
> +
> +	struct clk_bulk_data clks[2];
> +	struct regulator_bulk_data supplies[2];
> +} ;
> +
> +static int qcom_edp_phy_init(struct phy *phy)
> +{
> +	struct qcom_edp *edp = phy_get_drvdata(phy);
> +	int ret;
> +
> +	ret = regulator_bulk_enable(ARRAY_SIZE(edp->supplies), 
> edp->supplies);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_bulk_prepare_enable(ARRAY_SIZE(edp->clks), edp->clks);
> +	if (ret)
> +		goto out_disable_supplies;

I think the number of clk and regulator resources can vary based on 
platform.

> +
> +	writel(DP_PHY_PD_CTL_PWRDN | DP_PHY_PD_CTL_AUX_PWRDN |
> +	       DP_PHY_PD_CTL_PLL_PWRDN | DP_PHY_PD_CTL_DP_CLAMP_EN,
> +	       edp->edp + DP_PHY_PD_CTL);
> +
> +	writel(0x17, edp->pll + QSERDES_COM_BIAS_EN_CLKBUFLR_EN);
> +
> +	writel(DP_PHY_PD_CTL_PSR_PWRDN, edp->edp + DP_PHY_PD_CTL);
> +	msleep(20);
> +
> +	writel(DP_PHY_PD_CTL_PWRDN | DP_PHY_PD_CTL_AUX_PWRDN |
> +	       DP_PHY_PD_CTL_LANE_0_1_PWRDN | DP_PHY_PD_CTL_LANE_2_3_PWRDN |
> +	       DP_PHY_PD_CTL_PLL_PWRDN | DP_PHY_PD_CTL_DP_CLAMP_EN,
> +	       edp->edp + DP_PHY_PD_CTL);
> +
> +	writel(0x00, edp->edp + DP_PHY_AUX_CFG0);
> +	writel(0x13, edp->edp + DP_PHY_AUX_CFG1);
> +	writel(0x24, edp->edp + DP_PHY_AUX_CFG2);
> +	writel(0x00, edp->edp + DP_PHY_AUX_CFG3);
> +	writel(0x0a, edp->edp + DP_PHY_AUX_CFG4);
> +	writel(0x26, edp->edp + DP_PHY_AUX_CFG5);
> +	writel(0x0a, edp->edp + DP_PHY_AUX_CFG6);
> +	writel(0x03, edp->edp + DP_PHY_AUX_CFG7);
> +	writel(0x37, edp->edp + DP_PHY_AUX_CFG8);
> +	writel(0x03, edp->edp + DP_PHY_AUX_CFG9);
> +
> +	writel(0x1f, edp->edp + 0x58);
> +
> +	msleep(20);
> +
> +	return 0;
> +
> +out_disable_supplies:
> +	regulator_bulk_disable(ARRAY_SIZE(edp->supplies), edp->supplies);
> +
> +	return ret;
> +}
> +
> +static int qcom_edp_phy_configure(struct phy *phy, union
> phy_configure_opts *opts)
> +{
> +	const struct phy_configure_opts_dp *dp_opts = &opts->dp;
> +	struct qcom_edp *edp = phy_get_drvdata(phy);
> +
> +	memcpy(&edp->dp_opts, dp_opts, sizeof(*dp_opts));
> +
> +	return 0;
> +}
> +
> +static int qcom_edp_configure_ssc(struct qcom_edp *edp)
> +{
> +	const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
> +	u32 step1;
> +	u32 step2;
> +
> +	switch (dp_opts->link_rate) {
> +	case 1620:
> +	case 2700:
> +	case 8100:
> +		step1 = 0x45;
> +		step2 = 0x06;
> +		break;
> +	case 5400:
> +		step1 = 0x5c;
> +		step2 = 0x08;
> +		break;
> +	default:
> +		/* Other link rates aren't supported */
> +		return -EINVAL;
> +	}
> +
> +	writel(0x01, edp->pll + QSERDES_COM_SSC_EN_CENTER);
> +	writel(0x00, edp->pll + QSERDES_COM_SSC_ADJ_PER1);
> +	writel(0x36, edp->pll + QSERDES_COM_SSC_PER1);
> +	writel(0x01, edp->pll + QSERDES_COM_SSC_PER2);
> +	writel(step1, edp->pll + QSERDES_COM_SSC_STEP_SIZE1_MODE0);
> +	writel(step2, edp->pll + QSERDES_COM_SSC_STEP_SIZE2_MODE0);
> +
> +	return 0;
> +}
> +
> +static int qcom_edp_configure_pll(struct qcom_edp *edp)
> +{
> +	const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
> +	u32 div_frac_start2_mode0;
> +	u32 div_frac_start3_mode0;
> +	u32 dec_start_mode0;
> +	u32 lock_cmp1_mode0;
> +	u32 lock_cmp2_mode0;
> +	u32 hsclk_sel;
> +
> +	switch (dp_opts->link_rate) {
> +	case 1620:
> +		hsclk_sel = 0x5;
> +		dec_start_mode0 = 0x69;
> +		div_frac_start2_mode0 = 0x80;
> +		div_frac_start3_mode0 = 0x07;
> +		lock_cmp1_mode0 = 0x6f;
> +		lock_cmp2_mode0 = 0x08;
> +		break;
> +	case 2700:
> +		hsclk_sel = 0x3;
> +		dec_start_mode0 = 0x69;
> +		div_frac_start2_mode0 = 0x80;
> +		div_frac_start3_mode0 = 0x07;
> +		lock_cmp1_mode0 = 0x0f;
> +		lock_cmp2_mode0 = 0x0e;
> +		break;
> +	case 5400:
> +		hsclk_sel = 0x1;
> +		dec_start_mode0 = 0x8c;
> +		div_frac_start2_mode0 = 0x00;
> +		div_frac_start3_mode0 = 0x0a;
> +		lock_cmp1_mode0 = 0x1f;
> +		lock_cmp2_mode0 = 0x1c;
> +		break;
> +	case 8100:
> +		hsclk_sel = 0x0;
> +		dec_start_mode0 = 0x69;
> +		div_frac_start2_mode0 = 0x80;
> +		div_frac_start3_mode0 = 0x07;
> +		lock_cmp1_mode0 = 0x2f;
> +		lock_cmp2_mode0 = 0x2a;
> +		break;
> +	default:
> +		/* Other link rates aren't supported */
> +		return -EINVAL;
> +	}
> +
> +	writel(0x01, edp->pll + QSERDES_COM_SVS_MODE_CLK_SEL);
> +	writel(0x0b, edp->pll + QSERDES_COM_SYSCLK_EN_SEL);
> +	writel(0x02, edp->pll + QSERDES_COM_SYS_CLK_CTRL);
> +	writel(0x0c, edp->pll + QSERDES_COM_CLK_ENABLE1);
> +	writel(0x06, edp->pll + QSERDES_COM_SYSCLK_BUF_ENABLE);
> +	writel(0x30, edp->pll + QSERDES_COM_CLK_SEL);
> +	writel(hsclk_sel, edp->pll + QSERDES_COM_HSCLK_SEL);
> +	writel(0x0f, edp->pll + QSERDES_COM_PLL_IVCO);
> +	writel(0x08, edp->pll + QSERDES_COM_LOCK_CMP_EN);
> +	writel(0x36, edp->pll + QSERDES_COM_PLL_CCTRL_MODE0);
> +	writel(0x16, edp->pll + QSERDES_COM_PLL_RCTRL_MODE0);
> +	writel(0x06, edp->pll + QSERDES_COM_CP_CTRL_MODE0);
> +	writel(dec_start_mode0, edp->pll + QSERDES_COM_DEC_START_MODE0);
> +	writel(0x00, edp->pll + QSERDES_COM_DIV_FRAC_START1_MODE0);
> +	writel(div_frac_start2_mode0, edp->pll + 
> QSERDES_COM_DIV_FRAC_START2_MODE0);
> +	writel(div_frac_start3_mode0, edp->pll + 
> QSERDES_COM_DIV_FRAC_START3_MODE0);
> +	writel(0x02, edp->pll + QSERDES_COM_CMN_CONFIG);
> +	writel(0x3f, edp->pll + QSERDES_COM_INTEGLOOP_GAIN0_MODE0);
> +	writel(0x00, edp->pll + QSERDES_COM_INTEGLOOP_GAIN1_MODE0);
> +	writel(0x00, edp->pll + QSERDES_COM_VCO_TUNE_MAP);
> +	writel(lock_cmp1_mode0, edp->pll + QSERDES_COM_LOCK_CMP1_MODE0);
> +	writel(lock_cmp2_mode0, edp->pll + QSERDES_COM_LOCK_CMP2_MODE0);
> +
> +	writel(0x0a, edp->pll + QSERDES_COM_BG_TIMER);
> +	writel(0x14, edp->pll + QSERDES_COM_CORECLK_DIV_MODE0);
> +	writel(0x00, edp->pll + QSERDES_COM_VCO_TUNE_CTRL);
> +	writel(0x17, edp->pll + QSERDES_COM_BIAS_EN_CLKBUFLR_EN);
> +	writel(0x0f, edp->pll + QSERDES_COM_CORE_CLK_EN);
> +	writel(0xa0, edp->pll + QSERDES_COM_VCO_TUNE1_MODE0);
> +	writel(0x03, edp->pll + QSERDES_COM_VCO_TUNE2_MODE0);
> +
> +	return 0;
> +}
> +
> +static int qcom_edp_set_vco_div(struct qcom_edp *edp)
> +{
> +	const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
> +	unsigned long pixel_freq;
> +	u32 vco_div;
> +
> +	switch (dp_opts->link_rate) {
> +	case 1620:
> +		vco_div = 0x1;
> +		pixel_freq = 1620000000UL / 2;
> +		break;
> +	case 2700:
> +		vco_div = 0x1;
> +		pixel_freq = 2700000000UL / 2;
> +		break;
> +	case 5400:
> +		vco_div = 0x2;
> +		pixel_freq = 5400000000UL / 4;
> +		break;
> +	case 8100:
> +		vco_div = 0x0;
> +		pixel_freq = 8100000000UL / 6;
> +		break;
> +	default:
> +		/* Other link rates aren't supported */
> +		return -EINVAL;
> +	}
> +
> +	writel(vco_div, edp->edp + DP_PHY_VCO_DIV);
> +
> +	clk_set_rate(edp->dp_link_hw.clk, dp_opts->link_rate * 100000);
> +	clk_set_rate(edp->dp_pixel_hw.clk, pixel_freq);
> +
> +	return 0;
> +}
> +
> +static int qcom_edp_phy_power_on(struct phy *phy)
> +{
> +	struct qcom_edp *edp = phy_get_drvdata(phy);
> +	int timeout;
> +	int ret;
> +	u32 val;
> +
> +	writel(DP_PHY_PD_CTL_PWRDN | DP_PHY_PD_CTL_AUX_PWRDN |
> +	       DP_PHY_PD_CTL_LANE_0_1_PWRDN | DP_PHY_PD_CTL_LANE_2_3_PWRDN |
> +	       DP_PHY_PD_CTL_PLL_PWRDN | DP_PHY_PD_CTL_DP_CLAMP_EN,
> +	       edp->edp + DP_PHY_PD_CTL);
> +	writel(0xfc, edp->edp + DP_PHY_MODE);
> +
> +	timeout = readl_poll_timeout(edp->pll + QSERDES_COM_CMN_STATUS,
> +				     val, val & BIT(7), 5, 200);
> +	if (timeout)
> +		return timeout;
> +
> +	writel(0x01, edp->tx0 + TXn_LDO_CONFIG);
> +	writel(0x01, edp->tx1 + TXn_LDO_CONFIG);
> +	writel(0x00, edp->tx0 + TXn_LANE_MODE_1);
> +	writel(0x00, edp->tx1 + TXn_LANE_MODE_1);
> +
> +	ret = qcom_edp_configure_ssc(edp);
> +	if (ret)
> +		return ret;
> +
> +	ret = qcom_edp_configure_pll(edp);
> +	if (ret)
> +		return ret;
> +
> +	/* TX Lane configuration */
> +	writel(0x05, edp->edp + DP_PHY_TX0_TX1_LANE_CTL);
> +	writel(0x05, edp->edp + DP_PHY_TX2_TX3_LANE_CTL);
> +
> +	/* TX-0 register configuration */
> +	writel(0x03, edp->tx0 + TXn_TRANSCEIVER_BIAS_EN);
> +	writel(0x0f, edp->tx0 + TXn_CLKBUF_ENABLE);
> +	writel(0x03, edp->tx0 + TXn_RESET_TSYNC_EN);
> +	writel(0x01, edp->tx0 + TXn_TRAN_DRVR_EMP_EN);
> +	writel(0x04, edp->tx0 + TXn_TX_BAND);
> +
> +	/* TX-1 register configuration */
> +	writel(0x03, edp->tx1 + TXn_TRANSCEIVER_BIAS_EN);
> +	writel(0x0f, edp->tx1 + TXn_CLKBUF_ENABLE);
> +	writel(0x03, edp->tx1 + TXn_RESET_TSYNC_EN);
> +	writel(0x01, edp->tx1 + TXn_TRAN_DRVR_EMP_EN);
> +	writel(0x04, edp->tx1 + TXn_TX_BAND);
> +
> +	ret = qcom_edp_set_vco_div(edp);
> +	if (ret)
> +		return ret;
> +
> +	writel(0x01, edp->edp + DP_PHY_CFG);
> +        writel(0x05, edp->edp + DP_PHY_CFG);
> +        writel(0x01, edp->edp + DP_PHY_CFG);
> +        writel(0x09, edp->edp + DP_PHY_CFG);
> +
> +        writel(0x20, edp->pll + QSERDES_COM_RESETSM_CNTRL);
> +
> +	timeout = readl_poll_timeout(edp->pll + QSERDES_COM_C_READY_STATUS,
> +				     val, val & BIT(0), 500, 10000);
> +	if (timeout)
> +		return timeout;
> +
> +	writel(0x19, edp->edp + DP_PHY_CFG);
> +	writel(0x1f, edp->tx0 + TXn_HIGHZ_DRVR_EN);
> +	writel(0x04, edp->tx0 + TXn_HIGHZ_DRVR_EN);
> +	writel(0x00, edp->tx0 + TXn_TX_POL_INV);
> +	writel(0x1f, edp->tx1 + TXn_HIGHZ_DRVR_EN);
> +	writel(0x04, edp->tx1 + TXn_HIGHZ_DRVR_EN);
> +	writel(0x00, edp->tx1 + TXn_TX_POL_INV);
> +	writel(0x10, edp->tx0 + TXn_TX_DRV_LVL_OFFSET);
> +	writel(0x10, edp->tx1 + TXn_TX_DRV_LVL_OFFSET);
> +	writel(0x11, edp->tx0 + TXn_RES_CODE_LANE_OFFSET_TX0);
> +	writel(0x11, edp->tx0 + TXn_RES_CODE_LANE_OFFSET_TX1);
> +	writel(0x11, edp->tx1 + TXn_RES_CODE_LANE_OFFSET_TX0);
> +	writel(0x11, edp->tx1 + TXn_RES_CODE_LANE_OFFSET_TX1);
> +
> +	writel(0x10, edp->tx0 + TXn_TX_EMP_POST1_LVL);
> +	writel(0x10, edp->tx1 + TXn_TX_EMP_POST1_LVL);
> +	writel(0x1f, edp->tx0 + TXn_TX_DRV_LVL);
> +	writel(0x1f, edp->tx1 + TXn_TX_DRV_LVL);
> +
> +	writel(0x4, edp->tx0 + TXn_HIGHZ_DRVR_EN);
> +	writel(0x3, edp->tx0 + TXn_TRANSCEIVER_BIAS_EN);
> +	writel(0x4, edp->tx1 + TXn_HIGHZ_DRVR_EN);
> +	writel(0x0, edp->tx1 + TXn_TRANSCEIVER_BIAS_EN);
> +	writel(0x3, edp->edp + DP_PHY_CFG_1);
> +
> +	writel(0x18, edp->edp + DP_PHY_CFG);
> +	udelay(100);
> +
> +	writel(0x19, edp->edp + DP_PHY_CFG);
> +
> +	return readl_poll_timeout(edp->edp + DP_PHY_STATUS,
> +				  val, val & BIT(1), 500, 10000);
> +}
> +
> +static int qcom_edp_phy_power_off(struct phy *phy)
> +{
> +	struct qcom_edp *edp = phy_get_drvdata(phy);
> +
> +	writel(DP_PHY_PD_CTL_PSR_PWRDN, edp->edp + DP_PHY_PD_CTL);
> +
> +	return 0;
> +}
> +
> +static int qcom_edp_phy_exit(struct phy *phy)
> +{
> +	struct qcom_edp *edp = phy_get_drvdata(phy);
> +
> +	clk_bulk_disable_unprepare(ARRAY_SIZE(edp->clks), edp->clks);
> +	regulator_bulk_disable(ARRAY_SIZE(edp->supplies), edp->supplies);

I think the number of regulator and clk resources can vary based on 
platform.

> +
> +	return 0;
> +}
> +
> +static const struct phy_ops qcom_edp_ops = {
> +	.init		= qcom_edp_phy_init,
> +	.configure	= qcom_edp_phy_configure,
> +	.power_on	= qcom_edp_phy_power_on,
> +	.power_off	= qcom_edp_phy_power_off,
> +	.exit		= qcom_edp_phy_exit,
> +	.owner		= THIS_MODULE,
> +};
> +
> +/*
> + * Display Port PLL driver block diagram for branch clocks
> + *
> + *              +------------------------------+
> + *              |         DP_VCO_CLK           |
> + *              |                              |
> + *              |    +-------------------+     |
> + *              |    |   (DP PLL/VCO)    |     |
> + *              |    +---------+---------+     |
> + *              |              v               |
> + *              |   +----------+-----------+   |
> + *              |   | hsclk_divsel_clk_src |   |
> + *              |   +----------+-----------+   |
> + *              +------------------------------+
> + *                              |
> + *          +---------<---------v------------>----------+
> + *          |                                           |
> + * +--------v----------------+                          |
> + * |    dp_phy_pll_link_clk  |                          |
> + * |     link_clk            |                          |
> + * +--------+----------------+                          |
> + *          |                                           |
> + *          |                                           |
> + *          v                                           v
> + * Input to DISPCC block                                |
> + * for link clk, crypto clk                             |
> + * and interface clock                                  |
> + *                                                      |
> + *                                                      |
> + *      +--------<------------+-----------------+---<---+
> + *      |                     |                 |
> + * +----v---------+  +--------v-----+  +--------v------+
> + * | vco_divided  |  | vco_divided  |  | vco_divided   |
> + * |    _clk_src  |  |    _clk_src  |  |    _clk_src   |
> + * |              |  |              |  |               |
> + * |divsel_six    |  |  divsel_two  |  |  divsel_four  |
> + * +-------+------+  +-----+--------+  +--------+------+
> + *         |                 |                  |
> + *         v---->----------v-------------<------v
> + *                         |
> + *              +----------+-----------------+
> + *              |   dp_phy_pll_vco_div_clk   |
> + *              +---------+------------------+
> + *                        |
> + *                        v
> + *              Input to DISPCC block
> + *              for DP pixel clock
> + *
> + */
> +static int qcom_edp_dp_pixel_clk_determine_rate(struct clk_hw *hw,
> +						struct clk_rate_request *req)
> +{
> +	switch (req->rate) {
> +	case 1620000000UL / 2:
> +	case 2700000000UL / 2:
> +	/* 5.4 and 8.1 GHz are same link rate as 2.7GHz, i.e. div 4 and div 6 
> */
> +		return 0;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static unsigned long
> +qcom_edp_dp_pixel_clk_recalc_rate(struct clk_hw *hw, unsigned long 
> parent_rate)
> +{
> +	const struct qcom_edp *edp = container_of(hw, struct qcom_edp, 
> dp_pixel_hw);
> +	const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
> +
> +	switch (dp_opts->link_rate) {
> +	case 1620:
> +		return 1620000000UL / 2;
> +	case 2700:
> +		return 2700000000UL / 2;
> +	case 5400:
> +		return 5400000000UL / 4;
> +	case 8100:
> +		return 8100000000UL / 6;
> +	default:
> +		return 0;
> +	}
> +}
> +
> +static const struct clk_ops qcom_edp_dp_pixel_clk_ops = {
> +	.determine_rate = qcom_edp_dp_pixel_clk_determine_rate,
> +	.recalc_rate = qcom_edp_dp_pixel_clk_recalc_rate,
> +};
> +
> +static int qcom_edp_dp_link_clk_determine_rate(struct clk_hw *hw,
> +					       struct clk_rate_request *req)
> +{
> +	switch (req->rate) {
> +	case 162000000:
> +	case 270000000:
> +	case 540000000:
> +	case 810000000:
> +		return 0;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static unsigned long
> +qcom_edp_dp_link_clk_recalc_rate(struct clk_hw *hw, unsigned long 
> parent_rate)
> +{
> +	const struct qcom_edp *edp = container_of(hw, struct qcom_edp, 
> dp_link_hw);
> +	const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
> +
> +	switch (dp_opts->link_rate) {
> +	case 1620:
> +	case 2700:
> +	case 5400:
> +	case 8100:
> +		return dp_opts->link_rate * 100000;
> +	default:
> +		return 0;
> +	}
> +}
> +
> +static const struct clk_ops qcom_edp_dp_link_clk_ops = {
> +	.determine_rate = qcom_edp_dp_link_clk_determine_rate,
> +	.recalc_rate = qcom_edp_dp_link_clk_recalc_rate,
> +};
> +
> +static struct clk_hw *
> +qcom_edp_dp_clks_hw_get(struct of_phandle_args *clkspec, void *data)
> +{
> +	unsigned int idx = clkspec->args[0];
> +	struct qcom_edp *edp = data;
> +
> +	if (idx >= 2) {
> +		pr_err("%s: invalid index %u\n", __func__, idx);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	if (idx == 0)
> +		return &edp->dp_link_hw;
> +
> +	return &edp->dp_pixel_hw;
> +}
> +
> +static void phy_clk_release_provider(void *res)
> +{
> +	of_clk_del_provider(res);
> +}
> +
> +static int qcom_edp_clks_register(struct qcom_edp *edp, struct 
> device_node *np)
> +{
> +	struct clk_init_data init = { };
> +	int ret;
> +
> +	init.ops = &qcom_edp_dp_link_clk_ops;
> +	init.name = "edp_phy_pll_link_clk";
> +	edp->dp_link_hw.init = &init;
> +	ret = devm_clk_hw_register(edp->dev, &edp->dp_link_hw);
> +	if (ret)
> +		return ret;
> +
> +	init.ops = &qcom_edp_dp_pixel_clk_ops;
> +	init.name = "edp_phy_pll_vco_div_clk";
> +	edp->dp_pixel_hw.init = &init;
> +	ret = devm_clk_hw_register(edp->dev, &edp->dp_pixel_hw);
> +	if (ret)
> +		return ret;
> +
> +	ret = of_clk_add_hw_provider(np, qcom_edp_dp_clks_hw_get, edp);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * Roll a devm action because the clock provider is the child node, 
> but
> +	 * the child node is not actually a device.
> +	 */
> +	ret = devm_add_action(edp->dev, phy_clk_release_provider, np);
> +	if (ret)
> +		phy_clk_release_provider(np);
> +
> +	return ret;
> +}
> +
> +static int qcom_edp_phy_probe(struct platform_device *pdev)
> +{
> +	struct phy_provider *phy_provider;
> +	struct device *dev = &pdev->dev;
> +	struct qcom_edp *edp;
> +	int ret;
> +
> +	edp = devm_kzalloc(dev, sizeof(*edp), GFP_KERNEL);
> +	if (!edp)
> +		return -ENOMEM;
> +
> +	edp->dev = dev;
> +
> +	edp->edp = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(edp->edp))
> +		return PTR_ERR(edp->edp);
> +
> +	edp->tx0 = devm_platform_ioremap_resource(pdev, 1);
> +	if (IS_ERR(edp->tx0))
> +		return PTR_ERR(edp->tx0);
> +
> +	edp->tx1 = devm_platform_ioremap_resource(pdev, 2);
> +	if (IS_ERR(edp->tx1))
> +		return PTR_ERR(edp->tx1);
> +
> +	edp->pll = devm_platform_ioremap_resource(pdev, 3);
> +	if (IS_ERR(edp->pll))
> +		return PTR_ERR(edp->pll);
> +
> +	edp->clks[0].id = "aux";
> +	edp->clks[1].id = "cfg_ahb";
> +	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(edp->clks), edp->clks);
> +	if (ret)
> +		return ret;
> +
> +	edp->supplies[0].supply = "vdda-phy";
> +	edp->supplies[1].supply = "vdda-pll";
> +	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(edp->supplies), 
> edp->supplies);
> +	if (ret)
> +		return ret;

I believe, the combination of the number of regulator and clk resources 
may vary based on the platform.
I think we should not fail probe if all these resources are not present 
in the device tree file.
I think, these resources can be optional. We can get these resources if 
they are present in the device tree file and enable them as required.

> +
> +	ret = qcom_edp_clks_register(edp, pdev->dev.of_node);
> +	if (ret)
> +		return ret;
> +
> +	edp->phy = devm_phy_create(dev, pdev->dev.of_node, &qcom_edp_ops);
> +	if (IS_ERR(edp->phy)) {
> +		dev_err(dev, "failed to register phy\n");
> +		return PTR_ERR(edp->phy);
> +	}
> +
> +	phy_set_drvdata(edp->phy, edp);
> +
> +	phy_provider = devm_of_phy_provider_register(dev, 
> of_phy_simple_xlate);
> +	return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +static const struct of_device_id qcom_edp_phy_match_table[] = {
> +	{ .compatible = "qcom,sc8180x-edp-phy" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, qcom_edp_phy_match_table);
> +
> +static struct platform_driver qcom_edp_phy_driver = {
> +	.probe		= qcom_edp_phy_probe,
> +	.driver = {
> +		.name	= "qcom-edp-phy",
> +		.of_match_table = qcom_edp_phy_match_table,
> +	},
> +};
> +
> +module_platform_driver(qcom_edp_phy_driver);
> +
> +MODULE_DESCRIPTION("Qualcomm eDP PHY driver");
> +MODULE_LICENSE("GPL v2");

Thank you,
Sankeerth

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] phy: qcom: Introduce new eDP PHY driver
  2021-08-10  3:15   ` sbillaka
@ 2021-08-10  3:36     ` Bjorn Andersson
  2021-08-12  0:23       ` sbillaka
  0 siblings, 1 reply; 12+ messages in thread
From: Bjorn Andersson @ 2021-08-10  3:36 UTC (permalink / raw)
  To: sbillaka
  Cc: Andy Gross, Kishon Vijay Abraham I, Vinod Koul, Rob Herring,
	Rob Clark, Dmitry Baryshkov, Stephen Boyd, linux-arm-msm,
	linux-phy, devicetree, linux-kernel, kalyan_t, abhinavk

On Mon 09 Aug 22:15 CDT 2021, sbillaka@codeaurora.org wrote:
> On 2021-05-11 09:49, Bjorn Andersson wrote:
> > diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c
[..]
> > +#define DP_PHY_AUX_CFG0				0x0020
> > +#define DP_PHY_AUX_CFG1				0x0024
> > +#define DP_PHY_AUX_CFG2				0x0028
> > +#define DP_PHY_AUX_CFG3				0x002c
> > +#define DP_PHY_AUX_CFG4				0x0030
> > +#define DP_PHY_AUX_CFG5				0x0034
> > +#define DP_PHY_AUX_CFG6				0x0038
> > +#define DP_PHY_AUX_CFG7				0x003c
> > +#define DP_PHY_AUX_CFG8				0x0040
> > +#define DP_PHY_AUX_CFG9				0x0044
> 
> The DP_PHY_AUX_CFG0 offset for sc8180x eDP phy is 0x0024.
> Some of the eDP PHY offset addresses are shifted by 4 address locations,
> compared to the DP QMP PHY offset addresses for sc8180x.
> The DP_PHY_AUX_CFG* offsets for this eDP phy driver are as below:
> 
> #define DP_PHY_AUX_CFG0                         0x0024
> #define DP_PHY_AUX_CFG1                         0x0028
> #define DP_PHY_AUX_CFG2                         0x002c
> #define DP_PHY_AUX_CFG3                         0x0030
> #define DP_PHY_AUX_CFG4                         0x0034
> #define DP_PHY_AUX_CFG5                         0x0038
> #define DP_PHY_AUX_CFG6                         0x003c
> #define DP_PHY_AUX_CFG7                         0x0040
> #define DP_PHY_AUX_CFG8                         0x0044
> #define DP_PHY_AUX_CFG9                         0x0048
> 

I noticed this as well. During development I just used the numbers
directly in the code and I must have screwed up as I replaced them with
defined - and somehow missed this in the testing before posting.

Sorry about that.

[..]
> > +static int qcom_edp_phy_init(struct phy *phy)
> > +{
> > +	struct qcom_edp *edp = phy_get_drvdata(phy);
> > +	int ret;
> > +
> > +	ret = regulator_bulk_enable(ARRAY_SIZE(edp->supplies), edp->supplies);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = clk_bulk_prepare_enable(ARRAY_SIZE(edp->clks), edp->clks);
> > +	if (ret)
> > +		goto out_disable_supplies;
> 
> I think the number of clk and regulator resources can vary based on
> platform.
> 

If that's the case we should replace the ARRAY_SIZE() with an integer.
But I prefer to wait with that until the number actually is variable.

[..]
> > +static int qcom_edp_phy_probe(struct platform_device *pdev)
> > +{
> > +	struct phy_provider *phy_provider;
> > +	struct device *dev = &pdev->dev;
> > +	struct qcom_edp *edp;
> > +	int ret;
> > +
> > +	edp = devm_kzalloc(dev, sizeof(*edp), GFP_KERNEL);
> > +	if (!edp)
> > +		return -ENOMEM;
> > +
> > +	edp->dev = dev;
> > +
> > +	edp->edp = devm_platform_ioremap_resource(pdev, 0);
> > +	if (IS_ERR(edp->edp))
> > +		return PTR_ERR(edp->edp);
> > +
> > +	edp->tx0 = devm_platform_ioremap_resource(pdev, 1);
> > +	if (IS_ERR(edp->tx0))
> > +		return PTR_ERR(edp->tx0);
> > +
> > +	edp->tx1 = devm_platform_ioremap_resource(pdev, 2);
> > +	if (IS_ERR(edp->tx1))
> > +		return PTR_ERR(edp->tx1);
> > +
> > +	edp->pll = devm_platform_ioremap_resource(pdev, 3);
> > +	if (IS_ERR(edp->pll))
> > +		return PTR_ERR(edp->pll);
> > +
> > +	edp->clks[0].id = "aux";
> > +	edp->clks[1].id = "cfg_ahb";
> > +	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(edp->clks), edp->clks);
> > +	if (ret)
> > +		return ret;
> > +
> > +	edp->supplies[0].supply = "vdda-phy";
> > +	edp->supplies[1].supply = "vdda-pll";
> > +	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(edp->supplies),
> > edp->supplies);
> > +	if (ret)
> > +		return ret;
> 
> I believe, the combination of the number of regulator and clk resources may
> vary based on the platform.
> I think we should not fail probe if all these resources are not present in
> the device tree file.
> I think, these resources can be optional. We can get these resources if they
> are present in the device tree file and enable them as required.
> 

It's quite helpful to the DTS writer to actually encode in the driver
which resources the driver expects and provide useful error messages
when these expectations aren't met - so I think in line with most other
drivers this should be decided based on the compatible.

What clocks and regulators do you have on sc7280?


Thanks for the feedback, I see that I have a few more pieces of feedback
from others that I need to incorporate. I'll make sure to do that and
repost this patch shortly.

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] phy: qcom: Introduce new eDP PHY driver
  2021-08-10  3:36     ` Bjorn Andersson
@ 2021-08-12  0:23       ` sbillaka
  0 siblings, 0 replies; 12+ messages in thread
From: sbillaka @ 2021-08-12  0:23 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Andy Gross, Kishon Vijay Abraham I, Vinod Koul, Rob Herring,
	Rob Clark, Dmitry Baryshkov, Stephen Boyd, linux-arm-msm,
	linux-phy, devicetree, linux-kernel, kalyan_t, abhinavk

On 2021-08-10 09:06, Bjorn Andersson wrote:
> On Mon 09 Aug 22:15 CDT 2021, sbillaka@codeaurora.org wrote:
>> On 2021-05-11 09:49, Bjorn Andersson wrote:
>> > diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c
> [..]
>> > +#define DP_PHY_AUX_CFG0				0x0020
>> > +#define DP_PHY_AUX_CFG1				0x0024
>> > +#define DP_PHY_AUX_CFG2				0x0028
>> > +#define DP_PHY_AUX_CFG3				0x002c
>> > +#define DP_PHY_AUX_CFG4				0x0030
>> > +#define DP_PHY_AUX_CFG5				0x0034
>> > +#define DP_PHY_AUX_CFG6				0x0038
>> > +#define DP_PHY_AUX_CFG7				0x003c
>> > +#define DP_PHY_AUX_CFG8				0x0040
>> > +#define DP_PHY_AUX_CFG9				0x0044
>> 
>> The DP_PHY_AUX_CFG0 offset for sc8180x eDP phy is 0x0024.
>> Some of the eDP PHY offset addresses are shifted by 4 address 
>> locations,
>> compared to the DP QMP PHY offset addresses for sc8180x.
>> The DP_PHY_AUX_CFG* offsets for this eDP phy driver are as below:
>> 
>> #define DP_PHY_AUX_CFG0                         0x0024
>> #define DP_PHY_AUX_CFG1                         0x0028
>> #define DP_PHY_AUX_CFG2                         0x002c
>> #define DP_PHY_AUX_CFG3                         0x0030
>> #define DP_PHY_AUX_CFG4                         0x0034
>> #define DP_PHY_AUX_CFG5                         0x0038
>> #define DP_PHY_AUX_CFG6                         0x003c
>> #define DP_PHY_AUX_CFG7                         0x0040
>> #define DP_PHY_AUX_CFG8                         0x0044
>> #define DP_PHY_AUX_CFG9                         0x0048
>> 
> 
> I noticed this as well. During development I just used the numbers
> directly in the code and I must have screwed up as I replaced them with
> defined - and somehow missed this in the testing before posting.
> 
> Sorry about that.
> 
> [..]
>> > +static int qcom_edp_phy_init(struct phy *phy)
>> > +{
>> > +	struct qcom_edp *edp = phy_get_drvdata(phy);
>> > +	int ret;
>> > +
>> > +	ret = regulator_bulk_enable(ARRAY_SIZE(edp->supplies), edp->supplies);
>> > +	if (ret)
>> > +		return ret;
>> > +
>> > +	ret = clk_bulk_prepare_enable(ARRAY_SIZE(edp->clks), edp->clks);
>> > +	if (ret)
>> > +		goto out_disable_supplies;
>> 
>> I think the number of clk and regulator resources can vary based on
>> platform.
>> 
> 
> If that's the case we should replace the ARRAY_SIZE() with an integer.
> But I prefer to wait with that until the number actually is variable.
> 
> [..]
>> > +static int qcom_edp_phy_probe(struct platform_device *pdev)
>> > +{
>> > +	struct phy_provider *phy_provider;
>> > +	struct device *dev = &pdev->dev;
>> > +	struct qcom_edp *edp;
>> > +	int ret;
>> > +
>> > +	edp = devm_kzalloc(dev, sizeof(*edp), GFP_KERNEL);
>> > +	if (!edp)
>> > +		return -ENOMEM;
>> > +
>> > +	edp->dev = dev;
>> > +
>> > +	edp->edp = devm_platform_ioremap_resource(pdev, 0);
>> > +	if (IS_ERR(edp->edp))
>> > +		return PTR_ERR(edp->edp);
>> > +
>> > +	edp->tx0 = devm_platform_ioremap_resource(pdev, 1);
>> > +	if (IS_ERR(edp->tx0))
>> > +		return PTR_ERR(edp->tx0);
>> > +
>> > +	edp->tx1 = devm_platform_ioremap_resource(pdev, 2);
>> > +	if (IS_ERR(edp->tx1))
>> > +		return PTR_ERR(edp->tx1);
>> > +
>> > +	edp->pll = devm_platform_ioremap_resource(pdev, 3);
>> > +	if (IS_ERR(edp->pll))
>> > +		return PTR_ERR(edp->pll);
>> > +
>> > +	edp->clks[0].id = "aux";
>> > +	edp->clks[1].id = "cfg_ahb";
>> > +	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(edp->clks), edp->clks);
>> > +	if (ret)
>> > +		return ret;
>> > +
>> > +	edp->supplies[0].supply = "vdda-phy";
>> > +	edp->supplies[1].supply = "vdda-pll";
>> > +	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(edp->supplies),
>> > edp->supplies);
>> > +	if (ret)
>> > +		return ret;
>> 
>> I believe, the combination of the number of regulator and clk 
>> resources may
>> vary based on the platform.
>> I think we should not fail probe if all these resources are not 
>> present in
>> the device tree file.
>> I think, these resources can be optional. We can get these resources 
>> if they
>> are present in the device tree file and enable them as required.
>> 
> 
> It's quite helpful to the DTS writer to actually encode in the driver
> which resources the driver expects and provide useful error messages
> when these expectations aren't met - so I think in line with most other
> drivers this should be decided based on the compatible.
> 
> What clocks and regulators do you have on sc7280?

We have only one clock (edp refclk) and one regulator (phy-0p9) for 
sc7280.

> 
> 
> Thanks for the feedback, I see that I have a few more pieces of 
> feedback
> from others that I need to incorporate. I'll make sure to do that and
> repost this patch shortly.

Okay. Thank you.

> 
> Regards,
> Bjorn

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-08-12  0:24 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-11  4:19 [PATCH 1/2] dt-bindings: phy: Introduce Qualcomm eDP/DP PHY binding Bjorn Andersson
2021-05-11  4:19 ` [PATCH 2/2] phy: qcom: Introduce new eDP PHY driver Bjorn Andersson
2021-05-11  5:22   ` Stephen Boyd
2021-05-14 10:38     ` Vinod Koul
2021-05-11 14:24   ` Jeffrey Hugo
2021-05-11 14:46   ` Dmitry Baryshkov
2021-05-14 10:40   ` Vinod Koul
2021-08-10  3:15   ` sbillaka
2021-08-10  3:36     ` Bjorn Andersson
2021-08-12  0:23       ` sbillaka
2021-05-11 17:24 ` [PATCH 1/2] dt-bindings: phy: Introduce Qualcomm eDP/DP PHY binding Stephen Boyd
2021-05-17 21:46   ` Rob Herring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).