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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <972ac516-efac-54d6-febc-1b180ec36d4b@quicinc.com>
Date:   Sat, 14 May 2022 11:54:22 +0530
From:   Krishna Kurapati PSSNV <quic_kriskura@...cinc.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        "Andy Gross" <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Stephen Boyd <swboyd@...omium.org>,
        Doug Anderson <dianders@...omium.org>,
        "Matthias Kaehlcke" <mka@...omium.org>,
        Wesley Cheng <quic_wcheng@...cinc.com>,
        "Vinod Koul" <vkoul@...nel.org>
CC:     <devicetree@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
        <linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-phy@...ts.infradead.org>, <quic_pkondeti@...cinc.com>,
        <quic_ppratap@...cinc.com>, <quic_vpulyala@...cinc.com>
Subject: Re: [v4 1/3] dt-bindings: phy: qcom,usb-snps-femto-v2: Add phy
 override params bindings


On 5/12/2022 4:00 PM, Krzysztof Kozlowski wrote:
> On 12/05/2022 07:57, Krishna Kurapati PSSNV wrote:
>> On 5/11/2022 11:49 PM, Krzysztof Kozlowski wrote:
>>> On 11/05/2022 17:26, Krishna Kurapati wrote:
>>>> From: Sandeep Maheswaram <quic_c_sanm@...cinc.com>
>>>>
>>>> Add device tree bindings for SNPS phy tuning parameters.
>>>>
>>>> Signed-off-by: Sandeep Maheswaram <quic_c_sanm@...cinc.com>
>>>> Signed-off-by: Krishna Kurapati <quic_kriskura@...cinc.com>
>>>> ---
>>>>    .../bindings/phy/qcom,usb-snps-femto-v2.yaml       | 87 ++++++++++++++++++++++
>>>>    1 file changed, 87 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..70efffe 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,93 @@ properties:
>>>>      vdda33-supply:
>>>>        description: phandle to the regulator 3.3V supply node.
>>>>    
>>>> +  qcom,hs-disconnect-bps:
>>>> +    $ref: /schemas/types.yaml#/definitions/int32
>>>> +    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%).
>>> This means there is some minimum and maximum (100%)?
>> Hi Krzystof,
>>
>> Yes there are max and min for each parameter (not necessarily 0%/100%)
>>
>> As an example if we take squelch detector threshold, the register value
>> vs actual percentage changer as per data book is as follows :
>>
>> % change in voltage    |     corresponding reg value
>>
>>    -20.90%                        |    7
>>    -15.60%                        |    6
>> -10.30%                         |    5
>> -5.30%                           |    4
>> 0%                                  |    3
>> 5.30%                            |    2
>> 10.60%                          |    1
>> 15.90%                          |    0
>>
>> Here the min and max are 15.9% to -20.9%
>>
>> The min and max differ for each parameter and might not be necessarily
>> 0% and 100%
> Then it seems possible to define minimum and maximum values - please add
> them ("minimum: xxxx").
>
>
> Best regards,
> Krzysztof

Hi Krzysztof,

  Sorry for the late reply, missed this mail.

Currently, these values have a fixed maximum and minimum. But if these 
limits change in the

future (say on a per target basis) , would it be appropriate to add them 
here in bindings file ?

Also in the driver file for sc7280 target, we have added parameter 
mapping : (map b/w register value

and bps passed from device tree). For squelch detector, it is as follows:

+static struct override_param squelch_det_threshold_sc7280[] = {
+	OVERRIDE_PARAM(-2090, 7),
+	OVERRIDE_PARAM(-1560, 6),
+	OVERRIDE_PARAM(-1030, 5),
+	OVERRIDE_PARAM(-530, 4),
+	OVERRIDE_PARAM(0, 3),
+	OVERRIDE_PARAM(530, 2),
+	OVERRIDE_PARAM(1060, 1),
+	OVERRIDE_PARAM(1590, 0),
+};

And the code is written such that if we give a bps value in dt greater than max value in
table, we would automatically choose max value. And if we provide bps value lesser than
minimum value, we would choose the min value.

So, would it be appropriate to add the min and max in dt-bindings when there is a
slight chance of them changing in the future ?

Would like to know your thoughts on this,

Thanks,
Krishna,