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

[Jon Hunter <jonathanh@nvidia.com>]

On 13/06/2025 14:22, Andrew Lunn wrote:
>>> 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.
> 
> Lets just consider for the moment, that it never worked.

To be clear, by 'it never worked', you are referring to only PTP 
support? Then yes that is most likely.

> If we change the device tree to the expected 'ptp_ref', some devices
> actually start working. None regress, because none ever worked. We can
> also get the DT change added to stable, so older devices start
> working. We keep the code nice and clean, no special case.

Although PTP may not work, basic ethernet support does and 'correcting' 
the device-tree only, will break basic ethernet support for this device. 
That is what I am more concerned about and trying to avoid.

> Now, lets consider the case some devices do actually work. How are
> they working? Must it be the fallback? The ptp-ref clock is actually
> turned on, and if the ptp-ref clock and the main clock tick at the
> same rate, ptp would work. I _guess_, if the main clock and the
> ptp-ref clock tick at different rates, you get something from the ptp
> hardware, but it probably does not get sync with a grand master, or if
> it does, the jitter is high etc. So in effect it is still broken.

Given that we are seeing the error ...

  ERR KERN tegra-mgbe 6800000.ethernet eth0: Invalid PTP clock rate

Doesn't that imply that if we did attempt to use PTP on this device we 
would hit the bug reported by commit 030ce919e114 ("net: stmmac: make 
sure that ptp_rate is not 0 before configuring timestamping") and 
therefore, I would not expect PTP to work?

> 
> Can somebody with the datasheet actually determine where ptp-ref clock
> comes from? Is it just a gated main clock? Is it from a pin?

I can ask.

> If it does actually work, can we cause a regression by renaming the
> clock in DT? I _guess_ so, if the DT also has the clock wrong. So it
> is a fixed-clock, and that fixed clock has the wrong frequency set. It
> is not used at the moment, so being wrong does not matter. But when we
> start using it, things break. Is this possible? I don't know, i've not
> looked at the DT.
> 
> Before we decide how to fix this, we need a proper understanding of
> what is actually broken/works.

Yes that makes sense. Ethernet definitely works. I am not sure we have 
ever explicitly tested PTP with this driver, but I can ask.

Jon

-- 
nvpublic