[PATCH v2 0/3] Add QUSB2 PHY support for SC7180

[PATCH v2 0/3] Add QUSB2 PHY support for SC7180

[Sandeep Maheswaram]

Added QUSB2 PHY support for SC7180.
Converting dt binding to yaml.
Adding compatible for SC7180 in dt bindings.

Changes in v2:
Sorted the compatible in driver.
Converted dt binding to yaml.
Added compatible in yaml.


Sandeep Maheswaram (3):
  phy: qcom-qusb2: Add QUSB2 PHY support for SC7180
  dt-bindings: phy: qcom-qusb2: Convert QUSB2 phy bindings to yaml
  dt-bindings: phy: qcom-qusb2: Add SC7180 QUSB2 phy support

 .../devicetree/bindings/phy/qcom-qusb2-phy.txt     |  68 -----------
 .../devicetree/bindings/phy/qcom-qusb2-phy.yaml    | 130 +++++++++++++++++++++
 drivers/phy/qualcomm/phy-qcom-qusb2.c              |  57 ++++++++-
 3 files changed, 186 insertions(+), 69 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
 create mode 100644 Documentation/devicetree/bindings/phy/qcom-qusb2-phy.yaml

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH v2 1/3] phy: qcom-qusb2: Add QUSB2 PHY support for SC7180

[Sandeep Maheswaram]

Add QUSB2 PHY config data and compatible for SC7180.

Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
---
 drivers/phy/qualcomm/phy-qcom-qusb2.c | 57 ++++++++++++++++++++++++++++++++++-
 1 file changed, 56 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qusb2.c b/drivers/phy/qualcomm/phy-qcom-qusb2.c
index bf94a52..32a567b 100644
--- a/drivers/phy/qualcomm/phy-qcom-qusb2.c
+++ b/drivers/phy/qualcomm/phy-qcom-qusb2.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * Copyright (c) 2017, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2017-2019, The Linux Foundation. All rights reserved.
  */
 
 #include <linux/clk.h>
@@ -177,6 +177,41 @@ static const struct qusb2_phy_init_tbl msm8998_init_tbl[] = {
 	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_DIGITAL_TIMERS_TWO, 0x19),
 };
 
+static const unsigned int sc7180_regs_layout[] = {
+	[QUSB2PHY_PLL_CORE_INPUT_OVERRIDE] = 0xa8,
+	[QUSB2PHY_PLL_STATUS]		= 0x1a0,
+	[QUSB2PHY_PORT_TUNE1]		= 0x240,
+	[QUSB2PHY_PORT_TUNE2]		= 0x244,
+	[QUSB2PHY_PORT_TUNE3]		= 0x248,
+	[QUSB2PHY_PORT_TUNE4]		= 0x24c,
+	[QUSB2PHY_PORT_TUNE5]		= 0x250,
+	[QUSB2PHY_PORT_TEST1]		= 0x254,
+	[QUSB2PHY_PORT_TEST2]		= 0x258,
+	[QUSB2PHY_PORT_POWERDOWN]	= 0x210,
+	[QUSB2PHY_INTR_CTRL]		= 0x230,
+};
+
+static const struct qusb2_phy_init_tbl sc7180_init_tbl[] = {
+	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_ANALOG_CONTROLS_TWO, 0x03),
+	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_CLOCK_INVERTERS, 0x7c),
+	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_CMODE, 0x80),
+	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_LOCK_DELAY, 0x0a),
+	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_DIGITAL_TIMERS_TWO, 0x19),
+	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_BIAS_CONTROL_1, 0x40),
+	QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_BIAS_CONTROL_2, 0x22),
+	QUSB2_PHY_INIT_CFG(QUSB2PHY_PWR_CTRL2, 0x21),
+	QUSB2_PHY_INIT_CFG(QUSB2PHY_IMP_CTRL1, 0x08),
+	QUSB2_PHY_INIT_CFG(QUSB2PHY_IMP_CTRL2, 0x58),
+
+	QUSB2_PHY_INIT_CFG_L(QUSB2PHY_PORT_TUNE1, 0xc5),
+	QUSB2_PHY_INIT_CFG_L(QUSB2PHY_PORT_TUNE2, 0x29),
+	QUSB2_PHY_INIT_CFG_L(QUSB2PHY_PORT_TUNE3, 0xca),
+	QUSB2_PHY_INIT_CFG_L(QUSB2PHY_PORT_TUNE4, 0x04),
+	QUSB2_PHY_INIT_CFG_L(QUSB2PHY_PORT_TUNE5, 0x03),
+
+	QUSB2_PHY_INIT_CFG(QUSB2PHY_CHG_CTRL2, 0x30),
+};
+
 static const unsigned int sdm845_regs_layout[] = {
 	[QUSB2PHY_PLL_CORE_INPUT_OVERRIDE] = 0xa8,
 	[QUSB2PHY_PLL_STATUS]		= 0x1a0,
@@ -212,6 +247,8 @@ static const struct qusb2_phy_init_tbl sdm845_init_tbl[] = {
 	QUSB2_PHY_INIT_CFG(QUSB2PHY_CHG_CTRL2, 0x0),
 };
 
+
+
 struct qusb2_phy_cfg {
 	const struct qusb2_phy_init_tbl *tbl;
 	/* number of entries in the table */
@@ -258,6 +295,19 @@ static const struct qusb2_phy_cfg msm8998_phy_cfg = {
 	.update_tune1_with_efuse = true,
 };
 
+static const struct qusb2_phy_cfg sc7180_phy_cfg = {
+	.tbl		= sc7180_init_tbl,
+	.tbl_num	= ARRAY_SIZE(sc7180_init_tbl),
+	.regs		= sc7180_regs_layout,
+
+	.disable_ctrl	= (PWR_CTRL1_VREF_SUPPLY_TRIM | PWR_CTRL1_CLAMP_N_EN |
+			   POWER_DOWN),
+	.mask_core_ready = CORE_READY_STATUS,
+	.has_pll_override = true,
+	.autoresume_en	  = BIT(0),
+	.update_tune1_with_efuse = true,
+};
+
 static const struct qusb2_phy_cfg sdm845_phy_cfg = {
 	.tbl		= sdm845_init_tbl,
 	.tbl_num	= ARRAY_SIZE(sdm845_init_tbl),
@@ -271,6 +321,8 @@ static const struct qusb2_phy_cfg sdm845_phy_cfg = {
 	.update_tune1_with_efuse = true,
 };
 
+
+
 static const char * const qusb2_phy_vreg_names[] = {
 	"vdda-pll", "vdda-phy-dpdm",
 };
@@ -774,6 +826,9 @@ static const struct of_device_id qusb2_phy_of_match_table[] = {
 		.compatible	= "qcom,msm8998-qusb2-phy",
 		.data		= &msm8998_phy_cfg,
 	}, {
+		.compatible	= "qcom,sc7180-qusb2-phy",
+		.data		= &sc7180_phy_cfg,
+	}, {
 		.compatible	= "qcom,sdm845-qusb2-phy",
 		.data		= &sdm845_phy_cfg,
 	},
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH v2 2/3] dt-bindings: phy: qcom-qusb2: Convert QUSB2 phy bindings to yaml

[Sandeep Maheswaram]

Convert QUSB2 phy  bindings to DT schema format using json-schema.

Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
---
 .../devicetree/bindings/phy/qcom-qusb2-phy.txt     |  68 -----------
 .../devicetree/bindings/phy/qcom-qusb2-phy.yaml    | 129 +++++++++++++++++++++
 2 files changed, 129 insertions(+), 68 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
 create mode 100644 Documentation/devicetree/bindings/phy/qcom-qusb2-phy.yaml

diff --git a/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
deleted file mode 100644
index fe29f9e..0000000
--- a/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
+++ /dev/null
@@ -1,68 +0,0 @@
-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" for 14nm PHY on msm8996,
-	       "qcom,msm8998-qusb2-phy" for 10nm PHY on msm8998,
-	       "qcom,sdm845-qusb2-phy" for 10nm PHY on sdm845.
-
- - 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).
-
- - 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.
- - qcom,imp-res-offset-value: It is a 6 bit value that specifies offset to be
-		added to PHY refgen RESCODE via IMP_CTRL1 register. It is a PHY
-		tuning parameter that may vary for different boards of same SOC.
-		This property is applicable to only QUSB2 v2 PHY (sdm845).
- - qcom,hstx-trim-value: It is a 4 bit value that specifies tuning for HSTX
-		output current.
-		Possible range is - 15mA to 24mA (stepsize of 600 uA).
-		See dt-bindings/phy/phy-qcom-qusb2.h for applicable values.
-		This property is applicable to only QUSB2 v2 PHY (sdm845).
-		Default value is 22.2mA for sdm845.
- - qcom,preemphasis-level: It is a 2 bit value that specifies pre-emphasis level.
-		Possible range is 0 to 15% (stepsize of 5%).
-		See dt-bindings/phy/phy-qcom-qusb2.h for applicable values.
-		This property is applicable to only QUSB2 v2 PHY (sdm845).
-		Default value is 10% for sdm845.
-- qcom,preemphasis-width: It is a 1 bit value that specifies how long the HSTX
-		pre-emphasis (specified using qcom,preemphasis-level) must be in
-		effect. Duration could be half-bit of full-bit.
-		See dt-bindings/phy/phy-qcom-qusb2.h for applicable values.
-		This property is applicable to only QUSB2 v2 PHY (sdm845).
-		Default value is full-bit width for sdm845.
-
-Example:
-	hsusb_phy: phy@7411000 {
-		compatible = "qcom,msm8996-qusb2-phy";
-		reg = <0x7411000 0x180>;
-		#phy-cells = <0>;
-
-		clocks = <&gcc GCC_USB_PHY_CFG_AHB2PHY_CLK>,
-			<&gcc GCC_RX1_USB2_CLKREF_CLK>,
-		clock-names = "cfg_ahb", "ref";
-
-		vdda-pll-supply = <&pm8994_l12>;
-		vdda-phy-dpdm-supply = <&pm8994_l24>;
-
-		resets = <&gcc GCC_QUSB2PHY_PRIM_BCR>;
-		nvmem-cells = <&qusb2p_hstx_trim>;
-        };
diff --git a/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.yaml
new file mode 100644
index 0000000..3ef94bc
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.yaml
@@ -0,0 +1,129 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/phy/qcom-qusb2-phy.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Qualcomm QUSB2 phy controller
+
+maintainers:
+  - Manu Gautam <mgautam@codeaurora.org>
+
+description: |
+  QUSB2 controller supports LS/FS/HS usb connectivity on Qualcomm chipsets.
+
+properties:
+  compatible:
+    enum:
+      - qcom,msm8996-qusb2-phy
+      - qcom,msm8998-qusb2-phy
+      - qcom,sdm845-qusb2-phy
+
+  reg:
+    description:
+        offset and length of the PHY register set.
+
+  "#phy-cells":
+    const: 0
+
+  clocks:
+    maxItems: 3
+    description:
+        a list of phandles and clock-specifier pairs,
+        one for each entry in clock-names.
+
+  clock-names:
+    items:
+      - const: cfg_ahb #phy config clock
+      - const: ref #19.2 MHz ref clk
+      - const: iface #phy interface clock (Optional)
+
+  vdda-pll-supply:
+     description:
+       Phandle to 1.8V regulator supply to PHY refclk pll block.
+
+  vdda-phy-dpdm-supply:
+     description:
+       Phandle to 3.1V regulator supply to Dp/Dm port signals.
+
+  resets:
+    description:
+       Phandle to reset to phy block.
+
+  nvmem-cells:
+    description:
+        Phandle to nvmem cell that contains 'HS Tx trim'
+        tuning parameter value for qusb2 phy.
+
+  qcom,tcsr-syscon:
+    description:
+        Phandle to TCSR syscon register region.
+    $ref: /schemas/types.yaml#/definitions/cell
+
+  qcom,imp-res-offset-value:
+    description:
+        It is a 6 bit value that specifies offset to be
+        added to PHY refgen RESCODE via IMP_CTRL1 register. It is a PHY
+        tuning parameter that may vary for different boards of same SOC.
+        This property is applicable to only QUSB2 v2 PHY (sdm845).
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  qcom,hstx-trim-value:
+    description:
+        It is a 4 bit value that specifies tuning for HSTX
+        output current.
+        Possible range is - 15mA to 24mA (stepsize of 600 uA).
+        See dt-bindings/phy/phy-qcom-qusb2.h for applicable values.
+        This property is applicable to only QUSB2 v2 PHY (sdm845).
+        Default value is 22.2mA for sdm845.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  qcom,preemphasis-level:
+    description:
+        It is a 2 bit value that specifies pre-emphasis level.
+        Possible range is 0 to 15% (stepsize of 5%).
+        See dt-bindings/phy/phy-qcom-qusb2.h for applicable values.
+        This property is applicable to only QUSB2 v2 PHY (sdm845).
+        Default value is 10% for sdm845.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  qcom,preemphasis-width:
+    description:
+        It is a 1 bit value that specifies how long the HSTX
+        pre-emphasis (specified using qcom,preemphasis-level) must be in
+        effect. Duration could be half-bit of full-bit.
+        See dt-bindings/phy/phy-qcom-qusb2.h for applicable values.
+        This property is applicable to only QUSB2 v2 PHY (sdm845).
+        Default value is full-bit width for sdm845.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+required:
+  - compatible
+  - reg
+  - "#phy-cells"
+  - clocks
+  - clock-names
+  - vdda-pll-supply
+  - vdda-phy-dpdm-supply
+  - resets
+
+
+examples:
+  - |
+    hsusb_phy: phy@7411000 {
+        compatible = "qcom,msm8996-qusb2-phy";
+        reg = <0x7411000 0x180>;
+        #phy-cells = <0>;
+
+        clocks = <&gcc GCC_USB_PHY_CFG_AHB2PHY_CLK>,
+                <&gcc GCC_RX1_USB2_CLKREF_CLK>,
+        clock-names = "cfg_ahb", "ref";
+
+        vdda-pll-supply = <&pm8994_l12>;
+        vdda-phy-dpdm-supply = <&pm8994_l24>;
+
+        resets = <&gcc GCC_QUSB2PHY_PRIM_BCR>;
+        nvmem-cells = <&qusb2p_hstx_trim>;
+
+        };
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH v2 3/3] dt-bindings: phy: qcom-qusb2: Add SC7180 QUSB2 phy support

[Sandeep Maheswaram]

Add QUSB2 phy entries for SC7180 in device tree bindings.

Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
---
 Documentation/devicetree/bindings/phy/qcom-qusb2-phy.yaml | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.yaml
index 3ef94bc..5eff9016 100644
--- a/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.yaml
@@ -18,6 +18,7 @@ properties:
     enum:
       - qcom,msm8996-qusb2-phy
       - qcom,msm8998-qusb2-phy
+      - qcom,sc7180-qusb2-phy
       - qcom,sdm845-qusb2-phy
 
   reg:
@@ -66,7 +67,7 @@ properties:
         It is a 6 bit value that specifies offset to be
         added to PHY refgen RESCODE via IMP_CTRL1 register. It is a PHY
         tuning parameter that may vary for different boards of same SOC.
-        This property is applicable to only QUSB2 v2 PHY (sdm845).
+        This property is applicable to only QUSB2 v2 PHY (sc7180, sdm845).
     $ref: /schemas/types.yaml#/definitions/uint32
 
   qcom,hstx-trim-value:
@@ -75,7 +76,7 @@ properties:
         output current.
         Possible range is - 15mA to 24mA (stepsize of 600 uA).
         See dt-bindings/phy/phy-qcom-qusb2.h for applicable values.
-        This property is applicable to only QUSB2 v2 PHY (sdm845).
+        This property is applicable to only QUSB2 v2 PHY (sc7180, sdm845).
         Default value is 22.2mA for sdm845.
     $ref: /schemas/types.yaml#/definitions/uint32
 
@@ -84,7 +85,7 @@ properties:
         It is a 2 bit value that specifies pre-emphasis level.
         Possible range is 0 to 15% (stepsize of 5%).
         See dt-bindings/phy/phy-qcom-qusb2.h for applicable values.
-        This property is applicable to only QUSB2 v2 PHY (sdm845).
+        This property is applicable to only QUSB2 v2 PHY (sc7180, sdm845).
         Default value is 10% for sdm845.
     $ref: /schemas/types.yaml#/definitions/uint32
 
@@ -94,7 +95,7 @@ properties:
         pre-emphasis (specified using qcom,preemphasis-level) must be in
         effect. Duration could be half-bit of full-bit.
         See dt-bindings/phy/phy-qcom-qusb2.h for applicable values.
-        This property is applicable to only QUSB2 v2 PHY (sdm845).
+        This property is applicable to only QUSB2 v2 PHY (sc7180, sdm845).
         Default value is full-bit width for sdm845.
     $ref: /schemas/types.yaml#/definitions/uint32
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH v2 1/3] phy: qcom-qusb2: Add QUSB2 PHY support for SC7180

[Doug Anderson]

Hi,

On Wed, Dec 4, 2019 at 8:43 PM Sandeep Maheswaram <sanm@codeaurora.org> wrote:
>
> Add QUSB2 PHY config data and compatible for SC7180.
>
> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
> ---
>  drivers/phy/qualcomm/phy-qcom-qusb2.c | 57 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qusb2.c b/drivers/phy/qualcomm/phy-qcom-qusb2.c
> index bf94a52..32a567b 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qusb2.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qusb2.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /*
> - * Copyright (c) 2017, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2017-2019, The Linux Foundation. All rights reserved.
>   */
>
>  #include <linux/clk.h>
> @@ -177,6 +177,41 @@ static const struct qusb2_phy_init_tbl msm8998_init_tbl[] = {
>         QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_DIGITAL_TIMERS_TWO, 0x19),
>  };
>
> +static const unsigned int sc7180_regs_layout[] = {
> +       [QUSB2PHY_PLL_CORE_INPUT_OVERRIDE] = 0xa8,
> +       [QUSB2PHY_PLL_STATUS]           = 0x1a0,
> +       [QUSB2PHY_PORT_TUNE1]           = 0x240,
> +       [QUSB2PHY_PORT_TUNE2]           = 0x244,
> +       [QUSB2PHY_PORT_TUNE3]           = 0x248,
> +       [QUSB2PHY_PORT_TUNE4]           = 0x24c,
> +       [QUSB2PHY_PORT_TUNE5]           = 0x250,
> +       [QUSB2PHY_PORT_TEST1]           = 0x254,
> +       [QUSB2PHY_PORT_TEST2]           = 0x258,
> +       [QUSB2PHY_PORT_POWERDOWN]       = 0x210,
> +       [QUSB2PHY_INTR_CTRL]            = 0x230,
> +};

This table is exactly the same as "sdm845_regs_layout".  Just refer to
the "sdm845_regs_layout" below.  This should be OK because in general
the Linux convention is to name things (filenames, drivers, etc) based
on the first SoC that used it, but if it really bothers you you can
add a comment.  ;-)


> +static const struct qusb2_phy_init_tbl sc7180_init_tbl[] = {
> +       QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_ANALOG_CONTROLS_TWO, 0x03),
> +       QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_CLOCK_INVERTERS, 0x7c),
> +       QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_CMODE, 0x80),
> +       QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_LOCK_DELAY, 0x0a),
> +       QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_DIGITAL_TIMERS_TWO, 0x19),
> +       QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_BIAS_CONTROL_1, 0x40),
> +       QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_BIAS_CONTROL_2, 0x22),

Compared to sdm845, I see PLL_BIAS_CONTROL_2, was 0x20 and now it's
0x22.  Is this really a SoC-level tuning value (should be 0x22 on 100%
of all sc7180 boards and 0x20 on 100% of all sdm845 boards), or is
this really a board-specific tuning parameter that we need a device
tree property to control?


> +       QUSB2_PHY_INIT_CFG(QUSB2PHY_PWR_CTRL2, 0x21),
> +       QUSB2_PHY_INIT_CFG(QUSB2PHY_IMP_CTRL1, 0x08),

Compared to sdm845, I see IMP_CTRL1 was 0x0 and now it's 0x8.  If I
understand correctly, though, this is a board-specific tuning
parameter.  ...and, in fact, the device tree that's been submitted for
sc7180-idp has:

qcom,imp-res-offset-value = <8>;

...so I think you should match sdm845 and leave this as 0x0.


> +       QUSB2_PHY_INIT_CFG(QUSB2PHY_IMP_CTRL2, 0x58),
> +
> +       QUSB2_PHY_INIT_CFG_L(QUSB2PHY_PORT_TUNE1, 0xc5),

Compared to sdm845, I see PORT_TUNE1 was 0x30 and now it's 0xc5.  If I
understand correctly, though, this is a board-specific tuning
parameter and should be controlled by:

override_hstx_trim
override_preemphasis
override_preemphasis_width

...so you should make sure those are set right and then leave this as
matching sdm845.  If we truly think that (for some reason) nearly all
sc7180 boards will need a value that's closer to 0xc5 then we could
possibly justify this change, but I'd need a lot of convincing.


> +       QUSB2_PHY_INIT_CFG_L(QUSB2PHY_PORT_TUNE2, 0x29),
> +       QUSB2_PHY_INIT_CFG_L(QUSB2PHY_PORT_TUNE3, 0xca),
> +       QUSB2_PHY_INIT_CFG_L(QUSB2PHY_PORT_TUNE4, 0x04),
> +       QUSB2_PHY_INIT_CFG_L(QUSB2PHY_PORT_TUNE5, 0x03),
> +
> +       QUSB2_PHY_INIT_CFG(QUSB2PHY_CHG_CTRL2, 0x30),

Compared to sdm845, I see CHG_CTRL2 was 0x0 and now it's 0x30.  Is
this really a SoC-level tuning value (should be 0x30 on 100% of all
sc7180 boards and 0x0 on 100% of all sdm845 boards), or is this really
a board-specific tuning parameter that we need a device tree property
to control?


Overall summary is that from everything i see I think:

1. Probably the BIAS_CONTROL_2 and CHG_CTRL2 changes are really board
specific and need device tree properties.

2. Once we add the device tree properties, I think we can just totally
get rid of the sc7180 tables.  Then in the sc7180 device tree we could
probably do:

"qcom,sc7180-qusb2-phy", "qcom,sdm845-qusb2-phy";

...which says that we have a sc7180 PHY but it's (as far as we know)
compatible with the sdm845 PHY driver.  Then we don't need any sc7180
changes in this file at all.


> +};
> +
>  static const unsigned int sdm845_regs_layout[] = {
>         [QUSB2PHY_PLL_CORE_INPUT_OVERRIDE] = 0xa8,
>         [QUSB2PHY_PLL_STATUS]           = 0x1a0,
> @@ -212,6 +247,8 @@ static const struct qusb2_phy_init_tbl sdm845_init_tbl[] = {
>         QUSB2_PHY_INIT_CFG(QUSB2PHY_CHG_CTRL2, 0x0),
>  };
>
> +
> +

Eliminate arbitrary spacing changes from your patch.


>  struct qusb2_phy_cfg {
>         const struct qusb2_phy_init_tbl *tbl;
>         /* number of entries in the table */
> @@ -258,6 +295,19 @@ static const struct qusb2_phy_cfg msm8998_phy_cfg = {
>         .update_tune1_with_efuse = true,
>  };
>
> +static const struct qusb2_phy_cfg sc7180_phy_cfg = {
> +       .tbl            = sc7180_init_tbl,
> +       .tbl_num        = ARRAY_SIZE(sc7180_init_tbl),
> +       .regs           = sc7180_regs_layout,
> +
> +       .disable_ctrl   = (PWR_CTRL1_VREF_SUPPLY_TRIM | PWR_CTRL1_CLAMP_N_EN |
> +                          POWER_DOWN),
> +       .mask_core_ready = CORE_READY_STATUS,
> +       .has_pll_override = true,
> +       .autoresume_en    = BIT(0),
> +       .update_tune1_with_efuse = true,
> +};
> +
>  static const struct qusb2_phy_cfg sdm845_phy_cfg = {
>         .tbl            = sdm845_init_tbl,
>         .tbl_num        = ARRAY_SIZE(sdm845_init_tbl),
> @@ -271,6 +321,8 @@ static const struct qusb2_phy_cfg sdm845_phy_cfg = {
>         .update_tune1_with_efuse = true,
>  };
>
> +
> +

Eliminate arbitrary spacing changes from your patch.


-Doug

Re: [PATCH v2 3/3] dt-bindings: phy: qcom-qusb2: Add SC7180 QUSB2 phy support

[Doug Anderson]

Hi,

On Wed, Dec 4, 2019 at 8:43 PM Sandeep Maheswaram <sanm@codeaurora.org> wrote:
>
> Add QUSB2 phy entries for SC7180 in device tree bindings.
>
> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/phy/qcom-qusb2-phy.yaml | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.yaml
> index 3ef94bc..5eff9016 100644
> --- a/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.yaml
> @@ -18,6 +18,7 @@ properties:
>      enum:
>        - qcom,msm8996-qusb2-phy
>        - qcom,msm8998-qusb2-phy
> +      - qcom,sc7180-qusb2-phy
>        - qcom,sdm845-qusb2-phy

I would propose that we add generic PHY v2 tagging here, like this:

properties:
  compatible:
    anyOf:
      - items:
        - const: qcom,msm8996-qusb2-phy
      - items:
        - const: qcom,msm8998-qusb2-phy
      - items:
        # Suggested to also add "qcom,qusb2-v2-phy" as below.
        - const: qcom,sdm845-qusb2-phy
      - items:
        - enum:
          - qcom,sc7180-qusb2-phy
          - qcom,sdm845-qusb2-phy
        - const: qcom,qusb2-v2-phy

Given that this PHY has a fairly linear versioning within Qualcomm
(right?) this should make sense and should make code / adding new
device trees easier.  This is probably better than what I suggested in
the driver review [1] where I suggested that the compatible for sc7180
should be:

  compatible: "qcom,sc7180-qusb2-phy", "qcom,sdm845-qusb2-phy"


>    reg:
> @@ -66,7 +67,7 @@ properties:
>          It is a 6 bit value that specifies offset to be
>          added to PHY refgen RESCODE via IMP_CTRL1 register. It is a PHY
>          tuning parameter that may vary for different boards of same SOC.
> -        This property is applicable to only QUSB2 v2 PHY (sdm845).
> +        This property is applicable to only QUSB2 v2 PHY (sc7180, sdm845).

If you add my tagging, change this and other similar to just remove sdm845.


[1] https://lore.kernel.org/r/CAD=FV=W_z=_j==DSFbtCmTihmSyRtH85VnKpw03E=gATcqJx2Q@mail.gmail.com

-Doug

Re: [PATCH v2 2/3] dt-bindings: phy: qcom-qusb2: Convert QUSB2 phy bindings to yaml

[Doug Anderson]

Hi,

On Wed, Dec 4, 2019 at 8:43 PM Sandeep Maheswaram <sanm@codeaurora.org> wrote:
>
> diff --git a/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.yaml

Hrm.  Probably should have given this same comment on the USB3 yaml
bindings too, but I think the file name should be:

qcom,qusb2-phy.yaml

Specifically I think that's what Rob H has been requesting elsewhere.



> new file mode 100644
> index 0000000..3ef94bc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.yaml
> @@ -0,0 +1,129 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/phy/qcom-qusb2-phy.yaml#"

If you change the name to add the comma (as per above), don't forget
to update your ID.


> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Qualcomm QUSB2 phy controller
> +
> +maintainers:
> +  - Manu Gautam <mgautam@codeaurora.org>
> +
> +description: |

nit: don't need the "|" unless carriage returns are important.


> +  QUSB2 controller supports LS/FS/HS usb connectivity on Qualcomm chipsets.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,msm8996-qusb2-phy
> +      - qcom,msm8998-qusb2-phy
> +      - qcom,sdm845-qusb2-phy
> +
> +  reg:
> +    description:
> +        offset and length of the PHY register set.

No description, just maxItems: 1 I think.  This seemed to be what Rob
H suggested in USB3 bindings with regards to resets where the
description was just generic [1].


> +  "#phy-cells":
> +    const: 0
> +
> +  clocks:
> +    maxItems: 3
> +    description:
> +        a list of phandles and clock-specifier pairs,
> +        one for each entry in clock-names.
> +
> +  clock-names:
> +    items:
> +      - const: cfg_ahb #phy config clock
> +      - const: ref #19.2 MHz ref clk
> +      - const: iface #phy interface clock (Optional)

Please take the same type of feedback you got for the USB3 bindings
with regards to clocks / clock-names.


> +  vdda-pll-supply:
> +     description:
> +       Phandle to 1.8V regulator supply to PHY refclk pll block.
> +
> +  vdda-phy-dpdm-supply:
> +     description:
> +       Phandle to 3.1V regulator supply to Dp/Dm port signals.
> +
> +  resets:
> +    description:
> +       Phandle to reset to phy block.

No description, just maxItems: 1 I think [1].


> +  nvmem-cells:
> +    description:
> +        Phandle to nvmem cell that contains 'HS Tx trim'
> +        tuning parameter value for qusb2 phy.

Add maxItems: 1 ?


> +  qcom,tcsr-syscon:
> +    description:
> +        Phandle to TCSR syscon register region.
> +    $ref: /schemas/types.yaml#/definitions/cell
> +
> +  qcom,imp-res-offset-value:
> +    description:
> +        It is a 6 bit value that specifies offset to be
> +        added to PHY refgen RESCODE via IMP_CTRL1 register. It is a PHY
> +        tuning parameter that may vary for different boards of same SOC.
> +        This property is applicable to only QUSB2 v2 PHY (sdm845).
> +    $ref: /schemas/types.yaml#/definitions/uint32

minimum: 0
maximum: 63

...and, I think:
default: 0


> +  qcom,hstx-trim-value:
> +    description:
> +        It is a 4 bit value that specifies tuning for HSTX
> +        output current.
> +        Possible range is - 15mA to 24mA (stepsize of 600 uA).
> +        See dt-bindings/phy/phy-qcom-qusb2.h for applicable values.
> +        This property is applicable to only QUSB2 v2 PHY (sdm845).
> +        Default value is 22.2mA for sdm845.
> +    $ref: /schemas/types.yaml#/definitions/uint32

minimum: 0
maximum: 15
default: 3


> +  qcom,preemphasis-level:
> +    description:
> +        It is a 2 bit value that specifies pre-emphasis level.
> +        Possible range is 0 to 15% (stepsize of 5%).
> +        See dt-bindings/phy/phy-qcom-qusb2.h for applicable values.
> +        This property is applicable to only QUSB2 v2 PHY (sdm845).
> +        Default value is 10% for sdm845.
> +    $ref: /schemas/types.yaml#/definitions/uint32

minimum: 0
maximum: 3
default: 2


> +  qcom,preemphasis-width:
> +    description:
> +        It is a 1 bit value that specifies how long the HSTX
> +        pre-emphasis (specified using qcom,preemphasis-level) must be in
> +        effect. Duration could be half-bit of full-bit.
> +        See dt-bindings/phy/phy-qcom-qusb2.h for applicable values.
> +        This property is applicable to only QUSB2 v2 PHY (sdm845).
> +        Default value is full-bit width for sdm845.
> +    $ref: /schemas/types.yaml#/definitions/uint32

minimum: 0
maximum: 1
default: 0


> +required:
> +  - compatible
> +  - reg
> +  - "#phy-cells"
> +  - clocks
> +  - clock-names
> +  - vdda-pll-supply
> +  - vdda-phy-dpdm-supply
> +  - resets
> +
> +
> +examples:
> +  - |

#include <dt-bindings/clock/qcom,gcc-msm8996.h>


> +    hsusb_phy: phy@7411000 {
> +        compatible = "qcom,msm8996-qusb2-phy";
> +        reg = <0x7411000 0x180>;
> +        #phy-cells = <0>;
> +
> +        clocks = <&gcc GCC_USB_PHY_CFG_AHB2PHY_CLK>,
> +                <&gcc GCC_RX1_USB2_CLKREF_CLK>,

The comma at the end is a syntax error.

make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/phy/qcom-qusb2-phy.yaml

[1] https://lore.kernel.org/r/20191213212313.GA21092@bogus


-Doug