[PATCH v4 0/4] phy: USB and PCIe phy drivers for Qcom chipsets

[PATCH v4 0/4] phy: USB and PCIe phy drivers for Qcom chipsets

[Vivek Gautam]

This patch series adds couple of PHY drivers for Qualcomm chipsets.
a) qcom-qusb2 phy driver: that provides High Speed USB functionality.
b) qcom-qmp phy driver: that is a combo phy providing support for
   USB3, PCIe, UFS and few other controllers.

The patches are based on next branch of linux-phy tree.

These have been tested on Dragon board db820c hardware with
required set of dt patches on integration tree maintained by
Linaro landing team for Qualcomm [1].

Couple of other patches [2,3,4] for nvmem are also pulled in for testing.
Patch series fixing DMA config for XHCI [5] is also pulled in for testing.

Changes since v3:
 - Addressed review comments given by Rob and Stephen for qusb2 phy
   and qmp phy bindings respectively.
 - Addressed review comments given by Stephen and Bjorn for qmp phy driver.

Changes since v2:
 - Addressed review comments given by Rob and Stephen for bindings.
 - Addressed the review comments given by Stephen for the qusb2 and qmp
   phy drivers.

Changes since v1:
 - Moved device tree binding documentation to separate patches, as suggested
   by Rob.
 - Addressed review comment regarding qfprom accesses by qusb2 phy driver,
   given by Rob.
 - Addressed review comments from Kishon.
 - Addressed review comments from Srinivas for QMP phy driver.
 - Addressed kbuild warning.

Please see individual patches for detailed changelogs.

[1] https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/log/?h=integration-linux-qcomlt
[2] https://lkml.org/lkml/2016/11/17/21
[3] https://lkml.org/lkml/2016/12/19/18
[4] http://www.spinics.net/lists/linux-arm-msm/msg25483.html
[5] https://lkml.org/lkml/2016/11/17/339

Vivek Gautam (4):
  dt-bindings: phy: Add support for QUSB2 phy
  phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips
  dt-bindings: phy: Add support for QMP phy
  phy: qcom-qmp: new qmp phy driver for qcom-chipsets

 .../devicetree/bindings/phy/qcom-qmp-phy.txt       |   76 ++
 .../devicetree/bindings/phy/qcom-qusb2-phy.txt     |   45 +
 drivers/phy/Kconfig                                |   18 +
 drivers/phy/Makefile                               |    2 +
 drivers/phy/phy-qcom-qmp.c                         | 1206 ++++++++++++++++++++
 drivers/phy/phy-qcom-qusb2.c                       |  539 +++++++++
 6 files changed, 1886 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
 create mode 100644 Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
 create mode 100644 drivers/phy/phy-qcom-qmp.c
 create mode 100644 drivers/phy/phy-qcom-qusb2.c

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 0/4] phy: USB and PCIe phy drivers for Qcom chipsets

[Vivek Gautam]

This patch series adds couple of PHY drivers for Qualcomm chipsets.
a) qcom-qusb2 phy driver: that provides High Speed USB functionality.
b) qcom-qmp phy driver: that is a combo phy providing support for
   USB3, PCIe, UFS and few other controllers.

The patches are based on next branch of linux-phy tree.

These have been tested on Dragon board db820c hardware with
required set of dt patches on integration tree maintained by
Linaro landing team for Qualcomm [1].

Couple of other patches [2,3,4] for nvmem are also pulled in for testing.
Patch series fixing DMA config for XHCI [5] is also pulled in for testing.

Changes since v3:
 - Addressed review comments given by Rob and Stephen for qusb2 phy
   and qmp phy bindings respectively.
 - Addressed review comments given by Stephen and Bjorn for qmp phy driver.

Changes since v2:
 - Addressed review comments given by Rob and Stephen for bindings.
 - Addressed the review comments given by Stephen for the qusb2 and qmp
   phy drivers.

Changes since v1:
 - Moved device tree binding documentation to separate patches, as suggested
   by Rob.
 - Addressed review comment regarding qfprom accesses by qusb2 phy driver,
   given by Rob.
 - Addressed review comments from Kishon.
 - Addressed review comments from Srinivas for QMP phy driver.
 - Addressed kbuild warning.

Please see individual patches for detailed changelogs.

[1] https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/log/?h=integration-linux-qcomlt
[2] https://lkml.org/lkml/2016/11/17/21
[3] https://lkml.org/lkml/2016/12/19/18
[4] http://www.spinics.net/lists/linux-arm-msm/msg25483.html
[5] https://lkml.org/lkml/2016/11/17/339

Vivek Gautam (4):
  dt-bindings: phy: Add support for QUSB2 phy
  phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips
  dt-bindings: phy: Add support for QMP phy
  phy: qcom-qmp: new qmp phy driver for qcom-chipsets

 .../devicetree/bindings/phy/qcom-qmp-phy.txt       |   76 ++
 .../devicetree/bindings/phy/qcom-qusb2-phy.txt     |   45 +
 drivers/phy/Kconfig                                |   18 +
 drivers/phy/Makefile                               |    2 +
 drivers/phy/phy-qcom-qmp.c                         | 1206 ++++++++++++++++++++
 drivers/phy/phy-qcom-qusb2.c                       |  539 +++++++++
 6 files changed, 1886 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
 create mode 100644 Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
 create mode 100644 drivers/phy/phy-qcom-qmp.c
 create mode 100644 drivers/phy/phy-qcom-qusb2.c

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v4 1/4] dt-bindings: phy: Add support for QUSB2 phy

[Vivek Gautam]

Qualcomm chipsets have QUSB2 phy controller that provides
HighSpeed functionality for DWC3 controller.
Adding dt binding information for the same.

Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
Acked-by: Rob Herring <robh@kernel.org>
---

Changes since v3:
 - Added 'Acked-by' from Rob.
 - Removed 'reset-names' and 'nvmem-cell-names' from the bindings
   since we use only one cell.

Changes since v2:
 - Removed binding for "ref_clk_src" since we don't request this
   clock in the driver.
 - Addressed s/vdda-phy-dpdm/vdda-phy-dpdm-supply.
 - Addressed s/ref_clk/ref. Don't need to add '_clk' suffix to clock names.
 - Addressed s/tune2_hstx_trim_efuse/tune2_hstx_trim. Don't need to add
   'efuse' suffix to nvmem cell.
 - Addressed s/qusb2phy/phy for the node name.

Changes since v1:
 - New patch, forked out of the original driver patch:
   "phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips"
 - Updated dt bindings to remove 'hstx-trim-bit-offset' and
   'hstx-trim-bit-len' bindings.

 .../devicetree/bindings/phy/qcom-qusb2-phy.txt     | 45 ++++++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt

diff --git a/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
new file mode 100644
index 000000000000..218595fe8824
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
@@ -0,0 +1,45 @@
+Qualcomm QUSB2 phy controller
+=============================
+
+QUSB2 controller supports LS/FS/HS usb connectivity on Qualcomm chipsets.
+
+Required properties:
+ - compatible: compatible list, contains "qcom,msm8996-qusb2-phy".
+ - reg: offset and length of the PHY register set.
+ - #phy-cells: must be 0.
+
+ - clocks: a list of phandles and clock-specifier pairs,
+	   one for each entry in clock-names.
+ - clock-names: must be "cfg_ahb" for phy config clock,
+			"ref" for 19.2 MHz ref clk,
+			"iface" for phy interface clock (Optional).
+
+ - vdd-phy-supply: Phandle to a regulator supply to PHY core block.
+ - vdda-pll-supply: Phandle to 1.8V regulator supply to PHY refclk pll block.
+ - vdda-phy-dpdm-supply: Phandle to 3.1V regulator supply to Dp/Dm port signals.
+
+ - resets: Phandle to reset to phy block.
+
+Optional properties:
+ - nvmem-cells: Phandle to nvmem cell that contains 'HS Tx trim'
+		tuning parameter value for qusb2 phy.
+
+ - qcom,tcsr-syscon: Phandle to TCSR syscon register region.
+
+Example:
+	hsusb_phy: phy@7411000 {
+		compatible = "qcom,msm8996-qusb2-phy";
+		reg = <0x07411000 0x180>;
+		#phy-cells = <0>;
+
+		clocks = <&gcc GCC_USB_PHY_CFG_AHB2PHY_CLK>,
+			<&gcc GCC_RX1_USB2_CLKREF_CLK>,
+		clock-names = "cfg_ahb", "ref";
+
+		vdd-phy-supply = <&pm8994_s2>;
+		vdda-pll-supply = <&pm8994_l12>;
+		vdda-phy-dpdm-supply = <&pm8994_l24>;
+
+		resets = <&gcc GCC_QUSB2PHY_PRIM_BCR>;
+		nvmem-cells = <&qusb2p_hstx_trim>;
+        };
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v4 2/4] phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips

[Vivek Gautam]

PHY transceiver driver for QUSB2 phy controller that provides
HighSpeed functionality for DWC3 controller present on
Qualcomm chipsets.

Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---

Changes since v3:
 - Added 'Reviewed-by' from Stephen.
 - Fixed debug message for qusb2_phy_set_tune2_param().
 - Replaced devm_reset_control_get() with devm_reset_control_get_by_index()
   since we are requesting only one reset.
 - Updated devm_nvmem_cell_get() with a NULL cell id.
 - Made error labels more idiomatic.
 - Refactored qusb2_setbits() and qusb2_clrbits() a little bit to accept
   base address and register offset as two separate arguments.

Changes since v2:
 - Removed selecting 'RESET_CONTROLLER' config.
 - Added error handling for clk_prepare_enable paths.
 - Removed explicitly setting ref_clk rate to 19.2 MHz. Don't need to
   do that since 'xo' is modeled as parent to this clock.
 - Removed 'ref_clk_src' handling. Driver doesn't need to request and
   handle this clock.
 - Moved nvmem_cell_get() to probe function.
 - Simplified phy pll status handling.
 - Using of_device_get_match_data() to get match data.
 - Uniformly using lowercase for hex numbers.
 - Fixed sparse warnings.
 - Using shorter variable names in structure and in functions.
 - Handling various comment style shortcomings.

Changes since v1:
 - removed reference to clk_enabled/pwr_enabled.
 - moved clock and regulator enable code to phy_power_on/off() callbacks.
 - fixed return on EPROBE_DEFER in qusb2_phy_probe().
 - fixed phy create and phy register ordering.
 - removed references to non-lkml links from commit message.
 - took care of other minor nits.
 - Fixed coccinelle warnings -
   'PTR_ERR applied after initialization to constant'
 - Addressed review comment, regarding qfprom access for tune2 param value.
   This driver is now based on qfprom patch[1] that allows byte access now.

 drivers/phy/Kconfig          |  10 +
 drivers/phy/Makefile         |   1 +
 drivers/phy/phy-qcom-qusb2.c | 539 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 550 insertions(+)
 create mode 100644 drivers/phy/phy-qcom-qusb2.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index e8eb7f225a88..0ed53d018b23 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -430,6 +430,16 @@ config PHY_STIH407_USB
 	  Enable this support to enable the picoPHY device used by USB2
 	  and USB3 controllers on STMicroelectronics STiH407 SoC families.
 
+config PHY_QCOM_QUSB2
+	tristate "Qualcomm QUSB2 PHY Driver"
+	depends on OF && (ARCH_QCOM || COMPILE_TEST)
+	select GENERIC_PHY
+	help
+	  Enable this to support the HighSpeed QUSB2 PHY transceiver for USB
+	  controllers on Qualcomm chips. This driver supports the high-speed
+	  PHY which is usually paired with either the ChipIdea or Synopsys DWC3
+	  USB IPs on MSM SOCs.
+
 config PHY_QCOM_UFS
 	tristate "Qualcomm UFS PHY driver"
 	depends on OF && ARCH_QCOM
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 65eb2f436a41..dad1682b80e3 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_PHY_ST_SPEAR1310_MIPHY)	+= phy-spear1310-miphy.o
 obj-$(CONFIG_PHY_ST_SPEAR1340_MIPHY)	+= phy-spear1340-miphy.o
 obj-$(CONFIG_PHY_XGENE)			+= phy-xgene.o
 obj-$(CONFIG_PHY_STIH407_USB)		+= phy-stih407-usb.o
+obj-$(CONFIG_PHY_QCOM_QUSB2) 	+= phy-qcom-qusb2.o
 obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs.o
 obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs-qmp-20nm.o
 obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs-qmp-14nm.o
diff --git a/drivers/phy/phy-qcom-qusb2.c b/drivers/phy/phy-qcom-qusb2.c
new file mode 100644
index 000000000000..c69118610164
--- /dev/null
+++ b/drivers/phy/phy-qcom-qusb2.c
@@ -0,0 +1,539 @@
+/*
+ * Copyright (c) 2016, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
+#include <linux/slab.h>
+
+#define QUSB2PHY_PLL_TEST		0x04
+#define CLK_REF_SEL			BIT(7)
+
+#define QUSB2PHY_PLL_TUNE		0x08
+#define QUSB2PHY_PLL_USER_CTL1		0x0c
+#define QUSB2PHY_PLL_USER_CTL2		0x10
+#define QUSB2PHY_PLL_AUTOPGM_CTL1	0x1c
+#define QUSB2PHY_PLL_PWR_CTRL		0x18
+
+#define QUSB2PHY_PLL_STATUS		0x38
+#define PLL_LOCKED			BIT(5)
+
+#define QUSB2PHY_PORT_TUNE1		0x80
+#define QUSB2PHY_PORT_TUNE2		0x84
+#define QUSB2PHY_PORT_TUNE3		0x88
+#define QUSB2PHY_PORT_TUNE4		0x8c
+#define QUSB2PHY_PORT_TUNE5		0x90
+#define QUSB2PHY_PORT_TEST2		0x9c
+
+#define QUSB2PHY_PORT_POWERDOWN		0xb4
+#define CLAMP_N_EN			BIT(5)
+#define FREEZIO_N			BIT(1)
+#define POWER_DOWN			BIT(0)
+
+#define QUSB2PHY_REFCLK_ENABLE		BIT(0)
+
+#define PHY_CLK_SCHEME_SEL		BIT(0)
+
+struct qusb2_phy_init_tbl {
+	unsigned int offset;
+	unsigned int val;
+};
+#define QUSB2_PHY_INIT_CFG(o, v) \
+	{			\
+		.offset = o,	\
+		.val = v,	\
+	}
+
+static const struct qusb2_phy_init_tbl msm8996_init_tbl[] = {
+	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE1, 0xf8),
+	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE2, 0xb3),
+	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE3, 0x83),
+	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE4, 0xc0),
+	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_TUNE, 0x30),
+	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL1, 0x79),
+	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL2, 0x21),
+	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TEST2, 0x14),
+	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_AUTOPGM_CTL1, 0x9f),
+	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_PWR_CTRL, 0x00),
+};
+
+struct qusb2_phy_cfg {
+	const struct qusb2_phy_init_tbl *tbl;
+	/* number of entries in the table */
+	unsigned int tbl_num;
+	/* offset to PHY_CLK_SCHEME register in TCSR map */
+	unsigned int clk_scheme_offset;
+};
+
+static const struct qusb2_phy_cfg msm8996_phy_cfg = {
+	.tbl = msm8996_init_tbl,
+	.tbl_num = ARRAY_SIZE(msm8996_init_tbl),
+};
+
+/**
+ * struct qusb2_phy - structure holding qusb2 phy attributes
+ *
+ * @phy: generic phy
+ * @base: iomapped memory space for qubs2 phy
+ *
+ * @cfg_ahb_clk: AHB2PHY interface clock
+ * @ref_clk: phy reference clock
+ * @iface_clk: phy interface clock
+ *
+ * @phy_reset: phy reset control
+ *
+ * @vdda_phy: vdd supply to the phy core block
+ * @vdda_pll: 1.8V vdd supply to ref_clk block
+ * @vdda_phy_dpdm: 3.1V vdd supply to Dp/Dm port signals
+ * @tcsr: TCSR syscon register map
+ * @cell: nvmem cell containing phy tuning value
+ *
+ * @cfg: phy config data
+ * @has_se_clk_scheme: indicate if PHY has single-ended ref clock scheme
+ */
+struct qusb2_phy {
+	struct phy *phy;
+	void __iomem *base;
+
+	struct clk *cfg_ahb_clk;
+	struct clk *ref_clk;
+	struct clk *iface_clk;
+
+	struct reset_control *phy_reset;
+
+	struct regulator *vdd_phy;
+	struct regulator *vdda_pll;
+	struct regulator *vdda_phy_dpdm;
+
+	struct regmap *tcsr;
+	struct nvmem_cell *cell;
+
+	const struct qusb2_phy_cfg *cfg;
+	bool has_se_clk_scheme;
+};
+
+static inline void qusb2_setbits(void __iomem *base, u32 offset, u32 val)
+{
+	u32 reg;
+
+	reg = readl_relaxed(base + offset);
+	reg |= val;
+	writel_relaxed(reg, base + offset);
+
+	/* Ensure above write is completed */
+	mb();
+}
+
+static inline void qusb2_clrbits(void __iomem *base, u32 offset, u32 val)
+{
+	u32 reg;
+
+	reg = readl_relaxed(base + offset);
+	reg &= ~val;
+	writel_relaxed(reg, base + offset);
+
+	/* Ensure above write is completed */
+	mb();
+}
+
+static inline void qcom_qusb2_phy_configure(void __iomem *base,
+				const struct qusb2_phy_init_tbl tbl[],
+				int num)
+{
+	int i;
+
+	for (i = 0; i < num; i++)
+		writel_relaxed(tbl[i].val, base + tbl[i].offset);
+
+	/* flush buffered writes */
+	mb();
+}
+
+static int qusb2_phy_toggle_power(struct qusb2_phy *qphy, bool on)
+{
+	struct device *dev = &qphy->phy->dev;
+	int ret;
+
+	if (!on) {
+		ret = 0;
+		goto disable_vdda_phy_dpdm;
+	}
+
+	ret = regulator_enable(qphy->vdd_phy);
+	if (ret) {
+		dev_err(dev, "failed to enable vdd-phy, %d\n", ret);
+		goto err_vdd_phy;
+	}
+
+	ret = regulator_enable(qphy->vdda_pll);
+	if (ret) {
+		dev_err(dev, "failed to enable vdda-pll, %d\n", ret);
+		goto disable_vdd_phy;
+	}
+
+	ret = regulator_enable(qphy->vdda_phy_dpdm);
+	if (ret) {
+		dev_err(dev, "failed to enable vdda-phy-dpdm, %d\n", ret);
+		goto disable_vdda_pll;
+	}
+
+	dev_vdbg(dev, "%s(): regulators are turned on\n", __func__);
+
+	return ret;
+
+disable_vdda_phy_dpdm:
+	regulator_disable(qphy->vdda_phy_dpdm);
+disable_vdda_pll:
+	regulator_disable(qphy->vdda_pll);
+disable_vdd_phy:
+	regulator_disable(qphy->vdd_phy);
+err_vdd_phy:
+	dev_vdbg(dev, "%s(): regulators are turned off\n", __func__);
+	return ret;
+}
+
+/*
+ * Fetches HS Tx tuning value from nvmem and sets the
+ * QUSB2PHY_PORT_TUNE2 register.
+ * For error case, skip setting the value and use the default value.
+ */
+static void qusb2_phy_set_tune2_param(struct qusb2_phy *qphy)
+{
+	struct device *dev = &qphy->phy->dev;
+	u8 *val;
+
+	/*
+	 * Read efuse register having TUNE2 parameter's high nibble.
+	 * If efuse register shows value as 0x0, or if we fail to find
+	 * a valid efuse register settings, then use default value
+	 * as 0xB for high nibble that we have already set while
+	 * configuring phy.
+	 */
+	val = nvmem_cell_read(qphy->cell, NULL);
+	if (IS_ERR(val) || !val[0]) {
+		dev_dbg(dev, "failed to read a valid hs-tx trim value\n");
+		return;
+	}
+
+	/* Fused TUNE2 value is the higher nibble only */
+	qusb2_setbits(qphy->base, QUSB2PHY_PORT_TUNE2, val[0] << 0x4);
+}
+
+static int qusb2_phy_poweron(struct phy *phy)
+{
+	struct qusb2_phy *qphy = phy_get_drvdata(phy);
+	int ret;
+
+	dev_vdbg(&phy->dev, "%s(): Powering-on QUSB2 phy\n", __func__);
+
+	/* turn on regulator supplies */
+	ret = qusb2_phy_toggle_power(qphy, true);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(qphy->iface_clk);
+	if (ret)
+		dev_err(&phy->dev, "failed to enable iface_clk, %d\n", ret);
+
+	return ret;
+}
+
+static int qusb2_phy_poweroff(struct phy *phy)
+{
+	struct qusb2_phy *qphy = phy_get_drvdata(phy);
+
+	clk_disable_unprepare(qphy->iface_clk);
+
+	qusb2_phy_toggle_power(qphy, false);
+
+	return 0;
+}
+
+static int qusb2_phy_init(struct phy *phy)
+{
+	struct qusb2_phy *qphy = phy_get_drvdata(phy);
+	unsigned int val;
+	unsigned int clk_scheme;
+	int ret;
+
+	dev_vdbg(&phy->dev, "%s(): Initializing QUSB2 phy\n", __func__);
+
+	/* enable ahb interface clock to program phy */
+	ret = clk_prepare_enable(qphy->cfg_ahb_clk);
+	if (ret) {
+		dev_err(&phy->dev, "failed to enable cfg ahb clock, %d\n", ret);
+		return ret;
+	}
+
+	/* Perform phy reset */
+	ret = reset_control_assert(qphy->phy_reset);
+	if (ret) {
+		dev_err(&phy->dev, "failed to assert phy_reset, %d\n", ret);
+		goto disable_ahb_clk;
+	}
+
+	/* 100 us delay to keep PHY in reset mode */
+	usleep_range(100, 150);
+
+	ret = reset_control_deassert(qphy->phy_reset);
+	if (ret) {
+		dev_err(&phy->dev, "failed to de-assert phy_reset, %d\n", ret);
+		goto disable_ahb_clk;
+	}
+
+	/* Disable the PHY */
+	qusb2_setbits(qphy->base, QUSB2PHY_PORT_POWERDOWN,
+			CLAMP_N_EN | FREEZIO_N | POWER_DOWN);
+
+	/* save reset value to override reference clock scheme later */
+	val = readl_relaxed(qphy->base + QUSB2PHY_PLL_TEST);
+
+	qcom_qusb2_phy_configure(qphy->base, qphy->cfg->tbl,
+					qphy->cfg->tbl_num);
+
+	/* Set efuse value for tuning the PHY */
+	qusb2_phy_set_tune2_param(qphy);
+
+	/* Enable the PHY */
+	qusb2_clrbits(qphy->base, QUSB2PHY_PORT_POWERDOWN, POWER_DOWN);
+
+	/* Required to get phy pll lock successfully */
+	usleep_range(150, 160);
+
+	/* Default is single-ended clock on msm8996 */
+	qphy->has_se_clk_scheme = true;
+	/*
+	 * read TCSR_PHY_CLK_SCHEME register to check if single-ended
+	 * clock scheme is selected. If yes, then disable differential
+	 * ref_clk and use single-ended clock, otherwise use differential
+	 * ref_clk only.
+	 */
+	if (qphy->tcsr) {
+		ret = regmap_read(qphy->tcsr, qphy->cfg->clk_scheme_offset,
+							&clk_scheme);
+		if (ret) {
+			dev_err(&phy->dev, "failed to read clk scheme reg\n");
+			goto assert_phy_reset;
+		}
+
+		/* is it a differential clock scheme ? */
+		if (!(clk_scheme & PHY_CLK_SCHEME_SEL)) {
+			dev_vdbg(&phy->dev, "%s(): select differential clk\n",
+								__func__);
+			qphy->has_se_clk_scheme = false;
+		} else {
+			dev_vdbg(&phy->dev, "%s(): select single-ended clk\n",
+								__func__);
+		}
+	}
+
+	if (!qphy->has_se_clk_scheme) {
+		val &= ~CLK_REF_SEL;
+		ret = clk_prepare_enable(qphy->ref_clk);
+		if (ret) {
+			dev_err(&phy->dev, "failed to enable ref clk, %d\n",
+									ret);
+			goto assert_phy_reset;
+		}
+	} else {
+		val |= CLK_REF_SEL;
+	}
+
+	writel_relaxed(val, qphy->base + QUSB2PHY_PLL_TEST);
+
+	/* Make sure that above write is completed to get PLL source clock */
+	wmb();
+
+	/* Required to get phy pll lock successfully */
+	usleep_range(100, 110);
+
+	val = readb_relaxed(qphy->base + QUSB2PHY_PLL_STATUS);
+	if (!(val & PLL_LOCKED)) {
+		dev_err(&phy->dev,
+			"QUSB2PHY pll lock failed: status reg = %x\n", val);
+		ret = -EBUSY;
+		goto disable_ref_clk;
+	}
+
+	return 0;
+
+disable_ref_clk:
+	if (!qphy->has_se_clk_scheme)
+		clk_disable_unprepare(qphy->ref_clk);
+assert_phy_reset:
+	reset_control_assert(qphy->phy_reset);
+disable_ahb_clk:
+	clk_disable_unprepare(qphy->cfg_ahb_clk);
+	return ret;
+}
+
+static int qusb2_phy_exit(struct phy *phy)
+{
+	struct qusb2_phy *qphy = phy_get_drvdata(phy);
+
+	/* Disable the PHY */
+	qusb2_setbits(qphy->base, QUSB2PHY_PORT_POWERDOWN,
+			CLAMP_N_EN | FREEZIO_N | POWER_DOWN);
+
+	if (!qphy->has_se_clk_scheme)
+		clk_disable_unprepare(qphy->ref_clk);
+
+	reset_control_assert(qphy->phy_reset);
+
+	clk_disable_unprepare(qphy->cfg_ahb_clk);
+
+	return 0;
+}
+
+static const struct phy_ops qusb2_phy_gen_ops = {
+	.init		= qusb2_phy_init,
+	.exit		= qusb2_phy_exit,
+	.power_on	= qusb2_phy_poweron,
+	.power_off	= qusb2_phy_poweroff,
+	.owner		= THIS_MODULE,
+};
+
+static const struct of_device_id qusb2_phy_of_match_table[] = {
+	{
+		.compatible	= "qcom,msm8996-qusb2-phy",
+		.data		= &msm8996_phy_cfg,
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, qusb2_phy_of_match_table);
+
+static int qusb2_phy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct qusb2_phy *qphy;
+	struct phy_provider *phy_provider;
+	struct phy *generic_phy;
+	struct resource *res;
+	int ret;
+
+	qphy = devm_kzalloc(dev, sizeof(*qphy), GFP_KERNEL);
+	if (!qphy)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	qphy->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(qphy->base))
+		return PTR_ERR(qphy->base);
+
+	qphy->cfg_ahb_clk = devm_clk_get(dev, "cfg_ahb");
+	if (IS_ERR(qphy->cfg_ahb_clk)) {
+		ret = PTR_ERR(qphy->cfg_ahb_clk);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "failed to get cfg ahb clk, %d\n", ret);
+		return ret;
+	}
+
+	qphy->ref_clk = devm_clk_get(dev, "ref");
+	if (IS_ERR(qphy->ref_clk)) {
+		ret = PTR_ERR(qphy->ref_clk);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "failed to get ref clk, %d\n", ret);
+		return ret;
+	}
+
+	qphy->iface_clk = devm_clk_get(dev, "iface");
+	if (IS_ERR(qphy->iface_clk)) {
+		ret = PTR_ERR(qphy->iface_clk);
+		if (ret == -EPROBE_DEFER)
+			return ret;
+		qphy->iface_clk = NULL;
+		dev_dbg(dev, "failed to get iface clk, %d\n", ret);
+	}
+
+	qphy->phy_reset = devm_reset_control_get_by_index(&pdev->dev, 0);
+	if (IS_ERR(qphy->phy_reset)) {
+		dev_err(dev, "failed to get phy core reset\n");
+		return PTR_ERR(qphy->phy_reset);
+	}
+
+	qphy->vdd_phy = devm_regulator_get(dev, "vdd-phy");
+	if (IS_ERR(qphy->vdd_phy))
+		return PTR_ERR(qphy->vdd_phy);
+
+	qphy->vdda_pll = devm_regulator_get(dev, "vdda-pll");
+	if (IS_ERR(qphy->vdda_pll))
+		return PTR_ERR(qphy->vdda_pll);
+
+	qphy->vdda_phy_dpdm = devm_regulator_get(dev, "vdda-phy-dpdm");
+	if (IS_ERR(qphy->vdda_phy_dpdm))
+		return PTR_ERR(qphy->vdda_phy_dpdm);
+
+	/* Get the specific init parameters of QMP phy */
+	qphy->cfg = of_device_get_match_data(dev);
+
+	qphy->tcsr = syscon_regmap_lookup_by_phandle(dev->of_node,
+							"qcom,tcsr-syscon");
+	if (IS_ERR(qphy->tcsr)) {
+		dev_dbg(dev, "failed to lookup TCSR regmap\n");
+		qphy->tcsr = NULL;
+	}
+
+	qphy->cell = devm_nvmem_cell_get(dev, NULL);
+	if (IS_ERR(qphy->cell)) {
+		if (PTR_ERR(qphy->cell) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+		qphy->cell = NULL;
+		dev_dbg(dev, "failed to lookup tune2 hstx trim value\n");
+	}
+
+	generic_phy = devm_phy_create(dev, NULL, &qusb2_phy_gen_ops);
+	if (IS_ERR(generic_phy)) {
+		ret = PTR_ERR(generic_phy);
+		dev_err(dev, "failed to create phy, %d\n", ret);
+		return ret;
+	}
+	qphy->phy = generic_phy;
+
+	dev_set_drvdata(dev, qphy);
+	phy_set_drvdata(generic_phy, qphy);
+
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	if (IS_ERR(phy_provider)) {
+		ret = PTR_ERR(phy_provider);
+		dev_err(dev, "failed to register phy, %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static struct platform_driver qusb2_phy_driver = {
+	.probe		= qusb2_phy_probe,
+	.driver = {
+		.name	= "qcom-qusb2-phy",
+		.of_match_table = of_match_ptr(qusb2_phy_of_match_table),
+	},
+};
+
+module_platform_driver(qusb2_phy_driver);
+
+MODULE_AUTHOR("Vivek Gautam <vivek.gautam@codeaurora.org>");
+MODULE_DESCRIPTION("Qualcomm QUSB2 PHY driver");
+MODULE_LICENSE("GPL v2");
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v4 2/4] phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips

[Kishon Vijay Abraham I]

Hi,

On Tuesday 10 January 2017 04:21 PM, Vivek Gautam wrote:
> PHY transceiver driver for QUSB2 phy controller that provides
> HighSpeed functionality for DWC3 controller present on
> Qualcomm chipsets.
> 
> Signed-off-by: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Reviewed-by: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
> 
> Changes since v3:
>  - Added 'Reviewed-by' from Stephen.
>  - Fixed debug message for qusb2_phy_set_tune2_param().
>  - Replaced devm_reset_control_get() with devm_reset_control_get_by_index()
>    since we are requesting only one reset.
>  - Updated devm_nvmem_cell_get() with a NULL cell id.
>  - Made error labels more idiomatic.
>  - Refactored qusb2_setbits() and qusb2_clrbits() a little bit to accept
>    base address and register offset as two separate arguments.
> 
> Changes since v2:
>  - Removed selecting 'RESET_CONTROLLER' config.
>  - Added error handling for clk_prepare_enable paths.
>  - Removed explicitly setting ref_clk rate to 19.2 MHz. Don't need to
>    do that since 'xo' is modeled as parent to this clock.
>  - Removed 'ref_clk_src' handling. Driver doesn't need to request and
>    handle this clock.
>  - Moved nvmem_cell_get() to probe function.
>  - Simplified phy pll status handling.
>  - Using of_device_get_match_data() to get match data.
>  - Uniformly using lowercase for hex numbers.
>  - Fixed sparse warnings.
>  - Using shorter variable names in structure and in functions.
>  - Handling various comment style shortcomings.
> 
> Changes since v1:
>  - removed reference to clk_enabled/pwr_enabled.
>  - moved clock and regulator enable code to phy_power_on/off() callbacks.
>  - fixed return on EPROBE_DEFER in qusb2_phy_probe().
>  - fixed phy create and phy register ordering.
>  - removed references to non-lkml links from commit message.
>  - took care of other minor nits.
>  - Fixed coccinelle warnings -
>    'PTR_ERR applied after initialization to constant'
>  - Addressed review comment, regarding qfprom access for tune2 param value.
>    This driver is now based on qfprom patch[1] that allows byte access now.
> 
>  drivers/phy/Kconfig          |  10 +
>  drivers/phy/Makefile         |   1 +
>  drivers/phy/phy-qcom-qusb2.c | 539 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 550 insertions(+)
>  create mode 100644 drivers/phy/phy-qcom-qusb2.c
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index e8eb7f225a88..0ed53d018b23 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -430,6 +430,16 @@ config PHY_STIH407_USB
>  	  Enable this support to enable the picoPHY device used by USB2
>  	  and USB3 controllers on STMicroelectronics STiH407 SoC families.
>  
> +config PHY_QCOM_QUSB2
> +	tristate "Qualcomm QUSB2 PHY Driver"
> +	depends on OF && (ARCH_QCOM || COMPILE_TEST)
> +	select GENERIC_PHY
> +	help
> +	  Enable this to support the HighSpeed QUSB2 PHY transceiver for USB
> +	  controllers on Qualcomm chips. This driver supports the high-speed
> +	  PHY which is usually paired with either the ChipIdea or Synopsys DWC3
> +	  USB IPs on MSM SOCs.
> +
>  config PHY_QCOM_UFS
>  	tristate "Qualcomm UFS PHY driver"
>  	depends on OF && ARCH_QCOM
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 65eb2f436a41..dad1682b80e3 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -49,6 +49,7 @@ obj-$(CONFIG_PHY_ST_SPEAR1310_MIPHY)	+= phy-spear1310-miphy.o
>  obj-$(CONFIG_PHY_ST_SPEAR1340_MIPHY)	+= phy-spear1340-miphy.o
>  obj-$(CONFIG_PHY_XGENE)			+= phy-xgene.o
>  obj-$(CONFIG_PHY_STIH407_USB)		+= phy-stih407-usb.o
> +obj-$(CONFIG_PHY_QCOM_QUSB2) 	+= phy-qcom-qusb2.o
>  obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs.o
>  obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs-qmp-20nm.o
>  obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs-qmp-14nm.o
> diff --git a/drivers/phy/phy-qcom-qusb2.c b/drivers/phy/phy-qcom-qusb2.c
> new file mode 100644
> index 000000000000..c69118610164
> --- /dev/null
> +++ b/drivers/phy/phy-qcom-qusb2.c
> @@ -0,0 +1,539 @@
> +/*
> + * Copyright (c) 2016, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/reset.h>
> +#include <linux/slab.h>
> +
> +#define QUSB2PHY_PLL_TEST		0x04
> +#define CLK_REF_SEL			BIT(7)
> +
> +#define QUSB2PHY_PLL_TUNE		0x08
> +#define QUSB2PHY_PLL_USER_CTL1		0x0c
> +#define QUSB2PHY_PLL_USER_CTL2		0x10
> +#define QUSB2PHY_PLL_AUTOPGM_CTL1	0x1c
> +#define QUSB2PHY_PLL_PWR_CTRL		0x18
> +
> +#define QUSB2PHY_PLL_STATUS		0x38
> +#define PLL_LOCKED			BIT(5)
> +
> +#define QUSB2PHY_PORT_TUNE1		0x80
> +#define QUSB2PHY_PORT_TUNE2		0x84
> +#define QUSB2PHY_PORT_TUNE3		0x88
> +#define QUSB2PHY_PORT_TUNE4		0x8c
> +#define QUSB2PHY_PORT_TUNE5		0x90
> +#define QUSB2PHY_PORT_TEST2		0x9c
> +
> +#define QUSB2PHY_PORT_POWERDOWN		0xb4
> +#define CLAMP_N_EN			BIT(5)
> +#define FREEZIO_N			BIT(1)
> +#define POWER_DOWN			BIT(0)
> +
> +#define QUSB2PHY_REFCLK_ENABLE		BIT(0)
> +
> +#define PHY_CLK_SCHEME_SEL		BIT(0)
> +
> +struct qusb2_phy_init_tbl {
> +	unsigned int offset;
> +	unsigned int val;
> +};
> +#define QUSB2_PHY_INIT_CFG(o, v) \
> +	{			\
> +		.offset = o,	\
> +		.val = v,	\
> +	}
> +
> +static const struct qusb2_phy_init_tbl msm8996_init_tbl[] = {
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE1, 0xf8),
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE2, 0xb3),
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE3, 0x83),
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE4, 0xc0),
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_TUNE, 0x30),
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL1, 0x79),
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL2, 0x21),
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TEST2, 0x14),
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_AUTOPGM_CTL1, 0x9f),
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_PWR_CTRL, 0x00),
> +};

I wish all this data comes from device tree and one API in phy-core can do all
these settings. Your other driver qcom-qmp also seems to have a bunch of
similar settings.

The problem is every vnedor driver adds a bunch of code to perform the same
thing again and again when all of these settings can be done by a single phy API.

Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/4] phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips

[Kishon Vijay Abraham I]

Hi,

On Tuesday 10 January 2017 04:21 PM, Vivek Gautam wrote:
> PHY transceiver driver for QUSB2 phy controller that provides
> HighSpeed functionality for DWC3 controller present on
> Qualcomm chipsets.
> 
> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
> Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
> 
> Changes since v3:
>  - Added 'Reviewed-by' from Stephen.
>  - Fixed debug message for qusb2_phy_set_tune2_param().
>  - Replaced devm_reset_control_get() with devm_reset_control_get_by_index()
>    since we are requesting only one reset.
>  - Updated devm_nvmem_cell_get() with a NULL cell id.
>  - Made error labels more idiomatic.
>  - Refactored qusb2_setbits() and qusb2_clrbits() a little bit to accept
>    base address and register offset as two separate arguments.
> 
> Changes since v2:
>  - Removed selecting 'RESET_CONTROLLER' config.
>  - Added error handling for clk_prepare_enable paths.
>  - Removed explicitly setting ref_clk rate to 19.2 MHz. Don't need to
>    do that since 'xo' is modeled as parent to this clock.
>  - Removed 'ref_clk_src' handling. Driver doesn't need to request and
>    handle this clock.
>  - Moved nvmem_cell_get() to probe function.
>  - Simplified phy pll status handling.
>  - Using of_device_get_match_data() to get match data.
>  - Uniformly using lowercase for hex numbers.
>  - Fixed sparse warnings.
>  - Using shorter variable names in structure and in functions.
>  - Handling various comment style shortcomings.
> 
> Changes since v1:
>  - removed reference to clk_enabled/pwr_enabled.
>  - moved clock and regulator enable code to phy_power_on/off() callbacks.
>  - fixed return on EPROBE_DEFER in qusb2_phy_probe().
>  - fixed phy create and phy register ordering.
>  - removed references to non-lkml links from commit message.
>  - took care of other minor nits.
>  - Fixed coccinelle warnings -
>    'PTR_ERR applied after initialization to constant'
>  - Addressed review comment, regarding qfprom access for tune2 param value.
>    This driver is now based on qfprom patch[1] that allows byte access now.
> 
>  drivers/phy/Kconfig          |  10 +
>  drivers/phy/Makefile         |   1 +
>  drivers/phy/phy-qcom-qusb2.c | 539 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 550 insertions(+)
>  create mode 100644 drivers/phy/phy-qcom-qusb2.c
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index e8eb7f225a88..0ed53d018b23 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -430,6 +430,16 @@ config PHY_STIH407_USB
>  	  Enable this support to enable the picoPHY device used by USB2
>  	  and USB3 controllers on STMicroelectronics STiH407 SoC families.
>  
> +config PHY_QCOM_QUSB2
> +	tristate "Qualcomm QUSB2 PHY Driver"
> +	depends on OF && (ARCH_QCOM || COMPILE_TEST)
> +	select GENERIC_PHY
> +	help
> +	  Enable this to support the HighSpeed QUSB2 PHY transceiver for USB
> +	  controllers on Qualcomm chips. This driver supports the high-speed
> +	  PHY which is usually paired with either the ChipIdea or Synopsys DWC3
> +	  USB IPs on MSM SOCs.
> +
>  config PHY_QCOM_UFS
>  	tristate "Qualcomm UFS PHY driver"
>  	depends on OF && ARCH_QCOM
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 65eb2f436a41..dad1682b80e3 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -49,6 +49,7 @@ obj-$(CONFIG_PHY_ST_SPEAR1310_MIPHY)	+= phy-spear1310-miphy.o
>  obj-$(CONFIG_PHY_ST_SPEAR1340_MIPHY)	+= phy-spear1340-miphy.o
>  obj-$(CONFIG_PHY_XGENE)			+= phy-xgene.o
>  obj-$(CONFIG_PHY_STIH407_USB)		+= phy-stih407-usb.o
> +obj-$(CONFIG_PHY_QCOM_QUSB2) 	+= phy-qcom-qusb2.o
>  obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs.o
>  obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs-qmp-20nm.o
>  obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs-qmp-14nm.o
> diff --git a/drivers/phy/phy-qcom-qusb2.c b/drivers/phy/phy-qcom-qusb2.c
> new file mode 100644
> index 000000000000..c69118610164
> --- /dev/null
> +++ b/drivers/phy/phy-qcom-qusb2.c
> @@ -0,0 +1,539 @@
> +/*
> + * Copyright (c) 2016, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/reset.h>
> +#include <linux/slab.h>
> +
> +#define QUSB2PHY_PLL_TEST		0x04
> +#define CLK_REF_SEL			BIT(7)
> +
> +#define QUSB2PHY_PLL_TUNE		0x08
> +#define QUSB2PHY_PLL_USER_CTL1		0x0c
> +#define QUSB2PHY_PLL_USER_CTL2		0x10
> +#define QUSB2PHY_PLL_AUTOPGM_CTL1	0x1c
> +#define QUSB2PHY_PLL_PWR_CTRL		0x18
> +
> +#define QUSB2PHY_PLL_STATUS		0x38
> +#define PLL_LOCKED			BIT(5)
> +
> +#define QUSB2PHY_PORT_TUNE1		0x80
> +#define QUSB2PHY_PORT_TUNE2		0x84
> +#define QUSB2PHY_PORT_TUNE3		0x88
> +#define QUSB2PHY_PORT_TUNE4		0x8c
> +#define QUSB2PHY_PORT_TUNE5		0x90
> +#define QUSB2PHY_PORT_TEST2		0x9c
> +
> +#define QUSB2PHY_PORT_POWERDOWN		0xb4
> +#define CLAMP_N_EN			BIT(5)
> +#define FREEZIO_N			BIT(1)
> +#define POWER_DOWN			BIT(0)
> +
> +#define QUSB2PHY_REFCLK_ENABLE		BIT(0)
> +
> +#define PHY_CLK_SCHEME_SEL		BIT(0)
> +
> +struct qusb2_phy_init_tbl {
> +	unsigned int offset;
> +	unsigned int val;
> +};
> +#define QUSB2_PHY_INIT_CFG(o, v) \
> +	{			\
> +		.offset = o,	\
> +		.val = v,	\
> +	}
> +
> +static const struct qusb2_phy_init_tbl msm8996_init_tbl[] = {
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE1, 0xf8),
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE2, 0xb3),
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE3, 0x83),
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE4, 0xc0),
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_TUNE, 0x30),
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL1, 0x79),
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL2, 0x21),
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TEST2, 0x14),
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_AUTOPGM_CTL1, 0x9f),
> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_PWR_CTRL, 0x00),
> +};

I wish all this data comes from device tree and one API in phy-core can do all
these settings. Your other driver qcom-qmp also seems to have a bunch of
similar settings.

The problem is every vnedor driver adds a bunch of code to perform the same
thing again and again when all of these settings can be done by a single phy API.

Thanks
Kishon

Re: [PATCH v4 2/4] phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips

[Vivek Gautam]

Hi Kishon,


On 01/16/2017 02:15 PM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Tuesday 10 January 2017 04:21 PM, Vivek Gautam wrote:
>> PHY transceiver driver for QUSB2 phy controller that provides
>> HighSpeed functionality for DWC3 controller present on
>> Qualcomm chipsets.
>>
>> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
>> Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
>> ---
>>
>> Changes since v3:
>>   - Added 'Reviewed-by' from Stephen.
>>   - Fixed debug message for qusb2_phy_set_tune2_param().
>>   - Replaced devm_reset_control_get() with devm_reset_control_get_by_index()
>>     since we are requesting only one reset.
>>   - Updated devm_nvmem_cell_get() with a NULL cell id.
>>   - Made error labels more idiomatic.
>>   - Refactored qusb2_setbits() and qusb2_clrbits() a little bit to accept
>>     base address and register offset as two separate arguments.
>>
>> Changes since v2:
>>   - Removed selecting 'RESET_CONTROLLER' config.
>>   - Added error handling for clk_prepare_enable paths.
>>   - Removed explicitly setting ref_clk rate to 19.2 MHz. Don't need to
>>     do that since 'xo' is modeled as parent to this clock.
>>   - Removed 'ref_clk_src' handling. Driver doesn't need to request and
>>     handle this clock.
>>   - Moved nvmem_cell_get() to probe function.
>>   - Simplified phy pll status handling.
>>   - Using of_device_get_match_data() to get match data.
>>   - Uniformly using lowercase for hex numbers.
>>   - Fixed sparse warnings.
>>   - Using shorter variable names in structure and in functions.
>>   - Handling various comment style shortcomings.
>>
>> Changes since v1:
>>   - removed reference to clk_enabled/pwr_enabled.
>>   - moved clock and regulator enable code to phy_power_on/off() callbacks.
>>   - fixed return on EPROBE_DEFER in qusb2_phy_probe().
>>   - fixed phy create and phy register ordering.
>>   - removed references to non-lkml links from commit message.
>>   - took care of other minor nits.
>>   - Fixed coccinelle warnings -
>>     'PTR_ERR applied after initialization to constant'
>>   - Addressed review comment, regarding qfprom access for tune2 param value.
>>     This driver is now based on qfprom patch[1] that allows byte access now.
>>
>>   drivers/phy/Kconfig          |  10 +
>>   drivers/phy/Makefile         |   1 +
>>   drivers/phy/phy-qcom-qusb2.c | 539 +++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 550 insertions(+)
>>   create mode 100644 drivers/phy/phy-qcom-qusb2.c
>>
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index e8eb7f225a88..0ed53d018b23 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -430,6 +430,16 @@ config PHY_STIH407_USB
>>   	  Enable this support to enable the picoPHY device used by USB2
>>   	  and USB3 controllers on STMicroelectronics STiH407 SoC families.
>>   
>> +config PHY_QCOM_QUSB2
>> +	tristate "Qualcomm QUSB2 PHY Driver"
>> +	depends on OF && (ARCH_QCOM || COMPILE_TEST)
>> +	select GENERIC_PHY
>> +	help
>> +	  Enable this to support the HighSpeed QUSB2 PHY transceiver for USB
>> +	  controllers on Qualcomm chips. This driver supports the high-speed
>> +	  PHY which is usually paired with either the ChipIdea or Synopsys DWC3
>> +	  USB IPs on MSM SOCs.
>> +
>>   config PHY_QCOM_UFS
>>   	tristate "Qualcomm UFS PHY driver"
>>   	depends on OF && ARCH_QCOM
>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>> index 65eb2f436a41..dad1682b80e3 100644
>> --- a/drivers/phy/Makefile
>> +++ b/drivers/phy/Makefile
>> @@ -49,6 +49,7 @@ obj-$(CONFIG_PHY_ST_SPEAR1310_MIPHY)	+= phy-spear1310-miphy.o
>>   obj-$(CONFIG_PHY_ST_SPEAR1340_MIPHY)	+= phy-spear1340-miphy.o
>>   obj-$(CONFIG_PHY_XGENE)			+= phy-xgene.o
>>   obj-$(CONFIG_PHY_STIH407_USB)		+= phy-stih407-usb.o
>> +obj-$(CONFIG_PHY_QCOM_QUSB2) 	+= phy-qcom-qusb2.o
>>   obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs.o
>>   obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs-qmp-20nm.o
>>   obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs-qmp-14nm.o
>> diff --git a/drivers/phy/phy-qcom-qusb2.c b/drivers/phy/phy-qcom-qusb2.c
>> new file mode 100644
>> index 000000000000..c69118610164
>> --- /dev/null
>> +++ b/drivers/phy/phy-qcom-qusb2.c
>> @@ -0,0 +1,539 @@
>> +/*
>> + * Copyright (c) 2016, The Linux Foundation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/nvmem-consumer.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/reset.h>
>> +#include <linux/slab.h>
>> +
>> +#define QUSB2PHY_PLL_TEST		0x04
>> +#define CLK_REF_SEL			BIT(7)
>> +
>> +#define QUSB2PHY_PLL_TUNE		0x08
>> +#define QUSB2PHY_PLL_USER_CTL1		0x0c
>> +#define QUSB2PHY_PLL_USER_CTL2		0x10
>> +#define QUSB2PHY_PLL_AUTOPGM_CTL1	0x1c
>> +#define QUSB2PHY_PLL_PWR_CTRL		0x18
>> +
>> +#define QUSB2PHY_PLL_STATUS		0x38
>> +#define PLL_LOCKED			BIT(5)
>> +
>> +#define QUSB2PHY_PORT_TUNE1		0x80
>> +#define QUSB2PHY_PORT_TUNE2		0x84
>> +#define QUSB2PHY_PORT_TUNE3		0x88
>> +#define QUSB2PHY_PORT_TUNE4		0x8c
>> +#define QUSB2PHY_PORT_TUNE5		0x90
>> +#define QUSB2PHY_PORT_TEST2		0x9c
>> +
>> +#define QUSB2PHY_PORT_POWERDOWN		0xb4
>> +#define CLAMP_N_EN			BIT(5)
>> +#define FREEZIO_N			BIT(1)
>> +#define POWER_DOWN			BIT(0)
>> +
>> +#define QUSB2PHY_REFCLK_ENABLE		BIT(0)
>> +
>> +#define PHY_CLK_SCHEME_SEL		BIT(0)
>> +
>> +struct qusb2_phy_init_tbl {
>> +	unsigned int offset;
>> +	unsigned int val;
>> +};
>> +#define QUSB2_PHY_INIT_CFG(o, v) \
>> +	{			\
>> +		.offset = o,	\
>> +		.val = v,	\
>> +	}
>> +
>> +static const struct qusb2_phy_init_tbl msm8996_init_tbl[] = {
>> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE1, 0xf8),
>> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE2, 0xb3),
>> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE3, 0x83),
>> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE4, 0xc0),
>> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_TUNE, 0x30),
>> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL1, 0x79),
>> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL2, 0x21),
>> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TEST2, 0x14),
>> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_AUTOPGM_CTL1, 0x9f),
>> +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_PWR_CTRL, 0x00),
>> +};
> I wish all this data comes from device tree and one API in phy-core can do all
> these settings. Your other driver qcom-qmp also seems to have a bunch of
> similar settings.
>
> The problem is every vendor driver adds a bunch of code to perform the same
> thing again and again when all of these settings can be done by a single phy API.

Yes, i understand this. You have commented similar thing in the patch 
from Jaehoon -
[PATCH V2 2/5] phy: phy-exynos-pcie: Add support for Exynos PCIe phy

I would like to understand the requirements here.
Would you like me to get all this information from the device tree -
an array of register offset and value pair, which we can then program
by calling a phy_ops (may be calibrate) ? Something of this sort:

phy-calibrate-data = <val1, register_offset1>,
                                   <val2, register_offset2>,
                                   <val3, register_offset3>,
                                   ....

I am sure having such information in the driver (like i have in my patch)
makes the driver look more clumsy.
But, all this data can be pretty huge - a set of some 100+ 
register-value pairs
for QMP phy, for example. So, will it be okay to get this from device tree ?
We also note here that such information changes from one IP version to 
another.
I remember Rob having some concerns about it.


Regards
Vivek
> Thanks
> Kishon

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v4 2/4] phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips

[Bjorn Andersson]

On Wed 18 Jan 01:13 PST 2017, Vivek Gautam wrote:
> On 01/16/2017 02:15 PM, Kishon Vijay Abraham I wrote:
> > Hi,
> > 
> > On Tuesday 10 January 2017 04:21 PM, Vivek Gautam wrote:
[..]
> > > +static const struct qusb2_phy_init_tbl msm8996_init_tbl[] = {
> > > +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE1, 0xf8),
> > > +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE2, 0xb3),
> > > +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE3, 0x83),
> > > +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE4, 0xc0),
> > > +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_TUNE, 0x30),
> > > +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL1, 0x79),
> > > +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL2, 0x21),
> > > +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TEST2, 0x14),
> > > +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_AUTOPGM_CTL1, 0x9f),
> > > +	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_PWR_CTRL, 0x00),
> > > +};
> > I wish all this data comes from device tree and one API in phy-core can do all
> > these settings. Your other driver qcom-qmp also seems to have a bunch of
> > similar settings.
> > 
> > The problem is every vendor driver adds a bunch of code to perform the same
> > thing again and again when all of these settings can be done by a single phy API.
> 
> Yes, i understand this. You have commented similar thing in the patch from
> Jaehoon -
> [PATCH V2 2/5] phy: phy-exynos-pcie: Add support for Exynos PCIe phy
> 
> I would like to understand the requirements here.
> Would you like me to get all this information from the device tree -
> an array of register offset and value pair, which we can then program
> by calling a phy_ops (may be calibrate) ? Something of this sort:
> 
> phy-calibrate-data = <val1, register_offset1>,
>                                   <val2, register_offset2>,
>                                   <val3, register_offset3>,
>                                   ....
> 
> I am sure having such information in the driver (like i have in my patch)
> makes the driver look more clumsy.
> But, all this data can be pretty huge - a set of some 100+ register-value
> pairs
> for QMP phy, for example. So, will it be okay to get this from device tree ?
> We also note here that such information changes from one IP version to
> another.
> I remember Rob having some concerns about it.
> 

The devicetree is supposed to describe which hardware components a
certain device has, most of the time this carries a set of properties to
describe how this piece is connected and configured.

A dump of magic register values does not describe how the QMP is
connected to anything and is, as far as this patch shows, static for
this particular hardware block.

Further more moving this blob to devicetree will not allow us to treat
the various QMP configurations as one HW block, as there are other
differences as well.

Like many other drivers it's possible to create a generic version that
has every bit of logic driven by configuration from devicetree, but like
most of those cases this is not the way we split things.



And this has the side effect of keeping the dts files human readable,
human understandable and human maintainable.

Regards,
Bjorn

Re: [PATCH v4 2/4] phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips

[Vivek Gautam]

On Wed, Jan 18, 2017 at 11:33 PM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> On Wed 18 Jan 01:13 PST 2017, Vivek Gautam wrote:
>> On 01/16/2017 02:15 PM, Kishon Vijay Abraham I wrote:
>> > Hi,
>> >
>> > On Tuesday 10 January 2017 04:21 PM, Vivek Gautam wrote:
> [..]
>> > > +static const struct qusb2_phy_init_tbl msm8996_init_tbl[] = {
>> > > + QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE1, 0xf8),
>> > > + QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE2, 0xb3),
>> > > + QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE3, 0x83),
>> > > + QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE4, 0xc0),
>> > > + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_TUNE, 0x30),
>> > > + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL1, 0x79),
>> > > + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL2, 0x21),
>> > > + QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TEST2, 0x14),
>> > > + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_AUTOPGM_CTL1, 0x9f),
>> > > + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_PWR_CTRL, 0x00),
>> > > +};
>> > I wish all this data comes from device tree and one API in phy-core can do all
>> > these settings. Your other driver qcom-qmp also seems to have a bunch of
>> > similar settings.
>> >
>> > The problem is every vendor driver adds a bunch of code to perform the same
>> > thing again and again when all of these settings can be done by a single phy API.
>>
>> Yes, i understand this. You have commented similar thing in the patch from
>> Jaehoon -
>> [PATCH V2 2/5] phy: phy-exynos-pcie: Add support for Exynos PCIe phy
>>
>> I would like to understand the requirements here.
>> Would you like me to get all this information from the device tree -
>> an array of register offset and value pair, which we can then program
>> by calling a phy_ops (may be calibrate) ? Something of this sort:
>>
>> phy-calibrate-data = <val1, register_offset1>,
>>                                   <val2, register_offset2>,
>>                                   <val3, register_offset3>,
>>                                   ....
>>
>> I am sure having such information in the driver (like i have in my patch)
>> makes the driver look more clumsy.
>> But, all this data can be pretty huge - a set of some 100+ register-value
>> pairs
>> for QMP phy, for example. So, will it be okay to get this from device tree ?
>> We also note here that such information changes from one IP version to
>> another.
>> I remember Rob having some concerns about it.
>>
>
> The devicetree is supposed to describe which hardware components a
> certain device has, most of the time this carries a set of properties to
> describe how this piece is connected and configured.
>
> A dump of magic register values does not describe how the QMP is
> connected to anything and is, as far as this patch shows, static for
> this particular hardware block.

Yes, that's correct. The QMP and QUSB2 phy init sequences are a bunch
of static values for a particular IP version. These values hardly give a
meaningful data to put few phy bindings that could be referenced
to configure the phy further.

>
> Further more moving this blob to devicetree will not allow us to treat
> the various QMP configurations as one HW block, as there are other
> differences as well.
>
> Like many other drivers it's possible to create a generic version that
> has every bit of logic driven by configuration from devicetree, but like
> most of those cases this is not the way we split things.
>
> And this has the side effect of keeping the dts files human readable,
> human understandable and human maintainable.

That's right. These register-value pairs (100+ for qmp) don't give a human
understandable data when put in dts.

Kishon,
Please let me know if you have concerns.

If this looks good otherwise, please consider taking this for 4.11.


Regards
Vivek
>
> Regards,
> Bjorn
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v4 2/4] phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips

[Kishon Vijay Abraham I]

Hi,

On Monday 23 January 2017 03:43 PM, Vivek Gautam wrote:
> On Wed, Jan 18, 2017 at 11:33 PM, Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
>> On Wed 18 Jan 01:13 PST 2017, Vivek Gautam wrote:
>>> On 01/16/2017 02:15 PM, Kishon Vijay Abraham I wrote:
>>>> Hi,
>>>>
>>>> On Tuesday 10 January 2017 04:21 PM, Vivek Gautam wrote:
>> [..]
>>>>> +static const struct qusb2_phy_init_tbl msm8996_init_tbl[] = {
>>>>> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE1, 0xf8),
>>>>> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE2, 0xb3),
>>>>> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE3, 0x83),
>>>>> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE4, 0xc0),
>>>>> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_TUNE, 0x30),
>>>>> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL1, 0x79),
>>>>> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL2, 0x21),
>>>>> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TEST2, 0x14),
>>>>> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_AUTOPGM_CTL1, 0x9f),
>>>>> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_PWR_CTRL, 0x00),
>>>>> +};
>>>> I wish all this data comes from device tree and one API in phy-core can do all
>>>> these settings. Your other driver qcom-qmp also seems to have a bunch of
>>>> similar settings.
>>>>
>>>> The problem is every vendor driver adds a bunch of code to perform the same
>>>> thing again and again when all of these settings can be done by a single phy API.
>>>
>>> Yes, i understand this. You have commented similar thing in the patch from
>>> Jaehoon -
>>> [PATCH V2 2/5] phy: phy-exynos-pcie: Add support for Exynos PCIe phy
>>>
>>> I would like to understand the requirements here.
>>> Would you like me to get all this information from the device tree -
>>> an array of register offset and value pair, which we can then program
>>> by calling a phy_ops (may be calibrate) ? Something of this sort:
>>>
>>> phy-calibrate-data = <val1, register_offset1>,
>>>                                   <val2, register_offset2>,
>>>                                   <val3, register_offset3>,
>>>                                   ....
>>>
>>> I am sure having such information in the driver (like i have in my patch)
>>> makes the driver look more clumsy.
>>> But, all this data can be pretty huge - a set of some 100+ register-value
>>> pairs
>>> for QMP phy, for example. So, will it be okay to get this from device tree ?
>>> We also note here that such information changes from one IP version to
>>> another.
>>> I remember Rob having some concerns about it.
>>>
>>
>> The devicetree is supposed to describe which hardware components a
>> certain device has, most of the time this carries a set of properties to
>> describe how this piece is connected and configured.
>>
>> A dump of magic register values does not describe how the QMP is
>> connected to anything and is, as far as this patch shows, static for
>> this particular hardware block.

device tree has also been used to have configuration information. see [1]

> 
> Yes, that's correct. The QMP and QUSB2 phy init sequences are a bunch
> of static values for a particular IP version. These values hardly give a
> meaningful data to put few phy bindings that could be referenced
> to configure the phy further.

Not really. You can have clearly defined phy binding to give meaningful data.
Every driver doing the same configuration bloats the driver and these
configuration values are just magic values which hardly can be reviewed by anyone.
> 
>>
>> Further more moving this blob to devicetree will not allow us to treat
>> the various QMP configurations as one HW block, as there are other
>> differences as well.
>>
>> Like many other drivers it's possible to create a generic version that
>> has every bit of logic driven by configuration from devicetree, but like
>> most of those cases this is not the way we split things.
>>
>> And this has the side effect of keeping the dts files human readable,
>> human understandable and human maintainable.

right. That's why I recommend having clearly defined bindings.
	phy,tx-<param1> = <val, offset, mask>
	phy,tx-<param2> = <val, offset, mask>
	phy,tx-<param3> = <val, offset, mask>


[1] -> https://www.linuxplumbersconf.org/2016/ocw/proposals/4047

Thanks
Kishon

Re: [PATCH v4 2/4] phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips

[Kishon Vijay Abraham I]

Hi,

On Monday 23 January 2017 03:43 PM, Vivek Gautam wrote:
> On Wed, Jan 18, 2017 at 11:33 PM, Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
>> On Wed 18 Jan 01:13 PST 2017, Vivek Gautam wrote:
>>> On 01/16/2017 02:15 PM, Kishon Vijay Abraham I wrote:
>>>> Hi,
>>>>
>>>> On Tuesday 10 January 2017 04:21 PM, Vivek Gautam wrote:
>> [..]
>>>>> +static const struct qusb2_phy_init_tbl msm8996_init_tbl[] = {
>>>>> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE1, 0xf8),
>>>>> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE2, 0xb3),
>>>>> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE3, 0x83),
>>>>> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE4, 0xc0),
>>>>> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_TUNE, 0x30),
>>>>> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL1, 0x79),
>>>>> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL2, 0x21),
>>>>> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TEST2, 0x14),
>>>>> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_AUTOPGM_CTL1, 0x9f),
>>>>> + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_PWR_CTRL, 0x00),
>>>>> +};
>>>> I wish all this data comes from device tree and one API in phy-core can do all
>>>> these settings. Your other driver qcom-qmp also seems to have a bunch of
>>>> similar settings.
>>>>
>>>> The problem is every vendor driver adds a bunch of code to perform the same
>>>> thing again and again when all of these settings can be done by a single phy API.
>>>
>>> Yes, i understand this. You have commented similar thing in the patch from
>>> Jaehoon -
>>> [PATCH V2 2/5] phy: phy-exynos-pcie: Add support for Exynos PCIe phy
>>>
>>> I would like to understand the requirements here.
>>> Would you like me to get all this information from the device tree -
>>> an array of register offset and value pair, which we can then program
>>> by calling a phy_ops (may be calibrate) ? Something of this sort:
>>>
>>> phy-calibrate-data = <val1, register_offset1>,
>>>                                   <val2, register_offset2>,
>>>                                   <val3, register_offset3>,
>>>                                   ....
>>>
>>> I am sure having such information in the driver (like i have in my patch)
>>> makes the driver look more clumsy.
>>> But, all this data can be pretty huge - a set of some 100+ register-value
>>> pairs
>>> for QMP phy, for example. So, will it be okay to get this from device tree ?
>>> We also note here that such information changes from one IP version to
>>> another.
>>> I remember Rob having some concerns about it.
>>>
>>
>> The devicetree is supposed to describe which hardware components a
>> certain device has, most of the time this carries a set of properties to
>> describe how this piece is connected and configured.
>>
>> A dump of magic register values does not describe how the QMP is
>> connected to anything and is, as far as this patch shows, static for
>> this particular hardware block.

device tree has also been used to have configuration information. see [1]

> 
> Yes, that's correct. The QMP and QUSB2 phy init sequences are a bunch
> of static values for a particular IP version. These values hardly give a
> meaningful data to put few phy bindings that could be referenced
> to configure the phy further.

Not really. You can have clearly defined phy binding to give meaningful data.
Every driver doing the same configuration bloats the driver and these
configuration values are just magic values which hardly can be reviewed by anyone.
> 
>>
>> Further more moving this blob to devicetree will not allow us to treat
>> the various QMP configurations as one HW block, as there are other
>> differences as well.
>>
>> Like many other drivers it's possible to create a generic version that
>> has every bit of logic driven by configuration from devicetree, but like
>> most of those cases this is not the way we split things.
>>
>> And this has the side effect of keeping the dts files human readable,
>> human understandable and human maintainable.

right. That's why I recommend having clearly defined bindings.
	phy,tx-<param1> = <val, offset, mask>
	phy,tx-<param2> = <val, offset, mask>
	phy,tx-<param3> = <val, offset, mask>


[1] -> https://www.linuxplumbersconf.org/2016/ocw/proposals/4047

Thanks
Kishon

Re: [PATCH v4 2/4] phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips

[Bjorn Andersson]

On Tue 24 Jan 01:19 PST 2017, Kishon Vijay Abraham I wrote:
> On Monday 23 January 2017 03:43 PM, Vivek Gautam wrote:
> > On Wed, Jan 18, 2017 at 11:33 PM, Bjorn Andersson
[..]
> > 
> > Yes, that's correct. The QMP and QUSB2 phy init sequences are a bunch
> > of static values for a particular IP version. These values hardly give a
> > meaningful data to put few phy bindings that could be referenced
> > to configure the phy further.
> 
> Not really. You can have clearly defined phy binding to give meaningful data.
> Every driver doing the same configuration bloats the driver and these
> configuration values are just magic values which hardly can be reviewed by anyone.
> > 
> >>
> >> Further more moving this blob to devicetree will not allow us to treat
> >> the various QMP configurations as one HW block, as there are other
> >> differences as well.
> >>
> >> Like many other drivers it's possible to create a generic version that
> >> has every bit of logic driven by configuration from devicetree, but like
> >> most of those cases this is not the way we split things.
> >>
> >> And this has the side effect of keeping the dts files human readable,
> >> human understandable and human maintainable.
> 
> right. That's why I recommend having clearly defined bindings.
> 	phy,tx-<param1> = <val, offset, mask>
> 	phy,tx-<param2> = <val, offset, mask>
> 	phy,tx-<param3> = <val, offset, mask>

There's no doubt that this table needs to be encoded somewhere, so the
question is should we hard code this in a C file or in a DTSI file.

Skimming through [1] I see examples of things that differs based on how
the specific component is integrated in a SoC or on a particular board -
properties that are relevant to a "system integrator".

As far as I can tell this blob will, if ever, only be changed by a
driver developer and as such it's not carry information about how this
component relates to the rest of the system and should as such not be
part of the device tree.


If there are properties of the hardware that is affected by how the
component is integrated in the system I really would like for those to
be exposed as human-readable properties that I can understand and alter
without deep knowledge about the register map of the hardware block.

> [1] -> https://www.linuxplumbersconf.org/2016/ocw/proposals/4047

Regards,
Bjorn

Re: [PATCH v4 2/4] phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips

[Vivek Gautam]

On 01/26/2017 11:45 PM, Bjorn Andersson wrote:
> On Tue 24 Jan 01:19 PST 2017, Kishon Vijay Abraham I wrote:
>> On Monday 23 January 2017 03:43 PM, Vivek Gautam wrote:
>>> On Wed, Jan 18, 2017 at 11:33 PM, Bjorn Andersson
> [..]
>>> Yes, that's correct. The QMP and QUSB2 phy init sequences are a bunch
>>> of static values for a particular IP version. These values hardly give a
>>> meaningful data to put few phy bindings that could be referenced
>>> to configure the phy further.
>> Not really. You can have clearly defined phy binding to give meaningful data.
>> Every driver doing the same configuration bloats the driver and these
>> configuration values are just magic values which hardly can be reviewed by anyone.
>>>> Further more moving this blob to devicetree will not allow us to treat
>>>> the various QMP configurations as one HW block, as there are other
>>>> differences as well.
>>>>
>>>> Like many other drivers it's possible to create a generic version that
>>>> has every bit of logic driven by configuration from devicetree, but like
>>>> most of those cases this is not the way we split things.
>>>>
>>>> And this has the side effect of keeping the dts files human readable,
>>>> human understandable and human maintainable.
>> right. That's why I recommend having clearly defined bindings.
>> 	phy,tx-<param1> = <val, offset, mask>
>> 	phy,tx-<param2> = <val, offset, mask>
>> 	phy,tx-<param3> = <val, offset, mask>
> There's no doubt that this table needs to be encoded somewhere, so the
> question is should we hard code this in a C file or in a DTSI file.
>
> Skimming through [1] I see examples of things that differs based on how
> the specific component is integrated in a SoC or on a particular board -
> properties that are relevant to a "system integrator".
>
> As far as I can tell this blob will, if ever, only be changed by a
> driver developer and as such it's not carry information about how this
> component relates to the rest of the system and should as such not be
> part of the device tree.
>
>
> If there are properties of the hardware that is affected by how the
> component is integrated in the system I really would like for those to
> be exposed as human-readable properties that I can understand and alter
> without deep knowledge about the register map of the hardware block.

I am reaching out to our internal teams to get more information
on different possible phy configurations, based on which the registers
values are decided.
This is something that i tried to understand in the past as well, but 
couldn't
grab much information that time.
Will come back with relevant information on this.


Regards
Vivek

>> [1] -> https://www.linuxplumbersconf.org/2016/ocw/proposals/4047
> Regards,
> Bjorn
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v4 2/4] phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips

[Vivek Gautam]

Hi Kishon,


On Fri, Jan 27, 2017 at 11:54 AM, Vivek Gautam
<vivek.gautam@codeaurora.org> wrote:
>
>
> On 01/26/2017 11:45 PM, Bjorn Andersson wrote:
>>
>> On Tue 24 Jan 01:19 PST 2017, Kishon Vijay Abraham I wrote:
>>>
>>> On Monday 23 January 2017 03:43 PM, Vivek Gautam wrote:
>>>>
>>>> On Wed, Jan 18, 2017 at 11:33 PM, Bjorn Andersson
>>
>> [..]
>>>>
>>>> Yes, that's correct. The QMP and QUSB2 phy init sequences are a bunch
>>>> of static values for a particular IP version. These values hardly give a
>>>> meaningful data to put few phy bindings that could be referenced
>>>> to configure the phy further.
>>>
>>> Not really. You can have clearly defined phy binding to give meaningful
>>> data.
>>> Every driver doing the same configuration bloats the driver and these
>>> configuration values are just magic values which hardly can be reviewed
>>> by anyone.
>>>>>
>>>>> Further more moving this blob to devicetree will not allow us to treat
>>>>> the various QMP configurations as one HW block, as there are other
>>>>> differences as well.
>>>>>
>>>>> Like many other drivers it's possible to create a generic version that
>>>>> has every bit of logic driven by configuration from devicetree, but
>>>>> like
>>>>> most of those cases this is not the way we split things.
>>>>>
>>>>> And this has the side effect of keeping the dts files human readable,
>>>>> human understandable and human maintainable.
>>>
>>> right. That's why I recommend having clearly defined bindings.
>>>         phy,tx-<param1> = <val, offset, mask>
>>>         phy,tx-<param2> = <val, offset, mask>
>>>         phy,tx-<param3> = <val, offset, mask>
>>
>> There's no doubt that this table needs to be encoded somewhere, so the
>> question is should we hard code this in a C file or in a DTSI file.
>>
>> Skimming through [1] I see examples of things that differs based on how
>> the specific component is integrated in a SoC or on a particular board -
>> properties that are relevant to a "system integrator".
>>
>> As far as I can tell this blob will, if ever, only be changed by a
>> driver developer and as such it's not carry information about how this
>> component relates to the rest of the system and should as such not be
>> part of the device tree.
>>
>>
>> If there are properties of the hardware that is affected by how the
>> component is integrated in the system I really would like for those to
>> be exposed as human-readable properties that I can understand and alter
>> without deep knowledge about the register map of the hardware block.
>
>
> I am reaching out to our internal teams to get more information
> on different possible phy configurations, based on which the registers
> values are decided.
> This is something that i tried to understand in the past as well, but
> couldn't
> grab much information that time.
> Will come back with relevant information on this.
>

We have started looking into understanding the PHYs on msm and
eventually create a set of generic phy bindings that can serve multiple
platforms.
But this task, I presume, will take its course and will involve multi-party
discussions.

For these QUSB2 and QMP phy drivers, a good amount of work has
already gone in getting these drivers in upstream state.
The common QMP phy driver supports a bunch of controllers on msm
platforms - USB, PCIe and UFS and there are platforms such as DB820c
and others that want to pull in these changes from upstream.
The future phy controllers also depend on these drivers and we don't want
to hold other developers to contribute to these drivers.
So, we wish to not delay these drivers further because of the phy bindings.
I see that there are phys that still program the registers-value pairs for
phy initialization.

We will keep working on the bindings while these patches
make way to upstream.

Can you consider pulling in these drivers?
I can send the next version of these drivers with other comments addressed.
Please let me know your comments.


Regards
Vivek

>
> Regards
> Vivek
>
>>> [1] -> https://www.linuxplumbersconf.org/2016/ocw/proposals/4047
>>
>> Regards,
>> Bjorn
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm"
>> in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
> --
> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v4 2/4] phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips

[Vivek Gautam]

Hi Kishon,


On Wed, Feb 22, 2017 at 9:29 AM, Vivek Gautam
<vivek.gautam@codeaurora.org> wrote:
> Hi Kishon,
>
>
> On Fri, Jan 27, 2017 at 11:54 AM, Vivek Gautam
> <vivek.gautam@codeaurora.org> wrote:
>>
>>
>> On 01/26/2017 11:45 PM, Bjorn Andersson wrote:
>>>
>>> On Tue 24 Jan 01:19 PST 2017, Kishon Vijay Abraham I wrote:
>>>>
>>>> On Monday 23 January 2017 03:43 PM, Vivek Gautam wrote:
>>>>>
>>>>> On Wed, Jan 18, 2017 at 11:33 PM, Bjorn Andersson
>>>
>>> [..]
>>>>>
>>>>> Yes, that's correct. The QMP and QUSB2 phy init sequences are a bunch
>>>>> of static values for a particular IP version. These values hardly give a
>>>>> meaningful data to put few phy bindings that could be referenced
>>>>> to configure the phy further.
>>>>
>>>> Not really. You can have clearly defined phy binding to give meaningful
>>>> data.
>>>> Every driver doing the same configuration bloats the driver and these
>>>> configuration values are just magic values which hardly can be reviewed
>>>> by anyone.
>>>>>>
>>>>>> Further more moving this blob to devicetree will not allow us to treat
>>>>>> the various QMP configurations as one HW block, as there are other
>>>>>> differences as well.
>>>>>>
>>>>>> Like many other drivers it's possible to create a generic version that
>>>>>> has every bit of logic driven by configuration from devicetree, but
>>>>>> like
>>>>>> most of those cases this is not the way we split things.
>>>>>>
>>>>>> And this has the side effect of keeping the dts files human readable,
>>>>>> human understandable and human maintainable.
>>>>
>>>> right. That's why I recommend having clearly defined bindings.
>>>>         phy,tx-<param1> = <val, offset, mask>
>>>>         phy,tx-<param2> = <val, offset, mask>
>>>>         phy,tx-<param3> = <val, offset, mask>
>>>
>>> There's no doubt that this table needs to be encoded somewhere, so the
>>> question is should we hard code this in a C file or in a DTSI file.
>>>
>>> Skimming through [1] I see examples of things that differs based on how
>>> the specific component is integrated in a SoC or on a particular board -
>>> properties that are relevant to a "system integrator".
>>>
>>> As far as I can tell this blob will, if ever, only be changed by a
>>> driver developer and as such it's not carry information about how this
>>> component relates to the rest of the system and should as such not be
>>> part of the device tree.
>>>
>>>
>>> If there are properties of the hardware that is affected by how the
>>> component is integrated in the system I really would like for those to
>>> be exposed as human-readable properties that I can understand and alter
>>> without deep knowledge about the register map of the hardware block.
>>
>>
>> I am reaching out to our internal teams to get more information
>> on different possible phy configurations, based on which the registers
>> values are decided.
>> This is something that i tried to understand in the past as well, but
>> couldn't
>> grab much information that time.
>> Will come back with relevant information on this.
>>
>
> We have started looking into understanding the PHYs on msm and
> eventually create a set of generic phy bindings that can serve multiple
> platforms.
> But this task, I presume, will take its course and will involve multi-party
> discussions.
>
> For these QUSB2 and QMP phy drivers, a good amount of work has
> already gone in getting these drivers in upstream state.
> The common QMP phy driver supports a bunch of controllers on msm
> platforms - USB, PCIe and UFS and there are platforms such as DB820c
> and others that want to pull in these changes from upstream.
> The future phy controllers also depend on these drivers and we don't want
> to hold other developers to contribute to these drivers.
> So, we wish to not delay these drivers further because of the phy bindings.
> I see that there are phys that still program the registers-value pairs for
> phy initialization.
>
> We will keep working on the bindings while these patches
> make way to upstream.
>
> Can you consider pulling in these drivers?
> I can send the next version of these drivers with other comments addressed.
> Please let me know your comments.

Gentle ping. Any thoughts on this ?

>>
>>>> [1] -> https://www.linuxplumbersconf.org/2016/ocw/proposals/4047
>>>
>>> Regards,
>>> Bjorn


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v4 2/4] phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips

[Kishon Vijay Abraham I]

Hi,

On Thursday 02 March 2017 10:10 PM, Vivek Gautam wrote:
> Hi Kishon,
> 
> 
> On Wed, Feb 22, 2017 at 9:29 AM, Vivek Gautam
> <vivek.gautam@codeaurora.org> wrote:
>> Hi Kishon,
>>
>>
>> On Fri, Jan 27, 2017 at 11:54 AM, Vivek Gautam
>> <vivek.gautam@codeaurora.org> wrote:
>>>
>>>
>>> On 01/26/2017 11:45 PM, Bjorn Andersson wrote:
>>>>
>>>> On Tue 24 Jan 01:19 PST 2017, Kishon Vijay Abraham I wrote:
>>>>>
>>>>> On Monday 23 January 2017 03:43 PM, Vivek Gautam wrote:
>>>>>>
>>>>>> On Wed, Jan 18, 2017 at 11:33 PM, Bjorn Andersson
>>>>
>>>> [..]
>>>>>>
>>>>>> Yes, that's correct. The QMP and QUSB2 phy init sequences are a bunch
>>>>>> of static values for a particular IP version. These values hardly give a
>>>>>> meaningful data to put few phy bindings that could be referenced
>>>>>> to configure the phy further.
>>>>>
>>>>> Not really. You can have clearly defined phy binding to give meaningful
>>>>> data.
>>>>> Every driver doing the same configuration bloats the driver and these
>>>>> configuration values are just magic values which hardly can be reviewed
>>>>> by anyone.
>>>>>>>
>>>>>>> Further more moving this blob to devicetree will not allow us to treat
>>>>>>> the various QMP configurations as one HW block, as there are other
>>>>>>> differences as well.
>>>>>>>
>>>>>>> Like many other drivers it's possible to create a generic version that
>>>>>>> has every bit of logic driven by configuration from devicetree, but
>>>>>>> like
>>>>>>> most of those cases this is not the way we split things.
>>>>>>>
>>>>>>> And this has the side effect of keeping the dts files human readable,
>>>>>>> human understandable and human maintainable.
>>>>>
>>>>> right. That's why I recommend having clearly defined bindings.
>>>>>         phy,tx-<param1> = <val, offset, mask>
>>>>>         phy,tx-<param2> = <val, offset, mask>
>>>>>         phy,tx-<param3> = <val, offset, mask>
>>>>
>>>> There's no doubt that this table needs to be encoded somewhere, so the
>>>> question is should we hard code this in a C file or in a DTSI file.
>>>>
>>>> Skimming through [1] I see examples of things that differs based on how
>>>> the specific component is integrated in a SoC or on a particular board -
>>>> properties that are relevant to a "system integrator".
>>>>
>>>> As far as I can tell this blob will, if ever, only be changed by a
>>>> driver developer and as such it's not carry information about how this
>>>> component relates to the rest of the system and should as such not be
>>>> part of the device tree.
>>>>
>>>>
>>>> If there are properties of the hardware that is affected by how the
>>>> component is integrated in the system I really would like for those to
>>>> be exposed as human-readable properties that I can understand and alter
>>>> without deep knowledge about the register map of the hardware block.
>>>
>>>
>>> I am reaching out to our internal teams to get more information
>>> on different possible phy configurations, based on which the registers
>>> values are decided.
>>> This is something that i tried to understand in the past as well, but
>>> couldn't
>>> grab much information that time.
>>> Will come back with relevant information on this.
>>>
>>
>> We have started looking into understanding the PHYs on msm and
>> eventually create a set of generic phy bindings that can serve multiple
>> platforms.
>> But this task, I presume, will take its course and will involve multi-party
>> discussions.
>>
>> For these QUSB2 and QMP phy drivers, a good amount of work has
>> already gone in getting these drivers in upstream state.
>> The common QMP phy driver supports a bunch of controllers on msm
>> platforms - USB, PCIe and UFS and there are platforms such as DB820c
>> and others that want to pull in these changes from upstream.
>> The future phy controllers also depend on these drivers and we don't want
>> to hold other developers to contribute to these drivers.
>> So, we wish to not delay these drivers further because of the phy bindings.
>> I see that there are phys that still program the registers-value pairs for
>> phy initialization.
>>
>> We will keep working on the bindings while these patches
>> make way to upstream.
>>
>> Can you consider pulling in these drivers?
>> I can send the next version of these drivers with other comments addressed.
>> Please let me know your comments.
> 
> Gentle ping. Any thoughts on this ?

sure, lets hold the phy configuration binding and complete the rest of the patch.

Thanks
Kishon

Re: [PATCH v4 2/4] phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips

[Kishon Vijay Abraham I]

Hi,

On Thursday 02 March 2017 10:10 PM, Vivek Gautam wrote:
> Hi Kishon,
> 
> 
> On Wed, Feb 22, 2017 at 9:29 AM, Vivek Gautam
> <vivek.gautam@codeaurora.org> wrote:
>> Hi Kishon,
>>
>>
>> On Fri, Jan 27, 2017 at 11:54 AM, Vivek Gautam
>> <vivek.gautam@codeaurora.org> wrote:
>>>
>>>
>>> On 01/26/2017 11:45 PM, Bjorn Andersson wrote:
>>>>
>>>> On Tue 24 Jan 01:19 PST 2017, Kishon Vijay Abraham I wrote:
>>>>>
>>>>> On Monday 23 January 2017 03:43 PM, Vivek Gautam wrote:
>>>>>>
>>>>>> On Wed, Jan 18, 2017 at 11:33 PM, Bjorn Andersson
>>>>
>>>> [..]
>>>>>>
>>>>>> Yes, that's correct. The QMP and QUSB2 phy init sequences are a bunch
>>>>>> of static values for a particular IP version. These values hardly give a
>>>>>> meaningful data to put few phy bindings that could be referenced
>>>>>> to configure the phy further.
>>>>>
>>>>> Not really. You can have clearly defined phy binding to give meaningful
>>>>> data.
>>>>> Every driver doing the same configuration bloats the driver and these
>>>>> configuration values are just magic values which hardly can be reviewed
>>>>> by anyone.
>>>>>>>
>>>>>>> Further more moving this blob to devicetree will not allow us to treat
>>>>>>> the various QMP configurations as one HW block, as there are other
>>>>>>> differences as well.
>>>>>>>
>>>>>>> Like many other drivers it's possible to create a generic version that
>>>>>>> has every bit of logic driven by configuration from devicetree, but
>>>>>>> like
>>>>>>> most of those cases this is not the way we split things.
>>>>>>>
>>>>>>> And this has the side effect of keeping the dts files human readable,
>>>>>>> human understandable and human maintainable.
>>>>>
>>>>> right. That's why I recommend having clearly defined bindings.
>>>>>         phy,tx-<param1> = <val, offset, mask>
>>>>>         phy,tx-<param2> = <val, offset, mask>
>>>>>         phy,tx-<param3> = <val, offset, mask>
>>>>
>>>> There's no doubt that this table needs to be encoded somewhere, so the
>>>> question is should we hard code this in a C file or in a DTSI file.
>>>>
>>>> Skimming through [1] I see examples of things that differs based on how
>>>> the specific component is integrated in a SoC or on a particular board -
>>>> properties that are relevant to a "system integrator".
>>>>
>>>> As far as I can tell this blob will, if ever, only be changed by a
>>>> driver developer and as such it's not carry information about how this
>>>> component relates to the rest of the system and should as such not be
>>>> part of the device tree.
>>>>
>>>>
>>>> If there are properties of the hardware that is affected by how the
>>>> component is integrated in the system I really would like for those to
>>>> be exposed as human-readable properties that I can understand and alter
>>>> without deep knowledge about the register map of the hardware block.
>>>
>>>
>>> I am reaching out to our internal teams to get more information
>>> on different possible phy configurations, based on which the registers
>>> values are decided.
>>> This is something that i tried to understand in the past as well, but
>>> couldn't
>>> grab much information that time.
>>> Will come back with relevant information on this.
>>>
>>
>> We have started looking into understanding the PHYs on msm and
>> eventually create a set of generic phy bindings that can serve multiple
>> platforms.
>> But this task, I presume, will take its course and will involve multi-party
>> discussions.
>>
>> For these QUSB2 and QMP phy drivers, a good amount of work has
>> already gone in getting these drivers in upstream state.
>> The common QMP phy driver supports a bunch of controllers on msm
>> platforms - USB, PCIe and UFS and there are platforms such as DB820c
>> and others that want to pull in these changes from upstream.
>> The future phy controllers also depend on these drivers and we don't want
>> to hold other developers to contribute to these drivers.
>> So, we wish to not delay these drivers further because of the phy bindings.
>> I see that there are phys that still program the registers-value pairs for
>> phy initialization.
>>
>> We will keep working on the bindings while these patches
>> make way to upstream.
>>
>> Can you consider pulling in these drivers?
>> I can send the next version of these drivers with other comments addressed.
>> Please let me know your comments.
> 
> Gentle ping. Any thoughts on this ?

sure, lets hold the phy configuration binding and complete the rest of the patch.

Thanks
Kishon

Re: [PATCH v4 2/4] phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips

[Vivek Gautam]

Hi,


On 03/07/2017 02:34 PM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Thursday 02 March 2017 10:10 PM, Vivek Gautam wrote:
>> Hi Kishon,
>>
>>
>> On Wed, Feb 22, 2017 at 9:29 AM, Vivek Gautam
>> <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>>> Hi Kishon,
>>>
>>>
>>> On Fri, Jan 27, 2017 at 11:54 AM, Vivek Gautam
>>> <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>>>>
>>>> On 01/26/2017 11:45 PM, Bjorn Andersson wrote:
>>>>> On Tue 24 Jan 01:19 PST 2017, Kishon Vijay Abraham I wrote:
>>>>>> On Monday 23 January 2017 03:43 PM, Vivek Gautam wrote:
>>>>>>> On Wed, Jan 18, 2017 at 11:33 PM, Bjorn Andersson
>>>>> [..]
>>>>>>> Yes, that's correct. The QMP and QUSB2 phy init sequences are a bunch
>>>>>>> of static values for a particular IP version. These values hardly give a
>>>>>>> meaningful data to put few phy bindings that could be referenced
>>>>>>> to configure the phy further.
>>>>>> Not really. You can have clearly defined phy binding to give meaningful
>>>>>> data.
>>>>>> Every driver doing the same configuration bloats the driver and these
>>>>>> configuration values are just magic values which hardly can be reviewed
>>>>>> by anyone.
>>>>>>>> Further more moving this blob to devicetree will not allow us to treat
>>>>>>>> the various QMP configurations as one HW block, as there are other
>>>>>>>> differences as well.
>>>>>>>>
>>>>>>>> Like many other drivers it's possible to create a generic version that
>>>>>>>> has every bit of logic driven by configuration from devicetree, but
>>>>>>>> like
>>>>>>>> most of those cases this is not the way we split things.
>>>>>>>>
>>>>>>>> And this has the side effect of keeping the dts files human readable,
>>>>>>>> human understandable and human maintainable.
>>>>>> right. That's why I recommend having clearly defined bindings.
>>>>>>          phy,tx-<param1> = <val, offset, mask>
>>>>>>          phy,tx-<param2> = <val, offset, mask>
>>>>>>          phy,tx-<param3> = <val, offset, mask>
>>>>> There's no doubt that this table needs to be encoded somewhere, so the
>>>>> question is should we hard code this in a C file or in a DTSI file.
>>>>>
>>>>> Skimming through [1] I see examples of things that differs based on how
>>>>> the specific component is integrated in a SoC or on a particular board -
>>>>> properties that are relevant to a "system integrator".
>>>>>
>>>>> As far as I can tell this blob will, if ever, only be changed by a
>>>>> driver developer and as such it's not carry information about how this
>>>>> component relates to the rest of the system and should as such not be
>>>>> part of the device tree.
>>>>>
>>>>>
>>>>> If there are properties of the hardware that is affected by how the
>>>>> component is integrated in the system I really would like for those to
>>>>> be exposed as human-readable properties that I can understand and alter
>>>>> without deep knowledge about the register map of the hardware block.
>>>>
>>>> I am reaching out to our internal teams to get more information
>>>> on different possible phy configurations, based on which the registers
>>>> values are decided.
>>>> This is something that i tried to understand in the past as well, but
>>>> couldn't
>>>> grab much information that time.
>>>> Will come back with relevant information on this.
>>>>
>>> We have started looking into understanding the PHYs on msm and
>>> eventually create a set of generic phy bindings that can serve multiple
>>> platforms.
>>> But this task, I presume, will take its course and will involve multi-party
>>> discussions.
>>>
>>> For these QUSB2 and QMP phy drivers, a good amount of work has
>>> already gone in getting these drivers in upstream state.
>>> The common QMP phy driver supports a bunch of controllers on msm
>>> platforms - USB, PCIe and UFS and there are platforms such as DB820c
>>> and others that want to pull in these changes from upstream.
>>> The future phy controllers also depend on these drivers and we don't want
>>> to hold other developers to contribute to these drivers.
>>> So, we wish to not delay these drivers further because of the phy bindings.
>>> I see that there are phys that still program the registers-value pairs for
>>> phy initialization.
>>>
>>> We will keep working on the bindings while these patches
>>> make way to upstream.
>>>
>>> Can you consider pulling in these drivers?
>>> I can send the next version of these drivers with other comments addressed.
>>> Please let me know your comments.
>> Gentle ping. Any thoughts on this ?
> sure, lets hold the phy configuration binding and complete the rest of the patch.

Sure, thanks. Will post out the next version of patches soon.

Regards
Vivek

>
> Thanks
> Kishon

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/4] phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips

[Vivek Gautam]

Hi,


On 03/07/2017 02:34 PM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Thursday 02 March 2017 10:10 PM, Vivek Gautam wrote:
>> Hi Kishon,
>>
>>
>> On Wed, Feb 22, 2017 at 9:29 AM, Vivek Gautam
>> <vivek.gautam@codeaurora.org> wrote:
>>> Hi Kishon,
>>>
>>>
>>> On Fri, Jan 27, 2017 at 11:54 AM, Vivek Gautam
>>> <vivek.gautam@codeaurora.org> wrote:
>>>>
>>>> On 01/26/2017 11:45 PM, Bjorn Andersson wrote:
>>>>> On Tue 24 Jan 01:19 PST 2017, Kishon Vijay Abraham I wrote:
>>>>>> On Monday 23 January 2017 03:43 PM, Vivek Gautam wrote:
>>>>>>> On Wed, Jan 18, 2017 at 11:33 PM, Bjorn Andersson
>>>>> [..]
>>>>>>> Yes, that's correct. The QMP and QUSB2 phy init sequences are a bunch
>>>>>>> of static values for a particular IP version. These values hardly give a
>>>>>>> meaningful data to put few phy bindings that could be referenced
>>>>>>> to configure the phy further.
>>>>>> Not really. You can have clearly defined phy binding to give meaningful
>>>>>> data.
>>>>>> Every driver doing the same configuration bloats the driver and these
>>>>>> configuration values are just magic values which hardly can be reviewed
>>>>>> by anyone.
>>>>>>>> Further more moving this blob to devicetree will not allow us to treat
>>>>>>>> the various QMP configurations as one HW block, as there are other
>>>>>>>> differences as well.
>>>>>>>>
>>>>>>>> Like many other drivers it's possible to create a generic version that
>>>>>>>> has every bit of logic driven by configuration from devicetree, but
>>>>>>>> like
>>>>>>>> most of those cases this is not the way we split things.
>>>>>>>>
>>>>>>>> And this has the side effect of keeping the dts files human readable,
>>>>>>>> human understandable and human maintainable.
>>>>>> right. That's why I recommend having clearly defined bindings.
>>>>>>          phy,tx-<param1> = <val, offset, mask>
>>>>>>          phy,tx-<param2> = <val, offset, mask>
>>>>>>          phy,tx-<param3> = <val, offset, mask>
>>>>> There's no doubt that this table needs to be encoded somewhere, so the
>>>>> question is should we hard code this in a C file or in a DTSI file.
>>>>>
>>>>> Skimming through [1] I see examples of things that differs based on how
>>>>> the specific component is integrated in a SoC or on a particular board -
>>>>> properties that are relevant to a "system integrator".
>>>>>
>>>>> As far as I can tell this blob will, if ever, only be changed by a
>>>>> driver developer and as such it's not carry information about how this
>>>>> component relates to the rest of the system and should as such not be
>>>>> part of the device tree.
>>>>>
>>>>>
>>>>> If there are properties of the hardware that is affected by how the
>>>>> component is integrated in the system I really would like for those to
>>>>> be exposed as human-readable properties that I can understand and alter
>>>>> without deep knowledge about the register map of the hardware block.
>>>>
>>>> I am reaching out to our internal teams to get more information
>>>> on different possible phy configurations, based on which the registers
>>>> values are decided.
>>>> This is something that i tried to understand in the past as well, but
>>>> couldn't
>>>> grab much information that time.
>>>> Will come back with relevant information on this.
>>>>
>>> We have started looking into understanding the PHYs on msm and
>>> eventually create a set of generic phy bindings that can serve multiple
>>> platforms.
>>> But this task, I presume, will take its course and will involve multi-party
>>> discussions.
>>>
>>> For these QUSB2 and QMP phy drivers, a good amount of work has
>>> already gone in getting these drivers in upstream state.
>>> The common QMP phy driver supports a bunch of controllers on msm
>>> platforms - USB, PCIe and UFS and there are platforms such as DB820c
>>> and others that want to pull in these changes from upstream.
>>> The future phy controllers also depend on these drivers and we don't want
>>> to hold other developers to contribute to these drivers.
>>> So, we wish to not delay these drivers further because of the phy bindings.
>>> I see that there are phys that still program the registers-value pairs for
>>> phy initialization.
>>>
>>> We will keep working on the bindings while these patches
>>> make way to upstream.
>>>
>>> Can you consider pulling in these drivers?
>>> I can send the next version of these drivers with other comments addressed.
>>> Please let me know your comments.
>> Gentle ping. Any thoughts on this ?
> sure, lets hold the phy configuration binding and complete the rest of the patch.

Sure, thanks. Will post out the next version of patches soon.

Regards
Vivek

>
> Thanks
> Kishon

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v4 3/4] dt-bindings: phy: Add support for QMP phy

[Vivek Gautam]

Qualcomm chipsets have QMP phy controller that provides
support to a number of controller, viz. PCIe, UFS, and USB.
Adding dt binding information for the same.

Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
Acked-by: Rob Herring <robh@kernel.org>
---

Changes since v3:
 - Added #clock-cells = <1>, indicating that phy is a clock provider.

Changes since v2:
 - Removed binding for "ref_clk_src" since we don't request this
   clock in the driver.
 - Addressed s/ref_clk/ref. Don't need to add '_clk' suffix to clock names.
 - Using 'phy' for the node name.

Changes since v1:
 - New patch, forked out of the original driver patch:
   "phy: qcom-qmp: new qmp phy driver for qcom-chipsets"
 - Added 'Acked-by' from Rob.
 - Updated bindings to include mem resource as a list of
   offset - length pair for serdes block and for each lane.
 - Added a new binding for 'lane-offsets' that contains offsets
   to tx, rx and pcs blocks from each lane base address.

 .../devicetree/bindings/phy/qcom-qmp-phy.txt       | 76 ++++++++++++++++++++++
 1 file changed, 76 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt

diff --git a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
new file mode 100644
index 000000000000..6f510fe48f46
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
@@ -0,0 +1,76 @@
+Qualcomm QMP PHY controller
+===========================
+
+QMP phy controller supports physical layer functionality for a number of
+controllers on Qualcomm chipsets, such as, PCIe, UFS, and USB.
+
+Required properties:
+ - compatible: compatible list, contains:
+	       "qcom,msm8996-qmp-pcie-phy" for 14nm PCIe phy on msm8996,
+	       "qcom,msm8996-qmp-usb3-phy" for 14nm USB3 phy on msm8996.
+ - reg: list of offset and length pair of the PHY register sets.
+	at index 0: offset and length of register set for PHY common
+		    serdes block.
+	from index 1 - N: offset and length of register set for each lane,
+			  for N number of phy lanes (ports).
+ - lane-offsets: array of offsets to tx, rx and pcs blocks for phy lanes.
+ - #phy-cells: must be 1
+    - Cell after phy phandle should be the port (lane) number.
+ - #clock-cells: must be 1
+    - Phy pll outputs a bunch of clocks for Tx, Rx and Pipe
+      interface (for pipe based PHYs). These clock are then gate-controlled
+      by gcc.
+ - clocks: a list of phandles and clock-specifier pairs,
+	   one for each entry in clock-names.
+ - clock-names: must be "cfg_ahb" for phy config clock,
+			"aux" for phy aux clock,
+			"ref" for 19.2 MHz ref clk,
+			"pipe<port-number>" for pipe clock specific to
+			each port/lane (Optional).
+ - resets: a list of phandles and reset controller specifier pairs,
+	   one for each entry in reset-names.
+ - reset-names: must be "phy" for reset of phy block,
+			"common" for phy common block reset,
+			"cfg" for phy's ahb cfg block reset (Optional).
+			"port<port-number>" for reset specific to
+			each port/lane (Optional).
+ - vdda-phy-supply: Phandle to a regulator supply to PHY core block.
+ - vdda-pll-supply: Phandle to 1.8V regulator supply to PHY refclk pll block.
+
+Optional properties:
+ - vddp-ref-clk-supply: Phandle to a regulator supply to any specific refclk
+			pll block.
+
+Example:
+	pcie_phy: phy@34000 {
+		compatible = "qcom,msm8996-qmp-pcie-phy";
+		reg = <0x034000 0x48f>,
+			<0x035000 0x5bf>,
+			<0x036000 0x5bf>,
+			<0x037000 0x5bf>;
+				/* tx, rx, pcs */
+		lane-offsets = <0x0 0x200 0x400>;
+		#phy-cells = <1>;
+		#clock-cells = <1>;
+
+		clocks = <&gcc GCC_PCIE_PHY_AUX_CLK>,
+			<&gcc GCC_PCIE_PHY_CFG_AHB_CLK>,
+			<&gcc GCC_PCIE_CLKREF_CLK>,
+			<&gcc GCC_PCIE_0_PIPE_CLK>,
+			<&gcc GCC_PCIE_1_PIPE_CLK>,
+			<&gcc GCC_PCIE_2_PIPE_CLK>;
+		clock-names = "aux", "cfg_ahb", "ref",
+				"pipe0", "pipe1", "pipe2";
+
+		vdda-phy-supply = <&pm8994_l28>;
+		vdda-pll-supply = <&pm8994_l12>;
+
+		resets = <&gcc GCC_PCIE_PHY_BCR>,
+			<&gcc GCC_PCIE_PHY_COM_BCR>,
+			<&gcc GCC_PCIE_PHY_COM_NOCSR_BCR>,
+			<&gcc GCC_PCIE_0_PHY_BCR>,
+			<&gcc GCC_PCIE_1_PHY_BCR>,
+			<&gcc GCC_PCIE_2_PHY_BCR>;
+		reset-names = "phy", "common", "cfg",
+				"lane0", "lane1", "lane2";
+	};
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v4 3/4] dt-bindings: phy: Add support for QMP phy

[Kishon Vijay Abraham I]

Hi,

On Tuesday 10 January 2017 04:21 PM, Vivek Gautam wrote:
> Qualcomm chipsets have QMP phy controller that provides
> support to a number of controller, viz. PCIe, UFS, and USB.
> Adding dt binding information for the same.
> 
> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
> 
> Changes since v3:
>  - Added #clock-cells = <1>, indicating that phy is a clock provider.
> 
> Changes since v2:
>  - Removed binding for "ref_clk_src" since we don't request this
>    clock in the driver.
>  - Addressed s/ref_clk/ref. Don't need to add '_clk' suffix to clock names.
>  - Using 'phy' for the node name.
> 
> Changes since v1:
>  - New patch, forked out of the original driver patch:
>    "phy: qcom-qmp: new qmp phy driver for qcom-chipsets"
>  - Added 'Acked-by' from Rob.
>  - Updated bindings to include mem resource as a list of
>    offset - length pair for serdes block and for each lane.
>  - Added a new binding for 'lane-offsets' that contains offsets
>    to tx, rx and pcs blocks from each lane base address.
> 
>  .../devicetree/bindings/phy/qcom-qmp-phy.txt       | 76 ++++++++++++++++++++++
>  1 file changed, 76 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> new file mode 100644
> index 000000000000..6f510fe48f46
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> @@ -0,0 +1,76 @@
> +Qualcomm QMP PHY controller
> +===========================
> +
> +QMP phy controller supports physical layer functionality for a number of
> +controllers on Qualcomm chipsets, such as, PCIe, UFS, and USB.
> +
> +Required properties:
> + - compatible: compatible list, contains:
> +	       "qcom,msm8996-qmp-pcie-phy" for 14nm PCIe phy on msm8996,
> +	       "qcom,msm8996-qmp-usb3-phy" for 14nm USB3 phy on msm8996.
> + - reg: list of offset and length pair of the PHY register sets.
> +	at index 0: offset and length of register set for PHY common
> +		    serdes block.
> +	from index 1 - N: offset and length of register set for each lane,
> +			  for N number of phy lanes (ports).
> + - lane-offsets: array of offsets to tx, rx and pcs blocks for phy lanes.
> + - #phy-cells: must be 1
> +    - Cell after phy phandle should be the port (lane) number.
> + - #clock-cells: must be 1
> +    - Phy pll outputs a bunch of clocks for Tx, Rx and Pipe
> +      interface (for pipe based PHYs). These clock are then gate-controlled
> +      by gcc.
> + - clocks: a list of phandles and clock-specifier pairs,
> +	   one for each entry in clock-names.
> + - clock-names: must be "cfg_ahb" for phy config clock,
> +			"aux" for phy aux clock,
> +			"ref" for 19.2 MHz ref clk,
> +			"pipe<port-number>" for pipe clock specific to
> +			each port/lane (Optional).
> + - resets: a list of phandles and reset controller specifier pairs,
> +	   one for each entry in reset-names.
> + - reset-names: must be "phy" for reset of phy block,
> +			"common" for phy common block reset,
> +			"cfg" for phy's ahb cfg block reset (Optional).
> +			"port<port-number>" for reset specific to
> +			each port/lane (Optional).
> + - vdda-phy-supply: Phandle to a regulator supply to PHY core block.
> + - vdda-pll-supply: Phandle to 1.8V regulator supply to PHY refclk pll block.
> +
> +Optional properties:
> + - vddp-ref-clk-supply: Phandle to a regulator supply to any specific refclk
> +			pll block.
> +
> +Example:
> +	pcie_phy: phy@34000 {
> +		compatible = "qcom,msm8996-qmp-pcie-phy";
> +		reg = <0x034000 0x48f>,
> +			<0x035000 0x5bf>,
> +			<0x036000 0x5bf>,
> +			<0x037000 0x5bf>;
> +				/* tx, rx, pcs */
> +		lane-offsets = <0x0 0x200 0x400>;
> +		#phy-cells = <1>;
> +		#clock-cells = <1>;
> +
> +		clocks = <&gcc GCC_PCIE_PHY_AUX_CLK>,
> +			<&gcc GCC_PCIE_PHY_CFG_AHB_CLK>,
> +			<&gcc GCC_PCIE_CLKREF_CLK>,
> +			<&gcc GCC_PCIE_0_PIPE_CLK>,
> +			<&gcc GCC_PCIE_1_PIPE_CLK>,
> +			<&gcc GCC_PCIE_2_PIPE_CLK>;
> +		clock-names = "aux", "cfg_ahb", "ref",
> +				"pipe0", "pipe1", "pipe2";
> +
> +		vdda-phy-supply = <&pm8994_l28>;
> +		vdda-pll-supply = <&pm8994_l12>;
> +
> +		resets = <&gcc GCC_PCIE_PHY_BCR>,
> +			<&gcc GCC_PCIE_PHY_COM_BCR>,
> +			<&gcc GCC_PCIE_PHY_COM_NOCSR_BCR>,
> +			<&gcc GCC_PCIE_0_PHY_BCR>,
> +			<&gcc GCC_PCIE_1_PHY_BCR>,
> +			<&gcc GCC_PCIE_2_PHY_BCR>;
> +		reset-names = "phy", "common", "cfg",
> +				"lane0", "lane1", "lane2";

Each lane has a separate clock, separate reset.. why not create sub-nodes for
each lane?

Thanks
Kishon

Re: [PATCH v4 3/4] dt-bindings: phy: Add support for QMP phy

[Kishon Vijay Abraham I]

Hi,

On Tuesday 10 January 2017 04:21 PM, Vivek Gautam wrote:
> Qualcomm chipsets have QMP phy controller that provides
> support to a number of controller, viz. PCIe, UFS, and USB.
> Adding dt binding information for the same.
> 
> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
> 
> Changes since v3:
>  - Added #clock-cells = <1>, indicating that phy is a clock provider.
> 
> Changes since v2:
>  - Removed binding for "ref_clk_src" since we don't request this
>    clock in the driver.
>  - Addressed s/ref_clk/ref. Don't need to add '_clk' suffix to clock names.
>  - Using 'phy' for the node name.
> 
> Changes since v1:
>  - New patch, forked out of the original driver patch:
>    "phy: qcom-qmp: new qmp phy driver for qcom-chipsets"
>  - Added 'Acked-by' from Rob.
>  - Updated bindings to include mem resource as a list of
>    offset - length pair for serdes block and for each lane.
>  - Added a new binding for 'lane-offsets' that contains offsets
>    to tx, rx and pcs blocks from each lane base address.
> 
>  .../devicetree/bindings/phy/qcom-qmp-phy.txt       | 76 ++++++++++++++++++++++
>  1 file changed, 76 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> new file mode 100644
> index 000000000000..6f510fe48f46
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> @@ -0,0 +1,76 @@
> +Qualcomm QMP PHY controller
> +===========================
> +
> +QMP phy controller supports physical layer functionality for a number of
> +controllers on Qualcomm chipsets, such as, PCIe, UFS, and USB.
> +
> +Required properties:
> + - compatible: compatible list, contains:
> +	       "qcom,msm8996-qmp-pcie-phy" for 14nm PCIe phy on msm8996,
> +	       "qcom,msm8996-qmp-usb3-phy" for 14nm USB3 phy on msm8996.
> + - reg: list of offset and length pair of the PHY register sets.
> +	at index 0: offset and length of register set for PHY common
> +		    serdes block.
> +	from index 1 - N: offset and length of register set for each lane,
> +			  for N number of phy lanes (ports).
> + - lane-offsets: array of offsets to tx, rx and pcs blocks for phy lanes.
> + - #phy-cells: must be 1
> +    - Cell after phy phandle should be the port (lane) number.
> + - #clock-cells: must be 1
> +    - Phy pll outputs a bunch of clocks for Tx, Rx and Pipe
> +      interface (for pipe based PHYs). These clock are then gate-controlled
> +      by gcc.
> + - clocks: a list of phandles and clock-specifier pairs,
> +	   one for each entry in clock-names.
> + - clock-names: must be "cfg_ahb" for phy config clock,
> +			"aux" for phy aux clock,
> +			"ref" for 19.2 MHz ref clk,
> +			"pipe<port-number>" for pipe clock specific to
> +			each port/lane (Optional).
> + - resets: a list of phandles and reset controller specifier pairs,
> +	   one for each entry in reset-names.
> + - reset-names: must be "phy" for reset of phy block,
> +			"common" for phy common block reset,
> +			"cfg" for phy's ahb cfg block reset (Optional).
> +			"port<port-number>" for reset specific to
> +			each port/lane (Optional).
> + - vdda-phy-supply: Phandle to a regulator supply to PHY core block.
> + - vdda-pll-supply: Phandle to 1.8V regulator supply to PHY refclk pll block.
> +
> +Optional properties:
> + - vddp-ref-clk-supply: Phandle to a regulator supply to any specific refclk
> +			pll block.
> +
> +Example:
> +	pcie_phy: phy@34000 {
> +		compatible = "qcom,msm8996-qmp-pcie-phy";
> +		reg = <0x034000 0x48f>,
> +			<0x035000 0x5bf>,
> +			<0x036000 0x5bf>,
> +			<0x037000 0x5bf>;
> +				/* tx, rx, pcs */
> +		lane-offsets = <0x0 0x200 0x400>;
> +		#phy-cells = <1>;
> +		#clock-cells = <1>;
> +
> +		clocks = <&gcc GCC_PCIE_PHY_AUX_CLK>,
> +			<&gcc GCC_PCIE_PHY_CFG_AHB_CLK>,
> +			<&gcc GCC_PCIE_CLKREF_CLK>,
> +			<&gcc GCC_PCIE_0_PIPE_CLK>,
> +			<&gcc GCC_PCIE_1_PIPE_CLK>,
> +			<&gcc GCC_PCIE_2_PIPE_CLK>;
> +		clock-names = "aux", "cfg_ahb", "ref",
> +				"pipe0", "pipe1", "pipe2";
> +
> +		vdda-phy-supply = <&pm8994_l28>;
> +		vdda-pll-supply = <&pm8994_l12>;
> +
> +		resets = <&gcc GCC_PCIE_PHY_BCR>,
> +			<&gcc GCC_PCIE_PHY_COM_BCR>,
> +			<&gcc GCC_PCIE_PHY_COM_NOCSR_BCR>,
> +			<&gcc GCC_PCIE_0_PHY_BCR>,
> +			<&gcc GCC_PCIE_1_PHY_BCR>,
> +			<&gcc GCC_PCIE_2_PHY_BCR>;
> +		reset-names = "phy", "common", "cfg",
> +				"lane0", "lane1", "lane2";

Each lane has a separate clock, separate reset.. why not create sub-nodes for
each lane?

Thanks
Kishon

Re: [PATCH v4 3/4] dt-bindings: phy: Add support for QMP phy

[Vivek Gautam]

Hi Kishon,


On 01/16/2017 02:19 PM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Tuesday 10 January 2017 04:21 PM, Vivek Gautam wrote:
>> Qualcomm chipsets have QMP phy controller that provides
>> support to a number of controller, viz. PCIe, UFS, and USB.
>> Adding dt binding information for the same.
>>
>> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
>> Acked-by: Rob Herring <robh@kernel.org>
>> ---
>>
>> Changes since v3:
>>   - Added #clock-cells = <1>, indicating that phy is a clock provider.
>>
>> Changes since v2:
>>   - Removed binding for "ref_clk_src" since we don't request this
>>     clock in the driver.
>>   - Addressed s/ref_clk/ref. Don't need to add '_clk' suffix to clock names.
>>   - Using 'phy' for the node name.
>>
>> Changes since v1:
>>   - New patch, forked out of the original driver patch:
>>     "phy: qcom-qmp: new qmp phy driver for qcom-chipsets"
>>   - Added 'Acked-by' from Rob.
>>   - Updated bindings to include mem resource as a list of
>>     offset - length pair for serdes block and for each lane.
>>   - Added a new binding for 'lane-offsets' that contains offsets
>>     to tx, rx and pcs blocks from each lane base address.
>>
>>   .../devicetree/bindings/phy/qcom-qmp-phy.txt       | 76 ++++++++++++++++++++++
>>   1 file changed, 76 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
>>
>> diff --git a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
>> new file mode 100644
>> index 000000000000..6f510fe48f46
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
>> @@ -0,0 +1,76 @@
>> +Qualcomm QMP PHY controller
>> +===========================
>> +
>> +QMP phy controller supports physical layer functionality for a number of
>> +controllers on Qualcomm chipsets, such as, PCIe, UFS, and USB.
>> +
>> +Required properties:
>> + - compatible: compatible list, contains:
>> +	       "qcom,msm8996-qmp-pcie-phy" for 14nm PCIe phy on msm8996,
>> +	       "qcom,msm8996-qmp-usb3-phy" for 14nm USB3 phy on msm8996.
>> + - reg: list of offset and length pair of the PHY register sets.
>> +	at index 0: offset and length of register set for PHY common
>> +		    serdes block.
>> +	from index 1 - N: offset and length of register set for each lane,
>> +			  for N number of phy lanes (ports).
>> + - lane-offsets: array of offsets to tx, rx and pcs blocks for phy lanes.
>> + - #phy-cells: must be 1
>> +    - Cell after phy phandle should be the port (lane) number.
>> + - #clock-cells: must be 1
>> +    - Phy pll outputs a bunch of clocks for Tx, Rx and Pipe
>> +      interface (for pipe based PHYs). These clock are then gate-controlled
>> +      by gcc.
>> + - clocks: a list of phandles and clock-specifier pairs,
>> +	   one for each entry in clock-names.
>> + - clock-names: must be "cfg_ahb" for phy config clock,
>> +			"aux" for phy aux clock,
>> +			"ref" for 19.2 MHz ref clk,
>> +			"pipe<port-number>" for pipe clock specific to
>> +			each port/lane (Optional).
>> + - resets: a list of phandles and reset controller specifier pairs,
>> +	   one for each entry in reset-names.
>> + - reset-names: must be "phy" for reset of phy block,
>> +			"common" for phy common block reset,
>> +			"cfg" for phy's ahb cfg block reset (Optional).
>> +			"port<port-number>" for reset specific to
>> +			each port/lane (Optional).
>> + - vdda-phy-supply: Phandle to a regulator supply to PHY core block.
>> + - vdda-pll-supply: Phandle to 1.8V regulator supply to PHY refclk pll block.
>> +
>> +Optional properties:
>> + - vddp-ref-clk-supply: Phandle to a regulator supply to any specific refclk
>> +			pll block.
>> +
>> +Example:
>> +	pcie_phy: phy@34000 {
>> +		compatible = "qcom,msm8996-qmp-pcie-phy";
>> +		reg = <0x034000 0x48f>,
>> +			<0x035000 0x5bf>,
>> +			<0x036000 0x5bf>,
>> +			<0x037000 0x5bf>;
>> +				/* tx, rx, pcs */
>> +		lane-offsets = <0x0 0x200 0x400>;
>> +		#phy-cells = <1>;
>> +		#clock-cells = <1>;
>> +
>> +		clocks = <&gcc GCC_PCIE_PHY_AUX_CLK>,
>> +			<&gcc GCC_PCIE_PHY_CFG_AHB_CLK>,
>> +			<&gcc GCC_PCIE_CLKREF_CLK>,
>> +			<&gcc GCC_PCIE_0_PIPE_CLK>,
>> +			<&gcc GCC_PCIE_1_PIPE_CLK>,
>> +			<&gcc GCC_PCIE_2_PIPE_CLK>;
>> +		clock-names = "aux", "cfg_ahb", "ref",
>> +				"pipe0", "pipe1", "pipe2";
>> +
>> +		vdda-phy-supply = <&pm8994_l28>;
>> +		vdda-pll-supply = <&pm8994_l12>;
>> +
>> +		resets = <&gcc GCC_PCIE_PHY_BCR>,
>> +			<&gcc GCC_PCIE_PHY_COM_BCR>,
>> +			<&gcc GCC_PCIE_PHY_COM_NOCSR_BCR>,
>> +			<&gcc GCC_PCIE_0_PHY_BCR>,
>> +			<&gcc GCC_PCIE_1_PHY_BCR>,
>> +			<&gcc GCC_PCIE_2_PHY_BCR>;
>> +		reset-names = "phy", "common", "cfg",
>> +				"lane0", "lane1", "lane2";
> Each lane has a separate clock, separate reset.. why not create sub-nodes for
> each lane?

Yes, each lane has separate pipe clock and resets.
I can have a binding such as written below.
Does it makes sense to pull in the tx, rx and pcs offsets as well
to the child node, and iomap the entire address space of the phy ?

+ pcie_phy: pciephy@34000 {
+               compatible = "qcom,qmp-14nm-phy";
+               reg = <0x034000 0x3fff>;
+               #address-cells = <1>;
+               #size-cells = <0>;

                   ...

+               port@0 {
+                       reg = <0>;
+                       tx-offset = <0x1000>;
+                       rx-offset = <0x1200>;
+                       pcs-offset = <0x1400>;
+                       clocks = <&gcc GCC_USB3_PHY_PIPE_CLK>;
+                       clock-names = "pipe0";
+                       resets = <&gcc GCC_PCIE_0_PHY_BCR>;
+                       reset-names = "lane0";
+               };
+ };


Regards
Vivek
>
> Thanks
> Kishon
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v4 3/4] dt-bindings: phy: Add support for QMP phy

[Bjorn Andersson]

On Tue 17 Jan 22:54 PST 2017, Vivek Gautam wrote:
> On 01/16/2017 02:19 PM, Kishon Vijay Abraham I wrote:
> > On Tuesday 10 January 2017 04:21 PM, Vivek Gautam wrote:
[..]
> > > +		reset-names = "phy", "common", "cfg",
> > > +				"lane0", "lane1", "lane2";
> > Each lane has a separate clock, separate reset.. why not create sub-nodes for
> > each lane?
> 
> Yes, each lane has separate pipe clock and resets.
> I can have a binding such as written below.

+1

> Does it makes sense to pull in the tx, rx and pcs offsets as well
> to the child node, and iomap the entire address space of the phy ?
> 

Note that you don't have to follow the same structure in your device
driver as you describe your hardware in devicetree.

I would suggest that you replace the lane-offset and various lane
specific resources with subnodes, but keep the driver "as is".

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/4] dt-bindings: phy: Add support for QMP phy

[Bjorn Andersson]

On Tue 17 Jan 22:54 PST 2017, Vivek Gautam wrote:
> On 01/16/2017 02:19 PM, Kishon Vijay Abraham I wrote:
> > On Tuesday 10 January 2017 04:21 PM, Vivek Gautam wrote:
[..]
> > > +		reset-names = "phy", "common", "cfg",
> > > +				"lane0", "lane1", "lane2";
> > Each lane has a separate clock, separate reset.. why not create sub-nodes for
> > each lane?
> 
> Yes, each lane has separate pipe clock and resets.
> I can have a binding such as written below.

+1

> Does it makes sense to pull in the tx, rx and pcs offsets as well
> to the child node, and iomap the entire address space of the phy ?
> 

Note that you don't have to follow the same structure in your device
driver as you describe your hardware in devicetree.

I would suggest that you replace the lane-offset and various lane
specific resources with subnodes, but keep the driver "as is".

Regards,
Bjorn

Re: [PATCH v4 3/4] dt-bindings: phy: Add support for QMP phy

[Stephen Boyd]

On 01/18, Bjorn Andersson wrote:
> On Tue 17 Jan 22:54 PST 2017, Vivek Gautam wrote:
> > On 01/16/2017 02:19 PM, Kishon Vijay Abraham I wrote:
> > > On Tuesday 10 January 2017 04:21 PM, Vivek Gautam wrote:
> [..]
> > > > +		reset-names = "phy", "common", "cfg",
> > > > +				"lane0", "lane1", "lane2";
> > > Each lane has a separate clock, separate reset.. why not create sub-nodes for
> > > each lane?
> > 
> > Yes, each lane has separate pipe clock and resets.
> > I can have a binding such as written below.
> 
> +1
> 
> > Does it makes sense to pull in the tx, rx and pcs offsets as well
> > to the child node, and iomap the entire address space of the phy ?
> > 
> 
> Note that you don't have to follow the same structure in your device
> driver as you describe your hardware in devicetree.
> 
> I would suggest that you replace the lane-offset and various lane
> specific resources with subnodes, but keep the driver "as is".
> 

Didn't we already move away from subnodes for lanes in an earlier
revision of these patches? I seem to recall we did that because
lanes are not devices and the whole "phy as a bus" concept not
making sense.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v4 3/4] dt-bindings: phy: Add support for QMP phy

[Vivek Gautam]

On 01/19/2017 06:10 AM, Stephen Boyd wrote:
> On 01/18, Bjorn Andersson wrote:
>> On Tue 17 Jan 22:54 PST 2017, Vivek Gautam wrote:
>>> On 01/16/2017 02:19 PM, Kishon Vijay Abraham I wrote:
>>>> On Tuesday 10 January 2017 04:21 PM, Vivek Gautam wrote:
>> [..]
>>>>> +		reset-names = "phy", "common", "cfg",
>>>>> +				"lane0", "lane1", "lane2";
>>>> Each lane has a separate clock, separate reset.. why not create sub-nodes for
>>>> each lane?
>>> Yes, each lane has separate pipe clock and resets.
>>> I can have a binding such as written below.
>> +1
>>
>>> Does it makes sense to pull in the tx, rx and pcs offsets as well
>>> to the child node, and iomap the entire address space of the phy ?
>>>
>> Note that you don't have to follow the same structure in your device
>> driver as you describe your hardware in devicetree.
>>
>> I would suggest that you replace the lane-offset and various lane
>> specific resources with subnodes, but keep the driver "as is".
>>
> Didn't we already move away from subnodes for lanes in an earlier
> revision of these patches? I seem to recall we did that because
> lanes are not devices and the whole "phy as a bus" concept not
> making sense.

Yea, we started out without having any sub-nodes and we
argued that we don't require them since the qmp device is
represented by the qmp node itself.
The lanes otoh are representative of gen_phys and related properties.

In the driver -
"struct qmp_phy " represents the lanes and holds "struct phy",
"struct qcom_qmp" represents the qmp block as a whole and holds "struct 
device"
Does this make lanes qualify to be childs of qmp ?

"phy as a bus" (just trying to understand here) -
let's say a usb phy controller has one HSIC phy port and one USB2 phy port.
So, should this phy controller be a bus providing two ports (and so we 
will have
couple of child nodes to the phy controller) ?


Regards
Vivek

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/4] dt-bindings: phy: Add support for QMP phy

[Vivek Gautam]

On 01/19/2017 06:10 AM, Stephen Boyd wrote:
> On 01/18, Bjorn Andersson wrote:
>> On Tue 17 Jan 22:54 PST 2017, Vivek Gautam wrote:
>>> On 01/16/2017 02:19 PM, Kishon Vijay Abraham I wrote:
>>>> On Tuesday 10 January 2017 04:21 PM, Vivek Gautam wrote:
>> [..]
>>>>> +		reset-names = "phy", "common", "cfg",
>>>>> +				"lane0", "lane1", "lane2";
>>>> Each lane has a separate clock, separate reset.. why not create sub-nodes for
>>>> each lane?
>>> Yes, each lane has separate pipe clock and resets.
>>> I can have a binding such as written below.
>> +1
>>
>>> Does it makes sense to pull in the tx, rx and pcs offsets as well
>>> to the child node, and iomap the entire address space of the phy ?
>>>
>> Note that you don't have to follow the same structure in your device
>> driver as you describe your hardware in devicetree.
>>
>> I would suggest that you replace the lane-offset and various lane
>> specific resources with subnodes, but keep the driver "as is".
>>
> Didn't we already move away from subnodes for lanes in an earlier
> revision of these patches? I seem to recall we did that because
> lanes are not devices and the whole "phy as a bus" concept not
> making sense.

Yea, we started out without having any sub-nodes and we
argued that we don't require them since the qmp device is
represented by the qmp node itself.
The lanes otoh are representative of gen_phys and related properties.

In the driver -
"struct qmp_phy " represents the lanes and holds "struct phy",
"struct qcom_qmp" represents the qmp block as a whole and holds "struct 
device"
Does this make lanes qualify to be childs of qmp ?

"phy as a bus" (just trying to understand here) -
let's say a usb phy controller has one HSIC phy port and one USB2 phy port.
So, should this phy controller be a bus providing two ports (and so we 
will have
couple of child nodes to the phy controller) ?


Regards
Vivek

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v4 3/4] dt-bindings: phy: Add support for QMP phy

[Stephen Boyd]

On 01/19, Vivek Gautam wrote:
> 
> On 01/19/2017 06:10 AM, Stephen Boyd wrote:
> >>
> >Didn't we already move away from subnodes for lanes in an earlier
> >revision of these patches? I seem to recall we did that because
> >lanes are not devices and the whole "phy as a bus" concept not
> >making sense.
> 
> Yea, we started out without having any sub-nodes and we
> argued that we don't require them since the qmp device is
> represented by the qmp node itself.
> The lanes otoh are representative of gen_phys and related properties.
> 
> In the driver -
> "struct qmp_phy " represents the lanes and holds "struct phy",
> "struct qcom_qmp" represents the qmp block as a whole and holds
> "struct device"
> Does this make lanes qualify to be childs of qmp ?

Hmm... maybe I was recalling the DSI phy binding. I think there
are lanes there too but we decided to just have one node.

> 
> "phy as a bus" (just trying to understand here) -
> let's say a usb phy controller has one HSIC phy port and one USB2 phy port.
> So, should this phy controller be a bus providing two ports (and so
> we will have
> couple of child nodes to the phy controller) ?
> 

Typically in DT a subnode or collection of subnodes means there's
some sort of bus involved. Usually each node corresponds to a
struct device, and the parent node corresponds to the bus or
controller for the logical bus.

In this case (only PCIe though? not UFS or USB?) it seems like we
have multiple phys that share a common register space, but
otherwise they have their own register space and power
management. Would you have each PCIe controller point to a
different subnode for their associated phy? I'm trying to
understand the benefit of the subnodes if they aren't treated as
struct devices.

At the least, please get DT reviewers to ack the new binding
before rewriting the code.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v4 3/4] dt-bindings: phy: Add support for QMP phy

[Vivek Gautam]

On 01/20/2017 03:12 AM, Stephen Boyd wrote:
> On 01/19, Vivek Gautam wrote:
>> On 01/19/2017 06:10 AM, Stephen Boyd wrote:
>>> Didn't we already move away from subnodes for lanes in an earlier
>>> revision of these patches? I seem to recall we did that because
>>> lanes are not devices and the whole "phy as a bus" concept not
>>> making sense.
>> Yea, we started out without having any sub-nodes and we
>> argued that we don't require them since the qmp device is
>> represented by the qmp node itself.
>> The lanes otoh are representative of gen_phys and related properties.
>>
>> In the driver -
>> "struct qmp_phy " represents the lanes and holds "struct phy",
>> "struct qcom_qmp" represents the qmp block as a whole and holds
>> "struct device"
>> Does this make lanes qualify to be childs of qmp ?
> Hmm... maybe I was recalling the DSI phy binding. I think there
> are lanes there too but we decided to just have one node.
>
>> "phy as a bus" (just trying to understand here) -
>> let's say a usb phy controller has one HSIC phy port and one USB2 phy port.
>> So, should this phy controller be a bus providing two ports (and so
>> we will have
>> couple of child nodes to the phy controller) ?
>>
> Typically in DT a subnode or collection of subnodes means there's
> some sort of bus involved. Usually each node corresponds to a
> struct device, and the parent node corresponds to the bus or
> controller for the logical bus.
>
> In this case (only PCIe though? not UFS or USB?) it seems like we
> have multiple phys that share a common register space, but
> otherwise they have their own register space and power
> management. Would you have each PCIe controller point to a
> different subnode for their associated phy? I'm trying to
> understand the benefit of the subnodes if they aren't treated as
> struct devices.

AFAIU, It's not straight that forward to point each controller to
different subnodes and not associate a 'struct device' with subnodes.

The phy translation (of_phy_simple_xlate, in case we point each controller
to subnodes) will not be able to associate a correct phy_provider device
to the phy consumer.

Kishon,
Is my understanding correct here? Please correct me if i am missing things.


Regards
Vivek
>
> At the least, please get DT reviewers to ack the new binding
> before rewriting the code.
>

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v4 3/4] dt-bindings: phy: Add support for QMP phy

[Kishon Vijay Abraham I]

Hi,

On Friday 20 January 2017 03:12 AM, Stephen Boyd wrote:
> On 01/19, Vivek Gautam wrote:
>>
>> On 01/19/2017 06:10 AM, Stephen Boyd wrote:
>>>>
>>> Didn't we already move away from subnodes for lanes in an earlier
>>> revision of these patches? I seem to recall we did that because
>>> lanes are not devices and the whole "phy as a bus" concept not
>>> making sense.
>>
>> Yea, we started out without having any sub-nodes and we
>> argued that we don't require them since the qmp device is
>> represented by the qmp node itself.
>> The lanes otoh are representative of gen_phys and related properties.
>>
>> In the driver -
>> "struct qmp_phy " represents the lanes and holds "struct phy",
>> "struct qcom_qmp" represents the qmp block as a whole and holds
>> "struct device"
>> Does this make lanes qualify to be childs of qmp ?
> 
> Hmm... maybe I was recalling the DSI phy binding. I think there
> are lanes there too but we decided to just have one node.
> 
>>
>> "phy as a bus" (just trying to understand here) -
>> let's say a usb phy controller has one HSIC phy port and one USB2 phy port.
>> So, should this phy controller be a bus providing two ports (and so
>> we will have
>> couple of child nodes to the phy controller) ?
>>
> 
> Typically in DT a subnode or collection of subnodes means there's
> some sort of bus involved. Usually each node corresponds to a
> struct device, and the parent node corresponds to the bus or
> controller for the logical bus.
> 
> In this case (only PCIe though? not UFS or USB?) it seems like we
> have multiple phys that share a common register space, but
> otherwise they have their own register space and power
> management. Would you have each PCIe controller point to a
> different subnode for their associated phy? I'm trying to
> understand the benefit of the subnodes if they aren't treated as
> struct devices.

Yes, instead of having all the controller having a phandle to the same PHY and
then using other mechanisms to differentiate between the PHYs, each controller
can have a phandle to the exact port that it is connected to.

This also gives a better representation of the hardware and can avoid lot of
boilerplate code in the driver.

Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/4] dt-bindings: phy: Add support for QMP phy

[Kishon Vijay Abraham I]

Hi,

On Friday 20 January 2017 03:12 AM, Stephen Boyd wrote:
> On 01/19, Vivek Gautam wrote:
>>
>> On 01/19/2017 06:10 AM, Stephen Boyd wrote:
>>>>
>>> Didn't we already move away from subnodes for lanes in an earlier
>>> revision of these patches? I seem to recall we did that because
>>> lanes are not devices and the whole "phy as a bus" concept not
>>> making sense.
>>
>> Yea, we started out without having any sub-nodes and we
>> argued that we don't require them since the qmp device is
>> represented by the qmp node itself.
>> The lanes otoh are representative of gen_phys and related properties.
>>
>> In the driver -
>> "struct qmp_phy " represents the lanes and holds "struct phy",
>> "struct qcom_qmp" represents the qmp block as a whole and holds
>> "struct device"
>> Does this make lanes qualify to be childs of qmp ?
> 
> Hmm... maybe I was recalling the DSI phy binding. I think there
> are lanes there too but we decided to just have one node.
> 
>>
>> "phy as a bus" (just trying to understand here) -
>> let's say a usb phy controller has one HSIC phy port and one USB2 phy port.
>> So, should this phy controller be a bus providing two ports (and so
>> we will have
>> couple of child nodes to the phy controller) ?
>>
> 
> Typically in DT a subnode or collection of subnodes means there's
> some sort of bus involved. Usually each node corresponds to a
> struct device, and the parent node corresponds to the bus or
> controller for the logical bus.
> 
> In this case (only PCIe though? not UFS or USB?) it seems like we
> have multiple phys that share a common register space, but
> otherwise they have their own register space and power
> management. Would you have each PCIe controller point to a
> different subnode for their associated phy? I'm trying to
> understand the benefit of the subnodes if they aren't treated as
> struct devices.

Yes, instead of having all the controller having a phandle to the same PHY and
then using other mechanisms to differentiate between the PHYs, each controller
can have a phandle to the exact port that it is connected to.

This also gives a better representation of the hardware and can avoid lot of
boilerplate code in the driver.

Thanks
Kishon

Re: [PATCH v4 3/4] dt-bindings: phy: Add support for QMP phy

[Vivek Gautam]

On Tue, Jan 24, 2017 at 3:03 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
> Hi,
>
> On Friday 20 January 2017 03:12 AM, Stephen Boyd wrote:
>> On 01/19, Vivek Gautam wrote:
>>>
>>> On 01/19/2017 06:10 AM, Stephen Boyd wrote:
>>>>>
>>>> Didn't we already move away from subnodes for lanes in an earlier
>>>> revision of these patches? I seem to recall we did that because
>>>> lanes are not devices and the whole "phy as a bus" concept not
>>>> making sense.
>>>
>>> Yea, we started out without having any sub-nodes and we
>>> argued that we don't require them since the qmp device is
>>> represented by the qmp node itself.
>>> The lanes otoh are representative of gen_phys and related properties.
>>>
>>> In the driver -
>>> "struct qmp_phy " represents the lanes and holds "struct phy",
>>> "struct qcom_qmp" represents the qmp block as a whole and holds
>>> "struct device"
>>> Does this make lanes qualify to be childs of qmp ?
>>
>> Hmm... maybe I was recalling the DSI phy binding. I think there
>> are lanes there too but we decided to just have one node.
>>
>>>
>>> "phy as a bus" (just trying to understand here) -
>>> let's say a usb phy controller has one HSIC phy port and one USB2 phy port.
>>> So, should this phy controller be a bus providing two ports (and so
>>> we will have
>>> couple of child nodes to the phy controller) ?
>>>
>>
>> Typically in DT a subnode or collection of subnodes means there's
>> some sort of bus involved. Usually each node corresponds to a
>> struct device, and the parent node corresponds to the bus or
>> controller for the logical bus.
>>
>> In this case (only PCIe though? not UFS or USB?) it seems like we
>> have multiple phys that share a common register space, but
>> otherwise they have their own register space and power
>> management. Would you have each PCIe controller point to a
>> different subnode for their associated phy? I'm trying to
>> understand the benefit of the subnodes if they aren't treated as
>> struct devices.
>
> Yes, instead of having all the controller having a phandle to the same PHY and
> then using other mechanisms to differentiate between the PHYs, each controller
> can have a phandle to the exact port that it is connected to.
>
> This also gives a better representation of the hardware and can avoid lot of
> boilerplate code in the driver.

Below is one binding that works for me.
--------------------
               phy@34000 {
                        compatible = "qcom,msm8996-qmp-pcie-phy";
                        reg = <0x034000 0x488>;
                        #clock-cells = <1>;
                        #address-cells = <1>;
                        #size-cells = <1>;
                        ranges;

                        clocks = <&gcc GCC_PCIE_PHY_AUX_CLK>,
                                <&gcc GCC_PCIE_PHY_CFG_AHB_CLK>,
                                <&gcc GCC_PCIE_CLKREF_CLK>;
                        clock-names = "aux", "cfg_ahb", "ref";

                        vdda-phy-supply = <&pm8994_l28>;
                        vdda-pll-supply = <&pm8994_l12>;

                        resets = <&gcc GCC_PCIE_PHY_BCR>,
                                <&gcc GCC_PCIE_PHY_COM_BCR>,
                                <&gcc GCC_PCIE_PHY_COM_NOCSR_BCR>;
                        reset-names = "phy", "common", "cfg";

                        pciephy_p0: port@0 {
                                reg = <0x035000 0x130>,
                                        <0x035200 0x200>,
                                        <0x035400 0x1dc>;
                                #phy-cells = <0>;

                                clocks = <&gcc GCC_PCIE_0_PIPE_CLK>;
                                clock-names = "pipe0";
                                resets = <&gcc GCC_PCIE_0_PHY_BCR>;
                                reset-names = "lane0";
                        };

                      pciephy_p1: port@1 {
                                reg = <0x036000 0x130>,
                                        <0x036200 0x200>,
                                        <0x036400 0x1dc>;
                                #phy-cells = <0>;

                                clocks = <&gcc GCC_PCIE_1_PIPE_CLK>;
                                clock-names = "pipe1";
                                resets = <&gcc GCC_PCIE_1_PHY_BCR>;
                                reset-names = "lane1";
                        };

                        pciephy_p2: port@2 {
                                reg = <0x037000 0x130>,
                                        <0x037200 0x200>,
                                        <0x037400 0x1dc>;
                                #phy-cells = <0>;

                                clocks = <&gcc GCC_PCIE_2_PIPE_CLK>;
                                clock-names = "pipe2";
                                resets = <&gcc GCC_PCIE_2_PHY_BCR>;
                                reset-names = "lane2";
                        };
                };
--------------------

let me know if this looks okay.


Regards
Vivek

>
> Thanks
> Kishon
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v4 3/4] dt-bindings: phy: Add support for QMP phy

[Kishon Vijay Abraham I]

Hi,

On Tuesday 24 January 2017 07:35 PM, Vivek Gautam wrote:
> On Tue, Jan 24, 2017 at 3:03 PM, Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org> wrote:
>> Hi,
>>
>> On Friday 20 January 2017 03:12 AM, Stephen Boyd wrote:
>>> On 01/19, Vivek Gautam wrote:
>>>>
>>>> On 01/19/2017 06:10 AM, Stephen Boyd wrote:
>>>>>>
>>>>> Didn't we already move away from subnodes for lanes in an earlier
>>>>> revision of these patches? I seem to recall we did that because
>>>>> lanes are not devices and the whole "phy as a bus" concept not
>>>>> making sense.
>>>>
>>>> Yea, we started out without having any sub-nodes and we
>>>> argued that we don't require them since the qmp device is
>>>> represented by the qmp node itself.
>>>> The lanes otoh are representative of gen_phys and related properties.
>>>>
>>>> In the driver -
>>>> "struct qmp_phy " represents the lanes and holds "struct phy",
>>>> "struct qcom_qmp" represents the qmp block as a whole and holds
>>>> "struct device"
>>>> Does this make lanes qualify to be childs of qmp ?
>>>
>>> Hmm... maybe I was recalling the DSI phy binding. I think there
>>> are lanes there too but we decided to just have one node.
>>>
>>>>
>>>> "phy as a bus" (just trying to understand here) -
>>>> let's say a usb phy controller has one HSIC phy port and one USB2 phy port.
>>>> So, should this phy controller be a bus providing two ports (and so
>>>> we will have
>>>> couple of child nodes to the phy controller) ?
>>>>
>>>
>>> Typically in DT a subnode or collection of subnodes means there's
>>> some sort of bus involved. Usually each node corresponds to a
>>> struct device, and the parent node corresponds to the bus or
>>> controller for the logical bus.
>>>
>>> In this case (only PCIe though? not UFS or USB?) it seems like we
>>> have multiple phys that share a common register space, but
>>> otherwise they have their own register space and power
>>> management. Would you have each PCIe controller point to a
>>> different subnode for their associated phy? I'm trying to
>>> understand the benefit of the subnodes if they aren't treated as
>>> struct devices.
>>
>> Yes, instead of having all the controller having a phandle to the same PHY and
>> then using other mechanisms to differentiate between the PHYs, each controller
>> can have a phandle to the exact port that it is connected to.
>>
>> This also gives a better representation of the hardware and can avoid lot of
>> boilerplate code in the driver.
> 
> Below is one binding that works for me.
> --------------------
>                phy@34000 {
>                         compatible = "qcom,msm8996-qmp-pcie-phy";
>                         reg = <0x034000 0x488>;
>                         #clock-cells = <1>;
>                         #address-cells = <1>;
>                         #size-cells = <1>;
>                         ranges;
> 
>                         clocks = <&gcc GCC_PCIE_PHY_AUX_CLK>,
>                                 <&gcc GCC_PCIE_PHY_CFG_AHB_CLK>,
>                                 <&gcc GCC_PCIE_CLKREF_CLK>;
>                         clock-names = "aux", "cfg_ahb", "ref";
> 
>                         vdda-phy-supply = <&pm8994_l28>;
>                         vdda-pll-supply = <&pm8994_l12>;
> 
>                         resets = <&gcc GCC_PCIE_PHY_BCR>,
>                                 <&gcc GCC_PCIE_PHY_COM_BCR>,
>                                 <&gcc GCC_PCIE_PHY_COM_NOCSR_BCR>;
>                         reset-names = "phy", "common", "cfg";
> 
>                         pciephy_p0: port@0 {
>                                 reg = <0x035000 0x130>,
>                                         <0x035200 0x200>,
>                                         <0x035400 0x1dc>;
>                                 #phy-cells = <0>;
> 
>                                 clocks = <&gcc GCC_PCIE_0_PIPE_CLK>;
>                                 clock-names = "pipe0";
>                                 resets = <&gcc GCC_PCIE_0_PHY_BCR>;
>                                 reset-names = "lane0";
>                         };
> 
>                       pciephy_p1: port@1 {
>                                 reg = <0x036000 0x130>,
>                                         <0x036200 0x200>,
>                                         <0x036400 0x1dc>;
>                                 #phy-cells = <0>;
> 
>                                 clocks = <&gcc GCC_PCIE_1_PIPE_CLK>;
>                                 clock-names = "pipe1";
>                                 resets = <&gcc GCC_PCIE_1_PHY_BCR>;
>                                 reset-names = "lane1";
>                         };
> 
>                         pciephy_p2: port@2 {
>                                 reg = <0x037000 0x130>,
>                                         <0x037200 0x200>,
>                                         <0x037400 0x1dc>;
>                                 #phy-cells = <0>;
> 
>                                 clocks = <&gcc GCC_PCIE_2_PIPE_CLK>;
>                                 clock-names = "pipe2";
>                                 resets = <&gcc GCC_PCIE_2_PHY_BCR>;
>                                 reset-names = "lane2";
>                         };
>                 };
> --------------------
> 
> let me know if this looks okay.

looks good to me.

Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/4] dt-bindings: phy: Add support for QMP phy

[Kishon Vijay Abraham I]

Hi,

On Tuesday 24 January 2017 07:35 PM, Vivek Gautam wrote:
> On Tue, Jan 24, 2017 at 3:03 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>> Hi,
>>
>> On Friday 20 January 2017 03:12 AM, Stephen Boyd wrote:
>>> On 01/19, Vivek Gautam wrote:
>>>>
>>>> On 01/19/2017 06:10 AM, Stephen Boyd wrote:
>>>>>>
>>>>> Didn't we already move away from subnodes for lanes in an earlier
>>>>> revision of these patches? I seem to recall we did that because
>>>>> lanes are not devices and the whole "phy as a bus" concept not
>>>>> making sense.
>>>>
>>>> Yea, we started out without having any sub-nodes and we
>>>> argued that we don't require them since the qmp device is
>>>> represented by the qmp node itself.
>>>> The lanes otoh are representative of gen_phys and related properties.
>>>>
>>>> In the driver -
>>>> "struct qmp_phy " represents the lanes and holds "struct phy",
>>>> "struct qcom_qmp" represents the qmp block as a whole and holds
>>>> "struct device"
>>>> Does this make lanes qualify to be childs of qmp ?
>>>
>>> Hmm... maybe I was recalling the DSI phy binding. I think there
>>> are lanes there too but we decided to just have one node.
>>>
>>>>
>>>> "phy as a bus" (just trying to understand here) -
>>>> let's say a usb phy controller has one HSIC phy port and one USB2 phy port.
>>>> So, should this phy controller be a bus providing two ports (and so
>>>> we will have
>>>> couple of child nodes to the phy controller) ?
>>>>
>>>
>>> Typically in DT a subnode or collection of subnodes means there's
>>> some sort of bus involved. Usually each node corresponds to a
>>> struct device, and the parent node corresponds to the bus or
>>> controller for the logical bus.
>>>
>>> In this case (only PCIe though? not UFS or USB?) it seems like we
>>> have multiple phys that share a common register space, but
>>> otherwise they have their own register space and power
>>> management. Would you have each PCIe controller point to a
>>> different subnode for their associated phy? I'm trying to
>>> understand the benefit of the subnodes if they aren't treated as
>>> struct devices.
>>
>> Yes, instead of having all the controller having a phandle to the same PHY and
>> then using other mechanisms to differentiate between the PHYs, each controller
>> can have a phandle to the exact port that it is connected to.
>>
>> This also gives a better representation of the hardware and can avoid lot of
>> boilerplate code in the driver.
> 
> Below is one binding that works for me.
> --------------------
>                phy@34000 {
>                         compatible = "qcom,msm8996-qmp-pcie-phy";
>                         reg = <0x034000 0x488>;
>                         #clock-cells = <1>;
>                         #address-cells = <1>;
>                         #size-cells = <1>;
>                         ranges;
> 
>                         clocks = <&gcc GCC_PCIE_PHY_AUX_CLK>,
>                                 <&gcc GCC_PCIE_PHY_CFG_AHB_CLK>,
>                                 <&gcc GCC_PCIE_CLKREF_CLK>;
>                         clock-names = "aux", "cfg_ahb", "ref";
> 
>                         vdda-phy-supply = <&pm8994_l28>;
>                         vdda-pll-supply = <&pm8994_l12>;
> 
>                         resets = <&gcc GCC_PCIE_PHY_BCR>,
>                                 <&gcc GCC_PCIE_PHY_COM_BCR>,
>                                 <&gcc GCC_PCIE_PHY_COM_NOCSR_BCR>;
>                         reset-names = "phy", "common", "cfg";
> 
>                         pciephy_p0: port@0 {
>                                 reg = <0x035000 0x130>,
>                                         <0x035200 0x200>,
>                                         <0x035400 0x1dc>;
>                                 #phy-cells = <0>;
> 
>                                 clocks = <&gcc GCC_PCIE_0_PIPE_CLK>;
>                                 clock-names = "pipe0";
>                                 resets = <&gcc GCC_PCIE_0_PHY_BCR>;
>                                 reset-names = "lane0";
>                         };
> 
>                       pciephy_p1: port@1 {
>                                 reg = <0x036000 0x130>,
>                                         <0x036200 0x200>,
>                                         <0x036400 0x1dc>;
>                                 #phy-cells = <0>;
> 
>                                 clocks = <&gcc GCC_PCIE_1_PIPE_CLK>;
>                                 clock-names = "pipe1";
>                                 resets = <&gcc GCC_PCIE_1_PHY_BCR>;
>                                 reset-names = "lane1";
>                         };
> 
>                         pciephy_p2: port@2 {
>                                 reg = <0x037000 0x130>,
>                                         <0x037200 0x200>,
>                                         <0x037400 0x1dc>;
>                                 #phy-cells = <0>;
> 
>                                 clocks = <&gcc GCC_PCIE_2_PIPE_CLK>;
>                                 clock-names = "pipe2";
>                                 resets = <&gcc GCC_PCIE_2_PHY_BCR>;
>                                 reset-names = "lane2";
>                         };
>                 };
> --------------------
> 
> let me know if this looks okay.

looks good to me.

Thanks
Kishon

Re: [PATCH v4 3/4] dt-bindings: phy: Add support for QMP phy

[Vivek Gautam]

Hi,


On Tue, Jan 24, 2017 at 7:45 PM, Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org> wrote:
> Hi,
>
> On Tuesday 24 January 2017 07:35 PM, Vivek Gautam wrote:
>> On Tue, Jan 24, 2017 at 3:03 PM, Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org> wrote:
>>> Hi,
>>>
>>> On Friday 20 January 2017 03:12 AM, Stephen Boyd wrote:
>>>> On 01/19, Vivek Gautam wrote:
>>>>>
>>>>> On 01/19/2017 06:10 AM, Stephen Boyd wrote:
>>>>>>>
>>>>>> Didn't we already move away from subnodes for lanes in an earlier
>>>>>> revision of these patches? I seem to recall we did that because
>>>>>> lanes are not devices and the whole "phy as a bus" concept not
>>>>>> making sense.
>>>>>
>>>>> Yea, we started out without having any sub-nodes and we
>>>>> argued that we don't require them since the qmp device is
>>>>> represented by the qmp node itself.
>>>>> The lanes otoh are representative of gen_phys and related properties.
>>>>>
>>>>> In the driver -
>>>>> "struct qmp_phy " represents the lanes and holds "struct phy",
>>>>> "struct qcom_qmp" represents the qmp block as a whole and holds
>>>>> "struct device"
>>>>> Does this make lanes qualify to be childs of qmp ?
>>>>
>>>> Hmm... maybe I was recalling the DSI phy binding. I think there
>>>> are lanes there too but we decided to just have one node.
>>>>
>>>>>
>>>>> "phy as a bus" (just trying to understand here) -
>>>>> let's say a usb phy controller has one HSIC phy port and one USB2 phy port.
>>>>> So, should this phy controller be a bus providing two ports (and so
>>>>> we will have
>>>>> couple of child nodes to the phy controller) ?
>>>>>
>>>>
>>>> Typically in DT a subnode or collection of subnodes means there's
>>>> some sort of bus involved. Usually each node corresponds to a
>>>> struct device, and the parent node corresponds to the bus or
>>>> controller for the logical bus.
>>>>
>>>> In this case (only PCIe though? not UFS or USB?) it seems like we
>>>> have multiple phys that share a common register space, but
>>>> otherwise they have their own register space and power
>>>> management. Would you have each PCIe controller point to a
>>>> different subnode for their associated phy? I'm trying to
>>>> understand the benefit of the subnodes if they aren't treated as
>>>> struct devices.
>>>
>>> Yes, instead of having all the controller having a phandle to the same PHY and
>>> then using other mechanisms to differentiate between the PHYs, each controller
>>> can have a phandle to the exact port that it is connected to.
>>>
>>> This also gives a better representation of the hardware and can avoid lot of
>>> boilerplate code in the driver.
>>
>> Below is one binding that works for me.
>> --------------------
>>                phy@34000 {
>>                         compatible = "qcom,msm8996-qmp-pcie-phy";
>>                         reg = <0x034000 0x488>;
>>                         #clock-cells = <1>;
>>                         #address-cells = <1>;
>>                         #size-cells = <1>;
>>                         ranges;
>>
>>                         clocks = <&gcc GCC_PCIE_PHY_AUX_CLK>,
>>                                 <&gcc GCC_PCIE_PHY_CFG_AHB_CLK>,
>>                                 <&gcc GCC_PCIE_CLKREF_CLK>;
>>                         clock-names = "aux", "cfg_ahb", "ref";
>>
>>                         vdda-phy-supply = <&pm8994_l28>;
>>                         vdda-pll-supply = <&pm8994_l12>;
>>
>>                         resets = <&gcc GCC_PCIE_PHY_BCR>,
>>                                 <&gcc GCC_PCIE_PHY_COM_BCR>,
>>                                 <&gcc GCC_PCIE_PHY_COM_NOCSR_BCR>;
>>                         reset-names = "phy", "common", "cfg";
>>
>>                         pciephy_p0: port@0 {
>>                                 reg = <0x035000 0x130>,
>>                                         <0x035200 0x200>,
>>                                         <0x035400 0x1dc>;
>>                                 #phy-cells = <0>;
>>
>>                                 clocks = <&gcc GCC_PCIE_0_PIPE_CLK>;
>>                                 clock-names = "pipe0";
>>                                 resets = <&gcc GCC_PCIE_0_PHY_BCR>;
>>                                 reset-names = "lane0";
>>                         };
>>
>>                       pciephy_p1: port@1 {
>>                                 reg = <0x036000 0x130>,
>>                                         <0x036200 0x200>,
>>                                         <0x036400 0x1dc>;
>>                                 #phy-cells = <0>;
>>
>>                                 clocks = <&gcc GCC_PCIE_1_PIPE_CLK>;
>>                                 clock-names = "pipe1";
>>                                 resets = <&gcc GCC_PCIE_1_PHY_BCR>;
>>                                 reset-names = "lane1";
>>                         };
>>
>>                         pciephy_p2: port@2 {
>>                                 reg = <0x037000 0x130>,
>>                                         <0x037200 0x200>,
>>                                         <0x037400 0x1dc>;
>>                                 #phy-cells = <0>;
>>
>>                                 clocks = <&gcc GCC_PCIE_2_PIPE_CLK>;
>>                                 clock-names = "pipe2";
>>                                 resets = <&gcc GCC_PCIE_2_PHY_BCR>;
>>                                 reset-names = "lane2";
>>                         };
>>                 };
>> --------------------
>>
>> let me know if this looks okay.
>
> looks good to me.

Great.

Rob, please let me know if above bindings look okay to you.
I can re-spin the bindings patch then.

Thanks

Regards
Vivek

>
> Thanks
> Kishon
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/4] dt-bindings: phy: Add support for QMP phy

[Vivek Gautam]

Hi,


On Tue, Jan 24, 2017 at 7:45 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
> Hi,
>
> On Tuesday 24 January 2017 07:35 PM, Vivek Gautam wrote:
>> On Tue, Jan 24, 2017 at 3:03 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>>> Hi,
>>>
>>> On Friday 20 January 2017 03:12 AM, Stephen Boyd wrote:
>>>> On 01/19, Vivek Gautam wrote:
>>>>>
>>>>> On 01/19/2017 06:10 AM, Stephen Boyd wrote:
>>>>>>>
>>>>>> Didn't we already move away from subnodes for lanes in an earlier
>>>>>> revision of these patches? I seem to recall we did that because
>>>>>> lanes are not devices and the whole "phy as a bus" concept not
>>>>>> making sense.
>>>>>
>>>>> Yea, we started out without having any sub-nodes and we
>>>>> argued that we don't require them since the qmp device is
>>>>> represented by the qmp node itself.
>>>>> The lanes otoh are representative of gen_phys and related properties.
>>>>>
>>>>> In the driver -
>>>>> "struct qmp_phy " represents the lanes and holds "struct phy",
>>>>> "struct qcom_qmp" represents the qmp block as a whole and holds
>>>>> "struct device"
>>>>> Does this make lanes qualify to be childs of qmp ?
>>>>
>>>> Hmm... maybe I was recalling the DSI phy binding. I think there
>>>> are lanes there too but we decided to just have one node.
>>>>
>>>>>
>>>>> "phy as a bus" (just trying to understand here) -
>>>>> let's say a usb phy controller has one HSIC phy port and one USB2 phy port.
>>>>> So, should this phy controller be a bus providing two ports (and so
>>>>> we will have
>>>>> couple of child nodes to the phy controller) ?
>>>>>
>>>>
>>>> Typically in DT a subnode or collection of subnodes means there's
>>>> some sort of bus involved. Usually each node corresponds to a
>>>> struct device, and the parent node corresponds to the bus or
>>>> controller for the logical bus.
>>>>
>>>> In this case (only PCIe though? not UFS or USB?) it seems like we
>>>> have multiple phys that share a common register space, but
>>>> otherwise they have their own register space and power
>>>> management. Would you have each PCIe controller point to a
>>>> different subnode for their associated phy? I'm trying to
>>>> understand the benefit of the subnodes if they aren't treated as
>>>> struct devices.
>>>
>>> Yes, instead of having all the controller having a phandle to the same PHY and
>>> then using other mechanisms to differentiate between the PHYs, each controller
>>> can have a phandle to the exact port that it is connected to.
>>>
>>> This also gives a better representation of the hardware and can avoid lot of
>>> boilerplate code in the driver.
>>
>> Below is one binding that works for me.
>> --------------------
>>                phy@34000 {
>>                         compatible = "qcom,msm8996-qmp-pcie-phy";
>>                         reg = <0x034000 0x488>;
>>                         #clock-cells = <1>;
>>                         #address-cells = <1>;
>>                         #size-cells = <1>;
>>                         ranges;
>>
>>                         clocks = <&gcc GCC_PCIE_PHY_AUX_CLK>,
>>                                 <&gcc GCC_PCIE_PHY_CFG_AHB_CLK>,
>>                                 <&gcc GCC_PCIE_CLKREF_CLK>;
>>                         clock-names = "aux", "cfg_ahb", "ref";
>>
>>                         vdda-phy-supply = <&pm8994_l28>;
>>                         vdda-pll-supply = <&pm8994_l12>;
>>
>>                         resets = <&gcc GCC_PCIE_PHY_BCR>,
>>                                 <&gcc GCC_PCIE_PHY_COM_BCR>,
>>                                 <&gcc GCC_PCIE_PHY_COM_NOCSR_BCR>;
>>                         reset-names = "phy", "common", "cfg";
>>
>>                         pciephy_p0: port@0 {
>>                                 reg = <0x035000 0x130>,
>>                                         <0x035200 0x200>,
>>                                         <0x035400 0x1dc>;
>>                                 #phy-cells = <0>;
>>
>>                                 clocks = <&gcc GCC_PCIE_0_PIPE_CLK>;
>>                                 clock-names = "pipe0";
>>                                 resets = <&gcc GCC_PCIE_0_PHY_BCR>;
>>                                 reset-names = "lane0";
>>                         };
>>
>>                       pciephy_p1: port@1 {
>>                                 reg = <0x036000 0x130>,
>>                                         <0x036200 0x200>,
>>                                         <0x036400 0x1dc>;
>>                                 #phy-cells = <0>;
>>
>>                                 clocks = <&gcc GCC_PCIE_1_PIPE_CLK>;
>>                                 clock-names = "pipe1";
>>                                 resets = <&gcc GCC_PCIE_1_PHY_BCR>;
>>                                 reset-names = "lane1";
>>                         };
>>
>>                         pciephy_p2: port@2 {
>>                                 reg = <0x037000 0x130>,
>>                                         <0x037200 0x200>,
>>                                         <0x037400 0x1dc>;
>>                                 #phy-cells = <0>;
>>
>>                                 clocks = <&gcc GCC_PCIE_2_PIPE_CLK>;
>>                                 clock-names = "pipe2";
>>                                 resets = <&gcc GCC_PCIE_2_PHY_BCR>;
>>                                 reset-names = "lane2";
>>                         };
>>                 };
>> --------------------
>>
>> let me know if this looks okay.
>
> looks good to me.

Great.

Rob, please let me know if above bindings look okay to you.
I can re-spin the bindings patch then.

Thanks

Regards
Vivek

>
> Thanks
> Kishon
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v4 3/4] dt-bindings: phy: Add support for QMP phy

[Stephen Boyd]

On 01/24, Vivek Gautam wrote:
> 
> Below is one binding that works for me.
> --------------------
>                phy@34000 {
>                         compatible = "qcom,msm8996-qmp-pcie-phy";
>                         reg = <0x034000 0x488>;
>                         #clock-cells = <1>;
>                         #address-cells = <1>;
>                         #size-cells = <1>;
>                         ranges;
> 
>                         clocks = <&gcc GCC_PCIE_PHY_AUX_CLK>,
>                                 <&gcc GCC_PCIE_PHY_CFG_AHB_CLK>,
>                                 <&gcc GCC_PCIE_CLKREF_CLK>;
>                         clock-names = "aux", "cfg_ahb", "ref";
> 
>                         vdda-phy-supply = <&pm8994_l28>;
>                         vdda-pll-supply = <&pm8994_l12>;
> 
>                         resets = <&gcc GCC_PCIE_PHY_BCR>,
>                                 <&gcc GCC_PCIE_PHY_COM_BCR>,
>                                 <&gcc GCC_PCIE_PHY_COM_NOCSR_BCR>;
>                         reset-names = "phy", "common", "cfg";
> 
>                         pciephy_p0: port@0 {

The unit address '@0' should be replaced with something from the
reg properties. 

Also 'port' and 'ports' are almost keywords in DT now with the
graph binding so we need to be careful when using them.

>                                 reg = <0x035000 0x130>,
>                                         <0x035200 0x200>,
>                                         <0x035400 0x1dc>;
>                                 #phy-cells = <0>;
> 
>                                 clocks = <&gcc GCC_PCIE_0_PIPE_CLK>;
>                                 clock-names = "pipe0";
>                                 resets = <&gcc GCC_PCIE_0_PHY_BCR>;
>                                 reset-names = "lane0";
>                         };
> 
>                       pciephy_p1: port@1 {
>                                 reg = <0x036000 0x130>,
>                                         <0x036200 0x200>,
>                                         <0x036400 0x1dc>;
>                                 #phy-cells = <0>;
> 
>                                 clocks = <&gcc GCC_PCIE_1_PIPE_CLK>;
>                                 clock-names = "pipe1";
>                                 resets = <&gcc GCC_PCIE_1_PHY_BCR>;
>                                 reset-names = "lane1";
>                         };
> 
>                         pciephy_p2: port@2 {
>                                 reg = <0x037000 0x130>,
>                                         <0x037200 0x200>,
>                                         <0x037400 0x1dc>;
>                                 #phy-cells = <0>;
> 
>                                 clocks = <&gcc GCC_PCIE_2_PIPE_CLK>;
>                                 clock-names = "pipe2";
>                                 resets = <&gcc GCC_PCIE_2_PHY_BCR>;
>                                 reset-names = "lane2";
>                         };
>                 };
> --------------------
> 
> let me know if this looks okay.
> 
> 

What's the plan for non-pcie qmp phy binding? In that case we
don't have ports, so it gets folded into one node?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v4 3/4] dt-bindings: phy: Add support for QMP phy

[Vivek Gautam]

On 01/27/2017 05:13 AM, Stephen Boyd wrote:
> On 01/24, Vivek Gautam wrote:
>> Below is one binding that works for me.
>> --------------------
>>                 phy@34000 {
>>                          compatible = "qcom,msm8996-qmp-pcie-phy";
>>                          reg = <0x034000 0x488>;
>>                          #clock-cells = <1>;
>>                          #address-cells = <1>;
>>                          #size-cells = <1>;
>>                          ranges;
>>
>>                          clocks = <&gcc GCC_PCIE_PHY_AUX_CLK>,
>>                                  <&gcc GCC_PCIE_PHY_CFG_AHB_CLK>,
>>                                  <&gcc GCC_PCIE_CLKREF_CLK>;
>>                          clock-names = "aux", "cfg_ahb", "ref";
>>
>>                          vdda-phy-supply = <&pm8994_l28>;
>>                          vdda-pll-supply = <&pm8994_l12>;
>>
>>                          resets = <&gcc GCC_PCIE_PHY_BCR>,
>>                                  <&gcc GCC_PCIE_PHY_COM_BCR>,
>>                                  <&gcc GCC_PCIE_PHY_COM_NOCSR_BCR>;
>>                          reset-names = "phy", "common", "cfg";
>>
>>                          pciephy_p0: port@0 {
> The unit address '@0' should be replaced with something from the
> reg properties.

Sure, will take care of this.

>   
>
> Also 'port' and 'ports' are almost keywords in DT now with the
> graph binding so we need to be careful when using them.

 From "./Documentation/devicetree/bindings/graph.txt" -
"The device tree graph bindings described herein abstract more complex
devices that can have multiple specifiable ports, each of which can be
linked to one or more ports of other devices."

So, this means we use 'port', 'ports' and 'endpoint' for devices whose one
or more ports is connected to other device's one or more ports.

I can use 'lane' for the node name here.

>
>>                                  reg = <0x035000 0x130>,
>>                                          <0x035200 0x200>,
>>                                          <0x035400 0x1dc>;
>>                                  #phy-cells = <0>;
>>
>>                                  clocks = <&gcc GCC_PCIE_0_PIPE_CLK>;
>>                                  clock-names = "pipe0";
>>                                  resets = <&gcc GCC_PCIE_0_PHY_BCR>;
>>                                  reset-names = "lane0";
>>                          };
>>
>>                        pciephy_p1: port@1 {
>>                                  reg = <0x036000 0x130>,
>>                                          <0x036200 0x200>,
>>                                          <0x036400 0x1dc>;
>>                                  #phy-cells = <0>;
>>
>>                                  clocks = <&gcc GCC_PCIE_1_PIPE_CLK>;
>>                                  clock-names = "pipe1";
>>                                  resets = <&gcc GCC_PCIE_1_PHY_BCR>;
>>                                  reset-names = "lane1";
>>                          };
>>
>>                          pciephy_p2: port@2 {
>>                                  reg = <0x037000 0x130>,
>>                                          <0x037200 0x200>,
>>                                          <0x037400 0x1dc>;
>>                                  #phy-cells = <0>;
>>
>>                                  clocks = <&gcc GCC_PCIE_2_PIPE_CLK>;
>>                                  clock-names = "pipe2";
>>                                  resets = <&gcc GCC_PCIE_2_PHY_BCR>;
>>                                  reset-names = "lane2";
>>                          };
>>                  };
>> --------------------
>>
>> let me know if this looks okay.
>>
>>
> What's the plan for non-pcie qmp phy binding? In that case we
> don't have ports, so it gets folded into one node?
>
The non-pcie qmp phys still have one lane, that provides tx/rx.

I am of the opinion that we don't have two different ways to create
phys in the driver, and keep one port/lane for such phys in dt.


Regards
Vivek

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v4 3/4] dt-bindings: phy: Add support for QMP phy

[Stephen Boyd]

(Not sure I replied so here it is)

On 01/27, Vivek Gautam wrote:
> 
> 
> On 01/27/2017 05:13 AM, Stephen Boyd wrote:
> >On 01/24, Vivek Gautam wrote:
> 
> From "./Documentation/devicetree/bindings/graph.txt" -
> "The device tree graph bindings described herein abstract more complex
> devices that can have multiple specifiable ports, each of which can be
> linked to one or more ports of other devices."
> 
> So, this means we use 'port', 'ports' and 'endpoint' for devices whose one
> or more ports is connected to other device's one or more ports.
> 
> I can use 'lane' for the node name here.

Ok.

> 
> >
> >>                                 reg = <0x035000 0x130>,
> >>                                         <0x035200 0x200>,
> >>                                         <0x035400 0x1dc>;
> >>                                 #phy-cells = <0>;
> >>
> >>                                 clocks = <&gcc GCC_PCIE_0_PIPE_CLK>;
> >>                                 clock-names = "pipe0";
> >>                                 resets = <&gcc GCC_PCIE_0_PHY_BCR>;
> >>                                 reset-names = "lane0";
> >>                         };
> >>
> >>                       pciephy_p1: port@1 {
> >>                                 reg = <0x036000 0x130>,
> >>                                         <0x036200 0x200>,
> >>                                         <0x036400 0x1dc>;
> >>                                 #phy-cells = <0>;
> >>
> >>                                 clocks = <&gcc GCC_PCIE_1_PIPE_CLK>;
> >>                                 clock-names = "pipe1";
> >>                                 resets = <&gcc GCC_PCIE_1_PHY_BCR>;
> >>                                 reset-names = "lane1";
> >>                         };
> >>
> >>                         pciephy_p2: port@2 {
> >>                                 reg = <0x037000 0x130>,
> >>                                         <0x037200 0x200>,
> >>                                         <0x037400 0x1dc>;
> >>                                 #phy-cells = <0>;
> >>
> >>                                 clocks = <&gcc GCC_PCIE_2_PIPE_CLK>;
> >>                                 clock-names = "pipe2";
> >>                                 resets = <&gcc GCC_PCIE_2_PHY_BCR>;
> >>                                 reset-names = "lane2";
> >>                         };
> >>                 };
> >>--------------------
> >>
> >>let me know if this looks okay.
> >>
> >>
> >What's the plan for non-pcie qmp phy binding? In that case we
> >don't have ports, so it gets folded into one node?
> >
> The non-pcie qmp phys still have one lane, that provides tx/rx.
> 
> I am of the opinion that we don't have two different ways to create
> phys in the driver, and keep one port/lane for such phys in dt.
> 

Ok so we would still have a subnode in that case. Sounds ok.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v4 3/4] dt-bindings: phy: Add support for QMP phy

[Vivek Gautam]

On 03/07/2017 07:30 PM, Stephen Boyd wrote:
> (Not sure I replied so here it is)
>
> On 01/27, Vivek Gautam wrote:
>>
>> On 01/27/2017 05:13 AM, Stephen Boyd wrote:
>>> On 01/24, Vivek Gautam wrote:
>>  From "./Documentation/devicetree/bindings/graph.txt" -
>> "The device tree graph bindings described herein abstract more complex
>> devices that can have multiple specifiable ports, each of which can be
>> linked to one or more ports of other devices."
>>
>> So, this means we use 'port', 'ports' and 'endpoint' for devices whose one
>> or more ports is connected to other device's one or more ports.
>>
>> I can use 'lane' for the node name here.
> Ok.
>
>>>>                                  reg = <0x035000 0x130>,
>>>>                                          <0x035200 0x200>,
>>>>                                          <0x035400 0x1dc>;
>>>>                                  #phy-cells = <0>;
>>>>
>>>>                                  clocks = <&gcc GCC_PCIE_0_PIPE_CLK>;
>>>>                                  clock-names = "pipe0";
>>>>                                  resets = <&gcc GCC_PCIE_0_PHY_BCR>;
>>>>                                  reset-names = "lane0";
>>>>                          };
>>>>
>>>>                        pciephy_p1: port@1 {
>>>>                                  reg = <0x036000 0x130>,
>>>>                                          <0x036200 0x200>,
>>>>                                          <0x036400 0x1dc>;
>>>>                                  #phy-cells = <0>;
>>>>
>>>>                                  clocks = <&gcc GCC_PCIE_1_PIPE_CLK>;
>>>>                                  clock-names = "pipe1";
>>>>                                  resets = <&gcc GCC_PCIE_1_PHY_BCR>;
>>>>                                  reset-names = "lane1";
>>>>                          };
>>>>
>>>>                          pciephy_p2: port@2 {
>>>>                                  reg = <0x037000 0x130>,
>>>>                                          <0x037200 0x200>,
>>>>                                          <0x037400 0x1dc>;
>>>>                                  #phy-cells = <0>;
>>>>
>>>>                                  clocks = <&gcc GCC_PCIE_2_PIPE_CLK>;
>>>>                                  clock-names = "pipe2";
>>>>                                  resets = <&gcc GCC_PCIE_2_PHY_BCR>;
>>>>                                  reset-names = "lane2";
>>>>                          };
>>>>                  };
>>>> --------------------
>>>>
>>>> let me know if this looks okay.
>>>>
>>>>
>>> What's the plan for non-pcie qmp phy binding? In that case we
>>> don't have ports, so it gets folded into one node?
>>>
>> The non-pcie qmp phys still have one lane, that provides tx/rx.
>>
>> I am of the opinion that we don't have two different ways to create
>> phys in the driver, and keep one port/lane for such phys in dt.
>>
> Ok so we would still have a subnode in that case. Sounds ok.

Cool.

Thanks
Vivek

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v4 4/4] phy: qcom-qmp: new qmp phy driver for qcom-chipsets

[Vivek Gautam]

Qualcomm SOCs have QMP phy controller that provides support
to a number of controller, viz. PCIe, UFS, and USB.
Add a new driver, based on generic phy framework, for this
phy controller.

Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
Tested-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---

Changes since v3:
 - Renamed 'struct qcom_qmp_phy' to 'struct qcom_qmp' and
   'struct qmp_phy_desc' to 'struct qmp_phy' to avoid any confusion
   in distinguishing between QMP phy block and per-lane phy which is
   the actual phy in Linux eyes (suggested by Bjorn Andersson).
 - Made error labels more idiomatic.
 - Modified status checking for phy pcs.
 - Fixed power_down_delay check.
 - Refactored phy_pipe_clk_register() to register the pipe clock source
   using devm_clk_hw_register() (suggested by Stephen).
 - qcom_qmp_phy_xlate() function:
   - Removed unnecessary 'for loop'.
   - Added additional check for '0' or -ve args_count.
 - Fixed the mixed tabs and spaces in pipe_clk_src diagram.
 - Removed instances of memset() since we use snprintf() for the
   buffers.
 - Refactored qphy_setbits() and qphy_clrbits() a little bit to accept
   base address and register offset as two separate arguments.

Changes since v2:
 - Removed selecting 'RESET_CONTROLLER' config.
 - Added error handling for clk_prepare_enable paths.
 - Removed 'ref_clk_src' handling. Driver doesn't need to request and
   handle this clock.
 - Using readl_poll_timeout() to simplify pcs ready status polling.
   Also fixed the polling condition for pcs block ready status:
   'Common block ready status bit is set on phy init completion, while
   PCS block ready status bit (PHYSTATUS) is reset on phy init
   completion.'
 - Moved out the per-lane phy creation from probe() to separate
   function.
 - Registering pipe clock source as a fixed rate clock that comes
   out of the PLL block of QMP phy. These source clocks serve as
   parent to 'pipe_clks' that are requested by pcie or usb3 phys.
 - Using of_device_get_match_data() to get match data.
 - Fixed sparse warnings for 'static' and 'const'.
 - Using shorter variable names in structure and in functions.
 - Handling various comment style shortcomings.

Changes since v1:
 - Fixed missing mutex_unlock() calls in error cases, reported by
   Julia Lawall.
 - Selecting CONFIG_RESET_CONTROLLER when this driver is enabled.
 - Added a boolean property to check if the phy has individual lane
   reset available.
 - Took care or EPROBE_DEFER, dev_vdbg() and other minor nits.
 - Removed references to non-lkml links from commit message.
 - Moved to use separate iomem resources for each lanes.
   Tx, Rx and PCS offsets per lane can now come from dt bindings.

 drivers/phy/Kconfig        |    8 +
 drivers/phy/Makefile       |    1 +
 drivers/phy/phy-qcom-qmp.c | 1206 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1215 insertions(+)
 create mode 100644 drivers/phy/phy-qcom-qmp.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 0ed53d018b23..54296397452c 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -430,6 +430,14 @@ config PHY_STIH407_USB
 	  Enable this support to enable the picoPHY device used by USB2
 	  and USB3 controllers on STMicroelectronics STiH407 SoC families.
 
+config PHY_QCOM_QMP
+	tristate "Qualcomm QMP PHY Driver"
+	depends on OF && (ARCH_QCOM || COMPILE_TEST)
+	select GENERIC_PHY
+	help
+	  Enable this to support the QMP PHY transceiver that is used
+	  with controllers such as PCIe, UFS, and USB on Qualcomm chips.
+
 config PHY_QCOM_QUSB2
 	tristate "Qualcomm QUSB2 PHY Driver"
 	depends on OF && (ARCH_QCOM || COMPILE_TEST)
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index dad1682b80e3..dbe7731bbca9 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -50,6 +50,7 @@ obj-$(CONFIG_PHY_ST_SPEAR1340_MIPHY)	+= phy-spear1340-miphy.o
 obj-$(CONFIG_PHY_XGENE)			+= phy-xgene.o
 obj-$(CONFIG_PHY_STIH407_USB)		+= phy-stih407-usb.o
 obj-$(CONFIG_PHY_QCOM_QUSB2) 	+= phy-qcom-qusb2.o
+obj-$(CONFIG_PHY_QCOM_QMP) 	+= phy-qcom-qmp.o
 obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs.o
 obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs-qmp-20nm.o
 obj-$(CONFIG_PHY_QCOM_UFS) 	+= phy-qcom-ufs-qmp-14nm.o
diff --git a/drivers/phy/phy-qcom-qmp.c b/drivers/phy/phy-qcom-qmp.c
new file mode 100644
index 000000000000..cff01c815c20
--- /dev/null
+++ b/drivers/phy/phy-qcom-qmp.c
@@ -0,0 +1,1206 @@
+/*
+ * Copyright (c) 2016, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#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/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>
+
+/* QMP PHY QSERDES COM registers */
+#define QSERDES_COM_BG_TIMER				0x00c
+#define QSERDES_COM_SSC_EN_CENTER			0x010
+#define QSERDES_COM_SSC_ADJ_PER1			0x014
+#define QSERDES_COM_SSC_ADJ_PER2			0x018
+#define QSERDES_COM_SSC_PER1				0x01c
+#define QSERDES_COM_SSC_PER2				0x020
+#define QSERDES_COM_SSC_STEP_SIZE1			0x024
+#define QSERDES_COM_SSC_STEP_SIZE2			0x028
+#define QSERDES_COM_BIAS_EN_CLKBUFLR_EN			0x034
+#define QSERDES_COM_CLK_ENABLE1				0x038
+#define QSERDES_COM_SYS_CLK_CTRL			0x03c
+#define QSERDES_COM_SYSCLK_BUF_ENABLE			0x040
+#define QSERDES_COM_PLL_IVCO				0x048
+#define QSERDES_COM_LOCK_CMP1_MODE0			0x04c
+#define QSERDES_COM_LOCK_CMP2_MODE0			0x050
+#define QSERDES_COM_LOCK_CMP3_MODE0			0x054
+#define QSERDES_COM_LOCK_CMP1_MODE1			0x058
+#define QSERDES_COM_LOCK_CMP2_MODE1			0x05c
+#define QSERDES_COM_LOCK_CMP3_MODE1			0x060
+#define QSERDES_COM_BG_TRIM				0x070
+#define QSERDES_COM_CLK_EP_DIV				0x074
+#define QSERDES_COM_CP_CTRL_MODE0			0x078
+#define QSERDES_COM_CP_CTRL_MODE1			0x07c
+#define QSERDES_COM_PLL_RCTRL_MODE0			0x084
+#define QSERDES_COM_PLL_RCTRL_MODE1			0x088
+#define QSERDES_COM_PLL_CCTRL_MODE0			0x090
+#define QSERDES_COM_PLL_CCTRL_MODE1			0x094
+#define QSERDES_COM_SYSCLK_EN_SEL			0x0ac
+#define QSERDES_COM_RESETSM_CNTRL			0x0b4
+#define QSERDES_COM_RESTRIM_CTRL			0x0bc
+#define QSERDES_COM_RESCODE_DIV_NUM			0x0c4
+#define QSERDES_COM_LOCK_CMP_EN				0x0c8
+#define QSERDES_COM_LOCK_CMP_CFG			0x0cc
+#define QSERDES_COM_DEC_START_MODE0			0x0d0
+#define QSERDES_COM_DEC_START_MODE1			0x0d4
+#define QSERDES_COM_DIV_FRAC_START1_MODE0		0x0dc
+#define QSERDES_COM_DIV_FRAC_START2_MODE0		0x0e0
+#define QSERDES_COM_DIV_FRAC_START3_MODE0		0x0e4
+#define QSERDES_COM_DIV_FRAC_START1_MODE1		0x0e8
+#define QSERDES_COM_DIV_FRAC_START2_MODE1		0x0ec
+#define QSERDES_COM_DIV_FRAC_START3_MODE1		0x0f0
+#define QSERDES_COM_INTEGLOOP_GAIN0_MODE0		0x108
+#define QSERDES_COM_INTEGLOOP_GAIN1_MODE0		0x10c
+#define QSERDES_COM_INTEGLOOP_GAIN0_MODE1		0x110
+#define QSERDES_COM_INTEGLOOP_GAIN1_MODE1		0x114
+#define QSERDES_COM_VCO_TUNE_CTRL			0x124
+#define QSERDES_COM_VCO_TUNE_MAP			0x128
+#define QSERDES_COM_VCO_TUNE1_MODE0			0x12c
+#define QSERDES_COM_VCO_TUNE2_MODE0			0x130
+#define QSERDES_COM_VCO_TUNE1_MODE1			0x134
+#define QSERDES_COM_VCO_TUNE2_MODE1			0x138
+#define QSERDES_COM_VCO_TUNE_TIMER1			0x144
+#define QSERDES_COM_VCO_TUNE_TIMER2			0x148
+#define QSERDES_COM_BG_CTRL				0x170
+#define QSERDES_COM_CLK_SELECT				0x174
+#define QSERDES_COM_HSCLK_SEL				0x178
+#define QSERDES_COM_CORECLK_DIV				0x184
+#define QSERDES_COM_CORE_CLK_EN				0x18c
+#define QSERDES_COM_C_READY_STATUS			0x190
+#define QSERDES_COM_CMN_CONFIG				0x194
+#define QSERDES_COM_SVS_MODE_CLK_SEL			0x19c
+#define QSERDES_COM_DEBUG_BUS0				0x1a0
+#define QSERDES_COM_DEBUG_BUS1				0x1a4
+#define QSERDES_COM_DEBUG_BUS2				0x1a8
+#define QSERDES_COM_DEBUG_BUS3				0x1ac
+#define QSERDES_COM_DEBUG_BUS_SEL			0x1b0
+#define QSERDES_COM_CORECLK_DIV_MODE1			0x1bc
+
+/* QMP PHY TX registers */
+#define QSERDES_TX_RES_CODE_LANE_OFFSET			0x054
+#define QSERDES_TX_DEBUG_BUS_SEL			0x064
+#define QSERDES_TX_HIGHZ_TRANSCEIVEREN_BIAS_DRVR_EN	0x068
+#define QSERDES_TX_LANE_MODE				0x094
+#define QSERDES_TX_RCV_DETECT_LVL_2			0x0ac
+
+/* QMP PHY RX registers */
+#define QSERDES_RX_UCDR_SO_GAIN_HALF			0x010
+#define QSERDES_RX_UCDR_SO_GAIN				0x01c
+#define QSERDES_RX_UCDR_FASTLOCK_FO_GAIN		0x040
+#define QSERDES_RX_UCDR_SO_SATURATION_AND_ENABLE	0x048
+#define QSERDES_RX_RX_TERM_BW				0x090
+#define QSERDES_RX_RX_EQ_GAIN1_LSB			0x0c4
+#define QSERDES_RX_RX_EQ_GAIN1_MSB			0x0c8
+#define QSERDES_RX_RX_EQ_GAIN2_LSB			0x0cc
+#define QSERDES_RX_RX_EQ_GAIN2_MSB			0x0d0
+#define QSERDES_RX_RX_EQU_ADAPTOR_CNTRL2		0x0d8
+#define QSERDES_RX_RX_EQU_ADAPTOR_CNTRL3		0x0dc
+#define QSERDES_RX_RX_EQU_ADAPTOR_CNTRL4		0x0e0
+#define QSERDES_RX_RX_EQ_OFFSET_ADAPTOR_CNTRL1		0x108
+#define QSERDES_RX_RX_OFFSET_ADAPTOR_CNTRL2		0x10c
+#define QSERDES_RX_SIGDET_ENABLES			0x110
+#define QSERDES_RX_SIGDET_CNTRL				0x114
+#define QSERDES_RX_SIGDET_LVL				0x118
+#define QSERDES_RX_SIGDET_DEGLITCH_CNTRL		0x11c
+#define QSERDES_RX_RX_BAND				0x120
+#define QSERDES_RX_RX_INTERFACE_MODE			0x12c
+
+/* QMP PHY PCS registers */
+#define QPHY_SW_RESET					0x00
+#define QPHY_POWER_DOWN_CONTROL				0x04
+#define QPHY_START_CTRL					0x08
+#define QPHY_TXDEEMPH_M6DB_V0				0x24
+#define QPHY_TXDEEMPH_M3P5DB_V0				0x28
+#define QPHY_ENDPOINT_REFCLK_DRIVE			0x54
+#define QPHY_RX_IDLE_DTCT_CNTRL				0x58
+#define QPHY_POWER_STATE_CONFIG1			0x60
+#define QPHY_POWER_STATE_CONFIG2			0x64
+#define QPHY_POWER_STATE_CONFIG4			0x6c
+#define QPHY_LOCK_DETECT_CONFIG1			0x80
+#define QPHY_LOCK_DETECT_CONFIG2			0x84
+#define QPHY_LOCK_DETECT_CONFIG3			0x88
+#define QPHY_PWRUP_RESET_DLY_TIME_AUXCLK		0xa0
+#define QPHY_LP_WAKEUP_DLY_TIME_AUXCLK			0xa4
+
+/* QPHY_SW_RESET bit */
+#define SW_RESET				BIT(0)
+/* QPHY_POWER_DOWN_CONTROL */
+#define SW_PWRDN				BIT(0)
+#define REFCLK_DRV_DSBL				BIT(1)
+/* QPHY_START_CONTROL bits */
+#define SERDES_START				BIT(0)
+#define PCS_START				BIT(1)
+#define PLL_READY_GATE_EN			BIT(3)
+/* QPHY_PCS_STATUS bit */
+#define PHYSTATUS				BIT(6)
+/* QPHY_COM_PCS_READY_STATUS bit */
+#define PCS_READY				BIT(0)
+
+#define PHY_INIT_COMPLETE_TIMEOUT		1000
+#define POWER_DOWN_DELAY_US_MIN			10
+#define POWER_DOWN_DELAY_US_MAX			11
+
+#define MAX_PROP_NAME				32
+
+struct qmp_phy_init_tbl {
+	unsigned int offset;
+	unsigned int val;
+	/*
+	 * register part of layout ?
+	 * if yes, then offset gives index in the reg-layout
+	 */
+	int in_layout;
+};
+#define QMP_PHY_INIT_CFG(o, v)		\
+	{				\
+		.offset = o,		\
+		.val = v,		\
+	}
+#define QMP_PHY_INIT_CFG_L(o, v)	\
+	{				\
+		.offset = o,		\
+		.val = v,		\
+		.in_layout = 1,		\
+	}
+
+/* set of registers with offsets different per-PHY */
+enum qphy_reg_layout {
+	/* Common block control registers */
+	QPHY_COM_SW_RESET,
+	QPHY_COM_POWER_DOWN_CONTROL,
+	QPHY_COM_START_CONTROL,
+	QPHY_COM_PCS_READY_STATUS,
+	/* PCS registers */
+	QPHY_PLL_LOCK_CHK_DLY_TIME,
+	QPHY_FLL_CNTRL1,
+	QPHY_FLL_CNTRL2,
+	QPHY_FLL_CNT_VAL_L,
+	QPHY_FLL_CNT_VAL_H_TOL,
+	QPHY_FLL_MAN_CODE,
+	QPHY_PCS_READY_STATUS,
+};
+
+static const unsigned int pciephy_regs_layout[] = {
+	[QPHY_COM_SW_RESET]		= 0x400,
+	[QPHY_COM_POWER_DOWN_CONTROL]	= 0x404,
+	[QPHY_COM_START_CONTROL]	= 0x408,
+	[QPHY_COM_PCS_READY_STATUS]	= 0x448,
+	[QPHY_PLL_LOCK_CHK_DLY_TIME]	= 0xa8,
+	[QPHY_FLL_CNTRL1]		= 0xc4,
+	[QPHY_FLL_CNTRL2]		= 0xc8,
+	[QPHY_FLL_CNT_VAL_L]		= 0xcc,
+	[QPHY_FLL_CNT_VAL_H_TOL]	= 0xd0,
+	[QPHY_FLL_MAN_CODE]		= 0xd4,
+	[QPHY_PCS_READY_STATUS]		= 0x174,
+};
+
+static const unsigned int usb3phy_regs_layout[] = {
+	[QPHY_FLL_CNTRL1]		= 0xc0,
+	[QPHY_FLL_CNTRL2]		= 0xc4,
+	[QPHY_FLL_CNT_VAL_L]		= 0xc8,
+	[QPHY_FLL_CNT_VAL_H_TOL]	= 0xcc,
+	[QPHY_FLL_MAN_CODE]		= 0xd0,
+	[QPHY_PCS_READY_STATUS]		= 0x17c,
+};
+
+static const struct qmp_phy_init_tbl pciephy_serdes_tbl[] = {
+	QMP_PHY_INIT_CFG(QSERDES_COM_BIAS_EN_CLKBUFLR_EN, 0x1c),
+	QMP_PHY_INIT_CFG(QSERDES_COM_CLK_ENABLE1, 0x10),
+	QMP_PHY_INIT_CFG(QSERDES_COM_CLK_SELECT, 0x33),
+	QMP_PHY_INIT_CFG(QSERDES_COM_CMN_CONFIG, 0x06),
+	QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP_EN, 0x42),
+	QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_MAP, 0x00),
+	QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_TIMER1, 0xff),
+	QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_TIMER2, 0x1f),
+	QMP_PHY_INIT_CFG(QSERDES_COM_HSCLK_SEL, 0x01),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SVS_MODE_CLK_SEL, 0x01),
+	QMP_PHY_INIT_CFG(QSERDES_COM_CORE_CLK_EN, 0x00),
+	QMP_PHY_INIT_CFG(QSERDES_COM_CORECLK_DIV, 0x0a),
+	QMP_PHY_INIT_CFG(QSERDES_COM_BG_TIMER, 0x09),
+	QMP_PHY_INIT_CFG(QSERDES_COM_DEC_START_MODE0, 0x82),
+	QMP_PHY_INIT_CFG(QSERDES_COM_DIV_FRAC_START3_MODE0, 0x03),
+	QMP_PHY_INIT_CFG(QSERDES_COM_DIV_FRAC_START2_MODE0, 0x55),
+	QMP_PHY_INIT_CFG(QSERDES_COM_DIV_FRAC_START1_MODE0, 0x55),
+	QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP3_MODE0, 0x00),
+	QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP2_MODE0, 0x1a),
+	QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP1_MODE0, 0x0a),
+	QMP_PHY_INIT_CFG(QSERDES_COM_CLK_SELECT, 0x33),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SYS_CLK_CTRL, 0x02),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SYSCLK_BUF_ENABLE, 0x1f),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SYSCLK_EN_SEL, 0x04),
+	QMP_PHY_INIT_CFG(QSERDES_COM_CP_CTRL_MODE0, 0x0b),
+	QMP_PHY_INIT_CFG(QSERDES_COM_PLL_RCTRL_MODE0, 0x16),
+	QMP_PHY_INIT_CFG(QSERDES_COM_PLL_CCTRL_MODE0, 0x28),
+	QMP_PHY_INIT_CFG(QSERDES_COM_INTEGLOOP_GAIN1_MODE0, 0x00),
+	QMP_PHY_INIT_CFG(QSERDES_COM_INTEGLOOP_GAIN0_MODE0, 0x80),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_EN_CENTER, 0x01),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_PER1, 0x31),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_PER2, 0x01),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_ADJ_PER1, 0x02),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_ADJ_PER2, 0x00),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_STEP_SIZE1, 0x2f),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_STEP_SIZE2, 0x19),
+	QMP_PHY_INIT_CFG(QSERDES_COM_RESCODE_DIV_NUM, 0x15),
+	QMP_PHY_INIT_CFG(QSERDES_COM_BG_TRIM, 0x0f),
+	QMP_PHY_INIT_CFG(QSERDES_COM_PLL_IVCO, 0x0f),
+	QMP_PHY_INIT_CFG(QSERDES_COM_CLK_EP_DIV, 0x19),
+	QMP_PHY_INIT_CFG(QSERDES_COM_CLK_ENABLE1, 0x10),
+	QMP_PHY_INIT_CFG(QSERDES_COM_HSCLK_SEL, 0x00),
+	QMP_PHY_INIT_CFG(QSERDES_COM_RESCODE_DIV_NUM, 0x40),
+};
+
+static const struct qmp_phy_init_tbl pciephy_tx_tbl[] = {
+	QMP_PHY_INIT_CFG(QSERDES_TX_HIGHZ_TRANSCEIVEREN_BIAS_DRVR_EN, 0x45),
+	QMP_PHY_INIT_CFG(QSERDES_TX_LANE_MODE, 0x06),
+};
+
+static const struct qmp_phy_init_tbl pciephy_rx_tbl[] = {
+	QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_ENABLES, 0x1c),
+	QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL2, 0x01),
+	QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL3, 0x00),
+	QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL4, 0xdb),
+	QMP_PHY_INIT_CFG(QSERDES_RX_RX_BAND, 0x18),
+	QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_SO_GAIN, 0x04),
+	QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_SO_GAIN_HALF, 0x04),
+	QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_SO_SATURATION_AND_ENABLE, 0x4b),
+	QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_DEGLITCH_CNTRL, 0x14),
+	QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_LVL, 0x19),
+};
+
+static const struct qmp_phy_init_tbl pciephy_pcs_tbl[] = {
+	QMP_PHY_INIT_CFG(QPHY_RX_IDLE_DTCT_CNTRL, 0x4c),
+	QMP_PHY_INIT_CFG(QPHY_PWRUP_RESET_DLY_TIME_AUXCLK, 0x00),
+	QMP_PHY_INIT_CFG(QPHY_LP_WAKEUP_DLY_TIME_AUXCLK, 0x01),
+
+	QMP_PHY_INIT_CFG_L(QPHY_PLL_LOCK_CHK_DLY_TIME, 0x05),
+
+	QMP_PHY_INIT_CFG(QPHY_ENDPOINT_REFCLK_DRIVE, 0x05),
+	QMP_PHY_INIT_CFG(QPHY_POWER_DOWN_CONTROL, 0x02),
+	QMP_PHY_INIT_CFG(QPHY_POWER_STATE_CONFIG4, 0x00),
+	QMP_PHY_INIT_CFG(QPHY_POWER_STATE_CONFIG1, 0xa3),
+	QMP_PHY_INIT_CFG(QPHY_TXDEEMPH_M3P5DB_V0, 0x0e),
+};
+
+static const struct qmp_phy_init_tbl usb3phy_serdes_tbl[] = {
+	QMP_PHY_INIT_CFG(QSERDES_COM_SYSCLK_EN_SEL, 0x14),
+	QMP_PHY_INIT_CFG(QSERDES_COM_BIAS_EN_CLKBUFLR_EN, 0x08),
+	QMP_PHY_INIT_CFG(QSERDES_COM_CLK_SELECT, 0x30),
+	QMP_PHY_INIT_CFG(QSERDES_COM_CMN_CONFIG, 0x06),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SVS_MODE_CLK_SEL, 0x01),
+	QMP_PHY_INIT_CFG(QSERDES_COM_HSCLK_SEL, 0x00),
+	QMP_PHY_INIT_CFG(QSERDES_COM_BG_TRIM, 0x0f),
+	QMP_PHY_INIT_CFG(QSERDES_COM_PLL_IVCO, 0x0f),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SYS_CLK_CTRL, 0x04),
+	/* PLL and Loop filter settings */
+	QMP_PHY_INIT_CFG(QSERDES_COM_DEC_START_MODE0, 0x82),
+	QMP_PHY_INIT_CFG(QSERDES_COM_DIV_FRAC_START1_MODE0, 0x55),
+	QMP_PHY_INIT_CFG(QSERDES_COM_DIV_FRAC_START2_MODE0, 0x55),
+	QMP_PHY_INIT_CFG(QSERDES_COM_DIV_FRAC_START3_MODE0, 0x03),
+	QMP_PHY_INIT_CFG(QSERDES_COM_CP_CTRL_MODE0, 0x0b),
+	QMP_PHY_INIT_CFG(QSERDES_COM_PLL_RCTRL_MODE0, 0x16),
+	QMP_PHY_INIT_CFG(QSERDES_COM_PLL_CCTRL_MODE0, 0x28),
+	QMP_PHY_INIT_CFG(QSERDES_COM_INTEGLOOP_GAIN0_MODE0, 0x80),
+	QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_CTRL, 0x00),
+	QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP1_MODE0, 0x15),
+	QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP2_MODE0, 0x34),
+	QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP3_MODE0, 0x00),
+	QMP_PHY_INIT_CFG(QSERDES_COM_CORE_CLK_EN, 0x00),
+	QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP_CFG, 0x00),
+	QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_MAP, 0x00),
+	QMP_PHY_INIT_CFG(QSERDES_COM_BG_TIMER, 0x0a),
+	/* SSC settings */
+	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_EN_CENTER, 0x01),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_PER1, 0x31),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_PER2, 0x01),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_ADJ_PER1, 0x00),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_ADJ_PER2, 0x00),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_STEP_SIZE1, 0xde),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_STEP_SIZE2, 0x07),
+};
+
+static const struct qmp_phy_init_tbl usb3phy_tx_tbl[] = {
+	QMP_PHY_INIT_CFG(QSERDES_TX_HIGHZ_TRANSCEIVEREN_BIAS_DRVR_EN, 0x45),
+	QMP_PHY_INIT_CFG(QSERDES_TX_RCV_DETECT_LVL_2, 0x12),
+	QMP_PHY_INIT_CFG(QSERDES_TX_LANE_MODE, 0x06),
+};
+
+static const struct qmp_phy_init_tbl usb3phy_rx_tbl[] = {
+	QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_FASTLOCK_FO_GAIN, 0x0b),
+	QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_SO_GAIN, 0x04),
+	QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL2, 0x02),
+	QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL3, 0x4c),
+	QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL4, 0xbb),
+	QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQ_OFFSET_ADAPTOR_CNTRL1, 0x77),
+	QMP_PHY_INIT_CFG(QSERDES_RX_RX_OFFSET_ADAPTOR_CNTRL2, 0x80),
+	QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_CNTRL, 0x03),
+	QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_LVL, 0x18),
+	QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_DEGLITCH_CNTRL, 0x16),
+};
+
+static const struct qmp_phy_init_tbl usb3phy_pcs_tbl[] = {
+	/* FLL settings */
+	QMP_PHY_INIT_CFG_L(QPHY_FLL_CNTRL2, 0x03),
+	QMP_PHY_INIT_CFG_L(QPHY_FLL_CNTRL1, 0x02),
+	QMP_PHY_INIT_CFG_L(QPHY_FLL_CNT_VAL_L, 0x09),
+	QMP_PHY_INIT_CFG_L(QPHY_FLL_CNT_VAL_H_TOL, 0x42),
+	QMP_PHY_INIT_CFG_L(QPHY_FLL_MAN_CODE, 0x85),
+
+	/* Lock Det settings */
+	QMP_PHY_INIT_CFG(QPHY_LOCK_DETECT_CONFIG1, 0xd1),
+	QMP_PHY_INIT_CFG(QPHY_LOCK_DETECT_CONFIG2, 0x1f),
+	QMP_PHY_INIT_CFG(QPHY_LOCK_DETECT_CONFIG3, 0x47),
+	QMP_PHY_INIT_CFG(QPHY_POWER_STATE_CONFIG2, 0x08),
+};
+
+/* struct qmp_phy_cfg - per-PHY initialization config */
+struct qmp_phy_cfg {
+	/* phy-type - PCIE/UFS/USB */
+	unsigned int type;
+	/* number of lanes provided by phy */
+	int nlanes;
+
+	/* Init sequence for PHY blocks - serdes, tx, rx, pcs */
+	const struct qmp_phy_init_tbl *serdes_tbl;
+	int serdes_tbl_num;
+	const struct qmp_phy_init_tbl *tx_tbl;
+	int tx_tbl_num;
+	const struct qmp_phy_init_tbl *rx_tbl;
+	int rx_tbl_num;
+	const struct qmp_phy_init_tbl *pcs_tbl;
+	int pcs_tbl_num;
+
+	/* array of registers with different offsets */
+	const unsigned int *regs;
+
+	unsigned int start_ctrl;
+	unsigned int pwrdn_ctrl;
+	unsigned int mask_pcs_ready;
+	unsigned int mask_com_pcs_ready;
+
+	/* true, if PHY has a separate PHY_COM control block */
+	bool has_phy_com_ctrl;
+	/* true, if PHY has a reset for individual lanes */
+	bool has_lane_rst;
+	/* true, if PHY needs delay after POWER_DOWN */
+	bool has_pwrdn_delay;
+	/* power_down delay in usec */
+	int pwrdn_delay_min;
+	int pwrdn_delay_max;
+};
+
+/**
+ * struct qmp_phy - per-lane phy descriptor
+ *
+ * @phy: generic phy
+ * @tx: iomapped memory space for lane's tx
+ * @rx: iomapped memory space for lane's rx
+ * @pcs: iomapped memory space for lane's pcs
+ * @pipe_clk: pipe lock
+ * @index: lane index
+ * @qmp: QMP phy to which this lane belongs
+ * @lane_rst: lane's reset controller
+ */
+struct qmp_phy {
+	struct phy *phy;
+	void __iomem *tx;
+	void __iomem *rx;
+	void __iomem *pcs;
+	struct clk *pipe_clk;
+	unsigned int index;
+	struct qcom_qmp *qmp;
+	struct reset_control *lane_rst;
+};
+
+/**
+ * struct qcom_qmp - structure holding QMP phy block attributes
+ *
+ * @dev: device
+ * @serdes: iomapped memory space for phy's serdes
+ *
+ * @aux_clk: phy core clock
+ * @cfg_ahb_clk: AHB2PHY interface clock
+ * @ref_clk: reference clock
+ *
+ * @vdda_phy: vdd supply to the phy core block
+ * @vdda_pll: 1.8V vdd supply to ref_clk block
+ * @vddp_ref_clk: vdd supply to specific ref_clk block (Optional)
+ *
+ * @phy_rst: phy reset control
+ * @phycom_rst: phy common reset control
+ * @phycfg_rst: phy ahb cfg reset control (Optional)
+ *
+ * @cfg: phy specific configuration
+ * @phys: array of per-lane phy descriptors
+ * @phy_mutex: mutex lock for PHY common block initialization
+ * @init_count: phy common block initialization count
+ */
+struct qcom_qmp {
+	struct device *dev;
+	void __iomem *serdes;
+
+	struct clk *aux_clk;
+	struct clk *cfg_ahb_clk;
+	struct clk *ref_clk;
+
+	struct regulator *vdda_phy;
+	struct regulator *vdda_pll;
+	struct regulator *vddp_ref_clk;
+
+	struct reset_control *phy_rst;
+	struct reset_control *phycom_rst;
+	struct reset_control *phycfg_rst;
+
+	const struct qmp_phy_cfg *cfg;
+	struct qmp_phy **phys;
+
+	struct mutex phy_mutex;
+	int init_count;
+};
+
+static inline void qphy_setbits(void __iomem *base, u32 offset, u32 val)
+{
+	u32 reg;
+
+	reg = readl_relaxed(base + offset);
+	reg |= val;
+	writel_relaxed(reg, base + offset);
+
+	/* Make sure that above writes are completed */
+	mb();
+}
+
+static inline void qphy_clrbits(void __iomem *base, u32 offset, u32 val)
+{
+	u32 reg;
+
+	reg = readl_relaxed(base + offset);
+	reg &= ~val;
+	writel_relaxed(reg, base + offset);
+
+	/* Make sure that above writes are completed */
+	mb();
+}
+
+static const struct qmp_phy_cfg msm8996_pciephy_cfg = {
+	.type			= PHY_TYPE_PCIE,
+	.nlanes			= 3,
+
+	.serdes_tbl		= pciephy_serdes_tbl,
+	.serdes_tbl_num		= ARRAY_SIZE(pciephy_serdes_tbl),
+	.tx_tbl			= pciephy_tx_tbl,
+	.tx_tbl_num		= ARRAY_SIZE(pciephy_tx_tbl),
+	.rx_tbl			= pciephy_rx_tbl,
+	.rx_tbl_num		= ARRAY_SIZE(pciephy_rx_tbl),
+	.pcs_tbl		= pciephy_pcs_tbl,
+	.pcs_tbl_num		= ARRAY_SIZE(pciephy_pcs_tbl),
+	.regs			= pciephy_regs_layout,
+	.start_ctrl		= PCS_START | PLL_READY_GATE_EN,
+	.pwrdn_ctrl		= SW_PWRDN | REFCLK_DRV_DSBL,
+	.mask_com_pcs_ready	= PCS_READY,
+
+	.has_phy_com_ctrl	= true,
+	.has_lane_rst		= true,
+	.has_pwrdn_delay	= true,
+	.pwrdn_delay_min	= POWER_DOWN_DELAY_US_MIN,
+	.pwrdn_delay_max	= POWER_DOWN_DELAY_US_MAX,
+};
+
+static const struct qmp_phy_cfg msm8996_usb3phy_cfg = {
+	.type			= PHY_TYPE_USB3,
+	.nlanes			= 1,
+
+	.serdes_tbl		= usb3phy_serdes_tbl,
+	.serdes_tbl_num		= ARRAY_SIZE(usb3phy_serdes_tbl),
+	.tx_tbl			= usb3phy_tx_tbl,
+	.tx_tbl_num		= ARRAY_SIZE(usb3phy_tx_tbl),
+	.rx_tbl			= usb3phy_rx_tbl,
+	.rx_tbl_num		= ARRAY_SIZE(usb3phy_rx_tbl),
+	.pcs_tbl		= usb3phy_pcs_tbl,
+	.pcs_tbl_num		= ARRAY_SIZE(usb3phy_pcs_tbl),
+	.regs			= usb3phy_regs_layout,
+	.start_ctrl		= SERDES_START | PCS_START,
+	.pwrdn_ctrl		= SW_PWRDN,
+	.mask_pcs_ready		= PHYSTATUS,
+};
+
+static void qcom_qmp_phy_configure(void __iomem *base,
+				const unsigned int *regs,
+				const struct qmp_phy_init_tbl tbl[],
+				int num)
+{
+	int i;
+	const struct qmp_phy_init_tbl *t = tbl;
+
+	for (i = 0; i < num; i++, t++) {
+		if (t->in_layout)
+			writel_relaxed(t->val, base + regs[t->offset]);
+		else
+			writel_relaxed(t->val, base + t->offset);
+	}
+
+	/* flush buffered writes */
+	mb();
+}
+
+static int qcom_qmp_phy_poweron(struct phy *phy)
+{
+	struct qmp_phy *qphy = phy_get_drvdata(phy);
+	struct qcom_qmp *qmp = qphy->qmp;
+	int ret;
+
+	dev_vdbg(&phy->dev, "Powering on QMP phy\n");
+
+	ret = regulator_enable(qmp->vdda_phy);
+	if (ret) {
+		dev_err(qmp->dev, "%s: vdda-phy enable failed, err=%d\n",
+				__func__, ret);
+		return ret;
+	}
+
+	ret = regulator_enable(qmp->vdda_pll);
+	if (ret) {
+		dev_err(qmp->dev, "%s: vdda-pll enable failed, err=%d\n",
+				__func__, ret);
+		goto disable_vdda_phy;
+	}
+
+	ret = regulator_enable(qmp->vddp_ref_clk);
+	if (ret) {
+		dev_err(qmp->dev,
+			"%s: vdda-ref-clk enable failed, err=%d\n",
+			__func__, ret);
+		goto disable_vdda_pll;
+	}
+
+	ret = clk_prepare_enable(qmp->ref_clk);
+	if (ret) {
+		dev_err(qmp->dev, "%s: ref_clk enable failed, err=%d\n",
+				__func__, ret);
+		goto disable_vddp_ref_clk;
+	}
+
+	ret = clk_prepare_enable(qphy->pipe_clk);
+	if (ret) {
+		dev_err(qmp->dev, "%s: pipe_clk enable failed, err=%d\n",
+				__func__, ret);
+		goto disable_ref_clk;
+	}
+
+	return 0;
+
+disable_ref_clk:
+	clk_disable_unprepare(qmp->ref_clk);
+disable_vddp_ref_clk:
+	regulator_disable(qmp->vddp_ref_clk);
+disable_vdda_pll:
+	regulator_disable(qmp->vdda_pll);
+disable_vdda_phy:
+	regulator_disable(qmp->vdda_phy);
+	return ret;
+}
+
+static int qcom_qmp_phy_poweroff(struct phy *phy)
+{
+	struct qmp_phy *qphy = phy_get_drvdata(phy);
+	struct qcom_qmp *qmp = qphy->qmp;
+
+	clk_disable_unprepare(qphy->pipe_clk);
+	clk_disable_unprepare(qmp->ref_clk);
+
+	regulator_disable(qmp->vddp_ref_clk);
+	regulator_disable(qmp->vdda_pll);
+	regulator_disable(qmp->vdda_phy);
+
+	return 0;
+}
+
+static int qcom_qmp_phy_com_init(struct qcom_qmp *qmp)
+{
+	const struct qmp_phy_cfg *cfg = qmp->cfg;
+	void __iomem *serdes = qmp->serdes;
+	int ret;
+
+	mutex_lock(&qmp->phy_mutex);
+	if (qmp->init_count++) {
+		mutex_unlock(&qmp->phy_mutex);
+		return 0;
+	}
+
+	ret = reset_control_deassert(qmp->phy_rst);
+	if (ret) {
+		dev_err(qmp->dev, "phy reset deassert failed\n");
+		goto err_rst;
+	}
+
+	ret = reset_control_deassert(qmp->phycom_rst);
+	if (ret) {
+		dev_err(qmp->dev, "common reset deassert failed\n");
+		goto err_com_rst;
+	}
+
+	if (qmp->phycfg_rst) {
+		ret = reset_control_deassert(qmp->phycfg_rst);
+		if (ret) {
+			dev_err(qmp->dev, "ahb cfg reset deassert failed\n");
+			goto err_cfg_rst;
+		}
+	}
+
+	if (cfg->has_phy_com_ctrl)
+		qphy_setbits(serdes, cfg->regs[QPHY_COM_POWER_DOWN_CONTROL],
+				SW_PWRDN);
+
+	/* Serdes configuration */
+	qcom_qmp_phy_configure(serdes, cfg->regs, cfg->serdes_tbl,
+				cfg->serdes_tbl_num);
+
+	if (cfg->has_phy_com_ctrl) {
+		void __iomem *status;
+		unsigned int mask, val;
+
+		qphy_clrbits(serdes, cfg->regs[QPHY_COM_SW_RESET],
+				SW_RESET);
+		qphy_setbits(serdes, cfg->regs[QPHY_COM_START_CONTROL],
+				SERDES_START | PCS_START);
+
+		status = serdes + cfg->regs[QPHY_COM_PCS_READY_STATUS];
+		mask = cfg->mask_com_pcs_ready;
+
+		ret = readl_poll_timeout(status, val, (val & mask), 10,
+						PHY_INIT_COMPLETE_TIMEOUT);
+		if (ret) {
+			dev_err(qmp->dev,
+				"phy common block init timed-out\n");
+			goto err_com_init;
+		}
+	}
+
+	mutex_unlock(&qmp->phy_mutex);
+
+	return 0;
+
+err_com_init:
+	if (qmp->phycfg_rst)
+		reset_control_assert(qmp->phycfg_rst);
+err_cfg_rst:
+	reset_control_assert(qmp->phycom_rst);
+err_com_rst:
+	reset_control_assert(qmp->phy_rst);
+err_rst:
+	mutex_unlock(&qmp->phy_mutex);
+	return ret;
+}
+
+static int qcom_qmp_phy_com_exit(struct qcom_qmp *qmp)
+{
+	const struct qmp_phy_cfg *cfg = qmp->cfg;
+	void __iomem *serdes = qmp->serdes;
+
+	mutex_lock(&qmp->phy_mutex);
+	if (--qmp->init_count) {
+		mutex_unlock(&qmp->phy_mutex);
+		return 0;
+	}
+
+	if (cfg->has_phy_com_ctrl) {
+		qphy_setbits(serdes, cfg->regs[QPHY_COM_START_CONTROL],
+				SERDES_START | PCS_START);
+		qphy_clrbits(serdes, cfg->regs[QPHY_COM_SW_RESET],
+				SW_RESET);
+		qphy_setbits(serdes, cfg->regs[QPHY_COM_POWER_DOWN_CONTROL],
+				SW_PWRDN);
+	}
+
+	if (qmp->phycfg_rst)
+		reset_control_assert(qmp->phycfg_rst);
+
+	reset_control_assert(qmp->phycom_rst);
+	reset_control_assert(qmp->phy_rst);
+
+	mutex_unlock(&qmp->phy_mutex);
+
+	return 0;
+}
+
+/* PHY Initialization */
+static int qcom_qmp_phy_init(struct phy *phy)
+{
+	struct qmp_phy *qphy = phy_get_drvdata(phy);
+	struct qcom_qmp *qmp = qphy->qmp;
+	const struct qmp_phy_cfg *cfg = qmp->cfg;
+	void __iomem *tx = qphy->tx;
+	void __iomem *rx = qphy->rx;
+	void __iomem *pcs = qphy->pcs;
+	void __iomem *status;
+	unsigned int mask, val;
+	int ret;
+
+	dev_vdbg(qmp->dev, "Initializing QMP phy\n");
+
+	/* enable interface clocks to program phy */
+	ret = clk_prepare_enable(qmp->aux_clk);
+	if (ret) {
+		dev_err(qmp->dev, "failed to enable aux clk, err=%d\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(qmp->cfg_ahb_clk);
+	if (ret) {
+		dev_err(qmp->dev, "failed to enable cfg ahb clk, err=%d\n",
+									ret);
+		goto err;
+	}
+
+	ret = qcom_qmp_phy_com_init(qmp);
+	if (ret)
+		goto err_com_init;
+
+	if (cfg->has_lane_rst) {
+		ret = reset_control_deassert(qphy->lane_rst);
+		if (ret) {
+			dev_err(qmp->dev, "lane%d reset deassert failed\n",
+					qphy->index);
+			goto err_lane_rst;
+		}
+	}
+
+	/* Tx, Rx, and PCS configurations */
+	qcom_qmp_phy_configure(tx, cfg->regs, cfg->tx_tbl, cfg->tx_tbl_num);
+	qcom_qmp_phy_configure(rx, cfg->regs, cfg->rx_tbl, cfg->rx_tbl_num);
+	qcom_qmp_phy_configure(pcs, cfg->regs, cfg->pcs_tbl, cfg->pcs_tbl_num);
+
+	/*
+	 * Pull out PHY from POWER DOWN state.
+	 * This is active low enable signal to power-down PHY.
+	 */
+	qphy_setbits(pcs, QPHY_POWER_DOWN_CONTROL, cfg->pwrdn_ctrl);
+
+	if (cfg->has_pwrdn_delay)
+		usleep_range(cfg->pwrdn_delay_min, cfg->pwrdn_delay_max);
+
+	/* start SerDes and Phy-Coding-Sublayer */
+	qphy_setbits(pcs, QPHY_START_CTRL, cfg->start_ctrl);
+
+	/* Pull PHY out of reset state */
+	qphy_clrbits(pcs, QPHY_SW_RESET, SW_RESET);
+
+	status = pcs + cfg->regs[QPHY_PCS_READY_STATUS];
+	mask = cfg->mask_pcs_ready;
+
+	ret = readl_poll_timeout(status, val, !(val & mask), 1,
+					PHY_INIT_COMPLETE_TIMEOUT);
+	if (ret) {
+		dev_err(qmp->dev, "phy initialization timed-out\n");
+		goto err_pcs_ready;
+	}
+
+	return ret;
+
+err_pcs_ready:
+	if (cfg->has_lane_rst)
+		reset_control_assert(qphy->lane_rst);
+err_lane_rst:
+	qcom_qmp_phy_com_exit(qmp);
+err_com_init:
+	clk_disable_unprepare(qmp->cfg_ahb_clk);
+err:
+	clk_disable_unprepare(qmp->aux_clk);
+	return ret;
+}
+
+static int qcom_qmp_phy_exit(struct phy *phy)
+{
+	struct qmp_phy *qphy = phy_get_drvdata(phy);
+	struct qcom_qmp *qmp = qphy->qmp;
+	const struct qmp_phy_cfg *cfg = qmp->cfg;
+
+	/* PHY reset */
+	qphy_setbits(qphy->pcs, QPHY_SW_RESET, SW_RESET);
+
+	/* stop SerDes and Phy-Coding-Sublayer */
+	qphy_clrbits(qphy->pcs, QPHY_START_CTRL, cfg->start_ctrl);
+
+	/* Put PHY into POWER DOWN state: active low */
+	qphy_clrbits(qphy->pcs, QPHY_POWER_DOWN_CONTROL,
+			cfg->pwrdn_ctrl);
+
+	if (cfg->has_lane_rst)
+		reset_control_assert(qphy->lane_rst);
+
+	qcom_qmp_phy_com_exit(qmp);
+
+	clk_disable_unprepare(qmp->aux_clk);
+	clk_disable_unprepare(qmp->cfg_ahb_clk);
+
+	return 0;
+}
+
+static int qcom_qmp_phy_regulator_init(struct device *dev)
+{
+	struct qcom_qmp *qmp = dev_get_drvdata(dev);
+	int ret;
+
+	qmp->vdda_phy = devm_regulator_get(dev, "vdda-phy");
+	if (IS_ERR(qmp->vdda_phy)) {
+		ret = PTR_ERR(qmp->vdda_phy);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "failed to get vdda-phy, %d\n", ret);
+		return ret;
+	}
+
+	qmp->vdda_pll = devm_regulator_get(dev, "vdda-pll");
+	if (IS_ERR(qmp->vdda_pll)) {
+		ret = PTR_ERR(qmp->vdda_pll);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "failed to get vdda-pll, %d\n", ret);
+		return ret;
+	}
+
+	/* optional regulator */
+	qmp->vddp_ref_clk = devm_regulator_get(dev, "vddp-ref-clk");
+	if (IS_ERR(qmp->vddp_ref_clk)) {
+		if (PTR_ERR(qmp->vddp_ref_clk) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+		dev_dbg(dev, "failed to get vddp-ref-clk, %d\n", ret);
+	}
+
+	return 0;
+}
+
+static int qcom_qmp_phy_clk_init(struct device *dev)
+{
+	struct qcom_qmp *qmp = dev_get_drvdata(dev);
+	int ret;
+
+	qmp->aux_clk = devm_clk_get(dev, "aux");
+	if (IS_ERR(qmp->aux_clk)) {
+		ret = PTR_ERR(qmp->aux_clk);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "failed to get aux_clk, %d\n", ret);
+		return ret;
+	}
+
+	qmp->cfg_ahb_clk = devm_clk_get(dev, "cfg_ahb");
+	if (IS_ERR(qmp->cfg_ahb_clk)) {
+		ret = PTR_ERR(qmp->cfg_ahb_clk);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "failed to get cfg_ahb_clk, %d\n", ret);
+		return ret;
+	}
+
+	qmp->ref_clk = devm_clk_get(dev, "ref");
+	if (IS_ERR(qmp->ref_clk)) {
+		ret = PTR_ERR(qmp->ref_clk);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "failed to get ref_clk, %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static struct phy *qcom_qmp_phy_xlate(struct device *dev,
+					struct of_phandle_args *args)
+{
+	struct qcom_qmp *qmp = dev_get_drvdata(dev);
+	int i;
+
+	if (WARN_ON(args->args_count <= 0))
+		return ERR_PTR(-EINVAL);
+	if (WARN_ON(args->args[0] >= qmp->cfg->nlanes))
+		return ERR_PTR(-EINVAL);
+
+	i = args->args[0];
+
+	return qmp->phys[i]->phy;
+}
+
+/*
+ * Register a fixed rate pipe clock.
+ *
+ * The <s>_pipe_clksrc generated by PHY goes to the GCC that gate
+ * controls it. The <s>_pipe_clk coming out of the GCC is requested
+ * by the PHY driver for its operations.
+ * We register the <s>_pipe_clksrc here. The gcc driver takes care
+ * of assigning this <s>_pipe_clksrc as parent to <s>_pipe_clk.
+ * Below picture shows this relationship.
+ *
+ *         +---------------+
+ *         |   PHY block   |<<---------------------------------------+
+ *         |               |                                         |
+ *         |   +-------+   |                   +-----+               |
+ *   I/P---^-->|  PLL  |---^--->pipe_clksrc--->| GCC |--->pipe_clk---+
+ *    clk  |   +-------+   |                   +-----+
+ *         +---------------+
+ */
+static int phy_pipe_clk_register(struct qcom_qmp *qmp, int id)
+{
+	char name[24];
+	struct clk_fixed_rate *fixed;
+	struct clk_init_data init = { };
+	int ret;
+
+	switch (qmp->cfg->type) {
+	case PHY_TYPE_USB3:
+		snprintf(name, sizeof(name), "usb3_phy_pipe_clk_src");
+		break;
+	case PHY_TYPE_PCIE:
+		snprintf(name, sizeof(name), "pcie_%d_pipe_clk_src", id);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	fixed = devm_kzalloc(qmp->dev, sizeof(*fixed), GFP_KERNEL);
+	if (!fixed)
+		return -ENOMEM;
+
+	init.name = name;
+	init.ops = &clk_fixed_rate_ops;
+
+	/* controllers using QMP phys use 125MHz pipe clock interface */
+	fixed->fixed_rate = 125000000;
+	fixed->hw.init = &init;
+
+	ret = devm_clk_hw_register(qmp->dev, &fixed->hw);
+
+	return ret;
+}
+
+static const struct phy_ops qcom_qmp_phy_gen_ops = {
+	.init		= qcom_qmp_phy_init,
+	.exit		= qcom_qmp_phy_exit,
+	.power_on	= qcom_qmp_phy_poweron,
+	.power_off	= qcom_qmp_phy_poweroff,
+	.owner		= THIS_MODULE,
+};
+
+static
+struct qmp_phy *qcom_qmp_phy_create(struct platform_device *pdev, int id)
+{
+	struct device *dev = &pdev->dev;
+	struct qcom_qmp *qmp = dev_get_drvdata(dev);
+	struct phy *generic_phy;
+	struct qmp_phy *qphy;
+	struct resource *res;
+	void __iomem *base;
+	char prop_name[MAX_PROP_NAME];
+	unsigned int lane_offsets[3];
+	int ret;
+
+	/* mem resources from index 1 to N for N number of lanes */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, id + 1);
+	base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(base))
+		return ERR_CAST(base);
+
+	qphy = devm_kzalloc(dev, sizeof(*qphy), GFP_KERNEL);
+	if (!qphy)
+		return ERR_PTR(-ENOMEM);
+
+	/*
+	 * read offsets to Tx, Rx, and PCS blocks into a u32 array:
+	 * +------------------------------------+
+	 * | tx offset | rx offset | pcs offset |
+	 * +------------------------------------+
+	 */
+	ret = of_property_read_u32_array(dev->of_node, "lane-offsets",
+						   lane_offsets, 3);
+	if (ret) {
+		dev_err(dev, "failed to get tx/rx/pcs offsets for lane%d\n",
+			id);
+		return ERR_PTR(ret);
+	}
+
+	qphy->tx = base + lane_offsets[0];
+	qphy->rx = base + lane_offsets[1];
+	qphy->pcs = base + lane_offsets[2];
+
+	/*
+	 * Get PHY's Pipe clock, if any; USB3 and PCIe are PIPE3
+	 * based phys, so they essentially have pipe clock. So,
+	 * we return error in case phy is USB3 or PIPE type.
+	 * Otherwise, we initialize pipe clock to NULL for
+	 * all phys that don't need this.
+	 */
+	snprintf(prop_name, sizeof(prop_name), "pipe%d", id);
+	qphy->pipe_clk = devm_clk_get(dev, prop_name);
+	if (IS_ERR(qphy->pipe_clk)) {
+		if (qmp->cfg->type == PHY_TYPE_PCIE ||
+		    qmp->cfg->type == PHY_TYPE_USB3) {
+			ret = PTR_ERR(qphy->pipe_clk);
+			if (ret != -EPROBE_DEFER)
+				dev_err(dev,
+					"failed to get lane%d pipe_clk, %d\n",
+					id, ret);
+			return ERR_PTR(ret);
+		}
+		qphy->pipe_clk = NULL;
+	}
+
+	/* Get lane reset, if any */
+	if (qmp->cfg->has_lane_rst) {
+		snprintf(prop_name, sizeof(prop_name), "lane%d", id);
+		qphy->lane_rst = devm_reset_control_get(dev,
+							    prop_name);
+		if (IS_ERR(qphy->lane_rst)) {
+			dev_err(dev, "failed to get lane%d reset\n", id);
+			return ERR_CAST(qphy->lane_rst);
+		}
+	}
+
+	generic_phy = devm_phy_create(dev, NULL, &qcom_qmp_phy_gen_ops);
+	if (IS_ERR(generic_phy)) {
+		ret = PTR_ERR(generic_phy);
+		dev_err(dev, "failed to create qphy %d\n", ret);
+		return ERR_PTR(ret);
+	}
+
+	qphy->phy = generic_phy;
+	qphy->index = id;
+	qphy->qmp = qmp;
+	phy_set_drvdata(generic_phy, qphy);
+
+	return qphy;
+}
+
+static const struct of_device_id qcom_qmp_phy_of_match_table[] = {
+	{
+		.compatible = "qcom,msm8996-qmp-pcie-phy",
+		.data = &msm8996_pciephy_cfg,
+	}, {
+		.compatible = "qcom,msm8996-qmp-usb3-phy",
+		.data = &msm8996_usb3phy_cfg,
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, qcom_qmp_phy_of_match_table);
+
+static int qcom_qmp_phy_probe(struct platform_device *pdev)
+{
+	struct qcom_qmp *qmp;
+	struct device *dev = &pdev->dev;
+	struct phy_provider *phy_provider;
+	struct resource *res;
+	void __iomem *base;
+	int ret;
+	int id;
+
+	qmp = devm_kzalloc(dev, sizeof(*qmp), GFP_KERNEL);
+	if (!qmp)
+		return -ENOMEM;
+
+	qmp->dev = dev;
+	dev_set_drvdata(dev, qmp);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	/* per PHY serdes; usually located at base address */
+	qmp->serdes = base;
+
+	mutex_init(&qmp->phy_mutex);
+
+	/* Get the specific init parameters of QMP phy */
+	qmp->cfg = of_device_get_match_data(dev);
+
+	ret = qcom_qmp_phy_clk_init(dev);
+	if (ret)
+		return ret;
+
+	ret = qcom_qmp_phy_regulator_init(dev);
+	if (ret)
+		return ret;
+
+	qmp->phy_rst = devm_reset_control_get(dev, "phy");
+	if (IS_ERR(qmp->phy_rst)) {
+		dev_err(dev, "failed to get phy core reset\n");
+		return PTR_ERR(qmp->phy_rst);
+	}
+
+	qmp->phycom_rst = devm_reset_control_get(dev, "common");
+	if (IS_ERR(qmp->phycom_rst)) {
+		dev_err(dev, "failed to get phy common reset\n");
+		return PTR_ERR(qmp->phycom_rst);
+	}
+
+	qmp->phycfg_rst = devm_reset_control_get(dev, "cfg");
+	if (IS_ERR(qmp->phycfg_rst)) {
+		dev_dbg(dev, "failed to get phy ahb cfg reset\n");
+		qmp->phycfg_rst = NULL;
+	}
+
+
+	qmp->phys = devm_kcalloc(dev, qmp->cfg->nlanes,
+					sizeof(*qmp->phys), GFP_KERNEL);
+	if (!qmp->phys)
+		return -ENOMEM;
+
+	for (id = 0; id < qmp->cfg->nlanes; id++) {
+		/* Create per-lane phy */
+		qmp->phys[id] = qcom_qmp_phy_create(pdev, id);
+		if (IS_ERR(qmp->phys[id])) {
+			dev_err(dev, "failed to create lane%d phy, %d\n",
+				id, ret);
+			return PTR_ERR(qmp->phys[id]);
+		}
+
+		/*
+		 * Register the pipe clock provided by phy.
+		 * See function description to see details of this pipe clock.
+		 */
+		ret = phy_pipe_clk_register(qmp, id);
+		if (ret) {
+			dev_err(qmp->dev,
+				"failed to register pipe clock source\n");
+			return ret;
+		}
+	}
+
+	phy_provider = devm_of_phy_provider_register(dev, qcom_qmp_phy_xlate);
+	if (IS_ERR(phy_provider)) {
+		ret = PTR_ERR(phy_provider);
+		dev_err(dev, "failed to register qphy, %d\n", ret);
+	}
+
+	return ret;
+}
+
+static struct platform_driver qcom_qmp_phy_driver = {
+	.probe		= qcom_qmp_phy_probe,
+	.driver = {
+		.name	= "qcom-qmp-phy",
+		.of_match_table = of_match_ptr(qcom_qmp_phy_of_match_table),
+	},
+};
+
+module_platform_driver(qcom_qmp_phy_driver);
+
+MODULE_AUTHOR("Vivek Gautam <vivek.gautam@codeaurora.org>");
+MODULE_DESCRIPTION("Qualcomm QMP PHY driver");
+MODULE_LICENSE("GPL v2");
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v4 4/4] phy: qcom-qmp: new qmp phy driver for qcom-chipsets

[Andy Gross]

On Tue, Jan 10, 2017 at 04:21:59PM +0530, Vivek Gautam wrote:
> Qualcomm SOCs have QMP phy controller that provides support
> to a number of controller, viz. PCIe, UFS, and USB.
> Add a new driver, based on generic phy framework, for this
> phy controller.
> 
> Signed-off-by: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Tested-by: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---

< snip>

> +static const struct of_device_id qcom_qmp_phy_of_match_table[] = {
> +	{
> +		.compatible = "qcom,msm8996-qmp-pcie-phy",
> +		.data = &msm8996_pciephy_cfg,
> +	}, {
> +		.compatible = "qcom,msm8996-qmp-usb3-phy",
> +		.data = &msm8996_usb3phy_cfg,
> +	},

Which other chips would this driver support?


Regards,

Andy Gross
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 4/4] phy: qcom-qmp: new qmp phy driver for qcom-chipsets

[Andy Gross]

On Tue, Jan 10, 2017 at 04:21:59PM +0530, Vivek Gautam wrote:
> Qualcomm SOCs have QMP phy controller that provides support
> to a number of controller, viz. PCIe, UFS, and USB.
> Add a new driver, based on generic phy framework, for this
> phy controller.
> 
> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
> Tested-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---

< snip>

> +static const struct of_device_id qcom_qmp_phy_of_match_table[] = {
> +	{
> +		.compatible = "qcom,msm8996-qmp-pcie-phy",
> +		.data = &msm8996_pciephy_cfg,
> +	}, {
> +		.compatible = "qcom,msm8996-qmp-usb3-phy",
> +		.data = &msm8996_usb3phy_cfg,
> +	},

Which other chips would this driver support?


Regards,

Andy Gross

Re: [PATCH v4 4/4] phy: qcom-qmp: new qmp phy driver for qcom-chipsets

[Vivek Gautam]

Hi,


On 01/11/2017 04:50 AM, Andy Gross wrote:
> On Tue, Jan 10, 2017 at 04:21:59PM +0530, Vivek Gautam wrote:
>> Qualcomm SOCs have QMP phy controller that provides support
>> to a number of controller, viz. PCIe, UFS, and USB.
>> Add a new driver, based on generic phy framework, for this
>> phy controller.
>>
>> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
>> Tested-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> ---
> < snip>
>
>> +static const struct of_device_id qcom_qmp_phy_of_match_table[] = {
>> +	{
>> +		.compatible = "qcom,msm8996-qmp-pcie-phy",
>> +		.data = &msm8996_pciephy_cfg,
>> +	}, {
>> +		.compatible = "qcom,msm8996-qmp-usb3-phy",
>> +		.data = &msm8996_usb3phy_cfg,
>> +	},
> Which other chips would this driver support?

msm8998 has the same combo PHY IP. We may need
minor changes in the initialization tables to support that.


Regards
Vivek

>
>
> Regards,
>
> Andy Gross
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project