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

From: Jon Hunter <jonathanh@nvidia.com>
To: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>,
	Mathias Nyman <mathias.nyman@intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Thierry Reding <thierry.reding@gmail.com>,
	JC Kuo <jckuo@nvidia.com>, Vinod Koul <vkoul@kernel.org>,
	Kishon Vijay Abraham I <kishon@kernel.org>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Neil Armstrong <neil.armstrong@linaro.org>
Cc: linux-usb@vger.kernel.org, linux-tegra@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-phy@lists.infradead.org,
	devicetree@vger.kernel.org
Subject: Re: [PATCH v2 6/6] phy: tegra: xusb: Move T186 .set_mode() to common implementation
Date: Tue, 24 Mar 2026 13:33:17 +0000	[thread overview]
Message-ID: <7a6f8967-c635-4d84-bbab-9e019ff79134@nvidia.com> (raw)
In-Reply-To: <5a5397c8-cc32-4d6b-86a4-76f924ae6d75@tecnico.ulisboa.pt>

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.

Jon

-- 
nvpublic

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy