|SUCCESS| net/ice: improve log messages for DDP loading

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/164048

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-05-16

I'll review this patch against the DPDK coding standards and contribution guidelines.

Commit: (SHA not available in patch format)
Author: David Marchand <david.marchand at redhat.com>
Subject: net/ice: improve log messages for DDP loading

This patch adjusts log levels and improves DDP (Dynamic Device
Personalization) loading error messages in the Intel ICE driver.

> diff --git a/drivers/net/intel/ice/ice_ethdev.c b/drivers/net/intel/ice/ice_ethdev.c
> index 0f2e7aee14..e065581ccf 100644
> --- a/drivers/net/intel/ice/ice_ethdev.c
> +++ b/drivers/net/intel/ice/ice_ethdev.c

[ ... ]

> @@ -2003,6 +2003,17 @@ static int ice_read_customized_path(char *pkg_file, uint16_t buff_len)
>  	return n;
>  }
>  
> +static int
> +ice_firmware_read(const char *file, void *buf, size_t *bufsz)
> +{
> +	int ret = rte_firmware_read(file, buf, bufsz);
> +
> +	if (ret < 0)
> +		PMD_INIT_LOG(DEBUG, "Cannot read DDP file %s", file);
> +
> +	return ret;
> +}

The implicit comparison 'if (ret < 0)' does not comply with the coding
standard. Forbidden tokens require explicit '!= 0' or '== 0' for all
comparisons, including negative/positive checks:

  if (ret != 0)

> +
>  int ice_load_pkg(struct ice_adapter *adapter, bool use_dsn, uint64_t dsn)
>  {
>  	struct ice_hw *hw = &adapter->hw;

[ ... ]

> @@ -2016,7 +2027,7 @@ int ice_load_pkg(struct ice_adapter *adapter, bool use_dsn, uint64_t dsn)
>  	/* first read any explicitly referenced DDP file*/
>  	if (adapter->devargs.ddp_filename != NULL) {

The comparison '!= NULL' violates the coding standard. Pointers should be
checked using implicit boolean context:

  if (adapter->devargs.ddp_filename)

>  		strlcpy(pkg_file, adapter->devargs.ddp_filename, sizeof(pkg_file));
> -		if (rte_firmware_read(pkg_file, &buf, &bufsz) == 0) {
> +		if (ice_firmware_read(pkg_file, &buf, &bufsz) == 0) {
>  			goto load_fw;
>  		} else {
>  			PMD_INIT_LOG(ERR, "Cannot load DDP file: %s", pkg_file);

[ ... ]

> @@ -2658,13 +2669,13 @@ ice_dev_init(struct rte_eth_dev *dev)
>  	if (pos) {
>  		if (rte_pci_read_config(pci_dev, &dsn_low, 4, pos + 4) < 0 ||
>  				rte_pci_read_config(pci_dev, &dsn_high, 4, pos + 8) < 0) {

The implicit comparisons '< 0' should be explicit '!= 0' per coding
standards. Does this logic properly handle both negative and positive
error returns from rte_pci_read_config()?

> -			PMD_INIT_LOG(ERR, "Failed to read pci config space");
> +			PMD_INIT_LOG(WARNING, "Failed to read pci config space");
>  		} else {
>  			use_dsn = true;
>  			dsn = (uint64_t)dsn_high << 32 | dsn_low;
>  		}
>  	} else {
> -		PMD_INIT_LOG(ERR, "Failed to read device serial number");
> +		PMD_INIT_LOG(INFO, "Failed to read device serial number");
>  	}

In this error path where pos == 0, does the INFO level properly indicate
that initialization continues despite the failed read?

Overall, the patch contains several coding standard violations regarding
implicit comparisons. The new ice_firmware_read() wrapper function needs
explicit comparison syntax, and existing violations in the modified code
should be addressed.