Re: [PATCH v3 01/22] common/idpf: added NVME CPF specific code with defines

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

On 6/12/2024 5:52 AM, Soumyadeep Hore wrote:
> Removes NVME dependency on memory allocations and
> uses a prepared buffer instead.
> 
> The changes do not affect other components.
> 
> Signed-off-by: Soumyadeep Hore <soumyadeep.hore@intel.com>
> ---

<snip>

>   	return status;
>   }
> @@ -232,8 +244,13 @@ void idpf_ctlq_remove(struct idpf_hw *hw,
>    * destroyed. This must be called prior to using the individual add/remove
>    * APIs.
>    */
> +#ifdef NVME_CPF
> +int idpf_ctlq_init(struct idpf_hw *hw, u8 num_q,
> +			struct idpf_ctlq_create_info *q_info, struct idpf_ctlq_info **ctlq)
> +#else
>   int idpf_ctlq_init(struct idpf_hw *hw, u8 num_q,
>   		   struct idpf_ctlq_create_info *q_info)
> +#endif

Nitpicking, but the added function's indentation seems different from 
the rest of the functions in this file file. Is this how it is in base code?

>   {
>   	struct idpf_ctlq_info *cq = NULL, *tmp = NULL;
>   	int ret_code = 0;
> @@ -244,6 +261,10 @@ int idpf_ctlq_init(struct idpf_hw *hw, u8 num_q,
>   	for (i = 0; i < num_q; i++) {
>   		struct idpf_ctlq_create_info *qinfo = q_info + i;
>   
> +#ifdef NVME_CPF
> +		cq = *(ctlq + i);
> +#endif
> +
>   		ret_code = idpf_ctlq_add(hw, qinfo, &cq);
>   		if (ret_code)
>   			goto init_destroy_qs;
> diff --git a/drivers/common/idpf/base/idpf_controlq_api.h b/drivers/common/idpf/base/idpf_controlq_api.h
> index 38f5d2df3c..6b6f3e84c2 100644
> --- a/drivers/common/idpf/base/idpf_controlq_api.h
> +++ b/drivers/common/idpf/base/idpf_controlq_api.h
> @@ -1,5 +1,5 @@
>   /* SPDX-License-Identifier: BSD-3-Clause
> - * Copyright(c) 2001-2023 Intel Corporation
> + * Copyright(c) 2001-2024 Intel Corporation
>    */
>   
>   #ifndef _IDPF_CONTROLQ_API_H_
> @@ -158,8 +158,13 @@ enum idpf_mbx_opc {
>   /* Will init all required q including default mb.  "q_info" is an array of
>    * create_info structs equal to the number of control queues to be created.
>    */
> +#ifdef NVME_CPF
> +int idpf_ctlq_init(struct idpf_hw *hw, u8 num_q,
> +			struct idpf_ctlq_create_info *q_info, struct idpf_ctlq_info **ctlq);
> +#else

Same question as above.

Also, a more general question on #ifdef - is it expected to be enabled 
somehow?

>   int idpf_ctlq_init(struct idpf_hw *hw, u8 num_q,
>   		   struct idpf_ctlq_create_info *q_info);
> +#endif
>   
>   /* Allocate and initialize a single control queue, which will be added to the
>    * control queue list; returns a handle to the created control queue

-- 
Thanks,
Anatoly