Re: [Intel-wired-lan] [PATCH iwl-next 1/2] idpf: add direct access to discipline the main timer

[Vadim Fedorenko <vadim.fedorenko@linux.dev>]

On 02/09/2025 11:50, Anton Nadezhdin wrote:
> From: Milena Olech <milena.olech@intel.com>
> 
> IDPF allows to access the clock through virtchnl messages, or directly,
> through PCI BAR registers. Registers offsets are negotiated with the
> Control Plane during driver initialization process.
> Add support for direct operations to modify the clock.
> 
> Signed-off-by: Milena Olech <milena.olech@intel.com>
> Signed-off-by: Anton Nadezhdin <anton.nadezhdin@intel.com>
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> ---

[...]

>   static void idpf_ptp_reg_init(const struct idpf_adapter *adapter)
>   {
> -	adapter->ptp->cmd.shtime_enable_mask = PF_GLTSYN_CMD_SYNC_SHTIME_EN_M;
> -	adapter->ptp->cmd.exec_cmd_mask = PF_GLTSYN_CMD_SYNC_EXEC_CMD_M;
> +	adapter->ptp->cmd.shtime_enable = PF_GLTSYN_CMD_SYNC_SHTIME_EN_M;
> +	adapter->ptp->cmd.exec_cmd = PF_GLTSYN_CMD_SYNC_EXEC_CMD_M;
>   }

[...]

> +/**
> + * idpf_ptp_set_dev_clk_time_direct- Set the time of the clock directly through
> + *				     BAR registers.
> + * @adapter: Driver specific private structure
> + * @dev_clk_time: Value expressed in nanoseconds to set
> + *
> + * Set the time of the device clock to provided value directly through BAR
> + * registers received during PTP capabilities negotiation.
> + */
> +static void idpf_ptp_set_dev_clk_time_direct(struct idpf_adapter *adapter,
> +					     u64 dev_clk_time)
> +{
> +	struct idpf_ptp *ptp = adapter->ptp;
> +	u32 dev_clk_time_l, dev_clk_time_h;
> +
> +	dev_clk_time_l = lower_32_bits(dev_clk_time);
> +	dev_clk_time_h = upper_32_bits(dev_clk_time);
> +
> +	writel(dev_clk_time_l, ptp->dev_clk_regs.dev_clk_ns_l);
> +	writel(dev_clk_time_h, ptp->dev_clk_regs.dev_clk_ns_h);
> +
> +	writel(dev_clk_time_l, ptp->dev_clk_regs.phy_clk_ns_l);
> +	writel(dev_clk_time_h, ptp->dev_clk_regs.phy_clk_ns_h);
> +
> +	idpf_ptp_tmr_cmd(adapter, ptp->cmd.init_time);
> +}
> +
[...]

> +/**
> + * idpf_ptp_adj_dev_clk_time_direct - Adjust the time of the clock directly
> + *				      through BAR registers.
> + * @adapter: Driver specific private structure
> + * @delta: Offset in nanoseconds to adjust the time by
> + *
> + * Adjust the time of the clock directly through BAR registers received during
> + * PTP capabilities negotiation.
> + */
> +static void idpf_ptp_adj_dev_clk_time_direct(struct idpf_adapter *adapter,
> +					     s64 delta)
> +{
> +	struct idpf_ptp *ptp = adapter->ptp;
> +	u32 delta_l = (s32)delta;
> +
> +	writel(0, ptp->dev_clk_regs.shadj_l);
> +	writel(delta_l, ptp->dev_clk_regs.shadj_h);
> +
> +	writel(0, ptp->dev_clk_regs.phy_shadj_l);
> +	writel(delta_l, ptp->dev_clk_regs.phy_shadj_h);
> +
> +	idpf_ptp_tmr_cmd(adapter, ptp->cmd.adj_time);
> +}

[...]

> - * struct idpf_ptp_cmd - PTP command masks
> - * @exec_cmd_mask: mask to trigger command execution
> - * @shtime_enable_mask: mask to enable shadow time
> + * struct idpf_ptp_cmd_mask - PTP command masks
> + * @exec_cmd: mask to trigger command execution
> + * @shtime_enable: mask to enable shadow time
> + * @init_time: initialize the device clock timer
> + * @init_incval: initialize increment value
> + * @adj_time: adjust the device clock timer
> + * @read_time: read the device clock timer
>    */
> -struct idpf_ptp_cmd {
> -	u32 exec_cmd_mask;
> -	u32 shtime_enable_mask;
> +struct idpf_ptp_cmd_mask {
> +	u32 exec_cmd;
> +	u32 shtime_enable;
> +	u32 init_time;
> +	u32 init_incval;
> +	u32 adj_time;
> +	u32 read_time;
>   };
>   
>   /* struct idpf_ptp_dev_clk_regs - PTP device registers
> @@ -183,7 +191,7 @@ struct idpf_ptp {
>   	struct idpf_adapter *adapter;
>   	u64 base_incval;
>   	u64 max_adj;
> -	struct idpf_ptp_cmd cmd;
> +	struct idpf_ptp_cmd_mask cmd;
>   	u64 cached_phc_time;
For the field cmd you changed the struct definition to add more commands
but this diff doesn't init values for new fields. At the same time these
new fields are used in several new functions (idpf_ptp_*_direct). We end
up using 0 while issuing the command.