Re: [PATCH 2/2] mips: generic: allow not building DTB in

[James Hogan <jhogan@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2909 bytes --]

On Wed, Apr 25, 2018 at 11:16:07PM +0200, Alexandre Belloni wrote:
> Allow not building any DTB in the generic kernel so it gets smaller. This
> is necessary for ocelot because it can be built as a legacy platform that
> needs a built-in DTB and it can also handle a separate DTB once it is
> updated with a more modern bootloader. In the latter case, it is preferable
> to not include any DTB in the kernel image so it is smaller.

Since bootloaders can modify DTs before passing them to the kernel (e.g.
to add cmdline args or memory nodes), that would seem to make it
impossible to have a kernel supporting both legacy bootloader, and a
u-boot which might do that DT tweaking.

Or perhaps its not important for this platform.

> diff --git a/arch/mips/Makefile b/arch/mips/Makefile
> index 5e9fce076ab6..3d3554c13710 100644
> --- a/arch/mips/Makefile
> +++ b/arch/mips/Makefile
> @@ -404,7 +404,7 @@ endif
>  CLEAN_FILES += vmlinux.32 vmlinux.64
>  
>  # device-trees
> -core-$(CONFIG_BUILTIN_DTB) += arch/mips/boot/dts/
> +core-y += arch/mips/boot/dts/

Won't that result in DTBs being built unnecessarily on other platforms
which support BUILTIN_DTB but don't have it enabled?

I suppose the alternative is another Kconfig symbol for when building of
DTBs is needed, selected by BUILTIN_DTB and MIPS_GENERIC.

>  
>  %.dtb %.dtb.S %.dtb.o: | scripts
>  	$(Q)$(MAKE) $(build)=arch/mips/boot/dts arch/mips/boot/dts/$@
> diff --git a/arch/mips/boot/dts/mscc/Makefile b/arch/mips/boot/dts/mscc/Makefile
> index 8982b19504a3..437ec65ec14a 100644
> --- a/arch/mips/boot/dts/mscc/Makefile
> +++ b/arch/mips/boot/dts/mscc/Makefile
> @@ -1,3 +1,3 @@
>  dtb-$(CONFIG_MSCC_OCELOT)	+= ocelot_pcb123.dtb
>  
> -obj-y				+= $(patsubst %.dtb, %.dtb.o, $(dtb-y))
> +obj-($CONFIG_BUILTIN_DTB)	+= $(patsubst %.dtb, %.dtb.o, $(dtb-y))

(This part is already merged in 4.18-rc1 in commit fca3aa166422 ("MIPS:
dts: Avoid unneeded built-in.a in DTS dirs"))

> diff --git a/arch/mips/generic/Kconfig b/arch/mips/generic/Kconfig
> index 6564f18b2012..012f283f99c4 100644
> --- a/arch/mips/generic/Kconfig
> +++ b/arch/mips/generic/Kconfig
> @@ -3,6 +3,7 @@ if MIPS_GENERIC
>  
>  config LEGACY_BOARDS
>  	bool
> +	select BUILTIN_DTB
>  	help
>  	  Select this from your board if the board must use a legacy, non-UHI,
>  	  boot protocol. This will cause the kernel to scan through the list of
> diff --git a/arch/mips/generic/vmlinux.its.S b/arch/mips/generic/vmlinux.its.S
> index 1a08438fd893..9c954f2ae561 100644
> --- a/arch/mips/generic/vmlinux.its.S
> +++ b/arch/mips/generic/vmlinux.its.S
> @@ -21,6 +21,7 @@
>  		};
>  	};
>  
> +#if IS_ENABLED(CONFIG_BUILTIN_DTB)

An #ifdef would do, but no matter if it works. Though shouldn't that be
if !IS_ENABLED (or #ifndef)?

A comment would be great here too to mention why its helpful.

Cheers
James

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]