diff mbox series

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

Message ID 1654066564-20518-2-git-send-email-quic_kriskura@quicinc.com
State New
Headers show
Series Add QCOM SNPS PHY overriding params support | expand

Commit Message

Krishna Kurapati June 1, 2022, 6:56 a.m. UTC
From: Sandeep Maheswaram <quic_c_sanm@quicinc.com>

Add device tree bindings for SNPS phy tuning parameters.

Signed-off-by: Sandeep Maheswaram <quic_c_sanm@quicinc.com>
Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../bindings/phy/qcom,usb-snps-femto-v2.yaml       | 96 ++++++++++++++++++++++
 1 file changed, 96 insertions(+)

Comments

Krishna Kurapati June 1, 2022, 5:36 p.m. UTC | #1
Thanks Rob !


On 6/1/2022 11:03 PM, Rob Herring wrote:
> On Wed, Jun 01, 2022 at 07:28:53AM -0500, Rob Herring wrote:
>> On Wed, 01 Jun 2022 12:26:02 +0530, Krishna Kurapati wrote:
>>> From: Sandeep Maheswaram <quic_c_sanm@quicinc.com>
>>>
>>> Add device tree bindings for SNPS phy tuning parameters.
>>>
>>> Signed-off-by: Sandeep Maheswaram <quic_c_sanm@quicinc.com>
>>> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>> ---
>>>   .../bindings/phy/qcom,usb-snps-femto-v2.yaml       | 96 ++++++++++++++++++++++
>>>   1 file changed, 96 insertions(+)
>>>
>> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
>> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>>
>> yamllint warnings/errors:
>>
>> dtschema/dtc warnings/errors:
>> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml: properties:qcom,hs-rise-fall-time-bp: 'oneOf' conditional failed, one must be fixed:
>> 	'type' is a required property
>> 		hint: A vendor boolean property can use "type: boolean"
>> 	Additional properties are not allowed ('maximum', 'minimum' were unexpected)
>> 		hint: A vendor boolean property can use "type: boolean"
>> 	/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml: properties:qcom,hs-rise-fall-time-bp: 'oneOf' conditional failed, one must be fixed:
>> 		'enum' is a required property
>> 		'const' is a required property
>> 		hint: A vendor string property with exact values has an implicit type
>> 		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
>> 	/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml: properties:qcom,hs-rise-fall-time-bp: 'oneOf' conditional failed, one must be fixed:
>> 		'$ref' is a required property
>> 		'allOf' is a required property
>> 		hint: A vendor property needs a $ref to types.yaml
>> 		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
>> 	hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
>> 	from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
>> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml: properties:qcom,ls-fs-output-impedance-bp: 'oneOf' conditional failed, one must be fixed:
>> 	'type' is a required property
>> 		hint: A vendor boolean property can use "type: boolean"
>> 	Additional properties are not allowed ('maximum', 'minimum' were unexpected)
>> 		hint: A vendor boolean property can use "type: boolean"
>> 	/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml: properties:qcom,ls-fs-output-impedance-bp: 'oneOf' conditional failed, one must be fixed:
>> 		'enum' is a required property
>> 		'const' is a required property
>> 		hint: A vendor string property with exact values has an implicit type
>> 		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
>> 	/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml: properties:qcom,ls-fs-output-impedance-bp: 'oneOf' conditional failed, one must be fixed:
>> 		'$ref' is a required property
>> 		'allOf' is a required property
>> 		hint: A vendor property needs a $ref to types.yaml
>> 		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
>> 	hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
>> 	from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
>> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml: properties:qcom,pre-emphasis-amplitude-bp: 'oneOf' conditional failed, one must be fixed:
>> 	'type' is a required property
>> 		hint: A vendor boolean property can use "type: boolean"
>> 	Additional properties are not allowed ('maximum', 'minimum' were unexpected)
>> 		hint: A vendor boolean property can use "type: boolean"
>> 	/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml: properties:qcom,pre-emphasis-amplitude-bp: 'oneOf' conditional failed, one must be fixed:
>> 		'enum' is a required property
>> 		'const' is a required property
>> 		hint: A vendor string property with exact values has an implicit type
>> 		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
>> 	/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml: properties:qcom,pre-emphasis-amplitude-bp: 'oneOf' conditional failed, one must be fixed:
>> 		'$ref' is a required property
>> 		'allOf' is a required property
>> 		hint: A vendor property needs a $ref to types.yaml
>> 		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
>> 	hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
>> 	from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
>> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml: properties:qcom,squelch-detector-bp: 'oneOf' conditional failed, one must be fixed:
>> 	'type' is a required property
>> 		hint: A vendor boolean property can use "type: boolean"
>> 	Additional properties are not allowed ('maximum', 'minimum' were unexpected)
>> 		hint: A vendor boolean property can use "type: boolean"
>> 	/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml: properties:qcom,squelch-detector-bp: 'oneOf' conditional failed, one must be fixed:
>> 		'enum' is a required property
>> 		'const' is a required property
>> 		hint: A vendor string property with exact values has an implicit type
>> 		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
>> 	/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml: properties:qcom,squelch-detector-bp: 'oneOf' conditional failed, one must be fixed:
>> 		'$ref' is a required property
>> 		'allOf' is a required property
>> 		hint: A vendor property needs a $ref to types.yaml
>> 		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
>> 	hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
>> 	from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
>> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml: properties:qcom,hs-disconnect-bp: 'oneOf' conditional failed, one must be fixed:
>> 	'type' is a required property
>> 		hint: A vendor boolean property can use "type: boolean"
>> 	Additional properties are not allowed ('maximum', 'minimum' were unexpected)
>> 		hint: A vendor boolean property can use "type: boolean"
>> 	/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml: properties:qcom,hs-disconnect-bp: 'oneOf' conditional failed, one must be fixed:
>> 		'enum' is a required property
>> 		'const' is a required property
>> 		hint: A vendor string property with exact values has an implicit type
>> 		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
>> 	/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml: properties:qcom,hs-disconnect-bp: 'oneOf' conditional failed, one must be fixed:
>> 		'$ref' is a required property
>> 		'allOf' is a required property
>> 		hint: A vendor property needs a $ref to types.yaml
>> 		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
>> 	hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
>> 	from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
>> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml: properties:qcom,hs-amplitude-bp: 'oneOf' conditional failed, one must be fixed:
>> 	'type' is a required property
>> 		hint: A vendor boolean property can use "type: boolean"
>> 	Additional properties are not allowed ('maximum', 'minimum' were unexpected)
>> 		hint: A vendor boolean property can use "type: boolean"
>> 	/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml: properties:qcom,hs-amplitude-bp: 'oneOf' conditional failed, one must be fixed:
>> 		'enum' is a required property
>> 		'const' is a required property
>> 		hint: A vendor string property with exact values has an implicit type
>> 		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
>> 	/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml: properties:qcom,hs-amplitude-bp: 'oneOf' conditional failed, one must be fixed:
>> 		'$ref' is a required property
>> 		'allOf' is a required property
>> 		hint: A vendor property needs a $ref to types.yaml
>> 		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
>> 	hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
>> 	from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
>> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml: properties:qcom,pre-emphasis-duration-bp: 'oneOf' conditional failed, one must be fixed:
>> 	'type' is a required property
>> 		hint: A vendor boolean property can use "type: boolean"
>> 	Additional properties are not allowed ('maximum', 'minimum' were unexpected)
>> 		hint: A vendor boolean property can use "type: boolean"
>> 	/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml: properties:qcom,pre-emphasis-duration-bp: 'oneOf' conditional failed, one must be fixed:
>> 		'enum' is a required property
>> 		'const' is a required property
>> 		hint: A vendor string property with exact values has an implicit type
>> 		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
>> 	/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml: properties:qcom,pre-emphasis-duration-bp: 'oneOf' conditional failed, one must be fixed:
>> 		'$ref' is a required property
>> 		'allOf' is a required property
>> 		hint: A vendor property needs a $ref to types.yaml
>> 		from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
>> 	hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
>> 	from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
>> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml: ignoring, error in schema: properties: qcom,hs-rise-fall-time-bp
>> Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.example.dtb:0:0: /example-0/phy@88e2000: failed to match any schema with compatible: ['qcom,sm8150-usb-hs-phy']
> Ignore these. A tool issue which is now fixed.
>
> Rob
Krzysztof Kozlowski June 8, 2022, 9:36 a.m. UTC | #2
On 01/06/2022 08:56, Krishna Kurapati wrote:
> From: Sandeep Maheswaram <quic_c_sanm@quicinc.com>
> 
> Add device tree bindings for SNPS phy tuning parameters.
> 
> Signed-off-by: Sandeep Maheswaram <quic_c_sanm@quicinc.com>
> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../bindings/phy/qcom,usb-snps-femto-v2.yaml       | 96 ++++++++++++++++++++++
>  1 file changed, 96 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 1ce251d..daeeb04 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml
> @@ -53,6 +53,102 @@ properties:
>    vdda33-supply:
>      description: phandle to the regulator 3.3V supply node.
>  
> +  qcom,hs-disconnect-bp:
> +    description:
> +      This adjusts the voltage level for the threshold used to
> +      detect a disconnect event at the host. Possible values are.
> +      The values defined are in multiples of basis points (1bp = 0.01%).
> +      The hardware accepts only discrete values. The value closest to the
> +      provided input will be chosen as the override value for this param.
> +    minimum: -272
> +    maximum: 2156
> +
> +  qcom,squelch-detector-bp:
> +    description:
> +      This adjusts the voltage level for the threshold used to
> +      detect valid high-speed data.
> +      The values defined are in multiples of basis points (1bp = 0.01%).
> +      The hardware accepts only discrete values. The value closest to the
> +      provided input will be chosen as the override value for this param.
> +    minimum: -2090
> +    maximum: 1590
> +
> +  qcom,hs-amplitude-bp:
> +    description:
> +      This adjusts the high-speed DC level voltage.
> +      The values defined are in multiples of basis points (1bp = 0.01%).
> +      The hardware accepts only discrete values. The value closest to the
> +      provided input will be chosen as the override value for this param.
> +    minimum: -660
> +    maximum: 2670
> +
> +  qcom,pre-emphasis-duration-bp:
> +    description:
> +      This signal controls the duration for which the
> +      HS pre-emphasis current is sourced onto DP<#> or DM<#>.
> +      The HS Transmitter pre-emphasis duration is defined in terms of
> +      unit amounts. One unit of pre-emphasis duration is approximately
> +      650 ps and is defined as 1X pre-emphasis duration.
> +      The values defined are in multiples of basis points (1bp = 0.01%).
> +      The hardware accepts only discrete values. The value closest to the
> +      provided input will be chosen as the override value for this param.
> +    minimum: 10000
> +    maximum: 20000
> +
> +  qcom,pre-emphasis-amplitude-bp:
> +    description:
> +      This signal controls the amount of current sourced to
> +      DP<#> and DM<#> after a J-to-K or K-to-J transition.
> +      The HS Transmitter pre-emphasis current is defined in terms of unit
> +      amounts. One unit amount is approximately 2 mA and is defined as
> +      1X pre-emphasis current.
> +      The values defined are in multiples of basis points (1bp = 0.01%).
> +      The hardware accepts only discrete values. The value closest to the
> +      provided input will be chosen as the override value for this param.
> +    minimum: 10000
> +    maximum: 40000
> +
> +  qcom,hs-rise-fall-time-bp:
> +    description:
> +      This adjusts the rise/fall times of the high-speed waveform.
> +      The values defined are in multiples of basis points (1bp = 0.01%).
> +      The hardware accepts only discrete values. The value closest to the
> +      provided input will be chosen as the override value for this param.
> +    minimum: -4100
> +    maximum: 5430
> +
> +  qcom,hs-crossover-voltage-microvolt:
> +    description:
> +      This adjusts the voltage at which the DP<#> and DM<#>
> +      signals cross while transmitting in HS mode.
> +      The values defined are in milli volts.

It's not accurate anymore - it's microvolt. I propose to skip this one
sentence, because unit is obvious from the type.

> +    maximum: 28000
> +
> +  qcom,hs-output-impedance-micro-ohms:
> +    description:
> +      In some applications, there can be significant series resistance
> +      on the D+ and D- paths between the transceiver and cable. This adjusts
> +      the driver source impedance to compensate for added series
> +      resistance on the USB. The values defined are in milli ohms.

The same. Other places might need similar change.

Best regards,
Krzysztof
Krishna Kurapati June 8, 2022, 11:03 a.m. UTC | #3
On 6/8/2022 3:06 PM, Krzysztof Kozlowski wrote:
> On 01/06/2022 08:56, Krishna Kurapati wrote:
>> From: Sandeep Maheswaram <quic_c_sanm@quicinc.com>
>>
>> Add device tree bindings for SNPS phy tuning parameters.
>>
>> Signed-off-by: Sandeep Maheswaram <quic_c_sanm@quicinc.com>
>> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>>   .../bindings/phy/qcom,usb-snps-femto-v2.yaml       | 96 ++++++++++++++++++++++
>>   1 file changed, 96 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 1ce251d..daeeb04 100644
>> --- a/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml
>> +++ b/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml
>> @@ -53,6 +53,102 @@ properties:
>>     vdda33-supply:
>>       description: phandle to the regulator 3.3V supply node.
>>   
>> +  qcom,hs-disconnect-bp:
>> +    description:
>> +      This adjusts the voltage level for the threshold used to
>> +      detect a disconnect event at the host. Possible values are.
>> +      The values defined are in multiples of basis points (1bp = 0.01%).
>> +      The hardware accepts only discrete values. The value closest to the
>> +      provided input will be chosen as the override value for this param.
>> +    minimum: -272
>> +    maximum: 2156
>> +
>> +  qcom,squelch-detector-bp:
>> +    description:
>> +      This adjusts the voltage level for the threshold used to
>> +      detect valid high-speed data.
>> +      The values defined are in multiples of basis points (1bp = 0.01%).
>> +      The hardware accepts only discrete values. The value closest to the
>> +      provided input will be chosen as the override value for this param.
>> +    minimum: -2090
>> +    maximum: 1590
>> +
>> +  qcom,hs-amplitude-bp:
>> +    description:
>> +      This adjusts the high-speed DC level voltage.
>> +      The values defined are in multiples of basis points (1bp = 0.01%).
>> +      The hardware accepts only discrete values. The value closest to the
>> +      provided input will be chosen as the override value for this param.
>> +    minimum: -660
>> +    maximum: 2670
>> +
>> +  qcom,pre-emphasis-duration-bp:
>> +    description:
>> +      This signal controls the duration for which the
>> +      HS pre-emphasis current is sourced onto DP<#> or DM<#>.
>> +      The HS Transmitter pre-emphasis duration is defined in terms of
>> +      unit amounts. One unit of pre-emphasis duration is approximately
>> +      650 ps and is defined as 1X pre-emphasis duration.
>> +      The values defined are in multiples of basis points (1bp = 0.01%).
>> +      The hardware accepts only discrete values. The value closest to the
>> +      provided input will be chosen as the override value for this param.
>> +    minimum: 10000
>> +    maximum: 20000
>> +
>> +  qcom,pre-emphasis-amplitude-bp:
>> +    description:
>> +      This signal controls the amount of current sourced to
>> +      DP<#> and DM<#> after a J-to-K or K-to-J transition.
>> +      The HS Transmitter pre-emphasis current is defined in terms of unit
>> +      amounts. One unit amount is approximately 2 mA and is defined as
>> +      1X pre-emphasis current.
>> +      The values defined are in multiples of basis points (1bp = 0.01%).
>> +      The hardware accepts only discrete values. The value closest to the
>> +      provided input will be chosen as the override value for this param.
>> +    minimum: 10000
>> +    maximum: 40000
>> +
>> +  qcom,hs-rise-fall-time-bp:
>> +    description:
>> +      This adjusts the rise/fall times of the high-speed waveform.
>> +      The values defined are in multiples of basis points (1bp = 0.01%).
>> +      The hardware accepts only discrete values. The value closest to the
>> +      provided input will be chosen as the override value for this param.
>> +    minimum: -4100
>> +    maximum: 5430
>> +
>> +  qcom,hs-crossover-voltage-microvolt:
>> +    description:
>> +      This adjusts the voltage at which the DP<#> and DM<#>
>> +      signals cross while transmitting in HS mode.
>> +      The values defined are in milli volts.
> It's not accurate anymore - it's microvolt. I propose to skip this one
> sentence, because unit is obvious from the type.
>
>> +    maximum: 28000
>> +
>> +  qcom,hs-output-impedance-micro-ohms:
>> +    description:
>> +      In some applications, there can be significant series resistance
>> +      on the D+ and D- paths between the transceiver and cable. This adjusts
>> +      the driver source impedance to compensate for added series
>> +      resistance on the USB. The values defined are in milli ohms.
> The same. Other places might need similar change.
>
> Best regards,
> Krzysztof

Hi Krzysztof,

     Sure, will modify the description for all the params.

Regards,

Krishna,
Krishna Kurapati June 8, 2022, 12:07 p.m. UTC | #4
On 6/8/2022 3:06 PM, Krzysztof Kozlowski wrote:
> On 01/06/2022 08:56, Krishna Kurapati wrote:
>> From: Sandeep Maheswaram <quic_c_sanm@quicinc.com>
>>
>> Add device tree bindings for SNPS phy tuning parameters.
>>
>> Signed-off-by: Sandeep Maheswaram <quic_c_sanm@quicinc.com>
>> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>>   .../bindings/phy/qcom,usb-snps-femto-v2.yaml       | 96 ++++++++++++++++++++++
>>   1 file changed, 96 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 1ce251d..daeeb04 100644
>> --- a/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml
>> +++ b/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml
>> @@ -53,6 +53,102 @@ properties:
>>     vdda33-supply:
>>       description: phandle to the regulator 3.3V supply node.
>>   
>> +  qcom,hs-disconnect-bp:
>> +    description:
>> +      This adjusts the voltage level for the threshold used to
>> +      detect a disconnect event at the host. Possible values are.
>> +      The values defined are in multiples of basis points (1bp = 0.01%).
>> +      The hardware accepts only discrete values. The value closest to the
>> +      provided input will be chosen as the override value for this param.
>> +    minimum: -272
>> +    maximum: 2156
>> +
>> +  qcom,squelch-detector-bp:
>> +    description:
>> +      This adjusts the voltage level for the threshold used to
>> +      detect valid high-speed data.
>> +      The values defined are in multiples of basis points (1bp = 0.01%).
>> +      The hardware accepts only discrete values. The value closest to the
>> +      provided input will be chosen as the override value for this param.
>> +    minimum: -2090
>> +    maximum: 1590
>> +
>> +  qcom,hs-amplitude-bp:
>> +    description:
>> +      This adjusts the high-speed DC level voltage.
>> +      The values defined are in multiples of basis points (1bp = 0.01%).
>> +      The hardware accepts only discrete values. The value closest to the
>> +      provided input will be chosen as the override value for this param.
>> +    minimum: -660
>> +    maximum: 2670
>> +
>> +  qcom,pre-emphasis-duration-bp:
>> +    description:
>> +      This signal controls the duration for which the
>> +      HS pre-emphasis current is sourced onto DP<#> or DM<#>.
>> +      The HS Transmitter pre-emphasis duration is defined in terms of
>> +      unit amounts. One unit of pre-emphasis duration is approximately
>> +      650 ps and is defined as 1X pre-emphasis duration.
>> +      The values defined are in multiples of basis points (1bp = 0.01%).
>> +      The hardware accepts only discrete values. The value closest to the
>> +      provided input will be chosen as the override value for this param.
>> +    minimum: 10000
>> +    maximum: 20000
>> +
>> +  qcom,pre-emphasis-amplitude-bp:
>> +    description:
>> +      This signal controls the amount of current sourced to
>> +      DP<#> and DM<#> after a J-to-K or K-to-J transition.
>> +      The HS Transmitter pre-emphasis current is defined in terms of unit
>> +      amounts. One unit amount is approximately 2 mA and is defined as
>> +      1X pre-emphasis current.
>> +      The values defined are in multiples of basis points (1bp = 0.01%).
>> +      The hardware accepts only discrete values. The value closest to the
>> +      provided input will be chosen as the override value for this param.
>> +    minimum: 10000
>> +    maximum: 40000
>> +
>> +  qcom,hs-rise-fall-time-bp:
>> +    description:
>> +      This adjusts the rise/fall times of the high-speed waveform.
>> +      The values defined are in multiples of basis points (1bp = 0.01%).
>> +      The hardware accepts only discrete values. The value closest to the
>> +      provided input will be chosen as the override value for this param.
>> +    minimum: -4100
>> +    maximum: 5430
>> +
>> +  qcom,hs-crossover-voltage-microvolt:
>> +    description:
>> +      This adjusts the voltage at which the DP<#> and DM<#>
>> +      signals cross while transmitting in HS mode.
>> +      The values defined are in milli volts.
> It's not accurate anymore - it's microvolt. I propose to skip this one
> sentence, because unit is obvious from the type.

Hi Krzysztof,

Sure, will omit this line in the next series.

>> +    maximum: 28000
>> +
>> +  qcom,hs-output-impedance-micro-ohms:
>> +    description:
>> +      In some applications, there can be significant series resistance
>> +      on the D+ and D- paths between the transceiver and cable. This adjusts
>> +      the driver source impedance to compensate for added series
>> +      resistance on the USB. The values defined are in milli ohms.
> The same. Other places might need similar change.
>
> Best regards,
> Krzysztof

Sure, Will make sure to remove the basis points sentence from other 
params as well as bp has been added to dtschema and is self-explanatory.


Thanks,

Krishna,
Vinod Koul June 8, 2022, 3:53 p.m. UTC | #5
On 01-06-22, 12:26, Krishna Kurapati wrote:
> From: Sandeep Maheswaram <quic_c_sanm@quicinc.com>
> 
> Add device tree bindings for SNPS phy tuning parameters.
> 
> Signed-off-by: Sandeep Maheswaram <quic_c_sanm@quicinc.com>
> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../bindings/phy/qcom,usb-snps-femto-v2.yaml       | 96 ++++++++++++++++++++++
>  1 file changed, 96 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 1ce251d..daeeb04 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml
> @@ -53,6 +53,102 @@ properties:
>    vdda33-supply:
>      description: phandle to the regulator 3.3V supply node.
>  
> +  qcom,hs-disconnect-bp:
> +    description:
> +      This adjusts the voltage level for the threshold used to
> +      detect a disconnect event at the host. Possible values are.
> +      The values defined are in multiples of basis points (1bp = 0.01%).
> +      The hardware accepts only discrete values. The value closest to the
> +      provided input will be chosen as the override value for this param.
> +    minimum: -272
> +    maximum: 2156
> +
> +  qcom,squelch-detector-bp:
> +    description:
> +      This adjusts the voltage level for the threshold used to
> +      detect valid high-speed data.
> +      The values defined are in multiples of basis points (1bp = 0.01%).
> +      The hardware accepts only discrete values. The value closest to the
> +      provided input will be chosen as the override value for this param.
> +    minimum: -2090
> +    maximum: 1590
> +
> +  qcom,hs-amplitude-bp:
> +    description:
> +      This adjusts the high-speed DC level voltage.
> +      The values defined are in multiples of basis points (1bp = 0.01%).
> +      The hardware accepts only discrete values. The value closest to the
> +      provided input will be chosen as the override value for this param.
> +    minimum: -660
> +    maximum: 2670
> +
> +  qcom,pre-emphasis-duration-bp:
> +    description:
> +      This signal controls the duration for which the
> +      HS pre-emphasis current is sourced onto DP<#> or DM<#>.
> +      The HS Transmitter pre-emphasis duration is defined in terms of
> +      unit amounts. One unit of pre-emphasis duration is approximately
> +      650 ps and is defined as 1X pre-emphasis duration.
> +      The values defined are in multiples of basis points (1bp = 0.01%).
> +      The hardware accepts only discrete values. The value closest to the
> +      provided input will be chosen as the override value for this param.
> +    minimum: 10000
> +    maximum: 20000
> +
> +  qcom,pre-emphasis-amplitude-bp:
> +    description:
> +      This signal controls the amount of current sourced to
> +      DP<#> and DM<#> after a J-to-K or K-to-J transition.
> +      The HS Transmitter pre-emphasis current is defined in terms of unit
> +      amounts. One unit amount is approximately 2 mA and is defined as
> +      1X pre-emphasis current.
> +      The values defined are in multiples of basis points (1bp = 0.01%).
> +      The hardware accepts only discrete values. The value closest to the
> +      provided input will be chosen as the override value for this param.
> +    minimum: 10000
> +    maximum: 40000
> +
> +  qcom,hs-rise-fall-time-bp:
> +    description:
> +      This adjusts the rise/fall times of the high-speed waveform.
> +      The values defined are in multiples of basis points (1bp = 0.01%).
> +      The hardware accepts only discrete values. The value closest to the
> +      provided input will be chosen as the override value for this param.
> +    minimum: -4100
> +    maximum: 5430
> +
> +  qcom,hs-crossover-voltage-microvolt:
> +    description:
> +      This adjusts the voltage at which the DP<#> and DM<#>
> +      signals cross while transmitting in HS mode.
> +      The values defined are in milli volts. The hardware accepts only
> +      discrete values. The value closest to the provided input will be
> +      chosen as the override value for this param.
> +    minimum: -31000
> +    maximum: 28000
> +
> +  qcom,hs-output-impedance-micro-ohms:
> +    description:
> +      In some applications, there can be significant series resistance
> +      on the D+ and D- paths between the transceiver and cable. This adjusts
> +      the driver source impedance to compensate for added series
> +      resistance on the USB. The values defined are in milli ohms.
> +      The hardware accepts only discrete values. The value closest to the
> +      provided input will be chosen as the override value for this param.
> +    minimum: -2300000
> +    maximum: 6100000
> +
> +  qcom,ls-fs-output-impedance-bp:
> +    description:
> +      This adjusts the low- and full-speed single-ended source
> +      impedance while driving high. The following adjustment values are based
> +      on nominal process, voltage, and temperature.
> +      The values defined are in multiples of basis points (1bp = 0.01%).
> +      The hardware accepts only discrete values. The value closest to the
> +      provided input will be chosen as the override value for this param.
> +    minimum: -1053
> +    maximum: 1310

do we need all these values in DT, till now we have these in driver..
what is the reasoning to add these in DT instead?
Krishna Kurapati June 8, 2022, 4:14 p.m. UTC | #6
On 6/8/2022 9:23 PM, Vinod Koul wrote:
> On 01-06-22, 12:26, Krishna Kurapati wrote:
>> From: Sandeep Maheswaram <quic_c_sanm@quicinc.com>
>>
>> Add device tree bindings for SNPS phy tuning parameters.
>>
>> Signed-off-by: Sandeep Maheswaram <quic_c_sanm@quicinc.com>
>> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>>   .../bindings/phy/qcom,usb-snps-femto-v2.yaml       | 96 ++++++++++++++++++++++
>>   1 file changed, 96 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 1ce251d..daeeb04 100644
>> --- a/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml
>> +++ b/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml
>> @@ -53,6 +53,102 @@ properties:
>>     vdda33-supply:
>>       description: phandle to the regulator 3.3V supply node.
>>   
>> +  qcom,hs-disconnect-bp:
>> +    description:
>> +      This adjusts the voltage level for the threshold used to
>> +      detect a disconnect event at the host. Possible values are.
>> +      The values defined are in multiples of basis points (1bp = 0.01%).
>> +      The hardware accepts only discrete values. The value closest to the
>> +      provided input will be chosen as the override value for this param.
>> +    minimum: -272
>> +    maximum: 2156
>> +
>> +  qcom,squelch-detector-bp:
>> +    description:
>> +      This adjusts the voltage level for the threshold used to
>> +      detect valid high-speed data.
>> +      The values defined are in multiples of basis points (1bp = 0.01%).
>> +      The hardware accepts only discrete values. The value closest to the
>> +      provided input will be chosen as the override value for this param.
>> +    minimum: -2090
>> +    maximum: 1590
>> +
>> +  qcom,hs-amplitude-bp:
>> +    description:
>> +      This adjusts the high-speed DC level voltage.
>> +      The values defined are in multiples of basis points (1bp = 0.01%).
>> +      The hardware accepts only discrete values. The value closest to the
>> +      provided input will be chosen as the override value for this param.
>> +    minimum: -660
>> +    maximum: 2670
>> +
>> +  qcom,pre-emphasis-duration-bp:
>> +    description:
>> +      This signal controls the duration for which the
>> +      HS pre-emphasis current is sourced onto DP<#> or DM<#>.
>> +      The HS Transmitter pre-emphasis duration is defined in terms of
>> +      unit amounts. One unit of pre-emphasis duration is approximately
>> +      650 ps and is defined as 1X pre-emphasis duration.
>> +      The values defined are in multiples of basis points (1bp = 0.01%).
>> +      The hardware accepts only discrete values. The value closest to the
>> +      provided input will be chosen as the override value for this param.
>> +    minimum: 10000
>> +    maximum: 20000
>> +
>> +  qcom,pre-emphasis-amplitude-bp:
>> +    description:
>> +      This signal controls the amount of current sourced to
>> +      DP<#> and DM<#> after a J-to-K or K-to-J transition.
>> +      The HS Transmitter pre-emphasis current is defined in terms of unit
>> +      amounts. One unit amount is approximately 2 mA and is defined as
>> +      1X pre-emphasis current.
>> +      The values defined are in multiples of basis points (1bp = 0.01%).
>> +      The hardware accepts only discrete values. The value closest to the
>> +      provided input will be chosen as the override value for this param.
>> +    minimum: 10000
>> +    maximum: 40000
>> +
>> +  qcom,hs-rise-fall-time-bp:
>> +    description:
>> +      This adjusts the rise/fall times of the high-speed waveform.
>> +      The values defined are in multiples of basis points (1bp = 0.01%).
>> +      The hardware accepts only discrete values. The value closest to the
>> +      provided input will be chosen as the override value for this param.
>> +    minimum: -4100
>> +    maximum: 5430
>> +
>> +  qcom,hs-crossover-voltage-microvolt:
>> +    description:
>> +      This adjusts the voltage at which the DP<#> and DM<#>
>> +      signals cross while transmitting in HS mode.
>> +      The values defined are in milli volts. The hardware accepts only
>> +      discrete values. The value closest to the provided input will be
>> +      chosen as the override value for this param.
>> +    minimum: -31000
>> +    maximum: 28000
>> +
>> +  qcom,hs-output-impedance-micro-ohms:
>> +    description:
>> +      In some applications, there can be significant series resistance
>> +      on the D+ and D- paths between the transceiver and cable. This adjusts
>> +      the driver source impedance to compensate for added series
>> +      resistance on the USB. The values defined are in milli ohms.
>> +      The hardware accepts only discrete values. The value closest to the
>> +      provided input will be chosen as the override value for this param.
>> +    minimum: -2300000
>> +    maximum: 6100000
>> +
>> +  qcom,ls-fs-output-impedance-bp:
>> +    description:
>> +      This adjusts the low- and full-speed single-ended source
>> +      impedance while driving high. The following adjustment values are based
>> +      on nominal process, voltage, and temperature.
>> +      The values defined are in multiples of basis points (1bp = 0.01%).
>> +      The hardware accepts only discrete values. The value closest to the
>> +      provided input will be chosen as the override value for this param.
>> +    minimum: -1053
>> +    maximum: 1310
> do we need all these values in DT, till now we have these in driver..
> what is the reasoning to add these in DT instead?

Hi Vinod,

   The patch series started out as you mentioned. We used to have phy 
tune register values passed from dT to driver and written to respective 
registers in the driver code.

But it was later suggested not to pass register values from dT. Instead 
define the meaningful properties that make up the phy tuning parameters 
and pass their values from dT. The driver code is supposed to convert 
these values and map them to required register values. This is why we 
had to come up with these parameters and declare them in bindings and dT.

More info regarding the discussion at : 
https://lore.kernel.org/linux-usb/b45b3b7e-e1c0-79b6-81c0-53c70427dd10@canonical.com/


Regards,

Krishna,
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 1ce251d..daeeb04 100644
--- a/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,usb-snps-femto-v2.yaml
@@ -53,6 +53,102 @@  properties:
   vdda33-supply:
     description: phandle to the regulator 3.3V supply node.
 
+  qcom,hs-disconnect-bp:
+    description:
+      This adjusts the voltage level for the threshold used to
+      detect a disconnect event at the host. Possible values are.
+      The values defined are in multiples of basis points (1bp = 0.01%).
+      The hardware accepts only discrete values. The value closest to the
+      provided input will be chosen as the override value for this param.
+    minimum: -272
+    maximum: 2156
+
+  qcom,squelch-detector-bp:
+    description:
+      This adjusts the voltage level for the threshold used to
+      detect valid high-speed data.
+      The values defined are in multiples of basis points (1bp = 0.01%).
+      The hardware accepts only discrete values. The value closest to the
+      provided input will be chosen as the override value for this param.
+    minimum: -2090
+    maximum: 1590
+
+  qcom,hs-amplitude-bp:
+    description:
+      This adjusts the high-speed DC level voltage.
+      The values defined are in multiples of basis points (1bp = 0.01%).
+      The hardware accepts only discrete values. The value closest to the
+      provided input will be chosen as the override value for this param.
+    minimum: -660
+    maximum: 2670
+
+  qcom,pre-emphasis-duration-bp:
+    description:
+      This signal controls the duration for which the
+      HS pre-emphasis current is sourced onto DP<#> or DM<#>.
+      The HS Transmitter pre-emphasis duration is defined in terms of
+      unit amounts. One unit of pre-emphasis duration is approximately
+      650 ps and is defined as 1X pre-emphasis duration.
+      The values defined are in multiples of basis points (1bp = 0.01%).
+      The hardware accepts only discrete values. The value closest to the
+      provided input will be chosen as the override value for this param.
+    minimum: 10000
+    maximum: 20000
+
+  qcom,pre-emphasis-amplitude-bp:
+    description:
+      This signal controls the amount of current sourced to
+      DP<#> and DM<#> after a J-to-K or K-to-J transition.
+      The HS Transmitter pre-emphasis current is defined in terms of unit
+      amounts. One unit amount is approximately 2 mA and is defined as
+      1X pre-emphasis current.
+      The values defined are in multiples of basis points (1bp = 0.01%).
+      The hardware accepts only discrete values. The value closest to the
+      provided input will be chosen as the override value for this param.
+    minimum: 10000
+    maximum: 40000
+
+  qcom,hs-rise-fall-time-bp:
+    description:
+      This adjusts the rise/fall times of the high-speed waveform.
+      The values defined are in multiples of basis points (1bp = 0.01%).
+      The hardware accepts only discrete values. The value closest to the
+      provided input will be chosen as the override value for this param.
+    minimum: -4100
+    maximum: 5430
+
+  qcom,hs-crossover-voltage-microvolt:
+    description:
+      This adjusts the voltage at which the DP<#> and DM<#>
+      signals cross while transmitting in HS mode.
+      The values defined are in milli volts. The hardware accepts only
+      discrete values. The value closest to the provided input will be
+      chosen as the override value for this param.
+    minimum: -31000
+    maximum: 28000
+
+  qcom,hs-output-impedance-micro-ohms:
+    description:
+      In some applications, there can be significant series resistance
+      on the D+ and D- paths between the transceiver and cable. This adjusts
+      the driver source impedance to compensate for added series
+      resistance on the USB. The values defined are in milli ohms.
+      The hardware accepts only discrete values. The value closest to the
+      provided input will be chosen as the override value for this param.
+    minimum: -2300000
+    maximum: 6100000
+
+  qcom,ls-fs-output-impedance-bp:
+    description:
+      This adjusts the low- and full-speed single-ended source
+      impedance while driving high. The following adjustment values are based
+      on nominal process, voltage, and temperature.
+      The values defined are in multiples of basis points (1bp = 0.01%).
+      The hardware accepts only discrete values. The value closest to the
+      provided input will be chosen as the override value for this param.
+    minimum: -1053
+    maximum: 1310
+
 required:
   - compatible
   - reg