[PATCH v2 6/6] phy: tegra: xusb: Move T186 .set_mode() to common implementation

[Diogo Ivo]

On 3/24/26 13:33, Jon Hunter wrote:
> 
> 
> On 24/03/2026 11:31, Diogo Ivo wrote:
>>
>>
>> On 3/24/26 10:16, Jon Hunter wrote:
>>>
>>> On 27/01/2026 15:11, Diogo Ivo wrote:
>>>> Move the Tegra186 PHY .set_mode() callback to a common implementation.
>>>> In order to do this first revert cefc1caee9dd.
>>>
>>> This commit message does not seem complete.
>>
>> How so? It is succint but it states exactly what the commit does. It
>> reverts cefc1caee9dd and changes T186 to the common implementation
>> prepared in the previous patch.
> 
> It does not read clearly to me. The 2nd sentence sounds like that's all 
> this is doing but we are not, we are reverting and doing the move.
>>> Furthermore, I am not sure why we want to revert cefc1caee9dd. We 
>>> purposely moved the regulator_enable/disable into 
>>> tegra186_xusb_padctl_id_override() because it is tied to setting the 
>>> USB2_VBUS_ID. So I would prefer to keep it this way and move the 
>>> Tegra210 implementation in the same direction (if possible).
>>
>> I don't agree that this is the best solution.
>>
>> We really benefit from a common implementation for the two platforms, not
>> only because of duplicate code but more importantly because without it
>> whenever a bug is found and fixed on either platform it most likely will
>> not be fixed on the other one. Case in point, cefc1caee9dd fixed a bug
>> on T186 but not the same bug on T210 (which then led to this series) 
>> since
>> the implementation was not shared among them. Were it the case that they
>> shared the implementation the fix would have come "free" for T210.
>>
>> This will keep happening for as long as we have duplicate 
>> implementations,
>> which becomes more relevant since there is a severe lack of testing in
>> older Tegra platforms. I also thought about making the id_override()
>> implementation shared between T186 and T210 but that would be take more
>> changes since register definitions would need to be moved somewhere
>> else too.
> 
> I am all for a common implementation. I believe that in the 
> tegra186_xusb_padctl_id_override() function the only thing that is 
> different is the offset for the USB2_VBUS_ID register, which should be 
> easy to handle.

Ok, I can make it common there as well. However I still feel like
reverting cefc1caee9dd leads to cleaner code since vbus_override() and
id_override() will look similar and only do exactly what they state in
their names and the overall logic looks cleaner.

Diogo

> Jon
>