[PATCH 1/2] mips: mscc: build FIT image for Ocelot

[PATCH 1/2] mips: mscc: build FIT image for Ocelot

[Alexandre Belloni]

Ocelot now has a u-boot port, allow building FIT images instead of relying
on the legacy detection and builtin DTB.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 arch/mips/boot/dts/mscc/Makefile            |  2 +-
 arch/mips/generic/Kconfig                   | 12 +++++++++--
 arch/mips/generic/Platform                  |  1 +
 arch/mips/generic/board-ocelot_pcb123.its.S | 23 +++++++++++++++++++++
 4 files changed, 35 insertions(+), 3 deletions(-)
 create mode 100644 arch/mips/generic/board-ocelot_pcb123.its.S

diff --git a/arch/mips/boot/dts/mscc/Makefile b/arch/mips/boot/dts/mscc/Makefile
index c51164537c02..8982b19504a3 100644
--- a/arch/mips/boot/dts/mscc/Makefile
+++ b/arch/mips/boot/dts/mscc/Makefile
@@ -1,3 +1,3 @@
-dtb-$(CONFIG_LEGACY_BOARD_OCELOT)	+= ocelot_pcb123.dtb
+dtb-$(CONFIG_MSCC_OCELOT)	+= ocelot_pcb123.dtb
 
 obj-y				+= $(patsubst %.dtb, %.dtb.o, $(dtb-y))
diff --git a/arch/mips/generic/Kconfig b/arch/mips/generic/Kconfig
index ba9b2c8cce68..6564f18b2012 100644
--- a/arch/mips/generic/Kconfig
+++ b/arch/mips/generic/Kconfig
@@ -35,13 +35,13 @@ config LEGACY_BOARD_OCELOT
 	depends on LEGACY_BOARD_SEAD3=n
 	select LEGACY_BOARDS
 	select MSCC_OCELOT
+	select SYS_HAS_EARLY_PRINTK
+	select USE_GENERIC_EARLY_PRINTK_8250
 
 config MSCC_OCELOT
 	bool
 	select GPIOLIB
 	select MSCC_OCELOT_IRQ
-	select SYS_HAS_EARLY_PRINTK
-	select USE_GENERIC_EARLY_PRINTK_8250
 
 comment "FIT/UHI Boards"
 
@@ -65,6 +65,14 @@ config FIT_IMAGE_FDT_XILFPGA
 	  Enable this to include the FDT for the MIPSfpga platform
 	  from Imagination Technologies in the FIT kernel image.
 
+config FIT_IMAGE_FDT_OCELOT_PCB123
+	bool "Include FDT for Microsemi Ocelot PCB123"
+	select MSCC_OCELOT
+	help
+	  Enable this to include the FDT for the Ocelot PCB123 platform
+	  from Microsemi in the FIT kernel image.
+	  This require u-boot on the platform.
+
 config VIRT_BOARD_RANCHU
 	bool "Support Ranchu platform for Android emulator"
 	help
diff --git a/arch/mips/generic/Platform b/arch/mips/generic/Platform
index 0dd0d5d460a5..879cb80396c8 100644
--- a/arch/mips/generic/Platform
+++ b/arch/mips/generic/Platform
@@ -16,4 +16,5 @@ all-$(CONFIG_MIPS_GENERIC)	:= vmlinux.gz.itb
 its-y					:= vmlinux.its.S
 its-$(CONFIG_FIT_IMAGE_FDT_BOSTON)	+= board-boston.its.S
 its-$(CONFIG_FIT_IMAGE_FDT_NI169445)	+= board-ni169445.its.S
+its-$(CONFIG_FIT_IMAGE_FDT_OCELOT_PCB123) += board-ocelot_pcb123.its.S
 its-$(CONFIG_FIT_IMAGE_FDT_XILFPGA)	+= board-xilfpga.its.S
diff --git a/arch/mips/generic/board-ocelot_pcb123.its.S b/arch/mips/generic/board-ocelot_pcb123.its.S
new file mode 100644
index 000000000000..f6f54ff6d64c
--- /dev/null
+++ b/arch/mips/generic/board-ocelot_pcb123.its.S
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
+/ {
+	images {
+		fdt@ocelot_pcb123 {
+			description = "MSCC Ocelot PCB123 Device Tree";
+			data = /incbin/("boot/dts/mscc/ocelot_pcb123.dtb");
+			type = "flat_dt";
+			arch = "mips";
+			compression = "none";
+			hash@0 {
+				algo = "sha1";
+			};
+		};
+	};
+
+	configurations {
+		conf@ocelot_pcb123 {
+			description = "Ocelot Linux kernel";
+			kernel = "kernel@0";
+			fdt = "fdt@ocelot_pcb123";
+		};
+	};
+};
-- 
2.17.0

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

[Alexandre Belloni]

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.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 arch/mips/Kconfig                | 1 -
 arch/mips/Makefile               | 2 +-
 arch/mips/boot/dts/mscc/Makefile | 2 +-
 arch/mips/generic/Kconfig        | 1 +
 arch/mips/generic/vmlinux.its.S  | 2 ++
 5 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 225c95da23ce..61057761d096 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -81,7 +81,6 @@ choice
 config MIPS_GENERIC
 	bool "Generic board-agnostic MIPS kernel"
 	select BOOT_RAW
-	select BUILTIN_DTB
 	select CEVT_R4K
 	select CLKSRC_MIPS_GIC
 	select COMMON_CLK
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/
 
 %.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))
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)
 	configurations {
 		default = "conf@default";
 
@@ -29,4 +30,5 @@
 			kernel = "kernel@0";
 		};
 	};
+#endif
 };
-- 
2.17.0

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

[Alexandre Belloni]

Hi,

On 25/04/2018 23:16:07+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.
> 
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
>  arch/mips/Kconfig                | 1 -
>  arch/mips/Makefile               | 2 +-
>  arch/mips/boot/dts/mscc/Makefile | 2 +-
>  arch/mips/generic/Kconfig        | 1 +
>  arch/mips/generic/vmlinux.its.S  | 2 ++
>  5 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 225c95da23ce..61057761d096 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -81,7 +81,6 @@ choice
>  config MIPS_GENERIC
>  	bool "Generic board-agnostic MIPS kernel"
>  	select BOOT_RAW
> -	select BUILTIN_DTB
>  	select CEVT_R4K
>  	select CLKSRC_MIPS_GIC
>  	select COMMON_CLK
> 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/
>  
>  %.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))

I made a typo here, I'll resend after waiting for a few comments.

> 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)
>  	configurations {
>  		default = "conf@default";
>  
> @@ -29,4 +30,5 @@
>  			kernel = "kernel@0";
>  		};
>  	};
> +#endif
>  };
> -- 
> 2.17.0
> 

-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

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

[Paul Burton]

Hi Alexandre,

On Thu, Apr 26, 2018 at 12:01:49AM +0200, Alexandre Belloni wrote:
> > 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/
> >  
> >  %.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))
> 
> I made a typo here, I'll resend after waiting for a few comments.

Are you happy with this series apart from fixing up the above?

If so I'm happy to s/(\$/$(/ whilst applying this.

Thanks,
    Paul

Re: [PATCH 1/2] mips: mscc: build FIT image for Ocelot

[James Hogan]

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

On Wed, Apr 25, 2018 at 11:16:06PM +0200, Alexandre Belloni wrote:
> +config FIT_IMAGE_FDT_OCELOT_PCB123
> +	bool "Include FDT for Microsemi Ocelot PCB123"
> +	select MSCC_OCELOT
> +	help
> +	  Enable this to include the FDT for the Ocelot PCB123 platform
> +	  from Microsemi in the FIT kernel image.
> +	  This require u-boot on the platform.

nit: s/require/requires/

Reviewed-by: James Hogan <jhogan@kernel.org>

Cheers
James

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

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

[James Hogan]

[-- 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 --]

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

[Alexandre Belloni]

On 20/06/2018 23:08:08+0100, James Hogan wrote:
> 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.
> 

It is still possible to generate a FIT image containing a kernel with a
built in DTB and a separate DTB:

> > 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.
> 
Does it matter? On arm all the DTBs for selected platforms are always
built. It is not adding much to the build time.

IIRC, without that change, it is not possible to build any dtb without
having CONFIG_BUILTIN_DTB set.

> > 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)?
> 

No, that is exactly the configuration I need to remove when no DTB has
been built in the kernel. In that case, it doesn't make sense to provide
a configuration using only the kernel.

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





-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com