[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <59a690bc-50c6-df52-5686-ae6b231fd0f3@intel.com>
Date: Wed, 3 Apr 2024 16:25:53 -0700
From: Tony Nguyen <anthony.l.nguyen@...el.com>
To: Karol Kolacinski <karol.kolacinski@...el.com>,
<intel-wired-lan@...ts.osuosl.org>
CC: <netdev@...r.kernel.org>, <jesse.brandeburg@...el.com>, Jacob Keller
<jacob.e.keller@...el.com>, Przemek Kitszel <przemyslaw.kitszel@...el.com>,
Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>
Subject: Re: [PATCH v4 iwl-next 02/12] ice: Introduce helper to get
tmr_cmd_reg values
On 3/29/2024 9:09 AM, Karol Kolacinski wrote:
> From: Jacob Keller <jacob.e.keller@...el.com>
>
> Multiple places in the driver code need to convert enum ice_ptp_tmr_cmd
> values into register bits for both the main timer and the PHY port
> timers. The main MAC register has one bit scheme for timer commands,
> while the PHY commands use a different scheme.
>
> The E810 and E830 devices use the same scheme for port commands as used
> for the main timer. However, E822 and ETH56G hardware has a separate
> scheme used by the PHY.
>
> Introduce helper functions to convert the timer command enumeration into
> the register values, reducing some code duplication, and making it
> easier to later refactor the individual port write commands.
>
> Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
> Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>
> Signed-off-by: Karol Kolacinski <karol.kolacinski@...el.com>
> ---
> drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 140 ++++++++++++--------
> drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 2 +-
> 2 files changed, 89 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> index e86ca6cada79..c892b966c3b8 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> @@ -227,40 +227,114 @@ static u64 ice_ptp_read_src_incval(struct ice_hw *hw)
> }
>
> /**
> - * ice_ptp_src_cmd - Prepare source timer for a timer command
> - * @hw: pointer to HW structure
> + * ice_ptp_tmr_cmd_to_src_reg - Convert to source timer command value
> + * @hw: pointer to HW struct
> * @cmd: Timer command
> *
> - * Prepare the source timer for an upcoming timer sync command.
> + * Returns: the source timer command register value for the given PTP timer
> + * command.
> */
> -void ice_ptp_src_cmd(struct ice_hw *hw, enum ice_ptp_tmr_cmd cmd)
> +static u32 ice_ptp_tmr_cmd_to_src_reg(struct ice_hw *hw,
> + enum ice_ptp_tmr_cmd cmd)
> {
> - u32 cmd_val;
> - u8 tmr_idx;
> + u32 cmd_val, tmr_idx;
> +
> + switch (cmd) {
> + case ICE_PTP_INIT_TIME:
> + cmd_val = GLTSYN_CMD_INIT_TIME;
> + break;
> + case ICE_PTP_INIT_INCVAL:
> + cmd_val = GLTSYN_CMD_INIT_INCVAL;
> + break;
> + case ICE_PTP_ADJ_TIME:
> + cmd_val = GLTSYN_CMD_ADJ_TIME;
> + break;
> + case ICE_PTP_ADJ_TIME_AT_TIME:
> + cmd_val = GLTSYN_CMD_ADJ_INIT_TIME;
> + break;
> + case ICE_PTP_NOP:
> + case ICE_PTP_READ_TIME:
> + cmd_val = GLTSYN_CMD_READ_TIME;
> + break;
> + default:
> + dev_warn(ice_hw_to_dev(hw),
> + "Ignoring unrecognized timer command %u\n", cmd);
> + cmd_val = 0;
> + }
>
> tmr_idx = ice_get_ptp_src_clock_index(hw);
> - cmd_val = tmr_idx << SEL_CPK_SRC;
> +
> + return tmr_idx | cmd_val << SEL_CPK_SRC;
This is not equivalent to what was returned before this patch. Is this
supposed to return different values now?
> +}
> +
> +/**
> + * ice_ptp_tmr_cmd_to_port_reg- Convert to port timer command value
> + * @hw: pointer to HW struct
> + * @cmd: Timer command
> + *
> + * Note that some hardware families use a different command register value for
> + * the PHY ports, while other hardware families use the same register values
> + * as the source timer.
> + *
> + * Returns: the PHY port timer command register value for the given PTP timer
> + * command.
> + */
> +static u32 ice_ptp_tmr_cmd_to_port_reg(struct ice_hw *hw,
> + enum ice_ptp_tmr_cmd cmd)
> +{
> + u32 cmd_val, tmr_idx;
> +
> + /* Certain hardware families share the same register values for the
> + * port register and source timer register.
> + */
> + switch (hw->ptp.phy_model) {
> + case ICE_PHY_E810:
> + return ice_ptp_tmr_cmd_to_src_reg(hw, cmd) & TS_CMD_MASK_E810;
> + default:
> + break;
> + }
>
> switch (cmd) {
> case ICE_PTP_INIT_TIME:
> - cmd_val |= GLTSYN_CMD_INIT_TIME;
> + cmd_val = PHY_CMD_INIT_TIME;
> break;
> case ICE_PTP_INIT_INCVAL:
> - cmd_val |= GLTSYN_CMD_INIT_INCVAL;
> + cmd_val = PHY_CMD_INIT_INCVAL;
> break;
> case ICE_PTP_ADJ_TIME:
> - cmd_val |= GLTSYN_CMD_ADJ_TIME;
> + cmd_val = PHY_CMD_ADJ_TIME;
> break;
> case ICE_PTP_ADJ_TIME_AT_TIME:
> - cmd_val |= GLTSYN_CMD_ADJ_INIT_TIME;
> + cmd_val = PHY_CMD_ADJ_TIME_AT_TIME;
> break;
> case ICE_PTP_READ_TIME:
> - cmd_val |= GLTSYN_CMD_READ_TIME;
> + cmd_val = PHY_CMD_READ_TIME;
> break;
> case ICE_PTP_NOP:
> + cmd_val = 0;
> break;
> + default:
> + dev_warn(ice_hw_to_dev(hw),
> + "Ignoring unrecognized timer command %u\n", cmd);
> + cmd_val = 0;
> }
>
> + tmr_idx = ice_get_ptp_src_clock_index(hw);
> +
> + return tmr_idx | cmd_val << SEL_PHY_SRC;
Just pointing out that this is the same as the above function in case
the previous needs to be changed.
> +}
> +
Powered by blists - more mailing lists