Re: [PATCH v2 05/14] selftests/nolibc: add menuconfig for development

From: Zhangjin Wu
Date: Thu Jul 27 2023 - 09:24:46 EST


Hi, Willy

> > On Wed, Jul 19, 2023 at 09:22:37PM +0800, Zhangjin Wu wrote:
> > > The default DEFCONFIG_<ARCH> may not always work for all architectures,
> > > let's allow users to tune some kernel config options with 'menuconfig'.
> > >
> > > This is important when porting nolibc to a new architecture, it also
> > > allows to speed up nolibc 'run' target testing via manually disabling
> > > tons of unnecessary kernel config options.
> > >
> > > Signed-off-by: Zhangjin Wu <falcon@xxxxxxxxxxx>
> > > ---
> > > tools/testing/selftests/nolibc/Makefile | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> > > index 058e7be070ea..06954b4b3885 100644
> > > --- a/tools/testing/selftests/nolibc/Makefile
> > > +++ b/tools/testing/selftests/nolibc/Makefile
> > > @@ -202,6 +202,9 @@ KERNEL_IMAGE = $(objtree)/$(IMAGE)
> > > defconfig:
> > > $(Q)$(MAKE_KERNEL) mrproper $(DEFCONFIG) prepare
> > >
> > > +menuconfig:
> > > + $(Q)$(MAKE_KERNEL) menuconfig
> >
> > What is the real benefit of this compared to just running the
> > standard "make menuconfig" ? We should avoid adding makefile targets
> > that do not bring value on top of that the top-level makefile does,
> > because it will make the useful ones much harder to spot, and will
> > tend to encourage users to use only that makefile during the tests,
> > which is a bad practice since many targets will always be missing
> > or work differently. It's important in my opinion that we strictly
> > stick to what is useful to operate the tests themselves and this
> > one doesn't make me feel like it qualifies as such.
>
> Ok, get it.
>
> I did like develop nolibc in tools/testing/selftests/nolibc/ without
> changing directories frequently or specifying the "-C" option
> unnecessary ;-)
>
> Since "make menuconfig" is only required during the first porting of a new
> architecture, so, it is ok to drop this patch.
>

Oh, Willy, I did find this is very important again.

I just want to check and test if we need to disable vsx for 32-bit
powerpc too, so, I plan to re-configure kernel via menuconfig to disable
VSX in kernel side, and forcely enable vsx in application side, but it
was very 'painful' when I was running something like this:

$ make defconfig ARCH=ppc CROSS_COMPILE=powerpc-linux-gnu-

To do a further menuconfig, I must switch to something else:

$ make menuconfig ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- -C ../../../../

Especially, our test is able to accept ARCH=ppc, but the top-level
kernel still only accept powerpc, it makes the development very
comfortable, of course, the typing of the relative or absolute path
every time is either not a good idea.

so, this doesn't simply duplicate with the one from top-level, it can
get the right ARCH, srctree for us, and this is heavily used by our
tinyconfig development to tune the config options very frequently, so,
let's still add this one in the new revision?

But I plan to merge the mrproper targets here to save another patch,
what about your idea?

menuconfig mrproper:
$(Q)$(MAKE_KERNEL) $@

Thanks,
Zhangjin

> Thanks,
> Zhangjin
>
> >
> > Willy