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

[Jon Hunter <jonathanh@nvidia.com>]

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@nvidia.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