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 14:22, Luca Weiss wrote:
> On Wed Jul 9, 2025 at 12:16 PM CEST, Krzysztof Kozlowski wrote:
>> 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.
> 
> Would this text in the commit message work for you?
> 
> ---
> Document the FS Differential TX Output Resistance Tuning value found on
> the eUSB2 repeater on Qualcomm PMICs. The tuning values have special
> meanings, being different offsets of the resistance to the default value
> in Ohms but the exact meaning is not relevant here, as the correct
> tuning is determined by hardware engineers to make sure the electrical
> properties are as expected.
> ---
> 
> I'm trying to avoid resending the whole series if we're not yet aligned
> on the wording.
Sounds fine.

Best regards,
Krzysztof