Re: [PATCH 2/4] mach-snapdragon: CapsuleUpdate: support all boot methods

[Caleb Connolly <caleb.connolly@linaro.org>]

On 4/1/25 14:42, Ilias Apalodimas wrote:
> Hi Caleb
> 
> On Wed, 26 Mar 2025 at 19:41, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>>
>> Expand capsule update support to correctly identify which partition
>> U-Boot is flashed to (between xbl, uefi, and boot including A/B
>> variants).
>>
>> Use qcom_boot_source to determine if we were chainloaded from ABL,
>> meaning U-Boot is on the boot partition, otherwise we assume uefi if
>> it's available, finally leaving the xbl partition.
>>
>> Set a different fw_name based on the target partition to prevent GUID
>> collisions, since a board may support U-Boot flashed to boot or XBL we
>> need to differentiate them since the U-Boot binary must be built
>> differently.
>>
> 
> [...]
> 
> 
>> -               if (!strncmp(info.name, partname, strlen(partname))) {
>> -                       log_debug("Found active %s partition: '%s'!\n", partname, info.name);
>> -                       strlcpy(name, info.name, sizeof(info.name));
>> -                       return partnum;
>> +               desc = dev_get_uclass_plat(dev);
>> +               if (!desc || desc->part_type == PART_TYPE_UNKNOWN)
>> +                       continue;
>> +               for (partnum = 1;; partnum++) {
>> +                       ret = part_get_info(desc, partnum, &info);
>> +                       if (ret)
>> +                               break;
>> +
>> +                       slot_status = (struct part_slot_status *)&info.type_flags;
>> +
>> +                       /*
>> +                        * Qualcomm Linux devices have a "uefi" partition, it's A/B but the
>> +                        * flags might not be set so we assume the A partition unless the B
>> +                        * partition is active.
>> +                        */
>> +                       if (!strncmp(info.name, "uefi", strlen("uefi")))
> 
> since it's a static string, use sizeof() to compute it at build time.

Ahh nice, I'll do that.

> I get what you are trying to do here and automatically detect the boot
> partition, I'll have a closer look in case we can somehow make this
> loop shorter.
> 
> {
>> +                               /*
>> +                                * If U-Boot was chainloaded somehow we can't be flashed to
>> +                                * the uefi partition
>> +                                */
>> +                               if (qcom_boot_source != QCOM_BOOT_SOURCE_XBL)
>> +                                       continue;
>> +
>> +                               *target_part_type = TARGET_PART_UEFI;
>> +                               /*
>> +                                * Found an active UEFI partition, this is where U-Boot is
>> +                                * flashed.
>> +                                */
>> +                               if (slot_status->active)
>> +                                       goto found;
>> +
>> +                               /* Prefer A slot if it's not marked active */
>> +                               if (get_part_slot(info.name) == SLOT_A) {
> 
> SLOT_NONE only applies to non-uefi partitions?

Yes, having a "uefi" partition is only the case on 2 boards that I'm 
aware of, and both have a/b variants. The xbl and boot partitions exist 
on A/B and non-A/B devices so we have to handle all variants there.
> 
>> +                                       /*
>> +                                        * If we found the A slot after the B slot (both
>> +                                        * inactive) then we assume U-Boot is on the A slot.
>> +                                        */
>> +                                       if (uefi_partnum >= 0)
>> +                                               goto found;
>> +
>> +                                       /* Didn't find the B slot yet */
>> +                                       uefi_partnum = partnum;
>> +                                       strlcpy(ptn_name, info.name, 32);
> 
> sizeof(ptn_name)

Thanks
> 
>> +                               } else {
>> +                                       /*
>> +                                        * Found inactive B slot after inactive A slot, return
>> +                                        * the A slot
>> +                                        */
>> +                                       if (uefi_partnum >= 0) {
>> +                                               partnum = uefi_partnum;
>> +                                               goto found;
>> +                                       }
>> +
>> +                                       /*
>> +                                        * Didn't find the A slot yet. Record that we found the
>> +                                        * B slot
> 
> [...]
> 
> Cheers
> /Ilias

-- 
Caleb (they/them)