Hi Colin, On mar., mars 12, 2024 at 14:04, "McAllister, Colin" <colin.mcallis...@garmin.com> wrote:
> 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. Yep, seems they got approved. I will follow-up on the v4 series. > > Best, > Colin > > From: Mattijs Korpershoek <mkorpersh...@baylibre.com> > Sent: Tuesday, March 12, 2024 4:47 AM > To: Sam Protsenko <semen.protse...@linaro.org>; McAllister, Colin > <colin.mcallis...@garmin.com> > Cc: u-boot@lists.denx.de; jpewhac...@gmail.com; s...@chromium.org; Igor > Opaniuk <igor.opan...@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.protse...@linaro.org<mailto:semen.protse...@linaro.org>> wrote: > > > >> On Fri, Mar 8, 2024 at 1:24 PM McAllister, Colin > >> <colin.mcallis...@garmin.com<mailto:colin.mcallis...@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.mcallis...@garmin.com<mailto:20240308165937.270499-1-colin.mcallis...@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]