Re: [dpdk-dev] [PATCH v4 01/15] net/avf/base: add base code for avf PMD

[Stephen Hemminger <stephen@networkplumber.org>]

O
> diff --git a/drivers/net/avf/base/avf_adminq.c b/drivers/net/avf/base/avf_adminq.c
> new file mode 100644
> index 0000000..616e2a9
> --- /dev/null
> +++ b/drivers/net/avf/base/avf_adminq.c
> @@ -0,0 +1,1010 @@
> +/*******************************************************************************
> +
> +Copyright (c) 2013 - 2015, Intel Corporation
> +All rights reserved.

SPDX instead of more boilerplate.
Copyright 2018?

> +STATIC void avf_adminq_init_regs(struct avf_hw *hw)

Why is there a STATIC macro??

...

> +/**
> + *  avf_config_asq_regs - configure ASQ registers
> + *  @hw: pointer to the hardware structure
> + *
> + *  Configure base address and length registers for the transmit queue
> + **/
> +STATIC enum avf_status_code avf_config_asq_regs(struct avf_hw *hw)
> +{
> +	enum avf_status_code ret_code = AVF_SUCCESS;
> +	u32 reg = 0;
> +
> +	/* Clear Head and Tail */
> +	wr32(hw, hw->aq.asq.head, 0);
> +	wr32(hw, hw->aq.asq.tail, 0);
> +
> +	/* set starting point */
> +#ifdef INTEGRATED_VF
> +	if (avf_is_vf(hw))
> +		wr32(hw, hw->aq.asq.len, (hw->aq.num_asq_entries |
> +					  AVF_ATQLEN1_ATQENABLE_MASK));
> +#else
> +	wr32(hw, hw->aq.asq.len, (hw->aq.num_asq_entries |
> +				  AVF_ATQLEN1_ATQENABLE_MASK));
> +#endif /* INTEGRATED_VF */

No ifdef please? do it in header file if you have to.
as in:
#ifdef INTERGRATED_VF
#define avf_is_vf(hw)	(1)

...


> +/* internal (0x00XX) commands */
> +
> +/* Get version (direct 0x0001) */
> +struct avf_aqc_get_version {
> +	__le32 rom_ver;
> +	__le32 fw_build;
> +	__le16 fw_major;
> +	__le16 fw_minor;
> +	__le16 api_major;
> +	__le16 api_minor;
> +};

The use of __le16 and __le32 is a Linux kernel code
style, typically not used in DPDK userland.

Are you trying to share code here?

...

> +/**
> + * virtchnl_vc_validate_vf_msg
> + * @ver: Virtchnl version info
> + * @v_opcode: Opcode for the message
> + * @msg: pointer to the msg buffer
> + * @msglen: msg length
> + *
> + * validate msg format against struct for each opcode
> + */
> +static inline int
> +virtchnl_vc_validate_vf_msg(struct virtchnl_version_info *ver, u32 v_opcode,
> +			    u8 *msg, u16 msglen)
> +{
>

This function is way to big to be an inline.