Thanks Kory, This looks cleaner. I got ont nit, I don't think you need to cast the states to a unit8.
On Tue, 20 Jan 2026 at 03:35, Kory Maincent <[email protected]> wrote: > > Change fwu_state_machine_updates() to accept an enum fwu_bank_states > parameter instead of a boolean. This makes the function interface more > explicit and prepares for adding FWU_BANK_INVALID support to handle > boot failures on the active bank. > > Convert the FWU_BANK_* defines to an enum and update all call sites > accordingly. > > Signed-off-by: Kory Maincent <[email protected]> > --- Reviewed-by: Ilias Apalodimas <[email protected]> > Changes in v2: > - Refactors fwu_state_machine_updates() to use enum as parameter instead > of adding a new fwu_invalid_bank() helper. > --- > include/fwu.h | 22 ++++++++-------- > lib/efi_loader/efi_capsule.c | 5 ++-- > lib/fwu_updates/fwu_v1.c | 16 ++++++------ > lib/fwu_updates/fwu_v2.c | 50 +++++++++++++++--------------------- > 4 files changed, 44 insertions(+), 49 deletions(-) > > diff --git a/include/fwu.h b/include/fwu.h > index e7bd1d492af..9cee8fb085c 100644 > --- a/include/fwu.h > +++ b/include/fwu.h > @@ -80,9 +80,11 @@ struct fwu_mdata_ops { > > #define FWU_IMAGE_ACCEPTED 0x1 > > -#define FWU_BANK_INVALID (uint8_t)0xFF > -#define FWU_BANK_VALID (uint8_t)0xFE > -#define FWU_BANK_ACCEPTED (uint8_t)0xFC > +enum fwu_bank_states { > + FWU_BANK_INVALID = 0xFF, > + FWU_BANK_VALID = 0xFE, > + FWU_BANK_ACCEPTED = 0xFC, > +}; > > enum { > PRIMARY_PART = 1, > @@ -396,24 +398,24 @@ int fwu_get_mdata_size(uint32_t *mdata_size); > > /** > * fwu_state_machine_updates() - Update FWU state of the platform > - * @trial_state: Is platform transitioning into Trial State > + * @state: FWU bank state > * @update_index: Bank number to which images have been updated > * > - * On successful completion of updates, transition the platform to > - * either Trial State or Regular State. > + * FWU_BANK_VALID transition the platform to Trial state > + * FWU_BANK_ACCEPTED accept the FWU bank state > + * FWU_BANK_INVALID invalid the FWU bank state > * > * To transition the platform to Trial State, start the > * TrialStateCtr counter, followed by setting the value of bank_state > * field of the metadata to Valid state(applicable only in version 2 > * of metadata). > * > - * In case, the platform is to transition directly to Regular State, > - * update the bank_state field of the metadata to Accepted > - * state(applicable only in version 2 of metadata). > + * Saving the bank_state field of the metadata is only applicable in > + * version 2 of metadata. > * > * Return: 0 if OK, -ve on error > */ > -int fwu_state_machine_updates(bool trial_state, uint32_t update_index); > +int fwu_state_machine_updates(enum fwu_bank_states state, uint32_t > update_index); > > /** > * fwu_init() - FWU specific initialisations > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c > index 89e63ed8dd5..e0f9a6efa25 100644 > --- a/lib/efi_loader/efi_capsule.c > +++ b/lib/efi_loader/efi_capsule.c > @@ -465,7 +465,7 @@ static __maybe_unused efi_status_t > fwu_empty_capsule_process( > log_err("Unable to set the Accept bit for the image > %pUs\n", > image_guid); > > - status = fwu_state_machine_updates(0, active_idx); > + status = fwu_state_machine_updates(FWU_BANK_ACCEPTED, > active_idx); > if (status < 0) > ret = EFI_DEVICE_ERROR; > > @@ -510,7 +510,8 @@ static __maybe_unused efi_status_t > fwu_post_update_process(bool fw_accept_os) > log_err("Failed to update FWU metadata index values\n"); > } else { > log_debug("Successfully updated the active_index\n"); > - status = fwu_state_machine_updates(fw_accept_os ? 1 : 0, > + status = fwu_state_machine_updates(fw_accept_os ? > + FWU_BANK_VALID : > FWU_BANK_ACCEPTED, > update_index); > if (status < 0) > ret = EFI_DEVICE_ERROR; > diff --git a/lib/fwu_updates/fwu_v1.c b/lib/fwu_updates/fwu_v1.c > index 974abf216f6..5824cca98cf 100644 > --- a/lib/fwu_updates/fwu_v1.c > +++ b/lib/fwu_updates/fwu_v1.c > @@ -98,27 +98,27 @@ void fwu_populate_mdata_image_info(struct fwu_data *data) > > /** > * fwu_state_machine_updates() - Update FWU state of the platform > - * @trial_state: Is platform transitioning into Trial State > + * @state: FWU bank state > * @update_index: Bank number to which images have been updated > * > - * On successful completion of updates, transition the platform to > - * either Trial State or Regular State. > + * FWU_BANK_VALID transition the platform to Trial state > + * FWU_BANK_ACCEPTED accept the FWU bank state > + * FWU_BANK_INVALID invalid the FWU bank state > * > * To transition the platform to Trial State, start the > * TrialStateCtr counter, followed by setting the value of bank_state > * field of the metadata to Valid state(applicable only in version 2 > * of metadata). > * > - * In case, the platform is to transition directly to Regular State, > - * update the bank_state field of the metadata to Accepted > - * state(applicable only in version 2 of metadata). > + * Saving the bank_state field of the metadata is only applicable in > + * version 2 of metadata. > * > * Return: 0 if OK, -ve on error > */ > -int fwu_state_machine_updates(bool trial_state, > +int fwu_state_machine_updates(enum fwu_bank_states state, > uint32_t update_index) > { > - return fwu_trial_state_update(trial_state, update_index); > + return fwu_trial_state_update(state == FWU_BANK_VALID, update_index); > } > > /** > diff --git a/lib/fwu_updates/fwu_v2.c b/lib/fwu_updates/fwu_v2.c > index 159315b45b9..f48b6d1264b 100644 > --- a/lib/fwu_updates/fwu_v2.c > +++ b/lib/fwu_updates/fwu_v2.c > @@ -80,42 +80,27 @@ static int fwu_mdata_sanity_checks(void) > return 0; > } > > -static int fwu_bank_state_update(bool trial_state, uint32_t bank) > +static int fwu_bank_state_update(enum fwu_bank_states state, uint32_t bank) > { > int ret; > struct fwu_data *data = fwu_get_data(); > struct fwu_mdata *mdata = data->fwu_mdata; > > - if (!trial_state && !fwu_bank_accepted(data, bank)) > + if (state == FWU_BANK_ACCEPTED && !fwu_bank_accepted(data, bank)) > return 0; > > - mdata->bank_state[bank] = data->bank_state[bank] = trial_state ? > - FWU_BANK_VALID : FWU_BANK_ACCEPTED; > + mdata->bank_state[bank] = (uint8_t)state; > + data->bank_state[bank] = (uint8_t)state; > > ret = fwu_sync_mdata(mdata, BOTH_PARTS); > if (ret) > log_err("Unable to set bank_state for bank %u\n", bank); > else > - data->trial_state = trial_state; > + data->trial_state = state == FWU_BANK_VALID ? 1 : 0; > > return ret; > } > > -static int fwu_trial_state_start(uint update_index) > -{ > - int ret; > - > - ret = fwu_trial_state_ctr_start(); > - if (ret) > - return ret; > - > - ret = fwu_bank_state_update(1, update_index); > - if (ret) > - return ret; > - > - return 0; > -} > - > static bool fwu_get_mdata_mandatory(uint part) > { > int ret = 0; > @@ -171,27 +156,34 @@ void fwu_populate_mdata_image_info(struct fwu_data > *data) > > /** > * fwu_state_machine_updates() - Update FWU state of the platform > - * @trial_state: Is platform transitioning into Trial State > + * @state: FWU bank state > * @update_index: Bank number to which images have been updated > * > - * On successful completion of updates, transition the platform to > - * either Trial State or Regular State. > + * FWU_BANK_VALID transition the platform to Trial state > + * FWU_BANK_ACCEPTED accept the FWU bank state > + * FWU_BANK_INVALID invalid the FWU bank state > * > * To transition the platform to Trial State, start the > * TrialStateCtr counter, followed by setting the value of bank_state > * field of the metadata to Valid state(applicable only in version 2 > * of metadata). > * > - * In case, the platform is to transition directly to Regular State, > - * update the bank_state field of the metadata to Accepted > - * state(applicable only in version 2 of metadata). > + * Saving the bank_state field of the metadata is only applicable in > + * version 2 of metadata. > * > * Return: 0 if OK, -ve on error > */ > -int fwu_state_machine_updates(bool trial_state, uint32_t update_index) > +int fwu_state_machine_updates(enum fwu_bank_states state, uint32_t > update_index) > { > - return trial_state ? fwu_trial_state_start(update_index) : > - fwu_bank_state_update(0, update_index); > + int ret; > + > + if (state == FWU_BANK_VALID) { > + ret = fwu_trial_state_ctr_start(); > + if (ret) > + return ret; > + } > + > + return fwu_bank_state_update(state, update_index); > } > > /** > -- > 2.43.0 >
