Re: [EXT] [PATCH] ARM: imx: romapi: Repair FlexSPI NOR boot offset
Hi Marek,
On Wed, 2022-03-23 at 22:16 +0100, Marek Vasut wrote:
> Caution: EXT Email
>
> On 3/23/22 03:42, Ye Li wrote:
> >
> > Hi Marek,
> Hi,
>
> >
> > >
> > > >
> > > > >
> > > > > diff --git a/arch/arm/mach-imx/spl_imx_romapi.c
> > > > > b/arch/arm/mach-
> > > > > imx/spl_imx_romapi.c
> > > > > index d827de375a6..c47f5a6bdb4 100644
> > > > > --- a/arch/arm/mach-imx/spl_imx_romapi.c
> > > > > +++ b/arch/arm/mach-imx/spl_imx_romapi.c
> > > > > @@ -38,14 +38,8 @@ ulong spl_romapi_raw_seekable_read(u32
> > > > > offset,
> > > > > u32
> > > > > size, void *buf)
> > > > >
> > > > > ulong __weak spl_romapi_get_uboot_base(u32 image_offset,
> > > > > u32
> > > > > rom_bt_dev)
> > > > > {
> > > > > - u32 offset;
> > > > > -
> > > > > - if (((rom_bt_dev >> 16) & 0xff)
> > > > > == BT_DEV_TYPE_FLEXSPINOR)
> > > > > - offset =
> > > > > CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR
> > > > > *
> > > > > 512;
> > > > > - else
> > > > > - offset = image_offset +
> > > > > CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512 - 0x8000;
> > > > > -
> > > > > - return offset;
> > > > > + return image_offset +
> > > > > + (CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR *
> > > > > 512 -
> > > > > 0x8000);
> > > > > }
> > > > The change is problematic to flexspi.
> > > Yes, I need this change to get boot from flexspi working on
> > > i.MX8MP,
> > > without this change writing flash.bin to flexspi results in
> > > unbootable
> > > system.
> > >
> > To support boot from flexspi, please try another two changes.
> > 1. Modify board/freescale/imx8mp_evk/imximage-8mp-lpddr4.cfg, set
> > "BOOT_FROM" to "fspi". (This change is unnecessary on 8MP after
> > switch to binman)
> I already use binman and I need to be able to boot from both SD and
> FlexSPI, with this patch I can do just that.
>
> >
> > 2. Update the u-boot.itb offset in imx8mp-u-boot.dtsi, set the
> > offset
> > to 0x5f000. The previous offset 0x58000 is for SD, calculated by
> > 0x6 - 0x8000 (32KB image offset).
> >
> > uboot: blob-ext@2 {
> > filename = "u-boot.itb";
> > offset = <0x5f000>;
> > };
> But that breaks booting from SD card for me ?
Do you want to use one flash.bin for both SD and flexspi?
When first introduced 8m support by imx8mimage.c, we expected the u-
boot.itb at same device offset (0x6) on SD/emmc and flexspi. The
imx8mimage will calculate the offset inside the flash.bin automatically
according to different IVT offset. The ROMAPI driver also works
correspondingly.
After using binman, the u-boot.itb offset inside the flash.bin has to
be manually set in this DTS node. To follow the original design, this
offset should be different. That's why I asked to update this dts node
for flexspi.
If you change the ROM API driver, that will break our design. You can
try to overwrite spl_romapi_get_uboot_base for your board only.
Best regards,
Ye Li
Re: [PATCH 19/23] video: Support showing the U-Boot logo
Hi Tim,
On Tue, 15 Mar 2022 at 10:14, Tim Harvey wrote:
>
> On Fri, Nov 19, 2021 at 12:28 PM Simon Glass wrote:
> >
> > Show the U-Boot logo by default. This is only 7KB in size so seems like
> > a useful default for boards that enable a display.
> >
> > If SPLASH_SCREEN is enabled, it is not enabled by default, so as not to
> > conflict with that feature.
> >
> > Also disable it for tests, since we don't want to complicate the output.
> >
> > Signed-off-by: Simon Glass
> > ---
> >
> > drivers/video/Kconfig | 1 +
> > drivers/video/Makefile| 3 +++
> > drivers/video/sandbox_sdl.c | 2 ++
> > drivers/video/u_boot_logo.bmp | Bin 0 -> 6932 bytes
> > drivers/video/video-uclass.c | 26
> > include/video.h | 2 ++
> > scripts/Makefile.lib | 21 +
> > test/dm/video.c | 43 +++---
> > 8 files changed, 89 insertions(+), 9 deletions(-)
> > create mode 100644 drivers/video/u_boot_logo.bmp
> >
> > diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> > index 7a73ecc1f40..e601b47806b 100644
> > --- a/drivers/video/Kconfig
> > +++ b/drivers/video/Kconfig
> > @@ -17,6 +17,7 @@ config DM_VIDEO
> > config VIDEO_LOGO
> > bool "Show the U-Boot logo on the display"
> > depends on DM_VIDEO
> > + select VIDEO_BMP_RLE8
> > help
> > This enables showing the U-Boot logo on the display when a video
> > device is probed. It appears at the top right. The logo itself is
> > at
> > diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> > index 8956b5f9b00..4038395b128 100644
> > --- a/drivers/video/Makefile
> > +++ b/drivers/video/Makefile
> > @@ -17,6 +17,9 @@ obj-$(CONFIG_DM_VIDEO) += video_bmp.o
> > obj-$(CONFIG_PANEL) += panel-uclass.o
> > obj-$(CONFIG_DM_PANEL_HX8238D) += hx8238d.o
> > obj-$(CONFIG_SIMPLE_PANEL) += simple_panel.o
> > +
> > +obj-$(CONFIG_VIDEO_LOGO) += u_boot_logo.o
> > +
> > endif
> >
> > obj-${CONFIG_EXYNOS_FB} += exynos/
> > diff --git a/drivers/video/sandbox_sdl.c b/drivers/video/sandbox_sdl.c
> > index 2afe66fab1a..9081c7da62e 100644
> > --- a/drivers/video/sandbox_sdl.c
> > +++ b/drivers/video/sandbox_sdl.c
> > @@ -82,12 +82,14 @@ static void set_bpp(struct udevice *dev, enum
> > video_log2_bpp l2bpp)
> >
> > int sandbox_sdl_set_bpp(struct udevice *dev, enum video_log2_bpp l2bpp)
> > {
> > + struct video_uc_plat *uc_plat = dev_get_uclass_plat(dev);
> > int ret;
> >
> > if (device_active(dev))
> > return -EINVAL;
> > sandbox_sdl_remove_display();
> >
> > + uc_plat->hide_logo = true;
> > set_bpp(dev, l2bpp);
> >
> > ret = device_probe(dev);
> > diff --git a/drivers/video/u_boot_logo.bmp b/drivers/video/u_boot_logo.bmp
> > new file mode 100644
> > index
> > ..47f1e9b99789584d2f6dd71e954b51927b35d783
> > GIT binary patch
> > literal 6932
> > zcmb7J3sjWXwf?sM%m9M}3aA(*B(X##snNz*U$IeRG>LimKpwtD@qvhnD4<9bH9<{c
> > zuq4JwK}|`-nLKJMq^L > z`OZH3?7h!E$7#<54T15u)dZ|(>zL;!@P@FRI}mW$dVc?6U;U=doSVeY|Ld>M|BzlY
> > zU9fTnu=epjSoH)D$KUFieXwSh32UFUVBJ&Rh=1COl}`m>&9h#JdwL+&%nnBEvw>Lm
> > zd|#}e>xWg(55mf4?nT1fepoYa2v*F!7i;GaMeMx$urA_mteig#@e4w+EMg?qN8W>k
> > zsCy9`^&r+pkHE^vM-U$qj%Cq5!TLoHAYsvi*sx?2Vi!)v#-$G-e(6{&T{IQzmyg5p
> > zrPHxt#ZQqKJ07viXJF~_S=h8{A`(_Vh83}MusC);HmsS7#I@6~aosd5i(7zA@sDHq
> > z>L?_{&q89tlUTBLA(pIJj*SU(uxwo{HYd)*qWCx@ZQ|HXQCPZREjA`aW8sEP*qpo&
> > zNn00T>E>i4rz}BC(l%^LU5-UbJFz)!1(LSMB06~&w(MAm > z`;e5e3DK#qV9N`ek-RGj3%9?DZJEi~oVg8))3Xts{w7lQZo`(nsff > za#t?4Wu+tKrA(ym--)!WJ&4(J9Lf7%#DdI|*m~e4MD8iTwu4znd3isg_BoLH$^pc@
> > z_#V<;J&5gxUPeULhuHesAw=yjLdxN6q-MW?B?nI<`oKqsICvImKYtV3bB > z^6!!U##=}|l85<+E+XyCV~BX|61E>bj`c@A#18ujL>?|f`de=!D*JQHJN!pvymbnx
> > zx$k2B&;J7vIajg$*hwsSy$U<>-bebe4>9M63p > z9Q_j_-uwa!j$X(1{4&1Au8_+>^%7qmgLnTqo5cu$Lq1{ > zL<1JR-G~)$H(-bJW30)qMY`itEdFH^b~;P3B>yj1$zR5M7qRr6udwXhzasA4udwTb
> > z%UFK08LLnJ6}vwy! > z-;76Z`g49@7P=PCnWy*Yi#_@4ZQH%YuI$gjorny
> > z*j#uMnP=;eRCEhlK5E0B^VgC5+ji_b_a*jy+<>jc9oTmEAK3kI3wHgk2`T3~k@?Br
> > zkb3?$Qa)+No(q3R+Q&Nf{{CyEUFg8Rk{j6m`+wlYPu > zu(MRho=fe>xY&igmpkynrGH}IXaB&i%iVbK@;~un8OMC~UwG-WZe*7I3t9Z_`NO|3
> > z69F=l`CuxfPl!xqYcht5^qI`o1pHVg@;eR>%TM`z7!$~On61&2{+WzsYZP14WfWTv
> > zVwOC}Z#XGWiD=<$kHB+mjbLjy=E`t>??;4$@%tl0$&dKGkJ2$jy^rmCu|)3WHx!~R
> > zmr%CuCfAGUFD!Cr_N!@gLjy!iGSNI8+LSRv;j1`{c4zNhJy!E!J9vIZcGl)87Pc+n
> > z@q2eh`kRP+H*rGXB@!lQymdJ#!}^KL@zJ4OPY#4dBuL)z(&dVbu?RHp%x+k8`9fxK
> > z#5S`o*%k>Q!cc`UPTH-y+>-JBB5}UD+>~*gAyBUQ=< > z5H&VXnoT-PU%hjz%B0H;3HBp~#+kHIX&~jcjL<}KG#%#doaOOVAdga{#i?=c$Hkqa
> > z)L71uN*Po8u~RwTq7IC~w3fHXMsnlCO!;Z@HOFRn@Dn& > zl4;QfMcYi8CcD`>#S!oDJJ%+I4SXMwD)uvJt7rLb7R}b9dji+_oS}_bW|Q+d`(suA
> > zI<3uqyV!^oD7V=TBtB=9Fh2<<>L6G}tkgM-sAjpLbP1yd!7$$?L6lXtQTSn8y%r=}
> > zl#W{{t0c8xnWg$C+5eE=NgnJFe;GvVf%F=p2b0q+8f||@=`^0Z?!|BTci2-DwoNh>
> > zRt&K|SJ>eat@>H)zsm1(-^U(+z9Pd&zd@VRN-#|mNl>^mJ-A%4O)e`=sYoUzBZyS0
> > zkjF@*Lf*~3qDS^3_m=
Re: [PATCH v2] core: devres: optionally build devres into the SPL
Hi Angus, On Sat, 19 Mar 2022 at 09:18, Angus Ainslie wrote: > > On 2022-03-19 07:15, Angus Ainslie wrote: > > Hi Simon, > > > > On 2022-03-18 15:41, Simon Glass wrote: > >> Hi Angus, > >> > >> On Tue, 1 Mar 2022 at 07:58, Simon Glass wrote: > >>> > >>> On Mon, 28 Feb 2022 at 13:33, Angus Ainslie wrote: > >>> > > >>> > Add a CONFIG_SPL_DEVRES option > >>> > > >>> > Signed-off-by: Angus Ainslie > >>> > --- > >>> > > >>> > Changes since v1: > >>> > > >>> > Instead of gaurding the source add an SPL_DEVRES option > >>> > > >>> > drivers/core/Kconfig | 13 + > >>> > drivers/core/Makefile | 2 +- > >>> > 2 files changed, 14 insertions(+), 1 deletion(-) > >> > >> Unfortunately this break the tests, e.g. building sandbox_spl - can > >> you please take a look? > >> > >> sandbox_spl: +make O=build-sandbox_spl -s sandbox_spl_defconfig > >> +make O=build-sandbox_spl -s -j4 > >> /usr/bin/ld: /tmp/ccsYY64W.ltrans0.ltrans.o: in function > >> `device_unbind': > >> build-sandbox_spl/spl/../../drivers/core/device-remove.c:120: > >> undefined reference to `devres_release_all' > >> /usr/bin/ld: /tmp/ccsYY64W.ltrans0.ltrans.o: in function > >> `device_free': > >> build-sandbox_spl/spl/../../drivers/core/device-remove.c:157: > >> undefined reference to `devres_release_probe' > >> collect2: error: ld returned 1 exit status > >> make[3]: *** [../scripts/Makefile.spl:509: spl/u-boot-spl] Error 1 > >> make[2]: *** [Makefile:2094: spl/u-boot-spl] Error 2 > >> make[1]: *** [Makefile:177: sub-make] Error 2 > >> Exit code: 2 > >> > > > > So the problem here is that CONFIG_DM_DEVICE_REMOVE can be defined > > without CONFIG_DEVRES being defined. > > > > Is there a way to make DM_DEVICE_REMOVE dependant on DEVRES or do I > > need to add "&& CONFIG_IS_ENABLED(DEVRES)" everywhere that I find > > "CONFIG_IS_ENABLED(DM_DEVICE_REMOVE)" ? > > > > The test failure can be "fixed" using this > > diff --git a/configs/sandbox_noinst_defconfig > b/configs/sandbox_noinst_defconfig > index ec912cf0ec8..69c97921744 100644 > --- a/configs/sandbox_noinst_defconfig > +++ b/configs/sandbox_noinst_defconfig > @@ -105,6 +105,7 @@ CONFIG_SYSCON=y > CONFIG_SPL_SYSCON=y > CONFIG_DEVRES=y > CONFIG_DEBUG_DEVRES=y > +CONFIG_SPL_DEVRES=y > # CONFIG_SPL_SIMPLE_BUS is not set > CONFIG_ADC=y > CONFIG_ADC_SANDBOX=y > diff --git a/configs/sandbox_spl_defconfig > b/configs/sandbox_spl_defconfig > index 1687ccf4530..2fcda46dfd1 100644 > --- a/configs/sandbox_spl_defconfig > +++ b/configs/sandbox_spl_defconfig > @@ -107,6 +107,7 @@ CONFIG_SYSCON=y > CONFIG_SPL_SYSCON=y > CONFIG_DEVRES=y > CONFIG_DEBUG_DEVRES=y > +CONFIG_SPL_DEVRES=y > # CONFIG_SPL_SIMPLE_BUS is not set > CONFIG_ADC=y > CONFIG_ADC_SANDBOX=y > > But is that the right way based on the questions above ? I just sent a patch to update DEVRES to only be enabled on SPL. I think DM_DEVICE_REMOVE should not depend on DEVRES., but perhaps the other way around would make sense? Regards, Simon
Re: [PATCH v3 12/12] rockchip: fix boot_devices constants
Hi Johan,
On Fri, 18 Mar 2022 at 09:17, Johan Jonker wrote:
>
> Hi Simon,
>
> On 3/12/22 03:24, Simon Glass wrote:
> > Hi Johan,
> >
> > On Thu, 3 Mar 2022 at 16:53, Johan Jonker wrote:
> >>
> >> The DT node name pattern in mmc-controller.yaml for mmc
> >> is "^mmc(@.*)?$". The Rockchip mmc nodes have been synced
> >> with Linux, so update the boot_devices constants as well.
> >>
> >> Signed-off-by: Johan Jonker
> >> Reviewed-by: Simon Glass
> >> ---
> >> arch/arm/mach-rockchip/rk3188/rk3188.c | 4 ++--
> >> arch/arm/mach-rockchip/rk322x/rk322x.c | 4 ++--
> >> arch/arm/mach-rockchip/rk3288/rk3288.c | 4 ++--
> >> arch/arm/mach-rockchip/rk3328/rk3328.c | 4 ++--
> >> arch/arm/mach-rockchip/rk3368/rk3368.c | 4 ++--
> >> 5 files changed, 10 insertions(+), 10 deletions(-)
> >
> > This series seems to work OK on snow. But on jerry I get:
> >
> > U-Boot 2022.04-rc3-00077-g088bbcb3fad (Mar 11 2022 - 15:59:54 -0700)
> >
> > Model: Google Jerry
> > DRAM: 2 GiB
> > PMIC: RK808
> > Core: 256 devices, 25 uclasses, devicetree: separate
> > MMC: mmc@ff0c: 1, mmc@ff0d: 2, mmc@ff0f: 0
> > Loading Environment from nowhere... OK
>
> > cros_ec_spi_command: Returned status 6
> > cros_ec_check_version: ERROR: old EC interface not supported
> > Failed to probe keyboard 'keyboard-controller'
>
> When I compare the ec node in the dtb before and after it looks like
> "ec-interrupt" is missing.
>
> Could you test/confirm by adding it to rk3288-veyron-jerry-u-boot.dtsi?
Yes that looks like it.
>
> It originates from rk3288-veyron-chromebook.dtsi, but what should be the
> u-boot specific correct location now for the other rk3288 veyron boards?
>
> &cros_ec {
> ec-interrupt = <&gpio7 7 GPIO_ACTIVE_LOW>;
> };
>
> ===
>
> This property is not in the binding:
> Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
>
> What to do with it? (no u-boot prefix)
There is a standard way of doing interrupts, so could the code be
updated to use that? For now a u-boot, prefix would be OK, I suppose,
to unblock your work.
>
> ===
>
> dtc -s -I dtb -O dts rk3288-veyron-jerry-before.dtb >
> rk3288-veyron-jerry-before-sort.dts
>
> dtc -s -I dtb -O dts rk3288-veyron-jerry-after.dtb >
> rk3288-veyron-jerry-after-sort.dts
>
> ec@0 {
> compatible = "google,cros-ec-spi";
> ===
> ec-interrupt = < 0x07 0x07 0x01 >;
> ===
> google,cros-ec-spi-pre-delay = < 0x1e >;
> interrupt-parent = < 0x07 >;
> interrupts = < 0x07 0x08 >;
> pinctrl-0 = < 0x20 >;
> pinctrl-names = "default";
> reg = < 0x00 >;
> spi-max-frequency = < 0x2dc6c0 >;
>
> i2c-tunnel {
>
> };
>
> keyboard-controller {
>
> };
> };
>
>
> > rockchip_rk3288_vop vop@ff93: failed to get ahb reset (ret=-524)
> > rockchip_rk3288_vop vop@ff93: failed to get ahb reset (ret=-524)
>
> Could you test before and after Linux sync and with reg size?
>
>
> vopb: vop@ff93 {
> compatible = "rockchip,rk3288-vop";
>
> reg = <0xff93 0x19c>, <0xff931000 0x1000>;
>
> vs.
> reg = <0xff93 0x19c>;
>
> }
>
> ===
>
> From rk_vop.c:
>
> priv->regs = (struct rk3288_vop *)dev_read_addr(dev);
>
> ===
>
> From rockchip-vop.yaml:
>
> reg:
> minItems: 1
> items:
> - description:
> Must contain one entry corresponding to the base address and
> length
> of the register space.
> - description:
> Can optionally contain a second entry corresponding to
> the CRTC gamma LUT address.
>
> ===
>
> Does vop driver need a fix to comply with binding?
Quite possibly.
>
>
> > In:serial
> >
> > So it seems the EC and video devicetree are broken in some way, or
> > drivers need updating for new bindings?
>
> Could the veyron board maintainer fix this?
> What to do with the things below to move forward with this serie?
>
> Before sync:
>
> panel: panel {
> compatible ="cnm,n116bgeea2","simple-panel";
> status = "okay";
> power-supply = <&vcc33_lcd>;
> backlight = <&backlight>;
> };
>
> &edp {
> status = "okay";
> rockchip,panel = <&panel>;
> };
>
> ===
>
> After sync:
>
> panel: panel {
> compatible = "innolux,n116bge";
> status = "okay";
> power-supply = <&panel_regulator>;
> backlight = <&backlight>;
>
> panel-timing {
> clock-frequency = <7425>;
> hactive = <1366>;
> hfront-porch = <136>;
> hback-porch = <60>;
>
Re: [PATCH v6 15/16] cmd: verify: initial import
Hi Philippe, On Thu, 10 Mar 2022 at 09:53, Philippe REYNES wrote: > > Hi Simon, > > > Le 03/03/2022 à 04:37, Simon Glass a écrit : > > Hi Philippe, > > > > On Fri, 25 Feb 2022 at 07:58, Philippe Reynes > > wrote: > >> Add the command verify that check the signature of > >> an image with the pre-load header. If the check > >> succeed, the u-boot env variable 'loadaddr_verified' > >> is set to the address of the image (without the header). > >> > >> It allows to run such commands: > >> tftp script.img && verify $loadaddr && source $loadaddr_verified > >> > >> Signed-off-by: Philippe Reynes > >> --- > >> cmd/Kconfig | 7 +++ > >> cmd/Makefile | 1 + > >> cmd/verify.c | 53 > >> 3 files changed, 61 insertions(+) > >> create mode 100644 cmd/verify.c > >> > > Using the 'verify' command seems a bit vague. Could it be a > > sub-command of bootm perhaps? > > > The command verify may be used with any binary (script, video firmware, > .). > So a lot of binaries that are not launched by bootm. > I think that it is not "logic" to used a bootm subcommand. > But we could use another name if you want. > For example : pre_load_verify ? I see. Well, I suppose this is a boot loader, so 'verify' would be expected to mean verifying an image or something to boot, so this seems reasonable to me. But I do like the idea of putting pre_load in there somewhere if you can, since we do most other verification as part of the 'bootm' command. Up to you. Reviewed-by: Simon Glass Regards, Simon
Re: [PATCH 4/9] linux/const.h: Upgrade & Merge vDSO and uAPI
Hi Pierre-Clément,
On Thu, 17 Mar 2022 at 05:00, Pierre-Clément Tosi wrote:
>
> On Wed, Mar 16, 2022 at 01:23:44PM -0600, Simon Glass wrote:
> > Hi Pierre-Clément,
> >
> > On Wed, 16 Mar 2022 at 09:40, Pierre-Clément Tosi wrote:
> > >
> > > Import the header from version 5.16 of the kernel:
> > >
> > > commit df0cc57e057f18e44dac8e6c18aba47ab53202f9
> > >
> > > Inline and . This is wrapped in
> > > "#ifndef __UBOOT__/#include/#else/{inline}" to better document the
> > > origin of the code being added to the U-Boot header (but not present in
> > > the original header) and make diff tools happier when comparing the file
> > > with its reference, which should be useful when porting future changes
> > > from the Linux header and/or if we decide to also import those included
> > > headers into the U-Boot codebase.
> > >
> > > Signed-off-by: Pierre-Clément Tosi
> > > Cc: Simon Glass
> > > Cc: Tom Rini
> > > ---
> > > include/linux/const.h | 22 +-
> > > 1 file changed, 21 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/linux/const.h b/include/linux/const.h
> > > index 379c889232..3e3803d767 100644
> > > --- a/include/linux/const.h
> > > +++ b/include/linux/const.h
> > > @@ -2,8 +2,13 @@
> > > #ifndef _LINUX_CONST_H
> > > #define _LINUX_CONST_H
> > >
> > > -/* const.h: Macros for dealing with constants. */
> > > +#ifndef __UBOOT__
> > > +#include
> > > +#else
> > >
> > > +#ifndef __UBOOT__
> > > +#include
> > > +#else
> > > /* Some constant macros are used in both assembler and
> > > * C code. Therefore we cannot annotate them always with
> > > * 'UL' and other type specifiers unilaterally. We
> > > @@ -28,7 +33,22 @@
> > > #define _BITUL(x) (_UL(1) << (x))
> > > #define _BITULL(x) (_ULL(1) << (x))
> > >
> > > +#define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x,
> > > (typeof(x))(a) - 1)
> > > +#define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask))
> >
> > How does this compare to the existing ALIGN()? It looks the same to me.
> >
>
> __ALIGN_KERNEL() is the uAPI counterpart to the ALIGN() kernel macro (or is it
> the other way around?) so they're intended to be similar in implementation;
> the
> key difference being the double-leading-underscore notation, which bears a
> special meaning as per the C Standard, and allows __ALIGN_KERNEL() to be
> considerably easier to export. In fact, the kernel currently defines
>
> #define ALIGN(x, a)__ALIGN_KERNEL((x), (a))
>
> For the reason why this patch introduces it here, I've taken the conservative
> approach of meticulously copying the original headers over, in their entirety
> (as far as feasible), in order to make future upgrades as smooth as possible.
>
> This patch inlines those vDSO and uAPI headers instead of bringing them as the
> stand-alone files they are in the kernel as it looks like no other kernel
> header
> of that kind has been imported but IMO, it would make more sense to do the
> later. What do you think?
OK I see. Using stand-alone files makes it easier to keep things in
sync in the future, but I don't have a strong opinion on it.
Reviewed-by: Simon Glass
>
> > > +
> > > +#define __KERNEL_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
> > > +#endif
> > > +
> > > #define UL(x) (_UL(x))
> > > #define ULL(x) (_ULL(x))
> > > +#endif
> > > +
> > > +/*
> > > + * This returns a constant expression while determining if an argument is
> > > + * a constant expression, most importantly without evaluating the
> > > argument.
> > > + * Glory to Martin Uecker
> > > + */
> > > +#define __is_constexpr(x) \
> > > + (sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int
> > > *)8)))
> > >
> > > #endif /* _LINUX_CONST_H */
> > > --
> > > 2.35.1.723.g4982287a31-goog
Regards,
Simon
Re: [PATCH v3 16/29] serial: Add semihosting driver
Hi Sean, On Tue, 22 Mar 2022 at 15:00, Sean Anderson wrote: > > This adds a serial driver which uses semihosting calls to read and write > to the host's console. For convenience, if CONFIG_DM_SERIAL is enabled, > we will instantiate a serial driver. This allows users to enable this > driver (which has no physical device) without modifying their device > trees or board files. We also implement a non-DM driver for SPL, or for > much faster output in U-Boot proper. > > There are three ways to print to the console: > > Method Baud > == = > smh_putc in a loop 170 > smh_puts1600 > smh_write with :tt 2 > == = > > These speeds were measured using a 175 character message with a J-Link > adapter. For reference, U-Boot typically prints around 2700 characters > during boot on this board. There are two major factors affecting the > speed of these functions. First, each breakpoint incurs a delay. Second, > each debugger memory transaction incurs a delay. smh_putc has a > breakpoint and memory transaction for every character. smh_puts has one > breakpoint, but still has to use a transaction for every character. This > is because we don't know the length up front, so OpenOCD has to check if > each character is nul. smh_write has only one breakpoint and one memory > transfer. > > DM serial drivers can only implement a putc interface, so we are stuck > with the slowest API. Non-DM drivers can implement puts, which is vastly > more efficient. When the driver starts up, we try to open :tt. Since > this is an extension, this may fail. If it does, we fall back to > smh_puts. We don't check :semihosting-features, since there are > nonconforming implementations (OpenOCD) which don't implement it (but > *do* implement :tt). > > Some semihosting implementations (QEMU) don't handle READC properly. To > work around this, we try to use open/read (much like for stdin) if > possible. > > There is no non-blocking I/O available, so we don't implement pending. > This will cause __serial_tstc to always return true. If > CONFIG_SERIAL_RX_BUFFER is enabled, _serial_tstc will try and read > characters forever. To avoid this, we depend on this config being > disabled. > > Signed-off-by: Sean Anderson > --- > > (no changes since v2) > > Changes in v2: > - Fix baud numbers being off by 10 > - Fix typos in commit message > - Rename non-DM driver struct to match format of other drivers > > drivers/serial/Kconfig | 22 + > drivers/serial/Makefile | 1 + > drivers/serial/serial.c | 2 + > drivers/serial/serial_semihosting.c | 147 > include/serial.h| 1 + > 5 files changed, 173 insertions(+) > create mode 100644 drivers/serial/serial_semihosting.c > > diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig > index 345d1881f5..cc20759505 100644 > --- a/drivers/serial/Kconfig > +++ b/drivers/serial/Kconfig > @@ -399,6 +399,15 @@ config DEBUG_UART_SANDBOX > start up driver model. The driver will be available until the real > driver model serial is running. > > +config DEBUG_UART_SEMIHOSTING > + bool "semihosting" > + depends on SEMIHOSTING_SERIAL > + help > + Select this to enable the debug UART using the semihosting driver. > + This provides basic serial output from the console without needing > to > + start up driver model. The driver will be available until the real > + driver model serial is running. > + > config DEBUG_UART_SIFIVE > bool "SiFive UART" > depends on SIFIVE_SERIAL > @@ -778,6 +787,19 @@ config SCIF_CONSOLE > on systems with RCar or SH SoCs, say Y to this option. If unsure, > say N. > > +config SEMIHOSTING_SERIAL > + bool "Semihosting UART support" > + depends on SEMIHOSTING && !SERIAL_RX_BUFFER > + help > + Select this to enable a serial UART using semihosting. Special halt > + instructions will be issued which an external debugger (such as a > + JTAG emulator) may interpret. The debugger will display U-Boot's > + console output on the host system. > + > + Enable this option only if you are using a debugger which supports > + semihosting. If you are not using a debugger, this driver will halt > + the boot. > + > config UNIPHIER_SERIAL > bool "Support for UniPhier on-chip UART" > depends on ARCH_UNIPHIER > diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile > index 52e70aa191..b68b5e7b2b 100644 > --- a/drivers/serial/Makefile > +++ b/drivers/serial/Makefile > @@ -52,6 +52,7 @@ endif > obj-$(CONFIG_XILINX_UARTLITE) += serial_xuartlite.o > obj-$(CONFIG_SANDBOX_SERIAL) += sandbox.o > obj-$(CONFIG_SCIF_CONSOLE) += serial_sh.o > +obj-$(CONFIG_SEMIHOSTING_SERIAL) += serial_semihosting.o > obj-$(CONFIG_ZYNQ_SERIAL) += serial_zynq.o > obj-$(CONFIG_FSL_LPUART
Re: [PATCH v3 26/29] serial: dm: Add support for puts
On Tue, 22 Mar 2022 at 15:00, Sean Anderson wrote: > > Some serial drivers can be vastly more efficient when printing multiple > characters at once. Non-DM serial has had a puts option for these sorts > of drivers; implement it for DM serial as well. > > Because we have to add carriage returns, we can't just pass the whole > string directly to the serial driver. Instead, we print up to the > newline, then print a carriage return, and then continue on. This is > less efficient, but it is better than printing each character > individually. It also avoids having to allocate memory just to add a few > characters. > > Drivers may perform short writes (such as filling a FIFO) and return the > number of characters written in len. We loop over them in the same way > that _serial_putc loops over putc. > > This results in around sizeof(void *) growth for all boards with > DM_SERIAL. The full implementation takes around 140 bytes. > > Signed-off-by: Sean Anderson > --- > > Changes in v3: > - Add a config for puts to reduce the impact on the (vast majority) of > boards which don't need it. > - Fix null pointer dereference in _serial_puts caused by a missing > return. > - Make puts return the number of characters written on success, instead > of reusing the len parameter. > > Changes in v2: > - New > > drivers/serial/Kconfig | 13 + > drivers/serial/serial-uclass.c | 26 -- > include/serial.h | 18 ++ > 3 files changed, 55 insertions(+), 2 deletions(-) Reviewed-by: Simon Glass
Re: Un-clean scripts/dtc causing buildman failure
Hi Marcel, On Sat, 8 Jan 2022 at 17:02, Marcel Ziswiler wrote: > > Hi Simon > > On Sat, 2022-01-08 at 07:54 -0700, Simon Glass wrote: > > Hi Marcel, > > > > On Sat, 8 Jan 2022 at 07:22, Marcel Ziswiler wrote: > > > > > > Hi Simon et al. > > > > > > Somehow doing an mrproper does not seem to clean intermediate file in > > > scripts/dtc: > > > > > > ⬢[zim@toolbox u-boot.git]$ make mrproper > > > ⬢[zim@toolbox u-boot.git]$ git status --ignored scripts/dtc > > > On branch master > > > Your branch is up to date with 'origin/master'. > > > > > > Ignored files: > > > (use "git add -f ..." to include in what will be committed) > > > scripts/dtc/dtc > > > scripts/dtc/pylibfdt/libfdt.i > > > scripts/dtc/pylibfdt/libfdt.py > > > scripts/dtc/pylibfdt/libfdt_wrap.c > > > > > > nothing to commit, working tree clean > > > > > > Unfortunately later running buildman off such a tree than fails: > > > > > > ⬢[zim@toolbox u-boot.git]$ tools/buildman/buildman verdin-imx8mm > > > Building current source for 1 boards (1 thread, 16 jobs per thread) > > >aarch64: + verdin-imx8mm > > > +/usr/bin/which: no dtc in (./scripts/dtc) > > > +Unable to find file 'scripts/dtc/pylibfdt/libfdt.i'. > > > +error: command '/usr/bin/swig' failed with exit code 1 > > > +make[3]: *** [scripts/dtc/pylibfdt/Makefile:30: rebuild] Error 1 > > > +make[2]: *** [scripts/Makefile.build:394: scripts/dtc/pylibfdt] Error 2 > > > +make[1]: *** [Makefile:1970: scripts_dtc] Error 2 > > > +make: *** [Makefile:177: sub-make] Error 2 > > > 001 /1 verdin-imx8mm > > > Completed: 1 total built, duration 0:00:05, rate 0.20 > > > > > > Is this a known issue? Shouldn't a mrproper also clean such intermediate > > > files? > > > > Yes, that seems like a bug. > > I had a look at it but, unfortunately, I am not too proficient with U-Boot's > Makefile stuff. What I found out > is that scripts/dtc/pylibfdt/Makefile actually ads them all to clean-files. > However, I guess that is never > actually used anywhere at least in U-Boot and upstream libfdt seems to have > changed considerably in that area. > > Anyway, the root Makefile has a whole section about cleaning plus there is > also a scripts/Makefile.clean. > However, I can't quite figure out how this all works together. > > > > BTW: Any news on getting that libfdt stuff updated to work with latest > > > tooling (e.g. as used in Fedora 35)? > > > > Can you give me more detail on this? > > Sorry, I thought I inquired about this once before. This is what I am talking > about: > > ⬢[zim@toolbox u-boot.git]$ cat /etc/fedora-release > Fedora release 35 (Thirty Five) > ⬢[zim@toolbox u-boot.git]$ tools/buildman/buildman verdin-imx8mm > Building current source for 1 boards (1 thread, 16 jobs per thread) >aarch64: w+ verdin-imx8mm > +/usr/bin/which: no dtc in (./scripts/dtc) > +scripts/dtc/pylibfdt/setup.py:21: DeprecationWarning: The distutils package > is deprecated and slated for > removal in Python 3.12. Use setuptools or check PEP 632 for potential > alternatives > + from distutils.core import setup, Extension > +tools/binman/binman:12: DeprecationWarning: The distutils package is > deprecated and slated for removal in > Python 3.12. Use setuptools or check PEP 632 for potential alternatives > + from distutils.sysconfig import get_python_lib > +tools/binman/binman:12: DeprecationWarning: The distutils.sysconfig module > is deprecated, use sysconfig > instead > 010 /1 verdin-imx8mm > Completed: 1 total built, duration 0:00:16, rate 0.06 Just to say that I have not looked at this one. So it doesn't get lost it might be worth filing an issue with all the info here: https://source.denx.de/u-boot/u-boot/-/issues/ Regards, Simon
Re: [PATCH v3 16/29] serial: Add semihosting driver
Hi Sean, On Tue, 22 Mar 2022 at 15:00, Sean Anderson wrote: > > This adds a serial driver which uses semihosting calls to read and write > to the host's console. For convenience, if CONFIG_DM_SERIAL is enabled, > we will instantiate a serial driver. This allows users to enable this > driver (which has no physical device) without modifying their device > trees or board files. We also implement a non-DM driver for SPL, or for > much faster output in U-Boot proper. > > There are three ways to print to the console: > > Method Baud > == = > smh_putc in a loop 170 > smh_puts1600 > smh_write with :tt 2 > == = > > These speeds were measured using a 175 character message with a J-Link > adapter. For reference, U-Boot typically prints around 2700 characters > during boot on this board. There are two major factors affecting the > speed of these functions. First, each breakpoint incurs a delay. Second, > each debugger memory transaction incurs a delay. smh_putc has a > breakpoint and memory transaction for every character. smh_puts has one > breakpoint, but still has to use a transaction for every character. This > is because we don't know the length up front, so OpenOCD has to check if > each character is nul. smh_write has only one breakpoint and one memory > transfer. > > DM serial drivers can only implement a putc interface, so we are stuck > with the slowest API. Non-DM drivers can implement puts, which is vastly > more efficient. When the driver starts up, we try to open :tt. Since > this is an extension, this may fail. If it does, we fall back to > smh_puts. We don't check :semihosting-features, since there are > nonconforming implementations (OpenOCD) which don't implement it (but > *do* implement :tt). > > Some semihosting implementations (QEMU) don't handle READC properly. To > work around this, we try to use open/read (much like for stdin) if > possible. > > There is no non-blocking I/O available, so we don't implement pending. > This will cause __serial_tstc to always return true. If > CONFIG_SERIAL_RX_BUFFER is enabled, _serial_tstc will try and read > characters forever. To avoid this, we depend on this config being > disabled. > > Signed-off-by: Sean Anderson > --- > > (no changes since v2) > > Changes in v2: > - Fix baud numbers being off by 10 > - Fix typos in commit message > - Rename non-DM driver struct to match format of other drivers > > drivers/serial/Kconfig | 22 + > drivers/serial/Makefile | 1 + > drivers/serial/serial.c | 2 + > drivers/serial/serial_semihosting.c | 147 > include/serial.h| 1 + > 5 files changed, 173 insertions(+) > create mode 100644 drivers/serial/serial_semihosting.c > Reviewed-by: Simon Glass But please can we drop the non-DM support?
Re: [PATCH 2/6] image: fit: Add some helpers for getting data
Hi Sean,
On Thu, 24 Mar 2022 at 12:23, Sean Anderson wrote:
>
> Several different firmware users have repetitive code to extract the
> firmware data from a FIT. Add some helper functions to reduce the amount
> of repetition. fit_conf_get_prop_node (eventually) calls
> fdt_check_node_offset_, so we can avoid an explicit if. In general, this
> version avoids printing on error because the callers are typically
> library functions, and because the FIT code generally has (debug)
> prints of its own. One difference in these helpers is that they use
> fit_image_get_data_and_size instead of fit_image_get_data, as the former
> handles external data correctly.
>
> Signed-off-by: Sean Anderson
> ---
>
> arch/arm/cpu/armv8/sec_firmware.c | 39 ++---
> boot/image-fit.c | 37 +++
> cmd/fpga.c | 24 +-
> drivers/net/fsl-mc/mc.c| 30 +++---
> drivers/net/pfe_eth/pfe_firmware.c | 40 +-
> include/image.h| 4 +++
> 6 files changed, 53 insertions(+), 121 deletions(-)
It feels like this patch should be split up into generic / armv8 / fsp things.
>
> diff --git a/arch/arm/cpu/armv8/sec_firmware.c
> b/arch/arm/cpu/armv8/sec_firmware.c
> index 41525a10d5..879eb6ec81 100644
> --- a/arch/arm/cpu/armv8/sec_firmware.c
> +++ b/arch/arm/cpu/armv8/sec_firmware.c
> @@ -42,43 +42,8 @@ phys_addr_t sec_firmware_addr;
> static int sec_firmware_get_data(const void *sec_firmware_img,
> const void **data, size_t *size)
> {
> - int conf_node_off, fw_node_off;
> - char *desc;
> - int ret;
> -
> - conf_node_off = fit_conf_get_node(sec_firmware_img, NULL);
> - if (conf_node_off < 0) {
> - puts("SEC Firmware: no config\n");
> - return -ENOENT;
> - }
> -
> - fw_node_off = fit_conf_get_prop_node(sec_firmware_img, conf_node_off,
> - SEC_FIRMWARE_FIT_IMAGE);
> - if (fw_node_off < 0) {
> - printf("SEC Firmware: No '%s' in config\n",
> - SEC_FIRMWARE_FIT_IMAGE);
> - return -ENOLINK;
> - }
> -
> - /* Verify secure firmware image */
> - if (!(fit_image_verify(sec_firmware_img, fw_node_off))) {
> - printf("SEC Firmware: Bad firmware image (bad CRC)\n");
> - return -EINVAL;
> - }
> -
> - if (fit_image_get_data(sec_firmware_img, fw_node_off, data, size)) {
> - printf("SEC Firmware: Can't get %s subimage data/size",
> - SEC_FIRMWARE_FIT_IMAGE);
> - return -ENOENT;
> - }
> -
> - ret = fit_get_desc(sec_firmware_img, fw_node_off, &desc);
> - if (ret)
> - printf("SEC Firmware: Can't get description\n");
> - else
> - printf("%s\n", desc);
> -
> - return ret;
> + return fit_get_data_conf_prop(sec_firmware_img,
> SEC_FIRMWARE_FIT_IMAGE,
> + data, size);
> }
>
> /*
> diff --git a/boot/image-fit.c b/boot/image-fit.c
> index 6610035d0a..b87378cbf6 100644
> --- a/boot/image-fit.c
> +++ b/boot/image-fit.c
> @@ -1919,6 +1919,43 @@ int fit_conf_get_prop_node(const void *fit, int
> noffset,
> return fit_conf_get_prop_node_index(fit, noffset, prop_name, 0);
> }
>
> +static int fit_get_data_tail(const void *fit, int noffset,
> +const void **data, size_t *size)
> +{
> + char *desc;
> +
> + if (noffset < 0)
> + return noffset;
> +
> + if (!fit_image_verify(fit, noffset))
> + return -EINVAL;
> +
> + if (fit_image_get_data_and_size(fit, noffset, data, size))
> + return -ENOENT;
> +
> + if (!fit_get_desc(fit, noffset, &desc))
> + printf("%s\n", desc);
> +
> + return 0;
> +}
> +
> +int fit_get_data_node(const void *fit, const char *image_uname,
> + const void **data, size_t *size)
> +{
> + int noffset = fit_image_get_node(fit, image_uname);
> +
> + return fit_get_data_tail(fit, noffset, data, size);
> +}
> +
> +int fit_get_data_conf_prop(const void *fit, const char *prop_name,
> + const void **data, size_t *size)
> +{
> + int noffset = fit_conf_get_node(fit, NULL);
> +
> + noffset = fit_conf_get_prop_node(fit, noffset, prop_name);
> + return fit_get_data_tail(fit, noffset, data, size);
> +}
> +
> static int fit_image_select(const void *fit, int rd_noffset, int verify)
> {
> fit_image_print(fit, rd_noffset, " ");
> diff --git a/cmd/fpga.c b/cmd/fpga.c
> index 3fdd0b35e8..1102a84d76 100644
> --- a/cmd/fpga.c
> +++ b/cmd/fpga.c
> @@ -322,7 +322,7 @@ static int do_fpga_loadmk(struct cmd_tbl *cmdtp, int
> flag, int argc,
> case IMAGE_FORMAT_FIT:
> {
> const void *fit_hdr
Re: [PATCH 3/6] misc: fs_loader: Add function to get the chosen loader
On Thu, 24 Mar 2022 at 12:23, Sean Anderson wrote: > > The fs_loader device is used to pull in settings via the chosen node. > However, there was no library function for this, so arria10 was doing it > explicitly. This function subsumes that, and uses ofnode_get_chosen_node > instead of navigating the device tree directly. Because fs_loader pulls > its config from the environment by default, it's fine to create a device > with nothing backing it at all. Doing this allows enabling > CONFIG_FS_LOADER without needing to modify the device tree. > > Signed-off-by: Sean Anderson > --- > > arch/arm/mach-k3/common.c | 2 +- > arch/arm/mach-omap2/boot-common.c | 2 +- > drivers/fpga/socfpga_arria10.c| 24 ++-- > drivers/misc/fs_loader.c | 27 +++ > include/fs_loader.h | 12 > 5 files changed, 43 insertions(+), 24 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH v3 28/29] test: serial: Add test for putc/puts
Hi Sean, On Tue, 22 Mar 2022 at 15:00, Sean Anderson wrote: > > This adds a test to ensure that puts is equivalent to putc called in a > loop. We don't verify the contents of the message to avoid having to > record console output a second time (though that could be added in the > future). The globals are initialized to non-zero values to avoid a > warning; in particular, the character count is off-by-one (but we always > make relative measurements). > > Signed-off-by: Sean Anderson > --- > > Changes in v3: > - New > > arch/sandbox/include/asm/serial.h | 6 ++ > drivers/serial/sandbox.c | 23 +++ > test/dm/serial.c | 19 +++ > 3 files changed, 44 insertions(+), 4 deletions(-) Reviewed-by: Simon Glass But please can you add functions to change sandbox_serial_enabled (and read from sandbox_serial_written) to asm/test.h since that is how these sorts of things are done in other drivers.
Re: [PATCH] image: fit: Align hash output buffers
On Thu, 24 Mar 2022 at 09:26, Sean Anderson wrote:
>
> Hardware-accelerated hash functions require that the input and output
> buffers be aligned to the minimum DMA alignment. memalign.h helpfully
> provides a macro just for this purpose. It doesn't exist on the host,
> but we don't need to be aligned there either.
>
> Fixes: 5dfb521386 ("[new uImage] New uImage low-level API")
> Signed-off-by: Sean Anderson
> ---
>
> boot/image-fit.c | 4 +++-
> tools/mkimage.h | 3 +++
> 2 files changed, 6 insertions(+), 1 deletion(-)
Reviewed-by: Simon Glass
>
> diff --git a/boot/image-fit.c b/boot/image-fit.c
> index f01cafe4e2..6610035d0a 100644
> --- a/boot/image-fit.c
> +++ b/boot/image-fit.c
> @@ -24,6 +24,7 @@
> #include
> #include
> #include
> +#include
> #include
> #ifdef CONFIG_DM_HASH
> #include
> @@ -1263,7 +1264,8 @@ int calculate_hash(const void *data, int data_len,
> const char *name,
> static int fit_image_check_hash(const void *fit, int noffset, const void
> *data,
> size_t size, char **err_msgp)
> {
> - uint8_t value[FIT_MAX_HASH_LEN];
> + DEFINE_ALIGN_BUFFER(uint8_t, value, FIT_MAX_HASH_LEN,
> + ARCH_DMA_MINALIGN);
> int value_len;
> const char *algo;
> uint8_t *fit_value;
> diff --git a/tools/mkimage.h b/tools/mkimage.h
> index 0d3148444c..7652c8b001 100644
> --- a/tools/mkimage.h
> +++ b/tools/mkimage.h
> @@ -41,6 +41,9 @@ static inline ulong map_to_sysmem(void *ptr)
> return (ulong)(uintptr_t)ptr;
> }
>
> +#define ARCH_DMA_MINALIGN 1
> +#define DEFINE_ALIGN_BUFFER(type, name, size, alugn) type name[size]
> +
> #define MKIMAGE_TMPFILE_SUFFIX ".tmp"
> #define MKIMAGE_MAX_TMPFILE_LEN256
> #define MKIMAGE_DEFAULT_DTC_OPTIONS"-I dts -O dtb -p 500"
> --
> 2.25.1
>
Re: [PATCH v3 27/29] serial: sandbox: Implement puts
On Tue, 22 Mar 2022 at 15:00, Sean Anderson wrote: > > This implements puts for sandbox. It is fairly straightforward, except > that we break out the shared color printing functionality into its own > function. > > Signed-off-by: Sean Anderson > --- > > (no changes since v2) > > Changes in v2: > - New > > drivers/serial/Kconfig | 1 + > drivers/serial/sandbox.c | 21 - > 2 files changed, 21 insertions(+), 1 deletion(-) Reviewed-by: Simon Glass
Re: [PATCH v2 7/9] Make EFI_LOADER depend on DM and OF_CONTROL
Hi Tom, On Fri, 30 Jul 2021 at 16:08, Tom Rini wrote: > > On Fri, Jul 30, 2021 at 03:48:15PM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Fri, 30 Jul 2021 at 15:33, Tom Rini wrote: > > > > > > On Fri, Jul 30, 2021 at 01:02:18PM -0600, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Thu, 29 Jul 2021 at 07:52, Tom Rini wrote: > > > > > > > > > > On Wed, Jul 28, 2021 at 07:44:37PM -0600, Simon Glass wrote: > > > > > > Hi, > > > > > > > > > > > > On Wed, 28 Jul 2021 at 17:55, Tom Rini wrote: > > > > > > > > > > > > > > On Thu, Jul 29, 2021 at 01:45:49AM +0200, Heinrich Schuchardt > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > On 7/27/21 12:07 AM, Tom Rini wrote: > > > > > > > > > On Fri, Jul 02, 2021 at 12:36:18PM -0600, Simon Glass wrote: > > > > > > > > > > > > > > > > > > > This feature should never have been made available when > > > > > > > > > > driver model > > > > > > > > > > or devicetree are disabled. Add these as conditions, so > > > > > > > > > > that we don't > > > > > > > > > > create even more barriers to migration. > > > > > > > > > > > > > > > > > > > > Add a note about the substantial size increment associated > > > > > > > > > > with this > > > > > > > > > > option. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Simon Glass > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > > > Changes in v2: > > > > > > > > > > - Split out new patch to make EFI_LOADER depend on DM and > > > > > > > > > > OF_CONTROL > > > > > > > > > > - Note the approximate size of this feature in the help > > > > > > > > > > > > > > > > > > > > lib/efi_loader/Kconfig | 4 +++- > > > > > > > > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > > > > > > > > > > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > > > > > > > > > index 6242caceb7f..466abfed300 100644 > > > > > > > > > > --- a/lib/efi_loader/Kconfig > > > > > > > > > > +++ b/lib/efi_loader/Kconfig > > > > > > > > > > @@ -1,6 +1,6 @@ > > > > > > > > > > config EFI_LOADER > > > > > > > > > > bool "Support running UEFI applications" > > > > > > > > > > - depends on OF_LIBFDT && ( \ > > > > > > > > > > + depends on OF_LIBFDT && DM && OF_CONTROL && ( \ > > > > > > > > > > > > > > > > Didn't Tom eliminate all boards without CONFIG_DM? Shouldn't we > > > > > > > > get rid > > > > > > > > of this symbol? > > > > > > > > > > > > > > No, but I did send out a message about that today as we're very > > > > > > > close. > > > > > > > Much closer than I had expected us to be. > > > > > > > > > > > > Note we will still have SPL_DM, I think. > > > > > > > > > > Right. > > > > > > > > > > > > > Are there boards using DM and not OF_CONTROL or OF_CONTROL and > > > > > > > > not DM? > > > > > > > > > > > > > > Yes, a few. It's vexpress_aemv8a_semi, warp (fixed by > > > > > > > https://patchwork.ozlabs.org/project/uboot/patch/20210402180552.1075997-2-pbrobin...@gmail.com/ > > > > > > > so false positive), cm_t335, devkit8000, armadillo-800eva, kzm9g > > > > > > > and sniper. > > > > > > > > > > > > > > > Why are these separate symbols? Isn't this symbol to be > > > > > > > > eliminated, too? > > > > > > > > > > > > > > Simon? > > > > > > > > > > > > If we do eliminate DM it will be in a separate series. Like Tom I am > > > > > > pleasantly surprised at how close we are. > > > > > > > > > > > > But please let's consider patches on their merits. It is fine to > > > > > > reply > > > > > > with a wishlist but even better to reply with a follow-up patch. > > > > > > > > > > OK. So, build-wise, EFI_LOADER does not require OF_CONTROL. > > > > > > > > I strongly believe it should (and it should have 5 years ago too). New > > > > features should not be built on pre-migration stuff. > > > > > > Well, what legacy interfaces is it using? That's something to figure > > > out and address as needed. > > > > It was built on non-DM and I believe it still has code for non-DM > > (e.g. look for DM_MMC). Without DM, OF_CONTROL has no purpose IMO. > > > > Perhaps Heinrich has cleaned a lot of that old cruft out now? > > Now that DM_MMC and DM_VIDEO are mandatory, there is some cruft that can > be removed, both there and probably tree-wide. But that's not the same > as OF_CONTROL. Not all DM_xxx requires OF_CONTROL support. > > > > > > > Somewhat related, I think we need to create a separate symbol which > > > > > > means (OF_CONTROL && !OF_PLATDATA) so we can just check one thing. > > > > > > > > > > > > Also I think we should push of-platdata, since otherwise we're going > > > > > > to hit the same problem of migrating SPL boards to DM one day. > > > > > > > > > > Note that we don't have CONFIG_OF_PLATDATA just > > > > > CONFIG_(SPL|TPL)_OF_PLATDATA. > > > > > > > > Indeed. But we haven't defined it because we don't want to permit it. > > > > It is just for constrained environments and we assume that U-Boot > > > > proper has enough space (how else could it load Linux?) > > > > >
Re: [PATCH v5 4/9] tpm: Add the RNG child device
Hi Sughosh, On Mon, 21 Mar 2022 at 00:01, Sughosh Ganu wrote: > > hi Simon, > > On Wed, 16 Mar 2022 at 02:45, Simon Glass wrote: > > > > Hi Ilias, > > > > On Tue, 15 Mar 2022 at 00:34, Ilias Apalodimas > > wrote: > > > > > > Hi Simon, > > > > > > On Mon, 14 Mar 2022 at 20:24, Simon Glass wrote: > > > > > > > > > > [...] > > > > > > > > > > + } > > > > > > > > > > > > This really should be in the device tree so what you are doing here > > > > > > is > > > > > > quite strange. > > > > > > > > > > Like I had mentioned in my earlier emails, the TPM device has a > > > > > builtin RNG functionality, which is non-optional. So I don't > > > > > understand why we need to use a device tree subnode here. Whether the > > > > > device is being bound to the parent is being controlled by the TPM_RNG > > > > > config that you asked me to put in my previous version, which I am > > > > > doing. > > > > > > > > See how PMICs work, for example. We have GPIOs, regulators and > > > > suchlike in the PMIC and we add subnodes for them in the DT. It is > > > > just how it is done. > > > > > > > > Driver model is designed to automatically bind devices based on the > > > > device tree. There are cases where it is necessary to manually bind > > > > things, but we mustn't prevent people from doing it 'properly'. > > > > > > There's a small difference here though. The RNG is not a device. The > > > TPM is the device and an encoded command to that device returns a > > > random number. There's no binding initiating etc. > > > > A device is just something with a struct udevice, so I don't see the > > random number generator as anything different from another device. We > > might have a white-noise generator which produces random numbers. Just > > because the feature is packaged inside a single chip doesn't make it > > any less a device. Just like the PMIC. > > > > > > > > > > > > > Finally, I know you keep saying that random numbers are only needed in > > > > U-Boot proper, but if I want a random number in SPL, it may not work, > > > > since device_bind() is often not available, for code-size reasons. > > > > > > And the entire tpm framework will fit? > > > > Yes. For verified boot it has to, since you cannot init RAM until you > > have selected your A/B/recovery image. > > > > > > > > > > > > > So that is why I say that what you are doing is quite strange. Perhaps > > > > you are coming from a different project, which does things > > > > differently. > > > > > > I don't personally find it strange. The device is already described > > > in the DTS and I don't see a strong reason to deviate for the upstream > > > version again. > > > > Linux tends to rely a lot more on manually adding devices. It can have > > a pretty dramatic bloating effect on code size in U-Boot. > > > > Anyway, so long as we can detect an existing device, as I explained > > below, it is fine to manually add it when it is missing. > > Just so that I understand what you are saying, do you want support for > both approaches. Meaning, using device tree when the rng node is > described in the device tree, and otherwise using the manual device > binding when the device tree node is absent. Do I understand this > right? Yes that's what we normally do in U-Boot. Regards, Simon
Re: [PATCH 7/9] include: Upgrade
Hi Pierre-Clément,
On Thu, 17 Mar 2022 at 05:46, Pierre-Clément Tosi wrote:
>
> On Wed, Mar 16, 2022 at 01:23:38PM -0600, Simon Glass wrote:
> > Hi,
> >
> > On Wed, 16 Mar 2022 at 09:41, Pierre-Clément Tosi wrote:
> > >
> > > Upgrade the header to version 5.16 of the kernel:
> > >
> > > commit df0cc57e057f18e44dac8e6c18aba47ab53202f9
> > >
> > > Signed-off-by: Pierre-Clément Tosi
> > > Cc: Simon Glass
> > > Cc: Tom Rini
> > > ---
> > > include/linux/typecheck.h | 10 ++
> > > 1 file changed, 10 insertions(+)
> >
> > Reviewed-by: Simon Glass
> >
> > But I don't understand how this works at all. Could you add a comment?
>
> This patch is a simple upgrade of the header. I needed it for another patch
> stack and, given how trivial and self-contained it is, I thought I would slip
> it
> into this one.
>
> If your "this" refers to the macro itself, an error should be generated by the
> compiler when dereferencing a non-pointer and it avoids introducing side
> effects
> through the use of __dummy but I'm not sure what the sizeof is there for; is
> it
> perhaps compliant for an implementation to keep generating code when
> encountering a dereferenced non-pointer (leading to a bogus result, from UB)
> so
> that sizeof forces it to depend on that expression being of a valid type? Or
> is
> it to do with catching variables of incomplete type e.g. arrays of unknown
> size?
> It was introduced by commit cb0f80039fb7 ("bitops: Add non-atomic bitops for
> pointers"), which doesn't provide much information either.
Hmm OK. The Linux people are quite averse to comments. If you have
time you could send a patch to comment the macro and see if someone
responds and dispels the uncertainty.
>
> >
> > >
> > > diff --git a/include/linux/typecheck.h b/include/linux/typecheck.h
> > > index eb5b74a575..46b15e2aae 100644
> > > --- a/include/linux/typecheck.h
> > > +++ b/include/linux/typecheck.h
> > > @@ -1,3 +1,4 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > #ifndef TYPECHECK_H_INCLUDED
> > > #define TYPECHECK_H_INCLUDED
> > >
> > > @@ -21,4 +22,13 @@
> > > (void)__tmp; \
> > > })
> > >
> > > +/*
> > > + * Check at compile time that something is a pointer type.
> > > + */
> > > +#define typecheck_pointer(x) \
> > > +({ typeof(x) __dummy; \
> > > + (void)sizeof(*__dummy); \
> > > + 1; \
> > > +})
> > > +
> > > #endif /* TYPECHECK_H_INCLUDED */
> > > --
> > > 2.35.1.723.g4982287a31-goog
Regards,
Simon
Re: [PATCH v2] pinctrl: nuvoton: Add NPCM8xx pinctrl driver
Hi Stanley,
On Thu, 17 Mar 2022 at 22:33, Stanley Chu wrote:
>
> Add Nuvoton BMC NPCM845 Pinmux and Pinconf support.
>
> Signed-off-by: Stanley Chu
> ---
> v2:
> - drop the WDnRCRB/CORSTCB register access, it is not for
>GPIO modules reset control
> ---
> drivers/pinctrl/Kconfig |1 +
> drivers/pinctrl/Makefile |1 +
> drivers/pinctrl/nuvoton/Kconfig |6 +
> drivers/pinctrl/nuvoton/Makefile |1 +
> drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c | 1163 +
> 5 files changed, 1172 insertions(+)
> create mode 100644 drivers/pinctrl/nuvoton/Kconfig
> create mode 100644 drivers/pinctrl/nuvoton/Makefile
> create mode 100644 drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c
>
Reviewed-by: Simon Glass
A few thoughts below
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index 03946245c7..076aff1a8d 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -329,6 +329,7 @@ source "drivers/pinctrl/mscc/Kconfig"
> source "drivers/pinctrl/mtmips/Kconfig"
> source "drivers/pinctrl/mvebu/Kconfig"
> source "drivers/pinctrl/nexell/Kconfig"
> +source "drivers/pinctrl/nuvoton/Kconfig"
> source "drivers/pinctrl/nxp/Kconfig"
> source "drivers/pinctrl/renesas/Kconfig"
> source "drivers/pinctrl/rockchip/Kconfig"
> diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
> index df37c32033..de84f8912b 100644
> --- a/drivers/pinctrl/Makefile
> +++ b/drivers/pinctrl/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_ARCH_ASPEED) += aspeed/
> obj-$(CONFIG_ARCH_ATH79) += ath79/
> obj-$(CONFIG_PINCTRL_INTEL) += intel/
> obj-$(CONFIG_ARCH_MTMIPS) += mtmips/
> +obj-$(CONFIG_ARCH_NPCM) += nuvoton/
> obj-$(CONFIG_ARCH_RMOBILE) += renesas/
> obj-$(CONFIG_PINCTRL_SANDBOX) += pinctrl-sandbox.o
>
> diff --git a/drivers/pinctrl/nuvoton/Kconfig b/drivers/pinctrl/nuvoton/Kconfig
> new file mode 100644
> index 00..b490b41a60
> --- /dev/null
> +++ b/drivers/pinctrl/nuvoton/Kconfig
> @@ -0,0 +1,6 @@
> +config PINCTRL_NPCM8XX
> + bool "Pinctrl driver for Nuvoton NPCM8XX"
> + depends on DM && PINCTRL_GENERIC && ARCH_NPCM8XX
> + help
> + Support pin muxing and pin configuration on
> + Nuvoton NPCM8XX SoC.
This should have more text
> diff --git a/drivers/pinctrl/nuvoton/Makefile
> b/drivers/pinctrl/nuvoton/Makefile
> new file mode 100644
> index 00..a6dfdf3672
> --- /dev/null
> +++ b/drivers/pinctrl/nuvoton/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_PINCTRL_NPCM8XX) += pinctrl-npcm8xx.o
> diff --git a/drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c
> b/drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c
> new file mode 100644
> index 00..4531923cff
> --- /dev/null
> +++ b/drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c
> @@ -0,0 +1,1163 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2022 Nuvoton Technology Corp.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +/* GCR register offsets */
> +#define WD0RCR 0x38
> +#define WD1RCR 0x3C
lower-case hex consistently
> +#define WD2RCR 0x40
> +#define SWRSTC10x44
> +#define SWRSTC20x48
> +#define SWRSTC30x4C
> +#define SWRSTC40x50
> +#define CORSTC 0x5C
> +#define FLOCKR10x74
> +#define INTCR4 0xC0
> +#define I2CSEGSEL 0xE0
> +#define MFSEL1 0x260
> +#define MFSEL2 0x264
> +#define MFSEL3 0x268
> +#define MFSEL4 0x26C
> +#define MFSEL5 0x270
> +#define MFSEL6 0x274
> +#define MFSEL7 0x278
> +
[..]
> +
> +struct group_config {
> + char *name;
> + const unsigned int *pins;
uint
> + unsigned int npins;
> + u32 reg; /* Register of setting func */
> + u32 bit;
> +};
> +
> +#define GRP(x, _reg, _bit) { \
> + .name = #x, \
> + .pins = x## _pins, \
> + .npins = ARRAY_SIZE(x## _pins), \
> + .reg = _reg,\
> + .bit = _bit,\
> + }
[..]
> +static int npcm8xx_pinmux_group_set(struct udevice *dev,
> + unsigned int group_selector,
> + unsigned int func_selector)
> +{
> + const struct group_config *group;
> + int pin_selector;
> + int i;
> +
> + dev_dbg(dev, "set_mux [grp %s][func %s]\n",
> + npcm8xx_groups[group_selector].name,
> + npcm8xx_groups[func_selector].name);
> + group = &npcm8xx_groups[group_selector];
> +
> + if (group->npins == 0) {
!group->npins
[..]
Regards,
Simon
Re: [PATCH u-boot-net 03/14] net: introduce helpers to get PHY ofnode from MAC
Hi Marek, On Thu, 17 Mar 2022 at 06:50, Marek Behún wrote: > > From: Marek Behún > > Add helpers ofnode_get_phy_node() and dev_get_phy_node() and use it in > net/mdio-uclass.c function dm_eth_connect_phy_handle(). > > This is useful because other part's of U-Boot may want to get PHY ofnode > without connecting a PHY. > > Signed-off-by: Marek Behún > --- > drivers/core/ofnode.c | 21 + > drivers/core/read.c | 5 + > include/dm/ofnode.h | 14 ++ > include/dm/read.h | 19 +++ > net/mdio-uclass.c | 24 ++-- > 5 files changed, 65 insertions(+), 18 deletions(-) Please add a test for your new function. With that: Reviewed-by: Simon Glass
Re: [PATCH 01/11] db-mv784mp-gp: Rename CONFIG_DB_784MP_GP to CONFIG_TARGET_DB_MV784MP_GP
On 3/24/22 22:17, Tom Rini wrote: The value CONFIG_DB_784MP_GP is only used in the DDR code to refer to CONFIG_TARGET_DB_MV784MP_GP so just use that second value directly. Cc: Stefan Roese Signed-off-by: Tom Rini Reviewed-by: Stefan Roese Thanks, Stefan --- arch/arm/mach-mvebu/serdes/axp/high_speed_env_lib.c | 2 +- drivers/ddr/marvell/axp/ddr3_axp.h | 2 +- drivers/ddr/marvell/axp/ddr3_axp_config.h | 2 +- include/configs/db-mv784mp-gp.h | 5 - 4 files changed, 3 insertions(+), 8 deletions(-) diff --git a/arch/arm/mach-mvebu/serdes/axp/high_speed_env_lib.c b/arch/arm/mach-mvebu/serdes/axp/high_speed_env_lib.c index 0b63664dd8be..68f8eade2722 100644 --- a/arch/arm/mach-mvebu/serdes/axp/high_speed_env_lib.c +++ b/arch/arm/mach-mvebu/serdes/axp/high_speed_env_lib.c @@ -62,7 +62,7 @@ static u32 board_id_get(void) return DB_78X60_AMC_ID; #elif defined(CONFIG_DB_78X60_PCAC_REV2) return DB_78X60_PCAC_REV2_ID; -#elif defined(CONFIG_DB_784MP_GP) +#elif defined(CONFIG_TARGET_DB_MV784MP_GP) return DB_784MP_GP_ID; #elif defined(CONFIG_RD_78460_CUSTOMER) return RD_78460_CUSTOMER_ID; diff --git a/drivers/ddr/marvell/axp/ddr3_axp.h b/drivers/ddr/marvell/axp/ddr3_axp.h index 970651f87029..a14c766dda71 100644 --- a/drivers/ddr/marvell/axp/ddr3_axp.h +++ b/drivers/ddr/marvell/axp/ddr3_axp.h @@ -37,7 +37,7 @@ #define ECC_SUPPORT #endif #define NEW_FABRIC_TWSI_ADDR 0x4E -#ifdef CONFIG_DB_784MP_GP +#ifdef CONFIG_TARGET_DB_MV784MP_GP #define BUS_WIDTH_ECC_TWSI_ADDR 0x4E #else #define BUS_WIDTH_ECC_TWSI_ADDR 0x4F diff --git a/drivers/ddr/marvell/axp/ddr3_axp_config.h b/drivers/ddr/marvell/axp/ddr3_axp_config.h index 437a02efbac9..ab09e72623a0 100644 --- a/drivers/ddr/marvell/axp/ddr3_axp_config.h +++ b/drivers/ddr/marvell/axp/ddr3_axp_config.h @@ -138,7 +138,7 @@ * Enables I2C auto detection different options */ #if defined(CONFIG_DB_88F78X60) || defined(CONFIG_DB_88F78X60_REV2) || \ -defined(CONFIG_DB_784MP_GP) +defined(CONFIG_TARGET_DB_MV784MP_GP) #define AUTO_DETECTION_SUPPORT #endif #endif diff --git a/include/configs/db-mv784mp-gp.h b/include/configs/db-mv784mp-gp.h index 41d469d79526..d6850bd32e72 100644 --- a/include/configs/db-mv784mp-gp.h +++ b/include/configs/db-mv784mp-gp.h @@ -6,11 +6,6 @@ #ifndef _CONFIG_DB_MV7846MP_GP_H #define _CONFIG_DB_MV7846MP_GP_H -/* - * High Level Configuration Options (easy to change) - */ -#define CONFIG_DB_784MP_GP /* Board target name for DDR training */ - /* * TEXT_BASE needs to be below 16MiB, since this area is scrubbed * for DDR ECC byte filling in the SPL before loading the main Viele Grüße, Stefan Roese -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH] doc: man-page for the wdt command
On 3/25/22 18:06, Heinrich Schuchardt wrote: Describe the wdt command. Signed-off-by: Heinrich Schuchardt Reviewed-by: Stefan Roese Thanks, Stefan --- doc/usage/index.rst | 1 + doc/usage/wdt.rst | 77 + 2 files changed, 78 insertions(+) create mode 100644 doc/usage/wdt.rst diff --git a/doc/usage/index.rst b/doc/usage/index.rst index 0aacf531b2..630a6cb28a 100644 --- a/doc/usage/index.rst +++ b/doc/usage/index.rst @@ -52,3 +52,4 @@ Shell commands size true ums + wdt diff --git a/doc/usage/wdt.rst b/doc/usage/wdt.rst new file mode 100644 index 00..8d80433c1f --- /dev/null +++ b/doc/usage/wdt.rst @@ -0,0 +1,77 @@ +.. SPDX-License-Identifier: GPL-2.0+: + +wdt command + + +Synopsis + + +:: + +wdt list +wdt dev [] +wdt start [flags] +wdt stop +wdt reset +wdt expirer [flags] + +Description +--- + +The wdt command is used to control watchdog timers. + +The 'wdt list' command shows a list of all watchdog devices. + +The 'wdt dev' command called without argument shows the current watchdog device. +The current device is set when passing the name of the device as argument. + +The 'wdt start' command starts the current watchdog timer. + +The 'wdt stop' command stops the current watchdog timer. + +The 'wdt reset' command resets the current watchdog timer without stopping it. + +The 'wdt expire' command let's the current watchdog timer expire immediately. +This will lead to a reset. + +name +name of the watchdog device + +timeout_ms +timeout interval in milliseconds + +flags +unsigned long value passed to the driver. The usage is driver specific. +The value is ignored by most drivers. + +Example +--- + +:: + +=> wdt dev +No watchdog timer device set! +=> wdt list +watchdog@1c20ca0 (sunxi_wdt) +=> wdt dev watchdog@1c20ca0 +=> wdt dev +dev: watchdog@1c20ca0 +=> wdt start 3000 +=> wdt reset +=> wdt stop +=> wdt expire + +U-Boot SPL 2022.04-rc3 (Mar 25 2022 - 13:48:33 +) + + In the example above '(sunxi_wdt)' refers to the driver for the watchdog + device. + +Configuration +- + +The command is only available if CONFIG_CMD_WDT=y. + +Return value + + +The return value $? is 0 if the command succeeds, 1 upon failure. Viele Grüße, Stefan Roese -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
[PATCH v1] net: eth-uclass: Fix eth_halt
From: qianfan Zhao eth_device_priv maybe unaccessable after @stop handler due to eth device is removed in @stop. Setting private data before @stop handler. This also fix data abort bug when run dhcp or tftp command via usbnet. Signed-off-by: qianfan Zhao --- net/eth-uclass.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/net/eth-uclass.c b/net/eth-uclass.c index 58c308f332..c6eb1bc8f8 100644 --- a/net/eth-uclass.c +++ b/net/eth-uclass.c @@ -338,9 +338,14 @@ void eth_halt(void) if (!priv || !priv->running) return; - eth_get_ops(current)->stop(current); - priv->state = ETH_STATE_PASSIVE; + /* Make sure setting private data before @stop handler, it may remove +* ethernet device and will cause @priv unaccessable. +* eg: +* usb_eth_stop -> usb_gadget_release -> device_remove +*/ priv->running = false; + + eth_get_ops(current)->stop(current); } int eth_is_active(struct udevice *dev) -- 2.17.1
[PATCH] doc: usage: Group all shell command docs into cmd/ sub-direcotry
Currently all shell command docs are put in the doc/usage root.
Let's group them into cmd/ sub-direcotry.
Signed-off-by: Bin Meng
---
doc/usage/{ => cmd}/acpi.rst | 0
doc/usage/{ => cmd}/addrmap.rst | 0
doc/usage/{ => cmd}/askenv.rst | 0
doc/usage/{ => cmd}/base.rst | 0
doc/usage/{ => cmd}/bootefi.rst | 0
doc/usage/{ => cmd}/booti.rst| 0
doc/usage/{ => cmd}/bootmenu.rst | 0
doc/usage/{ => cmd}/button.rst | 0
doc/usage/{x86 => cmd}/cbsysinfo.rst | 0
doc/usage/{ => cmd}/conitrace.rst| 0
doc/usage/{ => cmd}/echo.rst | 0
doc/usage/{ => cmd}/exception.rst| 0
doc/usage/{ => cmd}/exit.rst | 0
doc/usage/{ => cmd}/extension.rst| 0
doc/usage/{ => cmd}/false.rst| 0
doc/usage/{ => cmd}/fatinfo.rst | 0
doc/usage/{ => cmd}/fatload.rst | 0
doc/usage/{ => cmd}/for.rst | 0
doc/usage/{ => cmd}/load.rst | 0
doc/usage/{ => cmd}/loady.rst| 0
doc/usage/{ => cmd}/mbr.rst | 0
doc/usage/{ => cmd}/md.rst | 0
doc/usage/{ => cmd}/mmc.rst | 0
doc/usage/{ => cmd}/pinmux.rst | 0
doc/usage/{ => cmd}/pstore.rst | 0
doc/usage/{ => cmd}/qfw.rst | 0
doc/usage/{ => cmd}/reset.rst| 0
doc/usage/{ => cmd}/sbi.rst | 0
doc/usage/{ => cmd}/scp03.rst| 0
doc/usage/{ => cmd}/setexpr.rst | 0
doc/usage/{ => cmd}/sf.rst | 0
doc/usage/{ => cmd}/size.rst | 0
doc/usage/{ => cmd}/true.rst | 0
doc/usage/{ => cmd}/ums.rst | 0
doc/usage/index.rst | 68 ++--
35 files changed, 34 insertions(+), 34 deletions(-)
rename doc/usage/{ => cmd}/acpi.rst (100%)
rename doc/usage/{ => cmd}/addrmap.rst (100%)
rename doc/usage/{ => cmd}/askenv.rst (100%)
rename doc/usage/{ => cmd}/base.rst (100%)
rename doc/usage/{ => cmd}/bootefi.rst (100%)
rename doc/usage/{ => cmd}/booti.rst (100%)
rename doc/usage/{ => cmd}/bootmenu.rst (100%)
rename doc/usage/{ => cmd}/button.rst (100%)
rename doc/usage/{x86 => cmd}/cbsysinfo.rst (100%)
rename doc/usage/{ => cmd}/conitrace.rst (100%)
rename doc/usage/{ => cmd}/echo.rst (100%)
rename doc/usage/{ => cmd}/exception.rst (100%)
rename doc/usage/{ => cmd}/exit.rst (100%)
rename doc/usage/{ => cmd}/extension.rst (100%)
rename doc/usage/{ => cmd}/false.rst (100%)
rename doc/usage/{ => cmd}/fatinfo.rst (100%)
rename doc/usage/{ => cmd}/fatload.rst (100%)
rename doc/usage/{ => cmd}/for.rst (100%)
rename doc/usage/{ => cmd}/load.rst (100%)
rename doc/usage/{ => cmd}/loady.rst (100%)
rename doc/usage/{ => cmd}/mbr.rst (100%)
rename doc/usage/{ => cmd}/md.rst (100%)
rename doc/usage/{ => cmd}/mmc.rst (100%)
rename doc/usage/{ => cmd}/pinmux.rst (100%)
rename doc/usage/{ => cmd}/pstore.rst (100%)
rename doc/usage/{ => cmd}/qfw.rst (100%)
rename doc/usage/{ => cmd}/reset.rst (100%)
rename doc/usage/{ => cmd}/sbi.rst (100%)
rename doc/usage/{ => cmd}/scp03.rst (100%)
rename doc/usage/{ => cmd}/setexpr.rst (100%)
rename doc/usage/{ => cmd}/sf.rst (100%)
rename doc/usage/{ => cmd}/size.rst (100%)
rename doc/usage/{ => cmd}/true.rst (100%)
rename doc/usage/{ => cmd}/ums.rst (100%)
diff --git a/doc/usage/acpi.rst b/doc/usage/cmd/acpi.rst
similarity index 100%
rename from doc/usage/acpi.rst
rename to doc/usage/cmd/acpi.rst
diff --git a/doc/usage/addrmap.rst b/doc/usage/cmd/addrmap.rst
similarity index 100%
rename from doc/usage/addrmap.rst
rename to doc/usage/cmd/addrmap.rst
diff --git a/doc/usage/askenv.rst b/doc/usage/cmd/askenv.rst
similarity index 100%
rename from doc/usage/askenv.rst
rename to doc/usage/cmd/askenv.rst
diff --git a/doc/usage/base.rst b/doc/usage/cmd/base.rst
similarity index 100%
rename from doc/usage/base.rst
rename to doc/usage/cmd/base.rst
diff --git a/doc/usage/bootefi.rst b/doc/usage/cmd/bootefi.rst
similarity index 100%
rename from doc/usage/bootefi.rst
rename to doc/usage/cmd/bootefi.rst
diff --git a/doc/usage/booti.rst b/doc/usage/cmd/booti.rst
similarity index 100%
rename from doc/usage/booti.rst
rename to doc/usage/cmd/booti.rst
diff --git a/doc/usage/bootmenu.rst b/doc/usage/cmd/bootmenu.rst
similarity index 100%
rename from doc/usage/bootmenu.rst
rename to doc/usage/cmd/bootmenu.rst
diff --git a/doc/usage/button.rst b/doc/usage/cmd/button.rst
similarity index 100%
rename from doc/usage/button.rst
rename to doc/usage/cmd/button.rst
diff --git a/doc/usage/x86/cbsysinfo.rst b/doc/usage/cmd/cbsysinfo.rst
similarity index 100%
rename from doc/usage/x86/cbsysinfo.rst
rename to doc/usage/cmd/cbsysinfo.rst
diff --git a/doc/usage/conitrace.rst b/doc/usage/cmd/conitrace.rst
similarity index 100%
rename from doc/usage/conitrace.rst
rename to doc/usage/cmd/conitrace.rst
diff --git a/doc/usage/echo.rst b/doc/usage/cmd/echo.rst
similarity index 100%
rename from doc/usage/echo.rst
rename to doc/usage/cmd/echo.rst
diff --git a/doc/usage/exception.rst b/doc/usage/cmd
[PATCH 3/3] doc: usage: Convert README.plan9 to reST
This converts the existing README.plan9 to reST, and puts it under
the doc/usage/os directory.
Signed-off-by: Bin Meng
---
doc/usage/index.rst | 1 +
doc/{README.plan9 => usage/os/plan9.rst} | 12
2 files changed, 9 insertions(+), 4 deletions(-)
rename doc/{README.plan9 => usage/os/plan9.rst} (89%)
diff --git a/doc/usage/index.rst b/doc/usage/index.rst
index b8916971bc..ecb4908fd1 100644
--- a/doc/usage/index.rst
+++ b/doc/usage/index.rst
@@ -59,4 +59,5 @@ Booting OS
.. toctree::
:maxdepth: 1
+ os/plan9
os/vxworks
diff --git a/doc/README.plan9 b/doc/usage/os/plan9.rst
similarity index 89%
rename from doc/README.plan9
rename to doc/usage/os/plan9.rst
index 2d3d0e0cf6..f91712c009 100644
--- a/doc/README.plan9
+++ b/doc/usage/os/plan9.rst
@@ -1,3 +1,10 @@
+.. SPDX-License-Identifier: GPL-2.0+
+.. Steven Stallion
+.. June 2013
+
+Plan 9
+==
+
Plan 9 from Bell Labs kernel images require additional setup to pass
configuration information to the kernel. An environment variable named
confaddr must be defined with the same value as CONFADDR (see mem.h).
@@ -10,9 +17,6 @@ bootargs environment variable will be copied.
If no command line arguments or bootargs are defined, CONFADDR is left
uninitialized to permit manual configuration. For example, PC-style
-configuration could be simulated by issuing a fatload in bootcmd:
+configuration could be simulated by issuing a fatload in bootcmd::
# setenv bootcmd fatload mmc 0 $confaddr plan9.ini; ...; bootm
-
-Steven Stallion
-June 2013
--
2.25.1
[PATCH 2/3] doc: usage: Update vxworks doc to mention RISC-V support
At present the doc only mentions Arm, PowerPC and x86. RISC-V support has been added since VxWorks SR0650 support for a while, and U-Boot supports loading a RISC-V VxWorks kernel too. Let's document it. Signed-off-by: Bin Meng --- doc/usage/os/vxworks.rst | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/doc/usage/os/vxworks.rst b/doc/usage/os/vxworks.rst index e696c639b2..e732f711fb 100644 --- a/doc/usage/os/vxworks.rst +++ b/doc/usage/os/vxworks.rst @@ -13,13 +13,13 @@ Status U-Boot supports loading VxWorks kernels via 'bootvx' and 'bootm' commands. For booting old kernels (6.9.x) on PowerPC and ARM, and all kernel versions on other architectures, 'bootvx' shall be used. For booting VxWorks 7 kernels -on PowerPC and ARM, 'bootm' shall be used. +on PowerPC/ARM/RISC-V, 'bootm' shall be used. With CONFIG_EFI_LOADER option, it's possible to chain load a VxWorks x86 kernel via the UEFI boot loader application for VxWorks loaded by 'bootefi' command. -VxWorks 7 on PowerPC and ARM - +VxWorks 7 on PowerPC/ARM/RISC-V +--- From VxWorks 7, VxWorks starts adopting device tree as its hardware description mechanism (for PowerPC and ARM), thus requiring boot interface changes. This section will describe the new interface. @@ -45,6 +45,11 @@ For ARM, the calling convention is shown below:: When using the Linux compatible standard DTB, the calling convention of VxWorks entry point is exactly the same as the Linux kernel. +For RISC-V, there is no legacy bootm flow as VxWorks always uses the same boot +interface as the Linux kernel, with the calling convention below:: + +void (*kernel_entry)(unsigned long hartid, void *fdt_addr) + When booting a VxWorks 7 kernel (uImage format), the parameters passed to bootm is like below:: -- 2.25.1
[PATCH 1/3] doc: usage: Convert README.vxworks to reST
This converts the existing README.vxworks to reST, and puts it under
the doc/usage/os directory.
Signed-off-by: Bin Meng
---
doc/usage/index.rst | 8 +++
doc/{README.vxworks => usage/os/vxworks.rst} | 22 ++--
2 files changed, 19 insertions(+), 11 deletions(-)
rename doc/{README.vxworks => usage/os/vxworks.rst} (93%)
diff --git a/doc/usage/index.rst b/doc/usage/index.rst
index 0aacf531b2..b8916971bc 100644
--- a/doc/usage/index.rst
+++ b/doc/usage/index.rst
@@ -52,3 +52,11 @@ Shell commands
size
true
ums
+
+Booting OS
+--
+
+.. toctree::
+ :maxdepth: 1
+
+ os/vxworks
diff --git a/doc/README.vxworks b/doc/usage/os/vxworks.rst
similarity index 93%
rename from doc/README.vxworks
rename to doc/usage/os/vxworks.rst
index 12a0d744d8..e696c639b2 100644
--- a/doc/README.vxworks
+++ b/doc/usage/os/vxworks.rst
@@ -1,11 +1,10 @@
-# SPDX-License-Identifier: GPL-2.0+
-#
-# Copyright (C) 2013, Miao Yan
-# Copyright (C) 2015-2018, Bin Meng
-# Copyright (C) 2019, Lihua Zhao
+.. SPDX-License-Identifier: GPL-2.0+
+.. Copyright (C) 2013, Miao Yan
+.. Copyright (C) 2015-2018, Bin Meng
+.. Copyright (C) 2019, Lihua Zhao
-VxWorks Support
-===
+VxWorks
+===
This document describes the information about U-Boot loading VxWorks kernel.
@@ -20,7 +19,7 @@ With CONFIG_EFI_LOADER option, it's possible to chain load a
VxWorks x86 kernel
via the UEFI boot loader application for VxWorks loaded by 'bootefi' command.
VxWorks 7 on PowerPC and ARM
+
From VxWorks 7, VxWorks starts adopting device tree as its hardware description
mechanism (for PowerPC and ARM), thus requiring boot interface changes.
This section will describe the new interface.
@@ -35,11 +34,11 @@ For legacy bootm flow, make sure the least significant bit
of flags in bootargs
is cleared. The calling convention is described below:
For PowerPC, the calling convention of the new VxWorks entry point conforms to
-the ePAPR standard, which is shown below (see ePAPR for more details):
+the ePAPR standard, which is shown below (see ePAPR for more details)::
void (*kernel_entry)(fdt_addr, 0, 0, EPAPR_MAGIC, boot_IMA, 0, 0)
-For ARM, the calling convention is shown below:
+For ARM, the calling convention is shown below::
void (*kernel_entry)(void *fdt_addr)
@@ -47,7 +46,7 @@ When using the Linux compatible standard DTB, the calling
convention of VxWorks
entry point is exactly the same as the Linux kernel.
When booting a VxWorks 7 kernel (uImage format), the parameters passed to bootm
-is like below:
+is like below::
bootm -
@@ -108,6 +107,7 @@ BIOS of the graphics card first.
CONFIG_FRAMEBUFFER_SET_VESA_MODE need remain set but care must be taken
at which VESA mode is to be set. The supported pixel format is 32-bit
RGBA, hence the available VESA mode can only be one of the following:
+
* FRAMEBUFFER_VESA_MODE_10F
* FRAMEBUFFER_VESA_MODE_112
* FRAMEBUFFER_VESA_MODE_115
--
2.25.1
Re: [RFC PATCH 1/6] capsule: Add Image GUIDs for platforms using capsule updates
Hi Sughosh, 2022年3月27日(日) 18:11 Sughosh Ganu : > > hi Masami, > > On Sat, 26 Mar 2022 at 16:17, Masami Hiramatsu > wrote: > > > > Hi Sughosh, > > > > 2022年3月25日(金) 18:59 Sughosh Ganu : > > > > > > hi Masami, > > > > > > On Fri, 25 Mar 2022 at 10:58, Masami Hiramatsu > > > wrote: > > > > > > > > Hi Sughosh, > > > > > > > > OK I understand that if the platform uses the FIT capsule, the > > > > fw_images[] must have 1 entry and it is completely non relationship > > > > with dfu_alt_info... We may need a document for this case too. > > > > > > Actually, what you are stating above applies to both raw images as > > > well as FIT images. I have added a paragraph in the capsule update > > > related section in the uefi.rst. Can you check my patch 4 of this > > > series. Thanks. > > > > I've checked that you didn't change the FMP::set_image(), but updated > > FMP::get_image_info() to use the per-platform GUID list. > > Thus, the efi_fmp_find() ensures that the image type GUID in the > > capsule image is *included* in the platform GUID list (fw_images[] > > array). > > > > OK, at this point, it filters out the firmware image which is not > > supported on the platform. > > > > However, since you didn't update the FMP::set_image() and > > efi_capsule_update_firmware(), it directly uses the *index* number in > > the capsule image for updating the firmware. Is that correct? > > Yes, your observation is correct. The aim of this series is to fix the > issue of using a common GUID value across images and platforms for a > given FMP instance. OK, but I think this has made the thing complicated. Without this series, user will only care about the index number to make the capsule image and the GUID is correct for the platform, since the platform chooses FIP or RAW. With this series, user additionally needs to care about the combination of the index and the GUID of image type, because the U-Boot doesn't check the combination is correct or not. I would like to ask you to add this note in the document. Even if the U-Boot checks the combination, user still need to check it when making the capsule file. > > If so, if the platform supports several image types, the problem happens. > > Suppose that if the platform has TF-A and U-Boot, the DFU entity index > > are 1 and 2. > > And user missed to make a capsule file with index 1 for U-Boot image > > with U-Boot image type GUID of that platform. > > This capsule file passed the check in the efi_fmp_find(), because the > > GUID is included in the platform supported GUID list. However, > > FMP::set_image() will overwrite the TF-A with given U-Boot image > > without any error. > > > > I think we need one more patch to check the given image-index in the > > capsule image is correctly matched to the image-type GUID for safety. > > Yes, this is very much in the pipeline. I will be working on extending > the struct efi_fw_images to have information on the image index as > part of the structure. That is great! BTW, if we have 2 different dfu_alt_info entries for the same type of firmware, should we have different GUIDs for those? > But please note that what you mention above is > just making the process of capsule update more robust -- the current > implementation that we have in the capsule update module where the > image_index value is taken from the capsule is very much in compliance > with the UEFI specification. The current patch series is fixing an > issue which was not compliant with the spec. But what you are > suggesting above is on my Todo list, just that it is a separate task, > and not a fix as such. OK, I don't mind the actual robustness change, but please describe this scenario as a notice for users so that they can make capsule files correctly. Thank you, > > -sughosh > > > > > Thank you, > > > > > -sughosh > > > > > > > > > > > Thanks, > > > > > > > > 2022年3月25日(金) 10:09 Masami Hiramatsu : > > > > > > > > > > Hi Sughosh, > > > > > > > > > > 2022年3月24日(木) 23:40 Sughosh Ganu : > > > > > > > > > > > > hi Masami, > > > > > > > > > > > > On Thu, 24 Mar 2022 at 19:14, Masami Hiramatsu > > > > > > wrote: > > > > > > > > > > > > > > Hi Sughosh, > > > > > > > > > > > > > > 2022年3月24日(木) 21:39 Sughosh Ganu : > > > > > > > > > > > > > > > > Currently, all platforms that enable capsule updates do so using > > > > > > > > either EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID or > > > > > > > > EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID. This is based on the > > > > > > > > Firmware > > > > > > > > Management Protocol(FMP) instance used on the platform. > > > > > > > > However, this > > > > > > > > means that all platforms that enable a particular FMP instance > > > > > > > > have > > > > > > > > the same GUID value for all the updatable images, either the > > > > > > > > FIT image > > > > > > > > GUID or the raw image GUID, and that an image for some platform > > > > > > > > can be > > > > > > > > updated on any other platform which uses the same FMP instance. > > > > > > > > Another >
Re: [PATCH 5/8] usb: ehci-mx6: Allow building SDP for imx8mm
Hi Jaap, On Sun, Mar 27, 2022 at 6:19 PM Jaap Crezee wrote: > But then SDP boot still fails with: > > ( make flash.bin && uuu ./flash.bin ) > U-Boot SPL 2022.04-rc4-00079-gcbc05bba8c (Mar 27 2022 - 22:52:29 +0200) > PMIC: PFUZE100 ID=0x10 > Normal Boot > SPL: Unsupported Boot Device 9 > SPL: failed to boot from all boot devices (err=-6) > ### ERROR ### Please RESET the board ### > > I have attached my complete .config is that helps anyone. Have you tried applying this patch? https://lists.denx.de/pipermail/u-boot/2021-July/453633.html I need to resend the imx8mm_evk SDP support.
Re: [PATCH 5/8] usb: ehci-mx6: Allow building SDP for imx8mm
Hi all, On 7/3/21 21:58, Fabio Estevam wrote: After selecting USB SDP support for i.MX8MM, there are some build errors and warnings. Make the necessary adjustments for fixing the build. It seems this is also breaking imx8mq_evk on mainline (cbc05bba8cb7da62eae65f41e1b0ddbcadc06bba) as of now. What should be done to get it working on imx8mq_evk? It seems I can work around it a bit by configuring ... CONFIG_SPL_USB_SDP_SUPPORT=y CONFIG_SPL_SDP_USB_DEV=0 # CONFIG_CMD_USB_SDP is not set # CONFIG_USB_GADGET_BCM_UDC_OTG_PHY is not set CONFIG_USB_GADGET_DWC2_OTG=y # CONFIG_USB_GADGET_DWC2_OTG_PHY_BUS_WIDTH_8 is not set CONFIG_SDP_LOADADDR=0 CONFIG_USB_FUNCTION_SDP=y ... But then SDP boot still fails with: ( make flash.bin && uuu ./flash.bin ) U-Boot SPL 2022.04-rc4-00079-gcbc05bba8c (Mar 27 2022 - 22:52:29 +0200) PMIC: PFUZE100 ID=0x10 Normal Boot SPL: Unsupported Boot Device 9 SPL: failed to boot from all boot devices (err=-6) ### ERROR ### Please RESET the board ### I have attached my complete .config is that helps anyone. regards, Jaap Crezee# # Automatically generated file; DO NOT EDIT. # U-Boot 2022.04-rc4 Configuration # # # Compiler: aarch64-none-linux-gnu-gcc (GNU Toolchain for the A-profile Architecture 10.3-2021.07 (arm-10.29)) 10.3.1 20210621 # CONFIG_CREATE_ARCH_SYMLINK=y CONFIG_SYS_CACHE_SHIFT_6=y CONFIG_SYS_CACHELINE_SIZE=64 CONFIG_LINKER_LIST_ALIGN=8 # CONFIG_ARC is not set CONFIG_ARM=y # CONFIG_M68K is not set # CONFIG_MICROBLAZE is not set # CONFIG_MIPS is not set # CONFIG_NDS32 is not set # CONFIG_NIOS2 is not set # CONFIG_PPC is not set # CONFIG_RISCV is not set # CONFIG_SANDBOX is not set # CONFIG_SH is not set # CONFIG_X86 is not set # CONFIG_XTENSA is not set CONFIG_SYS_ARCH="arm" CONFIG_SYS_CPU="armv8" CONFIG_SYS_SOC="imx8m" CONFIG_SYS_VENDOR="freescale" CONFIG_SYS_BOARD="imx8mq_evk" CONFIG_SYS_CONFIG_NAME="imx8mq_evk" # CONFIG_SKIP_LOWLEVEL_INIT is not set # CONFIG_SPL_SKIP_LOWLEVEL_INIT is not set # CONFIG_TPL_SKIP_LOWLEVEL_INIT is not set # CONFIG_SKIP_LOWLEVEL_INIT_ONLY is not set # CONFIG_SPL_SKIP_LOWLEVEL_INIT_ONLY is not set # CONFIG_SYS_ICACHE_OFF is not set # CONFIG_SPL_SYS_ICACHE_OFF is not set # CONFIG_SYS_DCACHE_OFF is not set # CONFIG_SPL_SYS_DCACHE_OFF is not set # # ARM architecture # CONFIG_ARM64=y CONFIG_ARM64_CRC32=y # CONFIG_POSITION_INDEPENDENT is not set # CONFIG_INIT_SP_RELATIVE is not set # CONFIG_GIC_V3_ITS is not set CONFIG_STATIC_RELA=y CONFIG_DMA_ADDR_T_64BIT=y CONFIG_GPIO_EXTRA_HEADER=y CONFIG_ARM_ASM_UNIFIED=y # CONFIG_SYS_ARM_CACHE_CP15 is not set # CONFIG_SYS_ARM_MMU is not set # CONFIG_SYS_ARM_MPU is not set CONFIG_SYS_ARM_ARCH=8 CONFIG_SYS_ARM_CACHE_WRITEBACK=y # CONFIG_SYS_ARM_CACHE_WRITETHROUGH is not set # CONFIG_SYS_ARM_CACHE_WRITEALLOC is not set # CONFIG_ARCH_CPU_INIT is not set CONFIG_SYS_ARCH_TIMER=y CONFIG_ARM_SMCCC=y # CONFIG_SEMIHOSTING is not set # CONFIG_SYS_L2CACHE_OFF is not set # CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK is not set # CONFIG_USE_ARCH_MEMCPY is not set # CONFIG_SPL_USE_ARCH_MEMCPY is not set # CONFIG_USE_ARCH_MEMSET is not set # CONFIG_SPL_USE_ARCH_MEMSET is not set CONFIG_ARM64_SUPPORT_AARCH32=y # CONFIG_ARCH_AT91 is not set # CONFIG_ARCH_DAVINCI is not set # CONFIG_ARCH_KIRKWOOD is not set # CONFIG_ARCH_MVEBU is not set # CONFIG_ARCH_ORION5X is not set # CONFIG_TARGET_STV0991 is not set # CONFIG_ARCH_BCM283X is not set # CONFIG_ARCH_BCM63158 is not set # CONFIG_ARCH_BCM68360 is not set # CONFIG_ARCH_BCM6858 is not set # CONFIG_ARCH_BCMSTB is not set # CONFIG_TARGET_VEXPRESS_CA9X4 is not set # CONFIG_TARGET_BCMCYGNUS is not set # CONFIG_TARGET_BCMNS2 is not set # CONFIG_TARGET_BCMNS3 is not set # CONFIG_ARCH_EXYNOS is not set # CONFIG_ARCH_S5PC1XX is not set # CONFIG_ARCH_HIGHBANK is not set # CONFIG_ARCH_INTEGRATOR is not set # CONFIG_ARCH_IPQ40XX is not set # CONFIG_ARCH_KEYSTONE is not set # CONFIG_ARCH_K3 is not set # CONFIG_ARCH_OMAP2PLUS is not set # CONFIG_ARCH_MESON is not set # CONFIG_ARCH_MEDIATEK is not set # CONFIG_ARCH_LPC32XX is not set # CONFIG_ARCH_IMX8 is not set CONFIG_ARCH_IMX8M=y # CONFIG_ARCH_IMX8ULP is not set # CONFIG_ARCH_IMXRT is not set # CONFIG_ARCH_MX23 is not set # CONFIG_ARCH_MX28 is not set # CONFIG_ARCH_MX31 is not set # CONFIG_ARCH_MX7ULP is not set # CONFIG_ARCH_MX7 is not set # CONFIG_ARCH_MX6 is not set CONFIG_SPL_LDSCRIPT="arch/arm/cpu/armv8/u-boot-spl.lds" # CONFIG_ARCH_MX5 is not set # CONFIG_ARCH_NEXELL is not set # CONFIG_ARCH_APPLE is not set # CONFIG_ARCH_OWL is not set # CONFIG_ARCH_QEMU is not set # CONFIG_ARCH_RMOBILE is not set # CONFIG_ARCH_SNAPDRAGON is not set # CONFIG_ARCH_SOCFPGA is not set # CONFIG_ARCH_SUNXI is not set # CONFIG_ARCH_U8500 is not set # CONFIG_ARCH_VERSAL is not set # CONFIG_ARCH_VF610 is not set # CONFIG_ARCH_ZYNQ is not set # CONFIG_ARCH_ZYNQMP_R5 is not set # CONFIG_ARCH_ZYNQMP is not set # CONFIG_ARCH_TEGRA is not set # CONFIG_TARGET_VEXPRESS64_AEMV8A is not set # CONFIG_TARGET_VEXPRESS64_BASE_FVP is not set # CONFIG_TARGET_VEXPRESS64_JUNO is not
[PATCH 10/10] WIP: dm: core: Add a command to calculate memory usage
Driver model can use a lot of memory, as it is the core of all drivers
and devices in U-Boot. Add a command to show how much is in use, along
with the sizes of various data structures.
This patch can be used to analyse the impact of various potential changes
to driver model for SPL, none of which has been implemented. Most involve
shrinking the size of struct udevice, which is a particular problem on
64-bit machines since their pointers are so unnecessarily large in SPL.
To try this out, build and run on your board. You should see output for
SPL and U-Boot proper, like this:
Struct sizes: udevice 90, driver 78, uclass 30, uc_driver 78
Memory: device 11:990, device names 111, uclass a:1e0
Attached typeCount SizeCur Tags Save
--- - - - - -
plat 3 e0990914 7c (124)
parent_plat 2 40990910 80 (128)
uclass_plat 1 1099090c 84 (132)
priv 613d990920 70 (112)
parent_priv 0 0990908 88 (136)
uclass_priv 3 38990914 7c (124)
driver_data 0 0990908 88 (136)
uclass 0 0
Attached total f2a5 37c (892)
tags 0 0
Total size: e15 (3605)
With tags: a99 (2713)
- singly-linked: 901 (2305)
- driver index: 88a (2186)
- uclass index: 813 (2067)
Drop device name (not SRAM): 111 (273)
Signed-off-by: Simon Glass
---
cmd/dm.c | 15 -
common/spl/spl.c | 9 ++
drivers/core/device.c | 65 ++
drivers/core/dump.c | 73 +++
drivers/core/root.c | 53 +++
drivers/core/tag.c| 29 +
include/dm/device.h | 6
include/dm/root.h | 45 ++
include/dm/tag.h | 18 ++-
include/dm/util.h | 9 ++
10 files changed, 320 insertions(+), 2 deletions(-)
diff --git a/cmd/dm.c b/cmd/dm.c
index 1dd19fe45b5..ec32a13b177 100644
--- a/cmd/dm.c
+++ b/cmd/dm.c
@@ -64,6 +64,17 @@ static int do_dm_dump_static_driver_info(struct cmd_tbl
*cmdtp, int flag, int ar
return 0;
}
+static int do_dm_dump_mem(struct cmd_tbl *cmdtp, int flag, int argc,
+ char *const argv[])
+{
+ struct dm_stats mem;
+
+ dm_get_mem(&mem);
+ dm_dump_mem(&mem);
+
+ return 0;
+}
+
static struct cmd_tbl test_commands[] = {
U_BOOT_CMD_MKENT(tree, 0, 1, do_dm_dump_all, "", ""),
U_BOOT_CMD_MKENT(uclass, 1, 1, do_dm_dump_uclass, "", ""),
@@ -71,6 +82,7 @@ static struct cmd_tbl test_commands[] = {
U_BOOT_CMD_MKENT(drivers, 1, 1, do_dm_dump_drivers, "", ""),
U_BOOT_CMD_MKENT(compat, 1, 1, do_dm_dump_driver_compat, "", ""),
U_BOOT_CMD_MKENT(static, 1, 1, do_dm_dump_static_driver_info, "", ""),
+ U_BOOT_CMD_MKENT(mem, 1, 1, do_dm_dump_mem, "", ""),
};
static __maybe_unused void dm_reloc(void)
@@ -114,5 +126,6 @@ U_BOOT_CMD(
"dm devresDump list of device resources for each device\n"
"dm drivers Dump list of drivers with uclass and instances\n"
"dm compatDump list of drivers with compatibility strings\n"
- "dm staticDump list of drivers with static platform data"
+ "dm staticDump list of drivers with static platform data\n"
+ "dm mem Provide a summary of memory usage"
);
diff --git a/common/spl/spl.c b/common/spl/spl.c
index b452d4feeb2..3182f774f13 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -32,6 +32,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -743,6 +744,14 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
}
}
+ /* Dump drive model states to aid analysis */
+ if (1) {
+ struct dm_stats mem;
+
+ dm_get_mem(&mem);
+ dm_dump_mem(&mem);
+ }
+
#if CONFIG_IS_ENABLED(BOARD_INIT)
spl_board_init();
#endif
diff --git a/drivers/core/device.c b/drivers/core/device.c
index 3ab2583df38..5d9dca8a81f 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -680,6 +680,71 @@ void *dev_get_parent_priv(const struct udevice *dev)
return dm_priv_to_rw(dev->parent_priv_);
}
+void *dev_get_attach(const struct udevice *dev, enum dm_tag_t tag)
+{
+ switch (tag) {
+ case DM_TAG_PLAT:
+ return dev_get_plat(dev);
+ case DM_TAG_PARENT_PLAT:
+ return dev_get_parent_plat(dev);
+ case DM_TAG_UC_PLAT:
+ return dev_get_uclass_plat(dev);
+ case DM_TAG_PRIV:
+ return dev_get_priv(dev);
+ case DM_TAG_PARENT_PRIV:
+ return dev_get_parent_priv(dev);
+ case DM_TAG_UC_PRIV:
+
[PATCH 09/10] RFC: dm: add tag support
From: AKASHI Takahiro
Note: This patch is *still* pending, so it is included in this series
just to make it work.
With dm-tag feature, any U-Boot subsystem is allowed to associate
arbitrary number of data with a particular udevice. This can been
see as expanding "struct udevice" without modifying the definition.
As a first user, UEFI subsystem makes use of tags to associate
an efi_disk object with a block device.
Signed-off-by: AKASHI Takahiro
Reviewed-by: Simon Glass
Signed-off-by: Simon Glass
---
drivers/core/Makefile | 2 +-
drivers/core/root.c | 2 +
drivers/core/tag.c| 139 ++
include/asm-generic/global_data.h | 4 +
include/dm/tag.h | 110 +++
5 files changed, 256 insertions(+), 1 deletion(-)
create mode 100644 drivers/core/tag.c
create mode 100644 include/dm/tag.h
diff --git a/drivers/core/Makefile b/drivers/core/Makefile
index 0cbc3ab217e..7099073a533 100644
--- a/drivers/core/Makefile
+++ b/drivers/core/Makefile
@@ -2,7 +2,7 @@
#
# Copyright (c) 2013 Google, Inc
-obj-y += device.o fdtaddr.o lists.o root.o uclass.o util.o
+obj-y += device.o fdtaddr.o lists.o root.o uclass.o util.o tag.o
obj-$(CONFIG_$(SPL_TPL_)ACPIGEN) += acpi.o
obj-$(CONFIG_$(SPL_TPL_)DEVRES) += devres.o
obj-$(CONFIG_$(SPL_)DM_DEVICE_REMOVE) += device-remove.o
diff --git a/drivers/core/root.c b/drivers/core/root.c
index 6d1a4097e14..e09c12f4d6e 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -205,6 +205,8 @@ int dm_init(bool of_live)
return ret;
}
+ INIT_LIST_HEAD((struct list_head *)&gd->dmtag_list);
+
return 0;
}
diff --git a/drivers/core/tag.c b/drivers/core/tag.c
new file mode 100644
index 000..6829bcd8806
--- /dev/null
+++ b/drivers/core/tag.c
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2021 Linaro Limited
+ * Author: AKASHI Takahiro
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct udevice;
+
+DECLARE_GLOBAL_DATA_PTR;
+
+int dev_tag_set_ptr(struct udevice *dev, enum dm_tag_t tag, void *ptr)
+{
+ struct dmtag_node *node;
+
+ if (!dev || tag >= DM_TAG_COUNT)
+ return -EINVAL;
+
+ list_for_each_entry(node, &gd->dmtag_list, sibling) {
+ if (node->dev == dev && node->tag == tag)
+ return -EEXIST;
+ }
+
+ node = calloc(sizeof(*node), 1);
+ if (!node)
+ return -ENOSPC;
+
+ node->dev = dev;
+ node->tag = tag;
+ node->ptr = ptr;
+ list_add_tail(&node->sibling, (struct list_head *)&gd->dmtag_list);
+
+ return 0;
+}
+
+int dev_tag_set_val(struct udevice *dev, enum dm_tag_t tag, ulong val)
+{
+ struct dmtag_node *node;
+
+ if (!dev || tag >= DM_TAG_COUNT)
+ return -EINVAL;
+
+ list_for_each_entry(node, &gd->dmtag_list, sibling) {
+ if (node->dev == dev && node->tag == tag)
+ return -EEXIST;
+ }
+
+ node = calloc(sizeof(*node), 1);
+ if (!node)
+ return -ENOSPC;
+
+ node->dev = dev;
+ node->tag = tag;
+ node->val = val;
+ list_add_tail(&node->sibling, (struct list_head *)&gd->dmtag_list);
+
+ return 0;
+}
+
+int dev_tag_get_ptr(struct udevice *dev, enum dm_tag_t tag, void **ptrp)
+{
+ struct dmtag_node *node;
+
+ if (!dev || tag >= DM_TAG_COUNT)
+ return -EINVAL;
+
+ list_for_each_entry(node, &gd->dmtag_list, sibling) {
+ if (node->dev == dev && node->tag == tag) {
+ *ptrp = node->ptr;
+ return 0;
+ }
+ }
+
+ return -ENOENT;
+}
+
+int dev_tag_get_val(struct udevice *dev, enum dm_tag_t tag, ulong *valp)
+{
+ struct dmtag_node *node;
+
+ if (!dev || tag >= DM_TAG_COUNT)
+ return -EINVAL;
+
+ list_for_each_entry(node, &gd->dmtag_list, sibling) {
+ if (node->dev == dev && node->tag == tag) {
+ *valp = node->val;
+ return 0;
+ }
+ }
+
+ return -ENOENT;
+}
+
+int dev_tag_del(struct udevice *dev, enum dm_tag_t tag)
+{
+ struct dmtag_node *node, *tmp;
+
+ if (!dev || tag >= DM_TAG_COUNT)
+ return -EINVAL;
+
+ list_for_each_entry_safe(node, tmp, &gd->dmtag_list, sibling) {
+ if (node->dev == dev && node->tag == tag) {
+ list_del(&node->sibling);
+ free(node);
+
+ return 0;
+ }
+ }
+
+ return -ENOENT;
+}
+
+int dev_tag_del_all(struct udevice *dev)
+{
+ struct dmtag_node *node, *tmp;
+ bool found = false;
+
+ if (!dev)
+ return -EINVAL;
+
+ list_for_each_entry_safe(node, tmp, &gd->dmtag_list, sibling) {
[PATCH 08/10] dm: core: Deal with a wrinkle with linker lists
When every member of a linker list is aligned by the compiler, we can no
longer rely on the sizeof of the struct to determine the number of
entries.
For example, if the struct size is 0x90 but every entry is aligned to 0xa0
by the compiler, the linker list entries takes more space in memory and
the calculation of the number of entries is incorrect. For example, we may
see 0x12 entries when there are only 0x11.
This is a real problem. There may be a general solution, although I cannot
currently think of one. So far it only bites with OF_PLATDATA_RT which
creates a pointer to each entry of the 'struct udevice' linker_list. This
does not happen without that option, so it only affects SPL.
Work around it by manually calculating the aligned size of struct udevice,
then using that for the n_ent calculation.
Note: the alignment fix to linker list was here:
0b2fa98aa5e linker_lists: Fix alignment issue
Signed-off-by: Simon Glass
---
drivers/core/device.c | 3 ++-
drivers/core/root.c | 8 +++-
include/dm/device.h | 8
3 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/core/device.c b/drivers/core/device.c
index b7ce8544140..3ab2583df38 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -1186,7 +1186,8 @@ int dev_enable_by_path(const char *path)
static struct udevice_rt *dev_get_rt(const struct udevice *dev)
{
struct udevice *base = ll_entry_start(struct udevice, udevice);
- int idx = dev - base;
+ uint each_size = dm_udevice_size();
+ int idx = ((void *)dev - (void *)base) / each_size;
struct udevice_rt *urt = gd_dm_udevice_rt() + idx;
diff --git a/drivers/core/root.c b/drivers/core/root.c
index 8efb4256b27..6d1a4097e14 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -136,12 +136,18 @@ static int dm_setup_inst(void)
if (CONFIG_IS_ENABLED(OF_PLATDATA_RT)) {
struct udevice_rt *urt;
+ void *start, *end;
+ int each_size;
void *base;
int n_ents;
uint size;
/* Allocate the udevice_rt table */
- n_ents = ll_entry_count(struct udevice, udevice);
+ each_size = dm_udevice_size();
+ start = ll_entry_start(struct udevice, udevice);
+ end = ll_entry_end(struct udevice, udevice);
+ size = end - start;
+ n_ents = size / each_size;
urt = calloc(n_ents, sizeof(struct udevice_rt));
if (!urt)
return log_msg_ret("urt", -ENOMEM);
diff --git a/include/dm/device.h b/include/dm/device.h
index 3d8961f9ac6..e0f86f5df9f 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -192,6 +192,14 @@ struct udevice {
#endif
};
+static inline int dm_udevice_size(void)
+{
+ if (CONFIG_IS_ENABLED(OF_PLATDATA_RT))
+ return ALIGN(sizeof(struct udevice), CONFIG_LINKER_LIST_ALIGN);
+
+ return sizeof(struct udevice);
+}
+
/**
* struct udevice_rt - runtime information set up by U-Boot
*
--
2.35.1.1021.g381101b075-goog
[PATCH 07/10] dm: core: Allow devres to be disabled in SPL
At present if devres is enabled in U-Boot proper it is enabled in SPL.
We don't normally want it there, so disable it.
Signed-off-by: Simon Glass
---
drivers/core/Makefile| 2 +-
drivers/core/device.c| 2 +-
include/dm/device-internal.h | 6 +++---
include/dm/device.h | 2 +-
include/dm/devres.h | 4 ++--
test/dm/Makefile | 2 +-
6 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/core/Makefile b/drivers/core/Makefile
index 5edd4e41357..0cbc3ab217e 100644
--- a/drivers/core/Makefile
+++ b/drivers/core/Makefile
@@ -4,7 +4,7 @@
obj-y += device.o fdtaddr.o lists.o root.o uclass.o util.o
obj-$(CONFIG_$(SPL_TPL_)ACPIGEN) += acpi.o
-obj-$(CONFIG_DEVRES) += devres.o
+obj-$(CONFIG_$(SPL_TPL_)DEVRES) += devres.o
obj-$(CONFIG_$(SPL_)DM_DEVICE_REMOVE) += device-remove.o
obj-$(CONFIG_$(SPL_)SIMPLE_BUS)+= simple-bus.o
obj-$(CONFIG_SIMPLE_PM_BUS)+= simple-pm-bus.o
diff --git a/drivers/core/device.c b/drivers/core/device.c
index 1b356f12dd8..b7ce8544140 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -68,7 +68,7 @@ static int device_bind_common(struct udevice *parent, const
struct driver *drv,
INIT_LIST_HEAD(&dev->sibling_node);
INIT_LIST_HEAD(&dev->child_head);
INIT_LIST_HEAD(&dev->uclass_node);
-#ifdef CONFIG_DEVRES
+#if CONFIG_IS_ENABLED(DEVRES)
INIT_LIST_HEAD(&dev->devres_head);
#endif
dev_set_plat(dev, plat);
diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h
index c420726287e..2fc41f31f5a 100644
--- a/include/dm/device-internal.h
+++ b/include/dm/device-internal.h
@@ -396,7 +396,7 @@ fdt_addr_t simple_bus_translate(struct udevice *dev,
fdt_addr_t addr);
#define DM_UCLASS_ROOT_S_NON_CONST (((gd_t *)gd)->uclass_root_s)
/* device resource management */
-#ifdef CONFIG_DEVRES
+#if CONFIG_IS_ENABLED(DEVRES)
/**
* devres_release_probe - Release managed resources allocated after probing
@@ -416,7 +416,7 @@ void devres_release_probe(struct udevice *dev);
*/
void devres_release_all(struct udevice *dev);
-#else /* ! CONFIG_DEVRES */
+#else /* ! DEVRES */
static inline void devres_release_probe(struct udevice *dev)
{
@@ -426,7 +426,7 @@ static inline void devres_release_all(struct udevice *dev)
{
}
-#endif /* ! CONFIG_DEVRES */
+#endif /* DEVRES */
static inline int device_notify(const struct udevice *dev, enum event_t type)
{
diff --git a/include/dm/device.h b/include/dm/device.h
index cb52a0997c8..3d8961f9ac6 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -184,7 +184,7 @@ struct udevice {
#if CONFIG_IS_ENABLED(OF_REAL)
ofnode node_;
#endif
-#ifdef CONFIG_DEVRES
+#if CONFIG_IS_ENABLED(DEVRES)
struct list_head devres_head;
#endif
#if CONFIG_IS_ENABLED(DM_DMA)
diff --git a/include/dm/devres.h b/include/dm/devres.h
index 0ab277ec38e..697534aa5be 100644
--- a/include/dm/devres.h
+++ b/include/dm/devres.h
@@ -30,7 +30,7 @@ struct devres_stats {
int total_size;
};
-#ifdef CONFIG_DEVRES
+#if CONFIG_IS_ENABLED(DEVRES)
#ifdef CONFIG_DEBUG_DEVRES
void *__devres_alloc(dr_release_t release, size_t size, gfp_t gfp,
@@ -207,7 +207,7 @@ void devm_kfree(struct udevice *dev, void *ptr);
/* Get basic stats on allocations */
void devres_get_stats(const struct udevice *dev, struct devres_stats *stats);
-#else /* ! CONFIG_DEVRES */
+#else /* ! DEVRES */
static inline void *devres_alloc(dr_release_t release, size_t size, gfp_t gfp)
{
diff --git a/test/dm/Makefile b/test/dm/Makefile
index d46552fbf32..9a1a904d906 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -32,7 +32,7 @@ obj-$(CONFIG_CLK) += clk.o clk_ccf.o
obj-$(CONFIG_CPU) += cpu.o
obj-$(CONFIG_CROS_EC) += cros_ec.o
obj-$(CONFIG_PWM_CROS_EC) += cros_ec_pwm.o
-obj-$(CONFIG_DEVRES) += devres.o
+obj-$(CONFIG_$(SPL_TPL_)DEVRES) += devres.o
obj-$(CONFIG_DMA) += dma.o
obj-$(CONFIG_VIDEO_MIPI_DSI) += dsi_host.o
obj-$(CONFIG_DM_DSA) += dsa.o
--
2.35.1.1021.g381101b075-goog
[PATCH 06/10] sandbox: Align linker lists to a 32-byte boundary
Use this larger boundary to ensure that linker lists at least start on the
maximum possible alignment boundary. See also the CONFIG_LINKER_LIST_ALIGN
setting, but that is host-arch-specific, so it seems better to use the
largest value for every host architecture.
Signed-off-by: Simon Glass
---
arch/sandbox/cpu/u-boot-spl.lds | 2 +-
arch/sandbox/cpu/u-boot.lds | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/sandbox/cpu/u-boot-spl.lds b/arch/sandbox/cpu/u-boot-spl.lds
index 6754f4ef6cc..206e265e74b 100644
--- a/arch/sandbox/cpu/u-boot-spl.lds
+++ b/arch/sandbox/cpu/u-boot-spl.lds
@@ -8,7 +8,7 @@
SECTIONS
{
- . = ALIGN(4);
+ . = ALIGN(32);
.u_boot_list : {
KEEP(*(SORT(.u_boot_list*)));
}
diff --git a/arch/sandbox/cpu/u-boot.lds b/arch/sandbox/cpu/u-boot.lds
index 6d710618f59..92e834a8d2b 100644
--- a/arch/sandbox/cpu/u-boot.lds
+++ b/arch/sandbox/cpu/u-boot.lds
@@ -8,7 +8,7 @@
SECTIONS
{
- . = ALIGN(4);
+ . = ALIGN(32);
.u_boot_list : {
KEEP(*(SORT(.u_boot_list*)));
}
--
2.35.1.1021.g381101b075-goog
[PATCH 05/10] sandbox: Allow link flags to be given
At present the link flags are not used for sandbox. Update the command line to use them. Signed-off-by: Simon Glass --- arch/sandbox/config.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk index 2b1b657831c..02a3ba0c0e9 100644 --- a/arch/sandbox/config.mk +++ b/arch/sandbox/config.mk @@ -16,7 +16,7 @@ PLATFORM_CPPFLAGS += $(shell $(SDL_CONFIG) --cflags) endif cmd_u-boot__ = $(CC) -o $@ -Wl,-T u-boot.lds $(u-boot-init) \ - $(LTO_FINAL_LDFLAGS) \ + $(KBUILD_LDFLAGS:%=-Wl,%)$(LTO_FINAL_LDFLAGS) \ -Wl,--whole-archive \ $(u-boot-main) \ $(u-boot-keep-syms-lto) \ @@ -24,7 +24,7 @@ cmd_u-boot__ = $(CC) -o $@ -Wl,-T u-boot.lds $(u-boot-init) \ $(PLATFORM_LIBS) -Wl,-Map -Wl,u-boot.map cmd_u-boot-spl = (cd $(obj) && $(CC) -o $(SPL_BIN) -Wl,-T u-boot-spl.lds \ - $(LTO_FINAL_LDFLAGS) \ + $(KBUILD_LDFLAGS:%=-Wl,%) $(LTO_FINAL_LDFLAGS) \ $(patsubst $(obj)/%,%,$(u-boot-spl-init)) \ -Wl,--whole-archive \ $(patsubst $(obj)/%,%,$(u-boot-spl-main)) \ -- 2.35.1.1021.g381101b075-goog
[PATCH 04/10] Makefile: Avoid resetting link flags in config.mk
This makes it impossible to change them elsewhere. The default value is 'empty' anyway, so just drop it. Signed-off-by: Simon Glass --- config.mk | 1 - 1 file changed, 1 deletion(-) diff --git a/config.mk b/config.mk index 2595aed218b..b915c29b3f3 100644 --- a/config.mk +++ b/config.mk @@ -12,7 +12,6 @@ # If we did not have Tegra SoCs, build system would be much simpler...) PLATFORM_RELFLAGS := PLATFORM_CPPFLAGS := -KBUILD_LDFLAGS := LDFLAGS_FINAL := LDFLAGS_STANDALONE := OBJCOPYFLAGS := -- 2.35.1.1021.g381101b075-goog
[PATCH 03/10] Makefile: Drop a stale comment about linking
The bug mentioned here is fixed, so drop the comment. Signed-off-by: Simon Glass --- Makefile | 4 1 file changed, 4 deletions(-) diff --git a/Makefile b/Makefile index c9585ddebfc..e23c3204d21 100644 --- a/Makefile +++ b/Makefile @@ -1785,10 +1785,6 @@ quiet_cmd_u-boot__ ?= LTO $@ -Wl,-Map,u-boot.map; \ $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true) else -# Note: Linking efi-x86_app64 causes a segfault in the linker at present -# when using x86_64-linux-gnu-ld.bfd -# For now, disable --whole-archive which makes things link, although not -# correctly quiet_cmd_u-boot__ ?= LD $@ cmd_u-boot__ ?= $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_u-boot) -o $@ \ -T u-boot.lds $(u-boot-init) \ -- 2.35.1.1021.g381101b075-goog
[PATCH 02/10] sandbox: Correct loss of early output in SPL
At present fputc() is used before the console is available, then write()
is used. These are not compatible. Since fputc() buffers internally it is
better to use the write(), so that a partial line is immediately
displayed.
This has a slight effect on performance, but we are already using write()
for the vast majority of the output with no obvious impacts.
Signed-off-by: Simon Glass
---
arch/sandbox/cpu/os.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index d83c862182d..5ea54179176 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -644,7 +644,7 @@ int os_get_filesize(const char *fname, long long *size)
void os_putc(int ch)
{
- fputc(ch, stdout);
+ os_write(1, &ch, 1);
}
void os_puts(const char *str)
--
2.35.1.1021.g381101b075-goog
[PATCH 01/10] Makefile: v2 Allow LTO to be disabled for a build
LTO (Link-Time Optimisation) is an very useful feature which can
significantly reduce the size of U-Boot binaries. So far it has been
made available for selected ARM boards and sandbox.
However, incremental builds are much slower when LTO is used. For example,
an incremental build of sandbox takes 2.1 seconds on my machine, but 6.7
seconds with LTO enabled.
Add a LTO_BUILD=n parameter to the build, so it can be disabled during
development if needed, for faster builds.
Add some documentation about LTO while we are here.
Signed-off-by: Simon Glass
---
Makefile | 18 +-
arch/arm/config.mk | 4 ++--
arch/arm/include/asm/global_data.h | 2 +-
doc/build/gcc.rst | 17 +
scripts/Makefile.spl | 2 +-
5 files changed, 34 insertions(+), 9 deletions(-)
diff --git a/Makefile b/Makefile
index 06572ac07ee..c9585ddebfc 100644
--- a/Makefile
+++ b/Makefile
@@ -434,6 +434,9 @@ KBUILD_CFLAGS += -fshort-wchar -fno-strict-aliasing
KBUILD_AFLAGS := -D__ASSEMBLY__
KBUILD_LDFLAGS :=
+# Set this to "n" use of LTO for this build, e.g. LTO_BUILD=n
+LTO_BUILD ?= y
+
ifeq ($(cc-name),clang)
ifneq ($(CROSS_COMPILE),)
CLANG_TARGET := --target=$(notdir $(CROSS_COMPILE:%-=%))
@@ -643,6 +646,11 @@ export CFLAGS_EFI # Compiler flags to add when building
EFI app
export CFLAGS_NON_EFI # Compiler flags to remove when building EFI app
export EFI_TARGET # binutils target if EFI is natively supported
+export LTO_ENABLE
+
+# This is y if LTO is enabled for this build
+LTO_ENABLE=$(if $(CONFIG_LTO),$(LTO_BUILD),)
+
# If board code explicitly specified LDSCRIPT or CONFIG_SYS_LDSCRIPT, use
# that (or fail if absent). Otherwise, search for a linker script in a
# standard location.
@@ -690,16 +698,16 @@ endif
LTO_CFLAGS :=
LTO_FINAL_LDFLAGS :=
export LTO_CFLAGS LTO_FINAL_LDFLAGS
-ifdef CONFIG_LTO
+ifeq ($(LTO_ENABLE),y)
ifeq ($(cc-name),clang)
- LTO_CFLAGS += -flto
+ LTO_CFLAGS += -DLTO_ENABLE -flto
LTO_FINAL_LDFLAGS += -flto
AR = $(shell $(CC)
-print-prog-name=llvm-ar)
NM = $(shell $(CC)
-print-prog-name=llvm-nm)
else
NPROC := $(shell nproc 2>/dev/null || echo 1)
- LTO_CFLAGS += -flto=$(NPROC)
+ LTO_CFLAGS += -DLTO_ENABLE -flto=$(NPROC)
LTO_FINAL_LDFLAGS += -fuse-linker-plugin -flto=$(NPROC)
# use plugin aware tools
@@ -1740,7 +1748,7 @@ ARCH_POSTLINK := $(wildcard
$(srctree)/arch/$(ARCH)/Makefile.postlink)
# Generate linker list symbols references to force compiler to not optimize
# them away when compiling with LTO
-ifdef CONFIG_LTO
+ifeq ($(LTO_ENABLE),y)
u-boot-keep-syms-lto := keep-syms-lto.o
u-boot-keep-syms-lto_c := $(patsubst %.o,%.c,$(u-boot-keep-syms-lto))
@@ -1762,7 +1770,7 @@ endif
# Rule to link u-boot
# May be overridden by arch/$(ARCH)/config.mk
-ifdef CONFIG_LTO
+ifeq ($(LTO_ENABLE),y)
quiet_cmd_u-boot__ ?= LTO $@
cmd_u-boot__ ?=
\
$(CC) -nostdlib -nostartfiles
\
diff --git a/arch/arm/config.mk b/arch/arm/config.mk
index b107b1af27a..065dbec4064 100644
--- a/arch/arm/config.mk
+++ b/arch/arm/config.mk
@@ -15,11 +15,11 @@ CFLAGS_NON_EFI := -fno-pic -ffixed-r9 -ffunction-sections
-fdata-sections \
-fstack-protector-strong
CFLAGS_EFI := -fpic -fshort-wchar
-ifneq ($(CONFIG_LTO)$(CONFIG_USE_PRIVATE_LIBGCC),yy)
+ifneq ($(LTO_ENABLE)$(CONFIG_USE_PRIVATE_LIBGCC),yy)
LDFLAGS_FINAL += --gc-sections
endif
-ifndef CONFIG_LTO
+ifneq ($(LTO_ENABLE),y)
PLATFORM_RELFLAGS += -ffunction-sections -fdata-sections
endif
diff --git a/arch/arm/include/asm/global_data.h
b/arch/arm/include/asm/global_data.h
index 085e12b5d4d..b255b195aa0 100644
--- a/arch/arm/include/asm/global_data.h
+++ b/arch/arm/include/asm/global_data.h
@@ -98,7 +98,7 @@ struct arch_global_data {
#include
-#if defined(__clang__) || defined(CONFIG_LTO)
+#if defined(__clang__) || defined(LTO_ENABLE)
#define DECLARE_GLOBAL_DATA_PTR
#define gd get_gd()
diff --git a/doc/build/gcc.rst b/doc/build/gcc.rst
index 470a7aa3498..792be176aad 100644
--- a/doc/build/gcc.rst
+++ b/doc/build/gcc.rst
@@ -152,6 +152,23 @@ of dtc is new enough. It also makes sure that pylibfdt is
present, if needed
Note that the :doc:`tools` are always built with the included version of libfdt
so it is not possible to build U-Boot tools with a system libfdt, at present.
+Link-time optimisation (LTO)
+
+
+U-Boot supports link-time optimisation which can reduce the size of the final
+U-Boot binaries, particularly with SPL.
+
+At present this can be enabled by ARM boards
[PATCH 00/10] dm: Experiments for reducing SPL memory usage
This series explores using the (not yet applied) tag support in driver model to reduce memory usage in SPL. This is particularly important on 64-bit machines, which use a ridiculously large 8 bytes for each pointer into what what is sometimes only 64KB of available memory. The primary focus of this series is struct udevice, which can be shrunk in various ways: - Including devres_head only when DEVRES is enabled (i.e. not in SPL) - Using tags instead of pointers for attached data like plat_, priv_ and driver_data - Using singly linked lists, currently not supported in U-Boot - Using a table index for the driver pointer and uclass pointer Together these bring the size of struct udevice down from 0xa0 (160) bytes to 0x30 (48) bytes. Another option is to drop the device name, although this is a pain for debugging. It would also be possible to implement doubly linked lists with a 16-bit index into malloc space, in SPL, thus reducing the overhead in each node from 16 bytes to 2, or just using a fixed-size list for each data structure, since the number of items is quite limited in SPL. To implement the tag side of things, functions like dev_set_parent_priv() need to be modified to call dev_tag_set_ptr(), and dev_get_parent_priv() needs to call dev_tag_get_ptr(). Note there has been some previous work: - tiny-dm analysis[1] which we decided was too disruptive - build-time instantiation, to reduce SPL code size[2]. [1] https://patchwork.ozlabs.org/project/uboot/cover/20200702211004.1491489-1-...@chromium.org/ [2] https://u-boot.readthedocs.io/en/latest/develop/driver-model/of-plat.html#build-time-instantiation AKASHI Takahiro (1): RFC: dm: add tag support Simon Glass (9): Makefile: v2 Allow LTO to be disabled for a build sandbox: Correct loss of early output in SPL Makefile: Drop a stale comment about linking Makefile: Avoid resetting link flags in config.mk sandbox: Allow link flags to be given sandbox: Align linker lists to a 32-byte boundary dm: core: Allow devres to be disabled in SPL dm: core: Deal with a wrinkle with linker lists WIP: dm: core: Add a command to calculate memory usage Makefile | 22 ++-- arch/arm/config.mk | 4 +- arch/arm/include/asm/global_data.h | 2 +- arch/sandbox/config.mk | 4 +- arch/sandbox/cpu/os.c | 2 +- arch/sandbox/cpu/u-boot-spl.lds| 2 +- arch/sandbox/cpu/u-boot.lds| 2 +- cmd/dm.c | 15 ++- common/spl/spl.c | 9 ++ config.mk | 1 - doc/build/gcc.rst | 17 +++ drivers/core/Makefile | 4 +- drivers/core/device.c | 70 +++- drivers/core/dump.c| 73 + drivers/core/root.c| 63 ++- drivers/core/tag.c | 168 + include/asm-generic/global_data.h | 4 + include/dm/device-internal.h | 6 +- include/dm/device.h| 16 ++- include/dm/devres.h| 4 +- include/dm/root.h | 45 include/dm/tag.h | 126 ++ include/dm/util.h | 9 ++ scripts/Makefile.spl | 2 +- test/dm/Makefile | 2 +- 25 files changed, 640 insertions(+), 32 deletions(-) create mode 100644 drivers/core/tag.c create mode 100644 include/dm/tag.h -- 2.35.1.1021.g381101b075-goog
Re: [PATCH v2 13/23] pinctrl: sunxi: Add I2C pinmuxes
On Thu, 17 Mar 2022 22:54:10 -0500
Samuel Holland wrote:
Hi,
> Where multiple options were available, the one matching board.c and the
> device trees was chosen.
>
> Pin lists and mux values were taken from the Linux drivers.
>
> Signed-off-by: Samuel Holland
> ---
>
> (no changes since v1)
>
> drivers/pinctrl/sunxi/pinctrl-sunxi.c | 37 +++
> 1 file changed, 37 insertions(+)
>
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> index 2de793689a..3a2fbee324 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> @@ -225,6 +225,8 @@ static int sunxi_pinctrl_probe(struct udevice *dev)
> static const struct sunxi_pinctrl_function suniv_f1c100s_pinctrl_functions[]
> = {
> { "gpio_in",0 },
> { "gpio_out", 1 },
> + { "i2c0", 3 },/* PE11-PE12 */
> + { "i2c1", 3 },/* PD5-PD6 */
> #if IS_ENABLED(CONFIG_UART0_PORT_F)
> { "uart0", 3 },/* PF2-PF4 */
> #else
> @@ -243,6 +245,8 @@ static const struct sunxi_pinctrl_function
> sun4i_a10_pinctrl_functions[] = {
> { "emac", 2 },/* PA0-PA17 */
> { "gpio_in",0 },
> { "gpio_out", 1 },
> + { "i2c0", 2 },/* PB0-PB1 */
> + { "i2c1", 2 },/* PB18-PB19 */
> #if IS_ENABLED(CONFIG_UART0_PORT_F)
> { "uart0", 4 },/* PF2-PF4 */
> #else
> @@ -261,6 +265,8 @@ static const struct sunxi_pinctrl_function
> sun5i_a13_pinctrl_functions[] = {
> { "emac", 2 },/* PA0-PA17 */
> { "gpio_in",0 },
> { "gpio_out", 1 },
> + { "i2c0", 2 },/* PB0-PB1 */
> + { "i2c1", 2 },/* PB15-PB16 */
> #if IS_ENABLED(CONFIG_UART0_PORT_F)
> { "uart0", 4 },/* PF2-PF4 */
> #else
> @@ -280,6 +286,8 @@ static const struct sunxi_pinctrl_function
> sun6i_a31_pinctrl_functions[] = {
> { "gmac", 2 },/* PA0-PA27 */
> { "gpio_in",0 },
> { "gpio_out", 1 },
> + { "i2c0", 2 },/* PH14-PH15 */
> + { "i2c1", 2 },/* PH16-PH17 */
> #if IS_ENABLED(CONFIG_UART0_PORT_F)
> { "uart0", 3 },/* PF2-PF4 */
> #else
> @@ -297,6 +305,7 @@ static const struct sunxi_pinctrl_desc __maybe_unused
> sun6i_a31_pinctrl_desc = {
> static const struct sunxi_pinctrl_function sun6i_a31_r_pinctrl_functions[] =
> {
> { "gpio_in",0 },
> { "gpio_out", 1 },
> + { "s_i2c", 2 },/* PL0-PL1 */
> { "s_uart", 2 },/* PL2-PL3 */
> };
>
> @@ -312,6 +321,8 @@ static const struct sunxi_pinctrl_function
> sun7i_a20_pinctrl_functions[] = {
> { "gmac", 5 },/* PA0-PA17 */
> { "gpio_in",0 },
> { "gpio_out", 1 },
> + { "i2c0", 2 },/* PB0-PB1 */
> + { "i2c1", 2 },/* PB18-PB19 */
> #if IS_ENABLED(CONFIG_UART0_PORT_F)
> { "uart0", 4 },/* PF2-PF4 */
> #else
> @@ -329,6 +340,8 @@ static const struct sunxi_pinctrl_desc __maybe_unused
> sun7i_a20_pinctrl_desc = {
> static const struct sunxi_pinctrl_function sun8i_a23_pinctrl_functions[] = {
> { "gpio_in",0 },
> { "gpio_out", 1 },
> + { "i2c0", 2 },/* PH2-PH3 */
> + { "i2c1", 2 },/* PH4-PH5 */
> #if IS_ENABLED(CONFIG_UART0_PORT_F)
> { "uart0", 3 },/* PF2-PF4 */
> #endif
> @@ -346,6 +359,7 @@ static const struct sunxi_pinctrl_desc __maybe_unused
> sun8i_a23_pinctrl_desc = {
> static const struct sunxi_pinctrl_function sun8i_a23_r_pinctrl_functions[] =
> {
> { "gpio_in",0 },
> { "gpio_out", 1 },
> + { "s_i2c", 3 },/* PL0-PL1 */
> { "s_uart", 2 },/* PL2-PL3 */
> };
>
> @@ -359,6 +373,8 @@ static const struct sunxi_pinctrl_desc __maybe_unused
> sun8i_a23_r_pinctrl_desc =
> static const struct sunxi_pinctrl_function sun8i_a33_pinctrl_functions[] = {
> { "gpio_in",0 },
> { "gpio_out", 1 },
> + { "i2c0", 2 },/* PH2-PH3 */
> + { "i2c1", 2 },/* PH4-PH5 */
> #if IS_ENABLED(CONFIG_UART0_PORT_F)
> { "uart0", 3 },/* PF2-PF4 */
> #else
> @@ -379,6 +395,8 @@ static const struct sunxi_pinctrl_function
> sun8i_a83t_pinctrl_functions[] = {
> { "gmac", 4 },/* PD2-PD23 */
> { "gpio_in",0 },
> { "gpio_out", 1 },
> + { "i2c0", 2 },/* PH0-PH1 */
> + { "i2c1", 2 },/* PH2-PH3 */
> #if IS_ENABLED(CONFIG_UART0_PORT_F)
> { "uart0", 3 },/* PF2-PF4 */
> #else
> @@ -398,6 +416,7 @@ static const struct sunxi_pinctrl_desc __maybe_unused
> sun8i_a83t_pinctrl_desc =
> static const struct sunxi_pinctrl_function sun8i_a83t_r_pinctrl_functions[]
> = {
> { "gpio_in",0 },
> { "gpio_out", 1 },
> + { "s_i2c", 2 },/* PL8-PL9 */
> { "s_uart", 2 },/* PL2-PL3 */
> };
>
> @@ -412,6 +431,8 @@ static const struct sunxi_pinctrl_funct
[PATCH 7/7] binman: Refuse to replace sections for now
Binman interfaces allow attempts to replace any entry in the image with
arbitrary data. When trying to replace sections, the changes in the
section entry's data are not propagated to its child entries. This,
combined with how sections rebuild their contents from its children,
eventually causes the replaced contents to be silently overwritten by
rebuilt contents equivalent to the original data.
Add a simple test for replacing a section that is currently failing due
to this behaviour, and mark it as an expected failure. Also, raise an
error when replacing a section instead of silently pretending it was
replaced.
Signed-off-by: Alper Nebi Yasak
---
tools/binman/etype/section.py | 3 +++
tools/binman/ftest.py | 9
.../test/234_replace_section_simple.dts | 23 +++
3 files changed, 35 insertions(+)
create mode 100644 tools/binman/test/234_replace_section_simple.dts
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index ccac658c1831..bd67238b9199 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -788,6 +788,9 @@ def ReadChildData(self, child, decomp=True,
alt_format=None):
data = new_data
return data
+def WriteData(self, data, decomp=True):
+self.Raise("Replacing sections is not implemented yet")
+
def WriteChildData(self, child):
return True
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 43bec4a88841..058651cc18a0 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -5641,6 +5641,15 @@ def testReplaceFitSubentryLeafSmallerSize(self):
self.assertIsNotNone(path)
self.assertEqual(expected_fdtmap, fdtmap)
+@unittest.expectedFailure
+def testReplaceSectionSimple(self):
+"""Test replacing a simple section with arbitrary data"""
+new_data = b'w' * len(COMPRESS_DATA + U_BOOT_DATA)
+data, expected_fdtmap, _ = self._RunReplaceCmd(
+'section', new_data,
+dts='234_replace_section_simple.dts')
+self.assertEqual(new_data, data)
+
if __name__ == "__main__":
unittest.main()
diff --git a/tools/binman/test/234_replace_section_simple.dts
b/tools/binman/test/234_replace_section_simple.dts
new file mode 100644
index ..c9d5c3285615
--- /dev/null
+++ b/tools/binman/test/234_replace_section_simple.dts
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+ binman {
+ allow-repack;
+
+ u-boot-dtb {
+ };
+
+ section {
+ blob {
+ filename = "compress";
+ };
+
+ u-boot {
+ };
+ };
+
+ fdtmap {
+ };
+ };
+};
--
2.35.1
[PATCH 6/7] binman: Test replacing non-section entries in FIT subsections
A previous patch fixes binman to correctly extract FIT subentries. This
makes it easier to test replacing these entries as we can write tests
using an existing helper function that relies on extracting the replaced
entry.
Add tests that replace leaf entries in FIT subsections with data of
various sizes. Replacing the subsections or the whole FIT section does
not work yet due to the section contents being re-built from unreplaced
subentries' data.
Signed-off-by: Alper Nebi Yasak
---
tools/binman/ftest.py | 38 ++
1 file changed, 38 insertions(+)
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index a31568997f6f..43bec4a88841 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -5603,6 +5603,44 @@ def testExtractFitSubentries(self):
data = control.ReadEntry(image_fname, entry_path)
self.assertEqual(expected, data)
+def testReplaceFitSubentryLeafSameSize(self):
+"""Test replacing a FIT leaf subentry with same-size data"""
+new_data = b'x' * len(U_BOOT_DATA)
+data, expected_fdtmap, _ = self._RunReplaceCmd(
+'fit/kernel/u-boot', new_data,
+dts='233_fit_extract_replace.dts')
+self.assertEqual(new_data, data)
+
+path, fdtmap = state.GetFdtContents('fdtmap')
+self.assertIsNotNone(path)
+self.assertEqual(expected_fdtmap, fdtmap)
+
+def testReplaceFitSubentryLeafBiggerSize(self):
+"""Test replacing a FIT leaf subentry with bigger-size data"""
+new_data = b'ub' * len(U_BOOT_NODTB_DATA)
+data, expected_fdtmap, _ = self._RunReplaceCmd(
+'fit/fdt-1/u-boot-nodtb', new_data,
+dts='233_fit_extract_replace.dts')
+self.assertEqual(new_data, data)
+
+# Will be repacked, so fdtmap must change
+path, fdtmap = state.GetFdtContents('fdtmap')
+self.assertIsNotNone(path)
+self.assertNotEqual(expected_fdtmap, fdtmap)
+
+def testReplaceFitSubentryLeafSmallerSize(self):
+"""Test replacing a FIT leaf subentry with smaller-size data"""
+new_data = b'x'
+expected = new_data.ljust(len(U_BOOT_NODTB_DATA), b'\0')
+data, expected_fdtmap, _ = self._RunReplaceCmd(
+'fit/fdt-1/u-boot-nodtb', new_data,
+dts='233_fit_extract_replace.dts')
+self.assertEqual(expected, data)
+
+path, fdtmap = state.GetFdtContents('fdtmap')
+self.assertIsNotNone(path)
+self.assertEqual(expected_fdtmap, fdtmap)
+
if __name__ == "__main__":
unittest.main()
--
2.35.1
[PATCH 4/7] binman: Remove '/images/' fragment from FIT subentry paths
Binman FIT entry nodes describe their subentries in an 'images' subnode,
same as how they would be written for the mkimage executable. The entry
type initially manually managed its subentries keyed by their node paths
relative to its base node. It was later converted to a proper section
while still keeping the same keys for subentries.
These subentry keys of sections are used as path fragments, so they must
not contain the path separator character '/'. Otherwise, they won't be
addressable by binman extract/replace commands. Change these keys from
the '/images/foo' forms to the subentry node names. Extend the simple
FIT tests to check for this.
Signed-off-by: Alper Nebi Yasak
---
tools/binman/etype/fit.py | 13 -
tools/binman/ftest.py | 7 +++
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
index e0407715d819..035719871e04 100644
--- a/tools/binman/etype/fit.py
+++ b/tools/binman/etype/fit.py
@@ -384,7 +384,8 @@ def _add_entries(base_node, depth, node):
entry.ReadNode()
# The hash subnodes here are for mkimage, not binman.
entry.SetUpdateHash(False)
-self._entries[rel_path] = entry
+image_name = rel_path[len('/images/'):]
+self._entries[image_name] = entry
for subnode in node.subnodes:
_add_entries(base_node, depth + 1, subnode)
@@ -630,7 +631,8 @@ def _add_node(base_node, depth, node):
has_images = depth == 2 and in_images
if has_images:
-entry = self._priv_entries[rel_path]
+image_name = rel_path[len('/images/'):]
+entry = self._priv_entries[image_name]
data = entry.GetData()
fsw.property('data', bytes(data))
@@ -643,12 +645,12 @@ def _add_node(base_node, depth, node):
# fsw.add_node() or _add_node() for it.
pass
elif self.GetImage().generate and subnode.name.startswith('@'):
-entry = self._priv_entries.get(subnode_path)
+entry = self._priv_entries.get(subnode.name)
_gen_node(base_node, subnode, depth, in_images, entry)
# This is a generator (template) entry, so remove it from
# the list of entries used by PackEntries(), etc. Otherwise
# it will appear in the binman output
-to_remove.append(subnode_path)
+to_remove.append(subnode.name)
else:
with fsw.add_node(subnode.name):
_add_node(base_node, depth + 1, subnode)
@@ -693,7 +695,8 @@ def SetImagePos(self, image_pos):
fdt = Fdt.FromData(self.GetData())
fdt.Scan()
-for path, section in self._entries.items():
+for image_name, section in self._entries.items():
+path = f"/images/{image_name}"
node = fdt.GetNode(path)
data_prop = node.props.get("data")
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 3d79f82dff43..87c072ef9c9c 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -3761,6 +3761,13 @@ def _CheckSimpleFitData(self, fit_data, kernel_data,
fdt1_data):
self.assertEqual(len(kernel_data), int(data_sizes[0].split()[0]))
self.assertEqual(len(fdt1_data), int(data_sizes[1].split()[0]))
+# Check if entry listing correctly omits /images/
+image = control.images['image']
+fit_entry = image.GetEntries()['fit']
+subentries = list(fit_entry.GetEntries().keys())
+expected = ['kernel', 'fdt-1']
+self.assertEqual(expected, subentries)
+
def testSimpleFit(self):
"""Test an image with a FIT inside"""
data = self._DoReadFile('161_fit.dts')
--
2.35.1
[PATCH 5/7] binman: Create FIT subentries in the FIT section, not its parent
When reading images from a file, each entry's data is read from its
parent section as specified in the Entry.Create() call that created it.
The FIT entry type has been creating its subentries under its parent
(their grandparent), as creating them under the FIT entry resulted in an
error until FIT was converted into a proper section.
FIT subentries have their offsets relative to the FIT section, and
reading those offsets in the parent section results in wrong data. The
subentries rightfully belong under the FIT entries, so create them
there. Add tests checking that we can extract the correct data for a FIT
entry and its subentries.
Signed-off-by: Alper Nebi Yasak
---
tools/binman/etype/fit.py | 2 +-
tools/binman/ftest.py | 35 +
tools/binman/test/233_fit_extract_replace.dts | 74 +++
3 files changed, 110 insertions(+), 1 deletion(-)
create mode 100644 tools/binman/test/233_fit_extract_replace.dts
diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
index 035719871e04..12306623af67 100644
--- a/tools/binman/etype/fit.py
+++ b/tools/binman/etype/fit.py
@@ -380,7 +380,7 @@ def _add_entries(base_node, depth, node):
# section entries for them here to merge the content subnodes
# together and put the merged contents in the subimage node's
# 'data' property later.
-entry = Entry.Create(self.section, node, etype='section')
+entry = Entry.Create(self, node, etype='section')
entry.ReadNode()
# The hash subnodes here are for mkimage, not binman.
entry.SetUpdateHash(False)
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 87c072ef9c9c..a31568997f6f 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -5568,6 +5568,41 @@ def testReplaceResizeNoRepackSmallerSize(self):
self.assertIsNotNone(path)
self.assertEqual(expected_fdtmap, fdtmap)
+def testExtractFit(self):
+"""Test extracting a FIT section"""
+self._DoReadFileRealDtb('233_fit_extract_replace.dts')
+image_fname = tools.get_output_filename('image.bin')
+
+fit_data = control.ReadEntry(image_fname, 'fit')
+fit = fdt.Fdt.FromData(fit_data)
+fit.Scan()
+
+# Check subentry data inside the extracted fit
+for node_path, expected in [
+('/images/kernel', U_BOOT_DATA),
+('/images/fdt-1', U_BOOT_NODTB_DATA),
+('/images/scr-1', COMPRESS_DATA),
+]:
+node = fit.GetNode(node_path)
+data = fit.GetProps(node)['data'].bytes
+self.assertEqual(expected, data)
+
+def testExtractFitSubentries(self):
+"""Test extracting FIT section subentries"""
+self._DoReadFileRealDtb('233_fit_extract_replace.dts')
+image_fname = tools.get_output_filename('image.bin')
+
+for entry_path, expected in [
+('fit/kernel', U_BOOT_DATA),
+('fit/kernel/u-boot', U_BOOT_DATA),
+('fit/fdt-1', U_BOOT_NODTB_DATA),
+('fit/fdt-1/u-boot-nodtb', U_BOOT_NODTB_DATA),
+('fit/scr-1', COMPRESS_DATA),
+('fit/scr-1/blob', COMPRESS_DATA),
+]:
+data = control.ReadEntry(image_fname, entry_path)
+self.assertEqual(expected, data)
+
if __name__ == "__main__":
unittest.main()
diff --git a/tools/binman/test/233_fit_extract_replace.dts
b/tools/binman/test/233_fit_extract_replace.dts
new file mode 100644
index ..b44d05afe1af
--- /dev/null
+++ b/tools/binman/test/233_fit_extract_replace.dts
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ binman {
+ allow-repack;
+
+ fill {
+ size = <0x1000>;
+ fill-byte = [77];
+ };
+
+ fit {
+ description = "test-desc";
+ #address-cells = <1>;
+
+ images {
+ kernel {
+ description = "test u-boot";
+ type = "kernel";
+ arch = "arm64";
+ os = "linux";
+ compression = "none";
+ load = <>;
+ entry = <>;
+
+ u-boot {
+ };
+ };
+
+ fdt-1 {
+ description = "test u-boot-nodtb";
+ type = "flat_dt";
+ arch = "arm64";
+
[PATCH 3/7] binman: Don't reset offset/size if image doesn't allow repacking
When an image has the 'allow-repack' property, binman includes the
original offset and size properties from the image description in the
fdtmap. These are later used as the packing constraints when replacing
entries in an image, so other unconstrained entries can be freely
positioned.
Replacing an entry in an image without 'allow-repack' (and therefore the
original offsets) follows the same logic and results in entries being
merely concatenated. Instead, skip resetting the calculated offsets and
sizes to the missing originals for these images so that every entry is
constrained to its existing offset/size.
Add tests that replace an entry with smaller or equal-sized data, in an
image that doesn't allow repacking. Attempting to do so with bigger-size
data is already an error that is already being tested.
Signed-off-by: Alper Nebi Yasak
---
tools/binman/control.py | 2 +-
tools/binman/ftest.py | 21 +
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/tools/binman/control.py b/tools/binman/control.py
index e170aeae4fab..ce57dc7efc7b 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -303,7 +303,7 @@ def BeforeReplace(image, allow_resize):
# If repacking, drop the old offset/size values except for the original
# ones, so we are only left with the constraints.
-if allow_resize:
+if image.allow_repack and allow_resize:
image.ResetForPack()
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index da9733d39a6a..3d79f82dff43 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -5540,6 +5540,27 @@ def testReplaceCmdOtherWithBintool(self):
finally:
shutil.rmtree(tmpdir)
+def testReplaceResizeNoRepackSameSize(self):
+"""Test replacing entries with same-size data without repacking"""
+expected = b'x' * len(U_BOOT_DATA)
+data, expected_fdtmap, _ = self._RunReplaceCmd('u-boot', expected)
+self.assertEqual(expected, data)
+
+path, fdtmap = state.GetFdtContents('fdtmap')
+self.assertIsNotNone(path)
+self.assertEqual(expected_fdtmap, fdtmap)
+
+def testReplaceResizeNoRepackSmallerSize(self):
+"""Test replacing entries with smaller-size data without repacking"""
+new_data = b'x'
+data, expected_fdtmap, _ = self._RunReplaceCmd('u-boot', new_data)
+expected = new_data.ljust(len(U_BOOT_DATA), b'\0')
+self.assertEqual(expected, data)
+
+path, fdtmap = state.GetFdtContents('fdtmap')
+self.assertIsNotNone(path)
+self.assertEqual(expected_fdtmap, fdtmap)
+
if __name__ == "__main__":
unittest.main()
--
2.35.1
[PATCH 2/7] binman: Collect bintools for images when replacing entries
Binman entries can use other executables to compute their data, usually
in their ObtainContents() methods. Subclasses of Entry_section would use
bintools in their BuildSectionData() method instead, which is called
from several places including their Pack().
These binary tools are resolved correctly while building an image from a
device-tree description so that they can be used from these methods.
However, this is not being done when replacing entries in an image,
which can result in an error as the Pack() methods attempt to use them.
Collect and resolve entries' bintools also when replacing entries to fix
Pack() errors. Add a way to mock bintool usage in the testing entry type
and tests that check bintools are being resolved for such an entry.
Signed-off-by: Alper Nebi Yasak
---
tools/binman/control.py | 1 +
tools/binman/etype/_testing.py| 36 +
tools/binman/ftest.py | 38 ++
.../binman/test/232_replace_with_bintool.dts | 39 +++
4 files changed, 114 insertions(+)
create mode 100644 tools/binman/test/232_replace_with_bintool.dts
diff --git a/tools/binman/control.py b/tools/binman/control.py
index d4c8dc89201b..e170aeae4fab 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -299,6 +299,7 @@ def BeforeReplace(image, allow_resize):
"""
state.PrepareFromLoadedData(image)
image.LoadData()
+image.CollectBintools()
# If repacking, drop the old offset/size values except for the original
# ones, so we are only left with the constraints.
diff --git a/tools/binman/etype/_testing.py b/tools/binman/etype/_testing.py
index 5089de364294..696004878147 100644
--- a/tools/binman/etype/_testing.py
+++ b/tools/binman/etype/_testing.py
@@ -39,6 +39,10 @@ class Entry__testing(Entry):
error if not)
force-bad-datatype: Force a call to GetEntryArgsOrProps() with a bad
data type (generating an error)
+require-bintool-for-contents: Raise an error if the specified
+bintool isn't usable in ObtainContents()
+require-bintool-for-pack: Raise an error if the specified
+bintool isn't usable in Pack()
"""
def __init__(self, section, etype, node):
super().__init__(section, etype, node)
@@ -82,6 +86,26 @@ def ReadNode(self):
self.return_contents = True
self.contents = b'aa'
+# Set to the required bintool when collecting bintools.
+self.bintool_for_contents = None
+self.require_bintool_for_contents = fdt_util.GetString(self._node,
+ 'require-bintool-for-contents')
+if self.require_bintool_for_contents == '':
+self.require_bintool_for_contents = '_testing'
+
+self.bintool_for_pack = None
+self.require_bintool_for_pack = fdt_util.GetString(self._node,
+ 'require-bintool-for-pack')
+if self.require_bintool_for_pack == '':
+self.require_bintool_for_pack = '_testing'
+
+def Pack(self, offset):
+"""Figure out how to pack the entry into the section"""
+if self.require_bintool_for_pack:
+if self.bintool_for_pack is None:
+self.Raise("Required bintool unusable in Pack()")
+return super().Pack(offset)
+
def ObtainContents(self, fake_size=0):
if self.return_unknown_contents or not self.return_contents:
return False
@@ -92,6 +116,9 @@ def ObtainContents(self, fake_size=0):
self.contents_size = len(self.data)
if self.return_contents_once:
self.return_contents = False
+if self.require_bintool_for_contents:
+if self.bintool_for_contents is None:
+self.Raise("Required bintool unusable in ObtainContents()")
return True
def GetOffsets(self):
@@ -127,3 +154,12 @@ def ProcessFdt(self, fdt):
if not self.never_complete_process_fdt:
self.process_fdt_ready = True
return ready
+
+def AddBintools(self, btools):
+"""Add the bintools used by this entry type"""
+if self.require_bintool_for_contents is not None:
+self.bintool_for_contents = self.AddBintool(btools,
+self.require_bintool_for_contents)
+if self.require_bintool_for_pack is not None:
+self.bintool_for_pack = self.AddBintool(btools,
+self.require_bintool_for_pack)
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index e6f0159a229f..da9733d39a6a 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -5502,6 +5502,44 @@ def testSafeUniqueNamesMulti(self):
self._CheckSafeUniqueNames(orig_image, image)
+def testReplaceCmdWithBintool(self):
+"""Test replacing an entry that needs a bintool to pack"""
+data = self._DoReadFileRealDtb
[PATCH 1/7] binman: Fix unique names having '/.' for images read from files
Binman can embed a copy of the image description into the images it
builds as a fdtmap entry, but it omits the /binman/ prefix
from the node paths while doing so. When reading an already-built image
file, entries are reconstructed using this fdtmap and their associated
nodes still lack that prefix.
Some entries like fit and vblock create intermediate files whose names
are based on an entry unique name. This name is constructed from their
node's path by concatenating the parents with dots up to the binman
node, e.g. /binman/image/foo/bar becomes 'image.foo.bar'.
However, we don't have this /binman/image prefix when replacing entries
in such an image. The /foo/bar entry we read when doing so erroneously
has the unique name of '/.foo.bar', causing permission errors when the
entry attempts to create files based on that.
Fix the unique-name generation by stopping at the '/' node like how it
stops at the binman node. As the unique names are used as filenames, add
tests that check if they're safe to use as filenames.
Signed-off-by: Alper Nebi Yasak
---
tools/binman/entry.py| 2 +-
tools/binman/ftest.py| 31
tools/binman/test/230_unique_names.dts | 34 ++
tools/binman/test/231_unique_names_multi.dts | 38
4 files changed, 104 insertions(+), 1 deletion(-)
create mode 100644 tools/binman/test/230_unique_names.dts
create mode 100644 tools/binman/test/231_unique_names_multi.dts
diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 18a7a3510548..a07a5888643a 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -775,7 +775,7 @@ def GetUniqueName(self):
node = self._node
while node.parent:
node = node.parent
-if node.name == 'binman':
+if node.name in ('binman', '/'):
break
name = '%s.%s' % (node.name, name)
return name
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 876953f11324..e6f0159a229f 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -5471,6 +5471,37 @@ def testFitSplitElfFaked(self):
err,
"Image '.*' is missing external blobs and is non-functional: .*")
+def _CheckSafeUniqueNames(self, *images):
+"""Check all entries of given images for unsafe unique names"""
+for image in images:
+entries = {}
+image._CollectEntries(entries, {}, image)
+for entry in entries.values():
+uniq = entry.GetUniqueName()
+
+# Used as part of a filename, so must not be absolute paths.
+self.assertFalse(os.path.isabs(uniq))
+
+def testSafeUniqueNames(self):
+"""Test entry unique names are safe in single image configuration"""
+data = self._DoReadFileRealDtb('230_unique_names.dts')
+
+orig_image = control.images['image']
+image_fname = tools.get_output_filename('image.bin')
+image = Image.FromFile(image_fname)
+
+self._CheckSafeUniqueNames(orig_image, image)
+
+def testSafeUniqueNamesMulti(self):
+"""Test entry unique names are safe with multiple images"""
+data = self._DoReadFileRealDtb('231_unique_names_multi.dts')
+
+orig_image = control.images['image']
+image_fname = tools.get_output_filename('image.bin')
+image = Image.FromFile(image_fname)
+
+self._CheckSafeUniqueNames(orig_image, image)
+
if __name__ == "__main__":
unittest.main()
diff --git a/tools/binman/test/230_unique_names.dts
b/tools/binman/test/230_unique_names.dts
new file mode 100644
index ..6780d37f71f4
--- /dev/null
+++ b/tools/binman/test/230_unique_names.dts
@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ binman {
+ size = <0xc00>;
+ allow-repack;
+
+ u-boot {
+ };
+
+ fdtmap {
+ };
+
+ u-boot2 {
+ type = "u-boot";
+ };
+
+ text {
+ text = "some text";
+ };
+
+ u-boot-dtb {
+ };
+
+ image-header {
+ location = "end";
+ };
+ };
+};
diff --git a/tools/binman/test/231_unique_names_multi.dts
b/tools/binman/test/231_unique_names_multi.dts
new file mode 100644
index ..db63afb445e0
--- /dev/null
+++ b/tools/binman/test/231_unique_names_multi.dts
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ binman {
+ multiple-images;
+
+ image {
+ size = <0xc00>;
+ allow-repack;
+
+ u-boot {
+ };
+
[PATCH 0/7] binman: Fix replacing FIT subentries
Converting the binman FIT entry type to a section ended up breaking the ability to replace these in images. The conversion to section partially enables extracting and replacing the subentries, but that also doesn't work as intended. This series gets binman to a point where it can extract FIT subentries and replace their non-section leaf entries correctly. Replacing sections of any kind doesn't really work right now, so the final patch disables that by raising an error. Initially, I had managed to replace sections by propagating changes in ProcessContentsUpdate() to the child entries, but writing arbitrary data into entries representing data in a certain format quickly gets into weird (maybe even undecidable?) edge cases. Just recently I had a better idea. Instead of writing incompatible data into a section, we can replace the section entry/node with for example a blob-ext entry/node for the input file. I've got this working as a proof of concept, but I need to experiment more to see what works best. Alper Nebi Yasak (7): binman: Fix unique names having '/.' for images read from files binman: Collect bintools for images when replacing entries binman: Don't reset offset/size if image doesn't allow repacking binman: Remove '/images/' fragment from FIT subentry paths binman: Create FIT subentries in the FIT section, not its parent binman: Test replacing non-section entries in FIT subsections binman: Refuse to replace sections for now tools/binman/control.py | 3 +- tools/binman/entry.py | 2 +- tools/binman/etype/_testing.py| 36 tools/binman/etype/fit.py | 15 +- tools/binman/etype/section.py | 3 + tools/binman/ftest.py | 179 ++ tools/binman/test/230_unique_names.dts| 34 tools/binman/test/231_unique_names_multi.dts | 38 .../binman/test/232_replace_with_bintool.dts | 39 tools/binman/test/233_fit_extract_replace.dts | 74 .../test/234_replace_section_simple.dts | 23 +++ 11 files changed, 438 insertions(+), 8 deletions(-) create mode 100644 tools/binman/test/230_unique_names.dts create mode 100644 tools/binman/test/231_unique_names_multi.dts create mode 100644 tools/binman/test/232_replace_with_bintool.dts create mode 100644 tools/binman/test/233_fit_extract_replace.dts create mode 100644 tools/binman/test/234_replace_section_simple.dts -- 2.35.1
[PATCH] doc: usage: Update the extension command title
Update the extension command title for consistency with other commands. Signed-off-by: Bin Meng --- doc/usage/extension.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/usage/extension.rst b/doc/usage/extension.rst index 2b88398b18..6366cf56e7 100644 --- a/doc/usage/extension.rst +++ b/doc/usage/extension.rst @@ -1,8 +1,8 @@ .. SPDX-License-Identifier: GPL-2.0+ .. Copyright 2021, Kory Maincent -U-Boot extension board usage (CONFIG_EXTENSION) -=== +extension command += Synopsis -- 2.25.1
Re: [PATCH v4 00/33] Initial implementation of standard boot
HI Simon On Sat, Mar 26, 2022 at 9:51 PM Simon Glass wrote: > > Hi Tom, > > On Sat, 26 Mar 2022 at 13:58, Tom Rini wrote: > > > > On Sat, Mar 26, 2022 at 01:56:36PM -0600, Simon Glass wrote: > > > Hi Tom, > > > > > > On Fri, 25 Mar 2022 at 08:50, Tom Rini wrote: > > > > > > > > On Fri, Mar 25, 2022 at 03:36:24PM +0100, Michael Nazzareno Trimarchi > > > > wrote: > > > > > Hi Tom > > > > > > > > > > > > > > > On Wed, Mar 23, 2022 at 9:07 PM Tom Rini wrote: > > > > > > > > > > > > On Wed, Mar 23, 2022 at 08:57:36PM +0100, Michael Nazzareno > > > > > > Trimarchi wrote: > > > > > > > Hi Tom > > > > > > > > > > > > > > On Wed, Mar 23, 2022 at 8:30 PM Tom Rini > > > > > > > wrote: > > > > > > > > > > > > > > > > On Wed, Mar 23, 2022 at 08:21:22PM +0100, Michael Nazzareno > > > > > > > > Trimarchi wrote: > > > > > > > > > Hi Tom > > > > > > > > > > > > > > > > > > On Wed, Mar 23, 2022 at 7:46 PM Simon Glass > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > Hi Tom, > > > > > > > > > > > > > > > > > > > > On Wed, 23 Mar 2022 at 08:05, Tom Rini > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > On Sun, Mar 06, 2022 at 05:49:43AM -0700, Simon Glass > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > The bootflow feature provide a built-in way for U-Boot > > > > > > > > > > > > to automatically > > > > > > > > > > > > boot an Operating System without custom scripting and > > > > > > > > > > > > other customisation. > > > > > > > > > > > > This is called 'standard boot' since it provides a > > > > > > > > > > > > standard way for > > > > > > > > > > > > U-Boot to boot a distro, without scripting. > > > > > > > > > > > > > > > > > > > > > > > > It introduces the following concepts: > > > > > > > > > > > > > > > > > > > > > > > >- bootdev - a device which can hold a distro > > > > > > > > > > > >- bootmeth - a method to scan a bootdev to find > > > > > > > > > > > > bootflows (owned by > > > > > > > > > > > > U-Boot) > > > > > > > > > > > >- bootflow - a description of how to boot (owned by > > > > > > > > > > > > the distro) > > > > > > > > > > > > > > > > > > > > > > > > This series provides an implementation of these, > > > > > > > > > > > > enabled to scan for > > > > > > > > > > > > bootflows from MMC, USB and Ethernet. It supports the > > > > > > > > > > > > existing distro > > > > > > > > > > > > boot as well as the EFI loader flow (bootefi/bootmgr). > > > > > > > > > > > > It works > > > > > > > > > > > > similiarly to the existing script-based approach, but > > > > > > > > > > > > is native to > > > > > > > > > > > > U-Boot. > > > > > > > > > > > > > > > > > > > > > > > > With this we can boot on a Raspberry Pi 3 with just one > > > > > > > > > > > > command: > > > > > > > > > > > > > > > > > > > > > > > >bootflow scan -lb > > > > > > > > > > > > > > > > > > > > > > > > which means to scan, listing (-l) each bootflow and > > > > > > > > > > > > trying to boot each > > > > > > > > > > > > one (-b). The final patch shows this. > > > > > > > > > > > > > > > > > > > > > > > > With a standard way to identify boot devices, booting > > > > > > > > > > > > become easier. It > > > > > > > > > > > > also should be possible to support U-Boot scripts, for > > > > > > > > > > > > backwards > > > > > > > > > > > > compatibility only. > > > > > > > > > > > > > > > > > > > > > > > > This series relies on the PXE clean-up series, posted > > > > > > > > > > > > here: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://patchwork.ozlabs.org/project/uboot/list/?series=267078 > > > > > > > > > > > > > > > > > > > > > > > > For documentation, see the 'doc' patch. > > > > > > > > > > > > > > > > > > > > > > > > For version 2, a new naming scheme is used as above: > > > > > > > > > > > > > > > > > > > > > > > >- bootdev is used instead of bootdevice, because > > > > > > > > > > > > 'device' is overused, > > > > > > > > > > > >is everywhere in U-Boot, can be confused with > > > > > > > > > > > > udevice > > > > > > > > > > > >- bootmeth - because 'method' is too vanilla, > > > > > > > > > > > > appears 1300 times in > > > > > > > > > > > >U-Boot > > > > > > > > > > > > > > > > > > > > > > > > Also in version 2, drivers are introduced for the boot > > > > > > > > > > > > methods, to make > > > > > > > > > > > > it more extensible. Booting a custom OS is simply a > > > > > > > > > > > > matter of creating a > > > > > > > > > > > > bootmeth for it and implementing the read_file() and > > > > > > > > > > > > boot() methods. > > > > > > > > > > > > > > > > > > > > > > > > Version 4 makes some minor improvements and leaves out > > > > > > > > > > > > the RFC patch for > > > > > > > > > > > > rpi conversion, in the hope of getting the base support > > > > > > > > > > > > applied sooner > > > > > > > > > > > > rather than later. > > > > > > > > > > > > > > > > > >
Re: [RFC PATCH 1/6] capsule: Add Image GUIDs for platforms using capsule updates
hi Masami, On Sat, 26 Mar 2022 at 16:17, Masami Hiramatsu wrote: > > Hi Sughosh, > > 2022年3月25日(金) 18:59 Sughosh Ganu : > > > > hi Masami, > > > > On Fri, 25 Mar 2022 at 10:58, Masami Hiramatsu > > wrote: > > > > > > Hi Sughosh, > > > > > > OK I understand that if the platform uses the FIT capsule, the > > > fw_images[] must have 1 entry and it is completely non relationship > > > with dfu_alt_info... We may need a document for this case too. > > > > Actually, what you are stating above applies to both raw images as > > well as FIT images. I have added a paragraph in the capsule update > > related section in the uefi.rst. Can you check my patch 4 of this > > series. Thanks. > > I've checked that you didn't change the FMP::set_image(), but updated > FMP::get_image_info() to use the per-platform GUID list. > Thus, the efi_fmp_find() ensures that the image type GUID in the > capsule image is *included* in the platform GUID list (fw_images[] > array). > > OK, at this point, it filters out the firmware image which is not > supported on the platform. > > However, since you didn't update the FMP::set_image() and > efi_capsule_update_firmware(), it directly uses the *index* number in > the capsule image for updating the firmware. Is that correct? Yes, your observation is correct. The aim of this series is to fix the issue of using a common GUID value across images and platforms for a given FMP instance. > > If so, if the platform supports several image types, the problem happens. > Suppose that if the platform has TF-A and U-Boot, the DFU entity index > are 1 and 2. > And user missed to make a capsule file with index 1 for U-Boot image > with U-Boot image type GUID of that platform. > This capsule file passed the check in the efi_fmp_find(), because the > GUID is included in the platform supported GUID list. However, > FMP::set_image() will overwrite the TF-A with given U-Boot image > without any error. > > I think we need one more patch to check the given image-index in the > capsule image is correctly matched to the image-type GUID for safety. Yes, this is very much in the pipeline. I will be working on extending the struct efi_fw_images to have information on the image index as part of the structure. But please note that what you mention above is just making the process of capsule update more robust -- the current implementation that we have in the capsule update module where the image_index value is taken from the capsule is very much in compliance with the UEFI specification. The current patch series is fixing an issue which was not compliant with the spec. But what you are suggesting above is on my Todo list, just that it is a separate task, and not a fix as such. -sughosh > > Thank you, > > > -sughosh > > > > > > > > Thanks, > > > > > > 2022年3月25日(金) 10:09 Masami Hiramatsu : > > > > > > > > Hi Sughosh, > > > > > > > > 2022年3月24日(木) 23:40 Sughosh Ganu : > > > > > > > > > > hi Masami, > > > > > > > > > > On Thu, 24 Mar 2022 at 19:14, Masami Hiramatsu > > > > > wrote: > > > > > > > > > > > > Hi Sughosh, > > > > > > > > > > > > 2022年3月24日(木) 21:39 Sughosh Ganu : > > > > > > > > > > > > > > Currently, all platforms that enable capsule updates do so using > > > > > > > either EFI_FIRMWARE_IMAGE_TYPE_UBOOT_RAW_GUID or > > > > > > > EFI_FIRMWARE_IMAGE_TYPE_UBOOT_FIT_GUID. This is based on the > > > > > > > Firmware > > > > > > > Management Protocol(FMP) instance used on the platform. However, > > > > > > > this > > > > > > > means that all platforms that enable a particular FMP instance > > > > > > > have > > > > > > > the same GUID value for all the updatable images, either the FIT > > > > > > > image > > > > > > > GUID or the raw image GUID, and that an image for some platform > > > > > > > can be > > > > > > > updated on any other platform which uses the same FMP instance. > > > > > > > Another > > > > > > > issue with this implementation is that the ESRT table shows the > > > > > > > same > > > > > > > GUID value for all images on the platform and also across > > > > > > > platforms, > > > > > > > which is not in compliance with the UEFI specification. > > > > > > > > > > > > > > Fix this by defining image GUID values and firmware names for > > > > > > > individual images per platform. The GetImageInfo FMP hook would > > > > > > > then > > > > > > > populate these values in the image descriptor array. > > > > > > > > > > > > OK, so you have generated GUIDs for each "dfu_alt_info" entry on the > > > > > > platforms, correct? > > > > > > > > > > No, I have generated the fw_images array based on the information that > > > > > I found in the dfu_alt_info variable for the platform. But this is not > > > > > correlated to the dfu_alt_info variable. If you think that the array > > > > > should have more/different entries for your platform, please let me > > > > > know, and I will change it. > > > > > > > > At least for the DeveloperBox, it looks good to me. > > > > (Hopefully, if you comment the stri
[PATCH 1/1] test: fix pylint warnings in test_efi_bootmgr
* Use f'' strings instead of .format().
* Correct sequence of imports.
* Remove a superfluous import.
* Add missing documentation.
* Replace yield by return.
Signed-off-by: Heinrich Schuchardt
---
test/py/tests/test_efi_bootmgr/conftest.py | 14 ++
test/py/tests/test_efi_bootmgr/test_efi_bootmgr.py | 13 -
2 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/test/py/tests/test_efi_bootmgr/conftest.py
b/test/py/tests/test_efi_bootmgr/conftest.py
index 69008fddce..a0a754afbe 100644
--- a/test/py/tests/test_efi_bootmgr/conftest.py
+++ b/test/py/tests/test_efi_bootmgr/conftest.py
@@ -4,9 +4,9 @@
"""
import os
-import pytest
import shutil
-from subprocess import call, check_call
+from subprocess import check_call
+import pytest
@pytest.fixture(scope='session')
def efi_bootmgr_data(u_boot_config):
@@ -14,7 +14,7 @@ def efi_bootmgr_data(u_boot_config):
tests
Args:
-u_boot_config: U-boot configuration.
+u_boot_config -- U-boot configuration.
Return:
A path to disk image to be used for testing
@@ -34,9 +34,7 @@ def efi_bootmgr_data(u_boot_config):
shutil.copyfile(u_boot_config.build_dir + '/lib/efi_loader/initrddump.efi',
mnt_point + '/initrddump.efi')
-check_call('virt-make-fs --partition=gpt --size=+1M --type=vfat {} {}'
- .format(mnt_point, image_path), shell=True)
-
-print(image_path)
+check_call(f'virt-make-fs --partition=gpt --size=+1M --type=vfat
{mnt_point} {image_path}',
+ shell=True)
-yield image_path
+return image_path
diff --git a/test/py/tests/test_efi_bootmgr/test_efi_bootmgr.py
b/test/py/tests/test_efi_bootmgr/test_efi_bootmgr.py
index f87e0a20b8..75a6e7c962 100644
--- a/test/py/tests/test_efi_bootmgr/test_efi_bootmgr.py
+++ b/test/py/tests/test_efi_bootmgr/test_efi_bootmgr.py
@@ -1,4 +1,6 @@
# SPDX-License-Identifier: GPL-2.0+
+""" Unit test for UEFI bootmanager
+"""
import pytest
@@ -6,7 +8,16 @@ import pytest
@pytest.mark.buildconfigspec('cmd_efidebug')
@pytest.mark.buildconfigspec('cmd_bootefi_bootmgr')
def test_efi_bootmgr(u_boot_console, efi_bootmgr_data):
-u_boot_console.run_command(cmd = 'host bind 0 {}'.format(efi_bootmgr_data))
+""" Unit test for UEFI bootmanager
+The efidebug command is used to set up UEFI load options.
+The bootefi bootmgr loads initrddump.efi as a payload.
+The crc32 of the loaded initrd.img is checked
+
+Args:
+u_boot_console -- U-Boot console
+efi_bootmgr_data -- Path to the disk image used for testing.
+"""
+u_boot_console.run_command(cmd = f'host bind 0 {efi_bootmgr_data}')
u_boot_console.run_command(cmd = 'efidebug boot add ' \
'-b 0001 label-1 host 0:1 initrddump.efi ' \
--
2.34.1
[PATCH 1/1] test: fix pylint warnings for test_efi_fit.py
* fix style of argument documentation
* add encoding to open() calls
Signed-off-by: Heinrich Schuchardt
---
test/py/tests/test_efi_fit.py | 31 ---
1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/test/py/tests/test_efi_fit.py b/test/py/tests/test_efi_fit.py
index 068a35a559..92d071f783 100644
--- a/test/py/tests/test_efi_fit.py
+++ b/test/py/tests/test_efi_fit.py
@@ -203,7 +203,7 @@ def test_efi_fit_launch(u_boot_console):
"""Compute the path of a given (temporary) file.
Args:
-file_name: The name of a file within U-Boot build dir.
+file_name -- The name of a file within U-Boot build dir.
Return:
The computed file path.
"""
@@ -217,8 +217,8 @@ def test_efi_fit_launch(u_boot_console):
build dir and, optionally, compresses the file using gzip.
Args:
-fname: The target file name within U-Boot build dir.
-comp: Flag to enable gzip compression.
+fname -- The target file name within U-Boot build dir.
+comp -- Flag to enable gzip compression.
Return:
The path of the created file.
"""
@@ -238,8 +238,8 @@ def test_efi_fit_launch(u_boot_console):
Creates a DTS file and compiles it to a DTB.
Args:
-fdt_type: The type of the FDT, i.e. internal, user.
-comp: Flag to enable gzip compression.
+fdt_type -- The type of the FDT, i.e. internal, user.
+comp -- Flag to enable gzip compression.
Return:
The path of the created file.
"""
@@ -252,7 +252,7 @@ def test_efi_fit_launch(u_boot_console):
# Generate a test FDT file.
dts = make_fpath('test-efi-fit-%s.dts' % fdt_type)
-with open(dts, 'w') as file:
+with open(dts, 'w', encoding='ascii') as file:
file.write(FDT_DATA % fdt_params)
# Build the test FDT.
@@ -268,7 +268,7 @@ def test_efi_fit_launch(u_boot_console):
Runs 'mkimage' to create a FIT image within U-Boot build dir.
Args:
-comp: Enable gzip compression for the EFI binary and FDT blob.
+comp -- Enable gzip compression for the EFI binary and FDT blob.
Return:
The path of the created file.
"""
@@ -285,7 +285,7 @@ def test_efi_fit_launch(u_boot_console):
# Generate a test ITS file.
its_path = make_fpath('test-efi-fit-helloworld.its')
-with open(its_path, 'w') as file:
+with open(its_path, 'w', encoding='ascii') as file:
file.write(ITS_DATA % its_params)
# Build the test ITS.
@@ -298,8 +298,9 @@ def test_efi_fit_launch(u_boot_console):
"""Load the FIT image using the 'host load' command and return its
address.
Args:
-fit: Dictionary describing the FIT image to load, see
env__efi_fit_test_file
- in the comment at the beginning of this file.
+fit -- Dictionary describing the FIT image to load, see
+ env__efi_fit_test_file in the comment at the beginning of
+ this file.
Return:
The address where the file has been loaded.
"""
@@ -325,8 +326,8 @@ def test_efi_fit_launch(u_boot_console):
CRC32 are validated.
Args:
-fit: Dictionary describing the FIT image to load, see
env__efi_fit_tftp_file
- in the comment at the beginning of this file.
+fit -- Dictionary describing the FIT image to load, see
env__efi_fit_tftp_file
+ in the comment at the beginning of this file.
Return:
The address where the file has been loaded.
"""
@@ -377,9 +378,9 @@ def test_efi_fit_launch(u_boot_console):
Eventually the 'Hello, world' message is expected in the U-Boot
console.
Args:
-enable_fdt: Flag to enable using the FDT blob inside FIT image.
-enable_comp: Flag to enable GZIP compression on EFI and FDT
-generated content.
+enable_fdt -- Flag to enable using the FDT blob inside FIT image.
+enable_comp -- Flag to enable GZIP compression on EFI and FDT
+ generated content.
"""
with cons.log.section('FDT=%s;COMP=%s' % (enable_fdt, enable_comp)):
--
2.34.1
Pull request for efi-2022-04-rc5-2
The following changes since commit 28c2ebef372b4c9bb18bed8373e0d9e65a09b42b: Merge https://source.denx.de/u-boot/custodians/u-boot-sunxi (2022-03-25 21:20:29 -0400) are available in the Git repository at: https://source.denx.de/u-boot/custodians/u-boot-efi.git tags/efi-2022-04-rc5-2 for you to fetch changes up to ccc41fcfaf33356f7789c8bd50551a9e37507f2d: cmd: efidebug: simplify do_efi_boot_add() (2022-03-26 07:46:21 +0100) Gitlab CI showed no problem: https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11459 Pull request for efi-2022-04-rc5-2 Documentation: * man-page for the wdt command UEFI: * Unit test for boot manager Heinrich Schuchardt (6): doc: man-page for the wdt command efi_loader: typo devie-path efi_loader: nocolor command line attr for initrddump.efi efi_loader: initrddump: drain input before prompt test: test UEFI boot manager cmd: efidebug: simplify do_efi_boot_add() cmd/efidebug.c | 5 +- doc/usage/index.rst| 1 + doc/usage/wdt.rst | 77 ++ lib/efi_loader/efi_device_path.c | 2 +- lib/efi_loader/initrddump.c| 91 ++ test/py/tests/test_efi_bootmgr/conftest.py | 42 ++ test/py/tests/test_efi_bootmgr/test_efi_bootmgr.py | 32 7 files changed, 233 insertions(+), 17 deletions(-) create mode 100644 doc/usage/wdt.rst create mode 100644 test/py/tests/test_efi_bootmgr/conftest.py create mode 100644 test/py/tests/test_efi_bootmgr/test_efi_bootmgr.py
