Re: [PATCH 09/13] dt-bindings: phy: Add documentation for Airoha AN7581 USB PHY

[Krzysztof Kozlowski]

On 18/03/2025 12:31, Christian Marangi wrote:
> On Sun, Mar 16, 2025 at 06:01:02PM +0100, Krzysztof Kozlowski wrote:
>> On 11/03/2025 19:51, Christian Marangi wrote:
>>>>
>>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>>> +    enum: [0, 1]
>>>>
>>>> I don't understand why do you need index property here (which are
>>>> usually not allowed).
>>>>
>>>
>>> Eh... As said in the description this is really to differentiate the 2
>>> different physical port...
>>>
>>> Each port have a dedicated oscillator for calibration and these
>>> calibration are identified by an offset (all placed one after another in
>>> a separate register space).
>>
>> So different oscillators? Then describe the oscillator and its differences.
>>
>> Different programing model? Different compatible.
>>
>> Other difference? Depending what is the difference.
>>
>> But there is no such thing as "different port ID" based on your
>> description above. You just claimed that they are different, but you do
>> not put that difference to hardware description. Instead you encode that
>> difference in the drivers and it should be opposite. The DTS, so the
>> hardware description, should tell you the difference. And I am sorry,
>> but in 99% of cases "I am the first phy" and "I am the second" is not
>> the actual difference we are interested in.
>>
> 
> Ok to make it as clear as possible. (hope I don't contraddict with the
> previous statement) (actually yes the "separate register space statement
> was wrong and sorry for the confusion")
> 
> - 2 USB port
> - USB 2.0 needs to be calibrated with an oscillator
> - Each USB port have his own dedicated oscillator somewhere in the HW
> - Each USB port have at the same offset a register to SELECT the
>   oscillator. This register refer to the same oscillator selection in
>   the HW.
>   
>   Example:
>   reg 0x1fac0100 MASK 27:26 can be set to source out of oscillator 0,1,2
>   reg 0x1fae0100 MASK 27:26 can be set to source out of oscillator 0,1,2

So that's your difference between devices - you choose the oscillator.

> 
>   Both register refer to the same oscillators in hardware (hence each
>   port should select the correct one)
>   Selecting oscillator 0 for both USB port is problematic.
> 
> With this in mind is it ok if I describe this with something like
> 
> airoha,usb2-monitor-clk-sel = <AIROHA_USB2_MONCLK_SEL0>;
> 
> and some dt-bindings include
> 
> #define AIROHA_USB2_MONCLK_SEL0 0
> #define AIROHA_USB2_MONCLK_SEL1 1
> #define AIROHA_USB2_MONCLK_SEL2 2
> #define AIROHA_USB2_MONCLK_SEL3 3
> 
> Or an enum of string like "osc0", "osc1", "osc2"...?

Both are fine.

Best regards,
Krzysztof