On Thu, 31 Jan 2019 19:26 Masahiro Yamada <yamada.masah...@socionext.com wrote:
> On Wed, Jan 30, 2019 at 5:55 PM Masahiro Yamada > <yamada.masah...@socionext.com> wrote: > > > > On Wed, Jan 30, 2019 at 4:23 PM Chris Packham <judge.pack...@gmail.com> > wrote: > > > > > > Moveconfig already attempts to remove empty #if/#endif blocks when > there > > > is a matching CONFIG_ being moved. Add a second pass which covers files > > > without a match. > > > > > > Signed-off-by: Chris Packham <judge.pack...@gmail.com> > > > --- > > > This was previously submitted as > > > http://patchwork.ozlabs.org/patch/924901/ there still seems to be > cases > > > of #if/#endif left over from Kconfig migrations so perhaps this is > still > > > needed/wanted. > > > > > > I've plumbed this in as a second pass because ultimately we may want to > > > make this a separate option. Also I couldn't figure out how to > implement > > > this without using re.M so I couldn't make it work in with the line by > > > line parsing of cleanup_one_header(). > > > > > > This seems useful to find leftover code > > regardless of the CONFIG options you are moving. > > > > One drawback is, it may fold unrelated cleanups. > > > > Maybe, it would be useful to add a separate option to turn on this > feature, > > or make it into a separate tool. > > > > > > > > > I tested this patch. > It detected one empty block. > > > diff --git a/include/configs/omap3_cairo.h b/include/configs/omap3_cairo.h > index 04bce2f..ac1a6cb 100644 > --- a/include/configs/omap3_cairo.h > +++ b/include/configs/omap3_cairo.h > @@ -197,8 +197,6 @@ > * are needed and peripheral clocks for UART2 must be enabled in > * function per_clocks_enable(). > */ > -#ifdef CONFIG_SPL_BUILD > -#endif > > /* Provide the MACH_TYPE value the vendor kernel requires */ > #define CONFIG_MACH_TYPE 3063 > > > > > > > > This seems a leftover of 9baa2bce. > Yes. I sent a patch for this as I was testing. > > $ git show 9baa2bce -- include/configs/omap3_cairo.h > commit 9baa2bce28901321d6f62399b5ebeb3fcb8e8a57 > Author: Adam Ford <aford...@gmail.com> > Date: Tue Aug 7 07:08:32 2018 -0500 > > Removed unused references to CONFIG_SERIALx > > After creating CONS_INDEX and migrating a bunch of boards to it, > there are a bunch of defined references to CONFIG_SERIALx which > are not referenced in any C code or #ifdef, so they can now be > removed > > Signed-off-by: Adam Ford <aford...@gmail.com> > > diff --git a/include/configs/omap3_cairo.h b/include/configs/omap3_cairo.h > index 0133416..04bce2f 100644 > --- a/include/configs/omap3_cairo.h > +++ b/include/configs/omap3_cairo.h > @@ -198,8 +198,6 @@ > * function per_clocks_enable(). > */ > #ifdef CONFIG_SPL_BUILD > -#undef CONFIG_SERIAL3 > -#define CONFIG_SERIAL2 > #endif > > /* Provide the MACH_TYPE value the vendor kernel requires */ > > > > > > However, I doubt it used the moveconfig tool. > > > I re-ran the tool against the previous commit. > > > $ git checkout 9baa2bce2890^ > $ tools/moveconfig.py -H SERIAL0 SERIAL1 SERIAL2 SERIAL3 > Clean up headers? [y/n]: y > y > ... > $ git diff -- include/configs/omap3_cairo.h > diff --git a/include/configs/omap3_cairo.h b/include/configs/omap3_cairo.h > index 0133416..ac1a6cb 100644 > --- a/include/configs/omap3_cairo.h > +++ b/include/configs/omap3_cairo.h > @@ -197,10 +197,6 @@ > * are needed and peripheral clocks for UART2 must be enabled in > * function per_clocks_enable(). > */ > -#ifdef CONFIG_SPL_BUILD > -#undef CONFIG_SERIAL3 > -#define CONFIG_SERIAL2 > -#endif > > /* Provide the MACH_TYPE value the vendor kernel requires */ > #define CONFIG_MACH_TYPE 3063 > > > > > > cleanup_one_header() did a good job. > > Is there a particular case that the second path is needed? > Has moveconfig been updated to handle such cases in another way? I only re-sent this because I found it while cleaning up some old branches. I believe at the time I originally wrote it there were many more instances of empty guards (most if which I submitted patches for). I agree that if moveconfig already handles this there is no point in adding a second pass (baring cases of double nesting). There still may be benefit in running something as a one-off pass over all files to remove existing empty guards but there's no need to store such a script in the repository. > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot