Re: [PATCH v3 1/2] imx8mm-evk: Generate a single bootable flash.bin again
Hi Fabio,
Am Do., 19. Aug. 2021 um 21:28 Uhr schrieb Fabio Estevam :
>
> After the conversion to binman in commit 8996e6b7c6a1 ("imx8mm_evk: switch
> to use binman to pack images"), it is necessary to flash both flash.bin and
> u-boot.itb to get a bootable system. Prior to this commit, only flash.bin
> was needed.
>
> Such new requirement breaks existing distro mechanisms to generate the
> final binary because the extra u-boot.itb is now required.
>
> Generate a final flash.bin that can be used again as a single
> bootable binary to keep the original behavior.
>
> After this change the SPL binary is called spl.bin, which is a more
> descriptive name for its purpose, and can still be used standalone
> (for example, for secure boot purposes).
>
> Also update imx8mm_evk.rst to remove the u-boot.itb copy step.
>
> Signed-off-by: Fabio Estevam
> Reviewed-by: Frieder Schrempf
> Reviewed-by: Heiko Schocher
>
> Signed-off-by: Fabio Estevam
> ---
> Changes since v2:
> - Change the LOADER to mkimage.spl.mkimage (Frieder)
>
> arch/arm/dts/imx8mm-evk-u-boot.dtsi | 17 -
> .../imx8mm_evk/imximage-8mm-lpddr4.cfg | 2 +-
> doc/board/freescale/imx8mm_evk.rst | 1 -
> 3 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> index f200afac9f..75cd59e545 100644
> --- a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> +++ b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> @@ -150,7 +150,7 @@
> };
>
>
> - flash {
> + spl {
> mkimage {
> args = "-n spl/u-boot-spl.cfgout -T imx8mimage -e
> 0x7e1000";
>
> @@ -217,4 +217,19 @@
> };
> };
> };
> +
> + imx-boot {
> + filename = "flash.bin";
> + pad-byte = <0x00>;
> +
> + spl: blob-ext@1 {
> + offset = <0x0>;
> + filename = "spl.bin";
> + };
> +
> + uboot: blob-ext@2 {
> + offset = <0x57c00>;
> + filename = "u-boot.itb";
> + };
> + };
> };
> diff --git a/board/freescale/imx8mm_evk/imximage-8mm-lpddr4.cfg
> b/board/freescale/imx8mm_evk/imximage-8mm-lpddr4.cfg
> index b89092a559..2c15dbc413 100644
> --- a/board/freescale/imx8mm_evk/imximage-8mm-lpddr4.cfg
> +++ b/board/freescale/imx8mm_evk/imximage-8mm-lpddr4.cfg
> @@ -6,4 +6,4 @@
> #define __ASSEMBLY__
>
> BOOT_FROM sd
> -LOADER mkimage.flash.mkimage 0x7E1000
> +LOADER mkimage.spl.mkimage 0x7E1000
> diff --git a/doc/board/freescale/imx8mm_evk.rst
> b/doc/board/freescale/imx8mm_evk.rst
> index 7fd3d72564..b377c4de27 100644
> --- a/doc/board/freescale/imx8mm_evk.rst
> +++ b/doc/board/freescale/imx8mm_evk.rst
> @@ -50,7 +50,6 @@ Burn the flash.bin to MicroSD card offset 33KB:
> .. code-block:: bash
>
> $sudo dd if=flash.bin of=/dev/sd[x] bs=1024 seek=33 conv=notrunc
> - $sudo dd if=u-boot.itb of=/dev/sdc bs=1024 seek=384 conv=sync
>
> Boot
>
> --
> 2.25.1
>
When building from a clean checkout I see the following warnings. It
seems that there are some dependency checks that are looking for the
files in mkimage config file.
AR arch/arm/lib/lib.a
AS arch/arm/lib/crt0_aarch64_efi.o
CC arch/arm/lib/reloc_aarch64_efi.o
WARNING 'mkimage.spl.mkimage' not found, resulting binary is not-functional
AS arch/arm/mach-imx/imx8m/lowlevel_init.o
CC arch/arm/mach-imx/imx8m/clock_slice.o
CC arch/arm/mach-imx/imx8m/soc.o
:
:
AS spl/arch/arm/lib/crt0_aarch64_efi.o
CC spl/arch/arm/lib/reloc_aarch64_efi.o
WARNING 'mkimage.spl.mkimage' not found, resulting binary is not-functional
AS spl/arch/arm/mach-imx/imx8m/lowlevel_init.o
CC spl/arch/arm/mach-imx/imx8m/clock_slice.o
CC spl/arch/arm/mach-imx/imx8m/soc.o
:
:
COPYspl/u-boot-spl.bin
SYM spl/u-boot-spl.sym
WARNING 'mkimage.spl.mkimage' not found, resulting binary is not-functional
make[1]: Nothing to be done for 'SPL'.
OBJCOPY u-boot.srec
OBJCOPY u-boot-nodtb.bin
--
Heiko
Re: [PATCH v3 2/2] Makefile: Clean the i.MX8MM artifacts
Hi Fabio, Am Do., 19. Aug. 2021 um 21:28 Uhr schrieb Fabio Estevam : > > Clean the binaries generated by binman on imx8mm-evk: > spl.* mkimage*.mkimage imx-boot.* > > Reported-by: Frieder Schrempf > Signed-off-by: Fabio Estevam > --- > Changes since v2: > - None. Newly introducedin this series. > > Makefile | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index 3c8437d21a..7096fdf895 100644 > --- a/Makefile > +++ b/Makefile > @@ -2095,7 +2095,8 @@ CLEAN_FILES += include/bmp_logo.h > include/bmp_logo_data.h tools/version.h \ >boot* u-boot* MLO* SPL System.map fit-dtb.blob* \ >u-boot-ivt.img.log u-boot-dtb.imx.log SPL.log u-boot.imx.log \ >lpc32xx-* bl31.c bl31.elf bl31_*.bin image.map tispl.bin* \ > - idbloader.img flash.bin flash.log defconfig keep-syms-lto.c > + idbloader.img flash.bin flash.log defconfig keep-syms-lto.c \ > + spl.* mkimage*.mkimage imx-boot.* it might be useful to use one variable for all BINMAN clean files. Otherwise it is difficult to understand by whom the files were created. Something like that: --- a/Makefile +++ b/Makefile @@ -2091,12 +2091,14 @@ CLEAN_DIRS += $(MODVERDIR) \ $(foreach d, spl tpl, $(patsubst %,$d/%, \ $(filter-out include, $(shell ls -1 $d 2>/dev/null +BINMAN_CLEAN_FILES = spl.* mkimage*.mkimage + CLEAN_FILES += include/bmp_logo.h include/bmp_logo_data.h tools/version.h \ boot* u-boot* MLO* SPL System.map fit-dtb.blob* \ u-boot-ivt.img.log u-boot-dtb.imx.log SPL.log u-boot.imx.log \ lpc32xx-* bl31.c bl31.elf bl31_*.bin image.map tispl.bin* \ idbloader.img flash.bin flash.log defconfig keep-syms-lto.c \ - spl.* mkimage*.mkimage imx-boot.* + imx-boot.* $(BINMAN_CLEAN_FILES) + I had the idea, if binman could create a list with the created files. This could then be used to set the files to be deleted. I think of other users where the output files have a different name. Then the files could be deleted "automatically" with clean. -- Heiko
Re: [PATCH 00/28] Initial implementation of bootmethod/bootflow
Hi Simon, On Thu, Aug 19, 2021 at 08:25:33AM -0600, Simon Glass wrote: > Hi Tom, > > On Thu, 19 Aug 2021 at 07:59, Tom Rini wrote: > > > > On Wed, Aug 18, 2021 at 09:45:33PM -0600, Simon Glass wrote: > > > > > Bootmethod and bootflow provide a built-in way for U-Boot to > > > automatically boot > > > an Operating System without custom scripting and other customisation: > > > > > > - bootmethod - a method to scan a device to find bootflows (owned by > > > U-Boot) > > > - bootflow - a description of how to boot (owned by the distro) > > > > > > This series provides an initial implementation of these, enable to scan > > > for bootflows from MMC and Ethernet. The only bootflow supported is > > > distro boot, i.e. an extlinux.conf file included on a filesystem or > > > tftp server. It works similiarly to the existing script-based approach, > > > but is native to U-Boot. > > > > > > With this we can boot on a Raspberry Pi 3 with just one command: > > > > > >bootflow scan -lb > > > > > > which means to scan, listing (-l) each bootflow and trying to boot each > > > one (-b). The final patch shows this. > > > > > > It is intended that this approach be expanded to support mechanisms other > > > than distro boot, including EFI-related ones. With a standard way to > > > identify boot devices, these features become easier. It also should > > > support U-Boot scripts, for backwards compatibility only. > > > > > > The first patch of this series moves boot-related code out of common/ and > > > into a new boot/ directory. This helps to collect these related files > > > in one place, as common/ is quite large. > > > > > > Like sysboot, this feature makes use of the existing PXE implementation. > > > Much of this series consists of cleaning up that code and refactoring it > > > into something closer to a module that can be called, teasing apart its > > > reliance on the command-line interpreter to access filesystems and the > > > like. Also it now uses function arguments and its own context struct > > > internally rather than environment variables, which is very hard to > > > follow. No core functional change is included in the included PXE patches. > > > > > > For documentation, see the 'doc' patch. > > > > > > There is quite a long list of future work included in the documentation. > > > One question is the choice of naming. Since this is a bootloader, should > > > we just call this a 'method' and a 'flow' ? The 'boot' prefix is already > > > shared by other commands like bootm, booti, etc. Regarding the naming, I'm still a bit confused with bootmethod vs. bootflow. Personally, I prefer a more intuitive name against bootmethod, say, bootmedia or bootdevice (as the original distro_bootcmd uses). > > > The design is described here: > > > > > > https://drive.google.com/file/d/1ggW0KJpUOR__vBkj3l61L2dav4ZkNC12/view?usp=sharing > > > > > > The series is available at u-boot-dm/bmea-working > > > > My question / concern is this. Would the next step here be to > > implement the generic UEFI boot path? Today, I can write Fedora 34 for > > AArch64 to a USB stick, boot U-Boot off of uSD card and the installer > > automatically boots. I'm sure I could do the same with the BSDs. > > Reading the documentation left me with the impression that every OSV > > would be expected to write something, so that their installer / OS boot. > > But there's already standards for that, which they do, and we should be > > implementing (and do, via the current distro_boot) or making easier to > > enable. Thanks! I had the same concern. > Here you are talking about scanning for a bootflow. It is not actually > OS-specific. If it were, there would be no point to this :-) > > If you look in the distro scripts you will see 'scan_dev_for_efi' (and > also scan_dev_for_scrips). At least the first needs to be implemented > a bit like the distro boot is at present. So far I have only > implemented scan_dev_for_extlinux (plus pxe) as it is enough to show > the concept. > > Adding EFI it's likely to be about the same amount of code as distro.c > at present, perhaps a little less since we don't have the network > case. It is used by Fedora 34, I believe, so is easy enough for me to > do. But I wanted to get something out as the concept is visible in > this series. If I correctly understand this framework, we will have to write several functions: (Here I will ignore "UEFI boot manager" sequence.) 1)boot/distro_efi.c distro_boot_efi_setup() ; almost the same as distro_boot_setup() distro_boot_efi(); search for /EFI/BOOT/BOOTAA64.EFI (and dtb) 2)boot/bootflow.c bootmethod_find_efi_in_blk() call distro_boot_efi_setup() bootflow_boot() CASE BOOTFLOWT_DISTRO_EFI: ; new bootflow type call distro_boot_efi() 3)drivers/mmc/mmc_bootmethod.c mmc_efi_get_bootflow() call bootmethod_find_efi_in_blk() U_BOOT_DRIVER(mmc_efi_bootmethod) = .name = "mmc_efi_bootmethod", .id
Re: [PATCH 1/2] imx8mm-cl-iot-gate: Do not build fip.bin by default
Hi Frieder,
I think I might have found a reason. The problem might be that the
board_get_usable_ram_top() doesn't subtract the memory used by optee. Optee
on imx8m uses the end of the memory. It is passed by arguments.
VERBOSE: Argument #1 = 0x7e00
VERBOSE: Argument #2 = 0x200
So if I extract 0x8000 by 0x200 then the board boots. That also
explains why Fabio can boot his board because he doesn't use OPTEE.
What I'm doing is I implemented a board_phys_sdram_size(). However because
all of my boards are 2G memory. And I did set PHYS_SDRAM_SIZE to 2G when
upstreaming Compulab's support previously. That means with or without
board_phys_sdram_size() the board just doesn't boot on the master branch
because the gd->ram_size is the same, 2GB.
But you are correct, we do need to have board_phys_sdram_size() implemented
because on Compulab's page we know that they have multiple choices. The
memory could be 1G/2G/4G. So this function is needed to tell how much
memory we have. But the hang problem is just not related to this function.
The problem I think is we need to deal with rom_pointer that contains the
OPTEE address in board_get_usable_ram_top().
Yours,
Paul
On Fri, 20 Aug 2021 at 04:51, Paul Liu wrote:
> Hi Frieder,
>
> I'll confirm it. But I guess you are correct. I'll send a patch soon when
> I implement this right.
>
> Yours,
> Paul
>
>
> On Thu, 19 Aug 2021 at 15:14, Frieder Schrempf <
> frieder.schre...@kontron.de> wrote:
>
>> On 19.08.21 02:27, Fabio Estevam wrote:
>> > [Adding Marek]
>> >
>> > On Wed, Aug 18, 2021 at 6:39 PM Fabio Estevam
>> wrote:
>> >>
>> >> Hi Paul,
>> >>
>> >> On Wed, Aug 18, 2021 at 6:32 PM Paul Liu wrote:
>> >>>
>> >>> Hi Fabio,
>> >>>
>> >>> I got several boards. With all different PN. But all of them are 2GB
>> memory. And the recent master doesn't boot on one of my board. I haven't
>> tried all of the combinations.
>> >>
>> >> With the U-Boot from Compulab, it reports 4GB. With mainline U-Boot it
>> >> reports 2GB, so yes, there is an issue indeed.
>> >>
>> >> However, I don't see a hang.
>> >>
>> >>> After bisect, I found commit e27bddff breaks the boot. It just hang
>> there.
>> >>
>> >> Adding Frieder as the author of the patch.
>> >
>> > Marek objected to this change, which is now:
>> > e27bdd ff4b97 ("imx8m: Restrict usable memory to space below 4G
>> boundary")
>>
>> Yes, Marek objected and it was still pulled in for some reason.
>>
>> >
>> > As this causes a regression on Paul's i.MX8MM IoT Gateway board,
>> > should this be reverted?
>>
>> Maybe, yes. I'll leave that decision to the maintainers.
>>
>> For the failure: The change in e27bddff4b97 assumes that gd->ram_size was
>> set during initialization/detection of the DDR. Could it be that the
>> Compulab board doesn't do this and gd->ram_size is 0 or differs from the
>> actual DDR size? That would probably cause some kind of issue.
>>
>> Paul, maybe you could check if gd->ram_size is set properly. Other boards
>> do this by implementing board_phys_sdram_size() [1], which also makes sure
>> that the memory map is updated with the detected size in dram_init() [2].
>>
>> [1]
>> https://elixir.bootlin.com/u-boot/latest/source/board/gateworks/venice/imx8mm_venice.c#L21
>> [2]
>> https://elixir.bootlin.com/u-boot/latest/source/arch/arm/mach-imx/imx8m/soc.c#L218
>>
>
Re: [PATCH 1/2] imx8mm-cl-iot-gate: Do not build fip.bin by default
Hi Frieder,
I'll confirm it. But I guess you are correct. I'll send a patch soon when I
implement this right.
Yours,
Paul
On Thu, 19 Aug 2021 at 15:14, Frieder Schrempf
wrote:
> On 19.08.21 02:27, Fabio Estevam wrote:
> > [Adding Marek]
> >
> > On Wed, Aug 18, 2021 at 6:39 PM Fabio Estevam
> wrote:
> >>
> >> Hi Paul,
> >>
> >> On Wed, Aug 18, 2021 at 6:32 PM Paul Liu wrote:
> >>>
> >>> Hi Fabio,
> >>>
> >>> I got several boards. With all different PN. But all of them are 2GB
> memory. And the recent master doesn't boot on one of my board. I haven't
> tried all of the combinations.
> >>
> >> With the U-Boot from Compulab, it reports 4GB. With mainline U-Boot it
> >> reports 2GB, so yes, there is an issue indeed.
> >>
> >> However, I don't see a hang.
> >>
> >>> After bisect, I found commit e27bddff breaks the boot. It just hang
> there.
> >>
> >> Adding Frieder as the author of the patch.
> >
> > Marek objected to this change, which is now:
> > e27bdd ff4b97 ("imx8m: Restrict usable memory to space below 4G
> boundary")
>
> Yes, Marek objected and it was still pulled in for some reason.
>
> >
> > As this causes a regression on Paul's i.MX8MM IoT Gateway board,
> > should this be reverted?
>
> Maybe, yes. I'll leave that decision to the maintainers.
>
> For the failure: The change in e27bddff4b97 assumes that gd->ram_size was
> set during initialization/detection of the DDR. Could it be that the
> Compulab board doesn't do this and gd->ram_size is 0 or differs from the
> actual DDR size? That would probably cause some kind of issue.
>
> Paul, maybe you could check if gd->ram_size is set properly. Other boards
> do this by implementing board_phys_sdram_size() [1], which also makes sure
> that the memory map is updated with the detected size in dram_init() [2].
>
> [1]
> https://elixir.bootlin.com/u-boot/latest/source/board/gateworks/venice/imx8mm_venice.c#L21
> [2]
> https://elixir.bootlin.com/u-boot/latest/source/arch/arm/mach-imx/imx8m/soc.c#L218
>
[PATCH] Kconfig: Use spaces not tabs in Kconfig entires
While the Kconfig language seems to accept either form of whitespace, we use a space throughout the project, except in these spots. Signed-off-by: Tom Rini --- arch/arm/mach-exynos/Kconfig | 2 +- board/freescale/mx6memcal/Kconfig | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig index 0b4276c03628..7df0e176179d 100644 --- a/arch/arm/mach-exynos/Kconfig +++ b/arch/arm/mach-exynos/Kconfig @@ -141,7 +141,7 @@ if ARCH_EXYNOS7 choice prompt "EXYNOS7 board select" -config TARGET_ESPRESSO7420 +config TARGET_ESPRESSO7420 bool "ESPRESSO7420 board" select ARM64 select ARMV8_MULTIENTRY diff --git a/board/freescale/mx6memcal/Kconfig b/board/freescale/mx6memcal/Kconfig index 9987cba5dcb7..481403ae855d 100644 --- a/board/freescale/mx6memcal/Kconfig +++ b/board/freescale/mx6memcal/Kconfig @@ -87,12 +87,12 @@ choice help Select the type of DDR (DDR3 or LPDDR2) used on your design -config DDR3 +config DDR3 bool "DDR3" help Select this if your board design uses DDR3. -config LPDDR2 +config LPDDR2 bool "LPDDR2" help Select this if your board design uses LPDDR2. -- 2.17.1
[PATCH v3 2/2] Makefile: Clean the i.MX8MM artifacts
Clean the binaries generated by binman on imx8mm-evk: spl.* mkimage*.mkimage imx-boot.* Reported-by: Frieder Schrempf Signed-off-by: Fabio Estevam --- Changes since v2: - None. Newly introducedin this series. Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 3c8437d21a..7096fdf895 100644 --- a/Makefile +++ b/Makefile @@ -2095,7 +2095,8 @@ CLEAN_FILES += include/bmp_logo.h include/bmp_logo_data.h tools/version.h \ boot* u-boot* MLO* SPL System.map fit-dtb.blob* \ u-boot-ivt.img.log u-boot-dtb.imx.log SPL.log u-boot.imx.log \ lpc32xx-* bl31.c bl31.elf bl31_*.bin image.map tispl.bin* \ - idbloader.img flash.bin flash.log defconfig keep-syms-lto.c + idbloader.img flash.bin flash.log defconfig keep-syms-lto.c \ + spl.* mkimage*.mkimage imx-boot.* # Directories & files removed with 'make mrproper' MRPROPER_DIRS += include/config include/generated spl tpl \ -- 2.25.1
[PATCH v3 1/2] imx8mm-evk: Generate a single bootable flash.bin again
After the conversion to binman in commit 8996e6b7c6a1 ("imx8mm_evk: switch
to use binman to pack images"), it is necessary to flash both flash.bin and
u-boot.itb to get a bootable system. Prior to this commit, only flash.bin
was needed.
Such new requirement breaks existing distro mechanisms to generate the
final binary because the extra u-boot.itb is now required.
Generate a final flash.bin that can be used again as a single
bootable binary to keep the original behavior.
After this change the SPL binary is called spl.bin, which is a more
descriptive name for its purpose, and can still be used standalone
(for example, for secure boot purposes).
Also update imx8mm_evk.rst to remove the u-boot.itb copy step.
Signed-off-by: Fabio Estevam
Reviewed-by: Frieder Schrempf
Reviewed-by: Heiko Schocher
Signed-off-by: Fabio Estevam
---
Changes since v2:
- Change the LOADER to mkimage.spl.mkimage (Frieder)
arch/arm/dts/imx8mm-evk-u-boot.dtsi | 17 -
.../imx8mm_evk/imximage-8mm-lpddr4.cfg | 2 +-
doc/board/freescale/imx8mm_evk.rst | 1 -
3 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
index f200afac9f..75cd59e545 100644
--- a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
+++ b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
@@ -150,7 +150,7 @@
};
- flash {
+ spl {
mkimage {
args = "-n spl/u-boot-spl.cfgout -T imx8mimage -e
0x7e1000";
@@ -217,4 +217,19 @@
};
};
};
+
+ imx-boot {
+ filename = "flash.bin";
+ pad-byte = <0x00>;
+
+ spl: blob-ext@1 {
+ offset = <0x0>;
+ filename = "spl.bin";
+ };
+
+ uboot: blob-ext@2 {
+ offset = <0x57c00>;
+ filename = "u-boot.itb";
+ };
+ };
};
diff --git a/board/freescale/imx8mm_evk/imximage-8mm-lpddr4.cfg
b/board/freescale/imx8mm_evk/imximage-8mm-lpddr4.cfg
index b89092a559..2c15dbc413 100644
--- a/board/freescale/imx8mm_evk/imximage-8mm-lpddr4.cfg
+++ b/board/freescale/imx8mm_evk/imximage-8mm-lpddr4.cfg
@@ -6,4 +6,4 @@
#define __ASSEMBLY__
BOOT_FROM sd
-LOADER mkimage.flash.mkimage 0x7E1000
+LOADER mkimage.spl.mkimage 0x7E1000
diff --git a/doc/board/freescale/imx8mm_evk.rst
b/doc/board/freescale/imx8mm_evk.rst
index 7fd3d72564..b377c4de27 100644
--- a/doc/board/freescale/imx8mm_evk.rst
+++ b/doc/board/freescale/imx8mm_evk.rst
@@ -50,7 +50,6 @@ Burn the flash.bin to MicroSD card offset 33KB:
.. code-block:: bash
$sudo dd if=flash.bin of=/dev/sd[x] bs=1024 seek=33 conv=notrunc
- $sudo dd if=u-boot.itb of=/dev/sdc bs=1024 seek=384 conv=sync
Boot
--
2.25.1
Re: [PATCH] imx8mm-evk: Generate a single bootable flash.bin again
On Wed, 2021-08-18 at 09:19 -0300, Fabio Estevam wrote:
> After the conversion to binman in commit 8996e6b7c6a1 ("imx8mm_evk: switch
> to use binman to pack images"), it is necessary to flash both flash.bin and
> u-boot.itb to get a bootable system. Prior to this commit, only flash.bin
> was needed.
>
> Such new requirement breaks existing distro mechanisms to generate the
> final binary because the extra u-boot.itb is now required.
>
> Generate a final flash.bin that can be used again as a single
> bootable binary to keep the original behavior.
>
> After this change the SPL binary is called spl.bin, which is a more
> descriptive name for its purpose, and can still be used standalone
> (for example, for secure boot purposes).
>
> Signed-off-by: Fabio Estevam
> ---
> arch/arm/dts/imx8mm-evk-u-boot.dtsi | 17 -
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> index f200afac9f..453fe1d259 100644
> --- a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> +++ b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> @@ -150,7 +150,7 @@
> };
>
>
> - flash {
> + spl {
> mkimage {
> args = "-n spl/u-boot-spl.cfgout -T imx8mimage -e
> 0x7e1000";
A second issue I found (besides imximage.cfg needing adjustments as pointed out
by Frieder) is that for me it
only works if I also explicitly set the filename here to spl.bin e.g. as
follows:
filename = "spl.bin";
Anyway, I am just about to send a patch set updating our Verdin iMX8M Mini to
also make use of all this. Stay
tuned...
> @@ -217,4 +217,19 @@
> };
> };
> };
> +
> + imx-boot {
> + filename = "flash.bin";
> + pad-byte = <0x00>;
> +
> + spl: blob-ext@1 {
> + offset = <0x0>;
> + filename = "spl.bin";
> + };
> +
> + uboot: blob-ext@2 {
> + offset = <0x57c00>;
> + filename = "u-boot.itb";
> + };
> + };
> };
[PATCH] astro_mcf5373l: Rework ASTRO_ID logic
Rather than using CONFIG namespace for logic internal to include/configs/astro_mcf5373l.h to select ASTRO_ID (and populate the default environment), strip CONFIG from the various options used and set. Signed-off-by: Tom Rini --- include/configs/astro_mcf5373l.h | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/include/configs/astro_mcf5373l.h b/include/configs/astro_mcf5373l.h index 12488b66c30b..65c7d03efc1c 100644 --- a/include/configs/astro_mcf5373l.h +++ b/include/configs/astro_mcf5373l.h @@ -22,17 +22,17 @@ * set the card type to actually compile for; either of * the possibilities listed below has to be used! */ -#define CONFIG_ASTRO_V532 1 +#define ASTRO_V532 1 -#if CONFIG_ASTRO_V532 +#if ASTRO_V532 #define ASTRO_ID 0xF8 -#elif CONFIG_ASTRO_V512 +#elif ASTRO_V512 #define ASTRO_ID 0xFA -#elif CONFIG_ASTRO_TWIN7S2 +#elif ASTRO_TWIN7S2 #define ASTRO_ID 0xF9 -#elif CONFIG_ASTRO_V912 +#elif ASTRO_V912 #define ASTRO_ID 0xFC -#elif CONFIG_ASTRO_COFDMDUOS2 +#elif ASTRO_COFDMDUOS2 #define ASTRO_ID 0xFB #else #error No card type defined! @@ -144,7 +144,7 @@ #ifdef CONFIG_MONITOR_IS_IN_RAM #define CONFIG_BOOTCOMMAND "" /* no autoboot in this case */ #else -#if CONFIG_ASTRO_V532 +#if ASTRO_V532 #define CONFIG_BOOTCOMMAND "protect off 0x8 0x1ff;run env_check;"\ "run xilinxload& alteraload& 0x8;"\ "update;reset" -- 2.17.1
[PATCH] mpc83xx: Update comment
Update the comment here to refer to PCI_CONFIG_ADDRESS rather than CONFIG_ADDRESS. Signed-off-by: Tom Rini --- include/mpc83xx.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/mpc83xx.h b/include/mpc83xx.h index 71cffa1b0fc8..0275b3184ea3 100644 --- a/include/mpc83xx.h +++ b/include/mpc83xx.h @@ -1372,7 +1372,7 @@ #endif /* !CONFIG_MPC83XX_SDRAM */ /* - * CONFIG_ADDRESS - PCI Config Address Register + * PCI_CONFIG_ADDRESS - PCI Config Address Register */ #define PCI_CONFIG_ADDRESS_EN 0x8000 #define PCI_CONFIG_ADDRESS_BN_SHIFT16 -- 2.17.1
Re: [PATCH] imx8mm-evk: Generate a single bootable flash.bin again
Hi Fabio and Frieder On Thu, 2021-08-19 at 15:58 -0300, Fabio Estevam wrote: > Hi Frieder, > > On Thu, Aug 19, 2021 at 3:52 PM Frieder Schrempf > wrote: > > > I tried to adapt this for my own board, but I needed to change the > > following in the imximage.cfg for the > > build to pass. Did you test this? > > > > -LOADER mkimage.flash.mkimage 0x7E1000 > > +LOADER mkimage.spl.mkimage 0x7E1000 Yes, I can copy that. And while investigating I noticed that doing a clean or even a mrproper does not seem to clean any of those mkimage.flash.mkimage or mkimage-out.flash.mkimage. > Interesting. I do not see the build error here. So likely, it was just picking up some old stuff for you Fabio. Not sure whether or not we would also want to fix this cleaning issue as it can be quite confusing. > My patch is against commit 78e786decb6c8 in mainline U-Boot. > > Are you able to reproduce the build error on imx8mm-evk too? Yes, once one does hand clean them old artifacts. > Thanks Cheers Marcel
Re: [PATCH] imx8mm-evk: Generate a single bootable flash.bin again
On 19.08.21 20:58, Fabio Estevam wrote: > Hi Frieder, > > On Thu, Aug 19, 2021 at 3:52 PM Frieder Schrempf > wrote: > >> I tried to adapt this for my own board, but I needed to change the following >> in the imximage.cfg for the build to pass. Did you test this? >> >> -LOADER mkimage.flash.mkimage 0x7E1000 >> +LOADER mkimage.spl.mkimage 0x7E1000 > > Interesting. I do not see the build error here. > > My patch is against commit 78e786decb6c8 in mainline U-Boot. > > Are you able to reproduce the build error on imx8mm-evk too? Yes, I'm building against the very same commit from master and I get the following at the end of the build, also for imx8mm-evk: MKIMAGE u-boot.img MKIMAGE u-boot-dtb.img CFGSspl/u-boot-spl.cfgout BINMAN all binman: Error 1 running 'mkimage -d ./mkimage.spl.mkimage -n spl/u-boot-spl.cfgout -T imx8mimage -e 0x7e1000 ./mkimage-out.spl.mkimage': mkimage.flash.mkimage: Can't open: No such file or directory
[PATCH] video: Remove ati_radeon_fb
This driver is currently unused. Remove.
Signed-off-by: Tom Rini
---
drivers/video/Makefile|1 -
drivers/video/ati_ids.h | 211
drivers/video/ati_radeon_fb.c | 761 -
drivers/video/ati_radeon_fb.h | 282 -
include/radeon.h | 1988 -
5 files changed, 3243 deletions(-)
delete mode 100644 drivers/video/ati_ids.h
delete mode 100644 drivers/video/ati_radeon_fb.c
delete mode 100644 drivers/video/ati_radeon_fb.h
delete mode 100644 include/radeon.h
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 7ae0ab2b35c3..f6d07b343f0a 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -25,7 +25,6 @@ obj-${CONFIG_VIDEO_STM32} += stm32/
obj-${CONFIG_VIDEO_TEGRA124} += tegra124/
obj-y += ti/
-obj-$(CONFIG_ATI_RADEON_FB) += ati_radeon_fb.o videomodes.o
obj-$(CONFIG_ATMEL_HLCD) += atmel_hlcdfb.o
obj-$(CONFIG_ATMEL_LCD) += atmel_lcdfb.o
obj-$(CONFIG_CFB_CONSOLE) += cfb_console.o
diff --git a/drivers/video/ati_ids.h b/drivers/video/ati_ids.h
deleted file mode 100644
index 3e72a7dd4c0d..
--- a/drivers/video/ati_ids.h
+++ /dev/null
@@ -1,211 +0,0 @@
-/*
- * ATI PCI IDs from XFree86, kept here to make sync'ing with
- * XFree much simpler. Currently, this list is only used by
- * radeonfb
- */
-
-#define PCI_CHIP_RV380_3150 0x3150
-#define PCI_CHIP_RV380_3151 0x3151
-#define PCI_CHIP_RV380_3152 0x3152
-#define PCI_CHIP_RV380_3153 0x3153
-#define PCI_CHIP_RV380_3154 0x3154
-#define PCI_CHIP_RV380_3156 0x3156
-#define PCI_CHIP_RV380_3E50 0x3E50
-#define PCI_CHIP_RV380_3E51 0x3E51
-#define PCI_CHIP_RV380_3E52 0x3E52
-#define PCI_CHIP_RV380_3E53 0x3E53
-#define PCI_CHIP_RV380_3E54 0x3E54
-#define PCI_CHIP_RV380_3E56 0x3E56
-#define PCI_CHIP_RS100_41360x4136
-#define PCI_CHIP_RS200_41370x4137
-#define PCI_CHIP_R300_AD 0x4144
-#define PCI_CHIP_R300_AE 0x4145
-#define PCI_CHIP_R300_AF 0x4146
-#define PCI_CHIP_R300_AG 0x4147
-#define PCI_CHIP_R350_AH0x4148
-#define PCI_CHIP_R350_AI0x4149
-#define PCI_CHIP_R350_AJ0x414A
-#define PCI_CHIP_R350_AK0x414B
-#define PCI_CHIP_RV350_AP 0x4150
-#define PCI_CHIP_RV350_AQ 0x4151
-#define PCI_CHIP_RV360_AR 0x4152
-#define PCI_CHIP_RV350_AS 0x4153
-#define PCI_CHIP_RV350_AT 0x4154
-#define PCI_CHIP_RV350_AV 0x4156
-#define PCI_CHIP_MACH320x4158
-#define PCI_CHIP_RS250_42370x4237
-#define PCI_CHIP_R200_BB 0x4242
-#define PCI_CHIP_R200_BC 0x4243
-#define PCI_CHIP_RS100_43360x4336
-#define PCI_CHIP_RS200_43370x4337
-#define PCI_CHIP_MACH64CT 0x4354
-#define PCI_CHIP_MACH64CX 0x4358
-#define PCI_CHIP_RS250_44370x4437
-#define PCI_CHIP_MACH64ET 0x4554
-#define PCI_CHIP_MACH64GB 0x4742
-#define PCI_CHIP_MACH64GD 0x4744
-#define PCI_CHIP_MACH64GI 0x4749
-#define PCI_CHIP_MACH64GL 0x474C
-#define PCI_CHIP_MACH64GM 0x474D
-#define PCI_CHIP_MACH64GN 0x474E
-#define PCI_CHIP_MACH64GO 0x474F
-#define PCI_CHIP_MACH64GP 0x4750
-#define PCI_CHIP_MACH64GQ 0x4751
-#define PCI_CHIP_MACH64GR 0x4752
-#define PCI_CHIP_MACH64GS 0x4753
-#define PCI_CHIP_MACH64GT 0x4754
-#define PCI_CHIP_MACH64GU 0x4755
-#define PCI_CHIP_MACH64GV 0x4756
-#define PCI_CHIP_MACH64GW 0x4757
-#define PCI_CHIP_MACH64GX 0x4758
-#define PCI_CHIP_MACH64GY 0x4759
-#define PCI_CHIP_MACH64GZ 0x475A
-#define PCI_CHIP_RV250_Id 0x4964
-#define PCI_CHIP_RV250_Ie 0x4965
-#define PCI_CHIP_RV250_If 0x4966
-#define PCI_CHIP_RV250_Ig 0x4967
-#define PCI_CHIP_R420_JH0x4A48
-#define PCI_CHIP_R420_JI0x4A49
-#define PCI_CHIP_R420_JJ0x4A4A
-#define PCI_CHIP_R420_JK0x4A4B
-#define PCI_CHIP_R420_JL0x4A4C
-#define PCI_CHIP_R420_JM0x4A4D
-#define PCI_CHIP_R420_JN0x4A4E
-#define PCI_CHIP_R420_JP0x4A50
-#define PCI_CHIP_MACH64LB 0x4C42
-#define PCI_CHIP_MACH64LD 0x4C44
-#define PCI_CHIP_RAGE128LE 0x4C45
-#define PCI_CHIP_RAGE128LF 0x4C46
-#define PCI_CHIP_MACH64LG 0x4C47
-#define PCI_CHIP_MACH64LI 0x4C49
-#define PCI_CHIP_MACH64LM 0x4C4D
-#define PCI_CHIP_MACH64LN 0x4C4E
-#define PCI_CHIP_MACH64LP 0x4C50
-#define
status="ok" treated as disabled by DM
Hi, If the device tree node has status="ok" then the u-boot DM will treat the device as disabled. There are still quite many devices using "ok" instead of "okay" or no status and those devices will not be bound. What is the right fix? 1) make u-boot DM to treat satus="ok" as enabled. 2) fix device tree by changing "ok" to "okay". cheers, -roger
[PATCH 10/16] Convert CONFIG_SYS_I2C_MXC et al to Kconfig
This converts the following to Kconfig: CONFIG_SYS_I2C_MXC CONFIG_SYS_I2C_MXC_I2C1 CONFIG_SYS_I2C_MXC_I2C2 CONFIG_SYS_I2C_MXC_I2C3 CONFIG_SYS_I2C_MXC_I2C4 Signed-off-by: Tom Rini --- README| 17 arch/arm/Kconfig | 1 + arch/arm/cpu/armv7/ls102xa/Kconfig| 1 + configs/apalis_imx6_defconfig | 5 configs/cl-som-imx7_defconfig | 2 ++ configs/cm_fx6_defconfig | 5 configs/colibri_imx6_defconfig| 5 configs/colibri_imx7_defconfig| 1 + configs/colibri_imx7_emmc_defconfig | 1 + configs/flea3_defconfig | 5 configs/gwventana_emmc_defconfig | 4 +++ configs/gwventana_gw5904_defconfig| 4 +++ configs/gwventana_nand_defconfig | 4 +++ configs/imx8mm-cl-iot-gate_defconfig | 4 --- configs/imx8mm-icore-mx8mm-ctouch2_defconfig | 1 - configs/imx8mm-icore-mx8mm-edimm2.2_defconfig | 1 - configs/imx8mm_beacon_defconfig | 4 --- configs/imx8mm_evk_defconfig | 4 --- configs/imx8mm_venice_defconfig | 4 --- configs/imx8mn_beacon_2g_defconfig| 4 --- configs/imx8mn_beacon_defconfig | 4 --- configs/imx8mn_ddr4_evk_defconfig | 4 --- configs/imx8mn_evk_defconfig | 4 --- configs/imx8mp_evk_defconfig | 1 - configs/imx8mq_cm_defconfig | 1 - configs/imx8mq_evk_defconfig | 4 ++- configs/imx8mq_phanbell_defconfig | 4 ++- configs/kp_imx53_defconfig| 1 + configs/kp_imx6q_tpc_defconfig| 2 -- configs/ls1021aiot_qspi_defconfig | 3 +++ configs/ls1021aiot_sdcard_defconfig | 3 +++ configs/ls1021aqds_ddr4_nor_defconfig | 3 +++ configs/ls1021aqds_ddr4_nor_lpuart_defconfig | 3 +++ configs/ls1021aqds_nand_defconfig | 3 +++ configs/ls1021aqds_nor_SECURE_BOOT_defconfig | 3 +++ configs/ls1021aqds_nor_defconfig | 3 +++ configs/ls1021aqds_nor_lpuart_defconfig | 3 +++ configs/ls1021aqds_qspi_defconfig | 3 +++ configs/ls1021aqds_sdcard_ifc_defconfig | 3 +++ configs/ls1021aqds_sdcard_qspi_defconfig | 3 +++ configs/ls1021atsn_qspi_defconfig | 3 +++ configs/ls1021atsn_sdcard_defconfig | 3 +++ configs/ls1021atwr_nor_SECURE_BOOT_defconfig | 3 +++ configs/ls1021atwr_nor_defconfig | 3 +++ configs/ls1021atwr_nor_lpuart_defconfig | 3 +++ configs/ls1021atwr_qspi_defconfig | 3 +++ ...s1021atwr_sdcard_ifc_SECURE_BOOT_defconfig | 3 +++ configs/ls1021atwr_sdcard_ifc_defconfig | 3 +++ configs/ls1021atwr_sdcard_qspi_defconfig | 3 +++ configs/ls1043aqds_defconfig | 4 +++ configs/ls1043aqds_lpuart_defconfig | 4 +++ configs/ls1043aqds_nand_defconfig | 4 +++ configs/ls1043aqds_nor_ddr3_defconfig | 4 +++ configs/ls1043aqds_qspi_defconfig | 4 +++ configs/ls1043aqds_sdcard_ifc_defconfig | 4 +++ configs/ls1043aqds_sdcard_qspi_defconfig | 4 +++ configs/ls1043aqds_tfa_SECURE_BOOT_defconfig | 4 +++ configs/ls1043aqds_tfa_defconfig | 4 +++ configs/ls1043ardb_SECURE_BOOT_defconfig | 4 +++ configs/ls1043ardb_defconfig | 4 +++ configs/ls1043ardb_nand_defconfig | 4 +++ configs/ls1043ardb_sdcard_defconfig | 4 +++ configs/ls1043ardb_tfa_SECURE_BOOT_defconfig | 4 +++ configs/ls1043ardb_tfa_defconfig | 4 +++ configs/ls1046aqds_SECURE_BOOT_defconfig | 4 +++ configs/ls1046aqds_defconfig | 4 +++ configs/ls1046aqds_lpuart_defconfig | 4 +++ configs/ls1046aqds_nand_defconfig | 4 +++ configs/ls1046aqds_qspi_defconfig | 4 +++ configs/ls1046aqds_sdcard_ifc_defconfig | 4 +++ configs/ls1046aqds_sdcard_qspi_defconfig | 4 +++ configs/ls1046aqds_tfa_SECURE_BOOT_defconfig | 4 +++ configs/ls1046aqds_tfa_defconfig | 4 +++ configs/ls1046ardb_emmc_defconfig | 4 +++ configs/ls1046ardb_qspi_SECURE_BOOT_defconfig | 4 +++ configs/ls1046ardb_qspi_defconfig | 4 +++ configs/ls1046ardb_qspi_spl_defconfig | 4 +++ .../ls1046ardb_sdcard_SECURE_BOOT_defconfig | 4 +++ configs/ls1046ardb_sdcard_defconfig | 4 +++ configs/ls1046ardb_tfa_SECURE_BOOT_defconfig | 4 +++ configs/ls1046ardb_tfa_defconfig | 4 +++ configs/ls1088ardb_qspi_SECURE_BOOT_defconfig | 2 ++ configs/ls1088ardb_qspi_defconfig | 2 ++ ...1088ardb_sdcard_qspi_SECURE_BOOT_defconfig | 2 ++ configs/ls1088ardb_sdcard_qspi_defconfig | 2 ++ configs/m53menlo_defconfig|
[PATCH] spi: altera_spi: Do not abuse CONFIG namespace
The value CONFIG_ALTERA_SPI_IDLE_VAL is never re-defined by a board.
Rename this to ALTERA_SPI_IDLE_VAL.
Signed-off-by: Tom Rini
---
drivers/spi/altera_spi.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/altera_spi.c b/drivers/spi/altera_spi.c
index fadc9f39657d..989679e881b5 100644
--- a/drivers/spi/altera_spi.c
+++ b/drivers/spi/altera_spi.c
@@ -19,9 +19,7 @@
#define ALTERA_SPI_STATUS_RRDY_MSK BIT(7)
#define ALTERA_SPI_CONTROL_SSO_MSK BIT(10)
-#ifndef CONFIG_ALTERA_SPI_IDLE_VAL
-#define CONFIG_ALTERA_SPI_IDLE_VAL 0xff
-#endif
+#define ALTERA_SPI_IDLE_VAL0xff
struct altera_spi_regs {
u32 rxdata;
@@ -119,7 +117,7 @@ static int altera_spi_xfer(struct udevice *dev, unsigned
int bitlen,
if (txp)
data = *txp++;
else
- data = CONFIG_ALTERA_SPI_IDLE_VAL;
+ data = ALTERA_SPI_IDLE_VAL;
debug("%s: tx:%x ", __func__, data);
writel(data, >txdata);
--
2.17.1
[PATCH] global: Remove dead code that starts with CONFIG_[0-9A]
This removes a number of spots of dead code based on symbols that start
with CONFIG_[0-9] or CONFIG_A.
Signed-off-by: Tom Rini
---
arch/powerpc/cpu/mpc83xx/pcie.c | 144 --
arch/powerpc/include/asm/processor.h | 4 -
board/freescale/mpc8349emds/mpc8349emds.c | 2 -
drivers/net/smc9.h| 13 +-
drivers/video/pxa_lcd.c | 68 +-
include/configs/MPC8349EMDS.h | 3 -
include/configs/MPC8349EMDS_SDRAM.h | 3 -
include/configs/aspenite.h| 1 -
include/configs/km/km-mpc8309.h | 1 -
include/configs/km/km-mpc832x.h | 1 -
include/configs/kmcoge5ne.h | 1 -
include/configs/sniper.h | 10 --
include/configs/total_compute.h | 2 -
include/configs/vexpress_common.h | 3 -
14 files changed, 3 insertions(+), 253 deletions(-)
diff --git a/arch/powerpc/cpu/mpc83xx/pcie.c b/arch/powerpc/cpu/mpc83xx/pcie.c
index 84797c871c95..c386e4ed3fde 100644
--- a/arch/powerpc/cpu/mpc83xx/pcie.c
+++ b/arch/powerpc/cpu/mpc83xx/pcie.c
@@ -34,148 +34,6 @@ static struct {
#endif
};
-#ifdef CONFIG_83XX_GENERIC_PCIE_REGISTER_HOSES
-
-/* private structure for mpc83xx pcie hose */
-static struct mpc83xx_pcie_priv {
- u8 index;
-} pcie_priv[PCIE_MAX_BUSES] = {
- {
- /* pcie controller 1 */
- .index = 0,
- },
- {
- /* pcie controller 2 */
- .index = 1,
- },
-};
-
-static int mpc83xx_pcie_remap_cfg(struct pci_controller *hose, pci_dev_t dev)
-{
- int bus = PCI_BUS(dev) - hose->first_busno;
- immap_t *immr = (immap_t *)CONFIG_SYS_IMMR;
- struct mpc83xx_pcie_priv *pcie_priv = hose->priv_data;
- pex83xx_t *pex = >pciexp[pcie_priv->index];
- struct pex_outbound_window *out_win = >bridge.pex_outbound_win[0];
- u8 devfn = PCI_DEV(dev) << 3 | PCI_FUNC(dev);
- u32 dev_base = bus << 24 | devfn << 16;
-
- if (hose->indirect_type == INDIRECT_TYPE_NO_PCIE_LINK)
- return -1;
- /*
-* Workaround for the HW bug: for Type 0 configure transactions the
-* PCI-E controller does not check the device number bits and just
-* assumes that the device number bits are 0.
-*/
- if (devfn & 0xf8)
- return -1;
-
- out_le32(_win->tarl, dev_base);
- return 0;
-}
-
-#define cfg_read(val, addr, type, op) \
- do { *val = op((type)(addr)); } while (0)
-#define cfg_write(val, addr, type, op) \
- do { op((type *)(addr), (val)); } while (0)
-
-#define cfg_read_err(val) do { *val = -1; } while (0)
-#define cfg_write_err(val) do { } while (0)
-
-#define PCIE_OP(rw, size, type, op)\
-static int pcie_##rw##_config_##size(struct pci_controller *hose, \
-pci_dev_t dev, int offset, \
-type val) \
-{ \
- int ret;\
- \
- ret = mpc83xx_pcie_remap_cfg(hose, dev);\
- if (ret) { \
- cfg_##rw##_err(val);\
- return ret; \
- } \
- cfg_##rw(val, (void *)hose->cfg_addr + offset, type, op); \
- return 0; \
-}
-
-PCIE_OP(read, byte, u8 *, in_8)
-PCIE_OP(read, word, u16 *, in_le16)
-PCIE_OP(read, dword, u32 *, in_le32)
-PCIE_OP(write, byte, u8, out_8)
-PCIE_OP(write, word, u16, out_le16)
-PCIE_OP(write, dword, u32, out_le32)
-
-static void mpc83xx_pcie_register_hose(int bus, struct pci_region *reg,
- u8 link)
-{
- extern void disable_addr_trans(void); /* start.S */
- static struct pci_controller pcie_hose[PCIE_MAX_BUSES];
- struct pci_controller *hose = _hose[bus];
- int i;
-
- /*
-* There are no spare BATs to remap all PCI-E windows for U-Boot, so
-* disable translations. In general, this is not great solution, and
-* that's why we don't register PCI-E hoses by default.
-*/
- disable_addr_trans();
-
- for (i = 0; i < 2; i++, reg++) {
- if (reg->size == 0)
- break;
-
- hose->regions[i] = *reg;
- hose->region_count++;
- }
-
- i = hose->region_count++;
- hose->regions[i].bus_start = 0;
- hose->regions[i].phys_start = 0;
- hose->regions[i].size = gd->ram_size;
- hose->regions[i].flags =
Re: [PATCH] imx8mm-evk: Generate a single bootable flash.bin again
Hi Frieder, On Thu, Aug 19, 2021 at 3:52 PM Frieder Schrempf wrote: > I tried to adapt this for my own board, but I needed to change the following > in the imximage.cfg for the build to pass. Did you test this? > > -LOADER mkimage.flash.mkimage 0x7E1000 > +LOADER mkimage.spl.mkimage 0x7E1000 Interesting. I do not see the build error here. My patch is against commit 78e786decb6c8 in mainline U-Boot. Are you able to reproduce the build error on imx8mm-evk too? Thanks
[PATCH] i8042: Do not abuse CONFIG namespace
This driver uses the CONFIG namespace to set the chips internal CONFIG namespace related bits. However, CONFIG is reserved for the top-level Kconfig based configuration system. Use CFG as the namespace here instead to avoid pollution. Signed-off-by: Tom Rini --- drivers/input/i8042.c | 4 ++-- include/i8042.h | 12 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/input/i8042.c b/drivers/input/i8042.c index 565d99e7e57f..d3743dc37f72 100644 --- a/drivers/input/i8042.c +++ b/drivers/input/i8042.c @@ -150,8 +150,8 @@ static int kbd_reset(int quirk) else if ((quirk & QUIRK_DUP_POR) && config == KBD_POR) config = kbd_cmd_read(CMD_RD_CONFIG); - config |= CONFIG_AT_TRANS; - config &= ~(CONFIG_KIRQ_EN | CONFIG_MIRQ_EN); + config |= CFG_AT_TRANS; + config &= ~(CFG_KIRQ_EN | CFG_MIRQ_EN); if (kbd_cmd_write(CMD_WR_CONFIG, config)) goto err; diff --git a/include/i8042.h b/include/i8042.h index 8d69fa13bc2a..687632058c95 100644 --- a/include/i8042.h +++ b/include/i8042.h @@ -20,12 +20,12 @@ #define STATUS_IBF (1 << 1) /* Configuration byte bit defines */ -#define CONFIG_KIRQ_EN (1 << 0) -#define CONFIG_MIRQ_EN (1 << 1) -#define CONFIG_SET_BIST(1 << 2) -#define CONFIG_KCLK_DIS(1 << 4) -#define CONFIG_MCLK_DIS(1 << 5) -#define CONFIG_AT_TRANS(1 << 6) +#define CFG_KIRQ_EN(1 << 0) +#define CFG_MIRQ_EN(1 << 1) +#define CFG_SET_BIST (1 << 2) +#define CFG_KCLK_DIS (1 << 4) +#define CFG_MCLK_DIS (1 << 5) +#define CFG_AT_TRANS (1 << 6) /* i8042 commands */ #define CMD_RD_CONFIG 0x20/* read configuration byte */ -- 2.17.1
Re: [PATCH] imx8mm-evk: Generate a single bootable flash.bin again
Hi Fabio,
On 18.08.21 16:07, Frieder Schrempf wrote:
> On 18.08.21 14:19, Fabio Estevam wrote:
>> After the conversion to binman in commit 8996e6b7c6a1 ("imx8mm_evk: switch
>> to use binman to pack images"), it is necessary to flash both flash.bin and
>> u-boot.itb to get a bootable system. Prior to this commit, only flash.bin
>> was needed.
>>
>> Such new requirement breaks existing distro mechanisms to generate the
>> final binary because the extra u-boot.itb is now required.
>>
>> Generate a final flash.bin that can be used again as a single
>> bootable binary to keep the original behavior.
>>
>> After this change the SPL binary is called spl.bin, which is a more
>> descriptive name for its purpose, and can still be used standalone
>> (for example, for secure boot purposes).
>>
>> Signed-off-by: Fabio Estevam
>
> Reviewed-by: Frieder Schrempf
I tried to adapt this for my own board, but I needed to change the following in
the imximage.cfg for the build to pass. Did you test this?
-LOADER mkimage.flash.mkimage 0x7E1000
+LOADER mkimage.spl.mkimage 0x7E1000
Best regards
Frieder
>> ---
>> arch/arm/dts/imx8mm-evk-u-boot.dtsi | 17 -
>> 1 file changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
>> b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
>> index f200afac9f..453fe1d259 100644
>> --- a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
>> +++ b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
>> @@ -150,7 +150,7 @@
>> };
>>
>>
>> - flash {
>> +spl {
>> mkimage {
>> args = "-n spl/u-boot-spl.cfgout -T imx8mimage -e
>> 0x7e1000";
>>
>> @@ -217,4 +217,19 @@
>> };
>> };
>> };
>> +
>> +imx-boot {
>> +filename = "flash.bin";
>> +pad-byte = <0x00>;
>> +
>> +spl: blob-ext@1 {
>> +offset = <0x0>;
>> +filename = "spl.bin";
>> +};
>> +
>> +uboot: blob-ext@2 {
>> +offset = <0x57c00>;
>> +filename = "u-boot.itb";
>> +};
>> +};
>> };
>>
[PATCH] ppc: Rework some hard-coded BOOTCOMMANDS
There are an assortment of hard-coded CONFIG_BOOTCOMMAND options in some board headers. Rework these so that they do not add to the CONFIG namespace. Signed-off-by: Tom Rini --- include/configs/MPC8349EMDS.h | 4 ++-- include/configs/MPC8349EMDS_SDRAM.h | 4 ++-- include/configs/MPC837XERDB.h | 4 ++-- include/configs/MPC8540ADS.h| 6 +++--- include/configs/MPC8548CDS.h| 6 +++--- include/configs/MPC8560ADS.h| 6 +++--- include/configs/P1010RDB.h | 4 ++-- include/configs/P2041RDB.h | 8 include/configs/T102xRDB.h | 6 +++--- include/configs/T104xRDB.h | 10 +- include/configs/T208xQDS.h | 16 include/configs/T208xRDB.h | 16 include/configs/T4240RDB.h | 14 +++--- include/configs/controlcenterdc.h | 4 ++-- include/configs/corenet_ds.h| 8 include/configs/gazerbeam.h | 6 +++--- include/configs/ge_bx50v3.h | 8 include/configs/ids8313.h | 2 +- include/configs/mx53ppd.h | 4 ++-- include/configs/p1_p2_rdb_pc.h | 8 include/configs/smartweb.h | 2 +- include/configs/uniphier.h | 2 +- include/configs/x86-common.h| 4 ++-- 23 files changed, 76 insertions(+), 76 deletions(-) diff --git a/include/configs/MPC8349EMDS.h b/include/configs/MPC8349EMDS.h index d6ae419456ae..2612904ee2bc 100644 --- a/include/configs/MPC8349EMDS.h +++ b/include/configs/MPC8349EMDS.h @@ -350,7 +350,7 @@ "fdtfile=mpc834x_mds.dtb\0" \ "" -#define CONFIG_NFSBOOTCOMMAND \ +#define NFSBOOTCOMMAND \ "setenv bootargs root=/dev/nfs rw " \ "nfsroot=$serverip:$rootpath " \ "ip=$ipaddr:$serverip:$gatewayip:$netmask:$hostname:" \ @@ -360,7 +360,7 @@ "tftp $fdtaddr $fdtfile;" \ "bootm $loadaddr - $fdtaddr" -#define CONFIG_RAMBOOTCOMMAND \ +#define RAMBOOTCOMMAND \ "setenv bootargs root=/dev/ram rw " \ "console=$consoledev,$baudrate $othbootargs;" \ "tftp $ramdiskaddr $ramdiskfile;" \ diff --git a/include/configs/MPC8349EMDS_SDRAM.h b/include/configs/MPC8349EMDS_SDRAM.h index 8ebca99d98b8..b599446d3ee4 100644 --- a/include/configs/MPC8349EMDS_SDRAM.h +++ b/include/configs/MPC8349EMDS_SDRAM.h @@ -407,7 +407,7 @@ "fdtfile=mpc834x_mds.dtb\0" \ "" -#define CONFIG_NFSBOOTCOMMAND \ +#define NFSBOOTCOMMAND \ "setenv bootargs root=/dev/nfs rw " \ "nfsroot=$serverip:$rootpath " \ "ip=$ipaddr:$serverip:$gatewayip:$netmask:$hostname:" \ @@ -417,7 +417,7 @@ "tftp $fdtaddr $fdtfile;" \ "bootm $loadaddr - $fdtaddr" -#define CONFIG_RAMBOOTCOMMAND \ +#define RAMBOOTCOMMAND \ "setenv bootargs root=/dev/ram rw " \ "console=$consoledev,$baudrate $othbootargs;" \ "tftp $ramdiskaddr $ramdiskfile;" \ diff --git a/include/configs/MPC837XERDB.h b/include/configs/MPC837XERDB.h index 0a136b4f92f5..72d02f4ee449 100644 --- a/include/configs/MPC837XERDB.h +++ b/include/configs/MPC837XERDB.h @@ -392,7 +392,7 @@ "$netdev:off " \ "root=$rootdev rw console=$console,$baudrate $othbootargs\0" -#define CONFIG_NFSBOOTCOMMAND \ +#define NFSBOOTCOMMAND \ "setenv rootdev /dev/nfs;" \ "run setbootargs;" \ "run setipargs;"\ @@ -400,7 +400,7 @@ "tftp $fdtaddr $fdtfile;" \ "bootm $loadaddr - $fdtaddr" -#define CONFIG_RAMBOOTCOMMAND \ +#define RAMBOOTCOMMAND \ "setenv rootdev /dev/ram;" \ "run setbootargs;" \ "tftp $ramdiskaddr $ramdiskfile;" \ diff --git a/include/configs/MPC8540ADS.h b/include/configs/MPC8540ADS.h index
[PATCH] arm: Migrate GICV2 / GICV3 to Kconfig
Migrate CONFIG_GICV2 and CONFIG_GICV3 to Kconfig. We still have the GIC related registers that need to be handled more cleanly but start by moving this symbol to Kconfig. Signed-off-by: Tom Rini --- arch/arm/Kconfig | 10 ++ arch/arm/cpu/armv8/fsl-layerscape/Kconfig | 8 arch/arm/include/asm/arch-fsl-layerscape/config.h | 2 -- arch/arm/mach-rmobile/Kconfig.64 | 12 arch/arm/mach-tegra/Kconfig | 2 ++ arch/arm/mach-versal/Kconfig | 3 --- include/configs/falcon.h | 9 - include/configs/ls1012a_common.h | 2 -- include/configs/ls1043a_common.h | 1 - include/configs/ls1046a_common.h | 1 - include/configs/ls2080a_common.h | 1 - include/configs/lx2160a_common.h | 1 - include/configs/presidio_asic.h | 1 - include/configs/rcar-gen3-common.h| 1 - include/configs/socfpga_soc64_common.h| 5 - include/configs/tegra186-common.h | 3 --- include/configs/tegra210-common.h | 3 --- include/configs/xilinx_zynqmp.h | 1 - 18 files changed, 36 insertions(+), 30 deletions(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index d692139199c4..55e6c73eef50 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -63,6 +63,12 @@ config LNX_KRNL_IMG_TEXT_OFFSET_BASE endif endif +config GICV2 + bool + +config GICV3 + bool + config GIC_V3_ITS bool "ARM GICV3 ITS" select REGMAP @@ -952,6 +958,7 @@ config ARCH_SOCFPGA select CPU_V7A if TARGET_SOCFPGA_GEN5 || TARGET_SOCFPGA_ARRIA10 select DM select DM_SERIAL + select GICV2 select GPIO_EXTRA_HEADER select ENABLE_ARM_SOC_BOOT0_HOOK if TARGET_SOCFPGA_GEN5 || TARGET_SOCFPGA_ARRIA10 select OF_CONTROL @@ -1062,6 +1069,7 @@ config ARCH_VERSAL select DM_ETH if NET select DM_MMC if MMC select DM_SERIAL + select GICV3 select GPIO_EXTRA_HEADER select OF_CONTROL imply BOARD_LATE_INIT @@ -1130,6 +1138,7 @@ config ARCH_ZYNQMP select DM_SPI if SPI select DM_SPI_FLASH if DM_SPI select FIRMWARE + select GICV2 select GPIO_EXTRA_HEADER select OF_CONTROL select SPL_BOARD_INIT if SPL @@ -1878,6 +1887,7 @@ config TARGET_DURIAN config TARGET_PRESIDIO_ASIC bool "Support Cortina Presidio ASIC Platform" select ARM64 + select GICV2 config TARGET_XENGUEST_ARM64 bool "Xen guest ARM64" diff --git a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig index 9c58f69dbd0d..5c7ec9ccab57 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig +++ b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig @@ -4,6 +4,7 @@ config ARCH_LS1012A select ARM_ERRATA_855873 if !TFABOOT select FSL_LAYERSCAPE select FSL_LSCH2 + select GICV2 select SYS_FSL_SRDS_1 select SYS_HAS_SERDES select SYS_FSL_DDR_BE @@ -25,6 +26,7 @@ config ARCH_LS1028A select ARMV8_SET_SMPEN select FSL_LAYERSCAPE select FSL_LSCH3 + select GICV3 select NXP_LSCH3_2 select SYS_FSL_HAS_CCI400 select SYS_FSL_SRDS_1 @@ -58,6 +60,7 @@ config ARCH_LS1043A select ARM_ERRATA_855873 if !TFABOOT select FSL_LAYERSCAPE select FSL_LSCH2 + select GICV2 select SYS_FSL_SRDS_1 select SYS_HAS_SERDES select SYS_FSL_DDR @@ -89,6 +92,7 @@ config ARCH_LS1046A select ARMV8_SET_SMPEN select FSL_LAYERSCAPE select FSL_LSCH2 + select GICV2 select SYS_FSL_SRDS_1 select SYS_HAS_SERDES select SYS_FSL_DDR @@ -124,6 +128,7 @@ config ARCH_LS1088A select ARM_ERRATA_855873 if !TFABOOT select FSL_LAYERSCAPE select FSL_LSCH3 + select GICV3 select SYS_FSL_SRDS_1 select SYS_HAS_SERDES select SYS_FSL_DDR @@ -168,6 +173,7 @@ config ARCH_LS2080A select ARM_ERRATA_833471 select FSL_LAYERSCAPE select FSL_LSCH3 + select GICV3 select SYS_FSL_SRDS_1 select SYS_HAS_SERDES select SYS_FSL_DDR @@ -214,6 +220,7 @@ config ARCH_LX2162A bool select ARMV8_SET_SMPEN select FSL_LSCH3 + select GICV3 select NXP_LSCH3_2 select SYS_HAS_SERDES select SYS_FSL_SRDS_1 @@ -245,6 +252,7 @@ config ARCH_LX2160A bool select ARMV8_SET_SMPEN select FSL_LSCH3 + select GICV3 select NXP_LSCH3_2 select SYS_HAS_SERDES select SYS_FSL_SRDS_1 diff --git a/arch/arm/include/asm/arch-fsl-layerscape/config.h b/arch/arm/include/asm/arch-fsl-layerscape/config.h index 3675ce763d1d..f932db10c307 100644 ---
[PATCH] nand: vf610_nfc: Do not abuse CONFIG namespace
This driver uses the CONFIG namespace to set the chips internal CONFIG
namespace related bits. However, CONFIG is reserved for the top-level
Kconfig based configuration system. Use CFG as the namespace here
instead to avoid pollution.
Signed-off-by: Tom Rini
---
drivers/mtd/nand/raw/vf610_nfc.c | 54
1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/drivers/mtd/nand/raw/vf610_nfc.c b/drivers/mtd/nand/raw/vf610_nfc.c
index e33953ec7c64..13fd631cb402 100644
--- a/drivers/mtd/nand/raw/vf610_nfc.c
+++ b/drivers/mtd/nand/raw/vf610_nfc.c
@@ -109,19 +109,19 @@
#define STATUS_BYTE1_MASK 0x00FF
/* NFC_FLASH_CONFIG Field */
-#define CONFIG_ECC_SRAM_ADDR_MASK 0x7FC0
-#define CONFIG_ECC_SRAM_ADDR_SHIFT 22
-#define CONFIG_ECC_SRAM_REQ_BIT(1<<21)
-#define CONFIG_DMA_REQ_BIT (1<<20)
-#define CONFIG_ECC_MODE_MASK 0x000E
-#define CONFIG_ECC_MODE_SHIFT 17
-#define CONFIG_FAST_FLASH_BIT (1<<16)
-#define CONFIG_16BIT (1<<7)
-#define CONFIG_BOOT_MODE_BIT (1<<6)
-#define CONFIG_ADDR_AUTO_INCR_BIT (1<<5)
-#define CONFIG_BUFNO_AUTO_INCR_BIT (1<<4)
-#define CONFIG_PAGE_CNT_MASK 0xF
-#define CONFIG_PAGE_CNT_SHIFT 0
+#define CFG_ECC_SRAM_ADDR_MASK 0x7FC0
+#define CFG_ECC_SRAM_ADDR_SHIFT22
+#define CFG_ECC_SRAM_REQ_BIT (1<<21)
+#define CFG_DMA_REQ_BIT(1<<20)
+#define CFG_ECC_MODE_MASK 0x000E
+#define CFG_ECC_MODE_SHIFT 17
+#define CFG_FAST_FLASH_BIT (1<<16)
+#define CFG_16BIT (1<<7)
+#define CFG_BOOT_MODE_BIT (1<<6)
+#define CFG_ADDR_AUTO_INCR_BIT (1<<5)
+#define CFG_BUFNO_AUTO_INCR_BIT(1<<4)
+#define CFG_PAGE_CNT_MASK 0xF
+#define CFG_PAGE_CNT_SHIFT 0
/* NFC_IRQ_STATUS Field */
#define IDLE_IRQ_BIT (1<<29)
@@ -342,8 +342,8 @@ static void vf610_nfc_addr_cycle(struct mtd_info *mtd, int
column, int page)
static inline void vf610_nfc_ecc_mode(struct mtd_info *mtd, int ecc_mode)
{
vf610_nfc_set_field(mtd, NFC_FLASH_CONFIG,
- CONFIG_ECC_MODE_MASK,
- CONFIG_ECC_MODE_SHIFT, ecc_mode);
+ CFG_ECC_MODE_MASK,
+ CFG_ECC_MODE_SHIFT, ecc_mode);
}
static inline void vf610_nfc_transfer_size(void __iomem *regbase, int size)
@@ -666,16 +666,16 @@ static int vf610_nfc_nand_init(struct vf610_nfc *nfc, int
devnum)
chip->ecc.size = PAGE_2K;
/* Set configuration register. */
- vf610_nfc_clear(mtd, NFC_FLASH_CONFIG, CONFIG_16BIT);
- vf610_nfc_clear(mtd, NFC_FLASH_CONFIG, CONFIG_ADDR_AUTO_INCR_BIT);
- vf610_nfc_clear(mtd, NFC_FLASH_CONFIG, CONFIG_BUFNO_AUTO_INCR_BIT);
- vf610_nfc_clear(mtd, NFC_FLASH_CONFIG, CONFIG_BOOT_MODE_BIT);
- vf610_nfc_clear(mtd, NFC_FLASH_CONFIG, CONFIG_DMA_REQ_BIT);
- vf610_nfc_set(mtd, NFC_FLASH_CONFIG, CONFIG_FAST_FLASH_BIT);
+ vf610_nfc_clear(mtd, NFC_FLASH_CONFIG, CFG_16BIT);
+ vf610_nfc_clear(mtd, NFC_FLASH_CONFIG, CFG_ADDR_AUTO_INCR_BIT);
+ vf610_nfc_clear(mtd, NFC_FLASH_CONFIG, CFG_BUFNO_AUTO_INCR_BIT);
+ vf610_nfc_clear(mtd, NFC_FLASH_CONFIG, CFG_BOOT_MODE_BIT);
+ vf610_nfc_clear(mtd, NFC_FLASH_CONFIG, CFG_DMA_REQ_BIT);
+ vf610_nfc_set(mtd, NFC_FLASH_CONFIG, CFG_FAST_FLASH_BIT);
/* Disable virtual pages, only one elementary transfer unit */
- vf610_nfc_set_field(mtd, NFC_FLASH_CONFIG, CONFIG_PAGE_CNT_MASK,
- CONFIG_PAGE_CNT_SHIFT, 1);
+ vf610_nfc_set_field(mtd, NFC_FLASH_CONFIG, CFG_PAGE_CNT_MASK,
+ CFG_PAGE_CNT_SHIFT, 1);
/* first scan to find the device and get the page size */
if (nand_scan_ident(mtd, CONFIG_SYS_MAX_NAND_DEVICE, NULL)) {
@@ -684,7 +684,7 @@ static int vf610_nfc_nand_init(struct vf610_nfc *nfc, int
devnum)
}
if (cfg.width == 16)
- vf610_nfc_set(mtd, NFC_FLASH_CONFIG, CONFIG_16BIT);
+ vf610_nfc_set(mtd, NFC_FLASH_CONFIG, CFG_16BIT);
/* Bad block options. */
if (cfg.flash_bbt)
@@ -734,12 +734,12 @@ static int vf610_nfc_nand_init(struct vf610_nfc *nfc, int
devnum)
/* Set ECC_STATUS offset */
vf610_nfc_set_field(mtd, NFC_FLASH_CONFIG,
- CONFIG_ECC_SRAM_ADDR_MASK,
- CONFIG_ECC_SRAM_ADDR_SHIFT,
+ CFG_ECC_SRAM_ADDR_MASK,
+ CFG_ECC_SRAM_ADDR_SHIFT,
Re: [PATCH 00/28] Initial implementation of bootmethod/bootflow
On Thu, Aug 19, 2021 at 08:25:33AM -0600, Simon Glass wrote: > Hi Tom, > > On Thu, 19 Aug 2021 at 07:59, Tom Rini wrote: > > > > On Wed, Aug 18, 2021 at 09:45:33PM -0600, Simon Glass wrote: > > > > > Bootmethod and bootflow provide a built-in way for U-Boot to > > > automatically boot > > > an Operating System without custom scripting and other customisation: > > > > > > - bootmethod - a method to scan a device to find bootflows (owned by > > > U-Boot) > > > - bootflow - a description of how to boot (owned by the distro) > > > > > > This series provides an initial implementation of these, enable to scan > > > for bootflows from MMC and Ethernet. The only bootflow supported is > > > distro boot, i.e. an extlinux.conf file included on a filesystem or > > > tftp server. It works similiarly to the existing script-based approach, > > > but is native to U-Boot. > > > > > > With this we can boot on a Raspberry Pi 3 with just one command: > > > > > >bootflow scan -lb > > > > > > which means to scan, listing (-l) each bootflow and trying to boot each > > > one (-b). The final patch shows this. > > > > > > It is intended that this approach be expanded to support mechanisms other > > > than distro boot, including EFI-related ones. With a standard way to > > > identify boot devices, these features become easier. It also should > > > support U-Boot scripts, for backwards compatibility only. > > > > > > The first patch of this series moves boot-related code out of common/ and > > > into a new boot/ directory. This helps to collect these related files > > > in one place, as common/ is quite large. > > > > > > Like sysboot, this feature makes use of the existing PXE implementation. > > > Much of this series consists of cleaning up that code and refactoring it > > > into something closer to a module that can be called, teasing apart its > > > reliance on the command-line interpreter to access filesystems and the > > > like. Also it now uses function arguments and its own context struct > > > internally rather than environment variables, which is very hard to > > > follow. No core functional change is included in the included PXE patches. > > > > > > For documentation, see the 'doc' patch. > > > > > > There is quite a long list of future work included in the documentation. > > > One question is the choice of naming. Since this is a bootloader, should > > > we just call this a 'method' and a 'flow' ? The 'boot' prefix is already > > > shared by other commands like bootm, booti, etc. > > > > > > The design is described here: > > > > > > https://drive.google.com/file/d/1ggW0KJpUOR__vBkj3l61L2dav4ZkNC12/view?usp=sharing > > > > > > The series is available at u-boot-dm/bmea-working > > > > My question / concern is this. Would the next step here be to > > implement the generic UEFI boot path? Today, I can write Fedora 34 for > > AArch64 to a USB stick, boot U-Boot off of uSD card and the installer > > automatically boots. I'm sure I could do the same with the BSDs. > > Reading the documentation left me with the impression that every OSV > > would be expected to write something, so that their installer / OS boot. > > But there's already standards for that, which they do, and we should be > > implementing (and do, via the current distro_boot) or making easier to > > enable. Thanks! > > Here you are talking about scanning for a bootflow. It is not actually > OS-specific. If it were, there would be no point to this :-) > > If you look in the distro scripts you will see 'scan_dev_for_efi' (and > also scan_dev_for_scrips). At least the first needs to be implemented > a bit like the distro boot is at present. So far I have only > implemented scan_dev_for_extlinux (plus pxe) as it is enough to show > the concept. > > Adding EFI it's likely to be about the same amount of code as distro.c > at present, perhaps a little less since we don't have the network > case. It is used by Fedora 34, I believe, so is easy enough for me to > do. But I wanted to get something out as the concept is visible in > this series. OK, good. I'd certainly like to see how this looks with EFI added. -- Tom signature.asc Description: PGP signature
Re: [PATCH 07/10] Makefile: Align fit-dtb.blob and u-boot.itb by 64bits for 64bit systems
On Thu, Aug 19, 2021 at 06:31:25PM +0200, Michal Simek wrote: > > > On 8/19/21 6:18 PM, Tom Rini wrote: > > On Thu, Aug 19, 2021 at 06:01:39PM +0200, Michal Simek wrote: > >> Hi Andre, > >> > >> On 8/19/21 5:56 PM, Andre Przywara wrote: > >>> On 8/19/21 12:19 PM, Michal Simek wrote: > >>> > >>> Hi, > >>> > Enabling MULTI_DTB_FIT and DTB_RESELECT can end up with multi DTBs in FIT > image placed and aligned only by 32bits (4bytes). For 64bit systems there > is 64bit (8bytes) alignment required. That's why make sure that > fit-dtb.blob and u-boot.itb as our primary target images for Xilinx > ZynqMP > are all 64bit aligned. The patch is using CONFIG_PHYS_64BIT macro to > identify 64bit systems (including 32bit systems with PAE). > > Signed-off-by: Michal Simek > --- > > Makefile | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/Makefile b/Makefile > index 269e353a28ad..1bbe95595efe 100644 > --- a/Makefile > +++ b/Makefile > @@ -1169,6 +1169,10 @@ MKIMAGEFLAGS_fit-dtb.blob = -f auto -A $(ARCH) > -T firmware -C none -O u-boot \ > -a 0 -e 0 -E \ > $(patsubst %,-b arch/$(ARCH)/dts/%.dtb,$(subst > ",,$(CONFIG_OF_LIST))) -d /dev/null > +ifeq ($(CONFIG_PHYS_64BIT),y) > >>> > >>> Why is this restricted to 64-bit "systems"? The DT spec[1] clearly > >>> states that some DT parts (/memreserved/ block, for instance), must be > >>> 64-bit aligned, which means the whole blobs needs to be 64-bit aligned. > >>> Granted this probably does not cause real issues on 32-bit systems, but > >>> is violating the spec anyway. > >>> So I'd say we add the alignment requirement unconditionally. > >> > >> That's even better for me and we need to make sure that dtbs itself are > >> aligned and also dtbs inside FIT image are aligned too. > > > > Right, all dtbs need to be 8 byte aligned to start with. Enforcing this > > is what will unblock moving to a newer libfdt where that's checked for > > up-front now. > > As is in the second thread. Does it make sense to also align the end? > I did that in 8/10 patch. > The problem with these alignments is that you also need to align the > start of FIT image. Maybe would the best to copy fdt to different > aligned location. Right, so a good question. I have suggested before that we stop assuming that we (U-Boot SPL, etc) can use the device tree in-place and instead move it somewhere aligned. I'm not sure off-hand what ends up being best, someone would need to investigate a bit. -- Tom signature.asc Description: PGP signature
Re: [PATCH 08/10] arm64: dts: Make sure that all DTBs are 64bit aligned for 64bit systems
Hi Andre, On 8/19/21 6:10 PM, Andre Przywara wrote: > On 8/19/21 12:19 PM, Michal Simek wrote: > > Hi Michal, > >> DTBs for 64bit systems should be also 64bit aligned. > > What does "align" mean here, exactly? This is about generating .dtb > *files*, right? dtc makes sure that the internal structures are properly > aligned, so what else should be aligned here? > >> Signed-off-by: Michal Simek >> --- >> >> arch/arm/dts/Makefile | 4 >> 1 file changed, 4 insertions(+) >> >> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile >> index 537c96bf5b35..8d4fc333ea7a 100644 >> --- a/arch/arm/dts/Makefile >> +++ b/arch/arm/dts/Makefile >> @@ -1,5 +1,9 @@ >> # SPDX-License-Identifier: GPL-2.0+ >> +ifdef CONFIG_PHYS_64BIT >> +DTC_FLAGS += -a 0x8 > > By looking into the dtc source this looks like to make sure the *size* > of the DTB is 8-byte aligned, is that the intention here, or even > useful? If it is, it should apply unconditionally, not only to 64-bit > systems. The question is correct. I did it mostly for being safe that DTBs start and end is 64bit aligned. I didn't hit any issue with not aligned end and maybe it is not useful to do it. It was just more convenient for me to work with also 64bit aligned image size. Thanks, Michal
Re: [PATCH 07/10] Makefile: Align fit-dtb.blob and u-boot.itb by 64bits for 64bit systems
On 8/19/21 6:18 PM, Tom Rini wrote: > On Thu, Aug 19, 2021 at 06:01:39PM +0200, Michal Simek wrote: >> Hi Andre, >> >> On 8/19/21 5:56 PM, Andre Przywara wrote: >>> On 8/19/21 12:19 PM, Michal Simek wrote: >>> >>> Hi, >>> Enabling MULTI_DTB_FIT and DTB_RESELECT can end up with multi DTBs in FIT image placed and aligned only by 32bits (4bytes). For 64bit systems there is 64bit (8bytes) alignment required. That's why make sure that fit-dtb.blob and u-boot.itb as our primary target images for Xilinx ZynqMP are all 64bit aligned. The patch is using CONFIG_PHYS_64BIT macro to identify 64bit systems (including 32bit systems with PAE). Signed-off-by: Michal Simek --- Makefile | 7 +++ 1 file changed, 7 insertions(+) diff --git a/Makefile b/Makefile index 269e353a28ad..1bbe95595efe 100644 --- a/Makefile +++ b/Makefile @@ -1169,6 +1169,10 @@ MKIMAGEFLAGS_fit-dtb.blob = -f auto -A $(ARCH) -T firmware -C none -O u-boot \ -a 0 -e 0 -E \ $(patsubst %,-b arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST))) -d /dev/null +ifeq ($(CONFIG_PHYS_64BIT),y) >>> >>> Why is this restricted to 64-bit "systems"? The DT spec[1] clearly >>> states that some DT parts (/memreserved/ block, for instance), must be >>> 64-bit aligned, which means the whole blobs needs to be 64-bit aligned. >>> Granted this probably does not cause real issues on 32-bit systems, but >>> is violating the spec anyway. >>> So I'd say we add the alignment requirement unconditionally. >> >> That's even better for me and we need to make sure that dtbs itself are >> aligned and also dtbs inside FIT image are aligned too. > > Right, all dtbs need to be 8 byte aligned to start with. Enforcing this > is what will unblock moving to a newer libfdt where that's checked for > up-front now. > As is in the second thread. Does it make sense to also align the end? I did that in 8/10 patch. The problem with these alignments is that you also need to align the start of FIT image. Maybe would the best to copy fdt to different aligned location. Thanks, Michal
Re: [PATCH 07/10] Makefile: Align fit-dtb.blob and u-boot.itb by 64bits for 64bit systems
On Thu, Aug 19, 2021 at 06:01:39PM +0200, Michal Simek wrote: > Hi Andre, > > On 8/19/21 5:56 PM, Andre Przywara wrote: > > On 8/19/21 12:19 PM, Michal Simek wrote: > > > > Hi, > > > >> Enabling MULTI_DTB_FIT and DTB_RESELECT can end up with multi DTBs in FIT > >> image placed and aligned only by 32bits (4bytes). For 64bit systems there > >> is 64bit (8bytes) alignment required. That's why make sure that > >> fit-dtb.blob and u-boot.itb as our primary target images for Xilinx > >> ZynqMP > >> are all 64bit aligned. The patch is using CONFIG_PHYS_64BIT macro to > >> identify 64bit systems (including 32bit systems with PAE). > >> > >> Signed-off-by: Michal Simek > >> --- > >> > >> Makefile | 7 +++ > >> 1 file changed, 7 insertions(+) > >> > >> diff --git a/Makefile b/Makefile > >> index 269e353a28ad..1bbe95595efe 100644 > >> --- a/Makefile > >> +++ b/Makefile > >> @@ -1169,6 +1169,10 @@ MKIMAGEFLAGS_fit-dtb.blob = -f auto -A $(ARCH) > >> -T firmware -C none -O u-boot \ > >> -a 0 -e 0 -E \ > >> $(patsubst %,-b arch/$(ARCH)/dts/%.dtb,$(subst > >> ",,$(CONFIG_OF_LIST))) -d /dev/null > >> +ifeq ($(CONFIG_PHYS_64BIT),y) > > > > Why is this restricted to 64-bit "systems"? The DT spec[1] clearly > > states that some DT parts (/memreserved/ block, for instance), must be > > 64-bit aligned, which means the whole blobs needs to be 64-bit aligned. > > Granted this probably does not cause real issues on 32-bit systems, but > > is violating the spec anyway. > > So I'd say we add the alignment requirement unconditionally. > > That's even better for me and we need to make sure that dtbs itself are > aligned and also dtbs inside FIT image are aligned too. Right, all dtbs need to be 8 byte aligned to start with. Enforcing this is what will unblock moving to a newer libfdt where that's checked for up-front now. -- Tom signature.asc Description: PGP signature
Re: [PATCH 08/10] arm64: dts: Make sure that all DTBs are 64bit aligned for 64bit systems
On 8/19/21 12:19 PM, Michal Simek wrote: Hi Michal, DTBs for 64bit systems should be also 64bit aligned. What does "align" mean here, exactly? This is about generating .dtb *files*, right? dtc makes sure that the internal structures are properly aligned, so what else should be aligned here? Signed-off-by: Michal Simek --- arch/arm/dts/Makefile | 4 1 file changed, 4 insertions(+) diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile index 537c96bf5b35..8d4fc333ea7a 100644 --- a/arch/arm/dts/Makefile +++ b/arch/arm/dts/Makefile @@ -1,5 +1,9 @@ # SPDX-License-Identifier: GPL-2.0+ +ifdef CONFIG_PHYS_64BIT +DTC_FLAGS += -a 0x8 By looking into the dtc source this looks like to make sure the *size* of the DTB is 8-byte aligned, is that the intention here, or even useful? If it is, it should apply unconditionally, not only to 64-bit systems. Cheers, Andre +endif + dtb-$(CONFIG_TARGET_SMARTWEB) += at91sam9260-smartweb.dtb dtb-$(CONFIG_TARGET_TAURUS) += at91sam9g20-taurus.dtb dtb-$(CONFIG_TARGET_CORVUS) += at91sam9g45-corvus.dtb
Re: [PATCH 07/10] Makefile: Align fit-dtb.blob and u-boot.itb by 64bits for 64bit systems
Hi Andre, On 8/19/21 5:56 PM, Andre Przywara wrote: > On 8/19/21 12:19 PM, Michal Simek wrote: > > Hi, > >> Enabling MULTI_DTB_FIT and DTB_RESELECT can end up with multi DTBs in FIT >> image placed and aligned only by 32bits (4bytes). For 64bit systems there >> is 64bit (8bytes) alignment required. That's why make sure that >> fit-dtb.blob and u-boot.itb as our primary target images for Xilinx >> ZynqMP >> are all 64bit aligned. The patch is using CONFIG_PHYS_64BIT macro to >> identify 64bit systems (including 32bit systems with PAE). >> >> Signed-off-by: Michal Simek >> --- >> >> Makefile | 7 +++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/Makefile b/Makefile >> index 269e353a28ad..1bbe95595efe 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -1169,6 +1169,10 @@ MKIMAGEFLAGS_fit-dtb.blob = -f auto -A $(ARCH) >> -T firmware -C none -O u-boot \ >> -a 0 -e 0 -E \ >> $(patsubst %,-b arch/$(ARCH)/dts/%.dtb,$(subst >> ",,$(CONFIG_OF_LIST))) -d /dev/null >> +ifeq ($(CONFIG_PHYS_64BIT),y) > > Why is this restricted to 64-bit "systems"? The DT spec[1] clearly > states that some DT parts (/memreserved/ block, for instance), must be > 64-bit aligned, which means the whole blobs needs to be 64-bit aligned. > Granted this probably does not cause real issues on 32-bit systems, but > is violating the spec anyway. > So I'd say we add the alignment requirement unconditionally. That's even better for me and we need to make sure that dtbs itself are aligned and also dtbs inside FIT image are aligned too. Cheers, Michal
Re: A mea culpa undefined reference in v2021.10-rc2, padding_algos, linker lists
Hi Alex,
On Wed, 18 Aug 2021 at 13:11, Alex G. wrote:
>
> Hi Simon,
>
> I'm seeing an undefined reference to padding_pkcs_15_verify with
> v2021.10-rc2. It happens when enabling FIT_SIGNATURE. I've tracked it
> down to the following two commits:
>
> commit 92c960bc1d ("lib: rsa: Remove #ifdefs from rsa.h")
> commit 61416fe9df ("Kconfig: FIT_SIGNATURE should not select RSA_VERIFY")
>
> Individually, each commit is fine, but when put together, they cause the
> issue, as the static inline padding_pkcs_15_verify() implementation is
> removed from rsa.h.
>
> My hypothesis is that moving padding_algos to a linker list will solve
> this specific problem. I realize you might be working on the same part
> of the code. Should I address this issue, or should I wait for your series?
No I'm not doing anything here actually. I did rebase my image series
but have not touched it since. One day.
Regards,
Simon
Re: [PATCH 07/10] Makefile: Align fit-dtb.blob and u-boot.itb by 64bits for 64bit systems
On 8/19/21 12:19 PM, Michal Simek wrote: Hi, Enabling MULTI_DTB_FIT and DTB_RESELECT can end up with multi DTBs in FIT image placed and aligned only by 32bits (4bytes). For 64bit systems there is 64bit (8bytes) alignment required. That's why make sure that fit-dtb.blob and u-boot.itb as our primary target images for Xilinx ZynqMP are all 64bit aligned. The patch is using CONFIG_PHYS_64BIT macro to identify 64bit systems (including 32bit systems with PAE). Signed-off-by: Michal Simek --- Makefile | 7 +++ 1 file changed, 7 insertions(+) diff --git a/Makefile b/Makefile index 269e353a28ad..1bbe95595efe 100644 --- a/Makefile +++ b/Makefile @@ -1169,6 +1169,10 @@ MKIMAGEFLAGS_fit-dtb.blob = -f auto -A $(ARCH) -T firmware -C none -O u-boot \ -a 0 -e 0 -E \ $(patsubst %,-b arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST))) -d /dev/null +ifeq ($(CONFIG_PHYS_64BIT),y) Why is this restricted to 64-bit "systems"? The DT spec[1] clearly states that some DT parts (/memreserved/ block, for instance), must be 64-bit aligned, which means the whole blobs needs to be 64-bit aligned. Granted this probably does not cause real issues on 32-bit systems, but is violating the spec anyway. So I'd say we add the alignment requirement unconditionally. Cheers, Andre [1] https://github.com/devicetree-org/devicetree-specification/releases/download/v0.3/devicetree-specification-v0.3.pdf +MKIMAGEFLAGS_fit-dtb.blob += -B 0x8 +endif + ifneq ($(EXT_DTB),) u-boot-fit-dtb.bin: u-boot-nodtb.bin $(EXT_DTB) $(call if_changed,cat) @@ -1431,6 +1435,9 @@ MKIMAGEFLAGS_u-boot.itb = else MKIMAGEFLAGS_u-boot.itb = -E endif +ifeq ($(CONFIG_PHYS_64BIT),y) +MKIMAGEFLAGS_u-boot.itb += -B 0x8 +endif ifdef U_BOOT_ITS u-boot.itb: u-boot-nodtb.bin \
[PATCH v2 6/6] doc: Add documentation for the Arm vexpress board configs
From: Peter Hoyes Create a new documentation section for Arm Ltd boards with a sub-page for the vexpress board (FVP-A, FVP-R and Juno). Document how the armv8_switch_to_el1 environment variable can be used to switch between booting from S-EL2/S-EL1 at runtime on the BASER_FVP. Signed-off-by: Peter Hoyes --- doc/arch/arm64.rst | 3 +- doc/board/armltd/index.rst | 9 ++ doc/board/armltd/vexpress64.rst | 57 + doc/board/index.rst | 1 + 4 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 doc/board/armltd/index.rst create mode 100644 doc/board/armltd/vexpress64.rst diff --git a/doc/arch/arm64.rst b/doc/arch/arm64.rst index 80498f6f6b..f20eb8f1b2 100644 --- a/doc/arch/arm64.rst +++ b/doc/arch/arm64.rst @@ -18,7 +18,8 @@ Notes classical firmware (like initial hardware setup, CPU errata workarounds or SMP bringup). U-Boot can be entered in EL2 when its main purpose is that of a boot loader. It can drop to lower exception levels before - entering the OS. + entering the OS. For ARMv8-R it is recommened to enter at S-EL2, as for this + architecture there is no S-EL3. 2. U-Boot for arm64 is compiled with AArch64-gcc. AArch64-gcc use rela relocation format, a tool(tools/relocate-rela) by Scott Wood diff --git a/doc/board/armltd/index.rst b/doc/board/armltd/index.rst new file mode 100644 index 00..b6786c114f --- /dev/null +++ b/doc/board/armltd/index.rst @@ -0,0 +1,9 @@ +.. SPDX-License-Identifier: GPL-2.0+ + +Arm Ltd += + +.. toctree:: + :maxdepth: 2 + + vexpress64.rst diff --git a/doc/board/armltd/vexpress64.rst b/doc/board/armltd/vexpress64.rst new file mode 100644 index 00..23cf6aec88 --- /dev/null +++ b/doc/board/armltd/vexpress64.rst @@ -0,0 +1,57 @@ +.. SPDX-License-Identifier: GPL-2.0+ + +Arm Versatile Express += + +The vexpress_* board configuration supports the following platforms: + + * FVP_Base_RevC-2xAEMvA + * FVP_BaseR_AEMv8R + * Juno development board + +Fixed Virtual Platforms +--- + +The Fixed Virtual Platforms (FVP) are complete simulations of an Arm system, +including processor, memory and peripherals. They are set out in a "programmer's +view", which gives a comprehensive model on which to build and test software. + +The supported FVPs are available free of charge and can be downloaded from the +Arm developer site [1]_ (user registration might be required). + +Supported features: + + * GICv3 + * Generic timer + * PL011 UART + * SMC9 network interface + +The default configuration assumes that U-Boot is boostrapped from the start of +the DRAM (address 0x8000 for AEMvA; 0x for AEMv8R) using a suitable +bootloader. Alternatively, U-Boot can be launched directly by mapping the binary +to the same address (using the FVP's --data argument). + +The FVPs can be debugged using Arm Development Studio [2]_. + +FVP_BaseR +^ + +On Armv8r64 platforms (such as the FVP_BaseR), U-Boot runs at S-EL2, so +CONFIG_ARMV8_SWITCH_TO_EL1 is defined so that the next stage boots at S-EL1. If +S-EL2 is desired instead, the *armv8_switch_to_el1* environment variable is +available. This can be set to *n* to override the config flag and boot the next +stage at S-EL2 instead. + +Juno + + +The Juno development board is an open, vendor-neutral Armv8-A development +platform that supports an out-of-the-box Linux software package. A range of +plug-in expansion options enables hardware and software applications to be +developped and debugged. + +References +-- + +.. [1] https://developer.arm.com/tools-and-software/simulation-models/fixed-virtual-platforms +.. [2] https://developer.arm.com/tools-and-software/embedded/arm-development-studio diff --git a/doc/board/index.rst b/doc/board/index.rst index 9e90978891..cd4f273b4d 100644 --- a/doc/board/index.rst +++ b/doc/board/index.rst @@ -10,6 +10,7 @@ Board-specific doc advantech/index AndesTech/index amlogic/index + armltd/index atmel/index congatec/index coreboot/index -- 2.25.1
[PATCH v2 5/6] arm: Use armv8_switch_to_el1 env to switch to EL1
From: Peter Hoyes
Use the environment variable armv8_switch_to_el1 to determine whether
to switch to EL1 at runtime. This is an alternative to the
CONFIG_ARMV8_SWITCH_TO_EL1 compile-time option.
The environment variable will be ineffective if the ARMV8_MULTIENTRY
config is used.
This is required by the Armv8r64 architecture, which must be able to
boot at S-EL1 for Linux but may need to boot at other ELs for other
systems.
Signed-off-by: Peter Hoyes
---
arch/arm/lib/bootm.c | 40 +---
1 file changed, 25 insertions(+), 15 deletions(-)
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index f60ee3a7e6..ea9bfe7570 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -317,7 +317,6 @@ __weak void update_os_arch_secondary_cores(uint8_t os_arch)
{
}
-#ifdef CONFIG_ARMV8_SWITCH_TO_EL1
static void switch_to_el1(void)
{
if ((IH_ARCH_DEFAULT == IH_ARCH_ARM64) &&
@@ -332,7 +331,6 @@ static void switch_to_el1(void)
ES_TO_AARCH64);
}
#endif
-#endif
/* Subcommand: GO */
static void boot_jump_linux(bootm_headers_t *images, int flag)
@@ -359,21 +357,33 @@ static void boot_jump_linux(bootm_headers_t *images, int
flag)
update_os_arch_secondary_cores(images->os.arch);
-#ifdef CONFIG_ARMV8_SWITCH_TO_EL1
- armv8_switch_to_el2((u64)images->ft_addr, 0, 0, 0,
- (u64)switch_to_el1, ES_TO_AARCH64);
+#ifdef CONFIG_ARMV8_MULTIENTRY
+ int armv8_switch_to_el1 = -1;
#else
- if ((IH_ARCH_DEFAULT == IH_ARCH_ARM64) &&
- (images->os.arch == IH_ARCH_ARM))
- armv8_switch_to_el2(0, (u64)gd->bd->bi_arch_number,
- (u64)images->ft_addr, 0,
- (u64)images->ep,
- ES_TO_AARCH32);
- else
- armv8_switch_to_el2((u64)images->ft_addr, 0, 0, 0,
- images->ep,
- ES_TO_AARCH64);
+ int armv8_switch_to_el1 = env_get_yesno("armv8_switch_to_el1");
#endif
+#ifdef CONFIG_ARMV8_SWITCH_TO_EL1
+ if (armv8_switch_to_el1 == -1) {
+ armv8_switch_to_el1 = 1;
+ }
+#endif
+ if (armv8_switch_to_el1 == 1) {
+ armv8_switch_to_el2((u64)images->ft_addr, 0, 0, 0,
+ (u64)switch_to_el1, ES_TO_AARCH64);
+ } else {
+ if ((IH_ARCH_DEFAULT == IH_ARCH_ARM64) &&
+ (images->os.arch == IH_ARCH_ARM))
+ armv8_switch_to_el2(0,
+ (u64)gd->bd->bi_arch_number,
+ (u64)images->ft_addr, 0,
+ (u64)images->ep,
+ ES_TO_AARCH32);
+ else
+ armv8_switch_to_el2((u64)images->ft_addr,
+ 0, 0, 0,
+ images->ep,
+ ES_TO_AARCH64);
+ }
}
#else
unsigned long machid = gd->bd->bi_arch_number;
--
2.25.1
[PATCH v2 4/6] vexpress64: Add BASER_FVP vexpress board variant
From: Peter Hoyes
The BASER_FVP board variant is implemented on top of the BASE_FVP board
config (which, in turn, is based on the Juno Versatile Express board
config). They all share a similar memory map - for BASER_FVP the map is
inverted from the BASE_FVP
(https://developer.arm.com/documentation/100964/1114/Base-Platform/Base---memory/BaseR-Platform-memory-map)
* Create new TARGET_VEXPRESS64_BASER_FVP target, which uses the same
board config as BASE_FVP and JUNO
* Adapt vexpress_aemv8a.h header file to support BASER_FVP (and rename
to vexpress_aemv8.h)
* Enable config to switch to EL1 for the BASER_FVP
* Create vexpress_aemv8r defconfig
* Provide an MPU memory map for the BASER_FVP
For now, only single core boot is supported.
Signed-off-by: Peter Hoyes
---
arch/arm/Kconfig | 7 +++
board/armltd/vexpress64/Kconfig | 5 +-
board/armltd/vexpress64/vexpress64.c | 22 +++
configs/vexpress_aemv8r_defconfig | 18 ++
doc/README.semihosting| 2 +-
.../{vexpress_aemv8a.h => vexpress_aemv8.h} | 63 ---
6 files changed, 93 insertions(+), 24 deletions(-)
create mode 100644 configs/vexpress_aemv8r_defconfig
rename include/configs/{vexpress_aemv8a.h => vexpress_aemv8.h} (83%)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index e935c60bd7..c96d89b655 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1168,6 +1168,13 @@ config TARGET_VEXPRESS64_BASE_FVP
select PL01X_SERIAL
select SEMIHOSTING
+config TARGET_VEXPRESS64_BASER_FVP
+ bool "Support Versatile Express ARMv8r64 FVP BASE model"
+ select ARM64
+ select DM
+ select DM_SERIAL
+ select PL01X_SERIAL
+
config TARGET_VEXPRESS64_JUNO
bool "Support Versatile Express Juno Development Platform"
select ARM64
diff --git a/board/armltd/vexpress64/Kconfig b/board/armltd/vexpress64/Kconfig
index 1d13f542e6..1f0c7ad969 100644
--- a/board/armltd/vexpress64/Kconfig
+++ b/board/armltd/vexpress64/Kconfig
@@ -1,4 +1,5 @@
-if TARGET_VEXPRESS64_BASE_FVP || TARGET_VEXPRESS64_JUNO
+if TARGET_VEXPRESS64_BASE_FVP || TARGET_VEXPRESS64_JUNO || \
+ TARGET_VEXPRESS64_BASER_FVP
config SYS_BOARD
default "vexpress64"
@@ -7,7 +8,7 @@ config SYS_VENDOR
default "armltd"
config SYS_CONFIG_NAME
- default "vexpress_aemv8a"
+ default "vexpress_aemv8"
config JUNO_DTB_PART
string "NOR flash partition holding DTB"
diff --git a/board/armltd/vexpress64/vexpress64.c
b/board/armltd/vexpress64/vexpress64.c
index 2e4260286b..eb4951d07c 100644
--- a/board/armltd/vexpress64/vexpress64.c
+++ b/board/armltd/vexpress64/vexpress64.c
@@ -18,6 +18,7 @@
#include
#include "pcie.h"
#include
+#include
DECLARE_GLOBAL_DATA_PTR;
@@ -54,6 +55,27 @@ static struct mm_region vexpress64_mem_map[] = {
struct mm_region *mem_map = vexpress64_mem_map;
+static struct mpu_region vexpress64_aemv8r_mem_map[] = {
+ {
+ .start = 0x0UL,
+ .end = 0x7fffUL,
+ .attrs = PRLAR_ATTRIDX(MT_NORMAL)
+ }, {
+ .start = 0x8000UL,
+ .end = 0xUL,
+ .attrs = PRLAR_ATTRIDX(MT_DEVICE_NGNRNE)
+ }, {
+ .start = 0x1UL,
+ .end = 0xffUL,
+ .attrs = PRLAR_ATTRIDX(MT_NORMAL)
+ }, {
+ /* List terminator */
+ 0,
+ }
+};
+
+struct mpu_region *mpu_mem_map = vexpress64_aemv8r_mem_map;
+
/* This function gets replaced by platforms supporting PCIe.
* The replacement function, eg. on Juno, initialises the PCIe bus.
*/
diff --git a/configs/vexpress_aemv8r_defconfig
b/configs/vexpress_aemv8r_defconfig
new file mode 100644
index 00..9b4cd1aa2b
--- /dev/null
+++ b/configs/vexpress_aemv8r_defconfig
@@ -0,0 +1,18 @@
+CONFIG_ARM=y
+CONFIG_TARGET_VEXPRESS64_BASER_FVP=y
+CONFIG_SYS_TEXT_BASE=0x1000
+CONFIG_SYS_MALLOC_F_LEN=0x2000
+CONFIG_NR_DRAM_BANKS=2
+CONFIG_SYS_MEMTEST_START=0x8000
+CONFIG_SYS_MEMTEST_END=0xff00
+CONFIG_ENV_SIZE=0x4
+CONFIG_ENV_SECT_SIZE=0x4
+CONFIG_IDENT_STRING=" vexpress_aemv8r64"
+CONFIG_DISTRO_DEFAULTS=y
+CONFIG_BOOTDELAY=3
+CONFIG_USE_BOOTARGS=y
+CONFIG_BOOTARGS="console=ttyAMA0 earlycon=pl011,0x9c09 rootfstype=ext4
root=/dev/vda1 rw rootwait"
+# CONFIG_USE_BOOTCOMMAND is not set
+# CONFIG_DISPLAY_CPUINFO is not set
+CONFIG_SYS_PROMPT="VExpress64# "
+CONFIG_OF_LIBFDT=y
diff --git a/doc/README.semihosting b/doc/README.semihosting
index c01bed..f382d0131e 100644
--- a/doc/README.semihosting
+++ b/doc/README.semihosting
@@ -25,7 +25,7 @@ or turning on CONFIG_BASE_FVP for the more full featured
model.
Rather than create a new armv8 board similar to armltd/vexpress64, add
semihosting calls to the existing one, enabled with CONFIG_SEMIHOSTING
and CONFIG_BASE_FVP both set. Also reuse the existing board config file
-vexpress_aemv8a.h
[PATCH v2 3/6] armv8: Add ARMv8 MPU configuration logic
From: Peter Hoyes
Armv8r64 is the first Armv8 platform that only has a PMSA at the
current exception level. The architecture supplement for Armv8r64
describes new fields in ID_AA64MMFR0_EL1 which can be used to detect
whether a VMSA or PMSA is present. These fields are RES0 on Armv8a.
Add logic to read these fields and, for the protection of the memory
used by U-Boot, initialize the MPU instead of the MMU during init, then
clear the MPU regions before transition to the next stage.
Provide a default (blank) MPU memory map, which can be overridden by
board configurations.
Signed-off-by: Peter Hoyes
---
arch/arm/cpu/armv8/cache_v8.c| 96 +++-
arch/arm/include/asm/armv8/mpu.h | 61
2 files changed, 154 insertions(+), 3 deletions(-)
create mode 100644 arch/arm/include/asm/armv8/mpu.h
diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c
index 3de18c7675..46625675bd 100644
--- a/arch/arm/cpu/armv8/cache_v8.c
+++ b/arch/arm/cpu/armv8/cache_v8.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
DECLARE_GLOBAL_DATA_PTR;
@@ -365,6 +366,86 @@ __weak u64 get_page_table_size(void)
return size;
}
+static void mpu_clear_regions(void)
+{
+ int i;
+
+ for (i = 0; mpu_mem_map[i].end || mpu_mem_map[i].attrs; i++) {
+ setup_el2_mpu_region(i, 0, 0);
+ }
+}
+
+static struct mpu_region default_mpu_mem_map[] = {{0,}};
+__weak struct mpu_region *mpu_mem_map = default_mpu_mem_map;
+
+static void mpu_setup(void)
+{
+ int i;
+
+ if (current_el() != 2) {
+ panic("MPU configuration is only supported at EL2");
+ }
+
+ set_sctlr(get_sctlr() & ~(CR_M | CR_WXN));
+
+ asm volatile("msr MAIR_EL2, %0" : : "r" MEMORY_ATTRIBUTES);
+
+ for (i = 0; mpu_mem_map[i].end || mpu_mem_map[i].attrs; i++) {
+ setup_el2_mpu_region(i,
+ PRBAR_ADDRESS(mpu_mem_map[i].start)
+ | PRBAR_OUTER_SH | PRBAR_AP_RW_ANY,
+ PRLAR_ADDRESS(mpu_mem_map[i].end)
+ | mpu_mem_map[i].attrs | PRLAR_EN_BIT
+ );
+ }
+
+ set_sctlr(get_sctlr() | CR_M);
+}
+
+static bool el_has_mmu(void)
+{
+ uint64_t id_aa64mmfr0;
+ asm volatile("mrs %0, id_aa64mmfr0_el1"
+ : "=r" (id_aa64mmfr0) : : "cc");
+ uint64_t msa = id_aa64mmfr0 & ID_AA64MMFR0_EL1_MSA_MASK;
+ uint64_t msa_frac = id_aa64mmfr0 & ID_AA64MMFR0_EL1_MSA_FRAC_MASK;
+
+ switch (msa) {
+ case ID_AA64MMFR0_EL1_MSA_VMSA:
+ /*
+* VMSA supported in all translation regimes.
+* No support for PMSA.
+*/
+ return true;
+ case ID_AA64MMFR0_EL1_MSA_USE_FRAC:
+ /* See MSA_frac for the supported MSAs. */
+ switch (msa_frac) {
+ case ID_AA64MMFR0_EL1_MSA_FRAC_NO_PMSA:
+ /*
+* PMSA not supported in any translation
+* regime.
+*/
+ return true;
+ case ID_AA64MMFR0_EL1_MSA_FRAC_VMSA:
+ /*
+ * PMSA supported in all translation
+ * regimes. No support for VMSA.
+ */
+ case ID_AA64MMFR0_EL1_MSA_FRAC_PMSA:
+ /*
+* PMSA supported in all translation
+* regimes.
+*/
+ return false;
+ default:
+ panic("Unsupported id_aa64mmfr0_el1 " \
+ "MSA_frac value");
+ }
+ default:
+ panic("Unsupported id_aa64mmfr0_el1 MSA value");
+ }
+}
+
void setup_pgtables(void)
{
int i;
@@ -479,8 +560,13 @@ void dcache_enable(void)
/* The data cache is not active unless the mmu is enabled */
if (!(get_sctlr() & CR_M)) {
invalidate_dcache_all();
- __asm_invalidate_tlb_all();
- mmu_setup();
+
+ if (el_has_mmu()) {
+ __asm_invalidate_tlb_all();
+ mmu_setup();
+ } else {
+ mpu_setup();
+ }
}
set_sctlr(get_sctlr() | CR_C);
@@ -499,7 +585,11 @@ void dcache_disable(void)
set_sctlr(sctlr & ~(CR_C|CR_M));
[PATCH v2 2/6] armv8: Ensure EL1&0 VMSA is enabled
From: Peter Hoyes On Armv8-R, the EL1&0 memory system architecture is configurable as a VMSA or PMSA, and resets to an "architecturally unknown" value. Add code to armv8_switch_to_el1_m which detects whether the MSA at EL1&0 is configurable using the id_aa64mmfr0_el1 register MSA fields. If it is we must ensure the VMSA is enabled so that a rich OS can boot. The MSA and MSA_FRAC fields are described in the Armv8-R architecture profile supplement (section G1.3.7): https://developer.arm.com/documentation/ddi0600/latest/ Signed-off-by: Peter Hoyes --- arch/arm/include/asm/macro.h | 17 + arch/arm/include/asm/system.h | 24 2 files changed, 41 insertions(+) diff --git a/arch/arm/include/asm/macro.h b/arch/arm/include/asm/macro.h index e1eefc283f..ecd8221c0d 100644 --- a/arch/arm/include/asm/macro.h +++ b/arch/arm/include/asm/macro.h @@ -316,6 +316,23 @@ lr .reqx30 csel\tmp, \tmp2, \tmp, eq msr hcr_el2, \tmp + /* +* Detect whether the system has a configurable memory system +* architecture at EL1&0 +*/ + mrs \tmp, id_aa64mmfr0_el1 + lsr \tmp, \tmp, #48 + and \tmp, \tmp, #((ID_AA64MMFR0_EL1_MSA_MASK | \ + ID_AA64MMFR0_EL1_MSA_FRAC_MASK) >> 48) + cmp \tmp, #((ID_AA64MMFR0_EL1_MSA_USE_FRAC | \ + ID_AA64MMFR0_EL1_MSA_FRAC_VMSA) >> 48) + bne 2f + + /* Ensure the EL1&0 VMSA is enabled */ + mov \tmp, #(VTCR_EL2_MSA) + msr vtcr_el2, \tmp +2: + /* Return to the EL1_SP1 mode from EL2 */ ldr \tmp, =(SPSR_EL_DEBUG_MASK | SPSR_EL_SERR_MASK |\ SPSR_EL_IRQ_MASK | SPSR_EL_FIQ_MASK |\ diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h index 77aa18909e..e4c11e830a 100644 --- a/arch/arm/include/asm/system.h +++ b/arch/arm/include/asm/system.h @@ -83,6 +83,30 @@ #define HCR_EL2_RW_AARCH32 (0 << 31) /* Lower levels are AArch32 */ #define HCR_EL2_HCD_DIS(1 << 29) /* Hypervisor Call disabled */ +/* + * VTCR_EL2 bits definitions + */ +#define VTCR_EL2_MSA (1 << 31) /* EL1&0 memory architecture*/ + +/* + * ID_AA64MMFR0_EL1 bits definitions + */ +#define ID_AA64MMFR0_EL1_MSA_FRAC_MASK (0xFUL << 52) /* Memory system +architecture +frac */ +#define ID_AA64MMFR0_EL1_MSA_FRAC_VMSA (0x2UL << 52) /* EL1&0 supports +VMSA */ +#define ID_AA64MMFR0_EL1_MSA_FRAC_PMSA (0x1UL << 52) /* EL1&0 only +supports PMSA*/ +#define ID_AA64MMFR0_EL1_MSA_FRAC_NO_PMSA (0x0UL << 52) /* No PMSA +support */ +#define ID_AA64MMFR0_EL1_MSA_MASK (0xFUL << 48) /* Memory system +architecture */ +#define ID_AA64MMFR0_EL1_MSA_USE_FRAC (0xFUL << 48) /* Use MSA_FRAC */ +#define ID_AA64MMFR0_EL1_MSA_VMSA (0x0UL << 48) /* Memory system +architecture +is VMSA */ + /* * ID_AA64ISAR1_EL1 bits definitions */ -- 2.25.1
[PATCH v2 1/6] armv8: Disable pointer authentication traps for EL1
From: Peter Hoyes The use of ARMv8.3 pointer authentication (PAuth) is governed by fields in HCR_EL2, which trigger a 'trap to EL2' if not enabled. The reset value of these fields is 'architecturally unknown' so we must ensure that the fields are enabled (to disable the traps) if we are entering the kernel at EL1. The APK field disables PAuth instruction traps and the API field disables PAuth register traps Add code to disable the traps in armv8_switch_to_el1_m. Prior to doing so, it checks fields in the ID_AA64ISAR1_EL1 register to ensure pointer authentication is supported by the hardware. The runtime checks require a second temporary register, so add this to the EL1 transition macro signature and update 2 call sites. Signed-off-by: Peter Hoyes --- arch/arm/cpu/armv8/fsl-layerscape/spintable.S | 2 +- arch/arm/cpu/armv8/transition.S | 2 +- arch/arm/include/asm/macro.h | 11 +-- arch/arm/include/asm/system.h | 15 +++ 4 files changed, 26 insertions(+), 4 deletions(-) diff --git a/arch/arm/cpu/armv8/fsl-layerscape/spintable.S b/arch/arm/cpu/armv8/fsl-layerscape/spintable.S index 363ded03e6..d6bd188459 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/spintable.S +++ b/arch/arm/cpu/armv8/fsl-layerscape/spintable.S @@ -93,7 +93,7 @@ __secondary_boot_func: 4: #ifdef CONFIG_ARMV8_SWITCH_TO_EL1 switch_el x7, _dead_loop, 0f, _dead_loop -0: armv8_switch_to_el1_m x4, x6, x7 +0: armv8_switch_to_el1_m x4, x6, x7, x9 #else switch_el x7, 0f, _dead_loop, _dead_loop 0: armv8_switch_to_el2_m x4, x6, x7 diff --git a/arch/arm/cpu/armv8/transition.S b/arch/arm/cpu/armv8/transition.S index a31af4ffc8..9dbdff3a4f 100644 --- a/arch/arm/cpu/armv8/transition.S +++ b/arch/arm/cpu/armv8/transition.S @@ -40,7 +40,7 @@ ENTRY(armv8_switch_to_el1) * now, jump to the address saved in x4. */ br x4 -1: armv8_switch_to_el1_m x4, x5, x6 +1: armv8_switch_to_el1_m x4, x5, x6, x7 ENDPROC(armv8_switch_to_el1) .popsection diff --git a/arch/arm/include/asm/macro.h b/arch/arm/include/asm/macro.h index 485310d660..e1eefc283f 100644 --- a/arch/arm/include/asm/macro.h +++ b/arch/arm/include/asm/macro.h @@ -256,7 +256,7 @@ lr .reqx30 * For loading 64-bit OS, x0 is physical address to the FDT blob. * They will be passed to the guest. */ -.macro armv8_switch_to_el1_m, ep, flag, tmp +.macro armv8_switch_to_el1_m, ep, flag, tmp, tmp2 /* Initialize Generic Timers */ mrs \tmp, cnthctl_el2 /* Enable EL1 access to timers */ @@ -306,7 +306,14 @@ lr .reqx30 b.eq1f /* Initialize HCR_EL2 */ - ldr \tmp, =(HCR_EL2_RW_AARCH64 | HCR_EL2_HCD_DIS) + /* Only disable PAuth traps if PAuth is supported */ + mrs \tmp, id_aa64isar1_el1 + ldr \tmp2, =(ID_AA64ISAR1_EL1_GPI | ID_AA64ISAR1_EL1_GPA | \ + ID_AA64ISAR1_EL1_API | ID_AA64ISAR1_EL1_APA) + tst \tmp, \tmp2 + mov \tmp2, #(HCR_EL2_RW_AARCH64 | HCR_EL2_HCD_DIS) + orr \tmp, \tmp2, #(HCR_EL2_APK | HCR_EL2_API) + csel\tmp, \tmp2, \tmp, eq msr hcr_el2, \tmp /* Return to the EL1_SP1 mode from EL2 */ diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h index 8b3a54e64c..77aa18909e 100644 --- a/arch/arm/include/asm/system.h +++ b/arch/arm/include/asm/system.h @@ -75,10 +75,25 @@ /* * HCR_EL2 bits definitions */ +#define HCR_EL2_API(1 << 41) /* Trap pointer authentication +instructions */ +#define HCR_EL2_APK(1 << 40) /* Trap pointer authentication +key access */ #define HCR_EL2_RW_AARCH64 (1 << 31) /* EL1 is AArch64 */ #define HCR_EL2_RW_AARCH32 (0 << 31) /* Lower levels are AArch32 */ #define HCR_EL2_HCD_DIS(1 << 29) /* Hypervisor Call disabled */ +/* + * ID_AA64ISAR1_EL1 bits definitions + */ +#define ID_AA64ISAR1_EL1_GPI (0xF << 28) /* Implementation-defined generic + code auth algorithm*/ +#define ID_AA64ISAR1_EL1_GPA (0xF << 24) /* QARMA generic code auth + algorithm */ +#define ID_AA64ISAR1_EL1_API (0xF << 8) /* Implementation-defined address + auth algorithm */ +#define ID_AA64ISAR1_EL1_APA (0xF << 4) /* QARMA address auth algorithm */ + /* * ID_AA64PFR0_EL1 bits definitions */ -- 2.25.1
[PATCH v2 0/6] Armv8r64 + BASER_FVP board support
From: Peter Hoyes
Add support for the Armv8r64 architecture and the BASER_FVP, which uses
the Armv8r64 architecture.
The Armv8r64 architecture has the following features:
* No non-secure exception levels
* Highest exception level is always S-EL2
* There is only a PMSA at S-EL2, which requires new MPU initialization
logic
* The VMSA at S-EL1 may not be enabled by default
* We boot Linux at S-EL1, so a mechanism is required to boot other
systems (e.g. Xen) at S-EL2
The BASER_FVP board config is implemented on top of the BASE_FVP and
Juno "VExpress" board configs.
The Armv8-R64 architecture reference manual supplement can be found at
https://developer.arm.com/documentation/ddi0600/latest/
Peter Hoyes (6):
armv8: Disable pointer authentication traps for EL1
armv8: Ensure EL1&0 VMSA is enabled
armv8: Add ARMv8 MPU configuration logic
vexpress64: Add BASER_FVP vexpress board variant
arm: Use armv8_switch_to_el1 env to switch to EL1
doc: Add documentation for the Arm vexpress board configs
arch/arm/Kconfig | 7 ++
arch/arm/cpu/armv8/cache_v8.c | 96 ++-
arch/arm/cpu/armv8/fsl-layerscape/spintable.S | 2 +-
arch/arm/cpu/armv8/transition.S | 2 +-
arch/arm/include/asm/armv8/mpu.h | 61
arch/arm/include/asm/macro.h | 28 +-
arch/arm/include/asm/system.h | 39
arch/arm/lib/bootm.c | 40 +---
board/armltd/vexpress64/Kconfig | 5 +-
board/armltd/vexpress64/vexpress64.c | 22 +
configs/vexpress_aemv8r_defconfig | 18
doc/README.semihosting| 2 +-
doc/arch/arm64.rst| 3 +-
doc/board/armltd/index.rst| 9 ++
doc/board/armltd/vexpress64.rst | 57 +++
doc/board/index.rst | 1 +
.../{vexpress_aemv8a.h => vexpress_aemv8.h} | 63
17 files changed, 408 insertions(+), 47 deletions(-)
create mode 100644 arch/arm/include/asm/armv8/mpu.h
create mode 100644 configs/vexpress_aemv8r_defconfig
create mode 100644 doc/board/armltd/index.rst
create mode 100644 doc/board/armltd/vexpress64.rst
rename include/configs/{vexpress_aemv8a.h => vexpress_aemv8.h} (83%)
--
2.25.1
Re: [PATCH 1/2] armv8: Disable pointer authentication traps for EL1
On 8/13/21 1:22 PM, Peter Hoyes wrote: Hi, From: Peter Hoyes The use of ARMv8.3 pointer authentication (PAuth) is governed by fields in HCR_EL2, which trigger a 'trap to EL2' if not enabled. The reset value of these fields is 'architecturally unknown' so we must ensure that the fields are enabled (to disable the traps) if we are entering the kernel at EL1. The APK field disables PAuth instruction traps and the API field disables PAuth register traps Add code to disable the traps in armv8_switch_to_el1_m. Prior to doing so, it checks fields in the ID_AA64ISAR1_EL1 register to ensure pointer authentication is supported by the hardware. The runtime checks require a second temporary register, so add this to the EL1 transition macro signature and update 2 call sites. Signed-off-by: Peter Hoyes --- arch/arm/cpu/armv8/fsl-layerscape/spintable.S | 2 +- arch/arm/cpu/armv8/transition.S | 2 +- arch/arm/include/asm/macro.h | 11 +-- arch/arm/include/asm/system.h | 15 +++ 4 files changed, 26 insertions(+), 4 deletions(-) diff --git a/arch/arm/cpu/armv8/fsl-layerscape/spintable.S b/arch/arm/cpu/armv8/fsl-layerscape/spintable.S index 363ded03e6..d6bd188459 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/spintable.S +++ b/arch/arm/cpu/armv8/fsl-layerscape/spintable.S @@ -93,7 +93,7 @@ __secondary_boot_func: 4: #ifdef CONFIG_ARMV8_SWITCH_TO_EL1 switch_el x7, _dead_loop, 0f, _dead_loop -0: armv8_switch_to_el1_m x4, x6, x7 +0: armv8_switch_to_el1_m x4, x6, x7, x9 #else switch_el x7, 0f, _dead_loop, _dead_loop 0:armv8_switch_to_el2_m x4, x6, x7 diff --git a/arch/arm/cpu/armv8/transition.S b/arch/arm/cpu/armv8/transition.S index a31af4ffc8..9dbdff3a4f 100644 --- a/arch/arm/cpu/armv8/transition.S +++ b/arch/arm/cpu/armv8/transition.S @@ -40,7 +40,7 @@ ENTRY(armv8_switch_to_el1) * now, jump to the address saved in x4. */ br x4 -1: armv8_switch_to_el1_m x4, x5, x6 +1: armv8_switch_to_el1_m x4, x5, x6, x7 ENDPROC(armv8_switch_to_el1) .popsection diff --git a/arch/arm/include/asm/macro.h b/arch/arm/include/asm/macro.h index 485310d660..e1eefc283f 100644 --- a/arch/arm/include/asm/macro.h +++ b/arch/arm/include/asm/macro.h @@ -256,7 +256,7 @@ lr .reqx30 * For loading 64-bit OS, x0 is physical address to the FDT blob. * They will be passed to the guest. */ -.macro armv8_switch_to_el1_m, ep, flag, tmp +.macro armv8_switch_to_el1_m, ep, flag, tmp, tmp2 /* Initialize Generic Timers */ mrs \tmp, cnthctl_el2 /* Enable EL1 access to timers */ @@ -306,7 +306,14 @@ lr .reqx30 b.eq1f /* Initialize HCR_EL2 */ - ldr \tmp, =(HCR_EL2_RW_AARCH64 | HCR_EL2_HCD_DIS) + /* Only disable PAuth traps if PAuth is supported */ + mrs \tmp, id_aa64isar1_el1 + ldr \tmp2, =(ID_AA64ISAR1_EL1_GPI | ID_AA64ISAR1_EL1_GPA | \ + ID_AA64ISAR1_EL1_API | ID_AA64ISAR1_EL1_APA) So I don't know how people feel about this, but we can drop the additional register at the cost of a bit more code (splitting the test in two) and some shifting. But actually I think we have reached the point where this should still be a macro. Are there are reasons this cannot be a function? We seem to end with an eret, so the content of LR should not matter? Cheers, Andre + tst \tmp, \tmp2 + mov \tmp2, #(HCR_EL2_RW_AARCH64 | HCR_EL2_HCD_DIS) + orr \tmp, \tmp2, #(HCR_EL2_APK | HCR_EL2_API) + csel\tmp, \tmp2, \tmp, eq msr hcr_el2, \tmp /* Return to the EL1_SP1 mode from EL2 */ diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h index 8b3a54e64c..77aa18909e 100644 --- a/arch/arm/include/asm/system.h +++ b/arch/arm/include/asm/system.h @@ -75,10 +75,25 @@ /* * HCR_EL2 bits definitions */ +#define HCR_EL2_API(1 << 41) /* Trap pointer authentication +instructions */ +#define HCR_EL2_APK(1 << 40) /* Trap pointer authentication +key access */ #define HCR_EL2_RW_AARCH64(1 << 31) /* EL1 is AArch64 */ #define HCR_EL2_RW_AARCH32(0 << 31) /* Lower levels are AArch32 */ #define HCR_EL2_HCD_DIS (1 << 29) /* Hypervisor Call disabled */ +/* + * ID_AA64ISAR1_EL1 bits definitions + */ +#define ID_AA64ISAR1_EL1_GPI (0xF << 28) /* Implementation-defined generic + code auth algorithm*/ +#define ID_AA64ISAR1_EL1_GPA (0xF << 24) /* QARMA generic code auth + algorithm */ +#define ID_AA64ISAR1_EL1_API (0xF << 8) /* Implementation-defined address +
[PATCH] board: atmel: sama7g5ek: avoid rewriting of configured CONFIG_BOOTCOMMAND
Rewrite the CONFIG_BOOTCOMMAND only if it's not previously configured from defconfig file. This allows the user to select from defconfig/menuconfig the desired boot command. Adjust the current board defconfigs to reflect the default booting command for the specific ENV configuration. Signed-off-by: Eugen Hristev --- configs/sama7g5ek_mmc1_defconfig | 2 ++ configs/sama7g5ek_mmc_defconfig | 2 ++ include/configs/sama7g5ek.h | 6 +- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/configs/sama7g5ek_mmc1_defconfig b/configs/sama7g5ek_mmc1_defconfig index e076e07e11..a78adf0dd8 100644 --- a/configs/sama7g5ek_mmc1_defconfig +++ b/configs/sama7g5ek_mmc1_defconfig @@ -18,6 +18,8 @@ CONFIG_FIT=y CONFIG_SD_BOOT=y CONFIG_USE_BOOTARGS=y CONFIG_BOOTARGS="console=ttyS0,115200 root=/dev/mmcblk1p2 rw rootwait" +CONFIG_USE_BOOTCOMMAND=y +CONFIG_BOOTCOMMAND="fatload mmc 1:1 0x6100 at91-sama7g5ek.dtb; fatload mmc 1:1 0x6200 zImage; bootz 0x6200 - 0x6100" CONFIG_MISC_INIT_R=y CONFIG_HUSH_PARSER=y CONFIG_CMD_BOOTZ=y diff --git a/configs/sama7g5ek_mmc_defconfig b/configs/sama7g5ek_mmc_defconfig index 96549c23f8..de845178e6 100644 --- a/configs/sama7g5ek_mmc_defconfig +++ b/configs/sama7g5ek_mmc_defconfig @@ -18,6 +18,8 @@ CONFIG_FIT=y CONFIG_SD_BOOT=y CONFIG_USE_BOOTARGS=y CONFIG_BOOTARGS="console=ttyS0,115200 root=/dev/mmcblk0p2 rw rootwait" +CONFIG_USE_BOOTCOMMAND=y +CONFIG_BOOTCOMMAND="fatload mmc 0:1 0x6100 at91-sama7g5ek.dtb; fatload mmc 0:1 0x6200 zImage; bootz 0x6200 - 0x6100" CONFIG_MISC_INIT_R=y CONFIG_HUSH_PARSER=y CONFIG_CMD_BOOTZ=y diff --git a/include/configs/sama7g5ek.h b/include/configs/sama7g5ek.h index 96db82e9d4..07e14ff507 100644 --- a/include/configs/sama7g5ek.h +++ b/include/configs/sama7g5ek.h @@ -26,7 +26,7 @@ #define CONFIG_SYS_LOAD_ADDR 0x6200 /* load address */ -#undef CONFIG_BOOTCOMMAND +#ifndef CONFIG_BOOTCOMMAND #ifdef CONFIG_SD_BOOT /* u-boot env in sd/mmc card */ @@ -34,6 +34,10 @@ #define CONFIG_BOOTCOMMAND "fatload mmc " CONFIG_ENV_FAT_DEVICE_AND_PART " 0x6100 at91-sama7g5ek.dtb; " \ "fatload mmc " CONFIG_ENV_FAT_DEVICE_AND_PART " 0x6200 zImage; " \ "bootz 0x6200 - 0x6100" +#else +#define CONFIG_BOOTCOMMAND "Place your bootcommand here" +#endif + #endif /* Size of malloc() pool */ -- 2.25.1
Re: [PATCH v6 10/12] watchdog: add gpio watchdog driver
Dear Simon, In message you wrote: > > - programming errors > - security errors where user input is insufficiently checked > > IMO the former should not be present if you have sufficient tests and > trying to catch them in the field at runtime is not very kind to your > users. Wow. I think I'll add this to my signature database: | "Trying to catch [programming errors] in the field at runtime is not | very kind to your users." | | - Simon Glass in Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de It is wrong always, everywhere and for everyone to believe anything upon insufficient evidence. - W. K. Clifford, British philosopher, circa 1876
Re: [PATCH v6 10/12] watchdog: add gpio watchdog driver
Hi,
On Thu, 19 Aug 2021 at 08:16, Wolfgang Denk wrote:
>
> Dear Tom,
>
> In message <20210819130806.GW858@bill-the-cat> you wrote:
> >
> > > So we have now a policy to wave through code, and ask others to
> > > clean it up later? That's ... sad.
> >
> > No, we continue to have the policy of expecting reviewers to follow the
> > whole discussion and relevant subsystems.
>
> Once upon a time there has also been a policy that if a function
> might return error codes, these need to be checked and handled.
>
> > Changing _every_ caller of dev_get_priv to check for NULL and
> > then, what? is clearly not the right answer.
Note that we should not check these calls, as the priv data is
allocated by driver model and a device cannot be probed until it gets
past this step.
I know that sometimes people add this check, but whenever I see it I
ask them to remove it. It is pointless and confusing, since it
suggests that driver model may not have allocated it yet. This sort of
confusion can really make things hard to understand for new people.
>
> Then what is the right answer in your opinion?
If a device is probed, you can rely on the priv data being set up. The
only way to access a probed device is via the device-internal.h or
uclass-internal.h APIs. Be careful with those if you must use them.
>
> I mean, look at the implementation of dev_get_priv():
>
> 628 void *dev_get_priv(const struct udevice *dev)
> 629 {
> 630 if (!dev) {
> 631 dm_warn("%s: null device\n", __func__);
> 632 return NULL;
> 633 }
> 634
> 635 return dm_priv_to_rw(dev->priv_);
> 636 }
>
> If there is guaranteed no way that dev_get_priv() can return a NULL
> pointer, that means that it must be guaranteed that the "dev"
> argument can never be a NULL pointer, either. So why do we check it
> at all?
>
> The same applies for all functions in "drivers/core/device.c" - they
> all check for valid input parameters, like any code should do.
I think did a series on this many years ago - a way to turn on checks
for this and that the right struct is obtained when you call
dev_get_priv(), etc. We could certainly add this with a CONFIG option
for debugging purposes. The runtime cost is actually not terrible but
it does require collusion with malloc() to be efficient.
>
> > If you think you see a problem here please go audit the DM code
> > itself more and propose some changes.
>
> I can see that the DM code itself implements proper error checking
> and reporting; it's the callers where negligent code prevails.
>
>
> If you are consequent, you must decide what you want:
>
> - Either we want robust and reliable code - then we have to handle
> the error codes which functions like dev_get_priv() etc. return.
>
> - Or you don't care about software quality, then we can omit such
> handling, but then it would also be consequent to remove all the
> error checking from "drivers/core/device.c" etc. - hey, that would
> even save a few hundred bytes of code size.
>
>
> Sugarcoating code which fails to handle error codes because "these
> errors can never happen" does not seem to be a clever approach to
> software engineering to me.
>
>
> I stop here. You know my opinion. You are the maintainer.
There is a wider issue here about arrg checking. We sometimes use
assert but at present that only appears in the code if DEBUG is
enabled. Also it just halts.
OTOH if we add arg checking everywhere it cluttles the code and can
substantially increase the size (I know of a project where it doubled
the size). I like to distinguish between:
- programming errors
- security errors where user input is insufficiently checked
IMO the former should not be present if you have sufficient tests and
trying to catch them in the field at runtime is not very kind to your
users.
Regards,
Simon
Re: [PATCH 00/28] Initial implementation of bootmethod/bootflow
Hi Tom, On Thu, 19 Aug 2021 at 07:59, Tom Rini wrote: > > On Wed, Aug 18, 2021 at 09:45:33PM -0600, Simon Glass wrote: > > > Bootmethod and bootflow provide a built-in way for U-Boot to automatically > > boot > > an Operating System without custom scripting and other customisation: > > > > - bootmethod - a method to scan a device to find bootflows (owned by > > U-Boot) > > - bootflow - a description of how to boot (owned by the distro) > > > > This series provides an initial implementation of these, enable to scan > > for bootflows from MMC and Ethernet. The only bootflow supported is > > distro boot, i.e. an extlinux.conf file included on a filesystem or > > tftp server. It works similiarly to the existing script-based approach, > > but is native to U-Boot. > > > > With this we can boot on a Raspberry Pi 3 with just one command: > > > >bootflow scan -lb > > > > which means to scan, listing (-l) each bootflow and trying to boot each > > one (-b). The final patch shows this. > > > > It is intended that this approach be expanded to support mechanisms other > > than distro boot, including EFI-related ones. With a standard way to > > identify boot devices, these features become easier. It also should > > support U-Boot scripts, for backwards compatibility only. > > > > The first patch of this series moves boot-related code out of common/ and > > into a new boot/ directory. This helps to collect these related files > > in one place, as common/ is quite large. > > > > Like sysboot, this feature makes use of the existing PXE implementation. > > Much of this series consists of cleaning up that code and refactoring it > > into something closer to a module that can be called, teasing apart its > > reliance on the command-line interpreter to access filesystems and the > > like. Also it now uses function arguments and its own context struct > > internally rather than environment variables, which is very hard to > > follow. No core functional change is included in the included PXE patches. > > > > For documentation, see the 'doc' patch. > > > > There is quite a long list of future work included in the documentation. > > One question is the choice of naming. Since this is a bootloader, should > > we just call this a 'method' and a 'flow' ? The 'boot' prefix is already > > shared by other commands like bootm, booti, etc. > > > > The design is described here: > > > > https://drive.google.com/file/d/1ggW0KJpUOR__vBkj3l61L2dav4ZkNC12/view?usp=sharing > > > > The series is available at u-boot-dm/bmea-working > > My question / concern is this. Would the next step here be to > implement the generic UEFI boot path? Today, I can write Fedora 34 for > AArch64 to a USB stick, boot U-Boot off of uSD card and the installer > automatically boots. I'm sure I could do the same with the BSDs. > Reading the documentation left me with the impression that every OSV > would be expected to write something, so that their installer / OS boot. > But there's already standards for that, which they do, and we should be > implementing (and do, via the current distro_boot) or making easier to > enable. Thanks! Here you are talking about scanning for a bootflow. It is not actually OS-specific. If it were, there would be no point to this :-) If you look in the distro scripts you will see 'scan_dev_for_efi' (and also scan_dev_for_scrips). At least the first needs to be implemented a bit like the distro boot is at present. So far I have only implemented scan_dev_for_extlinux (plus pxe) as it is enough to show the concept. Adding EFI it's likely to be about the same amount of code as distro.c at present, perhaps a little less since we don't have the network case. It is used by Fedora 34, I believe, so is easy enough for me to do. But I wanted to get something out as the concept is visible in this series. Regards, Simon
Re: [PATCH v6 10/12] watchdog: add gpio watchdog driver
Dear Tom,
In message <20210819130806.GW858@bill-the-cat> you wrote:
>
> > So we have now a policy to wave through code, and ask others to
> > clean it up later? That's ... sad.
>
> No, we continue to have the policy of expecting reviewers to follow the
> whole discussion and relevant subsystems.
Once upon a time there has also been a policy that if a function
might return error codes, these need to be checked and handled.
> Changing _every_ caller of dev_get_priv to check for NULL and
> then, what? is clearly not the right answer.
Then what is the right answer in your opinion?
I mean, look at the implementation of dev_get_priv():
628 void *dev_get_priv(const struct udevice *dev)
629 {
630 if (!dev) {
631 dm_warn("%s: null device\n", __func__);
632 return NULL;
633 }
634
635 return dm_priv_to_rw(dev->priv_);
636 }
If there is guaranteed no way that dev_get_priv() can return a NULL
pointer, that means that it must be guaranteed that the "dev"
argument can never be a NULL pointer, either. So why do we check it
at all?
The same applies for all functions in "drivers/core/device.c" - they
all check for valid input parameters, like any code should do.
> If you think you see a problem here please go audit the DM code
> itself more and propose some changes.
I can see that the DM code itself implements proper error checking
and reporting; it's the callers where negligent code prevails.
If you are consequent, you must decide what you want:
- Either we want robust and reliable code - then we have to handle
the error codes which functions like dev_get_priv() etc. return.
- Or you don't care about software quality, then we can omit such
handling, but then it would also be consequent to remove all the
error checking from "drivers/core/device.c" etc. - hey, that would
even save a few hundred bytes of code size.
Sugarcoating code which fails to handle error codes because "these
errors can never happen" does not seem to be a clever approach to
software engineering to me.
I stop here. You know my opinion. You are the maintainer.
Wolfgang Denk
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
A witty saying proves nothing, but saying something pointless gets
people's attention.
Re: [PULL] u-boot-riscv/master
On Thu, Aug 19, 2021 at 04:56:39PM +0800, Leo Liang wrote: > Hi Tom, > > The following changes since commit a0da2dda4ed9d0aee5265e9cd8876734f9f80e09: > > Prepare v2021.10-rc2 (2021-08-16 14:18:45 -0400) > > are available in the Git repository at: > > g...@source.denx.de:u-boot/custodians/u-boot-riscv.git > > for you to fetch changes up to 47d73ba4f4a40f17622d93f96b48e285b73c3061: > > board: sifive: overwrite board_fdt_blob_setup in u-boot proper (2021-08-17 > 19:28:37 +0800) > > CI result shows no issue: > https://source.denx.de/u-boot/custodians/u-boot-riscv/-/pipelines/8749 > Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH 00/28] Initial implementation of bootmethod/bootflow
On Wed, Aug 18, 2021 at 09:45:33PM -0600, Simon Glass wrote: > Bootmethod and bootflow provide a built-in way for U-Boot to automatically > boot > an Operating System without custom scripting and other customisation: > > - bootmethod - a method to scan a device to find bootflows (owned by U-Boot) > - bootflow - a description of how to boot (owned by the distro) > > This series provides an initial implementation of these, enable to scan > for bootflows from MMC and Ethernet. The only bootflow supported is > distro boot, i.e. an extlinux.conf file included on a filesystem or > tftp server. It works similiarly to the existing script-based approach, > but is native to U-Boot. > > With this we can boot on a Raspberry Pi 3 with just one command: > >bootflow scan -lb > > which means to scan, listing (-l) each bootflow and trying to boot each > one (-b). The final patch shows this. > > It is intended that this approach be expanded to support mechanisms other > than distro boot, including EFI-related ones. With a standard way to > identify boot devices, these features become easier. It also should > support U-Boot scripts, for backwards compatibility only. > > The first patch of this series moves boot-related code out of common/ and > into a new boot/ directory. This helps to collect these related files > in one place, as common/ is quite large. > > Like sysboot, this feature makes use of the existing PXE implementation. > Much of this series consists of cleaning up that code and refactoring it > into something closer to a module that can be called, teasing apart its > reliance on the command-line interpreter to access filesystems and the > like. Also it now uses function arguments and its own context struct > internally rather than environment variables, which is very hard to > follow. No core functional change is included in the included PXE patches. > > For documentation, see the 'doc' patch. > > There is quite a long list of future work included in the documentation. > One question is the choice of naming. Since this is a bootloader, should > we just call this a 'method' and a 'flow' ? The 'boot' prefix is already > shared by other commands like bootm, booti, etc. > > The design is described here: > > https://drive.google.com/file/d/1ggW0KJpUOR__vBkj3l61L2dav4ZkNC12/view?usp=sharing > > The series is available at u-boot-dm/bmea-working My question / concern is this. Would the next step here be to implement the generic UEFI boot path? Today, I can write Fedora 34 for AArch64 to a USB stick, boot U-Boot off of uSD card and the installer automatically boots. I'm sure I could do the same with the BSDs. Reading the documentation left me with the impression that every OSV would be expected to write something, so that their installer / OS boot. But there's already standards for that, which they do, and we should be implementing (and do, via the current distro_boot) or making easier to enable. Thanks! -- Tom signature.asc Description: PGP signature
[PATCH] xilinx: zynqmp: Enable gpio-key/button driver
Enable button uclass and also gpio-key driver by default. Signed-off-by: Michal Simek --- configs/xilinx_zynqmp_virt_defconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/configs/xilinx_zynqmp_virt_defconfig b/configs/xilinx_zynqmp_virt_defconfig index ea65563a1f2d..f516ab62d3c8 100644 --- a/configs/xilinx_zynqmp_virt_defconfig +++ b/configs/xilinx_zynqmp_virt_defconfig @@ -89,6 +89,8 @@ CONFIG_NETCONSOLE=y CONFIG_SPL_DM_SEQ_ALIAS=y CONFIG_SCSI_AHCI=y CONFIG_SATA_CEVA=y +CONFIG_BUTTON=y +CONFIG_BUTTON_GPIO=y CONFIG_CLK_ZYNQMP=y CONFIG_DFU_TFTP=y CONFIG_DFU_TIMEOUT=y -- 2.32.0
[PATCH] Revert "configs: synquacer: Make U-Boot binary position independent"
This reverts commit f7e16bb0c5362c9b01d7e6e96bf6c77fd6b3d89e, since the U-Boot doesn't boot if it is booted directly from SPI-NOR with CONFIG_POSITION_INDEPENDENT=y. Unless fixing this issue, it is better to revert this change. Signed-off-by: Masami Hiramatsu --- configs/synquacer_developerbox_defconfig |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/configs/synquacer_developerbox_defconfig b/configs/synquacer_developerbox_defconfig index 13736a4f03..338eb9b288 100644 --- a/configs/synquacer_developerbox_defconfig +++ b/configs/synquacer_developerbox_defconfig @@ -1,7 +1,6 @@ CONFIG_ARM=y -CONFIG_POSITION_INDEPENDENT=y CONFIG_ARCH_SYNQUACER=y -CONFIG_SYS_TEXT_BASE=0x +CONFIG_SYS_TEXT_BASE=0x0820 CONFIG_ENV_SIZE=0x3 CONFIG_ENV_OFFSET=0x30 CONFIG_ENV_SECT_SIZE=0x1
Re: [PATCH v6 10/12] watchdog: add gpio watchdog driver
On Thu, Aug 19, 2021 at 03:03:46PM +0200, Wolfgang Denk wrote: > Dear Tom, > > In message <20210819123540.GV858@bill-the-cat> you wrote: > > > > Since I literally just sent this in another email you couldn't have seen > > yet, I'll repeat it here. Feel free to follow up to this with a series > > to further update things, Wolfgang. > > So we have now a policy to wave through code, and ask others to > clean it up later? That's ... sad. No, we continue to have the policy of expecting reviewers to follow the whole discussion and relevant subsystems. Changing _every_ caller of dev_get_priv to check for NULL and then, what? is clearly not the right answer. If you think you see a problem here please go audit the DM code itself more and propose some changes. -- Tom signature.asc Description: PGP signature
Re: [PATCH v6 10/12] watchdog: add gpio watchdog driver
Dear Tom, In message <20210819123540.GV858@bill-the-cat> you wrote: > > Since I literally just sent this in another email you couldn't have seen > yet, I'll repeat it here. Feel free to follow up to this with a series > to further update things, Wolfgang. So we have now a policy to wave through code, and ask others to clean it up later? That's ... sad. Wolfgang Denk -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de "Consistency requires you to be as ignorant today as you were a year ago." - Bernard Berenson
Re: [PATCH v6 10/12] watchdog: add gpio watchdog driver
On Thu, Aug 19, 2021 at 02:32:01PM +0200, Wolfgang Denk wrote:
> Dear Rasmus,
>
> In message <62540f7b-0e07-8759-8e12-125527c2e...@prevas.dk> you wrote:
> >
> > >> +static int gpio_wdt_reset(struct udevice *dev)
> > >> +{
> > >> +struct gpio_wdt_priv *priv = dev_get_priv(dev);
> > >> +
> > >> +priv->state = !priv->state;
> > >
> > > Potential NULL pointer dereference.
> >
> > No, no and no. If allocation of the (driver or uclass) private data
> > fails, the device probe would have failed, so this code can never get
> > called with such a struct udevice.
>
> Famous last words...
>
> > Perhaps try doing a
> >
> > git grep -10 -E 'dev_get(_uclass)?_priv'
> >
> > and see how many cases you can find where that is followed by a NULL check?
>
> The existence of bad code is not a justification to add more of it.
Since I literally just sent this in another email you couldn't have seen
yet, I'll repeat it here. Feel free to follow up to this with a series
to further update things, Wolfgang.
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH v5 09/12] watchdog: wdt-uclass.c: handle all DM watchdogs in watchdog_reset()
On Thu, Aug 19, 2021 at 01:10:54PM +0200, Wolfgang Denk wrote:
> Dear Rasmus,
>
> In message <4798abb5-07d9-fa88-931f-dbaff951e...@prevas.dk> you wrote:
> > >>
> > >> +ret = uclass_get(UCLASS_WDT, );
> > >> +if (ret) {
> > >> +log_debug("Error getting UCLASS_WDT: %d\n", ret);
> > >> +return 0;
> > >> +}
> > >
> > > Here the error goes silent, so we should fix the callers to report
> > > it.
> >
> > The caller (singular) is the initr sequence, so returning an error is
> > effectively the same as halting the boot process, and as I've already
> > explained, I'm not going to change the semantics of initr_watchdog in
> > this regard.
>
> In this case you must print an error message here.
>
> > Feel free to submit a patch if you feel a change in this area is in
> > order. That's completely unrelated to what these patches are trying to
> > achieve.
>
> You add new code here, so please make sure not to add known issues.
>
> > >> +uclass_foreach_dev(dev, uc) {
> > >> +ret = device_probe(dev);
> > >> +if (ret) {
> > >> +log_debug("Error probing %s: %d\n", dev->name,
> > >> ret);
> > >> +continue;
> > >> }
> > >
> > > Here the situation is different. The probing error is never
> > > reported anywhere. Is it really a normal condition that a
> > > device_probe() fails here?
> >
> > No, it is not a normal condition. I added the log_debug() after a
> > request from Simon.
>
> But log_debug() is nothing any user will see in the field. We need
> an error message here, too.
Wolfgang,
If you would like to come along afterwards with additional logging
changes, please do so. As Rasmus has pointed out numerous times at this
point, what he's doing is consistent with everything else in these
areas. Thanks.
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH v6 10/12] watchdog: add gpio watchdog driver
Dear Rasmus,
In message <62540f7b-0e07-8759-8e12-125527c2e...@prevas.dk> you wrote:
>
> >> +static int gpio_wdt_reset(struct udevice *dev)
> >> +{
> >> + struct gpio_wdt_priv *priv = dev_get_priv(dev);
> >> +
> >> + priv->state = !priv->state;
> >
> > Potential NULL pointer dereference.
>
> No, no and no. If allocation of the (driver or uclass) private data
> fails, the device probe would have failed, so this code can never get
> called with such a struct udevice.
Famous last words...
> Perhaps try doing a
>
> git grep -10 -E 'dev_get(_uclass)?_priv'
>
> and see how many cases you can find where that is followed by a NULL check?
The existence of bad code is not a justification to add more of it.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
No, I'm not going to explain it. If you can't figure it out, you
didn't want to know anyway... :-)
- Larry Wall in <1991aug7.180856.2...@netlabs.com>
Re: [PATCH v6 10/12] watchdog: add gpio watchdog driver
On 19/08/2021 13.46, Wolfgang Denk wrote:
> Dear Rasmus,
>
> again: error handling.
>
> In message <20210819095706.3585923-11-rasmus.villem...@prevas.dk> you wrote:
>>
>> diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c
>> new file mode 100644
>> index 00..982a66b3f9
>> --- /dev/null
>> +++ b/drivers/watchdog/gpio_wdt.c
>> @@ -0,0 +1,68 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +struct gpio_wdt_priv {
>> +struct gpio_desc gpio;
>> +bool always_running;
>> +int state;
>> +};
>> +
>> +static int gpio_wdt_reset(struct udevice *dev)
>> +{
>> +struct gpio_wdt_priv *priv = dev_get_priv(dev);
>> +
>> +priv->state = !priv->state;
>
> Potential NULL pointer dereference.
No, no and no. If allocation of the (driver or uclass) private data
fails, the device probe would have failed, so this code can never get
called with such a struct udevice.
Perhaps try doing a
git grep -10 -E 'dev_get(_uclass)?_priv'
and see how many cases you can find where that is followed by a NULL check?
Rasmus
Re: [PATCH v1 7/7] i2c: stm32f7: compute i2cclk only one time
HI Patrick
On 8/3/21 12:05 PM, Patrice Chotard wrote:
> From: Patrick Delaunay
>
> Compute i2cclk only one time in stm32_i2c_compute_timing()
> and remove setup parameter (accessible in i2c_priv).
>
> Signed-off-by: Patrick Delaunay
>
> Signed-off-by: Patrice Chotard
> ---
>
> drivers/i2c/stm32f7_i2c.c | 18 --
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c
> index 2b2dae67a3..c6ae65badb 100644
> --- a/drivers/i2c/stm32f7_i2c.c
> +++ b/drivers/i2c/stm32f7_i2c.c
> @@ -507,14 +507,13 @@ static int stm32_i2c_xfer(struct udevice *bus, struct
> i2c_msg *msg,
> return 0;
> }
>
> -static int stm32_i2c_compute_solutions(struct stm32_i2c_setup *setup,
> +static int stm32_i2c_compute_solutions(u32 i2cclk,
> +struct stm32_i2c_setup *setup,
> const struct stm32_i2c_spec *specs,
> struct list_head *solutions)
> {
> struct stm32_i2c_timings *v;
> u32 p_prev = STM32_PRESC_MAX;
> - u32 i2cclk = DIV_ROUND_CLOSEST(STM32_NSEC_PER_SEC,
> -setup->clock_src);
> u32 af_delay_min, af_delay_max;
> u16 p, l, a;
> int sdadel_min, sdadel_max, scldel_min;
> @@ -582,7 +581,8 @@ static int stm32_i2c_compute_solutions(struct
> stm32_i2c_setup *setup,
> return ret;
> }
>
> -static int stm32_i2c_choose_solution(struct stm32_i2c_setup *setup,
> +static int stm32_i2c_choose_solution(u32 i2cclk,
> + struct stm32_i2c_setup *setup,
>const struct stm32_i2c_spec *specs,
>struct list_head *solutions,
>struct stm32_i2c_timings *s)
> @@ -591,8 +591,6 @@ static int stm32_i2c_choose_solution(struct
> stm32_i2c_setup *setup,
> u32 i2cbus = DIV_ROUND_CLOSEST(STM32_NSEC_PER_SEC,
> setup->speed_freq);
> u32 clk_error_prev = i2cbus;
> - u32 i2cclk = DIV_ROUND_CLOSEST(STM32_NSEC_PER_SEC,
> -setup->clock_src);
> u32 clk_min, clk_max;
> u32 af_delay_min;
> u32 dnf_delay;
> @@ -679,9 +677,9 @@ static const struct stm32_i2c_spec *get_specs(u32 rate)
> }
>
> static int stm32_i2c_compute_timing(struct stm32_i2c_priv *i2c_priv,
> - struct stm32_i2c_setup *setup,
> struct stm32_i2c_timings *output)
> {
> + struct stm32_i2c_setup *setup = _priv->setup;
> const struct stm32_i2c_spec *specs;
> struct stm32_i2c_timings *v, *_v;
> struct list_head solutions;
> @@ -712,11 +710,11 @@ static int stm32_i2c_compute_timing(struct
> stm32_i2c_priv *i2c_priv,
> }
>
> INIT_LIST_HEAD();
> - ret = stm32_i2c_compute_solutions(setup, specs, );
> + ret = stm32_i2c_compute_solutions(i2cclk, setup, specs, );
> if (ret)
> goto exit;
>
> - ret = stm32_i2c_choose_solution(setup, specs, , output);
> + ret = stm32_i2c_choose_solution(i2cclk, setup, specs, ,
> output);
> if (ret)
> goto exit;
>
> @@ -761,7 +759,7 @@ static int stm32_i2c_setup_timing(struct stm32_i2c_priv
> *i2c_priv,
> }
>
> do {
> - ret = stm32_i2c_compute_timing(i2c_priv, setup, timing);
> + ret = stm32_i2c_compute_timing(i2c_priv, timing);
> if (ret) {
> log_debug("failed to compute I2C timings.\n");
> if (setup->speed_freq > I2C_SPEED_STANDARD_RATE) {
>
Reviewed-by: Patrice Chotard
Thanks
Patrice
Re: [PATCH v1 6/7] i2c: stm32f7: add support for DNF i2c-digital-filter binding
HI Patrick
On 8/3/21 12:05 PM, Patrice Chotard wrote:
> From: Patrick Delaunay
>
> Add the support for the i2c-digital-filter binding, allowing to enable
> the digital filter via the device-tree and indicate its value in the DT
>
> Signed-off-by: Patrick Delaunay
> Signed-off-by: Patrice Chotard
> ---
>
> drivers/i2c/stm32f7_i2c.c | 11 +--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c
> index 7e6c65fadc..2b2dae67a3 100644
> --- a/drivers/i2c/stm32f7_i2c.c
> +++ b/drivers/i2c/stm32f7_i2c.c
> @@ -107,7 +107,6 @@ struct stm32_i2c_regs {
>
> #define STM32_I2C_MAX_LEN0xff
>
> -#define STM32_I2C_DNF_DEFAULT0
> #define STM32_I2C_DNF_MAX15
>
> #define STM32_I2C_ANALOG_FILTER_DELAY_MIN50 /* ns */
> @@ -204,6 +203,7 @@ struct stm32_i2c_timings {
> * @regmap_sreg: register address for setting Fast Mode Plus bits
> * @regmap_creg: register address for clearing Fast Mode Plus bits
> * @regmap_mask: mask for Fast Mode Plus bits
> + * @dnf_dt: value of digital filter requested via dt
> */
> struct stm32_i2c_priv {
> struct stm32_i2c_regs *regs;
> @@ -214,6 +214,7 @@ struct stm32_i2c_priv {
> u32 regmap_sreg;
> u32 regmap_creg;
> u32 regmap_mask;
> + u32 dnf_dt;
> };
>
> static const struct stm32_i2c_spec i2c_specs[] = {
> @@ -684,6 +685,7 @@ static int stm32_i2c_compute_timing(struct stm32_i2c_priv
> *i2c_priv,
> const struct stm32_i2c_spec *specs;
> struct stm32_i2c_timings *v, *_v;
> struct list_head solutions;
> + u32 i2cclk = DIV_ROUND_CLOSEST(STM32_NSEC_PER_SEC, setup->clock_src);
> int ret;
>
> specs = get_specs(setup->speed_freq);
> @@ -701,6 +703,8 @@ static int stm32_i2c_compute_timing(struct stm32_i2c_priv
> *i2c_priv,
> return -EINVAL;
> }
>
> + /* Analog and Digital Filters */
> + setup->dnf = DIV_ROUND_CLOSEST(i2c_priv->dnf_dt, i2cclk);
> if (setup->dnf > STM32_I2C_DNF_MAX) {
> log_err("DNF out of bound %d/%d\n",
> setup->dnf, STM32_I2C_DNF_MAX);
> @@ -923,7 +927,10 @@ static int stm32_of_to_plat(struct udevice *dev)
> fall_time = dev_read_u32_default(dev, "i2c-scl-falling-time-ns",
>STM32_I2C_FALL_TIME_DEFAULT);
>
> - i2c_priv->setup.dnf = STM32_I2C_DNF_DEFAULT;
> + i2c_priv->dnf_dt = dev_read_u32_default(dev,
> "i2c-digital-filter-width-ns", 0);
> + if (!dev_read_bool(dev, "i2c-digital-filter"))
> + i2c_priv->dnf_dt = 0;
> +
> i2c_priv->setup.analog_filter = dev_read_bool(dev, "i2c-analog-filter");
>
> /* Optional */
>
Reviewed-by: Patrice Chotard
Thanks
Patrice
Re: [PATCH v1 5/7] i2c: stm32f7: fix configuration of the digital filter
HI Patrick
On 8/3/21 12:05 PM, Patrice Chotard wrote:
> From: Patrick Delaunay
>
> The digital filter related computation are present in the driver
> however the programming of the filter within the IP is missing.
> The maximum value for the DNF is wrong and should be 15 instead of 16.
>
> Signed-off-by: Patrick Delaunay
> Signed-off-by: Patrice Chotard
> ---
>
> drivers/i2c/stm32f7_i2c.c | 10 --
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c
> index e71a0e0aa3..7e6c65fadc 100644
> --- a/drivers/i2c/stm32f7_i2c.c
> +++ b/drivers/i2c/stm32f7_i2c.c
> @@ -45,6 +45,8 @@ struct stm32_i2c_regs {
>
> /* STM32 I2C control 1 */
> #define STM32_I2C_CR1_ANFOFF BIT(12)
> +#define STM32_I2C_CR1_DNF_MASK GENMASK(11, 8)
> +#define STM32_I2C_CR1_DNF(n) (((n) & 0xf) << 8)
> #define STM32_I2C_CR1_ERRIE BIT(7)
> #define STM32_I2C_CR1_TCIE BIT(6)
> #define STM32_I2C_CR1_STOPIE BIT(5)
> @@ -106,7 +108,7 @@ struct stm32_i2c_regs {
> #define STM32_I2C_MAX_LEN0xff
>
> #define STM32_I2C_DNF_DEFAULT0
> -#define STM32_I2C_DNF_MAX16
> +#define STM32_I2C_DNF_MAX15
>
> #define STM32_I2C_ANALOG_FILTER_DELAY_MIN50 /* ns */
> #define STM32_I2C_ANALOG_FILTER_DELAY_MAX260 /* ns */
> @@ -155,7 +157,7 @@ struct stm32_i2c_spec {
> * @clock_src: I2C clock source frequency (Hz)
> * @rise_time: Rise time (ns)
> * @fall_time: Fall time (ns)
> - * @dnf: Digital filter coefficient (0-16)
> + * @dnf: value of digital filter to apply
> * @analog_filter: Analog filter delay (On/Off)
> */
> struct stm32_i2c_setup {
> @@ -842,6 +844,10 @@ static int stm32_i2c_hw_config(struct stm32_i2c_priv
> *i2c_priv)
> else
> setbits_le32(>cr1, STM32_I2C_CR1_ANFOFF);
>
> + /* Program the Digital Filter */
> + clrsetbits_le32(>cr1, STM32_I2C_CR1_DNF_MASK,
> + STM32_I2C_CR1_DNF(i2c_priv->setup.dnf));
> +
> setbits_le32(>cr1, STM32_I2C_CR1_PE);
>
> return 0;
>
Reviewed-by: Patrice Chotard
Thanks
Patrice
Re: [PATCH v1 4/7] i2c: stm32f7: support DT binding i2c-analog-filter
Hi Patrick
On 8/3/21 12:05 PM, Patrice Chotard wrote:
> From: Patrick Delaunay
>
> Replace driver internally coded enabling/disabling of the
> analog-filter with the DT binding "i2c-analog-filter".
>
> Signed-off-by: Patrick Delaunay
> Signed-off-by: Patrice Chotard
> ---
>
> drivers/i2c/stm32f7_i2c.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c
> index b449084b5f..e71a0e0aa3 100644
> --- a/drivers/i2c/stm32f7_i2c.c
> +++ b/drivers/i2c/stm32f7_i2c.c
> @@ -108,7 +108,6 @@ struct stm32_i2c_regs {
> #define STM32_I2C_DNF_DEFAULT0
> #define STM32_I2C_DNF_MAX16
>
> -#define STM32_I2C_ANALOG_FILTER_ENABLE 1
> #define STM32_I2C_ANALOG_FILTER_DELAY_MIN50 /* ns */
> #define STM32_I2C_ANALOG_FILTER_DELAY_MAX260 /* ns */
>
> @@ -919,7 +918,7 @@ static int stm32_of_to_plat(struct udevice *dev)
>STM32_I2C_FALL_TIME_DEFAULT);
>
> i2c_priv->setup.dnf = STM32_I2C_DNF_DEFAULT;
> - i2c_priv->setup.analog_filter = STM32_I2C_ANALOG_FILTER_ENABLE;
> + i2c_priv->setup.analog_filter = dev_read_bool(dev, "i2c-analog-filter");
>
> /* Optional */
> i2c_priv->regmap = syscon_regmap_lookup_by_phandle(dev,
>
Reviewed-by: Patrice Chotard
Thanks
Patrice
Re: [PATCH v1 3/7] arm: dts: stm32: Add i2c-analog-filter property in I2C nodes for stm32h743
Hi Patrick On 8/3/21 12:05 PM, Patrice Chotard wrote: > Add i2c-analog-filter property in I2C nodes to enable analog > filter feature. > > Signed-off-by: Patrice Chotard > --- > > arch/arm/dts/stm32h743.dtsi | 4 > 1 file changed, 4 insertions(+) > > diff --git a/arch/arm/dts/stm32h743.dtsi b/arch/arm/dts/stm32h743.dtsi > index ed6857512f..dbfebf07f2 100644 > --- a/arch/arm/dts/stm32h743.dtsi > +++ b/arch/arm/dts/stm32h743.dtsi > @@ -124,6 +124,7 @@ ><32>; > resets = < STM32H7_APB1L_RESET(I2C1)>; > clocks = < I2C1_CK>; > + i2c-analog-filter; > status = "disabled"; > }; > > @@ -136,6 +137,7 @@ ><34>; > resets = < STM32H7_APB1L_RESET(I2C2)>; > clocks = < I2C2_CK>; > + i2c-analog-filter; > status = "disabled"; > }; > > @@ -148,6 +150,7 @@ ><73>; > resets = < STM32H7_APB1L_RESET(I2C3)>; > clocks = < I2C3_CK>; > + i2c-analog-filter; > status = "disabled"; > }; > > @@ -395,6 +398,7 @@ ><96>; > resets = < STM32H7_APB4_RESET(I2C4)>; > clocks = < I2C4_CK>; > + i2c-analog-filter; > status = "disabled"; > }; > > Reviewed-by: Patrice Chotard Thanks Patrice
Re: [PATCH v1 2/7] arm: dts: stm32: Add i2c-analog-filter property in I2C nodes for stm32f746
Hi Patrick On 8/3/21 12:05 PM, Patrice Chotard wrote: > Add i2c-analog-filter property in I2C nodes to enable analog > filter feature. > > Signed-off-by: Patrice Chotard > --- > > arch/arm/dts/stm32f746.dtsi | 4 > 1 file changed, 4 insertions(+) > > diff --git a/arch/arm/dts/stm32f746.dtsi b/arch/arm/dts/stm32f746.dtsi > index ba9b3cd03c..78facde2b5 100644 > --- a/arch/arm/dts/stm32f746.dtsi > +++ b/arch/arm/dts/stm32f746.dtsi > @@ -313,6 +313,7 @@ > clocks = < 1 CLK_I2C1>; > #address-cells = <1>; > #size-cells = <0>; > + i2c-analog-filter; > status = "disabled"; > }; > > @@ -325,6 +326,7 @@ > clocks = < 1 CLK_I2C2>; > #address-cells = <1>; > #size-cells = <0>; > + i2c-analog-filter; > status = "disabled"; > }; > > @@ -337,6 +339,7 @@ > clocks = < 1 CLK_I2C3>; > #address-cells = <1>; > #size-cells = <0>; > + i2c-analog-filter; > status = "disabled"; > }; > > @@ -349,6 +352,7 @@ > clocks = < 1 CLK_I2C4>; > #address-cells = <1>; > #size-cells = <0>; > + i2c-analog-filter; > status = "disabled"; > }; > > Reviewed-by: Patrice Chotard Thanks Patrice
Re: [PATCH v1 1/7] i2c: stm32f7: move driver data of each instance in a privdata
HI Patrick
On 8/3/21 12:05 PM, Patrice Chotard wrote:
> From: Patrick Delaunay
>
> Today all the I2C instance point on the same global
> variable stm32_i2c_setup according the compatible: i2c_priv->setup =
> pointer to the same driver data.
>
> This patch changes this driver data (stm32f7_setup and stm32mp15_setup)
> to a const struct and move the timing struct 'setup' as element of i2c
> privdata, initialized in stm32_ofdata_to_platdata() with the driver
> configuration data.
>
> This patch solves issues when several I2C instance have not the same
> clock source or not the same configuration: each timing setup is saved
> is the I2C privdata.
>
> Signed-off-by: Patrick Delaunay
> Signed-off-by: Patrice Chotard
> ---
>
> drivers/i2c/stm32f7_i2c.c | 53 ---
> 1 file changed, 27 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c
> index 7b04a09de0..b449084b5f 100644
> --- a/drivers/i2c/stm32f7_i2c.c
> +++ b/drivers/i2c/stm32f7_i2c.c
> @@ -158,7 +158,6 @@ struct stm32_i2c_spec {
> * @fall_time: Fall time (ns)
> * @dnf: Digital filter coefficient (0-16)
> * @analog_filter: Analog filter delay (On/Off)
> - * @fmp_clr_offset: Fast Mode Plus clear register offset from set register
> */
> struct stm32_i2c_setup {
> u32 speed_freq;
> @@ -167,6 +166,13 @@ struct stm32_i2c_setup {
> u32 fall_time;
> u8 dnf;
> bool analog_filter;
> +};
> +
> +/**
> + * struct stm32_i2c_data - driver data for I2C configuration by compatible
> + * @fmp_clr_offset: Fast Mode Plus clear register offset from set register
> + */
> +struct stm32_i2c_data {
> u32 fmp_clr_offset;
> };
>
> @@ -201,7 +207,7 @@ struct stm32_i2c_timings {
> struct stm32_i2c_priv {
> struct stm32_i2c_regs *regs;
> struct clk clk;
> - struct stm32_i2c_setup *setup;
> + struct stm32_i2c_setup setup;
> u32 speed;
> struct regmap *regmap;
> u32 regmap_sreg;
> @@ -251,18 +257,11 @@ static const struct stm32_i2c_spec i2c_specs[] = {
> },
> };
>
> -static const struct stm32_i2c_setup stm32f7_setup = {
> - .rise_time = STM32_I2C_RISE_TIME_DEFAULT,
> - .fall_time = STM32_I2C_FALL_TIME_DEFAULT,
> - .dnf = STM32_I2C_DNF_DEFAULT,
> - .analog_filter = STM32_I2C_ANALOG_FILTER_ENABLE,
> +static const struct stm32_i2c_data stm32f7_data = {
> + .fmp_clr_offset = 0x00,
> };
>
> -static const struct stm32_i2c_setup stm32mp15_setup = {
> - .rise_time = STM32_I2C_RISE_TIME_DEFAULT,
> - .fall_time = STM32_I2C_FALL_TIME_DEFAULT,
> - .dnf = STM32_I2C_DNF_DEFAULT,
> - .analog_filter = STM32_I2C_ANALOG_FILTER_ENABLE,
> +static const struct stm32_i2c_data stm32mp15_data = {
> .fmp_clr_offset = 0x40,
> };
>
> @@ -745,7 +744,7 @@ static u32 get_lower_rate(u32 rate)
> static int stm32_i2c_setup_timing(struct stm32_i2c_priv *i2c_priv,
> struct stm32_i2c_timings *timing)
> {
> - struct stm32_i2c_setup *setup = i2c_priv->setup;
> + struct stm32_i2c_setup *setup = _priv->setup;
> int ret = 0;
>
> setup->speed_freq = i2c_priv->speed;
> @@ -839,10 +838,11 @@ static int stm32_i2c_hw_config(struct stm32_i2c_priv
> *i2c_priv)
> writel(timing, >timingr);
>
> /* Enable I2C */
> - if (i2c_priv->setup->analog_filter)
> + if (i2c_priv->setup.analog_filter)
> clrbits_le32(>cr1, STM32_I2C_CR1_ANFOFF);
> else
> setbits_le32(>cr1, STM32_I2C_CR1_ANFOFF);
> +
> setbits_le32(>cr1, STM32_I2C_CR1_PE);
>
> return 0;
> @@ -903,21 +903,23 @@ clk_free:
>
> static int stm32_of_to_plat(struct udevice *dev)
> {
> + const struct stm32_i2c_data *data;
> struct stm32_i2c_priv *i2c_priv = dev_get_priv(dev);
> u32 rise_time, fall_time;
> int ret;
>
> - i2c_priv->setup = (struct stm32_i2c_setup *)dev_get_driver_data(dev);
> - if (!i2c_priv->setup)
> + data = (const struct stm32_i2c_data *)dev_get_driver_data(dev);
> + if (!data)
> return -EINVAL;
>
> - rise_time = dev_read_u32_default(dev, "i2c-scl-rising-time-ns", 0);
> - if (rise_time)
> - i2c_priv->setup->rise_time = rise_time;
> + rise_time = dev_read_u32_default(dev, "i2c-scl-rising-time-ns",
> + STM32_I2C_RISE_TIME_DEFAULT);
> +
> + fall_time = dev_read_u32_default(dev, "i2c-scl-falling-time-ns",
> + STM32_I2C_FALL_TIME_DEFAULT);
>
> - fall_time = dev_read_u32_default(dev, "i2c-scl-falling-time-ns", 0);
> - if (fall_time)
> - i2c_priv->setup->fall_time = fall_time;
> + i2c_priv->setup.dnf = STM32_I2C_DNF_DEFAULT;
> + i2c_priv->setup.analog_filter = STM32_I2C_ANALOG_FILTER_ENABLE;
>
> /* Optional */
> i2c_priv->regmap = syscon_regmap_lookup_by_phandle(dev,
> @@ -930,8 +932,7 @@ static int stm32_of_to_plat(struct
Re: [PATCH v6 10/12] watchdog: add gpio watchdog driver
Dear Rasmus,
again: error handling.
In message <20210819095706.3585923-11-rasmus.villem...@prevas.dk> you wrote:
>
> diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c
> new file mode 100644
> index 00..982a66b3f9
> --- /dev/null
> +++ b/drivers/watchdog/gpio_wdt.c
> @@ -0,0 +1,68 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +#include
> +#include
> +#include
> +#include
> +
> +struct gpio_wdt_priv {
> + struct gpio_desc gpio;
> + bool always_running;
> + int state;
> +};
> +
> +static int gpio_wdt_reset(struct udevice *dev)
> +{
> + struct gpio_wdt_priv *priv = dev_get_priv(dev);
> +
> + priv->state = !priv->state;
Potential NULL pointer dereference.
> +static int gpio_wdt_start(struct udevice *dev, u64 timeout, ulong flags)
> +{
> + struct gpio_wdt_priv *priv = dev_get_priv(dev);
> +
> + if (priv->always_running)
> + return 0;
Ditto.
> +static int dm_probe(struct udevice *dev)
> +{
> + struct gpio_wdt_priv *priv = dev_get_priv(dev);
> + int ret;
> +
> + priv->always_running = dev_read_bool(dev, "always-running");
Ditto.
> + ret = gpio_request_by_name(dev, "gpios", 0, >gpio, GPIOD_IS_OUT);
> + if (ret < 0) {
> + dev_err(dev, "Request for wdt gpio failed: %d\n", ret);
> + return ret;
> + }
> +
> + if (priv->always_running)
> + ret = gpio_wdt_reset(dev);
Ditto.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Even if you aren't in doubt, consider the mental welfare of the per-
son who has to maintain the code after you, and who will probably put
parens in the wrong place. - Larry Wall in the perl man page
Re: [PATCH v6 09/12] watchdog: wdt-uclass.c: handle all DM watchdogs in watchdog_reset()
Dear Rasmus,
In message <20210819095706.3585923-10-rasmus.villem...@prevas.dk> you wrote:
>
> diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
> index 5b1c0df5d6..7570710c4d 100644
> --- a/drivers/watchdog/wdt-uclass.c
> +++ b/drivers/watchdog/wdt-uclass.c
> @@ -61,20 +61,24 @@ static void init_watchdog_dev(struct udevice *dev)
...
> + ret = uclass_get(UCLASS_WDT, );
> + if (ret) {
> + log_debug("Error getting UCLASS_WDT: %d\n", ret);
> + return 0;
> + }
> +
> + uclass_foreach_dev(dev, uc) {
> + ret = device_probe(dev);
> + if (ret) {
> + log_debug("Error probing %s: %d\n", dev->name, ret);
> + continue;
> }
As discussed - errors need to be shown to the user, and not only in
images with debugging enabled.
> @@ -182,22 +186,34 @@ void watchdog_reset(void)
> {
> struct wdt_priv *priv;
> struct udevice *dev;
> + struct uclass *uc;
> ulong now;
>
> /* Exit if GD is not ready or watchdog is not initialized yet */
> if (!gd || !(gd->flags & GD_FLG_WDT_READY))
> return;
>
> - dev = gd->watchdog_dev;
> - priv = dev_get_uclass_priv(dev);
> - if (!priv->running)
> + if (uclass_get(UCLASS_WDT, ))
> return;
Do I see this crrectly that you remove here the code which you just
added in patch 02 of this series?
Why not doing it right from the beginning?
> + uclass_foreach_dev(dev, uc) {
> + if (!device_active(dev))
> + continue;
> + priv = dev_get_uclass_priv(dev);
> + if (!priv->running)
> + continue;
Potential NULL pointer dereference.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
An Ada exception is when a routine gets in trouble and says
'Beam me up, Scotty'.
Re: [PATCH v6 07/12] watchdog: wdt-uclass.c: add wdt_stop_all() helper
Dear Rasmus,
again: error handling.
In message <20210819095706.3585923-8-rasmus.villem...@prevas.dk> you wrote:
>
> --- a/drivers/watchdog/wdt-uclass.c
> +++ b/drivers/watchdog/wdt-uclass.c
> @@ -116,6 +116,31 @@ int wdt_stop(struct udevice *dev)
> return ret;
> }
>
> +int wdt_stop_all(void)
> +{
> + struct wdt_priv *priv;
> + struct udevice *dev;
> + struct uclass *uc;
> + int ret, err;
> +
> + ret = uclass_get(UCLASS_WDT, );
> + if (ret)
> + return ret;
> +
> + uclass_foreach_dev(dev, uc) {
> + if (!device_active(dev))
> + continue;
> + priv = dev_get_uclass_priv(dev);
> + if (!priv->running)
> + continue;
Potential NULL pointer dereferencing.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
I paid too much for it, but its worth it.
Re: status="ok" treated as disabled by DM
On 14:03-20210819, Roger Quadros wrote: > Hi, > > If the device tree node has status="ok" then the u-boot DM will treat the > device as disabled. > There are still quite many devices using "ok" instead of "okay" or no status > and those devices will not be bound. > > What is the right fix? > 1) make u-boot DM to treat satus="ok" as enabled. > 2) fix device tree by changing "ok" to "okay". > > cheers, > -roger https://github.com/devicetree-org/dt-schema/blob/master/schemas/dt-core.yaml#L36 I believe, the correct fix is (2) change the dt to "okay". -- Regards, Nishanth Menon Key (0xDDB5849D1736249D)/Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
Re: [PATCH v6 05/12] watchdog: wdt-uclass.c: keep track of each device's running state
Dear Rasmus,
please check your patches for proper error handling.
In message <20210819095706.3585923-6-rasmus.villem...@prevas.dk> you wrote:
>
...
> diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
> index 0a1f43771c..358fc68e27 100644
> --- a/drivers/watchdog/wdt-uclass.c
> +++ b/drivers/watchdog/wdt-uclass.c
...
> @@ -86,8 +89,11 @@ int wdt_start(struct udevice *dev, u64 timeout_ms, ulong
> flags)
> return -ENOSYS;
>
> ret = ops->start(dev, timeout_ms, flags);
> - if (ret == 0)
> - gd->flags |= GD_FLG_WDT_READY;
> + if (ret == 0) {
> + struct wdt_priv *priv = dev_get_uclass_priv(dev);
> +
> + priv->running = true;
dev_get_uclass_priv() can return NULL, in which case you would be
dereferencing a NULL pointer...
> @@ -101,8 +107,11 @@ int wdt_stop(struct udevice *dev)
> return -ENOSYS;
>
> ret = ops->stop(dev);
> - if (ret == 0)
> - gd->flags &= ~GD_FLG_WDT_READY;
> + if (ret == 0) {
> + struct wdt_priv *priv = dev_get_uclass_priv(dev);
> +
> + priv->running = false;
Same here.
> @@ -156,6 +165,9 @@ void watchdog_reset(void)
>
> dev = gd->watchdog_dev;
> priv = dev_get_uclass_priv(dev);
> + if (!priv->running)
> + return;
> +
And here again.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
I used to be indecisive, now I'm not sure.
Re: [PATCH] configs: Layerscape: Remove the 'fdt_addr' env
Dear Priyanka, In message you wrote: > > I agree we cant add copyright for minor changes. > > But this file already contains Freescale (now merged into NXP) copyright. > Zhiqiang is updating to latest copyright text used by NXP(Freescale) with > proper year. > I hope this is fine. Hm... I cannot see this. > diff --git a/include/configs/ls1012afrdm.h b/include/configs/ls1012afrdm.h > index 2711f651d7..c7fdd10cf5 100644 > --- a/include/configs/ls1012afrdm.h > +++ b/include/configs/ls1012afrdm.h > @@ -1,6 +1,7 @@ > /* SPDX-License-Identifier: GPL-2.0+ */ > /* > * Copyright 2016 Freescale Semiconductor, Inc. > + * Copyright 2021 NXP > */ This is adding a NEW copyright entry - for deleting one line of data (not even code). > diff --git a/include/configs/ls1021atsn.h b/include/configs/ls1021atsn.h > index 58c2d97a32..97fd61f6f9 100644 > --- a/include/configs/ls1021atsn.h > +++ b/include/configs/ls1021atsn.h > @@ -1,5 +1,5 @@ > /* SPDX-License-Identifier: GPL-2.0 > - * Copyright 2016-2019 NXP Semiconductors > + * Copyright 2016-2019, 2021 NXP Semiconductors > * Copyright 2019 Vladimir Oltean > */ Same here, again for deleting one line of data. > diff --git a/include/configs/ls1021atwr.h b/include/configs/ls1021atwr.h > index ba308c514b..5be179a36b 100644 > --- a/include/configs/ls1021atwr.h > +++ b/include/configs/ls1021atwr.h > @@ -1,7 +1,7 @@ > /* SPDX-License-Identifier: GPL-2.0+ */ > /* > * Copyright 2014 Freescale Semiconductor, Inc. > - * Copyright 2019 NXP > + * Copyright 2019, 2021 NXP > */ Again, for deleting 2 lines of data. And so on. None of these changes has substance to claim a copyright for. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de "Plan to throw one away. You will anyway." - Fred Brooks, "The Mythical Man Month"
Re: [PATCH 05/16] am43xx: Drop non-DM_I2C code
On 19/08/21 8:42 am, Tom Rini wrote: > On this platform, we have DM_I2C and SPL_DM_I2C always enabled. > Remove legacy options. > > Cc: Lokesh Vutla > Signed-off-by: Tom Rini Acked-by: Lokesh Vutla Thanks and regards, Lokesh
[PATCH 09/10] xilinx: zynqmp: Generate different u-boot.itb for MULTI_DTB_FIT
When MULTI_DTB_FIT is enabled fit-dtb.blob fit image is created which
contain all DTBs listed by CONFIG_OF_LIST. And with DTB_RELESELECT there is
a need to handle it as one file with DTBs in it not as separate DTBs in
u-boot.its/itb.
That's why extend mkimage_fit_atf.sh to generate u-boot.itb correctly.
Signed-off-by: Michal Simek
---
arch/arm/mach-zynqmp/mkimage_fit_atf.sh | 47 +
1 file changed, 47 insertions(+)
diff --git a/arch/arm/mach-zynqmp/mkimage_fit_atf.sh
b/arch/arm/mach-zynqmp/mkimage_fit_atf.sh
index 592be7f67066..37106909f1ee 100755
--- a/arch/arm/mach-zynqmp/mkimage_fit_atf.sh
+++ b/arch/arm/mach-zynqmp/mkimage_fit_atf.sh
@@ -111,6 +111,51 @@ cat << __TEE
__TEE
fi
+MULTI_DTB=`awk '/CONFIG_MULTI_DTB_FIT / { print $3 }'
include/generated/autoconf.h`
+
+if [ $MULTI_DTB -eq 1 ]; then
+ cat << __FDT_IMAGE_EOF
+ fdt_1 {
+ description = "Multi DTB fit image";
+ data = /incbin/("fit-dtb.blob");
+ type = "flat_dt";
+ arch = "arm64";
+ compression = "none";
+ $DTB_LOAD
+ hash {
+ algo = "md5";
+ };
+ };
+ };
+ configurations {
+ default = "config_1";
+__FDT_IMAGE_EOF
+
+if [ ! -f $BL31 ]; then
+cat << __CONF_SECTION1_EOF
+ config_1 {
+ description = "Multi DTB without TF-A";
+ firmware = "uboot";
+ loadables = "fdt_1";
+ };
+__CONF_SECTION1_EOF
+else
+cat << __CONF_SECTION1_EOF
+ config_1 {
+ description = "Multi DTB with TF-A";
+ firmware = "atf";
+ loadables = "uboot", "fdt_1";
+ };
+__CONF_SECTION1_EOF
+fi
+
+cat << __ITS_EOF
+ };
+};
+__ITS_EOF
+
+else
+
DEFAULT=1
cnt=1
for dtname in $DT
@@ -181,3 +226,5 @@ cat << __ITS_EOF
};
};
__ITS_EOF
+
+fi
--
2.32.0
[PATCH 10/10] xilinx: common: Enabling generic function for DT reselection
U-Boot support board detection at run time and based on it change DT.
This feature is implemented for SOM Kria platforms which contain two
eeproms which contain information about SOM module and CC (Carrier card).
Full U-Boot starts with minimal DT file defined by
CONFIG_DEFAULT_DEVICE_TREE which is available in multi DTB fit image.
It is using default setup of board_name variable initializaed to
DEVICE_TREE which corresponds to CONFIG_DEFAULT_DEVICE_TREE option.
When DTB_RESELECT is enabled board_detection() is called. Keep it your mind
that this code is called before relocation. board_detection() is calling
xilinx_read_eeprom() which fills board_info (xilinx_board_description)
structure which are parsed in board_name_decode().
Based on DT configuration and amount of nvmemX aliases name of the board is
composed by concatenating CONFIG_SYS_BOARD "-" "-rev"
"-" "-rev" .
If CC is not present or more are available it keeps going.
When board name is composed and returned from board_name_decode() it is
assigned to board_name variable which is used by
board_fit_config_name_match() which is called via fdtdec_setup() when it
goes over config options in multi dtb FIT image.
>From practical point of view multi DTB image is key point here which has to
contain configs for detected combinations. Unfortunately as of now they
have to be full DTBs and DTBOs are not supported.
That's why configuration like:
config_X {
description = "zynqmp-board-cc";
fdt = "board", "cc";
};
needs to be squashed together with:
fdtoverlay -o zynqmp-board-cc -i arch/arm/dts/zynqmp-board.dtb \
arch/arm/dts/zynqmp-cc.dtbo
and only one dtb is in fit:
config_X {
description = "zynqmp-board-cc";
fdt = "board-cc";
};
For creating multi DTBs fit image use mkimage -E, e.g.:
mkimage -E -f all.its all.dtb
When DTB_RESELECT is enabled xilinx_read_eeprom() is called before
relocation and it uses calloc for getting a buffer. Because this is dynamic
memory it is not relocated that's why xilinx_read_eeprom() is called again
as the part of board_init(). This second read with calloc buffer placed in
proper position board_late_init_xilinx() can setup u-boot variables as
before.
Signed-off-by: Michal Simek
---
arch/arm/dts/zynqmp-sm-k26-revA.dts | 3 ++
board/xilinx/common/board.c | 84 ++---
2 files changed, 81 insertions(+), 6 deletions(-)
diff --git a/arch/arm/dts/zynqmp-sm-k26-revA.dts
b/arch/arm/dts/zynqmp-sm-k26-revA.dts
index e274100a9bc5..5f55df28f331 100644
--- a/arch/arm/dts/zynqmp-sm-k26-revA.dts
+++ b/arch/arm/dts/zynqmp-sm-k26-revA.dts
@@ -204,17 +204,20 @@
{
status = "okay";
+ u-boot,dm-pre-reloc;
clock-frequency = <40>;
scl-gpios = < 24 GPIO_ACTIVE_HIGH>;
sda-gpios = < 25 GPIO_ACTIVE_HIGH>;
eeprom: eeprom@50 { /* u46 - also at address 0x58 */
+ u-boot,dm-pre-reloc;
compatible = "st,24c64", "atmel,24c64"; /* st m24c64 */
reg = <0x50>;
/* WP pin EE_WP_EN connected to slg7x644092@68 */
};
eeprom_cc: eeprom@51 { /* required by spec - also at address 0x59 */
+ u-boot,dm-pre-reloc;
compatible = "st,24c64", "atmel,24c64"; /* st m24c64 */
reg = <0x51>;
};
diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
index 92874272893a..979df44306af 100644
--- a/board/xilinx/common/board.c
+++ b/board/xilinx/common/board.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include "fru.h"
@@ -188,12 +189,14 @@ static int xilinx_read_eeprom_fru(struct udevice *dev,
char *name,
goto end;
}
- printf("Xilinx I2C FRU format at %s:\n", name);
- fru_capture((unsigned long)fru_content);
- ret = fru_display(0);
- if (ret) {
- printf("FRU format decoding failed.\n");
- goto end;
+ if (gd->flags & GD_FLG_RELOC || (_DEBUG &&
CONFIG_IS_ENABLED(DTB_RESELECT))) {
+ printf("Xilinx I2C FRU format at %s:\n", name);
+ fru_capture((unsigned long)fru_content);
+ ret = fru_display(0);
+ if (ret) {
+ printf("FRU format decoding failed.\n");
+ goto end;
+ }
}
if (desc->header == EEPROM_HEADER_MAGIC) {
@@ -465,13 +468,82 @@ int print_cpuinfo(void)
#endif
#if CONFIG_IS_ENABLED(DTB_RESELECT)
+#define MAX_NAME_LENGTH50
+
char * __maybe_unused __weak board_name_decode(void)
{
+ char *board_local_name;
+ struct xilinx_board_description *desc;
+ int i, id;
+
+ board_local_name = calloc(1, MAX_NAME_LENGTH);
+ if (!board_info)
+ return NULL;
+
+ for (id = 0; id <= highest_id; id++) {
+ desc = _info[id];
+
+ /* No board description */
+ if (!desc)
+ goto
[PATCH 07/10] Makefile: Align fit-dtb.blob and u-boot.itb by 64bits for 64bit systems
Enabling MULTI_DTB_FIT and DTB_RESELECT can end up with multi DTBs in FIT image placed and aligned only by 32bits (4bytes). For 64bit systems there is 64bit (8bytes) alignment required. That's why make sure that fit-dtb.blob and u-boot.itb as our primary target images for Xilinx ZynqMP are all 64bit aligned. The patch is using CONFIG_PHYS_64BIT macro to identify 64bit systems (including 32bit systems with PAE). Signed-off-by: Michal Simek --- Makefile | 7 +++ 1 file changed, 7 insertions(+) diff --git a/Makefile b/Makefile index 269e353a28ad..1bbe95595efe 100644 --- a/Makefile +++ b/Makefile @@ -1169,6 +1169,10 @@ MKIMAGEFLAGS_fit-dtb.blob = -f auto -A $(ARCH) -T firmware -C none -O u-boot \ -a 0 -e 0 -E \ $(patsubst %,-b arch/$(ARCH)/dts/%.dtb,$(subst ",,$(CONFIG_OF_LIST))) -d /dev/null +ifeq ($(CONFIG_PHYS_64BIT),y) +MKIMAGEFLAGS_fit-dtb.blob += -B 0x8 +endif + ifneq ($(EXT_DTB),) u-boot-fit-dtb.bin: u-boot-nodtb.bin $(EXT_DTB) $(call if_changed,cat) @@ -1431,6 +1435,9 @@ MKIMAGEFLAGS_u-boot.itb = else MKIMAGEFLAGS_u-boot.itb = -E endif +ifeq ($(CONFIG_PHYS_64BIT),y) +MKIMAGEFLAGS_u-boot.itb += -B 0x8 +endif ifdef U_BOOT_ITS u-boot.itb: u-boot-nodtb.bin \ -- 2.32.0
[PATCH 08/10] arm64: dts: Make sure that all DTBs are 64bit aligned for 64bit systems
DTBs for 64bit systems should be also 64bit aligned. Signed-off-by: Michal Simek --- arch/arm/dts/Makefile | 4 1 file changed, 4 insertions(+) diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile index 537c96bf5b35..8d4fc333ea7a 100644 --- a/arch/arm/dts/Makefile +++ b/arch/arm/dts/Makefile @@ -1,5 +1,9 @@ # SPDX-License-Identifier: GPL-2.0+ +ifdef CONFIG_PHYS_64BIT +DTC_FLAGS += -a 0x8 +endif + dtb-$(CONFIG_TARGET_SMARTWEB) += at91sam9260-smartweb.dtb dtb-$(CONFIG_TARGET_TAURUS) += at91sam9g20-taurus.dtb dtb-$(CONFIG_TARGET_CORVUS) += at91sam9g45-corvus.dtb -- 2.32.0
[PATCH 05/10] xilinx: Add support for generic board detection
Add support for changing DT at run time. It is done via board_detection()
which returns platform_id and platform_version which can be used via
board_name_decode() to compose board_local_name string which corresponds
with DT which is should be used.
Signed-off-by: Michal Simek
---
board/xilinx/common/board.c | 31 +++
1 file changed, 31 insertions(+)
diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
index 2aecb14d8e27..92874272893a 100644
--- a/board/xilinx/common/board.c
+++ b/board/xilinx/common/board.c
@@ -463,3 +463,34 @@ int print_cpuinfo(void)
return 0;
}
#endif
+
+#if CONFIG_IS_ENABLED(DTB_RESELECT)
+char * __maybe_unused __weak board_name_decode(void)
+{
+ return NULL;
+}
+
+bool __maybe_unused __weak board_detection(void)
+{
+ return false;
+}
+
+int embedded_dtb_select(void)
+{
+ if (board_detection()) {
+ char *board_local_name;
+
+ board_local_name = board_name_decode();
+ if (board_local_name) {
+ board_name = board_local_name;
+ debug("Detected name: %s\n", board_name);
+
+ /* Time to change DTB on fly */
+ /* Both ways should work here */
+ /* fdtdec_resetup(); */
+ fdtdec_setup();
+ }
+ }
+ return 0;
+}
+#endif
--
2.32.0
[PATCH 06/10] xilinx: zynqmp: Check that DT is 64bit aligned
DT needs to be 64bit aligned. If it is not fdt64_to_cpu will fail when try
to read information about reserved memory. The system ends in exception
without any clue what's going it. That's why detect not aligned DT and
panic to show where the issue is coming from.
Signed-off-by: Michal Simek
---
board/xilinx/zynqmp/zynqmp.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index e43177ea4e48..ea15e62eb21e 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -470,6 +470,9 @@ ulong board_get_usable_ram_top(ulong total_size)
phys_addr_t reg;
struct lmb lmb;
+ if (!IS_ALIGNED((ulong)gd->fdt_blob, 0x8))
+ panic("Not 64bit aligned DT location: %p\n", gd->fdt_blob);
+
/* found enough not-reserved memory to relocated U-Boot */
lmb_init();
lmb_add(, gd->ram_base, gd->ram_size);
--
2.32.0
[PATCH 04/10] xilinx: common: Free allocated structure
There is no need to keep fru_content around. Free this space.
Signed-off-by: Michal Simek
---
board/xilinx/common/board.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
index 44c8aa5eefbb..2aecb14d8e27 100644
--- a/board/xilinx/common/board.c
+++ b/board/xilinx/common/board.c
@@ -185,8 +185,7 @@ static int xilinx_read_eeprom_fru(struct udevice *dev, char
*name,
eeprom_size);
if (ret) {
debug("%s: I2C EEPROM read failed\n", __func__);
- free(fru_content);
- return ret;
+ goto end;
}
printf("Xilinx I2C FRU format at %s:\n", name);
@@ -194,12 +193,13 @@ static int xilinx_read_eeprom_fru(struct udevice *dev,
char *name,
ret = fru_display(0);
if (ret) {
printf("FRU format decoding failed.\n");
- return ret;
+ goto end;
}
if (desc->header == EEPROM_HEADER_MAGIC) {
debug("Information already filled\n");
- return -EINVAL;
+ ret = -EINVAL;
+ goto end;
}
/* It is clear that FRU was captured and structures were filled */
@@ -217,7 +217,9 @@ static int xilinx_read_eeprom_fru(struct udevice *dev, char
*name,
sizeof(desc->serial));
desc->header = EEPROM_HEADER_MAGIC;
- return 0;
+end:
+ free(fru_content);
+ return ret;
}
static bool xilinx_detect_fru(u8 *buffer)
--
2.32.0
[PATCH 02/10] xilinx: Use variable for passing board_name
Use variable which points to DEVICE_TREE by default. The reason for this
change is to enable DTB_RESELECT and MULTI_DTB_FIT where board detection
can be used for change DTB at run time. That's why there must be reference
in board_fit_config_name_match() via variable instead of hardcoding it
which is sufficient for that use case.
Signed-off-by: Michal Simek
---
board/xilinx/common/board.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
index 1458b31c21a9..db9705c1b7e4 100644
--- a/board/xilinx/common/board.c
+++ b/board/xilinx/common/board.c
@@ -436,11 +436,13 @@ int board_late_init_xilinx(void)
}
#endif
+static char *board_name = DEVICE_TREE;
+
int __maybe_unused board_fit_config_name_match(const char *name)
{
- debug("%s: Check %s, default %s\n", __func__, name, DEVICE_TREE);
+ debug("%s: Check %s, default %s\n", __func__, name, board_name);
- if (!strcmp(name, DEVICE_TREE))
+ if (!strcmp(name, board_name))
return 0;
return -1;
--
2.32.0
[PATCH 00/10] xilinx: Add support for DTB reselection
Hi, this series add support for board or board+cc runtime DT selection. EEPROM memory is read and based on that decoded if this is legacy/fru based format and proper DTB is used. There is a need to have all DTBs 64bit aligned. If you don't have it you will end up in exception. But one patch in this series is trying to detect it and panic before you reach it to let you know what's wrong. Enforcing mkimage/dtb alignment is done based on CONFIG_PHYS_64BIT and affects all 64bit systems but it is also not wrong for them to be properly aligned. Thanks, Michal Michal Simek (10): xilinx: fru: Replace spaces with \0 in detected name xilinx: Use variable for passing board_name xilinx: common: Change board_info[] handling xilinx: common: Free allocated structure xilinx: Add support for generic board detection xilinx: zynqmp: Check that DT is 64bit aligned Makefile: Align fit-dtb.blob and u-boot.itb by 64bits for 64bit systems arm64: dts: Make sure that all DTBs are 64bit aligned for 64bit systems xilinx: zynqmp: Generate different u-boot.itb for MULTI_DTB_FIT xilinx: common: Enabling generic function for DT reselection Makefile| 7 ++ arch/arm/dts/Makefile | 4 + arch/arm/dts/zynqmp-sm-k26-revA.dts | 3 + arch/arm/mach-zynqmp/mkimage_fit_atf.sh | 47 +++ board/xilinx/common/board.c | 160 +++- board/xilinx/zynqmp/zynqmp.c| 3 + 6 files changed, 194 insertions(+), 30 deletions(-) -- 2.32.0
[PATCH 01/10] xilinx: fru: Replace spaces with \0 in detected name
FRU spec expected \0 for unused symbols but unfortunately a lot of boards
are using spaces instead of \0. That's why after saving it to desc->name
name is checked again and all spaces are converted to \0. This will ensure
that names can be used for string manipulations like concatenation.
Signed-off-by: Michal Simek
---
board/xilinx/common/board.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
index 90c87bab5cff..1458b31c21a9 100644
--- a/board/xilinx/common/board.c
+++ b/board/xilinx/common/board.c
@@ -168,7 +168,7 @@ static bool xilinx_detect_legacy(u8 *buffer)
static int xilinx_read_eeprom_fru(struct udevice *dev, char *name,
struct xilinx_board_description *desc)
{
- int ret, eeprom_size;
+ int i, ret, eeprom_size;
u8 *fru_content;
/* FIXME this is shortcut - if eeprom type is wrong it will fail */
@@ -207,6 +207,10 @@ static int xilinx_read_eeprom_fru(struct udevice *dev,
char *name,
sizeof(desc->manufacturer));
strncpy(desc->name, (char *)fru_data.brd.product_name,
sizeof(desc->name));
+ for (i = 0; i < sizeof(desc->name); i++) {
+ if (desc->name[i] == ' ')
+ desc->name[i] = '\0';
+ }
strncpy(desc->revision, (char *)fru_data.brd.rev,
sizeof(desc->revision));
strncpy(desc->serial, (char *)fru_data.brd.serial_number,
--
2.32.0
[PATCH 03/10] xilinx: common: Change board_info[] handling
Origin code was allocating only pointers to struct xilinx_board_description
and there was separate allocation for structure self and freeing in case of
failure.
The code is directly allocating space for all structures by one calloc to
simlify logic.
Signed-off-by: Michal Simek
---
board/xilinx/common/board.c | 23 ++-
1 file changed, 6 insertions(+), 17 deletions(-)
diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
index db9705c1b7e4..44c8aa5eefbb 100644
--- a/board/xilinx/common/board.c
+++ b/board/xilinx/common/board.c
@@ -68,7 +68,7 @@ struct xilinx_board_description {
};
static int highest_id = -1;
-static struct xilinx_board_description **board_info;
+static struct xilinx_board_description *board_info;
#define XILINX_I2C_DETECTION_BITS sizeof(struct fru_common_hdr)
@@ -280,7 +280,7 @@ static int xilinx_read_eeprom_single(char *name,
__maybe_unused int xilinx_read_eeprom(void)
{
- int id, ret;
+ int id;
char name_buf[8]; /* 8 bytes should be enough for nvmem+number */
struct xilinx_board_description *desc;
@@ -289,7 +289,7 @@ __maybe_unused int xilinx_read_eeprom(void)
if (highest_id < 0)
return -EINVAL;
- board_info = calloc(1, sizeof(desc) * highest_id);
+ board_info = calloc(1, sizeof(*desc) * (highest_id + 1));
if (!board_info)
return -ENOMEM;
@@ -300,21 +300,10 @@ __maybe_unused int xilinx_read_eeprom(void)
snprintf(name_buf, sizeof(name_buf), "nvmem%d", id);
/* Alloc structure */
- desc = board_info[id];
- if (!desc) {
- desc = calloc(1, sizeof(*desc));
- if (!desc)
- return -ENOMEM;
-
- board_info[id] = desc;
- }
+ desc = _info[id];
/* Ignoring return value for supporting multiple chips */
- ret = xilinx_read_eeprom_single(name_buf, desc);
- if (ret) {
- free(desc);
- board_info[id] = NULL;
- }
+ xilinx_read_eeprom_single(name_buf, desc);
}
/*
@@ -400,7 +389,7 @@ int board_late_init_xilinx(void)
ret |= env_set_addr("bootm_size", (void *)bootm_size);
for (id = 0; id <= highest_id; id++) {
- desc = board_info[id];
+ desc = _info[id];
if (desc && desc->header == EEPROM_HEADER_MAGIC) {
if (desc->manufacturer[0])
ret |= env_set_by_index("manufacturer", id,
--
2.32.0
Re: [PATCH v5 09/12] watchdog: wdt-uclass.c: handle all DM watchdogs in watchdog_reset()
Dear Rasmus,
In message <4798abb5-07d9-fa88-931f-dbaff951e...@prevas.dk> you wrote:
> >>
> >> + ret = uclass_get(UCLASS_WDT, );
> >> + if (ret) {
> >> + log_debug("Error getting UCLASS_WDT: %d\n", ret);
> >> + return 0;
> >> + }
> >
> > Here the error goes silent, so we should fix the callers to report
> > it.
>
> The caller (singular) is the initr sequence, so returning an error is
> effectively the same as halting the boot process, and as I've already
> explained, I'm not going to change the semantics of initr_watchdog in
> this regard.
In this case you must print an error message here.
> Feel free to submit a patch if you feel a change in this area is in
> order. That's completely unrelated to what these patches are trying to
> achieve.
You add new code here, so please make sure not to add known issues.
> >> + uclass_foreach_dev(dev, uc) {
> >> + ret = device_probe(dev);
> >> + if (ret) {
> >> + log_debug("Error probing %s: %d\n", dev->name, ret);
> >> + continue;
> >>}
> >
> > Here the situation is different. The probing error is never
> > reported anywhere. Is it really a normal condition that a
> > device_probe() fails here?
>
> No, it is not a normal condition. I added the log_debug() after a
> request from Simon.
But log_debug() is nothing any user will see in the field. We need
an error message here, too.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
If the odds are a million to one against something occuring, chances
are 50-50 it will.
[PATCH 3/3] cmd: add acmconsole command
This command allows to start CDC ACM function and redirect console
(stdin, stdout, stderr) to USB (acmconsole start). The console can
then be accessed through the USB host for debugging purpose. The
host can stop the session (acmconsole stop) to revert back console
to serial and unregister CDC ACM USB function.
Signed-off-by: Loic Poulain
---
cmd/Kconfig | 8
cmd/Makefile | 2 ++
cmd/acmconsole.c | 50 ++
3 files changed, 60 insertions(+)
create mode 100644 cmd/acmconsole.c
diff --git a/cmd/Kconfig b/cmd/Kconfig
index ffef3cc..7cc9b1c 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1382,6 +1382,14 @@ config CMD_AXI
Interface) busses, a on-chip interconnect specification for managing
functional blocks in SoC designs, which is also often used in designs
involving FPGAs (e.g. communication with IP cores in Xilinx FPGAs).
+
+config CMD_USB_ACM_CONSOLE
+ bool "ACM USB console"
+ select USB_FUNCTION_ACM
+ help
+ Enable the command "acmconsole" for accessing u-boot console from a
+ USB host through CDC ACM protocol.
+
endmenu
diff --git a/cmd/Makefile b/cmd/Makefile
index ed36694..c116091 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_CMD_BOOTMENU) += bootmenu.o
obj-$(CONFIG_CMD_BOOTSTAGE) += bootstage.o
obj-$(CONFIG_CMD_BOOTZ) += bootz.o
obj-$(CONFIG_CMD_BOOTI) += booti.o
+ob-y += acmconsole.o
obj-$(CONFIG_CMD_BTRFS) += btrfs.o
obj-$(CONFIG_CMD_BUTTON) += button.o
obj-$(CONFIG_CMD_CACHE) += cache.o
@@ -174,6 +175,7 @@ obj-$(CONFIG_CMD_FS_UUID) += fs_uuid.o
obj-$(CONFIG_CMD_USB_MASS_STORAGE) += usb_mass_storage.o
obj-$(CONFIG_CMD_USB_SDP) += usb_gadget_sdp.o
obj-$(CONFIG_CMD_THOR_DOWNLOAD) += thordown.o
+obj-$(CONFIG_CMD_USB_ACM_CONSOLE) += acmconsole.o
obj-$(CONFIG_CMD_XIMG) += ximg.o
obj-$(CONFIG_CMD_YAFFS2) += yaffs2.o
obj-$(CONFIG_CMD_SPL) += spl.o
diff --git a/cmd/acmconsole.c b/cmd/acmconsole.c
new file mode 100644
index 000..dac8136
--- /dev/null
+++ b/cmd/acmconsole.c
@@ -0,0 +1,50 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * acmconsole.c -- Console over USB CDC serial (ACM) function gadget function
+ *
+ * Copyright (c) 2021, Linaro Ltd
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+int do_usb_acmconsole(cmd_tbl_t *cmdtp, int flag, int argc, char * const
argv[])
+{
+ int ret;
+
+ if (argc != 2)
+ return CMD_RET_USAGE;
+
+ if (!strcmp(argv[1], "start")) {
+ ret = g_dnl_register("usb_serial_acm");
+ if (ret)
+ return ret;
+
+ /* ACM function creates a stdio device name 'stdio_acm' */
+ console_assign(stdin, "stdio_acm");
+ console_assign(stdout, "stdio_acm");
+ console_assign(stderr, "stdio_acm");
+ } else if (!strcmp(argv[1], "stop")) {
+ /* default to serial (TODO: restore initial devices) */
+ console_assign(stdin, "serial");
+ console_assign(stdout, "serial");
+ console_assign(stderr, "serial");
+
+ g_dnl_unregister();
+ g_dnl_clear_detach();
+ } else {
+ return CMD_RET_USAGE;
+ }
+
+ return 0;
+}
+
+U_BOOT_CMD(acmconsole, CONFIG_SYS_MAXARGS, 1, do_usb_acmconsole,
+ "Console over USB CDC ACM",
+ "start - start console over USB CDC ACM gadget function\n" \
+ "acmconsole stop - stop USB console");
--
2.7.4
[PATCH 2/3] usb: gadget: Add CDC ACM function
Add support for CDC ACM using the new UDC and gadget API. This protocol
can be used for serial over USB data transfer and is widely supported
by various OS (GNU/Linux, MS-Windows, OSX...). The usual purpose of
such link is to access device debug console and can be useful for
products not exposing regular UART to the user.
Signed-off-by: Loic Poulain
---
drivers/usb/gadget/Kconfig | 9 +
drivers/usb/gadget/Makefile | 1 +
drivers/usb/gadget/f_acm.c | 663
3 files changed, 673 insertions(+)
create mode 100644 drivers/usb/gadget/f_acm.c
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 327ea86..d81a9c5 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -182,6 +182,15 @@ config USB_FUNCTION_THOR
Enable Tizen's THOR download protocol support in U-Boot. It
allows downloading images into memory and flash them to target device.
+config USB_FUNCTION_ACM
+ bool "Enable CDC ACM gadget"
+ select SYS_STDIO_DEREGISTER
+ select CIRCBUF
+ help
+ ACM serial link. This function can be used to create a stdio device to
+ interoperate with MS-Windows hosts or with the Linux-USB "cdc-acm"
+ driver.
+
endif # USB_GADGET_DOWNLOAD
config USB_ETHER
diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
index f560068..d5d891b 100644
--- a/drivers/usb/gadget/Makefile
+++ b/drivers/usb/gadget/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_USB_FUNCTION_MASS_STORAGE) += f_mass_storage.o
obj-$(CONFIG_USB_FUNCTION_FASTBOOT) += f_fastboot.o
obj-$(CONFIG_USB_FUNCTION_SDP) += f_sdp.o
obj-$(CONFIG_USB_FUNCTION_ROCKUSB) += f_rockusb.o
+obj-$(CONFIG_USB_FUNCTION_ACM) += f_acm.o
endif
endif
ifdef CONFIG_USB_ETHER
diff --git a/drivers/usb/gadget/f_acm.c b/drivers/usb/gadget/f_acm.c
new file mode 100644
index 000..5115f1d
--- /dev/null
+++ b/drivers/usb/gadget/f_acm.c
@@ -0,0 +1,663 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * USB CDC serial (ACM) function driver
+ *
+ * Copyright (C) 2003 Al Borchers (alborch...@steinerpoint.com)
+ * Copyright (C) 2008 by David Brownell
+ * Copyright (C) 2008 by Nokia Corporation
+ * Copyright (C) 2009 by Samsung Electronics
+ * Copyright (c) 2021, Linaro Ltd
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+
+#define REQ_SIZE_MAX 512
+
+struct f_acm {
+ int ctrl_id;
+ int data_id;
+
+ struct usb_ep *ep_in;
+ struct usb_ep *ep_out;
+ struct usb_ep *ep_notify;
+
+ struct usb_request *req_in;
+ struct usb_request *req_out;
+
+ bool connected;
+ bool tx_on;
+
+ circbuf_t rx_buf;
+ circbuf_t tx_buf;
+
+ struct stdio_dev stdio;
+ struct usb_function usb_function;
+
+ struct usb_cdc_line_coding line_coding;
+ u16 handshake_bits;
+#define ACM_CTRL_RTS BIT(1) /* unused with full duplex */
+#define ACM_CTRL_DTR BIT(0) /* host is ready for data r/w */
+
+ int controller_index;
+};
+
+static inline struct f_acm *func_to_acm(struct usb_function *f)
+{
+ return container_of(f, struct f_acm, usb_function);
+}
+
+static inline struct f_acm *stdio_to_acm(struct stdio_dev *s)
+{
+ /* stdio dev is cloned on registration, do not use container_of */
+ return s->priv;
+}
+
+static struct usb_interface_assoc_descriptor
+acm_iad_descriptor = {
+ .bLength = sizeof(acm_iad_descriptor),
+ .bDescriptorType = USB_DT_INTERFACE_ASSOCIATION,
+ .bFirstInterface = 0,
+ .bInterfaceCount = 2, // control + data
+ .bFunctionClass = USB_CLASS_COMM,
+ .bFunctionSubClass =USB_CDC_SUBCLASS_ACM,
+ .bFunctionProtocol =USB_CDC_ACM_PROTO_AT_V25TER,
+};
+
+static struct usb_interface_descriptor acm_control_intf_desc = {
+ .bLength = USB_DT_INTERFACE_SIZE,
+ .bDescriptorType = USB_DT_INTERFACE,
+ .bNumEndpoints =1,
+ .bInterfaceClass = USB_CLASS_COMM,
+ .bInterfaceSubClass = USB_CDC_SUBCLASS_ACM,
+ .bInterfaceProtocol = USB_CDC_ACM_PROTO_AT_V25TER,
+};
+
+static struct usb_interface_descriptor acm_data_intf_desc = {
+ .bLength = sizeof(acm_data_intf_desc),
+ .bDescriptorType = USB_DT_INTERFACE,
+ .bNumEndpoints =2,
+ .bInterfaceClass = USB_CLASS_CDC_DATA,
+};
+
+static struct usb_cdc_header_desc acm_header_desc = {
+ .bLength = sizeof(acm_header_desc),
+ .bDescriptorType = USB_DT_CS_INTERFACE,
+ .bDescriptorSubType = USB_CDC_HEADER_TYPE,
+ .bcdCDC = cpu_to_le16(0x0110),
+};
+
+static struct usb_cdc_call_mgmt_descriptor acm_call_mgmt_desc = {
+ .bLength = sizeof(acm_call_mgmt_desc),
+ .bDescriptorType = USB_DT_CS_INTERFACE,
+
[PATCH 1/3] lib/circbuf: Make circbuf selectable symbol
It is currenly only used from usbtty driver but make it properly selectable via Kconfig symbol, for future usage. Signed-off-by: Loic Poulain --- lib/Kconfig | 3 +++ lib/Makefile | 8 +++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/Kconfig b/lib/Kconfig index c535147..1bd54d8 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -290,6 +290,9 @@ config TRACE_EARLY_ADDR the size is too small then the message which says the amount of early data being coped will the the same as the +config CIRCBUF +bool + source lib/dhry/Kconfig menu "Security support" diff --git a/lib/Makefile b/lib/Makefile index 8ba745f..4daeee2 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -29,7 +29,13 @@ ifneq ($(CONFIG_CHARSET),) obj-y += charset.o endif endif -obj-$(CONFIG_USB_TTY) += circbuf.o + +ifdef CONFIG_USB_TTY +obj-y += circbuf.o +else +obj-$(CONFIG_CIRCBUF) += circbuf.o +endif + obj-y += crc8.o obj-y += crc16.o obj-$(CONFIG_ERRNO_STR) += errno_str.o -- 2.7.4
Re: [PATCH 04/11] btrfs: Suppress the message about missing filesystem
On 2021/8/19 上午11:40, Simon Glass wrote:
This message comes up a lot when scanning filesystems. It suggests to the
user that there is some sort of error, but in fact there is no reason to
expect that a particular partition has a btrfs filesystem. Other
filesystems don't print this error.
Turn it into a debug message.
Signed-off-by: Simon Glass
Reviewed-by: Qu Wenruo
Thanks,
Qu
---
fs/btrfs/disk-io.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 349411c3ccd..1b69346e231 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -886,7 +886,11 @@ static int btrfs_scan_fs_devices(struct blk_desc *desc,
ret = btrfs_scan_one_device(desc, part, fs_devices, _devs);
if (ret) {
- fprintf(stderr, "No valid Btrfs found\n");
+ /*
+* Avoid showing this when probing for a possible Btrfs
+*
+* fprintf(stderr, "No valid Btrfs found\n");
+*/
return ret;
}
return 0;
@@ -975,7 +979,7 @@ struct btrfs_fs_info *open_ctree_fs_info(struct blk_desc
*desc,
disk_super = fs_info->super_copy;
ret = btrfs_read_dev_super(desc, part, disk_super);
if (ret) {
- printk("No valid btrfs found\n");
+ debug("No valid btrfs found\n");
goto out_devices;
}
Re: [PATCH 5/7] imx: makefile: drop the use of imx8mimage.sh
On 16.08.21 05:48, Peng Fan (OSS) wrote: > From: Peng Fan > > After switch to use binman, no need to use the bash script > to check file exsiting or not. And there is bug that > the script will be executed everytime Makefile is used which is > confusing people. > > Signed-off-by: Peng Fan For my mx8mm board config using binman, this resolves the following warning: WARNING 'mkimage.flash.mkimage' not found, resulting binary is not-functional Tested-by: Frieder Schrempf Is this save to be used with boards that haven't been converted to binman yet? > --- > arch/arm/mach-imx/Makefile | 13 +++-- > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile > index 0ef269563d..f629751c48 100644 > --- a/arch/arm/mach-imx/Makefile > +++ b/arch/arm/mach-imx/Makefile > @@ -114,8 +114,7 @@ endif > DEPFILE_EXISTS := $(shell $(CPP) $(cpp_flags) -x c -o u-boot-dtb.cfgout > $(srctree)/$(IMX_CONFIG); if [ -f u-boot-dtb.cfgout ]; then $(CNTR_DEPFILES) > u-boot-dtb.cfgout; echo $$?; fi) > else ifeq ($(CONFIG_ARCH_IMX8M), y) > IMAGE_TYPE := imx8mimage > -IMX8M_DEPFILES := $(srctree)/tools/imx8m_image.sh > -DEPFILE_EXISTS := $(shell $(CPP) $(cpp_flags) -x c -o spl/u-boot-spl.cfgout > $(srctree)/$(IMX_CONFIG);if [ -f spl/u-boot-spl.cfgout ]; then > $(IMX8M_DEPFILES) spl/u-boot-spl.cfgout 0; echo $$?; fi) > +DEPFILE_EXISTS := 0 > else > IMAGE_TYPE := imximage > DEPFILE_EXISTS := 0 > @@ -150,16 +149,18 @@ endif > > ifdef CONFIG_ARM64 > ifeq ($(CONFIG_ARCH_IMX8M), y) > -SPL: > + > +SPL: spl/u-boot-spl.bin spl/u-boot-spl.cfgout FORCE > > MKIMAGEFLAGS_flash.bin = -n spl/u-boot-spl.cfgout \ > -T $(IMAGE_TYPE) -e $(CONFIG_SPL_TEXT_BASE) > flash.bin: MKIMAGEOUTPUT = flash.log > > +spl/u-boot-spl.cfgout: $(IMX_CONFIG) FORCE > + $(Q)mkdir -p $(dir $@) > + $(call if_changed_dep,cpp_cfg) > + > spl/u-boot-spl-ddr.bin: spl/u-boot-spl.bin spl/u-boot-spl.cfgout FORCE > -ifeq ($(DEPFILE_EXISTS),0) > - $(IMX8M_DEPFILES) spl/u-boot-spl.cfgout 1 > -endif > > flash.bin: spl/u-boot-spl-ddr.bin u-boot.itb FORCE > $(call if_changed,mkimage) >
Re: [PATCH 04/11] btrfs: Suppress the message about missing filesystem
On Wed, 18 Aug 2021 21:40:26 -0600 Simon Glass wrote: > This message comes up a lot when scanning filesystems. It suggests to the > user that there is some sort of error, but in fact there is no reason to > expect that a particular partition has a btrfs filesystem. Other > filesystems don't print this error. > > Turn it into a debug message. > > Signed-off-by: Simon Glass Reviewed-by: Marek Behún This must have been introduced after btrfs was reimplemented from the btrfs-progs sources. I remember that I sent a patch fixing this same issue for the previous implementation (or was it another filesystem?). Marek
Re: [PATCH 09/11] sandbox: Add a way to map a file into memory
On Wed, 18 Aug 2021 21:40:31 -0600
Simon Glass wrote:
> It is useful to map a file into memory so that it can be accessed using
> simple pointers. Add a function to support this.
>
> Signed-off-by: Simon Glass
> +int os_map_file(const char *pathname, int os_flags, void **bufp, int *sizep)
> +{
> + void *ptr;
> + int size;
> + int ifd;
> +
> + ifd = os_open(pathname, os_flags);
> + if (ifd < 0) {
> + printf("Cannot open file '%s'\n", pathname);
> + return -EIO;
> + }
> + size = os_filesize(ifd);
> + if (size < 0) {
> + printf("Cannot get file size of '%s'\n", pathname);
> + return -EIO;
> + }
> +
> + ptr = mmap(0, size, PROT_READ | PROT_WRITE, MAP_SHARED, ifd, 0);
> + if (ptr == MAP_FAILED) {
> + printf("Can't map file '%s': %s\n", pathname, strerror(errno));
> + return -EPERM;
> + }
> +
> + *bufp = ptr;
> + *sizep = size;
> +
> + return 0;
> +}
You need to close the file descriptor after mmapping.
Marek
Re: [PATCH 08/11] sandbox: Add a way to find the size of a file
On Wed, 18 Aug 2021 21:40:30 -0600 Simon Glass wrote: > Add a function to return the size of a file. This is useful in situations > where we need to allocate memory for it before reading it. > > Signed-off-by: Simon Glass Reviewed-by: Marek Behún
[PATCH v6 10/12] watchdog: add gpio watchdog driver
A rather common kind of external watchdog circuit is one that is kept
alive by toggling a gpio. Add a driver for handling such a watchdog.
The corresponding linux driver apparently has support for some
watchdog circuits which can be disabled by tri-stating the gpio, but I
have never actually encountered such a chip in the wild; the whole
point of adding an external watchdog is usually that it is not in any
way under software control. For forward-compatibility, and to make DT
describe the hardware, the current driver only supports devices that
have the always-running property. I went a little back and forth on
whether I should fail ->probe or only ->start, and ended up deciding
->start was the right place.
The compatible string is probably a little odd as it has nothing to do
with linux per se - however, I chose that to make .dts snippets
reusable between device trees used with U-Boot and linux, and this is
the (only) compatible string that linux' corresponding driver and DT
binding accepts. I have asked whether one should/could add "wdt-gpio"
to that binding, but the answer was no:
https://lore.kernel.org/lkml/CAL_JsqKEGaFpiFV_oAtE+S_bnHkg4qry+bhx2EDs=nsbvf_...@mail.gmail.com/
If someone feels strongly about this, I can certainly remove the
"linux," part from the string - it probably wouldn't the only place where
one can't reuse a DT snippet as-is between linux and U-Boot.
Reviewed-by: Simon Glass
Reviewed-by: Stefan Roese
Signed-off-by: Rasmus Villemoes
---
.../watchdog/gpio-wdt.txt | 19 ++
drivers/watchdog/Kconfig | 9 +++
drivers/watchdog/Makefile | 1 +
drivers/watchdog/gpio_wdt.c | 68 +++
4 files changed, 97 insertions(+)
create mode 100644 doc/device-tree-bindings/watchdog/gpio-wdt.txt
create mode 100644 drivers/watchdog/gpio_wdt.c
diff --git a/doc/device-tree-bindings/watchdog/gpio-wdt.txt
b/doc/device-tree-bindings/watchdog/gpio-wdt.txt
new file mode 100644
index 00..c9a8559a3e
--- /dev/null
+++ b/doc/device-tree-bindings/watchdog/gpio-wdt.txt
@@ -0,0 +1,19 @@
+GPIO watchdog timer
+
+Describes a simple watchdog timer which is reset by toggling a gpio.
+
+Required properties:
+
+- compatible: Must be "linux,wdt-gpio".
+- gpios: gpio to toggle when wdt driver reset method is called.
+- always-running: Boolean property indicating that the watchdog cannot
+ be disabled. At present, U-Boot only supports this kind of GPIO
+ watchdog.
+
+Example:
+
+ gpio-wdt {
+ gpios = < 1 0>;
+ compatible = "linux,wdt-gpio";
+ always-running;
+ };
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index f0ff2612a6..6fbb5c1b6d 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -147,6 +147,15 @@ config WDT_CORTINA
This driver support all CPU ISAs supported by Cortina
Access CA SoCs.
+config WDT_GPIO
+ bool "External gpio watchdog support"
+ depends on WDT
+ depends on DM_GPIO
+ help
+ Support for external watchdog fed by toggling a gpio. See
+ doc/device-tree-bindings/watchdog/gpio-wdt.txt for
+ information on how to describe the watchdog in device tree.
+
config WDT_MPC8xx
bool "MPC8xx watchdog timer support"
depends on WDT && MPC8xx
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 5c7ef593fe..f14415bb8e 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_WDT_BOOKE) += booke_wdt.o
obj-$(CONFIG_WDT_CORTINA) += cortina_wdt.o
obj-$(CONFIG_WDT_ORION) += orion_wdt.o
obj-$(CONFIG_WDT_CDNS) += cdns_wdt.o
+obj-$(CONFIG_WDT_GPIO) += gpio_wdt.o
obj-$(CONFIG_WDT_MPC8xx) += mpc8xx_wdt.o
obj-$(CONFIG_WDT_MT7620) += mt7620_wdt.o
obj-$(CONFIG_WDT_MT7621) += mt7621_wdt.o
diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c
new file mode 100644
index 00..982a66b3f9
--- /dev/null
+++ b/drivers/watchdog/gpio_wdt.c
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include
+#include
+#include
+#include
+
+struct gpio_wdt_priv {
+ struct gpio_desc gpio;
+ bool always_running;
+ int state;
+};
+
+static int gpio_wdt_reset(struct udevice *dev)
+{
+ struct gpio_wdt_priv *priv = dev_get_priv(dev);
+
+ priv->state = !priv->state;
+
+ return dm_gpio_set_value(>gpio, priv->state);
+}
+
+static int gpio_wdt_start(struct udevice *dev, u64 timeout, ulong flags)
+{
+ struct gpio_wdt_priv *priv = dev_get_priv(dev);
+
+ if (priv->always_running)
+ return 0;
+
+ return -ENOSYS;
+}
+
+static int dm_probe(struct udevice *dev)
+{
+ struct gpio_wdt_priv *priv = dev_get_priv(dev);
+ int ret;
+
+ priv->always_running = dev_read_bool(dev, "always-running");
+ ret = gpio_request_by_name(dev, "gpios", 0, >gpio, GPIOD_IS_OUT);
+ if (ret < 0) {
+
[PATCH v6 11/12] sandbox: add test of wdt_gpio driver
It seems that no other test has claimed gpio_a:7 yet, so use that.
The only small wrinkle is modifying the existing wdt test to use
uclass_get_device_by_driver() since we now have two UCLASS_WDT
instances in play, so it's a little more robust to fetch the device by
driver and not merely uclass+index.
Reviewed-by: Simon Glass
Reviewed-by: Stefan Roese
Signed-off-by: Rasmus Villemoes
---
arch/sandbox/dts/test.dts | 6 ++
configs/sandbox64_defconfig | 1 +
configs/sandbox_defconfig | 1 +
test/dm/wdt.c | 36 +++-
4 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index d5976318d1..fe5ac6ecd9 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -779,6 +779,12 @@
};
};
+ gpio-wdt {
+ gpios = <_a 7 0>;
+ compatible = "linux,wdt-gpio";
+ always-running;
+ };
+
mbox: mbox {
compatible = "sandbox,mbox";
#mbox-cells = <1>;
diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index 3627a066b4..6cad90b03e 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -225,6 +225,7 @@ CONFIG_SPLASH_SCREEN_ALIGN=y
CONFIG_VIDEO_BMP_RLE8=y
# CONFIG_WATCHDOG_AUTOSTART is not set
CONFIG_WDT=y
+CONFIG_WDT_GPIO=y
CONFIG_WDT_SANDBOX=y
CONFIG_FS_CBFS=y
CONFIG_FS_CRAMFS=y
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 34b749b47b..4a8b4f220d 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -284,6 +284,7 @@ CONFIG_W1_EEPROM=y
CONFIG_W1_EEPROM_SANDBOX=y
# CONFIG_WATCHDOG_AUTOSTART is not set
CONFIG_WDT=y
+CONFIG_WDT_GPIO=y
CONFIG_WDT_SANDBOX=y
CONFIG_FS_CBFS=y
CONFIG_FS_CRAMFS=y
diff --git a/test/dm/wdt.c b/test/dm/wdt.c
index 24b991dff6..abff853a02 100644
--- a/test/dm/wdt.c
+++ b/test/dm/wdt.c
@@ -6,6 +6,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -19,7 +20,8 @@ static int dm_test_wdt_base(struct unit_test_state *uts)
struct udevice *dev;
const u64 timeout = 42;
- ut_assertok(uclass_get_device(UCLASS_WDT, 0, ));
+ ut_assertok(uclass_get_device_by_driver(UCLASS_WDT,
+ DM_DRIVER_GET(wdt_sandbox),
));
ut_assertnonnull(dev);
ut_asserteq(0, state->wdt.counter);
ut_asserteq(false, state->wdt.running);
@@ -39,3 +41,35 @@ static int dm_test_wdt_base(struct unit_test_state *uts)
return 0;
}
DM_TEST(dm_test_wdt_base, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+
+static int dm_test_wdt_gpio(struct unit_test_state *uts)
+{
+ /*
+* The sandbox wdt gpio is "connected" to gpio bank a, offset
+* 7. Use the sandbox back door to verify that the gpio-wdt
+* driver behaves as expected.
+*/
+ struct udevice *wdt, *gpio;
+ const u64 timeout = 42;
+ const int offset = 7;
+ int val;
+
+ ut_assertok(uclass_get_device_by_driver(UCLASS_WDT,
+ DM_DRIVER_GET(wdt_gpio), ));
+ ut_assertnonnull(wdt);
+
+ ut_assertok(uclass_get_device_by_name(UCLASS_GPIO, "base-gpios",
));
+ ut_assertnonnull(gpio);
+ ut_assertok(wdt_start(wdt, timeout, 0));
+
+ val = sandbox_gpio_get_value(gpio, offset);
+ ut_assertok(wdt_reset(wdt));
+ ut_asserteq(!val, sandbox_gpio_get_value(gpio, offset));
+ ut_assertok(wdt_reset(wdt));
+ ut_asserteq(val, sandbox_gpio_get_value(gpio, offset));
+
+ ut_asserteq(-ENOSYS, wdt_stop(wdt));
+
+ return 0;
+}
+DM_TEST(dm_test_wdt_gpio, UT_TESTF_SCAN_FDT);
--
2.31.1
[PATCH v6 12/12] sandbox: add test of wdt-uclass' watchdog_reset()
Check that the watchdog_reset() implementation in wdt-uclass behaves
as expected:
- resets all activated watchdog devices
- leaves unactivated/stopped devices alone
- that the rate-limiting works, with a per-device threshold
Reviewed-by: Simon Glass
Reviewed-by: Stefan Roese
Signed-off-by: Rasmus Villemoes
---
arch/sandbox/dts/test.dts | 2 ++
test/dm/wdt.c | 54 +++
2 files changed, 56 insertions(+)
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index fe5ac6ecd9..bafc2e9494 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -782,6 +782,7 @@
gpio-wdt {
gpios = <_a 7 0>;
compatible = "linux,wdt-gpio";
+ hw_margin_ms = <100>;
always-running;
};
@@ -1264,6 +1265,7 @@
wdt0: wdt@0 {
compatible = "sandbox,wdt";
+ hw_margin_ms = <200>;
};
axi: axi@0 {
diff --git a/test/dm/wdt.c b/test/dm/wdt.c
index abff853a02..ee615f0e14 100644
--- a/test/dm/wdt.c
+++ b/test/dm/wdt.c
@@ -12,6 +12,8 @@
#include
#include
#include
+#include
+#include
/* Test that watchdog driver functions are called */
static int dm_test_wdt_base(struct unit_test_state *uts)
@@ -73,3 +75,55 @@ static int dm_test_wdt_gpio(struct unit_test_state *uts)
return 0;
}
DM_TEST(dm_test_wdt_gpio, UT_TESTF_SCAN_FDT);
+
+static int dm_test_wdt_watchdog_reset(struct unit_test_state *uts)
+{
+ struct sandbox_state *state = state_get_current();
+ struct udevice *gpio_wdt, *sandbox_wdt;
+ struct udevice *gpio;
+ const u64 timeout = 42;
+ const int offset = 7;
+ uint reset_count;
+ int val;
+
+ ut_assertok(uclass_get_device_by_driver(UCLASS_WDT,
+ DM_DRIVER_GET(wdt_gpio),
_wdt));
+ ut_assertnonnull(gpio_wdt);
+ ut_assertok(uclass_get_device_by_driver(UCLASS_WDT,
+ DM_DRIVER_GET(wdt_sandbox),
_wdt));
+ ut_assertnonnull(sandbox_wdt);
+ ut_assertok(uclass_get_device_by_name(UCLASS_GPIO, "base-gpios",
));
+ ut_assertnonnull(gpio);
+
+ /* Neither device should be "started", so watchdog_reset() should be a
no-op. */
+ reset_count = state->wdt.reset_count;
+ val = sandbox_gpio_get_value(gpio, offset);
+ watchdog_reset();
+ ut_asserteq(reset_count, state->wdt.reset_count);
+ ut_asserteq(val, sandbox_gpio_get_value(gpio, offset));
+
+ /* Start both devices. */
+ ut_assertok(wdt_start(gpio_wdt, timeout, 0));
+ ut_assertok(wdt_start(sandbox_wdt, timeout, 0));
+
+ /* Make sure both devices have just been pinged. */
+ timer_test_add_offset(100);
+ watchdog_reset();
+ reset_count = state->wdt.reset_count;
+ val = sandbox_gpio_get_value(gpio, offset);
+
+ /* The gpio watchdog should be pinged, the sandbox one not. */
+ timer_test_add_offset(30);
+ watchdog_reset();
+ ut_asserteq(reset_count, state->wdt.reset_count);
+ ut_asserteq(!val, sandbox_gpio_get_value(gpio, offset));
+
+ /* After another ~30ms, both devices should get pinged. */
+ timer_test_add_offset(30);
+ watchdog_reset();
+ ut_asserteq(reset_count + 1, state->wdt.reset_count);
+ ut_asserteq(val, sandbox_gpio_get_value(gpio, offset));
+
+ return 0;
+}
+DM_TEST(dm_test_wdt_watchdog_reset, UT_TESTF_SCAN_FDT);
--
2.31.1
[PATCH v6 07/12] watchdog: wdt-uclass.c: add wdt_stop_all() helper
Since the watchdog_dev member of struct global_data is going away in
favor of the wdt-uclass handling all watchdog devices, prepare for
that by adding a helper to call wdt_stop() on all known devices.
If an error is encountered, still do wdt_stop() on remaining devices,
but remember and return the first error seen.
Initially, this will only be used in one single
place (board/alliedtelesis/x530/x530.c).
Signed-off-by: Rasmus Villemoes
---
drivers/watchdog/wdt-uclass.c | 25 +
include/wdt.h | 8
2 files changed, 33 insertions(+)
diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index 358fc68e27..5b1c0df5d6 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -116,6 +116,31 @@ int wdt_stop(struct udevice *dev)
return ret;
}
+int wdt_stop_all(void)
+{
+ struct wdt_priv *priv;
+ struct udevice *dev;
+ struct uclass *uc;
+ int ret, err;
+
+ ret = uclass_get(UCLASS_WDT, );
+ if (ret)
+ return ret;
+
+ uclass_foreach_dev(dev, uc) {
+ if (!device_active(dev))
+ continue;
+ priv = dev_get_uclass_priv(dev);
+ if (!priv->running)
+ continue;
+ err = wdt_stop(dev);
+ if (!ret)
+ ret = err;
+ }
+
+ return ret;
+}
+
int wdt_reset(struct udevice *dev)
{
const struct wdt_ops *ops = device_get_ops(dev);
diff --git a/include/wdt.h b/include/wdt.h
index bc242c2eb2..baaa9db08a 100644
--- a/include/wdt.h
+++ b/include/wdt.h
@@ -37,6 +37,14 @@ int wdt_start(struct udevice *dev, u64 timeout_ms, ulong
flags);
*/
int wdt_stop(struct udevice *dev);
+/*
+ * Stop all registered watchdog devices.
+ *
+ * @return: 0 if ok, first error encountered otherwise (but wdt_stop()
+ * is still called on following devices)
+ */
+int wdt_stop_all(void);
+
/*
* Reset the timer, typically restoring the counter to
* the value configured by start()
--
2.31.1
[PATCH v6 08/12] board: x530: switch to wdt_stop_all()
Since the gd->watchdog_dev member is going away, switch to using the
new wdt_stop_all() helper.
While here, clean up the preprocessor conditional: The ->watchdog_dev
member is actually guarded by CONFIG_WDT [disabling that in
x530_defconfig while keeping CONFIG_WATCHDOG breaks the build], and in
the new world order so is the existence of the wdt_stop_all()
function.
Actually, existence of wdt_stop_all() depends on CONFIG_${SPL_}WDT, so
really spell the condition using CONFIG_IS_ENABLED, and make it a C
rather than cpp if.
Signed-off-by: Rasmus Villemoes
---
board/alliedtelesis/x530/x530.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/board/alliedtelesis/x530/x530.c b/board/alliedtelesis/x530/x530.c
index 7bcfa828d7..8b31045a07 100644
--- a/board/alliedtelesis/x530/x530.c
+++ b/board/alliedtelesis/x530/x530.c
@@ -121,9 +121,8 @@ int board_init(void)
void arch_preboot_os(void)
{
-#ifdef CONFIG_WATCHDOG
- wdt_stop(gd->watchdog_dev);
-#endif
+ if (CONFIG_IS_ENABLED(WDT))
+ wdt_stop_all();
}
static int led_7seg_init(unsigned int segments)
--
2.31.1
[PATCH v6 06/12] sandbox: disable CONFIG_WATCHDOG_AUTOSTART
For the unit tests, it is more convenient if the tests are in charge of when the watchdog devices are started and stopped, so prevent wdt-uclass from doing it automatically. Reviewed-by: Simon Glass Reviewed-by: Stefan Roese Signed-off-by: Rasmus Villemoes --- configs/sandbox64_defconfig | 1 + configs/sandbox_defconfig | 1 + 2 files changed, 2 insertions(+) diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig index f7098b4969..3627a066b4 100644 --- a/configs/sandbox64_defconfig +++ b/configs/sandbox64_defconfig @@ -223,6 +223,7 @@ CONFIG_OSD=y CONFIG_SANDBOX_OSD=y CONFIG_SPLASH_SCREEN_ALIGN=y CONFIG_VIDEO_BMP_RLE8=y +# CONFIG_WATCHDOG_AUTOSTART is not set CONFIG_WDT=y CONFIG_WDT_SANDBOX=y CONFIG_FS_CBFS=y diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index bcd82f76ff..34b749b47b 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -282,6 +282,7 @@ CONFIG_W1=y CONFIG_W1_GPIO=y CONFIG_W1_EEPROM=y CONFIG_W1_EEPROM_SANDBOX=y +# CONFIG_WATCHDOG_AUTOSTART is not set CONFIG_WDT=y CONFIG_WDT_SANDBOX=y CONFIG_FS_CBFS=y -- 2.31.1
[PATCH v6 09/12] watchdog: wdt-uclass.c: handle all DM watchdogs in watchdog_reset()
A board can have and make use of more than one watchdog device, say
one built into the SOC and an external gpio-petted one. Having
wdt-uclass only handle the first is both a little arbitrary and
unexpected.
So change initr_watchdog() so we visit (probe) all DM watchdog
devices, and call the init_watchdog_dev helper for each.
Similarly let watchdog_reset() loop over the whole uclass - each
having their own ratelimiting metadata, and a separate "is this device
running" flag.
This gets rid of the watchdog_dev member of struct global_data. We
do, however, still need the GD_FLG_WDT_READY set in
initr_watchdog(). This is because watchdog_reset() can get called
before DM is ready, and I don't think we can call uclass_get() that
early.
The current code just returns 0 if "getting" the first device fails -
that can of course happen because there are no devices, but it could
also happen if its ->probe call failed. In keeping with that, continue
with the handling of the remaining devices even if one fails to
probe. This is also why we cannot use uclass_probe_all().
If desired, it's possible to later add a per-device "u-boot,autostart"
boolean property, so that one can do CONFIG_WATCHDOG_AUTOSTART
per-device.
Reviewed-by: Simon Glass
Reviewed-by: Stefan Roese
Signed-off-by: Rasmus Villemoes
---
drivers/watchdog/wdt-uclass.c | 56 ---
include/asm-generic/global_data.h | 6
2 files changed, 36 insertions(+), 26 deletions(-)
diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index 5b1c0df5d6..7570710c4d 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -61,20 +61,24 @@ static void init_watchdog_dev(struct udevice *dev)
int initr_watchdog(void)
{
- /*
-* Init watchdog: This will call the probe function of the
-* watchdog driver, enabling the use of the device
-*/
- if (uclass_get_device_by_seq(UCLASS_WDT, 0,
-(struct udevice **)>watchdog_dev)) {
- debug("WDT: Not found by seq!\n");
- if (uclass_get_device(UCLASS_WDT, 0,
- (struct udevice **)>watchdog_dev)) {
- printf("WDT: Not found!\n");
- return 0;
+ struct udevice *dev;
+ struct uclass *uc;
+ int ret;
+
+ ret = uclass_get(UCLASS_WDT, );
+ if (ret) {
+ log_debug("Error getting UCLASS_WDT: %d\n", ret);
+ return 0;
+ }
+
+ uclass_foreach_dev(dev, uc) {
+ ret = device_probe(dev);
+ if (ret) {
+ log_debug("Error probing %s: %d\n", dev->name, ret);
+ continue;
}
+ init_watchdog_dev(dev);
}
- init_watchdog_dev(gd->watchdog_dev);
gd->flags |= GD_FLG_WDT_READY;
return 0;
@@ -182,22 +186,34 @@ void watchdog_reset(void)
{
struct wdt_priv *priv;
struct udevice *dev;
+ struct uclass *uc;
ulong now;
/* Exit if GD is not ready or watchdog is not initialized yet */
if (!gd || !(gd->flags & GD_FLG_WDT_READY))
return;
- dev = gd->watchdog_dev;
- priv = dev_get_uclass_priv(dev);
- if (!priv->running)
+ if (uclass_get(UCLASS_WDT, ))
return;
- /* Do not reset the watchdog too often */
- now = get_timer(0);
- if (time_after_eq(now, priv->next_reset)) {
- priv->next_reset = now + priv->reset_period;
- wdt_reset(dev);
+ /*
+* All devices bound to the wdt uclass should have been probed
+* in initr_watchdog(). But just in case something went wrong,
+* check device_active() before accessing the uclass private
+* data.
+*/
+ uclass_foreach_dev(dev, uc) {
+ if (!device_active(dev))
+ continue;
+ priv = dev_get_uclass_priv(dev);
+ if (!priv->running)
+ continue;
+ /* Do not reset the watchdog too often */
+ now = get_timer(0);
+ if (time_after_eq(now, priv->next_reset)) {
+ priv->next_reset = now + priv->reset_period;
+ wdt_reset(dev);
+ }
}
}
#endif
diff --git a/include/asm-generic/global_data.h
b/include/asm-generic/global_data.h
index e55070303f..28d749538c 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -447,12 +447,6 @@ struct global_data {
*/
fdt_addr_t translation_offset;
#endif
-#if CONFIG_IS_ENABLED(WDT)
- /**
-* @watchdog_dev: watchdog device
-*/
- struct udevice *watchdog_dev;
-#endif
#ifdef CONFIG_GENERATE_ACPI_TABLE
/**
* @acpi_ctx: ACPI context pointer
--
2.31.1
[PATCH v6 05/12] watchdog: wdt-uclass.c: keep track of each device's running state
As a step towards handling all DM watchdogs in watchdog_reset(), use a
per-device flag to keep track of whether the device has been started
instead of a bit in gd->flags.
We will still need that bit to know whether we are past
initr_watchdog() and hence have populated gd->watchdog_dev -
incidentally, that is how it was used prior to commit 9c44ff1c5f3c.
Reviewed-by: Simon Glass
Reviewed-by: Stefan Roese
Signed-off-by: Rasmus Villemoes
---
drivers/watchdog/wdt-uclass.c | 20
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index 0a1f43771c..358fc68e27 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -33,6 +33,8 @@ struct wdt_priv {
* ->reset().
*/
ulong next_reset;
+ /* Whether watchdog_start() has been called on the device. */
+ bool running;
};
static void init_watchdog_dev(struct udevice *dev)
@@ -74,6 +76,7 @@ int initr_watchdog(void)
}
init_watchdog_dev(gd->watchdog_dev);
+ gd->flags |= GD_FLG_WDT_READY;
return 0;
}
@@ -86,8 +89,11 @@ int wdt_start(struct udevice *dev, u64 timeout_ms, ulong
flags)
return -ENOSYS;
ret = ops->start(dev, timeout_ms, flags);
- if (ret == 0)
- gd->flags |= GD_FLG_WDT_READY;
+ if (ret == 0) {
+ struct wdt_priv *priv = dev_get_uclass_priv(dev);
+
+ priv->running = true;
+ }
return ret;
}
@@ -101,8 +107,11 @@ int wdt_stop(struct udevice *dev)
return -ENOSYS;
ret = ops->stop(dev);
- if (ret == 0)
- gd->flags &= ~GD_FLG_WDT_READY;
+ if (ret == 0) {
+ struct wdt_priv *priv = dev_get_uclass_priv(dev);
+
+ priv->running = false;
+ }
return ret;
}
@@ -156,6 +165,9 @@ void watchdog_reset(void)
dev = gd->watchdog_dev;
priv = dev_get_uclass_priv(dev);
+ if (!priv->running)
+ return;
+
/* Do not reset the watchdog too often */
now = get_timer(0);
if (time_after_eq(now, priv->next_reset)) {
--
2.31.1
[PATCH v6 04/12] watchdog: wdt-uclass.c: refactor initr_watchdog()
In preparation for handling all DM watchdogs in watchdog_reset(), pull
out the code which handles starting (or not) the gd->watchdog_dev
device.
Include the device name in various printfs.
Reviewed-by: Simon Glass
Reviewed-by: Stefan Roese
Signed-off-by: Rasmus Villemoes
---
drivers/watchdog/wdt-uclass.c | 37 ---
1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index 81287c759a..0a1f43771c 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -35,11 +35,30 @@ struct wdt_priv {
ulong next_reset;
};
-int initr_watchdog(void)
+static void init_watchdog_dev(struct udevice *dev)
{
struct wdt_priv *priv;
int ret;
+ priv = dev_get_uclass_priv(dev);
+
+ if (!IS_ENABLED(CONFIG_WATCHDOG_AUTOSTART)) {
+ printf("WDT: Not starting %s\n", dev->name);
+ return;
+ }
+
+ ret = wdt_start(dev, priv->timeout * 1000, 0);
+ if (ret != 0) {
+ printf("WDT: Failed to start %s\n", dev->name);
+ return;
+ }
+
+ printf("WDT: Started %s with%s servicing (%ds timeout)\n", dev->name,
+ IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", priv->timeout);
+}
+
+int initr_watchdog(void)
+{
/*
* Init watchdog: This will call the probe function of the
* watchdog driver, enabling the use of the device
@@ -53,21 +72,7 @@ int initr_watchdog(void)
return 0;
}
}
- priv = dev_get_uclass_priv(gd->watchdog_dev);
-
- if (!IS_ENABLED(CONFIG_WATCHDOG_AUTOSTART)) {
- printf("WDT: Not starting\n");
- return 0;
- }
-
- ret = wdt_start(gd->watchdog_dev, priv->timeout * 1000, 0);
- if (ret != 0) {
- printf("WDT: Failed to start\n");
- return 0;
- }
-
- printf("WDT: Started with%s servicing (%ds timeout)\n",
- IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", priv->timeout);
+ init_watchdog_dev(gd->watchdog_dev);
return 0;
}
--
2.31.1
[PATCH v6 03/12] watchdog: wdt-uclass.c: neaten UCLASS_DRIVER definition
The addition of .pre_probe and .per_device_auto made this look
bad. Fix it.
Reviewed-by: Simon Glass
Reviewed-by: Stefan Roese
Signed-off-by: Rasmus Villemoes
---
drivers/watchdog/wdt-uclass.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index b29d214724..81287c759a 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -210,10 +210,10 @@ static int wdt_pre_probe(struct udevice *dev)
}
UCLASS_DRIVER(wdt) = {
- .id = UCLASS_WDT,
- .name = "watchdog",
- .flags = DM_UC_FLAG_SEQ_ALIAS,
- .post_bind = wdt_post_bind,
+ .id = UCLASS_WDT,
+ .name = "watchdog",
+ .flags = DM_UC_FLAG_SEQ_ALIAS,
+ .post_bind = wdt_post_bind,
.pre_probe = wdt_pre_probe,
.per_device_auto= sizeof(struct wdt_priv),
};
--
2.31.1
