RE: [PATCH v2] spl: bootstage: move bootstage_stash before jumping to image

2023-08-29 Thread Chanho Park
Hi Simon,

> -Original Message-
> From: Simon Glass 
> Sent: Wednesday, August 30, 2023 1:38 AM
> To: Chanho Park 
> Cc: Nikhil M Jain ; Marek Vasut ; u-
> b...@lists.denx.de
> Subject: Re: [PATCH v2] spl: bootstage: move bootstage_stash before
> jumping to image
> 
> Hi Chanho,
> 
> On Mon, 28 Aug 2023 at 22:28, Chanho Park 
> wrote:
> >
> > Regarding IH_OS_OPENSBI, IH_OS_LINUX and IH_OS_TEE, there is no chance
> > to stash bootstage record because they do not return to SPL after
> > jumping to the image.
> > Hence, this patch separates the final stage bootstage code into
> > spl_bootstage_finish and call the function before jumping to the image.
> >
> > Signed-off-by: Chanho Park 
> > ---
> > Changes from v1
> > - Separate the final stage bootstage code into spl_bootstage_finish.
> > - As Simon suggests, call the function before jumping to the image.
> 
> I think you misunderstood me here. I mean, you cannot jump off somewhere
> in your board code. You must change it so it returns correctly, and the
> jump happens from spl.c's board_init_r() function.
> The way it works is you set up the spl_image structure, then it SPL jumps
> to it at the end of the functions.

I feel like I'm still not clear on what you mean. Sorry.

switch (spl_image.os) {
case IH_OS_U_BOOT:
case IH_OS_ARM_TRUSTED_FIRMWARE:
case IH_OS_TEE:
case IH_OS_OPENSBI:
case IH_OS_LINUX:
}

Regarding ATF/TEE/OPENSBI and Linux, they need different number of arguments 
and formats to jump to the image, respectively.
I think that's why they can't go to the final stage and can't use 
jump_to_image_no_args.

Do you want to move jump codes at the end of the board_init_r function?
The easiest way is that we just move the whole switch statements to the final 
stage of the function.
Otherwise, the arguments can be prepared from switch statement and make 
jump_to_image function to support variable length of arguments.
(Or we can put switch statement there to support various jump of the image)

Can you elaborate a bit more?

Best Regards,
Chanho Park



Re: [PATCH v11 00/15] Integrate EFI capsule tasks into U-Boot's build flow

2023-08-29 Thread Simon Glass
Hi,

On Tue, 29 Aug 2023 at 17:15, Tom Rini  wrote:
>
> On Tue, 22 Aug 2023 23:09:53 +0530, Sughosh Ganu wrote:
>
> > This patchset aims to bring two capsule related tasks under the U-Boot
> > build flow.
> >
> > The first task is related to generation of capsules. The capsules can
> > be generated as part of U-Boot build, and this is being achieved
> > through binman, by adding a capsule entry type. The capsules can be
> > generated by specifying the capsule parameters as properties under the
> > capsule entry node.
> >
> > [...]
>
> With v12 of that one patch and v11 of the rest, applied to u-boot/next, 
> thanks!

It's great to see this in!

Regards,
Simon


Re: [PATCH v11 00/15] Integrate EFI capsule tasks into U-Boot's build flow

2023-08-29 Thread Tom Rini
On Tue, 22 Aug 2023 23:09:53 +0530, Sughosh Ganu wrote:

> This patchset aims to bring two capsule related tasks under the U-Boot
> build flow.
> 
> The first task is related to generation of capsules. The capsules can
> be generated as part of U-Boot build, and this is being achieved
> through binman, by adding a capsule entry type. The capsules can be
> generated by specifying the capsule parameters as properties under the
> capsule entry node.
> 
> [...]

With v12 of that one patch and v11 of the rest, applied to u-boot/next, thanks!

-- 
Tom



[RFC PATCH 2/2] configs: Add am62x_beagleplay_* defconfigs

2023-08-29 Thread Andrew Davis
Add am62x_beagleplay_r5_defconfig for R5 SPL and
am62x_beagleplay_a53_defconfig for A53 SPL and U-Boot support.

These defconfigs are composite defconfigs built from the config fragment
board/ti/am62x/beagleplay_*.config applied onto the base
am62x_evm_*_defconfig.

Signed-off-by: Andrew Davis 
---
 configs/am62x_beagleplay_a53_defconfig | 3 +++
 configs/am62x_beagleplay_r5_defconfig  | 3 +++
 2 files changed, 6 insertions(+)
 create mode 100644 configs/am62x_beagleplay_a53_defconfig
 create mode 100644 configs/am62x_beagleplay_r5_defconfig

diff --git a/configs/am62x_beagleplay_a53_defconfig 
b/configs/am62x_beagleplay_a53_defconfig
new file mode 100644
index 000..ad708e15397
--- /dev/null
+++ b/configs/am62x_beagleplay_a53_defconfig
@@ -0,0 +1,3 @@
+// The BeaglePlay defconfig for A53 core
+#include "configs/am62x_evm_a53_defconfig"
+#include "board/ti/am62x/beagleplay_a53.config"
diff --git a/configs/am62x_beagleplay_r5_defconfig 
b/configs/am62x_beagleplay_r5_defconfig
new file mode 100644
index 000..276b1f81a3e
--- /dev/null
+++ b/configs/am62x_beagleplay_r5_defconfig
@@ -0,0 +1,3 @@
+// The BeaglePlay defconfig for R5 core
+#include "configs/am62x_evm_r5_defconfig"
+#include "board/ti/am62x/beagleplay_r5.config"
-- 
2.39.2



[RFC PATCH 0/2] Allow defconfigs defined from fragments

2023-08-29 Thread Andrew Davis
Hello all,

For context see thread ending here[0].

Thanks,
Andrew

[0] https://marc.info/?l=u-boot=169333616210919=2

Andrew Davis (2):
  Makefile: Run defconfig files through the C preprocessor
  configs: Add am62x_beagleplay_* defconfigs

 configs/am62x_beagleplay_a53_defconfig | 3 +++
 configs/am62x_beagleplay_r5_defconfig  | 3 +++
 scripts/kconfig/Makefile   | 3 ++-
 3 files changed, 8 insertions(+), 1 deletion(-)
 create mode 100644 configs/am62x_beagleplay_a53_defconfig
 create mode 100644 configs/am62x_beagleplay_r5_defconfig

-- 
2.39.2



[RFC PATCH 1/2] Makefile: Run defconfig files through the C preprocessor

2023-08-29 Thread Andrew Davis
This allows us to use some of the normal preprocessor directives inside
defconfig files. Such as #define and #include.

Signed-off-by: Andrew Davis 
---
 scripts/kconfig/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index 12e525ee31f..16db9b7cf60 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -93,7 +93,8 @@ endif
 endif
 
 %_defconfig: $(obj)/conf
-   $(Q)$< $(silent) --defconfig=arch/$(SRCARCH)/configs/$@ $(Kconfig)
+   $(Q)$(CPP) -nostdinc -I $(srctree) -undef -x assembler-with-cpp 
$(srctree)/arch/$(SRCARCH)/configs/$@ -o generated_defconfig
+   $(Q)$< $(silent) --defconfig=generated_defconfig $(Kconfig)
 
 # Added for U-Boot (backward compatibility)
 %_config: %_defconfig
-- 
2.39.2



Re: [PATCH v4 4/4] memory: Add ECC property

2023-08-29 Thread Rob Herring
On Tue, Aug 29, 2023 at 2:18 PM Simon Glass  wrote:
>
> Some memories provides ECC correction. For software which wants to check
> memory, it is helpful to see which regions provide this feature.
>
> Add this as a property of the /memory nodes, since it presumably follows
> the hardware-level memory system.
>
> Signed-off-by: Simon Glass 
> ---
>
> (no changes since v3)
>
> Changes in v3:
> - Add new patch to update the /memory nodes
>
>  dtschema/schemas/memory.yaml | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/dtschema/schemas/memory.yaml b/dtschema/schemas/memory.yaml
> index 1d74410..981af04 100644
> --- a/dtschema/schemas/memory.yaml
> +++ b/dtschema/schemas/memory.yaml
> @@ -34,7 +34,14 @@ patternProperties:
>  description:
>For the purpose of identification, each NUMA node is associated 
> with
>a unique token known as a node id.
> -
> +  attr:

Kind of vague.

> +$ref: /schemas/types.yaml#/definitions/string-array
> +description: |
> +  Attributes possessed by this memory region:
> +
> +"single-bit-ecc" - supports single-bit ECC
> +"multi-bit-ecc" - supports multiple-bit ECC

"supports" means corrects or reports? Most h/w supports both, but only
reports multi-bit errors.

> +"no-ecc" - non-ECC memory

Don't define values in free form text.

This form is difficult to validate especially when non-ECC related
attr's are added to the mix as we can't really define which
combinations are valid. For example how do we prevent:

attr = "single-bit-ecc", "multi-bit-ecc";

Or maybe that's valid? If so, how would we express that?

Why do we need "no-ecc"? Is that the same as no "attr" property?

I think it's better if we have 'ecc-type' or something? Or generally,
a property per class/type of attribute.

Rob


Re: [PATCH v3 1/2] schemas: Add a schema for memory map

2023-08-29 Thread Ard Biesheuvel
On Tue, 29 Aug 2023 at 21:18, Simon Glass  wrote:
>
> Hi Ard,
>
> On Thu, 24 Aug 2023 at 03:10, Ard Biesheuvel  wrote:
> >
> > On Wed, 23 Aug 2023 at 22:04, Simon Glass  wrote:
> > >
> > > Hi,
> > >
> > > On Wed, 23 Aug 2023 at 08:24, Ard Biesheuvel  wrote:
> > > >
> > > > On Wed, 23 Aug 2023 at 10:59, Mark Rutland  wrote:
> > > > >
> > > > > On Tue, Aug 22, 2023 at 02:34:42PM -0600, Simon Glass wrote:
> > > > > > The Devicetree specification skips over handling of a logical view 
> > > > > > of
> > > > > > the memory map, pointing users to the UEFI specification.
> > > > > >
> > > > > > It is common to split firmware into 'Platform Init', which does the
> > > > > > initial hardware setup and a "Payload" which selects the OS to be 
> > > > > > booted.
> > > > > > Thus an handover interface is required between these two pieces.
> > > > > >
> > > > > > Where UEFI boot-time services are not available, but UEFI firmware 
> > > > > > is
> > > > > > present on either side of this interface, information about memory 
> > > > > > usage
> > > > > > and attributes must be presented to the "Payload" in some form.
> > > >
> > > > Not quite.
> > > >
> > > > This seems to be intended for consumption by Linux booting in ACPI
> > > > mode, but not via UEFI, right?
> > >
> > > Actually, this is for consumption by firmware. The goal is to allow
> > > edk2 to boot into U-Boot and vice versa, i.e. provide some
> > > interoperability between firmware projects. I will use the "Platform
> > > Init" and "Payload" terminology here too.
> > >
> >
> > OK. It was the cc to linux-acpi@ and the authors of the
> > ACPI/SMBIOS-without-UEFI patches that threw me off here.
> >
> > If we are talking about an internal interface for firmware components,
> > I'd be inclined to treat this as an implementation detail, as long as
> > the OS is not expected to consume these DT nodes.
> >
> > However, I struggle to see the point of framing this information as a
> > 'UEFI memory map'. Neither EDK2 nor u-boot consume this information
> > natively, and there is already prior art in both projects to consume
> > nodes following the existing bindings for device_type=memory and the
> > /reserved-memory node. UEFI runtime memory is generally useless
> > without UEFI runtime services, and UEFI boot services memory is just
> > free memory.
> >
> > There is also an overlap with the handover between secure and
> > non-secure firmware on arm64, which is also DT based, and communicates
> > available memory as well as RAM regions that are reserved for firmware
> > use.
> >
> > In summary, I don't see why a non-UEFI payload would care about UEFI
> > semantics for pre-existing memory reservations, or vice versa. Note
> > that EDK2 will manage its own memory map, and expose it via UEFI boot
> > services and not via DT.
>
> Bear in mind that one or both sides of this interface may be UEFI.
> There is no boot-services link between the two parts that I have
> outlined.
>

I don't understand what this means.

UEFI specifies how one component invokes another, and it is not based
on a DT binding. If the second component calls UEFI boot or runtime
services, it should be invoked in this manner. If it doesn't, then it
doesn't care about these memory reservations (and the OS will not be
booted via UEFI either)

So I feel I am missing something here. Perhaps a practical example
would be helpful?


> >
> > ...
> > >
> > > There is no intent to implement the UEFI spec, here. It is simply that
> > > some payloads (EDK2) are used to having this information.
> > >
> > > Imagine splitting EDK2 into two parts, one of which does platform init
> > > and the other which (the payload) boots the OS. The payload wants
> > > information from Platform Init and it needs to be in a devicetree,
> > > since that is what we have chosen for this interface. So to some
> > > extent this is unrelated to whether you have EFI boot services. We
> > > just need to be able to pass the information across the interface.
> > > Note that the user can (without recompilation, etc.) replace the
> > > second part with U-Boot (for example) and it must still work.
> > >
> >
> > OK, so device tree makes sense for this. This is how I implemented the
> > EDK2 port that targets QEMU/mach-virt - it consumes the DT to discover
> > the UART, RTC,, memory, PCI host bridge, etc.
> >
> > But I don't see a use case for a UEFI memory map here.
> >
> >
...
> > >
> > > Here I believe you are talking about booting the kernel in EFI mode,
> > > but that is not the intent of this patch. This is all about things
> > > happening in firmware. Now, if the payload (second) part of the
> > > firmware decides it wants to offer EFI boot services and boot the
> > > kernel via the EFI stub, then it may very well pack this information
> > > (with a few changes) into a system table and make it available to the
> > > kernel stub. But by then this FDT binding is irrelevant, since it has
> > > served its purpose (which, to reiterate, is to 

Re: [PATCH v1] CI: Add jsonschema python module

2023-08-29 Thread Andrejs Cainikovs
On 29/08/2023 17:37, Andrejs Cainikovs wrote:
> On 29/08/2023 17:04, Tom Rini wrote:
>> Interesting.  How exactly are you using these CI images? We have
>> tools/buildman/requirements.txt now to cover the newly-added modules in
>> CI, as we call pip on that.  But, it's not being used in the prepopulate
>> the pip cache stage in the Dockerfile.  And, I'm not immune to the idea
>> that we should instead be installing the distro package here.

Tom, I got the full picture right after I sent you my previous email.
This particular file, tools/buildman/requirements.txt in used in *CI*,
right.

Well, in this case my patch doesn't make much sense. Or it does.

I'd like to propose to install packages to runner image instead. I see
the reason why it might be convenient to have packages installed to
runner for particular jobs via .gitlab.yml - a bit less maintenance of
Docker image, which would need to be rebuilt every time there's new
dependency. However, if you think about pipeline execution times, on a
global scale, that's a waste. As an example, I see four occasions of:

pip install -r tools/buildman/requirements.txt

Twice in world build stage and twice in testsuites stage.

If we take another example, packages from py/requirements.txt are
installed 37 (if I counted correctly) times during entire testsuite
stage [2]. And I see you run them a lot [1].

Since you are maintainer of this, I will leave this up to you. But if
you're up for this proposal, I can do this change by myself. Of course,
this would imply rebuilding Docker image and pushing to registry from
your side, to keep stuff synchronized.

[1]: https://source.denx.de/u-boot/u-boot/-/pipelines/17556
[2]: https://source.denx.de/u-boot/u-boot/-/jobs/685270#L366

Regards,
Andrejs.

> 
> I see in Dockerfile test/py/requirements.txt and
> doc/sphinx/requirements.txt, but not tools/buildman/requirements.txt, so
> whatever is mentioned there is not part of the image on CI runners.
> 
> Following is the part of *our own* .gitlab.yml file,
> which uses a public Docker image
> (docker.io/trini/u-boot-gitlab-ci-runner:jammy-20230624-20Jul2023, as of
> now):
> 
> verdin-am62:
>   stage: build
>   script: |
> ...
> tools/buildman/buildman -o /tmp -P -E "verdin-am62" -M
> ...
> 
> And this is how it fails in our CI without this change:
> 
> Building current source for 3 boards (3 threads, 14 jobs per thread)
>arm:  +   verdin-am62_r5
> +binman: Unknown entry type 'ti-board-config' in node
> '/binman/board-cfg/ti-board-config' (expected etype/ti_board_config.py,
> error 'No module named 'jsonschema''
> +make[1]: *** [Makefile:1108: .binman_stamp] Error 1
> +make: *** [Makefile:177: sub-make] Error 2
> 

-- 
Best regards,
Andrejs Cainikovs



Re: [PATCH v4 2/4] Bring in other reserved-memory files

2023-08-29 Thread Rob Herring
On Tue, Aug 29, 2023 at 2:18 PM Simon Glass  wrote:
>
> Add those files from v6.5 which are already converted to yaml.
>
> Signed-off-by: Simon Glass 
> ---
>
> Changes in v4:
> - New patch
>
>  .../schemas/reserved-memory/framebuffer.yaml  |  52 +++
>  .../reserved-memory/google,open-dice.yaml |  46 ++
>  .../reserved-memory/memory-region.yaml|  40 +
>  .../nvidia,tegra210-emc-table.yaml|  31 
>  .../nvidia,tegra264-bpmp-shmem.yaml   |  47 ++
>  dtschema/schemas/reserved-memory/phram.yaml   |  47 ++
>  .../schemas/reserved-memory/qcom,cmd-db.yaml  |  46 ++
>  .../reserved-memory/qcom,rmtfs-mem.yaml   |  55 +++
>  dtschema/schemas/reserved-memory/ramoops.yaml | 144 ++
>  .../reserved-memory/shared-dma-pool.yaml  |  97 
>  10 files changed, 605 insertions(+)

I don't think I want all of these in dtschema. Certainly not vendor
specific ones. Generally, if it's not something we'd put in the spec,
then I don't want it in dtschema. The u-boot stuff so far has been an
exception.

Probably a good rule is if it has a compatible, then it doesn't go
into dtschema. Though there's some exceptions already.

>  create mode 100644 dtschema/schemas/reserved-memory/framebuffer.yaml

Maybe

>  create mode 100644 dtschema/schemas/reserved-memory/google,open-dice.yaml

No

>  create mode 100644 dtschema/schemas/reserved-memory/memory-region.yaml

Yes

>  create mode 100644 
> dtschema/schemas/reserved-memory/nvidia,tegra210-emc-table.yaml
>  create mode 100644 
> dtschema/schemas/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml

No

>  create mode 100644 dtschema/schemas/reserved-memory/phram.yaml

Depends on mtd.yaml in the kernel, so no.

>  create mode 100644 dtschema/schemas/reserved-memory/qcom,cmd-db.yaml
>  create mode 100644 dtschema/schemas/reserved-memory/qcom,rmtfs-mem.yaml

No.

>  create mode 100644 dtschema/schemas/reserved-memory/ramoops.yaml

Probably not.

>  create mode 100644 dtschema/schemas/reserved-memory/shared-dma-pool.yaml

Maybe.

Rob


[PATCH v2 2/4] fdt: kaslr seed from tpm entropy

2023-08-29 Thread seanedmond
From: Dhananjay Phadke 

Add support for KASLR seed from TPM device. Invokes tpm_get_random()
API to read 8-bytes of random bytes for KASLR.

Signed-off-by: Dhananjay Phadke 
Signed-off-by: Drew Kluemke 
Signed-off-by: Sean Edmond 
---
 boot/image-fdt.c  | 15 +++
 common/fdt_support.c  | 30 ++
 include/fdt_support.h |  8 
 lib/Kconfig   |  9 +
 4 files changed, 62 insertions(+)

diff --git a/boot/image-fdt.c b/boot/image-fdt.c
index f10200f647..ed38ed77b9 100644
--- a/boot/image-fdt.c
+++ b/boot/image-fdt.c
@@ -624,6 +624,21 @@ int image_setup_libfdt(struct bootm_headers *images, void 
*blob,
goto err;
}
 
+   if (IS_ENABLED(CONFIG_KASLR_TPM_SEED)) {
+   ofnode root;
+
+   ret = root_ofnode_from_fdt(blob, );
+   if (ret) {
+   printf("ERROR: Unable to get root ofnode\n");
+   goto err;
+   }
+   ret = fdt_tpm_kaslr_seed(root);
+   if (ret) {
+   printf("ERROR: fdt fixup KASLR failed: %d\n", ret);
+   goto err;
+   }
+   }
+
fdt_ret = optee_copy_fdt_nodes(blob);
if (fdt_ret) {
printf("ERROR: transfer of optee nodes to new fdt failed: %s\n",
diff --git a/common/fdt_support.c b/common/fdt_support.c
index 52be4375b4..d338fcde54 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -13,6 +13,9 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -650,6 +653,33 @@ int fdt_fixup_kaslr_seed(ofnode node, const u8 *seed, int 
len)
return 0;
 }
 
+int fdt_tpm_kaslr_seed(ofnode node)
+{
+   u8 rand[8] = {0};
+   struct udevice *dev;
+   int ret;
+
+   ret = uclass_first_device_err(UCLASS_TPM, );
+   if (ret) {
+   printf("ERROR: Failed to find TPM device\n");
+   return ret;
+   }
+
+   ret = tpm_get_random(dev, rand, sizeof(rand));
+   if (ret) {
+   printf("ERROR: TPM GetRandom failed, ret=%d\n", ret);
+   return ret;
+   }
+
+   ret = fdt_fixup_kaslr_seed(node, rand, sizeof(rand));
+   if (ret) {
+   printf("ERROR: failed to add kaslr-seed to fdt\n");
+   return ret;
+   }
+
+   return 0;
+}
+
 int fdt_record_loadable(void *blob, u32 index, const char *name,
uintptr_t load_addr, u32 size, uintptr_t entry_point,
const char *type, const char *os, const char *arch)
diff --git a/include/fdt_support.h b/include/fdt_support.h
index d967118bed..117ca14ca5 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -130,6 +130,14 @@ void fdt_fixup_ethernet(void *fdt);
  */
 int fdt_fixup_kaslr_seed(ofnode node, const u8 *seed, int len);
 
+/*
+ * fdt_add_tpm_kaslr_seed - Add kalsr-seed node in Device tree with random
+ * bytes from TPM device
+ * @node:  ofnode
+ * @eret:  0 for success
+ */
+int fdt_tpm_kaslr_seed(ofnode node);
+
 int fdt_find_and_setprop(void *fdt, const char *node, const char *prop,
 const void *val, int len, int create);
 void fdt_fixup_qe_firmware(void *fdt);
diff --git a/lib/Kconfig b/lib/Kconfig
index 3926652db6..1530ef7c86 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -465,6 +465,15 @@ config VPL_TPM
  for the low-level TPM interface, but only one TPM is supported at
  a time by the TPM library.
 
+config KASLR_TPM_SEED
+   bool "Use TPM for KASLR random seed"
+   depends on TPM_V1 || TPM_V2
+   help
+ This enables support for using TPMs as entropy source for KASLR seed
+ populated in kernel's device tree. Both TPMv1 and TPMv2 are supported
+ for the low-level TPM interface, but only one TPM is supported at
+ a time by the library.
+
 endmenu
 
 menu "Android Verified Boot"
-- 
2.40.0



[PATCH v2 4/4] dm: core: Modify default for OFNODE_MULTI_TREE

2023-08-29 Thread seanedmond
From: Sean Edmond 

There is a preference to use the "ofnode" API for FDT fixups
moving forward.  The FDT fixup will usually be for the kernel FDT.  To
fixup the kernel FDT with the ofnode API, it's required to set the
OFNODE_MULTI_TREE option.

To ensure existing users of kaslr fdt fixup are not impacted, Let's modify
the default value for OFNODE_MULTI_TREE to ensure it's always set if
!OF_LIVE.  This will cause a 1007 byte increase in the code size.

Signed-off-by: Sean Edmond 
---
 drivers/core/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
index f0d848f45d..38e44ef6fc 100644
--- a/drivers/core/Kconfig
+++ b/drivers/core/Kconfig
@@ -424,7 +424,7 @@ config DM_DEV_READ_INLINE
 
 config OFNODE_MULTI_TREE
bool "Allow the ofnode interface to access any tree"
-   default y if EVENT && !DM_DEV_READ_INLINE && !DM_INLINE_OFNODE
+   default y if !OF_LIVE
help
  Normally U-Boot makes use of its control FDT, the one used to bind
  devices and provide options. In some cases, U-Boot must also process
-- 
2.40.0



[PATCH v2 3/4] cmd: kaslrseed: Use common API to fixup FDT

2023-08-29 Thread seanedmond
From: Sean Edmond 

Use the newly introduced common API fdt_fixup_kaslr_seed() in the
kaslrseed command.

Signed-off-by: Sean Edmond 
---
 cmd/kaslrseed.c | 22 --
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/cmd/kaslrseed.c b/cmd/kaslrseed.c
index 8a1d8120cd..c65607619b 100644
--- a/cmd/kaslrseed.c
+++ b/cmd/kaslrseed.c
@@ -19,7 +19,7 @@ static int do_kaslr_seed(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const
size_t n = 0x8;
struct udevice *dev;
u64 *buf;
-   int nodeoffset;
+   ofnode root;
int ret = CMD_RET_SUCCESS;
 
if (uclass_get_device(UCLASS_RNG, 0, ) || !dev) {
@@ -45,21 +45,15 @@ static int do_kaslr_seed(struct cmd_tbl *cmdtp, int flag, 
int argc, char *const
return CMD_RET_FAILURE;
}
 
-   ret = fdt_check_header(working_fdt);
-   if (ret < 0) {
-   printf("fdt_chosen: %s\n", fdt_strerror(ret));
-   return CMD_RET_FAILURE;
-   }
-
-   nodeoffset = fdt_find_or_add_subnode(working_fdt, 0, "chosen");
-   if (nodeoffset < 0) {
-   printf("Reading chosen node failed\n");
-   return CMD_RET_FAILURE;
+   ret = root_ofnode_from_fdt(working_fdt, );
+   if (ret) {
+   printf("ERROR: Unable to get root ofnode\n");
+   goto CMD_RET_FAILURE;
}
 
-   ret = fdt_setprop(working_fdt, nodeoffset, "kaslr-seed", buf, 
sizeof(buf));
-   if (ret < 0) {
-   printf("Unable to set kaslr-seed on chosen node: %s\n", 
fdt_strerror(ret));
+   ret = fdt_fixup_kaslr_seed(root, buf, sizeof(buf));
+   if (ret) {
+   printf("ERROR: failed to add kaslr-seed to fdt\n");
return CMD_RET_FAILURE;
}
 
-- 
2.40.0



[PATCH v2 1/4] fdt: common API to populate kaslr seed

2023-08-29 Thread seanedmond
From: Dhananjay Phadke 

fdt_fixup_kaslr_seed() will update given ofnode with random seed value.
Source for random seed can be TPM or RNG driver in u-boot or sec
firmware (ARM).

Signed-off-by: Dhananjay Phadke 
Signed-off-by: Sean Edmond 
---
 arch/arm/cpu/armv8/sec_firmware.c | 39 +++
 common/fdt_support.c  | 19 +++
 drivers/core/ofnode.c | 17 ++
 include/dm/ofnode.h   | 12 ++
 include/fdt_support.h |  9 +++
 5 files changed, 71 insertions(+), 25 deletions(-)

diff --git a/arch/arm/cpu/armv8/sec_firmware.c 
b/arch/arm/cpu/armv8/sec_firmware.c
index c0e8726346..5f04cd8aec 100644
--- a/arch/arm/cpu/armv8/sec_firmware.c
+++ b/arch/arm/cpu/armv8/sec_firmware.c
@@ -411,46 +411,35 @@ int sec_firmware_init(const void *sec_firmware_img,
 /*
  * fdt_fix_kaslr - Add kalsr-seed node in Device tree
  * @fdt:   Device tree
- * @eret:  0 in case of error, 1 for success
+ * @eret:  0 for success
  */
 int fdt_fixup_kaslr(void *fdt)
 {
-   int nodeoffset;
-   int err, ret = 0;
-   u8 rand[8];
+   int ret = 0;
 
 #if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT)
+   u8 rand[8];
+   ofnode root;
+
/* Check if random seed generation is  supported */
if (sec_firmware_support_hwrng() == false) {
printf("WARNING: SEC firmware not running, no kaslr-seed\n");
-   return 0;
+   return -EOPNOTSUPP;
}
 
-   err = sec_firmware_get_random(rand, 8);
-   if (err < 0) {
+   ret = sec_firmware_get_random(rand, 8);
+   if (ret < 0) {
printf("WARNING: No random number to set kaslr-seed\n");
-   return 0;
+   return ret;
}
 
-   err = fdt_check_header(fdt);
-   if (err < 0) {
-   printf("fdt_chosen: %s\n", fdt_strerror(err));
-   return 0;
+   ret = root_ofnode_from_fdt(fdt, );
+   if (ret < 0) {
+   printf("WARNING: Unable to get root ofnode\n");
+   return ret;
}
 
-   /* find or create "/chosen" node. */
-   nodeoffset = fdt_find_or_add_subnode(fdt, 0, "chosen");
-   if (nodeoffset < 0)
-   return 0;
-
-   err = fdt_setprop(fdt, nodeoffset, "kaslr-seed", rand,
- sizeof(rand));
-   if (err < 0) {
-   printf("WARNING: can't set kaslr-seed %s.\n",
-  fdt_strerror(err));
-   return 0;
-   }
-   ret = 1;
+   ret = fdt_fixup_kaslr_seed(root, rand, sizeof(rand));
 #endif
 
return ret;
diff --git a/common/fdt_support.c b/common/fdt_support.c
index 5e49078f8c..52be4375b4 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -631,6 +631,25 @@ void fdt_fixup_ethernet(void *fdt)
}
 }
 
+int fdt_fixup_kaslr_seed(ofnode node, const u8 *seed, int len)
+{
+   ofnode chosen;
+   int ret;
+
+   /* find or create "/chosen" node. */
+   ret = ofnode_add_subnode(node, "chosen", );
+   if (ret && ret != -EEXIST)
+   return -ENOENT;
+
+   ret = ofnode_write_prop(chosen, "kaslr-seed", seed, len, true);
+   if (ret) {
+   printf("WARNING: can't set kaslr-seed\n");
+   return ret;
+   }
+
+   return 0;
+}
+
 int fdt_record_loadable(void *blob, u32 index, const char *name,
uintptr_t load_addr, u32 size, uintptr_t entry_point,
const char *type, const char *os, const char *arch)
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index 8df16e56af..4be21133b8 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -870,6 +870,23 @@ ofnode oftree_path(oftree tree, const char *path)
}
 }
 
+int root_ofnode_from_fdt(void *fdt, ofnode *root_node)
+{
+   oftree tree;
+   /* If OFNODE_MULTI_TREE is not set, and if fdt is not the control FDT,
+*  oftree_from_fdt() will return NULL
+*/
+   tree = oftree_from_fdt(fdt);
+
+   if (!oftree_valid(tree)) {
+   printf("Cannot create oftree\n");
+   return -EINVAL;
+   }
+   *root_node = oftree_root(tree);
+
+   return 0;
+}
+
 const void *ofnode_read_chosen_prop(const char *propname, int *sizep)
 {
ofnode chosen_node;
diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
index 0f38b3e736..e79bb62be8 100644
--- a/include/dm/ofnode.h
+++ b/include/dm/ofnode.h
@@ -901,6 +901,18 @@ ofnode oftree_path(oftree tree, const char *path);
  */
 ofnode oftree_root(oftree tree);
 
+/**
+ * root_ofnode_from_fdt() - Gets the root ofnode given an FDT blob.
+ *  Note, this will fail if OFNODE_MULTI_TREE
+ *  is not set.
+ *
+ * @fdt: Device tree to use
+ * @root_node : Root ofnode
+ *
+ * Return: 0 if OK, -ve on error
+ */
+int root_ofnode_from_fdt(void *fdt, ofnode 

[PATCH v2 0/4] Populate kaslr seed with TPM

2023-08-29 Thread seanedmond
From: Sean Edmond 

This patch series creates a common API (fdt_fixup_kaslr_seed()) for 
populating the kaslr seed in the DTB.  Existing users (kaslrseed,
and ARMv8 sec firmware) have been updated to use this common API.

New functionality has been introduced to populate the kaslr using
the TPM interface.  This can be enabled with CONFIG_KASLR_TPM_SEED.  

changes in v2:
- fdt_fixup_kaslr_seed() uses the ofnode API
- Add root_ofnode_from_fdt() to get the root node from an FDT and
  perform error checking on the oftree
- add comments to exported functions
- Add error checking in image_setup_libfdt() for return from
  fdt_tpm_kaslr_seed()
- uclass_get_device() -> uclass_first_device_err()
- Change default config for OFNODE_MULTI_TREE (y if !OF_LIVE)

Dhananjay Phadke (2):
  fdt: common API to populate kaslr seed
  fdt: kaslr seed from tpm entropy

Sean Edmond (2):
  cmd: kaslrseed: Use common API to fixup FDT
  dm: core: Modify default for OFNODE_MULTI_TREE

 arch/arm/cpu/armv8/sec_firmware.c | 39 +---
 boot/image-fdt.c  | 15 ++
 cmd/kaslrseed.c   | 22 +-
 common/fdt_support.c  | 49 +++
 drivers/core/Kconfig  |  2 +-
 drivers/core/ofnode.c | 17 +++
 include/dm/ofnode.h   | 12 
 include/fdt_support.h | 17 +++
 lib/Kconfig   |  9 ++
 9 files changed, 142 insertions(+), 40 deletions(-)

-- 
2.40.0



[PATCH v2 1/4] fdt: common API to populate kaslr seed

2023-08-29 Thread seanedmond
From: Dhananjay Phadke 

fdt_fixup_kaslr_seed() will update given ofnode with random seed value.
Source for random seed can be TPM or RNG driver in u-boot or sec
firmware (ARM).

Signed-off-by: Dhananjay Phadke 
Signed-off-by: Sean Edmond 
---
 arch/arm/cpu/armv8/sec_firmware.c | 39 +++
 common/fdt_support.c  | 19 +++
 drivers/core/ofnode.c | 17 ++
 include/dm/ofnode.h   | 12 ++
 include/fdt_support.h |  9 +++
 5 files changed, 71 insertions(+), 25 deletions(-)

diff --git a/arch/arm/cpu/armv8/sec_firmware.c 
b/arch/arm/cpu/armv8/sec_firmware.c
index c0e8726346..5f04cd8aec 100644
--- a/arch/arm/cpu/armv8/sec_firmware.c
+++ b/arch/arm/cpu/armv8/sec_firmware.c
@@ -411,46 +411,35 @@ int sec_firmware_init(const void *sec_firmware_img,
 /*
  * fdt_fix_kaslr - Add kalsr-seed node in Device tree
  * @fdt:   Device tree
- * @eret:  0 in case of error, 1 for success
+ * @eret:  0 for success
  */
 int fdt_fixup_kaslr(void *fdt)
 {
-   int nodeoffset;
-   int err, ret = 0;
-   u8 rand[8];
+   int ret = 0;
 
 #if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT)
+   u8 rand[8];
+   ofnode root;
+
/* Check if random seed generation is  supported */
if (sec_firmware_support_hwrng() == false) {
printf("WARNING: SEC firmware not running, no kaslr-seed\n");
-   return 0;
+   return -EOPNOTSUPP;
}
 
-   err = sec_firmware_get_random(rand, 8);
-   if (err < 0) {
+   ret = sec_firmware_get_random(rand, 8);
+   if (ret < 0) {
printf("WARNING: No random number to set kaslr-seed\n");
-   return 0;
+   return ret;
}
 
-   err = fdt_check_header(fdt);
-   if (err < 0) {
-   printf("fdt_chosen: %s\n", fdt_strerror(err));
-   return 0;
+   ret = root_ofnode_from_fdt(fdt, );
+   if (ret < 0) {
+   printf("WARNING: Unable to get root ofnode\n");
+   return ret;
}
 
-   /* find or create "/chosen" node. */
-   nodeoffset = fdt_find_or_add_subnode(fdt, 0, "chosen");
-   if (nodeoffset < 0)
-   return 0;
-
-   err = fdt_setprop(fdt, nodeoffset, "kaslr-seed", rand,
- sizeof(rand));
-   if (err < 0) {
-   printf("WARNING: can't set kaslr-seed %s.\n",
-  fdt_strerror(err));
-   return 0;
-   }
-   ret = 1;
+   ret = fdt_fixup_kaslr_seed(root, rand, sizeof(rand));
 #endif
 
return ret;
diff --git a/common/fdt_support.c b/common/fdt_support.c
index 5e49078f8c..52be4375b4 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -631,6 +631,25 @@ void fdt_fixup_ethernet(void *fdt)
}
 }
 
+int fdt_fixup_kaslr_seed(ofnode node, const u8 *seed, int len)
+{
+   ofnode chosen;
+   int ret;
+
+   /* find or create "/chosen" node. */
+   ret = ofnode_add_subnode(node, "chosen", );
+   if (ret && ret != -EEXIST)
+   return -ENOENT;
+
+   ret = ofnode_write_prop(chosen, "kaslr-seed", seed, len, true);
+   if (ret) {
+   printf("WARNING: can't set kaslr-seed\n");
+   return ret;
+   }
+
+   return 0;
+}
+
 int fdt_record_loadable(void *blob, u32 index, const char *name,
uintptr_t load_addr, u32 size, uintptr_t entry_point,
const char *type, const char *os, const char *arch)
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index 8df16e56af..4be21133b8 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -870,6 +870,23 @@ ofnode oftree_path(oftree tree, const char *path)
}
 }
 
+int root_ofnode_from_fdt(void *fdt, ofnode *root_node)
+{
+   oftree tree;
+   /* If OFNODE_MULTI_TREE is not set, and if fdt is not the control FDT,
+*  oftree_from_fdt() will return NULL
+*/
+   tree = oftree_from_fdt(fdt);
+
+   if (!oftree_valid(tree)) {
+   printf("Cannot create oftree\n");
+   return -EINVAL;
+   }
+   *root_node = oftree_root(tree);
+
+   return 0;
+}
+
 const void *ofnode_read_chosen_prop(const char *propname, int *sizep)
 {
ofnode chosen_node;
diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
index 0f38b3e736..e79bb62be8 100644
--- a/include/dm/ofnode.h
+++ b/include/dm/ofnode.h
@@ -901,6 +901,18 @@ ofnode oftree_path(oftree tree, const char *path);
  */
 ofnode oftree_root(oftree tree);
 
+/**
+ * root_ofnode_from_fdt() - Gets the root ofnode given an FDT blob.
+ *  Note, this will fail if OFNODE_MULTI_TREE
+ *  is not set.
+ *
+ * @fdt: Device tree to use
+ * @root_node : Root ofnode
+ *
+ * Return: 0 if OK, -ve on error
+ */
+int root_ofnode_from_fdt(void *fdt, ofnode 

[PATCH v2 2/4] fdt: kaslr seed from tpm entropy

2023-08-29 Thread seanedmond
From: Dhananjay Phadke 

Add support for KASLR seed from TPM device. Invokes tpm_get_random()
API to read 8-bytes of random bytes for KASLR.

Signed-off-by: Dhananjay Phadke 
Signed-off-by: Drew Kluemke 
Signed-off-by: Sean Edmond 
---
 boot/image-fdt.c  | 15 +++
 common/fdt_support.c  | 30 ++
 include/fdt_support.h |  8 
 lib/Kconfig   |  9 +
 4 files changed, 62 insertions(+)

diff --git a/boot/image-fdt.c b/boot/image-fdt.c
index f10200f647..ed38ed77b9 100644
--- a/boot/image-fdt.c
+++ b/boot/image-fdt.c
@@ -624,6 +624,21 @@ int image_setup_libfdt(struct bootm_headers *images, void 
*blob,
goto err;
}
 
+   if (IS_ENABLED(CONFIG_KASLR_TPM_SEED)) {
+   ofnode root;
+
+   ret = root_ofnode_from_fdt(blob, );
+   if (ret) {
+   printf("ERROR: Unable to get root ofnode\n");
+   goto err;
+   }
+   ret = fdt_tpm_kaslr_seed(root);
+   if (ret) {
+   printf("ERROR: fdt fixup KASLR failed: %d\n", ret);
+   goto err;
+   }
+   }
+
fdt_ret = optee_copy_fdt_nodes(blob);
if (fdt_ret) {
printf("ERROR: transfer of optee nodes to new fdt failed: %s\n",
diff --git a/common/fdt_support.c b/common/fdt_support.c
index 52be4375b4..d338fcde54 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -13,6 +13,9 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -650,6 +653,33 @@ int fdt_fixup_kaslr_seed(ofnode node, const u8 *seed, int 
len)
return 0;
 }
 
+int fdt_tpm_kaslr_seed(ofnode node)
+{
+   u8 rand[8] = {0};
+   struct udevice *dev;
+   int ret;
+
+   ret = uclass_first_device_err(UCLASS_TPM, );
+   if (ret) {
+   printf("ERROR: Failed to find TPM device\n");
+   return ret;
+   }
+
+   ret = tpm_get_random(dev, rand, sizeof(rand));
+   if (ret) {
+   printf("ERROR: TPM GetRandom failed, ret=%d\n", ret);
+   return ret;
+   }
+
+   ret = fdt_fixup_kaslr_seed(node, rand, sizeof(rand));
+   if (ret) {
+   printf("ERROR: failed to add kaslr-seed to fdt\n");
+   return ret;
+   }
+
+   return 0;
+}
+
 int fdt_record_loadable(void *blob, u32 index, const char *name,
uintptr_t load_addr, u32 size, uintptr_t entry_point,
const char *type, const char *os, const char *arch)
diff --git a/include/fdt_support.h b/include/fdt_support.h
index d967118bed..117ca14ca5 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -130,6 +130,14 @@ void fdt_fixup_ethernet(void *fdt);
  */
 int fdt_fixup_kaslr_seed(ofnode node, const u8 *seed, int len);
 
+/*
+ * fdt_add_tpm_kaslr_seed - Add kalsr-seed node in Device tree with random
+ * bytes from TPM device
+ * @node:  ofnode
+ * @eret:  0 for success
+ */
+int fdt_tpm_kaslr_seed(ofnode node);
+
 int fdt_find_and_setprop(void *fdt, const char *node, const char *prop,
 const void *val, int len, int create);
 void fdt_fixup_qe_firmware(void *fdt);
diff --git a/lib/Kconfig b/lib/Kconfig
index 3926652db6..1530ef7c86 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -465,6 +465,15 @@ config VPL_TPM
  for the low-level TPM interface, but only one TPM is supported at
  a time by the TPM library.
 
+config KASLR_TPM_SEED
+   bool "Use TPM for KASLR random seed"
+   depends on TPM_V1 || TPM_V2
+   help
+ This enables support for using TPMs as entropy source for KASLR seed
+ populated in kernel's device tree. Both TPMv1 and TPMv2 are supported
+ for the low-level TPM interface, but only one TPM is supported at
+ a time by the library.
+
 endmenu
 
 menu "Android Verified Boot"
-- 
2.40.0



[PATCH v2 4/4] dm: core: Modify default for OFNODE_MULTI_TREE

2023-08-29 Thread seanedmond
From: Sean Edmond 

There is a preference to use the "ofnode" API for FDT fixups
moving forward.  The FDT fixup will usually be for the kernel FDT.  To
fixup the kernel FDT with the ofnode API, it's required to set the
OFNODE_MULTI_TREE option.

To ensure existing users of kaslr fdt fixup are not impacted, Let's modify
the default value for OFNODE_MULTI_TREE to ensure it's always set if
!OF_LIVE.  This will cause a 1007 byte increase in the code size.

Signed-off-by: Sean Edmond 
---
 drivers/core/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
index f0d848f45d..38e44ef6fc 100644
--- a/drivers/core/Kconfig
+++ b/drivers/core/Kconfig
@@ -424,7 +424,7 @@ config DM_DEV_READ_INLINE
 
 config OFNODE_MULTI_TREE
bool "Allow the ofnode interface to access any tree"
-   default y if EVENT && !DM_DEV_READ_INLINE && !DM_INLINE_OFNODE
+   default y if !OF_LIVE
help
  Normally U-Boot makes use of its control FDT, the one used to bind
  devices and provide options. In some cases, U-Boot must also process
-- 
2.40.0



[PATCH v2 3/4] cmd: kaslrseed: Use common API to fixup FDT

2023-08-29 Thread seanedmond
From: Sean Edmond 

Use the newly introduced common API fdt_fixup_kaslr_seed() in the
kaslrseed command.

Signed-off-by: Sean Edmond 
---
 cmd/kaslrseed.c | 22 --
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/cmd/kaslrseed.c b/cmd/kaslrseed.c
index 8a1d8120cd..c65607619b 100644
--- a/cmd/kaslrseed.c
+++ b/cmd/kaslrseed.c
@@ -19,7 +19,7 @@ static int do_kaslr_seed(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const
size_t n = 0x8;
struct udevice *dev;
u64 *buf;
-   int nodeoffset;
+   ofnode root;
int ret = CMD_RET_SUCCESS;
 
if (uclass_get_device(UCLASS_RNG, 0, ) || !dev) {
@@ -45,21 +45,15 @@ static int do_kaslr_seed(struct cmd_tbl *cmdtp, int flag, 
int argc, char *const
return CMD_RET_FAILURE;
}
 
-   ret = fdt_check_header(working_fdt);
-   if (ret < 0) {
-   printf("fdt_chosen: %s\n", fdt_strerror(ret));
-   return CMD_RET_FAILURE;
-   }
-
-   nodeoffset = fdt_find_or_add_subnode(working_fdt, 0, "chosen");
-   if (nodeoffset < 0) {
-   printf("Reading chosen node failed\n");
-   return CMD_RET_FAILURE;
+   ret = root_ofnode_from_fdt(working_fdt, );
+   if (ret) {
+   printf("ERROR: Unable to get root ofnode\n");
+   goto CMD_RET_FAILURE;
}
 
-   ret = fdt_setprop(working_fdt, nodeoffset, "kaslr-seed", buf, 
sizeof(buf));
-   if (ret < 0) {
-   printf("Unable to set kaslr-seed on chosen node: %s\n", 
fdt_strerror(ret));
+   ret = fdt_fixup_kaslr_seed(root, buf, sizeof(buf));
+   if (ret) {
+   printf("ERROR: failed to add kaslr-seed to fdt\n");
return CMD_RET_FAILURE;
}
 
-- 
2.40.0



[PATCH 2/4] fdt: kaslr seed from tpm entropy

2023-08-29 Thread seanedmond
From: Dhananjay Phadke 

Add support for KASLR seed from TPM device. Invokes tpm_get_random()
API to read 8-bytes of random bytes for KASLR.

Signed-off-by: Dhananjay Phadke 
Signed-off-by: Drew Kluemke 
Signed-off-by: Sean Edmond 
---
 boot/image-fdt.c  | 15 +++
 common/fdt_support.c  | 30 ++
 include/fdt_support.h |  8 
 lib/Kconfig   |  9 +
 4 files changed, 62 insertions(+)

diff --git a/boot/image-fdt.c b/boot/image-fdt.c
index f10200f647..ed38ed77b9 100644
--- a/boot/image-fdt.c
+++ b/boot/image-fdt.c
@@ -624,6 +624,21 @@ int image_setup_libfdt(struct bootm_headers *images, void 
*blob,
goto err;
}
 
+   if (IS_ENABLED(CONFIG_KASLR_TPM_SEED)) {
+   ofnode root;
+
+   ret = root_ofnode_from_fdt(blob, );
+   if (ret) {
+   printf("ERROR: Unable to get root ofnode\n");
+   goto err;
+   }
+   ret = fdt_tpm_kaslr_seed(root);
+   if (ret) {
+   printf("ERROR: fdt fixup KASLR failed: %d\n", ret);
+   goto err;
+   }
+   }
+
fdt_ret = optee_copy_fdt_nodes(blob);
if (fdt_ret) {
printf("ERROR: transfer of optee nodes to new fdt failed: %s\n",
diff --git a/common/fdt_support.c b/common/fdt_support.c
index 52be4375b4..d338fcde54 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -13,6 +13,9 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -650,6 +653,33 @@ int fdt_fixup_kaslr_seed(ofnode node, const u8 *seed, int 
len)
return 0;
 }
 
+int fdt_tpm_kaslr_seed(ofnode node)
+{
+   u8 rand[8] = {0};
+   struct udevice *dev;
+   int ret;
+
+   ret = uclass_first_device_err(UCLASS_TPM, );
+   if (ret) {
+   printf("ERROR: Failed to find TPM device\n");
+   return ret;
+   }
+
+   ret = tpm_get_random(dev, rand, sizeof(rand));
+   if (ret) {
+   printf("ERROR: TPM GetRandom failed, ret=%d\n", ret);
+   return ret;
+   }
+
+   ret = fdt_fixup_kaslr_seed(node, rand, sizeof(rand));
+   if (ret) {
+   printf("ERROR: failed to add kaslr-seed to fdt\n");
+   return ret;
+   }
+
+   return 0;
+}
+
 int fdt_record_loadable(void *blob, u32 index, const char *name,
uintptr_t load_addr, u32 size, uintptr_t entry_point,
const char *type, const char *os, const char *arch)
diff --git a/include/fdt_support.h b/include/fdt_support.h
index d967118bed..117ca14ca5 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -130,6 +130,14 @@ void fdt_fixup_ethernet(void *fdt);
  */
 int fdt_fixup_kaslr_seed(ofnode node, const u8 *seed, int len);
 
+/*
+ * fdt_add_tpm_kaslr_seed - Add kalsr-seed node in Device tree with random
+ * bytes from TPM device
+ * @node:  ofnode
+ * @eret:  0 for success
+ */
+int fdt_tpm_kaslr_seed(ofnode node);
+
 int fdt_find_and_setprop(void *fdt, const char *node, const char *prop,
 const void *val, int len, int create);
 void fdt_fixup_qe_firmware(void *fdt);
diff --git a/lib/Kconfig b/lib/Kconfig
index 3926652db6..1530ef7c86 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -465,6 +465,15 @@ config VPL_TPM
  for the low-level TPM interface, but only one TPM is supported at
  a time by the TPM library.
 
+config KASLR_TPM_SEED
+   bool "Use TPM for KASLR random seed"
+   depends on TPM_V1 || TPM_V2
+   help
+ This enables support for using TPMs as entropy source for KASLR seed
+ populated in kernel's device tree. Both TPMv1 and TPMv2 are supported
+ for the low-level TPM interface, but only one TPM is supported at
+ a time by the library.
+
 endmenu
 
 menu "Android Verified Boot"
-- 
2.40.0



[PATCH v2 0/4] Populate kaslr seed with TPM

2023-08-29 Thread seanedmond
From: Sean Edmond 

This patch series creates a common API (fdt_fixup_kaslr_seed()) for 
populating the kaslr seed in the DTB.  Existing users (kaslrseed,
and ARMv8 sec firmware) have been updated to use this common API.

New functionality has been introduced to populate the kaslr using
the TPM interface.  This can be enabled with CONFIG_KASLR_TPM_SEED.  

changes in v2:
- fdt_fixup_kaslr_seed() uses the ofnode API
- Add root_ofnode_from_fdt() to get the root node from an FDT and
  perform error checking on the oftree
- add comments to exported functions
- Add error checking in image_setup_libfdt() for return from
  fdt_tpm_kaslr_seed()
- uclass_get_device() -> uclass_first_device_err()
- Change default config for OFNODE_MULTI_TREE (y if !OF_LIVE)

Dhananjay Phadke (2):
  fdt: common API to populate kaslr seed
  fdt: kaslr seed from tpm entropy

Sean Edmond (2):
  cmd: kaslrseed: Use common API to fixup FDT
  dm: core: Modify default for OFNODE_MULTI_TREE

 arch/arm/cpu/armv8/sec_firmware.c | 39 +---
 boot/image-fdt.c  | 15 ++
 cmd/kaslrseed.c   | 22 +-
 common/fdt_support.c  | 49 +++
 drivers/core/Kconfig  |  2 +-
 drivers/core/ofnode.c | 17 +++
 include/dm/ofnode.h   | 12 
 include/fdt_support.h | 17 +++
 lib/Kconfig   |  9 ++
 9 files changed, 142 insertions(+), 40 deletions(-)

-- 
2.40.0



[PATCH 4/4] dm: core: Modify default for OFNODE_MULTI_TREE

2023-08-29 Thread seanedmond
From: Sean Edmond 

There is a preference to use the "ofnode" API for FDT fixups
moving forward.  The FDT fixup will usually be for the kernel FDT.  To
fixup the kernel FDT with the ofnode API, it's required to set the
OFNODE_MULTI_TREE option.

To ensure exisiting users on kasls fixup are not impacted, Let's modify
the default value for OFNODE_MULTI_TREE to ensure it's always set if
!OF_LIVE.  This will cause a 1007 byte increase in the code size.

Signed-off-by: Sean Edmond 
---
 drivers/core/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
index f0d848f45d..38e44ef6fc 100644
--- a/drivers/core/Kconfig
+++ b/drivers/core/Kconfig
@@ -424,7 +424,7 @@ config DM_DEV_READ_INLINE
 
 config OFNODE_MULTI_TREE
bool "Allow the ofnode interface to access any tree"
-   default y if EVENT && !DM_DEV_READ_INLINE && !DM_INLINE_OFNODE
+   default y if !OF_LIVE
help
  Normally U-Boot makes use of its control FDT, the one used to bind
  devices and provide options. In some cases, U-Boot must also process
-- 
2.40.0



[PATCH 1/4] fdt: common API to populate kaslr seed

2023-08-29 Thread seanedmond
From: Dhananjay Phadke 

fdt_fixup_kaslr_seed() will update given FDT with random seed value.
Source for random seed can be TPM or RNG driver in u-boot or sec
firmware (ARM).

Signed-off-by: Dhananjay Phadke 
Signed-off-by: Sean Edmond 
---
 arch/arm/cpu/armv8/sec_firmware.c | 39 +++
 common/fdt_support.c  | 19 +++
 drivers/core/ofnode.c | 17 ++
 include/dm/ofnode.h   | 12 ++
 include/fdt_support.h |  9 +++
 5 files changed, 71 insertions(+), 25 deletions(-)

diff --git a/arch/arm/cpu/armv8/sec_firmware.c 
b/arch/arm/cpu/armv8/sec_firmware.c
index c0e8726346..5f04cd8aec 100644
--- a/arch/arm/cpu/armv8/sec_firmware.c
+++ b/arch/arm/cpu/armv8/sec_firmware.c
@@ -411,46 +411,35 @@ int sec_firmware_init(const void *sec_firmware_img,
 /*
  * fdt_fix_kaslr - Add kalsr-seed node in Device tree
  * @fdt:   Device tree
- * @eret:  0 in case of error, 1 for success
+ * @eret:  0 for success
  */
 int fdt_fixup_kaslr(void *fdt)
 {
-   int nodeoffset;
-   int err, ret = 0;
-   u8 rand[8];
+   int ret = 0;
 
 #if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT)
+   u8 rand[8];
+   ofnode root;
+
/* Check if random seed generation is  supported */
if (sec_firmware_support_hwrng() == false) {
printf("WARNING: SEC firmware not running, no kaslr-seed\n");
-   return 0;
+   return -EOPNOTSUPP;
}
 
-   err = sec_firmware_get_random(rand, 8);
-   if (err < 0) {
+   ret = sec_firmware_get_random(rand, 8);
+   if (ret < 0) {
printf("WARNING: No random number to set kaslr-seed\n");
-   return 0;
+   return ret;
}
 
-   err = fdt_check_header(fdt);
-   if (err < 0) {
-   printf("fdt_chosen: %s\n", fdt_strerror(err));
-   return 0;
+   ret = root_ofnode_from_fdt(fdt, );
+   if (ret < 0) {
+   printf("WARNING: Unable to get root ofnode\n");
+   return ret;
}
 
-   /* find or create "/chosen" node. */
-   nodeoffset = fdt_find_or_add_subnode(fdt, 0, "chosen");
-   if (nodeoffset < 0)
-   return 0;
-
-   err = fdt_setprop(fdt, nodeoffset, "kaslr-seed", rand,
- sizeof(rand));
-   if (err < 0) {
-   printf("WARNING: can't set kaslr-seed %s.\n",
-  fdt_strerror(err));
-   return 0;
-   }
-   ret = 1;
+   ret = fdt_fixup_kaslr_seed(root, rand, sizeof(rand));
 #endif
 
return ret;
diff --git a/common/fdt_support.c b/common/fdt_support.c
index 5e49078f8c..52be4375b4 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -631,6 +631,25 @@ void fdt_fixup_ethernet(void *fdt)
}
 }
 
+int fdt_fixup_kaslr_seed(ofnode node, const u8 *seed, int len)
+{
+   ofnode chosen;
+   int ret;
+
+   /* find or create "/chosen" node. */
+   ret = ofnode_add_subnode(node, "chosen", );
+   if (ret && ret != -EEXIST)
+   return -ENOENT;
+
+   ret = ofnode_write_prop(chosen, "kaslr-seed", seed, len, true);
+   if (ret) {
+   printf("WARNING: can't set kaslr-seed\n");
+   return ret;
+   }
+
+   return 0;
+}
+
 int fdt_record_loadable(void *blob, u32 index, const char *name,
uintptr_t load_addr, u32 size, uintptr_t entry_point,
const char *type, const char *os, const char *arch)
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index 8df16e56af..4be21133b8 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -870,6 +870,23 @@ ofnode oftree_path(oftree tree, const char *path)
}
 }
 
+int root_ofnode_from_fdt(void *fdt, ofnode *root_node)
+{
+   oftree tree;
+   /* If OFNODE_MULTI_TREE is not set, and if fdt is not the control FDT,
+*  oftree_from_fdt() will return NULL
+*/
+   tree = oftree_from_fdt(fdt);
+
+   if (!oftree_valid(tree)) {
+   printf("Cannot create oftree\n");
+   return -EINVAL;
+   }
+   *root_node = oftree_root(tree);
+
+   return 0;
+}
+
 const void *ofnode_read_chosen_prop(const char *propname, int *sizep)
 {
ofnode chosen_node;
diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
index 0f38b3e736..e79bb62be8 100644
--- a/include/dm/ofnode.h
+++ b/include/dm/ofnode.h
@@ -901,6 +901,18 @@ ofnode oftree_path(oftree tree, const char *path);
  */
 ofnode oftree_root(oftree tree);
 
+/**
+ * root_ofnode_from_fdt() - Gets the root ofnode given an FDT blob.
+ *  Note, this will fail if OFNODE_MULTI_TREE
+ *  is not set.
+ *
+ * @fdt: Device tree to use
+ * @root_node : Root ofnode
+ *
+ * Return: 0 if OK, -ve on error
+ */
+int root_ofnode_from_fdt(void *fdt, ofnode *root_node);

[PATCH 3/4] cmd: kaslrseed: Use common API to fixup FDT

2023-08-29 Thread seanedmond
From: Sean Edmond 

Use the newly introduced common API fdt_fixup_kaslr_seed() in the
kaslrseed command.

Signed-off-by: Sean Edmond 
---
 cmd/kaslrseed.c | 22 --
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/cmd/kaslrseed.c b/cmd/kaslrseed.c
index 8a1d8120cd..c65607619b 100644
--- a/cmd/kaslrseed.c
+++ b/cmd/kaslrseed.c
@@ -19,7 +19,7 @@ static int do_kaslr_seed(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const
size_t n = 0x8;
struct udevice *dev;
u64 *buf;
-   int nodeoffset;
+   ofnode root;
int ret = CMD_RET_SUCCESS;
 
if (uclass_get_device(UCLASS_RNG, 0, ) || !dev) {
@@ -45,21 +45,15 @@ static int do_kaslr_seed(struct cmd_tbl *cmdtp, int flag, 
int argc, char *const
return CMD_RET_FAILURE;
}
 
-   ret = fdt_check_header(working_fdt);
-   if (ret < 0) {
-   printf("fdt_chosen: %s\n", fdt_strerror(ret));
-   return CMD_RET_FAILURE;
-   }
-
-   nodeoffset = fdt_find_or_add_subnode(working_fdt, 0, "chosen");
-   if (nodeoffset < 0) {
-   printf("Reading chosen node failed\n");
-   return CMD_RET_FAILURE;
+   ret = root_ofnode_from_fdt(working_fdt, );
+   if (ret) {
+   printf("ERROR: Unable to get root ofnode\n");
+   goto CMD_RET_FAILURE;
}
 
-   ret = fdt_setprop(working_fdt, nodeoffset, "kaslr-seed", buf, 
sizeof(buf));
-   if (ret < 0) {
-   printf("Unable to set kaslr-seed on chosen node: %s\n", 
fdt_strerror(ret));
+   ret = fdt_fixup_kaslr_seed(root, buf, sizeof(buf));
+   if (ret) {
+   printf("ERROR: failed to add kaslr-seed to fdt\n");
return CMD_RET_FAILURE;
}
 
-- 
2.40.0



Re: [RFC PATCH 0/5] Allow for removal of DT nodes and properties

2023-08-29 Thread Simon Glass
Hi Peter,

On Tue, 29 Aug 2023 at 04:33, Peter Robinson  wrote:
>
> > > > > > > Provide a way for removing certain devicetree nodes and/or 
> > > > > > > properties
> > > > > > > from the devicetree. This is needed to purge certain nodes and
> > > > > > > properties which may be relevant only in U-Boot. Such nodes and
> > > > > > > properties are then removed from the devicetree before it is 
> > > > > > > passed to
> > > > > > > the kernel. This ensures that the devicetree passed to the OS 
> > > > > > > does not
> > > > > > > contain any non-compliant nodes and properties.
> > > > > > >
> > > > > > > The removal of the nodes and properties is being done through an
> > > > > > > EVT_FT_FIXUP handler. I am not sure if the removal code needs to 
> > > > > > > be
> > > > > > > behind any Kconfig symbol.
> > > > > > >
> > > > > > > I have only build tested this on sandbox, and tested on qemu arm64
> > > > > > > virt platform. This being a RFC, I have not put this through a CI 
> > > > > > > run.
> > > > > > >
> > > > > > > Sughosh Ganu (5):
> > > > > > >   dt: Provide a way to remove non-compliant nodes and properties
> > > > > > >   fwu: Add the fwu-mdata node for removal from devicetree
> > > > > > >   capsule: Add the capsule-key property for removal from 
> > > > > > > devicetree
> > > > > > >   bootefi: Call the EVT_FT_FIXUP event handler
> > > > > > >   doc: Add a document for non-compliant DT node/property removal
> > > > > > >
> > > > > > >  cmd/bootefi.c | 18 +
> > > > > > >  .../devicetree/dt_non_compliant_purge.rst | 64 
> > > > > > > 
> > > > > > >  drivers/fwu-mdata/fwu-mdata-uclass.c  |  5 ++
> > > > > > >  include/dt-structs.h  | 11 +++
> > > > > > >  lib/Makefile  |  1 +
> > > > > > >  lib/dt_purge.c| 73 
> > > > > > > +++
> > > > > > >  lib/efi_loader/efi_capsule.c  |  7 ++
> > > > > > >  7 files changed, 179 insertions(+)
> > > > > > >  create mode 100644 
> > > > > > > doc/develop/devicetree/dt_non_compliant_purge.rst
> > > > > > >  create mode 100644 lib/dt_purge.c
> > > > > >
> > > > > > What is the point of removing them? Instead, we should make sure 
> > > > > > that
> > > > > > we upstream the bindings and encourage SoC vendors to sync them. If 
> > > > > > we
> > > > > > remove them, no one will bother and U-Boot just becomes a dumping
> > > > > > ground.
> > > > >
> > > > > Well things like the binman entries in DT are U-Boot specific and not
> > > > > useful for HW related descriptions or for Linux or another OS being
> > > > > able to deal with HW so arguably we're already a dumping ground to
> > > > > some degree for not defining hardware.
> > > >
> > > > I have started the process to upstream the binman bindings.
> > >
> > > I don't think they should be in DT at all, they don't describe
> > > anything to do with hardware, or generally even the runtime of a
> > > device, they don't even describe the boot/runtime state of the
> > > firmware, they describe build time, so I don't see what that has to do
> > > with device tree? Can you explain that? To me those sorts of things
> > > should live in a build time style config file.
> >
> > I beg to differ. Devicetree is more than just hardware and always has
> > been. See, for example the /chosen and /options nodes.
>
> Can you give me an example of "options" as grep doesn't give me a
> single one in the kernel tree? I think we can just agree to disagree
> here.

See here: 
https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/options/u-boot.yaml

I don't mind disagreeing, so long as it doesn't result in any
restrictions on using devicetree in firmware. But it is very important
to the success of firmware that we can make full use of the
devicetree.

> > We need run-time configuration here, since we cannot know at build
> > time what we will be asked to do by a previous firmware phase.
>
> Can you provide an example as to how that is used during runtime? Do
> you mean runtime during the build process or runtime on the device?

I mean runtime on the device. An example is that we might want to
control whether the serial UART is enabled, without having to rebuild
each program in the firmware stack.

>
> > >
> > > > Perhaps we should use the issue tracker[1] to follow progress on all
> > > > of this. We need to clean it up.
> > > >
> > > > >
> > > > > > Instead of this, how about working on bringing the DT validation 
> > > > > > into
> > > > > > U-Boot so we can see what state things are in?
> > > > > >
> > > > > > Please send the bindings for Linaro's recent series (which I suspect
> > > > > > are motivating this series) so we can start the process.
> > > >
> > > > Regards,
> > > > Simon
> > > >
> > > > [1] https://source.denx.de/u-boot/u-boot/-/issues
> >
> > Regards,
> > Simon

Regards,
Simon


Re: [PATCH v3 1/2] schemas: Add a schema for memory map

2023-08-29 Thread Simon Glass
Hi Ard,

On Thu, 24 Aug 2023 at 03:10, Ard Biesheuvel  wrote:
>
> On Wed, 23 Aug 2023 at 22:04, Simon Glass  wrote:
> >
> > Hi,
> >
> > On Wed, 23 Aug 2023 at 08:24, Ard Biesheuvel  wrote:
> > >
> > > On Wed, 23 Aug 2023 at 10:59, Mark Rutland  wrote:
> > > >
> > > > On Tue, Aug 22, 2023 at 02:34:42PM -0600, Simon Glass wrote:
> > > > > The Devicetree specification skips over handling of a logical view of
> > > > > the memory map, pointing users to the UEFI specification.
> > > > >
> > > > > It is common to split firmware into 'Platform Init', which does the
> > > > > initial hardware setup and a "Payload" which selects the OS to be 
> > > > > booted.
> > > > > Thus an handover interface is required between these two pieces.
> > > > >
> > > > > Where UEFI boot-time services are not available, but UEFI firmware is
> > > > > present on either side of this interface, information about memory 
> > > > > usage
> > > > > and attributes must be presented to the "Payload" in some form.
> > >
> > > Not quite.
> > >
> > > This seems to be intended for consumption by Linux booting in ACPI
> > > mode, but not via UEFI, right?
> >
> > Actually, this is for consumption by firmware. The goal is to allow
> > edk2 to boot into U-Boot and vice versa, i.e. provide some
> > interoperability between firmware projects. I will use the "Platform
> > Init" and "Payload" terminology here too.
> >
>
> OK. It was the cc to linux-acpi@ and the authors of the
> ACPI/SMBIOS-without-UEFI patches that threw me off here.
>
> If we are talking about an internal interface for firmware components,
> I'd be inclined to treat this as an implementation detail, as long as
> the OS is not expected to consume these DT nodes.
>
> However, I struggle to see the point of framing this information as a
> 'UEFI memory map'. Neither EDK2 nor u-boot consume this information
> natively, and there is already prior art in both projects to consume
> nodes following the existing bindings for device_type=memory and the
> /reserved-memory node. UEFI runtime memory is generally useless
> without UEFI runtime services, and UEFI boot services memory is just
> free memory.
>
> There is also an overlap with the handover between secure and
> non-secure firmware on arm64, which is also DT based, and communicates
> available memory as well as RAM regions that are reserved for firmware
> use.
>
> In summary, I don't see why a non-UEFI payload would care about UEFI
> semantics for pre-existing memory reservations, or vice versa. Note
> that EDK2 will manage its own memory map, and expose it via UEFI boot
> services and not via DT.

Bear in mind that one or both sides of this interface may be UEFI.
There is no boot-services link between the two parts that I have
outlined.

>
> ...
> >
> > There is no intent to implement the UEFI spec, here. It is simply that
> > some payloads (EDK2) are used to having this information.
> >
> > Imagine splitting EDK2 into two parts, one of which does platform init
> > and the other which (the payload) boots the OS. The payload wants
> > information from Platform Init and it needs to be in a devicetree,
> > since that is what we have chosen for this interface. So to some
> > extent this is unrelated to whether you have EFI boot services. We
> > just need to be able to pass the information across the interface.
> > Note that the user can (without recompilation, etc.) replace the
> > second part with U-Boot (for example) and it must still work.
> >
>
> OK, so device tree makes sense for this. This is how I implemented the
> EDK2 port that targets QEMU/mach-virt - it consumes the DT to discover
> the UART, RTC,, memory, PCI host bridge, etc.
>
> But I don't see a use case for a UEFI memory map here.
>
>
> > >
> > > >
> > > > Today Linux does that by passing:
> > > >
> > > >   /chosen/linux,uefi-mmap-start
> > > >   /chosen/linux,uefi-mmap-size
> > > >   /chosen/linux,uefi-mmap-desc-size
> > > >   /chosen/linux,uefi-mmap-desc-ver
> > > >
> > > > ... or /chosen/xen,* variants of those.
> > > >
> > > > Can't we document / genericise that?
> >
> > That seems to me to be the fields from the EFI memory-map call, but
> > where is the actual content? I looked in the kernel but it seems to be
> > an internal interface (between the stub and the kernel)?
> >
> > > >
> > >
> > > Given the ACPI angle, promoting this to external ABI would introduce a
> > > DT dependency to ACPI boot. So we'll at least have to be very clear
> > > about which takes precedence, or maybe disregard everything except the
> > > /chosen node when doing ACPI boot?
> > >
> > > This also argues for not creating an ordinary binding for this (i.e.,
> > > describing it as part of the platform topology), but putting it under
> > > /chosen as a Linux-only boot tweak.
> > >
> > > > Pointing to that rather than re-encoding it in DT means that it stays 
> > > > in-sync
> > > > with the EFI spec and we won't back ourselves into a corner where we 
> > > > cannot
> > > > encode something due to a 

[PATCH v4 4/4] memory: Add ECC property

2023-08-29 Thread Simon Glass
Some memories provides ECC correction. For software which wants to check
memory, it is helpful to see which regions provide this feature.

Add this as a property of the /memory nodes, since it presumably follows
the hardware-level memory system.

Signed-off-by: Simon Glass 
---

(no changes since v3)

Changes in v3:
- Add new patch to update the /memory nodes

 dtschema/schemas/memory.yaml | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/dtschema/schemas/memory.yaml b/dtschema/schemas/memory.yaml
index 1d74410..981af04 100644
--- a/dtschema/schemas/memory.yaml
+++ b/dtschema/schemas/memory.yaml
@@ -34,7 +34,14 @@ patternProperties:
 description:
   For the purpose of identification, each NUMA node is associated with
   a unique token known as a node id.
-
+  attr:
+$ref: /schemas/types.yaml#/definitions/string-array
+description: |
+  Attributes possessed by this memory region:
+
+"single-bit-ecc" - supports single-bit ECC
+"multi-bit-ecc" - supports multiple-bit ECC
+"no-ecc" - non-ECC memory
 
 required:
   - device_type
-- 
2.42.0.rc2.253.gd59a3bf2b4-goog



[PATCH v4 3/4] schemas: Add a schema for memory map

2023-08-29 Thread Simon Glass
The Devicetree specification skips over handling of a logical view of
the memory map, pointing users to the UEFI specification.

It is common to split firmware into 'Platform Init', which does the
initial hardware setup and a "Payload" which selects the OS to be booted.
Thus an handover interface is required between these two pieces.

Where UEFI boot-time services are not available, but UEFI firmware is
present on either side of this interface, information about memory usage
and attributes must be presented to the "Payload" in some form.

This aims to provide an initial schema for this mapping.

Note that this is separate from the existing /memory and /reserved-memory
nodes, since it is mostly concerned with what the memory is used for. It
may cover only a small fraction of available memory.

For now, no attempt is made to create an exhaustive binding, so there are
some example types listed. This can be completed once this has passed
initial review.

The compatible string is not included, since the node name is enough to
indicate the purpose of a node, as per the existing reserved-memory
schema.

This binding does not include a binding for the memory 'attribute'
property, defined by EFI_BOOT_SERVICES.GetMemoryMap(). It may be useful
to have that as well, but perhaps not as a bit mask.

Signed-off-by: Simon Glass 
---

Changes in v4:
- Make use of the reserved-memory node instead of creating a new one

Changes in v3:
- Reword commit message again
- cc a lot more people, from the FFI patch
- Split out the attributes into the /memory nodes

Changes in v2:
- Reword commit message

 dtschema/schemas/memory-map.yaml  | 61 +++
 .../reserved-memory/common-reserved.yaml  | 53 
 2 files changed, 114 insertions(+)
 create mode 100644 dtschema/schemas/memory-map.yaml
 create mode 100644 dtschema/schemas/reserved-memory/common-reserved.yaml

diff --git a/dtschema/schemas/memory-map.yaml b/dtschema/schemas/memory-map.yaml
new file mode 100644
index 000..4b06583
--- /dev/null
+++ b/dtschema/schemas/memory-map.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: BSD-2-Clause
+# Copyright 2023 Google LLC
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/memory-map.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: /memory-map nodes
+description: |
+  Common properties always required in /memory-map nodes. These nodes are
+  intended to resolve the nonchalant clause 3.4.1 ("/memory node and UEFI")
+  in the Devicetree Specification.
+
+maintainers:
+  - Simon Glass 
+
+properties:
+  $nodename:
+const: 'memory-map'
+
+patternProperties:
+  "^([a-z][a-z0-9\\-]+@[0-9a-f]+)?$":
+type: object
+additionalProperties: false
+
+properties:
+  reg:
+minItems: 1
+maxItems: 1024
+
+  usage:
+$ref: /schemas/types.yaml#/definitions/string
+description: |
+  Describes the usage of the memory region, e.g.:
+
+"acpi-reclaim", "acpi-nvs", "bootcode", "bootdata", "bootdata",
+"runtime-code", "runtime-data".
+
+See enum EFI_MEMORY_TYPE in "Unified Extensible Firmware Interface
+(UEFI) Specification" for all the types. For now there are not
+listed here.
+
+required:
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+memory-map {
+acpi@f {
+reg = <0xf 0x4000>;
+usage = "acpi-reclaim";
+};
+
+runtime@1230 {
+reg = <0x1230 0x28000>;
+usage = "runtime-code";
+};
+};
+...
diff --git a/dtschema/schemas/reserved-memory/common-reserved.yaml 
b/dtschema/schemas/reserved-memory/common-reserved.yaml
new file mode 100644
index 000..70cdead
--- /dev/null
+++ b/dtschema/schemas/reserved-memory/common-reserved.yaml
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reserved-memory/common-reserved.yaml#
+$schema: http://devicetree.org/meta-schemas/base.yaml#
+
+title: Common memory reservations
+
+description: |
+  Specifies that the reserved memory region can be used for the purpose
+  indicated by its node name.
+
+  Clients may reuse this reserved memory if they understand what it is for.
+
+maintainers:
+  - Simon Glass 
+
+# always select the core schema
+select: true
+
+properties:
+$nodename:
+oneOf:
+  - acpi-reclaim
+  - acpi-nvs
+  - boot-code
+  - boot-data
+  - runtime-code
+  - runtime-data
+
+  reg:
+description: region of memory that is reserved for the purpose indicated
+  by the node name.
+
+required:
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+reserved-memory {
+#address-cells = <1>;
+#size-cells = <1>;
+
+boot-code@1234 {
+reg = <0x1234 0x0080>;
+};
+
+boot-data@4321 {
+reg = <0x4321 0x0080>;
+

[PATCH v4 2/4] Bring in other reserved-memory files

2023-08-29 Thread Simon Glass
Add those files from v6.5 which are already converted to yaml.

Signed-off-by: Simon Glass 
---

Changes in v4:
- New patch

 .../schemas/reserved-memory/framebuffer.yaml  |  52 +++
 .../reserved-memory/google,open-dice.yaml |  46 ++
 .../reserved-memory/memory-region.yaml|  40 +
 .../nvidia,tegra210-emc-table.yaml|  31 
 .../nvidia,tegra264-bpmp-shmem.yaml   |  47 ++
 dtschema/schemas/reserved-memory/phram.yaml   |  47 ++
 .../schemas/reserved-memory/qcom,cmd-db.yaml  |  46 ++
 .../reserved-memory/qcom,rmtfs-mem.yaml   |  55 +++
 dtschema/schemas/reserved-memory/ramoops.yaml | 144 ++
 .../reserved-memory/shared-dma-pool.yaml  |  97 
 10 files changed, 605 insertions(+)
 create mode 100644 dtschema/schemas/reserved-memory/framebuffer.yaml
 create mode 100644 dtschema/schemas/reserved-memory/google,open-dice.yaml
 create mode 100644 dtschema/schemas/reserved-memory/memory-region.yaml
 create mode 100644 
dtschema/schemas/reserved-memory/nvidia,tegra210-emc-table.yaml
 create mode 100644 
dtschema/schemas/reserved-memory/nvidia,tegra264-bpmp-shmem.yaml
 create mode 100644 dtschema/schemas/reserved-memory/phram.yaml
 create mode 100644 dtschema/schemas/reserved-memory/qcom,cmd-db.yaml
 create mode 100644 dtschema/schemas/reserved-memory/qcom,rmtfs-mem.yaml
 create mode 100644 dtschema/schemas/reserved-memory/ramoops.yaml
 create mode 100644 dtschema/schemas/reserved-memory/shared-dma-pool.yaml

diff --git a/dtschema/schemas/reserved-memory/framebuffer.yaml 
b/dtschema/schemas/reserved-memory/framebuffer.yaml
new file mode 100644
index 000..851ec24
--- /dev/null
+++ b/dtschema/schemas/reserved-memory/framebuffer.yaml
@@ -0,0 +1,52 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reserved-memory/framebuffer.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: /reserved-memory framebuffer node
+
+maintainers:
+  - devicetree-s...@vger.kernel.org
+
+allOf:
+  - $ref: reserved-memory.yaml
+
+properties:
+  compatible:
+const: framebuffer
+description: >
+  This indicates a region of memory meant to be used as a framebuffer for
+  a set of display devices. It can be used by an operating system to keep
+  the framebuffer from being overwritten and use it as the backing memory
+  for a display device (such as simple-framebuffer).
+
+unevaluatedProperties: false
+
+examples:
+  - |
+/ {
+compatible = "foo";
+model = "foo";
+#address-cells = <1>;
+#size-cells = <1>;
+
+chosen {
+framebuffer {
+compatible = "simple-framebuffer";
+memory-region = <>;
+};
+};
+
+reserved-memory {
+#address-cells = <1>;
+#size-cells = <1>;
+ranges;
+
+fb: framebuffer@8000 {
+compatible = "framebuffer";
+reg = <0x8000 0x007e9000>;
+};
+};
+};
+...
diff --git a/dtschema/schemas/reserved-memory/google,open-dice.yaml 
b/dtschema/schemas/reserved-memory/google,open-dice.yaml
new file mode 100644
index 000..c591ec3
--- /dev/null
+++ b/dtschema/schemas/reserved-memory/google,open-dice.yaml
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reserved-memory/google,open-dice.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Open Profile for DICE
+
+description: |
+  This binding represents a reserved memory region containing data
+  generated by the Open Profile for DICE protocol.
+
+  See https://pigweed.googlesource.com/open-dice/
+
+maintainers:
+  - David Brazdil 
+
+allOf:
+  - $ref: reserved-memory.yaml
+
+properties:
+  compatible:
+const: google,open-dice
+
+  reg:
+description: page-aligned region of memory containing DICE data
+
+required:
+  - compatible
+  - reg
+  - no-map
+
+unevaluatedProperties: false
+
+examples:
+  - |
+reserved-memory {
+#address-cells = <2>;
+#size-cells = <1>;
+
+dice: dice@1234 {
+compatible = "google,open-dice";
+reg = <0x00 0x1234 0x2000>;
+no-map;
+};
+};
diff --git a/dtschema/schemas/reserved-memory/memory-region.yaml 
b/dtschema/schemas/reserved-memory/memory-region.yaml
new file mode 100644
index 000..592f180
--- /dev/null
+++ b/dtschema/schemas/reserved-memory/memory-region.yaml
@@ -0,0 +1,40 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reserved-memory/memory-region.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Reserved Memory Region
+
+maintainers:
+  - devicetree-s...@vger.kernel.org
+
+description: |
+  Regions in the /reserved-memory node may be referenced by other device
+  nodes by adding 

[PATCH v4 1/4] Add reserved-memory

2023-08-29 Thread Simon Glass
Bring in this file from Linux v6.5

Signed-off-by: Simon Glass 
---

Changes in v4:
- New patch

 .../reserved-memory/reserved-memory.yaml  | 181 ++
 1 file changed, 181 insertions(+)
 create mode 100644 dtschema/schemas/reserved-memory/reserved-memory.yaml

diff --git a/dtschema/schemas/reserved-memory/reserved-memory.yaml 
b/dtschema/schemas/reserved-memory/reserved-memory.yaml
new file mode 100644
index 000..c680e39
--- /dev/null
+++ b/dtschema/schemas/reserved-memory/reserved-memory.yaml
@@ -0,0 +1,181 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reserved-memory/reserved-memory.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: /reserved-memory Child Node Common
+
+maintainers:
+  - devicetree-s...@vger.kernel.org
+
+description: >
+  Reserved memory is specified as a node under the /reserved-memory node. The
+  operating system shall exclude reserved memory from normal usage one can
+  create child nodes describing particular reserved (excluded from normal use)
+  memory regions. Such memory regions are usually designed for the special
+  usage by various device drivers.
+
+  Each child of the reserved-memory node specifies one or more regions
+  of reserved memory. Each child node may either use a 'reg' property to
+  specify a specific range of reserved memory, or a 'size' property with
+  optional constraints to request a dynamically allocated block of
+  memory.
+
+  Following the generic-names recommended practice, node names should
+  reflect the purpose of the node (ie. "framebuffer" or "dma-pool").
+  Unit address (@) should be appended to the name if the node
+  is a static allocation.
+
+properties:
+  reg: true
+
+  size:
+oneOf:
+  - $ref: /schemas/types.yaml#/definitions/uint32
+  - $ref: /schemas/types.yaml#/definitions/uint64
+description: >
+  Length based on parent's \#size-cells. Size in bytes of memory to
+  reserve.
+
+  alignment:
+oneOf:
+  - $ref: /schemas/types.yaml#/definitions/uint32
+  - $ref: /schemas/types.yaml#/definitions/uint64
+description: >
+  Length based on parent's \#size-cells. Address boundary for
+  alignment of allocation.
+
+  alloc-ranges:
+$ref: /schemas/types.yaml#/definitions/uint32-array
+description: >
+  Address and Length pairs. Specifies regions of memory that are
+  acceptable to allocate from.
+
+  iommu-addresses:
+$ref: /schemas/types.yaml#/definitions/phandle-array
+description: >
+  A list of phandle and specifier pairs that describe static IO virtual
+  address space mappings and carveouts associated with a given reserved
+  memory region. The phandle in the first cell refers to the device for
+  which the mapping or carveout is to be created.
+
+  The specifier consists of an address/size pair and denotes the IO
+  virtual address range of the region for the given device. The exact
+  format depends on the values of the "#address-cells" and "#size-cells"
+  properties of the device referenced via the phandle.
+
+  When used in combination with a "reg" property, an IOVA mapping is to
+  be established for this memory region. One example where this can be
+  useful is to create an identity mapping for physical memory that the
+  firmware has configured some hardware to access (such as a bootsplash
+  framebuffer).
+
+  If no "reg" property is specified, the "iommu-addresses" property
+  defines carveout regions in the IOVA space for the given device. This
+  can be useful if a certain memory region should not be mapped through
+  the IOMMU.
+
+  no-map:
+type: boolean
+description: >
+  Indicates the operating system must not create a virtual mapping
+  of the region as part of its standard mapping of system memory,
+  nor permit speculative access to it under any circumstances other
+  than under the control of the device driver using the region.
+
+  reusable:
+type: boolean
+description: >
+  The operating system can use the memory in this region with the
+  limitation that the device driver(s) owning the region need to be
+  able to reclaim it back. Typically that means that the operating
+  system can use that region to store volatile or cached data that
+  can be otherwise regenerated or migrated elsewhere.
+
+allOf:
+  - if:
+  required:
+- no-map
+
+then:
+  not:
+required:
+  - reusable
+
+  - if:
+  required:
+- reusable
+
+then:
+  not:
+required:
+  - no-map
+
+oneOf:
+  - oneOf:
+  - required:
+  - reg
+
+  - required:
+  - size
+
+  - oneOf:
+  # IOMMU reservations
+  - required:
+  - iommu-addresses
+
+  # IOMMU mappings
+  - required:
+  - reg
+  - iommu-addresses
+
+additionalProperties: true
+

Re: [PATCH] arm: mach-apple: Move M1/M2 specifics into a separate folder

2023-08-29 Thread Ivaylo Ivanov
I'm currently working on S5L8950X. I also have some T7000 and T8010
devices that I'll work on in the future.

There's a project called freemyipod that has a fork of U-Boot working on
iPod Nano's (specifically the S5L8730 in the iPod Nano 5). I've read
that they're planning on upstreaming it in the near future.

On 8/29/23 20:43, Mark Kettenis wrote:
>> From: ivo.iva...@null.net
>> Date: Tue, 29 Aug 2023 20:25:19 +0300
>>
>> From: Ivaylo Ivanov 
>>
>> Currently, mach-apple assumes we're working with M1/M2. Make room for
>> adding support for other Apple SoCs by moving everything from the M1/M2
>> SoC family in "mach-apple/" into "mach-apple/m1/".
>
> Which other Apple SoCs?
>
>> Signed-off-by: Ivaylo Ivanov 
>> ---
>>  arch/arm/Kconfig | 29 --
>>  arch/arm/mach-apple/Kconfig  | 42 
>>  arch/arm/mach-apple/Makefile |  5 +--
>>  arch/arm/mach-apple/m1/Makefile  |  6 +++
>>  arch/arm/mach-apple/{ => m1}/board.c |  0
>>  arch/arm/mach-apple/{ => m1}/lowlevel_init.S |  0
>>  arch/arm/mach-apple/{ => m1}/rtkit.c |  0
>>  arch/arm/mach-apple/{ => m1}/sart.c  |  0
>>  configs/apple_m1_defconfig   |  1 +
>>  9 files changed, 50 insertions(+), 33 deletions(-)
>>  create mode 100644 arch/arm/mach-apple/m1/Makefile
>>  rename arch/arm/mach-apple/{ => m1}/board.c (100%)
>>  rename arch/arm/mach-apple/{ => m1}/lowlevel_init.S (100%)
>>  rename arch/arm/mach-apple/{ => m1}/rtkit.c (100%)
>>  rename arch/arm/mach-apple/{ => m1}/sart.c (100%)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 97c25b4f14..5339da370c 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -980,38 +980,9 @@ config ARCH_NPCM
>>
>>  config ARCH_APPLE
>>  bool "Apple SoCs"
>> -select ARM64
>> -select CLK
>> -select CMD_PCI
>> -select CMD_USB
>>  select DM
>> -select DM_GPIO
>> -select DM_KEYBOARD
>> -select DM_MAILBOX
>>  select DM_RESET
>>  select DM_SERIAL
>> -select DM_SPI
>> -select DM_USB
>> -select VIDEO
>> -select IOMMU
>> -select LINUX_KERNEL_IMAGE_HEADER
>> -select OF_BOARD_SETUP
>> -select OF_CONTROL
>> -select PCI
>> -select PINCTRL
>> -select POSITION_INDEPENDENT
>> -select POWER_DOMAIN
>> -select REGMAP
>> -select SPI
>> -select SYSCON
>> -select SYSRESET
>> -select SYSRESET_WATCHDOG
>> -select SYSRESET_WATCHDOG_AUTO
>> -select USB
>> -imply CMD_DM
>> -imply CMD_GPT
>> -imply DISTRO_DEFAULTS
>> -imply OF_HAS_PRIOR_STAGE
>>
>>  config ARCH_OWL
>>  bool "Actions Semi OWL SoCs"
>> diff --git a/arch/arm/mach-apple/Kconfig b/arch/arm/mach-apple/Kconfig
>> index 294690ec0e..a38779b387 100644
>> --- a/arch/arm/mach-apple/Kconfig
>> +++ b/arch/arm/mach-apple/Kconfig
>> @@ -3,6 +3,46 @@ if ARCH_APPLE
>>  config TEXT_BASE
>>  default 0x
>>
>> +choice
>> +prompt "Apple Silicon architecture type select"
>> +optional
>> +
>> +config ARCH_APPLE_M1
>> +bool "Apple M1/M2 SoC family"
>> +select ARM64
>> +select CLK
>> +select CMD_PCI
>> +select CMD_USB
>> +select DM_GPIO
>> +select DM_KEYBOARD
>> +select DM_MAILBOX
>> +select DM_SPI
>> +select DM_USB
>> +select VIDEO
>> +select IOMMU
>> +select LINUX_KERNEL_IMAGE_HEADER
>> +select OF_BOARD_SETUP
>> +select OF_CONTROL
>> +select PCI
>> +select PINCTRL
>> +select POSITION_INDEPENDENT
>> +select POWER_DOMAIN
>> +select REGMAP
>> +select SPI
>> +select SYSCON
>> +select SYSRESET
>> +select SYSRESET_WATCHDOG
>> +select SYSRESET_WATCHDOG_AUTO
>> +select USB
>> +imply CMD_DM
>> +imply CMD_GPT
>> +imply DISTRO_DEFAULTS
>> +imply OF_HAS_PRIOR_STAGE
>> +
>> +endchoice
>> +
>> +if ARCH_APPLE_M1
>> +
>>  config SYS_CONFIG_NAME
>>  default "apple"
>>
>> @@ -19,3 +59,5 @@ config LNX_KRNL_IMG_TEXT_OFFSET_BASE
>>  default TEXT_BASE
>>
>>  endif
>> +
>> +endif
>> diff --git a/arch/arm/mach-apple/Makefile b/arch/arm/mach-apple/Makefile
>> index 50b465b947..d147ccdde2 100644
>> --- a/arch/arm/mach-apple/Makefile
>> +++ b/arch/arm/mach-apple/Makefile
>> @@ -1,6 +1,3 @@
>>  # SPDX-License-Identifier: GPL-2.0+
>>
>> -obj-y += board.o
>> -obj-y += lowlevel_init.o
>> -obj-y += rtkit.o
>> -obj-$(CONFIG_NVME_APPLE) += sart.o
>> +obj-$(CONFIG_ARCH_APPLE_M1) += m1/
>> diff --git a/arch/arm/mach-apple/m1/Makefile 
>> b/arch/arm/mach-apple/m1/Makefile
>> new file mode 100644
>> index 00..50b465b947
>> --- /dev/null
>> +++ b/arch/arm/mach-apple/m1/Makefile
>> @@ -0,0 +1,6 @@
>> +# SPDX-License-Identifier: GPL-2.0+
>> +
>> +obj-y += board.o
>> +obj-y += lowlevel_init.o
>> +obj-y += rtkit.o
>> +obj-$(CONFIG_NVME_APPLE) += sart.o
>> diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/m1/board.c
>> similarity index 100%
>> rename from arch/arm/mach-apple/board.c
>> rename 

Re: Config fragments

2023-08-29 Thread Simon Glass
Hi Andrew,

On Tue, 29 Aug 2023 at 11:05, Andrew Davis  wrote:
>
> On 8/29/23 11:47 AM, Simon Glass wrote:
> > Hi Andrew,
> >
> > On Wed, 23 Aug 2023 at 10:44, Andrew Davis  wrote:
> >>
> >> On 8/23/23 10:30 AM, Simon Glass wrote:
> >>> Hi,
> >>>
> >>> Up until 2023.04 it has been possible to build all the defconfigs but
> >>> with 2023.07 that changed. Tom mentioned this to me recently.
> >>>
> >>> Up until 2023.04 we can enumerate all the board configs that can be
> >>> built. We can build any of them using a single name and a single
> >>> defconfig. The boards.cfg file which buildman creates contains a full
> >>> list of things that can be built.
> >>>
> >>>   From 2023.07 this changes and we now have random .config files
> >>> sprinkled about the place. I say random because they are not connected
> >>> to anything. For example, here is
> >>> board/ti/am62x/MAINTAINERS:
> >>>
> >>> AM62x BOARD
> >>> M: Dave Gerlach 
> >>> M: Tom Rini 
> >>> S: Maintained
> >>> F: board/ti/am62x/
> >>> F: include/configs/am62x_evm.h
> >>> F: configs/am62x_evm_r5_defconfig
> >>> F: configs/am62x_evm_a53_defconfig
> >>>
> >>> BEAGLEPLAY BOARD
> >>> M: Nishanth Menon 
> >>> M: Robert Nelson 
> >>> M: Tom Rini 
> >>> S: Maintained
> >>> N: beagleplay
> >>>
> >>> In most cases the MAINTAINERS file tells us about each board and until
> >>> [1] I had assumed that was the case. With that patch reverted, these
> >>> are the only 'orphaned' MAINTAINERS entries (buildman
> >>> --maintainer-check):
> >>>
> >>> WARNING: orphaned defconfig in board/armltd/vexpress64/MAINTAINERS
> >>> ending at line 8
> >>> WARNING: orphaned defconfig in board/google/veyron/MAINTAINERS ending at 
> >>> line 44
> >>> WARNING: orphaned defconfig in
> >>> board/mikrotik/crs3xx-98dx3236/MAINTAINERS ending at line 7
> >>> WARNING: orphaned defconfig in board/st/common/MAINTAINERS ending at line 
> >>> 6
> >>> WARNING: orphaned defconfig in board/ti/am62x/MAINTAINERS ending at line 
> >>> 15
> >>>
> >>> But Tom advised me that MAINTAINERS is not intended for this purpose,
> >>> so the check has been dropped. I am not suggesting we bring it back,
> >>> necessarily, but if we did, we could use it to specify the .config and
> >>> defconfig files which are used by each board.
> >>>
> >>> *Failing that*, I suggest a new 'name.brd' file in the board directory
> >>> to contain this information, e.g.
> >>>
> >>> # Contents of board/ti/am62x/beagleplay.brd:
> >>> beagleplay-r5: am62x_evm_r5_defconfig beagleplay_r5.config
> >>> beagleplay-a53: am62x_evm_a53_defconfig beagleplay_a53.config
> >>>
> >>> This could be parsed by buildman and other tools, to produce a list of
> >>> available boards and to build each using a single name (e.g. make
> >>> beagleplay-r5_defconfig)
> >>>
> >>> What do people think?
> >>>
> >>
> >> How about instead of needing this new 'name.brd' files, we look into
> >> "include" directives inside these configs? So we could have a file:
> >>
> >> beagleplay_r5_defconfig
> >>
> >> in the normal configs/ directory, but with the contents:
> >>
> >> #include "configs/am62x_evm_r5_defconfig"
> >>
> >> CONFIG_DEFAULT_DEVICE_TREE="k3-am625-beagleplay"
> >> CONFIG_OF_LIST="k3-am625-beagleplay"
> >> ...
> >>
> >> The # is already a comment line so these should be safe
> >> for existing tools, and then we have in our Makefile
> >> an automatic pass through the C preprocessor.
> >>
> >> Some boards have can build with and without the fragments,
> >> so to have complete CI coverage, we have dummy defconfigs
> >> that have both the base and a fragment include:
> >>
> >> #include "configs/am62x_evm_r5_defconfig"
> >> #include "board/ti/am62x/high_security.config"
> >>
> >> Something like that, then as Heinrich mentioned, simply
> >> enumerating configs/*defconfig should yield all valid
> >> combinations for building/testing.
> >
> > I do agree it would be nice to have this information in the file it
> > relates to. But wouldn't that involve changing kconf and other tools?
> > What tools will parse those files? We really want 'make xxx_defconfig'
> > to work for these new 'combined config' boards and my understanding is
> > that kconf is used here.
>
> The U-Boot Makefile passes the provided xxx_defconfig into the kconfig
> scripts. All I'm suggesting is to have that Makefile run the passed
> in defconfig through the C preprocessor before handing it off to kconfig.

Could you take a look to see how hard that would be?

>
> For existing defconfigs the preprocessor will make no changes to the file.
> For the config fragments, the `#include` lines will be substituted with the
> contents, the result will be a normal defconfig file that can then also be
> passed into kconfig.

OK

>
> No changes to kconfig scripts, or anything other than the Makefile, are 
> needed.
>
> >
> > Your proposal certainly allows each 'combined config' to have a name -
> > it is just the filename of the defconfig file.
> >
>
> Exactly, this keeps all current CI 

Re: [PATCH] arm: mach-apple: Move M1/M2 specifics into a separate folder

2023-08-29 Thread Mark Kettenis
> From: ivo.iva...@null.net
> Date: Tue, 29 Aug 2023 20:25:19 +0300
> 
> From: Ivaylo Ivanov 
> 
> Currently, mach-apple assumes we're working with M1/M2. Make room for
> adding support for other Apple SoCs by moving everything from the M1/M2
> SoC family in "mach-apple/" into "mach-apple/m1/".

Which other Apple SoCs?

> Signed-off-by: Ivaylo Ivanov 
> ---
>  arch/arm/Kconfig | 29 --
>  arch/arm/mach-apple/Kconfig  | 42 
>  arch/arm/mach-apple/Makefile |  5 +--
>  arch/arm/mach-apple/m1/Makefile  |  6 +++
>  arch/arm/mach-apple/{ => m1}/board.c |  0
>  arch/arm/mach-apple/{ => m1}/lowlevel_init.S |  0
>  arch/arm/mach-apple/{ => m1}/rtkit.c |  0
>  arch/arm/mach-apple/{ => m1}/sart.c  |  0
>  configs/apple_m1_defconfig   |  1 +
>  9 files changed, 50 insertions(+), 33 deletions(-)
>  create mode 100644 arch/arm/mach-apple/m1/Makefile
>  rename arch/arm/mach-apple/{ => m1}/board.c (100%)
>  rename arch/arm/mach-apple/{ => m1}/lowlevel_init.S (100%)
>  rename arch/arm/mach-apple/{ => m1}/rtkit.c (100%)
>  rename arch/arm/mach-apple/{ => m1}/sart.c (100%)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 97c25b4f14..5339da370c 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -980,38 +980,9 @@ config ARCH_NPCM
> 
>  config ARCH_APPLE
>   bool "Apple SoCs"
> - select ARM64
> - select CLK
> - select CMD_PCI
> - select CMD_USB
>   select DM
> - select DM_GPIO
> - select DM_KEYBOARD
> - select DM_MAILBOX
>   select DM_RESET
>   select DM_SERIAL
> - select DM_SPI
> - select DM_USB
> - select VIDEO
> - select IOMMU
> - select LINUX_KERNEL_IMAGE_HEADER
> - select OF_BOARD_SETUP
> - select OF_CONTROL
> - select PCI
> - select PINCTRL
> - select POSITION_INDEPENDENT
> - select POWER_DOMAIN
> - select REGMAP
> - select SPI
> - select SYSCON
> - select SYSRESET
> - select SYSRESET_WATCHDOG
> - select SYSRESET_WATCHDOG_AUTO
> - select USB
> - imply CMD_DM
> - imply CMD_GPT
> - imply DISTRO_DEFAULTS
> - imply OF_HAS_PRIOR_STAGE
> 
>  config ARCH_OWL
>   bool "Actions Semi OWL SoCs"
> diff --git a/arch/arm/mach-apple/Kconfig b/arch/arm/mach-apple/Kconfig
> index 294690ec0e..a38779b387 100644
> --- a/arch/arm/mach-apple/Kconfig
> +++ b/arch/arm/mach-apple/Kconfig
> @@ -3,6 +3,46 @@ if ARCH_APPLE
>  config TEXT_BASE
>   default 0x
> 
> +choice
> + prompt "Apple Silicon architecture type select"
> + optional
> +
> +config ARCH_APPLE_M1
> + bool "Apple M1/M2 SoC family"
> + select ARM64
> + select CLK
> + select CMD_PCI
> + select CMD_USB
> + select DM_GPIO
> + select DM_KEYBOARD
> + select DM_MAILBOX
> + select DM_SPI
> + select DM_USB
> + select VIDEO
> + select IOMMU
> + select LINUX_KERNEL_IMAGE_HEADER
> + select OF_BOARD_SETUP
> + select OF_CONTROL
> + select PCI
> + select PINCTRL
> + select POSITION_INDEPENDENT
> + select POWER_DOMAIN
> + select REGMAP
> + select SPI
> + select SYSCON
> + select SYSRESET
> + select SYSRESET_WATCHDOG
> + select SYSRESET_WATCHDOG_AUTO
> + select USB
> + imply CMD_DM
> + imply CMD_GPT
> + imply DISTRO_DEFAULTS
> + imply OF_HAS_PRIOR_STAGE
> +
> +endchoice
> +
> +if ARCH_APPLE_M1
> +
>  config SYS_CONFIG_NAME
>   default "apple"
> 
> @@ -19,3 +59,5 @@ config LNX_KRNL_IMG_TEXT_OFFSET_BASE
>   default TEXT_BASE
> 
>  endif
> +
> +endif
> diff --git a/arch/arm/mach-apple/Makefile b/arch/arm/mach-apple/Makefile
> index 50b465b947..d147ccdde2 100644
> --- a/arch/arm/mach-apple/Makefile
> +++ b/arch/arm/mach-apple/Makefile
> @@ -1,6 +1,3 @@
>  # SPDX-License-Identifier: GPL-2.0+
> 
> -obj-y += board.o
> -obj-y += lowlevel_init.o
> -obj-y += rtkit.o
> -obj-$(CONFIG_NVME_APPLE) += sart.o
> +obj-$(CONFIG_ARCH_APPLE_M1) += m1/
> diff --git a/arch/arm/mach-apple/m1/Makefile b/arch/arm/mach-apple/m1/Makefile
> new file mode 100644
> index 00..50b465b947
> --- /dev/null
> +++ b/arch/arm/mach-apple/m1/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +
> +obj-y += board.o
> +obj-y += lowlevel_init.o
> +obj-y += rtkit.o
> +obj-$(CONFIG_NVME_APPLE) += sart.o
> diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/m1/board.c
> similarity index 100%
> rename from arch/arm/mach-apple/board.c
> rename to arch/arm/mach-apple/m1/board.c
> diff --git a/arch/arm/mach-apple/lowlevel_init.S 
> b/arch/arm/mach-apple/m1/lowlevel_init.S
> similarity index 100%
> rename from arch/arm/mach-apple/lowlevel_init.S
> rename to arch/arm/mach-apple/m1/lowlevel_init.S
> diff --git a/arch/arm/mach-apple/rtkit.c b/arch/arm/mach-apple/m1/rtkit.c
> similarity index 100%
> rename from arch/arm/mach-apple/rtkit.c
> rename to 

[PATCH] arm: mach-apple: Move M1/M2 specifics into a separate folder

2023-08-29 Thread ivo . ivanov
From: Ivaylo Ivanov 

Currently, mach-apple assumes we're working with M1/M2. Make room for
adding support for other Apple SoCs by moving everything from the M1/M2
SoC family in "mach-apple/" into "mach-apple/m1/".

Signed-off-by: Ivaylo Ivanov 
---
 arch/arm/Kconfig | 29 --
 arch/arm/mach-apple/Kconfig  | 42 
 arch/arm/mach-apple/Makefile |  5 +--
 arch/arm/mach-apple/m1/Makefile  |  6 +++
 arch/arm/mach-apple/{ => m1}/board.c |  0
 arch/arm/mach-apple/{ => m1}/lowlevel_init.S |  0
 arch/arm/mach-apple/{ => m1}/rtkit.c |  0
 arch/arm/mach-apple/{ => m1}/sart.c  |  0
 configs/apple_m1_defconfig   |  1 +
 9 files changed, 50 insertions(+), 33 deletions(-)
 create mode 100644 arch/arm/mach-apple/m1/Makefile
 rename arch/arm/mach-apple/{ => m1}/board.c (100%)
 rename arch/arm/mach-apple/{ => m1}/lowlevel_init.S (100%)
 rename arch/arm/mach-apple/{ => m1}/rtkit.c (100%)
 rename arch/arm/mach-apple/{ => m1}/sart.c (100%)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 97c25b4f14..5339da370c 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -980,38 +980,9 @@ config ARCH_NPCM

 config ARCH_APPLE
bool "Apple SoCs"
-   select ARM64
-   select CLK
-   select CMD_PCI
-   select CMD_USB
select DM
-   select DM_GPIO
-   select DM_KEYBOARD
-   select DM_MAILBOX
select DM_RESET
select DM_SERIAL
-   select DM_SPI
-   select DM_USB
-   select VIDEO
-   select IOMMU
-   select LINUX_KERNEL_IMAGE_HEADER
-   select OF_BOARD_SETUP
-   select OF_CONTROL
-   select PCI
-   select PINCTRL
-   select POSITION_INDEPENDENT
-   select POWER_DOMAIN
-   select REGMAP
-   select SPI
-   select SYSCON
-   select SYSRESET
-   select SYSRESET_WATCHDOG
-   select SYSRESET_WATCHDOG_AUTO
-   select USB
-   imply CMD_DM
-   imply CMD_GPT
-   imply DISTRO_DEFAULTS
-   imply OF_HAS_PRIOR_STAGE

 config ARCH_OWL
bool "Actions Semi OWL SoCs"
diff --git a/arch/arm/mach-apple/Kconfig b/arch/arm/mach-apple/Kconfig
index 294690ec0e..a38779b387 100644
--- a/arch/arm/mach-apple/Kconfig
+++ b/arch/arm/mach-apple/Kconfig
@@ -3,6 +3,46 @@ if ARCH_APPLE
 config TEXT_BASE
default 0x

+choice
+   prompt "Apple Silicon architecture type select"
+   optional
+
+config ARCH_APPLE_M1
+   bool "Apple M1/M2 SoC family"
+   select ARM64
+   select CLK
+   select CMD_PCI
+   select CMD_USB
+   select DM_GPIO
+   select DM_KEYBOARD
+   select DM_MAILBOX
+   select DM_SPI
+   select DM_USB
+   select VIDEO
+   select IOMMU
+   select LINUX_KERNEL_IMAGE_HEADER
+   select OF_BOARD_SETUP
+   select OF_CONTROL
+   select PCI
+   select PINCTRL
+   select POSITION_INDEPENDENT
+   select POWER_DOMAIN
+   select REGMAP
+   select SPI
+   select SYSCON
+   select SYSRESET
+   select SYSRESET_WATCHDOG
+   select SYSRESET_WATCHDOG_AUTO
+   select USB
+   imply CMD_DM
+   imply CMD_GPT
+   imply DISTRO_DEFAULTS
+   imply OF_HAS_PRIOR_STAGE
+
+endchoice
+
+if ARCH_APPLE_M1
+
 config SYS_CONFIG_NAME
default "apple"

@@ -19,3 +59,5 @@ config LNX_KRNL_IMG_TEXT_OFFSET_BASE
default TEXT_BASE

 endif
+
+endif
diff --git a/arch/arm/mach-apple/Makefile b/arch/arm/mach-apple/Makefile
index 50b465b947..d147ccdde2 100644
--- a/arch/arm/mach-apple/Makefile
+++ b/arch/arm/mach-apple/Makefile
@@ -1,6 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0+

-obj-y += board.o
-obj-y += lowlevel_init.o
-obj-y += rtkit.o
-obj-$(CONFIG_NVME_APPLE) += sart.o
+obj-$(CONFIG_ARCH_APPLE_M1) += m1/
diff --git a/arch/arm/mach-apple/m1/Makefile b/arch/arm/mach-apple/m1/Makefile
new file mode 100644
index 00..50b465b947
--- /dev/null
+++ b/arch/arm/mach-apple/m1/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0+
+
+obj-y += board.o
+obj-y += lowlevel_init.o
+obj-y += rtkit.o
+obj-$(CONFIG_NVME_APPLE) += sart.o
diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/m1/board.c
similarity index 100%
rename from arch/arm/mach-apple/board.c
rename to arch/arm/mach-apple/m1/board.c
diff --git a/arch/arm/mach-apple/lowlevel_init.S 
b/arch/arm/mach-apple/m1/lowlevel_init.S
similarity index 100%
rename from arch/arm/mach-apple/lowlevel_init.S
rename to arch/arm/mach-apple/m1/lowlevel_init.S
diff --git a/arch/arm/mach-apple/rtkit.c b/arch/arm/mach-apple/m1/rtkit.c
similarity index 100%
rename from arch/arm/mach-apple/rtkit.c
rename to arch/arm/mach-apple/m1/rtkit.c
diff --git a/arch/arm/mach-apple/sart.c b/arch/arm/mach-apple/m1/sart.c
similarity index 100%
rename from arch/arm/mach-apple/sart.c
rename to arch/arm/mach-apple/m1/sart.c
diff --git a/configs/apple_m1_defconfig b/configs/apple_m1_defconfig
index 755560971e..bd723881da 

Re: [PATCH v1] CI: Add jsonschema python module

2023-08-29 Thread Andrejs Cainikovs
On 29/08/2023 17:04, Tom Rini wrote:
> Interesting.  How exactly are you using these CI images? We have
> tools/buildman/requirements.txt now to cover the newly-added modules in
> CI, as we call pip on that.  But, it's not being used in the prepopulate
> the pip cache stage in the Dockerfile.  And, I'm not immune to the idea
> that we should instead be installing the distro package here.

I see in Dockerfile test/py/requirements.txt and
doc/sphinx/requirements.txt, but not tools/buildman/requirements.txt, so
whatever is mentioned there is not part of the image on CI runners.

Following is the part of *our own* .gitlab.yml file,
which uses a public Docker image
(docker.io/trini/u-boot-gitlab-ci-runner:jammy-20230624-20Jul2023, as of
now):

verdin-am62:
  stage: build
  script: |
...
tools/buildman/buildman -o /tmp -P -E "verdin-am62" -M
...

And this is how it fails in our CI without this change:

Building current source for 3 boards (3 threads, 14 jobs per thread)
   arm:  +   verdin-am62_r5
+binman: Unknown entry type 'ti-board-config' in node
'/binman/board-cfg/ti-board-config' (expected etype/ti_board_config.py,
error 'No module named 'jsonschema''
+make[1]: *** [Makefile:1108: .binman_stamp] Error 1
+make: *** [Makefile:177: sub-make] Error 2

-- 
Best regards,
Andrejs Cainikovs



Re: [PATCH] sunxi: psci: remove redundant initialization from psci_arch_init

2023-08-29 Thread Chen-Yu Tsai
On Tue, Aug 29, 2023 at 5:49 AM Sam Edwards  wrote:
>
> On 8/26/23 04:22, Marc Zyngier wrote:
>
> Hi Marc!
>
> > The GIC definitely has the NS bit routed to it. Otherwise, the secure
> > configuration would just be an utter joke. Just try it.
>
> Thank you for your response. I'd like to revisit my prior point about
> the distinction between the NS bit and AxPROT[1] bits in the context of
> monitor mode: in monitor mode, the NS bit does not determine the
> security state of the CPU core (monitor mode is always secure). But even
> then, the NS bit is still significant for other purposes, such as to
> bank accesses to certain CP15 registers -- and if I understand Chen-Yu
> correctly, some GIC registers also. That would require a special NS bit
> signal routed to the GIC so that it can distinguish between "secure,
> NS=0" and "secure, NS=1" accesses, which is why I asked if such a thing
> exists.
>
> I understand that the GIC is designed to be aware of the security state
> (using the existing AxPROT[1] signals) so that it can protect the
> sensitive registers. And unless I misunderstand, this seems to be the
> point that you made here (my interpretation -- correct me if I'm wrong
> -- is that you are using "NS bit" as a metonym for "security state").
> However I must clarify that my question was to seek further information
> from Chen-Yu about the possibility that NS is significant when accessing
> the GIC, even in monitor mode. Alternatively, his point might be merely
> highlighting that the GIC permits different types of access depending on
> the CPU's security state, which aligns with the viewpoint you've reiterated.
>
> I apologize if my previous message didn't convey this context clearly
> enough. My goal was to unravel this nuanced aspect of the NS bit when in
> monitor mode, and to determine if NS needs to be getting set/cleared
> during GIC setup to maneuver around the banking, or if the value of the
> NS bit when in psci_arch_init() is truly of no consequence due to
> monitor mode.

Sorry for my previous misleading comment. The banked registers refer to
the CP15 registers. From ARM's documentation [1]:

When the processor is in Monitor mode, the processor is in Secure state
regardless of the value of the SCR.NS bit. In Monitor mode, the SCR.NS
bit determines whether the Secure Banked CP15 registers or Non-secure
Banked CP15 registers are read or written using MRC or MCR instructions.

[1] 
https://developer.arm.com/documentation/ddi0406/b/System-Level-Architecture/Virtual-Memory-System-Architecture--VMSA-/CP15-registers-for-a-VMSA-implementation/Effect-of-the-Security-Extensions-on-the-CP15-registers?lang=en#Cbbdffad

> > Well, history is unfortunately against you on that front. Running on
> > the secure side definitely was a requirement when this code was
> > initially written, as the AW BSP *required* to run on the secure side.
> >
> > If that requirement is no more, great. But I don't think you can
> > decide that unilaterally.
>
> I have no idea when/if this requirement was changed. It might have never
> happened "formally": perhaps at some point, the SCR.NS=1 code got added
> after the call to psci_arch_init(), breaking (that version of) the AW
> BSP, and nobody ever complained or changed it back... so it stayed.

There was never any BSP code for PSCI before this. This code was written
by Marc in ARM assembly. I merely translated most of it to C code.
The snippet to clear SCR.NS is there in the first commit,

d5db7024aafc sunxi: HYP/non-sec: add sun7i PSCI backend

ChenYu

> But we're starting to digress from what _this_ patch does. The intent
> here is only to remove two lines of code that (we're discussing to
> confirm) have no effect. I'm not touching the code that *actually*
> determines the world into which monitor mode exits.
>
> Cheers,
> Sam


Re: [RFC PATCH 5/5] doc: Add a document for non-compliant DT node/property removal

2023-08-29 Thread Simon Glass
Hi Sughosh,

On Mon, 28 Aug 2023 at 12:35, Sughosh Ganu  wrote:
>
> hi Simon,
>
> On Mon, 28 Aug 2023 at 23:25, Simon Glass  wrote:
> >
> > Hi Sughosh,
> >
> > On Sat, 26 Aug 2023 at 03:07, Sughosh Ganu  wrote:
> > >
> > > Add a document explaining the need for removal of non-compliant
> > > devicetree nodes and properties. Also describe in brief, the macros
> > > that can be used for this removal.
> > >
> > > Signed-off-by: Sughosh Ganu 
> > > ---
> > >  .../devicetree/dt_non_compliant_purge.rst | 64 +++
> > >  1 file changed, 64 insertions(+)
> > >  create mode 100644 doc/develop/devicetree/dt_non_compliant_purge.rst
> > >
> > > diff --git a/doc/develop/devicetree/dt_non_compliant_purge.rst 
> > > b/doc/develop/devicetree/dt_non_compliant_purge.rst
> > > new file mode 100644
> > > index 00..c3a8feab5b
> > > --- /dev/null
> > > +++ b/doc/develop/devicetree/dt_non_compliant_purge.rst
> > > @@ -0,0 +1,64 @@
> > > +.. SPDX-License-Identifier: GPL-2.0+
> > > +
> > > +Removal of non-compliant nodes and properties
> > > +=
> > > +
> > > +The devicetree used in U-Boot might contain nodes and properties which
> > > +are specific only to U-Boot, and are not necessarily being used to
> > > +describe hardware but to pass information to U-Boot. An example of
> > > +such a property would be the public key being passed to U-Boot for
> > > +verification.
> >
> > It has nothing to do with describing hardware. The DT can describe
> > other things too. See the /options node, for example.
> >
> > Please don't bring this highly misleading language into U-Boot.
>
> Please point out what is misleading in the above paragraph. What is
> being emphasised in the above paragraph is that certain nodes and
> properties in the devicetree are relevant only in u-boot, and not the
> kernel. And this is precisely what the devicetree maintainers are
> saying [1].

That is not relevant though...we need to make sure all the nodes are
in the dt schema.

It is misleading because you imply that DT should only describe
hardware. That is not true.

>
> >
> > > +
> > > +This devicetree can then be passed to the OS. Since certain nodes and
> > > +properties are not really describing hardware, and more importantly,
> > > +these are only relevant to U-Boot, bindings for these cannot be
> > > +upstreamed into the devicetree repository. There have been instances
> > > +of attempts being made to upstream such bindings, and these deemed not
> > > +fit for upstreaming.
> >
> > Then either they should not be in U-Boot, or there is a problem with
> > the process.
> >
> > > Not having a binding for these nodes and
> > > +properties means that the devicetree fails the schema compliance tests
> > > +[1]. This also means that the platform cannot get certifications like
> > > +SystemReady [2] which, among other things require a devicetree which
> > > +passes the schema compliance tests.
> > > +
> > > +For such nodes and properties, it has been suggested by the devicetree
> > > +maintainers that the right thing to do is to remove them from the
> > > +devicetree before it gets passed on to the OS [3].
> >
> > Hard NAK. If we go this way, then no one will ever have an incentive
> > to do the right thing.
> >
> > Please send bindings for Linaro's work, instead. If something is
> > entirely U-Boot-specific, then it can go in /options/u-boot but it
> > still must be in the dt-schema.
>
> Please re-read the document including the last link [1]. If you go
> through that entire thread, you will notice that this is precisely
> what Linaro was trying to do here -- upstream the binding for the
> fwu-mdata node. It is only based on the feedback of the devicetree
> maintainers that this patchset was required.

It looks like it should go in /options/u-boot ? Can you resubmit it there?

The stripping is a no-go for me, sorry. It is absolutely going to
destroy any chance of tidying up DT in U-Boot.

Please also figure out how to add DT validation to U-Boot, so we can
see what the gap is. Once we have that in, I will be happier to do
workarounds.

Regards,
Simon

>
> -sughosh
>
> [1] - 
> https://lore.kernel.org/u-boot/cal_jsqjn4fehoml7z3yj0wj9bpx1ose7zf26l_gv2os6cg-...@mail.gmail.com/#t
>
> >
> > > +
> > > +Removing nodes/properties
> > > +-
> > > +
> > > +In U-Boot, this is been done through adding information on such nodes
> > > +and properties in a list. The entire node can be deleted, or a
> > > +specific property under a node can be deleted. The list of such nodes
> > > +and properties is generated at compile time, and the function to purge
> > > +these can be invoked through a EVT_FT_FIXUP event notify call.
> > > +
> > > +For deleting a node, this can be done by declaring a macro::
> > > +
> > > +   DT_NON_COMPLIANT_PURGE(fwu_mdata) = {
> > > +   .node_path  = "/fwu-mdata",
> > > +   };
> > > +
> > > +Similarly, for deleting a property under a node, 

Re: [PATCH] arm: kirkwood: Add support for ZyXEL NSA325 board

2023-08-29 Thread Tony Dinh
Hi Stefan,

For unknown reasons (probably Gmail glitches), I did not receive your
email review in this thread. But I did receive the other email review
for the Pogo V4 LTO! lore.kernel.org is OK.

https://lore.kernel.org/u-boot/65ab8aa6-c290-0649-0b2e-374c4f84e...@denx.de/T/#mf17689e3e963ab7a34fda34a97f67dea7a877b65

Stefan: "Just checking: Are the DT files imported from the Linux ones?"

Yes, these DT files were copied verbatim from Linux. That's why we see
some minor warnings about styles.

All the best,
Tony

On Fri, Aug 25, 2023 at 8:33 PM Tony Dinh  wrote:
>
> ZyXEL NSA325 specifications:
>
> Marvell Kirkwood 88F6282 SoC
> 1.6 GHz CPU
> 1x GBE LAN port (Marvell MV88E1318)
> 512 MB RAM
> 128 MB Eon NAND, SLC
> I2C
> 1x USB 3.0 (on PCIe bus)
> 2x USB 2.0
> 2x SATA (hot swap slots)
> Serial console
>
> Signed-off-by: Tony Dinh 
> ---
>
>  arch/arm/dts/Makefile|   1 +
>  arch/arm/dts/kirkwood-6282.dtsi  | 161 
>  arch/arm/dts/kirkwood-nsa325.dts | 231 +++
>  arch/arm/dts/kirkwood-nsa3x0-common.dtsi | 157 +++
>  arch/arm/mach-kirkwood/Kconfig   |   7 +
>  board/zyxel/nsa325/Kconfig   |  12 ++
>  board/zyxel/nsa325/MAINTAINERS   |   9 +
>  board/zyxel/nsa325/Makefile  |  11 ++
>  board/zyxel/nsa325/kwbimage.cfg  |  55 ++
>  board/zyxel/nsa325/nsa325.c  | 196 +++
>  configs/nsa325_defconfig |  81 
>  include/configs/nsa325.h |  37 
>  12 files changed, 958 insertions(+)
>  create mode 100644 arch/arm/dts/kirkwood-6282.dtsi
>  create mode 100644 arch/arm/dts/kirkwood-nsa325.dts
>  create mode 100644 arch/arm/dts/kirkwood-nsa3x0-common.dtsi
>  create mode 100644 board/zyxel/nsa325/Kconfig
>  create mode 100644 board/zyxel/nsa325/MAINTAINERS
>  create mode 100644 board/zyxel/nsa325/Makefile
>  create mode 100644 board/zyxel/nsa325/kwbimage.cfg
>  create mode 100644 board/zyxel/nsa325/nsa325.c
>  create mode 100644 configs/nsa325_defconfig
>  create mode 100644 include/configs/nsa325.h
>
> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> index 85fd5b1157..b9fc46544f 100644
> --- a/arch/arm/dts/Makefile
> +++ b/arch/arm/dts/Makefile
> @@ -66,6 +66,7 @@ dtb-$(CONFIG_ARCH_KIRKWOOD) += \
> kirkwood-ns2max.dtb \
> kirkwood-ns2mini.dtb \
> kirkwood-nsa310s.dtb \
> +   kirkwood-nsa325.dtb \
> kirkwood-openrd-base.dtb \
> kirkwood-openrd-client.dtb \
> kirkwood-openrd-ultimate.dtb \
> diff --git a/arch/arm/dts/kirkwood-6282.dtsi b/arch/arm/dts/kirkwood-6282.dtsi
> new file mode 100644
> index 00..e732c501ea
> --- /dev/null
> +++ b/arch/arm/dts/kirkwood-6282.dtsi
> @@ -0,0 +1,161 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/ {
> +   mbus@f100 {
> +   pciec: pcie@8200 {
> +   compatible = "marvell,kirkwood-pcie";
> +   status = "disabled";
> +   device_type = "pci";
> +
> +   #address-cells = <3>;
> +   #size-cells = <2>;
> +
> +   bus-range = <0x00 0xff>;
> +
> +   ranges =
> +  <0x8200 0 0x4 MBUS_ID(0xf0, 0x01) 
> 0x4 0 0x2000
> +   0x8200 0 0x44000 MBUS_ID(0xf0, 0x01) 
> 0x44000 0 0x2000
> +   0x8200 0 0x8 MBUS_ID(0xf0, 0x01) 
> 0x8 0 0x2000
> +   0x8200 0x1 0 MBUS_ID(0x04, 0xe8) 0
>1 0 /* Port 0.0 MEM */
> +   0x8100 0x1 0 MBUS_ID(0x04, 0xe0) 0
>1 0 /* Port 0.0 IO  */
> +   0x8200 0x2 0 MBUS_ID(0x04, 0xd8) 0
>1 0 /* Port 1.0 MEM */
> +   0x8100 0x2 0 MBUS_ID(0x04, 0xd0) 0
>1 0 /* Port 1.0 IO  */>;
> +
> +   pcie0: pcie@1,0 {
> +   device_type = "pci";
> +   assigned-addresses = <0x82000800 0 0x0004 
> 0 0x2000>;
> +   reg = <0x0800 0 0 0 0>;
> +   #address-cells = <3>;
> +   #size-cells = <2>;
> +   #interrupt-cells = <1>;
> +   ranges = <0x8200 0 0 0x8200 0x1 0 1 0
> + 0x8100 0 0 0x8100 0x1 0 1 
> 0>;
> +   bus-range = <0x00 0xff>;
> +   interrupt-names = "intx", "error";
> +   interrupts = <9>, <44>;
> +   interrupt-map-mask = <0 0 0 7>;
> +   interrupt-map = <0 0 0 1 _intc 0>,
> +   <0 0 0 2 _intc 1>,
> + 

Re: Config fragments

2023-08-29 Thread Andrew Davis

On 8/29/23 11:47 AM, Simon Glass wrote:

Hi Andrew,

On Wed, 23 Aug 2023 at 10:44, Andrew Davis  wrote:


On 8/23/23 10:30 AM, Simon Glass wrote:

Hi,

Up until 2023.04 it has been possible to build all the defconfigs but
with 2023.07 that changed. Tom mentioned this to me recently.

Up until 2023.04 we can enumerate all the board configs that can be
built. We can build any of them using a single name and a single
defconfig. The boards.cfg file which buildman creates contains a full
list of things that can be built.

  From 2023.07 this changes and we now have random .config files
sprinkled about the place. I say random because they are not connected
to anything. For example, here is
board/ti/am62x/MAINTAINERS:

AM62x BOARD
M: Dave Gerlach 
M: Tom Rini 
S: Maintained
F: board/ti/am62x/
F: include/configs/am62x_evm.h
F: configs/am62x_evm_r5_defconfig
F: configs/am62x_evm_a53_defconfig

BEAGLEPLAY BOARD
M: Nishanth Menon 
M: Robert Nelson 
M: Tom Rini 
S: Maintained
N: beagleplay

In most cases the MAINTAINERS file tells us about each board and until
[1] I had assumed that was the case. With that patch reverted, these
are the only 'orphaned' MAINTAINERS entries (buildman
--maintainer-check):

WARNING: orphaned defconfig in board/armltd/vexpress64/MAINTAINERS
ending at line 8
WARNING: orphaned defconfig in board/google/veyron/MAINTAINERS ending at line 44
WARNING: orphaned defconfig in
board/mikrotik/crs3xx-98dx3236/MAINTAINERS ending at line 7
WARNING: orphaned defconfig in board/st/common/MAINTAINERS ending at line 6
WARNING: orphaned defconfig in board/ti/am62x/MAINTAINERS ending at line 15

But Tom advised me that MAINTAINERS is not intended for this purpose,
so the check has been dropped. I am not suggesting we bring it back,
necessarily, but if we did, we could use it to specify the .config and
defconfig files which are used by each board.

*Failing that*, I suggest a new 'name.brd' file in the board directory
to contain this information, e.g.

# Contents of board/ti/am62x/beagleplay.brd:
beagleplay-r5: am62x_evm_r5_defconfig beagleplay_r5.config
beagleplay-a53: am62x_evm_a53_defconfig beagleplay_a53.config

This could be parsed by buildman and other tools, to produce a list of
available boards and to build each using a single name (e.g. make
beagleplay-r5_defconfig)

What do people think?



How about instead of needing this new 'name.brd' files, we look into
"include" directives inside these configs? So we could have a file:

beagleplay_r5_defconfig

in the normal configs/ directory, but with the contents:

#include "configs/am62x_evm_r5_defconfig"

CONFIG_DEFAULT_DEVICE_TREE="k3-am625-beagleplay"
CONFIG_OF_LIST="k3-am625-beagleplay"
...

The # is already a comment line so these should be safe
for existing tools, and then we have in our Makefile
an automatic pass through the C preprocessor.

Some boards have can build with and without the fragments,
so to have complete CI coverage, we have dummy defconfigs
that have both the base and a fragment include:

#include "configs/am62x_evm_r5_defconfig"
#include "board/ti/am62x/high_security.config"

Something like that, then as Heinrich mentioned, simply
enumerating configs/*defconfig should yield all valid
combinations for building/testing.


I do agree it would be nice to have this information in the file it
relates to. But wouldn't that involve changing kconf and other tools?
What tools will parse those files? We really want 'make xxx_defconfig'
to work for these new 'combined config' boards and my understanding is
that kconf is used here.


The U-Boot Makefile passes the provided xxx_defconfig into the kconfig
scripts. All I'm suggesting is to have that Makefile run the passed
in defconfig through the C preprocessor before handing it off to kconfig.

For existing defconfigs the preprocessor will make no changes to the file.
For the config fragments, the `#include` lines will be substituted with the
contents, the result will be a normal defconfig file that can then also be
passed into kconfig.

No changes to kconfig scripts, or anything other than the Makefile, are needed.



Your proposal certainly allows each 'combined config' to have a name -
it is just the filename of the defconfig file.



Exactly, this keeps all current CI tooling/flows the exact same.

Andrew


Any ideas?

Regards,
Simon


Re: [PATCH 1/1] doc: describe Kconfig fragments

2023-08-29 Thread Simon Glass
Hi,

On Tue, 29 Aug 2023 at 10:17, Tom Rini  wrote:
>
> On Tue, Aug 29, 2023 at 08:54:27AM +0200, Heinrich Schuchardt wrote:
>
> > In the 'Build with GCC' chapter mention Kconfig fragments and show how they
> > are applied.
> >
> > Adjust the formatting in the configuration section to consistently use
> > italics for file names.
> >
> > Signed-off-by: Heinrich Schuchardt 
> > ---
> >  doc/build/gcc.rst | 35 ++-
> >  1 file changed, 30 insertions(+), 5 deletions(-)
>
> We should really restructure this file as there's very few gcc-specific
> parts and the rest is also true for clang.

Also, let's decide how we are going to manage fragments. I replied to
that thread again.

Regards,
Simon


Re: Config fragments

2023-08-29 Thread Simon Glass
Hi Andrew,

On Wed, 23 Aug 2023 at 10:44, Andrew Davis  wrote:
>
> On 8/23/23 10:30 AM, Simon Glass wrote:
> > Hi,
> >
> > Up until 2023.04 it has been possible to build all the defconfigs but
> > with 2023.07 that changed. Tom mentioned this to me recently.
> >
> > Up until 2023.04 we can enumerate all the board configs that can be
> > built. We can build any of them using a single name and a single
> > defconfig. The boards.cfg file which buildman creates contains a full
> > list of things that can be built.
> >
> >  From 2023.07 this changes and we now have random .config files
> > sprinkled about the place. I say random because they are not connected
> > to anything. For example, here is
> > board/ti/am62x/MAINTAINERS:
> >
> > AM62x BOARD
> > M: Dave Gerlach 
> > M: Tom Rini 
> > S: Maintained
> > F: board/ti/am62x/
> > F: include/configs/am62x_evm.h
> > F: configs/am62x_evm_r5_defconfig
> > F: configs/am62x_evm_a53_defconfig
> >
> > BEAGLEPLAY BOARD
> > M: Nishanth Menon 
> > M: Robert Nelson 
> > M: Tom Rini 
> > S: Maintained
> > N: beagleplay
> >
> > In most cases the MAINTAINERS file tells us about each board and until
> > [1] I had assumed that was the case. With that patch reverted, these
> > are the only 'orphaned' MAINTAINERS entries (buildman
> > --maintainer-check):
> >
> > WARNING: orphaned defconfig in board/armltd/vexpress64/MAINTAINERS
> > ending at line 8
> > WARNING: orphaned defconfig in board/google/veyron/MAINTAINERS ending at 
> > line 44
> > WARNING: orphaned defconfig in
> > board/mikrotik/crs3xx-98dx3236/MAINTAINERS ending at line 7
> > WARNING: orphaned defconfig in board/st/common/MAINTAINERS ending at line 6
> > WARNING: orphaned defconfig in board/ti/am62x/MAINTAINERS ending at line 15
> >
> > But Tom advised me that MAINTAINERS is not intended for this purpose,
> > so the check has been dropped. I am not suggesting we bring it back,
> > necessarily, but if we did, we could use it to specify the .config and
> > defconfig files which are used by each board.
> >
> > *Failing that*, I suggest a new 'name.brd' file in the board directory
> > to contain this information, e.g.
> >
> > # Contents of board/ti/am62x/beagleplay.brd:
> > beagleplay-r5: am62x_evm_r5_defconfig beagleplay_r5.config
> > beagleplay-a53: am62x_evm_a53_defconfig beagleplay_a53.config
> >
> > This could be parsed by buildman and other tools, to produce a list of
> > available boards and to build each using a single name (e.g. make
> > beagleplay-r5_defconfig)
> >
> > What do people think?
> >
>
> How about instead of needing this new 'name.brd' files, we look into
> "include" directives inside these configs? So we could have a file:
>
> beagleplay_r5_defconfig
>
> in the normal configs/ directory, but with the contents:
>
> #include "configs/am62x_evm_r5_defconfig"
>
> CONFIG_DEFAULT_DEVICE_TREE="k3-am625-beagleplay"
> CONFIG_OF_LIST="k3-am625-beagleplay"
> ...
>
> The # is already a comment line so these should be safe
> for existing tools, and then we have in our Makefile
> an automatic pass through the C preprocessor.
>
> Some boards have can build with and without the fragments,
> so to have complete CI coverage, we have dummy defconfigs
> that have both the base and a fragment include:
>
> #include "configs/am62x_evm_r5_defconfig"
> #include "board/ti/am62x/high_security.config"
>
> Something like that, then as Heinrich mentioned, simply
> enumerating configs/*defconfig should yield all valid
> combinations for building/testing.

I do agree it would be nice to have this information in the file it
relates to. But wouldn't that involve changing kconf and other tools?
What tools will parse those files? We really want 'make xxx_defconfig'
to work for these new 'combined config' boards and my understanding is
that kconf is used here.

Your proposal certainly allows each 'combined config' to have a name -
it is just the filename of the defconfig file.

Any ideas?

Regards,
Simon


Re: [PATCH] spl: bootstage: move bootstage_stash before jumping to image

2023-08-29 Thread Simon Glass
Hi Chanho,

On Mon, 28 Aug 2023 at 21:46, Chanho Park  wrote:
>
> Hi Simon,
>
> > -Original Message-
> > From: Simon Glass 
> > Sent: Tuesday, August 29, 2023 2:55 AM
> > To: Chanho Park 
> > Cc: Nikhil M Jain ; Marek Vasut ; u-
> > b...@lists.denx.de
> > Subject: Re: [PATCH] spl: bootstage: move bootstage_stash before jumping
> > to image
> >
> > Hi Chanho,
> >
> > On Mon, 28 Aug 2023 at 03:46, Chanho Park 
> > wrote:
> > >
> > > For IH_OS_OPENSBI and IH_OS_LINUX, there is no chance to stash
> > > bootstare record because it will not return after jumping to the image.
> > > Hence, this patch moves the location of bootstage_stash before jumping
> > > to image.
> > >
> >
> > Please fix your implementation instead. You must jump to the OS at the end
> > of this function and not before.
>
> In case of OS_TEE/OPENSBI/LINUX images, they will not be returned to SPL so 
> they can't get a chance to stash boot stage records.
>
> >
> > In fact I have a patch to move bootstage later in this function!
>
> Will you post the patch soon? Or it was already posted?

Not posted yet. I'll reply on your new patch.

Regards,
Simon


Re: [PATCH v2] spl: bootstage: move bootstage_stash before jumping to image

2023-08-29 Thread Simon Glass
Hi Chanho,

On Mon, 28 Aug 2023 at 22:28, Chanho Park  wrote:
>
> Regarding IH_OS_OPENSBI, IH_OS_LINUX and IH_OS_TEE, there is no chance
> to stash bootstage record because they do not return to SPL after
> jumping to the image.
> Hence, this patch separates the final stage bootstage code into
> spl_bootstage_finish and call the function before jumping to the image.
>
> Signed-off-by: Chanho Park 
> ---
> Changes from v1
> - Separate the final stage bootstage code into spl_bootstage_finish.
> - As Simon suggests, call the function before jumping to the image.

I think you misunderstood me here. I mean, you cannot jump off
somewhere in your board code. You must change it so it returns
correctly, and the jump happens from spl.c's board_init_r() function.
The way it works is you set up the spl_image structure, then it SPL
jumps to it at the end of the functions.

Regards,
Simon


Re: [PATCH 6/6] stm32mp15: Use u-boot-spl-stm32.bin instead of u-boot-spl.stm32

2023-08-29 Thread Patrick DELAUNAY

Hi Simon,

On 8/24/23 17:14, Tom Rini wrote:

On Thu, Aug 24, 2023 at 05:09:07PM +0200, Marek Vasut wrote:

On 8/24/23 16:25, Tom Rini wrote:

On Thu, Aug 24, 2023 at 05:12:45AM +0200, Marek Vasut wrote:

On 8/24/23 05:02, Simon Glass wrote:

A '.stm32' extension is not allowed anymore, so change it.

Why?

This will likely break a huge amount of scripts, I'm tempted to NAK it
unless there is a very good reason.

This is in the cover letter.  Today, buildman --keep-outputs doesn't
actually keep the needed for booting outputs from a build for a number
of platforms.  Simon's response is to stop having a free-form list of
outputs. With I guess the caveat being ROM-defined names (for example,
we still keep "MLO" because that is the literal filename TI ROM looks
for on FAT partitions, on mos of their 32bit platforms).

Why not just place the free-form files into some output/ directory and be
done with it ? Then they can have whatever extension they want, as long as
the output/ directory name is stable.

Yes, an alternative here is to just extend the list that's removed in
patch 2/6.



The ".stm32" was choosen on output on mkimage to be aligned with:

- all STMicroelectonics documentation  (for example 
https://wiki.st.com/stm32mpu/wiki/STM32_header_for_binary_files)


- the proposed scripts or files, in particular in the YOCTO generated 
flashlayout files.


- this extension list expected by our tools: STM CubeProgrammer 
(https://wiki.st.com/stm32mpu/wiki/STM32CubeProgrammer)


and Signing tools (https://wiki.st.com/stm32mpu/wiki/Signing_tool)


So I prefer to kept the ".stm32" extension here:

filename = "u-boot-spl.stm32"


NB: the justification for buildman '-k' option seens not fully relevant here

   because in patch 2/6 you kept not only the ALLOWED extension but 
also some particular files


+to_copy = ['u-boot*', '*.map', 'MLO', 'SPL',
+   'include/autoconf.mk', 'spl/u-boot-spl*']
+to_copy += [f'*{ext}' for ext in ALLOWED_EXTS]


so all the files "u-boot*" are kept with buildman -k even if it is not a 
allowed extension.



I propose to change the patch 1/6 if you are agree

and allow binman to generate the file with same rules than buildman -k 
option in patch 2/6



The filename is valid if

- the file is named with the allowed prefix 'u-boot' => 'u-boot*' so 
"u-boot-spl.stm32" is allowed


- the file is with allowed extension =>.bin, .rom, .itb, .img


Regards


Patrick




Re: [PATCH 1/1] doc: describe Kconfig fragments

2023-08-29 Thread Tom Rini
On Tue, Aug 29, 2023 at 08:54:27AM +0200, Heinrich Schuchardt wrote:

> In the 'Build with GCC' chapter mention Kconfig fragments and show how they
> are applied.
> 
> Adjust the formatting in the configuration section to consistently use
> italics for file names.
> 
> Signed-off-by: Heinrich Schuchardt 
> ---
>  doc/build/gcc.rst | 35 ++-
>  1 file changed, 30 insertions(+), 5 deletions(-)

We should really restructure this file as there's very few gcc-specific
parts and the rest is also true for clang.

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v3 1/2] schemas: Add a schema for memory map

2023-08-29 Thread Simon Glass
(Adding a few more, will respond soon)

- Simon

On Thu, 24 Aug 2023 at 03:10, Ard Biesheuvel  wrote:
>
> On Wed, 23 Aug 2023 at 22:04, Simon Glass  wrote:
> >
> > Hi,
> >
> > On Wed, 23 Aug 2023 at 08:24, Ard Biesheuvel  wrote:
> > >
> > > On Wed, 23 Aug 2023 at 10:59, Mark Rutland  wrote:
> > > >
> > > > On Tue, Aug 22, 2023 at 02:34:42PM -0600, Simon Glass wrote:
> > > > > The Devicetree specification skips over handling of a logical view of
> > > > > the memory map, pointing users to the UEFI specification.
> > > > >
> > > > > It is common to split firmware into 'Platform Init', which does the
> > > > > initial hardware setup and a "Payload" which selects the OS to be 
> > > > > booted.
> > > > > Thus an handover interface is required between these two pieces.
> > > > >
> > > > > Where UEFI boot-time services are not available, but UEFI firmware is
> > > > > present on either side of this interface, information about memory 
> > > > > usage
> > > > > and attributes must be presented to the "Payload" in some form.
> > >
> > > Not quite.
> > >
> > > This seems to be intended for consumption by Linux booting in ACPI
> > > mode, but not via UEFI, right?
> >
> > Actually, this is for consumption by firmware. The goal is to allow
> > edk2 to boot into U-Boot and vice versa, i.e. provide some
> > interoperability between firmware projects. I will use the "Platform
> > Init" and "Payload" terminology here too.
> >
>
> OK. It was the cc to linux-acpi@ and the authors of the
> ACPI/SMBIOS-without-UEFI patches that threw me off here.
>
> If we are talking about an internal interface for firmware components,
> I'd be inclined to treat this as an implementation detail, as long as
> the OS is not expected to consume these DT nodes.
>
> However, I struggle to see the point of framing this information as a
> 'UEFI memory map'. Neither EDK2 nor u-boot consume this information
> natively, and there is already prior art in both projects to consume
> nodes following the existing bindings for device_type=memory and the
> /reserved-memory node. UEFI runtime memory is generally useless
> without UEFI runtime services, and UEFI boot services memory is just
> free memory.
>
> There is also an overlap with the handover between secure and
> non-secure firmware on arm64, which is also DT based, and communicates
> available memory as well as RAM regions that are reserved for firmware
> use.
>
> In summary, I don't see why a non-UEFI payload would care about UEFI
> semantics for pre-existing memory reservations, or vice versa. Note
> that EDK2 will manage its own memory map, and expose it via UEFI boot
> services and not via DT.
>
> ...
> >
> > There is no intent to implement the UEFI spec, here. It is simply that
> > some payloads (EDK2) are used to having this information.
> >
> > Imagine splitting EDK2 into two parts, one of which does platform init
> > and the other which (the payload) boots the OS. The payload wants
> > information from Platform Init and it needs to be in a devicetree,
> > since that is what we have chosen for this interface. So to some
> > extent this is unrelated to whether you have EFI boot services. We
> > just need to be able to pass the information across the interface.
> > Note that the user can (without recompilation, etc.) replace the
> > second part with U-Boot (for example) and it must still work.
> >
>
> OK, so device tree makes sense for this. This is how I implemented the
> EDK2 port that targets QEMU/mach-virt - it consumes the DT to discover
> the UART, RTC,, memory, PCI host bridge, etc.
>
> But I don't see a use case for a UEFI memory map here.
>
>
> > >
> > > >
> > > > Today Linux does that by passing:
> > > >
> > > >   /chosen/linux,uefi-mmap-start
> > > >   /chosen/linux,uefi-mmap-size
> > > >   /chosen/linux,uefi-mmap-desc-size
> > > >   /chosen/linux,uefi-mmap-desc-ver
> > > >
> > > > ... or /chosen/xen,* variants of those.
> > > >
> > > > Can't we document / genericise that?
> >
> > That seems to me to be the fields from the EFI memory-map call, but
> > where is the actual content? I looked in the kernel but it seems to be
> > an internal interface (between the stub and the kernel)?
> >
> > > >
> > >
> > > Given the ACPI angle, promoting this to external ABI would introduce a
> > > DT dependency to ACPI boot. So we'll at least have to be very clear
> > > about which takes precedence, or maybe disregard everything except the
> > > /chosen node when doing ACPI boot?
> > >
> > > This also argues for not creating an ordinary binding for this (i.e.,
> > > describing it as part of the platform topology), but putting it under
> > > /chosen as a Linux-only boot tweak.
> > >
> > > > Pointing to that rather than re-encoding it in DT means that it stays 
> > > > in-sync
> > > > with the EFI spec and we won't back ourselves into a corner where we 
> > > > cannot
> > > > encode something due to a structural difference. I don't think it's a 
> > > > good idea
> > > > to try to re-encode it, or we're 

Re: [PATCH] configs: stm32f769-disco: Enable VIDEO_LOGO flag

2023-08-29 Thread Patrick DELAUNAY

Hi,

On 8/25/23 18:24, Patrice Chotard wrote:

The patch removes the legacy mode of displaying the ST logo and adopts
the approach introduced by the commit 284b08fb51b6 ("board: stm32mp1: add
splash screen with stmicroelectronics logo").

Signed-off-by: Patrice Chotard 
---

  configs/stm32f769-disco_defconfig | 2 +-
  configs/stm32f769-disco_spl_defconfig | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/configs/stm32f769-disco_defconfig 
b/configs/stm32f769-disco_defconfig
index 72ef133fe4a..20dbb1af630 100644
--- a/configs/stm32f769-disco_defconfig
+++ b/configs/stm32f769-disco_defconfig
@@ -56,6 +56,7 @@ CONFIG_SPI=y
  CONFIG_DM_SPI=y
  CONFIG_STM32_QSPI=y
  CONFIG_VIDEO=y
+CONFIG_VIDEO_LOGO=y
  CONFIG_BACKLIGHT_GPIO=y
  CONFIG_VIDEO_LCD_ORISETECH_OTM8009A=y
  CONFIG_VIDEO_STM32=y
@@ -64,7 +65,6 @@ CONFIG_VIDEO_STM32_MAX_XRES=480
  CONFIG_VIDEO_STM32_MAX_YRES=800
  CONFIG_SPLASH_SCREEN=y
  CONFIG_SPLASH_SCREEN_ALIGN=y
-CONFIG_VIDEO_BMP_RLE8=y
  CONFIG_BMP_16BPP=y
  CONFIG_BMP_24BPP=y
  CONFIG_BMP_32BPP=y
diff --git a/configs/stm32f769-disco_spl_defconfig 
b/configs/stm32f769-disco_spl_defconfig
index dd17cad7362..a5298e7cdc1 100644
--- a/configs/stm32f769-disco_spl_defconfig
+++ b/configs/stm32f769-disco_spl_defconfig
@@ -82,6 +82,7 @@ CONFIG_DM_SPI=y
  CONFIG_STM32_QSPI=y
  CONFIG_SPL_TIMER=y
  CONFIG_VIDEO=y
+CONFIG_VIDEO_LOGO=y
  CONFIG_BACKLIGHT_GPIO=y
  CONFIG_VIDEO_LCD_ORISETECH_OTM8009A=y
  CONFIG_VIDEO_STM32=y
@@ -90,7 +91,6 @@ CONFIG_VIDEO_STM32_MAX_XRES=480
  CONFIG_VIDEO_STM32_MAX_YRES=800
  CONFIG_SPLASH_SCREEN=y
  CONFIG_SPLASH_SCREEN_ALIGN=y
-CONFIG_VIDEO_BMP_RLE8=y
  CONFIG_BMP_16BPP=y
  CONFIG_BMP_24BPP=y
  CONFIG_BMP_32BPP=y





Reviewed-by: Patrick Delaunay 

Thanks
Patrick




Re: [PATCH 3/3] Remove the hardcoded ST logo no longer in use

2023-08-29 Thread Patrick DELAUNAY

Hi,

On 8/20/23 18:24, Dario Binacchi wrote:

The patch removes the hardcoded ST logo from the code, as it is no
longer used.

Signed-off-by: Dario Binacchi 

---

  include/st_logo_data.h | 3265 
  1 file changed, 3265 deletions(-)
  delete mode 100644 include/st_logo_data.h





Reviewed-by: Patrick Delaunay 

Thanks
Patrick




Re: [PATCH 2/3] board: stm32f746-disco: refactor the display of the ST logo

2023-08-29 Thread Patrick DELAUNAY

Hi

On 8/20/23 18:24, Dario Binacchi wrote:

The patch removes the legacy mode of displaying the ST logo and adopts
the approach introduced by the commit 284b08fb51b6 ("board: stm32mp1: add
splash screen with stmicroelectronics logo"). It was necessary to use a
specific logo for the stm32f746-disco board.

Furthermore, the previous version didn't properly center the logo, hiding
its upper part.

Signed-off-by: Dario Binacchi 
---

  board/st/stm32f746-disco/stm32f746-disco.c |   6 --
  configs/stm32f746-disco_defconfig  |   2 +-
  configs/stm32f746-disco_spl_defconfig  |   2 +-
  include/configs/stm32f746-disco.h  |   7 ++-
  tools/logos/stm32f746-disco.bmp| Bin 0 -> 18052 bytes
  5 files changed, 8 insertions(+), 9 deletions(-)
  create mode 100644 tools/logos/stm32f746-disco.bmp



Reviewed-by: Patrick Delaunay 

Thanks
Patrick




Re: [PATCH] clk: Dont return error when assigned-clocks is empty or missing

2023-08-29 Thread Tom Rini
On Tue, Aug 29, 2023 at 09:18:53AM +0200, Michal Simek wrote:
> 
> 
> On 8/28/23 17:50, Tom Rini wrote:
> > On Mon, Aug 28, 2023 at 12:25:30PM +0200, Michal Simek wrote:
> > > 
> > > 
> > > On 8/25/23 16:39, Tom Rini wrote:
> > > > On Fri, Aug 25, 2023 at 09:15:09AM +0200, Michal Simek wrote:
> > > > > Hi Tom,
> > > > > 
> > > > > On 7/11/23 11:51, Ashok Reddy Soma wrote:
> > > > > > There is a chance that assigned-clock-rates is given and 
> > > > > > assigned-clocks
> > > > > > could be empty. Dont return error in that case, because the probe 
> > > > > > of the
> > > > > > corresponding driver will not be called at all if this fails.
> > > > > > Better to continue to look for it and return 0.
> > > > > > 
> > > > > > Signed-off-by: Ashok Reddy Soma 
> > > > > > ---
> > > > > > 
> > > > > > drivers/clk/clk-uclass.c | 8 +++-
> > > > > > 1 file changed, 7 insertions(+), 1 deletion(-)
> > > > > > 
> > > > > > diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
> > > > > > index dc3e9d6a26..f186fcbcdb 100644
> > > > > > --- a/drivers/clk/clk-uclass.c
> > > > > > +++ b/drivers/clk/clk-uclass.c
> > > > > > @@ -329,7 +329,13 @@ static int clk_set_default_rates(struct 
> > > > > > udevice *dev,
> > > > > > dev_dbg(dev,
> > > > > > "could not get assigned clock 
> > > > > > %d (err = %d)\n",
> > > > > > index, ret);
> > > > > > -   continue;
> > > > > > +   /* Skip if it is empty */
> > > > > > +   if (ret == -ENOENT) {
> > > > > > +   ret = 0;
> > > > > > +   continue;
> > > > > > +   }
> > > > > > +
> > > > > > +   return ret;
> > > > > > }
> > > > > > /* This is clk provider device trying to 
> > > > > > program itself
> > > > > 
> > > > > What's your take on this one?  I didn't get reply from Sean.
> > > > 
> > > > I guess, what's the validated upstream dts where this is the case?
> > > > 
> > > 
> > > It was found by incorrect DT. It means I don't think there is any DT which
> > > contains this issue.
> > > But that being said we can extend current clock tests to cover this case.
> > > Please look below.
> > 
> > Well, if the DT is invalid (and yes, we can't easily run the validation
> > suite in U-Boot today), I'd rather go with a debug we can optimize out
> > so that the next person with an invalid DT can more quickly find their
> > problem and we don't work-around it instead.  Or am I missing something?
> 
> There is already dev_dbg print above. It means when user enable DEBUG it
> gets information about it.
> 
> The thing is that it just sync behavior with the linux kernel.
> You can look at
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/clk-conf.c?h=v6.5#n90
> 
> There is also one more example like this.
>   clk-test4 {
>   compatible = "sandbox,clk-test";
>   assigned-clock-rates = <654>, <321>;
>   assigned-clocks = <_sandbox 1>;
>   };
> 
> But if you prefer to fail in all these cases I am also fine with it.

Ah, OK.  Yes, I guess matching the kernel here is a good idea then.  And
yes, updating the tests next.

Reviewed-by: Tom Rini 

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v1] CI: Add jsonschema python module

2023-08-29 Thread Tom Rini
On Tue, Aug 29, 2023 at 04:37:10PM +0200, Andrejs Cainikovs wrote:

> Some TI boards utilizes `ti-board-config` via binman device
> tree node, which when built via binman, triggers schema validation
> for board specific yaml configuration files.
> 
> This change adds jsonschema python module to CI Docker image, which
> allows these targets to be built by CI without errors.
> 
> Signed-off-by: Andrejs Cainikovs 
> ---
>  tools/docker/Dockerfile | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile
> index 3d2b64a355f..2f3121ffcf6 100644
> --- a/tools/docker/Dockerfile
> +++ b/tools/docker/Dockerfile
> @@ -98,6 +98,7 @@ RUN apt-get update && apt-get install -y \
>   python2.7 \
>   python3 \
>   python3-dev \
> + python3-jsonschema \
>   python3-pip \
>   python3-pyelftools \
>   python3-sphinx \

Interesting.  How exactly are you using these CI images? We have
tools/buildman/requirements.txt now to cover the newly-added modules in
CI, as we call pip on that.  But, it's not being used in the prepopulate
the pip cache stage in the Dockerfile.  And, I'm not immune to the idea
that we should instead be installing the distro package here.

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 1/3] configs: stm32f746-disco: limit resolution to 480x272

2023-08-29 Thread Patrick DELAUNAY

Hi,

On 8/20/23 18:24, Dario Binacchi wrote:

The patch fixes the y-resolution, which was causing the creation of a
framebuffer larger than actually needed, resulting in memory waste.

Fixes: cc1b0e7b8e55b ("board: Add display to STM32F746 SoC discovery board")
Signed-off-by: Dario Binacchi 
---

  configs/stm32f746-disco_defconfig | 2 +-
  configs/stm32f746-disco_spl_defconfig | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/configs/stm32f746-disco_defconfig 
b/configs/stm32f746-disco_defconfig
index bb98ee307a6e..8403679d7fa6 100644
--- a/configs/stm32f746-disco_defconfig
+++ b/configs/stm32f746-disco_defconfig
@@ -59,7 +59,7 @@ CONFIG_VIDEO=y
  CONFIG_BACKLIGHT_GPIO=y
  CONFIG_VIDEO_STM32=y
  CONFIG_VIDEO_STM32_MAX_XRES=480
-CONFIG_VIDEO_STM32_MAX_YRES=640
+CONFIG_VIDEO_STM32_MAX_YRES=272
  CONFIG_SPLASH_SCREEN=y
  CONFIG_SPLASH_SCREEN_ALIGN=y
  CONFIG_VIDEO_BMP_RLE8=y
diff --git a/configs/stm32f746-disco_spl_defconfig 
b/configs/stm32f746-disco_spl_defconfig
index 84aaec1e3390..50c2a36784af 100644
--- a/configs/stm32f746-disco_spl_defconfig
+++ b/configs/stm32f746-disco_spl_defconfig
@@ -85,7 +85,7 @@ CONFIG_VIDEO=y
  CONFIG_BACKLIGHT_GPIO=y
  CONFIG_VIDEO_STM32=y
  CONFIG_VIDEO_STM32_MAX_XRES=480
-CONFIG_VIDEO_STM32_MAX_YRES=640
+CONFIG_VIDEO_STM32_MAX_YRES=272
  CONFIG_SPLASH_SCREEN=y
  CONFIG_SPLASH_SCREEN_ALIGN=y
  CONFIG_VIDEO_BMP_RLE8=y



Reviewed-by: Patrick Delaunay 

Thanks
Patrick




Re: [PATCH v2 1/2] arm: stm32mp: Really fix compilation issue when SYS_DCACHE_OFF and/or SYS_DCACHE_SYS are enabled

2023-08-29 Thread Patrick DELAUNAY

Hi,

On 8/22/23 09:51, Bhupesh Sharma wrote:

While 23e20b2fa6 ("arm: stm32mp: Fix compilation issue when
SYS_DCACHE_OFF and/or SYS_DCACHE_SYS are enabled") tried fixing
this issue, fix it really by adding #if checks for SYS_ICACHE_OFF
and SYS_DCACHE_OFF.

Cc: Patrice Chotard 
Cc: Patrick Delaunay 
Signed-off-by: Bhupesh Sharma 
---
  arch/arm/mach-stm32mp/cpu.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c
index e2f67fc423..8ed065b389 100644
--- a/arch/arm/mach-stm32mp/cpu.c
+++ b/arch/arm/mach-stm32mp/cpu.c
@@ -90,10 +90,10 @@ static void early_enable_caches(void)
if (CONFIG_IS_ENABLED(SYS_DCACHE_OFF))
return;
  
-	if (!(CONFIG_IS_ENABLED(SYS_ICACHE_OFF) && CONFIG_IS_ENABLED(SYS_DCACHE_OFF))) {

+#if !(CONFIG_IS_ENABLED(SYS_ICACHE_OFF) && CONFIG_IS_ENABLED(SYS_DCACHE_OFF))
gd->arch.tlb_size = PGTABLE_SIZE;
gd->arch.tlb_addr = (unsigned long)_tlb;
-   }
+#endif
  
  	/* enable MMU (default configuration) */

dcache_enable();



Reviewed-by: Patrick Delaunay 

Thanks
Patrick




[PATCH v1] CI: Add jsonschema python module

2023-08-29 Thread Andrejs Cainikovs
Some TI boards utilizes `ti-board-config` via binman device
tree node, which when built via binman, triggers schema validation
for board specific yaml configuration files.

This change adds jsonschema python module to CI Docker image, which
allows these targets to be built by CI without errors.

Signed-off-by: Andrejs Cainikovs 
---
 tools/docker/Dockerfile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile
index 3d2b64a355f..2f3121ffcf6 100644
--- a/tools/docker/Dockerfile
+++ b/tools/docker/Dockerfile
@@ -98,6 +98,7 @@ RUN apt-get update && apt-get install -y \
python2.7 \
python3 \
python3-dev \
+   python3-jsonschema \
python3-pip \
python3-pyelftools \
python3-sphinx \
-- 
2.34.1



Re: [PATCH] sunxi: psci: remove redundant initialization from psci_arch_init

2023-08-29 Thread Chen-Yu Tsai
(resent from kernel.org address)

On Tue, Aug 29, 2023 at 5:49 AM Sam Edwards  wrote:
>
> On 8/26/23 04:22, Marc Zyngier wrote:
>
> Hi Marc!
>
> > The GIC definitely has the NS bit routed to it. Otherwise, the secure
> > configuration would just be an utter joke. Just try it.
>
> Thank you for your response. I'd like to revisit my prior point about
> the distinction between the NS bit and AxPROT[1] bits in the context of
> monitor mode: in monitor mode, the NS bit does not determine the
> security state of the CPU core (monitor mode is always secure). But even
> then, the NS bit is still significant for other purposes, such as to
> bank accesses to certain CP15 registers -- and if I understand Chen-Yu
> correctly, some GIC registers also. That would require a special NS bit
> signal routed to the GIC so that it can distinguish between "secure,
> NS=0" and "secure, NS=1" accesses, which is why I asked if such a thing
> exists.
>
> I understand that the GIC is designed to be aware of the security state
> (using the existing AxPROT[1] signals) so that it can protect the
> sensitive registers. And unless I misunderstand, this seems to be the
> point that you made here (my interpretation -- correct me if I'm wrong
> -- is that you are using "NS bit" as a metonym for "security state").
> However I must clarify that my question was to seek further information
> from Chen-Yu about the possibility that NS is significant when accessing
> the GIC, even in monitor mode. Alternatively, his point might be merely
> highlighting that the GIC permits different types of access depending on
> the CPU's security state, which aligns with the viewpoint you've reiterated.
>
> I apologize if my previous message didn't convey this context clearly
> enough. My goal was to unravel this nuanced aspect of the NS bit when in
> monitor mode, and to determine if NS needs to be getting set/cleared
> during GIC setup to maneuver around the banking, or if the value of the
> NS bit when in psci_arch_init() is truly of no consequence due to
> monitor mode.

Sorry for my previous misleading comment. The banked registers refer to
the CP15 registers. From ARM's documentation [1]:

When the processor is in Monitor mode, the processor is in Secure state
regardless of the value of the SCR.NS bit. In Monitor mode, the SCR.NS
bit determines whether the Secure Banked CP15 registers or Non-secure
Banked CP15 registers are read or written using MRC or MCR instructions.

[1] 
https://developer.arm.com/documentation/ddi0406/b/System-Level-Architecture/Virtual-Memory-System-Architecture--VMSA-/CP15-registers-for-a-VMSA-implementation/Effect-of-the-Security-Extensions-on-the-CP15-registers?lang=en#Cbbdffad

> > Well, history is unfortunately against you on that front. Running on
> > the secure side definitely was a requirement when this code was
> > initially written, as the AW BSP *required* to run on the secure side.
> >
> > If that requirement is no more, great. But I don't think you can
> > decide that unilaterally.
>
> I have no idea when/if this requirement was changed. It might have never
> happened "formally": perhaps at some point, the SCR.NS=1 code got added
> after the call to psci_arch_init(), breaking (that version of) the AW
> BSP, and nobody ever complained or changed it back... so it stayed.

There was never any BSP code for PSCI before this. This code was written
by Marc in ARM assembly. I merely translated most of it to C code.
The snippet to clear SCR.NS is there in the first commit,

d5db7024aafc sunxi: HYP/non-sec: add sun7i PSCI backend

ChenYu

> But we're starting to digress from what _this_ patch does. The intent
> here is only to remove two lines of code that (we're discussing to
> confirm) have no effect. I'm not touching the code that *actually*
> determines the world into which monitor mode exits.
>
> Cheers,
> Sam


Re: [U-BOOT TEST HOOKS PATCH] travis-ci: Do not run TPM tests on Versal QEMU target

2023-08-29 Thread Tom Rini


On Mon, 28 Aug 2023 16:29:53 +0200, Michal Simek wrote:
> TPM is going to be enabled by default but QEMU doesn't model it over SPI
> that's why disable it for xilinx_versal_virt_qemu target.
> 
> 

Applied, thanks!

[1/1] travis-ci: Do not run TPM tests on Versal QEMU target
  commit: 3c736fb3a2c81d3ffc2ae22d3ee264651ad6a7f9

Best regards,
-- 
Tom



Re: [PATCH] spl: crypto: fix including SHA* object files in SPL

2023-08-29 Thread Tom Rini
On Wed, Aug 23, 2023 at 05:56:27PM +0300, Oleksandr Suvorov wrote:

> If one of SHA* algorithms is disabled in u-boot, its code is not
> included in SPL even if a given SHA* option is enabled in SPL. Fix
> this.
> 
> Fixes: 603d15a572d ("spl: cypto: Bring back SPL_ versions of SHA")
> Signed-off-by: Oleksandr Suvorov 
> Reviewed-by: Tom Rini 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH] bootstd: Adjust the default bootmeth order

2023-08-29 Thread Tom Rini
On Sat, Aug 19, 2023 at 04:49:35PM -0600, Simon Glass wrote:

> The existing distro scripts check extlinux and scripts before EFI. Adjust
> the default ordering to do the same, to avoid breaking existing flows.
> 
> Add some documentation, mentioning that this order will likely change in
> future.
> 
> Signed-off-by: Simon Glass 
> Reported-by: Da Xue 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 1/4] Azure: Set the timeout for jobs to the maximum

2023-08-29 Thread Tom Rini
On Sun, Aug 20, 2023 at 01:31:26PM -0400, Tom Rini wrote:

> As per current Azure Pipelines documentation we qualify for 3600 minutes
> per job, if specified, as the timeout. The default unspecified timeout
> is 60 minutes. Rework things to specify 0 as the timeout (and so maximum
> allowed) so that we don't have failures due to running slightly past 60
> minutes total.
> 
> Link: 
> https://learn.microsoft.com/en-us/azure/devops/pipelines/process/phases?view=azure-devops=yaml#timeouts
> Signed-off-by: Tom Rini 
> Reviewed-by: Simon Glass 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH] boot: Fix reference to bootmenu doc

2023-08-29 Thread Tom Rini
On Fri, Aug 18, 2023 at 03:54:10PM +0100, Peter Robinson wrote:

> The Kconfig references a readme file that's moved and
> converted to rst so update the reference.
> 
> Signed-off-by: Peter Robinson 
> Reviewed-by: Simon Glass 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 2/2] Revert "binman: Add a temporary hack for duplicate phandles"

2023-08-29 Thread Tom Rini
On Wed, Aug 23, 2023 at 07:18:02PM -0600, Simon Glass wrote:

> The affected boards have been fixed, so drop this hack.
> 
> This reverts commit 288ae53cb73605500b7fc01e5919753c878466be.
> 
> Signed-off-by: Simon Glass 
> Acked-by: Tim Harvey 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 1/1] MAINTAINERS: remove Wolfgang Denk

2023-08-29 Thread Tom Rini
On Thu, Aug 24, 2023 at 12:12:47AM +0200, Heinrich Schuchardt wrote:

> Signed-off-by: Heinrich Schuchardt 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 1/2] imx: Drop unneeded phandle in FIT template

2023-08-29 Thread Tom Rini
On Wed, Aug 23, 2023 at 07:18:01PM -0600, Simon Glass wrote:

> Adding a phandle to a template node is not allowed, since when the node is
> instantiated multiple times, we end up with duplicate phandles.
> 
> Drop this invalid constructs.
> 
> Signed-off-by: Simon Glass 
> Acked-by: Tim Harvey 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH] Revert "arm: imx: mx7: Move CONFIG_OPTEE_TZDRAM_SIZE from lib/optee"

2023-08-29 Thread Tom Rini
On Fri, Aug 25, 2023 at 04:47:11PM +0300, Oleksandr Suvorov wrote:

> From: Ricardo Salveti 
> 
> This reverts commit c5b68ef8af3c2f515c1f5b8d63a69359a85d753b.
> 
> CONFIG_OPTEE_TZDRAM_SIZE is used by imx6-based SoCs as well. Move the
> option back.
> 
> Signed-off-by: Ricardo Salveti 
> Signed-off-by: Oleksandr Suvorov 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH] configs: set CONFIG_LMB_MAX_REGIONS=64 for MT7988 boards

2023-08-29 Thread Tom Rini
On Mon, Aug 21, 2023 at 08:38:23PM +0100, Daniel Golle wrote:

> Similar to MT7981 and MT7986 also MT7988 can have a high number of
> reserved-memory regions used by the various hardware offloading
> subsystems.
> 
> Raise CONFIG_LMB_MAX_REGIONS to 64 to avoid errors when trying to boot
> Linux with more then 6 reserved regions:
> 
> ERROR: reserving fdt memory region failed (addr=4f70 size=24 flags=4)
> ERROR: reserving fdt memory region failed (addr=15194000 size=1000 flags=4)
> ERROR: reserving fdt memory region failed (addr=15294000 size=1000 flags=4)
> ERROR: reserving fdt memory region failed (addr=15394000 size=1000 flags=4)
> ERROR: Failed to allocate 0xb161 bytes below 0x8000.
> device tree - allocation error
> 
> Fixes: bc4adc97cfb ("board: mediatek: add MT7988 reference boards")
> Reported-by: Lorenzo Bianconi 
> Signed-off-by: Daniel Golle 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH] configs: Enable CONFIG_DM_SCSI in am57xx_hs_evm_usb

2023-08-29 Thread Tom Rini
On Mon, Aug 21, 2023 at 08:59:10AM -0500, Andrew Davis wrote:

> This should have already been enabled but was missed when converting the
> base platform defconfig, fix this here.
> 
> Fixes: 3c5aa6caccab ("configs: Enable CONFIG_BLK in am57xx_evm and 
> am57xx_hs_evm")
> Signed-off-by: Andrew Davis 
> Reviewed-by: Tom Rini 
> Reviewed-by: Simon Glass 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH] arm: dts: mediatek: convert gmac link mode to 2500base-x for r3

2023-08-29 Thread Tom Rini
On Thu, Aug 03, 2023 at 06:52:58PM +0200, Frank Wunderlich wrote:

> From: Frank Wunderlich 
> 
> Ethernet on Bananapi-r3 is broken after
> 
> commit bd70f3cea353 ("net: mediatek: add support for SGMII 1Gbps 
> auto-negotiation mode")
> 
> because changes from this commit were not applied to bpi-r3 devicetree too:
> 
> commit aef54ea16cac ("arm: dts: medaitek: convert gmac link mode to 
> 2500base-x")
> 
> Signed-off-by: Frank Wunderlich 
> Reviewed-by: Weijie Gao 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v4 5/8] cmd: gpt: Add command to set bootable flags

2023-08-29 Thread Joshua Watt
On Mon, Aug 28, 2023, 4:53 PM Heinrich Schuchardt 
wrote:

> On 8/28/23 23:56, Joshua Watt wrote:
> > Adds a command that can be used to modify the GPT partition table to
> > indicate which partitions should have the bootable flag set
> >
> > Signed-off-by: Joshua Watt 
> > ---
> >   cmd/gpt.c | 80 +++
> >   doc/usage/cmd/gpt.rst | 12 ++
> >   test/py/tests/test_gpt.py | 22 +++
> >   3 files changed, 114 insertions(+)
> >
> > diff --git a/cmd/gpt.c b/cmd/gpt.c
> > index c6c8282ac9..45fbe07404 100644
> > --- a/cmd/gpt.c
> > +++ b/cmd/gpt.c
> > @@ -972,6 +972,81 @@ static int do_rename_gpt_parts(struct blk_desc
> *dev_desc, char *subcomm,
> >   free(partitions_list);
> >   return ret;
> >   }
> > +
> > +/**
> > + * gpt_set_bootable() - Set bootable flags for partitions
> > + *
> > + * Sets the bootable flag for any partition names in the comma
> separated list of
> > + * partition names. Any partitions not in the list have their bootable
> flag
> > + * cleared
> > + *
> > + * @desc: block device descriptor
> > + * @name: Comma separated list of partition names
> > + *
> > + * @Return: '0' on success and -ve error on failure
> > + */
> > +static int gpt_set_bootable(struct blk_desc *blk_dev_desc, char *const
> part_list)
> > +{
> > + char *name;
> > + char disk_guid[UUID_STR_LEN + 1];
> > + struct list_head *pos;
> > + struct disk_part *curr;
> > + struct disk_partition *partitions = NULL;
> > + int part_count = 0;
> > + int ret = get_disk_guid(blk_dev_desc, disk_guid);
> > +
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = get_gpt_info(blk_dev_desc);
> > + if (ret <= 0)
> > + goto out;
> > +
> > + part_count = ret;
> > + partitions = malloc(sizeof(*partitions) * part_count);
> > + if (!partitions) {
> > + ret = -ENOMEM;
> > + goto out;
> > + }
> > +
> > + /* Copy partitions and clear bootable flag */
> > + part_count = 0;
> > + list_for_each(pos, _partitions) {
> > + curr = list_entry(pos, struct disk_part, list);
> > + partitions[part_count] = curr->gpt_part_info;
> > + partitions[part_count].bootable &= ~PART_BOOTABLE;
> > + part_count++;
> > + }
> > +
> > + name = strtok(part_list, ",");
> > + while (name) {
> > + bool found = false;
> > +
> > + for (int i = 0; i < part_count; i++) {
> > + if (strcmp((char *)partitions[i].name, name) == 0)
> {
> > + partitions[i].bootable |= PART_BOOTABLE;
> > + found = true;
> > + }
> > + }
> > +
> > + if (!found) {
> > + printf("Warning: No partition matching '%s'
> found\n",
> > +name);
> > + }
> > +
> > + name = strtok(NULL, ",");
> > + }
> > +
> > + ret = gpt_restore(blk_dev_desc, disk_guid, partitions, part_count);
> > +
> > +out:
> > + del_gpt_info();
> > +
> > + if (partitions)
> > + free(partitions);
> > +
> > + return ret;
> > +}
> >   #endif
> >
> >   /**
> > @@ -1031,6 +1106,8 @@ static int do_gpt(struct cmd_tbl *cmdtp, int flag,
> int argc, char *const argv[])
> >   } else if ((strcmp(argv[1], "swap") == 0) ||
> >  (strcmp(argv[1], "rename") == 0)) {
> >   ret = do_rename_gpt_parts(blk_dev_desc, argv[1], argv[4],
> argv[5]);
> > + } else if ((strcmp(argv[1], "set-bootable") == 0)) {
> > + ret = gpt_set_bootable(blk_dev_desc, argv[4]);
> >   #endif
> >   } else {
> >   return CMD_RET_USAGE;
> > @@ -1082,8 +1159,11 @@ U_BOOT_CMD(gpt, CONFIG_SYS_MAXARGS, 1, do_gpt,
> >   "  and vice-versa\n"
> >   " gpt rename\n"
> >   "- rename the specified partition\n"
> > + " gpt set-bootable   \n"
> > + "- make partition names in list bootable\n"
> >   " Example usage:\n"
> >   " gpt swap mmc 0 foo bar\n"
> >   " gpt rename mmc 0 3 foo\n"
> > + " gpt set-bootable mmc 0 boot_a,boot_b\n"
> >   #endif
> >   );
> > diff --git a/doc/usage/cmd/gpt.rst b/doc/usage/cmd/gpt.rst
> > index b505b159d0..288dd365c0 100644
> > --- a/doc/usage/cmd/gpt.rst
> > +++ b/doc/usage/cmd/gpt.rst
> > @@ -13,6 +13,7 @@ Synopsis
> >   gpt read   []
> >   gpt rename
> >   gpt repair  
> > +gpt set-bootable   
> >   gpt setenv   
> >   gpt swap
> >   gpt verify   []
> > @@ -90,6 +91,13 @@ gpt repair
> >
> >   Repairs the GPT partition tables if it they become corrupted.
> >
> > +gpt set-bootable
> > +
> > +
> > +Sets the bootable flag for all partitions in the table. If the
> partition name
> > +is in 'partition list' (separated by ','), the bootable flag is set,
> otherwise
> > +it is cleared. CONFIG_CMD_GPT_RENAME=y is required.
>
> Why 

Re: [PATCH v4 2/8] doc: Add gpt command documentation

2023-08-29 Thread Joshua Watt
On Mon, Aug 28, 2023, 4:58 PM Heinrich Schuchardt 
wrote:

> On 8/29/23 00:45, Heinrich Schuchardt wrote:
> > /On 8/28/23 23:56, Joshua Watt wrote:
> >> Adds initial documentation for the gpt command
> >>
> >> Signed-off-by: Joshua Watt 
> >> ---
> >>   doc/usage/cmd/gpt.rst | 184 ++
> >>   doc/usage/index.rst   |   1 +
> >>   2 files changed, 185 insertions(+)
> >>   create mode 100644 doc/usage/cmd/gpt.rst
> >>
> >> diff --git a/doc/usage/cmd/gpt.rst b/doc/usage/cmd/gpt.rst
> >> new file mode 100644
> >> index 00..6387c8116f
> >> --- /dev/null
> >> +++ b/doc/usage/cmd/gpt.rst
> >> @@ -0,0 +1,184 @@
> >> +.. SPDX-License-Identifier: GPL-2.0+
> >> +
> >> +gpt command
> >> +===
> >> +
> >> +Synopsis
> >> +
> >> +
> >> +::
> >> +
> >> +gpt enumerate  
> >> +gpt guid   []
> >> +gpt read   []
> >> +gpt rename
> >> +gpt repair  
> >> +gpt setenv   
> >> +gpt swap
> >> +gpt verify   []
> >> +gpt write   
> >> +
> >> +Description
> >> +---
> >> +
> >> +The gpt command lets users read, create, modify, or verify the GPT
> (GUID
> >> +Partition Table) partition layout.
> >> +
> >> +Common arguments:
> >> +
> >> +interface
> >> +interface for accessing the block device (mmc, sata, scsi, usb,
> >> )
> >> +
> >> +dev
> >> +device number
> >> +
> >> +partition string
> >> +Describes the GPT partition layout for a disk.  The syntax is
> >> similar to
> >> +the one used by the :doc:`mbr command ` command. The string
> >> contains
> >> +one or more partition descriptors, each separated by a ";". Each
> >> descriptor
> >> +contains one or more fields, with each field separated by a ",".
> >> Fields are
> >> +either of the form "key=value" to set a specific value, or simple
> >> "flag" to
> >> +set a boolean flag
> >> +
> >> +The first descriptor can optionally be used to describe
> >> parameters for the
> >> +whole disk with the following fields:
> >> +
> >> +* uuid_disk=UUID - Set the UUID for the disk
> >> +
> >> +Partition descriptors can have the following fields:
> >> +
> >> +* name= - The partition name, required
> >> +* start= - The partition start offset in bytes, required
> >
> > The code has this comment: "start address is optional".
> >
> >> +* size= - The partition size in bytes or "-" to expand it
> >> to the whole free area
> >
> > This filed is mandatory.
> >
> >> +* bootable - Set the legacy bootable flag
> >
> > This field is optional
> >
> >> +* uuid= - The partition UUID, optional if
> >> CONFIG_RANDOM_UUID=y is enabled
> >> +* type= - The partition type GUID, requires
> >> CONFIG_PARTITION_TYPE_GUID=y
> >
> > This field is optional
> >
> > Thanks for all the updates. Looks much neater now.
> >
> > The sequence expected in set_gpt_info() is:
> >
> > uuid (optional if CONFIG_RANDOM_UUID=y),
> > type (optional, only usable for CONFIG_PARTITION_TYPE_GUID=y),
> > name (mandatory),
> > size (mandatory),
> > start (optional),
> > bootable (optional).
>
> A gross bug in the gpt command is that 'gpt read' creates a different
> sequence than 'gpt write' needs.
>
> Furthermore 'gpt read' does not write all fields needed by 'gpt write'
> to recreate the partition table: bootable and type are missing.
>

This is fixed in later patches in this series


> Best regards
>
> Heinrich
>
> >
> >  From the description above it is not clear that:
> >
> > * semicolons are used to separate partitions
> > * the exact sequence of fields must be kept
> > * commas are used to separate fields
> > * no extra white-space is allowed.
> >
> >> +
> >> +
> >> +If 'uuid' is not specified, but CONFIG_RANDOM_UUID is enabled, a
> >> random UUID
> >> +will be generated for the partition
> >> +
> >> +gpt enumerate
> >> +~
> >> +
> >> +Sets the variable 'gpt_partition_list' to be a list of all the
> >> partition names
> >> +on the device.
> >> +
> >> +gpt guid
> >> +
> >> +
> >> +Report the GUID of a disk. If 'varname' is specified, the command
> >> will set the
> >> +variable to the GUID, otherwise it will be printed out.
> >> +
> >> +gpt read
> >> +
> >> +
> >> +Prints the current state of the GPT partition table. If 'varname' is
> >> specified,
> >> +the variable will be filled with a partition string in the same
> >> format as a
> >> +'', suitable for passing to other 'gpt' commands.
> >> If the
> >> +argument is omitted, a human readable description is printed out.
> >> +CONFIG_CMD_GPT_RENAME=y is required.
> >> +
> >> +gpt rename
> >> +~~
> >> +
> >> +Renames all partitions named 'part' to be 'name'.
> >> CONFIG_CMD_GPT_RENAME=y is
> >> +required.
> >> +
> >> +gpt repair
> >> +~~
> >> +
> >> +Repairs the GPT partition tables if it they become corrupted.
> >> +
> >> +gpt setenv
> >> +~~
> >> +
> >> +The 'gpt setenv' command will set a series of environment variables
> with
> >> +information 

Re: [PATCH] fpga: define dummy fpga_load function for debug build

2023-08-29 Thread Michal Simek




On 8/16/23 08:54, Chanho Park wrote:

This fixes below build error when CC_OPTIMIZE_FOR_DEBUG is enabled and
CONFIG_SPL_FPGA is not enabled.


I would rewrite this because the connection to SPL_FPGA is just one part of it.
It is also taken when CONFIG_FPGA is not enabled.




../common/spl/spl_fit.c:591: undefined reference to `fpga_load'
collect2: error: ld returned 1 exit status

Signed-off-by: Chanho Park 
---
  include/fpga.h | 8 
  1 file changed, 8 insertions(+)

diff --git a/include/fpga.h b/include/fpga.h
index ed688cc0fa3b..44f2755a3f10 100644
--- a/include/fpga.h
+++ b/include/fpga.h
@@ -60,8 +60,16 @@ int fpga_add(fpga_type devtype, void *desc);
  int fpga_count(void);
  const fpga_desc *const fpga_get_desc(int devnum);
  int fpga_is_partial_data(int devnum, size_t img_len);
+#if CONFIG_IS_ENABLED(FPGA)
  int fpga_load(int devnum, const void *buf, size_t bsize,
  bitstream_type bstype, int flags);
+#else
+static inline int fpga_load(int devnum, const void *buf, size_t bsize,
+ bitstream_type bstype, int flags)
+{
+   return FPGA_FAIL;
+}
+#endif
  int fpga_fsload(int devnum, const void *buf, size_t size,
fpga_fs_info *fpga_fsinfo);
  int fpga_loads(int devnum, const void *buf, size_t size,


Thanks,
Michal


Re: [PATCH v2 5/6] docs: ti: j721s2_evm: Create documentation from J7200 docs

2023-08-29 Thread Manorit Chawdhry
Hi Nishanth,

On 08:30-20230828, Nishanth Menon wrote:
> i$subject: doc: board: ti: Add j721s2-evm documentation

Will be updating it, thanks.

> 
> On 16:47-20230825, Manorit Chawdhry wrote:
> > The documentation is based off J7200 documentation tailored for J721S2.
> > 
> > TRM for J721S2: https://www.ti.com/lit/pdf/spruj28
> > Product Page: https://www.ti.com/product/TDA4AL-Q1
> > 
> > Signed-off-by: Manorit Chawdhry 
> > ---
> >  doc/board/ti/j721s2_evm.rst | 263 
> > 
> >  doc/board/ti/k3.rst |   1 +
> >  2 files changed, 264 insertions(+)
> > 
> > diff --git a/doc/board/ti/j721s2_evm.rst b/doc/board/ti/j721s2_evm.rst
> > new file mode 100644
> > index ..1b560eb4a60e
> > --- /dev/null
> > +++ b/doc/board/ti/j721s2_evm.rst
> > @@ -0,0 +1,263 @@
> > +.. SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause
> > +.. sectionauthor:: Manorit Chawdhry 
> > +
> > +J721S2 Platforms
> > +
> > +
> > +Introduction:
> > +-
> > +The J721S2 family of SoCs are part of K3 Multicore SoC architecture 
> > platform
> > +targeting automotive applications. They are designed as a low power, high
> > +performance and highly integrated device architecture, adding significant
> > +enhancement on processing power, graphics capability, video and imaging
> > +processing, virtualization and coherent memory support.
> > +
> > +The device is partitioned into three functional domains, each containing
> > +specific processing cores and peripherals:
> > +
> > +1. Wake-up (WKUP) domain:
> > +* ARM Cortex-M4F processor, runs TI Foundational Security (TIFS)
> > +
> > +2. Microcontroller (MCU) domain:
> > +* Dual core ARM Cortex-R5F processor, runs device management
> > +  and SoC early boot
> > +
> > +3. MAIN domain:
> > +* Dual core 64-bit ARM Cortex-A72, runs HLOS
> > +
> > +More info can be found in TRM: https://www.ti.com/lit/pdf/spruj28
> > +
> > +Platform information:
> > +
> > +* https://www.ti.com/tool/J721S2XSOMXEVM
> 
> How about am68-sk documentation?

Thanks for catching this! Missed it. Will be updating it.

> 
> > +
> > +Boot Flow:
> > +--
> > +Below is the pictorial representation of boot flow:
> > +
> > +.. image:: img/boot_diagram_k3_current.svg
> > +
> > +- On this platform, "TI Foundational Security" (TIFS) functions as the
> > +  security enclave master while "Device Manager" (DM), also known as the
> > +  "TISCI server" in TI terminology, offers all the essential services.
> > +
> > +- As illustrated in the diagram above, R5 SPL manages power and clock
> > +  services independently before handing over control to "DM". The A72 or
> > +  the C7x (Aux core) software components request TIFS/DM to handle
> > +  security or device management services.
> > +
> > +Sources:
> > +
> > +
> > +.. include::  k3.rst
> > +:start-after: .. k3_rst_include_start_boot_sources
> > +:end-before: .. k3_rst_include_end_boot_sources
> > +
> > +Build procedure:
> > +
> > +0. Setup the environment variables:
> > +
> > +.. include::  k3.rst
> > +:start-after: .. k3_rst_include_start_common_env_vars_desc
> > +:end-before: .. k3_rst_include_end_common_env_vars_desc
> > +
> > +.. include::  k3.rst
> > +:start-after: .. k3_rst_include_start_board_env_vars_desc
> > +:end-before: .. k3_rst_include_end_board_env_vars_desc
> > +
> > +Set the variables corresponding to this platform:
> > +
> > +.. include::  k3.rst
> > +:start-after: .. k3_rst_include_start_common_env_vars_defn
> > +:end-before: .. k3_rst_include_end_common_env_vars_defn
> > +.. code-block:: bash
> > +
> > + $ export UBOOT_CFG_CORTEXR=j721s2_evm_r5_defconfig
> > + $ export UBOOT_CFG_CORTEXA=j721s2_evm_a72_defconfig
> > + $ export TFA_BOARD=generic
> > + $ export TFA_EXTRA_ARGS="K3_USART=0x8"
> > + $ # This is not a typo, j784s4 is the
> 
> is the  ?
> 
> > + $ # OP-TEE platform for j721s2

The next line completes it, had split the line in the comments though I
don't think it would be required given the line length. Would fix it.
Thanks.

Also, just realised that "This" is misleading. Would rework it to use
"The following".

> > + $ export OPTEE_PLATFORM=k3-j784s4
> > + $ export OPTEE_EXTRA_ARGS="CFG_CONSOLE_UART=0x8"
> > +
> > +.. j721s2_evm_rst_include_start_build_steps
> > +
> > +1. Trusted Firmware-A:
> > +
> > +.. include::  k3.rst
> > +:start-after: .. k3_rst_include_start_build_steps_tfa
> > +:end-before: .. k3_rst_include_end_build_steps_tfa
> > +
> > +
> > +2. OP-TEE:
> > +
> > +.. include::  k3.rst
> > +:start-after: .. k3_rst_include_start_build_steps_optee
> > +:end-before: .. k3_rst_include_end_build_steps_optee
> > +
> > +3. U-Boot:
> > +
> > +.. _j721s2_evm_rst_u_boot_r5:
> > +
> > +* 3.1 R5:
> > +
> > +.. include::  k3.rst
> > +:start-after: .. k3_rst_include_start_build_steps_spl_r5
> > +:end-before: .. k3_rst_include_end_build_steps_spl_r5
> > +
> > +.. 

Re: [RFC PATCH 0/5] Allow for removal of DT nodes and properties

2023-08-29 Thread Peter Robinson
> > > > > > Provide a way for removing certain devicetree nodes and/or 
> > > > > > properties
> > > > > > from the devicetree. This is needed to purge certain nodes and
> > > > > > properties which may be relevant only in U-Boot. Such nodes and
> > > > > > properties are then removed from the devicetree before it is passed 
> > > > > > to
> > > > > > the kernel. This ensures that the devicetree passed to the OS does 
> > > > > > not
> > > > > > contain any non-compliant nodes and properties.
> > > > > >
> > > > > > The removal of the nodes and properties is being done through an
> > > > > > EVT_FT_FIXUP handler. I am not sure if the removal code needs to be
> > > > > > behind any Kconfig symbol.
> > > > > >
> > > > > > I have only build tested this on sandbox, and tested on qemu arm64
> > > > > > virt platform. This being a RFC, I have not put this through a CI 
> > > > > > run.
> > > > > >
> > > > > > Sughosh Ganu (5):
> > > > > >   dt: Provide a way to remove non-compliant nodes and properties
> > > > > >   fwu: Add the fwu-mdata node for removal from devicetree
> > > > > >   capsule: Add the capsule-key property for removal from devicetree
> > > > > >   bootefi: Call the EVT_FT_FIXUP event handler
> > > > > >   doc: Add a document for non-compliant DT node/property removal
> > > > > >
> > > > > >  cmd/bootefi.c | 18 +
> > > > > >  .../devicetree/dt_non_compliant_purge.rst | 64 
> > > > > >  drivers/fwu-mdata/fwu-mdata-uclass.c  |  5 ++
> > > > > >  include/dt-structs.h  | 11 +++
> > > > > >  lib/Makefile  |  1 +
> > > > > >  lib/dt_purge.c| 73 
> > > > > > +++
> > > > > >  lib/efi_loader/efi_capsule.c  |  7 ++
> > > > > >  7 files changed, 179 insertions(+)
> > > > > >  create mode 100644 
> > > > > > doc/develop/devicetree/dt_non_compliant_purge.rst
> > > > > >  create mode 100644 lib/dt_purge.c
> > > > >
> > > > > What is the point of removing them? Instead, we should make sure that
> > > > > we upstream the bindings and encourage SoC vendors to sync them. If we
> > > > > remove them, no one will bother and U-Boot just becomes a dumping
> > > > > ground.
> > > >
> > > > Well things like the binman entries in DT are U-Boot specific and not
> > > > useful for HW related descriptions or for Linux or another OS being
> > > > able to deal with HW so arguably we're already a dumping ground to
> > > > some degree for not defining hardware.
> > >
> > > I have started the process to upstream the binman bindings.
> >
> > I don't think they should be in DT at all, they don't describe
> > anything to do with hardware, or generally even the runtime of a
> > device, they don't even describe the boot/runtime state of the
> > firmware, they describe build time, so I don't see what that has to do
> > with device tree? Can you explain that? To me those sorts of things
> > should live in a build time style config file.
>
> I beg to differ. Devicetree is more than just hardware and always has
> been. See, for example the /chosen and /options nodes.

Can you give me an example of "options" as grep doesn't give me a
single one in the kernel tree? I think we can just agree to disagree
here.

> We need run-time configuration here, since we cannot know at build
> time what we will be asked to do by a previous firmware phase.

Can you provide an example as to how that is used during runtime? Do
you mean runtime during the build process or runtime on the device?

> >
> > > Perhaps we should use the issue tracker[1] to follow progress on all
> > > of this. We need to clean it up.
> > >
> > > >
> > > > > Instead of this, how about working on bringing the DT validation into
> > > > > U-Boot so we can see what state things are in?
> > > > >
> > > > > Please send the bindings for Linaro's recent series (which I suspect
> > > > > are motivating this series) so we can start the process.
> > >
> > > Regards,
> > > Simon
> > >
> > > [1] https://source.denx.de/u-boot/u-boot/-/issues
>
> Regards,
> Simon


Re: [PATCH v3 5/9] board: qualcomm: Add support for dragonboard845c

2023-08-29 Thread Sumit Garg
Hi Simon,

On Tue, 29 Aug 2023 at 03:39, Simon Glass  wrote:
>
> Hi Peter,
>
> On Mon, 28 Aug 2023 at 14:24, Peter Robinson  wrote:
> >
> > On Mon, Aug 28, 2023 at 6:55 PM Simon Glass  wrote:
> > >
> > > Hi Sumit,
> > >
> > > On Thu, 24 Aug 2023 at 04:44, Sumit Garg  wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Thu, 24 Aug 2023 at 05:29, Simon Glass  wrote:
> > > > >
> > > > > Hi Sumit,
> > > > >
> > > > > On Tue, 12 Jul 2022 at 01:12, Sumit Garg  
> > > > > wrote:
> > > > > >
> > > > > > Add support for 96Boards Dragonboard 845C aka Robotics RB3 
> > > > > > development
> > > > > > platform. This board complies with 96Boards Open Platform 
> > > > > > Specifications.
> > > > > >
> > > > > > Features:
> > > > > > - Qualcomm Snapdragon SDA845 SoC
> > > > > > - 4GiB RAM
> > > > > > - 64GiB UFS drive
> > > > > >
> > > > > > U-boot is chain loaded by ABL in 64-bit mode as part of boot.img.
> > > > > > For detailed build and boot instructions, refer to
> > > > > > doc/board/qualcomm/sdm845.rst, board: dragonboard845c.
> > > > > >
> > > > > > Signed-off-by: Sumit Garg 
> > > > > > Reviewed-by: Ramon Fried 
> > > > > > ---
> > > > > >  arch/arm/dts/dragonboard845c-uboot.dtsi   |  37 +++
> > > > > >  arch/arm/dts/dragonboard845c.dts  |  44 
> > > > > >  arch/arm/mach-snapdragon/Kconfig  |  14 +++
> > > > > >  board/qualcomm/dragonboard845c/Kconfig|  12 +++
> > > > > >  board/qualcomm/dragonboard845c/MAINTAINERS|   6 ++
> > > > > >  board/qualcomm/dragonboard845c/Makefile   |   9 ++
> > > > > >  board/qualcomm/dragonboard845c/db845c.its |  63 +++
> > > > > >  .../dragonboard845c/dragonboard845c.c |   9 ++
> > > > > >  configs/dragonboard845c_defconfig |  28 +
> > > > > >  doc/board/qualcomm/sdm845.rst | 100 
> > > > > > +++---
> > > > > >  include/configs/dragonboard845c.h |  28 +
> > > > > >  11 files changed, 337 insertions(+), 13 deletions(-)
> > > > > >  create mode 100644 arch/arm/dts/dragonboard845c-uboot.dtsi
> > > > > >  create mode 100644 arch/arm/dts/dragonboard845c.dts
> > > > > >  create mode 100644 board/qualcomm/dragonboard845c/Kconfig
> > > > > >  create mode 100644 board/qualcomm/dragonboard845c/MAINTAINERS
> > > > > >  create mode 100644 board/qualcomm/dragonboard845c/Makefile
> > > > > >  create mode 100644 board/qualcomm/dragonboard845c/db845c.its
> > > > > >  create mode 100644 board/qualcomm/dragonboard845c/dragonboard845c.c
> > > > > >  create mode 100644 configs/dragonboard845c_defconfig
> > > > > >  create mode 100644 include/configs/dragonboard845c.h
> > > > > >
> > > > > [..]
> > > > >
> > > > > > diff --git a/doc/board/qualcomm/sdm845.rst 
> > > > > > b/doc/board/qualcomm/sdm845.rst
> > > > > > index b6642c9579..8ef4749287 100644
> > > > > > --- a/doc/board/qualcomm/sdm845.rst
> > > > > > +++ b/doc/board/qualcomm/sdm845.rst
> > > > > > @@ -35,9 +35,25 @@ Pack android boot image
> > > > > >  ^^^
> > > > > >  We'll assemble android boot image with ``u-boot.bin`` instead of 
> > > > > > linux kernel,
> > > > > >  and FIT image instead of ``initramfs``. Android bootloader expect 
> > > > > > gzipped kernel
> > > > > > -with appended dtb, so let's mimic linux to satisfy stock 
> > > > > > bootloader:
> > > > > > +with appended dtb, so let's mimic linux to satisfy stock 
> > > > > > bootloader.
> > > > > >
> > > > >
> > > > > [..]
> > > > >
> > > > > > +The dragonboard845c is a Qualcomm Robotics RB3 Development 
> > > > > > Platform, based on
> > > > > > +the Qualcomm SDM845 SoC.
> > > > > > +
> > > > > > +Steps:
> > > > > > +
> > > > > > +- Build u-boot::
> > > > > > +
> > > > > > +   $ export CROSS_COMPILE=
> > > > > > +   $ make dragonboard845c_defconfig
> > > > > > +   $ make
> > > > > > +
> > > > > > +- Create dummy dtb::
> > > > > > +
> > > > > > +   workdir=/tmp/prepare_payload
> > > > > > +   mkdir -p "$workdir"
> > > > > > +   mock_dtb="$workdir"/payload_mock.dtb
> > > > > > +
> > > > > > +   dtc -I dts -O dtb -o "$mock_dtb" << EOF
> > > > > > +   /dts-v1/;
> > > > > > +   / {
> > > > > > +   #address-cells = <2>;
> > > > > > +   #size-cells = <2>;
> > > > > > +
> > > > > > +   memory@8000 {
> > > > > > +   device_type = "memory";
> > > > > > +   /* We expect the bootloader to fill in the 
> > > > > > size */
> > > > > > +   reg = <0 0x8000 0 0>;
> > > > > > +   };
> > > > > > +
> > > > > > +   chosen { };
> > > > > > +   };
> > > > > > +   EOF
> > > > > > +
> > > > > > +- gzip u-boot::
> > > > > > +
> > > > > > +   gzip u-boot.bin
> > > > > > +
> > > > > > +- Append dtb to gzipped u-boot::
> > > > > > +
> > > > > > +cat u-boot.bin.gz "$mock_dtb" > u-boot.bin.gz-dtb
> > > > > > +
> > > > > > +- A ``db845c.its`` file can be found in 
> > > > > > 

Re: [PATCH v5 00/13] Add video damage tracking

2023-08-29 Thread Mark Kettenis
> Date: Tue, 29 Aug 2023 08:20:49 +0200
> From: Alexander Graf 
> 
> On 28.08.23 23:54, Heinrich Schuchardt wrote:
> > On 8/28/23 22:24, Alexander Graf wrote:
> >>
> >> On 28.08.23 19:54, Simon Glass wrote:
> >>> Hi Alex,
> >>>
> >>> On Wed, 23 Aug 2023 at 02:56, Alexander Graf  wrote:
>  Hey Simon,
> 
>  On 22.08.23 20:56, Simon Glass wrote:
> > Hi Alex,
> >
> > On Tue, 22 Aug 2023 at 01:47, Alexander Graf  wrote:
> >> On 22.08.23 01:03, Simon Glass wrote:
> >>> Hi Alex,
> >>>
> >>> On Mon, 21 Aug 2023 at 16:40, Alexander Graf  
> >>> wrote:
>  On 22.08.23 00:10, Simon Glass wrote:
> > Hi Alex,
> >
> > On Mon, 21 Aug 2023 at 14:20, Alexander Graf 
> > wrote:
> >> On 21.08.23 21:57, Simon Glass wrote:
> >>> Hi Alex,
> >>>
> >>> On Mon, 21 Aug 2023 at 13:33, Alexander Graf 
> >>> wrote:
>  On 21.08.23 21:11, Simon Glass wrote:
> > Hi Alper,
> >
> > On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak
> >  wrote:
> >> This is a rebase of Alexander Graf's video damage tracking
> >> series, with
> >> some tests and other changes. The original cover letter is
> >> as follows:
> >>
> >>> This patch set speeds up graphics output on ARM by a
> >>> factor of 60x.
> >>>
> >>> On most ARM SBCs, we keep the frame buffer in DRAM and map
> >>> it as cached,
> >>> but need it accessible by the display controller which
> >>> reads directly
> >>> from a later point of consistency. Hence, we flush the
> >>> frame buffer to
> >>> DRAM on every change. The full frame buffer.
> > It should not, see below.
> >
> >>> Unfortunately, with the advent of 4k displays, we are
> >>> seeing frame buffers
> >>> that can take a while to flush out. This was reported by
> >>> Da Xue with grub,
> >>> which happily print 1000s of spaces on the screen to draw
> >>> a menu. Every
> >>> printed space triggers a cache flush.
> > That is a bug somewhere in EFI.
>  Unfortunately not :). You may call it a bug in grub: It
>  literally prints
>  over space characters for every character in its menu that it
>  wants
>  cleared. On every text screen draw.
> 
>  This wouldn't be a big issue if we only flush the reactangle
>  that gets
>  modified. But without this patch set, we're flushing the full
>  DRAM
>  buffer on every u-boot text console character write, which
>  means for
>  every character (as that's the only API UEFI has).
> 
>  As a nice side effect, we speed up the normal U-Boot text
>  console as
>  well with this patch set, because even "normal" text prints
>  that write
>  for example a single line of text on the screen today flush
>  the full
>  frame buffer to DRAM.
> >>> No, I mean that it is a bug that U-Boot (apparently) flushes
> >>> the cache
> >>> after every character. It doesn't do that for normal character
> >>> output
> >>> and I don't think it makes sense to do it for EFI either.
> >> I see. Let's trace the calls:
> >>
> >> efi_cout_output_string()
> >> -> fputs()
> >> -> vidconsole_puts()
> >> -> video_sync()
> >> -> flush_dcache_range()
> >>
> >> Unfortunately grub abstracts character backends down to the 
> >> "print
> >> character" level, so it calls UEFI's sopisticated 
> >> "output_string"
> >> callback with single characters at a time, which means we do a
> >> full
> >> dcache flush for every character that we print:
> >>
> >> https://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/term/efi/console.c#n165
> >>  
> >>
> >>
> >>
> >>> This patch set implements the easiest mitigation against
> >>> this problem:
> >>> Damage tracking. We remember the lowest common denominator
> >>> region that was
> >>> touched since the last video_sync() call and only flush
> >>> that. The most
> >>> typical writer to the frame buffer is the video console,
> >>> which always
> >>> writes rectangles of characters on the screen and syncs
> >>> afterwards.
> >>>
> >>> With this patch set applied, we reduce drawing a large
> >>> grub menu (with
> >>> 

Re: [PATCH] arm: kirkwood: Add support for ZyXEL NSA325 board

2023-08-29 Thread Stefan Roese




On 8/26/23 05:33, Tony Dinh wrote:

ZyXEL NSA325 specifications:

Marvell Kirkwood 88F6282 SoC
1.6 GHz CPU
1x GBE LAN port (Marvell MV88E1318)
512 MB RAM
128 MB Eon NAND, SLC
I2C
1x USB 3.0 (on PCIe bus)
2x USB 2.0
2x SATA (hot swap slots)
Serial console

Signed-off-by: Tony Dinh 
---

  arch/arm/dts/Makefile|   1 +
  arch/arm/dts/kirkwood-6282.dtsi  | 161 
  arch/arm/dts/kirkwood-nsa325.dts | 231 +++
  arch/arm/dts/kirkwood-nsa3x0-common.dtsi | 157 +++


Just checking: Are the DT files imported from the Linux ones?

Other than this:

Reviewed-by: Stefan Roese 

Thanks,
Stefan



  arch/arm/mach-kirkwood/Kconfig   |   7 +
  board/zyxel/nsa325/Kconfig   |  12 ++
  board/zyxel/nsa325/MAINTAINERS   |   9 +
  board/zyxel/nsa325/Makefile  |  11 ++
  board/zyxel/nsa325/kwbimage.cfg  |  55 ++
  board/zyxel/nsa325/nsa325.c  | 196 +++
  configs/nsa325_defconfig |  81 
  include/configs/nsa325.h |  37 
  12 files changed, 958 insertions(+)
  create mode 100644 arch/arm/dts/kirkwood-6282.dtsi
  create mode 100644 arch/arm/dts/kirkwood-nsa325.dts
  create mode 100644 arch/arm/dts/kirkwood-nsa3x0-common.dtsi
  create mode 100644 board/zyxel/nsa325/Kconfig
  create mode 100644 board/zyxel/nsa325/MAINTAINERS
  create mode 100644 board/zyxel/nsa325/Makefile
  create mode 100644 board/zyxel/nsa325/kwbimage.cfg
  create mode 100644 board/zyxel/nsa325/nsa325.c
  create mode 100644 configs/nsa325_defconfig
  create mode 100644 include/configs/nsa325.h

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 85fd5b1157..b9fc46544f 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -66,6 +66,7 @@ dtb-$(CONFIG_ARCH_KIRKWOOD) += \
kirkwood-ns2max.dtb \
kirkwood-ns2mini.dtb \
kirkwood-nsa310s.dtb \
+   kirkwood-nsa325.dtb \
kirkwood-openrd-base.dtb \
kirkwood-openrd-client.dtb \
kirkwood-openrd-ultimate.dtb \
diff --git a/arch/arm/dts/kirkwood-6282.dtsi b/arch/arm/dts/kirkwood-6282.dtsi
new file mode 100644
index 00..e732c501ea
--- /dev/null
+++ b/arch/arm/dts/kirkwood-6282.dtsi
@@ -0,0 +1,161 @@
+// SPDX-License-Identifier: GPL-2.0
+/ {
+   mbus@f100 {
+   pciec: pcie@8200 {
+   compatible = "marvell,kirkwood-pcie";
+   status = "disabled";
+   device_type = "pci";
+
+   #address-cells = <3>;
+   #size-cells = <2>;
+
+   bus-range = <0x00 0xff>;
+
+   ranges =
+  <0x8200 0 0x4 MBUS_ID(0xf0, 0x01) 
0x4 0 0x2000
+   0x8200 0 0x44000 MBUS_ID(0xf0, 0x01) 
0x44000 0 0x2000
+   0x8200 0 0x8 MBUS_ID(0xf0, 0x01) 
0x8 0 0x2000
+   0x8200 0x1 0 MBUS_ID(0x04, 0xe8) 0  
 1 0 /* Port 0.0 MEM */
+   0x8100 0x1 0 MBUS_ID(0x04, 0xe0) 0  
 1 0 /* Port 0.0 IO  */
+   0x8200 0x2 0 MBUS_ID(0x04, 0xd8) 0  
 1 0 /* Port 1.0 MEM */
+   0x8100 0x2 0 MBUS_ID(0x04, 0xd0) 0   1 
0 /* Port 1.0 IO  */>;
+
+   pcie0: pcie@1,0 {
+   device_type = "pci";
+   assigned-addresses = <0x82000800 0 0x0004 0 
0x2000>;
+   reg = <0x0800 0 0 0 0>;
+   #address-cells = <3>;
+   #size-cells = <2>;
+   #interrupt-cells = <1>;
+   ranges = <0x8200 0 0 0x8200 0x1 0 1 0
+ 0x8100 0 0 0x8100 0x1 0 1 0>;
+   bus-range = <0x00 0xff>;
+   interrupt-names = "intx", "error";
+   interrupts = <9>, <44>;
+   interrupt-map-mask = <0 0 0 7>;
+   interrupt-map = <0 0 0 1 _intc 0>,
+   <0 0 0 2 _intc 1>,
+   <0 0 0 3 _intc 2>,
+   <0 0 0 4 _intc 3>;
+   marvell,pcie-port = <0>;
+   marvell,pcie-lane = <0>;
+   clocks = <_clk 2>;
+   status = "disabled";
+
+   pcie0_intc: interrupt-controller {
+   interrupt-controller;
+   #interrupt-cells = <1>;
+   };
+   };
+
+   

Re: [PATCH] arm: kirkwood: Pogo v4: Enable LTO

2023-08-29 Thread Stefan Roese

On 8/24/23 21:07, Tony Dinh wrote:

Enable building Pogo V4 u-boot image with LTO, which results in about 30K
reduction in size.

Signed-off-by: Tony Dinh 


Reviewed-by: Stefan Roese 

Thanks,
Stefan


---

  configs/pogo_v4_defconfig | 1 +
  1 file changed, 1 insertion(+)

diff --git a/configs/pogo_v4_defconfig b/configs/pogo_v4_defconfig
index 3e3e811543..ad36da712b 100644
--- a/configs/pogo_v4_defconfig
+++ b/configs/pogo_v4_defconfig
@@ -16,6 +16,7 @@ CONFIG_DEFAULT_DEVICE_TREE="kirkwood-pogoplug-series-4"
  CONFIG_SYS_PROMPT="Pogo_V4> "
  CONFIG_IDENT_STRING="\nPogoplug V4"
  CONFIG_SYS_LOAD_ADDR=0x80
+CONFIG_LTO=y
  CONFIG_PCI=y
  CONFIG_DISTRO_DEFAULTS=y
  CONFIG_BOOTSTAGE=y


Viele Grüße,
Stefan Roese

--
DENX Software Engineering GmbH,  Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de


Re: [PATCH v1 1/2] doc: board: toradex: minor documentation update

2023-08-29 Thread Heinrich Schuchardt

On 8/29/23 00:01, Marcel Ziswiler wrote:

From: Marcel Ziswiler 

- Update SPDX-License-Identifier from obsolete GPL-2.0+ to
   GPL-2.0-or-later.
- Add links to product websites of SoM and carrier board where missing.
- Add information about update U-Boot wrapper where missing.
- Add sectionauthor where missing.
- Update information about imx-seco from version 3.7.4 to 3.8.1.
- Various minor grammatic and spelling fixes.
- Improve whitespace by adding or removing new lines.
- Change from code-block for output to just Output::.

Signed-off-by: Marcel Ziswiler 
---

  doc/board/toradex/apalis-imx8.rst   | 22 +
  doc/board/toradex/colibri-imx8x.rst | 31 ---
  doc/board/toradex/colibri_imx7.rst  | 38 +++--
  doc/board/toradex/verdin-am62.rst   |  8 +++---
  doc/board/toradex/verdin-imx8mm.rst |  9 +++
  doc/board/toradex/verdin-imx8mp.rst |  9 +++
  6 files changed, 71 insertions(+), 46 deletions(-)

diff --git a/doc/board/toradex/apalis-imx8.rst 
b/doc/board/toradex/apalis-imx8.rst
index 849b1172bd4..ffc4c7d222a 100644
--- a/doc/board/toradex/apalis-imx8.rst
+++ b/doc/board/toradex/apalis-imx8.rst
@@ -1,7 +1,11 @@
-.. SPDX-License-Identifier: GPL-2.0+
+.. SPDX-License-Identifier: GPL-2.0-or-later
+.. sectionauthor:: Marcel Ziswiler 

-Apalis iMX8QM V1.0B Module
-==
+Apalis iMX8 Module
+==
+
+- SoM: https://www.toradex.com/computer-on-modules/apalis-arm-family/nxp-imx-8
+- Carrier board: 
https://www.toradex.com/products/carrier-board/apalis-evaluation-board

  Quick Start
  ---
@@ -49,6 +53,7 @@ Copy the following firmware to the U-Boot folder:

  Build U-Boot
  
+
  .. code-block:: bash

  $ make apalis-imx8_defconfig
@@ -61,8 +66,8 @@ Get the latest version of the universal update utility (uuu) 
aka ``mfgtools 3.0`

  
https://community.nxp.com/external-link.jspa?url=https%3A%2F%2Fgithub.com%2FNXPmicro%2Fmfgtools%2Freleases

-Put the module into USB recovery aka serial downloader mode, connect USB device
-to your host and execute uuu:
+Put the module into USB recovery aka serial downloader mode, connect the USB
+device to your host and execute ``uuu``:

  .. code-block:: bash

@@ -80,3 +85,10 @@ partition and boot:
  setexpr blkcnt ${filesize} + 0x1ff && setexpr blkcnt ${blkcnt} / 0x200
  mmc dev 0 1
  mmc write ${loadaddr} 0x0 ${blkcnt}
+
+As a convenience, instead of the last three commands, one may also use the
+update U-Boot wrapper:
+
+.. code-block:: bash
+
+> run update_uboot
diff --git a/doc/board/toradex/colibri-imx8x.rst 
b/doc/board/toradex/colibri-imx8x.rst
index 545568c844a..9e61d98c6b1 100644
--- a/doc/board/toradex/colibri-imx8x.rst
+++ b/doc/board/toradex/colibri-imx8x.rst
@@ -1,7 +1,11 @@
-.. SPDX-License-Identifier: GPL-2.0+
+.. SPDX-License-Identifier: GPL-2.0-or-later
+.. sectionauthor:: Marcel Ziswiler 

-Colibri iMX8QXP V1.0D Module
-
+Colibri iMX8X Module
+
+
+- SoM: 
https://www.toradex.com/computer-on-modules/colibri-arm-family/nxp-imx-8x
+- Carrier board: 
https://www.toradex.com/products/carrier-board/colibri-evaluation-board

  Quick Start
  ---
@@ -23,18 +27,19 @@ Get and Build the ARM Trusted Firmware

  Get scfw_tcm.bin and ahab-container.img
  ---
+
  .. code-block:: bash

  $ wget 
https://github.com/toradex/i.MX-System-Controller-Firmware/raw/master/src/scfw_export_mx8qx_b0/build_mx8qx_b0/mx8qx-colibri-scfw-tcm.bin
-$ wget https://www.nxp.com/lgfiles/NMG/MAD/YOCTO/imx-seco-3.7.4.bin
-$ sh imx-seco-3.7.4.bin --auto-accept
+$ wget https://www.nxp.com/lgfiles/NMG/MAD/YOCTO/imx-seco-3.8.1.bin
+$ sh imx-seco-3.8.1.bin --auto-accept

  Copy the following firmware to the U-Boot folder:

  .. code-block:: bash

  $ cp imx-atf/build/imx8qx/release/bl31.bin .
-$ cp imx-seco-3.7.4/firmware/seco/mx8qxc0-ahab-container.img 
mx8qx-ahab-container.img
+$ cp imx-seco-3.8.1/firmware/seco/mx8qxc0-ahab-container.img 
mx8qx-ahab-container.img

  Build U-Boot
  
@@ -51,8 +56,8 @@ Get the latest version of the universal update utility (uuu) 
aka ``mfgtools 3.0`

  
https://community.nxp.com/external-link.jspa?url=https%3A%2F%2Fgithub.com%2FNXPmicro%2Fmfgtools%2Freleases

-Put the module into USB recovery aka serial downloader mode, connect USB device
-to your host and execute ``uuu``:
+Put the module into USB recovery aka serial downloader mode, connect the USB
+device to your host and execute ``uuu``:

  .. code-block:: bash

@@ -61,7 +66,8 @@ to your host and execute ``uuu``:
  Flash the U-Boot Binary into the eMMC
  -

-Burn the ``u-boot-dtb.imx`` binary to the primary eMMC hardware boot area 
partition:
+Burn the ``u-boot-dtb.imx`` binary to the primary eMMC hardware boot area
+partition and boot:

  .. code-block:: bash

@@ -69,3 +75,10 @@ Burn the ``u-boot-dtb.imx`` binary 

Re: [PATCH v1 2/2] doc: board: toradex: verdin-am62: document update u-boot wrapper

2023-08-29 Thread Heinrich Schuchardt

On 8/29/23 00:01, Marcel Ziswiler wrote:

From: Marcel Ziswiler 

Now with the update U-Boot wrappers having been sorted out, document
their usage.

Signed-off-by: Marcel Ziswiler 

---

  doc/board/toradex/verdin-am62.rst | 14 ++
  1 file changed, 14 insertions(+)

diff --git a/doc/board/toradex/verdin-am62.rst 
b/doc/board/toradex/verdin-am62.rst
index ecc7e0777cb..e8d90273288 100644
--- a/doc/board/toradex/verdin-am62.rst
+++ b/doc/board/toradex/verdin-am62.rst
@@ -74,6 +74,20 @@ Flash to eMMC
  => fatload mmc 1 ${loadaddr} u-boot.img
  => mmc write ${loadaddr} 0x1400 0x2000

+As a convenience, instead of having to remember all those addresses and sizes,
+one may also use the update U-Boot wrappers:
+
+.. code-block:: bash
+
+> tftpboot ${loadaddr} tiboot3-am62x-gp-verdin.bin
+> run update_tiboot3


I wonder what the prompt '>' refers to. The rest of the document uses
'=>' as U-Boot prompt.

To make copying easier consider using

.. prompt:: bash =>

The html rendered page will contain a => before each line but you would
not need to type it here.

Best regards

Heinrich


+
+> tftpboot ${loadaddr} tispl.bin
+> run update_tispl
+
+> tftpboot ${loadaddr} u-boot.img
+> run update_uboot
+
  Boot
  





Re: [PATCH v1] include: configs: verdin-am62: drop unused sdram address

2023-08-29 Thread Mattijs Korpershoek
Hi Marcel,

Thank you for following up this quickly on this.

On lun., août 28, 2023 at 23:50, Marcel Ziswiler  wrote:

> From: Marcel Ziswiler 
>
> Drop unused macro. This was copied straight from the AM62x EVM but while
> meant for a second region of DDR this is not even needed for the AM62x
> EVM configurations and has meanwhile also been dropped there.
>
> Note that on the Verdin AM62, we do auto-detect the amount of SDRAM.
>
> While at it also update the comment noting that CFG_SYS_SDRAM_SIZE is
> the maximum which is only used for such auto-detection.
>
> Signed-off-by: Marcel Ziswiler 

Reviewed-by: Mattijs Korpershoek 

>
> ---
>
>  include/configs/verdin-am62.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/include/configs/verdin-am62.h b/include/configs/verdin-am62.h
> index 7990ea83102..e1a5f5ad44b 100644
> --- a/include/configs/verdin-am62.h
> +++ b/include/configs/verdin-am62.h
> @@ -13,8 +13,7 @@
>  
>  /* DDR Configuration */
>  #define CFG_SYS_SDRAM_BASE   0x8000
> -#define CFG_SYS_SDRAM_BASE1  0x88000
> -#define CFG_SYS_SDRAM_SIZE   SZ_2G /* Maximum supported size */
> +#define CFG_SYS_SDRAM_SIZE   SZ_2G /* Maximum supported size, auto-detection 
> is used */
>  
>  #define MEM_LAYOUT_ENV_SETTINGS \
>   "fdt_addr_r=0x9020\0" \
> -- 
> 2.36.1


[PATCH 1/1] Watchdog: Support WDIOF_CARDRESET on TI AM65x platform

2023-08-29 Thread huaqian . li
From: Li Hua Qian 

To have the WDIOF_CARDRESET support for the TI AM65x platform watchdog,
this patch reserves some memories, which indicate if the current boot due
to a watchdog reset.

Signed-off-by: Li Hua Qian 

---
Here is the Kernel Reviewed-by Submission:
  https://lore.kernel.org/all/20230718021007.1338761-1-huaqian...@siemens.com/

 arch/arm/dts/k3-am65-iot2050-common.dtsi | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/dts/k3-am65-iot2050-common.dtsi 
b/arch/arm/dts/k3-am65-iot2050-common.dtsi
index 65da226847..b6135b849f 100644
--- a/arch/arm/dts/k3-am65-iot2050-common.dtsi
+++ b/arch/arm/dts/k3-am65-iot2050-common.dtsi
@@ -64,6 +64,12 @@
alignment = <0x1000>;
no-map;
};
+
+   /* To reserve the power-on(PON) reason for watchdog reset */
+   wdt_reset_memory_region: wdt-memory@a220 {
+   reg = <0x00 0xa220 0x00 0x1000>;
+   no-map;
+   };
};
 
leds {
@@ -720,6 +726,11 @@
mboxes = <_cluster1 _mcu_r5fss0_core1>;
 };
 
+_rti1 {
+   memory-region = <_reset_memory_region>;
+
+};
+
 _mdio {
status = "disabled";
 };
-- 
2.34.1



Does u-boot support env for spi nand flash?

2023-08-29 Thread Tony He
Hi,

According to env/Kconfig file, seems like below env interfaces are
supported. In my understanding,
ENV_IS_IN_NAND is used by parallel nand flash while
ENV_IS_IN_SPI_FLASH is used by spi nor
flash. There is no direct support/interface for spi nand flash.
However, we can use ENV_IS_IN_UBI
for spi nand flash, which means we can put env into one volume of UBI.
Is this correct? If yes, any
reason why no direct support/interface for spi nand flash? Thanks!

   def_bool y if !ENV_IS_IN_EEPROM && !ENV_IS_IN_EXT4 && \
 !ENV_IS_IN_FAT && !ENV_IS_IN_FLASH && \
 !ENV_IS_IN_MMC && !ENV_IS_IN_NAND && \
 !ENV_IS_IN_NVRAM && !ENV_IS_IN_ONENAND && \
 !ENV_IS_IN_REMOTE && !ENV_IS_IN_SPI_FLASH && \
 !ENV_IS_IN_UBI

Tony


Re: [PATCH] clk: Dont return error when assigned-clocks is empty or missing

2023-08-29 Thread Michal Simek




On 8/28/23 17:50, Tom Rini wrote:

On Mon, Aug 28, 2023 at 12:25:30PM +0200, Michal Simek wrote:



On 8/25/23 16:39, Tom Rini wrote:

On Fri, Aug 25, 2023 at 09:15:09AM +0200, Michal Simek wrote:

Hi Tom,

On 7/11/23 11:51, Ashok Reddy Soma wrote:

There is a chance that assigned-clock-rates is given and assigned-clocks
could be empty. Dont return error in that case, because the probe of the
corresponding driver will not be called at all if this fails.
Better to continue to look for it and return 0.

Signed-off-by: Ashok Reddy Soma 
---

drivers/clk/clk-uclass.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index dc3e9d6a26..f186fcbcdb 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -329,7 +329,13 @@ static int clk_set_default_rates(struct udevice *dev,
dev_dbg(dev,
"could not get assigned clock %d (err = %d)\n",
index, ret);
-   continue;
+   /* Skip if it is empty */
+   if (ret == -ENOENT) {
+   ret = 0;
+   continue;
+   }
+
+   return ret;
}
/* This is clk provider device trying to program itself


What's your take on this one?  I didn't get reply from Sean.


I guess, what's the validated upstream dts where this is the case?



It was found by incorrect DT. It means I don't think there is any DT which
contains this issue.
But that being said we can extend current clock tests to cover this case.
Please look below.


Well, if the DT is invalid (and yes, we can't easily run the validation
suite in U-Boot today), I'd rather go with a debug we can optimize out
so that the next person with an invalid DT can more quickly find their
problem and we don't work-around it instead.  Or am I missing something?


There is already dev_dbg print above. It means when user enable DEBUG it gets 
information about it.


The thing is that it just sync behavior with the linux kernel.
You can look at
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/clk-conf.c?h=v6.5#n90

There is also one more example like this.
clk-test4 {
compatible = "sandbox,clk-test";
assigned-clock-rates = <654>, <321>;
assigned-clocks = <_sandbox 1>;
};

But if you prefer to fail in all these cases I am also fine with it.

Thanks,
Michal


[PATCH 1/1] doc: describe Kconfig fragments

2023-08-29 Thread Heinrich Schuchardt
In the 'Build with GCC' chapter mention Kconfig fragments and show how they
are applied.

Adjust the formatting in the configuration section to consistently use
italics for file names.

Signed-off-by: Heinrich Schuchardt 
---
 doc/build/gcc.rst | 35 ++-
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/doc/build/gcc.rst b/doc/build/gcc.rst
index a0650a51db..40fe207890 100644
--- a/doc/build/gcc.rst
+++ b/doc/build/gcc.rst
@@ -74,21 +74,46 @@ beforehand. Please, refer to the board specific 
documentation
 Configuration
 -
 
-Directory configs/ contains the template configuration files for the maintained
-boards following the naming scheme::
+Directory *configs/* contains the template configuration files for the
+maintained boards following the naming scheme::
 
 _defconfig
 
 These files have been stripped of default settings. So you cannot use them
 directly. Instead their name serves as a make target to generate the actual
-configuration file .config. For instance the configuration template for the
-Odroid C2 board is called odroid-c2_defconfig. The corresponding .config file
-is generated by
+configuration file *.config*. For instance the configuration template for the
+Odroid C2 board is called *odroid-c2_defconfig*. The corresponding *.config*
+file is generated by
 
 .. code-block:: bash
 
 make odroid-c2_defconfig
 
+For some of the boards configuration fragments exist in directory *configs/*
+which can be used to modify the generated *.config* file. These are named::
+
+.config
+
+They are applied to the current configuration file *.config* using the make
+command. E.g. the LG X3 T30 device family uses fragments for board specific
+configuration while providing a single defconfig file for the device family:
+
+.. code-block:: bash
+
+# Generate device family .config file
+make x3_t30_defconfig
+# Apply board specific configuration fragment
+make p895.config
+
+You can use a single command instead.
+
+.. code-block:: bash
+
+make x3_t30_defconfig p895.config
+
+Details about available configuration fragments are provided in the
+:doc:`board specific documentation<../board/index>`.
+
 You can adjust the configuration using
 
 .. code-block:: bash
-- 
2.40.1



Re: [PATCH v2] spl: watchdog: introduce SPL_HW_WATCHDOG

2023-08-29 Thread Oleksandr Suvorov
Hi Stefan,

On Mon, Aug 28, 2023 at 5:30 PM Stefan Roese  wrote:
>
> Hi Oleksandr,
>
> On 8/28/23 15:23, Oleksandr Suvorov wrote:
> > Hi Stefan,
> >
> > On Thu, Aug 24, 2023 at 2:24 PM Stefan Roese  wrote:
> >>
> >> On 8/23/23 14:00, Oleksandr Suvorov wrote:
> >>> Add SPL_HW_WATCHDOG Kconfig symbol which can be used to enable
> >>> non-WDT hardware watchdog in SPL.
> >>
> >> Hmmm, my hope / plan was to completely drop HW_WATCHDOG at some point.
> >> It's some old relict and if I don't miss something completely, it's more
> >> a "normal" watchdog that needs to get triggered very early in most
> >> cases (PowerPC etc).
> >
> > Oh, it's a good approach, thanks.
>
> Good that we agree. But...
>
> >> So adding some more *_HW_WATCHDOG stuff does not sound appealing to me.
> >> Could you perhaps investigate a bit, if your HW_WATCHDOG use case can
> >> be converted to the "normal" watchdog support instead?
> >
> > We just need to be able to exclude a hardware watchdog from SPL to
> > save some memory.
>
> Did you already enable LTO so decrease the image size? Just checking.

We use SPL->OP-TEE->U-boot booting scheme instead of standard
SPL->U-boot->OP-TEE, so SPL in our distro needs more preparation
than usual :) So LTO helps but doesn't solve a size issue for some SoCs.

> > So I resign this patch and we'll wait for your work and then revise
> > what we have again.
>
> ...but: Actually I don't have any plans to do this rework myself. At
> least not short-term. My hope was that someone for the platforms using
> this HW_WATCHDOG stuff would jump in here. ;)

Got it :) I'm looking into that stuff in a ~month.

> Thanks,
> Stefan
>
> >> Thanks,
> >> Stefan
> >
> > Cheers,
> > Oleksandr
> >
> >>
> >>> Co-developed-by: Igor Opaniuk 
> >>> Signed-off-by: Igor Opaniuk 
> >>> Signed-off-by: Oleksandr Suvorov 
> >>> ---
> >>>
> >>> Changes in v2:
> >>> - remove mistakenly included unwanted changes
> >>>
> >>>common/spl/Kconfig| 1 -
> >>>drivers/Makefile  | 1 +
> >>>drivers/sysreset/Kconfig  | 6 ++
> >>>drivers/sysreset/Makefile | 2 +-
> >>>drivers/watchdog/Kconfig  | 4 
> >>>5 files changed, 12 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> >>> index c5dd476db58..07d9dac29bd 100644
> >>> --- a/common/spl/Kconfig
> >>> +++ b/common/spl/Kconfig
> >>> @@ -1351,7 +1351,6 @@ config SPL_THERMAL
> >>>
> >>>config SPL_WATCHDOG
> >>>bool "Support watchdog drivers"
> >>> - imply SPL_WDT if !HW_WATCHDOG
> >>>help
> >>>  Enable support for watchdog drivers in SPL. A watchdog is
> >>>  typically a hardware peripheral which can reset the system when 
> >>> it
> >>> diff --git a/drivers/Makefile b/drivers/Makefile
> >>> index efc2a4afb24..2eb8ec0a894 100644
> >>> --- a/drivers/Makefile
> >>> +++ b/drivers/Makefile
> >>> @@ -62,6 +62,7 @@ obj-$(CONFIG_SPL_USB_GADGET) += usb/gadget/
> >>>obj-$(CONFIG_SPL_USB_GADGET) += usb/common/
> >>>obj-$(CONFIG_SPL_USB_GADGET) += usb/gadget/udc/
> >>>obj-$(CONFIG_SPL_WATCHDOG) += watchdog/
> >>> +obj-$(CONFIG_SPL_HW_WATCHDOG) += watchdog/
> >>>obj-$(CONFIG_SPL_USB_HOST) += usb/host/
> >>>obj-$(CONFIG_SPL_SATA) += ata/ scsi/
> >>>obj-$(CONFIG_SPL_LEGACY_BLOCK) += block/
> >>> diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
> >>> index bdbe2a95364..0d21673e402 100644
> >>> --- a/drivers/sysreset/Kconfig
> >>> +++ b/drivers/sysreset/Kconfig
> >>> @@ -157,6 +157,12 @@ config SYSRESET_WATCHDOG
> >>>help
> >>>  Reboot support for generic watchdog reset.
> >>>
> >>> +config SPL_SYSRESET_WATCHDOG
> >>> + bool "Enable support for watchdog reboot driver in SPL mode"
> >>> + select SPL_WDT
> >>> + help
> >>> +   Reboot support for generic watchdog reset in SPL mode.
> >>> +
> >>>config SYSRESET_WATCHDOG_AUTO
> >>>bool "Automatically register first watchdog with sysreset"
> >>>depends on SYSRESET_WATCHDOG
> >>> diff --git a/drivers/sysreset/Makefile b/drivers/sysreset/Makefile
> >>> index 40c876764af..e5a7fc07a81 100644
> >>> --- a/drivers/sysreset/Makefile
> >>> +++ b/drivers/sysreset/Makefile
> >>> @@ -18,7 +18,7 @@ obj-$(CONFIG_SYSRESET_SOCFPGA) += sysreset_socfpga.o
> >>>obj-$(CONFIG_SYSRESET_SOCFPGA_SOC64) += sysreset_socfpga_soc64.o
> >>>obj-$(CONFIG_SYSRESET_TI_SCI) += sysreset-ti-sci.o
> >>>obj-$(CONFIG_SYSRESET_SYSCON) += sysreset_syscon.o
> >>> -obj-$(CONFIG_SYSRESET_WATCHDOG) += sysreset_watchdog.o
> >>> +obj-$(CONFIG_$(SPL_)SYSRESET_WATCHDOG) += sysreset_watchdog.o
> >>>obj-$(CONFIG_SYSRESET_RESETCTL) += sysreset_resetctl.o
> >>>obj-$(CONFIG_$(SPL_TPL_)SYSRESET_AT91) += sysreset_at91.o
> >>>obj-$(CONFIG_$(SPL_TPL_)SYSRESET_X86) += sysreset_x86.o
> >>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> >>> index 07fc4940e91..d696a04fc18 100644
> >>> --- a/drivers/watchdog/Kconfig
> >>> +++ b/drivers/watchdog/Kconfig
> >>> @@ -39,9 

Re: [PATCH v5 00/13] Add video damage tracking

2023-08-29 Thread Alexander Graf

Hi Simon,

On 29.08.23 00:08, Simon Glass wrote:

Hi Alex,

On Mon, 28 Aug 2023 at 14:24, Alexander Graf  wrote:


On 28.08.23 19:54, Simon Glass wrote:

Hi Alex,

On Wed, 23 Aug 2023 at 02:56, Alexander Graf  wrote:

Hey Simon,

On 22.08.23 20:56, Simon Glass wrote:

Hi Alex,

On Tue, 22 Aug 2023 at 01:47, Alexander Graf  wrote:

On 22.08.23 01:03, Simon Glass wrote:

Hi Alex,

On Mon, 21 Aug 2023 at 16:40, Alexander Graf  wrote:

On 22.08.23 00:10, Simon Glass wrote:

Hi Alex,

On Mon, 21 Aug 2023 at 14:20, Alexander Graf  wrote:

On 21.08.23 21:57, Simon Glass wrote:

Hi Alex,

On Mon, 21 Aug 2023 at 13:33, Alexander Graf  wrote:

On 21.08.23 21:11, Simon Glass wrote:

Hi Alper,

On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak  wrote:

This is a rebase of Alexander Graf's video damage tracking series, with
some tests and other changes. The original cover letter is as follows:


This patch set speeds up graphics output on ARM by a factor of 60x.

On most ARM SBCs, we keep the frame buffer in DRAM and map it as cached,
but need it accessible by the display controller which reads directly
from a later point of consistency. Hence, we flush the frame buffer to
DRAM on every change. The full frame buffer.

It should not, see below.


Unfortunately, with the advent of 4k displays, we are seeing frame buffers
that can take a while to flush out. This was reported by Da Xue with grub,
which happily print 1000s of spaces on the screen to draw a menu. Every
printed space triggers a cache flush.

That is a bug somewhere in EFI.

Unfortunately not :). You may call it a bug in grub: It literally prints
over space characters for every character in its menu that it wants
cleared. On every text screen draw.

This wouldn't be a big issue if we only flush the reactangle that gets
modified. But without this patch set, we're flushing the full DRAM
buffer on every u-boot text console character write, which means for
every character (as that's the only API UEFI has).

As a nice side effect, we speed up the normal U-Boot text console as
well with this patch set, because even "normal" text prints that write
for example a single line of text on the screen today flush the full
frame buffer to DRAM.

No, I mean that it is a bug that U-Boot (apparently) flushes the cache
after every character. It doesn't do that for normal character output
and I don't think it makes sense to do it for EFI either.

I see. Let's trace the calls:

efi_cout_output_string()
-> fputs()
-> vidconsole_puts()
-> video_sync()
-> flush_dcache_range()

Unfortunately grub abstracts character backends down to the "print
character" level, so it calls UEFI's sopisticated "output_string"
callback with single characters at a time, which means we do a full
dcache flush for every character that we print:

https://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/term/efi/console.c#n165



This patch set implements the easiest mitigation against this problem:
Damage tracking. We remember the lowest common denominator region that was
touched since the last video_sync() call and only flush that. The most
typical writer to the frame buffer is the video console, which always
writes rectangles of characters on the screen and syncs afterwards.

With this patch set applied, we reduce drawing a large grub menu (with
serial console attached for size information) on an RK3399-ROC system
at 1440p from 55 seconds to less than 1 second.

Version 2 also implements VIDEO_COPY using this mechanism, reducing its
overhead compared to before as well. So even x86 systems should be faster
with this now :).


Alternatives considered:

 1) Lazy sync - Sandbox does this. It only calls video_sync(true) ever
so often. We are missing timers to do this generically.

 2) Double buffering - We could try to identify whether anything changed
at all and only draw to the FB if it did. That would require
maintaining a second buffer that we need to scan.

 3) Text buffer - Maintain a buffer of all text printed on the screen 
with
respective location. Don't write if the old and new character are
identical. This would limit applicability to text only and is an
optimization on top of this patch set.

 4) Hash screen lines - Create a hash (sha256?) over every line when it
changes. Only flush when it does. I'm not sure if this would waste
more time, memory and cache than the current approach. It would make
full screen updates much more expensive.

5) Fix the bug mentioned above?


Changes in v5:
- Add patch "video: test: Split copy frame buffer check into a function"
- Add patch "video: test: Support checking copy frame buffer contents"
- Add patch "video: test: Test partial updates of hardware frame buffer"
- Use xstart, ystart, xend, yend as names for damage region
- Document damage struct and fields in struct video_priv comment
- Return void from video_damage()
- Fix 

Re: [PATCH v5 00/13] Add video damage tracking

2023-08-29 Thread Alexander Graf



On 28.08.23 23:54, Heinrich Schuchardt wrote:

On 8/28/23 22:24, Alexander Graf wrote:


On 28.08.23 19:54, Simon Glass wrote:

Hi Alex,

On Wed, 23 Aug 2023 at 02:56, Alexander Graf  wrote:

Hey Simon,

On 22.08.23 20:56, Simon Glass wrote:

Hi Alex,

On Tue, 22 Aug 2023 at 01:47, Alexander Graf  wrote:

On 22.08.23 01:03, Simon Glass wrote:

Hi Alex,

On Mon, 21 Aug 2023 at 16:40, Alexander Graf  
wrote:

On 22.08.23 00:10, Simon Glass wrote:

Hi Alex,

On Mon, 21 Aug 2023 at 14:20, Alexander Graf 
wrote:

On 21.08.23 21:57, Simon Glass wrote:

Hi Alex,

On Mon, 21 Aug 2023 at 13:33, Alexander Graf 
wrote:

On 21.08.23 21:11, Simon Glass wrote:

Hi Alper,

On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak
 wrote:

This is a rebase of Alexander Graf's video damage tracking
series, with
some tests and other changes. The original cover letter is
as follows:


This patch set speeds up graphics output on ARM by a
factor of 60x.

On most ARM SBCs, we keep the frame buffer in DRAM and map
it as cached,
but need it accessible by the display controller which
reads directly
from a later point of consistency. Hence, we flush the
frame buffer to
DRAM on every change. The full frame buffer.

It should not, see below.


Unfortunately, with the advent of 4k displays, we are
seeing frame buffers
that can take a while to flush out. This was reported by
Da Xue with grub,
which happily print 1000s of spaces on the screen to draw
a menu. Every
printed space triggers a cache flush.

That is a bug somewhere in EFI.

Unfortunately not :). You may call it a bug in grub: It
literally prints
over space characters for every character in its menu that it
wants
cleared. On every text screen draw.

This wouldn't be a big issue if we only flush the reactangle
that gets
modified. But without this patch set, we're flushing the full
DRAM
buffer on every u-boot text console character write, which
means for
every character (as that's the only API UEFI has).

As a nice side effect, we speed up the normal U-Boot text
console as
well with this patch set, because even "normal" text prints
that write
for example a single line of text on the screen today flush
the full
frame buffer to DRAM.

No, I mean that it is a bug that U-Boot (apparently) flushes
the cache
after every character. It doesn't do that for normal character
output
and I don't think it makes sense to do it for EFI either.

I see. Let's trace the calls:

efi_cout_output_string()
-> fputs()
-> vidconsole_puts()
-> video_sync()
-> flush_dcache_range()

Unfortunately grub abstracts character backends down to the 
"print
character" level, so it calls UEFI's sopisticated 
"output_string"

callback with single characters at a time, which means we do a
full
dcache flush for every character that we print:

https://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/term/efi/console.c#n165 





This patch set implements the easiest mitigation against
this problem:
Damage tracking. We remember the lowest common denominator
region that was
touched since the last video_sync() call and only flush
that. The most
typical writer to the frame buffer is the video console,
which always
writes rectangles of characters on the screen and syncs
afterwards.

With this patch set applied, we reduce drawing a large
grub menu (with
serial console attached for size information) on an
RK3399-ROC system
at 1440p from 55 seconds to less than 1 second.

Version 2 also implements VIDEO_COPY using this mechanism,
reducing its
overhead compared to before as well. So even x86 systems
should be faster
with this now :).


Alternatives considered:

    1) Lazy sync - Sandbox does this. It only calls
video_sync(true) ever
   so often. We are missing timers to do this
generically.

    2) Double buffering - We could try to identify
whether anything changed
   at all and only draw to the FB if it did. That
would require
   maintaining a second buffer that we need to 
scan.


    3) Text buffer - Maintain a buffer of all text
printed on the screen with
   respective location. Don't write if the old and
new character are
   identical. This would limit applicability to
text only and is an
   optimization on top of this patch set.

    4) Hash screen lines - Create a hash (sha256?)
over every line when it
   changes. Only flush when it does. I'm not sure
if this would waste
   more time, memory and cache than the current
approach. It would make
   full screen updates much more expensive.

5) Fix the bug mentioned above?


Changes in v5:
- Add patch "video: test: Split copy frame buffer check
into a function"
- Add patch "video: test: Support checking copy frame
buffer contents"
- Add patch "video: test: Test partial updates of hardware
frame buffer"
- Use xstart, ystart, xend, yend as names for damage region
- Document damage struct and fields in struct video_priv
comment
- Return void from video_damage()
- Fix undeclared priv error in