[PATCH v3 0/2] Fix Android A/B backup

[Mattijs Korpershoek]

Hi Colin,

On ven., mars 08, 2024 at 15:59, Sam Protsenko <semen.protsenko at linaro.org> wrote:

> On Fri, Mar 8, 2024 at 1:24 PM McAllister, Colin
> <Colin.McAllister at garmin.com> wrote:
>>
>> > Ah, ok, I see you replied to my comment here.
>>
>> Yes, sorry. Outlook is terrible to send inline responses too. I figured
>> just adding responses in the patch contents would be better. Next time I'll submit
>> my patch with a different email :)
>>
>> > So when that config option is not defined at all, the build still
>> > works, right?
>>
>> Yes, the default value for CONFIG_ANDROID_AB_BACKUP_OFFSET is 0x0, which
>> would evaluate to a false bool value in the if conditions. I did do some
>> testing with the config value not defined for my board and confirmed
>> back-up data is not used.
>>
>
> Looks good to me, thanks.
>
>> In your other emails you include your reviewed-by tag. For clarity, Am I
>> supposed to append my patches and upload a new version? This is my
>> first time contributing to u-boot, so I'm still learning the workflow. I
>> didn't see anything glancing through the "Sending patches" page in the
>> U-Boot documentation.
>>
>
> Welcome to the community! And thanks for your patches :) U-Boot
> workflow is quite similar to Linux kernel one. It's useful to collect
> all tags when sending out your next version. When the maintainer takes
> your patch, they usually also apply all R-b tags for the final patch
> version, so you only have to worry about that when sending out a new
> version. I know that U-Boot contributors are often using `patman' tool
> [1] for submitting patches (and corresponding updated versions), and
> I'm pretty sure it collects all pending tags automatically for you.
> FWIW, I'm not experienced with `patman', as I'm trying to use somehow
> unified submitting process for both U-Boot and Linux kernel, and I
> know that using `patman' is sometimes discouraged in Linux kernel
> community.

Welcome to the U-Boot community! It takes quite some time to start
contributing so thank you for the patches.

The changes look fine and the detailed approach on how you tested is
very much appreciated.

I have a couple of suggestions on the whole series.

1. The patches don't apply:

$ b4 shazam -s -l 20240308165937.270499-1-colin.mcallister at garmin.com

error: patch failed: boot/android_ab.c:187
error: boot/android_ab.c: patch does not apply
error: Did you hand edit your patch?
It does not apply to blobs recorded in its index.
Patch failed at 0002 android_ab: Fix ANDROID_AB_BACKUP_OFFSET
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

I suspect your email provider swapped tabs to spaces. It's also possible
that the Garmin confidentiality footer caused this.

2. Using the khadas-vim3_android_ab_defconfig, this does not build:

boot/android_ab.c: In function 'ab_select_slot':
boot/android_ab.c:350:48: error: 'ANDROID_AB_BACKUP_OFFSET' undeclared (first use in this function); did you mean 'CONFIG_ANDROID_AB_BACKUP_OFFSET'?
  350 |                                                ANDROID_AB_BACKUP_OFFSET);
      |                                                ^~~~~~~~~~~~~~~~~~~~~~~~
      |                                                CONFIG_ANDROID_AB_BACKUP_OFFSET

Both are minor problems.
I re-applied the diffs manually to be able to build/boot test this.

Since this is your first contribution, I can either:
- fix both myself and merge this.
- let you spin a v4 to fix the above.

Please let me know what you prefer.

If you do intend to send a v4, please:
- Do it in a new email thread
- Make sure you cc me, Igor and Sam
- Make sure the patches you send can be applied.
  http://git-send-email.io/ is a friendly service you can use to test
  your workflow.

On workflow, tooling, I usually suggest the b4 tool:
https://people.kernel.org/monsieuricon/sending-a-kernel-patch-with-b4-part-1

Regards,
Mattijs

>
> [1] https://docs.u-boot.org/en/latest/develop/patman.html
>
> [snip]