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

Tue Mar 12 15:04:58 CET 2024

Hi Mattijs,

I’ve been using git send-email, but there might be issues with what the Garmin smtp server is doing to the email, like adding the footer. I sent a v4 PS in a new thread using my personal email, but that email isn’t subscribed to this ML so I think the patches are pending approval to be added to the ML.

Best,
Colin

From: Mattijs Korpershoek <mkorpershoek at baylibre.com>
Sent: Tuesday, March 12, 2024 4:47 AM
To: Sam Protsenko <semen.protsenko at linaro.org>; McAllister, Colin <Colin.McAllister at garmin.com>
Cc: u-boot at lists.denx.de; JPEWhacker at gmail.com; sjg at chromium.org; Igor Opaniuk <igor.opaniuk at gmail.com>
Subject: Re: [PATCH v3 0/2] Fix Android A/B backup

Hi Colin, On ven. , mars 08, 2024 at 15: 59, Sam Protsenko <semen. protsenko@ linaro. org> wrote: > On Fri, Mar 8, 2024 at 1: 24 PM McAllister, Colin > <Colin. McAllister@ garmin. com> wrote: >> >> > Ah, ok, I see you

Hi Colin,

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

> On Fri, Mar 8, 2024 at 1:24 PM McAllister, Colin

> <Colin.McAllister at garmin.com<mailto: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<mailto: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.

  https://urldefense.com/v3/__http://git-send-email.io/__;!!EJc4YC3iFmQ!SPI0-0rQ9vj-ywEEcv6DQJbMQAyFwaCwC-RJNoupu3oyPLgiELEAXx-cPq4j8G_SXOw5oXUX621ACOzFcWz8Ldw1zsFDgg$<https://urldefense.com/v3/__http:/git-send-email.io/__;!!EJc4YC3iFmQ!SPI0-0rQ9vj-ywEEcv6DQJbMQAyFwaCwC-RJNoupu3oyPLgiELEAXx-cPq4j8G_SXOw5oXUX621ACOzFcWz8Ldw1zsFDgg$> is a friendly service you can use to test

  your workflow.

On workflow, tooling, I usually suggest the b4 tool:

https://urldefense.com/v3/__https://people.kernel.org/monsieuricon/sending-a-kernel-patch-with-b4-part-1__;!!EJc4YC3iFmQ!SPI0-0rQ9vj-ywEEcv6DQJbMQAyFwaCwC-RJNoupu3oyPLgiELEAXx-cPq4j8G_SXOw5oXUX621ACOzFcWz8LdwuYhSjtw$<https://urldefense.com/v3/__https:/people.kernel.org/monsieuricon/sending-a-kernel-patch-with-b4-part-1__;!!EJc4YC3iFmQ!SPI0-0rQ9vj-ywEEcv6DQJbMQAyFwaCwC-RJNoupu3oyPLgiELEAXx-cPq4j8G_SXOw5oXUX621ACOzFcWz8LdwuYhSjtw$>

Regards,

Mattijs

>

> [1] https://urldefense.com/v3/__https://docs.u-boot.org/en/latest/develop/patman.html__;!!EJc4YC3iFmQ!SPI0-0rQ9vj-ywEEcv6DQJbMQAyFwaCwC-RJNoupu3oyPLgiELEAXx-cPq4j8G_SXOw5oXUX621ACOzFcWz8Ldyum8H6Ow$<https://urldefense.com/v3/__https:/docs.u-boot.org/en/latest/develop/patman.html__;!!EJc4YC3iFmQ!SPI0-0rQ9vj-ywEEcv6DQJbMQAyFwaCwC-RJNoupu3oyPLgiELEAXx-cPq4j8G_SXOw5oXUX621ACOzFcWz8Ldyum8H6Ow$>

>

> [snip]