On Wed, Jun 12, 2024 at 04:00:49PM +0100, Anatoly Burakov wrote:
> From: Ian Stokes <ian.sto...@intel.com>
>
> Code using ice_ptp_one_port_cmd() (or the device specific variants for E822
> and
> ETH56G) has a subtle bug where-in the executing of ice_ptp_exec_tmr_cmd() will
> cause all other ports to execute their last executed command.
>
> Most flows operate on all timers at once, but some flows such as handling link
> state change only operate on one port at a time. This can lead to issues if
> the
> userspace performs clock operations at just the right time.
>
> In addition to this bug, recent support for new hardware families has
> complicated the structure and organization of the various functions involved
> in
> programming PHY ports.
>
> For example:
>
> * ETH56G and E822 both re-implement the same looping structure for
> operating on all ports
> * E810 and E830 use "ice_ptp_port_cmd" which takes a register value to
> write and only operates on these two families despite having a generic
> name.
>
> Refactor ice_ptp_one_port_cmd() to both fix the issue of stale PHY port
> commands, and reduce the organizational complexity.
>
> Rename the existing device implementations from ice_ptp_one_port_cmd_<device>
> to
> ice_ptp_write_port_cmd_<device>. This function just performs the task of
> writing
> one command to one port. Do not export this outside of ice_ptp_hw.c, instead
> ensure all callers use the refactored ice_ptp_one_port_cmd() which properly
> initializes all ports.
>
> Fix ice_ptp_one_port_cmd() to correctly loop over all ports. For the
> configured
> port, call ice_ptp_write_port_cmd() with the configured command. For all other
> ports, call ice_ptp_write_port_cmd() with ICE_PTP_NOP.
>
> Stop duplicating the port iteration logic in each of the
> ice_ptp_port_cmd_<dev>() implementations, instead moving this into the generic
> ice_ptp_port_cmd() implementation.
>
> Since every flow now properly initializes both the main timer register and all
> port registers, ice_ptp_clean_cmd() is no longer needed, so remove it.
>
> This fixes the issue of using stale PHY port commands, and aligns better with
> the flow for how ice_ptp_hw.c is implemented.
>
> Signed-off-by: Jacob Keller <jacob.e.kel...@intel.com>
> Signed-off-by: Ian Stokes <ian.sto...@intel.com>
> ---
> drivers/net/ice/base/ice_ptp_hw.c | 649 +++++++++++++++---------------
> drivers/net/ice/base/ice_ptp_hw.h | 25 +-
> 2 files changed, 356 insertions(+), 318 deletions(-)
>
<snip>
> +/**
> + * ice_ptp_one_port_cmd - Program one PHY port for a timer command
> + * @hw: pointer to HW struct
> + * @configured_port: the port that should execute the command
> + * @configured_cmd: the command to be executed on the configured port
> + * @lock_sbq: true if the sideband queue lock must be acquired
> + *
> + * Prepare one port for executing a timer command, while preparing all other
> + * ports to ICE_PTP_NOP. This allows executing a command on a single port
> + * while ensuring all other ports do not execute stale commands.
> + */
> +enum ice_status
Compiler again throws build error on use of ice_status here.
/Bruce
> +ice_ptp_one_port_cmd(struct ice_hw *hw, u8 configured_port,
> + enum ice_ptp_tmr_cmd configured_cmd, bool lock_sbq)
> +{
