On Tue, Jun 18, 2024 at 12:41:36PM +0200, Karol Kolacinski wrote: > From: Milena Olech <milena.ol...@intel.com> > > Extts events are disabled and enabled by the application ts2phc. > However, in case where the driver is removed when the application is > running, channel remains enabled. As a result, in the next run of the > app, two channels are enabled and the information "extts on unexpected > channel" is printed to the user. > > To avoid that, extts events shall be disabled when PTP is released. > > Fixes: 172db5f91d5f ("ice: add support for auxiliary input/output pins") > Reviewed-by: Przemek Kitszel <przemyslaw.kits...@intel.com> > Co-developed-by: Jacob Keller <jacob.e.kel...@intel.com> > Signed-off-by: Jacob Keller <jacob.e.kel...@intel.com> > Signed-off-by: Milena Olech <milena.ol...@intel.com> > Signed-off-by: Karol Kolacinski <karol.kolacin...@intel.com>
Hi Milena and Karol, Some feedback from my side. ... > diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c > b/drivers/net/ethernet/intel/ice/ice_ptp.c > index 0f17fc1181d2..30f1f910e6d9 100644 > --- a/drivers/net/ethernet/intel/ice/ice_ptp.c > +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c > @@ -1584,27 +1584,24 @@ void ice_ptp_extts_event(struct ice_pf *pf) > /** > * ice_ptp_cfg_extts - Configure EXTTS pin and channel > * @pf: Board private structure > - * @ena: true to enable; false to disable > * @chan: GPIO channel (0-3) > - * @gpio_pin: GPIO pin > - * @extts_flags: request flags from the ptp_extts_request.flags > - */ > -static int > -ice_ptp_cfg_extts(struct ice_pf *pf, bool ena, unsigned int chan, u32 > gpio_pin, > - unsigned int extts_flags) > + * @config: desired EXTTS configuration. > + * @store: If set to true, the values will be stored > + * > + * Configure an external timestamp event on the requested channel. > + */ nit: There is an extra leading space on the line above. Also, although not strictly related to this change, please consider adding a Returns: section to this kernel doc. > +static void ice_ptp_cfg_extts(struct ice_pf *pf, unsigned int chan, > + struct ice_extts_channel *config, bool store) ... > @@ -1869,21 +1915,31 @@ static int ice_ptp_gpio_enable_e823(struct > ptp_clock_info *info, > struct ptp_clock_request *rq, int on) > { > struct ice_pf *pf = ptp_info_to_pf(info); > - struct ice_perout_channel clk_cfg = {0}; > int err; > > switch (rq->type) { > case PTP_CLK_REQ_PPS: > + { > + struct ice_perout_channel clk_cfg = {}; > + > clk_cfg.gpio_pin = PPS_PIN_INDEX; > clk_cfg.period = NSEC_PER_SEC; > clk_cfg.ena = !!on; > > err = ice_ptp_cfg_clkout(pf, PPS_CLK_GEN_CHAN, &clk_cfg, true); > break; > + } > case PTP_CLK_REQ_EXTTS: > - err = ice_ptp_cfg_extts(pf, !!on, rq->extts.index, > - TIME_SYNC_PIN_INDEX, rq->extts.flags); > + { > + struct ice_extts_channel extts_cfg = {}; > + > + extts_cfg.flags = rq->extts.flags; > + extts_cfg.gpio_pin = TIME_SYNC_PIN_INDEX; > + extts_cfg.ena = !!on; > + > + ice_ptp_cfg_extts(pf, rq->extts.index, &extts_cfg, true); > break; This function returns err. But with this patch err is uninitialised here. Perhaps err be set to the return value of ice_ptp_cfg_extts() as it was before this patch? Flagged by allmodconfig W=1 builds with gcc-13 and clang-18, and Smatch. > + } > default: > return -EOPNOTSUPP; > } ...