Re: [PATCH v12 06/15] FWU: Add helper functions for accessing FWU metadata

Hi Sughosh

[...]

...

...

+static int fwu_alt_num_for_dfu_dev(struct dfu_entity *dfu, int dev_num,

•                              int part, unsigned char dfu_dev,
  

•                              u8 *alt_num)
  

+{

•   int ret;
  

•   switch(dfu_dev) {
  

•   case DFU_DEV_MMC:
  

•           if (dfu->layout == DFU_RAW_ADDR &&
  

•               dfu->data.mmc.dev_num == dev_num &&
  

•               dfu->data.mmc.part == part) {
  

•                   *alt_num = dfu->alt;
  

•                   ret = 0;
  

•           } else {
  

•                   ret = -ENOENT;
  

•           }
  

•           break;
  

•   default:
  

•           ret = -ENOENT;
  

•   }
  

I think that looks better than the previous version and it's also easier to add new DFU cases. Thanks for fixing this

...

...

•   return ret;
  

+}

+static int fwu_gpt_get_alt_num(struct blk_desc *desc, efi_guid_t *image_guid,

•                          u8 *alt_num, unsigned char dfu_dev)
  

+{

•   int ret = -1;
  

•   int i, part, dev_num;
  

•   struct dfu_entity *dfu;
  

•   dev_num = desc->devnum;
  

•   part = get_gpt_dfu_identifier(desc, image_guid);
  

•   if (part < 0)
  

•           return -ENOENT;
  

•   dfu_init_env_entities(NULL, NULL);
  

•   while (true) {
  

•           dfu = dfu_get_entity(i);
  

'i' initialization and incrementation are missing.

Yea that's due to the change I requested on the previous version. But that's fine, I still think checking against NULL is better approach here. When you send the next version please add a check for dfu_init_env_entities() as well since it returns an int.

Oops. I had tested the functionality before sending out the patches, and it had worked(unfortunately). I will fix this in the next version. Thanks for catching this Etienne!

-sughosh

...

br, etienne

...

•           if (!dfu) {
  

•                   ret = -ENOENT;
  

•                   break;
  

•           }
  

•           if (dfu->dev_type != dfu_dev)
  

•                   continue;
  

•           ret = fwu_alt_num_for_dfu_dev(dfu, dev_num, part, dfu_dev,
  

•                                         alt_num);
  

•           if (!ret)
  

•                   break;
  

•   }
  

•   dfu_free_entities();
  

•   return ret;
  

+}

+/**

• • fwu_plat_get_alt_num() - Get the DFU alt number
• • @dev: FWU metadata device
• • @image_guid: GUID value of the image for which the alt num is to

• •          be obtained
    

• • @alt_num: The DFU alt number for the image that is to be updated
• • Get the DFU alt number for the image that is to be updated. The
• • image is identified with the image_guid parameter that is passed
• • to the function.
• • Note: This is a weak function and platforms can override this with
• • their own implementation for obtaining the alt number value.
• • Return: 0 if OK, -ve on error
• */

+__weak int fwu_plat_get_alt_num(struct udevice *dev, efi_guid_t *image_guid,

•                           u8 *alt_num)
  

+{

•   struct fwu_mdata_gpt_blk_priv *priv = dev_get_priv(dev);
  

•   return fwu_gpt_get_alt_num(dev_get_uclass_plat(priv->blk_dev),
  

•                              image_guid, alt_num, DFU_DEV_MMC);
  

+}

2.34.1

Regards /Ilias