[U-Boot] [PATCH v2 2/2] cmd: Add 'bcb' command to read/modify/write BCB fields

[Sam Protsenko]

Hi Eugeniu,

On Tue, May 21, 2019 at 2:20 PM Eugeniu Rosca <erosca at de.adit-jv.com> wrote:
>
> Hi Sam,
>
> On Mon, May 20, 2019 at 06:16:38PM +0300, Sam Protsenko wrote:
> > Hi Eugeniu,
> >
> > On Mon, May 20, 2019 at 10:23 AM Eugeniu Rosca <erosca at de.adit-jv.com> wrote:
> [..]
> > > Igor, Sam, what's your view on the above? Would you suggest creating
> > > a doc/README.android-bcb or there is a more elegant solution to it?
> > >
> >
> > Documentation would be nice. Although you already provided a generic
> > description of 'bcb' command in 'help bcb', the user often wants to
> > read more high-level documentation. I'd say, you can copy the
> > description from commit message to doc/README.android-bcb, extending
> > it with usual use-cases, and how this command can be used in those
> > use-cases. For example, your pseudo-code for reboot reason you
> > provided to me earlier, etc.
>
> Agreed. In my queue.
>

Just to be clear: can we expect it to be sent in v3, or it will be
separate patch-set?

> > Also, it can be useful to mention if
> > Google have any requirements (mandatory or optional) for the
> > bootloader (misc partition, reboot reason, recovery, etc), and how
> > this 'bcb' command can help in those requirements implementation.
>
> Well, a subset of the Android bootloader requirements is publicly
> available via https://source.android.com/devices/bootloader, but I saw
> traces of the "Android Boot/Bootloader Requirements" document name
> mentioned in the descriptions of U-Boot commits received from our
> suppliers. I _do not_ have access to the latter. If anybody from Google
> sees this message and can share the document or a subset of it, that
> would be great.
>
> To be clear, the background behind the 'bcb' command is not because I
> was assigned the task to implement any of the Android bootloader
> requirements, but because I saw improper implementations of some of
> those requirements in the patches received from our supplier and
> wanted to showcase a better/U-Boot-compliant way to accomplish those.
>
> So, I don't give any commitment to support the Android-related parts in
> U-Boot, but will signal and express my opinion whenever any features
> backported from AOSP don't follow the patterns established in community.
>

Sure, I just meant that it could be nice to mention it in the
documentation, that 'bcb' command can help in those Google
requirements implementation.

> >
> > All that said, IMHO documentation/test wise: it's not critical in this
> > case, you can add that later, just to speed-up the whole development
> > process and divide it into iterations. But that's for maintainers to
> > decide, of course.
> >
> > Also, I've a feeling that we have another problem, more common than
> > just a documentation. At the moment we have a bunch of Android related
> > features, which don't have namespace separation on several levels:
> >  - file/directories: we may want to move all Android related commands
> > to sub-directory
> >  - commands: we may want to add main command called "android" for all
> > Android-related commands, or maybe just a prefix
> >  - Kconfig: we may want to have some generic naming scheme for all
> > Android-related commands
> >  - Documentation, tests: the same here
> >
> > So at some point we will probably need to discuss and fix that
> > somehow. Not here, of course :)
>
> Agree with the most of the bullets. However, WRT merging all the
> available Android commands into a single command, I see room for
> more discussion. Here is some valuable feedback from Ruslan Trofymenko:
>
>  --- quote from https://patchwork.ozlabs.org/patch/1004002/#2049412 ---
> On Thu, 6 Dec 2018 at 03:31, Simon Glass <sjg at chromium.org> wrote:
> > Perhaps we need a new 'android' command with an 'ab_select'
> > subcommand? Then the automatica abbreviation will work.
> >
>
> Agree with all your previous comments, will send v2 shortly. But this
> one I'd like to leave as is (I will drop android_ prefix though). I
> think adding "android" command may lead to unneeded architecture
> complexity in future, e.g.:
>   - we will need to re-work next commands as sub-commands of "android"
> command: fastboot, dtimg, mmc_swrite (which can be hard as ab_select
> command file has BSD license and other commands have GPL license)
>  - we will need to implement sub-sub-commands (e.g. for "android dtimg
> dump ..." etc.)
>  - having "android" command can be inconvenient for users and may
> break existing API (e.g. it will force users to use "android fastboot"
> instead of just "fastboot" command)
>  - actually I don't see any namespace issues that can be solved by
> adding "android" command
>
> Also, in subsequent patch, I will add the dedicated menu option for
> Android-related commands and will pull all existing Android commands
> (along with ab_select) to that menu entry.
>
> So I hope it's fine with you if I leave this command as "ab_select"?
>  --- end quote https://patchwork.ozlabs.org/patch/1004002/#2049412 ---
>
> In addition, based on the feedback from Alex Kiernan, 'bcb' might be
> useful in non-Android contexts. If so, then embedding it as sub-command
> into a higher level command (e.g. 'android') does not make much sense.
>

Yeah, let's keep it aside of this thread, I realize it's off-topic :)
We probably just need to make sure that all Android-related commands
are located in corresponding menu in menuconfig.

> --
> Best Regards,
> Eugeniu.