2020-01-29 13:53:39

by Sandeep Maheswaram

[permalink] [raw]
Subject: [PATCH v4 0/8] Add QUSB2 PHY support for SC7180

Converting dt binding to yaml.
Adding compatible for SC7180 in dt bindings.
Added generic QUSB2 V2 PHY support and using the same SC7180 and SDM845.

Changes in v4:
*Addressed Rob Herrings comments in dt bindings.
*Added new structure for all the overriding tuning params.
*Removed the sc7180 and sdm845 compatible from driver and added qusb2 v2 phy.
*Added the qusb2 v2 phy compatible in device tree for sc7180 and sdm845.

Changes in v3:
*Using the generic phy cfg table for QUSB2 V2 phy.
*Added support for overriding tuning parameters in QUSB2 V2 PHY
from device tree.

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

Sandeep Maheswaram (8):
dt-bindings: phy: qcom,qusb2: Convert QUSB2 phy bindings to yaml
dt-bindings: phy: qcom,qusb2: Add compatibles for QUSB2 V2 phy and
SC7180
phy: qcom-qusb2: Add generic QUSB2 V2 PHY support
dt-bindings: phy: qcom-qusb2: Add support for overriding Phy tuning
parameters
phy: qcom-qusb2: Add support for overriding tuning parameters in QUSB2
V2 PHY
arm64: dts: qcom: sc7180: Add generic QUSB2 V2 Phy compatible
arm64: dts: qcom: sdm845: Add generic QUSB2 V2 Phy compatible
arm64: dts: qcom: sc7180: Update QUSB2 V2 Phy params for SC7180 IDP
device

.../devicetree/bindings/phy/qcom,qusb2-phy.yaml | 182 +++++++++++++++++++++
.../devicetree/bindings/phy/qcom-qusb2-phy.txt | 68 --------
arch/arm64/boot/dts/qcom/sc7180-idp.dts | 6 +-
arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +-
arch/arm64/boot/dts/qcom/sdm845.dtsi | 4 +-
drivers/phy/qualcomm/phy-qcom-qusb2.c | 143 +++++++++++-----
6 files changed, 291 insertions(+), 114 deletions(-)
create mode 100644 Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
delete mode 100644 Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt

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


2020-01-29 13:53:43

by Sandeep Maheswaram

[permalink] [raw]
Subject: [PATCH v4 8/8] arm64: dts: qcom: sc7180: Update QUSB2 V2 Phy params for SC7180 IDP device

Overriding the QUSB2 V2 Phy tuning parameters for SC7180 IDP device.

Signed-off-by: Sandeep Maheswaram <[email protected]>
---
arch/arm64/boot/dts/qcom/sc7180-idp.dts | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
index 388f50a..826cf02 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
@@ -276,9 +276,11 @@
vdda-pll-supply = <&vreg_l11a_1p8>;
vdda-phy-dpdm-supply = <&vreg_l17a_3p0>;
qcom,imp-res-offset-value = <8>;
- qcom,hstx-trim-value = <QUSB2_V2_HSTX_TRIM_21_6_MA>;
- qcom,preemphasis-level = <QUSB2_V2_PREEMPHASIS_5_PERCENT>;
+ qcom,preemphasis-level = <QUSB2_V2_PREEMPHASIS_15_PERCENT>;
qcom,preemphasis-width = <QUSB2_V2_PREEMPHASIS_WIDTH_HALF_BIT>;
+ qcom,bias-ctrl-value = <0x22>;
+ qcom,charge-ctrl-value = <3>;
+ qcom,hsdisc-trim-value = <0>;
};

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

2020-01-29 13:53:48

by Sandeep Maheswaram

[permalink] [raw]
Subject: [PATCH v4 2/8] dt-bindings: phy: qcom,qusb2: Add compatibles for QUSB2 V2 phy and SC7180

Add compatibles for generic QUSB2 V2 phy which can be used for
sdm845 and sc7180.

Signed-off-by: Sandeep Maheswaram <[email protected]>
---
Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
index 90b3cc6..43082c8 100644
--- a/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
@@ -15,10 +15,17 @@ description:

properties:
compatible:
- enum:
- - qcom,msm8996-qusb2-phy
- - qcom,msm8998-qusb2-phy
- - qcom,sdm845-qusb2-phy
+ oneOf:
+ - items:
+ - enum:
+ - qcom,msm8996-qusb2-phy
+ - qcom,msm8998-qusb2-phy
+ - qcom,qusb2-v2-phy
+ - items:
+ - enum:
+ - qcom,sc7180-qusb2-phy
+ - qcom,sdm845-qusb2-phy
+ - const: qcom,qusb2-v2-phy
reg:
maxItems: 1

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

2020-01-29 13:53:55

by Sandeep Maheswaram

[permalink] [raw]
Subject: [PATCH v4 1/8] dt-bindings: phy: qcom,qusb2: Convert QUSB2 phy bindings to yaml

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

Signed-off-by: Sandeep Maheswaram <[email protected]>
---
.../devicetree/bindings/phy/qcom,qusb2-phy.yaml | 142 +++++++++++++++++++++
.../devicetree/bindings/phy/qcom-qusb2-phy.txt | 68 ----------
2 files changed, 142 insertions(+), 68 deletions(-)
create mode 100644 Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
delete mode 100644 Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt

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..90b3cc6
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
@@ -0,0 +1,142 @@
+# 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 <[email protected]>
+
+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:
+ maxItems: 1
+
+ "#phy-cells":
+ const: 0
+
+ clocks:
+ minItems: 2
+ items:
+ - description: phy config clock
+ - description: 19.2 MHz ref clk
+ - description: phy interface clock (Optional)
+
+ clock-names:
+ minItems: 2
+ items:
+ - const: cfg_ahb
+ - const: ref
+ - const: iface
+
+ 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:
+ maxItems: 1
+
+ nvmem-cells:
+ maxItems: 1
+ 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.
+ allOf:
+ - $ref: /schemas/types.yaml#/definitions/uint32
+ - minimum: 0
+ maximum: 63
+ 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.
+ allOf:
+ - $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.
+ allOf:
+ - $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.
+ allOf:
+ - $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>;
+ 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.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>;
- };
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

2020-01-29 13:54:18

by Sandeep Maheswaram

[permalink] [raw]
Subject: [PATCH v4 3/8] phy: qcom-qusb2: Add generic QUSB2 V2 PHY support

Add generic QUSB2 V2 PHY table so the respective phys
can use the same table.

Signed-off-by: Sandeep Maheswaram <[email protected]>
---
drivers/phy/qualcomm/phy-qcom-qusb2.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qusb2.c b/drivers/phy/qualcomm/phy-qcom-qusb2.c
index bf94a52..70c9da6 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,7 +177,7 @@ static const struct qusb2_phy_init_tbl msm8998_init_tbl[] = {
QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_DIGITAL_TIMERS_TWO, 0x19),
};

-static const unsigned int sdm845_regs_layout[] = {
+static const unsigned int qusb2_v2_regs_layout[] = {
[QUSB2PHY_PLL_CORE_INPUT_OVERRIDE] = 0xa8,
[QUSB2PHY_PLL_STATUS] = 0x1a0,
[QUSB2PHY_PORT_TUNE1] = 0x240,
@@ -191,7 +191,7 @@ static const unsigned int sdm845_regs_layout[] = {
[QUSB2PHY_INTR_CTRL] = 0x230,
};

-static const struct qusb2_phy_init_tbl sdm845_init_tbl[] = {
+static const struct qusb2_phy_init_tbl qusb2_v2_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),
@@ -258,10 +258,10 @@ static const struct qusb2_phy_cfg msm8998_phy_cfg = {
.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),
- .regs = sdm845_regs_layout,
+static const struct qusb2_phy_cfg qusb2_v2_phy_cfg = {
+ .tbl = qusb2_v2_init_tbl,
+ .tbl_num = ARRAY_SIZE(qusb2_v2_init_tbl),
+ .regs = qusb2_v2_regs_layout,

.disable_ctrl = (PWR_CTRL1_VREF_SUPPLY_TRIM | PWR_CTRL1_CLAMP_N_EN |
POWER_DOWN),
@@ -774,8 +774,8 @@ static const struct of_device_id qusb2_phy_of_match_table[] = {
.compatible = "qcom,msm8998-qusb2-phy",
.data = &msm8998_phy_cfg,
}, {
- .compatible = "qcom,sdm845-qusb2-phy",
- .data = &sdm845_phy_cfg,
+ .compatible = "qcom,qusb2-v2-phy",
+ .data = &qusb2_v2_phy_cfg,
},
{ },
};
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

2020-01-29 13:54:25

by Sandeep Maheswaram

[permalink] [raw]
Subject: [PATCH v4 6/8] arm64: dts: qcom: sc7180: Add generic QUSB2 V2 Phy compatible

Use generic QUSB2 V2 Phy configuration for SC7180.

Signed-off-by: Sandeep Maheswaram <[email protected]>
---
arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index 8011c5f..0d6761b 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -1052,7 +1052,7 @@
};

usb_1_hsphy: phy@88e3000 {
- compatible = "qcom,sc7180-qusb2-phy";
+ compatible = "qcom,sc7180-qusb2-phy", "qcom,qusb2-v2-phy";
reg = <0 0x088e3000 0 0x400>;
status = "disabled";
#phy-cells = <0>;
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

2020-01-29 13:54:26

by Sandeep Maheswaram

[permalink] [raw]
Subject: [PATCH v4 4/8] dt-bindings: phy: qcom-qusb2: Add support for overriding Phy tuning parameters

Add support for overriding QUSB2 V2 phy tuning parameters
in device tree bindings.

Signed-off-by: Sandeep Maheswaram <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
.../devicetree/bindings/phy/qcom,qusb2-phy.yaml | 33 ++++++++++++++++++++++
1 file changed, 33 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
index 43082c8..dfef356 100644
--- a/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
@@ -80,6 +80,28 @@ properties:
maximum: 63
default: 0

+ qcom,bias-ctrl-value:
+ description:
+ It is a 6 bit value that specifies bias-ctrl-value. It is a PHY
+ tuning parameter that may vary for different boards of same SOC.
+ This property is applicable to only QUSB2 v2 PHY.
+ allOf:
+ - $ref: /schemas/types.yaml#/definitions/uint32
+ - minimum: 0
+ maximum: 63
+ default: 0
+
+ qcom,charge-ctrl-value:
+ description:
+ It is a 2 bit value that specifies charge-ctrl-value. It is a PHY
+ tuning parameter that may vary for different boards of same SOC.
+ This property is applicable to only QUSB2 v2 PHY.
+ allOf:
+ - $ref: /schemas/types.yaml#/definitions/uint32
+ - minimum: 0
+ maximum: 3
+ default: 0
+
qcom,hstx-trim-value:
description:
It is a 4 bit value that specifies tuning for HSTX
@@ -118,6 +140,17 @@ properties:
maximum: 1
default: 0

+ qcom,hsdisc-trim-value:
+ description:
+ It is a 2 bit value tuning parameter that control disconnect
+ threshold and may vary for different boards of same SOC.
+ This property is applicable to only QUSB2 v2 PHY.
+ allOf:
+ - $ref: /schemas/types.yaml#/definitions/uint32
+ - minimum: 0
+ maximum: 3
+ default: 0
+
required:
- compatible
- reg
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

2020-01-29 13:55:04

by Sandeep Maheswaram

[permalink] [raw]
Subject: [PATCH v4 5/8] phy: qcom-qusb2: Add support for overriding tuning parameters in QUSB2 V2 PHY

Added new structure for overriding tuning parameters in QUSB2 V2 PHY as the
override params are increased due to usage of generic QUSB2 V2 phy table.
Also added bias-ctrl-value,charge-ctrl-value and hsdisc-trim-value params.

Signed-off-by: Sandeep Maheswaram <[email protected]>
---
drivers/phy/qualcomm/phy-qcom-qusb2.c | 125 +++++++++++++++++++++++++---------
1 file changed, 93 insertions(+), 32 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qusb2.c b/drivers/phy/qualcomm/phy-qcom-qusb2.c
index 70c9da6..f45fda3 100644
--- a/drivers/phy/qualcomm/phy-qcom-qusb2.c
+++ b/drivers/phy/qualcomm/phy-qcom-qusb2.c
@@ -66,6 +66,14 @@
#define IMP_RES_OFFSET_MASK GENMASK(5, 0)
#define IMP_RES_OFFSET_SHIFT 0x0

+/* QUSB2PHY_PLL_BIAS_CONTROL_2 register bits */
+#define BIAS_CTRL2_RES_OFFSET_MASK GENMASK(5, 0)
+#define BIAS_CTRL2_RES_OFFSET_SHIFT 0x0
+
+/* QUSB2PHY_CHG_CONTROL_2 register bits */
+#define CHG_CTRL2_OFFSET_MASK GENMASK(5, 4)
+#define CHG_CTRL2_OFFSET_SHIFT 0x4
+
/* QUSB2PHY_PORT_TUNE1 register bits */
#define HSTX_TRIM_MASK GENMASK(7, 4)
#define HSTX_TRIM_SHIFT 0x4
@@ -73,6 +81,10 @@
#define PREEMPHASIS_EN_MASK GENMASK(1, 0)
#define PREEMPHASIS_EN_SHIFT 0x0

+/* QUSB2PHY_PORT_TUNE2 register bits */
+#define HSDISC_TRIM_MASK GENMASK(1, 0)
+#define HSDISC_TRIM_SHIFT 0x0
+
#define QUSB2PHY_PLL_ANALOG_CONTROLS_TWO 0x04
#define QUSB2PHY_PLL_CLOCK_INVERTERS 0x18c
#define QUSB2PHY_PLL_CMODE 0x2c
@@ -277,6 +289,34 @@ static const char * const qusb2_phy_vreg_names[] = {

#define QUSB2_NUM_VREGS ARRAY_SIZE(qusb2_phy_vreg_names)

+/* struct override_param - structure holding qusb2 v2 phy overriding param
+ * set override true if the device tree property exists and read and assign
+ * to value
+ */
+struct override_param {
+ bool override;
+ u8 value;
+};
+
+/*struct override_params - structure holding qusb2 v2 phy overriding params
+ * @imp_res_offset: rescode offset to be updated in IMP_CTRL1 register
+ * @hstx_trim: HSTX_TRIM to be updated in TUNE1 register
+ * @preemphasis: Amplitude Pre-Emphasis to be updated in TUNE1 register
+ * @preemphasis_width: half/full-width Pre-Emphasis updated via TUNE1
+ * @bias_ctrl: bias ctrl to be updated in BIAS_CONTROL_2 register
+ * @charge_ctrl: charge ctrl to be updated in CHG_CTRL2 register
+ * @hsdisc_trim: disconnect threshold to be updated in TUNE2 register
+ */
+struct override_params {
+ struct override_param imp_res_offset;
+ struct override_param hstx_trim;
+ struct override_param preemphasis;
+ struct override_param preemphasis_width;
+ struct override_param bias_ctrl;
+ struct override_param charge_ctrl;
+ struct override_param hsdisc_trim;
+};
+
/**
* struct qusb2_phy - structure holding qusb2 phy attributes
*
@@ -292,20 +332,15 @@ static const char * const qusb2_phy_vreg_names[] = {
* @tcsr: TCSR syscon register map
* @cell: nvmem cell containing phy tuning value
*
- * @override_imp_res_offset: PHY should use different rescode offset
- * @imp_res_offset_value: rescode offset to be updated in IMP_CTRL1 register
- * @override_hstx_trim: PHY should use different HSTX o/p current value
- * @hstx_trim_value: HSTX_TRIM value to be updated in TUNE1 register
- * @override_preemphasis: PHY should use different pre-amphasis amplitude
- * @preemphasis_level: Amplitude Pre-Emphasis to be updated in TUNE1 register
- * @override_preemphasis_width: PHY should use different pre-emphasis duration
- * @preemphasis_width: half/full-width Pre-Emphasis updated via TUNE1
+ * @overrides: pointer to structure for all overriding tuning params
*
* @cfg: phy config data
* @has_se_clk_scheme: indicate if PHY has single-ended ref clock scheme
* @phy_initialized: indicate if PHY has been initialized
* @mode: current PHY mode
*/
+
+
struct qusb2_phy {
struct phy *phy;
void __iomem *base;
@@ -319,14 +354,7 @@ struct qusb2_phy {
struct regmap *tcsr;
struct nvmem_cell *cell;

- bool override_imp_res_offset;
- u8 imp_res_offset_value;
- bool override_hstx_trim;
- u8 hstx_trim_value;
- bool override_preemphasis;
- u8 preemphasis_level;
- bool override_preemphasis_width;
- u8 preemphasis_width;
+ struct override_params overrides;

const struct qusb2_phy_cfg *cfg;
bool has_se_clk_scheme;
@@ -395,23 +423,33 @@ static void qusb2_phy_override_phy_params(struct qusb2_phy *qphy)
{
const struct qusb2_phy_cfg *cfg = qphy->cfg;

- if (qphy->override_imp_res_offset)
+ if (qphy->overrides.imp_res_offset.override)
qusb2_write_mask(qphy->base, QUSB2PHY_IMP_CTRL1,
- qphy->imp_res_offset_value << IMP_RES_OFFSET_SHIFT,
+ qphy->overrides.imp_res_offset.value << IMP_RES_OFFSET_SHIFT,
IMP_RES_OFFSET_MASK);

- if (qphy->override_hstx_trim)
+ if (qphy->overrides.bias_ctrl.override)
+ qusb2_write_mask(qphy->base, QUSB2PHY_PLL_BIAS_CONTROL_2,
+ qphy->overrides.bias_ctrl.value << BIAS_CTRL2_RES_OFFSET_SHIFT,
+ BIAS_CTRL2_RES_OFFSET_MASK);
+
+ if (qphy->overrides.charge_ctrl.override)
+ qusb2_write_mask(qphy->base, QUSB2PHY_CHG_CTRL2,
+ qphy->overrides.charge_ctrl.value << CHG_CTRL2_OFFSET_SHIFT,
+ CHG_CTRL2_OFFSET_MASK);
+
+ if (qphy->overrides.hstx_trim.override)
qusb2_write_mask(qphy->base, cfg->regs[QUSB2PHY_PORT_TUNE1],
- qphy->hstx_trim_value << HSTX_TRIM_SHIFT,
+ qphy->overrides.hstx_trim.value << HSTX_TRIM_SHIFT,
HSTX_TRIM_MASK);

- if (qphy->override_preemphasis)
+ if (qphy->overrides.preemphasis.override)
qusb2_write_mask(qphy->base, cfg->regs[QUSB2PHY_PORT_TUNE1],
- qphy->preemphasis_level << PREEMPHASIS_EN_SHIFT,
+ qphy->overrides.preemphasis.value << PREEMPHASIS_EN_SHIFT,
PREEMPHASIS_EN_MASK);

- if (qphy->override_preemphasis_width) {
- if (qphy->preemphasis_width ==
+ if (qphy->overrides.preemphasis_width.override) {
+ if (qphy->overrides.preemphasis_width.value ==
QUSB2_V2_PREEMPHASIS_WIDTH_HALF_BIT)
qusb2_setbits(qphy->base,
cfg->regs[QUSB2PHY_PORT_TUNE1],
@@ -421,6 +459,11 @@ static void qusb2_phy_override_phy_params(struct qusb2_phy *qphy)
cfg->regs[QUSB2PHY_PORT_TUNE1],
PREEMPH_WIDTH_HALF_BIT);
}
+
+ if (qphy->overrides.hsdisc_trim.override)
+ qusb2_write_mask(qphy->base, cfg->regs[QUSB2PHY_PORT_TUNE2],
+ qphy->overrides.hsdisc_trim.value << HSDISC_TRIM_SHIFT,
+ HSDISC_TRIM_MASK);
}

/*
@@ -864,26 +907,44 @@ static int qusb2_phy_probe(struct platform_device *pdev)

if (!of_property_read_u32(dev->of_node, "qcom,imp-res-offset-value",
&value)) {
- qphy->imp_res_offset_value = (u8)value;
- qphy->override_imp_res_offset = true;
+ qphy->overrides.imp_res_offset.value = (u8)value;
+ qphy->overrides.imp_res_offset.override = true;
+ }
+
+ if (!of_property_read_u32(dev->of_node, "qcom,bias-ctrl-value",
+ &value)) {
+ qphy->overrides.bias_ctrl.value = (u8)value;
+ qphy->overrides.bias_ctrl.override = true;
+ }
+
+ if (!of_property_read_u32(dev->of_node, "qcom,charge-ctrl-value",
+ &value)) {
+ qphy->overrides.charge_ctrl.value = (u8)value;
+ qphy->overrides.charge_ctrl.override = true;
}

if (!of_property_read_u32(dev->of_node, "qcom,hstx-trim-value",
&value)) {
- qphy->hstx_trim_value = (u8)value;
- qphy->override_hstx_trim = true;
+ qphy->overrides.hstx_trim.value = (u8)value;
+ qphy->overrides.hstx_trim.override = true;
}

if (!of_property_read_u32(dev->of_node, "qcom,preemphasis-level",
&value)) {
- qphy->preemphasis_level = (u8)value;
- qphy->override_preemphasis = true;
+ qphy->overrides.preemphasis.value = (u8)value;
+ qphy->overrides.preemphasis.override = true;
}

if (!of_property_read_u32(dev->of_node, "qcom,preemphasis-width",
&value)) {
- qphy->preemphasis_width = (u8)value;
- qphy->override_preemphasis_width = true;
+ qphy->overrides.preemphasis_width.value = (u8)value;
+ qphy->overrides.preemphasis_width.override = true;
+ }
+
+ if (!of_property_read_u32(dev->of_node, "qcom,hsdisc-trim-value",
+ &value)) {
+ qphy->overrides.hsdisc_trim.value = (u8)value;
+ qphy->overrides.hsdisc_trim.override = true;
}

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

2020-01-29 13:55:46

by Sandeep Maheswaram

[permalink] [raw]
Subject: [PATCH v4 7/8] arm64: dts: qcom: sdm845: Add generic QUSB2 V2 Phy compatible

Use generic QUSB2 V2 Phy configuration for sdm845.

Signed-off-by: Sandeep Maheswaram <[email protected]>
---
arch/arm64/boot/dts/qcom/sdm845.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index d42302b..317347a 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -2387,7 +2387,7 @@
};

usb_1_hsphy: phy@88e2000 {
- compatible = "qcom,sdm845-qusb2-phy";
+ compatible = "qcom,sdm845-qusb2-phy", "qcom,qusb2-v2-phy";
reg = <0 0x088e2000 0 0x400>;
status = "disabled";
#phy-cells = <0>;
@@ -2402,7 +2402,7 @@
};

usb_2_hsphy: phy@88e3000 {
- compatible = "qcom,sdm845-qusb2-phy";
+ compatible = "qcom,sdm845-qusb2-phy", "qcom,qusb2-v2-phy";
reg = <0 0x088e3000 0 0x400>;
status = "disabled";
#phy-cells = <0>;
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

2020-01-29 20:10:52

by Matthias Kaehlcke

[permalink] [raw]
Subject: Re: [PATCH v4 1/8] dt-bindings: phy: qcom,qusb2: Convert QUSB2 phy bindings to yaml

Hi Sandeep,

On Wed, Jan 29, 2020 at 07:21:52PM +0530, Sandeep Maheswaram wrote:
> Convert QUSB2 phy bindings to DT schema format using json-schema.
>
> Signed-off-by: Sandeep Maheswaram <[email protected]>
> ---
> .../devicetree/bindings/phy/qcom,qusb2-phy.yaml | 142 +++++++++++++++++++++
> .../devicetree/bindings/phy/qcom-qusb2-phy.txt | 68 ----------
> 2 files changed, 142 insertions(+), 68 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
> delete mode 100644 Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
>
> 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..90b3cc6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
> @@ -0,0 +1,142 @@
> +# 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 <[email protected]>
> +
> +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

If you wanted to maintain the comments from the .txt file you could add
them after the compatible string (e.g.
Documentation/devicetree/bindings/pwm/pwm-samsung.yaml)

> + reg:
> + maxItems: 1
> +
> + "#phy-cells":
> + const: 0
> +
> + clocks:
> + minItems: 2

maxItems: 3 ?

> + items:
> + - description: phy config clock
> + - description: 19.2 MHz ref clk
> + - description: phy interface clock (Optional)
> +
> + clock-names:
> + minItems: 2

maxItems: 3 ?

> + items:
> + - const: cfg_ahb
> + - const: ref
> + - const: iface
> +
> + 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:
> + maxItems: 1

description:
Phandle to reset to phy block.

> +
> + nvmem-cells:
> + maxItems: 1
> + 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.
> + allOf:
> + - $ref: /schemas/types.yaml#/definitions/uint32
> + - minimum: 0
> + maximum: 63
> + 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.
> + allOf:
> + - $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.
> + allOf:
> + - $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.
> + allOf:
> + - $ref: /schemas/types.yaml#/definitions/uint32
> + - minimum: 0
> + maximum: 1
> + default: 0

I think you could put the properties that are only applicable for QUSB2
v2 PHY into a block like this and remove the 'This property is
applicable to only QUSB2 v2 PHY.' comment from the property description:

if:
properties:
compatible:
contains:
const: qcom,sdm845-qusb2-phy
then:
qcom,imp-res-offset-value:
...


Reviewed-by: Matthias Kaehlcke <[email protected]>

2020-01-29 20:32:56

by Matthias Kaehlcke

[permalink] [raw]
Subject: Re: [PATCH v4 1/8] dt-bindings: phy: qcom,qusb2: Convert QUSB2 phy bindings to yaml

On Wed, Jan 29, 2020 at 07:21:52PM +0530, Sandeep Maheswaram wrote:
> Convert QUSB2 phy bindings to DT schema format using json-schema.
>
> Signed-off-by: Sandeep Maheswaram <[email protected]>
> ---
> .../devicetree/bindings/phy/qcom,qusb2-phy.yaml | 142 +++++++++++++++++++++
> .../devicetree/bindings/phy/qcom-qusb2-phy.txt | 68 ----------
> 2 files changed, 142 insertions(+), 68 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
> delete mode 100644 Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
>
> 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..90b3cc6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
> @@ -0,0 +1,142 @@
> +# 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 <[email protected]>
> +
> +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:
> + maxItems: 1
> +
> + "#phy-cells":
> + const: 0
> +
> + clocks:
> + minItems: 2
> + items:
> + - description: phy config clock
> + - description: 19.2 MHz ref clk
> + - description: phy interface clock (Optional)
> +
> + clock-names:
> + minItems: 2
> + items:
> + - const: cfg_ahb
> + - const: ref
> + - const: iface
> +
> + 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:
> + maxItems: 1
> +
> + nvmem-cells:
> + maxItems: 1
> + 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.
> + allOf:
> + - $ref: /schemas/types.yaml#/definitions/uint32
> + - minimum: 0
> + maximum: 63
> + 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.
> + allOf:
> + - $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.
> + allOf:
> + - $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.
> + allOf:
> + - $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>;
> + 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.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>;
> - };

Reviewed-by: Matthias Kaehlcke <[email protected]>

2020-01-29 20:33:16

by Matthias Kaehlcke

[permalink] [raw]
Subject: Re: [PATCH v4 3/8] phy: qcom-qusb2: Add generic QUSB2 V2 PHY support

On Wed, Jan 29, 2020 at 07:21:54PM +0530, Sandeep Maheswaram wrote:
> Add generic QUSB2 V2 PHY table so the respective phys
> can use the same table.
>
> Signed-off-by: Sandeep Maheswaram <[email protected]>
> ---
> drivers/phy/qualcomm/phy-qcom-qusb2.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qusb2.c b/drivers/phy/qualcomm/phy-qcom-qusb2.c
> index bf94a52..70c9da6 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,7 +177,7 @@ static const struct qusb2_phy_init_tbl msm8998_init_tbl[] = {
> QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_DIGITAL_TIMERS_TWO, 0x19),
> };
>
> -static const unsigned int sdm845_regs_layout[] = {
> +static const unsigned int qusb2_v2_regs_layout[] = {
> [QUSB2PHY_PLL_CORE_INPUT_OVERRIDE] = 0xa8,
> [QUSB2PHY_PLL_STATUS] = 0x1a0,
> [QUSB2PHY_PORT_TUNE1] = 0x240,
> @@ -191,7 +191,7 @@ static const unsigned int sdm845_regs_layout[] = {
> [QUSB2PHY_INTR_CTRL] = 0x230,
> };
>
> -static const struct qusb2_phy_init_tbl sdm845_init_tbl[] = {
> +static const struct qusb2_phy_init_tbl qusb2_v2_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),
> @@ -258,10 +258,10 @@ static const struct qusb2_phy_cfg msm8998_phy_cfg = {
> .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),
> - .regs = sdm845_regs_layout,
> +static const struct qusb2_phy_cfg qusb2_v2_phy_cfg = {
> + .tbl = qusb2_v2_init_tbl,
> + .tbl_num = ARRAY_SIZE(qusb2_v2_init_tbl),
> + .regs = qusb2_v2_regs_layout,
>
> .disable_ctrl = (PWR_CTRL1_VREF_SUPPLY_TRIM | PWR_CTRL1_CLAMP_N_EN |
> POWER_DOWN),
> @@ -774,8 +774,8 @@ static const struct of_device_id qusb2_phy_of_match_table[] = {
> .compatible = "qcom,msm8998-qusb2-phy",
> .data = &msm8998_phy_cfg,
> }, {
> - .compatible = "qcom,sdm845-qusb2-phy",
> - .data = &sdm845_phy_cfg,
> + .compatible = "qcom,qusb2-v2-phy",
> + .data = &qusb2_v2_phy_cfg,
> },
> { },
> };

Reviewed-by: Matthias Kaehlcke <[email protected]>

2020-01-29 20:39:35

by Matthias Kaehlcke

[permalink] [raw]
Subject: Re: [PATCH v4 4/8] dt-bindings: phy: qcom-qusb2: Add support for overriding Phy tuning parameters

On Wed, Jan 29, 2020 at 07:21:55PM +0530, Sandeep Maheswaram wrote:
> Add support for overriding QUSB2 V2 phy tuning parameters
> in device tree bindings.
>
> Signed-off-by: Sandeep Maheswaram <[email protected]>
> Reviewed-by: Rob Herring <[email protected]>
> ---
> .../devicetree/bindings/phy/qcom,qusb2-phy.yaml | 33 ++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
> index 43082c8..dfef356 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
> @@ -80,6 +80,28 @@ properties:
> maximum: 63
> default: 0
>
> + qcom,bias-ctrl-value:
> + description:
> + It is a 6 bit value that specifies bias-ctrl-value. It is a PHY
> + tuning parameter that may vary for different boards of same SOC.
> + This property is applicable to only QUSB2 v2 PHY.

As commented on 'dt-bindings: phy: qcom,qusb2: Convert QUSB2 phy bindings
to yaml' a possible improvement could be to restrict these properties to
the QUSB2 v2 PHY through the schema.

> + allOf:
> + - $ref: /schemas/types.yaml#/definitions/uint32
> + - minimum: 0
> + maximum: 63
> + default: 0
> +
> + qcom,charge-ctrl-value:
> + description:
> + It is a 2 bit value that specifies charge-ctrl-value. It is a PHY
> + tuning parameter that may vary for different boards of same SOC.
> + This property is applicable to only QUSB2 v2 PHY.
> + allOf:
> + - $ref: /schemas/types.yaml#/definitions/uint32
> + - minimum: 0
> + maximum: 3
> + default: 0
> +
> qcom,hstx-trim-value:
> description:
> It is a 4 bit value that specifies tuning for HSTX
> @@ -118,6 +140,17 @@ properties:
> maximum: 1
> default: 0
>
> + qcom,hsdisc-trim-value:
> + description:
> + It is a 2 bit value tuning parameter that control disconnect
> + threshold and may vary for different boards of same SOC.
> + This property is applicable to only QUSB2 v2 PHY.
> + allOf:
> + - $ref: /schemas/types.yaml#/definitions/uint32
> + - minimum: 0
> + maximum: 3
> + default: 0
> +
> required:
> - compatible
> - reg

Reviewed-by: Matthias Kaehlcke <[email protected]>

2020-01-29 21:00:22

by Matthias Kaehlcke

[permalink] [raw]
Subject: Re: [PATCH v4 1/8] dt-bindings: phy: qcom,qusb2: Convert QUSB2 phy bindings to yaml

On Wed, Jan 29, 2020 at 12:28:20PM -0800, Matthias Kaehlcke wrote:
> On Wed, Jan 29, 2020 at 07:21:52PM +0530, Sandeep Maheswaram wrote:
> > Convert QUSB2 phy bindings to DT schema format using json-schema.
> >
> > Signed-off-by: Sandeep Maheswaram <[email protected]>
> > ---
> > .../devicetree/bindings/phy/qcom,qusb2-phy.yaml | 142 +++++++++++++++++++++
> > .../devicetree/bindings/phy/qcom-qusb2-phy.txt | 68 ----------
> > 2 files changed, 142 insertions(+), 68 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
> > delete mode 100644 Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
> >
> > 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..90b3cc6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
> > @@ -0,0 +1,142 @@
> > +# 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 <[email protected]>
> > +
> > +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:
> > + maxItems: 1
> > +
> > + "#phy-cells":
> > + const: 0
> > +
> > + clocks:
> > + minItems: 2
> > + items:
> > + - description: phy config clock
> > + - description: 19.2 MHz ref clk
> > + - description: phy interface clock (Optional)
> > +
> > + clock-names:
> > + minItems: 2
> > + items:
> > + - const: cfg_ahb
> > + - const: ref
> > + - const: iface
> > +
> > + 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:
> > + maxItems: 1
> > +
> > + nvmem-cells:
> > + maxItems: 1
> > + 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.
> > + allOf:
> > + - $ref: /schemas/types.yaml#/definitions/uint32
> > + - minimum: 0
> > + maximum: 63
> > + 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.
> > + allOf:
> > + - $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.
> > + allOf:
> > + - $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.
> > + allOf:
> > + - $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>;
> > + 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.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>;
> > - };
>
> Reviewed-by: Matthias Kaehlcke <[email protected]>

This second 'Reviewed-by' was actually intended for '[v4 2/8]
dt-bindings: phy: qcom,qusb2: Add compatibles for QUSB2 V2 phy and SC7180'.
I replied to the wrong mail after reviewing on patchwork.

2020-01-29 21:16:33

by Matthias Kaehlcke

[permalink] [raw]
Subject: Re: [PATCH v4 5/8] phy: qcom-qusb2: Add support for overriding tuning parameters in QUSB2 V2 PHY

Hi Sandeep,

On Wed, Jan 29, 2020 at 07:21:56PM +0530, Sandeep Maheswaram wrote:
> Added new structure for overriding tuning parameters in QUSB2 V2 PHY as the
> override params are increased due to usage of generic QUSB2 V2 phy table.
> Also added bias-ctrl-value,charge-ctrl-value and hsdisc-trim-value params.
>
> Signed-off-by: Sandeep Maheswaram <[email protected]>
> ---
> drivers/phy/qualcomm/phy-qcom-qusb2.c | 125 +++++++++++++++++++++++++---------
> 1 file changed, 93 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qusb2.c b/drivers/phy/qualcomm/phy-qcom-qusb2.c
> index 70c9da6..f45fda3 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qusb2.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qusb2.c
> @@ -66,6 +66,14 @@
> #define IMP_RES_OFFSET_MASK GENMASK(5, 0)
> #define IMP_RES_OFFSET_SHIFT 0x0
>
> +/* QUSB2PHY_PLL_BIAS_CONTROL_2 register bits */
> +#define BIAS_CTRL2_RES_OFFSET_MASK GENMASK(5, 0)
> +#define BIAS_CTRL2_RES_OFFSET_SHIFT 0x0
> +
> +/* QUSB2PHY_CHG_CONTROL_2 register bits */
> +#define CHG_CTRL2_OFFSET_MASK GENMASK(5, 4)
> +#define CHG_CTRL2_OFFSET_SHIFT 0x4
> +
> /* QUSB2PHY_PORT_TUNE1 register bits */
> #define HSTX_TRIM_MASK GENMASK(7, 4)
> #define HSTX_TRIM_SHIFT 0x4
> @@ -73,6 +81,10 @@
> #define PREEMPHASIS_EN_MASK GENMASK(1, 0)
> #define PREEMPHASIS_EN_SHIFT 0x0
>
> +/* QUSB2PHY_PORT_TUNE2 register bits */
> +#define HSDISC_TRIM_MASK GENMASK(1, 0)
> +#define HSDISC_TRIM_SHIFT 0x0
> +
> #define QUSB2PHY_PLL_ANALOG_CONTROLS_TWO 0x04
> #define QUSB2PHY_PLL_CLOCK_INVERTERS 0x18c
> #define QUSB2PHY_PLL_CMODE 0x2c
> @@ -277,6 +289,34 @@ static const char * const qusb2_phy_vreg_names[] = {
>
> #define QUSB2_NUM_VREGS ARRAY_SIZE(qusb2_phy_vreg_names)
>
> +/* struct override_param - structure holding qusb2 v2 phy overriding param
> + * set override true if the device tree property exists and read and assign
> + * to value
> + */
> +struct override_param {
> + bool override;
> + u8 value;
> +};
> +
> +/*struct override_params - structure holding qusb2 v2 phy overriding params
> + * @imp_res_offset: rescode offset to be updated in IMP_CTRL1 register
> + * @hstx_trim: HSTX_TRIM to be updated in TUNE1 register
> + * @preemphasis: Amplitude Pre-Emphasis to be updated in TUNE1 register
> + * @preemphasis_width: half/full-width Pre-Emphasis updated via TUNE1
> + * @bias_ctrl: bias ctrl to be updated in BIAS_CONTROL_2 register
> + * @charge_ctrl: charge ctrl to be updated in CHG_CTRL2 register
> + * @hsdisc_trim: disconnect threshold to be updated in TUNE2 register
> + */
> +struct override_params {
> + struct override_param imp_res_offset;
> + struct override_param hstx_trim;
> + struct override_param preemphasis;
> + struct override_param preemphasis_width;

ideally the refactoring (struct override_param(s)) and the support for
the new override paramters would be in two separate patches, which would
make it easier to review the different steps.

> + struct override_param bias_ctrl;
> + struct override_param charge_ctrl;
> + struct override_param hsdisc_trim;
> +};
> +
> /**
> * struct qusb2_phy - structure holding qusb2 phy attributes
> *
> @@ -292,20 +332,15 @@ static const char * const qusb2_phy_vreg_names[] = {
> * @tcsr: TCSR syscon register map
> * @cell: nvmem cell containing phy tuning value
> *
> - * @override_imp_res_offset: PHY should use different rescode offset
> - * @imp_res_offset_value: rescode offset to be updated in IMP_CTRL1 register
> - * @override_hstx_trim: PHY should use different HSTX o/p current value
> - * @hstx_trim_value: HSTX_TRIM value to be updated in TUNE1 register
> - * @override_preemphasis: PHY should use different pre-amphasis amplitude
> - * @preemphasis_level: Amplitude Pre-Emphasis to be updated in TUNE1 register
> - * @override_preemphasis_width: PHY should use different pre-emphasis duration
> - * @preemphasis_width: half/full-width Pre-Emphasis updated via TUNE1
> + * @overrides: pointer to structure for all overriding tuning params
> *
> * @cfg: phy config data
> * @has_se_clk_scheme: indicate if PHY has single-ended ref clock scheme
> * @phy_initialized: indicate if PHY has been initialized
> * @mode: current PHY mode
> */
> +
> +
> struct qusb2_phy {
> struct phy *phy;
> void __iomem *base;
> @@ -319,14 +354,7 @@ struct qusb2_phy {
> struct regmap *tcsr;
> struct nvmem_cell *cell;
>
> - bool override_imp_res_offset;
> - u8 imp_res_offset_value;
> - bool override_hstx_trim;
> - u8 hstx_trim_value;
> - bool override_preemphasis;
> - u8 preemphasis_level;
> - bool override_preemphasis_width;
> - u8 preemphasis_width;
> + struct override_params overrides;
>
> const struct qusb2_phy_cfg *cfg;
> bool has_se_clk_scheme;
> @@ -395,23 +423,33 @@ static void qusb2_phy_override_phy_params(struct qusb2_phy *qphy)
> {
> const struct qusb2_phy_cfg *cfg = qphy->cfg;
>
> - if (qphy->override_imp_res_offset)
> + if (qphy->overrides.imp_res_offset.override)

you could consider introducing a local variable 'struct override_params
overrides *or' and assign it to &qphy->overrides, which would make
accessing the overrides slightly less clunky.

> qusb2_write_mask(qphy->base, QUSB2PHY_IMP_CTRL1,
> - qphy->imp_res_offset_value << IMP_RES_OFFSET_SHIFT,
> + qphy->overrides.imp_res_offset.value << IMP_RES_OFFSET_SHIFT,
> IMP_RES_OFFSET_MASK);
>
> - if (qphy->override_hstx_trim)
> + if (qphy->overrides.bias_ctrl.override)
> + qusb2_write_mask(qphy->base, QUSB2PHY_PLL_BIAS_CONTROL_2,
> + qphy->overrides.bias_ctrl.value << BIAS_CTRL2_RES_OFFSET_SHIFT,
> + BIAS_CTRL2_RES_OFFSET_MASK);
> +
> + if (qphy->overrides.charge_ctrl.override)
> + qusb2_write_mask(qphy->base, QUSB2PHY_CHG_CTRL2,
> + qphy->overrides.charge_ctrl.value << CHG_CTRL2_OFFSET_SHIFT,
> + CHG_CTRL2_OFFSET_MASK);
> +
> + if (qphy->overrides.hstx_trim.override)
> qusb2_write_mask(qphy->base, cfg->regs[QUSB2PHY_PORT_TUNE1],
> - qphy->hstx_trim_value << HSTX_TRIM_SHIFT,
> + qphy->overrides.hstx_trim.value << HSTX_TRIM_SHIFT,
> HSTX_TRIM_MASK);
>
> - if (qphy->override_preemphasis)
> + if (qphy->overrides.preemphasis.override)
> qusb2_write_mask(qphy->base, cfg->regs[QUSB2PHY_PORT_TUNE1],
> - qphy->preemphasis_level << PREEMPHASIS_EN_SHIFT,
> + qphy->overrides.preemphasis.value << PREEMPHASIS_EN_SHIFT,
> PREEMPHASIS_EN_MASK);
>
> - if (qphy->override_preemphasis_width) {
> - if (qphy->preemphasis_width ==
> + if (qphy->overrides.preemphasis_width.override) {
> + if (qphy->overrides.preemphasis_width.value ==
> QUSB2_V2_PREEMPHASIS_WIDTH_HALF_BIT)
> qusb2_setbits(qphy->base,
> cfg->regs[QUSB2PHY_PORT_TUNE1],
> @@ -421,6 +459,11 @@ static void qusb2_phy_override_phy_params(struct qusb2_phy *qphy)
> cfg->regs[QUSB2PHY_PORT_TUNE1],
> PREEMPH_WIDTH_HALF_BIT);
> }
> +
> + if (qphy->overrides.hsdisc_trim.override)
> + qusb2_write_mask(qphy->base, cfg->regs[QUSB2PHY_PORT_TUNE2],
> + qphy->overrides.hsdisc_trim.value << HSDISC_TRIM_SHIFT,
> + HSDISC_TRIM_MASK);
> }
>
> /*
> @@ -864,26 +907,44 @@ static int qusb2_phy_probe(struct platform_device *pdev)
>
> if (!of_property_read_u32(dev->of_node, "qcom,imp-res-offset-value",
> &value)) {
> - qphy->imp_res_offset_value = (u8)value;
> - qphy->override_imp_res_offset = true;
> + qphy->overrides.imp_res_offset.value = (u8)value;

same as above, consider whether 'or->imp_res_offset.value' is an improvement.

> + qphy->overrides.imp_res_offset.override = true;
> + }
> +
> + if (!of_property_read_u32(dev->of_node, "qcom,bias-ctrl-value",
> + &value)) {
> + qphy->overrides.bias_ctrl.value = (u8)value;
> + qphy->overrides.bias_ctrl.override = true;
> + }
> +
> + if (!of_property_read_u32(dev->of_node, "qcom,charge-ctrl-value",
> + &value)) {
> + qphy->overrides.charge_ctrl.value = (u8)value;
> + qphy->overrides.charge_ctrl.override = true;
> }
>
> if (!of_property_read_u32(dev->of_node, "qcom,hstx-trim-value",
> &value)) {
> - qphy->hstx_trim_value = (u8)value;
> - qphy->override_hstx_trim = true;
> + qphy->overrides.hstx_trim.value = (u8)value;
> + qphy->overrides.hstx_trim.override = true;
> }
>
> if (!of_property_read_u32(dev->of_node, "qcom,preemphasis-level",
> &value)) {
> - qphy->preemphasis_level = (u8)value;
> - qphy->override_preemphasis = true;
> + qphy->overrides.preemphasis.value = (u8)value;
> + qphy->overrides.preemphasis.override = true;
> }
>
> if (!of_property_read_u32(dev->of_node, "qcom,preemphasis-width",
> &value)) {
> - qphy->preemphasis_width = (u8)value;
> - qphy->override_preemphasis_width = true;
> + qphy->overrides.preemphasis_width.value = (u8)value;
> + qphy->overrides.preemphasis_width.override = true;
> + }
> +
> + if (!of_property_read_u32(dev->of_node, "qcom,hsdisc-trim-value",
> + &value)) {
> + qphy->overrides.hsdisc_trim.value = (u8)value;
> + qphy->overrides.hsdisc_trim.override = true;
> }
>
> pm_runtime_set_active(dev);

Reviewed-by: Matthias Kaehlcke <[email protected]>

2020-01-29 21:18:13

by Matthias Kaehlcke

[permalink] [raw]
Subject: Re: [PATCH v4 2/8] dt-bindings: phy: qcom,qusb2: Add compatibles for QUSB2 V2 phy and SC7180

On Wed, Jan 29, 2020 at 07:21:53PM +0530, Sandeep Maheswaram wrote:
> Add compatibles for generic QUSB2 V2 phy which can be used for
> sdm845 and sc7180.
>
> Signed-off-by: Sandeep Maheswaram <[email protected]>
> ---
> Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
> index 90b3cc6..43082c8 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
> @@ -15,10 +15,17 @@ description:
>
> properties:
> compatible:
> - enum:
> - - qcom,msm8996-qusb2-phy
> - - qcom,msm8998-qusb2-phy
> - - qcom,sdm845-qusb2-phy
> + oneOf:
> + - items:
> + - enum:
> + - qcom,msm8996-qusb2-phy
> + - qcom,msm8998-qusb2-phy
> + - qcom,qusb2-v2-phy
> + - items:
> + - enum:
> + - qcom,sc7180-qusb2-phy
> + - qcom,sdm845-qusb2-phy
> + - const: qcom,qusb2-v2-phy
> reg:
> maxItems: 1

Reviewed-by: Matthias Kaehlcke <[email protected]>

2020-01-29 21:36:18

by Matthias Kaehlcke

[permalink] [raw]
Subject: Re: [PATCH v4 6/8] arm64: dts: qcom: sc7180: Add generic QUSB2 V2 Phy compatible

On Wed, Jan 29, 2020 at 07:21:57PM +0530, Sandeep Maheswaram wrote:
> Use generic QUSB2 V2 Phy configuration for SC7180.
>
> Signed-off-by: Sandeep Maheswaram <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> index 8011c5f..0d6761b 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> @@ -1052,7 +1052,7 @@
> };
>
> usb_1_hsphy: phy@88e3000 {
> - compatible = "qcom,sc7180-qusb2-phy";
> + compatible = "qcom,sc7180-qusb2-phy", "qcom,qusb2-v2-phy";
> reg = <0 0x088e3000 0 0x400>;
> status = "disabled";
> #phy-cells = <0>;

FWIW

Reviewed-by: Matthias Kaehlcke <[email protected]>

2020-01-29 21:37:51

by Matthias Kaehlcke

[permalink] [raw]
Subject: Re: [PATCH v4 7/8] arm64: dts: qcom: sdm845: Add generic QUSB2 V2 Phy compatible

On Wed, Jan 29, 2020 at 07:21:58PM +0530, Sandeep Maheswaram wrote:
> Use generic QUSB2 V2 Phy configuration for sdm845.
>
> Signed-off-by: Sandeep Maheswaram <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index d42302b..317347a 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -2387,7 +2387,7 @@
> };
>
> usb_1_hsphy: phy@88e2000 {
> - compatible = "qcom,sdm845-qusb2-phy";
> + compatible = "qcom,sdm845-qusb2-phy", "qcom,qusb2-v2-phy";
> reg = <0 0x088e2000 0 0x400>;
> status = "disabled";
> #phy-cells = <0>;
> @@ -2402,7 +2402,7 @@
> };
>
> usb_2_hsphy: phy@88e3000 {
> - compatible = "qcom,sdm845-qusb2-phy";
> + compatible = "qcom,sdm845-qusb2-phy", "qcom,qusb2-v2-phy";
> reg = <0 0x088e3000 0 0x400>;
> status = "disabled";
> #phy-cells = <0>;

FWIW

Reviewed-by: Matthias Kaehlcke <[email protected]>

2020-02-03 19:17:40

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v4 0/8] Add QUSB2 PHY support for SC7180

On Wed 29 Jan 05:51 PST 2020, Sandeep Maheswaram wrote:

Kishon, afaict this is all reviewed, let me know when you're taking the
phy pieces and I'll pick up the dts changes.

Regards,
Bjorn

> Converting dt binding to yaml.
> Adding compatible for SC7180 in dt bindings.
> Added generic QUSB2 V2 PHY support and using the same SC7180 and SDM845.
>
> Changes in v4:
> *Addressed Rob Herrings comments in dt bindings.
> *Added new structure for all the overriding tuning params.
> *Removed the sc7180 and sdm845 compatible from driver and added qusb2 v2 phy.
> *Added the qusb2 v2 phy compatible in device tree for sc7180 and sdm845.
>
> Changes in v3:
> *Using the generic phy cfg table for QUSB2 V2 phy.
> *Added support for overriding tuning parameters in QUSB2 V2 PHY
> from device tree.
>
> Changes in v2:
> Sorted the compatible in driver.
> Converted dt binding to yaml.
> Added compatible in yaml.
>
> Sandeep Maheswaram (8):
> dt-bindings: phy: qcom,qusb2: Convert QUSB2 phy bindings to yaml
> dt-bindings: phy: qcom,qusb2: Add compatibles for QUSB2 V2 phy and
> SC7180
> phy: qcom-qusb2: Add generic QUSB2 V2 PHY support
> dt-bindings: phy: qcom-qusb2: Add support for overriding Phy tuning
> parameters
> phy: qcom-qusb2: Add support for overriding tuning parameters in QUSB2
> V2 PHY
> arm64: dts: qcom: sc7180: Add generic QUSB2 V2 Phy compatible
> arm64: dts: qcom: sdm845: Add generic QUSB2 V2 Phy compatible
> arm64: dts: qcom: sc7180: Update QUSB2 V2 Phy params for SC7180 IDP
> device
>
> .../devicetree/bindings/phy/qcom,qusb2-phy.yaml | 182 +++++++++++++++++++++
> .../devicetree/bindings/phy/qcom-qusb2-phy.txt | 68 --------
> arch/arm64/boot/dts/qcom/sc7180-idp.dts | 6 +-
> arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +-
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 4 +-
> drivers/phy/qualcomm/phy-qcom-qusb2.c | 143 +++++++++++-----
> 6 files changed, 291 insertions(+), 114 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
> delete mode 100644 Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
>

2020-02-04 00:40:19

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v4 1/8] dt-bindings: phy: qcom,qusb2: Convert QUSB2 phy bindings to yaml

Quoting Sandeep Maheswaram (2020-01-29 05:51:52)
> Convert QUSB2 phy bindings to DT schema format using json-schema.
>
> Signed-off-by: Sandeep Maheswaram <[email protected]>
> ---

Reviewed-by: Stephen Boyd <[email protected]>

2020-02-04 00:41:08

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v4 2/8] dt-bindings: phy: qcom,qusb2: Add compatibles for QUSB2 V2 phy and SC7180

Quoting Sandeep Maheswaram (2020-01-29 05:51:53)
> Add compatibles for generic QUSB2 V2 phy which can be used for
> sdm845 and sc7180.
>
> Signed-off-by: Sandeep Maheswaram <[email protected]>
> ---

Reviewed-by: Stephen Boyd <[email protected]>

2020-02-04 00:42:31

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v4 3/8] phy: qcom-qusb2: Add generic QUSB2 V2 PHY support

Quoting Sandeep Maheswaram (2020-01-29 05:51:54)
> Add generic QUSB2 V2 PHY table so the respective phys
> can use the same table.
>
> Signed-off-by: Sandeep Maheswaram <[email protected]>
> ---

Reviewed-by: Stephen Boyd <[email protected]>

2020-02-04 00:44:13

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v4 4/8] dt-bindings: phy: qcom-qusb2: Add support for overriding Phy tuning parameters

Quoting Matthias Kaehlcke (2020-01-29 12:38:19)
> On Wed, Jan 29, 2020 at 07:21:55PM +0530, Sandeep Maheswaram wrote:
> > Add support for overriding QUSB2 V2 phy tuning parameters
> > in device tree bindings.
> >
> > Signed-off-by: Sandeep Maheswaram <[email protected]>
> > Reviewed-by: Rob Herring <[email protected]>
> > ---
> > .../devicetree/bindings/phy/qcom,qusb2-phy.yaml | 33 ++++++++++++++++++++++
> > 1 file changed, 33 insertions(+)

Reviewed-by: Stephen Boyd <[email protected]>

> >
> > diff --git a/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
> > index 43082c8..dfef356 100644
> > --- a/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
> > +++ b/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
> > @@ -80,6 +80,28 @@ properties:
> > maximum: 63
> > default: 0
> >
> > + qcom,bias-ctrl-value:
> > + description:
> > + It is a 6 bit value that specifies bias-ctrl-value. It is a PHY
> > + tuning parameter that may vary for different boards of same SOC.
> > + This property is applicable to only QUSB2 v2 PHY.
>
> As commented on 'dt-bindings: phy: qcom,qusb2: Convert QUSB2 phy bindings
> to yaml' a possible improvement could be to restrict these properties to
> the QUSB2 v2 PHY through the schema.

Can this be done? It's nice to keep constraints type, otherwise the
yaml binding is not as useful.


2020-02-04 00:53:17

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v4 5/8] phy: qcom-qusb2: Add support for overriding tuning parameters in QUSB2 V2 PHY

Quoting Sandeep Maheswaram (2020-01-29 05:51:56)
> @@ -277,6 +289,34 @@ static const char * const qusb2_phy_vreg_names[] = {
>
> #define QUSB2_NUM_VREGS ARRAY_SIZE(qusb2_phy_vreg_names)
>
> +/* struct override_param - structure holding qusb2 v2 phy overriding param
> + * set override true if the device tree property exists and read and assign
> + * to value
> + */
> +struct override_param {
> + bool override;
> + u8 value;
> +};
> +
> +/*struct override_params - structure holding qusb2 v2 phy overriding params
> + * @imp_res_offset: rescode offset to be updated in IMP_CTRL1 register
> + * @hstx_trim: HSTX_TRIM to be updated in TUNE1 register
> + * @preemphasis: Amplitude Pre-Emphasis to be updated in TUNE1 register
> + * @preemphasis_width: half/full-width Pre-Emphasis updated via TUNE1
> + * @bias_ctrl: bias ctrl to be updated in BIAS_CONTROL_2 register
> + * @charge_ctrl: charge ctrl to be updated in CHG_CTRL2 register
> + * @hsdisc_trim: disconnect threshold to be updated in TUNE2 register
> + */
> +struct override_params {
> + struct override_param imp_res_offset;
> + struct override_param hstx_trim;
> + struct override_param preemphasis;
> + struct override_param preemphasis_width;
> + struct override_param bias_ctrl;
> + struct override_param charge_ctrl;
> + struct override_param hsdisc_trim;
> +};
> +
> /**
> * struct qusb2_phy - structure holding qusb2 phy attributes
> *
> @@ -395,23 +423,33 @@ static void qusb2_phy_override_phy_params(struct qusb2_phy *qphy)
> @@ -421,6 +459,11 @@ static void qusb2_phy_override_phy_params(struct qusb2_phy *qphy)
> cfg->regs[QUSB2PHY_PORT_TUNE1],
> PREEMPH_WIDTH_HALF_BIT);
> }
> +
> + if (qphy->overrides.hsdisc_trim.override)
> + qusb2_write_mask(qphy->base, cfg->regs[QUSB2PHY_PORT_TUNE2],
> + qphy->overrides.hsdisc_trim.value << HSDISC_TRIM_SHIFT,
> + HSDISC_TRIM_MASK);
> }
>
> /*
> @@ -864,26 +907,44 @@ static int qusb2_phy_probe(struct platform_device *pdev)
>
[...]
> if (!of_property_read_u32(dev->of_node, "qcom,preemphasis-width",
> &value)) {
> - qphy->preemphasis_width = (u8)value;
> - qphy->override_preemphasis_width = true;
> + qphy->overrides.preemphasis_width.value = (u8)value;
> + qphy->overrides.preemphasis_width.override = true;
> + }
> +
> + if (!of_property_read_u32(dev->of_node, "qcom,hsdisc-trim-value",
> + &value)) {
> + qphy->overrides.hsdisc_trim.value = (u8)value;
> + qphy->overrides.hsdisc_trim.override = true;
> }
>

Is it possible to make a local array that we can crank through the
overrides with? If they're all u8 values maybe we can have an array like

const char * const char override_names[] = {
[HSDISC_TRIM_VALUE] = "qcom,hsdisc-trim-value",
[PREEMP_WIDTH] = ...
};

that we then link to another array inside qphy named overrides:

struct override_param overrides[NUM_OVERRIDES];

and then we can bounce from overrides to override_names to parse out the
random u8 values from the DT node. The idea is to avoid "wasting"
pointers to the name when we don't care after parsing it. It may not be
any different vs. just having a const char *name in the override_paramt
struct though, so please measure it.

Also, why not use of_property_read_u8() and make DT writers have

/bits/ 8 <0xf0>

properties so that we can keep things smaller. I don't understand why
they're u32 in DT besides to make it simpler to specify a u32.

2020-02-04 00:53:56

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v4 6/8] arm64: dts: qcom: sc7180: Add generic QUSB2 V2 Phy compatible

Quoting Sandeep Maheswaram (2020-01-29 05:51:57)
> Use generic QUSB2 V2 Phy configuration for SC7180.
>
> Signed-off-by: Sandeep Maheswaram <[email protected]>
> ---

Reviewed-by: Stephen Boyd <[email protected]>

2020-02-04 00:54:15

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v4 7/8] arm64: dts: qcom: sdm845: Add generic QUSB2 V2 Phy compatible

Quoting Sandeep Maheswaram (2020-01-29 05:51:58)
> Use generic QUSB2 V2 Phy configuration for sdm845.
>
> Signed-off-by: Sandeep Maheswaram <[email protected]>
> ---

Reviewed-by: Stephen Boyd <[email protected]>

2020-02-04 00:54:48

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v4 8/8] arm64: dts: qcom: sc7180: Update QUSB2 V2 Phy params for SC7180 IDP device

Quoting Sandeep Maheswaram (2020-01-29 05:51:59)
> Overriding the QUSB2 V2 Phy tuning parameters for SC7180 IDP device.
>
> Signed-off-by: Sandeep Maheswaram <[email protected]>
> ---

Reviewed-by: Stephen Boyd <[email protected]>

2020-02-04 00:55:01

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v4 8/8] arm64: dts: qcom: sc7180: Update QUSB2 V2 Phy params for SC7180 IDP device

Quoting Sandeep Maheswaram (2020-01-29 05:51:59)
> Overriding the QUSB2 V2 Phy tuning parameters for SC7180 IDP device.
>
> Signed-off-by: Sandeep Maheswaram <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sc7180-idp.dts | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> index 388f50a..826cf02 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> @@ -276,9 +276,11 @@
> vdda-pll-supply = <&vreg_l11a_1p8>;
> vdda-phy-dpdm-supply = <&vreg_l17a_3p0>;
> qcom,imp-res-offset-value = <8>;
> - qcom,hstx-trim-value = <QUSB2_V2_HSTX_TRIM_21_6_MA>;
> - qcom,preemphasis-level = <QUSB2_V2_PREEMPHASIS_5_PERCENT>;
> + qcom,preemphasis-level = <QUSB2_V2_PREEMPHASIS_15_PERCENT>;
> qcom,preemphasis-width = <QUSB2_V2_PREEMPHASIS_WIDTH_HALF_BIT>;
> + qcom,bias-ctrl-value = <0x22>;
> + qcom,charge-ctrl-value = <3>;
> + qcom,hsdisc-trim-value = <0>;

Actually, I'd prefer it uses /bits/ 8 here if it's just 8 bits.

2020-02-04 17:05:18

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH v4 8/8] arm64: dts: qcom: sc7180: Update QUSB2 V2 Phy params for SC7180 IDP device

Hi,

On Mon, Feb 3, 2020 at 4:53 PM Stephen Boyd <[email protected]> wrote:
>
> Quoting Sandeep Maheswaram (2020-01-29 05:51:59)
> > Overriding the QUSB2 V2 Phy tuning parameters for SC7180 IDP device.
> >
> > Signed-off-by: Sandeep Maheswaram <[email protected]>
> > ---
> > arch/arm64/boot/dts/qcom/sc7180-idp.dts | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> > index 388f50a..826cf02 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> > +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> > @@ -276,9 +276,11 @@
> > vdda-pll-supply = <&vreg_l11a_1p8>;
> > vdda-phy-dpdm-supply = <&vreg_l17a_3p0>;
> > qcom,imp-res-offset-value = <8>;
> > - qcom,hstx-trim-value = <QUSB2_V2_HSTX_TRIM_21_6_MA>;
> > - qcom,preemphasis-level = <QUSB2_V2_PREEMPHASIS_5_PERCENT>;
> > + qcom,preemphasis-level = <QUSB2_V2_PREEMPHASIS_15_PERCENT>;
> > qcom,preemphasis-width = <QUSB2_V2_PREEMPHASIS_WIDTH_HALF_BIT>;
> > + qcom,bias-ctrl-value = <0x22>;
> > + qcom,charge-ctrl-value = <3>;
> > + qcom,hsdisc-trim-value = <0>;
>
> Actually, I'd prefer it uses /bits/ 8 here if it's just 8 bits.

I have been giving the opposite advice and I thought in general Rob H
suggests against adding "/bits/ 8" unless it's absolutely needed (like
for an array).

One example from
<http://lore.kernel.org/r/97cd8c1d98d7406347e4e48f4c7383a394a2ae93.1451997697.git.oreste.salerno@tomtom.com>

> > + lp_intrvl = /bits/ 8 <0x0A>;
>
> If the size is not 32-bits, you need to state that in the description.
> There is not really much point in making these 8-bit though.

-Doug

2020-02-04 17:26:25

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH v4 5/8] phy: qcom-qusb2: Add support for overriding tuning parameters in QUSB2 V2 PHY

Hi,

On Mon, Feb 3, 2020 at 4:52 PM Stephen Boyd <[email protected]> wrote:
>
> Also, why not use of_property_read_u8() and make DT writers have
>
> /bits/ 8 <0xf0>
>
> properties so that we can keep things smaller. I don't understand why
> they're u32 in DT besides to make it simpler to specify a u32.

As per the other thread, I think it's discouraged to specify /bits/ 8
in DT unless it's really needed.

2020-02-06 20:47:12

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v4 2/8] dt-bindings: phy: qcom,qusb2: Add compatibles for QUSB2 V2 phy and SC7180

On Wed, Jan 29, 2020 at 07:21:53PM +0530, Sandeep Maheswaram wrote:
> Add compatibles for generic QUSB2 V2 phy which can be used for
> sdm845 and sc7180.
>
> Signed-off-by: Sandeep Maheswaram <[email protected]>
> ---
> Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
> index 90b3cc6..43082c8 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
> @@ -15,10 +15,17 @@ description:
>
> properties:
> compatible:
> - enum:
> - - qcom,msm8996-qusb2-phy
> - - qcom,msm8998-qusb2-phy
> - - qcom,sdm845-qusb2-phy
> + oneOf:
> + - items:

You can omit 'items' here.

> + - enum:
> + - qcom,msm8996-qusb2-phy
> + - qcom,msm8998-qusb2-phy
> + - qcom,qusb2-v2-phy

This should not be valid alone. An SoC specific compatible is required.

> + - items:
> + - enum:
> + - qcom,sc7180-qusb2-phy
> + - qcom,sdm845-qusb2-phy
> + - const: qcom,qusb2-v2-phy

Is your intention that qcom,sdm845-qusb2-phy alone is no longer valid?

Rob

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

2020-02-06 20:47:21

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v4 1/8] dt-bindings: phy: qcom,qusb2: Convert QUSB2 phy bindings to yaml

On Wed, Jan 29, 2020 at 07:21:52PM +0530, Sandeep Maheswaram wrote:
> Convert QUSB2 phy bindings to DT schema format using json-schema.
>
> Signed-off-by: Sandeep Maheswaram <[email protected]>
> ---
> .../devicetree/bindings/phy/qcom,qusb2-phy.yaml | 142 +++++++++++++++++++++
> .../devicetree/bindings/phy/qcom-qusb2-phy.txt | 68 ----------
> 2 files changed, 142 insertions(+), 68 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
> delete mode 100644 Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
>
> 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..90b3cc6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
> @@ -0,0 +1,142 @@
> +# 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 <[email protected]>
> +
> +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:
> + maxItems: 1
> +
> + "#phy-cells":
> + const: 0
> +
> + clocks:
> + minItems: 2
> + items:
> + - description: phy config clock
> + - description: 19.2 MHz ref clk
> + - description: phy interface clock (Optional)
> +
> + clock-names:
> + minItems: 2
> + items:
> + - const: cfg_ahb
> + - const: ref
> + - const: iface
> +
> + 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:
> + maxItems: 1
> +
> + nvmem-cells:
> + maxItems: 1
> + 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

s/cell/phandle/

With that,

Reviewed-by: Rob Herring <[email protected]>

2020-03-05 18:53:18

by Matthias Kaehlcke

[permalink] [raw]
Subject: Re: [PATCH v4 0/8] Add QUSB2 PHY support for SC7180

On Mon, Feb 03, 2020 at 10:56:49AM -0800, Bjorn Andersson wrote:
> On Wed 29 Jan 05:51 PST 2020, Sandeep Maheswaram wrote:
>
> Kishon, afaict this is all reviewed, let me know when you're taking the
> phy pieces and I'll pick up the dts changes.

The series has a few minor comments. Sandeep, could you respin the
series so that it can be landed?

Thanks

Matthias

> > Converting dt binding to yaml.
> > Adding compatible for SC7180 in dt bindings.
> > Added generic QUSB2 V2 PHY support and using the same SC7180 and SDM845.
> >
> > Changes in v4:
> > *Addressed Rob Herrings comments in dt bindings.
> > *Added new structure for all the overriding tuning params.
> > *Removed the sc7180 and sdm845 compatible from driver and added qusb2 v2 phy.
> > *Added the qusb2 v2 phy compatible in device tree for sc7180 and sdm845.
> >
> > Changes in v3:
> > *Using the generic phy cfg table for QUSB2 V2 phy.
> > *Added support for overriding tuning parameters in QUSB2 V2 PHY
> > from device tree.
> >
> > Changes in v2:
> > Sorted the compatible in driver.
> > Converted dt binding to yaml.
> > Added compatible in yaml.
> >
> > Sandeep Maheswaram (8):
> > dt-bindings: phy: qcom,qusb2: Convert QUSB2 phy bindings to yaml
> > dt-bindings: phy: qcom,qusb2: Add compatibles for QUSB2 V2 phy and
> > SC7180
> > phy: qcom-qusb2: Add generic QUSB2 V2 PHY support
> > dt-bindings: phy: qcom-qusb2: Add support for overriding Phy tuning
> > parameters
> > phy: qcom-qusb2: Add support for overriding tuning parameters in QUSB2
> > V2 PHY
> > arm64: dts: qcom: sc7180: Add generic QUSB2 V2 Phy compatible
> > arm64: dts: qcom: sdm845: Add generic QUSB2 V2 Phy compatible
> > arm64: dts: qcom: sc7180: Update QUSB2 V2 Phy params for SC7180 IDP
> > device
> >
> > .../devicetree/bindings/phy/qcom,qusb2-phy.yaml | 182 +++++++++++++++++++++
> > .../devicetree/bindings/phy/qcom-qusb2-phy.txt | 68 --------
> > arch/arm64/boot/dts/qcom/sc7180-idp.dts | 6 +-
> > arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +-
> > arch/arm64/boot/dts/qcom/sdm845.dtsi | 4 +-
> > drivers/phy/qualcomm/phy-qcom-qusb2.c | 143 +++++++++++-----
> > 6 files changed, 291 insertions(+), 114 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
> > delete mode 100644 Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
> >
> > --
> > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> > of Code Aurora Forum, hosted by The Linux Foundation
> >

2020-03-06 11:22:58

by Sandeep Maheswaram

[permalink] [raw]
Subject: Re: [PATCH v4 0/8] Add QUSB2 PHY support for SC7180

Hi Matthias,

Will do it by 3/9.

Regards

Sandeep

On 3/6/2020 12:21 AM, Matthias Kaehlcke wrote:
> On Mon, Feb 03, 2020 at 10:56:49AM -0800, Bjorn Andersson wrote:
>> On Wed 29 Jan 05:51 PST 2020, Sandeep Maheswaram wrote:
>>
>> Kishon, afaict this is all reviewed, let me know when you're taking the
>> phy pieces and I'll pick up the dts changes.
> The series has a few minor comments. Sandeep, could you respin the
> series so that it can be landed?
>
> Thanks
>
> Matthias
>
>>> Converting dt binding to yaml.
>>> Adding compatible for SC7180 in dt bindings.
>>> Added generic QUSB2 V2 PHY support and using the same SC7180 and SDM845.
>>>
>>> Changes in v4:
>>> *Addressed Rob Herrings comments in dt bindings.
>>> *Added new structure for all the overriding tuning params.
>>> *Removed the sc7180 and sdm845 compatible from driver and added qusb2 v2 phy.
>>> *Added the qusb2 v2 phy compatible in device tree for sc7180 and sdm845.
>>>
>>> Changes in v3:
>>> *Using the generic phy cfg table for QUSB2 V2 phy.
>>> *Added support for overriding tuning parameters in QUSB2 V2 PHY
>>> from device tree.
>>>
>>> Changes in v2:
>>> Sorted the compatible in driver.
>>> Converted dt binding to yaml.
>>> Added compatible in yaml.
>>>
>>> Sandeep Maheswaram (8):
>>> dt-bindings: phy: qcom,qusb2: Convert QUSB2 phy bindings to yaml
>>> dt-bindings: phy: qcom,qusb2: Add compatibles for QUSB2 V2 phy and
>>> SC7180
>>> phy: qcom-qusb2: Add generic QUSB2 V2 PHY support
>>> dt-bindings: phy: qcom-qusb2: Add support for overriding Phy tuning
>>> parameters
>>> phy: qcom-qusb2: Add support for overriding tuning parameters in QUSB2
>>> V2 PHY
>>> arm64: dts: qcom: sc7180: Add generic QUSB2 V2 Phy compatible
>>> arm64: dts: qcom: sdm845: Add generic QUSB2 V2 Phy compatible
>>> arm64: dts: qcom: sc7180: Update QUSB2 V2 Phy params for SC7180 IDP
>>> device
>>>
>>> .../devicetree/bindings/phy/qcom,qusb2-phy.yaml | 182 +++++++++++++++++++++
>>> .../devicetree/bindings/phy/qcom-qusb2-phy.txt | 68 --------
>>> arch/arm64/boot/dts/qcom/sc7180-idp.dts | 6 +-
>>> arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +-
>>> arch/arm64/boot/dts/qcom/sdm845.dtsi | 4 +-
>>> drivers/phy/qualcomm/phy-qcom-qusb2.c | 143 +++++++++++-----
>>> 6 files changed, 291 insertions(+), 114 deletions(-)
>>> create mode 100644 Documentation/devicetree/bindings/phy/qcom,qusb2-phy.yaml
>>> delete mode 100644 Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
>>>
>>> --
>>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
>>> of Code Aurora Forum, hosted by The Linux Foundation
>>>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation