Re: [Intel-wired-lan] [PATCH iwl-next v7 01/12] virtchnl: add support for enabling PTP on iAVF

[Mateusz Polchlopek <mateusz.polchlopek@intel.com>]

On 6/8/2024 2:55 PM, Simon Horman wrote:
> On Tue, Jun 04, 2024 at 09:13:49AM -0400, Mateusz Polchlopek wrote:
>> From: Jacob Keller <jacob.e.keller@intel.com>
>>
>> Add support for allowing a VF to enable PTP feature - Rx timestamps
>>
>> The new capability is gated by VIRTCHNL_VF_CAP_PTP, which must be
>> set by the VF to request access to the new operations. In addition, the
>> VIRTCHNL_OP_1588_PTP_CAPS command is used to determine the specific
>> capabilities available to the VF.
>>
>> This support includes the following additional capabilities:
>>
>> * Rx timestamps enabled in the Rx queues (when using flexible advanced
>>    descriptors)
>> * Read access to PHC time over virtchnl using
>>    VIRTCHNL_OP_1588_PTP_GET_TIME
>>
>> Extra space is reserved in most structures to allow for future
>> extension (like set clock, Tx timestamps).  Additional opcode numbers
>> are reserved and space in the virtchnl_ptp_caps structure is
>> specifically set aside for this.
>> Additionally, each structure has some space reserved for future
>> extensions to allow some flexibility.
>>
>> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
>> Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
>> Reviewed-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
>> Signed-off-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com>
> 
> Hi Mateusz, Jacob, all,
> 
> If you need to respin this for some reason, please consider updating
> the Kernel doc for the following to include a short description.
> Else, please consider doing so as a follow-up
> 
> * struct virtchnl_ptp_caps
> * struct virtchnl_phc_time
> 
> Likewise as a follow-up, as it was not introduced by this patch, for:
> 
> * virtchnl_vc_validate_vf_msg
> 
> Flagged by kernel-doc -none -Wall
> 
> The above not withstanding, this looks good to me.
> 
> Reviewed-by: Simon Horman <horms@kernel.org>
> 
> ...

Hello Simon!

Thanks for Your review - I appreciate it.

I thought about followup series after this being merged but I received
one warning from kernel-bot regarding ARM architecture issue. That being
said I will post (probably tomorrow) v8 with fix for ARM architecture
issue and I will also include fixes for virtchnl_ptp_caps and
virtchnl_phc_time (and exceeded 80 chars issues in commit 6).

As You pointed, the virtchnl_vc_validate_vf_msg function has not been
introduced in this patch so I do not want to mix that. I will create
post-merge followup with documentation changes for mentioned function
(virtchnl_vc_validate_vf_msg) and also for one docs leftover from my
previous series (related to tx scheduler).

Mateusz