public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH iwl-next] i40e: PTP: set supported flags in ptp_clock_info
@ 2026-03-09 14:11 Przemyslaw Korba
  2026-03-09 16:34 ` [Intel-wired-lan] " Paul Menzel
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Przemyslaw Korba @ 2026-03-09 14:11 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: netdev, anthony.l.nguyen, przemyslaw.kitszel, Przemyslaw Korba

Since upstream commit d9f3e9ecc456 ("net: ptp: introduce
.supported_perout_flags to ptp_clock_info") and commit 7c571ac57d9d ("net:
ptp: introduce .supported_extts_flags to ptp_clock_info"), kernel core now
requires that the driver set the .supported_perout_flags and
.supported_extts_flags fields in PTP clock info. Otherwise, the additional
flags will be rejected by the kernel automatically.

i40e does not support perout flags, so reject any request with perout
flags.

Signed-off-by: Przemyslaw Korba <przemyslaw.korba@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_ptp.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
index 7bcea7d9720f..8d7958692235 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
@@ -601,10 +601,18 @@ static int i40e_ptp_feature_enable(struct ptp_clock_info *ptp,
 	/* TODO: Implement flags handling for EXTTS and PEROUT */
 	switch (rq->type) {
 	case PTP_CLK_REQ_EXTTS:
+		if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
+					PTP_RISING_EDGE |
+					PTP_FALLING_EDGE |
+					PTP_STRICT_FLAGS))
+			return -EOPNOTSUPP;
+
 		func = PTP_PF_EXTTS;
 		chan = rq->extts.index;
 		break;
 	case PTP_CLK_REQ_PEROUT:
+		if (rq->perout.flags)
+			return -EOPNOTSUPP;
 		func = PTP_PF_PEROUT;
 		chan = rq->perout.index;
 		break;
@@ -1340,7 +1348,9 @@ static int i40e_init_pin_config(struct i40e_pf *pf)
 	pf->ptp_caps.n_ext_ts = 2;
 	pf->ptp_caps.pps = 1;
 	pf->ptp_caps.n_per_out = 2;
-
+	pf->ptp_caps.supported_extts_flags = PTP_RISING_EDGE |
+					     PTP_FALLING_EDGE |
+					     PTP_STRICT_FLAGS;
 	pf->ptp_caps.pin_config = kzalloc_objs(*pf->ptp_caps.pin_config,
 					       pf->ptp_caps.n_pins);
 	if (!pf->ptp_caps.pin_config)

base-commit: d5fbc991435eac7a1ead7cd2ddb5a743528718bb
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [Intel-wired-lan] [PATCH iwl-next] i40e: PTP: set supported flags in ptp_clock_info
  2026-03-09 14:11 [PATCH iwl-next] i40e: PTP: set supported flags in ptp_clock_info Przemyslaw Korba
@ 2026-03-09 16:34 ` Paul Menzel
  2026-03-11 12:42   ` Korba, Przemyslaw
  2026-03-10 18:24 ` Simon Horman
  2026-03-11  8:01 ` [Intel-wired-lan] " Dawid Osuchowski
  2 siblings, 1 reply; 10+ messages in thread
From: Paul Menzel @ 2026-03-09 16:34 UTC (permalink / raw)
  To: Przemyslaw Korba
  Cc: intel-wired-lan, netdev, anthony.l.nguyen, przemyslaw.kitszel

Dear Przemyslaw,


Thank you for your patch.

Am 09.03.26 um 15:11 schrieb Przemyslaw Korba:
> Since upstream commit d9f3e9ecc456 ("net: ptp: introduce
> .supported_perout_flags to ptp_clock_info") and commit 7c571ac57d9d ("net:
> ptp: introduce .supported_extts_flags to ptp_clock_info"), kernel core now
> requires that the driver set the .supported_perout_flags and
> .supported_extts_flags fields in PTP clock info. Otherwise, the additional
> flags will be rejected by the kernel automatically.
> 
> i40e does not support perout flags, so reject any request with perout
> flags.

As you reference commits, why not add Fixes: tags?

> Signed-off-by: Przemyslaw Korba <przemyslaw.korba@intel.com>
> ---
>   drivers/net/ethernet/intel/i40e/i40e_ptp.c | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> index 7bcea7d9720f..8d7958692235 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> @@ -601,10 +601,18 @@ static int i40e_ptp_feature_enable(struct ptp_clock_info *ptp,
>   	/* TODO: Implement flags handling for EXTTS and PEROUT */
>   	switch (rq->type) {
>   	case PTP_CLK_REQ_EXTTS:
> +		if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
> +					PTP_RISING_EDGE |
> +					PTP_FALLING_EDGE |
> +					PTP_STRICT_FLAGS))
> +			return -EOPNOTSUPP;
> +
>   		func = PTP_PF_EXTTS;
>   		chan = rq->extts.index;
>   		break;
>   	case PTP_CLK_REQ_PEROUT:
> +		if (rq->perout.flags)
> +			return -EOPNOTSUPP;
>   		func = PTP_PF_PEROUT;
>   		chan = rq->perout.index;
>   		break;
> @@ -1340,7 +1348,9 @@ static int i40e_init_pin_config(struct i40e_pf *pf)
>   	pf->ptp_caps.n_ext_ts = 2;
>   	pf->ptp_caps.pps = 1;
>   	pf->ptp_caps.n_per_out = 2;
> -
> +	pf->ptp_caps.supported_extts_flags = PTP_RISING_EDGE |
> +					     PTP_FALLING_EDGE |
> +					     PTP_STRICT_FLAGS;
>   	pf->ptp_caps.pin_config = kzalloc_objs(*pf->ptp_caps.pin_config,
>   					       pf->ptp_caps.n_pins);
>   	if (!pf->ptp_caps.pin_config)

Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH iwl-next] i40e: PTP: set supported flags in ptp_clock_info
  2026-03-09 14:11 [PATCH iwl-next] i40e: PTP: set supported flags in ptp_clock_info Przemyslaw Korba
  2026-03-09 16:34 ` [Intel-wired-lan] " Paul Menzel
@ 2026-03-10 18:24 ` Simon Horman
  2026-03-11 12:42   ` Korba, Przemyslaw
  2026-03-11  8:01 ` [Intel-wired-lan] " Dawid Osuchowski
  2 siblings, 1 reply; 10+ messages in thread
From: Simon Horman @ 2026-03-10 18:24 UTC (permalink / raw)
  To: Przemyslaw Korba
  Cc: intel-wired-lan, netdev, anthony.l.nguyen, przemyslaw.kitszel,
	Jacob Keller

+ Jacob

On Mon, Mar 09, 2026 at 03:11:51PM +0100, Przemyslaw Korba wrote:
> Since upstream commit d9f3e9ecc456 ("net: ptp: introduce
> .supported_perout_flags to ptp_clock_info") and commit 7c571ac57d9d ("net:
> ptp: introduce .supported_extts_flags to ptp_clock_info"), kernel core now
> requires that the driver set the .supported_perout_flags and
> .supported_extts_flags fields in PTP clock info. Otherwise, the additional
> flags will be rejected by the kernel automatically.
> 
> i40e does not support perout flags, so reject any request with perout
> flags.
> 
> Signed-off-by: Przemyslaw Korba <przemyslaw.korba@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_ptp.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> index 7bcea7d9720f..8d7958692235 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> @@ -601,10 +601,18 @@ static int i40e_ptp_feature_enable(struct ptp_clock_info *ptp,
>  	/* TODO: Implement flags handling for EXTTS and PEROUT */
>  	switch (rq->type) {
>  	case PTP_CLK_REQ_EXTTS:
> +		if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
> +					PTP_RISING_EDGE |
> +					PTP_FALLING_EDGE |
> +					PTP_STRICT_FLAGS))
> +			return -EOPNOTSUPP;
> +
>  		func = PTP_PF_EXTTS;
>  		chan = rq->extts.index;
>  		break;
>  	case PTP_CLK_REQ_PEROUT:
> +		if (rq->perout.flags)
> +			return -EOPNOTSUPP;
>  		func = PTP_PF_PEROUT;
>  		chan = rq->perout.index;
>  		break;

I am a little confused.

My understanding of the cited patches is that they add checking
of flags to the code. So code like the above isn't needed in drivers.

> @@ -1340,7 +1348,9 @@ static int i40e_init_pin_config(struct i40e_pf *pf)
>  	pf->ptp_caps.n_ext_ts = 2;
>  	pf->ptp_caps.pps = 1;
>  	pf->ptp_caps.n_per_out = 2;
> -
> +	pf->ptp_caps.supported_extts_flags = PTP_RISING_EDGE |
> +					     PTP_FALLING_EDGE |
> +					     PTP_STRICT_FLAGS;
>  	pf->ptp_caps.pin_config = kzalloc_objs(*pf->ptp_caps.pin_config,
>  					       pf->ptp_caps.n_pins);
>  	if (!pf->ptp_caps.pin_config)
> 
> base-commit: d5fbc991435eac7a1ead7cd2ddb5a743528718bb
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Intel-wired-lan] [PATCH iwl-next] i40e: PTP: set supported flags in ptp_clock_info
  2026-03-09 14:11 [PATCH iwl-next] i40e: PTP: set supported flags in ptp_clock_info Przemyslaw Korba
  2026-03-09 16:34 ` [Intel-wired-lan] " Paul Menzel
  2026-03-10 18:24 ` Simon Horman
@ 2026-03-11  8:01 ` Dawid Osuchowski
  2026-03-11 12:45   ` Korba, Przemyslaw
  2 siblings, 1 reply; 10+ messages in thread
From: Dawid Osuchowski @ 2026-03-11  8:01 UTC (permalink / raw)
  To: Przemyslaw Korba, intel-wired-lan
  Cc: netdev, anthony.l.nguyen, przemyslaw.kitszel

On 2026-03-09 3:11 PM, Przemyslaw Korba wrote:
> Since upstream commit d9f3e9ecc456 ("net: ptp: introduce
> .supported_perout_flags to ptp_clock_info") and commit 7c571ac57d9d ("net:
> ptp: introduce .supported_extts_flags to ptp_clock_info"), kernel core now
> requires that the driver set the .supported_perout_flags and
> .supported_extts_flags fields in PTP clock info. Otherwise, the additional
> flags will be rejected by the kernel automatically.
> 
> i40e does not support perout flags, so reject any request with perout
> flags.
> 
> Signed-off-by: Przemyslaw Korba <przemyslaw.korba@intel.com>

Hey Przemek,

**Please don't circumvent our established process of going through 
internal review before posting to iwl**. You should send to our internal 
mailing list and get at least one Reviewed-by tag first, before posting 
to iwl. I think we make exceptions only when there is a strict time 
constraint and need to get the change out FAST (e.g. bug present in 
Tony's tree or netdev but not in Linus' tree yet). Reach out internally 
if you don't know the process and I can share the proper resources.

> ---
>   drivers/net/ethernet/intel/i40e/i40e_ptp.c | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> index 7bcea7d9720f..8d7958692235 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> @@ -601,10 +601,18 @@ static int i40e_ptp_feature_enable(struct ptp_clock_info *ptp,
>   	/* TODO: Implement flags handling for EXTTS and PEROUT */

Ignoring Simon's comment (not because it doesn't have merit, but because 
I don't know this part of the driver and the referenced patches), given 
you just seem to have implemented setting (or maybe handling) these 
flags in this commit, might be the time to remove this TODO comment? :)

-Dawid

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH iwl-next] i40e: PTP: set supported flags in ptp_clock_info
  2026-03-10 18:24 ` Simon Horman
@ 2026-03-11 12:42   ` Korba, Przemyslaw
  2026-03-13 13:34     ` Simon Horman
  0 siblings, 1 reply; 10+ messages in thread
From: Korba, Przemyslaw @ 2026-03-11 12:42 UTC (permalink / raw)
  To: Simon Horman
  Cc: intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
	Nguyen, Anthony L, Kitszel, Przemyslaw, Keller, Jacob E

> -----Original Message-----
> From: Simon Horman <horms@kernel.org>
> Sent: Tuesday, March 10, 2026 7:25 PM
> To: Korba, Przemyslaw <przemyslaw.korba@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Keller, Jacob E <jacob.e.keller@intel.com>
> Subject: Re: [PATCH iwl-next] i40e: PTP: set supported flags in ptp_clock_info
> 
> + Jacob
> 
> On Mon, Mar 09, 2026 at 03:11:51PM +0100, Przemyslaw Korba wrote:
> > Since upstream commit d9f3e9ecc456 ("net: ptp: introduce
> > .supported_perout_flags to ptp_clock_info") and commit 7c571ac57d9d ("net:
> > ptp: introduce .supported_extts_flags to ptp_clock_info"), kernel core
> > now requires that the driver set the .supported_perout_flags and
> > .supported_extts_flags fields in PTP clock info. Otherwise, the
> > additional flags will be rejected by the kernel automatically.
> >
> > i40e does not support perout flags, so reject any request with perout
> > flags.
> >
> > Signed-off-by: Przemyslaw Korba <przemyslaw.korba@intel.com>
> > ---
> >  drivers/net/ethernet/intel/i40e/i40e_ptp.c | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > index 7bcea7d9720f..8d7958692235 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > @@ -601,10 +601,18 @@ static int i40e_ptp_feature_enable(struct ptp_clock_info *ptp,
> >  	/* TODO: Implement flags handling for EXTTS and PEROUT */
> >  	switch (rq->type) {
> >  	case PTP_CLK_REQ_EXTTS:
> > +		if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
> > +					PTP_RISING_EDGE |
> > +					PTP_FALLING_EDGE |
> > +					PTP_STRICT_FLAGS))
> > +			return -EOPNOTSUPP;
> > +
> >  		func = PTP_PF_EXTTS;
> >  		chan = rq->extts.index;
> >  		break;
> >  	case PTP_CLK_REQ_PEROUT:
> > +		if (rq->perout.flags)
> > +			return -EOPNOTSUPP;
> >  		func = PTP_PF_PEROUT;
> >  		chan = rq->perout.index;
> >  		break;
> 
> I am a little confused.
> 
> My understanding of the cited patches is that they add checking of flags to the code. So code like the above isn't needed in drivers.
 
Hi Simon, thank you very much for the review. My understanding is that the driver needs to set the supported flags field, otherwise requests won't go through kernel. The test I've been doing confirm my theory. Here's also example patch, that adds supported flags to drivers: https://lore.kernel.org/intel-wired-lan/20250414-jk-supported-perout-flags-v2-1-f6b17d15475c@intel.com/

> > @@ -1340,7 +1348,9 @@ static int i40e_init_pin_config(struct i40e_pf *pf)
> >  	pf->ptp_caps.n_ext_ts = 2;
> >  	pf->ptp_caps.pps = 1;
> >  	pf->ptp_caps.n_per_out = 2;
> > -
> > +	pf->ptp_caps.supported_extts_flags = PTP_RISING_EDGE |
> > +					     PTP_FALLING_EDGE |
> > +					     PTP_STRICT_FLAGS;
> >  	pf->ptp_caps.pin_config = kzalloc_objs(*pf->ptp_caps.pin_config,
> >  					       pf->ptp_caps.n_pins);
> >  	if (!pf->ptp_caps.pin_config)
> >
> > base-commit: d5fbc991435eac7a1ead7cd2ddb5a743528718bb
> > --
> > 2.43.0
> >

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [Intel-wired-lan] [PATCH iwl-next] i40e: PTP: set supported flags in ptp_clock_info
  2026-03-09 16:34 ` [Intel-wired-lan] " Paul Menzel
@ 2026-03-11 12:42   ` Korba, Przemyslaw
  0 siblings, 0 replies; 10+ messages in thread
From: Korba, Przemyslaw @ 2026-03-11 12:42 UTC (permalink / raw)
  To: Paul Menzel
  Cc: intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
	Nguyen, Anthony L, Kitszel, Przemyslaw




> -----Original Message-----
> From: Paul Menzel <pmenzel@molgen.mpg.de>
> Sent: Monday, March 9, 2026 5:34 PM
> To: Korba, Przemyslaw <przemyslaw.korba@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>
> Subject: Re: [Intel-wired-lan] [PATCH iwl-next] i40e: PTP: set supported flags in ptp_clock_info
> 
> Dear Przemyslaw,
> 
> 
> Thank you for your patch.
> 
> Am 09.03.26 um 15:11 schrieb Przemyslaw Korba:
> > Since upstream commit d9f3e9ecc456 ("net: ptp: introduce
> > .supported_perout_flags to ptp_clock_info") and commit 7c571ac57d9d ("net:
> > ptp: introduce .supported_extts_flags to ptp_clock_info"), kernel core
> > now requires that the driver set the .supported_perout_flags and
> > .supported_extts_flags fields in PTP clock info. Otherwise, the
> > additional flags will be rejected by the kernel automatically.
> >
> > i40e does not support perout flags, so reject any request with perout
> > flags.
> 
> As you reference commits, why not add Fixes: tags?
> 

Thank you for the review! Good point, will send v2

> > Signed-off-by: Przemyslaw Korba <przemyslaw.korba@intel.com>
> > ---
> >   drivers/net/ethernet/intel/i40e/i40e_ptp.c | 12 +++++++++++-
> >   1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > index 7bcea7d9720f..8d7958692235 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > @@ -601,10 +601,18 @@ static int i40e_ptp_feature_enable(struct ptp_clock_info *ptp,
> >   	/* TODO: Implement flags handling for EXTTS and PEROUT */
> >   	switch (rq->type) {
> >   	case PTP_CLK_REQ_EXTTS:
> > +		if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
> > +					PTP_RISING_EDGE |
> > +					PTP_FALLING_EDGE |
> > +					PTP_STRICT_FLAGS))
> > +			return -EOPNOTSUPP;
> > +
> >   		func = PTP_PF_EXTTS;
> >   		chan = rq->extts.index;
> >   		break;
> >   	case PTP_CLK_REQ_PEROUT:
> > +		if (rq->perout.flags)
> > +			return -EOPNOTSUPP;
> >   		func = PTP_PF_PEROUT;
> >   		chan = rq->perout.index;
> >   		break;
> > @@ -1340,7 +1348,9 @@ static int i40e_init_pin_config(struct i40e_pf *pf)
> >   	pf->ptp_caps.n_ext_ts = 2;
> >   	pf->ptp_caps.pps = 1;
> >   	pf->ptp_caps.n_per_out = 2;
> > -
> > +	pf->ptp_caps.supported_extts_flags = PTP_RISING_EDGE |
> > +					     PTP_FALLING_EDGE |
> > +					     PTP_STRICT_FLAGS;
> >   	pf->ptp_caps.pin_config = kzalloc_objs(*pf->ptp_caps.pin_config,
> >   					       pf->ptp_caps.n_pins);
> >   	if (!pf->ptp_caps.pin_config)
> 
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
> 
> 
> Kind regards,
> 
> Paul

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [Intel-wired-lan] [PATCH iwl-next] i40e: PTP: set supported flags in ptp_clock_info
  2026-03-11  8:01 ` [Intel-wired-lan] " Dawid Osuchowski
@ 2026-03-11 12:45   ` Korba, Przemyslaw
  0 siblings, 0 replies; 10+ messages in thread
From: Korba, Przemyslaw @ 2026-03-11 12:45 UTC (permalink / raw)
  To: Dawid Osuchowski, intel-wired-lan@lists.osuosl.org
  Cc: netdev@vger.kernel.org, Nguyen, Anthony L, Kitszel, Przemyslaw


> -----Original Message-----
> From: Dawid Osuchowski <dawid.osuchowski@linux.intel.com>
> Sent: Wednesday, March 11, 2026 9:02 AM
> To: Korba, Przemyslaw <przemyslaw.korba@intel.com>; intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>
> Subject: Re: [Intel-wired-lan] [PATCH iwl-next] i40e: PTP: set supported flags in ptp_clock_info
> 
> On 2026-03-09 3:11 PM, Przemyslaw Korba wrote:
> > Since upstream commit d9f3e9ecc456 ("net: ptp: introduce
> > .supported_perout_flags to ptp_clock_info") and commit 7c571ac57d9d ("net:
> > ptp: introduce .supported_extts_flags to ptp_clock_info"), kernel core now
> > requires that the driver set the .supported_perout_flags and
> > .supported_extts_flags fields in PTP clock info. Otherwise, the additional
> > flags will be rejected by the kernel automatically.
> >
> > i40e does not support perout flags, so reject any request with perout
> > flags.
> >
> > Signed-off-by: Przemyslaw Korba <przemyslaw.korba@intel.com>
> 
> Hey Przemek,
> 
> **Please don't circumvent our established process of going through
> internal review before posting to iwl**. You should send to our internal
> mailing list and get at least one Reviewed-by tag first, before posting
> to iwl. I think we make exceptions only when there is a strict time
> constraint and need to get the change out FAST (e.g. bug present in
> Tony's tree or netdev but not in Linus' tree yet). Reach out internally
> if you don't know the process and I can share the proper resources.
> 

Hi, thank you for the review! Yes, I've been a bit too quick - got internal review, but did not receive reviewed-by tag. Will keep that in mind next time.

> > ---
> >   drivers/net/ethernet/intel/i40e/i40e_ptp.c | 12 +++++++++++-
> >   1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > index 7bcea7d9720f..8d7958692235 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > @@ -601,10 +601,18 @@ static int i40e_ptp_feature_enable(struct ptp_clock_info *ptp,
> >   	/* TODO: Implement flags handling for EXTTS and PEROUT */
> 
> Ignoring Simon's comment (not because it doesn't have merit, but because
> I don't know this part of the driver and the referenced patches), given
> you just seem to have implemented setting (or maybe handling) these
> flags in this commit, might be the time to remove this TODO comment? :)
> 

Hi, thank you for the review! Yes, I missed it - thank you

> -Dawid

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH iwl-next] i40e: PTP: set supported flags in ptp_clock_info
  2026-03-11 12:42   ` Korba, Przemyslaw
@ 2026-03-13 13:34     ` Simon Horman
  2026-03-13 13:47       ` Korba, Przemyslaw
  0 siblings, 1 reply; 10+ messages in thread
From: Simon Horman @ 2026-03-13 13:34 UTC (permalink / raw)
  To: Korba, Przemyslaw
  Cc: intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
	Nguyen, Anthony L, Kitszel, Przemyslaw, Keller, Jacob E

On Wed, Mar 11, 2026 at 12:42:10PM +0000, Korba, Przemyslaw wrote:
> > -----Original Message-----
> > From: Simon Horman <horms@kernel.org>
> > Sent: Tuesday, March 10, 2026 7:25 PM
> > To: Korba, Przemyslaw <przemyslaw.korba@intel.com>
> > Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> > <przemyslaw.kitszel@intel.com>; Keller, Jacob E <jacob.e.keller@intel.com>
> > Subject: Re: [PATCH iwl-next] i40e: PTP: set supported flags in ptp_clock_info
> > 
> > + Jacob
> > 
> > On Mon, Mar 09, 2026 at 03:11:51PM +0100, Przemyslaw Korba wrote:
> > > Since upstream commit d9f3e9ecc456 ("net: ptp: introduce
> > > .supported_perout_flags to ptp_clock_info") and commit 7c571ac57d9d ("net:
> > > ptp: introduce .supported_extts_flags to ptp_clock_info"), kernel core
> > > now requires that the driver set the .supported_perout_flags and
> > > .supported_extts_flags fields in PTP clock info. Otherwise, the
> > > additional flags will be rejected by the kernel automatically.
> > >
> > > i40e does not support perout flags, so reject any request with perout
> > > flags.
> > >
> > > Signed-off-by: Przemyslaw Korba <przemyslaw.korba@intel.com>
> > > ---
> > >  drivers/net/ethernet/intel/i40e/i40e_ptp.c | 12 +++++++++++-
> > >  1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > > b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > > index 7bcea7d9720f..8d7958692235 100644
> > > --- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > > +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > > @@ -601,10 +601,18 @@ static int i40e_ptp_feature_enable(struct ptp_clock_info *ptp,
> > >  	/* TODO: Implement flags handling for EXTTS and PEROUT */
> > >  	switch (rq->type) {
> > >  	case PTP_CLK_REQ_EXTTS:
> > > +		if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
> > > +					PTP_RISING_EDGE |
> > > +					PTP_FALLING_EDGE |
> > > +					PTP_STRICT_FLAGS))
> > > +			return -EOPNOTSUPP;
> > > +
> > >  		func = PTP_PF_EXTTS;
> > >  		chan = rq->extts.index;
> > >  		break;
> > >  	case PTP_CLK_REQ_PEROUT:
> > > +		if (rq->perout.flags)
> > > +			return -EOPNOTSUPP;
> > >  		func = PTP_PF_PEROUT;
> > >  		chan = rq->perout.index;
> > >  		break;
> > 
> > I am a little confused.
> > 
> > My understanding of the cited patches is that they add checking of flags to the code. So code like the above isn't needed in drivers.
>  
> Hi Simon, thank you very much for the review. My understanding is that the driver needs to set the supported flags field, otherwise requests won't go through kernel. The test I've been doing confirm my theory. Here's also example patch, that adds supported flags to drivers: https://lore.kernel.org/intel-wired-lan/20250414-jk-supported-perout-flags-v2-1-f6b17d15475c@intel.com/

Sorry for the slow response.

My understanding is that the hunk above is not required.
But the hunk below is.

> 
> > > @@ -1340,7 +1348,9 @@ static int i40e_init_pin_config(struct i40e_pf *pf)
> > >  	pf->ptp_caps.n_ext_ts = 2;
> > >  	pf->ptp_caps.pps = 1;
> > >  	pf->ptp_caps.n_per_out = 2;
> > > -
> > > +	pf->ptp_caps.supported_extts_flags = PTP_RISING_EDGE |
> > > +					     PTP_FALLING_EDGE |
> > > +					     PTP_STRICT_FLAGS;
> > >  	pf->ptp_caps.pin_config = kzalloc_objs(*pf->ptp_caps.pin_config,
> > >  					       pf->ptp_caps.n_pins);
> > >  	if (!pf->ptp_caps.pin_config)
> > >
> > > base-commit: d5fbc991435eac7a1ead7cd2ddb5a743528718bb
> > > --
> > > 2.43.0
> > >

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH iwl-next] i40e: PTP: set supported flags in ptp_clock_info
  2026-03-13 13:34     ` Simon Horman
@ 2026-03-13 13:47       ` Korba, Przemyslaw
  2026-03-26  0:15         ` Jacob Keller
  0 siblings, 1 reply; 10+ messages in thread
From: Korba, Przemyslaw @ 2026-03-13 13:47 UTC (permalink / raw)
  To: Simon Horman
  Cc: intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
	Nguyen, Anthony L, Kitszel, Przemyslaw, Keller, Jacob E

> -----Original Message-----
> From: Simon Horman <horms@kernel.org>
> Sent: Friday, March 13, 2026 2:35 PM
> To: Korba, Przemyslaw <przemyslaw.korba@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Keller, Jacob E <jacob.e.keller@intel.com>
> Subject: Re: [PATCH iwl-next] i40e: PTP: set supported flags in ptp_clock_info
> 
> On Wed, Mar 11, 2026 at 12:42:10PM +0000, Korba, Przemyslaw wrote:
> > > -----Original Message-----
> > > From: Simon Horman <horms@kernel.org>
> > > Sent: Tuesday, March 10, 2026 7:25 PM
> > > To: Korba, Przemyslaw <przemyslaw.korba@intel.com>
> > > Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> > > <przemyslaw.kitszel@intel.com>; Keller, Jacob E <jacob.e.keller@intel.com>
> > > Subject: Re: [PATCH iwl-next] i40e: PTP: set supported flags in ptp_clock_info
> > >
> > > + Jacob
> > >
> > > On Mon, Mar 09, 2026 at 03:11:51PM +0100, Przemyslaw Korba wrote:
> > > > Since upstream commit d9f3e9ecc456 ("net: ptp: introduce
> > > > .supported_perout_flags to ptp_clock_info") and commit 7c571ac57d9d ("net:
> > > > ptp: introduce .supported_extts_flags to ptp_clock_info"), kernel core
> > > > now requires that the driver set the .supported_perout_flags and
> > > > .supported_extts_flags fields in PTP clock info. Otherwise, the
> > > > additional flags will be rejected by the kernel automatically.
> > > >
> > > > i40e does not support perout flags, so reject any request with perout
> > > > flags.
> > > >
> > > > Signed-off-by: Przemyslaw Korba <przemyslaw.korba@intel.com>
> > > > ---
> > > >  drivers/net/ethernet/intel/i40e/i40e_ptp.c | 12 +++++++++++-
> > > >  1 file changed, 11 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > > > b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > > > index 7bcea7d9720f..8d7958692235 100644
> > > > --- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > > > +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > > > @@ -601,10 +601,18 @@ static int i40e_ptp_feature_enable(struct ptp_clock_info *ptp,
> > > >  	/* TODO: Implement flags handling for EXTTS and PEROUT */
> > > >  	switch (rq->type) {
> > > >  	case PTP_CLK_REQ_EXTTS:
> > > > +		if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
> > > > +					PTP_RISING_EDGE |
> > > > +					PTP_FALLING_EDGE |
> > > > +					PTP_STRICT_FLAGS))
> > > > +			return -EOPNOTSUPP;
> > > > +
> > > >  		func = PTP_PF_EXTTS;
> > > >  		chan = rq->extts.index;
> > > >  		break;
> > > >  	case PTP_CLK_REQ_PEROUT:
> > > > +		if (rq->perout.flags)
> > > > +			return -EOPNOTSUPP;
> > > >  		func = PTP_PF_PEROUT;
> > > >  		chan = rq->perout.index;
> > > >  		break;
> > >
> > > I am a little confused.
> > >
> > > My understanding of the cited patches is that they add checking of flags to the code. So code like the above isn't needed in drivers.
> >
> > Hi Simon, thank you very much for the review. My understanding is that the driver needs to set the supported flags field, otherwise requests
> won't go through kernel. The test I've been doing confirm my theory. Here's also example patch, that adds supported flags to drivers:
> https://lore.kernel.org/intel-wired-lan/20250414-jk-supported-perout-flags-v2-1-f6b17d15475c@intel.com/
> 
> Sorry for the slow response.
> 
> My understanding is that the hunk above is not required.
> But the hunk below is.
> 

Well, you are very correct. Thank you so much for thorough review and let me send a new version!

> >
> > > > @@ -1340,7 +1348,9 @@ static int i40e_init_pin_config(struct i40e_pf *pf)
> > > >  	pf->ptp_caps.n_ext_ts = 2;
> > > >  	pf->ptp_caps.pps = 1;
> > > >  	pf->ptp_caps.n_per_out = 2;
> > > > -
> > > > +	pf->ptp_caps.supported_extts_flags = PTP_RISING_EDGE |
> > > > +					     PTP_FALLING_EDGE |
> > > > +					     PTP_STRICT_FLAGS;
> > > >  	pf->ptp_caps.pin_config = kzalloc_objs(*pf->ptp_caps.pin_config,
> > > >  					       pf->ptp_caps.n_pins);
> > > >  	if (!pf->ptp_caps.pin_config)
> > > >
> > > > base-commit: d5fbc991435eac7a1ead7cd2ddb5a743528718bb
> > > > --
> > > > 2.43.0
> > > >

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH iwl-next] i40e: PTP: set supported flags in ptp_clock_info
  2026-03-13 13:47       ` Korba, Przemyslaw
@ 2026-03-26  0:15         ` Jacob Keller
  0 siblings, 0 replies; 10+ messages in thread
From: Jacob Keller @ 2026-03-26  0:15 UTC (permalink / raw)
  To: Korba, Przemyslaw, Simon Horman
  Cc: intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
	Nguyen, Anthony L, Kitszel, Przemyslaw

On 3/13/2026 6:47 AM, Korba, Przemyslaw wrote:
>> -----Original Message-----
>> From: Simon Horman <horms@kernel.org>
>> Sent: Friday, March 13, 2026 2:35 PM
>> To: Korba, Przemyslaw <przemyslaw.korba@intel.com>
>> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
>> <przemyslaw.kitszel@intel.com>; Keller, Jacob E <jacob.e.keller@intel.com>
>> Subject: Re: [PATCH iwl-next] i40e: PTP: set supported flags in ptp_clock_info
>>
>> On Wed, Mar 11, 2026 at 12:42:10PM +0000, Korba, Przemyslaw wrote:
>>>> -----Original Message-----
>>>> From: Simon Horman <horms@kernel.org>
>>>> Sent: Tuesday, March 10, 2026 7:25 PM
>>>> To: Korba, Przemyslaw <przemyslaw.korba@intel.com>
>>>> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
>>>> <przemyslaw.kitszel@intel.com>; Keller, Jacob E <jacob.e.keller@intel.com>
>>>> Subject: Re: [PATCH iwl-next] i40e: PTP: set supported flags in ptp_clock_info
>>>>
>>>> + Jacob
>>>>
>>>> On Mon, Mar 09, 2026 at 03:11:51PM +0100, Przemyslaw Korba wrote:
>>>>> Since upstream commit d9f3e9ecc456 ("net: ptp: introduce
>>>>> .supported_perout_flags to ptp_clock_info") and commit 7c571ac57d9d ("net:
>>>>> ptp: introduce .supported_extts_flags to ptp_clock_info"), kernel core
>>>>> now requires that the driver set the .supported_perout_flags and
>>>>> .supported_extts_flags fields in PTP clock info. Otherwise, the
>>>>> additional flags will be rejected by the kernel automatically.
>>>>>
>>>>> i40e does not support perout flags, so reject any request with perout
>>>>> flags.
>>>>>
>>>>> Signed-off-by: Przemyslaw Korba <przemyslaw.korba@intel.com>
>>>>> ---
>>>>>  drivers/net/ethernet/intel/i40e/i40e_ptp.c | 12 +++++++++++-
>>>>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
>>>>> b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
>>>>> index 7bcea7d9720f..8d7958692235 100644
>>>>> --- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
>>>>> +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
>>>>> @@ -601,10 +601,18 @@ static int i40e_ptp_feature_enable(struct ptp_clock_info *ptp,
>>>>>  	/* TODO: Implement flags handling for EXTTS and PEROUT */
>>>>>  	switch (rq->type) {
>>>>>  	case PTP_CLK_REQ_EXTTS:
>>>>> +		if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
>>>>> +					PTP_RISING_EDGE |
>>>>> +					PTP_FALLING_EDGE |
>>>>> +					PTP_STRICT_FLAGS))
>>>>> +			return -EOPNOTSUPP;
>>>>> +
>>>>>  		func = PTP_PF_EXTTS;
>>>>>  		chan = rq->extts.index;
>>>>>  		break;
>>>>>  	case PTP_CLK_REQ_PEROUT:
>>>>> +		if (rq->perout.flags)
>>>>> +			return -EOPNOTSUPP;
>>>>>  		func = PTP_PF_PEROUT;
>>>>>  		chan = rq->perout.index;
>>>>>  		break;
>>>>
>>>> I am a little confused.
>>>>
>>>> My understanding of the cited patches is that they add checking of flags to the code. So code like the above isn't needed in drivers.
>>>
>>> Hi Simon, thank you very much for the review. My understanding is that the driver needs to set the supported flags field, otherwise requests
>> won't go through kernel. The test I've been doing confirm my theory. Here's also example patch, that adds supported flags to drivers:
>> https://lore.kernel.org/intel-wired-lan/20250414-jk-supported-perout-flags-v2-1-f6b17d15475c@intel.com/
>>
>> Sorry for the slow response.
>>
>> My understanding is that the hunk above is not required.
>> But the hunk below is.
>>
> 
> Well, you are very correct. Thank you so much for thorough review and let me send a new version!
> 
Yes, Simon is correct, but we do have to be certain that the driver
actually implements the facts correctly, i.e. that it will actually
honor the RISING or FALLING edge, before you actually add the flags to
the supported flags list.

I don't see any mention of PTP_RISING_EDGE nor PTP_FALLING_EDGE in the
driver. Thus, I can't confirm which edge is actually timestamped.

Thus I would NACK this patch until you can confirm whether the hardware
either a) timestamps one edge, in which case you should set only that
flag as allowed, b) timestamps both edges, in which case you should set
all flags and then explicitly reject the case where only one flag is
set, or c) can be configured based on which flag is set, in which case
you should set all the flags and then check the flags when programming
to enable the appropriate edge.

This patch does none of these, and is therefor incorrect. Applying it
will "allow" the userspace to work but they will not get the strict
behavior of timestamping the desired edge, which completely negates the
point of the strict mode!

As an example, look at the ice driver:

#define GLTSYN_AUX_IN_0_EVNTLVL_RISING_EDGE     BIT(0)
#define GLTSYN_AUX_IN_0_EVNTLVL_FALLING_EDGE    BIT(1)

                /* set event level to requested edge */
                if (rq->flags & PTP_FALLING_EDGE)
                        aux_reg |= GLTSYN_AUX_IN_0_EVNTLVL_FALLING_EDGE;
                if (rq->flags & PTP_RISING_EDGE)
                        aux_reg |= GLTSYN_AUX_IN_0_EVNTLVL_RISING_EDGE;


It sets the appropriate register values to ensure the correct edges are
timestamped as requested.

Thanks,
Jake


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-03-26  0:15 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-09 14:11 [PATCH iwl-next] i40e: PTP: set supported flags in ptp_clock_info Przemyslaw Korba
2026-03-09 16:34 ` [Intel-wired-lan] " Paul Menzel
2026-03-11 12:42   ` Korba, Przemyslaw
2026-03-10 18:24 ` Simon Horman
2026-03-11 12:42   ` Korba, Przemyslaw
2026-03-13 13:34     ` Simon Horman
2026-03-13 13:47       ` Korba, Przemyslaw
2026-03-26  0:15         ` Jacob Keller
2026-03-11  8:01 ` [Intel-wired-lan] " Dawid Osuchowski
2026-03-11 12:45   ` Korba, Przemyslaw

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox