Re: [PATCH net] ice: avoid executing commands on other ports when driving sync

[Simon Horman <horms@kernel.org>]

On Wed, Aug 23, 2023 at 08:18:14AM -0700, Tony Nguyen wrote:
> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> The ice hardware has a synchronization mechanism used to drive the
> simultaneous application of commands on both PHY ports and the source timer
> in the MAC.
> 
> When issuing a sync via ice_ptp_exec_tmr_cmd(), the hardware will
> simultaneously apply the commands programmed for the main timer and each
> PHY port. Neither the main timer command register, nor the PHY port command
> registers auto clear on command execution.
> 
> During the execution of a timer command intended for a single port on E822
> devices, such as those used to configure a PHY during link up, the driver
> is not correctly clearing the previous commands.
> 
> This results in unintentionally executing the last programmed command on
> the main timer and other PHY ports whenever performing reconfiguration on
> E822 ports after link up. This results in unintended side effects on other
> timers, depending on what command was previously programmed.
> 
> To fix this, the driver must ensure that the main timer and all other PHY
> ports are properly initialized to perform no action.
> 
> The enumeration for timer commands does not include an enumeration value
> for doing nothing. Introduce ICE_PTP_NOP for this purpose. When writing a
> timer command to hardware, leave the command bits set to zero which
> indicates that no operation should be performed on that port.
> 
> Modify ice_ptp_one_port_cmd() to always initialize all ports. For all ports
> other than the one being configured, write their timer command register to
> ICE_PTP_NOP. This ensures that no side effect happens on the timer command.
> 
> To fix this for the PHY ports, modify ice_ptp_one_port_cmd() to always
> initialize all other ports to ICE_PTP_NOP. This ensures that no side
> effects happen on the other ports.
> 
> Call ice_ptp_src_cmd() with a command value if ICE_PTP_NOP in
> ice_sync_phy_timer_e822() and ice_start_phy_timer_e822().
> 
> With both of these changes, the driver should no longer execute a stale
> command on the main timer or another PHY port when reconfiguring one of the
> PHY ports after link up.
> 
> Fixes: 3a7496234d17 ("ice: implement basic E822 PTP support")
> Signed-off-by: Siddaraju DH <siddaraju.dh@intel.com>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Tested-by: Sunitha Mekala <sunithax.d.mekala@intel.com> (A Contingent worker at Intel)
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>

Reviewed-by: Simon Horman <horms@kernel.org>