[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aT-DmZTh_8I13Mg1@pie>
Date: Mon, 15 Dec 2025 03:42:17 +0000
From: Yao Zi <me@...ao.cc>
To: David Wang <00107082@....com>, thostet@...gle.com
Cc: daniel.gabay@...el.com, jeffbai@...c.io, johannes.berg@...el.com,
kexybiscuit@...c.io, linux-kernel@...r.kernel.org,
linux-wireless@...r.kernel.org, miriam.rachel.korenblit@...el.com,
nathan@...nel.org, netdev@...r.kernel.org,
pagadala.yesu.anjaneyulu@...el.com, richardcochran@...il.com,
Kuniyuki Iwashima <kuniyu@...gle.com>
Subject: Re: [PATCH iwlwifi-fixes] wifi: iwlwifi: Implement settime64 as stub
for MVM/MLD PTP
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@...nel.org>
> > Closes: https://lore.kernel.org/all/20251108044822.GA3262936@ax162/
> > Signed-off-by: Yao Zi <ziyao@...root.org>
> > ---
>
[1]: https://lore.kernel.org/all/20251028095143.396385-1-junjie.cao@intel.com/
Powered by blists - more mailing lists