On Thu, Aug 24, 2023 at 08:41:23AM -0600, Simon Glass wrote: > Hi, > > On Thu, 24 Aug 2023 at 08:20, Tom Rini <tr...@konsulko.com> wrote: > > > > On Thu, Aug 24, 2023 at 06:46:57PM +0530, Neha Malcom Francis wrote: > > > Hi Simon, Nishanth > > > > > > On 24/08/23 08:57, Nishanth Menon wrote: > > > > On 21:01-20230823, Simon Glass wrote: > > > > > Hi Nishanth, > > > > > > > > > > On Wed, 23 Aug 2023 at 18:18, Nishanth Menon <n...@ti.com> wrote: > > > > > > > > > > > > On 17:57-20230823, Simon Glass wrote: > > > > > > [...] > > > > > > > > This is how we have a common bit of rST for how to build N > > > > > > > > boards, > > > > > > > > without having to do a literal copy and paste N times. > > > > > > > > > > > > > > How about using this? > > > > > > > > > > > > > > https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#substitution-definitions > > > > > > > > > > > > I was not able to succeed with complex includes such as: > > > > > > https://github.com/u-boot/u-boot/blob/master/doc/board/ti/am62x_sk.rst?plain=1#L89 > > > > > > > > > > > > am62x complete build procedure defined once and reused in other > > > > > > am62x > > > > > > platforms.. But the am62x build procedure itself is reused from > > > > > > common > > > > > > k3 build steps. > > > > > > > > > > I followed through these instructions. I find the env vars quite > > > > > confusing, since I don't really know what it is doing. It feels like a > > > > > script: > > > > > > > > > > do $a $b $c > > > > > do $f $e > > > > > > > > > > it is pretty hard to follow. I think it would be better to write > > > > > everything out in full for each board, like rockchip does. > > > > > > > > Unfortunately, this is a few major steps that is repeated for > > > > (currently): > > > > AM62x SK > > > > Toradex Verdin > > > > (pending: beagleplay - ) > > > > (once all the dust clears up, hopefully phytec) > > > > SK-LP > > > > .... > > > > > > > > I have no reasonable way to offer to keep them all in sync. > > > > https://libera.irclog.whitequark.org/u-boot/2023-07-26#34662854; > > > > is kind of why I went down this path.S > > > > > > > > > > > > > > Some other minor feedback: > > > > > > > > > > - The 'make' lines should really have -j $(nproc) added > > > > > > > > Different styles of shells.. > > > > > > > > > - The $ signs at the start of each command in the docs are a pain > > > > > since it stops me copying the commands into the terminal - can you > > > > > remove them? > > > > > > > > hehe.. "dont" let people blindly copy paste without understanding what > > > > is > > > > going on argument? > > > > > > > > If folks are OK, I sure can send a different patch series for that.. (or > > > > maybe motivate someone to do that instead of me ;)) > > > > > > > > > > > > > - It doesn't build for me: > > > > > > > > > > BINMAN .binman_stamp > > > > > Image 'ti-dm' is missing external blobs and is non-functional: > > > > > blob-ext > > > > > > > > > > /binman/ti-dm/blob-ext > > > > > (ti-dm/am62xx/ipc_echo_testb_mcu1_0_release_strip.xer5f): > > > > > Missing blob > > > > > > > > > > Some images are invalid > > > > > make[1]: *** > > > > > [/scratch/sglass/cosarm/src/third_party/u-boot/files/Makefile:1115: > > > > > .binman_stamp] Error 103 > > > > > make[1]: Leaving directory '/tmp/b/play' > > > > > make: *** [Makefile:177: sub-make] Error 2 > > > > > > > > > > > > ^^ Neha: This is what I was complaining about. > > > > > > > > https://u-boot.readthedocs.io/en/latest/board/ti/am62x_sk.html?highlight=am62#sources > > > > > > > > source: https://git.ti.com/git/processor-firmware/ti-linux-firmware.git > > > > is missing, we never used to break build previously binman converted > > > > now does. > > > > > > > > I am wondering if I need to explicitly call out git clone instructions > > > > out.. > > > > > > > > > > Right... this does seem to be a complaint that keeps coming up. > > > > > > Simon, my intention at the time of sending out the patch was that anyone > > > building the board should "not NOT" have the DM binary. The way we > > > structured the filename was that it looks at BINMAN_INDIRS to find > > > ti-dm/ipc_echo_testb_mcu1_0_release_strip.xer5f but I guess this is a > > > confusing way to put it across. Maybe we should rework that. Or not throw > > > an > > > error at all when DM isn't found. > > > > Well, is DM required or not? If it's optional, we have flags to mark it > > optional. If it's required, we have flags for make and buildman to tell > > binman "fake it". I am wondering if we still have the ability to have a > > more verbose "need it, can't find it" error message, and so that's where > > it should say that you forgot to set BINMAN_INDIRS or clone the firmware > > repository. > > If it is optional, use the 'optional' tag on the entry You will get a > warning but not an error. At the moment it is saying that the image is > non-functional, which is not good.
Yes, and I believe these are non-optional firmwares, so do we still have the ability to produce a more specific and verbose error message? -- Tom
signature.asc
Description: PGP signature
