> -----Original Message----- > From: Simon Horman <[email protected]> > Sent: Friday, March 13, 2026 2:35 PM > To: Korba, Przemyslaw <[email protected]> > Cc: [email protected]; [email protected]; Nguyen, Anthony > L <[email protected]>; Kitszel, Przemyslaw > <[email protected]>; Keller, Jacob E <[email protected]> > 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 <[email protected]> > > > Sent: Tuesday, March 10, 2026 7:25 PM > > > To: Korba, Przemyslaw <[email protected]> > > > Cc: [email protected]; [email protected]; Nguyen, > > > Anthony L <[email protected]>; Kitszel, Przemyslaw > > > <[email protected]>; Keller, Jacob E <[email protected]> > > > 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 <[email protected]> > > > > --- > > > > 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/[email protected]/ > > 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 > > > >
