wifi: iwlwifi: Implement settime64 as stub for MVM/MLD PTP

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

Posted by Yao Zi 2 weeks, 1 day ago

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.

Reported-by: Nathan Chancellor <nathan@kernel.org>
Closes: https://lore.kernel.org/all/20251108044822.GA3262936@ax162/
Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 drivers/net/wireless/intel/iwlwifi/mld/ptp.c | 7 +++++++
 drivers/net/wireless/intel/iwlwifi/mvm/ptp.c | 7 +++++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/net/wireless/intel/iwlwifi/mld/ptp.c b/drivers/net/wireless/intel/iwlwifi/mld/ptp.c
index ffeb37a7f830..231920425c06 100644
--- a/drivers/net/wireless/intel/iwlwifi/mld/ptp.c
+++ b/drivers/net/wireless/intel/iwlwifi/mld/ptp.c
@@ -121,6 +121,12 @@ static int iwl_mld_ptp_gettime(struct ptp_clock_info *ptp,
 	return 0;
 }
 
+static int iwl_mld_ptp_settime(struct ptp_clock_info *ptp,
+			       const struct timespec64 *ts)
+{
+	return -EOPNOTSUPP;
+}
+
 static int iwl_mld_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 {
 	struct iwl_mld *mld = container_of(ptp, struct iwl_mld,
@@ -279,6 +285,7 @@ void iwl_mld_ptp_init(struct iwl_mld *mld)
 
 	mld->ptp_data.ptp_clock_info.owner = THIS_MODULE;
 	mld->ptp_data.ptp_clock_info.gettime64 = iwl_mld_ptp_gettime;
+	mld->ptp_data.ptp_clock_info.settime64 = iwl_mld_ptp_settime;
 	mld->ptp_data.ptp_clock_info.max_adj = 0x7fffffff;
 	mld->ptp_data.ptp_clock_info.adjtime = iwl_mld_ptp_adjtime;
 	mld->ptp_data.ptp_clock_info.adjfine = iwl_mld_ptp_adjfine;
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c b/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c
index 06a4c9f74797..ad156b82eaa9 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c
@@ -220,6 +220,12 @@ static int iwl_mvm_ptp_gettime(struct ptp_clock_info *ptp,
 	return 0;
 }
 
+static int iwl_mvm_ptp_settime(struct ptp_clock_info *ptp,
+			       const struct timespec64 *ts)
+{
+	return -EOPNOTSUPP;
+}
+
 static int iwl_mvm_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 {
 	struct iwl_mvm *mvm = container_of(ptp, struct iwl_mvm,
@@ -281,6 +287,7 @@ void iwl_mvm_ptp_init(struct iwl_mvm *mvm)
 	mvm->ptp_data.ptp_clock_info.adjfine = iwl_mvm_ptp_adjfine;
 	mvm->ptp_data.ptp_clock_info.adjtime = iwl_mvm_ptp_adjtime;
 	mvm->ptp_data.ptp_clock_info.gettime64 = iwl_mvm_ptp_gettime;
+	mvm->ptp_data.ptp_clock_info.settime64 = iwl_mvm_ptp_settime;
 	mvm->ptp_data.scaled_freq = SCALE_FACTOR;
 
 	/* Give a short 'friendly name' to identify the PHC clock */
-- 
2.51.2

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

Posted by David Wang 5 days, 6 hours ago

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.

Before fixing the warning, the expected behavior of settime64 should be specified clearly,
hence why the dfb073d32cac ("ptp: Return -EINVAL on ptp_clock_register if required ops are NULL")?



David

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

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

Posted by Yao Zi 4 days, 12 hours ago

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/

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

Posted by David Wang 4 days, 11 hours ago

At 2025-12-15 11:42:17, "Yao Zi" <me@ziyao.cc> wrote:
>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.

Thanks for the information, this link holds way more relevant information than the *Link* tag in dfb073d32cac.  :)

But isn't  the patch in [1]  and similar change to settime64 better? 
What is the  difference between a null callback and a callback returning error? aren't they saying the same
thing: "this device does not support it"?



David

>
>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/

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

Posted by Yao Zi 3 days, 22 hours ago

On Mon, Dec 15, 2025 at 12:20:43PM +0800, David Wang wrote:
> 
> At 2025-12-15 11:42:17, "Yao Zi" <me@ziyao.cc> wrote:
> >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.
> 
> Thanks for the information, this link holds way more relevant information than the *Link* tag in dfb073d32cac.  :)
> 
> But isn't  the patch in [1]  and similar change to settime64 better? 
> What is the  difference between a null callback and a callback returning error? aren't they saying the same
> thing: "this device does not support it"?

As I said earlier,

> However, it's likely a programming bug when gettime64()/settime64() is
> missing

but usually you know what exactly happens here when writing a stub
returning -EOPNOTSUPP. IMO this should be the difference.

Quoting Vadim in the original series,

> WARN_ON_ONCE is better in terms of reducing the amount of review work.
> Driver developers will be automatically notified about improper
> implementation while Junjie's patch will simply hide the problem.[2]

Regards,
Yao Zi

> 
> 
> David
> 
> >
> >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/


[2]: https://lore.kernel.org/all/b4d675a4-b7ad-4ecf-8d19-6bf08b452472@linux.dev/

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

Posted by Korenblit, Miriam Rachel 5 days, 18 hours ago


> -----Original Message-----
> From: Yao Zi <ziyao@disroot.org>
> Sent: Thursday, December 4, 2025 2:32 PM
> To: Korenblit, Miriam Rachel <miriam.rachel.korenblit@intel.com>; Richard
> Cochran <richardcochran@gmail.com>; Berg, Johannes
> <johannes.berg@intel.com>; Anjaneyulu, Pagadala Yesu
> <pagadala.yesu.anjaneyulu@intel.com>; Gabay, Daniel
> <daniel.gabay@intel.com>
> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> netdev@vger.kernel.org; Mingcong Bai <jeffbai@aosc.io>; Kexy Biscuit
> <kexybiscuit@aosc.io>; Yao Zi <ziyao@disroot.org>; Nathan Chancellor
> <nathan@kernel.org>
> Subject: [PATCH iwlwifi-fixes] wifi: iwlwifi: Implement settime64 as stub for
> MVM/MLD PTP
> 
> 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.
> 
> Reported-by: Nathan Chancellor <nathan@kernel.org>
> Closes: https://lore.kernel.org/all/20251108044822.GA3262936@ax162/
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  drivers/net/wireless/intel/iwlwifi/mld/ptp.c | 7 +++++++
> drivers/net/wireless/intel/iwlwifi/mvm/ptp.c | 7 +++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/drivers/net/wireless/intel/iwlwifi/mld/ptp.c
> b/drivers/net/wireless/intel/iwlwifi/mld/ptp.c
> index ffeb37a7f830..231920425c06 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mld/ptp.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mld/ptp.c
> @@ -121,6 +121,12 @@ static int iwl_mld_ptp_gettime(struct ptp_clock_info
> *ptp,
>  	return 0;
>  }
> 
> +static int iwl_mld_ptp_settime(struct ptp_clock_info *ptp,
> +			       const struct timespec64 *ts)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
>  static int iwl_mld_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)  {
>  	struct iwl_mld *mld = container_of(ptp, struct iwl_mld, @@ -279,6
> +285,7 @@ void iwl_mld_ptp_init(struct iwl_mld *mld)
> 
>  	mld->ptp_data.ptp_clock_info.owner = THIS_MODULE;
>  	mld->ptp_data.ptp_clock_info.gettime64 = iwl_mld_ptp_gettime;
> +	mld->ptp_data.ptp_clock_info.settime64 = iwl_mld_ptp_settime;
>  	mld->ptp_data.ptp_clock_info.max_adj = 0x7fffffff;
>  	mld->ptp_data.ptp_clock_info.adjtime = iwl_mld_ptp_adjtime;
>  	mld->ptp_data.ptp_clock_info.adjfine = iwl_mld_ptp_adjfine; diff --git
> a/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c
> b/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c
> index 06a4c9f74797..ad156b82eaa9 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c
> @@ -220,6 +220,12 @@ static int iwl_mvm_ptp_gettime(struct ptp_clock_info
> *ptp,
>  	return 0;
>  }
> 
> +static int iwl_mvm_ptp_settime(struct ptp_clock_info *ptp,
> +			       const struct timespec64 *ts)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
>  static int iwl_mvm_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)  {
>  	struct iwl_mvm *mvm = container_of(ptp, struct iwl_mvm, @@ -281,6
> +287,7 @@ void iwl_mvm_ptp_init(struct iwl_mvm *mvm)
>  	mvm->ptp_data.ptp_clock_info.adjfine = iwl_mvm_ptp_adjfine;
>  	mvm->ptp_data.ptp_clock_info.adjtime = iwl_mvm_ptp_adjtime;
>  	mvm->ptp_data.ptp_clock_info.gettime64 = iwl_mvm_ptp_gettime;
> +	mvm->ptp_data.ptp_clock_info.settime64 = iwl_mvm_ptp_settime;
>  	mvm->ptp_data.scaled_freq = SCALE_FACTOR;
> 
>  	/* Give a short 'friendly name' to identify the PHC clock */
> --
> 2.51.2
Acked-by: Miri Korenblit <miriam.rachel.korenblit@intel.com>

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

Posted by Oliver Hartkopp 6 days, 7 hours ago


On 04.12.25 13:32, 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.
> 
> Reported-by: Nathan Chancellor <nathan@kernel.org>
> Closes: https://lore.kernel.org/all/20251108044822.GA3262936@ax162/
> Signed-off-by: Yao Zi <ziyao@disroot.org>

Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>

I can see no progress that this patch gets upstreamed.
Who is picking up this fix?

Best regards,
Oliver


> ---
>   drivers/net/wireless/intel/iwlwifi/mld/ptp.c | 7 +++++++
>   drivers/net/wireless/intel/iwlwifi/mvm/ptp.c | 7 +++++++
>   2 files changed, 14 insertions(+)
> 
> diff --git a/drivers/net/wireless/intel/iwlwifi/mld/ptp.c b/drivers/net/wireless/intel/iwlwifi/mld/ptp.c
> index ffeb37a7f830..231920425c06 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mld/ptp.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mld/ptp.c
> @@ -121,6 +121,12 @@ static int iwl_mld_ptp_gettime(struct ptp_clock_info *ptp,
>   	return 0;
>   }
>   
> +static int iwl_mld_ptp_settime(struct ptp_clock_info *ptp,
> +			       const struct timespec64 *ts)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
>   static int iwl_mld_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
>   {
>   	struct iwl_mld *mld = container_of(ptp, struct iwl_mld,
> @@ -279,6 +285,7 @@ void iwl_mld_ptp_init(struct iwl_mld *mld)
>   
>   	mld->ptp_data.ptp_clock_info.owner = THIS_MODULE;
>   	mld->ptp_data.ptp_clock_info.gettime64 = iwl_mld_ptp_gettime;
> +	mld->ptp_data.ptp_clock_info.settime64 = iwl_mld_ptp_settime;
>   	mld->ptp_data.ptp_clock_info.max_adj = 0x7fffffff;
>   	mld->ptp_data.ptp_clock_info.adjtime = iwl_mld_ptp_adjtime;
>   	mld->ptp_data.ptp_clock_info.adjfine = iwl_mld_ptp_adjfine;
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c b/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c
> index 06a4c9f74797..ad156b82eaa9 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c
> @@ -220,6 +220,12 @@ static int iwl_mvm_ptp_gettime(struct ptp_clock_info *ptp,
>   	return 0;
>   }
>   
> +static int iwl_mvm_ptp_settime(struct ptp_clock_info *ptp,
> +			       const struct timespec64 *ts)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
>   static int iwl_mvm_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
>   {
>   	struct iwl_mvm *mvm = container_of(ptp, struct iwl_mvm,
> @@ -281,6 +287,7 @@ void iwl_mvm_ptp_init(struct iwl_mvm *mvm)
>   	mvm->ptp_data.ptp_clock_info.adjfine = iwl_mvm_ptp_adjfine;
>   	mvm->ptp_data.ptp_clock_info.adjtime = iwl_mvm_ptp_adjtime;
>   	mvm->ptp_data.ptp_clock_info.gettime64 = iwl_mvm_ptp_gettime;
> +	mvm->ptp_data.ptp_clock_info.settime64 = iwl_mvm_ptp_settime;
>   	mvm->ptp_data.scaled_freq = SCALE_FACTOR;
>   
>   	/* Give a short 'friendly name' to identify the PHC clock */

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

Posted by Damian Tometzki 1 week, 4 days ago

On Thu, 04. Dec 12:32, 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.
> 
> Reported-by: Nathan Chancellor <nathan@kernel.org>
> Closes: https://lore.kernel.org/all/20251108044822.GA3262936@ax162/
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  drivers/net/wireless/intel/iwlwifi/mld/ptp.c | 7 +++++++
>  drivers/net/wireless/intel/iwlwifi/mvm/ptp.c | 7 +++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/drivers/net/wireless/intel/iwlwifi/mld/ptp.c b/drivers/net/wireless/intel/iwlwifi/mld/ptp.c
> index ffeb37a7f830..231920425c06 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mld/ptp.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mld/ptp.c
> @@ -121,6 +121,12 @@ static int iwl_mld_ptp_gettime(struct ptp_clock_info *ptp,
>  	return 0;
>  }
>  
> +static int iwl_mld_ptp_settime(struct ptp_clock_info *ptp,
> +			       const struct timespec64 *ts)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
>  static int iwl_mld_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
>  {
>  	struct iwl_mld *mld = container_of(ptp, struct iwl_mld,
> @@ -279,6 +285,7 @@ void iwl_mld_ptp_init(struct iwl_mld *mld)
>  
>  	mld->ptp_data.ptp_clock_info.owner = THIS_MODULE;
>  	mld->ptp_data.ptp_clock_info.gettime64 = iwl_mld_ptp_gettime;
> +	mld->ptp_data.ptp_clock_info.settime64 = iwl_mld_ptp_settime;
>  	mld->ptp_data.ptp_clock_info.max_adj = 0x7fffffff;
>  	mld->ptp_data.ptp_clock_info.adjtime = iwl_mld_ptp_adjtime;
>  	mld->ptp_data.ptp_clock_info.adjfine = iwl_mld_ptp_adjfine;
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c b/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c
> index 06a4c9f74797..ad156b82eaa9 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c
> @@ -220,6 +220,12 @@ static int iwl_mvm_ptp_gettime(struct ptp_clock_info *ptp,
>  	return 0;
>  }
>  
> +static int iwl_mvm_ptp_settime(struct ptp_clock_info *ptp,
> +			       const struct timespec64 *ts)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
>  static int iwl_mvm_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
>  {
>  	struct iwl_mvm *mvm = container_of(ptp, struct iwl_mvm,
> @@ -281,6 +287,7 @@ void iwl_mvm_ptp_init(struct iwl_mvm *mvm)
>  	mvm->ptp_data.ptp_clock_info.adjfine = iwl_mvm_ptp_adjfine;
>  	mvm->ptp_data.ptp_clock_info.adjtime = iwl_mvm_ptp_adjtime;
>  	mvm->ptp_data.ptp_clock_info.gettime64 = iwl_mvm_ptp_gettime;
> +	mvm->ptp_data.ptp_clock_info.settime64 = iwl_mvm_ptp_settime;
>  	mvm->ptp_data.scaled_freq = SCALE_FACTOR;
>  
>  	/* Give a short 'friendly name' to identify the PHC clock */
> -- 
> 2.51.2
> 
Hi,

the patch solved my issue too. 

tested-by: damian Tometzki damian@riscv-rocks.de
-- 
VG
Damian Tometzki

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

Posted by Genes Lists 1 week, 4 days ago

On Thu, 2025-12-04 at 12:32 +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

I do see this warning at boot (mainline kernel),  but you noted, it all
works fine nonetheless.
I didn't see this in next or mainline yet.

Do you know when this might find it's way to mainline?

thanks

-- 
Gene

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

Posted by Simon Horman 1 week, 6 days ago

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

Reviewed-by: Simon Horman <horms@kernel.org>

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

Posted by Nathan Chancellor 1 week, 6 days ago

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

Thanks, I can confirm this clears up the warning.

Tested-by: Nathan Chancellor <nathan@kernel.org>

>  drivers/net/wireless/intel/iwlwifi/mld/ptp.c | 7 +++++++
>  drivers/net/wireless/intel/iwlwifi/mvm/ptp.c | 7 +++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/drivers/net/wireless/intel/iwlwifi/mld/ptp.c b/drivers/net/wireless/intel/iwlwifi/mld/ptp.c
> index ffeb37a7f830..231920425c06 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mld/ptp.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mld/ptp.c
> @@ -121,6 +121,12 @@ static int iwl_mld_ptp_gettime(struct ptp_clock_info *ptp,
>  	return 0;
>  }
>  
> +static int iwl_mld_ptp_settime(struct ptp_clock_info *ptp,
> +			       const struct timespec64 *ts)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
>  static int iwl_mld_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
>  {
>  	struct iwl_mld *mld = container_of(ptp, struct iwl_mld,
> @@ -279,6 +285,7 @@ void iwl_mld_ptp_init(struct iwl_mld *mld)
>  
>  	mld->ptp_data.ptp_clock_info.owner = THIS_MODULE;
>  	mld->ptp_data.ptp_clock_info.gettime64 = iwl_mld_ptp_gettime;
> +	mld->ptp_data.ptp_clock_info.settime64 = iwl_mld_ptp_settime;
>  	mld->ptp_data.ptp_clock_info.max_adj = 0x7fffffff;
>  	mld->ptp_data.ptp_clock_info.adjtime = iwl_mld_ptp_adjtime;
>  	mld->ptp_data.ptp_clock_info.adjfine = iwl_mld_ptp_adjfine;
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c b/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c
> index 06a4c9f74797..ad156b82eaa9 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/ptp.c
> @@ -220,6 +220,12 @@ static int iwl_mvm_ptp_gettime(struct ptp_clock_info *ptp,
>  	return 0;
>  }
>  
> +static int iwl_mvm_ptp_settime(struct ptp_clock_info *ptp,
> +			       const struct timespec64 *ts)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
>  static int iwl_mvm_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
>  {
>  	struct iwl_mvm *mvm = container_of(ptp, struct iwl_mvm,
> @@ -281,6 +287,7 @@ void iwl_mvm_ptp_init(struct iwl_mvm *mvm)
>  	mvm->ptp_data.ptp_clock_info.adjfine = iwl_mvm_ptp_adjfine;
>  	mvm->ptp_data.ptp_clock_info.adjtime = iwl_mvm_ptp_adjtime;
>  	mvm->ptp_data.ptp_clock_info.gettime64 = iwl_mvm_ptp_gettime;
> +	mvm->ptp_data.ptp_clock_info.settime64 = iwl_mvm_ptp_settime;
>  	mvm->ptp_data.scaled_freq = SCALE_FACTOR;
>  
>  	/* Give a short 'friendly name' to identify the PHC clock */
> -- 
> 2.51.2
>