Re: [PATCH 1/4] dt-bindings: phy: qcom,snps-eusb2-repeater: Document qcom,tune-res-fsdif

[Krzysztof Kozlowski <krzk@kernel.org>]

On 09/07/2025 11:40, Luca Weiss wrote:
> Hi Krzysztof,
> 
> On Tue Jul 8, 2025 at 10:31 AM CEST, Luca Weiss wrote:
>> On Tue Jul 8, 2025 at 10:21 AM CEST, Krzysztof Kozlowski wrote:
>>> On Tue, Jul 08, 2025 at 10:13:24AM +0200, Krzysztof Kozlowski wrote:
>>>> On Wed, Jun 25, 2025 at 11:14:56AM +0200, Luca Weiss wrote:
>>>>> Document the FS Differential TX Output Resistance Tuning value found on
>>>>> the eUSB2 repeater on Qualcomm PMICs.
>>>>>
>>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
>>>>> ---
>>>>>  Documentation/devicetree/bindings/phy/qcom,snps-eusb2-repeater.yaml | 6 ++++++
>>>>>  1 file changed, 6 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/phy/qcom,snps-eusb2-repeater.yaml b/Documentation/devicetree/bindings/phy/qcom,snps-eusb2-repeater.yaml
>>>>> index 27f064a71c9fb8cb60e8333fb285f0510a4af94f..6bfd11657e2992735998063b3ca390e04a03930d 100644
>>>>> --- a/Documentation/devicetree/bindings/phy/qcom,snps-eusb2-repeater.yaml
>>>>> +++ b/Documentation/devicetree/bindings/phy/qcom,snps-eusb2-repeater.yaml
>>>>> @@ -52,6 +52,12 @@ properties:
>>>>>      minimum: 0
>>>>>      maximum: 7
>>>>>  
>>>>> +  qcom,tune-res-fsdif:
>>>>> +    $ref: /schemas/types.yaml#/definitions/uint8
>>>>> +    description: FS Differential TX Output Resistance Tuning
>>>>
>>>> Resistance is in Ohms, tuning could be in dB, so I wonder what are the
>>>> actual units here. Neither commit msg nor this description helps me to
>>>> understand that.
>>>
>>> I checked and the values are in Ohms.
>>
>> Yes it's Ohms but not 0x00 = 0 ohms, and it's also an offset in ohms
>> from the nominal value according to the Hardware Register Description I
>> have, e.g. 0x7 = -12.1ohm from the default
>>
>> I can try and create bindings using these Ohm offset values, I didn't
>> worry about it too much since the other tuning values in these bindings
>> are also just register values, presumably from before Konrad had access
>> to the docs.
> 
> I've taken some more looks, and checked how similar tuning is handled in
> qcom,usb-snps-femto-v2.yaml and phy-qcom-snps-femto-v2.c, and changing up
> the concept of tuning in the eUSB2-repeater bindings+driver is not a
> trivial task.
> 
> Since this is adding just one more property in-line with the already
> supported properties in the bindings+driver, can we get this in as-is,
> and deprecate all 4 qcom,tune-* properties later with a replacement that
> describes the values better?

This is a new property, so other existing properties do not matter here.
We cannot take new code which you already think should be deprecated.

register-like values are acceptable for vendor properties, but that does
not make them usually more readable. The question is whether this should
be more readable for hardware engineers or anyone writing/validating
DTS. Is the actual resistance important or no one ever cares because you
paste whatever qcom told you and you do not know what should be actually
there?

I can imagine the first - that some document explains you should have
resistance of foo because of bar, which would mean the property should
be more readable. But I can also imagine the second. Make your claim in
commit msg.

> We have enough people at Qualcomm by now that should be able to do that,
> and have the required resources to answer any potential questions.
They are busy sending vendor/downstream tree patches...

Best regards,
Krzysztof