Re: [PATCH iwlwifi-fixes] wifi: iwlwifi: Implement settime64 as stub for MVM/MLD PTP

[Yao Zi <me@ziyao.cc>]

On Sun, Dec 14, 2025 at 06:12:57PM +0800, David Wang wrote:
> On Thu, Dec 04, 2025 at 12:32:04PM +0000, Yao Zi wrote:
> > Since commit dfb073d32cac ("ptp: Return -EINVAL on ptp_clock_register if
> > required ops are NULL"), PTP clock registered through ptp_clock_register
> > is required to have ptp_clock_info.settime64 set, however, neither MVM
> > nor MLD's PTP clock implementation sets it, resulting in warnings when
> > the interface starts up, like
> > 
> > WARNING: drivers/ptp/ptp_clock.c:325 at ptp_clock_register+0x2c8/0x6b8, CPU#1: wpa_supplicant/469
> > CPU: 1 UID: 0 PID: 469 Comm: wpa_supplicant Not tainted 6.18.0+ #101 PREEMPT(full)
> > ra: ffff800002732cd4 iwl_mvm_ptp_init+0x114/0x188 [iwlmvm]
> > ERA: 9000000002fdc468 ptp_clock_register+0x2c8/0x6b8
> > iwlwifi 0000:01:00.0: Failed to register PHC clock (-22)
> > 
> > I don't find an appropriate firmware interface to implement settime64()
> > for iwlwifi MLD/MVM, thus instead create a stub that returns
> > -EOPTNOTSUPP only, suppressing the warning and allowing the PTP clock to
> > be registered.
> 
> This seems disturbing....If a null settime64 deserve a kernel WARN dump, so should
> a settime64 which returns error.

They're separate things. A ptp clock implementing not provinding
settime64() or gettime64()/gettimex64() callback will crash when
userspace tries to call clock_gettime()/clock_settime() on it, since
either ptp_clock_settime() or ptp_clock_gettime() invokes these
callbacks unconditionally.

However, failing with -ENOTSUPP/-EOPNOTSUPP when clock_settime() isn't
supported by a dynamic POSIX clock device is a documented behavior, see
man-page clock_getres(2).

> Before fixing the warning, the expected behavior of settime64 should be specified clearly,

I think failing with -EOPNOTSUPP (which is the same as -ENOTSUPP on
Linux) when the operation isn't supported is well-documented, and is
suitable for this case.

One may argue that it'd be helpful for ptp_clock_register() to provide
a default implementation of settime64() that always fails with
-EOPNOTSUPP when the driver doesn't provide one.

However, it's likely a programming bug when gettime64()/settime64() is
missing, so the current behavior of warning sounds reasonable to me.

> hence why the dfb073d32cac ("ptp: Return -EINVAL on ptp_clock_register if required ops are NULL")?

You may be interested in the original series[1] where the idea of
warning for missing settime64/gettime64/gettimex64 callbacks came up.

Also cc Kuniyuki, in case that I missed something or got it wrong.

> 
> David

Best regards,
Yao Zi

> > 
> > Reported-by: Nathan Chancellor <nathan@kernel.org>
> > Closes: https://lore.kernel.org/all/20251108044822.GA3262936@ax162/
> > Signed-off-by: Yao Zi <ziyao@disroot.org>
> > ---
> 

[1]: https://lore.kernel.org/all/20251028095143.396385-1-junjie.cao@intel.com/