Re: [dpdk-dev] [PATCH v3 06/14] test-bbdev: support HARQ validation

["Chautru, Nicolas" <nicolas.chautru@intel.com>]

From: Akhil Goyal <akhil.goyal@nxp.com> 
>> +	/* ignore parity mismatch false alarms for long iterations */
>> +	{
>> +		if (!(expected_status & (1 << RTE_BBDEV_SYNDROME_ERROR))
>
>What is the need of these extra braces.
>

Missing check added now on new patchset. Thanks. 

>> +
>> +/* Compute K0 for a given configuration for HARQ output length 
>> +computation
>> */
>> +static inline uint16_t
>> +get_k0(uint16_t n_cb, uint16_t z_c, uint8_t basegraph, uint8_t 
>> +rv_index) {
>> +	if (rv_index == 0)
>> +		return 0;
>> +	uint16_t n = (basegraph == 1 ? 66 : 50) * z_c;
>> +	if (n_cb == n) {
>> +		if (rv_index == 1)
>> +			return (basegraph == 1 ? 17 : 13) * z_c;
>> +		else if (rv_index == 2)
>> +			return (basegraph == 1 ? 33 : 25) * z_c;
>> +		else
>> +			return (basegraph == 1 ? 56 : 43) * z_c;
>> +	}
>> +	/* LBRM case - includes a division by N */
>> +	if (rv_index == 1)
>> +		return (((basegraph == 1 ? 17 : 13) * n_cb)
>> +				/ n) * z_c;
>> +	else if (rv_index == 2)
>> +		return (((basegraph == 1 ? 33 : 25) * n_cb)
>> +				/ n) * z_c;
>> +	else
>> +		return (((basegraph == 1 ? 56 : 43) * n_cb)
>> +				/ n) * z_c;
>> +}
>
>Please add comments for the logic behind these calculations. 
>It would be better to remove these hard codings and define some macros.
>There are some other hard codings in the patch. Please fix those as well.
>> +

Changed that function and added reference to related 3GPP table. Thanks. 

>> +/* HARQ output length including the Filler bits */ static inline 
>> +uint16_t compute_Harq_Len(struct rte_bbdev_op_ldpc_dec *ops_ld) {
>
>Camel Casing?
>

Thanks.

>
>One generic comment on the patches. I can see that the name of MACROS Is quite big. It would be better to rename them with a shorter logical name.
>

Are you refering to the RTE_BBDEV_LDPC_... ones? These come from the API and are not defined in this patchset, I would not want to change them now as these are exposed out for some time now.

Thanks
Nic