Re: [PATCH net-next v4 00/15][pull request] Introduce Intel IDPF driver

[Jakub Kicinski <kuba@kernel.org>]

On Tue, 8 Aug 2023 17:35:27 -0700 Linga, Pavan Kumar wrote:
> On 8/8/2023 1:32 PM, Jakub Kicinski wrote:
> > On Mon,  7 Aug 2023 17:34:01 -0700 Tony Nguyen wrote:  
> >> This patch series introduces the Intel Infrastructure Data Path Function
> >> (IDPF) driver. It is used for both physical and virtual functions. Except
> >> for some of the device operations the rest of the functionality is the
> >> same for both PF and VF. IDPF uses virtchnl version2 opcodes and
> >> structures defined in the virtchnl2 header file which helps the driver
> >> to learn the capabilities and register offsets from the device
> >> Control Plane (CP) instead of assuming the default values.  
> > 
> > Patches 4 and 10 add kdoc warnings, please fix those.
> > And double check all the checkpatch warning about lines > 80 chars.  
> 
> Thanks for the feedback.
> 
> Will review the warnings regarding 80char limit. Are you wanting them 
> all removed or is it okay to leave the ones that help readability?

For some definition of helps readability, yes.
I glanced at the warnings on one of the first patches and most of them
were just comments.

>  > drivers/net/ethernet/intel/idpf/idpf.h:123: warning: Enum value   
> 'csum_caps' not described in enum 'idpf_cap_field'
>  > drivers/net/ethernet/intel/idpf/idpf.h:123: warning: Enum value   
> 'seg_caps' not described in enum 'idpf_cap_field'
>  > drivers/net/ethernet/intel/idpf/idpf.h:123: warning: Enum value   
> 'rss_caps' not described in enum 'idpf_cap_field'
>  > drivers/net/ethernet/intel/idpf/idpf.h:123: warning: Enum value   
> 'hsplit_caps' not described in enum 'idpf_cap_field'
>  > drivers/net/ethernet/intel/idpf/idpf.h:123: warning: Enum value   
> 'rsc_caps' not described in enum 'idpf_cap_field'
>  > drivers/net/ethernet/intel/idpf/idpf.h:123: warning: Enum value   
> 'other_caps' not described in enum 'idpf_cap_field'
>  > drivers/net/ethernet/intel/idpf/idpf_txrx.h:153: warning: Function   
> parameter or member 'DEFINE_DMA_UNMAP_ADDR(dma' not described in 
> 'idpf_tx_buf'
>  > drivers/net/ethernet/intel/idpf/idpf_txrx.h:153: warning: Function   
> parameter or member 'DEFINE_DMA_UNMAP_LEN(len' not described in 
> 'idpf_tx_buf'
> 
> /**
>   * enum idpf_cap_field - Offsets into capabilities struct for specific caps
>   * @IDPF_BASE_CAPS: generic base capabilities
>   * @IDPF_CSUM_CAPS: checksum offload capabilities
> ...
>   */
> enum idpf_cap_field {
>           IDPF_BASE_CAPS          = -1,
>           IDPF_CSUM_CAPS          = offsetof(struct virtchnl2_get_capabilities,
>                                              csum_caps),
>           IDPF_SEG_CAPS           = offsetof(struct
> ...
> }
> 
> 
> /**
>   * struct idpf_tx_buf
>   * @next_to_watch: Next descriptor to clean
>   * @skb: Pointer to the skb
>   * @dma: DMA address
>   * @len: DMA length
> ...
>   */
> struct idpf_tx_buf {
>          void *next_to_watch;
>          struct sk_buff *skb;
>          DEFINE_DMA_UNMAP_ADDR(dma);
>          DEFINE_DMA_UNMAP_LEN(len);
> ...
> }
> 
> The script is parsing the offsetof() argument as part of the enum, which 
> is not true. I believe it to be a false positive. Same for the second 
> one where it parses 'DEFINE_DMA_UNMAP_ADDR(dma'. Is it okay to use 'dma' 
> and 'len' in the kdoc header as-is or please suggest if you prefer 
> something?

The parser is in scripts/kernel-doc, it's not very complicated.
You can teach it new tricks.