All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tom Rini <trini@konsulko.com>
To: Adam Ford <aford173@gmail.com>
Cc: "Peng Fan (OSS)" <peng.fan@oss.nxp.com>,
	"u-boot@lists.denx.de" <u-boot@lists.denx.de>,
	"sbabic@denx.de" <sbabic@denx.de>,
	"festevam@gmail.com" <festevam@gmail.com>,
	dl-uboot-imx <uboot-imx@nxp.com>, Peng Fan <peng.fan@nxp.com>,
	"aford@beaconembedded.com" <aford@beaconembedded.com>
Subject: Re: [PATCH] imx8mn_beacon: Fix booting hang and switch to binman
Date: Fri, 19 Nov 2021 15:33:03 -0500	[thread overview]
Message-ID: <20211119203303.GK24579@bill-the-cat> (raw)
In-Reply-To: <CAHCN7x+xw-hGHyiHKMdEcP=krD28mZ7N7pMQAqWfNn1pu-FJ9A@mail.gmail.com>

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

On Fri, Nov 19, 2021 at 06:52:57AM -0600, Adam Ford wrote:
> On Thu, Nov 18, 2021 at 3:30 PM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Nov 18, 2021 at 03:26:40PM -0600, Adam Ford wrote:
> > > On Thu, Nov 18, 2021 at 3:12 PM Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Wed, Nov 17, 2021 at 08:08:10AM -0600, Adam Ford wrote:
> > > >
> > > > > On Fri, Oct 22, 2021 at 2:36 PM Adam Ford <aford173@gmail.com> wrote:
> > > > > >
> > > > > > On Wed, Sep 29, 2021 at 8:26 AM Adam Ford <aford173@gmail.com> wrote:
> > > > > > >
> > > > > > > On Thu, Sep 23, 2021 at 3:03 PM Adam Ford <aford173@gmail.com> wrote:
> > > > > > > >
> > > > > > > > On Tue, Sep 14, 2021 at 9:49 PM Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote:
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > >
> > > > > +Tom Rini
> > > > >
> > > > > > > > > On 2021/9/11 4:19, Adam Ford wrote:
> > > > > > > > > > Somewhere along the line, the board stopped being able to boot.
> > > > > > > > > > Rather than just fixing the issue, let's fix the issue and migrate
> > > > > > > > > > to binman to eliminate a warning when using custom imx tools for
> > > > > > > > > > generating the binary.
> > > > > > > > > >
> > > > > > > >
> > > > > > > > Any chance this can make it into 2021.10?  it's currently broken, and
> > > > > > > > this seems to fix the problem.  i didn't bisect to see where the issue
> > > > > > > > broke, or i would have added a fixes tag.
> > > > > > >
> > > > > > > Stefano or Tom?  could someone comment and/or merge?  the board is
> > > > > > > currently not booting, and this fixes the issue
> > > > > >
> > > > > > It's been over a month, and we missed the 2021.10 release.  Is there
> > > > > > something stopping this from being applied?
> > > > >
> > > > > Tom,
> > > > >
> > > > > Is there any chance you can apply this?  I can't get anyone to respond
> > > > > to me, and it's been over 2 months since the initial submission.
> > > > >
> > > > > thanks,
> > > > >
> > > > > adam
> > > > > >
> > > > > > adam
> > > > > > > >
> > > > > > > > adam
> > > > > > > > > > Signed-off-by: Adam Ford <aford173@gmail.com>
> > > > > > > > > >
> > > > > > > > > > diff --git a/arch/arm/dts/imx8mn-beacon-kit-u-boot.dtsi b/arch/arm/dts/imx8mn-beacon-kit-u-boot.dtsi
> > > > > > > > > > index bbc64a2819..69fd69c8d0 100644
> > > > > > > > > > --- a/arch/arm/dts/imx8mn-beacon-kit-u-boot.dtsi
> > > > > > > > > > +++ b/arch/arm/dts/imx8mn-beacon-kit-u-boot.dtsi
> > > > > > > > > > @@ -4,6 +4,10 @@
> > > > > > > > > >    */
> > > > > > > > > >
> > > > > > > > > >   / {
> > > > > > > > > > +     binman: binman {
> > > > > > > > > > +             multiple-images;
> > > > > > > > > > +     };
> > > > > > > > > > +
> > > > > > > > > >       wdt-reboot {
> > > > > > > > > >               compatible = "wdt-reboot";
> > > > > > > > > >               wdt = <&wdog1>;
> > > > > > > > > > @@ -40,6 +44,9 @@
> > > > > > > > > >   &clk {
> > > > > > > > > >       u-boot,dm-spl;
> > > > > > > > > >       u-boot,dm-pre-reloc;
> > > > > > > > > > +     /delete-property/ assigned-clocks;
> > > > > > > > > > +     /delete-property/ assigned-clock-parents;
> > > > > > > > > > +     /delete-property/ assigned-clock-rates;
> > > > > > > > > >   };
> > > > > > > > > >
> > > > > > > > > >   &gpio1 {
> > > > > > > > > > @@ -99,6 +106,10 @@
> > > > > > > > > >       u-boot,off-on-delay-us = <20000>;
> > > > > > > > > >   };
> > > > > > > > > >
> > > > > > > > > > +&uart2 {
> > > > > > > > > > +     u-boot,dm-spl;
> > > > > > > > > > +};
> > > > > > > > > > +
> > > > > > > > > >   &usdhc1 {
> > > > > > > > > >       u-boot,dm-spl;
> > > > > > > > > >       sd-uhs-sdr104;
> > > > > > > > > > @@ -120,3 +131,122 @@
> > > > > > > > > >   &wdog1 {
> > > > > > > > > >       u-boot,dm-spl;
> > > > > > > > > >   };
> > > > > > > > > > +
> > > > > > > > > > +&binman {
> > > > > > > > > > +      u-boot-spl-ddr {
> > > > > > > > > > +             filename = "u-boot-spl-ddr.bin";
> > > > > > > > > > +             pad-byte = <0xff>;
> > > > > > > > > > +             align-size = <4>;
> > > > > > > > > > +             align = <4>;
> > > > > > > > > > +
> > > > > > > > > > +             u-boot-spl {
> > > > > > > > > > +                     align-end = <4>;
> > > > > > > > > > +             };
> > > > > > > > > > +
> > > > > > > > > > +             blob_1: blob-ext@1 {
> > > > > > > > > > +                     filename = "lpddr4_pmu_train_1d_imem.bin";
> > > > > > > > > > +                     size = <0x8000>;
> > > > > > > > > > +             };
> > > > > > > > > > +
> > > > > > > > > > +             blob_2: blob-ext@2 {
> > > > > > > > > > +                     filename = "lpddr4_pmu_train_1d_dmem.bin";
> > > > > > > > > > +                     size = <0x4000>;
> > > > > > > > > > +             };
> > > > > > > > > > +
> > > > > > > > > > +             blob_3: blob-ext@3 {
> > > > > > > > > > +                     filename = "lpddr4_pmu_train_2d_imem.bin";
> > > > > > > > > > +                     size = <0x8000>;
> > > > > > > > > > +             };
> > > > > > > > > > +
> > > > > > > > > > +             blob_4: blob-ext@4 {
> > > > > > > > > > +                     filename = "lpddr4_pmu_train_2d_dmem.bin";
> > > > > > > > > > +                     size = <0x4000>;
> > > > > > > > > > +             };
> > > > > > > > > > +     };
> > > > > > > > > > +
> > > > > > > > > > +
> > > > > > > > > > +     spl {
> > > > > > > > > > +             filename = "spl.bin";
> > > > > > > > > > +
> > > > > > > > > > +             mkimage {
> > > > > > > > > > +                     args = "-n spl/u-boot-spl.cfgout -T imx8mimage -e 0x912000";
> > > > > > > > > > +
> > > > > > > > > > +                     blob {
> > > > > > > > > > +                             filename = "u-boot-spl-ddr.bin";
> > > > > > > > > > +                     };
> > > > > > > > > > +             };
> > > > > > > > > > +     };
> > > > > > > > > > +
> > > > > > > > > > +     itb {
> > > > > > > > > > +             filename = "u-boot.itb";
> > > > > > > > > > +
> > > > > > > > > > +             fit {
> > > > > > > > > > +                     description = "Configuration to load ATF before U-Boot";
> > > > > > > > > > +                     #address-cells = <1>;
> > > > > > > > > > +                     fit,external-offset = <CONFIG_FIT_EXTERNAL_OFFSET>;
> > > > > > > > > > +
> > > > > > > > > > +                     images {
> > > > > > > > > > +                             uboot {
> > > > > > > > > > +                                     description = "U-Boot (64-bit)";
> > > > > > > > > > +                                     type = "standalone";
> > > > > > > > > > +                                     arch = "arm64";
> > > > > > > > > > +                                     compression = "none";
> > > > > > > > > > +                                     load = <CONFIG_SYS_TEXT_BASE>;
> > > > > > > > > > +
> > > > > > > > > > +                                     uboot_blob: blob-ext {
> > > > > > > > > > +                                             filename = "u-boot-nodtb.bin";
> > > > > > > > > > +                                     };
> > > > > > > > > > +                             };
> > > > > > > > > > +
> > > > > > > > > > +                             atf {
> > > > > > > > > > +                                     description = "ARM Trusted Firmware";
> > > > > > > > > > +                                     type = "firmware";
> > > > > > > > > > +                                     arch = "arm64";
> > > > > > > > > > +                                     compression = "none";
> > > > > > > > > > +                                     load = <0x960000>;
> > > > > > > > > > +                                     entry = <0x960000>;
> > > > > > > > > > +
> > > > > > > > > > +                                     atf_blob: blob-ext {
> > > > > > > > > > +                                             filename = "bl31.bin";
> > > > > > > > > > +                                     };
> > > > > > > > > > +                             };
> > > > > > > > > > +
> > > > > > > > > > +                             fdt {
> > > > > > > > > > +                                     description = "NAME";
> > > > > > > > > > +                                     type = "flat_dt";
> > > > > > > > > > +                                     compression = "none";
> > > > > > > > > > +
> > > > > > > > > > +                                     uboot_fdt_blob: blob-ext {
> > > > > > > > > > +                                             filename = "u-boot.dtb";
> > > > > > > > > > +                                     };
> > > > > > > > > > +                             };
> > > > > > > > > > +                     };
> > > > > > > > > > +
> > > > > > > > > > +                     configurations {
> > > > > > > > > > +                             default = "conf";
> > > > > > > > > > +
> > > > > > > > > > +                             conf {
> > > > > > > > > > +                                     description = "NAME";
> > > > > > > > > > +                                     firmware = "uboot";
> > > > > > > > > > +                                     loadables = "atf";
> > > > > > > > > > +                                     fdt = "fdt";
> > > > > > > > > > +                             };
> > > > > > > > > > +                     };
> > > > > > > > > > +             };
> > > > > > > > > > +     };
> > > > > > > > > > +
> > > > > > > > > > +     imx-boot {
> > > > > > > > > > +             filename = "flash.bin";
> > > > > > > > > > +             pad-byte = <0x00>;
> > > > > > > > > > +
> > > > > > > > > > +             spl: blob-ext@1 {
> > > > > > > > > > +                     offset = <0x0>;
> > > > > > > > > > +                     filename = "spl.bin";
> > > > > > > > > > +             };
> > > > > > > > > > +
> > > > > > > > > > +             uboot: blob-ext@2 {
> > > > > > > > > > +                     offset = <0x58000>;
> > > > > > > > > > +                     filename = "u-boot.itb";
> > > > > > > > > > +             };
> > > > > > > > > > +     };
> > > > > > > > > > +};
> > > > > > > > > > diff --git a/arch/arm/mach-imx/imx8m/Kconfig b/arch/arm/mach-imx/imx8m/Kconfig
> > > > > > > > > > index 8fedf45987..9a1eb108e4 100644
> > > > > > > > > > --- a/arch/arm/mach-imx/imx8m/Kconfig
> > > > > > > > > > +++ b/arch/arm/mach-imx/imx8m/Kconfig
> > > > > > > > > > @@ -116,6 +116,7 @@ config TARGET_IMX8MM_BEACON
> > > > > > > > > >
> > > > > > > > > >   config TARGET_IMX8MN_BEACON
> > > > > > > > > >       bool "imx8mn Beacon Embedded devkit"
> > > > > > > > > > +     select BINMAN
> > > > > > > > > >       select IMX8MN
> > > > > > > > > >       select SUPPORT_SPL
> > > > > > > > > >       select IMX8M_LPDDR4
> > > > > > > > > > diff --git a/board/beacon/imx8mn/imximage-8mn-lpddr4.cfg b/board/beacon/imx8mn/imximage-8mn-lpddr4.cfg
> > > > > > > > > > new file mode 100644
> > > > > > > > > > index 0000000000..7286b26494
> > > > > > > > > > --- /dev/null
> > > > > > > > > > +++ b/board/beacon/imx8mn/imximage-8mn-lpddr4.cfg
> > > > > > > > > > @@ -0,0 +1,10 @@
> > > > > > > > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > > > > > > > +/*
> > > > > > > > > > + * Copyright 2021 NXP
> > > > > > > > > > + */
> > > > > > > > > > +
> > > > > > > > > > +#define __ASSEMBLY__
> > > > > > > > > > +
> > > > > > > > > > +ROM_VERSION  v2
> > > > > > > > > > +BOOT_FROM    sd
> > > > > > > > > > +LOADER               u-boot-spl-ddr.bin      0x912000
> > > > > > > > > > diff --git a/configs/imx8mn_beacon_defconfig b/configs/imx8mn_beacon_defconfig
> > > > > > > > > > index 5296204aad..f2515874a2 100644
> > > > > > > > > > --- a/configs/imx8mn_beacon_defconfig
> > > > > > > > > > +++ b/configs/imx8mn_beacon_defconfig
> > > > > > > > > > @@ -26,9 +26,9 @@ CONFIG_LTO=y
> > > > > > > > > >   CONFIG_FIT=y
> > > > > > > > > >   CONFIG_FIT_EXTERNAL_OFFSET=0x3000
> > > > > > > > > >   CONFIG_SPL_LOAD_FIT=y
> > > > > > > > > > -CONFIG_SPL_FIT_GENERATOR="arch/arm/mach-imx/mkimage_fit_atf.sh"
> > > > > > > > > > +# CONFIG_USE_SPL_FIT_GENERATOR is not set
> > > > > > > > > >   CONFIG_OF_SYSTEM_SETUP=y
> > > > > > > > > > -CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=arch/arm/mach-imx/imx8m/imximage-8mn-lpddr4.cfg"
> > > > > > > > > > +CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/beacon/imx8mn/imximage-8mn-lpddr4.cfg"
> > > > > > > > > >   CONFIG_DEFAULT_FDT_FILE="imx8mn-beacon-kit.dtb"
> > > > > > > > > >   CONFIG_ARCH_MISC_INIT=y
> > > > > > > > > >   CONFIG_SPL_BOARD_INIT=y
> > > > > > > > > > @@ -36,6 +36,7 @@ CONFIG_SPL_BOOTROM_SUPPORT=y
> > > > > > > > > >   CONFIG_SPL_SEPARATE_BSS=y
> > > > > > > > > >   CONFIG_SPL_I2C=y
> > > > > > > > > >   CONFIG_SPL_POWER=y
> > > > > > > > > > +CONFIG_SPL_WATCHDOG=y
> > > > > > > > > >   CONFIG_HUSH_PARSER=y
> > > > > > > > > >   CONFIG_SYS_PROMPT="u-boot=> "
> > > > > > > > > >   # CONFIG_BOOTM_NETBSD is not set
> > > > > > > > > > @@ -62,7 +63,6 @@ CONFIG_CMD_EXT4_WRITE=y
> > > > > > > > > >   CONFIG_CMD_FAT=y
> > > > > > > > > >   CONFIG_OF_CONTROL=y
> > > > > > > > > >   CONFIG_SPL_OF_CONTROL=y
> > > > > > > > > > -CONFIG_OF_SPL_REMOVE_PROPS="interrupt-parent interrupts"
> > > > > > > > > >   CONFIG_ENV_IS_IN_MMC=y
> > > > > > > > > >   CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> > > > > > > > > >   CONFIG_SYS_MMC_ENV_DEV=2
> > > > > > > > > > @@ -106,6 +106,7 @@ CONFIG_PHY_GIGE=y
> > > > > > > > > >   CONFIG_FEC_MXC=y
> > > > > > > > > >   CONFIG_MII=y
> > > > > > > > > >   CONFIG_PINCTRL=y
> > > > > > > > > > +CONFIG_SPL_PINCTRL=y
> > > > > > > > > >   CONFIG_PINCTRL_IMX8M=y
> > > > > > > > > >   CONFIG_DM_PMIC=y
> > > > > > > > > >   # CONFIG_SPL_PMIC_CHILDREN is not set
> > > > > > > > > > @@ -122,7 +123,9 @@ CONFIG_SPI=y
> > > > > > > > > >   CONFIG_DM_SPI=y
> > > > > > > > > >   CONFIG_NXP_FSPI=y
> > > > > > > > > >   CONFIG_SYSRESET=y
> > > > > > > > > > +CONFIG_SPL_SYSRESET=y
> > > > > > > > > >   CONFIG_SYSRESET_PSCI=y
> > > > > > > > > > +CONFIG_SYSRESET_WATCHDOG=y
> > > > > > > > > >   CONFIG_DM_THERMAL=y
> > > > > > > > > >   CONFIG_USB=y
> > > > > > > > > >   # CONFIG_SPL_DM_USB is not set
> > > > > > > > > > @@ -134,3 +137,4 @@ CONFIG_USB_GADGET_PRODUCT_NUM=0xa4a5
> > > > > > > > > >   CONFIG_CI_UDC=y
> > > > > > > > > >   CONFIG_SDP_LOADADDR=0x0
> > > > > > > > > >   CONFIG_OF_LIBFDT_OVERLAY=y
> > > > > > > > > > +CONFIG_IMX_WATCHDOG=y
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > Reviewed-by: Peng Fan <peng.fan@nxp.com>
> > > >
> > > > This introduces failure to build now:
> > > > https://source.denx.de/u-boot/u-boot/-/jobs/352491
> > >
> > > This goes to another discussion [1] because the binman requires some
> > > file outside of U-Boot in order to create the binary.
> > > If we could let the CI build with missing files, that would be helpful.
> > >
> > > [1] - https://lore.kernel.org/all/20211109194224.GB24579@bill-the-cat/
> >
> > Oh this fun.. :(
> 
> Does that mean I should find a solution without binman?

I think it depends on how the RFC to allow binman to handle dummy files
goes.

-- 
Tom

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

  reply	other threads:[~2021-11-19 20:33 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-10 20:19 [PATCH] imx8mn_beacon: Fix booting hang and switch to binman Adam Ford
2021-09-15  2:49 ` Peng Fan (OSS)
2021-09-23 20:03   ` Adam Ford
2021-09-29 13:26     ` Adam Ford
2021-10-22 19:36       ` Adam Ford
2021-11-17 14:08         ` Adam Ford
2021-11-18 21:12           ` Tom Rini
2021-11-18 21:26             ` Adam Ford
2021-11-18 21:30               ` Tom Rini
2021-11-19 12:52                 ` Adam Ford
2021-11-19 20:33                   ` Tom Rini [this message]
2021-11-17 14:10 ` Fabio Estevam
2022-01-08  2:08 ` Tom Rini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211119203303.GK24579@bill-the-cat \
    --to=trini@konsulko.com \
    --cc=aford173@gmail.com \
    --cc=aford@beaconembedded.com \
    --cc=festevam@gmail.com \
    --cc=peng.fan@nxp.com \
    --cc=peng.fan@oss.nxp.com \
    --cc=sbabic@denx.de \
    --cc=u-boot@lists.denx.de \
    --cc=uboot-imx@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.