All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Horman <horms@kernel.org>
To: Karol Kolacinski <karol.kolacinski@intel.com>
Cc: Jacob Keller <jacob.e.keller@intel.com>,
	netdev@vger.kernel.org, anthony.l.nguyen@intel.com,
	intel-wired-lan@lists.osuosl.org, jesse.brandeburg@intel.com
Subject: Re: [Intel-wired-lan] [PATCH v4 iwl-next 2/6] ice: pass reset type to PTP reset functions
Date: Sat, 23 Dec 2023 18:22:13 +0000	[thread overview]
Message-ID: <20231223182213.GK201037@kernel.org> (raw)
In-Reply-To: <20231221100326.1030761-3-karol.kolacinski@intel.com>

On Thu, Dec 21, 2023 at 11:03:22AM +0100, Karol Kolacinski wrote:
> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> The ice_ptp_prepare_for_reset() and ice_ptp_reset() functions currently
> check the pf->flags ICE_FLAG_PFR_REQ bit to determine if the current
> reset is a PF reset or not.
> 
> This is problematic, because it is possible that a PF reset and a higher
> level reset (CORE reset, GLOBAL reset, EMP reset) are requested
> simultaneously. In that case, the driver performs the highest level
> reset requested. However, the ICE_FLAG_PFR_REQ flag will still be set.
> 
> The main driver reset functions take an enum ice_reset_req indicating
> which reset is actually being performed. Pass this data into the PTP
> functions and rely on this instead of relying on the driver flags.
> 
> This ensures that the PTP code performs the proper level of reset that
> the driver is actually undergoing.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

...

> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h b/drivers/net/ethernet/intel/ice/ice_ptp.h
> index 2457380142e1..bbac053bd099 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp.h
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.h
> @@ -314,8 +314,8 @@ enum ice_tx_tstamp_work ice_ptp_process_ts(struct ice_pf *pf);
>  
>  u64 ice_ptp_get_rx_hwts(const union ice_32b_rx_flex_desc *rx_desc,
>  			const struct ice_pkt_ctx *pkt_ctx);
> -void ice_ptp_reset(struct ice_pf *pf);

Hi Karol and Jacob,

I think that the declaration of ice_ptp_reset() is
needed for the case where CONFIG_PTP_1588_CLOCK=y
until patch 5/6 of this series.

> -void ice_ptp_prepare_for_reset(struct ice_pf *pf);
> +void ice_ptp_prepare_for_reset(struct ice_pf *pf,
> +			       enum ice_reset_req reset_type);
>  void ice_ptp_init(struct ice_pf *pf);
>  void ice_ptp_release(struct ice_pf *pf);
>  void ice_ptp_link_change(struct ice_pf *pf, u8 port, bool linkup);

...
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

WARNING: multiple messages have this Message-ID (diff)
From: Simon Horman <horms@kernel.org>
To: Karol Kolacinski <karol.kolacinski@intel.com>
Cc: intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
	anthony.l.nguyen@intel.com, jesse.brandeburg@intel.com,
	Jacob Keller <jacob.e.keller@intel.com>
Subject: Re: [PATCH v4 iwl-next 2/6] ice: pass reset type to PTP reset functions
Date: Sat, 23 Dec 2023 18:22:13 +0000	[thread overview]
Message-ID: <20231223182213.GK201037@kernel.org> (raw)
In-Reply-To: <20231221100326.1030761-3-karol.kolacinski@intel.com>

On Thu, Dec 21, 2023 at 11:03:22AM +0100, Karol Kolacinski wrote:
> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> The ice_ptp_prepare_for_reset() and ice_ptp_reset() functions currently
> check the pf->flags ICE_FLAG_PFR_REQ bit to determine if the current
> reset is a PF reset or not.
> 
> This is problematic, because it is possible that a PF reset and a higher
> level reset (CORE reset, GLOBAL reset, EMP reset) are requested
> simultaneously. In that case, the driver performs the highest level
> reset requested. However, the ICE_FLAG_PFR_REQ flag will still be set.
> 
> The main driver reset functions take an enum ice_reset_req indicating
> which reset is actually being performed. Pass this data into the PTP
> functions and rely on this instead of relying on the driver flags.
> 
> This ensures that the PTP code performs the proper level of reset that
> the driver is actually undergoing.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

...

> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h b/drivers/net/ethernet/intel/ice/ice_ptp.h
> index 2457380142e1..bbac053bd099 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp.h
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.h
> @@ -314,8 +314,8 @@ enum ice_tx_tstamp_work ice_ptp_process_ts(struct ice_pf *pf);
>  
>  u64 ice_ptp_get_rx_hwts(const union ice_32b_rx_flex_desc *rx_desc,
>  			const struct ice_pkt_ctx *pkt_ctx);
> -void ice_ptp_reset(struct ice_pf *pf);

Hi Karol and Jacob,

I think that the declaration of ice_ptp_reset() is
needed for the case where CONFIG_PTP_1588_CLOCK=y
until patch 5/6 of this series.

> -void ice_ptp_prepare_for_reset(struct ice_pf *pf);
> +void ice_ptp_prepare_for_reset(struct ice_pf *pf,
> +			       enum ice_reset_req reset_type);
>  void ice_ptp_init(struct ice_pf *pf);
>  void ice_ptp_release(struct ice_pf *pf);
>  void ice_ptp_link_change(struct ice_pf *pf, u8 port, bool linkup);

...

  reply	other threads:[~2023-12-23 18:22 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-21 10:03 [Intel-wired-lan] [PATCH v4 iwl-next 0/6] ice: fix timestamping in reset process Karol Kolacinski
2023-12-21 10:03 ` Karol Kolacinski
2023-12-21 10:03 ` [Intel-wired-lan] [PATCH v4 iwl-next 1/6] ice: introduce PTP state machine Karol Kolacinski
2023-12-21 10:03   ` Karol Kolacinski
2023-12-21 10:03 ` [Intel-wired-lan] [PATCH v4 iwl-next 2/6] ice: pass reset type to PTP reset functions Karol Kolacinski
2023-12-21 10:03   ` Karol Kolacinski
2023-12-23 18:22   ` Simon Horman [this message]
2023-12-23 18:22     ` Simon Horman
2023-12-21 10:03 ` [Intel-wired-lan] [PATCH v4 iwl-next 3/6] ice: rename verify_cached to has_ready_bitmap Karol Kolacinski
2023-12-21 10:03   ` Karol Kolacinski
2023-12-21 10:03 ` [Intel-wired-lan] [PATCH v4 iwl-next 4/6] ice: rename ice_ptp_tx_cfg_intr Karol Kolacinski
2023-12-21 10:03   ` Karol Kolacinski
2023-12-21 10:03 ` [Intel-wired-lan] [PATCH v4 iwl-next 5/6] ice: factor out ice_ptp_rebuild_owner() Karol Kolacinski
2023-12-21 10:03   ` Karol Kolacinski
2023-12-21 10:20   ` [Intel-wired-lan] " Sokolowski, Jan
2023-12-21 10:20     ` Sokolowski, Jan
2023-12-21 23:54     ` [Intel-wired-lan] " Keller, Jacob E
2023-12-21 23:54       ` Keller, Jacob E
2023-12-21 10:03 ` [Intel-wired-lan] [PATCH v4 iwl-next 6/6] ice: stop destroying and reinitalizing Tx tracker during reset Karol Kolacinski
2023-12-21 10:03   ` Karol Kolacinski
2023-12-21 18:20 ` [Intel-wired-lan] [PATCH v4 iwl-next 0/6] ice: fix timestamping in reset process Brett Creeley
2023-12-21 18:20   ` Brett Creeley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231223182213.GK201037@kernel.org \
    --to=horms@kernel.org \
    --cc=anthony.l.nguyen@intel.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=jacob.e.keller@intel.com \
    --cc=jesse.brandeburg@intel.com \
    --cc=karol.kolacinski@intel.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.