Re: [U-Boot] What if ATF can be part of U-Boot source, like SPL?
Hi Simon and all, > I have been avoiding this thread but today I attended a talk on ATF and > ARM's approach in general. For the benefits of others, that was me talking at the OSFC conference (video and slides should appear soon here: https://osfc.io/talks/trustedfirmware-org-a-collaborative-effort-into-firmware-security-and-the-path-towards-standardized-open-source-firmware-on-arm). Had a chat with Simon and Jagan afterwards, so repeating my answers here for the records. > ARM seems to be moving towards an approach > of providing increasingly complex source code to add more layers of security > software to fully round out two mostly separate worlds (secure and normal). The requirement of having more complex source code running in the Secure world comes from our partners (SoC vendors, ODMs, OSVs...Google included). For example, multi-tenancy in the Secure world (the possibility of running multiple Trusted OSs owned by different vendors) on an Android phone is a real requirement raised by many manufacturers and by Google. Arm is trying to fulfil this requirement by providing both an architecture and a clean reference implementation to support this and to protect the Normal world from attacks (malicious code) running in the Secure world. Worth noticing that all will be developed, reviewed and tested in public as part of the TrustedFirmware.org project, the new "home" of Trusted Firmware-A (TF-A, the former Arm Trusted Firmware). > I came away with the impression that the two companies are going in > different directions. ARM seems to be heading where Intel was and Intel is > heading back. This is a gross simplification of course. As explained verbally, Arm is going into the same direction it has always gone: open source every piece of software running on an Arm platform (from the very first line of code), including all the Power Management software implementation (PSCI into Trusted Firmware, SCMI into the SCP firmware project - https://github.com/ARM-software/SCP-firmware) and encouraging all our partners to do the same for their platforms ports. You can run a full open source software stack on any Arm platform that has upstream support available and the direction is not going to change with more software running in the Secure world (Secure EL2 included)...it will just be some more open source software. > 1. ARM releases new ATF versions as source drops that firmware projects like > U-Boot expected to take. In fact, not even U-Boot...this is just users picking > up whatever they find That's an accurate description (apart from the fact that releases will be tagged by the TrustedFirmware.org community project, not by Arm as such). We have the Linux Kernel model in mind (despite the BSD license topic) and the requirement to reliably point to a precise Trusted Firmware version running on an Arm device, as much as you do with the Kernel. This is practically impossible today due to the number of different implementations deployed on every device by different manufacturers. And that's the problem we are trying to fix. > 2. ATF keeps getting more complex, adding its own drivers for serial, storage, > etc., duplicating and perhaps conflicting with those in U-Boot Apart from platforms support, the additional software to be developed in the Secure world is meant to implement Arm architecture and Arm specifications, not to bloat the code with unnecessary drivers...moreover, as Andre pointed out, Trusted Firmware runs on many non-U-Boot platforms, so there *might* be overlaps for things that are needed elsewhere. > 3. Over time we end up with binary blobs on ARM that are every bit as > impenetrable as what Intel used to have > 4. Firmware security and open-sourceness suffers, overall. That's definitely not our goal. We'd like to ease the audit/certification process and ultimately the security of the highest privileged firmware running on Arm, by encouraging our partners to adopt a clean EL3 (and in the future Secure EL2) generic firmware (ideally a specific x.y version) and moving all their proprietary/platform code elsewhere in lowest exception levels. That's the goal of the architecture we are working on (described in my talk) and we believe it would ultimately help the Arm firmware security ecosystem in general. If by doing so, partners will still end up deploying binary blobs (signed, certified, whatever), these would be built from the corresponding open source project. I don't see how the situation would be any worst that it is today with proprietary EL3 firmware binaries deployed everywhere (despite all the originating core code being upstream!). From my point of view, it could only evolve towards a more open situation...every partner that we gain on this approach will be a partner bought into more open firmware and increased security. > But in the meantime, I think it makes more sense to incorporate the relevant > parts of ATF into U-Boot and maintain them
Re: [U-Boot] What if ATF can be part of U-Boot source, like SPL?
On 04/09/2019 18:56, Marek Vasut wrote: > On 9/4/19 7:32 PM, Andre Przywara wrote: Hi Marek, >>> I have been avoiding this thread but today I attended a talk on ATF >>> and ARM's approach in general. ARM seems to be moving towards an >>> approach of providing increasingly complex source code to add more >>> layers of security software to fully round out two mostly separate >>> worlds (secure and normal). >>> >>> Oddly enough Intel was also there talking about how they are breaking >>> things down into pieces and slowly releasing more things into the open >>> source world. >>> >>> I came away with the impression that the two companies are going in >>> different directions. ARM seems to be heading where Intel was and >>> Intel is heading back. This is a gross simplification of course. >>> >>> To me it seems that the following scenario might play out: >>> >>> 1. ARM releases new ATF versions as source drops that firmware >>> projects like U-Boot expected to take. In fact, not even U-Boot...this >>> is just users picking up whatever they find >>> >>> 2. ATF keeps getting more complex, adding its own drivers for serial, >>> storage, etc., duplicating and perhaps conflicting with those in >>> U-Boot >> >> A bit like U-Boot, ATF can look quite differently on the various platforms: >> - There are platforms like Arm's Juno or the fastmodel, where ATF does some >> loading. This is mostly for features like secure boot or secure payloads >> (OP-Tee). Typically we don't even use U-Boot primarily on those platforms >> (but EDK II instead). >> - There are platforms (low end SoCs like Allwinner, Rockchip, for instance) >> which don't do any loading at all, actually rely on other code (SPL?) to >> load ATF itself. >> >> So I don't see how this would duplicate effort or even conflict. > > So why don't we put all the platform init code into U-Boot SPL, which > already has all the loading code and only load this ATF BL31 with SPL? But this is what we actually do on those platforms: SPL is loaded by the BootROM, PLLs and bus clocks are set up in SPL, then DRAM and MMC are initialised, then we load U-Boot proper and ATF BL31. BL31 does SMP init and errata workaround, and stays resident in EL3 to provide PSCI services. This BL31-only model is considered a perfectly fine way of using ATF. >>> 3. Over time we end up with binary blobs on ARM that are every bit as >>> impenetrable as what Intel used to have >> >> I am not quite sure I follow how you come from 2. to 3. Was there something >> in the presentation (I guess OSFC?) you are referring to? >> In contrast to the x86 world the firmware is actually Open Source based, so >> you actually have a realistic chance to rebuild and/or verify it. If this is >> not true in a particular case, poke your vendor. > > I thought ATF was designed to be BSD-licensed to allow vendors to take > all the open source contributions, benefit from it, but also allow the > vendors to never release their changes if they so desire. BSD was simply chosen because basically every vendor said they would not (be able to) use GPL licensed code for their firmware. So it was BSD license or no (Open Source) firmware at all. You might find that sad (I do), but that's what it is. > It seems like some platforms even went down this path already and only > released blobs, hence, no security fixes etc. If you don't control the platform (servers), then you still have some power as a customer to ask for releasing the firmware source. AFAIK this is well understood by the vendors, as it is a good selling point against x86. And what I meant above is that this is in contrast to x86, where most of the firmware ("BIOS") is proprietary and consists of many third-party parts. So even when your system vendor wanted, it couldn't possibly publish the code. With the core firmware being Open Source, you have a much better chance of getting the source. If the vendor doesn't want to give code away, it would just use proprietary code anyway, probably of much worse quality. >>> 4. Firmware security and open-sourceness suffers, overall. >>> >>> One approach might be for ATF to split into a library which can be >>> linked into a new U-Boot phase and a driver/init bit that could be >>> used for other bootloaders, whatever they might be. >>> >>> But in the meantime, I think it makes more sense to incorporate the >>> relevant parts of ATF into U-Boot and maintain them there, only for >>> the platforms that U-Boot supports. If ATF starts using GUIDs and the >>> like, we at least have somewhere to hide :-) >> >> What are the relevant parts, exactly? Do you want to rip them out? Use git >> submodules? >> For reasons I detailed before, I doubt the viability of the practical aspect >> alone. >> >> But on top of that, I see that ATF and U-Boot address two separate areas: >> CPU initialisation and secure world management on one side, and a very >> versatile bootloader running in the non-secure world on the other. Keeping >>
Re: [U-Boot] [PATCH v2 3/3] doc: lion_rk3368: use idbloader.img for rk3368
On 2019/9/4 上午12:29, Matwey V. Kornilov wrote: Makefile now produces ready-to-deploy idbloader.img file. Signed-off-by: Matwey V. Kornilov Reviewed-by: Kever Yang Thanks, - Kever --- board/theobroma-systems/lion_rk3368/README | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/board/theobroma-systems/lion_rk3368/README b/board/theobroma-systems/lion_rk3368/README index 83e4332984..ad3ac93bd4 100644 --- a/board/theobroma-systems/lion_rk3368/README +++ b/board/theobroma-systems/lion_rk3368/README @@ -18,8 +18,6 @@ Build the TPL/SPL stage === > make CROSS_COMPILE=aarch64-unknown-elf- ARCH=arm - > tools/mkimage -n rk3368 -T rksd -d tpl/u-boot-tpl.bin spl-3368.img - > cat spl/u-boot-spl-dtb.bin >> spl-3368.img Build the full U-Boot and a FIT image including the ATF === @@ -35,7 +33,7 @@ Copy the SPL to offset 32k and the FIT image containing the payloads SD-Card --- - > dd if=spl-3368.img of=/dev/sdb seek=64 + > dd if=idbloader.img of=/dev/sdb seek=64 > dd if=u-boot.itb of=/dev/sdb seek=512 eMMC ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/3] doc: rockchip: use idbloader.img for rk3288, rk3328, rk3399
On 2019/9/4 上午12:29, Matwey V. Kornilov wrote: Makefile now produces ready-to-deploy idbloader.img file. Signed-off-by: Matwey V. Kornilov Reviewed-by: Kever Yang Thanks, - Kever --- doc/README.rockchip | 21 ++--- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/doc/README.rockchip b/doc/README.rockchip index 7d4dc1b33b..6007f3321e 100644 --- a/doc/README.rockchip +++ b/doc/README.rockchip @@ -276,9 +276,7 @@ As of now TPL is added on Vyasa-RK3288 board. To write an image that boots from an SD card (assumed to be /dev/mmcblk0): - ./tools/mkimage -n rk3288 -T rksd -d ./tpl/u-boot-tpl.bin out && -cat ./spl/u-boot-spl-dtb.bin >> out && -sudo dd if=out of=/dev/mmcblk0 seek=64 && +sudo dd if=idbloader.img of=/dev/mmcblk0 seek=64 && sudo dd if=u-boot-dtb.img of=/dev/mmcblk0 seek=16384 Booting from an SD card on RK3188 @@ -311,11 +309,6 @@ Booting from an SD card on Pine64 Rock64 (RK3328) For Rock64 rk3328 board the following three parts are required: TPL, SPL, and the u-boot image tree blob. - - Create TPL/SPL image - -=> tools/mkimage -n rk3328 -T rksd -d tpl/u-boot-tpl.bin idbloader.img -=> cat spl/u-boot-spl.bin >> idbloader.img - - Write TPL/SPL image at 64 sector => sudo dd if=idbloader.img of=/dev/mmcblk0 seek=64 @@ -474,19 +467,9 @@ Hit any key to stop autoboot: 0 Option 3: Package the image with TPL: - - Prefix rk3399 header to TPL image - -=> cd /path/to/u-boot -=> ./tools/mkimage -n rk3399 -T rksd -d tpl/u-boot-tpl-dtb.bin out - - - Concatinate tpl with spl - -=> cd /path/to/u-boot -=> cat ./spl/u-boot-spl-dtb.bin >> out - - Write tpl+spl at 64th sector -=> sudo dd if=out of=/dev/sdc seek=64 +=> sudo dd if=idbloader.img of=/dev/sdc seek=64 - Write U-Boot proper at 16384 sector ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] What if ATF can be part of U-Boot source, like SPL?
On Wed, Sep 04, 2019 at 11:51:41AM +0900, Masahiro Yamada wrote: > On Sun, Jun 30, 2019 at 11:20 PM Marek Vasut wrote: > > > > On 6/30/19 4:17 PM, Tom Rini wrote: > > > On Sun, Jun 30, 2019 at 04:03:52PM +0200, Marek Vasut wrote: > > >> On 6/30/19 3:57 PM, Tom Rini wrote: > > >>> On Sat, Jun 29, 2019 at 08:32:00PM +0530, Jagan Teki wrote: > > >>> > > In terms of code maintenance and development feasibility it is always > > a better approach to have out-of-tree code or binary to be part of > > in-house source tree. > > > > This is what exactly it was done for SPL, if I'm not wrong. So can we > > do the same thing for ATF on ARM64 SoCs? > > > > We are using ATF (on Allwinner) to switch EL3 to EL2 for start loading > > U-Boot proper and minimal PSCI, PMIC initialization. So assuming the > > functionality of ATF (like here) is limited so the code it require can > > be limited too, so why can't this code to be part of U-Boot tree? > > > > This would ultimately avoid out-off-tree ATF builds with associated > > variable exporting during u-boot builds. > > > > More over this idea would also help to design a single-step bootloader > > where it can't depends on out-of-tree sources. > > > > Code sync from ATF source to U-Boot can be possible in-terms licensing > > point-of-view since ATF licensed under BSD-3-Clause. > > > > I'm thinking this can be a worth-idea to look at it and I'm sure It > > may require some hard changes and other things to consider but just > > posted to understand how hard or feasible or meaningful it is? > > > > Feel free for any comments? > > >>> > > >>> Given that we have "TPL" and "SPL", my "pie in the sky" wish would be > > >>> for the ability to build different U-Boots to fill the different aspects > > >>> of the aarch64 boot flow. > > >>> > > >>> That said, patches that would in turn allow for users to locally add ATF > > >>> as a git submodule and then build that, if cleanly done, could be > > >>> interesting. But must not impact the normal build flow. > > >> > > >> So can we also add Linux as a submodule ? And glibc ? Maybe busybox too ? > > > > > > Just as you suggested Jagan look at other SoCs and how they assemble > > > images, I think you also need to take a wider look around. The concept > > > of "take U-Boot, other firmware blobs, combine and mangle that" is > > > somewhat widely used. It's not just sunxi that spits out a "can't find > > > ATF, this image won't boot!" warning. > > > > So, U-Boot spits out that it cannot find kernel image and refuses to > > boot, do we also import Linux into the codebase because of that ? > > > > But Linux also spits that it cannot find init and refuses to boot. Do we > > import systemd/sysvinit/upstart/... because of that ? > > > > No, we do not. That's what build systems like OE or buildroot or whatnot > > are for. If you want to assemble your image by hand, that's also fine, > > surely you should be capable of replicating what the documentation / OE > > / buildroot recipe suggests. > > > I agree with Marek. > U-Boot should be independent. > > I do not like the git-submodule approach. > Jagan's proposal..., no way! To repeat myself, as I said in the thread at the time, I still do not believe integration of ATF sources in-to U-Boot to be the right approach. -- Tom signature.asc Description: PGP signature ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v3 0/3] spi: Split CONFIG_DM_SPI* to CONFIG_{SPL_TPL}DM_SPI*
This patch series introduces new SPL and TPL specific Kconfig entries for DM_SPI* options. Such change allows using the spi driver in SPL/TPL or U-Boot proper. First two patches - related to ls10{42}* NXP soc fix some issues with defining the DM_SPI* defines in .h file instead of Kconfig. This series doesn't introduce build breaks, but board maintainers are kindly asked to check if their boards still boots. Buildman setup for binary size regression checking: ./tools/buildman/buildman.py -b HEAD --count=4 ls1043 --output-dir=../BUILD/ --force-build -CveE ./tools/buildman/buildman.py -b HEAD --count=4 ls1043 --output-dir=../BUILD/ -Ssdel Changes in v3: - Provide better Kconfig flags conditionals to avoid not compiling relavant code (there is negative diff observed after applying those patches) - Fix da850evm_nand and da850evm by enabling SPL_DM_SPI support in Kconfig - Drop patch from the series [U-Boot,v2,1/4] kconfig: doc: Update comment regarding CONFIG_IS_ENABLED(FOO) for TPL as not being related to this particular change Changes in v2: - Resend patches with some not yet in mainline code removed as suggested by Adam Ford Lukasz Majewski (3): spi: Move DM_SPI_FLASH to Kconfig (for NXP's ls1043a) spi: Move DM_SPI_FLASH and SPI_FLASH_DATAFLASH to Kconfig (for ls1021aXXX) spi: Convert CONFIG_DM_SPI* to CONFIG_$(SPL_TPL_)DM_SPI* arch/arm/Kconfig| 24 ++-- board/l+g/vinco/vinco.c | 4 ++-- cmd/sf.c| 4 ++-- cmd/spi.c | 6 +++--- common/spl/Kconfig | 28 configs/am57xx_evm_defconfig| 2 ++ configs/am57xx_hs_evm_defconfig | 2 ++ configs/am57xx_hs_evm_usb_defconfig | 2 ++ configs/axm_defconfig | 2 ++ configs/chromebook_link64_defconfig | 2 ++ configs/chromebook_samus_tpl_defconfig | 4 configs/dra7xx_evm_defconfig| 2 ++ configs/dra7xx_hs_evm_defconfig | 2 ++ configs/dra7xx_hs_evm_usb_defconfig | 2 ++ configs/j721e_evm_a72_defconfig | 2 ++ configs/j721e_evm_r5_defconfig | 2 ++ configs/ls1021aiot_qspi_defconfig | 2 ++ configs/ls1021aiot_sdcard_defconfig | 2 ++ configs/ls1021aqds_qspi_defconfig | 1 + configs/ls1021aqds_sdcard_qspi_defconfig| 1 + configs/ls1021atwr_qspi_defconfig | 1 + configs/sama5d2_xplained_spiflash_defconfig | 2 ++ configs/sama5d3xek_spiflash_defconfig | 6 -- configs/sama5d4_xplained_spiflash_defconfig | 2 ++ configs/sama5d4ek_spiflash_defconfig| 2 ++ configs/stm32mp15_basic_defconfig | 2 ++ configs/taurus_defconfig| 2 ++ drivers/mtd/spi/Makefile| 4 ++-- drivers/mtd/spi/sf_probe.c | 8 drivers/net/fm/fm.c | 4 ++-- drivers/spi/Makefile| 2 +- drivers/spi/atmel_spi.c | 4 ++-- drivers/spi/davinci_spi.c | 6 +++--- drivers/spi/fsl_dspi.c | 5 +++-- drivers/spi/kirkwood_spi.c | 2 +- drivers/spi/mxc_spi.c | 6 +++--- drivers/spi/omap3_spi.c | 4 ++-- drivers/spi/sh_qspi.c | 4 ++-- env/sf.c| 2 +- include/configs/ls1021aiot.h| 6 -- include/configs/ls1021aqds.h| 8 include/configs/ls1021atwr.h| 5 - include/configs/ls1043a_common.h| 2 -- include/spi.h | 8 include/spi_flash.h | 2 +- test/dm/spi.c | 2 +- 46 files changed, 134 insertions(+), 63 deletions(-) -- 2.11.0 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v3 3/3] spi: Convert CONFIG_DM_SPI* to CONFIG_$(SPL_TPL_)DM_SPI*
This change allows more fine tuning of driver model based SPI support in SPL and TPL. It is now possible to explicitly enable/disable the DM_SPI support in SPL and TPL via Kconfig option. Before this change it was necessary to use: /* SPI Flash Configs */ #if defined(CONFIG_SPL_BUILD) #undef CONFIG_DM_SPI #undef CONFIG_DM_SPI_FLASH #undef CONFIG_SPI_FLASH_MTD #endif in the ./include/configs/.h, which is error prone and shall be avoided when we strive to switch to Kconfig. The goal of this patch: Provide distinction for DM_SPI support in both U-Boot proper and SPL (TPL). Valid use case is when U-Boot proper wants to use DM_SPI, but SPL must still support non DM driver. Another use case is the conversion of non DM/DTS SPI driver to support DM/DTS. When such driver needs to work in both SPL and U-Boot proper, the distinction is needed in Kconfig (also if SPL version of the driver supports OF_PLATDATA). In the end of the day one would have to support following use cases (in single driver file - e.g. mxs_spi.c): - U-Boot proper driver supporting DT/DTS - U-Boot proper driver without DT/DTS support (deprecated) - SPL driver without DT/DTS support - SPL (and TPL) driver with DT/DTS (when the SoC has enough resources to run full blown DT/DTS) - SPL driver with DT/DTS and SPL_OF_PLATDATA (when one have constrained environment with no fitImage and OF_LIBFDT support). Some boards do require SPI support (with DM) in SPL (TPL) and some only have DM_SPI{_FLASH} defined to allow compiling SPL. This patch converts #ifdef CONFIG_DM_SPI* to #if CONFIG_IS_ENABLED(DM_SPI) and provides corresponding defines in Kconfig. Signed-off-by: Lukasz Majewski Tested-by: Adam Ford #da850-evm --- Changes in v3: - Fix da850evm_nand and da850evm by enabling SPL_DM_SPI support in Kconfig - Drop patch from the series [U-Boot,v2,1/4] kconfig: doc: Update comment regarding CONFIG_IS_ENABLED(FOO) for TPL as not being related to this particular change Changes in v2: - Resend patches with some not yet in mainline code removed as suggested by Adam Ford === BIG FAT NOTE (REQUEST FOR TESTING): === The adjustments were done only to make the buildman / travis-CI build clean. Board maintainers are kindly asked to check if there are no failures on boards booting (or unintentional increase of binary size). Note: = Some boards require adjustments in ./configs/_defconfig (like Siemens). Other boards require modification in arch/arm/Kconfig (like MVEBU) Yet another ones - like ones from Samsung - doesn't use SPL at all. There are also boards - like i.MX{25678} - which (till now) doesn't use SPI drivers supporting DM in their SPL. Applied on top of -master branch: SHA1: 448f11f7503995746a7b71e5e3b3a831c4651be9 --- arch/arm/Kconfig| 14 ++ board/l+g/vinco/vinco.c | 4 ++-- cmd/sf.c| 4 ++-- cmd/spi.c | 6 +++--- common/spl/Kconfig | 28 configs/am57xx_evm_defconfig| 2 ++ configs/am57xx_hs_evm_defconfig | 2 ++ configs/am57xx_hs_evm_usb_defconfig | 2 ++ configs/axm_defconfig | 2 ++ configs/chromebook_link64_defconfig | 2 ++ configs/chromebook_samus_tpl_defconfig | 4 configs/dra7xx_evm_defconfig| 2 ++ configs/dra7xx_hs_evm_defconfig | 2 ++ configs/dra7xx_hs_evm_usb_defconfig | 2 ++ configs/j721e_evm_a72_defconfig | 2 ++ configs/j721e_evm_r5_defconfig | 2 ++ configs/ls1021aiot_qspi_defconfig | 2 ++ configs/ls1021aiot_sdcard_defconfig | 2 ++ configs/ls1021aqds_qspi_defconfig | 1 + configs/ls1021aqds_sdcard_qspi_defconfig| 1 + configs/ls1021atwr_qspi_defconfig | 1 + configs/sama5d2_xplained_spiflash_defconfig | 2 ++ configs/sama5d3xek_spiflash_defconfig | 6 -- configs/sama5d4_xplained_spiflash_defconfig | 2 ++ configs/sama5d4ek_spiflash_defconfig| 2 ++ configs/stm32mp15_basic_defconfig | 2 ++ configs/taurus_defconfig| 2 ++ drivers/mtd/spi/Makefile| 4 ++-- drivers/mtd/spi/sf_probe.c | 8 drivers/net/fm/fm.c | 4 ++-- drivers/spi/Makefile| 2 +- drivers/spi/atmel_spi.c | 4 ++-- drivers/spi/davinci_spi.c | 6 +++--- drivers/spi/fsl_dspi.c | 5 +++-- drivers/spi/kirkwood_spi.c | 2 +- drivers/spi/mxc_spi.c | 6 +++--- drivers/spi/omap3_spi.c | 4 ++-- drivers/spi/sh_qspi.c | 4 ++-- env/sf.c| 2 +- include/spi.h
[U-Boot] [PATCH v3 2/3] spi: Move DM_SPI_FLASH and SPI_FLASH_DATAFLASH to Kconfig (for ls1021aXXX)
This patch moves the CONFIG_DM_SPI_FLASH and CONFIG_SPI_FLASH_DATAFLASH to be defined in Kconfig, not in board specific header file (include/configs/.h). Before this change the CONFIG_DM_SPI_FLASH was not set in .config (so it was not possible to use CONFIG_IS_ENABLED(DM_SPI_FLASH) in SPI DM/DTS converted drivers), but it was set in u-boot.cfg file. Signed-off-by: Lukasz Majewski --- Changes in v3: None Changes in v2: None arch/arm/Kconfig | 6 -- include/configs/ls1021aiot.h | 6 -- include/configs/ls1021aqds.h | 8 include/configs/ls1021atwr.h | 5 - 4 files changed, 4 insertions(+), 21 deletions(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index b13173d7a0..490f2abcf1 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1353,6 +1353,8 @@ config TARGET_LS1021AQDS select SUPPORT_SPL select SYS_FSL_DDR select FSL_DDR_INTERACTIVE + select DM_SPI_FLASH if FSL_DSPI || FSL_QSPI + select SPI_FLASH_DATAFLASH if FSL_DSPI || FSL_QSPI imply SCSI config TARGET_LS1021ATWR @@ -1366,6 +1368,7 @@ config TARGET_LS1021ATWR select CPU_V7_HAS_VIRT select LS1_DEEP_SLEEP select SUPPORT_SPL + select DM_SPI_FLASH if FSL_DSPI || FSL_QSPI imply SCSI config TARGET_LS1021ATSN @@ -1390,6 +1393,7 @@ config TARGET_LS1021AIOT select CPU_V7_HAS_NONSEC select CPU_V7_HAS_VIRT select SUPPORT_SPL + select DM_SPI_FLASH if FSL_DSPI || FSL_QSPI imply SCSI help Support for Freescale LS1021AIOT platform. @@ -1825,5 +1829,3 @@ config SPL_LDSCRIPT default "arch/arm/cpu/arm926ejs/mxs/u-boot-spl.lds" if (ARCH_MX23 || ARCH_MX28) && !SPL_FRAMEWORK default "arch/arm/cpu/arm1136/u-boot-spl.lds" if CPU_ARM1136 default "arch/arm/cpu/armv8/u-boot-spl.lds" if ARM64 - - diff --git a/include/configs/ls1021aiot.h b/include/configs/ls1021aiot.h index ee570bc1a9..0ab1fb17a6 100644 --- a/include/configs/ls1021aiot.h +++ b/include/configs/ls1021aiot.h @@ -139,12 +139,6 @@ #define CONFIG_SPI_FLASH_SPANSION #endif -/* DM SPI */ -#if defined(CONFIG_FSL_DSPI) || defined(CONFIG_FSL_QSPI) -#define CONFIG_CMD_SF -#define CONFIG_DM_SPI_FLASH -#endif - /* * eTSEC */ diff --git a/include/configs/ls1021aqds.h b/include/configs/ls1021aqds.h index 66771e279b..e2529dfb9c 100644 --- a/include/configs/ls1021aqds.h +++ b/include/configs/ls1021aqds.h @@ -363,14 +363,6 @@ unsigned long get_board_ddr_clk(void); #define QSPI0_AMBA_BASE0x4000 #define FSL_QSPI_FLASH_SIZE(1 << 24) #define FSL_QSPI_FLASH_NUM 2 - -/* DSPI */ - -/* DM SPI */ -#if defined(CONFIG_FSL_DSPI) || defined(CONFIG_FSL_QSPI) -#define CONFIG_DM_SPI_FLASH -#define CONFIG_SPI_FLASH_DATAFLASH -#endif #endif /* diff --git a/include/configs/ls1021atwr.h b/include/configs/ls1021atwr.h index 31abee81ed..397afb668b 100644 --- a/include/configs/ls1021atwr.h +++ b/include/configs/ls1021atwr.h @@ -238,11 +238,6 @@ /* DSPI */ #endif -/* DM SPI */ -#if defined(CONFIG_FSL_DSPI) || defined(CONFIG_FSL_QSPI) -#define CONFIG_DM_SPI_FLASH -#endif - /* * Video */ -- 2.11.0 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v3 1/3] spi: Move DM_SPI_FLASH to Kconfig (for NXP's ls1043a)
This patch fixes issue with defining the DM_SPI_FLASH in the configs/include/ instead of enabling this option in Kconfig. The problem is that CONFIG_IS_ENABLED(DM_SPI_FLASH) shows false as there is no DM_SPI_FLASH=y in .config (but the define is set in u-boot.cfg). As a result conversion of DM_SPI_FLASH to using CONFIG_IS_ENABLED() is not working properly. Signed-off-by: Lukasz Majewski --- Changes in v3: - Provide better Kconfig flags conditionals to avoid not compiling relavant code (there is negative diff observed after applying those patches) Changes in v2: None Buildman test code: ./tools/buildman/buildman.py -b HEAD --count=4 ls1043 --output-dir=../BUILD/ --force-build -CveE ./tools/buildman/buildman.py -b HEAD --count=4 ls1043 --output-dir=../BUILD/ -Ssdel --- arch/arm/Kconfig | 4 include/configs/ls1043a_common.h | 2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 3b0e315061..b13173d7a0 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1407,6 +1407,8 @@ config TARGET_LS1043AQDS select BOARD_LATE_INIT select SUPPORT_SPL select FSL_DDR_INTERACTIVE if !SPL + select FSL_DSPI if !SPL_NO_DSPI + select DM_SPI_FLASH if FSL_DSPI && !SPL_NO_DSPI imply SCSI imply SCSI_AHCI help @@ -1421,6 +1423,8 @@ config TARGET_LS1043ARDB select BOARD_EARLY_INIT_F select BOARD_LATE_INIT select SUPPORT_SPL + select FSL_DSPI if !SPL_NO_DSPI + select DM_SPI_FLASH if FSL_DSPI && !SPL_NO_DSPI help Support for Freescale LS1043ARDB platform. diff --git a/include/configs/ls1043a_common.h b/include/configs/ls1043a_common.h index 70447a2183..fd8bf705cd 100644 --- a/include/configs/ls1043a_common.h +++ b/include/configs/ls1043a_common.h @@ -165,9 +165,7 @@ /* DSPI */ #ifndef SPL_NO_DSPI -#define CONFIG_FSL_DSPI #ifdef CONFIG_FSL_DSPI -#define CONFIG_DM_SPI_FLASH #define CONFIG_SPI_FLASH_STMICRO /* cs0 */ #define CONFIG_SPI_FLASH_SST /* cs1 */ #define CONFIG_SPI_FLASH_EON /* cs2 */ -- 2.11.0 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] U-Boot: Environment flags broken for U-Boot
On Tue, Sep 03, 2019 at 06:03:40PM -0500, Joe Hershberger wrote: > On Tue, Sep 3, 2019 at 3:05 AM Wolfgang Denk wrote: > > > > Dear Tom, > > > > In message Heiko Schocher > > wrote: > > > > > > I am just testing U-Boot Environment flags and they do not work anymore > > > with > > > current mainline U-Boot ... > > ... > > > reason is your commit: > > > > > > commit 7d4776545b0f8a8827e5d061206faf61c9ba6ea9 > > > Author: Patrick Delaunay > > > Date: Thu Apr 18 17:32:49 2019 +0200 > > > > > > env: solve compilation error in SPL > > > > > > Looking into the history of this, I wonder if we could / should > > have prevented this. > > > > As far as I can see, Patrick's patch series has not been reviewed by > > others, probably because general intetest in STM32 is not that big > > at the moment. I can see no Acked-by:, Reviewed-by: nor Tested-by: > > tags - nothing. > > > > The whole patch series was then pulled from the u-boot-stm > > repository. > > > > > > However, there was not only STM related code in there. There were > > changes to common code like the environment handling. common code > > was changed without review and without testing. > > It seems this should be unacceptable even if it's in the area of > interest. Isn't an Acked-by generally accepted as required? > > > Are there ways to prevent this? > > > > Yes, we can appeal to the custodians to be more careful, but I > > assume they are already doing their best. > > It seems the diffstat should be a quick way to see this, so I would > think not quite their best. Maybe a reminder / recommendation that it > be reviewed by custodians? Part of the problem here is that yes, I need to rework my tooling a bit, and am. But another part of the problem is that this exact code area is not widely used. So the things that cause me concern didn't trigger. But looking at the code by itself, I would have acked it. It would have then been on noticing the size change and function-removal to see there's a not-so-obvious problem. > > It might have even been better if this had been a sub-system with a > > clear maintainer, but there is no such person for the environment > > code. > > > > How can we prevent this in the future? > > Is there any tooling around the MAINTAINERS file that can be used to > reg-flag PRs that contain changes outside of the tree's area of > effect? > > > Should we define "interested developers" for such areas that have no > > custodian (the "Designated reviewer") entry in the MAINTAINERS file > > could be used for this, for example)? > > I like that idea, though in this specific case I think there should be > a maintainer for env. I do wish we had a dedicated custodian for environment changes, yes. -- Tom signature.asc Description: PGP signature ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] U-Boot: Environment flags broken for U-Boot
On Wed, Sep 4, 2019 at 1:01 PM Tom Rini wrote: > > On Tue, Sep 03, 2019 at 10:04:42AM +0200, Wolfgang Denk wrote: > > Dear Tom, > > > > In message Heiko Schocher > > wrote: > > > > > > I am just testing U-Boot Environment flags and they do not work anymore > > > with > > > current mainline U-Boot ... > > ... > > > reason is your commit: > > > > > > commit 7d4776545b0f8a8827e5d061206faf61c9ba6ea9 > > > Author: Patrick Delaunay > > > Date: Thu Apr 18 17:32:49 2019 +0200 > > > > > > env: solve compilation error in SPL > > > > > > Looking into the history of this, I wonder if we could / should > > have prevented this. > > Looking over my scripts, yes, I overlooked the problem. The 'edison' > platform shows the same issue that Heiko's platform does but I > overlooked the size change. I'm modifying my script currently so it > will show more details and this should jump out more rather than the > size noise of "changes in a general area". Now interesting enough, > sandbox didn't blow up here but does also enable the env flags options. > > > As far as I can see, Patrick's patch series has not been reviewed by > > others, probably because general intetest in STM32 is not that big > > at the moment. I can see no Acked-by:, Reviewed-by: nor Tested-by: > > tags - nothing. > > > > The whole patch series was then pulled from the u-boot-stm > > repository. > > > > > > However, there was not only STM related code in there. There were > > changes to common code like the environment handling. common code > > was changed without review and without testing. > > To be clear, it was tested, but sadly the environment flags code is not > heavily used / enabled. More in a moment. > > > Are there ways to prevent this? > > > > Yes, we can appeal to the custodians to be more careful, but I > > assume they are already doing their best. > > > > It might have even been better if this had been a sub-system with a > > clear maintainer, but there is no such person for the environment > > code. > > > > How can we prevent this in the future? > > > > Should we define "interested developers" for such areas that have no > > custodian (the "Designated reviewer") entry in the MAINTAINERS file > > could be used for this, for example)? > > This, along with some other environment related patches were things I > was keeping an eye on to see if perhaps Joe would have had time to look > at before it went in (as the env flag stuff came from him). I also try I wasn't aware of it as I wasn't Cc'ed on this series. I generally don't have time to troll the list in general, which is a bit of a problem since I also missed the discussions on the UEFI env changes, some of which are already in, and are not how I would have implemented it. I only found out that it was in work from Grant Likely at his talk in San Diego. > and make use of the "Needs Review / ACK" flag in patchwork for things > that stand out. Looking over the merge contents again, that particular > one would not have. > > So, things that would help in the future: > - An explicit environment maintainer I would gladly volunteer for this role if Wolfgang would co-maintain to keep me in line. He seems to have an uncanny ability to keep all the cases in his head. > - test.py tests for the environment flags, but only if they're also run > on some platform(s) that also would have failed here. Perhaps we need > to enable more functionality in something like qemu-x86 that is less > of a special case build than sandbox is? In fact, since I know we > have the QEMU targets in for "real" uses and not just testing, > and while I worry about adding in more complex logic, we might want to > rework the "build and run test.py in QEMU" parts of CI to first make > use of scripts/kconfig/merge_config.sh to turn ON a whole bunch of > testing related options. > -- > Tom > ___ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] Regressions in MTD / SPI FLASH
We faced with regressions caused by commit c4e8862308d4 (mtd: spi: Switch to new SPI NOR framework) This switch was performed by removing entire u-boot spi-flash core implementation and copying it from another project. However the switch is performed without proper testing and investigations about fixes/improvements were made in u-boot spi-flash core. This results in regressions. One of regression we faced with is related to SST26 flash series which is used on HSDK board. The cause is that SST26 protection ops implementation was dropped. The fix of this regression is send as a patch in this series. However there are another regressions. I.E: we also faced with broken SPI flash on another SNPS boards - AXS101 and AXS103. They use different flash IC (n25q512ax3) and I didn't investigate the cause yet. I can also expect regressions among other u-boot users and I believe that subsystem changes mustn't be done such harmful way. Eugeniy Paltsev (1): MTD: SPI: revert removing SST26* flash IC protection ops drivers/mtd/spi/sf_internal.h | 1 + drivers/mtd/spi/spi-nor-core.c | 181 + drivers/mtd/spi/spi-nor-ids.c | 8 +- include/linux/mtd/spi-nor.h| 4 + 4 files changed, 190 insertions(+), 4 deletions(-) -- 2.21.0 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 1/1] MTD: SPI: revert removing SST26* flash IC protection ops
Commit c4e8862308d4 (mtd: spi: Switch to new SPI NOR framework) performs switch from previous 'spi_flash' infrastructure without proper testing/investigations which results in regressions. Fix regression related to SST26 flash IC series which is lacking protection ops implementation which were introduced previously by Commit 3d4fed87a5fa (mtd: sf: Add support of sst26wf* flash ICs protection ops) Signed-off-by: Eugeniy Paltsev --- Tom, could you please pick this patch to 2019.10? drivers/mtd/spi/sf_internal.h | 1 + drivers/mtd/spi/spi-nor-core.c | 181 + drivers/mtd/spi/spi-nor-ids.c | 8 +- include/linux/mtd/spi-nor.h| 4 + 4 files changed, 190 insertions(+), 4 deletions(-) diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h index a6bf734830a..e6da768bf36 100644 --- a/drivers/mtd/spi/sf_internal.h +++ b/drivers/mtd/spi/sf_internal.h @@ -65,6 +65,7 @@ struct flash_info { #define NO_CHIP_ERASE BIT(12) /* Chip does not support chip erase */ #define SPI_NOR_SKIP_SFDP BIT(13) /* Skip parsing of SFDP tables */ #define USE_CLSR BIT(14) /* use CLSR command */ +#define SPI_NOR_HAS_SST26LOCK BIT(15) /* Flash supports lock/unlock via BPR */ }; extern const struct flash_info spi_nor_ids[]; diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 1acff745d1a..990e39d7c2f 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -945,6 +945,177 @@ read_err: } #ifdef CONFIG_SPI_FLASH_SST +/* + * sst26 flash series has its own block protection implementation: + * 4x - 8 KByte blocks - read & write protection bits - upper addresses + * 1x - 32 KByte blocks - write protection bits + * rest - 64 KByte blocks - write protection bits + * 1x - 32 KByte blocks - write protection bits + * 4x - 8 KByte blocks - read & write protection bits - lower addresses + * + * We'll support only per 64k lock/unlock so lower and upper 64 KByte region + * will be treated as single block. + */ +#define SST26_BPR_8K_NUM 4 +#define SST26_MAX_BPR_REG_LEN (18 + 1) +#define SST26_BOUND_REG_SIZE ((32 + SST26_BPR_8K_NUM * 8) * SZ_1K) + +enum lock_ctl { + SST26_CTL_LOCK, + SST26_CTL_UNLOCK, + SST26_CTL_CHECK +}; + +static bool sst26_process_bpr(u32 bpr_size, u8 *cmd, u32 bit, enum lock_ctl ctl) +{ + switch (ctl) { + case SST26_CTL_LOCK: + cmd[bpr_size - (bit / 8) - 1] |= BIT(bit % 8); + break; + case SST26_CTL_UNLOCK: + cmd[bpr_size - (bit / 8) - 1] &= ~BIT(bit % 8); + break; + case SST26_CTL_CHECK: + return !!(cmd[bpr_size - (bit / 8) - 1] & BIT(bit % 8)); + } + + return false; +} + +/* + * Lock, unlock or check lock status of the flash region of the flash (depending + * on the lock_ctl value) + */ +static int sst26_lock_ctl(struct spi_nor *nor, loff_t ofs, uint64_t len, enum lock_ctl ctl) +{ + struct mtd_info *mtd = >mtd; + u32 i, bpr_ptr, rptr_64k, lptr_64k, bpr_size; + bool lower_64k = false, upper_64k = false; + u8 bpr_buff[SST26_MAX_BPR_REG_LEN] = {}; + int ret; + + /* Check length and offset for 64k alignment */ + if ((ofs & (SZ_64K - 1)) || (len & (SZ_64K - 1))) { + dev_err(nor->dev, "length or offset is not 64KiB allighned\n"); + return -EINVAL; + } + + if (ofs + len > mtd->size) { + dev_err(nor->dev, "range is more than device size: %#llx + %#llx > %#llx\n", + ofs, len, mtd->size); + return -EINVAL; + } + + /* SST26 family has only 16 Mbit, 32 Mbit and 64 Mbit IC */ + if (mtd->size != SZ_2M && + mtd->size != SZ_4M && + mtd->size != SZ_8M) + return -EINVAL; + + bpr_size = 2 + (mtd->size / SZ_64K / 8); + + ret = nor->read_reg(nor, SPINOR_OP_READ_BPR, bpr_buff, bpr_size); + if (ret < 0) { + dev_err(nor->dev, "fail to read block-protection register\n"); + return ret; + } + + rptr_64k = min_t(u32, ofs + len, mtd->size - SST26_BOUND_REG_SIZE); + lptr_64k = max_t(u32, ofs, SST26_BOUND_REG_SIZE); + + upper_64k = ((ofs + len) > (mtd->size - SST26_BOUND_REG_SIZE)); + lower_64k = (ofs < SST26_BOUND_REG_SIZE); + + /* Lower bits in block-protection register are about 64k region */ + bpr_ptr = lptr_64k / SZ_64K - 1; + + /* Process 64K blocks region */ + while (lptr_64k < rptr_64k) { + if (sst26_process_bpr(bpr_size, bpr_buff, bpr_ptr, ctl)) + return EACCES; + + bpr_ptr++; + lptr_64k += SZ_64K; + } + + /* 32K and 8K region bits in BPR are after 64k region bits */ + bpr_ptr = (mtd->size - 2 * SST26_BOUND_REG_SIZE) / SZ_64K; + + /* Process lower 32K block region
Re: [U-Boot] U-Boot: Environment flags broken for U-Boot
On Tue, Sep 03, 2019 at 10:04:42AM +0200, Wolfgang Denk wrote: > Dear Tom, > > In message Heiko Schocher > wrote: > > > > I am just testing U-Boot Environment flags and they do not work anymore with > > current mainline U-Boot ... > ... > > reason is your commit: > > > > commit 7d4776545b0f8a8827e5d061206faf61c9ba6ea9 > > Author: Patrick Delaunay > > Date: Thu Apr 18 17:32:49 2019 +0200 > > > > env: solve compilation error in SPL > > > Looking into the history of this, I wonder if we could / should > have prevented this. Looking over my scripts, yes, I overlooked the problem. The 'edison' platform shows the same issue that Heiko's platform does but I overlooked the size change. I'm modifying my script currently so it will show more details and this should jump out more rather than the size noise of "changes in a general area". Now interesting enough, sandbox didn't blow up here but does also enable the env flags options. > As far as I can see, Patrick's patch series has not been reviewed by > others, probably because general intetest in STM32 is not that big > at the moment. I can see no Acked-by:, Reviewed-by: nor Tested-by: > tags - nothing. > > The whole patch series was then pulled from the u-boot-stm > repository. > > > However, there was not only STM related code in there. There were > changes to common code like the environment handling. common code > was changed without review and without testing. To be clear, it was tested, but sadly the environment flags code is not heavily used / enabled. More in a moment. > Are there ways to prevent this? > > Yes, we can appeal to the custodians to be more careful, but I > assume they are already doing their best. > > It might have even been better if this had been a sub-system with a > clear maintainer, but there is no such person for the environment > code. > > How can we prevent this in the future? > > Should we define "interested developers" for such areas that have no > custodian (the "Designated reviewer") entry in the MAINTAINERS file > could be used for this, for example)? This, along with some other environment related patches were things I was keeping an eye on to see if perhaps Joe would have had time to look at before it went in (as the env flag stuff came from him). I also try and make use of the "Needs Review / ACK" flag in patchwork for things that stand out. Looking over the merge contents again, that particular one would not have. So, things that would help in the future: - An explicit environment maintainer - test.py tests for the environment flags, but only if they're also run on some platform(s) that also would have failed here. Perhaps we need to enable more functionality in something like qemu-x86 that is less of a special case build than sandbox is? In fact, since I know we have the QEMU targets in for "real" uses and not just testing, and while I worry about adding in more complex logic, we might want to rework the "build and run test.py in QEMU" parts of CI to first make use of scripts/kconfig/merge_config.sh to turn ON a whole bunch of testing related options. -- Tom signature.asc Description: PGP signature ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] What if ATF can be part of U-Boot source, like SPL?
On 9/4/19 7:32 PM, Andre Przywara wrote: [...] >> I have been avoiding this thread but today I attended a talk on ATF >> and ARM's approach in general. ARM seems to be moving towards an >> approach of providing increasingly complex source code to add more >> layers of security software to fully round out two mostly separate >> worlds (secure and normal). >> >> Oddly enough Intel was also there talking about how they are breaking >> things down into pieces and slowly releasing more things into the open >> source world. >> >> I came away with the impression that the two companies are going in >> different directions. ARM seems to be heading where Intel was and >> Intel is heading back. This is a gross simplification of course. >> >> To me it seems that the following scenario might play out: >> >> 1. ARM releases new ATF versions as source drops that firmware >> projects like U-Boot expected to take. In fact, not even U-Boot...this >> is just users picking up whatever they find >> >> 2. ATF keeps getting more complex, adding its own drivers for serial, >> storage, etc., duplicating and perhaps conflicting with those in >> U-Boot > > A bit like U-Boot, ATF can look quite differently on the various platforms: > - There are platforms like Arm's Juno or the fastmodel, where ATF does some > loading. This is mostly for features like secure boot or secure payloads > (OP-Tee). Typically we don't even use U-Boot primarily on those platforms > (but EDK II instead). > - There are platforms (low end SoCs like Allwinner, Rockchip, for instance) > which don't do any loading at all, actually rely on other code (SPL?) to load > ATF itself. > > So I don't see how this would duplicate effort or even conflict. So why don't we put all the platform init code into U-Boot SPL, which already has all the loading code and only load this ATF BL31 with SPL? >> 3. Over time we end up with binary blobs on ARM that are every bit as >> impenetrable as what Intel used to have > > I am not quite sure I follow how you come from 2. to 3. Was there something > in the presentation (I guess OSFC?) you are referring to? > In contrast to the x86 world the firmware is actually Open Source based, so > you actually have a realistic chance to rebuild and/or verify it. If this is > not true in a particular case, poke your vendor. I thought ATF was designed to be BSD-licensed to allow vendors to take all the open source contributions, benefit from it, but also allow the vendors to never release their changes if they so desire. It seems like some platforms even went down this path already and only released blobs, hence, no security fixes etc. >> 4. Firmware security and open-sourceness suffers, overall. >> >> One approach might be for ATF to split into a library which can be >> linked into a new U-Boot phase and a driver/init bit that could be >> used for other bootloaders, whatever they might be. >> >> But in the meantime, I think it makes more sense to incorporate the >> relevant parts of ATF into U-Boot and maintain them there, only for >> the platforms that U-Boot supports. If ATF starts using GUIDs and the >> like, we at least have somewhere to hide :-) > > What are the relevant parts, exactly? Do you want to rip them out? Use git > submodules? > For reasons I detailed before, I doubt the viability of the practical aspect > alone. > > But on top of that, I see that ATF and U-Boot address two separate areas: CPU > initialisation and secure world management on one side, and a very versatile > bootloader running in the non-secure world on the other. Keeping them > separate allows a clean separation and lets each project focus on its core > functionality. > > I understand that this separation is not always as clear or as well > implemented on every platform, but at least we should aim at this. Shouldn't the "secure world" management be as open as possible then ? -- Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH] drivers: nand: brcmnand: fix nand_chip ecc layout structure
The current brcmnand driver is based on 4.18 linux kernel which uses mtd_set_ooblayout to set ecc layout. But nand base code in u-boot is from old kernel which does not use this new API and expect nand_chip.ecc.layout structure to be set. This cause nand_scan_tail function running into a bug check if the device has a different oob size than the default ones. This patch ports the brcmstb_choose_ecc_layout function from kernel 4.6.7 that supports the ecc layout struture and replaces the mtd_set_ooblayout method Signed-off-by: William Zhang --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 260 +-- 1 file changed, 104 insertions(+), 156 deletions(-) diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index faa6da42d5..0745929253 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -888,183 +888,131 @@ static inline bool is_hamming_ecc(struct brcmnand_controller *ctrl, } /* - * Set mtd->ooblayout to the appropriate mtd_ooblayout_ops given - * the layout/configuration. - * Returns -ERRCODE on failure. + * Returns a nand_ecclayout strucutre for the given layout/configuration. + * Returns NULL on failure. */ -static int brcmnand_hamming_ooblayout_ecc(struct mtd_info *mtd, int section, - struct mtd_oob_region *oobregion) +static struct nand_ecclayout *brcmnand_create_layout(int ecc_level, +struct brcmnand_host *host) { - struct nand_chip *chip = mtd_to_nand(mtd); - struct brcmnand_host *host = nand_get_controller_data(chip); - struct brcmnand_cfg *cfg = >hwcfg; - int sas = cfg->spare_area_size << cfg->sector_size_1k; - int sectors = cfg->page_size / (512 << cfg->sector_size_1k); - - if (section >= sectors) - return -ERANGE; - - oobregion->offset = (section * sas) + 6; - oobregion->length = 3; - - return 0; -} - -static int brcmnand_hamming_ooblayout_free(struct mtd_info *mtd, int section, - struct mtd_oob_region *oobregion) -{ - struct nand_chip *chip = mtd_to_nand(mtd); - struct brcmnand_host *host = nand_get_controller_data(chip); struct brcmnand_cfg *cfg = >hwcfg; - int sas = cfg->spare_area_size << cfg->sector_size_1k; - int sectors = cfg->page_size / (512 << cfg->sector_size_1k); + int i, j; + struct nand_ecclayout *layout; + int req; + int sectors; + int sas; + int idx1, idx2; - if (section >= sectors * 2) - return -ERANGE; - - oobregion->offset = (section / 2) * sas; - - if (section & 1) { - oobregion->offset += 9; - oobregion->length = 7; - } else { - oobregion->length = 6; - - /* First sector of each page may have BBI */ - if (!section) { - /* -* Small-page NAND use byte 6 for BBI while large-page -* NAND use byte 0. -*/ - if (cfg->page_size > 512) - oobregion->offset++; - oobregion->length--; +#ifndef __UBOOT__ + layout = devm_kzalloc(>pdev->dev, sizeof(*layout), GFP_KERNEL); +#else + layout = devm_kzalloc(host->pdev, sizeof(*layout), GFP_KERNEL); +#endif + if (!layout) + return NULL; + + sectors = cfg->page_size / (512 << cfg->sector_size_1k); + sas = cfg->spare_area_size << cfg->sector_size_1k; + + /* Hamming */ + if (is_hamming_ecc(host->ctrl, cfg)) { + for (i = 0, idx1 = 0, idx2 = 0; i < sectors; i++) { + /* First sector of each page may have BBI */ + if (i == 0) { + layout->oobfree[idx2].offset = i * sas + 1; + /* Small-page NAND use byte 6 for BBI */ + if (cfg->page_size == 512) + layout->oobfree[idx2].offset--; + layout->oobfree[idx2].length = 5; + } else { + layout->oobfree[idx2].offset = i * sas; + layout->oobfree[idx2].length = 6; + } + idx2++; + layout->eccpos[idx1++] = i * sas + 6; + layout->eccpos[idx1++] = i * sas + 7; + layout->eccpos[idx1++] = i * sas + 8; + layout->oobfree[idx2].offset = i * sas + 9; + layout->oobfree[idx2].length = 7; + idx2++; + /* Leave zero-terminated entry for OOBFREE */ + if (idx1 >= MTD_MAX_ECCPOS_ENTRIES_LARGE || +
Re: [U-Boot] What if ATF can be part of U-Boot source, like SPL?
On Tue, 3 Sep 2019 19:17:14 -0700 Simon Glass wrote: Hi Simon, > On Tue, 2 Jul 2019 at 11:42, Marek Vasut wrote: > > > > On 6/30/19 3:38 AM, André Przywara wrote: > > > On 30/06/2019 00:03, Marek Vasut wrote: > > > > > > Marek, > > > > > > you seem to be quite defensive in your answer > > > > I am just correcting the mistakes I perceive in the previous email. > > > > >, but I was just talking > > > against merging ATF into U-Boot, not against U-Boot - I think we agree > > > on this. I don't think there is much of a point in comparing ATF and > > > U-Boot, as the two do not compete against each other. They just > > > complement each other nicely, hence we use ATF and save us the burden of > > > having to reprogram something in U-Boot. > > > > I suspect I answered those already in the thread. > > > > >> On 6/29/19 8:49 PM, André Przywara wrote: > > >>> On 29/06/2019 16:02, Jagan Teki wrote: > > In terms of code maintenance and development feasibility it is always > > a better approach to have out-of-tree code or binary to be part of > > in-house source tree. > > >>> > > >>> I am not sure this is really true. If I get you right, you want to > > >>> mirror and sync the ATF source into the U-Boot tree, which sounds like a > > >>> pain: > > >>> - ATF development is quite dynamic, with just shy of 1000 commits alone > > >>> this year. > > >> > > >> I don't think ATF development is anywhere near as dynamic as U-Boot, > > >> there's over 1000 commits in each U-Boot release, with quarterly release > > >> cycle. > > > > > > Wasn't comparing, just saying that it's nothing you want to sync > > > regularly. Unlike libfdt, for instance. > > > > > >>> This would mean constant churn of keeping the source up-to-date. > > >>> - The overlap of U-Boot and ATF users is not that big: ATF has support > > >>> for a lot more platforms (most of them typically paired with EDK-2), > > >> > > >> Can you elaborate on how you came to this conclusion ? Last time I > > >> counted (a few days ago, in ATF master), the upstream ATF supported some > > >> low tens of boards, upstream U-Boot around a thousand. And then there's > > >> the part where ATF is ARM-centric while U-Boot is platform-agnostic. > > > > > > I now see that my wording was unclear: ATF supports more platforms than > > > those that also use U-Boot. So you pull in code that you will never need. > > > > > > > I think s/more/different/ > > > > [...] > > > > >>> I am not questioning the current design, but just want to point out that > > >>> this might not be best thing since sliced bread. > > >> > > >> It would certainly help if we could have one, or a few, bootloader > > >> project(s) and work on those few, to make them great. Currently there > > >> are many, each with a limited developer community. Adding more does not > > >> help, it only increases the fragmentation and the mess. > > > > > > And that's exactly the point of ATF: You can use the tested PSCI code, > > > the secure GIC setup and all those core errata workarounds and spare > > > yourself from wrongly implementing this on your own. > > > > Which is a benefit of one single vendor being in control of everything? > > Surely, this single vendor has perfect engineers that make no mistakes, > > right? I think we had that before, and it didn't work out very well :) > > > > > Also ATF is not a bootloader, it explicitly leaves out that part. > > > > > So can we > > do the same thing for ATF on ARM64 SoCs? > > > > We are using ATF (on Allwinner) to switch EL3 to EL2 for start loading > > U-Boot proper and minimal PSCI > > >>> > > >>> Minimal PSCI? TF-A is the full fledged reference implementation of PSCI, > > >>> it always supports the newest revisions, including Spectre/Meltdown > > >>> support. This alone is a killer argument for me to use ATF. > > >> > > >> So ARM creates new PSCI version, ARM implements it into ARM trusted > > >> firmware, and then presents it to the users as the latest greatest. > > > > > > Yes, that's the definition of a reference implementation. And it's Open > > > Source with a permissive license. > > > > Yes, it's Open Source and it's not Free Software, which is unfortunate. > > It allows disasters like vendors taking ATF, reaping all the benefits of > > external contributions, but releasing only closed-source blobs and > > giving everyone who wants to study the code or change it the finger. > > > > >> That sounds a bit like the old windows development model -- one company > > >> does everything and then presents it to developers for them to consume. > > > > > > PSCI is an interface, with a publicly available spec. You can go ahead > > > and implement it yourself - and U-Boot does. And by using PSCI, you win > > > so much, because SMP in *any* kernel suddenly becomes very > > > straightforward. > > > > Just about like ACPI on x86. > > > > > So if you mean with "Windows model" that there is a
Re: [U-Boot] [PATCH] disk: part_dos: Allocate at least one block size for mbr
Hi Faiz, > -Original Message- > From: Faiz Abbas > Sent: Wednesday, September 4, 2019 5:40 PM > To: u-boot@lists.denx.de > Cc: tr...@konsulko.com; Alexey Brodkin ; > paule...@forallsecure.com; > faiz_ab...@ti.com > Subject: [PATCH] disk: part_dos: Allocate at least one block size for mbr > > The blk_dread() following the mbr allocation reads one block from the > device. This will lead to overflow if block size is greater than the > size of legacy_mbr. Fix this by allocating at least one block size. > > Signed-off-by: Faiz Abbas Acked-by: Alexey Brodkin ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] env: net: U_BOOT_ENV_CALLBACKs should not depend on CMD_NET
Hi Heinrich, https://patchwork.ozlabs.org/patch/1156517/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] net: designware: drop compatible altr, socfpga-stmmac
Hi Ralph, https://patchwork.ozlabs.org/patch/1149481/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] CVE: nfs: fix stack-based buffer overflow in some nfs_handler reply helper functions
Hi liucheng, https://patchwork.ozlabs.org/patch/1155275/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] net: nfs: remove superfluous conversions
Hi Heinrich, https://patchwork.ozlabs.org/patch/1156730/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] Pull request: u-boot-net.git master
Hi Tom, This PR has bug, CVE, and regression fixes as well as a few features that took some back-and-forth. Travis tested here: https://travis-ci.org/jhershbe/u-boot/builds/580493530 The following changes since commit 448f11f7503995746a7b71e5e3b3a831c4651be9: Merge tag 'arc-for-2019.10-rc4' of https://gitlab.denx.de/u-boot/custodians/u-boot-arc (2019-09-03 12:40:50 -0400) are available in the git repository at: https://gitlab.denx.de/u-boot/custodians/u-boot-net.git master for you to fetch changes up to 5a5d1def59024dd3225e2a6142f8ee3ee10180a8: net: nfs: remove superfluous packed attribute (2019-09-04 11:37:19 -0500) Alex Marginean (6): drivers: net: driver for MDIO muxes controlled over I2C net: mdio-uclass: name MDIO according to device-name property if preset doc: bindings: add mdio.txt describing generic MDIO properties drivers: net: add marvell MDIO driver arm: dts: Set custom names for cp110 master/slave MDIO buses drivers: net: fsl_enet_mdio: fix missing terminator in PCI ID array Bin Meng (1): Revert "net: macb: Fixed reading MII_LPA register" Florinel Iordache (1): drivers/fsl-mc: Create Kconfig file to manage driver specific configs better Heinrich Schuchardt (6): test: dm_mdio: avoid out of bounds access drivers: net: pfe_eth: undefined return value network: set timeline for CONFIG_DM_ETH conversion env: net: U_BOOT_ENV_CALLBACKs should not depend on CMD_NET net: nfs: remove superfluous conversions net: nfs: remove superfluous packed attribute Joe Hershberger (3): net: Fix Covarity Defect 244093 net: mdio: Clarify code flow Covarity 244085 & 244090 Revert "drivers: net: driver for MDIO muxes controlled over I2C" Matt Pelland (1): net: mvpp2: support setting hardware addresses from ethernet core Michael Walle (1): net: make net_random_ethaddr() more random Patrick Delaunay (2): net: dwc_eth_qos: Change eqos_ops function to static net: dwc_et_qos: update weak function board_interface_eth_init Ralph Siemsen (1): net: designware: drop compatible altr, socfpga-stmmac Ramon Fried (3): net: introduce packet capture support doc: pcap: add pcap cmd documentation configs: sandbox: enable PCAP capture cmd Stefan Roese (1): net: macb: Fix rx buffer cache handling liucheng (G) (5): CVE: net: fix unbounded memcpy of UDP packet CVE: nfs: fix stack-based buffer overflow in some nfs_handler reply helper functions CVE-2019-14194/CVE-2019-14198: nfs: fix unbounded memcpy with a failed length check at nfs_read_reply CVE-2019-14195: nfs: fix unbounded memcpy with unvalidated length at nfs_readlink_reply CVE-2019-14196: nfs: fix unbounded memcpy with a failed length check at nfs_lookup_reply Makefile | 11 ++ arch/arm/cpu/armv8/fsl-layerscape/Kconfig | 17 -- arch/arm/dts/armada-cp110-master.dtsi | 1 + arch/arm/dts/armada-cp110-slave.dtsi | 1 + board/st/stm32mp1/stm32mp1.c | 16 +- cmd/Kconfig | 7 + cmd/Makefile | 1 + cmd/mdio.c| 3 +- cmd/pcap.c| 71 configs/sandbox_defconfig | 1 + doc/README.pcap | 62 +++ doc/device-tree-bindings/net/marvell-mdio.txt | 15 ++ doc/device-tree-bindings/net/mdio.txt | 36 doc/driver-model/migration.rst| 8 + drivers/net/Kconfig | 11 ++ drivers/net/Makefile | 1 + drivers/net/designware.c | 1 - drivers/net/dwc_eth_qos.c | 28 +-- drivers/net/fsl-mc/Kconfig| 25 +++ drivers/net/fsl_enetc_mdio.c | 1 + drivers/net/macb.c| 8 +- drivers/net/mdio_mux_i2creg.c | 108 drivers/net/mdio_sandbox.c| 4 +- drivers/net/mvmdio.c | 236 ++ drivers/net/mvpp2.c | 8 + drivers/net/pfe_eth/pfe_mdio.c| 3 +- include/env_callback.h| 2 +- include/env_flags.h | 2 +- include/net.h | 2 +- include/net/pcap.h| 55 ++ include/netdev.h | 3 + net/Makefile | 1 + net/eth-uclass.c | 5 + net/eth_legacy.c | 10 +- net/mdio-uclass.c | 13 +- net/net.c | 14 ++ net/nfs.c
Re: [U-Boot] net: dwc_et_qos: update weak function board_interface_eth_init
Hi Patrick, https://patchwork.ozlabs.org/patch/1140379/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] network: set timeline for CONFIG_DM_ETH conversion
Hi Heinrich, https://patchwork.ozlabs.org/patch/1144982/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] net: dwc_eth_qos: Change eqos_ops function to static
Hi Patrick, https://patchwork.ozlabs.org/patch/1140378/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] net: macb: Fix rx buffer cache handling
Hi Stefan, https://patchwork.ozlabs.org/patch/1152998/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] net: make net_random_ethaddr() more random
Hi Michael, https://patchwork.ozlabs.org/patch/1153632/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] CVE-2019-14195: nfs: fix unbounded memcpy with unvalidated length at nfs_readlink_reply
Hi liucheng, https://patchwork.ozlabs.org/patch/1155278/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] doc: bindings: add mdio.txt describing generic MDIO properties
Hi Alex, https://patchwork.ozlabs.org/patch/1136772/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] net: nfs: remove superfluous packed attribute
Hi Heinrich, https://patchwork.ozlabs.org/patch/1156731/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] drivers: net: pfe_eth: undefined return value
Hi Heinrich, https://patchwork.ozlabs.org/patch/1139395/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] CVE-2019-14196: nfs: fix unbounded memcpy with a failed length check at nfs_lookup_reply
Hi liucheng, https://patchwork.ozlabs.org/patch/1155276/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] drivers: net: fsl_enet_mdio: fix missing terminator in PCI ID array
Hi Alex, https://patchwork.ozlabs.org/patch/1143592/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] net: mvpp2: support setting hardware addresses from ethernet core
Hi Matt, https://patchwork.ozlabs.org/patch/1138980/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] CVE: net: fix unbounded memcpy of UDP packet
Hi liucheng, https://patchwork.ozlabs.org/patch/1155274/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] test: dm_mdio: avoid out of bounds access
Hi Heinrich, https://patchwork.ozlabs.org/patch/1139392/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] Revert "net: macb: Fixed reading MII_LPA register"
Hi Bin, https://patchwork.ozlabs.org/patch/1146935/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] configs: sandbox: enable PCAP capture cmd
Hi Ramon, https://patchwork.ozlabs.org/patch/1133816/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] net: mdio-uclass: name MDIO according to device-name property if preset
Hi Alex, https://patchwork.ozlabs.org/patch/1136774/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] drivers: net: add marvell MDIO driver
Hi Alex, https://patchwork.ozlabs.org/patch/1136771/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] doc: pcap: add pcap cmd documentation
Hi Ramon, https://patchwork.ozlabs.org/patch/1133815/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] CVE-2019-14194/CVE-2019-14198: nfs: fix unbounded memcpy with a failed length check at nfs_read_reply
Hi liucheng, https://patchwork.ozlabs.org/patch/1155277/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] net: mdio: Clarify code flow Covarity 244085 & 244090
Hi Joe, https://patchwork.ozlabs.org/patch/1139393/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] arm: dts: Set custom names for cp110 master/slave MDIO buses
Hi Alex, https://patchwork.ozlabs.org/patch/1136773/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] drivers/fsl-mc: Create Kconfig file to manage driver specific configs better
Hi Florinel, https://patchwork.ozlabs.org/patch/1099898/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] net: introduce packet capture support
Hi Ramon, https://patchwork.ozlabs.org/patch/1133814/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] drivers: net: driver for MDIO muxes controlled over I2C
Hi Alex, https://patchwork.ozlabs.org/patch/1132514/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] net: Fix Covarity Defect 244093
Hi Joe, https://patchwork.ozlabs.org/patch/1139394/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git Thanks! -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 2/2] libfdt: return correct value if #size-cells property is not present
Hi all, On 13/08/2019 11:34, Simon Glass wrote: > +Stephen Warren > > Hi Matthias, > > On Thu, 1 Aug 2019 at 05:42, Matthias Brugger wrote: >> >> Hi all, >> >> On 26/07/2019 11:13, matthias@kernel.org wrote: >>> From: Matthias Brugger >>> >>> According to the device tree specification, the default value for >>> was not present. >>> >>> This patch also makes fdt_address_cells() and fdt_size_cells() conform >>> to the behaviour documented in libfdt.h. The defaults are only returned >>> if fdt_getprop() returns -FDT_ERR_NOTFOUND, otherwise the actual error >>> is returned. >>> >>> This is based on upstream commit: >>> aa7254d ("libfdt: return correct value if #size-cells property is not >>> present") >>> but misses the test case part, as we don't implement them in u-boot. >>> >>> Signed-off-by: Matthias Brugger >> >> After running these two patches through the CI [1] I realized that three test >> are failing: >> test/py sandbox >> test/py sandbox with clang >> test/py sandbox_flattree >> >> All three fail dm_test_fdt_translation() in the case "No translation for >> busses >> with #size-cells == 0" [2]. >> >> Can anybody with more insight in the test infrastructure and the sandbox >> architecture help me to identify if this is >> a) a bug in the sandbox >> b) a bug in our test >> c) a bug in my patch >> >> I write this because I'm pretty sure that it is not option c), as we just >> stick >> to the specs here. > > Can you check the test and see? It might well be that the test is wrong. > > I hope we don't have tet code relying on this. > I think I found the error. I missed a commit in libftd which fixes the issue. I'll send a v2 soon. Thanks, Matthias ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] Revert "net: macb: Fixed reading MII_LPA register"
On Wed, Sep 4, 2019 at 11:39 PM Joe Hershberger wrote: > > Hi Bin, > > On Wed, Sep 4, 2019 at 6:51 AM Bin Meng wrote: > > > > Hi Joe, > > > > On Thu, Aug 15, 2019 at 3:03 AM Joe Hershberger > > wrote: > > > > > > Hi Radu, > > > > > > Is there something you can switch on to select the correct register on > > > the appropriate platform? > > > > > > On Wed, Aug 14, 2019 at 5:31 AM Bin Meng wrote: > > > > > > > > This reverts commit 1b0c9914cc75d1570359181ebd493cd5746cb0ed. > > > > > > > > Commit 1b0c9914cc75 ("net: macb: Fixed reading MII_LPA register") > > > > causes 100Mbps does not work any more with SiFive FU540 GEM on the > > > > HiFive Unleashed board. Revert it. > > > > > > > > Signed-off-by: Bin Meng > > > > > > Acked-by: Joe Hershberger > > > > Could you please pick this patch for v2019.10? > > Yes. It's already build-tested and I will send a PR later today. Thank you Joe! Regards, Bin ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] Revert "net: macb: Fixed reading MII_LPA register"
Hi Bin, On Wed, Sep 4, 2019 at 6:51 AM Bin Meng wrote: > > Hi Joe, > > On Thu, Aug 15, 2019 at 3:03 AM Joe Hershberger > wrote: > > > > Hi Radu, > > > > Is there something you can switch on to select the correct register on > > the appropriate platform? > > > > On Wed, Aug 14, 2019 at 5:31 AM Bin Meng wrote: > > > > > > This reverts commit 1b0c9914cc75d1570359181ebd493cd5746cb0ed. > > > > > > Commit 1b0c9914cc75 ("net: macb: Fixed reading MII_LPA register") > > > causes 100Mbps does not work any more with SiFive FU540 GEM on the > > > HiFive Unleashed board. Revert it. > > > > > > Signed-off-by: Bin Meng > > > > Acked-by: Joe Hershberger > > Could you please pick this patch for v2019.10? Yes. It's already build-tested and I will send a PR later today. Cheers, -Joe ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 04/26] remoteproc: elf-loader: Add 64 bit elf loading support
Hi Lokesh Thank you for the patch. BR Fabien On 04/09/2019 12:31 PM, Lokesh Vutla wrote: > The current rproc-elf-loader supports loading of only 32 bit elf files. > Introduce support for loading of 64 bit elf files in rproc-elf-loader. > > Signed-off-by: Lokesh Vutla Reviewed-by: Fabien Dessenne > --- > drivers/remoteproc/rproc-elf-loader.c | 110 ++ > include/remoteproc.h | 26 ++ > 2 files changed, 136 insertions(+) > > diff --git a/drivers/remoteproc/rproc-elf-loader.c > b/drivers/remoteproc/rproc-elf-loader.c > index 238f51d3b5..1aece2fc31 100644 > --- a/drivers/remoteproc/rproc-elf-loader.c > +++ b/drivers/remoteproc/rproc-elf-loader.c > @@ -64,6 +64,62 @@ int rproc_elf32_sanity_check(ulong addr, ulong size) > return 0; > } > > +/* Basic function to verify ELF64 image format */ > +int rproc_elf64_sanity_check(ulong addr, ulong size) > +{ > + Elf64_Ehdr *ehdr = (Elf64_Ehdr *)addr; > + char class; > + > + if (!addr) { > + pr_debug("Invalid fw address?\n"); > + return -EFAULT; > + } > + > + if (size < sizeof(Elf64_Ehdr)) { > + pr_debug("Image is too small\n"); > + return -ENOSPC; > + } > + > + class = ehdr->e_ident[EI_CLASS]; > + > + if (!IS_ELF(*ehdr) || ehdr->e_type != ET_EXEC || class != ELFCLASS64) { > + pr_debug("Not an executable ELF64 image\n"); > + return -EPROTONOSUPPORT; > + } > + > + /* We assume the firmware has the same endianness as the host */ > +# ifdef __LITTLE_ENDIAN > + if (ehdr->e_ident[EI_DATA] != ELFDATA2LSB) { > +# else /* BIG ENDIAN */ > + if (ehdr->e_ident[EI_DATA] != ELFDATA2MSB) { > +# endif > + pr_debug("Unsupported firmware endianness\n"); > + return -EILSEQ; > + } > + > + if (size < ehdr->e_shoff + sizeof(Elf64_Shdr)) { > + pr_debug("Image is too small\n"); > + return -ENOSPC; > + } > + > + if (memcmp(ehdr->e_ident, ELFMAG, SELFMAG)) { > + pr_debug("Image is corrupted (bad magic)\n"); > + return -EBADF; > + } > + > + if (ehdr->e_phnum == 0) { > + pr_debug("No loadable segments\n"); > + return -ENOEXEC; > + } > + > + if (ehdr->e_phoff > size) { > + pr_debug("Firmware size is too small\n"); > + return -ENOSPC; > + } > + > + return 0; > +} > + > int rproc_elf32_load_image(struct udevice *dev, unsigned long addr, ulong > size) > { > Elf32_Ehdr *ehdr; /* Elf header structure pointer */ > @@ -110,3 +166,57 @@ int rproc_elf32_load_image(struct udevice *dev, unsigned > long addr, ulong size) > > return 0; > } > + > +int rproc_elf64_load_image(struct udevice *dev, ulong addr, ulong size) > +{ > + const struct dm_rproc_ops *ops = rproc_get_ops(dev); > + u64 da, memsz, filesz, offset; > + Elf64_Ehdr *ehdr; > + Elf64_Phdr *phdr; > + int i, ret = 0; > + void *ptr; > + > + dev_dbg(dev, "%s: addr = 0x%lx size = 0x%lx\n", __func__, addr, size); > + > + if (rproc_elf64_sanity_check(addr, size)) > + return -EINVAL; > + > + ehdr = (Elf64_Ehdr *)addr; > + phdr = (Elf64_Phdr *)(addr + (ulong)ehdr->e_phoff); > + > + /* go through the available ELF segments */ > + for (i = 0; i < ehdr->e_phnum; i++, phdr++) { > + da = phdr->p_paddr; > + memsz = phdr->p_memsz; > + filesz = phdr->p_filesz; > + offset = phdr->p_offset; > + > + if (phdr->p_type != PT_LOAD) > + continue; > + > + dev_dbg(dev, "%s:phdr: type %d da 0x%llx memsz 0x%llx filesz > 0x%llx\n", > + __func__, phdr->p_type, da, memsz, filesz); > + > + ptr = (void *)(uintptr_t)da; > + if (ops->device_to_virt) { > + ptr = ops->device_to_virt(dev, da, phdr->p_memsz); > + if (!ptr) { > + dev_err(dev, "bad da 0x%llx mem 0x%llx\n", da, > + memsz); > + ret = -EINVAL; > + break; > + } > + } > + > + if (filesz) > + memcpy(ptr, (void *)addr + offset, filesz); > + if (filesz != memsz) > + memset(ptr + filesz, 0x00, memsz - filesz); > + > + flush_cache(rounddown((ulong)ptr, ARCH_DMA_MINALIGN), > + roundup((ulong)ptr + filesz, ARCH_DMA_MINALIGN) - > + rounddown((ulong)ptr, ARCH_DMA_MINALIGN)); > + } > + > + return ret; > +} > diff --git a/include/remoteproc.h b/include/remoteproc.h > index 1ef88be7f7..56a11db6e3 100644 > --- a/include/remoteproc.h > +++ b/include/remoteproc.h > @@ -214,6 +214,18 @@ int rproc_is_running(int id); >*/ > int rproc_elf32_sanity_check(ulong addr,
Re: [U-Boot] [PATCH v2 05/26] remoteproc: elf_loader: Introduce a common elf loader and checker functions
Hi Lokesh On 04/09/2019 12:31 PM, Lokesh Vutla wrote: > Introduce a common remoteproc elf loader and checker functions that > automatically detects the 64 bit elf file or 32 bit elf file and > loads/checks the sections accordingly. > > Signed-off-by: Lokesh Vutla Reviewed-by: Fabien Dessenne > --- > drivers/remoteproc/rproc-elf-loader.c | 31 +++ > include/remoteproc.h | 29 + > 2 files changed, 60 insertions(+) > > diff --git a/drivers/remoteproc/rproc-elf-loader.c > b/drivers/remoteproc/rproc-elf-loader.c > index 1aece2fc31..42a78f4207 100644 > --- a/drivers/remoteproc/rproc-elf-loader.c > +++ b/drivers/remoteproc/rproc-elf-loader.c > @@ -120,6 +120,22 @@ int rproc_elf64_sanity_check(ulong addr, ulong size) > return 0; > } > > +/* Basic function to verify ELF image format */ > +int rproc_elf_sanity_check(ulong addr, ulong size) > +{ > + Elf32_Ehdr *ehdr = (Elf32_Ehdr *)addr; > + > + if (!addr) { > + dev_err(dev, "Invalid firmware address\n"); > + return -EFAULT; > + } > + > + if (ehdr->e_ident[EI_CLASS] == ELFCLASS64) > + return rproc_elf64_sanity_check(addr, size); > + else > + return rproc_elf32_sanity_check(addr, size); > +} > + > int rproc_elf32_load_image(struct udevice *dev, unsigned long addr, ulong > size) > { > Elf32_Ehdr *ehdr; /* Elf header structure pointer */ > @@ -220,3 +236,18 @@ int rproc_elf64_load_image(struct udevice *dev, ulong > addr, ulong size) > > return ret; > } > + > +int rproc_elf_load_image(struct udevice *dev, ulong addr, ulong size) > +{ > + Elf32_Ehdr *ehdr = (Elf32_Ehdr *)addr; > + > + if (!addr) { > + dev_err(dev, "Invalid firmware address\n"); > + return -EFAULT; > + } > + > + if (ehdr->e_ident[EI_CLASS] == ELFCLASS64) > + return rproc_elf64_load_image(dev, addr, size); > + else > + return rproc_elf32_load_image(dev, addr, size); > +} > diff --git a/include/remoteproc.h b/include/remoteproc.h > index 56a11db6e3..812e0f47c4 100644 > --- a/include/remoteproc.h > +++ b/include/remoteproc.h > @@ -226,6 +226,19 @@ int rproc_elf32_sanity_check(ulong addr, ulong size); >*/ > int rproc_elf64_sanity_check(ulong addr, ulong size); > > +/** > + * rproc_elf_sanity_check() - Verify if an image is a valid ELF one > + * > + * Check if a valid ELF image exists at the given memory location. Auto > + * detects ELF32/ELF64 and verifies basic ELF64/ELF32 format requirements > + * like magic number and sections size. > + * > + * @addr:address of the image to verify > + * @size:size of the image > + * @return 0 if the image looks good, else appropriate error value. > + */ > +int rproc_elf_sanity_check(ulong addr, ulong size); > + > /** >* rproc_elf32_load_image() - load an ELF32 image >* @dev:device loading the ELF32 image > @@ -243,6 +256,17 @@ int rproc_elf32_load_image(struct udevice *dev, unsigned > long addr, ulong size); >* @return 0 if the image is successfully loaded, else appropriate error > value. >*/ > int rproc_elf64_load_image(struct udevice *dev, ulong addr, ulong size); > + > +/** > + * rproc_elf_load_image() - load an ELF image > + * @dev: device loading the ELF image > + * @addr:valid ELF image address > + * @size:size of the image > + * > + * Auto detects if the image is ELF32 or ELF64 image and load accordingly. > + * @return 0 if the image is successfully loaded, else appropriate error > value. > + */ > +int rproc_elf_load_image(struct udevice *dev, unsigned long addr, ulong > size); > #else > static inline int rproc_init(void) { return -ENOSYS; } > static inline int rproc_dev_init(int id) { return -ENOSYS; } > @@ -257,12 +281,17 @@ static inline int rproc_elf32_sanity_check(ulong addr, > ulong size) { return -ENOSYS; } > static inline int rproc_elf64_sanity_check(ulong addr, > ulong size) { return -ENOSYS; } > +static inline int rproc_elf_sanity_check(ulong addr, > + ulong size) { return -ENOSYS; } > static inline int rproc_elf32_load_image(struct udevice *dev, >unsigned long addr, ulong size) > { return -ENOSYS; } > static inline int rproc_elf64_load_image(struct udevice *dev, ulong addr, >ulong size) > { return -ENOSYS; } > +static inline int rproc_elf_load_image(struct udevice *dev, ulong addr, > +ulong size) > +{ return -ENOSYS; } > #endif > > #endif /* _RPROC_H_ */ ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 03/26] remoteproc: elf_loader: Always check the validity of the image before loading
Hi Lokesh On 04/09/2019 12:31 PM, Lokesh Vutla wrote: > rproc_elf32_load_image() rely on user to send a valid address for elf loading. > Instead do a sanity check on the address passed by user. This will help > all rproc elf users to not call sanity_check explicitly before calling > elf_loading. > > Signed-off-by: Lokesh Vutla Reviewed-by: Fabien Dessenne > --- > drivers/remoteproc/rproc-elf-loader.c | 11 --- > drivers/remoteproc/stm32_copro.c | 9 + > include/remoteproc.h | 6 -- > test/dm/remoteproc.c | 5 + > 4 files changed, 14 insertions(+), 17 deletions(-) > > diff --git a/drivers/remoteproc/rproc-elf-loader.c > b/drivers/remoteproc/rproc-elf-loader.c > index 7574ba3fb3..238f51d3b5 100644 > --- a/drivers/remoteproc/rproc-elf-loader.c > +++ b/drivers/remoteproc/rproc-elf-loader.c > @@ -64,13 +64,18 @@ int rproc_elf32_sanity_check(ulong addr, ulong size) > return 0; > } > > -/* A very simple elf loader, assumes the image is valid */ > -int rproc_elf32_load_image(struct udevice *dev, unsigned long addr) > +int rproc_elf32_load_image(struct udevice *dev, unsigned long addr, ulong > size) > { > Elf32_Ehdr *ehdr; /* Elf header structure pointer */ > Elf32_Phdr *phdr; /* Program header structure pointer */ > const struct dm_rproc_ops *ops; > - unsigned int i; > + unsigned int i, ret; > + > + ret = rproc_elf32_sanity_check(addr, size); > + if (ret) { > + dev_err(dev, "Invalid ELF32 Image %d\n", ret); > + return ret; > + } > > ehdr = (Elf32_Ehdr *)addr; > phdr = (Elf32_Phdr *)(addr + ehdr->e_phoff); > diff --git a/drivers/remoteproc/stm32_copro.c > b/drivers/remoteproc/stm32_copro.c > index 71895daf9c..40bba37211 100644 > --- a/drivers/remoteproc/stm32_copro.c > +++ b/drivers/remoteproc/stm32_copro.c > @@ -155,14 +155,7 @@ static int stm32_copro_load(struct udevice *dev, ulong > addr, ulong size) > return ret; > } > > - /* Support only ELF32 image */ > - ret = rproc_elf32_sanity_check(addr, size); > - if (ret) { > - dev_err(dev, "Invalid ELF32 image (%d)\n", ret); > - return ret; > - } > - > - return rproc_elf32_load_image(dev, addr); > + return rproc_elf32_load_image(dev, addr, size); > } > > /** > diff --git a/include/remoteproc.h b/include/remoteproc.h > index dbff1ce3cf..1ef88be7f7 100644 > --- a/include/remoteproc.h > +++ b/include/remoteproc.h > @@ -218,9 +218,10 @@ int rproc_elf32_sanity_check(ulong addr, ulong size); >* rproc_elf32_load_image() - load an ELF32 image >* @dev:device loading the ELF32 image >* @addr: valid ELF32 image address > + * @size:size of the image >* @return 0 if the image is successfully loaded, else appropriate error > value. >*/ > -int rproc_elf32_load_image(struct udevice *dev, unsigned long addr); > +int rproc_elf32_load_image(struct udevice *dev, unsigned long addr, ulong > size); > #else > static inline int rproc_init(void) { return -ENOSYS; } > static inline int rproc_dev_init(int id) { return -ENOSYS; } > @@ -234,7 +235,8 @@ static inline int rproc_is_running(int id) { return > -ENOSYS; } > static inline int rproc_elf32_sanity_check(ulong addr, > ulong size) { return -ENOSYS; } > static inline int rproc_elf32_load_image(struct udevice *dev, > - unsigned long addr) { return -ENOSYS; } > + unsigned long addr, ulong size) > +{ return -ENOSYS; } > #endif > > #endif /* _RPROC_H_ */ > diff --git a/test/dm/remoteproc.c b/test/dm/remoteproc.c > index a2c4be7c27..c77361c8f4 100644 > --- a/test/dm/remoteproc.c > +++ b/test/dm/remoteproc.c > @@ -171,11 +171,8 @@ static int dm_test_remoteproc_elf(struct unit_test_state > *uts) > ut_assertnonnull(loaded_firmware); > memset(loaded_firmware, 0, loaded_firmware_size); > > - /* Verify valid ELF format */ > - ut_assertok(rproc_elf32_sanity_check((ulong)valid_elf32, size)); > - > /* Load firmware in loaded_firmware, and verify it */ > - ut_assertok(rproc_elf32_load_image(dev, (unsigned long)valid_elf32)); > + ut_assertok(rproc_elf32_load_image(dev, (ulong)valid_elf32, size)); > ut_assertok(memcmp(loaded_firmware, valid_elf32, loaded_firmware_size)); > unmap_physmem(loaded_firmware, MAP_NOCACHE); > ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 02/26] remoteproc: ops: Add elf section size as input parameter to device_to_virt api
Hi Lokesh I have successfully tested this patch for stm32_copro. BR Fabien On 04/09/2019 12:31 PM, Lokesh Vutla wrote: > Introduce a new parameter "size" that accepts size of the region to > remoteproc ops callback device_to_virt(). This can enforce more checks > on the region that device_to_virt() is dealing with. > > Signed-off-by: Lokesh Vutla Tested-by: Fabien Dessenne Reviewed-by: Fabien Dessenne > --- > drivers/remoteproc/rproc-elf-loader.c | 3 ++- > drivers/remoteproc/sandbox_testproc.c | 4 +++- > drivers/remoteproc/stm32_copro.c | 12 ++-- > include/remoteproc.h | 3 ++- > 4 files changed, 17 insertions(+), 5 deletions(-) > > diff --git a/drivers/remoteproc/rproc-elf-loader.c > b/drivers/remoteproc/rproc-elf-loader.c > index 67937a7595..7574ba3fb3 100644 > --- a/drivers/remoteproc/rproc-elf-loader.c > +++ b/drivers/remoteproc/rproc-elf-loader.c > @@ -86,7 +86,8 @@ int rproc_elf32_load_image(struct udevice *dev, unsigned > long addr) > continue; > > if (ops->device_to_virt) > - dst = ops->device_to_virt(dev, (ulong)dst); > + dst = ops->device_to_virt(dev, (ulong)dst, > + phdr->p_memsz); > > dev_dbg(dev, "Loading phdr %i to 0x%p (%i bytes)\n", > i, dst, phdr->p_filesz); > diff --git a/drivers/remoteproc/sandbox_testproc.c > b/drivers/remoteproc/sandbox_testproc.c > index 5f35119ab7..49c4dd 100644 > --- a/drivers/remoteproc/sandbox_testproc.c > +++ b/drivers/remoteproc/sandbox_testproc.c > @@ -306,9 +306,11 @@ static int sandbox_testproc_ping(struct udevice *dev) >* sandbox_testproc_device_to_virt() - Convert device address to virtual > address >* @dev:device to operate upon >* @da: device address > + * @size:Size of the memory region @da is pointing to >* @return converted virtual address >*/ > -static void *sandbox_testproc_device_to_virt(struct udevice *dev, ulong da) > +static void *sandbox_testproc_device_to_virt(struct udevice *dev, ulong da, > + ulong size) > { > u64 paddr; > > diff --git a/drivers/remoteproc/stm32_copro.c > b/drivers/remoteproc/stm32_copro.c > index ad941f67e8..71895daf9c 100644 > --- a/drivers/remoteproc/stm32_copro.c > +++ b/drivers/remoteproc/stm32_copro.c > @@ -107,11 +107,13 @@ static int stm32_copro_set_hold_boot(struct udevice > *dev, bool hold) >* stm32_copro_device_to_virt() - Convert device address to virtual address >* @dev:corresponding STM32 remote processor device >* @da: device address > + * @size:Size of the memory region @da is pointing to >* @return converted virtual address >*/ > -static void *stm32_copro_device_to_virt(struct udevice *dev, ulong da) > +static void *stm32_copro_device_to_virt(struct udevice *dev, ulong da, > + ulong size) > { > - fdt32_t in_addr = cpu_to_be32(da); > + fdt32_t in_addr = cpu_to_be32(da), end_addr; > u64 paddr; > > paddr = dev_translate_dma_address(dev, _addr); > @@ -120,6 +122,12 @@ static void *stm32_copro_device_to_virt(struct udevice > *dev, ulong da) > return NULL; > } > > + end_addr = cpu_to_be32(da + size - 1); > + if (dev_translate_dma_address(dev, _addr) == OF_BAD_ADDR) { > + dev_err(dev, "Unable to convert address %ld\n", da + size - 1); > + return NULL; > + } > + > return phys_to_virt(paddr); > } > > diff --git a/include/remoteproc.h b/include/remoteproc.h > index 4987194905..dbff1ce3cf 100644 > --- a/include/remoteproc.h > +++ b/include/remoteproc.h > @@ -122,9 +122,10 @@ struct dm_rproc_ops { >* >* @dev:Remote proc device >* @da: Device address > + * @size: Size of the memory region @da is pointing to >* @return virtual address. >*/ > - void * (*device_to_virt)(struct udevice *dev, ulong da); > + void * (*device_to_virt)(struct udevice *dev, ulong da, ulong size); > }; > > /* Accessor */ ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v1 5/7] power: pmic: rk8xx: add pmic_shutdown() implement
From: Joseph Chen Signed-off-by: Joseph Chen Signed-off-by: Elaine Zhang --- drivers/power/pmic/rk8xx.c | 39 +++ 1 file changed, 39 insertions(+) diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c index 00c8a2e091d8..df2056913ced 100644 --- a/drivers/power/pmic/rk8xx.c +++ b/drivers/power/pmic/rk8xx.c @@ -49,6 +49,44 @@ static int rk8xx_read(struct udevice *dev, uint reg, uint8_t *buff, int len) return 0; } +static int rk8xx_shutdown(struct udevice *dev) +{ + struct rk8xx_priv *priv = dev_get_priv(dev); + u8 val, dev_off; + int ret = 0; + + switch (priv->variant) { + case RK808_ID: + dev_off = BIT(3); + break; + case RK805_ID: + case RK816_ID: + case RK818_ID: + dev_off = BIT(0); + break; + default: + printf("Unknown PMIC: RK%x\n", priv->variant); + return -EINVAL; + } + + ret = dm_i2c_read(dev, REG_DEVCTRL, , 1); + if (ret) { + printf("read error from device: %p register: %#x!", + dev, REG_DEVCTRL); + return ret; + } + + val |= dev_off; + ret = dm_i2c_write(dev, REG_DEVCTRL, , 1); + if (ret) { + printf("write error to device: %p register: %#x!", + dev, REG_DEVCTRL); + return ret; + } + + return 0; +} + #if CONFIG_IS_ENABLED(PMIC_CHILDREN) static int rk8xx_bind(struct udevice *dev) { @@ -91,6 +129,7 @@ static struct dm_pmic_ops rk8xx_ops = { .reg_count = rk8xx_reg_count, .read = rk8xx_read, .write = rk8xx_write, + .shutdown = rk8xx_shutdown, }; static const struct udevice_id rk8xx_ids[] = { -- 1.9.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH] remoteproc: elf_loader: fix program header parsing
Fix an issue where some sections are never loaded : if p_type is different from PT_LOAD the phdr pointer must be incremented. Signed-off-by: Fabien Dessenne --- drivers/remoteproc/rproc-elf-loader.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/remoteproc/rproc-elf-loader.c b/drivers/remoteproc/rproc-elf-loader.c index 67937a7..23d502d 100644 --- a/drivers/remoteproc/rproc-elf-loader.c +++ b/drivers/remoteproc/rproc-elf-loader.c @@ -78,7 +78,7 @@ int rproc_elf32_load_image(struct udevice *dev, unsigned long addr) ops = rproc_get_ops(dev); /* Load each program header */ - for (i = 0; i < ehdr->e_phnum; ++i) { + for (i = 0; i < ehdr->e_phnum; i++, phdr++) { void *dst = (void *)(uintptr_t)phdr->p_paddr; void *src = (void *)addr + phdr->p_offset; @@ -99,7 +99,6 @@ int rproc_elf32_load_image(struct udevice *dev, unsigned long addr) roundup((unsigned long)dst + phdr->p_filesz, ARCH_DMA_MINALIGN) - rounddown((unsigned long)dst, ARCH_DMA_MINALIGN)); - ++phdr; } return 0; -- 2.7.4 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFC PATCH] net: designware: drop compatible altr, socfpga-stmmac
From: Alexey Brodkin Date: Sep/04/2019, 06:59:01 (UTC+00:00) > Hi Joe, > > > -Original Message- > > From: Joe Hershberger > > Sent: Wednesday, September 4, 2019 1:10 AM > > To: Alexey Brodkin > > Cc: Ralph Siemsen ; Joseph Hershberger > > ; u- > > b...@lists.denx.de; linux-snps-...@lists.infradead.org; Eugeniy Paltsev > > > > Subject: Re: [U-Boot] [RFC PATCH] net: designware: drop compatible altr, > > socfpga-stmmac > > > > On Tue, Aug 20, 2019 at 3:07 AM Alexey Brodkin > > wrote: > > > > > > Hi Ralph, > > > > > > > -Original Message- > > > > From: Ralph Siemsen > > > > Sent: Monday, August 19, 2019 9:43 PM > > > > To: u-boot@lists.denx.de; Joe Hershberger ; > > > > Alexey Brodkin > > > > ; Vlad Zakharov > > > > Cc: Ralph Siemsen > > > > Subject: [RFC PATCH] net: designware: drop compatible > > > > altr,socfpga-stmmac > > > > > > > > The same compatible = "altr,socfpga-stmmac" appears in both > > > > drivers/net/designware.c and drivers/net/dwmac_socfgpa.c, > > > > creating ambiguity in which driver will be bound. > > > > > > > > For Intel/Altera SoC devices, dwmac_socfpga.c is the correct driver. > > > > So drop the compatible string from designware.c. > > > > > > > > Signed-off-by: Ralph Siemsen > > > > --- > > > > This compatible string also appears in: axs10x_mb.dtsi and hsdk.dts. > > > > Maintainers of those boards have been copied, kindly review. > > > > > > Thanks for this clean-up. > > > > > > Speaking about AXS10x board where we do have DW GMAC > > > I cannot recall the reason I chose to use "altr,socfpga-stmmac" > > > for the board here [1] 3 years ago. > > > > > > But given we don't have any special quirks implemented whatever > > > is the most generic DW GMAC compatible string is should be good for us. > > > > > > Joe, could you please suggest if we need to use just "st,stm32-dwmac" > > > or add our own compatible as the ASIC is not from ST obviously but > > > an FPGA with Synopsys ARC SoC? > > > > I think we should only be using what Linux does, so I'm afraid I have > > to defer to what exists in the DTs there. > > In the Linux kernel we use "snps,dwmac", see [1]. > But maybe we need to switch to "snps,dwmac-3.70a" even as what we really have > is 3.73a. > > While in U-Boot we don't have either one, the most generic is > "st,stm32-dwmac", see [2]. > So maybe we want to add at least "snps,dwmac". > > @Jose Abreu could you please confirm if "snps,dwmac-3.70a" is OK for GMAC IP > v3.73a? Yes it should be okay. > > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arc/boot/dts/axs10x_mb.dtsi#n74 > [2] > https://gitlab.denx.de/u-boot/u-boot/blob/master/drivers/net/designware.c#L855 > > -Alexey --- Thanks, Jose Miguel Abreu ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v1 2/7] power: pmic: rk816: support rk816 pmic
Signed-off-by: Elaine Zhang --- drivers/power/pmic/rk8xx.c | 1 + drivers/power/regulator/rk8xx.c | 674 +++- include/power/rk8xx_pmic.h | 11 +- 3 files changed, 600 insertions(+), 86 deletions(-) diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c index 25c339ab12cc..1900de9d1cdb 100644 --- a/drivers/power/pmic/rk8xx.c +++ b/drivers/power/pmic/rk8xx.c @@ -95,6 +95,7 @@ static struct dm_pmic_ops rk8xx_ops = { static const struct udevice_id rk8xx_ids[] = { { .compatible = "rockchip,rk808" }, + { .compatible = "rockchip,rk816" }, { .compatible = "rockchip,rk818" }, { } }; diff --git a/drivers/power/regulator/rk8xx.c b/drivers/power/regulator/rk8xx.c index aa4f3a161c47..a62af0b467ef 100644 --- a/drivers/power/regulator/rk8xx.c +++ b/drivers/power/regulator/rk8xx.c @@ -1,4 +1,3 @@ -// SPDX-License-Identifier: GPL-2.0+ /* * Copyright (C) 2015 Google, Inc * Written by Simon Glass @@ -6,6 +5,8 @@ * Based on Rockchip's drivers/power/pmic/pmic_rk808.c: * Copyright (C) 2012 rockchips * zyw + * + * SPDX-License-Identifier:GPL-2.0+ */ #include @@ -19,6 +20,9 @@ #define ENABLE_DRIVER #endif +/* Not used or exisit register and configure */ +#define NA -1 + /* Field Definitions */ #define RK808_BUCK_VSEL_MASK 0x3f #define RK808_BUCK4_VSEL_MASK 0xf @@ -32,49 +36,85 @@ #define RK818_USB_ILIM_SEL_MASK0x0f #define RK818_USB_CHG_SD_VSEL_MASK 0x70 +/* + * Ramp delay + */ +#define RK808_RAMP_RATE_OFFSET 3 +#define RK808_RAMP_RATE_MASK (3 << RK808_RAMP_RATE_OFFSET) +#define RK808_RAMP_RATE_2MV_PER_US (0 << RK808_RAMP_RATE_OFFSET) +#define RK808_RAMP_RATE_4MV_PER_US (1 << RK808_RAMP_RATE_OFFSET) +#define RK808_RAMP_RATE_6MV_PER_US (2 << RK808_RAMP_RATE_OFFSET) +#define RK808_RAMP_RATE_10MV_PER_US(3 << RK808_RAMP_RATE_OFFSET) struct rk8xx_reg_info { uint min_uv; uint step_uv; - s8 vsel_reg; + u8 vsel_reg; + u8 vsel_sleep_reg; + u8 config_reg; u8 vsel_mask; + u8 min_sel; }; static const struct rk8xx_reg_info rk808_buck[] = { - { 712500, 12500, REG_BUCK1_ON_VSEL, RK808_BUCK_VSEL_MASK, }, - { 712500, 12500, REG_BUCK2_ON_VSEL, RK808_BUCK_VSEL_MASK, }, - { 712500, 12500, -1, RK808_BUCK_VSEL_MASK, }, - { 180, 10, REG_BUCK4_ON_VSEL, RK808_BUCK4_VSEL_MASK, }, + { 712500, 12500, REG_BUCK1_ON_VSEL, REG_BUCK1_SLP_VSEL, REG_BUCK1_CONFIG, RK808_BUCK_VSEL_MASK, }, + { 712500, 12500, REG_BUCK2_ON_VSEL, REG_BUCK2_SLP_VSEL, REG_BUCK2_CONFIG, RK808_BUCK_VSEL_MASK, }, + { 712500, 12500, NA,NA, REG_BUCK3_CONFIG, RK808_BUCK_VSEL_MASK, }, + { 180, 10, REG_BUCK4_ON_VSEL, REG_BUCK4_SLP_VSEL, REG_BUCK4_CONFIG, RK808_BUCK4_VSEL_MASK, }, +}; + +static const struct rk8xx_reg_info rk816_buck[] = { + /* buck 1 */ + { 712500, 12500, REG_BUCK1_ON_VSEL, REG_BUCK1_SLP_VSEL, REG_BUCK1_CONFIG, RK818_BUCK_VSEL_MASK, 0x00, }, + { 180, 20, REG_BUCK1_ON_VSEL, REG_BUCK1_SLP_VSEL, REG_BUCK1_CONFIG, RK818_BUCK_VSEL_MASK, 0x3c, }, + { 230, 0, REG_BUCK1_ON_VSEL, REG_BUCK1_SLP_VSEL, REG_BUCK1_CONFIG, RK818_BUCK_VSEL_MASK, 0x3f, }, + /* buck 2 */ + { 712500, 12500, REG_BUCK2_ON_VSEL, REG_BUCK2_SLP_VSEL, REG_BUCK2_CONFIG, RK818_BUCK_VSEL_MASK, 0x00, }, + { 180, 20, REG_BUCK2_ON_VSEL, REG_BUCK2_SLP_VSEL, REG_BUCK2_CONFIG, RK818_BUCK_VSEL_MASK, 0x3c, }, + { 230, 0, REG_BUCK2_ON_VSEL, REG_BUCK2_SLP_VSEL, REG_BUCK2_CONFIG, RK818_BUCK_VSEL_MASK, 0x3f, }, + /* buck 3 */ + { 712500, 12500, NA,NA, REG_BUCK3_CONFIG, RK818_BUCK_VSEL_MASK, }, + /* buck 4 */ + { 80, 10, REG_BUCK4_ON_VSEL, REG_BUCK4_SLP_VSEL, REG_BUCK4_CONFIG, RK818_BUCK4_VSEL_MASK, }, }; static const struct rk8xx_reg_info rk818_buck[] = { - { 712500, 12500, REG_BUCK1_ON_VSEL, RK818_BUCK_VSEL_MASK, }, - { 712500, 12500, REG_BUCK2_ON_VSEL, RK818_BUCK_VSEL_MASK, }, - { 712500, 12500, -1, RK818_BUCK_VSEL_MASK, }, - { 180, 10, REG_BUCK4_ON_VSEL, RK818_BUCK4_VSEL_MASK, }, + { 712500, 12500, REG_BUCK1_ON_VSEL, REG_BUCK1_SLP_VSEL, REG_BUCK1_CONFIG, RK818_BUCK_VSEL_MASK, }, + { 712500, 12500, REG_BUCK2_ON_VSEL, REG_BUCK2_SLP_VSEL, REG_BUCK2_CONFIG, RK818_BUCK_VSEL_MASK, }, + { 712500, 12500, NA,NA, REG_BUCK3_CONFIG, RK818_BUCK_VSEL_MASK, }, + { 180, 10, REG_BUCK4_ON_VSEL, REG_BUCK4_SLP_VSEL, REG_BUCK4_CONFIG, RK818_BUCK4_VSEL_MASK, }, }; #ifdef ENABLE_DRIVER static const struct rk8xx_reg_info rk808_ldo[] = { - { 180, 10, REG_LDO1_ON_VSEL, RK808_LDO_VSEL_MASK, }, - { 180, 10, REG_LDO2_ON_VSEL, RK808_LDO_VSEL_MASK, }, - { 80, 10,
[U-Boot] [PATCH v1 7/7] pmic: add rk809 support
From: Joseph Chen include sub modules: pmic, regulator Signed-off-by: Joseph Chen Signed-off-by: Elaine Zhang --- drivers/power/pmic/rk8xx.c | 6 - drivers/power/regulator/rk8xx.c | 55 - include/power/rk8xx_pmic.h | 1 + 3 files changed, 60 insertions(+), 2 deletions(-) diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c index b7ce87d0eff5..4cf581cd8b1b 100644 --- a/drivers/power/pmic/rk8xx.c +++ b/drivers/power/pmic/rk8xx.c @@ -82,6 +82,7 @@ static int rk8xx_shutdown(struct udevice *dev) devctrl_reg = REG_DEVCTRL; dev_off = BIT(0); break; + case RK809_ID: case RK817_ID: devctrl_reg = RK817_REG_SYS_CFG3; dev_off = BIT(0); @@ -145,7 +146,8 @@ static int rk8xx_probe(struct udevice *dev) u8 value; /* read Chip variant */ - if (device_is_compatible(dev, "rockchip,rk817")) { + if (device_is_compatible(dev, "rockchip,rk817") || + device_is_compatible(dev, "rockchip,rk809")) { id_msb = RK817_ID_MSB; id_lsb = RK817_ID_LSB; } else { @@ -172,6 +174,7 @@ static int rk8xx_probe(struct udevice *dev) on_source = RK8XX_ON_SOURCE; off_source = RK8XX_OFF_SOURCE; break; + case RK809_ID: case RK817_ID: on_source = RK817_ON_SOURCE; off_source = RK817_OFF_SOURCE; @@ -227,6 +230,7 @@ static struct dm_pmic_ops rk8xx_ops = { static const struct udevice_id rk8xx_ids[] = { { .compatible = "rockchip,rk805" }, { .compatible = "rockchip,rk808" }, + { .compatible = "rockchip,rk809" }, { .compatible = "rockchip,rk816" }, { .compatible = "rockchip,rk817" }, { .compatible = "rockchip,rk818" }, diff --git a/drivers/power/regulator/rk8xx.c b/drivers/power/regulator/rk8xx.c index 0d95e54220b0..0817e1a35e89 100644 --- a/drivers/power/regulator/rk8xx.c +++ b/drivers/power/regulator/rk8xx.c @@ -28,6 +28,10 @@ #define RK808_BUCK4_VSEL_MASK 0xf #define RK808_LDO_VSEL_MASK0x1f +/* RK809 BUCK5 */ +#define RK809_BUCK5_CONFIG(n) (0xde + (n) * 1) +#define RK809_BUCK5_VSEL_MASK 0x07 + /* RK817 BUCK */ #define RK817_BUCK_ON_VSEL(n) (0xbb + 3 * (n - 1)) #define RK817_BUCK_SLP_VSEL(n) (0xbc + 3 * (n - 1)) @@ -60,6 +64,7 @@ #define RK805_RAMP_RATE_6MV_PER_US (1 << RK805_RAMP_RATE_OFFSET) #define RK805_RAMP_RATE_12_5MV_PER_US (2 << RK805_RAMP_RATE_OFFSET) #define RK805_RAMP_RATE_25MV_PER_US(3 << RK805_RAMP_RATE_OFFSET) + #define RK808_RAMP_RATE_OFFSET 3 #define RK808_RAMP_RATE_MASK (3 << RK808_RAMP_RATE_OFFSET) #define RK808_RAMP_RATE_2MV_PER_US (0 << RK808_RAMP_RATE_OFFSET) @@ -106,6 +111,14 @@ static const struct rk8xx_reg_info rk816_buck[] = { { 80, 10, REG_BUCK4_ON_VSEL, REG_BUCK4_SLP_VSEL, REG_BUCK4_CONFIG, RK818_BUCK4_VSEL_MASK, }, }; +static const struct rk8xx_reg_info rk809_buck5[] = { + /* buck 5 */ + { 150, 0, RK809_BUCK5_CONFIG(0), RK809_BUCK5_CONFIG(1), NA, RK809_BUCK5_VSEL_MASK, 0x00, }, + { 180, 20, RK809_BUCK5_CONFIG(0), RK809_BUCK5_CONFIG(1), NA, RK809_BUCK5_VSEL_MASK, 0x01, }, + { 280, 20, RK809_BUCK5_CONFIG(0), RK809_BUCK5_CONFIG(1), NA, RK809_BUCK5_VSEL_MASK, 0x04, }, + { 330, 30, RK809_BUCK5_CONFIG(0), RK809_BUCK5_CONFIG(1), NA, RK809_BUCK5_VSEL_MASK, 0x06, }, +}; + static const struct rk8xx_reg_info rk817_buck[] = { /* buck 1 */ { 50, 12500, RK817_BUCK_ON_VSEL(1), RK817_BUCK_SLP_VSEL(1), RK817_BUCK_CONFIG(1), RK817_BUCK_VSEL_MASK, 0x00, }, @@ -224,6 +237,7 @@ static const struct rk8xx_reg_info *get_buck_reg(struct udevice *pmic, return _buck[num + 4]; } + case RK809_ID: case RK817_ID: switch (num) { case 0 ... 2: @@ -240,6 +254,16 @@ static const struct rk8xx_reg_info *get_buck_reg(struct udevice *pmic, return _buck[num * 3 + 1]; else return _buck[num * 3 + 2]; + /* BUCK5 for RK809 */ + default: + if (uvolt < 180) + return _buck5[0]; + else if (uvolt < 280) + return _buck5[1]; + else if (uvolt < 330) + return _buck5[2]; + else + return _buck5[3]; } case RK818_ID: return _buck[num]; @@ -309,6 +333,7 @@ static int _buck_set_enable(struct udevice *pmic, int buck, bool enable) ret = pmic_clrsetbits(pmic, REG_DCDC_EN, mask, enable ? mask : 0);
[U-Boot] [PATCH v1 6/7] pmic: add RK817 support
From: Joseph Chen include sub modules: pmic, regulator Signed-off-by: Joseph Chen Signed-off-by: Elaine Zhang --- drivers/power/pmic/rk8xx.c | 109 -- drivers/power/regulator/rk8xx.c | 198 +++- include/power/rk8xx_pmic.h | 31 +++ 3 files changed, 329 insertions(+), 9 deletions(-) diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c index df2056913ced..b7ce87d0eff5 100644 --- a/drivers/power/pmic/rk8xx.c +++ b/drivers/power/pmic/rk8xx.c @@ -10,6 +10,22 @@ #include #include +static struct reg_data rk817_init_reg[] = { +/* enable the under-voltage protection, + * the under-voltage protection will shutdown the LDO3 and reset the PMIC + */ + { RK817_BUCK4_CMIN, 0x60, 0x60}, +/* + * Only when system suspend while U-Boot charge needs this config support + */ +#ifdef CONFIG_DM_CHARGE_DISPLAY + /* Set pmic_sleep as sleep function */ + { RK817_PMIC_SYS_CFG3, 0x08, 0x18 }, + /* Set pmic_int active low */ + { RK817_GPIO_INT_CFG, 0x00, 0x02 }, +#endif +}; + static const struct pmic_child_info pmic_children_info[] = { { .prefix = "DCDC_REG", .driver = "rk8xx_buck"}, { .prefix = "LDO_REG", .driver = "rk8xx_ldo"}, @@ -52,16 +68,22 @@ static int rk8xx_read(struct udevice *dev, uint reg, uint8_t *buff, int len) static int rk8xx_shutdown(struct udevice *dev) { struct rk8xx_priv *priv = dev_get_priv(dev); - u8 val, dev_off; + u8 val, dev_off, devctrl_reg; int ret = 0; switch (priv->variant) { case RK808_ID: + devctrl_reg = REG_DEVCTRL; dev_off = BIT(3); break; case RK805_ID: case RK816_ID: case RK818_ID: + devctrl_reg = REG_DEVCTRL; + dev_off = BIT(0); + break; + case RK817_ID: + devctrl_reg = RK817_REG_SYS_CFG3; dev_off = BIT(0); break; default: @@ -69,18 +91,18 @@ static int rk8xx_shutdown(struct udevice *dev) return -EINVAL; } - ret = dm_i2c_read(dev, REG_DEVCTRL, , 1); + ret = dm_i2c_read(dev, devctrl_reg, , 1); if (ret) { printf("read error from device: %p register: %#x!", - dev, REG_DEVCTRL); + dev, devctrl_reg); return ret; } val |= dev_off; - ret = dm_i2c_write(dev, REG_DEVCTRL, , 1); + ret = dm_i2c_write(dev, devctrl_reg, , 1); if (ret) { printf("write error to device: %p register: %#x!", - dev, REG_DEVCTRL); + dev, devctrl_reg); return ret; } @@ -114,13 +136,83 @@ static int rk8xx_bind(struct udevice *dev) static int rk8xx_probe(struct udevice *dev) { struct rk8xx_priv *priv = dev_get_priv(dev); - uint8_t msb, lsb; + struct reg_data *init_data = NULL; + int init_data_num = 0; + int ret = 0, i, show_variant; + u8 msb, lsb, id_msb, id_lsb; + u8 on_source = 0, off_source = 0; + u8 power_en0, power_en1, power_en2, power_en3; + u8 value; /* read Chip variant */ - rk8xx_read(dev, ID_MSB, , 1); - rk8xx_read(dev, ID_LSB, , 1); + if (device_is_compatible(dev, "rockchip,rk817")) { + id_msb = RK817_ID_MSB; + id_lsb = RK817_ID_LSB; + } else { + id_msb = ID_MSB; + id_lsb = ID_LSB; + } + + ret = rk8xx_read(dev, id_msb, , 1); + if (ret) + return ret; + ret = rk8xx_read(dev, id_lsb, , 1); + if (ret) + return ret; priv->variant = ((msb << 8) | lsb) & RK8XX_ID_MSK; + show_variant = priv->variant; + switch (priv->variant) { + case RK808_ID: + show_variant = 0x808; /* RK808 hardware ID is 0 */ + break; + case RK805_ID: + case RK816_ID: + case RK818_ID: + on_source = RK8XX_ON_SOURCE; + off_source = RK8XX_OFF_SOURCE; + break; + case RK817_ID: + on_source = RK817_ON_SOURCE; + off_source = RK817_OFF_SOURCE; + init_data = rk817_init_reg; + init_data_num = ARRAY_SIZE(rk817_init_reg); + power_en0 = pmic_reg_read(dev, RK817_POWER_EN0); + power_en1 = pmic_reg_read(dev, RK817_POWER_EN1); + power_en2 = pmic_reg_read(dev, RK817_POWER_EN2); + power_en3 = pmic_reg_read(dev, RK817_POWER_EN3); + + value = (power_en0 & 0x0f) | ((power_en1 & 0x0f) << 4); + pmic_reg_write(dev, RK817_POWER_EN_SAVE0, value); + value = (power_en2 & 0x0f) | ((power_en3 & 0x0f) << 4); + pmic_reg_write(dev, RK817_POWER_EN_SAVE1, value); + break; + default: +
[U-Boot] [PATCH v1 1/7] dm: regulator: support regulator more state
From: Joseph Chen support parse regulator standard property: regulator-off-in-suspend; regulator-init-microvolt; regulator-suspend-microvolt: regulator_get_suspend_enable regulator_set_suspend_enable regulator_get_suspend_value regulator_set_suspend_value Signed-off-by: Joseph Chen Signed-off-by: Elaine Zhang --- drivers/power/regulator/regulator-uclass.c | 67 ++ include/power/regulator.h | 42 +++ 2 files changed, 109 insertions(+) diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c index 76be95bcd159..972a5b210c3c 100644 --- a/drivers/power/regulator/regulator-uclass.c +++ b/drivers/power/regulator/regulator-uclass.c @@ -77,6 +77,33 @@ int regulator_set_value(struct udevice *dev, int uV) return ret; } +int regulator_set_suspend_value(struct udevice *dev, int uV) +{ + const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); + struct dm_regulator_uclass_platdata *uc_pdata; + + uc_pdata = dev_get_uclass_platdata(dev); + if (uc_pdata->min_uV != -ENODATA && uV < uc_pdata->min_uV) + return -EINVAL; + if (uc_pdata->max_uV != -ENODATA && uV > uc_pdata->max_uV) + return -EINVAL; + + if (!ops || !ops->set_suspend_value) + return -ENOSYS; + + return ops->set_suspend_value(dev, uV); +} + +int regulator_get_suspend_value(struct udevice *dev) +{ + const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); + + if (!ops || !ops->get_suspend_value) + return -ENOSYS; + + return ops->get_suspend_value(dev); +} + /* * To be called with at most caution as there is no check * before setting the actual voltage value. @@ -170,6 +197,26 @@ int regulator_set_enable_if_allowed(struct udevice *dev, bool enable) return ret; } +int regulator_set_suspend_enable(struct udevice *dev, bool enable) +{ + const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); + + if (!ops || !ops->set_suspend_enable) + return -ENOSYS; + + return ops->set_suspend_enable(dev, enable); +} + +int regulator_get_suspend_enable(struct udevice *dev) +{ + const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); + + if (!ops || !ops->get_suspend_enable) + return -ENOSYS; + + return ops->get_suspend_enable(dev); +} + int regulator_get_mode(struct udevice *dev) { const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); @@ -235,6 +282,11 @@ int regulator_autoset(struct udevice *dev) int ret = 0; uc_pdata = dev_get_uclass_platdata(dev); + + ret = regulator_set_suspend_enable(dev, uc_pdata->suspend_on); + if (!ret && uc_pdata->suspend_on) + ret = regulator_set_suspend_value(dev, uc_pdata->suspend_uV); + if (!uc_pdata->always_on && !uc_pdata->boot_on) return -EMEDIUMTYPE; @@ -243,6 +295,8 @@ int regulator_autoset(struct udevice *dev) if (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UV) ret = regulator_set_value(dev, uc_pdata->min_uV); + if (uc_pdata->init_uV > 0) + ret = regulator_set_value(dev, uc_pdata->init_uV); if (!ret && (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UA)) ret = regulator_set_current(dev, uc_pdata->min_uA); @@ -363,6 +417,7 @@ static int regulator_post_bind(struct udevice *dev) static int regulator_pre_probe(struct udevice *dev) { struct dm_regulator_uclass_platdata *uc_pdata; + ofnode node; uc_pdata = dev_get_uclass_platdata(dev); if (!uc_pdata) @@ -373,6 +428,8 @@ static int regulator_pre_probe(struct udevice *dev) -ENODATA); uc_pdata->max_uV = dev_read_u32_default(dev, "regulator-max-microvolt", -ENODATA); + uc_pdata->init_uV = dev_read_u32_default(dev, "regulator-init-microvolt", + -ENODATA); uc_pdata->min_uA = dev_read_u32_default(dev, "regulator-min-microamp", -ENODATA); uc_pdata->max_uA = dev_read_u32_default(dev, "regulator-max-microamp", @@ -382,6 +439,16 @@ static int regulator_pre_probe(struct udevice *dev) uc_pdata->ramp_delay = dev_read_u32_default(dev, "regulator-ramp-delay", 0); + node = dev_read_subnode(dev, "regulator-state-mem"); + if (ofnode_valid(node)) { + uc_pdata->suspend_on = !ofnode_read_bool(node, "regulator-off-in-suspend"); + if (ofnode_read_u32(node, "regulator-suspend-microvolt", _pdata->suspend_uV)) + uc_pdata->suspend_uV = uc_pdata->max_uA; + } else { + uc_pdata->suspend_on = true; + uc_pdata->suspend_uV =
[U-Boot] [PATCH v1 3/7] power: pmic: rk805: support rk805 pmic
Signed-off-by: Elaine Zhang --- drivers/power/pmic/rk8xx.c | 1 + drivers/power/regulator/rk8xx.c | 17 + include/power/rk8xx_pmic.h | 1 + 3 files changed, 19 insertions(+) diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c index 1900de9d1cdb..00c8a2e091d8 100644 --- a/drivers/power/pmic/rk8xx.c +++ b/drivers/power/pmic/rk8xx.c @@ -94,6 +94,7 @@ static struct dm_pmic_ops rk8xx_ops = { }; static const struct udevice_id rk8xx_ids[] = { + { .compatible = "rockchip,rk805" }, { .compatible = "rockchip,rk808" }, { .compatible = "rockchip,rk816" }, { .compatible = "rockchip,rk818" }, diff --git a/drivers/power/regulator/rk8xx.c b/drivers/power/regulator/rk8xx.c index a62af0b467ef..2416477f5c06 100644 --- a/drivers/power/regulator/rk8xx.c +++ b/drivers/power/regulator/rk8xx.c @@ -39,7 +39,14 @@ /* * Ramp delay */ +#define RK805_RAMP_RATE_OFFSET 3 +#define RK805_RAMP_RATE_MASK (3 << RK805_RAMP_RATE_OFFSET) +#define RK805_RAMP_RATE_3MV_PER_US (0 << RK805_RAMP_RATE_OFFSET) +#define RK805_RAMP_RATE_6MV_PER_US (1 << RK805_RAMP_RATE_OFFSET) +#define RK805_RAMP_RATE_12_5MV_PER_US (2 << RK805_RAMP_RATE_OFFSET) +#define RK805_RAMP_RATE_25MV_PER_US(3 << RK805_RAMP_RATE_OFFSET) #define RK808_RAMP_RATE_OFFSET 3 + #define RK808_RAMP_RATE_MASK (3 << RK808_RAMP_RATE_OFFSET) #define RK808_RAMP_RATE_2MV_PER_US (0 << RK808_RAMP_RATE_OFFSET) #define RK808_RAMP_RATE_4MV_PER_US (1 << RK808_RAMP_RATE_OFFSET) @@ -132,6 +139,7 @@ static const struct rk8xx_reg_info *get_buck_reg(struct udevice *pmic, struct rk8xx_priv *priv = dev_get_priv(pmic); switch (priv->variant) { + case RK805_ID: case RK816_ID: switch (num) { case 0: @@ -186,6 +194,7 @@ static int _buck_set_enable(struct udevice *pmic, int buck, bool enable) struct rk8xx_priv *priv = dev_get_priv(pmic); switch (priv->variant) { + case RK805_ID: case RK816_ID: if (buck >= 4) { buck -= 4; @@ -247,6 +256,7 @@ static int _buck_get_enable(struct udevice *pmic, int buck) int ret = 0; switch (priv->variant) { + case RK805_ID: case RK816_ID: if (buck >= 4) { mask = 1 << (buck - 4); @@ -278,6 +288,7 @@ static int _buck_set_suspend_enable(struct udevice *pmic, int buck, bool enable) struct rk8xx_priv *priv = dev_get_priv(pmic); switch (priv->variant) { + case RK805_ID: case RK816_ID: mask = 1 << buck; ret = pmic_clrsetbits(pmic, RK816_REG_DCDC_SLP_EN, mask, @@ -303,6 +314,7 @@ static int _buck_get_suspend_enable(struct udevice *pmic, int buck) uint mask; switch (priv->variant) { + case RK805_ID: case RK816_ID: mask = 1 << buck; val = pmic_reg_read(pmic, RK816_REG_DCDC_SLP_EN); @@ -331,6 +343,7 @@ static const struct rk8xx_reg_info *get_ldo_reg(struct udevice *pmic, struct rk8xx_priv *priv = dev_get_priv(pmic); switch (priv->variant) { + case RK805_ID: case RK816_ID: return _ldo[num]; case RK818_ID: @@ -347,6 +360,7 @@ static int _ldo_get_enable(struct udevice *pmic, int ldo) int ret = 0; switch (priv->variant) { + case RK805_ID: case RK816_ID: if (ldo >= 4) { mask = 1 << (ldo - 4); @@ -379,6 +393,7 @@ static int _ldo_set_enable(struct udevice *pmic, int ldo, bool enable) int ret = 0; switch (priv->variant) { + case RK805_ID: case RK816_ID: if (ldo >= 4) { ldo -= 4; @@ -411,6 +426,7 @@ static int _ldo_set_suspend_enable(struct udevice *pmic, int ldo, bool enable) int ret = 0; switch (priv->variant) { + case RK805_ID: case RK816_ID: mask = 1 << ldo; ret = pmic_clrsetbits(pmic, RK816_REG_LDO_SLP_EN, mask, @@ -434,6 +450,7 @@ static int _ldo_get_suspend_enable(struct udevice *pmic, int ldo) uint mask; switch (priv->variant) { + case RK805_ID: case RK816_ID: mask = 1 << ldo; val = pmic_reg_read(pmic, RK816_REG_LDO_SLP_EN); diff --git a/include/power/rk8xx_pmic.h b/include/power/rk8xx_pmic.h index 44e8d687dfba..7784c2a5e473 100644 --- a/include/power/rk8xx_pmic.h +++ b/include/power/rk8xx_pmic.h @@ -179,6 +179,7 @@ enum { }; enum { + RK805_ID = 0x8050, RK808_ID = 0x, RK816_ID = 0x8160, RK818_ID = 0x8180, -- 1.9.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v1 4/7] dm: pmic: add pmic_shutdown() interface
From: Joseph Chen Signed-off-by: Joseph Chen Signed-off-by: Elaine Zhang --- drivers/power/pmic/pmic-uclass.c | 11 +++ include/power/pmic.h | 9 + 2 files changed, 20 insertions(+) diff --git a/drivers/power/pmic/pmic-uclass.c b/drivers/power/pmic/pmic-uclass.c index db68c766f5d7..28cfe0c987a2 100644 --- a/drivers/power/pmic/pmic-uclass.c +++ b/drivers/power/pmic/pmic-uclass.c @@ -191,6 +191,17 @@ static int pmic_pre_probe(struct udevice *dev) return 0; } + +int pmic_shutdown(struct udevice *dev) +{ + const struct dm_pmic_ops *ops = dev_get_driver_ops(dev); + + if (!ops || !ops->shutdown) + return -ENOSYS; + + return ops->shutdown(dev); +} + UCLASS_DRIVER(pmic) = { .id = UCLASS_PMIC, .name = "pmic", diff --git a/include/power/pmic.h b/include/power/pmic.h index be9de6b4de7e..231195e5ea85 100644 --- a/include/power/pmic.h +++ b/include/power/pmic.h @@ -164,6 +164,7 @@ struct dm_pmic_ops { int (*read)(struct udevice *dev, uint reg, uint8_t *buffer, int len); int (*write)(struct udevice *dev, uint reg, const uint8_t *buffer, int len); + int (*shutdown)(struct udevice *dev); }; /** @@ -306,6 +307,14 @@ struct uc_pmic_priv { uint trans_len; }; +/** + * pmic_shutdown() - power off supplies of PMIC + * + * @dev: PMIC device to update + * @return 0 on success or negative value of errno. + */ +int pmic_shutdown(struct udevice *dev); + #endif /* CONFIG_DM_PMIC */ #ifdef CONFIG_POWER -- 1.9.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v1 0/8] power: pmic: support more PMIC
Support more PMIC and improve compatibility between pmics. Elaine Zhang (2): power: pmic: rk816: support rk816 pmic power: pmic: rk805: support rk805 pmic Joseph Chen (6): dm: regulator: support regulator more state power: pmic: rk8xx: bind more function dm: pmic: add pmic_shutdown() interface power: pmic: rk8xx: add pmic_shutdown() implement pmic: add RK817 support pmic: add rk809 support drivers/power/pmic/pmic-uclass.c | 11 + drivers/power/pmic/rk8xx.c | 173 +- drivers/power/regulator/regulator-uclass.c | 67 ++ drivers/power/regulator/rk8xx.c| 940 ++--- include/power/pmic.h | 9 + include/power/regulator.h | 42 ++ include/power/rk8xx_pmic.h | 42 ++ 7 files changed, 1196 insertions(+), 88 deletions(-) -- 1.9.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH] disk: part_dos: Allocate at least one block size for mbr
The blk_dread() following the mbr allocation reads one block from the device. This will lead to overflow if block size is greater than the size of legacy_mbr. Fix this by allocating at least one block size. Signed-off-by: Faiz Abbas --- disk/part_dos.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/disk/part_dos.c b/disk/part_dos.c index aae9d95906..8ddc13b50c 100644 --- a/disk/part_dos.c +++ b/disk/part_dos.c @@ -93,7 +93,8 @@ static int test_block_type(unsigned char *buffer) static int part_test_dos(struct blk_desc *dev_desc) { #ifndef CONFIG_SPL_BUILD - ALLOC_CACHE_ALIGN_BUFFER(legacy_mbr, mbr, 1); + ALLOC_CACHE_ALIGN_BUFFER(legacy_mbr, mbr, + DIV_ROUND_UP(dev_desc->blksz, sizeof(legacy_mbr))); if (blk_dread(dev_desc, 0, 1, (ulong *)mbr) != 1) return -1; -- 2.19.2 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] python3 support for pylibfdt
Hi Tom, Ah OK. I was worried about doing it in one step. It might be too late now but will take a look when I get time and we can see if it is safe. Regards, Simon On Wed, 28 Aug 2019 at 18:19, Tom Rini wrote: > > On Wed, Aug 28, 2019 at 10:11:16AM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Wed, 28 Aug 2019 at 07:47, Tom Rini wrote: > > > > > > On Wed, Aug 28, 2019 at 07:44:11AM -0600, Simon Glass wrote: > > > > Hi Peter, > > > > > > > > On Wed, 28 Aug 2019 at 05:46, Peter Robinson > > > > wrote: > > > > > > > > > > Hi Simon, > > > > > > > > > > > > > > > > With the EOL of python2 soon I've been looking at the > > > > > > > > > > > Fedora U-Boot > > > > > > > > > > > builds to see what it would take to move over to python3. > > > > > > > > > > > There's a > > > > > > > > > > > couple of issues building the bundled pylibfdt, the first > > > > > > > > > > > is the > > > > > > > > > > > Makefile hard codes python2, the second is that the > > > > > > > > > > > generated > > > > > > > > > > > libfdt_wrap.c doesn't seem to find the python3 version of > > > > > > > > > > > Python.h > > > > > > > > > > > (errors below). > > > > > > > > > > > > > > > > > > > > > > It seems upstream now supports building pylibfdt with dtc > > > > > > > > > > > 1.5.0 but I > > > > > > > > > > > couldn't quite work out how this fits into the U-Boot > > > > > > > > > > > bundled version. > > > > > > > > > > > Is there plans to be able to support pylibfdt with > > > > > > > > > > > python3? > > > > > > > > > > > > > > > > > > > > Sounds like we need to run the normal kernel script to > > > > > > > > > > re-sync with > > > > > > > > > > upstream? Thanks! > > > > > > > > > > > > > > > > > > Seems reasonable, I'll keep an eye out for a patch series to > > > > > > > > > test, > > > > > > > > > it's quite straight forward to test from my PoV. > > > > > > > > > > > > > > > > It won't be any time soon, sadly. Updating to the same dtc in > > > > > > > > the > > > > > > > > kernel (so just v1.4.7+) causes both massive amount of new > > > > > > > > device tree > > > > > > > > warnings as well as several fail to link due to size growth > > > > > > > > problems. > > > > > > > > > > > > > > For reference the kernel moved to v1.5.0-30-g702c1b6c0e73 in the > > > > > > > 5.3 > > > > > > > merge window. > > > > > > > > > > > > Also I sent a series for libfdt to reduce the size, as a first step > > > > > > to > > > > > > syncing up U-Boot again. It needs work, but I expect to get back to > > > > > > it > > > > > > next week. > > > > > > > > > > What's the latest on this? > > > > > > > > No new progress but I just emailed David again about my pending > > > > question. > > > > > > Should we perhaps sync including your proposed changes for now and > > > re-sync once it's in? There's more than just Fedora folks unhappy about > > > us being one of the last requires python2 things they support. > > > > OK I can try this, but there is a bit of work before we can move to > > Python 3. I asked a month or two ago whether we should move to default > > Python 3 for this release, but I don't think I got an answer so did > > not focus on it. Then again, maybe I imagined it or missed it. > > Hmm, I think I had taken your statement at the time as more of a > declaration of intent than a question, sorry! > > -- > Tom ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] Buffer overrun risk in UBI SPL for secure boot
Hi Heiko, The place where the issue comes up is in ubispl_load_volumes(), but that calls ipl_load() internally. I guess there are several options 1) Create a distinct ubispl_scan() function to do the scan without loading anything and then a new load volume function that takes offset and limit arguments. This would have the advantage that a SPL that needs to parse one volume before loading the next would only need to scan once, then could check sizes, then load the individual volumes as needed. At the same time, when we need to validate a FIT header before even deciding what we want to load from it, we would be able to read more incrementally. 2) Add the size limit to struct ubispl_load and not worry about anyone with non-upstreamed code forgetting to set it 3) Add the size limit to struct ubispl_load but have ubispl_load_volumes() set the size limit to 0 (unlimited) before calling a new ubispl_load_volumes_bounded() function. Which do you prefer? Do I presume correctly that your denx.de email address implies that an approach you approve will be acceptable if correctly implemented? Thanks, Joel Peshkin On Wed, Sep 4, 2019 at 6:01 AM Heiko Schocher wrote: > Hello Joel, > > Am 04.09.2019 um 06:57 schrieb Joel Peshkin: > > It seems that, in the process of doing any sort of secure boot chain of > > trust, anything loading a UBI volume in preparation to authenticate it, > > will load a volume of unknown size into a buffer prior to checking the > > signature of that volume. > > Hmm.. it is not an unknown size, it loads the complete UBI Volume, so > it is the size of the UBI Volume ... > but yes, if an attacker changes the size of UBI Volumes ... > > Could you please give us a reference to which piece of code you refer? > > is it this part: > > drivers/mtd/ubispl/ubispl.c > static int ipl_load(struct ubi_scan_info *ubi, const u32 vol_id, uint8_t > *laddr) > > yes ... no size... > > > Has anyone considered a solution for this? Should all implementations > just > > carve out a buffer at the top of memory for ubispl_load_volume or should > > the ubispl_load data structure be amended to include a size? It would > seem > > appropriate to include a size, but not clear how to do that without > > breaking compatibility with existing implementations. > > Hmm... yes may an option to add a maxsize to ubispl_load data struct > ubispl_load. > > regarding backwards compatibility, there are not much boards yet, which > use this feature: > > hs@threadripper:u-boot [master] $ grep -lr SPL_UBI | grep defconfig > configs/am335x_igep003x_defconfig > configs/igep00x0_defconfig > hs@threadripper:u-boot [master] $ > > So this should be solveable problem. > > bye, > Heiko > -- > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: h...@denx.de > ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 4/4] spi: Convert CONFIG_DM_SPI* to CONFIG_$(SPL_TPL_)DM_SPI*
Hi Tom, > On Tue, Sep 03, 2019 at 05:56:36PM +0200, Lukasz Majewski wrote: > > Hi Tom, > > > > > On Fri, Aug 23, 2019 at 11:02:27PM +0200, Lukasz Majewski wrote: > > > > Hi Tom, > > > > > > > > > On Tue, Aug 13, 2019 at 03:47:31PM +0200, Lukasz Majewski > > > > > wrote: > > > > > > This change allows more fine tuning of driver model based > > > > > > SPI support in SPL and TPL. It is now possible to explicitly > > > > > > enable/disable the DM_SPI support in SPL and TPL via Kconfig > > > > > > option. > > > > > > > > > > > > Before this change it was necessary to use: > > > > > > /* SPI Flash Configs */ > > > > > > #if defined(CONFIG_SPL_BUILD) > > > > > > #undef CONFIG_DM_SPI > > > > > > #undef CONFIG_DM_SPI_FLASH > > > > > > #undef CONFIG_SPI_FLASH_MTD > > > > > > #endif > > > > > > > > > > > > in the ./include/configs/.h, which is error prone and > > > > > > shall be avoided when we strive to switch to Kconfig. > > > > > > > > > > > > The goal of this patch: > > > > > > > > > > > > Provide distinction for DM_SPI support in both U-Boot > > > > > > proper and SPL (TPL). Valid use case is when U-Boot proper > > > > > > wants to use DM_SPI, but SPL must still support non DM > > > > > > driver. > > > > > > > > > > > > Another use case is the conversion of non DM/DTS SPI driver > > > > > > to support DM/DTS. When such driver needs to work in both > > > > > > SPL and U-Boot proper, the distinction is needed in Kconfig > > > > > > (also if SPL version of the driver supports OF_PLATDATA). > > > > > > > > > > > > In the end of the day one would have to support following > > > > > > use cases (in single driver file - e.g. mxs_spi.c): > > > > > > > > > > > > - U-Boot proper driver supporting DT/DTS > > > > > > - U-Boot proper driver without DT/DTS support (deprecated) > > > > > > - SPL driver without DT/DTS support > > > > > > - SPL (and TPL) driver with DT/DTS (when the SoC has enough > > > > > > resources to run full blown DT/DTS) > > > > > > - SPL driver with DT/DTS and SPL_OF_PLATDATA (when one have > > > > > > constrained environment with no fitImage and OF_LIBFDT > > > > > > support). > > > > > > > > > > > > Some boards do require SPI support (with DM) in SPL (TPL) > > > > > > and some only have DM_SPI{_FLASH} defined to allow > > > > > > compiling SPL. > > > > > > > > > > > > This patch converts #ifdef CONFIG_DM_SPI* to #if > > > > > > CONFIG_IS_ENABLED(DM_SPI) and provides corresponding > > > > > > defines in Kconfig. > > > > > > > > > > > > Signed-off-by: Lukasz Majewski > > > > > > Tested-by: Adam Ford #da850-evm > > > > > > > > > > Sorry I didn't bisect down to which part of the series is > > > > > doing this, but I see problems with: > > > > > ls1046ardb_sdcard ls1046ardb_qspi_spl ls1043ardb_nand > > > > > ls1046aqds_tfa ls1046aq ds_nand ls1046ardb_qspi > > > > > ls1046aqds_sdcard_ifc ls1046aqds_SECURE_BOOT > > > > > ls1046aqds_sdcard_qspi ls1 046aqds_qspi ls1043ardb_sdcard > > > > > ls1043aqds_sdcard_ifc ls1046ardb_tfa > > > > > ls1046ardb_tfa_SECURE_BOOT ls1043aqds_nand ls1046aqds_lpuart > > > > > ls1046ardb_emmc ls1046aqds_tfa_SECURE_BOOT ls1046afrwy_tfa ls > > > > > 1046aqds ls1043ardb_nand_SECURE_BOOT > > > > > ls1046ardb_qspi_SECURE_BOOT ls1043aqds_sdcard_qspi > > > > > > > > > > Some of which are dependency problems about SPL/SPL_DM. I > > > > > also see: aarch64: (for 225/225 boards) all -294.2 bss -0.0 > > > > > data -11.7 rodata -72.5 spl/u-boot-spl:all -0.3 > > > > > spl/u-boot-spl:text -0.3 text -210.0 [snip] ls1043ardb_nand: > > > > > all -9435 data -376 rodata -2331 text -6728 > > > > > ls1043ardb_sdcard: all -9435 data -376 rodata -2331 text > > > > > -6728 ls1043aqds_sdcard_ifc: all -9435 data -376 rodata -2331 > > > > > text -6728 ls1043aqds_nand: all -9435 data -376 rodata -2331 > > > > > text -6728 ls1043ardb_nand_SECURE_BOOT: all -9435 data -376 > > > > > rodata -2331 text -6728 ls1043ardb_sdcard_SECURE_BOOT: all > > > > > -9435 data -376 rodata -2331 text -6728 > > > > > ls1043aqds_sdcard_qspi: all -9436 bss -8 data -376 rodata > > > > > -2324 text -6728 > > > > > > > > > > which I think means a few conversions weren't right. > > > > > > > > It looks like some parts of the SPL are not build > > > > > > Yes, there's some dependency problem Kconfig is spotting related > > > to SPL/SPL_DM. > > > > > > > Is the delta of size available from travis-CI or from any other > > > > script (maybe there is some buildman switch)? > > > > > > Yes, it's part of buildman. I do: > > > $ export SOURCE_DATE_EPOCH=`date +%s` > > > $ ./tools/buildman/buildman -o /tmp/test --step 0 -b > > > origin/master.. --force-build -CveE $ ./tools/buildman/buildman > > > -o /tmp/test --step 0 -b origin/master.. -Ssdel > > > > > > to get size changes between point A and point Z in a branch, and > > > omit --step 0 when I need to see which patch in between them > > > caused the size change. > > > > > > > I cannot reproduce your
Re: [U-Boot] [PATCH] Revert "part: Allocate only one legacy_mbr buffer"
Alexey, On 04/09/19 6:53 PM, Alexey Brodkin wrote: > Hi Faiz, > >> -Original Message- >> From: Faiz Abbas >> Sent: Wednesday, September 4, 2019 4:09 PM >> To: Alexey Brodkin >> Cc: paule...@forallsecure.com; tr...@konsulko.com; u-boot@lists.denx.de >> Subject: Re: [PATCH] Revert "part: Allocate only one legacy_mbr buffer" >> >> Hi Alexey, >> >> On 04/09/19 6:27 PM, Alexey Brodkin wrote: >>> Hi Faiz, >>> >>> [snip] >>> >>> I guess what you really want to do is to allocate buffer for "mbr" >>> dynamically of size which is max(sizeof(legacy_mbr), dev_desc->blksz). >>> >> >> With the assumption that blksz is always greater than >> sizeof(legacy_mbr), this should work: >> >> ALLOC_CACHE_ALIGN_BUFFER(legacy_mbr, mbr, DIV_ROUND_UP(dev_desc->blksz, >> sizeof(legacy_mbr))); > > No this won't work. See ALLOC_CACHE_ALIGN_BUFFER does static allocation > in compile-time while blksz is set in runtime. > > That's why I mentioned switch to runtime allocation. > Hmm. So the problem isn't as much about allocating too much in the buffer but about using dynamically determined size for static array declaration. >>> >>> In fact it was a problem of huge static allocation I discovered just by >>> chance building U-Boot for a very memory-limited device, see [1]. >>> This is being used all over this disk/part_dos.c file. Shouldn't we fix all of that? Not sure how it has been working all along. >>> >>> That part I don't quite understand. What's being used, where and how? >>> And what's the problem with dynamic allocation of the buffer for MBR? >>> >> >> Isn't the following line (being used in different functions in >> disk/part_dos.c) also problematic because we don't know the value of >> blksz at compile time? >> >> ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, dev_desc->blksz); > > Ok I see now. Looks like I missed that Variable-Length Array (VLA) feature > added to C99 so up-to-date GCC apparently supports it. > > But I would strongly recommend to get rid of VLA usage by all means, > see [1] & [2]. > > [1] https://lwn.net/Articles/749064/ > [2] https://lwn.net/Articles/749089/ > Interesting. This is news to me. Thanks, Faiz ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] Revert "part: Allocate only one legacy_mbr buffer"
Hi Tom, Alexey, On 04/09/19 7:19 PM, Tom Rini wrote: > On Wed, Sep 04, 2019 at 01:46:52PM +, Alexey Brodkin wrote: >> Hi Faiz, >> >>> -Original Message- >>> From: Alexey Brodkin >>> Sent: Wednesday, September 4, 2019 4:23 PM >>> To: Faiz Abbas >>> Cc: paule...@forallsecure.com; tr...@konsulko.com; u-boot@lists.denx.de >>> Subject: RE: [PATCH] Revert "part: Allocate only one legacy_mbr buffer" >>> >>> Hi Faiz, >>> -Original Message- From: Faiz Abbas Sent: Wednesday, September 4, 2019 4:09 PM To: Alexey Brodkin Cc: paule...@forallsecure.com; tr...@konsulko.com; u-boot@lists.denx.de Subject: Re: [PATCH] Revert "part: Allocate only one legacy_mbr buffer" Hi Alexey, On 04/09/19 6:27 PM, Alexey Brodkin wrote: > Hi Faiz, > > [snip] > > I guess what you really want to do is to allocate buffer for "mbr" > dynamically of size which is max(sizeof(legacy_mbr), dev_desc->blksz). > With the assumption that blksz is always greater than sizeof(legacy_mbr), this should work: ALLOC_CACHE_ALIGN_BUFFER(legacy_mbr, mbr, DIV_ROUND_UP(dev_desc->blksz, sizeof(legacy_mbr))); >>> >>> No this won't work. See ALLOC_CACHE_ALIGN_BUFFER does static allocation >>> in compile-time while blksz is set in runtime. >>> >>> That's why I mentioned switch to runtime allocation. >>> >> >> Hmm. So the problem isn't as much about allocating too much in the >> buffer but about using dynamically determined size for static array >> declaration. > > In fact it was a problem of huge static allocation I discovered just by > chance building U-Boot for a very memory-limited device, see [1]. > >> This is being used all over this disk/part_dos.c file. >> Shouldn't we fix all of that? Not sure how it has been working all along. > > That part I don't quite understand. What's being used, where and how? > And what's the problem with dynamic allocation of the buffer for MBR? > Isn't the following line (being used in different functions in disk/part_dos.c) also problematic because we don't know the value of blksz at compile time? ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, dev_desc->blksz); >>> >>> Ok I see now. Looks like I missed that Variable-Length Array (VLA) feature >>> added to C99 so up-to-date GCC apparently supports it. >>> >>> But I would strongly recommend to get rid of VLA usage by all means, >>> see [1] & [2]. >>> >>> [1] https://lwn.net/Articles/749064/ >>> [2] https://lwn.net/Articles/749089/ >> >> So if I add "-Wvla" to CFLAGS I see 22 usages of them: >> >8- >> diff --git a/Makefile b/Makefile >> index c02accfc26..c6e8d12809 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -389,7 +389,7 @@ CHECKFLAGS := -D__linux__ -Dlinux -D__STDC__ -Dunix >> -D__unix__ \ >> >> KBUILD_CPPFLAGS := -D__KERNEL__ -D__UBOOT__ >> >> -KBUILD_CFLAGS := -Wall -Wstrict-prototypes \ >> +KBUILD_CFLAGS := -Wvla -Wall -Wstrict-prototypes \ >>-Wno-format-security \ >>-fno-builtin -ffreestanding $(CSTD_FLAG) >> KBUILD_CFLAGS += -fshort-wchar -fno-strict-aliasing >> >8- >> >> So that's what we have: >> >8- >> # make -j 48 2>&1 | grep "\-Wvla" >> disk/part_dos.c:129:2: warning: ISO C90 forbids variable length array >> ‘__buffer’ [-Wvla] >> disk/part_dos.c:200:2: warning: ISO C90 forbids variable length array >> ‘__buffer’ [-Wvla] >> drivers/spi/spi-mem.c:350:2: warning: ISO C90 forbids variable length array >> ‘op_buf’ [-Wvla] >> drivers/input/input.c:341:2: warning: ISO C90 forbids variable length array >> ‘temp’ [-Wvla] >> drivers/input/input.c:513:2: warning: ISO C90 forbids variable length array >> ‘ch’ [-Wvla] >> env/attr.c:124:2: warning: ISO C90 forbids variable length array ‘regex’ >> [-Wvla] >> env/attr.c:129:10: warning: ISO C90 forbids variable length array ‘caps’ >> [-Wvla] >> common/command.c:29:3: warning: ISO C90 forbids variable length array >> ‘cmd_array’ [-Wvla] >> drivers/mmc/dw_mmc.c:248:34: warning: ISO C90 forbids variable length array >> ‘__cur_idmac’ [-Wvla] >> fs/fat/fat.c:61:2: warning: ISO C90 forbids variable length array ‘__buffer’ >> [-Wvla] >> fs/fat/fat.c:262:3: warning: ISO C90 forbids variable length array >> ‘__tmpbuf’ [-Wvla] >> fs/fat/fat.c:290:3: warning: ISO C90 forbids variable length array >> ‘__tmpbuf’ [-Wvla] >> fs/fat/fat_write.c:410:3: warning: ISO C90 forbids variable length array >> ‘__tmpbuf’ [-Wvla] >> fs/fat/fat_write.c:439:3: warning: ISO C90 forbids variable length array >> ‘__tmpbuf’ [-Wvla] >> fs/ext4/ext4_common.c:200:2: warning: ISO C90 forbids variable length array >> ‘__sec_buf’ [-Wvla] >>
Re: [U-Boot] [PATCH] Revert "part: Allocate only one legacy_mbr buffer"
On Wed, Sep 04, 2019 at 01:46:52PM +, Alexey Brodkin wrote: > Hi Faiz, > > > -Original Message- > > From: Alexey Brodkin > > Sent: Wednesday, September 4, 2019 4:23 PM > > To: Faiz Abbas > > Cc: paule...@forallsecure.com; tr...@konsulko.com; u-boot@lists.denx.de > > Subject: RE: [PATCH] Revert "part: Allocate only one legacy_mbr buffer" > > > > Hi Faiz, > > > > > -Original Message- > > > From: Faiz Abbas > > > Sent: Wednesday, September 4, 2019 4:09 PM > > > To: Alexey Brodkin > > > Cc: paule...@forallsecure.com; tr...@konsulko.com; u-boot@lists.denx.de > > > Subject: Re: [PATCH] Revert "part: Allocate only one legacy_mbr buffer" > > > > > > Hi Alexey, > > > > > > On 04/09/19 6:27 PM, Alexey Brodkin wrote: > > > > Hi Faiz, > > > > > > > > [snip] > > > > > > > > I guess what you really want to do is to allocate buffer for "mbr" > > > > dynamically of size which is max(sizeof(legacy_mbr), > > > > dev_desc->blksz). > > > > > > > > > > With the assumption that blksz is always greater than > > > sizeof(legacy_mbr), this should work: > > > > > > ALLOC_CACHE_ALIGN_BUFFER(legacy_mbr, mbr, > > > DIV_ROUND_UP(dev_desc->blksz, > > > sizeof(legacy_mbr))); > > > >>> > > > >>> No this won't work. See ALLOC_CACHE_ALIGN_BUFFER does static > > > >>> allocation > > > >>> in compile-time while blksz is set in runtime. > > > >>> > > > >>> That's why I mentioned switch to runtime allocation. > > > >>> > > > >> > > > >> Hmm. So the problem isn't as much about allocating too much in the > > > >> buffer but about using dynamically determined size for static array > > > >> declaration. > > > > > > > > In fact it was a problem of huge static allocation I discovered just by > > > > chance building U-Boot for a very memory-limited device, see [1]. > > > > > > > >> This is being used all over this disk/part_dos.c file. > > > >> Shouldn't we fix all of that? Not sure how it has been working all > > > >> along. > > > > > > > > That part I don't quite understand. What's being used, where and how? > > > > And what's the problem with dynamic allocation of the buffer for MBR? > > > > > > > > > > Isn't the following line (being used in different functions in > > > disk/part_dos.c) also problematic because we don't know the value of > > > blksz at compile time? > > > > > > ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, dev_desc->blksz); > > > > Ok I see now. Looks like I missed that Variable-Length Array (VLA) feature > > added to C99 so up-to-date GCC apparently supports it. > > > > But I would strongly recommend to get rid of VLA usage by all means, > > see [1] & [2]. > > > > [1] https://lwn.net/Articles/749064/ > > [2] https://lwn.net/Articles/749089/ > > So if I add "-Wvla" to CFLAGS I see 22 usages of them: > >8- > diff --git a/Makefile b/Makefile > index c02accfc26..c6e8d12809 100644 > --- a/Makefile > +++ b/Makefile > @@ -389,7 +389,7 @@ CHECKFLAGS := -D__linux__ -Dlinux -D__STDC__ -Dunix > -D__unix__ \ > > KBUILD_CPPFLAGS := -D__KERNEL__ -D__UBOOT__ > > -KBUILD_CFLAGS := -Wall -Wstrict-prototypes \ > +KBUILD_CFLAGS := -Wvla -Wall -Wstrict-prototypes \ >-Wno-format-security \ >-fno-builtin -ffreestanding $(CSTD_FLAG) > KBUILD_CFLAGS += -fshort-wchar -fno-strict-aliasing > >8- > > So that's what we have: > >8- > # make -j 48 2>&1 | grep "\-Wvla" > disk/part_dos.c:129:2: warning: ISO C90 forbids variable length array > ‘__buffer’ [-Wvla] > disk/part_dos.c:200:2: warning: ISO C90 forbids variable length array > ‘__buffer’ [-Wvla] > drivers/spi/spi-mem.c:350:2: warning: ISO C90 forbids variable length array > ‘op_buf’ [-Wvla] > drivers/input/input.c:341:2: warning: ISO C90 forbids variable length array > ‘temp’ [-Wvla] > drivers/input/input.c:513:2: warning: ISO C90 forbids variable length array > ‘ch’ [-Wvla] > env/attr.c:124:2: warning: ISO C90 forbids variable length array ‘regex’ > [-Wvla] > env/attr.c:129:10: warning: ISO C90 forbids variable length array ‘caps’ > [-Wvla] > common/command.c:29:3: warning: ISO C90 forbids variable length array > ‘cmd_array’ [-Wvla] > drivers/mmc/dw_mmc.c:248:34: warning: ISO C90 forbids variable length array > ‘__cur_idmac’ [-Wvla] > fs/fat/fat.c:61:2: warning: ISO C90 forbids variable length array ‘__buffer’ > [-Wvla] > fs/fat/fat.c:262:3: warning: ISO C90 forbids variable length array ‘__tmpbuf’ > [-Wvla] > fs/fat/fat.c:290:3: warning: ISO C90 forbids variable length array ‘__tmpbuf’ > [-Wvla] > fs/fat/fat_write.c:410:3: warning: ISO C90 forbids variable length array > ‘__tmpbuf’ [-Wvla] > fs/fat/fat_write.c:439:3: warning: ISO C90 forbids variable length array > ‘__tmpbuf’ [-Wvla] > fs/ext4/ext4_common.c:200:2: warning: ISO C90 forbids variable length array
Re: [U-Boot] [PATCH] Revert "part: Allocate only one legacy_mbr buffer"
Hi Faiz, > -Original Message- > From: Alexey Brodkin > Sent: Wednesday, September 4, 2019 4:23 PM > To: Faiz Abbas > Cc: paule...@forallsecure.com; tr...@konsulko.com; u-boot@lists.denx.de > Subject: RE: [PATCH] Revert "part: Allocate only one legacy_mbr buffer" > > Hi Faiz, > > > -Original Message- > > From: Faiz Abbas > > Sent: Wednesday, September 4, 2019 4:09 PM > > To: Alexey Brodkin > > Cc: paule...@forallsecure.com; tr...@konsulko.com; u-boot@lists.denx.de > > Subject: Re: [PATCH] Revert "part: Allocate only one legacy_mbr buffer" > > > > Hi Alexey, > > > > On 04/09/19 6:27 PM, Alexey Brodkin wrote: > > > Hi Faiz, > > > > > > [snip] > > > > > > I guess what you really want to do is to allocate buffer for "mbr" > > > dynamically of size which is max(sizeof(legacy_mbr), dev_desc->blksz). > > > > > > > With the assumption that blksz is always greater than > > sizeof(legacy_mbr), this should work: > > > > ALLOC_CACHE_ALIGN_BUFFER(legacy_mbr, mbr, DIV_ROUND_UP(dev_desc->blksz, > > sizeof(legacy_mbr))); > > >>> > > >>> No this won't work. See ALLOC_CACHE_ALIGN_BUFFER does static allocation > > >>> in compile-time while blksz is set in runtime. > > >>> > > >>> That's why I mentioned switch to runtime allocation. > > >>> > > >> > > >> Hmm. So the problem isn't as much about allocating too much in the > > >> buffer but about using dynamically determined size for static array > > >> declaration. > > > > > > In fact it was a problem of huge static allocation I discovered just by > > > chance building U-Boot for a very memory-limited device, see [1]. > > > > > >> This is being used all over this disk/part_dos.c file. > > >> Shouldn't we fix all of that? Not sure how it has been working all along. > > > > > > That part I don't quite understand. What's being used, where and how? > > > And what's the problem with dynamic allocation of the buffer for MBR? > > > > > > > Isn't the following line (being used in different functions in > > disk/part_dos.c) also problematic because we don't know the value of > > blksz at compile time? > > > > ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, dev_desc->blksz); > > Ok I see now. Looks like I missed that Variable-Length Array (VLA) feature > added to C99 so up-to-date GCC apparently supports it. > > But I would strongly recommend to get rid of VLA usage by all means, > see [1] & [2]. > > [1] https://lwn.net/Articles/749064/ > [2] https://lwn.net/Articles/749089/ So if I add "-Wvla" to CFLAGS I see 22 usages of them: >8- diff --git a/Makefile b/Makefile index c02accfc26..c6e8d12809 100644 --- a/Makefile +++ b/Makefile @@ -389,7 +389,7 @@ CHECKFLAGS := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ \ KBUILD_CPPFLAGS := -D__KERNEL__ -D__UBOOT__ -KBUILD_CFLAGS := -Wall -Wstrict-prototypes \ +KBUILD_CFLAGS := -Wvla -Wall -Wstrict-prototypes \ -Wno-format-security \ -fno-builtin -ffreestanding $(CSTD_FLAG) KBUILD_CFLAGS += -fshort-wchar -fno-strict-aliasing >8- So that's what we have: >8- # make -j 48 2>&1 | grep "\-Wvla" disk/part_dos.c:129:2: warning: ISO C90 forbids variable length array ‘__buffer’ [-Wvla] disk/part_dos.c:200:2: warning: ISO C90 forbids variable length array ‘__buffer’ [-Wvla] drivers/spi/spi-mem.c:350:2: warning: ISO C90 forbids variable length array ‘op_buf’ [-Wvla] drivers/input/input.c:341:2: warning: ISO C90 forbids variable length array ‘temp’ [-Wvla] drivers/input/input.c:513:2: warning: ISO C90 forbids variable length array ‘ch’ [-Wvla] env/attr.c:124:2: warning: ISO C90 forbids variable length array ‘regex’ [-Wvla] env/attr.c:129:10: warning: ISO C90 forbids variable length array ‘caps’ [-Wvla] common/command.c:29:3: warning: ISO C90 forbids variable length array ‘cmd_array’ [-Wvla] drivers/mmc/dw_mmc.c:248:34: warning: ISO C90 forbids variable length array ‘__cur_idmac’ [-Wvla] fs/fat/fat.c:61:2: warning: ISO C90 forbids variable length array ‘__buffer’ [-Wvla] fs/fat/fat.c:262:3: warning: ISO C90 forbids variable length array ‘__tmpbuf’ [-Wvla] fs/fat/fat.c:290:3: warning: ISO C90 forbids variable length array ‘__tmpbuf’ [-Wvla] fs/fat/fat_write.c:410:3: warning: ISO C90 forbids variable length array ‘__tmpbuf’ [-Wvla] fs/fat/fat_write.c:439:3: warning: ISO C90 forbids variable length array ‘__tmpbuf’ [-Wvla] fs/ext4/ext4_common.c:200:2: warning: ISO C90 forbids variable length array ‘__sec_buf’ [-Wvla] fs/fs_internal.c:18:2: warning: ISO C90 forbids variable length array ‘__sec_buf’ [-Wvla] fs/fs_internal.c:62:3: warning: ISO C90 forbids variable length array ‘__p’ [-Wvla] fs/ext4/ext4_common.c:2075:4: warning: ISO C90 forbids variable length array ‘filename’ [-Wvla] fs/ext4/ext4_common.c:2213:2: warning: ISO C90 forbids
Re: [U-Boot] [PATCH] Revert "part: Allocate only one legacy_mbr buffer"
Hi Faiz, > -Original Message- > From: Faiz Abbas > Sent: Wednesday, September 4, 2019 4:09 PM > To: Alexey Brodkin > Cc: paule...@forallsecure.com; tr...@konsulko.com; u-boot@lists.denx.de > Subject: Re: [PATCH] Revert "part: Allocate only one legacy_mbr buffer" > > Hi Alexey, > > On 04/09/19 6:27 PM, Alexey Brodkin wrote: > > Hi Faiz, > > > > [snip] > > > > I guess what you really want to do is to allocate buffer for "mbr" > > dynamically of size which is max(sizeof(legacy_mbr), dev_desc->blksz). > > > > With the assumption that blksz is always greater than > sizeof(legacy_mbr), this should work: > > ALLOC_CACHE_ALIGN_BUFFER(legacy_mbr, mbr, DIV_ROUND_UP(dev_desc->blksz, > sizeof(legacy_mbr))); > >>> > >>> No this won't work. See ALLOC_CACHE_ALIGN_BUFFER does static allocation > >>> in compile-time while blksz is set in runtime. > >>> > >>> That's why I mentioned switch to runtime allocation. > >>> > >> > >> Hmm. So the problem isn't as much about allocating too much in the > >> buffer but about using dynamically determined size for static array > >> declaration. > > > > In fact it was a problem of huge static allocation I discovered just by > > chance building U-Boot for a very memory-limited device, see [1]. > > > >> This is being used all over this disk/part_dos.c file. > >> Shouldn't we fix all of that? Not sure how it has been working all along. > > > > That part I don't quite understand. What's being used, where and how? > > And what's the problem with dynamic allocation of the buffer for MBR? > > > > Isn't the following line (being used in different functions in > disk/part_dos.c) also problematic because we don't know the value of > blksz at compile time? > > ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, dev_desc->blksz); Ok I see now. Looks like I missed that Variable-Length Array (VLA) feature added to C99 so up-to-date GCC apparently supports it. But I would strongly recommend to get rid of VLA usage by all means, see [1] & [2]. [1] https://lwn.net/Articles/749064/ [2] https://lwn.net/Articles/749089/ -Alexey ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] Revert "part: Allocate only one legacy_mbr buffer"
Hi Alexey, On 04/09/19 6:27 PM, Alexey Brodkin wrote: > Hi Faiz, > > [snip] > > I guess what you really want to do is to allocate buffer for "mbr" > dynamically of size which is max(sizeof(legacy_mbr), dev_desc->blksz). > With the assumption that blksz is always greater than sizeof(legacy_mbr), this should work: ALLOC_CACHE_ALIGN_BUFFER(legacy_mbr, mbr, DIV_ROUND_UP(dev_desc->blksz, sizeof(legacy_mbr))); >>> >>> No this won't work. See ALLOC_CACHE_ALIGN_BUFFER does static allocation >>> in compile-time while blksz is set in runtime. >>> >>> That's why I mentioned switch to runtime allocation. >>> >> >> Hmm. So the problem isn't as much about allocating too much in the >> buffer but about using dynamically determined size for static array >> declaration. > > In fact it was a problem of huge static allocation I discovered just by > chance building U-Boot for a very memory-limited device, see [1]. > >> This is being used all over this disk/part_dos.c file. >> Shouldn't we fix all of that? Not sure how it has been working all along. > > That part I don't quite understand. What's being used, where and how? > And what's the problem with dynamic allocation of the buffer for MBR? > Isn't the following line (being used in different functions in disk/part_dos.c) also problematic because we don't know the value of blksz at compile time? ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, dev_desc->blksz); Thanks, Faiz ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] Buffer overrun risk in UBI SPL for secure boot
Hello Joel, Am 04.09.2019 um 06:57 schrieb Joel Peshkin: It seems that, in the process of doing any sort of secure boot chain of trust, anything loading a UBI volume in preparation to authenticate it, will load a volume of unknown size into a buffer prior to checking the signature of that volume. Hmm.. it is not an unknown size, it loads the complete UBI Volume, so it is the size of the UBI Volume ... but yes, if an attacker changes the size of UBI Volumes ... Could you please give us a reference to which piece of code you refer? is it this part: drivers/mtd/ubispl/ubispl.c static int ipl_load(struct ubi_scan_info *ubi, const u32 vol_id, uint8_t *laddr) yes ... no size... Has anyone considered a solution for this? Should all implementations just carve out a buffer at the top of memory for ubispl_load_volume or should the ubispl_load data structure be amended to include a size? It would seem appropriate to include a size, but not clear how to do that without breaking compatibility with existing implementations. Hmm... yes may an option to add a maxsize to ubispl_load data struct ubispl_load. regarding backwards compatibility, there are not much boards yet, which use this feature: hs@threadripper:u-boot [master] $ grep -lr SPL_UBI | grep defconfig configs/am335x_igep003x_defconfig configs/igep00x0_defconfig hs@threadripper:u-boot [master] $ So this should be solveable problem. bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: h...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] Revert "part: Allocate only one legacy_mbr buffer"
Hi Faiz, [snip] > >>> I guess what you really want to do is to allocate buffer for "mbr" > >>> dynamically of size which is max(sizeof(legacy_mbr), dev_desc->blksz). > >>> > >> > >> With the assumption that blksz is always greater than > >> sizeof(legacy_mbr), this should work: > >> > >> ALLOC_CACHE_ALIGN_BUFFER(legacy_mbr, mbr, DIV_ROUND_UP(dev_desc->blksz, > >> sizeof(legacy_mbr))); > > > > No this won't work. See ALLOC_CACHE_ALIGN_BUFFER does static allocation > > in compile-time while blksz is set in runtime. > > > > That's why I mentioned switch to runtime allocation. > > > > Hmm. So the problem isn't as much about allocating too much in the > buffer but about using dynamically determined size for static array > declaration. In fact it was a problem of huge static allocation I discovered just by chance building U-Boot for a very memory-limited device, see [1]. > This is being used all over this disk/part_dos.c file. > Shouldn't we fix all of that? Not sure how it has been working all along. That part I don't quite understand. What's being used, where and how? And what's the problem with dynamic allocation of the buffer for MBR? [1] https://elinux.org/images/d/d6/U-Boot-Bootloader-for-IoT-Platform-Alexey-Brodkin-Synopsys-2.pdf -Alexey ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] Revert "part: Allocate only one legacy_mbr buffer"
Hi Alexey, On 04/09/19 5:16 PM, Alexey Brodkin wrote: > Hi Faiz, > >> -Original Message- >> From: Faiz Abbas >> Sent: Wednesday, September 4, 2019 2:44 PM >> To: Alexey Brodkin >> Cc: paule...@forallsecure.com; tr...@konsulko.com; u-boot@lists.denx.de >> Subject: Re: [PATCH] Revert "part: Allocate only one legacy_mbr buffer" >> >> Hi Alexey, >> >> On 04/09/19 3:42 PM, Alexey Brodkin wrote: >>> Hi Faiz, >>> -Original Message- From: Faiz Abbas Sent: Wednesday, September 4, 2019 12:22 PM To: u-boot@lists.denx.de Cc: paule...@forallsecure.com; faiz_ab...@ti.com; Alexey Brodkin ; tr...@konsulko.com Subject: [PATCH] Revert "part: Allocate only one legacy_mbr buffer" This reverts commit 8639e34d2c5e12cc2e45c95b1a2e97c22bf6a711. The blk_dread() call following the allocation will read one block from the device. This will lead to overflow if the blocksize is greater than the size of legacy_mbr. Fix this by allocating one block size. >>> >>> Did you read justification of my change that you're reverting now? >>> With your change in place we'll allocate a buffer >>> of size = (sizeof(legacy_mbr) * dev_desc->blksz). >>> >>> Is that something you really want? >> >> Oops. You're right. I thought your patch was changing buffer size from >> blksz to legacy_mbr. >> >>> >>> I guess what you really want to do is to allocate buffer for "mbr" >>> dynamically of size which is max(sizeof(legacy_mbr), dev_desc->blksz). >>> >> >> With the assumption that blksz is always greater than >> sizeof(legacy_mbr), this should work: >> >> ALLOC_CACHE_ALIGN_BUFFER(legacy_mbr, mbr, DIV_ROUND_UP(dev_desc->blksz, >> sizeof(legacy_mbr))); > > No this won't work. See ALLOC_CACHE_ALIGN_BUFFER does static allocation > in compile-time while blksz is set in runtime. > > That's why I mentioned switch to runtime allocation. > Hmm. So the problem isn't as much about allocating too much in the buffer but about using dynamically determined size for static array declaration. This is being used all over this disk/part_dos.c file. Shouldn't we fix all of that? Not sure how it has been working all along. Thanks, Faiz ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 1/1] efi_loader: correctly render UsbClass DP nodes as text
Correct the text representation of UsbClass device path nodes. Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_device_path_to_text.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c index 6e27508fba..b20b7c097c 100644 --- a/lib/efi_loader/efi_device_path_to_text.c +++ b/lib/efi_loader/efi_device_path_to_text.c @@ -141,7 +141,7 @@ static char *dp_msging(char *s, struct efi_device_path *dp) struct efi_device_path_usb_class *ucdp = (struct efi_device_path_usb_class *)dp; - s += sprintf(s, "USBClass(%x,%x,%x,%x,%x)", + s += sprintf(s, "UsbClass(0x%x,0x%x,0x%x,0x%x,0x%x)", ucdp->vendor_id, ucdp->product_id, ucdp->device_class, ucdp->device_subclass, ucdp->device_protocol); -- 2.23.0.rc1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] Revert "net: macb: Fixed reading MII_LPA register"
Hi Joe, On Thu, Aug 15, 2019 at 3:03 AM Joe Hershberger wrote: > > Hi Radu, > > Is there something you can switch on to select the correct register on > the appropriate platform? > > On Wed, Aug 14, 2019 at 5:31 AM Bin Meng wrote: > > > > This reverts commit 1b0c9914cc75d1570359181ebd493cd5746cb0ed. > > > > Commit 1b0c9914cc75 ("net: macb: Fixed reading MII_LPA register") > > causes 100Mbps does not work any more with SiFive FU540 GEM on the > > HiFive Unleashed board. Revert it. > > > > Signed-off-by: Bin Meng > > Acked-by: Joe Hershberger Could you please pick this patch for v2019.10? Regards, Bin ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] Revert "part: Allocate only one legacy_mbr buffer"
Hi Faiz, > -Original Message- > From: Faiz Abbas > Sent: Wednesday, September 4, 2019 2:44 PM > To: Alexey Brodkin > Cc: paule...@forallsecure.com; tr...@konsulko.com; u-boot@lists.denx.de > Subject: Re: [PATCH] Revert "part: Allocate only one legacy_mbr buffer" > > Hi Alexey, > > On 04/09/19 3:42 PM, Alexey Brodkin wrote: > > Hi Faiz, > > > >> -Original Message- > >> From: Faiz Abbas > >> Sent: Wednesday, September 4, 2019 12:22 PM > >> To: u-boot@lists.denx.de > >> Cc: paule...@forallsecure.com; faiz_ab...@ti.com; Alexey Brodkin > >> ; > >> tr...@konsulko.com > >> Subject: [PATCH] Revert "part: Allocate only one legacy_mbr buffer" > >> > >> This reverts commit 8639e34d2c5e12cc2e45c95b1a2e97c22bf6a711. > >> > >> The blk_dread() call following the allocation will read one block from > >> the device. This will lead to overflow if the blocksize is greater than > >> the size of legacy_mbr. Fix this by allocating one block size. > > > > Did you read justification of my change that you're reverting now? > > With your change in place we'll allocate a buffer > > of size = (sizeof(legacy_mbr) * dev_desc->blksz). > > > > Is that something you really want? > > Oops. You're right. I thought your patch was changing buffer size from > blksz to legacy_mbr. > > > > > I guess what you really want to do is to allocate buffer for "mbr" > > dynamically of size which is max(sizeof(legacy_mbr), dev_desc->blksz). > > > > With the assumption that blksz is always greater than > sizeof(legacy_mbr), this should work: > > ALLOC_CACHE_ALIGN_BUFFER(legacy_mbr, mbr, DIV_ROUND_UP(dev_desc->blksz, > sizeof(legacy_mbr))); No this won't work. See ALLOC_CACHE_ALIGN_BUFFER does static allocation in compile-time while blksz is set in runtime. That's why I mentioned switch to runtime allocation. -Alexey ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] Revert "part: Allocate only one legacy_mbr buffer"
Hi Alexey, On 04/09/19 3:42 PM, Alexey Brodkin wrote: > Hi Faiz, > >> -Original Message- >> From: Faiz Abbas >> Sent: Wednesday, September 4, 2019 12:22 PM >> To: u-boot@lists.denx.de >> Cc: paule...@forallsecure.com; faiz_ab...@ti.com; Alexey Brodkin >> ; >> tr...@konsulko.com >> Subject: [PATCH] Revert "part: Allocate only one legacy_mbr buffer" >> >> This reverts commit 8639e34d2c5e12cc2e45c95b1a2e97c22bf6a711. >> >> The blk_dread() call following the allocation will read one block from >> the device. This will lead to overflow if the blocksize is greater than >> the size of legacy_mbr. Fix this by allocating one block size. > > Did you read justification of my change that you're reverting now? > With your change in place we'll allocate a buffer > of size = (sizeof(legacy_mbr) * dev_desc->blksz). > > Is that something you really want? Oops. You're right. I thought your patch was changing buffer size from blksz to legacy_mbr. > > I guess what you really want to do is to allocate buffer for "mbr" > dynamically of size which is max(sizeof(legacy_mbr), dev_desc->blksz). > With the assumption that blksz is always greater than sizeof(legacy_mbr), this should work: ALLOC_CACHE_ALIGN_BUFFER(legacy_mbr, mbr, DIV_ROUND_UP(dev_desc->blksz, sizeof(legacy_mbr))); Will send a proper fix. Thanks, Faiz ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 1/1] efi_loader: correctly render MAC address device path nodes
If the interface type is greater than 1, render all 32 bytes of the MAC address. Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_device_path_to_text.c | 15 +++ 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c index 133542ad40..2617694a8b 100644 --- a/lib/efi_loader/efi_device_path_to_text.c +++ b/lib/efi_loader/efi_device_path_to_text.c @@ -124,17 +124,16 @@ static char *dp_msging(char *s, struct efi_device_path *dp) break; } case DEVICE_PATH_SUB_TYPE_MSG_MAC_ADDR: { + int i, n = sizeof(struct efi_mac_addr); struct efi_device_path_mac_addr *mdp = (struct efi_device_path_mac_addr *)dp; - if (mdp->if_type != 0 && mdp->if_type != 1) - break; - - s += sprintf(s, "MAC(%02x%02x%02x%02x%02x%02x,0x%1x)", - mdp->mac.addr[0], mdp->mac.addr[1], - mdp->mac.addr[2], mdp->mac.addr[3], - mdp->mac.addr[4], mdp->mac.addr[5], - mdp->if_type); + if (mdp->if_type <= 1) + n = 6; + s += sprintf(s, "MAC("); + for (i = 0; i < n; ++i) + s += sprintf(s, "%02x", mdp->mac.addr[i]); + s += sprintf(s, ",%u)", mdp->if_type); break; } -- 2.23.0.rc1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 1/1] efi_loader: correct text conversion for vendor DP
Vendor device paths may contain data. When converting vendor device paths to text this binary data has to be rendered. UEFI SCT, 2017, 5.4.7.1.3 Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_device_path_to_text.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c index 96fd08971b..133542ad40 100644 --- a/lib/efi_loader/efi_device_path_to_text.c +++ b/lib/efi_loader/efi_device_path_to_text.c @@ -60,9 +60,18 @@ static char *dp_hardware(char *s, struct efi_device_path *dp) break; } case DEVICE_PATH_SUB_TYPE_VENDOR: { + int i, n; struct efi_device_path_vendor *vdp = (struct efi_device_path_vendor *)dp; - s += sprintf(s, "VenHw(%pUl)", >guid); + + s += sprintf(s, "VenHw(%pUl", >guid); + n = (int)vdp->dp.length - sizeof(struct efi_device_path_vendor); + if (n > 0) { + s += sprintf(s, ","); + for (i = 0; i < n; ++i) + s += sprintf(s, "%02x", vdp->vendor_data[i]); + } + s += sprintf(s, ")"); break; } default: -- 2.23.0.rc1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2 25/26] armv8: K3: j721e: Updated ddr address regions in MMU table
From: Kedar Chitnis The A72 U-Boot code loads and boots a number of remote processors including the C71x DSP, both the C66_0 and C66_1 DSPs, and the various Main R5FSS Cores. In order to view the code loaded by the U-Boot by remote cores, U-Boot should configure the memory region with right memory attributes. Right now U-Boot carves out a memory region which is not sufficient for all the images to be loaded. So, increase this carve out region by 256MB. Signed-off-by: Kedar Chitnis Signed-off-by: Lokesh Vutla --- arch/arm/mach-k3/arm64-mmu.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/arch/arm/mach-k3/arm64-mmu.c b/arch/arm/mach-k3/arm64-mmu.c index 98c5a777e5..7f908eee80 100644 --- a/arch/arm/mach-k3/arm64-mmu.c +++ b/arch/arm/mach-k3/arm64-mmu.c @@ -80,13 +80,13 @@ struct mm_region j721e_mem_map[NR_MMU_REGIONS] = { }, { .virt = 0xa000UL, .phys = 0xa000UL, - .size = 0x0bc0UL, + .size = 0x1bc0UL, .attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL_NC) | PTE_BLOCK_NON_SHARE }, { - .virt = 0xabc0UL, - .phys = 0xabc0UL, - .size = 0x5440UL, + .virt = 0xbbc0UL, + .phys = 0xbbc0UL, + .size = 0x4440UL, .attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) | PTE_BLOCK_INNER_SHARE }, { -- 2.22.0 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2 17/26] env: ti: k3_rproc: Add common rproc environment variables
From: Suman Anna Add a new file include/environment/ti/k3_rproc.h that defines common environment variables useful for booting various remote processors from U-Boot. This file is expected to be included in the board config files with the EXTRA_ENV_RPROC_SETTINGS added to CONFIG_EXTRA_ENV_SETTINGS and DEFAULT_RPROCS macro overwritten to include the actual list of processors to be booted. The 'boot_rprocs' variable just needs to be added to the board's bootcmd to automatically boot the processors, and runtime control can be achieved through the 'dorprocboot' variable. The variables are currently defined to use MMC as the boot media, and can be expanded in the future to include other boot media. The immediate usage is intended for K3 J721E SoCs. Signed-off-by: Jean-Jacques Hiblot Signed-off-by: Suman Anna --- include/environment/ti/k3_rproc.h | 52 +++ 1 file changed, 52 insertions(+) create mode 100644 include/environment/ti/k3_rproc.h diff --git a/include/environment/ti/k3_rproc.h b/include/environment/ti/k3_rproc.h new file mode 100644 index 00..3418cb42be --- /dev/null +++ b/include/environment/ti/k3_rproc.h @@ -0,0 +1,52 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com + * + * rproc environment variable definitions for various TI K3 SoCs. + */ + +#ifndef __TI_RPROC_H +#define __TI_RPROC_H + +/* + * should contain a list of tuplies, + * override in board config files with the actual list + */ +#define DEFAULT_RPROCS "" + +#ifdef CONFIG_CMD_REMOTEPROC +#define EXTRA_ENV_RPROC_SETTINGS \ + "dorprocboot=0\0" \ + "boot_rprocs=" \ + "if test ${dorprocboot} -eq 1 && test ${boot} = mmc; then "\ + "rproc init;" \ + "run boot_rprocs_mmc;" \ + "fi;\0" \ + "rproc_load_and_boot_one=" \ + "if load mmc ${bootpart} $loadaddr ${rproc_fw}; then " \ + "if rproc load ${rproc_id} ${loadaddr} ${filesize}; then "\ + "rproc start ${rproc_id};" \ + "fi;" \ + "fi\0" \ + "boot_rprocs_mmc=" \ + "env set rproc_id;" \ + "env set rproc_fw;" \ + "for i in ${rproc_fw_binaries} ; do " \ + "if test -z \"${rproc_id}\" ; then "\ + "env set rproc_id $i;" \ + "else " \ + "env set rproc_fw $i;" \ + "run rproc_load_and_boot_one;" \ + "env set rproc_id;" \ + "env set rproc_fw;" \ + "fi;" \ + "done\0"\ + "rproc_fw_binaries="\ + DEFAULT_RPROCS \ + "\0" +#else +#define EXTRA_ENV_RPROC_SETTINGS \ + "boot_rprocs= \0" +#endif /* CONFIG_CMD_REMOTEPROC */ + +#endif /* __TI_RPROC_H */ -- 2.22.0 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2 26/26] board: j721e: Add README
Add README file explaining the build and boot procedure for J721E evm. Signed-off-by: Lokesh Vutla --- board/ti/j721e/README | 227 ++ 1 file changed, 227 insertions(+) create mode 100644 board/ti/j721e/README diff --git a/board/ti/j721e/README b/board/ti/j721e/README new file mode 100644 index 00..5be7d099db --- /dev/null +++ b/board/ti/j721e/README @@ -0,0 +1,227 @@ +Introduction: +- +The J721e family of SoCs are part of K3 Multicore SoC architecture platform +targeting automotive applications. They are designed as a low power, high +performance and highly integrated device architecture, adding significant +enhancement on processing power, graphics capability, video and imaging +processing, virtualization and coherent memory support. + +The device is partitioned into three functional domains, each containing +specific processing cores and peripherals: +1. Wake-up (WKUP) domain: + - Device Management and Security Controller (DMSC) +2. Microcontroller (MCU) domain: + - Dual Core ARM Cortex-R5F processor +3. MAIN domain: + - Dual core 64-bit ARM Cortex-A72 + - 2 x Dual cortex ARM Cortex-R5 subsystem + - 2 x C66x Digital signal processor sub system + - C71x Digital signal processor sub-system with MMA. + +More info can be found in TRM: http://www.ti.com/lit/pdf/spruil1 + +Boot Flow: +-- +Boot flow is similar to that of AM65x SoC and extending it with remoteproc +support. Below is the pictorial representation of boot flow: + +++---+ +|DMSC| MCU R5 |A72| MAIN R5/C66x/C7x | +++---+ +|++ | | | | +|| Reset | | | | | +|++ | | | | +| : | | | | +|++ | +---+ | | | +|| *ROM* |--|-->| Reset rls | | | | +|++ | +---+ | | | +||| | : | | | +|| ROM | | : | | | +||services| | : | | | +||| | +-+ | | | +||| | | *R5 ROM* | | | | +||| | +-+ | | | +|||<-|---|Load and auth| | | | +||| | | tiboot3.bin | | | | +||| | +-+ | | | +||| | : | | | +||| | : | | | +||| | : | | | +||| | +-+ | | | +||| | | *R5 SPL* | | | | +||| | +-+ | | | +||| | |Load | | | | +||| | | sysfw.itb | | | | +|| Start | | +-+ | | | +|| System |<-|---|Start| | | | +||Firmware| | |SYSFW| | | | +|++ | +-+ | | | +|: | | | | | | +|+-+ | | Load | | | | +|| *SYSFW* | | | system| | | | +|
[U-Boot] [PATCH v2 22/26] configs: am65x_evm_a53: Enable R5F remoteproc driver
From: Suman Anna Enable the R5F remoteproc driver for the AM65x GP EVM so that the MCU domain R5F cores can be booted from A53 U-boot. Signed-off-by: Suman Anna Signed-off-by: Lokesh Vutla --- configs/am65x_evm_a53_defconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/configs/am65x_evm_a53_defconfig b/configs/am65x_evm_a53_defconfig index 9d02dbc01a..d6a0d8583c 100644 --- a/configs/am65x_evm_a53_defconfig +++ b/configs/am65x_evm_a53_defconfig @@ -35,6 +35,7 @@ CONFIG_CMD_GPT=y CONFIG_CMD_I2C=y CONFIG_CMD_MMC=y CONFIG_CMD_PCI=y +CONFIG_CMD_REMOTEPROC=y # CONFIG_CMD_SETEXPR is not set CONFIG_CMD_TIME=y # CONFIG_ISO_PARTITION is not set @@ -83,6 +84,7 @@ CONFIG_SPL_PINCTRL=y CONFIG_PINCTRL_SINGLE=y CONFIG_POWER_DOMAIN=y CONFIG_TI_SCI_POWER_DOMAIN=y +CONFIG_REMOTEPROC_TI_K3_R5F=y CONFIG_DM_RESET=y CONFIG_RESET_TI_SCI=y CONFIG_DM_SERIAL=y -- 2.22.0 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2 20/26] configs: j721e_evm_a72: Enable R5F and DSP remoteproc driver
Enable R5F and DSP remoteproc drivers for j721e running on a72. Signed-off-by: Lokesh Vutla --- configs/j721e_evm_a72_defconfig | 3 +++ 1 file changed, 3 insertions(+) diff --git a/configs/j721e_evm_a72_defconfig b/configs/j721e_evm_a72_defconfig index 6e355f5247..6e0c3f05fb 100644 --- a/configs/j721e_evm_a72_defconfig +++ b/configs/j721e_evm_a72_defconfig @@ -34,6 +34,7 @@ CONFIG_SPL_YMODEM_SUPPORT=y CONFIG_CMD_ASKENV=y # CONFIG_CMD_FLASH is not set CONFIG_CMD_MMC=y +CONFIG_CMD_REMOTEPROC=y CONFIG_CMD_SF=y # CONFIG_CMD_SETEXPR is not set CONFIG_CMD_TIME=y @@ -72,6 +73,8 @@ CONFIG_SPL_PINCTRL=y CONFIG_PINCTRL_SINGLE=y CONFIG_POWER_DOMAIN=y CONFIG_TI_SCI_POWER_DOMAIN=y +CONFIG_REMOTEPROC_TI_K3_DSP=y +CONFIG_REMOTEPROC_TI_K3_R5F=y CONFIG_DM_RESET=y CONFIG_RESET_TI_SCI=y CONFIG_DM_SERIAL=y -- 2.22.0 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2 23/26] configs: am65x_evm_a53: Enhance bootcmd to start remoteprocs
From: Suman Anna The A53 U-boot can support early booting of the MCU R5F remote processor(s) from U-boot prompt to achieve various system usecases before booting the Linux kernel. Update the default BOOTCOMMAND to provide an automatic and easier way to start the MCU R5F cores through added environment variables. Signed-off-by: Suman Anna Signed-off-by: Lokesh Vutla --- configs/am65x_evm_a53_defconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configs/am65x_evm_a53_defconfig b/configs/am65x_evm_a53_defconfig index d6a0d8583c..2d91d25b6a 100644 --- a/configs/am65x_evm_a53_defconfig +++ b/configs/am65x_evm_a53_defconfig @@ -16,7 +16,7 @@ CONFIG_DISTRO_DEFAULTS=y # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set CONFIG_SPL_LOAD_FIT=y CONFIG_OF_BOARD_SETUP=y -CONFIG_BOOTCOMMAND="run findfdt; run envboot; run init_${boot}; run get_kern_${boot}; run get_fdt_${boot}; run get_overlay_${boot}; run run_kern" +CONFIG_BOOTCOMMAND="run findfdt; run envboot; run init_${boot}; run boot_rprocs; run get_kern_${boot}; run get_fdt_${boot}; run get_overlay_${boot}; run run_kern" # CONFIG_DISPLAY_CPUINFO is not set CONFIG_SPL_TEXT_BASE=0x8008 CONFIG_SPL_SYS_MALLOC_SIMPLE=y -- 2.22.0 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2 18/26] env: ti: j721e-evm: Add support to boot rprocs including R5Fs and DSPs
From: Suman Anna Add support to boot some remoteprocs at U-boot prompt on the J721E EVM boards by using the 'boot_rprocs' and other env variables defined in the common environment file k3_rproc.h, and updating the 'DEFAULT_RPROCS' macro. The list of R5F cores to be started before loading and booting the Linux kernel are as follows, and in this order: Main R5FSS0 (Split) Core1 : 3 /lib/firmware/j7-main-r5f0_1-fw Main R5FSS1 (LockStep): 4 /lib/firmware/j7-main-r5f1_0-fw The MCU R5FSS0 and Main R5FSS1 are currently in LockStep mode, so the equivalent Core1 rprocs (rproc #1 and #5) are not included. The Main R5FSS0 Core0 (rproc #2) is already started by R5 SPL, so is not included in the list either. The DSP cores are started in the following order before loading and booting the Linux kernel: C66_0: 6 /lib/firmware/j7-c66_0-fw C66_1: 7 /lib/firmware/j7-c66_1-fw C71_0: 8 /lib/firmware/j7-c71_0-fw The order of the rprocs to boot can be changed at runtime if desired by overwriting the 'rproc_fw_binaries' environment variable at U-boot prompt. Signed-off-by: Jean-Jacques Hiblot Signed-off-by: Suman Anna Signed-off-by: Lokesh Vutla --- include/configs/j721e_evm.h | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/include/configs/j721e_evm.h b/include/configs/j721e_evm.h index 5fe77ef16d..dbe226b46d 100644 --- a/include/configs/j721e_evm.h +++ b/include/configs/j721e_evm.h @@ -12,6 +12,7 @@ #include #include #include +#include #define CONFIG_ENV_SIZE(128 << 10) @@ -87,11 +88,22 @@ "get_kern_mmc=load mmc ${bootpart} ${loadaddr} "\ "${bootdir}/${name_kern}\0" +#ifdef DEFAULT_RPROCS +#undef DEFAULT_RPROCS +#endif +#define DEFAULT_RPROCS "" \ + "3 /lib/firmware/j7-main-r5f0_1-fw "\ + "4 /lib/firmware/j7-main-r5f1_0-fw "\ + "6 /lib/firmware/j7-c66_0-fw " \ + "7 /lib/firmware/j7-c66_1-fw " \ + "8 /lib/firmware/j7-c71_0-fw " + /* Incorporate settings into the U-Boot environment */ #define CONFIG_EXTRA_ENV_SETTINGS \ DEFAULT_MMC_TI_ARGS \ EXTRA_ENV_J721E_BOARD_SETTINGS \ - EXTRA_ENV_J721E_BOARD_SETTINGS_MMC + EXTRA_ENV_J721E_BOARD_SETTINGS_MMC \ + EXTRA_ENV_RPROC_SETTINGS /* Now for the remaining common defines */ #include -- 2.22.0 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] DFU and UBIFS Volume upgrade
Thanks Heiko, On Wed, 4 Sep 2019 at 07:24, Heiko Schocher wrote: > Hello Loic, > > added Lukasz as he is the DFU custodian. > > Am 03.09.2019 um 09:59 schrieb Loic Poulain: > > Hi, > > > > AFAIU, today it's possible to update a UBI partition via DFU with a new > UBI > > blob using 'partubi'. > > Yes. > > > However, this causes two issues/limitations: > > - It erases the partition, causing PEB erase counters amnesia (contrary > to > > Linux ubiformat) > > Yes, patches which fixes this are welcome :-P > > > - It's no possible to have a volume-grained upgrade (per UBIFS volume) > > I am not to deep in the DFU topic involved, but I think the problem > is that ubi is not an interface like "nand" or "mmc" it is more something > like a partition type ... > > The big problem with writting an ubi image into nand is, that you > need to store the image first in RAM and you may have not enough ... > so may writting volume serverally it may help out here. > > On the other side, it is may possible to introduce an ubi interface for > UBI volumes. But what if your board has not setup yet UBI, nor the UBI > Volumes? Is there an interface in DFU for setting up something like > "partitions" ? You need to pass several parameters to create a new > UBI Volume ... > That's a good point. This makes things a bit complicated. > May Lukasz can say here more ... > > I prefer nowadays to boot a linux and use swupdate, which can handle > UBI Volumes ... > I agree, so let's keep that out of DFU. So, the priority is maybe to preserve erase-counters to keep optimal wear leveling. This should be quite simple to fix, I'll look at the UBI spec and come back with a patch. Regards, Loic ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] imx: Introduce CONFIG_SPL_FORCE_MMC_BOOT to force MMC boot on falcon mode
On Wed, 4 Sep 2019 11:54:40 +0200 Lukasz Majewski wrote: > Hi Stefano, Heiko, > > > On 04/09/19 10:46, Lukasz Majewski wrote: > > > Hi Heiko, > > > > > >> Hello Lukasz, > > >> > > >> added Stefano to cc as he is the imx custodian. > > > > > > I've relied on patman ... Thanks for adding Stefano to CC. > > > > > >> > > >> Am 03.09.2019 um 23:43 schrieb Lukasz Majewski: > > >>> This change tries to fix the following problem: > > >>> > > >>> - The board boots (to be more precise - ROM loads SPL) from a > > >>> slow SPI-NOR memory. > > >>>As a result the spl_boot_device() will return SPI-NOR as a > > >>> boot device (which is correct). > > >>> > > >>> - The problem is that in 'falcon boot' the eMMC is used as a > > >>> boot medium to load kernel from its partition. > > >>>Calling spl_boot_device() will break things as it returns > > >>> SPI-NOR device. > > >>> > > >>> To fix this issue the new CONFIG_SPL_FORCE_MMC_BOOT Kconfig flag > > >>> is introduced to handle this special use case. By default it is > > >>> not defined, so there is no change in the legacy code flow. > > >>> > > >>> Signed-off-by: Lukasz Majewski > > >>> > > >>> > > >>> --- > > >>> > > >>> This patch is a follow up of previous discussion/fix: > > >>> https://patchwork.ozlabs.org/patch/796237/ > > >>> > > >>> Travis-CI: > > >>> https://travis-ci.org/lmajewski/u-boot-dfu/builds/580272414 > > >>> --- > > >>> arch/arm/mach-imx/spl.c | 11 +++ > > >>> common/spl/Kconfig | 7 +++ > > >>> 2 files changed, 18 insertions(+) > > >> > > >> I have just similiar change for an imx6ull based board ... > > > > > > Also one more of my boards uses this trick (i.MX28 based one). > > > > > >> just antoher usecase: In factory empty board boots into USB SDP > > >> mode, and SPL gets loaded with usb_loader ... SPL detects a sd > > >> card (there is no sd card slot mounted, mmc boot is disabled! > > >> They attach only one for installing software) > > > > > > So there is no dedicated SD card slot (also the mmc is disabled on > > > that point). > > > Instead, in the factory the sd card is attached to pads - just for > > > this time. > > > > > >> and boots U-Boot and system from sd card. > > >> Than all software gets installed fully automated with the help of > > >> swupdate ... > > > > > > Ok. > > > > > >> > > >>> diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c > > >>> index 1f230aca3397..daa3d8f7ed94 100644 > > >>> --- a/arch/arm/mach-imx/spl.c > > >>> +++ b/arch/arm/mach-imx/spl.c > > >>> @@ -178,7 +178,18 @@ int g_dnl_bind_fixup(struct > > >>> usb_device_descriptor *dev, const char *name) /* called from > > >>> spl_mmc to see type of boot mode for storage (RAW or FAT) */ u32 > > >>> spl_boot_mode(const u32 boot_device) { > > >>> +/* > > >>> + * When CONFIG_SPL_FORCE_MMC_BOOT is defined the 'boot_device' > > >>> is used > > >>> + * unconditionally to decide about device to use for booting. > > >>> + * This is crucial for falcon boot mode, when board boots up > > >>> (i.e. ROM > > >>> + * loads SPL) from slow SPI-NOR memory and afterwards the SPL's > > >>> 'falcon' boot > > >>> + * mode is used to load Linux OS from eMMC partition. > > >>> + */ > > >>> +#ifdef CONFIG_SPL_FORCE_MMC_BOOT > > >>> + switch (boot_device) { > > >>> +#else > > >>> switch (spl_boot_device()) { > > >>> +#endif > > >> > > >> Just dummy question .. couldn;t we switch to use always > > >> boot_device? > > > > > > Stefano has provided some rationale for the need of > > > spl_boot_device() in the previous version of this fix [1]: > > > > > > https://patchwork.ozlabs.org/patch/796237/ > > > > > > > > >> > > >> In my case I had a board specific: > > >> > > >> void board_boot_order(u32 *spl_boot_list) > > >> { > > >> spl_boot_list[0] = BOOT_DEVICE_MMC1; > > >> spl_boot_list[1] = BOOT_DEVICE_SPI; > > >> spl_boot_list[2] = BOOT_DEVICE_BOARD; > > >> spl_boot_list[3] = BOOT_DEVICE_NONE; > > >> } > > >> > > >> which leads that BOOT_DEVICE_MMC1 is passed to spl_boot_mode() > > >> > > > > > > Is your board normally booting from MMC or SPI-NOR (from where SoC > > > ROM loads SPL) ? > > > > > >> > > >> If MMC1 is not found, it tries to boot U-Boot from SPI, and if > > >> this is empty, as fallback board go into USB SDP mode. > > >> > > >> The weak function of board_boot_order is: > > >> > > >> __weak void board_boot_order(u32 *spl_boot_list) > > >> { > > >> spl_boot_list[0] = spl_boot_device(); > > >> } > > >> > > >> which is exactly what we pass to spl_boot_mode() ... so instead > > >> calling spl_boot_device() twice we can use the passed boot_device > > >> ... and save the ifdef and new Kconfig option. > > >> > > >> But may I oversee something ? > > > > > > Please read the Stefano's reply from [1] - the spl_boot_device() > > > has a valid use cases as well. > > > > Nevertheless, why do we need to add a
[U-Boot] [PATCH v2 09/26] remoteproc: Introduce K3 remoteproc driver for R5F subsystem
SoCs with K3 architecture have an integrated Arm Cortex-R5F subsystem that is comprised of dual-core Arm Cortex-R5F processor cores. This R5 subsytem can be configured at boot time to be either run in a LockStep mode or in an Asymmetric Multi Processing (AMP) fashion in Split-mode. This subsystem has each Tightly-Coupled Memory (TCM) internal memories for each core split between two banks - TCMA and TCMB. Add a remoteproc driver to support this subsystem to be able to load and boot the R5 cores primarily in LockStep mode or split mode. Signed-off-by: Lokesh Vutla Signed-off-by: Suman Anna --- drivers/remoteproc/Kconfig | 10 + drivers/remoteproc/Makefile | 1 + drivers/remoteproc/ti_k3_r5f_rproc.c | 816 +++ 3 files changed, 827 insertions(+) create mode 100644 drivers/remoteproc/ti_k3_r5f_rproc.c diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig index f54a245424..c2d59ba000 100644 --- a/drivers/remoteproc/Kconfig +++ b/drivers/remoteproc/Kconfig @@ -52,6 +52,16 @@ config REMOTEPROC_TI_K3_ARM64 on various TI K3 family of SoCs through the remote processor framework. +config REMOTEPROC_TI_K3_R5F + bool "TI K3 R5F remoteproc support" + select REMOTEPROC + depends on ARCH_K3 + depends on TI_SCI_PROTOCOL + help + Say y here to support TI's R5F remote processor subsystems + on various TI K3 family of SoCs through the remote processor + framework. + config REMOTEPROC_TI_POWER bool "Support for TI Power processor" select REMOTEPROC diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile index 271ba55b09..9d247ba5e7 100644 --- a/drivers/remoteproc/Makefile +++ b/drivers/remoteproc/Makefile @@ -11,4 +11,5 @@ obj-$(CONFIG_K3_SYSTEM_CONTROLLER) += k3_system_controller.o obj-$(CONFIG_REMOTEPROC_SANDBOX) += sandbox_testproc.o obj-$(CONFIG_REMOTEPROC_STM32_COPRO) += stm32_copro.o obj-$(CONFIG_REMOTEPROC_TI_K3_ARM64) += ti_k3_arm64_rproc.o +obj-$(CONFIG_REMOTEPROC_TI_K3_R5F) += ti_k3_r5f_rproc.o obj-$(CONFIG_REMOTEPROC_TI_POWER) += ti_power_proc.o diff --git a/drivers/remoteproc/ti_k3_r5f_rproc.c b/drivers/remoteproc/ti_k3_r5f_rproc.c new file mode 100644 index 00..ae1e4b9e04 --- /dev/null +++ b/drivers/remoteproc/ti_k3_r5f_rproc.c @@ -0,0 +1,816 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Texas Instruments' K3 R5 Remoteproc driver + * + * Copyright (C) 2018-2019 Texas Instruments Incorporated - http://www.ti.com/ + * Lokesh Vutla + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include "ti_sci_proc.h" + +/* + * R5F's view of this address can either be for ATCM or BTCM with the other + * at address 0x0 based on loczrama signal. + */ +#define K3_R5_TCM_DEV_ADDR 0x4101 + +/* R5 TI-SCI Processor Configuration Flags */ +#define PROC_BOOT_CFG_FLAG_R5_DBG_EN 0x0001 +#define PROC_BOOT_CFG_FLAG_R5_DBG_NIDEN0x0002 +#define PROC_BOOT_CFG_FLAG_R5_LOCKSTEP 0x0100 +#define PROC_BOOT_CFG_FLAG_R5_TEINIT 0x0200 +#define PROC_BOOT_CFG_FLAG_R5_NMFI_EN 0x0400 +#define PROC_BOOT_CFG_FLAG_R5_TCM_RSTBASE 0x0800 +#define PROC_BOOT_CFG_FLAG_R5_BTCM_EN 0x1000 +#define PROC_BOOT_CFG_FLAG_R5_ATCM_EN 0x2000 +#define PROC_BOOT_CFG_FLAG_GEN_IGN_BOOTVECTOR 0x1000 + +/* R5 TI-SCI Processor Control Flags */ +#define PROC_BOOT_CTRL_FLAG_R5_CORE_HALT 0x0001 + +/* R5 TI-SCI Processor Status Flags */ +#define PROC_BOOT_STATUS_FLAG_R5_WFE 0x0001 +#define PROC_BOOT_STATUS_FLAG_R5_WFI 0x0002 +#define PROC_BOOT_STATUS_FLAG_R5_CLK_GATED 0x0004 +#define PROC_BOOT_STATUS_FLAG_R5_LOCKSTEP_PERMITTED0x0100 + +#define NR_CORES 2 + +enum cluster_mode { + CLUSTER_MODE_SPLIT = 0, + CLUSTER_MODE_LOCKSTEP, +}; + +/** + * struct k3_r5_mem - internal memory structure + * @cpu_addr: MPU virtual address of the memory region + * @bus_addr: Bus address used to access the memory region + * @dev_addr: Device address from remoteproc view + * @size: Size of the memory region + */ +struct k3_r5f_mem { + void __iomem *cpu_addr; + phys_addr_t bus_addr; + u32 dev_addr; + size_t size; +}; + +/** + * struct k3_r5f_core - K3 R5 core structure + * @dev: cached device pointer + * @cluster: pointer to the parent cluster. + * @reset: reset control handle + * @tsp: TI-SCI processor control handle + * @mem: Array of available internal memories + * @num_mem: Number of available memories + * @atcm_enable: flag to control ATCM enablement + * @btcm_enable: flag to control BTCM enablement + * @loczrama: flag to dictate which TCM is at device address 0x0 + * @in_use: flag to tell if the core is already in use. + */ +struct
[U-Boot] [PATCH v2 24/26] armv8: K3: am65x: Update DDR address regions in MMU table
From: Suman Anna The A53 U-Boot code can load and boot the MCU domain R5F cores (either a single core in LockStep mode or 2 cores in Split mode) to achieve various early system functionalities. Change the memory attributes for the DDR regions used by the remote processors so that the cores can see and execute the proper code loaded by U-Boot. These regions are currently limited to 0xa000 to 0xa210 as per the DDR carveouts assigned for these R5F cores in the overall DDR memory map. Signed-off-by: Suman Anna Signed-off-by: Lokesh Vutla --- arch/arm/mach-k3/arm64-mmu.c | 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-k3/arm64-mmu.c b/arch/arm/mach-k3/arm64-mmu.c index 82778d2197..98c5a777e5 100644 --- a/arch/arm/mach-k3/arm64-mmu.c +++ b/arch/arm/mach-k3/arm64-mmu.c @@ -14,7 +14,7 @@ #ifdef CONFIG_SOC_K3_AM6 /* NR_DRAM_BANKS + 32bit IO + 64bit IO + terminator */ -#define NR_MMU_REGIONS (CONFIG_NR_DRAM_BANKS + 3) +#define NR_MMU_REGIONS (CONFIG_NR_DRAM_BANKS + 5) /* ToDo: Add 64bit IO */ struct mm_region am654_mem_map[NR_MMU_REGIONS] = { @@ -28,7 +28,19 @@ struct mm_region am654_mem_map[NR_MMU_REGIONS] = { }, { .virt = 0x8000UL, .phys = 0x8000UL, - .size = 0x8000UL, + .size = 0x2000UL, + .attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) | +PTE_BLOCK_INNER_SHARE + }, { + .virt = 0xa000UL, + .phys = 0xa000UL, + .size = 0x0210UL, + .attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL_NC) | +PTE_BLOCK_INNER_SHARE + }, { + .virt = 0xa210UL, + .phys = 0xa210UL, + .size = 0x5df0UL, .attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) | PTE_BLOCK_INNER_SHARE }, { -- 2.22.0 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2 07/26] remoteproc: tisci_proc: Add helper api for controlling core power domain
Power domain for the remote cores needs to be handled in a right sequence as mandated by the spec. Introduce tisci helper apis that can control power-domains of remote cores. TISCI clients can use this api and control the remote cores power domain instead of hooking it to power-domain layer. Signed-off-by: Lokesh Vutla --- drivers/remoteproc/ti_sci_proc.h | 27 +++ 1 file changed, 27 insertions(+) diff --git a/drivers/remoteproc/ti_sci_proc.h b/drivers/remoteproc/ti_sci_proc.h index ccfc39ec88..f8299d1aff 100644 --- a/drivers/remoteproc/ti_sci_proc.h +++ b/drivers/remoteproc/ti_sci_proc.h @@ -19,12 +19,14 @@ * @proc_id: processor id for the consumer remoteproc device * @host_id: host id to pass the control over for this consumer remoteproc * device + * @dev_id: Device ID as identified by system controller. */ struct ti_sci_proc { const struct ti_sci_handle *sci; const struct ti_sci_proc_ops *ops; u8 proc_id; u8 host_id; + u16 dev_id; }; static inline int ti_sci_proc_request(struct ti_sci_proc *tsp) @@ -118,4 +120,29 @@ static inline int ti_sci_proc_set_control(struct ti_sci_proc *tsp, return ret; } +static inline int ti_sci_proc_power_domain_on(struct ti_sci_proc *tsp) +{ + int ret; + + debug("%s: dev_id = %d\n", __func__, tsp->dev_id); + + ret = tsp->sci->ops.dev_ops.get_device_exclusive(tsp->sci, tsp->dev_id); + if (ret) + pr_err("Power-domain on failed for dev = %d\n", tsp->dev_id); + + return ret; +} + +static inline int ti_sci_proc_power_domain_off(struct ti_sci_proc *tsp) +{ + int ret; + + debug("%s: dev_id = %d\n", __func__, tsp->dev_id); + + ret = tsp->sci->ops.dev_ops.put_device(tsp->sci, tsp->dev_id); + if (ret) + pr_err("Power-domain off failed for dev = %d\n", tsp->dev_id); + + return ret; +} #endif /* REMOTEPROC_TI_SCI_PROC_H */ -- 2.22.0 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2 19/26] env: ti: am65x_evm: Add env support to boot the MCU R5F rprocs
From: Suman Anna Add support to boot the MCU domain R5F Core0 remoteproc at U-boot prompt on the AM65x EVM boards by using the 'boot_rprocs' and other env variables defined in the common environment file k3_rproc.h, and updating the 'DEFAULT_RPROCS' macro. The default configuration is to use the MCU R5F in Split mode, so both the R5F Core0 and Core1 are started before loading and booting the Linux kernel using the following firmware: MCU R5FSS0 Core0 (Split) : 0 /lib/firmware/am65x-mcu-r5f0_0-fw MCU R5FSS0 Core1 (Split) : 1 /lib/firmware/am65x-mcu-r5f0_1-fw The MCU R5FSS was initially running the R5 SPL in LockStep mode with ATCM disabled, and is actually shutdown to enable it to be reconfigured and booted by either A53 U-Boot or Linux kernel in remoteproc mode and using ATCM. The MCU R5FSS would need to be reconfigured for Lockstep mode through DT if a fault-tolerant/safety application were to be run on the cluster with the DEFAULT_RPROCS macro updated to remove the Core1 firmware. Signed-off-by: Suman Anna Signed-off-by: Lokesh Vutla --- include/configs/am65x_evm.h | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/include/configs/am65x_evm.h b/include/configs/am65x_evm.h index 6072e4a48c..5c44b22ead 100644 --- a/include/configs/am65x_evm.h +++ b/include/configs/am65x_evm.h @@ -12,6 +12,7 @@ #include #include #include +#include #define CONFIG_ENV_SIZE(128 << 10) @@ -96,11 +97,19 @@ "${bootdir}/${name_kern}\0" \ "partitions=" PARTS_DEFAULT +#ifdef DEFAULT_RPROCS +#undef DEFAULT_RPROCS +#endif +#define DEFAULT_RPROCS "" \ + "0 /lib/firmware/am65x-mcu-r5f0_0-fw " \ + "1 /lib/firmware/am65x-mcu-r5f0_1-fw " + /* Incorporate settings into the U-Boot environment */ #define CONFIG_EXTRA_ENV_SETTINGS \ DEFAULT_MMC_TI_ARGS \ EXTRA_ENV_AM65X_BOARD_SETTINGS \ - EXTRA_ENV_AM65X_BOARD_SETTINGS_MMC + EXTRA_ENV_AM65X_BOARD_SETTINGS_MMC \ + EXTRA_ENV_RPROC_SETTINGS /* MMC ENV related defines */ #ifdef CONFIG_ENV_IS_IN_MMC -- 2.22.0 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2 10/26] dt-bindings: remoteproc: Add bindings for DSP C66x clusters on TI K3 SoCs
From: Suman Anna Some Texas Instruments K3 family of SoCs have one of more Digital Signal Processor (DSP) subsystems that are comprised of either a TMS320C66x CorePac and/or a next-generation TMS320C71x CorePac processor subsystem. Add the device tree bindings document for the C66x DSP devices on these SoCs. The added example illustrates the DT nodes for the first C66x DSP device present on the K3 J721E family of SoCs. Signed-off-by: Suman Anna Signed-off-by: Lokesh Vutla --- .../remoteproc/ti,k3-dsp-rproc.txt| 101 ++ 1 file changed, 101 insertions(+) create mode 100644 doc/device-tree-bindings/remoteproc/ti,k3-dsp-rproc.txt diff --git a/doc/device-tree-bindings/remoteproc/ti,k3-dsp-rproc.txt b/doc/device-tree-bindings/remoteproc/ti,k3-dsp-rproc.txt new file mode 100644 index 00..80ab7a4090 --- /dev/null +++ b/doc/device-tree-bindings/remoteproc/ti,k3-dsp-rproc.txt @@ -0,0 +1,101 @@ +TI K3 DSP devices += + +The TI K3 family of SoCs usually have one or more TI DSP Core sub-systems that +are used to offload some of the processor-intensive tasks or algorithms, for +achieving various system level goals. + +These processor sub-systems usually contain additional sub-modules like L1 +and/or L2 caches/SRAMs, an Interrupt Controller, an external memory controller, +a dedicated local power/sleep controller etc. The DSP processor cores in the +K3 SoCs is usually either a TMS320C66x CorePac processor or a TMS320C71x CorePac +processor. + +DSP Device Node: + +Each DSP Core sub-system is represented as a single DT node. Each node has a +number of required or optional properties that enable the OS running on the +host processor (Arm CorePac) to perform the device management of the remote +processor and to communicate with the remote processor. + +Required properties: + +The following are the mandatory properties: + +- compatible: Should be one of the following, + "ti,j721e-c66-dsp" for C66x DSPs on K3 J721E SoCs + "ti,j721e-c71-dsp" for C71x DSPs on K3 J721E SoCs + +- reg: Should contain an entry for each value in 'reg-names'. + Each entry should have the memory region's start address + and the size of the region, the representation matching + the parent node's '#address-cells' and '#size-cells' values. + +- reg-names: Should contain strings with the following names, each + representing a specific internal memory region (if + present), and should be defined in this order, +"l2sram", "l1pram", "l1dram" + NOTE: C71x DSPs do not have a "l1pram" memory. + +- ti,sci: Should be a phandle to the TI-SCI System Controller node + +- ti,sci-dev-id: Should contain the TI-SCI device id corresponding to the + DSP Core. Please refer to the corresponding System + Controller documentation for valid values for the DSP + cores. + +- ti,sci-proc-ids: Should contain 2 integer values. The first cell should + contain the TI-SCI processor id for the DSP core device + and the second cell should contain the TI-SCI host id to + which the processor control ownership should be + transferred to. + +- resets: Should contain the phandle to the reset controller node + managing the resets for this device, and a reset + specifier. Please refer to the following reset bindings + for the reset argument specifier, + Documentation/devicetree/bindings/reset/ti,sci-reset.txt + +Example: +- + +1. J721E SoC + /* J721E remoteproc alias */ + aliases { + rproc6 = _0; + rproc8 = _0; + }; + + cbass_main: interconnect@10 { + compatible = "simple-bus"; + #address-cells = <2>; + #size-cells = <2>; + ranges = <0x00 0x6480 0x00 0x6480 0x00 0x0080>, /* C71_0 */ +<0x4d 0x8080 0x4d 0x8080 0x00 0x0080>, /* C66_0 */ +<0x4d 0x8180 0x4d 0x8180 0x00 0x0080>; /* C66_1 */ + + /* J721E C66_0 DSP node */ + c66_0: dsp@4d8080 { + compatible = "ti,j721e-c66-dsp"; + reg = <0x4d 0x8080 0x00 0x00048000>, + <0x4d 0x80e0 0x00 0x8000>, + <0x4d 0x80f0 0x00 0x8000>; + reg-names = "l2sram", "l1pram", "l1dram"; + ti,sci = <>; + ti,sci-dev-id = <142>; +
[U-Boot] [PATCH v2 08/26] dt-bindings: remoteproc: Add bindings for R5F subsystem on TI K3 SoCs
From: Suman Anna The Texas Instruments K3 family of SoCs have one of more dual-core Arm Cortex R5F processor subsystems/clusters (R5FSS). Add the device tree bindings document for these R5F subsystem devices. These R5F processors do not have an MMU, and so require fixed memory carveout regions matching the firmware image addresses. The nodes require more than one memory region, with the first memory region used for DMA allocations at runtime. The remaining memory regions are reserved and are used for the loading and running of the R5F remote processors. The added example illustrates the DT nodes for the single R5FSS device present on K3 AM65x family of SoCs. Signed-off-by: Suman Anna Signed-off-by: Lokesh Vutla --- .../remoteproc/ti,k3-r5f-rproc.txt| 164 ++ 1 file changed, 164 insertions(+) create mode 100644 doc/device-tree-bindings/remoteproc/ti,k3-r5f-rproc.txt diff --git a/doc/device-tree-bindings/remoteproc/ti,k3-r5f-rproc.txt b/doc/device-tree-bindings/remoteproc/ti,k3-r5f-rproc.txt new file mode 100644 index 00..c2fa6e8344 --- /dev/null +++ b/doc/device-tree-bindings/remoteproc/ti,k3-r5f-rproc.txt @@ -0,0 +1,164 @@ +TI K3 R5F processor subsystems +== + +The TI K3 family of SoCs usually have one or more dual-core Arm Cortex +R5F processor subsystems/clusters (R5FSS). The dual core cluster can be +used either in a LockStep mode providing safety/fault tolerance features +or in a Split mode providing two individual compute cores for doubling +the compute capacity. These are used together with other processors +present on the SoC to achieve various system level goals. + +R5F Sub-System Device Node: +=== +Each Dual-Core R5F sub-system is represented as a single DTS node representing +the cluster, with a pair of child DT nodes representing the individual R5F +cores. Each node has a number of required or optional properties that enable +the OS running on the host processor to perform the device management of the +remote processor and to communicate with the remote processor. + +Required properties: + +The following are the mandatory properties: + +- compatible: Should be one of the following, + "ti,am654-r5fss" for R5F clusters/subsystems on + K3 AM65x SoCs + "ti,j721e-r5fss" for R5F clusters/subsystems on + K3 J721E SoCs +- power-domains: Should contain a phandle to a PM domain provider node + and an args specifier containing the R5FSS device id + value. This property is as per the binding, + Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt +- #address-cells: Should be 1 +- #size-cells: Should be 1 +- ranges: Standard ranges definition providing translations for + R5F TCM address spaces + +Optional properties: + +- lockstep-mode: Configuration Mode for the Dual R5F cores within the R5F + cluster. Should be either a value of 1 (LockStep mode) or + 0 (Split mode), default is LockStep mode if omitted. + + +R5F Processor Child Nodes: +== +The R5F Sub-System device node should define two R5F child nodes, each node +representing a TI instantiation of the Arm Cortex R5F core. There are some +specific integration differences for the IP like the usage of a Region Address +Translator (RAT) for translating the larger SoC bus addresses into a 32-bit +address space for the processor. + +Required properties: + +The following are the mandatory properties: + +- compatible: Should be one of the following, + "ti,am654-r5f" for the R5F cores in K3 AM65x SoCs + "ti,j721e-r5f" for the R5F cores in K3 J721E SOCs +- reg: Should contain an entry for each value in 'reg-names'. + Each entry should have the memory region's start address + and the size of the region, the representation matching + the parent node's '#address-cells' and '#size-cells' values. +- reg-names: Should contain strings with the following names, each + representing a specific internal memory region, and + should be defined in this order, +"atcm", "btcm" +- ti,sci: Should be a phandle to the TI-SCI System Controller node +- ti,sci-dev-id: Should contain the TI-SCI device id corresponding to the + R5F Core. Please refer to the corresponding System + Controller documentation for valid values for the R5F + cores. +- ti,sci-proc-ids: Should contain 2