diff mbox series

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

Message ID 1644952755-15527-2-git-send-email-quic_c_sanm@quicinc.com (mailing list archive)
State Superseded
Headers show
Series Add QCOM SNPS PHY overriding params support | expand

Commit Message

Sandeep Maheswaram Feb. 15, 2022, 7:19 p.m. UTC
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(+)

Comments

Dmitry Baryshkov Feb. 15, 2022, 7:55 p.m. UTC | #1
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
Stephen Boyd Feb. 16, 2022, 2:10 a.m. UTC | #2
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
Pavan Kondeti Feb. 16, 2022, 3:47 a.m. UTC | #3
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
Krzysztof Kozlowski Feb. 16, 2022, 7:40 a.m. UTC | #4
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
Sandeep Maheswaram Feb. 21, 2022, 5:49 a.m. UTC | #5
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.
diff mbox series

Patch

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