|SUCCESS| [v2] raw/ifpga: replace printf calls with logging
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Wed Apr 8 19:23:38 CEST 2026
Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/162943
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-04-08
**Review of Patch: raw/ifpga: replace printf calls with logging**
Git SHA: 20260407143656.9960-1-wpan36 at wisc.edu
Author: Weijun Pan <wpan36 at wisc.edu>
Subject: [PATCH v2] raw/ifpga: replace printf calls with logging
This patch replaces direct printf() calls with driver logging macros in
ifpga_rawdev.c to comply with DPDK logging conventions.
> diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c
> index 5b9b596435..e004e002b7 100644
> --- a/drivers/raw/ifpga/ifpga_rawdev.c
> +++ b/drivers/raw/ifpga/ifpga_rawdev.c
> @@ -434,7 +434,7 @@ static int set_surprise_link_check_aer(
[ ... ]
> if (!ifpga_rdev || !ifpga_rdev->rawdev) {
> - printf("\n device does not exist\n");
> + IFPGA_RAWDEV_PMD_ERR("device does not exist");
> return -EFAULT;
> >
[ ... ]
> @@ -491,7 +491,7 @@ static int set_surprise_link_check_aer(
[ ... ]
> - printf(">>>>>>Set AER %x,%x %x,%x\n",
> + IFPGA_RAWDEV_PMD_INFO(">>>>>>Set AER %x,%x %x,%x",
> ifpga_rdev->aer_old[0], ifpga_rdev->aer_old[1],
> aer_new0, aer_new1);
In this logging conversion, are the ">>>>>>Set AER" prefix characters
appropriate for a production log message? This appears to be debug-style
output that might not be suitable for INFO level logging. Should this be
logged at DEBUG level with a more descriptive message instead?
> @@ -527,7 +527,7 @@ ifpga_rawdev_gsd_handle(__rte_unused void *param)
> }
>
> if (gsd_enable)
> - printf(">>>>>>Pls Shutdown APP\n");
> + IFPGA_RAWDEV_PMD_WARN(">>>>>>Pls Shutdown APP");
Similar to the previous comment, does the ">>>>>>Pls Shutdown APP" message
meet DPDK's logging quality standards? The use of "Pls" (abbreviation) and
"APP" (all caps) seems inconsistent with typical DPDK log messages. Should
this be rephrased to something more professional like "Application shutdown
required" or "GSD event detected, graceful shutdown recommended"?
Additionally, is WARN the appropriate log level here? Given that this appears
in a gsd_handle function, is this message actionable by the user, or should it
be INFO or NOTICE level?
More information about the test-report
mailing list