netdev - Re: [PATCH] net: stmmac: Fix PTP ref clock for Tegra234

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9544a718-1c1a-4c6b-96ae-d777400305a7@nvidia.com>
Date: Fri, 13 Jun 2025 12:15:48 +0100
From: Jon Hunter <jonathanh@...dia.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: Subbaraya Sundeep <sbhatta@...vell.com>,
 Andrew Lunn <andrew+netdev@...n.ch>, "David S . Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Maxime Coquelin <mcoquelin.stm32@...il.com>,
 Alexandre Torgue <alexandre.torgue@...s.st.com>, netdev@...r.kernel.org,
 linux-stm32@...md-mailman.stormreply.com, linux-tegra@...r.kernel.org,
 Alexis Lothorrr <alexis.lothore@...tlin.com>
Subject: Re: [PATCH] net: stmmac: Fix PTP ref clock for Tegra234

On 12/06/2025 13:45, Andrew Lunn wrote:
> On Thu, Jun 12, 2025 at 01:26:55PM +0100, Jon Hunter wrote:
>>
>> On 12/06/2025 13:10, Andrew Lunn wrote:
>>> On Thu, Jun 12, 2025 at 10:57:49AM +0000, Subbaraya Sundeep wrote:
>>>> Hi,
>>>>
>>>> On 2025-06-12 at 06:20:32, Jon Hunter (jonathanh@...dia.com) wrote:
>>>>> Since commit 030ce919e114 ("net: stmmac: make sure that ptp_rate is not
>>>>> 0 before configuring timestamping") was added the following error is
>>>>> observed on Tegra234:
>>>>>
>>>>>    ERR KERN tegra-mgbe 6800000.ethernet eth0: Invalid PTP clock rate
>>>>>    WARNING KERN tegra-mgbe 6800000.ethernet eth0: PTP init failed
>>>>>
>>>>> It turns out that the Tegra234 device-tree binding defines the PTP ref
>>>>> clock name as 'ptp-ref' and not 'ptp_ref' and the above commit now
>>>>> exposes this and that the PTP clock is not configured correctly.
>>>>>
>>>>> Ideally, we would rename the PTP ref clock for Tegra234 to fix this but
>>>>> this will break backward compatibility with existing device-tree blobs.
>>>>> Therefore, fix this by using the name 'ptp-ref' for devices that are
>>>>> compatible with 'nvidia,tegra234-mgbe'.
>>>
>>>> AFAIU for Tegra234 device from the beginning, entry in dts is ptp-ref.
>>>> Since driver is looking for ptp_ref it is getting 0 hence the crash
>>>> and after the commit 030ce919e114 result is Invalid error instead of crash.
>>>> For me PTP is not working for Tegra234 from day 1 so why to bother about
>>>> backward compatibility and instead fix dts.
>>>> Please help me understand it has been years I worked on dts.
>>>
>>> Please could you expand on that, because when i look at the code....
>>>
>>>
>>>     	/* Fall-back to main clock in case of no PTP ref is passed */
>>>    	plat->clk_ptp_ref = devm_clk_get(&pdev->dev, "ptp_ref");
>>>     	if (IS_ERR(plat->clk_ptp_ref)) {
>>>     		plat->clk_ptp_rate = clk_get_rate(plat->stmmac_clk);
>>>     		plat->clk_ptp_ref = NULL;
>>>
>>> if the ptp_ref does not exist, it falls back to stmmac_clk. Why would
>>> that cause a crash?
>>>   > While i agree if this never worked, we can ignore backwards
>>> compatibility and just fix the DT, but i would like a fuller
>>> explanation why the fallback is not sufficient to prevent a crash.
>>
>> The problem is that in the 'ptp-ref' clock name is also defined in the
>> 'mgbe_clks' array in dwmac-tegra.c driver. All of these clocks are requested
>> and enabled using the clk_bulk_xxx APIs and so I don't see how we can simply
>> fix this now without breaking support for older device-trees.
> 
> So you can definitively say, PTP does actually work? You have ptp4l
> running with older kernels and DT blob, and it has sync to a grand
> master?

So no I can't say that and I have not done any testing with PTP to be 
clear. However, the problem I see, is that because the driver defines 
the name as 'ptp-ref', if we were to update both the device-tree and the 
driver now to use the expected name 'ptp_ref', then and older 
device-tree will no longer work with the new driver regardless of the 
PTP because the devm_clk_bulk_get() in tegra_mgbe_probe() will fail.

I guess we could check to see if 'ptp-ref' or 'ptp_ref' is present 
during the tegra_mgbe_probe() and then update the mgbe_clks array as 
necessary.

Jon

-- 
nvpublic