[dpdk-dev] [PATCH v2 06/13] baseband/fpga_5gnr_fec: add queue configuration

Chautru, Nicolas nicolas.chautru at intel.com
Tue Apr 14 02:16:22 CEST 2020


Thanks Rosen for your thorough code review. Some comments in-line below. 

> From: Xu, Rosen <rosen.xu at intel.com>
> 
> Hi,
> 
> Could you prefix all functions name with the FPGA IP name? FPGA is a very
> common device name.
> 

I don't see such a guideline being used across all other PMDs. 
Unsure it always help notably as this would create many long function names with fpga_5gnr_fec_ added each time, and these names are not exposed outside of this .c file.
Also some of the fpga_ prefixes would apply for either fpga_lte_fec or fpga_5gnr_fec PMD. 
If this becomes of a new guide lines we can rename every single function in each existing baseband PMD in future release (not just this new PMD). 

> >  /* Read a register of FPGA 5GNR FEC device */  static uint32_t
> > fpga_reg_read_32(void *mmio_base, uint32_t offset) @@ -31,9 +106,115
> > @@
> >  	return rte_le_to_cpu_32(ret);
> >  }
> 
> Why you didn't use rte_writeXXX() API of DPDK rte library?
> 

rte_write32 includes some memory barriers which are not required here.
Also we handle the byte endianness in these internal implementations. 

Thanks, 
Nic


More information about the dev mailing list