Re: From: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

[Tomasz Lichwala <tomasz.lichwala@linux.intel.com>]

On 17.09.2026 11:43, Aleksandr Loktionov wrote:

> @@ -220,12 +238,28 @@ ice_send_component_table(struct pldmfw *context, struct pldmfw_component *compon
>  	struct ice_pf *pf = priv->pf;
>  	struct ice_hw *hw = &pf->hw;
>  	size_t length;
> +	u8 comp_bit;

Nit: initialize to u8 comp_bit = 0; - harmless as-is since the default: path returns early, but it silences static-analysis warnings about a variable set only inside a switch.

>  	int status;
>  
>  	switch (component->identifier) {
>  	case NVM_COMP_ID_OROM:
> +		comp_bit = ICE_FWU_COMP_OROM;
> +		break;
>  	case NVM_COMP_ID_NVM:
> +		comp_bit = ICE_FWU_COMP_NVM;
> +		break;
>  	case NVM_COMP_ID_NETLIST:
> +		comp_bit = ICE_FWU_COMP_NETLIST;
> +		break;
> +	case NVM_COMP_ID_MANIFEST:
> +		if (!ice_is_feature_supported(pf, ICE_F_EROT)) {
> +			dev_err(dev, "Unable to update due to a firmware component with unknown ID %u\n",
> +				component->identifier);
> +			NL_SET_ERR_MSG_MOD(extack,
> +					   "Unable to update due to unknown firmware component");
> +			return -EOPNOTSUPP;
> +		}
> +		comp_bit = ICE_FWU_COMP_MANIFEST;
>  		break;
>  	default:
>  		dev_err(dev, "Unable to update due to a firmware component with unknown ID %u\n",



> @@ -676,6 +746,16 @@ ice_flash_component(struct pldmfw *context, struct pldmfw_component *component)
>  		return -EINVAL;
>  	}
>  
> +	/* ice_send_component_table() is called for every component in the
> +	 * PLDM image before pldmfw_flash_image() ever calls this function,
> +	 * so priv->seen_components is already fully populated by the time
> +	 * the first component reaches ice_flash_component().
> +	 */
> +	if (ice_has_erot_incomplete(priv)) {

This check runs on every call to ice_flash_component(), but pldmfw_ops guarantees all send_component_table() calls finish before any flash_component() call. Since transfer_flag on the last call to ice_send_component_table() carries PLDM_TRANSFER_FLAG_END, the completeness check could run exactly once there instead of being repeated for every component in ice_flash_component().

> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "eRoT adapter requires all four components (NVM, OROM, NetList, Manifest) in a single update");
> +		return -EINVAL;

Commit message says "reject with -EPERM", but the code returns -EINVAL. Please align the commit message with the actual errno, or fix the errno if -EPERM was intended.

> +	}
>  	/* Mark this component for activating at the end */
>  	priv->activate_flags |= flag;
>  




Both patches are missing a proper Subject: line — the commit message starts directly with the body text, so git log --oneline shows the From: line instead of a subject, and there is no [PATCH net-next vN x/2] ice: ... tag prefix as listed in the cover letter's shortlog. Please regenerate/resend with git format-patch so each patch carries its own subject line and revision/series tag, matching the titles already given in the cover letter shortlog.


Thanks,
Tomasz