[<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