[PATCH 0/3] Add QCOM SNPS PHY overriding params support

[PATCH 0/3] Add QCOM SNPS PHY overriding params support

[Sandeep Maheswaram]

Added support for overriding tuning parameters in QCOM SNPS PHY
from device tree.

Sandeep Maheswaram (3):
  dt-bindings: phy: qcom,usb-snps-femto-v2: Add phy override params
    bindings
  phy: qcom-snps: Add support for overriding phy tuning parameters
  arm64: dts: qcom: sc7280: Update SNPS Phy params for SC7280 IDP device

 .../bindings/phy/qcom,usb-snps-femto-v2.yaml       | 16 ++++++++
 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi           |  4 ++
 drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c      | 45 ++++++++++++++++++++++
 3 files changed, 65 insertions(+)

-- 
2.7.4

[PATCH 1/3] dt-bindings: phy: qcom,usb-snps-femto-v2: Add phy override params bindings

[Sandeep Maheswaram]

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

Signed-off-by: Sandeep Maheswaram <quic_c_sanm@quicinc.com>
---
 .../devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml  | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml b/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml
index 0dfe691..44cf3bf 100644
--- a/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml
@@ -50,6 +50,22 @@ properties:
   vdda33-supply:
     description: phandle to the regulator 3.3V supply node.
 
+  qcom,override_x0:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Override value for hs disconnect and squelch detector threshold.
+
+  qcom,override_x1:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Override value for hs amplitude, pre-emphasis duration and amplitude.
+
+  qcom,override_x2:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Override value for hs rise/fall time, cross over voltage, output impedance.
+
+  qcom,override_x3:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Override value for ls/fs output impedance.
+
 required:
   - compatible
   - reg
-- 
2.7.4

Re: [PATCH 1/3] dt-bindings: phy: qcom,usb-snps-femto-v2: Add phy override params bindings

[Dmitry Baryshkov]

On 15/02/2022 22:19, Sandeep Maheswaram wrote:
> Add support for overriding SNPS phy tuning parameters in device tree
> bindings.

This does not really benefit the users and does not help developers.
Could you please change the dt bindings to specify values for 
thresholds, durations, impedance, etc. The values should be represented 
in the human units (e.g. us, Ohms, mV), not in the internal register 
'bits' representation.

> 
> Signed-off-by: Sandeep Maheswaram <quic_c_sanm@quicinc.com>
> ---
>   .../devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml  | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml b/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml
> index 0dfe691..44cf3bf 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml
> @@ -50,6 +50,22 @@ properties:
>     vdda33-supply:
>       description: phandle to the regulator 3.3V supply node.
>   
> +  qcom,override_x0:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Override value for hs disconnect and squelch detector threshold.
> +
> +  qcom,override_x1:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Override value for hs amplitude, pre-emphasis duration and amplitude.
> +
> +  qcom,override_x2:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Override value for hs rise/fall time, cross over voltage, output impedance.
> +
> +  qcom,override_x3:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Override value for ls/fs output impedance.
> +
>   required:
>     - compatible
>     - reg


-- 
With best wishes
Dmitry

Re: [PATCH 1/3] dt-bindings: phy: qcom,usb-snps-femto-v2: Add phy override params bindings

[Stephen Boyd]

Quoting Dmitry Baryshkov (2022-02-15 11:55:18)
> On 15/02/2022 22:19, Sandeep Maheswaram wrote:
> > Add support for overriding SNPS phy tuning parameters in device tree
> > bindings.
>
> This does not really benefit the users and does not help developers.
> Could you please change the dt bindings to specify values for
> thresholds, durations, impedance, etc. The values should be represented
> in the human units (e.g. us, Ohms, mV), not in the internal register
> 'bits' representation.

+1

Re: [PATCH 1/3] dt-bindings: phy: qcom,usb-snps-femto-v2: Add phy override params bindings

[Pavan Kondeti]

On Tue, Feb 15, 2022 at 06:10:45PM -0800, Stephen Boyd wrote:
> Quoting Dmitry Baryshkov (2022-02-15 11:55:18)
> > On 15/02/2022 22:19, Sandeep Maheswaram wrote:
> > > Add support for overriding SNPS phy tuning parameters in device tree
> > > bindings.
> >
> > This does not really benefit the users and does not help developers.
> > Could you please change the dt bindings to specify values for
> > thresholds, durations, impedance, etc. The values should be represented
> > in the human units (e.g. us, Ohms, mV), not in the internal register
> > 'bits' representation.
> 
> +1

Agreed to this proposal.

Sandeep,

We have a similar implemention in QUSB phy driver. can we have something like
that for SNPSHS PHY too?

Thanks,
Pavan

Re: [PATCH 1/3] dt-bindings: phy: qcom,usb-snps-femto-v2: Add phy override params bindings

[Sandeep Maheswaram]

On 2/16/2022 9:17 AM, Pavan Kondeti wrote:
> On Tue, Feb 15, 2022 at 06:10:45PM -0800, Stephen Boyd wrote:
>> Quoting Dmitry Baryshkov (2022-02-15 11:55:18)
>>> On 15/02/2022 22:19, Sandeep Maheswaram wrote:
>>>> Add support for overriding SNPS phy tuning parameters in device tree
>>>> bindings.
>>> This does not really benefit the users and does not help developers.
>>> Could you please change the dt bindings to specify values for
>>> thresholds, durations, impedance, etc. The values should be represented
>>> in the human units (e.g. us, Ohms, mV), not in the internal register
>>> 'bits' representation.
>> +1
> Agreed to this proposal.
>
> Sandeep,
>
> We have a similar implemention in QUSB phy driver. can we have something like
> that for SNPSHS PHY too?
>
> Thanks,
> Pavan
Okay. Will do in the next version.

Re: [PATCH 1/3] dt-bindings: phy: qcom,usb-snps-femto-v2: Add phy override params bindings

[Krzysztof Kozlowski]

On 15/02/2022 20:19, Sandeep Maheswaram wrote:
> Add support for overriding SNPS phy tuning parameters in device tree
> bindings.
> 
> Signed-off-by: Sandeep Maheswaram <quic_c_sanm@quicinc.com>
> ---
>  .../devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml  | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml b/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml
> index 0dfe691..44cf3bf 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml
> @@ -50,6 +50,22 @@ properties:
>    vdda33-supply:
>      description: phandle to the regulator 3.3V supply node.
>  
> +  qcom,override_x0:

Do not use underscore in properties, but hyphen. Just like everywhere in
bindings.

This does not look like description of hardware but hard-coding some
register values. Bindings should rather describe the actual hardware
parameters instead of values written into registers.

Plus what other reviewers pointed about usefulness.

Best regards,
Krzysztof

[PATCH 2/3] phy: qcom-snps: Add support for overriding phy tuning parameters

[Sandeep Maheswaram]

Added support for overriding x0,x1,x2,x3 params for SNPS PHY.

Signed-off-by: Sandeep Maheswaram <quic_c_sanm@quicinc.com>
---
 drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c | 45 +++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c b/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
index 7e61202..3cf90fa 100644
--- a/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
+++ b/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
@@ -51,6 +51,12 @@
 #define USB2_SUSPEND_N				BIT(2)
 #define USB2_SUSPEND_N_SEL			BIT(3)
 
+#define USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X0		(0x6c)
+#define USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X1		(0x70)
+#define USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X2		(0x74)
+#define USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X3		(0x78)
+#define PARAM_OVRD_MASK			0xFF
+
 #define USB2_PHY_USB_PHY_CFG0			(0x94)
 #define UTMI_PHY_DATAPATH_CTRL_OVERRIDE_EN	BIT(0)
 #define UTMI_PHY_CMN_CTRL_OVERRIDE_EN		BIT(1)
@@ -90,6 +96,11 @@ struct qcom_snps_hsphy {
 
 	bool phy_initialized;
 	enum phy_mode mode;
+
+	u8 override_x0;
+	u8 override_x1;
+	u8 override_x2;
+	u8 override_x3;
 };
 
 static inline void qcom_snps_hsphy_write_mask(void __iomem *base, u32 offset,
@@ -222,6 +233,19 @@ static int qcom_snps_hsphy_init(struct phy *phy)
 	qcom_snps_hsphy_write_mask(hsphy->base, USB2_PHY_USB_PHY_HS_PHY_CTRL1,
 					VBUSVLDEXT0, VBUSVLDEXT0);
 
+	if (hsphy->override_x0)
+		qcom_snps_hsphy_write_mask(hsphy->base, USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X0,
+					PARAM_OVRD_MASK, hsphy->override_x0);
+	if (hsphy->override_x1)
+		qcom_snps_hsphy_write_mask(hsphy->base, USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X1,
+					PARAM_OVRD_MASK, hsphy->override_x1);
+	if (hsphy->override_x2)
+		qcom_snps_hsphy_write_mask(hsphy->base, USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X2,
+					PARAM_OVRD_MASK, hsphy->override_x2);
+	if (hsphy->override_x3)
+		qcom_snps_hsphy_write_mask(hsphy->base, USB2_PHY_USB_PHY_HS_PHY_OVERRIDE_X3,
+					PARAM_OVRD_MASK, hsphy->override_x3);
+
 	qcom_snps_hsphy_write_mask(hsphy->base,
 					USB2_PHY_USB_PHY_HS_PHY_CTRL_COMMON2,
 					VREGBYPASS, VREGBYPASS);
@@ -294,6 +318,7 @@ static int qcom_snps_hsphy_probe(struct platform_device *pdev)
 	struct phy *generic_phy;
 	int ret, i;
 	int num;
+	u32 value;
 
 	hsphy = devm_kzalloc(dev, sizeof(*hsphy), GFP_KERNEL);
 	if (!hsphy)
@@ -329,6 +354,26 @@ static int qcom_snps_hsphy_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	if (!of_property_read_u32(dev->of_node, "qcom,override_x0",
+				  &value)) {
+		hsphy->override_x0 = (u8)value;
+	}
+
+	if (!of_property_read_u32(dev->of_node, "qcom,override_x1",
+				  &value)) {
+		hsphy->override_x1 = (u8)value;
+	}
+
+	if (!of_property_read_u32(dev->of_node, "qcom,override_x2",
+				  &value)) {
+		hsphy->override_x2  = (u8)value;
+	}
+
+	if (!of_property_read_u32(dev->of_node, "qcom,override_x3",
+				  &value)) {
+		hsphy->override_x3 = (u8)value;
+	}
+
 	pm_runtime_set_active(dev);
 	pm_runtime_enable(dev);
 	/*
-- 
2.7.4

[PATCH 3/3] arm64: dts: qcom: sc7280: Update SNPS Phy params for SC7280 IDP device

[Sandeep Maheswaram]

Overriding the SNPS Phy tuning parameters for SC7280 IDP device.

Signed-off-by: Sandeep Maheswaram <quic_c_sanm@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
index ecbf2b8..88ca9d5 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
@@ -317,6 +317,10 @@
 	vdda-pll-supply = <&vreg_l10c_0p8>;
 	vdda33-supply = <&vreg_l2b_3p0>;
 	vdda18-supply = <&vreg_l1c_1p8>;
+	qcom,override_x0 = <0xe6>;
+	qcom,override_x1 = <0x8b>;
+	qcom,override_x2 = <0x16>;
+	qcom,override_x3 = <0x03>;
 };
 
 &usb_1_qmpphy {
-- 
2.7.4