Re: [PATCH RFC u-boot-mvebu 0/6] arm: mvebu: Fix boot mode detection
On Monday 06 March 2023 20:15:07 Tony Dinh wrote: > Hi Pali, > > On Mon, Mar 6, 2023 at 4:11 PM Pali Rohár wrote: > > > > On Monday 06 March 2023 16:01:58 Tony Dinh wrote: > > > Hi Pali, > > > > > > On Sun, Mar 5, 2023 at 4:41 PM Tony Dinh wrote: > > > > > > > > Hi Pali, > > > > > > > > On Sun, Mar 5, 2023 at 2:54 PM Pali Rohár wrote: > > > > > > > > > > On Sunday 05 March 2023 14:46:55 Tony Dinh wrote: > > > > > > On Sun, Mar 5, 2023 at 2:44 PM Tony Dinh wrote: > > > > > > > > > > > > > > Hi Pali, > > > > > > > > > > > > > > On Sun, Mar 5, 2023 at 3:55 AM Pali Rohár wrote: > > > > > > > > > > > > > > > > On Sunday 05 March 2023 04:21:42 Martin Rowe wrote: > > > > > > > > > On Sat, 4 Mar 2023 at 10:51, Pali Rohár > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > Improve code for checking strapping pins which specifies > > > > > > > > > > boot mode source. > > > > > > > > > > > > > > > > > > > > Martin, could you test if Clearfog can be still configured > > > > > > > > > > into UART > > > > > > > > > > booting mode via HW switches and if it still works > > > > > > > > > > correctly? First > > > > > > > > > > patch is reverting UART related commit for Clearfog which I > > > > > > > > > > think it not > > > > > > > > > > needed anymore. > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Clearfog the logic in the CONFIG_ARMADA_38X ifdef before > > > > > > > > > the switch that > > > > > > > > > you refactored in cpu.c/get_boot_device is all that gets > > > > > > > > > processed. It > > > > > > > > > decides there is an error and returns BOOT_DEVICE_UART, > > > > > > > > > probably because of > > > > > > > > > the invalid boot workaround for broken UART selection that > > > > > > > > > you identified. > > > > > > > > > > > > > > > > Ok, so I figured out correctly how this invalid mode works. > > > > > > > > > > > > > > > > > UART only works if I use the clearfog_spi_defconfig or if I > > > > > > > > > select > > > > > > > > > CONFIG_MVEBU_SPL_BOOT_DEVICE_UART=y. It does not work with > > > > > > > > > the MMC or SATA > > > > > > > > > defconfigs. I get the same result without this patch series > > > > > > > > > applied, though. > > > > > > > > > > > > > > > > > > The failed cases have the same output (other than kwboot > > > > > > > > > header patching > > > > > > > > > output) until after sending boot image data is complete. The > > > > > > > > > output stops > > > > > > > > > after: > > > > > > > > > > > > > > > > > > 98 % > > > > > > > > > [. > > > > > > > > > ] > > > > > > > > > Done > > > > > > > > > Finishing transfer > > > > > > > > > [Type Ctrl-\ + c to quit] > > > > > > > > > > > > > > > > > > > > > > > > > This is very strange because CONFIG_MVEBU_SPL_BOOT_DEVICE_UART > > > > > > > > just > > > > > > > > instruct mkimage what to put into kwbimage header. > > > > > > > > > > > > > > > > If I'm looking at the output correctly then SPL was booted, it > > > > > > > > correctly > > > > > > > > trained DDR RAM, returned back to bootrom, kwboot continued > > > > > > > > sending main > > > > > > > > u-boot and bootrom confirmed that transfer of both SPL and main > > > > > > > > u-boot > > > > > > > > is complete. But then there is no output from main u-boot. > > > > > > > > > > > > > > > > > It looks like an unrelated issue with kwboot.c, which I was > > > > > > > > > sure was > > > > > > > > > working after the last patches but I can no longer reproduce > > > > > > > > > a successful > > > > > > > > > boot. > > > > > > > > > > > > > > > > Can you check that you are using _both_ mkimage and kwboot from > > > > > > > > version > > > > > > > > with applying _all_ my patches recently sent to ML? Because > > > > > > > > both mkimage > > > > > > > > and kwboot have fixes for SATA and SDIO images. > > > > > > > > > > > > > > > > For me it looks like that either mkimage generated incorrect > > > > > > > > image size > > > > > > > > for SATA or SDIO image. Or kwboot incorrectly parsed that image > > > > > > > > size > > > > > > > > from kwbimage header and sent smaller image. > > > > > > > > > > > > > > > > > > > > > > > > > > > Also could you check if SATA booting is still working > > > > > > > > > > correctly? > > > > > > > > > > > > > > > > > > > > > > > > > > > > SATA works correctly. > > > > > > > > > > > > > > > > Perfect! > > > > > > > > > > > > > > > > > > > > > > > > > > > Tony, should address problems with SPI booting when it is > > > > > > > > > > configured to > > > > > > > > > > different configuration. In fourth commit I added all > > > > > > > > > > possible boot mode > > > > > > > > > > strapping pin configurations which are recognized by A385 > > > > > > > > > > bootrom (and > > > > > > > > > > not the only one described in the HW spec, which is > > > > > > > > > > incomplete). > > > > > > > > > > > > > > It
[PATCH 2/2] ARM: imx: Enable SDP download in SPL on DH i.MX6 DHSOM
Enable SDP protocol support in SPL for DH i.MX6 DHSOM, now that those components fit into the SPL due to LTO. To start U-Boot via SDP upload on i.MX6 DHSOM based board, proceed as follows: - Compile imx_usb [1] . - Power off the i.MX6 DHSOM based board. - Connect both USB-serial console and USB-OTG miniB ports to host PC. - Switch board to USB boot mode. - Power on the board. - Verify using '$ dmesg' that a new device has been detected as follows: New USB device found, idVendor=15a2, idProduct=0054, bcdDevice= 0.01 New USB device strings: Mfr=1, Product=2, SerialNumber=0 Product: SE Blank ARIK Manufacturer: Freescale SemiConductor Inc - Upload U-Boot SPL: $ imx_usb u-boot-with-spl.imx - Wait for SPL to come up, the following print ought to be the last on UART console: SDP: handle requests... - Upload U-Boot proper: $ imx_usb u-boot.img [1] https://github.com/boundarydevices/imx_usb_loader.git Signed-off-by: Marek Vasut --- Cc: Andreas Geisreiter Cc: Christoph Niedermaier Cc: Fabio Estevam Cc: NXP i.MX U-Boot Team Cc: Stefano Babic Cc: u-b...@dh-electronics.com --- configs/dh_imx6_defconfig | 5 + include/configs/dh_imx6.h | 2 -- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/configs/dh_imx6_defconfig b/configs/dh_imx6_defconfig index cf52b0e3a00..3de974f139d 100644 --- a/configs/dh_imx6_defconfig +++ b/configs/dh_imx6_defconfig @@ -40,6 +40,10 @@ CONFIG_SYS_CONSOLE_OVERWRITE_ROUTINE=y CONFIG_SYS_SPL_MALLOC=y CONFIG_SPL_SPI_LOAD=y CONFIG_SYS_SPI_U_BOOT_OFFS=0x11400 +CONFIG_SPL_USB_HOST=y +CONFIG_SPL_USB_GADGET=y +CONFIG_SPL_USB_SDP_SUPPORT=y +CONFIG_SPL_WATCHDOG=y CONFIG_SYS_MAXARGS=32 CONFIG_SYS_PBSIZE=532 CONFIG_CMD_MEMTEST=y @@ -113,6 +117,7 @@ CONFIG_USB_GADGET_MANUFACTURER="dh" CONFIG_USB_GADGET_VENDOR_NUM=0x0525 CONFIG_USB_GADGET_PRODUCT_NUM=0xa4a5 CONFIG_CI_UDC=y +CONFIG_SDP_LOADADDR=0x17c0 CONFIG_USB_GADGET_DOWNLOAD=y CONFIG_WATCHDOG_TIMEOUT_MSECS=6 CONFIG_IMX_WATCHDOG=y diff --git a/include/configs/dh_imx6.h b/include/configs/dh_imx6.h index 5cf73274d5e..ef27e483295 100644 --- a/include/configs/dh_imx6.h +++ b/include/configs/dh_imx6.h @@ -31,7 +31,6 @@ #define CFG_MXC_UART_BASE UART1_BASE /* USB Configs */ -#ifdef CONFIG_CMD_USB #define CFG_MXC_USB_PORTSC (PORT_PTS_UTMI | PORT_PTS_PTW) #define CFG_MXC_USB_FLAGS 0 @@ -39,7 +38,6 @@ #if defined(CONFIG_CMD_DFU) || defined(CONFIG_CMD_USB_MASS_STORAGE) #define DFU_DEFAULT_POLL_TIMEOUT 300 #endif -#endif #define CFG_EXTRA_ENV_SETTINGS \ "console=ttymxc0,115200\0" \ -- 2.39.2
[PATCH 1/2] ARM: imx: Enable LTO for DH electronics i.MX6 DHCOM
Enable LTO to reduce the size of SPL, which with SPL SDP support may be close to the limit. Signed-off-by: Marek Vasut --- Cc: Andreas Geisreiter Cc: Christoph Niedermaier Cc: Fabio Estevam Cc: NXP i.MX U-Boot Team Cc: Stefano Babic Cc: u-b...@dh-electronics.com --- configs/dh_imx6_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/configs/dh_imx6_defconfig b/configs/dh_imx6_defconfig index 62c446f86a9..cf52b0e3a00 100644 --- a/configs/dh_imx6_defconfig +++ b/configs/dh_imx6_defconfig @@ -28,6 +28,7 @@ CONFIG_SPL_SPI=y CONFIG_AHCI=y CONFIG_SYS_MEMTEST_START=0x1000 CONFIG_SYS_MEMTEST_END=0x2000 +CONFIG_LTO=y CONFIG_DISTRO_DEFAULTS=y CONFIG_SYS_MONITOR_LEN=409600 CONFIG_FIT=y -- 2.39.2
[PATCH] usb: gadget: f_sdp: Add missing spl_board_prepare_for_boot() call
The spl_board_prepare_for_boot() should be called before jump_to_image_no_args() to perform board-specific deinitialization before jumping to the next stage. This board-specific deinitialization can be very much anything, e.g. disable dcache in case it was enabled, or such. Add the missing spl_board_prepare_for_boot() call into f_sdp . Signed-off-by: Marek Vasut --- Cc: Frieder Schrempf Cc: Lukasz Majewski Cc: Patrick Delaunay Cc: Peng Fan Cc: Sean Anderson Cc: Sherry Sun Cc: Simon Glass Cc: Ye Li --- drivers/usb/gadget/f_sdp.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/gadget/f_sdp.c b/drivers/usb/gadget/f_sdp.c index 9ea43f29cfb..4da5a160a09 100644 --- a/drivers/usb/gadget/f_sdp.c +++ b/drivers/usb/gadget/f_sdp.c @@ -865,6 +865,7 @@ static int sdp_handle_in_ep(struct spl_image_info *spl_image, struct spl_image_info spl_image = {}; struct spl_boot_device bootdev = {}; spl_parse_image_header(_image, , header); + spl_board_prepare_for_boot(); jump_to_image_no_args(_image); #else /* In U-Boot, allow jumps to scripts */ -- 2.39.2
Re: [PATCH v3 14/17] riscv: dts: jh7110: Add initial StarFive JH7110 device tree
On 7 March 2023 01:59:31 GMT, yanhong wang wrote: > > >On 2023/3/4 5:16, Conor Dooley wrote: >> On Fri, Mar 03, 2023 at 11:24:29AM +0800, Yanhong Wang wrote: >>> Add initial device tree for the JH7110 RISC-V SoC. >>> >>> Signed-off-by: Yanhong Wang >>> --- >>> arch/riscv/dts/jh7110.dtsi | 582 + >>> 1 file changed, 582 insertions(+) >>> create mode 100644 arch/riscv/dts/jh7110.dtsi >>> >>> diff --git a/arch/riscv/dts/jh7110.dtsi b/arch/riscv/dts/jh7110.dtsi >>> new file mode 100644 >>> index 00..d3e9f92987 >>> --- /dev/null >>> +++ b/arch/riscv/dts/jh7110.dtsi >>> @@ -0,0 +1,582 @@ >>> +// SPDX-License-Identifier: GPL-2.0 OR MIT >>> +/* >>> + * Copyright (C) 2022 StarFive Technology Co., Ltd. >>> + */ >>> + >>> +/dts-v1/; >>> +#include >>> +#include >>> + >>> +/ { >>> + compatible = "starfive,jh7110"; >>> + #address-cells = <2>; >>> + #size-cells = <2>; >>> + >>> + cpus { >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + S7_0: cpu@0 { >>> + compatible = "sifive,s7", "riscv"; >>> + reg = <0>; >>> + d-cache-block-size = <64>; >>> + d-cache-sets = <64>; >>> + d-cache-size = <8192>; >>> + d-tlb-sets = <1>; >>> + d-tlb-size = <40>; >>> + device_type = "cpu"; >>> + i-cache-block-size = <64>; >>> + i-cache-sets = <64>; >>> + i-cache-size = <16384>; >>> + i-tlb-sets = <1>; >>> + i-tlb-size = <40>; >>> + mmu-type = "riscv,sv39"; >>> + next-level-cache = <>; >>> + riscv,isa = "rv64imac_zba_zbb"; >> >> Hmm, based on what Sean said on the previous version, "We use strchr on >> it; so something like Zicsr is parsed as 5 extensions", are you sure that >> adding this here behaves correctly? >> > >As you said, u-boot does not parse the content after '_', zba/zbb has no >practical meaning in u-boot. That's not what Sean's comment on the previous version said. If it is actually ignored, this is fine, but Sean's comment read like it would be misinterpreted by U-Boot. I'll have to go read the code. >The definitions of 'riscv,isa' refer to linux and is consistent with this. > >https://patchwork.kernel.org/project/linux-riscv/patch/20230221024645.127922-18-hal.f...@starfivetech.com/ > > >Do you have any better suggestion? Referring to the definition of Sifive >fu740, remove '_zba_zbb'? The fu740 doesn't have those extensions. > >> https://lore.kernel.org/u-boot/22e805d4-f823-975c-a970-a4a19bb13...@gmail.com/ >> >> I know that having zba/zbb is *correct*, but if U-Boot doesn't parse it >> correctly... >> >> Cheers, >> Conor.
Re: [PATCH RFC u-boot-mvebu 0/6] arm: mvebu: Fix boot mode detection
Hi Pali, On Mon, Mar 6, 2023 at 4:11 PM Pali Rohár wrote: > > On Monday 06 March 2023 16:01:58 Tony Dinh wrote: > > Hi Pali, > > > > On Sun, Mar 5, 2023 at 4:41 PM Tony Dinh wrote: > > > > > > Hi Pali, > > > > > > On Sun, Mar 5, 2023 at 2:54 PM Pali Rohár wrote: > > > > > > > > On Sunday 05 March 2023 14:46:55 Tony Dinh wrote: > > > > > On Sun, Mar 5, 2023 at 2:44 PM Tony Dinh wrote: > > > > > > > > > > > > Hi Pali, > > > > > > > > > > > > On Sun, Mar 5, 2023 at 3:55 AM Pali Rohár wrote: > > > > > > > > > > > > > > On Sunday 05 March 2023 04:21:42 Martin Rowe wrote: > > > > > > > > On Sat, 4 Mar 2023 at 10:51, Pali Rohár wrote: > > > > > > > > > > > > > > > > > Improve code for checking strapping pins which specifies boot > > > > > > > > > mode source. > > > > > > > > > > > > > > > > > > Martin, could you test if Clearfog can be still configured > > > > > > > > > into UART > > > > > > > > > booting mode via HW switches and if it still works correctly? > > > > > > > > > First > > > > > > > > > patch is reverting UART related commit for Clearfog which I > > > > > > > > > think it not > > > > > > > > > needed anymore. > > > > > > > > > > > > > > > > > > > > > > > > > On Clearfog the logic in the CONFIG_ARMADA_38X ifdef before the > > > > > > > > switch that > > > > > > > > you refactored in cpu.c/get_boot_device is all that gets > > > > > > > > processed. It > > > > > > > > decides there is an error and returns BOOT_DEVICE_UART, > > > > > > > > probably because of > > > > > > > > the invalid boot workaround for broken UART selection that you > > > > > > > > identified. > > > > > > > > > > > > > > Ok, so I figured out correctly how this invalid mode works. > > > > > > > > > > > > > > > UART only works if I use the clearfog_spi_defconfig or if I > > > > > > > > select > > > > > > > > CONFIG_MVEBU_SPL_BOOT_DEVICE_UART=y. It does not work with the > > > > > > > > MMC or SATA > > > > > > > > defconfigs. I get the same result without this patch series > > > > > > > > applied, though. > > > > > > > > > > > > > > > > The failed cases have the same output (other than kwboot header > > > > > > > > patching > > > > > > > > output) until after sending boot image data is complete. The > > > > > > > > output stops > > > > > > > > after: > > > > > > > > > > > > > > > > 98 % > > > > > > > > [. > > > > > > > > ] > > > > > > > > Done > > > > > > > > Finishing transfer > > > > > > > > [Type Ctrl-\ + c to quit] > > > > > > > > > > > > > > > > > > > > > > This is very strange because CONFIG_MVEBU_SPL_BOOT_DEVICE_UART > > > > > > > just > > > > > > > instruct mkimage what to put into kwbimage header. > > > > > > > > > > > > > > If I'm looking at the output correctly then SPL was booted, it > > > > > > > correctly > > > > > > > trained DDR RAM, returned back to bootrom, kwboot continued > > > > > > > sending main > > > > > > > u-boot and bootrom confirmed that transfer of both SPL and main > > > > > > > u-boot > > > > > > > is complete. But then there is no output from main u-boot. > > > > > > > > > > > > > > > It looks like an unrelated issue with kwboot.c, which I was > > > > > > > > sure was > > > > > > > > working after the last patches but I can no longer reproduce a > > > > > > > > successful > > > > > > > > boot. > > > > > > > > > > > > > > Can you check that you are using _both_ mkimage and kwboot from > > > > > > > version > > > > > > > with applying _all_ my patches recently sent to ML? Because both > > > > > > > mkimage > > > > > > > and kwboot have fixes for SATA and SDIO images. > > > > > > > > > > > > > > For me it looks like that either mkimage generated incorrect > > > > > > > image size > > > > > > > for SATA or SDIO image. Or kwboot incorrectly parsed that image > > > > > > > size > > > > > > > from kwbimage header and sent smaller image. > > > > > > > > > > > > > > > > > > > > > > > > Also could you check if SATA booting is still working > > > > > > > > > correctly? > > > > > > > > > > > > > > > > > > > > > > > > > SATA works correctly. > > > > > > > > > > > > > > Perfect! > > > > > > > > > > > > > > > > > > > > > > > > Tony, should address problems with SPI booting when it is > > > > > > > > > configured to > > > > > > > > > different configuration. In fourth commit I added all > > > > > > > > > possible boot mode > > > > > > > > > strapping pin configurations which are recognized by A385 > > > > > > > > > bootrom (and > > > > > > > > > not the only one described in the HW spec, which is > > > > > > > > > incomplete). > > > > > > > > > > > > It works great! Here the strapping is SPI 1 (0x34), and the boot > > > > > > mode > > > > > > is set to "Trying to boot from SPI" correctly. I'm having a problem > > > > > > with SPL SPI probing the device. But I don't think it is not related > > > > > > to this boot mode patch. There is something in SPL
Re: [PATCH v3 14/17] riscv: dts: jh7110: Add initial StarFive JH7110 device tree
On 2023/3/4 5:16, Conor Dooley wrote: > On Fri, Mar 03, 2023 at 11:24:29AM +0800, Yanhong Wang wrote: >> Add initial device tree for the JH7110 RISC-V SoC. >> >> Signed-off-by: Yanhong Wang >> --- >> arch/riscv/dts/jh7110.dtsi | 582 + >> 1 file changed, 582 insertions(+) >> create mode 100644 arch/riscv/dts/jh7110.dtsi >> >> diff --git a/arch/riscv/dts/jh7110.dtsi b/arch/riscv/dts/jh7110.dtsi >> new file mode 100644 >> index 00..d3e9f92987 >> --- /dev/null >> +++ b/arch/riscv/dts/jh7110.dtsi >> @@ -0,0 +1,582 @@ >> +// SPDX-License-Identifier: GPL-2.0 OR MIT >> +/* >> + * Copyright (C) 2022 StarFive Technology Co., Ltd. >> + */ >> + >> +/dts-v1/; >> +#include >> +#include >> + >> +/ { >> +compatible = "starfive,jh7110"; >> +#address-cells = <2>; >> +#size-cells = <2>; >> + >> +cpus { >> +#address-cells = <1>; >> +#size-cells = <0>; >> + >> +S7_0: cpu@0 { >> +compatible = "sifive,s7", "riscv"; >> +reg = <0>; >> +d-cache-block-size = <64>; >> +d-cache-sets = <64>; >> +d-cache-size = <8192>; >> +d-tlb-sets = <1>; >> +d-tlb-size = <40>; >> +device_type = "cpu"; >> +i-cache-block-size = <64>; >> +i-cache-sets = <64>; >> +i-cache-size = <16384>; >> +i-tlb-sets = <1>; >> +i-tlb-size = <40>; >> +mmu-type = "riscv,sv39"; >> +next-level-cache = <>; >> +riscv,isa = "rv64imac_zba_zbb"; > > Hmm, based on what Sean said on the previous version, "We use strchr on > it; so something like Zicsr is parsed as 5 extensions", are you sure that > adding this here behaves correctly? > As you said, u-boot does not parse the content after '_', zba/zbb has no practical meaning in u-boot. The definitions of 'riscv,isa' refer to linux and is consistent with this. https://patchwork.kernel.org/project/linux-riscv/patch/20230221024645.127922-18-hal.f...@starfivetech.com/ Do you have any better suggestion? Referring to the definition of Sifive fu740, remove '_zba_zbb'? > https://lore.kernel.org/u-boot/22e805d4-f823-975c-a970-a4a19bb13...@gmail.com/ > > I know that having zba/zbb is *correct*, but if U-Boot doesn't parse it > correctly... > > Cheers, > Conor.
Re: [PATCH RFC u-boot-mvebu 0/6] arm: mvebu: Fix boot mode detection
On Monday 06 March 2023 16:01:58 Tony Dinh wrote: > Hi Pali, > > On Sun, Mar 5, 2023 at 4:41 PM Tony Dinh wrote: > > > > Hi Pali, > > > > On Sun, Mar 5, 2023 at 2:54 PM Pali Rohár wrote: > > > > > > On Sunday 05 March 2023 14:46:55 Tony Dinh wrote: > > > > On Sun, Mar 5, 2023 at 2:44 PM Tony Dinh wrote: > > > > > > > > > > Hi Pali, > > > > > > > > > > On Sun, Mar 5, 2023 at 3:55 AM Pali Rohár wrote: > > > > > > > > > > > > On Sunday 05 March 2023 04:21:42 Martin Rowe wrote: > > > > > > > On Sat, 4 Mar 2023 at 10:51, Pali Rohár wrote: > > > > > > > > > > > > > > > Improve code for checking strapping pins which specifies boot > > > > > > > > mode source. > > > > > > > > > > > > > > > > Martin, could you test if Clearfog can be still configured into > > > > > > > > UART > > > > > > > > booting mode via HW switches and if it still works correctly? > > > > > > > > First > > > > > > > > patch is reverting UART related commit for Clearfog which I > > > > > > > > think it not > > > > > > > > needed anymore. > > > > > > > > > > > > > > > > > > > > > > On Clearfog the logic in the CONFIG_ARMADA_38X ifdef before the > > > > > > > switch that > > > > > > > you refactored in cpu.c/get_boot_device is all that gets > > > > > > > processed. It > > > > > > > decides there is an error and returns BOOT_DEVICE_UART, probably > > > > > > > because of > > > > > > > the invalid boot workaround for broken UART selection that you > > > > > > > identified. > > > > > > > > > > > > Ok, so I figured out correctly how this invalid mode works. > > > > > > > > > > > > > UART only works if I use the clearfog_spi_defconfig or if I select > > > > > > > CONFIG_MVEBU_SPL_BOOT_DEVICE_UART=y. It does not work with the > > > > > > > MMC or SATA > > > > > > > defconfigs. I get the same result without this patch series > > > > > > > applied, though. > > > > > > > > > > > > > > The failed cases have the same output (other than kwboot header > > > > > > > patching > > > > > > > output) until after sending boot image data is complete. The > > > > > > > output stops > > > > > > > after: > > > > > > > > > > > > > > 98 % > > > > > > > [. > > > > > > > ] > > > > > > > Done > > > > > > > Finishing transfer > > > > > > > [Type Ctrl-\ + c to quit] > > > > > > > > > > > > > > > > > > > This is very strange because CONFIG_MVEBU_SPL_BOOT_DEVICE_UART just > > > > > > instruct mkimage what to put into kwbimage header. > > > > > > > > > > > > If I'm looking at the output correctly then SPL was booted, it > > > > > > correctly > > > > > > trained DDR RAM, returned back to bootrom, kwboot continued sending > > > > > > main > > > > > > u-boot and bootrom confirmed that transfer of both SPL and main > > > > > > u-boot > > > > > > is complete. But then there is no output from main u-boot. > > > > > > > > > > > > > It looks like an unrelated issue with kwboot.c, which I was sure > > > > > > > was > > > > > > > working after the last patches but I can no longer reproduce a > > > > > > > successful > > > > > > > boot. > > > > > > > > > > > > Can you check that you are using _both_ mkimage and kwboot from > > > > > > version > > > > > > with applying _all_ my patches recently sent to ML? Because both > > > > > > mkimage > > > > > > and kwboot have fixes for SATA and SDIO images. > > > > > > > > > > > > For me it looks like that either mkimage generated incorrect image > > > > > > size > > > > > > for SATA or SDIO image. Or kwboot incorrectly parsed that image size > > > > > > from kwbimage header and sent smaller image. > > > > > > > > > > > > > > > > > > > > > Also could you check if SATA booting is still working correctly? > > > > > > > > > > > > > > > > > > > > > > SATA works correctly. > > > > > > > > > > > > Perfect! > > > > > > > > > > > > > > > > > > > > > Tony, should address problems with SPI booting when it is > > > > > > > > configured to > > > > > > > > different configuration. In fourth commit I added all possible > > > > > > > > boot mode > > > > > > > > strapping pin configurations which are recognized by A385 > > > > > > > > bootrom (and > > > > > > > > not the only one described in the HW spec, which is incomplete). > > > > > > > > > > It works great! Here the strapping is SPI 1 (0x34), and the boot mode > > > > > is set to "Trying to boot from SPI" correctly. I'm having a problem > > > > > with SPL SPI probing the device. But I don't think it is not related > > > > > to this boot mode patch. There is something in SPL SPI that either I > > > > > don't understand or it is a bug. > > > > > > > > I meant "But I don't think it is related to this boot mode patch". > > > > > > 0x34 uses SPI controller 1. So maybe you need to adjust some config > > > options? In your log is usage of bus 0, so maybe this could be the > > > reason. > > > > Previously I did not use bus 1, and used the default bus 0 and
Re: [PATCH RFC u-boot-mvebu 0/6] arm: mvebu: Fix boot mode detection
Hi Pali, On Sun, Mar 5, 2023 at 4:41 PM Tony Dinh wrote: > > Hi Pali, > > On Sun, Mar 5, 2023 at 2:54 PM Pali Rohár wrote: > > > > On Sunday 05 March 2023 14:46:55 Tony Dinh wrote: > > > On Sun, Mar 5, 2023 at 2:44 PM Tony Dinh wrote: > > > > > > > > Hi Pali, > > > > > > > > On Sun, Mar 5, 2023 at 3:55 AM Pali Rohár wrote: > > > > > > > > > > On Sunday 05 March 2023 04:21:42 Martin Rowe wrote: > > > > > > On Sat, 4 Mar 2023 at 10:51, Pali Rohár wrote: > > > > > > > > > > > > > Improve code for checking strapping pins which specifies boot > > > > > > > mode source. > > > > > > > > > > > > > > Martin, could you test if Clearfog can be still configured into > > > > > > > UART > > > > > > > booting mode via HW switches and if it still works correctly? > > > > > > > First > > > > > > > patch is reverting UART related commit for Clearfog which I think > > > > > > > it not > > > > > > > needed anymore. > > > > > > > > > > > > > > > > > > > On Clearfog the logic in the CONFIG_ARMADA_38X ifdef before the > > > > > > switch that > > > > > > you refactored in cpu.c/get_boot_device is all that gets processed. > > > > > > It > > > > > > decides there is an error and returns BOOT_DEVICE_UART, probably > > > > > > because of > > > > > > the invalid boot workaround for broken UART selection that you > > > > > > identified. > > > > > > > > > > Ok, so I figured out correctly how this invalid mode works. > > > > > > > > > > > UART only works if I use the clearfog_spi_defconfig or if I select > > > > > > CONFIG_MVEBU_SPL_BOOT_DEVICE_UART=y. It does not work with the MMC > > > > > > or SATA > > > > > > defconfigs. I get the same result without this patch series > > > > > > applied, though. > > > > > > > > > > > > The failed cases have the same output (other than kwboot header > > > > > > patching > > > > > > output) until after sending boot image data is complete. The output > > > > > > stops > > > > > > after: > > > > > > > > > > > > 98 % > > > > > > [. > > > > > > ] > > > > > > Done > > > > > > Finishing transfer > > > > > > [Type Ctrl-\ + c to quit] > > > > > > > > > > > > > > > > This is very strange because CONFIG_MVEBU_SPL_BOOT_DEVICE_UART just > > > > > instruct mkimage what to put into kwbimage header. > > > > > > > > > > If I'm looking at the output correctly then SPL was booted, it > > > > > correctly > > > > > trained DDR RAM, returned back to bootrom, kwboot continued sending > > > > > main > > > > > u-boot and bootrom confirmed that transfer of both SPL and main u-boot > > > > > is complete. But then there is no output from main u-boot. > > > > > > > > > > > It looks like an unrelated issue with kwboot.c, which I was sure was > > > > > > working after the last patches but I can no longer reproduce a > > > > > > successful > > > > > > boot. > > > > > > > > > > Can you check that you are using _both_ mkimage and kwboot from > > > > > version > > > > > with applying _all_ my patches recently sent to ML? Because both > > > > > mkimage > > > > > and kwboot have fixes for SATA and SDIO images. > > > > > > > > > > For me it looks like that either mkimage generated incorrect image > > > > > size > > > > > for SATA or SDIO image. Or kwboot incorrectly parsed that image size > > > > > from kwbimage header and sent smaller image. > > > > > > > > > > > > > > > > > > Also could you check if SATA booting is still working correctly? > > > > > > > > > > > > > > > > > > > SATA works correctly. > > > > > > > > > > Perfect! > > > > > > > > > > > > > > > > > > Tony, should address problems with SPI booting when it is > > > > > > > configured to > > > > > > > different configuration. In fourth commit I added all possible > > > > > > > boot mode > > > > > > > strapping pin configurations which are recognized by A385 bootrom > > > > > > > (and > > > > > > > not the only one described in the HW spec, which is incomplete). > > > > > > > > It works great! Here the strapping is SPI 1 (0x34), and the boot mode > > > > is set to "Trying to boot from SPI" correctly. I'm having a problem > > > > with SPL SPI probing the device. But I don't think it is not related > > > > to this boot mode patch. There is something in SPL SPI that either I > > > > don't understand or it is a bug. > > > > > > I meant "But I don't think it is related to this boot mode patch". > > > > 0x34 uses SPI controller 1. So maybe you need to adjust some config > > options? In your log is usage of bus 0, so maybe this could be the > > reason. > > Previously I did not use bus 1, and used the default bus 0 and it > still works! so I've suspected there is some problem in SPL SPI (i.e. > it works when it should not). But now default bus 0 no longer works. > > I am configuring bus 1, but I'm not yet successful. There are > multiple places where bus 1 is needed to be specified. Also, might I > also need -u-boot.dtsi to
Re: [PATCH] cmd: fdt: Drop the 0x prefix
Hi Marek, On Mon, 6 Mar 2023 at 12:07, Marek Vasut wrote: > > On 3/6/23 18:53, Simon Glass wrote: > > Hi Marek, > > > > On Wed, 1 Mar 2023 at 20:04, Marek Vasut > > wrote: > >> > >> The 'fdt get addr' is always assumed to be hex value, drop the prefix. > >> Since this might break existing users who depend on the existing > >> behavior with 0x prefix, this is a separate patch. Revert if this > >> breaks anything. > >> > >> Signed-off-by: Marek Vasut > >> --- > >> Cc: Heinrich Schuchardt > >> Cc: Simon Glass > >> Cc: Tom Rini > >> --- > >> cmd/fdt.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/cmd/fdt.c b/cmd/fdt.c > >> index f38fe909c3e..04b664e652c 100644 > >> --- a/cmd/fdt.c > >> +++ b/cmd/fdt.c > >> @@ -478,7 +478,7 @@ static int do_fdt(struct cmd_tbl *cmdtp, int flag, int > >> argc, char *const argv[]) > >> /* Get address */ > >> char buf[19]; > >> > >> - snprintf(buf, sizeof(buf), "0x%lx", > >> + snprintf(buf, sizeof(buf), "%lx", > >> > >> (ulong)map_to_sysmem(nodep)); > >> env_set(var, buf); > >> } else if (subcmd[0] == 's') { > >> -- > >> 2.39.2 > >> > > > > iwc how about using env_sethex() ? > > The 'env get size' 's' case below could likely use similar treatment , > do I read it right ? Yes...I think the helpers were added more recently than this code. Regards, Simon
Re: [PATCH v5 0/6] FWU: Handle meta-data in common code
On Mon, Feb 27, 2023 at 7:28 PM Tom Rini wrote: > > On Mon, Feb 27, 2023 at 07:00:10PM -0600, Jassi Brar wrote: > > On Mon, 27 Feb 2023 at 18:58, Tom Rini wrote: > > > > > > On Mon, Feb 27, 2023 at 06:51:35PM -0600, jassisinghb...@gmail.com wrote: > > > > > > > From: Jassi Brar > > > > > > > > The patchset reduces ~400 lines of code, while keeping the > > > > functionality same and making > > > > meta-data operations much faster (by using cached structures). > > > > > > > > Issue: > > > > meta-data copies (primary and secondary) are being handled by the > > > > backend/storage layer > > > > instead of the common core in fwu.c (as also noted by Ilias) that is, > > > > gpt_blk.c manages > > > > meta-data and similarly raw_mtd.c will have to do the same when it > > > > arrives. The code > > > > could by make smaller, cleaner and optimised. > > > > > > > > Basic idea: > > > > Introduce .read_mdata() and .write_mdata() in fwu_mdata_ops that > > > > simply read/write > > > > meta-data copy. The core code takes care of integrity and redundancy of > > > > the meta-data, > > > > as a result we can get rid of every other callback .get_mdata() > > > > .update_mdata() > > > > .get_mdata_part_num() .read_mdata_partition() > > > > .write_mdata_partition() and the > > > > corresponding wrapper functions thereby making the code 100s of LOC > > > > smaller. > > > > > > > > Get rid of fwu_check_mdata_validity() and fwu_mdata_check() which > > > > expected underlying > > > > layer to manage and verify mdata copies. > > > > Implement fwu_get_verified_mdata(struct fwu_mdata *mdata) public > > > > function that reads, > > > > verifies and, if needed, fixes the meta-data copies. > > > > > > > > Verified copy of meta-data is now cached as 'g_mdata' in fwu.c, which > > > > avoids multiple > > > > low-level expensive read and parse calls. > > > > gpt meta-data partition numbers are now cached in gpt_blk.c, so that we > > > > don't have to do expensive part_get_info() and uid ops. > > > > > > > > Changes since v4: > > > > * Change fwu-mdata-mtd bindings to not require external changes > > > > * Handle 'part == BOTH_PARTS' in fwu_sync_mdata > > > > * use parts_ok[] and parts_mdata[] instead of pri/sec_ok and > > > > p/s_mdata > > > > > > Did you run this through CI / build sandbox? This doesn't read like you > > > fixed the problem I reported in CI, when I was trying to merge v4. > > > > > I know that remains to be done. > > The dt-bindings for fwu-mdata is changed in this patchset and I > > thought any testcase may be impacted by it. > > So you're not expecting this iteration to be merged, as CI doesn't pass, > and that's known? OK. > Sorry I got confused. I thought new test cases are expected to be written, whereas you only wanted the sandbox compilation breakage fixed. My replies must have sounded so stupid :( I have a patch to fix sandbox compilation in the v6 revision. thanks.
[PATCH v6 7/7] test: dm: fwu: fix for the updated api
From: Jassi Brar fwu_get_mdata() no more requires 'dev' argument and fwu_check_mdata_validity() has been rendered useless and dropped. Fix the test cases to work with aforementioned changes. Signed-off-by: Jassi Brar --- test/dm/fwu_mdata.c | 22 +++--- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/test/dm/fwu_mdata.c b/test/dm/fwu_mdata.c index b179a65c15..8b5c83ef4e 100644 --- a/test/dm/fwu_mdata.c +++ b/test/dm/fwu_mdata.c @@ -98,7 +98,7 @@ static int dm_test_fwu_mdata_read(struct unit_test_state *uts) ut_assertok(populate_mmc_disk_image(uts)); ut_assertok(write_mmc_blk_device(uts)); - ut_assertok(fwu_get_mdata(dev, )); + ut_assertok(fwu_get_mdata()); ut_asserteq(mdata.version, 0x1); @@ -118,30 +118,14 @@ static int dm_test_fwu_mdata_write(struct unit_test_state *uts) ut_assertok(uclass_first_device_err(UCLASS_FWU_MDATA, )); - ut_assertok(fwu_get_mdata(dev, )); + ut_assertok(fwu_get_mdata()); active_idx = (mdata.active_index + 1) % CONFIG_FWU_NUM_BANKS; ut_assertok(fwu_set_active_index(active_idx)); - ut_assertok(fwu_get_mdata(dev, )); + ut_assertok(fwu_get_mdata()); ut_asserteq(mdata.active_index, active_idx); return 0; } DM_TEST(dm_test_fwu_mdata_write, UT_TESTF_SCAN_FDT); - -static int dm_test_fwu_mdata_check(struct unit_test_state *uts) -{ - struct udevice *dev; - - ut_assertok(setup_blk_device(uts)); - ut_assertok(populate_mmc_disk_image(uts)); - ut_assertok(write_mmc_blk_device(uts)); - - ut_assertok(uclass_first_device_err(UCLASS_FWU_MDATA, )); - - ut_assertok(fwu_check_mdata_validity()); - - return 0; -} -DM_TEST(dm_test_fwu_mdata_check, UT_TESTF_SCAN_FDT); -- 2.34.1
[PATCH v6 5/7] fwu: meta-data: switch to management by common code
From: Jassi Brar The common code can now read, verify and fix meta-data copies while exposing one consistent structure to users. Only the .read_mdata() and .write_mdata() callbacks of fwu_mdata_ops are needed. Get rid of .get_mdata() .update_mdata() .get_mdata_part_num() .read_mdata_partition() and .write_mdata_partition() and also the corresponding wrapper functions. Signed-off-by: Jassi Brar Reviewed-by: Etienne Carriere --- cmd/fwu_mdata.c | 17 +- drivers/fwu-mdata/fwu-mdata-uclass.c | 165 --- drivers/fwu-mdata/gpt_blk.c | 124 +- include/fwu.h| 199 --- lib/fwu_updates/fwu.c| 235 --- 5 files changed, 38 insertions(+), 702 deletions(-) diff --git a/cmd/fwu_mdata.c b/cmd/fwu_mdata.c index f04af27de6..9b70340368 100644 --- a/cmd/fwu_mdata.c +++ b/cmd/fwu_mdata.c @@ -43,23 +43,10 @@ static void print_mdata(struct fwu_mdata *mdata) int do_fwu_mdata_read(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[]) { - struct udevice *dev; int ret = CMD_RET_SUCCESS, res; - struct fwu_mdata mdata = { 0 }; + struct fwu_mdata mdata; - if (uclass_get_device(UCLASS_FWU_MDATA, 0, ) || !dev) { - log_err("Unable to get FWU metadata device\n"); - return CMD_RET_FAILURE; - } - - res = fwu_check_mdata_validity(); - if (res < 0) { - log_err("FWU Metadata check failed\n"); - ret = CMD_RET_FAILURE; - goto out; - } - - res = fwu_get_mdata(dev, ); + res = fwu_get_verified_mdata(); if (res < 0) { log_err("Unable to get valid FWU metadata\n"); ret = CMD_RET_FAILURE; diff --git a/drivers/fwu-mdata/fwu-mdata-uclass.c b/drivers/fwu-mdata/fwu-mdata-uclass.c index e03773c584..0a8edaaa41 100644 --- a/drivers/fwu-mdata/fwu-mdata-uclass.c +++ b/drivers/fwu-mdata/fwu-mdata-uclass.c @@ -14,7 +14,6 @@ #include #include -#include /** * fwu_read_mdata() - Wrapper around fwu_mdata_ops.read_mdata() @@ -50,170 +49,6 @@ int fwu_write_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary) return ops->write_mdata(dev, mdata, primary); } -/** - * fwu_get_mdata_part_num() - Get the FWU metadata partition numbers - * @dev: FWU metadata device - * @mdata_parts: array for storing the metadata partition numbers - * - * Get the partition numbers on the storage device on which the - * FWU metadata is stored. Two partition numbers will be returned. - * - * Return: 0 if OK, -ve on error - * - */ -int fwu_get_mdata_part_num(struct udevice *dev, uint *mdata_parts) -{ - const struct fwu_mdata_ops *ops = device_get_ops(dev); - - if (!ops->get_mdata_part_num) { - log_debug("get_mdata_part_num() method not defined\n"); - return -ENOSYS; - } - - return ops->get_mdata_part_num(dev, mdata_parts); -} - -/** - * fwu_read_mdata_partition() - Read the FWU metadata from a partition - * @dev: FWU metadata device - * @mdata: Copy of the FWU metadata - * @part_num: Partition number from which FWU metadata is to be read - * - * Read the FWU metadata from the specified partition number - * - * Return: 0 if OK, -ve on error - * - */ -int fwu_read_mdata_partition(struct udevice *dev, struct fwu_mdata *mdata, -uint part_num) -{ - const struct fwu_mdata_ops *ops = device_get_ops(dev); - - if (!ops->read_mdata_partition) { - log_debug("read_mdata_partition() method not defined\n"); - return -ENOSYS; - } - - return ops->read_mdata_partition(dev, mdata, part_num); -} - -/** - * fwu_write_mdata_partition() - Write the FWU metadata to a partition - * @dev: FWU metadata device - * @mdata: Copy of the FWU metadata - * @part_num: Partition number to which FWU metadata is to be written - * - * Write the FWU metadata to the specified partition number - * - * Return: 0 if OK, -ve on error - * - */ -int fwu_write_mdata_partition(struct udevice *dev, struct fwu_mdata *mdata, - uint part_num) -{ - const struct fwu_mdata_ops *ops = device_get_ops(dev); - - if (!ops->write_mdata_partition) { - log_debug("write_mdata_partition() method not defined\n"); - return -ENOSYS; - } - - return ops->write_mdata_partition(dev, mdata, part_num); -} - -/** - * fwu_mdata_check() - Check if the FWU metadata is valid - * @dev: FWU metadata device - * - * Validate both copies of the FWU metadata. If one of the copies - * has gone bad, restore it from the other copy. - * - * Return: 0 if OK, -ve on error - * - */ -int fwu_mdata_check(struct udevice *dev) -{ - const struct fwu_mdata_ops *ops = device_get_ops(dev); - - if (!ops->check_mdata) { - log_debug("check_mdata() method not
[PATCH v6 6/7] fwu: rename fwu_get_verified_mdata to fwu_get_mdata
From: Jassi Brar fwu_get_mdata() sounds more appropriate than fwu_get_verified_mdata() Signed-off-by: Jassi Brar Reviewed-by: Etienne Carriere Reviewed-by: Ilias Apalodimas --- cmd/fwu_mdata.c | 2 +- include/fwu.h | 4 ++-- lib/fwu_updates/fwu.c | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cmd/fwu_mdata.c b/cmd/fwu_mdata.c index 9b70340368..5ecda455df 100644 --- a/cmd/fwu_mdata.c +++ b/cmd/fwu_mdata.c @@ -46,7 +46,7 @@ int do_fwu_mdata_read(struct cmd_tbl *cmdtp, int flag, int ret = CMD_RET_SUCCESS, res; struct fwu_mdata mdata; - res = fwu_get_verified_mdata(); + res = fwu_get_mdata(); if (res < 0) { log_err("Unable to get valid FWU metadata\n"); ret = CMD_RET_FAILURE; diff --git a/include/fwu.h b/include/fwu.h index 314aadea59..33b4d0b3db 100644 --- a/include/fwu.h +++ b/include/fwu.h @@ -80,7 +80,7 @@ int fwu_read_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary); int fwu_write_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary); /** - * fwu_get_verified_mdata() - Read, verify and return the FWU metadata + * fwu_get_mdata() - Read, verify and return the FWU metadata * * Read both the metadata copies from the storage media, verify their checksum, * and ascertain that both copies match. If one of the copies has gone bad, @@ -88,7 +88,7 @@ int fwu_write_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary); * * Return: 0 if OK, -ve on error */ -int fwu_get_verified_mdata(struct fwu_mdata *mdata); +int fwu_get_mdata(struct fwu_mdata *mdata); /** * fwu_get_active_index() - Get active_index from the FWU metadata diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c index 49c9316c70..a24ccf567a 100644 --- a/lib/fwu_updates/fwu.c +++ b/lib/fwu_updates/fwu.c @@ -189,7 +189,7 @@ static inline int mdata_crc_check(struct fwu_mdata *mdata) } /** - * fwu_get_verified_mdata() - Read, verify and return the FWU metadata + * fwu_get_mdata() - Read, verify and return the FWU metadata * @mdata: Output FWU metadata read or NULL * * Read both the metadata copies from the storage media, verify their checksum, @@ -198,7 +198,7 @@ static inline int mdata_crc_check(struct fwu_mdata *mdata) * * Return: 0 if OK, -ve on error */ -int fwu_get_verified_mdata(struct fwu_mdata *mdata) +int fwu_get_mdata(struct fwu_mdata *mdata) { int err; bool parts_ok[2] = { false }; @@ -615,7 +615,7 @@ static int fwu_boottime_checks(void *ctx, struct event *event) return ret; } - ret = fwu_get_verified_mdata(NULL); + ret = fwu_get_mdata(NULL); if (ret) { log_debug("Unable to read meta-data\n"); return ret; -- 2.34.1
[PATCH v6 4/7] fwu: gpt: implement read_mdata and write_mdata callbacks
From: Jassi Brar Moving towards using common code for meta-data management, implement the read/write mdata hooks. Signed-off-by: Jassi Brar Reviewed-by: Etienne Carriere Reviewed-by: Ilias Apalodimas --- drivers/fwu-mdata/gpt_blk.c | 36 1 file changed, 36 insertions(+) diff --git a/drivers/fwu-mdata/gpt_blk.c b/drivers/fwu-mdata/gpt_blk.c index 28f5d23e1e..be1a6b03a1 100644 --- a/drivers/fwu-mdata/gpt_blk.c +++ b/drivers/fwu-mdata/gpt_blk.c @@ -272,7 +272,43 @@ static int fwu_mdata_gpt_blk_probe(struct udevice *dev) return 0; } +static int fwu_gpt_read_mdata(struct udevice *dev, struct fwu_mdata *mdata, +bool primary) +{ + struct fwu_mdata_gpt_blk_priv *priv = dev_get_priv(dev); + struct blk_desc *desc = dev_get_uclass_plat(priv->blk_dev); + int ret; + + ret = gpt_get_mdata_partitions(desc); + if (ret < 0) { + log_debug("Error getting the FWU metadata partitions\n"); + return -ENOENT; + } + + return gpt_read_write_mdata(desc, mdata, MDATA_READ, + primary ? g_mdata_part[0] : g_mdata_part[1]); +} + +static int fwu_gpt_write_mdata(struct udevice *dev, struct fwu_mdata *mdata, +bool primary) +{ + struct fwu_mdata_gpt_blk_priv *priv = dev_get_priv(dev); + struct blk_desc *desc = dev_get_uclass_plat(priv->blk_dev); + int ret; + + ret = gpt_get_mdata_partitions(desc); + if (ret < 0) { + log_debug("Error getting the FWU metadata partitions\n"); + return -ENOENT; + } + + return gpt_read_write_mdata(desc, mdata, MDATA_WRITE, + primary ? g_mdata_part[0] : g_mdata_part[1]); +} + static const struct fwu_mdata_ops fwu_gpt_blk_ops = { + .read_mdata = fwu_gpt_read_mdata, + .write_mdata = fwu_gpt_write_mdata, .get_mdata = fwu_gpt_get_mdata, .update_mdata = fwu_gpt_update_mdata, .get_mdata_part_num = fwu_gpt_get_mdata_partitions, -- 2.34.1
[PATCH v6 3/7] fwu: move meta-data management in core
From: Jassi Brar Instead of each i/f having to implement their own meta-data verification and storage, move the logic in common code. This simplifies the i/f code much simpler and compact. Signed-off-by: Jassi Brar --- drivers/fwu-mdata/fwu-mdata-uclass.c | 34 +++ include/fwu.h| 41 + lib/fwu_updates/fwu.c| 131 ++- 3 files changed, 201 insertions(+), 5 deletions(-) diff --git a/drivers/fwu-mdata/fwu-mdata-uclass.c b/drivers/fwu-mdata/fwu-mdata-uclass.c index b477e9603f..e03773c584 100644 --- a/drivers/fwu-mdata/fwu-mdata-uclass.c +++ b/drivers/fwu-mdata/fwu-mdata-uclass.c @@ -16,6 +16,40 @@ #include #include +/** + * fwu_read_mdata() - Wrapper around fwu_mdata_ops.read_mdata() + * + * Return: 0 if OK, -ve on error + */ +int fwu_read_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary) +{ + const struct fwu_mdata_ops *ops = device_get_ops(dev); + + if (!ops->read_mdata) { + log_debug("read_mdata() method not defined\n"); + return -ENOSYS; + } + + return ops->read_mdata(dev, mdata, primary); +} + +/** + * fwu_write_mdata() - Wrapper around fwu_mdata_ops.write_mdata() + * + * Return: 0 if OK, -ve on error + */ +int fwu_write_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary) +{ + const struct fwu_mdata_ops *ops = device_get_ops(dev); + + if (!ops->write_mdata) { + log_debug("write_mdata() method not defined\n"); + return -ENOSYS; + } + + return ops->write_mdata(dev, mdata, primary); +} + /** * fwu_get_mdata_part_num() - Get the FWU metadata partition numbers * @dev: FWU metadata device diff --git a/include/fwu.h b/include/fwu.h index 0919ced812..13f8fdeb28 100644 --- a/include/fwu.h +++ b/include/fwu.h @@ -24,6 +24,26 @@ struct fwu_mdata_gpt_blk_priv { * @update_mdata() - Update the FWU metadata copy */ struct fwu_mdata_ops { + /** +* read_mdata() - Populate the asked FWU metadata copy +* @dev: FWU metadata device +* @mdata: Output FWU mdata read +* @primary: If primary or secondary copy of metadata is to be read +* +* Return: 0 if OK, -ve on error +*/ + int (*read_mdata)(struct udevice *dev, struct fwu_mdata *mdata, bool primary); + + /** +* write_mdata() - Write the given FWU metadata copy +* @dev: FWU metadata device +* @mdata: Copy of the FWU metadata to write +* @primary: If primary or secondary copy of metadata is to be written +* +* Return: 0 if OK, -ve on error +*/ + int (*write_mdata)(struct udevice *dev, struct fwu_mdata *mdata, bool primary); + /** * check_mdata() - Check if the FWU metadata is valid * @dev:FWU device @@ -126,6 +146,27 @@ struct fwu_mdata_ops { EFI_GUID(0x0c996046, 0xbcc0, 0x4d04, 0x85, 0xec, \ 0xe1, 0xfc, 0xed, 0xf1, 0xc6, 0xf8) +/** + * fwu_read_mdata() - Wrapper around fwu_mdata_ops.read_mdata() + */ +int fwu_read_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary); + +/** + * fwu_write_mdata() - Wrapper around fwu_mdata_ops.write_mdata() + */ +int fwu_write_mdata(struct udevice *dev, struct fwu_mdata *mdata, bool primary); + +/** + * fwu_get_verified_mdata() - Read, verify and return the FWU metadata + * + * Read both the metadata copies from the storage media, verify their checksum, + * and ascertain that both copies match. If one of the copies has gone bad, + * restore it from the good copy. + * + * Return: 0 if OK, -ve on error +*/ +int fwu_get_verified_mdata(struct fwu_mdata *mdata); + /** * fwu_check_mdata_validity() - Check for validity of the FWU metadata copies * diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c index 5313d07302..8f1c05ad1c 100644 --- a/lib/fwu_updates/fwu.c +++ b/lib/fwu_updates/fwu.c @@ -15,13 +15,13 @@ #include #include +#include + +static struct fwu_mdata g_mdata; /* = {0} makes uninit crc32 always invalid */ +static struct udevice *g_dev; static u8 in_trial; static u8 boottime_check; -#include -#include -#include - enum { IMAGE_ACCEPT_SET = 1, IMAGE_ACCEPT_CLEAR, @@ -161,6 +161,127 @@ static int fwu_get_image_type_id(u8 *image_index, efi_guid_t *image_type_id) return -ENOENT; } +/** + * fwu_sync_mdata() - Update given meta-data partition(s) with the copy provided + * @mdata: FWU metadata structure + * @part: Bitmask of FWU metadata partitions to be written to + * + * Return: 0 if OK, -ve on error + */ +static int fwu_sync_mdata(struct fwu_mdata *mdata, int part) +{ + void *buf = >version; + int err; + + if (part == BOTH_PARTS) { + err = fwu_sync_mdata(mdata, SECONDARY_PART); + if (err) + return err; + part = PRIMARY_PART; + } + + /* +* Calculate the
[PATCH v6 2/7] fwu: gpt: use cached meta-data partition numbers
From: Jassi Brar Use cached values and avoid parsing and scanning through partitions everytime for meta-data partitions because they can't change after bootup. Acked-by: Etienne Carriere Reviewed-by: Ilias Apalodimas Signed-off-by: Jassi Brar --- drivers/fwu-mdata/gpt_blk.c | 43 + 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/drivers/fwu-mdata/gpt_blk.c b/drivers/fwu-mdata/gpt_blk.c index d35ce49c5c..28f5d23e1e 100644 --- a/drivers/fwu-mdata/gpt_blk.c +++ b/drivers/fwu-mdata/gpt_blk.c @@ -24,8 +24,9 @@ enum { MDATA_WRITE, }; -static int gpt_get_mdata_partitions(struct blk_desc *desc, - uint mdata_parts[2]) +static uint g_mdata_part[2]; /* = {0, 0} to check against uninit parts */ + +static int gpt_get_mdata_partitions(struct blk_desc *desc) { int i, ret; u32 nparts; @@ -33,18 +34,19 @@ static int gpt_get_mdata_partitions(struct blk_desc *desc, struct disk_partition info; const efi_guid_t fwu_mdata_guid = FWU_MDATA_GUID; + /* if primary and secondary partitions already found */ + if (g_mdata_part[0] && g_mdata_part[1]) + return 0; + nparts = 0; - for (i = 1; i < MAX_SEARCH_PARTITIONS; i++) { + for (i = 1; i < MAX_SEARCH_PARTITIONS && nparts < 2; i++) { if (part_get_info(desc, i, )) continue; uuid_str_to_bin(info.type_guid, part_type_guid.b, UUID_STR_FORMAT_GUID); - if (!guidcmp(_mdata_guid, _type_guid)) { - if (nparts < 2) - mdata_parts[nparts] = i; - ++nparts; - } + if (!guidcmp(_mdata_guid, _type_guid)) + g_mdata_part[nparts++] = i; } if (nparts != 2) { @@ -127,26 +129,25 @@ static int fwu_gpt_update_mdata(struct udevice *dev, struct fwu_mdata *mdata) { int ret; struct blk_desc *desc; - uint mdata_parts[2]; struct fwu_mdata_gpt_blk_priv *priv = dev_get_priv(dev); desc = dev_get_uclass_plat(priv->blk_dev); - ret = gpt_get_mdata_partitions(desc, mdata_parts); + ret = gpt_get_mdata_partitions(desc); if (ret < 0) { log_debug("Error getting the FWU metadata partitions\n"); return -ENOENT; } /* First write the primary partition */ - ret = gpt_read_write_mdata(desc, mdata, MDATA_WRITE, mdata_parts[0]); + ret = gpt_read_write_mdata(desc, mdata, MDATA_WRITE, g_mdata_part[0]); if (ret < 0) { log_debug("Updating primary FWU metadata partition failed\n"); return ret; } /* And now the replica */ - ret = gpt_read_write_mdata(desc, mdata, MDATA_WRITE, mdata_parts[1]); + ret = gpt_read_write_mdata(desc, mdata, MDATA_WRITE, g_mdata_part[1]); if (ret < 0) { log_debug("Updating secondary FWU metadata partition failed\n"); return ret; @@ -158,16 +159,14 @@ static int fwu_gpt_update_mdata(struct udevice *dev, struct fwu_mdata *mdata) static int gpt_get_mdata(struct blk_desc *desc, struct fwu_mdata *mdata) { int ret; - uint mdata_parts[2]; - - ret = gpt_get_mdata_partitions(desc, mdata_parts); + ret = gpt_get_mdata_partitions(desc); if (ret < 0) { log_debug("Error getting the FWU metadata partitions\n"); return -ENOENT; } - ret = gpt_read_write_mdata(desc, mdata, MDATA_READ, mdata_parts[0]); + ret = gpt_read_write_mdata(desc, mdata, MDATA_READ, g_mdata_part[0]); if (ret < 0) { log_debug("Failed to read the FWU metadata from the device\n"); return -EIO; @@ -182,7 +181,7 @@ static int gpt_get_mdata(struct blk_desc *desc, struct fwu_mdata *mdata) * Try to read the replica. */ memset(mdata, '\0', sizeof(struct fwu_mdata)); - ret = gpt_read_write_mdata(desc, mdata, MDATA_READ, mdata_parts[1]); + ret = gpt_read_write_mdata(desc, mdata, MDATA_READ, g_mdata_part[1]); if (ret < 0) { log_debug("Failed to read the FWU metadata from the device\n"); return -EIO; @@ -206,9 +205,15 @@ static int fwu_gpt_get_mdata(struct udevice *dev, struct fwu_mdata *mdata) static int fwu_gpt_get_mdata_partitions(struct udevice *dev, uint *mdata_parts) { struct fwu_mdata_gpt_blk_priv *priv = dev_get_priv(dev); + int err; + + err = gpt_get_mdata_partitions(dev_get_uclass_plat(priv->blk_dev)); + if (!err) { + mdata_parts[0] = g_mdata_part[0]; + mdata_parts[1] = g_mdata_part[1]; + } - return gpt_get_mdata_partitions(dev_get_uclass_plat(priv->blk_dev), - mdata_parts); + return err;
[PATCH v6 1/7] dt/bindings: fwu-mdata-mtd: drop changes outside FWU
From: Jassi Brar Any requirement of FWU should not require changes to bindings of other subsystems. For example, for mtd-backed storage we can do without requiring 'fixed-partitions' children to also carry 'uuid', a property which is non-standard and not in the bindings. There exists no code yet, so we can change the fwu-mtd bindings to contain all properties within the fwu-mdata node. Signed-off-by: Jassi Brar --- .../firmware/fwu-mdata-mtd.yaml | 105 +++--- 1 file changed, 91 insertions(+), 14 deletions(-) diff --git a/doc/device-tree-bindings/firmware/fwu-mdata-mtd.yaml b/doc/device-tree-bindings/firmware/fwu-mdata-mtd.yaml index 4f5404f999..6a22aeea30 100644 --- a/doc/device-tree-bindings/firmware/fwu-mdata-mtd.yaml +++ b/doc/device-tree-bindings/firmware/fwu-mdata-mtd.yaml @@ -1,13 +1,13 @@ # SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2 --- -$id: http://devicetree.org/schemas/firmware/u-boot,fwu-mdata-sf.yaml# -$schema: http://devicetree.org/meta-schemas/core.yaml# +$id: http://devicetree.org/schemas/firmware/u-boot,fwu-mdata-mtd.yaml# +$schema: http://devicetree.org/meta-schemas/base.yaml# title: FWU metadata on MTD device without GPT maintainers: - - Masami Hiramatsu + - Jassi Brar properties: compatible: @@ -15,24 +15,101 @@ properties: - const: u-boot,fwu-mdata-mtd fwu-mdata-store: -maxItems: 1 -description: Phandle of the MTD device which contains the FWU medatata. +$ref: /schemas/types.yaml#/definitions/phandle +description: Phandle of the MTD device which contains the FWU MetaData and Banks. - mdata-offsets: + mdata-parts: +$ref: /schemas/types.yaml#/definitions/non-unique-string-array minItems: 2 -description: Offsets of the primary and secondary FWU metadata in the NOR flash. +maxItems: 2 +description: labels of the primary and secondary FWU metadata partitions in the 'fixed-partitions' subnode of the 'jedec,spi-nor' flash device node. + + patternProperties: +"fwu-bank[0-9]": +type: object +description: List of FWU mtd-backed banks. Typically two banks. + +properties: + id: +$ref: /schemas/types.yaml#/definitions/uint32 +description: Index of the bank. + + label: +$ref: /schemas/types.yaml#/definitions/non-unique-string-array +minItems: 1 +maxItems: 1 +description: label of the partition, in the 'fixed-partitions' subnode of the 'jedec,spi-nor' flash device node, that holds this bank. + + patternProperties: +"fwu-image[0-9]": +type: object +description: List of images in the FWU mtd-backed bank. + +properties: + id: +$ref: /schemas/types.yaml#/definitions/uint32 +description: Index of the bank. + + offset: +$ref: /schemas/types.yaml#/definitions/uint32 +description: Offset, from start of the bank, where the image is located. + + size: +$ref: /schemas/types.yaml#/definitions/uint32 +description: Size reserved for the image. + + uuid: +$ref: /schemas/types.yaml#/definitions/non-unique-string-array +minItems: 1 +maxItems: 1 +description: UUID of the image. + +required: + - id + - offset + - size + - uuid +additionalProperties: false + +required: + - id + - label + - fwu-images +additionalProperties: false required: - compatible - fwu-mdata-store - - mdata-offsets - + - mdata-parts + - fwu-banks additionalProperties: false examples: - | -fwu-mdata { -compatible = "u-boot,fwu-mdata-mtd"; -fwu-mdata-store = <>; -mdata-offsets = <0x50 0x53>; -}; + fwu-mdata { + compatible = "u-boot,fwu-mdata-mtd"; + fwu-mdata-store = <>; + mdata-parts = "MDATA-Pri", "MDATA-Sec"; + + fwu-bank0 { + id = <0>; + label = "FIP-Bank0"; + fwu-image0 { + id = <0>; + offset = <0x0>; + size = <0x40>; + uuid = "5a66a702-99fd-4fef-a392-c26e261a2828"; + }; + }; + fwu-bank1 { + id = <1>; + label = "FIP-Bank1"; + fwu-image0 { + id = <0>; + offset = <0x0>; + size = <0x40>; + uuid = "a8f868a1-6e5c-4757-878d-ce63375ef2c0"; + }; + }; + }; +... -- 2.34.1
[PATCH v6 0/7] fwu: gpt: implement read_mdata and write_mdata callbacks
From: Jassi Brar The patchset reduces ~400 lines of code, while keeping the functionality same and making meta-data operations much faster (by using cached structures). Issue: meta-data copies (primary and secondary) are being handled by the backend/storage layer instead of the common core in fwu.c (as also noted by Ilias) that is, gpt_blk.c manages meta-data and similarly raw_mtd.c will have to do the same when it arrives. The code could by make smaller, cleaner and optimised. Basic idea: Introduce .read_mdata() and .write_mdata() in fwu_mdata_ops that simply read/write meta-data copy. The core code takes care of integrity and redundancy of the meta-data, as a result we can get rid of every other callback .get_mdata() .update_mdata() .get_mdata_part_num() .read_mdata_partition() .write_mdata_partition() and the corresponding wrapper functions thereby making the code 100s of LOC smaller. Get rid of fwu_check_mdata_validity() and fwu_mdata_check() which expected underlying layer to manage and verify mdata copies. Implement fwu_get_verified_mdata(struct fwu_mdata *mdata) public function that reads, verifies and, if needed, fixes the meta-data copies. Verified copy of meta-data is now cached as 'g_mdata' in fwu.c, which avoids multiple low-level expensive read and parse calls. gpt meta-data partition numbers are now cached in gpt_blk.c, so that we don't have to do expensive part_get_info() and uid ops. Changes since v5: * Fix SANDBOX tests * Removed '@' from dt nodes * misc cosmetic changes suggested by Etienne Changes since v4: * Change fwu-mdata-mtd bindings to not require external changes * Handle 'part == BOTH_PARTS' in fwu_sync_mdata * use parts_ok[] and parts_mdata[] instead of pri/sec_ok and p/s_mdata Changes since v3: * Fix error log wording * call fwu_write_mdata() with part & PRIMARY_PART ? true: false Changes since v2: * Drop whitespace changes * Fix missing mdata copy before return Changes since v1: * Fix typos and misc cosmetic changes * Catch error returns Jassi Brar (7): dt/bindings: fwu-mdata-mtd: drop changes outside FWU fwu: gpt: use cached meta-data partition numbers fwu: move meta-data management in core fwu: gpt: implement read_mdata and write_mdata callbacks fwu: meta-data: switch to management by common code fwu: rename fwu_get_verified_mdata to fwu_get_mdata test: dm: fwu: fix for the updated api cmd/fwu_mdata.c | 17 +- .../firmware/fwu-mdata-mtd.yaml | 105 ++- drivers/fwu-mdata/fwu-mdata-uclass.c | 151 + drivers/fwu-mdata/gpt_blk.c | 175 +++ include/fwu.h | 198 ++-- lib/fwu_updates/fwu.c | 296 -- test/dm/fwu_mdata.c | 22 +- 7 files changed, 299 insertions(+), 665 deletions(-) -- 2.34.1
Re: [PATCH v6 12/22] core: fdtaddr: add devfdt_get_addr_size_index_ptr function
On 3/6/23 18:53, Simon Glass wrote: > Hi Johan, > > On Thu, 2 Mar 2023 at 17:15, Johan Jonker wrote: >> >> Add devfdt_get_addr_size_index_ptr function with the same >> functionality as devfdt_get_addr_size_index, but instead >> a return pointer is given. >> >> Suggested-by: Michael Nazzareno Trimarchi >> Signed-off-by: Johan Jonker >> Reviewed-by: Michael Trimarchi >> --- >> >> Changed V5: >> fix spelling >> use tabs >> --- >> drivers/core/fdtaddr.c | 8 >> include/dm/fdtaddr.h | 17 - >> 2 files changed, 24 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/core/fdtaddr.c b/drivers/core/fdtaddr.c >> index 91bcd1a2..f5906ff9 100644 >> --- a/drivers/core/fdtaddr.c >> +++ b/drivers/core/fdtaddr.c >> @@ -122,6 +122,14 @@ fdt_addr_t devfdt_get_addr_size_index(const struct >> udevice *dev, int index, >> #endif >> } >> >> +void *devfdt_get_addr_size_index_ptr(const struct udevice *dev, int index, >> +fdt_size_t *size) >> +{ >> + fdt_addr_t addr = devfdt_get_addr_size_index(dev, index, size); >> + >> + return (addr == FDT_ADDR_T_NONE) ? NULL : (void *)(uintptr_t)addr; > Just wondering, as a side question: Why is Uboot maintaining/exporting 2 sets of functions that do the do the more or less the same thing. For example: devfdt_get_addr_size_index_ptr vs. dev_read_addr_size_index_ptr Or should we standardize and replace all by dev_read_addr_size_index_ptr if possible? Johan > > [..] > > Regards, > SImon
Re: [PATCH v5 3/6] fwu: move meta-data management in core
On Wed, Mar 1, 2023 at 5:16 AM Etienne Carriere wrote: > On Tue, 28 Feb 2023 at 01:52, wrote: > > > > From: Jassi Brar > > > > Instead of each i/f having to implement their own meta-data verification > > and storage, move the logic in common code. This simplifies the i/f code > > much simpler and compact. > > ... > > diff --git a/drivers/fwu-mdata/fwu-mdata-uclass.c > > b/drivers/fwu-mdata/fwu-mdata-uclass.c > > index b477e9603f..e03773c584 100644 > > --- a/drivers/fwu-mdata/fwu-mdata-uclass.c > > +++ b/drivers/fwu-mdata/fwu-mdata-uclass.c > > @@ -16,6 +16,40 @@ > > #include > > #include > > > > +/** > > + * fwu_read_mdata() - Wrapper around fwu_mdata_ops.read_mdata() > > + * > > + * Return: 0 if OK, -ve on error > > + */ > > Inline description only in header file, or duplicated in source and > header files? > This is the original practice in FWU stack - to have the description in header as well as source code. I just didn't want to stick out. > > diff --git a/include/fwu.h b/include/fwu.h > > index 0919ced812..1a700c9e6a 100644 > > --- a/include/fwu.h > > +++ b/include/fwu.h > > @@ -24,6 +24,26 @@ struct fwu_mdata_gpt_blk_priv { > > * @update_mdata() - Update the FWU metadata copy > > */ > > struct fwu_mdata_ops { > > + /** > > +* read_mdata() - Populate the asked FWU metadata copy > > +* @dev: FWU metadata device > > +* @mdata: Copy of the FWU metadata > > @mdata: Output FWU mdata read > > > +* @primary: If primary or secondary copy of meta-data is to be read > > s/meta-data/FWU metadata/ > Ditto in .write_mdata description > ok > > +/** > > + * fwu_get_verified_mdata() - Read, verify and return the FWU metadata > > + * > > + * Read both the metadata copies from the storage media, verify their > > checksum, > > + * and ascertain that both copies match. If one of the copies has gone bad, > > + * restore it from the good copy. > > @mdata: Output FWU metadata read or NULL > ok > > +static int fwu_sync_mdata(struct fwu_mdata *mdata, int part) > > +{ > > + void *buf = >version; > > + int err; > > + > > + if (part == BOTH_PARTS) { > > + err = fwu_sync_mdata(mdata, SECONDARY_PART); > > + if (err) > > + return err; > > + part = PRIMARY_PART; > > + } > > + > > + /* > > +* Calculate the crc32 for the updated FWU metadata > > +* and put the updated value in the FWU metadata crc32 > > +* field > > +*/ > > + mdata->crc32 = crc32(0, buf, sizeof(*mdata) - sizeof(u32)); > > + > > + err = fwu_write_mdata(g_dev, mdata, part & PRIMARY_PART ? true : > > false); > > Since this expects part is either PRIMARY_PART or SECONDARY_PART, prefer: > err = fwu_write_mdata(g_dev, mdata, part == PRIMARY_PART); > > And ditto below: > part == PRIMARY_PART ? "primary": "secondary"); > yes, now that we handle out the BOTH_PARTS case already. > > +int fwu_get_verified_mdata(struct fwu_mdata *mdata) > > +{ > > + int err; > > + bool parts_ok[2] = { false }; > > + struct fwu_mdata s, *parts_mdata[2]; > > + > > + parts_mdata[0] = _mdata; > > + parts_mdata[1] = > > + > > + /* if mdata already read and ready */ > > + err = mdata_crc_check(parts_mdata[0]); > > + if (!err) > > + goto ret_mdata; > > + /* else read, verify and, if needed, fix mdata */ > > + > > + for (int i = 0; i < 2; i++) { > > Define "int i;" at function block entry? > Hmm... I prefer this way - limiting scope of the scratch variables. thanks.
Re: [PATCH v6 15/22] drivers: use dev_read_addr_index_ptr when cast to pointer
On 3/6/23 19:20, Simon Glass wrote: > Hi Johan, > > On Thu, 2 Mar 2023 at 17:15, Johan Jonker wrote: >> >> The fdt_addr_t and phys_addr_t size have been decoupled. A 32bit CPU >> can expect 64-bit data from the device tree parser, so use > > Why is that? It seems quite inefficient. 1: === Because the device tree does describes more then just only the internal io/mem range. When a NAND chip is connected it must be able to describe partitions far beyond that 32bit range. This change only changes the PARSE capacity defined by fdt_addr_t and fdt_size_t and not the phys_addr_t and phys_size_t. Most drivers make a little mess when taking a DT reg value and then trying to make it fit in a structure of multiple registers with various offsets. Fixing all of that is beyond my capacity/this serie and more a MAINTAINER task. https://elixir.bootlin.com/u-boot/latest/source/drivers/mtd/mtdpart.c#L933 struct mtd_partition { const char *name; /* identifier string */ uint64_t size; /* partition size */ uint64_t offset;/* offset within the master MTD space */ uint32_t mask_flags;/* master MTD flags to mask out for this partition */ struct nand_ecclayout *ecclayout; /* out of band layout for this partition (NAND only) */ }; int add_mtd_partitions_of(struct mtd_info *master) struct mtd_partition part = { 0 }; fdt_addr_t offset; fdt_size_t size; [..] part.offset = offset; part.size = size; While the mtd_partition structure is ready with uint64_t size all the parse functions that export this reg value were typedef to a 32bit value. === Given mk808 rk3066a with NAND: [ 38.750789] nand: Hynix H27UCG8T2ATR-BC 64G 3.3V 8-bit [ 38.756650] nand: 8192 MiB, MLC, erase size: 2048 KiB, page size: 8192, OOB size: 640 === BEFORE: List of MTD devices: * nand0 - type: MLC NAND flash - block size: 0x20 bytes - min I/O: 0x2000 bytes - OOB size: 640 bytes - OOB available: 4294967290 bytes - ECC strength: 40 bits - ECC step size: 1024 bytes - bitflip threshold: 30 bits - 0x-0x0002 : "nand0" - 0x0040-0x0060 : "boot-blk-0" - 0x0060-0x0080 : "boot-blk-1" - 0x0080-0x00a0 : "boot-blk-2" - 0x00a0-0x00c0 : "boot-blk-3" - 0x00c0-0x00e0 : "boot-blk-4" - 0x0100-0xfe00 : "rootfs" - 0xfe00-0x0001 : "bbt" # This output is corrupted. === AFTER: List of MTD devices: * nand0 - type: MLC NAND flash - block size: 0x20 bytes - min I/O: 0x2000 bytes - OOB size: 640 bytes - OOB available: 4294967290 bytes - ECC strength: 40 bits - ECC step size: 1024 bytes - bitflip threshold: 30 bits - 0x-0x0002 : "nand0" - 0x0040-0x0060 : "boot-blk-0" - 0x0060-0x0080 : "boot-blk-1" - 0x0080-0x00a0 : "boot-blk-2" - 0x00a0-0x00c0 : "boot-blk-3" - 0x00c0-0x00e0 : "boot-blk-4" - 0x0100-0x0001fe00 : "rootfs" - 0x0001fe00-0x0002 : "bbt" === Example: partitions { compatible = "fixed-partitions"; #address-cells = <2>; #size-cells = <2>; partition@40 { reg = <0x0 0x0040 0x0 0x0020>; label = "boot-blk-0"; }; partition@60 { reg = <0x0 0x0060 0x0 0x0020>; label = "boot-blk-1"; }; partition@80 { reg = <0x0 0x0080 0x0 0x0020>; label = "boot-blk-2"; }; partition@a0 { reg = <0x0 0x00a0 0x0 0x0020>; label = "boot-blk-3"; }; partition@c0 { reg = <0x0 0x00c0 0x0 0x0020>; label = "boot-blk-4"; }; partition@100 { reg = <0x0 0x0100 0x1 0xfd00>; label = "rootfs"; }; partition@1fe00 { reg = <0x1 0xfe00 0x0 0x0200>; label = "bbt"; }; }; === 2: As a side effect Rockchip rk3288 with 32bit reg should be
Re: Please pull u-boot-marvell/master
On Mon, Mar 06, 2023 at 12:52:52PM +0100, Stefan Roese wrote: > Hi Tom, > > please pull this small Marvell related fix: > Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] arm: mach-k3: introduce generic board detction kconfig option
On Fri, Mar 03, 2023 at 08:16:28PM +0100, Christian Gmeiner wrote: > For non TI boards it is not possible to enable the do_board_detect() > call as TI_I2C_BOARD_DETECT is defined in board/ti/common/Kconfig. > > I want to use do_board_detect() to dectect boards and properties based > on some SPI communication with a FPGA. > > Signed-off-by: Christian Gmeiner Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Re: [PATCH v3 18/19] test/py: android: extend abootimg test
On Mon, Mar 06, 2023 at 08:49:02PM +0100, Safae Ouajih wrote: > > On 27/02/2023 15:18, Tom Rini wrote: > > On Mon, Feb 27, 2023 at 03:15:31PM +0100, Safae Ouajih wrote: > > > On 07/02/2023 20:02, Tom Rini wrote: > > > > On Mon, Feb 06, 2023 at 12:50:20AM +0100, Safae Ouajih wrote: > > > > > > > > > test_abootimg is extended to include the testing of boot images > > > > > version 4. For this, boot.img and vendor_boot.img have been > > > > > generated using mkbootimg tool with setting the header > > > > > version to 4. > > > > > > > > > > This tests: > > > > > - Getting the header version using abootimg > > > > > - Extracting the load address of the dtb > > > > > - Extracting the dtb start address in RAM > > > > > > > > > > Running test: > > > > > $ ./test/py/test.py --bd sandbox --build -k test_abootimg > > > > > > > > > > Signed-off-by: Safae Ouajih > > > > > Reviewed-by: Simon Glass > > > > > --- > > > > >test/py/tests/test_android/test_abootimg.py | 136 > > > > > ++-- > > > > >1 file changed, 123 insertions(+), 13 deletions(-) > > > > Alright, so I don't know where the failure starts, exactly. And to make > > > > testing this easier, there's currently the > > > > trini/u-boot-gitlab-ci-runner:jammy-20230126-07Feb2023 container you can > > > > use to replicate my problem. The problem is that while this test passes > > > > in CI, with GCC, with Clang it fails, consistently: > > > > https://source.denx.de/u-boot/u-boot/-/jobs/572239#L284 > > > > and I'm not quite sure why. I hope that building sandbox with clang and > > > > also just trying these features out interactively will fail too, and so > > > > debugging this will be less of a problem. > > > > > > > Hello Tom, > > > > > > I have investigated this issue, clang has a strange behavior in: > > > > > > * abootimg_get_dtb_load_addr() : cmd/abootimg.c > > > * android_image_get_dtb_by_index() : boot/image-android.c > > > > > > That is probably linked to some sort of optimization clang does. > > > > > > However, The fail is not reproducible using clang-15 and clang-16 and also > > > not reproducible when turning off clang optimizations. > > > > > > I suggest using clang-15 to run the test or I can remove all optimizations > > > > > > on the related functions if clang-14 is used. > > Thanks for investigating. I see that 15 is now considered stable, so > > I'll update the next branch for that, then re-take this series. > > > > Hello Tom, > > Thank you. > > I am a bit confused, do you mean that you will apply this series after > clang-15 update? Yes, and the series to move CI to clang-15 is currently stuck on some pytest (real) failures that need to be resolved. But I would expect this to all be resolved in time for this series here to be included in v2023.07. -- Tom signature.asc Description: PGP signature
Re: [PATCH v3 18/19] test/py: android: extend abootimg test
On 27/02/2023 15:18, Tom Rini wrote: On Mon, Feb 27, 2023 at 03:15:31PM +0100, Safae Ouajih wrote: On 07/02/2023 20:02, Tom Rini wrote: On Mon, Feb 06, 2023 at 12:50:20AM +0100, Safae Ouajih wrote: test_abootimg is extended to include the testing of boot images version 4. For this, boot.img and vendor_boot.img have been generated using mkbootimg tool with setting the header version to 4. This tests: - Getting the header version using abootimg - Extracting the load address of the dtb - Extracting the dtb start address in RAM Running test: $ ./test/py/test.py --bd sandbox --build -k test_abootimg Signed-off-by: Safae Ouajih Reviewed-by: Simon Glass --- test/py/tests/test_android/test_abootimg.py | 136 ++-- 1 file changed, 123 insertions(+), 13 deletions(-) Alright, so I don't know where the failure starts, exactly. And to make testing this easier, there's currently the trini/u-boot-gitlab-ci-runner:jammy-20230126-07Feb2023 container you can use to replicate my problem. The problem is that while this test passes in CI, with GCC, with Clang it fails, consistently: https://source.denx.de/u-boot/u-boot/-/jobs/572239#L284 and I'm not quite sure why. I hope that building sandbox with clang and also just trying these features out interactively will fail too, and so debugging this will be less of a problem. Hello Tom, I have investigated this issue, clang has a strange behavior in: * abootimg_get_dtb_load_addr() : cmd/abootimg.c * android_image_get_dtb_by_index() : boot/image-android.c That is probably linked to some sort of optimization clang does. However, The fail is not reproducible using clang-15 and clang-16 and also not reproducible when turning off clang optimizations. I suggest using clang-15 to run the test or I can remove all optimizations on the related functions if clang-14 is used. Thanks for investigating. I see that 15 is now considered stable, so I'll update the next branch for that, then re-take this series. Hello Tom, Thank you. I am a bit confused, do you mean that you will apply this series after clang-15 update? Best regards, Safae
Re: [PATCH] cmd: fdt: Drop the 0x prefix
On 3/6/23 18:53, Simon Glass wrote: Hi Marek, On Wed, 1 Mar 2023 at 20:04, Marek Vasut wrote: The 'fdt get addr' is always assumed to be hex value, drop the prefix. Since this might break existing users who depend on the existing behavior with 0x prefix, this is a separate patch. Revert if this breaks anything. Signed-off-by: Marek Vasut --- Cc: Heinrich Schuchardt Cc: Simon Glass Cc: Tom Rini --- cmd/fdt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/fdt.c b/cmd/fdt.c index f38fe909c3e..04b664e652c 100644 --- a/cmd/fdt.c +++ b/cmd/fdt.c @@ -478,7 +478,7 @@ static int do_fdt(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) /* Get address */ char buf[19]; - snprintf(buf, sizeof(buf), "0x%lx", + snprintf(buf, sizeof(buf), "%lx", (ulong)map_to_sysmem(nodep)); env_set(var, buf); } else if (subcmd[0] == 's') { -- 2.39.2 iwc how about using env_sethex() ? The 'env get size' 's' case below could likely use similar treatment , do I read it right ?
Re: [PATCH v4 3/4] Kconfig: j721e: Change K3_MCU_SCRATCHPAD_BASE to non firewalled region
On Mon, Mar 06, 2023 at 11:12:53AM +0530, Manorit Chawdhry wrote: > In non-combined boot flow for K3, all the firewalls are locked by default > until sysfw comes up. Rom configures some of the firewall for its usage > along with the SRAM for R5 but the PSRAM region is still locked. > > The K3 MCU Scratchpad for j721e was set to a PSRAM region triggering the > firewall exception before sysfw came up. The exception started happening > after adding multi dtb support that accesses the scratchpad for reading > EEPROM contents. > > The commit changes R5 MCU scratchpad for j721e to an SRAM region. > > Old Map: > ┌─┐ 0x41c0 > │ SPL │ > ├─┤ 0x41c4 (approx) > │STACK│ > ├─┤ 0x41c85b20 > │ Global data │ > │ sizeof(struct global_data) = 0xd8 │ > ├─┤ gd->malloc_base = 0x41c85bfc > │HEAP │ > │ CONFIG_SYS_MALLOC_F_LEN = 0x7 │ > ├─┤ CONFIG_SPL_BSS_START_ADDR > │ SPL BSS │ (0x41cf5bfc) > │ CONFIG_SPL_BSS_MAX_SIZE = 0xA000 │ > └─┘ CONFIG_SYS_K3_BOOT_PARAM_TABLE_INDEX > (0x41cffbfc) > > New Map: > ┌─┐ 0x41c0 > │ SPL │ > ├─┤ 0x41c4 (approx) > │EMPTY│ > ├─┤ 0x41c81920 > │STACK│ > │ SPL_SIZE_LIMIT_PROVIDE_STACK=0x4000 │ > ├─┤ 0x41c85920 > │ Global data │ > │ sizeof(struct global_data) = 0xd8 │ > ├─┤ gd->malloc_base = 0x41c859f0 > │HEAP │ > │ CONFIG_SYS_MALLOC_F_LEN = 0x7 │ > ├─┤ CONFIG_SPL_BSS_START_ADDR > │ SPL BSS │ (0x41cf59f0) > │ CONFIG_SPL_BSS_MAX_SIZE = 0xA000 │ > ├─┤ 0x41cff9fc > │ NEW MCU SCRATCHPAD │ > │ SYS_K3_MCU_SCRATCHPAD_SIZE = 0x200 │ > └─┘ CONFIG_SYS_K3_BOOT_PARAM_TABLE_INDEX > (0x41cffbfc) > > Fixes: ab977c8b91b4 ("configs: j721s2_evm_r5: Enable support for building > multiple dtbs into FIT") > > Signed-off-by: Manorit Chawdhry > [n-fran...@ti.com: SRAM allocation addressing diagram] > Signed-off-by: Neha Francis > Reviewed-by: Tom Rini > Reviewed-by: Kamlesh Gurudasani > --- > arch/arm/mach-k3/Kconfig | 3 ++- > configs/j721e_evm_r5_defconfig | 10 -- > doc/board/ti/j721e_evm.rst | 27 +++ OK, but now this just renders differently poorly. Please see the list-table directive as used for example in doc/board/apple/m1.rst and it would be good to get other ascii tables updated to produce nice output as well. -- Tom signature.asc Description: PGP signature
Re: [PATCH v5 00/44] More tidy-ups of Kconfig options
Hi Tom, On Fri, 3 Mar 2023 at 16:43, Tom Rini wrote: > > On Wed, Feb 22, 2023 at 09:33:41AM -0700, Simon Glass wrote: > > > This series was split out of the old 'split config' splc series. It > > contains clean-up patches which do not depend on split config. > > > > This is available at u-boot-dm/spld-working > > > > The size changes look pretty good: https://paste.debian.net/1271742/ > > > > The remaining patches will move into a new 'splg' series (G for Good). > > > > [1] https://patchwork.ozlabs.org/project/uboot/list/?series=341504=* > > > > Changes in v5: > > - Fix reply typo > > - Change approach and expand notes after more investigation > > - Drop FSL_ISBC_KEY_EXT patch as it changes the size > > - Drop PHY_CADENCE_SIERRA patch as it changes the size > > > > Changes in v4: > > - Avoid use of def_bool > > - Modify to get rid of def_bool > > - Adjust Kconfig ordering > > - Just fix the typo > > - Reduce and rename commit > > - Reduce and rename commit > > - Fix 'wanderboard' typo > > - Reduce and rename commit > > > > Changes in v3: > > - Add new patch to disable QFW bootmeth in SPL > > - Move the option down to the non-SPL part of drivers/Makefile > > - Correct 'VPL' typo > > - Use a consistent format for the comment > > - Fix a transitory build error with sandbox_spl > > - Add a new patch to disallow commands in SPL > > > > Changes in v2: > > - Rebase to previous series > > > > Simon Glass (44): > > mtd: Drop unused kb9202_nand driver > > mtd: Drop unused CONFIG_ONENAND_U_BOOT > > sh4: Drop unused twl6030 driver > > moveconfig: Update to detect / correct missing SPL Kconfigs > > bootstd: Disable QFW bootmeth in SPL > > Correct SPL uses of ARCH_MVEBU > > Correct SPL uses of DISPLAY_AER_FULL > > Correct SPL uses of MULTIPLEXER > > Correct SPL use of PG_WCOM_UBOOT_UPDATE_SUPPORTED > > Correct SPL uses of PHY_FIXED > > boot: Add Kconfigs for BOOTMETH_VBE_REQUEST > > Correct SPL use of DM_RNG > > lib: Add a Kconfig for SPL_BZIP2 > > moveconfig: Various minor improvements > > sandbox: Expand size for VPL image > > event: Add Kconfig options for SPL > > bootstd: Correct 'VPL' typo > > env: Avoid checking ENV_IS_IN when env disabled > > env: Allow VPL environment to be nowhere > > lib: Add VPL options for SHA1 and SHA256 > > x86: Use string functions for all 32-bit builds > > lib: Fix build condition for tiny-printf > > sandbox: Tidy up RTC options > > sandbox: Use the generic VPL option to enable VPL > > sandbox: Tidy up I2C options > > fixdep: Add support for VPL > > fixdep: Refactor to make testing easier > > fixdep: Add some tests for parse_config_line() > > test: Add SPL versions of the TEST_KCONFIG options > > lib: Add an SPL config for LIB_UUID > > test: Tidy up sandbox handling in test-main > > x86: Fix up use of X86_32BIT_INIT and X86_64 options > > Add VPL options for BLOBLIST > > rockchip: Avoid checking environment without ENV_SUPPORT > > freescale: Drop old pre-DM_ETH code > > imx: Use SATA instead of CMD_SATA > > net: Add an SPL config for atheros > > freescale: Fix odd use of ESDHCI_QUIRK_BROKEN_TIMEOUT_VALUE > > serial: Support ns16550 driver in TPL > > dm: Add a TPL symbol for simple-bus > > x86: coral: Add missing TPL options > > power: wandboard: Add a missing CONFIG > > venice: Simplify conditions for network init > > command: Don't allow commands in SPL > > I've applied almost all of this series. I've left the moveconfig.py > stuff for now as I'm not sure it's useful until split config. I've > followed up about the X86_32BIT_INIT patch as I do want to see that > boot-tested, and I've left the ns16650 TPL patch as both that feel like > "split config makes this an issue" rather than an issue we have today > and fwiw, the dependencies are wrong in that there's no TPL for omap2 > which I didn't reply directly to, I only noticed as reviewing the series > locally before merging. OK, thanks for that. Do you think it is worth picking up the fixdep ones also? The VPL thing is a bug and the others are for tests. I will see if I can get some time by next week to do another spin. How can we get more eyes on the Kconfig-language proposal? Regards, Simon
Re: [PATCH RFC u-boot-mvebu 0/2] arm: mvebu: Fix eMMC boot
On Monday 06 March 2023 11:15:35 Martin Rowe wrote: > On Sun, 5 Mar 2023 at 16:04, Pali Rohár wrote: > > Could you try another test by completely erasing BOOT0, BOOT1 and USER > > > data? And see what BootROM prints. > > > > = > BootROM - 1.73 > > Booting from MMC > BootROM: Bad header at offset > BootROM: Bad header at offset 0020 > Switching BootPartitions. > BootROM: Bad header at offset > BootROM: Bad header at offset 0020 > Switching BootPartitions. > BootROM: Bad header at offset > BootROM: Bad header at offset 0020 > Switching BootPartitions. > BootROM: Bad header at offset > BootROM: Bad header at offset 0020 > Switching BootPartitions. > BootROM: Bad header at offset > BootROM: Bad header at offset 0020 > Switching BootPartitions. > BootROM: Bad header at offset > BootROM: Bad header at offset 0020 > Switching BootPartitions. > BootROM: Bad header at offset > BootROM: Bad header at offset 0020 > Switching BootPartitions. > BootROM: Bad header at offset > BootROM: Bad header at offset 0020 > Switching BootPartitions. > BootROM: Bad header at offset > BootROM: Bad header at offset 0020 > Switching BootPartitions. > BootROM: Bad header at offset > BootROM: Bad h > Trying Uart > = Originally I though that I did not understand disassembled bootrom code correctly but this logs proves that I did it correctly. Log is very strange. There is a loop which tries partition numbers 0x1, 0x2, ... 0x9, 0xa. And if I'm looking at the bootrom code correctly it does bit-AND operation on partition number with constant 0x7 and result is set into mmc register 179 (partition_config). So if I understand it correctly it means that bootrom automatically clears boot_ack and boot_partition. And into partition_access it sets the partition number. Hence numbers 0x9 and 0xa are trimmed and aliased to 0x1 and 0x2; and number 0x8 overflows to 0x0. Completely strange behavior and probably against how HW mmc boot partitions should be used. eMMC spec defines: partition_access (low 3 bits of mmc 179/partition_config register): 0x0 : No access to boot partition (default) 0x1 : R/W boot partition 1 0x2 : R/W boot partition 2 0x3 : R/W Replay Protected Memory Block (RPMB) 0x4 : Access to General Purpose partition 1 0x5 : Access to General Purpose partition 2 0x6 : Access to General Purpose partition 3 0x7 : Access to General Purpose partition 4 I guess that you do not have general purpose partitions layout on emmc and RPMB is not used too. So technically only 0x0, 0x1, and 0x2 are available. To confirm my theory, could you try to do following tests? 1. Check u-boot's "mmc partconf" settings are not preserved across reboots. 2. Put valid image into userdata partition; erase boot 0 and boot 1; and post bootrom output. There should be 7 invalid attempts with Switching BootPartitions message. 3. Take valid image, invalidate kwb header checksum and put it on boot0; plus erase boot1 and user. Bootrom should print "Invalid header checksum" message and it should be two times. Once for 0x1 and second time for overflowed 0x9.
Re: [PATCH v6 13/22] core: read: add dev_read_addr_index_ptr function
On Thu, 2 Mar 2023 at 17:15, Johan Jonker wrote: > > Add dev_read_addr_index_ptr function with the > same functionality as dev_read_addr_index, > but instead a return pointer is given. > Use map_sysmem() function as cast for the return. > Make same fix for dev_read_addr_ptr() function. > > Signed-off-by: Johan Jonker > --- > > Changed V6: > use map_sysmem() > > Changed V5: > new patch > --- > drivers/core/read.c | 15 ++- > 1 file changed, 14 insertions(+), 1 deletion(-) Reviewed-by: Simon Glass
Re: [PATCH v6 15/22] drivers: use dev_read_addr_index_ptr when cast to pointer
Hi Johan, On Thu, 2 Mar 2023 at 17:15, Johan Jonker wrote: > > The fdt_addr_t and phys_addr_t size have been decoupled. A 32bit CPU > can expect 64-bit data from the device tree parser, so use Why is that? It seems quite inefficient. > dev_read_addr_index_ptr instead of the dev_read_addr_index function > in the various files in the drivers directory that cast to a pointer. > > Signed-off-by: Johan Jonker > Reviewed-by: Michael Trimarchi > --- > > Changed V6: > use -EINVAL on return > drop cast > --- > drivers/mtd/nand/raw/cortina_nand.c | 4 ++-- > drivers/net/dm9000x.c | 2 +- > drivers/net/dwmac_meson8b.c | 4 ++-- > drivers/pci/pcie_dw_meson.c | 8 > drivers/pci/pcie_dw_rockchip.c | 8 > drivers/watchdog/sbsa_gwdt.c| 12 ++-- > 6 files changed, 19 insertions(+), 19 deletions(-) [..] Regards, SImon
Re: [PATCH v5 32/44] x86: Fix up use of X86_32BIT_INIT and X86_64 options
Hi Tom, On Fri, 3 Mar 2023 at 07:50, Tom Rini wrote: > > On Wed, Feb 22, 2023 at 09:34:13AM -0700, Simon Glass wrote: > > > Drop the invalid SPL_ in a CONFIG_IS_ENABLED() usage. Use the correct > > X86_64 option in msr.h since SPL may be 32-bit when U-Bout proper is > > not. > > > > Signed-off-by: Simon Glass > > --- > > > > (no changes since v4) > > > > Changes in v4: > > - Reduce and rename commit > > > > arch/x86/cpu/qemu/qemu.c | 2 +- > > arch/x86/include/asm/msr.h | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > This changes spl on chromebook_link64 a lot, have you confirmed it > there? No, the lab is broken...will do at some point. Regards, Simon
Re: [PATCH V7 09/15] iot2050: Add script for signing artifacts
Hi Jan, On Tue, 28 Feb 2023 at 11:21, Jan Kiszka wrote: > > From: Jan Kiszka > > There are many ways to get a signed firmware for the IOT2050 devices, > namely for the parts under user-control. This script documents one way > of doing it, given a signing key. Augment the board documentation with > the required procedure around it. > > Signed-off-by: Jan Kiszka > --- > doc/board/siemens/iot2050.rst | 52 +++ > tools/iot2050-sign-fw.sh | 51 ++ > 2 files changed, 103 insertions(+) > create mode 100755 tools/iot2050-sign-fw.sh I sent a series which: - attempts to do this with binman (providing x509 support) - allows use of 'binman replace' in your script, by enhancing support for updating sections Please take a look at see what you think. Regards, SImon
Re: [PATCH V7 04/15] iot2050: Migrate settings into board env file
Hi Jan, On Wed, 1 Mar 2023 at 23:38, Jan Kiszka wrote: > > On 02.03.23 00:38, Simon Glass wrote: > > Hi Jan, > > > > On Tue, 28 Feb 2023 at 11:20, Jan Kiszka wrote: > >> > >> From: Jan Kiszka > >> > >> Anything that is not boot-env related is better kept there by now. > >> > >> At this chance, also drop a stale comment from iot2050.h > >> > >> Signed-off-by: Jan Kiszka > >> --- > >> board/siemens/iot2050/iot2050.env | 9 + > >> include/configs/iot2050.h | 11 ++- > >> 2 files changed, 11 insertions(+), 9 deletions(-) > >> create mode 100644 board/siemens/iot2050/iot2050.env > >> > >> diff --git a/board/siemens/iot2050/iot2050.env > >> b/board/siemens/iot2050/iot2050.env > >> new file mode 100644 > >> index 000..4bd93f0b2f4 > >> --- /dev/null > >> +++ b/board/siemens/iot2050/iot2050.env > >> @@ -0,0 +1,9 @@ > >> +// SPDX-License-Identifier: GPL-2.0+ > >> +/* > >> + * Copyright (c) Siemens AG, 2023 > >> + * > >> + * Authors: > >> + * Jan Kiszka > >> + */ > >> + > >> +usb_pgood_delay=900 > >> diff --git a/include/configs/iot2050.h b/include/configs/iot2050.h > >> index cfff46ce339..8dfeaddf541 100644 > >> --- a/include/configs/iot2050.h > >> +++ b/include/configs/iot2050.h > >> @@ -13,12 +13,6 @@ > >> > >> #include > >> > >> -/* SPL Loader Configuration */ > >> - > >> -/* U-Boot general configuration */ > >> -#define EXTRA_ENV_IOT2050_BOARD_SETTINGS \ > >> - "usb_pgood_delay=900\0" > >> - > >> #if IS_ENABLED(CONFIG_CMD_USB) > >> # define BOOT_TARGET_USB(func) \ > >> func(USB, usb, 0) \ > >> @@ -40,10 +34,9 @@ > >> > >> #include > >> > >> -#define CFG_EXTRA_ENV_SETTINGS \ > >> +#define CFG_EXTRA_ENV_SETTINGS \ > >> DEFAULT_LINUX_BOOT_ENV \ > >> - BOOTENV \ > >> - EXTRA_ENV_IOT2050_BOARD_SETTINGS > >> + BOOTENV > >> > >> #include > >> > >> -- > >> 2.35.3 > >> > > > > You might want to move to standard boot so you can use a text-based > > environment. See for example [1] [2] and later patches from [3]. > > > > Err, this patch is about introducing a text-based env for the parts that > can be moved. I don't see a relevant delta after this patch to the > referenced examples (btw, [2] is missing). Sorry, yes. But if you move to standard boot then you don't need BOOTENV [2] is https://patchwork.ozlabs.org/project/uboot/list/?series=344332=* > > Jan > > > Regards, > > Simon > > > > [1] https://patchwork.ozlabs.org/project/uboot/list/?series=342718 > > [2] > > [3] from > > https://patchwork.ozlabs.org/project/uboot/list/?series=338993=* > > -- > Siemens AG, Technology > Competence Center Embedded Linux > Regards, SImon
Re: [PATCH v6 17/22] drivers: use devfdt_get_addr_size_index_ptr when cast to pointer
On Thu, 2 Mar 2023 at 17:16, Johan Jonker wrote: > > The fdt_addr_t and phys_addr_t size have been decoupled. A 32bit CPU > can expect 64-bit data from the device tree parser, so use > devfdt_get_addr_size_index_ptr instead of the devfdt_get_addr_size_index > function in the various files in the drivers directory that cast to > a pointer. > > Signed-off-by: Johan Jonker > Reviewed-by: Michael Trimarchi > --- > drivers/pci/pcie_dw_mvebu.c | 6 +++--- > drivers/spi/cadence_qspi.c | 3 +-- > 2 files changed, 4 insertions(+), 5 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH] cmd: fdt: Drop the 0x prefix
Hi Marek, On Wed, 1 Mar 2023 at 20:04, Marek Vasut wrote: > > The 'fdt get addr' is always assumed to be hex value, drop the prefix. > Since this might break existing users who depend on the existing > behavior with 0x prefix, this is a separate patch. Revert if this > breaks anything. > > Signed-off-by: Marek Vasut > --- > Cc: Heinrich Schuchardt > Cc: Simon Glass > Cc: Tom Rini > --- > cmd/fdt.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/cmd/fdt.c b/cmd/fdt.c > index f38fe909c3e..04b664e652c 100644 > --- a/cmd/fdt.c > +++ b/cmd/fdt.c > @@ -478,7 +478,7 @@ static int do_fdt(struct cmd_tbl *cmdtp, int flag, int > argc, char *const argv[]) > /* Get address */ > char buf[19]; > > - snprintf(buf, sizeof(buf), "0x%lx", > + snprintf(buf, sizeof(buf), "%lx", > (ulong)map_to_sysmem(nodep)); > env_set(var, buf); > } else if (subcmd[0] == 's') { > -- > 2.39.2 > iwc how about using env_sethex() ? Regards, Simon
Re: [PATCH 2/2] test: cmd: fdt: Drop new unneeded curly brackets
On Wed, 1 Mar 2023 at 20:05, Marek Vasut wrote: > > Drop no longer needed { } around ut_assert*() functions in FDT test. > No functional change. > > Signed-off-by: Marek Vasut > --- > Cc: Heinrich Schuchardt > Cc: Simon Glass > Cc: Tom Rini > --- > test/cmd/fdt.c | 14 ++ > 1 file changed, 6 insertions(+), 8 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH 1/1] doc: man-page for panic command
On Fri, 3 Mar 2023 at 14:51, Heinrich Schuchardt wrote: > > Provide a man-page for the panic command. > > Signed-off-by: Heinrich Schuchardt > --- > doc/usage/cmd/panic.rst | 33 + > doc/usage/index.rst | 1 + > 2 files changed, 34 insertions(+) > create mode 100644 doc/usage/cmd/panic.rst Reviewed-by: Simon Glass
Re: [PATCH] nvedit: simplify do_env_indirect()
On Mon, 6 Mar 2023 at 06:27, Rasmus Villemoes wrote: > > Instead of calling env_get(from) up to three times, just do it once, > computing the value we will put into 'to' and error out if that is > NULL (i.e. no 'from' variable and no default provided). > > No functional change. > > Signed-off-by: Rasmus Villemoes > --- > cmd/nvedit.c | 11 --- > 1 file changed, 4 insertions(+), 7 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH v6 12/22] core: fdtaddr: add devfdt_get_addr_size_index_ptr function
Hi Johan, On Thu, 2 Mar 2023 at 17:15, Johan Jonker wrote: > > Add devfdt_get_addr_size_index_ptr function with the same > functionality as devfdt_get_addr_size_index, but instead > a return pointer is given. > > Suggested-by: Michael Nazzareno Trimarchi > Signed-off-by: Johan Jonker > Reviewed-by: Michael Trimarchi > --- > > Changed V5: > fix spelling > use tabs > --- > drivers/core/fdtaddr.c | 8 > include/dm/fdtaddr.h | 17 - > 2 files changed, 24 insertions(+), 1 deletion(-) > > diff --git a/drivers/core/fdtaddr.c b/drivers/core/fdtaddr.c > index 91bcd1a2..f5906ff9 100644 > --- a/drivers/core/fdtaddr.c > +++ b/drivers/core/fdtaddr.c > @@ -122,6 +122,14 @@ fdt_addr_t devfdt_get_addr_size_index(const struct > udevice *dev, int index, > #endif > } > > +void *devfdt_get_addr_size_index_ptr(const struct udevice *dev, int index, > +fdt_size_t *size) > +{ > + fdt_addr_t addr = devfdt_get_addr_size_index(dev, index, size); > + > + return (addr == FDT_ADDR_T_NONE) ? NULL : (void *)(uintptr_t)addr; Shouldn't this use map_to_sysmem()? We should not cast addresses to pointers. [..] Regards, SImon
Re: [PATCH 1/2] test: Wrap assert macros in ({ ... }) and fix missing semicolons
On Wed, 1 Mar 2023 at 20:05, Marek Vasut wrote: > > Wrap the assert macros in ({ ... }) so they can be safely used both as > right side argument as well as in conditionals without curly brackets > around them. In the process, find a bunch of missing semicolons, fix > them. > > Signed-off-by: Marek Vasut > --- > Cc: Heinrich Schuchardt > Cc: Simon Glass > Cc: Tom Rini > --- > include/test/ut.h | 152 ++--- > test/cmd/pwm.c | 4 +- > test/dm/acpigen.c | 2 +- > test/dm/misc.c | 4 +- > test/dm/phy.c | 8 +-- > test/dm/scmi.c | 4 +- > test/lib/kconfig.c | 10 +-- > test/unicode_ut.c | 6 +- > 8 files changed, 121 insertions(+), 69 deletions(-) > Gosh. That was pretty bad. Thanks for fixing. Reviewed-by: Simon Glass
Re: [PATCH v2 02/32] cmd: fdt: Fix handling of empty properties for fdt get addr and fdt get size
Hi Marek, On Wed, 1 Mar 2023 at 20:09, Marek Vasut wrote: > > It is perfectly valid to request an address or size of FDT property > without value, the only special case if requesting of the value of > FDT property without value. Invert the test such, that properties > without value still set the variable from 'fdt get addr/size' to > address of the property or size of the property, where the later > is 0. > > Signed-off-by: Marek Vasut > Reviewed-by: Simon Glass > --- > Cc: Heinrich Schuchardt > Cc: Simon Glass > Cc: Tom Rini > --- > V2: Add RB from Simon BTW 'patman status' can collect review tags and add them to your commits automatically. Regards, Simon
Re: [PATCH 23/32] test: cmd: fdt: Test fdt set
On Wed, 1 Mar 2023 at 20:07, Marek Vasut wrote: > > On 3/1/23 16:02, Simon Glass wrote: > > Hi Marek, > > > > On Mon, 27 Feb 2023 at 12:55, Marek Vasut > > wrote: > >> > >> Add 'fdt set' test which works as follows: > >> - Create fuller FDT, map it to sysmem > >> - Set either existing property to overwrite it, or new property > >> - Test setting both single properties as well as string and integer arrays > >> - Test setting to non-existent nodes and aliases > >> - Verify set values using 'fdt get value' > >> > >> The test case can be triggered using: > >> " > >> ./u-boot -Dc 'ut fdt' > >> " > >> To dump the full output from commands used during test, add '-v' flag. > >> > >> Signed-off-by: Marek Vasut > >> --- > >> Cc: Heinrich Schuchardt > >> Cc: Simon Glass > >> Cc: Tom Rini > >> --- > >> test/cmd/fdt.c | 123 + > >> 1 file changed, 123 insertions(+) > >> > >> diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c > >> index ae67b468b71..42d067090aa 100644 > >> --- a/test/cmd/fdt.c > >> +++ b/test/cmd/fdt.c > >> @@ -777,6 +777,129 @@ static int fdt_test_get_size(struct unit_test_state > >> *uts) > >> } > >> FDT_TEST(fdt_test_get_size, UT_TESTF_CONSOLE_REC); > >> > >> +static int fdt_test_set_single(struct unit_test_state *uts, > >> + const char *path, const char *prop, > >> + const char *sval, int ival, bool integer) > > > > Please add a comment for this function. > > > >> +{ > >> + ut_assertok(console_record_reset_enable()); > >> + if (sval) { > >> + ut_assertok(run_commandf("fdt set %s %s %s", path, prop, > >> sval)); > >> + } else if (integer) { > >> + ut_assertok(run_commandf("fdt set %s %s <%d>", path, prop, > >> ival)); > >> + } else { > >> + ut_assertok(run_commandf("fdt set %s %s", path, prop)); > >> + } > > > > Should drop {} on single-line statements - please check patman > > This one isn't as simple as "drop the {}" in fact, I sent a separate > series to address that: > > https://patchwork.ozlabs.org/project/uboot/list/?series=344329 Oh yes, I hit that a while back. Reviewed-by: Simon Glass
Re: [PATCH v3 3/5] doc: document read/write commands
On Thu, 2 Mar 2023 at 01:12, Rasmus Villemoes wrote: > > The read and write commands are, deliberately, implemented in the same > file, so that they stay feature-compatible (e.g. if someone implements > support for "read the full partition, however large that is", that > same syntax should also work for write). In order to ensure the > documentation for both are similarly kept in sync, and to avoid > duplication, document them both in read.rst, and add a stub write.rst > referring to read.rst. > > Signed-off-by: Rasmus Villemoes > --- > doc/usage/cmd/read.rst | 44 + > doc/usage/cmd/write.rst | 6 ++ > doc/usage/index.rst | 2 ++ > 3 files changed, 52 insertions(+) > create mode 100644 doc/usage/cmd/read.rst > create mode 100644 doc/usage/cmd/write.rst > Reviewed-by: Simon Glass
[PATCH v4 12/14] arm64: dts: imx8mp: Drop EQoS clock workaround
The assigned-clock no longer have to be dropped, the clock are now defined in clk-imx8mp.c and used by DWMAC driver to configure the DWMAC clock. Drop the workarounds from U-Boot specific DT extras. Signed-off-by: Marek Vasut --- Cc: "Ariel D'Alessandro" Cc: "NXP i.MX U-Boot Team" Cc: Andrey Zhizhikin Cc: Fabio Estevam Cc: Joe Hershberger Cc: Lukasz Majewski Cc: Marcel Ziswiler Cc: Marek Vasut Cc: Michael Trimarchi Cc: Peng Fan Cc: Ramon Fried Cc: Sean Anderson Cc: Stefano Babic Cc: Tim Harvey Cc: Tommaso Merciai Cc: u-boot@lists.denx.de --- V2: No change V3: No change V4: No change --- arch/arm/dts/imx8mp-dhcom-u-boot.dtsi| 6 -- arch/arm/dts/imx8mp-evk-u-boot.dtsi | 6 -- arch/arm/dts/imx8mp-icore-mx8mp-edimm2.2-u-boot.dtsi | 6 -- arch/arm/dts/imx8mp-venice-gw74xx-u-boot.dtsi| 6 -- arch/arm/dts/imx8mp-verdin-wifi-dev-u-boot.dtsi | 6 -- 5 files changed, 30 deletions(-) diff --git a/arch/arm/dts/imx8mp-dhcom-u-boot.dtsi b/arch/arm/dts/imx8mp-dhcom-u-boot.dtsi index ae838caebcf..ea6ab9f2e17 100644 --- a/arch/arm/dts/imx8mp-dhcom-u-boot.dtsi +++ b/arch/arm/dts/imx8mp-dhcom-u-boot.dtsi @@ -33,12 +33,6 @@ u-boot,dm-spl; }; - { - /delete-property/ assigned-clocks; - /delete-property/ assigned-clock-parents; - /delete-property/ assigned-clock-rates; -}; - { u-boot,dm-spl; }; diff --git a/arch/arm/dts/imx8mp-evk-u-boot.dtsi b/arch/arm/dts/imx8mp-evk-u-boot.dtsi index f43eb6238d0..cd0fb815c79 100644 --- a/arch/arm/dts/imx8mp-evk-u-boot.dtsi +++ b/arch/arm/dts/imx8mp-evk-u-boot.dtsi @@ -131,12 +131,6 @@ u-boot,dm-spl; }; - { - /delete-property/ assigned-clocks; - /delete-property/ assigned-clock-parents; - /delete-property/ assigned-clock-rates; -}; - { reset-gpios = < 22 GPIO_ACTIVE_LOW>; reset-delay-us = <15000>; diff --git a/arch/arm/dts/imx8mp-icore-mx8mp-edimm2.2-u-boot.dtsi b/arch/arm/dts/imx8mp-icore-mx8mp-edimm2.2-u-boot.dtsi index 342c523b0c5..3e48cf8ec5c 100644 --- a/arch/arm/dts/imx8mp-icore-mx8mp-edimm2.2-u-boot.dtsi +++ b/arch/arm/dts/imx8mp-icore-mx8mp-edimm2.2-u-boot.dtsi @@ -130,12 +130,6 @@ u-boot,dm-spl; }; - { - /delete-property/ assigned-clocks; - /delete-property/ assigned-clock-parents; - /delete-property/ assigned-clock-rates; -}; - { reset-gpios = < 22 GPIO_ACTIVE_LOW>; reset-delay-us = <15000>; diff --git a/arch/arm/dts/imx8mp-venice-gw74xx-u-boot.dtsi b/arch/arm/dts/imx8mp-venice-gw74xx-u-boot.dtsi index d8721124526..849950fe026 100644 --- a/arch/arm/dts/imx8mp-venice-gw74xx-u-boot.dtsi +++ b/arch/arm/dts/imx8mp-venice-gw74xx-u-boot.dtsi @@ -20,12 +20,6 @@ }; }; - { - /delete-property/ assigned-clocks; - /delete-property/ assigned-clock-parents; - /delete-property/ assigned-clock-rates; -}; - { reset-gpios = < 30 GPIO_ACTIVE_LOW>; reset-delay-us = <1000>; diff --git a/arch/arm/dts/imx8mp-verdin-wifi-dev-u-boot.dtsi b/arch/arm/dts/imx8mp-verdin-wifi-dev-u-boot.dtsi index 8a4cdc717d2..5f021d17230 100644 --- a/arch/arm/dts/imx8mp-verdin-wifi-dev-u-boot.dtsi +++ b/arch/arm/dts/imx8mp-verdin-wifi-dev-u-boot.dtsi @@ -39,12 +39,6 @@ u-boot,dm-spl; }; - { - /delete-property/ assigned-clocks; - /delete-property/ assigned-clock-parents; - /delete-property/ assigned-clock-rates; -}; - { u-boot,dm-spl; }; -- 2.39.2
[PATCH v4 14/14] arm64: imx8mm: imx8mn: imx8mp: Drop FEC GPR[1] board workaround
The FEC interface mode is now configured in common board_interface_eth_init() and called by FEC MAC driver when appropriate. Drop the board side duplicates if the same functionality. Signed-off-by: Marek Vasut --- Cc: "Ariel D'Alessandro" Cc: "NXP i.MX U-Boot Team" Cc: Andrey Zhizhikin Cc: Fabio Estevam Cc: Joe Hershberger Cc: Lukasz Majewski Cc: Marcel Ziswiler Cc: Marek Vasut Cc: Michael Trimarchi Cc: Peng Fan Cc: Ramon Fried Cc: Sean Anderson Cc: Stefano Babic Cc: Tim Harvey Cc: Tommaso Merciai Cc: u-boot@lists.denx.de --- V3: New patch V4: Drop ifdef MX8MP around imx8mp_fec_interface_init --- arch/arm/mach-imx/imx8m/clock_imx8mm.c| 47 --- .../dh_imx8mp/imx8mp_dhcom_pdk2.c | 12 - board/engicam/imx8mm/icore_mx8mm.c| 15 +- board/kontron/pitx_imx8m/pitx_imx8m.c | 14 +- 4 files changed, 2 insertions(+), 86 deletions(-) diff --git a/arch/arm/mach-imx/imx8m/clock_imx8mm.c b/arch/arm/mach-imx/imx8m/clock_imx8mm.c index e26658a08de..c380d9d2950 100644 --- a/arch/arm/mach-imx/imx8m/clock_imx8mm.c +++ b/arch/arm/mach-imx/imx8m/clock_imx8mm.c @@ -875,53 +875,6 @@ static int imx8mp_eqos_interface_init(struct udevice *dev, #endif #ifdef CONFIG_FEC_MXC -int set_clk_enet(enum enet_freq type) -{ - u32 target; - u32 enet1_ref; - - switch (type) { - case ENET_125MHZ: - enet1_ref = ENET1_REF_CLK_ROOT_FROM_PLL_ENET_MAIN_125M_CLK; - break; - case ENET_50MHZ: - enet1_ref = ENET1_REF_CLK_ROOT_FROM_PLL_ENET_MAIN_50M_CLK; - break; - case ENET_25MHZ: - enet1_ref = ENET1_REF_CLK_ROOT_FROM_PLL_ENET_MAIN_25M_CLK; - break; - default: - return -EINVAL; - } - - /* disable the clock first */ - clock_enable(CCGR_ENET1, 0); - clock_enable(CCGR_SIM_ENET, 0); - - /* set enet axi clock 266Mhz */ - target = CLK_ROOT_ON | ENET_AXI_CLK_ROOT_FROM_SYS1_PLL_266M | -CLK_ROOT_PRE_DIV(CLK_ROOT_PRE_DIV1) | -CLK_ROOT_POST_DIV(CLK_ROOT_POST_DIV1); - clock_set_target_val(ENET_AXI_CLK_ROOT, target); - - target = CLK_ROOT_ON | enet1_ref | -CLK_ROOT_PRE_DIV(CLK_ROOT_PRE_DIV1) | -CLK_ROOT_POST_DIV(CLK_ROOT_POST_DIV1); - clock_set_target_val(ENET_REF_CLK_ROOT, target); - - target = CLK_ROOT_ON | - ENET1_TIME_CLK_ROOT_FROM_PLL_ENET_MAIN_100M_CLK | - CLK_ROOT_PRE_DIV(CLK_ROOT_PRE_DIV1) | - CLK_ROOT_POST_DIV(CLK_ROOT_POST_DIV4); - clock_set_target_val(ENET_TIMER_CLK_ROOT, target); - - /* enable clock */ - clock_enable(CCGR_SIM_ENET, 1); - clock_enable(CCGR_ENET1, 1); - - return 0; -} - static int imx8mp_fec_interface_init(struct udevice *dev, phy_interface_t interface_type, bool mx8mp) diff --git a/board/dhelectronics/dh_imx8mp/imx8mp_dhcom_pdk2.c b/board/dhelectronics/dh_imx8mp/imx8mp_dhcom_pdk2.c index cb9973900bd..5edb85e1de5 100644 --- a/board/dhelectronics/dh_imx8mp/imx8mp_dhcom_pdk2.c +++ b/board/dhelectronics/dh_imx8mp/imx8mp_dhcom_pdk2.c @@ -37,17 +37,6 @@ int board_phys_sdram_size(phys_size_t *size) return 0; } -static void setup_fec(void) -{ - struct iomuxc_gpr_base_regs *gpr = - (struct iomuxc_gpr_base_regs *)IOMUXC_GPR_BASE_ADDR; - - /* Enable RGMII TX clk output. */ - setbits_le32(>gpr[1], BIT(22)); - - set_clk_enet(ENET_125MHZ); -} - static int dh_imx8_setup_ethaddr(void) { unsigned char enetaddr[6]; @@ -114,7 +103,6 @@ int dh_setup_mac_address(void) int board_init(void) { - setup_fec(); return 0; } diff --git a/board/engicam/imx8mm/icore_mx8mm.c b/board/engicam/imx8mm/icore_mx8mm.c index 4f7c699d7d1..320388faae3 100644 --- a/board/engicam/imx8mm/icore_mx8mm.c +++ b/board/engicam/imx8mm/icore_mx8mm.c @@ -29,7 +29,7 @@ static iomux_v3_cfg_t const fec1_rst_pads[] = { IMX8MM_PAD_NAND_DATA01_GPIO3_IO7 | MUX_PAD_CTRL(NO_PAD_CTRL), }; -static void setup_iomux_fec(void) +static void setup_fec(void) { imx_iomux_v3_setup_multiple_pads(fec1_rst_pads, ARRAY_SIZE(fec1_rst_pads)); @@ -40,19 +40,6 @@ static void setup_iomux_fec(void) gpio_direction_output(FEC_RST_PAD, 1); } -static int setup_fec(void) -{ - struct iomuxc_gpr_base_regs *gpr = - (struct iomuxc_gpr_base_regs *)IOMUXC_GPR_BASE_ADDR; - - setup_iomux_fec(); - - /* Use 125M anatop REF_CLK1 for ENET1, not from external */ - clrsetbits_le32(>gpr[1], 13, 0); - - return set_clk_enet(ENET_125MHZ); -} - int board_phy_config(struct phy_device *phydev) { /* enable rgmii rxc skew and phy mode select to RGMII copper */ diff --git a/board/kontron/pitx_imx8m/pitx_imx8m.c b/board/kontron/pitx_imx8m/pitx_imx8m.c
[PATCH v4 13/14] arm64: imx8mp: Drop EQoS GPR[1] board workaround
The EQoS interface mode is now configured in common board_interface_eth_init() and called by EQoS MAC driver when appropriate. Drop the board side duplicates if the same functionality. Signed-off-by: Marek Vasut --- Cc: "Ariel D'Alessandro" Cc: "NXP i.MX U-Boot Team" Cc: Andrey Zhizhikin Cc: Fabio Estevam Cc: Joe Hershberger Cc: Lukasz Majewski Cc: Marcel Ziswiler Cc: Marek Vasut Cc: Michael Trimarchi Cc: Peng Fan Cc: Ramon Fried Cc: Sean Anderson Cc: Stefano Babic Cc: Tim Harvey Cc: Tommaso Merciai Cc: u-boot@lists.denx.de --- V2: Fix the advantech board build V3: Drop now unused architecture set_clk_eqos() code as well V4: No change --- arch/arm/include/asm/arch-imx8m/clock.h | 1 - arch/arm/mach-imx/imx8m/clock_imx8mm.c| 47 --- .../imx8mp_rsb3720a1/imx8mp_rsb3720a1.c | 17 +-- .../dh_imx8mp/imx8mp_dhcom_pdk2.c | 14 -- board/engicam/imx8mp/icore_mx8mp.c| 16 --- board/freescale/imx8mp_evk/imx8mp_evk.c | 17 --- board/gateworks/venice/venice.c | 15 -- board/msc/sm2s_imx8mp/sm2s_imx8mp.c | 15 -- board/toradex/verdin-imx8mp/verdin-imx8mp.c | 16 --- 9 files changed, 1 insertion(+), 157 deletions(-) diff --git a/arch/arm/include/asm/arch-imx8m/clock.h b/arch/arm/include/asm/arch-imx8m/clock.h index e4433763bc4..a861cd6db3a 100644 --- a/arch/arm/include/asm/arch-imx8m/clock.h +++ b/arch/arm/include/asm/arch-imx8m/clock.h @@ -276,5 +276,4 @@ int set_clk_qspi(void); void enable_ocotp_clk(unsigned char enable); int enable_i2c_clk(unsigned char enable, unsigned int i2c_num); int set_clk_enet(enum enet_freq type); -int set_clk_eqos(enum enet_freq type); void hab_caam_clock_enable(unsigned char enable); diff --git a/arch/arm/mach-imx/imx8m/clock_imx8mm.c b/arch/arm/mach-imx/imx8m/clock_imx8mm.c index 32f8623f235..e26658a08de 100644 --- a/arch/arm/mach-imx/imx8m/clock_imx8mm.c +++ b/arch/arm/mach-imx/imx8m/clock_imx8mm.c @@ -827,53 +827,6 @@ u32 mxc_get_clock(enum mxc_clock clk) } #if defined(CONFIG_IMX8MP) && defined(CONFIG_DWC_ETH_QOS) -int set_clk_eqos(enum enet_freq type) -{ - u32 target; - u32 enet1_ref; - - switch (type) { - case ENET_125MHZ: - enet1_ref = ENET1_REF_CLK_ROOT_FROM_PLL_ENET_MAIN_125M_CLK; - break; - case ENET_50MHZ: - enet1_ref = ENET1_REF_CLK_ROOT_FROM_PLL_ENET_MAIN_50M_CLK; - break; - case ENET_25MHZ: - enet1_ref = ENET1_REF_CLK_ROOT_FROM_PLL_ENET_MAIN_25M_CLK; - break; - default: - return -EINVAL; - } - - /* disable the clock first */ - clock_enable(CCGR_QOS_ETHENET, 0); - clock_enable(CCGR_SDMA2, 0); - - /* set enet axi clock 266Mhz */ - target = CLK_ROOT_ON | ENET_AXI_CLK_ROOT_FROM_SYS1_PLL_266M | -CLK_ROOT_PRE_DIV(CLK_ROOT_PRE_DIV1) | -CLK_ROOT_POST_DIV(CLK_ROOT_POST_DIV1); - clock_set_target_val(ENET_AXI_CLK_ROOT, target); - - target = CLK_ROOT_ON | enet1_ref | -CLK_ROOT_PRE_DIV(CLK_ROOT_PRE_DIV1) | -CLK_ROOT_POST_DIV(CLK_ROOT_POST_DIV1); - clock_set_target_val(ENET_QOS_CLK_ROOT, target); - - target = CLK_ROOT_ON | - ENET1_TIME_CLK_ROOT_FROM_PLL_ENET_MAIN_100M_CLK | - CLK_ROOT_PRE_DIV(CLK_ROOT_PRE_DIV1) | - CLK_ROOT_POST_DIV(CLK_ROOT_POST_DIV4); - clock_set_target_val(ENET_QOS_TIMER_CLK_ROOT, target); - - /* enable clock */ - clock_enable(CCGR_QOS_ETHENET, 1); - clock_enable(CCGR_SDMA2, 1); - - return 0; -} - static int imx8mp_eqos_interface_init(struct udevice *dev, phy_interface_t interface_type) { diff --git a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c index 34109c69ddb..9191ddbb682 100644 --- a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c +++ b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c @@ -113,7 +113,7 @@ static const iomux_v3_cfg_t eqos_rst_pads[] = { MX8MP_PAD_SAI2_RXC__GPIO4_IO22 | MUX_PAD_CTRL(NO_PAD_CTRL), }; -static void setup_iomux_eqos(void) +static void setup_eqos(void) { imx_iomux_v3_setup_multiple_pads(eqos_rst_pads, ARRAY_SIZE(eqos_rst_pads)); @@ -124,21 +124,6 @@ static void setup_iomux_eqos(void) gpio_direction_output(EQOS_RST_PAD, 1); mdelay(100); } - -static int setup_eqos(void) -{ - struct iomuxc_gpr_base_regs *gpr = - (struct iomuxc_gpr_base_regs *)IOMUXC_GPR_BASE_ADDR; - - setup_iomux_eqos(); - - /* set INTF as RGMII, enable RGMII TXC clock */ - clrsetbits_le32(>gpr[1], - IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_MASK, BIT(16)); - setbits_le32(>gpr[1], BIT(19) | BIT(21)); - - return set_clk_eqos(ENET_125MHZ); -} #endif /*
[PATCH v4 08/14] net: dwc_eth_qos: Add i.MX8M Plus RMII support
With DM clock support in place, it is easy to add RMII support into the MAC driver. The RMII cannot operate at 1000 Mbps and at 100 and 10 Mbps the clock frequency is 50 MHz and 5 MHz instead of 25 MHz and 2.5 MHz. The board DT requires the following adjustments to EQoS node: phy-mode = "rmii"; assigned-clock-parents = < IMX8MP_SYS_PLL1_266M>, < IMX8MP_SYS_PLL2_100M>, < IMX8MP_SYS_PLL2_50M>; assigned-clock-rates = <0>, <1>, <5000>; Reviewed-by: Ramon Fried Signed-off-by: Marek Vasut --- Cc: "Ariel D'Alessandro" Cc: "NXP i.MX U-Boot Team" Cc: Andrey Zhizhikin Cc: Fabio Estevam Cc: Joe Hershberger Cc: Lukasz Majewski Cc: Marcel Ziswiler Cc: Marek Vasut Cc: Michael Trimarchi Cc: Peng Fan Cc: Ramon Fried Cc: Sean Anderson Cc: Stefano Babic Cc: Tim Harvey Cc: Tommaso Merciai Cc: u-boot@lists.denx.de --- V2: Add RB from Ramon V3: Handle RGMII_*ID variants V4: No change --- drivers/net/dwc_eth_qos_imx.c | 29 ++--- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/drivers/net/dwc_eth_qos_imx.c b/drivers/net/dwc_eth_qos_imx.c index f5f3f2099f0..962c5373243 100644 --- a/drivers/net/dwc_eth_qos_imx.c +++ b/drivers/net/dwc_eth_qos_imx.c @@ -179,21 +179,28 @@ static int eqos_set_tx_clk_speed_imx(struct udevice *dev) debug("%s(dev=%p):\n", __func__, dev); - switch (eqos->phy->speed) { - case SPEED_1000: - rate = 125 * 1000 * 1000; - break; - case SPEED_100: - rate = 25 * 1000 * 1000; - break; - case SPEED_10: - rate = 2.5 * 1000 * 1000; - break; - default: + if (eqos->phy->interface == PHY_INTERFACE_MODE_RMII) + rate = 5000;/* 5000 kHz = 5 MHz */ + else + rate = 2500;/* 2500 kHz = 2.5 MHz */ + + if (eqos->phy->speed == SPEED_1000 && + (eqos->phy->interface == PHY_INTERFACE_MODE_RGMII || +eqos->phy->interface == PHY_INTERFACE_MODE_RGMII_ID || +eqos->phy->interface == PHY_INTERFACE_MODE_RGMII_RXID || +eqos->phy->interface == PHY_INTERFACE_MODE_RGMII_TXID)) { + rate *= 50; /* Use 50x base rate i.e. 125 MHz */ + } else if (eqos->phy->speed == SPEED_100) { + rate *= 10; /* Use 10x base rate */ + } else if (eqos->phy->speed == SPEED_10) { + rate *= 1; /* Use base rate */ + } else { pr_err("invalid speed %d", eqos->phy->speed); return -EINVAL; } + rate *= 1000; /* clk_set_rate() operates in Hz */ + ret = clk_set_rate(>clk_tx, rate); if (ret < 0) { pr_err("imx (tx_clk, %lu) failed: %d", rate, ret); -- 2.39.2
[PATCH v4 09/14] net: dwc_eth_qos: Add board_interface_eth_init() for i.MX8M Plus
Implement common board_interface_eth_init() and call it from the DWMAC driver to configure IOMUXC GPR[1] register according to the PHY mode obtained from DT. This supports all three interface modes supported by the i.MX8M Plus DWMAC and supersedes current board-side configuration of the same IOMUX GPR[1] duplicated in the board files. Reviewed-by: Ramon Fried Signed-off-by: Marek Vasut --- Cc: "Ariel D'Alessandro" Cc: "NXP i.MX U-Boot Team" Cc: Andrey Zhizhikin Cc: Fabio Estevam Cc: Joe Hershberger Cc: Lukasz Majewski Cc: Marcel Ziswiler Cc: Marek Vasut Cc: Michael Trimarchi Cc: Peng Fan Cc: Ramon Fried Cc: Sean Anderson Cc: Stefano Babic Cc: Tim Harvey Cc: Tommaso Merciai Cc: u-boot@lists.denx.de --- V2: - Add RB from Ramon - Handle RGMII_ID/RGMII_RXID/RGMII_TXID just like plain RGMII V3: Make the function more generic, so it can be shared by eqos and fec V4: No change --- arch/arm/include/asm/arch-imx8m/imx-regs.h | 8 ++- arch/arm/mach-imx/imx8m/clock_imx8mm.c | 59 +- drivers/net/dwc_eth_qos_imx.c | 4 ++ 3 files changed, 69 insertions(+), 2 deletions(-) diff --git a/arch/arm/include/asm/arch-imx8m/imx-regs.h b/arch/arm/include/asm/arch-imx8m/imx-regs.h index 1559bf6d218..1818b459fa6 100644 --- a/arch/arm/include/asm/arch-imx8m/imx-regs.h +++ b/arch/arm/include/asm/arch-imx8m/imx-regs.h @@ -89,7 +89,13 @@ #define DDRC_IPS_BASE_ADDR(X) (0x3d40 + ((X) * 0x200)) #define DDR_CSD1_BASE_ADDR 0x4000 -#define IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_MASK 0x7 +#define IOMUXC_GPR_GPR1_GPR_ENET_QOS_RGMII_EN BIT(21) +#define IOMUXC_GPR_GPR1_GPR_ENET_QOS_CLK_TX_CLK_SELBIT(20) +#define IOMUXC_GPR_GPR1_GPR_ENET_QOS_CLK_GEN_ENBIT(19) +#define IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_MASK GENMASK(18, 16) +#define IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_MII (0 << 16) +#define IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_RGMII(1 << 16) +#define IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_RMII (4 << 16) #define FEC_QUIRK_ENET_MAC #ifdef CONFIG_ARMV8_PSCI /* Final jump location */ diff --git a/arch/arm/mach-imx/imx8m/clock_imx8mm.c b/arch/arm/mach-imx/imx8m/clock_imx8mm.c index 494bfbedc8c..1546c9ba9a0 100644 --- a/arch/arm/mach-imx/imx8m/clock_imx8mm.c +++ b/arch/arm/mach-imx/imx8m/clock_imx8mm.c @@ -15,6 +15,7 @@ #include #include #include +#include DECLARE_GLOBAL_DATA_PTR; @@ -825,7 +826,7 @@ u32 mxc_get_clock(enum mxc_clock clk) return 0; } -#ifdef CONFIG_DWC_ETH_QOS +#if defined(CONFIG_IMX8MP) && defined(CONFIG_DWC_ETH_QOS) int set_clk_eqos(enum enet_freq type) { u32 target; @@ -872,6 +873,52 @@ int set_clk_eqos(enum enet_freq type) return 0; } + +static int imx8mp_eqos_interface_init(struct udevice *dev, + phy_interface_t interface_type) +{ + struct iomuxc_gpr_base_regs *gpr = + (struct iomuxc_gpr_base_regs *)IOMUXC_GPR_BASE_ADDR; + + clrbits_le32(>gpr[1], +IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_MASK | +IOMUXC_GPR_GPR1_GPR_ENET_QOS_RGMII_EN | +IOMUXC_GPR_GPR1_GPR_ENET_QOS_CLK_TX_CLK_SEL | +IOMUXC_GPR_GPR1_GPR_ENET_QOS_CLK_GEN_EN); + + switch (interface_type) { + case PHY_INTERFACE_MODE_MII: + setbits_le32(>gpr[1], +IOMUXC_GPR_GPR1_GPR_ENET_QOS_CLK_GEN_EN | +IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_MII); + break; + case PHY_INTERFACE_MODE_RMII: + setbits_le32(>gpr[1], +IOMUXC_GPR_GPR1_GPR_ENET_QOS_CLK_TX_CLK_SEL | +IOMUXC_GPR_GPR1_GPR_ENET_QOS_CLK_GEN_EN | +IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_RMII); + break; + case PHY_INTERFACE_MODE_RGMII: + case PHY_INTERFACE_MODE_RGMII_ID: + case PHY_INTERFACE_MODE_RGMII_RXID: + case PHY_INTERFACE_MODE_RGMII_TXID: + setbits_le32(>gpr[1], +IOMUXC_GPR_GPR1_GPR_ENET_QOS_RGMII_EN | +IOMUXC_GPR_GPR1_GPR_ENET_QOS_CLK_GEN_EN | +IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_RGMII); + break; + default: + return -EINVAL; + } + + return 0; +} +#else +static int imx8mp_eqos_interface_init(struct udevice *dev, + phy_interface_t interface_type) +{ + return 0; +} #endif #ifdef CONFIG_FEC_MXC @@ -922,3 +969,13 @@ int set_clk_enet(enum enet_freq type) return 0; } #endif + +int board_interface_eth_init(struct udevice *dev, phy_interface_t interface_type) +{ + if (IS_ENABLED(CONFIG_IMX8MP) && + IS_ENABLED(CONFIG_DWC_ETH_QOS) && + device_is_compatible(dev, "nxp,imx8mp-dwmac-eqos")) + return imx8mp_eqos_interface_init(dev,
[PATCH v4 06/14] net: dwc_eth_qos: Set DMA_MODE SWR bit to reset the MAC
The driver currently only waits for DMA_MODE SWR bit to clear itself. This is insufficient e.g. on i.MX8M Plus, where the MAC must be reset before IOMUX GPR[1] content is latched into the MAC and used. Without the proper reset, the i.MX8M Plus MAC variant does not take the value in IOMUX GPR[1] into account, which makes it impossible e.g. to switch interface mode from RGMII to any other. Since proper reset is desired in general to put the block into defined state, always assert the DMA_MODE SWR bit before waiting for the bit to clear itself. Reviewed-by: Ramon Fried Signed-off-by: Marek Vasut --- Cc: "Ariel D'Alessandro" Cc: "NXP i.MX U-Boot Team" Cc: Andrey Zhizhikin Cc: Fabio Estevam Cc: Joe Hershberger Cc: Lukasz Majewski Cc: Marcel Ziswiler Cc: Marek Vasut Cc: Michael Trimarchi Cc: Peng Fan Cc: Ramon Fried Cc: Sean Anderson Cc: Stefano Babic Cc: Tim Harvey Cc: Tommaso Merciai Cc: u-boot@lists.denx.de --- V2: Add RB from Ramon V3: No change V4: No change --- drivers/net/dwc_eth_qos.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c index 9a5575e7b83..ec58697b311 100644 --- a/drivers/net/dwc_eth_qos.c +++ b/drivers/net/dwc_eth_qos.c @@ -761,6 +761,12 @@ static int eqos_start(struct udevice *dev) eqos->reg_access_ok = true; + /* +* Assert the SWR first, the actually reset the MAC and to latch in +* e.g. i.MX8M Plus GPR[1] content, which selects interface mode. +*/ + setbits_le32(>dma_regs->mode, EQOS_DMA_MODE_SWR); + ret = wait_for_bit_le32(>dma_regs->mode, EQOS_DMA_MODE_SWR, false, eqos->config->swr_wait, false); -- 2.39.2
[PATCH v4 11/14] net: fec_mxc: Add board_interface_eth_init() for i.MX8M Mini/Nano/Plus
Implement common board_interface_eth_init() and call it from the FEC driver to configure IOMUXC GPR[1] register according to the PHY mode obtained from DT. This supports all three interface modes supported by the i.MX8M Mini/Nano/Plus FEC and supersedes the current board-side configuration of the same IOMUX GPR[1] duplicated in the board files. Signed-off-by: Marek Vasut --- Cc: "Ariel D'Alessandro" Cc: "NXP i.MX U-Boot Team" Cc: Andrey Zhizhikin Cc: Fabio Estevam Cc: Joe Hershberger Cc: Lukasz Majewski Cc: Marcel Ziswiler Cc: Marek Vasut Cc: Michael Trimarchi Cc: Peng Fan Cc: Ramon Fried Cc: Sean Anderson Cc: Stefano Babic Cc: Tim Harvey Cc: Tommaso Merciai Cc: u-boot@lists.denx.de --- V3: New patch V4: Drop ifdef MX8MP around imx8mp_fec_interface_init --- arch/arm/include/asm/arch-imx8m/imx-regs.h | 2 + arch/arm/mach-imx/imx8m/clock_imx8mm.c | 46 ++ drivers/net/fec_mxc.c | 4 ++ 3 files changed, 52 insertions(+) diff --git a/arch/arm/include/asm/arch-imx8m/imx-regs.h b/arch/arm/include/asm/arch-imx8m/imx-regs.h index 1818b459fa6..6e2fc82a0e4 100644 --- a/arch/arm/include/asm/arch-imx8m/imx-regs.h +++ b/arch/arm/include/asm/arch-imx8m/imx-regs.h @@ -89,6 +89,7 @@ #define DDRC_IPS_BASE_ADDR(X) (0x3d40 + ((X) * 0x200)) #define DDR_CSD1_BASE_ADDR 0x4000 +#define IOMUXC_GPR_GPR1_GPR_ENET1_RGMII_EN BIT(22) #define IOMUXC_GPR_GPR1_GPR_ENET_QOS_RGMII_EN BIT(21) #define IOMUXC_GPR_GPR1_GPR_ENET_QOS_CLK_TX_CLK_SELBIT(20) #define IOMUXC_GPR_GPR1_GPR_ENET_QOS_CLK_GEN_ENBIT(19) @@ -96,6 +97,7 @@ #define IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_MII (0 << 16) #define IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_RGMII(1 << 16) #define IOMUXC_GPR_GPR1_GPR_ENET_QOS_INTF_SEL_RMII (4 << 16) +#define IOMUXC_GPR_GPR1_GPR_ENET1_TX_CLK_SEL BIT(13) #define FEC_QUIRK_ENET_MAC #ifdef CONFIG_ARMV8_PSCI /* Final jump location */ diff --git a/arch/arm/mach-imx/imx8m/clock_imx8mm.c b/arch/arm/mach-imx/imx8m/clock_imx8mm.c index 1546c9ba9a0..32f8623f235 100644 --- a/arch/arm/mach-imx/imx8m/clock_imx8mm.c +++ b/arch/arm/mach-imx/imx8m/clock_imx8mm.c @@ -968,10 +968,56 @@ int set_clk_enet(enum enet_freq type) return 0; } + +static int imx8mp_fec_interface_init(struct udevice *dev, +phy_interface_t interface_type, +bool mx8mp) +{ + /* i.MX8MP has extra RGMII_EN bit in IOMUXC GPR1 register */ + const u32 rgmii_en = mx8mp ? IOMUXC_GPR_GPR1_GPR_ENET1_RGMII_EN : 0; + struct iomuxc_gpr_base_regs *gpr = + (struct iomuxc_gpr_base_regs *)IOMUXC_GPR_BASE_ADDR; + + clrbits_le32(>gpr[1], +rgmii_en | +IOMUXC_GPR_GPR1_GPR_ENET1_TX_CLK_SEL); + + switch (interface_type) { + case PHY_INTERFACE_MODE_MII: + case PHY_INTERFACE_MODE_RMII: + setbits_le32(>gpr[1], IOMUXC_GPR_GPR1_GPR_ENET1_TX_CLK_SEL); + break; + case PHY_INTERFACE_MODE_RGMII: + case PHY_INTERFACE_MODE_RGMII_ID: + case PHY_INTERFACE_MODE_RGMII_RXID: + case PHY_INTERFACE_MODE_RGMII_TXID: + setbits_le32(>gpr[1], rgmii_en); + break; + default: + return -EINVAL; + } + + return 0; +} #endif int board_interface_eth_init(struct udevice *dev, phy_interface_t interface_type) { + if (IS_ENABLED(CONFIG_IMX8MM) && + IS_ENABLED(CONFIG_FEC_MXC) && + device_is_compatible(dev, "fsl,imx8mm-fec")) + return imx8mp_fec_interface_init(dev, interface_type, false); + + if (IS_ENABLED(CONFIG_IMX8MN) && + IS_ENABLED(CONFIG_FEC_MXC) && + device_is_compatible(dev, "fsl,imx8mn-fec")) + return imx8mp_fec_interface_init(dev, interface_type, false); + + if (IS_ENABLED(CONFIG_IMX8MP) && + IS_ENABLED(CONFIG_FEC_MXC) && + device_is_compatible(dev, "fsl,imx8mp-fec")) + return imx8mp_fec_interface_init(dev, interface_type, true); + if (IS_ENABLED(CONFIG_IMX8MP) && IS_ENABLED(CONFIG_DWC_ETH_QOS) && device_is_compatible(dev, "nxp,imx8mp-dwmac-eqos")) diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c index 7a8577158ae..ac937676f9c 100644 --- a/drivers/net/fec_mxc.c +++ b/drivers/net/fec_mxc.c @@ -1232,6 +1232,10 @@ static int fecmxc_probe(struct udevice *dev) uint32_t start; int ret; + ret = board_interface_eth_init(dev, pdata->phy_interface); + if (ret) + return ret; + if (IS_ENABLED(CONFIG_IMX_MODULE_FUSE)) { if (enet_fused((ulong)priv->eth)) { printf("SoC fuse indicates Ethernet@0x%lx is unavailable.\n", (ulong)priv->eth); -- 2.39.2
[PATCH v4 10/14] net: fec_mxc: Add ref clock setup support for i.MX8M Mini/Nano/Plus
The FEC ref clock frequency on i.MX8M Mini/Nano/Plus was so far configured via ad-hoc board code. Replace that with DM clock clk_set_rate() instead. This way, the driver claims all its required clock and sets the ref clock rate, without any need of architecture specific register fiddling. Signed-off-by: Marek Vasut --- Cc: "Ariel D'Alessandro" Cc: "NXP i.MX U-Boot Team" Cc: Andrey Zhizhikin Cc: Fabio Estevam Cc: Joe Hershberger Cc: Lukasz Majewski Cc: Marcel Ziswiler Cc: Marek Vasut Cc: Michael Trimarchi Cc: Peng Fan Cc: Ramon Fried Cc: Sean Anderson Cc: Stefano Babic Cc: Tim Harvey Cc: Tommaso Merciai Cc: u-boot@lists.denx.de --- V3: New patch V4: No change --- drivers/net/fec_mxc.c | 32 1 file changed, 32 insertions(+) diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c index 1a6c18a441f..7a8577158ae 100644 --- a/drivers/net/fec_mxc.c +++ b/drivers/net/fec_mxc.c @@ -1196,6 +1196,33 @@ static void fec_gpio_reset(struct fec_priv *priv) } #endif +static int fecmxc_set_ref_clk(struct clk *clk_ref, phy_interface_t interface) +{ + unsigned int freq; + int ret; + + if (!CONFIG_IS_ENABLED(CLK_CCF)) + return 0; + + if (interface == PHY_INTERFACE_MODE_MII) + freq = 2500; + else if (interface == PHY_INTERFACE_MODE_RMII) + freq = 5000; + else if (interface == PHY_INTERFACE_MODE_RGMII || +interface == PHY_INTERFACE_MODE_RGMII_ID || +interface == PHY_INTERFACE_MODE_RGMII_RXID || +interface == PHY_INTERFACE_MODE_RGMII_TXID) + freq = 12500; + else + return -EINVAL; + + ret = clk_set_rate(clk_ref, freq); + if (ret < 0) + return ret; + + return 0; +} + static int fecmxc_probe(struct udevice *dev) { bool dm_mii_bus = true; @@ -1253,6 +1280,11 @@ static int fecmxc_probe(struct udevice *dev) ret = clk_get_by_name(dev, "enet_clk_ref", >clk_ref); if (!ret) { + ret = fecmxc_set_ref_clk(>clk_ref, +pdata->phy_interface); + if (ret) + return ret; + ret = clk_enable(>clk_ref); if (ret) return ret; -- 2.39.2
[PATCH v4 02/14] net: Pull board_interface_eth_init() into common code
Move the board_interface_eth_init() into common ethernet uclass code, since this function could be shared by multiple drivers. Reviewed-by: Simon Glass Signed-off-by: Marek Vasut --- Cc: "Ariel D'Alessandro" Cc: "NXP i.MX U-Boot Team" Cc: Andrey Zhizhikin Cc: Fabio Estevam Cc: Joe Hershberger Cc: Lukasz Majewski Cc: Marcel Ziswiler Cc: Marek Vasut Cc: Michael Trimarchi Cc: Peng Fan Cc: Ramon Fried Cc: Sean Anderson Cc: Stefano Babic Cc: Tim Harvey Cc: Tommaso Merciai Cc: u-boot@lists.denx.de --- V3: New patch V4: Add RB from Simon --- drivers/net/dwc_eth_qos.c | 7 --- net/eth-uclass.c | 7 +++ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c index 112deb546de..0cae2cf2064 100644 --- a/drivers/net/dwc_eth_qos.c +++ b/drivers/net/dwc_eth_qos.c @@ -1412,13 +1412,6 @@ err_free_reset_eqos: return ret; } -/* board-specific Ethernet Interface initializations. */ -__weak int board_interface_eth_init(struct udevice *dev, - phy_interface_t interface_type) -{ - return 0; -} - static int eqos_probe_resources_stm32(struct udevice *dev) { struct eqos_priv *eqos = dev_get_priv(dev); diff --git a/net/eth-uclass.c b/net/eth-uclass.c index b01a910938e..c393600fabc 100644 --- a/net/eth-uclass.c +++ b/net/eth-uclass.c @@ -49,6 +49,13 @@ struct eth_uclass_priv { /* eth_errno - This stores the most recent failure code from DM functions */ static int eth_errno; +/* board-specific Ethernet Interface initializations. */ +__weak int board_interface_eth_init(struct udevice *dev, + phy_interface_t interface_type) +{ + return 0; +} + static struct eth_uclass_priv *eth_get_uclass_priv(void) { struct uclass *uc; -- 2.39.2
[PATCH v4 04/14] net: dwc_eth_qos: Drop unused dm_gpio_free() on STM32
The dm_gpio_free() is never called, because for stm32, the phy_reset_gpio pointer is never valid. This is because only tegra186 ever claims the phy_reset_gpio, all other platforms use the PHY framework to reset the PHY instead. Drop the dm_gpio_free() and dm_gpio_is_valid(). Reviewed-by: Ramon Fried Signed-off-by: Marek Vasut --- Cc: "Ariel D'Alessandro" Cc: "NXP i.MX U-Boot Team" Cc: Andrey Zhizhikin Cc: Fabio Estevam Cc: Joe Hershberger Cc: Lukasz Majewski Cc: Marcel Ziswiler Cc: Marek Vasut Cc: Michael Trimarchi Cc: Peng Fan Cc: Ramon Fried Cc: Sean Anderson Cc: Stefano Babic Cc: Tim Harvey Cc: Tommaso Merciai Cc: u-boot@lists.denx.de --- V2: - Add RB from Ramon - Mark eqos variable in eqos_remove_resources_stm32() with __maybe_unused V3: No change V4: No change --- drivers/net/dwc_eth_qos.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c index 00690b28ca6..b97b3ea2db6 100644 --- a/drivers/net/dwc_eth_qos.c +++ b/drivers/net/dwc_eth_qos.c @@ -1493,7 +1493,7 @@ static int eqos_remove_resources_tegra186(struct udevice *dev) static int eqos_remove_resources_stm32(struct udevice *dev) { - struct eqos_priv *eqos = dev_get_priv(dev); + struct eqos_priv * __maybe_unused eqos = dev_get_priv(dev); debug("%s(dev=%p):\n", __func__, dev); @@ -1505,9 +1505,6 @@ static int eqos_remove_resources_stm32(struct udevice *dev) clk_free(>clk_ck); #endif - if (dm_gpio_is_valid(>phy_reset_gpio)) - dm_gpio_free(dev, >phy_reset_gpio); - debug("%s: OK\n", __func__); return 0; } -- 2.39.2
[PATCH v4 07/14] net: dwc_eth_qos: Add DM CLK support for i.MX8M Plus
The DWMAC clock in i.MX8M Plus were so far configured via ad-hoc architecture code. Replace that with DM clock instead. This way, the driver claims all its required clock, enables and disables them, and even gets the CSR clock rate and sets the TX clock rate, without any need of architecture specific register fiddling. Drop the architecture specific code while at it too. The adjustment here is modeled after STM32MP15xx clock handling in this driver. Signed-off-by: Marek Vasut --- Cc: "Ariel D'Alessandro" Cc: "NXP i.MX U-Boot Team" Cc: Andrey Zhizhikin Cc: Fabio Estevam Cc: Joe Hershberger Cc: Lukasz Majewski Cc: Marcel Ziswiler Cc: Marek Vasut Cc: Michael Trimarchi Cc: Peng Fan Cc: Ramon Fried Cc: Sean Anderson Cc: Stefano Babic Cc: Tim Harvey Cc: Tommaso Merciai Cc: u-boot@lists.denx.de --- V2: Turn all the pr_err() into dev_dbg() V3: No change V4: No change --- arch/arm/mach-imx/imx8m/clock_imx8mm.c | 41 drivers/net/dwc_eth_qos_imx.c | 131 +++-- 2 files changed, 121 insertions(+), 51 deletions(-) diff --git a/arch/arm/mach-imx/imx8m/clock_imx8mm.c b/arch/arm/mach-imx/imx8m/clock_imx8mm.c index 64ad57e9b39..494bfbedc8c 100644 --- a/arch/arm/mach-imx/imx8m/clock_imx8mm.c +++ b/arch/arm/mach-imx/imx8m/clock_imx8mm.c @@ -872,47 +872,6 @@ int set_clk_eqos(enum enet_freq type) return 0; } - -int imx_eqos_txclk_set_rate(ulong rate) -{ - u32 val; - u32 eqos_post_div; - - /* disable the clock first */ - clock_enable(CCGR_QOS_ETHENET, 0); - clock_enable(CCGR_SDMA2, 0); - - switch (rate) { - case 12500: - eqos_post_div = 1; - break; - case 2500: - eqos_post_div = 12500 / 2500; - break; - case 250: - eqos_post_div = 12500 / 250; - break; - default: - return -EINVAL; - } - - clock_get_target_val(ENET_QOS_CLK_ROOT, ); - val &= ~(CLK_ROOT_PRE_DIV_MASK | CLK_ROOT_POST_DIV_MASK); - val |= CLK_ROOT_PRE_DIV(CLK_ROOT_PRE_DIV1) | - CLK_ROOT_POST_DIV(eqos_post_div - 1); - clock_set_target_val(ENET_QOS_CLK_ROOT, val); - - /* enable clock */ - clock_enable(CCGR_QOS_ETHENET, 1); - clock_enable(CCGR_SDMA2, 1); - - return 0; -} - -u32 imx_get_eqos_csr_clk(void) -{ - return get_root_clk(ENET_AXI_CLK_ROOT); -} #endif #ifdef CONFIG_FEC_MXC diff --git a/drivers/net/dwc_eth_qos_imx.c b/drivers/net/dwc_eth_qos_imx.c index 42cb164ad14..f5f3f2099f0 100644 --- a/drivers/net/dwc_eth_qos_imx.c +++ b/drivers/net/dwc_eth_qos_imx.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -32,20 +33,18 @@ __weak u32 imx_get_eqos_csr_clk(void) return 100 * 100; } -__weak int imx_eqos_txclk_set_rate(unsigned long rate) -{ - return 0; -} - static ulong eqos_get_tick_clk_rate_imx(struct udevice *dev) { - return imx_get_eqos_csr_clk(); + struct eqos_priv *eqos = dev_get_priv(dev); + + return clk_get_rate(>clk_master_bus); } static int eqos_probe_resources_imx(struct udevice *dev) { struct eqos_priv *eqos = dev_get_priv(dev); phy_interface_t interface; + int ret; debug("%s(dev=%p):\n", __func__, dev); @@ -56,6 +55,118 @@ static int eqos_probe_resources_imx(struct udevice *dev) return -EINVAL; } + eqos->max_speed = dev_read_u32_default(dev, "max-speed", 0); + + ret = clk_get_by_name(dev, "stmmaceth", >clk_master_bus); + if (ret) { + dev_dbg(dev, "clk_get_by_name(master_bus) failed: %d", ret); + goto err_probe; + } + + ret = clk_get_by_name(dev, "ptp_ref", >clk_ptp_ref); + if (ret) { + dev_dbg(dev, "clk_get_by_name(ptp_ref) failed: %d", ret); + goto err_free_clk_master_bus; + } + + ret = clk_get_by_name(dev, "tx", >clk_tx); + if (ret) { + dev_dbg(dev, "clk_get_by_name(tx) failed: %d", ret); + goto err_free_clk_ptp_ref; + } + + ret = clk_get_by_name(dev, "pclk", >clk_ck); + if (ret) { + dev_dbg(dev, "clk_get_by_name(pclk) failed: %d", ret); + goto err_free_clk_tx; + } + + debug("%s: OK\n", __func__); + return 0; + +err_free_clk_tx: + clk_free(>clk_tx); +err_free_clk_ptp_ref: + clk_free(>clk_ptp_ref); +err_free_clk_master_bus: + clk_free(>clk_master_bus); +err_probe: + + debug("%s: returns %d\n", __func__, ret); + return ret; +} + +static int eqos_remove_resources_imx(struct udevice *dev) +{ + struct eqos_priv *eqos = dev_get_priv(dev); + + debug("%s(dev=%p):\n", __func__, dev); + + clk_free(>clk_ck); + clk_free(>clk_tx); + clk_free(>clk_ptp_ref); + clk_free(>clk_master_bus); + + debug("%s: OK\n", __func__); +
[PATCH v4 05/14] net: dwc_eth_qos: Staticize eqos_inval_buffer_tegra186()
This function is only used within the driver, staticize it. Fixes: 149e80f74b6 ("net: dwc_eth_qos: public some functions") Signed-off-by: Marek Vasut --- Cc: "Ariel D'Alessandro" Cc: "NXP i.MX U-Boot Team" Cc: Andrey Zhizhikin Cc: Fabio Estevam Cc: Joe Hershberger Cc: Lukasz Majewski Cc: Marcel Ziswiler Cc: Marek Vasut Cc: Michael Trimarchi Cc: Peng Fan Cc: Ramon Fried Cc: Sean Anderson Cc: Stefano Babic Cc: Tim Harvey Cc: Tommaso Merciai Cc: u-boot@lists.denx.de --- V2: - New patch V3: No change V4: No change --- drivers/net/dwc_eth_qos.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c index b97b3ea2db6..9a5575e7b83 100644 --- a/drivers/net/dwc_eth_qos.c +++ b/drivers/net/dwc_eth_qos.c @@ -108,7 +108,7 @@ void eqos_flush_desc_generic(void *desc) flush_dcache_range(start, end); } -void eqos_inval_buffer_tegra186(void *buf, size_t size) +static void eqos_inval_buffer_tegra186(void *buf, size_t size) { unsigned long start = (unsigned long)buf & ~(ARCH_DMA_MINALIGN - 1); unsigned long end = ALIGN(start + size, ARCH_DMA_MINALIGN); -- 2.39.2
[PATCH v4 01/14] clk: imx8mp: Add EQoS MAC clock
Add clock for the DWMAC EQoS block. This is used among other things to configure the MII clock via DM CLK. Acked-by: Sean Anderson Reviewed-by: Peng Fan Signed-off-by: Marek Vasut --- Cc: "Ariel D'Alessandro" Cc: "NXP i.MX U-Boot Team" Cc: Andrey Zhizhikin Cc: Fabio Estevam Cc: Joe Hershberger Cc: Lukasz Majewski Cc: Marcel Ziswiler Cc: Marek Vasut Cc: Michael Trimarchi Cc: Peng Fan Cc: Ramon Fried Cc: Sean Anderson Cc: Stefano Babic Cc: Tim Harvey Cc: Tommaso Merciai Cc: u-boot@lists.denx.de --- V2: Add AB from Sean V3: No change V4: No change --- drivers/clk/imx/clk-imx8mp.c | 13 + 1 file changed, 13 insertions(+) diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c index ffbc1d1ba9f..6dda0403e35 100644 --- a/drivers/clk/imx/clk-imx8mp.c +++ b/drivers/clk/imx/clk-imx8mp.c @@ -70,6 +70,14 @@ static const char *imx8mp_i2c6_sels[] = {"clock-osc-24m", "sys_pll1_160m", "sys_ "sys_pll3_out", "audio_pll1_out", "video_pll1_out", "audio_pll2_out", "sys_pll1_133m", }; +static const char *imx8mp_enet_qos_sels[] = {"clock-osc-24m", "sys_pll2_125m", "sys_pll2_50m", +"sys_pll2_100m", "sys_pll1_160m", "audio_pll1_out", +"video_pll1_out", "clk_ext4", }; + +static const char *imx8mp_enet_qos_timer_sels[] = {"clock-osc-24m", "sys_pll2_100m", "audio_pll1_out", + "clk_ext1", "clk_ext2", "clk_ext3", + "clk_ext4", "video_pll1_out", }; + static const char *imx8mp_usdhc1_sels[] = {"clock-osc-24m", "sys_pll1_400m", "sys_pll1_800m", "sys_pll2_500m", "sys_pll3_out", "sys_pll1_266m", "audio_pll2_out", "sys_pll1_100m", }; @@ -250,6 +258,8 @@ static int imx8mp_clk_probe(struct udevice *dev) clk_dm(IMX8MP_CLK_DRAM_APB, imx8m_clk_composite_critical("dram_apb", imx8mp_dram_apb_sels, base + 0xa080)); clk_dm(IMX8MP_CLK_I2C5, imx8m_clk_composite("i2c5", imx8mp_i2c5_sels, base + 0xa480)); clk_dm(IMX8MP_CLK_I2C6, imx8m_clk_composite("i2c6", imx8mp_i2c6_sels, base + 0xa500)); + clk_dm(IMX8MP_CLK_ENET_QOS, imx8m_clk_composite("enet_qos", imx8mp_enet_qos_sels, base + 0xa880)); + clk_dm(IMX8MP_CLK_ENET_QOS_TIMER, imx8m_clk_composite("enet_qos_timer", imx8mp_enet_qos_timer_sels, base + 0xa900)); clk_dm(IMX8MP_CLK_ENET_REF, imx8m_clk_composite("enet_ref", imx8mp_enet_ref_sels, base + 0xa980)); clk_dm(IMX8MP_CLK_ENET_TIMER, imx8m_clk_composite("enet_timer", imx8mp_enet_timer_sels, base + 0xaa00)); clk_dm(IMX8MP_CLK_ENET_PHY_REF, imx8m_clk_composite("enet_phy_ref", imx8mp_enet_phy_ref_sels, base + 0xaa80)); @@ -292,10 +302,13 @@ static int imx8mp_clk_probe(struct udevice *dev) clk_dm(IMX8MP_CLK_I2C2_ROOT, imx_clk_gate4("i2c2_root_clk", "i2c2", base + 0x4180, 0)); clk_dm(IMX8MP_CLK_I2C3_ROOT, imx_clk_gate4("i2c3_root_clk", "i2c3", base + 0x4190, 0)); clk_dm(IMX8MP_CLK_I2C4_ROOT, imx_clk_gate4("i2c4_root_clk", "i2c4", base + 0x41a0, 0)); + clk_dm(IMX8MP_CLK_QOS_ROOT, imx_clk_gate4("qos_root_clk", "ipg_root", base + 0x42c0, 0)); + clk_dm(IMX8MP_CLK_QOS_ENET_ROOT, imx_clk_gate4("qos_enet_root_clk", "ipg_root", base + 0x42e0, 0)); clk_dm(IMX8MP_CLK_QSPI_ROOT, imx_clk_gate4("qspi_root_clk", "qspi", base + 0x42f0, 0)); clk_dm(IMX8MP_CLK_I2C5_ROOT, imx_clk_gate2("i2c5_root_clk", "i2c5", base + 0x4330, 0)); clk_dm(IMX8MP_CLK_I2C6_ROOT, imx_clk_gate2("i2c6_root_clk", "i2c6", base + 0x4340, 0)); clk_dm(IMX8MP_CLK_SIM_ENET_ROOT, imx_clk_gate4("sim_enet_root_clk", "enet_axi", base + 0x4400, 0)); + clk_dm(IMX8MP_CLK_ENET_QOS_ROOT, imx_clk_gate4("enet_qos_root_clk", "sim_enet_root_clk", base + 0x43b0, 0)); clk_dm(IMX8MP_CLK_UART1_ROOT, imx_clk_gate4("uart1_root_clk", "uart1", base + 0x4490, 0)); clk_dm(IMX8MP_CLK_UART2_ROOT, imx_clk_gate4("uart2_root_clk", "uart2", base + 0x44a0, 0)); clk_dm(IMX8MP_CLK_UART3_ROOT, imx_clk_gate4("uart3_root_clk", "uart3", base + 0x44b0, 0)); -- 2.39.2
[PATCH v4 03/14] net: dwc_eth_qos: Drop bogus return after goto
The return is never triggered due to the goto just above it. Drop it. No functional change. Reviewed-by: Ramon Fried Signed-off-by: Marek Vasut --- Cc: "Ariel D'Alessandro" Cc: "NXP i.MX U-Boot Team" Cc: Andrey Zhizhikin Cc: Fabio Estevam Cc: Joe Hershberger Cc: Lukasz Majewski Cc: Marcel Ziswiler Cc: Marek Vasut Cc: Michael Trimarchi Cc: Peng Fan Cc: Ramon Fried Cc: Sean Anderson Cc: Stefano Babic Cc: Tim Harvey Cc: Tommaso Merciai Cc: u-boot@lists.denx.de --- V2: Add RB from Ramon V3: No change V4: No change --- drivers/net/dwc_eth_qos.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c index 0cae2cf2064..00690b28ca6 100644 --- a/drivers/net/dwc_eth_qos.c +++ b/drivers/net/dwc_eth_qos.c @@ -1383,7 +1383,6 @@ static int eqos_probe_resources_tegra186(struct udevice *dev) if (ret) { pr_err("clk_get_by_name(ptp_ref) failed: %d", ret); goto err_free_clk_rx; - return ret; } ret = clk_get_by_name(dev, "tx", >clk_tx); -- 2.39.2
Re: [PATCH 1/1] api: move API related config options into submenu
On Mon, Mar 06, 2023 at 11:18:17AM +0100, Heinrich Schuchardt wrote: > On 3/4/23 16:32, Tom Rini wrote: > > On Fri, Mar 03, 2023 at 11:31:22PM +0100, Heinrich Schuchardt wrote: > > > > > Kconfig settings that are related to the API for standalone applications > > > should be in the API sub-menu and not on the top level. > > > > > > CONFIG_STANDALONE_LOAD_ADDR is only relevant if standalone example > > > applications are built. > > > > > > Signed-off-by: Heinrich Schuchardt > > > --- > > > Kconfig | 8 > > > api/Kconfig | 11 ++- > > > 2 files changed, 10 insertions(+), 9 deletions(-) > > > > Did you put this through CI? It's possible that some envs don't do > > "loadaddr=CONFIG_STANDALONE_LOAD_ADDR" and not enable API stuff anymore, > > but I think that's why I did what I did when migrating. > > > > Hello Tom, > > we should keep the main Kconfig menu clean of detail settings. I don't thin > that there is an issue with the current patch. Yes, there's many ways Kconfig needs to be cleaned up, especially now that the migration of symbols from the board.h files is done. Some of the oddities were a result of symbol misuse/abuse, which can be fixed now. > STANDALONE_LOAD_ADDR is not used for loadaddr: > > $ git grep -n STANDALONE_LOAD_ADDR > (based on origin/master) > > api/Kconfig:15 > config STANDALONE_LOAD_ADDR > > config.mk:79 > export CONFIG_STANDALONE_LOAD_ADDR > > configs/display5_defconfig:33 > CONFIG_STANDALONE_LOAD_ADDR=0x10001000 > > configs/display5_factory_defconfig:30 > CONFIG_STANDALONE_LOAD_ADDR=0x10001000 > > configs/microchip_mpfs_icicle_defconfig:15 > CONFIG_STANDALONE_LOAD_ADDR=0x8020 > > configs/qemu-riscv32_defconfig:12 > CONFIG_STANDALONE_LOAD_ADDR=0x8020 > > configs/qemu-riscv32_smode_defconfig:13 > CONFIG_STANDALONE_LOAD_ADDR=0x8020 > > configs/qemu-riscv32_spl_defconfig:15 > CONFIG_STANDALONE_LOAD_ADDR=0x8020 > > configs/qemu-riscv64_defconfig:12 > CONFIG_STANDALONE_LOAD_ADDR=0x8020 > > configs/qemu-riscv64_smode_defconfig:13 > CONFIG_STANDALONE_LOAD_ADDR=0x8020 > > configs/qemu-riscv64_spl_defconfig:14 > CONFIG_STANDALONE_LOAD_ADDR=0x8020 > > configs/sifive_unleashed_defconfig:21 > CONFIG_STANDALONE_LOAD_ADDR=0x8020 > > configs/sifive_unmatched_defconfig:24 > CONFIG_STANDALONE_LOAD_ADDR=0x8020 > > configs/xtfpga_defconfig:12 > CONFIG_STANDALONE_LOAD_ADDR=0x0080 > > examples/standalone/Makefile:45 > LDFLAGS_STANDALONE += -Ttext $(CONFIG_STANDALONE_LOAD_ADDR) > > tools/patman/test_checkpatch.py:208 > CONFIG_STANDALONE_LOAD_ADDR > > With the patch applied > https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/15474 > showed no issues. Thanks for checking. In general, a CI run for making symbols less visible will make life easier on me when merging. -- Tom signature.asc Description: PGP signature
[PATCH] nvedit: simplify do_env_indirect()
Instead of calling env_get(from) up to three times, just do it once, computing the value we will put into 'to' and error out if that is NULL (i.e. no 'from' variable and no default provided). No functional change. Signed-off-by: Rasmus Villemoes --- cmd/nvedit.c | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/cmd/nvedit.c b/cmd/nvedit.c index 53e6b57b60..4844eb7f0c 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -1026,6 +1026,7 @@ static int do_env_indirect(struct cmd_tbl *cmdtp, int flag, char *from = argv[2]; char *default_value = NULL; int ret = 0; + char *val; if (argc < 3 || argc > 4) { return CMD_RET_USAGE; @@ -1035,18 +1036,14 @@ static int do_env_indirect(struct cmd_tbl *cmdtp, int flag, default_value = argv[3]; } - if (env_get(from) == NULL && default_value == NULL) { + val = env_get(from) ?: default_value; + if (!val) { printf("## env indirect: Environment variable for (%s) does not exist.\n", from); return CMD_RET_FAILURE; } - if (env_get(from) == NULL) { - ret = env_set(to, default_value); - } - else { - ret = env_set(to, env_get(from)); - } + ret = env_set(to, val); if (ret == 0) { return CMD_RET_SUCCESS; -- 2.37.2
Re: [PATCH] ARM: imx: Include on-SoM microSD in list of i.MX6 DHCOM boot devices
On 05/03/2023 20:21, Marek Vasut wrote: Add mmc1, which is mapped to optional on-SoM microSD socket, to the list of distro boot command boot devices. Signed-off-by: Marek Vasut Reviewed-by: Fabio Estevam
Re: [PATCH] ARM: dts: imx: Add WDT bindings on DH i.MX6 DHSOM
On 05/03/2023 17:49, Marek Vasut wrote: Add WDT reboot bindings on DH i.MX6 DHSOM to permit the platform to reboot via WDT in U-Boot. These are custom U-Boot bindings, hence they are placed in -u-boot.dtsi . Signed-off-by: Marek Vasut Reviewed-by: Fabio Estevam
Re: [PATCH] ARM: imx: Convert DH i.MX6 DHSOM to DM_SERIAL
On 05/03/2023 17:48, Marek Vasut wrote: Enable CONFIG_DM_SERIAL on DH i.MX6 DHSOM to convert it to DM serial . Signed-off-by: Marek Vasut Reviewed-by: Fabio Estevam
Re: [PATCH RFC u-boot-mvebu 0/2] arm: mvebu: Fix eMMC boot
On Monday 06 March 2023 11:15:35 Martin Rowe wrote: > On Sun, 5 Mar 2023 at 16:04, Pali Rohár wrote: > > > On Sunday 05 March 2023 12:46:34 Pali Rohár wrote: > > > On Sunday 05 March 2023 02:24:27 Martin Rowe wrote: > > > > On Sat, 4 Mar 2023 at 10:40, Pali Rohár wrote: > > > > > > > > > Boot configuration stored in EXT_CSC register is completely ignored > > by > > > > > BootROM: > > > > > > > > > > > > https://lore.kernel.org/u-boot/CAOAjy5SYPPzWKok-BSGYwZwcKOQt_aZPgh6FTbrFd3F=8dm...@mail.gmail.com/ > > > > > > > > > > Reflect this eMMC booting in documentation and in the code. > > > > > > > > > > Martin, can you test this patch series if SPL and main U-Boot is > > loaded > > > > > from the same eMMC HW partition? > > > > > > > > > > > > > boot0: u-boot > > > > > > > > Works fine, no issues. > > > > > > > > > > > > boot0: zeroed > > > > boot1: u-boot > > > > user: zeroed > > > > > > > > It succeeds, eventually... > > > > == > > > > BootROM - 1.73 > > > > > > > > Booting from MMC > > > > BootROM: Bad header at offset > > > > BootROM: Bad header at offset 0020 > > > > Switching BootPartitions. > > > > > > > > U-Boot SPL 2023.04-rc3-00159-gd1653548d2-dirty (Mar 05 2023 - 11:50:45 > > > > +1000) > > > > High speed PHY - Version: 2.0 > > > > EEPROM TLV detection failed: Using static config for Clearfog Pro. > > > > Detected Device ID 6828 > > > > board SerDes lanes topology details: > > > > | Lane # | Speed | Type | > > > > > > > > | 0| 3 | SATA0 | > > > > | 1| 0 | SGMII1 | > > > > | 2| 5 | PCIe1 | > > > > | 3| 5 | USB3 HOST1 | > > > > | 4| 5 | PCIe2 | > > > > | 5| 0 | SGMII2 | > > > > > > > > High speed PHY - Ended Successfully > > > > mv_ddr: 14.0.0 > > > > DDR3 Training Sequence - Switching XBAR Window to FastPath Window > > > > mv_ddr: completed successfully > > > > Trying to boot from MMC1 > > > > ERROR: Invalid kwbimage v1 > > > > mmc_load_image_raw_sector: mmc block read error > > > > spl: mmc: wrong boot mode > > > > Trying to boot from BOOTROM > > > > Returning to BootROM (return address 0x05c4)... > > > > Timeout waiting card ready > > > > BootROM: Image checksum verification PASSED > > > > > > > > > > > > U-Boot 2023.04-rc3-00159-gd1653548d2-dirty (Mar 05 2023 - 11:50:45 > > +1000) > > > > > > > > SoC: MV88F6828-A0 at 1600 MHz > > > > DRAM: 1 GiB (800 MHz, 32-bit, ECC not enabled) > > > > Core: 38 devices, 22 uclasses, devicetree: separate > > > > MMC: mv_sdh: 0 > > > > Loading Environment from MMC... *** Warning - bad CRC, using default > > > > environment > > > > > > > > Model: SolidRun Clearfog A1 > > > > Board: SolidRun Clearfog Pro > > > > Net: > > > > Warning: ethernet@7 (eth1) using random MAC address - > > 12:07:8b:f9:7a:6f > > > > eth1: ethernet@7 > > > > Warning: ethernet@3 (eth2) using random MAC address - > > ee:ed:f3:bb:c2:af > > > > , eth2: ethernet@3 > > > > Warning: ethernet@34000 (eth3) using random MAC address - > > ae:34:b9:bb:28:c6 > > > > , eth3: ethernet@34000 > > > > Hit any key to stop autoboot: 0 > > > > => > > > > == > > > > > > > > Between "Returning to BootROM" and "Timeout waiting card ready" takes > > > > around 315 seconds. That's long enough that I thought it had hung > > > > completely and I only noticed it continue because I left it running > > while > > > > working on something else. I tried several things to reduce this > > timeout, > > > > including reverting to the "non-removable" dts for shdci, but nothing > > seems > > > > to affect it. > > > > > > Ok. So now it is in the state that it is working but is slow. Better > > > than nothing. > > > > > > Message "Returning to BootROM" is printed by SPL and message > > > "Timeout waiting card ready" is printed by BootROM. After printing > > > "Returning to BootROM" is SPL jumping back to the BootROM so the delay > > > is for sure in the BootROM. So seems that SPL reconfigures eMMC into > > > state in which BootROM cannot work with it. Something timeouts, BootROM > > > reconfigure/retry it and then it work again. It would be needed to > > > investigate what is happening here. My guess is that this could have > > > something with eMMC HW partition access, and code for switching > > > partitions near SPL MMCSD_MODE_EMMCBOOT. > > > > Try this change? > > > > diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c > > index b20eac3dcd38..eb59c41a824e 100644 > > --- a/arch/arm/mach-mvebu/spl.c > > +++ b/arch/arm/mach-mvebu/spl.c > > @@ -11,6 +11,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -297,11 +298,33 @@ u32 spl_boot_device(void) > > > > #endif > > > > +void restore_emmc_boot_part_config(void) > > +{ > > +#ifdef CONFIG_SPL_MMC > > + struct mmc *mmc; > > + int
Please pull u-boot-marvell/master
Hi Tom, please pull this small Marvell related fix: - mvebu: Use 4K sector for Thecus N2350 SPI flash (Tony) Here the Azure build, without any issues: https://dev.azure.com/sr0718/u-boot/_build/results?buildId=287=results Thanks, Stefan The following changes since commit 33fb2d130e28982b488c2a54978031835ed2aa71: Merge tag 'dm-pull-29feb23' of https://source.denx.de/u-boot/custodians/u-boot-dm (2023-03-01 16:07:24 -0500) are available in the Git repository at: g...@source.denx.de:u-boot/custodians/u-boot-marvell.git for you to fetch changes up to aed49a05c71897f787f54a204bb5bf5e620c81fc: arm: mvebu: Use 4K sector for Thecus N2350 SPI flash (2023-03-06 10:16:07 +0100) Tony Dinh (1): arm: mvebu: Use 4K sector for Thecus N2350 SPI flash configs/n2350_defconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Re: [PATCH] arm: mvebu: Use 4K sector for Thecus N2350 SPI flash
On 2/17/23 04:34, Tony Dinh wrote: Since the SPI flash chip mx25l3205d on this board has 4K-sector capability, enable it for the envs. Signed-off-by: Tony Dinh Applied to u-boot-marvell/master Thanks, Stefan --- configs/n2350_defconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configs/n2350_defconfig b/configs/n2350_defconfig index dcb2c96791..b85ef0dfeb 100644 --- a/configs/n2350_defconfig +++ b/configs/n2350_defconfig @@ -14,7 +14,7 @@ CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xff CONFIG_TARGET_N2350=y CONFIG_ENV_SIZE=0x1 CONFIG_ENV_OFFSET=0x10 -CONFIG_ENV_SECT_SIZE=0x1 +CONFIG_ENV_SECT_SIZE=0x1000 CONFIG_DEFAULT_DEVICE_TREE="armada-385-thecus-n2350" CONFIG_SPL_TEXT_BASE=0x4030 CONFIG_SYS_PROMPT="N2350 > " Viele Grüße, Stefan Roese -- DENX Software Engineering GmbH, Managing Director: Erika Unter 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 RFC u-boot-mvebu 0/2] arm: mvebu: Fix eMMC boot
On Sun, 5 Mar 2023 at 16:04, Pali Rohár wrote: > On Sunday 05 March 2023 12:46:34 Pali Rohár wrote: > > On Sunday 05 March 2023 02:24:27 Martin Rowe wrote: > > > On Sat, 4 Mar 2023 at 10:40, Pali Rohár wrote: > > > > > > > Boot configuration stored in EXT_CSC register is completely ignored > by > > > > BootROM: > > > > > > > > > https://lore.kernel.org/u-boot/CAOAjy5SYPPzWKok-BSGYwZwcKOQt_aZPgh6FTbrFd3F=8dm...@mail.gmail.com/ > > > > > > > > Reflect this eMMC booting in documentation and in the code. > > > > > > > > Martin, can you test this patch series if SPL and main U-Boot is > loaded > > > > from the same eMMC HW partition? > > > > > > > > > > boot0: u-boot > > > > > > Works fine, no issues. > > > > > > > > > boot0: zeroed > > > boot1: u-boot > > > user: zeroed > > > > > > It succeeds, eventually... > > > == > > > BootROM - 1.73 > > > > > > Booting from MMC > > > BootROM: Bad header at offset > > > BootROM: Bad header at offset 0020 > > > Switching BootPartitions. > > > > > > U-Boot SPL 2023.04-rc3-00159-gd1653548d2-dirty (Mar 05 2023 - 11:50:45 > > > +1000) > > > High speed PHY - Version: 2.0 > > > EEPROM TLV detection failed: Using static config for Clearfog Pro. > > > Detected Device ID 6828 > > > board SerDes lanes topology details: > > > | Lane # | Speed | Type | > > > > > > | 0| 3 | SATA0 | > > > | 1| 0 | SGMII1 | > > > | 2| 5 | PCIe1 | > > > | 3| 5 | USB3 HOST1 | > > > | 4| 5 | PCIe2 | > > > | 5| 0 | SGMII2 | > > > > > > High speed PHY - Ended Successfully > > > mv_ddr: 14.0.0 > > > DDR3 Training Sequence - Switching XBAR Window to FastPath Window > > > mv_ddr: completed successfully > > > Trying to boot from MMC1 > > > ERROR: Invalid kwbimage v1 > > > mmc_load_image_raw_sector: mmc block read error > > > spl: mmc: wrong boot mode > > > Trying to boot from BOOTROM > > > Returning to BootROM (return address 0x05c4)... > > > Timeout waiting card ready > > > BootROM: Image checksum verification PASSED > > > > > > > > > U-Boot 2023.04-rc3-00159-gd1653548d2-dirty (Mar 05 2023 - 11:50:45 > +1000) > > > > > > SoC: MV88F6828-A0 at 1600 MHz > > > DRAM: 1 GiB (800 MHz, 32-bit, ECC not enabled) > > > Core: 38 devices, 22 uclasses, devicetree: separate > > > MMC: mv_sdh: 0 > > > Loading Environment from MMC... *** Warning - bad CRC, using default > > > environment > > > > > > Model: SolidRun Clearfog A1 > > > Board: SolidRun Clearfog Pro > > > Net: > > > Warning: ethernet@7 (eth1) using random MAC address - > 12:07:8b:f9:7a:6f > > > eth1: ethernet@7 > > > Warning: ethernet@3 (eth2) using random MAC address - > ee:ed:f3:bb:c2:af > > > , eth2: ethernet@3 > > > Warning: ethernet@34000 (eth3) using random MAC address - > ae:34:b9:bb:28:c6 > > > , eth3: ethernet@34000 > > > Hit any key to stop autoboot: 0 > > > => > > > == > > > > > > Between "Returning to BootROM" and "Timeout waiting card ready" takes > > > around 315 seconds. That's long enough that I thought it had hung > > > completely and I only noticed it continue because I left it running > while > > > working on something else. I tried several things to reduce this > timeout, > > > including reverting to the "non-removable" dts for shdci, but nothing > seems > > > to affect it. > > > > Ok. So now it is in the state that it is working but is slow. Better > > than nothing. > > > > Message "Returning to BootROM" is printed by SPL and message > > "Timeout waiting card ready" is printed by BootROM. After printing > > "Returning to BootROM" is SPL jumping back to the BootROM so the delay > > is for sure in the BootROM. So seems that SPL reconfigures eMMC into > > state in which BootROM cannot work with it. Something timeouts, BootROM > > reconfigure/retry it and then it work again. It would be needed to > > investigate what is happening here. My guess is that this could have > > something with eMMC HW partition access, and code for switching > > partitions near SPL MMCSD_MODE_EMMCBOOT. > > Try this change? > > diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c > index b20eac3dcd38..eb59c41a824e 100644 > --- a/arch/arm/mach-mvebu/spl.c > +++ b/arch/arm/mach-mvebu/spl.c > @@ -11,6 +11,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -297,11 +298,33 @@ u32 spl_boot_device(void) > > #endif > > +void restore_emmc_boot_part_config(void) > +{ > +#ifdef CONFIG_SPL_MMC > + struct mmc *mmc; > + int ret; > + > + mmc = find_mmc_device(0); > + if (!mmc || !mmc->has_init || mmc->part_config == > MMCPART_NOAVAILABLE) > + return; > + > + ret = mmc_set_part_conf(mmc, > + EXT_CSD_EXTRACT_BOOT_ACK(mmc->part_config), > + EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config), > +
Re: [PATCH 1/1] api: move API related config options into submenu
On 3/4/23 16:32, Tom Rini wrote: On Fri, Mar 03, 2023 at 11:31:22PM +0100, Heinrich Schuchardt wrote: Kconfig settings that are related to the API for standalone applications should be in the API sub-menu and not on the top level. CONFIG_STANDALONE_LOAD_ADDR is only relevant if standalone example applications are built. Signed-off-by: Heinrich Schuchardt --- Kconfig | 8 api/Kconfig | 11 ++- 2 files changed, 10 insertions(+), 9 deletions(-) Did you put this through CI? It's possible that some envs don't do "loadaddr=CONFIG_STANDALONE_LOAD_ADDR" and not enable API stuff anymore, but I think that's why I did what I did when migrating. Hello Tom, we should keep the main Kconfig menu clean of detail settings. I don't thin that there is an issue with the current patch. STANDALONE_LOAD_ADDR is not used for loadaddr: $ git grep -n STANDALONE_LOAD_ADDR (based on origin/master) api/Kconfig:15 config STANDALONE_LOAD_ADDR config.mk:79 export CONFIG_STANDALONE_LOAD_ADDR configs/display5_defconfig:33 CONFIG_STANDALONE_LOAD_ADDR=0x10001000 configs/display5_factory_defconfig:30 CONFIG_STANDALONE_LOAD_ADDR=0x10001000 configs/microchip_mpfs_icicle_defconfig:15 CONFIG_STANDALONE_LOAD_ADDR=0x8020 configs/qemu-riscv32_defconfig:12 CONFIG_STANDALONE_LOAD_ADDR=0x8020 configs/qemu-riscv32_smode_defconfig:13 CONFIG_STANDALONE_LOAD_ADDR=0x8020 configs/qemu-riscv32_spl_defconfig:15 CONFIG_STANDALONE_LOAD_ADDR=0x8020 configs/qemu-riscv64_defconfig:12 CONFIG_STANDALONE_LOAD_ADDR=0x8020 configs/qemu-riscv64_smode_defconfig:13 CONFIG_STANDALONE_LOAD_ADDR=0x8020 configs/qemu-riscv64_spl_defconfig:14 CONFIG_STANDALONE_LOAD_ADDR=0x8020 configs/sifive_unleashed_defconfig:21 CONFIG_STANDALONE_LOAD_ADDR=0x8020 configs/sifive_unmatched_defconfig:24 CONFIG_STANDALONE_LOAD_ADDR=0x8020 configs/xtfpga_defconfig:12 CONFIG_STANDALONE_LOAD_ADDR=0x0080 examples/standalone/Makefile:45 LDFLAGS_STANDALONE += -Ttext $(CONFIG_STANDALONE_LOAD_ADDR) tools/patman/test_checkpatch.py:208 CONFIG_STANDALONE_LOAD_ADDR With the patch applied https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/15474 showed no issues. Best regards Heinrich
Re: [PATCH 09/10] efi: Support showing tables
On 2/26/23 02:33, Simon Glass wrote: Add a command to display the tables provided by EFI. Signed-off-by: Simon Glass --- cmd/efi.c | 40 +++- doc/usage/cmd/efi.rst | 22 ++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/cmd/efi.c b/cmd/efi.c index c0384e0db28..86988d9e9e2 100644 --- a/cmd/efi.c +++ b/cmd/efi.c @@ -7,10 +7,12 @@ #include #include #include +#include #include #include #include #include +#include #include DECLARE_GLOBAL_DATA_PTR; @@ -273,8 +275,43 @@ done: return ret ? CMD_RET_FAILURE : 0; } +static int do_efi_tables(struct cmd_tbl *cmdtp, int flag, int argc, +char *const argv[]) We already have function do_efi_show_tables() to print the list of EFI configuration tables. Please, avoid code duplication. +{ + struct efi_system_table *systab; + int i; + + if (IS_ENABLED(CONFIG_EFI_APP)) { + systab = efi_get_sys_table(); + if (!systab) { + printf("Cannot read system table\n"); + return CMD_RET_FAILURE; + } + } else { + int size; + int ret; + + ret = efi_info_get(EFIET_SYS_TABLE, (void **), ); + if (ret) { + printf("Cannot find EFI system table (err=%d)\n", ret); + return CMD_RET_FAILURE; + } + } + for (i = 0; i < systab->nr_tables; i++) { + struct efi_configuration_table *tab = >tables[i]; + char guid_str[37]; + + uuid_bin_to_str(tab->guid.b, guid_str, 1); + printf("%p %s %s\n", tab->table, guid_str, + uuid_guid_get_str(tab->guid.b)); do_efi_show_tables() does this with printf("%pUl (%pUs)\n",..). + } + + return 0; +} + static struct cmd_tbl efi_commands[] = { U_BOOT_CMD_MKENT(mem, 1, 1, do_efi_mem, "", ""), + U_BOOT_CMD_MKENT(tables, 1, 1, do_efi_tables, "", ""), }; static int do_efi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) @@ -298,5 +335,6 @@ static int do_efi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) U_BOOT_CMD( efi, 3, 1, do_efi, "EFI access", - "mem [all]Dump memory information [include boot services]" + "mem [all]Dump memory information [include boot services]\n" + "tables Dump tables" ); diff --git a/doc/usage/cmd/efi.rst b/doc/usage/cmd/efi.rst index c029c423879..2424d3bb587 100644 --- a/doc/usage/cmd/efi.rst +++ b/doc/usage/cmd/efi.rst @@ -10,6 +10,7 @@ Synopsis :: efi mem [all] +efi tables Description --- @@ -54,6 +55,14 @@ Attributes Shows a code for memory attributes. The key for this is shown below the table. +efi tables +~~ + +This shows a list of the EFI tables provided in the system table. These use +GUIDs so it is not possible in general to show the name of a table. But some +effort is made to provide a useful table, where the GUID is known by U-Boot. + + Example --- @@ -195,3 +204,16 @@ Example f: uncached, write-coalescing, write-through, write-back rf: uncached, write-coalescing, write-through, write-back, needs runtime mapping 1: uncached + + +=> efi tables +1f8eda98 ee4e5898-3914-4259-9d6e-dc7bd79403cf EFI_LZMA_COMPRESSED +1ff2ace0 05ad34ba-6f02-4214-952e-4da0398e2bb9 EFI_DXE_SERVICES +1f8ea018 7739f24c-93d7-11d4-9a3a-0090273fc14d EFI_HOB_LIST +1ff2bac0 4c19049f-4137-4dd3-9c10-8b97a83ffdfa EFI_MEMORY_TYPE +1ff2cb10 49152e77-1ada-4764-b7a2-7afefed95e8b This is what 'efi tables' / printf("%pUl (%pUs)\n",..). would output for an unknown table: 8868e871-e4f1-11d3-bc22-0080c73c8881 (8868e871-e4f1-11d3-bc22-0080c73c8881) Maybe uuid_guid_get_str() should return an empty string in case of an unknown GUID. +1f9ac018 060cc026-4c0d-4dda-8f41-595fef00a502 EFI_MEM_STATUS_CODE_REC +1f9ab000 eb9d2d31-2d88-11d3-9a16-0090273fc14d EFI_SMBIOS +1fb7e000 eb9d2d30-2d88-11d3-9a16-0090273fc14d EFI_GUID_EFI_ACPI1 +1fb7e014 8868e871-e4f1-11d3-bc22-0080c73c8881 EFI_GUID_EFI_ACPI2 +1e550018 dcfa911d-26eb-469f-a220-38b7dc461220 Who would care about the address of the table? Best regards Heinrich