Re: [PATCH net-next 3/5] dpll: zl3073x: Add firmware loading functionality

[Simon Horman <horms@kernel.org>]

On Fri, Jul 25, 2025 at 05:41:34PM +0200, Ivan Vecera wrote:
> Add functionality for loading firmware files provided by the vendor
> to be flashed into the device's internal flash memory. The firmware
> consists of several components, such as the firmware executable itself,
> chip-specific customizations, and configuration files.
> 
> The firmware file contains at least a flash utility, which is executed
> on the device side, and one or more flashable components. Each component
> has its own specific properties, such as the address where it should be
> loaded during flashing, one or more destination flash pages, and
> the flashing method that should be used.
> 
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>

Hi Ivan,

Some minor feedback from my side.

...

> diff --git a/drivers/dpll/zl3073x/fw.c b/drivers/dpll/zl3073x/fw.c

...

> +/* Santity check */

Sanity

> +static_assert(ARRAY_SIZE(component_info) == ZL_FW_NUM_COMPONENTS);

...

> +int zl3073x_fw_flash(struct zl3073x_dev *zldev, struct zl3073x_fw *zlfw,
> +		     struct netlink_ext_ack *extack)
> +{
> +	int i, rc;
> +
> +	for (i = 0; i < ZL_FW_NUM_COMPONENTS; i++) {
> +		if (!zlfw->component[i])
> +			continue; /* Component is not present */
> +
> +		rc = zl3073x_fw_component_flash(zldev, zlfw->component[i],
> +						extack);
> +		if (rc)
> +			break;
> +	}

Perhaps it cannot happen in practice.
But Smatch warns that rc may be used uninitialised below.
And that does seem theoretically possible if all
iterations of the loop above hit the "continue" path.

> +
> +	return rc;
> +}

...