On 5/27/2020 2:23 PM, Hemant Agrawal wrote:
> This library is required for configuring FMAN for
> various flow configurations.
This is a big patch with new files, looks like a new base code drop.
Can you please give more explanation on the patch and what 'fmlib' is?
>
> Signed-off-by: Sachin Saxena <sachin.sax...@nxp.com>
> Signed-off-by: Hemant Agrawal <hemant.agra...@nxp.com>
<...>
> +#if defined(FM_LIB_DBG)
> + #define _fml_dbg(format, arg...) \
> + printf("fmlib [%s:%u] - " format, \
> + __func__, __LINE__, ##arg)
> +#else
> + #define _fml_dbg(arg...)
> +#endif
Shouldn't use 'printf' directly, this prevents using dynamic logging and our log
APIs. Please use a registered logtype instead.
> +
> +/*#define FM_IOCTL_DBG*/
> +
> +#if defined(FM_IOCTL_DBG)
> + #define _fm_ioctl_dbg(format, arg...) \
> + printk("fm ioctl [%s:%u](cpu:%u) - " format, \
> + __func__, __LINE__, smp_processor_id(), ##arg)
printk? :)
> +#else
> +# define _fm_ioctl_dbg(arg...)
> +#endif
> +
> +/**
> + @Group lnx_ioctl_ncsw_grp NetCommSw Linux User-Space (IOCTL) API
> + @{
> +*/
> +
> +#define NCSW_IOC_TYPE_BASE 0xe0
> + /**< defines the IOCTL type for all the NCSW Linux module commands */
> +
> +/**
> + @Group lnx_usr_FM_grp Frame Manager API
> +
> + @Description FM API functions, definitions and enums.
> +
> + @{
> +*/
There are lots of checkpatch warning in the block comment syntax, about missing
" * " on each line.
Other base dpaa/dpaa2 base code seems have it in the block comments, if this
won't create a maintance problem, what do you think to fix the syntax on
comments?
<...>
> + e_IOC_FM_PCD_PRS_COUNTERS_SHIM_PARSE_RESULT_RETURNED_WITH_ERR,
> + /**< Parser counter - counts the number of times SHIM parse result is
> returned with errors. */
> + e_IOC_FM_PCD_PRS_COUNTERS_SOFT_PRS_CYCLES,
> + /**< Parser counter - counts the number of cycles spent executing soft
> parser instruction (including stall cycles). */
> + e_IOC_FM_PCD_PRS_COUNTERS_SOFT_PRS_STALL_CYCLES,
> + /**< Parser counter - counts the number of cycles stalled waiting for
> parser internal memory reads while executing soft parser instruction. */
Can you please break long lines?
<...>
> +#if 0
> +TODO: unused IOCTL
> +/**
> + @Function FM_PCD_ModifyCounter
> +
> + @Description Writes a value to an enabled counter. Use "0" to reset the
> counter.
> +
> + @Param[in] ioc_fm_pcd_counters_params_t - The requested counter parameters.
> +
> + @Return 0 on success; Error code otherwise.
> +*/
> +#define FM_PCD_IOC_MODIFY_COUNTER _IOW(FM_IOC_TYPE_BASE,
> FM_PCD_IOC_NUM(10), ioc_fm_pcd_counters_params_t)
> +#define FM_PCD_IOC_SET_COUNTER FM_PCD_IOC_MODIFY_COUNTER
> +#endif
Can you please remove dead code?
<...>
> +/**
> + @Description Enumeration type for selecting the policer profile packet
> frame length selector
> +*/
> +typedef enum ioc_fm_pcd_plcr_frame_length_select {
> + e_IOC_FM_PCD_PLCR_L2_FRM_LEN, /**< L2 frame length */
> + e_IOC_FM_PCD_PLCR_L3_FRM_LEN, /**< L3 frame length */
> + e_IOC_FM_PCD_PLCR_L4_FRM_LEN, /**< L4 frame length */
> + e_IOC_FM_PCD_PLCR_FULL_FRM_LEN /**< Full frame length */
> +} ioc_fm_pcd_plcr_frame_length_select;
> +
> +/**
> + @Description Enumeration type for selecting roll-back frame
> +*/
> +typedef enum ioc_fm_pcd_plcr_roll_back_frame_select {
> + e_IOC_FM_PCD_PLCR_ROLLBACK_L2_FRM_LEN, /**< Rollback L2 frame length */
> + e_IOC_FM_PCD_PLCR_ROLLBACK_FULL_FRM_LEN /**< Rollback Full frame length
> */
> +} ioc_fm_pcd_plcr_roll_back_frame_select;
Please fix the leading whitespace for above two enums.
