Re: [PATCH net v3] net: stmmac: propagate PTP addend and system time programming errors

[Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>]

[-- Attachment #1: Type: text/plain, Size: 5774 bytes --]

On Oct 06, Anirudh Srinivasan wrote:
> Hi Lorenzo,
> 
> On Tue, Oct 6, 2026 at 1:57 AM Lorenzo Bianconi
> <lorenzo.bianconi@oss.qualcomm.com> wrote:
> >
> > > Hi Lorenzo,
> > >
> > > On Mon, Oct 5, 2026 at 4:53 PM Lorenzo Bianconi
> > > <lorenzo.bianconi@oss.qualcomm.com> wrote:
> > > >
> > > > > On Tue, Sep 29, 2026 at 03:10:11PM +0200, Lorenzo Bianconi wrote:
> > > > > > stmmac_update_subsecond_increment() ignores the error returned by
> > > > > > stmmac_config_addend(), and stmmac_init_tstamp_counter() discards the
> > > > > > addend and system time programming errors, always returning success. A
> > > > > > failure to program the addend (PTP_TCR_TSADDREG) or to initialize the
> > > > > > system time counter (PTP_TCR_TSINIT) is therefore silently swallowed,
> > > > > > leaving the hardware timestamp counter in a non-running or partially
> > > > > > configured state while the driver keeps operating as if timestamping
> > > > > > were up. This matters for TAPRIO/EST offloading, which derives the gate
> > > > > > base time from the hardware timestamp counter.
> > > > > >
> > > > > > The same hooks are also called from the PHC callbacks: settime64 and
> > > > > > adjfine drop the error and report success to clock_settime() and
> > > > > > clock_adjtime(), so a dead PTP reference clock goes unnoticed by
> > > > > > ptp4l/phc2sys.
> > > > > >
> > > > > > Return error codes from stmmac_update_subsecond_increment(),
> > > > > > stmmac_init_tstamp_counter(), stmmac_dl_ts_coarse_set() and the
> > > > > > settime64/adjfine callbacks instead of silently returning success. On
> > > > > > failure, roll back the partially applied configuration so the hardware
> > > > > > and the driver bookkeeping stay consistent, and report the reason
> > > > > > through the devlink extack. Also guard against a zero sub-second
> > > > > > increment, which would otherwise divide by zero when computing the
> > > > > > addend.
> > > > > >
> > > > > > Reset the persistent timestamping state (hwts_tx_en, hwts_rx_en,
> > > > > > tstamp_config, systime_flags and tsfupdt_coarse) when (re)initializing
> > > > > > timestamping, so a failed init does not leave TX/RX timestamping
> > > > > > enabled on a counter that never started.
> > > > > >
> > > > > > Fixes: cc4c9001ce31 ("net: stmmac: Switch stmmac_hwtimestamp to generic HW Interface Helpers")
> > > > > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
> > > > > > ---
> > > > > > Changes in v3:
> > > > > > - Do not run stmmac_config_addend() in
> > > > > >   stmmac_update_subsecond_increment() error path.
> > > > > > - Return error from stmmac_adjust_freq() and stmmac_set_time().
> > > > > > - Reset hw ts configuration in stmmac_init_timestamping().
> > > > > > - Link to v2: https://lore.kernel.org/r/20260924-stmmac-ptp-added-systime-error-v2-1-beb2a6b5f866@oss.qualcomm.com
> > > > > >
> > > > > > Changes in v2:
> > > > > > - Initialize sec_inc to 0 in stmmac_restore_subsecond_increment()
> > > > > >   routine.
> > > > > > - Link to v1: https://lore.kernel.org/r/20260920-stmmac-ptp-added-systime-error-v1-1-8ac9e7a3fce2@oss.qualcomm.com
> > > > > > ---
> > > > > >  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 106 ++++++++++++++++------
> > > > > >  drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c  |  10 +-
> > > > > >  2 files changed, 84 insertions(+), 32 deletions(-)
> > > > >
> > > > > Hello, I'm noticing that after this patch was merged into linux-next,
> > > > > boot seems to hang when ip=dhcp is used because ethernet isn't working
> > > > > on the TH1520 Lichee Pi 4a. These lines get printed in a loop over and
> > > > > over
> > > > >
> > > > > IP-Config: end0 hardware address 72:ca:a8:eb:27:[   24.872982] thead-dwmac ffe7070000.ethernet end0: Register MEM_TYPE_PAGE_POOL RxQ-0
> > > > > f2 mtu 1500 DHCP
> > > > > [   24.900412] thead-dwmac ffe7070000.ethernet end0: PHY [stmmac-0:01] driver [RTL8211F Gigabit Ethernet] (irq=POLL)
> > > > > [   24.912780] dwmac1000: Master AXI performs any burst length
> > > > > [   24.912801] thead-dwmac ffe7070000.ethernet end0: No Safety Features support found
> > > > > [   25.010229] thead-dwmac ffe7070000.ethernet end0: PTP init failed
> > > > > SIOCSIFFLAGS: Connection timed out
> > > >
> > > > Hi Anirudh,
> > > >
> > > > based on the reported error, stmmac_init_tstamp_counter() fails with
> > > > -ETIMEDOUT. In particular this can occurs if:
> > > >
> > > > stmmac_init_tstamp_counter()
> > > >  -> stmmac_update_subsecond_increment() -> stmmac_config_addend() -> -ETIMEDOUT
> > > >  -> stmmac_init_systime() -> -ETIMEDOUT
> > > >
> > > > I guess we should understand which one is failing and why it is failing.
> > >
> > > It seems like both are timing out, both config_addend
> > > (PTP_TCR_TSADDREG) and init_systime (PTP_TCR_TSINIT).
> >
> > It seems hw timestamping has never worked on this board, it was just
> > undiscovered since stmmac_init_tstamp_counter() was not reporting any
> > error before (this is exactly the goal of this patch).
> >
> > What are the output for:
> > - IEEE 1588-2002 Time Stamp
> > - IEEE 1588-2008 Advanced Time Stamp
> >
> > root@rb3-gen2:~# grep 'Time Stamp' /sys/kernel/debug/stmmaceth/eth0/dma_cap
> > IEEE 1588-2002 Time Stamp: N
> > IEEE 1588-2008 Advanced Time Stamp: Y
> 
> This is what I see
> 
> root@debian-trixie-riscv64:/# grep 'Time Stamp'
> /sys/kernel/debug/stmmaceth/end0/dma_cap
> IEEE 1588-2002 Time Stamp: N
> IEEE 1588-2008 Advanced Time Stamp: Y

Unfortunately I do not have this board for debugging. The first idea
I got is maybe 100ms is too small for this SoC? Can you please try to
increase it to like 500ms?

Regards,
Lorenzo
 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]