[PATCH v2 2/2] Makefile: Don't allow new boards with SPL_FIT_GENERATOR

[Tom Rini]

On Mon, Jan 31, 2022 at 06:25:33PM -0500, Tom Rini wrote:
> On Mon, Jan 31, 2022 at 03:59:08PM -0700, Simon Glass wrote:
> > Hi Tom,
> > 
> > (yes Mark I am trying to stop further boards going in that use the
> > shell scripts)
> > 
> > On Mon, 31 Jan 2022 at 15:05, Tom Rini <trini at konsulko.com> wrote:
> > >
> > > On Mon, Jan 31, 2022 at 02:22:41PM -0700, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Mon, 31 Jan 2022 at 13:40, Tom Rini <trini at konsulko.com> wrote:
> > > > >
> > > > > On Mon, Jan 31, 2022 at 12:57:57PM -0700, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Mon, 31 Jan 2022 at 11:00, Tom Rini <trini at konsulko.com> wrote:
> > > > > > >
> > > > > > > On Mon, Jan 31, 2022 at 10:27:41AM -0700, Simon Glass wrote:
> > > > > > > > Hi Tom,
> > > > > > > >
> > > > > > > > On Mon, 31 Jan 2022 at 09:15, Tom Rini <trini at konsulko.com> wrote:
> > > > > > > > >
> > > > > > > > > On Mon, Jan 31, 2022 at 09:13:02AM -0700, Simon Glass wrote:
> > > > > > > > > > Hi Tom,
> > > > > > > > > >
> > > > > > > > > > On Mon, 31 Jan 2022 at 07:24, Tom Rini <trini at konsulko.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Sun, Jan 30, 2022 at 08:52:25AM -0700, Simon Glass wrote:
> > > > > > > > > > >
> > > > > > > > > > > > More than a year after this migration message appeared, we still have new
> > > > > > > > > > > > boards being added with this option. Add a check against this.
> > > > > > > > > > > >
> > > > > > > > > > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > > > > > > > > >
> > > > > > > > > > > Please just make this an error in checkpatch.pl instead.
> > > > > > > > > >
> > > > > > > > > > I couldn't think of a way of doing that...do you have an idea?
> > > > > > > > >
> > > > > > > > > Yes, 2f3e8d6a86cb ("checkpatch: report ERROR only on disabling of fdt
> > > > > > > > > and initrd relocation") updates the check I had for fdt_high/initrd_high
> > > > > > > > > being in the file at all to only be for additions.  And yes, I check
> > > > > > > > > every PR for new checkpatch ERROR lines and only ignore the ones for
> > > > > > > > > code imported from other projects.
> > > > > > > >
> > > > > > > > Yes, I understand that, but SPL_FIT_GENERATOR defaults to on for
> > > > > > > > certain boards, so there is no need to mention it anywhere in the
> > > > > > > > patch. Also someone could adjust the condition in the Kconfig to add
> > > > > > > > other boards.
> > > > > > >
> > > > > > > Then you want something a bit more like the fdt|initrd_high check now,
> > > > > > > along with updating the help around SPL_FIT_GENERATOR to note that this
> > > > > > > option is deprecated, is the path forward then I think.
> > > > > >
> > > > > > I'm still a bit lost.
> > > > > >
> > > > > > What I want: break the build if someone adds a new board that uses
> > > > > > SPL_FIT_GENERATOR
> > > > > >
> > > > > > What you are offering: checkpatch check for people adding that option
> > > > > >
> > > > > > But the patch doesn't generally include that option.
> > > > > >
> > > > > > I can certainly mention in the Kconfig help that the option is
> > > > > > deprecated, but without checking if it is defined for a NEW board, I
> > > > > > cannot prevent it from growing.
> > > > > >
> > > > > > What am I missing? Can you be more specific?
> > > > >
> > > > > How do you add a new board that enables SPL_FIT_GENERATOR without
> > > > > "SPL_FIT_GENERATOR" being in the resulting patch, other than being
> > > > > ARCH_ZYNQMP/ARCH_ROCKCHIP ?
> > > >
> > > > Well that's the case I am most concerned with, actually. Also, someone
> > > > might add a new condition to SPL_FIT_GENERATOR.
> > >
> > > For the current cases, we just need to get them migrated since it's all
> > > the same logic?  So it would I think be a one-and-done thing.  For a new
> > 
> > Yes I think so and some of them are done. These are what I can find:
> > 
> > ./arch/riscv/lib/mkimage_fit_opensbi.sh
> > ./arch/arm/mach-zynqmp/mkimage_fit_atf.sh
> > ./arch/arm/mach-imx/mkimage_fit_atf.sh
> > ./arch/arm/mach-rockchip/make_fit_atf.py
> > 
> > but they are not used by that many boards.
> > 
> > I feel that the amount of pending migration is somewhat overwhelming
> > and we should take a stronger line in mainline.
> > 
> > Perhaps I should send a patch to simply remove the option? Would that
> > be acceptable?
> 
> Is there something technically preventing their migration to buildman?
> Looking over examples for imx8* conversions, it's just adding a binman
> node and describing things there, yes?

Poking at this a bit more, it seems like the outstanding imx platforms
to be converted still have pending patches and it's just part of the
general imx backlog.  The riscv one isn't used and should be removed.
That leaves rockchip and zynqmp needing conversion.  Michal has already
commented in this thread and I'll leave it to him to say how long he
needs to see how long zynqmp needs to update.  That leaves Rockchip.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20220131/e07797d5/attachment.sig>