[PATCH v4 7/7] arm: remove redundant section alignments

2024-03-14 Thread Ilias Apalodimas
Previous patches cleaning up linker symbols, also merged any explicit
. = ALIGN(x); into section definitions -- e.g
.bss ALIGN(x) : instead of

. = ALIGN(x);
. bss : {...}

However, if the output address is not specified then one will be chosen
for the section. This address will be adjusted to fit the alignment
requirement of the output section following the strictest alignment of
any input section contained within the output section. So let's get rid
of the redundant ALIGN directives when they are not needed.

While at add comments for the alignment of __bss_start/end since our
C runtime setup assembly assumes that __bss_start - __bss_end will be
a multiple of 4/8 for armv7 and armv8 respectively.

It's worth noting that the alignment is preserved on .rel.dyn for
mach-zynq which was explicitly aligning that section on an 8b
boundary instead of 4b one.

Reviewed-by: Richard Henderson 
Signed-off-by: Ilias Apalodimas 
---
 arch/arm/cpu/armv8/u-boot.lds | 9 ++---
 arch/arm/cpu/u-boot.lds   | 8 ++--
 arch/arm/mach-zynq/u-boot.lds | 4 ++--
 3 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds
index 147a6e8028d5..857f44412e07 100644
--- a/arch/arm/cpu/armv8/u-boot.lds
+++ b/arch/arm/cpu/armv8/u-boot.lds
@@ -115,7 +115,7 @@ SECTIONS
KEEP(*(SORT(__u_boot_list*)));
}
 
-   .efi_runtime_rel ALIGN(8) : {
+   .efi_runtime_rel : {
 __efi_runtime_rel_start = .;
*(.rel*.efi_runtime)
*(.rel*.efi_runtime.*)
@@ -125,7 +125,7 @@ SECTIONS
. = ALIGN(8);
__image_copy_end = .;
 
-   .rela.dyn ALIGN(8) : {
+   .rela.dyn : {
__rel_dyn_start = .;
*(.rela*)
__rel_dyn_end = .;
@@ -133,7 +133,10 @@ SECTIONS
 
_end = .;
 
-   .bss ALIGN(8): {
+   /*
+* arch/arm/lib/crt0_64.S assumes __bss_start - __bss_end % 8 == 0
+*/
+   .bss ALIGN(8) : {
__bss_start = .;
*(.bss*)
. = ALIGN(8);
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index 798858e3ed6e..707b19795f08 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -43,7 +43,7 @@ SECTIONS
}
 
/* This needs to come before *(.text*) */
-   .efi_runtime ALIGN(4) : {
+   .efi_runtime : {
__efi_runtime_start = .;
*(.text.efi_runtime*)
*(.rodata.efi_runtime*)
@@ -146,7 +146,7 @@ SECTIONS
KEEP(*(SORT(__u_boot_list*)));
}
 
-   .efi_runtime_rel ALIGN(4) : {
+   .efi_runtime_rel : {
__efi_runtime_rel_start = .;
*(.rel*.efi_runtime)
*(.rel*.efi_runtime.*)
@@ -156,6 +156,10 @@ SECTIONS
. = ALIGN(4);
__image_copy_end = .;
 
+   /*
+* if CONFIG_USE_ARCH_MEMSET is not selected __bss_end - __bss_start
+* needs to be a multiple of 4 and we overlay .bss with .rel.dyn
+*/
.rel.dyn ALIGN(4) : {
__rel_dyn_start = .;
*(.rel*)
diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds
index f6c99a8ce218..3e0c96c50556 100644
--- a/arch/arm/mach-zynq/u-boot.lds
+++ b/arch/arm/mach-zynq/u-boot.lds
@@ -22,7 +22,7 @@ SECTIONS
}
 
/* This needs to come before *(.text*) */
-   .efi_runtime ALIGN(4) : {
+   .efi_runtime : {
__efi_runtime_start = .;
*(.text.efi_runtime*)
*(.rodata.efi_runtime*)
@@ -52,7 +52,7 @@ SECTIONS
KEEP(*(SORT(__u_boot_list*)));
}
 
-   .efi_runtime_rel ALIGN(4) : {
+   .efi_runtime_rel : {
__efi_runtime_rel_start = .;
*(.rel*.efi_runtime)
*(.rel*.efi_runtime.*)
-- 
2.37.2



[PATCH v4 6/7] arm: move image_copy_start/end to linker symbols

2024-03-14 Thread Ilias Apalodimas
image_copy_start/end are defined as c variables in order to force the compiler
emit relative references. However, defining those within a section definition
will do the same thing since [0].

So let's remove the special sections from the linker scripts, the
variable definitions from sections.c and define them as a symbols within
a section.

[0] binutils commit 6b3b0ab89663 ("Make linker assigned symbol dynamic only for 
shared object")

Suggested-by: Sam Edwards 
Reviewed-by: Richard Henderson 
Tested-by: Sam Edwards  # Binary output identical
Signed-off-by: Ilias Apalodimas 
---
 arch/arm/cpu/armv8/u-boot-spl.lds   | 8 +++-
 arch/arm/cpu/armv8/u-boot.lds   | 8 ++--
 arch/arm/cpu/u-boot-spl.lds | 2 +-
 arch/arm/cpu/u-boot.lds | 8 ++--
 arch/arm/lib/sections.c | 2 --
 arch/arm/mach-aspeed/ast2600/u-boot-spl.lds | 2 +-
 arch/arm/mach-rockchip/u-boot-tpl-v8.lds| 8 +++-
 arch/arm/mach-zynq/u-boot-spl.lds   | 2 +-
 arch/arm/mach-zynq/u-boot.lds   | 7 ++-
 9 files changed, 15 insertions(+), 32 deletions(-)

diff --git a/arch/arm/cpu/armv8/u-boot-spl.lds 
b/arch/arm/cpu/armv8/u-boot-spl.lds
index 8998c4985eac..ef8af67e11c3 100644
--- a/arch/arm/cpu/armv8/u-boot-spl.lds
+++ b/arch/arm/cpu/armv8/u-boot-spl.lds
@@ -21,9 +21,9 @@ OUTPUT_ARCH(aarch64)
 ENTRY(_start)
 SECTIONS
 {
+   __image_copy_start = ADDR(.text);
.text : {
. = ALIGN(8);
-   __image_copy_start = .;
CPUDIR/start.o (.text*)
*(.text*)
} >.sram
@@ -51,10 +51,8 @@ SECTIONS
KEEP(*(SORT(__u_boot_list*)));
} >.sram
 
-   .image_copy_end : {
-   . = ALIGN(8);
-   *(.__image_copy_end)
-   } >.sram
+   . = ALIGN(8);
+   __image_copy_end = .;
 
.end : {
. = ALIGN(8);
diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds
index 5ba54dcedf24..147a6e8028d5 100644
--- a/arch/arm/cpu/armv8/u-boot.lds
+++ b/arch/arm/cpu/armv8/u-boot.lds
@@ -21,9 +21,9 @@ SECTIONS
. = 0x;
 
. = ALIGN(8);
+   __image_copy_start = ADDR(.text);
.text :
{
-   *(.__image_copy_start)
CPUDIR/start.o (.text*)
}
 
@@ -123,11 +123,7 @@ SECTIONS
}
 
. = ALIGN(8);
-
-   .image_copy_end :
-   {
-   *(.__image_copy_end)
-   }
+   __image_copy_end = .;
 
.rela.dyn ALIGN(8) : {
__rel_dyn_start = .;
diff --git a/arch/arm/cpu/u-boot-spl.lds b/arch/arm/cpu/u-boot-spl.lds
index fb2189d50dea..9ed62395a9c5 100644
--- a/arch/arm/cpu/u-boot-spl.lds
+++ b/arch/arm/cpu/u-boot-spl.lds
@@ -14,9 +14,9 @@ SECTIONS
. = 0x;
 
. = ALIGN(4);
+   __image_copy_start = ADDR(.text);
.text :
{
-   __image_copy_start = .;
*(.vectors)
CPUDIR/start.o (.text*)
*(.text*)
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index 6813d8aeb838..798858e3ed6e 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -35,9 +35,9 @@ SECTIONS
. = 0x;
 
. = ALIGN(4);
+   __image_copy_start = ADDR(.text);
.text :
{
-   *(.__image_copy_start)
*(.vectors)
CPUDIR/start.o (.text*)
}
@@ -154,11 +154,7 @@ SECTIONS
}
 
. = ALIGN(4);
-
-   .image_copy_end :
-   {
-   *(.__image_copy_end)
-   }
+   __image_copy_end = .;
 
.rel.dyn ALIGN(4) : {
__rel_dyn_start = .;
diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c
index a4d4202e99f5..db5463b2bbbc 100644
--- a/arch/arm/lib/sections.c
+++ b/arch/arm/lib/sections.c
@@ -19,8 +19,6 @@
  * aliasing warnings.
  */
 
-char __image_copy_start[0] __section(".__image_copy_start");
-char __image_copy_end[0] __section(".__image_copy_end");
 char __secure_start[0] __section(".__secure_start");
 char __secure_end[0] __section(".__secure_end");
 char __secure_stack_start[0] __section(".__secure_stack_start");
diff --git a/arch/arm/mach-aspeed/ast2600/u-boot-spl.lds 
b/arch/arm/mach-aspeed/ast2600/u-boot-spl.lds
index 37f0ccd92201..ada6570d9712 100644
--- a/arch/arm/mach-aspeed/ast2600/u-boot-spl.lds
+++ b/arch/arm/mach-aspeed/ast2600/u-boot-spl.lds
@@ -22,9 +22,9 @@ SECTIONS
. = 0x;
 
. = ALIGN(4);
+   __image_copy_start = ADDR(.text);
.text :
{
-   __image_copy_start = .;
*(.vectors)
CPUDIR/start.o (.text*)
*(.text*)
diff --git a/arch/arm/mach-rockchip/u-boot-tpl-v8.lds 
b/arch/arm/mach-rockchip/u-boot-tpl-v8.lds
index 712c485d4d0b..ad32654085b3 100644
--- a/arch/arm/mach-rockchip/u-boot-tpl-v8.lds
+++ b/arch/arm/mach-rockchip/u-boot-tpl-v8.lds
@@ -22,9 +22,9 @@ SE

[PATCH v4 5/7] arm: fix __efi_runtime_start/end definitions

2024-03-14 Thread Ilias Apalodimas
__efi_runtime_start/end are defined as c variables for arm7 only in
order to force the compiler emit relative references. However, defining
those within a section definition will do the same thing since [0].
On top of that the v8 linker scripts define it as a symbol.

So let's remove the special sections from the linker scripts, the
variable definitions from sections.c and define them as a symbols within
the correct section.

[0] binutils commit 6b3b0ab89663 ("Make linker assigned symbol dynamic only for 
shared object")

Suggested-by: Sam Edwards 
Reviewed-by: Sam Edwards 
Reviewed-by: Richard Henderson 
Tested-by: Sam Edwards  # Binary output identical
Signed-off-by: Ilias Apalodimas 
---
 arch/arm/cpu/u-boot.lds| 12 +++-
 arch/arm/lib/sections.c|  2 --
 arch/arm/mach-zynq/u-boot.lds  | 12 +++-
 include/asm-generic/sections.h |  1 +
 4 files changed, 7 insertions(+), 20 deletions(-)

diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index 0682d34207fa..6813d8aeb838 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -43,18 +43,12 @@ SECTIONS
}
 
/* This needs to come before *(.text*) */
-   .__efi_runtime_start : {
-   *(.__efi_runtime_start)
-   }
-
-   .efi_runtime : {
+   .efi_runtime ALIGN(4) : {
+   __efi_runtime_start = .;
*(.text.efi_runtime*)
*(.rodata.efi_runtime*)
*(.data.efi_runtime*)
-   }
-
-   .__efi_runtime_stop : {
-   *(.__efi_runtime_stop)
+   __efi_runtime_stop = .;
}
 
.text_rest :
diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c
index 1ee3dd3667ba..a4d4202e99f5 100644
--- a/arch/arm/lib/sections.c
+++ b/arch/arm/lib/sections.c
@@ -25,6 +25,4 @@ char __secure_start[0] __section(".__secure_start");
 char __secure_end[0] __section(".__secure_end");
 char __secure_stack_start[0] __section(".__secure_stack_start");
 char __secure_stack_end[0] __section(".__secure_stack_end");
-char __efi_runtime_start[0] __section(".__efi_runtime_start");
-char __efi_runtime_stop[0] __section(".__efi_runtime_stop");
 char _end[0] __section(".__end");
diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds
index 3b1f0d349356..9eac7de0dcbd 100644
--- a/arch/arm/mach-zynq/u-boot.lds
+++ b/arch/arm/mach-zynq/u-boot.lds
@@ -22,18 +22,12 @@ SECTIONS
}
 
/* This needs to come before *(.text*) */
-   .__efi_runtime_start : {
-   *(.__efi_runtime_start)
-   }
-
-   .efi_runtime : {
+   .efi_runtime ALIGN(4) : {
+   __efi_runtime_start = .;
*(.text.efi_runtime*)
*(.rodata.efi_runtime*)
*(.data.efi_runtime*)
-   }
-
-   .__efi_runtime_stop : {
-   *(.__efi_runtime_stop)
+   __efi_runtime_stop = .;
}
 
.text_rest :
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index 60949200dd93..b6bca53db10d 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -35,6 +35,7 @@ extern char __priv_data_start[], __priv_data_end[];
 extern char __ctors_start[], __ctors_end[];
 
 extern char __efi_runtime_rel_start[], __efi_runtime_rel_stop[];
+extern char __efi_runtime_start[], __efi_runtime_stop[];
 
 /* function descriptor handling (if any).  Override
  * in asm/sections.h */
-- 
2.37.2



[PATCH v4 4/7] arm: clean up v7 and v8 linker scripts for __rel_dyn_start/end

2024-03-14 Thread Ilias Apalodimas
commit 47bd65ef057f ("arm: make __rel_dyn_{start, end} compiler-generated")
were moving the __rel_dyn_start/end on c generated variables that were
injected in their own sections. The reason was that we needed relative
relocations for position independent code and linker bugs back then
prevented us from doing so [0].

However, the linker documentation pages states that symbols that are
defined within a section definition will create a relocatable
type with the value being a fixed offset from the base of a section [1].

[0] binutils commit 6b3b0ab89663 ("Make linker assigned symbol dynamic only for 
shared object")
[1] https://sourceware.org/binutils/docs/ld/Expression-Section.html

Suggested-by: Sam Edwards 
Reviewed-by: Sam Edwards 
Reviewed-by: Richard Henderson 
Tested-by: Sam Edwards  # Binary output identical
Signed-off-by: Ilias Apalodimas 
---
 arch/arm/cpu/armv8/u-boot.lds | 16 +++-
 arch/arm/cpu/u-boot.lds   | 14 +++---
 arch/arm/lib/sections.c   |  2 --
 arch/arm/mach-zynq/u-boot.lds | 14 +++---
 4 files changed, 9 insertions(+), 37 deletions(-)

diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds
index 8561e1b3142e..5ba54dcedf24 100644
--- a/arch/arm/cpu/armv8/u-boot.lds
+++ b/arch/arm/cpu/armv8/u-boot.lds
@@ -129,20 +129,10 @@ SECTIONS
*(.__image_copy_end)
}
 
-   . = ALIGN(8);
-
-   .rel_dyn_start :
-   {
-   *(.__rel_dyn_start)
-   }
-
-   .rela.dyn : {
+   .rela.dyn ALIGN(8) : {
+   __rel_dyn_start = .;
*(.rela*)
-   }
-
-   .rel_dyn_end :
-   {
-   *(.__rel_dyn_end)
+   __rel_dyn_end = .;
}
 
_end = .;
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index f19f2812ee91..0682d34207fa 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -166,18 +166,10 @@ SECTIONS
*(.__image_copy_end)
}
 
-   .rel_dyn_start :
-   {
-   *(.__rel_dyn_start)
-   }
-
-   .rel.dyn : {
+   .rel.dyn ALIGN(4) : {
+   __rel_dyn_start = .;
*(.rel*)
-   }
-
-   .rel_dyn_end :
-   {
-   *(.__rel_dyn_end)
+   __rel_dyn_end = .;
}
 
.end :
diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c
index ddfde52163fc..1ee3dd3667ba 100644
--- a/arch/arm/lib/sections.c
+++ b/arch/arm/lib/sections.c
@@ -21,8 +21,6 @@
 
 char __image_copy_start[0] __section(".__image_copy_start");
 char __image_copy_end[0] __section(".__image_copy_end");
-char __rel_dyn_start[0] __section(".__rel_dyn_start");
-char __rel_dyn_end[0] __section(".__rel_dyn_end");
 char __secure_start[0] __section(".__secure_start");
 char __secure_end[0] __section(".__secure_end");
 char __secure_stack_start[0] __section(".__secure_stack_start");
diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds
index bb0e0ceb32ec..3b1f0d349356 100644
--- a/arch/arm/mach-zynq/u-boot.lds
+++ b/arch/arm/mach-zynq/u-boot.lds
@@ -71,18 +71,10 @@ SECTIONS
*(.__image_copy_end)
}
 
-   .rel_dyn_start :
-   {
-   *(.__rel_dyn_start)
-   }
-
-   .rel.dyn : {
+   .rel.dyn ALIGN(8) : {
+   __rel_dyn_start = .;
*(.rel*)
-   }
-
-   .rel_dyn_end :
-   {
-   *(.__rel_dyn_end)
+   __rel_dyn_end = .;
}
 
.end :
-- 
2.37.2



[PATCH v4 3/7] arm: fix __efi_runtime_rel_start/end definitions

2024-03-14 Thread Ilias Apalodimas
__efi_runtime_rel_start/end are defined as c variables for arm7 only in
order to force the compiler emit relative references. However, defining
those within a section definition will do the same thing since [0].
On top of that the v8 linker scripts define it as a symbol.

So let's remove the special sections from the linker scripts, the
variable definitions from sections.c and define them as a symbols within
the correct section.

[0] binutils commit 6b3b0ab89663 ("Make linker assigned symbol dynamic only for 
shared object")

Suggested-by: Sam Edwards 
Reviewed-by: Sam Edwards 
Tested-by: Sam Edwards  # Binary output identical
Reviewed-by: Richard Henderson 
Signed-off-by: Ilias Apalodimas 
---
 arch/arm/cpu/armv8/u-boot.lds  |  4 +---
 arch/arm/cpu/u-boot.lds| 16 +++-
 arch/arm/lib/sections.c|  2 --
 arch/arm/mach-zynq/u-boot.lds  | 16 +++-
 include/asm-generic/sections.h |  2 ++
 lib/efi_loader/efi_runtime.c   |  1 +
 6 files changed, 10 insertions(+), 31 deletions(-)

diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds
index 9640cc7a04b8..8561e1b3142e 100644
--- a/arch/arm/cpu/armv8/u-boot.lds
+++ b/arch/arm/cpu/armv8/u-boot.lds
@@ -115,9 +115,7 @@ SECTIONS
KEEP(*(SORT(__u_boot_list*)));
}
 
-   . = ALIGN(8);
-
-   .efi_runtime_rel : {
+   .efi_runtime_rel ALIGN(8) : {
 __efi_runtime_rel_start = .;
*(.rel*.efi_runtime)
*(.rel*.efi_runtime.*)
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index 0dfe5f633b16..f19f2812ee91 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -152,21 +152,11 @@ SECTIONS
KEEP(*(SORT(__u_boot_list*)));
}
 
-   . = ALIGN(4);
-
-   .efi_runtime_rel_start :
-   {
-   *(.__efi_runtime_rel_start)
-   }
-
-   .efi_runtime_rel : {
+   .efi_runtime_rel ALIGN(4) : {
+   __efi_runtime_rel_start = .;
*(.rel*.efi_runtime)
*(.rel*.efi_runtime.*)
-   }
-
-   .efi_runtime_rel_stop :
-   {
-   *(.__efi_runtime_rel_stop)
+   __efi_runtime_rel_stop = .;
}
 
. = ALIGN(4);
diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c
index 8e8bd5797e16..ddfde52163fc 100644
--- a/arch/arm/lib/sections.c
+++ b/arch/arm/lib/sections.c
@@ -29,6 +29,4 @@ char __secure_stack_start[0] 
__section(".__secure_stack_start");
 char __secure_stack_end[0] __section(".__secure_stack_end");
 char __efi_runtime_start[0] __section(".__efi_runtime_start");
 char __efi_runtime_stop[0] __section(".__efi_runtime_stop");
-char __efi_runtime_rel_start[0] __section(".__efi_runtime_rel_start");
-char __efi_runtime_rel_stop[0] __section(".__efi_runtime_rel_stop");
 char _end[0] __section(".__end");
diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds
index 3c5008b57392..bb0e0ceb32ec 100644
--- a/arch/arm/mach-zynq/u-boot.lds
+++ b/arch/arm/mach-zynq/u-boot.lds
@@ -58,21 +58,11 @@ SECTIONS
KEEP(*(SORT(__u_boot_list*)));
}
 
-   . = ALIGN(4);
-
-   .efi_runtime_rel_start :
-   {
-   *(.__efi_runtime_rel_start)
-   }
-
-   .efi_runtime_rel : {
+   .efi_runtime_rel ALIGN(4) : {
+   __efi_runtime_rel_start = .;
*(.rel*.efi_runtime)
*(.rel*.efi_runtime.*)
-   }
-
-   .efi_runtime_rel_stop :
-   {
-   *(.__efi_runtime_rel_stop)
+   __efi_runtime_rel_stop = .;
}
 
. = ALIGN(8);
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index 1e1657a01673..60949200dd93 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -34,6 +34,8 @@ extern char __priv_data_start[], __priv_data_end[];
 /* Start and end of .ctors section - used for constructor calls. */
 extern char __ctors_start[], __ctors_end[];
 
+extern char __efi_runtime_rel_start[], __efi_runtime_rel_stop[];
+
 /* function descriptor handling (if any).  Override
  * in asm/sections.h */
 #ifndef dereference_function_descriptor
diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index 18da6892e796..9185f1894c47 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* For manual relocation support */
 DECLARE_GLOBAL_DATA_PTR;
-- 
2.37.2



[PATCH v4 2/7] arm: clean up v7 and v8 linker scripts for bss_start/end

2024-03-14 Thread Ilias Apalodimas
commit 3ebd1cbc49f0 ("arm: make __bss_start and __bss_end__ compiler-generated")
and
commit f84a7b8f54db ("ARM: Fix __bss_start and __bss_end in linker scripts")
were moving the bss_start/end on c generated variables that were
injected in their own sections. The reason was that we needed relative
relocations for position independent code and linker bugs back then
prevented us from doing so [0].

However, the linker documentation pages states that symbols that are
defined within a section definition will create a relocatable type with
the value being a fixed offset from the base of a section [1].
So let's start cleaning this up starting with the bss_start and bss_end
variables. Convert them into symbols within the .bss section definition.

[0] binutils commit 6b3b0ab89663 ("Make linker assigned symbol dynamic only for 
shared object")
[1] https://sourceware.org/binutils/docs/ld/Expression-Section.html

Tested-by: Caleb Connolly  # Qualcomm sdm845
Tested-by: Sam Edwards  # Binary output identical
Signed-off-by: Ilias Apalodimas 
---
 arch/arm/cpu/armv8/u-boot-spl.lds| 18 +++---
 arch/arm/cpu/armv8/u-boot.lds| 16 
 arch/arm/cpu/u-boot.lds  | 22 +++---
 arch/arm/lib/sections.c  |  2 --
 arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 15 ---
 arch/arm/mach-zynq/u-boot.lds| 22 +++---
 6 files changed, 29 insertions(+), 66 deletions(-)

diff --git a/arch/arm/cpu/armv8/u-boot-spl.lds 
b/arch/arm/cpu/armv8/u-boot-spl.lds
index 7cb9d731246d..8998c4985eac 100644
--- a/arch/arm/cpu/armv8/u-boot-spl.lds
+++ b/arch/arm/cpu/armv8/u-boot-spl.lds
@@ -63,18 +63,11 @@ SECTIONS
 
_image_binary_end = .;
 
-   .bss_start (NOLOAD) : {
-   . = ALIGN(8);
-   KEEP(*(.__bss_start));
-   } >.sdram
-
-   .bss (NOLOAD) : {
+   .bss : {
+   __bss_start = .;
*(.bss*)
-. = ALIGN(8);
-   } >.sdram
-
-   .bss_end (NOLOAD) : {
-   KEEP(*(.__bss_end));
+   . = ALIGN(8);
+   __bss_end = .;
} >.sdram
 
/DISCARD/ : { *(.rela*) }
@@ -89,3 +82,6 @@ SECTIONS
 #include "linux-kernel-image-header-vars.h"
 #endif
 }
+
+ASSERT(ADDR(.bss) % 8 == 0, \
+   ".bss must be 8-byte aligned");
diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds
index fb6a30c922f7..9640cc7a04b8 100644
--- a/arch/arm/cpu/armv8/u-boot.lds
+++ b/arch/arm/cpu/armv8/u-boot.lds
@@ -149,19 +149,11 @@ SECTIONS
 
_end = .;
 
-   . = ALIGN(8);
-
-   .bss_start : {
-   KEEP(*(.__bss_start));
-   }
-
-   .bss : {
+   .bss ALIGN(8): {
+   __bss_start = .;
*(.bss*)
-. = ALIGN(8);
-   }
-
-   .bss_end : {
-   KEEP(*(.__bss_end));
+   . = ALIGN(8);
+   __bss_end = .;
}
 
/DISCARD/ : { *(.dynsym) }
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index 7724c9332c3b..0dfe5f633b16 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -207,23 +207,15 @@ SECTIONS
}
 
 /*
- * Compiler-generated __bss_start and __bss_end, see arch/arm/lib/bss.c
- * __bss_base and __bss_limit are for linker only (overlay ordering)
+ * These sections occupy the same memory, but their lifetimes do
+ * not overlap: U-Boot initializes .bss only after applying dynamic
+ * relocations and therefore after it doesn't need .rel.dyn any more.
  */
-
-   .bss_start __rel_dyn_start (OVERLAY) : {
-   KEEP(*(.__bss_start));
-   __bss_base = .;
-   }
-
-   .bss __bss_base (OVERLAY) : {
+   .bss ADDR(.rel.dyn) (OVERLAY): {
+   __bss_start = .;
*(.bss*)
-. = ALIGN(4);
-__bss_limit = .;
-   }
-
-   .bss_end __bss_limit (OVERLAY) : {
-   KEEP(*(.__bss_end));
+   . = ALIGN(4);
+   __bss_end = .;
}
 
.dynsym _image_binary_end : { *(.dynsym) }
diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c
index 857879711c6a..8e8bd5797e16 100644
--- a/arch/arm/lib/sections.c
+++ b/arch/arm/lib/sections.c
@@ -19,8 +19,6 @@
  * aliasing warnings.
  */
 
-char __bss_start[0] __section(".__bss_start");
-char __bss_end[0] __section(".__bss_end");
 char __image_copy_start[0] __section(".__image_copy_start");
 char __image_copy_end[0] __section(".__image_copy_end");
 char __rel_dyn_start[0] __section(".__rel_dyn_start");
diff --git a/arch/arm/mach-rockchip/u-boot-tpl-v8.lds 
b/arch/arm/mach-rockchip/u-boot-tpl-v8.lds
index 74618eba591b..712c485d4d0b 100644
--- a/arch/arm/mach-rockchip/u-boot-tpl-v8.lds
+++ b/arch/arm/mach-rockchip/u-boot-tpl-v8.lds
@@ -56,18 +56,11 @@ SECTIONS
 
_image_binary_end = .;
 
-   .bss_start (NOLOAD) : {
-   . = ALIGN(8);
-   KEEP(*(.__bss_start))

[PATCH v4 1/7] arm: baltos: remove custom linker script

2024-03-14 Thread Ilias Apalodimas
commit 3d74a0977f514 ("ti: am335x: Remove unused linker script") removed
the linker script for the TI variant. This linker script doesn't seem to
do anything special and on top of that, has no definitions for the EFI
runtime sections.

So let's get rid of it and use the generic linker script which defines
those correctly

Signed-off-by: Ilias Apalodimas 
Reviewed-by: Tom Rini 
---
 board/vscom/baltos/u-boot.lds | 128 --
 1 file changed, 128 deletions(-)
 delete mode 100644 board/vscom/baltos/u-boot.lds

diff --git a/board/vscom/baltos/u-boot.lds b/board/vscom/baltos/u-boot.lds
deleted file mode 100644
index cb2ee6769753..
--- a/board/vscom/baltos/u-boot.lds
+++ /dev/null
@@ -1,128 +0,0 @@
-/*
- * Copyright (c) 2004-2008 Texas Instruments
- *
- * (C) Copyright 2002
- * Gary Jennejohn, DENX Software Engineering, 
- *
- * See file CREDITS for list of people who contributed to this
- * project.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation; either version 2 of
- * the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- * MA 02111-1307 USA
- */
-
-OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
-OUTPUT_ARCH(arm)
-ENTRY(_start)
-SECTIONS
-{
-   . = 0x;
-
-   . = ALIGN(4);
-   .text :
-   {
-   *(.__image_copy_start)
-   *(.vectors)
-   CPUDIR/start.o (.text*)
-   board/vscom/baltos/built-in.o (.text*)
-   *(.text*)
-   }
-
-   . = ALIGN(4);
-   .rodata : { *(SORT_BY_ALIGNMENT(SORT_BY_NAME(.rodata*))) }
-
-   . = ALIGN(4);
-   .data : {
-   *(.data*)
-   }
-
-   . = ALIGN(4);
-
-   . = .;
-
-   . = ALIGN(4);
-   __u_boot_list : {
-   KEEP(*(SORT(__u_boot_list*)));
-   }
-
-   . = ALIGN(4);
-
-   .image_copy_end :
-   {
-   *(.__image_copy_end)
-   }
-
-   .rel_dyn_start :
-   {
-   *(.__rel_dyn_start)
-   }
-
-   .rel.dyn : {
-   *(.rel*)
-   }
-
-   .rel_dyn_end :
-   {
-   *(.__rel_dyn_end)
-   }
-
-   .hash : { *(.hash*) }
-
-   .end :
-   {
-   *(.__end)
-   }
-
-   _image_binary_end = .;
-
-   /*
-* Deprecated: this MMU section is used by pxa at present but
-* should not be used by new boards/CPUs.
-*/
-   . = ALIGN(4096);
-   .mmutable : {
-   *(.mmutable)
-   }
-
-/*
- * Compiler-generated __bss_start and __bss_end, see arch/arm/lib/bss.c
- * __bss_base and __bss_limit are for linker only (overlay ordering)
- */
-
-   .bss_start __rel_dyn_start (OVERLAY) : {
-   KEEP(*(.__bss_start));
-   __bss_base = .;
-   }
-
-   .bss __bss_base (OVERLAY) : {
-   *(.bss*)
-. = ALIGN(4);
-__bss_limit = .;
-   }
-
-   .bss_end __bss_limit (OVERLAY) : {
-   KEEP(*(.__bss_end));
-   }
-
-   .dynsym _image_binary_end : { *(.dynsym) }
-   .dynbss : { *(.dynbss) }
-   .dynstr : { *(.dynstr*) }
-   .dynamic : { *(.dynamic*) }
-   .gnu.hash : { *(.gnu.hash) }
-   .plt : { *(.plt*) }
-   .interp : { *(.interp*) }
-   .gnu : { *(.gnu*) }
-   .ARM.exidx : { *(.ARM.exidx*) }
-}
-- 
2.37.2



[PATCH v4 0/7] Clean up arm linker scripts

2024-03-14 Thread Ilias Apalodimas
The arm linker scripts had a mix of symbols and C defined variables in an
effort to emit relative references instead of absolute ones e.g [0]. A
linker bug prevented us from doing so [1] -- fixed since 2016.
This has led to confusion over the years, ending up with mixed section
definitions. Some sections are defined with overlays and different
definitions between v7 and v8 architectures.
For example __efi_runtime_rel_start/end is defined as a linker symbol for
armv8 and a C variable in armv7.

Linker scripts nowadays can emit relative references, as long as the symbol
definition is contained within the section definition. So let's switch most
of the C defined variables and clean up the arm sections.c file.
There's still a few symbols remaining -- __secure_start/end,
__secure_stack_start/end and __end which can be cleaned up
in a followup series.

For both QEMU v7/v8 bloat-o-meter shows now size difference
$~ ./scripts/bloat-o-meter u-boot u-boot.new
add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
Function old new   delta
Total: Before=798861, After=798861, chg +0.00%

The symbols seem largely unchanged apart from a difference in .bss
as well as the emited sections and object types of the affected variables.

On the output below the first value is from -next and the second comes from
-next + this patchset. The .bss_start/end sections have disappeared from
the newer binaries.

# For example on QEMU v8:
efi_runtime_start
  7945: 0178 0 OBJECT  GLOBAL DEFAULT2 __efi_runtime_start
  7942: 0178 0 NOTYPE  GLOBAL DEFAULT2 __efi_runtime_start
efi_runtime_stop
  9050: 0d38 0 OBJECT  GLOBAL DEFAULT2 __efi_runtime_stop
  9047: 0d38 0 NOTYPE  GLOBAL DEFAULT2 __efi_runtime_stop
__efi_runtime_rel_start
  7172: 000dc2f0 0 OBJECT  GLOBAL DEFAULT   10 
__efi_runtime_rel_start
  7169: 000dc2f0 0 NOTYPE  GLOBAL DEFAULT   10 
__efi_runtime_rel_start
__efi_runtime_rel_stop
  7954: 000dc4a0 0 OBJECT  GLOBAL DEFAULT   10 
__efi_runtime_rel_stop
  7951: 000dc4a0 0 NOTYPE  GLOBAL DEFAULT   10 
__efi_runtime_rel_stop
__rel_dyn_start
  7030: 000dc4a0 0 OBJECT  GLOBAL DEFAULT   11 __rel_dyn_start
  7027: 000dc4a0 0 NOTYPE  GLOBAL DEFAULT   11 __rel_dyn_start
__rel_dyn_end
  8959: 00102b10 0 OBJECT  GLOBAL DEFAULT   12 __rel_dyn_end
  8956: 00102b10 0 NOTYPE  GLOBAL DEFAULT   11 __rel_dyn_end
image_copy_start
  9051:  0 OBJECT  GLOBAL DEFAULT1 __image_copy_start
  9048:  0 NOTYPE  GLOBAL DEFAULT1 __image_copy_start
image_copy_end
  7467: 000dc4a0 0 OBJECT  GLOBAL DEFAULT   11 __image_copy_end
  7464: 000dc4a0 0 NOTYPE  GLOBAL DEFAULT   11 __image_copy_end
bss_start
12: 00102b10 0 SECTION LOCAL  DEFAULT   12 .bss_start
  8087: 0018 0 NOTYPE  GLOBAL DEFAULT1 _bss_start_ofs
  8375: 00102b10 0 OBJECT  GLOBAL DEFAULT   12 __bss_start
  8084: 0018 0 NOTYPE  GLOBAL DEFAULT1 _bss_start_ofs
  8372: 00102b10 0 NOTYPE  GLOBAL DEFAULT   12 __bss_start
bss_end
14: 0010bc30 0 SECTION LOCAL  DEFAULT   14 .bss_end
  7683: 0010bc30 0 OBJECT  GLOBAL DEFAULT   14 __bss_end
  8479: 0020 0 NOTYPE  GLOBAL DEFAULT1 _bss_end_ofs
  7680: 0010bbb0 0 NOTYPE  GLOBAL DEFAULT   12 __bss_end
  8476: 0020 0 NOTYPE  GLOBAL DEFAULT1 _bss_end_ofs

# For QEMU v7:
efi_runtime_start
 10703: 03bc 0 OBJECT  GLOBAL DEFAULT2 __efi_runtime_start
 10699: 03c0 0 NOTYPE  GLOBAL DEFAULT2 __efi_runtime_start
efi_runtime_stop
 11796: 12ec 0 OBJECT  GLOBAL DEFAULT2 __efi_runtime_stop
 11792: 12ec 0 NOTYPE  GLOBAL DEFAULT2 __efi_runtime_stop
__efi_runtime_rel_start
  9937: 000c40dc 0 OBJECT  GLOBAL DEFAULT8 __efi_runtime_rel_start
  9935: 000c40dc 0 NOTYPE  GLOBAL DEFAULT9 __efi_runtime_rel_start
__efi_runtime_rel_stop
 10712: 000c41dc 0 OBJECT  GLOBAL DEFAULT   10 __efi_runtime_rel_stop
 10708: 000c41dc 0 NOTYPE  GLOBAL DEFAULT9 __efi_runtime_rel_stop
__rel_dyn_start
  9791: 000c41dc 0 OBJECT  GLOBAL DEFAULT   10 __rel_dyn_start
  9789: 000c41dc 0 NOTYPE  GLOBAL DEFAULT   10 __rel_dyn_start
__rel_dyn_end
 11708: 000da5f4 0 OBJECT  GLOBAL DEFAULT   10 __rel_dyn_end
 11704: 000da5f4 0 NOTYPE  GLOBAL DEFAULT   10 __rel_dyn_end
image_copy_start
   448: 177c 0 NOTYPE  LOCAL  DEFAULT3 _image_copy_start_ofs
 11797:  0 OBJECT  GLOBAL DEFAULT1 __image_copy_start
   445: 177c 0 NOTYPE  LOCAL  DEFAULT3 _image_copy_start_ofs
 11793:  0 NOTYPE  GLOBAL DEFAULT1 __image_copy_start
image_copy_end
   450: 1780 0 NOTYPE  LOCAL  DEFAULT3 _image_copy_end_ofs
 10225: 000c41dc 0 OBJECT  GLOBAL DEFAULT   

Re: [PATCH v3 08/11] pci: Add DW PCIe controller support for iMX8MP SoC

2024-03-14 Thread Sumit Garg
On Thu, 14 Mar 2024 at 09:45, Marek Vasut  wrote:
>
> On 3/12/24 8:03 AM, Sumit Garg wrote:
> > pcie_imx doesn't seem to share any useful code for iMX8 SoC and it is
> > tied to quite old port of pcie_designware driver from Linux which
> > suffices only iMX6 specific needs.
> >
> > But currently we have the common DWC specific bits which alligns pretty
> > well with DW PCIe controller on iMX8MP SoC. So lets reuse those common
> > bits instead as a new driver for iMX8 SoCs. It should be fairly easy to
> > add support for other iMX8 variants to this driver.
> >
> > iMX8MP SoC also comes up with standalone PCIe PHY support, so hence we
> > can reuse the generic PHY infrastructure to power on PCIe PHY.
> >
> > Tested-by: Tim Harvey  #imx8mp-venice*
> > Tested-by: Adam Ford  #imx8mp-beacon-kit
> > Signed-off-by: Sumit Garg 
>
> [...]
>
> > +static int pcie_dw_imx_of_to_plat(struct udevice *dev)
> > +{
> > + struct pcie_dw_imx *priv = dev_get_priv(dev);
> > + ofnode gpr;
> > + int ret;
> > +
> > + /* Get the controller base address */
> > + priv->dw.dbi_base = (void *)dev_read_addr_name(dev, "dbi");
> > + if ((fdt_addr_t)priv->dw.dbi_base == FDT_ADDR_T_NONE) {
> > + dev_err(dev, "failed to get dbi_base address\n");
> > + return -EINVAL;
> > + }
> > +
> > + /* Get the config space base address and size */
> > + priv->dw.cfg_base = (void *)dev_read_addr_size_name(dev, "config",
> > + 
> > &priv->dw.cfg_size);
> > + if ((fdt_addr_t)priv->dw.cfg_base == FDT_ADDR_T_NONE) {
> > + dev_err(dev, "failed to get cfg_base address\n");
> > + return -EINVAL;
> > + }
> > +
> > + ret = clk_get_bulk(dev, &priv->clks);
> > + if (ret) {
> > + dev_err(dev, "failed to get PCIe clks\n");
> > + return ret;
> > + }
> > +
> > + ret = reset_get_by_name(dev, "apps", &priv->apps_reset);
> > + if (ret) {
> > + dev_err(dev,
> > + "Failed to get PCIe apps reset control\n");
> > + goto err_reset;
> > + }
> > +
> > + ret = gpio_request_by_name(dev, "reset-gpio", 0, &priv->reset_gpio,
> > +(GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE));
>
> The extra parenthesis () is not needed.
>

Ack.

> With that fixed:
>
> Reviewed-by: Marek Vasut 

Thanks.

-Sumit


Re: [PATCH v3 06/11] imx8mp: power-domain: Expose high performance PLL clock

2024-03-14 Thread Sumit Garg
On Thu, 14 Mar 2024 at 09:45, Marek Vasut  wrote:
>
> On 3/12/24 8:03 AM, Sumit Garg wrote:
> > Expose the high performance PLL as a regular Linux clock
>
> ... as clock framework clock ...
>

Ack.

> > , so the
> > PCIe PHY can use it when there is no external refclock provided.
> >
> > Inspired from counterpart Linux kernel v6.8-rc3 driver:
> > drivers/pmdomain/imx/imx8mp-blk-ctrl.c. Use last Linux kernel driver
> > reference commit 7476ddfd36ac ("pmdomain: imx8mp-blk-ctrl: Convert to
> > platform remove callback returning void").
>
> With that fixed:
>
> Reviewed-by: Marek Vasut 

Thanks.

-Sumit


Re: [PATCH v3 07/11] phy: phy-imx8m-pcie: Add support for i.MX8M{M/P} PCIe PHY

2024-03-14 Thread Sumit Garg
On Thu, 14 Mar 2024 at 09:46, Marek Vasut  wrote:
>
> On 3/12/24 8:03 AM, Sumit Garg wrote:
> > Add initial support for i.MX8M{M/P} PCIe PHY. On i.MX8M{M/P} SoCs PCIe
> > PHY initialization moved to this standalone PHY driver.
> >
> > Inspired from counterpart Linux kernel v6.8-rc3 driver:
> > drivers/phy/freescale/phy-fsl-imx8m-pcie.c. Use last Linux kernel driver
> > reference commit 7559e7572c03 ("phy: Explicitly include correct DT
> > includes").
>
> [...]
>
> > +static int imx8_pcie_phy_probe(struct udevice *dev)
> > +{
> > + struct imx8_pcie_phy *imx8_phy = dev_get_priv(dev);
> > + ofnode gpr;
> > + int ret = 0;
> > +
> > + imx8_phy->drvdata = (void *)dev_get_driver_data(dev);
> > + imx8_phy->base = dev_read_addr(dev);
> > + if (!imx8_phy->base)
> > + return -EINVAL;
> > +
> > + /* get PHY refclk pad mode */
> > + dev_read_u32(dev, "fsl,refclk-pad-mode", &imx8_phy->refclk_pad_mode);
> > +
> > + imx8_phy->tx_deemph_gen1 = dev_read_u32_default(dev,
> > + "fsl,tx-deemph-gen1",
> > + 0);
> > + imx8_phy->tx_deemph_gen2 = dev_read_u32_default(dev,
> > + "fsl,tx-deemph-gen2",
> > + 0);
> > + imx8_phy->clkreq_unused = dev_read_bool(dev, 
> > "fsl,clkreq-unsupported");
> > +
> > + /* Grab GPR config register range */
> > + gpr = ofnode_by_compatible(ofnode_null(), imx8_phy->drvdata->gpr);
> > + if (ofnode_equal(gpr, ofnode_null())) {
> > + dev_err(dev, "unable to find GPR node\n");
> > + return -ENODEV;
> > + }
> > +
> > + imx8_phy->iomuxc_gpr = syscon_node_to_regmap(gpr);
> > + if (IS_ERR(imx8_phy->iomuxc_gpr)) {
> > + dev_err(dev, "unable to find iomuxc registers\n");
> > + return PTR_ERR(imx8_phy->iomuxc_gpr);
> > + }
>
> syscon_regmap_lookup_by_compatible() should simplify these two steps ^ .
>

Ack.

> With that fixed:
>
> Reviewed-by: Marek Vasut 
>

Thanks.

-Sumit

> [...]


Re: [PATCH v3 02/11] reset: imx: Refactor driver to simplify function names

2024-03-14 Thread Sumit Garg
On Thu, 14 Mar 2024 at 09:45, Marek Vasut  wrote:
>
> On 3/12/24 8:03 AM, Sumit Garg wrote:
> > imx7_reset_{deassert/assert}_imx* are a bit more confusing when compared
> > with imx*_reset_{deassert/assert}. So refactor driver to use function
> > names easier to understand. This shouldn't affect the functionality
> > though.
> >
> > Suggested-by: Marek Vasut 
> > Signed-off-by: Sumit Garg 
> > ---
> >   drivers/reset/reset-imx7.c | 24 
>
> You could even rename the driver itself now, but that would be separate
> patch.

That can be done as a followup patch.

>
> >   1 file changed, 12 insertions(+), 12 deletions(-)
> Reviewed-by: Marek Vasut 

Thanks.

-Sumit


Re: [PATCH v3 03/11] reset: imx: Add support for i.MX8MP reset controller

2024-03-14 Thread Sumit Garg
On Thu, 14 Mar 2024 at 09:45, Marek Vasut  wrote:
>
> On 3/12/24 8:03 AM, Sumit Garg wrote:
> > Add support for i.MX8MP reset controller, it has same reset IP inside
> > as the other iMX7 and iMX8 variants but with different module layout.
>
> iMX8M , iMX8 is a different SoC .

Ack.

>
> > Inspired from counterpart Linux kernel v6.8-rc3 driver:
> > drivers/reset/reset-imx7.c. Use last Linux kernel driver reference
> > commit bad8a8afe19f ("reset: Explicitly include correct DT includes").
> >
> > Tested-by: Tim Harvey  #imx8mp-venice*
> > Tested-by: Adam Ford  #imx8mp-beacon-kit
> > Signed-off-by: Sumit Garg 
> > ---
> >   drivers/reset/reset-imx7.c | 101 +
> >   1 file changed, 101 insertions(+)
> >
> > diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c
> > index 4c7fa19d495..90d3d75255e 100644
> > --- a/drivers/reset/reset-imx7.c
> > +++ b/drivers/reset/reset-imx7.c
> > @@ -10,6 +10,7 @@
> >   #include 
> >   #include 
> >   #include 
> > +#include 
>
> Keep the list sorted (P is before Q).

Ack.

>
> >   #include 
> >   #include 
> >   #include 
> > @@ -252,6 +253,102 @@ static int imx8mq_reset_assert(struct reset_ctl *rst)
> >   return 0;
> >   }
> >
> > +enum imx8mp_src_registers {
> > + SRC_SUPERMIX_RCR= 0x0018,
> > + SRC_AUDIOMIX_RCR= 0x001c,
> > + SRC_MLMIX_RCR   = 0x0028,
> > + SRC_GPU2D_RCR   = 0x0038,
> > + SRC_GPU3D_RCR   = 0x003c,
> > + SRC_VPU_G1_RCR  = 0x0048,
> > + SRC_VPU_G2_RCR  = 0x004c,
> > + SRC_VPUVC8KE_RCR= 0x0050,
> > + SRC_NOC_RCR = 0x0054,
> > +};
> > +
> > +static const struct imx7_src_signal imx8mp_src_signals[IMX8MP_RESET_NUM] = 
> > {
> > + [IMX8MP_RESET_A53_CORE_POR_RESET0]  = { SRC_A53RCR0, BIT(0) },
> > + [IMX8MP_RESET_A53_CORE_POR_RESET1]  = { SRC_A53RCR0, BIT(1) },
> > + [IMX8MP_RESET_A53_CORE_POR_RESET2]  = { SRC_A53RCR0, BIT(2) },
> > + [IMX8MP_RESET_A53_CORE_POR_RESET3]  = { SRC_A53RCR0, BIT(3) },
> > + [IMX8MP_RESET_A53_CORE_RESET0]  = { SRC_A53RCR0, BIT(4) },
> > + [IMX8MP_RESET_A53_CORE_RESET1]  = { SRC_A53RCR0, BIT(5) },
> > + [IMX8MP_RESET_A53_CORE_RESET2]  = { SRC_A53RCR0, BIT(6) },
> > + [IMX8MP_RESET_A53_CORE_RESET3]  = { SRC_A53RCR0, BIT(7) },
> > + [IMX8MP_RESET_A53_DBG_RESET0]   = { SRC_A53RCR0, BIT(8) },
> > + [IMX8MP_RESET_A53_DBG_RESET1]   = { SRC_A53RCR0, BIT(9) },
> > + [IMX8MP_RESET_A53_DBG_RESET2]   = { SRC_A53RCR0, BIT(10) },
> > + [IMX8MP_RESET_A53_DBG_RESET3]   = { SRC_A53RCR0, BIT(11) },
> > + [IMX8MP_RESET_A53_ETM_RESET0]   = { SRC_A53RCR0, BIT(12) },
> > + [IMX8MP_RESET_A53_ETM_RESET1]   = { SRC_A53RCR0, BIT(13) },
> > + [IMX8MP_RESET_A53_ETM_RESET2]   = { SRC_A53RCR0, BIT(14) },
> > + [IMX8MP_RESET_A53_ETM_RESET3]   = { SRC_A53RCR0, BIT(15) },
> > + [IMX8MP_RESET_A53_SOC_DBG_RESET]= { SRC_A53RCR0, BIT(20) },
> > + [IMX8MP_RESET_A53_L2RESET]  = { SRC_A53RCR0, BIT(21) },
> > + [IMX8MP_RESET_SW_NON_SCLR_M7C_RST]  = { SRC_M4RCR, BIT(0) },
> > + [IMX8MP_RESET_OTG1_PHY_RESET]   = { SRC_USBOPHY1_RCR, BIT(0) 
> > },
> > + [IMX8MP_RESET_OTG2_PHY_RESET]   = { SRC_USBOPHY2_RCR, BIT(0) 
> > },
> > + [IMX8MP_RESET_SUPERMIX_RESET]   = { SRC_SUPERMIX_RCR, BIT(0) 
> > },
> > + [IMX8MP_RESET_AUDIOMIX_RESET]   = { SRC_AUDIOMIX_RCR, BIT(0) 
> > },
> > + [IMX8MP_RESET_MLMIX_RESET]  = { SRC_MLMIX_RCR, BIT(0) },
> > + [IMX8MP_RESET_PCIEPHY]  = { SRC_PCIEPHY_RCR, BIT(2) },
> > + [IMX8MP_RESET_PCIEPHY_PERST]= { SRC_PCIEPHY_RCR, BIT(3) },
> > + [IMX8MP_RESET_PCIE_CTRL_APPS_EN]= { SRC_PCIEPHY_RCR, BIT(6) },
> > + [IMX8MP_RESET_PCIE_CTRL_APPS_TURNOFF]   = { SRC_PCIEPHY_RCR, BIT(11) 
> > },
> > + [IMX8MP_RESET_HDMI_PHY_APB_RESET]   = { SRC_HDMI_RCR, BIT(0) },
> > + [IMX8MP_RESET_MEDIA_RESET]  = { SRC_DISP_RCR, BIT(0) },
> > + [IMX8MP_RESET_GPU2D_RESET]  = { SRC_GPU2D_RCR, BIT(0) },
> > + [IMX8MP_RESET_GPU3D_RESET]  = { SRC_GPU3D_RCR, BIT(0) },
> > + [IMX8MP_RESET_GPU_RESET]= { SRC_GPU_RCR, BIT(0) },
> > + [IMX8MP_RESET_VPU_RESET]= { SRC_VPU_RCR, BIT(0) },
> > + [IMX8MP_RESET_VPU_G1_RESET] = { SRC_VPU_G1_RCR, BIT(0) },
> > + [IMX8MP_RESET_VPU_G2_RESET] = { SRC_VPU_G2_RCR, BIT(0) },
> > + [IMX8MP_RESET_VPUVC8KE_RESET]   = { SRC_VPUVC8KE_RCR, BIT(0) 
> > },
> > + [IMX8MP_RESET_NOC_RESET]= { SRC_NOC_RCR, BIT(0) },
> > +};
> > +
> > +static int imx8mp_reset_set(struct reset_ctl *rst, bool assert)
> > +{
> > + struct imx7_reset_priv *priv = dev_get_priv(rst->dev);
>
> It wouldn't hurt to rename imx7_reset_priv to imx_reset_priv i

Re: [PATCH v3 04/11] imx8mp: power-domain: Don't power off pd_bus

2024-03-14 Thread Sumit Garg
On Thu, 14 Mar 2024 at 09:45, Marek Vasut  wrote:
>
> On 3/12/24 8:03 AM, Sumit Garg wrote:
> > power_domain_on/off() isn't refcounted and power domain bus shouldn't be
> > turned off for a single peripheral domain as it would negatively affect
> > other peripheral domains. So lets just skip turning off bus power
> > domain.
>
> What exactly is the issue and how did you trigger it ?
>
> Details please.

I suppose the issue can be triggered via the "=> usb start => usb
stop" sequence where one of the USB controllers is configured in
peripheral mode.

>
> > Fixes: 898e7610c62a ("imx: power-domain: Add i.MX8MP HSIOMIX driver")
> > Signed-off-by: Sumit Garg 
> > ---
> >   drivers/power/domain/imx8mp-hsiomix.c | 6 +-
> >   1 file changed, 1 insertion(+), 5 deletions(-)
> >
> > diff --git a/drivers/power/domain/imx8mp-hsiomix.c 
> > b/drivers/power/domain/imx8mp-hsiomix.c
> > index e2d772c5ec7..448746432a2 100644
> > --- a/drivers/power/domain/imx8mp-hsiomix.c
> > +++ b/drivers/power/domain/imx8mp-hsiomix.c
> > @@ -50,7 +50,7 @@ static int imx8mp_hsiomix_on(struct power_domain 
> > *power_domain)
> >
> >   ret = power_domain_on(domain);
> >   if (ret)
> > - goto err_pd;
> > + return ret;
> >
> >   ret = clk_enable(&priv->clk_usb);
> >   if (ret)
> > @@ -63,8 +63,6 @@ static int imx8mp_hsiomix_on(struct power_domain 
> > *power_domain)
> >
> >   err_clk:
> >   power_domain_off(domain);
> > -err_pd:
> > - power_domain_off(&priv->pd_bus);
> >   return ret;
>
> Why not add counter into imx8mp_hsiomix_priv structure in this driver ?

Sure I can do that but do you think the current approach can have any
side effects?

-Sumit


Re: [PATCH] imx8m*_venice: move venice to OF_UPSTREAM

2024-03-14 Thread Sumit Garg
On Thu, 14 Mar 2024 at 21:28, Tim Harvey  wrote:
>
> On Thu, Mar 14, 2024 at 12:50 AM Sumit Garg  wrote:
> >
> > + Tom
> >
> > Hi Tim,
> >
> > On Wed, 13 Mar 2024 at 22:01, Tim Harvey  wrote:
> > >
> > > On Wed, Mar 13, 2024 at 6:20 AM Sumit Garg  wrote:
> > > >
> > > > On Wed, 13 Mar 2024 at 06:46, Fabio Estevam  wrote:
> > > > >
> > > > > Hi Tim,
> > > > >
> > > > > On Tue, Mar 12, 2024 at 4:05 PM Tim Harvey  
> > > > > wrote:
> > > > > >
> > > > > > Move to imx8m{m,n,p}-venice to OF_UPSTREAM:
> > > > > >  - replace the non-upstream generic imx8m{m,n,p}-venice dt with one 
> > > > > > of the
> > > > > >dt's from the OF_LIST
> > > > > >  - handle the fact that dtbs now have a 'freescale/' prefix
> > > > > >  - imply OF_UPSTREAM
> > > > > >  - remove rudundant files from arch/arm/dts leaving only the
> > > > > >*-u-boot.dtsi files
> > > > > >
> > > > > > Signed-off-by: Tim Harvey 
> > > > > ...
> > > > > >  33 files changed, 13 insertions(+), 10658 deletions(-)
> > > > >
> > > > > This diff looks great :-)
> > > >
> > > > +1
> > > >
> > > > Reviewed-by: Sumit Garg 
> > > >
> > >
> > > Hi Sumit,
> > >
> > > Thanks for your work on this - I imagine over time this will
> > > de-duplicate a lot of work!
> > >
> > > I have a couple of questions about OF_UPSTREAM I haven't found the
> > > answer to yet:
> > > 1. how do you determine what the last sync point was in dts/upstream?
> > > (ie what kernel version is it currently sync'd with)
> >
> > You can get that information from merge commits that happen over
> > dts/upstream sub-directory. It was somewhat raw information for the
> > first time we added dts/upstream as (commit aaba2d45dc2a pointing to
> > v6.7-dts [1]):
> >
> > commit 53633a893a06bd5a0c807287d9cc29337806eaf7
> > Author: Tom Rini 
> > Date:   Thu Feb 29 12:33:36 2024 -0500
> >
> > Squashed 'dts/upstream/' content from commit aaba2d45dc2a
> >
> > git-subtree-dir: dts/upstream
> > git-subtree-split: aaba2d45dc2a1b3bbb710f2a3808ee1c9f340abe
> >
> > [1] 
> > https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git/tag/?h=v6.7-dts
> >
> > However, further syncs would be more clear from merge commit
> > description, try following:
> >
> > $ ./dts/update-dts-subtree.sh pull v6.8-dts
> >
> > You will see merge commit saying:
> >
> > commit 57508745cd2b07e55b5c414c6d646de4865418d2 (HEAD -> dt_uprev)
> > Merge: 3987e15e88a 14c9e8c0856
> > Author: Sumit Garg 
> > Date:   Thu Mar 14 12:20:27 2024 +0530
> >
> > Subtree merge tag 'v6.8-dts' of devicetree-rebasing repo [1] into
> > dts/upstream
> >
> > [1] 
> > https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git/
> >
> > > 2. how often will dts/upstream get re-synced (not the
> > > devicetree-rebasing.git but u-boot dts/upstream),
> >
> > You can see the re-syncing details here (section: "Resyncing with
> > devicetree-rebasing",
> > https://source.denx.de/u-boot/u-boot/-/blob/next/doc/develop/devicetree/control.rst?ref_type=heads).
> >
>
> Thanks - I was confused about this as I originally figured it was done
> via a git submodule. When I looked at the commit history of
> dts/upstream it wasn't clear at all because of the first mergebeing
> done differently as you explain above. As long as
> update-dts-subtree.sh is run with a clear named tag that has the
> kernel version in it then it's easy to follow via the short commit
> description.
>
> > > who do you suspect
> > > will do be doing it?
> >
> > Tom will be doing that. I suppose we are just in time for a resync
> > since the Linux kernel v6.8 was released.
> >
>
> Ok, that makes sense. I assume Tom will be updating it on major kernel
> releases and possibly rc's leading up to as well.

The current policy is to sync on major kernel releases only after the
U-Boot next branch opens. Since we would like to avoid any DT ABI
breakages for U-Boot and at the same time provide sufficient time to
fix problems if any. However if DT ABI turns out to be much more
stable for U-Boot then we can revisit this policy.

>
> > Tom,
> >
> > Do you think we can make v6.8-dts sync for the U-Boot next?
> >
> > > 3. how would one go about adding a new feature via dt to uboot when
> > > the same feature has not yet landed in dts upstream? perhaps the
> > > answer is it must land upstream first or do you suspect it would be ok
> > > to put something in a u-boot.dtsi that can later get removed as
> > > redundant?
> >
> > From an upstream perspective, we should aim for minimal contents in
> > *-u-boot.dtsi, ideally it would be better to get rid of them. For eg.
> > all bootph_* properties should all be pushed upstream.
> >
> > However, from the new features perspective we can consider using
> > *-u-boot.dtsi if the changes are minimal like enabling some DT nodes.
> > But if the changes are significant then that board can just opt out of
> > OF_USTREAM but still reusing all the DT includes. The reasoning behind
> > this is to minimize any chances of breaka

[PATCH] CI: Move to latest container image

2024-03-14 Thread Tom Rini
This moves us to our latest container image, which is now based on the
current "Jammy" tag.

Signed-off-by: Tom Rini 
---
 .azure-pipelines.yml| 2 +-
 .gitlab-ci.yml  | 2 +-
 tools/docker/Dockerfile | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml
index f8a3a9a1c8c5..27f69583c655 100644
--- a/.azure-pipelines.yml
+++ b/.azure-pipelines.yml
@@ -2,7 +2,7 @@ variables:
   windows_vm: windows-2019
   ubuntu_vm: ubuntu-22.04
   macos_vm: macOS-12
-  ci_runner_image: trini/u-boot-gitlab-ci-runner:jammy-20240125-12Feb2024
+  ci_runner_image: trini/u-boot-gitlab-ci-runner:jammy-20240227-14Mar2024
   # Add '-u 0' options for Azure pipelines, otherwise we get "permission
   # denied" error when it tries to "useradd -m -u 1001 vsts_azpcontainer",
   # since our $(ci_runner_image) user is not root.
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 6930b4158e14..165f765a8332 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -10,7 +10,7 @@ default:
 
 # Grab our configured image.  The source for this is found
 # in the u-boot tree at tools/docker/Dockerfile
-image: ${MIRROR_DOCKER}/trini/u-boot-gitlab-ci-runner:jammy-20240125-12Feb2024
+image: ${MIRROR_DOCKER}/trini/u-boot-gitlab-ci-runner:jammy-20240227-14Mar2024
 
 # We run some tests in different order, to catch some failures quicker.
 stages:
diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile
index 6d9931be7a81..cda87354566d 100644
--- a/tools/docker/Dockerfile
+++ b/tools/docker/Dockerfile
@@ -2,7 +2,7 @@
 # This Dockerfile is used to build an image containing basic stuff to be used
 # to build U-Boot and run our test suites.
 
-FROM ubuntu:jammy-20240125
+FROM ubuntu:jammy-20240227
 MAINTAINER Tom Rini 
 LABEL Description=" This image is for building U-Boot inside a container"
 
-- 
2.34.1



Re: [PATCH 2/3] riscv: dts: sophgo: Add ethernet node

2024-03-14 Thread Kongyang Liu
Conor Dooley  于2024年3月12日周二 21:20写道:
>
> On Tue, Mar 12, 2024 at 05:59:44PM +0800, Leo Liang wrote:
> > On Sun, Mar 10, 2024 at 01:56:45PM +0800, Kongyang Liu wrote:
> > > Add ethernet node for cv1800b SoC
> > >
> > > Signed-off-by: Kongyang Liu 
> > > ---
> > >
> > >  arch/riscv/dts/cv18xx.dtsi | 6 ++
> > >  1 file changed, 6 insertions(+)
> >
> > Hi KongYang,
> >
> > Will there be a patch adding this ethernet node for kernel as well ?
>
> It's highly like that the compatible of "cv1800b-ethernet" will be
> requested to be changed to "cv1800b-dwmac" to match the designware IP
> used in other SoCs.
>

I will change it in next version

> The added node also looks suspiciously missing any clocks or interrupts.

If we only consider u-boot, since u-boot does not utilize interrupts and
clocks, this code can function properly. However, if compatibility with
the kernel is a concern, I will discuss this issue with who is working on
Milk-V Duo Ethernet support for the kernel.

Best regards
Kongyang Liu


Re: [PATCH 2/3] riscv: dts: sophgo: Add ethernet node

2024-03-14 Thread Kongyang Liu
Leo Liang  于2024年3月12日周二 17:59写道:

>
> On Sun, Mar 10, 2024 at 01:56:45PM +0800, Kongyang Liu wrote:
> > Add ethernet node for cv1800b SoC
> >
> > Signed-off-by: Kongyang Liu 
> > ---
> >
> >  arch/riscv/dts/cv18xx.dtsi | 6 ++
> >  1 file changed, 6 insertions(+)
>
> Hi KongYang,
>
> Will there be a patch adding this ethernet node for kernel as well ?
>

Currently, I'm only focusing on the u-boot for the Milk-V Duo board, while
the kernel part is being handled by others. Therefore, this patch will not
be added to the kernel.

Best regards
Kongyang Liu

> Best regards,
> Leo


Pull request: u-boot-rockchip/for-next

2024-03-14 Thread Kever Yang
Hi Tom,

This is for next;
Please pull the updates for rockchip platform:
- Add board: rk3588 Generic, Cool Pi CM5, Theobroma-Systems RK3588 Jaguar SBC,
 Toybrick TB-RK3588X;
 rk3588s Cool Pi 4B;
 rk3566 Pine64 PineTab2;
- Add saradc v2 support;
- Add PMIC RK806 support;
- rk3588 disable force_jtag by default;
- Migrate to use IO-domain driver for all boards;
- Use common bss and stack addresses for rk33xx and rk35xx boards;
- Other updates for driver, config and dts;

CI:
https://source.denx.de/u-boot/custodians/u-boot-rockchip/-/pipelines/19951

Thanks,
- Kever

The following changes since commit 20a0ce574d6642e0dfe651467159039fac48cc4f:

  Merge tag 'v2024.04-rc4' into next (2024-03-11 15:27:20 -0400)

are available in the Git repository at:

  https://source.denx.de/u-boot/custodians/u-boot-rockchip.git 
tags/u-boot-rockchip-20240315

for you to fetch changes up to 12bc1a5462a22f6dc5b91ecbf092cbaf94e66820:

  rockchip: boot_mode: fix rockchip_dnl_key_pressed requiring ADC support 
(2024-03-14 18:20:04 +0800)


Andy Yan (2):
  board: rockchip: Add support for rk3588s based Cool Pi 4B
  board: rockchip: Add support for rk3588 based Cool Pi CM5 EVB

Ben Wolsieffer (1):
  rockchip: load env from boot MMC device

Chen-Yu Tsai (5):
  rockchip: rk3328: Read cpuid and generate MAC address from efuse
  rockchip: rk3399: Read cpuid and generate MAC address from efuse
  rockchip: rk3328: regenerate defconfigs
  rockchip: rk3399: regenerate defconfigs
  rockchip: nanopi-r4s: Drop ROCKCHIP_OTP

Chris Morgan (4):
  arm: dts: rockchip: rk3566: Remove unnecessary clks from rgxx3
  board: rockchip: Add support for Powkiddy RGB10MAX3
  configs: Remove unnecessary options from RGxx3 config
  board: rockchip: Add early ADC button detect for RGxx3

Elon Zhang (1):
  board: rockchip: add Rockchip Toybrick TB-RK3588X board

Jonas Karlman (42):
  rockchip: rk3328: Update default u-boot, spl-boot-order prop
  rockchip: rk3328-evb: Update defconfig
  rockchip: rk3328-rock64: Update defconfig
  rockchip: rk3328-roc-cc: Update defconfig
  rockchip: rk3328-rock-pi-e: Update defconfig
  rockchip: rk3328-nanopi-r2: Update defconfig
  rockchip: rk3328-orangepi-r1-plus: Update defconfig
  rockchip: rk3328: Fix loading FIT from SD-card when booting from eMMC
  gpio: rockchip: Use gpio alias id as gpio bank id
  rng: rockchip: Use same compatible as linux
  rockchip: rk3328: Sync device tree from linux v6.8-rc1
  Revert "rockchip: Allow booting from SPI"
  rockchip: rk3328: Add support to build bootable SPI image
  rockchip: rk3328-rock64: Enable boot from SPI NOR flash
  rockchip: rk3328-orangepi-r1-plus: Enable boot from SPI NOR flash
  rockchip: spl: Enable caches to speed up checksum validation
  phy: rockchip-inno-usb2: Write to correct GRF
  phy: rockchip-inno-usb2: Limit changes made to regs
  rockchip: board: Add minimal generic RK3588S/RK3588 board
  board: rockchip: Add Pine64 PineTab2
  rockchip: Add common default bss and stack addresses
  rockchip: Use common bss and stack addresses on RK3308
  rockchip: Use common bss and stack addresses on RK3328
  rockchip: Use common bss and stack addresses on RK3399
  rockchip: Use common bss and stack addresses on RK356x
  rockchip: Use common bss and stack addresses on RK3588
  rockchip: Update the default USB Product ID value
  rockchip: board: Prepare for use of DM_USB_GADGET with DWC2_OTG
  rockchip: board: Use a common USB Product ID for UMS
  rockchip: Migrate to use DM_USB_GADGET on RK3328
  board: rockchip: rk3399: Add device tree files to MAINTAINERS
  board: rockchip: rk3399: Add myself as reviewer to MAINTAINERS
  board: rockchip: rk3399: Remove unused board_early_init_f functions
  board: rockchip: Add a common ROCK Pi 4 target
  rockchip: io-domain: Add support for RK3399
  rockchip: pine64: rockpro64: Migrate to use IO-domain driver
  rockchip: pine64: pinebook-pro: Migrate to use IO-domain driver
  rockchip: pine64: pinephone-pro: Migrate to use IO-domain driver
  rockchip: vamrs: rock960: Migrate to use IO-domain driver
  rockchip: theobroma-systems: puma: Migrate to use IO-domain driver
  rockchip: google: gru: Migrate to use IO-domain driver
  rockchip: board: Move gpt_capsule_update_setup() call

Quentin Schulz (35):
  rockchip: avoid out-of-bounds when computing cpuid
  rockchip: add weak function symbol called at the beginning of misc_init_r
  rockchip: google: gru: migrate to rockchip_early_misc_init_r
  rockchip: pine64: pinebook-pro: migrate to rockchip_early_misc_init_r
  rockchip: pine64: pinephone-pro: migrate to rockchip_early_misc_init_r
  rockchip: pine64: rockpro64: migrate to rockchip_early_misc_init_r
  rockchi

RE: [PATCH] efi_loader: accept append write with valid size and data

2024-03-14 Thread kojima.masahisa
Hi Ilias,

> -Original Message-
> From: Ilias Apalodimas 
> Sent: Thursday, March 14, 2024 10:54 PM
> To: Kojima, Masahisa/小島 雅久 
> Cc: u-boot@lists.denx.de; Heinrich Schuchardt 
> Subject: Re: [PATCH] efi_loader: accept append write with valid size and data
> 
> Hi Kojima-san
> 
> Apologies for the late reply
> 
> On Mon, 4 Mar 2024 at 08:10, Masahisa Kojima
>  wrote:
> >
> > Current "variables" efi_selftest result is inconsistent
> > between the U-Boot file storage and the tee-based StandaloneMM
> > RPMB secure storage.
> > U-Boot file storage implementation does not accept SetVariale
> > call to non-existent variable with EFI_VARIABLE_APPEND_WRITE
> > attribute and valid size and data, however it is accepted
> > in EDK II StandaloneMM implementation.
> 
> Yes,
> 
> >
> > Since UEFI specification does not clearly describe the behavior
> > of the append write to non-existent variable, let's update
> > the U-Boot file storage implementation to get aligned with
> > the EDK II reference implementation.
> >
> > Signed-off-by: Masahisa Kojima 
> > ---
> >  lib/efi_loader/efi_variable.c | 13 +-
> >  lib/efi_selftest/efi_selftest_variables.c | 30
> +--
> >  2 files changed, 35 insertions(+), 8 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> > index 40f7a0fb10..1693c3387b 100644
> > --- a/lib/efi_loader/efi_variable.c
> > +++ b/lib/efi_loader/efi_variable.c
> > @@ -282,11 +282,8 @@ efi_status_t efi_set_variable_int(const u16
> *variable_name,
> > }
> > time = var->time;
> > } else {
> > -   if (delete || append)
> > -   /*
> > -* Trying to delete or to update a non-existent
> > -* variable.
> > -*/
> > +   if (delete)
> > +   /* Trying to delete a non-existent variable. */
> > return EFI_NOT_FOUND;
> 
> I am not sure about this tbh. The UEFI spec says
> "When the EFI_VARIABLE_APPEND_WRITE attribute is set, then a
> SetVariable() call with a DataSize of zero will not cause any change
> to the variable value (the timestamp
> associated with the variable may be updated however, even if no new
> data value is provided;see the description
> of the EFI_VARIABLE_AUTHENTICATION_2 descriptor below). In this case
> the DataSize will not be zero
> since the EFI_VARIABLE_AUTHENTICATION_2 descriptor will be populated)"
> 
> I think this assumes the variable exists. Is there a different chapter
> in the spec that describes the behavior?

No, I could not find it.
I'm also not sure what the UEFI spec expects.

> 
> > }
> >
> > @@ -329,7 +326,11 @@ efi_status_t efi_set_variable_int(const u16
> *variable_name,
> > /* EFI_NOT_FOUND has been handled before */
> > attributes = var->attr;
> > ret = EFI_SUCCESS;
> > -   } else if (append) {
> > +   } else if (append && var) {
> > +   /*
> > +* data is appended if EFI_VARIABLE_APPEND_WRITE is
> set and
> > +* variable exists.
> > +*/
> > u16 *old_data = var->name;
> >
> > for (; *old_data; ++old_data)
> > diff --git a/lib/efi_selftest/efi_selftest_variables.c
> b/lib/efi_selftest/efi_selftest_variables.c
> > index c7a3fdbaa6..0c2c76599e 100644
> > --- a/lib/efi_selftest/efi_selftest_variables.c
> > +++ b/lib/efi_selftest/efi_selftest_variables.c
> > @@ -131,13 +131,39 @@ static int execute(void)
> > (unsigned int)len);
> > if (memcmp(data, v, len))
> > efi_st_todo("GetVariable returned wrong value\n");
> > -   /* Append variable 2 */
> > +   /* Append variable 2, write to non-existent variable */
> > ret = runtime->set_variable(u"efi_none", &guid_vendor1,
> >
> EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > EFI_VARIABLE_APPEND_WRITE,
> > 15, v);
> > +   if (ret != EFI_SUCCESS) {
> > +   efi_st_error("SetVariable(APPEND_WRITE) with valid size
> and data to non-existent variable must be succcessful\n");
> > +   return EFI_ST_FAILURE;
> > +   }
> > +   len = EFI_ST_MAX_DATA_SIZE;
> > +   ret = runtime->get_variable(u"efi_none", &guid_vendor1,
> > +   &attr, &len, data);
> > +   if (ret != EFI_SUCCESS) {
> > +   efi_st_error("GetVariable failed\n");
> > +   return EFI_ST_FAILURE;
> > +   }
> > +   if (len != 15)
> > +   efi_st_todo("GetVariable returned wrong length %u\n",
> > +   (unsigned int)len);
> > +   if (memcmp(data, v, len))
> > +   efi_st_todo("GetVariable returned wrong value\n");
> > +   /* Delete variable efi_none */
> > +   ret = runtime-

Re: [PATCH 2/3] rockchip: Add a board_gen_ethaddr() function

2024-03-14 Thread Jonas Karlman
Hi Detlev,

On 2024-03-14 15:43, Detlev Casanova wrote:
> Set the MAC address based on the CPU ID only if the ethernet device has
> no ROM or DT address set.

This patch changes behavior and once again require CONFIG_NET to fixup
the device tree with local-mac-address for the ethernet0/1 alias nodes.

I.e. reverts one of the intentions with the commit 628fb0683b65
("rockchip: misc: Set eth1addr mac address"), fixup of ethaddr in DT
should not depend on enabled and working ethernet in U-Boot.

Maybe just having a Kconfig to control if ENV ethaddr should overwrite
ROM ethaddr could as easily solve your issue?

Please also note that misc.c is merging into board.c in another pending
series, see custodian u-boot-rockchip/for-next branch.

> 
> Signed-off-by: Detlev Casanova 
> ---
>  arch/arm/Kconfig  |  1 +
>  arch/arm/include/asm/arch-rockchip/misc.h |  1 +
>  arch/arm/mach-rockchip/board.c| 30 ++-
>  arch/arm/mach-rockchip/misc.c | 22 -
>  4 files changed, 42 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 01d6556c42b..21b41675ef6 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -2003,6 +2003,7 @@ config ARCH_ROCKCHIP
>   select DM_SPI_FLASH
>   select DM_USB_GADGET if USB_DWC3_GADGET
>   select ENABLE_ARM_SOC_BOOT0_HOOK
> + select NET_BOARD_ETHADDR

You are selecting here, so why the need for IS_ENABLED checks below?

>   select OF_CONTROL
>   select MTD
>   select SPI
> diff --git a/arch/arm/include/asm/arch-rockchip/misc.h 
> b/arch/arm/include/asm/arch-rockchip/misc.h
> index 4155af8c3b0..6e972de6279 100644
> --- a/arch/arm/include/asm/arch-rockchip/misc.h
> +++ b/arch/arm/include/asm/arch-rockchip/misc.h
> @@ -10,5 +10,6 @@ int rockchip_cpuid_from_efuse(const u32 cpuid_offset,
> const u32 cpuid_length,
> u8 *cpuid);
>  int rockchip_cpuid_set(const u8 *cpuid, const u32 cpuid_length);
> +int rockchip_gen_macaddr(int dev_num, u8 *mac_addr);
>  int rockchip_setup_macaddr(void);
>  void rockchip_capsule_update_board_setup(void);
> diff --git a/arch/arm/mach-rockchip/board.c b/arch/arm/mach-rockchip/board.c
> index 2620530e03f..283d3b9ed3a 100644
> --- a/arch/arm/mach-rockchip/board.c
> +++ b/arch/arm/mach-rockchip/board.c
> @@ -296,8 +296,8 @@ int fastboot_set_reboot_flag(enum fastboot_reboot_reason 
> reason)
>  }
>  #endif
>  
> -#ifdef CONFIG_MISC_INIT_R
> -__weak int misc_init_r(void)
> +#if IS_ENABLED(CONFIG_MISC_INIT_R) || IS_ENABLED(CONFIG_NET_BOARD_ETHADDR)

NET_BOARD_ETHADDR is selected so this #if will always be true.

> +static int set_cpuid(void)
>  {
>   const u32 cpuid_offset = CFG_CPUID_OFFSET;
>   const u32 cpuid_length = 0x10;
> @@ -309,10 +309,6 @@ __weak int misc_init_r(void)
>   return ret;
>  
>   ret = rockchip_cpuid_set(cpuid, cpuid_length);
> - if (ret)
> - return ret;
> -
> - ret = rockchip_setup_macaddr();
>  
>   return ret;
>  }
> @@ -349,3 +345,25 @@ __weak int board_rng_seed(struct abuf *buf)
>   return 0;
>  }
>  #endif
> +
> +#if IS_ENABLED(CONFIG_MISC_INIT_R)
> +__weak int misc_init_r(void)
> +{
> + return set_cpuid();
> +}
> +#endif
> +
> +int board_gen_ethaddr(int dev_num, u8 *mac_addr)
> +{
> + if (!IS_ENABLED(CONFIG_NET_BOARD_ETHADDR))
> + return 0;

NET_BOARD_ETHADDR is selected so this if return 0 is dead code?

Regards,
Jonas

> +
> + if (!env_get("cpuid#")) {
> + int err = set_cpuid();
> +
> + if (err)
> + return err;
> + }
> +
> + return rockchip_gen_macaddr(dev_num, mac_addr);
> +}
> diff --git a/arch/arm/mach-rockchip/misc.c b/arch/arm/mach-rockchip/misc.c
> index 7d03f0c2b67..9c7b04ee5a8 100644
> --- a/arch/arm/mach-rockchip/misc.c
> +++ b/arch/arm/mach-rockchip/misc.c
> @@ -21,14 +21,13 @@
>  
>  #include 
>  
> -int rockchip_setup_macaddr(void)
> +int rockchip_gen_macaddr(int dev_num, u8 *mac_addr)
>  {
>  #if CONFIG_IS_ENABLED(HASH) && CONFIG_IS_ENABLED(SHA256)
>   int ret;
>   const char *cpuid = env_get("cpuid#");
>   u8 hash[SHA256_SUM_LEN];
>   int size = sizeof(hash);
> - u8 mac_addr[6];
>  
>   /* Only generate a MAC address, if none is set in the environment */
>   if (env_get("ethaddr"))
> @@ -51,15 +50,26 @@ int rockchip_setup_macaddr(void)
>   /* Make this a valid MAC address and set it */
>   mac_addr[0] &= 0xfe;  /* clear multicast bit */
>   mac_addr[0] |= 0x02;  /* set local assignment bit (IEEE802) */
> - eth_env_set_enetaddr("ethaddr", mac_addr);
>  
> - /* Make a valid MAC address for ethernet1 */
> - mac_addr[5] ^= 0x01;
> - eth_env_set_enetaddr("eth1addr", mac_addr);
> + /* Make a valid MAC address for the given device number */
> + mac_addr[5] ^= dev_num;
>  #endif
>   return 0;
>  }
>  
> +int rockchip_setup_macaddr(void)
> +{

Re: [PATCH 2/3] rockchip: Add a board_gen_ethaddr() function

2024-03-14 Thread Marek Vasut

On 3/14/24 3:43 PM, Detlev Casanova wrote:

Set the MAC address based on the CPU ID only if the ethernet device has
no ROM or DT address set.

Signed-off-by: Detlev Casanova 


More of a general design question -- how much of this can be done in MAC 
driver specific .read_rom_hwaddr callback ?


Re: [PATCH 1/3] net: Add a CONFIG_NET_BOARD_ETHADDR

2024-03-14 Thread Marek Vasut

On 3/14/24 3:43 PM, Detlev Casanova wrote:

On some boards, a MAC address is set based on the CPU ID or other
information. This is usually done in the misc_init_r() function.

This becomes a problem for net devices that are probed after the call to
misc_init_r(), for example, when the ethernet is on a PCI port, which
needs to be enumerated.

In this case, misc_init_r() will set the ethaddr variable, then, when
the ethernet device is probed, if it has a ROM address, u-boot will warn
about a MAC address mismatch and use the misc_init_r() address instead
of the one in ROM.

The operating system later will most likely use the ROM MAC address,
which can be confusing.

To avoid that, this commit introduces a CONFIG_NET_BOARD_ETHADDR that
allows board files to implement a function to set an ethaddr in the
environment, that will only be called when necessary. The logic is now:

- If there is there an ethaddr env var, use it.
- If not, if there is a DT MAC address, use it.
- If not, if there is a ROM MAC address, use it.
- If not, if CONFIG_NET_BOARD_ETHADDR, call board_gen_ethaddr() and use
   it.
- If not, if CONFIG_NET_RANDOM_ETHADDR, generate random MAC
- If not, fail with No valid MAC address found

Signed-off-by: Detlev Casanova 
---
  net/Kconfig  |  7 +++
  net/eth-uclass.c | 17 +++--
  2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/net/Kconfig b/net/Kconfig
index 5dff6336293..6dd333ddb9e 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -54,6 +54,13 @@ config NET_RANDOM_ETHADDR
  generated. It will be saved to the appropriate environment variable,
  too.
  
+config NET_BOARD_ETHADDR

+   bool "Board specific ethaddr if unset"
+   help
+ Allow a board function to set a specific Ethernet address that can
+ be, e.g., based on the CPU ID. If set, this will be tried before
+ setting a random address (if set).
+
  config NETCONSOLE
bool "NetConsole support"
help
diff --git a/net/eth-uclass.c b/net/eth-uclass.c
index 3d0ec91dfa4..f194df8512a 100644
--- a/net/eth-uclass.c
+++ b/net/eth-uclass.c
@@ -56,6 +56,12 @@ __weak int board_interface_eth_init(struct udevice *dev,
return 0;
  }
  
+/* board-specific MAC Address generation. */

+__weak int board_gen_ethaddr(int dev_num, u8 *mac_addr)
+{
+   return 0;
+}
+
  static struct eth_uclass_priv *eth_get_uclass_priv(void)
  {
struct uclass *uc;
@@ -563,13 +569,20 @@ static int eth_post_probe(struct udevice *dev)
if (!eth_dev_get_mac_address(dev, pdata->enetaddr) ||
!is_valid_ethaddr(pdata->enetaddr)) {
/* Check if the device has a MAC address in ROM */
+   int ret = -1;
if (eth_get_ops(dev)->read_rom_hwaddr) {
-   int ret;
-
ret = eth_get_ops(dev)->read_rom_hwaddr(dev);
if (!ret)
source = "ROM";
}
+   if (IS_ENABLED(CONFIG_NET_BOARD_ETHADDR) && ret) {
+   board_gen_ethaddr(dev_seq(dev), pdata->enetaddr);


You do need to pass in the udevice *dev directly, otherwise this DM 
uclass patch would behave like non-DM code.


I'd personally just create a __weak board_gen_ethaddr() function here 
and let boards override it, without new Kconfig symbol.


Re: [PATCH] imx8m*_venice: move venice to OF_UPSTREAM

2024-03-14 Thread Tim Harvey
On Thu, Mar 14, 2024 at 12:50 AM Sumit Garg  wrote:
>
> + Tom
>
> Hi Tim,
>
> On Wed, 13 Mar 2024 at 22:01, Tim Harvey  wrote:
> >
> > On Wed, Mar 13, 2024 at 6:20 AM Sumit Garg  wrote:
> > >
> > > On Wed, 13 Mar 2024 at 06:46, Fabio Estevam  wrote:
> > > >
> > > > Hi Tim,
> > > >
> > > > On Tue, Mar 12, 2024 at 4:05 PM Tim Harvey  
> > > > wrote:
> > > > >
> > > > > Move to imx8m{m,n,p}-venice to OF_UPSTREAM:
> > > > >  - replace the non-upstream generic imx8m{m,n,p}-venice dt with one 
> > > > > of the
> > > > >dt's from the OF_LIST
> > > > >  - handle the fact that dtbs now have a 'freescale/' prefix
> > > > >  - imply OF_UPSTREAM
> > > > >  - remove rudundant files from arch/arm/dts leaving only the
> > > > >*-u-boot.dtsi files
> > > > >
> > > > > Signed-off-by: Tim Harvey 
> > > > ...
> > > > >  33 files changed, 13 insertions(+), 10658 deletions(-)
> > > >
> > > > This diff looks great :-)
> > >
> > > +1
> > >
> > > Reviewed-by: Sumit Garg 
> > >
> >
> > Hi Sumit,
> >
> > Thanks for your work on this - I imagine over time this will
> > de-duplicate a lot of work!
> >
> > I have a couple of questions about OF_UPSTREAM I haven't found the
> > answer to yet:
> > 1. how do you determine what the last sync point was in dts/upstream?
> > (ie what kernel version is it currently sync'd with)
>
> You can get that information from merge commits that happen over
> dts/upstream sub-directory. It was somewhat raw information for the
> first time we added dts/upstream as (commit aaba2d45dc2a pointing to
> v6.7-dts [1]):
>
> commit 53633a893a06bd5a0c807287d9cc29337806eaf7
> Author: Tom Rini 
> Date:   Thu Feb 29 12:33:36 2024 -0500
>
> Squashed 'dts/upstream/' content from commit aaba2d45dc2a
>
> git-subtree-dir: dts/upstream
> git-subtree-split: aaba2d45dc2a1b3bbb710f2a3808ee1c9f340abe
>
> [1] 
> https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git/tag/?h=v6.7-dts
>
> However, further syncs would be more clear from merge commit
> description, try following:
>
> $ ./dts/update-dts-subtree.sh pull v6.8-dts
>
> You will see merge commit saying:
>
> commit 57508745cd2b07e55b5c414c6d646de4865418d2 (HEAD -> dt_uprev)
> Merge: 3987e15e88a 14c9e8c0856
> Author: Sumit Garg 
> Date:   Thu Mar 14 12:20:27 2024 +0530
>
> Subtree merge tag 'v6.8-dts' of devicetree-rebasing repo [1] into
> dts/upstream
>
> [1] 
> https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git/
>
> > 2. how often will dts/upstream get re-synced (not the
> > devicetree-rebasing.git but u-boot dts/upstream),
>
> You can see the re-syncing details here (section: "Resyncing with
> devicetree-rebasing",
> https://source.denx.de/u-boot/u-boot/-/blob/next/doc/develop/devicetree/control.rst?ref_type=heads).
>

Thanks - I was confused about this as I originally figured it was done
via a git submodule. When I looked at the commit history of
dts/upstream it wasn't clear at all because of the first mergebeing
done differently as you explain above. As long as
update-dts-subtree.sh is run with a clear named tag that has the
kernel version in it then it's easy to follow via the short commit
description.

> > who do you suspect
> > will do be doing it?
>
> Tom will be doing that. I suppose we are just in time for a resync
> since the Linux kernel v6.8 was released.
>

Ok, that makes sense. I assume Tom will be updating it on major kernel
releases and possibly rc's leading up to as well.

> Tom,
>
> Do you think we can make v6.8-dts sync for the U-Boot next?
>
> > 3. how would one go about adding a new feature via dt to uboot when
> > the same feature has not yet landed in dts upstream? perhaps the
> > answer is it must land upstream first or do you suspect it would be ok
> > to put something in a u-boot.dtsi that can later get removed as
> > redundant?
>
> From an upstream perspective, we should aim for minimal contents in
> *-u-boot.dtsi, ideally it would be better to get rid of them. For eg.
> all bootph_* properties should all be pushed upstream.
>
> However, from the new features perspective we can consider using
> *-u-boot.dtsi if the changes are minimal like enabling some DT nodes.
> But if the changes are significant then that board can just opt out of
> OF_USTREAM but still reusing all the DT includes. The reasoning behind
> this is to minimize any chances of breakage if DT features in
> *-u-boot.dtsi diverge from DT upstream.
>
> > I ask mainly for being able to add things quickly to a
> > downstream U-Boot repo that lags behind upstream U-Boot
> >

The specific case I imagine is for example I have a couple of boards
that have TPM's that I have submitted dts updates to enable which did
not make the 6.8 cutoff so will be in 6.9. Meanwhile I will want to
pull them manually to our downstream vendor fork of an earlier U-Boot
(which in the future will have OF_UPSTEAM for our boards). Now that I
see this doesn't use git submodules I can just alter the files as
needed if I'm in a dow

[PATCH 3/3] net: eth-uclass: Add driver source possibility

2024-03-14 Thread Detlev Casanova
Some net driver, like rtl8169, can set/get the MAC address from the
registers and store it in pdata->enetaddr.

When that happens, if there is a mismatch with the environment MAC
address, u-boot will show that the MAC address source is DT. This patch
ensures that the shown source is "driver" instead to avoid confusion.

Signed-off-by: Detlev Casanova 
---
 net/eth-uclass.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/eth-uclass.c b/net/eth-uclass.c
index f194df8512a..6e521955fa5 100644
--- a/net/eth-uclass.c
+++ b/net/eth-uclass.c
@@ -565,9 +565,13 @@ static int eth_post_probe(struct udevice *dev)
priv->state = ETH_STATE_INIT;
priv->running = false;
 
+   /* Check if the driver has already set a valid MAC address */
+   if (is_valid_ethaddr(pdata->enetaddr)) {
+   source = "driver";
+   }
/* Check if the device has a valid MAC address in device tree */
-   if (!eth_dev_get_mac_address(dev, pdata->enetaddr) ||
-   !is_valid_ethaddr(pdata->enetaddr)) {
+   else if (!eth_dev_get_mac_address(dev, pdata->enetaddr) ||
+!is_valid_ethaddr(pdata->enetaddr)) {
/* Check if the device has a MAC address in ROM */
int ret = -1;
if (eth_get_ops(dev)->read_rom_hwaddr) {
-- 
2.43.2



[PATCH 2/3] rockchip: Add a board_gen_ethaddr() function

2024-03-14 Thread Detlev Casanova
Set the MAC address based on the CPU ID only if the ethernet device has
no ROM or DT address set.

Signed-off-by: Detlev Casanova 
---
 arch/arm/Kconfig  |  1 +
 arch/arm/include/asm/arch-rockchip/misc.h |  1 +
 arch/arm/mach-rockchip/board.c| 30 ++-
 arch/arm/mach-rockchip/misc.c | 22 -
 4 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 01d6556c42b..21b41675ef6 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -2003,6 +2003,7 @@ config ARCH_ROCKCHIP
select DM_SPI_FLASH
select DM_USB_GADGET if USB_DWC3_GADGET
select ENABLE_ARM_SOC_BOOT0_HOOK
+   select NET_BOARD_ETHADDR
select OF_CONTROL
select MTD
select SPI
diff --git a/arch/arm/include/asm/arch-rockchip/misc.h 
b/arch/arm/include/asm/arch-rockchip/misc.h
index 4155af8c3b0..6e972de6279 100644
--- a/arch/arm/include/asm/arch-rockchip/misc.h
+++ b/arch/arm/include/asm/arch-rockchip/misc.h
@@ -10,5 +10,6 @@ int rockchip_cpuid_from_efuse(const u32 cpuid_offset,
  const u32 cpuid_length,
  u8 *cpuid);
 int rockchip_cpuid_set(const u8 *cpuid, const u32 cpuid_length);
+int rockchip_gen_macaddr(int dev_num, u8 *mac_addr);
 int rockchip_setup_macaddr(void);
 void rockchip_capsule_update_board_setup(void);
diff --git a/arch/arm/mach-rockchip/board.c b/arch/arm/mach-rockchip/board.c
index 2620530e03f..283d3b9ed3a 100644
--- a/arch/arm/mach-rockchip/board.c
+++ b/arch/arm/mach-rockchip/board.c
@@ -296,8 +296,8 @@ int fastboot_set_reboot_flag(enum fastboot_reboot_reason 
reason)
 }
 #endif
 
-#ifdef CONFIG_MISC_INIT_R
-__weak int misc_init_r(void)
+#if IS_ENABLED(CONFIG_MISC_INIT_R) || IS_ENABLED(CONFIG_NET_BOARD_ETHADDR)
+static int set_cpuid(void)
 {
const u32 cpuid_offset = CFG_CPUID_OFFSET;
const u32 cpuid_length = 0x10;
@@ -309,10 +309,6 @@ __weak int misc_init_r(void)
return ret;
 
ret = rockchip_cpuid_set(cpuid, cpuid_length);
-   if (ret)
-   return ret;
-
-   ret = rockchip_setup_macaddr();
 
return ret;
 }
@@ -349,3 +345,25 @@ __weak int board_rng_seed(struct abuf *buf)
return 0;
 }
 #endif
+
+#if IS_ENABLED(CONFIG_MISC_INIT_R)
+__weak int misc_init_r(void)
+{
+   return set_cpuid();
+}
+#endif
+
+int board_gen_ethaddr(int dev_num, u8 *mac_addr)
+{
+   if (!IS_ENABLED(CONFIG_NET_BOARD_ETHADDR))
+   return 0;
+
+   if (!env_get("cpuid#")) {
+   int err = set_cpuid();
+
+   if (err)
+   return err;
+   }
+
+   return rockchip_gen_macaddr(dev_num, mac_addr);
+}
diff --git a/arch/arm/mach-rockchip/misc.c b/arch/arm/mach-rockchip/misc.c
index 7d03f0c2b67..9c7b04ee5a8 100644
--- a/arch/arm/mach-rockchip/misc.c
+++ b/arch/arm/mach-rockchip/misc.c
@@ -21,14 +21,13 @@
 
 #include 
 
-int rockchip_setup_macaddr(void)
+int rockchip_gen_macaddr(int dev_num, u8 *mac_addr)
 {
 #if CONFIG_IS_ENABLED(HASH) && CONFIG_IS_ENABLED(SHA256)
int ret;
const char *cpuid = env_get("cpuid#");
u8 hash[SHA256_SUM_LEN];
int size = sizeof(hash);
-   u8 mac_addr[6];
 
/* Only generate a MAC address, if none is set in the environment */
if (env_get("ethaddr"))
@@ -51,15 +50,26 @@ int rockchip_setup_macaddr(void)
/* Make this a valid MAC address and set it */
mac_addr[0] &= 0xfe;  /* clear multicast bit */
mac_addr[0] |= 0x02;  /* set local assignment bit (IEEE802) */
-   eth_env_set_enetaddr("ethaddr", mac_addr);
 
-   /* Make a valid MAC address for ethernet1 */
-   mac_addr[5] ^= 0x01;
-   eth_env_set_enetaddr("eth1addr", mac_addr);
+   /* Make a valid MAC address for the given device number */
+   mac_addr[5] ^= dev_num;
 #endif
return 0;
 }
 
+int rockchip_setup_macaddr(void)
+{
+   u8 mac_addr[6];
+
+   if (rockchip_gen_macaddr(0, mac_addr) == 0)
+   eth_env_set_enetaddr("ethaddr", mac_addr);
+
+   if (rockchip_gen_macaddr(1, mac_addr) == 0)
+   eth_env_set_enetaddr("eth1addr", mac_addr);
+
+   return 0;
+}
+
 int rockchip_cpuid_from_efuse(const u32 cpuid_offset,
  const u32 cpuid_length,
  u8 *cpuid)
-- 
2.43.2



[PATCH 1/3] net: Add a CONFIG_NET_BOARD_ETHADDR

2024-03-14 Thread Detlev Casanova
On some boards, a MAC address is set based on the CPU ID or other
information. This is usually done in the misc_init_r() function.

This becomes a problem for net devices that are probed after the call to
misc_init_r(), for example, when the ethernet is on a PCI port, which
needs to be enumerated.

In this case, misc_init_r() will set the ethaddr variable, then, when
the ethernet device is probed, if it has a ROM address, u-boot will warn
about a MAC address mismatch and use the misc_init_r() address instead
of the one in ROM.

The operating system later will most likely use the ROM MAC address,
which can be confusing.

To avoid that, this commit introduces a CONFIG_NET_BOARD_ETHADDR that
allows board files to implement a function to set an ethaddr in the
environment, that will only be called when necessary. The logic is now:

- If there is there an ethaddr env var, use it.
- If not, if there is a DT MAC address, use it.
- If not, if there is a ROM MAC address, use it.
- If not, if CONFIG_NET_BOARD_ETHADDR, call board_gen_ethaddr() and use
  it.
- If not, if CONFIG_NET_RANDOM_ETHADDR, generate random MAC
- If not, fail with No valid MAC address found

Signed-off-by: Detlev Casanova 
---
 net/Kconfig  |  7 +++
 net/eth-uclass.c | 17 +++--
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/net/Kconfig b/net/Kconfig
index 5dff6336293..6dd333ddb9e 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -54,6 +54,13 @@ config NET_RANDOM_ETHADDR
  generated. It will be saved to the appropriate environment variable,
  too.
 
+config NET_BOARD_ETHADDR
+   bool "Board specific ethaddr if unset"
+   help
+ Allow a board function to set a specific Ethernet address that can
+ be, e.g., based on the CPU ID. If set, this will be tried before
+ setting a random address (if set).
+
 config NETCONSOLE
bool "NetConsole support"
help
diff --git a/net/eth-uclass.c b/net/eth-uclass.c
index 3d0ec91dfa4..f194df8512a 100644
--- a/net/eth-uclass.c
+++ b/net/eth-uclass.c
@@ -56,6 +56,12 @@ __weak int board_interface_eth_init(struct udevice *dev,
return 0;
 }
 
+/* board-specific MAC Address generation. */
+__weak int board_gen_ethaddr(int dev_num, u8 *mac_addr)
+{
+   return 0;
+}
+
 static struct eth_uclass_priv *eth_get_uclass_priv(void)
 {
struct uclass *uc;
@@ -563,13 +569,20 @@ static int eth_post_probe(struct udevice *dev)
if (!eth_dev_get_mac_address(dev, pdata->enetaddr) ||
!is_valid_ethaddr(pdata->enetaddr)) {
/* Check if the device has a MAC address in ROM */
+   int ret = -1;
if (eth_get_ops(dev)->read_rom_hwaddr) {
-   int ret;
-
ret = eth_get_ops(dev)->read_rom_hwaddr(dev);
if (!ret)
source = "ROM";
}
+   if (IS_ENABLED(CONFIG_NET_BOARD_ETHADDR) && ret) {
+   board_gen_ethaddr(dev_seq(dev), pdata->enetaddr);
+
+   if (!is_zero_ethaddr(pdata->enetaddr) &&
+   is_valid_ethaddr(pdata->enetaddr)) {
+   source = "board";
+   }
+   }
}
 
eth_env_get_enetaddr_by_index("eth", dev_seq(dev), env_enetaddr);
-- 
2.43.2



[PATCH 0/3] net: Add a CONFIG_NET_BOARD_ETHADDR

2024-03-14 Thread Detlev Casanova
Basically, the MAC address generation needs to use a board function in
its decision algorithm so that there is no MAC mismatch between ROM and
environment depending on the order of function calling.

See the first commit message for details.

Detlev Casanova (3):
  net: Add a CONFIG_NET_BOARD_ETHADDR
  rockchip: Add a board_gen_ethaddr() function
  net: eth-uclass: Add driver source possibility

 arch/arm/Kconfig  |  1 +
 arch/arm/include/asm/arch-rockchip/misc.h |  1 +
 arch/arm/mach-rockchip/board.c| 30 ++-
 arch/arm/mach-rockchip/misc.c | 22 -
 net/Kconfig   |  7 ++
 net/eth-uclass.c  | 25 ---
 6 files changed, 70 insertions(+), 16 deletions(-)

-- 
2.43.2



Re: [PATCH v6] remoteproc: uclass: Add methods to load firmware to rproc and boot rproc

2024-03-14 Thread Anwar, Md Danish



On 3/14/2024 6:16 PM, Tom Rini wrote:
> On Tue, Mar 12, 2024 at 02:02:08PM +0530, MD Danish Anwar wrote:
>>
>>
>> On 11/03/24 10:34 am, Anwar, Md Danish wrote:
>>>
>>>
>>> On 3/7/2024 6:16 PM, Tom Rini wrote:
 On Wed, Feb 28, 2024 at 05:36:45PM +0530, MD Danish Anwar wrote:
> Add APIs to set a firmware_name to a rproc and boot the rproc with the

> same firmware.
>
> Clients can call rproc_set_firmware() API to set firmware_name for a rproc
> whereas rproc_boot() will load the firmware set by rproc_set_firmware() to
> a buffer by calling request_firmware_into_buf(). rproc_boot() will then
> load the firmware file to the remote processor and start the remote
> processor.
>
> Also include "fs-loader.h" and make remoteproc driver select FS_LOADER in
> Kconfig so that we can call request_firmware_into_buf() from remoteproc
> driver.
>
> Signed-off-by: MD Danish Anwar 
> Acked-by: Ravi Gunasekaran 
> Reviewed-by: Roger Quadros 

 This breaks building on am64x_evm_r5 am65x_evm_r5_usbdfu
 am65x_evm_r5_usbmsc in next currently, thanks.

>>> I will work on fixing this build error and re-spin the patch.
>>>
>>
>> Hi Tom, Roger,
>>
>> This patch adds "request_firmware_into_buf()" in the rproc driver. To
>> use this API, FS_LOADER is needed. So I am adding "select FS_LOADER" in
>> REMOTEPROC Kconfig option. As a result whenever REMOTEPROC is enabled,
>> FS_LOADER also gets enabled.
>>
>> Now arch/arm/mach-k3/common.c [1] and arch/arm/mach-omap2/boot-common.c
>> [2] has a "load_firmware()" API which calls fs-loader APIs and they have
>> below if condition before calling fs-loader APIs.
>>
>> if (!IS_ENABLED(CONFIG_FS_LOADER))
>>  return 0;
>>
>> Till now, CONFIG_FS_LOADER was not set as a result the load_firmware()
>> API in above mentioned files, was returning 0.
>>
>> Now as this patch enables CONFIG_FS_LOADER, as a result the code after
>> the if check starts getting executed and it tries to look for
>> get_fs_loader() and other fs-loader APIs but this is done at SPL and at
>> this time FS_LOADER is not built yet as a result we see below error.
>> The if checks only checks for CONFIG_FS_LOADER but not for
>> CONFIG_SPL_FS_LOADER.
>>
>>   AR  spl/boot/built-in.o
>>   LD  spl/u-boot-spl
>> arm-none-linux-gnueabihf-ld.bfd: arch/arm/mach-k3/common.o: in function
>> `load_firmware':
>> /home/danish/workspace/u-boot/arch/arm/mach-k3/common.c:184: undefined
>> reference to `get_fs_loader'
>> arm-none-linux-gnueabihf-ld.bfd:
>> /home/danish/workspace/u-boot/arch/arm/mach-k3/common.c:185: undefined
>> reference to `request_firmware_into_buf'
>> make[2]: *** [/home/danish/workspace/u-boot/scripts/Makefile.spl:527:
>> spl/u-boot-spl] Error 1
>> make[1]: *** [/home/danish/workspace/u-boot/Makefile:2055:
>> spl/u-boot-spl] Error 2
>> make[1]: Leaving directory '/home/danish/uboot_images/am64x/r5'
>> make: *** [Makefile:177: sub-make] Error 2
>>
>> This bug has always been there but as CONFIG_FS_LOADER was never
>> enabled, this build error was never seen as the load_firmware() API will
>> return 0 without calling fs-loader APIs.
>>
>> Now that this patch enables CONFIG_FS_LOADER, the bug gets exposed and
>> build error is seen.
>>
>> My opinion here would be, to check for CONFIG_IS_ENABLED(FS_LOADER)
>> instead of IS_ENABLED(CONFIG_FS_LOADER) as the former will check for the
>> appropriate config option (CONFIG_SPL_FS_LOADER / CONFIG_FS_LOADER)
>> based on the build stage.
>>
>> I tested with the below diff and I don't see build errors with
>> am64x_evm_r5, am65x_evm_r5_usbdfu, am65x_evm_r5_usbmsc configs.
>>
>> diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c
>> index f411366778..6792ff7467 100644
>> --- a/arch/arm/mach-k3/common.c
>> +++ b/arch/arm/mach-k3/common.c
>> @@ -162,7 +162,7 @@ int load_firmware(char *name_fw, char
>> *name_loadaddr, u32 *loadaddr)
>> char *name = NULL;
>> int size = 0;
>>
>> -   if (!IS_ENABLED(CONFIG_FS_LOADER))
>> +   if (!CONFIG_IS_ENABLED(FS_LOADER))
>> return 0;
>>
>> *loadaddr = 0;
>> diff --git a/arch/arm/mach-omap2/boot-common.c
>> b/arch/arm/mach-omap2/boot-common.c
>> index 57917da25c..aa0ab13d5f 100644
>> --- a/arch/arm/mach-omap2/boot-common.c
>> +++ b/arch/arm/mach-omap2/boot-common.c
>> @@ -190,7 +190,7 @@ int load_firmware(char *name_fw, u32 *loadaddr)
>> struct udevice *fsdev;
>> int size = 0;
>>
>> -   if (!IS_ENABLED(CONFIG_FS_LOADER))
>> +   if (!CONFIG_IS_ENABLED(FS_LOADER))
>> return 0;
>>
>> if (!*loadaddr)
>>
>>
>> Tom, Roger, Please let me know if this looks ok.
>> If it's ok, I will post this diff as a separate patch and once that is
>> merged Tom can merge this patch or I can send a v7 if needed.
> 
> Yes, this seems like the right path, thanks.
> 
Thanks Tom. Posted this diff as patch
https://lore.kernel.org/all/20240314143311.259568-1-danishan...@ti.c

[PATCH] arm: mach-k3: Fix config check for FS_LOADER

2024-03-14 Thread MD Danish Anwar
load_firmware() API calls fs-loader APIs and checks for CONFIG_FS_LOADER
before calling those APIs. The if check only checks for CONFIG_FS_LOADER
but not for CONFIG_SPL_FS_LOADER.

When CONFIG_FS_LOADER is enabled, load_firmware() API calls fs-loader APIs
but this is done at SPL stage and at this time FS_LOADER is not built yet
as a result we see below build error.

  AR  spl/boot/built-in.o
  LD  spl/u-boot-spl
arm-none-linux-gnueabihf-ld.bfd: arch/arm/mach-k3/common.o: in function
`load_firmware':
/home/danish/workspace/u-boot/arch/arm/mach-k3/common.c:184: undefined
reference to `get_fs_loader'
arm-none-linux-gnueabihf-ld.bfd:
/home/danish/workspace/u-boot/arch/arm/mach-k3/common.c:185: undefined
reference to `request_firmware_into_buf'
make[2]: *** [/home/danish/workspace/u-boot/scripts/Makefile.spl:527:
spl/u-boot-spl] Error 1
make[1]: *** [/home/danish/workspace/u-boot/Makefile:2055:
spl/u-boot-spl] Error 2
make[1]: Leaving directory '/home/danish/uboot_images/am64x/r5'
make: *** [Makefile:177: sub-make] Error 2

Fix this by modifying the if check to CONFIG_IS_ENABLED(FS_LOADER) instead
of IS_ENABLED(CONFIG_FS_LOADER) as the former will check for the
appropriate config option (CONFIG_SPL_FS_LOADER / CONFIG_FS_LOADER) based
on the build stage.

Signed-off-by: MD Danish Anwar 
---
 arch/arm/mach-k3/r5/common.c  | 2 +-
 arch/arm/mach-omap2/boot-common.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-k3/r5/common.c b/arch/arm/mach-k3/r5/common.c
index 7309573a3f..c02f8d3309 100644
--- a/arch/arm/mach-k3/r5/common.c
+++ b/arch/arm/mach-k3/r5/common.c
@@ -70,7 +70,7 @@ int load_firmware(char *name_fw, char *name_loadaddr, u32 
*loadaddr)
char *name = NULL;
int size = 0;
 
-   if (!IS_ENABLED(CONFIG_FS_LOADER))
+   if (!CONFIG_IS_ENABLED(FS_LOADER))
return 0;
 
*loadaddr = 0;
diff --git a/arch/arm/mach-omap2/boot-common.c 
b/arch/arm/mach-omap2/boot-common.c
index 57917da25c..aa0ab13d5f 100644
--- a/arch/arm/mach-omap2/boot-common.c
+++ b/arch/arm/mach-omap2/boot-common.c
@@ -190,7 +190,7 @@ int load_firmware(char *name_fw, u32 *loadaddr)
struct udevice *fsdev;
int size = 0;
 
-   if (!IS_ENABLED(CONFIG_FS_LOADER))
+   if (!CONFIG_IS_ENABLED(FS_LOADER))
return 0;
 
if (!*loadaddr)
-- 
2.34.1



[PATCH] arm: mach-k3: Fix config check for FS_LOADER

2024-03-14 Thread MD Danish Anwar
load_firmware() API calls fs-loader APIs and checks for CONFIG_FS_LOADER
before calling those APIs. The if check only checks for CONFIG_FS_LOADER
but not for CONFIG_SPL_FS_LOADER.

When CONFIG_FS_LOADER is enabled, load_firmware() API calls fs-loader APIs
but this is done at SPL stage and at this time FS_LOADER is not built yet
as a result we see below build error.

  AR  spl/boot/built-in.o
  LD  spl/u-boot-spl
arm-none-linux-gnueabihf-ld.bfd: arch/arm/mach-k3/common.o: in function
`load_firmware':
/home/danish/workspace/u-boot/arch/arm/mach-k3/common.c:184: undefined
reference to `get_fs_loader'
arm-none-linux-gnueabihf-ld.bfd:
/home/danish/workspace/u-boot/arch/arm/mach-k3/common.c:185: undefined
reference to `request_firmware_into_buf'
make[2]: *** [/home/danish/workspace/u-boot/scripts/Makefile.spl:527:
spl/u-boot-spl] Error 1
make[1]: *** [/home/danish/workspace/u-boot/Makefile:2055:
spl/u-boot-spl] Error 2
make[1]: Leaving directory '/home/danish/uboot_images/am64x/r5'
make: *** [Makefile:177: sub-make] Error 2

Fix this by modifying the if check to CONFIG_IS_ENABLED(FS_LOADER) instead
of IS_ENABLED(CONFIG_FS_LOADER) as the former will check for the
appropriate config option (CONFIG_SPL_FS_LOADER / CONFIG_FS_LOADER) based
on the build stage.

Signed-off-by: MD Danish Anwar 
---
 arch/arm/mach-k3/r5/common.c  | 2 +-
 arch/arm/mach-omap2/boot-common.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-k3/r5/common.c b/arch/arm/mach-k3/r5/common.c
index 7309573a3f..c02f8d3309 100644
--- a/arch/arm/mach-k3/r5/common.c
+++ b/arch/arm/mach-k3/r5/common.c
@@ -70,7 +70,7 @@ int load_firmware(char *name_fw, char *name_loadaddr, u32 
*loadaddr)
char *name = NULL;
int size = 0;
 
-   if (!IS_ENABLED(CONFIG_FS_LOADER))
+   if (!CONFIG_IS_ENABLED(FS_LOADER))
return 0;
 
*loadaddr = 0;
diff --git a/arch/arm/mach-omap2/boot-common.c 
b/arch/arm/mach-omap2/boot-common.c
index 57917da25c..aa0ab13d5f 100644
--- a/arch/arm/mach-omap2/boot-common.c
+++ b/arch/arm/mach-omap2/boot-common.c
@@ -190,7 +190,7 @@ int load_firmware(char *name_fw, u32 *loadaddr)
struct udevice *fsdev;
int size = 0;
 
-   if (!IS_ENABLED(CONFIG_FS_LOADER))
+   if (!CONFIG_IS_ENABLED(FS_LOADER))
return 0;
 
if (!*loadaddr)
-- 
2.34.1



Re: [PATCH v4 00/13] Introduce basic support for TI's AM62Px SoC family

2024-03-14 Thread Tom Rini
On Tue, 12 Mar 2024 15:20:18 -0500, Bryan Brattlof wrote:

> The AM62Px is an extension of the existing Sitara AM62x low-cost family
> of application processors built for Automotive and Linux Application
> development. Scalable Arm Cortex-A53 performance and embedded features,
> such as: multi high-definition display support, 3D-graphics
> acceleration, 4K video acceleration, and extensive peripherals make the
> AM62Px well-suited for a broad range of automation and industrial
> application, including automotive digital instrumentation, automotive
> displays, industrial HMI, and more.
> 
> [...]

Applied to u-boot/master, thanks!

-- 
Tom




Re: [PATCH] efi_loader: accept append write with valid size and data

2024-03-14 Thread Ilias Apalodimas
Hi Kojima-san

Apologies for the late reply

On Mon, 4 Mar 2024 at 08:10, Masahisa Kojima
 wrote:
>
> Current "variables" efi_selftest result is inconsistent
> between the U-Boot file storage and the tee-based StandaloneMM
> RPMB secure storage.
> U-Boot file storage implementation does not accept SetVariale
> call to non-existent variable with EFI_VARIABLE_APPEND_WRITE
> attribute and valid size and data, however it is accepted
> in EDK II StandaloneMM implementation.

Yes,

>
> Since UEFI specification does not clearly describe the behavior
> of the append write to non-existent variable, let's update
> the U-Boot file storage implementation to get aligned with
> the EDK II reference implementation.
>
> Signed-off-by: Masahisa Kojima 
> ---
>  lib/efi_loader/efi_variable.c | 13 +-
>  lib/efi_selftest/efi_selftest_variables.c | 30 +--
>  2 files changed, 35 insertions(+), 8 deletions(-)
>
> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index 40f7a0fb10..1693c3387b 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -282,11 +282,8 @@ efi_status_t efi_set_variable_int(const u16 
> *variable_name,
> }
> time = var->time;
> } else {
> -   if (delete || append)
> -   /*
> -* Trying to delete or to update a non-existent
> -* variable.
> -*/
> +   if (delete)
> +   /* Trying to delete a non-existent variable. */
> return EFI_NOT_FOUND;

I am not sure about this tbh. The UEFI spec says
"When the EFI_VARIABLE_APPEND_WRITE attribute is set, then a
SetVariable() call with a DataSize of zero will not cause any change
to the variable value (the timestamp
associated with the variable may be updated however, even if no new
data value is provided;see the description
of the EFI_VARIABLE_AUTHENTICATION_2 descriptor below). In this case
the DataSize will not be zero
since the EFI_VARIABLE_AUTHENTICATION_2 descriptor will be populated)"

I think this assumes the variable exists. Is there a different chapter
in the spec that describes the behavior?

> }
>
> @@ -329,7 +326,11 @@ efi_status_t efi_set_variable_int(const u16 
> *variable_name,
> /* EFI_NOT_FOUND has been handled before */
> attributes = var->attr;
> ret = EFI_SUCCESS;
> -   } else if (append) {
> +   } else if (append && var) {
> +   /*
> +* data is appended if EFI_VARIABLE_APPEND_WRITE is set and
> +* variable exists.
> +*/
> u16 *old_data = var->name;
>
> for (; *old_data; ++old_data)
> diff --git a/lib/efi_selftest/efi_selftest_variables.c 
> b/lib/efi_selftest/efi_selftest_variables.c
> index c7a3fdbaa6..0c2c76599e 100644
> --- a/lib/efi_selftest/efi_selftest_variables.c
> +++ b/lib/efi_selftest/efi_selftest_variables.c
> @@ -131,13 +131,39 @@ static int execute(void)
> (unsigned int)len);
> if (memcmp(data, v, len))
> efi_st_todo("GetVariable returned wrong value\n");
> -   /* Append variable 2 */
> +   /* Append variable 2, write to non-existent variable */
> ret = runtime->set_variable(u"efi_none", &guid_vendor1,
> EFI_VARIABLE_BOOTSERVICE_ACCESS |
> EFI_VARIABLE_APPEND_WRITE,
> 15, v);
> +   if (ret != EFI_SUCCESS) {
> +   efi_st_error("SetVariable(APPEND_WRITE) with valid size and 
> data to non-existent variable must be succcessful\n");
> +   return EFI_ST_FAILURE;
> +   }
> +   len = EFI_ST_MAX_DATA_SIZE;
> +   ret = runtime->get_variable(u"efi_none", &guid_vendor1,
> +   &attr, &len, data);
> +   if (ret != EFI_SUCCESS) {
> +   efi_st_error("GetVariable failed\n");
> +   return EFI_ST_FAILURE;
> +   }
> +   if (len != 15)
> +   efi_st_todo("GetVariable returned wrong length %u\n",
> +   (unsigned int)len);
> +   if (memcmp(data, v, len))
> +   efi_st_todo("GetVariable returned wrong value\n");
> +   /* Delete variable efi_none */
> +   ret = runtime->set_variable(u"efi_none", &guid_vendor1,
> +   0, 0, NULL);
> +   if (ret != EFI_SUCCESS) {
> +   efi_st_error("SetVariable failed\n");
> +   return EFI_ST_FAILURE;
> +   }
> +   len = EFI_ST_MAX_DATA_SIZE;
> +   ret = runtime->get_variable(u"efi_none", &guid_vendor1,
> +   &attr, &len, data);
> if (ret != EFI_NOT_FOUND) {
> -   efi_st_error("SetVariable(APPEND_WRITE) with size 0 to 
> non-exi

Re: [PATCH v2 2/2] rockchip: spl-boot-order: show DT path for missing device

2024-03-14 Thread Quentin Schulz

Hi Christopher,

On 3/14/24 12:57, Christopher Obbard wrote:

When debugging the SPL boot order, the node ID of a device which hasn't
been found is printed but it can be quite hard to relate that to the
specific devicetree node. To aid debugging, print the node path instead of
the cryptic node ID.

Original debug message:

 board_boot_order: could not map node @73c to a boot-device

With this patch applied this becomes e.g:

board_boot_order: could not map node /spi@ff1d/flash@0 to a boot-device

Reviewed-by: Dragan Simic 
Signed-off-by: Christopher Obbard 


Reviewed-by: Quentin Schulz 

Thanks,
Quentin


Re: [PATCH v6] remoteproc: uclass: Add methods to load firmware to rproc and boot rproc

2024-03-14 Thread Tom Rini
On Tue, Mar 12, 2024 at 02:02:08PM +0530, MD Danish Anwar wrote:
> 
> 
> On 11/03/24 10:34 am, Anwar, Md Danish wrote:
> > 
> > 
> > On 3/7/2024 6:16 PM, Tom Rini wrote:
> >> On Wed, Feb 28, 2024 at 05:36:45PM +0530, MD Danish Anwar wrote:
> >>> Add APIs to set a firmware_name to a rproc and boot the rproc with the
> >>
> >>> same firmware.
> >>>
> >>> Clients can call rproc_set_firmware() API to set firmware_name for a rproc
> >>> whereas rproc_boot() will load the firmware set by rproc_set_firmware() to
> >>> a buffer by calling request_firmware_into_buf(). rproc_boot() will then
> >>> load the firmware file to the remote processor and start the remote
> >>> processor.
> >>>
> >>> Also include "fs-loader.h" and make remoteproc driver select FS_LOADER in
> >>> Kconfig so that we can call request_firmware_into_buf() from remoteproc
> >>> driver.
> >>>
> >>> Signed-off-by: MD Danish Anwar 
> >>> Acked-by: Ravi Gunasekaran 
> >>> Reviewed-by: Roger Quadros 
> >>
> >> This breaks building on am64x_evm_r5 am65x_evm_r5_usbdfu
> >> am65x_evm_r5_usbmsc in next currently, thanks.
> >>
> > I will work on fixing this build error and re-spin the patch.
> > 
> 
> Hi Tom, Roger,
> 
> This patch adds "request_firmware_into_buf()" in the rproc driver. To
> use this API, FS_LOADER is needed. So I am adding "select FS_LOADER" in
> REMOTEPROC Kconfig option. As a result whenever REMOTEPROC is enabled,
> FS_LOADER also gets enabled.
> 
> Now arch/arm/mach-k3/common.c [1] and arch/arm/mach-omap2/boot-common.c
> [2] has a "load_firmware()" API which calls fs-loader APIs and they have
> below if condition before calling fs-loader APIs.
> 
> if (!IS_ENABLED(CONFIG_FS_LOADER))
>   return 0;
> 
> Till now, CONFIG_FS_LOADER was not set as a result the load_firmware()
> API in above mentioned files, was returning 0.
> 
> Now as this patch enables CONFIG_FS_LOADER, as a result the code after
> the if check starts getting executed and it tries to look for
> get_fs_loader() and other fs-loader APIs but this is done at SPL and at
> this time FS_LOADER is not built yet as a result we see below error.
> The if checks only checks for CONFIG_FS_LOADER but not for
> CONFIG_SPL_FS_LOADER.
> 
>   AR  spl/boot/built-in.o
>   LD  spl/u-boot-spl
> arm-none-linux-gnueabihf-ld.bfd: arch/arm/mach-k3/common.o: in function
> `load_firmware':
> /home/danish/workspace/u-boot/arch/arm/mach-k3/common.c:184: undefined
> reference to `get_fs_loader'
> arm-none-linux-gnueabihf-ld.bfd:
> /home/danish/workspace/u-boot/arch/arm/mach-k3/common.c:185: undefined
> reference to `request_firmware_into_buf'
> make[2]: *** [/home/danish/workspace/u-boot/scripts/Makefile.spl:527:
> spl/u-boot-spl] Error 1
> make[1]: *** [/home/danish/workspace/u-boot/Makefile:2055:
> spl/u-boot-spl] Error 2
> make[1]: Leaving directory '/home/danish/uboot_images/am64x/r5'
> make: *** [Makefile:177: sub-make] Error 2
> 
> This bug has always been there but as CONFIG_FS_LOADER was never
> enabled, this build error was never seen as the load_firmware() API will
> return 0 without calling fs-loader APIs.
> 
> Now that this patch enables CONFIG_FS_LOADER, the bug gets exposed and
> build error is seen.
> 
> My opinion here would be, to check for CONFIG_IS_ENABLED(FS_LOADER)
> instead of IS_ENABLED(CONFIG_FS_LOADER) as the former will check for the
> appropriate config option (CONFIG_SPL_FS_LOADER / CONFIG_FS_LOADER)
> based on the build stage.
> 
> I tested with the below diff and I don't see build errors with
> am64x_evm_r5, am65x_evm_r5_usbdfu, am65x_evm_r5_usbmsc configs.
> 
> diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c
> index f411366778..6792ff7467 100644
> --- a/arch/arm/mach-k3/common.c
> +++ b/arch/arm/mach-k3/common.c
> @@ -162,7 +162,7 @@ int load_firmware(char *name_fw, char
> *name_loadaddr, u32 *loadaddr)
> char *name = NULL;
> int size = 0;
> 
> -   if (!IS_ENABLED(CONFIG_FS_LOADER))
> +   if (!CONFIG_IS_ENABLED(FS_LOADER))
> return 0;
> 
> *loadaddr = 0;
> diff --git a/arch/arm/mach-omap2/boot-common.c
> b/arch/arm/mach-omap2/boot-common.c
> index 57917da25c..aa0ab13d5f 100644
> --- a/arch/arm/mach-omap2/boot-common.c
> +++ b/arch/arm/mach-omap2/boot-common.c
> @@ -190,7 +190,7 @@ int load_firmware(char *name_fw, u32 *loadaddr)
> struct udevice *fsdev;
> int size = 0;
> 
> -   if (!IS_ENABLED(CONFIG_FS_LOADER))
> +   if (!CONFIG_IS_ENABLED(FS_LOADER))
> return 0;
> 
> if (!*loadaddr)
> 
> 
> Tom, Roger, Please let me know if this looks ok.
> If it's ok, I will post this diff as a separate patch and once that is
> merged Tom can merge this patch or I can send a v7 if needed.

Yes, this seems like the right path, thanks.

-- 
Tom


signature.asc
Description: PGP signature


Re: Pull request efi-2024-04-rc5

2024-03-14 Thread Tom Rini
On Thu, Mar 14, 2024 at 01:19:40AM +0100, Heinrich Schuchardt wrote:

> Dear Tom,
> 
> The following changes since commit f3c979dd0053c082d2df170446923e7ce5edbc2d:
> 
>   Prepare v2024.04-rc4 (2024-03-11 13:11:46 -0400)
> 
> are available in the Git repository at:
> 
>   https://source.denx.de/u-boot/custodians/u-boot-efi.git
> tags/efi-2024-04-rc5
> 
> for you to fetch changes up to c8a2567475508156f4f43ea2caf3532790d47f8e:
> 
>   doc: fix incorrect path Documentation (2024-03-13 08:16:16 +0100)
> 
> Gitlab CI showed no issues:
> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/19917
> 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PULL] u-boot-mips fixes for v2024.04

2024-03-14 Thread Tom Rini
On Wed, Mar 13, 2024 at 10:14:01PM +0100, Daniel Schwierzeck wrote:

> Hi Tom,
> 
> please pull two bugfixes for MIPS, thanks.
> 
> CI: https://source.denx.de/u-boot/custodians/u-boot-mips/-/pipelines/19933
> 
> The following changes since commit f3c979dd0053c082d2df170446923e7ce5edbc2d:
> 
>   Prepare v2024.04-rc4 (2024-03-11 13:11:46 -0400)
> 
> are available in the Git repository at:
> 
>   https://source.denx.de/u-boot/custodians/u-boot-mips.git/ 
> tags/mips-fixes-for-v2024.04
> 
> for you to fetch changes up to 6806a133cde6f99777925953ee046bf2f050d4ef:
> 
>   mips: fix change_k0_cca() (2024-03-13 21:15:40 +0100)
> 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v2 2/2] rockchip: spl-boot-order: show DT path for missing device

2024-03-14 Thread Philipp Tomsich
On Thu, 14 Mar 2024 at 12:58, Christopher Obbard
 wrote:
>
> When debugging the SPL boot order, the node ID of a device which hasn't
> been found is printed but it can be quite hard to relate that to the
> specific devicetree node. To aid debugging, print the node path instead of
> the cryptic node ID.
>
> Original debug message:
>
> board_boot_order: could not map node @73c to a boot-device
>
> With this patch applied this becomes e.g:
>
>board_boot_order: could not map node /spi@ff1d/flash@0 to a boot-device
>
> Reviewed-by: Dragan Simic 
> Signed-off-by: Christopher Obbard 

Reviewed-by: Philipp Tomsich 


Re: [PATCH] starfive: visionfive2: switch to standard boot

2024-03-14 Thread Nam Cao
On 13/Mar/2024 Milan P. Stanić wrote:
> On Wed, 2024-02-21 at 13:00, Nam Cao wrote:
> > Distro boot scripts are deprecated. Use standard boot instead.  
> I had to enable 'CONFIG_CMD_SYSBOOT=y' in
> configs/starfive_visionfive2_defconfig because it doesn't boot without
> it. With this option it boots fine with this patch.

You should not have to do that for it to work, otherwise this patch
introduced a bug.

CONFIG_CMD_SYSBOOT adds the "sysboot" command, which is only used by
distroboot. So I suspect that you were actually still using distro boot,
because you didn't "update" your environment variable (that can be checked
with "printenv bootcmd"). For this patch to work, the environment variables
also need to be updated with:
env default
env save -a

This makes me realize that the patch breaks boards if the users forget to
update the environment variables. I am not sure if this is considered a
bug. What do maintainers think?

Best regards,
Nam

> Tested on u-boot version 2024.04-rc4-dirty
> 
> > Signed-off-by: Nam Cao 
> > Reviewed-by: Leo Yu-Chi Liang   
> Tested-by: Milan P. Stanić 
> 
> > ---
> >  configs/starfive_visionfive2_defconfig |  2 +-
> >  include/configs/starfive-visionfive2.h | 14 +-
> >  2 files changed, 2 insertions(+), 14 deletions(-)
> > 
> > diff --git a/configs/starfive_visionfive2_defconfig 
> > b/configs/starfive_visionfive2_defconfig
> > index b11be7ac86..aec751f871 100644
> > --- a/configs/starfive_visionfive2_defconfig
> > +++ b/configs/starfive_visionfive2_defconfig
> > @@ -31,8 +31,8 @@ CONFIG_RISCV_SMODE=y
> >  # CONFIG_OF_BOARD_FIXUP is not set
> >  # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
> >  CONFIG_FIT=y
> > +CONFIG_BOOTSTD_DEFAULTS=y
> >  CONFIG_SYS_BOOTM_LEN=0x400
> > -CONFIG_DISTRO_DEFAULTS=y
> >  CONFIG_BOOTSTAGE=y
> >  CONFIG_QSPI_BOOT=y
> >  CONFIG_SD_BOOT=y
> > diff --git a/include/configs/starfive-visionfive2.h 
> > b/include/configs/starfive-visionfive2.h
> > index 29c74470c7..049b0a0630 100644
> > --- a/include/configs/starfive-visionfive2.h
> > +++ b/include/configs/starfive-visionfive2.h
> > @@ -15,17 +15,6 @@
> >  
> >  #define __io
> >  
> > -/* Environment options */
> > -
> > -#define BOOT_TARGET_DEVICES(func) \
> > -   func(NVME, nvme, 0) \
> > -   func(USB, usb, 0) \
> > -   func(MMC, mmc, 0) \
> > -   func(MMC, mmc, 1) \
> > -   func(DHCP, dhcp, na)
> > -
> > -#include 
> > -
> >  #define TYPE_GUID_SPL  "2E54B353-1271-4842-806F-E436D6AF6985"
> >  #define TYPE_GUID_UBOOT"BC13C2FF-59E6-4262-A352-B275FD6F7172"
> >  #define TYPE_GUID_SYSTEM   "EBD0A0A2-B9E5-4433-87C0-68B6B72699C7"
> > @@ -48,7 +37,6 @@
> > "type_guid_gpt_loader2=" TYPE_GUID_UBOOT "\0" \
> > "type_guid_gpt_system=" TYPE_GUID_SYSTEM "\0" \
> > "partitions=" PARTS_DEFAULT "\0" \
> > -   "fdtfile=" CONFIG_DEFAULT_FDT_FILE "\0" \
> > -   BOOTENV
> > +   "fdtfile=" CONFIG_DEFAULT_FDT_FILE "\0"
> >  
> >  #endif /* _STARFIVE_VISIONFIVE2_H */  



[PATCH v2 2/2] rockchip: spl-boot-order: show DT path for missing device

2024-03-14 Thread Christopher Obbard
When debugging the SPL boot order, the node ID of a device which hasn't
been found is printed but it can be quite hard to relate that to the
specific devicetree node. To aid debugging, print the node path instead of
the cryptic node ID.

Original debug message:

board_boot_order: could not map node @73c to a boot-device

With this patch applied this becomes e.g:

   board_boot_order: could not map node /spi@ff1d/flash@0 to a boot-device

Reviewed-by: Dragan Simic 
Signed-off-by: Christopher Obbard 
---

Changes in v2:
- Improve patch subject (suggested by Dragan S).
- Print node path instead of node ID (suggested by Quentin S).
- Collect Reviewed-by tag from Dragan S.

 arch/arm/mach-rockchip/spl-boot-order.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-rockchip/spl-boot-order.c 
b/arch/arm/mach-rockchip/spl-boot-order.c
index 8f479ac0ec0..3543267aa57 100644
--- a/arch/arm/mach-rockchip/spl-boot-order.c
+++ b/arch/arm/mach-rockchip/spl-boot-order.c
@@ -148,8 +148,8 @@ void board_boot_order(u32 *spl_boot_list)
/* Try to map this back onto SPL boot devices */
boot_device = spl_node_to_boot_device(node);
if (boot_device < 0) {
-   debug("%s: could not map node @%x to a boot-device\n",
- __func__, node);
+   debug("%s: could not map node %s to a boot-device\n",
+ __func__, conf);
continue;
}
 
-- 
2.43.0



[PATCH v2 1/2] rockchip: spl-boot-order: fix typo in comment succes→success

2024-03-14 Thread Christopher Obbard
Fix a simple spelling mistake in a comment.

Reviewed-by: Dragan Simic 
Reviewed-by: Quentin Schulz 
Signed-off-by: Christopher Obbard 
---

Changes in v2:
- Improve patch subject (suggested by Dragan S).
- Collect Reviewed-by tags from Dragan S and Quentin S.

 arch/arm/mach-rockchip/spl-boot-order.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-rockchip/spl-boot-order.c 
b/arch/arm/mach-rockchip/spl-boot-order.c
index 79c856d2a0a..8f479ac0ec0 100644
--- a/arch/arm/mach-rockchip/spl-boot-order.c
+++ b/arch/arm/mach-rockchip/spl-boot-order.c
@@ -29,7 +29,7 @@
  *   -ENOSYS, if the device matching the node can not be mapped onto a
  *SPL boot device (e.g. the third MMC device)
  *   -1, for unspecified failures
- *   a positive integer (from the BOOT_DEVICE_... family) on succes.
+ *   a positive integer (from the BOOT_DEVICE_... family) on success.
  */
 
 static int spl_node_to_boot_device(int node)
-- 
2.43.0



[PATCH v2 0/2] Trivial fixes for Rockchip SPL boot order

2024-03-14 Thread Christopher Obbard
This series contains some trivial fixes for the Rockchip SPL boot order
driver.

The first patch fixes a typo in a comment.

The second patch prints the full devicetree node path (rather than just
the node ID) in the debug message when a boot device can't be found.

This series may be found at [0].

[0]: 
https://gitlab.collabora.com/obbardc/u-boot/-/tree/wip/obbardc/rockchip-spl-boot-order-fixes-v2

Changes in v2:
- Print node path instead of node ID (suggested by Quentin S).
- Improve patch subjects (suggested by Dragan S).
- Improve cover letter a little.
- Collect Reviewed-by tags.
- Rebase series on top of rockchip/for-next.
- v1: https://lists.denx.de/pipermail/u-boot/2024-March/547175.html

Christopher Obbard (2):
  rockchip: spl-boot-order: fix typo in comment succes→success
  rockchip: spl-boot-order: show DT path for missing device

 arch/arm/mach-rockchip/spl-boot-order.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

-- 
2.43.0



Re: [PATCH] fs: ext4: Fixed file permissions

2024-03-14 Thread Dan Carpenter
On Thu, Mar 14, 2024 at 02:41:29PM +0800, Jixiong Hu wrote:
> Modified the ext4fs_write function to create a new file that
> inherits the inode->mode of existing file. To fix an issue
> where file permissions are changed after modifying the contents
> of an existing file.

I'm not an expert, but honestly the current behavior sound more correct
than the proposed behavior.  What's the issue specifically so we can
understand it better?  Also bug fixes need a Fixes tag.

The patch has a number of style issues and the existing_file_inode
allocation is not always freed so those memory leaks would need to be
fixed.  For the style issues run ./scripts/checkpatch.pl on your patch.

regards,
dan carpenter



[PATCH] fs: ext4: Fixed file permissions

2024-03-14 Thread Jixiong Hu
Modified the ext4fs_write function to create a new file that
inherits the inode->mode of existing file. To fix an issue
where file permissions are changed after modifying the contents
of an existing file.
---
 fs/ext4/ext4_write.c | 20 ++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c
index ea4c5d4157..6da4a26b71 100644
--- a/fs/ext4/ext4_write.c
+++ b/fs/ext4/ext4_write.c
@@ -847,6 +847,7 @@ int ext4fs_write(const char *fname, const char *buffer,
 {
int ret = 0;
struct ext2_inode *file_inode = NULL;
+   struct ext2_inode *existing_file_inode = NULL;
unsigned char *inode_buffer = NULL;
int parent_inodeno;
int inodeno;
@@ -900,6 +901,16 @@ int ext4fs_write(const char *fname, const char *buffer,
/* check if the filename is already present in root */
existing_file_inodeno = ext4fs_filename_unlink(filename);
if (existing_file_inodeno != -1) {
+   existing_file_inode = (struct ext2_inode *)zalloc(fs->inodesz);
+   if (!existing_file_inode)
+   goto fail;
+   ret = ext4fs_iget(existing_file_inodeno, existing_file_inode);
+   if (ret)
+   {
+   free(existing_file_inode);
+   goto fail;
+   }
+
ret = ext4fs_delete_file(existing_file_inodeno);
fs->first_pass_bbmap = 0;
fs->curr_blkno = 0;
@@ -948,8 +959,13 @@ int ext4fs_write(const char *fname, const char *buffer,
sizebytes = 0;
}
} else {
-   file_inode->mode = cpu_to_le16(S_IFREG | S_IRWXU | S_IRGRP |
-  S_IROTH | S_IXGRP | S_IXOTH);
+   if(existing_file_inode) {
+   file_inode->mode = existing_file_inode->mode;
+   free(existing_file_inode);
+   } else {
+   file_inode->mode = cpu_to_le16(S_IFREG | S_IRWXU | 
S_IRGRP |
+  S_IROTH | S_IXGRP | 
S_IXOTH);
+   }
}
/* ToDo: Update correct time */
file_inode->mtime = cpu_to_le32(timestamp);
2.18.0



[PATCH v4 16/16] rockchip: boot_mode: fix rockchip_dnl_key_pressed requiring ADC support

2024-03-14 Thread Quentin Schulz
From: Quentin Schulz 

ADC support is implied by the Rockchip arch Kconfig but that means it
should be possible to disable ADC support and still be able to build.

However the weak implementation of rockchip_dnl_key_pressed() currently
blindly use functions from the ADC subsystem which do not exist when ADC
is not enabled, failing the build.

Therefore, let's encapsulate this logic with a check on the ADC symbol
being selected.

Cc: Quentin Schulz 
Reviewed-by: Kever Yang 
Signed-off-by: Quentin Schulz 
---
 arch/arm/mach-rockchip/boot_mode.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/mach-rockchip/boot_mode.c 
b/arch/arm/mach-rockchip/boot_mode.c
index eb8f65ae4e9..f9be396aa55 100644
--- a/arch/arm/mach-rockchip/boot_mode.c
+++ b/arch/arm/mach-rockchip/boot_mode.c
@@ -40,6 +40,7 @@ void set_back_to_bootrom_dnl_flag(void)
 
 __weak int rockchip_dnl_key_pressed(void)
 {
+#if CONFIG_IS_ENABLED(ADC)
unsigned int val;
struct udevice *dev;
struct uclass *uc;
@@ -69,6 +70,9 @@ __weak int rockchip_dnl_key_pressed(void)
return true;
else
return false;
+#else
+   return false;
+#endif
 }
 
 void rockchip_dnl_mode_check(void)

-- 
2.44.0



[PATCH v4 15/16] button: add missing ADC dependency for BUTTON_ADC

2024-03-14 Thread Quentin Schulz
From: Quentin Schulz 

The BUTTON_ADC symbol guards the compilation of button-adc driver whose
name very well makes it explicit that it requires ADC support to be
enabled.

Fix build issue of button-adc driver when ADC support isn't enabled by
making sure it cannot be built without ADC support.

Cc: Quentin Schulz 
Reviewed-by: Kever Yang 
Signed-off-by: Quentin Schulz 
---
 drivers/button/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig
index 097b05f822e..3918b05ae03 100644
--- a/drivers/button/Kconfig
+++ b/drivers/button/Kconfig
@@ -12,6 +12,7 @@ config BUTTON
 config BUTTON_ADC
bool "Button adc"
depends on BUTTON
+   depends on ADC
help
  Enable support for buttons which are connected to Analog to Digital
  Converter device. The ADC driver must use driver model. Buttons are

-- 
2.44.0



[PATCH v4 14/16] adc: add missing depends on ADC for controller drivers

2024-03-14 Thread Quentin Schulz
From: Quentin Schulz 

The ADC controller drivers are obviously all depending on ADC symbol
being selected.

While they don't seem to fail to build without, they won't be useful
without that symbol selected, so let's make sure the options aren't
shown in menuconfig when ADC isn't selected.

Cc: Quentin Schulz 
Reviewed-by: Kever Yang 
Signed-off-by: Quentin Schulz 
---
 drivers/adc/Kconfig | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/adc/Kconfig b/drivers/adc/Kconfig
index a01d73846b7..c9cdbe6942d 100644
--- a/drivers/adc/Kconfig
+++ b/drivers/adc/Kconfig
@@ -13,6 +13,7 @@ config ADC
 
 config ADC_EXYNOS
bool "Enable Exynos 54xx ADC driver"
+   depends on ADC
help
  This enables basic driver for Exynos ADC compatible with Exynos54xx.
  It provides:
@@ -22,6 +23,7 @@ config ADC_EXYNOS
 
 config ADC_SANDBOX
bool "Enable Sandbox ADC test driver"
+   depends on ADC
help
  This enables driver for Sandbox ADC device emulation.
  It provides:
@@ -31,6 +33,7 @@ config ADC_SANDBOX
 
 config SARADC_MESON
bool "Enable Amlogic Meson SARADC driver"
+   depends on ADC
imply REGMAP
help
  This enables driver for Amlogic Meson SARADC.
@@ -41,6 +44,7 @@ config SARADC_MESON
 
 config SARADC_ROCKCHIP
bool "Enable Rockchip SARADC driver"
+   depends on ADC
help
  This enables driver for Rockchip SARADC.
  It provides:

-- 
2.44.0



[PATCH v4 13/16] rockchip: jaguar-rk3588: enable SARADC and derivatives

2024-03-14 Thread Quentin Schulz
From: Quentin Schulz 

The SARADC is used on Jaguar for multiple things:
- channel 0 is used (at runtime) as a BIOS button,
- channel 2 is exposed on the Mezzanine connector for customer specific
  logic,
- channel 5 and 6 are used for identification,

Since the SARADC requires a vref-supply provided by the RK806 PMIC, its
support and the support for its regulators are also enabled.

The button, adc, pmic and regulator commands are also enabled for CLI
use in U-Boot for debugging and scripting purposes.

The RK806 PMIC on Jaguar being routed on the SPI bus, let's enable
Rockchip SPI controller driver.

Finally, the SARADC channel 1 on Jaguar is hardwired so will never
change in the lifetime of a unit, for that reason, disable the Rockchip
Download Mode check by setting ROCKCHIP_BOOT_MODE_REG symbol to 0.

Cc: Quentin Schulz 
Reviewed-by: Kever Yang 
Signed-off-by: Quentin Schulz 
---
 configs/jaguar-rk3588_defconfig | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/configs/jaguar-rk3588_defconfig b/configs/jaguar-rk3588_defconfig
index f55bfb1c82b..275d70ae008 100644
--- a/configs/jaguar-rk3588_defconfig
+++ b/configs/jaguar-rk3588_defconfig
@@ -15,6 +15,7 @@ CONFIG_ENV_SIZE=0x1f000
 CONFIG_DEFAULT_DEVICE_TREE="rk3588-jaguar"
 CONFIG_ROCKCHIP_RK3588=y
 CONFIG_SPL_ROCKCHIP_COMMON_BOARD=y
+CONFIG_ROCKCHIP_BOOT_MODE_REG=0x0
 CONFIG_SPL_SERIAL=y
 CONFIG_SPL_STACK_R_ADDR=0x60
 CONFIG_TARGET_JAGUAR_RK3588=y
@@ -47,6 +48,7 @@ CONFIG_SPL_ATF=y
 # CONFIG_BOOTM_RTEMS is not set
 # CONFIG_BOOTM_VXWORKS is not set
 # CONFIG_CMD_ELF is not set
+CONFIG_CMD_ADC=y
 CONFIG_CMD_GPIO=y
 CONFIG_CMD_GPT=y
 CONFIG_CMD_I2C=y
@@ -59,6 +61,7 @@ CONFIG_CMD_USB=y
 # CONFIG_CMD_MII is not set
 # CONFIG_CMD_BLOCK_CACHE is not set
 # CONFIG_CMD_EFICONFIG is not set
+CONFIG_CMD_PMIC=y
 CONFIG_CMD_REGULATOR=y
 CONFIG_CMD_EROFS=y
 CONFIG_CMD_SQUASHFS=y
@@ -73,7 +76,8 @@ CONFIG_SYS_RELOC_GD_ENV_ADDR=y
 CONFIG_SPL_DM_SEQ_ALIAS=y
 CONFIG_SPL_REGMAP=y
 CONFIG_SPL_SYSCON=y
-# CONFIG_SARADC_ROCKCHIP is not set
+CONFIG_BUTTON=y
+CONFIG_BUTTON_ADC=y
 CONFIG_SPL_CLK=y
 CONFIG_CLK_GPIO=y
 CONFIG_ROCKCHIP_GPIO=y
@@ -101,10 +105,14 @@ CONFIG_DWC_ETH_QOS=y
 CONFIG_DWC_ETH_QOS_ROCKCHIP=y
 CONFIG_PHY_ROCKCHIP_INNO_USB2=y
 CONFIG_SPL_PINCTRL=y
+CONFIG_DM_PMIC=y
+CONFIG_PMIC_RK8XX=y
+CONFIG_REGULATOR_RK8XX=y
 CONFIG_SPL_RAM=y
 CONFIG_SCSI=y
 CONFIG_DEBUG_UART_SHIFT=2
 CONFIG_SYS_NS16550_MEM32=y
+CONFIG_ROCKCHIP_SPI=y
 CONFIG_SYSRESET=y
 CONFIG_USB=y
 CONFIG_USB_EHCI_HCD=y

-- 
2.44.0



[PATCH v4 12/16] power: pmic: rk8xx: fix duplicate prompt

2024-03-14 Thread Quentin Schulz
From: Quentin Schulz 

SPL_PMIC_RK8XX and PMIC_RK8XX both share the same prompt making it
difficult to know at first glance in menuconfig what's for what, let's
fix this by adding "in SPL" at the end of the prompt for the SPL symbol.

Cc: Quentin Schulz 
Reviewed-by: Kever Yang 
Signed-off-by: Quentin Schulz 
---
 drivers/power/pmic/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig
index 9b61b18e11f..562c1a3b122 100644
--- a/drivers/power/pmic/Kconfig
+++ b/drivers/power/pmic/Kconfig
@@ -250,7 +250,7 @@ config PMIC_RK8XX
This driver implements register read/write operations.
 
 config SPL_PMIC_RK8XX
-   bool "Enable support for Rockchip PMIC RK8XX"
+   bool "Enable support for Rockchip PMIC RK8XX in SPL"
depends on SPL_DM_PMIC
---help---
The Rockchip RK808 PMIC provides four buck DC-DC convertors, 8 LDOs,

-- 
2.44.0



[PATCH v4 11/16] rockchip: adc: rockchip-saradc: add support for RK3588

2024-03-14 Thread Quentin Schulz
From: Quentin Schulz 

This adds support for the SARADCv2 found on RK3588.

There is no stop callback as it is currently configured in single
conversion mode, where the ADC is powered down after a single conversion
has been made.

Due to what seems to be a silicon bug, a controller reset needs to be
issued before starting a channel conversion otherwise Rockchip says that
channel 1 will error whatever that means. This is aligned with upstream
and downstream Linux kernel as well as downstream U-Boot.

Cc: Quentin Schulz 
Reviewed-by: Kever Yang 
Signed-off-by: Quentin Schulz 
---
 drivers/adc/rockchip-saradc.c | 102 +-
 1 file changed, 101 insertions(+), 1 deletion(-)

diff --git a/drivers/adc/rockchip-saradc.c b/drivers/adc/rockchip-saradc.c
index b5df58fe3eb..10ded1b088f 100644
--- a/drivers/adc/rockchip-saradc.c
+++ b/drivers/adc/rockchip-saradc.c
@@ -10,12 +10,17 @@
 #include 
 #include 
 #include 
-#include 
+#include 
+#include 
+#include 
 #include 
+#include 
 #include 
 #include 
 #include 
 
+#define usleep_range(a, b) udelay((b))
+
 #define SARADC_CTRL_CHN_MASK   GENMASK(2, 0)
 #define SARADC_CTRL_POWER_CTRL BIT(3)
 #define SARADC_CTRL_IRQ_ENABLE BIT(5)
@@ -30,8 +35,37 @@ struct rockchip_saradc_regs_v1 {
unsigned int dly_pu_soc;
 };
 
+struct rockchip_saradc_regs_v2 {
+   unsigned int conv_con;
+#define SARADC2_SINGLE_MODEBIT(5)
+#define SARADC2_START  BIT(4)
+#define SARADC2_CONV_CHANNELS  GENMASK(3, 0)
+   unsigned int t_pd_soc;
+   unsigned int t_as_soc;
+   unsigned int t_das_soc;
+   unsigned int t_sel_soc;
+   unsigned int high_comp[16];
+   unsigned int low_comp[16];
+   unsigned int debounce;
+   unsigned int ht_int_en;
+   unsigned int lt_int_en;
+   unsigned int reserved[24];
+   unsigned int mt_int_en;
+   unsigned int end_int_en;
+#define SARADC2_EN_END_INT BIT(0)
+   unsigned int st_con;
+   unsigned int status;
+   unsigned int end_int_st;
+   unsigned int ht_int_st;
+   unsigned int lt_int_st;
+   unsigned int mt_int_st;
+   unsigned int data[16];
+   unsigned int auto_ch_en;
+};
+
 union rockchip_saradc_regs {
struct rockchip_saradc_regs_v1  *v1;
+   struct rockchip_saradc_regs_v2  *v2;
 };
 struct rockchip_saradc_data {
int num_bits;
@@ -46,6 +80,7 @@ struct rockchip_saradc_priv {
union rockchip_saradc_regs  regs;
int active_channel;
const struct rockchip_saradc_data   *data;
+   struct reset_ctl*reset;
 };
 
 int rockchip_saradc_channel_data_v1(struct udevice *dev, int channel,
@@ -66,6 +101,22 @@ int rockchip_saradc_channel_data_v1(struct udevice *dev, 
int channel,
return 0;
 }
 
+int rockchip_saradc_channel_data_v2(struct udevice *dev, int channel,
+   unsigned int *data)
+{
+   struct rockchip_saradc_priv *priv = dev_get_priv(dev);
+
+   if (!(readl(&priv->regs.v2->end_int_st) & SARADC2_EN_END_INT))
+   return -EBUSY;
+
+   /* Read value */
+   *data = readl(&priv->regs.v2->data[channel]);
+
+   /* Acknowledge the interrupt */
+   writel(SARADC2_EN_END_INT, &priv->regs.v2->end_int_st);
+
+   return 0;
+}
 int rockchip_saradc_channel_data(struct udevice *dev, int channel,
 unsigned int *data)
 {
@@ -104,6 +155,40 @@ int rockchip_saradc_start_channel_v1(struct udevice *dev, 
int channel)
return 0;
 }
 
+static void rockchip_saradc_reset_controller(struct reset_ctl *reset)
+{
+   reset_assert(reset);
+   usleep_range(10, 20);
+   reset_deassert(reset);
+}
+
+int rockchip_saradc_start_channel_v2(struct udevice *dev, int channel)
+{
+   struct rockchip_saradc_priv *priv = dev_get_priv(dev);
+
+   /*
+* Downstream says
+* """If read other chn at anytime, then chn1 will error, assert
+* controller as a workaround."""
+*/
+   if (priv->reset)
+   rockchip_saradc_reset_controller(priv->reset);
+
+   writel(0xc, &priv->regs.v2->t_das_soc);
+   writel(0x20, &priv->regs.v2->t_pd_soc);
+
+   /* Acknowledge any previous interrupt */
+   writel(SARADC2_EN_END_INT, &priv->regs.v2->end_int_st);
+
+   rk_clrsetreg(&priv->regs.v2->conv_con,
+SARADC2_CONV_CHANNELS | SARADC2_START | 
SARADC2_SINGLE_MODE,
+FIELD_PREP(SARADC2_CONV_CHANNELS, channel) |
+FIELD_PREP(SARADC2_START, 1) |
+FIELD_PREP(SARADC2_SINGLE_MODE, 1));
+
+   return 0;
+}
+
 int rockchip_saradc_start_channel(struct udevice *dev, int channel)
 {
struct rockchip_saradc_priv *priv = dev_get_priv(dev);
@@ -162,6 +247,8 @@ int rockchip_saradc_probe(struct udevice *dev)
int vref_uv;
int ret;
 
+   priv->re

[PATCH v4 10/16] rockchip: adc: rockchip-saradc: factor out stop callback

2024-03-14 Thread Quentin Schulz
From: Quentin Schulz 

SARADC v2 doesn't have a stop mechanism once in single mode. In series
conversion, the logic is different anyway. Therefore, let's abstract
this function so that it can be provided from the udevice.data pointer.

Cc: Quentin Schulz 
Reviewed-by: Kever Yang 
Signed-off-by: Quentin Schulz 
---
 drivers/adc/rockchip-saradc.c | 22 +-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/adc/rockchip-saradc.c b/drivers/adc/rockchip-saradc.c
index 607d10b5b70..b5df58fe3eb 100644
--- a/drivers/adc/rockchip-saradc.c
+++ b/drivers/adc/rockchip-saradc.c
@@ -39,6 +39,7 @@ struct rockchip_saradc_data {
unsigned long   clk_rate;
int (*channel_data)(struct udevice *dev, int channel, unsigned int 
*data);
int (*start_channel)(struct udevice *dev, int channel);
+   int (*stop)(struct udevice *dev);
 };
 
 struct rockchip_saradc_priv {
@@ -124,13 +125,29 @@ int rockchip_saradc_start_channel(struct udevice *dev, 
int channel)
return 0;
 }
 
-int rockchip_saradc_stop(struct udevice *dev)
+int rockchip_saradc_stop_v1(struct udevice *dev)
 {
struct rockchip_saradc_priv *priv = dev_get_priv(dev);
 
/* Power down adc */
writel(0, &priv->regs.v1->ctrl);
 
+   return 0;
+}
+
+int rockchip_saradc_stop(struct udevice *dev)
+{
+   struct rockchip_saradc_priv *priv = dev_get_priv(dev);
+
+   if (priv->data->stop) {
+   int ret = priv->data->stop(dev);
+
+   if (ret) {
+   pr_err("Error stopping channel, %d!", ret);
+   return ret;
+   }
+   }
+
priv->active_channel = -1;
 
return 0;
@@ -209,6 +226,7 @@ static const struct rockchip_saradc_data saradc_data = {
.clk_rate = 100,
.channel_data = rockchip_saradc_channel_data_v1,
.start_channel = rockchip_saradc_start_channel_v1,
+   .stop = rockchip_saradc_stop_v1,
 };
 
 static const struct rockchip_saradc_data rk3066_tsadc_data = {
@@ -217,6 +235,7 @@ static const struct rockchip_saradc_data rk3066_tsadc_data 
= {
.clk_rate = 5,
.channel_data = rockchip_saradc_channel_data_v1,
.start_channel = rockchip_saradc_start_channel_v1,
+   .stop = rockchip_saradc_stop_v1,
 };
 
 static const struct rockchip_saradc_data rk3399_saradc_data = {
@@ -225,6 +244,7 @@ static const struct rockchip_saradc_data rk3399_saradc_data 
= {
.clk_rate = 100,
.channel_data = rockchip_saradc_channel_data_v1,
.start_channel = rockchip_saradc_start_channel_v1,
+   .stop = rockchip_saradc_stop_v1,
 };
 
 static const struct udevice_id rockchip_saradc_ids[] = {

-- 
2.44.0



[PATCH v4 09/16] rockchip: adc: rockchip-saradc: factor out start_channel callback

2024-03-14 Thread Quentin Schulz
From: Quentin Schulz 

SARADC v1 and v2 have a different way of starting a channel, therefore
let's abstract this function so that it can be provided from the
udevice.data pointer.

Cc: Quentin Schulz 
Reviewed-by: Kever Yang 
Signed-off-by: Quentin Schulz 
---
 drivers/adc/rockchip-saradc.c | 30 --
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/adc/rockchip-saradc.c b/drivers/adc/rockchip-saradc.c
index 1cc57beecc2..607d10b5b70 100644
--- a/drivers/adc/rockchip-saradc.c
+++ b/drivers/adc/rockchip-saradc.c
@@ -38,6 +38,7 @@ struct rockchip_saradc_data {
int num_channels;
unsigned long   clk_rate;
int (*channel_data)(struct udevice *dev, int channel, unsigned int 
*data);
+   int (*start_channel)(struct udevice *dev, int channel);
 };
 
 struct rockchip_saradc_priv {
@@ -88,15 +89,10 @@ int rockchip_saradc_channel_data(struct udevice *dev, int 
channel,
return 0;
 }
 
-int rockchip_saradc_start_channel(struct udevice *dev, int channel)
+int rockchip_saradc_start_channel_v1(struct udevice *dev, int channel)
 {
struct rockchip_saradc_priv *priv = dev_get_priv(dev);
 
-   if (channel < 0 || channel >= priv->data->num_channels) {
-   pr_err("Requested channel is invalid!");
-   return -EINVAL;
-   }
-
/* 8 clock periods as delay between power up and start cmd */
writel(8, &priv->regs.v1->dly_pu_soc);
 
@@ -104,6 +100,25 @@ int rockchip_saradc_start_channel(struct udevice *dev, int 
channel)
writel(SARADC_CTRL_POWER_CTRL | (channel & SARADC_CTRL_CHN_MASK) |
   SARADC_CTRL_IRQ_ENABLE, &priv->regs.v1->ctrl);
 
+   return 0;
+}
+
+int rockchip_saradc_start_channel(struct udevice *dev, int channel)
+{
+   struct rockchip_saradc_priv *priv = dev_get_priv(dev);
+   int ret;
+
+   if (channel < 0 || channel >= priv->data->num_channels) {
+   pr_err("Requested channel is invalid!");
+   return -EINVAL;
+   }
+
+   ret = priv->data->start_channel(dev, channel);
+   if (ret) {
+   pr_err("Error starting channel, %d!", ret);
+   return ret;
+   }
+
priv->active_channel = channel;
 
return 0;
@@ -193,6 +208,7 @@ static const struct rockchip_saradc_data saradc_data = {
.num_channels = 3,
.clk_rate = 100,
.channel_data = rockchip_saradc_channel_data_v1,
+   .start_channel = rockchip_saradc_start_channel_v1,
 };
 
 static const struct rockchip_saradc_data rk3066_tsadc_data = {
@@ -200,6 +216,7 @@ static const struct rockchip_saradc_data rk3066_tsadc_data 
= {
.num_channels = 2,
.clk_rate = 5,
.channel_data = rockchip_saradc_channel_data_v1,
+   .start_channel = rockchip_saradc_start_channel_v1,
 };
 
 static const struct rockchip_saradc_data rk3399_saradc_data = {
@@ -207,6 +224,7 @@ static const struct rockchip_saradc_data rk3399_saradc_data 
= {
.num_channels = 6,
.clk_rate = 100,
.channel_data = rockchip_saradc_channel_data_v1,
+   .start_channel = rockchip_saradc_start_channel_v1,
 };
 
 static const struct udevice_id rockchip_saradc_ids[] = {

-- 
2.44.0



[PATCH v4 08/16] rockchip: adc: rockchip-saradc: factor out channel_data callback

2024-03-14 Thread Quentin Schulz
From: Quentin Schulz 

SARADC v1 and v2 have a different way of reading data, therefore let's
abstract this function so that it can be provided from the udevice.data
pointer.

Cc: Quentin Schulz 
Reviewed-by: Kever Yang 
Signed-off-by: Quentin Schulz 
---
 drivers/adc/rockchip-saradc.c | 37 +
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/adc/rockchip-saradc.c b/drivers/adc/rockchip-saradc.c
index 4a842556a60..1cc57beecc2 100644
--- a/drivers/adc/rockchip-saradc.c
+++ b/drivers/adc/rockchip-saradc.c
@@ -37,6 +37,7 @@ struct rockchip_saradc_data {
int num_bits;
int num_channels;
unsigned long   clk_rate;
+   int (*channel_data)(struct udevice *dev, int channel, unsigned int 
*data);
 };
 
 struct rockchip_saradc_priv {
@@ -45,28 +46,45 @@ struct rockchip_saradc_priv {
const struct rockchip_saradc_data   *data;
 };
 
+int rockchip_saradc_channel_data_v1(struct udevice *dev, int channel,
+   unsigned int *data)
+{
+   struct rockchip_saradc_priv *priv = dev_get_priv(dev);
+
+   if ((readl(&priv->regs.v1->ctrl) & SARADC_CTRL_IRQ_STATUS) !=
+   SARADC_CTRL_IRQ_STATUS)
+   return -EBUSY;
+
+   /* Read value */
+   *data = readl(&priv->regs.v1->data);
+
+   /* Power down adc */
+   writel(0, &priv->regs.v1->ctrl);
+
+   return 0;
+}
+
 int rockchip_saradc_channel_data(struct udevice *dev, int channel,
 unsigned int *data)
 {
struct rockchip_saradc_priv *priv = dev_get_priv(dev);
struct adc_uclass_plat *uc_pdata = dev_get_uclass_plat(dev);
+   int ret;
 
if (channel != priv->active_channel) {
pr_err("Requested channel is not active!");
return -EINVAL;
}
 
-   if ((readl(&priv->regs.v1->ctrl) & SARADC_CTRL_IRQ_STATUS) !=
-   SARADC_CTRL_IRQ_STATUS)
-   return -EBUSY;
+   ret = priv->data->channel_data(dev, channel, data);
+   if (ret) {
+   if (ret != -EBUSY)
+   pr_err("Error reading channel data, %d!", ret);
+   return ret;
+   }
 
-   /* Read value */
-   *data = readl(&priv->regs.v1->data);
*data &= uc_pdata->data_mask;
 
-   /* Power down adc */
-   writel(0, &priv->regs.v1->ctrl);
-
return 0;
 }
 
@@ -174,18 +192,21 @@ static const struct rockchip_saradc_data saradc_data = {
.num_bits = 10,
.num_channels = 3,
.clk_rate = 100,
+   .channel_data = rockchip_saradc_channel_data_v1,
 };
 
 static const struct rockchip_saradc_data rk3066_tsadc_data = {
.num_bits = 12,
.num_channels = 2,
.clk_rate = 5,
+   .channel_data = rockchip_saradc_channel_data_v1,
 };
 
 static const struct rockchip_saradc_data rk3399_saradc_data = {
.num_bits = 10,
.num_channels = 6,
.clk_rate = 100,
+   .channel_data = rockchip_saradc_channel_data_v1,
 };
 
 static const struct udevice_id rockchip_saradc_ids[] = {

-- 
2.44.0



[PATCH v4 07/16] rockchip: adc: rockchip-saradc: use union for preparing for v2

2024-03-14 Thread Quentin Schulz
From: Quentin Schulz 

The registers are entirely different between SARADC v1 and SARADC v2, so
let's prepare to add another struct for accessing v2 registers by adding
a union.

Cc: Quentin Schulz 
Reviewed-by: Kever Yang 
Signed-off-by: Quentin Schulz 
---
 drivers/adc/rockchip-saradc.c | 23 +--
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/adc/rockchip-saradc.c b/drivers/adc/rockchip-saradc.c
index 03caca78b5f..4a842556a60 100644
--- a/drivers/adc/rockchip-saradc.c
+++ b/drivers/adc/rockchip-saradc.c
@@ -23,13 +23,16 @@
 
 #define SARADC_TIMEOUT (100 * 1000)
 
-struct rockchip_saradc_regs {
+struct rockchip_saradc_regs_v1 {
unsigned int data;
unsigned int stas;
unsigned int ctrl;
unsigned int dly_pu_soc;
 };
 
+union rockchip_saradc_regs {
+   struct rockchip_saradc_regs_v1  *v1;
+};
 struct rockchip_saradc_data {
int num_bits;
int num_channels;
@@ -37,7 +40,7 @@ struct rockchip_saradc_data {
 };
 
 struct rockchip_saradc_priv {
-   struct rockchip_saradc_regs *regs;
+   union rockchip_saradc_regs  regs;
int active_channel;
const struct rockchip_saradc_data   *data;
 };
@@ -53,16 +56,16 @@ int rockchip_saradc_channel_data(struct udevice *dev, int 
channel,
return -EINVAL;
}
 
-   if ((readl(&priv->regs->ctrl) & SARADC_CTRL_IRQ_STATUS) !=
+   if ((readl(&priv->regs.v1->ctrl) & SARADC_CTRL_IRQ_STATUS) !=
SARADC_CTRL_IRQ_STATUS)
return -EBUSY;
 
/* Read value */
-   *data = readl(&priv->regs->data);
+   *data = readl(&priv->regs.v1->data);
*data &= uc_pdata->data_mask;
 
/* Power down adc */
-   writel(0, &priv->regs->ctrl);
+   writel(0, &priv->regs.v1->ctrl);
 
return 0;
 }
@@ -77,11 +80,11 @@ int rockchip_saradc_start_channel(struct udevice *dev, int 
channel)
}
 
/* 8 clock periods as delay between power up and start cmd */
-   writel(8, &priv->regs->dly_pu_soc);
+   writel(8, &priv->regs.v1->dly_pu_soc);
 
/* Select the channel to be used and trigger conversion */
writel(SARADC_CTRL_POWER_CTRL | (channel & SARADC_CTRL_CHN_MASK) |
-  SARADC_CTRL_IRQ_ENABLE, &priv->regs->ctrl);
+  SARADC_CTRL_IRQ_ENABLE, &priv->regs.v1->ctrl);
 
priv->active_channel = channel;
 
@@ -93,7 +96,7 @@ int rockchip_saradc_stop(struct udevice *dev)
struct rockchip_saradc_priv *priv = dev_get_priv(dev);
 
/* Power down adc */
-   writel(0, &priv->regs->ctrl);
+   writel(0, &priv->regs.v1->ctrl);
 
priv->active_channel = -1;
 
@@ -146,8 +149,8 @@ int rockchip_saradc_of_to_plat(struct udevice *dev)
struct rockchip_saradc_data *data;
 
data = (struct rockchip_saradc_data *)dev_get_driver_data(dev);
-   priv->regs = dev_read_addr_ptr(dev);
-   if (!priv->regs) {
+   priv->regs.v1 = dev_read_addr_ptr(dev);
+   if (!priv->regs.v1) {
pr_err("Dev: %s - can't get address!", dev->name);
return -EINVAL;
}

-- 
2.44.0



[PATCH v4 05/16] power: rk8xx: add support for RK806

2024-03-14 Thread Quentin Schulz
From: Quentin Schulz 

This adds support for RK806, only the SPI variant has been tested.

The communication "protocol" over SPI is the following:
 - write three bytes:
   - 1 byte: [0:3] length of the payload, [6] Enable CRC, [7] Write
   - 1 byte: LSB register address
   - 1 byte: MSB register address
 - write/read length of payload

The CRC is always disabled for now.

The RK806 technically supports I2C as well, and this should be able to
support it without any change, but it wasn't tested.

The DT node name prefix for the buck converters has changed in the
Device Tree and is now dcdc-reg. The logic for buck converters is
however manageable within the current logic inside the rk8xx regulator
driver. The same cannot be said for the NLDO and PLDO.

Because pmic_bind_children() parses the DT nodes and extracts the LDO
index from the DT node name, NLDO and PLDO will have overlapping
indices. Therefore, we need a separate logic from the already-existing
ldo callbacks. Let's reuse as much as possible though.

Cc: Quentin Schulz 
Reviewed-by: Kever Yang 
Signed-off-by: Quentin Schulz 
---
 drivers/power/pmic/rk8xx.c  |  91 +++
 drivers/power/regulator/rk8xx.c | 514 +++-
 include/power/rk8xx_pmic.h  |  19 ++
 3 files changed, 622 insertions(+), 2 deletions(-)

diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c
index 4e3a17337ee..3a8261d1749 100644
--- a/drivers/power/pmic/rk8xx.c
+++ b/drivers/power/pmic/rk8xx.c
@@ -9,8 +9,10 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
+#include 
 #include 
 
 static int rk8xx_sysreset_request(struct udevice *dev, enum sysreset_t type)
@@ -32,6 +34,10 @@ static int rk8xx_sysreset_request(struct udevice *dev, enum 
sysreset_t type)
pmic_clrsetbits(dev->parent, RK817_REG_SYS_CFG3, 0,
BIT(0));
break;
+   case RK806_ID:
+   pmic_clrsetbits(dev->parent, RK806_REG_SYS_CFG3, 0,
+   BIT(0));
+   break;
default:
printf("Unknown PMIC RK%x: Cannot shutdown\n",
   priv->variant);
@@ -83,6 +89,11 @@ void rk8xx_off_for_plugin(struct udevice *dev)
}
 }
 
+static struct reg_data rk806_init_reg[] = {
+   /* RST_FUN */
+   { RK806_REG_SYS_CFG3, GENMASK(7, 6), BIT(7)},
+};
+
 static struct reg_data rk817_init_reg[] = {
 /* enable the under-voltage protection,
  * the under-voltage protection will shutdown the LDO3 and reset the PMIC
@@ -92,7 +103,10 @@ static struct reg_data rk817_init_reg[] = {
 
 static const struct pmic_child_info pmic_children_info[] = {
{ .prefix = "DCDC_REG", .driver = "rk8xx_buck"},
+   { .prefix = "dcdc-reg", .driver = "rk8xx_buck"},
{ .prefix = "LDO_REG", .driver = "rk8xx_ldo"},
+   { .prefix = "nldo-reg", .driver = "rk8xx_nldo"},
+   { .prefix = "pldo-reg", .driver = "rk8xx_pldo"},
{ .prefix = "SWITCH_REG", .driver = "rk8xx_switch"},
{ },
 };
@@ -102,11 +116,51 @@ static int rk8xx_reg_count(struct udevice *dev)
return RK808_NUM_OF_REGS;
 }
 
+#if CONFIG_IS_ENABLED(SPI) && CONFIG_IS_ENABLED(DM_SPI)
+struct rk806_cmd {
+   uint8_t len: 4; /* Payload size in bytes - 1 */
+   uint8_t reserved: 2;
+   uint8_t crc_en: 1;
+   uint8_t op: 1; /* READ=0; WRITE=1; */
+   uint8_t reg_l;
+#define REG_L_MASK GENMASK(7, 0)
+   uint8_t reg_h;
+#define REG_H_MASK GENMASK(15, 8)
+};
+#endif
+
 static int rk8xx_write(struct udevice *dev, uint reg, const uint8_t *buff,
  int len)
 {
int ret;
 
+#if CONFIG_IS_ENABLED(SPI) && CONFIG_IS_ENABLED(DM_SPI)
+   if (device_get_uclass_id(dev->parent) == UCLASS_SPI) {
+   struct spi_slave *spi = dev_get_parent_priv(dev);
+   struct rk806_cmd cmd = {
+   .op = 1,
+   .len = len - 1,
+   .reg_l = FIELD_GET(REG_L_MASK, reg),
+   .reg_h = FIELD_GET(REG_H_MASK, reg),
+   };
+
+   ret = dm_spi_claim_bus(dev);
+   if (ret) {
+   debug("Couldn't claim bus for device: %p!\n", dev);
+   return ret;
+   }
+
+   ret = spi_write_then_read(spi, (u8 *)&cmd, sizeof(cmd), buff, 
NULL, len);
+   if (ret)
+   debug("write error to device: %p register: %#x!\n",
+ dev, reg);
+
+   dm_spi_release_bus(dev);
+
+   return ret;
+   }
+#endif
+
ret = dm_i2c_write(dev, reg, buff, len);
if (ret) {
debug("write error to device: %p register: %#x!\n", dev, reg);
@@ -120,6 +174,33 @@ static int rk8xx_read(struct udevice *dev, uint reg, 
uint8_t *buff, int len)
 {
int ret;
 
+#if CONFIG_IS_ENABLED(SPI) && CONFIG_IS_ENABLED(DM_SPI)
+   if (device_get_uclass_id(dev->parent

[PATCH v4 06/16] pmic: reword help text

2024-03-14 Thread Quentin Schulz
From: Quentin Schulz 

Reword the help text for the pmic read and pmic write commands to better
match what's expected from the user.

Cc: Quentin Schulz 
Reviewed-by: Kever Yang 
Signed-off-by: Quentin Schulz 
---
 cmd/pmic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cmd/pmic.c b/cmd/pmic.c
index 49a405fa297..c9e9730adf9 100644
--- a/cmd/pmic.c
+++ b/cmd/pmic.c
@@ -225,6 +225,6 @@ U_BOOT_CMD(pmic, CONFIG_SYS_MAXARGS, 1, do_pmic,
"list  - list pmic devices\n"
"pmic dev [name]- show or [set] operating PMIC device\n"
"pmic dump  - dump registers\n"
-   "pmic read address  - read byte of register at address\n"
-   "pmic write address - write byte to register at address\n"
+   "pmic read - read byte of 'reg' register\n"
+   "pmic write   - write 'byte' byte to 'reg' register\n"
 );

-- 
2.44.0



[PATCH v4 04/16] regulator: rk8xx: add indirection level for some ldo callbacks

2024-03-14 Thread Quentin Schulz
From: Quentin Schulz 

By passing a rk8xx_reg_info directly to the internal get_value, it'd be
possible to call this same function with a logic for getting the
rk8xx_reg_info different from the current get_ldo_reg, e.g. for NLDO and
PLDO support for RK806.

No logic change is expected.

Cc: Quentin Schulz 
Reviewed-by: Kever Yang 
Signed-off-by: Quentin Schulz 
---
 drivers/power/regulator/rk8xx.c | 48 ++---
 1 file changed, 36 insertions(+), 12 deletions(-)

diff --git a/drivers/power/regulator/rk8xx.c b/drivers/power/regulator/rk8xx.c
index e905df3a800..212bb752a76 100644
--- a/drivers/power/regulator/rk8xx.c
+++ b/drivers/power/regulator/rk8xx.c
@@ -780,10 +780,8 @@ static int buck_get_enable(struct udevice *dev)
return _buck_get_enable(dev->parent, buck);
 }
 
-static int ldo_get_value(struct udevice *dev)
+static int _ldo_get_value(struct udevice *dev, const struct rk8xx_reg_info 
*info)
 {
-   int ldo = dev->driver_data - 1;
-   const struct rk8xx_reg_info *info = get_ldo_reg(dev->parent, ldo, 0);
int mask = info->vsel_mask;
int ret, val;
 
@@ -797,10 +795,16 @@ static int ldo_get_value(struct udevice *dev)
return info->min_uv + val * info->step_uv;
 }
 
-static int ldo_set_value(struct udevice *dev, int uvolt)
+static int ldo_get_value(struct udevice *dev)
 {
int ldo = dev->driver_data - 1;
-   const struct rk8xx_reg_info *info = get_ldo_reg(dev->parent, ldo, 
uvolt);
+   const struct rk8xx_reg_info *info = get_ldo_reg(dev->parent, ldo, 0);
+
+   return _ldo_get_value(dev, info);
+}
+
+static int _ldo_set_value(struct udevice *dev, const struct rk8xx_reg_info 
*info, int uvolt)
+{
int mask = info->vsel_mask;
int val;
 
@@ -812,16 +816,22 @@ static int ldo_set_value(struct udevice *dev, int uvolt)
else
val = ((uvolt - info->min_uv) / info->step_uv) + info->min_sel;
 
-   debug("%s: volt=%d, ldo=%d, reg=0x%x, mask=0x%x, val=0x%x\n",
- __func__, uvolt, ldo + 1, info->vsel_reg, mask, val);
+   debug("%s: volt=%d, reg=0x%x, mask=0x%x, val=0x%x\n",
+ __func__, uvolt, info->vsel_reg, mask, val);
 
return pmic_clrsetbits(dev->parent, info->vsel_reg, mask, val);
 }
 
-static int ldo_set_suspend_value(struct udevice *dev, int uvolt)
+static int ldo_set_value(struct udevice *dev, int uvolt)
 {
int ldo = dev->driver_data - 1;
const struct rk8xx_reg_info *info = get_ldo_reg(dev->parent, ldo, 
uvolt);
+
+   return _ldo_set_value(dev, info, uvolt);
+}
+
+static int _ldo_set_suspend_value(struct udevice *dev, const struct 
rk8xx_reg_info *info, int uvolt)
+{
int mask = info->vsel_mask;
int val;
 
@@ -833,16 +843,22 @@ static int ldo_set_suspend_value(struct udevice *dev, int 
uvolt)
else
val = ((uvolt - info->min_uv) / info->step_uv) + info->min_sel;
 
-   debug("%s: volt=%d, ldo=%d, reg=0x%x, mask=0x%x, val=0x%x\n",
- __func__, uvolt, ldo + 1, info->vsel_sleep_reg, mask, val);
+   debug("%s: volt=%d, reg=0x%x, mask=0x%x, val=0x%x\n",
+ __func__, uvolt, info->vsel_sleep_reg, mask, val);
 
return pmic_clrsetbits(dev->parent, info->vsel_sleep_reg, mask, val);
 }
 
-static int ldo_get_suspend_value(struct udevice *dev)
+static int ldo_set_suspend_value(struct udevice *dev, int uvolt)
 {
int ldo = dev->driver_data - 1;
-   const struct rk8xx_reg_info *info = get_ldo_reg(dev->parent, ldo, 0);
+   const struct rk8xx_reg_info *info = get_ldo_reg(dev->parent, ldo, 
uvolt);
+
+   return _ldo_set_suspend_value(dev->parent, info, uvolt);
+}
+
+static int _ldo_get_suspend_value(struct udevice *dev, const struct 
rk8xx_reg_info *info)
+{
int mask = info->vsel_mask;
int val, ret;
 
@@ -858,6 +874,14 @@ static int ldo_get_suspend_value(struct udevice *dev)
return info->min_uv + val * info->step_uv;
 }
 
+static int ldo_get_suspend_value(struct udevice *dev)
+{
+   int ldo = dev->driver_data - 1;
+   const struct rk8xx_reg_info *info = get_ldo_reg(dev->parent, ldo, 0);
+
+   return _ldo_get_suspend_value(dev->parent, info);
+}
+
 static int ldo_set_enable(struct udevice *dev, bool enable)
 {
int ldo = dev->driver_data - 1;

-- 
2.44.0



[PATCH v4 03/16] regulator: rk8xx: fix SWITCH enable on RK809

2024-03-14 Thread Quentin Schulz
From: William Wu 

On RK809 in PMIC_POWER_ENX registers, in order to set or clear a bit N,
the bit at offset N + 4 needs to be set otherwise nothing is done.

This fixes the inability to modify the SWITCH state on RK809.

Cc: Quentin Schulz 
Signed-off-by: William Wu 
[reworded commit log]
Reviewed-by: Kever Yang 
Signed-off-by: Quentin Schulz 
---
 drivers/power/regulator/rk8xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/power/regulator/rk8xx.c b/drivers/power/regulator/rk8xx.c
index 97d73ac95e0..e905df3a800 100644
--- a/drivers/power/regulator/rk8xx.c
+++ b/drivers/power/regulator/rk8xx.c
@@ -901,7 +901,7 @@ static int switch_set_enable(struct udevice *dev, bool 
enable)
case RK809_ID:
mask = (1 << (sw + 2)) | (1 << (sw + 6));
ret = pmic_clrsetbits(dev->parent, RK817_POWER_EN(3), mask,
- enable ? mask : 0);
+ enable ? mask : (1 << (sw + 6)));
break;
case RK818_ID:
mask = 1 << 6;

-- 
2.44.0



[PATCH v4 02/16] regulator: rk8xx: remove unused functions

2024-03-14 Thread Quentin Schulz
From: Quentin Schulz 

Those two functions had their last user removed in commit f9c68a566c4d
("rockchip: phycore_rk3288: remove phycore_init() function") part of
v2023.01 release, so let's do some cleanup here.

Cc: Quentin Schulz 
Reviewed-by: Kever Yang 
Signed-off-by: Quentin Schulz 
---
 drivers/power/regulator/rk8xx.c | 31 ---
 include/power/rk8xx_pmic.h  |  2 --
 2 files changed, 33 deletions(-)

diff --git a/drivers/power/regulator/rk8xx.c b/drivers/power/regulator/rk8xx.c
index e80bd6c3723..97d73ac95e0 100644
--- a/drivers/power/regulator/rk8xx.c
+++ b/drivers/power/regulator/rk8xx.c
@@ -210,14 +210,6 @@ static const struct rk8xx_reg_info rk818_ldo[] = {
 };
 #endif
 
-static const u16 rk818_chrg_cur_input_array[] = {
-   450, 800, 850, 1000, 1250, 1500, 1750, 2000, 2250, 2500, 2750, 3000
-};
-
-static const uint rk818_chrg_shutdown_vsel_array[] = {
-   278, 285, 292, 299, 306, 313, 319, 326
-};
-
 static const struct rk8xx_reg_info *get_buck_reg(struct udevice *pmic,
 int num, int uvolt)
 {
@@ -1160,26 +1152,3 @@ int rk8xx_spl_configure_buck(struct udevice *pmic, int 
buck, int uvolt)
 
return _buck_set_enable(pmic, buck, true);
 }
-
-int rk818_spl_configure_usb_input_current(struct udevice *pmic, int current_ma)
-{
-   uint i;
-
-   for (i = 0; i < ARRAY_SIZE(rk818_chrg_cur_input_array); i++)
-   if (current_ma <= rk818_chrg_cur_input_array[i])
-   break;
-
-   return pmic_clrsetbits(pmic, REG_USB_CTRL, RK818_USB_ILIM_SEL_MASK, i);
-}
-
-int rk818_spl_configure_usb_chrg_shutdown(struct udevice *pmic, int uvolt)
-{
-   uint i;
-
-   for (i = 0; i < ARRAY_SIZE(rk818_chrg_shutdown_vsel_array); i++)
-   if (uvolt <= rk818_chrg_shutdown_vsel_array[i])
-   break;
-
-   return pmic_clrsetbits(pmic, REG_USB_CTRL, RK818_USB_CHG_SD_VSEL_MASK,
-  i);
-}
diff --git a/include/power/rk8xx_pmic.h b/include/power/rk8xx_pmic.h
index 3cbfc021956..0db82419d4f 100644
--- a/include/power/rk8xx_pmic.h
+++ b/include/power/rk8xx_pmic.h
@@ -233,7 +233,5 @@ struct rk8xx_priv {
 };
 
 int rk8xx_spl_configure_buck(struct udevice *pmic, int buck, int uvolt);
-int rk818_spl_configure_usb_input_current(struct udevice *pmic, int 
current_ma);
-int rk818_spl_configure_usb_chrg_shutdown(struct udevice *pmic, int uvolt);
 
 #endif

-- 
2.44.0



[PATCH v4 01/16] rockchip: spi: rk_spi: do not write bytes when in read-only mode

2024-03-14 Thread Quentin Schulz
From: Quentin Schulz 

The read-only mode is currently supported but only for 16b-aligned
buffers. For unaligned buffers, the last byte will be read in RW mode
right now, which isn't what is desired. Instead, let's put the
controller back into RO mode for that last byte and skip any write in
the xfer loop.

This is required for 3-wire SPI mode where PICO/POCI lanes are shorted
on HW level. This incidentally the recommended design for RK806 PMIC for
RK3588 products.

Cc: Quentin Schulz 
Reviewed-by: Kever Yang 
Signed-off-by: Quentin Schulz 
---
 drivers/spi/rk_spi.c | 20 +---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/rk_spi.c b/drivers/spi/rk_spi.c
index 7de943356ad..c8694fdff95 100644
--- a/drivers/spi/rk_spi.c
+++ b/drivers/spi/rk_spi.c
@@ -453,8 +453,17 @@ static int rockchip_spi_xfer(struct udevice *dev, unsigned 
int bitlen,
 * case of read-only transfers by using the full 16bits of each
 * FIFO element.
 */
-   if (!out)
+   if (!out) {
ret = rockchip_spi_16bit_reader(dev, &in, &len);
+   /*
+* If "in" isn't 16b-aligned, we need to send the last byte
+* ourselves. We however need to have the controller in RO mode
+* which differs from the default.
+*/
+   clrsetbits_le32(®s->ctrlr0,
+   TMOD_MASK << TMOD_SHIFT,
+   TMOD_RO << TMOD_SHIFT);
+   }
 
/* This is the original 8bit reader/writer code */
while (len > 0) {
@@ -465,12 +474,13 @@ static int rockchip_spi_xfer(struct udevice *dev, 
unsigned int bitlen,
rkspi_enable_chip(regs, true);
 
toread = todo;
-   towrite = todo;
+   /* Only write if we have something to write */
+   towrite = out ? todo : 0;
while (toread || towrite) {
u32 status = readl(®s->sr);
 
if (towrite && !(status & SR_TF_FULL)) {
-   writel(out ? *out++ : 0, regs->txdr);
+   writel(*out++, regs->txdr);
towrite--;
}
if (toread && !(status & SR_RF_EMPT)) {
@@ -501,6 +511,10 @@ static int rockchip_spi_xfer(struct udevice *dev, unsigned 
int bitlen,
spi_cs_deactivate(dev, slave_plat->cs);
 
rkspi_enable_chip(regs, false);
+   if (!out)
+   clrsetbits_le32(®s->ctrlr0,
+   TMOD_MASK << TMOD_SHIFT,
+   TMOD_TR << TMOD_SHIFT);
 
return ret;
 }

-- 
2.44.0



[PATCH v4 00/16] rockchip: add support for SARADCv2 and RK806 PMIC and regulators

2024-03-14 Thread Quentin Schulz
The RK3588 has a new IP for SARADC compared to older SoCs of the same
vendor but most of the boilerplate is shared, so rockchip-saradc.c
driver is adapted instead of creating a new one.

Because the SARADC requires a vref-supply which is commonly coming from
a regulator of the RK806 PMIC, support for the RK806 PMIC and regulators
are also added in this patch series. Finally, RK806 PMIC is the first
one to actually support SPI as communication layer between the SoC and
the PMIC (but supports I2C as well according to the datasheet). This
adds support for RK806 over SPI though there should be no reason for
RK806 over I2C to not work.

Note that the DT now uses a different prefix for the buck and ldo node
names and the RK806 is the first one to use nldo and pldo and while they
both are LDOs, they have overlapping indices which makes it impossible
to simply reuse the rk8xx_ldo driver and need to define new ones (though
some callbacks can be reused, as is done in this patch series).

The SARADC was tested by toggling the BIOS button connected over channel
0 on RK3588 Jaguar and using the adc or button commands to watch over
the value.
The PMIC was tested with the pmic command for reading the CHIP_VERS
registers and writing to the WDT_REG (0x73).
The regulators were tested by using a modified DT for RK3588 Jaguar and
playing with the regulator command to enable/disable some buck/nldo/pldo
and modify their output voltage.

This has a light dependency on
https://lore.kernel.org/u-boot/20240221-jaguar-v3-0-1f256a822...@theobroma-systems.com/T/#t
because of:
 - remove of asm/io.h include in favor of asm/arch-rockchip/hardware.h
   (both should be included if applied before the dependency)
 - Jaguar defconfig modified in this patch series is added in the
   aforementioned patch series,

Signed-off-by: Quentin Schulz 
---
Changes in v4:
- use uint8_t instead of char to fix clang's
  -Wsingle-bit-bitfield-constant-conversion for the members of rk806_cmd
  data structure, they all are storing unsigned numbers anyway,
- Link to v3: 
https://lore.kernel.org/r/20240304-rk3588-saradc-v3-0-7424e2ed5...@theobroma-systems.com

Changes in v3:
- added ifdef safeguards for the spi sections in the rk8xx PMIC driver,
- removed SPL_PMIC_RK8XXX from Jaguar defconfig since it's doing nothing
  at the moment (issue around missing Kconfig symbol dependency, will be
  fixed in another patch series),
- Link to v2: 
https://lore.kernel.org/r/20240223-rk3588-saradc-v2-0-11c046b7e...@theobroma-systems.com

Changes in v2:
- fixed incorrect bitmasking when selecting channel
- added optional controller reset to match downstream U-Boot and both
  downstream and upstream Linux kernel logic,
- Link to v1: 
https://lore.kernel.org/r/20240221-rk3588-saradc-v1-0-c39a5601d...@theobroma-systems.com

---
Quentin Schulz (15):
  rockchip: spi: rk_spi: do not write bytes when in read-only mode
  regulator: rk8xx: remove unused functions
  regulator: rk8xx: add indirection level for some ldo callbacks
  power: rk8xx: add support for RK806
  pmic: reword help text
  rockchip: adc: rockchip-saradc: use union for preparing for v2
  rockchip: adc: rockchip-saradc: factor out channel_data callback
  rockchip: adc: rockchip-saradc: factor out start_channel callback
  rockchip: adc: rockchip-saradc: factor out stop callback
  rockchip: adc: rockchip-saradc: add support for RK3588
  power: pmic: rk8xx: fix duplicate prompt
  rockchip: jaguar-rk3588: enable SARADC and derivatives
  adc: add missing depends on ADC for controller drivers
  button: add missing ADC dependency for BUTTON_ADC
  rockchip: boot_mode: fix rockchip_dnl_key_pressed requiring ADC support

William Wu (1):
  regulator: rk8xx: fix SWITCH enable on RK809

 arch/arm/mach-rockchip/boot_mode.c |   4 +
 cmd/pmic.c |   4 +-
 configs/jaguar-rk3588_defconfig|  10 +-
 drivers/adc/Kconfig|   4 +
 drivers/adc/rockchip-saradc.c  | 202 +++--
 drivers/button/Kconfig |   1 +
 drivers/power/pmic/Kconfig |   2 +-
 drivers/power/pmic/rk8xx.c |  91 ++
 drivers/power/regulator/rk8xx.c| 595 ++---
 drivers/spi/rk_spi.c   |  20 +-
 include/power/rk8xx_pmic.h |  21 +-
 11 files changed, 879 insertions(+), 75 deletions(-)
---
base-commit: 40e06db4c0a9893d613adbe002e794115397a150
change-id: 20240212-rk3588-saradc-ab0a9bda9306

Best regards,
-- 
Quentin Schulz 



Re: [PATCH v3 00/17] video: dw_hdmi: Support Vendor PHY

2024-03-14 Thread Jagan Teki
Hi Anatolij,

On Mon, Feb 19, 2024 at 5:19 PM Jagan Teki  wrote:
>
> Hi Anatolij,
>
> On Wed, Jan 17, 2024 at 1:22 PM Jagan Teki  wrote:
> >
> > From: Jagan Teki 
> >
> > Unlike RK3399, Sunxi/Meson DW HDMI the new Rockchip SoC Rk3328 would
> > support external vendor PHY with DW HDMI chip.
> >
> > Support this vendor PHY by adding new platform PHY ops via DW HDMI
> > driver and call the respective generic phy from platform driver code.
> >
> > This series tested in RK3328 with 1080p (1920x1080) resolution.
> >
> > Patch 0001/0005: Support Vendor PHY
> > Patch 0006/0008: VOP extension for win, dsp offsets
> > Patch 0009/0010: RK3328 VOP, HDMI clocks
> > Patch 0011:  Rockchip Inno HDMI PHY
> > Patch 0012:  RK3328 HDMI driver
> > Patch 0013:  RK3328 VOP driver
> > Patch 0014/0017: Enable HDMI Out for RK3328
> >
> > Changes for v3:
> > - updated phy_ops logic
> > - tested in BPI-M64
> > - updated handoff logic for rk3328
> >
> > Changes for v2:
> > - Use proper cfg function for meson
> > - Add VOP cleanup code.
> > - Add DCLK get rate
> >
> > Linux VOP/HDMI out issues seems resolved with explicit WIN0 disable.
> >
> > Any inputs?
> > Jagan.
> >
> > Jagan Teki (17):
> >   video: rockchip: hdmi: Detect hpd after controller init
> >   video: dw_hdmi: Add Vendor PHY handling
> >   video: dw_hdmi: Extend the HPD detection
> >   video: dw_hdmi: Add read_hpd hook
> >   video: dw_hdmi: Add setup_hpd hook
> >   video: rockchip: vop: Simplify rkvop_enable
> >   video: rockchip: vop: Add win offset support
> >   video: rockchip: vop: Add dsp offset support
> >   clk: rockchip: rk3328: Add VOP clk support
> >   clk: rk3328: Add get hdmiphy clock
> >   phy: rockchip: Add Rockchip INNO HDMI PHY driver
> >   video: rockchip: Add rk3328 hdmi support
> >   video: rockchip: Add rk3328 vop support
> >   ARM: dts: rk3328: Enable VOP for bootph-all
> >   rockchip: Enable preconsole for rk3328
> >   configs: evb-rk3328: Enable vidconsole for rk3328
> >   configs: Enable HDMI Out for ROC-RK3328-CC
>
> Any comments on this? Or is it okay if I send a PR for this?

Any update?

Jagan.


Re: [PATCH v2 0/4] Add initial support for Microchip SAMA7G54 Curiosity board

2024-03-14 Thread Eugen Hristev
On 2/27/24 15:43, Mihai Sain wrote:
> This patch series adds initial support for Microchip SAMA7G54 Curiosity board.
> 
> Changes in v2:
> --
> 
> * Update flexcom 10 node in order to match previous flexcom definitions.
> * Sort in alphabetical order all nodes and pinctrl.
> * Remove status okay from leds, nand, eeprom, flash.
> * Add CONFIG_ENV_SIZE=0x4000 in mmc defconfig.
> * Set CONFIG_ENV_SECT_SIZE=0x1 in qspiflash defconfig.
> 
> Mihai Sain (4):
>   ARM: dts: at91: sama7g5: Add flexcom 10 node
>   ARM: dts: at91: sama7g54_curiosity: Add initial device tree of the board
>   board: at91: sama7g54_curiosity: Add initial board support
>   configs: at91: sama7g54_curiosity: Add initial default configs
> 
>  arch/arm/dts/Makefile |   3 +
>  .../dts/at91-sama7g54_curiosity-u-boot.dtsi   |  59 +
>  arch/arm/dts/at91-sama7g54_curiosity.dts  | 242 ++
>  arch/arm/dts/sama7g5.dtsi |  24 ++
>  arch/arm/mach-at91/Kconfig|  10 +
>  board/atmel/sama7g54_curiosity/Kconfig|  15 ++
>  board/atmel/sama7g54_curiosity/MAINTAINERS|   9 +
>  board/atmel/sama7g54_curiosity/Makefile   |   7 +
>  .../sama7g54_curiosity/sama7g54_curiosity.c   |  36 +++
>  configs/sama7g54_curiosity_mmc_defconfig  | 123 +
>  .../sama7g54_curiosity_nandflash_defconfig| 122 +
>  .../sama7g54_curiosity_qspiflash_defconfig| 122 +
>  include/configs/sama7g54_curiosity.h  |  17 ++
>  13 files changed, 789 insertions(+)
>  create mode 100644 arch/arm/dts/at91-sama7g54_curiosity-u-boot.dtsi
>  create mode 100644 arch/arm/dts/at91-sama7g54_curiosity.dts
>  create mode 100644 board/atmel/sama7g54_curiosity/Kconfig
>  create mode 100644 board/atmel/sama7g54_curiosity/MAINTAINERS
>  create mode 100644 board/atmel/sama7g54_curiosity/Makefile
>  create mode 100644 board/atmel/sama7g54_curiosity/sama7g54_curiosity.c
>  create mode 100644 configs/sama7g54_curiosity_mmc_defconfig
>  create mode 100644 configs/sama7g54_curiosity_nandflash_defconfig
>  create mode 100644 configs/sama7g54_curiosity_qspiflash_defconfig
>  create mode 100644 include/configs/sama7g54_curiosity.h
> 


Applied to u-boot-at91/next , thanks !


Re: [PATCH] arm64: zynqmp: Do not describe u-boot.itb if SPL is disabled

2024-03-14 Thread Michal Simek

Hi,

On 3/14/24 09:34, Ilias Apalodimas wrote:

Hi Michal

On Wed, 13 Mar 2024 at 09:01, Ilias Apalodimas
 wrote:


On Wed, 13 Mar 2024 at 08:42, Michal Simek  wrote:




On 3/12/24 20:12, Ilias Apalodimas wrote:

On Tue, 12 Mar 2024 at 17:55, Michal Simek  wrote:




On 3/12/24 07:14, Ilias Apalodimas wrote:

Hi Michal

Apologies for the late reply

On Wed, 6 Mar 2024 at 09:48, Michal Simek  wrote:




On 3/5/24 16:47, Ilias Apalodimas wrote:

On Fri, Feb 23, 2024 at 05:18:42PM +0100, Michal Simek wrote:

There is no reason to describe u-boot.itb on system without SPL. Pretty
much this is cover all systems which are using only boot.bin which contains
all images inside.

Signed-off-by: Michal Simek 
---

 board/xilinx/common/board.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
index 9641ed307b75..4f38b7d27684 100644
--- a/board/xilinx/common/board.c
+++ b/board/xilinx/common/board.c
@@ -43,7 +43,7 @@ struct efi_fw_image fw_images[] = {
.image_index = 1,
},
 #endif
-#if defined(XILINX_UBOOT_IMAGE_GUID)
+#if defined(XILINX_UBOOT_IMAGE_GUID) && 
defined(CONFIG_SPL_FS_LOAD_PAYLOAD_NAME)


What happens if this is defined with CONFIG_SPL_FS_LOAD_PAYLOAD_NAME="" ?


Your comment is valid but I am not aware about any CONFIG_IS, etc which checks
that string is not empty. If name is "" it will return yes and second image is
doing to be defined.

But I found handling in the code like this.

 36 #ifdef CONFIG_DEFAULT_FDT_FILE
 37 if (strlen(CONFIG_DEFAULT_FDT_FILE)) {

which can be used in my second patch not to describe second image in
set_dfu_alt_info() if string is empty.


Yes, I think that's ok. The problem is that if we merge this as-is, we
would have to disable CONFIG_SPL_FS_FAT to make this work, which is a
bit misleading


As Heinrich said not just this if you want to do it like this.
I think you will simply disable the whole SPL which will disable this symbol 
too.
But from my perspective SPL payload name is driving this option. Data can end up
on partition or in raw mode but for dfu you need to use the name.


Yes, but isn't SPL selected by the Kconfig automatically? I can't seem
to be able to disable it for the kria platforms


Not in upstream but via your/AMD build in meta-ts.

Thanks,
Michal



Reviewed-by: Ilias Apalodimas 


Trying to compile xilinx_zynqmp_kria_defconfig with CONFIG_SPL unset
blows up with

  HOSTCC  scripts/dtc/dtc-lexer.lex.o
   HOSTCC  scripts/dtc/dtc-parser.tab.o
   COPYu-boot.its
cp: missing destination file operand after 'u-boot.its'
Try 'cp --help' for more information.
make: *** [Makefile:1405: u-boot.its] Error 1
make: *** Waiting for unfinished jobs
   HOSTLD  scripts/dtc/dtc


That's because u-boot.itb is selected in .config as target binary.
Because that entry is string, setup by default when SPL is enabled via 
defconfig. Then when you disable SPL via defconfig default setting is not 
changed and is still at origin value.

CONFIG_BUILD_TARGET="u-boot.itb"

If you do sed -i '/CONFIG_SPL/d' configs/xilinx_zynqmp_kria_defconfig
you will get that CONFIG_BUILD_TARGET=""
which is correct value without SPL.

Thanks,
Michal



Re: [PATCH] arm64: zynqmp: Do not describe u-boot.itb if SPL is disabled

2024-03-14 Thread Ilias Apalodimas
Hi Michal

On Wed, 13 Mar 2024 at 09:01, Ilias Apalodimas
 wrote:
>
> On Wed, 13 Mar 2024 at 08:42, Michal Simek  wrote:
> >
> >
> >
> > On 3/12/24 20:12, Ilias Apalodimas wrote:
> > > On Tue, 12 Mar 2024 at 17:55, Michal Simek  wrote:
> > >>
> > >>
> > >>
> > >> On 3/12/24 07:14, Ilias Apalodimas wrote:
> > >>> Hi Michal
> > >>>
> > >>> Apologies for the late reply
> > >>>
> > >>> On Wed, 6 Mar 2024 at 09:48, Michal Simek  wrote:
> > 
> > 
> > 
> >  On 3/5/24 16:47, Ilias Apalodimas wrote:
> > > On Fri, Feb 23, 2024 at 05:18:42PM +0100, Michal Simek wrote:
> > >> There is no reason to describe u-boot.itb on system without SPL. 
> > >> Pretty
> > >> much this is cover all systems which are using only boot.bin which 
> > >> contains
> > >> all images inside.
> > >>
> > >> Signed-off-by: Michal Simek 
> > >> ---
> > >>
> > >> board/xilinx/common/board.c | 2 +-
> > >> 1 file changed, 1 insertion(+), 1 deletion(-)
> > >>
> > >> diff --git a/board/xilinx/common/board.c 
> > >> b/board/xilinx/common/board.c
> > >> index 9641ed307b75..4f38b7d27684 100644
> > >> --- a/board/xilinx/common/board.c
> > >> +++ b/board/xilinx/common/board.c
> > >> @@ -43,7 +43,7 @@ struct efi_fw_image fw_images[] = {
> > >>.image_index = 1,
> > >>},
> > >> #endif
> > >> -#if defined(XILINX_UBOOT_IMAGE_GUID)
> > >> +#if defined(XILINX_UBOOT_IMAGE_GUID) && 
> > >> defined(CONFIG_SPL_FS_LOAD_PAYLOAD_NAME)
> > >
> > > What happens if this is defined with 
> > > CONFIG_SPL_FS_LOAD_PAYLOAD_NAME="" ?
> > 
> >  Your comment is valid but I am not aware about any CONFIG_IS, etc 
> >  which checks
> >  that string is not empty. If name is "" it will return yes and second 
> >  image is
> >  doing to be defined.
> > 
> >  But I found handling in the code like this.
> > 
> >  36 #ifdef CONFIG_DEFAULT_FDT_FILE
> >  37 if (strlen(CONFIG_DEFAULT_FDT_FILE)) {
> > 
> >  which can be used in my second patch not to describe second image in
> >  set_dfu_alt_info() if string is empty.
> > >>>
> > >>> Yes, I think that's ok. The problem is that if we merge this as-is, we
> > >>> would have to disable CONFIG_SPL_FS_FAT to make this work, which is a
> > >>> bit misleading
> > >>
> > >> As Heinrich said not just this if you want to do it like this.
> > >> I think you will simply disable the whole SPL which will disable this 
> > >> symbol too.
> > >> But from my perspective SPL payload name is driving this option. Data 
> > >> can end up
> > >> on partition or in raw mode but for dfu you need to use the name.
> > >
> > > Yes, but isn't SPL selected by the Kconfig automatically? I can't seem
> > > to be able to disable it for the kria platforms
> >
> > Not in upstream but via your/AMD build in meta-ts.
> >
> > Thanks,
> > Michal
> >
>
> Reviewed-by: Ilias Apalodimas 

Trying to compile xilinx_zynqmp_kria_defconfig with CONFIG_SPL unset
blows up with

 HOSTCC  scripts/dtc/dtc-lexer.lex.o
  HOSTCC  scripts/dtc/dtc-parser.tab.o
  COPYu-boot.its
cp: missing destination file operand after 'u-boot.its'
Try 'cp --help' for more information.
make: *** [Makefile:1405: u-boot.its] Error 1
make: *** Waiting for unfinished jobs
  HOSTLD  scripts/dtc/dtc

Cheers
/Ilias


Re: [PATCH v3 2/7] arm: clean up v7 and v8 linker scripts for bss_start/end

2024-03-14 Thread Ilias Apalodimas
Hi Richard,


On Thu, 14 Mar 2024 at 00:43, Richard Henderson
 wrote:
>
> On 3/13/24 11:43, Ilias Apalodimas wrote:
> > Hi Richard,
> >
> > On Wed, 13 Mar 2024 at 22:19, Richard Henderson
> >  wrote:
> >>
> >> On 3/13/24 06:23, Ilias Apalodimas wrote:
> >>> +++ b/arch/arm/cpu/armv8/u-boot-spl.lds
> >>> @@ -63,18 +63,11 @@ SECTIONS
> >>>
> >>>_image_binary_end = .;
> >>>
> >>> - .bss_start (NOLOAD) : {
> >>> - . = ALIGN(8);
> >>> - KEEP(*(.__bss_start));
> >>> - } >.sdram
> >>> -
> >>> - .bss (NOLOAD) : {
> >>> + .bss : {
> >>> + __bss_start = .;
> >>>*(.bss*)
> >>> -  . = ALIGN(8);
> >>> - } >.sdram
> >>> -
> >>> - .bss_end (NOLOAD) : {
> >>> - KEEP(*(.__bss_end));
> >>> + . = ALIGN(8);
> >>> + __bss_end = .;
> >>
> >> Still missing the alignment on .bss, previously in .bss_start.
> >
> > Since this is emitted in .sdram memory I can't define it as
> > .bss ALIGN(8) : {} since the calculated memory will be outside the
> > sdram-defined region
> >
> > I could define it as
> > .bss : {
> >  . = ALIGN(8);
> >  __bss_start = .;
> >  ..
> > }
> >
> > But instead, I added an assert at the bottom which will break the
> > linking if the __bss_start is not 8byte aligned.
>
> I think it would be clearer to assert on __bss_start address rather than
> CONFIG_SPL_BSS_START_ADDR, if that's possible.  If not, then the constant 
> will have to do.


Fair enough and it is possible. I can convert that assertion to
ASSERT(ADDR(.bss) % 8 == 0, ".bss must be 8-byte aligned");

Keep in mind that .bss, due to the linker aligning the section to the
strictest input section requirement will end up aligning that to 8b
regardless. IOW compiling with CONFIG_SPL_BSS_START_ADDR=0x1001, won't
break the linking since .bss will end up at 0x1008.

Testing the assertion with a section definition which looks like this
will trigger the assert.
.bss 0x1001 : {
__bss_start = .;
*(.bss*)
. = ALIGN(8);
__bss_end = .;
} >.sdram

I don't have a really strong opinion here. I think the assertion above
is ok. What we could also do is

.bss  : {
. = ALIGN(8);
__bss_start = .;
*(.bss*)
. = ALIGN(8);
__bss_end = .;
} >.sdram

which would also fix the __bss_start alignment regardless of the .bss
output address.
For example with the linker entry below __bss_start will end up at 0x1008
.bss  0x1001 : {
. = ALIGN(8);
__bss_start = .;
*(.bss*)
. = ALIGN(8);
__bss_end = .;
} >.sdram

As you already mentioned, the bss_end - bss_start % 8 == 0 can be
fixed by using memset. So since I plan to continue working on this to
get RO, RW^x etc mappings I think I'll just go for the assertion for
now -- but test the .bss address instead of the config option.

Anyone objects?

Thanks
/Ilias

>
>
> r~


Re: [PATCH 2/5] mmc: zynq-sdhci: refactor tapdelay settings

2024-03-14 Thread Steffen Dirkwinkel
Hi Love,

On Thu, 2024-03-14 at 12:15 +0530, Love Kumar wrote:
> Hi,
> 
> When we run in el3 for zynqmp board, we are seeing below issue with this 
> patch:
> 
> 
> Model: ZynqMP MINI EMMC0
> Board: Xilinx ZynqMP
> DRAM:  512 MiB
> EL Level:  EL3
> Secure Boot:   not authenticated, not encrypted
> Multiboot: 0
> Core:  10 devices, 9 uclasses, devicetree: embed
> MMC:   sdhci@ff16: 0
> Loading Environment from ... OK
> In:    dcc
> Out:   dcc
> Err:   dcc
> ZynqMP> mmc list
> sdhci@ff16: 0
> ZynqMP> mmc dev 0 0
> arasan_sdhci sdhci@ff16: Error setting Input Tap Delay
> sdhci_set_clock: Error while setting tap delay
> sdhci_send_command: Timeout for status update:  0001
> ZynqMP>

Thank you for testing this.
It looks like the zynqmp-mini-emmc0 device lacks the power-domain node and 
doesn't have
a sd node id. The code before my change would have ignored the unknown node id, 
applied
settings for sdhci1 instead of sdhci0 and returned without an error. Maybe 
there's a different
way to find out which sdhci we want to operate on?

Can you try setting the node id in device tree?
Something like this:

diff --git a/arch/arm/dts/zynqmp-mini-emmc0.dts 
b/arch/arm/dts/zynqmp-mini-emmc0.dts
index 02e80bd85e1..87c4a1b6ad0 100644
--- a/arch/arm/dts/zynqmp-mini-emmc0.dts
+++ b/arch/arm/dts/zynqmp-mini-emmc0.dts
@@ -7,6 +7,8 @@
  * Siva Durga Prasad Paladugu 
  */
 
+#include 
+
 /dts-v1/;
 
 / {
@@ -41,6 +43,12 @@
clock-frequency = <2>;
};
 
+   firmware {
+   zynqmp_firmware: zynqmp-firmware {
+   #power-domain-cells = <1>;
+   };
+   };
+
amba: amba {
compatible = "simple-bus";
#address-cells = <2>;
@@ -56,6 +64,7 @@
reg = <0x0 0xff16 0x0 0x1000>;
clock-names = "clk_xin", "clk_ahb";
clocks = <&clk_xin &clk_xin>;
+   power-domains = <&zynqmp_firmware PD_SD_0>;
};
};
 };

Thanks,
Steffen

> 
> Regards,
> Love Kumar
> 
> On 23/02/24 7:36 pm, Steffen Dirkwinkel wrote:
> > From: Steffen Dirkwinkel 
> > 
> > Previously we were setting in tapdelay for SD1 every time even if SD0 was
> > requested.
> > The SD tapdelay settings are shifted by 16 bits between SD0 and SD1.
> > We can use that to make our tapdelay setup simpler. This is also how it
> > currently works in arm-trusted-firmware.
> > 
> > Signed-off-by: Steffen Dirkwinkel 
> > ---
> > 
> >   drivers/mmc/zynq_sdhci.c | 65 +---
> >   1 file changed, 28 insertions(+), 37 deletions(-)
> > 
> > diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
> > index 935540d1719..d4845245b2a 100644
> > --- a/drivers/mmc/zynq_sdhci.c
> > +++ b/drivers/mmc/zynq_sdhci.c
> > @@ -42,14 +42,11 @@
> >   #define SD_OTAP_DLY   0xFF180318
> >   #define SD0_DLL_RST   BIT(2)
> >   #define SD1_DLL_RST   BIT(18)
> > +#define SD1_TAP_OFFSET 16
> >   #define SD0_ITAPCHGWINBIT(9)
> > -#define SD1_ITAPCHGWIN BIT(25)
> >   #define SD0_ITAPDLYENABIT(8)
> > -#define SD1_ITAPDLYENA BIT(24)
> >   #define SD0_ITAPDLYSEL_MASK   GENMASK(7, 0)
> > -#define SD1_ITAPDLYSEL_MASKGENMASK(23, 16)
> >   #define SD0_OTAPDLYSEL_MASK   GENMASK(5, 0)
> > -#define SD1_OTAPDLYSEL_MASKGENMASK(21, 16)
> >   
> >   #define MIN_PHY_CLK_HZ5000
> >   
> > @@ -275,44 +272,32 @@ static int arasan_sdhci_config_dll(struct sdhci_host 
> > *host, unsigned int clock,
> >   static inline int arasan_zynqmp_set_in_tapdelay(u32 node_id, u32 
> > itap_delay)
> >   {
> >     int ret;
> > +   u32 shift;
> >   
> > -   if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3) {
> > -   if (node_id == NODE_SD_0) {
> > -   ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPCHGWIN,
> > -   SD0_ITAPCHGWIN);
> > -   if (ret)
> > -   return ret;
> > -
> > -   ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPDLYENA,
> > -   SD0_ITAPDLYENA);
> > -   if (ret)
> > -   return ret;
> > -
> > -   ret = zynqmp_mmio_write(SD_ITAP_DLY, 
> > SD0_ITAPDLYSEL_MASK,
> > -   itap_delay);
> > -   if (ret)
> > -   return ret;
> > +   if (node_id == NODE_SD_0)
> > +   shift = 0;
> > +   else if (node_id == NODE_SD_1)
> > +   shift = SD1_TAP_OFFSET;
> > +   else
> > +   return -EINVAL;
> >   
> > -   ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPCHGWIN, 0);
> > -   if (ret)
> > -  

Re: [PATCH] imx8m*_venice: move venice to OF_UPSTREAM

2024-03-14 Thread Sumit Garg
+ Tom

Hi Tim,

On Wed, 13 Mar 2024 at 22:01, Tim Harvey  wrote:
>
> On Wed, Mar 13, 2024 at 6:20 AM Sumit Garg  wrote:
> >
> > On Wed, 13 Mar 2024 at 06:46, Fabio Estevam  wrote:
> > >
> > > Hi Tim,
> > >
> > > On Tue, Mar 12, 2024 at 4:05 PM Tim Harvey  wrote:
> > > >
> > > > Move to imx8m{m,n,p}-venice to OF_UPSTREAM:
> > > >  - replace the non-upstream generic imx8m{m,n,p}-venice dt with one of 
> > > > the
> > > >dt's from the OF_LIST
> > > >  - handle the fact that dtbs now have a 'freescale/' prefix
> > > >  - imply OF_UPSTREAM
> > > >  - remove rudundant files from arch/arm/dts leaving only the
> > > >*-u-boot.dtsi files
> > > >
> > > > Signed-off-by: Tim Harvey 
> > > ...
> > > >  33 files changed, 13 insertions(+), 10658 deletions(-)
> > >
> > > This diff looks great :-)
> >
> > +1
> >
> > Reviewed-by: Sumit Garg 
> >
>
> Hi Sumit,
>
> Thanks for your work on this - I imagine over time this will
> de-duplicate a lot of work!
>
> I have a couple of questions about OF_UPSTREAM I haven't found the
> answer to yet:
> 1. how do you determine what the last sync point was in dts/upstream?
> (ie what kernel version is it currently sync'd with)

You can get that information from merge commits that happen over
dts/upstream sub-directory. It was somewhat raw information for the
first time we added dts/upstream as (commit aaba2d45dc2a pointing to
v6.7-dts [1]):

commit 53633a893a06bd5a0c807287d9cc29337806eaf7
Author: Tom Rini 
Date:   Thu Feb 29 12:33:36 2024 -0500

Squashed 'dts/upstream/' content from commit aaba2d45dc2a

git-subtree-dir: dts/upstream
git-subtree-split: aaba2d45dc2a1b3bbb710f2a3808ee1c9f340abe

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git/tag/?h=v6.7-dts

However, further syncs would be more clear from merge commit
description, try following:

$ ./dts/update-dts-subtree.sh pull v6.8-dts

You will see merge commit saying:

commit 57508745cd2b07e55b5c414c6d646de4865418d2 (HEAD -> dt_uprev)
Merge: 3987e15e88a 14c9e8c0856
Author: Sumit Garg 
Date:   Thu Mar 14 12:20:27 2024 +0530

Subtree merge tag 'v6.8-dts' of devicetree-rebasing repo [1] into
dts/upstream

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git/

> 2. how often will dts/upstream get re-synced (not the
> devicetree-rebasing.git but u-boot dts/upstream),

You can see the re-syncing details here (section: "Resyncing with
devicetree-rebasing",
https://source.denx.de/u-boot/u-boot/-/blob/next/doc/develop/devicetree/control.rst?ref_type=heads).

> who do you suspect
> will do be doing it?

Tom will be doing that. I suppose we are just in time for a resync
since the Linux kernel v6.8 was released.

Tom,

Do you think we can make v6.8-dts sync for the U-Boot next?

> 3. how would one go about adding a new feature via dt to uboot when
> the same feature has not yet landed in dts upstream? perhaps the
> answer is it must land upstream first or do you suspect it would be ok
> to put something in a u-boot.dtsi that can later get removed as
> redundant?

>From an upstream perspective, we should aim for minimal contents in
*-u-boot.dtsi, ideally it would be better to get rid of them. For eg.
all bootph_* properties should all be pushed upstream.

However, from the new features perspective we can consider using
*-u-boot.dtsi if the changes are minimal like enabling some DT nodes.
But if the changes are significant then that board can just opt out of
OF_USTREAM but still reusing all the DT includes. The reasoning behind
this is to minimize any chances of breakage if DT features in
*-u-boot.dtsi diverge from DT upstream.

> I ask mainly for being able to add things quickly to a
> downstream U-Boot repo that lags behind upstream U-Boot
>

Working downstream can always be fun, allowing additional options (can
be useful for advance testing too):

1. You can play around with $ ./dts/update-dts-subtree.sh pull 
2. You can manually sync the vendor directories
(dts/upstream/src///).
3. You can manually sync/update the individual DTS files too.

-Sumit

> Best Regards,
>
> Tim


Re: [PATCH v2] rockchip: load env from boot MMC device

2024-03-14 Thread Kever Yang



On 2024/3/8 11:00, Ben Wolsieffer wrote:

Currently, if the environment is stored on an MMC device, the device
number is hardcoded by CONFIG_SYS_MMC_ENV_DEV. This is problematic
because many boards can choose between booting from an SD card or a
removable eMMC. For example, the Rock64 defconfig sets
CONFIG_SYS_MMC_ENV_DEV=1, which corresponds to the SD card. If an eMMC
is used as the boot device and no SD card is installed, it is impossible
to save the environment.

To avoid this problem, we can choose the environment MMC device based on
the boot device. The theobroma-systems boards already contain code to do
this, so this commit simply moves it to the common Rockchip board file,
with some refactoring. I also removed another implementation of
mmc_get_env_dev() from tinker_rk3288 that performed MMC boot device
detection by reading a bootrom register.

This has been tested on a Rock64v2.

Signed-off-by: Ben Wolsieffer 

Reviewed-by: Kever Yang 

Thanks,
- Kever

---
v2:
* Use #ifdef rather than if(CONFIG_IS_ENABLED(...))
* Add a debug message if boot device is not found

  arch/arm/mach-rockchip/board.c   | 31 
  board/rockchip/tinker_rk3288/tinker-rk3288.c | 12 
  board/theobroma-systems/common/common.c  | 30 ---
  3 files changed, 31 insertions(+), 42 deletions(-)

diff --git a/arch/arm/mach-rockchip/board.c b/arch/arm/mach-rockchip/board.c
index 2620530e03..8aa09f44b3 100644
--- a/arch/arm/mach-rockchip/board.c
+++ b/arch/arm/mach-rockchip/board.c
@@ -6,6 +6,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -349,3 +350,33 @@ __weak int board_rng_seed(struct abuf *buf)
return 0;
  }
  #endif
+
+int mmc_get_env_dev(void)
+{
+   int devnum;
+   const char *boot_device;
+   struct udevice *dev;
+
+#ifdef CONFIG_SYS_MMC_ENV_DEV
+   devnum = CONFIG_SYS_MMC_ENV_DEV;
+#else
+   devnum = 0;
+#endif
+
+   boot_device = ofnode_read_chosen_string("u-boot,spl-boot-device");
+   if (!boot_device) {
+   debug("%s: /chosen/u-boot,spl-boot-device not set\n", __func__);
+   return devnum;
+   }
+
+   debug("%s: booted from %s\n", __func__, boot_device);
+
+   if (uclass_find_device_by_ofnode(UCLASS_MMC, ofnode_path(boot_device), 
&dev)) {
+   debug("%s: no U-Boot device found for %s\n", __func__, 
boot_device);
+   return devnum;
+   }
+
+   devnum = dev->seq_;
+   debug("%s: get MMC env from mmc%d\n", __func__, devnum);
+   return devnum;
+}
diff --git a/board/rockchip/tinker_rk3288/tinker-rk3288.c 
b/board/rockchip/tinker_rk3288/tinker-rk3288.c
index f85209c649..eff3a00c30 100644
--- a/board/rockchip/tinker_rk3288/tinker-rk3288.c
+++ b/board/rockchip/tinker_rk3288/tinker-rk3288.c
@@ -11,8 +11,6 @@
  #include 
  #include 
  #include 
-#include 
-#include 
  
  static int get_ethaddr_from_eeprom(u8 *addr)

  {
@@ -38,13 +36,3 @@ int rk3288_board_late_init(void)
  
  	return 0;

  }
-
-int mmc_get_env_dev(void)
-{
-   u32 bootdevice_brom_id = readl(BROM_BOOTSOURCE_ID_ADDR);
-
-   if (bootdevice_brom_id == BROM_BOOTSOURCE_EMMC)
-   return 0;
-
-   return 1;
-}
diff --git a/board/theobroma-systems/common/common.c 
b/board/theobroma-systems/common/common.c
index 864bcdd46f..585da43884 100644
--- a/board/theobroma-systems/common/common.c
+++ b/board/theobroma-systems/common/common.c
@@ -89,36 +89,6 @@ int setup_boottargets(void)
return 0;
  }
  
-int mmc_get_env_dev(void)

-{
-   const char *boot_device =
-   ofnode_read_chosen_string("u-boot,spl-boot-device");
-   struct udevice *devp;
-
-   if (!boot_device) {
-   debug("%s: /chosen/u-boot,spl-boot-device not set\n",
- __func__);
-#ifdef CONFIG_SYS_MMC_ENV_DEV
-   return CONFIG_SYS_MMC_ENV_DEV;
-#else
-   return 0;
-#endif
-   }
-
-   debug("%s: booted from %s\n", __func__, boot_device);
-
-   if (uclass_find_device_by_ofnode(UCLASS_MMC, ofnode_path(boot_device), 
&devp))
-#ifdef CONFIG_SYS_MMC_ENV_DEV
-   return CONFIG_SYS_MMC_ENV_DEV;
-#else
-   return 0;
-#endif
-
-   debug("%s: get MMC ENV from mmc%d\n", __func__, devp->seq_);
-
-   return devp->seq_;
-}
-
  enum env_location arch_env_get_location(enum env_operation op, int prio)
  {
const char *boot_device =


Re: [PATCH] rockchip: board: Add minimal generic RK3588S/RK3588 board

2024-03-14 Thread Jonas Karlman
Hi Kever,

On 2024-03-14 08:33, Kever Yang wrote:
> Hi Jonas,
> 
> On 2024/3/14 15:04, Jonas Karlman wrote:
>> Hi Kever,
>>
>> On 2024-03-14 07:58, Kever Yang wrote:
>>> On 2024/2/1 06:08, Jonas Karlman wrote:
 Add a minimal generic RK3588S/RK3588 board that only have eMMC and SDMMC
 enabled. This defconfig can be used to boot from eMMC or SD-card on most
 RK3588S/RK3588 boards that follow reference board design.

 Also fix the alphabetical order of RK3588 boards listed in Makefile and
 documentation.

 Signed-off-by: Jonas Karlman 
>>> Since rk356x already have a generic board, rk3588 can also have one.
>>>
>>> Reviewed-by: Kever Yang 
>> Thanks, I will send a v2 with an updated defconfig to take advantage of
>> the new ROCKCHIP_COMMON_STACK_ADDR bss and stack addresses.
> 
> I have fix this and also other new boards defconfig, so you don't need 
> to do this.

Great, thanks!

Regards,
Jonas

> 
> 
> Thanks,
> 
> - Kever
> 
>>
>> Regards,
>> Jonas
>>
>>> Thanks,
>>> - Kever
>>>
 ---
 This patch depend on the series "rockchip: rk35xx: Sync device tree
 with linux v6.8-rc1" [1].

 [1] https://patchwork.ozlabs.org/cover/1891669/
 ---
arch/arm/dts/Makefile   |  5 +-
arch/arm/dts/rk3588-generic-u-boot.dtsi |  3 ++
arch/arm/dts/rk3588-generic.dts | 44 
board/rockchip/evb_rk3588/MAINTAINERS   |  7 +++
configs/generic-rk3588_defconfig| 68 +
doc/board/rockchip/rockchip.rst |  3 +-
6 files changed, 127 insertions(+), 3 deletions(-)
create mode 100644 arch/arm/dts/rk3588-generic-u-boot.dtsi
create mode 100644 arch/arm/dts/rk3588-generic.dts
create mode 100644 configs/generic-rk3588_defconfig

>> [snip]



Re: [PATCH] rockchip: board: Add minimal generic RK3588S/RK3588 board

2024-03-14 Thread Kever Yang

Hi Jonas,

On 2024/3/14 15:04, Jonas Karlman wrote:

Hi Kever,

On 2024-03-14 07:58, Kever Yang wrote:

On 2024/2/1 06:08, Jonas Karlman wrote:

Add a minimal generic RK3588S/RK3588 board that only have eMMC and SDMMC
enabled. This defconfig can be used to boot from eMMC or SD-card on most
RK3588S/RK3588 boards that follow reference board design.

Also fix the alphabetical order of RK3588 boards listed in Makefile and
documentation.

Signed-off-by: Jonas Karlman 

Since rk356x already have a generic board, rk3588 can also have one.

Reviewed-by: Kever Yang 

Thanks, I will send a v2 with an updated defconfig to take advantage of
the new ROCKCHIP_COMMON_STACK_ADDR bss and stack addresses.


I have fix this and also other new boards defconfig, so you don't need 
to do this.



Thanks,

- Kever



Regards,
Jonas


Thanks,
- Kever


---
This patch depend on the series "rockchip: rk35xx: Sync device tree
with linux v6.8-rc1" [1].

[1] https://patchwork.ozlabs.org/cover/1891669/
---
   arch/arm/dts/Makefile   |  5 +-
   arch/arm/dts/rk3588-generic-u-boot.dtsi |  3 ++
   arch/arm/dts/rk3588-generic.dts | 44 
   board/rockchip/evb_rk3588/MAINTAINERS   |  7 +++
   configs/generic-rk3588_defconfig| 68 +
   doc/board/rockchip/rockchip.rst |  3 +-
   6 files changed, 127 insertions(+), 3 deletions(-)
   create mode 100644 arch/arm/dts/rk3588-generic-u-boot.dtsi
   create mode 100644 arch/arm/dts/rk3588-generic.dts
   create mode 100644 configs/generic-rk3588_defconfig


[snip]


Re: [PATCH v3 3/6] rockchip: Use common bss and stack addresses on RK3328

2024-03-14 Thread Jonas Karlman
Hi Kever,

On 2024-03-13 11:39, Kever Yang wrote:
> Hi Jonas,
> 
>      This patch does not able to apply on next, could you help to take a 
> look.
> 
> And also add document of memory layout in rockchip.rst if possible.

I will send a follow-up patch that adds details about this memory layout
and the new Kconfig option to the rockchip documentation.

Regards,
Jonas

> 
> 
> Thanks,
> 
> - Kever
> 
> On 2024/3/3 03:16, Jonas Karlman wrote:
>> With the stack and text base used by U-Boot SPL and proper on RK3328
>> there is a high likelihood of overlapping when U-Boot proper + FDT nears
>> or exceeded 1 MiB in size.
>>
>> Currently the following memory layout is typically used on RK3328:
>> [0, 256K) - SPL binary
>> [ 256K,   2M) - TF-A / reserved
>> [   2M,   +X) - U-Boot proper binary (TEXT_BASE)
>> [   -X,   3M) - U-Boot proper pre-reloc stack (CUSTOM_SYS_INIT_SP_ADDR)
>> [  -8K,   3M)   - pre-reloc malloc heap (SYS_MALLOC_F_LEN)
>> [   -X,   4M) - SPL pre-reloc stack (SPL_STACK)
>> [  -8K,   4M)   - pre-reloc malloc heap (SPL_SYS_MALLOC_F_LEN)
>> [   -X,   6M) - SPL reloc stack (SPL_STACK_R_ADDR)
>> [   5M,   6M)   - reloc malloc heap (SPL_STACK_R_MALLOC_SIMPLE_LEN)
>> [  32M,  +8K) - SPL bss (SPL_BSS_START_ADDR, SPL_BSS_MAX_SIZE)
>>
>> SPL can safely load U-Boot proper + FDT to [2M, 4M-8K) with this layout.
>> However, the stack at [-X, 3M) used during U-Boot proper pre-reloc is
>> restricting the safe size of U-Boot proper + FDT to be less than 1 MiB.
>>
>> Migrate to use common bss, stack and malloc heap size and addresses to
>> fix this restriction and allow for a larger U-Boot proper image size.
>>
>> Signed-off-by: Jonas Karlman 
>> ---
>>   arch/arm/mach-rockchip/rk3328/Kconfig | 11 ---
>>   configs/evb-rk3328_defconfig  | 17 -
>>   configs/nanopi-r2c-plus-rk3328_defconfig  | 15 ---
>>   configs/nanopi-r2c-rk3328_defconfig   | 15 ---
>>   configs/nanopi-r2s-rk3328_defconfig   | 15 ---
>>   configs/orangepi-r1-plus-lts-rk3328_defconfig | 15 ---
>>   configs/orangepi-r1-plus-rk3328_defconfig | 15 ---
>>   configs/roc-cc-rk3328_defconfig   | 15 ---
>>   configs/rock-pi-e-rk3328_defconfig| 17 -
>>   configs/rock64-rk3328_defconfig   | 15 ---
>>   10 files changed, 4 insertions(+), 146 deletions(-)
>>

[snip]


Re: [PATCH] rockchip: board: Add minimal generic RK3588S/RK3588 board

2024-03-14 Thread Jonas Karlman
Hi Kever,

On 2024-03-14 07:58, Kever Yang wrote:
> 
> On 2024/2/1 06:08, Jonas Karlman wrote:
>> Add a minimal generic RK3588S/RK3588 board that only have eMMC and SDMMC
>> enabled. This defconfig can be used to boot from eMMC or SD-card on most
>> RK3588S/RK3588 boards that follow reference board design.
>>
>> Also fix the alphabetical order of RK3588 boards listed in Makefile and
>> documentation.
>>
>> Signed-off-by: Jonas Karlman 
> 
> Since rk356x already have a generic board, rk3588 can also have one.
> 
> Reviewed-by: Kever Yang 

Thanks, I will send a v2 with an updated defconfig to take advantage of
the new ROCKCHIP_COMMON_STACK_ADDR bss and stack addresses.

Regards,
Jonas

> 
> Thanks,
> - Kever
> 
>> ---
>> This patch depend on the series "rockchip: rk35xx: Sync device tree
>> with linux v6.8-rc1" [1].
>>
>> [1] https://patchwork.ozlabs.org/cover/1891669/
>> ---
>>   arch/arm/dts/Makefile   |  5 +-
>>   arch/arm/dts/rk3588-generic-u-boot.dtsi |  3 ++
>>   arch/arm/dts/rk3588-generic.dts | 44 
>>   board/rockchip/evb_rk3588/MAINTAINERS   |  7 +++
>>   configs/generic-rk3588_defconfig| 68 +
>>   doc/board/rockchip/rockchip.rst |  3 +-
>>   6 files changed, 127 insertions(+), 3 deletions(-)
>>   create mode 100644 arch/arm/dts/rk3588-generic-u-boot.dtsi
>>   create mode 100644 arch/arm/dts/rk3588-generic.dts
>>   create mode 100644 configs/generic-rk3588_defconfig
>>

[snip]


Re: [PATCH] board: rockchip: Add Pine64 PineTab2

2024-03-14 Thread Jonas Karlman
Hi Kever,

On 2024-03-14 07:50, Kever Yang wrote:
> 
> On 2024/2/5 01:30, Jonas Karlman wrote:
>> The Pine64 PineTab2 is a tablet computer based on the Rockchip RK3566
>> SoC. The table features 4/8 GB LPDDR4 RAM and 64/128 GB eMMC storage.
>>
>> Features tested on a Pine64 PineTab2 8GB v2.0:
>> - SD-card boot
>> - eMMC boot
>> - SPI Flash boot
>> - USB host
>>
>> Device tree is imported from linux maintainer branch v6.9-armsoc/dts64,
>> commit 1b7e19448f8f ("arm64: dts: rockchip: Add devicetree for Pine64
>> PineTab2").
>>
>> Signed-off-by: Jonas Karlman 
> Reviewed-by: Kever Yang 

Thanks, I will send a v2 with small adjustments to bootph flags used in
board u-boot.dtsi and update defconfig to take advantage of the new
ROCKCHIP_COMMON_STACK_ADDR bss and stack addresses.

Regards,
Jonas

> 
> Thanks,
> - Kever
>> ---
>> This patch depend on the series "rockchip: rk35xx: Sync device tree with
>> linux v6.8-rc1" [1].
>>
>> [1] https://patchwork.ozlabs.org/cover/1891669/
>> ---
>>   arch/arm/dts/Makefile |   2 +
>>   arch/arm/dts/rk3566-pinetab2-u-boot.dtsi  |  44 +
>>   arch/arm/dts/rk3566-pinetab2-v0.1-u-boot.dtsi |   3 +
>>   arch/arm/dts/rk3566-pinetab2-v0.1.dts |  28 +
>>   arch/arm/dts/rk3566-pinetab2-v2.0-u-boot.dtsi |   3 +
>>   arch/arm/dts/rk3566-pinetab2-v2.0.dts |  48 +
>>   arch/arm/dts/rk3566-pinetab2.dtsi | 943 ++
>>   board/pine64/quartz64_rk3566/MAINTAINERS  |  11 +
>>   configs/pinetab2-rk3566_defconfig | 119 +++
>>   doc/board/rockchip/rockchip.rst   |   1 +
>>   10 files changed, 1202 insertions(+)
>>   create mode 100644 arch/arm/dts/rk3566-pinetab2-u-boot.dtsi
>>   create mode 100644 arch/arm/dts/rk3566-pinetab2-v0.1-u-boot.dtsi
>>   create mode 100644 arch/arm/dts/rk3566-pinetab2-v0.1.dts
>>   create mode 100644 arch/arm/dts/rk3566-pinetab2-v2.0-u-boot.dtsi
>>   create mode 100644 arch/arm/dts/rk3566-pinetab2-v2.0.dts
>>   create mode 100644 arch/arm/dts/rk3566-pinetab2.dtsi
>>   create mode 100644 configs/pinetab2-rk3566_defconfig
>>

[snip]