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: <e720596d-6fbb-40a4-9567-e8d05755cf6f@nvidia.com>
Date: Thu, 12 Jun 2025 13:26:55 +0100
From: Jon Hunter <jonathanh@...dia.com>
To: Andrew Lunn <andrew@...n.ch>, Subbaraya Sundeep <sbhatta@...vell.com>
Cc: 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: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.

Jon

-- 
nvpublic


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ